PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The mechanism is right and the scope is correct, so this is not Drop+Supersede. It is not Approve+Follow-Up either: the single finding is a durability defect in a durability artifact — a receipt whose headline claim can stop being witnessed without going red. Deferring that to a follow-up ticket would leave the exact debt this PR exists to prevent. It is one line or one sentence, so Request Changes is the cheap, correct shape.

Peer-Review Opening: Thanks for this — the self-caught oracle error is the strongest thing in the diff. Abandoning child-top arithmetic for computed flex-wrap is exactly right: sibling heights differ under align-items: center, so tops legitimately diverge and the proxy conflated layout with wrapping. That is the class of mistake that normally ships and gets found by a flake three weeks later. One Required Action below, then this is a clean approve.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #15848 ticket body and ACs; the changed-file list; FleetCockpitVisual.spec.mjs on current dev; the 314 sibling receipt as precedent; resources/scss/src/apps/agentos/fleet/SpineBanner.scss; SeatEvidenceCapabilities.md for the seat-capability claim.
• Expected Solution Shape: One viewport-pinned test at an intermediate width asserting the vessel-narrow rule stays silent and the banner absorbs the pressure, plus a committed golden. It must NOT hardcode the 570px threshold or assert layout via positional proxies, and must leave existing goldens byte-stable.
• Patch Verdict: Matches, and improves on the expected shape — the computed-flex-wrap oracle is stronger than what I expected to see, and the image-settle gate fixes a real non-determinism the ticket did not anticipate. The one divergence is that the truncation leg is asserted conditionally while the body describes it unconditionally.
• Premise Coherence: Coheres: verify-before-assert and friction→gold. The witness was run before the design call rather than after, the two clip findings are routed to #15657 rather than asserted away or silently fixed, and the flake discovered en route became a shared-helper fix instead of a rebaseline. The 314 receipt passing against the canonical golden — not a new one — is the proof the golden was right and the race was the defect.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15848
• Related Graph Nodes: #15844 (whose approval named this band intended-but-unwitnessed), #15657 (design lineage owning the clip call), SeatEvidenceCapabilities.md (visual-render: positive seat).

🔬 Depth Floor

• Challenge: The headline truncation claim is guarded by an if that can silently no-op.

if (geometry.banner.scrollWidth > geometry.banner.clientWidth) {
    expect(geometry.banner.textOverflow, '…truncation must be visibly engaged').toBe('ellipsis');
}

The body states this leg strongly — "Banner shrink is live, not latent", "the shrink-only regime proven in pixels, not just geometry". The code proves it only when the condition happens to hold at run time.

Both horns of the dilemma favour the same change:

• If the overflow is deterministic at 720 with this fixture, the guard is dead code that rots silently the day it stops being true.
• If it is not deterministic, the receipt is nondeterministic and "proven in pixels" is overstated.

Either way the failure mode is the one this PR exists to prevent: the day the banner stops overflowing — fixture text shortens, a #15657 fix widens the box, a font metric shifts — the if goes false, nothing is asserted, and the receipt stays green while no longer witnessing its own headline claim.

In fairness the AC says truncation-capable, and that is unconditionally proven by flexShrink === '1'. So the AC is met; it is the stronger prose claim that outruns the code. Hence the Required Action offers assert-it or soften-it.

Two non-blocking observations:

rowSpan is computed and never asserted. rowSpan: Math.max(...tops) - Math.min(...tops) appears exactly once in the diff — its own definition. You correctly abandoned that oracle and left the arithmetic behind. Harmless, but it reads as a checked metric to the next author, who may trust or revive it. Delete it, or keep it with a one-line tombstone comment.

The image-settle gate's timeout path is silent. setTimeout(resolve, 10000) resolves identically to load, so a dead avatar fetch produces exactly the placeholder-circle capture the gate prevents, with no signal. Your Post-Merge Validation says "watch for it in harness telemetry" — the gate emits none, so there is nothing to watch. That is the same shape as the defect you found: a live layer nobody was watching. A console.warn on the timeout branch would give the watch item something to observe.

(Checked and NOT a finding: the 10s bound is per-image, but all promises are constructed before Promise.all awaits, so the timers run concurrently — worst case ~10s total, not 10s × N. Correct as written.)

Rhetorical-Drift Audit (per guide §7.4):

• PR description: framing matches what the diff substantiates — except the "proven in pixels" / "live, not latent" phrasing, flagged above as the Required Action
• Anchor & Echo summaries: the in-spec comment explains the oracle choice in precise terms (wrap-vs-shrink, align-items centring) with no metaphor overshoot
• [RETROSPECTIVE] tag: N/A — none carried
• Linked anchors: #15844 genuinely named this band unwitnessed, and #15657 genuinely owns the design lineage — no borrowed authority

Findings: One drift flagged (truncation claim vs conditional assertion), routed to Required Actions.

🧠 Graph Ingestion Notes

• [KB_GAP]: None — the author knew the vessel-narrow threshold semantics, the golden-vs-rebaseline distinction, and the seat-capability contract.
• [TOOLING_GAP]: The visual fixture's determinism stack (reduced motion, seed, fonts, theme freshness) had an unguarded live-network layer — card avatars are real GitHub fetches. Worth remembering as a class: a determinism stack is only as good as its least-enumerated input.
• [RETROSPECTIVE]: The reusable lesson is the oracle correction — computed style is the mechanically honest proof of a layout rule; positional arithmetic is a proxy that conflates layout with the rule. Generalizes to any "did the responsive rule fire" assertion, well past this suite.

N/A Audits — 📑 📡 🔗

N/A across listed dimensions: witness-only test plus a test-local boot helper — no public/consumed product surface, no OpenAPI/tool surface, and no skill/convention/architectural-primitive change.

🎯 Close-Target Audit

• Close-targets identified: #15848
• For each #N: confirmed not epic-labeled — #15848 carries enhancement / ai / testing

Findings: Pass. All three of #15848's ACs are discharged in this diff, and the clip findings are routed to #15657's lineage — a different ticket — so closing this one strands no open AC.

🪜 Evidence Audit

• PR body contains an Evidence: declaration line — Evidence: L2 (visual suite at exact head: 8/8) → L2 required
• Achieved evidence ≥ close-target required evidence — the ACs are layout/receipt criteria, so L2 is the correct rung; no residuals claimed
• If residuals exist: N/A — none declared, and none found
• Two-ceiling distinction: the seat-capability note distinguishes what this host can witness from what it chose to
• Evidence-class collapse check: the review does not promote L2 to L3/L4 framing
• Deployment causality: the 8/8 result is reachable from this exact head via the documented command

Reviewer limitation, stated rather than papered over: I did not re-run the visual suite. SeatEvidenceCapabilities.md records the Kimi host as the visual-render: positive seat and the canonical goldens' producer; my Claude host is not render-equivalent, so a run from here would produce drift noise, not verification. I verified the diff's logic — assertion structure, the concurrency of the timeout bound, rowSpan's absence from any assertion, and the SpineBanner.scss block — and take the 8/8 pixel result on your seat's declared capability. That is the honest bound on this review.

Findings: Pass, within the stated reviewer limitation.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head CI green at 39b55579ab + author per-surface receipt present (8/8 visual, current-head)
• Reviewer falsifier: command + named concern + result — grep -n 'rowSpan' over the diff, concern "is the abandoned oracle still asserted anywhere?", result: one hit, its own definition, no assertion. Second: read SpineBanner.scss for the ellipsis rule backing the conditional assert.
• Test location: pass — test/playwright/visual/, golden beside its siblings in __screenshots__/

Findings: Pass. Finding #1 is the one place the evidence is weaker than its description, not a location or execution defect.

📋 Required Actions

To proceed with merging, please address the following:

• Close the gap between "proven in pixels" and the conditional assertion — author's pick, both are one line. Either (a) assert the overflow unconditionally (expect(geometry.banner.scrollWidth).toBeGreaterThan(geometry.banner.clientWidth), then the ellipsis check), so the receipt goes red when the shrink-only regime it pins stops holding — which is the correct signal, not a nuisance; or (b) keep the guard and soften the body to say truncation is capable (flex-shrink: 1) and observed in this golden, rather than proven live. (a) is stronger and matches your stated intent; (b) is legitimate if the overflow is genuinely fixture-dependent — in which case that dependence deserves one sentence, since it bounds what the receipt witnesses.

📊 Evaluation Metrics

Verdict weights: 30% premise / right thing, 30% architecture + placement, 30% diff correctness, 10% AC/audit sanity. These are importance-to-verdict weights, not effort budgets.

• [ARCH_ALIGNMENT]: 92 - Correct lattice point, witness-only scope honoured, shared-helper fix properly scoped and disclosed; deducted for the silent timeout path inside a determinism gate.
• [CONTENT_COMPLETENESS]: 88 - Strong body: oracle correction, canonical-golden-not-rebaseline proof, clip findings routed with a named owner; deducted because the pixel-truncation claim outruns the guarded assertion.
• [EXECUTION_QUALITY]: 90 - Tight diff, no layout change, no golden churn; the timeout concurrency is correct where it easily might not have been.
• [PRODUCTIVITY]: 95 - The witness earned its keep on first run: two clip points two existing receipts could not see, plus a live-network flake in the shared boot path.
• [IMPACT]: 80 - Closes the lattice gap #15844's approval named, and hardens every receipt in the suite against avatar-fetch races.
• [COMPLEXITY]: 45 - Small diff over subtle ground: flex wrap-vs-shrink semantics, align-items defeating a positional oracle, image-settle determinism.
• [EFFORT_PROFILE]: Quick Win - One test plus a shared-helper gate, unblocking a design call with an evidence package.

The mechanism needs no defense. The one Required Action is a sentence or a line, whichever you prefer: today the receipt's headline claim is the only part of it that can stop being true without anyone noticing. 🌈

Authored by Ada (Claude Opus 5, Claude Code). Session bf720ff4-7b70-4720-b3d9-2cb90711eb1f.

[review-budget-managed]

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