fix: normalize legacy claudeCode provider at all decode boundaries#34
fix: normalize legacy claudeCode provider at all decode boundaries#34aaditagrawal merged 2 commits intomainfrom
Conversation
…gent Local databases may contain events with provider="claudeCode" from before the rename to "claudeAgent". Since migration 016 already ran on existing databases, add migration 017 to normalize these legacy values in events, projections, and session tables.
Ensures existing users with claudeCode data in their chat history don't crash on upgrade: - Event store: normalize claudeCode in payload before Schema decoding - composerDraftStore: normalizeProviderKind maps claudeCode → claudeAgent - threadProvider: toProviderKind maps claudeCode → claudeAgent - Migration 017 handles DB normalization for projection tables and events
📝 WalkthroughWalkthroughThese changes implement a migration strategy to normalize legacy provider identifiers from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/server/src/persistence/Migrations/016_CanonicalizeModelSelections.ts (1)
249-290: Consider consolidating with Migration 017.These SQL statements (lines 251-290) perform the same
claudeCode→claudeAgentnormalization that Migration 017 also performs on the same tables and fields. While the duplication is idempotent and safe, it results in unnecessary database operations for users upgrading through both migrations.The docstring in Migration 017 explains this was intentional to handle users who ran Migration 016 before the rename landed. If this is the intended behavior for different upgrade paths, consider adding a comment here explaining why the duplication exists.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/src/persistence/Migrations/016_CanonicalizeModelSelections.ts` around lines 249 - 290, Migration 016 currently repeats the same `claudeCode` → `claudeAgent` updates as Migration 017 for the orchestration_events (payload_json $.modelSelection.provider and $.defaultModelSelection.provider) and projection tables (projection_threads.model_selection_json $.provider and projection_projects.default_model_selection_json $.provider); either remove the duplicate SQL or, if duplication is intentional to support different upgrade paths, add a concise comment above these UPDATE blocks in 016_CanonicalizeModelSelections explaining that this repetition mirrors Migration 017 to handle users who ran 016 before the rename landed and is intentionally idempotent. Ensure the comment references Migration 017 and the specific JSON fields (payload_json.modelSelection/provider, payload_json.defaultModelSelection/provider, model_selection_json.provider, default_model_selection_json.provider) so future readers understand the rationale.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/server/src/persistence/Migrations/016_CanonicalizeModelSelections.ts`:
- Around line 249-290: Migration 016 currently repeats the same `claudeCode` →
`claudeAgent` updates as Migration 017 for the orchestration_events
(payload_json $.modelSelection.provider and $.defaultModelSelection.provider)
and projection tables (projection_threads.model_selection_json $.provider and
projection_projects.default_model_selection_json $.provider); either remove the
duplicate SQL or, if duplication is intentional to support different upgrade
paths, add a concise comment above these UPDATE blocks in
016_CanonicalizeModelSelections explaining that this repetition mirrors
Migration 017 to handle users who ran 016 before the rename landed and is
intentionally idempotent. Ensure the comment references Migration 017 and the
specific JSON fields (payload_json.modelSelection/provider,
payload_json.defaultModelSelection/provider, model_selection_json.provider,
default_model_selection_json.provider) so future readers understand the
rationale.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4151b352-4125-4940-a6ce-a156fa9829e5
📒 Files selected for processing (6)
apps/server/src/persistence/Layers/OrchestrationEventStore.tsapps/server/src/persistence/Migrations.tsapps/server/src/persistence/Migrations/016_CanonicalizeModelSelections.tsapps/server/src/persistence/Migrations/017_NormalizeLegacyClaudeCodeProvider.tsapps/web/src/composerDraftStore.tsapps/web/src/lib/threadProvider.ts
Summary
claudeCode→claudeAgentin existing DB data (events, projections, sessions)claudeCode→claudeAgentmapping intoProviderKind()andnormalizeProviderKind()for UI/store pathsTest plan
bun run devstarts cleanly on a database withclaudeCodeeventsbun typecheckpasses all 7 packagesSummary by CodeRabbit
Release Notes
Bug Fixes
Chores