Skip to content

🐛 fix: apply initial focus and support bounded static fades - #146

Merged
cfviotti merged 10 commits into
mainfrom
fix/static-focus-transitions
Oct 20, 2026
Merged

cfviotti merged 10 commits into
mainfrom
fix/static-focus-transitions

Conversation

@cfviotti

@cfviotti cfviotti commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Description

On still images, focus drawn after annotations become ready could remain transparent until another display change. Apply it before rendering the ready image, and add optional transitionDurationMs to FocusStyle / BaseFocusStyle for subsequent focus changes.

Current SDK WebGL session: initially dimmed background, selection cleared, ticker stopped

Actual WebGL media session at fc6dab4. The marked background pixel is RGB 112,96,80 at darkness 0.5; the undimmed source is 224,192,160. Selection and clearing fade, then rendering stops.

focusStyle: new BaseFocusStyle({
  targetMode: FocusTargetMode.Ambient,
  transitionDurationMs: 150,
})
Route Behavior
Still image, duration omitted Focus settles before the first ready render; later changes are immediate.
Still image or engine-backed video, positive duration Initial resolved focus settles; later changes animate outside atomic video presents. The on-demand ticker stops after the fade.
Explicit 0 Focus changes settle without an animation ticker.
Engine-backed video, duration omitted Default fades advance with presented media time; existing interaction notifications settle them, independent content/visibility redraws preserve alpha.

The duration also accepts a focus-context resolver. Default src video retains its continuous pull ticker. See the presentation styles guide.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation or example update
  • Refactor or maintenance
  • Performance improvement

Validation

  • Full npm run verify at fc6dab4: formatting, lint, typechecks, 3,594 tests, documentation/contracts, package smoke and builds. Clean build-order regression also passes without pre-existing web output.

  • Focused removal checks distinguish initial settlement, default visibility redraws, explicit settlement, ticker scheduling and cleanup. Installed Pixi 8.20.1 and Vitest 4.1.11 match the lockfile.

  • Real ANGLE WebGL still-image checks at fc6dab4: first ready dimming; 150 ms selection/clear fades; settled idle counters; explicit 0 ms changes without ticker starts; focus removal and destruction. Application paints match scheduler counts.

  • Real engine-backed WebGPU video at fc6dab4: default playback, explicit 150 ms playback, and explicit paused fades. Paint counts match scheduler counts, geometry follows the presented frame, and settled counters remain unchanged.

  • At 9e77637: 61 focused runtime tests, web build, lint and formatting pass. The public-factory default-to-explicit duration case fails with zero ticker starts when the scene repair is removed; the omitted-duration case preserves immediate interaction settlement.

  • A fresh portable package at 9e77637 supports both public focus assignments in a clean external TypeScript consumer with strict and exactOptionalPropertyTypes.

  • Fresh visible GPU checks at 9e77637: WebGL still-image 150→300 ms retiming preserves fade direction; explicit zero stays immediate. Engine-backed WebGPU video, paused and playing, preserves partial alpha when changing from the default to an explicit 300 ms duration, then fades and stops. Settled idle and destruction cleanup pass.

  • Required CI verify passes at 9e77637.

Notes For Reviewers

The browser fixture uses generated images, shapes and video. GPU readbacks are behavioral checks, not rendering-cost benchmarks. A consuming application's annotation-count policy and saved data need their own validation.

Direct settlement no longer expires a paused missing-detection hold as the previous synthetic clock advance did. A paused gap can retain dimming until detections arrive, a seek, or media-time advancement clears it. This recoverable difference is accepted here without redesigning the hold lifecycle. No version bump or publication is included.

@cfviotti cfviotti self-assigned this Oct 16, 2026
@cfviotti

Copy link
Copy Markdown
Contributor Author

Mega Review

Status: NOT CLEAN @ ce076b7 · round 1 · full review

Two Warnings remain. Review-clean is separate from tests and CI.

Blocking

What needs correction Where
⚠️ Default pushed-video focus fades now snap to their final alpha; the atomic-present path also reads wall-clock time for opted-in fades. pixi-media-scene.ts:2817 · new
⚠️ Opted-in pushed-video fades start Pixi's automatic renderer outside the scene scheduler, so these paints bypass its render count and can add renders alongside scheduler-driven playback presents. pixi-media-scene.ts:2897 · new
Non-blocking observations
  • Normalize non-finite transition durations so the opt-in ticker remains bounded.
  • Remove the orphaned static-settlement comment and update ticker/playback documentation to the final behavior.
  • Describe immediate focus as applying to the initial resolved frame; later detection arrivals and style replacement need not be immediate.

Caveats

Source review only; no actual video-playback reproduction or cost measurement in this round. Four independent Codex passes, one outside Claude pass, a fresh scheduler skeptic and the orchestrator's pass returned. The outside pass covered changed regions but did not finish three complete test/public-guide files; the four Codex passes read all ten changed files. This review makes no ticket-compliance claim. Required CI is assessed separately.

Review Ledger

Open findings

  • packages/web/src/renderers/pixi-media-scene.ts:2817:push-focus-clock-regression | Warning | packages/web/src/renderers/pixi-media-scene.ts:2817 | shared scheduler now settles default push fades after media-time advancement and ticks opted-in fades with now() during atomic present; base only settled on-demand renders
  • packages/web/src/renderers/pixi-media-scene.ts:2897:push-ticker-scheduler-bypass | Warning | packages/web/src/renderers/pixi-media-scene.ts:2897 | new push-video ticker path enables Pixi LOW automatic app.render outside SceneRenderScheduler/getRenderCount; matching Pixi8.20.1 and fresh skeptic corroborate

Prior key statuses

  • None.

Unresolved prior open findings

  • None.

Deliberate tradeoffs

  • packages/web/src/renderers/pixi-focus-layer.ts:336:initial-resolved-frame | Minor | first resolved frame is the immediate-settlement boundary; an initially empty frame followed by later detections is a later transition, not a reason to add readiness state

Refuted

  • Ticker stop loses the final animation paint: Pixi's current listener walk continues to the LOW application render after stop cancels future scheduling; no dropped final paint established.

Implausible

  • None.

Pre-existing

  • Still-image GIF/spinner ticker paints already bypassed the render counter at MB; the open scheduler key concerns the newly added pushed-video path only.

@cfviotti

Copy link
Copy Markdown
Contributor Author

Mega Review

Status: NOT CLEAN @ 62ae2cf · round 2 · narrowed to ce076b7..62ae2cf

The atomic-present clock reads and automatic-render bypass are repaired. Two Warnings remain before this head is ready:

Reproduction and result Where
⚠️ During a default pushed-video fade, restore tab visibility or receive an annotation redraw between presents: the scheduler now settles alpha immediately. An executed visibility probe changed 0.704 → 1 at this head; the same probe with the base implementation retained 0.704. Default media-time fades should retain that alpha until a frame or an existing explicit focus interaction advances them. pixi-media-scene.ts:2818 · prior clock/default-fade key partially addressed
⚠️ Clean CI cannot typecheck the new public-renderer test: its supervision self-import depends on web build output that does not exist yet at this step. Use the existing source type boundary and verify the clean build order. media-renderer.focus-push.test.ts:3 · new · CI failure

Non-blocking observations:

  • A positive explicit duration skips held-overlay expiry during a missing-detection gap. An executed probe retained alpha 1 at 2.4 seconds with no detection frame. The base hold path also retargets an expired overlay on the next missing frame, so permanent clearing is not an established baseline promise. Keep any correction focused; do not expand this fade change into a hold-state redesign.
  • The new public guide overstates immediate paused focus and bounded rendering for the default pull-video route, whose continuous ticker still eases and paints. Scope those claims to still images and engine-backed push video. The scene overview also needs to describe the opt-in exception.

Coverage and limits

Two isolated Codex lanes, one outside Claude lane, a separate carried-key verifier and the parent pass completed. All three search lanes read the nine narrowed files in full and applied all thirteen lenses; relevant baseline guidance and all four pinned review doctrines were covered. Matching installed Pixi 8.20.1 confirms callback ownership, priority and ticker startup behavior. Canonical guidance links reach changed presentation policy, so CLI isolation was used.

Independent scratch probes confirmed the default-alpha regression and held-overlay observation. Actual generated-video browser checks passed ordinary default playback, explicit 150 ms playback and paused fades on WebGPU, with paint counts matching scheduler counts and idle counts stable. Those successful paths do not cover the missing-detection or visibility regressions and are not cost measurements. Full local verification passed 3,594 tests, but clean CI exposed the self-import failure. No private ticket-compliance claim is made. This is an incremental repair review, not a repeat of unrelated full-PR scope.

Review Ledger

Open findings

  • packages/web/src/renderers/pixi-media-scene.ts:2817:push-focus-clock-regression | Warning | packages/web/src/renderers/pixi-media-scene.ts:2818 | atomic presents preserve default alpha and avoid clocks, but off-present scheduler redraws still snap default fades; base/head visibility probes confirm the remaining regression | rounds=2
  • packages/web/src/renderers/media-renderer.focus-push.test.ts:3:clean-typecheck-self-import | Warning | packages/web/src/renderers/media-renderer.focus-push.test.ts:3 | new self-package type import cannot resolve before web build output exists, failing clean CI | rounds=1

Prior key statuses

  • push-focus-clock-regression — Partially addressed: atomic-present behavior is fixed; default off-present redraw behavior still regresses.
  • push-ticker-scheduler-bypass — Resolved: on-demand scenes detach Pixi automatic rendering and each animation tick paints through the scheduler, including its final paint.

Unresolved prior open findings

  • push-focus-clock-regression remains open as above.

Deliberate tradeoffs

  • packages/web/src/renderers/pixi-focus-layer.ts:336:initial-resolved-frame | Minor | initial resolved frame is the immediate-settlement boundary; later detection arrival is a later transition

Refuted

  • Stopping the ticker loses the final paint: the current implementation explicitly renders through the scheduler before stopping.

Implausible

  • None.

Pre-existing

  • The default held-overlay path can retarget an expired hold on a subsequent missing frame. The explicit-duration branch additionally skips expiry; recorded above as a non-blocking aggravation, not evidence of a previously permanent clear.
  • Still-image GIF/spinner paints bypassed the render counter at MB; this repair now routes them through it.

@cfviotti

Copy link
Copy Markdown
Contributor Author

Mega Review

Status: REVIEW-CLEAN @ fc6dab4 · round 3 · converged · narrowed to 62ae2cf..fc6dab4 (4 files, 70 changed lines)

Both carried Warnings are resolved. Default pushed-video alpha survives content/visibility redraws; ordinary interaction settlement remains at its existing owner. The new test imports its source type without requiring a prior web build. The earlier scheduler repair remains intact.

Verification and limits

One isolated Codex search, two separate carried-key verifiers, one outside Claude search and the orchestrator pass completed. Codex read all four changed files in full; Claude read the full scene, test and public guide, and the changed internal-policy section with its full baseline. Both families applied all thirteen lenses and all four pinned doctrines. Canonical guidance points at changed presentation policy, so isolated CLI review was required. The host verified the review checkout clean; one CLI status command was blocked by its LFS filter.

Full local verification passes 3,594 tests, including a clean web typecheck without generated output. Removal experiments fail their intended assertions. Current CI is green at this head. Actual current-build WebGL still-image and engine-backed WebGPU video checks cover initial dimming, configured fades, scheduler paint accounting, settled idle and teardown. They do not measure rendering cost or prove missing-detection recovery. Ticket compliance: NOT REQUESTED for this public SDK review.

The outside pass identified a rare, recoverable held-focus difference: the base's synthetic 10-second settlement also expired a paused missing-detection hold. Direct settlement no longer advances that hold clock. Accepted as a Minor tradeoff here: preserve actual media-time semantics and avoid broadening static fades into a missing-frame lifecycle redesign. The existing explicit-duration hold-expiry caveat remains disclosed. The prose's interaction list gives examples, not an exhaustive list of every renderOnChange caller.

Review Ledger

Open findings

  • None.

Prior key statuses

  • packages/web/src/renderers/pixi-media-scene.ts:2817:push-focus-clock-regression | resolved | default off-present redraws no longer settle alpha; production visibility regression and explicit-settlement removal checks protect both paths; atomic presents avoid clock reads
  • packages/web/src/renderers/media-renderer.focus-push.test.ts:3:clean-typecheck-self-import | resolved | #types/media-renderer resolves directly to source through committed TypeScript paths; clean typecheck and current CI pass
  • packages/web/src/renderers/pixi-media-scene.ts:2897:push-ticker-scheduler-bypass | resolved | on-demand scenes detach Pixi automatic rendering; ticks and final paint pass through the scheduler; actual application/scheduler paint counts agree

Unresolved prior open findings

  • None.

Deliberate tradeoffs

  • packages/web/src/renderers/pixi-focus-layer.ts:336:initial-resolved-frame | Minor | initial resolved frame is the immediate-settlement boundary; later detection arrival is a later transition, without new readiness state
  • packages/web/src/renderers/pixi-media-scene.ts:2880:paused-gap-hold-settlement | Minor | orchestrator accepts direct settlement without the base's synthetic 10-second hold-clock advance; a paused missing-detection gap may retain dimming until new detections, seek or advancing media time; recoverable, no saved data affected, no hold redesign in this PR
  • packages/web/src/renderers/pixi-media-scene.ts:2804:explicit-gap-hold-expiry | Minor | unchanged R2 caveat: positive explicit durations skip hold expiry; keep this separate from the repaired default-alpha contract and from a claim of permanent baseline clearing

Refuted

  • Final animation paint is dropped: each on-demand tick renders through the scheduler before ticker shutdown; current actual GPU accounting agrees.
  • Current visibility test merely edits mock alpha: it invokes the production scene's registered visibility handler, observes an extra render and compares intermediate alpha. Reintroducing unconditional settlement fails the assertion.
  • Interaction examples claim to list every settlement caller: neither prose span says the list is exhaustive; resize and viewport changes retain their existing renderOnChange behavior.

Implausible

  • None.

Pre-existing

  • Default held focus can be retargeted on the next missing frame after expiry at the merge base; permanently clearing every missing-detection gap was not established.
  • Still-image GIF/spinner paints bypassed the render counter at the merge base; the current on-demand scheduler correction also covers them.

@cfviotti cfviotti added the LGTMegaReview 🤖 Approved by Jarbas Mega Review label Oct 16, 2026
@cfviotti
cfviotti requested a review from a team October 16, 2026 23:24
@cfviotti
cfviotti marked this pull request as ready for review October 16, 2026 23:24
@cfviotti
cfviotti marked this pull request as draft October 17, 2026 00:28
@cfviotti cfviotti removed the LGTMegaReview 🤖 Approved by Jarbas Mega Review label Oct 17, 2026
@cfviotti

cfviotti commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor Author

Additional consumer validation confirms that configured focus appears automatically once the render artifacts are ready, without an unrelated display interaction. The earlier bare frame is the documented opening state while preparation is pending; it is separate from the persistent missing-focus problem fixed here.

No additional opening gate or production change is needed. This PR remains at the reviewed fc6dab4; the temporary draft status was used while checking that distinction.

@cfviotti
cfviotti marked this pull request as ready for review October 17, 2026 00:31
@cfviotti cfviotti added the LGTMegaReview 🤖 Approved by Jarbas Mega Review label Oct 17, 2026
@cfviotti cfviotti removed the LGTMegaReview 🤖 Approved by Jarbas Mega Review label Oct 17, 2026
@cfviotti

Copy link
Copy Markdown
Contributor Author

Mega Review

Status: NOT CLEAN @ 1c682c6 · round 4 · full review after main refresh · not converged

One Warning blocks convergence: BaseFocusStyle is no longer assignable to its public FocusStyle interface for consumers using TypeScript's exactOptionalPropertyTypes. A clean consumer importing the portable package fails both a direct assignment and presentation.focusStyle; the same fixture passes against the approved main package. The class permits a present undefined value while the optional interface property excludes it. The minimal repair is to include undefined in that interface property's value type. Source

Three Minor corrections are also warranted:

  • A supported live style update can change the resolved duration without changing the focus target. The fade retains its previous time origin and starting alpha: positive-duration changes can reverse progress, and default-to-explicit changes can mix media and wall clocks. Restarting from the current alpha when the resolved duration changes addresses both cases. Source
  • The BaseFocusStyleOptions hover text should match the interface and guide: explicit fades also work during playback, and nonpositive/non-finite values apply immediately. Source
  • The test harness should use installed Pixi's LOW priority, -25, instead of -50. Both priorities remain below NORMAL, so this does not invalidate the prior scheduler repair. Source

Verification and limits

The merge itself changes only four release metadata/docs files; dependency resolutions and feature source are unchanged. The current review protocol requires a full round because that integration touches a package manifest and lockfile. Four isolated Codex searches, one outside Claude search and the orchestrator pass completed over all eleven feature files and all thirteen lenses. Both families used the pinned review doctrines. Canonical guidance points at changed presentation policy, requiring isolated CLI review. Installed Pixi source was checked by the orchestrator; the outside lane could not inspect installed dependencies in its detached checkout.

Required CI verify passes at this head. The clean TypeScript consumer comparison was executed during adjudication. Other findings are grounded in source and public runtime-update contracts; no new GPU test or broad local matrix ran for the metadata refresh. Earlier GPU evidence remains bound to fc6dab4, whose feature source this merge preserves. Ticket compliance: NOT REQUESTED for this public SDK review.

Review Ledger

Open findings

  • packages/core/src/styles/focus-style.ts:51:exact-optional-focus-compatibility | Warning | clean portable-package consumer fails assigning BaseFocusStyle to FocusStyle and presentation.focusStyle with exactOptionalPropertyTypes; the approved main package passes the same fixture
  • packages/web/src/renderers/pixi-focus-layer.ts:335:duration-change-transition-origin | Minor | live duration changes retain the prior start time and starting alpha; changing positive durations can reverse progress and default-to-explicit updates can mix media and wall clocks
  • packages/core/src/styles/focus-style.ts:19:duration-option-documentation | Minor | option hover text omits playback and the negative/non-finite immediate behavior already described by the interface and guide
  • test/media-renderer-harness.ts:270:pixi-low-priority-fidelity | Minor | mock calls -50 LOW while installed Pixi 8.20.1 uses -25; both run below NORMAL, so prior scheduler assertions remain valid

Prior key statuses

  • packages/web/src/renderers/pixi-media-scene.ts:2817:push-focus-clock-regression | resolved | default off-present redraws no longer settle alpha; production visibility regression and explicit-settlement removal checks protect both paths; atomic presents avoid clock reads
  • packages/web/src/renderers/media-renderer.focus-push.test.ts:3:clean-typecheck-self-import | resolved | #types/media-renderer resolves directly to source through committed TypeScript paths; clean typecheck and current CI pass
  • packages/web/src/renderers/pixi-media-scene.ts:2897:push-ticker-scheduler-bypass | resolved | on-demand scenes detach Pixi automatic rendering; ticks and final paint pass through the scheduler; actual application/scheduler paint counts agree

Unresolved prior open findings

  • None.

Deliberate tradeoffs

  • packages/web/src/renderers/pixi-media-scene.ts:2797:bounded-playback-fade-paints | Minor | explicit fades may add ticker paints alongside media presents; the opt-in cost is bounded by the fade and all paints use the scheduler; no new measured performance claim

  • packages/web/src/renderers/pixi-focus-layer.ts:336:initial-resolved-frame | Minor | initial resolved frame is the immediate-settlement boundary; later detection arrival is a later transition, without new readiness state

  • packages/web/src/renderers/pixi-media-scene.ts:2880:paused-gap-hold-settlement | Minor | orchestrator accepts direct settlement without the base's synthetic 10-second hold-clock advance; a paused missing-detection gap may retain dimming until new detections, seek or advancing media time; recoverable, no saved data affected, no hold redesign in this PR

  • packages/web/src/renderers/pixi-media-scene.ts:2804:explicit-gap-hold-expiry | Minor | unchanged R2 caveat: positive explicit durations skip hold expiry; keep this separate from the repaired default-alpha contract and from a claim of permanent baseline clearing

Refuted

  • Final animation paint is dropped: each on-demand tick renders through the scheduler before ticker shutdown; current actual GPU accounting agrees.
  • Current visibility test merely edits mock alpha: it invokes the production scene's registered visibility handler, observes an extra render and compares intermediate alpha. Reintroducing unconditional settlement fails the assertion.
  • Interaction examples claim to list every settlement caller: neither prose span says the list is exhaustive; resize and viewport changes retain their existing renderOnChange behavior.

Implausible

  • None.

Pre-existing

  • Default held focus can be retargeted on the next missing frame after expiry at the merge base; permanently clearing every missing-detection gap was not established.
  • Still-image GIF/spinner paints bypassed the render counter at the merge base; the current on-demand scheduler correction also covers them.

@cfviotti

Copy link
Copy Markdown
Contributor Author

Mega Review

Status: NOT CLEAN for convergence @ 4adb727 · round 5 · narrowed to 1c682c6..4adb727 (6 files, 143 changed lines)

The TypeScript Warning is resolved, as are the option-text and Pixi-priority Minors. The review is clean by the Critical/Warning bar, but the duration-change Minor is only partially addressed and keeps this repair loop open.

The layer now preserves visible alpha when a resolved duration changes. A separate actual public-renderer check found a remaining path: change a partially faded engine-backed video from the default duration to an explicit 300 ms duration with the same target. During preparation reconfiguration the overlay reaches its target roughly 10 ms later, with no animation-ticker start. The result repeats with playback continuing, so it is not explained by pausing. The layer-only regression does not exercise those preparation notifications. Default settlement currently consults the layer's cached duration, which can remain undefined until a valid prepared frame resolves the replacement style. Scene update, settlement predicate

A second Minor needs one documentation clarification: changing the resolved duration restarts from the current alpha. A resolver that changes repeatedly can therefore extend the overall transition; the duration bound assumes a stable resolved value. No additional runtime policy is proposed.

Verification and limits

One fresh native Codex search, four separate carried-key verifiers, one outside Claude search and the orchestrator pass completed. Both search families applied all thirteen lenses and the pinned review doctrines; all six changed files were read in full. The outside lane's architecture/Pixi baseline-document reads were incomplete, while the native lane covered those contracts. The repair delta changes no applicable project instruction, permitting the native search route. The detached review checkout remained clean. This round covered the repair delta and carried keys; untouched feature code was reviewed earlier.

Focused verification passes 60 tests, and removing the layer origin reset makes both new continuity cases fail. Core/web builds, lint and formatting pass. A freshly built portable tarball passes the clean external TypeScript consumer with strict and exactOptionalPropertyTypes; the prior feature artifact fails the same two public assignments, while the approved main artifact passes. Required CI verify passes at this exact head.

Fresh visible WebGL still checks at this head pass positive-duration retiming, immediate focus, settled idle and teardown. The visible WebGPU public-video check exposes the remaining style-update failure above. These are correctness observations, not performance measurements. The outside review also noted that the unit test does not separately prove unchanged-duration redraws retain their origin; current source has the correct guard. This is a coverage limitation, not another demonstrated runtime failure.

No hold policy changed. Ticket compliance: NOT REQUESTED for this public SDK review.

Review Ledger

Open findings

  • packages/web/src/renderers/pixi-focus-layer.ts:335:duration-change-transition-origin | Minor | partially addressed: layer continuity is repaired, but actual public engine-backed style replacement can still snap before the new duration takes effect while preparation reconfigures; current default settlement consults a cached unresolved duration
  • docs/public/guides/presentation-styles.md:373:duration-restart-semantics | Minor | document that changing a resolved duration restarts from current alpha; continuously changing resolvers can extend the overall fade

Prior key statuses

  • packages/core/src/styles/focus-style.ts:51:exact-optional-focus-compatibility | resolved | optional interface explicitly allows undefined, matching the class without changing absence semantics; both public assignments pass against a freshly built portable tarball
  • packages/core/src/styles/focus-style.ts:19:duration-option-documentation | resolved | option and interface hover text agree with public normalization and playback semantics
  • test/media-renderer-harness.ts:270:pixi-low-priority-fidelity | resolved | mock uses installed Pixi 8.20.1 LOW=-25 and retains NORMAL-before-LOW ordering
  • packages/web/src/renderers/pixi-media-scene.ts:2817:push-focus-clock-regression | resolved | default off-present redraws no longer settle alpha; production visibility regression and explicit-settlement removal checks protect both paths; atomic presents avoid clock reads
  • packages/web/src/renderers/media-renderer.focus-push.test.ts:3:clean-typecheck-self-import | resolved | #types/media-renderer resolves directly to source through committed TypeScript paths; clean typecheck and current CI pass
  • packages/web/src/renderers/pixi-media-scene.ts:2897:push-ticker-scheduler-bypass | resolved | on-demand scenes detach Pixi automatic rendering; ticks and final paint pass through the scheduler; prior fc6dab4 actual application/scheduler paint counts agreed; scheduling source is unchanged by this repair

Unresolved prior open findings

  • duration-change-transition-origin remains partially addressed; see the public-renderer reproduction above.

Deliberate tradeoffs

  • packages/web/src/renderers/pixi-media-scene.ts:2797:bounded-playback-fade-paints | Minor | explicit fades may add ticker paints alongside media presents; the opt-in cost is bounded by the fade and all paints use the scheduler; no new measured performance claim

  • packages/web/src/renderers/pixi-focus-layer.ts:336:initial-resolved-frame | Minor | initial resolved frame is the immediate-settlement boundary; later detection arrival is a later transition, without new readiness state

  • packages/web/src/renderers/pixi-media-scene.ts:2880:paused-gap-hold-settlement | Minor | orchestrator accepts direct settlement without the base's synthetic 10-second hold-clock advance; a paused missing-detection gap may retain dimming until new detections, seek or advancing media time; recoverable, no saved data affected, no hold redesign in this PR

  • packages/web/src/renderers/pixi-media-scene.ts:2804:explicit-gap-hold-expiry | Minor | unchanged R2 caveat: positive explicit durations skip hold expiry; keep this separate from the repaired default-alpha contract and from a claim of permanent baseline clearing

Refuted

  • Final animation paint is dropped: each on-demand tick renders through the scheduler before ticker shutdown; prior fc6dab4 actual GPU accounting agreed; scheduling source is unchanged by this repair.
  • Current visibility test merely edits mock alpha: it invokes the production scene's registered visibility handler, observes an extra render and compares intermediate alpha. Reintroducing unconditional settlement fails the assertion.
  • Interaction examples claim to list every settlement caller: neither prose span says the list is exhaustive; resize and viewport changes retain their existing renderOnChange behavior.

Implausible

  • None.

Pre-existing

  • Default held focus can be retargeted on the next missing frame after expiry at the merge base; permanently clearing every missing-detection gap was not established.
  • Still-image GIF/spinner paints bypassed the render counter at the merge base; the current on-demand scheduler correction also covers them.

@cfviotti

Copy link
Copy Markdown
Contributor Author

Mega Review

Status: REVIEW-CLEAN @ 9e77637 · round 6 · converged · narrowed to 4adb727..9e77637 (3 files, 128 changed lines)

Both remaining Minors are resolved. Default settlement now reads the current style configuration, so a newly explicit duration is not mistaken for the default while preparation leaves the layer's duration unresolved. The existing layer transition state remains the interpolation owner. The guide now explains that changing a resolved duration restarts from the current alpha.

Verification and limits

One fresh native Codex search, two separate carried-key verifiers, one outside Claude search and the orchestrator pass completed. Both search families applied all thirteen lenses and the pinned review doctrines. All three changed files were read in full. The repair changes no applicable project instruction, permitting the native search route; the disposable checkout remained clean. The native search read the baseline contracts; the outside lane used partial spans/searches. Its source-only question about the test’s asynchronous preparation path is resolved by the executed predicate-removal failure below.

Focused runtime verification passes 61 tests. Restoring the old settlement predicate makes the public-factory 300 ms case fail with zero ticker starts; its omitted-duration case still passes. The repaired test requires preserved starting alpha, intermediate progress, final settlement, ticker shutdown and matching application/scheduler paint counts. Web build, lint and formatting pass. A freshly rebuilt portable package also passes the clean external TypeScript consumer with strict and exactOptionalPropertyTypes. Required CI verify passes at this exact head. Fresh visible WebGL still checks pass first-ready dimming, positive-duration retiming, explicit zero, settled idle and destruction. Fresh engine-backed WebGPU checks, paused and playing, preserve partial alpha on default-to-explicit replacement, then fade, stop the ticker and remain idle. These are correctness checks, not performance measurements.

This narrowed round covers the repair and carried keys; unchanged feature source was reviewed earlier. REVIEW-CLEAN reflects the accumulated rounds. No hold policy changed; accepted Minor tradeoffs remain below. Ticket compliance: NOT REQUESTED for this public SDK review.

Review Ledger

Open findings

  • None.

Prior key statuses

  • packages/web/src/renderers/pixi-focus-layer.ts:335:duration-change-transition-origin | resolved | layer resets its existing origin/from-alpha on normalized-duration changes; scene settlement now classifies the current style before preparation can leave the layer duration unresolved; public-factory regression fails with zero ticker starts when the old predicate is restored
  • docs/public/guides/presentation-styles.md:373:duration-restart-semantics | resolved | guide states current-alpha restart when resolved duration changes and conditions the duration bound on a stable resolved value
  • packages/core/src/styles/focus-style.ts:51:exact-optional-focus-compatibility | resolved | optional interface explicitly allows undefined, matching the class without changing absence semantics; both public assignments pass against a freshly built portable tarball
  • packages/core/src/styles/focus-style.ts:19:duration-option-documentation | resolved | option and interface hover text agree with public normalization and playback semantics
  • test/media-renderer-harness.ts:270:pixi-low-priority-fidelity | resolved | mock uses installed Pixi 8.20.1 LOW=-25 and retains NORMAL-before-LOW ordering
  • packages/web/src/renderers/pixi-media-scene.ts:2817:push-focus-clock-regression | resolved | default off-present redraws no longer settle alpha; production visibility regression and explicit-settlement removal checks protect both paths; atomic presents avoid clock reads
  • packages/web/src/renderers/media-renderer.focus-push.test.ts:3:clean-typecheck-self-import | resolved | #types/media-renderer resolves directly to source through committed TypeScript paths; clean typecheck and current CI pass
  • packages/web/src/renderers/pixi-media-scene.ts:2897:push-ticker-scheduler-bypass | resolved | on-demand scenes detach Pixi automatic rendering; ticks and final paint pass through the scheduler; prior fc6dab4 actual application/scheduler paint counts agreed; scheduling source is unchanged by this repair

Unresolved prior open findings

  • None.

Deliberate tradeoffs

  • packages/web/src/renderers/pixi-media-scene.ts:2880:duration-classifier-rationale | Minor | the outside reviewer suggested a source comment explaining current-style classification versus resolved-duration advancement; accepted as a documentation nit, with the distinction explained here and protected by the executed public-factory removal check; no additional source comment added

  • packages/web/src/renderers/pixi-media-scene.ts:2880:unresolved-duration-paused-hold | Minor | a new explicit style cannot resolve its duration during a missing-frame gap or Creating/DragSelecting gesture; partial alpha is retained without starting a ticker until a valid frame resolves; accepted without new hold state or policy, recoverable and no saved data affected

  • packages/web/src/renderers/pixi-media-scene.ts:2797:bounded-playback-fade-paints | Minor | explicit fades may add ticker paints alongside media presents; the opt-in cost is bounded by the fade and all paints use the scheduler; no new measured performance claim

  • packages/web/src/renderers/pixi-focus-layer.ts:336:initial-resolved-frame | Minor | initial resolved frame is the immediate-settlement boundary; later detection arrival is a later transition, without new readiness state

  • packages/web/src/renderers/pixi-media-scene.ts:2880:paused-gap-hold-settlement | Minor | orchestrator accepts direct settlement without the base's synthetic 10-second hold-clock advance; a paused missing-detection gap may retain dimming until new detections, seek or advancing media time; recoverable, no saved data affected, no hold redesign in this PR

  • packages/web/src/renderers/pixi-media-scene.ts:2804:explicit-gap-hold-expiry | Minor | unchanged R2 caveat: positive explicit durations skip hold expiry; keep this separate from the repaired default-alpha contract and from a claim of permanent baseline clearing

Refuted

  • Final animation paint is dropped: each on-demand tick renders through the scheduler before ticker shutdown; prior fc6dab4 actual GPU accounting agreed; scheduling source is unchanged by this repair.
  • Current visibility test merely edits mock alpha: it invokes the production scene's registered visibility handler, observes an extra render and compares intermediate alpha. Reintroducing unconditional settlement fails the assertion.
  • Interaction examples claim to list every settlement caller: neither prose span says the list is exhaustive; resize and viewport changes retain their existing renderOnChange behavior.

Implausible

  • None.

Pre-existing

  • Default held focus can be retargeted on the next missing frame after expiry at the merge base; permanently clearing every missing-detection gap was not established.
  • Still-image GIF/spinner paints bypassed the render counter at the merge base; the current on-demand scheduler correction also covers them.

@cfviotti cfviotti added LGTMegaReview 🤖 Approved by Jarbas Mega Review and removed LGTMegaReview 🤖 Approved by Jarbas Mega Review labels Oct 17, 2026
@cfviotti

Copy link
Copy Markdown
Contributor Author

Mega Review

Status: REVIEW-CLEAN @ 0261b37 · round 7 · full fallback · converged

No new blocking findings. Review-clean does not replace tests or CI.

Fixed since last round

  • Merged current main (a79430f) while preserving the focus changes and the merged mask-order behavior. The shared Pixi test mock retains focus alpha/cut and geometry destroy support.

Non-blocking

Three documentation/naming nits and six carried Minor tradeoffs
  • The duration example is a property fragment that needs the surrounding presentation call and matching punctuation.
  • The media-boundary summary could distinguish the single media-frame clock from opted-in alpha animation more explicitly.
  • The still-image regression sits under a describe block named for push presentation.

These low-risk documentation and naming nits are accepted for this conflict reconciliation. The existing initial-frame boundary, missing-frame hold behavior, duration-classifier comment nit, and bounded extra paints also remain accepted. Conditions and evidence are retained in the ledger below.

Caveats

Full review of the current base-to-head PR delta: 12 files, 864 changed lines. The base moved since round 6 and the shared test harness needed a manual resolution, so the mechanical base-refresh exception does not apply. Applicable changed guidance was reviewed as source through isolated reviewer processes.

The focused focus-transition, still-image, ordered-mask, prepared-window and atomic-present suites pass 106 tests, with scoped lint and formatting passing. Required CI verify passes at this exact head. Earlier GPU and portable-package observations remain tied to their documented commits; no fresh GPU or performance result is claimed for this merge. Ticket compliance: NOT REQUESTED for this public supervision-js review.

Review Ledger

Open findings

  • None.

Prior key statuses

  • packages/web/src/renderers/pixi-focus-layer.ts:335:duration-change-transition-origin | resolved | layer resets its existing origin/from-alpha on normalized-duration changes; scene settlement now classifies the current style before preparation can leave the layer duration unresolved; current focused public-factory regression passes; the old-predicate removal failure remains historical at 9e77637
  • docs/public/guides/presentation-styles.md:373:duration-restart-semantics | resolved | guide states current-alpha restart when resolved duration changes and conditions the duration bound on a stable resolved value
  • packages/core/src/styles/focus-style.ts:51:exact-optional-focus-compatibility | resolved | optional interface explicitly allows undefined, matching the class without changing absence semantics; class/interface source is byte-identical to 9e77637, where both public portable-tarball assignments passed; no new portable artifact proof claimed
  • packages/core/src/styles/focus-style.ts:19:duration-option-documentation | resolved | option and interface hover text agree with public normalization and playback semantics
  • test/media-renderer-harness.ts:270:pixi-low-priority-fidelity | resolved | mock uses installed Pixi 8.20.1 LOW=-25 and retains NORMAL-before-LOW ordering
  • packages/web/src/renderers/pixi-media-scene.ts:2817:push-focus-clock-regression | resolved | default off-present redraws no longer settle alpha; current production visibility and duration regressions pass; atomic-present test observes no clock reads
  • packages/web/src/renderers/media-renderer.focus-push.test.ts:3:clean-typecheck-self-import | resolved | #types/media-renderer resolves directly to source through committed TypeScript paths; source alias and test are unchanged from 9e77637; current exact-head CI covers typecheck
  • packages/web/src/renderers/pixi-media-scene.ts:2897:push-ticker-scheduler-bypass | resolved | on-demand scenes detach Pixi automatic rendering; ticks and final paint pass through the scheduler; current focused public-factory tests verify application/scheduler paint counts, final paint and idle ticker

Unresolved prior open findings

  • None.

Deliberate tradeoffs

  • docs/public/guides/presentation-styles.md:366:guide-snippet-fragment | Minor | focusStyle example is a property fragment with statement punctuation; a reader embedding it must supply a presentation object and correct punctuation; accepted documentation nit, no runtime defect or change in this conflict reconciliation

  • docs/internal/library-contract.md:166:push-second-clock-wording | Minor | matching-frame summary could distinguish the canonical media clock from opted-in wall-time alpha; detailed presentation contract already documents the distinction; accepted summary-wording nit, no second media-frame selection clock introduced

  • packages/web/src/renderers/pixi-media-scene.prepared-window.test.ts:1045:still-test-in-push-describe | Minor | still-image regression is grouped under push presentation because it exercises shared on-demand scheduling; accepted naming/placement nit, assertion remains meaningful

  • packages/web/src/renderers/pixi-media-scene.ts:2880:duration-classifier-rationale | Minor | the outside reviewer suggested a source comment explaining current-style classification versus resolved-duration advancement; accepted as a documentation nit; current focused public-factory regression passes and the removal failure remains historical at 9e77637; no additional source comment added

  • packages/web/src/renderers/pixi-media-scene.ts:2880:unresolved-duration-paused-hold | Minor | a new explicit style cannot resolve its duration during a missing-frame gap or Creating/DragSelecting gesture; partial alpha is retained without starting a ticker until a valid frame resolves; accepted without new hold state or policy, recoverable and no saved data affected

  • packages/web/src/renderers/pixi-media-scene.ts:2797:bounded-playback-fade-paints | Minor | explicit fades may add ticker paints alongside media presents; the opt-in cost is bounded by the fade and all paints use the scheduler; no new measured performance claim

  • packages/web/src/renderers/pixi-focus-layer.ts:336:initial-resolved-frame | Minor | initial resolved frame is the immediate-settlement boundary; later detection arrival is a later transition, without new readiness state

  • packages/web/src/renderers/pixi-media-scene.ts:2880:paused-gap-hold-settlement | Minor | orchestrator accepts direct settlement without the base's synthetic 10-second hold-clock advance; a paused missing-detection gap may retain dimming until new detections, seek or advancing media time; recoverable, no saved data affected, no hold redesign in this PR

  • packages/web/src/renderers/pixi-media-scene.ts:2804:explicit-gap-hold-expiry | Minor | unchanged R2 caveat: positive explicit durations skip hold expiry; keep this separate from the repaired default-alpha contract and from a claim of permanent baseline clearing

Refuted

  • Final animation paint is dropped: each on-demand tick renders through the scheduler before ticker shutdown; current focused public-factory accounting agrees, while actual GPU accounting remains historical at fc6dab4.
  • Current visibility test merely edits mock alpha: it invokes the production scene's registered visibility handler, observes an extra render and compares intermediate alpha. The historical fc6dab4 removal check failed; the registered-handler regression passes at this head.
  • Interaction examples claim to list every settlement caller: neither prose span says the list is exhaustive; resize and viewport changes retain their existing renderOnChange behavior.

Implausible

  • None.

Pre-existing

  • Default held focus can be retargeted on the next missing frame after expiry at the merge base; permanently clearing every missing-detection gap was not established.
  • Still-image GIF/spinner paints bypassed the render counter at the merge base; the current on-demand scheduler correction also covers them.

@cfviotti cfviotti added the LGTMegaReview 🤖 Approved by Jarbas Mega Review label Oct 19, 2026
@cfviotti
cfviotti merged commit 623f279 into main Oct 20, 2026
3 checks passed
@cfviotti
cfviotti deleted the fix/static-focus-transitions branch October 20, 2026 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

LGTMegaReview 🤖 Approved by Jarbas Mega Review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants