Skip to content

Commit fb58b2b

Browse files
Your Nameclaude
andcommitted
fix: full-reindex leaves stale T1 facts + bundle import ignores config drift
Two independent correctness fixes found together this session: - A full reindex cleared symbols/call_sites/etc but not type_relations/ symbol_effects. Both key off qualified_name (not symbols.id), so a stale fact from a prior full reindex could silently coexist with the current fact for any symbol whose qualified name survived the rebuild -- corrupting T1 facts and the Architecture Digest built from them. Regression tests lock this in via a real two-pass reindex. - import_bundle computed force_full_reindex from only commit_matches/ version_matches, silently ignoring config_matches even though config_fingerprint was already being compared and returned in the same ImportReport. A bundle built with different ignore rules than the importing repo's current config would activate without the recommended full reindex. Also threads the real ignore_patterns into rebuild_graph/ incremental_graph_update's compute_package_dependencies call, which previously always passed an empty list. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent e8764f4 commit fb58b2b

3 files changed

Lines changed: 234 additions & 11 deletions

File tree

crates/calm-core/src/bundle.rs

Lines changed: 70 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,12 @@
3434
//! needs a C compiler, the exact problem class this workspace has
3535
//! repeatedly hit and removed on musl cross-compiles.
3636
//!
37-
//! **Import-as-seed triggers on ANY git-commit mismatch, not just "tree
38-
//! doesn't match".** If the bundle's `git_commit` differs from the
39-
//! importing repo's current HEAD (or either side has none), the bundle is
40-
//! still activated -- but `ImportReport::force_full_reindex` is set,
37+
//! **Import-as-seed triggers on ANY git-commit, `calm_version`, or
38+
//! `config_fingerprint` mismatch, not just "tree doesn't match".** If the
39+
//! bundle's `git_commit` differs from the importing repo's current HEAD (or
40+
//! either side has none), or the bundle's languages/ignore rules
41+
//! (`config_fingerprint`) don't match this project's current config, the
42+
//! bundle is still activated -- but `ImportReport::force_full_reindex` is set,
4143
//! telling the caller to run a full reindex afterward rather than trust
4244
//! incremental reconciliation, which only re-touches files whose content
4345
//! actually changed and so could never notice "the code that INTERPRETS
@@ -134,9 +136,11 @@ pub struct ImportReport {
134136
pub manifest: BundleManifest,
135137
pub commit_matches: bool,
136138
pub config_matches: bool,
137-
/// `true` when EITHER the git commit doesn't match OR `calm_version`
138-
/// differs from this binary's own version -- see the module doc
139-
/// comment's "Import-as-seed" section for why both trigger this.
139+
/// `true` when the git commit doesn't match, OR `calm_version` differs
140+
/// from this binary's own version, OR `config_fingerprint` (languages/
141+
/// ignore rules) doesn't match this project's current config -- see the
142+
/// module doc comment's "Import-as-seed" section for why all three
143+
/// trigger this.
140144
pub force_full_reindex: bool,
141145
pub activated_path: PathBuf,
142146
}
@@ -310,7 +314,7 @@ pub fn import_bundle(
310314
};
311315
let config_matches = manifest.config_fingerprint == config_fingerprint(config);
312316
let version_matches = manifest.calm_version == env!("CARGO_PKG_VERSION");
313-
let force_full_reindex = !commit_matches || !version_matches;
317+
let force_full_reindex = !commit_matches || !version_matches || !config_matches;
314318

315319
// Drop any stale WAL/SHM sidecars belonging to the file about to be
316320
// replaced -- VACUUM INTO's own output is always a single plain file
@@ -683,6 +687,64 @@ mod tests {
683687
assert!(report.config_matches);
684688
}
685689

690+
/// Bug fix 2026-08-08: `force_full_reindex` used to be computed from
691+
/// only `commit_matches`/`version_matches`, silently ignoring
692+
/// `config_matches` even though `config_fingerprint` was already being
693+
/// compared and returned in the same `ImportReport`. Uses a real git
694+
/// repo (same commit on both sides) so `commit_matches` is isolated as
695+
/// `true`, proving `config_matches` alone now drives the recommendation.
696+
#[test]
697+
fn import_reports_force_full_reindex_when_only_config_differs() {
698+
let src_dir = tempfile::tempdir().unwrap();
699+
let run = |args: &[&str]| {
700+
std::process::Command::new("git")
701+
.args(args)
702+
.current_dir(src_dir.path())
703+
.output()
704+
.unwrap()
705+
};
706+
run(&["init", "-q"]);
707+
run(&[
708+
"-c",
709+
"user.email=t@t",
710+
"-c",
711+
"user.name=t",
712+
"commit",
713+
"--allow-empty",
714+
"-q",
715+
"-m",
716+
"x",
717+
]);
718+
719+
let db_path = src_dir.path().join("index.db");
720+
seed_index_db(&db_path);
721+
let export_config = Config::default();
722+
let archive_path = src_dir.path().join("bundle.tar.gz");
723+
export_bundle(&db_path, src_dir.path(), &export_config, &archive_path).unwrap();
724+
725+
let dest_db_path = src_dir.path().join("imported.db");
726+
let mut import_config = Config::default();
727+
import_config
728+
.ignore
729+
.push("some_custom_ignore_rule".to_string());
730+
let report =
731+
import_bundle(&archive_path, src_dir.path(), &dest_db_path, &import_config).unwrap();
732+
733+
assert!(
734+
report.commit_matches,
735+
"same repo, same HEAD commit on both sides"
736+
);
737+
assert!(
738+
!report.config_matches,
739+
"ignore rules differ between export and import config"
740+
);
741+
assert!(
742+
report.force_full_reindex,
743+
"a config_fingerprint mismatch alone must force a full reindex \
744+
recommendation, even when the commit matches"
745+
);
746+
}
747+
686748
#[test]
687749
fn import_removes_stale_wal_sidecars_next_to_the_activated_path() {
688750
let src_dir = tempfile::tempdir().unwrap();

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

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1403,6 +1403,7 @@ fn rebuild_graph(
14031403
churn_since: &str,
14041404
hub_config: &crate::config::HubThresholdConfig,
14051405
maps: &ResolutionMaps,
1406+
ignore: &[String],
14061407
) -> rusqlite::Result<()> {
14071408
let ctx = build_resolution_context(tx, &maps.namespace_map)?;
14081409

@@ -1461,7 +1462,7 @@ fn rebuild_graph(
14611462
crate::graph::boundary::update_boundary_ambiguous_flags(tx)?;
14621463
crate::graph::churn::update_churn_scores(tx, project_root, churn_since)?;
14631464
crate::graph::digest::compute_digests(tx)?;
1464-
crate::indexer::package_deps::compute_package_dependencies(tx, project_root, &[])?;
1465+
crate::indexer::package_deps::compute_package_dependencies(tx, project_root, ignore)?;
14651466
Ok(())
14661467
}
14671468

@@ -1488,6 +1489,7 @@ pub fn rebuild_graph_from_index(
14881489
&config.hotspots.default_since,
14891490
&config.hub_threshold,
14901491
&maps,
1492+
&config.ignore,
14911493
)?;
14921494
tx.execute(
14931495
"UPDATE graph_generation_state SET generation = generation + 1 WHERE id = 1",
@@ -1520,6 +1522,7 @@ pub enum IncrementalOutcome {
15201522
/// loaded and (b) how much of `call_edges` gets deleted first; see plan
15211523
/// §3.1 for the proof that `delta_paths` below is sufficient to catch every
15221524
/// input `resolve_sites_to_edges` depends on.
1525+
#[allow(clippy::too_many_arguments)]
15231526
pub fn incremental_graph_update(
15241527
tx: &rusqlite::Transaction,
15251528
project_root: &std::path::Path,
@@ -1528,6 +1531,7 @@ pub fn incremental_graph_update(
15281531
names_delta: &HashSet<String>,
15291532
hub_config: &crate::config::HubThresholdConfig,
15301533
maps: &ResolutionMaps,
1534+
ignore: &[String],
15311535
) -> rusqlite::Result<IncrementalOutcome> {
15321536
// Step 1 (plan D1): delta_paths = delta_seed ∪ {from_path of call_sites
15331537
// whose callee_name ∈ names_delta} — a site in an UNCHANGED file that
@@ -1562,7 +1566,7 @@ pub fn incremental_graph_update(
15621566
"delta_paths.len()={} > {MAX_INCREMENTAL_DELTA_PATHS}",
15631567
delta_paths.len()
15641568
);
1565-
rebuild_graph(tx, project_root, churn_since, hub_config, maps)?;
1569+
rebuild_graph(tx, project_root, churn_since, hub_config, maps, ignore)?;
15661570
return Ok(IncrementalOutcome::FellBackToFull(reason));
15671571
}
15681572

@@ -1652,7 +1656,7 @@ pub fn incremental_graph_update(
16521656
crate::graph::boundary::update_boundary_ambiguous_flags(tx)?;
16531657
crate::graph::churn::update_churn_scores(tx, project_root, churn_since)?;
16541658
crate::graph::digest::compute_digests(tx)?;
1655-
crate::indexer::package_deps::compute_package_dependencies(tx, project_root, &[])?;
1659+
crate::indexer::package_deps::compute_package_dependencies(tx, project_root, ignore)?;
16561660

16571661
Ok(IncrementalOutcome::Applied)
16581662
}
@@ -2280,6 +2284,17 @@ fn reindex_all_cancellable_with_phase(
22802284
tx.execute("DELETE FROM symbols", [])?;
22812285
tx.execute("DELETE FROM file_index", [])?;
22822286
tx.execute("DELETE FROM code_chunks", [])?;
2287+
// Bug fix 2026-08-08: these two were missing from the full-reindex clear
2288+
// even though `remove_file_rows` (the per-file incremental path) already
2289+
// clears them. Both tables key off `qualified_name`/`source_path`, not
2290+
// `symbols.id` (see their schema.rs comments), so a stale row here is not
2291+
// just orphaned garbage -- it silently re-attaches to whatever symbol the
2292+
// NEXT full reindex assigns the same qualified name, corrupting T1 facts
2293+
// and the Architecture Digest built from them for a symbol whose actual
2294+
// semantics changed. See golden_graph_equivalence.rs for the regression
2295+
// test locking this in (full-rebuild-on-old-DB must equal fresh-build).
2296+
tx.execute("DELETE FROM type_relations", [])?;
2297+
tx.execute("DELETE FROM symbol_effects", [])?;
22832298
// A full baseline invalidates all cached SCIP results. In particular, D4's
22842299
// byte-span identity migration must never let a line-derived cache key skip
22852300
// the first exact overlay pass after rebuilding the graph.
@@ -2340,6 +2355,7 @@ fn reindex_all_cancellable_with_phase(
23402355
&config.hotspots.default_since,
23412356
&config.hub_threshold,
23422357
&maps,
2358+
&ignore_patterns,
23432359
)?;
23442360
tx.execute(
23452361
"UPDATE graph_generation_state SET generation = generation + 1 WHERE id = 1",
@@ -2858,6 +2874,7 @@ pub fn reindex_changed_cancellable(
28582874
&summary.names_delta,
28592875
&config.hub_threshold,
28602876
&maps,
2877+
&ignore_patterns,
28612878
)? {
28622879
IncrementalOutcome::Applied => summary.graph_mode = GraphMode::Incremental,
28632880
IncrementalOutcome::FellBackToFull(reason) => {
@@ -2871,6 +2888,7 @@ pub fn reindex_changed_cancellable(
28712888
&config.hotspots.default_since,
28722889
&config.hub_threshold,
28732890
&maps,
2891+
&ignore_patterns,
28742892
)?;
28752893
summary.graph_mode = GraphMode::Full;
28762894
}
@@ -3013,6 +3031,7 @@ pub fn reindex_paths(
30133031
&summary.names_delta,
30143032
&config.hub_threshold,
30153033
&maps,
3034+
&config.ignore,
30163035
)? {
30173036
IncrementalOutcome::Applied => summary.graph_mode = GraphMode::Incremental,
30183037
IncrementalOutcome::FellBackToFull(reason) => {
@@ -3026,6 +3045,7 @@ pub fn reindex_paths(
30263045
&config.hotspots.default_since,
30273046
&config.hub_threshold,
30283047
&maps,
3048+
&config.ignore,
30293049
)?;
30303050
summary.graph_mode = GraphMode::Full;
30313051
}

crates/calm-core/tests/golden_graph_equivalence.rs

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1371,3 +1371,144 @@ fn names_delta_over_chunk_size_matches_full() {
13711371
"names_delta chunk-boundary: incremental must match full rebuild",
13721372
);
13731373
}
1374+
1375+
/// Bug fix 2026-08-08: a full reindex used to clear `symbols`/`call_sites`/
1376+
/// etc. but NOT `type_relations`/`symbol_effects` -- see the fix in
1377+
/// `pipeline.rs`'s `reindex_all_cancellable_with_phase`. Both tables key off
1378+
/// `qualified_name` (not `symbols.id`), so a stale fact from a prior full
1379+
/// reindex used to silently coexist with the current fact for any symbol
1380+
/// whose qualified name survived the rebuild -- corrupting T1 facts and the
1381+
/// Architecture Digest built from them. `run_indexing_pipeline` always takes
1382+
/// the full-baseline path (never incremental), so calling it twice on the
1383+
/// same connection with different file content between calls is exactly the
1384+
/// regression scenario.
1385+
#[test]
1386+
fn full_reindex_does_not_leave_stale_semantic_facts() {
1387+
let tmp = tempfile::tempdir().unwrap();
1388+
let root = tmp.path().join("ws");
1389+
std::fs::create_dir_all(&root).unwrap();
1390+
std::fs::write(
1391+
root.join("service.py"),
1392+
"class Service:\n def save(self):\n self.cache = 1\n raise ValueError(\"boom\")\n",
1393+
)
1394+
.unwrap();
1395+
1396+
let mut db = index_fresh(&root);
1397+
let effects_before: i64 = db
1398+
.query_row("SELECT COUNT(*) FROM symbol_effects", [], |r| r.get(0))
1399+
.unwrap();
1400+
assert_eq!(
1401+
effects_before, 2,
1402+
"fixture must produce exactly 1 throw + 1 write fact to exercise the bug"
1403+
);
1404+
1405+
// Second full reindex (run_indexing_pipeline always takes the
1406+
// full-baseline path) with the SAME qualified name (`Service::save`
1407+
// survives) but neither a raise nor a field write in the new source.
1408+
std::fs::write(
1409+
root.join("service.py"),
1410+
"class Service:\n def save(self):\n return 1\n",
1411+
)
1412+
.unwrap();
1413+
let phase = std::sync::Arc::new(std::sync::RwLock::new(
1414+
calm_core::types::IndexingPhase::Scanning,
1415+
));
1416+
calm_core::indexer::pipeline::run_indexing_pipeline(&mut db, &root, phase).unwrap();
1417+
1418+
let effects_after: i64 = db
1419+
.query_row("SELECT COUNT(*) FROM symbol_effects", [], |r| r.get(0))
1420+
.unwrap();
1421+
assert_eq!(
1422+
effects_after, 0,
1423+
"full reindex must not leave stale symbol_effects behind for a symbol \
1424+
whose qualified name survived the rebuild"
1425+
);
1426+
}
1427+
1428+
/// Same bug, `type_relations` half: a base-class relation recorded in
1429+
/// version A must not survive a full reindex into a version B that dropped
1430+
/// the base class entirely, even though the class's own qualified name is
1431+
/// unchanged.
1432+
#[test]
1433+
fn full_reindex_does_not_leave_stale_type_relations() {
1434+
let tmp = tempfile::tempdir().unwrap();
1435+
let root = tmp.path().join("ws");
1436+
std::fs::create_dir_all(&root).unwrap();
1437+
std::fs::write(
1438+
root.join("service.py"),
1439+
"class Base:\n pass\n\nclass Service(Base):\n pass\n",
1440+
)
1441+
.unwrap();
1442+
1443+
let mut db = index_fresh(&root);
1444+
let relations_before: i64 = db
1445+
.query_row("SELECT COUNT(*) FROM type_relations", [], |r| r.get(0))
1446+
.unwrap();
1447+
assert_eq!(
1448+
relations_before, 1,
1449+
"fixture must produce exactly 1 extends fact to exercise the bug"
1450+
);
1451+
1452+
std::fs::write(root.join("service.py"), "class Service:\n pass\n").unwrap();
1453+
let phase = std::sync::Arc::new(std::sync::RwLock::new(
1454+
calm_core::types::IndexingPhase::Scanning,
1455+
));
1456+
calm_core::indexer::pipeline::run_indexing_pipeline(&mut db, &root, phase).unwrap();
1457+
1458+
let relations_after: i64 = db
1459+
.query_row("SELECT COUNT(*) FROM type_relations", [], |r| r.get(0))
1460+
.unwrap();
1461+
assert_eq!(
1462+
relations_after, 0,
1463+
"full reindex must not leave a stale type_relations row behind for a \
1464+
symbol whose qualified name survived the rebuild"
1465+
);
1466+
}
1467+
1468+
/// Bug fix 2026-08-08: `rebuild_graph`/`incremental_graph_update` used to
1469+
/// call `compute_package_dependencies` with a hardcoded empty ignore list,
1470+
/// so a user's configured `ignore` patterns never applied to package
1471+
/// manifest scanning even though they applied to every other part of
1472+
/// indexing. Both functions now take and forward a real `ignore: &[String]`
1473+
/// parameter sourced from `config.ignore`.
1474+
#[test]
1475+
fn package_dependencies_scan_respects_configured_ignore_rules() {
1476+
let tmp = tempfile::tempdir().unwrap();
1477+
let root = tmp.path().join("ws");
1478+
std::fs::create_dir_all(root.join("vendor")).unwrap();
1479+
// Written before the first index, same reasoning as
1480+
// enable_incremental_graph's doc comment: config.json must never appear
1481+
// as a changed path in a later delta.
1482+
std::fs::write(root.join("config.json"), r#"{"ignore":["vendor"]}"#).unwrap();
1483+
std::fs::write(
1484+
root.join("Cargo.toml"),
1485+
"[package]\nname = \"root_pkg\"\nversion = \"0.1.0\"\n\n[dependencies]\nfoo = \"1\"\n",
1486+
)
1487+
.unwrap();
1488+
std::fs::write(
1489+
root.join("vendor/Cargo.toml"),
1490+
"[package]\nname = \"vendored_pkg\"\nversion = \"0.1.0\"\n\n[dependencies]\nserde = \"1\"\n",
1491+
)
1492+
.unwrap();
1493+
1494+
let db = index_fresh(&root);
1495+
let names: Vec<String> = {
1496+
let mut stmt = db
1497+
.prepare("SELECT dependency_name FROM package_dependencies ORDER BY dependency_name")
1498+
.unwrap();
1499+
stmt.query_map([], |r| r.get::<_, String>(0))
1500+
.unwrap()
1501+
.collect::<Result<Vec<_>, _>>()
1502+
.unwrap()
1503+
};
1504+
1505+
assert!(
1506+
names.iter().any(|n| n == "foo"),
1507+
"root Cargo.toml's dependency must still be scanned: {names:?}"
1508+
);
1509+
assert!(
1510+
!names.iter().any(|n| n == "serde"),
1511+
"vendor/ is configured as ignored, so vendor/Cargo.toml's dependency \
1512+
must not appear: {names:?}"
1513+
);
1514+
}

0 commit comments

Comments
 (0)