fix(search): ignore empty-string label in search_graph name_pattern path#483
Open
halindrome wants to merge 2 commits into
Open
fix(search): ignore empty-string label in search_graph name_pattern path#483halindrome wants to merge 2 commits into
halindrome wants to merge 2 commits into
Conversation
search_where_basic guarded the label filter on NULL only (`if (params->label)`), so a search passing label="" appended a literal `n.label = ''` clause that matches no node — silently returning zero results. The BM25 query path already treats an empty label as absent, so name_pattern/qn_pattern searches and BM25 searches behaved inconsistently for the same `"label":""` payload (issue DeusData#481): agents fell back to grep because structural class/service discovery returned nothing. Guard the clause on `params->label && params->label[0]` so an empty-string label is a no-op, identical to an omitted label and to the BM25 path. Adds store_search_empty_label_ignored: name_pattern + label="" returns the same match as name_pattern alone, while a non-empty label still filters. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Shane McCarron <shane.mccarron@corvexconnect.com>
Contributor
Author
QA Round 1 — CLEANReviewer model: claude-opus-4-8 · Contract source: issue #481 · Base: Contract Verification
Findings
Advisory (non-blocking)
VerdictCLEAN. One-line guard fully satisfies #481 for both pattern paths, genuinely matches BM25 behavior, no regression, memory-safe. Regression test fails on old code ( |
QA round 1 noted store_search_empty_label_ignored asserted only the name_pattern path though the comment referenced qn_pattern too. Add a qn_pattern + label="" assertion so a future change that special-cased the qn_pattern branch can't silently reintroduce the empty-label bug. Both paths share search_where_basic, so this locks in the shared guard. Test-only; no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Shane McCarron <shane.mccarron@corvexconnect.com>
Contributor
Author
|
Addressed QA round 1 advisory F-01 in 19a0fc5 (test-only): |
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
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.
Closes #481
Problem
search_graphreturns zero results when a request passes an empty-stringlabel("label": "") together withname_pattern(orqn_pattern). The reporter saw it as "name_patternmisses indexed Class nodes," but the trigger is the empty label, not the regex:name_pattern=".*Foo.*"(nolabelkey)name_pattern=".*Foo.*",label=""name_pattern=".*Foo.*",label="Class"query="Foo",label=""(BM25)The BM25
querypath treats an empty label as absent; thename_patternpath does not — so the same"label":""payload works forquerybut returns nothing forname_pattern. Agents then fall back to grep even though the symbol is in the graph.Root cause
search_where_basic(src/store/store.c) guarded the label clause on NULL only:Fix
An empty
labelis now a no-op, identical to an omitted label and to the BM25 path. Non-empty labels still filter exactly as before.Test
store_search_empty_label_ignored(tests/test_store_search.c):name_pattern+label=""returns the same match asname_patternalone, and a non-emptylabelstill filters (.*Order.*+label="Class"→ only the Class node).Verification
scripts/build.shclean;scripts/test.shpasses (the lonecli_hook_gate_script_no_predictable_tmp_issue384failure is pre-existing onmain, environment-dependent, untouched here).name_patternmatches all expected Class nodes.🤖 Generated with Claude Code