fix(convex): make customer source field required in create mutations#47
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThe pull request makes the Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
📜 Recent review detailsConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro (Legacy) 📒 Files selected for processing (4)
🧰 Additional context used🧠 Learnings (11)📚 Learning: 2025-12-26T03:04:07.995ZApplied to files:
📚 Learning: 2025-12-30T03:24:33.770ZApplied to files:
📚 Learning: 2025-12-30T06:21:13.183ZApplied to files:
📚 Learning: 2025-11-30T03:53:00.316ZApplied to files:
📚 Learning: 2025-12-15T14:01:55.275ZApplied to files:
📚 Learning: 2025-11-30T03:53:00.316ZApplied to files:
📚 Learning: 2025-12-15T14:44:09.823ZApplied to files:
📚 Learning: 2025-10-03T11:34:20.628ZApplied to files:
📚 Learning: 2025-12-15T14:44:04.593ZApplied to files:
📚 Learning: 2025-12-30T06:20:39.422ZApplied to files:
📚 Learning: 2025-12-30T06:20:14.869ZApplied to files:
🧬 Code graph analysis (2)services/platform/convex/customers.ts (1)
services/platform/convex/workflow/actions/customer/customer_action.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (4)
Comment |
![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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/platform/convex/workflow/actions/customer/customer_action.ts (1)
56-56: Consider usingcustomerSourceValidatorfor type consistency.The
customerUpdateValidatorusesv.optional(v.string())forsource, while the create operation now usescustomerSourceValidator. For consistency and stricter validation, consider:const customerUpdateValidator = v.object({ name: v.optional(v.string()), email: v.optional(v.string()), status: v.optional(customerStatusValidator), - source: v.optional(v.string()), + source: v.optional(customerSourceValidator), locale: v.optional(v.string()), address: v.optional(customerAddressValidator), metadata: v.optional(v.record(v.string(), v.any())), });This ensures updates also only accept valid source values at the action layer, rather than deferring to downstream validation.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro (Legacy)
📒 Files selected for processing (4)
services/platform/convex/customers.tsservices/platform/convex/model/customers/create_customer.tsservices/platform/convex/model/customers/find_or_create_customer.tsservices/platform/convex/workflow/actions/customer/customer_action.ts
🧰 Additional context used
🧠 Learnings (11)
📚 Learning: 2025-12-26T03:04:07.995Z
Learnt from: larryro
Repo: tale-project/tale PR: 35
File: services/platform/convex/approvals.ts:51-62
Timestamp: 2025-12-26T03:04:07.995Z
Learning: In Convex approvals API (services/platform/convex/approvals.ts), continue the pattern of maintaining separate internalQuery functions for internal vs public API access (e.g., getApprovalInternal vs getApprovalById) even if implementations are identical. This separation preserves the ability to diverge access control patterns in the future without breaking call sites. Apply this guideline broadly to the Convex-related API files under services/platform/convex/, using the pattern services/platform/convex/**/*.ts to cover similar modules. Ensure new or refactored internal/public wrappers follow this convention and document intent where access rules may evolve.
Applied to files:
services/platform/convex/model/customers/find_or_create_customer.tsservices/platform/convex/customers.tsservices/platform/convex/model/customers/create_customer.tsservices/platform/convex/workflow/actions/customer/customer_action.ts
📚 Learning: 2025-12-30T03:24:33.770Z
Learnt from: larryro
Repo: tale-project/tale PR: 36
File: services/platform/convex/wf_step_defs.ts:33-39
Timestamp: 2025-12-30T03:24:33.770Z
Learning: In Convex API files under services/platform/convex (e.g., wf_step_defs.ts and peers) refrain from delegating trivial single-line database calls like ctx.db.get(id) to model helpers. Use direct calls for simple operations with no extra business logic. Reserve model helpers for complex tasks (ordering, filtering, validation, transformation). This guideline applies to all .ts files in this Convex API area.
Applied to files:
services/platform/convex/model/customers/find_or_create_customer.tsservices/platform/convex/customers.tsservices/platform/convex/model/customers/create_customer.tsservices/platform/convex/workflow/actions/customer/customer_action.ts
📚 Learning: 2025-12-30T06:21:13.183Z
Learnt from: larryro
Repo: tale-project/tale PR: 37
File: services/platform/convex/model/documents/validators.ts:89-102
Timestamp: 2025-12-30T06:21:13.183Z
Learning: Do not flag a missing trailing newline for TypeScript files in code reviews. POSIX text files should end with a trailing newline and Prettier (or your formatter) will enforce this. Treat the trailing newline as a non-issue in reviews for all TS files.
Applied to files:
services/platform/convex/model/customers/find_or_create_customer.tsservices/platform/convex/customers.tsservices/platform/convex/model/customers/create_customer.tsservices/platform/convex/workflow/actions/customer/customer_action.ts
📚 Learning: 2025-11-30T03:53:00.316Z
Learnt from: CR
Repo: tale-project/poc2 PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-30T03:53:00.316Z
Learning: Applies to convex/**/*.ts : Always include argument and return validators for all Convex functions (query, internalQuery, mutation, internalMutation, action, internalAction)
Applied to files:
services/platform/convex/customers.tsservices/platform/convex/workflow/actions/customer/customer_action.ts
📚 Learning: 2025-12-15T14:01:55.275Z
Learnt from: larryro
Repo: tale-project/tale PR: 18
File: services/platform/convex/workflow/actions/conversation/helpers/update_conversations.ts:7-10
Timestamp: 2025-12-15T14:01:55.275Z
Learning: In Convex action helpers (services/platform/convex/workflow/actions/**/helpers/*.ts), using Record<string, unknown> for update parameters is acceptable when field validation is handled at the mutation level in Convex. This provides flexibility for dynamic field updates while keeping validation centralized at the mutation layer.
Applied to files:
services/platform/convex/customers.tsservices/platform/convex/workflow/actions/customer/customer_action.ts
📚 Learning: 2025-11-30T03:53:00.316Z
Learnt from: CR
Repo: tale-project/poc2 PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-30T03:53:00.316Z
Learning: Applies to convex/**/*.ts : Always use the new Convex function syntax with `query`, `mutation`, `internalQuery`, `internalMutation`, `action`, or `internalAction` with explicit `args`, `returns`, and `handler` properties
Applied to files:
services/platform/convex/customers.ts
📚 Learning: 2025-12-15T14:44:09.823Z
Learnt from: larryro
Repo: tale-project/tale PR: 18
File: services/platform/convex/workflow/actions/conversation/conversation_action.ts:47-98
Timestamp: 2025-12-15T14:44:09.823Z
Learning: In Convex action files (services/platform/convex/workflow/actions/**/), maintaining a separate TypeScript type alongside the parametersValidator is an acceptable pattern when documented. The TypeScript type provides IDE support and compile-time checking, while the validator provides runtime validation. This intentional separation should be documented with a comment explaining the design choice.
Applied to files:
services/platform/convex/customers.ts
📚 Learning: 2025-10-03T11:34:20.628Z
Learnt from: CR
Repo: talecorp/poc2 PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-10-03T11:34:20.628Z
Learning: Applies to convex/**/*.{ts,js} : Always use the new Convex function syntax (query/mutation/action with args/returns/handler)
Applied to files:
services/platform/convex/customers.ts
📚 Learning: 2025-12-15T14:44:04.593Z
Learnt from: larryro
Repo: tale-project/tale PR: 18
File: services/platform/convex/workflow/actions/conversation/conversation_action.ts:47-98
Timestamp: 2025-12-15T14:44:04.593Z
Learning: In Convex action files under services/platform/convex/workflow/actions/**, prefer maintaining a separate TypeScript type for the action parameters alongside the runtime validators (parametersValidator). The TypeScript type provides IDE support and compile-time checking, while the validator handles runtime validation. Document this design with a clear comment next to the type/validator pair explaining the rationale (e.g., separate types for static typing vs runtime checks, and how they relate). This pattern applies across all actions in this directory, not just a single file.
Applied to files:
services/platform/convex/workflow/actions/customer/customer_action.ts
📚 Learning: 2025-12-30T06:20:39.422Z
Learnt from: larryro
Repo: tale-project/tale PR: 37
File: services/platform/convex/model/conversations/validators.ts:106-139
Timestamp: 2025-12-30T06:20:39.422Z
Learning: In services/platform/convex/model/conversations/validators.ts, conversationItemValidator and conversationWithMessagesValidator are intentionally kept as separate validators despite having identical structures. This separation serves different semantic purposes: conversationItemValidator is for list responses while conversationWithMessagesValidator is for single conversation detail views. The duplication allows future divergence when list views might need different fields than detail views for performance or feature reasons.
Applied to files:
services/platform/convex/workflow/actions/customer/customer_action.ts
📚 Learning: 2025-12-30T06:20:14.869Z
Learnt from: larryro
Repo: tale-project/tale PR: 37
File: services/platform/convex/model/conversations/validators.ts:31-39
Timestamp: 2025-12-30T06:20:14.869Z
Learning: In services/platform/convex/model/conversations/validators.ts, the messageValidator intentionally uses v.string() for the status field because message statuses are dynamic and come from different sources (email providers, internal state, etc.) and are not yet constrained to a fixed set of values.
Applied to files:
services/platform/convex/workflow/actions/customer/customer_action.ts
🧬 Code graph analysis (2)
services/platform/convex/customers.ts (1)
services/platform/convex/model/customers/validators.ts (1)
customerSourceValidator(19-23)
services/platform/convex/workflow/actions/customer/customer_action.ts (1)
services/platform/convex/model/customers/validators.ts (1)
customerSourceValidator(19-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build Docker Image
🔇 Additional comments (4)
services/platform/convex/model/customers/find_or_create_customer.ts (1)
14-14: LGTM! Consistent withCreateCustomerArgschanges.The
sourcefield is now required inFindOrCreateCustomerArgsand passed directly tocreateCustomerwithout a fallback. This maintains consistency across the customer creation flows.Also applies to: 51-51
services/platform/convex/customers.ts (1)
35-35: LGTM! Validators aligned with model layer changes.Both
createCustomerandfindOrCreateCustomermutation validators now requiresourceusingcustomerSourceValidator. This correctly mirrors the updatedCreateCustomerArgsandFindOrCreateCustomerArgsinterfaces in the model layer.Good that
updateCustomer(line 215) remains optional—partial updates shouldn't require all fields.Also applies to: 58-58
services/platform/convex/workflow/actions/customer/customer_action.ts (1)
38-38: LGTM! Workflow action correctly requires source for create operations.The changes properly:
- Import
customerSourceValidatorfor runtime validation- Update the TypeScript type to require
sourcein the create operation- Use
customerSourceValidatorin the parameters validator- Pass
params.sourcedirectly without fallbackThis follows the established pattern of maintaining separate TypeScript types alongside runtime validators. Based on learnings, this dual approach provides IDE support and compile-time checking alongside runtime validation.
Also applies to: 84-84, 119-119, 163-163
services/platform/convex/model/customers/create_customer.ts (1)
12-12: LGTM! Source field is now correctly required.The change properly enforces
sourceat the type level and removes the implicit'manual_import'fallback. This aligns with the schema validator and ensures callers explicitly specify the customer source. Verified that all callers—both incustomers.ts(line 46) andfind_or_create_customer.ts(line 56)—have been updated to provide an explicitsourcevalue.
Fixes inconsistency between mutations and document validator where source was optional in createCustomer and findOrCreateCustomer mutations but required in the schema and validator. - Make source required in CreateCustomerArgs interface - Make source required in FindOrCreateCustomerArgs interface - Update Convex mutation validators to require source - Update customer_action.ts to require source for create operation - Keep source optional in updateCustomer (correct for partial updates) Closes #40
larryro
force-pushed
the
claude/fix-customer-source-validation-2zBCm
branch
from
4839806 to
1f175d5
Compare
yannickmonney
pushed a commit
that referenced
this pull request
Apr 8, 2026
…47) Co-authored-by: Claude <noreply@anthropic.com>
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
2 participants
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
sourcefield required increateCustomerandfindOrCreateCustomermutations to match the schema and document validatorProblem
The
sourcefield had inconsistent required/optional status:sourceis requiredsourcewas optional (with fallback)sourcewas optional (with fallback)sourcewas already requiredThis mismatch could lead to type confusion and obscured the actual data requirements.
Solution
Aligned all create mutations with the schema by making
sourcerequired:CreateCustomerArgsandFindOrCreateCustomerArgsinterfacescustomer_action.tsworkflow action'manual_import'Note:
updateCustomercorrectly keepssourceoptional since it's a partial update.Test plan
sourcevaluessource: 'manual_import')Closes #40
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.