Repository navigation
Conversation
…ght default and the WebKit shader cache - castStoryboard: a filmstrip of real screencast frames (e.g. a busy window), labelled from a zero. - sampleRects: one element rect per animation frame, the cheap path when the main thread is free. - openStory turns Storybook's highlight module off (it reads every element's computed style on each insertion, inside the inserting task) and checks it is off; storybookHighlight: true keeps it. - The WebKit container keeps Mesa's shader cache on the host, so only the first run on a machine pays 450-700 ms per new shader. Self-tests for each, README. Split out of the frame evidence pull request for scope; the code is unchanged. Co-authored-by: Christopher Chedeau <vjeuxx@gmail.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
PR Analysis Report
|
There was a problem hiding this comment.
Changes requested, for scope only: as it stands this pull request carries 4,147 changed lines because its diff includes #8348, #8045 and #8548, while its own change is the 478-line last commit. Once those land, or the base is retargeted onto test/harness-traces, the diff will be just the four instruments, and nothing in them blocks: their self-tests pass in Chromium, and the highlight-module switch matches how Storybook 10.6.2 initializes that module. One small fix while you are there: castStoryboard accepts clip through StoryboardOptions but never applies it, so either drop it from its options type or crop the frames.
[Automated review]
| readonly data: string; | ||
| readonly format: 'png' | 'jpeg'; | ||
| }[], | ||
| options: StoryboardOptions & { |
There was a problem hiding this comment.
castStoryboard takes StoryboardOptions, so clip type-checks here, but the frames are pushed whole and resolveClip never runs (only capture calls it): a caller's clip is silently ignored and the trace reports "clip": null. Either type this as Omit<StoryboardOptions, 'clip'> & {…} or crop each frame to the clip before the sheet is composed.
…8549 and meta#8554 into ChatLayout ChatLayout's keyboard ride now stacks on its dependencies, merged as one base (58f88dc): the keyboard module (meta#8559, on main), the sheet's ride on Gboard's slides (on meta#8559, meta#8076 and meta#8484), the split frame evidence (meta#8548 under meta#8549 and meta#8554), and the rendering trace's invalidations with the compositor-update classifier (meta#8554). Every file those carry takes their version; ChatLayout's parts are put back on top: - `useMobileKeyboard.ts`: the sheet ride's version (its coarse-pointer gate, main's Reduce Motion query), plus `claimTapFocus` and the ChatLayout lines of its header; its tests and `index.ts` likewise. - The keyboard README and the guide's, model test's, rideKeyboard's and layoutViewport's headers: theirs, plus ChatLayout's lines. - `keyboardLayoutGuide.ts` takes the frame-watch fix, and `timing.ts` meta#8484's constants with meta#8076's change. - The harness is meta#8554's, beside meta#8549's and meta#8446's content. - The browser-support guide names ChatLayout's composer beside the sheet; the modules' changesets stay theirs, and ChatLayout's own changeset names the guide (and the CLI).
There was a problem hiding this comment.
Changes requested, for scope only: this pull request's diff carries 3,105 changed lines because it still includes #8045 and #8548, while its own change is the 478-line commit. Once those land, or the base is retargeted onto test/harness-traces, the diff will be just the four instruments, and nothing in them blocks: their self-tests pass in Chromium, and the highlight-module switch matches how Storybook 10.6.2 initializes that module. One small fix while you are there: castStoryboard accepts clip through StoryboardOptions but never applies it, so either drop it from its options type or crop the frames.
[Automated review]
# Conflicts: # internal/interaction-harness/README.md
castStoryboard accepted StoryboardOptions' clip through its options type but never applied it to the frames. Its options now leave clip out, so a caller can't pass one that does nothing.
…EADME) into the recorder stand-down # Conflicts: # internal/interaction-harness/src/harness.interaction.browser.spec.ts
…eta#8566) and meta#8554 A dependency-only merge. The base (ad271a7) is the sheet's Gboard ride (meta#8566, on meta#8559 39d37ef, meta#8484 and meta#8076 62ecda1) merged with meta#8554 21ea4dc, which carries cixzhang's meta#8446 66c651f, meta#8549, meta#8548 and meta#8045 at their current heads, so no harness file is resolved here. ChatLayout's own change (this branch's commits since 58f88dc) applies on it with meta#8559's names (`subscribeKeyboardHeight`, `getKeyboardHeight`, `keyboardHeight.ts`).
meta#8176 now stacks on the keyboard module (meta#8559), the sheet's Gboard ride (meta#8566), meta#8549 and meta#8554, and its ride attaches only under a coarse pointer. One conflict, in `useChatKeyboard.ts`: the touch presentation is read once, before both effects (this branch's touch listeners and meta#8176's gated guide subscription), and the transcript's effect keeps its touch listeners beside the gated ride.
meta#8176 now sits on the foundation heads (the sheet's ride on meta#8559 39d37ef and meta#8076 62ecda1, and meta#8554 21ea4dc with cixzhang's meta#8446, meta#8549, meta#8548 and meta#8045). A base move: the dismissal's own change applies on it with meta#8559's names (getKeyboardHeight, subscribeKeyboardHeight).
There was a problem hiding this comment.
Changes requested, for scope only: this pull request's diff carries 3,154 changed lines because it still includes #8045 and #8548, while its own change is 479 lines. Once those land, or the base is retargeted onto test/harness-traces, the diff will be just the four instruments, and nothing in them blocks: their self-tests pass in Chromium, the highlight-module switch matches how Storybook 10.6.2 initializes that module, and castStoryboard no longer accepts a clip it would ignore. One small fix while you are there: resolve ASTRYX_WEBKIT_SHADER_CACHE with path.resolve in webkit-container.mjs and fall back to the default on an empty value, since a bare relative value is mounted as a named volume and an empty one makes mkdirSync throw.
[Automated review]
| // Where the container sees the host's shader cache (see the header). | ||
| const containerShaderCache = '/astryx-shader-cache'; | ||
| const shaderCache = | ||
| process.env.ASTRYX_WEBKIT_SHADER_CACHE ?? |
There was a problem hiding this comment.
This value reaches -v as given (line 105), while mkdirSync and the marker write resolve it against the current directory. A bare relative value such as shader-cache is mounted by docker and podman as a named volume, so the container never sees the marker and the shader-cache self-test fails; an empty value passes the ?? and fs.mkdirSync('') throws ENOENT. path.resolve(process.env.ASTRYX_WEBKIT_SHADER_CACHE || <the default path>) covers both.
A bare relative ASTRYX_WEBKIT_SHADER_CACHE was mounted as a named volume instead of a host directory, and an empty one made mkdirSync throw. The script now resolves a set value with path.resolve and falls back to the default cache directory on an empty one.
… path) into the recorder stand-down # Conflicts: # internal/interaction-harness/src/harness.interaction.browser.spec.ts
…ss renders; the base moves to the foundation heads The base (3f66f76) is the sheet's Gboard ride (meta#8566, on meta#8559 6be79f4, meta#8484 and meta#8076 62ecda1) merged with meta#8554 e4f3c8d, on cixzhang's meta#8446 87e516b, meta#8549, meta#8548 and meta#8045, with no conflict. meta#8559 moved `useKeyboardHeight` here, to its first consumer: ChatLayout lays its dock and transcript padding out from it. Its subscribe and snapshot functions are now stable for the component's life (per window), so React keeps one subscription across renders; before, every render unsubscribed and resubscribed, and a lone reader detached the module's listeners and measured again on each keyboard change (INV11). Red first: "reads the layout once per keyboard change for a lone component" expected no listener re-added and one read, and got two `addEventListener` calls. The hook's tests (one re-render per change, a keyboard already up, StrictMode's replay leaving no listener) and the README's layout example come with it.
There was a problem hiding this comment.
Looks good to merge: with the base now test/harness-traces, this pull request is just the four instruments, the clip and ASTRYX_WEBKIT_SHADER_CACHE fixes are in, and the self-tests that run outside the WebKit container pass in Chromium and WebKit and fail when the instrument they cover is broken. One optional cleanup before merging: the storyboard.ts file header still says every filmstrip in that file is deterministic and "never from wall-clock capture", which castStoryboard now contradicts, and the harness spec's header does not yet name the rect-sampling and shader-cache proofs.
[Automated review]
| /** | ||
| * A filmstrip of screencast frames (real presented frames, e.g. the frames of | ||
| * a busy-main-thread window), labelled with their time relative to `zero`. | ||
| * Evidence for what the compositor presented, not a deterministic storyboard. |
There was a problem hiding this comment.
The file header (lines 5-16) still lists only Storyboard, animationStoryboard and touchStoryboard under @output and says frames come "never from wall-clock capture, so the same code produces the same sheet", which this function now contradicts. Worth adding castStoryboard to @input/@output and confining that sentence to the seek and touch storyboards, per the Update Protocol in AGENTS.md.
Stacked on #8548 (rendering traces), #8045 and #8348. Until those land, this diff includes them; this pull request's own change is its single commit.
What it adds
Four smaller harness instruments, split out of #8045 for scope (the code is unchanged):
castStoryboard: a filmstrip of real screencast frames (for example a busy-main-thread window), labelled from a zero.sampleRects: one element rect per animation frame, the cheap path when the main thread is free.openStoryturns Storybook's highlight module off and checks it is off. Left on, the module reads the computed style of every element on the page on each insertion, inside the inserting task, so a proof would measure Storybook as well as the component.{storybookHighlight: true}keeps it.Who it helps
Authors of interaction proofs: real-time filmstrips as evidence, a cheap rect sampler, story timings free of Storybook's own work, and WebKit runs that don't lose short animations to shader compilation.
Testing
sampleRects; in the WebKit container compiled shaders outlive the run;openStoryleaves no highlight scan on an insertion (storybook.interaction.browser.spec.ts).internal/), nothing published: no changeset.Co-authored with vjeux.