h-dv/code-index

Watch

Fork 0

`change_impact` returns an empty, confident answer where `find_callers` finds 5 call sites — and nothing in its payload says why #122

New issue

Closed

opened 2026-09-04 17:01:44 +02:00 by buildagent · 1 comment

buildagent commented

2026-09-04 17:01:44 +02:00

Member

Dogfood finding from #51. Measured while hand-verifying benchmark answers, on real code.

The measurement

Same symbol, same index, same session:

find_callers(AsList)      → 5 test call sites, resolution: "name_fallback"
change_impact(<same id>)  → affected: [], test_count: 0, test_role_files: [],
                            consulted_files: 1, depth: 20

change_impact answered test_count: 0 at depth 20 for a symbol with five same-name call sites that find_callers returns.

The benchmark's "which tests cover W" question came back empty for 7 of 7 hand-verified files across two C# symbols.

The cause is legitimate; the silence is not

change_impact traverses resolved edges only. find_callers additionally reports a name-fallback channel. Both behaviours are defensible and deliberate — a reverse closure over unresolved same-name edges would fabricate impact, which is exactly what this project refuses.

The defect is that change_impact's payload contains nothing pointing at the five calls that exist. affected: [] with consulted_files: 1 reads as a measured, confident "nothing depends on this". An agent that has learned to trust our disclosure vocabulary — which is what we ask of it — will conclude the symbol has no test coverage and no blast radius.

search_symbols solves this exact problem with name_fallback_count, described as "an UPPER BOUND on how much ref_count undercounts". change_impact has no equivalent, so its zero cannot be distinguished from an earned one.

Why this is a high-value fix

change_impact is the "what breaks if I change Y" tool. An empty answer is the most consequential answer it can give, and it is currently the least qualified one. It is the same asymmetry as #99 — a fact the system knows, disclosed on one surface and silent on its neighbour — but on the tool whose empty result most directly invites a destructive decision.

safe_delete and check_rename already got the stronger treatment in #101: when evidence is incomplete they remove the absence verdict rather than annotating it. change_impact is in the same family and did not.

Shape of the fix

Reuse the existing vocabulary rather than inventing a second one. When the seeds have same-name unresolved refs that the resolved-edge traversal did not follow, say so in the payload, per the standing rule that disclosures ride with the data:

{"affected": [], "test_count": 0,
 "unfollowed_name_fallback": 5,
 "unfollowed_semantics": "5 same-name references to the seed did not resolve to it, so
   they were not traversed. This 0 is a floor over RESOLVED edges, not a measurement
   that nothing depends on this symbol. `find_callers` reports that channel."}

Absent when there are none, so a bare affected: [] keeps meaning "measured, and nothing depends on it".

Check the neighbours in the same pass rather than patching one tool: explain_dependency and repo_map seed from the same graph and may carry the same silence. #97 covered all tools by construction for exactly this reason, and #101's grader sits above the router for the same reason.

#99 (structural zero, disclosed on one surface), #101 (the downgrade contract for absence verdicts), #118 (a falsely earned zero from search_symbols). Found by #51's benchmark, which now grades this question class and will keep grading it.

Dogfood finding from #51. Measured while hand-verifying benchmark answers, on real code. ## The measurement Same symbol, same index, same session: ``` find_callers(AsList) → 5 test call sites, resolution: "name_fallback" change_impact(<same id>) → affected: [], test_count: 0, test_role_files: [], consulted_files: 1, depth: 20 ``` `change_impact` answered `test_count: 0` at depth 20 for a symbol with **five** same-name call sites that `find_callers` returns. The benchmark's *"which tests cover W"* question came back empty for **7 of 7** hand-verified files across two C# symbols. ## The cause is legitimate; the silence is not `change_impact` traverses **resolved edges only**. `find_callers` additionally reports a **name-fallback** channel. Both behaviours are defensible and deliberate — a reverse closure over unresolved same-name edges would fabricate impact, which is exactly what this project refuses. **The defect is that `change_impact`'s payload contains nothing pointing at the five calls that exist.** `affected: []` with `consulted_files: 1` reads as a measured, confident "nothing depends on this". An agent that has learned to trust our disclosure vocabulary — which is what we ask of it — will conclude the symbol has no test coverage and no blast radius. `search_symbols` solves this exact problem with `name_fallback_count`, described as *"an UPPER BOUND on how much `ref_count` undercounts"*. `change_impact` has no equivalent, so its zero cannot be distinguished from an earned one. ## Why this is a high-value fix `change_impact` is the "what breaks if I change Y" tool. An empty answer is the most consequential answer it can give, and it is currently the least qualified one. It is the same asymmetry as #99 — a fact the system knows, disclosed on one surface and silent on its neighbour — but on the tool whose empty result most directly invites a destructive decision. `safe_delete` and `check_rename` already got the stronger treatment in #101: when evidence is incomplete they **remove** the absence verdict rather than annotating it. `change_impact` is in the same family and did not. ## Shape of the fix Reuse the existing vocabulary rather than inventing a second one. When the seeds have same-name unresolved refs that the resolved-edge traversal did not follow, say so **in the payload**, per the standing rule that disclosures ride with the data: ```json {"affected": [], "test_count": 0, "unfollowed_name_fallback": 5, "unfollowed_semantics": "5 same-name references to the seed did not resolve to it, so they were not traversed. This 0 is a floor over RESOLVED edges, not a measurement that nothing depends on this symbol. `find_callers` reports that channel."} ``` Absent when there are none, so a bare `affected: []` keeps meaning "measured, and nothing depends on it". **Check the neighbours in the same pass** rather than patching one tool: `explain_dependency` and `repo_map` seed from the same graph and may carry the same silence. #97 covered all tools by construction for exactly this reason, and #101's grader sits above the router for the same reason. ## Related #99 (structural zero, disclosed on one surface), #101 (the downgrade contract for absence verdicts), #118 (a falsely earned zero from `search_symbols`). Found by #51's benchmark, which now grades this question class and will keep grading it.

buildagent commented

2026-09-04 18:29:36 +02:00

Author

Member

Fixed — riding the existing grader, gated on a structural fact rather than a tool list.

Where it lives

It rides annotate_evidence_gaps, in the same get_symbol loop #99 already runs (probe_unmeasured_population → probe_seed_symbols), reading name_fallback_count off the same row search_symbols publishes it from. No second producer, no second placement — so read_resource gets it too, by construction, and a future tool 25 is covered without opting in.

The gate, and why the first version was wrong

The first attempt was conditional on name_fallback_count > 0 but rode on every id-taking tool. A parallel lane's benchmark caught it immediately: a near-constant +226…+235 tokens on every find_callers-shaped answer, four of four growing by the same amount — the signature of an unconditional block, and exactly what this issue must not become.

The gate is now one clause on a fact the server already publishes: the block ships only on a reply whose graph_semantics begins resolved_dependencies. Measured across every id-taking tool, that string cleanly separates:

asserts a closure — change_impact, explain_dependency, context_pack → gets the block;
asserts static evidence — find_callers, find_references, find_callees, safe_delete, check_rename → does not, because they already say in the same sentence that unresolved uses may be absent, and carry the channel row by row.

A tool list would have needed maintaining; a structural clause does not.

Measured A/B, old binary vs new

call	Δ
`find_callers` / `find_references` / `find_callees`	+0 B
`safe_delete` / `check_rename` / `get_symbol`	+0 B
`change_impact` / `explain_dependency` / `context_pack`, with a finding	+510 B (~128 tok)
any of them, seed with an earned zero	+0 B

Prose cut 950 → 359 bytes.

Disclosure, not downgrade — and the reason is specific

change_impact renders no verdict string; there is no reasons array to remove from. And safe_delete/check_rename already publish same_name_unresolved, so downgrading them here would double-count a channel they already report. That is the narrowing #101's lane had to make on evidence, applied up front rather than after a guard fired on Widget.

Two corrections to this issue's own assumptions

explain_dependency was ungraded, because seed_symbol_ids read only symbol_id/symbol_ids and it takes from/to. Now driven by the shared SYMBOL_ID_ARGS const that #121's registry pins, so the two halves cannot drift.
repo_map cannot be covered and does not need to be. seed_symbol_ids' doc claimed it takes focus_ids; it takes focus, a list of symbol names. The comment was simply wrong, and this issue repeated it.

Mutations — five, all run, all red

delete the name_fallback_count arm → change_impact + explain_dependency red on the reproducing payload, positive control stays green;
drop .filter(|c| *c > 0) → positive control red, primary green (the pair that proves the block is conditional);
drop the traversal gate → find_callers grows the block, red — the coordinator's regression, now a test;
remove from/to from SYMBOL_ID_ARGS → red in two files;
empty SYMBOL_LOCATOR_ARGS → read_code/get_dependencies red.

The fixture reproduces the measurement rather than asserting it: Telemetry::record (ref_count 0, name_fallback_count 2, find_callers → 2 sites all name_fallback) against Quiet::ping, whose zero is earned because a resolved ping ref exists elsewhere.

The residual, itemised and accepted

The benchmark is green on flask (2695 vs 2700, under) and ripgrep (3659 vs 3653, within ceiling). cs-dapper moved 3489 → 3746, and I have accepted and re-recorded it with the reasoning in tests/bench/ratchet.json's new _moves field:

The entire +257 is two questions, both which_tests — i.e. both change_impact: AsList 220→348, ResetTypeHandlers 293→427. Every who_calls question is unchanged.

Those two are exactly the questions the benchmark scores as MISSED — they return empty while 7 hand-verified test files exist — and the run's own footer says it: "change_impact has no such channel, so a question answered this way by find_callers comes back EMPTY from the impact tool." The disclosure fires on 2 of 33 corpus questions, and only where the tool would otherwise report a confident zero that invites a destructive decision.

~128 tokens is the price of not telling an agent that nothing depends on a symbol when five things do. The available trim (~35 tokens/question, by deleting the "traversing them would fabricate impact" sentence) was declined by the implementing lane as tuning honesty to a threshold, and that judgement is upheld. Re-verified after re-recording: 5 passed, 0 failed, including the_benchmark_never_writes_its_own_expectations.

Reproduced in this repo before anything changed

CodeIndexServer::record (server.rs:499) answered affected: [], test_count: 0, consulted_files: 1 while find_callers returned its call site at line 17985 marked name_fallback. This issue's C# measurement, reproduced in Rust, in our own tree.

## Fixed — riding the existing grader, gated on a structural fact rather than a tool list. ### Where it lives It rides **`annotate_evidence_gaps`**, in the same `get_symbol` loop #99 already runs (`probe_unmeasured_population` → `probe_seed_symbols`), reading `name_fallback_count` off **the same row `search_symbols` publishes it from**. No second producer, no second placement — so `read_resource` gets it too, by construction, and a future tool 25 is covered without opting in. ### The gate, and why the first version was wrong The first attempt was conditional on `name_fallback_count > 0` but rode on **every** id-taking tool. A parallel lane's benchmark caught it immediately: a near-constant **+226…+235 tokens** on every `find_callers`-shaped answer, four of four growing by the same amount — the signature of an unconditional block, and exactly what this issue must not become. The gate is now one clause on a fact the server already publishes: **the block ships only on a reply whose `graph_semantics` begins `resolved_dependencies`.** Measured across every id-taking tool, that string cleanly separates: - **asserts a closure** — `change_impact`, `explain_dependency`, `context_pack` → gets the block; - **asserts static evidence** — `find_callers`, `find_references`, `find_callees`, `safe_delete`, `check_rename` → does not, because they already say in the same sentence that unresolved uses may be absent, and carry the channel row by row. A tool list would have needed maintaining; a structural clause does not. ### Measured A/B, old binary vs new | call | Δ | |---|---| | `find_callers` / `find_references` / `find_callees` | **+0 B** | | `safe_delete` / `check_rename` / `get_symbol` | **+0 B** | | `change_impact` / `explain_dependency` / `context_pack`, **with a finding** | **+510 B (~128 tok)** | | any of them, seed with an **earned** zero | **+0 B** | Prose cut 950 → 359 bytes. ### Disclosure, not downgrade — and the reason is specific `change_impact` renders no verdict string; there is no `reasons` array to remove from. And `safe_delete`/`check_rename` already publish `same_name_unresolved`, so downgrading them here would double-count a channel they already report. That is the narrowing #101's lane had to make on evidence, applied up front rather than after a guard fired on `Widget`. ### Two corrections to this issue's own assumptions - **`explain_dependency` was ungraded**, because `seed_symbol_ids` read only `symbol_id`/`symbol_ids` and it takes `from`/`to`. Now driven by the shared `SYMBOL_ID_ARGS` const that #121's registry pins, so the two halves cannot drift. - **`repo_map` cannot be covered and does not need to be.** `seed_symbol_ids`' doc claimed it takes `focus_ids`; it takes **`focus`, a list of symbol names**. The comment was simply wrong, and this issue repeated it. ### Mutations — five, all run, all red 1. delete the `name_fallback_count` arm → `change_impact` + `explain_dependency` red on the reproducing payload, **positive control stays green**; 2. drop `.filter(|c| *c > 0)` → **positive control red**, primary green (the pair that proves the block is conditional); 3. drop the traversal gate → `find_callers` grows the block, red — the coordinator's regression, now a test; 4. remove `from`/`to` from `SYMBOL_ID_ARGS` → red in **two** files; 5. empty `SYMBOL_LOCATOR_ARGS` → `read_code`/`get_dependencies` red. The fixture **reproduces** the measurement rather than asserting it: `Telemetry::record` (`ref_count` 0, `name_fallback_count` 2, `find_callers` → 2 sites all `name_fallback`) against `Quiet::ping`, whose zero is *earned* because a resolved `ping` ref exists elsewhere. ### The residual, itemised and accepted The benchmark is green on flask (2695 vs 2700, under) and ripgrep (3659 vs 3653, within ceiling). cs-dapper moved **3489 → 3746**, and I have accepted and re-recorded it with the reasoning in `tests/bench/ratchet.json`'s new `_moves` field: **The entire +257 is two questions, both `which_tests` — i.e. both `change_impact`:** `AsList` 220→348, `ResetTypeHandlers` 293→427. Every `who_calls` question is unchanged. Those two are exactly the questions the benchmark scores as **MISSED** — they return empty while 7 hand-verified test files exist — and the run's own footer says it: *"`change_impact` has no such channel, so a question answered this way by `find_callers` comes back EMPTY from the impact tool."* The disclosure fires on **2 of 33** corpus questions, and only where the tool would otherwise report a confident zero that invites a destructive decision. ~128 tokens is the price of not telling an agent that nothing depends on a symbol when five things do. The available trim (~35 tokens/question, by deleting the "traversing them would fabricate impact" sentence) was **declined by the implementing lane as tuning honesty to a threshold**, and that judgement is upheld. Re-verified after re-recording: **5 passed, 0 failed**, including `the_benchmark_never_writes_its_own_expectations`. ### Reproduced in this repo before anything changed `CodeIndexServer::record` (`server.rs:499`) answered `affected: [], test_count: 0, consulted_files: 1` while `find_callers` returned its call site at line 17985 marked `name_fallback`. This issue's C# measurement, reproduced in Rust, in our own tree.

buildagent closed this issue

2026-09-04 18:29:39 +02:00

dhoyer referenced this issue from a commit

2026-09-04 19:37:24 +02:00

feat: the plugin architecture proves itself on a real language, and the gates that were watching it start working

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

h-dv/code-index#122

No description provided.

Rows
Columns

change_impact returns an empty, confident answer where find_callers finds 5 call sites — and nothing in its payload says why #122