Skip to content

fix: hide thinking indicator under a finished assistant reply - #614

Open
ladydd wants to merge 1 commit into
LodyAI:mainfrom
ladydd:fix/hide-thinking-under-finished-reply
Open

ladydd wants to merge 1 commit into
LodyAI:mainfrom
ladydd:fix/hide-thinking-under-finished-reply

Conversation

@ladydd

@ladydd ladydd commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

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 durable running and presence thinking after prompt-end has already published finalizing and idle. Checking phase around setStatus is not enough: the real document write also awaits getDocMeta / 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 durable SessionMeta.status, so an in-flight image callback cannot undo finalizing or 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

prompt in flight       → running                      → Thinking
image generating       → running + activity           → image activity
prompt complete        → running + phase: finalizing  → no Thinking row
late image end         → still finalizing             → no Thinking row
autoPrompt starts      → running                      → Thinking
turn scope released    → presence cleared             → idle

Test plan

  • Replayed onto current main after revert: keep Codex reasoning in session history, and bound the ephemeral presence channel #852 as cf968c13. This replaces the old conflicted branch. finalizing is one setPhase at prompt completion and carries no detail. Image generation remains one presence phase change and does not write durable status. The activity row hides only when live presence is running and phase === 'finalizing'.

  • Human confirmation of the original behavior on Windows 0.93.3:

Windows 0.93.3: reply 123 already complete, Thinking still shown below

  • Execution-service regression: resume a goal while viewer history still ends on the previous finished assistant; preserve running presence, report finalizing while usage flush is held open, and restore running during autoPrompt.
  • Presence-controller regression: retain ownership and heartbeat during finalization, then transition to thinking or permission.
  • Image activity: presence-only begin/end; a queued image end after finalizing + idle does not restore running.
  • UI tests: hide only for explicit finalizing presence.
  • Shared tests: parse the new field and preserve compatibility with the earlier running-status shape.
  • Independent review: the same two image-callback gates (stale meta read; in-flight setStatus) fail on 991558a2 and pass on a3ce0746. An expanded CLI suite of 136 tests (image-upload, activity-status, permission-notification, execution, presence controller) passed, including those two counterexamples.
  • GitHub on a3ce0746: Tests, Static checks, and Desktop E2E (smoke) all passed.
  • Packaged Windows/Electron click-through of the fix has not been repeated. Exact original overlap duration was not measured.

Context handoff

Instructions for reviewing agents

  • Review focus: Prompt/autoPrompt phase transitions, optional presence-field compatibility, the UI's presence-only decision, and Codex image callbacks that must not write durable status.
  • Decisions to challenge: Whether the shared prompt-completion boundary accurately identifies finalization while retaining execution ownership. Whether image activity should stay presence-only instead of serializing durable writes with prompt-end.
  • Plausible failures / evidence gaps: Presence and history delivery can still differ in timing. This change uses the execution owner's reported phase; it does not make the two streams atomic. No packaged desktop retest of the fix.

Authoring context

  • User goal / directives: Fix the misleading Thinking label beneath a completed reply and address the goal-resume and image-callback regressions identified in review.
  • Constraints / non-goals: Preserve execution-owned presence until scope release. Do not infer live completion from history, mix in fix: keep unsaved editor drafts after Refresh remounts #611, or describe the original transient overlap as a stuck turn.
  • Risk-bearing decisions: Add optional display metadata to running presence. Old clients retain running status; old hosts without the field retain the old display behavior. Image generation activity is presence-only.
  • Destructive or irreversible behavior: None introduced. No changes to persisted history, ACP packages, or cancellation ownership.
  • Deliberately not done or tested: No packaged Windows retest of the fix and no claim of atomic cross-stream delivery.
  • Unknowns / confidence: The original behavior was human-confirmed. Deterministic service/controller tests cover goal history lag and the two image-callback gates. Independent review: those gates fail on 991558a2 and pass on a3ce0746; 136 CLI tests passed. GitHub on a3ce0746 is all green.

Original user prompt

After the assistant reply is finished, Thinking still appears under that message. Include the screenshot.

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.

@ladydd
ladydd force-pushed the fix/hide-thinking-under-finished-reply branch from 6ce66f5 to 19dd415 Compare September 11, 2026 14:28

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +3627 to +3628
: hideThinkingUnderFinishedAssistant
? null

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@ladydd
ladydd force-pushed the fix/hide-thinking-under-finished-reply branch from 19dd415 to 9490c2d Compare September 12, 2026 10:38

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +55 to +56
case 'finalizing':
return { type: 'running', phase: 'finalizing' };

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@github-actions github-actions Bot added status:needs-pr-attention External PR needs contributor attention before review and removed status:needs-pr-attention External PR needs contributor attention before review labels Sep 21, 2026
@ladydd
ladydd force-pushed the fix/hide-thinking-under-finished-reply branch from a3ce074 to e2c6013 Compare September 24, 2026 02:10
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.
@ladydd
ladydd force-pushed the fix/hide-thinking-under-finished-reply branch from e2c6013 to cf968c1 Compare September 24, 2026 02:10

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Finished assistant reply still shows Thinking underneath

1 participant