fix(cortex): harden startup warmup and bulletin coordination#248
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
WalkthroughAdds a bounded per-agent startup warmup pass and integrates warmup as the primary bulletin refresh when enabled; introduces guarded warmup-run lifecycle (WarmupRunGuard), lock-aware bulletin generation, updated bulletin/warmup loops, docs updates, and related tests. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main.rs (1)
1836-1836: Minor: Prefer.ok()overlet _ =for channel sends.Per coding guidelines, channel sends where the receiver may be dropped should use
.ok()rather thanlet _ =.🔧 Suggested change
- let _ = locked_tx.send(()); + locked_tx.send(()).ok();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.rs` at line 1836, Replace the discard pattern "let _ = locked_tx.send(())" with the idiomatic ".ok()" on the send call; specifically change the call site that invokes locked_tx.send(()) so it becomes locked_tx.send(()).ok() to explicitly ignore the Result when the receiver may have been dropped (refer to the locked_tx.send(()) usage).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/main.rs`:
- Line 1836: Replace the discard pattern "let _ = locked_tx.send(())" with the
idiomatic ".ok()" on the send call; specifically change the call site that
invokes locked_tx.send(()) so it becomes locked_tx.send(()).ok() to explicitly
ignore the Result when the receiver may have been dropped (refer to the
locked_tx.send(()) usage).
b4248bb to
b5a050e
Compare
Summary
JoinSetand enforce a bounded wait that aborts unfinished warmup tasks on timeoutrun_warmup_oncecancellation-safe with a guard that demotes stuckWarmingstate toDegradedwith actionable error contextWarm+ refresh timestamp for initial-pass completionbulletin_age_secs >= max(1, warmup.refresh_secs)Testing
cargo fmt --allcargo test -q startup_warmup_wait --bin spacebotcargo test -q startup_warmup_wait_timeout_stays_bounded_for_non_cooperative_task --bin spacebotcargo test -q cancelled_warmup_demotes_warming_state_to_degraded --libcargo test -q initial_warmup_completion_not_detected_when_timestamp_exists_but_state_is_not_warm --libcargo test -q bulletin_loop_generation_lock_snapshot_skips_after_fresh_update --libjust preflightjust gate-prNotes (Optional)
imap-proto v0.10.2; gates remain green.Note
Changes Summary
This PR hardens the startup warmup and memory bulletin coordination by introducing a bounded timeout for warmup tasks and state machine guards for cancellation safety. Key changes include moving from unbounded async drains to a
JoinSet-based approach with explicit abort on timeout, adding aWarmupRunGuardthat demotes incomplete warmup states to degraded with error context, gating initial warmup completion by both state and timestamp to prevent duplicate passes, and introducing a bulletin loop snapshot check to skip redundant synthesis when the cached bulletin is still fresh. Documentation updates clarify the relationship between warmup refresh cadence and bulletin staleness thresholds. Five new regression tests ensure timeout bounds hold even for non-cooperative tasks, cancellation safety, and initial completion gating logic.Written by Tembo for commit 3563f11