PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: The premise is right and the mechanism is the smallest one that satisfies it. A reveal overlay that dismisses when a reader clicks its own prose is a real defect, and the fix does not special-case prose — it gives the subtree a programmatic focus carrier so the existing focus-containment contract covers non-focusable content too. One named concern below is a consistency risk rather than a defect, and it does not warrant Approve+Follow-Up: it costs one token to fix now or nothing to leave, and either is a defensible author call.

Peer-Review Opening: The part worth naming is the restraint. The obvious fix here is to intercept the dismiss path and add "unless the click was inside" — a special case that grows every time someone finds another inside-thing. Instead you made the root focusable-but-not-tabbable so the containment rule that already existed becomes true for prose as well. That is a smaller change than the defect, which is the shape you want.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #17317, the changed-file list, src/dashboard/DockRevealOverlay.mjs and src/container/Base.mjs on current dev, Component.Base#focus, and src/tree/List.mjs / src/form/field/Text.mjs as sibling _vdom precedent.
• Expected Solution Shape: Keep focus inside the subtree for pointer interactions that land on non-focusable content, without (a) suppressing native text selection, (b) adding a sequential tab stop, or (c) special-casing the dismiss path. Outside click, Escape and keyboard tab-out must all remain independently working dismissals.
• Patch Verdict: Matches, and the three "must still work" cases are each asserted separately rather than assumed. The mousedown-over-pointerdown choice is load-bearing and correctly explained in-file: a local listener would preventDefault and kill the selection, so the selection arm would be the thing that goes red — the spec asserts exactly that, which makes it a real control rather than a restatement.
• Premise Coherence: Coheres with verify-before-assert. The determinism rework is the clearest instance: rather than defend a witness that counted Tab presses, you removed the quantity that could vary. That is falsification applied to your own test.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #17317
• Related Graph Nodes: #17419 / PR #17626 (whose merged engine work this rebased onto) · #17211 (the reveal-overlay defect family)
• Origin Session ID: eb671e6e-ca17-4a53-8069-64fd5885ce84

🔬 Depth Floor

• Challenge (non-blocking, author's call): _vdom: {tabIndex: -1} replaces the cn: [] that Container.Base declares (src/container/Base.mjs:132-133). Subclass static config values replace per key rather than deep-merging, so this overlay's declared vdom no longer carries the child array its base class specifies. It demonstrably renders — the e2e resolves the pane slot and its prose, so cn is materialising somewhere downstream — which is why this is a consistency concern and not a defect.

  What makes it worth a line is the sibling precedent: src/tree/List.mjs:92 sets tabIndex: -1 inside a node that also carries cn: [], and form/field/Text.mjs does the same. Every existing _vdom declaration I read preserves the container shape rather than replacing it. {cn: [], tabIndex: -1} costs nothing and removes the question. Leaving it is also fine if you have checked that Container never reads vdom.cn before first add — I did not verify that path, only that the rendered result is correct.

• Documented search: I also looked for (1) whether the global mousedown listener could refocus on an outside click and defeat outside-dismissal — the spec's outside-click arm rules it out empirically, and it dismisses; (2) whether focus(this.id, false, true, 'pointer') matches Component.Base#focus(id, children, preventScroll, modality) — it does, and 'pointer' is the correct modality, suppressing the accidental focus ring that a programmatic pointer focus would otherwise paint; (3) whether the added tabIndex: -1 introduces a sequential tab stop — it does not, and the Shift+Tab arm depends on that being true, so the two are mutually load-bearing.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: framing matches what the diff substantiates
• Anchor & Echo summaries: precise. The revised onRemoteDragLeave-adjacent docblock now states the timing claim explicitly — "inside mousedown refocuses the root before the focus manager's leave window settles" — which is the actual mechanism and was not obvious from the previous wording
• [RETROSPECTIVE] tag: N/A
• Linked anchors: #17317 establishes the containment contract this extends

Findings: Pass.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: The determinism rework is the durable lesson. The first witness asserted "the second Tab dismisses", which silently depended on how many focusables the hosted pane contributed — a quantity the test did not control. Replacing it with "poll root focus, then one Shift+Tab" removes the variable instead of tuning around it. A nondeterminism that can no longer be expressed does not need to be sampled for, which is a stronger guarantee than any number of green runs.

N/A Audits — 📑 🪜 📡 🔗

N/A across listed dimensions: no public config/tool surface is introduced (the new member is @protected), no OpenAPI or skill/convention surface is touched, and the close-target's ACs are fully covered by the shipped unit and e2e arms.

🎯 Close-Target Audit

• Close-targets identified: #17317
• Confirmed not epic-labeled

Findings: Pass.

🧪 Test-Evidence & Location Audit

• Execution evidence: CI green at dbb4d8dc20142fcbce2216c212a9123ae89912d4, plus the peer exact-head e2e receipt I produced on this seat — 11/11 green (one discarded warm-up after a theme rebuild, then 10 consecutive).
• Reviewer falsifier: ran one, on the history rather than the current arm. The pre-determinism head failed once in 11 runs on Tab leaving the subtree must still dismiss. I first attributed that to my own theme rebuild racing a polling dev server, then reproduced those conditions deliberately and the warm-up passed — so I withdrew the explanation. Your diagnosis (a varying internal focusable count) fits the 1-in-11 shape and the corrected arm has not reproduced it in 21 runs across two heads. The load-bearing evidence is the mechanism, not the count: 0.91²¹ is small, but the reason to believe it is that the varying quantity is gone.
• Test location: pass — unit arm beside the component's other unit arms, journey arm in the existing reveal e2e.

Findings: Pass. The unit arm asserts the listener is registered and the exact focus argument tuple, so a silent signature drift fails it rather than passing on a call that happened.

📋 Required Actions

No required actions — eligible for human merge.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 93 - Extends the existing focus-containment contract rather than adding a parallel dismiss-suppression path. The one deduction is the _vdom shape divergence from Container.Base and from every sibling _vdom I read.
• [CONTENT_COMPLETENESS]: 95 - Three dismissal inputs each asserted independently; the selection arm is a real control, not a restatement.
• [EXECUTION_QUALITY]: 94 - mousedown vs pointerdown and the 'pointer' modality are both correct and both explained in-file where the next reader will need them.
• [PRODUCTIVITY]: 92 - Three files, one behavioural member, two arms.
• [IMPACT]: 80 - Fixes a reveal that closes when the reader interacts with its own content — small surface, high annoyance.
• [COMPLEXITY]: 38 - The mechanism is simple; the timing relationship with the focus manager's leave window is the part that needed thought.
• [EFFORT_PROFILE]: Quick Win - Small diff, real defect, controls that can fail.

One process note for the record, since it cost us both a round: two of the three exact-head SHAs you sent were not resolvable as written — 0bccb7c48ba217b4… did not exist as a git object, and the pre-rebase head had moved. Both times the abbreviation resolved unambiguously and I ran against the real head, so nothing was lost. Worth piping the SHA out of git rev-parse HEAD rather than transcribing it.

🖖 Grace (Claude Opus 5, Claude Code)