Skip to content

fix(convex): add messageStatusValidator for stricter message status validation#45

Merged
larryro merged 5 commits into
mainfrom
claude/fix-issue-kT29K
Dec 30, 2025
Merged

fix(convex): add messageStatusValidator for stricter message status validation#45
larryro merged 5 commits into
mainfrom
claude/fix-issue-kT29K

Conversation

@larryro
Copy link
Copy Markdown
Collaborator

@larryro larryro commented Dec 30, 2025

Summary

  • Created messageStatusValidator union validator for stricter type safety on message status field
  • Replaced permissive v.string() with union of valid delivery states: queued, sent, delivered, failed
  • Exported new MessageStatus type from types.ts

Context

Addresses CodeRabbit comment #2652223400 from PR #37 review, which identified that messageValidator was using v.string() for the status field, allowing any string value instead of enforcing valid message statuses.

Test plan

  • Verify TypeScript compilation passes
  • Confirm message status values align with schema deliveryState definition
  • Test conversation message display in UI still works correctly

Summary by CodeRabbit

  • Chores
    • Enhanced message delivery state handling with stricter validation across the system.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Dec 30, 2025

📝 Walkthrough

Walkthrough

Two validation and type files were updated to introduce stricter message status validation. A new messageStatusValidator was added to the validators module that defines allowed message delivery states (queued, sent, delivered, failed). A corresponding type alias MessageStatus was created in the types module by inferring from this validator. The messageValidator was updated to use the new validator for its status field instead of accepting any string, tightening the validation constraints.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related issues


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro (Legacy)

📥 Commits

Reviewing files that changed from the base of the PR and between 9c2e381 and 47d371c.

📒 Files selected for processing (2)
  • services/platform/convex/model/conversations/types.ts
  • services/platform/convex/model/conversations/validators.ts
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
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.
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.
📚 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/model/conversations/types.ts
  • services/platform/convex/model/conversations/validators.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/model/conversations/types.ts
  • services/platform/convex/model/conversations/validators.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/model/conversations/types.ts
  • services/platform/convex/model/conversations/validators.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} : Use the documented Convex validators for all supported types (e.g., v.id, v.int64, v.number, v.boolean, v.string, v.bytes, v.array, v.object, v.record)

Applied to files:

  • services/platform/convex/model/conversations/types.ts
  • services/platform/convex/model/conversations/validators.ts
📚 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/conversations/types.ts
  • services/platform/convex/model/conversations/validators.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/conversations/types.ts
  • services/platform/convex/model/conversations/validators.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/conversations/types.ts
  • services/platform/convex/model/conversations/validators.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/model/conversations/validators.ts
🧬 Code graph analysis (1)
services/platform/convex/model/conversations/types.ts (1)
services/platform/convex/model/conversations/validators.ts (1)
  • messageStatusValidator (32-37)
🔇 Additional comments (2)
services/platform/convex/model/conversations/types.ts (1)

15-15: LGTM! Type definition follows established patterns.

The import and type export are correctly structured and consistent with existing patterns in this file.

Also applies to: 25-25

services/platform/convex/model/conversations/validators.ts (1)

28-37: The validator formalization is not a breaking change. The database schema (schema.ts:520-524) already constrains deliveryState to these same 4 values, and all code paths across email providers, workflows, and message handlers consistently use only these values. No existing data or writers would be affected by this validator change.

Likely an incorrect or invalid review comment.

Comment thread services/platform/convex/model/conversations/validators.ts Outdated
@larryro
Copy link
Copy Markdown
Collaborator Author

larryro commented Dec 30, 2025

Addressed CodeRabbit's nitpick comment in commit 5233bf1:

  • Updated the doc comment for messageStatusValidator to correctly reference "message status field" instead of "deliveryState field" for clarity, as suggested by CodeRabbit.

claude and others added 3 commits December 30, 2025 18:22
…alidation

Replace permissive v.string() with strict union validator for message status field.
The validator covers all valid delivery states: queued, sent, delivered, failed.
Updated the doc comment to correctly reference "message status field"
instead of "deliveryState field" for clarity, as this validator is
used for the status field in messageValidator.

Addresses CodeRabbit nitpick in PR review.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…dator

- transform_conversation.ts: Cast deliveryState to the correct union type
- conversation-panel.tsx: Remove redundant filter for non-existent 'pending'
  status and simplify pendingMessage object (status field not used)
- message-editor.tsx: Update pendingMessage prop type to not require status
- conversations-list.tsx: Replace 'approved' with 'sent'/'delivered' to match
  actual delivery states from the database schema

These changes ensure type safety after introducing the stricter
messageStatusValidator which only allows: queued, sent, delivered, failed.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@larryro larryro force-pushed the claude/fix-issue-kT29K branch from e344c79 to df03953 Compare December 30, 2025 10:22
… types

- Rename lib/action-cache to lib/action_cache for Convex naming convention
- Rename lib/rate-limiter to lib/rate_limiter for Convex naming convention
- Add explicit types to array callback parameters across React components
- Fix import statements to use `type` keyword where appropriate
- Update validator types for stricter type checking in internal actions

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Remove explicit type annotations from callback parameters where
TypeScript can infer the types from the array context.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@larryro larryro merged commit 9527c68 into main Dec 30, 2025
2 checks passed
@larryro larryro deleted the claude/fix-issue-kT29K branch December 30, 2025 12:12
yannickmonney pushed a commit that referenced this pull request Apr 8, 2026
…alidation (#45)

Co-authored-by: Claude <noreply@anthropic.com>
larryro added a commit that referenced this pull request May 17, 2026
…— docs + openai.json honesty

Closes round-5 findings #1, #2, #42, #43, #45.

- Docs across en/de/fr no longer claim a browser `speechSynthesis`
  fallback. The code (use-voice-output-player.ts + tts/schema.ts) is
  explicitly provider-only — failed chunks are skipped silently. The
  "Provider vs browser fallback" section is replaced with a "When no
  provider is configured" section that states the actual behaviour:
  the personalization toggle is disabled and surfaces a Settings link.
- Docs across en/de/fr correct the "no cron required" lie. The lifecycle
  text now reflects reality: a daily org-sweep cron is the primary GC,
  with opportunistic per-thread cleanup scheduled from the write path
  as a secondary trigger.
- `examples/providers/openai.json` raises the `centsPerMillionCharacters`
  default from 200 to 1500 (tts-1 list rate, matches
  `providers.test.ts:81` and the docs' canonical example), and expands
  the description / de+fr i18n descriptions with the per-token-vs-per-
  character billing caveat that previously existed only in the English
  description. Operators on a non-English UI no longer miss the
  calibration warning.
- The English doc consistently uses "Settings → AI providers" (matching
  the UI label) instead of "Settings > Providers".
- `docs/fr/self-hosted/configuration/providers.md:319` anchor link to
  the FR attachments page is fixed (`#audio-and-video-transcription`
  was the English slug; the localized heading auto-slugifies to
  `#transcription-audio-et-video`). DE line 319 had the same English-
  slug bug, fixed alongside (DE auto-slugs to
  `#audio-und-video-transkription`).
larryro added a commit that referenced this pull request May 17, 2026
…— docs + openai.json honesty

Closes round-5 findings #1, #2, #42, #43, #45.

- Docs across en/de/fr no longer claim a browser `speechSynthesis`
  fallback. The code (use-voice-output-player.ts + tts/schema.ts) is
  explicitly provider-only — failed chunks are skipped silently. The
  "Provider vs browser fallback" section is replaced with a "When no
  provider is configured" section that states the actual behaviour:
  the personalization toggle is disabled and surfaces a Settings link.
- Docs across en/de/fr correct the "no cron required" lie. The lifecycle
  text now reflects reality: a daily org-sweep cron is the primary GC,
  with opportunistic per-thread cleanup scheduled from the write path
  as a secondary trigger.
- `examples/providers/openai.json` raises the `centsPerMillionCharacters`
  default from 200 to 1500 (tts-1 list rate, matches
  `providers.test.ts:81` and the docs' canonical example), and expands
  the description / de+fr i18n descriptions with the per-token-vs-per-
  character billing caveat that previously existed only in the English
  description. Operators on a non-English UI no longer miss the
  calibration warning.
- The English doc consistently uses "Settings → AI providers" (matching
  the UI label) instead of "Settings > Providers".
- `docs/fr/self-hosted/configuration/providers.md:319` anchor link to
  the FR attachments page is fixed (`#audio-and-video-transcription`
  was the English slug; the localized heading auto-slugifies to
  `#transcription-audio-et-video`). DE line 319 had the same English-
  slug bug, fixed alongside (DE auto-slugs to
  `#audio-und-video-transkription`).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants