ENG-148: Complete 9-step cash ledger posting pipeline#244
Merged
Connorbelez merged 4 commits intomainfrom Mar 21, 2026
Merged
Conversation
Close remaining gaps in postCashEntry pipeline: - Strengthen CORRECTION constraint check: require causedBy (REQ-242), actorId, admin actorType, and reason (was missing causedBy + actorId) - Add BORROWER_RECEIVABLE to NEGATIVE_BALANCE_EXEMPT_FAMILIES alongside CONTROL (Tech Design §9.1 Step 5) - Wire NUDGE step 9 as no-op placeholder for Phase 4 cursor consumers - Extract validators to validators.ts for reuse across mutations 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
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe changes extract and centralize post-entry argument validation, add a negative-balance exemption set including BORROWER_RECEIVABLE, tighten CORRECTION entry constraints (admin actor, actorId, causedBy, reason), and sequence post-entry persistence to await a new async nudge step before returning. Changes
Sequence Diagram(s)mermaid Client->>Service: postCashEntry(args) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
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 docstrings
🧪 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 |
![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
Completes remaining pieces of the cash-ledger postCashEntry 9-step posting pipeline by tightening CORRECTION constraints, expanding negative-balance exemptions per the tech design, wiring a step-9 NUDGE placeholder, and centralizing Convex arg validators for reuse.
Changes:
- Extracted
postCashEntryargs validation into a sharedvalidators.ts. - Added
NEGATIVE_BALANCE_EXEMPT_FAMILIES(CONTROL + BORROWER_RECEIVABLE) and applied it in balance checks. - Strengthened CORRECTION constraint checks and wired the 9-step pipeline flow including a no-op NUDGE step.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| convex/payments/cashLedger/validators.ts | Adds reusable Convex validators for postCashEntry inputs. |
| convex/payments/cashLedger/types.ts | Introduces NEGATIVE_BALANCE_EXEMPT_FAMILIES for balance-check exemptions. |
| convex/payments/cashLedger/postEntry.ts | Applies new exemption set, strengthens CORRECTION constraints, and wires step-9 NUDGE + validator extraction. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Owner
Author
|
@coderabbitai please review |
Contributor
|
🧠 Learnings used✅ Actions performedReview triggered.
|
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
convex/payments/cashLedger/postEntry.ts (1)
155-159: Placeholder is clear; consider removingasyncuntil Phase 4.The
nudgefunction is declaredasyncbut performs no async operations. While harmless, removingasyncwould avoid creating an unnecessary microtask on each call until Phase 4 wires real functionality.♻️ Optional simplification
-async function nudge(_ctx: MutationCtx): Promise<void> { +function nudge(_ctx: MutationCtx): void { // Intentionally empty — Phase 4 will wire cursor consumer notifications here. }Then at the call site (line 252):
- await nudge(ctx); + nudge(ctx);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/payments/cashLedger/postEntry.ts` around lines 155 - 159, The nudge function is declared async but does no async work; remove the async keyword from the nudge declaration (keep signature async-free: function nudge(_ctx: MutationCtx): void) and then update its call sites to drop unnecessary awaits (e.g., replace await nudge(ctx) with nudge(ctx) where invoked in postEntry or related functions) so no microtask is created until Phase 4.convex/payments/cashLedger/validators.ts (1)
4-16: Consider deriving validator from the source constant to stay DRY.The
cashEntryTypeValidatorduplicates the 11 entry types already defined inCASH_ENTRY_TYPES(types.ts). If one is updated without the other, they'll drift.You could derive the validator programmatically, though
v.unionrequires explicit arguments for type inference. Provide the first two validators directly, then spread the rest:♻️ Suggested refactor
+import { CASH_ENTRY_TYPES } from "./types"; + -export const cashEntryTypeValidator = v.union( - v.literal("OBLIGATION_ACCRUED"), - v.literal("CASH_RECEIVED"), - v.literal("CASH_APPLIED"), - v.literal("LENDER_PAYABLE_CREATED"), - v.literal("SERVICING_FEE_RECOGNIZED"), - v.literal("LENDER_PAYOUT_SENT"), - v.literal("OBLIGATION_WAIVED"), - v.literal("OBLIGATION_WRITTEN_OFF"), - v.literal("REVERSAL"), - v.literal("CORRECTION"), - v.literal("SUSPENSE_ESCALATED") -); +export const cashEntryTypeValidator = v.union( + v.literal(CASH_ENTRY_TYPES[0]), + v.literal(CASH_ENTRY_TYPES[1]), + ...CASH_ENTRY_TYPES.slice(2).map((t) => v.literal(t)) +);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/payments/cashLedger/validators.ts` around lines 4 - 16, cashEntryTypeValidator duplicates the literals in CASH_ENTRY_TYPES; change cashEntryTypeValidator to derive its union from the source constant by constructing the v.union call with the first two v.literal(...) arguments explicitly and then spreading the rest generated from CASH_ENTRY_TYPES (map each remaining entry to v.literal(entry)) so the validator stays DRY and updates automatically when CASH_ENTRY_TYPES changes; reference cashEntryTypeValidator, CASH_ENTRY_TYPES, and v.union to locate and implement this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@convex/payments/cashLedger/postEntry.ts`:
- Around line 155-159: The nudge function is declared async but does no async
work; remove the async keyword from the nudge declaration (keep signature
async-free: function nudge(_ctx: MutationCtx): void) and then update its call
sites to drop unnecessary awaits (e.g., replace await nudge(ctx) with nudge(ctx)
where invoked in postEntry or related functions) so no microtask is created
until Phase 4.
In `@convex/payments/cashLedger/validators.ts`:
- Around line 4-16: cashEntryTypeValidator duplicates the literals in
CASH_ENTRY_TYPES; change cashEntryTypeValidator to derive its union from the
source constant by constructing the v.union call with the first two
v.literal(...) arguments explicitly and then spreading the rest generated from
CASH_ENTRY_TYPES (map each remaining entry to v.literal(entry)) so the validator
stays DRY and updates automatically when CASH_ENTRY_TYPES changes; reference
cashEntryTypeValidator, CASH_ENTRY_TYPES, and v.union to locate and implement
this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b788078a-f5b1-4188-92e8-6f98bad4a293
📒 Files selected for processing (3)
convex/payments/cashLedger/postEntry.tsconvex/payments/cashLedger/types.tsconvex/payments/cashLedger/validators.ts
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Addresses PR review comments by adding integration tests for: - CORRECTION constraint validation (admin actorType, actorId, causedBy, reason) - REVERSAL constraint validation (causedBy required) - BORROWER_RECEIVABLE negative balance exemption - Non-exempt family (TRUST_CASH) negative balance rejection Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Mar 22, 2026
Merged
Merged
Closed
Merged
Connorbelez
added a commit
that referenced
this pull request
Apr 20, 2026
## Close remaining gaps in postCashEntry pipeline - Strengthen CORRECTION constraint check: require causedBy (REQ-242), actorId, admin actorType, and reason (was missing causedBy + actorId) - Add BORROWER_RECEIVABLE to NEGATIVE_BALANCE_EXEMPT_FAMILIES alongside CONTROL (Tech Design §9.1 Step 5) - Wire NUDGE step 9 as no-op placeholder for Phase 4 cursor consumers - Extract validators to validators.ts for reuse across mutations <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Balance checks now exempt additional account family for negative-balance rules, fixing inconsistent behavior. * **Refactor** * Validation for cash entry submissions reworked to use a centralized schema and stricter rules for CORRECTION entries (requires admin actor, causal fields, and reason). * **Tests** * Added tests covering correction/reversal constraints and the negative-balance exemption behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
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.
Close remaining gaps in postCashEntry pipeline
Summary by CodeRabbit
Bug Fixes
Refactor
Tests