vscode: inline composer for preview-pane review comments (#1107)

[amrmelsayed]

PIR Review: Inline composer for preview-pane review comments

Fixes #1107

Summary

Adding a review comment from the Codev Markdown Preview previously opened
vscode.window.showInputBox — a center-top Quick Pick far from the block the
reviewer clicked + on, breaking the visual anchor while typing. This replaces it
with an inline composer rendered in-flow directly below the block (a textarea
with Submit/Cancel), so the reviewer types the comment exactly where it will live.
The comment-intent seam widened from onAddComment(line) to
onAddComment(line, text); the host dropped its text prompt and writes the marker
straight from the body the composer collected. The on-disk REVIEW-marker format is
unchanged (multi-line input collapses to single-line at write time, as before).

Files Changed

• packages/artifact-canvas/src/overlays/CommentComposer.tsx (+88 / −0) — new composer component
• packages/artifact-canvas/src/overlays/__tests__/comment-composer.test.tsx (+83 / −0) — new unit tests
• packages/artifact-canvas/src/components/ArtifactCanvas.tsx (+110 / −24) — composer state, in-flow placeholder injection, portal wiring; JSX conversion
• packages/artifact-canvas/src/components/__tests__/artifact-canvas.test.tsx (+80 / −15) — open-composer→submit flow, Esc/reload coverage
• packages/artifact-canvas/src/__tests__/end-to-end.test.tsx (+20 / −14) — drive round-trip through the composer
• packages/artifact-canvas/src/types.ts (+8 / −4) — onAddComment(line, text) contract amendment
• packages/artifact-canvas/src/overlays/CommentAffordance.tsx (+11 / −9) — JSX conversion
• packages/artifact-canvas/src/overlays/MarkerMinimap.tsx (+21 / −17) — JSX conversion
• packages/artifact-canvas/src/renderer/MarkdownView.tsx (+7 / −3) — JSX conversion
• packages/artifact-canvas/src/styles/default-theme.css (+58 / −0) — composer styling
• packages/artifact-canvas/examples/main.tsx (+4 / −4) — example uses the new seam
• packages/core/src/review-markers.ts (+19 / −0) — new markerAppendLine codec helper (append-below-thread)
• packages/core/src/__tests__/review-markers.test.ts (+27 / −0) — markerAppendLine regression tests
• packages/vscode/src/markdown-preview/messages.ts (+24 / −0) — new named webview↔host protocol types
• packages/vscode/src/markdown-preview/preview-provider.ts (+22 / −15) — drop showInputBox; write from posted text; use protocol types
• packages/vscode/src/markdown-preview/webview/main.ts (+9 / −6) — post {line, text}; use protocol types
• codev/plans/1107-preview-inline-comment-composer.md, codev/reviews/1107-preview-inline-comment-composer.md, codev/state/pir-1107_thread.md — PIR artifacts
• codev/resources/arch.md, codev/resources/lessons-learned.md — governance updates (below)

Commits

• 87a6788d [PIR vscode: replace top-of-window Quick Pick input for adding review comments from the markdown preview with an inline composer #1107] artifact-canvas: inline comment composer; widen onAddComment seam to (line, text)
• 1f5e0614 [PIR vscode: replace top-of-window Quick Pick input for adding review comments from the markdown preview with an inline composer #1107] vscode preview: drop showInputBox; write marker from composer-collected text
• 69b6e39c [PIR vscode: replace top-of-window Quick Pick input for adding review comments from the markdown preview with an inline composer #1107] Define named webview<->host message protocol types (review feedback)
• 2b9528c4 [PIR vscode: replace top-of-window Quick Pick input for adding review comments from the markdown preview with an inline composer #1107] Convert artifact-canvas source components from React.createElement to JSX (review feedback)
• (plus thread-update commits)

Test Results

• pnpm --filter @cluesmith/codev-artifact-canvas build: ✓ pass
• pnpm --filter @cluesmith/codev-artifact-canvas test: ✓ pass (68 tests; 9 new composer unit tests + updated canvas/e2e tests driving the open-composer→submit flow)
• pnpm --filter @cluesmith/codev-artifact-canvas check-types: ✓ pass
• vscode check-types (host + tsconfig.webview.json): ✓ pass
• vscode esbuild bundle (node esbuild.js --production): ✓ pass
• vscode lint: ✓ pass
• vscode test:unit: ✓ pass (516 tests)
• Manual verification: confirmed by the human at the dev-approval gate — composer appears at the block (not the window top), block stays visible while typing, ⌘/Ctrl+Enter submits, Esc cancels and restores focus, multi-line input works, submitted comment collapses into the persisted card in place.

Architecture Updates

COLD — codev/resources/arch.md updated (the "Markdown Preview / artifact-canvas
host integration" entry). The previous text described the now-removed flow
(onAddComment(line) → host showInputBox → WorkspaceEdit). Updated to reflect:
the inline composer (overlays/CommentComposer.tsx, portalled in-flow below the
block), the widened onAddComment(line, text) seam, and the new named host↔webview
message protocol (markdown-preview/messages.ts, shared by both ends; inbound data
still validated at runtime because it's untrusted).

HOT — no arch-critical.md change. This is a package-level seam/UX change, not a
framework-wide cross-cutting fact; it does not meet the hot-tier bar (and the file is
at/near its cap). Correctly routed to the cold reference.

Lessons Learned Updates

COLD — codev/resources/lessons-learned.md updated (Architecture section): the
reusable pattern for placing an interactive React widget inside an
innerHTML-managed body — inject a placeholder node in an effect and createPortal
the component into it, with an idempotent injection effect to avoid a
setState-on-inject loop. Spec-narrow recipe → cold tier, not the capped hot file.

HOT — no lessons-critical.md change. No behavior-changing cross-cutting rule
emerged that warrants displacing a capped hot lesson.

Consultation Findings (PIR single advisory pass)

The PR-stage consultation (claude + codex, type=impl, max_iterations: 1) returned:
claude = APPROVE (HIGH), codex = REQUEST_CHANGES (HIGH).

Codex finding (real defect — FIXED in 4a… + regression test): the host inserted
the new marker at markerInsertionLine(line) (line+1), which prepends ahead of any
markers already stacked on the block — so for a block with existing comments, the new
card rendered at the top of the thread, while the inline composer had appeared
below the existing cards. Visual position ≠ result, and it violated the approved
plan's append-to-thread behavior.

• Fix: added markerAppendLine(text, annotatedLine) to @cluesmith/codev-core/review-markers
  (returns the line after the contiguous marker run; falls back to markerInsertionLine
  when the block has none). preview-provider.ts now inserts there, so the new comment
  lands where the composer was (newest-last).
• Regression test: packages/core/src/__tests__/review-markers.test.ts — markerAppendLine
  appends after a 2-marker run (asserts file/render order first, second, third, all
  anchored to the block) and handles end-of-file; without the fix the new comment would
  be first.
• Codex's other two points: (a) the stacked-comment path lacked coverage — addressed
  by the regression test above; (b) the stale stripMarkersForRender header comment in
  preview-provider.ts — corrected to describe the raw-text + renderer-strips flow.

⚠️ Escalation for the pr gate (PIR will not re-review this): the fix changes the
canvas surface to append-below (newest-last). The editor Comments-API path
(comments/plan-review.ts:171) still inserts at markerInsertionLine(line) (newest-first),
so the two authoring surfaces now stack a new comment on an already-commented block in
different order. Aligning the editor path is out of scope for #1107 (the issue lists
it as out of scope) and is a one-line change best done deliberately — recommend a
follow-up issue. Please confirm the append-below choice (it matches this PR's approved
plan and conventional thread UX) at the gate.

Things to Look At During PR Review

• ArtifactCanvas.tsx placeholder-injection effect — the trickiest part. It
  injects an in-flow .codev-canvas-comment-composer-host below the block (after the
  marker-card stack if present) and is guarded to be idempotent: it bails when a
  correctly-placed host already exists (isConnected && previousElementSibling === anchor), so the setComposerHost call doesn't loop. An html rebuild disconnects
  the node and the guard re-creates it. Worth a careful read.
• Event isolation via portal — the composer's DOM lives inside the body div, but
  React routes synthetic events through the fiber tree (the portal's parent is the
  canvas root, not the body div), so the body's onKeyDown "open composer" handler
  does not fire for keystrokes typed into the composer. This is why plain Enter in the
  textarea inserts a newline rather than re-triggering the open path.
• Keystroke choice — ⌘/Ctrl+Enter submits, Enter = newline, Esc cancels (GitHub
  review-composer convention). This changed the old single-line Enter-to-submit muscle
  memory; confirmed acceptable at the dev-approval gate.
• Untrusted postMessage — preview-provider.ts casts inbound to the named union
  for the discriminant but still validates line/text at runtime. The named type
  documents the shape; it does not vouch for an arbitrary runtime value.
• Scope note (JSX conversion) — at the reviewer's request, all five artifact-canvas
  source components were converted from React.createElement to JSX in this PR (not
  just the new file), so the package is internally consistent. Behavior is unchanged;
  it inflates the diff. The package's tsconfig.json already had "jsx": "react-jsx".

How to Test Locally

• View diff: VSCode sidebar → right-click builder pir-1107 → View Diff
• Run dev server: VSCode sidebar → Run Dev Server, or afx dev pir-1107
• What to verify:
  • Open a codev/specs|plans|reviews/*.md in the Codev Markdown Preview ("Reopen With… → Codev Markdown Preview").
  • Click + on blocks at the top, middle, and bottom of a long doc — the composer appears at the block each time, and the block stays visible while typing.
  • ⌘/Ctrl+Enter submits and the comment collapses into the persisted card in place; Esc cancels and returns focus to the block.
  • Keyboard-only: Tab to +, Enter to open, type, ⌘/Ctrl+Enter to submit.
  • Multi-line input works (Enter = newline); the on-disk marker is still single-line.

[amrmelsayed]

Architect Integration Review

Verified all three claims in the builder's disposition independently against PR head.

Codex finding addressed (commit 27c2c87)

The defect codex identified — new comments on an already-commented block insert at the top of the existing thread while the composer appears below — is real and now fixed:

• markerAppendLine helper added at packages/core/src/review-markers.ts:71-78. Walks past the run of existing markers from markerInsertionLine's position; returns the position AFTER the last marker. Pure, host-agnostic (correctly placed in core, not the host).
• Preview-provider consumption switched at packages/vscode/src/markdown-preview/preview-provider.ts: import + write site both moved from markerInsertionLine → markerAppendLine. New marker lands at the visually correct position (below the existing thread, matching the composer's render location).
• Regression test at packages/core/src/__tests__/review-markers.test.ts covers the load-bearing case: a block with two stacked markers receives a third; markerAppendLine returns position 3 (past both existing markers); resulting render order is ['first', 'second', 'third'] with all three anchored to the heading. Plus an empty-thread equivalence case and an end-of-file edge case.

Implementation quality observations

• Helper placement is correct: the codec is a cross-host contract (the canvas package, the VSCode editor Comments-API path, and any future host all round-trip through it), so it belongs in @cluesmith/codev-core/review-markers not in either host. The PR keeps the placement clean.
• JSX conversion (commit 2b9528c) — the artifact-canvas component source moved from React.createElement to JSX during the dev-gate iteration. Out of scope for vscode: replace top-of-window Quick Pick input for adding review comments from the markdown preview with an inline composer #1107 but mentioned in the thread as architect-requested feedback; reads as a quality improvement, not a deviation.
• Named webview ↔ host message protocol types added in markdown-preview/messages.ts (commit 69b6e39) — separates WebviewToHostMessage / HostToWebviewMessage so the composer's addComment intent carrying { line, text } is a typed contract, not an inline shape.

Escalation acknowledged — filing follow-up

The builder's escalation is correct and surfaced honestly: packages/vscode/src/comments/plan-review.ts:171 still uses markerInsertionLine (prepends new comments at the top of the existing thread), so the editor Comments-API path and the preview-composer path now write in opposite stack orders. Two surfaces, one on-disk format, divergent UX.

Filing a follow-up to align the editor path with the preview's append semantics (and the canvas adapter's rendered order). The fix is mechanical (swap markerInsertionLine → markerAppendLine at the one call site) and the regression test in core/review-markers.test.ts already validates the underlying helper. Tracking as a separate issue rather than expanding #1107's scope after dev-gate approval. (Follow-up issue link will be appended to this PR comment once filed.)

Recommendation

APPROVE for the pr gate. Ready for merge.

The fix is correct, well-tested, properly placed in the core codec, and the escalation is handled by a follow-up rather than scope-creep on this PR.

---

Architect integration review

[amrmelsayed]

Follow-up issue for the escalation: #1122 (editor Comments-API path parity, BUGFIX-scoped). Tracked separately so #1121 ships clean.