Skip to content

Dirty-lifecycle correctness: guarded dirty set, undo phantom sweep, animation split - #758

Merged
wass08 merged 5 commits into
mainfrom
perf/dirty-lifecycle
Sep 2, 2026
Merged

Dirty-lifecycle correctness: guarded dirty set, undo phantom sweep, animation split#758
wass08 merged 5 commits into
mainfrom
perf/dirty-lifecycle

Conversation

@wass08

@wass08 wass08 commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Metric (protocol step 1): every fixture settles to DIRTY 0 after every scripted action (charter backlog #1, findings 7/8).

Three mechanisms, three fixes

  1. Phantom marks — the undo/redo temporal subscriber marked a deleted node's parent + siblings but never cleared the deleted node's own mark. Undoing a wall split left phantoms for the whole session. → The subscriber's microtask now sweeps marks whose node is gone.
  2. Bypassed guard — ~120 call sites dirtyNodes.add() directly, skipping markDirty's consumer-kind guard. A wall's parentId is a level (dirtyTracking: false, no consumer), so blind parent-marks wedged forever ("1 level" dirty on the tower fixture). → dirtyNodes is now a GuardedDirtySet: the guard lives in add() itself, covering every call site with zero call-site churn. markDirty keeps its own guard (tests inject plain Sets).
  3. Animation as dirty — door/window animation systems marked dirty every tween tick, so DIRTY 0 was unreachable while anything animated. → A dirty mark is one-shot work again: DoorSystem rebuilds doors straight off doorAnimations entries, window types without a direct pose path use a transient pendingWindowAnimationRebuilds set, and only the settled pose gets a final one-shot mark. Skylight/elevator already pose directly — untouched.

The ?perf settle detector counts the raw set again (its live-only filter papered over exactly the leaks it must catch), and __pascalPerf.dirtyResidue() exposes the raw census (total / phantom / live-by-kind) to the scaling-matrix runner.

Before/after (protocol step 2, quiet machine)

metric before after
end-of-run stuck marks, rich 2× 47 (all phantom) 0
end-of-run stuck marks, rich 4× 273 (103 phantom) 0
end-of-run stuck marks, tower 2×18 82 0
end-of-run marks, Maxi 8× 169 wedged 87 in-flight → drains to 0 in 1.6 s
door-toggle quiet (all fixtures) 284–583 ms 265–370 ms
idle frame cpu rich 2× / 4× / tower 11.9 / 19.8 / 12.6 ms 10.7 / 17.3 / 11.1 ms (~−10%)

Stuck marks were defeating every consumer's empty-set early exit each frame — hence the idle gain on furnished/tall fixtures. Everything else within noise. JSONs linked from the charter evidence row (plans/performance/editor-scalable-scene-runtime.md in private-editor).

Gates (protocol step 3)

  • Editing: perf-regression-pack 5/5; click-batched-wall, click-hidden-wall, opening-placement-preview green. placement-preview-follows-cursor fails identically on the base build — pre-existing.
  • Visual: screenshot parity on Maxi 1× / rich 2× / tower — 0 / 4 / 122 differing pixels of 1.6 M (≤0.01%, GPU dither).
  • Bake: headless bake of Maxi 1× produces a byte-identical GLB hash on base and PR builds.
  • Unit: 1426/1426 core tests, incl. 3 new dirty-lifecycle tests. Door swing verified live with DIRTY flat at 0 throughout the tween.

Docs sweep (protocol step 5)

node-definitions.md taught the finding-8 pattern verbatim ("system advances animation, then calls markDirty") — corrected; dirtyTracking section now documents the GuardedDirtySet enforcement; the "markDirty per tick is fine" aside in tools.md + the review-architecture skill is scoped to bounded gestures, with per-animation-tick marks called out as a blocker.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Nq2rLE18tFVES2LU6HGokK


Note

Medium Risk
Touches core scene dirty semantics, undo/redo, and per-frame door/window rebuild paths—high churn but localized; behavior is heavily tested and perf-gated.

Overview
Fixes dirty-set lifecycle so perf settle can reach DIRTY 0 and stuck marks stop forcing full-frame rebuild work.

Scene store (use-scene) replaces the plain dirtyNodes Set with GuardedDirtySet: add() applies the same rules as markDirty (skip dirtyTracking: false, disabled plugin kinds; allow ids with no node yet). markDirty keeps an explicit guard for tests that inject plain sets. After undo/redo, a temporal subscriber sweeps phantom marks for node ids that no longer exist.

Door/window animation stops markDirty every tween tick (that blocked settle). DoorSystem rebuilds doors with active doorAnimations entries even when the dirty set is empty; WindowAnimationSystem uses pendingWindowAnimationRebuilds for types without direct pose paths; both issue a one-shot markDirty on completion.

Perf: PerfActionSettleSystem counts the raw dirty set again; __pascalPerf.dirtyResidue() exposes total/phantom/live-by-kind for scripted runs.

Tests + docs: three dirty-lifecycle unit tests; wiki/skill text clarifies bounded-gesture markDirty vs animation-loop marking.

Reviewed by Cursor Bugbot for commit 7c2fe83. Bugbot is set up for automated code reviews on this repo. Configure here.

wass08 and others added 5 commits September 2, 2026 13:30
The panel's DIRTY readout filters to live nodes, so scripted matrix runs
could not see phantom marks (deleted-node ids) or distinguish stuck kinds.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nq2rLE18tFVES2LU6HGokK
…nimation split

Three stuck-mark classes kept scenes from ever settling to DIRTY 0
(charter findings 7/8):

- ~120 call sites add to dirtyNodes directly, bypassing markDirty's
  consumer-kind guard — a wall's parentId is a level, so unconsumable
  level/building marks wedged forever. dirtyNodes is now a GuardedDirtySet
  whose add() applies the guard itself.
- Undo/redo rewrites nodes without the delete actions, leaving marks for
  nodes that no longer exist (rich-2x: 47 phantoms after one scripted
  run). The temporal subscriber now sweeps marks whose node is gone.
- Door/window animation systems marked dirty every tween tick, so DIRTY 0
  was unreachable while anything animated. A dirty mark is one-shot work:
  DoorSystem rebuilds doors straight off doorAnimations entries, window
  types without a direct pose path use a transient rebuild set, and only
  the settled pose gets a final one-shot mark.

The ?perf settle detector counts the raw set again (its live-only filter
papered over the phantoms it now must catch).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nq2rLE18tFVES2LU6HGokK
…ords, not markDirty per tick

The 'system advances animation then calls markDirty' recipe was the
exact pattern behind charter finding 8; the dirtyTracking section now
describes the GuardedDirtySet enforcement.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nq2rLE18tFVES2LU6HGokK
Unqualified, the aside could wave through an animation loop that marks
dirty every frame — the finding-8 class the GuardedDirtySet cannot
block, since animating kinds are legitimately consumable.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nq2rLE18tFVES2LU6HGokK
@pascal

pascal Bot commented Sep 2, 2026

Copy link
Copy Markdown

I hit an error while handling your request (Model unavailable on AI Gateway free tier: Free tier users do not have access to this model. Upgrade to paid credits at https://vercel.com/d?to=%2F%5Bteam%5D%2F%7E%2Fai%3Fmodal%3Dtop-up for unrestricted…).

Please try again, rephrase, or reach out if it keeps failing.

Error id: b9ac14df-209c-41bc-b454-bc690c3583a2

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 7c2fe83. Configure here.

if (nodes[id]?.type === 'window' && !dirtyWindowIds.includes(id)) dirtyWindowIds.push(id)
}
pendingWindowAnimationRebuilds.clear()
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pending window rebuilds dropped under load

Medium Severity

pendingWindowAnimationRebuilds is cleared before the rebuild loop, so a progressive cap or time budget can drop animation-only windows that are not already in dirtyNodes. Those ids are gone until the next animation tick, so mid-tween poses can skip frames when many windows are already dirty. Doors avoid this by keeping doorAnimations until the work actually runs.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 7c2fe83. Configure here.

@wass08
wass08 merged commit 17e7f4d into main Sep 2, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant