Repository navigation
ci: gallery format edits own only entry problems (no bead) - #19761
Conversation
The gallery coverage check listed validateEntries and its helpers (format.ts, play.ts, experiments) as the whole check's own code, so a pull request editing format.ts owned every base coverage gap. They now decide only "the entries are valid"; the coverage tests keep the test file, prScope.ts and entries.ts as their rules. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
|
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 |
Review: format.ts also held componentEntry and the other entry constructors, which build the covers and ids the coverage tests read. They move to src/gallery/entry.ts (format.ts re-exports them, so no importer changes) and that file stays a rule of every gallery test. format.ts edits still own entry problems. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
This comment has been minimized.
This comment has been minimized.
|
Passes: CI passes on CI passes on Written by |
|
|
Gallery122 states changed: pages.settings/browser-group-2, pages.settings/keyboard-group-3, pages.settings/browser-group-6, pages.settings/browser-group-7, pages.settings/general-group-9, ... · 2 new states · 38 broken states · 82 play checks failed Diff page and renders (the run's
116 more on the diff page. Live preview (follows this branch; start it with
Play checks failed (0 px anchor movement, 0 layout shift, no frame over 33 ms unless the entry gives a reason):
553 nondeterministic states differed from a second render of the same head (a clock or fixture leak) and are not counted as changes. |
Review: the open-ended hold let programmatic scrolls (jump to latest, previous/next turn) snap back to the match, and padding a border-box row fires no measure, so the late-measure step proved nothing. Back to the base reveal; the fold case now runs under 40x CPU throttling, the loaded runner of #19761's red, where the 600 ms window ends before the fold and the rows above the match measure. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…19779) * test: Find keeps its match in view when the rows above measure late chat-find's fold case failed on #19761's react-apps-check (inView false after 5 s): Find holds a new match in view for only 600 ms, and on a loaded runner the opened fold and the rows above it measure later. Grow a row above the match after that window. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: Find keeps its match in view until the reader scrolls A new match was held in view for 600 ms after it was drawn. On a slow machine the fold it opens and the rows above it measure later, so the match slid out of view. It now stays in view until the reader scrolls the transcript (outside Find's own bar) or Find moves on, as a pending jump to latest already does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test: Find lands its match in view on a CPU-starved page Review: the open-ended hold let programmatic scrolls (jump to latest, previous/next turn) snap back to the match, and padding a border-box row fires no measure, so the late-measure step proved nothing. Back to the base reveal; the fold case now runs under 40x CPU throttling, the loaded runner of #19761's red, where the 600 ms window ends before the fold and the rows above the match measure. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: count Find's reveal window in frames, not milliseconds Find holds a new match in view while the fold it opens and the rows above it measure. The 600 ms window ran out on a starved page before those measures landed, leaving the match out of view. Count 36 animation frames instead: the same hold on an idle page, and a starved page that draws fewer frames keeps it until its measures land. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: keep Find's 600 ms floor, cap the hold, own each window Review: on a 120 Hz display 36 frames are 300 ms, half the base hold; the hold now ends after 36 frames and 600 ms, or 5 s at most, so a page hidden mid-reveal (no frames) does not snap a reader back once shown. Each window owns a number, so a stale loop from an earlier reveal of the same match cannot end a newer one early. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: drop the 5 s cap on Find's reveal Under 40x throttling the fold measures after 5 s, so the cap ended the hold before the match landed (chat-find.test.ts:210 red on 34ddcba). A page hidden mid-reveal now finishes its remaining frames once shown, at most 36 frames and 600 ms. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Summary
Between 04:23 and 05:20Z, #19699 and #19703 went red in
web / react-apps-checkongallery-coverage. The failing finding waspages/settings/components/ThemeFiles.tsx#ThemeFiles has no entry. That gap came from #19688, which neither PR touched. #19572 already limits the check to findings the PR owns. But both PRs editedwebviews/src/gallery/format.ts, and that file was in the check'srules, which makes every finding count. format.ts changed in 5 feat PRs in the last three days. So every PR that edited the gallery format inherited every coverage gap on the base branch.validateEntries. It,play.tsand the experiments files decide only "the entries are valid".gallery-coverage.test.ts,prScope.tsandscripts/gallery/entries.tsas their rules. Editing the check itself still owns every finding, and a new case pins that.componentEntryand the other entry constructors, which build thecoversandidthe coverage tests read. They now live insrc/gallery/entry.ts, which is a rule for every gallery test. format.ts re-exports them, so no importer changes. New cases pin both sides: editing entry.ts owns coverage gaps, and editing format.ts still owns entry problems.This was the largest still-active cause of unrelated reds in the last 12 hours, after same-tree, which #19615/#19616 fixed.
Testing
heavy --mem 4G python3 -m unittest scripts.tests.test_pr_scoped_webviews_checks:test_coverage_base_gap_warns_on_a_pr_changing_the_gallery_formatfailed with1 != 0;every component and page has an entry or an allowlist linefailed.bun test test/gallery-coverage.test.tspasses (6/6), andbun run checkpasses.Changelog
none
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Stops gallery coverage checks from flagging unrelated base-branch coverage gaps on PRs that edit
webviews/src/gallery/format.ts.The check previously listed
format.tsas owning the whole check, so editing it made every existing base-branch finding (likeThemeFilesmissing an entry) count against the PR. The entry constructors moved to the newwebviews/src/gallery/entry.ts(format.tsre-exports them), and that file stays a rule for every gallery test, whileformat.tsedits now own only "the entries are valid" findings. New tests pin thatformat.tsedits don't own base coverage gaps but do own entry problems, and that editing the check orentry.tsstill owns every finding.Written for commit c1f4b5f. Summary will update on new commits.