feat(rendering): render richlink content type#2
Conversation
Companion to photon-hq/spectrum-ts#42. Adds Url / Title / Summary fields to protocol.Content and case branches in the four renderer switches (messagelog.go main + quoteBody, select.go reply-quote, plain.go). Cover field deferred — kitty-protocol image rendering is a separate design call. Without these renderer cases, richlink messages round-trip cleanly through the wire layer (SendResult{id, timestamp}, no -32099) but display as blank in the TUI. Tracks photon-hq#1.
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR introduces support for a new "richlink" content type across the protocol, plain-text rendering, and UI layers. It extends the ChangesRichlink Content Type Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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. Review rate limit: 0/1 reviews remaining, refill in 34 minutes and 38 seconds.Comment |
Adds richlink to the public Content union spec. The wire shape mirrors what spectrum-ts emits: required url, optional title/summary, optional cover with mimeType + base64 bytes. Renderer support for cover is implementation-defined and may be deferred (this PR defers cover rendering — see PR description). Closes documentation gap noted during PR photon-hq#2 review.
Removes the 'spike: explicit modeling vs Raw passthrough TBD' inline comment. Design tradeoffs belong in the PR discussion, not in the struct. Field names and JSON tags are self-documenting; PROTOCOL.md is the canonical wire reference.
Adds the first test file in the repo: a stdlib table-driven test for
plainFormat's richlink branch. Covers:
- url only -> '[link] {url}'
- title without summary -> '[link] {title}'
- title with summary -> '[link] {title} — {summary}'
- summary without title falls back to url label
- missing url and title renders '[link] ' (cosmetic edge case)
Stdlib only, no third-party deps. Same-package test so plainFormat
stays unexported.
|
Iteration since initial draft (3 atomic commits on top of the spike):
PR description updated to reflect current state. Still DRAFT pending direction on Pattern A vs B and cover rendering. |
|
Re-ran E2E against spectrum-ts#42 — patched binary renders |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/ui/entry_preview.go`:
- Around line 40-44: The current CacheKey for the HoveredPreview uses
e.Content.Name which is not unique; change the CacheKey in the return of the
function creating &store.HoveredPreview to a stable unique identifier (for
example use e.AttachmentPath or a combination like e.AttachmentPath + ":" +
e.Content.Name or a content ID if available) so previews for different files
with the same filename do not collide when dispatchClick compares
preview.CacheKey; update the CacheKey field only (leave Name/Path as-is) and
ensure any downstream comparisons still use the new unique key.
- Around line 21-32: entrySupportsPreview currently advertises previews for
richlink covers based only on non-empty Bytes and MIME type, but previewForEntry
may reject malformed base64; update entrySupportsPreview (function
entrySupportsPreview, type store.LogEntry) to validate the cover payload by
attempting to base64-decode e.Content.Cover.Bytes and ensuring the decode
succeeds and yields non-zero-length data before returning true (in addition to
checking kitty.SupportedMimeType on e.Content.Cover.MimeType); this keeps the UI
affordance consistent with previewForEntry's behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c10bd0f5-5ee7-4849-93cd-176187ce0b3e
📒 Files selected for processing (11)
PROTOCOL.mdinternal/plain/plain.gointernal/plain/plain_test.gointernal/protocol/types.gointernal/protocol/types_test.gointernal/ui/entry_preview.gointernal/ui/entry_preview_test.gointernal/ui/messagelog.gointernal/ui/mouse.gointernal/ui/select.gointernal/ui/view.go
📜 Review details
🔇 Additional comments (11)
internal/protocol/types.go (1)
23-31: Richlink payload fields are wired in cleanly.The new
Url/Title/Summaryfields and nestedCovertype line up with the protocol contract without introducing any obvious schema mismatches.internal/protocol/types_test.go (1)
8-30: Good coverage for the nested cover shape.This test verifies the new
richlink.coverunmarshalling path and locks in themimeType/bytesmapping added by the protocol change.internal/plain/plain.go (1)
159-167: Richlink plain-mode formatting looks correct.The new branch keeps the output predictable by preferring
Title, falling back toUrl, and appendingSummaryonly when present.internal/ui/view.go (1)
313-314: Preview zone gating now follows the shared predicate.Using
entrySupportsPreview(e)here keeps the view in sync with the richer preview rules, including the new richlink cover path.internal/ui/select.go (1)
95-100: Richlink reply quoting is wired up consistently.The new branch uses the same label fallback as the rest of the rendering code, so reply banners won’t drop richlink targets on the floor.
internal/plain/plain_test.go (1)
9-48: Nice, focused coverage for the new richlink formatting.The table-driven cases exercise the fallback and summary combinations that matter for the new plain-mode branch.
internal/ui/messagelog.go (2)
53-82: Render richlinks and preview hints through the shared predicate.This keeps the attachment hinting logic and the new richlink branch aligned with the shared preview support instead of duplicating MIME checks.
203-221: Richlink reply quotes stay consistent with the new label helper.The
quoteBodybranch now emits the same link label shape used elsewhere, so reply quoting remains predictable for richlink entries.PROTOCOL.md (1)
55-63: Protocol docs now match the new richlink wire shape.The added
richlinkunion member and nestedcoverobject line up with the implementation and make the contract clear for adapters.internal/ui/mouse.go (1)
188-203: Shared preview plumbing looks good.Routing attachment/preview clicks through
previewForEntrykeeps the hover-preview behavior centralized and removes the old MIME-specific branching from mouse handling.internal/ui/entry_preview_test.go (1)
12-85: Good coverage for the new richlink preview path.The happy-path and rejection cases cover the important MIME/base64 branches, which should make regressions in
previewForEntryeasier to catch.
1 participant
Status: ready for review — companion to photon-hq/spectrum-ts#42.
Tracks #1: makes tuichat render the
richlinkcontent type that spectrum-ts#42 sends over the terminal protocol. Without this, a richlink message round-trips cleanly through the wire (no-32099, returnsSendResult{id, timestamp}) butmainrenders a blank line.What changed
PROTOCOL.mdrichlinkin the public Content union specinternal/protocol/types.goUrl/Title/Summary/CovertoContentinternal/ui/messagelog.gointernal/ui/entry_preview.gointernal/ui/mouse.go/internal/ui/view.gointernal/ui/select.gointernal/plain/plain.goDesign question
Pattern A (this PR) vs Pattern B (Raw passthrough)?
This PR uses Pattern A: explicit richlink fields on
Content. I considered Pattern B (richlink fields ride underc.Raw, parsed in renderer) because it aligns with the existing comment atinternal/protocol/types.go:20-22:Happy to switch to Pattern B if you prefer. For the current spectrum-ts#42 wire shape, Pattern A is the direct match:
coverstays nested under the richlink payload and no spectrum-ts-side change is needed.Cover preview
Implemented after Ryan's question on spectrum-ts#42. If a richlink has
cover.bytesand a supported image MIME type, tuichat now shows the same(click to preview)affordance as attachments and opens the existing floating preview panel. This reuses the current Kitty/Ghostty image path and mosaic fallback; no new hover system or protocol shape is introduced.Non-TTY/plain mode remains text-only:
[link] Title — Summary.Verification
go build ./...cleango vet ./...cleango test ./... -racecleaninternal/plain: 5 richlink formatting cases passinternal/protocol: nestedcover.{mimeType, bytes}unmarshal test passesinternal/ui: richlink cover preview decode/reject tests passSendResult, no-32099, stdout renders[link] E2E Test Article — Round-trip verification...mainreturned message id but rendered blank, confirming the rendering bug is realCaveats
cover.bytesand supported image MIME; unsupported/malformed cover payloads still render the text richlinkCross-link: photon-hq/spectrum-ts#42 (wire-side companion), #1 (issue tracking this work)
Summary by CodeRabbit
New Features
Tests