Repository navigation
feat(agent): per-conversation reasoning effort (picker row, /effort, session.set_effort) - #1113
Handsome-wzw wants to merge 9 commits into
Conversation
…session.set_effort) One conversation can now ask for more or less thinking than the configured default without moving that default. The pick rides the session's metadata beside `model` (`raven.agent.session_effort`), so it outlives the gateway and a resumed conversation runs at what it was left at; the turn entrance hands it to `_run_agent_loop`, which ranks it over the mode's effort and the configured `agents.defaults.reasoningEffort`, under a hook's rollback override. Raven's own calls only: sub-agents keep the configured default. Surfaces: `session.set_effort` (report, set or clear; every answer names what the default is on this install and the words), `session.info` carries `reasoning_effort` for the status bar, the WebUI's model picker gains an effort row under the tier row, and the TUI gains `/effort`. The settings page drew an unset `agents.defaults.reasoningEffort` as Low; the unset state is now its own rung, Provider default, and the key accepts null so that rung clears it. The using-raven guide documents both levels. Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
|
Verification on the head commit (local, this Mac):
|
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: the Web UI loads the previous conversation's reasoning effort when switching sessions.
I found one blocking issue, marked inline, and no other actionable findings. I covered the repository rules and canonical terms, the full diff, affected callers and the single-commit history, RPC/backward compatibility, the runtime/Web/TUI architecture boundaries, and the test changes; I found no weakened tests.
Verification after initializing this worktree from its lockfiles (uv sync --extra dev, npm ci in both frontends):
uv run pytest tests/test_session_effort.py tests/test_agent_loop_usage_sink.py tests/test_rpc_session.py tests/test_rpc_session_init_bundle.py tests/test_rpc_settings.py -q: 416 passeduv run pytest tests/test_rpc_schema_match.py -q: 517 passed- Web focused Vitest: 140 passed;
npm run type-check: passed - TUI focused Vitest through
npm test -- ...: 11 passed;npm run type-check: passed git diff --check github/main...HEAD: passed
The first attempts before dependency initialization failed during setup (raven_everos, vitest, and esbuild were absent); the commands above are the completed reruns.
|
Windows compatibility review — PR #1113 @ Scope: base Result: no Windows-specific issue found. What I checked:
Verification (native Windows 11, CPython 3.12.13, isolated worktree The one Python failure is inherited, not this PR: Limitations: |
…g_effort # Conflicts: # CHANGELOG.md
… left A switch asks for the conversation being opened before the page's current session moves over to it, and the effort read built its request from "the current session": opening B from A asked for A's effort and painted it on B -- and a pick of that very rung on B was taken as already in force. The read now names the conversation and the switch it belongs to, the way the permission-mode read does, and the source drops an answer whose switch has been overtaken rather than painting a view the reader has left. Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
The prior session-switch blocker is fixed in 516c7e7d4284; I replied on and resolved that thread. I reviewed the focused fix commit separately from the merged main history, its callers and regression tests, the Web UI source/store/session architecture boundary, backward compatibility, repository rules and canonical terms, and the test changes; no tests were weakened and I found no new actionable issue.
Verification:
npx vitest run src/features/model/source.test.ts src/state/effort.test.ts src/state/session/registry.test.ts src/features/model/ModelPicker.test.tsx: 159 passednpm run type-check: passednpm run lint: 0 errors, 4 pre-existing warnings in untouched filesgit diff --check github/main...HEAD: passed
|
On the blocking thread at
The tier row next door reads the same way ( |
|
Windows compatibility re-review — PR #1113 @ Scope: the delta from the previously reviewed head Result: no Windows-specific issue. What changed: the effort read is now keyed to the conversation being opened and the session-switch generation ( Verification (native Windows 11, isolated worktree The one Python failure fails identically at base Limitations: TUI tests not run (its harness is unavailable in this checkout); no real-browser walkthrough. |
|
Not a blocker -- smaller things from the same pass, separate from the blocking thread. Each was read at this head; none was executed. Severity of this one finding, not a verdict on the pull request. Any blocking findings from this pass are review threads on the changed files; GitHub renders those collapsed, as a file name with no text.
|
|
Merge conflict: @Handsome-wzw this PR no longer merges cleanly into Measured just now with After a rebase onto current main I re-check what moved before the grade comes back. |
…g_effort # Conflicts: # CHANGELOG.md
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: clearing Provider default still sends the gateway boot-time effort.
This head is a merge of current main; its only conflict resolution is in CHANGELOG.md, and it does not change the open Provider-default blocker. In the merged code, a null live default still makes default_reasoning_effort return None, and gen_overrides still omits the key when that value is falsey, so the provider falls back to its frozen boot-time generation setting. The measured reproduction and details remain on the existing open thread; I found no new actionable issue in this revision.
I reviewed the head delta and merge resolution, the blocker path and its callers, relevant architecture boundaries, backward compatibility, repository rules and canonical terms, and test changes; I found no weakened tests.
Verification:
uv run pytest tests/test_agent_loop_usage_sink.py tests/test_rpc_session.py tests/test_rpc_settings.py -q: 399 passedgit merge-tree --write-tree HEAD github/main: cleangit diff --check github/main...HEAD: passed
|
Windows compatibility re-review — PR #1113 @ Scope: the head moved from the previously reviewed Result: no Windows-specific issue; the previous conclusion holds. Verification (native Windows 11, CPython 3.12.13, isolated worktree The single failure is the inherited POSIX Limitations: TUI tests not run (harness unavailable in this checkout); no real-browser walkthrough. |
…xt turn Picking Provider default clears agents.defaults.reasoningEffort, and session.info / session.set_effort reported the clear, but a gateway that booted while the key held a word kept calling at that word: the turn left reasoning_effort out of the call when the live reader answered None, and the provider filled the missing argument from its own copy of the key, frozen at construction (ResolvingProvider). The display and the wire disagreed in exactly the way the per-conversation effort set out to end. The turn now sends the effort in force on every call, None included: an explicit None reaches the provider as None (its own default, nothing on the wire) instead of the boot copy, so a clear lands on the next turn the way a word change already did. The exhaustion wrap-up and the watch-work judgement send it the same way. The ranking has one home, session_effort.in_force (pick > mode's word > configured default, None one answer with one meaning), which the turn runs at through session_reasoning_effort on the loop and the RPC reports from, the way session_tier is shared; the two used to spell it out apart and to read an absent word differently. Tests: a real provider booted at high with the live key cleared records None on the next turn (low as the control), the three cases that pinned "the key is left out" now pin "the key is sent as None", and in_force has its own unit tests. CONTEXT and the CHANGELOG entry say the rule. Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
|
On the blocking thread at
Tests, as the thread asked: Verification on this head: the 16 related test files ( The six non-blocking points from the same pass are not in this commit; I can take them in a follow-up on this branch or in a small PR, whichever the reviewer prefers. |
|
Merged On the merged tree: ui-web The head to review is still 5057b49 for the fix itself (the comment above); this commit only carries main. |
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
The Provider default blocker is fixed: the shared ranking now supplies the turn and RPC surfaces from one rule, and every model-call path forwards the resolved value, including explicit None. The real-provider regression covers the exact boot-high/live-null failure and its low control.
I reviewed the delta from the prior head and the resulting github/main...HEAD change, the provider sentinel and streaming callers, RPC reporting, hook/auxiliary calls, history and backward compatibility, the AGENTS.md and CONTEXT.md constraints, and whether tests were merely weakened. The latest-main merge leaves the feature implementation unchanged; its manual CHANGELOG resolution preserves both sides. No new plain error survived refutation.
Verification on the exact head: uv run pytest across the touched session-effort, loop, RPC, watch-work, usage-sink, provider-reasoning and LiteLLM streaming suites: 587 passed, with 26 dependency warnings. git diff --check passed.
|
Windows compatibility re-review — PR #1113 @ Scope: the head moved from the previously reviewed Result: no Windows-specific issue. The change is effort-selection and reporting logic — strings and Verification (native Windows 11, CPython 3.12.13, isolated worktree The single failure is the inherited POSIX Limitations: TUI tests not run; no live provider call (the explicit-None wire behaviour is covered by the provider-protocol/usage-sink tests). |
| if not isinstance(metadata, dict): | ||
| return None | ||
| stored = metadata.get(METADATA_KEY) | ||
| return stored if isinstance(stored, str) and stored in REASONING_EFFORTS else None |
There was a problem hiding this comment.
Blocker - derive the effort options from the conversation's model, not from one global word list. session.set_effort returns "options": list(REASONING_EFFORTS) (raven/rpc/methods/session.py:1653) and validates against the same tuple (:1663), and session_effort.of() drops any stored word outside it. All three are model-blind, so every conversation is offered minimal/low/medium/high whatever its model accepts. A Codex conversation can therefore store minimal; openai_codex_provider.chat forwards the word verbatim in body["reasoning"] with no mapping and no fallback, and no Codex model accepts it: every entry in this machine's ~/.codex/models_cache.json (fetched 2026-10-27 from the same chatgpt.com/backend-api/codex/models endpoint codex_catalog.py calls) lists low/medium/high/xhigh/max, none lists minimal. The rejection is measured, not inferred: on 2026-10-20 at main 2e390adff the Codex backend answered HTTP 400 "only low/medium/high/xhigh/max are supported" for minimal (session tui:20261018_143429_bfe12b). I did not re-run a Codex turn at this head; the wire path here is code-reading. The reverse gap is the same defect: xhigh, max and ultra are supported by every Codex model and cannot be stored, because of() and the setter reject them. On the Anthropic path _ADAPTIVE_EFFORTS maps minimal and low to the same wire value, so two of the five offered options send the same request.
The data is already in hand: codex_catalog.py::_visible_slugs keeps only slug from each entry and drops supported_reasoning_levels and default_reasoning_level, and the model registry already has a reasoning capability tag. Remedy direction: compute options from the conversation's current model (the Web row and the TUI draw the server's list, so no client change is needed), have the setter and of() accept the same list, and decide what a model switch does with a pick the new model does not accept.
There was a problem hiding this comment.
Blocking: per-conversation effort options must not allow a value that the conversation's model rejects.
I confirmed the concrete chain on 01f24551809d: session.set_effort advertises and accepts minimal from the global tuple, and a credential-free payload probe of OpenAICodexProvider.chat(..., reasoning_effort="minimal") captured {"effort": "minimal"} unchanged on the Codex wire. That corroborates the reported HTTP 400 rather than refuting it.
The focused RPC/session/Codex suite still passes (17 passed), but those tests assert the global list and do not cover model-specific acceptance. This new evidence withdraws my clean stance; I am leaving resolution to the reviewer who opened the thread.
| on_token_delta=on_token_delta, | ||
| on_reasoning_delta=on_reasoning_delta, | ||
| reasoning_effort=policy.reasoning_effort, | ||
| reasoning_effort=turn_effort, |
There was a problem hiding this comment.
Nonblocking - pin the two side calls to the live effort; the code is right, but no test fails if it regresses. The main call now forwards turn_effort explicitly, and the wrap-up (_synthesize_final_on_exhaustion, called at :2151) and the watch-work judgement (_note_watch_work, called at :1572) share the shape by construction. Nothing pins it: in an isolated copy of this head I reverted the watch call site to policy.reasoning_effort (M1), the exhaustion call site the same way (M2), and the wrap-up helper at :648 to omit the key on None (M5); all 980 tests across the 25 related files stayed green. The controls behave: reverting the main call at :1230 fails 4 tests (including the new real-provider regression) and the same change in organ_glue.py:615 fails 1. So the two paths this PR's earlier blocker was about are correct today and unpinned tomorrow. Remedy direction: one assertion per side call that the provider received the session's live value, including the cleared case.
There was a problem hiding this comment.
Pinned at 1a0e9c2 (tests/test_agent_loop_session_effort.py): each side call is driven to a real provider booted at high under the configured default where the policy names none, the session's own pick, and a cleared key that must arrive as an explicit None. Your three mutants, re-applied at this head: M1 (:1572) fails the watch-work test's configured-default and own-pick cases; M2 (:2151) fails the wrap-up test's same two; M5 (:648) fails the wrap-up test's cleared case, where the provider records high. Details in the top-level comment.
|
Blocking: 1 finding(s) need correction before this revision can merge; see the inline notes. Scope: I reviewed only the This round covers the delta from Confirmed findings:
Verification:
Checked and deliberately not reported: the undocumented |
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: per-conversation effort options must not allow a value that the conversation's model rejects.
The new model-capability evidence changes my stance on this unchanged revision. I confirmed that the RPC offers and stores minimal without consulting the conversation's model and that the Codex provider forwards it verbatim, matching the reported backend rejection. The existing 17 focused tests pass but do not exercise model-specific effort acceptance.
This is the outstanding blocker from the open thread at raven/agent/session_effort.py; I added measured corroboration there and did not resolve a thread opened by another reviewer. The prior Provider-default fixes remain verified.
…ords Inside a direct chat with a sub-agent instance, /effort sent the main conversation's session_key and then said "this conversation" in the instance's pane: the main record moved while the reader was told the instance's had. The pick is the main conversation's and an instance has none of its own to set from here, so the command is refused there, naming the instance and the way back, the way /instance does. The reset words are /mode's RESET_WORDS now rather than a second copy of the same three. Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
…omotion's ticket A draft's effort pick was staged before the session.set_effort read that answers the default and the words, and left staged when that read failed: the toast said the pick was not made and the old rung was repainted, but the first send's applyStagedEffort ran the turn at it. The pick is still staged first, so a send during the round trip carries it, and a failed read puts back the last pick whose read landed -- not merely the value staged before the failing call, which is another in-flight pick when two went out and both failed. The re-read after the promotion wrote the staged effort took the view's ticket after the awaited write, so a reader who opened another conversation while the write was in flight got the minted session's rung painted over that conversation's own read. The promotion's ticket now rides into applyStagedEffort the way it rides into applyStagedModel, and the source drops the late answer. The tier row's tooltip (gui.tier.scope) said reasoning depth follows the tier, one row above a row titled with the same words; the shipped tiers leave the effort alone, so the tooltip now names sub-agents only. The TUI catalogue is regenerated from it. Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
…arry the pick on a fork The exhaustion wrap-up and the watch-work judgement forward the effort in force, explicit None included, but nothing failed when either call site went back to the policy's word or the wrap-up helper left the key out on None. Two tests now drive each side call to a provider booted at high: the configured default where the policy names none, the session's own pick, and a cleared key that must arrive as None rather than be filled from the boot copy. The watch-work case is pinned at the provider, through chat_stream, with the spawn roster naming an on-call agent so the look earns the judgement. The agent's raven_config door kept its own copy of the effort words and refused null for agents.defaults.reasoningEffort while the settings page accepted it. The setting reads REASONING_EFFORTS and is nullable, so the words have one owner and the agent can clear the key the way the page does. A fork carried the parent's model but dropped its effort pick, so a forked conversation ran at the default until resumed; FORK_CARRIES names what a fork carries, with the pick beside the model. session_effort.write's docstring promised a "pending" report that session.set_effort never makes; it now says what the answer is for. CONTEXT.md, the using-raven guide and the CHANGELOG entry carry the three user-visible facts. Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
|
The non-blocking items from the two passes are in, as three commits on this branch (TUI LivXue F1 -- the two side calls are pinned (
0xKT's six:
Not changed, as LivXue noted and left unfiled: with effort set only through the Verification at
A short reply sits in LivXue's F1 thread as well; the threads are left for their authors to close. #1122 and #1123 sit on this branch and will need this head merged in. |
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; suggestions only, and they are marked inline.
The new revision correctly addresses the accepted follow-ups: the wrap-up and watch-work tests now exercise the live configured value, session pick, and explicit-None clear through a real provider; raven_config can clear the global key; forks carry a non-null conversation pick; TUI Direct Chat refuses /effort instead of mutating the main session; and the Web draft/promotion races restore the last landed pick and retain the promotion ticket. I found no new plain error in those paths.
I covered the 01f24551809d..HEAD delta and github/main...HEAD relationship, AGENTS.md/CONTEXT.md constraints, the changed callers, persistence/restart and concurrency compatibility, history, and whether tests were weakened. Verification passed: 439 focused Python tests, 45 Web tests, 11 TUI tests, both UI type-checks, and git diff --check.
Named follow-up: the model-blind effort vocabulary remains a real compatibility gap (for example Codex rejects the offered minimal value), but the author has explicitly deferred the ownership/split to #1123. Per that scope decision I no longer hold it as a blocker on this PR; LivXue's thread remains open for its opener.
|
Windows compatibility re-review — PR #1113 @ Scope: the head moved from the previously reviewed Result: no Windows-specific issue. The additions are config-catalogue entries, session-metadata slots (plain strings in a dict), and browser/TUI state logic — no path, filesystem, subprocess, signal, encoding, or platform primitive. Verification (native Windows 11, CPython 3.12.13, isolated worktree Limitations: TUI tests not run; no live provider call. |
|
Merge conflict: @Handsome-wzw, this PR no longer merges onto the current
The judge's verdict on |
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: this head must be reconciled with current main because raven/agent/loop/turn_path.py conflicts.
I independently confirmed the new repository state: current github/main is 959cd0a9, the merge base is 984d47d4, and git merge-tree --write-tree HEAD github/main exits 1 with a content conflict in raven/agent/loop/turn_path.py. That makes my prior nonblocking stance stale even though the reviewed head itself has not changed.
The unchanged-head effort baseline still passes: 23 tests across test_agent_loop_session_effort.py and test_session_effort.py. Those results cannot validate the unresolved merged tree; the relevant tests need rerunning after the conflict is reconciled. The model-specific option vocabulary remains the named #1123 follow-up rather than an additional blocker from my side.
Summary
One conversation can now ask for more or less thinking than the configured default
without moving that default:
row: Default / Minimal / Low / Medium / High. The help mark says what Default means
on this install (the global setting, or the provider's own default when none is set).
/effort [minimal|low|medium|high|default];/effortalone reports,and the status bar follows the pick.
session.set_effort(report, set or clear, told apart by which fields arepresent, the shape
session.set_modehas). Every answer carriesdefaultandoptions, so one round trip draws the control.session.infocarriesreasoning_effortfor the status bar.raven/agent/session_effort.py, metadatakey
reasoning_effort, null once cleared), so it outlives a gateway restart andcomes back on resume. The turn entrance reads it beside the model and hands it to
_run_agent_loop, which ranks it: configured default < the mode's effort < thispick < a hook's rollback override. Raven's own calls only; sub-agents keep the
configured default. Lands on the next turn.
REASONING_EFFORTSinconfig/schema.py, shared with the console'schecker for
agents.defaults.reasoningEffort.Also fixed: the settings page drew an unset
agents.defaults.reasoningEffortasLow while the provider's own default was what ran. The unset state is now its own
rung, Provider default, and the key accepts null so that rung clears it.
Docs: a "Set the reasoning effort" section in the using-raven guide (en + zh), a row
in the command reference's task table, CHANGELOG (Added + Fixed), CONTEXT.md gains
Session Reasoning Effort.
Review follow-ups folded in: a fork keeps its parent's pick with its model; the
agent's
raven_configdoor readsREASONING_EFFORTSand accepts null, as thesettings page does;
/effortis refused inside a TUI direct chat (the pick is themain conversation's); a draft pick whose read fails is put back; the re-read after a
promotion carries the promotion's ticket; the tier row's tooltip names sub-agents
only; and the exhaustion wrap-up and watch-work calls are pinned to the effort in
force by tests that kill the three surviving mutants.
Out of scope, deliberately: the
xhigh/maxrungs some providers accept (a separatesmall PR), and the sub-agent inheritance of the pick.
Type
Verification
pytest tests/test_session_effort.py tests/test_rpc_session.py tests/test_agent_loop_usage_sink.py tests/test_rpc_settings.py tests/test_rpc_schema_match.py tests/test_rpc_registration.py-- passfull
pytest tests-- see the first comment for the count and the baseline failuresscripts/coverage_gate.py diff --base-ref origin/main-- see the first commentruff check/ruff format --check/ty checkon the touched modules /lint-imports-- cleanui-web:
npm run gen(contract regenerated),vitest run(261 files, all gates),type-check,lint-- cleanui-tui:
npm run gen:rpc,npm run gen:i18n,vitest run(146 files),type-check,lint-- cleanmkdocs build --strict-- cleanThe picker row was checked on the built page's offline library (
?stub=1) in light, dark and zhRelevant tests pass locally
Relevant lint / type checks pass locally
User-facing docs or screenshots are updated when needed
Risk
session.infogains an optional fieldRelated Issues
N/A