Skip to content

ENG-197: Derive bridge transfer types from obligation types#294

Merged
Connorbelez merged 1 commit intomainfrom
eng197-bridge
Mar 27, 2026
Merged

ENG-197: Derive bridge transfer types from obligation types#294
Connorbelez merged 1 commit intomainfrom
eng197-bridge

Conversation

@Connorbelez
Copy link
Copy Markdown
Owner

@Connorbelez Connorbelez commented Mar 27, 2026

TL;DR

Fixed hardcoded transfer type in collection attempt bridge to dynamically derive transfer types from obligation types, ensuring proper categorization of interest, principal, late fees, and arrears cure collections.

What changed?

  • Added reverse mapping OBLIGATION_TYPE_TO_TRANSFER_TYPE and helper function obligationTypeToTransferType() to derive transfer types from obligation types
  • Replaced hardcoded borrower_interest_collection in the collection attempt bridge with dynamic lookup based on the first obligation's type
  • Updated bridge tests to reflect actual production behavior where transfers start as initiated status and transition to confirmed via GT
  • Added comprehensive test coverage for obligation-to-transfer type mapping with proper fallback handling

How to test?

Run the following commands to verify the changes:

bunx convex codegen
bun check
bun typecheck
bun test convex/payments/transfers/__tests__/types.test.ts convex/payments/transfers/__tests__/bridge.test.ts

Test that collection attempts for different obligation types (regular_interest, principal_repayment, late_fee, arrears_cure) create bridge transfers with the correct transfer types.

Why make this change?

The bridge was incorrectly labeling all transfers as interest collections regardless of the actual obligation type. This caused misclassification of principal payments, late fees, and arrears cures, making financial reporting and reconciliation inaccurate. The fix ensures proper transfer categorization while maintaining existing Phase M2a behavior and idempotency guarantees.

Summary by CodeRabbit

  • Bug Fixes

    • Bridge transfers now correctly determine their type classification based on the underlying obligation, ensuring transfers are processed with the appropriate categorization instead of using a default value.
  • Tests

    • Enhanced test coverage for transfer type derivation logic and idempotency validation to ensure consistent transfer handling.

Copy link
Copy Markdown

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @Connorbelez, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 27, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 50f83766-32c6-424a-862a-97ad97e2b36a

📥 Commits

Reviewing files that changed from the base of the PR and between b547b62 and e7b92b5.

⛔ Files ignored due to path filters (1)
  • convex/_generated/api.d.ts is excluded by !**/_generated/**
📒 Files selected for processing (12)
  • convex/engine/effects/collectionAttempt.ts
  • convex/payments/transfers/__tests__/bridge.test.ts
  • convex/payments/transfers/__tests__/types.test.ts
  • convex/payments/transfers/types.ts
  • specs/ENG-197/chunks/chunk-01-bridge-mapping/context.md
  • specs/ENG-197/chunks/chunk-01-bridge-mapping/status.md
  • specs/ENG-197/chunks/chunk-01-bridge-mapping/tasks.md
  • specs/ENG-197/chunks/chunk-02-tests-verification/context.md
  • specs/ENG-197/chunks/chunk-02-tests-verification/status.md
  • specs/ENG-197/chunks/chunk-02-tests-verification/tasks.md
  • specs/ENG-197/chunks/manifest.md
  • specs/ENG-197/tasks.md

📝 Walkthrough

Walkthrough

The PR replaces a hardcoded transferType: "borrower_interest_collection" in bridge transfer creation with a dynamic lookup using the first bridged obligation's type. This requires introducing a reverse mapping (OBLIGATION_TYPE_TO_TRANSFER_TYPE), a default constant, and a helper function in the types module, alongside comprehensive test coverage and supporting specification documentation.

Changes

Cohort / File(s) Summary
Bridge Transfer Type Mapping
convex/payments/transfers/types.ts
Added OBLIGATION_TYPE_TO_TRANSFER_TYPE reverse mapping, DEFAULT_OBLIGATION_TRANSFER_TYPE constant, and obligationTypeToTransferType() helper function to derive inbound transfer types from obligation types with safe fallback handling.
Bridge Transfer Creation
convex/engine/effects/collectionAttempt.ts
Replaced hardcoded transferType: "borrower_interest_collection" with dynamic derivation using obligationTypeToTransferType(firstOblForBridge.type) when creating bridge transfer requests.
Test Coverage
convex/payments/transfers/__tests__/bridge.test.ts, convex/payments/transfers/__tests__/types.test.ts
Updated bridge test fixtures from "confirmed" status to "initiated" semantics; added test helpers for settlement payloads; expanded types tests to cover obligation-type→transfer-type mapping, helper function behavior, and default fallback scenarios.
Implementation Specification
specs/ENG-197/chunks/chunk-01-bridge-mapping/*, specs/ENG-197/chunks/chunk-02-tests-verification/*, specs/ENG-197/chunks/manifest.md, specs/ENG-197/tasks.md
Added comprehensive specification documentation defining two execution chunks (bridge mapping implementation and test verification), task definitions, completion status records, and execution constraints to preserve Phase M2a behavior and idempotency semantics.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Possibly related PRs

Poem

🐰 A bridge awaits no hardcoded fate,
Obligations map, the transfer types align,
From interest to principal, the defaults state,
Safe fallbacks glow when unknowns combine—
The borrower's flow, at last, divine! ✨

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch eng197-bridge

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Owner Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@Connorbelez Connorbelez changed the title Derive bridge transfer types from obligation types ENG-197: Derive bridge transfer types from obligation types Mar 27, 2026
@linear
Copy link
Copy Markdown

linear bot commented Mar 27, 2026

@Connorbelez Connorbelez marked this pull request as ready for review March 27, 2026 21:48
Copilot AI review requested due to automatic review settings March 27, 2026 21:48
Copy link
Copy Markdown

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the Phase M2a collection-attempt → transferRequest bridge to derive inbound transferType from the bridged obligation’s type, avoiding misclassification of non-interest collections and aligning tests with the actual initiated→confirmed GT flow.

Changes:

  • Added reverse obligation→transfer mapping plus obligationTypeToTransferType() helper with a safe default.
  • Updated emitPaymentReceived to use the derived transfer type instead of a hardcoded value.
  • Refreshed transfer bridge tests to reflect initiated insert + immediate FUNDS_SETTLED transition, and added mapping coverage.

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated no comments.

Show a summary per file
File Description
specs/ENG-197/tasks.md Master task list documenting the mapping + bridge + test updates.
specs/ENG-197/chunks/manifest.md Chunk breakdown for implementation and verification.
specs/ENG-197/chunks/chunk-01-bridge-mapping/tasks.md Tasks for reverse mapping + bridge integration.
specs/ENG-197/chunks/chunk-01-bridge-mapping/status.md Status/verification notes for mapping + bridge changes.
specs/ENG-197/chunks/chunk-01-bridge-mapping/context.md Context and constraints for the bridge mapping work.
specs/ENG-197/chunks/chunk-02-tests-verification/tasks.md Tasks for updating bridge tests and verification.
specs/ENG-197/chunks/chunk-02-tests-verification/status.md Status/verification notes for test updates.
specs/ENG-197/chunks/chunk-02-tests-verification/context.md Context describing how tests now match production GT behavior.
convex/payments/transfers/types.ts Adds OBLIGATION_TYPE_TO_TRANSFER_TYPE, default, and helper function.
convex/engine/effects/collectionAttempt.ts Uses obligationTypeToTransferType(firstOblForBridge.type) when inserting the bridged transfer request.
convex/payments/transfers/tests/types.test.ts Adds unit coverage for reverse mapping + fallback behavior.
convex/payments/transfers/tests/bridge.test.ts Updates bridge tests to initiated insert semantics and validates type derivation + settlement payload shape.
convex/_generated/api.d.ts Regenerated Convex API typings after module/function surface changes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Owner Author

@coderabbitai please review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 27, 2026

@Connorbelez, I'll start reviewing PR #294 right away!


🧠 Learnings used
Learnt from: Connorbelez
Repo: Connorbelez/tanstackTemplate PR: 5
File: specs/governed-transitions/chunks/chunk-01-schema-machine/context.md:209-213
Timestamp: 2026-03-13T03:17:28.559Z
Learning: In the Connorbelez/tanstackTemplate repository, do not flag .md files for markdown formatting/linting issues (e.g., MD040 fenced code block language). Only reference .md files (specs, PRDs, design docs) when they reveal architectural drift or implementation inconsistencies against actual code.

Learnt from: CR
Repo: Connorbelez/tanstackTemplate PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-16T15:27:40.455Z
Learning: Use React, TanStack Router, TanStack Query with Convex query integration, Tailwind CSS, and ShadCN UI for frontend development

Learnt from: CR
Repo: Connorbelez/tanstackTemplate PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-21T18:28:26.698Z
Learning: After completing a major unit of work, run `coderabbit review --plain` to get a code review summary and check for potential issues or improvements

Learnt from: CR
Repo: Connorbelez/tanstackTemplate PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-16T06:17:10.192Z
Learning: After completing a major unit of work, run `coderabbit review --plain` to get a code review summary and check for potential issues or improvements

Learnt from: CR
Repo: Connorbelez/tanstackTemplate PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-16T16:49:07.666Z
Learning: After completing a major unit of work, run `coderabbit review --plain` to get a code review summary and check for potential issues or improvements

Learnt from: CR
Repo: Connorbelez/tanstackTemplate PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-16T16:49:07.666Z
Learning: Applies to src/**/*.{tsx,ts} : Use React, TanStack Router, TanStack Query with Convex integration, Tailwind CSS, and ShadCN UI for frontend development

Learnt from: Connorbelez
Repo: Connorbelez/tanstackTemplate PR: 286
File: specs/ENG-184/chunks/chunk-01-schema-types/context.md:68-73
Timestamp: 2026-03-27T02:22:04.381Z
Learning: In Connorbelez/tanstackTemplate, `convex/payments/transfers/types.ts` exports both `TransferStatus` (active machine states: initiated, pending, processing, confirmed, failed, cancelled, reversed) and `PersistedTransferStatus = TransferStatus | LegacyTransferStatus` (includes legacy `approved`/`completed` from the ENG-190 stub). For query return types that read from the `transferRequests` table, `PersistedTransferStatus` should be used until ENG-190 migration retires legacy rows. Do NOT flag `TransferStatus` narrowing in control-flow/state-machine code as missing legacy values — that is intentional.

Learnt from: Connorbelez
Repo: Connorbelez/tanstackTemplate PR: 133
File: specs/ENG-65/chunks/chunk-03-generation-rules/context.md:10-10
Timestamp: 2026-03-18T20:58:32.818Z
Learning: In Connorbelez/tanstackTemplate, the mortgage obligation generation implementation in `convex/payments/obligations/generate.ts` and `convex/payments/obligations/generateImpl.ts` uses `principal` (not `principalBalance`) and `maturityDate` (not `termEndDate`) as the destructured field names from the mortgage document. Do not flag these as incorrect field names.
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@Connorbelez Connorbelez merged commit 47d2abe into main Mar 27, 2026
0 of 3 checks passed
Connorbelez added a commit that referenced this pull request Mar 28, 2026
### TL;DR

Fixed hardcoded transfer type in collection attempt bridge to dynamically derive transfer types from obligation types, ensuring proper categorization of interest, principal, late fees, and arrears cure collections.

### What changed?

- Added reverse mapping `OBLIGATION_TYPE_TO_TRANSFER_TYPE` and helper function `obligationTypeToTransferType()` to derive transfer types from obligation types
- Replaced hardcoded `borrower_interest_collection` in the collection attempt bridge with dynamic lookup based on the first obligation's type
- Updated bridge tests to reflect actual production behavior where transfers start as `initiated` status and transition to `confirmed` via GT
- Added comprehensive test coverage for obligation-to-transfer type mapping with proper fallback handling

### How to test?

Run the following commands to verify the changes:
```bash
bunx convex codegen
bun check
bun typecheck
bun test convex/payments/transfers/__tests__/types.test.ts convex/payments/transfers/__tests__/bridge.test.ts
```

Test that collection attempts for different obligation types (regular_interest, principal_repayment, late_fee, arrears_cure) create bridge transfers with the correct transfer types.

### Why make this change?

The bridge was incorrectly labeling all transfers as interest collections regardless of the actual obligation type. This caused misclassification of principal payments, late fees, and arrears cures, making financial reporting and reconciliation inaccurate. The fix ensures proper transfer categorization while maintaining existing Phase M2a behavior and idempotency guarantees.
Connorbelez added a commit that referenced this pull request Apr 20, 2026
### TL;DR

Fixed hardcoded transfer type in collection attempt bridge to dynamically derive transfer types from obligation types, ensuring proper categorization of interest, principal, late fees, and arrears cure collections.

### What changed?

- Added reverse mapping `OBLIGATION_TYPE_TO_TRANSFER_TYPE` and helper function `obligationTypeToTransferType()` to derive transfer types from obligation types
- Replaced hardcoded `borrower_interest_collection` in the collection attempt bridge with dynamic lookup based on the first obligation's type
- Updated bridge tests to reflect actual production behavior where transfers start as `initiated` status and transition to `confirmed` via GT
- Added comprehensive test coverage for obligation-to-transfer type mapping with proper fallback handling

### How to test?

Run the following commands to verify the changes:
```bash
bunx convex codegen
bun check
bun typecheck
bun test convex/payments/transfers/__tests__/types.test.ts convex/payments/transfers/__tests__/bridge.test.ts
```

Test that collection attempts for different obligation types (regular_interest, principal_repayment, late_fee, arrears_cure) create bridge transfers with the correct transfer types.

### Why make this change?

The bridge was incorrectly labeling all transfers as interest collections regardless of the actual obligation type. This caused misclassification of principal payments, late fees, and arrears cures, making financial reporting and reconciliation inaccurate. The fix ensures proper transfer categorization while maintaining existing Phase M2a behavior and idempotency guarantees.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants