Skip to content

feat(highcharts): implement radar-basic#9751

Merged
MarkusNeusinger merged 6 commits into
mainfrom
implementation/radar-basic/highcharts
Jul 24, 2026
Merged

feat(highcharts): implement radar-basic#9751
MarkusNeusinger merged 6 commits into
mainfrom
implementation/radar-basic/highcharts

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

Implementation: radar-basic - javascript/highcharts

Implements the javascript/highcharts version of radar-basic.

File: plots/radar-basic/implementations/javascript/highcharts.js

Parent Issue: #744


🤖 impl-generate workflow

@claude

claude Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

AI Review - Attempt 1/3

Image Description

Light render (plot-light.png): Warm off-white background (consistent with #FAF8F1). Bold dark-ink title "radar-basic · javascript · highcharts · anyplot.ai", six bold dark category labels at the outer edge of each axis, thin gray spokes and pentagon grid rings at 20/40/60/80/100 with small gray ring-scale numbers along the top spoke, and a bottom-center legend with two rounded chips ("Q3 Review" green, "Q4 Review" purple). Two overlapping filled hexagons — brand green #009E73 and lavender #C475FD — with ~22% fill opacity so the overlap reads as a blended blue-gray, plus stroked circular vertex markers. All text is clearly readable against the light background — no light-on-light issues.

Dark render (plot-dark.png): Warm near-black background (consistent with #1A1A17). Same title, category labels, ring-scale numbers, and legend now render in light/cream text and are all clearly legible — no dark-on-dark failures. Data colors are confirmed identical to the light render (#009E73 / #C475FD); only chrome (background, text, grid) flipped as expected.

Both renders pass the theme-readability checklist; no missing/clipped pixels at either canvas edge.

Score: 88/100

Category Score Max
Visual Quality 30 30
Design Excellence 15 20
Spec Compliance 15 15
Data Quality 15 15
Code Quality 10 10
Library Mastery 3 10
Total 88 100

Visual Quality (30/30)

  • VQ-01: Text Legibility (8/8)
  • VQ-02: No Overlap (6/6)
  • VQ-03: Element Visibility (6/6)
  • VQ-04: Color Accessibility (2/2)
  • VQ-05: Layout & Canvas (4/4)
  • VQ-06: Axis Labels & Title (2/2)
  • VQ-07: Palette Compliance (2/2)

Design Excellence (15/20)

  • DE-01: Aesthetic Sophistication (6/8) - Thoughtful typography hierarchy and clean geometry, above a configured default
  • DE-02: Visual Refinement (5/6) - Subtle grid, no frame clutter, generous whitespace
  • DE-03: Data Storytelling (4/6) - Overlap communicates the shift, but no explicit callout of the biggest change

Spec Compliance (15/15)

  • SC-01: Plot Type (5/5)
  • SC-02: Required Features (4/4)
  • SC-03: Data Mapping (3/3)
  • SC-04: Title & Legend (3/3)

Data Quality (15/15)

  • DQ-01: Feature Coverage (6/6)
  • DQ-02: Realistic Context (5/5)
  • DQ-03: Appropriate Scale (4/4)

Code Quality (10/10)

  • CQ-01: KISS Structure (3/3)
  • CQ-02: Reproducibility (2/2)
  • CQ-03: Clean Imports (2/2)
  • CQ-04: Code Elegance (2/2)
  • CQ-05: Output & API (1/1)

Library Mastery (3/10)

  • LM-01: Idiomatic Usage (2/5) - series: []; the chart never uses Highcharts' series/data-binding API
  • LM-02: Distinctive Features (1/5) - Generic renderer-based SVG composition, tooltip explicitly disabled

Score Caps Applied

  • None (Library Mastery is low but no cap threshold is crossed)

Strengths

  • Clean hand-built polar geometry (angleOf/pointAt helpers) produces a well-proportioned hexagonal radar with balanced margins and no text overlap or edge clipping in either theme.
  • Correct Imprint palette usage — first series #009E73, second series canonical position 2 (#C475FD) — identical across light and dark renders; theme-adaptive chrome correctly threads through every drawn element.
  • Realistic, well-differentiated data: Q3-to-Q4 employee review scores rise in most competencies but dip slightly in Adaptability, giving the two overlapping polygons genuine comparative meaning.
  • Ring scale labels, spokes, and outer category labels are all legible and non-overlapping in both themes, including at the crowded top-spoke intersection near the highest-value data points.

Weaknesses

  • The chart renders zero real Highcharts series (series: []) — every visual element (grid rings, spokes, polygons, markers, legend, even axis labels) is drawn via chart.renderer primitives, sidestepping Highcharts' data-binding/series model entirely.
  • Because there is no real series or tooltip, the interactive HTML output has no hover functionality at all — the static PNG and the HTML view are identical for an interactive library.
  • Design Excellence is solid but not exceptional — no explicit callout highlighting the strongest single-axis change (e.g. Leadership +10).

Issues Found

  1. LM-01/LM-02 LOW: No real Highcharts series is used — the entire chart (grid, spokes, polygons, markers, legend, labels) is drawn manually via chart.renderer, bypassing Highcharts' series/data-binding model and disabling tooltips entirely.
    • Fix: Add an actual (possibly marker-only) scatter series at the same pixel-projected vertex points with tooltip: { enabled: true } and a formatter showing category: value, layered on top of the existing renderer-drawn visual. This keeps the PNG output identical while giving the HTML deliverable real interactive value and grounding the implementation in Highcharts' actual series API.
  2. DE-03 MEDIUM: No explicit emphasis on the biggest Q3→Q4 change.
    • Fix: Consider a small annotation or data-label callout on the axis with the largest delta (Leadership: 65→75) to sharpen the storytelling, though this is optional polish.

AI Feedback for Next Attempt

The visual execution, spec compliance, and data quality are excellent — no repair needed there. The one substantive gap is Library Mastery: this implementation only uses Highcharts as an SVG canvas (chart.renderer) with series: [] and tooltips disabled, so the interactive HTML output provides zero value over the static PNG. Add a real (invisible-line or marker) scatter series at the computed vertex coordinates with tooltips enabled, so hovering shows each category's value — this preserves the current visual exactly while grounding the chart in Highcharts' actual series/data-binding API. Optionally, add a small callout for the largest single-axis change to strengthen data storytelling.

Verdict: REJECTED

@github-actions github-actions Bot added quality:88 Quality score: 88/100 ai-rejected Quality not OK, triggers update labels Jul 24, 2026
@github-actions github-actions Bot added ai-attempt-1 First repair attempt and removed ai-rejected Quality not OK, triggers update labels Jul 24, 2026
Attempt 1/4 - fixes based on AI review
@github-actions

Copy link
Copy Markdown
Contributor Author

🔧 Repair Attempt 1/4

Applied fixes based on AI review feedback.

Status: Repair completed, re-triggering review...


🤖 impl-repair

@claude

claude Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

AI Review - Attempt 2/3

Image Description

Light render (plot-light.png): Warm off-white background (consistent with #FAF8F1). Bold dark-ink title "radar-basic · javascript · highcharts · anyplot.ai", six bold dark category labels (Communication, Technical Skills, Teamwork, Leadership, Adaptability, Problem Solving) at the outer edge of each axis, thin gray spokes and pentagon-like hexagon grid rings at 20/40/60/80/100 with small gray ring-scale numbers along the top spoke, and a bottom-center legend with two rounded chips ("Q3 Review" green, "Q4 Review" purple). Two overlapping filled hexagons — brand green #009E73 and lavender #C475FD — with ~22% fill opacity so the overlap reads as a blended tone, plus stroked circular vertex markers. All text is clearly readable against the light background — no light-on-light issues.

Dark render (plot-dark.png): Warm near-black background (consistent with #1A1A17). Same title, category labels, ring-scale numbers, and legend now render in light/cream text and remain fully legible — no dark-on-dark failures. Data colors are confirmed identical to the light render (#009E73 / #C475FD); only chrome (background, text, grid) flipped as expected.

Both renders pass the theme-readability checklist; no missing/clipped pixels at either canvas edge. Canvas confirmed 2400×2400 (square).

Score: 91/100

Category Score Max
Visual Quality 30 30
Design Excellence 15 20
Spec Compliance 15 15
Data Quality 15 15
Code Quality 10 10
Library Mastery 6 10
Total 91 100

Visual Quality (30/30)

  • VQ-01: Text Legibility (8/8)
  • VQ-02: No Overlap (6/6)
  • VQ-03: Element Visibility (6/6)
  • VQ-04: Color Accessibility (2/2)
  • VQ-05: Layout & Canvas (4/4)
  • VQ-06: Axis Labels & Title (2/2)
  • VQ-07: Palette Compliance (2/2)

Design Excellence (15/20)

  • DE-01: Aesthetic Sophistication (6/8) - Thoughtful typography hierarchy and clean hand-built polar geometry, above a configured default
  • DE-02: Visual Refinement (5/6) - Subtle grid, no frame clutter, generous whitespace
  • DE-03: Data Storytelling (4/6) - Overlap communicates the shift, but no explicit callout of the biggest single-axis change

Spec Compliance (15/15)

  • SC-01: Plot Type (5/5)
  • SC-02: Required Features (4/4)
  • SC-03: Data Mapping (3/3)
  • SC-04: Title & Legend (3/3)

Data Quality (15/15)

  • DQ-01: Feature Coverage (6/6)
  • DQ-02: Realistic Context (5/5)
  • DQ-03: Appropriate Scale (4/4)

Code Quality (10/10)

  • CQ-01: KISS Structure (3/3)
  • CQ-02: Reproducibility (2/2)
  • CQ-03: Clean Imports (2/2)
  • CQ-04: Code Elegance (2/2)
  • CQ-05: Output & API (1/1)

Library Mastery (6/10)

  • LM-01: Idiomatic Usage (3/5) - Repair adds a real, data-bound scatter series (addSeries, fixed-extent axes) — correct usage, but the chart's core visual body (grid, spokes, polygons, markers, legend, labels) still bypasses series/data-binding via chart.renderer primitives
  • LM-02: Distinctive Features (3/5) - Genuine Highcharts-specific tooltip formatter (this.point.custom.actualValue) and hover marker states, directly addressing the attempt-1 gap

Score Caps Applied

  • None

Strengths

  • Clean hand-built polar geometry (angleOf/pointAt helpers) produces a well-proportioned hexagonal radar with balanced margins and no text overlap or edge clipping in either theme.
  • Correct Imprint palette usage — first series #009E73, second series canonical position 2 (#C475FD) — identical across light and dark renders; theme-adaptive chrome correctly threads through every drawn element.
  • The repair adds a genuine data-bound Highcharts scatter series at the same vertex coordinates, with enableMouseTracking and a tooltip formatter reporting the real category/value — the interactive HTML now provides real hover value beyond the static PNG, while the PNG output stays pixel-identical to attempt 1.
  • Realistic, well-differentiated data: Q3-to-Q4 employee review scores rise in most competencies but dip slightly in Adaptability, giving the two overlapping polygons genuine comparative meaning.
  • Ring scale labels, spokes, and outer category labels are all legible and non-overlapping in both themes, including at the crowded top-spoke intersection near the highest-value data points.

Weaknesses

  • The core visual (grid rings, spokes, category/ring labels, data polygons, markers, legend) is still drawn entirely via chart.renderer primitives rather than Highcharts' series/data-binding model — the newly added scatter series is a thin, invisible (marker.enabled: false) layer bolted on purely for tooltip purposes, so Library Mastery is improved but not maximized.
  • No explicit callout highlighting the single biggest Q3→Q4 change (Leadership 65→75) — optional polish for Data Storytelling, not required.

Issues Found

None blocking — remaining gaps are minor and optional polish only.

AI Feedback for Next Attempt

This attempt resolves the attempt-1 blocker: a real, data-bound Highcharts series with a working tooltip formatter now backs the interactive HTML output. Visual quality, spec compliance, data quality, and code quality are all excellent. If further polish is desired: (1) push Library Mastery further by having more of the visual body (e.g. the data polygons themselves) participate in a real series definition rather than pure chart.renderer composition, and (2) optionally add a small data-label callout on the axis with the largest Q3→Q4 delta to sharpen the storytelling.

Verdict: APPROVED

@github-actions github-actions Bot added quality:91 Quality score 91/100 ai-approved Quality OK, ready for merge and removed quality:88 Quality score: 88/100 labels Jul 24, 2026
@MarkusNeusinger
MarkusNeusinger merged commit 2edc737 into main Jul 24, 2026
@MarkusNeusinger
MarkusNeusinger deleted the implementation/radar-basic/highcharts branch July 24, 2026 20:30
MarkusNeusinger added a commit that referenced this pull request Jul 24, 2026
…illed (#9772)

## What happened

On **2026-07-24, ~16:05–16:26 UTC** GitHub's API browned out — HTTP 502
on the
workflow-dispatch endpoint, HTTP 504 on GraphQL, HTTP 429 on codeload.
Four `radar-basic`
PRs fell out of the pipeline and stayed out, because the calls that hand
work to the next
workflow had no retry:

| PR | Library | State it was left in | Call that died |
|---|---|---|---|
| #9742 | seaborn | `ai-approved` q90 | `impl-merge` → `gh pr view` →
**504** |
| #9744 | pygal | `ai-rejected` q87 | repair dispatch → **504**;
`ai-attempt-1` also lost |
| #9749 | makie | `ai-rejected` q88 | repair dispatch → **502** |
| #9751 | highcharts | `ai-rejected` q88 | repair dispatch → **502** |

`impl-review`'s verdict step is the pipeline's **only hand-off point** —
every downstream
workflow starts from a call made there. So an unretried blip doesn't
merely fail a step; it
leaves a PR carrying a verdict label with nothing listening.

All four have since been recovered manually and merged; this PR stops it
recurring.

## Changes

**`impl-review.yml` — the verdict step**
- every API call retries 3× with linear backoff (same shape as the
existing `Extract PR info`
  retry, added 2026-05-06 for this same class of failure)
- the repair is **dispatched before** the attempt label is added, and
the label can no longer
gate it. Losing the label costs one over-strict review; losing the
dispatch costs the whole PR
- if the dispatch still exhausts its retries, the step drops
`ai-rejected` on the way out:
`ai-rejected` + `ai-attempt-N` matches **no** watchdog case (Case 2
excludes any verdict
label, Case 4 excludes PRs that already have an attempt label), while
the label-less state is
exactly Case 2 — which re-dispatches the repair at the attempt number
the remaining label
  still encodes
- the label read tracks success explicitly instead of inferring it from
empty output, and a
successful read with no verdict label now fails loudly instead of ending
the step green

**Two bugs found while verifying the above**
- **`ai-attempt-4` never existed as a label.** The attempt label encodes
the cascading
threshold (90 → 80 → 70 → 60 → 50), but the repo only has
`ai-attempt-1..3`, so on the 4th
repair the add failed every time and `|| true` swallowed it — #7268 and
#7039 both reached
"Repair Attempt 4/4" carrying only `ai-attempt-1..3`. Every 4th review
therefore re-applied
the ≥ 90 attempt-0 bar, and the attempts-exhausted branch (close PR,
remove stale
implementation) could never be reached. Labels are now created on
demand.
*This also means my first draft of this PR was a hard blocker:* with the
label add fatal, the
  4th repair would never have been dispatched at all.
- **That newly-reachable branch rendered a literal `$SCORE/100`** —
quoted heredoc. Now
expands, with the markdown backticks escaped so they can't become
command substitution.

**`impl-merge.yml`**
- the 5× merge retry re-reads PR state first. A merge that succeeded
server-side but lost its
HTTP response used to fail four more times with "already merged" and
exit 1 — and because
every post-merge step is gated on `should_run == 'true'` (implicit
`success()`), that skipped
GCS promotion, the `impl:{lib}:done` label, closing the issue and the
Postgres sync. A merged
PR with none of its bookkeeping: exactly the silent partial completion
CLAUDE.md warns about

**`auto-update-pr-branches.yml` — Dependabot PRs could never merge while
the pipeline ran**
- the workflow called `update-branch` on Dependabot PRs. That push is
authored by
`github-actions[bot]`, so GitHub gates the resulting runs behind manual
approval
(`action_required`) — and because `main` advances every few minutes
during impl merges, each
branch was re-updated long before anyone could approve. **PR #9674
accumulated 174 runs in
22 h: 162 `action_required` against only 4 green** (the original
`dependabot[bot]` push)
- Dependabot branches are now skipped. They merge fine while behind —
the `main` ruleset is
**not** strict (`strict_required_status_checks_policy: false`); the
file's header claimed the
  opposite and is corrected
- `/dependabot`'s playbook told operators to run that same harmful
`update-branch` by hand; it
  now says the opposite and documents the `@dependabot recreate` remedy

## Verification

`.github/workflows/` has no verification loop (CLAUDE.md), so this was
reviewed by a 4-lens
adversarial pass with an independent refutation round (regression /
shell-under-`bash -e` / retry idempotency / the Dependabot
premise), and every claim was re-checked against the live repo before
acting. That pass is what
caught the `ai-attempt-4` blocker and the false Dependabot-rebase
premise in my first draft.

- YAML parses; `bash -n` clean on the extracted steps
- `gh_retry` semantics exercised under `bash -e`: succeeds first try /
recovers after 2
  failures / multi-arg passthrough / hard failure aborts the step
- the corrected control flow tested for all three cases — normal,
**attempt 4 with a
permanently failing label add (dispatch must still fire)**, and dispatch
exhaustion
  (`ai-rejected` dropped, exit 1)
- confirmed the CI runner's `gh` adds labels fine; only the
locally-installed gh 2.45.0 hits
the `projectCards` GraphQL deprecation, so hard-failing on `--add-label`
is safe in CI
- ruleset strictness, the `ai-attempt-*` label set, and the poisoned
Dependabot head commits
  were all read from the live repo, not assumed

## Follow-up (not in this PR)

- #9674, #9648 and #8820 already have a `github-actions[bot]` merge
commit as their head. Once
this merges their head stops moving, so approving each gated run
**once** is enough — verified
that the required contexts do report as SUCCESS on a bot-authored head
after approval (#9674,
commit `59545076`). Doing it before this merges would just get re-gated
by the next push to
`main`. (`@dependabot recreate` is an equivalent alternative; it also
restores a clean
  `dependabot[bot]` head but re-resolves the versions.)
- Whether `auto-update-pr-branches.yml` should exist at all now that the
ruleset is non-strict
is worth deciding separately — it affects every auto-merge PR, so it
isn't folded in here.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-approved Quality OK, ready for merge ai-attempt-1 First repair attempt quality:91 Quality score 91/100

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant