Skip to content

feat(tui): show user pasted content#2784

Merged
dgageot merged 2 commits into
docker:mainfrom
joshbarrington:feature/show-user-paste-content
May 13, 2026
Merged

feat(tui): show user pasted content#2784
dgageot merged 2 commits into
docker:mainfrom
joshbarrington:feature/show-user-paste-content

Conversation

@joshbarrington
Copy link
Copy Markdown
Contributor

@joshbarrington joshbarrington commented May 13, 2026

Summary

This PR enhances the TUI to render pasted content back to the user - automatically collapsing large pasted file contents (exceeding 30 lines) into a toggleable view, improving overall chat readability and scrolling performance.

Key Changes

  • Show User Pasted Content: Currently pasting multiple lines into docker agent TUI renders as @paste-1 for example. This shows the pasted content back to the user to allow for better readability.
  • Collapsible User Messages: Added expand/collapse functionality to user messages exceeding 30 lines (rendered via messages.go and message.go).
  • Interactive Toggles: Implemented a generic toggleableView interface in messages.go to handle click-to-toggle interactions on message views (now supporting both reasoning blocks and long user messages).
  • Robust File Replacements: Modified editor.go to sort paste attachments by length descending before inline string replacement (@paste-1 vs @paste-10), preventing partial match corruption, while fully retaining proper order of standard metadata file attachments.
  • Performance Enhancements: Prevented memory allocation spikes by utilizing zero-allocation string ops (strings.Count and strings.SplitN) instead of heavy array allocations (strings.Split) during truncation calculations.
  • Coverage: Added comprehensive TestUserMessageCollapsible and TestUserMessageNotCollapsible unit tests to secure boundary math and UI layout mechanics.

@joshbarrington joshbarrington requested a review from a team as a code owner May 13, 2026 12:02
@rumpl
Copy link
Copy Markdown
Member

rumpl commented May 13, 2026

@joshbarrington thanks! Could you show me some screenshots? I won't have the time to check out your branch

@joshbarrington
Copy link
Copy Markdown
Contributor Author

joshbarrington commented May 13, 2026

@rumpl

Usual user pasted content

Screenshot From 2026-05-13 14-08-49

Long user pasted content (> 30 lines) with expand option

Screenshot From 2026-05-13 14-11-18

Expanded content with collapsible option

Screenshot From 2026-05-13 14-11-33

@joshbarrington
Copy link
Copy Markdown
Contributor Author

Current behaviour on main:

Screenshot From 2026-05-13 14-17-27

@dgageot
Copy link
Copy Markdown
Member

dgageot commented May 13, 2026

/review

Copy link
Copy Markdown

@docker-agent docker-agent left a comment

Choose a reason for hiding this comment

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

Assessment: 🟡 NEEDS ATTENTION

This PR renders pasted content in the TUI with collapsible messages for large pastes. The implementation is solid overall — the sort-by-length trick for placeholder disambiguation is clever, the cache invalidation on toggle is correct, and the toggleableView interface dispatch is sound.

One medium-severity issue was found in the paste expansion loop.

})

for _, att := range pastes {
content = strings.ReplaceAll(content, "@"+att.Name, att.Content)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[MEDIUM] Paste content expansion can corrupt messages if pasted content contains @paste-N strings

The loop in resetAndSend replaces each @paste-N placeholder with the paste's content using strings.ReplaceAll. Sorting by name-length descending correctly prevents @paste-1 from partially matching @paste-10 in the original input — but it doesn't guard against a later paste's ReplaceAll matching an @paste-N string that was introduced by a previous paste's content.

Trigger scenario:

  1. User pastes two items: paste-1 and paste-2
  2. The content of paste-1 happens to contain the literal string @paste-2 (e.g., a shell script, documentation, or any text referencing another paste)
  3. After paste-1 is expanded (longer name goes first), content now contains @paste-2 embedded in the replacement text
  4. When the loop reaches paste-2, strings.ReplaceAll finds that injected @paste-2 and replaces it with paste-2's content — double-expanding and corrupting the message

Suggested fix: Replace only the first occurrence in the original content position, or process on a copy of the original content string (tracking expansions separately), or escape @paste-N references in the source string before looping:

// Option: replace on a snapshot so injected tokens aren't re-processed
original := content
for _, att := range pastes {
    content = strings.ReplaceAll(original, "@"+att.Name, att.Content)
    original = content // or accumulate replacements on original only
}

A cleaner approach is to do all replacements against the original content value and accumulate them without re-scanning previously expanded text.

@dgageot dgageot merged commit f566471 into docker:main May 13, 2026
7 of 8 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.

4 participants