fix(knowledge): align document tag provenance selections with the serialized request - #6332
Conversation
…ialized request
The create/upsert document tools counted one provenance selection pair per
parseDocumentTags entry, while the write route built targets from the
serialized documentTagsData and dropped entries whose value is the empty
string. A tag value that is truthy before coercion but stringifies to empty
(`[]`, `[null]`, `{ toString: () => '' }`) was therefore counted by the tool
and not by the route, and the bundle length check rejected the write with 400.
Both sides now read one shared parser over the exact bytes that go on the
wire, so their counts cannot diverge.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview The tool path now mirrors the wire format: Adds Reviewed by Cursor Bugbot for commit 6e07db1. Configure here. |
Greptile SummaryThe PR aligns knowledge-document tag provenance selections with the serialized request consumed by the API route.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/lib/knowledge/secret-provenance-selection.ts | Centralizes the shared parser that filters serialized document-tag provenance targets. |
| apps/sim/tools/knowledge/secret-provenance.ts | Derives producer-side provenance selections from the same serialized representation used by the API. |
| apps/sim/app/api/knowledge/secret-provenance.ts | Updates the route-side import to use the shared database-independent target parser. |
| apps/sim/lib/knowledge/secret-provenance.ts | Removes the former database-coupled copy of the document-tag target parser. |
| apps/sim/tools/knowledge/secret-provenance.test.ts | Covers divergent pre- and post-serialization tag values and fully applies the previously requested const assertion. |
Reviews (2): Last reviewed commit: "test(knowledge): use as const for the em..." | Re-trigger Greptile
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 6e07db1. Configure here.
Problem
Same bug class as the
selectTableRowSecretProvenanceoutage fixed in #6325: a private-provenance producer counting pre-serialization semantics while the consumer counts post-serialization.Knowledge document writes (
knowledge_create_document,knowledge_upsert_document):selectKnowledgeDocumentWriteSecretProvenancecounted one name/value selection pair perparseDocumentTagsentry.parseDocumentTagskeeps an entry whosevalueis merely truthy, then coercesvalue: String(entry.value).parseKnowledgeDocumentTagProvenanceTargetsbuilds targets from the serializeddocumentTagsDataand drops entries wherevalue === ''.resolveKnowledgeWriteSecretProvenancerejects the write with HTTP 400 whenbundle.selections.length !== selectionKeys.length.So any tag value that is truthy before coercion but stringifies to empty —
[],[null],[undefined], an object whosetoString()returns''— was counted by the tool and dropped by the route.Reproduced by executing both parsers against
[{ tagName: 'kept', value: 'value' }, { tagName: 'dropped', value: [] }]: the tool emitted 6 selection keys, the route derived 4.Not observed in production logs — found by code-level audit.
Semantics
Drop, matching the route. The functional write path agrees already:
resolveDocumentTagsskips any tag whose value fails itshasValuecheck, so an empty-stringifying tag never persists a value and has nothing to attribute provenance to.Fix
Following the
update_chunkpattern (its selector mirrors its own body builder exactly), the producer now describes exactly what goes on the wire: it runs the sameformatDocumentTagsForAPI(parseDocumentTags(...))the body builder runs, then feeds the resultingdocumentTagsDatathrough the same target parser the route uses.parseKnowledgeDocumentTagProvenanceTargetsmoved fromlib/knowledge/secret-provenance.ts(which pulls in@sim/db) to the db-freelib/knowledge/secret-provenance-selection.tsboth sides already import, so there is one parser, not two rules that can drift.Tests
apps/sim/tools/knowledge/secret-provenance.test.tsasserts the tool's selection keys equal the route-derived target keys for each divergent value, plus a non-divergent case.Verified red before the fix (
expected 6 keys to equal 4), green after.Audit of the other provenance producer/consumer pairs
selectTableRowSecretProvenance↔createTableWriteProvenanceTargets)undefined, butJSON.stringifyalso drops function- and symbol-valued keys, which the consumer'sObject.entriesover the parsed row would then not see. Not reachable from JSON-sourced tool params — left alone.file_write,file_append)['content']on both sides.memory_add)1on both sides.upload_chunk,update_chunk)update_chunk's conditional producer mirrors both its body builder and the route'svalidatedData.content === undefinedcheck.1on both sides.Checks
bunx tsc --noEmit -p tsconfig.json— cleanbun run lint— clean (9 pre-existing unrelated warnings)app/api/knowledge/secret-provenance.test.ts+ new test — 14 passedtools/knowledge/knowledge.test.tsfails identically on a clean checkout (postcss/tailwind env issue), unrelated