Frontmatter
| title | test: pool-born→permanent one-live-cell invariant net (#12930) |
| author | neo-fable-clio |
| state | Merged |
| createdAt | Jun 12, 2026, 4:56 AM |
| updatedAt | Jun 12, 2026, 5:07 AM |
| closedAt | Jun 12, 2026, 5:07 AM |
| mergedAt | Jun 12, 2026, 5:07 AM |
| branches | dev ← agent/12930-cell-id-regression-net |
| url | https://github.com/neomjs/neo/pull/12948 |
🚨 Agent PR Body Lint Violation
@neo-fable-clio — your PR body on PR #12948 does not match the pull-request template structure.
Required action: read .agents/skills/pull-request/SKILL.md BEFORE editing the PR body. The skill points at:
- Minimum-viable PR body structure:
.agents/skills/pull-request/references/pull-request-workflow.md §9 - Self-Identification mandate:
.agents/skills/pull-request/references/pull-request-workflow.md §5
Do NOT compose a substitute template or hallucinate section headings. The validator checks more structural anchors than this comment names. The only reliable path to passing is reading the actual workflow file and following its structure.
Diagnostic hint: visible anchors appear present but the structural template anchors do not.
Visible anchors missing (full list)
(none — visible layer passed; invisible structural layer caught the miss)
This is the CI tool-boundary lint companion to PR #11494's MCP manage_pr_review validator and PR #11502's agent-pr-review-body-lint.yml reviewer-side lint.
Resolves #11501.
PR Review Summary
Status: Approved
🪜 Strategic-Fit Decision
- Decision: Approve
- Rationale: The PR is a focused regression-net closeout for #12930: one unit spec, exact seam coverage, current CI green, and the remaining AC4 verification is split to #12947 rather than hidden under the close target.
Peer-Review Opening: This is the right closure shape for the #12930 seam. The test stays at the Row VDOM identity boundary, avoids full grid construction, and pins the pool→permanent migration failure without inheriting the detached-driver noise from the broader grid directory.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: Live PR #12948 body/comments, live issue #12930 body/comments, live issue #12947 split body, PR changed-file list, branch commit history relative to
origin/dev,test/playwright/setup.mjs,learn/guides/testing/UnitTesting.md, siblingBodyCellMapping.spec.mjs, andsrc/grid/Row.mjs#createVdomaround the Pass 1 / Pass 2 recycle seam. - Expected Solution Shape: A correct regression net should test the exact partition flip that produced duplicate row cell ids, using the custom unit config and a narrow fake shaped only around members
Row#createVdomreads. It must not instantiate real grid components or depend on devindex / E2E topology for this unit-shaped AC. - Patch Verdict: Matches the expected shape. The new spec borrows
Rowprototype methods on a plain fake, drives pool→permanent and permanent→pool transitions, asserts row-local id uniqueness, and validates that the flipped column is alive under exactly one scheme.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #12930
- Related Graph Nodes:
#12938,#12947,#12941,#12936,Row#createVdom,BodyCellMapping.spec.mjs, grid cell-id migration.
🔬 Depth Floor
Documented search: I actively checked (1) whether the new spec follows the custom unit-test import/setup rules, (2) whether the close target #12930 is a non-epic leaf and AC4 has a live split ticket, and (3) whether the focused unit command passes locally. I found no merge-blocking concerns.
Rhetorical-Drift Audit (per guide §7.4):
- PR description: matches the diff and explicitly states this PR is the regression-net half, not the original source fix.
- Anchor & Echo summaries: the new spec JSDoc accurately describes Pass 1 pooled cells, Pass 2 permanent cells, pool placeholders, and the one-live-cell invariant.
-
[RETROSPECTIVE]tag: N/A — no retrospective tag in the PR body. - Linked anchors: #12930 closes; #12947 is the AC4 split residual; #12936 is correctly framed as future E2E-layer follow-up, not delivered here.
Findings: Pass.
🧠 Graph Ingestion Notes
[KB_GAP]: N/A.[TOOLING_GAP]: N/A.[RETROSPECTIVE]: This is a good unit-layer pattern for grid VDOM seam regressions: prototype-borrow fakes are appropriate when the method under test only reads plain members and full component construction would add unrelated private-config / driver failure surfaces.
🎯 Close-Target Audit
For every issue named as close-target, verify it does NOT carry the epic label:
- Close-targets identified: #12930 from PR body
Resolves #12930; GitHubclosingIssuesReferencesreports #12930. - #12930 confirmed not
epic-labeled: labels arebug,ai,grid; project itemNeo v13 ReleasestatusIn Progress. - Branch commit history contains only
test(grid): pool-born→permanent one-live-cell invariant net (#12930)and no staleResolves/Closes/Fixesbody keyword.
Findings: Pass.
📑 Contract Completeness Audit
- Originating ticket ACs are explicit.
- PR body maps AC1/AC2 to the already-merged fix/probe work, AC3 to this unit regression net, and AC4 to #12947.
Findings: Pass.
🪜 Evidence Audit
- PR body contains a greppable Evidence line:
Evidence: L2 ... → L2 required ... Residual: AC4 verification [#12947]. - Achieved evidence matches this PR's unit-shaped AC3.
- Residual AC4 has a live split issue (#12947), boardless unless fresh recount proves the invariant still broken.
- Review language does not promote the unit-layer proof into E2E/full-drag validation; PR body correctly leaves the whitebox-e2e layer as post-merge/follow-up.
Findings: Pass.
N/A Audits — 📡 🔗
N/A across listed dimensions: no MCP OpenAPI surface changed, and the PR does not introduce a new cross-skill workflow/convention.
🧪 Test-Execution & Location Audit
- Branch checked out locally via
checkout_pull_request. - Canonical Location: new unit test is under
test/playwright/unit/grid/. - Ran the specific new test file with the custom unit config through the npm script.
- CI verified current head:
gh pr checks 12948reportsAnalyze (javascript),Classify test scope,CodeQL,integration-unified,lint-pr-body, andunitall passing.
Findings: Pass. Local command: npm run test-unit -- test/playwright/unit/grid/RowCellIdMigration.spec.mjs → 4 passed.
📋 Required Actions
No required actions — eligible for human merge.
📊 Evaluation Metrics
[ARCH_ALIGNMENT]: 95 - 5 points deducted only because the full whitebox drag layer remains a follow-up; the unit seam itself is well-aligned with Neo's single-thread Playwright unit strategy.[CONTENT_COMPLETENESS]: 95 - 5 points deducted because the PR body is comprehensive and evidence-mapped, with only the usual dependency on linked issue comments for the #12938 fix provenance.[EXECUTION_QUALITY]: 95 - 5 points deducted because I did not independently rerun the destructive red-proof against historicalRow.mjs; current-head local focused test and CI are green, and the test shape directly targets the documented failing seam.[PRODUCTIVITY]: 100 - I actively considered under-delivery, hidden residuals, and close-target mismatch; #12947 cleanly carries AC4, so #12930's retained regression-net work is complete here.[IMPACT]: 70 - Release-relevant grid corruption guard, but localized to one unit regression net rather than a core behavior change.[COMPLEXITY]: 45 - Moderate-low: one 202-line spec with careful fake construction and source-seam understanding, but no production code or cross-package wiring.[EFFORT_PROFILE]: Quick Win - High release value for a small, focused test-only diff.
Approved for the human merge gate. Agents still do not merge PRs.
Resolves #12930
Authored by Claude Fable 5 (Claude Code). Session c29b5ccc-d711-4cde-8401-32d1e4bbc4f1.
The regression-net half of the
#12930lane split (PR#12938review cycle 2, RA-2: the fix landed there; the ticket closes when the committed net lands). One new unit spec —test/playwright/unit/grid/RowCellIdMigration.spec.mjs, 4 tests — pinningRow#createVdom's cell-identity migration across the P(column) = (hideMode === 'removeDom' && !locked) partition flip on a mounted row:__cell-N→__dataField; no duplicate id with Pass 1's re-issued placeholder; exactly one live cell per column — the0c6d5cf8eregression, pinned.The spec follows the
BodyCellMapping.spec.mjsprototype-borrow pattern (plainthiscarrying only what the borrowed methods read — noNeo.create, no #configs private-brand trip, no grid scroll plumbing). Deliberately immune to the detached-driver failure class triaged in#12941; like its pattern siblings it constructs no components, so it carries no bucket-B CI skip guard and runs in CI.Evidence: L2 (A/B falsification — net RED on pre-fix Row.mjs, GREEN at HEAD) → L2 required (AC3 is unit-shaped; AC1/AC2 delivered probe-verified by PR
#12938; AC4 split). Residual: AC4 verification [#12947].Deltas from ticket
#12938(0c6d5cf8e, lane split recorded in its review cycle 2) — this PR delivers the ticket's retained regression-net half only.#12936examples/grid/lockedColumnsfixture (PR#12945, in review) — tracked below as follow-up.Close-target AC mapping
IC_kwDODSospM8AAAABF1WsFw) + PR#12938item 3a; the unit-level repro is test 1 here.#12938(duplication ×8-14/walk → ×1-2/drop residual, attributed to#12939); the seam invariant is pinned here.Test Evidence
npm run test-unit -- test/playwright/unit/grid/RowCellIdMigration.spec.mjs→ 4/4 (713ms).src/grid/Row.mjsrestored fromc79eebb19(pre-fix dev; the only Row.mjs delta to HEAD is the 11-line id-migration hunk — verified viagit diff), the same command → 2 failed / 2 passed: exactly the two lock-flip-direction tests fail with the duplicate-id signature (6 ids, 5 unique — the__cell-1collision), while unlock-flip + steady-state (mechanisms already correct pre-fix) stay green. The net catches precisely the defect, nothing broader.npm run test-unit -- test/playwright/unit/grid→ 38 passed / 7 failed — the 7 are the pre-existing#12941set (identical baseline reproduced on clean dev tipc3d372054before this branch). This head adds 4 passing tests and breaks none.Post-Merge Validation
examples/grid/lockedColumnsonce PR#12945merges — the e2e-layer dual of this unit seam (operator-flagged second angle; rides the#12930fixture-retarget note or a dedicated follow-up leaf)