test(ledger): add reservation void-in-mutex and concurrency test suites (ENG-36)#104
Conversation
…es (ENG-36) Extends reservation.test.ts with a void-in-mutex scenario that verifies voiding one reservation correctly restores available balance for subsequent reservations. Creates concurrency.test.ts covering sequential transfer contention, double-mint prevention, gap-free sequence numbering, and supply invariant preservation across interleaved operations including mixed reservation lifecycle + direct transfers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @Connorbelez, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Warning Rate limit exceeded
⌛ 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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughTwo test suites added to validate concurrency and reservation scenarios in the Mortgage Ownership Ledger. The concurrency tests verify serialization, idempotency, supply invariants, and sequence number integrity under interleaved operations. The reservation test validates balance restoration when pending reservations are voided. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Greptile SummaryThis PR extends the ledger test suite with a void-in-mutex scenario in Key findings:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Test
participant Convex as convex-test harness
participant Ledger as Ledger Mutations
participant DB as Convex DB
Note over Test,DB: T-083b: Reservation lifecycle mixed with direct transfer
Test->>Convex: mintMortgage(m-inv-mixed)
Convex->>Ledger: mintMortgage
Ledger->>DB: INSERT TREASURY, POST MORTGAGE_MINTED
Test->>Convex: issueShares(A=6000, B=4000)
Convex->>Ledger: issueShares x2
Ledger->>DB: POST SHARES_ISSUED x2
Test->>Convex: t.run → reserveShares(A→C, 3000)
Convex->>Ledger: reserveShares._handler
Ledger->>DB: POST SHARES_RESERVED, PATCH A.pendingCredits+=3000
Test->>Convex: transferShares(B→A, 1000)
Convex->>Ledger: transferShares
Ledger->>DB: POST SHARES_TRANSFERRED
Test->>Convex: t.run → commitReservation(reservationId)
Convex->>Ledger: commitReservation._handler
Ledger->>DB: PATCH A.pendingCredits-=3000, POST SHARES_COMMITTED
Test->>Convex: validateSupplyInvariant(m-inv-mixed)
Convex-->>Test: {valid:true, total:10000n, treasuryBalance:0n}
Note over Test,DB: Expected: A=4000, B=3000, C=3000, Treasury=0
Last reviewed commit: b50e542 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds additional ledger reservation lifecycle coverage and introduces a new concurrency-oriented test suite to validate serialization/idempotency behavior and invariants in the Mortgage Ownership Ledger.
Changes:
- Extend
reservation.test.tswith a “void within mutex” scenario to ensure voiding a reservation releases available balance for subsequent reservations. - Add
concurrency.test.tsto cover sequential transfer contention, double-mint prevention, gap-free sequence numbering, and supply invariant checks across interleaved operations.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
convex/ledger/__tests__/reservation.test.ts |
Adds a void-reservation test to confirm pending locks are released and available balance is restored. |
convex/ledger/__tests__/concurrency.test.ts |
New suite validating sequentialized “would-be concurrent” behaviors, idempotency/serialization guards, sequence integrity, and supply invariants. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
convex/ledger/__tests__/concurrency.test.ts (1)
92-203: Consider extracting shared test utilities to reduce duplication.The
getConvexErrorCodefunction and typed wrappers (ReserveSharesMutation,CommitReservationMutation, etc.) are duplicated between this file andreservation.test.ts. Additionally, the implementations differ slightly —reservation.test.tsincludes a regex fallback for error code extraction that this version lacks.Extracting these to a shared module (e.g.,
__tests__/helpers.ts) would reduce maintenance burden and ensure consistent behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/ledger/__tests__/concurrency.test.ts` around lines 92 - 203, Extract the duplicated test utilities into a shared test helper module (e.g., __tests__/helpers.ts): move getConvexErrorCode (include the regex fallback present in reservation.test.ts), the typed types ReserveSharesArgs/ReserveSharesResult/ReserveSharesMutation and CommitReservationArgs/CommitReservationResult/CommitReservationMutation, the casts reserveSharesMutation and commitReservationMutation, and the executor helpers executeReserveShares and executeCommitReservation into that module; then import and use those symbols in both concurrency.test.ts and reservation.test.ts to replace the local duplicates so both tests share the exact same implementation and behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@convex/ledger/__tests__/concurrency.test.ts`:
- Around line 412-420: The inline comment above the redeemShares call in the
concurrency.test (the auth.mutation(api.ledger.mutations.redeemShares, {...})
invocation) incorrectly states "Redeem A 1,000" while the payload sets amount:
2_000; update that comment to reflect the actual amount (2,000) so the comment
matches the call and avoids confusion.
---
Nitpick comments:
In `@convex/ledger/__tests__/concurrency.test.ts`:
- Around line 92-203: Extract the duplicated test utilities into a shared test
helper module (e.g., __tests__/helpers.ts): move getConvexErrorCode (include the
regex fallback present in reservation.test.ts), the typed types
ReserveSharesArgs/ReserveSharesResult/ReserveSharesMutation and
CommitReservationArgs/CommitReservationResult/CommitReservationMutation, the
casts reserveSharesMutation and commitReservationMutation, and the executor
helpers executeReserveShares and executeCommitReservation into that module; then
import and use those symbols in both concurrency.test.ts and reservation.test.ts
to replace the local duplicates so both tests share the exact same
implementation and behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0a26517a-40cc-4cc3-a63e-172d936b913b
📒 Files selected for processing (2)
convex/ledger/__tests__/concurrency.test.tsconvex/ledger/__tests__/reservation.test.ts
…review feedback - Create testUtils.ts with shared helpers, typed mutation wrappers, and getConvexErrorCode (now includes regex string fallback for all error codes) - Update reservation.test.ts and concurrency.test.ts to import from testUtils - Fix misleading "Redeem A 1,000" comments (actual amount is 2,000) - Fix mintAndIssueMultiple JSDoc to reflect that validation is in mutations Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…es (ENG-36) (#104) ## Extends reservation testing and adds comprehensive concurrency test suite Extends reservation.test.ts with a void-in-mutex scenario that verifies voiding one reservation correctly restores available balance for subsequent reservations. Creates concurrency.test.ts covering sequential transfer contention, double-mint prevention, gap-free sequence numbering, and supply invariant preservation across interleaved operations including mixed reservation lifecycle + direct transfers. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Added comprehensive concurrency tests validating correct ledger behavior across concurrent transfers, idempotency, and sequence integrity. * Added reservation lifecycle tests ensuring proper balance restoration when reservations are voided. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Extends reservation testing and adds comprehensive concurrency test suite
Extends reservation.test.ts with a void-in-mutex scenario that verifies voiding one reservation correctly restores available balance for subsequent reservations. Creates concurrency.test.ts covering sequential transfer contention, double-mint prevention, gap-free sequence numbering, and supply invariant preservation across interleaved operations including mixed reservation lifecycle + direct transfers.
Summary by CodeRabbit