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
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughThe changes add three new Convex query handlers for CRM record management: Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
Owner
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
![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 core CRM record retrieval for the EAV data model, including field assembly from typed value tables, filtering/sorting, record link hydration, and label full-text search via a new Convex search index.
Changes:
- Added
searchIndex("search_label")onrecords.labelValueto support full-text search with org/object/deleted filtering. - Introduced shared CRM query types (
UnifiedRecord, filters, sorts, link shapes). - Added
queryRecords,getRecord, andsearchRecordsqueries with EAV fan-out field assembly and filtering/sorting logic.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| specs/ENG-251/tasks.md | Task checklist for ENG-251 implementation work. |
| specs/ENG-251/chunks/manifest.md | Chunking/rollup notes for ENG-251. |
| convex/schema.ts | Adds search_label search index on records for label search with filter fields. |
| convex/crm/types.ts | Adds CRM query/response types for unified record and linking shapes. |
| convex/crm/recordQueries.ts | Implements record querying, field assembly, filtering/sorting, link hydration, and label search. |
💡 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: 1
🧹 Nitpick comments (2)
convex/crm/recordQueries.ts (2)
270-275: Operator validation is loose compared to the type definition.The
RecordFiltertype intypes.tsdefinesoperatoras a strict union of valid operators, but the input validation here usesv.string(). This allows invalid operators to pass validation and silently match (due to thedefault: return trueinmatchesFilter). Consider using a union validator to catch invalid operators at the API boundary.♻️ Suggested fix to validate operators at input level
filters: v.optional( v.array( v.object({ fieldDefId: v.id("fieldDefs"), - operator: v.string(), + operator: v.union( + v.literal("eq"), + v.literal("gt"), + v.literal("lt"), + v.literal("gte"), + v.literal("lte"), + v.literal("contains"), + v.literal("starts_with"), + v.literal("is_any_of"), + v.literal("is_true"), + v.literal("is_false") + ), value: v.any(), }) ) ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/crm/recordQueries.ts` around lines 270 - 275, The operator field in the input validator currently uses v.string(), which is too permissive compared to the RecordFilter operator union in types.ts and lets invalid operators pass (leading to silent matches via matchesFilter). Replace operator: v.string() inside the v.object validator with a union/literal validator that enumerates the exact operator values from the RecordFilter union (e.g., v.union or repeating v.literal(...) entries matching the operators defined in types.ts) so the API rejects invalid operators at validation time and stays consistent with RecordFilter and matchesFilter.
513-525: Consider capping the search limit to prevent excessive reads.The
limitparameter has no upper bound, which could allow callers to request very large result sets. Given the subsequent assembly step queries multiple EAV tables per record, a large limit could exceed Convex's read limits.♻️ Suggested fix to cap the search limit
-const limit = args.limit ?? 20; +const MAX_SEARCH_LIMIT = 100; +const limit = Math.min(args.limit ?? 20, MAX_SEARCH_LIMIT);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/crm/recordQueries.ts` around lines 513 - 525, The search `limit` is unbounded (using `args.limit ?? 20`) which can permit very large queries; enforce a hard cap and validate the input before calling `.take(limit)` on the `ctx.db.query("records").withSearchIndex("search_label", ...)` chain. Update the code around the `limit` variable (the `args.limit` usage) to coerce to an integer, clamp it to a safe maximum (e.g., MAX_LIMIT constant such as 100), and ensure it is >= 1 (use Math.min/Math.max or equivalent) so the `.take(limit)` call always receives a sane bounded value.
🤖 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 368-370: The cursor parsing using
Number.parseInt(args.paginationOpts.cursor, 10) can yield NaN for malformed
input; update the logic around offset (the variable computed from
args.paginationOpts.cursor in recordQueries.ts) to validate the parsed value and
default to 0 when invalid: parse the cursor, ensure it's a finite non-negative
integer (e.g., check Number.isFinite or !Number.isNaN and value >= 0), and use 0
as the fallback for any invalid or out-of-range values before using offset in
slice operations.
---
Nitpick comments:
In `@convex/crm/recordQueries.ts`:
- Around line 270-275: The operator field in the input validator currently uses
v.string(), which is too permissive compared to the RecordFilter operator union
in types.ts and lets invalid operators pass (leading to silent matches via
matchesFilter). Replace operator: v.string() inside the v.object validator with
a union/literal validator that enumerates the exact operator values from the
RecordFilter union (e.g., v.union or repeating v.literal(...) entries matching
the operators defined in types.ts) so the API rejects invalid operators at
validation time and stays consistent with RecordFilter and matchesFilter.
- Around line 513-525: The search `limit` is unbounded (using `args.limit ??
20`) which can permit very large queries; enforce a hard cap and validate the
input before calling `.take(limit)` on the
`ctx.db.query("records").withSearchIndex("search_label", ...)` chain. Update the
code around the `limit` variable (the `args.limit` usage) to coerce to an
integer, clamp it to a safe maximum (e.g., MAX_LIMIT constant such as 100), and
ensure it is >= 1 (use Math.min/Math.max or equivalent) so the `.take(limit)`
call always receives a sane bounded value.
🪄 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: 1b98835a-77d5-4b5c-a35e-3e24834f107b
📒 Files selected for processing (5)
convex/crm/recordQueries.tsconvex/crm/types.tsconvex/schema.tsspecs/ENG-251/chunks/manifest.mdspecs/ENG-251/tasks.md
- Return null cursor instead of 'done' string when pagination complete - Add objectDef validation to getRecord (active/system/org checks) - Add isSystem guard to searchRecords - Validate and clamp limit parameter (1-100 range) - Fail-closed on unknown fieldDefId in filters - Validate pagination cursor format (reject NaN) - Derive FILTERED_QUERY_CAP from Convex read limits - Normalize cursor format with tagged prefixes (native:/offset:) across both paths - Use v.union literals for filter operator validation - matchesFilter default returns false (fail-closed) - Handle multi-select fields in is_any_of operator - Update QueryRecordsResult type to allow null continueCursor Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Mar 30, 2026
Merged
Merged
Merged
Merged
Merged
Connorbelez
added a commit
that referenced
this pull request
Apr 20, 2026
### TL;DR Implements comprehensive record querying and search functionality for the CRM system with EAV (Entity-Attribute-Value) field assembly, filtering, sorting, and full-text search capabilities. ### What changed? Added three new query functions to handle CRM record operations: - **`queryRecords`**: Retrieves records with optional filtering and sorting. Uses dual code paths - native Convex pagination for simple queries, and in-memory filtering/sorting for complex queries with a 500-record cap - **`getRecord`**: Fetches a single record with all field values assembled from EAV tables, plus inbound/outbound linked records via the `recordLinks` table - **`searchRecords`**: Performs full-text search on record labels using Convex's search index The implementation includes: - EAV field assembly that queries only relevant value tables based on field types - Comprehensive filtering with operators like `eq`, `gt`, `contains`, `is_any_of`, etc. - Sorting by field values with ascending/descending direction - Parallel field value loading for performance optimization - Type definitions for unified record shapes, filters, and linked records - Search index on the `records` table's `labelValue` field ### How to test? 1. Run `bun check` and `bun typecheck` to verify type safety 2. Execute `bunx convex codegen` to generate updated types 3. Test the three query endpoints: - Call `queryRecords` with various filter/sort combinations - Use `getRecord` to fetch individual records with their links - Try `searchRecords` with different search terms ### Why make this change? This implements the core querying infrastructure needed for the CRM system to retrieve and display record data. The EAV model requires specialized assembly logic to reconstruct records from normalized value tables, while the dual pagination approach ensures both performance (for simple queries) and functionality (for complex filtered queries). The search capability enables users to quickly find records by their display labels. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added record querying with support for field filtering, sorting, and pagination * Added record search functionality over label values * Added ability to retrieve individual record details with linked entity information * Implemented support for outbound and inbound record relationships * **Chores** * Added documentation for record query implementation tasks <!-- 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.
TL;DR
Implements comprehensive record querying and search functionality for the CRM system with EAV (Entity-Attribute-Value) field assembly, filtering, sorting, and full-text search capabilities.
What changed?
Added three new query functions to handle CRM record operations:
queryRecords: Retrieves records with optional filtering and sorting. Uses dual code paths - native Convex pagination for simple queries, and in-memory filtering/sorting for complex queries with a 500-record capgetRecord: Fetches a single record with all field values assembled from EAV tables, plus inbound/outbound linked records via therecordLinkstablesearchRecords: Performs full-text search on record labels using Convex's search indexThe implementation includes:
eq,gt,contains,is_any_of, etc.recordstable'slabelValuefieldHow to test?
bun checkandbun typecheckto verify type safetybunx convex codegento generate updated typesqueryRecordswith various filter/sort combinationsgetRecordto fetch individual records with their linkssearchRecordswith different search termsWhy make this change?
This implements the core querying infrastructure needed for the CRM system to retrieve and display record data. The EAV model requires specialized assembly logic to reconstruct records from normalized value tables, while the dual pagination approach ensures both performance (for simple queries) and functionality (for complex filtered queries). The search capability enables users to quickly find records by their display labels.
Summary by CodeRabbit
Release Notes
New Features
Documentation