Fix the CI failures from #380, plus polar CodSpeed coverage and the pie legend - #387
Merged
Merged
Conversation
CodSpeed on #370 reported "103 untouched benchmarks" for a change that added a whole coordinate system, rewrote wedge geometry in three renderers, and shipped the most expensive mark in the engine. Nothing in the suite could see any of it, which is why a ~50k-polar-bar performance cliff was found by hand instead. `benchmarks/test_codspeed_polar.py` — six rows, all Python cost (what simulation mode measures): - payload prep for the three shapes with materially different validation and emit paths: a 100k polar line, a 16-sector / 50k-observation wind rose (Python-side binning plus stacked wedges), and a 24-slice pie, whose unequal widths take the four-edge column path rather than the compact scalar-width one; - SVG and native-PNG export of the same rose, bracketing the arc-flattening term. SVG draws real `A` arcs and needs no subdivision count, so it is the control; PNG flattens every wedge at `config.polar_bar_segments(span, turn)` vertices, so a regression back to a flat full-turn count lands here as an arc-flattening step change rather than as a bug report; - a polar heatmap's bounded screen-space inverse raster, which has no Cartesian twin. The payload rows assert bounds rather than sizes — the rose's bytes must stay bounded by sector and band count, never by observation count — so a row cannot get cheaper by shipping a different chart. The report's other item was 2 skipped rows falling back to baseline results. Those are orphans in the dashboard, not skipped tests: the suite collects exactly 103 rows and CodSpeed measured 103, so the 2 extra rows have no code behind them and need archiving there by hand. What the repo can do is stop it recurring — `test_codspeed_row_count_matches_the_methodology_spec` collects the modules by AST and gates the total against `spec/benchmarks/methodology.md` §8, whose count was itself already one row stale. Deleting a benchmark stays allowed; deleting one silently does not. Also adds a `polar_coordinate_system` benchmark category, documents the module in §8 and `benchmarks/README.md`, and repoints the triangle-mesh cleanup assertion at the shared `TRACE_GPU_BUFFERS` list the buffer names moved into.
Two defects reported against the pie chart on #370, with one shared cause. **The legend scrolled horizontally.** The box is capped at `--xy-legend-max-width`, but its grid columns were `max-content` and so refused to shrink below their content: an over-wide row overflowed and `overflow:auto` answered with a sideways scrollbar, hiding the label it was meant to be showing. Columns are now `minmax(0, max-content)` and the box scrolls on the block axis only. Overflow on the inline axis ellipsizes per row instead, which is what the static exporters already do — and the full text stays reachable in `title`/ARIA, the same rule categorical tick labels use. Only the label clips: the swatch is `flex:none` and keeps `overflow:visible`, so an authored oversized marker still draws outside its 18x14 box. **The legend repeated the percentage.** `pie_chart` printed the value and the share, and for values that already sum to 100 — how most pie data arrives — those are the same digits: `[40, 30, 20, 10]` rendered `Direct 40 (40%)`. The share keeps the unit, so the bare value is dropped when it says nothing new. Decided once for the whole pie rather than per slice, because a legend where one row carries a bare value and the next does not is harder to read than either consistent shape; zero slices draw no wedge and get no row, so they cannot veto it. The doubled label was also what overflowed the box, so the two reports share a root cause. The polar legend gutter added earlier in this branch was a flat 96 px, which ellipsized `Partner (30%)` — an ordinary slice's default name — while being a fifth of a phone canvas and a fifteenth of a wide one. It is now 22% of the canvas width clamped to 120-200 px: still derived from the canvas rather than measured from the label set, so all three renderers reserve the identical box instead of drifting with their font metrics, and `floor` rather than `round` so Python and JavaScript land on the same integer pixel. `legend_item` and `legend_label` join the `:where()` chrome layer, so an author's utility class still wins, and `test_static_client_security.py` now requires both rules to stay defeatable.
`alek/polar-axes` went red at 7449360 (the squash of #380) after being green at a3e28bf, and all five failures are mine. **Two were tests pinning code I moved.** `test_tick_sides_bump_wire_protocol_and_client_in_lockstep` asserted the exact `00_header` import line, which grew a third name; it now checks that the client imports PROTOCOL rather than the spelling of the list. `test_triangle_mesh_resource_cleanup_deletes_every_coordinate_buffer` looked for the buffer names inside `_destroyTraceResources`, where they no longer live; it reads the shared `TRACE_GPU_BUFFERS` list and asserts the teardown uses it. (That second fix was already written, just not in the squash.) **Two were the polar legend gutter ellipsizing static labels.** At a flat 96 px the exporters' legend had 44 px of label room, so `series one` came out `series...` and `Organic 20 (33%)` came out `Orga...`. The gutter is now 22% of the canvas clamped to 120-200 px, which gives 68 px at the narrowest non-compact width and 146 px at the default export size — both failing labels fit. **One was the compact colorbar spending plot width.** Reserving a side gutter for the endpoint tick labels cost 36 px, and `test_narrow_fluid_resize_stays_painted_and_preserves_plot_space` guards exactly that (plot width >= 280, colorbar width 18). The endpoints now stack above and below the gradient instead of sitting beside it: centred on an 18 px bar they overflow ~4 px a side into the gap already reserved, so the reservation goes back to what it was and the fix is free. The rotated title joins the interior ladder in dropping — at phone width it has nowhere to go, and `box.title` already names the scale and its range. That test's "hide every tick and the title" assertion was the defect the audit reported, so it now states the new contract: the two extremes stay, everything else goes. Also reverts the DPR-change deferral. `render_smoke_nonumpy.py`'s `dprw` probe calls `_onDprChange()` and reads `dpr`/`canvas.width`/`chrome.width` on the very next line — a DPR change with no container resize has no later event to piggyback on — and routing it through `_queueResize` broke that contract. It also saved nothing: the ResizeObserver's queued pass already early-returns when width, height and dpr are all unchanged, and when the CSS size did change too, the second pass is doing real work at a new size. The dpr-baked width/radius rescale stays.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
The compact colorbar restack records each tick's beside-the-bar cssText on the node itself, but the tick was typed HTMLSpanElement by createElement, so tsc rejected the ad-hoc property and js/build.mjs failed before emitting a bundle. Every CI job that builds the render client failed on that one typecheck error. Annotate the local `any`, the same way the legend box's `lg` is annotated for its stashed row list.
Merging this PR will not alter performance
Performance Changes
Comparing 
Footnotes
|
…croll Two pytest regressions from the previous round, both from the same over-reach: the legend row was made a flex line with an ellipsizing label. A flex container blockifies its children's computed display, so an author's `inline-flex` swatch utility computed as `flex` and the Tailwind slot-cascade test failed. And `white-space:nowrap` removed the label wrapping that made a long legend in a narrow chart taller than its height cap, so the box no longer overflowed at all and the resize regression test's `legendHasOverflow` went false. Neither the flex row nor the ellipsis was needed for the reported bug. The sideways scrollbar came from grid columns that refused to shrink; with `minmax(0, max-content)` a long label wraps inside its column, which fixes the scrollbar without dropping text, without touching the swatch's formatting context, and while keeping the wrapping the narrow-chart contract depends on. The row keeps its full name in title/ARIA. The chrome stylesheet is byte-identical to its state before this branch again. Also fix a buffer-shrinking hazard the dpr rescale introduced: it re-uploads whole buffers from _cpuStyle/_cpuRadius, but the streaming-append fast path extends styleBuf on the GPU and advances n without growing those mirrors, so after an append the mirror was short — re-uploading it would shrink the store out from under the appended rows, and scaling it would leave that tail at the old dpr regardless. Such a record is now skipped with its dpr stamp left stale, which is exactly what makes the append guard fall back to the rebuild that renormalizes every row (scripts/append_stream_smoke.py's dprChangeRebuilds).
scripts/render_smoke_nonumpy.py timed out at 120 s on both Test jobs for 71c2ba6, with no failing assertion — chromium simply had not finished. The same step measured 98.4 s on the previous commit (18:05:00 to 18:06:38), so the probe had 22 s of margin against a software rasterizer on a shared runner, and a slightly slower runner spends it. It is by far the widest page in the tree: every mark family, LOD drill-in, picking, box select, the modebar, context loss and recovery, and the DPR watch, all through SwiftShader. append_stream_smoke.py already allows 180 s for much less. 300 s keeps a genuine hang failing while jitter no longer does. Also correct the note on the synchronous-DPR test: deferring _onDprChange would have made the dprw probe read a stale dpr and fail its assertion. It was not the cause of the earlier timeout, which was this same marginal cap.
Alek99
added a commit
that referenced
this pull request
Jul 30, 2026
…tocol v12) (#370) * Add a polar coordinate system (protocol v11) Polar is one coordinate system, not a family of chart types — which is how both incumbents work: matplotlib has a PolarAxes projection that ordinary plot/scatter/bar/fill calls render into, and Plotly composes radar, spider and wind roses out of three polar traces. So this adds a coordinate system and lets the existing mark registry render through it. MARK_KINDS gains no entries and no _emit_<K> is rewritten. `xy.polar_chart(...)` sets a chart-level `coords: "polar"`; each mark's first channel becomes the angle and its second the radius, so `xy.line` and `xy.scatter` are reused verbatim. `xy.theta_axis(unit=, zero=, direction=)` and `xy.r_axis(...)` configure the two axes — sugar over the x/y axes rather than new axis ids, since four separate places require an id to start with 'x'/'y' and the interaction axis policies are built on that grammar. The (theta, r) -> pixel transform is specified normatively in spec/design/polar-axes.md §3 and implemented twice: once in GLSL (xyPolarPos) and once in Python (_svg._PolarProjection, which both exporters share). Prose does not bind those; tests/fixtures/polar_transform.json does. The fixtures are authored from the spec rather than generated from either implementation, and every case is checkable by inspection — theta=0 lands due right, zero="N" with clockwise puts 90 degrees due east. This is deliberately stronger than the existing tick-math arrangement, where 30_ticks.ts and its hand port in _svg.py are bound by nothing executable. Details worth knowing: - The y term differs by sign between the two implementations on purpose: screen space grows downward, GL clip space grows upward. A mirrored chart is otherwise entirely plausible-looking, so a fixture pins it. - All three point-family shaders are transformed, not just POINT_VS. Scatter silently switches to POINT_SIMPLE_VS on its fast path, and PICK_VS feeds the hover hit test — untransformed, the picture stays right while hover reports the wrong row. - _PolarProjection reports affine=False. Several emitters bake a straight-line data->pixel map into Rust behind `sx.affine and sy.affine`, which a polar chart on linear axes would otherwise satisfy while being non-affine. - Data lines are chords between projected points (Plotly semantics), which is what makes radar edges straight; grid rings and the frame are true arcs. SVG draws rings as <circle>, and the raster path flattens the same rings to polylines because its display list has no arc opcode. - Angular ticks need their own ladder: niceStep's [1, 2, 2.5, 5, 10] cannot reach 15/30/45/90, so degrees came out as 0/50/100/150. - The radial axis starts at the centre and the angular axis spans a full turn. An autoscaled radial axis puts the smallest datum at the centre; an autoscaled angular axis puts spokes at arbitrary angles. Scope is line and scatter. Every other kind is refused at build time with an error naming the supported set, rather than approximated — the rect, area, segment and mesh shaders expand geometry in pixel space after the coordinate map, so under polar they would draw chord-edged shapes where arcs belong. Polar traces ship tier="direct": M4 buckets on a monotonic screen-x column, which a spiral is not, and density binning in (theta, r) distorts by area near the origin. Protocol 10 -> 11: a v10 client ignores `coords` and draws the columns as cartesian x/y, so it must reject the payload rather than render a plausible wrong picture. * Polar area/radar and polar bars/wind rose Extends the polar coordinate system to two more mark families, both of which Plotly composes rather than implements: radar/spider out of a filled Scatterpolar, and wind roses out of Barpolar. Area (and so radar): the quad interpolates in DATA space and projects the result, rather than interpolating already-projected clip coordinates, so the two radial edges run along true radii while the inner and outer edges come out as chords between projected corners — which is what makes a radar polygon's edges straight. `xy.radar_chart(categories, ...)` closes each series itself, because the seam is easy to get wrong: appending the first *angle* makes the closing segment sweep backwards through the whole circle, so the closing sample goes at a full turn instead. Spokes are labelled with the categories. Bars: a polar bar is an annular sector, which four corners cannot express. BAR_VS now sweeps a triangle strip of POLAR_BAR_SEGMENTS+1 vertex pairs across the bar's angular span; SVG draws the same wedge with real `A` arcs; the raster path flattens it, since its display list has no arc opcode. Subdivision is fixed rather than view-adaptive — bar counts are small, and a view-dependent count would have to be recorded per §28 rather than chosen silently. `xy.wind_rose(directions, speeds)` bins in Python (the arrangement `hist` already uses) and stacks polar bars, defaulting to the compass convention (zero="N", clockwise) that makes 90 degrees read as east. Band edges round to three significant figures, and the top edge rounds *up* so it still covers the fastest observation instead of putting "27.2197" in the legend next to "2.77". Two bugs this turned up, both found by comparing renderers rather than by tests: - The disc clip was reusing the id that also bounds every legend, so a legend sitting outside the circle vanished from the SVG while the raster still drew it. Marks now clip to the disc through a second id. - Angular tick labels called the angle formatter directly, so authored `tick_labels` — the category names on a radar chart — lost to pi notation. Both exporters now route through `_tick_text`, and `_fmt_axis` gained the angle branch its JS twin already had. Bars name their axis uniforms u_p/u_v rather than u_x/u_y, so `_drawBars` needed the polar uniform upload explicitly; without it a wind rose drew cartesian rectangles inside correct polar chrome. Also lands the start of pyplot `projection="polar"` — the Axes projection state, the PolarAxes method surface (set_theta_zero_location, set_theta_direction, set_rlim/rmin/rmax, set_rgrids, set_thetagrids), and `coords` flowing into the built chart. Sector limits raise NotImplementedError naming the spec rather than silently ignoring the call. Routing is not yet wired end to end. * Polar styling parity and a symmetric plot rect Styling audit fixes, all found by exercising knobs against all three renderers with distinctive values: - The polar tick-label writers in both static exporters read only the chart-level `tick_label` slot, never the axis's own tick_label_color/tick_color. That silently disabled the `text=False` and `show=False` shorthands (which work by setting tick_label_color transparent) and any explicit per-axis label colour — while the browser client honoured them. Both exporters now apply the same axis-first precedence the cartesian labels use, per axis, so the theta and r labels style independently. - The plot rect kept the cartesian tick-label gutters (L=76 R=38 T=36 B=66 on a 400x400 chart), which exist to hold edge-hugging labels a polar chart does not have — its labels ring the disc. The disc sat off-centre (cx=219) and smaller than it needed to be (r=143). `_inset_polar_plot` now gives those gutters back symmetrically: radius 143 -> 167 on a 400x400 chart, centred. Reservations that still mean something keep their room — the title band, a colorbar's right gutter, and the left gutter when the radial axis has a title (which is drawn there and would otherwise land at x = -10, off the canvas). - The client re-cuts its rect identically (`_recutPolarPlot`), and bumps its `_topAxisRoom` the same way the exporters bump `top_axis_room` — without that the figure title anchored onto the topmost angular label. Regression tests pin the per-axis independence (ring colour vs spoke colour, theta text=False vs r text=False) in both SVG attributes and raster pixels. * Bind the GLSL polar transform to the fixtures (polar_parity_smoke) tests/test_polar_transform.py and the fixture file both named scripts/polar_parity_smoke.py as the consumer that binds the client's GLSL to the shared fixtures — but the script did not exist, so the two-implementation contract was only half enforced: the Python projection was fixture-bound while xyPolarPos was verified by eye. The probe renders one single-point scatter trace per fixture sample, each in a unique saturated colour, reads the pixels back, and compares each colour's centroid to the fixture value within 1.5 px. Two details make it exact: - The fixture stores positions for its authored plot rect; the client computes its own. They reconcile without a third copy of the transform because a point's offset from the centre in units of the disc radius is rect-independent — pure arithmetic on fixture data rescales it to the runtime canvas. - Points sit at 60% of each fixture radius so no sprite clips at the canvas edge; a half-clipped disc's centroid shifts inward, which would read as a transform error. Under a linear radial scale the rescale stays exact. Scatter colour rides the channel dict (`t.color.color`), not `style.color` — the probe's first version set only the style and every point rendered in the palette fallback, which is worth a comment because hand-rolled specs will hit it again. Runs in the stdlib-only CI lane next to the other Chromium smokes. * Bring polar spec and roadmap current with the shipped scope CLAUDE.md treats a change as incomplete while its spec is stale, and three increments had outrun the design doc: the shader table still called AREA_VS and bars future, §7 still said line+scatter only, and the deferred table still listed area/radar and bars/wind rose. Also records the plot-rect re-cut, the POLAR_BAR_SEGMENTS contract, the radar full-turn closure rule, and the parity probe's name now that it exists. Roadmap items 18/27/29/32/34 move to their shipped statuses. * Polar audit round: styling parity, zoom semantics, and structure cleanups Two parallel audits (styling/customizability across the three renderers, and structure/abstraction against the repo's conventions) plus a reported zoom problem. Findings were verified by reproducing each one before fixing. Interaction — the reported problem. Polar inherited the full cartesian gesture set, so a drag panned the theta range and rescaled the disc as if the chart were rectilinear, and wheel zoom anchored at the cursor's screen HEIGHT rather than its radius. Polar now resolves its own axis policy: pan disabled, zoom radial-only, and box-zoom/select/brush/crosshair off (a screen rectangle neither matches a (theta, r) region nor reads as one — polar-axes.md §8). The wheel anchor is the cursor's normalized radius, and xyPolarPos returns NaN below the radial minimum so zoom-in culls those points instead of reflecting them through the centre. Renderer parity (each was: one renderer honoured it, another silently did not): - A colormapped or size-channelled polar scatter took a SECOND Rust affine fast path — `affine_channel_points` — that projected (theta, r) as cartesian x/y: a diagonal line of points outside the frame ring. All six such gates now go through one `affine_fast_path(sx, sy, polar)` predicate rather than a `polar is None` conjunct repeated per site, which is how this one was missed. - Cartesian edge tick marks leaked into polar in SVG and the client (the raster drew none). Guarded in both; tick_length/width/direction are documented as ignored under polar rather than half-drawn. - `curve="smooth"` was honoured by the client and skipped by both exporters — two visibly different shapes from one chart. The client now chords too, for the reason the exporters already did. - A constant `base=` on polar bars was dropped by the client (full pie slices from the centre) because the cartesian baseline uniform is clip-space. Bars now carry a data-space baseline uniform. - PDF export of ANY polar chart raised: the converter's clip subset was rect-only. It now accepts a single <circle> clip, emitted as four Bezier quarter-arcs, and tolerates inert data-* markers. - `tick_label_strategy="off"` and `tick_label_angle` were ignored by the polar label writers in all three renderers. - radar_chart silently replaced its category spokes with numeric angles as soon as any theta_axis child was supplied; an authored axis now merges. - The theta-axis title was drawn below the canvas edge because the rect re-cut reclaimed the bottom gutter it lives in. Structure: - Restore @cached_measurements to render_raster. Inserting a helper above it had silently transplanted the decorator onto that helper, costing every raster export its text-measurement cache. - The polar label placement was copied between the two exporters despite a docstring claiming otherwise. Extracted `polar_tick_label_layout` in _svg.py returning renderer-neutral placements; each exporter keeps only its sink. - Hoisted the GLSL cartesian/polar dispatch, pasted verbatim into four shaders, into POLAR_XYPOS_GLSL. AREA_VS/BAR_VS stay separate on purpose — they interpolate in data space before projecting. - Fixed a dead guard in the rect re-cut (clamping before testing made the small-chart escape unreachable), renamed _inset_polar_plot to _recut_polar_plot to match its client twin, hoisted the client's duplicated rlabel/gap/compass constants, and corrected mirror comments that named symbols which do not exist (xyPolar, THETA_ZERO "in 30_ticks.ts"). - POLAR_DIRECT_CEILING was a constant the spec advertised and nothing enforced; polar traces past it now refuse with the reason. - radar_chart's `fill=` was a documented no-op: it now rebuilds area children as line outlines. Non-area/line marks and column-name values raise instead of failing anonymously inside numpy. Dropped a no-op setdefault in wind_rose and a docstring frozen at the line+scatter increment. * Polar layout round 2, radial-zoom bounds cull, and pyplot routing Interactive testing and the styling audit's layout dimension caught four more: - Zooming in drew marks past the outer ring into the rect corners (the GL canvas is the plot RECT; the disc clip only existed in the SVG exporter). xyPolarPos now culls rn > 1 the same way it culls rn < 0 — NaN position, which is the client's equivalent of the exporters' disc clipPath. Verified by pixel readback: zoomed to [0.59, 0.84], zero lit pixels outside the ring. - A horizontal colorbar hangs off the plot's bottom edge, so the rect re-cut extending the plot downward pushed it clean off the canvas. The bottom band is kept whole when a horizontal colorbar (or a theta title) claims it, in both the exporters and the client. - The re-cut's small-chart escape fell back to the cartesian rect, whose own 40px floor can exceed a tiny canvas — an 80x80 chart drew its circle out to x=86. Too-small charts now take the largest centred box the canvas itself allows. (Also fixes the guard that clamped before testing, making the escape unreachable.) - The angular label allowance was a fixed 30px, so authored radar category names ("EAST-NORTH-EAST") were hard-clipped at the canvas edge. The room is now measured from the widest authored label, capped so a pathological label shrinks the disc rather than erasing it. Generated angle text keeps the floor. Mirrored in the client. Also finishes the pyplot polar routing: set_theta_zero_location / set_theta_direction / set_theta_offset collected into _polar_options which nothing read — the write-only state the structure review flagged. The options now land on the built figure's theta axis, so subplot(projection="polar") + compass conventions render correctly end to end. * Clip polar fills and bars at the radial range instead of culling them Interactive testing found the NaN cull was the wrong tool for filled marks. Culling is right for a point or a line vertex — there is no honest position outside the range — but a fill or a bar has an EXTENT, and its visible extent at a given angle is [base, top] intersected with [r_lo, r_hi]. Culling on one out-of-range endpoint threw the whole primitive away: - a radar polygon vanished the moment radial zoom lifted r_lo above its baseline, because every quad's inner corner went NaN; - a wind-rose bar disappeared whole as soon as its tip crossed the outer ring, rather than clipping at the ring the way matplotlib and Plotly do. AREA_VS and BAR_VS now clamp their radial span into the visible annulus, and a span entirely outside collapses to zero and draws nothing. Both static exporters clamp identically — the wedge builders bound outer and inner radius (the raster has no disc clip to save it), and the area emitters clamp their r columns before projecting, which also stops a below-minimum baseline mirroring through the centre INSIDE the disc where the SVG clip cannot catch it. Radial zoom now anchors at the CENTRE rather than the cursor's radius. The cursor anchor was the natural reading of "anchor where you point", but on a disc it lifts r_lo and carves a hole in the middle — an annulus view that reads as broken rather than as zoom, and only looked right when the pointer happened to be dead centre. Scaling the maximum about a fixed minimum is Plotly's radial semantics and stays legible from any cursor position. Applied at _zoomAt, so the wheel, the modebar buttons and axis-band gestures agree. * Polar sectors for unequal slice widths, and polar-aware point annotations Probing customizability by rebuilding four ECharts-style donut/gauge designs (evilcharts.com pie blocks) turned up the two things that stood between the polar system and a pie chart. Unequal angular widths. A bar's width may vary per bar; equal widths take the compact path (one scalar width) while unequal ones ship four edge columns — which under polar ARE an annular sector: (x0, x1) is the angular span and (y0, y1) the radial one. That path was not polar-capable, so a donut drew as cartesian rectangles inside polar chrome. RECT_VS now sweeps a sector exactly as BAR_VS does, and both exporters build the same wedge (SVG real arcs, raster flattened). This is what makes pie/donut a composition rather than a chart type: a slice is one bar carrying its own width. Point-anchored annotations. Centre text is `(any angle, r = 0)`, and the separable scales read that as the bottom-left corner — a donut's centre label landed outside the disc. `text`, `label`, `marker` and `arrow` now project jointly through the transform in both exporters. `rule` and `band` deliberately do not: a theta rule is a spoke, an r rule is a ring, a band is an annulus or a sector, and drawing them as straight cartesian bars would be a wrong picture rather than a missing one. Recorded in the spec's deferred table along with sector layout — a gauge drawn as a partial arc still gets a full-circle plot rect, so the unused portion is dead space. All four reference blocks now reproduce: a 6-slice donut with a 52% hole, 3° padding and centre text; dotted progress rings (40 sectors, 85-92% band); a revenue donut with a 62% hole and right-hand legend; and a -30°..210° gauge with four colour bands. Build script and a side-by-side page live outside the repo, under /tmp/evil. * Format the polar test additions CI's ruff format gate caught tests/test_polar_charts.py: the last test block was appended after the formatting pass, and only ruff check ran before the commit. No behaviour change. * Complete pyplot polar projection coverage * Cull out-of-range radii in exporter line/scatter, and two polar label/arc fixes Review fixes for three confirmed defects: - Out-of-range radial data was neither culled nor clamped by the exporters' line and scatter paths. Below r_lo a point normalizes negative and mirrors through the centre to a position INSIDE the disc — no clip can hide it — and above r_hi the raster path (which has no disc clip) drew past the outer ring. The client shader NaN-culls both. _PolarProjection.visible_mask now applies the same predicate (same 1e-6 epsilon) in both exporters: scatter drops the rows, lines split into visible runs so a chord with a culled endpoint is dropped whole in every renderer. Spec §8 updated — it previously claimed the exports clip at the ring, which only SVG's above-range half actually delivered — and now also records the fill/bar clamp semantics it never stated. - A full-turn sector (a 100% donut slice) rendered as nothing in SVG: the A arc's endpoints coincide and SVG omits such segments entirely. Spans of a full turn or more now draw each circle as two half-turn arcs, the inner ring wound oppositely so nonzero fill keeps the hole open. The raster polygon and the BAR_VS sweep were already correct. - _fmt_angle/fmtAngle hardcoded degree precision to a step of 1, so an authored 22.5-degree grid labelled itself 22deg/68deg (round-half-even). The tick step now threads through from fmtAxis/_fmt_axis on both sides. * Antialias polar wedges with an annular-sector SDF Radial bar edges rendered jagged in the client: the GL context runs with antialias: false, so every smooth edge is fragment-shader coverage — and the polar wedge branch switched the rect SDF off (v_half = 1e6), leaving all four edges of every wedge hard-aliased. Wide slices additionally showed the 24-segment arc flattening as visible facets. POLAR_WEDGE_GLSL now places the strip vertices XY_POLAR_AA px outside the true sector (computed from the same uniforms as xyPolarPos, in pixel form, because xyPolarPos's rn > 1 cull would eat the expanded outer vertices), and RECT_FS trims the expansion back against the true annular-sector SDF in device px. The fringe gets room to ramp on both sides of each edge, and because the expanded chords stay outside the true outer arc the trimmed arc is exactly round rather than faceted. Collapsed spans (radial clamp, zero width) cull explicitly — the expansion would otherwise leave a ghost sliver where nothing drew before. A full turn skips angular expansion and angular coverage: its two ends are one seam, not edges. POLAR_BAR_SEGMENTS rises 24 -> 96, sized so a full-turn wedge's chord sagitta stays inside the expansion up to a ~1400-device-px disc; the raster flattens the same count through its coverage-scanline fill, and SVG's real arcs never needed one. Verified: polar GLSL parity smoke green, full suite 3,541 passing, and pixel-level headless-Chrome crops of a wind rose, a 100% donut ring and a 90-degree slice at dpr 1 and 2; cartesian bars (shared RECT_FS) unchanged. render_smoke_nonumpy times out on this machine on the unmodified HEAD bundle too — pre-existing local SwiftShader issue, covered by CI. * Close four polar customizability gaps found rebuilding the ECharts pie blocks Rebuilding evilcharts' four ECharts pie blocks (market-share donut, dotted progress rings, gradient revenue donut, banded gauge) as a customizability probe surfaced four defects. All four were silent — every block built and exported without a warning. - The client projected NO annotation through the polar transform: js/src/51_annotations.ts had zero polar references and routed every kind through the separable _dataPxX/_dataPxY. Both exporters place them correctly, so the browser strung a donut's slice labels out in a horizontal row in theta order and dropped centre text at the left edge. This is the divergence the coordinate system exists to prevent, and the fix had landed only on the export side. A new _dataPxPoint mirrors the exporters' point() helper for text/label/marker/arrow/callout and the authored-scatter glyph pass; rule/band stay cartesian and deferred, as they are in Python. - `padding=` was discarded under polar. _recut_polar_plot symmetrised the authored gutters away and gave the disc the whole canvas, so the band every donut composition reserves for its legend or caption vanished — measured on one 400x420 chart, cartesian moved the plot bottom 384 -> 280 while polar moved 383 -> 380. An authored box is now only inset by the label room. - A gradient `fill=` reached the SVG and the browser but the raster painted it flat: the polar branches called cmd.fill(poly, flat) and never consulted style["fill"] the way the cartesian path does. - `corner_radius` was accepted, shipped on the wire as style.corner_radius, and ignored by all three renderers — a silent approximation. It is now defined in the unrolled (arc, radial) frame, where a wedge is a rectangle and the standard rounded-rect profile applies: the client evaluates it in the annular-sector SDF, the exporters sample the same profile through _rounded_wedge_points. Plain wedges keep their exact A arcs. Three of the four blocks depend on this (6px dots, 12px slices, 10px bands). Wedge strokes in the client fall out of the same SDF, which now carries the stroke ring the rectangle path always had. Suite 3,547 passing, polar GLSL parity smoke green, ruff/ty clean. Remaining from the probe and unchanged here: sector layout (a 240-degree gauge still gets a full-circle rect), polar rule/band, and polar select. * Complete polar heatmap and axis depth * Fix radar_chart(fill=False), which crashed unconditionally `xy.radar_chart(..., fill=False)` — the documented switch for outline radars, and the whole "Lines Variant" family in the competitor gallery — raised `KeyError: 'width'` for every input. Swapping only `kind` from "area" to "line" handed `_apply_line` an area's prop dict, and the two marks do not share a vocabulary: an area carries line_color/line_width/line_opacity where a line carries color/width/opacity. `_radar_outline` now translates them, falling back to the fill colour when the stroke was never given one. The existing `test_radar_fill_false_outlines_instead_of_filling` passed throughout because it asserts on the child mark kinds and never builds the figure, so the crash sat one `.figure()` call beyond its reach. Found by rebuilding the evilcharts ECharts radar blocks. The raster marker/arrow/callout half of this fix landed independently in 3d41a74f. Suite 3,547 passing, ruff/ty clean. * Lead the tooltip with the series name, and speak polar on polar charts Hovering a donut slice reported "x: 102.6, y: 0.94" for a slice whose whole identity is "Cloudpeak $13B". Three separate reasons, all fixed here. - The default readout never showed the series name. The hover row carries `trace` (an id) and \_defaultTooltipItems emitted only x/y/color/size, so a mark was identified purely by its coordinates. It now leads with the trace's name when it has one, which is what every comparable library does and the only label that means anything on a pie. - Under polar the channels were still called x and y. They are now theta and r (an explicit `labels=` override still wins). - Angular values were raw numbers: "x: 1.5708" on a radar spoke the chart itself labels "power", and no degree sign anywhere. The angular value now goes through the axis's own text function, so degrees read "338deg", radians read as pi-fractions, and an authored tick label wins outright. Authored labels match with a tolerance of an eighth of the spoke spacing: the hovered angle arrives as decoded offset-encoded f32 while the tick was authored in f64, so pi/2 missed its own label by ~1e-7 and fell back to a number. Verified by hovering real charts in headless Chrome across polar line, scatter, area, polar bar, radar and a six-slice donut; cartesian readouts are unchanged apart from now naming their series. Suite 3,597 passing, polar GLSL parity smoke green, ruff clean. * Keep the polar clip out of the per-pixel blend hot path CodSpeed flagged test_png_export_line_pyplot at -16.84% (34 -> 40.9 ms sim) on this branch — a cartesian benchmark regressed by polar work. The annular- sector clip (OP_POLAR_CLIP) was applied inside Canvas::blend_u8 and Surface::blend_u8 by calling apply_polar_clip(self.polar_clip, ..), which passes the ~40-byte Option<PolarClip> BY VALUE — one struct copy per blended pixel, for every mark on every chart, polar or not. Local interleaved A/B on the benchmark's own workload (100k-point pyplot line, PNG): main 1.58-1.62 ms, branch 1.89-1.92 ms (+18%, matching the sim number), across six alternating-order rounds. The hot path now tests only the Option discriminant — one byte load and a branch — and the clipped blend is outlined behind #[cold]/#[inline(never)], so only pixels painted while a polar clip is actually active take the call. Fixed dylib: 1.61-1.67 ms on the same interleaved harness, recovering ~90% of the regression; the residual byte-test is the price of the clip existing at all short of monomorphizing every painter loop. Output is byte-identical before/after on both a cartesian export and a polar wedge chart (sha256-verified), the Rust polar-clip parity tests pass, and the Python suite is 3,597 passing. Diagnosis note for the next reader: the branch adds only ~1% Python calls to this benchmark (5,828 -> 5,884 profiled calls); the regression was entirely native. A local checkout that had an unpushed emit_tick_labels optimization masqueraded as "main" during the first bisect — measure baselines from origin/main, not from whatever a sibling worktree happens to have. * Fix the eight confirmed review majors: exporters, pyplot rlim, wheel zoom Every major from the PR review reproduced before fixing; each fix carries a regression test asserting the behaviour the review named. Exporters: - Reversed radial axes dropped every wedge from SVG/PNG: norm_radius is decreasing under reverse, so taking the normalized endpoints positionally made outer <= inner in both shared wedge functions while the shader (which min/maxes) kept drawing. The normalized fractions are now ordered before clamping. - PDF export crashed for any polar chart with a hole or sector: those clips are a single <path> clipPath, which the converter refused. It now lowers the parsed path (arcs already cubic) to PDF clip ops, with SVG's clip-rule mapped onto W/W* so the annular hole stays a hole. - The raster polar area branch only clamped radii; it now applies the same position mask as SVG's _curve_path and the shader, splitting the fill into visible runs — no more chords across a sector boundary, and NaN no longer reaches the display list (§19). Ranges: - Constant-radius data bypassed the centre-origin default via the singleton early-return: line(theta, [5,5,5,5]) resolved to [4.75, 5.25] and drew a unit circle as a ring floating mid-disc. The singleton path now applies the same centre-origin/no-pad rule as the main polar branch. One existing cross-renderer test placed its marks via the old padded band and needed an explicit domain to keep its negative probe on empty canvas. pyplot: - set_rmax/set_rmin/getters snapshotted the cartesian-PADDED preview: set_rmax(2.0) shipped [0.85, 2.0] where matplotlib gives [0, 2]. The radial auto-domain preview now matches the engine's polar autorange. - set_rlim accepts matplotlib's documented rmin/rmax keywords, and refuses unknown keywords by name instead of TypeErroring inside set_ylim. - plt.subplot(111, projection="polar") on a claimed slot bounced off the stale pre-polar NotImplementedError in Axes.set; both that path and activate_subplot now delegate to _set_projection, which stays loud for genuinely unsupported projections. Interaction (the review's "pan and zoom do not work", root-caused): - Radial wheel zoom was dead on arrival: polar disables pan/box/select, so its RESOLVED default drag tool is "none", and the wheel gate treated that identically to the user choosing the none tool from the modebar. The gate now distinguishes user opt-out (releases page scroll, unchanged) from a chart that simply has no drag tools. Verified with real WheelEvents in headless Chromium: r_hi scales, r_lo stays pinned at 0 — the §8 contract, now asserted by a browser test rather than after != before. - Bar/rect hover was not seam-aware: |dataX - centre| in unwrapped data space missed a wind-rose "N" sector on its wrap side (|355 - 0| = 355). Both hovers now re-base through the same positive-mod the heatmap inverse uses; a browser test hovers both sides of the seam. - The radial zoom precision floor was defeated at r_lo = 0 (minSpan ~ 1e-42): the floor now scales with the interval magnitude, which also fixes centre-anchored cartesian zoom on symmetric ranges. Sustained deep zoom ultimately needs §16-style radial re-centering, recorded in the deferred table rather than half-built. Spec currency: §6's shader table now says RECT_VS is polar-capable (it is — it sweeps the unequal-slice path) and stops listing error bands under AREA_VS; the PDF hole/sector capability is recorded in §6; §8 documents the wheel-with-no-drag-tool rule; the deferred table gains the radial re-centering row. Suite 3,608 passing, polar GLSL parity smoke green, ruff clean. * Let hover values reach pi-fraction angle labels Hover values arrive f32-decoded (§4/§16), so theta = pi/2 comes back ~2e-8 off its f64 self — and fmtAngle's pi-fraction match used a 1e-9 gate, so the tooltip on a radians chart said "1.57" while the tick at the same spoke said "pi/2". Loosened to 1e-6 in both mirrors (js/src/30_ticks.ts and python/xy/_svg.py); no real tick sits within 1e-6 of a pi-fraction without being one. Verified by hovering every polar chart type in Chromium at this head: radians line/scatter/area (name + pi-fraction + r), degrees bar (theta with the degree sign), radar (authored spoke label, "power" not 1.5708), wind rose, 90-degree sector, donut with hole, and the polar heatmap (theta/r plus the color value). Two browser tests pin the tooltip content pipeline — the coverage gap the review named. * xy.pie_chart: a pie whose tooltip shows the category and value, not theta A pie encodes its value in ANGULAR WIDTH, so the generic bar readout has nothing meaningful to print for a slice: theta is layout, the radius is the constant rim, and the category lives only in the mark's name. Hovering the Market Share donut said "theta: 257, r: 1" under the name — truthful and useless. Rather than teach the tooltip machinery to guess when a wedge is a pie, the pie becomes a first-class composition that owns its readout: - xy.pie_chart(labels, values, *, hole=0.55, pad=3.0, colors=None, corner_radius=6.0, show_values=True, show_percent=True) composes polar wedge bars — hole=0 for a full pie — and names each slice "label value (share%)", which is also what the legend shows (the same convention the reference designs use). - The composition passes tooltip(title="{name}"), so hovering a slice shows exactly that one line. A user-supplied xy.tooltip child still wins. - "{name}" is a new tooltip-template pseudo-field resolving to the hovered trace's series name — generally useful for any composition whose category lives in the mark name (wind-rose bands, gauge segments). Verified by real hovers in Chromium on the donut and a hole=0 pie: the tooltip reads "Streamforge 12 (12%)" / "Affiliate 16300 (12%)" — no theta, no r. Refusals for mismatched lengths, negative values, zero totals, and holes outside [0, 1). Suite 3,616 passing, ruff clean, roadmap rows 6/14 updated. * Polar tooltips show values, not angles; free wheel/reset from dragMode Four audit findings, all reproduced first. P1 — wheel zoom and double-click reset were dead on every default polar chart. Polar resolves to dragMode "none" because no drag tool applies, and the dblclick handler bailed on that unconditionally. The wheel handler already drew the right distinction (only a USER-CHOSEN "none" releases navigation, since that is the modebar's page-scroll escape hatch); the dblclick handler now draws the same one. Examples that hide the modebar had no reset path at all. Verified in Chromium on polar line, wind rose and a pie: wheel takes r_hi 1.4 -> 0.48 and a double-click restores home exactly. P1 — wind rose bands were the WRONG SIZE, not merely mislabelled. `bar` measures its value as a height above `base`, so authoring `base + counts` stacked each band on its own cumulative offset a second time: three observations reached radius 5, and every band above the first was too thick. The height is the band's count. Two existing tests encoded the bug — one subtracted the bases back out so the error cancelled — and now assert the real semantics. P2 -> default — the numeric angle row is gone from every polar readout. On most polar charts the angle is where layout put the mark and the cursor is already sitting on it, so "theta: 72" answers a question nobody asked. What shows is the values: series name, radial value, colour/size. Two things survive because they are not numeric angles: an authored spoke label (a radar category reads "power"), and any row the user names explicitly via labels={"x": ...}, which opts the angle back in formatted through the axis's own text function. Compositions that genuinely need the angle now say so. A wind rose reads "<= 15.6 / direction 45 deg / count 38" — the bearing IS its data. A pie or gauge band reads its label alone, and a named single-wedge trace suppresses theta/r generally, since one named wedge's angle and radius are layout. Suite 3,619 passing, parity smoke green, ruff clean. Verified visually across pie, wind rose, gauge, polar line and radar. Not addressed here (docs, not engine): the two fixed-grid radial-bar demos that overflow at 375px, responsive legend placement, mobile anchor offsets under the sticky header, and pie slice keyboard traversal. * Even wedge spacing, wedge share in the tooltip, and three audit P1s The pie's seam narrowed toward the centre because the gap was a constant ANGLE: its width is r*dtheta, so it was 35 px at the rim and 20 px inboard, tapering to nothing at the hole. Measured on a five-slice donut before the fix: 19.8 / 26.8 / 34.2 px at r = 0.42 / 0.55 / 0.68. A gap is a LENGTH, so the angular inset has to grow as the radius shrinks — the same construction as d3's padAngle/padRadius pair. After: 10.6 / 10.4 / 10.1 px. New `wedge-gap` style property (px), threaded through bar/column and applied in one place per renderer — the unrolled (arc, radial) frame the corner radius already uses, where it is a single subtraction from the arc half-width. Both exporters keep exact `A` arcs: only the endpoints move inward and the radial edges become straight offset lines, which `L` draws. Registered in xy.styling.capabilities, matrix regenerated. `pie_chart(pad=...)` is now px, and each slice ships its FULL angular span, so the wire carries true shares. A named single wedge now reports its share: a gauge band reads "At risk / share 45.0%", which is exactly 0-450 of 1000, because the denominator is the span the wedges cover between them rather than the axis range — four bands sweeping 240 degrees of a full-turn axis must total 100% of the gauge, not 67% of a circle. Skipped when the name already carries a percentage, so a pie slice never reads "40% ... 40%". From the continued audit, the three findings that were real on this branch: - Modebar zoom moved the radial minimum. `_zoomBy` anchored at 0.5 for every coordinate system while the wheel path anchors polar radial zoom at r_lo; the documented fixed-minimum contract (polar-axes.md §8) only held for the gesture that was itself unreachable until the previous commit. Verified: 0..48 -> 0..24 with r_lo fixed. - radar_chart ignored an authored `theta_axis(unit="degrees")`: spokes were always generated in radians, so 0..2pi samples sat in a 0..360 frame and the whole radar compressed into the first 6.28 degrees. Spokes now follow the declared unit. Already fixed earlier on this branch, and re-verified rather than assumed — the audit was run against a different checkout (PR #374's worktree): - The wind-rose double-add (P0). A 300-observation rose renders its largest sector at 48, which is the true count; the audit saw 77 -> 142. - Hover across the north seam. A band centred on 0 degrees and 40 wide is hit at 340/350/0/10/20 degrees and correctly missed at 30. Suite 3,623 passing, parity smoke green, ruff clean. Still open from the audit, none of it engine-side: keyboard traversal for polar bar/area marks, CSV export of angular width and radial base, a11y summary staleness after zoom, the two fixed-grid radial-bar demos at 375px, and responsive legend placement. * Document wedge_gap so the docs API tables stay complete CI's "Docs tests and lint" failed on test_documented_factories_describe_every_parameter with ('bar', 'wedge_gap'): the generated API tables are sourced from docstrings, so every parameter needs an Args: entry. `bar`, `column` and `wind_rose` (which gained *children_in for its tooltip override) were missing theirs. Checked by applying the same rule the test applies to every public factory in xy.__all__ — no factory has an undocumented parameter now. The docs test env is not installable locally (needs reflex_site_shared), so that check plus codespell over docs/ stand in for the job. Suite 3,623 passing, ruff check/format and the repo hooks clean. * Drain rAF synchronously in the polar wheel-zoom probe The probe raced the frame it was waiting for. Wheel deltas are coalesced and applied inside a requestAnimationFrame, but the probe read the radial range after a fixed 260 ms setTimeout. Headless probes run under --virtual-time-budget, which fast-forwards timers while rAF still needs a real frame, so the timed read could land before the zoom committed and report [0, 1.4] -> [0, 1.4]. It failed that way on both "Python 3.11 floor" and "Test (Rust + Python + JS)" while passing locally every time. Use the idiom the repo's other two wheel probes already use: stub requestAnimationFrame, dispatch, then drain the queue synchronously. No wall clock, no frame dependency. The assertion is unchanged and still live. Reintroducing the original gate (dragMode === "none" without the _dragModeUserSet distinction) fails the test with exactly the CI signature, and neutering the drain also fails it — confirming the synchronous drain, not an incidental real frame, is what commits the zoom. * Refuse polar secondary axes and non-linear theta; fix seam tick trimming Three defects from an external audit, each reproduced first. The audit's two headline claims (PDF path clips, constant-radius [-0.5, 0.5]) were generated against a pre-9d0abbee tree and are already fixed and pinned here; four more findings reproduce identically on origin/main and are not this PR's. Secondary axes. A polar figure accepted y_axis="y2", validated the binding as legitimate, then every renderer read only the primary pair: an overlapping secondary range drew pixel-identical to the primary, inviting the reader to decode it against a tick ladder it does not belong to, and a disjoint one culled the series away entirely — while the axis still got a straight Cartesian spine and title in the gutter of a disc. Payload build now rejects any axis id outside {x, y} under coords="polar", per the rule this method already states: a plausible wrong picture is worse than an error. Non-linear theta. theta_axis(type_="log"/"symlog") was accepted, serialized, and honoured by exactly one renderer — the client scales theta before projecting, the static exporters ignore the scale outright (their SVG is byte-identical across linear/log/symlog). The same datum landed at 4.000 rad in SVG and 0.602 rad in the browser. The angle must be linear; a log or symlog radial axis is untouched. Seam ticks. Tick trimming was linear while mark culling is modular, so a sector crossing 0/turn threw away every tick authored on the far side of it: sector=(-30, 30) plotted a data point at theta=20 while silently dropping the tick at 20. Both trims now use the same modular containment as _angular_value_visible_mask, mirrored in the client's _axisTicks. Verified by mutation: reverting the client's modular filter fails the new browser probe, and both refusals build cleanly before the change. Cartesian secondary axes and Cartesian tick trimming are unaffected, each with a test. Suite 3,632 passing (+9), hooks and ruff clean, spec updated. * Fix polar area path joins, wedge hover span, and two inert axis switches Four defects from CodeRabbit and a second audit pass, each reproduced first. Empty and split polar areas. _curve_path returned "" for a fully culled trace and the area join stitched that into " L Z" — malformed path data that also reached the PDF converter's _parse_path — while a trace splitting into several visible runs had its first top run stitched onto the base with a stray L. An empty vertex array separately reached the native poly-path builder, which rejects a zero-length buffer, so a log radial axis that annihilates every row and an all-NaN radar polygon raised instead of drawing nothing. Each visible run now closes against its own base, and an empty curve yields "". This is one root cause behind three separately reported symptoms. Wedge hover span. _rectHover measured a directional span, mod(x1 - x0, turn), while anchoring the offset at min(x0, x1). Both renderers draw the band as the direct unwrapped interval between the edges — GLSL takes abs(a1 - a0) with dir = a1 >= a0 ? 1 : -1, and wedge_angles takes min..max — so edge order carries no meaning and the two disagreed: a descending pair (350, 300) covered 300..610 instead of 300..350. Only the offset needs wrapping, so a seam-crossing bar whose edges are emitted unwrapped (-15..15) stays reachable from 355. Note the review's suggested shortest-arc fix was not taken: it breaks any wedge wider than 180 degrees, such as a dominant pie slice. The new test fails against both the original code and that suggestion. theta_axis(reverse=True) was accepted, rode the wire, and was ignored by every renderer — the same accepted-but-inert trap as a secondary axis. Refused, with a pointer to direction='clockwise'. r_axis(reverse=True) is honoured and untouched. wind_rose no longer leaks a raw NumPy TypeError about "a sequence of integers" for a fractional sector count. Suite 3,638 passing (+6), hooks and ruff clean, spec updated. * Fix the pie zero-slice crash and three silently-wrong polar inputs Third audit pass, 19 claims, each reproduced before acting. Nine of twelve groups were pre-existing on main or wrong about their baseline; these four are this PR's. pie_chart crashed at render on any zero-valued slice. The factory validates values as finite and non-negative, so 0 is legal input by its own gate, but a zero span reached bar(width=...) and died as "bar width must be positive" — an error from a layer below naming neither pie_chart nor the label, and a 500 at page render rather than an authoring error. A slice of no size draws nothing, so it is skipped. coords="cartesian" silently un-polared five helpers. `coords` is the only thing making them polar (Chart.kind is inert), so the keyword returned unlabelled rects with no axes and dropped any authored theta/r axis. Worse, it re-opened every refusal this PR added: _validate_coords returns early for a non-polar figure, so histogram marks, theta reverse and secondary radial axes all built cleanly through it. Verified no test, example, doc, script or pyplot call site passes coords to these five. A time angular axis shipped when inferred. The declared spelling theta_axis(type_="time") was already refused, but a datetime column resolved to kind="time" pinned to a fixed 0..2pi range — twelve consecutive days wrapped the disc billions of times under radian spoke labels, with no escape hatch since theta_axis(domain=) is aliased to sector. The check now uses the resolved kind and explains the mapping to write instead. A time radial axis is unaffected. bar/column/errorbar had fallen out of POLAR_DIRECT_CEILING — a regression inside this PR, not something the audit found: 8d887a2e narrowed the gate to {line, scatter, area} so heatmap/contour cell grids could exceed the point ceiling, and un-capped the three most expensive marks as collateral. A polar bar costs 2*(96+1) verts per wedge against a cartesian quad's 4, and a million of them built without a word. Also: radar's fill=False outline treated a legal line_width=0 as unset and substituted the 2.0 default, so asking for no outline gave the thickest one. Suite 3,647 passing (+9), hooks and ruff clean, spec updated. * Guard polar bar animation, pad negative radii, thin radial labels Three confirmed polar defects from the third audit pass. Polar bar update transitions rendered a hybrid matching neither old nor new data. BAR_VS mixes the transition into p/v0/v1 in clip space, but its polar branch needs data space and re-derives theta and both radii from the raw attributes, so only the scalar half-width survived: the wedge jumped to its destination angle on frame 0 while its angular width animated. polar-axes.md already defers polar animation, so the deferral is now guarded like its siblings — _prepareBarPositionInterpolation bails under polar and records "snap:polar-unsupported". Polar scatter and line interpolate correctly (they mix before projecting) and entrance grow still honours u_animationProgress. Negative radial autorange threw the pad away. min(0.0, lo) collapses to lo once the data goes negative, so four readings within 0.7% of each other resolved to [-100.8, -100.1] and drew as a full-disc star — the exact picture the adjacent comment says the branch exists to forbid. Centre origin is only meaningful when zero ends the range; below zero it is vacuous, so the ordinary padded extent stands. Non-negative data is byte-identical. Radial tick labels overlapped. They march along a 22.5-degree spoke, so their usable run is the annulus width projected onto it — about a fifth of the plot — while the tick request was sized off full plot height, and the polar path skips the collision pass that would thin them. Measured 6 overlapping pairs at 390px and 10 at 700px, now 0 at both. Note the first attempt thinned the tick list itself, which also thinned the GRID: rings and labels share one list, and a 520px disc dropped from three rings to two. Ring density stays tied to the plot; only the labels are strided. Also corrects the audit's framing: this is not a 390px effect — 700px was worse. Suite 3,651 passing (+4), hooks and ruff clean. * Return matplotlib's theta offset for a southern zero location get_theta_offset() read "S" out of the render tables, which spell it -pi/2. Matplotlib maps the compass to 0..2pi counter-clockwise from east, so it returns 3*pi/2 — the same angle, but a compat getter has to return matplotlib's number: `get_theta_offset() > 0` and a round-trip through set_theta_offset both break on the negative. Verified against matplotlib 3.11.0 directly; all eight compass points now agree to 1e-9. The render tables keep -pi/2. They are angle math, where the two are interchangeable, and they are mirrored across THETA_ZERO in _svg.py and 50_chartview.ts. Also normalizes an explicitly-authored radian offset into the same range. * Fix the polar audit round: buffer leak, mobile chrome, wedge cost (#380) Nine reported failures across the polar surface and the responsive chrome it shares with every other chart. **P1 — repeated data updates leaked GPU buffers.** Trace teardown walked a hand-kept list of geometry buffer names, so every rebuilt trace (a state-driven update, an append that could not patch in place, an animated spec swap) orphaned its style, direct-RGBA colour, stroke, corner-radius, LOD-blend and dashed-line-length buffers. All three teardown paths now read one shared TRACE_GPU_BUFFERS list, pinned against the build paths by a test so a new channel cannot reintroduce the leak. **P1/P2 — mobile Wind Rose chrome.** The browser wraps a long title while layout measured one line, so a compact title lost ~10 px off the canvas top. Titles now wrap at one shared width in all three renderers and the browser caps the element at that same width; single-line titles are byte-identical. A polar legend now reserves a gutter beside the disc — 96 px on the loc's side, or a 64 px band beneath at compact widths — instead of covering the north-east sectors and the outer radial label. An authored anchor or four-tuple padding still wins. **P2 — wedge vertex cost.** Subdivision is span-proportional: clamp(ceil(96 * |span| / turn), 2, 96) over the *authored* angular width, in every renderer. Sagitta is quadratic in the per-segment angle, so this holds the flattening bound while a 16-sector wind-rose bar costs 14 vertices instead of 194; a full turn still uses 96. **P2 — inert polar axis options.** theta_axis/r_axis refuse minor_tick_values, minor_style, tick_label_min_gap, tick_label_anchor and the collision spellings of tick_label_strategy, each naming the control that does work. pyplot's projection="polar" drops the same keywords instead, because every matplotlib Axes carries an rcParam minor style it never authored; recorded in the compat spec. theta_axis(format=) now wins over the built-in degree text, and an r_axis(margin=) is honoured instead of discarded. **P2 — radial time axes.** A time radius autoranges from its data instead of epoch zero, which had squeezed every modern instant into a hairline ring at the rim. The signed-radius contract (a position, never a mirrored direction) is now normative in the spec and in the r_axis docstring. **P2 — compact colorbars.** They keep their two extreme tick labels and the scale title; only the interior ladder and the text-free minor ticks drop. Hiding everything left an unlabelled gradient. **P2 — redundant frames.** Data animations use the view animation's 80 ms label cadence instead of rebuilding the whole tick-label DOM every frame, and force one settled rebuild at the end. A DPR change coalesces into a single resize frame and rescales the per-instance widths and corner radii that are baked in device pixels. **P2 — pie_chart.** Listed in the generated chart-factory API reference with the other polar compositions. A zero-width bar is now legal and draws nothing, like line_width=0, so a 0% progress ring or an empty aggregated category no longer dies with "bar width must be positive". * Fix the CI failures from #380, plus polar CodSpeed coverage and the pie legend (#387) Restores alek/polar-axes to green and adds polar benchmark coverage. Fixes the five test failures the merged polar audit round introduced: the triangle-mesh buffer-cleanup assertion, the compact-colorbar plot width, the two SVG legend truncations, and the pyplot import-line check. - Widen the polar legend gutter to 22% of the canvas width, clamped to 120-200 px and floored so all three renderers land on the same integer pixel. - Restack the compact colorbar's two extreme ticks above and below the gradient, so the reservation stays at GAP + THICKNESS + 8 and the plot keeps its width. - Type the colorbar tick node that carries the stashed beside-bar CSS, so tsc accepts it and js/build.mjs can emit a bundle again. - Skip the DPR rescale for a trace whose CPU style/radius mirror no longer spans every row on the GPU, which a streaming tail append leaves behind; the existing append-time rebuild renormalizes those instead. - Fix the pie legend's sideways scrollbar with shrinkable grid columns, and stop pie_chart printing the value and an identical share. - Add benchmarks/test_codspeed_polar.py (6 rows) and the polar_coordinate_system category, taking the CodSpeed suite to 109 rows, with a test that pins the count to the methodology spec. - Raise the widest render smoke's chromium timeout, which sat 22 s from its cap. * Bound the static legend to the polar gutter, not the plot rect The polar phase 6/7 CI step failed on symlog_origin: the native PNG had zero #0f766e pixels where it wanted at least ten. The teal in that case is a single scatter marker at 0.8 opacity, which never matched the exact-colour probe; the pixels the probe had always been counting were the legend swatch, and the legend had disappeared from the raster. Both exporters clip their legend to a rect so an oversized one ellipsizes instead of escaping the file — static files have no scrollbar to fall back on. The polar legend gutter, however, is reserved OUTSIDE the plot rect by construction, so bounding the legend to the plot rect does not shrink it, it deletes it. The SVG emitter already unioned the two boxes; the raster still clipped to the plot alone, so every polar figure with a legend exported a PNG without one while its SVG kept one. That went unnoticed because the gutter only arrived with #380 — before it, the legend sat inside the plot rect. The union is now `legend_clip_rect`, read by the SVG clipPath and the raster clip command alike, so the two cannot drift again. SVG output is byte-identical. Pinned three ways: the rect covers the gutter, a cartesian rect is still exactly the plot, and the PNG pixels carry the swatch — the last is the one that fails on the old code. Suite 3,706 passing (+3), hooks and ruff clean, and the full polar phase 6/7 smoke (browser, SVG and PNG) is green on all six cases. * Preserve polar path order, cover wind-rose bins, restore the drag escape hatch Five findings from the cubic review, each reproduced first. Polar paths kept the caller's sequence. Theta is the order marks are JOINED in, not a domain to be scanned, but Figure.line/area/error_band sorted x at ingest to satisfy the M4 precondition. An authored track of [350, 10, 30, 5] shipped as [5, 10, 30, 350] with the radii permuted to match, so a path crossing the 0/turn seam or doubling back was redrawn as an ascending-angle fan. Skipping the sort is safe under polar because polar forces tier="direct" — config.py already says as much: "M4 decimation buckets on a monotonic screen-x column, which a spiral is not". Cartesian still sorts. wind_rose dropped observations above an authored top edge. The default path rounds its top edge UP precisely so it covers the fastest observation; authored edges were taken as given, so speed_bins=[10, 20] counted 2 of 3 observations when one blew at 25 and the rose under-reported its own input with no warning. Non-covering and non-finite edges are now refused. The declarative drag escape hatch was broken by my own earlier fix. Freeing polar's wheel zoom gated on _dragModeUserSet, which only the modebar sets, so interaction_config(default_drag_action="none") — the documented opt-out that releases page scroll for embedded charts — still wheel-zoomed AND still called preventDefault, trapping the page. Both gates now ask whether `none` was REQUESTED, in either spelling. Probed: declared "none" no longer zooms and no longer prevents default; polar radial zoom and the modebar behaviour are unchanged (34 client tests). polar_bar_segments now mirrors the client's finite check on span. math.ceil raised ValueError on NaN and OverflowError on infinity where JS falls back to the full-turn count, so the renderers disagreed about a degenerate wedge — one drew it, the other crashed. The seven polar names are in the TYPE_CHECKING block, so `from xy import polar_chart` no longer resolves to Any and loses its signature. Suite 3,712 passing (+61), hooks and ruff clean. * Exempt a time radius from the centre-origin default Zero is the Unix epoch on a time axis, not a centre. Pinning a time radius there spanned the disc from 1970 to the data and parked every ring on the rim; a singleton instant was the worst case, resolving to [0.0, 1767225600000.0]. Time now takes the ordinary padded window, byte-identical to what the cartesian axis resolves for the same column. Both polar radial branches needed the guard — the singleton early return has its own copy of the centre-origin rule, so fixing only the general branch left the worst case untouched. Numeric radii, including constant and negative, are unchanged. * Mirror the polar label-room and legend-gutter fixes into the exporters The client half of both fixes was sitting uncommitted; the Python twins were never made, so each was a one-renderer fix — the divergence this coordinate system keeps having to be defended against. Both reproduce in the exporters: - `_polar_label_room` measured only `tick_labels`, but a category axis carries its names in `categories` (`axis_ticks` hands those straight to `_category_ticks`) and has no `tick_labels` at all. Long names therefore fell back to the uniform default and spilled over the disc. `radar_chart` hid this because it authors `tick_labels` explicitly; a categorical `polar_bar_chart` measured 30 px where it needed 90. - `_recut_polar_plot` returned early when the angular tick labels were hidden, which skipped `_polar_legend_reserve` as well as the label inset. The legend then fell back to the plain plot rect and drew over the marks, and the disc kept the cartesian gutters the recut exists to give back. Hiding the labels removes the LABEL inset only; track the flag and skip just that. Tests pin both, including the client source strings, so the two halves cannot drift apart again. * Refuse horizontal bars under polar `POLAR_MARK_KINDS` admits `bar`, and the compact bar payload happily serialized `orientation="horizontal"` for a polar trace — but every renderer reads the position column as theta and the value column as radius regardless, so the bar came out transposed rather than rotated. A polar bar's position IS its angle and its value IS its radius, so the orientation is not a free choice; refuse it rather than draw a plausible wrong picture (§28). Caught by review. Also corrects the §3 property count in the polar spec, which said "Five" over eight bullets after later increments added to the list.
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.
Follow-up to #380 (squash-merged as
7449360). Three things:alek/polar-axesis red and it's my fault, plus the CodSpeed coverage and the pie-legend fix that were written after the squash.1. The five CI failures —
alek/polar-axeswas green ata3e28bfand red at7449360All five are mine.
Two tests pinned code I moved.
test_tick_sides_bump_wire_protocol_and_client_in_lockstepasserted the exact00_headerimport line, which grew a third name. It now checks that the client importsPROTOCOL, not the spelling of the list.test_triangle_mesh_resource_cleanup_deletes_every_coordinate_bufferlooked for"x0Buf"…"y2Buf"inside_destroyTraceResources, where they no longer live. It reads the sharedTRACE_GPU_BUFFERSlist and asserts the teardown uses it. (Already written before the squash, just not in it.)Two were the polar legend gutter ellipsizing static labels. At a flat 96 px the exporters' legend had 44 px of label room, so
series onecame outseries...andOrganic 20 (33%)came outOrga.... The gutter is now 22% of the canvas width clamped to 120–200 px — still derived from the canvas rather than measured from the label set, so all three renderers reserve the identical box instead of drifting with their font metrics, andfloorrather thanroundso Python and JavaScript land on the same integer pixel. That gives 68 px of label room at the narrowest non-compact width and 146 px at the default export size; both failing labels fit with margin.One was the compact colorbar spending plot width. Reserving a side gutter for the endpoint tick labels cost 36 px, and
test_narrow_fluid_resize_stays_painted_and_preserves_plot_spaceguards exactly that (plotWidth >= 280,colorbarWidth == 18). The endpoints now stack above and below the gradient instead of sitting beside it: centred on an 18 px bar they overflow ~4 px a side into the gap already reserved, so the reservation goes back to what it was and the readability fix costs nothing. The rotated title joins the interior ladder in dropping — at phone width it has nowhere to go, andbox.titlealready names the scale and its range.That test's
hiddenChrome == chromeCountassertion was the defect the audit reported ("hide every numeric tick and the scale title, leaving an unlabeled gradient"), so it now states the new contract explicitly: the two extremes stay visible (visibleTicks == ["0", "1"]), the title hides, everything else drops. The width and plot-space assertions are unchanged — they're what keep the fix honest.Also reverted: the DPR-change deferral.
render_smoke_nonumpy.py'sdprwprobe calls_onDprChange()and readsdpr/canvas.width/chrome.widthon the very next line — a DPR change with no container resize has no later event to piggyback on — and routing it through_queueResizebroke that contract. It also saved nothing I can demonstrate:_resizealready early-returns when width, height and dpr are all unchanged, so the ResizeObserver's queued pass is not a redundant second frame; and when the CSS size did change too, that pass is doing real work at a new size. The dpr-baked width/radius rescale — the part that fixes actual wrong output — stays.2. CodSpeed coverage for the polar surface
CodSpeed reported "103 untouched benchmarks" for a change that rewrote wedge geometry in three renderers; that blind spot is why the ~50k-polar-bar cliff was found by hand.
benchmarks/test_codspeed_polar.pyadds six rows: payload prep for a polar line / wind rose / pie (the three materially different validation and emit paths), SVG vs native-PNG export of the same rose (bracketing the arc-flattening term, so a regression back to a flat subdivision count is a step change rather than a bug report), and a polar heatmap's bounded inverse raster.The report's other item — 2 benchmarks skipped, baseline reused — is orphan rows in the dashboard, not skipped tests: the suite collects exactly 103 rows and 103 were measured, so those two have no code behind them and need archiving there by hand.
test_codspeed_row_count_matches_the_methodology_specnow gates the collected total againstspec/benchmarks/methodology.md§8 (which was itself one row stale), so a renamed or deleted benchmark fails CI instead of becoming a permanent "skipped" row.3. The pie legend (reported here)
Both, and they share a cause.
--xy-legend-max-width, but its grid columns weremax-contentand refused to shrink, so an over-wide row overflowed andoverflow:autoanswered with a horizontal scrollbar — hiding the label it was meant to show. Columns areminmax(0, max-content)and the box scrolls on the block axis only; inline overflow ellipsizes per row, which is what the static exporters already do, with the full text intitle/ARIA. Only the label clips — the swatch isflex:noneand keepsoverflow:visible, so an authored oversized marker still draws outside its 18×14 box.pie_chartprinted the value and the share, and for values that already sum to 100 — how most pie data arrives — those are the same digits:[40, 30, 20, 10]renderedDirect 40 (40%). The share keeps the unit, so the bare value is dropped when it says nothing new, decided once for the whole pie so rows stay uniform (a legend where one row carries a bare value and the next doesn't is worse than either consistent shape). Zero slices draw no wedge and get no row, so they can't veto it. The doubled label was also what overflowed the box.legend_itemandlegend_labeljoin the:where()chrome layer so an author's utility class still wins, andtest_static_client_security.pyrequires both to stay defeatable.Verification
Ruff check and format clean worktree-wide. Still no local test execution — this sandbox has no egress to PyPI, npm or crates.io, so the native core and client bundle can't be built. Verified directly:
js/src, plus the multi-line concatenated ones checked individually;_legend_layout's own sizing (44 px of label room at 96 px vs 68/146 px now) and confirmed to fit;visibleTicks == ["0", "1"]derived by emulatingfmtGeneralover this colorbar'sticks=[0, 0.25, 0.5, 0.75, 1.0];floor(0.22·w)clamped, verified integer-stable across 100–3000 px so Python and JS cannot disagree;polar_bar_segmentsand_textblock.wrap_lines/measureexercised standalone.CI is the first place the full suite runs.
Generated by Claude Code