PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The implementation is on the correct owning seam and is salvageable in place, so Drop+Supersede would be disproportionate. The ticket explicitly required classification, and the classification legitimately falsified its initial registration hypothesis. The remaining defects are bounded: one lifecycle leak in the race implementation plus source-ticket, ledger, and evidence truth-sync.

Peer-Review Opening: The classification is substantive: current dev confirms the Sparkline registration triangle is message-driven, while DockFlip contains the raw paint-gated awaits that can wedge the tour. The two-environment repair is directionally right; the items below close the remaining lifecycle and graph-truth gaps.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: Issue #16425 body and both classification receipts; changed-file list; current dev versions of Neo.component.Canvas, Neo.main.DomAccess, Neo.worker.Canvas, Neo.worker.App, Neo.main.addon.DockFlip, and Neo.main.addon.ResizeObserver; the #16402 Contract Ledger precedent; DockFlip consumers in Workstation, FleetCockpit, DemoA, and DemoB; related issues #16391, #16434, and #16435.
• Expected Solution Shape: Registration should remain unchanged if its main → canvas-worker → app-worker acknowledgement chain is already paint-independent. The actual raw-rAF owner should receive an environment-aware liveness guard: an intentional hidden-document instant path, a timer-raced visible/occluded frame path, symmetric cancellation of whichever race arm loses, and witnesses that distinguish presenting motion from starvation settlement without screenplay delays.
• Patch Verdict: Improves the expected shape and lands at the right main-addon seam. The hidden discriminator, timer dam, release hard-cut, and test placement match; however, the timer-win branch leaves every losing rAF callback registered, unlike the sibling ResizeObserver dam that disarms both arms.
• Premise Coherence: Coheres with verify-before-assert because instrumentation falsified the filing hypothesis before modifying the registration triangle. The public graph is not coherent yet: #16425 still names registration starvation as fact and lacks the contract ledger for the remote play() behavior it will close.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #16425
• Related Graph Nodes: #16402 · #16391 · #16434 · #16435 · #15137
• Origin Session ID: 059d5e0b-716b-4e5f-967c-3e6263109283

🔬 Depth Floor

Challenge OR documented search (per guide §7.1):

• Challenge: At exact head 480966acc1, DockFlip.mjs:431-445 clears the timer when rAF wins, but the timer callback only nulls its own handle and resolves false; it never retains or cancels the rAF ID. In a long-lived occluded-but-visible window, each timer-won wait therefore leaves a callback queued until a future frame, and repeated dock operations accumulate them. The sibling ResizeObserver implementation explicitly cancels both race arms before dispatch, so this is a lifecycle defect rather than an optional optimization.
• Documented search: I also checked the intentional hidden hard-cut, transform/opacity-only motion, token-owned transition duration/easing, presenting-path frame witnesses, exact close target, registration handshake, consumer set, and current exact-head CI. No second code-path blocker surfaced.

Rhetorical-Drift Audit (per guide §7.4):

• PR description fully matches current repository truth: the Commits section still names a4a013aa1b, which is not the current head 480966acc1.
• Source-ticket framing matches the classified mechanism: #16425 still presents Sparkline registration starvation as the problem even though exact source plus both receipts falsify it.
• Anchor & Echo JSDoc names the hidden versus occluded carrier split and does not promote the 100ms dam into a motion-duration token.
• Linked #16402 precedent actually establishes a timer-raced rAF dam with losing-arm cancellation.

Findings: Drift is bounded but blocking because the PR uses Resolves #16425 and the stale source claim would become closed graph truth. Required Actions 2-3 truth-fold it.

🧠 Graph Ingestion Notes

• [KB_GAP]: Classification comments currently carry the corrected mechanism while the close-target body and title retain the falsified registration claim.
• [TOOLING_GAP]: This harness exposes the managed GitHub review path but not Memory Core or Agent Network add_message, so identity-bound merge-readiness projection and the required A2A lifecycle hand-off cannot be completed here.
• [RETROSPECTIVE]: A paint-independent race owns both arms. Cancelling the losing carrier is part of the liveness contract because an occluded surface can remain frame-starved across arbitrarily many dock operations.

🎯 Close-Target Audit

• Close-targets identified: #16425
• #16425 is open and not epic-labeled.

Findings: Label shape passes. Semantic close readiness remains blocked until the ticket is truth-folded to the classified DockFlip owner and its consumed-surface contract.

📑 Contract Completeness Audit

• Originating ticket contains a Contract Ledger matrix.
• Implemented PR diff matches the Contract Ledger exactly.

Findings: #16425 has no Contract Ledger, while this PR changes the behavior of remote app-scoped DockFlip.play(), consumed by Workstation, FleetCockpit, DemoA, and DemoB. Backfill the ledger and bind the hidden-entry, occluded-dam, presenting-frame, return-value, and fallback semantics before approval.

🪜 Evidence Audit

• Achieved evidence meets the written close-target runtime floor: current exact-head CI is green; the author provides a CI-runnable starved-rig E2E and a live hidden-pane receipt.
• The synthetic witness uses bounded state polling rather than fixed-delay settlement.
• The real-pane overflow residual is independently owned by #16434; the >5-minute timer-carrier audit is independently owned by #16435.
• The PR body uses the canonical one-line evidence declaration and identifies its current exact head.

Findings: Evidence class is sufficient for the written #16425 ACs, but the declaration and commit anchor are stale/noncanonical. Required Action 3 restores parser-friendly exact-head truth after the repair.

N/A Audits — 📡 🔗

N/A across listed dimensions: no MCP/OpenAPI surface, skill convention, turn-memory substrate, or new cross-skill workflow is introduced.

🎞️ Demo-Surface Motion Audit

• Hard-cut decision: Pass. Hidden-document instant landing is intentional, named, and scoped to a document that cannot present motion.
• Motion properties: Pass. The rendered transition remains transform/opacity-only.
• Token discipline: Pass. Motion duration/easing remain sourced from dashboard motion tokens; 100ms is a liveness race budget, not a call-site animation duration.
• Motion evidence: Pass. Existing unit frame-count pins plus the authored headed DockFlip witnesses exercise presenting behavior; the starved witness exercises the no-frame branch.

Findings: No motion-contract Required Action.

🧩 Core-Idiom Audit

• Instance/reactive state: Pass. No chained reactive-config mutation, bespoke instance lookup, or provider-state drift is introduced.
• Lifecycle ownership: Fail narrowly at #nextFrame: the timer arm wins without cancelling its outstanding rAF arm.

Findings: Required Action 1.

🧪 Test-Evidence & Location Audit

• Execution evidence: all 12 current exact-head checks are green at 480966acc119549cd0eb564d28a226f9264cf7ff; required integration-parity is green. Author receipts cover DockFlip unit behavior, the starved Workstation tour, sibling starvation, and presenting motion.
• Reviewer falsifier: exact-head source-arm census for #nextFrame plus comparison with ResizeObserver.dispatchResizeEvents(). Result: failed — timer-win has no cancelAnimationFrame path, and the new unit stubs count requests without asserting cancellation.
• Test location: the DockFlip unit extension and Workstation whitebox E2E are in their canonical suites.

Findings: Test placement and broad evidence are sound; the missing losing-arm lifecycle witness is part of Required Action 1.

📋 Required Actions

To proceed with merging, please address the following:

• Make #nextFrame a symmetric race: retain the rAF ID, cancel it when the timer dam wins, keep clearing the timer when rAF wins, and add a discriminating unit witness that records cancellation and proves timer-won waits leave no late frame callback queued.
• Truth-fold #16425 before it is closed: amend its title/problem/fix/AC framing to name DockFlip.play as the classified owner, explicitly preserve the Sparkline premise as falsified archaeology, and add the required Contract Ledger for the remote play() behavior and its four consumer families.
• Refresh the PR body at the repaired exact head: remove or update the stale a4a013aa1b commit anchor, use the canonical Evidence: L (...) → L required (...). Residual: ... line, and publish current-head unit plus starved-tour receipts.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 90 — correct main-addon ownership, environment split, sibling precedent, and fail-safe boundary; symmetric carrier disposal is the remaining architectural miss.
• [CONTENT_COMPLETENESS]: 74 — strong code-level JSDoc and evidence narrative, but the closing ticket is semantically stale, the consumed API lacks its ledger, and the PR carries an obsolete commit anchor.
• [EXECUTION_QUALITY]: 82 — exact-head CI and layered runtime witnesses are strong; timer-win callback retention is a real long-lived lifecycle defect.
• [PRODUCTIVITY]: 88 — the classified synthetic-tour wedge is repaired and the registration hypothesis is decisively retired; graph truth and one race invariant remain.
• [IMPACT]: 88 — DockFlip is a shared presentation seam across Workstation, Fleet, and both docking demos, and its promise liveness gates awaited consumer work.
• [COMPLEXITY]: 84 — browser visibility classes, rAF/timer scheduling, worker-driven projection, FLIP cleanup, and live versus synthetic evidence interact.
• [EFFORT_PROFILE]: Heavy Lift — a focused diff, but cross-surface scheduler semantics and multi-layer witnesses make the reasoning load substantial.

The repair direction is right. One code-lifecycle correction and a complete truth-fold should make the next review a narrow exact-head delta pass.

📨 A2A Hand-Off

Recipient: @neo-fable. Agent Network add_message is unavailable in this harness, so this review records the undelivered lifecycle hand-off rather than claiming it occurred.

[review-budget-managed]

• outcome: within-budget
• ordinary-limit: 2
• activation-issue: 15257
• activation-pr: 15307
• activated-at: 2026-07-16T20:54:31Z