Merged
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, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThis PR implements system object bootstrapping for CRM entities (mortgage, borrower, lender, broker, deal, obligation) by introducing metadata and idempotent creation logic, integrating organization-creation webhooks to trigger automatic setup, and enhancing record querying to support native table pagination and system object retrieval. Changes
Sequence DiagramsequenceDiagram
participant WorkOS as WorkOS API
participant Handler as auth.ts<br/>(organization.created)
participant Scheduler as Convex Scheduler
participant Bootstrap as bootstrap.ts<br/>(bootstrapSystemObjects)
participant DB as Convex Database
WorkOS->>Handler: organization.created event<br/>(event.data.id)
Handler->>DB: upsertOrganization(event.data)
DB-->>Handler: organization updated/created
Handler->>Scheduler: ctx.scheduler.runAfter(0,<br/>bootstrapSystemObjects,<br/>{orgId: event.data.id})
Scheduler-->>Handler: scheduled (returns)
Handler-->>WorkOS: webhook complete
Note over Scheduler,Bootstrap: Async execution in<br/>separate transaction
Scheduler->>Bootstrap: invoke(ctx, {orgId})
Bootstrap->>DB: query existing objectDefs<br/>for orgId
DB-->>Bootstrap: [existing objects]
loop For each SYSTEM_OBJECT_CONFIGS
Bootstrap->>DB: insert objectDefs<br/>(if not exists)
DB-->>Bootstrap: objectDef created
Bootstrap->>DB: insert viewDefs<br/>(default view)
DB-->>Bootstrap: viewDef created
loop For each field in object
Bootstrap->>DB: insert fieldDefs<br/>(nativeReadOnly: true)
Bootstrap->>DB: derive & insert<br/>fieldCapabilities
Bootstrap->>DB: insert viewFields
end
end
Bootstrap-->>Scheduler: return {created, skipped}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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.
Sorry @Connorbelez, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
![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
Implements the initial “system objects” foundation for CRM by adding native-table adapters (ENG-255) and introducing automated bootstrapping of system objectDefs/fieldDefs for new organizations (ENG-256), including a webhook trigger on org creation.
Changes:
- Added system adapter utilities (
columnResolver,queryAdapter) and wiredqueryRecordsto query native tables for system objects. - Added system object bootstrap mutation(s) to create system objectDefs/fieldDefs/views + scheduled bootstrap from
organization.createdwebhook. - Added ENG-255/ENG-256 spec/chunk planning docs.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
convex/crm/systemAdapters/columnResolver.ts |
Resolves native column paths (including nested paths) with basic coercion for date/datetime fields. |
convex/crm/systemAdapters/queryAdapter.ts |
Adds switch-based routing to query 6 native tables and assemble UnifiedRecord[] for system objects. |
convex/crm/recordQueries.ts |
Routes system-object queryRecords through the native adapter; updates system-object behavior in getRecord/searchRecords. |
convex/crm/systemAdapters/bootstrap.ts |
Adds idempotent system object bootstrapping (internal + admin entrypoint) creating objectDefs/fieldDefs/capabilities/views. |
convex/auth.ts |
Schedules system bootstrap after WorkOS organization.created upsert. |
specs/ENG-256/** |
ENG-256 task lists + chunk context/manifest docs. |
specs/ENG-255/** |
ENG-255 task lists + chunk context/manifest docs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Owner
Author
|
@coderabbitai please review |
Contributor
|
🧠 Learnings used✅ Actions performedReview triggered.
|
![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: 2
🧹 Nitpick comments (2)
convex/crm/recordQueries.ts (1)
590-593: Prefer explicit unsupported signaling over unconditional empty results.Always returning
[]for system-object search makes the UI look like “no matches” instead of “not supported yet.” Consider throwing a specificConvexError(or adding a bounded fallback) so behavior is explicit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/crm/recordQueries.ts` around lines 590 - 593, The current branch that handles native/system-object search returns an unconditional empty array which makes the UI show “no matches”; instead modify the branch in the search handler (e.g., the function handling system-object search in recordQueries.ts—searchRecords or the native-table search branch) to explicitly signal unsupported behavior: throw a ConvexError with a clear message like "system-object search not supported yet" (or return a bounded fallback result plus a flag), and ensure the calling code/UI can detect and surface this unsupported state rather than treating it as an empty result set.convex/crm/systemAdapters/queryAdapter.ts (1)
77-116:updatedAtalways uses_creationTime— verify this is intentional.The
updatedAtfield is set todoc._creationTime(line 113) rather than attempting to use a nativeupdatedAtfield. This means records will show their creation time as the update time.Per the spec in
context.md, this appears intentional since native tables don't have a consistentupdatedAtfield. However, if any native tables do track updates (e.g.,lastTransitionAton some tables), you may want to use that instead for a more accurate timestamp.💡 Optional: Consider using `lastTransitionAt` where available
return { _id: String(doc._id), _kind: "native" as const, objectDefId: objectDef._id, fields, createdAt: (doc.createdAt as number) ?? (doc._creationTime as number), - updatedAt: doc._creationTime as number, + updatedAt: + (doc.lastTransitionAt as number) ?? + (doc.createdAt as number) ?? + (doc._creationTime as number), };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/crm/systemAdapters/queryAdapter.ts` around lines 77 - 116, The updatedAt is currently always set from doc._creationTime in queryNativeRecords; change it to prefer a native updated timestamp when available (e.g., check doc.updatedAt or doc.lastTransitionAt) and fall back to doc._creationTime if none exists. Update the mapping in queryNativeRecords (and the returned object’s updatedAt) to first look for doc.updatedAt, then doc.lastTransitionAt, then doc._creationTime; keep createdAt logic unchanged (createdAt: (doc.createdAt as number) ?? (doc._creationTime as number)). Ensure types/casts match existing pattern and only adjust the return construction where updatedAt is assigned.
🤖 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/crm/recordQueries.ts`:
- Around line 335-349: The native-system branch (where objectDef.isSystem &&
objectDef.nativeTable) ignores args.paginationOpts.cursor and always returns
continueCursor: null/isDone: true/truncated: false; update this path to accept
and honor args.paginationOpts.cursor and args.paginationOpts.numItems when
calling queryNativeRecords (or extend queryNativeRecords to accept a
cursor/offset), and compute/return the correct pagination metadata
(continueCursor when more rows remain, isDone only when no more records,
truncated if the returned page was cut by numItems) instead of always signalling
completion; keep references to objectDef.isSystem, objectDef.nativeTable,
queryNativeRecords, args.paginationOpts.cursor, args.paginationOpts.numItems and
the continueCursor/isDone/truncated fields so you modify the exact branch and
return shape.
- Around line 471-474: The error message thrown in the unsupported path inside
the getRecord logic (the throw new ConvexError(...) that mentions ENG-256) is
stale; update the string to remove or replace the ENG-256 reference with the
correct follow-up ticket or a generic "future enhancement" note (e.g., "deferred
to follow-up ticket" or "future enhancement") so the ConvexError thrown by
getRecord/system-object handling no longer references ENG-256.
---
Nitpick comments:
In `@convex/crm/recordQueries.ts`:
- Around line 590-593: The current branch that handles native/system-object
search returns an unconditional empty array which makes the UI show “no
matches”; instead modify the branch in the search handler (e.g., the function
handling system-object search in recordQueries.ts—searchRecords or the
native-table search branch) to explicitly signal unsupported behavior: throw a
ConvexError with a clear message like "system-object search not supported yet"
(or return a bounded fallback result plus a flag), and ensure the calling
code/UI can detect and surface this unsupported state rather than treating it as
an empty result set.
In `@convex/crm/systemAdapters/queryAdapter.ts`:
- Around line 77-116: The updatedAt is currently always set from
doc._creationTime in queryNativeRecords; change it to prefer a native updated
timestamp when available (e.g., check doc.updatedAt or doc.lastTransitionAt) and
fall back to doc._creationTime if none exists. Update the mapping in
queryNativeRecords (and the returned object’s updatedAt) to first look for
doc.updatedAt, then doc.lastTransitionAt, then doc._creationTime; keep createdAt
logic unchanged (createdAt: (doc.createdAt as number) ?? (doc._creationTime as
number)). Ensure types/casts match existing pattern and only adjust the return
construction where updatedAt is assigned.
🪄 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: a352cb35-32cf-4a10-ad99-fc3ec6d8e63d
📒 Files selected for processing (17)
convex/auth.tsconvex/crm/recordQueries.tsconvex/crm/systemAdapters/bootstrap.tsconvex/crm/systemAdapters/columnResolver.tsconvex/crm/systemAdapters/queryAdapter.tsspecs/ENG-255/chunks/chunk-01-adapter-infrastructure/context.mdspecs/ENG-255/chunks/chunk-01-adapter-infrastructure/tasks.mdspecs/ENG-255/chunks/chunk-02-integration/context.mdspecs/ENG-255/chunks/chunk-02-integration/tasks.mdspecs/ENG-255/chunks/manifest.mdspecs/ENG-255/tasks.mdspecs/ENG-256/chunks/chunk-01-bootstrap-core/context.mdspecs/ENG-256/chunks/chunk-01-bootstrap-core/tasks.mdspecs/ENG-256/chunks/chunk-02-webhook-integration/context.mdspecs/ENG-256/chunks/chunk-02-webhook-integration/tasks.mdspecs/ENG-256/chunks/manifest.mdspecs/ENG-256/tasks.md
Connorbelez
force-pushed
the
Connorbelez/eng-256-system-bootstrap
branch
from
2141fed to
7d3f5a2
Compare
Merged
Connorbelez
added a commit
that referenced
this pull request
Apr 20, 2026
ENG-255 - CRM system-object adapter work and records-query integration. ENG-256 - CRM system bootstrap for organization setup and admin bootstrap flows. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * System objects (Mortgage, Borrower, Lender, Broker, Deal, Obligation) can now be queried through the standard records interface * System objects are automatically set up when organizations are created * Added admin capability to manually bootstrap system objects * **Bug Fixes** * System object queries no longer return \"not yet implemented\" errors <!-- end of auto-generated comment: release notes by coderabbit.ai -->
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.
ENG-255
ENG-256
Summary by CodeRabbit