feat: add reasoning effort slider to the chat model selector - #26977

The frontend wiring and Storybook coverage are well structured. The slider component reuse, keyboard navigation stories, and pickReasoningEffort unit tests show care. Two mechanical findings from the first pass below.

This is a first-pass review only: these are mechanical findings from Netero. The full review panel has not yet reviewed this PR and will review after these findings are addressed.

1 P2, 1 P3.

coderd/x/chatd/chatd.go:2951

P3 [CRF-2] appendMessageFields no longer appends to params.ReasoningEffort, so the ReasoningEffort slice stays at length 0 while every other field slice is length 1. PostgreSQL 10+ pads mismatched UNNEST lengths with NULLs, so the insert succeeds and reasoning_effort is NULL, which is functionally correct. But the explicit insertion path at line 2830 sets ReasoningEffort: []string{""} to keep arrays aligned. The inconsistency is fragile: if any validation or middleware starts asserting array length parity, this path breaks.

Add params.ReasoningEffort = append(params.ReasoningEffort, "") to keep the arrays aligned. (Netero)

🤖

🤖 This review was automatically generated with Coder Agents.

Resolved in the latest push.

CRF-2 is fixed by appending "" to ReasoningEffort in appendMessageFields, matching the explicit insertion path and keeping the UNNEST arrays aligned.

This reply was generated with Coder Agents.

CRF-1 and CRF-2 verified fixed in 76f68fe. The constraint consistency is restored and the UNNEST arrays are aligned. One new finding from this pass.

This is a first-pass review only: these are mechanical findings from Netero. The full review panel has not yet reviewed this PR and will review after these findings are addressed.

1 P2.

coderd/x/chatd/chatd_test.go:11663

P2 [CRF-3] Five backend test functions were deleted with no replacement, but the code paths they covered still exist:

• TestEditMessageReasoningEffort: edit preserves effort by default, overrides when explicitly set
• TestPromoteQueuedPreservesReasoningEffort: promote carries effort from queued row to chat
• TestSendMessageQueuesReasoningEffort: queuing preserves reasoning effort
• TestPatchChatMessage/ReasoningEffort: HTTP-level edit effort preservation/override
• TestPatchChatMessage/RejectsInvalidReasoningEffort: invalid effort rejection on edit

EditMessage at chatd.go:1679-1691 still accepts ReasoningEffort and chatstate.EditMessageInput.ReasoningEffortOverride still threads it through. No remaining test covers edit+effort, promote+effort, or queue+effort after this PR. The frontend stories cover the UI slider but not the backend round-trip. (Netero)

🤖

🤖 This review was automatically generated with Coder Agents.

CRF-1, CRF-2, CRF-3 all verified fixed. The slider component is well-structured, the prop threading mirrors the established model selection pattern, and test density at 63.8% is solid with genuine assertions throughout.

One P1 that is the likely root cause of the Go CI failures, plus two P3s, one P4, two Nits, and three Notes.

1 P1, 2 P3, 1 P4, 2 Nit, 3 Note.

"The comment paraphrases six lines of self-documenting code. Delete." — Gon, on the narrating comment pattern

site/src/pages/AgentsPage/utils/reasoningEffort.ts:2

P3 [CRF-6] formatReasoningEffort("xhigh") produces "Xhigh", which reads like an internal enum value. Every other effort level renders as a real English word (None, Minimal, Low, Medium, High, Max). A simple display map with a fallback to the capitalize path fixes it:

const effortLabels: Record<string, string> = { xhigh: "Extra high" };
export const formatReasoningEffort = (value: string): string =>
  effortLabels[value] ?? value.charAt(0).toUpperCase() + value.slice(1);

The test and the EffortRow story badge assertion would need updating. If the Figma spec genuinely says "Xhigh", push back on the spec. (Leorio P3, Luffy P3)

🤖

site/src/pages/AgentsPage/utils/reasoningEffort.ts:5

Nit [CRF-8] Doc comment says "the highest effort" but the code returns efforts[efforts.length - 1], which is only "highest" if the array is sorted ascending. The server sends them sorted today, but the comment documents a semantic guarantee the function doesn't enforce. Either note the precondition ("efforts must be sorted ascending") or describe the mechanical behavior ("the last selectable effort"). (Leorio)

🤖

🤖 This review was automatically generated with Coder Agents.

All prior findings verified fixed. CRF-6 ("Xhigh" label) panel vote: 5/5 accept the product decision defense — the cosmetic inconsistency is bounded by slider position context and does not affect functionality.

The iteration from R3 to R4 was thorough: the isEditReasoningEffortDirtyRef fix for CRF-12 went beyond the Note's assessment and improved the design (truthiness would break on default-value selection since "" is falsy). The InfoIcon keyboard accessibility fix added proper <button> wrapping with story coverage. Test density climbed to 67.8%.

One residual Nit below.

"I tried to build a case against this change and couldn't." — Pariston

site/src/pages/AgentsPage/utils/reasoningEffort.test.ts:21

Nit [CRF-13] Test description says "falls back to the highest server-provided effort" but CRF-8 fixed the pickReasoningEffort doc comment to say "the last selectable effort" (since the function returns efforts[efforts.length - 1] without enforcing sort order). The assertion is correct, but the description makes an ordering claim the function doesn't enforce. Consider: "falls back to the last selectable effort when value and default are out of range". (Leorio)

🤖

🤖 This review was automatically generated with Coder Agents.

Stamp