Repository navigation
test: compare Cloud tree title columns on the backing-pixel grid - #16092
Merged
Merged
Conversation
"Cloud, locked, local and pending machine titles share the folder icon column" has failed on main since #16004 on 1x runners, sections style at 150%: title starts [33, 39, 39, 38, 37] against a 1.5pt allowance. Every machine title there is laid out at 37.5pt, between two backing pixels. #16004 widened the rows (trailing padding 12 to 8), which flipped the pending row's snap one pixel left; the local row's semibold title already crossed the 0.2 ink threshold a pixel before the medium ones. The layout did not move: before and after screenshots from the same runner differ only in that one title's raster, shifted by exactly one pixel. Ink edges are whole pixels, so express the allowance on that grid, as iconLabelSpacing in the same file already does. At 1x this allows 2px at 150% and 1px at 75%; 100% and 200% are unchanged, and a missing band inset (6pt) or an unscaled gap (3pt at 150%) still fails. Refs #15488 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
All contributors have signed the CLA ✍️ ✅ |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: manaflow-ai/cmux/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe compact layout test now rounds its title-alignment tolerance to a whole backing pixel, using the window’s backing scale. ChangesCompact layout test
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Other Suggested reviewers: Merge Risk: ⚪ Minimal · up to No concrete regression is evident in this test-only tolerance change; the threshold remains pixel-aligned while staying below the stated layout differences. 🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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
|
Merge receipt for |
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.
CloudTreeCompactLayoutTests"Cloud, locked, local and pending machine titles share the folder icon column" has failed on main since #16004, on the 1x fleet runners. It fails in the sections style at 150%, with title starts[33, 39, 39, 38, 37]against a 1.5pt allowance. The layout didn't move: one title's raster moved by one backing pixel, and the allowance can't absorb that. This is one of the #15488 failures.Cause
CloudTreeMachineBand,CloudTreeStyle). The CI windows render at 1x (380×620pt screenshots are 380×620px), so each title snaps to one of the two neighbouring pixels.Evidence
Change
The allowance is rounded to the backing-pixel grid, as
iconLabelSpacingin the same file already does ("rather than rejecting a 2px delta against 1.875px").What still fails:
This changes an assertion's precision, deliberately. The assertion compared ink edges, which are whole pixels, against a fractional-point allowance, so a half-point layout position failed or passed depending on which way the rasterizer rounded.
Verification
Run 36744359319 runs the suite twice, on the validation build (which includes #16004), with this change and against the unmodified bundle as a baseline. It ran on cmux14, a 1x fleet mini. With this change the test passed all 20 cases in both iterations. The unmodified bundle failed the sections case in both iterations, as it does on main. This PR's changed-suites job also passed all 20 cases on the head commit.
Overlaps
Changelog
🤖 Generated with Claude Code