Skip to content

Conversation

@ThanhNguyxn
Copy link
Contributor

Fixes #7919.

This PR addresses a TUI display bug where the "Worked for" separator would appear prematurely during the planning stage.

Changes:

  • Added had_work_activity flag to ChatWidget to track if actual work (exec commands, MCP tool calls, patches) was performed in the current turn.
  • Updated handle_streaming_delta to only display the FinalMessageSeparator if both needs_final_message_separator AND had_work_activity are true.
  • Updated handle_exec_end_now, handle_patch_apply_end_now, and handle_mcp_end_now to set had_work_activity = true.

Verification:

  • Ran cargo test -p codex-tui to ensure no regressions.
  • Manual verification confirms the separator now only appears after actual work is completed.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 9, 2026

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@ThanhNguyxn
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

github-actions bot added a commit that referenced this pull request Jan 9, 2026
@etraut-openai
Copy link
Collaborator

@codex review

Copy link
Contributor

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

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: 93baa9bf16

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ThanhNguyxn
Copy link
Contributor Author

Thanks for the review feedback! Here's my analysis:

Regarding view-image tool calls

The view-image tool call is intentionally not counted as "work activity" because:

  1. It's a read-only operation - viewing an image doesn't execute code, modify files, or call external tools
  2. The "Worked for X seconds" separator is meant to indicate actual work/modifications were performed (exec commands, patch applications, MCP tool calls)
  3. Semantic correctness - saying "Worked for 5 seconds" when the only action was viewing an image would be misleading to users

The current behavior is correct: separator only shows when exec, patch apply, or MCP tool calls complete, which represent actual work.

@joshka-oai joshka-oai self-assigned this Jan 13, 2026
@ThanhNguyxn
Copy link
Contributor Author

Thanks for the review! Regarding the view-image concern:

I intentionally didn't count view_image as work activity because:

  1. It's read-only - no code execution, no file modifications, no external API calls
  2. Semantically misleading - showing "Worked for 2s" when only viewing an image doesn't match user expectations of "work"
  3. Issue context - the original issue Sometimes see "Worked for" when job at planning stage #7919 is about "Worked for" appearing during planning phase when no actual work occurred

However, I'm happy to add it if you prefer consistency (any tool call = work).

Also, I'll rename had_work_activitywork_occurred as you suggested.

Please let me know which direction you'd prefer!

@etraut-openai etraut-openai added the needs-response Additional information is requested label Jan 15, 2026
- Add per-turn work tracking (exec/patch/MCP) in tui2 so 'Worked for …' only appears when work happened.
- Mirror the separator reset behavior for no-work turns.
- Add/align rustdoc/field comments explaining the separator and gating flags in both tui and tui2.
Copy link
Collaborator

@joshka-oai joshka-oai left a comment

Choose a reason for hiding this comment

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

I added docs and updated this to also contain logic for tui2

@joshka-oai joshka-oai enabled auto-merge (squash) January 16, 2026 01:31
@joshka-oai joshka-oai merged commit a6324ab into openai:main Jan 16, 2026
26 of 32 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jan 16, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

needs-response Additional information is requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sometimes see "Worked for" when job at planning stage

3 participants