Open
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, your pull request is larger than the review limit of 150000 diff characters
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 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)
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 was referenced Apr 18, 2026
Owner
Author
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
This was referenced Apr 18, 2026
Open
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Pull request overview
This PR expands the admin origination “commit” path to also bootstrap payments (obligations + initial collection plan entries), generate/refresh a mortgage-backed listing projection, and optionally attempt immediate Rotessa activation (“provider_managed_now”), with updated UI surfaces and broad test coverage (unit + Convex integration + Playwright e2e).
Changes:
- Add payment bootstrap on canonical mortgage activation (obligations + planned app-owned
collectionPlanEntries) and surface the resulting “Payment Setup” context in admin mortgage details. - Introduce mortgage-backed listing projection generation + refresh, preserving curated fields while overwriting projection-owned fields.
- Adopt a copied Rotessa SDK client (types/manifest/client) and wire durable activation state + retry flows into origination collections.
Reviewed changes
Copilot reviewed 45 out of 46 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/convex/payments/rotessaApiClient.test.ts | Verifies Rotessa SDK client behavior + legacy adapter contract compatibility. |
| src/test/convex/listings/projection.test.ts | Tests listing projection refresh preserves curated fields and remains idempotent. |
| src/test/convex/listings/create.test.ts | Updates listing create tests to align with new mortgage-backed creation constraints. |
| src/test/convex/admin/origination/validators.test.ts | Updates origination validator expectations for collections activation + warnings. |
| src/test/convex/admin/origination/commit.test.ts | Extensive coverage for payment bootstrap, listing projection, Rotessa activation + retries, replay/backfill. |
| src/test/admin/origination/origination-workflow.test.tsx | Aligns UI helper text with new activation semantics. |
| src/test/admin/origination/collections-step.test.tsx | Adds UI test for selecting eligible bank accounts for immediate Rotessa activation. |
| src/test/admin/mortgage-dedicated-details.test.tsx | Expands mortgage detail tests to cover the new “Payment Setup” surface + retry UI. |
| src/lib/admin-origination.ts | Extends OriginationCollectionsDraft with durable activation fields. |
| src/components/admin/shell/entity-view-adapters.tsx | Makes objectDef optional for adapter rendering and updates listing layout to use computed payment summary. |
| src/components/admin/shell/RecordSidebar.tsx | Allows adapters to render details without an object definition (with a fallback record). |
| src/components/admin/origination/workflow.ts | Updates document step copy to reflect new phase naming/activation point. |
| src/components/admin/origination/ReviewStep.tsx | Updates review step descriptions to include payment bootstrap + listing projection + immediate activation behavior. |
| src/components/admin/origination/ParticipantsStep.tsx | Updates copy to reflect commit-time canonical resolution for co-borrowers. |
| src/components/admin/origination/OriginationWorkspacePage.tsx | Passes caseId into collections step and updates phase copy/description. |
| src/components/admin/origination/MortgageTermsStep.tsx | Updates step copy to reflect payment bootstrap created on commit. |
| src/components/admin/origination/ListingCurationStep.tsx | Updates curation copy to reflect projection preservation behavior. |
| src/components/admin/origination/CollectionsStep.tsx | Replaces freeform provider inputs with a bank-account-driven immediate Rotessa activation UI backed by a query. |
| e2e/origination/payment-bootstrap.spec.ts | E2E coverage for obligations + planned collection entries surfacing in mortgage details. |
| e2e/origination/listing-projection.spec.ts | E2E coverage for listing creation, curation persistence, and projection refresh. |
| e2e/helpers/origination.ts | Adds e2e helper queries for mortgage detail context and listing lookup by mortgage. |
| docs/architecture/admin-origination-workspace.md | Updates architecture doc to reflect payment bootstrap, listing projection, and immediate Rotessa activation. |
| convex/test/originationE2e.ts | Extends committed-origination cleanup to include linked listings. |
| convex/test/moduleMaps.ts | Registers new Convex modules (origination collections, listings projection/curation, rotessa helpers). |
| convex/schema.ts | Adds paymentBootstrapScheduleRuleMissing to mortgages. |
| convex/payments/rotessa/types.ts | Adds Rotessa SDK type definitions. |
| convex/payments/rotessa/manifest.ts | Adds Rotessa endpoint manifest for future integrations. |
| convex/payments/rotessa/client.ts | Adds fetch-based Rotessa SDK client with error normalization + timeouts. |
| convex/payments/rotessa/api.ts | Re-implements legacy RotessaApiClient on top of the copied SDK client. |
| convex/payments/recurringSchedules/rotessaCustomerReference.ts | Centralizes Rotessa customer reference resolution for bank accounts. |
| convex/payments/recurringSchedules/activation.ts | Uses the shared Rotessa customer reference helper during activation. |
| convex/payments/origination/bootstrap.ts | Implements origination payment bootstrap (obligations + initial scheduling) with replay-safety. |
| convex/payments/collectionPlan/readModels.ts | Includes executionMode in collection plan entry read model. |
| convex/mortgages/activateMortgageAggregate.ts | Wires payment bootstrap + listing projection into canonical activation + replay/backfill logic. |
| convex/listings/validators.ts | Splits curation vs projection-owned listing fields and adds curation update validator. |
| convex/listings/projection.ts | Implements mortgage-backed listing projection + refresh mutation. |
| convex/listings/curation.ts | Adds curation update mutation (listing-manage). |
| convex/listings/create.ts | Exports canonical insert helper and prevents public creation of mortgage-backed listings. |
| convex/crm/entityViewFields.ts | Adds computed listingPaymentSummary. |
| convex/crm/entityAdapterRegistry.ts | Uses computed listing payment summary in dedicated listing adapter fields. |
| convex/crm/detailContextQueries.ts | Adds richer mortgage “Payment Setup” context and introduces listing detail context query. |
| convex/admin/origination/validators.ts | Extends collections validation + draft normalization and updates warnings. |
| convex/admin/origination/commit.ts | Persists committed listing id, normalizes collections state, and triggers immediate activation post-commit. |
| convex/admin/origination/collections.ts | New module for collections setup context, activation attempt tracking, and retry flow. |
| convex/_generated/api.d.ts | Updates generated API typing to include new modules. |
💡 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: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/admin/origination/ListingCurationStep.tsx (1)
85-94:⚠️ Potential issue | 🟡 MinorStale copy: "future listing projection" no longer matches Phase-5 behavior.
The surrounding copy was updated (card description, featured helper, adminNotes placeholder) to reflect that the projector now materializes listings during commit via
upsertMortgageListingProjection. ThismarketplaceCopyplaceholder still says “future listing projection”, which now contradicts the rest of the step.📝 Suggested wording
- placeholder="Operator-only curation copy for future listing projection." + placeholder="Operator-only curation copy surfaced on the marketplace listing."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/admin/origination/ListingCurationStep.tsx` around lines 85 - 94, Update the stale placeholder in ListingCurationStep: find the textarea handling nextDraft.marketplaceCopy (inside the ListingCurationStep component) and replace the placeholder string "Operator-only curation copy for future listing projection." with wording that reflects Phase-5 behavior, e.g. "Operator-only curation copy for listing that will be materialized on commit." to match the other updated copy and the upsertMortgageListingProjection flow.
🧹 Nitpick comments (7)
src/test/admin/origination/origination-workflow.test.tsx (1)
163-167: Consider importing the shared warning constant to avoid drift.The literal here duplicates
ORIGINATION_PROVIDER_MANAGED_COLLECTIONS_WARNINGfromconvex/admin/origination/validators.ts. If the copy is tweaked again, this test will silently fall out of sync. Importing the constant removes that class of failure.♻️ Suggested refactor
+import { ORIGINATION_PROVIDER_MANAGED_COLLECTIONS_WARNING } from "../../../../convex/admin/origination/validators"; ... snapshot={{ - reviewWarnings: [ - "Provider-managed now will attempt immediate Rotessa activation after canonical commit. The mortgage still commits even if activation fails, and the payment setup screen will surface status and retry.", - ], + reviewWarnings: [ORIGINATION_PROVIDER_MANAGED_COLLECTIONS_WARNING], }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/admin/origination/origination-workflow.test.tsx` around lines 163 - 167, Replace the duplicated literal in the test snapshot with the shared constant ORIGINATION_PROVIDER_MANAGED_COLLECTIONS_WARNING from convex/admin/origination/validators.ts: import that constant into origination-workflow.test.tsx and use it for the reviewWarnings snapshot entry so the test references the single source of truth instead of an inline string.convex/crm/entityAdapterRegistry.ts (1)
277-287: AddmaterializationMode: "sync"for consistency with siblings.Every other computed field in this array sets
materializationModeexplicitly ("hydrated"for the property/mortgage summaries,"sync"formortgagePaymentSummaryandobligationPaymentProgressSummary).listingPaymentSummaryomits it. It works today only becauseevaluateComputedFieldValueshort-circuits on"hydrated"and falls through the switch otherwise, but leaving the mode unset is a correctness trap if the short-circuit logic is ever inverted or extended.♻️ Proposed fix
{ description: "Payment amount paired with the actual cadence so mortgage-backed listings never imply a synthetic monthly-equivalent payment.", expressionKey: "listingPaymentSummary", fieldName: "paymentSummary", fieldType: "text", isVisibleByDefault: true, label: "Payment", + materializationMode: "sync", rendererHint: "computed", sourceFieldNames: ["monthlyPayment", "paymentFrequency"], },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/crm/entityAdapterRegistry.ts` around lines 277 - 287, The computed field object for listingPaymentSummary is missing an explicit materializationMode and should set materializationMode: "sync" to match its siblings; update the object with expressionKey "listingPaymentSummary" (fieldName "paymentSummary") to include materializationMode: "sync" so behavior remains explicit and robust if evaluateComputedFieldValue changes.src/components/admin/shell/RecordSidebar.tsx (1)
513-536: Fallback record with syntheticobjectDefIdand zero timestamps risks silent failures in future adapters.The
fallbackAdapterRecordcreated at lines 513–520 usesobjectDefId: "fallback" as Id<"objectDefs">and zero-valued timestamps. This fallback is passed toadapter.renderDetailsTab()when!objectDef && adapter.renderDetailsTab != null. While current dedicated adapters (Listings, Mortgages, Obligations) only accessrecord._idandrecord.fields, the type signature (RecordDetailsRenderArgs.record: RecordDetailRecord) permits any adapter to readobjectDefId,createdAt, orupdatedAt. A future adapter that derives display labels or performs lookups from these fields would silently consume bogus values without warning.The type escape hatch (
as Id<"objectDefs">) masks this design weakness. Instead, consider either:
- Making
RecordDetailsRenderArgs.recordoptional (RecordDetailRecord | undefined) so adapters in the!objectDefbranch explicitly handle absence, or- Removing the fallback record entirely and passing
record: undefined— clearer intent for adapter authors.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/admin/shell/RecordSidebar.tsx` around lines 513 - 536, The code creates a synthetic fallbackAdapterRecord (and adapterRecord) with bogus objectDefId and zero timestamps and passes it into adapter?.renderDetailsTab, which can silently hide bugs; remove fallbackAdapterRecord and adapterRecord, pass the real record (which may be undefined) into adapter?.renderDetailsTab instead, and update the type of RecordDetailsRenderArgs.record from RecordDetailRecord to RecordDetailRecord | undefined (or make it optional) so adapters must explicitly handle the absence of a record; adjust any callers of renderDetailsTab accordingly to accept an undefined record.convex/payments/rotessa/types.ts (1)
224-230: ClarifyfiltervsstatusonRotessaTransactionReportQuery.Both optional fields share
RotessaReportStatusFilterand there's no encoded rule about which wins if a caller sets both. If this mirrors Rotessa accepting either alias, a one-line JSDoc noting precedence (or narrowing to the one the client actually sends) would save future readers the trip to the Rotessa docs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/payments/rotessa/types.ts` around lines 224 - 230, The two optional fields on RotessaTransactionReportQuery (filter and status, both typed RotessaReportStatusFilter) are ambiguous about precedence; add a one-line JSDoc above the RotessaTransactionReportQuery interface (or remove one alias) clarifying which field the client sends or which takes precedence when both are provided (e.g., "If both filter and status are set, status takes precedence"), and update any code that constructs queries to use the canonical property (filter or status) referenced in the doc so callers and future readers know the intended single source of truth.convex/payments/origination/bootstrap.ts (1)
317-341: Duplicate full-table scan and O(n)includeson obligations.
listExistingMortgageObligationsis called a second time here aftergenerateInitialMortgageObligationsalready walked/returnedobligationIds. With the obligation set from the first call you can avoid the second.collect()and also drop the O(n²).includes()filter. Low-impact while obligation counts stay small, but trivial to fix.♻️ Suggested refactor
Return the persisted docs (or a lookup map) from
generateInitialMortgageObligationsand pass them directly intoensureDefaultEntriesForObligationsImpl, replacing.includes()with aSetif you still need to filter.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/payments/origination/bootstrap.ts` around lines 317 - 341, The code is doing a duplicate full-table scan by calling listExistingMortgageObligations after generateInitialMortgageObligations and then using an O(n) .includes() filter; modify generateInitialMortgageObligations to return the persisted obligation documents (or a Map of _id -> doc) in addition to obligationIds, then in bootstrapOriginationPayments use that returned list/map instead of calling listExistingMortgageObligations again and replace the .includes() check with a Set or direct lookup (update the callsite to pass the filtered obligation objects into ensureDefaultEntriesForObligationsImpl and adjust any affected types like GenerateInitialMortgageObligationsInput/BootstrapOriginationPaymentsResult as needed).convex/listings/projection.ts (1)
75-106:normalizeListingOverridescannot express "explicitly clear".For string fields,
hasOwn(overrides, "title") ? trimToUndefined(...) : undefinedmaps both "not present" and "present but empty/whitespace" toundefined.buildCuratedFieldPatchthen treatsundefinedas "keep existing". Net effect: a caller cannot use the overrides payload to clear a previously settitle/seoSlug/adminNotes. FordisplayOrderandfeaturedthe same ambiguity exists, sinceundefinedin the draft also means "keep existing".If this is intentional (clearing happens via
updateListingCuration), a short comment would prevent a future caller from assuming they can clear via this path. Otherwise, consider a sentinel (e.g.,null) to distinguish clear-vs-keep.Also applies to: 231-270
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/listings/projection.ts` around lines 75 - 106, normalizeListingOverrides currently maps both "field absent" and "field present but empty" to undefined (via trimToUndefined), which buildCuratedFieldPatch interprets as "keep existing", preventing callers from clearing fields like title/seoSlug/adminNotes or unsetting displayOrder/featured; either explicitly support a sentinel for "clear" (e.g., preserve null as a distinct value and pass null through for string fields and primitives) or add a clear comment documenting that clearing must be done via updateListingCuration and not via normalizeListingOverrides; to fix, update normalizeListingOverrides to return null for explicit clear intent (e.g., when hasOwn(...) and trimmed value === undefined treat as null) and ensure consumers (buildCuratedFieldPatch) treat null as "clear" while undefined remains "keep", and/or add a short comment in normalizeListingOverrides referencing buildCuratedFieldPatch and updateListingCuration to make the intended behavior explicit.convex/admin/origination/collections.ts (1)
378-432: O(entries × obligationIds)db.getfan-out inregularInterestEntryIds.For each eligible plan entry you issue one
db.getper obligation id in parallel, which is fine at small N but will compound quickly if a case accumulates many future entries or multi-obligation entries. Consider deduping obligation ids across entries, batching the lookups once, and building the regular-interest filter off an in-memory map.Also: the outer loop (lines 424-432) breaks on the first non-matching entry once collection has started, which means a stray/non-regular_interest entry interleaved with valid ones silently truncates the activation set. If that's intentional (Rotessa refuses gaps), a short comment would help future readers; otherwise filter rather than break.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/admin/origination/collections.ts` around lines 378 - 432, The current implementation builds regularInterestEntryIds by calling ctx.db.get for every obligationId per entry (in regularInterestEntryIds computation over eligiblePlanEntries), which can fan out massively; instead dedupe all obligationIds from eligiblePlanEntries into a single set, batch-fetch those rows once via ctx.db.get (or a batched query), build an in-memory map of obligationId -> obligation and then re-evaluate each entry against that map to produce regularInterestEntryIds; also address the outer loop over eligiblePlanEntries that currently breaks on the first non-matching entry (activationPlanEntryIds push loop) by either adding a clarifying comment if intentional (Rotessa gap semantics) or replacing the break with a continue/filter so non-regular entries don’t prematurely truncate activationPlanEntryIds.
🤖 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/admin/origination/collections.ts`:
- Around line 518-554: The code currently always sets incrementRetryCount when
calling patchCollectionsActivationStateRef, which increments retryCount even on
first-time activations; change the patch call to only set incrementRetryCount:
true when the existing activation status indicates a retry (e.g.,
collectionsDraft.activationStatus === "failed"), otherwise pass
incrementRetryCount: false (or omit the field); update the initial mutation (the
one that sets activationStatus: "activating") and any other calls that currently
unconditionally set incrementRetryCount to use this conditional logic,
referencing collectionsDraft and patchCollectionsActivationStateRef so
first-time activations don't increment the retry counter.
In `@convex/admin/origination/commit.ts`:
- Around line 688-699: The call to
internal.admin.origination.collections.activateCommittedCaseCollections from
commitCase is making post-commit Rotessa activation blocking and can cause
commitCase to reject after finalizeCommit persisted the commit; change this so
activation failures are swallowed and only logged: after finalizeCommit
completes, invoke
ctx.runAction(internal.admin.origination.collections.activateCommittedCaseCollections,
{ caseId: args.caseId, viewerUserId: input.viewerUserId }) but wrap it in a
non-propagating try/catch (or attach .catch) so any rejection is caught and
logged (use the existing request/context logger or console.error) and do not
rethrow, keeping collectionsDraft.activationStatus handling to the activation
flow.
In `@convex/crm/detailContextQueries.ts`:
- Around line 69-70: The code currently does an unbounded scan with
ctx.db.query("documentAssets").collect(); instead, resolve only the IDs in
args.listing.publicDocumentIds via an indexed lookup or direct get(s) and apply
the same org/access boundary used for this listing context (the listing's
orgId/access fields) so you don't return global rows. Replace the collect() call
with per-id fetches or a single indexed query (e.g., using the documentAssets
index keyed by fileRef/publicDocumentId) filtered by the listing's org/access
values so only the requested documents for that org are returned.
- Around line 21-42: The helper requireListingForDetailContext currently lets
listings with no mortgage pass without org ownership checks; update it to fail
closed by ensuring either the linked mortgage exists and has mortgage.orgId ===
ctx.viewer.orgId OR the listing itself carries ownership (e.g., listing.orgId
=== ctx.viewer.orgId). Concretely, inside requireListingForDetailContext after
fetching listing and optionally mortgage, if (!mortgage && listing.orgId !==
orgId) throw new ConvexError("Listing not found or access denied"); and if
(mortgage && mortgage.orgId !== orgId) throw the same error; this enforces org
ownership whether via mortgage or listing.orgId.
In `@convex/payments/origination/bootstrap.ts`:
- Around line 96-115: The loop building dueDates uses implicit if/else branches
for args.paymentFrequency (checking "monthly", "bi_weekly",
"accelerated_bi_weekly") and falls back to a 7-day advance for the rest; replace
this with an exhaustive switch on args.paymentFrequency (the PaymentFrequency
type) that explicitly handles "monthly", "weekly", "bi_weekly", and
"accelerated_bi_weekly" and advances currentDate via advanceMonth or adding
MS_PER_DAY multiples accordingly; add a default case that asserts a never type
(e.g., const _exhaustiveCheck: never = args.paymentFrequency) so TypeScript will
force you to update the switch if PaymentFrequency gains new values, keeping
dueDates logic explicit and type-safe.
In `@convex/payments/recurringSchedules/rotessaCustomerReference.ts`:
- Around line 9-37: hasRotessaCustomerReference currently treats NaN/Infinity
and empty/whitespace strings as valid; update it to mirror
resolveRotessaCustomerReference by checking
Number.isFinite(bankAccountMetadata?.rotessaCustomerId) for rotessaCustomerId
and ensuring rotessaCustomerCustomIdentifier and rotessaCustomIdentifier are
strings with non-empty trimmed content (e.g. .trim().length > 0); also update
resolveRotessaCustomerReference to reject empty/whitespace custom identifiers by
validating trimmed length before returning customIdentifier so both functions
are consistent (refer to hasRotessaCustomerReference,
resolveRotessaCustomerReference, rotessaCustomerId,
rotessaCustomerCustomIdentifier, rotessaCustomIdentifier).
In `@convex/payments/rotessa/api.ts`:
- Around line 17-26: The parseRotessaNumericId function currently uses
Number.parseInt which accepts prefixes like "123abc" — update
parseRotessaNumericId to first validate that value is a full positive integer
string (e.g. match /^[0-9]+$/) and throw the same RotessaRequestError for any
non-matching or empty input, then safely parse (Number.parseInt or
Number(value)) and return the integer; keep the existing error object shape
(message, method, path) and the function name parseRotessaNumericId so callers
still work.
In `@convex/payments/rotessa/client.ts`:
- Around line 614-621: getRotessaClient currently ignores configInput after the
first call because it returns the cached rotessaClient; change it to guard
against accidental overrides by checking if rotessaClient is already set and
configInput is non-empty (e.g. inspect Object.keys(configInput).length > 0 or
specific fields like apiKey/baseUrl/fetchFn/reporter) and throw a clear error or
assert informing callers to use createRotessaClient or resetRotessaClient for
custom configs; update getRotessaClient, referencing the functions
rotessaClient, getRotessaClient, createRotessaClient, resetRotessaClient and the
type RotessaClientConfigInput so the guard is applied before returning the
cached instance.
In `@src/components/admin/shell/dedicated-detail-panels.tsx`:
- Around line 260-280: The current useEffect that calls
setCurationForm(buildListingCurationFormState(listing)) runs on every change to
listing (which is a live query) and overwrites any unsaved edits; modify it so
it only seeds/reset the form when the listing identity changes or when the form
is not dirty: track the current listing id (e.g., via a ref or state like
initialListingId) and only call setCurationForm when listing?.id !==
initialListingId (and then update initialListingId), or add a dirty flag (e.g.,
curationFormDirty) and skip overwriting if dirty; reference useEffect,
setCurationForm, buildListingCurationFormState, detailContext/listing and the
curationForm state to locate where to implement the check.
---
Outside diff comments:
In `@src/components/admin/origination/ListingCurationStep.tsx`:
- Around line 85-94: Update the stale placeholder in ListingCurationStep: find
the textarea handling nextDraft.marketplaceCopy (inside the ListingCurationStep
component) and replace the placeholder string "Operator-only curation copy for
future listing projection." with wording that reflects Phase-5 behavior, e.g.
"Operator-only curation copy for listing that will be materialized on commit."
to match the other updated copy and the upsertMortgageListingProjection flow.
---
Nitpick comments:
In `@convex/admin/origination/collections.ts`:
- Around line 378-432: The current implementation builds regularInterestEntryIds
by calling ctx.db.get for every obligationId per entry (in
regularInterestEntryIds computation over eligiblePlanEntries), which can fan out
massively; instead dedupe all obligationIds from eligiblePlanEntries into a
single set, batch-fetch those rows once via ctx.db.get (or a batched query),
build an in-memory map of obligationId -> obligation and then re-evaluate each
entry against that map to produce regularInterestEntryIds; also address the
outer loop over eligiblePlanEntries that currently breaks on the first
non-matching entry (activationPlanEntryIds push loop) by either adding a
clarifying comment if intentional (Rotessa gap semantics) or replacing the break
with a continue/filter so non-regular entries don’t prematurely truncate
activationPlanEntryIds.
In `@convex/crm/entityAdapterRegistry.ts`:
- Around line 277-287: The computed field object for listingPaymentSummary is
missing an explicit materializationMode and should set materializationMode:
"sync" to match its siblings; update the object with expressionKey
"listingPaymentSummary" (fieldName "paymentSummary") to include
materializationMode: "sync" so behavior remains explicit and robust if
evaluateComputedFieldValue changes.
In `@convex/listings/projection.ts`:
- Around line 75-106: normalizeListingOverrides currently maps both "field
absent" and "field present but empty" to undefined (via trimToUndefined), which
buildCuratedFieldPatch interprets as "keep existing", preventing callers from
clearing fields like title/seoSlug/adminNotes or unsetting
displayOrder/featured; either explicitly support a sentinel for "clear" (e.g.,
preserve null as a distinct value and pass null through for string fields and
primitives) or add a clear comment documenting that clearing must be done via
updateListingCuration and not via normalizeListingOverrides; to fix, update
normalizeListingOverrides to return null for explicit clear intent (e.g., when
hasOwn(...) and trimmed value === undefined treat as null) and ensure consumers
(buildCuratedFieldPatch) treat null as "clear" while undefined remains "keep",
and/or add a short comment in normalizeListingOverrides referencing
buildCuratedFieldPatch and updateListingCuration to make the intended behavior
explicit.
In `@convex/payments/origination/bootstrap.ts`:
- Around line 317-341: The code is doing a duplicate full-table scan by calling
listExistingMortgageObligations after generateInitialMortgageObligations and
then using an O(n) .includes() filter; modify generateInitialMortgageObligations
to return the persisted obligation documents (or a Map of _id -> doc) in
addition to obligationIds, then in bootstrapOriginationPayments use that
returned list/map instead of calling listExistingMortgageObligations again and
replace the .includes() check with a Set or direct lookup (update the callsite
to pass the filtered obligation objects into
ensureDefaultEntriesForObligationsImpl and adjust any affected types like
GenerateInitialMortgageObligationsInput/BootstrapOriginationPaymentsResult as
needed).
In `@convex/payments/rotessa/types.ts`:
- Around line 224-230: The two optional fields on RotessaTransactionReportQuery
(filter and status, both typed RotessaReportStatusFilter) are ambiguous about
precedence; add a one-line JSDoc above the RotessaTransactionReportQuery
interface (or remove one alias) clarifying which field the client sends or which
takes precedence when both are provided (e.g., "If both filter and status are
set, status takes precedence"), and update any code that constructs queries to
use the canonical property (filter or status) referenced in the doc so callers
and future readers know the intended single source of truth.
In `@src/components/admin/shell/RecordSidebar.tsx`:
- Around line 513-536: The code creates a synthetic fallbackAdapterRecord (and
adapterRecord) with bogus objectDefId and zero timestamps and passes it into
adapter?.renderDetailsTab, which can silently hide bugs; remove
fallbackAdapterRecord and adapterRecord, pass the real record (which may be
undefined) into adapter?.renderDetailsTab instead, and update the type of
RecordDetailsRenderArgs.record from RecordDetailRecord to RecordDetailRecord |
undefined (or make it optional) so adapters must explicitly handle the absence
of a record; adjust any callers of renderDetailsTab accordingly to accept an
undefined record.
In `@src/test/admin/origination/origination-workflow.test.tsx`:
- Around line 163-167: Replace the duplicated literal in the test snapshot with
the shared constant ORIGINATION_PROVIDER_MANAGED_COLLECTIONS_WARNING from
convex/admin/origination/validators.ts: import that constant into
origination-workflow.test.tsx and use it for the reviewWarnings snapshot entry
so the test references the single source of truth instead of an inline string.
🪄 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: 3c71d04d-b1a5-41e6-9da7-a6e6dde42c29
⛔ Files ignored due to path filters (1)
convex/_generated/api.d.tsis excluded by!**/_generated/**
📒 Files selected for processing (45)
convex/admin/origination/collections.tsconvex/admin/origination/commit.tsconvex/admin/origination/validators.tsconvex/crm/detailContextQueries.tsconvex/crm/entityAdapterRegistry.tsconvex/crm/entityViewFields.tsconvex/listings/create.tsconvex/listings/curation.tsconvex/listings/projection.tsconvex/listings/validators.tsconvex/mortgages/activateMortgageAggregate.tsconvex/payments/collectionPlan/readModels.tsconvex/payments/origination/bootstrap.tsconvex/payments/recurringSchedules/activation.tsconvex/payments/recurringSchedules/rotessaCustomerReference.tsconvex/payments/rotessa/api.tsconvex/payments/rotessa/client.tsconvex/payments/rotessa/manifest.tsconvex/payments/rotessa/types.tsconvex/schema.tsconvex/test/moduleMaps.tsconvex/test/originationE2e.tsdocs/architecture/admin-origination-workspace.mde2e/helpers/origination.tse2e/origination/listing-projection.spec.tse2e/origination/payment-bootstrap.spec.tssrc/components/admin/origination/CollectionsStep.tsxsrc/components/admin/origination/ListingCurationStep.tsxsrc/components/admin/origination/MortgageTermsStep.tsxsrc/components/admin/origination/OriginationWorkspacePage.tsxsrc/components/admin/origination/ParticipantsStep.tsxsrc/components/admin/origination/ReviewStep.tsxsrc/components/admin/origination/workflow.tssrc/components/admin/shell/RecordSidebar.tsxsrc/components/admin/shell/dedicated-detail-panels.tsxsrc/components/admin/shell/entity-view-adapters.tsxsrc/lib/admin-origination.tssrc/test/admin/mortgage-dedicated-details.test.tsxsrc/test/admin/origination/collections-step.test.tsxsrc/test/admin/origination/origination-workflow.test.tsxsrc/test/convex/admin/origination/commit.test.tssrc/test/convex/admin/origination/validators.test.tssrc/test/convex/listings/create.test.tssrc/test/convex/listings/projection.test.tssrc/test/convex/payments/rotessaApiClient.test.ts
Open
Connorbelez
added a commit
that referenced
this pull request
Apr 19, 2026
Keep commitCase committed after finalizeCommit even if the follow-up collections activation action rejects unexpectedly. Refs #412
Connorbelez
force-pushed
the
04-17-eng-284_eng-285
branch
from
5e245e7 to
54d5c25
Compare
This was referenced Apr 20, 2026
Open
Connorbelez
added a commit
that referenced
this pull request
Apr 20, 2026
Keep commitCase committed after finalizeCommit even if the follow-up collections activation action rejects unexpectedly. Refs #412
This was referenced Apr 20, 2026
Open
Open
Connorbelez
force-pushed
the
04-17-eng-284_eng-285
branch
from
54d5c25 to
e2e2820
Compare
This was referenced Apr 20, 2026
This was referenced Apr 21, 2026
Open
Open
Open
Keep commitCase committed after finalizeCommit even if the follow-up collections activation action rejects unexpectedly. Refs #412
Connorbelez
force-pushed
the
04-17-eng-284_eng-285
branch
from
e2e2820 to
184b2eb
Compare
This was referenced Apr 21, 2026
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.
Summary by CodeRabbit
Release Notes
New Features
Improvements