LearnNewsExamplesServices
Frontmatter
titlefeat: implement incremental updates for Card Layouts #8867
authorjhawpetoss6-collab
stateClosed
createdAtMar 24, 2026, 2:30 AM
updatedAtJun 2, 2026, 8:40 AM
closedAtJun 2, 2026, 8:40 AM
mergedAt
branchesdev ← strike/card-layout-incremental-updates
urlhttps://github.com/neomjs/neo/pull/9537
Closed
jhawpetoss6-collab
jhawpetoss6-collab commented on Mar 24, 2026, 2:30 AM

This PR implements the "Incremental Update" strategy for Neo.layout.Card, significantly reducing VDOM serialization and IPC overhead during card switches (#8867).

Changes:

  • Added incrementalUpdates config (default: true).
  • Refactored afterSetActiveIndex to use scoped updates.
  • The active card receives a full update (depth: -1), while others are pruned at depth: 1.
  • Ensures O(1) serialization performance regardless of the number of inactive cards.

/claim #8867

tobiu
tobiu commented on Mar 24, 2026, 10:54 AM

Input from Gemini 3.1 Pro:

✦ Thank you for submitting this PR! We really appreciate your enthusiasm for performance optimizations. The "Incremental Updates" strategy for Card layouts is a brilliant approach, and you've nailed the core concept: updating the container shell at depth: 1 while selectively expanding the active items.

Because our TreeBuilder prunes both the vdom and vnode trees symmetrically using neoIgnore, the VDOM worker handles these sparse payloads perfectly. This is a highly viable optimization.

However, there are a few framework-specific implementation details and verification steps that need to be addressed before we can merge this. If you are up for adjusting your PR, please follow this roadmap:

1. Fix the Hallucinated API

In src/layout/Card.mjs, the code attempts to call item.vdomUpdate.registerMerged(...). This API does not exist on the component instance.

VDomUpdate is a global singleton manager. To merge a child's update into the container's payload, you must call the manager directly.

What to do in src/layout/Card.mjs: First, add the import at the top:

import VDomUpdate from '../manager/VDomUpdate.mjs';

Then, replace your registerMerged logic inside afterSetActiveIndex with the correct signature VDomUpdate.registerMerged(ownerId, childId, childUpdateDepth, distance):

if (incrementalUpdates && !needsTransition && !needsUpdate) {
    if (isActiveIndex) {
        // Full subtree update for the active card. Distance is 1.
        VDomUpdate.registerMerged(container.id, item.id, -1, 1);
    } else if (i === oldValue) {
        // Shell only update for the old active card to apply the 'inactive' class / removeDom.
        VDomUpdate.registerMerged(container.id, items[oldValue].id, 1, 1); 
    }
}

2. Honor the Cube Layout Constraint

Neo.layout.Cube (src/layout/Cube.mjs) extends Neo.layout.Card. The Cube layout maps up to 6 items simultaneously onto 3D faces.

If incrementalUpdates defaults to true on the Card layout, the Cube layout inherits it. Pruning the inactive faces of the Cube layout will completely break the 3D integrity of the container.

What to do in src/layout/Cube.mjs: You must explicitly opt-out of incremental updates for the Cube layout by adding incrementalUpdates: false to its static config block:

    static config = {
        // ... existing configs
        className: 'Neo.layout.Cube',
    /**
     * Cube layout relies on all 6 faces being in the DOM. 
     * Incremental updates would break the 3D integrity.
     * @member {Boolean} incrementalUpdates=false
     */
    incrementalUpdates: false,

3. Verification via Unit Tests (Crucial Step)

To ensure this logic works deterministically and doesn't introduce regressions, we need to add a unit test.

Neo.mjs uses a "Single-Thread Simulation" architecture for testing VDOM logic instantly in Node.js.

Please take a look at test/playwright/unit/vdom/RealWorldUpdates.spec.mjs. It demonstrates how we test complex update scenarios by inspecting the resulting deltas directly.

What to do: Create a new test file: test/playwright/unit/layout/CardIncrementalUpdates.spec.mjs. Your test should:

  1. Create a Container with layout: {ntype: 'card'} and 3 child items (e.g., buttons or simple components).
  2. Manually trigger await container.initVnode().
  3. Switch the activeIndex (e.g., container.layout.activeIndex = 1).
  4. Wait for the update: const { deltas } = await container.promiseUpdate();.
  5. Assert: Inspect the deltas array. Verify that it contains operations for the shell and the newly active item, but does not contain updates for the deeply nested structure of the previously inactive items.

You can run your specific test locally using: npm run test-unit -- test/playwright/unit/layout/CardIncrementalUpdates.spec.mjs

(For more info on our testing setup, see: learn/guides/testing/UnitTesting.md).

Next Steps

If you can update your PR with the API fix, the Cube layout constraint, and a verifying unit test, we will be thrilled to merge it. This will be a massive O(1) performance win for large applications. Let us know if you have any questions along the way!

neo-opus-ada
neo-opus-ada CHANGES_REQUESTED reviewed on May 18, 2026, 12:00 AM

PR Review Summary

Status: Drop+Supersede (Cycle 1)

Peer-Review Opening: Thanks for raising this performance optimization — the core insight (incremental updates to avoid O(N) VDOM serialization on inactive cards) is architecturally valuable, and the initial implementation captures the right pattern. The roadmap @neo-gemini-pro relayed on 2026-03-24 (via @tobiu) gave a clear path to merge-ready state, but the PR has been silent for ~2 months. With the source ticket auto-stale-closed and the technical roadmap unaddressed, the substrate-correct path is closure-with-maintainer-salvage rather than continued wait.

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

  • Decision: Drop+Supersede (close this PR; maintainer-authored superseding PR will carry the implementation forward)
  • Rationale: §9.0 Cycle-1 Premise Pre-Flight fires on source-ticket-stale/currency-risk (#8867 auto-stale-closed 2026-05-09 by github-actions[bot] after stale-labeling 2026-04-24). Combined with author 2-month silence + 3 unaddressed technical defects from the cycle-1 roadmap, the iterative-fix-path is no longer viable. Per pull-request-workflow.md §10 abandoned-PR exception, maintainer salvage is the substrate-correct continuation path.

🕸️ Context & Graph Linking

  • Target Issue: #8867 (auto-stale-closed; will be reopened OR superseded by fresh ticket for the salvage PR)
  • Related Graph Nodes: Gemini's cycle-1 roadmap comment (2026-03-24); src/layout/Card.mjs; src/layout/Cube.mjs; Neo.manager.VDomUpdate singleton

🔬 Depth Floor

Challenge: Three unaddressed cycle-1 defects from Gemini's roadmap make the PR non-merge-eligible as-is:

  1. Hallucinated API — item.vdomUpdate.registerMerged({depth: -1}) is not a valid component-instance API. The correct call is the global manager: VDomUpdate.registerMerged(ownerId, childId, childUpdateDepth, distance) (import from ../manager/VDomUpdate.mjs).
  2. Cube layout inheritance bug — Neo.layout.Cube extends Neo.layout.Card. With incrementalUpdates: true as default, Cube inherits behavior that prunes inactive 3D faces and breaks the cube's 6-face integrity. Cube must explicitly opt out via incrementalUpdates: false in its static config.
  3. Missing unit test — The deltas-array assertion test at test/playwright/unit/layout/CardIncrementalUpdates.spec.mjs requested by the cycle-1 roadmap was never added. Without it, the optimization can regress silently.

🎯 Close-Target Audit

PR body uses (#8867) reference (informal) but no magic Closes #N keyword — so no auto-close cascade risk. #8867 is currently CLOSED via github-actions[bot] stale-bot (2026-05-09), not via this PR.

Findings: Pass on close-target syntax; source-ticket-currency-risk fires §9.0 trigger.

N/A Audits — 🛂 📑 🪜 📜 📡 🔌 🔗

N/A across listed dimensions: substrate-mutation PR with no public-surface impact (internal layout class only), no openapi.yaml touched, no JSON-RPC schemas, no architectural-pillar provenance triggers, no authority citations, no cross-skill integration surfaces.

🧪 Test-Execution & Location Audit

  • Changed surface class: code (src/layout/Card.mjs)
  • Location check: N/A — no test file present
  • Related verification run: None possible — the PR's implementation calls a hallucinated API; running tests would not reach the optimization path correctly
  • Findings: FAIL — no unit test exists at the canonical location (test/playwright/unit/layout/CardIncrementalUpdates.spec.mjs). Required per cycle-1 roadmap.

🛡️ CI / Security Checks Audit

  • Ran gh pr checks 9537: "no checks reported on the 'strike/card-layout-incremental-updates' branch"

Findings: No CI — branch was force-pushed (operator-relayed) but CI never re-triggered or didn't run on the latest state. Salvage path will need to trigger CI on the superseding branch.

📋 Required Actions

To resolve this PR, please address the following:

  • Close this PR (Drop+Supersede per §9.0 — author silence + auto-stale-closed source-ticket make iterative continuation non-viable). Maintainer-salvage commitment: @neo-opus-ada will author a superseding PR with the 3 cycle-1 defects resolved (correct VDomUpdate manager API + Cube opt-out + canonical unit test) per pull-request-workflow.md §10 abandoned-PR exception.

The contributor's original surfacing of the optimization is preserved in this PR's conversation + #8867's body + Gemini's roadmap. Salvage attribution will credit @jhawpetoss6-collab as co-author of the idea via the superseding PR's body.

📊 Evaluation Metrics

  • [ARCH_ALIGNMENT]: 80 — "Strong: incremental-update pattern aligns with Neo's reactive substrate; 20 points deducted because the hallucinated item.vdomUpdate.registerMerged API misuses the singleton-manager pattern (should call VDomUpdate.registerMerged directly, not via component-instance)."
  • [CONTENT_COMPLETENESS]: 60 — "Acceptable: PR description names the strategy and changes; 40 points deducted for absent unit test + missing Cube opt-out + un-updated implementation post cycle-1 roadmap."
  • [EXECUTION_QUALITY]: 40 — "Weak: hallucinated API + missing Cube constraint + no test execution evidence. Tests don't fail because they don't exist."
  • [PRODUCTIVITY]: 50 — "Mixed: identifies the optimization correctly + initial structure is right; doesn't complete the work per cycle-1 roadmap."
  • [IMPACT]: 70 — "Solid: O(1) vs O(N) VDOM serialization for inactive cards in card-layout containers — meaningful for apps with many inactive cards (deep card stacks, dashboard layouts)."
  • [COMPLEXITY]: 60 — "Moderate: single-file core layout change with cross-layout inheritance constraint (Cube) requiring careful opt-out; novel reasoning for the depth-vs-shell update interaction."
  • [EFFORT_PROFILE]: Maintenance — "Bounded substrate-discipline iteration with a clear roadmap; effort cost is real but well-scoped (~2-3 hours focused work for the maintainer salvage)."

🧠 Graph Ingestion Notes

  • [KB_GAP]: The PR's item.vdomUpdate.registerMerged({depth: -1}) reveals a documentation gap on how Neo.manager.VDomUpdate is consumed — it's a global singleton, not a component-instance attribute. External contributors may benefit from a learn/guides/ entry on the singleton-manager pattern for VDOM update merging.
  • [TOOLING_GAP]: No automated stale-check on PRs that link to auto-stale-closed tickets — would have surfaced this PR's currency-risk earlier in the lifecycle.
  • [RETROSPECTIVE]: Cycle-1 Premise Pre-Flight §9.0 trigger fires cleanly when source-ticket is auto-stale-closed (not just stale-labeled). The Drop+Supersede framing preserves substrate value (idea + roadmap) while clearing the abandoned-PR backlog. Maintainer-salvage via §10 abandoned-PR exception is the substrate-correct continuation when the technical fixes are bounded and the architectural value is real.

— @neo-opus-ada (Claude Opus 4.7, Claude Code) | Session 39eee906-3fd4-424f-9348-828b46ece38c

neo-gpt
neo-gpt CHANGES_REQUESTED reviewed on Jun 2, 2026, 8:17 AM

PR Review Summary

Status: Drop+Supersede

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

  • Decision: Drop+Supersede
  • Rationale: The optimization idea is still valuable, but this PR is no longer a good iterative merge path. The source issue #8867 is now CLOSED / NOT_PLANNED with the stale label, the March roadmap in the PR conversation remains unaddressed, and the current diff still carries the same framework-specific defects. A maintainer salvage PR should carry the idea forward on a fresh ticket/branch rather than asking this stale contributor branch to keep iterating.

Peer-Review Opening: Thanks for surfacing the Card-layout incremental-update idea. The performance goal is legitimate; the issue is that the current branch still misses the framework contracts needed to merge it safely.

🕸️ Context & Graph Linking

  • Target Epic / Issue ID: #8867 (closed stale / not planned); PR #9537
  • Related Graph Nodes: src/layout/Card.mjs; src/layout/Cube.mjs; src/manager/VDomUpdate.mjs; test/playwright/unit/vdom/layout/Cube.spec.mjs

🔬 Depth Floor

Challenge OR documented search (per guide §7.1):

Challenge: the PR still assumes a component-instance item.vdomUpdate.registerMerged({depth}) API. I verified the source directly: the PR branch only references item.vdomUpdate.registerMerged(...) inside src/layout/Card.mjs, while the actual implementation is VDomUpdate.registerMerged(ownerId, childId, childUpdateDepth, distance) in src/manager/VDomUpdate.mjs. That is not a harmless spelling issue; it means the core optimization path does not call the Neo manager contract.

Rhetorical-Drift Audit (per guide §7.4):

  • PR description claims O(1) serialization and scoped active-card updates; the diff does not substantiate a mergeable implementation because the merge API call is wrong.
  • The March roadmap comment already names the required API shape, Cube opt-out, and Card incremental-update unit test; those are still absent.
  • Source-ticket currency checked: #8867 is closed stale/not-planned as of 2026-05-09.

Findings: Fail; Drop+Supersede is the correct review shape.

🧠 Graph Ingestion Notes

  • [KB_GAP]: Contributor-facing docs still do not make the Neo.manager.VDomUpdate singleton-manager contract obvious enough; this PR reached for a component-instance vdomUpdate surface that does not exist.
  • [TOOLING_GAP]: The existing Cube unit spec passes but does not cover Card active-index incremental updates. The missing CardIncrementalUpdates.spec.mjs is exactly the coverage gap named in the prior roadmap.
  • [RETROSPECTIVE]: This is a useful stale-PR salvage case: preserve the contributor's optimization idea, close the stale branch, and re-implement with the manager API, Cube opt-out, and direct Card delta coverage.

🎯 Close-Target Audit

  • Close-targets identified: none. The PR title/body references #8867 and /claim #8867, but does not use a Closes / Resolves / Fixes magic-close keyword.

Findings: Pass on close-target syntax. Source-ticket currency still fails because #8867 is closed stale/not-planned.

📑 Contract Completeness Audit

  • The PR modifies consumed layout behavior on Neo.layout.Card but has no current open source ticket contract ledger to bind the public behavior.
  • Implemented diff does not match the already documented maintainer roadmap: manager API, Cube opt-out, and focused unit test are absent.

Findings: Fail; superseding maintainer work needs a fresh contract-bearing ticket or refreshed source issue.

🪜 Evidence Audit

  • The PR body has no Evidence: declaration.
  • Existing verification is insufficient for the claimed behavior: the branch lacks a Card incremental-update test and the only related focused test I ran (npm run test-unit -- test/playwright/unit/vdom/layout/Cube.spec.mjs) passes while not exercising the faulty Card active-index update path.

Findings: Fail for merge-readiness; this supports Drop+Supersede rather than iteration on this stale branch.

N/A Audits — 📡 🔗

N/A across listed dimensions: no MCP OpenAPI tool descriptions, agent skills, workflow conventions, or cross-skill substrate are modified.

🔌 Wire-Format Compatibility Audit

The PR changes VDOM update payload behavior for Card layout switching. The current implementation does not use the VDomUpdate manager wire/merge contract correctly (ownerId, childId, childUpdateDepth, distance), so it is not wire-compatible with the merge queue contract.

Findings: Fail.

🧪 Test-Execution & Location Audit

  • Branch checked out locally: PR #9537 at e1978e9fe in /private/tmp/neo-pr-9537-review.
  • CI check state verified: gh pr checks 9537 reports only lint-pr-review-body passing; no branch unit/integration CI is present.
  • Focused related test run: npm run test-unit -- test/playwright/unit/vdom/layout/Cube.spec.mjs -> 4 passed.
  • Coverage gap remains: no test/playwright/unit/layout/CardIncrementalUpdates.spec.mjs or equivalent Card active-index delta test exists.

Findings: Existing Cube coverage passes, but it does not cover the PR's changed Card behavior. The required Card incremental-update test is still missing.

📋 Required Actions

To proceed, please address the following:

  • Close this PR as superseded/stale. Preserve the optimization idea in a maintainer-owned follow-up that re-implements it with the correct VDomUpdate.registerMerged(ownerId, childId, childUpdateDepth, distance) manager API, Neo.layout.Cube opt-out, and focused Card incremental-update unit coverage.

📊 Evaluation Metrics

  • [ARCH_ALIGNMENT]: 45 - The performance direction aligns with Neo's VDOM update model, but 55 points are deducted because the implementation calls a non-existent component-instance API instead of the VDomUpdate singleton manager contract and lets Cube inherit behavior that conflicts with its six-face layout semantics.
  • [CONTENT_COMPLETENESS]: 45 - 55 points deducted because the source issue is closed stale, the PR body has no refreshed contract/evidence declaration, and the prior maintainer roadmap remains unaddressed.
  • [EXECUTION_QUALITY]: 35 - 65 points deducted because the changed Card path still contains a functional API defect and lacks the direct Card incremental-update test; the related Cube spec passes but does not exercise the faulty path.
  • [PRODUCTIVITY]: 50 - The PR contributes a valuable optimization idea, but it does not reach mergeable delivery for #8867 and now requires maintainer salvage.
  • [IMPACT]: 70 - If implemented correctly, O(1)-style Card switching could be meaningful for large inactive card stacks and dashboard layouts.
  • [COMPLEXITY]: 60 - Moderate: a one-file Card diff touches VDOM merge semantics and has cross-layout inheritance implications for Cube.
  • [EFFORT_PROFILE]: Maintenance - Stale contributor-PR closure plus bounded maintainer salvage; the architectural value is real, but the current branch is no longer the right merge vehicle.

Requesting changes with Drop+Supersede framing. Merge execution remains human-only under AGENTS.md.