Skip to content

Commit 407e5f4

Browse files
committed
fix(daemon): stop heal loop duplicating symbols on re-extraction
The on-demand heal path re-extracted "changed" files with a bare `extractor.extract` that never removed the file's existing nodes. Because `insert_node` always allocates a fresh id, every heal duplicated the file's symbols, accumulating edge-less phantom orphans that inflated the daemon's live symbol count and orphan report relative to a clean in-process build (orphans 933 vs 177 on a 921-file Spring monorepo). Extract the surgical core of `reindex_file_in_place` into `reindex_file_in_place_core` (remove old nodes -> re-extract -> re-link cross-file edges; idempotent on an unchanged file) and route the heal loop through it, so heal and the `reindex` IPC fast-path can no longer diverge. Add `reindex_file_in_place_core_is_idempotent_on_unchanged_file` regression test. Verified on the live daemon: symbol/edge counts stay stable across repeated heal passes.
1 parent 6732b8e commit 407e5f4

2 files changed

Lines changed: 151 additions & 36 deletions

File tree

crates/cli/src/commands/server.rs

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1018,19 +1018,17 @@ fn run_foreground(
10181018
stale.push(path.clone());
10191019
continue;
10201020
}
1021-
let source = match std::fs::read_to_string(path) {
1022-
Ok(s) => s,
1023-
Err(_) => continue,
1024-
};
1025-
for extractor in coregraph_extractor::all_extractors() {
1026-
if coregraph_extractor::scanner::extension_matches(
1027-
path,
1028-
extractor.file_extensions(),
1029-
) {
1030-
let _ = extractor.extract(path, &source, &mut g);
1031-
break;
1032-
}
1033-
}
1021+
// Surgical reindex: REMOVE the file's stale nodes
1022+
// (dropping incident edges) before re-extracting, so a
1023+
// heal pass over an unchanged file nets zero node change.
1024+
// The previous additive `extractor.extract` here left the
1025+
// old nodes in place; because `insert_node` always
1026+
// allocates a fresh id, every heal duplicated the file's
1027+
// symbols, accumulating phantom edge-less orphans that
1028+
// inflated the daemon's symbol/orphan counts over time.
1029+
// Shares the exact path used by the `reindex` IPC fast
1030+
// mode so the two can never diverge again.
1031+
let _ = crate::dispatch::reindex_file_in_place_core(&mut g, path);
10341032
healed.push(path.clone());
10351033
}
10361034
Some(coregraph_graph::HealingReport {

crates/cli/src/dispatch.rs

Lines changed: 140 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1439,8 +1439,59 @@ pub fn dispatch_reindex_mutable(
14391439
}
14401440
}
14411441

1442-
/// Fast single-file reindex: remove the file's old nodes (which drops
1443-
/// all incident edges via petgraph), then re-extract into the live graph.
1442+
/// Telemetry returned by [`reindex_file_in_place_core`].
1443+
pub(crate) struct FileReindexTelemetry {
1444+
pub nodes_removed: usize,
1445+
pub nodes_inserted: usize,
1446+
pub edges_removed_with_evidence: usize,
1447+
pub edges_inserted: usize,
1448+
pub cross_file_edges_staled: usize,
1449+
pub cross_file_edges_dropped: usize,
1450+
pub file_missing: bool,
1451+
}
1452+
1453+
/// `reindex` IPC fast-mode adapter: runs [`reindex_file_in_place_core`] and
1454+
/// formats its telemetry as a JSON [`Response`].
1455+
fn reindex_file_in_place(
1456+
graph: &mut SymbolGraph,
1457+
path: &Path,
1458+
started: std::time::Instant,
1459+
) -> Response {
1460+
let t = reindex_file_in_place_core(graph, path);
1461+
Response {
1462+
ok: true,
1463+
body: serde_json::json!({
1464+
"reindexed": true,
1465+
"mode": "fast",
1466+
"file": path.display().to_string(),
1467+
"elapsed_ms": started.elapsed().as_millis() as u64,
1468+
"nodes_removed": t.nodes_removed,
1469+
"nodes_inserted": t.nodes_inserted,
1470+
"edges_removed_with_evidence": t.edges_removed_with_evidence,
1471+
"edges_inserted": t.edges_inserted,
1472+
"cross_file_edges_staled": t.cross_file_edges_staled,
1473+
"cross_file_edges_dropped": t.cross_file_edges_dropped,
1474+
"file_missing": t.file_missing,
1475+
"note": "cross_file_edges_staled counts re-link ATTEMPTS that succeeded at the graph level; dedup no-ops count once per attempt, not once per underlying edge. cross_file_edges_dropped = edges whose file-side endpoint no longer exists post-reindex.",
1476+
})
1477+
.to_string(),
1478+
error: None,
1479+
}
1480+
}
1481+
1482+
/// Surgical single-file reindex shared by the `reindex` IPC fast-path and the
1483+
/// daemon's on-demand heal loop. In order: remove the file's existing nodes
1484+
/// (which drops incident edges), re-extract the file, and re-link captured
1485+
/// cross-file edges. Idempotent on an unchanged file — re-applying it nets zero
1486+
/// node change — so the heal loop can re-run it on a quiescent tree without
1487+
/// duplicating symbols. (The heal loop previously re-extracted additively,
1488+
/// WITHOUT this REMOVE phase, leaking a fresh duplicate of every symbol in the
1489+
/// healed file on each pass — `insert_node` always allocates a new id — which
1490+
/// inflated the daemon's live symbol count and orphan report relative to a
1491+
/// clean in-process build.)
1492+
///
1493+
/// Removes the file's old nodes (which drops all incident edges via petgraph),
1494+
/// then re-extracts into the live graph.
14441495
///
14451496
/// Telemetry note: `edges_removed_with_evidence` reports only edges whose
14461497
/// `evidence_file` matched the reindexed path. `remove_node` additionally
@@ -1467,11 +1518,10 @@ pub fn dispatch_reindex_mutable(
14671518
/// counter, even though the graph holds a single edge. This overcount is
14681519
/// accepted as-is; tighten the dedup accounting only if it becomes
14691520
/// measurable in practice.
1470-
fn reindex_file_in_place(
1521+
pub(crate) fn reindex_file_in_place_core(
14711522
graph: &mut SymbolGraph,
14721523
path: &Path,
1473-
started: std::time::Instant,
1474-
) -> Response {
1524+
) -> FileReindexTelemetry {
14751525
use coregraph_core::DirectEdge;
14761526

14771527
// Read source; if the file is gone, treat as deletion (remove only).
@@ -1683,24 +1733,14 @@ fn reindex_file_in_place(
16831733

16841734
graph.mark_fast_update(path);
16851735

1686-
Response {
1687-
ok: true,
1688-
body: serde_json::json!({
1689-
"reindexed": true,
1690-
"mode": "fast",
1691-
"file": path.display().to_string(),
1692-
"elapsed_ms": started.elapsed().as_millis() as u64,
1693-
"nodes_removed": nodes_removed,
1694-
"nodes_inserted": nodes_inserted,
1695-
"edges_removed_with_evidence": edges_removed_with_evidence,
1696-
"edges_inserted": edges_inserted,
1697-
"cross_file_edges_staled": cross_file_edges_staled,
1698-
"cross_file_edges_dropped": cross_file_edges_dropped,
1699-
"file_missing": source.is_none(),
1700-
"note": "cross_file_edges_staled counts re-link ATTEMPTS that succeeded at the graph level; dedup no-ops count once per attempt, not once per underlying edge. cross_file_edges_dropped = edges whose file-side endpoint no longer exists post-reindex.",
1701-
})
1702-
.to_string(),
1703-
error: None,
1736+
FileReindexTelemetry {
1737+
nodes_removed,
1738+
nodes_inserted,
1739+
edges_removed_with_evidence,
1740+
edges_inserted,
1741+
cross_file_edges_staled,
1742+
cross_file_edges_dropped,
1743+
file_missing: source.is_none(),
17041744
}
17051745
}
17061746

@@ -3815,6 +3855,83 @@ mod tests {
38153855
);
38163856
}
38173857

3858+
/// Regression guard for the daemon heal-loop symbol-duplication bug.
3859+
///
3860+
/// The on-demand heal path re-runs `reindex_file_in_place_core` on every
3861+
/// file `filter_real_changes` reports. That primitive MUST be idempotent on
3862+
/// an unchanged file — REMOVE the file's nodes, then re-extract — so a heal
3863+
/// over a quiescent tree nets zero node change. The heal loop previously used
3864+
/// a bare additive `extractor.extract`; because `insert_node` always
3865+
/// allocates a fresh id and never removes the old nodes, every heal
3866+
/// duplicated the file's symbols, leaking edge-less phantom orphans that
3867+
/// inflated the daemon's live symbol count and orphan report relative to a
3868+
/// clean in-process build. This test pins the idempotency invariant and
3869+
/// contrasts it with the additive path that caused the leak.
3870+
#[test]
3871+
fn reindex_file_in_place_core_is_idempotent_on_unchanged_file() {
3872+
use std::time::{SystemTime, UNIX_EPOCH};
3873+
3874+
let nonce = SystemTime::now()
3875+
.duration_since(UNIX_EPOCH)
3876+
.unwrap()
3877+
.subsec_nanos();
3878+
let temp_dir =
3879+
std::env::temp_dir().join(format!("cg-heal-idem-{}-{}", std::process::id(), nonce));
3880+
std::fs::create_dir_all(&temp_dir).unwrap();
3881+
let file = temp_dir.join("m.rs");
3882+
std::fs::write(&file, "pub fn a() {}\npub fn b() {}\npub struct C;\n").unwrap();
3883+
3884+
let mut g = SymbolGraph::new();
3885+
3886+
// Initial index of the file (populate from empty).
3887+
let first = super::reindex_file_in_place_core(&mut g, &file);
3888+
assert!(
3889+
first.nodes_inserted >= 3,
3890+
"extractor should produce the file's symbols (got {})",
3891+
first.nodes_inserted
3892+
);
3893+
let nodes_after_first = g.node_count();
3894+
let edges_after_first = g.edge_count();
3895+
3896+
// Re-run on the UNCHANGED file — exactly what the heal loop does on a
3897+
// quiescent tree. The REMOVE phase must drop the same nodes it
3898+
// re-extracts, so the counts stay put.
3899+
let second = super::reindex_file_in_place_core(&mut g, &file);
3900+
assert_eq!(
3901+
second.nodes_removed, first.nodes_inserted,
3902+
"second pass must REMOVE exactly the nodes the first pass inserted"
3903+
);
3904+
assert_eq!(
3905+
g.node_count(),
3906+
nodes_after_first,
3907+
"node_count must be stable across a heal of an unchanged file (no duplication)"
3908+
);
3909+
assert_eq!(
3910+
g.edge_count(),
3911+
edges_after_first,
3912+
"edge_count must be stable across a heal of an unchanged file"
3913+
);
3914+
3915+
// Contrast: the additive `extractor.extract` the heal loop used before
3916+
// the fix DOES duplicate the file's symbols (fresh ids, no removal).
3917+
// This is the leak the surgical reindex above prevents.
3918+
let source = std::fs::read_to_string(&file).unwrap();
3919+
for extractor in coregraph_extractor::all_extractors() {
3920+
if coregraph_extractor::scanner::extension_matches(&file, extractor.file_extensions()) {
3921+
let _ = extractor.extract(&file, &source, &mut g);
3922+
break;
3923+
}
3924+
}
3925+
let _ = std::fs::remove_dir_all(&temp_dir);
3926+
assert!(
3927+
g.node_count() > nodes_after_first,
3928+
"sanity: a bare additive extract DOES duplicate (the bug the surgical \
3929+
reindex fixes) — node_count {} should exceed {}",
3930+
g.node_count(),
3931+
nodes_after_first
3932+
);
3933+
}
3934+
38183935
/// Verify that when the callee symbol is absent from the rewritten file,
38193936
/// the edge is dropped and reported in cross_file_edges_dropped.
38203937
#[test]

0 commit comments

Comments
 (0)