Skip to content

fix(context): handle literal search topics safely (#145) - #146

Open
TheRealBecks wants to merge 1 commit into
neuledge:mainfrom
TheRealBecks:TheRealBecks/issues145
Open

TheRealBecks wants to merge 1 commit into
neuledge:mainfrom
TheRealBecks:TheRealBecks/issues145

Conversation

@TheRealBecks

Copy link
Copy Markdown
Contributor

Fixes #145.

Search topics such as "ExecStart and AND could 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:

  • Expected documents appear within the top three results.
  • GDScript and C# queries rank the intended language first.
  • Results preserve source attribution and code examples within the token budget.

Updates CLI documentation and MCP guidance, and includes a patch changeset.

Validation: frozen dependency install, lint, build, and all 300 tests pass.

@changeset-bot

changeset-bot Bot commented Sep 9, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: f176ded

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@neuledge/context Patch
@neuledge/registry Patch

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 moshest left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Handle literal search topics safely and add retrieval regression fixtures

2 participants