Conversation
Document multi-thread chat (Star/Rename/Delete) and file attachments (images and PDFs, up to 5 per message at 10 MB each), as shipped in toolhive-studio v0.30.0. Refs #773.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Updates the ToolHive UI playground guide to cover newly added v0.30.0 features for managing threaded conversations and attaching files, addressing the documentation gap reported in #773.
Changes:
- Adds “Threaded conversations” and “Attachments” to the playground’s key capabilities.
- Documents thread management actions (new, rename, star/unstar, delete with confirmation) and file attachment flows (drag-and-drop, toolbar, limits, and UI behaviors).
- Adds recommended “Thread and attachment hygiene” practices.
| - **PDFs and other non-image files** show as `📎 <FILE_NAME>` with a | ||
| **Download** link so you can save the original file. |
danbarr
left a comment
danbarr
left a comment
There was a problem hiding this comment.
Useful coverage of the threaded chat and attachments. The main issues are content duplication across the new subsections, plus the same code-vs-bold UI string formatting issue from PR 851.
Primary issues
1. Same content stated in two or three places
The PR introduces parallel subsections that repeat the same facts:
- Attachments: 5 files / 10 MB / images-and-PDFs limits appear in (a) Key capabilities > Attachments (lines 47-51), (b) Getting started step 6 (lines 119-123), and (c) Using the playground > Attach files to a message (lines 230-232). Three statements of the same constraint.
- Threads: thread sidebar behavior is summarized in Key capabilities > Threaded conversations, walked through in Getting started step 5, and walked through again in Using the playground > Manage playground threads with overlapping bullet content.
Readers walking the page top-to-bottom will hit the same content three times. The "Manage chat threads" / "Manage playground threads" headings even use slightly different names for the same thing.
Ask: Pick one section as the canonical reference (the detailed subsections under Using the playground are the natural fit) and reduce the others to short pointers: a one-sentence callout under Key capabilities and a "see Manage playground threads" / "see Attach files to a message" pointer in the Getting started steps. That keeps the journey clean without losing discoverability.
2. Concur with Copilot: "PDFs and other non-image files" is internally inconsistent
Line 249 says PDFs and other non-image files, but the same section states "supports image files and PDFs" (line 226) and the rejection toast says Only images and PDFs are supported (line 245). Only PDFs and images are supported.
Ask: Replace with just "PDFs."
3. Inconsistent code-vs-bold for UI strings
Same pattern as PR 851. Short UI labels are correctly bold (New chat, Thread options, Star, Add images or PDFs), but longer placeholder text, toast messages, and confirmation copy are wrapped in inline code: Delete "<THREAD_NAME>"? This cannot be undone., Sent with attachments, Select an AI model to get started, Type your message..., Double-click to rename, plus the rejection toast messages. The project style guide reserves inline code for code elements; UI strings should be bold or, for multi-sentence alert text, blockquoted (the pattern PR 851's drift section uses).
Ask: Bold the short labels, blockquote the longer dialog/toast text, keep code only for actual code (no examples in this PR's additions).
Secondary issues
| Issue | Recommendation |
|---|---|
| The new Threaded conversations and Attachments entries under Key capabilities sit next to genuine differentiators (Instant testing, Detailed logs, ToolHive management); they're useful UI features but not headline capabilities | If you keep them at all after collapsing the duplication, consider whether they belong here or only in Using the playground. |
| "remove old runs when you're done" (line 44): "runs" is unexpected here since the items are chat threads, not test executions | Use "threads" consistently. |
Placeholder timestamps Nm ago, Nh ago, Nd ago (line 220) read as literal UI text |
Use realistic examples: 5m ago, 2h ago, 3d ago. |
| "Treat attachments as inputs that may leak data" (line 297): the actual risk is sending sensitive content to the AI provider, not the playground leaking data on its own | Reframe, e.g., "Attachments are sent to your AI provider. Strip credentials, customer information, and other sensitive content from PDFs and screenshots before sharing them." |
Re: Copilot's comment
"PDFs and other non-image files" inconsistency (line 249): Concur. Captured as primary issue 2.
Address danbarr's CHANGES_REQUESTED review on #853: dedupe threads and attachments across sections, fix PDF-only wording, replace placeholder timestamps with concrete examples, reframe attachment risk around the AI provider, and align UI string formatting. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Thanks for the review. Pushed updates as
A couple of judgment calls:
|
danbarr
left a comment
danbarr
left a comment
There was a problem hiding this comment.
Most of the previous review is addressed cleanly: duplication collapsed (Key capabilities and Getting started are now one-line pointers to the canonical subsections), "PDFs and other non-image files" replaced with "PDFs", UI strings moved into blockquotes, realistic timestamps in place of Nm ago / Nh ago / Nd ago, and the attachments-security framing rewritten as suggested. Two small follow-ups remaining:
1. Composer placeholder section reads as broken prose
The new structure interleaves blockquotes with sentence fragments that don't connect to surrounding sentences:
The composer placeholder reflects the playground state:
Select an AI model to get started
is shown when no model is selected, then:
Type your message...
once you've chosen one.
"is shown when no model is selected" and "once you've chosen one" read as orphaned verb fragments. Ask: Restructure, e.g., as a short bullet list mapping state to placeholder:
The composer placeholder reflects the playground state:
- Before you select a model:
> Select an AI model to get started
- After you select a model:
> Type your message...2. Bullet label/body mismatch in Manage playground threads
The fourth bullet is labeled Delete or unstar threads: but the body only describes the Delete action. Star/Unstar is already covered in the bullet above it.
Ask: Change the label to Delete a thread:.
Description
Closes #773.
Summary
playground.mdxfor v0.30.0: document the threaded chat sidebar (New chat, Starred / Recents, double-click rename, Star / Unstar, Delete with confirmation copy) and file attachments (Add images or PDFs, drag-and-drop, 5-file / 10 MB limits, supported types, attachment-only sends, image modal,📎 <FILE_NAME>plus Download for non-image files).stacklok/toolhive-studioat tagv0.30.0. No new screenshots added (none currently exist understatic/img/).Test plan
npm run buildpassesType of change
Related issues/PRs
Closes #773
Submitter checklist
Content and formatting
Made with Cursor