Skip to content

Commit fd8cefd

Browse files
Your Nameclaude
andcommitted
fix(read-tools): dedup derived-edge tables + sharpen search/edit-context precision
Implements the fixes from the 2026-07-28 CALM read-tool self-audit (round-2 meta-audit corrections C1–C5, plus F3–F7). All findings were re-grounded against .calm/index.db directly before fixing. C1–C4 — derived-edge duplication (the severe one, ~62% of call_edges were byte-duplicate rows on a long-lived index): - Root cause: the SCIP overlay (scip::ingest::insert_missing_edges) re-inserts the same formal edge every run — its within-run dedup set keys on the target's symbols.line_start while the next run reloads under the SCIP def-occurrence line (key-space mismatch), and neither call_edges nor import_edges had a UNIQUE constraint. import_edges had an independent instance of the same class (extractor emits byte-identical rows). - Defense-in-depth fix: UNIQUE indexes on both tables (call_edges: from_symbol,to_symbol,COALESCE(call_site_line,-1),edge_kind; import_edges: from_path,COALESCE(to_path,''),module_name,COALESCE(symbols_used,'[]')) make every insert path idempotent; all three production inserts now use INSERT OR IGNORE, and insert_missing_edges counts only rows the constraint accepted. One-off, guarded cleanup migration collapses pre-existing dups (kept MIN(id) per logical edge). Verified on a copy of this repo's real DB: call_edges 10412→10126, import_edges 584→551, both unique indexes created. Payoff: correct edit_context risk + callers counts (stored caller_count was already COUNT(DISTINCT)-protected, so hub/coreness were never affected) and large token savings on affected symbols. C5 — search precision: search_symbol now guarantees exact-whole-name candidacy (a dedicated idx_symbols_name lookup, since a short exact name like `path` could fall outside the FTS fetch window entirely) and folds coreness + a decisive exact-name bonus into ranking, gated behind the existing noise check. `search("path")` now surfaces the symbol literally named `path` (the repo's highest-coreness hub) on top instead of PathMatcher/PathCache. rank_multiplier / rrf_merge_n and their tests are untouched. F3 — edit_context.blast_radius now confidence-filters `ambiguous` edges, matching risk_assessment (they were disagreeing on the same response). F4 — the shared NOT_FOUND caveat now names the stdlib/builtin/third-party scope boundary and warns that hybrid surfaces usages, not definitions. F5 — weak_cross_reference_languages gains an indexed_file_count denominator and suppresses low-match-rate flags below a 10-file sample floor (a handful of incidental scripts is not a cross-reference-quality signal). F6 — c/cpp install_hint now explains build/translation-unit coverage, the real lever on scip-clang match rate (per benchmarks/resolution). F7 — direct test coverage for the CalmServer::search handler (kind dispatch, suggested_next DAG, include_tests hard-filter) — previously only the core algorithm was tested, not the server-layer glue. Tests: cargo test --workspace green; all-languages+overlays green; clippy -D warnings clean (default + all-languages); fmt clean; tool schema snapshots regenerated for the new optional repo_overview/indexing_status field. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 1d58967 commit fd8cefd

13 files changed

Lines changed: 1148 additions & 32 deletions

File tree

crates/calm-core/src/analysis/martin.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,9 @@ mod tests {
267267

268268
fn insert_import(conn: &Connection, from_path: &str, to_path: &str) {
269269
conn.execute(
270-
"INSERT INTO import_edges (from_path, to_path, module_name) VALUES (?1, ?2, 'x')",
270+
// OR IGNORE mirrors production (indexer::edges): the UNIQUE index
271+
// on import_edges makes a duplicate insert a silent no-op.
272+
"INSERT OR IGNORE INTO import_edges (from_path, to_path, module_name) VALUES (?1, ?2, 'x')",
271273
rusqlite::params![from_path, to_path],
272274
)
273275
.unwrap();
@@ -334,11 +336,14 @@ mod tests {
334336
insert_symbol(&conn, "a.rs", "function", "rust", "f");
335337
insert_symbol(&conn, "b.rs", "function", "rust", "g");
336338
insert_import(&conn, "a.rs", "b.rs");
337-
insert_import(&conn, "a.rs", "b.rs"); // duplicate, as seen in this repo's own table
339+
insert_import(&conn, "a.rs", "b.rs"); // duplicate attempt — dropped by the UNIQUE index
338340

339341
let summary = compute_martin_metrics(&conn).unwrap();
340342
let a = summary.files.iter().find(|f| f.path == "a.rs").unwrap();
341-
assert_eq!(a.ce, 1, "DISTINCT must collapse the duplicate edge");
343+
// The UNIQUE index on import_edges (2026-07-28) prevents the duplicate
344+
// from ever landing; compute_martin_metrics' own COUNT(DISTINCT) is
345+
// retained as defense-in-depth. Either way, coupling stays at 1.
346+
assert_eq!(a.ce, 1, "duplicate import edge must not inflate coupling");
342347
}
343348

344349
#[test]

crates/calm-core/src/db/schema.rs

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,72 @@ fn run_migrations(conn: &Connection) -> rusqlite::Result<()> {
358358
migrate_add_column(conn, "symbols", "arity", "INTEGER")?;
359359
migrate_fts_add_signature(conn)?;
360360
migrate_add_project_memory_fts(conn)?;
361+
dedup_edges_and_add_unique_indexes(conn)?;
362+
Ok(())
363+
}
364+
365+
/// One-off cleanup + hardening for the two derived-edge tables (`call_edges`,
366+
/// `import_edges`), added 2026-07-28 after a self-audit found ~62% of
367+
/// `call_edges` rows on a long-lived index were byte-duplicate copies.
368+
///
369+
/// Root cause: the SCIP overlay (`scip::ingest::insert_missing_edges`) is a
370+
/// separate background pass with no pre-clear, and its within-run dedup set —
371+
/// keyed on the target's `symbols.line_start` — never matched the *next* run's
372+
/// reload, keyed on the SCIP def-occurrence line, so every overlay run
373+
/// re-inserted the same `formal` edge. `import_edges` had an independent
374+
/// instance of the same class (the per-file extractor can emit byte-identical
375+
/// rows). Neither table had a UNIQUE constraint to catch it.
376+
///
377+
/// Fix is defense-in-depth: a UNIQUE index makes *every* insert path
378+
/// idempotent regardless of per-path delete discipline (all three production
379+
/// inserts now use `INSERT OR IGNORE`). This first collapses any pre-existing
380+
/// duplicates — keeping the lowest `id` per logical edge; the copies are
381+
/// byte-identical so which survivor is kept is immaterial — then creates the
382+
/// indexes. Guarded on the (last-created) import index's existence so the
383+
/// whole-table cleanup scan runs exactly once, on the first open after
384+
/// upgrade; every later open short-circuits. Re-running is safe either way
385+
/// (both DELETEs and both `CREATE ... IF NOT EXISTS` are idempotent).
386+
///
387+
/// `call_site_line` / `to_path` / `symbols_used` are wrapped in `COALESCE`
388+
/// because SQLite treats NULLs as *distinct* in a UNIQUE index, which would
389+
/// otherwise let NULL-keyed duplicates slip past the constraint. The key
390+
/// deliberately excludes `edge_confidence`/`formal_source`/`ruled_out_by_scip`:
391+
/// those are attributes the overlay mutates in place on the *same* logical
392+
/// edge (`from_symbol`,`to_symbol`,`call_site_line`,`edge_kind`), never a
393+
/// second row.
394+
fn dedup_edges_and_add_unique_indexes(conn: &Connection) -> rusqlite::Result<()> {
395+
let already_hardened: i64 = conn.query_row(
396+
"SELECT COUNT(*) FROM sqlite_master \
397+
WHERE type = 'index' AND name = 'idx_import_edges_unique'",
398+
[],
399+
|r| r.get(0),
400+
)?;
401+
if already_hardened > 0 {
402+
return Ok(());
403+
}
404+
405+
// Collapse pre-constraint duplicates, keeping one row per logical edge.
406+
conn.execute(
407+
"DELETE FROM call_edges WHERE id NOT IN ( \
408+
SELECT MIN(id) FROM call_edges \
409+
GROUP BY from_symbol, to_symbol, COALESCE(call_site_line, -1), edge_kind)",
410+
[],
411+
)?;
412+
conn.execute(
413+
"DELETE FROM import_edges WHERE id NOT IN ( \
414+
SELECT MIN(id) FROM import_edges \
415+
GROUP BY from_path, COALESCE(to_path, ''), module_name, COALESCE(symbols_used, '[]'))",
416+
[],
417+
)?;
418+
419+
// call_edges index first, import index last: the guard above keys on the
420+
// import index, so a crash between the two CREATEs re-runs both cleanly.
421+
conn.execute_batch(
422+
"CREATE UNIQUE INDEX IF NOT EXISTS idx_call_edges_unique \
423+
ON call_edges(from_symbol, to_symbol, COALESCE(call_site_line, -1), edge_kind); \
424+
CREATE UNIQUE INDEX IF NOT EXISTS idx_import_edges_unique \
425+
ON import_edges(from_path, COALESCE(to_path, ''), module_name, COALESCE(symbols_used, '[]'));",
426+
)?;
361427
Ok(())
362428
}
363429

@@ -486,6 +552,96 @@ mod tests {
486552
assert_eq!(count, 0);
487553
}
488554

555+
#[test]
556+
fn dedup_migration_collapses_duplicate_edge_rows_and_blocks_new_dups() {
557+
// Simulate a pre-hardening DB: base tables exist (SCHEMA_SQL) but the
558+
// unique edge indexes / dedup migration have not run yet. This is the
559+
// 2026-07-28 self-audit repro (`boundaries.rs::PathMatcher` had 113
560+
// duplicate caller rows) reduced to its data-layer essence.
561+
let conn = Connection::open_in_memory().unwrap();
562+
conn.execute_batch(SCHEMA_SQL).unwrap();
563+
564+
// Three byte-identical call edges for one logical edge, plus a
565+
// genuinely-distinct edge (different to_symbol) that must survive.
566+
for _ in 0..3 {
567+
conn.execute(
568+
"INSERT INTO call_edges (from_symbol, to_symbol, call_site_line, edge_confidence, edge_kind) \
569+
VALUES ('a::f', 'b::g', 10, 'formal', 'call')",
570+
[],
571+
)
572+
.unwrap();
573+
}
574+
conn.execute(
575+
"INSERT INTO call_edges (from_symbol, to_symbol, call_site_line, edge_confidence, edge_kind) \
576+
VALUES ('a::f', 'c::h', 10, 'formal', 'call')",
577+
[],
578+
)
579+
.unwrap();
580+
// Two byte-identical imports + one distinct (different symbols_used).
581+
for _ in 0..2 {
582+
conn.execute(
583+
"INSERT INTO import_edges (from_path, to_path, module_name, symbols_used) \
584+
VALUES ('x.rs', 'y.rs', 'y', '[\"A\"]')",
585+
[],
586+
)
587+
.unwrap();
588+
}
589+
conn.execute(
590+
"INSERT INTO import_edges (from_path, to_path, module_name, symbols_used) \
591+
VALUES ('x.rs', 'y.rs', 'y', '[\"B\"]')",
592+
[],
593+
)
594+
.unwrap();
595+
596+
dedup_edges_and_add_unique_indexes(&conn).unwrap();
597+
598+
let call_rows: i64 = conn
599+
.query_row("SELECT COUNT(*) FROM call_edges", [], |r| r.get(0))
600+
.unwrap();
601+
assert_eq!(call_rows, 2, "3 identical + 1 distinct call edge -> 2");
602+
let import_rows: i64 = conn
603+
.query_row("SELECT COUNT(*) FROM import_edges", [], |r| r.get(0))
604+
.unwrap();
605+
assert_eq!(import_rows, 2, "2 identical + 1 distinct import edge -> 2");
606+
607+
// The constraint now makes further duplicate inserts idempotent, even
608+
// at a different confidence (the key excludes edge_confidence).
609+
let changed = conn
610+
.execute(
611+
"INSERT OR IGNORE INTO call_edges (from_symbol, to_symbol, call_site_line, edge_confidence, edge_kind) \
612+
VALUES ('a::f', 'b::g', 10, 'textual', 'call')",
613+
[],
614+
)
615+
.unwrap();
616+
assert_eq!(changed, 0, "duplicate (from,to,line,kind) is ignored");
617+
618+
// NULL call_site_line duplicates are caught too (COALESCE in the key).
619+
conn.execute(
620+
"INSERT OR IGNORE INTO call_edges (from_symbol, to_symbol, edge_confidence, edge_kind) \
621+
VALUES ('a::f', 'd::k', 'textual', 'call')",
622+
[],
623+
)
624+
.unwrap();
625+
let null_dup = conn
626+
.execute(
627+
"INSERT OR IGNORE INTO call_edges (from_symbol, to_symbol, edge_confidence, edge_kind) \
628+
VALUES ('a::f', 'd::k', 'textual', 'call')",
629+
[],
630+
)
631+
.unwrap();
632+
assert_eq!(null_dup, 0, "NULL-line duplicate ignored via COALESCE key");
633+
634+
// Idempotent: re-running the migration is a cheap no-op.
635+
dedup_edges_and_add_unique_indexes(&conn).unwrap();
636+
let call_rows_final: i64 = conn
637+
.query_row("SELECT COUNT(*) FROM call_edges", [], |r| r.get(0))
638+
.unwrap();
639+
assert_eq!(
640+
call_rows_final, 3,
641+
"2 survivors + 1 new distinct null-line edge"
642+
);
643+
}
644+
489645
#[test]
490646
fn migration_adds_boundary_ambiguous_column_defaulting_to_zero() {
491647
let conn = Connection::open_in_memory().unwrap();

crates/calm-core/src/indexer/edges.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,12 @@ pub fn insert_symbols_batch(tx: &Transaction, symbols: &[ParsedSymbol]) -> rusql
5151
}
5252

5353
pub fn insert_call_edges_batch(tx: &Transaction, edges: &[CallEdge]) -> rusqlite::Result<()> {
54+
// OR IGNORE: the UNIQUE index on call_edges (see db::schema
55+
// dedup_edges_and_add_unique_indexes) makes every insert path idempotent,
56+
// so a redundant re-insert (e.g. the same edge extracted twice in one
57+
// pass) collapses to a no-op instead of a duplicate row.
5458
let mut stmt = tx.prepare(
55-
"INSERT INTO call_edges (from_symbol, to_symbol, call_site_line, edge_confidence, from_path, to_path, edge_kind)
59+
"INSERT OR IGNORE INTO call_edges (from_symbol, to_symbol, call_site_line, edge_confidence, from_path, to_path, edge_kind)
5660
VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7)"
5761
)?;
5862
for e in edges {
@@ -70,8 +74,10 @@ pub fn insert_call_edges_batch(tx: &Transaction, edges: &[CallEdge]) -> rusqlite
7074
}
7175

7276
pub fn insert_import_edges_batch(tx: &Transaction, edges: &[ImportEdge]) -> rusqlite::Result<()> {
77+
// OR IGNORE: byte-identical import rows collapse via the UNIQUE index
78+
// (see db::schema dedup_edges_and_add_unique_indexes).
7379
let mut stmt = tx.prepare(
74-
"INSERT INTO import_edges (from_path, to_path, module_name, symbols_used)
80+
"INSERT OR IGNORE INTO import_edges (from_path, to_path, module_name, symbols_used)
7581
VALUES (?1, ?2, ?3, ?4)",
7682
)?;
7783
for e in edges {

crates/calm-core/src/scip/ingest.rs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -327,8 +327,16 @@ fn insert_missing_edges(
327327
}
328328

329329
let mut inserted = 0usize;
330+
// OR IGNORE + the UNIQUE index on call_edges (db::schema
331+
// dedup_edges_and_add_unique_indexes) are what actually keep this pass
332+
// idempotent across overlay runs. `already_represented` is keyed on the
333+
// target's `symbols.line_start` (from the JOIN in `ingest_occurrences`),
334+
// but the re-resolved SCIP def-occurrence line often differs from it, so
335+
// the in-memory check below can fail to recognize an edge inserted on a
336+
// prior run. The constraint backstops that miss: a re-insert becomes a
337+
// no-op instead of the duplicate that once inflated caller counts ~19x.
330338
let mut insert_stmt = conn.prepare(
331-
"INSERT INTO call_edges \
339+
"INSERT OR IGNORE INTO call_edges \
332340
(from_symbol, to_symbol, call_site_line, edge_confidence, from_path, to_path, \
333341
formal_source, ruled_out_by_scip) \
334342
VALUES (?1, ?2, ?3, 'formal', ?4, ?5, 'scip', 0)",
@@ -345,7 +353,9 @@ fn insert_missing_edges(
345353
let Some(to_qn) = resolve_unique_symbol_at(conn, def_path, def_line as i64)? else {
346354
continue;
347355
};
348-
insert_stmt.execute(rusqlite::params![
356+
// Count only rows the constraint actually accepted — an IGNOREd
357+
// duplicate returns 0, keeping `inserted`/telemetry honest.
358+
let n = insert_stmt.execute(rusqlite::params![
349359
enc_qn,
350360
to_qn,
351361
call_line as i64,
@@ -354,7 +364,7 @@ fn insert_missing_edges(
354364
])?;
355365
already_represented.insert((from_path, call_line, def_path, def_line as i64));
356366
satisfied_sites.insert((from_path.to_string(), call_line));
357-
inserted += 1;
367+
inserted += n;
358368
}
359369
}
360370
Ok(inserted)

0 commit comments

Comments
 (0)