Conversation
6ce66f5 to
19dd415
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19dd415165
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| : hideThinkingUnderFinishedAssistant | ||
| ? null |
There was a problem hiding this comment.
Honor live presence while the next goal turn starts
When an idle goal is resumed, startGoalTurn intentionally creates no user message, and continueSession publishes thinking presence before openAssistantEntry() completes. During that interval the transcript still ends with the previous finished assistant, so this branch nulls the activity label despite authoritative live presence and makes the newly running goal turn appear inactive; keep the presence-driven label until the presence itself ends or introduce an explicit finalization phase rather than inferring inactivity from history.
AGENTS.md reference: packages/components/src/components/sessions/AGENTS.md:L88-L95
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed. Last-history finished is no longer the hide signal.
The execution owner now reports leftover finalize as optional running.phase = finalizing from markPromptWorkingEnded (prompt complete and autoPrompt complete). markPromptWorkingStarted restores thinking. The activity row hides only when live presence is that phase, so a goal resume with no user row keeps Thinking while presence is running.
continueSession already opens the assistant entry before initializing; moving the later thinking publish cannot fence a viewer whose history snapshot still lags. Old presence parsers strip unknown phase and keep { type: 'running' }.
Head 991558a2.
19dd415 to
9490c2d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9490c2d5c2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return false; | ||
| } | ||
| const last = history[history.length - 1]; | ||
| return last?.role === 'assistant' && last.finished === true; |
There was a problem hiding this comment.
Preserve authoritative running presence across history lag
When another client receives running presence for a new goal/system turn before its session-history stream receives the new assistant entry, this check still sees the previous finished assistant and suppresses the activity label for a genuinely running turn. The fresh evidence after the prior comment is that moving the phase update after the host's openAssistantEntry() only orders local calls; it does not fence independent presence and history synchronization for viewers. Correlate the status with a turn/finalization phase rather than allowing lagging transcript state to override live presence.
AGENTS.md reference: packages/components/src/components/sessions/AGENTS.md:L88-L95
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed. Host-local call order does not fence independent presence and history delivery.
The hide helper now reads only live presence: shouldHideThinkingDuringFinalization is true iff status.type === 'running' and status.phase === 'finalizing'. A lagged finished assistant row cannot suppress a genuine running turn. The host publishes that phase at the shared prompt/autoPrompt exit and does not add activity: 'finalizing', which older ActiveSessionStatusSchema would reject.
Head 991558a2.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 991558a2e5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| case 'finalizing': | ||
| return { type: 'running', phase: 'finalizing' }; |
There was a problem hiding this comment.
Prevent image callbacks from undoing finalization
When Codex image generation ends near prompt completion, the queued enqueueImageGenerationActivityStatusSync callback can read the old running(image_generation) status before this transition, then complete afterward and write both durable running status and presence phase thinking. That overwrites the new finalizing phase, so once finalizeACPState stamps the assistant entry finished, the exact Thinking-under-finished-reply bug reappears until presence releases. Fence queued image-status writes against finalization or serialize both transitions through the presence owner.
AGENTS.md reference: apps/cli/src/lib/loro/AGENTS.md:L73-L77
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Image generation is a live presence activity. The callback no longer writes durable SessionMeta.status (that path awaits getDocMeta/upsert and can land after prompt-end). It only retargets thinking ↔ image_generation through the presence owner; finalizing, permission, and initializing stay owned by the turn scope.
Head a3ce0746. A queued image end after finalizing + idle keeps both presence and durable status.
a3ce074 to
e2c6013
Compare
Replay finalizing onto current main after LodyAI#852. Prompt completion calls setPhase('finalizing') once, with no detail. The activity row hides only when live presence is running in that phase. Image generation stays one presence phase change and does not write durable status.
e2c6013 to
cf968c1
Compare
Related issue
Closes #613
Problem / pressure
A human tester on Windows Lody 0.93.3 saw a completed first Codex reply, including its completion time, with Thinking / 思考中 still shown underneath until host finalization ended.
History and live presence arrive independently. Hiding Thinking whenever the last visible assistant is finished can also hide a new goal turn that is already running while its history update is still in transit.
A queued Codex image-generation status callback can read the old
running(image_generation)snapshot, then write durablerunningand presencethinkingafter prompt-end has already publishedfinalizingand idle. Checking phase aroundsetStatusis not enough: the real document write also awaitsgetDocMeta/upsert.Summary
The execution owner reports optional
phase: 'finalizing'on running presence when a prompt finishes. The UI hides Thinking only for that explicit phase. A resumed goal or autoPrompt reports ordinary running presence and keeps its activity indicator even when viewer history still shows the previous completed reply.Codex image begin/end update presence activity only (
thinking↔image_generation) through the presence owner. They do not write durableSessionMeta.status, so an in-flight image callback cannot undofinalizingor idle.Presence remains owned by the execution scope through finalization. Initializing and permission labels retain their existing behavior. Older clients ignore the optional field and still recognize the running session; older hosts without the field retain the existing Thinking display during finalization.
Visual explanation
Test plan
Replayed onto current
mainafter revert: keep Codex reasoning in session history, and bound the ephemeral presence channel #852 ascf968c13. This replaces the old conflicted branch.finalizingis onesetPhaseat prompt completion and carries nodetail. Image generation remains one presence phase change and does not write durable status. The activity row hides only when live presence isrunningandphase === 'finalizing'.Human confirmation of the original behavior on Windows 0.93.3:
finalizing+ idle does not restorerunning.setStatus) fail on991558a2and pass ona3ce0746. An expanded CLI suite of 136 tests (image-upload, activity-status, permission-notification, execution, presence controller) passed, including those two counterexamples.a3ce0746: Tests, Static checks, and Desktop E2E (smoke) all passed.Context handoff
Instructions for reviewing agents
Authoring context
991558a2and pass ona3ce0746; 136 CLI tests passed. GitHub ona3ce0746is all green.Original user prompt
Shared conversation
Status: unavailable
Reason: The user cannot supply a public conversation URL, and this Grok session cannot publish the authoring conversation to an HTTP(S) link.