fix(thinking): clamp below-range requests down to the cheapest level,… by obuchowski · Pull Request #93335 · openclaw/openclaw

obuchowski · 2026-06-15T15:14:29Z

TL;DR For session sitting at thinking off no-off ("reasoning only") models get silently bumped to the most expensive reasoning level — and the wrong value is persisted into the session.

Summary

What's wrong? resolveSupportedThinkingLevelFromProfile (src/auto-reply/thinking.ts) clamps a requested thinking level to the closest supported one. It sorts the supported levels descending by rank, then:

const ranked = profile.levels.toSorted((a, b) => b.rank - a.rank); // descending
return (
  ranked.find((entry) => entry.id !== "off" && entry.rank <= requestedRank)?.id ??
  ranked.find((entry) => entry.id !== "off")?.id ??   // ← bug: first of a descending list = most expensive
  "off"
);

The second fallback only fires when every supported level exceeds the request — exactly the case of a stored off on a profile that has no off level. ranked.find(non-off) returns the first entry of a descending list, i.e. the highest level. So off → high (max spend) instead of off → low.

Who hits it? src/auto-reply/reply/directive-handling.persist.ts: when a session's current thinking level is not supported by a newly-selected model, it calls resolveSupportedThinkingLevel(...) and writes the result back to the session (sessionEntry.thinkingLevel = remappedThinkingLevel). So a user who keeps thinking off and switches their session onto a no-off model is silently remapped to maximum reasoning, persisted, on every later turn — never having asked for it.

The fix. One word: ranked.find → ranked.findLast on the second fallback. On a descending list, findLast(non-off) is the lowest non-off level. A below-range request now clamps down to the cheapest reasoning level, never up. Mid-range requests are unaffected (they resolve via the first .find branch). Regression test included.

-    ranked.find((entry) => entry.id !== "off")?.id ??
+    ranked.findLast((entry) => entry.id !== "off")?.id ??

Real behavior proof

Behavior or issue addressed: a session at thinking off switched onto a model whose profile has no off level was remapped (and persisted) to the most expensive reasoning level instead of the cheapest.
Real environment tested: OpenClaw checkout of this branch on Linux/Node 24 (Oracle Linux aarch64); the change is pure core clamp logic, exercised by a deterministic unit test that mocks a no-off provider profile (the exact shape directive-handling.persist.ts feeds in).
Exact steps or command run after this patch: node scripts/run-vitest.mjs src/auto-reply/thinking.test.ts (green); to prove the regression, swap in main's pre-fix file and run the new case — git checkout origin/main -- src/auto-reply/thinking.ts && node scripts/run-vitest.mjs src/auto-reply/thinking.test.ts -t "clamps a below-range request down" (red), then git checkout HEAD -- src/auto-reply/thinking.ts to restore.
Evidence after fix: screenshot of the gree→red run below. With the old ranked.find the new test fails AssertionError: expected 'high' to be 'low'; with ranked.findLast the full suite is green (54 passed). The mocked profile is [low, medium, high] (no off) and the request is off, "reasoning-only" models.
Observed result after fix: resolveSupportedThinkingLevel({ provider, model, level: "off" }) against a no-off profile returns low (cheapest), where on main it returned high (most expensive). Mid-range clamps (adaptive→medium, xhigh→high) are unchanged.
What was not tested: a live end-to-end model switch needs a bundled no-off model in the catalog — fireworks/gpt-oss-120b

$ node scripts/run-vitest.mjs src/auto-reply/thinking.test.ts
[test] starting test/vitest/vitest.auto-reply.config.ts

 RUN  v4.1.7 /tmp/oc-clamp

 ✓  auto-reply  src/auto-reply/thinking.test.ts (54 tests) 98ms

 Test Files  1 passed (1)
      Tests  54 passed (54)
   Start at  15:12:38
   Duration  795ms (transform 218ms, setup 218ms, import 305ms, tests 98ms, environment 0ms)

[test] passed 1 Vitest shard in 8.66s
$ sed -i 's/ranked\.findLast(/ranked.find(/' src/auto-reply/thinking.ts
$ node scripts/run-vitest.mjs src/auto-reply/thinking.test.ts | grep -A100 "Failed Tests"
[test] starting test/vitest/vitest.auto-reply.config.ts

⎯⎯⎯⎯⎯⎯⎯ Failed Tests 1 ⎯⎯⎯⎯⎯⎯⎯

 FAIL   auto-reply  src/auto-reply/thinking.test.ts > listThinkingLevels > clamps a below-range request down to the cheapest level on a no-off profile
AssertionError: expected 'high' to be 'low' // Object.is equality

Expected: "low"
Received: "high"

 ❯ src/auto-reply/thinking.test.ts:664:7
    662|         level: "off",
    663|       }),
    664|     ).toBe("low");
       |       ^
    665|   });
    666| });

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯

[test] failed 1 Vitest shard in 8.74s

Tests and validation

src/auto-reply/thinking.test.ts: 54 passed, incl. the new clamps a below-range request down to the cheapest level on a no-off profile case — proven red on main (expected 'high' to be 'low') and green with the fix.
tsgo core + core-test lanes: clean. oxfmt: clean.
No existing test pinned the old escalate-up behavior, so nothing was rewritten to accommodate the change.

Risk checklist

Strictly cost-reducing: the only behavior change is that a below-range request resolves to the cheapest supported level instead of the most expensive. It cannot raise spend or block a model.
Single fallback branch touched; the first .find (closest level at or below the request) and the final "off" default are unchanged. Off-only, binary, and full profiles are unaffected (they either contain the requested level or have a level at/below it).
No config, schema, persistence, or protocol surface changes. Plugin-agnostic core fix; benefits every provider with a no-off profile, not just Fireworks.
Net +21/−1 across 2 files (1 prod line + a regression test).

Current review state

What is the next action? Maintainer review.

What is still waiting on author, maintainer, CI, or external proof?
Maintainer: review/land + ordering vs the sibling PR.
CI: standard checks. No external proof outstanding.

Which bot or reviewer comments were addressed? None posted yet.

Review state guidance

Keep this as the durable state for review progress. If useful information appears in comments, fold the current next action or blocker back here so maintainers and ClawSweeper do not need to reconstruct state from comment history.

clawsweeper · 2026-06-15T15:16:17Z

Codex review: needs real behavior proof before merge. Reviewed July 7, 2026, 1:50 AM ET / 05:50 UTC.

Summary
The PR changes resolveSupportedThinkingLevelFromProfile to choose the lowest supported non-off level for below-range requests and adds a regression test for a no-off profile.

PR surface: Source 0, Tests +14. Total +14 across 2 files.

Reproducibility: yes. by source inspection. Current main sorts profile levels descending and falls back to the first non-off entry, so a stored off remapped against a no-off low/medium/high profile resolves to high.

Review metrics: 1 noteworthy metric.

Shared fallback callers inspected: 5 caller paths. Directive persistence, gateway patching, reply execution, agent-command execution, and cron execution all use the shared remap helper, so the one-line policy change is centralized.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

[P1] Add redacted real CLI/provider/session proof for a no-off thinking profile or obtain a maintainer proof override.
Update the PR body after adding proof so ClawSweeper can re-review automatically; if it does not, ask a maintainer to comment @clawsweeper re-review.

Proof guidance:

[P1] Needs real behavior proof before merge: The PR body and screenshot show deterministic unit-test output against a mocked provider profile, not a real OpenClaw no-off session/model path; the contributor should add redacted CLI/provider/session proof, or a maintainer should apply a proof override before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

[P1] External-PR proof remains mock-only; merge should wait for redacted real CLI/provider/session proof of a no-off remap or an explicit maintainer proof override.
[P1] The companion Fireworks catalog PR exposes the first bundled no-off profile, so that PR should not ship the no-off profile before this helper fix or equivalent mitigation.

Maintainer options:

Decide the mitigation before merge
Land this shared helper fix after real no-off session/provider proof or a maintainer proof override, and coordinate it before or together with the companion Fireworks no-off catalog work.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

[P1] No automated repair is needed; the remaining blocker is real behavior proof or a maintainer proof override plus normal maintainer review.

Maintainer decision needed

Question: Should maintainers accept a proof override for this narrow core helper change, or require redacted real no-off session/provider proof before merge?
Rationale: The source review supports the patch, but the external-PR proof gate is still mock-only unless a maintainer intentionally overrides it.
Likely owner: steipete — He is assigned to this PR and authored the central provider-thinking/model-switch behavior this helper supports.
Options:
- Require real proof (recommended): Wait for redacted CLI/provider/session proof showing a real no-off profile remaps off to the cheapest supported level before merge.
- Override proof: A maintainer may explicitly accept the unit-test proof for this narrow shared helper change and clear the proof gate.

Security
Cleared: No security or supply-chain concern found; the diff changes one TypeScript fallback expression and a colocated unit test only.

Review details

Best possible solution:

Land this shared helper fix after real no-off session/provider proof or a maintainer proof override, and coordinate it before or together with the companion Fireworks no-off catalog work.

Do we have a high-confidence way to reproduce the issue?

Yes by source inspection. Current main sorts profile levels descending and falls back to the first non-off entry, so a stored off remapped against a no-off low/medium/high profile resolves to high.

Is this the best way to solve the issue?

Yes for the code fix. The shared helper is the narrowest maintainable fix point because durable model switching, gateway patching, reply execution, agent command execution, and cron execution already delegate unsupported thinking-level remaps there.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 094c0d421faf.

Label changes

Label justifications:

P2: The PR addresses a source-reproducible thinking-level remap bug with a narrow shared core helper change and limited blast radius.
rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body and screenshot show deterministic unit-test output against a mocked provider profile, not a real OpenClaw no-off session/model path; the contributor should add redacted CLI/provider/session proof, or a maintainer should apply a proof override before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence reviewed

PR surface:

Source 0, Tests +14. Total +14 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	1	1	0
Tests	1	14	0	+14
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	15	1	+14

What I checked:

Current main still has the old fallback: Current main sorts profile levels descending, then falls back to the first non-off entry; for a no-off low/medium/high profile and requested off, that returns high. (src/auto-reply/thinking.ts:352, 094c0d421faf)
PR changes only the fallback target: The PR changes the second fallback from find to findLast, preserving exact matches and the strongest-at-or-below-rank branch while choosing the lowest non-off level for all-below-range requests. (src/auto-reply/thinking.ts:350, e2d9002cac6a)
Regression test covers the reported no-off case: The added test mocks a provider profile with low, medium, and high only, requests off, and expects low. (src/auto-reply/thinking.test.ts:648, e2d9002cac6a)
Durable model switch persists helper output: When a stored thinking level becomes unsupported after model selection, directive persistence remaps through resolveSupportedThinkingLevel and writes the result back to sessionEntry.thinkingLevel. (src/auto-reply/reply/directive-handling.persist.ts:334, 094c0d421faf)
Gateway patch path uses the same helper: Gateway session patch normalization also remaps inherited unsupported thinking levels through resolveSupportedThinkingLevel when the patch did not explicitly set thinkingLevel. (src/gateway/sessions-patch.ts:655, 094c0d421faf)
Turn execution paths share the fallback policy: Reply execution, agent command execution, and isolated cron agent execution all delegate unsupported implicit thinking levels to the same helper, so the shared helper is the right owner boundary. (src/auto-reply/reply/get-reply-run.ts:1042, 094c0d421faf)

Likely related people:

steipete: Peter Steinberger authored the central provider-thinking profile work, max-thinking support gate, and model-switch remap behavior that define this helper’s current contract. (role: feature owner; confidence: high; commits: f1805ab54d63, bcfa781a1b37, 6ce17db11aa1; files: src/auto-reply/thinking.ts, src/auto-reply/reply/directive-handling.persist.ts, src/plugins/provider-thinking.types.ts)
vincentkoc: Current blame in this shallow checkout points at Vincent Koc’s recent main snapshot, and he merged the adjacent anthropic-messages thinking-profile PR. (role: recent area contributor and merger; confidence: medium; commits: 0fd69dc3d2b8, 43b4e2769928; files: src/auto-reply/thinking.ts, src/auto-reply/thinking.test.ts)
wangwllu: Lu Wang authored the recent anthropic-messages thinking-profile fallback work in the same shared helper and test surface. (role: recent adjacent thinking-profile contributor; confidence: medium; commits: 43b4e2769928; files: src/auto-reply/thinking.ts, src/auto-reply/thinking.test.ts)
jalehman: Josh Lehman recently routed auto-reply sessions through the session seam, touching the directive persistence area that durably writes remapped thinking levels. (role: recent session persistence contributor; confidence: medium; commits: 127e174c9e4d; files: src/auto-reply/reply/directive-handling.persist.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Review history (2 earlier review cycles)

reviewed 2026-06-22T21:19:55.188Z sha 95773d1 :: needs real behavior proof before merge. :: none
reviewed 2026-07-01T03:15:51.933Z sha 95773d1 :: needs real behavior proof before merge. :: none

… not up resolveSupportedThinkingLevelFromProfile sorts the supported levels descending, then, when every supported level exceeds the request (e.g. a stored `off` on a profile that has no off level), fell through to `ranked.find(non-off)` — the first entry of a descending list, i.e. the most expensive level. A session carrying thinking `off` that switched onto such a model was silently remapped to maximum reasoning and persisted (src/auto-reply/reply/directive-handling.persist.ts). Use `ranked.findLast(non-off)` so a below-range request resolves to the cheapest non-off level instead. Regression test included (red before, green after). This surfaces with the first bundled no-off thinking profiles (Fireworks GPT-OSS 120B, see openclaw#92217), but the clamp lives in shared core logic, so it ships on its own.

steipete · 2026-07-07T05:55:16Z

Land-ready proof for exact head e2d9002cac6a89c63e224916420527e82f1389cf:

Best-fix review: the shared ranked resolver remains the single owner. In-range requests still choose the strongest supported level at or below the request; only the exhausted below-range fallback now chooses the cheapest supported non-off level instead of the most expensive one.
Caller proof: the model-switch persistence path uses this resolver and stores the remapped level, so subsequent turns cannot retain the invalid higher effort.
Focused Linux proof: sanitized AWS Crabbox lease cbx_de7bb50bca88 (provider aws, public networking, no Tailscale) passed run run_cad4f538c880: src/auto-reply/thinking.test.ts (54) plus src/auto-reply/reply/directive-handling.model.test.ts (74), 128 tests total.
Dependency contract: current Codex source directly checked at cca16a10878202cb2f6e9666b6b4330329ea7e65. ReasoningEffort accepts known and future string levels; model presets and app-server Model expose supported efforts plus the default (protocol, preset, app-server model). OpenClaw consumes that advertised list into its provider profile.
Static/review proof: git diff --check; fresh exact-head autoreview clean, no actionable findings (0.87).
Hosted proof: CI run 28844388990 passed on the exact head (attempt 2). Attempt 1 had one unrelated gateway admission-race failure in server.chat.gateway-server-chat-b.test.ts:1477; rerunning only that shard passed, with every other job already green.
Docs/changelog: no public option changes; no docs or changelog change required for this focused correction.

Known gap: no credentialed external-provider model-switch probe. The deterministic shared resolver and durable persistence path are covered together by focused tests; the direct Codex protocol check confirms the dependency surface.

steipete · 2026-07-07T05:55:29Z

Merged via squash.

Prepared head SHA: e2d9002cac6a89c63e224916420527e82f1389cf
Landed commit: 72bd74e9ddbff6794b924fbcf9e5fb255937d573

… not up (openclaw#93335) resolveSupportedThinkingLevelFromProfile sorts the supported levels descending, then, when every supported level exceeds the request (e.g. a stored `off` on a profile that has no off level), fell through to `ranked.find(non-off)` — the first entry of a descending list, i.e. the most expensive level. A session carrying thinking `off` that switched onto such a model was silently remapped to maximum reasoning and persisted (src/auto-reply/reply/directive-handling.persist.ts). Use `ranked.findLast(non-off)` so a below-range request resolves to the cheapest non-off level instead. Regression test included (red before, green after). This surfaces with the first bundled no-off thinking profiles (Fireworks GPT-OSS 120B, see openclaw#92217), but the clamp lives in shared core logic, so it ships on its own.

openclaw-barnacle Bot added the size: XS label Jun 15, 2026

obuchowski mentioned this pull request Jun 15, 2026

feat(fireworks): catalog DeepSeek V4 Pro, MiniMax M2.7, GLM-5.1, GPT-OSS 120B reasoning models #92217

Open

obuchowski force-pushed the fix/thinking-clamp-below-range branch from 1af4e7a to 95773d1 Compare June 15, 2026 15:20

openclaw-barnacle Bot added the proof: supplied External PR includes structured after-fix real behavior proof. label Jun 15, 2026

steipete self-assigned this Jul 7, 2026

steipete force-pushed the fix/thinking-clamp-below-range branch from 95773d1 to e2d9002 Compare July 7, 2026 05:42

steipete merged commit 72bd74e into openclaw:main Jul 7, 2026
157 of 158 checks passed

github-actions Bot mentioned this pull request Jul 7, 2026

📡 Upstream Digest — 2026-07-07 07:31 UTC curtismercier/openclaw-mods#1228

Open

clawsweeper Bot mentioned this pull request Jul 7, 2026

fix(openrouter): bound music SSE buffering #101488

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(thinking): clamp below-range requests down to the cheapest level,…#93335

fix(thinking): clamp below-range requests down to the cheapest level,…#93335
steipete merged 1 commit into
openclaw:mainfrom
obuchowski:fix/thinking-clamp-below-range

obuchowski commented Jun 15, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 15, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

obuchowski commented Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Real behavior proof

Tests and validation

Risk checklist

Current review state

Uh oh!

clawsweeper Bot commented Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

obuchowski commented Jun 15, 2026 •

edited

Loading

clawsweeper Bot commented Jun 15, 2026 •

edited

Loading