test(glyph): cover the metrics-only run-topology split from #66 - #81
Merged
Conversation
Ports the regression scenario from #66, which was written against the pre-rename `packages/text` tree and so could never merge. A colored span whose fontSize animates through value-equality with its root keeps its style segment while the shaping-run table merges across it -- runs coalesce on `same_layout_style`, which compares layout scalars and ignores paint, and `font_size` is one of those scalars. The next tick moves the size off equality and the table splits again under metrics-only invalidation. The engine once retained the merged shape against the rebuilt table and rejected every later frame with invalidRequest, poisoning the session; `shaping_run_topology_stable` was added to stop that and is already on main. The test does not isolate that guard. Forcing `shaping_run_topology_stable` to return `true` -- the pre-fix behaviour -- and rebuilding the shaper leaves the whole suite green, so a later path absorbs the split as well. It lands as a canary over the retained-shape path, and the comment says so rather than claiming a proof it does not deliver. That guard has no other coverage: no Rust test exercises it.
size-limit report 📦
|
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.
Ports the regression scenario from #66, which targets the pre-rename
packages/texttree and therefore can never merge. The fix it guards (shaping_run_topology_stable) is already onmainatpackages/glyph/rust/shaper/src/engine/state.rs:1246; only the test was left behind.The scenario
A colored span whose
fontSizeanimates through value-equality with its root keeps its style segment (the paint differs) while the shaping-run table merges across it. Runs coalesce onsame_layout_style, which compares layout scalars and ignores paint:So frame 1 (span size == root, different color) merges to one run, and frame 2 (17.5) splits back to three under metrics-only invalidation. The engine once retained the merged shape against the rebuilt table and rejected every later frame with
invalidRequest, poisoning the session.What this test does and does not prove
It drives the merge → split path end to end and asserts the session stays healthy across three frames.
It does not isolate
shaping_run_topology_stable. Forcing that function toreturn true— the pre-fix behaviour — and rebuilding the shaper leaves the entirethree-engine-runtimesuite green, including this scenario. Some later path absorbs the split as well; the most likely candidate is therefresh_scales_from_streamfallback dropping through to a full rebuild, though that is unverified.The test therefore lands as a canary over the retained-shape path, and its comment says exactly that rather than claiming a proof it does not deliver.
Follow-up worth taking separately
shaping_run_topology_stablehas no other coverage — no Rust test exercises it (the only topology test in the crate covers the unrelatedsame_edit_run_topology). Combined with the result above, it is worth determining whether the guard plus its two call sites are now redundant. That has a size payoff and belongs with the open items from the Wasm size study, not in this PR.Verification
node --test tests/integration/three-engine-runtime.test.mjs— 1 test, 1 passpnpm check— exit 0, OKF 0 errors / 0 warningsdocs/packages/glyph.mdsource_digestregenerated for the new package sourceCloses #66.