Skip to content

feat(tui): show and open generated media outputs - #6254

Open
huong8373tt-beep wants to merge 8 commits into
amd:mainfrom
huong8373tt-beep:feat/tui-media-output-cards-6093
Open

huong8373tt-beep wants to merge 8 commits into
amd:mainfrom
huong8373tt-beep:feat/tui-media-output-cards-6093

Conversation

@huong8373tt-beep

Copy link
Copy Markdown
Contributor

Summary

  • add a typed media card for successful generated image, music, sound-effect, and speech outputs
  • preserve only verified absolute regular-file paths in the SSE card payload; do not scrape model prose for paths
  • add one-key actions to open the generated file or reveal it in the file manager, with an explicit SSH limitation
  • show path, size, dimensions/voice, duration, model and keyboard hints in the terminal transcript

Closes #6093.

Validation

  • go test ./internal/ui/cards ./internal/ui/chat
  • python -m pytest tests/unit/chat/ui/test_sse_handler.py tests/unit/test_sse_narration.py -q — 378 passed
  • python -m pytest hub/agents/email/python/tests/test_sse_translation.py -q — 54 passed
  • python -m py_compile on modified Python streaming modules
  • git diff --check

The available local Lemonade media service was not used; this PR validates the wire, path-safety and TUI rendering seams without claiming a live generation run.

Slai Agent Runtime performed the investigation, implementation, and test execution. The account owner authorized this contribution and is responsible for its content.

@github-actions github-actions Bot added tests Test changes agent::email Email agent changes tui Go terminal UI (gaia-tui) labels Oct 31, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Verdict: Request changes

The PR adds a card for generated images and audio in the TUI, with Ctrl+F to open the file and Ctrl+G to show it in the file manager. The safety design is sound: the card only carries a path the tool really wrote and that exists on disk, never a path taken from the model's text. But turning on cards for the flagship agent also turns on card types it can't fill, so ordinary turns now show broken "Invalid payload" boxes.

  • Every email thread read in the flagship now shows an "Invalid table payload" box. The flagship now receives the whole card map, including the email agent's get_thread → table entry. The flagship's own get_thread returns plain text with no table in it, so the TUI draws an error card where it used to show a normal step. Pass the flagship only the media entries, or tag a result as a card only when a card payload was actually built.
  • A media tool that reports success without a usable file shows an "Invalid media payload" error box. That covers a missing file, a relative path, or a deleted temp file. The same fix covers this: no card payload, no card tag.
  • The new open/reveal logic in the chat view has no tests. That covers finding the latest card, the SSH refusal, and the "no media yet" message. Neither bug above is tested on the path that actually streams results to the TUI.

Real-world evidence

N/A: no evidence bundle was produced for this fork PR, and the PR itself says no live generation was run. I ran go test ./internal/ui/cards ./internal/ui/chat locally and both pass. The verdict rests on static review. A real generate_image turn in the TUI showing the card and Ctrl+F opening the file, plus a flagship get_thread turn, would close the gap.

🔍 Technical details

🟡 Flagship now tags render for tools it can't build cards for (hub/agents/gaia/python/gaia_agent/server.py:1778, stdio.py:1468)
CanonicalTranslator._on_tool_result sets canonical["render"] from the tool name alone (sse_translation.py:441), whether or not result_data held a card. SSEOutputHandler._RENDER_TOOL_TO_LANG includes get_thread: "table" and pre_scan_inbox. The flagship's get_thread (src/gaia/agents/tools/email_tools.py:775) returns a JSON string with no {ok, data:{kind:"table"}} envelope, so _render_card_payload returns None. The translator still emits render: "table" with the summary dict as data, and canonical.go:215 calls upsertCard, which hits renderTable → renderInvalid("table", "columns is required…"). The same path covers media: _media_card_payload returning None (missing file, relative path, non-success status that isn't classified as a failure) still yields render: "media", which renderMedia turns into "Invalid media payload".
Two possible fixes, either one works:

  • Pass the flagship only the media subset: render_tool_map={k: v for k, v in SSEOutputHandler._RENDER_TOOL_TO_LANG.items() if v == "media"}.
  • More robust: in _on_tool_result, set render only when event.get("result_data") was present. That stops a tagged card with no payload on every agent.
    Add a translator/handler test for get_thread and for a missing-file media result that asserts no render key.

🟡 _on_tool_result now changes data for every tool (src/gaia/ui/sse_translation.py:426-430)
status/*_path keys are copied into every data object, including typed card payloads (file_list, search_results, email_pre_scan, table). Non-dict result_data is also stringified into {"summary": …}. Neither is needed for the media card, which already carries path. This widens the canonical tool_result.data contract for every client without a stated reason or a test. Drop it, or limit it to the summary-view branch (data is None).

🟡 No tests for tui/internal/ui/chat/media.go
latestMediaCard (stat check, relative-path rejection), the remoteSession refusal, and the mediaActionResultMsg → status line are untested. media_test.go only covers card rendering.

🟢 Nits

  • media.go:410 uses strings.Title, which is deprecated (SA1019 under staticcheck). Use strings.ToUpper(action[:1]) + action[1:] or fixed labels.
  • mediaStatus with no card prints could not open : no generated media card… (empty path, stray space). Skip the path when it's empty.
  • primitives.go:376: renderMedia was inserted between renderImage's doc comment and renderImage, so that comment now documents the wrong function. Move renderMedia below renderImage.
  • hints.go:138: latestMediaCard() runs os.Stat on every render of the hint bar. It's cheap, but consider caching the card when it arrives.

Strengths

  • The path never comes from model prose: only an allowlisted tool, status == "success", an absolute path and a regular file produce a card, and the TUI re-checks before opening.
  • The SSH refusal and the fixed argv per OS (no shell) for open/reveal are good choices.
  • The email sidecar's copy of the map was kept in sync, so the parity test still holds.

@huong8373tt-beep

Copy link
Copy Markdown
Contributor Author

Follow-up pushed in c32eb60c0: the flagship now publishes only verified media cards (not email/table mappings); canonical translation only sets render when a typed card payload exists; missing media output remains a normal tool result instead of an invalid card. Added regression coverage for verified/missing media paths, Ctrl+F/Ctrl+G selection, SSH refusal, and status wording. Local: 379 Python tests; Go chat/cards tests and vet pass; Black/isort/py_compile/diff check pass. CI has been re-triggered; no live generation claim.

@huong8373tt-beep

Copy link
Copy Markdown
Contributor Author

The only current red check is the Linux CLI integration job. I inspected its failed log: it stops before this PR's code/tests, while add-apt-repository ppa:lemonade-team/stable receives Launchpad HTTP 500 / GPGKeyTemporarilyNotFoundError. The workflow reports that as a named Launchpad outage. All TUI, Python, lint, build, installer, and Windows/macOS checks on the updated head are green; no code change is appropriate for this external PPA failure.

Slai Agent Runtime performed the investigation. The account owner authorized this contribution and is responsible for its content.

@huong8373tt-beep

Copy link
Copy Markdown
Contributor Author

Rechecked the updated head 8aa757f9842e17554988c94093db10585c0df2f2. The latest TUI/Python/Go checks are green; the only red check is Test GAIA CLI on Linux (Full Integration), whose log stops in the workflow while Launchpad returns HTTP 500 / GPGKeyTemporarilyNotFoundError for ppa:lemonade-team/stable. This is an external PPA outage, not a failure in this PR's code. The remaining updated-head checks are green, including TUI, unit, security, installer, and code quality. Slai Agent Runtime performed the investigation. The account owner authorized this contribution and is responsible for its content.

@huong8373tt-beep

Copy link
Copy Markdown
Contributor Author

CI follow-up: the remaining failed Test GAIA CLI on Linux (Full Integration) job did not reach this PR's code. Its Lemonade PPA setup failed because Launchpad returned HTTP 500 / GPGKeyTemporarilyNotFoundError while fetching the signing key. The job's own diagnosis identifies it as a Launchpad-side outage. All three platform Unit Test lanes, TUI build/test lanes, Code Quality, Agent/UI checks, and Security checks passed on 8aa757f9842e17554988c94093db10585c0df2f2. A rerun after Launchpad recovers is the appropriate next CI action; no speculative TUI patch was added. Slai Agent Runtime performed the investigation. The account owner authorized this contribution and is responsible for its content.

@kovtcharov-amd

Copy link
Copy Markdown
Collaborator

The card design is sound, and the invalid-card bugs from the first review are fixed. Three things remain before merge:

  • Only open known media files. Ctrl+F should launch nothing but the media types these tools produce. Check the extension against a short allowlist (for example .png .jpg .jpeg .webp .wav .mp3 .ogg .flac) in the TUI right before launching, and in the backend card check too. Add a test that a card with any other extension is refused.
  • The TUI guide doesn't mention the new keys. Add Ctrl+F (open) and Ctrl+G (reveal) to the keyboard table in the TUI guide, with the SSH limitation.
  • One earlier review point is still open: the translator now copies status and *_path into every tool result's data, and turns non-dict data into {"summary": ...}, for all tools, not just media. Limit that to the summary-view branch or drop it.
🔍 Technical details
  • TUI: tui/internal/ui/chat/media.go, in latestMediaCard or mediaAction. Check at launch time, because a card can outlive the file it was made for.
  • Backend: SSEOutputHandler._media_card_payload in src/gaia/ui/sse_handler.py.
  • Docs: docs/guides/tui.mdx keyboard table (around line 534, next to Ctrl+O).
  • Translator: src/gaia/ui/sse_translation.py _on_tool_result, the for key in ("status", "file_path", "image_path", "audio_path") loop and the {"summary": str(data)} coercion.
  • Non-blocking: Ctrl+F is also readline "cursor forward" in the input box. With no media card, each press now adds a "could not open" status line instead of moving the cursor. Consider handling the keys only when a media card exists.
  • The red Linux CLI integration job is a Launchpad PPA outage (GPGKeyTemporarilyNotFoundError) and is unrelated.

@huong8373tt-beep
huong8373tt-beep force-pushed the feat/tui-media-output-cards-6093 branch from 8aa757f to 9784ae7 Compare November 3, 2026 13:45
@github-actions github-actions Bot added the documentation Documentation changes label Nov 3, 2026
@huong8373tt-beep

Copy link
Copy Markdown
Contributor Author

Addressed the remaining review items in commit 9784ae790 and force-updated the PR branch after rebasing onto current main.

Changes:

  • Backend media-card validation now requires a known generated-media extension (.png, .jpg, .jpeg, .webp, .wav, .mp3, .ogg, .flac) in addition to an absolute, existing regular file.
  • TUI revalidates the same extension allowlist immediately before Ctrl+F/Ctrl+G actions; unknown extensions are refused.
  • Canonical SSE translation no longer copies status/*_path fields into every tool result or stringifies non-dict results; summary data remains limited to the existing summary branch.
  • Email-side canonical translator now emits render only when a typed payload is present, preventing plain get_thread results from becoming invalid table cards.
  • Added regression coverage for unknown extensions and mapped tools without typed card payloads.
  • Added the Ctrl+F/Ctrl+G keyboard documentation and SSH limitation.

Validation:

  • Python focused suites: 380 passed.
  • Email translator suite: 55 passed.
  • Go TUI cards/chat: go test ./internal/ui/cards ./internal/ui/chat passed.
  • Black, isort, py_compile, gofmt, and git diff --check passed.

The Linux CLI integration failure previously reported for this PR remains a Launchpad PPA outage, not a PR-code failure. No live media generation was claimed locally.

@huong8373tt-beep

Copy link
Copy Markdown
Contributor Author

Follow-up on the rebased head 9784ae790: the media/TUI-specific suites, TUI builds on Linux/macOS/Windows, code quality, Agent/UI, security, docs, and CLI integration all passed. The remaining red checks are the generic Python 3.12 Unit matrix on Linux, macOS, and Windows. I attempted to fetch their failure logs and rerun only failed jobs, but this fork token is not authorized for Actions log download/rerun; the public check annotations expose only the workflow exit code, not failed test names. I will not make speculative changes to the media-card implementation without the actual failures. Please rerun or expose the three Unit job logs when possible; I can address concrete failures promptly.

Slai Agent Runtime performed the investigation. The account owner authorized this contribution and is responsible for its content.

The terminal's new media card replaced the payload the Agent UI's inline
image/audio card reads, so a relayed flagship turn lost its player and the
unit suite failed on all three OSes. The card now carries the tool's own
path key, and the relay leaves the terminal-only render to the TUI.

- Restore the pathlib import main dropped, which the media check needs.
- Refuse UNC and device paths on the string alone, in the backend and the
  TUI, so neither stats nor opens a network share.
- Label image vs audio by the file's extension, not the result key.
- Build the open/reveal argv per OS as a pure function and test that the
  path is always one argument and no shell is involved.
- Test a deleted file is refused without a crash.
- CHANGELOG entry for the flagship's new media card.
@kovtcharov-amd

Copy link
Copy Markdown
Collaborator

Thanks @huong8373tt-beep, this is a nice feature. I merged current main into the branch and fixed what was keeping the unit tests red on all three OSes.

The red Unit Tests were caused by this PR: main had since added inline images and audio to the Agent UI (#6384), and the new media card replaced the data that feature reads, so a relayed flagship turn would have lost its player. The card now keeps the tool's own path, and the Agent UI keeps drawing its own card. I also made the TUI and the backend refuse network-share paths before touching them, and added tests that a deleted file is refused without a crash and that open/reveal never goes through a shell on Windows, macOS or Linux.

🔍 Technical details
  • _media_card_payload now carries status and the original image_path/audio_path/file_path, so generated_media.media_card still finds the path in a relayed data; relay._dispatch_one drops render: "media" so _chat_helpers applies media_card. New end-to-end test: sidecar handler → translator → relay → media_card.
  • Restored from pathlib import Path in sse_handler.py (main's fix(ui): cite chat attachments by their own name #6896 removed it; the merged tree would have raised NameError on every media result).
  • UNC/device paths are refused on the string alone, via the now-public generated_media.is_remote_or_device_path and a Go isRemoteOrDevicePath.
  • media_type comes from the extension, so a .png reported as file_path is no longer labelled audio.
  • Open/reveal argv lives in a pure mediaArgv(goos, action, path), tested for all three OSes.
  • CHANGELOG entry for the flagship's new media card.
  • Not changed: Ctrl+F still shows a "could not open" line when there's no media card, instead of moving the cursor (same pattern as Ctrl+B).
  • Verified: go build/vet/test ./... in tui/; SSE handler, translator, relay and generated-media suites; email translator and render-map sync tests; black/isort.

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

Labels

agent::email Email agent changes documentation Documentation changes tests Test changes tui Go terminal UI (gaia-tui)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(tui): open the images and audio the agent generates

2 participants