diff --git a/crates/tracedecay-domain/src/code_intelligence/graph.rs b/crates/tracedecay-domain/src/code_intelligence/graph.rs index d75c556f0..e2c4c4530 100644 --- a/crates/tracedecay-domain/src/code_intelligence/graph.rs +++ b/crates/tracedecay-domain/src/code_intelligence/graph.rs @@ -896,11 +896,19 @@ pub struct CodeBlock { /// /// The ID format is `"kind:32hexchars"` where the hex portion is the first 32 /// characters of the SHA-256 hash of the input components. +/// +/// `name` is extractor output derived from arbitrary user source, not a +/// programmer-supplied invariant, so an empty name is accepted input rather +/// than a contract violation: legal constructs such as `test.describe("", …)`, +/// `export default () => {}`, and IIFEs are genuinely unnamed. Identity does +/// not depend on the name being non-empty — file path, kind, and line already +/// separate two anonymous constructs — so an empty name hashes like any other +/// and behaves identically in debug and release builds. A `debug_assert!` here +/// previously panicked the indexing pool on such files in debug/perf profiles +/// while release silently produced this same id; callers that want a readable +/// display name must synthesize one (see the `` convention used +/// across the extractors) instead of relying on an assertion here. pub fn generate_node_id(file_path: &str, kind: &NodeKind, name: &str, line: u32) -> String { - debug_assert!( - !name.is_empty(), - "generate_node_id called with empty name for {file_path}:{line}" - ); let input = format!("{}:{}:{}:{}", file_path, kind.as_str(), name, line); let mut hasher = Sha256::new(); hasher.update(input.as_bytes()); @@ -924,3 +932,43 @@ pub struct ResolvedRef { pub confidence: f64, pub resolved_by: String, } + +#[cfg(test)] +mod empty_name_node_id_tests { + use super::{NodeKind, generate_node_id}; + + /// Real source contains anonymous constructs (`test.describe("", …)`, + /// `export default () => {}`). Extracted names are parser output, not a + /// programmer contract, so an empty name must produce an id in every + /// build profile instead of tripping a debug-only assertion. + #[test] + fn empty_name_yields_a_deterministic_id_in_every_profile() { + let first = generate_node_id( + "integration/fs-routes-test.ts", + &NodeKind::Function, + "", + 286, + ); + let second = generate_node_id( + "integration/fs-routes-test.ts", + &NodeKind::Function, + "", + 286, + ); + assert_eq!(first, second, "empty-name ids must be deterministic"); + assert!( + first.starts_with("function:"), + "unexpected id shape: {first}" + ); + } + + /// Uniqueness of an empty-name id comes from file path, kind, and line, + /// so distinct anonymous constructs never collide onto one node. + #[test] + fn empty_name_ids_stay_distinct_per_file_kind_and_line() { + let base = generate_node_id("a.ts", &NodeKind::Function, "", 286); + assert_ne!(base, generate_node_id("b.ts", &NodeKind::Function, "", 286)); + assert_ne!(base, generate_node_id("a.ts", &NodeKind::Class, "", 286)); + assert_ne!(base, generate_node_id("a.ts", &NodeKind::Function, "", 287)); + } +} diff --git a/src/daemon/code_index_scheduler.rs b/src/daemon/code_index_scheduler.rs index 97158bf3a..130402aa5 100644 --- a/src/daemon/code_index_scheduler.rs +++ b/src/daemon/code_index_scheduler.rs @@ -4114,6 +4114,7 @@ pub(in crate::daemon) mod observability; mod privacy; pub(in crate::daemon) mod queries; pub(in crate::daemon) mod query_runtime; +mod reconcile_panic_guard; mod registry; pub(crate) mod semantic_query_runtime; pub(crate) mod semantic_vector_graph; diff --git a/src/daemon/code_index_scheduler/reconcile_panic_guard.rs b/src/daemon/code_index_scheduler/reconcile_panic_guard.rs new file mode 100644 index 000000000..b1a754f26 --- /dev/null +++ b/src/daemon/code_index_scheduler/reconcile_panic_guard.rs @@ -0,0 +1,247 @@ +//! Bounded retry policy for a background reconcile pass whose blocking task +//! panicked. +//! +//! A reconcile fans per-file work across the indexing pool over arbitrary user +//! source. A panic there aborts the pass and surfaces as an opaque `JoinError` +//! on the worker loop. The loop then restored the pending arrival and waited +//! for the next wake — and because the offending input is still on disk, the +//! next wake reproduced the identical panic. One malformed file therefore +//! blocked a project's entire code index indefinitely, retrying forever with +//! no backoff and no terminal state. +//! +//! This policy makes that failure degrade instead of loop: repeated panics +//! back off exponentially, and after a bounded number of consecutive panics +//! the worker stops re-attempting until the input actually changes (the +//! code-index control epoch advances) or a pass makes progress. The shape +//! mirrors the sealed-generation activation backoff already used by the +//! registry worker; tests shrink the clock, not the shape. + +use std::time::Duration; + +use tokio::time::Instant; + +/// Bounded exponential backoff between reconcile retries after a panicking +/// pass. The floor keeps a transient panic from hot-looping the pool; the +/// ceiling keeps a persistently panicking input from being retried more than a +/// few times an hour. +pub(super) const RECONCILE_PANIC_BACKOFF_FLOOR: Duration = if cfg!(test) { + Duration::from_millis(50) +} else { + Duration::from_secs(30) +}; +pub(super) const RECONCILE_PANIC_BACKOFF_CEILING: Duration = if cfg!(test) { + Duration::from_millis(400) +} else { + Duration::from_mins(10) +}; + +/// Consecutive panics over unchanged input after which retrying is pointless: +/// the same bytes reproduce the same panic. Further attempts are suppressed +/// until the input changes. +pub(super) const MAX_CONSECUTIVE_RECONCILE_PANICS_V1: u32 = 4; + +/// What the worker loop should do after a reconcile pass panicked. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(super) enum ReconcilePanicDecisionV1 { + /// Re-arm a wake after this delay and try the same input again. + RetryAfter(Duration), + /// The bound is exhausted. Stop re-arming; only changed input or a + /// progressing pass resumes reconciles. + Quarantine, +} + +/// Consecutive-panic accounting for one mounted worktree's worker loop. +#[derive(Debug)] +pub(super) struct ReconcilePanicGuardV1 { + consecutive_panics: u32, + backoff: Duration, + next_attempt_at: Option, + quarantined: bool, + /// Control epoch observed when the guard last quarantined. An advance + /// means new input, which is worth one more attempt. + quarantined_at_epoch: u64, +} + +impl Default for ReconcilePanicGuardV1 { + fn default() -> Self { + Self::new() + } +} + +impl ReconcilePanicGuardV1 { + pub(super) const fn new() -> Self { + Self { + consecutive_panics: 0, + backoff: RECONCILE_PANIC_BACKOFF_FLOOR, + next_attempt_at: None, + quarantined: false, + quarantined_at_epoch: 0, + } + } + + /// Any pass that completed without panicking clears the accounting: the + /// next panic starts again at the floor. + pub(super) fn record_progress(&mut self) { + self.consecutive_panics = 0; + self.backoff = RECONCILE_PANIC_BACKOFF_FLOOR; + self.next_attempt_at = None; + self.quarantined = false; + } + + /// Record a panicking pass and decide whether the same input is worth + /// another attempt. + pub(super) fn record_panic(&mut self, now: Instant, epoch: u64) -> ReconcilePanicDecisionV1 { + self.consecutive_panics = self.consecutive_panics.saturating_add(1); + if self.consecutive_panics >= MAX_CONSECUTIVE_RECONCILE_PANICS_V1 { + self.quarantined = true; + self.quarantined_at_epoch = epoch; + self.next_attempt_at = None; + return ReconcilePanicDecisionV1::Quarantine; + } + let delay = self.backoff; + self.next_attempt_at = Some(now + delay); + self.backoff = self + .backoff + .saturating_mul(2) + .min(RECONCILE_PANIC_BACKOFF_CEILING); + ReconcilePanicDecisionV1::RetryAfter(delay) + } + + /// Consecutive panics observed since the last progressing pass. Reported + /// on the warn path so an operator sees a bounded counter rather than an + /// undifferentiated repeating line. + pub(super) const fn consecutive_panics(&self) -> u32 { + self.consecutive_panics + } + + /// True while this wake must be skipped: either the backoff window is + /// still open, or the guard is quarantined and the input has not changed. + pub(super) fn suppresses_pass(&mut self, now: Instant, epoch: u64) -> bool { + if self.quarantined { + if epoch != self.quarantined_at_epoch { + // New input is not the input that panicked; allow one attempt + // and let it re-quarantine if it panics again. + self.quarantined = false; + self.consecutive_panics = 0; + self.backoff = RECONCILE_PANIC_BACKOFF_FLOOR; + self.next_attempt_at = None; + return false; + } + return true; + } + self.next_attempt_at.is_some_and(|at| now < at) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn origin() -> Instant { + Instant::now() + } + + #[test] + fn repeated_panics_back_off_and_then_quarantine() { + let mut guard = ReconcilePanicGuardV1::new(); + let now = origin(); + + let mut delays = Vec::new(); + let mut decisions = Vec::new(); + for _ in 0..MAX_CONSECUTIVE_RECONCILE_PANICS_V1 { + let decision = guard.record_panic(now, 7); + decisions.push(decision); + if let ReconcilePanicDecisionV1::RetryAfter(delay) = decision { + delays.push(delay); + } + } + + assert_eq!( + decisions.last(), + Some(&ReconcilePanicDecisionV1::Quarantine), + "an unchanged panicking input must reach a terminal state, not retry forever" + ); + assert_eq!( + delays.len() as u32, + MAX_CONSECUTIVE_RECONCILE_PANICS_V1 - 1, + "every attempt before the bound is a delayed retry" + ); + assert!( + delays.windows(2).all(|pair| pair[1] > pair[0]), + "retries must back off, not repeat identically: {delays:?}" + ); + assert!( + delays + .iter() + .all(|delay| *delay >= RECONCILE_PANIC_BACKOFF_FLOOR + && *delay <= RECONCILE_PANIC_BACKOFF_CEILING), + "delays stay inside the bounded window: {delays:?}" + ); + } + + #[test] + fn a_quarantined_guard_suppresses_further_passes_over_unchanged_input() { + let mut guard = ReconcilePanicGuardV1::new(); + let now = origin(); + for _ in 0..MAX_CONSECUTIVE_RECONCILE_PANICS_V1 { + guard.record_panic(now, 7); + } + + let far_future = now + Duration::from_secs(86_400); + assert!( + guard.suppresses_pass(far_future, 7), + "unchanged input must stay quarantined however long the wait" + ); + } + + #[test] + fn changed_input_lifts_the_quarantine() { + let mut guard = ReconcilePanicGuardV1::new(); + let now = origin(); + for _ in 0..MAX_CONSECUTIVE_RECONCILE_PANICS_V1 { + guard.record_panic(now, 7); + } + assert!(guard.suppresses_pass(now, 7)); + + assert!( + !guard.suppresses_pass(now, 8), + "an advanced control epoch is new input and earns another attempt" + ); + assert_eq!(guard.consecutive_panics(), 0, "accounting restarts"); + } + + #[test] + fn the_backoff_window_suppresses_only_until_it_elapses() { + let mut guard = ReconcilePanicGuardV1::new(); + let now = origin(); + let ReconcilePanicDecisionV1::RetryAfter(delay) = guard.record_panic(now, 7) else { + panic!("the first panic must schedule a retry"); + }; + + assert!(guard.suppresses_pass(now, 7), "window is open"); + assert!( + !guard.suppresses_pass(now + delay, 7), + "the pass runs once the window elapses" + ); + } + + #[test] + fn a_progressing_pass_clears_the_accounting() { + let mut guard = ReconcilePanicGuardV1::new(); + let now = origin(); + for _ in 0..MAX_CONSECUTIVE_RECONCILE_PANICS_V1 { + guard.record_panic(now, 7); + } + assert!(guard.suppresses_pass(now, 7)); + + guard.record_progress(); + + assert!(!guard.suppresses_pass(now, 7), "progress lifts suppression"); + assert_eq!(guard.consecutive_panics(), 0); + assert_eq!( + guard.record_panic(now, 7), + ReconcilePanicDecisionV1::RetryAfter(RECONCILE_PANIC_BACKOFF_FLOOR), + "the next panic restarts at the floor" + ); + } +}