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
Owner
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Contributor
📝 WalkthroughWalkthroughIntroduces system adapter infrastructure and integrates it into record handlers: adds Changes
Sequence DiagramsequenceDiagram
participant Client as Client Code
participant QR as recordQueries.queryRecords()
participant QNR as queryNativeRecords()
participant QNT as queryNativeTable()
participant DB as Native DB Tables
participant RCP as resolveColumnPath()
Client->>QR: queryRecords(objectDef, pagination...)
QR->>QNR: isSystem && nativeTable -> queryNativeRecords(...)
QNR->>QNT: queryNativeTable(tableName, orgId, limit)
QNT->>DB: ctx.db.query(...).eq("orgId", orgId).take(limit)
DB-->>QNT: Raw documents
QNT-->>QNR: documents[]
QNR->>RCP: resolveColumnPath(doc, fieldDef) for each mapped field
RCP-->>QNR: resolved values
QNR-->>QR: UnifiedRecord[]
QR-->>Client: { records: UnifiedRecord[], continueCursor: null, isDone: true, truncated: false }
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
![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 Convex CRM module by adding view definition CRUD + child-entity operations (ENG-252) and introducing system-adapter infrastructure to query native tables through the existing UnifiedRecord shape (ENG-255), including wiring native querying into recordQueries.
Changes:
- Added CRM view CRUD (
viewDefs) plus child entity handlers (viewFields,viewFilters,viewKanbanGroups) with org-scoping + audit logging. - Added system-adapter utilities (
columnResolver,queryAdapter) and integrated native querying intorecordQueriesfor system objects. - Added ENG-252 / ENG-255 spec/task breakdown documentation.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
convex/crm/recordQueries.ts |
Routes system objects to native adapter querying; adjusts system behavior for getRecord/searchRecords. |
convex/crm/systemAdapters/columnResolver.ts |
Resolves mapped native column paths (including nested paths + date coercion). |
convex/crm/systemAdapters/queryAdapter.ts |
Switch-based native table querying and assembly into UnifiedRecord[]. |
convex/crm/filterOperatorValidation.ts |
Adds a FieldType → valid-operators mapping and validation helper for filters. |
convex/crm/viewDefs.ts |
Implements create/update/delete/duplicate/list/get for view definitions with capability validation and kanban group auto-creation. |
convex/crm/viewFields.ts |
Implements visibility toggling (create-if-missing), reorder, width setting, and listing for view fields. |
convex/crm/viewFilters.ts |
Implements filter CRUD with operator-vs-fieldType validation. |
convex/crm/viewKanbanGroups.ts |
Implements kanban group reorder, collapse toggle, and list operations. |
specs/ENG-255/** |
ENG-255 task planning and chunk manifests/contexts for adapter + integration work. |
specs/ENG-252/** |
ENG-252 task planning, chunk manifests/contexts, and a type design review doc. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…able check, filter/sort handling, and page size validation Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![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
🤖 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 system-object native shortcut in queryRecords (branch
checking objectDef.isSystem and using queryNativeRecords) ignores args.filters,
args.sort, and args.paginationOpts.cursor/continuation and always returns
isDone:true/truncated:false; fix by either: (A) piping the nativeRecords result
into the same filtering, sorting and pagination/continuation logic used by the
EAV path so filters/sort/cursor behave identically, preserving accurate
continueCursor/isDone/truncated semantics, or (B) defensively fail-fast: if
args.filters, args.sort, or args.paginationOpts.cursor are present (or if
objectDef.isSystem is true but objectDef.nativeTable is missing/misconfigured),
throw an error indicating native queries don’t support those options yet; update
the branch around queryNativeRecords and the objectDef.isSystem &&
objectDef.nativeTable check accordingly to ensure we don’t silently drop rows or
fall back to EAV for misconfigured system objects.
In `@specs/ENG-255/tasks.md`:
- Line 50: Update the task bullet that mentions implementing getRecord to
reflect the shipped scope: change the line "In `getRecord()`: Replace stub with
native record fetch..." to indicate that single-record native lookup is deferred
to ENG-256 (matching convex/crm/recordQueries.ts behavior and
specs/ENG-255/chunks/chunk-02-integration/context.md), and remove or mark as
resolved any TODO that claims getRecord currently throws for system calls;
ensure the task list no longer promises a native single-record implementation
for this ticket and instead references ENG-256 for that work.
🪄 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: 4e2788ae-bf72-46dd-aa6f-8e366e1209e9
📒 Files selected for processing (9)
convex/crm/recordQueries.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.md
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
convex/crm/recordQueries.ts (1)
350-388:⚠️ Potential issue | 🟠 MajorSystem-object pagination is still incomplete here.
queryNativeRecords(... clampedNumItems)still bounds the native scan beforeapplyFilters()/applySort(), and this branch always returnsisDone: true. That means matches and sort winners beyond the first raw slice are still invisible, and an unfiltered system table larger thannumItemsis still reported as complete. Please mirror path B’sFILTERED_QUERY_CAP + 1scan + offset slicing here (or fail fast) sorecords,continueCursor,isDone, andtruncatedstay truthful.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/crm/recordQueries.ts` around lines 350 - 388, The native-records branch clamps the raw scan before filtering (using clampedNumItems and queryNativeRecords) and always returns isDone: true, hiding matching records beyond the first slice; change the logic to mirror path B by scanning for FILTERED_QUERY_CAP + 1 raw rows (use FILTERED_QUERY_CAP constant, not rawNumItems), then run applyFilters and applySort on the full scan result, slice the sorted results to the caller's requested numItems, and set continueCursor/isDone/truncated based on whether there were more than FILTERED_QUERY_CAP matches (or alternatively fail fast if cursor-based pagination is passed); update uses of queryNativeRecords, clampedNumItems, applyFilters, applySort, continueCursor, isDone, and truncated accordingly so the reported page and completion flags are truthful.
🤖 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 628-631: In searchRecords (in recordQueries.ts) returning [] when
objectDef.isSystem hides the fact native search isn’t implemented; instead
either rethrow a ConvexError indicating “native search not implemented for
system objects” or implement an in-memory fallback that queries the native
adapter and filters by labelValue before returning results; update the branch
that currently does "if (objectDef.isSystem) { return [] }" to throw the
ConvexError (or call the native adapter search + filter) so callers can
distinguish unimplemented behavior from a genuine no-match result.
---
Duplicate comments:
In `@convex/crm/recordQueries.ts`:
- Around line 350-388: The native-records branch clamps the raw scan before
filtering (using clampedNumItems and queryNativeRecords) and always returns
isDone: true, hiding matching records beyond the first slice; change the logic
to mirror path B by scanning for FILTERED_QUERY_CAP + 1 raw rows (use
FILTERED_QUERY_CAP constant, not rawNumItems), then run applyFilters and
applySort on the full scan result, slice the sorted results to the caller's
requested numItems, and set continueCursor/isDone/truncated based on whether
there were more than FILTERED_QUERY_CAP matches (or alternatively fail fast if
cursor-based pagination is passed); update uses of queryNativeRecords,
clampedNumItems, applyFilters, applySort, continueCursor, isDone, and truncated
accordingly so the reported page and completion flags are truthful.
🪄 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: 418a7714-63c3-46eb-826f-aeb061c46ae3
📒 Files selected for processing (2)
convex/crm/recordQueries.tsspecs/ENG-255/tasks.md
This was referenced Mar 30, 2026
Merged
Merged
Connorbelez
added a commit
that referenced
this pull request
Apr 20, 2026
eng-252 Merge branch 'main' into eng-255 eng-255 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * System objects can be queried natively and returned as unified records with field mappings and pagination controls. * **Bug Fixes** * Replaced several “not yet implemented” errors with functional query behavior. * System-object search now returns empty results instead of throwing. * Improved error message when fetching single system records to direct users to querying. * **Documentation** * Added spec and task docs describing adapter architecture and integration steps. <!-- 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-252
Merge branch 'main' into eng-255
eng-255
Summary by CodeRabbit
New Features
Bug Fixes
Documentation