fix(query): skip covered-term seed guarantee; relax starving heuristic context filters (#2507) - #2516
fix(query): skip covered-term seed guarantee; relax starving heuristic context filters (#2507)#2516filipechagas wants to merge 2 commits into
Conversation
…c filters; bump to 0.9.36 (Graphify-Labs#2507) Two follow-ups to 0.9.35's relational-verb demotion, on the same natural-language impact question ("who calls X?") and graded on the same 48.5k-node PHP corpus. - `_pick_seeds` gains a keyword-only, default-off `skip_covered_terms`: a term that is a substring of an already-picked seed's normalized label is not starved, so it claims no additional guaranteed seed. Layered after the Graphify-Labs#1766 dedup gate, in the same sorted-term order. Coverage reads the seed's LABEL only, never the `or nid` dedup key, so a labelless (`_fold_node_aliases`) seed cannot declare unrelated terms covered through its path fragments. Opted into by `_query_graph_text` alone; `path`, `explain`, the legacy-equality property tests and the benchmark's arms are unchanged. - `_query_graph_text` retraverses unfiltered when a *heuristically inferred* context filter discovers nothing beyond the seeds, and annotates the `Context:` header. A class node owns no call edges, so the `call` filter that "Who calls X?" infers strands a correctly-seeded class at exactly one node. Threshold is zero expansion (`nodes <= set(start_nodes)`), not a tuning constant; identical in BFS and DFS; explicit filters are never relaxed. 0.9.35 added `caller`/`callers` to `_CONTEXT_HINTS`, so "callers of X" infers the same filter and hit the same stranding — measured on the corpus, both phrasings returned 1 node before this change and 128 after, with all three known callers in the shown portion. New `tests/seeding_fixtures.py` carries a fixture wired the way extraction really wires a service (calls land on the method; the class->method edge has no context), which is what makes the stranding observable; `test_serve_seeding.py` pins all three seeding behaviours, including regression pins for the landed 0.9.35 demotion.
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.
Graphify review — findings
This PR bumps the version from 0.9.35 to 0.9.36 and modifies the query seeding logic in graphify/serve.py. It adds a skip_covered_terms option to _pick_seeds (opted into only by the natural-language query path) that suppresses the per-term seed guarantee when a term is already a substring of a picked seed's normalized label, and adds a fallback in _query_graph_text that retraverses unfiltered when a heuristically inferred context filter reaches nothing beyond the seeds, annotating the header accordingly. It also adds a new shared test fixture file (tests/seeding_fixtures.py) and corresponding test changes, plus changelog entries describing these two fixes (#2507).
Worth a look
- Covered-term skip can starve a legitimately distinct term whose winner shares a substring with an unrelated seed —
graphify/serve.py:758· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 649 functions depend on the 345 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
_query_graph_text()— 29 callers, 6 callees - worse:
_score_query()— 21 callers, 5 callees - worse:
_query_terms()— 22 callers, 3 callees - new:
make_charge_fixture()— 14 callers, 3 callees - new:
test_generic_noun_phrasing_seeds_no_hub_and_stays_bounded()— 0 callers, 11 callees - new:
test_callers_of_phrasing_renders_all_callers_and_drops_junk_seed()— 0 callers, 7 callees - new:
test_who_calls_phrasing_falls_back_when_heuristic_filter_strands_the_seed()— 0 callers, 7 callees - new:
test_expanding_heuristic_filter_is_left_in_force()— 0 callers, 6 callees - …and 1 more
Verification — 649 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 509 function(s) in the blast radius were not formally verified this run
· 7 grounded finding(s) anchored inline below; 3 more finding(s) on lines outside this diff (see the check run).
| # Layered after that dedup gate, in this same sorted-term order, so | ||
| # the coverage check sees the gap-window seeds plus every guarantee | ||
| # seed appended so far. | ||
| if skip_covered_terms and any(term in _seed_norm_label(s) for s in seeds): | ||
| continue | ||
| seen_labels.add(key) |
There was a problem hiding this comment.
Covered-term skip can starve a legitimately distinct term whose winner shares a substring with an unrelated seed — agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Graphify suggests a fix:
| # Layered after that dedup gate, in this same sorted-term order, so | |
| # the coverage check sees the gap-window seeds plus every guarantee | |
| # seed appended so far. | |
| if skip_covered_terms and any(term in _seed_norm_label(s) for s in seeds): | |
| continue | |
| seen_labels.add(key) | |
| # Layered after that dedup gate, but only the TOP-ranked seed — the | |
| # query's dominant match — can declare a term covered. A coincidental | |
| # substring collision inside some unrelated, lower-ranked seed's label | |
| # must not silently starve a distinct term's real winner. | |
| if skip_covered_terms and seeds and term in _seed_norm_label(seeds[0]): | |
| continue | |
| seen_labels.add(key) |
There was a problem hiding this comment.
Verified and adopted in 234c5f0 — thanks. The finding reproduces: with a node labelled exactly port (its term's winner by ~7,300x in _score_query) and an unrelated ReportService in the gap window, the old predicate let ReportService absorb "port" and the real winner was starved, while the legacy picker seeds it correctly. I've taken the suggested seeds[0] refinement (guarded on seeds being non-empty), which restates the invariant as every term with any match is matched by the top-ranked seed or by a seed of its own, and added the repro as a regression test along with the docstring, comment and CHANGELOG corrections. Measured behaviour on the graded corpus is unchanged — all three phrasings return byte-identical output to the previous commit, since the covering seed there was always the top seed.
| confidence="EXTRACTED", _src=src, _tgt=tgt) | ||
|
|
||
|
|
||
| def make_charge_fixture(*, calls_symbol: bool = False) -> nx.Graph: |
There was a problem hiding this comment.
make_charge_fixture()
14 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
There was a problem hiding this comment.
Intended shape rather than a regression: the 14 callers are the point of this fixture — all three behaviours (verb demotion, covered-term skip, filter relaxation) grade the same corpus instead of each test re-inventing a graph, which is what makes the phrasing results comparable across the suite. The fan-in is confined to tests/ and no production code depends on it.
| _COVERED_AND_STARVED_QUESTION = "ChargeCustomerService customer code" | ||
|
|
||
|
|
||
| def test_generic_noun_phrasing_seeds_no_hub_and_stays_bounded(): |
There was a problem hiding this comment.
test_generic_noun_phrasing_seeds_no_hub_and_stays_bounded()
fans out to 11 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
There was a problem hiding this comment.
The callees here are the shared fixture plus the small output-parsing helpers (start_labels, shown_nodes, label_of, caller_labels) that keep every assertion reading rendered text instead of internals. That indirection is deliberate — it's what makes these tests robust to vocabulary tweaks — and it's test-local; no production coupling changes.
| # *explicit* filter is always honored. # | ||
| # --------------------------------------------------------------------------- # | ||
|
|
||
| def test_who_calls_phrasing_falls_back_when_heuristic_filter_strands_the_seed(): |
There was a problem hiding this comment.
test_who_calls_phrasing_falls_back_when_heuristic_filter_strands_the_seed()
fans out to 7 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
There was a problem hiding this comment.
Same as the sibling threads: the fan-out is the shared fixture plus the header/output-parsing helpers that keep assertions on external behaviour (rendered Start: seeds, NODE lines, the Context: note) rather than internals. Test-local by design; no production coupling changes. No action taken — happy to restructure if the metrics gate treats this as blocking.
| assert "relaxed" in context, f"header does not report the relaxation: {context!r}" | ||
|
|
||
|
|
||
| def test_callers_of_phrasing_renders_all_callers_and_drops_junk_seed(): |
There was a problem hiding this comment.
test_callers_of_phrasing_renders_all_callers_and_drops_junk_seed()
fans out to 7 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
There was a problem hiding this comment.
Same as the sibling threads: the fan-out is the shared fixture plus the header/output-parsing helpers that keep assertions on external behaviour (rendered Start: seeds, NODE lines, the Context: note) rather than internals. Test-local by design; no production coupling changes. No action taken — happy to restructure if the metrics gate treats this as blocking.
| ) | ||
|
|
||
|
|
||
| def test_expanding_heuristic_filter_is_left_in_force(): |
There was a problem hiding this comment.
test_expanding_heuristic_filter_is_left_in_force()
fans out to 6 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
There was a problem hiding this comment.
Same as the sibling threads: the fan-out is the shared fixture plus the header/output-parsing helpers that keep assertions on external behaviour (rendered Start: seeds, NODE lines, the Context: note) rather than internals. Test-local by design; no production coupling changes. No action taken — happy to restructure if the metrics gate treats this as blocking.
| ) | ||
|
|
||
|
|
||
| def test_single_node_expansion_is_not_starvation(): |
There was a problem hiding this comment.
test_single_node_expansion_is_not_starvation()
fans out to 6 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
There was a problem hiding this comment.
Same as the sibling threads: the fan-out is the shared fixture plus the header/output-parsing helpers that keep assertions on external behaviour (rendered Start: seeds, NODE lines, the Context: note) rather than internals. Test-local by design; no production coupling changes. No action taken — happy to restructure if the metrics gate treats this as blocking.
…hify-Labs#2507) Review follow-up. The covered-term skip let ANY picked seed absorb a term by substring, so a coincidental collision inside an unrelated, lower-ranked seed's label starved the term's real winner: `ReportService` contains "port", which cost a corpus symbol literally named `port` the guaranteed seat that was its only way into the seed list — Graphify-Labs#1597's concern one layer down. Coverage asserts "the query's dominant match already answers this term", which is a claim only the top-ranked seed is entitled to make, so the predicate now reads `seeds[0]` alone (guarded on `seeds` being non-empty). `_seed_norm_label` keeps its label-only semantics, so a labelless seed still covers nothing. The refined invariant: every term with any match is matched by the top-ranked seed or by a seed of its own. Behaviour on the graded corpus is unchanged — all three phrasings return output byte-identical to the previous commit's, because the covering seed there is always the top seed.
|
Review round addressed in 234c5f0: the covered-term advisory was verified with a reproducing execution (details in the thread) and the suggested top-ranked-seed refinement adopted, with the repro added as a regression test and the docstring/comment/CHANGELOG wording corrected. Both mutation probes still kill their intended tests, the full suite is green (4,021 passed; the 4 |
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.
Graphify review — findings
This PR bumps the package version from 0.9.35 to 0.9.36 and adds two related changes to the natural-language query seeding pipeline in graphify/serve.py. It introduces a skip_covered_terms option to _pick_seeds (opted into only by _query_graph_text) that skips the per-term seed guarantee when a term is a substring of the top-ranked seed's normalized label, and adds a fallback in _query_graph_text that retraverses unfiltered when a heuristically inferred context filter discovers nothing beyond the seeds, annotating the header accordingly. The change also adds a new shared test fixture file (tests/seeding_fixtures.py) and associated tests, plus CHANGELOG entries describing both fixes. The surface area is concentrated in seed selection, context-filter handling, header formatting, and test scaffolding.
Worth a look
- Covered-term skip uses substring test, matching arbitrary short terms —
graphify/serve.py:771· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 651 functions depend on the 347 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
_query_graph_text()— 29 callers, 6 callees - worse:
_score_query()— 22 callers, 5 callees - worse:
_query_terms()— 22 callers, 3 callees - new:
make_charge_fixture()— 14 callers, 3 callees - new:
test_generic_noun_phrasing_seeds_no_hub_and_stays_bounded()— 0 callers, 11 callees - new:
test_callers_of_phrasing_renders_all_callers_and_drops_junk_seed()— 0 callers, 7 callees - new:
test_who_calls_phrasing_falls_back_when_heuristic_filter_strands_the_seed()— 0 callers, 7 callees - new:
test_expanding_heuristic_filter_is_left_in_force()— 0 callers, 6 callees - …and 1 more
Verification — 651 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 511 function(s) in the blast radius were not formally verified this run
· 6 grounded finding(s) anchored inline below; 3 more finding(s) on lines outside this diff (see the check run).
| confidence="EXTRACTED", _src=src, _tgt=tgt) | ||
|
|
||
|
|
||
| def make_charge_fixture(*, calls_symbol: bool = False) -> nx.Graph: |
There was a problem hiding this comment.
make_charge_fixture()
14 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| _COVERED_AND_STARVED_QUESTION = "ChargeCustomerService customer code" | ||
|
|
||
|
|
||
| def test_generic_noun_phrasing_seeds_no_hub_and_stays_bounded(): |
There was a problem hiding this comment.
test_generic_noun_phrasing_seeds_no_hub_and_stays_bounded()
fans out to 11 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| # *explicit* filter is always honored. # | ||
| # --------------------------------------------------------------------------- # | ||
|
|
||
| def test_who_calls_phrasing_falls_back_when_heuristic_filter_strands_the_seed(): |
There was a problem hiding this comment.
test_who_calls_phrasing_falls_back_when_heuristic_filter_strands_the_seed()
fans out to 7 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| assert "relaxed" in context, f"header does not report the relaxation: {context!r}" | ||
|
|
||
|
|
||
| def test_callers_of_phrasing_renders_all_callers_and_drops_junk_seed(): |
There was a problem hiding this comment.
test_callers_of_phrasing_renders_all_callers_and_drops_junk_seed()
fans out to 7 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| ) | ||
|
|
||
|
|
||
| def test_expanding_heuristic_filter_is_left_in_force(): |
There was a problem hiding this comment.
test_expanding_heuristic_filter_is_left_in_force()
fans out to 6 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| ) | ||
|
|
||
|
|
||
| def test_single_node_expansion_is_not_starvation(): |
There was a problem hiding this comment.
test_single_node_expansion_is_not_starvation()
fans out to 6 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
|
Status note, since #2507 is now closed as fixed and this PR is the part of it that remains open. 0.9.35 fixed direction 2 of the report — the relational-verb demotion — and this branch takes that commit byte-identical and does not relitigate it. The two failure modes from the original report that survive it are what is here:
Worth separating out, because it is a regression rather than a leftover: 0.9.35 also added
Confidently empty is the part that concerns us more than the burying did. The existing Branch status: rebased on No urgency from our side — we run these on our fork. Happy to split the two changes into separate PRs, or to take the structural question from the description (deriving the demotion vocabulary from |
|
One more consolidated note, on the automated coupling threads (there are six distinct ones, each posted twice). I answered these inline in the earlier round; collecting the argument in one place since they all reduce to the same thing. Every flag on this PR targets test code, and all of it is in the two new files this PR adds:
These counts are the fixture pattern working, not coupling debt. The efferent counts on the individual tests are that same shared fixture plus the small output-parsing helpers ( No production symbol's coupling changed on this PR. Happy to restructure if the metrics gate treats this as blocking, but I'd be trading a deliberate design for a number. |
1 participant
Addresses the two failure modes of #2507 that remain after 0.9.35's relational-verb demotion. One commit on top of
9f25a3a; your demotion (_RELATIONAL_INTENT_TERMS, the inline terms-keyed fallback, thecaller/callershint entry, and the four #2507 tests) is byte-identical — this layers on it, it does not relitigate it.The interaction worth stating plainly
0.9.35 added
caller/callersto thecallentry of_CONTEXT_HINTS, so "callers of X" now infers acallcontext filter. A class node owns no call edges — calls attach to its methods, and the class→member edge carriescontext=None— so the filtered traversal cannot leave a class-node seed. Measured on a 48.5k-node Laravel corpus where the queried service has exactly 3 grep-verified production callers: "callers of ChargeCustomerService" went from 147 nodes / all 3 callers present-but-buried (0.9.34) to 1 node / 0 of 3, delivered confidently (0.9.35). The existing_make_callers_graphfixture can't observe this because it hangscontext="call"edges directly off the class node. The hint entry itself is good and is untouched here — the class-node stranding underneath it is what this PR repairs.The two changes (both default-inert, opted into only by
_query_graph_text)_pick_seedsparameter. In the guarantee loop, a term that is a substring of an already-picked seed's normalized label (the scorer's own weakest match tier, judged on the label alone — a labelless alias-only node covers nothing) claims no additional seed: it is not starved, and starvation is the only thing the guarantee exists to prevent. Refined invariant: every term with any match is matched by at least one seed. Runs after the existing label-dedup gate, same sorted-term order. Without it, "what code uses ChargeCustomerService to charge a customer" seeds theCustomerhub and.charge()on top ofChargeCustomerServiceand explodes: measured 2,286 → 138 nodes.Context: call (heuristic; relaxed — no matches beyond seeds). Zero-expansion threshold (no tuning constant, and pinned by a mutation-checked boundary test: exactly one node of expansion is NOT relaxed), identical in BFS/DFS, explicit filters always honored.Measured, 9f25a3a → this branch (same corpus, CLI defaults: depth 2, 2,000-token budget)
call (heuristic; relaxed — no matches beyond seeds)explainon a listener question is regression-free at an identical token cost — it shares none of the changed code (_find_nodeonly).Tests
tests/seeding_fixtures.py(new): a fixture wired the way extraction really wires things — calls attach to methods, the class→method edge carriescontext=None, one caller reachable only through areferencesedge, a verb-prefix decoy with a busy test-method neighborhood, a generic-noun hub, a doc-file prefix decoy. This is what makes the stranding observable in a test at all.tests/test_serve_seeding.py(new): 14 tests, 7 red at9f25a3a(both changes red-first), 7 green-by-design pins including regression pins for 0.9.35's demotion restated on extraction-shaped wiring. Both threshold/predicate mutants kill exactly their intended test.git diff --stat 9f25a3a..HEAD -- tests/is two new files only; your four query: relational-intent verbs ("calls"/"uses") survive stopwording, prefix-match unrelated identifiers, and the #1445 per-term guarantee seats the junk match as a seed #2507 tests and the scoring benchmark are untouched and green. Full suite 4,006 → 4,020 passed (the 4test_ollama_retry_cap.pyfailures are a local missing-openaienv issue, present at base). Bench equality gate holds.ruff checkclean.Disclosures and questions for the maintainer
usage(the onlyuse-family member not demoted), the listen family (listeners of <Event>is the same question shape on an event-driven corpus), andinvocation(already a_CONTEXT_FILTER_ALIASESkey). Also a structural question: since 0.9.35 hardcodes the demotion list while_CONTEXT_HINTSgrows independently (caller/callerswas exactly such a growth), do you want the set derived from the hint/alias tables so they can't drift apart?_RELATIONAL_INTENT_TERMS(there is no named demotion helper to call). It asserts nothing about the set's contents, and the realistic vocabulary extensions above leave it green (measured); the known latent coupling is that demoting a test-query's own domain nouns (e.g. adding both "charge" and "customer" — contrary to the set's documented verbs-only rule) would collapse the comparison to equality. Happy to restructure if you'd rather extract the inline demotion into a helper.coder.md, 10 of 138 traversed nodes above); your open work on test-path seed exclusion and bounded recovery would shrink it further.thanks @…line — it felt wrong to self-credit; add attribution as you see fit.Prior art: the same two changes (plus an equivalent demotion) shipped on the LawnStarter fork as 0.9.36 (lawnstarter#37, PR lawnstarter#44) with matching graded numbers on the same corpus.
🤖 Generated with Claude Code