Add a report-only style-compatibility preflight and the applicable-slot matrix - #452
Add a report-only style-compatibility preflight and the applicable-slot matrix#452Alek99 wants to merge 2 commits into
Conversation
…ot matrix chart.style_compatibility_report(target=..., engine=..., custom_css=...) routes every declared slot style for one export target into exactly one of four outcomes — survives, native-subset (naming the kept and lost properties per format family), browser-only, or state-gated — and mirrors the export path's own refusals (custom_css with a pinned native engine, Chromium SVG) instead of re-deciding them. Report-only: computing it never changes an export; the staged compatibility= modes that act on it come separately. Two properties are load-bearing. The report is constant-time when there is nothing to route: no class_names, no per-slot styles, no custom_css means no slot walk, so preflight stays free exactly where exports are hot. And state-gated is not lost: the capability registry now tags every slot with an applicability — present in a clean static export, or gated by hover, selection, crosshair, modebar, or view (reduction badges) — so a styled tooltip is recorded with its gating state rather than counted against a file that never contains a tooltip. Counting it before this change overstated the static parity gap by exactly the chrome a static file cannot contain: of 48 slots, 24 are clean-static and 24 state-gated, and all 10 native-capable slots are in the static set. Routing derives from the capability registry, the honored property subsets from the writers' own constants (xy._svg.SLOT_TEXT_PROPS / SLOT_RASTER_PROPS), and engine selection from export._resolve_image_engine — the preflight restates none of them, so it cannot disagree with them. The legend slot stays at declaration granularity (its box properties route through the merged legend declaration, which has no constant yet) and is qualified rather than guessed either direction (§28). The generated capability matrices gain the applicable-in column and the applicable-slot counts; new registry tests pin the partition (a new modebar_*/tooltip/crosshair/badge slot that forgets its family state fails the suite, and native support on a state-gated slot is rejected until the interaction-snapshot phase adds it deliberately). export.md §9 documents the report as its programmatic form.
|
Warning Review limit reached
Next review available in: 2 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe change adds export applicability metadata for chart slots and introduces report-only style compatibility analysis. Figure and Chart expose the report API. Documentation and tests cover routing, state-gated slots, export refusals, and capability counts. ChangesStyle compatibility reporting
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Chart
participant Figure
participant Preflight
participant ExportResolver
participant Report
Chart->>Figure: request style_compatibility_report(target, engine, custom_css)
Figure->>Preflight: analyze figure styles
Preflight->>ExportResolver: resolve target and engine
ExportResolver-->>Preflight: return writer capabilities and refusals
Preflight->>Report: classify style routes and findings
Report-->>Figure: return compatibility report
Figure-->>Chart: return report without export
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThe PR adds a report-only styling compatibility preflight and records whether each chrome slot applies to static exports or requires interaction/view state.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| python/xy/styling/preflight.py | Adds compatibility report data structures and routing logic; the previously reported custom-CSS validation bypass is fixed by invoking the export path’s validator. |
| python/xy/styling/capabilities.py | Adds explicit static and state-gated applicability metadata to chrome slot capabilities. |
| python/xy/_figure.py | Adds the Figure-level compatibility-report entry point with a deferred import. |
| python/xy/components.py | Adds the public Chart wrapper for compatibility reporting. |
| tests/test_style_compatibility_report.py | Covers routing, writer subsets, state-gated slots, engine refusals, malformed styling, and custom-CSS validation parity. |
| tests/test_capability_registry.py | Extends registry invariants to pin applicability coverage and state-family assignments. |
Reviews (2): Last reviewed commit: "Close the preflight report's two mirror ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Merging this PR will not alter performance
Comparing 
Footnotes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (5)
scripts/gen_capability_matrix.py (1)
66-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the native slot-style count from the registry.
counts['slots_styleable_natively']is dynamic, but Line 69 hard-codesnineand therootexception. If a slot changes its channel or native support, the generated summary can become stale while the table and total remain current. Add a registry-derived count for thestyles={...}path, or remove the numeric split. The generatedspec/api/capability-matrix.mdsummary inherits this literal.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/gen_capability_matrix.py` around lines 66 - 74, Update the summary generation near counts['slots_styleable_natively'] to derive the styles={...} slot count from the capability registry instead of hard-coding “nine” and the root exception. Reuse the registry’s native-support/channel data so the generated prose stays synchronized with the table and total when slots change.python/xy/styling/preflight.py (3)
42-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClassify the writer family from an explicit format set.
Line 183 treats every non-raster format as vector, and
_VECTOR_FORMATSis never read. A format added later toexport._normalize_formatwould silently be reported against the vector subset. Select the family from both sets and refuse an unclassified format.♻️ Proposed change
-def _styles_finding(slot: str, decls: dict[str, Any], fmt: str) -> SlotFinding: - meta = _SLOTS_BY_ID[slot] - family = "native_raster" if fmt in _RASTER_FORMATS else "native_vector" +def _slot_family(fmt: str) -> str: + if fmt in _RASTER_FORMATS: + return "native_raster" + if fmt in _VECTOR_FORMATS: + return "native_vector" + raise ValueError(f"preflight has no writer family for format {fmt!r}") + + +def _styles_finding(slot: str, decls: dict[str, Any], fmt: str) -> SlotFinding: + meta = _SLOTS_BY_ID[slot] + family = _slot_family(fmt)
_honored_propsreadsfmt in _RASTER_FORMATSfor the same decision, so it can take the resolved family instead of re-deriving it.Also applies to: 183-183
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/xy/styling/preflight.py` around lines 42 - 43, Update the writer-family classification near _honored_props to resolve the format explicitly against _RASTER_FORMATS and _VECTOR_FORMATS, rather than treating every non-raster format as vector. Reject or otherwise fail clearly for formats in neither set, then pass the resolved family through so _honored_props does not re-derive it from fmt.
61-80: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
sourcesmakes the frozen report mutable and unhashable.
StyleCompatibilityReportisfrozen=True, butsourcesholds a plaindict, so a caller can mutate the report contents after it is returned. The generated__hash__also fails on the dict field, so the report cannot go into a set or a dict key even though every other field is hashable. AMappingfield normalized at construction keeps the report read-only.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/xy/styling/preflight.py` around lines 61 - 80, Update StyleCompatibilityReport.sources to use an immutable Mapping representation rather than a plain dict, and normalize incoming mappings during construction so callers cannot mutate the frozen report. Ensure the normalized sources value remains hashable for generated __hash__ support while preserving existing source lookups and defaults.
275-281: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDo not skip invalid style entries in
preflight.Figure.class_namesandFigure.chrome_stylesare directly assignable, so callers can request a report before_dom_specvalidates them. Report unknown slots and non-dict declarations explicitly, or validate them before routing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/xy/styling/preflight.py` around lines 275 - 281, Update the preflight finding collection around Figure.class_names and Figure.chrome_styles so invalid entries are not silently skipped before _dom_spec validation. Report unknown slots and non-dict chrome style declarations explicitly, or route them through the existing validation logic before creating _class_finding or _styles_finding results.python/xy/_figure.py (1)
2220-2237: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
style_compatibility_reportreturnsAnyat both public entry points. The preflight module returnsStyleCompatibilityReport, andpreflightimportsFigureonly underTYPE_CHECKING, so both call sites can import the report type underTYPE_CHECKINGwithout a runtime import cycle. Callers then get type checking onlossless,findings,losses, andexplain().
python/xy/_figure.py#L2220-L2237: addfrom .styling.preflight import StyleCompatibilityReportto theTYPE_CHECKINGblock and change the return annotation fromAnyto"StyleCompatibilityReport".python/xy/components.py#L4172-L4191: add the sameTYPE_CHECKINGimport and changeChart.style_compatibility_reportto return"StyleCompatibilityReport".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/xy/_figure.py` around lines 2220 - 2237, Replace the public Any return annotations for style_compatibility_report with StyleCompatibilityReport and add the report-type import under TYPE_CHECKING in python/xy/_figure.py lines 2220-2237 and python/xy/components.py lines 4172-4191. Keep the runtime preflight import structure unchanged and use the forward-reference annotation to avoid import cycles.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@python/xy/_figure.py`:
- Around line 2220-2237: Replace the public Any return annotations for
style_compatibility_report with StyleCompatibilityReport and add the report-type
import under TYPE_CHECKING in python/xy/_figure.py lines 2220-2237 and
python/xy/components.py lines 4172-4191. Keep the runtime preflight import
structure unchanged and use the forward-reference annotation to avoid import
cycles.
In `@python/xy/styling/preflight.py`:
- Around line 42-43: Update the writer-family classification near _honored_props
to resolve the format explicitly against _RASTER_FORMATS and _VECTOR_FORMATS,
rather than treating every non-raster format as vector. Reject or otherwise fail
clearly for formats in neither set, then pass the resolved family through so
_honored_props does not re-derive it from fmt.
- Around line 61-80: Update StyleCompatibilityReport.sources to use an immutable
Mapping representation rather than a plain dict, and normalize incoming mappings
during construction so callers cannot mutate the frozen report. Ensure the
normalized sources value remains hashable for generated __hash__ support while
preserving existing source lookups and defaults.
- Around line 275-281: Update the preflight finding collection around
Figure.class_names and Figure.chrome_styles so invalid entries are not silently
skipped before _dom_spec validation. Report unknown slots and non-dict chrome
style declarations explicitly, or route them through the existing validation
logic before creating _class_finding or _styles_finding results.
In `@scripts/gen_capability_matrix.py`:
- Around line 66-74: Update the summary generation near
counts['slots_styleable_natively'] to derive the styles={...} slot count from
the capability registry instead of hard-coding “nine” and the root exception.
Reuse the registry’s native-support/channel data so the generated prose stays
synchronized with the table and total when slots change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b6ec73fb-cdf4-4123-a9de-5e3244115ad4
📒 Files selected for processing (12)
CHANGELOG.mddocs/styling/capabilities.mdpython/xy/_figure.pypython/xy/components.pypython/xy/styling/__init__.pypython/xy/styling/capabilities.pypython/xy/styling/preflight.pyscripts/gen_capability_matrix.pyspec/api/capability-matrix.mdspec/api/export.mdtests/test_capability_registry.pytests/test_style_compatibility_report.py
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 12 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
…ounts
Review follow-ups on the preflight change, all in the mirror-the-export
direction:
Browser-resolved targets now validate custom_css through the export path's
own _custom_css_block (type check, </style> and comment-sequence rejection),
in the export's own order — engine resolution first. The report could
previously call an export lossless that would refuse its stylesheet.
Malformed figure styling raises exactly like the export instead of being
skipped: class_names/chrome_styles are assignable, so a report can be
requested before the spec build validates them, and a silently omitted entry
would be a report hiding a declaration (§28). validate_dom_slots runs the
same check the spec build runs; a non-mapping declaration set is refused by
name.
The writer family is now classified from both format sets with a refusal
for anything unclassified, so a future format cannot silently be reported
against the vector subset. The generated summary derives the styles={...}
slot count from the registry instead of hard-coding "nine" (the
axis_style_keys lesson: prose cannot hold a count). Both public entry
points return the typed StyleCompatibilityReport rather than Any.
There was a problem hiding this comment.
All reported issues were addressed across 7 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
1 participant
First change of the styling-compatibility program (Phase 0 of the plan): make every export explain itself before changing anything about how exports behave.
What this adds
chart.style_compatibility_report(target=..., engine=..., custom_css=...)— a report-only preflight that routes every declared slot style for one export target into exactly one of four outcomes:survives,native-subset(naming the kept and lost properties per format family),browser-only, orstate-gated. It mirrors the export path's own refusals (custom_csswith a pinned native engine, Chromium SVG) by calling the same resolver, so it can never disagree with what running the export would do. Report-only: computing it changes no export. The stagedcompatibility=modes that act on the report come in the follow-up PR.hover(5),modebar(14),crosshair(2),selection(1),view/reduction badges (2). The generated matrices gain the applicable in column and applicable-slot counts. A styled tooltip is no longer counted as "dropped" by a static PNG that never contains a tooltip — recorded with its gating state instead (§28: the distinction is written down, not silent).Contract properties (tested)
class_names, no per-slotstyles, nocustom_css→ no slot walk. Preflight stays free exactly where exports are hot._svg.SLOT_TEXT_PROPS/SLOT_RASTER_PROPS), engine selection fromexport._resolve_image_engine. The preflight restates none of them.modebar_*/tooltip*/crosshair_*/badge*slot forgets its family state, and reject native support claims on state-gated slots until the interaction-snapshot phase adds them deliberately.Evidence
spec/api/export.md§9 gains the report as its programmatic form;spec/api/capability-matrix.mdanddocs/styling/capabilities.mdregenerated (the--checkdrift gate covers both).ruff check/ruff format --check/ pre-commit: clean.ty check: no diagnostics in the new code (baseline count unchanged).Part of the full styling-compatibility plan (resolved-style IR → live snapshot → native chrome parity); next in the stack: opt-in
compatibility=warn/strict modes and the engine-collision contract.Summary by CodeRabbit
New Features
style_compatibility_report()for charts and figures to preview how styling will behave for a selected export format and engine.Documentation