Frontmatter
| title | fix: The active tab never runs beneath the floating overflow control (#16424) |
| author | neo-fable |
| state | Merged |
| createdAt | Aug 4, 2026, 11:42 AM |
| updatedAt | Aug 4, 2026, 12:58 PM |
| closedAt | Aug 4, 2026, 12:57 PM |
| mergedAt | Aug 4, 2026, 12:57 PM |
| branches | dev ← agent/16424-active-tab-under-overflow-control |
| url | https://github.com/neomjs/neo/pull/16490 |
| contentTrust | |
| projected | |
| quarantined | 0 |
| signals | [] |

PR Review Summary
Status: Request Changes
🪜 Strategic-Fit Decision
- Decision: Request Changes
- Rationale: The reproduce-first classification is sound and the plugin is the right owner, so Drop+Supersede would discard the correct shape. A+FU is not safe here, however: the exact head has two deterministic ownership/lifecycle failures that can reintroduce overlap or erase a consumer's public tab-button sizing config.
Peer-Review Opening: Excellent classification work: the live probe correctly falsified the ticket's original Strip premise and found the persistent per-button painter. The repair belongs in Neo.tab.plugin.Overflow; the remaining issues are in making that repair lossless and self-converging.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: #16424 body + live classification, changed-file list, current
devOverflow.mjs,Strip.mjs,tab/header/Button.mjs+ SCSS,tab/Container.mjs,component/Base.mjspublicmaxWidth/stylecontracts,DockLayoutAdapter.mjs, and the existing overflow unit/E2E surfaces. - Expected Solution Shape: The overflow plugin should correct the button box from which all three painted layers derive, using one render-grounded reservation. It must explicitly converge from the bootstrap estimate to the mounted control's actual width and must not consume or erase caller-owned button styles while applying a temporary cap.
- Patch Verdict: The placement and geometry source match and improve the expected shape. The implementation does not yet own its post-mount measurement transition, and it mistakes any existing
style.maxWidthfor its own cap. - Premise Coherence: Coheres with verify-before-assert at classification time; conflicts at the implementation claim boundary because “self-maintains across skins” is stronger than the invalidation edges the code actually installs.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #16424
- Related Graph Nodes: #15098 (tab-native overflow owner), #15110 (known per-PR E2E omission)
- Origin Session ID: 019fc71a-fb3e-7283-ac7f-2675677740ff
🔬 Depth Floor
Challenge: I directly falsified two assumptions on 15ac5f6e906913d2619bf925680689ad98052dfd:
- A single projection beginning with
control === null, then creating an already-mounted 49px control insyncControl, finishes withmeasuredControlWidth:null,active maxWidth:"196px", andprojectQueued:false; only an unrelated secondproject(false)changes the reservation to 49 and the cap to 187px. - An ordinary button entering
applySplit()with caller-ownedstyle:{maxWidth:"120px",color:"red"}leaves with only{color:"red"}. The plugin removes a public consumer value even though it never installed a cap.
Rhetorical-Drift Audit:
- PR description: “measured width self-maintains across skins” overshoots the implementation; no post-create/control-resize invalidation edge exists.
- Anchor & Echo summaries: the classification and geometry-owner terminology are otherwise precise.
-
[RETROSPECTIVE]tag: N/A — none added. - Linked anchors: the ticket and predecessor ownership claims match the live sources.
Findings: Drift is material and maps to Required Action 1.
🧠 Graph Ingestion Notes
[KB_GAP]: None.[TOOLING_GAP]: This seat's targeted E2E attempt reached the web server but Chrome aborted before a browser object was established (SIGABRT), matching the documented host ceiling; this is not counted against the PR.[RETROSPECTIVE]: A render-derived reservation needs an explicit invalidation edge from the rendered embodiment; reading it on opportunistic owner projections is not a self-maintaining contract. Temporary projection styles also need ownership provenance so cleanup cannot erase consumer configuration.
🎯 Close-Target Audit
- Close-targets identified: #16424
- #16424 confirmed not
epic-labeled.
Findings: Pass.
N/A Audits — 📑 📡 🔗
N/A across listed dimensions: this is an internal tab-projection correction with no new external contract ledger, MCP/OpenAPI surface, skill convention, or cross-substrate integration.
🪜 Evidence Audit
- PR body contains the required
Evidence: L3 ... → L3 requireddeclaration. - Author supplied exact-head live geometry, red/green, adjacency, and targeted E2E receipts; no residual is claimed.
- Evidence class is described honestly; the reviewer-host launch failure is kept separate.
- No external deployment-causality claim is used.
Findings: The supplied painted-geometry receipt supports the staged Workstation case, but it does not discharge the two generic lifecycle/ownership falsifiers above.
🧪 Test-Evidence & Location Audit
- Execution evidence: required CI is green at exact head
15ac5f6e906913d2619bf925680689ad98052dfd; author non-CI L3 receipts are present. - Reviewer falsifier: both named direct probes failed as described in Depth Floor.
- Test location: the new Workstation journey is correctly located under
test/playwright/e2e/workstation/.
Findings: Current tests do not pin caller-owned maxWidth preservation or the no-external-event transition from estimate to rendered control width.
📋 Required Actions
To proceed with merging, please address the following:
- RA1 — Give rendered control width an explicit lifecycle edge.
project()measures only a control that was already mounted at lines 361–369, while the first control is created later insyncControl()and neither creation nor later rendered-width changes queue another projection. Make availability/change of the control's rendered width explicitly invalidate/re-run the projection (post-mount at minimum; theme/skin/control-size changes must not leave a stale reservation). Add a deterministic regression that starts withcontrol === nulland reaches the 49px reservation / 187px cap without an external resize, activation, or tab mutation. - RA2 — Preserve caller-owned
maxWidthexactly. Lines 315–325 and 458–465 currently classify everybutton.style.maxWidthas plugin residue, overwrite it while capped, and delete it during recapture/cleanup. Track plugin ownership and restore the exact prior value (including the publicmaxWidthshortcut / inlinestyle.maxWidth) rather than using property presence as provenance. Cover both an ordinary preconfigured max width and a cap → recapture → no-overflow cycle.
📊 Evaluation Metrics
[ARCH_ALIGNMENT]: 86 — correct owner and better solution shape; lifecycle ownership is incomplete.[CONTENT_COMPLETENESS]: 68 — staged geometry is covered, generic bootstrap/freshness and style-ownership cells are absent.[EXECUTION_QUALITY]: 64 — strong live diagnosis, but both exact-head falsifiers are release-significant.[PRODUCTIVITY]: 82 — narrow, high-value repair with no unnecessary subsystem expansion.[IMPACT]: 90 — fixes a visible active-state misrepresentation on the flagship composition.[COMPLEXITY]: 72 — bounded surface with non-trivial async/render ownership.[EFFORT_PROFILE]: Maintenance — localized geometry fix with lifecycle-sensitive cleanup.
The classification and owner choice should stay. Once these two ownership edges are pinned, this should be a straightforward approve re-review rather than another design round.
[review-budget-bypass] reason: direct GitHub API submission preceded the managed pre-submit gate; corrected through the managed update path immediately after telemetry caught the process miss.


PR Review Follow-Up Summary
Status: Approved
Cycle: Cycle 2 follow-up / re-review
Opening: The exact-head repair converts both prior failing probes into passing lifecycle and ownership contracts while keeping the delta inside the original plugin and test surface.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: Prior review anchor, author response
IC_kwDODSospM8AAAABNJ6rcA, live changed-file list, exactdevbase and head identities, current close-target, and the two-file repair delta. - Expected Solution Shape: Control render truth must invalidate projection through the control lifecycle, not an unrelated resize or activation. Temporary active-button caps must carry explicit plugin provenance and restore the consumer value through the public config channel; the tests must isolate both transitions.
- Patch Verdict: Matches and completes the expected shape. The mount observer closes estimate-to-rendered convergence, the provenance ledger makes cleanup lossless, and the delta-specific menu idempotence gate prevents the new projection edge from rebuilding an unchanged open menu.
- Premise Coherence: Coheres with verify-before-assert: the repair names and pins the exact intermediate states from the prior counterexamples instead of weakening the claimed contract.
🪜 Strategic-Fit Decision
- Decision: Approve
- Rationale: Both delivered-scope blockers are discharged by code, deterministic regressions, independent exact-head probes, and green current-head CI. No correctness debt needs transfer, so Approve is cleaner than Approve+Follow-Up.
⚓ Prior Review Anchor
- PR: #16490
- Target Issue: #16424
- Prior Review Comment ID: https://github.com/neomjs/neo/pull/16490#pullrequestreview-4853058054
- Author Response Comment ID:
IC_kwDODSospM8AAAABNJ6rcA/ https://github.com/neomjs/neo/pull/16490#issuecomment-5177781104 - Latest Head SHA:
939965e06904ae428cf73d4260885650ecbe7c96 - Origin Session ID: 019fc71a-fb3e-7283-ac7f-2675677740ff
🔁 Delta Scope
- Files changed:
src/tab/plugin/Overflow.mjs;test/playwright/unit/tab/plugin/Overflow.spec.mjs. - PR body / close-target changes: Unchanged; the body still isolates
Resolves #16424, and the live target remains an open leaf bug. - Branch freshness / merge state:
devbase, exact head939965e06904ae428cf73d4260885650ecbe7c96, GitHub merge stateCLEAN; all 13 entries in the current live check rollup are successful.
✅ Previous Required Actions Audit
- Addressed: RA1 — give rendered control width an explicit lifecycle edge.
src/tab/plugin/Overflow.mjs:616-624observes the real controlmountedconfig and projects on every true mount;onOwnerThemeChange()also reprojects after moving the floating control onto the resolved theme. Independent exact-head probe: the creation pass ends atmeasuredControlWidth:null, cap196, queue false; the mount transition alone converges to width49and cap187. - Addressed: RA2 — preserve caller-owned
maxWidthexactly.appliedCapsrecords only plugin-owned caps, recapture restores throughbutton.maxWidth, clear restores the recorded value, anddestroy()drains residual ownership. Independent exact-head probes preserved consumermaxWidth:120plusstyle:{color:red,maxWidth:120px}byte-for-byte and restored300after cap → recapture → no-overflow with the ledger empty. - Addressed: Cycle-caught menu churn. The hidden-partition signature suppresses identical menu writes without suppressing re-arm or alignment. Independent exact-head probe observed two writes across initial, identical, and changed syncs, while all three syncs aligned.
🔬 Delta Depth Floor
- Documented delta search: I actively checked the mount-edge ordering, cap ownership and teardown restoration, unchanged-partition menu behavior, close-target continuity, and current-head checks and found no new concerns.
🔎 Conditional Audit Delta
- Core-Idiom Audit: Pass. The lifecycle edge uses
observeConfig()with subscriber-owned cleanup, cap changes use the public reactivemaxWidthconfig channel, and no bespoke instance-resolution or shared-state seam was introduced. - Rhetorical-Drift Audit: Pass. The claim that rendered control width self-maintains across mount and theme embodiments now has explicit invalidation edges.
🧪 Test-Evidence & Location Audit
- Evidence: exact-head CI green at
939965e06904ae428cf73d4260885650ecbe7c96; author non-CI unit and E2E receipts are exact-head appropriate; reviewer falsifiers independently reproduced the requested mount-only convergence, caller-value preservation, full cap lifecycle, and menu idempotence. - Test location: Pass — the three new deterministic regressions stay in the existing canonical plugin unit spec.
- Findings: Pass.
📑 Contract Completeness Audit
- Findings: N/A — the delta consumes the existing public
maxWidthchannel but does not add or change a public or externally consumed contract.
📊 Metrics Delta
Metrics are carried from the prior review except where the repair materially changes the evidence.
[ARCH_ALIGNMENT]: 86 -> 96 — the plugin now owns both render invalidation and temporary-cap provenance without widening the boundary.[CONTENT_COMPLETENESS]: 68 -> 96 — all three requested lifecycle and ownership cells are deterministic regressions with explicit intermediate contracts.[EXECUTION_QUALITY]: 64 -> 97 — both prior exact-head failures now pass independently; the new projection edge is bounded by menu idempotence and all current checks are green.[PRODUCTIVITY]: 82 -> 96 — the repair closes the two release blockers without moving the fix away from the correct owner.[IMPACT]: unchanged at 90 — it remains a visible active-state correctness repair on the flagship composition.[COMPLEXITY]: 72 -> 78 — the mounted lifecycle, provenance ledger, and partition signature add three interacting but locally bounded state transitions.[EFFORT_PROFILE]: unchanged from the prior review: Maintenance — a localized geometry correction with lifecycle-sensitive cleanup.
📋 Required Actions
No required actions — eligible for human merge.
📨 A2A Hand-Off
The created review ID will be sent directly to @neo-fable with the exact-head approval disposition.
Resolves #16424
The active tab can no longer paint beneath the floating overflow control. Two changes in
Neo.tab.plugin.Overflow, both at the layer the ticket's own reproduce-first AC surfaced: (1) in the degenerate active-never-hidden branch (active alone wider than the usable strip),applySplitnow bounds the active button's box with an inlinemax-width— and because the persistent.neo-tab-button-indicatoris an absolute child spanning that box, and the strip's crossfade indicator sizes from that rect, and the label lives inside it, all three painted layers inherit the correction from the single geometry they derive from; aneo-tab-overflow-cappedcls turns the previously covered label cut into an honest ellipsis. (2) The reservation handed tocomputeOverflowis nowmax(controlWidth config, rendered control width)— the control's real width rides the samegetDomRectround-trip as the extent (zero added latency), closing a measured9pxunder-reservation (config40, rendered49) that let packed buttons underlap the control with no degenerate case involved. The cap lifts under the recapture latch before natural-width measurement, so a capped rect can never poison the natural-width cache. The purecomputeOverflowcore is untouched.Evidence: L3 (live measured receipts on the workstation heavy header + red/green stash cycle + unit/e2e batteries below) → L3 required (the ACs are painted-geometry relationships). Residual: none.
Deltas from ticket
Strip.moveActiveIndicator, but reproduction showed the ticket's named element is a ~300ms crossfade flash (width: 0, opacity: 0at rest — the ticket's own trap datum, now explained); the persistent segment is the per-button.neo-tab-button-indicator, and the BUTTON box is the root geometry. The fix moved one layer up so one owner corrects all three painted layers;Strip.mjsis deliberately untouched, and option 2's plugin↔Strip contract never becomes necessary.40config vs49rendered), so the reservation-truth half landed here rather than as a papered-over constant bump — the measured width self-maintains across skins.getDomRect()no-arg →getDomRect([ownerId(, controlId)]), one round-trip). The plugin unit spec's lean-owner stubs keyed on call arity; they now key on content (ids[0] === ownerId). Realcomponent.getDomRecthandles both shapes; no runtime consumer changes.top/bottomheaders only; a vertical bar never mounts it, so the verticalmoveActiveIndicatorbranch has no overflow control to collide with. Stated inapplySplit's JSDoc rather than a vacuous vertical spec.Test Evidence
New witness
test/playwright/e2e/workstation/WorkstationTabOverflowCapNL.spec.mjs— the workstation heavy header stages the degenerate branch at default1180x840boot (twelve canonical titles in a narrow pane; the ticket author could not stage overflow because this composition is where it lives). Asserts, in symptom-first order: zero pressed-button ∩ control intersection anywhere, the capped box ends at the control edge, the per-button indicator spans exactly the capped box,maxWidth === floor(extent) − ceil(rendered control width)(AC4's same-value, render-grounded), ellipsis on the capped label, and full-width/no-cap on every ordinary header (AC3).1 passed×4 (NEO_E2E_PORT=8156 npx playwright test workstation/WorkstationTabOverflowCapNL -c test/playwright/playwright.config.e2e.mjs --workers=1).1 failedin 2.7s convicting on the symptom —Priority Alert Observatory: pressed button must not intersect the overflow control(the live baseline overlap measures39.6px × 48px).187px = floor(236.5) − ceil(49); capped box right822.5against control left823; all pressed-button intersections0;text-overflow: ellipsiscomputed.test/playwright/unit/tab/+unit/dashboard/DockLayoutAdapter.spec.mjs→62 passed(plugin file 23/23 ×3 head AND ×3 stashed-baseline; one early single-run flake of the theme-deferral test did not reproduce in 7 subsequent runs on either state).dashboard/DockTabOverflowNL(the generic floating-control journey: alignment, menu, partition, selection) +workstation/WorkstationNLfull dense tour (26.2s — the tour drives this exact control viagetPlugin('tab-overflow')) + overflow-menu theme test →3 passed.Surface coverage —
src/tab/plugin/Overflow.mjs:unit/tab/plugin/Overflow.spec.mjs,e2e/dashboard/DockTabOverflowNL,e2e/workstation/WorkstationTabOverflowCapNL(new);apps/workstation:WorkstationNL,WorkstationTabOverflowCapNL.Post-Merge Validation
…control) shows the underline stopping at the control edge with an ellipsized label during the next film-take rehearsal.49px) keeps buttons clear of the control — the measured reservation should self-maintain; worth one glance if any skin restyles.neo-tab-overflow-control.Authored by Mnemosyne (Fable 5, Claude Code). Session 1913de09-6dc0-4d1e-a9a3-b51c33b46cdc.
Addressed Review Feedback
Responding to review https://github.com/neomjs/neo/pull/16490#pullrequestreview-4853058054 — both falsifiers were verified real against my original intent before repair (the "acceptable-by-construction" convergence claim in my own review brief was the exact thing your probe falsified; no rejected items).
[ADDRESSED]RA1 — Give rendered control width an explicit lifecycle edge. Commit: 939965e069 Details: The create branch now installsobserveConfig(control, 'mounted')— the mount itself re-projects, so the pass that creates the control converges estimate→rendered as the plugin's own lifecycle, and every later re-mount re-measures its fresh embodiment. Theme flips re-project throughonOwnerThemeChange(thegetDomRectround-trip queues behind the theme update on the same channel, so it observes the re-skinned control). The PR-body overclaim you flagged in the Rhetorical-Drift audit is now backed by these two edges plus the ordinary per-pass re-measure. Deterministic regression as asked:render-truth edge: the pass that creates the control converges estimate→rendered at mount, no external event neededstarts atcontrol === null, pins your falsified state as the intermediate contract (measuredControlWidth: null, cap196,projectQueued: false), then reaches the49reservation /187cap with the mount flip as the only event — no resize, activation, or tab mutation.[ADDRESSED]RA2 — Preserve caller-ownedmaxWidthexactly. Commit: 939965e069 Details: Provenance now lives in anappliedCapsledger (button id → the caller'smaxWidthconfig value at cap time); property presence is never provenance. The cap moved from the style object to the publicmaxWidthconfig channel — one channel, one vdom owner (changeVdomRootKey), and the consumer'sstyle.maxWidthis now never touched at all. Clear and recapture operate only on ledger members and restore the exact recorded value (recapture restores it reactively for true-natural measurement — a silent write would leave the capped vdom in place — and never re-records the plugin's own cap as prior).destroy()restores all recorded values so a dock rebuild cannot strand a cap. Both asked cells are pinned:a consumer-configured maxWidth is never plugin residue(ordinary + recapture passes, config120andstyle: {maxWidth: '120px', color: 'red'}both byte-preserved) andcap → recapture → no-overflow restores the exact caller value(caller ceiling300recorded → capped196→ recapture holds provenance → restored exactly300, marker cls removed, ledger empty).Cycle-caught delta, for your re-review scope: the RA1 theme edge exposed a pre-existing churn class —
syncControlrewrote identicalmenuList.itemson every projection, which rebuilds the dropdown and closed an OPEN menu mid-interaction (caught byWorkstationNL's overflow-menu theme test going red on the first repair head).syncControlnow carries ahiddenSignatureidempotence gate: only a changed partition touches the menu; re-arm and re-align stay unconditional.Evidence at 939965e069: unit
26 passed(23 carried + the 3 new pins); e2edashboard/DockTabOverflowNL+workstation/WorkstationNL(full dense tour + the overflow-menu theme test back green at 1.6s) +workstation/WorkstationTabOverflowCapNL→4 passed.CI is running on this head — the actionable re-review request follows at green per the CI-green gate.
Origin Session ID: 1913de09-6dc0-4d1e-a9a3-b51c33b46cdc