Repository navigation
Gallery: keep the previous preview through frame swaps - #18345
Merged
Merged
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
Passes: CI passes on CI passes on Written by |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…llery-white-flash
This comment has been minimized.
This comment has been minimized.
| ? current | ||
| : requested, | ||
| ); | ||
| }, [display.query, display.run, frameHeight, frameWidth, query, run, scale]); |
There was a problem hiding this comment.
Stale play report after swap
Low Severity
Variant view now reuses Stage across swaps, but report is never cleared when query or run change. After a new frame is promoted, the previous variant's play metrics can stay visible, especially when the incoming variant has no play and never posts a replacement.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 402a9fe. Configure here.
This comment has been minimized.
This comment has been minimized.
| stages.map((stage) => ( | ||
| <Stage | ||
| key={stage.key} | ||
| key={search.view === "variant" ? "stage" : stage.key} |
There was a problem hiding this comment.
Play report persists across variants
Low Severity
The variant-view Stage now keeps a stable key, but report is never cleared when query changes. The previous variant’s play metrics stay in the caption, and they remain indefinitely when the next variant has no play and never sends a replacement report.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 1e46b26. Configure here.
…llery-white-flash
* ci: format staged files in the trusted pre-commit hook * ci: guard formatter scope for staged files * ci: scope Rust formatter guard to its package * ci: use direct rustfmt and skip merge formatting * ci: pass the cmux-tui Rust edition to rustfmt
* test(ci): red test, PRs check rustfmt on the cmux-tui packages they touch PR CI never runs cmux-tui.yml (dispatch only), so #18706 landed unformatted Rust and broke base cargo fmt --check. These tests pin a helper that runs cargo fmt --check once per touched package, checks every package when a rustfmt config changes or there is no base, and names each unformatted package. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci: check rustfmt on the cmux-tui packages a PR touches PR CI never runs cmux-tui.yml (dispatch only), so #18706 landed unformatted Rust and broke base cargo fmt --check. cmux-tui-spec.yml already runs on every PR and push touching cmux-tui/** on hosted Linux; its new rustfmt job runs cmux-tui-fmt-changed.py, which maps each changed .rs file or Cargo.toml to its nearest tracked package and runs cargo fmt --check on it with the pinned toolchain. No build and no dependency download. A rustfmt config or toolchain change, or no base, checks every package; vendored crates and deleted packages are skipped. A PR compares with its merge commit's first parent, a push with the commit before it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * style(cmux-tui): rustfmt acpmux's draft tests and the relay circuit The new rustfmt job's full run found two base packages that fail cargo fmt --check: acpmux, from #18706's draft tests and their module order, and the Cloudflare relay's circuit.rs. Formatting only, exactly the hosted rustfmt diff. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * style(cmux-tui): leave the relay circuit to its own repair Touching the Cloudflare relay runs its lane, whose npm audit is red on base (sharp and undici advisories through wrangler, fixed only by a wrangler bump outside the stated range). That is the relay's own repair, so this PR keeps acpmux's formatting only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* ci: add mini-first Swift canary for cmux-next * ci: remove unit tests from Swift canary change
…e cmux-tui (#18698) * test(cmux-next): red: a PR that changes no cmux-tui fails same-tree on an unpublished merge tree #16614, #17551 and #18605 change no cmux-tui or ghostty-next path, yet same-tree cmux-tui failed them because the base tip's tree was never published (its artifacts run was cancelled). Such a PR should reuse the newest published tree in its history (tree_fetch_key, fetched through CMUX_TUI_TREE_KEY), or skip the tree jobs when none is published, never fail. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci(cmux-next): same-tree cmux-tui is a hard check only for PRs that change cmux-tui Path routing records whether the pull request's own diff touches cmux-tui/ or ghostty-next. For one that does not, a merge tree that is not ready no longer fails: the probe (CMUX_TUI_TREE_BASE_FALLBACK=1) resolves the newest published tree in the merge's history and reports ready with tree_fetch_key, which the daemon tests and the Debug scheme compile fetch through CMUX_TUI_TREE_KEY; with none published it reports skipped. Pushes, dispatches and PRs that change cmux-tui keep the hard check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(cmux-next): the newest-published search deepens only this commit's history A bare `git fetch --deepen=50 origin` deepens every branch of origin: on this PR's first run it took path routing past its 5-minute limit (job 113609407061) before the probe's base fallback, which then resolved in under a second. Deepening only HEAD takes about 30 s on a depth-2 clone of feat-cmux-next. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* docs(cmux-next): map UI parity reference sources * docs(cmux-next): add media preview reference source * docs(cmux-next): define reference freshness checks
* test(nightly): the notes step cannot spend the publish budget before the upload (red) Run 37611677848 spent 29 of publish-nightly's 30 minutes on release notes and was cancelled mid-upload, leaving partial release assets. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nightly): bound the publish notes step at 10 minutes The notes take seconds since 0115b26. A regression now fails the step before any asset upload, instead of spending publish-nightly's 30 minutes and being cancelled mid-upload with partial assets left on the release. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…self (#17622) * test(cmux-next): the cmux-tui gate pins and dispatches an unpublished tree itself Publisher run 37505519359 failed transiently and its retry was replaced by a newer feat-cmux-next push, so the tree was never published and lanes pushed cmux-tui-pin-* branches by hand to unblock their gates. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(cmux-next): the tree gate watches the base and may pin the tree Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci(cmux-next): the cmux-tui gate pins and dispatches an unpublished tree When two run checks find no artifacts run that will publish the tree, the gate creates cmux-tui-pin-<key12> at the publisher commit and dispatches the artifacts workflow there, once, then keeps waiting. A pull request now watches its base commit (the merge's first parent) instead of no publisher, so it neither waits 45 minutes for a tree nobody builds nor needs a lane to push a pin branch by hand. A superseded push still skips; forks keep a read-only token and no autopin. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(cmux-tui): a transient wrong binary read must not fail publication Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(cmux-tui): read a mismatched published binary again before failing R2 objects are write-once, so a digest mismatch on a binary the run just uploaded is a bad read. Fetch it again from a cache-busting URL up to three times, and name the response headers when it still differs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(cmux-next): a pull request gate pins a tree its base never builds Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci(cmux-next): a pull request gate pins a tree its base never builds A pull request that changes cmux-tui has a merge tree no push builds, and the artifacts workflow's pull_request_target trigger has never started a run, so its gate waited 45 minutes with the run check off. With CMUX_TUI_TREE_AUTOPIN the gate now pins its own checkout at once and watches that run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci(cmux-next): retain base tree dispatch tests --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* ci(gallery): capture narrow responsive states * test(gallery): cover workflow matrix changes * fix(gallery): dedupe equal responsive widths * ci: shard gallery matrix renders * ci: retrigger sharded gallery validation
Stage.tsx: kept useRef for the frame swap alongside base's useEffect (useRoom now an effect). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 347d3de. Configure here.
| contents: read | ||
| timeout-minutes: 20 | ||
| env: | ||
| CMUX_NEXT_RUNNER: ${{ needs.macos-placement.outputs.runner || github.repository_owner != 'manaflow-ai' && 'macos-26' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository && 'blacksmith-6vcpu-macos-26' || vars.CI_PR_POOL_OWNED == '1' && contains(fromJSON('["pull_request","push","workflow_dispatch"]'), github.event_name) && (github.run_attempt <= 2 || github.triggering_actor != 'github-actions[bot]') && vars.CI_SIDE_LANE_RUNNER || vars.MACOS_RUNNER_26 || 'blacksmith-6vcpu-macos-26') }} |
There was a problem hiding this comment.
Canary skips overflow runner fallback
Medium Severity
swift-canary still uses the old runs-on / CMUX_NEXT_RUNNER expression, so a bot re-run keeps macos-placement's mini label on attempts 4 and later. The other Mac jobs now drop that label after attempt 3. A mini refusal can leave the canary queued instead of moving to Blacksmith.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 347d3de. Configure here.
Contributor
GalleryNo state changed · 3766 removed states · 14 broken states · 16 play checks failed Diff page and renders (the run's Play checks failed (0 px anchor movement, 0 layout shift, no frame over 33 ms unless the entry gives a reason):
|
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
1 participant
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.
Summary
Keep the last painted gallery preview visible while a new variant, theme, or control state loads. The incoming frame stays hidden until the stage reports ready, and the neutral frame background follows the shell theme.
Changelog
Proof
Validation
git diff --checkNeed help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Keeps the previous gallery preview visible until the new frame reports ready, so the preview no longer flashes white when switching variant, theme, or control state.
Written for commit 347d3de. Summary will update on new commits.
Note
Medium Risk
Changes PR merge gates (Swift canary, cmux-tui tree fallback, rustfmt) and Mac runner placement; gallery and pre-commit edits are lower risk.
Overview
Gallery previews no longer flash white when variant, theme, or controls change:
Stageloads the nextframe.htmlin a hidden iframe and swaps only aftercmux-gallery-stagereports ready/error (matched by query and run), with a stable stage key in variant view and a themed frame background.Gallery PR CI shards each render stage four ways, adds compact and normal pane widths to the matrix (deduping identical narrow/normal pixels), and merges shard
results.jsonbefore compare.cmux-tui formatting adds
cmux-tui-fmt-changed.pyplus unit tests and arustfmtjob incmux-tui-spec.ymlthat runscargo fmt --checkonly on packages touched by the diff (or all packages when rustfmt/toolchain changes). The pre-commit hook now formats staged webview files withvp fmtand stagedcmux-tuiRust withrustfmt, skips formatters during merges, and documents the behavior inCONTRIBUTING.md.cmux-next CI introduces a Swift canary job (debug
swift build --targetfor small package-only Swift diffs) as the PR gate while Release compile stays advisory on that class; routing addsswift_canaryoutputs. PRs that do not touchcmux-tui/orghostty-nextcan useCMUX_TUI_TREE_BASE_FALLBACKso daemon/scheme jobs fetch the newest published tree instead of failing on an unpublished merge tree. Mac jobs keep side-lane placement through bot attempt 3; attempt 4 drops to Blacksmith. Published manifest verification retries remote binary digest checks with cache-busted URLs. Nightly publish bounds the release-notes step with its own timeout.Reviewed by Cursor Bugbot for commit 347d3de. Bugbot is set up for automated code reviews on this repo. Configure here.