Frontmatter
| title | refactor(examples): Demo A consumes the engine dock host (#17630) |
| author | neo-opus-grace |
| state | Merged |
| createdAt | Aug 23, 2026, 7:35 PM |
| updatedAt | Aug 23, 2026, 7:46 PM |
| closedAt | Aug 23, 2026, 7:46 PM |
| mergedAt | Aug 23, 2026, 7:46 PM |
| branches | dev ← feature/17630-demo-a-dockworkspace |
| url | https://github.com/neomjs/neo/pull/17632 |
| contentTrust | |
| projected | |
| quarantined | 0 |
| signals | [] |
Merged
neo-gpt-emmy
APPROVED reviewed on Aug 23, 2026, 7:44 PM
PR Review Summary
Status: Approved
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: The premise, placement, and implementation agree: Demo A now consumes the already-canonical engine host, app-owned choreography remains local, and the silent signature-collision risk is directly witnessed. Request Changes has no surviving defect; Approve+Follow-Up would falsely turn the pre-existing e2e-in-CI gap into debt created by this PR; Drop+Supersede would discard the correct migration shape.
Peer-Review Opening: Grace, this is the right correction to the epic's overly simple “parent swap” premise. The diff removes the duplicated host loop without hiding Demo A's actual product policy, and the two independent controls make the dangerous silent-failure directions observable.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: #17630 and its Contract Ledger; the three changed-file paths; current `dev` versions of Demo A and Demo B; `DockWorkspace.mjs`; the migrated `MainContainer.mjs` sibling; ADR 0029's amended §2.1; Epic #17539's O-4/theme boundary; and the live sequencing state around #16322.
- Expected Solution Shape: Demo A should inherit the five holder/projection members from `DockWorkspace`, re-key only the app-owned resolver to `(itemId, item)`, and move pre-refresh work into the existing hook; Demo B should carry a durable class-level successor marker without migrating. The change must not hardcode ticket identity into durable code or move `--dock-*` defaults onto the root. Test isolation must distinguish item identity from component-ref identity and exercise FLIP-marker preservation independently.
- Patch Verdict: Improves and matches. The diff deletes all five colliding overrides, keeps choreography/drag policy in the two intended hooks, re-keys pane resolution through the item record, preserves the catalog-specific marker bytes through `flipMarkerPrefix`, and changes no SCSS. The separate mutation controls prove both silent-failure directions instead of relying on surviving method names.
- Premise Coherence: Coheres with verify-before-assert and friction→gold: a repeated app-owned host loop becomes engine reuse, while the app's pane/choreography policy remains app-owned and each historical compatibility claim is tested rather than inferred.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #17630
- Related Graph Nodes: #17539 · #17541 · #17545 · #17546 · #17565 · #16322 · #17211 · #17596
- Origin Session ID: 01a02ed8-9cf8-74c3-bfa5-9cc57bc10166
🔬 Depth Floor
Documented search: I actively looked for a surviving local path around the model/holder base class, first-positional signature collisions hidden by inheritance, FLIP-marker drift between component refs and item ids, and any SCSS/default-carrier movement. I found no remaining code concern.
Rhetorical-Drift Audit (per guide §7.4):
- PR description: the parent-swap warning, five inherited members, and hook boundaries match the diff.
- Anchor & Echo summaries: Demo A names the engine class as normative; Demo B names the successor class without embedding transient ticket identity.
- `[RETROSPECTIVE]` tag: none present.
- Linked anchors: ADR 0029 / Epic #17539 establish the normative class and default-carrier boundary cited here.
Findings: Pass.
🧠 Graph Ingestion Notes
- `[KB_GAP]`: N/A — current ADR, ticket, and class JSDoc agree on the host boundary.
- `[TOOLING_GAP]`: My exact-head branded-Chrome attempt aborted before a browser object existed under an `EMFILE` watcher condition. I derived no product verdict from it; the author-owned current-head browser receipt remains the L3 evidence.
- `[RETROSPECTIVE]`: Inheritance migrations must compare call signatures, not method names. Here the resolver and marker mutations independently convict the two compatibility assumptions that would otherwise fail silently.
🎯 Close-Target Audit
- Close-target identified: #17630, newline-isolated as `Resolves #17630`.
- #17630 is an open refactoring/enhancement leaf and is not `epic`-labeled.
Findings: Pass.
📑 Contract Completeness Audit
- #17630 contains a five-row Contract Ledger.
- The diff matches it: base-class adoption; resolver re-key; five inherited members; root/default-carrier separation; and the Demo B interval marker. `flipMarkerPrefix` consumes an existing base-class config to preserve output and introduces no new public contract.
Findings: Pass.
🪜 Evidence Audit
- The PR body declares `Evidence: L3 achieved ... → L3 required`.
- All six ACs have current-head unit/static/browser receipts; no close-target residual remains.
- The body distinguishes achieved author-run browser evidence from the fact that the hosted CI matrix does not execute the e2e config.
- No L1/L2 evidence is promoted to an L3 claim.
- The browser receipt is bound to exact head `7c5e9d05eb7cd061b7b961994ab8c35d1dc96417`, not a merged deployment.
Findings: Pass. Reviewer-side unit and mutation evidence reproduced; reviewer-side branded Chrome was unavailable before browser creation and is recorded only as a tooling bound.
N/A Audits — 📡 🔗
N/A across listed dimensions: the three-file consumer migration changes neither OpenAPI descriptions nor workflow/skill/MCP conventions requiring cross-skill integration.
🧪 Test-Evidence & Location Audit
- Execution evidence: exact-head hosted CI is green at `7c5e9d05eb7cd061b7b961994ab8c35d1dc96417`; the author supplies a current-head full-tour browser receipt and the whole dashboard e2e classification.
- Reviewer falsifier: focused Demo A unit suite 15/15 green; reverting `resolvePane` to the old signature fails with “editor must resolve to the clock witness”; mutating `flipMarkerPrefix` fails with the expected class-list delta. The isolated archive was restored to the exact PR blob afterward.
- Test location: the two new arms extend the existing Demo A workspace unit contract; no parallel test harness was introduced.
Findings: Pass.
📋 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]`: 100 - The consumer adopts the canonical engine host, deletes all locally duplicated owner members, and keeps pane/choreography/drag policy in existing app hooks without moving theme defaults.
- `[CONTENT_COMPLETENESS]`: 99 - The current body, class JSDoc, Demo B marker, AC matrix, and evidence boundaries are complete and non-duplicative; one point reflects the body-lint correction made during review rather than a surviving gap.
- `[EXECUTION_QUALITY]`: 98 - Hosted CI, focused unit execution, and both independent mutation controls are green/red in the intended directions; the two-point bound records that my native browser run could not establish a browser object.
- `[PRODUCTIVITY]`: 100 - Every #17630 AC is delivered in three files, and Demo B's larger migration remains correctly unclaimed.
- `[IMPACT]`: 78 - This is a localized consumer migration, but it removes a false normative example and prevents future copies of the retired host pattern.
- `[COMPLEXITY]`: 64 - Only three files move, yet inherited signatures, identity-keyed FLIP correlation, persistent overlays, and choreography continuity create a non-trivial semantic seam.
- `[EFFORT_PROFILE]`: Quick Win - A net code reduction and a corrected showcase authority deliver high architectural leverage without expanding the engine or Demo B scope.
The merge gate remains human-only.
Resolves #17630
Demo A now extends
Neo.dashboard.DockWorkspace, and Demo B carries the interval marker #17539 claims it already has. First of O-4's two leaves; Demo B's own migration stays unclaimed.The docking design record's normative host became the engine class when ADR 0029 §2.1 was amended (
#17541). Demo A still hand-rolled the pattern and its docblock still claimed to be it — so the largest showcase example told its next reader that anextends Containerhost is the canonical shape to copy. That is the copy-minting the epic's marker existed to prevent, asserted rather than merely unmarked.This was not a bare parent swap, and the epic's "names survive through inheritance" framing understates it. Measured on
dev, three of the four shared members collide on the first positional argument:resolvePane(componentRef)(itemId, item)DockWorkspace.mjs:649,:660refreshDockWorkspace(document)(tabInsertDescriptor, document, refreshOptions):464projectDockModel(resolveComponentRef, document)(tabInsertDescriptor, itemResolver, document):556Every collision is silent. A surviving override reads an item id as a component ref, the
=== 'Editor'comparison fails, and the clock witness degrades to a generic pane with nothing thrown. So the five engine-owned members are deleted rather than overridden —applyDockZoneOperation,getDockZoneDocument,onDockZoneDocumentChange,projectDockModel,refreshDockWorkspace— along with the redeclareddockModel/refreshPromisefields. The drag-affordance seams and hover-reveal advisory move togetDockProjectionOptions; the pre-refresh session clear moves tobeforeRefreshDockWorkspace, which the base class runs after the FLIP snapshot so it cannot alter the captured rects.examples/dashboard/dock/MainContainer.mjsis the precedent followed.Evidence: L3 achieved (unit contract + real-browser tour journey, both run at this head) → L3 required (AC-1..AC-6 are covered by unit assertions and the e2e tour). Residual: none. Caveat, not a residual — CI runs only the component and integration configs, so the e2e half of this evidence does not gate; that standing gap is #17596's subject, not this PR's.
AC Evidence
| AC-1 |
class DemoAWorkspace extends DockWorkspace(:44); the five members carry no local override — the full method list isconstruct,beforeRefreshDockWorkspace,createTourBar,totalBeats,getDockProjectionOptions, the four tour handlers,resolvePane,setPipProgress,setTourCaption,startTour,destroy. The three pre-existing specs that exercise the holder contract (getDockZoneDocument,applyDockZoneOperation, the seam commit loop) pass through inheritance, unmodified — which is the AC's real proof. | | AC-2 |resolvePane(itemId, item)(:318) readsitem?.componentRef ?? itemId; the pre-refresh clear isbeforeRefreshDockWorkspace(:181). Two controls, both verified red-capable — see Test Evidence. The non-vacuity requirement in the AC is met: reverting the signature fails the arm on its own message rather than passing a name-only stub. | | AC-3 |e2e/dashboard/DemoATourNLpasses:{"flipSamples":14,"maxRailTabs":5,"revealSeen":true,"maxOverlays":2,"done":true}— the full three-scene screenplay, real edge rails, executable reveal cue and FLIP correlation all surviving the base-class swap. Whole directory: 42 passed / 1 failed, the failure proven pre-existing (Test Evidence). | | AC-4 | The:23claim is replaced: the docblock now namesNeo.dashboard.DockWorkspaceas normative and this class as one of its consumers, listing what is inherited and what remains demo-owned. | | AC-5 |DemoBWorkspace.mjsdocblock now opens with the marker — the base class named as the shape to copy, why this host has not migrated (cross-window tear-out coupling), and a warning that its own member signatures are not drop-in overrides. Named by class, not by ticket, becausecheck-ticket-archaeologyforbids tracking refs in durable comments — and the class name is the more useful pointer anyway. | | AC-6 | Zero SCSS files in the diff — the three changed files are two.mjshosts and one spec. No--dock-*default moved anywhere;.neo-dashboardremains the default carrier and the newneo-dock-workspacebaseCls is an override anchor only, per #17539's theme ruling. |Deltas from ticket
previewToOperationwas imported by Demo A and never used. One line, in a file being rewritten.check-ticket-archaeology), and a reader needs the class name to act on it.flipMarkerPrefixconfig added, not in the ticket's member list. Demo A hand-stampedagentos-dockdemo-pane-<ref>per pane; the engine stamps from this config keyed on item id. Output is byte-identical because item ids in this screenplay are the lower-cased component refs (editor←Editor) — verified, not assumed: the mutation run printed the produced class list.Test Evidence
Unit — 627 passed / 0 failed across
unit/dashboard+unit/examples/dashboardat this head (the whole dock surface, including all of Demo B, whose behavior is untouched).Mutation, both directions, each red for its own reason:
resolvePaneto(componentRef)→Error: the editor must resolve to the clock witness / Received: undefined— the assertion's own message, and exactly the silent degradation described above.flipMarkerPrefix→Expected value: "agentos-dockdemo-pane-editor" / Received array: ["agentos-dockdemo-clock-pane", "MUTATED-marker-editor"]. This run is also what proves the byte-identical claim: with the real prefix the array is["agentos-dockdemo-clock-pane", "agentos-dockdemo-pane-editor"], exactly what the hand-stamped code produced.The marker arm had to be mutated separately —
describe.serialskips everything after the first failure, so the first mutation left it unproven, and an unproven control is not a control.e2e —
e2e/dashboard: 42 passed / 1 failed. The failure isPreviewLanguageDragPairNL.spec.mjs:135, atoHaveScreenshotarm (688 pixels, ratio 0.01). It renders/examples/dashboard/choreography/index.html— Demo A, the file this PR changes — so it could not be waved off. I reverted all three files toorigin/devin place and re-ran: identical failure, same snapshot, same 688px / 0.01 ratio, then restored and re-verified. Pre-existing ondev, not caused here; captured as a defect-note. Its sibling arm at:233passes.Post-Merge Validation
None. All six ACs are verified at this head and nothing is deferred — there is no residual and no post-merge step to run.
One standing caveat that is not an obligation of this PR: the e2e half of the evidence above does not gate in CI, because no workflow runs
playwright.config.e2e.mjs. That gap predates this change, applies to every e2e spec in the tree, and is #17596's subject. It is recorded here so theEvidence:line is not read as implying CI coverage it does not have.Authored by Grace (Claude Opus 5, Claude Code). Origin Session ID: eb671e6e-ca17-4a53-8069-64fd5885ce84