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
Open
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. |
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
Adds Phase 7 “deal package” materialization (immutable deal-time document packages derived from private mortgage blueprints), along with admin + lender UI surfaces and supporting tests/e2e coverage.
Changes:
- Introduces
dealDocumentPackages/dealDocumentInstancesschema + a newconvex/documents/dealPackagesmodule to materialize immutable deal-time packages (static references + generated non-signable PDFs + signable placeholders) and support retries. - Adds admin deal detail panel support (package status, instances, retry button) and a lender portal route
/lender/deals/$dealIdto view only available/downloadable private docs. - Adds unit/integration tests and an e2e spec validating package immutability and “no listing public docs” behavior; updates architecture docs.
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/lender/deal-detail-page.test.tsx | Adds lender portal UI test for showing only available private docs. |
| src/test/convex/documents/dealPackages.test.ts | Adds Convex tests for package creation, retry behavior, and frozen blueprint snapshots. |
| src/test/admin/deal-dedicated-details.test.tsx | Adds admin UI test for package status + retry action wiring. |
| src/routes/lender.deals.tsx | Adds lender /lender/deals parent route outlet. |
| src/routes/lender.deals.$dealId.tsx | Adds lender deal detail route rendering LenderDealDetailPage. |
| src/routeTree.gen.ts | Registers new lender deal routes in generated route tree. |
| src/lib/document-engine/contracts.ts | Extends supported deal document variable keys used for template generation. |
| src/components/lender/deals/LenderDealDetailPage.tsx | Implements lender deal package UI (snapshot + downloadable docs + placeholders count). |
| src/components/admin/shell/entity-view-adapters.tsx | Wires DealsDedicatedDetails into admin entity detail adapters. |
| src/components/admin/shell/dedicated-detail-panels.tsx | Implements DealsDedicatedDetails panel (package metrics, instances, retry). |
| e2e/origination/deal-package-materialization.spec.ts | Adds Playwright e2e validating package immutability + PDF accessibility + listing isolation. |
| e2e/helpers/origination.ts | Adds e2e client helpers for seeding blueprints/templates/assets and driving package actions. |
| docs/architecture/admin-origination-workspace.md | Documents Phase 7 deal package contract and lender portal surface. |
| convex/test/moduleMaps.ts | Adds module map entry for documents/dealPackages in Convex test harness. |
| convex/test/dealPackageE2e.ts | Adds Convex admin actions/mutations to support e2e seeding and cleanup. |
| convex/schema.ts | Adds dealDocumentPackages and dealDocumentInstances tables + indexes. |
| convex/engine/effects/dealClosingEffects.ts | Hooks DEAL_LOCKED effect to run real package materialization action (replaces stub). |
| convex/documents/dealPackages.ts | Core implementation: snapshotting, work item processing, generation, retries, surfaces. |
| convex/documents/contracts.ts | Adds validators/types for deal package + instance kinds/statuses and blueprint snapshots. |
| convex/deals/queries.ts | Adds getPortalDealDetail (dealQuery) including package surface for lender portal. |
| convex/crm/detailContextQueries.ts | Adds getDealDetailContext (crmQuery) including package surface + parties + audit. |
| convex/_generated/api.d.ts | Updates generated API types for new modules/endpoints. |
💡 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: 11
🧹 Nitpick comments (1)
src/routes/lender.deals.$dealId.tsx (1)
1-6: Consider lazy-loading the deal detail page.This route shell eagerly imports
LenderDealDetailPage; moving the component into a.lazy.tsxroute module or usinglazyRouteComponentpreserves route-level code splitting for this heavier detail surface.As per coding guidelines, "Use the
componentproperty with lazy-loaded components vialazyRouteComponentor dynamic imports for code-splitting to reduce initial bundle size."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/lender.deals`.$dealId.tsx around lines 1 - 6, The route is eagerly importing LenderDealDetailPage; change it to a lazy-loaded route component to enable route-level code splitting. Replace the static import of LenderDealDetailPage and the current use of component: RouteComponent with a lazily created component using your router's lazy helper (e.g., lazyRouteComponent or a dynamic import) so the createFileRoute("/lender/deals/$dealId") uses the lazy-loaded module; reference the Route export, createFileRoute call, and LenderDealDetailPage symbol when making the change and ensure the lazy import resolves to the LenderDealDetailPage export.
🤖 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/deals/queries.ts`:
- Around line 285-286: The query currently returns packageSurface directly as
documentPackage/documentInstances; instead project a lender-safe DTO instead of
exposing internal fields. Replace use of packageSurface.package and
packageSurface.instances with a mapped object (e.g., build a lenderDocumentDTO)
that selects only lender-facing fields (title/label, status,
createdAt/updatedAt, signer list metadata, and any permitted metadata) and omits
internal properties such as lastError, blueprint/template IDs,
asset/generated-document IDs, archived timestamps, raw URLs, and any internal
flags. Update the code paths that set documentPackage and documentInstances to
use this mapped DTO (use a mapping helper function like toLenderDocumentDTO if
helpful) so only approved attributes are returned to the lender portal.
In `@convex/documents/dealPackages.ts`:
- Around line 1039-1069: In createStaticReferenceInstance, after obtaining
assetId via getWorkItemAssetId(workItem) and before calling
createPackageInstance with status "available", verify the referenced
documentAssets row still exists (e.g., query the documentAssets collection/table
for that assetId); if the row is missing or corrupted, call
createPackageInstance with status "generation_failed" and an explanatory
lastError instead of marking it "available". Update
createStaticReferenceInstance to perform this existence check and branch
accordingly so packages aren’t finalized as available when documentAssets.url
could be null.
- Around line 1417-1428: getPortalDocumentPackage currently returns the full
admin package surface from buildPackageSurface for any deal-access viewer;
change it so the portal response only contains downloadable rows by filtering
out failed rows, signable placeholders, archived rows and rows with lastError or
unresolved URLs, keeping only instances marked available with a resolvable URL.
Implement this by either adding a boolean flag to buildPackageSurface (e.g.,
includeAdminFields = false) or by calling buildPackageSurface(ctx, args.dealId)
and post-filtering its returned package rows to retain only rows where
instance.available === true and a resolvable URL field is present, while leaving
the original unfiltered helper intact for admin/detail contexts; keep the
existing access check using canAccessDeal and getPortalDocumentPackage
signature.
- Around line 1156-1164: createNonSignableGeneratedInstance currently returns
early when sourceBlueprintSnapshot.templateId is missing, causing the package
member to vanish and summarizePackageStatus() to think the package is ready;
instead, record a failed generated instance for the work item so retries/admins
surface the problem. In createNonSignableGeneratedInstance create and persist a
generated-instance entry tied to the given workItem (use the existing
runtime/save methods used elsewhere in this file) with a status of
"generation_failed" and an explanatory message referencing the missing
templateId (include sourceBlueprintSnapshot.id or similar identifiers), then
return; ensure summarizePackageStatus() will see this member as failed.
Reference symbols: createNonSignableGeneratedInstance, runtime, workItem,
sourceBlueprintSnapshot, summarizePackageStatus.
- Around line 667-864: The exported Convex handlers in this diff are created
with raw internalQuery/internalMutation calls instead of the repository's
fluent-convex builder pattern; update each exported function
(resolveDealParticipantSnapshotInternal, resolveDealDocumentVariablesInternal,
resolveDealDocumentSignatoriesInternal, getPackageByDealInternal,
listPackageInstancesInternal, listActivePackageBlueprintInputsInternal,
ensurePackageHeaderInternal, createDealDocumentInstance,
archiveDealDocumentInstance, insertGeneratedDocumentInternal,
finalizePackageInternal) to use the fluent builder form (the existing
args/handler stays the same) and append the explicit visibility call
(.internal()) as per project convention; apply the same change to the other
affected export block around the 1395-1403 area.
In `@convex/test/dealPackageE2e.ts`:
- Around line 616-640: The current cleanup fetches all
mortgageDocumentBlueprints for the mortgage (using query on
"mortgageDocumentBlueprints" withIndex "by_mortgage_created_at" and
eq("mortgageId", args.mortgageId")), which can remove unrelated blueprints;
change the cleanup to only delete blueprints created by this test by further
filtering the query with the assetId and/or templateId used by this scenario (or
accept explicit blueprint IDs returned from the seed helper and delete only
those IDs). Update the code that builds blueprintRows (or replace blueprintRows
with a list of explicit blueprint IDs from the seed helper) so the subsequent
loop only deletes matching blueprint rows instead of every blueprint for the
mortgage.
- Around line 288-350: The exported helpers (resolveViewerUserIdInternal,
insertStaticBlueprintFixtureInternal,
insertNonSignableTemplateBlueprintFixtureInternal) must use the fluent
visibility builder rather than raw internal* constructors; replace the raw
internalQuery/internalMutation usage with the corresponding builder form and
append the explicit .internal() visibility marker (e.g., use
query(...).internal() or mutation(...).internal() as appropriate) so each
exported Convex entry ends with .internal().
In `@e2e/origination/deal-package-materialization.spec.ts`:
- Around line 286-287: The test currently calls page.reload() then immediately
asserts expect(page.getByText("Late addendum")).toHaveCount(0), which can race
with the page load; after calling page.reload() (or before the assertion), await
a stable indicator that the detail view has finished loading—e.g., await
page.waitForLoadState('networkidle') or await a known detail-view locator (like
a heading/locator for the deal detail) to be visible—then run
expect(page.getByText("Late addendum")).toHaveCount(0) to ensure the absence
check happens after the view is fully loaded.
- Around line 75-84: The teardown currently aborts committed origination cleanup
if cleanupDealPackageScenario throws; make both cleanups best-effort by invoking
them independently and handling errors (e.g., use Promise.allSettled on the two
cleanup promises or wrap each call in its own try/catch). Specifically, when
accessToken is present, call createOriginationE2eClient(accessToken) and
schedule/await cleanupDealPackageScenario(dealCleanup) and
cleanupCommittedOrigination(caseId) as independent operations so a failure in
one does not skip the other, and log or swallow errors appropriately.
In `@src/components/admin/shell/dedicated-detail-panels.tsx`:
- Around line 1080-1191: The retry handler allows duplicate submissions because
there's no in-flight guard; add a local boolean state (e.g., isRetrying) used by
handleRetryPackageGeneration and the Button to prevent re-entry and disable the
button while the async retryPackageGeneration runs: at start of
handleRetryPackageGeneration check/return if isRetrying, set isRetrying = true
before awaiting retryPackageGeneration({ dealId }), set isRetrying = false in a
finally block, and pass disabled={isRetrying} (or similar) to the Button (and
optionally short-circuit to avoid duplicate toast calls).
In `@src/components/lender/deals/LenderDealDetailPage.tsx`:
- Around line 96-108: The "Deal not found" branch in LenderDealDetailPage checks
if detail === null but getPortalDealDetail currently throws when a deal/mortgage
is missing, so this UI path never runs; update the data layer or the component
to match the contract: either modify getPortalDealDetail to return null for
404/not-found cases (preserve its signature and update any callers to handle
null) or remove the null-branch in LenderDealDetailPage and let the route/error
boundary handle thrown errors—locate getPortalDealDetail and the detail variable
in LenderDealDetailPage and make the chosen behavior consistent across both
places.
---
Nitpick comments:
In `@src/routes/lender.deals`.$dealId.tsx:
- Around line 1-6: The route is eagerly importing LenderDealDetailPage; change
it to a lazy-loaded route component to enable route-level code splitting.
Replace the static import of LenderDealDetailPage and the current use of
component: RouteComponent with a lazily created component using your router's
lazy helper (e.g., lazyRouteComponent or a dynamic import) so the
createFileRoute("/lender/deals/$dealId") uses the lazy-loaded module; reference
the Route export, createFileRoute call, and LenderDealDetailPage symbol when
making the change and ensure the lazy import resolves to the
LenderDealDetailPage export.
🪄 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: 12b16425-fe46-4aa7-a803-a9d3c2bd8785
⛔ Files ignored due to path filters (1)
convex/_generated/api.d.tsis excluded by!**/_generated/**
📒 Files selected for processing (21)
convex/crm/detailContextQueries.tsconvex/deals/queries.tsconvex/documents/contracts.tsconvex/documents/dealPackages.tsconvex/engine/effects/dealClosingEffects.tsconvex/schema.tsconvex/test/dealPackageE2e.tsconvex/test/moduleMaps.tsdocs/architecture/admin-origination-workspace.mde2e/helpers/origination.tse2e/origination/deal-package-materialization.spec.tssrc/components/admin/shell/dedicated-detail-panels.tsxsrc/components/admin/shell/entity-view-adapters.tsxsrc/components/lender/deals/LenderDealDetailPage.tsxsrc/lib/document-engine/contracts.tssrc/routeTree.gen.tssrc/routes/lender.deals.$dealId.tsxsrc/routes/lender.deals.tsxsrc/test/admin/deal-dedicated-details.test.tsxsrc/test/convex/documents/dealPackages.test.tssrc/test/lender/deal-detail-page.test.tsx
Connorbelez
force-pushed
the
04-18-eng-286
branch
from
67f47b5 to
a4f5f76
Compare
Connorbelez
force-pushed
the
04-18-eng-287
branch
from
d18ea57 to
1888759
Compare
This was referenced Apr 20, 2026
Open
Open
Open
Connorbelez
force-pushed
the
04-18-eng-287
branch
from
455cd01 to
d70fa17
Compare
Connorbelez
force-pushed
the
04-18-eng-286
branch
from
4c36123 to
62afe07
Compare
This was referenced Apr 20, 2026
This was referenced Apr 21, 2026
Open
Open
Open
Connorbelez
force-pushed
the
04-18-eng-287
branch
from
d70fa17 to
8e66b5b
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