websem: close direct SVG paint-order - #133
Merged
Merged
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: Team Run ID: ⛔ Files ignored due to path filters (67)
📒 Files selected for processing (17)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughThe change adds SVG ChangesSVG paint-order support
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The SVG paint-order support is internally consistent and comprehensively tested, with no actionable merge-blocking risk identified. Sequence Diagram(s)sequenceDiagram
participant SVGCompiler
participant PaintOrderResolver
participant MarkerCompiler
participant FrameItems
SVGCompiler->>PaintOrderResolver: resolve inherited paint-order
SVGCompiler->>MarkerCompiler: compile marker positions
PaintOrderResolver->>FrameItems: normalize fill/stroke/marker order
MarkerCompiler->>FrameItems: provide marker span
SVGCompiler->>FrameItems: emit ordered channels with opacity scope
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 24 functions across 4 files. (13 skipped: 12 unsupported, 1 too large.)
✨ 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 |
13 tasks
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.
Verdict
PO1 CLOSE/SPLIT: close the direct SVG
paint-orderpresentation-attribute row at the complete standard-track grammar, while leaving the CSS property row open at the pinned Stylo boundary.The pinned Servo-mode Stylo has no
paint-orderlonghand. This change therefore resolves only direct SVG presentation attributes through one SVG-namespace-bounded inherited walk; authored inline and stylesheet CSS continue to refuse by a stable property name. No matcher is added beside the cascade.What changed
normal | [ fill || stroke || markers ]with CSS token rules, including comments, escapes, ASCII case, CSS-wide values, invalid fallback, inheritance, and normal-order completion of omitted members.var(),env(),attr(), experimentalif()) plus the independent CSS-property row.Measured cruxes
Chromium 149.0.7827.55 was measured through the shared hash-pinned capture module, and every candidate also rendered through both actual
n0CLI admissions.stroke var(...),stroke env(...), typedstroke attr(...), andstroke if(...)all equalstroke markersin Chromium. The first implementation silently selected normal in both admissions, changing 816 pixels at delta 161. The parser now patrols the complete top-level token stream, and focused refusal cells place each function afterstroke.paint-orderon<line>. A dedicated stroke/marker line witness now differs from normal by 92 pixels at delta 197 and is committed.feFlooddiffers between markers-first and normal by 112 pixels at delta 197, with Chromium pixels[37,99,235,255]versus[220,38,38,255]at(44,25). It already reaches the registered transactional marker-source-effect refusal in strict and best-effort paths, so it remains outside this rung (measured, not celled).Gate sensitivity
Each deliberate defect was restored before the final green run:
just gatefail 27 named cells (largest 2,017 pixels; max delta 214);Identity(1);Verification
just bake— Chromium 149.0.7827.55 verified 1,157 primitive oracles without replacing an existing oraclejust gatejust statuscargo test --workspace --exclude grida-canvas-wasmcargo clippy --no-deps --workspace --exclude grida-canvas-wasm -- -D warningscargo fmt --all -- --checkpnpm fmt:checkpnpm exec oxlint --deny-warningspnpm --filter www types:checkpnpm --filter @grida/reftest typecheckpnpm --filter @grida/reftest buildpnpm --filter @grida/reftest test— 51 passed, one skippedpnpm --filter www exec next build --webpack— 177 static pages generatedThe saved
.agents/workflows/verify-rung.jsrunner is not exposed in this environment. I reproduced its independent TICK/LAW and REPRO roles manually; those passes found and fixed the two issues above. The exact local Turbopacknext buildremained silent in its optimized-build phase and was stopped after a bounded wait; the equivalent webpack production build passed, and hosted Node CI remains required for the exact Turbopack command.No conformance score was produced. FLIP records, rules, and baselines are untouched. No tolerance was added.
Advances #43.