From 9e68e14cca89cec61b126bc946876f356a1aaec3 Mon Sep 17 00:00:00 2001 From: Marc Sturlese Date: Fri, 4 Sep 2026 00:39:25 +0200 Subject: [PATCH 1/4] fix(charts): keep negative weekly hours inside the chart viewBox The weekly line chart floored its y-domain at zero, so a week with negative hours_saved -- which docs/metrics.md defines as normal (a rejected run earns -m minutes) -- mapped far below the 236px viewBox and was clipped away by the browser. The dashboard silently hid exactly the week an executive most needs to see. Derive the domain from both extremes so it always contains zero, wash the area back to the zero line rather than the frame floor, and draw the axis on zero. Non-negative data keeps bottom=0 and renders coordinate-for-coordinate as before. The terminal sparkline already handled this input; the HTML surface was the unfixed sibling. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01XjLWb6igee2tVTG7wg93Fx --- src/flightdeck/report/charts.py | 30 ++++++++++++++------ tests/test_charts.py | 49 +++++++++++++++++++++++++++++++-- 2 files changed, 69 insertions(+), 10 deletions(-) diff --git a/src/flightdeck/report/charts.py b/src/flightdeck/report/charts.py index bc1d84e..19025aa 100644 --- a/src/flightdeck/report/charts.py +++ b/src/flightdeck/report/charts.py @@ -49,17 +49,21 @@ def _money(value: float, unit: str) -> str: return f"{'−' if negative else ''}{unit}{_fmt(abs(value))}" -def _grid_and_axis(top: float, unit: str, plot_h: float) -> str: +def _grid_and_axis(top: float, unit: str, plot_h: float, bottom: float = 0.0) -> str: + """Gridlines across the domain [bottom, top]; the axis sits on the zero line. + With the default bottom=0 the zero line IS the plot floor, so a non-negative + chart renders byte-for-byte as before.""" + span = (top - bottom) or 1.0 parts = [] for index in range(1, 4): # 3 hairlines + baseline y = _PAD_T + plot_h * (1 - index / 3) - tick = top * index / 3 + tick = bottom + span * index / 3 parts.append(f'') parts.append( f'{_fmt(tick)}{unit}' ) - baseline_y = _PAD_T + plot_h - parts.append(f'') + zero_y = _PAD_T + plot_h * (1 - (0.0 - bottom) / span) + parts.append(f'') return "".join(parts) @@ -79,14 +83,24 @@ def line_chart(chart_id: str, labels: list[str], values: list[float], unit: str, return '

no data yet

' plot_h = _H - _PAD_T - _PAD_B plot_w = _W - _PAD_L - _PAD_R - top = _nice_top(max(values)) + # The domain must contain zero AND every value: hours_saved goes negative in a + # rejection-heavy week (docs/metrics.md — a rejected run earns −m minutes), and + # a domain floored at zero maps that week far below the viewBox, where the + # browser clips it and the card silently hides the bad news. Non-negative data + # keeps bottom=0, i.e. exactly the previous framing. + top = _nice_top(max(values)) if max(values) > 0 else 0.0 + bottom = -_nice_top(-min(values)) if min(values) < 0 else 0.0 + span = (top - bottom) or 1.0 n = len(values) xs = [_PAD_L + plot_w * (i + 0.5) / n for i in range(n)] - ys = [_PAD_T + plot_h * (1 - v / top) for v in values] + ys = [_PAD_T + plot_h * (1 - (v - bottom) / span) for v in values] baseline_y = _PAD_T + plot_h + zero_y = _PAD_T + plot_h * (1 - (0.0 - bottom) / span) line_path = "M" + " L".join(f"{x:.1f} {y:.1f}" for x, y in zip(xs, ys, strict=True)) - area_path = f"{line_path} L{xs[-1]:.1f} {baseline_y} L{xs[0]:.1f} {baseline_y} Z" + # The area washes back to zero, not to the frame floor, so a negative week + # reads as a dip below the axis instead of a full-height fill. + area_path = f"{line_path} L{xs[-1]:.1f} {zero_y:.1f} L{xs[0]:.1f} {zero_y:.1f} Z" hover = [] slot = plot_w / n @@ -100,7 +114,7 @@ def line_chart(chart_id: str, labels: list[str], values: list[float], unit: str, end_label = f"{_fmt(values[-1], 1)}{unit}" end_x = min(xs[-1] + 8, _W - _PAD_R - 4) return f""" -{_grid_and_axis(top, unit, plot_h)} +{_grid_and_axis(top, unit, plot_h, bottom)} diff --git a/tests/test_charts.py b/tests/test_charts.py index 372c9b0..c9d7132 100644 --- a/tests/test_charts.py +++ b/tests/test_charts.py @@ -1,7 +1,24 @@ -"""Chart rendering edge cases — cross-surface consistency of money labels.""" +"""Chart rendering edge cases — cross-surface consistency of money labels, and +the weekly line chart's numeric domain. + +hours_saved per week can be negative (docs/metrics.md: a rejected run earns −m +minutes), and that series feeds line_chart on the dashboard, so its y-domain +must reach below zero — the terminal sparkline already does (test_terminal.py). +""" + +import re from flightdeck.format import money -from flightdeck.report.charts import hbar_chart +from flightdeck.report.charts import _H, _PAD_B, _PAD_T, hbar_chart, line_chart + +# The band a point may legally occupy: outside it the browser clips against the +# viewBox and the value disappears from the page. +_BAND = (_PAD_T, _H - _PAD_B) + + +def _line_ys(svg: str) -> list[float]: + path = re.search(r'class="fd-line" d="([^"]+)"', svg).group(1) + return [float(y) for y in re.findall(r"[ML]-?[\d.]+ (-?[\d.]+)", path)] def test_hbar_negative_matches_the_shared_money_minus_glyph(): @@ -20,3 +37,31 @@ def test_hbar_negative_matches_the_shared_money_minus_glyph(): def test_hbar_positive_and_empty_render(): assert money(3000.0, "EUR") in hbar_chart([("Up", 3000)], "€") # "€3,000" assert "no data yet" in hbar_chart([], "€") + + +def test_line_chart_keeps_a_negative_week_inside_the_viewbox(): + # A rejection-heavy week is exactly the week an executive must see. Before the + # fix the domain was floored at zero, so -20 mapped to y=582 in a 236-tall + # viewBox: clipped away, and the dashboard quietly hid the bad news. + ys = _line_ys(line_chart("hours", ["W27", "W28", "W29"], [10.0, -20.0, 5.0], " h", "hours saved")) + + assert len(ys) == 3 + assert all(_BAND[0] <= y <= _BAND[1] for y in ys), ys + assert ys[1] > ys[0] and ys[1] > ys[2] # the negative week sits lowest (SVG y grows downward) + + +def test_line_chart_all_negative_stays_in_band_and_hangs_below_the_axis(): + svg = line_chart("hours", ["W27", "W28"], [-5.0, -10.0], " h", "hours saved") + ys = _line_ys(svg) + + assert all(_BAND[0] <= y <= _BAND[1] for y in ys), ys + # Every point is below the zero axis, which is now drawn at the top of the band. + zero_y = float(re.search(r'class="fd-axis"[^/]*y1="([\d.]+)"', svg).group(1)) + assert all(y > zero_y for y in ys) + + +def test_line_chart_non_negative_framing_is_unchanged(): + # Regression guard: the fix must not move a single coordinate on the normal + # path — the demo dashboard is CI's end-to-end golden. + assert _line_ys(line_chart("hours", ["a", "b", "c"], [1.0, 2.0, 3.0], " h", "s")) == [168.4, 130.8, 93.2] + assert _line_ys(line_chart("hours", ["a", "b"], [0.0, 0.0], " h", "s")) == [206.0, 206.0] From 678655fba2acaca3d54ae2dbdc110ca02506a34c Mon Sep 17 00:00:00 2001 From: Marc Sturlese Date: Fri, 4 Sep 2026 00:46:11 +0200 Subject: [PATCH 2/4] fix(charts): keep the zero line's integer form so spend charts stay identical Gate review: the first commit widened the y-domain correctly but reformatted the axis from "206" to "206.0", which also reached column_chart -- the AI-spend card, which has nothing to do with negative hours -- and made the docstring's "byte-for-byte as before" claim false on every non-negative input. Emit whole coordinates through _coord() so they keep their integer form. The non-negative path is now genuinely byte-identical: 0 diffs over 506 generated cases for both line_chart and column_chart, and the demo dashboard matches main exactly. The regression tests now pin the axis and area verbatim -- the two marks the fix actually rewrote -- instead of only the line coordinates. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01XjLWb6igee2tVTG7wg93Fx --- src/flightdeck/report/charts.py | 18 +++++++++++++----- tests/test_charts.py | 32 ++++++++++++++++++++++++++------ 2 files changed, 39 insertions(+), 11 deletions(-) diff --git a/src/flightdeck/report/charts.py b/src/flightdeck/report/charts.py index 19025aa..44613ce 100644 --- a/src/flightdeck/report/charts.py +++ b/src/flightdeck/report/charts.py @@ -40,6 +40,14 @@ def _fmt(value: float, decimals: int | None = None) -> str: return f"{value:,.{decimals}f}" +def _coord(value: float) -> str: + """A coordinate rendered at its natural precision: whole numbers keep their + integer form. The zero line lands exactly on the plot floor whenever the + domain floors at zero, and this keeps that emission identical to the + fixed-baseline output it replaced.""" + return str(int(value)) if float(value).is_integer() else f"{value:.1f}" + + def _money(value: float, unit: str) -> str: """A currency label that matches format.money's cross-surface contract: the U+2212 minus sits BEFORE the symbol (not an ASCII '€-5,000'), and a value that @@ -52,7 +60,7 @@ def _money(value: float, unit: str) -> str: def _grid_and_axis(top: float, unit: str, plot_h: float, bottom: float = 0.0) -> str: """Gridlines across the domain [bottom, top]; the axis sits on the zero line. With the default bottom=0 the zero line IS the plot floor, so a non-negative - chart renders byte-for-byte as before.""" + chart -- column_chart included -- renders byte-for-byte as before.""" span = (top - bottom) or 1.0 parts = [] for index in range(1, 4): # 3 hairlines + baseline @@ -62,8 +70,8 @@ def _grid_and_axis(top: float, unit: str, plot_h: float, bottom: float = 0.0) -> parts.append( f'{_fmt(tick)}{unit}' ) - zero_y = _PAD_T + plot_h * (1 - (0.0 - bottom) / span) - parts.append(f'') + zero = _coord(_PAD_T + plot_h * (1 - (0.0 - bottom) / span)) + parts.append(f'') return "".join(parts) @@ -95,12 +103,12 @@ def line_chart(chart_id: str, labels: list[str], values: list[float], unit: str, xs = [_PAD_L + plot_w * (i + 0.5) / n for i in range(n)] ys = [_PAD_T + plot_h * (1 - (v - bottom) / span) for v in values] baseline_y = _PAD_T + plot_h - zero_y = _PAD_T + plot_h * (1 - (0.0 - bottom) / span) + zero_y = _coord(_PAD_T + plot_h * (1 - (0.0 - bottom) / span)) line_path = "M" + " L".join(f"{x:.1f} {y:.1f}" for x, y in zip(xs, ys, strict=True)) # The area washes back to zero, not to the frame floor, so a negative week # reads as a dip below the axis instead of a full-height fill. - area_path = f"{line_path} L{xs[-1]:.1f} {zero_y:.1f} L{xs[0]:.1f} {zero_y:.1f} Z" + area_path = f"{line_path} L{xs[-1]:.1f} {zero_y} L{xs[0]:.1f} {zero_y} Z" hover = [] slot = plot_w / n diff --git a/tests/test_charts.py b/tests/test_charts.py index c9d7132..9b76b2f 100644 --- a/tests/test_charts.py +++ b/tests/test_charts.py @@ -9,7 +9,7 @@ import re from flightdeck.format import money -from flightdeck.report.charts import _H, _PAD_B, _PAD_T, hbar_chart, line_chart +from flightdeck.report.charts import _H, _PAD_B, _PAD_T, column_chart, hbar_chart, line_chart # The band a point may legally occupy: outside it the browser clips against the # viewBox and the value disappears from the page. @@ -60,8 +60,28 @@ def test_line_chart_all_negative_stays_in_band_and_hangs_below_the_axis(): assert all(y > zero_y for y in ys) -def test_line_chart_non_negative_framing_is_unchanged(): - # Regression guard: the fix must not move a single coordinate on the normal - # path — the demo dashboard is CI's end-to-end golden. - assert _line_ys(line_chart("hours", ["a", "b", "c"], [1.0, 2.0, 3.0], " h", "s")) == [168.4, 130.8, 93.2] - assert _line_ys(line_chart("hours", ["a", "b"], [0.0, 0.0], " h", "s")) == [206.0, 206.0] +def test_line_chart_non_negative_output_is_byte_identical(): + # Regression guard, pinned against the pre-fix output: widening the domain must + # not move — or reformat — anything on the non-negative path. The axis and the + # area are the two marks the fix actually rewrote, so assert those verbatim + # rather than only the line coordinates. + svg = line_chart("hours", ["a", "b", "c"], [1.0, 2.0, 3.0], " h", "s") + + assert _line_ys(svg) == [168.4, 130.8, 93.2] + assert '' in svg + assert '' in svg + + flat = line_chart("hours", ["a", "b"], [0.0, 0.0], " h", "s") + assert _line_ys(flat) == [206.0, 206.0] + assert '' in flat + + +def test_column_chart_is_untouched_by_the_line_chart_domain_fix(): + # column_chart shares _grid_and_axis with line_chart. It plots AI spend, which + # is never negative, so it must come out exactly as it did before the domain + # became two-sided — the fix has no business changing the spend card. + svg = column_chart(["a", "b", "c"], [10.0, 20.0, 30.0], "€", "AI spend") + + # The zero line keeps its integer form; the bars' own ":.1f" coordinates are + # untouched by the fix and stay as they were. + assert '' in svg From ab2d6d3e073fdb17329df399b8295f21b1cba56c Mon Sep 17 00:00:00 2001 From: Marc Sturlese Date: Fri, 4 Sep 2026 00:51:46 +0200 Subject: [PATCH 3/4] fix(charts): put zero on a labeled gridline when the domain goes negative Gate review, second round. Keeping the negative week inside the viewBox was only half the job: the ticks stayed anchored to thirds of the band while the axis moved to a value-derived position, so a mixed-sign chart showed a heavier unlabeled rule at an arbitrary height and three ticks none of which was zero. The reader could see the dip but not read how deep it went -- which is the number the fix exists to surface. Two smaller faults shared that root cause: adjacent ticks could both render "-0.1" (or "-0.0", a signed zero the project forbids twice), and zero's gridline could land on the axis, stacking two opaque strokes and costing the chart one of its three promised hairlines. Snap a two-sided domain to multiples of a nice step, so zero and the domain floor are always labeled gridlines; take tick precision from the step rather than the value, so neighbouring ticks stay distinguishable; skip the hairline under the axis; and normalize a signed zero away in _fmt, matching format.money's contract. A domain floored at zero still takes the established path and is byte-identical: 0 diffs over 2,006 non-negative cases for line_chart and column_chart, and the demo dashboard matches main exactly. Over 5,583 mixed-sign cases: no value escapes the plot band, no signed-zero tick, no duplicate or coincident rule. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01XjLWb6igee2tVTG7wg93Fx --- src/flightdeck/report/charts.py | 91 ++++++++++++++++++++++++++------- tests/test_charts.py | 43 +++++++++++++++- 2 files changed, 113 insertions(+), 21 deletions(-) diff --git a/src/flightdeck/report/charts.py b/src/flightdeck/report/charts.py index 44613ce..95b6de2 100644 --- a/src/flightdeck/report/charts.py +++ b/src/flightdeck/report/charts.py @@ -34,10 +34,24 @@ def _nice_top(value: float) -> float: return raw * 10 +def _nice_step(span: float) -> float: + """Smallest 1/2/2.5/5 x 10^k step that cuts span into roughly three intervals.""" + raw = span / 3 + magnitude = 10 ** math.floor(math.log10(raw)) + for mult in (1, 2, 2.5, 5, 10): + if magnitude * mult >= raw: + return magnitude * mult + return magnitude * 10 + + def _fmt(value: float, decimals: int | None = None) -> str: if decimals is None: decimals = 0 if abs(value) >= 100 or value == int(value) else 1 - return f"{value:,.{decimals}f}" + text = f"{value:,.{decimals}f}" + # Never a signed zero. format.money makes the same promise for currency and + # _money for the bar labels; once ticks can go negative, "-0.0 h" would sit on + # the same axis as "0 h" and read as two different zeros. + return text.lstrip("-") if float(text.replace(",", "")) == 0 else text def _coord(value: float) -> str: @@ -57,20 +71,53 @@ def _money(value: float, unit: str) -> str: return f"{'−' if negative else ''}{unit}{_fmt(abs(value))}" -def _grid_and_axis(top: float, unit: str, plot_h: float, bottom: float = 0.0) -> str: - """Gridlines across the domain [bottom, top]; the axis sits on the zero line. - With the default bottom=0 the zero line IS the plot floor, so a non-negative - chart -- column_chart included -- renders byte-for-byte as before.""" - span = (top - bottom) or 1.0 - parts = [] - for index in range(1, 4): # 3 hairlines + baseline - y = _PAD_T + plot_h * (1 - index / 3) - tick = bottom + span * index / 3 - parts.append(f'') - parts.append( - f'{_fmt(tick)}{unit}' - ) - zero = _coord(_PAD_T + plot_h * (1 - (0.0 - bottom) / span)) +def _tick_decimals(step: float) -> int: + """Decimals enough to tell one tick from the next. _fmt picks its precision from + the value, which collapses two adjacent small ticks to the same label (-0.05 and + -0.10 both render "-0.1"); a tick scale has to be read off the step instead.""" + decimals = max(0, -math.floor(math.log10(step))) + if round(step, decimals) != step: # a 2.5-style step needs one more + decimals += 1 + return min(decimals, 6) + + +def _tick(y: float, value: float, unit: str, decimals: int | None = None, rule: bool = True) -> str: + line = f'' if rule else "" + return ( + f'{line}{_fmt(value, decimals)}{unit}' + ) + + +def _grid_and_axis(top: float, unit: str, plot_h: float, bottom: float = 0.0, step: float = 0.0) -> str: + """Gridlines and the zero axis over the domain [bottom, top]. + + A domain floored at zero keeps the established framing -- three hairlines at + thirds of ``top``, the axis ruling the plot floor where zero is self-evident + and needs no tick -- so those charts, column_chart included, render + byte-for-byte as before. + + A two-sided domain instead walks multiples of ``step``. Zero then always + lands on a gridline and carries a label, and so does the domain minimum: + without them the axis is just a heavier rule at an arbitrary height, and a + reader can see the dip but not read how deep it goes.""" + if bottom == 0.0: + parts = [ + _tick(_PAD_T + plot_h * (1 - index / 3), top * index / 3, unit) + for index in range(1, 4) # 3 hairlines + baseline + ] + zero = _coord(_PAD_T + plot_h) + else: + span = top - bottom + decimals = _tick_decimals(step) + parts = [] + for k in range(round(bottom / step), round(top / step) + 1): + value = k * step + y = _PAD_T + plot_h * (1 - (value - bottom) / span) + # Zero's rule is the axis itself, drawn below; a hairline there too + # would put two opaque 1px strokes on the same pixel. + parts.append(_tick(y, value, unit, decimals, rule=bool(k))) + zero = _coord(_PAD_T + plot_h * (1 - (0.0 - bottom) / span)) parts.append(f'') return "".join(parts) @@ -95,9 +142,15 @@ def line_chart(chart_id: str, labels: list[str], values: list[float], unit: str, # rejection-heavy week (docs/metrics.md — a rejected run earns −m minutes), and # a domain floored at zero maps that week far below the viewBox, where the # browser clips it and the card silently hides the bad news. Non-negative data - # keeps bottom=0, i.e. exactly the previous framing. - top = _nice_top(max(values)) if max(values) > 0 else 0.0 - bottom = -_nice_top(-min(values)) if min(values) < 0 else 0.0 + # keeps bottom=0 and step=top/3, i.e. exactly the previous framing. + hi, lo = max(max(values), 0.0), min(min(values), 0.0) + if lo == 0.0: + top, bottom = _nice_top(hi), 0.0 + step = top / 3 + else: + # Snap both ends to multiples of a nice step so zero is itself a gridline. + step = _nice_step(hi - lo) + top, bottom = math.ceil(hi / step) * step, math.floor(lo / step) * step span = (top - bottom) or 1.0 n = len(values) xs = [_PAD_L + plot_w * (i + 0.5) / n for i in range(n)] @@ -122,7 +175,7 @@ def line_chart(chart_id: str, labels: list[str], values: list[float], unit: str, end_label = f"{_fmt(values[-1], 1)}{unit}" end_x = min(xs[-1] + 8, _W - _PAD_R - 4) return f""" -{_grid_and_axis(top, unit, plot_h, bottom)} +{_grid_and_axis(top, unit, plot_h, bottom, step)} diff --git a/tests/test_charts.py b/tests/test_charts.py index 9b76b2f..6c99e90 100644 --- a/tests/test_charts.py +++ b/tests/test_charts.py @@ -2,8 +2,10 @@ the weekly line chart's numeric domain. hours_saved per week can be negative (docs/metrics.md: a rejected run earns −m -minutes), and that series feeds line_chart on the dashboard, so its y-domain -must reach below zero — the terminal sparkline already does (test_terminal.py). +minutes), and that series feeds line_chart on the dashboard, so its y-domain must +reach below zero and stay readable there. The terminal sparkline survives the same +input by flooring it (test_terminal.py), which a chart with a labeled axis cannot +do: it has to show how deep the week went. """ import re @@ -16,6 +18,10 @@ _BAND = (_PAD_T, _H - _PAD_B) +def _ticks(svg: str) -> list[str]: + return re.findall(r'class="fd-tick"[^>]*>([^<]+ h)<', svg) + + def _line_ys(svg: str) -> list[float]: path = re.search(r'class="fd-line" d="([^"]+)"', svg).group(1) return [float(y) for y in re.findall(r"[ML]-?[\d.]+ (-?[\d.]+)", path)] @@ -85,3 +91,36 @@ def test_column_chart_is_untouched_by_the_line_chart_domain_fix(): # The zero line keeps its integer form; the bars' own ":.1f" coordinates are # untouched by the fix and stay as they were. assert '' in svg + + +def test_line_chart_labels_zero_and_the_depth_of_a_bad_week(): + # Keeping the negative week on the page is only half the fix: without a labeled + # zero the axis is a heavier rule at an arbitrary height, and the reader can see + # the dip but not read how far it goes. + svg = line_chart("hours", ["W27", "W28", "W29"], [10.0, -20.0, 5.0], " h", "hours saved") + ticks = _ticks(svg) + + assert "0 h" in ticks # zero is on the scale, not implied by the frame + assert "-20 h" in ticks # so is the floor of the domain: the depth of the week + values = [float(t.removesuffix(" h")) for t in ticks] + assert values == sorted(values) # monotonic, emitted bottom of the band upward + assert len(set(ticks)) == len(ticks) # every label distinguishable from its neighbour + + +def test_line_chart_ticks_never_render_a_signed_zero(): + # format.money and charts._money both promise a value that rounds to zero is + # unsigned. Ticks only started going negative with the two-sided domain, so the + # same promise has to hold here: "-0.0 h" beside "0.0 h" reads as two zeros. + ticks = _ticks(line_chart("hours", ["W27", "W28"], [0.0, -0.1], " h", "hours saved")) + + assert not [t for t in ticks if t.startswith("-") and float(t.removesuffix(" h")) == 0], ticks + assert len(set(ticks)) == len(ticks), ticks # decimals come off the step, not the value + + +def test_line_chart_draws_no_hairline_under_the_zero_axis(): + # The axis already rules zero; a gridline at the same y would stack two opaque + # 1px strokes and cost the chart one of its three promised hairlines. + svg = line_chart("hours", ["W27", "W28"], [-5.0, -10.0], " h", "hours saved") + + axis_y = re.search(r'class="fd-axis" x1="44" y1="([\d.]+)"', svg).group(1) + assert axis_y not in re.findall(r'class="fd-grid" x1="44" y1="([\d.]+)"', svg) From 295c60c5cfb8310f207805c0c98a590af78cf520 Mon Sep 17 00:00:00 2001 From: Marc Sturlese Date: Fri, 4 Sep 2026 01:03:38 +0200 Subject: [PATCH 4/4] test(charts): make the hairline guard compare numbers, not strings Round-2 review: test_line_chart_draws_no_hairline_under_the_zero_axis passed unchanged on 678655f -- the exact code that did stack a hairline on the axis. The axis is emitted through _coord ("18") while gridlines use ":.1f" ("18.0"), so the string comparison missed at precisely the two integral positions where a regression lands. Compare floats; verified it now fails on 678655f. Also narrow two claims the review showed were overstated: the byte-identity of a zero-floored domain holds except for a label that rounds to zero losing its minus sign (the deliberate _fmt normalization, which also reaches column_chart's peak label for -0.0), and a two-sided scale yields 2-4 hairlines rather than the fixed three, since readable tick values matter more than a tidy count. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01XjLWb6igee2tVTG7wg93Fx --- src/flightdeck/report/charts.py | 17 ++++++++++------- tests/test_charts.py | 8 ++++++-- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/flightdeck/report/charts.py b/src/flightdeck/report/charts.py index 95b6de2..d0418c8 100644 --- a/src/flightdeck/report/charts.py +++ b/src/flightdeck/report/charts.py @@ -94,13 +94,16 @@ def _grid_and_axis(top: float, unit: str, plot_h: float, bottom: float = 0.0, st A domain floored at zero keeps the established framing -- three hairlines at thirds of ``top``, the axis ruling the plot floor where zero is self-evident - and needs no tick -- so those charts, column_chart included, render - byte-for-byte as before. - - A two-sided domain instead walks multiples of ``step``. Zero then always - lands on a gridline and carries a label, and so does the domain minimum: - without them the axis is just a heavier rule at an arbitrary height, and a - reader can see the dip but not read how deep it goes.""" + and needs no tick. Those charts, column_chart included, render byte-for-byte + as before, with one deliberate exception: a label that rounds to zero loses + its minus sign (see _fmt). + + A two-sided domain instead walks multiples of ``step``. Zero always lands on + a labeled position and so does the domain minimum: without them the axis is + just a heavier rule at an arbitrary height, and a reader can see the dip but + not read how deep it goes. The hairline count is whatever the step yields + (2-4 in practice) rather than a fixed three, because on a two-sided scale the + tick values have to be readable numbers before they are a tidy count.""" if bottom == 0.0: parts = [ _tick(_PAD_T + plot_h * (1 - index / 3), top * index / 3, unit) diff --git a/tests/test_charts.py b/tests/test_charts.py index 6c99e90..fae05ee 100644 --- a/tests/test_charts.py +++ b/tests/test_charts.py @@ -122,5 +122,9 @@ def test_line_chart_draws_no_hairline_under_the_zero_axis(): # 1px strokes and cost the chart one of its three promised hairlines. svg = line_chart("hours", ["W27", "W28"], [-5.0, -10.0], " h", "hours saved") - axis_y = re.search(r'class="fd-axis" x1="44" y1="([\d.]+)"', svg).group(1) - assert axis_y not in re.findall(r'class="fd-grid" x1="44" y1="([\d.]+)"', svg) + # Compare numbers, not strings: the axis goes through _coord ("18") while + # gridlines use ":.1f" ("18.0"), so a string test silently passes at exactly + # the two integral positions where a regression would land. + axis_y = float(re.search(r'class="fd-axis" x1="44" y1="([\d.]+)"', svg).group(1)) + grid_ys = [float(y) for y in re.findall(r'class="fd-grid" x1="44" y1="([\d.]+)"', svg)] + assert axis_y not in grid_ys, (axis_y, grid_ys)