diff --git a/.github/workflows/scip-nightly.yml b/.github/workflows/scip-nightly.yml index 13e6de58..2d9ad42e 100644 --- a/.github/workflows/scip-nightly.yml +++ b/.github/workflows/scip-nightly.yml @@ -54,6 +54,30 @@ jobs: cargo test -p calm-cli --features scip-overlay -- --ignored \ calm_index_cli_upgrades_a_real_edge_on_the_fixture + # B2 (benchmarks/b2_call_graph_quality) -- self-repo Rust call-graph + # precision/recall against a rust-analyzer SCIP oracle. Self-contained (no + # external corpus clone, pure-stdlib Python script) so unlike B7 it's + # cheap enough to gate nightly rather than staying manual-only -- see + # scripts/check-b2-thresholds.sh for the floors and why they were picked. + # Piggybacks this job's own rust-analyzer install (same reasoning as the + # calm-cli step in scip-rust above) rather than getting a separate one. + b2-call-graph-quality: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + - uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable + with: + components: rust-analyzer + - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2 + with: + key: b2-call-graph-quality + - name: Build calm-cli with scip-overlay (needed for the hidden scip-dump subcommand) + run: cargo build --release -p calm-cli --features scip-overlay + - name: Run B2 benchmark (self-repo) + run: python3 benchmarks/b2_call_graph_quality/run_benchmark.py + - name: Check B2 results against regression floors + run: scripts/check-b2-thresholds.sh + scip-go: runs-on: ubuntu-latest steps: diff --git a/crates/calm-core/src/indexer/parser.rs b/crates/calm-core/src/indexer/parser.rs index 79030231..4790f5e3 100644 --- a/crates/calm-core/src/indexer/parser.rs +++ b/crates/calm-core/src/indexer/parser.rs @@ -1531,6 +1531,29 @@ fn module_hint_of(raw: &str) -> Option { } } +// The rightmost meaningful segment of a module specifier string -- "utils" +// from "../lib/utils" or "./utils", "os" from "os", "path" from Python's +// dotted "os.path". Counterpart to `module_hint_of` above for languages +// that name their module via `ctx.import_map` (populated from a parsed +// import/require statement) rather than a `::`-delimited path in the call +// text itself -- see `extract_file_data`'s receiver-import-alias branch, +// the only caller. Deliberately simple (slash-split, then dot-split, +// first non-empty segment) to match `module_hint_of`'s own "best-effort +// hint, not a real module resolver" scope: `resolve_sites_to_edges` +// treats whatever this returns as a fail-open filter over an +// already-name-matched candidate list, never a source of new candidates, +// so an imprecise segment on some exotic module-path shape just narrows +// nothing (today's behavior) rather than resolving wrong. +pub fn module_path_last_segment(module_path: &str) -> Option { + let after_slash = module_path.rsplit('/').next().unwrap_or(module_path); + let after_dot = after_slash.rsplit('.').next().unwrap_or(after_slash); + if after_dot.is_empty() { + None + } else { + Some(after_dot.to_string()) + } +} + /// Heuristic: a path segment is "type-like" when it starts with an uppercase /// letter, matching Rust/C#/Java/Kotlin/Swift convention for types/classes /// (vs. snake_case modules or lowerCamelCase namespaces/packages). Not diff --git a/crates/calm-core/src/indexer/pipeline.rs b/crates/calm-core/src/indexer/pipeline.rs index b07936f4..f0eece49 100644 --- a/crates/calm-core/src/indexer/pipeline.rs +++ b/crates/calm-core/src/indexer/pipeline.rs @@ -576,6 +576,7 @@ fn extract_file_data( if let Some(enc_qn) = enc_qn { let mut confidence; let mut target_class: Option = None; + let mut module_hint = c.module_hint.clone(); if c.receiver_is_type_path && let Some(receiver) = &c.receiver @@ -663,6 +664,37 @@ fn extract_file_data( // runs per-file, in parallel, before it's built). confidence = EdgeConfidence::Inferred; target_class = Some(receiver.clone()); + } else if confidence == EdgeConfidence::Textual + && module_hint.is_none() + && let Some(receiver) = &c.receiver + && let Some(module_path) = ctx.import_map.get(receiver) + && let Some(seg) = crate::indexer::parser::module_path_last_segment(module_path) + { + // Whole-module require/import binding (`var utils = + // require('../lib/utils')`, Python `import os`): tier-1 + // above only checks `ctx.import_map` keyed by the + // CALLEE name (`setCharset`), and tier-2 only checks + // `ctx.type_map` (a real type annotation) or self/this — + // neither ever asks "is the RECEIVER itself a name this + // file imported as a whole module?", so a call like + // `utils.setCharset(...)` fell all the way through to + // Textual with no receiver-derived signal at all, unlike + // the destructured-import sibling call `setCharset(...)` + // (bare, resolves fine via tier-1's own import_map + // check). Reuses the exact same `module_hint` file-stem + // filter `resolve_sites_to_edges` already applies for + // Rust's `crate::module::function()` (module_hint_of + // above) -- a pure NARROWING filter over the + // already-name-matched `ctx.by_name` candidate list, + // fail-open when the hint matches nothing (identical to + // today's behavior), never a source of new candidates on + // its own. Found live via B7 (benchmarks/ + // b7_task_correctness/README.md): express's + // `test/utils.js` calls `utils.setCharset(...)` after + // `var utils = require('../lib/utils')` -- completely + // absent from `callers()`/`blast_radius` before this fix. + confidence = EdgeConfidence::Inferred; + module_hint = Some(seg); } } let callee = aliases.get(&c.callee).unwrap_or(&c.callee).clone(); @@ -683,7 +715,7 @@ fn extract_file_data( receiver: c.receiver.clone(), target_class, looks_option_or_result_chained: c.looks_option_or_result_chained, - module_hint: c.module_hint.clone(), + module_hint, edge_kind: "call".to_string(), arg_count: c.arg_count, }); @@ -5140,6 +5172,75 @@ impl StructB { let _ = std::fs::remove_dir_all(&dir); } + #[test] + fn test_js_require_namespace_object_property_call_resolves_via_module_hint() { + // Regression test for the real, reproducible gap found in + // benchmarks/b7_task_correctness/README.md (express's `test/utils.js`): + // `utils.setCharset(...)` after `var utils = require('./lib/utils')` + // was completely invisible to `callers()`/`blast_radius` -- tier-1 + // (extract_file_data) only checks `ctx.import_map` keyed by the CALLEE + // name, tier-2 only checks `ctx.type_map` (a real type annotation) or + // self/this, so a call whose RECEIVER is itself a whole-module import + // binding had no signal at all. See extract_file_data's + // receiver-import-alias branch (module_path_last_segment). + // + // Fixture deliberately includes a same-named DECOY (`other/helpers.js` + // also exports `setCharset`) with a DIFFERENT file basename than the + // required module ("helpers" vs "utils") -- without the fix, both + // candidates survive `resolve_sites_to_edges`'s unscoped by-name + // fallback (JS gets no same_dir narrowing) and the edge comes out + // `Ambiguous` with 2 targets; the fix's module_hint ("utils", from the + // require path) should filter the decoy out by file-stem mismatch, + // leaving exactly one edge to the real target. + let dir = std::env::temp_dir().join(format!("ci_idx_js_require_ns_{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(dir.join("lib")).unwrap(); + std::fs::create_dir_all(dir.join("other")).unwrap(); + std::fs::write( + dir.join("lib/utils.js"), + "exports.setCharset = function (type, charset) {\n return type + '; charset=' + charset;\n};\n", + ) + .unwrap(); + std::fs::write( + dir.join("other/helpers.js"), + "exports.setCharset = function (type, charset) {\n return 'decoy';\n};\n", + ) + .unwrap(); + std::fs::write( + dir.join("caller.js"), + "var utils = require('./lib/utils');\nfunction run() {\n utils.setCharset('text/html', 'utf-8');\n}\n", + ) + .unwrap(); + + let mut conn = Connection::open_in_memory().unwrap(); + init_db(&conn).unwrap(); + run_indexing_pipeline(&mut conn, &dir, dummy_phase()).unwrap(); + + let mut stmt = conn + .prepare( + "SELECT to_symbol, edge_confidence FROM call_edges \ + WHERE from_symbol = 'caller.js::run' AND to_symbol LIKE '%setCharset'", + ) + .unwrap(); + let rows: Vec<(String, String)> = stmt + .query_map([], |r| Ok((r.get(0)?, r.get(1)?))) + .unwrap() + .map(|r| r.unwrap()) + .collect(); + + assert_eq!( + rows, + vec![( + "lib/utils.js::setCharset".to_string(), + "inferred".to_string() + )], + "utils.setCharset(...) via `var utils = require(...)` must resolve to exactly \ + lib/utils.js::setCharset (not the other/helpers.js decoy, not Ambiguous): {rows:?}" + ); + + let _ = std::fs::remove_dir_all(&dir); + } + #[test] // P1.2 end-to-end DoD: all 4 steps together on one small PHP project — // require_once resolves its import_edge; a typed property's diff --git a/crates/calm-server/src/tools/common.rs b/crates/calm-server/src/tools/common.rs index 7acb108a..c4cf3a15 100644 --- a/crates/calm-server/src/tools/common.rs +++ b/crates/calm-server/src/tools/common.rs @@ -935,3 +935,55 @@ pub(crate) struct RelatedNoteOutput { /// confident freshness read). pub(crate) staleness: &'static str, } + +#[cfg(test)] +mod caller_set_digest_tests { + use super::CalmServer; + + // WS-2 Phase 2's whole safety guarantee rests on `caller_set_digest` + // hashing the SET of callers, not the list -- these lock in the + // property the docstring promises but no existing test isolates + // (the 2 integration tests in tools.rs exercise the gate end-to-end + // on one fixed caller list, never a reordered/duplicated one). + + #[test] + fn order_and_duplicates_do_not_change_the_digest() { + let a = vec![ + "b.rs::caller_one".to_string(), + "a.rs::caller_two".to_string(), + ]; + let b = vec![ + "a.rs::caller_two".to_string(), + "a.rs::caller_two".to_string(), + "b.rs::caller_one".to_string(), + ]; + assert_eq!( + CalmServer::caller_set_digest(&a), + CalmServer::caller_set_digest(&b), + "same underlying caller set, different order/duplicates -- must hash identically" + ); + } + + #[test] + fn a_real_change_in_the_caller_set_changes_the_digest() { + let before = vec!["a.rs::caller_one".to_string()]; + let after = vec![ + "a.rs::caller_one".to_string(), + "b.rs::caller_two".to_string(), + ]; + assert_ne!( + CalmServer::caller_set_digest(&before), + CalmServer::caller_set_digest(&after), + "adding a real caller must change the digest -- this is exactly what STALE_CALLER_SET detects" + ); + } + + #[test] + fn empty_caller_set_is_deterministic() { + let empty: Vec = Vec::new(); + assert_eq!( + CalmServer::caller_set_digest(&empty), + CalmServer::caller_set_digest(&empty) + ); + } +} diff --git a/scripts/check-b2-thresholds.sh b/scripts/check-b2-thresholds.sh new file mode 100755 index 00000000..1744e1cc --- /dev/null +++ b/scripts/check-b2-thresholds.sh @@ -0,0 +1,79 @@ +#!/usr/bin/env bash +# Regression gate for B2 (benchmarks/b2_call_graph_quality) -- the Rust +# call-graph precision/recall benchmark against a rust-analyzer SCIP oracle. +# +# Written 2026-08-04 after an audit found this repo's benchmark suite had +# rigorous methodology but ZERO of its correctness numbers (B2/B3/B4/B6/B7) +# wired into any CI gate -- thresholds.toml only covers architectural +# fitness metrics (hotspot risk, boundaries, config drift). A real indexer +# regression (e.g. the F1 anonymous-callback JS bug this repo already found +# and fixed once, commit 34918c4) could ship silently on the Rust resolver +# with nothing to catch it. This closes that gap for B2 specifically -- +# it's the one benchmark in the suite that's self-contained (self-repo only, +# no external corpus clone, no multi-language build matrix) and therefore +# cheap enough to run in CI, unlike B7 (clones + builds 6 external repos per +# run) which stays a manual/periodic benchmark for cost reasons. +# +# Floors below are set with headroom under the last real measurement +# (benchmarks/b2_call_graph_quality/README.md's "Kết quả đo lần đầu" table), +# same margin-not-exact-match reasoning thresholds.toml's own +# max_hotspot_risk comment already documents for this repo: catch a real +# regression, don't flake on ordinary noise. +# measured -> floor +# recall 0.193 -> 0.15 (resolver intentionally does no type +# inference; recall moving at all is +# expected as Tier-0/2 evolves -- this +# floor exists to catch a collapse, not +# to lock the number down) +# precision 0.795 -> 0.70 +# inferred precision 0.967 -> 0.85 +# resolved precision 0.935 -> 0.80 +# textual precision 0.514 -> 0.40 (already the weakest tier by design -- +# agents are told to trust it least -- +# but a further collapse is still a +# real regression worth catching) +# +# Usage: +# scripts/check-b2-thresholds.sh [path-to-results.json] +# (default: benchmarks/b2_call_graph_quality/results.json) +set -euo pipefail +cd "$(dirname "$0")/.." + +results="${1:-benchmarks/b2_call_graph_quality/results.json}" + +if [ ! -f "$results" ]; then + echo "check-b2-thresholds: $results missing -- run benchmarks/b2_call_graph_quality/run_benchmark.py first" >&2 + exit 1 +fi + +if ! command -v jq >/dev/null 2>&1; then + echo "check-b2-thresholds: jq not found -- install it first" >&2 + exit 1 +fi + +fail=0 + +check() { + local label="$1" floor="$2" actual="$3" + if [ "$actual" = "null" ]; then + echo "check-b2-thresholds: $label missing from $results (expected a number)" >&2 + fail=1 + return + fi + if ! awk -v a="$actual" -v f="$floor" 'BEGIN { exit !(a+0 >= f+0) }'; then + echo "check-b2-thresholds: $label = $actual, below floor $floor" >&2 + fail=1 + fi +} + +check "overall recall" 0.15 "$(jq -r '.recall' "$results")" +check "overall precision" 0.70 "$(jq -r '.precision' "$results")" +check "inferred-tier precision" 0.85 "$(jq -r '.by_confidence.inferred.precision // "null"' "$results")" +check "resolved-tier precision" 0.80 "$(jq -r '.by_confidence.resolved.precision // "null"' "$results")" +check "textual-tier precision" 0.40 "$(jq -r '.by_confidence.textual.precision // "null"' "$results")" + +if [ "$fail" -ne 0 ]; then + echo "check-b2-thresholds: regression detected -- see floors and rationale in this script's header" >&2 + exit 1 +fi +echo "check-b2-thresholds: all B2 metrics at or above their floor."