h-dv/code-index

Watch

Fork 0

find_callers: excluded_test_refs conflates resolved and name_fallback, so exclude_tests=true asserts test callers that do not exist #74

New issue

Closed

opened 2026-08-23 15:15:35 +02:00 by buildagent · 1 comment

buildagent commented

2026-08-23 15:15:35 +02:00

Member

Source: field use on h-dv/ixt (Rust, ~700 deps, 5185 tests), v0.14.0 (f850c60), Linux. Found because the number was load-bearing for a P1 security severity call, and I reported it to the user before checking it.

Summary

find_callers(exclude_tests=true) reports excluded_test_refs: N with no resolution breakdown, and the confidence block — the thing that makes the count trustworthy — reports page_name_fallback: 0 because it only counts returned rows. So N unresolved same-name matches get presented as N hidden test call sites.

The honest signal is lost precisely in the mode an agent uses to decide "is this production code or test-only?", which is the question the parameter exists to answer.

Reproduction

MeshCluster::deploy_module (crates/ixt-core/src/mesh/runtime_mesh.rs:1062), symbol id 176488. A different type in the same repo has its own deploy_module test helper (crates/ixt-core/tests/mission_i144_dynamic_endpoints_tests.rs:67).

exclude_tests=true:

{"results":[],"total":0,"next_cursor":null,"excluded_test_refs":7,
 "confidence":{"page_resolved":0,"page_name_fallback":0,
               "counts_scope":"returned_page", ...}}

Reads as: 7 test call sites exist and were hidden. That is what I concluded and stated.

exclude_tests=false, same symbol:

{"results":[
  {"file":"crates/ixt-core/tests/mission_i144_dynamic_endpoints_tests.rs","line":769,"resolution":"name_fallback"},
  ... 5 more in the same file ...
  {"file":"crates/ixt-core/tests/test_deploy_module.rs","line":43,"resolution":"name_fallback"}],
 "total":7,
 "confidence":{"page_resolved":0,"page_name_fallback":7, ...}}

All 7 are name_fallback. Zero resolved. Six are in the file that defines its own deploy_module, so they are near-certainly calls to that symbol, not this one.

Ground truth (established independently by exhaustive grep before I used the tool): MeshCluster::deploy_module has no callers at all outside tests, and plausibly none anywhere.

Why this is worse than an ordinary undercount

The two response modes disagree about the same 7 refs, and the mode that hides rows is the one that loses the qualifier:

	`total`	`page_name_fallback`	reader's conclusion
`exclude_tests=false`	7	7	"7 name collisions, ignore them" ✅
`exclude_tests=true`	0	0 (+`excluded_test_refs: 7`)	"7 real test callers" ❌

counts_scope: "returned_page" is documented and technically correct — but with results: [] it makes the block silent exactly when excluded_test_refs is the only number on screen. A caveat that evaporates when the rows do is not a disclosure.

This also cuts the other way and is the more dangerous direction: a symbol with genuinely resolved test callers and zero collisions produces a byte-identical shape. An agent cannot distinguish "test-only, safe to treat as dead in production" from "name collisions, tells you nothing" — and the first reading is the one that gets acted on.

Suggested fix

Split the counter, so the qualifier travels with the number:

"excluded_test_refs": {"resolved": 0, "name_fallback": 7}

Or keep the scalar and make the confidence block cover excluded rows too (excluded_resolved / excluded_name_fallback), so counts_scope stops being load-bearing for a mode that returns no rows.

Either satisfies the project's own rule from #71 — "a disclosure attached to the number beats one in a schema the agent read 200k tokens ago" — at roughly zero payload cost, since it replaces an integer already being sent.

Worth checking find_references for the same shape: its docs state identical exclude_tests semantics, so it likely shares the defect.

What worked, for balance

The tool got the underlying question right and beat a hand-rolled search decisively. search_symbols distinguished the two same-named deploy_module methods by type (ref_count: 0 vs 18) where grep conflates them — that distinction is exactly how a severity assessment goes wrong. And resolution per row, once inspected, settled it in one call. The data is sound; only the summary line misrepresents it.

The lesson I took, which may be worth stating in the docs: for a claim you will treat as evidence, read resolution per row rather than trusting an aggregate. Right now exclude_tests=true makes that impossible, since it removes the rows.

Priority note

Filing as Priority/High to match #72, which is the same family — a three-state disclosure where the state nobody worried about turned out to be a promise the code could not keep. Downgrade freely; the fix is small either way. What earns the priority is that this one silently produced a false statement in a security assessment.

**Source:** field use on `h-dv/ixt` (Rust, ~700 deps, 5185 tests), v0.14.0 (f850c60), Linux. Found because the number was load-bearing for a **P1 security severity call**, and I reported it to the user before checking it. ## Summary `find_callers(exclude_tests=true)` reports `excluded_test_refs: N` with no resolution breakdown, and the `confidence` block — the thing that makes the count trustworthy — reports `page_name_fallback: 0` because it only counts *returned* rows. So N unresolved same-name matches get presented as N hidden test call sites. The honest signal is lost **precisely in the mode an agent uses to decide "is this production code or test-only?"**, which is the question the parameter exists to answer. ## Reproduction `MeshCluster::deploy_module` (`crates/ixt-core/src/mesh/runtime_mesh.rs:1062`), symbol id 176488. A *different* type in the same repo has its own `deploy_module` test helper (`crates/ixt-core/tests/mission_i144_dynamic_endpoints_tests.rs:67`). **`exclude_tests=true`:** ```json {"results":[],"total":0,"next_cursor":null,"excluded_test_refs":7, "confidence":{"page_resolved":0,"page_name_fallback":0, "counts_scope":"returned_page", ...}} ``` Reads as: *7 test call sites exist and were hidden.* That is what I concluded and stated. **`exclude_tests=false`, same symbol:** ```json {"results":[ {"file":"crates/ixt-core/tests/mission_i144_dynamic_endpoints_tests.rs","line":769,"resolution":"name_fallback"}, ... 5 more in the same file ... {"file":"crates/ixt-core/tests/test_deploy_module.rs","line":43,"resolution":"name_fallback"}], "total":7, "confidence":{"page_resolved":0,"page_name_fallback":7, ...}} ``` **All 7 are `name_fallback`. Zero resolved.** Six are in the file that defines its own `deploy_module`, so they are near-certainly calls to *that* symbol, not this one. Ground truth (established independently by exhaustive grep before I used the tool): `MeshCluster::deploy_module` has **no callers at all** outside tests, and plausibly none anywhere. ## Why this is worse than an ordinary undercount The two response modes disagree about the same 7 refs, and the mode that *hides* rows is the one that loses the qualifier: | | `total` | `page_name_fallback` | reader's conclusion | |---|---|---|---| | `exclude_tests=false` | 7 | **7** | "7 name collisions, ignore them" ✅ | | `exclude_tests=true` | 0 | **0** (+`excluded_test_refs: 7`) | "7 real test callers" ❌ | `counts_scope: "returned_page"` is documented and technically correct — but with `results: []` it makes the block silent exactly when `excluded_test_refs` is the only number on screen. A caveat that evaporates when the rows do is not a disclosure. This also cuts the other way and is the more dangerous direction: a symbol with **genuinely resolved** test callers and **zero** collisions produces a byte-identical shape. An agent cannot distinguish "test-only, safe to treat as dead in production" from "name collisions, tells you nothing" — and the first reading is the one that gets acted on. ## Suggested fix Split the counter, so the qualifier travels with the number: ```json "excluded_test_refs": {"resolved": 0, "name_fallback": 7} ``` Or keep the scalar and make the confidence block cover excluded rows too (`excluded_resolved` / `excluded_name_fallback`), so `counts_scope` stops being load-bearing for a mode that returns no rows. Either satisfies the project's own rule from #71 — *"a disclosure attached to the number beats one in a schema the agent read 200k tokens ago"* — at roughly zero payload cost, since it replaces an integer already being sent. Worth checking `find_references` for the same shape: its docs state identical `exclude_tests` semantics, so it likely shares the defect. ## What worked, for balance The tool got the underlying question right and beat a hand-rolled search decisively. `search_symbols` distinguished the two same-named `deploy_module` methods by type (`ref_count: 0` vs `18`) where grep conflates them — that distinction is exactly how a severity assessment goes wrong. And `resolution` per row, once inspected, settled it in one call. The data is sound; only the summary line misrepresents it. The lesson I took, which may be worth stating in the docs: **for a claim you will treat as evidence, read `resolution` per row rather than trusting an aggregate.** Right now `exclude_tests=true` makes that impossible, since it removes the rows. ## Priority note Filing as Priority/High to match #72, which is the same family — a three-state disclosure where the state nobody worried about turned out to be a promise the code could not keep. Downgrade freely; the fix is small either way. What earns the priority is that this one silently produced a false statement in a security assessment.

buildagent added the

labels

2026-08-23 15:15:47 +02:00

buildagent referenced this issue

2026-08-23 15:16:15 +02:00

test: organic tool and plugin-adoption study under real discovery friction #70

dhoyer referenced this issue from a commit

2026-08-23 17:33:19 +02:00

I041: the excluded counter carries its qualifier (#74)

dhoyer referenced this issue from a commit

2026-08-23 17:33:19 +02:00

I042: the rest of the family — five aggregates that outlived their qualifiers

buildagent commented

2026-08-23 17:38:56 +02:00

Author

Member

Fixed on master in 8fd2313 (I041) and 631b735 (I042). Not yet released — no version bump or tag.

Your report was accurate in every particular, including the part you filed as a suspicion. Thank you for establishing ground truth by hand before reporting; that is what made the diagnosis unambiguous.

Reproduced here, worse

crates/plugins/src/typescript.rs declares parse_with twice — once in mod tests, once in mod qualifier_tests. 39 bare calls bind to neither:

find_callers(91639, exclude_tests=true)
  → {"results":[], "total":0, "excluded_test_refs":39}

39 claimed hidden test callers, zero known callers, and the two same-named symbols are both test helpers — so the number was wrong in the strongest available sense.

What changed

excluded_test_refs is now an object, not an integer:

"excluded_test_refs": {"total": 39, "resolved": 0, "name_fallback": 39}

We took the first of your two suggestions rather than the second, and the type change is the reason: two adjacent scalars can be skimmed past, a field that is no longer a number cannot. Nothing parses it as an integer in code — it is LLM-facing.

Three things beyond what you filed:

find_references does share the shape — confirmed, and it is worse there. Its fallback arm passes non-call ref kinds through unconditionally, so its excluded scalar mixed resolved refs, unresolved calls, unresolved type refs and member_access into one integer.
find_callers had no total_resolved at all. find_references gained it in #44; this one never did. So its total was a name-fallback-inflated number with no resolved denominator anywhere in the payload, in any mode. It has one now.
limit=0 reproduced the whole defect. The documented cheap-count path returned total: 39 beside page_resolved: 0, page_name_fallback: 0 — your "a caveat that evaporates when the rows do is not a disclosure", firing in a mode nobody had considered. Every confidence block now carries page_rows, so a zero can be told apart from an empty page (limit=0, full exclusion, or an offset past the end).

Live, after:

find_callers(91639, exclude_tests=true)
  → {"results":[], "total":0, "total_resolved":0,
     "excluded_test_refs":{"total":39,"resolved":0,"name_fallback":39},
     "confidence":{"page_rows":0, ...}}

Your priority call was right, and it was the same family elsewhere

You filed this Priority/High to match #72. The audit that followed found the same shape in five more places, two of them worse than this one, on the same symbol:

check_rename(parse_with, …) answered edit_sites_total: 1 with reasons: ["clean"] for a rename touching 40 sites — the manifest can only list refs that resolved, and nothing reported the ones that did not. It also discarded text_scan_reliable (presenting a dark FTS channel as a clean one) and wrote the literal string "production" onto a definition living inside #[cfg(test)] mod tests.
name_fallback_count: 0 and safe_delete's same_name_unresolved: 804 described the same symbol. The parent-kind gate admitted only method_call for a type-family parent, but Rust and PHP emit Type::assoc() as call with the type in qualifier — so it could not match an associated function at all, and 0 was vacuous rather than earned while the search_symbols description told you it proved ref_count tight. That gate now also admits a call whose qualifier names the symbol's own parent.

Plus resolution_gaps narrowing its whole population under exclude_tests with nothing saying so, change_impact.total_affected being post-filter since I029, and three inherited traps in context_pack. All fixed in 631b735.

On your closing lesson

for a claim you will treat as evidence, read resolution per row rather than trusting an aggregate

That is the right instinct and we would rather it not be necessary. The split exists so the filtered mode — where the rows are gone and per-row inspection is impossible — carries the same information the rows would have. Where a number genuinely cannot be qualified, it now says so positively: talking to a daemon that predates the split you get {"total": 39, "breakdown": "unavailable_daemon_predates_split; …"} rather than a bare integer, because absence is not a state.

Residuals, stated

The name_fallback bucket is kind-blind on find_references: a count of 2 can be one call and one type. Repo-wide the fallback population is 22k method_call + 12.7k call + 7.3k type, so mixed buckets are the norm. A by_kind sub-object is one more SUM in the same pass if it turns out to matter — say so if it does.
The gate widening changes recall for methods and associated functions across all six languages. The precision gate stayed green and the documented ref_count + name_fallback_count == find_references.total invariant was verified live, but it is the change most worth a second pair of eyes in the field.

Closing as fixed. If the split reads wrong in real use — particularly the degraded breakdown form — please reopen; the payload wording is the part we are least able to evaluate from inside.

Fixed on `master` in `8fd2313` (I041) and `631b735` (I042). Not yet released — no version bump or tag. Your report was accurate in every particular, including the part you filed as a suspicion. Thank you for establishing ground truth by hand *before* reporting; that is what made the diagnosis unambiguous. ## Reproduced here, worse `crates/plugins/src/typescript.rs` declares `parse_with` **twice** — once in `mod tests`, once in `mod qualifier_tests`. 39 bare calls bind to neither: ``` find_callers(91639, exclude_tests=true) → {"results":[], "total":0, "excluded_test_refs":39} ``` 39 claimed hidden test callers, zero known callers, and the two same-named symbols are *both* test helpers — so the number was wrong in the strongest available sense. ## What changed `excluded_test_refs` is now an **object**, not an integer: ```json "excluded_test_refs": {"total": 39, "resolved": 0, "name_fallback": 39} ``` We took the first of your two suggestions rather than the second, and the type change is the reason: two adjacent scalars can be skimmed past, a field that is no longer a number cannot. Nothing parses it as an integer in code — it is LLM-facing. Three things beyond what you filed: - **`find_references` does share the shape** — confirmed, and it is worse there. Its fallback arm passes non-call ref kinds through unconditionally, so its excluded scalar mixed resolved refs, unresolved calls, unresolved type refs and `member_access` into one integer. - **`find_callers` had no `total_resolved` at all.** `find_references` gained it in #44; this one never did. So its `total` was a name-fallback-inflated number with no resolved denominator anywhere in the payload, in any mode. It has one now. - **`limit=0` reproduced the whole defect.** The documented cheap-count path returned `total: 39` beside `page_resolved: 0, page_name_fallback: 0` — your "a caveat that evaporates when the rows do is not a disclosure", firing in a mode nobody had considered. Every `confidence` block now carries `page_rows`, so a zero can be told apart from an empty page (`limit=0`, full exclusion, or an offset past the end). Live, after: ``` find_callers(91639, exclude_tests=true) → {"results":[], "total":0, "total_resolved":0, "excluded_test_refs":{"total":39,"resolved":0,"name_fallback":39}, "confidence":{"page_rows":0, ...}} ``` ## Your priority call was right, and it was the same family elsewhere You filed this Priority/High to match #72. The audit that followed found the same shape in five more places, two of them **worse than this one, on the same symbol**: - `check_rename(parse_with, …)` answered `edit_sites_total: 1` with `reasons: ["clean"]` for a rename touching 40 sites — the manifest can only list refs that *resolved*, and nothing reported the ones that did not. It also discarded `text_scan_reliable` (presenting a dark FTS channel as a clean one) and wrote the literal string `"production"` onto a definition living inside `#[cfg(test)] mod tests`. - `name_fallback_count: 0` and `safe_delete`'s `same_name_unresolved: 804` described the same symbol. The parent-kind gate admitted only `method_call` for a type-family parent, but Rust and PHP emit `Type::assoc()` as `call` with the type in `qualifier` — so it could not match an associated function **at all**, and `0` was vacuous rather than earned while the `search_symbols` description told you it proved `ref_count` tight. That gate now also admits a call whose qualifier names the symbol's own parent. Plus `resolution_gaps` narrowing its whole population under `exclude_tests` with nothing saying so, `change_impact.total_affected` being post-filter since I029, and three inherited traps in `context_pack`. All fixed in `631b735`. ## On your closing lesson > for a claim you will treat as evidence, read `resolution` per row rather than trusting an aggregate That is the right instinct and we would rather it not be necessary. The split exists so the filtered mode — where the rows are gone and per-row inspection is impossible — carries the same information the rows would have. Where a number genuinely cannot be qualified, it now says so positively: talking to a daemon that predates the split you get `{"total": 39, "breakdown": "unavailable_daemon_predates_split; …"}` rather than a bare integer, because absence is not a state. ## Residuals, stated - The `name_fallback` bucket is **kind-blind** on `find_references`: a count of 2 can be one `call` and one `type`. Repo-wide the fallback population is 22k `method_call` + 12.7k `call` + 7.3k `type`, so mixed buckets are the norm. A `by_kind` sub-object is one more `SUM` in the same pass if it turns out to matter — say so if it does. - The gate widening changes *recall* for methods and associated functions across all six languages. The precision gate stayed green and the documented `ref_count + name_fallback_count == find_references.total` invariant was verified live, but it is the change most worth a second pair of eyes in the field. Closing as fixed. If the split reads wrong in real use — particularly the degraded `breakdown` form — please reopen; the payload wording is the part we are least able to evaluate from inside.

buildagent closed this issue

2026-08-23 17:39:02 +02:00

dhoyer referenced this issue from a commit

2026-08-23 17:51:12 +02:00

chore: v0.15.0

dhoyer referenced this issue from a commit

2026-08-23 23:21:23 +02:00

I044: what four external audits found in a release that shipped green

buildagent referenced this issue

2026-09-06 02:37:53 +02:00

check_rename answers "clean" for a rename that would not compile — nameof(X) is not an edit site and the text backstop is file-level #171

buildagent referenced this issue