Skip to content

Route constant scatter strokes through Scene exports - #242

Merged
DecisionNerd merged 1 commit into
mainfrom
feature/public-scatter-stroke
Aug 26, 2026
Merged

Route constant scatter strokes through Scene exports#242
DecisionNerd merged 1 commit into
mainfrom
feature/public-scatter-stroke

Conversation

@DecisionNerd

@DecisionNerd DecisionNerd commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • route literal constant scatter strokes and explicit finite nonnegative widths through public Scene exports
  • normalize authored constant strokes with omitted width to the shared 1px default across Python and Node
  • keep width-only match-fill, per-item styling, custom symbols, density/LOD, component opacity, polar, and secondary-axis cases on explicit compatibility paths
  • update authoritative Scene ownership and export specifications

Validation

  • Rust Scene tests (51 passed)
  • affected Python tests (187 passed)
  • full Node suite (199 passed)
  • browser render smoke
  • Scene ownership audit
  • Ruff check and format check
  • full pre-commit suite

Advances #58 without closing it.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Summary by CodeRabbit

  • New Features

    • Scatter markers now support constant stroke colors and optional non-negative stroke widths across SVG, PNG, PDF, and browser rendering.
    • Stroke-only scatter styles default to a 1px stroke width.
    • Static exports support all built-in scatter symbols with constant strokes.
  • Bug Fixes

    • Improved marker clipping so boundary-overlapping stroked circles remain visible.
    • Invalid stroke widths and unsupported per-item styling are now rejected consistently.
  • Documentation

    • Updated export and rendering support specifications to reflect scatter stroke capabilities.

@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c8bd3e13-ea94-471f-aaee-33c3481c8e44

📥 Commits

Reviewing files that changed from the base of the PR and between 9a1e83f and 9c26ce8.

📒 Files selected for processing (15)
  • crates/xyg-engine/src/scene.rs
  • packages/xy-node/src/figure.js
  • packages/xy-node/src/marks/scatter.js
  • packages/xy-node/src/scene.js
  • packages/xy-node/test/scene.test.mjs
  • python/xyg/_scene_v3.py
  • spec/api/export.md
  • spec/design-dossier.md
  • spec/design/host-parity.md
  • spec/design/ownership-audit.md
  • spec/design/scene-ir.md
  • tests/fixtures/figure_scene_v3.json
  • tests/test_css_mark_styles.py
  • tests/test_figure_scene_v3.py
  • tests/test_scene_export_support.py

Included review availability: 1 review is currently available. Your included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour.


📝 Walkthrough

Walkthrough

The change adds constant scatter stroke support to Scene v3 exports. It defaults stroke-only styles to 1px, preserves compatibility routing for unsupported styles, validates export output, and tests stroke-inclusive marker clipping.

Changes

Constant scatter stroke export

Layer / File(s) Summary
Scatter style normalization
packages/xy-node/src/marks/scatter.js, packages/xy-node/src/figure.js, packages/xy-node/src/scene.js
Scatter styles default stroke_width to 1 when stroke is present. Trace width resolution now uses stroke_width, width, line_width, then the kind default.
Scene routing and export contracts
python/xyg/_scene_v3.py, spec/design/*.md, spec/api/export.md
Public Scene routing accepts constant scatter strokes and finite non-negative widths. Width-only and richer unsupported styles remain on the compatibility route.
Rendering and compatibility validation
tests/test_figure_scene_v3.py, tests/test_scene_export_support.py, packages/xy-node/test/scene.test.mjs, tests/test_css_mark_styles.py, tests/fixtures/figure_scene_v3.json, crates/xyg-engine/src/scene.rs
Tests cover serialized styles, SVG/PNG/PDF/raster/browser output, invalid widths, compatibility boundaries, canonical CSS colors, fixture bytes, and stroked-marker clipping.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 9c26c

This localized change routes supported scatter-stroke cases through Scene exports and preserves compatibility paths, with no actionable merge-blocking risk remaining after normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant ScatterComposer
  participant PublicSceneRouter
  participant RustScene
  participant ExportConsumers
  ScatterComposer->>ScatterComposer: normalize constant stroke style
  ScatterComposer->>PublicSceneRouter: provide scatter stroke and width
  PublicSceneRouter->>RustScene: route supported constant style
  RustScene->>ExportConsumers: render SVG, PNG, PDF, raster, or browser output
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 35.71% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 9 files. (6 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: routing constant scatter strokes through Scene exports.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 35.71% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 9 files. (6 skipped: 6 unsupported.)

✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/public-scatter-stroke

Warning

Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption.


Comment @coderabbitai help to get the list of available commands.

@blacksmith-sh

This comment has been minimized.

@DecisionNerd
DecisionNerd force-pushed the feature/public-scatter-stroke branch from 8e73bfb to 9c26ce8 Compare August 25, 2026 20:37
@github-actions

Copy link
Copy Markdown

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@DecisionNerd
DecisionNerd merged commit 6f4289b into main Aug 26, 2026
17 checks passed
@DecisionNerd
DecisionNerd deleted the feature/public-scatter-stroke branch August 26, 2026 03:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant