Repository navigation
fix(ui-web): read the opened conversation's tier, not the one being left - #1122
Handsome-wzw wants to merge 11 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>
…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>
A switch asks for the conversation being opened before the page's current session moves over to it, and the tier read built its request from "the current session": opening B from A asked for A's tier and painted its rung 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 effort and permission-mode reads do, and the source drops an answer whose switch has been overtaken rather than painting a view the reader has left; the catalogue such an answer carries is still kept, being the deployment's and not the conversation's. Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
|
Test counts for this change (ui-web only):
The branch sits on top of #1113, so the diff here shows that PR's commits until it merges; this PR's own change is the last commit. |
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; suggestions only, and they are marked inline.
I reviewed git diff github/main...HEAD, then isolated the final commit because this branch is stacked on #1113. I also checked the tier source/store seam, every loadTier caller, session-switch and promotion ordering, relevant history, and the Web UI architecture terms in CONTEXT-MAP.md / ui-web/CONTEXT.md. The main A-to-B switch fix is sound: reads now name the opened session and carry the switch generation, while superseded successful replies cannot repaint the current view. The optional source arguments preserve existing callers, and the tests strengthen rather than weaken the prior coverage.
Verification: npm exec -- vitest run src/state/tier.test.ts src/state/session/registry.test.ts src/features/model/source.test.ts src/features/model/ModelPicker.test.tsx src/state/effort.test.ts passed 5 files / 180 tests; npm run type-check passed. git diff --check github/main...HEAD also passed. I found one narrow staged-promotion race and left it inline as a nonblocking follow-up; reopening the conversation recovers, so it does not hold this fix.
|
Windows compatibility review — PR #1122 @ Scope: full base Result: no Windows-specific issue. The delta is browser state logic (same pattern as the effort fix, applied to the reasoning tier); no path, filesystem, subprocess, signal, encoding, or platform primitive. The stacked effort code was already cleared for Windows in the #1113 review. Verification (native Windows 11, isolated worktree The single Python failure is the inherited POSIX Limitations: TUI tests not run (harness unavailable in this checkout); no real-browser walkthrough; the stacked #1113 code is covered by that PR's review rather than re-measured line by line here. |
The re-read after a draft's staged tier is written to the session its first message minted took its switch 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 `applyStagedTier` the way it already rides into `applyStagedModel`, and the source drops the late answer. Reopening the conversation recovered before; now there is nothing to recover from. Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
|
On the nonblocking thread at
The same shape exists in |
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
I reviewed the three-file delta from bcbdabe33f69 to this head against the full github/main...HEAD change, including the promotion caller, generation-ticket contract, source seam, relevant history, compatibility of the changed function signature, and the new tests. The prior staged-tier race is fixed without weakening coverage, and I found no new issue. The thread I opened is resolved.
Verification: the focused five-file Vitest run passed 182 tests, npm run type-check passed, and git diff --check github/main...HEAD passed.
|
Windows compatibility re-review — PR #1122 @ Scope: delta from the previously reviewed head Result: no Windows-specific issue.
Verification (native Windows 11, isolated worktree The new cases assert both halves: the re-read carries Limitations: TUI tests not run; no real-browser walkthrough. |
There was a problem hiding this comment.
Blocking: carried from #1113. This PR targets main and contains #1113's head 516c7e7d (git merge-base --is-ancestor 516c7e7d 321ac378 holds). Merging it as it stands therefore merges #1113 too, including the defect I filed there as blocking: the Provider default rung clears agents.defaults.reasoningEffort, but a running gateway keeps sending the effort it booted with while session.info / session.set_effort report the clear (thread on #1113, anchored to raven/agent/loop/turn_path.py).
This PR's own two commits are fine. I read 516c7e7d..321ac378 in full. tierSource.read(sid, gen) names the conversation being opened and drops an answer whose switch was overtaken (ui-web/src/features/model/source.ts). The catalogue is still kept from every answer. switchTo, switchToDraft and promote pass the conversation and the switch ticket (ui-web/src/state/session/registry.ts, ui-web/src/state/session/runtime.ts), and a dropped read paints nothing (ui-web/src/state/tier.ts). This matches the effort and permission-mode reads.
This clears when #1113's fix lands and this branch is rebased onto it, or once #1113 merges and this PR's diff against main is only its own commits. Read only; local tests not run.
|
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. |
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: this branch carries #1113's unresolved Provider default defect and conflicts with current main.
My prior clean stance is withdrawn based on the new record. This PR contains #1113's still-open blocker: clearing agents.defaults.reasoningEffort can leave a running gateway sending its boot-time effort while the RPC surfaces report provider default. I am not re-grading that reviewer's finding; I confirmed #1113 remains open at 516c7e7d4284 with that thread unresolved and with an independent reproduction already recorded there. This PR's own tier commits remain fine.
I also fetched current github/main (26352da09) and reproduced the reported merge conflict: git merge-tree --write-tree HEAD github/main conflicts in CHANGELOG.md. The branch therefore cannot merge as it stands.
Verification: the first Python run did not reach tests because the fresh environment lacked raven_everos (225 setup errors). After uv sync --extra dev, uv run pytest tests/test_agent_loop_usage_sink.py tests/test_rpc_session.py tests/test_rpc_settings.py -q passed 384 tests.
…_session # Conflicts: # CHANGELOG.md
|
Merged Verification on the merged tree (local):
The stacking is unchanged: the branch still carries #1113's head 516c7e7, so the Provider default thread carried from #1113 stays open here until #1113's fix lands. This PR's own change is still its two tier commits (bcbdabe, 321ac37). |
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: the branch still carries #1113's unresolved Provider default defect.
I reviewed the merge commit from 321ac3788631 to this head, the effective github/main...HEAD diff, the --remerge-diff conflict resolution, relevant callers, and the existing review record. The former CHANGELOG.md conflict is resolved correctly: both sides' entries are retained, git diff --check github/main...HEAD passes, and git merge-tree --write-tree HEAD github/main now succeeds. I found no merge-resolution regression, backward-compatibility issue, weakened test, or new architecture/rules problem.
The stance remains blocked because this branch still contains #1113 head 516c7e7d4284, and its Provider-default defect remains unresolved; the author explicitly confirms that stacking and open thread are unchanged. This is not a new finding or a re-grade of another reviewer's blocker.
Verification on the merged tree: the focused five-file Web Vitest run passed 182 tests, and uv run 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_session_init_bundle.py -q passed 431 tests.
…g_effort # Conflicts: # CHANGELOG.md
|
Windows compatibility re-review — PR #1122 @ Scope: the head moved from the previously reviewed Result: no Windows-specific issue; the previous conclusion holds. Verification (native Windows 11, isolated worktree The single failure is the inherited POSIX Limitations: TUI tests not run; 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>
…nto fix/tier_reads_opened_session # Conflicts: # CHANGELOG.md
|
Merged #1113's new head 01f2455 (its Provider default fix 5057b49, carrying main up to 984d47d) into the branch as 5fb1c55, then With that, the branch contains the fix for the thread carried from #1113 (Provider default), and the CHANGELOG conflict with main is gone. Verification on the merged tree (local):
This PR's own change is still its two tier commits (bcbdabe, 321ac37); everything else in the diff is #1113 and main. |
|
Windows compatibility re-review — PR #1122 @ Scope: the head moved from the previously reviewed Result: no Windows-specific issue; the previous conclusion holds. Verification (native Windows 11, isolated worktree The single failure is the inherited POSIX Limitations: TUI tests not run; no real-browser walkthrough. |
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: this branch still carries #1113's open model-specific reasoning-effort options defect.
I reviewed both merge commits from 44442a4e485b to this head, the effective github/main...HEAD diff, the 5057b49f0 Provider-default fix and its callers/tests, both CHANGELOG.md merge resolutions, relevant history, compatibility, repository rules, and the existing review record. The prior Provider-default blocker is fixed: the turn, wrap-up and watch-work paths now forward the live effort including explicit None, the RPC and loop share session_effort.in_force, its #1113 thread was re-verified and resolved, and current main merges cleanly. I found no new issue in those changes or in this PR's tier commits.
The revision remains blocked by a different #1113 thread opened after that fix: session.set_effort still advertises and accepts the global REASONING_EFFORTS list without regard to the conversation model, allowing values such as minimal to reach a Codex model that rejects them. This branch carries that code unchanged. This is another reviewer's blocker, not a new finding or re-grade from me; its source thread is #1113 (comment).
Verification on this merged tree: the focused five-file Web Vitest run passed 182 tests, the ten-file effort/loop/RPC Python run passed 689 tests, git diff --check github/main...HEAD passed, and git merge-tree --write-tree HEAD github/main succeeded.
Summary
The Mode row of the WebUI's model picker (the sub-agent tier) read the
conversation being left rather than the one being opened. A switch asks for
the conversation being opened before the page's current session moves over
to it, and the tier read built its
session.set_moderequest from "thecurrent session": opening B from A asked for A's tier and painted its rung
on B, and picking that very rung on B was taken as already in force, so no
request went out. #1113 fixed the same shape for the reasoning-effort row
and left the tier, which predates it, for this follow-up.
What changes, in ui-web only:
TierSource.read(sid?, gen?)takes the conversation it is about and theswitch it was started under, the shape
EffortSource.readandloadPermModealready have. The live source builds the request from thegiven key (an empty key for a draft, which the handler answers with the
catalogue and its default) and answers null when the switch ticket has
been overtaken, so a late answer about a view the reader has left is
dropped rather than painted. The catalogue such an answer carries is still
kept for the draft's staged echo, being the deployment's and not the
conversation's.
tier.load(sid?, gen?)passes them through and absorbs null as nothing:the chip keeps the last answer about the view on screen, and the
sub-agent instance pane's watcher is not told of a move that did not
happen.
conversation under its ticket, leaving for a draft reads with no key under
its ticket, and the staged-tier write after a draft is minted re-reads
that session. The boot's first read is unchanged (the view as it stands);
its comment, which still credited
session.onChangewith the later reads,now names the switch.
first message minted (
applyStagedTier) carries the ticket the promotionbegan under, as
applyStagedModelalready does. A ticket taken after theawaited write is whichever view the reader has opened since, and under
that one the minted session's rung was painted over that view's own read
(the review bot's nonblocking finding; reopening recovered). The same
hole exists in
applyStagedEffort(on the feat(agent): per-conversation reasoning effort (picker row, /effort, session.set_effort) #1113 branch) andapplyStagedPerm(on main) and is left for their own PRs.session.set_modealready takessession_key. Theoffline fixture, the picker row and the instance-pane watcher need no
change.
Stacked on #1113: until that PR merges, this branch carries its commits and
the diff here shows them as well; this PR's own change is its two tier
commits, bcbdabe and 321ac37. The branch has since merged #1113's head
01f2455 (its Provider default fix) and main 06509f8, as plain merges. It
will be brought onto main once #1113 lands.
Type
Verification
ui-web
vitest run-- 258 files, 4423 tests, the gates included, passui-web
type-check-- clean;lint-- 0 errors (4 pre-existingreact-hooks/exhaustive-depswarnings in files this PR does not touch)Before/after for the switch read: with the four source files put back to
their previous version and the new tests kept, 6 cases fail (the draft
read keyed to no conversation; the read keyed to the given conversation
and switch; the overtaken answer dropped, and its catalogue kept; the
switch reading the opened conversation before the current session moves
over); all pass with the change
Before/after for the staged re-read: with the previous
runtime.tsand thetwo new cases kept, both fail (the recorded ticket is undefined); pass
with the change
New and amended cases:
state/tier.test.ts(the store's read names theconversation and the switch; a dropped read paints nothing and tells no
listener),
state/session/registry.test.ts(opening B from A reads B'skey before
sessionSet('b'); leaving for a draft reads with null underthe switch),
features/model/source.test.ts(an overtaken read resolvesto null; a standing one answers; a draft reads with
session_key: '';the catalogue of a dropped answer is kept; the staged write's re-read is
keyed to the minted session under the promotion's ticket, which a switch
during the write has spent, and under a standing ticket when the reader
stayed)
CHANGELOG: an Unreleased / Fixed entry
Relevant tests pass locally
Relevant lint / type checks pass locally
User-facing docs or screenshots are updated when needed
Risk
with a key the page already holds, and nothing new is written
answers
session.set_modeas before; a reply that lands after a laterswitch is now dropped instead of painted
previous read is one line per call site
Related Issues
N/A