ENG-61: implement rules engine, ScheduleRule, RetryRule, LateFeeeRule, and seed rules#123
Conversation
…, and seed rules Add the collection plan rules engine with a Strategy Pattern handler registry, three initial rule handlers (schedule, retry, late fee), supporting queries and mutations, idempotent seed data, and 13 unit tests — all passing. 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
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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. |
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.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Pull request overview
Implements a data-driven collection plan rules engine in the Convex backend (strategy-pattern handler registry + internal action orchestration), along with initial rule handlers (schedule/retry/late fee), seeding of default rule configuration, and supporting obligation/collection-plan queries and mutations.
Changes:
- Added collection plan rules engine (
evaluateRules) and three rule handlers (ScheduleRule, RetryRule, LateFeeRule). - Added supporting Convex internal queries/mutations for obligations and collection plan entries, plus an idempotent seed mutation for default rules.
- Added a new unit test suite for the three handlers, and updated a few seed/engine files with formatting-only changes.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| specs/ENG-61/tasks.md | ENG-61 task tracking doc for the rules engine work |
| specs/ENG-61/chunks/manifest.md | Chunk manifest for ENG-61 specs |
| specs/ENG-61/chunks/chunk-01-foundation/tasks.md | Foundation chunk task list |
| specs/ENG-61/chunks/chunk-01-foundation/context.md | Foundation chunk design/context notes |
| specs/ENG-61/chunks/chunk-02-engine-rules/tasks.md | Engine + rules chunk task list |
| specs/ENG-61/chunks/chunk-02-engine-rules/context.md | Engine + rules design/context notes |
| specs/ENG-61/chunks/chunk-03-seed-tests/tasks.md | Seed + tests chunk task list |
| specs/ENG-61/chunks/chunk-03-seed-tests/context.md | Seed + tests design/context notes |
| convex/seed/seedObligationStates.ts | Formatting-only change to a query index call |
| convex/seed/seedObligation.ts | Formatting-only changes to gracePeriodEnd calculations |
| convex/engine/reconciliation.ts | Formatting-only change to ctx.db.get call |
| convex/payments/collectionPlan/engine.ts | New internalAction rules engine with handler registry dispatch |
| convex/payments/collectionPlan/queries.ts | New internal queries for enabled rules, entry lookup, and status filtering |
| convex/payments/collectionPlan/mutations.ts | New internal mutation to create collection plan entries |
| convex/payments/collectionPlan/seed.ts | New idempotent internal mutation to seed default collection rules |
| convex/payments/collectionPlan/rules/scheduleRule.ts | ScheduleRule handler to create plan entries for upcoming obligations |
| convex/payments/collectionPlan/rules/retryRule.ts | RetryRule handler to create retry plan entries with exponential backoff |
| convex/payments/collectionPlan/rules/lateFeeRule.ts | LateFeeRule handler to create late-fee obligations on overdue events |
| convex/payments/tests/rules.test.ts | New unit tests for the three rule handlers |
| convex/obligations/queries.ts | Added internal obligation helpers (upcoming window, getById, late-fee lookup) |
| convex/obligations/mutations.ts | New internal mutation to create obligations (used by LateFeeRule) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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/obligations/mutations.ts`:
- Around line 27-43: The mutation handler in convex/obligations/mutations.ts is
directly inserting an obligation with status, lastTransitionAt and
machineContext set rather than journaling the creation; change it to use the
same creation pathway used by the Transition Engine (or call the existing
creation helper used in convex/seed/seedObligation.ts) so the creation journal
entry and hash chain are emitted, or explicitly emit a creation journal entry
via the Transition Engine before/after insert; update the handler to stop
setting lifecycle fields (status, lastTransitionAt, machineContext) directly and
instead let the Transition Engine initialize them and record the journaled
transition for auditability.
- Around line 14-25: The schema currently makes sourceObligationId optional for
all obligation types, allowing a late_fee to be created without its backlink
which breaks getLateFeeForObligation() idempotency; fix by making
sourceObligationId required when type === "late_fee" — either change the
validator to use a discriminated union (one variant for type: "late_fee" that
includes sourceObligationId as required and another for the other types) or add
an explicit runtime check in the create/insert mutation (the function that
inserts obligations) that throws/returns an error if type === "late_fee" and
sourceObligationId is missing; reference the fields type and sourceObligationId
and the helper getLateFeeForObligation() to locate the logic to update.
In `@convex/obligations/queries.ts`:
- Around line 112-125: The getLateFeeForObligation internalQuery currently does
a full table scan on obligations; add a compound index
["sourceObligationId","type"] to the obligations table definition in
convex/schema.ts and update the query in getLateFeeForObligation (the handler in
internalQuery) to call withIndex("sourceObligationId_type") or the exact index
name you add on ctx.db.query("obligations") before filter() so the DB uses the
new compound index for the lookup by sourceObligationId and type.
In `@convex/payments/collectionPlan/engine.ts`:
- Around line 38-43: The action contract's args (in
convex/payments/collectionPlan/engine.ts) is too permissive; make it a
discriminated union so "schedule" requires mortgageId and "event" requires
eventType (and keeps eventPayload optional). Replace the current args shape with
a v.union of two object schemas: one with trigger: v.literal("schedule") and
mortgageId: v.id("mortgages") (required), and another with trigger:
v.literal("event"), eventType: v.string() (required) and eventPayload:
v.optional(v.any()) (and mortgageId optional or omitted as appropriate). Ensure
the discriminant is the trigger field so downstream handlers can rely on correct
typing/validation.
In `@convex/payments/collectionPlan/rules/lateFeeRule.ts`:
- Around line 41-79: The code currently uses mortgageId from the event payload
when creating the late_fee obligation which can be stale; instead read the
mortgageId from the canonical source obligation (the sourceObligation loaded via
internal.obligations.queries.getById) and pass that mortgageId into the
internal.obligations.mutations.createObligation call (replace the payload
mortgageId with sourceObligation.mortgageId and ensure borrowerId continues to
come from sourceObligation).
In `@specs/ENG-61/chunks/chunk-01-foundation/tasks.md`:
- Line 6: Update the T-004 checklist in
specs/ENG-61/chunks/chunk-01-foundation/tasks.md to include the newly shipped
createObligation work: add an item for creating convex/obligations/mutations.ts
— createObligation (internalMutation) alongside the existing
convex/payments/collectionPlan/mutations.ts — createEntry entry so the
foundation spec matches the implementation and master task list; ensure the
markdown checklist line mirrors the existing formatting for T-004 items.
In `@specs/ENG-61/chunks/chunk-02-engine-rules/context.md`:
- Line 234: The section header contains a typo "LateFeeeRule" instead of
"LateFeeRule"; update the header text in
specs/ENG-61/chunks/chunk-02-engine-rules/context.md to read "T-008: LateFeeRule
(convex/payments/collectionPlan/rules/lateFeeRule.ts)" so it matches the
implementation file name lateFeeRule.ts and other rule headers.
In `@specs/ENG-61/chunks/chunk-03-seed-tests/context.md`:
- Around line 73-95: The test matrix in the markdown (sections "Engine Tests",
"ScheduleRule Tests", "RetryRule Tests", "LateFeeeRule Tests") lists 16
scenarios but the PR and master task list declare 13 tests; update the
specs/ENG-61/chunks/chunk-03-seed-tests/context.md to make them consistent by
either removing or marking three items as future coverage (e.g., move items 11,
15, 16 or any three you choose into a "Future/Out of scope" subsection) and
correct the "LateFeeeRule Tests" header typo to "LateFeeRule Tests" so the
documented matrix matches the shipped test suite and avoids misleading
lint/format checks for .md files.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5afe502e-d848-4555-8294-09d8d72cafce
📒 Files selected for processing (21)
convex/engine/reconciliation.tsconvex/obligations/mutations.tsconvex/obligations/queries.tsconvex/payments/__tests__/rules.test.tsconvex/payments/collectionPlan/engine.tsconvex/payments/collectionPlan/mutations.tsconvex/payments/collectionPlan/queries.tsconvex/payments/collectionPlan/rules/lateFeeRule.tsconvex/payments/collectionPlan/rules/retryRule.tsconvex/payments/collectionPlan/rules/scheduleRule.tsconvex/payments/collectionPlan/seed.tsconvex/seed/seedObligation.tsconvex/seed/seedObligationStates.tsspecs/ENG-61/chunks/chunk-01-foundation/context.mdspecs/ENG-61/chunks/chunk-01-foundation/tasks.mdspecs/ENG-61/chunks/chunk-02-engine-rules/context.mdspecs/ENG-61/chunks/chunk-02-engine-rules/tasks.mdspecs/ENG-61/chunks/chunk-03-seed-tests/context.mdspecs/ENG-61/chunks/chunk-03-seed-tests/tasks.mdspecs/ENG-61/chunks/manifest.mdspecs/ENG-61/tasks.md
…, and seed rules (#123) ## Add the collection plan rules engine with a Strategy Pattern handler registry Add the collection plan rules engine with a Strategy Pattern handler registry, three initial rule handlers (schedule, retry, late fee), supporting queries and mutations, idempotent seed data, and 13 unit tests — all passing. ### Foundation - Create `convex/payments/collectionPlan/` directory structure with rules subdirectory - Add obligation query helpers: `getUpcomingInWindow`, `getLateFeeForObligation`, and `getById` - Implement collection plan queries: `getEnabledRules`, `getEntryForObligation`, `getPlanEntriesByStatus` - Create `createEntry` mutation for collection plan entries and `createObligation` mutation for obligations ### Rules Engine - Implement `RuleHandler` interface and `RuleEvalContext` type for rule evaluation - Create handler registry mapping rule names to implementations using Strategy Pattern - Build `evaluateRules` internal action that loads enabled rules by priority and executes handlers - Develop three rule handlers: - **ScheduleRule**: Creates plan entries for upcoming obligations within configurable delay window - **RetryRule**: Schedules retry attempts with exponential backoff on collection failures - **LateFeeRule**: Creates late fee obligations when payments become overdue ### Testing & Validation - Add idempotent seed mutation for default collection rules configuration - Implement comprehensive test suite covering engine orchestration and individual rule logic - Verify exponential backoff calculations, idempotency checks, and parameter handling - All 13 tests passing with proper mocking of action context 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 ## Release Notes * **New Features** * Introduced a collection plan rules engine for evaluating and processing collection rules with priority-based execution * Added obligation creation and query functionality, including support for retrieving upcoming obligations and late fee information * Added collection plan entry management with status tracking and entry creation capabilities <!-- end of auto-generated comment: release notes by coderabbit.ai -->

Add the collection plan rules engine with a Strategy Pattern handler registry
Add the collection plan rules engine with a Strategy Pattern handler registry,
three initial rule handlers (schedule, retry, late fee), supporting queries and
mutations, idempotent seed data, and 13 unit tests — all passing.
Foundation
convex/payments/collectionPlan/directory structure with rules subdirectorygetUpcomingInWindow,getLateFeeForObligation, andgetByIdgetEnabledRules,getEntryForObligation,getPlanEntriesByStatuscreateEntrymutation for collection plan entries andcreateObligationmutation for obligationsRules Engine
RuleHandlerinterface andRuleEvalContexttype for rule evaluationevaluateRulesinternal action that loads enabled rules by priority and executes handlersTesting & Validation
Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com
Summary by CodeRabbit
Release Notes