Conversation
Extend the simulation harness with replayable seeded schedule actions for ingress, expiration, and retry interleavings. Add regression coverage and update the testing/status docs for M4-T04. Closes #19
Summary by CodeRabbitRelease Notes
WalkthroughAdds seeded, label-driven schedule exploration to the simulation harness and exposes engine expiration internals by widening visibility and moving per-tick expiration bounding into tick logic; introduces schedule action/observation types, deterministic schedule execution, and reproducible tests for ingress contention, expiration ordering, and retry timing. Changes
Sequence Diagram(s)sequenceDiagram
participant Harness as SimulationHarness
participant Engine as SingleNodeEngine
participant TickLogic as TickExpirationsLogic
participant Observation as ObservationCapture
Harness->>Harness: explore_schedule(actions)
loop each ScheduleAction
Harness->>Engine: resolve_schedule_slot(candidate_slots)
Engine-->>Harness: selected_slot
alt action is Submit
Harness->>Engine: submit ClientRequest at selected_slot
Engine-->>Observation: SimulationObservation (includes from_retry_cache)
Observation-->>Harness: ScheduleObservation(Submit)
else action is TickExpirations
Harness->>Engine: tick_expirations_seeded(request_slot)
Engine->>TickLogic: collect_due_expirations(request_slot)
TickLogic-->>Engine: Vec\<DueExpiration>
Engine->>TickLogic: process up to remaining_due (per-tick limit)
Engine-->>Observation: TickObservation (ExpirationObservations)
Observation-->>Harness: ScheduleObservation(Tick)
end
end
Harness->>Harness: produce transcript (seed + observations)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
crates/allocdb-node/src/simulation.rs (2)
212-229: Add explicit negative-path regression tests for schedule invariants.The new guards (unique labels, non-empty candidate slots) are good, but they should be pinned with dedicated failing-case tests to keep those invariants intentional.
As per coding guidelines "Write extensive tests for every meaningful behavior change. Favor invariant tests, negative-path tests, recovery tests, and regression tests over shallow happy-path coverage."
Also applies to: 386-390
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/allocdb-node/src/simulation.rs` around lines 212 - 229, Add explicit negative-path regression tests exercising the schedule invariants enforced in explore_schedule and resolve_schedule_slot: write tests that (1) submit actions with duplicate labels to ensure the unique-label assert triggers (use the same ScheduleAction.label twice) and (2) submit an action with empty candidate_slots to ensure resolve_schedule_slot handles/rejects empty candidates (test the path around resolve_schedule_slot called from explore_schedule). Place tests alongside existing simulation tests, use assert_panics or the test framework's equivalent to verify the failure modes, and name them to reflect the invariant they protect so future changes cannot regress these guards.
349-384: Consider unifying seeded tick execution with the engine tick primitive.
tick_expirations_seededre-implements queue drain + due collection + bounded internal apply logic. Extracting a shared engine-level primitive would reduce semantic drift risk as tick behavior evolves.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/allocdb-node/src/simulation.rs` around lines 349 - 384, tick_expirations_seeded duplicates engine tick behavior by manually draining process_next, collecting due expirations and applying up to max_expirations_per_tick; refactor by adding a new engine-level method (e.g., Engine::tick_with_seed or Engine::apply_due_expirations_for_slot) that encapsulates the sequence of process_next draining, expiration_request_slot lookup, collect_due_expirations, and bounded apply_due_expiration logic, returning the same data needed to build TickObservation (processed count, last_applied_lsn, and applied expirations); then replace the body of tick_expirations_seeded to call that new Engine method (using engine_mut/engine_ref) and translate its return into TickObservation, preserving use of max_expirations_per_tick, expiration_request_slot, collect_due_expirations, and apply_due_expiration to avoid semantic drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/allocdb-node/src/simulation.rs`:
- Around line 212-229: Add explicit negative-path regression tests exercising
the schedule invariants enforced in explore_schedule and resolve_schedule_slot:
write tests that (1) submit actions with duplicate labels to ensure the
unique-label assert triggers (use the same ScheduleAction.label twice) and (2)
submit an action with empty candidate_slots to ensure resolve_schedule_slot
handles/rejects empty candidates (test the path around resolve_schedule_slot
called from explore_schedule). Place tests alongside existing simulation tests,
use assert_panics or the test framework's equivalent to verify the failure
modes, and name them to reflect the invariant they protect so future changes
cannot regress these guards.
- Around line 349-384: tick_expirations_seeded duplicates engine tick behavior
by manually draining process_next, collecting due expirations and applying up to
max_expirations_per_tick; refactor by adding a new engine-level method (e.g.,
Engine::tick_with_seed or Engine::apply_due_expirations_for_slot) that
encapsulates the sequence of process_next draining, expiration_request_slot
lookup, collect_due_expirations, and bounded apply_due_expiration logic,
returning the same data needed to build TickObservation (processed count,
last_applied_lsn, and applied expirations); then replace the body of
tick_expirations_seeded to call that new Engine method (using
engine_mut/engine_ref) and translate its return into TickObservation, preserving
use of max_expirations_per_tick, expiration_request_slot,
collect_due_expirations, and apply_due_expiration to avoid semantic drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 119af9ff-b6e5-4764-bc94-652dacc5c36a
📒 Files selected for processing (5)
crates/allocdb-node/src/engine.rscrates/allocdb-node/src/simulation.rscrates/allocdb-node/src/simulation_tests.rsdocs/status.mddocs/testing.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Keep documentation up to date with the code and design. If a change affects behavior, invariants, failure modes, operational semantics, testing strategy, or implementation sequencing, update the relevant docs in the same task or PR.
Files:
docs/testing.mddocs/status.md
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Write extensive tests for every meaningful behavior change. Favor invariant tests, negative-path tests, recovery tests, and regression tests over shallow happy-path coverage.
Add extensive logging where it materially improves debuggability or operational clarity. Use the right log level:errorfor invariant breaks, corruption, and failed operations that require intervention;warnfor degraded but expected conditions such as overload, lag, or rejected requests;infofor meaningful lifecycle and state-transition events;debugfor detailed execution traces useful in development;traceonly for very high-volume diagnostic detail.
Logging must be structured and purposeful. Do not add noisy logs that obscure signal or hide bugs.
Files:
crates/allocdb-node/src/simulation_tests.rscrates/allocdb-node/src/engine.rscrates/allocdb-node/src/simulation.rs
docs/status.md
📄 CodeRabbit inference engine (AGENTS.md)
Keep
docs/status.mdcurrent as the single-file progress snapshot for the repository. Update it whenever milestone state, implementation coverage, or the recommended next step materially changes.
Files:
docs/status.md
🧠 Learnings (3)
📚 Learning: 2026-03-12T15:18:53.046Z
Learnt from: CR
Repo: skel84/allocdb PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-12T15:18:53.046Z
Learning: Applies to **/*.md : Keep documentation up to date with the code and design. If a change affects behavior, invariants, failure modes, operational semantics, testing strategy, or implementation sequencing, update the relevant docs in the same task or PR.
Applied to files:
docs/testing.md
📚 Learning: 2026-03-12T15:18:53.046Z
Learnt from: CR
Repo: skel84/allocdb PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-12T15:18:53.046Z
Learning: Applies to **/*.rs : Write extensive tests for every meaningful behavior change. Favor invariant tests, negative-path tests, recovery tests, and regression tests over shallow happy-path coverage.
Applied to files:
docs/status.md
📚 Learning: 2026-03-12T15:18:53.046Z
Learnt from: CR
Repo: skel84/allocdb PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-12T15:18:53.046Z
Learning: Applies to docs/status.md : Keep [`docs/status.md`](./docs/status.md) current as the single-file progress snapshot for the repository. Update it whenever milestone state, implementation coverage, or the recommended next step materially changes.
Applied to files:
docs/status.md
🧬 Code graph analysis (1)
crates/allocdb-node/src/engine.rs (1)
crates/allocdb-core/src/state_machine_metrics.rs (2)
last_applied_lsn(21-23)last_request_slot(26-28)
🔇 Additional comments (6)
crates/allocdb-node/src/simulation_tests.rs (2)
64-136: Clean schedule helper surface for deterministic test setup.The
schedule_submit/schedule_tick/run_schedule/ observation helpers make the seeded scenarios compact and reproducible without obscuring intent.
271-435: Strong reproducibility assertions for the new schedule exploration paths.These tests directly validate same-seed replay equality and cross-seed divergence for ingress contention, expiration ordering, and retry timing, which lines up well with the PR objective.
docs/status.md (1)
97-123: Status snapshot is well-aligned with M4-T04 scope.The deterministic simulation and current-focus sections now accurately capture seeded schedule exploration and the new regression areas.
crates/allocdb-node/src/engine.rs (2)
626-633: Per-tick expiration bounding is explicit and easy to reason about.This makes the cap enforcement local to the processing loop and supports deterministic tick behavior.
804-863: Good testability seam for due-expiration exploration.Exposing due-expiration collection/request-slot/application as crate-visible helpers provides a clear bridge for simulation-driven schedule testing over the real engine path.
docs/testing.md (1)
108-113: Testing strategy updates are aligned with the new seeded schedule model.The added guidance around labeled schedule actions, replayable transcripts, and non-promoted opaque random loops is clear and consistent with the code changes.
Also applies to: 120-122, 129-129
Merge origin/main into the issue branch, keep the storage-fault and seeded schedule simulation coverage together, and add the schedule invariant regression tests raised in CodeRabbit. Validation rerun: - cargo test -p allocdb-node simulation -- --nocapture - cargo test -p allocdb-node engine -- --nocapture - ./scripts/preflight.sh
|
Addressed the substantive CodeRabbit feedback in
Validation rerun:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/allocdb-node/src/simulation.rs`:
- Around line 372-399: tick_expirations_seeded currently picks uniformly from
the whole due list which allows later deadlines to leapfrog earlier ones; change
the selection so randomness is restricted to the earliest-deadline cohort: while
expirations.len() < limit and due not empty, determine the smallest
deadline_slot present (peek due[0].deadline_slot or scan), collect the subrange
of due with that same deadline_slot, call slot_driver.choose_index with that
cohort length and remove the chosen element from the overall due list (not from
other deadlines), apply it with engine_mut().apply_due_expiration and push the
observation; only after that cohort is exhausted (or the tick budget is reached)
move on to the next deadline cohort—this preserves the (deadline_slot,
reservation_id) priority the real engine enforces (see tick_expirations_seeded,
collect_due_expirations, slot_driver.choose_index, apply_due_expiration, and
SingleNodeEngine::tick_expirations).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 95e9c214-969f-4116-a516-447b25ef7945
📒 Files selected for processing (5)
crates/allocdb-node/src/engine.rscrates/allocdb-node/src/simulation.rscrates/allocdb-node/src/simulation_tests.rsdocs/status.mddocs/testing.md
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/status.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Write extensive tests for every meaningful behavior change. Favor invariant tests, negative-path tests, recovery tests, and regression tests over shallow happy-path coverage.
Add extensive logging where it materially improves debuggability or operational clarity. Use the right log level:errorfor invariant breaks, corruption, and failed operations that require intervention;warnfor degraded but expected conditions such as overload, lag, or rejected requests;infofor meaningful lifecycle and state-transition events;debugfor detailed execution traces useful in development;traceonly for very high-volume diagnostic detail.
Logging must be structured and purposeful. Do not add noisy logs that obscure signal or hide bugs.
Files:
crates/allocdb-node/src/simulation_tests.rscrates/allocdb-node/src/simulation.rscrates/allocdb-node/src/engine.rs
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Keep documentation up to date with the code and design. If a change affects behavior, invariants, failure modes, operational semantics, testing strategy, or implementation sequencing, update the relevant docs in the same task or PR.
Files:
docs/testing.md
🧬 Code graph analysis (1)
crates/allocdb-node/src/engine.rs (1)
crates/allocdb-core/src/state_machine_metrics.rs (2)
last_applied_lsn(21-23)last_request_slot(26-28)
|
Addressed the remaining CodeRabbit finding in
Validation rerun:
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/allocdb-node/src/simulation.rs (1)
228-237: Validate all candidate slots against current slot before selection.Right now only the resolved slot is asserted, so a schedule containing past-slot candidates can pass or panic depending on seed. Validate the full candidate window up front to keep this invariant seed-independent.
Suggested patch
for action in actions { assert!( seen_labels.insert(action.label), "schedule actions must have unique labels" ); + assert!( + action + .candidate_slots + .iter() + .all(|candidate| *candidate >= current_slot), + "schedule action candidate slots must not be before the current simulated slot" + ); let slot = self.resolve_schedule_slot(&action.candidate_slots); - assert!( - slot >= current_slot, - "schedule actions must not resolve before the current simulated slot" - ); pending.push(ResolvedScheduleAction { label: action.label, slot, action: action.action, });Also applies to: 416-427
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/allocdb-node/src/simulation.rs` around lines 228 - 237, The loop that asserts the resolved slot currently only checks the chosen slot, so modify the validation in the schedule-processing loops that contain seen_labels.insert(...) and call to self.resolve_schedule_slot(&action.candidate_slots) (the loop starting around the seen_labels/assert block and the similar loop later around lines 416-427) to first verify every candidate in action.candidate_slots is >= current_slot before calling resolve_schedule_slot; if any candidate < current_slot, return/panic with a clear message (same style as existing asserts) so the invariant is enforced deterministically prior to selection.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/allocdb-node/src/simulation.rs`:
- Around line 228-237: The loop that asserts the resolved slot currently only
checks the chosen slot, so modify the validation in the schedule-processing
loops that contain seen_labels.insert(...) and call to
self.resolve_schedule_slot(&action.candidate_slots) (the loop starting around
the seen_labels/assert block and the similar loop later around lines 416-427) to
first verify every candidate in action.candidate_slots is >= current_slot before
calling resolve_schedule_slot; if any candidate < current_slot, return/panic
with a clear message (same style as existing asserts) so the invariant is
enforced deterministically prior to selection.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f0444b57-6fcf-4ace-b6d4-e7992fc0f139
📒 Files selected for processing (4)
crates/allocdb-node/src/simulation.rscrates/allocdb-node/src/simulation_tests.rsdocs/status.mddocs/testing.md
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/allocdb-node/src/simulation_tests.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Keep documentation up to date with the code and design. If a change affects behavior, invariants, failure modes, operational semantics, testing strategy, or implementation sequencing, update the relevant docs in the same task or PR.
Files:
docs/status.mddocs/testing.md
docs/status.md
📄 CodeRabbit inference engine (AGENTS.md)
Keep
docs/status.mdcurrent as the single-file progress snapshot for the repository. Update it whenever milestone state, implementation coverage, or the recommended next step materially changes.
Files:
docs/status.md
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Write extensive tests for every meaningful behavior change. Favor invariant tests, negative-path tests, recovery tests, and regression tests over shallow happy-path coverage.
Add extensive logging where it materially improves debuggability or operational clarity. Use the right log level:errorfor invariant breaks, corruption, and failed operations that require intervention;warnfor degraded but expected conditions such as overload, lag, or rejected requests;infofor meaningful lifecycle and state-transition events;debugfor detailed execution traces useful in development;traceonly for very high-volume diagnostic detail.
Logging must be structured and purposeful. Do not add noisy logs that obscure signal or hide bugs.
Files:
crates/allocdb-node/src/simulation.rs
🧠 Learnings (1)
📚 Learning: 2026-03-12T15:18:53.046Z
Learnt from: CR
Repo: skel84/allocdb PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-12T15:18:53.046Z
Learning: Applies to **/*.rs : Write extensive tests for every meaningful behavior change. Favor invariant tests, negative-path tests, recovery tests, and regression tests over shallow happy-path coverage.
Applied to files:
docs/status.md
🔇 Additional comments (4)
crates/allocdb-node/src/simulation.rs (2)
372-406: Earliest-deadline priority is preserved while exploring expiration order.The seeded selection constrained to the earliest-deadline cohort keeps schedule exploration aligned with production tick semantics under bounded throughput.
195-214:from_retry_cachepropagation in transcripts is a strong observability improvement.This makes retry-timing and dedupe-window regressions much easier to diagnose and replay.
Also applies to: 345-358
docs/testing.md (1)
108-113: Testing strategy doc is well-aligned with the seeded schedule model.The updated sections clearly capture replayability, bounded expiration behavior, and transcript-driven reproducibility.
Also applies to: 123-125, 134-135
docs/status.md (1)
31-36:docs/status.mdis updated in the right places for this PR.The snapshot now captures the seeded schedule exploration capabilities and corresponding regression coverage in a way that matches the implementation changes.
Also applies to: 99-113, 129-130
1 participant
Linked Issue
Summary
Changes
crates/allocdb-node/src/engine.rs: expose due-expiration seams needed by schedule exploration while keeping production tick bounding intactcrates/allocdb-node/src/simulation.rs: add labeled schedule actions, replayable transcripts, seeded expiration selection, and storage-fault injection helpers over the real engine pathcrates/allocdb-node/src/simulation_tests.rs: add reproducible regression cases for ingress contention, expiration order, retry timing, storage faults, and schedule invariant failuresdocs/testing.md: document the seeded schedule model and the storage-fault plus schedule-exploration evidencedocs/status.md: update the current validated chunk, simulation coverage snapshot, and M4-T04 focusDocs
docs/testing.mddocs/status.mdValidation
cargo test -p allocdb-node simulation -- --nocapturecargo test -p allocdb-node engine -- --nocapture./scripts/preflight.sh