Skip to content

Add GPT-5.6 max reasoning effort - #481

Merged
m-aebrer merged 8 commits into
aebrer:masterfrom
maxscheurer:feature/issue-338-gpt-56-max-effort
Aug 21, 2026
Merged

Add GPT-5.6 max reasoning effort#481
m-aebrer merged 8 commits into
aebrer:masterfrom
maxscheurer:feature/issue-338-gpt-56-max-effort

Conversation

@maxscheurer

Copy link
Copy Markdown
Contributor

Closes #338

Add model-aware max reasoning effort for GPT-5.6 Sol, Terra, and Luna while preserving xhigh as a distinct tier. Codex-style ultra is documented as separate client orchestration rather than sent as a raw effort value.

Implementation plan posted as a comment below.

@maxscheurer

Copy link
Copy Markdown
Contributor Author

Implementation Plan

Problem analysis

dreb models reasoning as a normalized ThinkingLevel shared across the AI, agent, CLI, RPC, subagent, TUI, Telegram, and dashboard layers. That scale currently ends at xhigh. GPT-5.6 Sol, Terra, and Luna expose a separate higher wire-level effort, max, so mapping xhigh to max would lose a real tier and change existing behavior.

The implementation should add max as a seventh normalized level and expose it only for models that advertise the capability. Provider resolution must preserve the existing behavior of models such as Claude and Kimi, where dreb's existing xhigh already maps to a provider-native max value.

OpenAI Codex source confirms that ultra is not another wire-level effort: it activates local proactive multi-agent orchestration and downgrades the provider request to max. This PR will implement max and document that distinction; it will not add a misleading raw ultra thinking level.

Deliverables

1. Extend the normalized scale and model capabilities

  • Add max to the AI and agent ThinkingLevel unions.
  • Add a model capability helper for native max support, initially covering the GPT-5.6 alias and Sol/Terra/Luna variants across provider-qualified model IDs.
  • Extend effective-level resolution and ordered fallback so unsupported requests resolve predictably as max → xhigh → high while non-reasoning models still resolve to off.
  • Preserve supportsXhigh() and all existing xhigh behavior independently from the new capability.

Primary files:

  • packages/ai/src/types.ts
  • packages/ai/src/models.ts
  • packages/ai/src/providers/simple-options.ts
  • packages/agent/src/types.ts
  • packages/agent/src/agent-loop.ts
  • packages/coding-agent/src/core/thinking.ts

2. Carry max through provider request paths safely

  • Extend OpenAI Responses, OpenAI Codex Responses, Azure Responses, and OpenAI-compatible option types and request builders to carry max.
  • Send reasoning.effort: "max" unchanged for GPT-5.6 models on direct OpenAI and ChatGPT OAuth / Responses Lite paths.
  • Apply model-aware fallback before unsupported providers receive a request, including Google, older OpenAI families, Qwen, Kimi, Anthropic, and Bedrock paths.
  • Keep existing provider-native mappings intact, especially xhigh → max for Claude adaptive thinking and Kimi K3.
  • Upgrade the pinned OpenAI SDK within the current major line from 6.26.0 to a v6 release whose types include max, then resolve any compile/test fallout without introducing casts solely to bypass stale SDK types.

Primary files:

  • packages/ai/src/providers/openai-responses.ts
  • packages/ai/src/providers/openai-codex-responses.ts
  • packages/ai/src/providers/azure-openai-responses.ts
  • packages/ai/src/providers/openai-completions.ts
  • packages/ai/src/providers/anthropic.ts
  • packages/ai/src/providers/amazon-bedrock.ts
  • packages/ai/src/providers/google.ts
  • packages/ai/src/providers/google-gemini-cli.ts
  • packages/ai/src/providers/google-vertex.ts
  • packages/ai/package.json
  • package-lock.json

3. Update configuration, validation, routing, and persistence

  • Accept max in CLI flags and model shorthand, settings, SDK/extension types, RPC commands, dispatch-arbiter output, and subagent single/parallel/chain overrides.
  • Validate explicit model-bound max overrides using the new capability and return a clear unsupported-level error.
  • Extend session availability, cycling, restoration, and model-switch clamping with the new ordered tier.
  • Keep persisted historical levels backward-compatible; no session migration should be required.
  • Consolidate or consistently update duplicated canonical level lists so no entry path silently rejects max.

Primary files:

  • packages/coding-agent/src/cli/args.ts
  • packages/coding-agent/src/main.ts
  • packages/coding-agent/src/core/agent-session.ts
  • packages/coding-agent/src/core/settings-manager.ts
  • packages/coding-agent/src/core/model-resolver.ts
  • packages/coding-agent/src/core/dispatch-arbiter.ts
  • packages/coding-agent/src/core/tools/subagent.ts
  • packages/coding-agent/src/modes/rpc/rpc-types.ts
  • packages/coding-agent/src/modes/rpc/rpc-mode.ts
  • packages/coding-agent/src/modes/rpc/rpc-client.ts
  • packages/coding-agent/src/sdk.ts
  • packages/coding-agent/src/extensions/types.ts

4. Make selectors and displays model-aware

  • Add max to the TUI selector only when the current model supports it; retain the existing session-provided availability flow for cycling and selection.
  • Add a distinct max description and theme border token, updating built-in themes, schema validation, and theme docs together.
  • Replace dashboard session cycling's static level list with server/session-provided available levels so unsupported models do not offer max; propagate that availability through RPC/dashboard runtime state and model-change updates.
  • Allow dashboard settings/configuration forms to represent max while preserving runtime model validation.
  • Bring Telegram validation in line with the canonical scale, fixing its current omission of xhigh while adding max.

Primary files:

  • packages/coding-agent/src/modes/interactive/components/thinking-selector.ts
  • packages/coding-agent/src/modes/interactive/theme/theme.ts
  • packages/coding-agent/src/modes/interactive/theme/theme-schema.json
  • packages/coding-agent/src/modes/interactive/theme/dark.json
  • packages/coding-agent/src/modes/interactive/theme/light.json
  • packages/dashboard/src/shared/protocol.ts
  • packages/dashboard/src/server/runtime-pool.ts
  • packages/dashboard/src/client/screens/session.tsx
  • packages/dashboard/src/client/screens/settings.tsx
  • packages/dashboard/src/client/state/store.ts
  • packages/telegram/src/commands/agent.ts
  • packages/telegram/src/commands/core.ts

5. Update public documentation and examples

  • Document max as GPT-5.6-specific and distinct from xhigh.
  • Explain that Codex ultra means max plus local multi-agent orchestration and is intentionally not exposed as a raw effort value by this change.
  • Update every documented level list and relevant SDK/example annotation; check the root README as required by repository policy.

Documentation to inspect/update:

  • README.md
  • packages/coding-agent/README.md
  • packages/coding-agent/docs/agent-models.md
  • packages/coding-agent/docs/settings.md
  • packages/coding-agent/docs/rpc.md
  • packages/coding-agent/docs/models.md
  • packages/coding-agent/docs/tui.md
  • packages/coding-agent/docs/themes.md
  • packages/coding-agent/examples/sdk/README.md
  • Relevant SDK and extension examples containing level lists

Acceptance criteria

  • max is a distinct normalized level; selecting xhigh on GPT-5.6 still sends xhigh.
  • GPT-5.6, Sol, Terra, and Luna expose max through model-aware TUI/dashboard session selectors and session cycling.
  • CLI/model shorthand, settings, RPC, SDK, dispatch, and subagent overrides can represent max.
  • Direct OpenAI and OpenAI Codex requests for GPT-5.6 emit reasoning.effort: "max", including Responses Lite payloads.
  • Explicit max overrides for unsupported models fail with a clear validation error; non-explicit model switches/defaults clamp in the documented order.
  • Existing Claude, Kimi, Qwen, Google, Bedrock, and pre-5.6 OpenAI reasoning behavior remains unchanged.
  • Telegram accepts the complete supported scale, including the previously omitted xhigh.
  • ultra is not sent as a provider effort and its orchestration semantics are documented.
  • Tests, type-checking, formatting, build, workspace-link verification, and the full test suite pass.

Testing approach

AI/provider tests

Modify or add focused cases in:

  • packages/ai/test/supports-xhigh.test.ts or a dedicated supports-max.test.ts: GPT-5.6 alias/variants and negative families/providers.
  • packages/ai/test/openai-codex-stream.test.ts and WebSocket/Responses Lite tests: emitted max, preserved xhigh, and existing minimal-to-low clamp.
  • OpenAI/Azure Responses request tests: direct emitted max and unsupported-family fallback.
  • packages/ai/test/openai-completions-kimi.test.ts and Qwen tests: existing xhigh native mappings remain unchanged and unsupported max falls back safely.
  • Anthropic, Bedrock, and Google thinking tests: provider-specific mappings/budgets remain valid with the extended union.

Agent and coding-agent tests

Modify or add cases in:

  • packages/agent/test/agent-loop.test.ts: effective event level for supported and unsupported max.
  • packages/coding-agent/test/thinking.test.ts: capability validation and fallback order.
  • packages/coding-agent/test/agent-session-model-switch-thinking.test.ts: availability, cycling, restoration, and model-switch clamping.
  • packages/coding-agent/test/args.test.ts and model-resolver.test.ts: CLI and shorthand parsing.
  • packages/coding-agent/test/subagent-thinking-override.test.ts, dispatch-arbiter.test.ts, and subagent-arbiter.test.ts: accepted supported overrides and rejected unsupported overrides.
  • RPC settings/command tests: accepted max, canonical error lists, and available-level state.
  • Theme/selector tests: new description/color token and exhaustive theme schema coverage.

Dashboard and Telegram tests

  • Extend dashboard runtime/protocol and browser/integration tests to verify session controls use server-provided available levels and update after a model switch.
  • Add or extend Telegram command tests to verify xhigh and max acceptance and invalid-level rejection.

Validation commands

  • Run targeted Vitest files while developing.
  • Run Biome on every changed source/test/doc-adjacent file where applicable.
  • Run npm run build before any real-binary verification.
  • Run npm run verify-workspace-links.
  • Run npm test with live API tests skipped according to repository convention, then address every failure.

Risks and open questions

  • Provider leakage: extending the union without model-aware clamping could send max to unsupported APIs. Provider tests must cover negative paths, not only GPT-5.6 success.
  • Semantic regression: Claude and Kimi already use native max as the downstream representation of dreb xhigh; the new normalized level must not alter that established mapping.
  • Duplicated enums: many UI/protocol/settings surfaces hardcode level lists. Missing one can create runtime rejection despite successful type-checking.
  • Dashboard synchronization: model-aware controls require available levels to change atomically with the active model rather than being inferred independently in the browser.
  • SDK upgrade: stay on OpenAI v6 to limit dependency risk, but compile and run all AI/provider tests after updating the exact pin and lockfile.
  • Ultra scope: implementing Codex-equivalent ultra later would require an explicit orchestration design tied to dreb's subagent system; adding the literal alone is intentionally excluded.

Plan created by mach6

@maxscheurer

Copy link
Copy Markdown
Contributor Author

Progress Update

Implemented the GPT-5.6 max reasoning tier across the normalized AI/agent scale, provider request paths, CLI/model shorthand, settings, RPC, dispatch arbitration, subagents, session persistence and model switching.

  • Added model-aware max → xhigh → high fallback while preserving existing xhigh semantics and Claude/Kimi provider-native mappings.
  • Added native max passthrough for GPT-5.6 Sol, Terra, Luna, and aliases across OpenAI Responses, Codex/Responses Lite, Azure Responses, and compatible request types.
  • Updated TUI/dashboard selectors, cycling, theme tokens, Telegram validation, and atomic dashboard model/thinking availability updates.
  • Upgraded the pinned OpenAI SDK to 6.49.0.
  • Updated public docs and examples, including the distinction between raw max and Codex ultra orchestration.
  • Added regression tests for capability detection, fallback, provider payloads, session switching, RPC, subagents, dashboard state, and Telegram commands.

Verification completed: build, repository checks, workspace-link verification, and all test suites passed (5804 passed, 714 skipped live/conditional tests).

Commit: af57dfb


Progress tracked by mach6

@maxscheurer
maxscheurer marked this pull request as ready for review August 20, 2026 14:22
@m-aebrer

Copy link
Copy Markdown
Collaborator

hey so, supportsXhigh exists and allows us to flag models that support xhigh. We should likely have supportsMaxEffort or MaxThink or something too, so then we don't limit this change to just the GPT 5.6 series models. Some other models also support Max or could in the future.

@maxscheurer

Copy link
Copy Markdown
Contributor Author

Unverified Review Candidates — Pending Assessment

Review round: 1
Reviewed commit: af57dfb

These are unverified candidates. Severity reflects reviewer confidence; do not treat any item as a merge blocker until the assessment comment is posted.

Critical

No critical findings.

Important

No important findings.

Suggestions

1. Duplicated clamping logic between resolveEffectiveThinkingLevel and resolveReasoningEffort (confidence: 88)

  • packages/coding-agent/src/core/thinking.ts lines 22-23 and packages/ai/src/providers/simple-options.ts lines 36-38 contain identical max→xhigh→high fallback chains.
  • coding-agent already imports from @dreb/ai. Exporting resolveReasoningEffort from @dreb/ai's public barrel and reusing it in resolveEffectiveThinkingLevel would eliminate the duplication and prevent the two copies from diverging when future levels are added.

2. No test for adjustMaxTokensForThinking with "max" reasoning level (confidence: 85)

  • adjustMaxTokensForThinking is called by Bedrock and Google providers. When reasoningLevel is "max", clampReasoning maps it to "high" for budget calculation. There are no unit tests for this function with "max" (or "xhigh"). A test should verify that passing "max" produces the same budget as "high".

3. No test for Anthropic mapThinkingLevelToEffort with "max" input (confidence: 82)

  • When "max" is passed to the Anthropic provider, mapThinkingLevelToEffort handles it. This mapping is untested — a regression here would silently send the wrong effort level to the Anthropic API.

4. CLI --thinking max not tested in args parser (confidence: 80)

  • The args test only checks --thinking high. While "max" was added to VALID_THINKING_LEVELS, there's no test confirming parseArgs(["--thinking", "max"]) returns { thinking: "max" }. Low risk since the implementation is a simple array lookup.

5. resolveReasoningEffort fallback for xhigh on non-xhigh models lacks a direct test (confidence: 80)

  • The existing tests exercise the xhigh fallback indirectly via the max path. A test passing "xhigh" directly to a non-xhigh model (e.g., GPT-4o) would exercise the second if branch of resolveReasoningEffort independently.

Strengths

  • Thorough cross-cutting implementation: max is consistently threaded through all 65 files across 7 packages — types, providers, agent session, CLI, RPC, dashboard protocol, TUI, Telegram, and documentation.
  • Safe fallback chain: resolveReasoningEffort (max→xhigh→high) and clampReasoning prevent max from leaking to unsupported providers. Double-guarded by both the AI layer and coding-agent's resolveEffectiveThinkingLevel.
  • Existing behavior preserved: Claude, Kimi, Qwen, Google, Bedrock, and pre-5.6 OpenAI reasoning paths are verified unchanged. Kimi's native max (downstream of dreb xhigh) is unaffected.
  • Model-aware UI: Dashboard availableThinkingLevels updates atomically with model switches; TUI selector and session cycling are correctly gated by supportsMax().
  • Pre-existing bug fix: Telegram's missing xhigh acceptance is fixed alongside adding max.
  • Documentation completeness: All listed docs updated including root README, with ultra correctly documented as orchestration-only.

Agents run: code-reviewer, error-auditor, test-reviewer, completeness-checker, simplifier


Reviewed by mach6

@maxscheurer

Copy link
Copy Markdown
Contributor Author

Review Assessment

#481 (comment)

Classifications

Finding Classification Reasoning
1. Duplicated clamping logic nitpick Factual: Yes, similar fallback chains exist in both packages. Scope: Not required by any acceptance criterion. Practical: The two functions serve different architectural layers (AI package provider clamping vs coding-agent session resolution with defaults/non-reasoning handling). Duplication is intentional layering, not a bug. No user-visible harm.
2. No test for adjustMaxTokensForThinking with "max" deferred Factual: True, no direct unit test. Scope: Acceptance criteria say "Tests pass" but don't mandate specific new budget-calc test cases. Practical: clampReasoning maps max→high, same path as xhigh→high already tested. No realistic regression vector unique to "max" that existing coverage misses.
3. No test for Anthropic mapThinkingLevelToEffort with "max" deferred Factual: True, the new case "max": fallthrough lacks a dedicated test. Scope: Not an explicit acceptance criterion. Practical: Trivial fallthrough to existing case "xhigh": branch which is already tested. A regression would require removing the case label, which would cause a TypeScript exhaustiveness or lint error. Minimal risk.
4. CLI --thinking max not tested deferred Factual: True. Scope: Not explicitly required. Practical: Parsed from a static array; no meaningful regression vector unique to "max".
5. resolveReasoningEffort xhigh fallback lacks direct test nitpick Factual: True, xhigh branch is only exercised indirectly via max path. Scope: Not required. Practical: Trivial if statement already covered indirectly. No material risk.

Action Plan

No merge blockers. All findings are either nitpicks (intentional design choices) or deferred low-priority hardening opportunities.

Useful follow-ups (findings 2–4): Add targeted unit tests for the new "max" paths across adjustMaxTokensForThinking, Anthropic effort mapping, and CLI arg parsing. These are low-priority since the code paths are trivial fallthroughs or reuse of well-tested logic.


Assessment by mach6

@maxscheurer

Copy link
Copy Markdown
Contributor Author

Progress Update

Addressed maintainer feedback: made supportsMax() extensible like supportsXhigh() and removed GPT-5.6-specific wording throughout.

Changes (15 files)

  • packages/ai/src/models.ts — Restructured supportsMax() from a single-return regex to a multi-check pattern matching supportsXhigh(). Added proper JSDoc listing supported families. Future model families can be added as simple if clauses. Fixed misplaced JSDoc comment separation.
  • Error messages (thinking.ts, main.ts) — "choose a max-capable GPT-5.6 model" → "choose a max-capable model"
  • TUI descriptions (thinking-selector.ts, settings-selector.ts) — "GPT-5.6 maximum reasoning" → "Maximum reasoning effort"
  • Theme schema — "GPT-5.6 max" → "max"
  • Agent session — "GPT-5.6's native max tier" → "native max tier for supported models"
  • Documentation (README.md, ai/README.md, coding-agent/README.md, docs/agent-models.md, docs/models.md, docs/themes.md) — Reframed from "GPT-5.6 only" to "model-aware, currently GPT-5.6"
  • Tests — Updated test descriptions from "GPT-5.6 models" → "max-capable models"

All 5804 tests pass, build clean, workspace links verified.

Commit: 5f27652


Progress tracked by mach6

@maxscheurer

Copy link
Copy Markdown
Contributor Author

Unverified Review Candidates — Pending Assessment

Review round: 2
Reviewed commit: 5f27652

These are unverified candidates. Severity reflects reviewer confidence; do not treat any item as a merge blocker until the assessment comment is posted.

Critical

No critical findings.

Important

1. Custom themes silently replaced by dark theme on startup (confidence: 95)

  • thinkingMax is now a required color token in theme-schema.json and the TypeBox schema. User custom themes created before this change that lack thinkingMax will fail validation.
  • initTheme() catches the validation error with catch (_error) (underscore = intentionally discarded) and silently falls back to dark theme. The detailed "Missing required color tokens: thinkingMax" guidance is constructed but never shown to the user.
  • Users lose their custom theme after upgrading with no warning or indication of why.

2. Dashboard screens.test.tsx mock returns stale setModel response shape (confidence: 88)

  • The setModel mock in screens.test.tsx (line 219) returns { provider: "test", id: "m1" } — the old response shape.
  • The actual API now returns { model: { provider, id }, thinkingLevel, availableThinkingLevels }.
  • ModelSelectorModal's onSelected callback expects the new shape. If tests exercise the model selection flow, the stale mock would silently test wrong behavior (thinking levels not updating after model switch).

Suggestions

3. Qwen maxxhigh effort mapping not tested (confidence: 95)

  • QWEN38_PLUS_EFFORT_MAP in openai-completions.ts adds max: "xhigh", but the it.each table in openai-completions-qwen-chat-template.test.ts was not extended with a ["max", "xhigh"] case.

4. Dashboard setModel endpoint's expanded response shape not tested server-side (confidence: 90)

  • The POST /api/runtimes/:key/model handler now calls h.client.getState() after setModel() and returns { model, thinkingLevel, availableThinkingLevels }. No server-side test exists for this endpoint.

5. supportsMax uses redundant if-return-true pattern (confidence: 95)

  • if (/regex/.test(x)) return true; return false; can be simplified to return /regex/.test(x); since .test() already returns boolean. Note: supportsXhigh uses the multi-clause pattern intentionally for multiple model families.

6. Unrelated blank-line removals and required array reformatting inflate the diff (confidence: 90)

  • dark.json (6 blank lines removed), light.json (5 blank lines removed), and theme-schema.json (required array reformatted to multi-line) are formatting-only changes unrelated to the feature, adding ~20 lines of diff noise.

7. resolveReasoningEffort not tested for undefined effort or xhigh passthrough (confidence: 85)

  • The new function replaces clampReasoning in 4 providers but its undefinedundefined and xhigh-on-non-xhigh → high paths have no direct tests in supports-max.test.ts.

8. Dashboard model-switch endpoint non-atomic two-step operation (confidence: 85)

  • If setModel() RPC succeeds but getState() fails, the client gets 502 while the server has already switched. Self-correcting via next SSE sync, but transiently stale UI.

9. Theme schema thinkingMax required — no validation test (confidence: 82)

  • No theme validation tests exist; the addition of a new required property makes custom theme breakage harder to catch in CI.

10. Dispatch arbiter's expanded THINKING_LEVELS set not tested (confidence: 80)

  • The arbiter's THINKING_LEVELS Set was expanded to include max, but dispatch-arbiter.test.ts was not modified to verify max acceptance.

Strengths

  • Completeness verified: All issue 338 acceptance criteria and plan acceptance criteria confirmed met, including maintainer feedback about extensibility.
  • Correct dual-layer safety model: resolveReasoningEffort (AI layer) and resolveEffectiveThinkingLevel (coding-agent layer) provide defense-in-depth for provider leakage prevention.
  • Provider paths thoroughly correct: Each provider (Anthropic, Bedrock, Google, OpenAI, Azure, Codex) handles max appropriately for its native capabilities.
  • OpenAI SDK upgrade clean: v6.49.0 confirms ReasoningEffort type includes "max", and no breaking stream handling changes in the 6.26→6.49 range.
  • Dashboard model-switch atomicity: The setRuntimeModel store update now applies model, thinking level, and available levels in one mutation, preventing UI inconsistency.
  • Safe cycling degradation: If current is not in levels, indexOf returns -1, yielding (0) % length = 0 — wraps to "off", which is safe.

Agents run: code-reviewer, error-auditor, test-reviewer, completeness-checker, simplifier


Reviewed by mach6

@maxscheurer

Copy link
Copy Markdown
Contributor Author

Review Assessment

#481 (comment)

Classifications

Finding Classification Reasoning
1. Custom themes silently replaced by dark theme on startup merge blocker Factual: Confirmed. thinkingMax added to TypeBox schema's required colors; custom themes without it fail validateThemeJson, throw from parseThemeJson, and initTheme() catches with catch (_error) discarding the diagnostic. Scope: Plan deliverable 4 lists "updating built-in themes, schema validation, and theme docs together" and testing approach mentions "exhaustive theme schema coverage." This is the first new required token added post-initial-release — the first time existing valid custom themes would break. Practical: Actor: user with custom ~/.dreb/agent/themes/<name>.json. Trigger: upgrade to this version. Consequence: custom theme silently replaced by dark with zero diagnostic output. The helpful error message ("Missing required color tokens: thinkingMax") is caught and discarded at line 699. User has no path to discover the issue without reading source. Fix: one-line stderr write before fallback.
2. Dashboard screens.test.tsx stale mock false positive Factual: Mock returns old shape, but setModel is defined at line 219 and never called in any test — it's an inert stub. Scope: N/A. Practical: No test exercises this path; no runtime impact.
3. Qwen max→xhigh effort mapping not tested deferred Factual: QWEN38_PLUS_EFFORT_MAP has max: "xhigh" but test's it.each table stops at ["xhigh", "xhigh"]. Scope: Not an explicit acceptance criterion. Practical: Trivial record lookup; adjacent entries well-tested. Low regression risk.
4. Dashboard setModel endpoint response not tested server-side deferred Factual: True — no server-side test. Scope: Not required. Practical: TypeScript types enforce shape at compile time; client types match.
5. supportsMax uses redundant if-return-true nitpick Factual: .test() already returns boolean. Scope: Style only. Practical: Zero runtime impact.
6. Unrelated blank-line removals and reformatting nitpick Factual: ~20 lines of formatting-only diff noise. Scope: Not material. Practical: Harmless.
7. resolveReasoningEffort not tested for undefined/xhigh deferred Factual: True; xhigh passthrough is tested via codex stream test ["gpt-5.6-sol", "xhigh", "xhigh"] but not via the unit test. Scope: Not explicit. Practical: 3-line function; regressions caught by integration tests.
8. Dashboard model-switch non-atomic two-step deferred Factual: setModel() then getState() — second failure leaves client stale. Scope: Pre-existing pattern. Practical: Self-correcting via SSE sync within milliseconds.
9. Theme schema thinkingMax — no validation test deferred Factual: No theme validation tests exist. Scope: Pre-existing gap. Practical: Runtime validates themes; this is defense-in-depth only.
10. Dispatch arbiter THINKING_LEVELS not tested for max false positive Factual: Set expanded but test doesn't exercise "max" specifically. Scope: N/A. Practical: The set is a simple allowlist; "max" behaves identically to any other valid level in routing logic. No distinct behavior to test.

Action Plan

1. Finding 1 — Emit warning when custom theme validation fails at startup (priority: high, fix: trivial)

In packages/coding-agent/src/modes/interactive/theme/theme.ts line 699, change:

} catch (_error) {
    // Theme is invalid - fall back to dark theme silently
    currentThemeName = "dark";
    setGlobalTheme(loadTheme("dark"));
}

to:

} catch (error) {
    // Theme is invalid - warn user and fall back to dark theme
    if (error instanceof Error) {
        process.stderr.write(`\n${error.message}\nFalling back to dark theme.\n\n`);
    }
    currentThemeName = "dark";
    setGlobalTheme(loadTheme("dark"));
}

This gives users with custom themes a clear diagnostic when upgrading, showing the "Missing required color tokens: thinkingMax" message with instructions to add the color.


Assessment by mach6

@maxscheurer

Copy link
Copy Markdown
Contributor Author

Progress Update

Fixed review finding 1 (merge blocker): custom themes now emit a stderr warning when validation fails at startup instead of silently falling back to dark theme.

Change (1 file)

  • packages/coding-agent/src/modes/interactive/theme/theme.tsinitTheme() catch block changed from catch (_error) (silently discarded) to catch (error) with process.stderr.write() that surfaces the validation error message before falling back to dark. Users with custom themes missing the new thinkingMax token will now see a clear diagnostic with instructions to add the missing color.

All 5804 tests pass, build clean.

Commit: 5613482


Progress tracked by mach6

…pt-56-max-effort

# Conflicts:
#	packages/coding-agent/test/agent-session-model-switch-thinking.test.ts
#	packages/dashboard/src/client/api.ts
#	packages/dashboard/src/client/screens/session.tsx
#	packages/dashboard/src/client/state/store.ts
#	packages/dashboard/src/server/server.ts
#	packages/dashboard/test/client/store.test.ts
@m-aebrer

Copy link
Copy Markdown
Collaborator

Unverified Review Candidates — Pending Assessment

Review round: 3
Reviewed commit: 5009b48

These are unverified candidates. Severity reflects reviewer confidence; do not treat any item as a merge blocker until the assessment comment is posted.

Critical

No critical findings.

Important

1. Prior custom-theme warning fix has no regression test (confidence: 98)

  • packages/coding-agent/src/modes/interactive/theme/theme.ts now warns on invalid custom-theme fallback, fixing the prior blocker, but no test exercises a custom theme missing thinkingMax or asserts the stderr diagnostic.
  • A regression to the former silent catch would again replace existing custom themes with dark without explaining why.

2. Native max is not tested on direct OpenAI or Azure Responses requests (confidence: 94)

  • The issue explicitly requires direct OpenAI requests to emit reasoning.effort: "max", and the PR also changes Azure Responses. Current payload coverage asserts native max only on the Codex/Responses Lite path; resolver unit tests do not exercise these request builders.
  • A builder or wrapper regression could clamp or omit max for direct API users while Codex tests continue passing.

3. Agent-loop effective max reporting is untested (confidence: 92)

  • packages/agent/src/agent-loop.ts adds model-aware max → xhigh → high reporting, but packages/agent/test/agent-loop.test.ts does not cover it; coding-agent tests exercise a separate resolver.
  • Divergence could make agent_start.thinkingLevel disagree with the provider's actual effective request level in session, UI, and RPC consumers.

4. Persisted max restoration is not covered (confidence: 89)

  • Session model-switch tests cover live clamping, but no test restores a persisted thinking_level_change of max on either a max-capable or xhigh-only model.
  • Resume is a normal path for users selecting max; regressions could silently restore the wrong effective tier.

Suggestions

5. Merge resolution added redundant casts with stale SDK-version comments (confidence: 92)

  • packages/ai/src/providers/openai-responses.ts and azure-openai-responses.ts say SDK 6.26 predates max and cast away the effort type, but this PR pins openai@6.49.0, whose generated type includes max.
  • These lines compiled without casts at the prior reviewed commit. The merge-time casts appear to accommodate stale local node_modules/openai@6.26.0, weaken type checking, and leave factually incorrect comments in durable code.

Strengths

  • The prior custom-theme blocker is factually fixed: validation errors now reach stderr before dark-theme fallback.
  • Full acceptance scope is implemented across normalized types, capability detection, provider clamping, CLI/settings/RPC/subagents, selectors, persistence, Telegram, and docs.
  • Dashboard merge-conflict resolution keeps model, thinking level, available levels, and settings revision synchronized against stale snapshots.
  • Provider leakage is guarded independently in the AI and coding-agent layers while preserving existing xhigh behavior.
  • supportsMax() is intentionally structured for additional future model-family clauses, directly addressing maintainer feedback.

Agents run: code-reviewer, error-auditor, test-reviewer, completeness-checker, simplifier


Reviewed by mach6

@m-aebrer

Copy link
Copy Markdown
Collaborator

Review Assessment

#481 (comment)

Classifications

Finding Classification Reasoning
1. Prior custom-theme warning fix has no regression test useful follow-up Factual: Confirmed; the warning fix is present and no test reproduces the former silent fallback. Scope: Theme validation coverage was named in the plan, and this protects the prior blocker fix. Practical: Current users receive the diagnostic correctly. A future maintainer would have to remove the exact stderr write to recreate the cosmetic, recoverable failure, and review remains a safeguard. Worth locking down, but missing a test does not make the correct implementation unsafe to ship.
2. Native max is not tested on direct OpenAI or Azure Responses requests useful follow-up Factual: Confirmed; resolver and Codex payload tests exist, while direct OpenAI/Azure builders lack a max payload assertion. Scope: Direct OpenAI wire emission is an explicit acceptance criterion, and the plan named both request tests. Practical: The tested resolver feeds a verbatim builder path already exercised with xhigh; current code inspection confirms max is emitted. A regression would require changing the passthrough despite these safeguards. Direct evidence would be useful, but there is no current behavioral defect.
3. Agent-loop effective max reporting is untested useful follow-up Factual: Confirmed; this separate hand-written fallback branch has no max test. Scope: The plan explicitly named effective agent-loop reporting tests. Practical: Actor: a future maintainer changes only this duplicated branch; trigger: a max request on a less-capable model; result: agent_start.thinkingLevel may display the wrong tier. The independent provider clamp still sends the correct safe effort, limiting harm to reporting/UI inconsistency. Useful regression coverage, not a blocker.
4. Persisted max restoration is not covered deferred Factual: Confirmed. Scope: Persistence/restoration is in scope. Practical: Restoration delegates to the same setThinkingLevel path already tested for live max preservation and clamping, while persistence plumbing is level-agnostic. A max-unique regression requires unsupported special-casing that does not exist. Additional composition coverage is low value and not required for safe shipping.
5. Merge resolution added redundant casts with stale SDK-version comments useful follow-up Factual: Confirmed; the PR pins OpenAI SDK 6.49, whose types include max, while merge-only comments cite stale 6.26 and casts accommodate the stale local install. Scope: This conflicts with the plan's instruction not to cast around stale SDK types. Practical: Independent assessor favored fixing before merge, but the developer's advocate found no material runtime or CI impact: actor is a future maintainer, trigger is trusting the inaccurate comment, harm is maintenance confusion and weakened future type checking; current payloads and clean installs behave identically, and CI installs 6.49. Removing the casts materially cleans durable code but does not change supported-user outcomes, so round-3 merge-blocker consensus is absent.

Action Plan

No merge blockers.


Assessment by mach6

@m-aebrer

Copy link
Copy Markdown
Collaborator

Progress Update

Fixed review finding 5 by removing the redundant OpenAI/Azure Responses effort casts and the stale SDK 6.26 comments. The providers now rely directly on the pinned OpenAI SDK 6.49 types, which include max.

Verification passed: Biome, full build, workspace-link verification, and all tests (5,879 passed; 717 skipped).

Commit: 1eaec4e


Progress tracked by mach6

@m-aebrer
m-aebrer merged commit b29f468 into aebrer:master Aug 21, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support GPT-5.6 max/ultra reasoning modes

2 participants