fix(selection): judge containment by text at the bubble and doc pane (#7891)

[chenmingwei23]

Problem / Motivation

PR #7875 fixed the chat selection toolbar dismissing a multi-click
(double/triple-click) selection of a message's LAST line. The same predicate
survived at two more surfaces, each carrying the identical reported failure:

• website/src/pages/chat/UserMessage.tsx -- multi-click the last line of a user
  bubble containing a paste chip, then copy: the chip-expansion handler bails and
  the clipboard receives the literal [ Paste #1 - 4 lines ] label that the
  handler exists to replace.
• website/src/apps/spec-builder/components/DocView.tsx -- triple-click the
  document's last paragraph and the Comment pill never appears, so the review
  affordance is unreachable for that paragraph.

Why it matters

Both are silent failures on the last line only, which is the hardest shape for a
user to attribute: copy appears to work and delivers the wrong text, and the
Comment pill's absence reads as "this paragraph is not commentable" rather than
as a bug. The last paragraph of a spec document is disproportionately the one a
reviewer wants to comment on, and a paste chip is most often the last thing in a
message. Left as is, the predicate was also on track to be copied a fourth time.

What changed (motivation -> approach -> change)

Browsers normalize a multi-click selection of a container's first or last block
to a boundary point just OUTSIDE the container: a triple-click paragraph
selection ends "at the start of the next block", and for the last block that
position lives in the container's parent. That hoists
range.commonAncestorContainer above the container, so a bare
container.contains(range.commonAncestorContainer) early return rejects a
selection whose every character is inside the container. Ancestry is the wrong
question; what the caller needs to know is whether any SELECTED TEXT lies
outside.

Rather than copy #7875's inline block a third time, the predicate moves to
containedSelectionRange(range, container) in
website/src/utils/selectionContainment.ts, and all three call sites read from
it. It returns the part of the range lying inside the container -- the original
range untouched on the fast path, a container-clamped clone when an accepted
overhang has to be trimmed, null on rejection -- so each caller measures,
stringifies, or clones from a range that cannot reach past the surface it owns.
That single return value is what lets one helper serve three different consumers:
the toolbar positions from it, UserMessage clones the clipboard fragment from
it, and DocView takes the pill's rect from it.

Both rejection tiers move with it, in order:

1. The O(1) endpoint check runs first. One toolbar instance mounts per message
   and each listens on document, so the N-1 non-owning instances must not fall
   through to stringification, which would serialize text growing with transcript
   distance on every event (select-all being the worst case). A
   boundary-normalized multi-click always keeps at least one endpoint inside its
   own container, so this tier never rejects the case the predicate exists for.
2. Only then the text tier: clamp a clone to each side of the container and
   require both overhangs to hold no text. A selection genuinely spanning into a
   sibling stays rejected in either direction.

SelectionToolbar behavior is unchanged -- its call site reads the clamped
measurement range from the helper instead of computing it inline, and its 11
existing tests pass untouched, including the cloneRange spy that pins the O(1)
reject.

The document pane's mechanism is narrower than the issue states

Building the real-Chromium capture turned up a condition the issue does not
mention, and it is worth recording because it decides whether any evidence of
this defect is real. The Comment pill renders only while a selection is held
({sel && !note && ...}), and the double-click stage of a triple-click normally
sets it -- so the pill's own absolutely-positioned overlay becomes the pane's
last child and ABSORBS the boundary normalization: the end lands in that overlay,
which is still inside the pane, and the pre-fix code works.

The defect needs the pill ABSENT when the third click settles, which is exactly
what happens when the double-click selected a word below
onSelectionSettled's 3-character floor. Reading the live selection
capture-phase (before the pane's own listener re-renders anything) on the
pre-fix component confirms both halves:

triple-click target	pane children at 3rd click	pane.contains(cac)	end normalized to	pill
a long word (pill)	2 (pill overlay present)	true	<div class="absolute z-[5]"> inside the pane	appears
a two-letter word (to)	1	false	<p> outside the pane	never appears

So the user-facing bug is: triple-clicking a last paragraph while the pointer
happens to sit over a short word (to, a, is, of) silently produces
nothing, while the same gesture over a longer word works. That is why the
committed capture script deliberately targets a two-letter word -- targeting a
long one produces a passing shot against the pre-fix component, i.e. a portrait
rather than a proof.

Tests

happy-dom performs no native multi-click selection, so all new tests model the
boundary-normalized geometry directly, as PR #7875 does.

• website/src/test/selectionContainment.test.ts (new, 5 tests) pins the
  helper's contract: identity on the fast path (the common case allocates
  nothing), end-normalized and start-normalized selections accepted AND clamped
  to the container, a sibling-text overhang rejected, and the O(1) tier pinned by
  asserting Range.cloneRange is never called for a selection holding neither
  endpoint. The first and last of these are branches no single consumer
  exercises alone.
• website/src/test/UserMessagePasteCov80.test.tsx (+2) -- the chip still
  expands when the selection end is normalized past the bubble; a selection
  running into a text-bearing neighbour is still ignored.
• website/src/test/SpecBuilderDocView.test.tsx (+2) -- the Comment pill appears
  for a selection normalized past the pane; a selection running past the pane
  into outside text is still dismissed.

Red-first, verified by reverting only the two component sources to main with
the new tests in place: exactly the two acceptance tests failed
(AssertionError: expected "vi.fn()" to be called with arguments), while both
new rejection tests and all 14 pre-existing tests in those files passed. With
the fix: 32/32 across all four affected files, including SelectToAsk.test.tsx
untouched.

Gates run locally after the rebase onto current main: tsc -b clean, eslint
clean on all seven source files, i18n:check exit 0 with I18N_BASE_REF set to
the merge base (0 added literals; this diff adds no user-facing strings),
lint:i18n at baseline, jscpd 0 clones, and added lines carry no flagged
inclusive-language vocabulary.

Manual verification

website/scripts/capture-docview-multiclick.mjs drives real Chromium against
website/capture/docview-multiclick-last-paragraph.html: a genuine triple-click
(so the browser makes its own boundary-normalized selection) on a two-letter word
in the last paragraph. It is a live regression proof, not a portrait -- against
the pre-fix component the pill wait fails with
locator.waitFor: Timeout 5000ms exceeded, and against the fix it captures.

The UserMessage half has no visual delta: the change is to the clipboard
payload, so it is covered by the tests above rather than a screenshot. Its
geometry is the same class the probe measured on #7875's own toolbar harness --
a container whose last block is followed by a sibling, where the end normalizes
to <p> @0 outside the container (containerContainsCac: false).

Screenshots / video

Comment pill raised over a triple-click selection of the document's last
paragraph, which produced nothing before this change:

Light theme

The pill overlapping the line above is its normal placement (top: sel.y - 34),
not an artifact of the harness. The dimmed line below the pane is the harness
standing in for the comment tray: content after the pane is load-bearing, because
without a following block the normalized boundary has nowhere outside to land.

Related Issues

Closes #7891

Pattern harvest

Pattern: DOM containment tested via range.commonAncestorContainer instead of
by the selected text, which rejects any boundary-normalized multi-click of a
container's first or last block.

Rule candidate: review-prompt -- flag contains(range.commonAncestorContainer)
in any selection handler. Three independent sites in this repo carried the same
line, and the third was found only because a reviewer went looking after the
first was fixed; a grep-level rule would have surfaced all three at once. With
the predicate now centralized, a new occurrence of that expression under
website/src is a signal the shared helper was bypassed.

A second, subtler lesson from the capture: a screenshot or capture script can
PASS against the unfixed component and still look like proof. Here the fix's own
UI element absorbed the geometry the defect depends on, so the first capture
attempt was green pre-fix. Any capture offered as regression evidence should be
run against the pre-fix component and required to fail.

[github-actions]

UX Review (Fable 5) — ✅ PASS

UX-level review of 40b915cf552ed7472204696b108a87a81cda1c43 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

UX-Verdict: PASS

Restores consistent selection behavior — last paragraph/line now works like every other; screenshots confirm the Comment pill renders correctly in both themes.

The shipped change is purely behavioral: triple-click on a spec document's last paragraph now raises the Comment pill, and copying a bubble's last line now expands paste chips instead of shipping the raw [ Paste #1 - 4 lines ] label — both closing a label-breaks-its-promise gap this PR fixes rather than introduces. No new UI strings, controls, or states reach users (harness copy lives under dev-only capture/); positioning is clamped so the pill anchors to the selection, verified in both dark and light captures.

[UX-REVIEWED] 40b915c

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 40b915cf552ed7472204696b108a87a81cda1c43 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Third occurrence of a shipped-and-proven predicate correctly centralized instead of copied; scope, tests, and evidence all match the stated problem.

[DESIGN-REVIEWED] 40b915c

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 40b915cf552ed7472204696b108a87a81cda1c43 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All checks complete. The helper has 3 real consumers, zero unfixed siblings of the predicate remain, and the capture/screenshot artifacts follow an established repo convention. Final review:

First-Principles-Verdict: PASS

Two reported last-line defects fixed at cause level — the wrong predicate is deleted everywhere it existed, replaced by one shared helper with three counted consumers.

What this change ships

Intent: make multi-click selections of a container's last line work at the two surfaces still carrying #7875's bug — a FIX.

1. Copying a bubble's last line now expands paste chips instead of shipping the chip label — justified (reported defect)
2. Comment pill now appears for a triple-clicked last paragraph — justified (reported defect)
3. Containment predicate extracted to selectionContainment.ts, shared by three call sites — justified (3 consumers counted; deletes fix(chat): judge selection containment by text, not ancestor (#7847) #7875's inline block)
4. SelectionToolbar reads the clamped measure range from the helper — net deletion, behavior pinned by 11 existing tests
5. New Chromium capture harness + script for the DocView case — declared; matches repo convention (171 harness files, 356 capture-*.mjs scripts)
6. Two screenshots committed under temp-screenshots/ — declared; matches convention (685 existing files)

Depth check: grepped contains(range.commonAncestorContainer) as an early-return predicate across website/ — 0 instances remain outside the helper's own fast path (the other four commonAncestorContainer uses are TreeWalker roots or ancestor lookups, not containment rejections). The fix removes the root cause — ancestry as a proxy for text containment — rather than patching each symptom, and the helper's three consumers are all wired in this change, so nothing is speculative surface.

[FIRST-PRINCIPLES-REVIEWED] 40b915c

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 40b915cf552ed7472204696b108a87a81cda1c43 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 40b915c

Verdict parsed from the review's SHA-scoped output markers for commit 40b915cf552ed7472204696b108a87a81cda1c43.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 40b915cf552ed7472204696b108a87a81cda1c43: <one-sentence reason>

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 40b915cf552ed7472204696b108a87a81cda1c43 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 40b915c

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 40b915cf552ed7472204696b108a87a81cda1c43: <one-sentence reason>

[buluoray]

Verdict: Approve — no blocking findings. Reviewed at head 40b915cf552ed7472204696b108a87a81cda1c43.

What I verified

• The change is not a string-occurrence match, despite the title. website/src/utils/selectionContainment.ts containedSelectionRange(range, container) operates on the live Range's real boundary points, not on a text search of the region. So the "same string appears twice → matches the wrong occurrence" failure mode does not apply: nothing is located by content. The only text it inspects is the overhang (the part of the range outside the container), and only to check it is whitespace-only.
• Boundary-spanning selections are still rejected. When commonAncestorContainer is hoisted out of the container, the helper clamps clones to each side (before.setEnd(container,0), after.setStart(container, childNodes.length)) and rejects if (before.toString()+after.toString()).trim() is non-empty. A selection that genuinely runs into a sibling carries that sibling's text in the overhang and is rejected in either direction. Verified by tests in website/src/test/selectionContainment.test.ts ("rejects a selection whose overhang holds a sibling's text") and by the "keeps dismissing…"/"keeps ignoring…" tests in SpecBuilderDocView.test.tsx and UserMessagePasteCov80.test.tsx.
• The single-occurrence (contained) path is byte-identical. if (container.contains(range.commonAncestorContainer)) return range returns the original range (identity, asserted with toBe(range)), so the three consumers keep their prior behavior on the common path: DocView.tsx still does contained.getBoundingClientRect(), UserMessage.tsx still does contained.cloneContents(), SelectionToolbar.tsx still measures from the returned range.
• The SelectionToolbar guard is an equivalent extraction, not a weakening. I diffed the removed inline block against the helper: identical tiers, identical clamping, identical .toString().trim() overhang check and O(1) !startInside && !endInside early reject. selectionContainment.test.ts pins the O(1) tier (cloneRange spy asserted un-called on a foreign selection).
• New tests redden on revert. The DocView/UserMessage "raises the pill / still expands the chip when normalized past the pane/bubble" tests exercise the exact branch the fix adds; reverting to the bare commonAncestorContainer check makes them fail (pill absent / copy not intercepted).
• Whitespace/zero-width handling is safe-by-direction. The overhang uses String.prototype.trim(), which removes NBSP; a zero-width char is not trimmed and reads as non-empty, so it errs toward rejecting (never a false-accept of foreign content). Empty/short selections are gated upstream (text.length < 3 in DocView, sel.isCollapsed in UserMessage).
• Signals: all check-runs green at head (cancelled runs superseded by same-named success). All five review bots (GPT 5.6, Opus 4.8, Design, UX, First Principles) show PASS / no-blocking on the current full SHA. No AUTOSDE blocking rule is tripped (no inline <svg>, no emoji-as-icon, no unlabeled icon button, no onClick div/span, no max-w-[9xx+px], no innerHTML/eval); the diff is logic + tests + an isolated capture harness + screenshots.

Findings

None blocking. No non-blocking findings worth raising as work.

What I could not verify

• I did not run the tests or the Playwright capture locally; I judged the fix and test coverage by reading source and confirming the assertions bracket the changed branch in both directions.
• I did not exhaustively audit website/AGENTS.md for a spec-update obligation, but this is a bug fix to existing selection behavior (no new feature or documented surface), and no new i18n catalog strings are introduced.

[bolichen97]

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

• PR #7911 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #7911: CONTINUE_DEVELOPMENT. Merged main does not implement double-click-to-edit at all, so nothing is superseded. It does, however, make the reviewers' selection objection concrete: guard startEdit() on a collapsed window.getSelection() inside the bubble (or scope the trigger away from the prose), so a double-click that selected text keeps that selection. Files: website/src/pages/chat/UserMessage.tsx.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.