feat(trogon-decider-runtime): add snapshot execution#172
Merged
Conversation
yordis
commented
May 20, 2026
yordis
commented
Member
- Runtime consumers need a stable snapshot boundary so long-lived streams can avoid full replay without coupling to a storage backend.
- Snapshot failures need to stay phase-specific so applications can retry infrastructure paths separately from domain rejections.
PR SummaryMedium Risk Overview Extends Introduces snapshot task scheduling ( Reviewed by Cursor Bugbot for commit a71d254. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
WalkthroughThis PR adds comprehensive snapshot support to the trogon-decider-runtime crate, enabling command execution to optionally read snapshots, replay only event deltas, and persist new snapshots via pluggable policies and async task scheduling, with new error variants for snapshot ordering validation. ChangesSnapshot-Aware Command Execution
Sequence DiagramsequenceDiagram
participant Client as Client
participant Exec as CommandExecution
participant SnapRead as SnapshotRead
participant StreamRead as StreamRead
participant Decider as Decider
participant StreamAppend as StreamAppend
participant Policy as SnapshotPolicy
participant Scheduler as SnapshotTaskScheduler
Client->>Exec: execute()
Exec->>SnapRead: read_snapshot(stream_id)
SnapRead-->>Exec: Snapshot or empty
Exec->>StreamRead: read from position (or beginning)
StreamRead-->>Exec: event delta
Exec->>Decider: decide(state from snapshot+delta)
Decider-->>Exec: emitted events
Exec->>StreamAppend: append events with preconditions
StreamAppend-->>Exec: stream position
Exec->>Policy: decide_snapshot(state, events, position)
Policy-->>Exec: Skip or Take
Exec->>Scheduler: schedule async snapshot write (best-effort)
Scheduler-->>Exec: fire-and-forget (log failures)
Exec-->>Client: CommandResult
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 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)
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 |
|
Code Coverage SummaryDetailsDiff against mainResults for commit: a71d254 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
yordis
force-pushed
the
yordis/chore-create-pr-v3
branch
from
a2f2a15 to
32b38a8
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
rsworkspace/crates/trogon-decider-runtime/src/snapshot/snapshot_type.rs (1)
1-2: 🏗️ Heavy liftUse a typed prefix value object instead of raw
&'static str.This contract allows invalid prefixes (e.g., empty/unsupported format). Expose a dedicated prefix type with validated construction so invalid values are unrepresentable at the API boundary.
As per coding guidelines, "Prefer domain-specific value objects over primitives (e.g.,
AcpPrefixinstead ofString). Each type's factory must guarantee correctness at construction—invalid instances should be unrepresentable."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rsworkspace/crates/trogon-decider-runtime/src/snapshot/snapshot_type.rs` around lines 1 - 2, Replace the raw &'static str constant with a domain value object: add a new SnapshotStreamPrefix type (newtype struct with a private field and a public validated constructor like SnapshotStreamPrefix::try_from(&str) -> Result<Self, PrefixError>) that enforces non-empty/format rules at construction, then update the SnapshotType trait to expose that type (e.g., fn snapshot_stream_prefix() -> SnapshotStreamPrefix or an associated constant of type SnapshotStreamPrefix) instead of const SNAPSHOT_STREAM_PREFIX: &'static str so implementors must produce a validated SnapshotStreamPrefix instance; update all implementors to construct the prefix via the validated constructor and remove direct use of raw &'static str.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rsworkspace/crates/trogon-decider-runtime/README.md`:
- Line 70: Update the awkward sentence in the README that currently reads "Use
`NoSnapshot` when a caller wants snapshot reads" to clearer phrasing; locate the
sentence near the phrase "events since the loaded snapshot" and change it to:
"Use `NoSnapshot` when a caller wants to read snapshots without writes." Ensure
the `NoSnapshot` symbol remains inline code.
In
`@rsworkspace/crates/trogon-decider-runtime/src/snapshot/codec/snapshot_payload_decode.rs`:
- Around line 14-16: The associated type Error on SnapshotPayloadDecode is
unconstrained; add trait bounds to make it a proper error type (e.g. Error:
std::error::Error + Send + Sync + 'static) and update the trait signature
accordingly so decode returns Result<Self, Self::Error> with a typed error;
apply the identical bound change to the SnapshotPayloadEncode trait's Error
associated type so both codec traits match the project’s other codec/stream
trait bounds (use the same Error: std::error::Error + Send + Sync + 'static
constraint and adjust any impls as needed).
In
`@rsworkspace/crates/trogon-decider-runtime/src/snapshot/codec/snapshot_payload_encode.rs`:
- Around line 2-4: Constrain the associated Error types to match other infra
traits by changing the trait declarations so the associated type has the
std::error::Error + Send + Sync + 'static bounds; specifically update
SnapshotPayloadEncode to declare "type Error: std::error::Error + Send + Sync +
'static;" (and likewise update SnapshotPayloadDecode's Error associated type if
present) so implementations like TestPayload (which uses serde_json::Error)
satisfy the crate-wide error bounds.
In `@rsworkspace/crates/trogon-decider-runtime/src/snapshot/read_snapshot.rs`:
- Around line 20-25: The associated type SnapshotRead::Error must be constrained
to enforce proper typed errors; update the trait declaration for SnapshotRead so
Error has bounds like implementing std::error::Error (and typically Debug), and
being Send + Sync + 'static to ensure it can be propagated across async
boundaries; adjust the trait signature around type Error and ensure the
read_snapshot return type stays the same (refer to SnapshotRead::Error,
read_snapshot, ReadSnapshotRequest, and ReadSnapshotResponse<SnapshotPayload>)
so implementations cannot use plain primitives or String and must supply
concrete error structs/enums.
In `@rsworkspace/crates/trogon-decider-runtime/src/snapshot/write_snapshot.rs`:
- Around line 13-18: Add concrete trait bounds to the associated Error type on
the SnapshotWrite and SnapshotRead traits so implementations must provide a
typed, sendable error; specifically update the associated type declarations
(SnapshotWrite::Error and SnapshotRead::Error) to require something like
std::error::Error + Send + Sync + 'static (or your crate's preferred error trait
bounds) and ensure any method signatures that reference Error (e.g.,
write_snapshot returning Result<WriteSnapshotResponse, Self::Error>) continue to
compile with those bounds.
---
Nitpick comments:
In `@rsworkspace/crates/trogon-decider-runtime/src/snapshot/snapshot_type.rs`:
- Around line 1-2: Replace the raw &'static str constant with a domain value
object: add a new SnapshotStreamPrefix type (newtype struct with a private field
and a public validated constructor like SnapshotStreamPrefix::try_from(&str) ->
Result<Self, PrefixError>) that enforces non-empty/format rules at construction,
then update the SnapshotType trait to expose that type (e.g., fn
snapshot_stream_prefix() -> SnapshotStreamPrefix or an associated constant of
type SnapshotStreamPrefix) instead of const SNAPSHOT_STREAM_PREFIX: &'static str
so implementors must produce a validated SnapshotStreamPrefix instance; update
all implementors to construct the prefix via the validated constructor and
remove direct use of raw &'static str.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 243ddf1a-7a85-4c09-b954-e9f083c5984f
📒 Files selected for processing (15)
rsworkspace/crates/trogon-decider-runtime/README.mdrsworkspace/crates/trogon-decider-runtime/src/execution.rsrsworkspace/crates/trogon-decider-runtime/src/lib.rsrsworkspace/crates/trogon-decider-runtime/src/snapshot/codec/encoded_snapshot.rsrsworkspace/crates/trogon-decider-runtime/src/snapshot/codec/mod.rsrsworkspace/crates/trogon-decider-runtime/src/snapshot/codec/snapshot_decode_error.rsrsworkspace/crates/trogon-decider-runtime/src/snapshot/codec/snapshot_encode_error.rsrsworkspace/crates/trogon-decider-runtime/src/snapshot/codec/snapshot_envelope_decode_error.rsrsworkspace/crates/trogon-decider-runtime/src/snapshot/codec/snapshot_envelope_encode_error.rsrsworkspace/crates/trogon-decider-runtime/src/snapshot/codec/snapshot_payload_decode.rsrsworkspace/crates/trogon-decider-runtime/src/snapshot/codec/snapshot_payload_encode.rsrsworkspace/crates/trogon-decider-runtime/src/snapshot/mod.rsrsworkspace/crates/trogon-decider-runtime/src/snapshot/read_snapshot.rsrsworkspace/crates/trogon-decider-runtime/src/snapshot/snapshot_type.rsrsworkspace/crates/trogon-decider-runtime/src/snapshot/write_snapshot.rs
yordis
force-pushed
the
yordis/chore-create-pr-v3
branch
2 times, most recently
from
aefc5a1 to
7e8094b
Compare
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
yordis
force-pushed
the
yordis/chore-create-pr-v3
branch
from
7e8094b to
a71d254
Compare
This was referenced May 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.