feat(sampling): replace InlineSamplingRequest responseText with draftResult

[cliffhall]

Closes #1239.

Summary

• InlineSamplingRequest now takes draftResult?: CreateMessageResult instead of responseText: string, matching SamplingRequestPanel's prop shape so a future shared-draft store can feed both views.
• No-draft state: removed the readonly placeholder Textarea. The card renders request preview + model hints + action buttons, identical to before minus the empty textarea slot. Buttons (onAutoRespond, onEditAndSend, onReject, onViewDetails) — and their callback shapes — are unchanged.
• Text draft: 2-line preview of draftResult.content.text via Mantine's lineClamp.
• Image / audio draft: [Image content] / [Audio content] label, matching the ContentViewer fallback convention.
• Stories: replaced PrefilledResponse with WithTextDraft and added WithNonTextDraft covering the image branch. Existing Default, WithModelHints, InQueue stories now exercise the no-draft state. Updated the three consumer stories (PendingClientRequests × 2, ToolsScreen) that threaded responseText="" through.

Open design question (deferred)

The audit asked whether the inline preview should mirror the panel's live draft (shared state) or stay a static snapshot. That's a parent-wiring concern — this PR only changes the prop shape. The component is read-only from this layer's perspective; whoever wires the pending-request store in #1243 / #1244 picks the answer.

Scope notes

• Removed the readonly Textarea rather than keeping it as an empty placeholder. The acceptance criteria call out the four action buttons as "unchanged" but don't list the textarea — and a permanent empty textarea felt redundant with the action buttons already signaling the affordance. Happy to revisit if reviewers prefer a placeholder slot in the no-draft state.
• git grep responseText shows only HistoryListPanel.tsx, where it's a local variable for entry-search matching — unrelated.

Test plan

• npm run validate passes.
• npx vitest run --project=storybook src/components/groups/InlineSamplingRequest src/components/groups/PendingClientRequests src/components/screens/ToolsScreen — 17/17 stories pass.
• Visual diff in Storybook: no-draft state matches existing card minus the empty Textarea; text-draft and non-text-draft variants render their expected previews.

🤖 Generated with Claude Code

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 1m 54s —— View job

---

PR Review: feat(sampling): replace InlineSamplingRequest responseText with draftResult

• Gather context and read changed files
• Analyze diff and component logic
• Review stories and consumer changes
• Post final review

---

Overall this is a clean, well-scoped change. The prop rename aligns InlineSamplingRequest with SamplingRequestPanel's shape (both now use CreateMessageResult), and the conditional draft preview is correctly wired. A few observations below:

Minor: DraftPreview and PreviewText are near-duplicates

clients/web/src/components/groups/InlineSamplingRequest/InlineSamplingRequest.tsx:27-36

const PreviewText = Text.withProps({ size: "sm", c: "dimmed", lineClamp: 2 });
const DraftPreview = Text.withProps({ size: "sm", lineClamp: 2 });

The only difference is c: "dimmed" on PreviewText. These could be collapsed to one component with the color toggled via a prop (e.g. variant), or DraftPreview could just be PreviewText.withProps({ c: undefined }). Minor DRY nit — the semantic naming is clear, so this is not a blocker.

Observation: extractDraftPreview vs extractPreview inconsistency in fallback strategy

InlineSamplingRequest.tsx:59-78

// extractPreview — dynamic fallback
return content.type === "text" ? content.text : `[${content.type}]`;

// extractDraftPreview — hardcoded per-type
case "image": return "[Image content]";
case "audio": return "[Audio content]";

extractPreview uses a dynamic [${content.type}] template that auto-handles future content types, while extractDraftPreview hardcodes per-type labels. Both approaches work: the explicit : string return type annotation on extractDraftPreview (line 70) means TypeScript will catch any uncovered variants if the SDK adds new content types. The hardcoded strings do read better ("Image content" vs "image"), so this is a style choice — just worth noting the inconsistency in case someone adds a third content type and only updates one function.

Fix this →

Minor: No visual differentiation between request preview and draft preview

In the rendered card the layout is:

1. Badge + queue label
2. Model hints (if any)
3. PreviewText — the request message, dimmed
4. "View Details" button
5. DraftPreview — the draft, default color
6. Action buttons

The "View Details" button creates a natural break, and the color difference (dimmed vs. normal) provides some distinction. But given this is a read-only inline view, it might be worth adding a small label like "Draft:" as a c="dimmed" prefix text before DraftPreview so the card is unambiguous when both sections are populated. This is deferred design work per the PR description, flagging for #1243/#1244.

Story fixtures use a real model ID

InlineSamplingRequest.stories.tsx:23

model: "claude-sonnet-4-20250514",

Functional for stories, but a placeholder like "test-model" is typically cleaner in fixtures to avoid looking like SDK documentation. Not a blocker.

What's working well

• Prop alignment with SamplingRequestPanel (draftResult?: CreateMessageResult) is the right shape for the upcoming shared-draft store work.
• The extractDraftPreview switch on draft.content.type is clean and TypeScript will enforce exhaustiveness via the : string return annotation.
• Consumer cleanups (PendingClientRequests, ToolsScreen) are minimal and correct — just removing the deleted prop.
• Story coverage is good: WithTextDraft and WithNonTextDraft cover both branches, and existing stories (Default, WithModelHints, InQueue) naturally exercise the no-draft state.
• Removing the read-only Textarea placeholder is the right call — an empty Textarea with no interaction added visual noise without communicating anything new.

---

Verdict: Approve with the above as optional follow-ups. No blocking issues.
|

[cliffhall]

Thanks for the review. Walking through the four points:

DraftPreview vs PreviewText near-duplicates — keeping as-is. The two constants carry different semantic roles that happen to share most of their styling: PreviewText is the dimmed request-message excerpt (less-important contextual breadcrumb), DraftPreview is the actual response content the user is about to act on (foreground emphasis, normal color). Collapsing them into one variant-toggled component would lose that signaling at the call site. Happy to revisit if a third near-duplicate appears.

extractPreview vs extractDraftPreview fallback strategy — keeping extractDraftPreview as-is. The verbatim labels ([Image content] / [Audio content]) come straight from the issue's acceptance criteria, which call out the ContentViewer convention. The extractPreview dynamic template predates this PR, and changing either one to match the other is out of scope for the prop-shape swap. The exhaustive switch + : string return type already gives us a compile-time guard if the SDK adds a new variant. Worth a small follow-up to align both sides if the inconsistency continues to bother — but I'd want to align on the verbose form (clearer end-user text) rather than the terse one.

"Draft:" label — agreed it would help disambiguate when both previews are populated. Punting to the parent-wiring PRs (#1243 / #1244) as you suggest, since the label only matters once a real draft store can produce content.

Story fixture model ID (claude-sonnet-4-20250514) — keeping. It matches the established convention in this codebase: ToolsScreen.stories.tsx uses the same string in WithSamplingModal, WithPendingRequests, etc. Switching this story to a placeholder would create a new inconsistency rather than removing one. If we want to move all story fixtures to placeholders, that's a separate sweep.

Filed #1251 as a follow-up: the test plan reference to npx vitest run --project=storybook ... in this PR pointed at a gap — there's no test:storybook script and no CI gate on play-function tests, which is how #1248 landed silently. The new issue covers the script, the CI step, and notes #1248 as a precondition (since you can't turn the gate on while v2/main has a known red play function).

No code changes in this PR; ready to merge if you're happy.