fix(context): handle literal search topics safely (#145) - #146
TheRealBecks wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: f176ded The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
moshest
left a comment
There was a problem hiding this comment.
Sorry this sat for nine days. Reviewed properly now, and the core fix is good — I verified it rather than reading it.
The crashes are real and this fixes them. I reproduced both from #145 against main:
"ExecStart THREW: unterminated string
AND THREW: fts5: syntax error near "AND"
and the same probes on this branch:
"ExecStart ok
AND ok
NEAR ok
(ExecStart*) ok
The approach is sound: after the Unicode strip only letters, numbers, marks, spaces and quotes survive, inner quotes are removed, and every part is re-wrapped — so no FTS metacharacter can reach MATCH. pnpm lint, pnpm build and pnpm test are all clean (251 context + 49 registry).
One regression worth your call
Dropping _ from the preserved set changes snake_case identifiers. \w used to keep it, so spring_boot reached FTS5 as a bareword and was tokenized into an adjacent phrase. Now it becomes two independently-ANDed terms.
Fixtures: a.md contains "Use spring_boot_app and health_changed.emit here."; b.md contains "Unrelated spring text and separate boot text."
| query | main |
this PR |
|---|---|---|
spring_boot_app |
[a.md] |
[a.md] |
health_changed |
[a.md] |
[a.md] |
spring_boot |
[a.md] |
[b.md, a.md] |
So searching a snake_case identifier now matches a document containing those words separately — and here ranks it first, above the document that actually contains the identifier. That's the same failure mode #145 describes for GDScript/C#: a plausible match from elsewhere dominating the intended documentation.
The obvious fix is preserving _ in the class:
.replace(/[^\p{L}\p{N}\p{M}_\s"]/gu, " ")spring_boot then stays one part, becomes "spring_boot", and FTS5 tokenizes inside the quoted string back into an adjacent phrase — restoring the old behaviour while keeping all the injection safety, since the part is still quoted.
One thing to check if you take that: your "___" case expects no results. With _ preserved, ___ survives as a part and becomes "___", which unicode61 tokenizes to zero tokens. I haven't verified whether FTS5 treats an empty phrase as no-match or as an error, and that's worth confirming before adopting the one-character change — it may need parts with no letters or digits filtered out after quoting.
Not blocking on my account: it's narrow, and you may reasonably decide looser identifier matching is an acceptable trade for the safety. But it is a behaviour change that isn't in the changeset or the README section, so it should be a decision rather than a side effect.
The retrieval fixtures are a genuinely good addition — building through the real pipeline rather than hand-stuffing the FTS table is what makes them worth having.
Generated by Claude Code
Fixes #145.
Search topics such as
"ExecStartandANDcould trigger SQLite FTS syntax errors. This change safely quotes literal search terms while preserving paired double quotes for phrase searches. Unmatched quotes are ignored, punctuation separates words, Unicode keywords are preserved, and empty or punctuation-only topics return no results.Adds local fixtures built through the real ingestion pipeline to verify:
Updates CLI documentation and MCP guidance, and includes a patch changeset.
Validation: frozen dependency install, lint, build, and all 300 tests pass.