Review UI: collapsible identity-aware comments rail, clean dialogs, editor spacing fixes

.beads/interactions.jsonl

@@ -192,3 +192,9 @@
 {"id":"int-1f4f8a2b","kind":"field_change","created_at":"2026-05-28T20:54:26.223302Z","actor":"Angus Bezzina","issue_id":"attn-zb5.2","extra":{"field":"status","new_value":"closed","old_value":"open","reason":"Closed"}}
 {"id":"int-d6c93840","kind":"field_change","created_at":"2026-05-28T20:54:26.306244Z","actor":"Angus Bezzina","issue_id":"attn-zb5.3","extra":{"field":"status","new_value":"closed","old_value":"open","reason":"Closed"}}
 {"id":"int-4cc271e7","kind":"field_change","created_at":"2026-05-28T20:54:26.385364Z","actor":"Angus Bezzina","issue_id":"attn-zb5.4","extra":{"field":"status","new_value":"closed","old_value":"open","reason":"Closed"}}
+{"id":"int-fd265fab","kind":"field_change","created_at":"2026-06-10T20:42:34.962462Z","actor":"Angus Bezzina","issue_id":"attn-d7y","extra":{"field":"status","new_value":"closed","old_value":"in_progress","reason":"Closed"}}
+{"id":"int-b5122651","kind":"field_change","created_at":"2026-06-10T23:24:31.985272Z","actor":"Angus Bezzina","issue_id":"attn-0sv","extra":{"field":"status","new_value":"closed","old_value":"in_progress","reason":"Closed"}}
+{"id":"int-20e67379","kind":"field_change","created_at":"2026-06-11T01:04:13.463975Z","actor":"Angus Bezzina","issue_id":"attn-42y","extra":{"field":"status","new_value":"closed","old_value":"in_progress","reason":"Closed"}}
+{"id":"int-c027a4a5","kind":"field_change","created_at":"2026-06-11T02:17:08.736047Z","actor":"Angus Bezzina","issue_id":"attn-23m","extra":{"field":"status","new_value":"closed","old_value":"in_progress","reason":"Closed"}}
+{"id":"int-6e5885d9","kind":"field_change","created_at":"2026-06-11T13:35:27.549825Z","actor":"Angus Bezzina","issue_id":"attn-z46","extra":{"field":"status","new_value":"closed","old_value":"in_progress","reason":"Closed"}}
+{"id":"int-c105ce23","kind":"field_change","created_at":"2026-06-11T14:20:18.718761Z","actor":"Angus Bezzina","issue_id":"attn-2aj","extra":{"field":"status","new_value":"closed","old_value":"in_progress","reason":"Closed"}}

CLAUDE.md

@@ -44,9 +44,11 @@ This boots three processes:
    user provides an invite.
 
 Interactive flow: click **[Share]** in the owner window, copy the invite URL,
-paste it at the script's prompt — it runs `attn review join <invite>
---as-agent reviewer` against the reviewer daemon and the second window joins
-the room. Ctrl+C cleans up all three processes.
+paste it at the script's prompt — it runs `attn review join <invite>` routed
+to the reviewer daemon (deliberately NOT `--as-agent`, which forks a separate
+headless agent process and leaves the reviewer window idle; the daemon-routed
+join flips the reviewer's own window onto the shared document). Ctrl+C cleans
+up all three processes.
 
 Env overrides:
 

planning/collab/ui/review-panel-design.md

@@ -233,31 +233,47 @@ spatial-align.
 
 ## 3. Resolved State
 
-Resolved threads do **not** disappear. They shrink to a **single-line grey
-strip** in place (same y-position as the active card would have had):
+> **Amended by attn-d7y** (shipped): the full-width strip became a
+> content-sized **chip**, the rail width adapts, and clicking a chip
+> expands a read-only card. The original strip spec is superseded by the
+> behavior below; the >5 "show all resolved" pill survives unchanged.
+
+Resolved threads do **not** disappear. They shrink to a **28px rounded
+chip** in place (same y-position as the active card would have had):
 
 ```text
-│ ─── ✓ rufus · resolved 2m · §Anchor resolver ─── │
+│ (✓ rufus · resolved)                │ ← content-sized pill, full rail
 ```
 
-The strip is 24px tall, uses the `--muted-foreground` color, and is dismissed
-from the document-order layout queue (it can be skipped over for collision
-purposes — newer active cards collapse past it).
-
-If there are more than 5 collapsed strips visible in the current viewport,
-the design collapses them further to a "show all resolved" pill at the
-**bottom of the margin** (still inside the overlay, after the last anchored
-card):
+When the margin holds ONLY resolved threads (no active cards, no orphan
+tray, nothing expanded) the right-rail aside slims from 320px to a **48px
+gutter** and the chips render icon-only — `(✓)` centered in the gutter —
+so the document reclaims the width instead of facing a vacant column. The
+mode derivation lives in `web/src/lib/review/rail-mode.ts`
+(`closed`/`slim`/`full`), exposed as `reviewStore.railMode`, and drives
+both the aside width (App.svelte) and the chip variant (ReviewMargin).
+
+Clicking a chip expands it **in place to the full card** in read-only
+resolved state (author, quote, body, replies, `resolved` badge) with a
+single `Collapse` action — no Reply/Resolve/Accept/Reject. Expanding
+forces the rail to full width; Collapse (or Escape) shrinks it back. The
+expanded card participates in the SAME collision pass as active cards
+(one unified `layoutCards` call), so it pushes neighbors instead of
+overlapping them.
+
+If there are more than 5 collapsed chips visible in full mode, they
+collapse further to a "show all resolved" pill at the **bottom of the
+margin** (still inside the overlay, after the last anchored card):
 
 ```text
 │ ─────────────────────────────────── │
-│       ⌃ 12 resolved · show          │ ← bottom pill, click to expand all strips
+│       ⌃ 12 resolved · show          │ ← bottom pill, click to expand all chips
 │ ─────────────────────────────────── │
 ```
 
-Clicking the pill expands all strips inline at their anchor positions.
-Clicking a strip pops it back to a full card for one cycle (until next focus
-change).
+Clicking the pill expands all chips inline at their anchor positions.
+Slim mode ignores the pill (the 48px gutter can't fit it; icon chips are
+cheap to render).
 
 ---
 

scripts/test-review-e2e.sh

@@ -232,11 +232,12 @@ for cb in reviewStatus reviewEvent reviewSnapshot reviewAnchorResolution; do
 done
 
 echo ""
-echo "--- Review store scaffold (from 12.10, pending) ---"
-# The review store module should be importable and expose a default shape.
-# Today the module does not exist — record as PEND.
+echo "--- Review store scaffold ---"
+# The review store singleton is exposed for E2E seeding (App.svelte wires
+# `window.__attn_review_store__` on mount). Hard assertion — the adaptive
+# rail suite below depends on it.
 result=$("$ATTN" --eval "typeof window.__attn_review_store__")
-expect_eq_soft "window.__attn_review_store__ exposed" "$result" '"object"' "attn-nnj.12.10"
+assert_eq "window.__attn_review_store__ exposed" "$result" '"object"'
 
 screenshot "02-shape-asserted"
 
@@ -321,6 +322,239 @@ echo "        (run scripts/test-apply-e2e.sh to verify the contract directly)"
 
 screenshot "03-apply-flow-pending"
 
+# ===================================================================
+# TEST SUITE: Collapsible rail + identity chips (attn-d7y / attn-42y)
+# ===================================================================
+#
+# Seeds the review store directly through `window.__attn_review_store__`
+# (no relay needed): one comment thread + its CommentResolved event. In a
+# review room the rail is always present: collapsed to a 48px gutter
+# (✓ chips for resolved, author-avatar chips for unresolved) unless
+# expanded by the user/auto-open. Clicking a ✓ chip expands the rail with
+# the full read-only card (no action buttons); clicking the card shrinks
+# it back to a labeled chip. The ReviewBar dock carries the rail toggle.
+
+echo ""
+echo "=== Review E2E: collapsible rail + identity chips (attn-d7y/attn-42y) ==="
+
+# Poll an --eval expression until it returns the expected value (the aside
+# width animates over 200ms, so one-shot reads race the transition).
+poll_eval() {
+    local expr="$1" expected="$2" attempts=30
+    local result=""
+    while [ $attempts -gt 0 ]; do
+        result=$("$ATTN" --eval "$expr" 2>/dev/null || echo "")
+        if [ "$result" = "$expected" ]; then
+            echo "$result"
+            return 0
+        fi
+        sleep 0.1
+        attempts=$((attempts - 1))
+    done
+    echo "$result"
+}
+
+"$ATTN" --wait-for '.ProseMirror' --timeout 10000 >/dev/null 2>&1 || true
+
+seed_js=$(cat <<'EOF'
+(() => {
+  const s = window.__attn_review_store__;
+  if (!s) return 'no-store';
+  const anchor = {
+    v: 2, fileId: 'file-x', snapshotId: 'snap-x', baseHash: 'h-x',
+    position: { byteRange: [0, 10], lineRange: [1, 1] },
+  };
+  const meta = (id) => ({
+    v: 2, eventId: id, roomId: 'room-x', authorId: 'p-reviewer',
+    deviceId: 'd-x', createdAt: Date.now(), parentEventIds: [],
+    snapshotId: 'snap-x',
+  });
+  const auth = { signature: 'sig', signingKeyId: 'kid' };
+  s.applyEvent({ meta: meta('e-1'), body: { type: 'comment_created', threadId: 't-1', anchor, body: 'Consider tightening this wording.' }, auth });
+  s.applyEvent({ meta: meta('e-2'), body: { type: 'comment_resolved', threadId: 't-1', resolvedBy: 'p-owner' }, auth });
+  s.selectRoom('room-x');
+  s.setCurrentFile('file-x');
+  return 'ok';
+})()
+EOF
+)
+result=$("$ATTN" --eval "$seed_js")
+assert_eq "Seeded resolved-only review thread" "$result" '"ok"'
+
+echo ""
+echo "--- Collapsed gutter (resolved-only margin, default) ---"
+result=$(poll_eval "document.querySelector('[data-slot=\\\"right-rail\\\"]')?.getAttribute('data-mode') ?? 'missing'" '"collapsed"')
+assert_eq "Rail data-mode is collapsed (no unresolved threads)" "$result" '"collapsed"'
+
+result=$(poll_eval "document.querySelector('[data-slot=\\\"right-rail\\\"]')?.offsetWidth" '48')
+assert_eq "Rail width is exactly the 48px gutter" "$result" "48"
+
+result=$("$ATTN" --query '[data-testid="review-margin-resolved-chip"]' | jq -r '.count' 2>/dev/null || echo "0")
+assert_eq "One resolved chip rendered" "$result" "1"
+
+result=$("$ATTN" --eval "document.querySelector('[data-testid=\\\"review-margin-resolved-chip\\\"]')?.getAttribute('data-variant') ?? 'missing'")
+assert_eq "Chip is icon variant in the gutter" "$result" '"icon"'
+
+result=$("$ATTN" --eval "parseInt(document.querySelector('[data-testid=\\\"review-margin-resolved-chip\\\"]')?.style.top ?? '-1', 10) >= 8")
+assert_eq "Gutter chip respects the inner clearance (top ≥ 8)" "$result" "true"
+
+result=$("$ATTN" --eval "(() => { const t = document.querySelector('[data-slot=\\\"rail-toggle\\\"]'); const rail = document.querySelector('[data-slot=\\\"right-rail\\\"]'); if (!t || !rail) return 'missing'; const tr = t.getBoundingClientRect(); const rr = rail.getBoundingClientRect(); return Math.abs((tr.left + tr.width / 2) - (rr.left + rr.width / 2)) <= 2 ? 'centered' : 'off-center'; })()")
+assert_eq "Toggle is horizontally centered in the collapsed gutter" "$result" '"centered"'
+
+screenshot "04-resolved-collapsed-gutter"
+
+echo ""
+echo "--- Expand ✓ chip → rail expands with read-only card ---"
+"$ATTN" --click '[data-testid="review-margin-resolved-chip"]' >/dev/null 2>&1 || true
+
+result=$(poll_eval "document.querySelector('[data-slot=\\\"right-rail\\\"]')?.getAttribute('data-mode') ?? 'missing'" '"expanded"')
+assert_eq "Rail expands on chip click" "$result" '"expanded"'
+
+result=$(poll_eval "document.querySelector('[data-slot=\\\"right-rail\\\"]')?.offsetWidth" '320')
+assert_eq "Rail width is exactly 320px when expanded" "$result" "320"
+
+result=$(poll_eval "document.querySelector('[data-testid=\\\"review-margin-card\\\"][data-state=\\\"resolved\\\"]') !== null" 'true')
+assert_eq "Resolved card rendered" "$result" "true"
+
+result=$("$ATTN" --eval "parseInt(document.querySelector('[data-testid=\\\"review-margin-card\\\"][data-state=\\\"resolved\\\"]')?.parentElement?.style.top ?? '-1', 10) >= 8")
+assert_eq "Card keeps breathing room from the rail top (top ≥ 8)" "$result" "true"
+
+result=$("$ATTN" --eval "(() => { const c = document.querySelector('[data-testid=\\\"review-margin-card\\\"][data-state=\\\"resolved\\\"]'); if (!c) return 'no-card'; return [c.querySelectorAll('[data-action]').length === 0, c.querySelector('.rmc-avatar') !== null].join(','); })()")
+assert_eq "Card is read-only (no action buttons) with an author avatar" "$result" '"true,true"'
+
+result=$("$ATTN" --eval "document.querySelector('[data-testid=\\\"review-margin-card\\\"][data-state=\\\"resolved\\\"]')?.textContent.includes('Consider tightening this wording.')")
+assert_eq "Card shows the resolved comment body" "$result" "true"
+
+screenshot "05-resolved-expanded-card"
+
+echo ""
+echo "--- Click card → shrinks back to labeled chip (rail stays expanded) ---"
+"$ATTN" --click '[data-testid="review-margin-card"][data-state="resolved"]' >/dev/null 2>&1 || true
+
+result=$(poll_eval "document.querySelector('[data-testid=\\\"review-margin-card\\\"][data-state=\\\"resolved\\\"]') === null" 'true')
+assert_eq "Resolved card gone after card click" "$result" "true"
+
+result=$(poll_eval "document.querySelector('[data-testid=\\\"review-margin-resolved-chip\\\"]')?.getAttribute('data-variant') ?? 'missing'" '"label"')
+assert_eq "Labeled chip back in the expanded rail" "$result" '"label"'
+
+result=$("$ATTN" --eval "document.querySelector('[data-slot=\\\"right-rail\\\"]')?.getAttribute('data-mode')")
+assert_eq "Rail stays expanded after card collapse" "$result" '"expanded"'
+
+echo ""
+echo "--- Mixed margin (active + resolved) → cards + labeled chip ---"
+mixed_js=$(cat <<'EOF'
+(() => {
+  const s = window.__attn_review_store__;
+  if (!s) return 'no-store';
+  // Anchored deep in the document (not the top band): the scroll-tracking
+  // suite below needs this card to move 1:1 with a 150px scroll, and the
+  // rail-top breathing-room clamp (attn-2aj) intentionally pins cards
+  // whose anchors sit near the viewport top.
+  const anchor = {
+    v: 2, fileId: 'file-x', snapshotId: 'snap-x', baseHash: 'h-x',
+    position: { byteRange: [600, 640], lineRange: [20, 20] },
+  };
+  s.applyEvent({
+    meta: { v: 2, eventId: 'e-3', roomId: 'room-x', authorId: 'p-reviewer', deviceId: 'd-x', createdAt: Date.now(), parentEventIds: [], snapshotId: 'snap-x' },
+    body: { type: 'comment_created', threadId: 't-2', anchor, body: 'An open question.' },
+    auth: { signature: 'sig', signingKeyId: 'kid' },
+  });
+  return 'ok';
+})()
+EOF
+)
+result=$("$ATTN" --eval "$mixed_js")
+assert_eq "Seeded an additional unresolved thread" "$result" '"ok"'
+
+result=$(poll_eval "document.querySelector('[data-testid=\\\"review-margin-card\\\"][data-state=\\\"open\\\"]') !== null" 'true')
+assert_eq "Active card rendered alongside the chip" "$result" "true"
+
+result=$("$ATTN" --eval "(() => { const c = document.querySelector('[data-testid=\\\"review-margin-card\\\"][data-state=\\\"open\\\"]'); if (!c) return 'no-card'; const cs = getComputedStyle(c); return [c.querySelector('.rmc-avatar') !== null, cs.borderLeftColor.length > 0].join(','); })()")
+assert_eq "Active card carries author avatar + colored border" "$result" '"true,true"'
+
+screenshot "06-mixed-expanded-rail"
+
+echo ""
+echo "--- Rail-header toggle → collapsed gutter with avatar chips ---"
+result=$("$ATTN" --query '[data-slot="rail-toggle"]' | jq -r '.status' 2>/dev/null || echo "not_found")
+assert_eq "Rail toggle present in the rail header" "$result" "found"
+
+"$ATTN" --click '[data-slot="rail-toggle"]' >/dev/null 2>&1 || true
+
+result=$(poll_eval "document.querySelector('[data-slot=\\\"right-rail\\\"]')?.getAttribute('data-mode') ?? 'missing'" '"collapsed"')
+assert_eq "Toggle collapses the rail" "$result" '"collapsed"'
+
+result=$("$ATTN" --query '[data-testid="review-margin-avatar-chip"]' | jq -r '.count' 2>/dev/null || echo "0")
+assert_eq "Unresolved thread shows an author avatar chip in the gutter" "$result" "1"
+
+result=$("$ATTN" --query '[data-testid="review-margin-resolved-chip"][data-variant="icon"]' | jq -r '.count' 2>/dev/null || echo "0")
+assert_eq "Resolved thread shows a ✓ chip in the gutter" "$result" "1"
+
+screenshot "07-collapsed-gutter-chips"
+
+echo ""
+echo "--- Avatar chip click → rail expands onto the thread ---"
+"$ATTN" --click '[data-testid="review-margin-avatar-chip"]' >/dev/null 2>&1 || true
+
+result=$(poll_eval "document.querySelector('[data-slot=\\\"right-rail\\\"]')?.getAttribute('data-mode') ?? 'missing'" '"expanded"')
+assert_eq "Avatar chip expands the rail" "$result" '"expanded"'
+
+result=$(poll_eval "document.querySelector('[data-testid=\\\"review-margin-card\\\"][data-state=\\\"open\\\"]') !== null" 'true')
+assert_eq "Thread card visible after avatar expand" "$result" "true"
+
+screenshot "08-avatar-expanded"
+
+echo ""
+echo "--- Cards track their anchors while the document scrolls (attn-23m) ---"
+# Scroll the EDITOR viewport and assert the card's on-screen top moves by
+# the same amount (opposite sign). Tolerance ±4px for rounding/collision.
+scroll_js=$(cat <<'EOF'
+(() => {
+  const card = document.querySelector('[data-testid="review-margin-card"][data-state="open"]');
+  const vp = document.querySelector('.attn-content-viewport [data-slot="scroll-area-viewport"]');
+  if (!card || !vp) return 'missing';
+  const before = card.getBoundingClientRect().top;
+  vp.scrollTop = 150;
+  const scrolled = vp.scrollTop;
+  window.__attn_scroll_probe__ = { before, scrolled };
+  return scrolled > 0 ? 'scrolled' : 'no-scroll';
+})()
+EOF
+)
+result=$("$ATTN" --eval "$scroll_js")
+assert_eq "Editor viewport scrolled" "$result" '"scrolled"'
+
+verify_js=$(cat <<'EOF'
+(() => {
+  const probe = window.__attn_scroll_probe__;
+  const card = document.querySelector('[data-testid="review-margin-card"][data-state="open"]');
+  if (!probe || !card) return 'missing';
+  const after = card.getBoundingClientRect().top;
+  const drift = Math.abs((probe.before - after) - probe.scrolled);
+  return drift <= 4 ? 'tracked' : `drifted by ${Math.round(drift)}px (before ${Math.round(probe.before)}, after ${Math.round(after)}, scrolled ${probe.scrolled})`;
+})()
+EOF
+)
+result=$(poll_eval "$verify_js" '"tracked"')
+assert_eq "Card moved 1:1 with the document scroll" "$result" '"tracked"'
+
+# Scroll back and confirm it returns to its original position.
+"$ATTN" --eval "document.querySelector('.attn-content-viewport [data-slot=\\\"scroll-area-viewport\\\"]').scrollTop = 0" >/dev/null
+restore_js=$(cat <<'EOF'
+(() => {
+  const probe = window.__attn_scroll_probe__;
+  const card = document.querySelector('[data-testid="review-margin-card"][data-state="open"]');
+  if (!probe || !card) return 'missing';
+  const drift = Math.abs(card.getBoundingClientRect().top - probe.before);
+  return drift <= 4 ? 'restored' : `off by ${Math.round(drift)}px`;
+})()
+EOF
+)
+result=$(poll_eval "$restore_js" '"restored"')
+assert_eq "Card returns to its anchor when scrolled back" "$result" '"restored"'
+
+screenshot "09-scroll-tracking"
+
 # ===================================================================
 # Summary
 # ===================================================================