docs + tests: 1.2 architecture README + batch-commit / N2N pipelining coverage#176
Merged
Conversation
Permanent coverage for the 1.2 release's new code: - AdaptivePipelineDepthTest (CI): pins N2NProvider.AdaptivePipelineDepth — collapses to 1 at the tip, grows with the tip gap, monotonic, clamped to the configured max (made internal for this). - ReducerGraphBatchCommitTest (CI): drives ReducerGraphProcessor directly against the committed TestData blocks with an in-memory unit of work, deterministic by pre-loading the inbox. Proves (a) a fault mid-batch rolls back the WHOLE open batch — no prior clean block survives; (b) a partial batch commits as soon as the inbox drains at the tip, without the size/delay triggers. - N2NPipeliningLiveTest (Integration, live preprod): pipelined catch-up pulls 150 blocks across >1 batch (strictly ascending, no gaps/dupes across batch boundaries); and the tip path — starting at the node's tip exercises MessageAwaitReply and follows new blocks in order without hanging. 22/22 unit tests pass; both live N2N tests pass against preprod. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The README described the pre-1.2 fork/pipeline model throughout. Updated to the shipped architecture: - ReducerPipeline -> ReducerGraphProcessor (one per root; the whole graph in topological order through one batched unit of work). - Per-branch -> whole-graph atomicity: one transaction per batch covers every reducer + checkpoint; a fault rolls back the whole open batch (stronger than the old per-branch fork, which let a committed sibling survive). No separate fork path. - Batch triggers (size / max-delay / drain-at-tip) and the per-batch durability/replay model documented. - New config keys: Sync:Commit:BatchSize (500), Sync:Commit:MaxDelayMs (1000), CardanoNodeConnection:TCP:PipelineDepth (100). - N2N noted as pipelined; features + testing blurb updated; a 1.1 -> 1.2 migration note (no code changes). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Updates Argus 1.2 documentation and adds test coverage around the shipped “whole-graph batch commit” architecture and N2N pipelining behavior.
Changes:
- Update README to describe
ReducerGraphProcessor, whole-graph batch atomicity, new batch triggers, and N2N pipelining + new config keys. - Expose
N2NProvider.AdaptivePipelineDepthasinternalfor unit testing and add a unit test that pins its mapping/clamping behavior. - Add CI unit tests for reducer-graph batch commit semantics and new integration coverage for multi-batch N2N pipelining + tip-following.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Argus.Sync/Providers/N2NProvider.cs | Makes AdaptivePipelineDepth internal for unit-test access. |
| src/Argus.Sync.Tests/Unit/ReducerGraphBatchCommitTest.cs | Adds CI unit tests for whole-batch rollback and drain-at-tip commits using an in-memory UoW. |
| src/Argus.Sync.Tests/Unit/AdaptivePipelineDepthTest.cs | Adds unit tests pinning the depth-vs-tip-gap mapping and clamping. |
| src/Argus.Sync.Tests/EndToEnd/N2NPipeliningLiveTest.cs | Adds live integration tests for pipelined multi-batch catch-up and tip-following behavior. |
| README.md | Updates architecture + config docs for 1.2 batched commits and N2N pipelining. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+28
to
+33
| IBlock[] blocks = LoadBlocks(3); | ||
| if (blocks.Length < 3) | ||
| { | ||
| _output.WriteLine("SKIP: TestData/Blocks not present."); | ||
| return; | ||
| } |
Comment on lines
+63
to
+68
| IBlock[] blocks = LoadBlocks(2); | ||
| if (blocks.Length < 2) | ||
| { | ||
| _output.WriteLine("SKIP: TestData/Blocks not present."); | ||
| return; | ||
| } |
| } | ||
| processor.Complete(); | ||
|
|
||
| _ = await Assert.ThrowsAnyAsync<Exception>(() => processor.RunAsync(CancellationToken.None)); |
Comment on lines
+155
to
+159
| public Task RollbackAsync(CancellationToken ct = default) | ||
| { | ||
| _store.Staged.Clear(); | ||
| return Task.CompletedTask; | ||
| } |
Comment on lines
+54
to
+60
| await foreach (NextResponse response in provider.StartChainSyncAsync(intersection, NetworkMagic, cts.Token)) | ||
| { | ||
| if (response.Action != NextResponseAction.RollForward) | ||
| { | ||
| continue; | ||
| } | ||
| IBlock block = response.Block!; |
Hand-authored, self-contained SVG — no scripts, external fonts, or <style> block, so it renders as a white card on GitHub light and dark, and diffs in PRs. Depicts the 1.2 flow: node -> CardanoIndexWorker -> one ReducerGraphProcessor per root running the root + its dependents (nested A->A1 and fan-out B) in topological order into one batched unit of work (size/delay/tip-drain triggers, whole-graph atomic) -> PostgreSQL or MongoDB. README image now points at the SVG; the old argus_architecture.png is unreferenced (left in place). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
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.
Docs + permanent tests for the 1.2 release (the batched-graph + N2N-pipelining work shipped in #175). For review — not merged.
Docs (README)
The README still described the pre-1.2 fork/pipeline model. Corrected to the shipped architecture:
ReducerPipeline→ReducerGraphProcessor(one per root; the whole graph in topological order through one batched unit of work).Sync:Commit:BatchSize(500),Sync:Commit:MaxDelayMs(1000),CardanoNodeConnection:TCP:PipelineDepth(100).Tests
AdaptivePipelineDepthTestReducerGraphBatchCommitTestN2NPipeliningLiveTestAwaitReply→ follow new blocks, in order, no hang)The two CI tests drive
ReducerGraphProcessordirectly against the 100 committedTestDatablocks with an in-memory unit of work — deterministic (the inbox is pre-loaded so the drain trigger is predictable) and free of any node or database.AdaptivePipelineDepthwas madeinternalso the unit test can call it.Validation:
dotnet formatclean, 22/22 non-integration tests, both live N2N tests pass against the local preprod node.Note
assets/argus_architecture.pngstill depicts the old pipeline — I can't regenerate the image, so the diagram is worth refreshing separately if you want it to match the prose.🤖 Generated with Claude Code