ENG-156: Implement SUSPENSE routing for unmatched cash#252
Conversation
Add SUSPENSE_ROUTED entry type and integration functions so cash that cannot be matched to an obligation routes to a SUSPENSE account with diagnostic metadata instead of being silently dropped. - Register SUSPENSE_ROUTED across types, validators, schema, and posting pipeline balance check exclusion - Add postToSuspense helper with audit logging - Add postCashReceiptWithSuspenseFallback wrapper that tries obligation resolution first, falls back to SUSPENSE on failure - Enrich getSuspenseItems() with createdAt and ageMs for escalation workflow support Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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 (6)
✨ 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 |
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.
Actionable comments posted: 3
🧹 Nitpick comments (1)
convex/payments/cashLedger/integrations.ts (1)
292-299: Consider passing source directly to avoid double normalization.The
sourceis normalized at line 286, butpostCashReceiptForObligationalso callsnormalizeSourceinternally (line 143). This is functionally correct since normalization is idempotent, but you could passargs.sourcedirectly here to avoid redundant processing.♻️ Optional: pass raw source
return postCashReceiptForObligation(ctx, { obligationId: obligation._id, amount: args.amount, idempotencyKey: args.idempotencyKey, effectiveDate, attemptId: args.attemptId, - source: normalizedSource, + source: args.source, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/payments/cashLedger/integrations.ts` around lines 292 - 299, The code currently passes normalizedSource into postCashReceiptForObligation while normalizeSource is also invoked inside postCashReceiptForObligation (via normalizeSource), causing redundant normalization; change the call in integrations.ts to pass args.source (the raw source) instead of normalizedSource so postCashReceiptForObligation handles normalization once, leaving the local normalizeSource call unused or removable (see normalizedSource variable and the normalizeSource and postCashReceiptForObligation functions to update accordingly).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@specs/ENG-156/chunks/chunk-01-schema-types/tasks.md`:
- Around line 5-9: Update the checklist in
specs/ENG-156/chunks/chunk-01-schema-types/tasks.md so it matches the manifest
and root tasks: either mark items T-001 through T-005 as completed (check the
boxes for T-001, T-002, T-003, T-004, T-005) if those changes were implemented,
or add a short note at the top of the file stating this is a planning template
and the checklist is intentionally left unchecked; ensure the note references
the task IDs (T-001..T-005) so it's clear why the file differs from
specs/ENG-156/chunks/manifest.md and specs/ENG-156/tasks.md.
In `@specs/ENG-156/chunks/manifest.md`:
- Line 6: Update the chunk description to correctly reflect what was
implemented: change the phrase "enrich reconciliation query" to "enrich suspense
query" (or equivalent) in the manifest entry for chunk-02-integration-queries,
since the enrichment was implemented in getSuspenseItems()
(convex/payments/cashLedger/queries.ts); ensure the updated text mentions
getSuspenseItems or "suspense query" so the manifest accurately maps to the
implemented function.
In `@specs/ENG-156/tasks.md`:
- Line 16: Update the T-009 checklist entry so the quality-gate commands include
`bunx convex codegen` alongside `bun check` and `bun typecheck`; specifically
modify the checkbox text for **T-009** to read something like: Run quality
gates: `bun check && bun typecheck && bunx convex codegen && bun run test` so
the pre-completion gate set includes the required `bunx convex codegen` step.
---
Nitpick comments:
In `@convex/payments/cashLedger/integrations.ts`:
- Around line 292-299: The code currently passes normalizedSource into
postCashReceiptForObligation while normalizeSource is also invoked inside
postCashReceiptForObligation (via normalizeSource), causing redundant
normalization; change the call in integrations.ts to pass args.source (the raw
source) instead of normalizedSource so postCashReceiptForObligation handles
normalization once, leaving the local normalizeSource call unused or removable
(see normalizedSource variable and the normalizeSource and
postCashReceiptForObligation functions to update accordingly).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 91e95899-386c-4432-aa43-399316dd8778
📒 Files selected for processing (12)
convex/payments/cashLedger/integrations.tsconvex/payments/cashLedger/postEntry.tsconvex/payments/cashLedger/queries.tsconvex/payments/cashLedger/types.tsconvex/payments/cashLedger/validators.tsconvex/schema.tsspecs/ENG-156/chunks/chunk-01-schema-types/context.mdspecs/ENG-156/chunks/chunk-01-schema-types/tasks.mdspecs/ENG-156/chunks/chunk-02-integration-queries/context.mdspecs/ENG-156/chunks/chunk-02-integration-queries/tasks.mdspecs/ENG-156/chunks/manifest.mdspecs/ENG-156/tasks.md
There was a problem hiding this comment.
Pull request overview
Implements SUSPENSE routing for unmatched cash in the cash ledger by introducing a new SUSPENSE_ROUTED entry type, wiring it through schema/validators/types, and adding integration/query support so unmatched receipts can be routed (with diagnostics) instead of being dropped.
Changes:
- Add
SUSPENSE_ROUTEDto cash-ledger entry type definitions/validators/schema and exclude it from balance checks. - Add
postToSuspense+postCashReceiptWithSuspenseFallbackto route unmatched receipts into SUSPENSE with audit logging + diagnostic metadata. - Enrich
getSuspenseItems()withcreatedAtandageMsto support escalation workflows.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| specs/ENG-156/tasks.md | Master task list documenting implementation steps and gates. |
| specs/ENG-156/chunks/manifest.md | Chunk manifest describing completed work. |
| specs/ENG-156/chunks/chunk-01-schema-types/tasks.md | Work plan for schema/type registration of SUSPENSE_ROUTED. |
| specs/ENG-156/chunks/chunk-01-schema-types/context.md | Design context for entry-type registration + balance-check rationale. |
| specs/ENG-156/chunks/chunk-02-integration-queries/tasks.md | Work plan for new integrations + query enrichment. |
| specs/ENG-156/chunks/chunk-02-integration-queries/context.md | Design context/patterns for suspense routing + audit logging. |
| convex/schema.ts | Adds SUSPENSE_ROUTED to cash_ledger_journal_entries.entryType union. |
| convex/payments/cashLedger/validators.ts | Extends runtime validator union with SUSPENSE_ROUTED. |
| convex/payments/cashLedger/types.ts | Registers SUSPENSE_ROUTED in CASH_ENTRY_TYPES and family-constraint map. |
| convex/payments/cashLedger/postEntry.ts | Excludes SUSPENSE_ROUTED from balance checks in posting pipeline. |
| convex/payments/cashLedger/integrations.ts | Adds suspense routing helper + fallback wrapper + audit logging. |
| convex/payments/cashLedger/queries.ts | Adds createdAt + ageMs fields to suspense item query results. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…tions - Mark T-001 through T-005 as completed in chunk-01 tasks.md - Fix manifest: "reconciliation query" → "suspense query (getSuspenseItems)" - Add bunx convex codegen to T-009 quality gate description Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ense path - Use args.source.actorId ?? "system" instead of hardcoded "system" for audit log actorId, preserving admin/webhook actor traceability - Include actorType and channel in audit log metadata - Add attemptId to postToSuspense args and pass through to postCashEntryInternal for consistent traceability across both paths Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…geMs Remove Date.now()-based ageMs computation from Convex reactive query. Callers should compute ageMs client-side from the stable createdAt timestamp, since time-based values in reactive queries become stale. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add 8 tests covering valid posting semantics, family rejection, and balance-exemption behavior for the new SUSPENSE_ROUTED entry type, following the same patterns as existing SUSPENSE_ESCALATED tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
## Add SUSPENSE_ROUTED entry type and integration functions Add SUSPENSE_ROUTED entry type and integration functions so cash that cannot be matched to an obligation routes to a SUSPENSE account with diagnostic metadata instead of being silently dropped. - Register SUSPENSE_ROUTED across types, validators, schema, and posting pipeline balance check exclusion - Add postToSuspense helper with audit logging - Add postCashReceiptWithSuspenseFallback wrapper that tries obligation resolution first, falls back to SUSPENSE on failure - Enrich getSuspenseItems() with createdAt and ageMs for escalation workflow support Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added automatic routing of unmatched cash receipts to suspense account when no matching obligation is found, ensuring cash is never dropped from the system. * Enhanced suspense items view with creation timestamp and age metrics to improve visibility and tracking of pending items for escalation. * **Documentation** * Added implementation specifications for the new suspense routing system. <!-- end of auto-generated comment: release notes by coderabbit.ai -->

Add SUSPENSE_ROUTED entry type and integration functions
Add SUSPENSE_ROUTED entry type and integration functions so cash that cannot be matched to an obligation routes to a SUSPENSE account with diagnostic metadata instead of being silently dropped.
Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com
Summary by CodeRabbit
New Features
Documentation