feat: microsvc context#42
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR comprehensively refactors the sourced_rust framework: the ChangesDependency-Centric Service and Read-Model Write-Plan Migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/commit_builder/mod.rs (1)
12-30:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRecreate the builder in each example snippet.
The first
repo.read_models(read_models)consumesread_models, so the later examples in this rustdoc block reuse a moved value. Anyone copying the example will hit a compile error immediately.🤖 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 `@src/commit_builder/mod.rs` around lines 12 - 30, The doc example reuses the moved variable read_models (constructed via ReadModelWritePlanBuilder::new()) across multiple repo.read_models(...) calls which consumes it; fix by recreating the builder before each example use (or use a Clone if implemented) so each snippet calls ReadModelWritePlanBuilder::new() to produce a fresh read_models prior to repo.read_models(...), repo.outbox(...).read_models(...).commit(...), and repo.aggregate(...).read_models(...).outbox(...).src/read_model/session.rs (1)
828-868:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRefresh or invalidate the tracked baseline after
sync().
sync()stages diffs against the original loaded baseline but leaves that baseline intact. If the same model is mutated andsync()is called again in the same workspace, the second patch is still emitted with the old expected version, so the batch can self-conflict when the first patch has already advanced the row version.At minimum, reject a second
sync()for the same tracked model until it is reloaded; ideally, update the tracked baseline/expected version once staging succeeds.🤖 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 `@src/read_model/session.rs` around lines 828 - 868, sync() currently stages diffs against an immutable tracked baseline (in ReadModel::sync) so repeated sync() calls for the same model use a stale baseline/version; update behavior to prevent self-conflicts by either rejecting a second sync for the same RowIdentity until the baseline is reloaded or (preferred) update the stored baseline entry in self.baselines after successful staging: after the calls to stage_row_diff and stage_include_changes complete, replace or refresh the matching baseline (the entry found via RowIdentity matching baseline.root_schema.table_name and key_fingerprint(&baseline.root_key)) with a new baseline that uses current_row (and its included rows), and set root_version to the new expected version (or mark it invalid if you choose the reject approach); adjust ReadModel::sync to perform this update using the same RowIdentity/baseline variables so subsequent sync() calls use the updated baseline/version.
🤖 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.
Outside diff comments:
In `@src/commit_builder/mod.rs`:
- Around line 12-30: The doc example reuses the moved variable read_models
(constructed via ReadModelWritePlanBuilder::new()) across multiple
repo.read_models(...) calls which consumes it; fix by recreating the builder
before each example use (or use a Clone if implemented) so each snippet calls
ReadModelWritePlanBuilder::new() to produce a fresh read_models prior to
repo.read_models(...), repo.outbox(...).read_models(...).commit(...), and
repo.aggregate(...).read_models(...).outbox(...).
In `@src/read_model/session.rs`:
- Around line 828-868: sync() currently stages diffs against an immutable
tracked baseline (in ReadModel::sync) so repeated sync() calls for the same
model use a stale baseline/version; update behavior to prevent self-conflicts by
either rejecting a second sync for the same RowIdentity until the baseline is
reloaded or (preferred) update the stored baseline entry in self.baselines after
successful staging: after the calls to stage_row_diff and stage_include_changes
complete, replace or refresh the matching baseline (the entry found via
RowIdentity matching baseline.root_schema.table_name and
key_fingerprint(&baseline.root_key)) with a new baseline that uses current_row
(and its included rows), and set root_version to the new expected version (or
mark it invalid if you choose the reject approach); adjust ReadModel::sync to
perform this update using the same RowIdentity/baseline variables so subsequent
sync() calls use the updated baseline/version.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 79292b23-f77d-43bb-8da8-3c6781f101f6
📒 Files selected for processing (53)
.github/workflows/on-pr-quality.yamlREADME.mddocs/async-repositories.mddocs/read-models.mdsrc/commit_builder/mod.rssrc/hashmap_repo/repository.rssrc/lib.rssrc/microsvc/context.rssrc/microsvc/dependencies.rssrc/microsvc/grpc.rssrc/microsvc/http.rssrc/microsvc/mod.rssrc/microsvc/service.rssrc/read_model/in_memory.rssrc/read_model/metadata.rssrc/read_model/mod.rssrc/read_model/queued.rssrc/read_model/session.rssrc/repository/async_repository.rssrc/repository/mod.rssrc/sqlite_repo/mod.rstests/async_repository/main.rstests/distributed_read_model/checkout_saga_service/service.rstests/distributed_read_model/main.rstests/distributed_read_model/projection_service/handlers/checkout.rstests/distributed_read_model/projection_service/handlers/mod.rstests/distributed_read_model/projection_service/handlers/seat.rstests/distributed_read_model/projection_service/mod.rstests/distributed_read_model/projection_service/service.rstests/distributed_read_model/query_service/mod.rstests/distributed_read_model/seat_inventory_service/service.rstests/distributed_read_model_board/board_service/service.rstests/distributed_read_model_board/main.rstests/distributed_read_model_board/projections_service/handlers/board.rstests/distributed_read_model_board/projections_service/handlers/mod.rstests/distributed_read_model_board/projections_service/mod.rstests/distributed_read_model_board/query_service/mod.rstests/microsvc/basic.rstests/microsvc/convention.rstests/microsvc/session.rstests/microsvc/transport_grpc.rstests/microsvc/transport_http.rstests/microsvc/transport_listen.rstests/microsvc/transport_subscribe.rstests/postgres_repository/main.rstests/read_model_commit_bridge/main.rstests/read_model_distributed_idempotency/main.rstests/read_model_document_conformance/main.rstests/read_model_relationship_includes/main.rstests/read_model_session/main.rstests/read_models/main.rstests/sagas/microsvc_saga.rstests/sqlite_repository/main.rs
💤 Files with no reviewable changes (1)
- .github/workflows/on-pr-quality.yaml
|
@coderabbitai addressed both outside-diff findings:
Validation:
|
|
Tip For best results, initiate chat on the files or code changes.
Full test suite passing confirms everything is in order. 🐇✨ |
Summary by CodeRabbit
New Features
HasRepoandHasReadModelStoretraits for cleaner handler context access.ReadModelWorkspaceAPI for streamlined read-model operations with improved method naming.Refactor
save()→upsert(),save_changes()→sync()for clarity.