Render file previews via widget-pulled reads; make structuredContent …#536
Render file previews via widget-pulled reads; make structuredContent …#536edgarsskore wants to merge 3 commits into
Conversation
…widget-only The host may strip structuredContent from ui/notifications/tool-result and swallows the notification entirely for image results (it preempts delivery to inline-render the image), so previews rendered from notification payloads hung on "Preparing preview..." or fell back to a bare status row. The preview widget now renders every file type from its own read_file RPC call (origin:'ui'), which the host passes through untouched: - Reads pull at tool-input time (tool-result never arrives for images); mutations (write_file/edit_block, inferred from arg shape since the host doesn't send the tool name) pull at tool-result time, after the file changed. - Image bytes return as a base64 text block on ui reads - an image content block would make the host inline-render the RPC response and stall it. - structuredContent is returned only on origin:'ui' calls and is metadata-only (fileName/filePath/fileType/editor metadata/mimeType): no content duplication, no sourceTool (the widget stamps that client-side for telemetry). LLM-facing responses carry content[] only. - An 8s timeout bounds the RPC pull and a 10s watchdog replaces a stuck "Preparing preview..." with a failure row instead of spinning forever. - read_file directory listings drop the "use list_directory instead" hint on ui reads so it doesn't render as a notice in the directory preview. Tests updated to the new contract. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR makes file handlers and edit responses origin-aware for ChangesOrigin-aware structuredContent and preview pull
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/ui/file-preview/src/payload-utils.ts`:
- Around line 68-72: The PDF read payload currently drops the extracted page
text because payload building in payload-utils.ts only uses
extractToolText(value), which returns the first summary/header block. Update the
read path handling in buildRenderPayload so it preserves the later content[]
text blocks for PDF previews, either by concatenating the text blocks or
filtering out the summary header before passing text into the payload.
In `@test/test-markdown-preview.js`:
- Around line 525-540: The current unsupported-content preview test only covers
a single text block, so it misses the multi-block PDF path mentioned in
payload-utils.ts. Update testUnsupportedRawContentPreview to use a content[]
array with multiple blocks and verify extractRenderPayload still selects the
correct raw text while preserving the expected summary-first behavior. Keep the
assertions anchored on extractRenderPayload and payload.content so the test
covers the new contract, not just the simple happy path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 844c3a3b-e2cf-4aaa-9e29-fa2a5032f074
📒 Files selected for processing (7)
src/handlers/filesystem-handlers.tssrc/tools/edit.tssrc/ui/file-preview/src/app.tssrc/ui/file-preview/src/payload-utils.tstest/test-edit-block-line-endings.jstest/test-file-handlers.jstest/test-markdown-preview.js
A PDF ui-read returns a summary text block followed by per-page image/text blocks; assert extractRenderPayload selects the first non-empty text block, skipping whitespace-only and image blocks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
extractRenderPayload took only the first content[] text block, which for multi-block PDF reads is the "PDF file: ... (N pages)" summary — dropping all extracted page text from the preview (main carried the joined page text in structuredContent.content). Join all non-empty text blocks instead; single-block reads are unaffected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The preview widget now renders every file type from its own read_file RPC call (origin:'ui'), which the host passes through untouched:
Tests updated to the new contract.
Summary by CodeRabbit