mirror of
https://github.com/coder/coder.git
synced 2026-10-16 14:48:56 +08:00
Michael Suchacz
f5092aaa4a
Adds deployment options for the chat limits that shape turn behavior and API acceptance, replacing hardcoded constants in `coderd/x/chatd` and the chat API. Defaults are unchanged. | Option | Default | Limit | |---|---|---| | `CODER_CHAT_MAX_STEPS_PER_TURN` | 1200 | steps (model responses) in a chat turn | | `CODER_CHAT_MAX_GENERATION_RETRIES` | 25 | consecutive retries after a model generation fails with a transient error | | `CODER_CHAT_MAX_QUEUED_MESSAGES_PER_CHAT` | 20 | messages that can be queued in a chat | | `CODER_CHAT_MAX_ATTACHMENTS_PER_CHAT` | 50 | files linked to a chat; linking more deletes the earliest uploads | | `CODER_CHAT_MAX_PROMPT_BYTES` | 131072 | system prompt, plan mode instructions, and each user's custom prompt | | `CODER_CHAT_MAX_CONCURRENT_RECORDING_UPLOADS` | 25 | desktop recordings each server stores at the same time | `codersdk.DefaultChat*` constants remain the single source of truth. Values below 1 fail startup validation, and each option's description ends with "Must be at least 1." `chatd.New` fills any zero field in `chatd.Config.Limits` with its default once, `LimitsFromConfig` is a plain conversion, and the code that receives a limit as a parameter (the `chatstate` queue and file link checks, the turn step check, `chatretry.Retry`, and `chatadvisor.NewRuntime`) returns an error for a limit below 1 instead of choosing a default or disabling the limit. The options are documented in the generated configuration reference and in the platform controls docs. `CODER_CHAT_MAX_GENERATION_RETRIES` counts consecutive retries, and the count resets after each successful step. The default allows 25 retries (26 attempts) where the old constants allowed 24. The same limit now applies to each nested advisor model call and to background title, summary, and turn status label generation, which previously used fixed `chatretry` budgets. Chat creation and tool result bodies keep the fixed 256 KiB limit from main. The prompt-saving endpoints accept bodies up to twice the prompt limit, for JSON escaping and characters that sanitization removes, but never less than 256 KiB, so small prompt limits still return the 400 length error. At the default this equals the 256 KiB limit on main. Four settings from earlier revisions were removed, and the fixed limits from main are back: - `CODER_CHAT_MAX_TOOL_OUTPUT_BYTES`: the agent already keeps a 16 KiB head and a 16 KiB tail of command output, so raising it had no effect and lowering it cut the tail. - `CODER_CHAT_MAX_DYNAMIC_TOOLS_PER_CHAT`: tool definitions share the 256 KiB chat creation body limit, so raising the count did little. - `CODER_CHAT_DEBUG_MAX_BODY_BYTES`: one setting covered two unrelated debug caps. - `CODER_CHAT_DEBUG_MAX_TEXT_RUNES`: the name did not say what it applied to. Root and subagent concurrency caps stay fixed because they are licensing behavior, and `MaxChatFileSizeBytes` stays fixed because the web UI hardcodes the generated constant for client-side checks. `codersdk.MaxChatFileIDs` is removed; the web UI's "Attachment expired" tooltip no longer quotes the number because the cap is now per deployment, and it says that attaching a file beyond the limit permanently deletes the chat's earliest-uploaded attachments. ## Review record | Round | Reviewed head | Result | Disposition | |---|---|---|---| | Codex 1 and 2 (Sep 2 to 3) | `7f10a1572f8`, `8ee09f8fa3b`, `197bb78d5b` | 10 findings, then OK on `197bb78d5b` | all threads resolved | | Codex 3 (Sep 17) | `e4efac249a5` (pre-rebase) | 4 findings | advisor retry limit confirmed and fixed in `9e462b7b3d0` (rebased from `67341e458e8`); ARCHITECTURE.md TODOs added; JSON-escaping body sizing and streamed-argument rune cap declined as pre-existing design (evidence in threads) | | Codex 4 (Sep 18) | `9e462b7b3d0` | 6 findings | recording evicted at attachment cap 1 confirmed and fixed in `5b61895decb`; `Limits` field docs and zero-value fallback documented; prompt retro-enforcement, generated multi-sentence lines, and tool-output truncation metadata declined as pre-existing design (evidence in threads) | | Codex 5 (Sep 18) | `5b61895decb` | 1 finding | over-cap batch of already-linked files accepted after lowering the cap: confirmed and fixed in `f44063e74e5` | | mafredri (Sep 23) | `f44063e74e5` | 24 threads, changes requested | all fixed in `05e3e3d622a`, `c1f78e09e8b`, `709ddb67977`, `d0587a49829`, `ca4347b24de`, and `1f6c9e2a46f`, then answered and resolved; the ARCHITECTURE.md text was written at the author's direction and approved by the author | | Sibling sweep (Sep 24) | `1f6c9e2a46f` | author-requested audit of all 24 mafredri threads for sibling issues, plus a test refactor | fixed in `37eaf07d34d` through `1f14ac041b7`: the advisor retry limit, `chatretry.Retry`, and the turn step check reject values below 1 instead of disabling retries or the limit; limit tests run against both the default and configured values set through `chatd.Config` or `DeploymentValues`; the expired-attachment tooltip is concrete. A required coder-frontend-review run (8 roles in one agent, CD re-run once) found a stale story comment, removed. | | Codex 6 (Sep 27) | `a71383e3a31` | OK, no findings | requested after the rebase at the author's direction, past the six-pass review budget | The branch was rebased onto main `4851c7b60c8` on Sep 27; the previous head was `91b84000adb`, and the earlier SHAs in the review record predate the rebase. Conflicts in the `chatstate` transitions and `CreateChat` keep both main's `InitialStatus` and this PR's `MaxFileLinks`, and main's rewrite of `advisor_internal_test.go` in #28754 keeps main's version plus this PR's limit tests. Two tests added on main needed the new retry arguments: `TestGenerateStructuredTitleWithUsage_FallsBackToTextWhenToolChoiceRejected` and `TestAdvisorRunStreamsReasoningDeltasSeparatelyFromAdvice`. Validation on `a71383e3a31`: the pre-commit hook (generation, golangci-lint, build), the full `coderd/x/chatd/...` suite (20 packages), the chat tests in `codersdk`, `coderd`, and `enterprise/coderd`, the CLI help tests, race-detector runs of the `chatd`, `chatstate`, `chatadvisor`, and `chatretry` packages, and `go build` plus `go vet` on each of the 27 commits. No remote UAT was run. > Xum acted on behalf of @ibetitsmike for this pull request.