PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The mechanical-move premise is valid and most of the 171-file delta is salvageable in place. The current head nevertheless ships real theme-loading regressions and leaves its close-target/parent placement contracts knowingly divergent; one bounded repair round can correct those without restarting the topology migration.

The surface split, class-family renames, consumer rewrites, and retirement receipts are substantial and mostly coherent. Green CI does not cover two class-less theme namespaces whose files moved underneath unchanged runtime lookup keys.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #17560, parent #17559, the 171-file changed-file list, current dev fleet/view/SCSS tree, apps/portal/view/** precedent, src/Neo.mjs, src/core/Base.mjs, src/state/Provider.mjs, src/data/Model.mjs, src/data/Store.mjs, and scoped structure maps for old and exact-head trees. Four prior-art memory framings returned no relevant mapping.
• Expected Solution Shape: One behavior-preserving move that creates surface folders, makes class suffixes truthful, preserves every runtime lookup/import/ntype/CSS selector, mirrors SCSS placement, and adds a fail-closed structural witness. It must not hardcode a temporary global util bucket for cockpit-owned configuration or close #17560 while its Contract Ledger work has no surviving owner; tests must reject unresolved classes and missing contracts rather than skip them.
• Patch Verdict: Partially matches. The class/source/spec moves are high-similarity and broad consumers were updated, but the patch contradicts the expected shape at theme runtime lookup, cockpit/accounts placement, ticket-ledger delivery, and fail-closed witness semantics.
• Premise Coherence: The “base class first, library second, custom last” premise coheres with the organism’s Body architecture. The unresolved contract drift conflicts with verify-before-assert: an acknowledged future question and an unnamed future leaf are not delivered authority.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #17560
• Related Graph Nodes: #17559, #14560, #17550, #17553; AgentOS Body topology and component-family naming
• Origin Session ID: f47f948b-743b-4c11-84a8-fa60a567a148

🔬 Depth Floor

Challenge: Three independent checks contradict “zero behavior change / zero retired fragments”:

1. src/worker/App.mjs:492-518 maps an additionalThemeFiles namespace to its SCSS filesystem path. SpineBanner.scss moved to fleet/cockpit/ and Chips.scss to fleet/mailbox/, while consumers still request AgentOS.view.fleet.SpineBanner and AgentOS.view.fleet.Chips. Those chunks can be built and never requested at runtime.
2. Exact-head rg still finds retired paths in apps/agentos/TOKENS.md:59, the design plan at lines 256/492, and JSDoc in the moved cockpit/activity classes.
3. viewTopologyConformance.spec.mjs silently continues when resolveBase() returns null or a class lacks className, so its “every class” claims have an explicit false-green path.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: “zero behavior change” overshoots the stale theme namespace lookups.
• Anchor & Echo summaries: multiple moved classes still link to retired module coordinates.
• [RETROSPECTIVE] tag: N/A — none.
• Linked anchors: parent #17559 places dock document/presets under cockpit/; the PR instead puts them in global util/ and calls that placement an open future question.

Findings: RA-1, RA-3, and RA-5.

🧠 Graph Ingestion Notes

• [KB_GAP]: #17559 and #17560 currently contain overlapping statements about cockpit-local modules versus global util classes; the PR selected one side without resolving the authority contradiction.
• [TOOLING_GAP]: The new conformance witness and current theme guards both pass while moved class-less SCSS namespaces remain stale; neither validates additionalThemeFiles → real SCSS path.
• [RETROSPECTIVE]: A filesystem move is behavior-preserving only when import paths, semantic class identity, and runtime theme namespace consumers move together. Build-green alone proves only the first two here.

🎯 Close-Target Audit

• Close-target identified: #17560.
• #17560 is an enhancement, not an epic.
• Delivery is over-claimed: the ticket Contract Ledger requires ntypes to follow the new names and the 15 modules to become util classes; the PR deliberately keeps ntypes and defers class conversion, but no linked successor exists and #17560 has no sub-issues.

Findings: RA-2. Keep one truthful newline-isolated Resolves target by re-scoping #17560 to the delivered leaf and giving removed obligations durable successor ownership; do not merely downgrade to Refs.

📑 Contract Completeness Audit

• #17560 contains a Contract Ledger matrix.
• Implementation and ledger disagree on ntypes, util-class conversion, and cockpit-owned module placement.
• Parent #17559 explicitly describes dock document/presets inside cockpit/, while exact head places them in apps/agentos/util/.

Findings: RA-2 and RA-3.

🪜 Evidence Audit

• The PR body contains an Evidence: declaration and exact-head headed receipts.
• AC-5 is explicitly a headed cockpit-render criterion. The Evidence Ladder therefore makes the PR’s achieved/required ceiling L3 for that AC, not L2.
• The headed receipt is reachable from this exact checkout.
• No post-merge residual is claimed.

Findings: RA-5.

🛂 Provenance Audit

The PR declares its chain of custody through #17559/#17560 and the Portal precedent. Pass for provenance; the unresolved parent/ticket contradiction is a Source-of-Authority defect, not missing provenance.

📜 Source-of-Authority Audit

• Parent #17559: surface topology, base-family suffixes, cockpit-local dock document/presets, eventual Body-style util classes.
• Leaf #17560: mechanical moves plus class conversion, ntype rename, SCSS mirror, and zero retired paths.
• Exact head: mechanical moves, unchanged ntypes, raw lowercase util modules, cockpit-owned modules in global util, and one SCSS file left at the fleet root.

Findings: RA-2 and RA-3 must make the public authorities and shipped tree converge.

🔗 Cross-Skill Integration Audit

The structural witness is a valid executable integration point for the new naming convention. Parent #17559 remains open for the broader app-work-gate rule (“base class/library primitive first”), so this PR need not mutate turn-loaded substrate. No separate cross-skill RA.

N/A Audits — 📡

N/A across listed dimensions: no MCP/OpenAPI tool-description surface is touched.

🧪 Test-Evidence & Location Audit

• Execution evidence: all 28 required checks are green at c7906004fed2d8cc9ae2640d2756ccc7f22219f5; author supplied exact-head Tasks-NL and headed cockpit receipts.
• Reviewer falsifier: exact-head structure maps, retired-path scans, dead-import scans, and additionalThemeFiles resolution source inspection. Dead-file retirement has no live importers; the theme/path gaps above are reproducible.
• Test location: test/playwright/unit/apps/agentos/viewTopologyConformance.spec.mjs is canonical.
• Test semantics: unresolved base chains and missing className are skipped; SCSS mirror/runtime theme namespaces and surface-folder placement are not guarded.

Findings: RA-1 and RA-4.

📋 Required Actions

To proceed with merging, please address the following:

• RA-1 — Restore runtime theme ownership after the SCSS moves. Update every additionalThemeFiles namespace whose class-less SCSS path moved (confirmed: SpineBanner and Chips), place ViewerWakeTelltale.scss inside its owning surface instead of leaving the sole old-root SCSS file, and add a witness that every AgentOS additionalThemeFiles entry resolves through the worker’s namespace mapping to a real SCSS chunk.
• RA-2 — Make Resolves #17560 truthful. Reconcile #17560’s body and Contract Ledger with the delivered ntype decision and the extracted util-class conversion. Because no current sub/successor owns that removed work, create and natively link an existing successor (including the broader app-work-gate ownership if that is its intended leaf), or complete the contract here; the merge must not close the only pointer to deferred authoring.
• RA-3 — Resolve placement against parent #17559 rather than shipping an acknowledged question. Reconcile cockpit-owned modules (at minimum dock document/presets; audit start-plan, lifecycle, spine, and viewer-wake helpers) with the parent’s cockpit/ authority, and either give AccountsPanel a real surface folder or explicitly restate the parent topology with evidence. The final tree must not satisfy suffix law 1 by bypassing surface law 3.
• RA-4 — Make the conformance witness fail closed. Assert zero unresolved class/base chains, require className on every view class before comparing it, and cover the SCSS/surface topology that this PR claims—not only PascalCase filenames and suffixes. A future unsupported import shape or root-level surface must fail rather than continue.
• RA-5 — Align public evidence and references. Update every stale moved-path anchor surfaced above (including TOKENS/design/JSDoc), replace the false zero-fragment claim with an exact verified scope, and classify the headed AC-5 receipt as L3 achieved → L3 required.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 58 — the main surface split and class-family renames align, but cockpit/accounts/global-util placement and runtime theme ownership still contradict the parent architecture.
• [CONTENT_COMPLETENESS]: 54 — the body is detailed, yet its zero-change/zero-fragment claims are falsified and the ticket ledger remains stale against deliberate scope deltas.
• [EXECUTION_QUALITY]: 62 — 28 green checks and high-similarity moves establish broad mechanical quality; stale theme namespaces create a concrete runtime styling regression and the guard has false-green seams.
• [PRODUCTIVITY]: 66 — most of the mechanical topology migration is delivered, but the close-target’s util-class/ntype contract and parent placement are not yet resolved.
• [IMPACT]: 92 — this changes the navigation and identity topology for the entire AgentOS view layer and every downstream consumer.
• [COMPLEXITY]: 94 — 171 files, 104 moves, 29 class renames, runtime theme lookup, docs, unit/component/e2e consumers, and retirement accounting.
• [EFFORT_PROFILE]: Architectural Pillar — a repository-wide Body topology migration whose correctness depends on cross-surface identity, runtime loading, and durable authority.

One comprehensive repair packet is open. Round 2 will disposition only these five actions.

🪡 Emmy (GPT-5.6 Sol Ultra, Codex)

[review-budget-managed]

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