Skip to content

fix(plot): prevent crashes, hangs, and wrong-series rendering#27

Open
balloob wants to merge 1 commit into
OpenDisplay:mainfrom
balloob:fix/plot-correctness
Open

fix(plot): prevent crashes, hangs, and wrong-series rendering#27
balloob wants to merge 1 commit into
OpenDisplay:mainfrom
balloob:fix/plot-correctness

Conversation

@balloob

@balloob balloob commented Jul 4, 2026

Copy link
Copy Markdown

Summary

Correctness and robustness fixes for the plot and diagram elements. These address pathological inputs that can crash the render or block the Home Assistant event loop, plus a couple of wrong-output bugs. Each fix is small and self-contained.

Findings addressed

  • A1 — wrong series style. draw_plot skipped entities whose states were all invalid when building raw_data, then zipped raw_data against the full element["data"] config list. An unavailable first entity would shift the mapping, so a later series rendered with the wrong entity's color/width/smooth. Segments are now paired with their own config.
  • A2 — event-loop hang (DoS). The y-axis tick/grid/legend loops step from min_v to max_v by tick_every (default 1). One bogus sensor reading (a counter, a glitch to 2**32) makes the spread millions wide and each loop issues millions of draw calls, freezing the async loop. Tick spacing is now clamped so at most _MAX_Y_TICKS (100) ticks are drawn; reasonable configs are untouched.
  • A3 — ZeroDivisionError in diagram. The all-bars-zero guard set ctx.pos_y but was missing a return, so execution fell through to height_factor = (height - offset_lines) / max_val. Added the return.
  • A5 — 3-point smooth dropped the middle point. The Catmull-Rom boundary extension only ran for len(points) > 3, so a 3-point series collapsed to a straight first→last line (the peak vanished). Boundary extension now runs at >= 3 points.
  • A10 — documentation. Clarified in the docstring that low/high seed but do not pin the y-axis range (data still auto-expands it).
  • B6 — dotted grid perf. Dotted grid lines called draw.point() once per pixel; now batched into a single draw.point(seq) call per line.
  • B8 — smoothing perf. The four Catmull-Rom basis weights depend only on the step parameter and are constant across segments; they're now precomputed once per smoothing call instead of per interpolated point.

Tests

Adds regression tests for A1 (skipped-entity style leakage), A2 (huge spread renders without hanging), A3 (all-zero bars), and A5 (3-point smoothing keeps the middle). Full suite: no new failures (the pre-existing tests/visual/* byte-snapshot failures under Pillow 12 are unrelated and addressed separately).

🤖 Generated with Claude Code

Correctness and robustness fixes for the plot and diagram elements:

- A1: pair each entity's segments with its own config so an entity with no
  valid data no longer shifts the config/segment mapping and renders another
  series in the wrong color/width/smooth style.
- A2: clamp the y-axis tick spacing so the tick/grid/legend loops draw at most
  _MAX_Y_TICKS ticks. A single bogus sensor reading (huge value spread) could
  otherwise issue millions of draw calls and freeze the async event loop.
- A3: return after the all-zero-bars guard in draw_diagram, avoiding a
  ZeroDivisionError from the height_factor division that followed.
- A5: smooth a 3-point series through its middle point instead of collapsing it
  to a straight first-to-last line (boundary extension now runs at >= 3 points).
- A10: document that plot low/high seed but do not pin the y-axis range.
- B6: batch dotted grid lines into a single draw.point() call per line.
- B8: precompute the Catmull-Rom basis weights once per smoothing call instead
  of recomputing them for every interpolated point.

Adds regression tests for A1, A2, A3, and A5.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UVpLN4Rzdv7y3ud7QzutVJ
@balloob balloob requested a review from g4bri3lDev as a code owner July 4, 2026 07:13
@codecov

codecov Bot commented Jul 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.66667% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/odl_renderer/elements/visualizations.py 96.66% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant