fix(coderd): stop manual title generation from writing to chat_messages - #27087
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2959572b94
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review re: Track manual title spend in chat usage limits Valid catch, but this is the intentional tradeoff of the fix rather than an oversight, and the blast radius is smaller than it looks: checkUsageLimit still runs before every title generation, so a user who is over their chat limit is still blocked from these endpoints. The gap is only that title-call spend by an under-limit user no longer advances the counter. The clean fix is to rebase chat usage limits onto AI Gateway spend data entirely, which is a redesign, not something I want to fold into this bug fix. Accepting the gap here, and it's called out in the PR description. |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
4226d1a to
e3dad7f
Compare
Manual title generation (RegenerateChatTitle/ProposeChatTitle) recorded token cost by inserting a hidden assistant message into chat_messages and immediately soft-deleting it. Both statements fire AFTER-STATEMENT triggers that sync chats.history_version to snapshot_version. This was the only chat_messages writer outside the chatstate state machine, so it bumped history_version without advancing snapshot_version or publishing a state update. An in-flight generation task captures history_version at spawn and verifies it via a fence at commit. The out-of-band bump made that fence fail, exiting the task via the non-retryable path that performs no cleanup because it assumes a replacement task exists. None is spawned, so the chat stayed running forever and the UI showed "Thinking" indefinitely. Remove the chatd-side accounting path entirely. AI Gateway (aibridge) already records title-call usage independently in aibridge_interceptions and aibridge_token_usages, so the hidden-message accounting was redundant. persistManualTitle now only performs the optimistic title write. This intentionally removes title-generation cost and tokens from chatd's own chat-level cost surfaces (GetChatCostSummary and the spend-limit query paths); that usage now lives only in AI Gateway data.
…CTURE.md Drop the redundant WARNING comment on InsertChatMessages (and its generated propagation into querier.go/queries.sql.go) and the standalone ARCHITECTURE.md paragraph. The rule that every chat_messages write must go through a state machine transition now lives as a single sentence alongside the existing runtime-code guardrail in the message revision section.
e3dad7f to
240b118
Compare
2 participants
Coder Agents chats could get stuck showing "Thinking" forever when a title regenerate/propose request ran while a generation was in flight.
Manual title generation recorded token cost by inserting a hidden assistant message into
chat_messagesand immediately soft-deleting it. Triggers on that table syncchats.history_versiontosnapshot_version, so this out-of-band write broke thehistory_versionfence of an in-flight generation task, killing it without a replacement and leaving the chat stuck inrunning.Remove the accounting path entirely; AI Gateway already records title-call usage in
aibridge_interceptions/aibridge_token_usages. The manual title endpoints no longer write tochat_messagesat all, and new regression tests asserthistory_versionstays untouched. Note this intentionally drops title-generation cost from chatd's chat-level cost surfaces; it still counts against the user's AI budget via AI Gateway.Closes CODAGT-595