Skip to content

feat(tui): unify image presentation across message origins - #130

Merged
danielkov merged 4 commits into
feat/compose-files-phase-3from
feat/compose-files-phase-4
Sep 8, 2026
Merged

feat(tui): unify image presentation across message origins#130
danielkov merged 4 commits into
feat/compose-files-phase-3from
feat/compose-files-phase-4

Conversation

@danielkov

@danielkov danielkov commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Unifies TUI image presentation for user attachments, native assistant-generated images, typed tool results, and parsed Markdown image nodes. Shares asynchronous decoding, terminal protocols, resource limits, and live/replay behavior without attaching displayed media to model context.

Stacked on #129; this PR intentionally targets feat/compose-files-phase-3.

Impact

Assistant-generated images now retain pixels in the transcript instead of becoming text-only placeholders. Authorized Markdown images load only when visible, with alt/source fallbacks, stable viewports, and message-local content deduplication that preserves repeated Markdown occurrences.

Technical details

  • Managed URLs require current-session authority. Local paths follow OS permissions and bounded regular-file reads; external references are temporary presentation snapshots, not managed imports.
  • Remote loading is disabled by default. KIT_TUI_IMAGE_ORIGINS explicitly authorizes exact HTTPS origins; every DNS address must be public and is pinned while TLS hostname verification remains active. Redirects, ambient proxies, cookies, and credential-bearing URLs are excluded.
  • Source acquisition and decoding use independently bounded worker admission. Cancellation stops stale async stages while actual blocking work retains its admission permits. Source-specific invalidation avoids rebuilding unrelated Markdown history.
  • Adds the audited, feature-minimal pulldown-cmark =0.13.0 parser and its sole new transitive dependency, unicase =2.9.0; no other dependency versions change.

@danielkov

Copy link
Copy Markdown
Contributor Author

Local verification for 8fa7932932d6cc2a87a04fe61cf468e2bb2472f5:

  • cargo fmt --check, release-version script tests, and cargo clippy --locked --all-targets --all-features -- -D warnings passed.
  • cargo test --locked passed: 1,250 library tests plus binary/integration suites; 8 opt-in tests ignored. One prior run hit the existing filesystem lease test's WouldBlock failure; both its isolated rerun and the subsequent complete suite passed.
  • Explicit opt-in HTTPS smoke passed against https://httpbin.org: authorized image acquisition and redirect rejection through the production resolver.
  • Seven Swift generator tests and generated ACP drift check passed.
  • Independent exact-SHA review approved this head after resolving cancellation, occurrence-admission, and targeted-invalidation findings. Configured GitHub review and CI are running separately.

Coverage includes typed user/tool/assistant live and replay transport, completed versus partial Markdown nodes, managed session authority, local permissions/symlinks/FIFOs, rejected remote origins/nonpublic addresses, response limits/redirects, stale-task cancellation, corrupt formats, pixel/source budgets, cache/resize/fallback, content-exact deduplication, and text-only search/copy behavior.

Presentation evidence uses real TestBackend rendering and the shared halfblock protocol, including visible colored image cells. No physical Kitty/iTerm2/Sixel terminal demo was recorded; protocol behavior beyond the exercised backend remains dependent on terminal support. Mutable external references are intentionally re-resolved after reset/replay, and blocking OS/codec work cannot be forcibly interrupted even though admission remains bounded.

This stack remains unmerged. Base is feat/compose-files-phase-3 at eeedaffc740148c0cad0a361c0e5b2b82a6f4517 (#129).

@kit-code-agent kit-code-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found. The changes look good to merge.

@kit-code-agent kit-code-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Two image-loading correctness issues remain: URL-escaped local Markdown destinations are interpreted as literal filenames, and aggregate source-cache pressure leaves otherwise valid images negatively cached instead of reclaiming older snapshots.

Comment thread src/tui/image_source.rs Outdated
Comment thread src/tui/markdown_images.rs Outdated

@kit-code-agent kit-code-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Byte-pressure eviction can repeatedly reacquire still-visible Markdown images when their combined snapshots exceed the source-cache budget, causing repeated I/O, decoding, and loading-placeholder churn without user interaction.

Comment thread src/tui/markdown_images.rs

@kit-code-agent kit-code-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A simultaneously visible image set exceeding the source-cache byte budget causes continuous eviction and reacquisition, including repeated downloads for authorized HTTPS sources.

Comment thread src/tui/markdown_images.rs

@kit-code-agent kit-code-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found. The changes look good to merge.

kit-code-agent[bot]
kit-code-agent Bot previously approved these changes Sep 8, 2026

@kit-code-agent kit-code-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found. The changes look good to merge.

@danielkov
danielkov force-pushed the feat/compose-files-phase-3 branch from eeedaff to d84baa2 Compare September 8, 2026 17:10
@danielkov
danielkov force-pushed the feat/compose-files-phase-4 branch from b3723c8 to 4998666 Compare September 8, 2026 17:10

@kit-code-agent kit-code-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found. The changes look good to merge.

auto-merge was automatically disabled September 8, 2026 17:18

Pull Request is not mergeable

auto-merge was automatically disabled September 8, 2026 17:21

Pull Request is not mergeable

auto-merge was automatically disabled September 8, 2026 17:28

Pull Request is not mergeable

auto-merge was automatically disabled September 8, 2026 17:31

Pull Request is not mergeable

auto-merge was automatically disabled September 8, 2026 17:33

Pull Request is not mergeable

auto-merge was automatically disabled September 8, 2026 17:33

Pull Request is not mergeable

auto-merge was automatically disabled September 8, 2026 17:35

Pull Request is not mergeable

auto-merge was automatically disabled September 8, 2026 17:36

Pull Request is not mergeable

auto-merge was automatically disabled September 8, 2026 17:36

Pull Request is not mergeable

auto-merge was automatically disabled September 8, 2026 17:38

Pull Request is not mergeable

auto-merge was automatically disabled September 8, 2026 17:39

Pull Request is not mergeable

auto-merge was automatically disabled September 8, 2026 17:40

Pull Request is not mergeable

auto-merge was automatically disabled September 8, 2026 17:41

Pull Request is not mergeable

auto-merge was automatically disabled September 8, 2026 17:44

Pull Request is not mergeable

@danielkov
danielkov merged commit e900f95 into feat/compose-files-phase-3 Sep 8, 2026
9 checks passed
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