Skip to content

Commit 7d209a6

Browse files
Your Nameclaude
andcommitted
fix(indexer,server): B7-v2 Express+Zod root-cause fixes, canonical PolicyDecision gate
Three fixes from this session's audit of the product-uplift roadmap, each root-caused, fixed, and verified with a full workspace test run + real B7 benchmark re-run: 1. Express (`rename_express_set_charset`): `build_resolution_context`'s `path_lang` map was populated only from the `symbols` table, so a file with zero top-level named declarations (any Mocha/Jest-style `describe`/`it` test file) never got a `path_lang` entry, silently zeroing every outgoing call edge from that file regardless of confidence. Fixed by seeding `path_lang` from `file_index` instead, which records every indexed file regardless of symbol count. 2. Zod (`rename_zod_prettify_error`): `import_node_types` never walked `export_statement` for JS/TS, so `export { x } from 'y'` re-exports never produced an `import_edges` row. Zod's real re-export is a 2-hop wildcard barrel chain, so `reference_impact`'s import-edge lookup is now a bounded BFS (`REFERENCE_IMPACT_MAX_REEXPORT_HOPS`) instead of a single-hop query. New `parse_js_export_from` reuses the existing `symbols_used = '[]'` wildcard convention from `parse_rust_import`. 3. Canonical PolicyDecision (roadmap item 3): `compute_touch_risk` — the function feeding both real write gates (`edit_lines_impl_gated`'s block decision and `edit_context`'s `gate_prediction`) — never considered `touches_manifest`/`touches_uncovered_code`, two axes `policy::evaluate()` already escalates to `Policy::default()`'s maximally-conservative `high` floor. Those axes were only ever computed on the CCK-10 authority-digest branch, never on the plain confirm/reason path every edit_lines/edit_symbol call takes by default. Fixed by folding both floors into compute_touch_risk's own escalation chain (reads the project's real policy.toml config, not hardcoded), so a manifest edit or an edit to uncovered code is finally gated the same way on both paths. Also corrected a stale policy/mod.rs doc comment claiming nothing reads the policy module yet. B7 benchmark now passes 6/6 via the calm_v2 arm with zero regressions. Full details, root-cause narratives, and verification chains in docs/plans/2026-08-20-product-uplift-and-b7v2-roadmap.md (§8-§14). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 13bc2c6 commit 7d209a6

12 files changed

Lines changed: 1601 additions & 63 deletions

File tree

benchmarks/b7_task_correctness/README.md

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,125 @@ which is not committed and safe to delete between runs.
298298

299299
## Next steps
300300

301+
## v2 (2026-08-20): a third `calm_v2` arm, and why it did NOT close the gap
302+
303+
## Update (same day): Express's real bug found and FIXED — `path_lang` gap in `context.rs`
304+
305+
## Update (same day): Zod's real bug found and FIXED too — transitive re-export walk
306+
307+
Per an explicit follow-up ask, the zod gap (§7.1/§6.1's "materially bigger feature") was fixed in
308+
the same session. Two changes: (1) `crates/calm-core/src/indexer/imports.rs` now walks
309+
`export_statement` for JS/TS and parses `export {...} from '...'` / `export * from '...'` into
310+
`import_edges` (previously zero export syntax was ever indexed, for any JS/TS project); (2)
311+
`reference_impact`'s import-edge lookup (`crates/calm-server/src/tools/trace.rs`) is now a bounded
312+
BFS through wildcard re-export chains instead of a single-hop query, closing zod's actual two-hop
313+
barrel-file case. Full detail, including a real fallback-chain bug caught and fixed before it
314+
shipped: `docs/plans/2026-08-20-product-uplift-and-b7v2-roadmap.md` §10.
315+
316+
`cargo test --workspace --release`: 0 failed. Re-ran all 6 B7 tasks end-to-end:
317+
318+
| task | calm_v2, before | calm_v2, after |
319+
|---|---|---|
320+
| rename_zod_prettify_error | 0.5/False | **1.0/True** |
321+
| the other 5 tasks (incl. express) | 1.0/True | 1.0/True (unchanged) |
322+
323+
**B7 now passes 6/6 via the `calm_v2` arm.** `calm` v1 stays at 0.5/False for zod by design (v1
324+
never calls `reference_impact`, the tool this fix lives in). Not covered, documented as a known
325+
scoped-out gap: `export * as ns from` namespace re-exports, and an `as`-aliased re-export changing
326+
the name partway through a chain.
327+
328+
The deeper audit above (triggered by a user follow-up: "is there truly no way to fix this?") found
329+
that the actual root cause was neither `reference_impact`'s coverage nor the parser — it was
330+
`build_resolution_context`'s `path_lang` map being derived only from the `symbols` table, so a
331+
file with zero top-level named declarations anywhere (any Mocha/Jest/Vitest-style `describe`/`it`/
332+
`test` file, including the real `test/utils.js`) never got a language entry, causing
333+
`resolve_sites_to_edges`'s same-language safety filter to empty out every outgoing call that file
334+
made — regardless of confidence tier. Full root-cause trace: `docs/plans/2026-08-20-product-uplift-
335+
and-b7v2-roadmap.md` §8.
336+
337+
**Fixed** in `crates/calm-core/src/indexer/pipeline/context.rs` (`path_lang` now seeded from
338+
`file_index`, which already tracks every indexed file's language regardless of symbol count) —
339+
same doc's §9 has the full verification chain. Re-ran all 6 B7 tasks end-to-end against the fixed
340+
binary:
341+
342+
| task | calm / calm_v2, before | calm / calm_v2, after |
343+
|---|---|---|
344+
| rename_express_set_charset | 0.667/False · 0.667/False | **1.0/True · 1.0/True** |
345+
| rename_zod_prettify_error | 0.5/False · 0.5/False | 0.5/False · 0.5/False (unchanged — separate bug, §7.1) |
346+
| the other 4 tasks | 1.0/True | 1.0/True (unchanged, no regression) |
347+
348+
`cargo test --workspace --release`: 0 failed. New permanent regression test:
349+
`test_call_from_a_file_with_no_named_symbols_still_gets_a_call_edge` in
350+
`crates/calm-core/src/indexer/pipeline.rs`. Fix is currently uncommitted, pending the user's
351+
go-ahead to commit/push.
352+
353+
Full rationale and audit trail: `docs/plans/2026-08-20-product-uplift-and-b7v2-roadmap.md`.
354+
355+
A third arm, `calm_v2`, was added (`run_calm_arm_v2` in `run_benchmark.py`, alongside `naive`/
356+
`calm``calm` v1 kept unchanged for comparison). It supplements `edit_context`'s `callers()`
357+
with `reference_impact`'s `must_change`/`likely_change` hits, on the hypothesis (from
358+
`reference_impact`'s own source comment, `crates/calm-server/src/tools/trace.rs:780-784`, which
359+
names both tasks below) that this tool — built specifically to close this benchmark's own
360+
documented gap — would fix Express and Zod.
361+
362+
**Result: it did not.** Both tasks scored identically to v1:
363+
364+
| task | v1 (`calm`) recall/build | v2 (`calm_v2`) recall/build |
365+
|---|---|---|
366+
| rename_express_set_charset | 0.667 / False | 0.667 / False |
367+
| rename_zod_prettify_error | 0.5 / False | 0.5 / False |
368+
369+
The other four tasks (fd, flask, gin, spring-petclinic) stayed at 1.0/True in `calm_v2` too — no
370+
regression from the wider `reference_impact` surface.
371+
372+
**Root-caused, not assumed:**
373+
374+
- **Zod**: `reference_impact` returned `must_change_count: 0` for `prettifyError`. Both missing
375+
files re-export it via `export { …, prettifyError, … } from "../core/index.js";`. Traced to
376+
`crates/calm-core/src/indexer/imports.rs::import_node_types`, which for `"javascript" |
377+
"typescript"` only walks `["import_statement", "variable_declarator"]` — **`export_statement` is
378+
never walked at all**, so no `import_edges` row is ever created for a re-export, for any JS/TS
379+
project. This is a real, previously-undocumented gap in the import extraction, not something
380+
`reference_impact`'s existing import-edge tier can see. (The two real `z.prettifyError(...)` call
381+
sites did get a call_edge, but at `"ambiguous"` confidence, correctly bucketed as `review`
382+
`edit_context.callers()` already covered those, so this was never where the miss came from.)
383+
- **Express**: `reference_impact` produced 0 `review` hits and 7 `textual_only` hits for
384+
`setCharset``test/utils.js`'s `utils.setCharset(...)` (property access on a bare
385+
`require('../lib/utils')`) still produces no call_edge at all. Confirms this is exactly the
386+
pre-existing `parser.rs` call-site-extraction gap already named in this section's own "Next
387+
steps" above — orthogonal to what an import/export-edge fix could address.
388+
389+
**Correction to `reference_impact`'s own source comment**: its claim to catch "the exact gap
390+
behind" both tasks holds for a plain-`import`-side reference, but not for Zod's actual `export {
391+
X } from 'y'` re-export shape, and not for Express's case at all (a different bug class — call-site
392+
extraction, not import/export tracking). Worth narrowing that comment once the `export_statement`
393+
gap below is fixed.
394+
395+
**Follow-up, not done in this pass — revised after a deeper audit** (full detail:
396+
`docs/plans/2026-08-20-product-uplift-and-b7v2-roadmap.md` §6-§7): walking `export_statement` in
397+
`import_node_types`/`extract_imports_from_tree` is real and necessary, but **not sufficient on its
398+
own** for Zod — its actual re-export is a two-hop barrel chain (`external.ts` names `prettifyError`
399+
re-exporting from `core/index.ts`, which itself re-exports `errors.ts` via a *wildcard*
400+
`export * from './errors.js'` naming no symbols at all). A single-hop `import_edges` fix would
401+
point `external.ts` at `core/index.ts`, not at `errors.ts` where the symbol is actually defined —
402+
`reference_impact`'s direct `to_path` lookup still wouldn't find it. Closing this needs transitive
403+
import-edge resolution through wildcard re-export chains, not a single-function patch — re-run
404+
`rename_zod_prettify_error` after landing whichever shape of fix to confirm empirically, not
405+
assume (this file's own history is a live example of why: the original `reference_impact` source
406+
comment claimed to close this gap and, empirically re-run here, did not). Express's gap needs a
407+
separate `parser.rs` call-site-extraction fix (property-access call through a required module's
408+
bare identifier), already scoped above — unaffected by anything import/export-related.
409+
410+
Also independently re-verified this session (adversarial self-audit, not just re-reading the
411+
numbers): both failures reproduce identically outside the benchmark harness (`npm test` /
412+
`pnpm test` run directly against the renamed corpus copies — same `TypeError`/`TypeCheckError` as
413+
above); both symbols are collision-free and false-positive-free in the pinned corpora (manual grep
414+
matches `oracle_callsite_files` exactly for both); and naive's "1.0 recall" on every task is close
415+
to tautological by construction (`run_naive_arm`'s file-selection regex and `oracle.py`'s
416+
ground-truth regex are near-identical) — the informative, independent signal is `build_pass` on
417+
both arms, which is real compiler/test ground truth and was the first thing re-verified. See the
418+
roadmap doc's §7 for the full adversarial audit trail.
419+
301420
1. Investigate the express `setCharset` call-graph gap directly in
302421
`parser.rs`'s JS/TS call-site extraction (property-access call through a
303422
required module's bare identifier vs. a destructured bare-name call to

benchmarks/b7_task_correctness/run_benchmark.py

Lines changed: 83 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,74 @@ def run_calm_arm(corpus: Path, task: dict, real_repo_root: Path, lang: str) -> d
183183
finally:
184184
client.close()
185185

186+
def run_calm_arm_v2(corpus: Path, task: dict, real_repo_root: Path, lang: str) -> dict:
187+
"""CALM-scripted, v2 -- docs/plans/2026-08-20-product-uplift-and-b7v2-roadmap.md
188+
Phase 0 item 1. Same as run_calm_arm above, but supplements edit_context's
189+
`callers()` with `reference_impact`'s must_change/likely_change hits.
190+
`reference_impact` was purpose-built (landed 2026-08-04, commit 60ff9c9 --
191+
AFTER this file's Phase-2 results were last recorded, 2026-07-30) to close
192+
exactly the two gaps this benchmark's own README documents in its Phase 2
193+
section: a bare re-export/import edge that never becomes a call edge
194+
(zod's `prettifyError`), and (empirically checked here, not assumed) a
195+
property-access call through a required module's bare identifier
196+
(express's `setCharset`) -- see that function's own doc comment in
197+
crates/calm-server/src/tools/trace.rs, which names both tasks by id.
198+
199+
`review`/`textual_only` hits are counted but deliberately NOT
200+
auto-renamed -- same caution AGENTS.md's documented workflow already
201+
recommends (a textual-only match can be an unrelated same-named symbol;
202+
see this README's own `slugify` 3-way-collision finding). A real agent
203+
would surface those counts and decide, not silently skip or silently
204+
rename them; this arm reports them in the result row instead of hiding
205+
the number, matching this repo's stated benchmark policy of not hiding
206+
an inconvenient measurement."""
207+
client = MCPClient(project_root=str(corpus), repo_root=str(real_repo_root))
208+
tool_calls = 0
209+
try:
210+
client.wait_until_indexed()
211+
provider = _SCIP_PROVIDER_BY_LANG.get(lang, lang)
212+
try:
213+
client.call_tool("scip_refresh", {"lang": provider})
214+
except Exception: # noqa: BLE001 -- best-effort, matches B12's posture
215+
pass
216+
raw = client.call_tool("edit_context", {"symbol": task["symbol"]})
217+
tool_calls += 1
218+
ctx = json.loads(raw)
219+
touch_files = set()
220+
for c in ctx.get("callers", []):
221+
sym = c.get("symbol", "")
222+
if "::" in sym:
223+
touch_files.add(sym.split("::", 1)[0])
224+
touch_files.add(task["def_path"])
225+
226+
ri_raw = client.call_tool("reference_impact", {"symbol": task["symbol"]})
227+
tool_calls += 1
228+
ri = json.loads(ri_raw)
229+
review_hits = 0
230+
textual_only_hits = 0
231+
for hit in ri.get("references", []):
232+
cls = hit.get("classification")
233+
if cls in ("must_change", "likely_change"):
234+
touch_files.add(hit["path"])
235+
elif cls == "review":
236+
review_hits += 1
237+
elif cls == "textual_only":
238+
textual_only_hits += 1
239+
240+
edited = [f for f in sorted(touch_files) if apply_rename_at(corpus, f, task["symbol"], task["new_name"])]
241+
tool_calls += len(touch_files)
242+
client.call_tool("diff_impact", {}) # mandatory post-edit verification, AGENTS.md Stage 7
243+
tool_calls += 1
244+
return {
245+
"arm": "calm_v2", "files_touched": sorted(edited),
246+
"calm_reported_touch_files": sorted(touch_files), "tool_calls": tool_calls,
247+
"reference_impact_review_count": review_hits,
248+
"reference_impact_textual_only_count": textual_only_hits,
249+
}
250+
finally:
251+
client.close()
252+
253+
186254

187255
def score_arm(oracle_files: set[str], result: dict) -> dict:
188256
touched = set(result["files_touched"])
@@ -245,10 +313,19 @@ def main() -> int:
245313
calm_result["output_tail"] = calm_bt.output[-1500:]
246314
row["calm"] = score_arm(oracle_files, calm_result)
247315

316+
print(f"[b7] calm_v2 arm (edit_context + reference_impact) ...", file=sys.stderr)
317+
calm_v2_corpus = fresh_clone(lang, "calm-v2")
318+
calm_v2_result = run_calm_arm_v2(calm_v2_corpus, task, real_repo_root, lang)
319+
calm_v2_bt = build_test_gate(calm_v2_corpus, task.get("build_cmd"), task["test_cmd"])
320+
calm_v2_result["build_pass"] = calm_v2_bt.passed
321+
calm_v2_result["output_tail"] = calm_v2_bt.output[-1500:]
322+
row["calm_v2"] = score_arm(oracle_files, calm_v2_result)
323+
248324
rows.append(row)
249325

250326
summary = {
251-
"phase": "B7 Phase 1-3 (fd/Rust, flask/Python, express/JS, zod/TS, gin/Go, spring-petclinic/Java)",
327+
"phase": "B7 Phase 1-3 (fd/Rust, flask/Python, express/JS, zod/TS, gin/Go, spring-petclinic/Java)"
328+
" + v2 (2026-08-20 roadmap doc, calm_v2 = edit_context + reference_impact)",
252329
"methodology": "deterministic oracle only (build/test pass + independent "
253330
"callsite recall via B12's ground_truth, extension-filtered) "
254331
"-- no LLM judge, per design spec constraint",
@@ -258,14 +335,14 @@ def main() -> int:
258335
out_path.write_text(json.dumps(summary, indent=2))
259336

260337
print()
261-
print("| task | baseline | naive build_pass | naive recall | calm build_pass | calm recall |")
262-
print("|---|---|---|---|---|---|")
338+
print("| task | baseline | naive build_pass | naive recall | calm build_pass | calm recall | calm_v2 build_pass | calm_v2 recall |")
339+
print("|---|---|---|---|---|---|---|---|")
263340
for row in rows:
264341
if "skipped_reason" in row:
265-
print(f"| {row['id']} | SKIPPED | - | - | - | - | ({row['skipped_reason']}) |")
342+
print(f"| {row['id']} | SKIPPED | - | - | - | - | - | - | ({row['skipped_reason']}) |")
266343
continue
267-
n, c = row["naive"], row["calm"]
268-
print(f"| {row['id']} | green | {n['build_pass']} | {n['recall']} | {c['build_pass']} | {c['recall']} |")
344+
n, c, v2 = row["naive"], row["calm"], row["calm_v2"]
345+
print(f"| {row['id']} | green | {n['build_pass']} | {n['recall']} | {c['build_pass']} | {c['recall']} | {v2['build_pass']} | {v2['recall']} |")
269346
print(f"\nfull results written to {out_path}")
270347
return 0
271348

0 commit comments

Comments
 (0)