Skip to content

[codex] Polish Lab TUI agent output#623

Open
willccbb wants to merge 3 commits into
mainfrom
codex/hide-skills-in-tui
Open

[codex] Polish Lab TUI agent output#623
willccbb wants to merge 3 commits into
mainfrom
codex/hide-skills-in-tui

Conversation

@willccbb
Copy link
Copy Markdown
Member

@willccbb willccbb commented May 9, 2026

Summary

  • Replace Lab sync TUI log filtering with typed LabSyncProgressEvent milestones; the TUI renders only those event kinds.
  • Move doctor row copy to the check source so the TUI displays source-provided Lab asset labels directly.
  • Keep agent transcript presentation deterministic: non-widget dynamic tool calls stay hidden by status, widgets use action metadata, ACP tool events render only text-bearing updates, and assistant text is no longer filtered by phrases.
  • Make eval config widgets foreground picker controls instead of config paths, including environment/model selects when values are known.

Validation

  • uv run --project packages/prime pytest packages/prime/tests/test_lab_view.py -q -> 209 passed
  • Focused Lab TUI slice -> 17 passed, 192 deselected
  • uv run --project packages/prime ruff check packages/prime/src/prime_lab_app/agent_runtime.py packages/prime/src/prime_cli/lab_setup.py packages/prime/src/prime_lab_app/setup_screens.py packages/prime/tests/test_lab_view.py -> passed
  • uv run --project packages/prime ty check packages/prime/src/prime_lab_app/agent_runtime.py packages/prime/src/prime_cli/lab_setup.py packages/prime/src/prime_lab_app/setup_screens.py -> passed
  • git diff --check -> passed

Note

Medium Risk
Changes how prime lab sync progress and agent/tool events are emitted and rendered in the Lab TUI, which can affect user-visible status updates and chat transcript behavior. Risk is moderate due to modified message-filtering logic across multiple agent transports, but changes are largely presentation-focused and covered by updated tests.

Overview
Lab sync now emits typed progress milestones (LabSyncProgressEvent) and the TUI renders only these events, replacing raw log streaming; sync failure also reports a failed milestone.

Lab doctor and sync UI copy is updated to consistently refer to Lab assets (not skills/templates/docs) and to show clearer sync progress messages.

Agent chat output rendering is tightened: ACP tool events are recorded only when they include non-empty text and now include tool_kind metadata; non-widget dynamic tool chatter is hidden, widget messages use action titles for deterministic display, and eval/run widget cards/transcripts show a picker summary instead of config paths.

Eval config widgets adjust field rendering so known envs values use a Select control, and tests are expanded/updated to assert the new progress events, copy, and transcript behaviors.

Reviewed by Cursor Bugbot for commit 994af56. Bugbot is set up for automated code reviews on this repo. Configure here.

) -> None:
display_content = _dynamic_tool_chat_content(status, content, action)
if display_content is None:
return
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Early return skips streaming assistant finalization

Low Severity

When _dynamic_tool_chat_content returns None (i.e., status == "tool"), _record_dynamic_tool_call now returns before reaching _finish_latest_streaming_assistant_locked(fallback=""). Previously, this finalization was always called, ensuring any active streaming assistant message was properly completed (or removed if empty) before appending a system message. Now, if there happens to be an active streaming assistant message when a "tool" status event arrives, it will remain in the "streaming" state until some other event finalizes it, potentially causing a lingering streaming indicator in the UI.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 1924dd4. Configure here.

@willccbb willccbb marked this pull request as ready for review May 9, 2026 20:35
@willccbb willccbb requested review from d42me and mrmoxon May 9, 2026 20:35
Copy link
Copy Markdown

@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: 1924dd40af

ℹ️ 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 +508 to +509
if normalized.startswith("sync failed:"):
return "Sync failed: Lab asset refresh failed"
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 sync failure cause when redacting skill lines

When a sync error message contains skill (for example exceptions like Skill 'x' is missing SKILL.md. or duplicate-skill errors), this branch rewrites the full Sync failed: ... line to the generic Sync failed: Lab asset refresh failed, which removes the actionable failure reason from the TUI. run_lab_sync_service already emits the precise exception text, so this redaction makes real sync failures hard to diagnose and forces users to rerun outside the screen to understand what broke.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 994af56. Configure here.

with self._lock:
if (
not text
and _is_lab_widget_tool_event_title(label)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Redundant truthiness check after early return guard

Low Severity

The if text and condition on line 745 is redundant because lines 743–744 already return early when not text. At this point, text is guaranteed to be truthy, making the truthiness check in if text and _is_lab_widget_tool_result_text(text) unnecessary.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 994af56. Configure here.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant