Conversation
Greptile SummaryThe PR scopes reasoning effort to individual sessions across persistence, API, local-agent, gateway, copy, and UI paths.
Confidence Score: 4/5The PR is not yet safe to merge because a failed reasoning selection during session hydration can leave the active session displaying another session's effort indefinitely. The POST dispatch can supersede the active session's in-flight GET, while its failure path retains the optimistic fetch key and stale cached effort, preventing subsequent topbar synchronization from repairing the chip. Files Needing Attention: static/ui.js Important Files Changed
Sequence DiagramsequenceDiagram
participant U as User
participant UI as Reasoning chip
participant API as /api/reasoning
participant S as Session storage
participant R as Runtime resolver
U->>UI: Select effort in session
UI->>API: POST effort + session_id
API->>S: Persist Session.reasoning_effort
API-->>UI: Effective reasoning status
R->>S: Read session metadata
alt Session override is not None
S-->>R: Session effort
else Existing session or non-session caller
R->>R: Fall back to profile config
end
Reviews (2): Last reviewed commit: "fix(reasoning): validate ownership depen..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # Fields are listed in the order they should appear in the JSON file. | ||
| METADATA_FIELDS = [ | ||
| 'session_id', 'title', 'workspace', 'model', 'model_provider', 'model_explicit_pick_signature', 'created_at', 'updated_at', | ||
| 'session_id', 'title', 'workspace', 'model', 'model_provider', 'reasoning_effort', 'model_explicit_pick_signature', 'created_at', 'updated_at', |
There was a problem hiding this comment.
This change persists reasoning effort on sessions and changes both endpoint and runtime resolution semantics, but it does not document the session override, profile fallback, or empty-string reset state, leaving maintainers and API consumers without an authoritative description of the new contract.
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
SummaryReading all seven changed files at PR HEAD and on Code reference
const ctx={};
if(S&&S.session&&S.session.session_id) ctx.session_id=S.session.session_id;
if(model) ctx.model=model;
if(provider) ctx.provider=provider;At Diagnosis / recommendationOn a blank chat, selecting Low can therefore modify the global default. Opening another existing session that has no explicit override will now also run at Low. The UI looks session-scoped, but the request took the global branch. I would block the picker until a session exists, or materialize a session before posting the change. Another option is a pending new-chat value that is copied into the Test planThe current tests cover two persisted sessions but not the blank-chat branch. Please add a route/UI regression that posts a reasoning change without an active session and verifies the profile setter is not called, then create the session and verify only its |
|
Thanks @samfoy — this is a real and well-diagnosed data-isolation bug (the chip looking per-session while all four paths shared one global key), and the core of the fix is solid. I gated the current head Holding for a revision — the scoping fix is incomplete across three sibling paths that still route through the old global behavior. All three were reproduced with real probes: 1. SILENT — session-switch race poisons another session's chip (
|
nesquena-hermes
added
the
changes-requested
Setting the reasoning-effort chip in one WebUI session changed it in
every session. All read and write paths shared a single global key,
agent.reasoning_effort in config.yaml, so the per-session chip was a
global setting with a per-session appearance.
Add Session.reasoning_effort and prefer it wherever the effort is
resolved:
- api/models.py persist the field in the metadata prefix
- static/ui.js send session_id with the chip GET and POST
- api/routes.py GET reads the session value; POST writes it and
evicts the cached agent so the next turn rebuilds
- api/config.py get_reasoning_status() override parameter
- api/streaming.py local agent path prefers the session value
- api/gateway_chat.py gateway path prefers the session value
A session value of None keeps the previous behaviour and falls back to
profile config, so existing sessions, the CLI, and cron are unchanged.
An explicit empty string means "provider default" for that session
only, preserving the nesquena#6219 thinking-toggle re-enable path.
Both request paths are updated because the gateway path reads the same
key; fixing only the local path would leave gateway-routed WebUI chats
globally scoped.
f79da53 to
c0d76a8
Compare
|
Rebased onto One conflict, in
No human review yet on this one, so it's ready whenever someone has a slot. |
The reasoning-option click handler POSTs /api/reasoning with the active session in the payload, then applies the response to the chip. The request is asynchronous, so a POST dispatched from session A can resolve AFTER the user switches to session B. The late response then wrote A's effort onto B's chip and poisoned B's cached _currentReasoningEffort. That is the same session-confusion class the per-session reasoning_effort storage removes, reappearing on the async-completion path. Snapshot the dispatch key before the request and take a sequence number from the existing _reasoningFetchSeq counter. Apply the chip write, the cache write, and the toast only when BOTH still match at completion time. Both halves are load-bearing: the sequence number rejects a dispatch that a newer one superseded for the same session, and the key comparison rejects a response that lands after the session, model, or provider changed. A stale response is discarded silently. A toast naming an effort for a session the user already left is itself misinformation, so the guard covers the rejection path as well as the success path. The guard lives in one helper, _reasoningDispatchIsCurrent(), so the slash-command path can share it.
The composer chip POST carries _reasoningEffortContext() so its write lands
on the active session. The /reasoning <effort> slash command still POSTed a
bare {effort} and hit the profile-global default.
A session holding a persisted override therefore kept its old effort while
the command's toast claimed the new value applied. The UI reported a state
the session did not have.
Include _reasoningEffortContext() in the command's POST body, and apply the
same stale-context guard as the chip: snapshot the dispatch key, take a
sequence number from the shared _reasoningFetchSeq counter, and discard a
superseded response silently. Sharing one counter across both paths means a
chip pick and a slash command can supersede each other, which is what the
user sees as a single control.
With NO active session the behaviour is unchanged: _reasoningEffortContext()
adds session_id only when a session exists, so the command still writes the
profile-global default. That is correct, and it is what /reasoning does
before the first chat starts.
The guard reads through typeof checks because commands.js loads after ui.js
but is not guaranteed to see it in every embed path; a missing helper falls
back to applying the response, which is the pre-change behaviour.
reasoning_effort is a per-session override. Three constructors build a child session from a source session and none of them carried the field: - POST /api/session/duplicate (duplicate) - POST /api/session/branch (fork / branch) - POST /api/session/compression-recovery/start (focused continuation) Each child therefore read None and silently fell back to the profile-global default. A source session set to xhigh produced a copy running at whatever the profile said, with nothing in the UI to show the level had changed. Pass reasoning_effort=getattr(source, "reasoning_effort", None) in all three. getattr with a None default keeps a source that has no override producing a child with no override, which is what holds the profile-global fall-back for existing sessions, the CLI, and cron. The regression tests set the profile-global default to a value DIFFERENT from the source override, so a silent fall-back to the global fails the assertion. A test where the two values match would pass either way and prove nothing. Note for reviewers: the reasoning_effort=getattr(...) lines already present near the GET and POST /api/reasoning endpoints are a different fix. These are three separate constructor sites, and the static backstop test slices each constructor individually so it cannot be satisfied by the endpoint hunks.
|
The three blockers are closed, the PR head is My 26 August statement that no human review existed was wrong, because your 7 August review still had three open blockers. Thank you for the detailed review. I pushed three commits:
Blocker 1: session-switch race in
|
| Fix | Mutation | Result |
|---|---|---|
| Blocker 1 | Revert the dispatch guard | 3 dispatch-race tests failed |
| Blocker 2 | Drop the session context | The scope test failed |
| Blocker 2 | Neutralize the dispatch guard | 2 tests failed |
| Blocker 3 | Remove each constructor line independently | 2 inheritance tests failed per line |
All tests returned to green after restoration.
The blocker 3 behavioral test uses a persisted session. After mutation it reads reasoning_effort=None instead of 'xhigh', so source-text matching is not the only oracle.
The two JavaScript test modules extract production blocks from the shipped source. They run those blocks verbatim under Node against the real ui.js helpers.
Merge gate
ESLint did not run because node_modules is absent. I skipped npm run lint:runtime and scripts/scope_undef_gate.py, then ran node --check on both JavaScript files.
Please require the CI runtime-guard lint to pass before merge.
Pre-existing flaky test
At the unmodified baseline, test_recovery_start_reuses_existing_focused_session fails intermittently in 3 of 15 runs. These commits did not cause the failure.
_persisted_session_ids_snapshot() likely causes it, because the function caches the session-directory listing by st_mtime_ns. This is a likely cause, not a confirmed root cause.
When the child file lands within the same mtime tick, the cache can return a stale set and create a duplicate sibling. This is a latent bug in a retryable production endpoint. I can file it separately.
nesquena-hermes
left a comment
nesquena-hermes
left a comment
There was a problem hiding this comment.
Re-gate result at a57e73d9
Thank you for the thorough rework. Two of the three prior blockers are closed:
- The composer-chip POST now rejects late success and failure callbacks after a session/key or sequence change.
- Duplicate, branch, and focused-continuation constructors now preserve
reasoning_effort, with tests that distinguish the source override from the global fallback.
One objective fail-open path remains in the /reasoning <effort> command.
Blocker: missing ownership helpers reopen the global-write and stale-publication bugs
In static/commands.js, the new effort branch uses these fallbacks:
const ctx=(typeof _reasoningEffortContext==='function')?_reasoningEffortContext():{};
const key=(typeof _reasoningEffortQuery==='function')?_reasoningEffortQuery():'';
const seq=(typeof _reasoningFetchSeq==='undefined')?null:++_reasoningFetchSeq;
const current=function(){
if(seq===null||typeof _reasoningDispatchIsCurrent!=='function') return true;
return _reasoningDispatchIsCurrent(seq,key);
};If _reasoningEffortContext is unavailable, the command deliberately sends the original bare {effort} payload and mutates the profile-global default. If the sequence or predicate helper is unavailable, the command deliberately applies the late chip update and toast. Those are the two failure modes this rework is meant to close. The normal page loads ui.js before commands.js, so dependency failure or bundle skew should fail closed rather than perform a higher-scope mutation.
The new slash-command tests always inject all three helpers, so all eight pass without exercising this branch.
Required fix
Make the ownership dependencies mandatory in the effort branch:
const ctx=_reasoningEffortContext();
const payload=Object.assign({effort:arg},ctx);
const key=_reasoningEffortQuery();
const seq=++_reasoningFetchSeq;
const current=()=>_reasoningDispatchIsCurrent(seq,key);Keep the existing current() checks in both success and rejection. No-active-session behavior remains global through _reasoningEffortContext() itself, which already omits session_id when there is no active session. Do not preserve it through a missing-helper fallback.
Please add a fail-closed regression that evaluates the real command effort block without each required ownership helper and asserts zero API mutations, chip writes, and toasts (or replace the fallback with direct required calls and pin that source/integration contract).
Mandatory sandbox gate at this head was clean and ran the focused suite: 23 passed across dispatch-race, slash-command scope, copy inheritance, and original session-scope tests. The remaining issue is an uncovered production branch, not a red existing test.
Manny7717
left a comment
Manny7717
left a comment
There was a problem hiding this comment.
Fully locally verified (4 test files run against real agent on PYTHONPATH).
Bug is real. Every read/write path (GET /api/reasoning, POST /api/reasoning, streaming local-agent, gateway) shared the single profile-global agent.reasoning_effort config key, so a chip pick in one session silently changed every session — including other models and other tabs.
Fix is correct and complete.
- Session gains a persisted
reasoning_effortfield (top-level metadata, listed in METADATA_FIELDS). Storage is RAW; coercion happens at each use site per resolved model/provider (coerce_reasoning_effort_for_model), so a stored xhigh on a session later switched to a capped model degrades correctly instead of poisoning the stored value. - Local-agent path (streaming.py): session effort preferred over config when not None → coerced →
parse_reasoning_effort. Gateway path: session effort threaded into_gateway_reasoning_effort_for_requestidentically. POST validates against VALID_REASONING_EFFORTS (+ 'none', + '' for Default/clear) and evicts the cached agent under the session lock so the next turn rebuilds with the new reasoning_config (matches "applies to next turn"). - All FOUR copy constructors carry the field: duplicate (route ~15415), branch (~16295), fork (~16300 region), and the compression-recovery focused continuation (~23800) — plus the tests pin each.
- Frontend: chip POST and /reasoning slash command both attach
session_idvia_reasoningEffortContext(), with snapshot (seq,key) stale-response guards (_reasoningDispatchIsCurrent) so a late response from session A can't write A's effort onto B's chip/cache. The #4650 no-storm suite still passes with the widened seq semantics. - Backward compat: old session files lack the key →
cls(**data)default None → config fallback; metadata-only prefix path passes the field through when present._evict_session_agentpre-exists (config.py:9510).
Regression proven: all 4 test files applied to base (e168b67) → 22/23 FAIL (session write path, slash-command guards, and all three copy-constructor inheritances); on head 23/23 PASS. Neighbors (test_issue4650_reasoning_chip_no_storm, test_issue1103_reasoning_chip_visibility, test_custom_provider_bare_model_reasoning, test_issue3958_reasoning_post_session_context): 54/54 head == 54/54 base — zero regressions. node --check clean on ui.js/commands.js; repo ruff gate: 0 new violations.
Non-blocking notes:
- Picking the chip's "Default" stores
''on the session, which reads asnot Noneforever after — i.e. Default pins model-default and wins over any LATER profileagent.reasoning_effortchange for that session (matching the codebase's documented'' = defaultconvention and the old global clear semantics, but if "follow the profile" was the intent, storing None/removing the key would be the alternative). Worth a docstring line so future maintainers don't "fix" it accidentally. - Repeated POST of the same effort still evicts the cached agent (minor churn; a no-op guard could skip save+evict when unchanged).
- The 4 commits could be squashed for a cleaner history (optional; content is coherent as-is).
nesquena-hermes
added
size:L
The /reasoning <effort> branch reached its four ui.js ownership symbols
through typeof fallbacks. Every one failed OPEN into the defect this
change set exists to close.
Without _reasoningEffortContext the substituted empty context produced a
bare {effort} POST, which mutates the PROFILE-GLOBAL default instead of
the session the user is looking at. Without _reasoningFetchSeq or
_reasoningDispatchIsCurrent, current() returned true unconditionally, so
a superseded response still wrote the chip and raised a toast naming an
effort for a session the user had already left.
index.html loads ui.js (line 1775) before commands.js (line 1779), both
defer, so ui.js runs to completion before this handler can exist. No
legitimate load order needs those fallbacks. They covered only dependency
failure and bundle skew, and in both cases a higher-scope mutation is
worse than no mutation.
Call all four symbols directly. A missing symbol now raises a
ReferenceError, which the branch catches and reports as a toast before
any request goes out. The throw is caught rather than propagated because
messages.js:1494 invokes the handler with no try/catch inside an async
send() that ui.js:8418 calls unawaited: an escaping throw would skip the
composer clear and the dropdown hide, and would surface only as an
unhandled rejection. The /pet handler at messages.js:1505 already
establishes catch-and-report for a failing command handler.
_reasoningDispatchIsCurrent is captured into a local BEFORE dispatch
rather than referenced lazily inside current(). A lazy reference resolves
only when the response settles, which is after the POST has already
mutated server state; the new tests caught that ordering directly.
Behaviour deliberately preserved: with no active session the command
still writes the profile-global default. That falls out of
_reasoningEffortContext() omitting session_id on its own, never out of a
missing-helper fallback. Positive control covers it.
The prior slash-command suite injected all three helpers in every
scenario, so all eight tests passed without ever entering a fallback.
The new file drives the real production effort block with one required
helper absent at a time and asserts zero API requests, zero chip writes,
and zero effort-claiming toasts. It also pins the index.html load order
the mandatory calls depend on.
Line 1885's q fallback is left alone: it sits in the no-args STATUS
branch and degrades to an unscoped read, not a higher-scope mutation.
Mutation witness: restoring each of the four fallbacks in turn turns the
new suite red (3, 5, 5, and 5 failures), each mutant passing node --check
so the signal is behavioural. Restoring the fix returns 25 passed.
Verified:
- tests/test_reasoning_effort_slash_command_fail_closed.py: 25 passed
- dispatch_race + slash_command_scope + session_copy_inheritance +
session_scope: 23 passed, matching the pre-change baseline
- pytest -k "reasoning or slash or command": 781 passed, 3 skipped
- node --check static/commands.js: clean
|
The remaining fail-open path from the 2 September re-gate is closed at head Commit 1. The
|
nesquena-hermes
left a comment
nesquena-hermes
left a comment
There was a problem hiding this comment.
Changes requested: exact-head re-gate at d8a9c33cb920
Thanks for removing the permissive helper fallbacks. The direct calls close the bare global-write path for undeclared context/query dependencies, and the new production-extracted target is green. One ordering/type-validation gap still fails open in the exact branch being fixed.
Blocker: dependency failure mutates the shared dispatch generation before it fails
In static/commands.js::cmdReasoning(), the effort branch does this in order:
key = _reasoningEffortQuery();
seq = ++_reasoningFetchSeq;
const isCurrent = _reasoningDispatchIsCurrent;
current = function(){ return isCurrent(seq, key); };If _reasoningDispatchIsCurrent is undeclared, the catch correctly prevents the POST, but _reasoningFetchSeq has already advanced. A cold in-flight fetchReasoningChip() captured the old sequence/key. Its success or rejection then returns early at the stale-generation checks in static/ui.js, while _lastReasoningFetchKey remains set; same-key syncReasoningChip() can short-circuit instead of retrying. Merely invoking the unavailable command can therefore strand chip hydration.
There are two related present-but-unavailable shapes. If _reasoningDispatchIsCurrent exists but is undefined or non-callable, assignment succeeds and the POST is sent; only the response callback throws from current(), after server state was mutated. If _reasoningFetchSeq exists with value undefined, prefix increment produces NaN rather than throwing and dispatch continues. The prior code explicitly treated typeof _reasoningFetchSeq === 'undefined' as unavailable, so this is part of the dependency contract the rework is meant to close.
The new test slices and executes the real command block, which is good, but it deletes each declaration entirely. It does not exercise present-but-undefined/non-callable dependencies, never reports/asserts the counter, and only resolves _pendingResolve; _pendingReject is assigned but not invoked. Its 25 green cases therefore miss the mutation-order and rejection shapes above.
Fix: resolve and validate every dependency before mutating _reasoningFetchSeq. Require a callable captured predicate and a finite safe-integer counter, then increment/assign the next generation and build current(). Add present-but-undefined/non-callable counter/predicate cases and assert zero API/chip/saved-effort publication and no sequence advance. Add a production-extracted interleaving: start a cold chip fetch, invoke the command with the predicate unavailable, settle the earlier GET as success and rejection, and prove it is neither superseded nor left permanently short-circuited. Preserve the no-active-session global-write positive control and the existing A→B success/rejection guards.
Verification
The mandatory wrapper classified this exact head CLEAN, entered Layer 3, and ran only the new focused target: 25 passed in 7.60s. No bare PR code execution, full suite, browser/server, merge, or contributor-branch action was performed.
…eneration The 3 September re-gate at d8a9c33 found that a dependency failure mutated shared state before it could fail. The effort branch of cmdReasoning() ran: key = _reasoningEffortQuery(); seq = ++_reasoningFetchSeq; # mutates FIRST const isCurrent = _reasoningDispatchIsCurrent; # may throw AFTER _reasoningFetchSeq is a shared dispatch generation. fetchReasoningChip() and syncReasoningChip() in ui.js compare their captured sequence against it. Advancing it and then failing supersedes a cold in-flight chip fetch that captured the old value. That fetch returns early at its stale-generation check while _lastReasoningFetchKey stays set, so a same-key syncReasoningChip() short-circuits instead of retrying, and chip hydration strands. Merely invoking an unavailable command caused that. Two related shapes were also unguarded: - _reasoningDispatchIsCurrent present but undefined or non-callable. The assignment succeeded, the POST went out, and only the response callback threw — after server state changed. - _reasoningFetchSeq holding undefined. Prefix increment yields NaN, which throws nothing and makes every later generation comparison false. The branch now resolves and validates every dependency before the first mutation: read the context and the query, require a callable predicate, require a safe-integer counter read BEFORE it is written, and only then increment and bind. A float counter is rejected too, since a non-integer generation cannot be compared for equality reliably. Measured on the pre-fix head by driving the real branch under node, with dependencies installed as genuine globals so the increment is observable: predicate undeclared 0 POSTs counter 7 -> 8 (wrongly advanced) predicate present-but-undefined 1 POST counter 7 -> 8 (threw in .then()) predicate non-callable 1 POST counter 7 -> 8 (threw in .then()) counter present-but-undefined 1 POST counter -> NaN After the fix every one of those sends 0 POSTs, writes no chip, and leaves the counter untouched, while both positive controls still POST once and advance the counter exactly once. tests/test_reasoning_effort_dependency_order.py adds 21 tests covering what the sibling module missed. That module omits each helper entirely, so it only exercises the UNDECLARED shape; it never reports or asserts the counter, and it assigns _pendingReject without ever invoking it. New coverage: - present-but-undefined and non-callable predicates, and undefined / NaN / absent / float counters - an assertion that the shared generation does NOT advance on any failure path - the production-shaped interleaving: a cold chip fetch captures generation N, then an unavailable command must not supersede it, with a discriminating control proving a SUCCESSFUL command still does - the rejection arm, by actually invoking _pendingReject - source-order assertions, so moving the increment back above the guards fails Mutation witness, all four mutants passing node --check so the signal is behavioural: - restore the pre-fix head d8a9c33: 14 of 21 fail, while the sibling module stays 25 of 25 green — direct evidence the existing suite could not see this - hoist the increment back above both guards: 7 fail - drop only the callable check: 7 fail - drop only the safe-integer check: 6 fail Filtered suite (-k "reasoning or effort or chip"): 595 passed, 3 skipped, 0 failed.
|
I fixed the blocker at head Dependency order and fixIn
The branch now resolves every dependency and applies each guard before the first mutation. It reads the context and query, requires a callable predicate, and requires a safe-integer generation read before the write. Only then does it increment and bind. The guard also rejects non-integer generations because float generations cannot support reliable equality comparisons. My first reproduction was wrongMy first harness passed dependencies as I rebuilt the harness on The starting generation is 7 in every row:
Both positive controls still POST once, write the chip once, and advance the generation from 7 to 8 exactly once. Tests and mutation evidence
The new tests cover:
All four mutants pass
The sibling module staying green under the pre-fix code directly shows that the existing suite cannot detect this defect. Verification: 56 passed across the four reasoning-effort test files. The suite filtered on reasoning, effort, and chip reports 595 passed, 3 skipped, 0 failed. Two open questions
The branch reports BEHIND |
| .catch(function(){ | ||
| if(!_reasoningDispatchIsCurrent(seq,key)) return; | ||
| showToast('🧠 Failed to set effort'); |
There was a problem hiding this comment.
When a user switches from session X to session Y and selects an effort before Y's chip GET completes, the POST supersedes that GET. If the POST fails, this handler retains Y's optimistic fetch key and X's cached effort, causing later topbar synchronizations to keep displaying X's effort in session Y instead of refetching.
nesquena-hermes
left a comment
nesquena-hermes
left a comment
There was a problem hiding this comment.
The new dependency validation and generation ordering close the previously requested blockers. One same-owner rejection race remains in both reasoning POST surfaces.
A cold fetchReasoningChip() GET sets _lastReasoningFetchKey=K at generation N. /reasoning high or the composer reasoning option then advances the shared generation to N+1 and dispatches a POST. If that POST rejects while the same owner remains current, the catch only reports an error. The old GET is now stale and cannot hydrate the chip, while _lastReasoningFetchKey remains K with _currentReasoningEffort still null. The next same-key syncReasoningChip() short-circuits, so hydration stays stranded until an unrelated invalidation.
Please add a shared current-owner-only rejection recovery for both static/commands.js and static/ui.js: after the existing (seq,key) ownership check, start a fresh authoritative fetch or clear only the current owner's cached key so the next sync must retry. A stale rejection must not disturb a newer owner. Add production-composed regressions that queue the real cold GET, reject each real POST while current, settle the stale GET, then prove a fresh recovery GET hydrates the chip; also cover the inverse stale-error schedule.
Exact-head evidence: threat scan CLEAN; mandatory sandbox targets passed 21/21 and 8/8. The submitted rejection case uses cold_fetch=False, so it does not cover this interleaving.
3 participants
Problem
Setting the reasoning-effort chip in one WebUI session changed it in every session, including sessions on other models and other browser tabs.
Every read and write path shared one global key —
agent.reasoning_effortinconfig.yaml:GET /api/reasoning(chip label)agent.reasoning_effortPOST /api/reasoning(chip pick)agent.reasoning_effortapi/streaming.py(local agent)agent.reasoning_effortapi/gateway_chat.py(gateway)agent.reasoning_effortThe chip is rendered per session in the composer, so it looked per-session while being global. Picking
xhighin a scratch session silently upgraded every other open conversation.Fix
Add
Session.reasoning_effortand prefer it wherever the effort is resolved.api/models.py— persist the field in the metadata prefixstatic/ui.js— sendsession_idwith the chip GET and POSTapi/routes.py— GET reads the session value; POST writes it and evicts the cached agent so the next turn rebuilds with the new configapi/config.py—get_reasoning_status()override parameterapi/streaming.py— local agent path prefers the session valueapi/gateway_chat.py— gateway path prefers the session valueBoth request paths are updated because they read the same key. Fixing only the local path would leave gateway-routed WebUI chats globally scoped.
Compatibility
Three-state, so nothing existing changes behaviour:
session.reasoning_effortNone(existing sessions, CLI, cron)config.yaml— unchanged""No migration. Sessions saved before this change load with
Noneand behave exactly as before. The CLI and cron never sendsession_id, so they keep reading profile config.Testing
tests/test_reasoning_effort_session_scope.py— 4 tests:get_reasoning_status()prefers the session override, including""vsNonePOST /api/reasoningwith asession_idmutates only the target sessionRevert-sensitive: removing just the
Session.reasoning_effortfield fails tests 1 and 3 withAttributeError: 'Session' object has no attribute 'reasoning_effort'. Restored, 4 pass.Suite: 373 pass across
tests/test_*reasoning*.py+tests/test_webui_gateway_chat_backend.pyon a branch off currentmaster. Full suite on this host: 13966 passed, 10 failed — all Playwright, failing onGLIBC_2.27 not foundbefore this change and unrelated to it.Not included
No UI badge distinguishing "session override" from "profile default". The chip shows the effective value either way. Worth adding if it turns out people can't tell which they're looking at.