fix(platform): knowledge folder search, crawl revival, admin doors - #3174
Draft
larryro wants to merge 6 commits into
Draft
fix(platform): knowledge folder search, crawl revival, admin doors#3174larryro wants to merge 6 commits into
larryro wants to merge 6 commits into
Conversation
applyPin returned early when <schema>.chunks did not exist, yet pinDimensions
still recorded the database as pinned and the schema as applied. Every later
write then skipped the ALTER and the HNSW build for the process lifetime, so a
knowledge database that migrated after the platform booted kept an untyped
vector column — any width accepted, no index — until a restart.
applyPin now reports whether the pin took effect; nothing is recorded when the
table was absent, and the next write tries again. The test that codified the
drift ("leaves nothing pinned" asserting 1536) now asserts nothing pinned, and
a new one proves the pin lands once the table appears.
Three dead-ends on the knowledge routes:
- The BYO connection body re-declared sslmode as a four-value enum that
omitted verify-ca, one of the five modes the data-residency picker offers
and the shared pgConnectionSchema accepts. Saving or testing such a
connection died at the gate as a bare "invalid body". The body schema is
now the shared field schemas in a strip-mode object — one source of truth.
- checkProviderHostPolicy refuses with a coded AppError; the routes mapped
only KnowledgeAdminError, so a private-host or metadata-host refusal fell
through to the generic handler as a 500 with an error report, and the
sentence naming the TALE_ALLOW_PRIVATE_PROVIDER_HOSTS opt-in never reached
the admin. admin.ts translates it to a KnowledgeAdminError (400 with code
and message); the probe reports it as { ok: false, error } like every other
test failure; /connection/test gained the error mapping it lacked.
- /fetch accepted a page parameter and ignored it, shipping the whole
document regardless. page is now a 1-based FETCH_WINDOW_CHARS window with
page/totalPages/totalChars alongside; no page keeps today's response.
Route tests drive all three through Hono; admin tests prove the policy
translation without any I/O.
A page that answered 404/410 once was gone for good: the crawler marked its website_urls row 'deleted', the due-page predicate skipped it, and every door that re-admits URLs left existing rows alone — discovery and rendered- link admission with ON CONFLICT DO NOTHING, registerUrlList only flipping listed. A deploy gap or a misconfigured origin dropped the page from the index until someone deleted and re-added the whole website. All three doors now speak one admission statement (admitUrls): a new row starts 'discovered', a 'deleted' row is revived to 'discovered' with its fail_count cleared, a live row is untouched except listed (which only widens). Discovery finding the link again, or the operator re-listing the URL, is the only signal a restored page sends, and the next fetch decides. Listed URLs are a standing instruction, so each scan also revives listed 'deleted' rows (reviveListedUrls) — one probe per scan is the bounded cost, the scan interval the backoff. MAX_FETCH_FAILURES saturation is left as is. Unit tests pin the statement and the batching; the integration check proves a 404'd page (site and list kind) re-indexes when it is back.
The failed-indexing dialog branches on rag_error_code === 'embedding_not_configured' to show the Settings → Data residency deep link (or "ask an admin"), and the document view selects the column — but no 0.5 writer ever set it: writeRagStatus had no field, and the EmbeddingNotConfigured catch wrote prose telling members to "set knowledge/embedding.json", an operator file path they cannot act on. The guided in-app fix never appeared. writeRagStatus now carries ragErrorCode next to ragError — written together, cleared together, so a retry that succeeds leaves no stale cause — and the EmbeddingNotConfigured path writes RAG_ERROR_EMBEDDING_NOT_CONFIGURED with prose that points at Settings → Data residency → Embedding model. Proof rides the integration check: an upload before any model is configured fails with the code and the Settings prose; retry-rag after configuring one completes with both cleared.
Folder-scoped knowledge search returned nothing, always. indexUploadedFile never passed folderPath to indexDocument (which accepts it), so every 0.5 corpus row carried folder_path NULL and the corpus prefix clause matched no document; the retrievable re-check received folder and silently dropped it. /search accepted and forwarded the filter the whole time. The stamp is the document's canonical folder path — the folder tree's breadcrumb when the document sits in a folder (fresh across renames and moves), else the denormalized documents.folder_path a connector wrote, normalized to 'A/B' (two spellings coexist today: WebDAV stores the 0.4 '/A/B', the tree and the sync engines produce 'A/B'; normalizeFolderPath makes the spelling unable to decide whether a document is found). The helpers live in folders/paths.ts and are the one rule every path-comparing surface applies: - ingest stamps folder_path from the bound document; - searchKnowledgeForOrg normalizes the incoming folder, and the re-check decides from each document's CURRENT folder (decideRetrievable gains folder; unbound files — filed nowhere — never admit under a folder filter); - syncRagDocumentScope reads the current row and re-stamps scope AND folder; called on document move (app door and REST v1 — the latter never re-stamped even on a team change), OneDrive re-file, and WebDAV MOVE of a document; - syncRagFolderSubtree re-stamps every live file-backed document beneath a folder after rename (app door) or WebDAV folder MOVE; - the agent-facing listing prefers the tree and normalizes too, so an agent can hand a listed folderPath straight back as a search filter. Unit tests cover the decision and the path helpers; the integration check proves a filed document is found under its folder in both spellings, not under another folder, that the root document stays out, and that the stamp follows a move and a folder rename.
Seven integration lanes, each red on the base commit and green here: - indexing without an embedding model writes rag_error_code and Settings prose; retry-rag after configuring a model clears both; - folder-scoped search finds the folder's documents (both spellings), not another folder's, and never the root document; the stamp follows a document move and a folder rename; - /fetch honours page as a FETCH_WINDOW_CHARS window with totalPages; - the admin doors accept sslmode verify-ca and answer a host-policy refusal as a reported probe result / a coded 400, writing nothing; - a site-kind page that 404'd once re-indexes when it is back (sitemap discovery revives the deleted row); - a list-kind listed page that 404'd is re-probed on the next scan. The websites lane's fake fetch now also serves the URL-list domain — it never did, so the listed page of step 4 was never actually indexed by that lane; the site-domain page count in step 4 is 3 again after the revival step.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Six verified medium-severity knowledge-domain defects — dead-ends where a feature silently returned nothing, and misleading signals — from the backend deep-review campaign. Base is
fedc8cc15(has #3140 / #3160). No migration.Per-finding outcome
service.ts:447)indexUploadedFilenever passedfolderPathtoindexDocument, so every 0.5 corpus row hadfolder_path NULLand the prefix clause matched nothing; the retrievable re-check droppedfolder. Now: the ingest stamps the document's canonical folder path; the re-check decides from the document's CURRENT folder (decideRetrievable(…, folder));syncRagDocumentScopere-stamps folder + scope from the current row and is called on document move (app + REST v1), OneDrive re-file, WebDAV MOVE;syncRagFolderSubtreere-stamps everything beneath a renamed/moved folder (app rename, WebDAV folder MOVE). Base:stamp=NULL, zero hits underReports; branch: found underReportsand/Reports/, not underInvoices, follows move →Archiveand rename →Vault. Also in this finding:/fetchacceptedpageand ignored it — now aFETCH_WINDOW_CHARSwindow withpage/totalPages/totalChars.crawl.ts:130)status='deleted'; all three re-admission doors skipped existing rows (DO NOTHING×2,SET listed = TRUE). One admission statement (admitUrls) now revives adeletedrow todiscoveredwithfail_countcleared when discovery links it again or the operator re-lists it; each scan also revives listeddeletedrows (reviveListedUrls) — the list is a standing instruction, one probe per scan is the bounded cost. Base: restored page staysdeleted, 0 chunks; branch:active, chunks ≥ 1, page count back to 3 (site kind) and the listed page re-indexes (list kind).rag_error_codeis read by the UI but never written (service.ts:312)view.tsselects it, the badge branches onembedding_not_configuredto show the Settings → Data residency deep link — butwriteRagStatushad no field. Now written alongsiderag_error(and cleared with it); theEmbeddingNotConfiguredprose points at Settings instead of an operator file path. Base:code=NULL, prose namesknowledge/embedding.json; branch:code=embedding_not_configured, retry after configuring clears both.dimensions.ts:115)applyPinreturned early on 42P01 yet.then()recorded pinned + applied, so later writes skipped the ALTER/HNSW for the process lifetime.applyPinnow reports whether it took effect; nothing is recorded otherwise and the next write retries. The test that codified the drift ("leaves nothing pinned"asserting 1536) now asserts nothing pinned; a new test proves the pin lands once the table appears.verify-caoffered in UI but rejected by route-local enum copy (routes.ts:157)knowledgeConnectionSchemafield schemas in a strip-mode object (one source of truth; strictness stays the config-file policy). Base: probe withverify-ca→ 400invalid body; branch: accepted.admin.ts:168)checkProviderHostPolicythrowsAppError; the routes mapped onlyKnowledgeAdminError(and/connection/testhad no mapping at all).assertHostAllowedtranslates toKnowledgeAdminError(code, message, 400); the probe reports{ ok: false, error }like every other test failure. Base: save → 500; branch: 400BLOCKED_HOST/PRIVATE_HOST_BLOCKEDwith the sentence namingTALE_ALLOW_PRIVATE_PROVIDER_HOSTS=1.Tests
Colocated vitest (all red on base — 27 of the new/changed assertions fail against
fedc8cc15, the three invariants pass on both sides):core/knowledge/dimensions.test.ts— corrected drift assertion + late-table pin (17 tests)core/knowledge/crawl.test.ts— the admission statement (revive/listed/placeholders), batching, dedupe,registerUrlList,reviveListedUrls(8)domains/knowledge/retrievable.test.ts— folder filter: containment, separator, siblings, root, access on top, unbound files never admit, no-op without folder (+5)domains/folders/paths.test.ts—normalizeFolderPath,documentFolderPathFrom(6)domains/knowledge/routes.test.ts— verify-ca save/probe, out-of-set mode still 400, policy refusal → 400 with code on save and probe, fetch paging window / whole / miss (8)domains/knowledge/admin.test.ts— metadata host →BLOCKED_HOST400, private host names the env opt-in, blocked embedding baseUrl, probe reportsok:false(4)Integration check (
backend:integration, real Postgres + MinIO), seven new lanes: unconfigured-embedding upload writes the code; retry after configuring clears it; folder-scoped search finds the folder's documents (both spellings, not other folders, root doc excluded); stamp follows move and rename; fetch page window; admin doors (verify-ca accepted, host refusal coded 4xx, nothing written); site-kind 404'd page re-indexes when back; list-kind listed page re-probed next scan.Verification
bunx tsc --noEmit(platform): clean.bunx oxlint --type-awareon every touched file: clean.backend:integrationon throwawaytale-db+ MinIO, fresh containers per run,SANDBOX_LLM_GATEWAY_ADMIN_PASSWORDset: branch 382/383, base 374/383 with this branch'sintegration-check.ts— the base reds are exactly the new knowledge lanes plus the pre-existingwebdav re-homelane, which fails on base with byte-identical detail (unrelated, noted below).Cross-class discoveries (not fixed here)
MAX_FETCH_FAILURESsaturation is a sibling dead-end: a page that failed 5 scans in a row (non-404 errors) is never fetched again and nothing resetsfail_countexcept a success it can no longer have. Left as designed (the cap's intent is consecutive-failure backoff); reviving on re-discovery would defeat it. Worth a bounded retry policy of its own.app.documents.folder_path: WebDAV writes the 0.4'/A/B', OneDrive/buildHubFolderPathwrite'A/B'; and the denormalized column goes stale after a folder rename (only the folder row is renamed).normalizeFolderPath+ tree-first resolution make the knowledge filter immune;agent-list.tsnow prefers the tree too (and normalizes), so agents can hand a listed path straight back as afolderfilter. Other readers of the raw column (searchDocumentsViewsnippet) still show the stale/unnormalized value.PATCH /documents/:idnever re-stamped the corpus on a team change (only the app door did) — fixed in passing here since the same seam carries the folder re-stamp.webdav re-homeintegration lane fails on base and branch with identical detail (chunked=500 (want 500) … ghost={"deleted":false}) — pre-existing, not touched here.