websem: close the shared SVG offset attribute row - #118
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (8)
📒 Files selected for processing (11)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughThe change adds scalar SVG number parsing, updates gradient-stop and component-transfer ChangesSVG offset closure
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The PR aligns existing SVG offset parsing with browser behavior while preserving bounded fallback and clamping, with focused tests and fixtures covering the changed cases. No actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant SVGInput
participant stop_offset
participant parse_one
participant GradientRenderer
participant ComponentTransfer
participant GammaTable
SVGInput->>stop_offset: stop offset text
stop_offset->>parse_one: parse one SVG number
parse_one-->>stop_offset: ordered scalar or None
stop_offset->>GradientRenderer: normalized stop offset
SVGInput->>ComponentTransfer: feFunc* offset
ComponentTransfer->>GammaTable: gamma offset value
GammaTable-->>ComponentTransfer: transfer table
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 4 files. (7 skipped: 7 unsupported.) ✨ Finishing Touches📝 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 |
1 participant
Verdict
CLOSE the shared SVG
offsetattribute row across both static consumerfamilies: gradient
<stop>and<feFuncR>/G/B/A. The CSS motionoffsetshorthand,<feOffset>, and animation remain separate open work.The audit found and repaired a shipped silent wrong pixel in the stop route:
raw Rust
f32parsing selected a different adjacent value from Chromium'sordered SVG-number evaluation. Component-transfer offset already used the
ordered evaluator; its distinct number-only grammar is now independently
celled.
What landed
evaluator without inheriting number-list separator recovery.
<stop offset>through it before binary32 percentage normalization.Stops accept one number or percentage, use initial zero for invalid text,
clamp to
[0, 1], then clamp to the running maximum; equal offsets preservea hard transition.
<feFunc*> offsetas number-only with initial zero, Chromium's measuredlone-trailing-comma prefix, gamma-only use, and invalid percentage fallback.
split.
just add: stop grammar, stop numberorder, function grammar, and function number order.
fixture catalog, generated status, manifests, and every current count.
Measured boundary
Chromium 149.0.7827.55 and both actual n0 admissions agree exactly on all four
committed cells. Every scratch candidate was captured twice through the shared
hash-pinned module and rendered through strict and best-effort n0; a clean
process exit was not treated as pixel proof.
The former stop route chose the wrong neighbour for two source classes:
0.502541578243925989is Chromium-identical to0.50254154, not the rawparser's
0.5025416neighbour.86.591846437450251439%is Chromium-identical to86.59185%, not the rawroute's
86.59184%neighbour.The combined committed witness moves 48 pixels at maximum channel delta 202
under the old raw parser. The separate function witness makes
57384.267578125007select57384.265625, not57384.26953125, and amplifiesthat source choice through a second transfer.
The wider parser matrix established the consumer split: stops accept numbers
and percentages but reject a lone trailing comma; transfer functions accept
numbers only and retain that comma prefix. Units, trailing dots, calculations,
variables, CSS-wide keywords, comments, non-ASCII whitespace, extra members,
malformed exponents, finite extremes, and overflow all reach the measured
initial or clamped behavior. After the repair, no second silent mismatch class
remained. No refusal row was added because every valid class in this rung is
represented exactly.
Gate sensitivity
f32parsing madesvg-stop-offset-grammarfail by144 pixels at maximum delta 202 and
svg-stop-offset-number-orderfail by48 pixels at the same maximum delta.
svg-filter-component-transfer-offset-grammarby 1,152 pixels at delta 191and the number-order cell by 1,344 pixels at delta 255.
Restoring each semantic branch returned the complete 1,047-cell gate to exact
green. No tolerance was added.
Checklist and counts
offset:[ ]→[x].offset: remains[ ].baseline are untouched.
Independent rung review
No Workflow runner is exposed in this environment, so
.agents/workflows/verify-rung.jscould not be invoked through itsscriptPath. I reproduced its two independent roles by hand from the ignoredsvg-offset.plan.mdbrief:both complete applicabilities and their distinct grammars, verified that
only the SVG attribute row ticks, traced the silent defect through its API
contract, and checked the no-matcher, no-silent-pixel, no-score, and FLIP
laws.
admissions, checked that all four oracles are additions, verified both
1,047-entry ledgers are unique and hash-current, proved sensitivity in both
consumer paths, rebaked every oracle, and reran exact gate/status checks.
The skeptical pass caught and corrected an initial documentation edit that
would have ticked the unrelated CSS motion property; the final diff changes
exactly one checklist status, the SVG attribute row.
Verification
cargo test -p websem -p n0_cli -p rframe -p n0cargo fmt --allandcargo fmt --all -- --checkcargo clippy --no-depscargo clippy --no-deps --workspace --exclude grida-canvas-wasm -- -D warningsjust bake(Chromium 149.0.7827.55; 1,047 oracles)just gatejust statuspnpm fmt:checkpnpm exec oxlint --deny-warningspnpm --filter www types:checkpnpm --filter www buildpnpm --filter @grida/reftest typecheckpnpm --filter @grida/reftest buildpnpm --filter @grida/reftest test(51 passed, 1 skipped)git diff --check