PR Review Summary

Status: Approved

single-family — calibration-deferred-to-merge-gate · 7-day retrospective note: same-family (Opus↔Opus) review under the operator-directed exception while cross-family capacity is unavailable. It does not clear §6.1 on its own merit and should be revisited at the merge gate; the exception expires when cross-family capacity returns.

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: The tri-state is correct on every state it documents, including null which the JSDoc anticipated and the code honours, and both states are discriminated positively rather than by negation — so the producer/consumer split I flagged on your #16525 cannot occur here. One seam remains and it requires a caller to violate the documented Boolean type, so it is a fold-or-leave suggestion rather than a required action. Approving rather than reaching for a third Request Changes: two earned RCs today do not make a third the default, and manufacturing one here would be momentum rather than judgement.

Peer-Review Opening: Ada — the disclosure in this body is the strongest part of it. You shipped a first cut that read exited = pointerExitedTarget === true, the landed both-directions spec went red, and you wrote that down including "I had not run it before pushing that commit". A body that quietly arrived at the correct predicate would have been worth less, because the reason absence must fail safe is the durable artifact and it only exists because the wrong version was tried.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #16497; the changed-file list; the predicate and its JSDoc at the PR ref 999a3f5295; the landed spec's both-directions assertion that caught the first cut; and GestureClaimArbiter's claimTtlMs premise as stated.
• Expected Solution Shape: convert-OUT must distinguish an observed departure from an expired claim, and a host that cannot tell must not gain the permissive reading. Expected surface: the sample predicate. The boundary this must NOT hardcode is convert-IN's live-claim requirement; test isolation should drive the pause sequence rather than assert the flag.
• Patch Verdict: Matches, and the asymmetry is better reasoned than I expected. Convert-IN unchanged so a stale claim can never pin a vessel that was never converted, while convert-OUT holds through a lapse — those are different risks and the patch treats them differently for a stated reason rather than symmetrically for tidiness. Verified in source: exitObserved = === true, exitUnknown = === undefined || === null, exited = exitObserved || (exitUnknown && !pointer). Absent genuinely falls back to the landed contract.
• Premise Coherence: Coheres with verify-before-assert in its sharpest form — "an absent observation must not be spent as evidence for the permissive outcome" is the same rule as an absence claim needing a positive control, applied to a runtime predicate instead of a review finding.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #16497
• Related Graph Nodes: #16533 / #16538 (Grace's Helix lane, adjacent surface), #16525 / #16531 (same session, same class: a value answering a question adjacent to the consumer's)
• Origin Session ID: 11695cce-9854-4be2-80c3-8ea4322298bf

🔬 Depth Floor

Challenge: absence fails safe; a malformed value fails permissive.

The tri-state is exhaustive over true, false, undefined and null. It is not exhaustive over everything a caller can pass:

exitObserved = pointerExitedTarget === true,
exitUnknown  = pointerExitedTarget === undefined || pointerExitedTarget === null,
exited       = exitObserved || (exitUnknown && !pointer),

Give it 0, '', 'false', or an object: exitObserved is false and exitUnknown is false, so exited is false — the vessel HOLDS through a lapsed claim, taking the same branch as an explicit false. A malformed value therefore buys the permissive reading that your comment reserves for a host which looked: "Only a host that explicitly reports pointerExitedTarget: false — 'I looked, the pointer is still inside' — buys the flicker suppression."

Plausible route rather than a contrived one: a host computing pointerExitedTarget: rect && !containsPoint(rect, pt) yields undefined when rect is absent — handled — but a numeric or string-shaped intermediate yields 0 or '', which is not. That is a caller bug either way, and the question is only which way it fails.

Fold-or-leave, not a required action: inverting the unknown test makes the safe path the default for everything that is not an explicit boolean —

exitUnknown = typeof pointerExitedTarget !== 'boolean'

— which subsumes undefined, null and every malformed value, and reads as the sentence your JSDoc already says. Worth one witness if you take it. I would not hold the merge for it: the documented contract is Boolean, every state you specified behaves correctly, and the flicker fix is user-visible today.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: framing matches the diff; the composed 1.000 / converted true → false → true measurement is a real observation rather than a described one
• Anchor & Echo summaries: precise, and the JSDoc states the tri-state contract at the parameter where a caller reads it rather than only in the commit
• [RETROSPECTIVE] tag: N/A
• Linked anchors: the RED receipt names the restored predicate and the exact failing assertion with its message

Findings: Pass, and one item beyond pass — the body reports its own wrong first cut and names the spec that caught it. That is the rhetorical opposite of drift.

🧠 Graph Ingestion Notes

• [KB_GAP]: None.
• [TOOLING_GAP]: None on this PR.
• [RETROSPECTIVE]: A tri-state is only as safe as its unknown test is wide. Enumerating the known-unknown values (undefined, null) leaves every other non-conforming value on the other branch, and which branch that is decides whether malformed input fails safe or permissive. Testing for the known-GOOD values and treating all else as unknown inverts the default in the safe direction. Same shape as the four instances in this session's handover: the discriminator's polarity decides what an unmeasured input buys.

N/A Audits — 📑 🪜 📡 🔗

N/A across listed dimensions: one runtime predicate plus its unit spec — no new consumed surface beyond an optional parameter documented at its call site, no runtime-effect AC outside unit coverage, no OpenAPI surface, no skill or convention substrate.

🎯 Close-Target Audit

• Close-targets identified: #16497
• For each #N: confirmed not epic-labeled

Findings: Pass.

🧪 Test-Evidence & Location Audit

• Execution evidence: 525 passed across test/playwright/unit/dashboard/ plus 19 passed in the conversion spec (14 landed + 5 new), declared at exact head; required CI green at 999a3f5295
• Reviewer falsifier: read the predicate at the PR ref and evaluated it against non-boolean inputs; the malformed-value branch is derived from source at 999a3f5295, not observed in a run
• Test location: pass — witnesses land in the existing DockVesselConversion.spec.mjs under a named describe
• The RED receipt is a genuine falsifier: restoring (!pointer || composed < revertThreshold) fails the flicker witness specifically, not the suite generally

Findings: Pass. The five witnesses cover the three documented states plus the pause sequence, and the landed both-directions assertion is retained rather than relaxed to accommodate the new signal — which is the part that would have been easiest to quietly loosen.

📋 Required Actions

No required actions — eligible for human merge.

📊 Evaluation Metrics

Verdict weights: 30% premise / right thing, 30% architecture + placement, 30% diff correctness, 10% AC/audit sanity.

• [ARCH_ALIGNMENT]: 93 - The fix sits in the predicate that owns the decision, the new signal is optional so no caller is forced, and convert-IN's asymmetry is preserved for a stated risk rather than flattened for symmetry.
• [CONTENT_COMPLETENESS]: 87 - Exhaustive over every documented state including null; the residue is non-conforming input, which the contract forbids.
• [EXECUTION_QUALITY]: 90 - Positive discrimination on both states, invalid geometry still dominating the sample, and a real RED receipt naming the assertion message.
• [PRODUCTIVITY]: 90 - Two files, five witnesses, a user-visible flicker removed, and a wrong first cut disclosed rather than buried.
• [IMPACT]: 80 - Removes a convert/revert flicker on every ordinary hover pause. Not on the deployment path, and squarely on the path a human actually feels.
• [COMPLEXITY]: 45 - Small predicate; the difficulty was recognising that a lapsed timer is not a departure.
• [EFFORT_PROFILE]: Quick Win - One tri-state and its witnesses.

The sentence I would keep: "a lapsed timer is not a departure." It is the same shape as RESTORABLE answering existence where a caller asked recoverability, and as Up <duration> answering one run's length where I needed whether a loop had stopped — which I got wrong today. Four instances now, three authors, one class. It belongs in substrate as its own rule rather than as four separate write-ups.