#615: Six render snapshot tests pass locally and fail in CI: nerd-font glyph widths resolve differently, so the Test stage has been reporting green on machine-dependent assertions - #616
Closed
JDonaghy wants to merge 2 commits into
Conversation
…biguity Six render snapshot tests (snapshot_normal_mode, snapshot_insert_mode, snapshot_visual_selection, snapshot_command_line, snapshot_split_panes, snapshot_line_numbers) passed locally but failed in CI on identical source, off by one column around nerd-font glyphs. Investigation ruled out both candidate root causes empirically, not just by inspection: - Toolchain skew: installed rustc 1.97.1 (CI's exact build, 8bab26f4f 2026-07-14) locally and ran the literal CI commands (`cargo test --verbose`, default features including `gui`) — all 6 pass, byte-identical to the 1.95.0 baseline. - unicode-width version drift: force-resolved unicode-width down to 0.2.0 (the floor allowed by the vendored vt100 patch) via `cargo update --precise` — output is byte-identical to the locked 0.2.2. Verified directly that `UnicodeWidthChar::width()` returns `Some(1)` for the specific Nerd Font PUA codepoints in both versions. - Confirmed vimcode source is bit-identical between this branch and the actual failing CI run: the issue-613 branch that ran CI is this exact commit plus only workflow-file and Cargo.toml-comment changes (verified via `git diff`). Given both mechanisms are ruled out, and quadraui's own `cell_width()` doc comment already documents that these exact codepoints (Nerd Font Supplement PUA) fall into Unicode's "Ambiguous" East Asian Width category — which by UAX #11 permits either 1- or 2-column rendering — the columns around these glyphs are not something the rendering code can guarantee across otherwise-compatible builds. Asserting exact padding there tests the environment, not the renderer. Rewrite the snapshot assertions per the issue's second sanctioned outcome: collapse whitespace runs only on rows that actually contain a PUA glyph before snapshotting, so the tests assert on content and ordering rather than the exact column count separating them. Rows without nerd-font glyphs (editor content, gutters, dividers) are untouched and still asserted byte-for-byte. Pre-existing, unrelated: `cargo clippy -- -D warnings` currently fails repo-wide on ~90 snake_case naming violations in test files (e.g. tests/new_vim_features.rs, tests/nvim_conformance.rs, src/core/engine/tests.rs) that predate this change and are out of scope for #615. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…g it Review of 89ea46a found that desensitize_glyph_width() collapsed *every* whitespace run on a row merely because the row contained a PUA glyph somewhere, rather than only the whitespace actually adjacent to that glyph. Since a Nerd Font icon prefixes almost every gutter/status/tab row in these fixtures, this silently blanked spacing assertions far from any width ambiguity — e.g. line_numbers.snap's gutter alignment, and normal_mode.snap's code-indentation column (` println!(...)` collapsed to a single space even though the indentation has nothing to do with the PUA glyph two columns away, separated by a `│` divider). Rewrite desensitize_glyph_width() to only collapse a whitespace run when it is immediately adjacent (directly touching, no intervening non-space char) to a PUA codepoint on either side. Every other whitespace run on the same row — gutter padding, code indentation, status-line column alignment — is left untouched and asserted byte-for-byte, same as rows with no PUA glyph at all. Re-accepted the six snapshots against this narrower rule (`cargo insta accept`, run locally, not against CI output); most of them now assert *more* exact spacing than before, e.g. the code indentation and gutter-divider columns that the full-row collapse had been silently discarding. Also corrects the previous commit's claim that "rows without nerd-font glyphs are untouched and still asserted byte-for-byte" — byte-inspection of the pre-fix .snap files showed a PUA icon prefixes nearly every content/gutter/status row in all six fixtures, so that framing understated how much of each row the old full-row collapse actually touched. Flagging back to the issue thread (not resolved here): the previous commit's local `+1.97.1` toolchain reproduction came back byte-identical/ passing, which contradicts this issue's own pinned "verified" root-cause note asserting rustc 1.97.1-vs-1.93.1 toolchain skew *is* the reproducible mechanism. That disagreement is unreconciled — either the pinned lead needs correcting, or there's an environment factor (LANG/locale, ubuntu- 24.04 ICU/glibc) the local repro didn't capture. Doesn't block this fix since the narrowed desensitization is correct either way, but worth someone reconciling on #615/#613 directly. cargo test --verbose: all tests pass, including the six snapshot tests run individually via `cargo test --bin vimcode`. No new warnings. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner
Author
|
Closing as superseded — the change here was built on a root cause that turned out to be false. The PUA/nerd-font glyph-width theory is dead, along with three others. Established this session:
Real cause: Proven by bisection: seeding a fresh $HOME with These two commits weakened six snapshots to hide a symptom of a cause that never existed, which was also this PR's first review round's blocking finding. Reverted on the Full corrected root cause is pinned on #615. |
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.
Closes #615
Automated PR opened by coordinator for review of issue #615.