Merged
Conversation
![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
Contributor
|
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 (7)
📝 WalkthroughWalkthroughThis PR implements a corrective obligation feature triggered by payment reversals. It adds a mutation to create corrective obligations with validation and idempotency checks, wires the creation into the reversal cascade flow via scheduler, extends the schema with source-obligation indexing, and provides comprehensive test coverage for the new functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant PM as Payment Manager
participant EA as emitPaymentReversed
participant PRC as postPaymentReversalCascade
participant Scheduler as ctx.scheduler
participant COM as createCorrectiveObligation
participant ODB as Obligation DB
participant CL as Cash Ledger
PM->>EA: Trigger payment reversal
EA->>PRC: Call cascade for each obligation
PRC-->>EA: Return cascadeResult with reversedAmount
alt Obligation Status = "settled"
EA->>Scheduler: Schedule corrective creation (runAfter 0)
Scheduler->>COM: Invoke after 0ms
COM->>ODB: Load & validate original obligation
alt Original valid and settled
COM->>ODB: Check idempotency (by_type_and_source)
alt No existing corrective
COM->>ODB: Insert new obligation (status: upcoming)
ODB-->>COM: Return new obligationId
COM->>CL: Post OBLIGATION_ACCRUED entry
CL-->>COM: Ledger written
COM-->>COM: Return {obligationId, created: true}
else Existing corrective found
COM-->>COM: Return {obligationId, created: false}
end
else Validation failed
COM-->>COM: Throw validation error
end
else Obligation not settled
EA->>EA: Skip corrective scheduling
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
Owner
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Connorbelez has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
Pull request overview
Implements “corrective obligations” to restore domain-layer receivables after cash-ledger payment reversals, keeping the original (final-state) obligation settled while creating a new upcoming obligation linked via sourceObligationId.
Changes:
- Added
createCorrectiveObligationinternal mutation that creates and accrues a new obligation linked to a settled original, plus audit journaling/logging. - Wired
emitPaymentReversedto schedule corrective obligation creation afterpostPaymentReversalCascade. - Added
by_source_obligationindex and new internal queries to fetch correctives for an obligation, plus a dedicated test suite.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| specs/ENG-180/tasks.md | ENG-180 execution plan/task checklist. |
| specs/ENG-180/chunks/manifest.md | Chunk manifest for implementation progress tracking. |
| specs/ENG-180/chunks/chunk-01-corrective-mutation/tasks.md | Tasks for the corrective obligation mutation + schema/index work. |
| specs/ENG-180/chunks/chunk-01-corrective-mutation/context.md | Implementation context/plan excerpt for the mutation. |
| specs/ENG-180/chunks/chunk-02-wiring-and-query/tasks.md | Tasks for wiring corrective creation into reversal flow + queries. |
| specs/ENG-180/chunks/chunk-02-wiring-and-query/context.md | Context/plan excerpt for wiring + queries. |
| specs/ENG-180/chunks/chunk-03-tests/tasks.md | Tasks for ENG-180 test coverage. |
| specs/ENG-180/chunks/chunk-03-tests/context.md | Testing plan/context and references. |
| convex/schema.ts | Adds obligations.by_source_obligation index. |
| convex/payments/obligations/queries.ts | Adds getCorrectiveObligations and getObligationWithCorrectives internal queries. |
| convex/payments/obligations/createCorrectiveObligation.ts | New internal mutation to create + accrue corrective obligations and emit audit entries. |
| convex/payments/obligations/tests/correctiveObligation.test.ts | New Vitest/convex-test suite covering creation, validation, idempotency, and queries. |
| convex/engine/effects/collectionAttempt.ts | Schedules corrective obligation creation after reversal cascade in emitPaymentReversed. |
| convex/_generated/api.d.ts | Codegen update for the new Convex module. |
💡 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/payments/obligations/createCorrectiveObligation.ts`:
- Around line 56-79: The current idempotency check using existingCorrectives =
ctx.db.query("obligations").withIndex("by_type_and_source", ...) and then
finding existingCorrective treats any corrective for the same
type+sourceObligationId as a duplicate; change this to key idempotency to the
reversal identity instead: include the reversal identity fields (e.g.
args.postingGroupId or args.reversalId and args.reversedAmount) in the
query/index or, if you must keep the same index, filter existingCorrectives to
match postingGroupId/reversedAmount before returning created: false;
alternatively, if you intend to accumulate, update the found corrective
(existingCorrective) by adding the new reversedAmount/postingGroupId instead of
short-circuiting — adjust the logic around withIndex("by_type_and_source"),
existingCorrectives, existingCorrective, args.originalObligationId,
args.postingGroupId and args.reversedAmount accordingly.
In `@convex/payments/obligations/queries.ts`:
- Around line 112-119: The current queries filter out obligations by checking
o.type !== "late_fee", which hides reversed late_fee correctives because
createCorrectiveObligation copies original.type; update the logic so late_fee
originals aren't lost: either (A) change createCorrectiveObligation to
reject/throw when original.type === "late_fee" so you never create a corrective
with type "late_fee", or (B) add an explicit corrective marker (e.g.,
corrective: true or correctiveOfId: string) on corrective documents when
creating them in createCorrectiveObligation and update the query helpers that
currently use the type check to instead filter by that marker (preserving the
original.type metadata) so correctives are discovered reliably.
In `@specs/ENG-180/chunks/chunk-02-wiring-and-query/context.md`:
- Around line 116-117: The spec currently instructs scheduling
createCorrectiveObligation with reversedAmount = obligation.amount which will
over-create receivables for partial reversals; instead, after
postPaymentReversalCascade returns (it yields { reversalEntries, postingGroupId,
clawbackRequired }), compute reversedAmount from the actual reversal entry
amounts (e.g., sum the reversed cash amounts in reversalEntries or use the
specific reversed amount field provided by the cascade) and pass that as
reversedAmount to ctx.scheduler.runAfter(0, () =>
createCorrectiveObligation(...)), and still only schedule when obligation.status
=== "settled"; also update the emitPaymentReversed snippet to use the same
reversalEntries-derived reversedAmount rather than obligation.amount.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 57f2a15d-9a9c-4b2d-82ba-494c892eb24a
⛔ Files ignored due to path filters (1)
convex/_generated/api.d.tsis excluded by!**/_generated/**
📒 Files selected for processing (13)
convex/engine/effects/collectionAttempt.tsconvex/payments/obligations/__tests__/correctiveObligation.test.tsconvex/payments/obligations/createCorrectiveObligation.tsconvex/payments/obligations/queries.tsconvex/schema.tsspecs/ENG-180/chunks/chunk-01-corrective-mutation/context.mdspecs/ENG-180/chunks/chunk-01-corrective-mutation/tasks.mdspecs/ENG-180/chunks/chunk-02-wiring-and-query/context.mdspecs/ENG-180/chunks/chunk-02-wiring-and-query/tasks.mdspecs/ENG-180/chunks/chunk-03-tests/context.mdspecs/ENG-180/chunks/chunk-03-tests/tasks.mdspecs/ENG-180/chunks/manifest.mdspecs/ENG-180/tasks.md
…d of obligation.amount Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…uard against late_fee sources - Idempotency now keyed to (sourceObligationId, postingGroupId) instead of (type, sourceObligationId), allowing multiple independent reversals of the same obligation to each create their own corrective while still deduplicating retries of the same reversal event. - postingGroupId is now persisted on the corrective obligation record. - Added UNSUPPORTED_SOURCE_TYPE guard rejecting late_fee source obligations in the mutation, plus a defensive check at the scheduler call site. - Updated tests: idempotency test uses same postingGroupId, added test for distinct reversals creating separate correctives, added late_fee guard test. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Exercises the full reversal flow end-to-end: - Partial payment: corrective obligation uses CASH_RECEIVED reversal amount (60k), not obligation.amount (100k) - Full payment: corrective obligation matches the full settled amount - Non-settled obligations: no corrective obligation is created - Ledger assertions: reversal entries share correct postingGroupId Addresses PR #281 review thread 2 (missing integration test). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Merged
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.
TL;DR
Implemented corrective obligation creation after payment reversals to re-establish receivables in the domain layer when settled obligations are reversed.
What changed?
Added a complete corrective obligation system with three main components:
Core mutation (
createCorrectiveObligation.ts):sourceObligationIdOBLIGATION_ACCRUEDentries and audit journal entriesIntegration wiring:
emitPaymentReversedto schedule corrective obligation creation after reversal cascadectx.scheduler.runAftergetCorrectiveObligationsandgetObligationWithCorrectivesSchema and indexing:
by_source_obligationindex to enable querying corrective obligationsHow to test?
Run the test suite in
convex/payments/obligations/__tests__/correctiveObligation.test.tswhich covers:Why make this change?
When payments are reversed, the original obligation remains in
settledstatus (XState final state cannot be reopened). Without corrective obligations, the borrower appears fully paid in all domain systems despite the cash ledger showing the reversed receivable balance. This creates a disconnect between the cash layer and domain layer, breaking collection workflows and admin views. Corrective obligations restore this link by creating new receivables that enter the normal obligation lifecycle.Summary by CodeRabbit
Release Notes
New Features
Tests