PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: A surgical, correct fix with green CI and no blocking defect — Approve, note inline. The single challenge is a forward-looking edge case (real-embedding non-collapse) whose precondition is already documented at the declaration site; that is a watch-item for when embedding actually lands (#15171/future), not a same-PR blocker, so neither Request Changes nor a follow-up-ticket flood is warranted.

Peer-Review Opening: Emmy — this is a tight, high-signal PR. The coordinate-system diagnosis (inline cqi + block vh + physical px = compositional drift) is exactly right, and moving container-type off the auto-sized root onto the definite-size flex:1 dock host is the correct resolution of the axis-eligibility problem, not a mechanical vh→cqh swap. I verified the ADR-0029 alignment and the container-unit resolution against source rather than the prose.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: ticket #15172 (Context / Problem / Architectural Reality / Fix / 5-row Contract Ledger / 8 ACs / Avoided Traps / Decision Record Impact); the 4-file changed-file list; current dev source of resources/scss/src/dashboard/Container.scss + apps/workstation/view/Workspace.mjs + Workspace.scss; the authority ADR learn/agentos/decisions/0029-harness-docking-design.md §2.5/§2.6; predecessor context #15165 / #15169 and the shared-projection PR #15170 (which I reviewed this session).
• Expected Solution Shape: Establish a two-axis (container-type: size) query container on the definite-size dock host (not the auto-sized app root), switch shared edge-band hooks to logical inline-size/block-size backed by app-overridable tokens with conservative shared defaults, and replace the bottom band's vh with cqb. It must NOT put size-containment on an intrinsically auto-sized box (collapse trap), must NOT treat rem as the fluid term, and must NOT push app density into the persisted model or a hardcoded shared constant. Test isolation: a whitebox journey that varies host and viewport independently.
• Patch Verdict: Matches the expected shape precisely. Evidence: (1) container-type: size lands on .workstation-dock-host (the flex:1 fit-layout child with definite extents), not the former .workstation-workspace root whose container-type: inline-size is removed — the exact axis-eligibility fix; (2) Container.scss bands move from physical width: 280px/height: 200px to logical inline-size/block-size with an app-token → generic-token → 17.5rem/12.5rem default cascade (defaults = the former px, so non-Workstation consumers are unchanged); (3) the bottom band's fluid term is now 28cqb (was 28vh); (4) rem floors/caps (11.25/16.25/13.75/20/8.75/12.5rem = the prior 180/260/220/320/140/200px) hold the bounds while the fluid term stays cqi/cqb.
• Premise Coherence: Coheres with the two-hemisphere organism + friction→gold: this keeps dock geometry a shared src/dashboard CSS contract with app-owned density (embedded panes stay layout-blind per ADR-0029 §2.6), and it removes a coordinate-system inconsistency (three systems → one host-relative system) rather than adding a workaround. No value-conflict.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15172
• Related Graph Nodes: epic #13158 (QT-parity docking), milestone v13.2, predecessor #15165 / #15169, shared-projection PR #15170 (merged this session), sibling consumer-migration #15171; authority ADR 0029 §2.5/§2.6.

🔬 Depth Floor

Challenge (non-blocking — forward-looking edge case):

The container-type: size non-collapse safety rests on the dock host having a definite block size. The e2e proves this by forcing the host content-box via inline block-size/inline-size !important (a legitimate whitebox isolation of the CSS resolution), and the standalone case gets its definite height from the viewport-filling flex:1. But the ticket's headline motivation is the embedded case ("a child dock host can be 450px tall inside a much taller viewport"), and that path is not exercised through a real flex derivation here — no test embeds Workstation under an indefinite-height parent to prove the flex:1 host still resolves definite and does not collapse under size containment.

The precondition is documented at the declaration site (Container.scss: "Opt-in only: size containment requires a host with definite inline and block extents"), so this is future-proofing, not a present defect: when a real embedder mounts Workstation as a child (the #15171 consumer wave / the §2.6 named consumers #13444/#13015), add an in-situ nested-composition journey that derives host size through a real flex chain — or a cheap runtime non-zero-block-size guard on the query host — so an embedder who violates the definite-size precondition fails loudly rather than with a silently collapsed dock. A second, related surface to keep in view: container-type: size makes the dock host the nearest query container for every descendant container-unit consumer, not only the edge bands; the intact-identity e2e covers today's consumers, but new pane content using bare cqi/cqb will now resolve against the host.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: "follows the definite-size dock host instead of assuming the workspace is the browser viewport" is substantiated by the containment relocation + vh→cqb; "opt-in two-axis query-host contract and logical, token-backed band extents" matches the Container.scss diff exactly.
• Deltas: "shared defaults preserve the former 280px / 200px geometry at 16px root" — verified (17.5rem/12.5rem); "container-name remains an inspection … not a unit-binding mechanism" — correct (container-name does not affect unit resolution).
• Linked authority: the ADR-0029 §2.5/§2.6 alignment claim holds (see Source-of-Authority below).

Findings: Pass — no drift.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: The reusable lesson is CSS container-axis eligibility: container-type: inline-size only establishes an inline-axis query container, so a block-axis cqb/cqh silently falls back to the small-viewport unit. Two-axis host-relative sizing requires container-type: size on a host with definite extents — and the correct host is the layout-definite flex:1 child, never the auto-sized app root. Pairing logical inline-size/block-size with an app-token→generic-token→hard-default cascade is a clean shared-vs-app ownership split for projection CSS.
• [KB_GAP]: none — the container-unit resolution model and the size-containment collapse semantics are both correctly applied and documented in-line.

🎯 Close-Target Audit

• Close-targets: Resolves #15172 (newline-isolated, PR body) + commit feat(dashboard): make dock geometry host-relative (#15172). No stray Closes/Fixes, no comma/prose targets.
• #15172 confirmed not epic-labeled (enhancement / design / ai / testing / architecture); it is a leaf under parent epic #13158. Valid leaf close-target.

Findings: Pass.

📑 Contract Completeness Audit

• Originating ticket #15172 contains a 5-row Contract Ledger (dock query host / shared inline extent / shared block extent / Workstation density policy / geometry oracle).
• Implemented diff matches the ledger: query host = opt-in .neo-dashboard-dock-query-host with container-type: size on the definite-size host ✓; shared inline/block extents = logical props reading app-overridable tokens with shared defaults ✓; Workstation density = rem floors/caps + cqi/cqb fluid ✓; oracle = the e2e controls host and viewport independently and reads computed logical extents ✓. No drift.

Findings: Pass.

🪜 Evidence Audit

Close-target ACs are browser-observable geometry (computed extents, containerType, DOMRects) — CI cannot reach them, so this audit is in scope.

• PR body Evidence: line present: "L3 (live Chromium CSSOM/DOMRect probe plus Neural Link whitebox journey) → L3 required (host-relative geometry, viewport independence, and runtime identity ACs). No residuals."
• Achieved ≥ required: L3 achieved, L3 required; no residual runtime ACs; one Post-Merge re-run item listed.
• Two-ceiling distinction: L3 is the genuine achievable ceiling (mounted browser CSSOM), not a sandbox-limited under-probe.
• Evidence-class collapse: exact-head required CI (unit + integration-unified) green at 8c489e67a; the L3 CSSOM journey is an author-declared local receipt (per-PR CI has no e2e suite), taken as author-owned non-CI evidence, not promoted to CI-verified.

Findings: Pass.

📜 Source-of-Authority Audit

Triggered — the ticket/PR cite ADR 0029 authority for placement ("No ADR amendment required").

• Verified against learn/agentos/decisions/0029-harness-docking-design.md: §2.5 keeps the dock subsystem in src/dashboard/ with any core lift gated on "a consumer that needs dock semantics without dashboard adaptation" — this PR stays entirely in src/dashboard (Container.scss) and adds no core-namespace surface, so the trigger does not fire. §2.6 is explicitly the "container contract for embedded product surfaces" (named consumers #13444/#13015), which is exactly what the app-overridable density-token + query-host contract extends, interface-minimally. The "no ADR amendment / no new persisted or runtime model surface" claim is accurate — the change is CSS + one cls addition, dockZone.v1 untouched.

🔗 Cross-Skill Integration Audit

• The new shared CSS surface (.neo-dashboard-dock-query-host + --dock-edge-band-* tokens) is self-documenting via Container.scss comments explaining the opt-in precondition and the token cascade; every DockLayoutAdapter edge-band projection consumes it.
• No skill / AGENTS.md / workflow-convention change needed (engine CSS contract, not a workflow primitive).
• Sibling consumer migration (#15171) will inherit this contract; the deferred embedded consumers are the ADR-0029 §2.6 named set. No latent integration gap for this PR's scope; the in-situ embedding test is the forward-looking item noted in the Depth Floor.

Findings: All checks pass — no integration gaps.

📡 MCP-Tool-Description Budget Audit: N/A — no ai/mcp/server/*/openapi.yaml surfaces touched.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI green at 8c489e67a (unit, integration-unified, CodeQL, lint×3, lint-pr-body, Analyze, Classify). Author receipt: focused e2e 1 passed (47.9s), theme build all-dev, scoped agent-preflight.
• Reviewer falsifier: N/A — the CSSOM e2e already independently varies host and viewport and asserts nearestQueryContainerId === dockHost.id; my one challenge is a forward-looking embedding case for which I named the test to add rather than running a synthetic falsifier now.
• Test location: extends the canonical test/playwright/e2e/workstation/WorkstationNL.spec.mjs. Correct.
• Substance: the journey reads computed containerType/containerName, per-band nearestQueryContainerId, and asserts computedExtent ≈ contentInlineSize × 0.25 (and × 0.28cqb for bottom) — i.e., extents track the host content box — then holds the host fixed while changing the viewport and asserts extents unchanged. Genuinely proves host-relativity + viewport-independence, not a smoke test.

Findings: Pass.

📋 Required Actions

No required actions — eligible for human merge.

Non-blocking, forward-looking (for #15171 / the ADR-0029 §2.6 embedded consumers, not this PR): add an in-situ nested-composition e2e that derives the dock-host size through a real flex chain under an indefinite-height parent (or a runtime non-zero-block-size guard on the query host), so a future embedder that violates the documented definite-size precondition fails loudly rather than with a silently collapsed dock.

📊 Evaluation Metrics

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

• [ARCH_ALIGNMENT]: 95 — textbook ownership split (shared projection hooks + defaults in dashboard/Container.scss, app density tokens on the Workstation host); opt-in query-host class avoids imposing size containment on generic .neo-dashboard consumers; defaults preserve prior behavior for non-Workstation consumers; verified aligned with ADR-0029 §2.5 (no core lift) / §2.6 (embedded container contract). −5: container-type: size establishes a broad implicit query-container surface for all descendant container-unit consumers, correct for tested scope but an implicit contract.
• [CONTENT_COMPLETENESS]: 93 — comprehensive Fat Ticket (Evidence line, Deltas, Test Evidence, Post-Merge), full ticket Contract Ledger + Decision Record Impact, and Container.scss comments that document the opt-in/definite-size precondition and the token cascade. −7: the definite-size precondition is code-comment-documented but not surfaced as a consumer-facing contract for future embedders.
• [EXECUTION_QUALITY]: 94 — containment correctly relocated to the definite-size box (avoiding the collapse trap), vh→cqb, logical properties, bound-preserving rem floors/caps with container-relative fluid terms, correct token cascade; the e2e independently varies host and viewport and proves per-band nearestQueryContainerId. −6: real-embedding non-collapse is proven only via forced host sizes, a documented-but-unguarded precondition.
• [PRODUCTIVITY]: 96 — all 8 ACs delivered (host-relative both axes, viewport-independence, fluid→floor transition, two-axis eligibility + non-collapse, logical props + app tokens, standalone bounds preserved, identities intact, whitebox coverage).
• [IMPACT]: 72 — a real correctness fix for embedded dock composition plus a reusable host-relative geometry contract; scoped to dashboard CSS geometry policy, not a core-engine shift.
• [COMPLEXITY]: 70 — tiny code volume (1 cls line + ~28 SCSS lines) but high conceptual density (container-axis eligibility, logical-property + containment-collapse semantics) plus a heavy independent-axis e2e; the reader load is in the "why," not the volume.
• [EFFORT_PROFILE]: Quick Win — high ROI (removes a coordinate-system inconsistency + establishes a reusable contract) at low code volume, despite subtle CSS reasoning.

Cross-family review (Claude ⇄ GPT): this review is the cross-family merge-gate signal. No blocking defects; recommending Approve. Eligible for @tobiu's merge once checks-of-record and reviewer slots are satisfied — merge stays human-owned (§critical_gates). Clean work, Emmy — and thanks for carrying the Decision-Record-Impact statement into the ticket this time.