fix(runs): persist the resolved modelId onto the chat row - #804
fix(runs): persist the resolved modelId onto the chat row#804sweetmantech wants to merge 1 commit into
Conversation
handleStartChatRun resolved modelId and threaded it into the workflow input, but called provisionRunSession without it, so createSessionWithInitialChat inserted the chat with no model_id. The generation path reads the model from the chat row, so chats.model_id recorded the default for every headless run regardless of what the task configured. Measured on prod over 30 days: chats.model_id reads anthropic/claude-haiku-4.5 for 100% of scheduled-generation rooms, while the generation metadata shows 746 runs on sonnet-5, 238 on kimi-k3 and 55 on gpt-5.4-mini. The runs were correct; the column was not — so every audit keyed on it was wrong. chat#1918
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Warning Review limit reached
Next review available in: 51 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (3)
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 6 files
Confidence score: 4/5
- In
lib/sessions/createSessionWithInitialChat.ts, thecreateSessionWithInitialChatmodel selection path treats an explicitmodelId: ""as missing, which can override caller intent and route sessions to an unintended fallback model; switch the check toundefined(or explicitly reject empty IDs) so provided values are handled consistently.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/sessions/createSessionWithInitialChat.ts">
<violation number="1" location="lib/sessions/createSessionWithInitialChat.ts:65">
P2: An explicitly supplied `modelId: ""` is treated as absent, despite the stated undefined-only fallback. Check for `undefined` rather than truthiness so supplied values are forwarded consistently (or reject empty IDs at validation).</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client as HTTP Client/Caller
participant Handler as handleStartChatRun
participant Provisioner as provisionRunSession
participant Creator as createSessionWithInitialChat
participant DB as Database
Note over Handler,DB: Headless Run Session Creation Flow
Client->>Handler: POST /chat/run (authenticated)
Handler->>Handler: Resolve modelId from task configuration
Handler->>Provisioner: provisionRunSession({accountId, title, artistId, modelId})
Note over Handler,Provisioner: modelId now forwarded (was omitted)
Provisioner->>Creator: createSessionWithInitialChat({accountId, title, chatTitle, artistId, modelId})
Note over Provisioner,Creator: modelId passed through
Creator->>DB: Insert session
DB-->>Creator: Session record
alt modelId provided
Creator->>DB: insertChat({id, session_id, title, model_id: modelId})
Note over Creator,DB: Sets chats.model_id to resolved model
else no modelId
Creator->>DB: insertChat({id, session_id, title})
Note over Creator,DB: model_id omitted (default behavior)
end
DB-->>Creator: Chat record
alt Chat insert failed
Creator->>DB: deleteSessionById(session.id)
DB-->>Creator: Rollback complete
Creator-->>Provisioner: {ok: false, reason: "insert"}
Provisioner-->>Handler: Throw error
else Success
Creator-->>Provisioner: Session with initial chat
Provisioner-->>Handler: ProvisionedRunSession
end
Note over Client,DB: Later: Chat generation reads model
Handler->>Handler: mintEphemeralAccountKey()
Handler-->>Client: Response with session
Note over Client,DB: Downstream: generation reads chats.model_id
Note over DB: chats.model_id now accurate (was always default)
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| id: generateUUID(), | ||
| session_id: session.id, | ||
| title: chatTitle, | ||
| ...(modelId ? { model_id: modelId } : {}), |
There was a problem hiding this comment.
P2: An explicitly supplied modelId: "" is treated as absent, despite the stated undefined-only fallback. Check for undefined rather than truthiness so supplied values are forwarded consistently (or reject empty IDs at validation).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/sessions/createSessionWithInitialChat.ts, line 65:
<comment>An explicitly supplied `modelId: ""` is treated as absent, despite the stated undefined-only fallback. Check for `undefined` rather than truthiness so supplied values are forwarded consistently (or reject empty IDs at validation).</comment>
<file context>
@@ -50,7 +58,12 @@ export async function createSessionWithInitialChat({
+ id: generateUUID(),
+ session_id: session.id,
+ title: chatTitle,
+ ...(modelId ? { model_id: modelId } : {}),
+ });
if (!chat) {
</file context>
| ...(modelId ? { model_id: modelId } : {}), | |
| ...(modelId !== undefined ? { model_id: modelId } : {}), |
1 participant
Closes the "persist the resolved
modelId" row of chat#1918. No docs change — no contract or response shape changes.The bug
handleStartChatRun.tsL40 resolvesmodelIdand passes it to the workflow at L59, but theprovisionRunSessioncall at L44 omitted it:createSessionWithInitialChatthen inserted the chat with nomodel_id. Because the generation path reads the model from the chat row (handleChatWorkflowStream.tsL93:chat.model_id ?? DEFAULT_MODEL_ID),chats.model_idrecorded the default for every headless run.Impact: the column lies
Measured on prod, scheduled-generation rooms over 30 days (test accounts excluded):
chats.model_idsaysanthropic/claude-sonnet-5anthropic/claude-haiku-4.5moonshotai/kimi-k3anthropic/claude-haiku-4.5openai/gpt-5.4-minianthropic/claude-haiku-4.5anthropic/claude-haiku-4.5anthropic/claude-haiku-4.5anthropic/claude-sonnet-4.5anthropic/claude-haiku-4.5gpt-4o-minianthropic/claude-haiku-4.5The runs are correct — the configured model is applied. Only the recorded column is wrong, which silently invalidates any audit or dashboard keyed on
chats.model_id. This also corrects the hypothesis originally written on the issue ("the customer's selection is not reaching the run"), which the data disproves.The fix
Thread
modelIdthrough the three layers it was already available in:handleStartChatRun→ passesmodelIdtoprovisionRunSessionprovisionRunSession→ forwards it tocreateSessionWithInitialChatcreateSessionWithInitialChat→ setsmodel_idon the insert, omitted entirely when undefined so existing behavior is unchanged for callers that don't supply oneTests
RED→GREEN at each layer (4 new tests):
createSessionWithInitialChatcreateSessionWithInitialChatprovisionRunSessionhandleStartChatRun(the bug site)The
handleStartChatRuntest was verified genuinely RED by removing themodelIdargument and confirming anAssertionError(not a setup error), then GREEN on restore.pnpm exec vitest run lib/chat lib/sessions lib/agent→ 136 files, 936 tests — all pass with the fix appliedpnpm exec eslint lib/sessions lib/chat/runs→ cleanpnpm exec tsc --noEmit→ no errors in any touched fileVerification note
Done-when in the issue asks that a task set to a non-default model records that model on the chat and in
usage_events. This PR fixes the chat row. Whetherusage_events.model_idderives from the same source should be confirmed against a preview run before the issue row is closed.🤖 Generated with Claude Code
Summary by cubic
Persist the resolved
modelIdto the chat row so headless runs record the correct model. Fixes the bug from chat#1918 wherechats.model_idalways stored the default.modelIdfromhandleStartChatRun→provisionRunSession→createSessionWithInitialChat.model_idon chat insert when provided; omit when undefined.usage_events.model_idin preview per chat#1918.Written for commit da5a92e. Summary will update on new commits.