diff --git a/docker/docker-compose.dev.yaml b/docker/docker-compose.dev.yaml index 27da1aee..e1c39d9d 100644 --- a/docker/docker-compose.dev.yaml +++ b/docker/docker-compose.dev.yaml @@ -72,7 +72,7 @@ services: postgres: condition: service_healthy meilisearch: - condition: service_started + condition: service_healthy volumes: # optional GitHub App private key: drop oxynote.private-key.pem into # docker/github/ and set OXYNOTE_CORE_GITHUB_APP_ID to enable the @@ -164,6 +164,12 @@ services: - MEILI_MASTER_KEY=devmasterkey - MEILI_ENV=development - MEILI_LOG_LEVEL=WARN + - MEILI_NO_ANALYTICS=true + healthcheck: + test: ["CMD", "curl", "--fail", "http://localhost:7700/health"] + interval: 5s + timeout: 5s + retries: 20 restart: unless-stopped logging: driver: local diff --git a/docker/prod/docker-compose.example.yaml b/docker/prod/docker-compose.example.yaml index 590d0a77..0911e320 100644 --- a/docker/prod/docker-compose.example.yaml +++ b/docker/prod/docker-compose.example.yaml @@ -66,6 +66,7 @@ services: - MEILI_ENV=production - MEILI_MASTER_KEY=change-me-meili-key - MEILI_LOG_LEVEL=WARN + - MEILI_NO_ANALYTICS=true volumes: - meilisearch_data:/meili_data healthcheck: diff --git a/e2e/docker-compose.dev.yaml b/e2e/docker-compose.dev.yaml index abbaea8e..e8964767 100644 --- a/e2e/docker-compose.dev.yaml +++ b/e2e/docker-compose.dev.yaml @@ -150,6 +150,7 @@ services: - MEILI_MASTER_KEY=devmasterkey - MEILI_ENV=development - MEILI_LOG_LEVEL=WARN + - MEILI_NO_ANALYTICS=true healthcheck: test: ["CMD", "curl", "--fail", "http://localhost:7700/health"] interval: 5s diff --git a/e2e/tests/search.test.ts b/e2e/tests/search.test.ts index f53af9c9..c5f89683 100644 --- a/e2e/tests/search.test.ts +++ b/e2e/tests/search.test.ts @@ -8,6 +8,15 @@ import { waitForEditor, } from "../helpers/editor" import { t } from "../helpers/i18n" +import { documentId } from "../helpers/api" +import { authorizeMCPClient, connectMCPClient } from "../helpers/mcp" +import { + branchLabel, + branchSwitcher, + makeReviewable, + switchToBranch, +} from "../helpers/review" +import { visit } from "../helpers/page" import { openSearch, searchFor } from "../helpers/search" import { signUpWithWorkspace } from "../helpers/workspace" @@ -56,6 +65,98 @@ test.describe("search", () => { await expect(page).toHaveURL(/New-Page-[a-z0-9]{20}/) }) + test("finds text written on a draft branch", async ({ page, request }) => { + await signUpWithWorkspace(page, request) + await createDocument(page) + await documentPersisted(page) + await makeReviewable(page) + await switchToBranch(page, "draft") + await contentEditor(page).click() + await page.keyboard.type("The marmalade committee convenes") + await documentPersisted(page) + await sidebarDocument(page, "Welcome to Oxynote!").click() + await expect(page).toHaveURL(/Welcome-to-Oxynote-[a-z0-9]{20}$/) + + const dialog = await openSearch(page) + const result = dialog.locator("a", { hasText: "marmalade" }) + await searchFor(dialog, "marmalade", result) + + // the text exists on the draft alone, so that is the branch the + // hit names and the one the link opens + await expect(result.getByTestId("search-result-branch")).toHaveText("draft") + await result.click() + + await expect(page).toHaveURL(/New-Page-[a-z0-9]{20}\?branch=[a-z0-9]{20}/) + await expect(branchSwitcher(page)).toContainText(branchLabel("draft"), { + timeout: 15_000, + }) + }) + + test("finds a metric block by its title", async ({ page, request }) => { + await signUpWithWorkspace(page, request) + await createDocument(page) + await documentPersisted(page) + const id = documentId(page) + + // a metric block has no text to type; its title is configuration, + // so the block goes in through the MCP surface the way an assistant + // would write it + const client = await connectMCPClient( + await authorizeMCPClient(page, request, "documents:read documents:write"), + ) + + const listing = await client.callTool({ + name: "list_documents", + arguments: {}, + }) + const listed = JSON.parse( + (listing.content as { text: string }[])[0]?.text ?? "{}", + ) as { documents: { id: string; default_branch_id: string }[] } + const branchId = listed.documents.find( + (d) => d.id === id, + )?.default_branch_id + expect(branchId).toBeDefined() + + const inserted = await client.callTool({ + name: "insert_block", + arguments: { + document_id: id, + branch_id: branchId, + position: "end", + // a metric lives in a grid; the root refuses a bare one + block: { + type: "metric_grid", + items: [ + { type: "metric", attrs: { title: "Pizza Fridays Scheduled" } }, + ], + }, + }, + }) + expect(inserted.isError, JSON.stringify(inserted.content)).toBeFalsy() + await client.close() + + // the authorization walked the page off the app, and the search + // starts from another page so that landing on the found one is + // observable in the URL + await visit(page, "/") + await sidebarDocument(page, "Welcome to Oxynote!").click() + await expect(page).toHaveURL(/Welcome-to-Oxynote-[a-z0-9]{20}$/) + + const dialog = await openSearch(page) + const result = dialog.locator("a", { hasText: "Pizza Fridays" }) + await searchFor(dialog, "Pizza", result) + await expect(result).toContainText("metricBlock") + + const href = await result.getAttribute("href") + const uid = decodeURIComponent(href?.split("#")[1] ?? "") + expect(uid).not.toBe("") + + await result.click() + + await expect(page).toHaveURL(/New-Page-[a-z0-9]{20}#/) + await expect(page.locator(`[id="${uid}"]`)).toBeInViewport() + }) + test("reports when nothing matches", async ({ page, request }) => { await signUpWithWorkspace(page, request) diff --git a/server/CLAUDE.md b/server/CLAUDE.md index 78388f4d..32c932c0 100644 --- a/server/CLAUDE.md +++ b/server/CLAUDE.md @@ -102,7 +102,7 @@ Uploaded images live under `organizations/{org}/documents/{doc}/files/{blockUID} - **Ordering is inverted between create and delete.** Creating writes the row before the object (upload, duplication); deleting removes the object before the row. Hooks follow the same rule from outside the transaction: `copyHooksToBranch` runs after the fork or merge commits, because `hook.NewHook` creates the watcher as a side effect and a rollback cannot take that back; a failed insert tears the just-created watcher down again. Both leave at most a row without an object, which the sweep resolves — an object without a row is invisible to every cleanup path. - **Duplication copies, never shares.** `Document.Duplicate` regenerates every block uid, rewrites image `src` paths to the new document (swapping only the path, never the host), and returns the old→new id map the handler uses to copy each object server-side into the duplicate's own folder. Files are therefore owned by exactly one document. - **Search jobs apply in order, per document.** A diff is a delta against the state its predecessors left, so the manager holds back every later diff of a document whose earlier one failed, rather than replaying it on top of newer work; an organization removal is global and holds everything after it. Without this a failed add replayed after a delete re-inserts the blocks of a document that no longer exists, and no later job would ever remove them. -- **Search index entries are removed from the ids the delete reports.** Only default-branch blocks are indexed, keyed by block uid with a `documentId` field. `agent.DeleteDocument` collects the subtree ids before the row goes away and returns them, and the deletion paths queue a search job carrying them through `search.Jobs` in the same transaction, so the manager clears the index by `documentId` filter — the descendants the cascade destroys have no other trace to clean up by. +- **Search index entries are branch-scoped.** Every branch of every document is indexed; an entry's id is `-` (`-docname` for the name entry), since a fork copies its source's uids, and it carries `documentId`, `branchId`, `branchName` and `branchDefault`. Every path that changes a branch queues its diff through `search.Jobs` in the same transaction: a persist on any branch, a fork (a full add of the new branch), a rename (every entry of the branch changes, since each carries the name), a merge (the target's diff). A branch deletion queues a `RemovedBranches` removal the manager applies by `branchId` filter, and a document deletion clears every branch at once: `agent.DeleteDocument` collects the subtree ids before the row goes away and returns them, so the manager clears the index by `documentId` filter — the descendants the cascade destroys have no other trace to clean up by. Index settings are declared in one object in `search.Client` and sent only when they differ from what the index reports, since a searchable or filterable attribute change re-indexes everything; every write task is awaited, so a batch Meilisearch rejects fails its job and is retried in order. - History entries pin the files they reference, so `DB_MAX_DOCUMENT_HISTORY_ENTRIES` (count, trimmed on insert) and `DB_DOCUMENT_HISTORY_RETENTION` (age, trimmed in the sweep) also decide how long a removed image survives. The ProseMirror schema is defined in `server/auth-realtime/src/schema/` (one file per block kind, `index.ts` aggregates). The Go-side mirror is `server/core/internal/document/node.go` (`RootBlock`, `Block`, `Mark`). @@ -201,16 +201,16 @@ onto that. Depth in those rows counts from the block itself. **Branches are addressed by id, always.** Every content tool (`get_document`, `read_block`, the block writes) requires `branch_id`; the default branch is a branch like any other, and -its id travels with every listing (`list_documents` and search hits carry -`default_branch_id`, `create_document` returns `branch_id`, `get_document` lists every -branch with its id). `Input.Branch`, `DocumentContent`, `DocumentBlock`, `ApplyEdit` and the +its id travels with every listing (`list_documents` carries `default_branch_id`, a search +hit carries the `branch_id` it was found on, `create_document` returns `branch_id`, +`get_document` lists every branch with its id). `Input.Branch`, `DocumentContent`, `DocumentBlock`, `ApplyEdit` and the placement checks take the branch id and resolve it through `FetchDocumentByBranchID`, refusing a branch that belongs to another document; an unknown id is refused with the branches the document has (`ErrUnknownBranch`, each as "name (id)"), and a protected branch reads but refuses every write, naming the unprotected branches to write to instead. `Input.Document` is the default-branch fetch the document-level tools use to name and -change a document; they and search stay branch-free, and only the default branch is -indexed. No tool creates, renames, deletes or merges a branch. MCP resources name a +change a document; they stay branch-free. Search covers every branch and each hit names +its own. No tool creates, renames, deletes or merges a branch. MCP resources name a document and a branch (`oxynote://documents/{id}/branches/{branch_id}`); the list carries each document on its default branch. The chat client reports the document and branch it is viewing; the session stores both unchecked, as a hint the next call verifies. diff --git a/server/core/internal/assistant/prompt.go b/server/core/internal/assistant/prompt.go index 228c7607..9a04abb3 100644 --- a/server/core/internal/assistant/prompt.go +++ b/server/core/internal/assistant/prompt.go @@ -34,7 +34,7 @@ Keep replies focused and brief: lead with the answer, keep caveats short, and us Read tools run immediately. Write tools wait for the user's confirmation, and every write in a turn shares one confirmation, so make all the related edits in the same turn. Independent calls also go in one turn. Never invent a uid, document id or parameter value; read it first. -Read before you write: find the document, read it with get_document, and read a block in full only when you are about to edit its inner structure. Documents have branches, and every read or write of content names one by branch_id: list_documents and search hits carry each document's default_branch_id, and get_document lists every branch of a document with its id. A protected branch is read-only, so write to another branch of the document or say there is none rather than retrying. +Read before you write: find the document, read it with get_document, and read a block in full only when you are about to edit its inner structure. Documents have branches, and every read or write of content names one by branch_id: list_documents carries each document's default_branch_id, a search hit carries the branch_id it was found on, and get_document lists every branch of a document with its id. A protected branch is read-only, so write to another branch of the document or say there is none rather than retrying. The data-source tools are read-only. Answer a question about the data from the query results; add a metric block only when the user asks for a chart, a metric or a dashboard in a document. Discover metric, label and table names with the metadata tools instead of guessing them, and run the query with the chart_type you intend before writing a metric block, so you know it renders. @@ -168,7 +168,7 @@ const _basePrompt = _personaSection + _blockModelSection + _etiquetteSection + _ // instead of waiting on a confirmation. const _mcpIntroSection = `You are connected to Oxynote, a collaborative product for writing technical documentation. The tools operate on the documents of one organisation, which are also listed as resources. Blocks are the unit of content, addressed by uid; comments, hooks and files hang off those uids. Documents are written for humans first, balancing prose with technical detail where it sharpens meaning. -Read before you write: find the document, read it with get_document, and read a block in full only when you are about to edit its inner structure. Documents have branches, and every read or write of content names one by branch_id: list_documents and search hits carry each document's default_branch_id, and get_document lists every branch of a document with its id. Write tools apply immediately, and a protected branch is read-only, so write to another branch of the document. The data-source tools are read-only; discover metric, label and table names with the metadata tools instead of guessing them, and run a query with the chart_type you intend before writing a metric block, so you know it renders. +Read before you write: find the document, read it with get_document, and read a block in full only when you are about to edit its inner structure. Documents have branches, and every read or write of content names one by branch_id: list_documents carries each document's default_branch_id, a search hit carries the branch_id it was found on, and get_document lists every branch of a document with its id. Write tools apply immediately, and a protected branch is read-only, so write to another branch of the document. The data-source tools are read-only; discover metric, label and table names with the metadata tools instead of guessing them, and run a query with the chart_type you intend before writing a metric block, so you know it renders. ` diff --git a/server/core/internal/assistant/tools/schema.go b/server/core/internal/assistant/tools/schema.go index f73d0e8b..acf41f8d 100644 --- a/server/core/internal/assistant/tools/schema.go +++ b/server/core/internal/assistant/tools/schema.go @@ -34,7 +34,7 @@ const ( // _descBranchID describes the branch-id property every content tool // takes. - _descBranchID = "The id of the branch to read or write: a document's default_branch_id from list_documents or a search hit, or any id from the branches get_document lists. A protected branch can be read but refuses every write." + _descBranchID = "The id of the branch to read or write: a document's default_branch_id from list_documents, a search hit's branch_id, or any id from the branches get_document lists. A protected branch can be read but refuses every write." // _keyDocumentID is the shared document-id property name. _keyDocumentID = "document_id" diff --git a/server/core/internal/assistant/tools/search.go b/server/core/internal/assistant/tools/search.go index 5738d4d6..5180b015 100644 --- a/server/core/internal/assistant/tools/search.go +++ b/server/core/internal/assistant/tools/search.go @@ -3,6 +3,7 @@ package tools import ( "fmt" "log/slog" + "strings" "github.com/oxynote/oxynote/server/core/internal/document" "github.com/rs/xid" @@ -46,7 +47,7 @@ type searchDocuments struct { func (searchDocuments) Info() Info { return Info{ Name: NameSearchDocuments, - Description: "Full-text search across the default branch of every document in the organisation for blocks whose text matches the query; other branches are not indexed. Returns hits with document_id, document_name, default_branch_id, block_uid and text; the branch id is what a follow-up get_document or read_block takes. Use it to find where a topic is discussed or which documents the user might mean; use list_documents when you know the title. A hit names the innermost block holding the text, which may sit below anything get_document lists; read_block resolves it.", + Description: "Full-text search across every branch of every document in the organisation for blocks whose text matches the query. Returns hits with document_id, document_name, branch_id, branch_name, default (whether that branch is the document's default), block_uid and text; branch_id is what a follow-up get_document or read_block takes. A block a fork copied unchanged is found on every branch that holds it. Use it to find where a topic is discussed or which documents the user might mean; use list_documents when you know the title. A hit names the innermost block holding the text, which may sit below anything get_document lists; read_block resolves it.", Properties: map[string]any{ _keyQuery: stringProp("The full-text search query (typo-tolerant, matches block text)."), "limit": map[string]any{ @@ -96,8 +97,8 @@ func (searchDocuments) Execute(inp Input) (string, error) { return "", fmt.Errorf("search_documents: search: %w", err) } - // the index stores block text keyed by (document, block uid) but not - // display names; join names in from the document tree so the AI can + // the index stores block text keyed by (branch, block uid) but not + // document names; join names in from the document tree so the AI can // talk about hits without a follow-up lookup per document. Zero hits // have nothing to decorate, so the fetch is skipped. names := map[xid.ID]document.Summary{} @@ -123,11 +124,13 @@ func (searchDocuments) Execute(inp Input) (string, error) { for _, b := range blocks { hits = append(hits, searchHit{ - DocumentID: b.DocumentID, - DocumentName: names[b.DocumentID].DocumentName, - DefaultBranchID: names[b.DocumentID].DefaultBranchID, - BlockUID: b.ID, - Text: b.Text, + DocumentID: b.DocumentID, + DocumentName: names[b.DocumentID].DocumentName, + BranchID: b.BranchID, + BranchName: b.BranchName, + Default: b.BranchDefault, + BlockUID: strings.TrimPrefix(b.ID, b.BranchID.String()+"-"), + Text: b.Text, }) } @@ -152,10 +155,15 @@ type searchHit struct { // and the search. DocumentName string `json:"document_name,omitempty"` - // DefaultBranchID is the branch the hit was indexed from, which is - // the document's default one and what a follow-up read takes; zero - // when the tree lookup failed. - DefaultBranchID xid.ID `json:"default_branch_id,omitzero"` + // BranchID is the branch the hit was indexed from, which is what a + // follow-up read takes. + BranchID xid.ID `json:"branch_id"` + + // BranchName is that branch's name. + BranchName string `json:"branch_name"` + + // Default reports whether that branch is the document's default. + Default bool `json:"default"` // BlockUID is the matching block's uid attribute, usable with // read_block and the edit tools. diff --git a/server/core/internal/assistant/tools/search_test.go b/server/core/internal/assistant/tools/search_test.go index ae89b4cc..24f7f3f8 100644 --- a/server/core/internal/assistant/tools/search_test.go +++ b/server/core/internal/assistant/tools/search_test.go @@ -54,7 +54,9 @@ func Test_searchDocuments_Title(t *testing.T) { func Test_searchDocuments_Execute(t *testing.T) { t.Parallel() - docID := xid.New() + docID, branchID := xid.New(), xid.New() + + branchFields := `"branch_id":"` + branchID.String() + `","branch_name":"draft","default":false,` hitSearcher := func(limit *int) *SearcherMock { return &SearcherMock{ @@ -63,7 +65,14 @@ func Test_searchDocuments_Execute(t *testing.T) { *limit = l } - return []search.Block{{DocumentID: docID, ID: "b1", Text: "rate limiting"}}, nil + return []search.Block{{ + DocumentID: docID, + BranchID: branchID, + BranchName: "draft", + BranchDefault: false, + ID: branchID.String() + "-b1", + Text: "rate limiting", + }}, nil }, } } @@ -120,7 +129,7 @@ func Test_searchDocuments_Execute(t *testing.T) { Args: `{"query":"rate limit"}`, Limit: _searchLimitDefault, Result: `{"hits":[{"document_id":"` + docID.String() + `","document_name":"Runbook",` + - `"block_uid":"b1","text":"rate limiting"}]}`, + branchFields + `"block_uid":"b1","text":"rate limiting"}]}`, }, "Losing the names does not fail the search": { Searcher: hitSearcher(nil), @@ -131,7 +140,7 @@ func Test_searchDocuments_Execute(t *testing.T) { }, Args: `{"query":"rate limit"}`, Limit: _searchLimitDefault, - Result: `{"hits":[{"document_id":"` + docID.String() + `","block_uid":"b1","text":"rate limiting"}]}`, + Result: `{"hits":[{"document_id":"` + docID.String() + `",` + branchFields + `"block_uid":"b1","text":"rate limiting"}]}`, Logged: "cannot fetch the document tree for search hit names", }, "Requested limit is honoured": { @@ -139,7 +148,7 @@ func Test_searchDocuments_Execute(t *testing.T) { DB: &DBMock{}, Args: `{"query":"rate limit","limit":5}`, Limit: 5, - Result: `{"hits":[{"document_id":"` + docID.String() + `","block_uid":"b1",` + + Result: `{"hits":[{"document_id":"` + docID.String() + `",` + branchFields + `"block_uid":"b1",` + `"text":"rate limiting"}]}`, }, "Oversized limit is clipped": { @@ -147,7 +156,7 @@ func Test_searchDocuments_Execute(t *testing.T) { DB: &DBMock{}, Args: `{"query":"rate limit","limit":5000}`, Limit: _searchLimitMax, - Result: `{"hits":[{"document_id":"` + docID.String() + `","block_uid":"b1",` + + Result: `{"hits":[{"document_id":"` + docID.String() + `",` + branchFields + `"block_uid":"b1",` + `"text":"rate limiting"}]}`, }, } diff --git a/server/core/internal/assistant/tools/testdata/tool_schemas.golden b/server/core/internal/assistant/tools/testdata/tool_schemas.golden index 779855b8..25c6c8e8 100644 --- a/server/core/internal/assistant/tools/testdata/tool_schemas.golden +++ b/server/core/internal/assistant/tools/testdata/tool_schemas.golden @@ -1 +1 @@ -{"create_document":{"desc":"Create a new document and return {document_id, branch_id}, the branch_id being its default branch. The document starts with a single empty paragraph, so follow up with insert_block calls (position end, that branch_id) in the same turn to fill it. Omit parent_id to create it at the organisation root.","has_params_one_of":true,"json_schema":{"properties":{"icon":{"description":"Iconify identifier, as \"collection:name\". The product's own icons are MingCute fills (e.g. \"mingcute:file-code-fill\"); prefer one so the document matches the rest of the sidebar. Defaults to \"mingcute:document-2-fill\" when empty.","type":"string"},"name":{"description":"Display name for the new document.","type":"string"},"parent_id":{"description":"Optional parent document id. Omit to create at the org root.","type":"string"}},"required":["name"],"type":"object"},"name":"create_document"},"delete_block":{"desc":"Delete one block by uid, including anything nested inside it. Its comments, hooks and files go with it and cannot be restored, so use move_block when the aim is to reorder and update_block_text when the aim is new wording. Returns {deleted: uid}.","has_params_one_of":true,"json_schema":{"properties":{"block_uid":{"description":"The uid of the block to delete.","type":"string"},"branch_id":{"description":"The id of the branch to read or write: a document's default_branch_id from list_documents or a search hit, or any id from the branches get_document lists. A protected branch can be read but refuses every write.","type":"string"},"document_id":{"description":"The target document id.","type":"string"}},"required":["document_id","branch_id","block_uid"],"type":"object"},"name":"delete_block"},"delete_document":{"desc":"Delete a document and every document nested under it. The whole subtree goes and cannot be restored, so check the tree with list_documents first, and use update_document when the aim is to relocate rather than remove. Returns {document_id, deleted}.","has_params_one_of":true,"json_schema":{"properties":{"document_id":{"description":"The document id to delete.","type":"string"}},"required":["document_id"],"type":"object"},"name":"delete_document"},"get_document":{"desc":"Read one document on one branch: its name, icon, parent_id, protected flag and updated_at, the branch read and every branch the document has (each with the id a branch_id argument takes), followed by one row per block with the block's uid, kind, flattened text, depth, parent_uid and the few attrs that matter for reading (heading level, callout icon, code language, task checked). Use it as the way to read a document before editing it; branch_id is the default_branch_id a listing or search hit carries, or any id from branches. A protected branch can be read but refuses every write, so pick an unprotected branch id from branches to write to. Rows marked has_children hold nested blocks the rows do not list, and read_block returns those when you need them.","has_params_one_of":true,"json_schema":{"properties":{"branch_id":{"description":"The id of the branch to read or write: a document's default_branch_id from list_documents or a search hit, or any id from the branches get_document lists. A protected branch can be read but refuses every write.","type":"string"},"document_id":{"description":"The document id.","type":"string"}},"required":["document_id","branch_id"],"type":"object"},"name":"get_document"},"get_prometheus_metadata":{"desc":"List the metrics a Prometheus data source exposes, with each metric's type, help text and unit. Use it to find the metric names to write a PromQL query against instead of guessing them.","has_params_one_of":true,"json_schema":{"properties":{"data_source_id":{"description":"The data source id, from list_data_sources.","type":"string"}},"required":["data_source_id"],"type":"object"},"name":"get_prometheus_metadata"},"get_sql_metadata":{"desc":"List the tables and their columns in a PostgreSQL, MariaDB or MySQL data source, plus the default schema. Read it before writing a query so the table and column names are the real ones.","has_params_one_of":true,"json_schema":{"properties":{"data_source_id":{"description":"The data source id, from list_data_sources.","type":"string"}},"required":["data_source_id"],"type":"object"},"name":"get_sql_metadata"},"get_sql_query_labels":{"desc":"Run a SQL query limited to one row and return its string columns with an example value each, which are the columns a chart would treat as series labels. Use it to check what a query returns before charting it; it is cheaper than query_sql.","has_params_one_of":true,"json_schema":{"properties":{"data_source_id":{"description":"The data source id, from list_data_sources.","type":"string"},"from":{"description":"Optional. Range start as an RFC3339 timestamp. Defaults to an hour before 'to'.","type":"string"},"query":{"description":"The SQL query to probe. $__ macros are expanded as they are for a metric block.","type":"string"},"to":{"description":"Optional. Range end as an RFC3339 timestamp. Defaults to now.","type":"string"}},"required":["data_source_id","query"],"type":"object"},"name":"get_sql_query_labels"},"insert_block":{"desc":"Insert one canonical block into a document. position start or end puts it at the document's start or end; before or after puts it beside the block reference_block_uid names, which stays in place. The block has to be legal where it lands: the document root takes every type except titled_code, metric and split_doc_param_list, and a reference inside a split_doc or metric_grid takes only what that container holds. Returns the summary rows of the new block and everything nested in it, uids included, with depth counted from the block itself.","has_params_one_of":true,"json_schema":{"properties":{"block":{"description":"A canonical block. Which fields apply depends on type; supply only that type's own.","properties":{"attrs":{"description":"Per-type attributes. Required where the type says so: heading level, titled_code title, image and figma src. Also code language, callout icon, image alt and width, and a metric's chart configuration, whose width is compact, standard or wide.","properties":{"level":{"description":"heading: the level. Required on a heading.","enum":[1,2,3],"type":"integer"},"refreshInterval":{"description":"metric: how often the chart re-queries.","enum":["5s","10s","30s","1m","5m","15m","30m","1h","2h","1d"],"type":"string"},"simulationPreset":{"description":"metric: draws this generated series in place of the query's own result, for a block documenting a metric that has no real data yet. Omit it to chart the query.","enum":["cpu_usage","memory_usage","disk_usage","http_requests","http_latency","error_rate"],"type":"string"},"timeRange":{"description":"metric: the window queried.","enum":["last_5_minutes","last_15_minutes","last_30_minutes","last_1_hour","last_3_hours","last_6_hours","last_12_hours","last_24_hours","last_2_days","last_7_days","last_30_days","last_90_days","last_6_months","last_1_year","last_2_years","last_5_years","today","yesterday","today_so_far","this_week","this_week_so_far","this_month","this_month_so_far","this_year","this_year_so_far","previous_week","previous_month","previous_year"],"type":"string"},"unitType":{"description":"metric: the unit values are read in. With custom, put the label in unitCustom.","enum":["custom","nanoseconds","microseconds","milliseconds","seconds","minutes","hours","days","bytes","kilobytes","megabytes","gigabytes","terabytes","bits","kilobits","megabits","gigabits","terabits","percent0to100","percent0to1"],"type":"string"},"visualizationType":{"description":"metric: the chart kind.","enum":["line_chart","bar_chart","gauge_chart"],"type":"string"}},"type":"object"},"children":{"description":"Blocks nested under a single list entry. A list entry is a paragraph; anything else nested under it goes here, never in the entry itself and never in a second entry.","type":"array"},"header":{"description":"split_doc_param_list heading, as plain text.","type":"string"},"items":{"description":"Entries of bullet_list and ordered_list, which are paragraphs; items of metric_grid, and of blockquote or callout when not using text.","type":"array"},"left":{"description":"split_doc concept side. Must start with a level-1 heading; split_doc_param_list is legal only here.","type":"array"},"params":{"description":"split_doc_param_list rows, each {name, type, description}.","type":"array"},"right":{"description":"split_doc example side. Holds titled_code or metric only, and is the one place titled_code is legal.","type":"array"},"task_items":{"description":"Rows of task_list, each {checked, block}.","type":"array"},"text":{"description":"Inline text for paragraph, heading, blockquote, code, titled_code, mermaid and callout. Raw inside code, titled_code and mermaid; minimal markdown elsewhere. One block is one paragraph; a newline inside text does not start a new one.","type":"string"},"type":{"description":"The block's canonical type.","enum":["paragraph","heading","blockquote","bullet_list","ordered_list","task_list","callout","code","titled_code","mermaid","horizontal_rule","image","figma","metric","metric_grid","split_doc","split_doc_param_list"],"type":"string"},"uid":{"description":"Leave unset. Uids are generated server-side and are the handle later edits use.","type":"string"}},"required":["type"],"type":"object"},"branch_id":{"description":"The id of the branch to read or write: a document's default_branch_id from list_documents or a search hit, or any id from the branches get_document lists. A protected branch can be read but refuses every write.","type":"string"},"document_id":{"description":"The target document id.","type":"string"},"position":{"description":"Where the block lands: before or after the reference block, or at the start or end of the document.","enum":["before","after","start","end"],"type":"string"},"reference_block_uid":{"description":"The uid of the block to insert beside. Required with before and after; leave it out with start and end.","type":"string"}},"required":["document_id","branch_id","position","block"],"type":"object"},"name":"insert_block"},"list_data_sources":{"desc":"List the organisation's data sources. Returns id, name, type (prometheus, postgresql, mariadb, mysql) and connection status. Start here: every other data-source tool takes an id from this list, and the type decides which of them serves it.","has_params_one_of":true,"json_schema":{"properties":{},"type":"object"},"name":"list_data_sources"},"list_documents":{"desc":"List the organisation's documents as a tree of {id, name, default_branch_id, children}; default_branch_id is what get_document and the block tools take as branch_id for the document's default branch. Use it to find a document by name or to see what sits under a parent; use search_documents when you are looking for content rather than a title. Pass parent_id to get only that document's direct children, or omit it for the whole organisation.","has_params_one_of":true,"json_schema":{"properties":{"parent_id":{"description":"Optional. Return only the direct children of this document. Omit for the full tree.","type":"string"}},"type":"object"},"name":"list_documents"},"list_prometheus_label_names":{"desc":"List the label names present in a Prometheus data source, optionally only those on the series the matchers select. Pair it with list_prometheus_label_values to build a filtered PromQL query.","has_params_one_of":true,"json_schema":{"properties":{"data_source_id":{"description":"The data source id, from list_data_sources.","type":"string"},"from":{"description":"Optional. Range start as an RFC3339 timestamp. Defaults to an hour before 'to'.","type":"string"},"matchers":{"description":"Optional. PromQL series selectors, e.g. [\"up\", \"{job=\\\"api\\\"}\"].","items":{"type":"string"},"type":"array"},"to":{"description":"Optional. Range end as an RFC3339 timestamp. Defaults to now.","type":"string"}},"required":["data_source_id"],"type":"object"},"name":"list_prometheus_label_names"},"list_prometheus_label_values":{"desc":"List the values a label takes in a Prometheus data source, optionally only on the series the matchers select. Use it to discover the concrete label values a query should filter on.","has_params_one_of":true,"json_schema":{"properties":{"data_source_id":{"description":"The data source id, from list_data_sources.","type":"string"},"from":{"description":"Optional. Range start as an RFC3339 timestamp. Defaults to an hour before 'to'.","type":"string"},"label":{"description":"The label whose values to list, e.g. \"job\".","type":"string"},"matchers":{"description":"Optional. PromQL series selectors, e.g. [\"up\", \"{job=\\\"api\\\"}\"].","items":{"type":"string"},"type":"array"},"to":{"description":"Optional. Range end as an RFC3339 timestamp. Defaults to now.","type":"string"}},"required":["data_source_id","label"],"type":"object"},"name":"list_prometheus_label_values"},"list_prometheus_series":{"desc":"List the series matching a set of selectors in a Prometheus data source, each as its full label set. Use it to see which label combinations a metric actually has before querying it.","has_params_one_of":true,"json_schema":{"properties":{"data_source_id":{"description":"The data source id, from list_data_sources.","type":"string"},"from":{"description":"Optional. Range start as an RFC3339 timestamp. Defaults to an hour before 'to'.","type":"string"},"matchers":{"description":"PromQL series selectors, e.g. [\"up\", \"{job=\\\"api\\\"}\"].","items":{"type":"string"},"type":"array"},"to":{"description":"Optional. Range end as an RFC3339 timestamp. Defaults to now.","type":"string"}},"required":["data_source_id","matchers"],"type":"object"},"name":"list_prometheus_series"},"move_block":{"desc":"Move an existing block before or after another block in the same document. The block keeps its uid, attrs and nested content, so comments, hooks and files attached to it stay attached; deleting it and inserting a copy would lose them. The landing spot has to accept the block's type, by the same rule insert_block applies. Returns the summary rows of the moved block and everything nested in it.","has_params_one_of":true,"json_schema":{"properties":{"block_uid":{"description":"The uid of the block to move.","type":"string"},"branch_id":{"description":"The id of the branch to read or write: a document's default_branch_id from list_documents or a search hit, or any id from the branches get_document lists. A protected branch can be read but refuses every write.","type":"string"},"document_id":{"description":"The target document id.","type":"string"},"position":{"description":"Landing side relative to the reference block.","enum":["before","after"],"type":"string"},"reference_block_uid":{"description":"The uid of the block to move relative to.","type":"string"}},"required":["document_id","branch_id","block_uid","position","reference_block_uid"],"type":"object"},"name":"move_block"},"query_prometheus":{"desc":"Run a PromQL range query against a Prometheus data source over the given window. Returns the raw result by default; with chart_type set it instead describes what a metric block would render, which is how to check a query before writing it into a block.","has_params_one_of":true,"json_schema":{"properties":{"chart_type":{"description":"Optional. One of line_chart, bar_chart, gauge_chart. When set, the result describes what the metric block would draw (render status, series count, and each series' labels, point count and endpoints) instead of the raw data. Use it to check a query before putting it in a metric block; omit it when you need the values themselves.","type":"string"},"data_source_id":{"description":"The data source id, from list_data_sources.","type":"string"},"from":{"description":"Optional. Range start as an RFC3339 timestamp. Defaults to an hour before 'to'.","type":"string"},"query":{"description":"The PromQL expression to run.","type":"string"},"to":{"description":"Optional. Range end as an RFC3339 timestamp. Defaults to now.","type":"string"}},"required":["data_source_id","query"],"type":"object"},"name":"query_prometheus"},"query_sql":{"desc":"Run a read-only query against a PostgreSQL, MariaDB or MySQL data source. Returns columns and rows by default; with chart_type set it instead describes what a metric block would render, which is how to check a query before writing it into a block. $__ macros ($__timeFilter, $__timeGroupAlias and the rest) are expanded against the window.","has_params_one_of":true,"json_schema":{"properties":{"chart_type":{"description":"Optional. One of line_chart, bar_chart, gauge_chart. When set, the result describes what the metric block would draw (render status, series count, and each series' labels, point count and endpoints) instead of the raw data. Use it to check a query before putting it in a metric block; omit it when you need the values themselves.","type":"string"},"data_source_id":{"description":"The data source id, from list_data_sources.","type":"string"},"from":{"description":"Optional. Range start as an RFC3339 timestamp. Defaults to an hour before 'to'.","type":"string"},"query":{"description":"The SQL query to run. For a chart, select a time column aliased \"time\" plus one or more numeric columns.","type":"string"},"to":{"description":"Optional. Range end as an RFC3339 timestamp. Defaults to now.","type":"string"}},"required":["data_source_id","query"],"type":"object"},"name":"query_sql"},"read_block":{"desc":"Return the full canonical content of one block by uid, including any nested children. Use it only when get_document's rows are not enough: to edit a split_doc, a nested list or a split_doc_param_list, whose inner structure has to be written back in full. Fails when the uid is not in the document.","has_params_one_of":true,"json_schema":{"properties":{"block_uid":{"description":"The block uid to fetch.","type":"string"},"branch_id":{"description":"The id of the branch to read or write: a document's default_branch_id from list_documents or a search hit, or any id from the branches get_document lists. A protected branch can be read but refuses every write.","type":"string"},"document_id":{"description":"The document id.","type":"string"}},"required":["document_id","branch_id","block_uid"],"type":"object"},"name":"read_block"},"read_tool_output":{"desc":"Retrieve the full output of an earlier tool call that was too large to keep in the conversation. Pass the path shown in the truncation notice.","has_params_one_of":true,"json_schema":{"properties":{"file_path":{"description":"The stored output path from the truncation notice.","type":"string"}},"required":["file_path"],"type":"object"},"name":"read_tool_output"},"replace_block":{"desc":"Replace a block by uid with a new block in the same position. The old block's uid, content and children are all gone unless the new block carries them, so use it to change a block's type or its whole structure. For a wording change use update_block_text, and for an attribute change update_block_attrs; both keep the uid, which comments, hooks and files hang off. Returns the summary rows of the new block and everything nested in it, uids included, with depth counted from the block itself.","has_params_one_of":true,"json_schema":{"properties":{"block":{"description":"A canonical block. Which fields apply depends on type; supply only that type's own.","properties":{"attrs":{"description":"Per-type attributes. Required where the type says so: heading level, titled_code title, image and figma src. Also code language, callout icon, image alt and width, and a metric's chart configuration, whose width is compact, standard or wide.","properties":{"level":{"description":"heading: the level. Required on a heading.","enum":[1,2,3],"type":"integer"},"refreshInterval":{"description":"metric: how often the chart re-queries.","enum":["5s","10s","30s","1m","5m","15m","30m","1h","2h","1d"],"type":"string"},"simulationPreset":{"description":"metric: draws this generated series in place of the query's own result, for a block documenting a metric that has no real data yet. Omit it to chart the query.","enum":["cpu_usage","memory_usage","disk_usage","http_requests","http_latency","error_rate"],"type":"string"},"timeRange":{"description":"metric: the window queried.","enum":["last_5_minutes","last_15_minutes","last_30_minutes","last_1_hour","last_3_hours","last_6_hours","last_12_hours","last_24_hours","last_2_days","last_7_days","last_30_days","last_90_days","last_6_months","last_1_year","last_2_years","last_5_years","today","yesterday","today_so_far","this_week","this_week_so_far","this_month","this_month_so_far","this_year","this_year_so_far","previous_week","previous_month","previous_year"],"type":"string"},"unitType":{"description":"metric: the unit values are read in. With custom, put the label in unitCustom.","enum":["custom","nanoseconds","microseconds","milliseconds","seconds","minutes","hours","days","bytes","kilobytes","megabytes","gigabytes","terabytes","bits","kilobits","megabits","gigabits","terabits","percent0to100","percent0to1"],"type":"string"},"visualizationType":{"description":"metric: the chart kind.","enum":["line_chart","bar_chart","gauge_chart"],"type":"string"}},"type":"object"},"children":{"description":"Blocks nested under a single list entry. A list entry is a paragraph; anything else nested under it goes here, never in the entry itself and never in a second entry.","type":"array"},"header":{"description":"split_doc_param_list heading, as plain text.","type":"string"},"items":{"description":"Entries of bullet_list and ordered_list, which are paragraphs; items of metric_grid, and of blockquote or callout when not using text.","type":"array"},"left":{"description":"split_doc concept side. Must start with a level-1 heading; split_doc_param_list is legal only here.","type":"array"},"params":{"description":"split_doc_param_list rows, each {name, type, description}.","type":"array"},"right":{"description":"split_doc example side. Holds titled_code or metric only, and is the one place titled_code is legal.","type":"array"},"task_items":{"description":"Rows of task_list, each {checked, block}.","type":"array"},"text":{"description":"Inline text for paragraph, heading, blockquote, code, titled_code, mermaid and callout. Raw inside code, titled_code and mermaid; minimal markdown elsewhere. One block is one paragraph; a newline inside text does not start a new one.","type":"string"},"type":{"description":"The block's canonical type.","enum":["paragraph","heading","blockquote","bullet_list","ordered_list","task_list","callout","code","titled_code","mermaid","horizontal_rule","image","figma","metric","metric_grid","split_doc","split_doc_param_list"],"type":"string"},"uid":{"description":"Leave unset. Uids are generated server-side and are the handle later edits use.","type":"string"}},"required":["type"],"type":"object"},"block_uid":{"description":"The uid of the block being replaced.","type":"string"},"branch_id":{"description":"The id of the branch to read or write: a document's default_branch_id from list_documents or a search hit, or any id from the branches get_document lists. A protected branch can be read but refuses every write.","type":"string"},"document_id":{"description":"The target document id.","type":"string"}},"required":["document_id","branch_id","block_uid","block"],"type":"object"},"name":"replace_block"},"search_documents":{"desc":"Full-text search across the default branch of every document in the organisation for blocks whose text matches the query; other branches are not indexed. Returns hits with document_id, document_name, default_branch_id, block_uid and text; the branch id is what a follow-up get_document or read_block takes. Use it to find where a topic is discussed or which documents the user might mean; use list_documents when you know the title. A hit names the innermost block holding the text, which may sit below anything get_document lists; read_block resolves it.","has_params_one_of":true,"json_schema":{"properties":{"limit":{"description":"Maximum number of hits to return. Defaults to 20; cap is 50.","type":"integer"},"query":{"description":"The full-text search query (typo-tolerant, matches block text).","type":"string"}},"required":["query"],"type":"object"},"name":"search_documents"},"update_block_attrs":{"desc":"Set or override named attributes on an existing block, such as a heading's level or a callout's icon. Attributes not mentioned are kept and uid cannot change. Values are validated for the block's type, so a level outside 1 to 3 or a metric width other than compact, standard or wide is rejected. Use replace_block when the type itself has to change. Returns the summary rows of the block and everything nested in it, attrs as they now stand.","has_params_one_of":true,"json_schema":{"properties":{"attrs":{"description":"Attribute keys and values to set (e.g. {\"level\": 2}, {\"icon\": \"lucide:warning\"}).","type":"object"},"block_uid":{"description":"The uid of the block whose attrs should be updated.","type":"string"},"branch_id":{"description":"The id of the branch to read or write: a document's default_branch_id from list_documents or a search hit, or any id from the branches get_document lists. A protected branch can be read but refuses every write.","type":"string"},"document_id":{"description":"The target document id.","type":"string"}},"required":["document_id","branch_id","block_uid","attrs"],"type":"object"},"name":"update_block_attrs"},"update_block_text":{"desc":"Replace the inline text of one text-bearing block: paragraph, heading, blockquote, code, titled_code, mermaid, or a callout written with text. Type, attrs and uid are kept, so this is the tool for wording changes. Text follows the canonical markdown subset (**bold**, *italic*, _underline_, ~~strike~~, backtick code, [label](url)), and is raw in code, titled_code and mermaid. One block is one paragraph; to add a paragraph, insert a block instead. Returns the block's summary row with the new text.","has_params_one_of":true,"json_schema":{"properties":{"block_uid":{"description":"The uid of the block whose text should be replaced.","type":"string"},"branch_id":{"description":"The id of the branch to read or write: a document's default_branch_id from list_documents or a search hit, or any id from the branches get_document lists. A protected branch can be read but refuses every write.","type":"string"},"document_id":{"description":"The target document id.","type":"string"},"text":{"description":"New inline text in canonical markdown.","type":"string"}},"required":["document_id","branch_id","block_uid","text"],"type":"object"},"name":"update_block_text"},"update_document":{"desc":"Change a document's name, icon or place in the tree, any combination in one call; content is untouched. Give only the fields to change: name is the new display name, icon an Iconify identifier as \"collection:name\" (the product's own icons are MingCute fills, e.g. \"mingcute:rocket-fill\", so prefer one to match the sidebar), and parent_id the new parent, or an empty string for the organisation root. A call with none of the three is refused, and a parent that is the document itself or one of its descendants fails. Returns {document_id} with the fields that changed.","has_params_one_of":true,"json_schema":{"properties":{"document_id":{"description":"The document id.","type":"string"},"icon":{"description":"Optional. The new icon identifier; omit to keep the current one.","type":"string"},"name":{"description":"Optional. The new display name; omit to keep the current one.","type":"string"},"parent_id":{"description":"Optional. The new parent document id, or an empty string to move the document to the organisation root; omit to leave it where it is.","type":"string"}},"required":["document_id"],"type":"object"},"name":"update_document"}} \ No newline at end of file +{"create_document":{"desc":"Create a new document and return {document_id, branch_id}, the branch_id being its default branch. The document starts with a single empty paragraph, so follow up with insert_block calls (position end, that branch_id) in the same turn to fill it. Omit parent_id to create it at the organisation root.","has_params_one_of":true,"json_schema":{"properties":{"icon":{"description":"Iconify identifier, as \"collection:name\". The product's own icons are MingCute fills (e.g. \"mingcute:file-code-fill\"); prefer one so the document matches the rest of the sidebar. Defaults to \"mingcute:document-2-fill\" when empty.","type":"string"},"name":{"description":"Display name for the new document.","type":"string"},"parent_id":{"description":"Optional parent document id. Omit to create at the org root.","type":"string"}},"required":["name"],"type":"object"},"name":"create_document"},"delete_block":{"desc":"Delete one block by uid, including anything nested inside it. Its comments, hooks and files go with it and cannot be restored, so use move_block when the aim is to reorder and update_block_text when the aim is new wording. Returns {deleted: uid}.","has_params_one_of":true,"json_schema":{"properties":{"block_uid":{"description":"The uid of the block to delete.","type":"string"},"branch_id":{"description":"The id of the branch to read or write: a document's default_branch_id from list_documents, a search hit's branch_id, or any id from the branches get_document lists. A protected branch can be read but refuses every write.","type":"string"},"document_id":{"description":"The target document id.","type":"string"}},"required":["document_id","branch_id","block_uid"],"type":"object"},"name":"delete_block"},"delete_document":{"desc":"Delete a document and every document nested under it. The whole subtree goes and cannot be restored, so check the tree with list_documents first, and use update_document when the aim is to relocate rather than remove. Returns {document_id, deleted}.","has_params_one_of":true,"json_schema":{"properties":{"document_id":{"description":"The document id to delete.","type":"string"}},"required":["document_id"],"type":"object"},"name":"delete_document"},"get_document":{"desc":"Read one document on one branch: its name, icon, parent_id, protected flag and updated_at, the branch read and every branch the document has (each with the id a branch_id argument takes), followed by one row per block with the block's uid, kind, flattened text, depth, parent_uid and the few attrs that matter for reading (heading level, callout icon, code language, task checked). Use it as the way to read a document before editing it; branch_id is the default_branch_id a listing or search hit carries, or any id from branches. A protected branch can be read but refuses every write, so pick an unprotected branch id from branches to write to. Rows marked has_children hold nested blocks the rows do not list, and read_block returns those when you need them.","has_params_one_of":true,"json_schema":{"properties":{"branch_id":{"description":"The id of the branch to read or write: a document's default_branch_id from list_documents, a search hit's branch_id, or any id from the branches get_document lists. A protected branch can be read but refuses every write.","type":"string"},"document_id":{"description":"The document id.","type":"string"}},"required":["document_id","branch_id"],"type":"object"},"name":"get_document"},"get_prometheus_metadata":{"desc":"List the metrics a Prometheus data source exposes, with each metric's type, help text and unit. Use it to find the metric names to write a PromQL query against instead of guessing them.","has_params_one_of":true,"json_schema":{"properties":{"data_source_id":{"description":"The data source id, from list_data_sources.","type":"string"}},"required":["data_source_id"],"type":"object"},"name":"get_prometheus_metadata"},"get_sql_metadata":{"desc":"List the tables and their columns in a PostgreSQL, MariaDB or MySQL data source, plus the default schema. Read it before writing a query so the table and column names are the real ones.","has_params_one_of":true,"json_schema":{"properties":{"data_source_id":{"description":"The data source id, from list_data_sources.","type":"string"}},"required":["data_source_id"],"type":"object"},"name":"get_sql_metadata"},"get_sql_query_labels":{"desc":"Run a SQL query limited to one row and return its string columns with an example value each, which are the columns a chart would treat as series labels. Use it to check what a query returns before charting it; it is cheaper than query_sql.","has_params_one_of":true,"json_schema":{"properties":{"data_source_id":{"description":"The data source id, from list_data_sources.","type":"string"},"from":{"description":"Optional. Range start as an RFC3339 timestamp. Defaults to an hour before 'to'.","type":"string"},"query":{"description":"The SQL query to probe. $__ macros are expanded as they are for a metric block.","type":"string"},"to":{"description":"Optional. Range end as an RFC3339 timestamp. Defaults to now.","type":"string"}},"required":["data_source_id","query"],"type":"object"},"name":"get_sql_query_labels"},"insert_block":{"desc":"Insert one canonical block into a document. position start or end puts it at the document's start or end; before or after puts it beside the block reference_block_uid names, which stays in place. The block has to be legal where it lands: the document root takes every type except titled_code, metric and split_doc_param_list, and a reference inside a split_doc or metric_grid takes only what that container holds. Returns the summary rows of the new block and everything nested in it, uids included, with depth counted from the block itself.","has_params_one_of":true,"json_schema":{"properties":{"block":{"description":"A canonical block. Which fields apply depends on type; supply only that type's own.","properties":{"attrs":{"description":"Per-type attributes. Required where the type says so: heading level, titled_code title, image and figma src. Also code language, callout icon, image alt and width, and a metric's chart configuration, whose width is compact, standard or wide.","properties":{"level":{"description":"heading: the level. Required on a heading.","enum":[1,2,3],"type":"integer"},"refreshInterval":{"description":"metric: how often the chart re-queries.","enum":["5s","10s","30s","1m","5m","15m","30m","1h","2h","1d"],"type":"string"},"simulationPreset":{"description":"metric: draws this generated series in place of the query's own result, for a block documenting a metric that has no real data yet. Omit it to chart the query.","enum":["cpu_usage","memory_usage","disk_usage","http_requests","http_latency","error_rate"],"type":"string"},"timeRange":{"description":"metric: the window queried.","enum":["last_5_minutes","last_15_minutes","last_30_minutes","last_1_hour","last_3_hours","last_6_hours","last_12_hours","last_24_hours","last_2_days","last_7_days","last_30_days","last_90_days","last_6_months","last_1_year","last_2_years","last_5_years","today","yesterday","today_so_far","this_week","this_week_so_far","this_month","this_month_so_far","this_year","this_year_so_far","previous_week","previous_month","previous_year"],"type":"string"},"unitType":{"description":"metric: the unit values are read in. With custom, put the label in unitCustom.","enum":["custom","nanoseconds","microseconds","milliseconds","seconds","minutes","hours","days","bytes","kilobytes","megabytes","gigabytes","terabytes","bits","kilobits","megabits","gigabits","terabits","percent0to100","percent0to1"],"type":"string"},"visualizationType":{"description":"metric: the chart kind.","enum":["line_chart","bar_chart","gauge_chart"],"type":"string"}},"type":"object"},"children":{"description":"Blocks nested under a single list entry. A list entry is a paragraph; anything else nested under it goes here, never in the entry itself and never in a second entry.","type":"array"},"header":{"description":"split_doc_param_list heading, as plain text.","type":"string"},"items":{"description":"Entries of bullet_list and ordered_list, which are paragraphs; items of metric_grid, and of blockquote or callout when not using text.","type":"array"},"left":{"description":"split_doc concept side. Must start with a level-1 heading; split_doc_param_list is legal only here.","type":"array"},"params":{"description":"split_doc_param_list rows, each {name, type, description}.","type":"array"},"right":{"description":"split_doc example side. Holds titled_code or metric only, and is the one place titled_code is legal.","type":"array"},"task_items":{"description":"Rows of task_list, each {checked, block}.","type":"array"},"text":{"description":"Inline text for paragraph, heading, blockquote, code, titled_code, mermaid and callout. Raw inside code, titled_code and mermaid; minimal markdown elsewhere. One block is one paragraph; a newline inside text does not start a new one.","type":"string"},"type":{"description":"The block's canonical type.","enum":["paragraph","heading","blockquote","bullet_list","ordered_list","task_list","callout","code","titled_code","mermaid","horizontal_rule","image","figma","metric","metric_grid","split_doc","split_doc_param_list"],"type":"string"},"uid":{"description":"Leave unset. Uids are generated server-side and are the handle later edits use.","type":"string"}},"required":["type"],"type":"object"},"branch_id":{"description":"The id of the branch to read or write: a document's default_branch_id from list_documents, a search hit's branch_id, or any id from the branches get_document lists. A protected branch can be read but refuses every write.","type":"string"},"document_id":{"description":"The target document id.","type":"string"},"position":{"description":"Where the block lands: before or after the reference block, or at the start or end of the document.","enum":["before","after","start","end"],"type":"string"},"reference_block_uid":{"description":"The uid of the block to insert beside. Required with before and after; leave it out with start and end.","type":"string"}},"required":["document_id","branch_id","position","block"],"type":"object"},"name":"insert_block"},"list_data_sources":{"desc":"List the organisation's data sources. Returns id, name, type (prometheus, postgresql, mariadb, mysql) and connection status. Start here: every other data-source tool takes an id from this list, and the type decides which of them serves it.","has_params_one_of":true,"json_schema":{"properties":{},"type":"object"},"name":"list_data_sources"},"list_documents":{"desc":"List the organisation's documents as a tree of {id, name, default_branch_id, children}; default_branch_id is what get_document and the block tools take as branch_id for the document's default branch. Use it to find a document by name or to see what sits under a parent; use search_documents when you are looking for content rather than a title. Pass parent_id to get only that document's direct children, or omit it for the whole organisation.","has_params_one_of":true,"json_schema":{"properties":{"parent_id":{"description":"Optional. Return only the direct children of this document. Omit for the full tree.","type":"string"}},"type":"object"},"name":"list_documents"},"list_prometheus_label_names":{"desc":"List the label names present in a Prometheus data source, optionally only those on the series the matchers select. Pair it with list_prometheus_label_values to build a filtered PromQL query.","has_params_one_of":true,"json_schema":{"properties":{"data_source_id":{"description":"The data source id, from list_data_sources.","type":"string"},"from":{"description":"Optional. Range start as an RFC3339 timestamp. Defaults to an hour before 'to'.","type":"string"},"matchers":{"description":"Optional. PromQL series selectors, e.g. [\"up\", \"{job=\\\"api\\\"}\"].","items":{"type":"string"},"type":"array"},"to":{"description":"Optional. Range end as an RFC3339 timestamp. Defaults to now.","type":"string"}},"required":["data_source_id"],"type":"object"},"name":"list_prometheus_label_names"},"list_prometheus_label_values":{"desc":"List the values a label takes in a Prometheus data source, optionally only on the series the matchers select. Use it to discover the concrete label values a query should filter on.","has_params_one_of":true,"json_schema":{"properties":{"data_source_id":{"description":"The data source id, from list_data_sources.","type":"string"},"from":{"description":"Optional. Range start as an RFC3339 timestamp. Defaults to an hour before 'to'.","type":"string"},"label":{"description":"The label whose values to list, e.g. \"job\".","type":"string"},"matchers":{"description":"Optional. PromQL series selectors, e.g. [\"up\", \"{job=\\\"api\\\"}\"].","items":{"type":"string"},"type":"array"},"to":{"description":"Optional. Range end as an RFC3339 timestamp. Defaults to now.","type":"string"}},"required":["data_source_id","label"],"type":"object"},"name":"list_prometheus_label_values"},"list_prometheus_series":{"desc":"List the series matching a set of selectors in a Prometheus data source, each as its full label set. Use it to see which label combinations a metric actually has before querying it.","has_params_one_of":true,"json_schema":{"properties":{"data_source_id":{"description":"The data source id, from list_data_sources.","type":"string"},"from":{"description":"Optional. Range start as an RFC3339 timestamp. Defaults to an hour before 'to'.","type":"string"},"matchers":{"description":"PromQL series selectors, e.g. [\"up\", \"{job=\\\"api\\\"}\"].","items":{"type":"string"},"type":"array"},"to":{"description":"Optional. Range end as an RFC3339 timestamp. Defaults to now.","type":"string"}},"required":["data_source_id","matchers"],"type":"object"},"name":"list_prometheus_series"},"move_block":{"desc":"Move an existing block before or after another block in the same document. The block keeps its uid, attrs and nested content, so comments, hooks and files attached to it stay attached; deleting it and inserting a copy would lose them. The landing spot has to accept the block's type, by the same rule insert_block applies. Returns the summary rows of the moved block and everything nested in it.","has_params_one_of":true,"json_schema":{"properties":{"block_uid":{"description":"The uid of the block to move.","type":"string"},"branch_id":{"description":"The id of the branch to read or write: a document's default_branch_id from list_documents, a search hit's branch_id, or any id from the branches get_document lists. A protected branch can be read but refuses every write.","type":"string"},"document_id":{"description":"The target document id.","type":"string"},"position":{"description":"Landing side relative to the reference block.","enum":["before","after"],"type":"string"},"reference_block_uid":{"description":"The uid of the block to move relative to.","type":"string"}},"required":["document_id","branch_id","block_uid","position","reference_block_uid"],"type":"object"},"name":"move_block"},"query_prometheus":{"desc":"Run a PromQL range query against a Prometheus data source over the given window. Returns the raw result by default; with chart_type set it instead describes what a metric block would render, which is how to check a query before writing it into a block.","has_params_one_of":true,"json_schema":{"properties":{"chart_type":{"description":"Optional. One of line_chart, bar_chart, gauge_chart. When set, the result describes what the metric block would draw (render status, series count, and each series' labels, point count and endpoints) instead of the raw data. Use it to check a query before putting it in a metric block; omit it when you need the values themselves.","type":"string"},"data_source_id":{"description":"The data source id, from list_data_sources.","type":"string"},"from":{"description":"Optional. Range start as an RFC3339 timestamp. Defaults to an hour before 'to'.","type":"string"},"query":{"description":"The PromQL expression to run.","type":"string"},"to":{"description":"Optional. Range end as an RFC3339 timestamp. Defaults to now.","type":"string"}},"required":["data_source_id","query"],"type":"object"},"name":"query_prometheus"},"query_sql":{"desc":"Run a read-only query against a PostgreSQL, MariaDB or MySQL data source. Returns columns and rows by default; with chart_type set it instead describes what a metric block would render, which is how to check a query before writing it into a block. $__ macros ($__timeFilter, $__timeGroupAlias and the rest) are expanded against the window.","has_params_one_of":true,"json_schema":{"properties":{"chart_type":{"description":"Optional. One of line_chart, bar_chart, gauge_chart. When set, the result describes what the metric block would draw (render status, series count, and each series' labels, point count and endpoints) instead of the raw data. Use it to check a query before putting it in a metric block; omit it when you need the values themselves.","type":"string"},"data_source_id":{"description":"The data source id, from list_data_sources.","type":"string"},"from":{"description":"Optional. Range start as an RFC3339 timestamp. Defaults to an hour before 'to'.","type":"string"},"query":{"description":"The SQL query to run. For a chart, select a time column aliased \"time\" plus one or more numeric columns.","type":"string"},"to":{"description":"Optional. Range end as an RFC3339 timestamp. Defaults to now.","type":"string"}},"required":["data_source_id","query"],"type":"object"},"name":"query_sql"},"read_block":{"desc":"Return the full canonical content of one block by uid, including any nested children. Use it only when get_document's rows are not enough: to edit a split_doc, a nested list or a split_doc_param_list, whose inner structure has to be written back in full. Fails when the uid is not in the document.","has_params_one_of":true,"json_schema":{"properties":{"block_uid":{"description":"The block uid to fetch.","type":"string"},"branch_id":{"description":"The id of the branch to read or write: a document's default_branch_id from list_documents, a search hit's branch_id, or any id from the branches get_document lists. A protected branch can be read but refuses every write.","type":"string"},"document_id":{"description":"The document id.","type":"string"}},"required":["document_id","branch_id","block_uid"],"type":"object"},"name":"read_block"},"read_tool_output":{"desc":"Retrieve the full output of an earlier tool call that was too large to keep in the conversation. Pass the path shown in the truncation notice.","has_params_one_of":true,"json_schema":{"properties":{"file_path":{"description":"The stored output path from the truncation notice.","type":"string"}},"required":["file_path"],"type":"object"},"name":"read_tool_output"},"replace_block":{"desc":"Replace a block by uid with a new block in the same position. The old block's uid, content and children are all gone unless the new block carries them, so use it to change a block's type or its whole structure. For a wording change use update_block_text, and for an attribute change update_block_attrs; both keep the uid, which comments, hooks and files hang off. Returns the summary rows of the new block and everything nested in it, uids included, with depth counted from the block itself.","has_params_one_of":true,"json_schema":{"properties":{"block":{"description":"A canonical block. Which fields apply depends on type; supply only that type's own.","properties":{"attrs":{"description":"Per-type attributes. Required where the type says so: heading level, titled_code title, image and figma src. Also code language, callout icon, image alt and width, and a metric's chart configuration, whose width is compact, standard or wide.","properties":{"level":{"description":"heading: the level. Required on a heading.","enum":[1,2,3],"type":"integer"},"refreshInterval":{"description":"metric: how often the chart re-queries.","enum":["5s","10s","30s","1m","5m","15m","30m","1h","2h","1d"],"type":"string"},"simulationPreset":{"description":"metric: draws this generated series in place of the query's own result, for a block documenting a metric that has no real data yet. Omit it to chart the query.","enum":["cpu_usage","memory_usage","disk_usage","http_requests","http_latency","error_rate"],"type":"string"},"timeRange":{"description":"metric: the window queried.","enum":["last_5_minutes","last_15_minutes","last_30_minutes","last_1_hour","last_3_hours","last_6_hours","last_12_hours","last_24_hours","last_2_days","last_7_days","last_30_days","last_90_days","last_6_months","last_1_year","last_2_years","last_5_years","today","yesterday","today_so_far","this_week","this_week_so_far","this_month","this_month_so_far","this_year","this_year_so_far","previous_week","previous_month","previous_year"],"type":"string"},"unitType":{"description":"metric: the unit values are read in. With custom, put the label in unitCustom.","enum":["custom","nanoseconds","microseconds","milliseconds","seconds","minutes","hours","days","bytes","kilobytes","megabytes","gigabytes","terabytes","bits","kilobits","megabits","gigabits","terabits","percent0to100","percent0to1"],"type":"string"},"visualizationType":{"description":"metric: the chart kind.","enum":["line_chart","bar_chart","gauge_chart"],"type":"string"}},"type":"object"},"children":{"description":"Blocks nested under a single list entry. A list entry is a paragraph; anything else nested under it goes here, never in the entry itself and never in a second entry.","type":"array"},"header":{"description":"split_doc_param_list heading, as plain text.","type":"string"},"items":{"description":"Entries of bullet_list and ordered_list, which are paragraphs; items of metric_grid, and of blockquote or callout when not using text.","type":"array"},"left":{"description":"split_doc concept side. Must start with a level-1 heading; split_doc_param_list is legal only here.","type":"array"},"params":{"description":"split_doc_param_list rows, each {name, type, description}.","type":"array"},"right":{"description":"split_doc example side. Holds titled_code or metric only, and is the one place titled_code is legal.","type":"array"},"task_items":{"description":"Rows of task_list, each {checked, block}.","type":"array"},"text":{"description":"Inline text for paragraph, heading, blockquote, code, titled_code, mermaid and callout. Raw inside code, titled_code and mermaid; minimal markdown elsewhere. One block is one paragraph; a newline inside text does not start a new one.","type":"string"},"type":{"description":"The block's canonical type.","enum":["paragraph","heading","blockquote","bullet_list","ordered_list","task_list","callout","code","titled_code","mermaid","horizontal_rule","image","figma","metric","metric_grid","split_doc","split_doc_param_list"],"type":"string"},"uid":{"description":"Leave unset. Uids are generated server-side and are the handle later edits use.","type":"string"}},"required":["type"],"type":"object"},"block_uid":{"description":"The uid of the block being replaced.","type":"string"},"branch_id":{"description":"The id of the branch to read or write: a document's default_branch_id from list_documents, a search hit's branch_id, or any id from the branches get_document lists. A protected branch can be read but refuses every write.","type":"string"},"document_id":{"description":"The target document id.","type":"string"}},"required":["document_id","branch_id","block_uid","block"],"type":"object"},"name":"replace_block"},"search_documents":{"desc":"Full-text search across every branch of every document in the organisation for blocks whose text matches the query. Returns hits with document_id, document_name, branch_id, branch_name, default (whether that branch is the document's default), block_uid and text; branch_id is what a follow-up get_document or read_block takes. A block a fork copied unchanged is found on every branch that holds it. Use it to find where a topic is discussed or which documents the user might mean; use list_documents when you know the title. A hit names the innermost block holding the text, which may sit below anything get_document lists; read_block resolves it.","has_params_one_of":true,"json_schema":{"properties":{"limit":{"description":"Maximum number of hits to return. Defaults to 20; cap is 50.","type":"integer"},"query":{"description":"The full-text search query (typo-tolerant, matches block text).","type":"string"}},"required":["query"],"type":"object"},"name":"search_documents"},"update_block_attrs":{"desc":"Set or override named attributes on an existing block, such as a heading's level or a callout's icon. Attributes not mentioned are kept and uid cannot change. Values are validated for the block's type, so a level outside 1 to 3 or a metric width other than compact, standard or wide is rejected. Use replace_block when the type itself has to change. Returns the summary rows of the block and everything nested in it, attrs as they now stand.","has_params_one_of":true,"json_schema":{"properties":{"attrs":{"description":"Attribute keys and values to set (e.g. {\"level\": 2}, {\"icon\": \"lucide:warning\"}).","type":"object"},"block_uid":{"description":"The uid of the block whose attrs should be updated.","type":"string"},"branch_id":{"description":"The id of the branch to read or write: a document's default_branch_id from list_documents, a search hit's branch_id, or any id from the branches get_document lists. A protected branch can be read but refuses every write.","type":"string"},"document_id":{"description":"The target document id.","type":"string"}},"required":["document_id","branch_id","block_uid","attrs"],"type":"object"},"name":"update_block_attrs"},"update_block_text":{"desc":"Replace the inline text of one text-bearing block: paragraph, heading, blockquote, code, titled_code, mermaid, or a callout written with text. Type, attrs and uid are kept, so this is the tool for wording changes. Text follows the canonical markdown subset (**bold**, *italic*, _underline_, ~~strike~~, backtick code, [label](url)), and is raw in code, titled_code and mermaid. One block is one paragraph; to add a paragraph, insert a block instead. Returns the block's summary row with the new text.","has_params_one_of":true,"json_schema":{"properties":{"block_uid":{"description":"The uid of the block whose text should be replaced.","type":"string"},"branch_id":{"description":"The id of the branch to read or write: a document's default_branch_id from list_documents, a search hit's branch_id, or any id from the branches get_document lists. A protected branch can be read but refuses every write.","type":"string"},"document_id":{"description":"The target document id.","type":"string"},"text":{"description":"New inline text in canonical markdown.","type":"string"}},"required":["document_id","branch_id","block_uid","text"],"type":"object"},"name":"update_block_text"},"update_document":{"desc":"Change a document's name, icon or place in the tree, any combination in one call; content is untouched. Give only the fields to change: name is the new display name, icon an Iconify identifier as \"collection:name\" (the product's own icons are MingCute fills, e.g. \"mingcute:rocket-fill\", so prefer one to match the sidebar), and parent_id the new parent, or an empty string for the organisation root. A call with none of the three is refused, and a parent that is the document itself or one of its descendants fails. Returns {document_id} with the fields that changed.","has_params_one_of":true,"json_schema":{"properties":{"document_id":{"description":"The document id.","type":"string"},"icon":{"description":"Optional. The new icon identifier; omit to keep the current one.","type":"string"},"name":{"description":"Optional. The new display name; omit to keep the current one.","type":"string"},"parent_id":{"description":"Optional. The new parent document id, or an empty string to move the document to the organisation root; omit to leave it where it is.","type":"string"}},"required":["document_id"],"type":"object"},"name":"update_document"}} \ No newline at end of file diff --git a/server/core/internal/document/document.go b/server/core/internal/document/document.go index 03edf2ab..6d9e9d07 100644 --- a/server/core/internal/document/document.go +++ b/server/core/internal/document/document.go @@ -262,21 +262,22 @@ func (d Document) HistoryEntry() HistoryEntry { } } -// Search converts the document to a search.Document for indexing. +// Search converts the document's branch to search blocks for indexing. func (d Document) Search() map[string]search.Block { - res := d.Content.Search(d.OrganizationID, d.ID) - - res[d.ID.String()] = search.Block{ - // a special block carrying the document name. The map is keyed by the - // document ID while the indexed block ID is prefixed, so the title - // never collides with a content block that happens to share the ID. - ID: "docname" + d.ID.String(), + scope := search.Scope{ OrganizationID: d.OrganizationID, DocumentID: d.ID, - Type: "document", - Text: d.DocumentName, + BranchID: d.BranchID, + BranchName: d.BranchName, + BranchDefault: d.Default, } + res := d.Content.Search(scope) + + // a special block carrying the document name. The map is keyed by the + // document ID, which no content block shares. + res[d.ID.String()] = scope.Block("docname", "document", d.DocumentName) + return res } diff --git a/server/core/internal/document/document_test.go b/server/core/internal/document/document_test.go index c91bd9f4..8162e48b 100644 --- a/server/core/internal/document/document_test.go +++ b/server/core/internal/document/document_test.go @@ -270,15 +270,20 @@ func Test_Document_Search(t *testing.T) { nameBlock, ok := res[doc.ID.String()] require.True(t, ok) assert.Equal(t, search.Block{ - ID: "docname" + doc.ID.String(), + ID: doc.BranchID.String() + "-docname", OrganizationID: "org-1", DocumentID: doc.ID, + BranchID: doc.BranchID, + BranchName: DefaultBranch, + BranchDefault: true, Type: "document", Text: "Runbook", }, nameBlock) contentBlock, ok := res["p1"] require.True(t, ok) + assert.Equal(t, doc.BranchID.String()+"-p1", contentBlock.ID) + assert.Equal(t, doc.BranchID, contentBlock.BranchID) assert.Equal(t, "hello", contentBlock.Text) } diff --git a/server/core/internal/document/node.go b/server/core/internal/document/node.go index cd0b304f..7990547e 100644 --- a/server/core/internal/document/node.go +++ b/server/core/internal/document/node.go @@ -70,11 +70,11 @@ func (rb RootBlock) HasBlock(blockID string) bool { } // Search transforms RootBlock into a search compatible type. -func (rb RootBlock) Search(organizationID string, documentID xid.ID) map[string]search.Block { +func (rb RootBlock) Search(scope search.Scope) map[string]search.Block { res := make(map[string]search.Block) for _, b := range rb.Content { - content := b.Search(organizationID, documentID) + content := b.Search(scope) maps.Insert(res, maps.All(content)) } @@ -210,7 +210,7 @@ func (b Block) HasBlock(blockID string) bool { } // Search transforms Block into a search compatible type. -func (b Block) Search(organizationID string, documentID xid.ID) map[string]search.Block { +func (b Block) Search(scope search.Scope) map[string]search.Block { res := make(map[string]search.Block) var text strings.Builder @@ -224,20 +224,21 @@ func (b Block) Search(organizationID string, documentID xid.ID) map[string]searc continue } - content := cb.Search(organizationID, documentID) + content := cb.Search(scope) maps.Insert(res, maps.All(content)) } + // a metric block has no text children; its title is an attribute. + if b.Type == BlockNodeMetricBlock { + if title, ok := b.Attrs[AttrTitle].(string); ok { + text.WriteString(title) + } + } + if text.Len() != 0 { if id, ok := b.UID(); ok && id != "" { - res[id] = search.Block{ - ID: id, - OrganizationID: organizationID, - DocumentID: documentID, - Type: string(b.Type), - Text: text.String(), - } + res[id] = scope.Block(id, string(b.Type), text.String()) } } diff --git a/server/core/internal/document/node_test.go b/server/core/internal/document/node_test.go index 0cc6a645..d63ba682 100644 --- a/server/core/internal/document/node_test.go +++ b/server/core/internal/document/node_test.go @@ -296,33 +296,37 @@ func Test_RootBlock_Search(t *testing.T) { }, // no text: nothing to index. {Type: BlockNodeHorizontalRule, Attrs: Attributes{"uid": "hr1"}}, + // a metric block's title is an attribute, not a text child. + { + Type: BlockNodeMetricBlock, + Attrs: Attributes{"uid": "m1", "title": "Pizza Fridays"}, + }, + // an untitled metric block has nothing to index. + { + Type: BlockNodeMetricBlock, + Attrs: Attributes{"uid": "m2", "title": ""}, + }, + { + Type: BlockNodeMetricBlock, + Attrs: Attributes{"uid": "m3"}, + }, }, } - res := rb.Search("org-1", documentID) + scope := search.Scope{ + OrganizationID: "org-1", + DocumentID: documentID, + BranchID: xid.New(), + BranchName: "draft", + } + + res := rb.Search(scope) assert.Equal(t, map[string]search.Block{ - "p1": { - ID: "p1", - OrganizationID: "org-1", - DocumentID: documentID, - Type: "paragraph", - Text: "first", - }, - "li1": { - ID: "li1", - OrganizationID: "org-1", - DocumentID: documentID, - Type: "listItem", - Text: "nested", - }, - "p2": { - ID: "p2", - OrganizationID: "org-1", - DocumentID: documentID, - Type: "paragraph", - Text: "plain bold tail", - }, + "p1": scope.Block("p1", "paragraph", "first"), + "li1": scope.Block("li1", "listItem", "nested"), + "p2": scope.Block("p2", "paragraph", "plain bold tail"), + "m1": scope.Block("m1", "metricBlock", "Pizza Fridays"), }, res) } diff --git a/server/core/internal/search/client.go b/server/core/internal/search/client.go index 65cff36e..a92457f4 100644 --- a/server/core/internal/search/client.go +++ b/server/core/internal/search/client.go @@ -7,7 +7,9 @@ import ( "encoding/json" "errors" "fmt" + "maps" "net/http" + "slices" "strconv" "strings" "time" @@ -41,8 +43,37 @@ const ( // _taskPollInterval is how often an asynchronous Meilisearch task is // polled while waiting for it to settle. _taskPollInterval = 50 * time.Millisecond + + // _setupTimeout bounds the index setup at boot. + _setupTimeout = time.Minute + + // _writeTimeout bounds one batch of index writes, task waits included. + _writeTimeout = 2 * time.Minute + + // _searchTimeout bounds one search request. + _searchTimeout = 10 * time.Second ) +// _displayedAttributes are the entry fields search hits carry. +var _displayedAttributes = []string{ + "id", + "organizationId", + "documentId", + "branchId", + "branchName", + "branchDefault", + "type", + _textAttribute, +} + +// _filterableAttributes are the entry fields searches and removals filter +// by. +var _filterableAttributes = []string{ + "organizationId", + "documentId", + "branchId", +} + // Client is a Meilisearch client wrapper. type Client struct { meiliMan meilisearch.ServiceManager @@ -76,8 +107,12 @@ func (c *Client) Configured() bool { return c.meiliMan != nil } -// ensureIndex ensures the documents index exists. +// ensureIndex ensures the documents index exists and runs with the +// declared settings. func (c *Client) ensureIndex(ctx context.Context) error { + ctx, cancel := context.WithTimeout(ctx, _setupTimeout) + defer cancel() + _, err := c.meiliMan.GetIndexWithContext(ctx, _documentsIndex) switch merr, ok := errors.AsType[*meilisearch.Error](err); { @@ -99,30 +134,70 @@ func (c *Client) ensureIndex(ctx context.Context) error { return fmt.Errorf("getting documents index: %w", err) } - // the settings updates are idempotent and are therefore applied on every - // boot. Applying them only when the index is created leaves an index - // whose settings task failed broken for good — with organizationId not - // filterable every search fails — and never rolls new synonyms out to an - // existing deployment. - task, err := c.meiliMan.Index(_documentsIndex).UpdateFilterableAttributesWithContext(ctx, &[]any{ - "organizationId", - "documentId", - }) + // the settings are checked on every boot rather than only when the + // index is created: an index whose settings task failed would otherwise + // stay broken for good, and new synonyms would never reach an existing + // deployment. They are sent only when they differ, since a searchable + // or filterable attribute change re-indexes every entry. + want, err := indexSettings() if err != nil { - return fmt.Errorf("updating filterable attributes: %w", err) + return err + } + + have, err := c.meiliMan.Index(_documentsIndex).GetSettingsWithContext(ctx) + if err != nil { + return fmt.Errorf("getting settings: %w", err) + } + + if settingsMatch(have, want) { + return nil + } + + task, err := c.meiliMan.Index(_documentsIndex).UpdateSettingsWithContext(ctx, want) + if err != nil { + return fmt.Errorf("updating settings: %w", err) } if err = c.awaitTask(ctx, task); err != nil { - return fmt.Errorf("updating filterable attributes: %w", err) - } - - if err = c.setupSynonyms(ctx, _documentsIndex); err != nil { - return fmt.Errorf("setting up synonyms: %w", err) + return fmt.Errorf("updating settings: %w", err) } return nil } +// indexSettings returns the settings the documents index runs with. Only +// the fields it sets are sent; everything else keeps Meilisearch's default. +func indexSettings() (*meilisearch.Settings, error) { + var synonyms map[string][]string + + if err := json.Unmarshal(synonymsFile, &synonyms); err != nil { + return nil, fmt.Errorf("unmarshaling synonyms: %w", err) + } + + return &meilisearch.Settings{ + SearchableAttributes: []string{_textAttribute}, + DisplayedAttributes: _displayedAttributes, + FilterableAttributes: _filterableAttributes, + Synonyms: synonyms, + }, nil +} + +// settingsMatch reports whether the index already runs with every field +// want sets. Attribute lists are compared as sets, since Meilisearch does +// not promise their order back. +func settingsMatch(have, want *meilisearch.Settings) bool { + sameSet := func(a, b []string) bool { + a, b = slices.Sorted(slices.Values(a)), slices.Sorted(slices.Values(b)) + + return slices.Equal(a, b) + } + + return sameSet(have.SearchableAttributes, want.SearchableAttributes) && + sameSet(have.DisplayedAttributes, want.DisplayedAttributes) && + sameSet(have.FilterableAttributes, want.FilterableAttributes) && + maps.EqualFunc(have.Synonyms, want.Synonyms, sameSet) +} + // awaitTask blocks until an asynchronous Meilisearch task settles and reports // a rejected task as an error, so a failed index or settings update surfaces // at boot instead of leaving the index quietly misconfigured. @@ -133,27 +208,28 @@ func (c *Client) awaitTask(ctx context.Context, info *meilisearch.TaskInfo) erro } if task.Status != meilisearch.TaskStatusSucceeded { - return fmt.Errorf("task %d finished with status %q", info.TaskUID, task.Status) + return fmt.Errorf( + "task %d finished with status %q: %s (%s)", + info.TaskUID, + task.Status, + task.Error.Message, + task.Error.Code, + ) } return nil } -// setupSynonyms sets up synonyms for the documents index. -func (c *Client) setupSynonyms(ctx context.Context, index string) error { - var synonyms map[string][]string - - if err := json.Unmarshal(synonymsFile, &synonyms); err != nil { - return fmt.Errorf("unmarshaling synonyms: %w", err) - } - - task, err := c.meiliMan.Index(index).UpdateSynonymsWithContext(ctx, &synonyms) +// settle reports the outcome of one index write: the request error when the +// call itself failed, otherwise the task's result once Meilisearch has +// applied it. +func (c *Client) settle(ctx context.Context, what string, task *meilisearch.TaskInfo, err error) error { if err != nil { - return fmt.Errorf("updating synonyms: %w", err) + return fmt.Errorf("%s: %w", what, err) } - if err = c.awaitTask(ctx, task); err != nil { - return fmt.Errorf("updating synonyms: %w", err) + if err := c.awaitTask(ctx, task); err != nil { + return fmt.Errorf("%s: %w", what, err) } return nil @@ -165,6 +241,9 @@ func (c *Client) SearchDocuments(ctx context.Context, organizationID, query stri return nil, ErrNotConfigured } + ctx, cancel := context.WithTimeout(ctx, _searchTimeout) + defer cancel() + res, err := c.meiliMan.Index(_documentsIndex).SearchWithContext(ctx, query, &meilisearch.SearchRequest{ AttributesToSearchOn: []string{_textAttribute}, AttributesToCrop: []string{_textAttribute}, @@ -206,6 +285,9 @@ func (c *Client) SearchDocumentBlocks(ctx context.Context, organizationID, query return nil, ErrNotConfigured } + ctx, cancel := context.WithTimeout(ctx, _searchTimeout) + defer cancel() + res, err := c.meiliMan.Index(_documentsIndex).SearchWithContext(ctx, query, &meilisearch.SearchRequest{ AttributesToSearchOn: []string{_textAttribute}, Filter: fmt.Sprintf("organizationId = %q", organizationID), @@ -231,23 +313,25 @@ func (c *Client) SearchDocumentBlocks(ctx context.Context, organizationID, query } // ReplaceDocumentBlocks replaces document blocks in the index based on the -// provided differences. +// provided differences. Every write is awaited, so a batch Meilisearch +// rejects is reported rather than silently dropped. func (c *Client) ReplaceDocumentBlocks(ctx context.Context, bd BlocksDifference) error { if !c.Configured() { return ErrNotConfigured } - if len(bd.Added) != 0 { - _, err := c.meiliMan.Index(_documentsIndex).AddDocumentsWithContext(ctx, bd.Added, nil) - if err != nil { - return fmt.Errorf("adding documents: %w", err) - } - } + ctx, cancel := context.WithTimeout(ctx, _writeTimeout) + defer cancel() - if len(bd.Updated) != 0 { - _, err := c.meiliMan.Index(_documentsIndex).UpdateDocumentsWithContext(ctx, bd.Updated, nil) - if err != nil { - return fmt.Errorf("updating documents: %w", err) + idx := c.meiliMan.Index(_documentsIndex) + + // every entry is complete, so added and updated ones alike are sent as + // one add-or-replace request. + if docs := slices.Concat(bd.Added, bd.Updated); len(docs) != 0 { + task, err := idx.AddDocumentsWithContext(ctx, docs, nil) + + if err = c.settle(ctx, "adding documents", task, err); err != nil { + return err } } @@ -258,9 +342,22 @@ func (c *Client) ReplaceDocumentBlocks(ctx context.Context, bd BlocksDifference) ids = append(ids, block.ID) } - _, err := c.meiliMan.Index(_documentsIndex).DeleteDocumentsWithContext(ctx, ids, nil) - if err != nil { - return fmt.Errorf("deleting documents: %w", err) + task, err := idx.DeleteDocumentsWithContext(ctx, ids, nil) + + if err = c.settle(ctx, "deleting documents", task, err); err != nil { + return err + } + } + + if len(bd.RemovedBranches) != 0 { + ids := make([]string, 0, len(bd.RemovedBranches)) + + for _, br := range bd.RemovedBranches { + ids = append(ids, br.BranchID.String()) + } + + if err := c.deleteByFilter(ctx, idx, "branchId", ids); err != nil { + return fmt.Errorf("deleting branches by filter: %w", err) } } @@ -268,31 +365,35 @@ func (c *Client) ReplaceDocumentBlocks(ctx context.Context, bd BlocksDifference) ids := make([]string, 0, len(bd.RemovedDocuments)) for _, id := range bd.RemovedDocuments { - ids = append(ids, strconv.Quote(id.String())) + ids = append(ids, id.String()) } - filter := fmt.Sprintf("documentId IN [%s]", strings.Join(ids, ", ")) - - _, err := c.meiliMan.Index(_documentsIndex).DeleteDocumentsByFilterWithContext(ctx, filter, nil) - if err != nil { + if err := c.deleteByFilter(ctx, idx, "documentId", ids); err != nil { return fmt.Errorf("deleting documents by filter: %w", err) } } if len(bd.RemovedOrganizations) != 0 { - ids := make([]string, 0, len(bd.RemovedOrganizations)) - - for _, id := range bd.RemovedOrganizations { - ids = append(ids, strconv.Quote(id)) - } - - filter := fmt.Sprintf("organizationId IN [%s]", strings.Join(ids, ", ")) - - _, err := c.meiliMan.Index(_documentsIndex).DeleteDocumentsByFilterWithContext(ctx, filter, nil) - if err != nil { + if err := c.deleteByFilter(ctx, idx, "organizationId", bd.RemovedOrganizations); err != nil { return fmt.Errorf("deleting organizations by filter: %w", err) } } return nil } + +// deleteByFilter removes every entry whose field is one of the ids and +// waits for the removal to apply. +func (c *Client) deleteByFilter(ctx context.Context, idx meilisearch.IndexManager, field string, ids []string) error { + quoted := make([]string, 0, len(ids)) + + for _, id := range ids { + quoted = append(quoted, strconv.Quote(id)) + } + + filter := fmt.Sprintf("%s IN [%s]", field, strings.Join(quoted, ", ")) + + task, err := idx.DeleteDocumentsByFilterWithContext(ctx, filter, nil) + + return c.settle(ctx, "deleting by "+field, task, err) +} diff --git a/server/core/internal/search/client_test.go b/server/core/internal/search/client_test.go index 33715661..b0464148 100644 --- a/server/core/internal/search/client_test.go +++ b/server/core/internal/search/client_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "slices" "testing" "time" @@ -86,70 +87,116 @@ func Test_NewClient(t *testing.T) { } } - wasFilterableUpdated := func(count int) check { + wasSettingsUpdated := func(count int) check { return func(t *testing.T, _ *mock.MeiliServiceManager, idx *mock.MeiliIndexManager, _ error) { - ff := idx.UpdateFilterableAttributesWithContextCalls() + ff := idx.UpdateSettingsWithContextCalls() require.Len(t, ff, count) if count == 0 { return } - assert.Equal(t, &[]any{"organizationId", "documentId"}, ff[0].Request) + assert.Equal(t, []string{"text"}, ff[0].Request.SearchableAttributes) + assert.Equal(t, _displayedAttributes, ff[0].Request.DisplayedAttributes) + assert.Equal(t, _filterableAttributes, ff[0].Request.FilterableAttributes) + assert.NotEmpty(t, ff[0].Request.Synonyms) } } - wasSynonymsUpdated := func(count int) check { - return func(t *testing.T, _ *mock.MeiliServiceManager, idx *mock.MeiliIndexManager, _ error) { - ff := idx.UpdateSynonymsWithContextCalls() - require.Len(t, ff, count) + // current is what a settled index reports: the declared settings, with + // the attribute lists in an order of Meilisearch's choosing. + current := func() *meilisearch.Settings { + want, err := indexSettings() + require.NoError(t, err) - if count == 0 { - return - } + have := *want + have.DisplayedAttributes = slices.Clone(want.DisplayedAttributes) + slices.Reverse(have.DisplayedAttributes) - require.NotNil(t, ff[0].Request) - assert.NotEmpty(t, *ff[0].Request) + return &have + } + + // fresh is what a just-created index reports. + fresh := func() *meilisearch.Settings { + return &meilisearch.Settings{ + SearchableAttributes: []string{"*"}, + DisplayedAttributes: []string{"*"}, } } tests := map[string]struct { - GetIndexErr error - CreateErr error - FilterableErr error - SynonymsErr error - WaitErr error - TaskStatus meilisearch.TaskStatus - Checks []check + GetIndexErr error + CreateErr error + Settings *meilisearch.Settings + GetSettingsErr error + SettingsErr error + WaitErr error + TaskStatus meilisearch.TaskStatus + Checks []check }{ - "Existing index still has its settings applied": { + "Existing index with the declared settings is left alone": { + Settings: current(), Checks: checks( hasError(false), wasCreateIndexCalled(0), - wasFilterableUpdated(1), - wasSynonymsUpdated(1), + wasSettingsUpdated(0), + ), + }, + "Existing index with stale settings gets them updated": { + Settings: func() *meilisearch.Settings { + st := current() + st.FilterableAttributes = []string{"organizationId", "documentId"} + + return st + }(), + Checks: checks( + hasError(false), + wasCreateIndexCalled(0), + wasSettingsUpdated(1), + ), + }, + "Existing index with stale synonyms gets them updated": { + Settings: func() *meilisearch.Settings { + st := current() + st.Synonyms = map[string][]string{"2fa": {"mfa"}} + + return st + }(), + Checks: checks( + hasError(false), + wasSettingsUpdated(1), + ), + }, + "Settings fetch failure fails": { + GetSettingsErr: assert.AnError, + Checks: checks( + hasError(true), + wasSettingsUpdated(0), ), }, "Rejected settings task fails": { + Settings: fresh(), TaskStatus: meilisearch.TaskStatusFailed, Checks: checks( hasError(true), wasCreateIndexCalled(0), + wasSettingsUpdated(1), ), }, "Task wait failure fails": { - WaitErr: assert.AnError, + Settings: fresh(), + WaitErr: assert.AnError, Checks: checks( hasError(true), ), }, "Missing index is created with settings": { GetIndexErr: indexNotFoundErr(), + Settings: fresh(), Checks: checks( hasError(false), wasCreateIndexCalled(1), - wasFilterableUpdated(1), - wasSynonymsUpdated(1), + wasSettingsUpdated(1), ), }, "Other meilisearch errors fail": { @@ -171,22 +218,16 @@ func Test_NewClient(t *testing.T) { CreateErr: assert.AnError, Checks: checks( hasError(true), - wasFilterableUpdated(0), + wasSettingsUpdated(0), ), }, - "Filterable attribute failure fails": { - GetIndexErr: indexNotFoundErr(), - FilterableErr: assert.AnError, - Checks: checks( - hasError(true), - wasSynonymsUpdated(0), - ), - }, - "Synonyms failure fails": { + "Settings update failure fails": { GetIndexErr: indexNotFoundErr(), - SynonymsErr: assert.AnError, + Settings: fresh(), + SettingsErr: assert.AnError, Checks: checks( hasError(true), + wasSettingsUpdated(1), ), }, } @@ -196,11 +237,11 @@ func Test_NewClient(t *testing.T) { t.Parallel() idx := &mock.MeiliIndexManager{ - UpdateFilterableAttributesWithContextFunc: func(context.Context, *[]any) (*meilisearch.TaskInfo, error) { - return &meilisearch.TaskInfo{TaskUID: 2}, tc.FilterableErr + GetSettingsWithContextFunc: func(context.Context) (*meilisearch.Settings, error) { + return tc.Settings, tc.GetSettingsErr }, - UpdateSynonymsWithContextFunc: func(context.Context, *map[string][]string) (*meilisearch.TaskInfo, error) { - return &meilisearch.TaskInfo{TaskUID: 3}, tc.SynonymsErr + UpdateSettingsWithContextFunc: func(context.Context, *meilisearch.Settings) (*meilisearch.TaskInfo, error) { + return &meilisearch.TaskInfo{TaskUID: 2}, tc.SettingsErr }, } @@ -234,9 +275,19 @@ func Test_NewClient(t *testing.T) { } // newTestClient builds a Client wired straight to the given index mock, -// bypassing ensureIndex. -func newTestClient(idx *mock.MeiliIndexManager) *Client { - return &Client{meiliMan: stubManagers(idx)} +// bypassing ensureIndex. Every task the index reports settles with the +// given status. +func newTestClient(idx *mock.MeiliIndexManager, status meilisearch.TaskStatus) *Client { + svc := stubManagers(idx) + svc.WaitForTaskWithContextFunc = func(context.Context, int64, time.Duration) (*meilisearch.Task, error) { + task := &meilisearch.Task{Status: status} + task.Error.Message = "boom" + task.Error.Code = "invalid_document_id" + + return task, nil + } + + return &Client{meiliMan: svc} } func Test_Client_SearchDocuments(t *testing.T) { @@ -259,7 +310,7 @@ func Test_Client_SearchDocuments(t *testing.T) { }, } - data, err := newTestClient(idx).SearchDocuments(context.Background(), "org-1", "hello") + data, err := newTestClient(idx, meilisearch.TaskStatusSucceeded).SearchDocuments(context.Background(), "org-1", "hello") require.NoError(t, err) assert.JSONEq(t, `[{"id": "b1", "text": "hello"}]`, string(data)) @@ -281,7 +332,7 @@ func Test_Client_SearchDocuments(t *testing.T) { }, } - _, err := newTestClient(idx).SearchDocuments(context.Background(), "org-1", "hello") + _, err := newTestClient(idx, meilisearch.TaskStatusSucceeded).SearchDocuments(context.Background(), "org-1", "hello") require.Error(t, err) }) } @@ -314,7 +365,7 @@ func Test_Client_SearchDocumentBlocks(t *testing.T) { }, } - blocks, err := newTestClient(idx).SearchDocumentBlocks(context.Background(), "org-1", "hello", 7) + blocks, err := newTestClient(idx, meilisearch.TaskStatusSucceeded).SearchDocumentBlocks(context.Background(), "org-1", "hello", 7) require.NoError(t, err) assert.Equal(t, []Block{{ @@ -343,7 +394,7 @@ func Test_Client_SearchDocumentBlocks(t *testing.T) { }, } - _, err := newTestClient(idx).SearchDocumentBlocks(context.Background(), "org-1", "hello", 7) + _, err := newTestClient(idx, meilisearch.TaskStatusSucceeded).SearchDocumentBlocks(context.Background(), "org-1", "hello", 7) require.Error(t, err) }) @@ -356,7 +407,7 @@ func Test_Client_SearchDocumentBlocks(t *testing.T) { }, } - _, err := newTestClient(idx).SearchDocumentBlocks(context.Background(), "org-1", "hello", 7) + _, err := newTestClient(idx, meilisearch.TaskStatusSucceeded).SearchDocumentBlocks(context.Background(), "org-1", "hello", 7) require.Error(t, err) }) } @@ -385,12 +436,6 @@ func Test_Client_ReplaceDocumentBlocks(t *testing.T) { } } - wasUpdateCalled := func(count int) check { - return func(t *testing.T, idx *mock.MeiliIndexManager, _ error) { - require.Len(t, idx.UpdateDocumentsWithContextCalls(), count) - } - } - wasDeleteCalled := func(count int) check { return func(t *testing.T, idx *mock.MeiliIndexManager, _ error) { require.Len(t, idx.DeleteDocumentsWithContextCalls(), count) @@ -403,7 +448,16 @@ func Test_Client_ReplaceDocumentBlocks(t *testing.T) { } } + hasFilter := func(filter string) check { + return func(t *testing.T, idx *mock.MeiliIndexManager, _ error) { + ff := idx.DeleteDocumentsByFilterWithContextCalls() + require.NotEmpty(t, ff) + assert.Equal(t, filter, ff[0].Filter) + } + } + docID1, docID2 := xid.New(), xid.New() + branchID1, branchID2 := xid.New(), xid.New() fullDiff := BlocksDifference{ Added: []Block{{ID: "a"}}, @@ -413,8 +467,8 @@ func Test_Client_ReplaceDocumentBlocks(t *testing.T) { tests := map[string]struct { Diff BlocksDifference + TaskStatus meilisearch.TaskStatus AddErr error - UpdateErr error DeleteErr error DeleteFilterErr error Checks []check @@ -423,26 +477,29 @@ func Test_Client_ReplaceDocumentBlocks(t *testing.T) { Checks: checks( hasError(false), wasAddCalled(0), - wasUpdateCalled(0), wasDeleteCalled(0), wasDeleteByFilterCalled(0), ), }, + "Removed branches are cleared by filter": { + Diff: BlocksDifference{RemovedBranches: []BranchRemoval{ + {DocumentID: docID1, BranchID: branchID1}, + {DocumentID: docID1, BranchID: branchID2}, + }}, + Checks: checks( + hasError(false), + wasDeleteCalled(0), + wasDeleteByFilterCalled(1), + hasFilter(fmt.Sprintf("branchId IN [%q, %q]", branchID1, branchID2)), + ), + }, "Removed documents are cleared by filter": { Diff: BlocksDifference{RemovedDocuments: []xid.ID{docID1, docID2}}, Checks: checks( hasError(false), wasDeleteCalled(0), wasDeleteByFilterCalled(1), - func(t *testing.T, idx *mock.MeiliIndexManager, _ error) { - ff := idx.DeleteDocumentsByFilterWithContextCalls() - require.NotEmpty(t, ff) - assert.Equal( - t, - fmt.Sprintf("documentId IN [%q, %q]", docID1, docID2), - ff[0].Filter, - ) - }, + hasFilter(fmt.Sprintf("documentId IN [%q, %q]", docID1, docID2)), ), }, "Removed organizations are cleared by filter": { @@ -451,11 +508,7 @@ func Test_Client_ReplaceDocumentBlocks(t *testing.T) { hasError(false), wasDeleteCalled(0), wasDeleteByFilterCalled(1), - func(t *testing.T, idx *mock.MeiliIndexManager, _ error) { - ff := idx.DeleteDocumentsByFilterWithContextCalls() - require.NotEmpty(t, ff) - assert.Equal(t, `organizationId IN ["org-1", "org-2"]`, ff[0].Filter) - }, + hasFilter(`organizationId IN ["org-1", "org-2"]`), ), }, "Delete by filter failure is propagated": { @@ -466,17 +519,17 @@ func Test_Client_ReplaceDocumentBlocks(t *testing.T) { wasDeleteByFilterCalled(1), ), }, - "Full difference dispatches all three": { + // added and updated entries are complete either way, so one + // add-or-replace request carries both. + "Full difference sends one add and one delete": { Diff: fullDiff, Checks: checks( hasError(false), wasAddCalled(1), - wasUpdateCalled(1), wasDeleteCalled(1), func(t *testing.T, idx *mock.MeiliIndexManager, _ error) { - ff := idx.DeleteDocumentsWithContextCalls() - require.NotEmpty(t, ff) - assert.Equal(t, []string{"r1", "r2"}, ff[0].Identifiers) + assert.Equal(t, []Block{{ID: "a"}, {ID: "u"}}, idx.AddDocumentsWithContextCalls()[0].DocumentsPtr) + assert.Equal(t, []string{"r1", "r2"}, idx.DeleteDocumentsWithContextCalls()[0].Identifiers) }, ), }, @@ -485,16 +538,6 @@ func Test_Client_ReplaceDocumentBlocks(t *testing.T) { AddErr: assert.AnError, Checks: checks( hasError(true), - wasUpdateCalled(0), - wasDeleteCalled(0), - ), - }, - "Update failure short-circuits": { - Diff: fullDiff, - UpdateErr: assert.AnError, - Checks: checks( - hasError(true), - wasAddCalled(1), wasDeleteCalled(0), ), }, @@ -504,7 +547,30 @@ func Test_Client_ReplaceDocumentBlocks(t *testing.T) { Checks: checks( hasError(true), wasAddCalled(1), - wasUpdateCalled(1), + ), + }, + // the request is accepted but the task fails later; the batch is + // reported so the job is retried rather than dropped. + "Failed task is reported with its cause": { + Diff: fullDiff, + TaskStatus: meilisearch.TaskStatusFailed, + Checks: checks( + hasError(true), + wasAddCalled(1), + wasDeleteCalled(0), + func(t *testing.T, _ *mock.MeiliIndexManager, err error) { + assert.ErrorContains(t, err, "boom (invalid_document_id)") + }, + ), + }, + "Calls run under a deadline": { + Diff: fullDiff, + Checks: checks( + hasError(false), + func(t *testing.T, idx *mock.MeiliIndexManager, _ error) { + _, ok := idx.AddDocumentsWithContextCalls()[0].Ctx.Deadline() + assert.True(t, ok) + }, ), }, } @@ -515,20 +581,22 @@ func Test_Client_ReplaceDocumentBlocks(t *testing.T) { idx := &mock.MeiliIndexManager{ AddDocumentsWithContextFunc: func(context.Context, any, *meilisearch.DocumentOptions) (*meilisearch.TaskInfo, error) { - return nil, tc.AddErr - }, - UpdateDocumentsWithContextFunc: func(context.Context, any, *meilisearch.DocumentOptions) (*meilisearch.TaskInfo, error) { - return nil, tc.UpdateErr + return &meilisearch.TaskInfo{TaskUID: 1}, tc.AddErr }, DeleteDocumentsWithContextFunc: func(context.Context, []string, *meilisearch.DocumentOptions) (*meilisearch.TaskInfo, error) { - return nil, tc.DeleteErr + return &meilisearch.TaskInfo{TaskUID: 2}, tc.DeleteErr }, DeleteDocumentsByFilterWithContextFunc: func(context.Context, any, *meilisearch.DocumentOptions) (*meilisearch.TaskInfo, error) { - return nil, tc.DeleteFilterErr + return &meilisearch.TaskInfo{TaskUID: 3}, tc.DeleteFilterErr }, } - err := newTestClient(idx).ReplaceDocumentBlocks(context.Background(), tc.Diff) + status := meilisearch.TaskStatusSucceeded + if tc.TaskStatus != "" { + status = tc.TaskStatus + } + + err := newTestClient(idx, status).ReplaceDocumentBlocks(context.Background(), tc.Diff) for _, ch := range tc.Checks { ch(t, idx, err) diff --git a/server/core/internal/search/document.go b/server/core/internal/search/document.go index 926a650b..a3941832 100644 --- a/server/core/internal/search/document.go +++ b/server/core/internal/search/document.go @@ -21,6 +21,15 @@ type Block struct { // DocumentID is the identifier for the document this block belongs to. DocumentID xid.ID `json:"documentId"` + // BranchID is the identifier for the branch this block belongs to. + BranchID xid.ID `json:"branchId"` + + // BranchName is the name of the branch this block belongs to. + BranchName string `json:"branchName"` + + // BranchDefault reports whether the branch is the document's default. + BranchDefault bool `json:"branchDefault"` + // Type is the type of the block (e.g., "paragraph", "heading"). Type string `json:"type"` @@ -28,6 +37,50 @@ type Block struct { Text string `json:"text"` } +// Scope is the branch every block of one indexing pass belongs to. +type Scope struct { + // OrganizationID is the organization owning the document. + OrganizationID string + + // DocumentID is the document the branch belongs to. + DocumentID xid.ID + + // BranchID is the branch the blocks are read from. + BranchID xid.ID + + // BranchName is the branch's name. + BranchName string + + // BranchDefault reports whether the branch is the document's default. + BranchDefault bool +} + +// Block builds an index entry for uid within the scope. The entry id is +// prefixed with the branch id: a fork copies its source's content, uids +// included, so the uid alone names one block on every branch at once. +func (s Scope) Block(uid, typ, text string) Block { + return Block{ + ID: s.BranchID.String() + "-" + uid, + OrganizationID: s.OrganizationID, + DocumentID: s.DocumentID, + BranchID: s.BranchID, + BranchName: s.BranchName, + BranchDefault: s.BranchDefault, + Type: typ, + Text: text, + } +} + +// BranchRemoval names a branch whose every block is removed. The document +// id is carried so the removal is ordered with the document's other jobs. +type BranchRemoval struct { + // DocumentID is the document the branch belonged to. + DocumentID xid.ID `json:"documentId"` + + // BranchID is the removed branch. + BranchID xid.ID `json:"branchId"` +} + // BlocksDifference represents the differences between two slices of Blocks. type BlocksDifference struct { // Updated contains blocks that have been modified. @@ -39,6 +92,11 @@ type BlocksDifference struct { // Removed contains blocks that have been removed. Removed []Block `json:"removed"` + // RemovedBranches contains branches whose every block is removed. + // Deleting a branch drops its content row, so the index is cleared by + // branch instead of by block. + RemovedBranches []BranchRemoval `json:"removedBranches"` + // RemovedDocuments contains documents whose every block is removed. // Deleting a document cascades to its descendants in the database, // which makes enumerating their blocks impossible after the fact, so diff --git a/server/core/internal/search/document_test.go b/server/core/internal/search/document_test.go index 827c5d7b..5d5e5f8c 100644 --- a/server/core/internal/search/document_test.go +++ b/server/core/internal/search/document_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "testing" + "github.com/rs/xid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -79,3 +80,26 @@ func Test_BlocksDifference_Scan(t *testing.T) { assert.Error(t, decoded.Scan("not bytes")) assert.Error(t, decoded.Scan([]byte(`{not json`))) } + +func Test_Scope_Block(t *testing.T) { + t.Parallel() + + scope := Scope{ + OrganizationID: "org-1", + DocumentID: xid.New(), + BranchID: xid.New(), + BranchName: "draft", + BranchDefault: false, + } + + assert.Equal(t, Block{ + ID: scope.BranchID.String() + "-p1", + OrganizationID: "org-1", + DocumentID: scope.DocumentID, + BranchID: scope.BranchID, + BranchName: "draft", + BranchDefault: false, + Type: "paragraph", + Text: "hello", + }, scope.Block("p1", "paragraph", "hello")) +} diff --git a/server/core/internal/search/manager/manager.go b/server/core/internal/search/manager/manager.go index 08238b32..b6e8fe07 100644 --- a/server/core/internal/search/manager/manager.go +++ b/server/core/internal/search/manager/manager.go @@ -145,6 +145,10 @@ func jobScope(bd search.BlocksDifference) (documentIDs []xid.ID, global bool) { } } + for _, br := range bd.RemovedBranches { + add(br.DocumentID) + } + for _, id := range bd.RemovedDocuments { add(id) } diff --git a/server/core/internal/search/manager/manager_test.go b/server/core/internal/search/manager/manager_test.go index 22c20f33..5ffa2c61 100644 --- a/server/core/internal/search/manager/manager_test.go +++ b/server/core/internal/search/manager/manager_test.go @@ -212,6 +212,34 @@ func Test_Manager_processJobs(t *testing.T) { }, ), }, + "A branch removal waits behind its document's failed diff": { + Batches: [][]search.DocumentSearchJob{{ + docJob(1, docA), + { + ID: 2, + BlockDiff: search.BlocksDifference{ + RemovedBranches: []search.BranchRemoval{{DocumentID: docA, BranchID: xid.New()}}, + }, + }, + { + ID: 3, + BlockDiff: search.BlocksDifference{ + RemovedBranches: []search.BranchRemoval{{DocumentID: docB, BranchID: xid.New()}}, + }, + }, + }}, + FailJobs: map[int64]bool{1: true}, + Checks: checks( + hasError(false), + wasReplaceCalled(2), + wasDeleteCalled(1), + func(t *testing.T, db *DBMock, _ *SearchGatewayMock, _ error) { + ff := db.DeleteDocumentSearchJobCalls() + require.Len(t, ff, 1) + assert.Equal(t, int64(3), ff[0].ID, "the other document's branch removal keeps moving") + }, + ), + }, "An organization removal waits for anything held": { Batches: [][]search.DocumentSearchJob{{ docJob(1, docA), diff --git a/server/core/internal/server/internal/document/branch_http_test.go b/server/core/internal/server/internal/document/branch_http_test.go index 493975b6..3572c119 100644 --- a/server/core/internal/server/internal/document/branch_http_test.go +++ b/server/core/internal/server/internal/document/branch_http_test.go @@ -6,6 +6,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "strings" "testing" "github.com/guregu/null/v5" @@ -147,6 +148,7 @@ func Test_Handler_UpdateDocumentBranchByIDUnsafe(t *testing.T) { Metadata int Maintainers int SearchJobs int + BranchID xid.ID }{ "Missing branch ID parameter": { DB: &DBMock{}, @@ -273,7 +275,7 @@ func Test_Handler_UpdateDocumentBranchByIDUnsafe(t *testing.T) { Metadata: 1, SearchJobs: 1, }, - "Non-default branch skips the search job": { + "Non-default branch queues its own search job": { DB: &DBMock{ FetchDocumentUnsafeByBranchIDFunc: func(context.Context, xid.ID) (*documentCore.Document, error) { return branchDoc(_branchID2), nil @@ -284,10 +286,12 @@ func Test_Handler_UpdateDocumentBranchByIDUnsafe(t *testing.T) { return []string{"u1", "u9"}, nil }, }, - Body: validBody, - RespCode: http.StatusOK, - Committed: 1, - Metadata: 1, + Body: validBody, + RespCode: http.StatusOK, + Committed: 1, + Metadata: 1, + SearchJobs: 1, + BranchID: _branchID2, }, } @@ -306,6 +310,18 @@ func Test_Handler_UpdateDocumentBranchByIDUnsafe(t *testing.T) { assert.Equal(t, c.Metadata, cnt.metadata) assert.Equal(t, c.Maintainers, cnt.maintainers) assert.Len(t, c.Tx.InsertDocumentSearchJobCalls(), c.SearchJobs) + + // the diff is scoped to the persisted branch: entries are keyed + // by its id, whichever branch it is. + if !c.BranchID.IsZero() { + diff := c.Tx.InsertDocumentSearchJobCalls()[0].Diff + require.NotEmpty(t, diff.Updated) + + for _, b := range diff.Updated { + assert.Equal(t, c.BranchID, b.BranchID) + assert.True(t, strings.HasPrefix(b.ID, c.BranchID.String()+"-")) + } + } }) } } @@ -1188,6 +1204,23 @@ func Test_Handler_CreateDocumentBranch(t *testing.T) { RespCode: http.StatusInternalServerError, Committed: 1, }, + "Search job insert error": { + DB: &DBMock{ + FetchDocumentByBranchIDFunc: func(context.Context, xid.ID, string) (*documentCore.Document, error) { + return storedDoc(), nil + }, + }, + Tx: &TxMock{ + FetchDocumentFunc: func(context.Context, xid.ID, string, string) (*documentCore.Document, error) { + return branchDoc(_branchID2), nil + }, + InsertDocumentSearchJobFunc: func(context.Context, search.BlocksDifference) error { + return errors.New("boom") + }, + }, + Body: validBody, + RespCode: http.StatusInternalServerError, + }, "Successful branch creation": { DB: &DBMock{ FetchDocumentByBranchIDFunc: func(context.Context, xid.ID, string) (*documentCore.Document, error) { @@ -1227,6 +1260,17 @@ func Test_Handler_CreateDocumentBranch(t *testing.T) { if c.RespCode == http.StatusCreated { require.Len(t, c.Tx.ForkDocumentBranchCalls(), 1) assert.Equal(t, "feature", c.Tx.ForkDocumentBranchCalls()[0].TargetBranch) + + // the fork's entries are added under its own branch id before + // the commit, so the branch is searchable without an edit. + require.Len(t, c.Tx.InsertDocumentSearchJobCalls(), 1) + diff := c.Tx.InsertDocumentSearchJobCalls()[0].Diff + require.NotEmpty(t, diff.Added) + + for _, b := range diff.Added { + assert.Equal(t, _branchID2, b.BranchID) + assert.True(t, strings.HasPrefix(b.ID, _branchID2.String()+"-")) + } } if len(c.CopiedHooks) == 0 { @@ -1244,12 +1288,14 @@ func Test_Handler_CreateDocumentBranch(t *testing.T) { func Test_Handler_DeleteDocumentBranch(t *testing.T) { cc := map[string]struct { DB *DBMock + Tx *TxMock NoSession bool OmitDoc bool OmitBranch bool RespCode int RespJSON string Deleted int + Committed int }{ "No session in context": { DB: &DBMock{}, @@ -1326,6 +1372,8 @@ func Test_Handler_DeleteDocumentBranch(t *testing.T) { CountDocumentBranchesFunc: func(context.Context, xid.ID, string) (int, error) { return 2, nil }, + }, + Tx: &TxMock{ DeleteDocumentBranchByIDFunc: func(context.Context, xid.ID, string) error { return errors.New("boom") }, @@ -1333,6 +1381,23 @@ func Test_Handler_DeleteDocumentBranch(t *testing.T) { RespCode: http.StatusInternalServerError, Deleted: 1, }, + "Search job insert error": { + DB: &DBMock{ + FetchDocumentByBranchIDFunc: func(context.Context, xid.ID, string) (*documentCore.Document, error) { + return branchDoc(_branchID), nil + }, + CountDocumentBranchesFunc: func(context.Context, xid.ID, string) (int, error) { + return 2, nil + }, + }, + Tx: &TxMock{ + InsertDocumentSearchJobFunc: func(context.Context, search.BlocksDifference) error { + return errors.New("boom") + }, + }, + RespCode: http.StatusInternalServerError, + Deleted: 1, + }, "Successful deletion": { DB: &DBMock{ FetchDocumentByBranchIDFunc: func(context.Context, xid.ID, string) (*documentCore.Document, error) { @@ -1342,8 +1407,9 @@ func Test_Handler_DeleteDocumentBranch(t *testing.T) { return 2, nil }, }, - RespCode: http.StatusNoContent, - Deleted: 1, + RespCode: http.StatusNoContent, + Deleted: 1, + Committed: 1, }, } @@ -1351,7 +1417,12 @@ func Test_Handler_DeleteDocumentBranch(t *testing.T) { t.Run(cn, func(t *testing.T) { t.Parallel() - hdl, _ := newTestHandler(c.DB, &fakePublisher{}) + tx := c.Tx + if tx == nil { + tx = &TxMock{} + } + + hdl, _ := newTestHandler(withTx(c.DB, tx, nil), &fakePublisher{}) rec := httptest.NewRecorder() @@ -1363,7 +1434,17 @@ func Test_Handler_DeleteDocumentBranch(t *testing.T) { assert.JSONEq(t, c.RespJSON, rec.Body.String()) } - assert.Len(t, c.DB.DeleteDocumentBranchByIDCalls(), c.Deleted) + assert.Len(t, tx.DeleteDocumentBranchByIDCalls(), c.Deleted) + assert.Len(t, tx.CommitCalls(), c.Committed) + + // the branch row is gone with its content, so the index is + // cleared by branch id. + if c.RespCode == http.StatusNoContent { + require.Len(t, tx.InsertDocumentSearchJobCalls(), 1) + assert.Equal(t, search.BlocksDifference{ + RemovedBranches: []search.BranchRemoval{{DocumentID: _documentID, BranchID: _branchID}}, + }, tx.InsertDocumentSearchJobCalls()[0].Diff) + } }) } } diff --git a/server/core/internal/server/internal/document/http.go b/server/core/internal/server/internal/document/http.go index 89e20f61..7028853c 100644 --- a/server/core/internal/server/internal/document/http.go +++ b/server/core/internal/server/internal/document/http.go @@ -617,7 +617,29 @@ func (h *Handler) UpdateDocumentBranch(w http.ResponseWriter, r *http.Request) { ndoc := doc.ApplyBranchUpdate(ui.Name, ui.Protected, session.UserID) - if err := h.db.UpdateDocumentBranchMetadata(r.Context(), ndoc); err != nil { + var tx Tx + + if err := h.db.BeginTx(r.Context(), &tx); err != nil { + httpserver.RespondError(h.log, w, err) + return + } + + defer tx.Rollback() //nolint:errcheck // error provides no meaningful info + + if err := tx.UpdateDocumentBranchMetadata(r.Context(), ndoc); err != nil { + httpserver.RespondError(h.log, w, err) + return + } + + // the index carries the branch name on every entry of the branch. + if ndoc.BranchName != doc.BranchName { + if err := h.searchJobs.Enqueue(r.Context(), tx, search.BlocksDiff(doc.Search(), ndoc.Search())); err != nil { + httpserver.RespondError(h.log, w, err) + return + } + } + + if err := tx.Commit(); err != nil { httpserver.RespondError(h.log, w, err) return } @@ -755,15 +777,13 @@ func (h *Handler) UpdateDocumentBranchByIDUnsafe(w http.ResponseWriter, r *http. } } - if ndoc.BranchName == documentCore.DefaultBranch { - if err = h.searchJobs.Enqueue( - r.Context(), - tx, - search.BlocksDiff(doc.Search(), ndoc.Search()), - ); err != nil { - httpserver.RespondError(h.log, w, err) - return - } + if err = h.searchJobs.Enqueue( + r.Context(), + tx, + search.BlocksDiff(doc.Search(), ndoc.Search()), + ); err != nil { + httpserver.RespondError(h.log, w, err) + return } err = tx.Commit() @@ -1258,6 +1278,11 @@ func (h *Handler) CreateDocumentBranch(w http.ResponseWriter, r *http.Request) { return } + if err := h.searchJobs.Enqueue(r.Context(), tx, search.BlocksDiff(nil, newDoc.Search())); err != nil { + httpserver.RespondError(h.log, w, err) + return + } + if err := tx.Commit(); err != nil { httpserver.RespondError(h.log, w, err) return @@ -1333,7 +1358,28 @@ func (h *Handler) DeleteDocumentBranch(w http.ResponseWriter, r *http.Request) { return } - if err := h.db.DeleteDocumentBranchByID(r.Context(), branchID, session.ActiveOrganizationID); err != nil { + var tx Tx + + if err := h.db.BeginTx(r.Context(), &tx); err != nil { + httpserver.RespondError(h.log, w, err) + return + } + + defer tx.Rollback() //nolint:errcheck // error provides no meaningful info + + if err := tx.DeleteDocumentBranchByID(r.Context(), branchID, session.ActiveOrganizationID); err != nil { + httpserver.RespondError(h.log, w, err) + return + } + + if err := h.searchJobs.Enqueue(r.Context(), tx, search.BlocksDifference{ + RemovedBranches: []search.BranchRemoval{{DocumentID: branchDoc.ID, BranchID: branchID}}, + }); err != nil { + httpserver.RespondError(h.log, w, err) + return + } + + if err := tx.Commit(); err != nil { httpserver.RespondError(h.log, w, err) return } diff --git a/server/core/internal/server/internal/document/http_test.go b/server/core/internal/server/internal/document/http_test.go index 588e76c4..052da9bb 100644 --- a/server/core/internal/server/internal/document/http_test.go +++ b/server/core/internal/server/internal/document/http_test.go @@ -1395,6 +1395,7 @@ func Test_Handler_UpdateDocumentBranch(t *testing.T) { cc := map[string]struct { DB *DBMock + Tx *TxMock NoSession bool OmitDoc bool Body string @@ -1402,6 +1403,7 @@ func Test_Handler_UpdateDocumentBranch(t *testing.T) { RespCode int Updated int Metadata int + Jobs int }{ "No session in context": { DB: &DBMock{}, @@ -1450,6 +1452,8 @@ func Test_Handler_UpdateDocumentBranch(t *testing.T) { FetchDocumentByBranchIDFunc: func(context.Context, xid.ID, string) (*documentCore.Document, error) { return branchDoc(_branchID), nil }, + }, + Tx: &TxMock{ UpdateDocumentBranchMetadataFunc: func(context.Context, documentCore.Document) error { return errors.New("boom") }, @@ -1458,6 +1462,22 @@ func Test_Handler_UpdateDocumentBranch(t *testing.T) { RespCode: http.StatusInternalServerError, Updated: 1, }, + "Search job insert error": { + DB: &DBMock{ + FetchDocumentByBranchIDFunc: func(context.Context, xid.ID, string) (*documentCore.Document, error) { + return branchDoc(_branchID), nil + }, + }, + Tx: &TxMock{ + InsertDocumentSearchJobFunc: func(context.Context, search.BlocksDifference) error { + return errors.New("boom") + }, + }, + Body: validBody, + RespCode: http.StatusInternalServerError, + Updated: 1, + Jobs: 1, + }, // the main branch is found by its flag in the tree and content // queries, but its name is what the user sees; renaming it would // leave the document looking nameless. @@ -1493,6 +1513,7 @@ func Test_Handler_UpdateDocumentBranch(t *testing.T) { RespCode: http.StatusOK, Updated: 1, Metadata: 1, + Jobs: 1, }, } @@ -1500,21 +1521,37 @@ func Test_Handler_UpdateDocumentBranch(t *testing.T) { t.Run(cn, func(t *testing.T) { t.Parallel() - hdl, cnt := newTestHandler(c.DB, &fakePublisher{}) + tx := c.Tx + if tx == nil { + tx = &TxMock{} + } + + hdl, cnt := newTestHandler(withTx(c.DB, tx, nil), &fakePublisher{}) rec := httptest.NewRecorder() hdl.UpdateDocumentBranch(rec, newRequest(http.MethodPut, c.Body, c.NoSession, c.OmitDoc, false)) assert.Equal(t, c.RespCode, rec.Code) - assert.Len(t, c.DB.UpdateDocumentBranchMetadataCalls(), c.Updated) + assert.Len(t, tx.UpdateDocumentBranchMetadataCalls(), c.Updated) + assert.Len(t, tx.InsertDocumentSearchJobCalls(), c.Jobs) assert.Equal(t, c.Metadata, cnt.metadata) if c.RespCode == http.StatusOK { - ff := c.DB.UpdateDocumentBranchMetadataCalls() + ff := tx.UpdateDocumentBranchMetadataCalls() require.NotEmpty(t, ff) assert.Equal(t, c.WantName, ff[0].Doc.BranchName) assert.True(t, ff[0].Doc.Protected) + assert.Len(t, tx.CommitCalls(), 1) + } + + // a rename rewrites every entry of the branch, since each one + // carries the branch name. + if c.Jobs == 1 && c.RespCode == http.StatusOK { + diff := tx.InsertDocumentSearchJobCalls()[0].Diff + require.Len(t, diff.Updated, 1) + assert.Equal(t, _branchID.String()+"-docname", diff.Updated[0].ID) + assert.Equal(t, "Renamed", diff.Updated[0].BranchName) } }) } diff --git a/web/app/components/SearchModal.nuxt.test.ts b/web/app/components/SearchModal.nuxt.test.ts index 057a07a5..59b2b876 100644 --- a/web/app/components/SearchModal.nuxt.test.ts +++ b/web/app/components/SearchModal.nuxt.test.ts @@ -12,6 +12,23 @@ import { clearTeleportedOverlays, seedAuthOrganization } from "./test-helpers" const DEBOUNCE_MS = 300 const DOC_ID = "doc1".padEnd(20, "0") +const MAIN_ID = "main".padEnd(20, "0") +const DRAFT_ID = "draft".padEnd(20, "0") + +// hit builds a search hit on the main branch, the way core indexes one: +// the entry id carries the branch prefix +function hit(uid: string, type: string, text: string) { + return { + id: `${MAIN_ID}-${uid}`, + documentId: DOC_ID, + organizationId: "org1", + branchId: MAIN_ID, + branchName: "main", + branchDefault: true, + type, + text, + } +} function seedTree() { seedQueryData( @@ -130,9 +147,7 @@ describe("", { concurrent: false }, () => { }) it("lists the matching documents", async ({ expect }) => { - mockSearch([ - { id: DOC_ID, documentId: DOC_ID, type: "document", text: "Runbook" }, - ]) + mockSearch([hit("docname", "document", "Runbook")]) await mountModal() await search("run") @@ -145,9 +160,7 @@ describe("", { concurrent: false }, () => { }) => { seedAuthOrganization({ name: "Acme Corp", slug: "acme-corp" }) seedTree() - mockSearch([ - { id: DOC_ID, documentId: DOC_ID, type: "document", text: "Runbook" }, - ]) + mockSearch([hit("docname", "document", "Runbook")]) await mountModal() await search("run") @@ -162,14 +175,7 @@ describe("", { concurrent: false }, () => { }) => { seedAuthOrganization({ name: "Acme Corp", slug: "acme-corp" }) seedTree() - mockSearch([ - { - id: "block-7", - documentId: DOC_ID, - type: "heading", - text: "Rollback steps", - }, - ]) + mockSearch([hit("block-7", "heading", "Rollback steps")]) await mountModal() await search("roll") @@ -179,13 +185,57 @@ describe("", { concurrent: false }, () => { ) }) + it("links a hit on another branch with that branch selected", async ({ + expect, + }) => { + seedAuthOrganization({ name: "Acme Corp", slug: "acme-corp" }) + seedTree() + mockSearch([ + { + ...hit("block-7", "heading", "Rollback steps"), + id: `${DRAFT_ID}-block-7`, + branchId: DRAFT_ID, + branchName: "draft", + branchDefault: false, + }, + ]) + await mountModal() + + await search("roll") + + expect(document.body.querySelector("a")?.getAttribute("href")).toBe( + `/Acme-Corp/Runbook-${DOC_ID}?branch=${DRAFT_ID}#block-7`, + ) + }) + + it("names the branch of every hit", async ({ expect }) => { + seedTree() + mockSearch([ + hit("docname", "document", "Runbook"), + { + ...hit("block-7", "heading", "Rollback steps"), + id: `${DRAFT_ID}-block-7`, + branchId: DRAFT_ID, + branchName: "draft", + branchDefault: false, + }, + ]) + await mountModal() + + await search("r") + + const branches = [ + ...document.body.querySelectorAll("[data-testid='search-result-branch']"), + ].map((el) => el.textContent.trim()) + + expect(branches).toEqual(["main", "draft"]) + }) + it("falls back to the bare document id when the tree has no such document", async ({ expect, }) => { seedAuthOrganization({ name: "Acme Corp", slug: "acme-corp" }) - mockSearch([ - { id: DOC_ID, documentId: DOC_ID, type: "document", text: "Runbook" }, - ]) + mockSearch([hit("docname", "document", "Runbook")]) await mountModal() await search("run") @@ -199,9 +249,7 @@ describe("", { concurrent: false }, () => { expect, }) => { seedTree() - mockSearch([ - { id: DOC_ID, documentId: DOC_ID, type: "document", text: "Runbook" }, - ]) + mockSearch([hit("docname", "document", "Runbook")]) await mountModal() await search("run") @@ -218,7 +266,7 @@ describe("", { concurrent: false }, () => { ])( "marks a $type hit with its own icon", async ({ type, expected }, { expect }) => { - mockSearch([{ id: "x", documentId: DOC_ID, type: type, text: "Hit" }]) + mockSearch([hit("x", type, "Hit")]) await mountModal() await search("hit") @@ -242,9 +290,7 @@ describe("", { concurrent: false }, () => { it("closes when a result is followed", async ({ expect }) => { seedTree() - mockSearch([ - { id: DOC_ID, documentId: DOC_ID, type: "document", text: "Runbook" }, - ]) + mockSearch([hit("docname", "document", "Runbook")]) const wrapper = await mountModal() await search("run") diff --git a/web/app/components/SearchModal.vue b/web/app/components/SearchModal.vue index b358e27c..c1fc0560 100644 --- a/web/app/components/SearchModal.vue +++ b/web/app/components/SearchModal.vue @@ -82,13 +82,28 @@ function resultLink(result: DocumentSearchResult) { let href = orgSlug ? `/${orgSlug}/${docSlug}` : `/${docSlug}` + // the document page opens the default branch unless told otherwise + if (!result.branchDefault) { + href = href + `?branch=${encodeURIComponent(result.branchId)}` + } + if (result.type !== "document") { - href = href + `#${encodeURIComponent(result.id)}` + href = href + `#${encodeURIComponent(blockUid(result))}` } return href } +// blockUid strips the branch prefix off an index entry id, leaving the +// block's uid, which is what the page anchors on +function blockUid(result: DocumentSearchResult) { + const prefix = `${result.branchId}-` + + return result.id.startsWith(prefix) + ? result.id.slice(prefix.length) + : result.id +} + function resultIcon(type: string) { switch (type) { case "document": @@ -171,7 +186,7 @@ function resultTypeText(type: string) {
-
- {{ resultTypeText(result.type) }} +
+ + {{ resultTypeText(result.type) }} + + + {{ result.branchName }} +
diff --git a/web/app/pages/[[organizationSlug]]/[[documentSlug]].vue b/web/app/pages/[[organizationSlug]]/[[documentSlug]].vue index 62eaf493..e4ba0b8d 100644 --- a/web/app/pages/[[organizationSlug]]/[[documentSlug]].vue +++ b/web/app/pages/[[organizationSlug]]/[[documentSlug]].vue @@ -90,10 +90,14 @@ definePageMeta({ (!docSlugName || !equalNameSlugs(docSlugName, docRealName))) ) { return navigateTo( - `/${createNameSlug(orgRealName)}/${createNameSlugWithId( - docRealName, - docSlugInfo.id, - )}${to.hash || ""}`, + { + path: `/${createNameSlug(orgRealName)}/${createNameSlugWithId( + docRealName, + docSlugInfo.id, + )}`, + query: to.query, + hash: to.hash, + }, { replace: true, }, @@ -223,6 +227,33 @@ watchImmediate( }, ) +// a ?branch= query opens that branch of the document. It keys on the +// route rather than the store so a later switch through the header is not +// undone; an id the document does not have is ignored and the watch below +// falls back to the default branch +watchImmediate( + [() => pageRoute.query.branch, fetchBranches.state], + ([queryBranch, branchState]) => { + const branches = branchState.data + + if (typeof queryBranch !== "string" || !branches?.length) { + return + } + + const branch = branches.find((b) => b.branchId === queryBranch) + const defaultBranch = branches.find((b) => b.default) + + if (!branch || branch.branchId === editorStore.activeBranchId) { + return + } + + editorStore.updateActiveBranchId(branch.branchId) + editorStore.updateTargetBranchId( + branch.default ? null : (defaultBranch?.branchId ?? null), + ) + }, +) + watchImmediate( [ fetchBranches.state, @@ -371,12 +402,14 @@ function applyDocumentNameChange(name: string) { protected: activeDocMetadata.value.protected, }) - void pageRouter.replace( - `/${createNameSlug(fetchOrganization.data.value?.data?.slug || "")}/${createNameSlugWithId( + void pageRouter.replace({ + path: `/${createNameSlug(fetchOrganization.data.value?.data?.slug || "")}/${createNameSlugWithId( name, activeDocMetadata.value.id, - )}${pageRoute.hash || ""}`, - ) + )}`, + query: pageRoute.query, + hash: pageRoute.hash, + }) } function refreshOrganizationRouteSlug() { @@ -384,12 +417,14 @@ function refreshOrganizationRouteSlug() { return } - void pageRouter.replace( - `/${createNameSlug(fetchOrganization.data.value?.data?.slug || "")}/${createNameSlugWithId( + void pageRouter.replace({ + path: `/${createNameSlug(fetchOrganization.data.value?.data?.slug || "")}/${createNameSlugWithId( activeDocMetadata.value.name, activeDocMetadata.value.id, - )}${pageRoute.hash || ""}`, - ) + )}`, + query: pageRoute.query, + hash: pageRoute.hash, + }) } function applyIconChange(icon: string) { diff --git a/web/app/utils/api/document.ts b/web/app/utils/api/document.ts index 18d4bc5c..6b9024a3 100644 --- a/web/app/utils/api/document.ts +++ b/web/app/utils/api/document.ts @@ -225,9 +225,12 @@ export function defaultDocumentHookState( } export interface DocumentSearchResult { - id: string // block/node ID + id: string // branch-scoped index entry ID documentId: string organizationId: string + branchId: string + branchName: string + branchDefault: boolean type: string text: string }