fix(#448): one animator per Tiles wall — and the first real consumer of the #419 SourceRegistry - #511
Merged
Merged
Conversation
Answers #447 (T5.1) by naming exactly which #419 foundations slice 2 consumes, and makes #448 the first real consumer so those foundations can land on main wired rather than as an unwired island. The residual defect: adoptSettledFrom only hands a wall across when every tile is atRest, so a wall taken MID-ANIMATION still re-animates. With two per-bus animators there is no correct answer to that, which is why the fix is structural -- one animator per wall, one texture, buses sample it. Owner decisions recorded in the spec: the wall registers in SourceRegistry under a composed kind (not a capture kind, whose subscription/availability fields are meaningless for a wall); the plan carries ONE tiles-wall layer rather than collapsing expanded tile layers inside the D3D adapter, which pulls some Metal and CPU-preview parity forward deliberately; and the multiview PVW cell moves to sampling the Preview bus texture the way the PGM cell already samples Program. Identity was verified rather than assumed: the shell already emits tiles:<sceneId> (TilesLayerPayloadBuilder.cs:50) and the core takes it verbatim (MediaCore.cpp:1808), so dropping today's redundant sceneId prefix preserves the exact equivalence classes. Gates are recorded with their honest state: the iGPU budget number CANNOT be produced here (needs #425) and Metal will compile and pass CI but not be executed. Neither is quietly skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ
…uling)
The first draft said the composed kind "carries only Token, displayName, format
and publication state", which implied splitting SourceRegistry::Source into a
common core plus a capture payload. Investigating the cost changed the answer.
SourceRegistry::Source is one struct with a kind discriminator, and about five
of its fields are meaningless for a wall: no personId, no externalId (there is
no SDK handle), it cannot be Departed (a wall is released when unreferenced),
and it is never subscribed.
Two findings made this more than cosmetic:
- The defaults are ASSERTIONS. subscriptionObserved{false} is initialised
engaged, and its own comment says nullopt is what means unknown -- so a
registered wall would assert "subscription observed = false". That is the
shape of #468 and of "absent lifecycle means UNKNOWN, never healthy".
- This is a PUBLISHED, golden-tested contract, not an internal struct:
test/data/wave1-authority.json is authority-goldens-v2 with ~1,700 lines of
scenarios, and a serialized source carries personId, videoAvailable,
videoFresh, audioAvailable, audioFresh, audioMuted. Registering a wall forces
an answer to "is this wall's audio muted?".
Splitting the struct was therefore rejected on cost: it touches every existing
#419 consumer and its tests and reshapes those goldens -- #419 surgery smuggled
in under a Tiles-wall fix, when the owner asked for a minimal carve. Leaving the
fields as-is was rejected too: a field that serializes a false claim is how #468
happened.
The fields become std::optional and stay nullopt for composed sources, the
serializer omits them, and two new tests pin it: a wall never claims a
subscription state, and the existing capture goldens stay byte-identical.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ
Found while gathering detail for the implementation plan. Both errors were mine,
and one of them was the argument AGAINST the option I talked the owner out of.
1. The spec claimed SourceRegistry::Snapshot is a published, golden-tested wire
contract. IT IS NOT. That snapshot is consumed in-process by
ShowPlanGenerator and SceneVersionShadow and never reaches the wire or the
session state. The authority-goldens-v2 scenarios belong to
ZoomSourceAuthorityAdapter::Observation::Source -- the Zoom ROSTER OBSERVATION
synced INTO the registry, carrying videoFresh/audioMuted/personId. A wall
registered directly in SourceRegistry never appears there.
2. Because of (1), the stated cost of splitting the struct ("reshapes ~1,700
lines of goldens") was fabricated. Measured: about seven files in the Zoom
authority path plus tests. Real but modest.
The conclusion still stands after re-deciding with correct facts (owner: keep
the nullopt approach), but on NARROWER grounds: subscriptionObserved{false} is
initialised engaged while its own comment says nullopt means unknown, so a
registered wall asserts "subscription observed = false" -- and that feeds PLAN
GENERATION. The split is deferred to slice 3, where more than one composed kind
would justify the shape.
Also records a finding that applies whichever option is chosen:
validRegistration requires a non-empty externalId, so add() returns Invalid for
a wall today, and externalConflict matches kind+processEpoch+externalId, so two
walls with empty external ids would collide. Both now have named tests.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ
Four tasks, each ending in an independently testable deliverable:
1. SourceRegistry admits a composed source and never claims a subscription
state for it (both current rules reject a wall today: validRegistration
demands a non-empty externalId, and externalConflict would merge two walls
that both have none).
2. TilesWallSource owns one wall's animation, keyed by wall id.
3. MediaCore uses one animator per wall and adoptSettledFrom is DELETED.
This is the task that fixes #448.
4. The wall registers as a composed source and the take record proves its
continuity with a generation.
Split from the spec deliberately. Plan 1 changes NO drawing, so it can land
while beta-2026-09-12-c425e3c is still being live-checked; plan 2 (wall texture,
one tiles-wall layer, Metal/CPU parity, PVW cell, AtomicTake) changes the render
path and should wait for that beta to be shaken out. Each plan produces working,
testable software on its own.
Self-review caught a type inconsistency worth recording: the first draft had
TilesPlanAnimation calling a std::function installed by TilesWallSource, which
Task 2 never installed AND which would allocate on the render tick. Replaced by
advance() returning bool "did I reset", which the wrapper turns into a
generation bump -- so the generation cannot move without the animator moving,
and vice versa.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ
…nchanged
Prerequisite for plan 1 of persistent-sources slice 2. No behaviour change: the
files are taken verbatim from origin/codex/production-realtime-architecture and
wired into the existing targets.
The carve is clean because SourceRegistry.cpp includes only its own header plus
<stdexcept> and <utility> -- zero project dependencies -- which is the condition
the parent spec set ("SourceRegistry.h is reused only if it fits cleanly").
Nothing else from #419 is needed: SceneVersionShadow and ShowPlanGenerator
consume the registry but are not part of this plan.
Verified: its own 22 tests pass, and the full suite is 953 green (931 + 22).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ
…n state for them Also fixes a dangling-reference bug in the ACaptureSourceStillCarriesItsCaptureFields test: `registry.snapshot()->sources.front()` as one expression drops the temporary shared_ptr (and its Snapshot) at the semicolon, leaving `source` a dangling reference. Bind the shared_ptr first, same pattern already used by the other new test in this file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ
…narrow the composed conflict bypass to externalId Fix round 1 review findings: 1. retireProcessEpoch and setAvailability reached a composed source's fields unconditionally, reintroducing the false claim install() was fixed to remove (nullopt flipping to concrete Departed/false). Both now SKIP a composed source instead of marking/mutating it: retirement has nothing to fence (no provider process, no callbacks) and availability does not apply to a wall at all. Not erased, not marked Departed - a wall's lifetime is scene-reference, released by a separate sweep in a later task. 2. externalConflict's composed bypass disabled the instanceId collision check too, not just the externalId one. Restructured so only the externalId clause is skipped for a composed registration; two composed registrations sharing an explicit instanceId still conflict. Adds 3 covering tests, two confirmed red (via a temporary guard revert) before the fix and green after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ
TilesWallSource wraps TilesPlanAnimation and bumps a generation whenever the wrapped animation reports it reset, giving the take record's proof that nothing restarted (spec section 5) as a number instead of an opinion. TilesWallSources maps wall id -> source so both buses asking for the same wall get the same object; lifetime follows "referenced by a scene" via releaseAllExcept, and a recreated wall starts at generation 0 rather than continuing a retired one's count. TilesPlanAnimation::advance now returns bool (true when it reset the animator) instead of void, so TilesWallSource::advance can detect a reset with no std::function/allocation on the render tick. Every existing caller (MediaCore, TilesAnimatorTest) already discards the return value as a statement, so this is a source-compatible widening. Not wired into MediaCore yet - that is the next task. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ
Review round 1 finding 1: the original 5 tests never called TilesWallSource::advance - they only exercised noteReset()/generation bookkeeping directly, leaving the actual glue (bump the generation iff the wrapped TilesPlanAnimation::advance reports a reset) completely untested. Add two tests driving advance() directly: a different wall key arriving moves the generation by exactly one (the reset path), and the same wall key arriving twice does not move it. Verified both are load-bearing by temporarily breaking the glue two ways (unconditional noteReset(), and an inverted condition) and confirming each break turns at least one test red before restoring the fix. Finding 2: add the missing <iterator> include for std::next, previously compiling only because MSVC's <map> transitively pulls it in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ
…ontinuous MediaCore held programTilesAnimation_ and previewTilesAnimation_, and handed settled state from one to the other on a Take via TilesPlanAnimation::adoptSettledFrom. That hand-off refused a wall whose tiles were still flying (mid-flight state had no correct owner with two animators), so a wall taken mid-animation re-animated its entrance from alpha 0 on Program. MediaCore now keeps ONE core::TilesWallSource per wall id (tilesWallSources_, from Task 1/2), shared by whichever bus is looking at it, so there is nothing to hand over. adoptSettledFrom is deleted. TilesPlanAnimation::advance already reports whether it reset (bool return); TilesWallSource turns that into a generation bump. TakeRecordPolicy's wallAdoptedSettled is renamed wallContinuous: it no longer means "state was handed over" but "the wall's generation did not move across this take", computed in MediaCore by comparing tilesWallGeneration() before and after the take-tick's advance. The verdict string "adopted-settled" becomes "continuous". Rewrote the three TilesAnimatorTest.cpp hand-off tests that pinned the old per-bus behaviour: a wall settled on one bus is now trivially settled on the other (same object); the refused-hand-off case becomes "a wall never cued starts cold, at generation 0, in its own object, contaminated by nothing else"; the all-stale-beat guard test is kept as-is (it already tested the shared animator directly, and matters more now that a wipe hits every bus at once). Added the headline regression test (TilesRenderPlanTest.cpp, AWallTakenMidAnimationIsContinuous). A wall's own first tick with content pops in fully opaque by design (TilesAnimator treats a fresh animator's first non-empty sample as pre-existing, not entering — see DifferentWallCannotReuseAnotherWallsCachedGeometry), so the test instead joins a member onto an ALREADY-ESTABLISHED wall and takes it before the join settles. Verified red-before-green by temporarily restoring the base (aa35f4f) per-bus mechanism: the same scenario failed with the new tile snapping straight to full opacity instead of continuing its entrance, confirming the test exercises the real defect. Closes #448 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ
…, freeze fix, verdict/rect assertions Fixes five review findings on the #448 one-animator-per-wall change: - Finding 0: TilesPlanAnimation::advance's not-present/disabled early return now reports whether it actually reset (key_ was non-empty), not "true" unconditionally — a repeated disabled tick is a harmless no-op, not a generation tick-counter. - Finding A (Critical): the preview GPU-composite applyLatest call used forWall(), inserting a phantom ""-keyed wall for any preview scene with no wall, under coreMutex, every render tick. Switched to find() + null-check, matching the two multiview sites. - Finding B: the double-advance guard keyed on wall-id equality alone, so a wall with animateLayout=false on Program and true on Preview at the SAME id was advanced by neither branch (frozen). Hoisted `programAdvanced = present && animateLayout` and gated preview's advance on `!programAdvanced || previewWallId != programWallId`. - Finding C: advance() was only called when animateLayout was true, so a disabled wall's sampled_ was retained forever and kept feeding stale geometry to the multiview PGM cell and the preview composite. Restored the unconditional call whenever the wall is present, letting `enabled` decide reset-vs-sample inside advance() (safe now under Finding 0's guard). - Finding D: added an end-to-end assertion in the headline test that the take record itself reads wall=="continuous", verdict=="cut" — not just the raw generation number. - Finding E: added rect-continuity assertions for tiles whose opacity was already 1 at the mid-flight tick (only the newly-joined tile had a ramping alpha; the pre-existing two tiles' RECTS were mid-spring from the layout change, unasserted before this). Self-caught while implementing Finding C: widening the advance gate to "present" alone broke ATakeIsRecordedWithBothSidesOfTheWall — a wall id seen for the first time starts at generation 0, and with Finding 0's idempotence a disabled first call also reports "did not reset", so raw generation equality misread a brand-new non-animated wall as continuous. Fixed by requiring wall EXISTENCE (find() before forWall()), not just generation equality. Self-caught while implementing Finding E: the rect-continuity check first failed against the CORRECT implementation too, because the shared wall's 100ms clamped-floor duration converges audibly close to target within a few ms of real test-process overhead. Added wallSceneWithDuration() and drove the test at the animator's 2000ms clamp ceiling for a reliable margin. Re-verified red against the reverted base mechanism with both new teeth (Finding D's take-record assertion and Finding E's rect check both fail); re-verified green on the fixed tree, 968/968, three consecutive runs with no flakiness. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ
…ith contradictory enabled Re-review confirmed round 1's wallExistedBefore term and the 2000ms tolerance retune were both sound. It found one new Important finding introduced by Findings B and C combining: a wall present on both buses with DISAGREEING animateLayout (Program false, Preview true, or vice versa) was advanced TWICE in one tick on the SAME shared object with contradictory `enabled` values — Program's call (enabled=false) reset it, then Preview's separate call (enabled=true) saw an empty key and reset it AGAIN. Net per tick: generation +2, no animation ever completing, the object re-adopted from scratch every tick. Not a verdict inversion (it read the conservative "rebuilt"), but incoherent under the one-animator-per-wall model this task exists to establish. Restructured the take-tick block so exactly one advance() call happens per wall per tick, true by construction: - `sameWall` decides whether Program and Preview name one object or two. - When they share an id, `enabled` is the OR of both buses' animateLayout — a wall animates if EITHER bus wants it to — and there is exactly one advance() call to carry that decision. - Preview only gets its own separate advance() call when it names a DIFFERENT wall id (a genuinely different object). Kept everything the round-1 re-review confirmed: the `had` idempotence guard in TilesPlanAnimation::advance, find() on every read path, the wallExistedBefore existence check, and the unconditional-whenever-present call shape that fixed Finding C (now applied symmetrically to both branches). Added TilesRenderPlan.ASharedWallWithDisagreeingAnimateFlagsAdvancesAtMostOncePerTick — no prior coverage exercised this configuration, which is why it took two review rounds to surface. Verified red against c11862d (generation climbed every tick) and green after this restructure, across three consecutive full suite runs (969/969, 0 failed). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ
…ns for a shared wall, drop empty-id exemption Three fixes from re-review of round 2's restructure: Finding 1 (performance): the preview branch's `!sameWall` gate lost its `animateLayout` check, so buildPreviewCompositorRenderPlan(videoFrames) — a full buildRenderPlanForScene deep build, ~13 strings per layer, plus the paused-clip-cue pass — ran every render tick under coreMutex for any disabled preview wall (the DEFAULT) with a different id from Program, then was thrown away: advance() returns at its very first line when !enabled and never touches the plan. Same class of waste the "Task 4 review fix (I6)" comment a few lines below already exists to prevent. Now the deep plan is only built when previewEnabled is true; the disabled path passes the already-built program `renderPlan` instead (advance() never reads it there). Finding 2 (behaviour, RULING): `enabled = programEnabled || previewEnabled` let a PREVIEW-side animateLayout toggle start motion on PROGRAM for a shared wall, with no take involved, because the shared advance samples straight into the program renderPlan. This codebase's bedrock rule is that an off-air Preview look never changes what is on air (CLAUDE.md: "an off-air Preview look can never take video ... from a Program source") — the preview scene is an operator-editable draft (S2b), so a draft edit reaching Program is a live-show hazard, and Program is inherited by the vcam, every recording and every stream. Changed to `enabled = programEnabled` — Program always wins for a shared wall. Cost-free thanks to round 1's Finding C: a disabled shared wall still resets (idempotently), sampled_ clears, and applyLatest no-ops on the mismatched key_, so Preview simply falls back to raw (non-animated) geometry rather than stale rects — nothing on air moves because of an off-air edit. Finding 3 (minor): dropped the `!programWallId.empty()` term from `sameWall` — two empty ids are the SAME map entry (TilesWallSources keys on the string), so excluding them re-opened the exact two-advance-on-one-object shape round 2 closed. No generation effect, but the wrong shape regardless. Rewrote the round-2 test to assert the NEW property directly: with Program's animateLayout=false, a wall shared with an animateLayout=true Preview never leaves generation 0 — not "settles after climbing once" (round 2's OR behaviour), which the rewritten test now catches if ever restored (verified by temporarily reintroducing the OR: the test fails on tick 0, as expected). Verified: covering suites green (TilesRenderPlan 29, TilesAnimator 11, TakeRecord 11, RenderedSceneAttribution 3), full binary 969/969 across three consecutive runs, check-string-escapes.py clean. Deferred per coordinator instruction, NOT fixed: forWall() admitting an empty wall id on the write path; the rect-tolerance/duration headroom. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ
…real precondition Finding 1 (Important, coordinator's own prescribed shape): the disabled preview-wall branch passed the PROGRAM render plan into a PREVIEW wall's advance() call. Correct today only because advance() returns at its very first statement when !enabled — an invariant living in a different file from the call site depending on it. If that early return is ever reordered or gains code above it, applyLatest would silently rewrite rect/opacity on every tile:* layer of the PROGRAM plan (the same object handed to compositor->render() and cached into lastRenderPlan_): on-air geometry corruption inherited by the virtual camera, every recording and every stream. Removed the hazard rather than documenting it further: - TilesPlanAnimation::releaseIfIdle() — plan-free by construction, cannot ever read a plan because it takes none. Same idempotent generation contract as advance()'s early return (had = !key_.empty() before reset). - advance()'s early return now just calls releaseIfIdle(). - TilesWallSource::releaseIfIdle() wraps it and calls noteReset() on a real release, matching advance()'s generation semantics exactly. - MediaCore's disabled-preview branch now calls tilesWallSources_.forWall(previewWallId).releaseIfIdle() — no plan argument exists to misuse. - Documented advance()'s `plan` parameter as read ONLY on the present-and-enabled path, so the next person knows releaseIfIdle() is the only correct call when there is no real plan for a wall. - Left the all-stale early return (`targets.empty() && !sampled_.empty()`) untouched — it runs after the key_ assignment and was never part of this hazard. Finding 2 (Minor): the round-3 test was entirely negative assertions (generation == 0), which pass vacuously if the shared configuration itself stopped existing. Added preconditions: Program's wall must actually render "tile:zoom:1" (forced via the existing setTilesMemberFrameAgesForTest seam, which only rebuilds lastRenderPlan_ and cannot disturb tilesWallSources_/generation), and Preview's scene must have landed on the SAME "tiles:s" wall id with animateLayout true — before the zero-generation assertions run. Verified: covering suites green (TilesRenderPlan 29, TilesAnimator 11, TakeRecord 11, RenderedSceneAttribution 3), full binary 969/969 across two consecutive runs, check-string-escapes.py clean. Generation semantics are unchanged by the refactor by construction (releaseIfIdle IS the old early return, extracted) — the round-2/3 tests pass for the same reason as before. Still deferred per coordinator instruction: forWall() admitting an empty wall id on the write path; the rect-tolerance/duration headroom. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ
…n unreferenced The wall is the first real consumer of SourceRegistry's Kind::Composed (#448 slice 2, task 4). Registers as Composed the tick a wall becomes live (sourceId = its layerId, externalId empty, a fixed core-process epoch) and releases it the tick nothing on either bus names it any longer, in lockstep with tilesWallSources_.releaseAllExcept — the same "named by a live scene" lifetime, one level up. registeredWallIds_ is the idempotence guard so a live wall's steady-state tick never touches the registry mutex. SourceRegistry had no way to actually remove a composed source (real sources are tombstoned via retireProcessEpoch/setAvailability, which explicitly SKIP composed sources with a comment deferring removal to "a separate sweep (later task)"). Added SourceRegistry::removeComposed, which erases outright (never tombstones) and refuses non-Composed kinds - the wall has no provider process to fence, so a tombstone would be a permanent, meaningless entry that also makes add() answer Conflict forever for a freed wall id. Per explicit ruling, this task does NOT touch TakeRecordPolicy or the take record's sources[] - that continuity fact already exists via wallContinuous, and a second representation of it is exactly what this codebase forbids elsewhere (one status machine per fact). Tests: SourceRegistryComposedTest.cpp (removeComposed: erases, frees the id for reuse, refuses non-composed, NotFound on unknown) and TilesRenderPlanTest.cpp (a live wall registers as Composed with all five capture-only fields nullopt; an unreferenced wall is gone from the registry; a released-then-re-cued wall gets a new instanceId; a wall staying live across many ticks holds exactly one registry entry). Full suite: 977 passed, 0 failed (was 969). Red-before-green verified by reverting MediaCore.cpp/.h to HEAD with the new tests in place (compile failure), then restoring. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ
…ailure, alloc-free by construction Fix round 1 of the Task 4 wall-registers-as-composed-source review. Finding 1 (Important): ALiveWallHoldsExactlyOneRegistryEntryAcrossManyTicks asserted a count std::map makes structurally impossible to violate, and was near-unfalsifiable - dropping the idempotence guard entirely (add() answering Conflict on every tick, taking the registry mutex 60x/s on the render path) still passed it. Replaced with ALiveWallKeepsTheSameRegistryIdentityAcrossManyTicks, asserting the wall's instanceId is identical across 25 ticks - the realistic regression (remove-and-re-add every tick) mints a fresh instanceId, which this catches and a count cannot. Verified genuinely falsifiable by simulating that exact regression and confirming red, then restoring and confirming green. Finding 2 (Minor): a registration that failed (Invalid is reachable, not just Conflict/Exhausted) was remembered as registered anyway, so it was never retried while the wall stayed live. Now the guard id is only added on Result::Applied, so the next tick retries a failed wall. Finding 3 (Minor): registeredWallIds_.insert(id).second is not guaranteed allocation-free on an already-present key on every implementation (MSVC's unordered_set has historically built the node before detecting the duplicate). Switched to contains()-then-insert so the render-path no-allocation rule holds by construction. Finding 4 (Minor): three SourceRegistry comment fixes - removeComposed takes a bare SourceId deliberately (sole-owner lifetime, no replace()- style fence); retireProcessEpoch's composed-skip comment names removeComposed instead of "later task" and explains why a tombstone was never actually reachable (setAvailability refuses Composed, and a permanently-nullopt availability makes externalConflict's Departed check permanently true); Snapshot::sources notes composed sources vanish rather than tombstone. Finding 5 (Minor, merge-blocking): added a CLAUDE.md section documenting the composed-source erase-vs-tombstone asymmetry and both reasons for it, per this repo's docs-updated-is-part-of-done rule. Full suite: 977 passed, 0 failed (x2 consecutive). Escape scanner clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ
…a gate `live-meeting-soak.mjs` gates "did the picture move" on frame-to-frame YAVG exceeding 0.05, because 8995 frames of flat luma once passed every validator that counted frames. The fake engine's motion was a ONE-ROW band that brightens row N and restores row N-1, which changes the frame mean by ~0.0007 — so a rig delivering a measured 60.0/s scored 0% of frames moving, and the gate could never pass on the only engine that can drive this soak headlessly. A gate no harness can satisfy is unfalsifiable, which is worse than no gate. Pulse a BLOCK instead: h/8 rows stepping 7 luma per frame is ~0.9 YAVG at the frame level, 18x the threshold, at 1/8 the cost of the full repaint this function deliberately avoids (that repaint made the PRODUCER the bottleneck at ~22fps/source). Offset by participant id so a wall's tiles carry different values while all advancing together. Measured after: 5 minutes, 20 takes, 8 sources — movingFrameRatio 0.0 -> 0.998, LIVE-MEETING SOAK PASS, with the take property intact (20 cut, 0 rebuilt) and 60.0/s render, 0 deadline misses, 0 dropped video. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ
…l-registration retry, correct the defect narrative Final-review findings on the #448 slice, plus the documentation defect the review surfaced. setSubscription refused Kind::Composed nowhere, while CLAUDE.md asserted it did. An observed:true call was already refused as a side effect (a nullopt availability is not Available), but requested:true with observed nullopt applied cleanly and turned a NOT-APPLICABLE field into a concrete claim — the exact lie the nullopt fields exist to prevent. Nothing in the tree calls it for a wall, so only the docs were wrong, which is how an invariant rots. Red-verified first. The wall registration loop retried a PERMANENTLY invalid id forever. A failed add is deliberately not remembered as registered so transient failures retry on the next liveness transition — but an id refused on its SPELLING can never become valid, so that retry became a registry-mutex acquisition plus an unbounded log line on every render tick. SourceRegistry::kMaxIdBytes is now public (one declared bound, used by validRegistration) so a caller can tell the two cases apart; unregisterableWallIds_ skips the permanent ones once and loudly, and warnedWallRegistrationIds_ bounds the retryable failures' line to once per id while keeping the retry. Both are pruned on the same liveness rule as registeredWallIds_. THE DEFECT NARRATIVE WAS BACKWARDS in CLAUDE.md, the spec, the plan and the headline test's own comment: a reset does not "replay from alpha 0". TilesAnimator treats a reset animator's next non-empty sample() as an ADOPTION, so the wall SNAPS TO ITS FINAL STATE — alpha pops to 1, mid-spring rects jump to settled. The consequence is not cosmetic: EXPECT_GE(alpha_after, alpha_before) is satisfied by a snap just as well as by continuity and catches NOTHING (the first draft of that test passed against the unfixed code). The falsifying assertions are the ones bounding the other side — post-take alpha below 0.9, and a tile already at opacity 1 keeping its mid-spring rect — and those are what was verified red. Documented at all four sites, with the honest limit that the generation-equality check is not independently falsified by that revert. Also: CLAUDE.md claimed build-dev is a single-config generator. It is Visual Studio 18 2026 with CMAKE_RUNTIME_OUTPUT_DIRECTORY pinned for every config, so Debug and Release write the same exe path and a missing --config Release silently installs a Debug core. That cost a false regression report on 2026-09-12 (coreMutex 1% -> 81%); the tell was uniform inflation across trivial stages and an 8,322,560-byte binary against Release's 2,168,832. Documented with the check. Plus: the registry section now says it has a production writer and no production reader yet, and the shared-wall applyLatest coupling is commented at both multiview cell sites and filed to plan 2. Tests: 978 passed, 0 failed on the Release core. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ
This was referenced Sep 13, 2026
… the backlog The final gates re-run on the post-fix binary (978 tests, SHOW DRILL PASSED, LIVE-MEETING SOAK PASS at 20 takes / 0 rebuilt), the soak motion gate's unfalsifiability and its ruling, the environmental cause of the first soak failure (we join as host, so killing the app ended the meeting), and the record that the defect narrative was backwards in four places. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ
…sm confirmed, trigger not reproduced Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is
Slice 2 of the persistent-sources redesign (#448), and the first real consumer of the #419 realtime-architecture foundations — which is why
SourceRegistrylands here rather than as an unwired island (docs/BACKLOG.md: a #419 foundation lands onmainonly with its first real consumer).Spec:
docs/superpowers/specs/2026-09-12-tiles-wall-persistent-source-design.md(owner-approved)Plan:
docs/superpowers/plans/2026-09-12-tiles-wall-one-animator.md(plan 1 of 2)The defect
A Tiles wall cued in Preview and taken to Program loses its in-flight animation on the cut. Owner, live show 2026-09-09: "what I can't have is a total rerender from what is in preview to program like it is loading for the first time."
The first fix was a hand-over (
TilesPlanAnimation::adoptSettledFrom), which could only move a settled wall — with two per-bus animators, mid-flight state had no correct owner. So a wall taken mid-animation still broke.This replaces the hand-over with one animator PER WALL (
core::TilesWallSource, keyed by wall id, shared by both buses). There is nothing to hand over, so the cut is continuous even mid-flight.adoptSettledFromis deleted.The narrative was backwards, and it mattered
A reset does not replay from alpha 0.
TilesAnimatortreats a reset animator's next non-emptysample()as an adoption — content already present, not entering — so the wall snaps to its final state: alpha pops to 1, mid-spring rects jump to settled. On air that reads as a wall that stops moving and jumps.The consequence is not cosmetic:
EXPECT_GE(alpha_after, alpha_before)is satisfied by a snap just as well as by continuity and catches nothing — the first draft of the headline test passed against the unfixed code. The falsifying assertions are the ones bounding the other side (post-take alpha stays below 0.9 for a tile that was mid-ramp; a tile already at opacity 1 keeps its mid-spring rect), and those are what was verified red by reverting. Corrected in the spec, the plan,CLAUDE.mdand the test's own comment.The #419 carve
SourceRegistryarrives on main's line unchanged (one commit), then gainsKind::Composedfor sources the core renders rather than captures. The Tiles wall is the first one.personId,externalId,availability,subscriptionRequested,subscriptionObservedare allnullopt—nulloptmeans not applicable, neverfalse.setAvailability/setSubscriptionrefuseComposedoutright.removeComposed). Not a simplification — the only mechanism that works:setAvailabilityrefusesComposed, so nothing could ever flip it toDeparted, and anulloptavailability makesexternalConflict's!= Departedtest read true permanently, so a tombstoned wall id could never be reused.TilesWallSources::releaseAllExceptone level down.CLAUDE.md.Verification
mac-show-drill.py --seconds 40 --load 8live-meeting-soak.mjs --minutes 5 --takes 20 --sources 8MonitorRenderFaultInjection(COREVIDEO_TIMING_TESTS=1)Two measurement traps this branch fell into and fixed, both worth reading:
coreMutexover-budget 1% → 81% and I reported it to the owner as a real regression.native/build-devis a multi-config generator withCMAKE_RUNTIME_OUTPUT_DIRECTORYpinned for every config, so Debug and Release write the same exe path and a missing--config Releasesilently installs a Debug core (8,322,560 bytes vs 2,168,832). The tell was uniform inflation across trivial stages (emit 32×, plan 19×). With--config Releaseevery metric matched baseline.CLAUDE.mdclaimed the opposite ("single-config generator") and now documents the check.movingFrameRatio0.0 → 0.998.Not done here (plan 2)
The wall texture / one-layer compositor slice, Metal + CPU-preview parity, the PVW cell as the registry's first real reader, and
AtomicTakeCoordinator. The shared-wall coupling is the deliberate trade this slice accepts and is commented at both multiview cell sites: one wall id has one animator, so a gallery cued in Preview and live on Program shows Program's wall geometry in the PVW cell rather than an independent Preview animation. Program is the authority (enabled = programEnabled),applyLatestis a pure read, and making the cut continuous is the entire point of #448.🤖 Generated with Claude Code
https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ