feat(mcp): relevance scoring overhaul for explore — kill incidental name-collision matches (CG-10, #1500)
Explore's per-file relevance awarded +50/+10/+3/+1 by match class and admitted
anything scoring >= 3. Neither half held up: the tier said HOW a symbol reached
us, never whether the match was evidence, and an absolute floor admits noise on
any repo where the top file scores 50+. Three scripts/agent-eval/*.mjs harnesses
took 63% of this repo's own "how does explore allocate its output budget" answer
on nothing but an unused `const explore` and a `const BUDGET`.
Four levers:
- KIND WEIGHT (RELEVANCE_KIND_WEIGHT): callables and types 1.0, members ~0.5,
variable/constant/parameter 0.15-0.35. A weak-kind symbol with no usage edge
anywhere in the graph (`contains` excluded — nesting is not usage) drops to
0.08. Only weak kinds in the top two tiers pay for the DB probe; the subgraph's
own edges answer most cases free. No measurable latency change (210 vs 211
ms/call, n=12 interleaved).
- PERIPHERAL CAP: nodes >=2 hops from any match accumulate into a bucket capped
at 5. Uncapped they added a flat +1 each, so a file grew more relevant by being
bigger — parse-session.mjs reached 22 off one constant plus twelve unrelated
symbols.
- RANK PENALTY: generated files x0.3, low-value x0.5, applied to the score AND
the graph mass. Score alone would not have fixed #1500 — the generated CRUD
carries MORE graph mass than the hand-written use-case, and graph mass outranks
score in the comparator. Self-normalizing, never a hard exclusion.
- RELATIVE FLOOR: clamp(topScore * 0.2, 1, 10). Capped at one full-strength
direct match so concentration elsewhere can never exclude one (without it a
named-seed-heavy file pushed the floor to 21 and dropped a file the agent had
named by class name). Backfills to 3 candidates when it would leave fewer, and
drops the evidence requirement rather than return nothing at all.
excludeLowValueFiles was dead config — declared per tier, read nowhere; the
test/spec exclusion has been unconditional for a while. Removed. The real gap was
the detector: `isLowValue` anchored on a leading `/`, so a repo-ROOT `test/` dir
(express, cobra, most of npm and Go) never matched — express's routing question
spent 59% of its envelope on three test files. Anchored at `^` too, and the
filter now runs before the floor and judges "are there other candidates?" on the
whole gather.
Measured before/after on the same indexes (baseline bd86ad2):
- payroll-go fixture: generated 57.4% -> 23.5%; answer 25.6% -> 61.5%; cycle.go
delivered 0 -> 38.9%. Generated ranks #3/#4, was #1/#2.
- self-query fixture: eval scripts 72% -> 0%; tools.ts ranks #1.
- express "route a request": 59% to test/* -> lib/application.js + lib/response.js
- cobra x3, codegraph "indexing pipeline": byte-identical (control)
Diagnostic gains a per-file penalty multiplier and NodeKind mix, so "why did this
file score X" is legible. Selection stages reordered to match the pipeline.
CG-6's gates flip from it.fails to live regressions except the byte-split ones,
which stay open for CG-12 (allocation still follows file size within the ranked
set).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
bd86ad2061
commit
a3898cdc70
@@ -90,12 +90,112 @@ being the truncation notice at the end.
|
||||
|
||||
This is the gap the rest of the epic closes: relevance-proportional allocation with a
|
||||
relative cliff (CG-12), on top of scoring that stops rewarding incidental name collisions
|
||||
(CG-10).
|
||||
(CG-10 — landed; see below).
|
||||
|
||||
## CG-10 — relevance scoring
|
||||
|
||||
CG-10 changes **what gets into the response**, ahead of how bytes are split among what's
|
||||
in. Four levers, all multiplicative so they compose without ordering surprises.
|
||||
|
||||
### 1. Kind weighting
|
||||
|
||||
The tier a symbol reached us by (named seed `+50`, query match `+10`, adjacent to one `+3`,
|
||||
peripheral `+1`) says *how it got here*; `RELEVANCE_KIND_WEIGHT` says *whether the match is
|
||||
evidence*. Callables and types weigh 1.0, members ~0.5, and `constant`/`variable`/
|
||||
`parameter` 0.15–0.35 — a local named `explore` is a name collision until something
|
||||
corroborates it.
|
||||
|
||||
**Isolation.** For a weak-kind symbol in the top two tiers, "is anything using it?" is the
|
||||
corroboration: no usage edge anywhere in the graph (`contains` excluded — lexical nesting
|
||||
is not usage) drops it to 0.08. Cost is bounded — only weak kinds in the tiers whose weight
|
||||
can carry a file pay for the probe, and the subgraph's own edges answer most cases for
|
||||
free. Measured: no latency change (210 vs 211 ms/call, n=12 interleaved).
|
||||
|
||||
**Peripheral cap.** Nodes ≥2 hops from any match now accumulate into a separate bucket
|
||||
capped at 5. Uncapped, every such node added a flat `+1`, so a file grew more relevant by
|
||||
being bigger — `parse-session.mjs` reached score 22 off one incidental constant plus twelve
|
||||
unrelated symbols. Size is not evidence.
|
||||
|
||||
### 2. Relative score floor
|
||||
|
||||
`score >= 3` admits noise on any repo where the top file scores 50+. The floor is now
|
||||
`clamp(topScore × 0.2, 1, 10)`:
|
||||
|
||||
- **relative** — on a diffuse question no file dominates, every candidate sits near the top,
|
||||
and the whole spread survives; on a precise one it cuts the tail.
|
||||
- **capped at 10** — one direct query match on a callable. A single full-strength match is
|
||||
never incidental, so no amount of concentration elsewhere may exclude it. Without this
|
||||
cap, one named-seed-heavy file pushed the floor to 21 and dropped a file the agent had
|
||||
named by *class* name (classes enter at `+10`, not `+50` — named seeds are callables).
|
||||
- **backfill** — if fewer than 3 files survive, the best of what the floor cut comes back,
|
||||
but only from files with real evidence (≥ the absolute floor). If *nothing* survives, the
|
||||
backfill drops that requirement: returning "no relevant code found" when the gather did
|
||||
find candidates sends the agent straight back to grep.
|
||||
|
||||
### 3. Generated status in the score, not the tiebreak
|
||||
|
||||
`rankPenalty(file)` multiplies both the relevance score and the graph mass by 0.3 for
|
||||
generated files (0.5 for low-value ones). Applying it to the score alone would not have
|
||||
fixed #1500: the generated CRUD carries **more** graph mass than the hand-written use-case,
|
||||
and graph mass outranks score in the comparator. The penalty is self-normalizing — in an
|
||||
all-generated repo everything scales together and relative ranking is untouched — and it
|
||||
never hard-excludes: ask about the generated API by name and the named-seed tier still puts
|
||||
it first.
|
||||
|
||||
### 4. `excludeLowValueFiles` — the finding
|
||||
|
||||
The per-tier flag the task asked to reconsider was **dead config**: declared on
|
||||
`ExploreOutputBudget` and set per tier, but read nowhere. A later change had already made
|
||||
the test/spec/icon/i18n exclusion unconditional at all tiers. The flag is removed.
|
||||
|
||||
The substantive gap was in the *detector*, not the gating: `isLowValue` matched
|
||||
`/\/(tests?|__tests?__|spec)\//`, anchored on a leading slash, so a **repo-root** `test/`
|
||||
directory — express, cobra, and most of npm and Go — never matched. Express's "how does
|
||||
express route a request to a handler?" spent 59% of its envelope on three test files while
|
||||
`lib/application.js` was clipped. Anchored at `^` as well, that query now returns
|
||||
`lib/application.js` + `lib/response.js` and no tests.
|
||||
|
||||
Two related changes: the filter now runs **before** the score floor and judges "are there
|
||||
other candidates?" on the whole gather rather than the post-floor set (judging it after was
|
||||
how the floor's keep-minimum pulled test files back in as the "spread"); and low-value
|
||||
files that survive the filter's `≥2 non-test candidates` escape hatch are down-weighted via
|
||||
`rankPenalty` rather than left at full strength.
|
||||
|
||||
### Measured effect
|
||||
|
||||
Before/after on the same indexes, deterministic (`CODEGRAPH_EXPLORE_DEBUG` diagnostic, both
|
||||
arms same build system, baseline = `bd86ad2`):
|
||||
|
||||
| repo · query | before | after |
|
||||
|---|---|---|
|
||||
| this repo · self-query fixture | 72% to eval scripts, `tools.ts` 18.5% | scripts **0%**, `tools.ts` #1 |
|
||||
| this repo · `handleExplore buildFlowFromNamedSymbols …` | 82% to eval scripts | `tools.ts` 48% + `index.ts` 32% |
|
||||
| this repo · "how is error handling done" | 58% to eval scripts, `tools.ts` delivered 0 | transport/tools/cobol/api |
|
||||
| this repo · "what languages does codegraph support" | 63% to `scripts/add-lang/*` | grammars/index/cli |
|
||||
| this repo · "main components of the indexing pipeline" | — | **byte-identical** |
|
||||
| payroll-go fixture | generated 57.4%, answer 25.6% | answer **61.5%**, generated **23.5%** |
|
||||
| express · route a request | 59% to `test/*` | `application.js` + `response.js` |
|
||||
| cobra · 3 queries | — | **byte-identical** |
|
||||
|
||||
The two byte-identical rows are the control: where the answer was already concentrated, the
|
||||
new floor prunes the same tail earlier and cheaper and arrives at the same response.
|
||||
|
||||
**Known thin case.** Express's "how does the app object get created and what does it
|
||||
expose" drops from 4 files (top one an `examples/` file at 38%) to `lib/express.js` alone,
|
||||
2.6 KB against a 13 KB budget. `lib/application.js` matched on nothing but an unused
|
||||
file-scope `var app` — indistinguishable, at the symbol level, from the eval scripts' unused
|
||||
`const explore`; express models its API surface as properties assigned to that object,
|
||||
which the graph has no edges for. That is extraction coverage, not ranking. Backfilling it
|
||||
was tried and rejected: node-count ties handed the slot to `examples/route-middleware`
|
||||
instead, at 48% of the envelope. Thin-and-precise beats padded-with-noise — a wrong file
|
||||
does not save the agent the follow-up call it would pad against.
|
||||
|
||||
## The regression fixtures (CG-6)
|
||||
|
||||
Two fixtures pin the failure mode so it can never silently return. Both **fail today** —
|
||||
that is what they are for. They become the pass gate for CG-10 + CG-12.
|
||||
Two fixtures pin the failure mode so it can never silently return. They were written to
|
||||
**fail** — that is what they were for. CG-10 closed the ranking half of both; the byte-split
|
||||
assertions still fail and are the pass gate for CG-12. The numbers quoted below are the
|
||||
**pre-CG-10 baseline**; see "Measured effect" above for where they stand now.
|
||||
|
||||
They are declared in `scripts/agent-eval/allocation-fixtures.json` and run by
|
||||
`scripts/agent-eval/probe-allocation.mjs`, which drives the CG-4 diagnostic through a JSONL
|
||||
@@ -138,15 +238,22 @@ never reach the agent, and every byte that did arrive describes either CRUD or t
|
||||
|
||||
This fixture is hermetic: the probe copies the tree to a temp dir and re-indexes per run,
|
||||
so two runs on one build are byte-identical (verified). `__tests__/explore-allocation-1500.test.ts`
|
||||
runs the same assertions in vitest — the fixture-shape half green, the allocation half as
|
||||
`it.fails` so the suite stays green while the bug is open and goes **red when it is fixed**.
|
||||
runs the same assertions in vitest.
|
||||
|
||||
**After CG-10** the generated files rank #3/#4 instead of #1/#2, `cycle.go` delivers 38.9%
|
||||
(it delivered nothing), and `runPayrollCycleAll` + the real `s.store.Upsert(ctx, slip)`
|
||||
reach the agent. Those assertions are now live regressions. What remains `it.fails` is
|
||||
`payslip_builder.go`: it ranks #6, the tier's `maxFiles` is 4, and the render loop still
|
||||
spends by file size — CG-12's job.
|
||||
|
||||
**Finding, deliberately left unfixed:** `runPayrollCycleAll` calls `s.store.Upsert` on a
|
||||
`*payslipstore.Store`, but the graph resolves that edge to the **generated**
|
||||
`internal/gen/fkit/payroll/store.go` `Store.Upsert`. Same-name method resolution across two
|
||||
packages that both define `Store.Upsert` picks the wrong receiver. It is upstream of the
|
||||
allocation bug — a wrong edge pulls the generated store into the subgraph and inflates its
|
||||
score — so it belongs with CG-10's scoring work, not with the fixture.
|
||||
packages that both define `Store.Upsert` picks the wrong receiver. It is upstream of
|
||||
allocation — a wrong edge pulls the generated store into the subgraph. CG-10 mitigates the
|
||||
*symptom* (the generated store is penalized on both score and graph mass, so it no longer
|
||||
displaces the real one) without fixing the resolution bug itself, which belongs with the
|
||||
same-name method resolution work (see `samename-method-resolution-1079`).
|
||||
|
||||
### 2. `self-query` — the same bug with no generated code in sight
|
||||
|
||||
|
||||
Reference in New Issue
Block a user