## Why

A comment marker stuck to the top edge of the document pane once its passage scrolled out of view. Scroll a document with a comment on its title and the marker sat at the top of the pane, over whatever prose was there, at every scroll position, on desktop and phone. The geometry clamped every marker into the visible document, even when its passage was off screen.

## What changed

- `markerPoints` in `comment-geometry.ts` no longer clamps a marker whose passage is entirely above or below the document. The marker follows its passage and is flagged `offscreen`. It is not counted as an obstacle, so it no longer pushes visible markers out of place. - `comment-layer.tsx` hides offscreen markers (`visibility: hidden`, which also takes them out of the tab order) and shows no hover preview for them. A pinned thread card keeps its current edge-docked behaviour. - Exception: a marker whose thread is open (pinned) or which has keyboard focus is `held`. It keeps the old placement, clamped to the nearest edge, so focus is not dropped and the open card keeps its anchor. - Markers for visible or partly visible passages keep all existing collision avoidance. - Decision markers did not have this bug. `resolved-layer.tsx` already drops decisions whose blocks are off screen, and `.plan-workspace` clips any marker that is partly out of view. The unused `marginPoint` helper still clamps, but nothing calls it at runtime.

## Screenshots

Document scrolled down 1500px, with the title comment out of view.

Before (desktop): the marker is pinned over the callout at the top of the pane. 

After (desktop): 

Before (phone 390×844): 

After (phone 390×844): 

## Testing

- `bun test packages/editor`: 451 pass, 0 fail. This includes new `comment-geometry.test.ts` cases for a passage above the document, a passage below it, and an offscreen marker that no longer displaces a visible one. - `bun run types`: pass. - New e2e cases in `e2e/responsive-comments.e2e.ts` (desktop 1440×900 and phone 390×844) check that the marker is hidden and above the scroller after scrolling, then back in its resting position after scrolling to the top. Local E2E is suspended, so CI runs them. - Checked by hand on a fake-GitHub server: after scrolling, the marker's y goes from 96 to -1280 on desktop and is hidden. On phone it goes from 68 to -1385 and is hidden. - `bun run ci`: fails only on the design-contract review hash below.

## Needs a design-contract review

- File: `packages/editor/src/comment-layer.tsx` - Exception file: `scripts/design-contract/exceptions/dynamic-editor.json` (both `comment-layer.tsx` groups; `sourceHash` is currently `c60690040f4b0911e23027b7b975596e0db27e2d4e20c4ec4ce3f1955c5b81e8`) - New SHA-256: `de45eb676c49bcb1f0d96ae7ab241763284f57d446e8c32c073be7352b78b152` - Why it's safe: the marker button's style changed from `style={button}` to an object literal with `top`, `left` and `visibility`, where `visibility` is either `"hidden"` or `undefined`. It adds no colour, type or spacing values. I removed the stale `button` dynamic-style case from the exception. The other flagged owners (preview, surface, hit, presence classes) are unchanged and are flagged only because the file hash changed.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

**Review:** Looks good

Checked in a real browser (seeded long document, desktop 1440×900 and phone 390×844) plus `bun test comment-geometry` (20 pass).

- Partly visible passages: correct. Title comment stays visible and clamps to the pane top while any of the passage is in view (scroll 48 to 80 held at the pane edge), then hides once the passage is fully out (scroll 100). `target` is the union rect of the range, so a multi-line passage with any line in view keeps its marker. - Collision avoidance for visible markers is unchanged, and the new unit test shows an offscreen marker no longer displaces a visible one. - Phone: same behaviour, marker follows the passage out of view and returns at the same y on scroll back. - `dynamic-editor.json` removal is legitimate: `style={button}` was the unresolved case; the new inline `{ top, left, visibility }` literal resolves, so that stale entry is gone. `bun run ci` fails only on the `comment-layer.tsx` review-hash renewals, which are expected.

Suggestion (non-blocking), focus: if a marker has keyboard focus and its passage scrolls away, it becomes `visibility: hidden` and focus falls to `BODY` (verified). The same happens when a pinned card's marker is hidden (card stays docked at the edge, focus on BODY; Tab then reaches "Close comment", Esc works from there). Recommend not hiding a marker that is focused or pinned: e.g. `visibility: button.offscreen && !focused && pinned !== id ? "hidden" : undefined`, with the offscreen point clamped to the pane edge for that case. Simpler alternative: when a pinned thread's marker goes offscreen, move focus to the card's close button. A plain-focus (unpinned) loss on scroll is arguably fine since scrolling is a deliberate action, so prioritise the pinned case.

Addressed the review in 6dd23786:

- `MarkerTarget` has a new `held` flag. A held target skips the offscreen branch and keeps the old edge-clamped placement. `comment-layer.tsx` sets `held` when the thread is pinned (read through a ref; `pinned` is also now a re-measure dependency) or when its marker button has keyboard focus. The button's `onBlur` re-measures, so a marker that was held only because it had focus hides once focus leaves it. - Unit test: a held marker for a passage above or below the document clamps to `top: 0` or `top: 576`. - E2E: a new desktop case pins a thread, scrolls 1500px, and checks that the marker and card stay visible, the marker stays inside the scroller, and focus is not on `body`.

CI: e2e passes (335). The validation job fails only on the `comment-layer.tsx` design-contract hash. The new hash, `de45eb676c49bcb1f0d96ae7ab241763284f57d446e8c32c073be7352b78b152`, is updated in the PR body.

@copilot investigate and fix failing CI tests

> @copilot investigate and fix failing CI tests

Fixed in 865d918. CI was failing on stale design-contract review hashes, not browser tests. After reviewing the unchanged geometry/presence boundaries, I renewed only the two `comment-layer.tsx` hashes.

`bun run ci`, types, build, and all 3,670 unit tests pass (3 PostgreSQL tests skipped without `TEST_DATABASE_URL`). The targeted 58 tests also pass. Final read-only review found no issues.