gateway/session-utils: honor per-agent thinkingDefault in buildGatewaySessionRow#70302
Conversation
Greptile SummaryAdds the same three-step Confidence Score: 5/5Safe to merge — single call-site change with thorough test coverage and no P0/P1 issues found. The implementation is correct: resolveThinkingDefault does not already cover per-agent thinkingDefault (it only reads cfg.agents.defaults.thinkingDefault), so step 2 adds the missing level without redundancy. sessionAgentId is normalized before use, optional-chaining is correct, and the 6-test suite covers all fallback branches plus both TUI-feeding code paths. No files require special attention. Reviews (1): Last reviewed commit: "gateway/session-utils: honor per-agent t..." | Re-trigger Greptile |
|
Closing this as duplicate or superseded after Codex automated review. PR #70302 identifies a real gateway session-row thinking-default gap, but the remaining work is now superseded by the open maintainer replacement PR #72324, which explicitly carries forward #70302 and preserves contributor credit. Current main has not shipped the fix, so this should close as superseded rather than implemented. Best possible solution: Close #70302 as superseded by the maintainer-owned #72324. Continue the fix there, preserving #70302 attribution, and make the final implementation keep explicit persisted session overrides in thinkingLevel while exposing inherited per-agent/per-model/global defaults through thinkingDefault with correct precedence and regression coverage. What I checked:
So I’m closing this here and keeping the remaining discussion on the canonical linked item. Codex Review notes: model gpt-5.5, reasoning high; reviewed against edf40ab6c944. |
Summary
state.sessionInfo.thinkingLevelis fed throughsessions.list/sessions.changed, which flow throughbuildGatewaySessionRow(...)insrc/gateway/session-utils.ts. That builder currently reads onlyentry?.thinkingLevel, with no fallback. When a session has no persistedthinkingLevel(fresh session, post-restart, new webchat connection), the row'sthinkingLevelisundefined, the TUI picks up whatever it defaults to (typically the model-centric default fromresolveThinkingDefaultForModel, which foropenai-codex/gpt-5.4is"low"), and drifts below the configured agent level.chat.historyRPC handler uses (PR gateway/chat: honor per-agent thinkingDefault in chat.history fallback #70259):entry.thinkingLevel— persisted session value (unchanged priority).cfg.agents.list[].thinkingDefaultviaresolveAgentConfig— per-agent override.resolveThinkingDefault(...)— which internally covers per-model override,cfg.agents.defaults.thinkingDefault, anthropic heuristics, and the model-centric fallback.Scope
src/gateway/session-utils.tsinsidebuildGatewaySessionRow(...). Both RPC paths that feed the TUI —sessions.list(listSessionsFromStore) andsessions.changedbroadcasts (loadGatewaySessionRow→buildGatewaySessionRow) — benefit from the one edit.resolveAgentConfighelper fromsrc/agents/agent-scope.ts(same one PR gateway/chat: honor per-agent thinkingDefault in chat.history fallback #70259 uses in the chat.history handler), so agent id normalization is shared./thinkcommand path,auto-reply/reply/model-selection,agent-command,cron/isolated-agent, and plugin runtime callers are untouched — this is strictly the TUI-effective SessionRow fallback.Deliberate design notes
buildGatewaySessionRowis synchronous and is called from many places (including hot paths).resolveThinkingDefaultacceptscatalog?as optional. This PR intentionally calls it without a model catalog, which means the Claude-4.6 "adaptive" heuristic that depends oncatalogCandidate.namedoes not fire in this sync row-builder. The opus-4-7 / per-model / global / model-centric branches continue to work because they do not require the catalog. Introducing async + catalog plumbing intobuildGatewaySessionRowwould be a broader refactor outside this PR's scope; it can be added later if the Claude-4.6 sonnet-adaptive case turns out to matter for the TUI footer in practice.thinkingLevel(including the catalog-aware heuristic, because it is async and has catalog access). When the TUI issues bothsessions.listandchat.historyfor the same session, the two paths now produce consistent values (and PR gateway/chat: honor per-agent thinkingDefault in chat.history fallback #70259 remains the authoritative, catalog-aware one for chat.history).Rollout / verification note
Because the TUI footer caches
state.sessionInfo.thinkingLevellocally after the initial handshake, a live client that was already connected before this change may continue to show the stale value for its existing session. A reconnect / TUI restart is the expected verification step after rollout — the new SessionRow values take effect on the nextsessions.list/sessions.changeddelivery.Tests
New focused suite
src/gateway/session-utils.thinking-fallback.test.ts— 6 tests, all passing locally:entry.thinkingLevelwins over per-agent default (regression guard).thinkingDefaultfills the gap when the entry has nothinkingLevel.agents.defaults.thinkingDefaultfills the gap when per-agent is unset.undefined).sessions.listandloadGatewaySessionRowstay consistent onthinkingLevel(covers both TUI-feeding paths).sessionKey: "agent:main:main"resolves via the normalized agent lookup.Test plan
pnpm test src/gateway/session-utils.thinking-fallback.test.ts— 6/6 PASSpnpm test src/gateway/session-utils.test.ts src/gateway/session-utils.fs.test.ts src/gateway/session-utils.search.test.ts src/gateway/session-utils.subagent.test.ts— 139/139 PASSpnpm check(tsgo core + core:test + extensions + extensions:test, oxlint, import-cycle + madge checks all green; ran viascripts/committer)Related
chat.historyRPC handler (catalog-aware sibling).🤖 Generated with Claude Code