feat(web): fidelity closeout — detail skeleton, optimistic status, attachment rendering (HT-23)

[zaridan]

Summary

Implemented all three HT-23 fidelity-closeout items in the worktree /Users/tjbaker/Projects/helpthread-worktrees/feat-ht-23-fidelity-closeout (branch feat/ht-23-fidelity-closeout, commit e470500):

1. Conversation detail-view loading skeleton — new file web/src/app/(shell)/conversations/[id]/loading.tsx. Mirrors inbox/[folder]/loading.tsx's pattern (bare Skeleton + ToolbarBand primitives, no data) but shaped to the real conversation layout: toolbar band, subject-row placeholder, 3 message-band placeholders (avatar circle + line skeletons, one with the same-speaker hairline), and the 240px context-panel aside (panel-tone toolbar, avatar-circle skeleton, name/email lines, divider, one summary line).

2. Optimistic status changes with rollback — in web/src/components/ConversationScreen.tsx, changeStatus now mirrors updateTags/updateAssignee exactly: captures previous, sets localStatus immediately, awaits setStatusAction, and rolls back + toasts on failure (dropped the old post-then-refresh startTransition/router.refresh() shape to match). The four status MenuItem onClicks now call void changeStatus(...), matching the assignee menu's pattern. The reply-to-closed reopen was already correctly handled in sendReply's success branch (it flips status only once the send is confirmed — the send itself can't be optimistic — so no change was needed there; noted this explicitly rather than silently leaving it).

3. Minimal inbound-attachment rendering (flagged for the maintainer's visual sign-off — this is beyond the Claude Design prototype, per his explicit instruction to flag it prominently) — added AttachmentView and ThreadView.attachments?: AttachmentView[] to web/src/lib/api-types.ts, 1:1 with specs/api/agent-inbox-v1.md §2 (HT-46). Made attachments optional in the TS type (not just empty-array-default) because the task explicitly calls out that older API responses may omit the field entirely — the component guards on thread.attachments !== undefined && thread.attachments.length > 0. Added a humanFileSize() helper to web/src/lib/format.ts. Rendering is a plain pill-style list (background --ht-surface-2, radius 999 — same token composition as TagChip, no new ds component) with a paperclip icon, filename (or "Attachment" if null), and size, wrapped in an <a target="_blank" rel="noopener noreferrer"> using the signed URL exactly as given (no caching/rewriting). Only rendered on kind === 'inbound' message bands.

No new npm dependencies. web/src/components/ds/** was read but never edited.

Flagged for the maintainer's visual sign-off — the attachment rendering is an approved addition beyond the design prototype:

1. Status optimistic-update shape: removed the pre-existing startTransition/router.refresh() wrapper entirely rather than keeping it alongside optimism, because tags/assignee (the exact pattern to mirror) use neither — they rely on the server action's own revalidatePath and don't gate on isPending. Kept consistent rather than hybrid.

2. ThreadView.attachments made optional (attachments?: AttachmentView[]) rather than required-with-empty-array-default, even though the spec models it as always-present ([] default). Rationale: the task instructions explicitly say "If a thread has no attachments field (older API)... render nothing," which only makes sense if the type permits the field's absence — so I read "1:1 with spec §2" as shape-fidelity (same fields, same names) rather than requiring TS-level non-optionality that would fight the stated defensive-UI requirement.

3. Attachment styling: chose a TagChip-derived pill (surface-2 background, radius 999, ink/ink-dim/ink-muted text) as "the most conservative composition" the task asked to pick between (TagChip-like vs. plain list) — it reuses only existing color/radius tokens and no new component.

4. Did not modify sendReply's existing closed→active-on-success logic for item 2, since it already satisfies the "reply-to-closed reopen" requirement correctly (status flips only on confirmed send, which is the right semantics — the send itself isn't safe to represent optimistically before delivery is confirmed). Documented this explicitly rather than silently treating the requirement as already met without comment.

Review — 6 findings (4 actionable), fixes applied

Verification — gate exits: typecheck 0, lint 0, tests 0, web build 0, clean tree

link https://resonantiq.atlassian.net/browse/HT-23

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Conversation messages now display downloadable inbound attachments with filenames and readable file sizes.
  • Added a dedicated loading layout for conversations, including message, toolbar, and conversation-details placeholders.

• Bug Fixes

  • Improved conversation status updates with immediate feedback, error recovery, and protection against conflicting simultaneous changes.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 86f7289b-d700-4be3-bd3f-486c97e92dec

📥 Commits

Reviewing files that changed from the base of the PR and between b4284f3 and a963344.

📒 Files selected for processing (4)

• web/src/app/(shell)/conversations/[id]/loading.tsx
• web/src/components/ConversationScreen.tsx
• web/src/lib/api-types.ts
• web/src/lib/format.ts

---

📝 Walkthrough

Walkthrough

The conversation screen now renders inbound attachments, formats file sizes, serializes status updates with rollback handling, and adds a dedicated loading skeleton. Thread API types now require an attachments array.

Changes

Conversation UI updates

Layer / File(s)	Summary
Attachment contract and rendering web/src/lib/api-types.ts, web/src/lib/format.ts, web/src/components/ConversationScreen.tsx	Adds the required ThreadView.attachments contract, the humanFileSize formatter, and inbound attachment links with filenames and sizes.
Serialized status updates web/src/components/ConversationScreen.tsx	Guards concurrent status changes, applies optimistic updates, rolls back rejected or failed requests, and updates dropdown callbacks for asynchronous handling.
Conversation loading skeleton web/src/app/(shell)/conversations/[id]/loading.tsx	Adds reusable message skeleton rows and a composed conversation page loading layout with a details sidebar.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• Helpthread/helpthread#47: Adds the backend and specification support for ThreadView.attachments and signed BlobStore URLs.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main additions: detail skeleton, optimistic status updates, and attachment rendering.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/ht-23-fidelity-closeout

---

_{Comment @coderabbitai help to get the list of available commands.}