From c9496e22a114ed962649036ced367852886968cb Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Fri, 10 Jul 2026 08:34:44 +0200 Subject: [PATCH 1/3] =?UTF-8?q?fix:=20review=20rubric=20knows=20all=2015?= =?UTF-8?q?=20libraries=20=E2=80=94=20extensions,=20SC-04,=20fairness,=20c?= =?UTF-8?q?hecklist=20shape?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The workflow-active reviewer prompt and scoring criteria still described the 9-Python-library era. Corrected against the canonical registry in core/constants.py: file-extension guide (.js/.tsx, highcharts is JS), SC-04 title language set now includes julia and javascript, LANGUAGE/EXT are actually passed by impl-review.yml, the interactive-fairness rules moved into the prompt that runs (with a JS-harness note for CQ-05), AR-05 gained chartjs/d3/echarts/muix rows, AR-08's static-library list includes ggplot2 and makie, and the step-10 checklist example uses the canonical six keys. New registry-derived tests in tests/unit/prompts/ fail on any future library-list drift. Audit 2026-07-08 Medium#1 + Medium#12. Co-Authored-By: Claude Fable 5 --- .github/workflows/impl-review.yml | 2 + CHANGELOG.md | 16 +++++ prompts/quality-criteria.md | 12 ++-- prompts/quality-evaluator.md | 8 +-- prompts/workflow-prompts/ai-quality-review.md | 36 +++++++--- tests/unit/prompts/test_prompts.py | 70 +++++++++++++++++++ 6 files changed, 125 insertions(+), 19 deletions(-) diff --git a/.github/workflows/impl-review.yml b/.github/workflows/impl-review.yml index e1324cd351..bd5b420965 100644 --- a/.github/workflows/impl-review.yml +++ b/.github/workflows/impl-review.yml @@ -327,6 +327,8 @@ jobs: - SPEC_ID: ${{ steps.pr.outputs.specification_id }} - PR_NUMBER: ${{ steps.pr.outputs.pr_number }} - ATTEMPT: ${{ steps.attempts.outputs.display }} + - LANGUAGE: ${{ steps.lang.outputs.language }} + - EXT: ${{ steps.lang.outputs.ext }} - name: Extract quality score id: score diff --git a/CHANGELOG.md b/CHANGELOG.md index 4855e02df9..4b2b7976b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,22 @@ aggregate instead: an italic *Catalog* line at the end of the version section an ## [Unreleased] +### Fixed + +- **Review rubric finally knows all 15 libraries** — the workflow-active reviewer prompt and + scoring criteria still described the 9-Python-library era: the file-extension guide called + highcharts Python and knew no `.js`/`.tsx`, SC-04's title rule rejected `javascript` (and in + `quality-criteria.md` even `julia`) as a language token — silently costing every correct + JS/Julia title points — `${LANGUAGE}`/`${EXT}` were referenced in the prompt but never passed + by `impl-review.yml`, the interactive-fairness rules (don't penalize hover/zoom invisible in + the PNG) lived only in a file no workflow reads, and the step-10 `review_checklist.json` + example still showed the pre-rebalance 5-category shape (`library_features`, 40/25/20/10/5 + maxima) that corrupted ~6% of stored checklists. All lists are corrected to the canonical + registry, the fairness rules (plus a JS-harness note for CQ-05) moved into the running prompt, + AR-05 gained rows for chartjs/d3/echarts/muix, and new registry-derived tests in + `tests/unit/prompts/` fail on any future library-list drift (audit 2026-07-08 Medium#1 + + Medium#12). + ### Added - **`llms.txt` for AI agents** — `/llms.txt` previously fell through to the SPA shell (flagged diff --git a/prompts/quality-criteria.md b/prompts/quality-criteria.md index a7363e105f..a0d280f2e1 100644 --- a/prompts/quality-criteria.md +++ b/prompts/quality-criteria.md @@ -37,7 +37,7 @@ Checks that gate quality scoring. On fail: Score=0. Workflow-handled checks (AR- | ID | Check | Description | Verification | |----|-------|-------------|--------------| -| AR-01 | SYNTAX_ERROR | Code cannot be parsed | `python -m py_compile` | +| AR-01 | SYNTAX_ERROR | Code cannot be parsed | Language parser (`python -m py_compile`, `Rscript -e parse`, `node --check`, …) | | AR-02 | RUNTIME_ERROR | Code throws exception | Execution with timeout | | AR-03 | NO_OUTPUT | No `plot.png` created | File exists? | | AR-04 | EMPTY_PLOT | Image empty | < 10KB or > 95% white | @@ -61,8 +61,12 @@ Implementation must use **plot functions** from the library, not just styling. | altair | `alt.Chart().mark_*()` | Only `configure_*()` | | plotnine | `ggplot() + geom_*()` | Only `theme()` | | pygal | Chart classes with data | Only config | -| highcharts | Chart with series | Only options | +| highcharts | `Highcharts.chart()` with series | Only options objects | | letsplot | `ggplot() + geom_*()` | Only `ggsize()` | +| chartjs | `new Chart(...)` with datasets | Only options/plugin config | +| d3 | Data joins + scales rendering marks (`selection.data(...).join(...)`) | Only selections/styling | +| echarts | `chart.setOption(...)` with series | Only `echarts.init()`/theme config | +| muix | MUI X chart components (``, ``, …) with series | Only `ThemeProvider`/styling | **Note:** Using data loading utilities (e.g., `sns.load_dataset()`, `sklearn.datasets`) from other libraries is allowed and does not count as library usage. This check only evaluates whether the implementation uses the library's **plotting functions**. @@ -72,7 +76,7 @@ When a library cannot technically implement a spec (e.g., pygal cannot do 3D), t ### AR-08: Fake Functionality -A static library (matplotlib, seaborn, plotnine) simulates interactive features that cannot work in a PNG image. +A static library (matplotlib, seaborn, plotnine, ggplot2, makie) simulates interactive features that cannot work in a PNG image. **Triggers (auto-reject):** - Simulated tooltips (annotation boxes styled to look like hover tooltips) @@ -358,7 +362,7 @@ This category evaluates aesthetic sophistication beyond mere correctness. A plot | Points | Criterion | |--------|-----------| -| 3 | Title is `{spec-id} · {language} · {library} · anyplot.ai`, optionally prefixed with `{Descriptive Title} · ` (language ∈ {python, r}). Legend labels correct | +| 3 | Title is `{spec-id} · {language} · {library} · anyplot.ai`, optionally prefixed with `{Descriptive Title} · ` (language ∈ {python, r, julia, javascript}). Legend labels correct | | 2 | Title format correct but legend issues, or vice versa | | 1 | Partially correct | | 0 | Missing or wrong | diff --git a/prompts/quality-evaluator.md b/prompts/quality-evaluator.md index b53191bae3..9892209328 100644 --- a/prompts/quality-evaluator.md +++ b/prompts/quality-evaluator.md @@ -4,7 +4,7 @@ ## Role -You are a strict code reviewer for data visualizations. Most implementations are Python; ggplot2 is R; Makie.jl is Julia. You evaluate plot implementations against `prompts/quality-criteria.md`. +You are a strict code reviewer for data visualizations. Most implementations are Python; ggplot2 is R; Makie.jl is Julia; chartjs, d3, echarts, and highcharts are JavaScript; muix is JavaScript (React, TSX). You evaluate plot implementations against `prompts/quality-criteria.md`. ## Two-Stage Evaluation @@ -44,7 +44,7 @@ You evaluate implementations that passed all auto-reject checks. Focus purely on ## Input 1. **Specification**: From `plots/{spec-id}/specification.md` -2. **Code**: From `plots/{spec-id}/implementations/{language}/{library}{ext}` — `{ext}` is `.py` for python libraries, `.R` for ggplot2, and `.jl` for makie +2. **Code**: From `plots/{spec-id}/implementations/{language}/{library}{ext}` — `{ext}` is `.py` for Python libraries, `.R` for ggplot2, `.jl` for makie, `.js` for the JavaScript libraries (chartjs, d3, echarts, highcharts), and `.tsx` for muix 3. **Previews**: BOTH theme renders of the plot image — `plot-light.png` and `plot-dark.png`. You must inspect both. For interactive libraries, also `plot-light.html` and `plot-dark.html`. 4. **Library Rules**: From `prompts/library/{library}.md` 5. **Style Guide** (canonical palette + theme tokens): `prompts/default-style-guide.md` — consult its "Categorical Palette", "Continuous Data", "Background", and "Theme-adaptive Chrome" sections for VQ-07 scoring. @@ -234,7 +234,7 @@ Not AR-09 (handle via VQ-05 instead): text overflowing its axis but staying on t | SC-01 | Plot Type | 5 | Correct chart type? | | SC-02 | Required Features | 4 | All spec features present? | | SC-03 | Data Mapping | 3 | X/Y correctly assigned? All data visible? | -| SC-04 | Title & Legend | 3 | Title is `{spec-id} · {language} · {library} · anyplot.ai`, optionally prefixed with `{Descriptive Title} · ` (language ∈ {python, r, julia}). Legend labels correct? | +| SC-04 | Title & Legend | 3 | Title is `{spec-id} · {language} · {library} · anyplot.ai`, optionally prefixed with `{Descriptive Title} · ` (language ∈ {python, r, julia, javascript}). Legend labels correct? | ### Step 4: Data Quality (15 pts) @@ -316,7 +316,7 @@ Both have ~5.76M pixels, so font size recommendations apply equally. **Important**: Interactive libraries also produce HTML output alongside PNG. -**Interactive libraries**: plotly, bokeh, altair, highcharts, pygal, letsplot +**Interactive libraries**: altair, bokeh, chartjs, d3, echarts, highcharts, letsplot, muix, plotly, pygal **DO NOT penalize** interactive features that aren't visible in PNG: - HoverTool, tooltips, hover info diff --git a/prompts/workflow-prompts/ai-quality-review.md b/prompts/workflow-prompts/ai-quality-review.md index dc778f04fe..9100a3de0c 100644 --- a/prompts/workflow-prompts/ai-quality-review.md +++ b/prompts/workflow-prompts/ai-quality-review.md @@ -17,7 +17,7 @@ Evaluate if the **${LIBRARY}** implementation matches the specification for `${S - Note all required features ### 2. Read the Implementation -`plots/${SPEC_ID}/implementations/${LANGUAGE}/${LIBRARY}${EXT}` — `${EXT}` is `.py` for python libraries (matplotlib, seaborn, plotly, bokeh, altair, plotnine, pygal, highcharts, letsplot), `.R` for ggplot2, and `.jl` for makie +`plots/${SPEC_ID}/implementations/${LANGUAGE}/${LIBRARY}${EXT}` — the workflow supplies `${LANGUAGE}` and `${EXT}` for this run. Reference: `.py` for the Python libraries (matplotlib, seaborn, plotly, bokeh, altair, plotnine, pygal, letsplot), `.R` for ggplot2 (R), `.jl` for makie (Julia), `.js` for the JavaScript libraries (chartjs, d3, echarts, highcharts), and `.tsx` for muix (JavaScript, React) ### 3. Read Library-Specific Rules `prompts/library/${LIBRARY}.md` @@ -104,6 +104,15 @@ Visually estimate from each PNG — no pixel measurement needed. These are soft - "Title spans ~80% of width at fontsize=14pt." → Expected for the long mandated anyplot title; no deduction. - "Y-axis label 'Fläche von Häusern in Quadratmetern' takes ~40% of axis length at fontsize=12pt." → Genuinely long label at sensible fontsize; no deduction as long as it doesn't overflow the axis. +### 5e. Interactive & JavaScript Libraries — Fairness Rules + +**Interactive libraries** (produce `plot-{theme}.html` alongside the PNG): altair, bokeh, chartjs, d3, echarts, highcharts, letsplot, muix, plotly, pygal. + +- **DO NOT penalize** interactive features that aren't visible in the PNG — tooltips/hover info, zoom/pan/selection tools, interactive legends, crossfiltering. The PNG is a static preview; these features add real value in the HTML detail view. Treat well-configured interactivity as a strength (e.g. "Uses HoverTool for detailed data inspection in HTML output"). +- **Only criticize** interactivity when it hurts the static render: hover-only labels that leave the PNG unreadable, misconfigured features, or errors. + +**JavaScript libraries** (chartjs, d3, echarts, highcharts as `.js`; muix as `.tsx`): the snippet renders into the browser harness's pre-sized `#container` mount node and never saves files itself — the harness (`automation/js-render/render.mjs`) captures `plot-{theme}.png` and `plot-{theme}.html`. For CQ-05, check the mount-node contract and current library API instead of `savefig`-style calls; animations must be disabled (`animation: false` or the library's equivalent) so the harness doesn't screenshot mid-animation. + ### 6. Check for Auto-Reject (AR-08, AR-09) **AR-08 — Fake interactivity (static libraries only — matplotlib, seaborn, plotnine, ggplot2, makie):** @@ -167,7 +176,7 @@ Read `prompts/quality-criteria.md` and evaluate: | SC-01 | Plot Type | 5 | Correct chart type? | | SC-02 | Required Features | 4 | All features from spec? | | SC-03 | Data Mapping | 3 | X/Y correct? Axes show all data? | -| SC-04 | Title & Legend | 3 | Title is `{spec-id} · {language} · {library} · anyplot.ai`, optionally prefixed with `{Descriptive Title} · ` (language ∈ {python, r, julia}). Legend labels match? | +| SC-04 | Title & Legend | 3 | Title is `{spec-id} · {language} · {library} · anyplot.ai`, optionally prefixed with `{Descriptive Title} · ` (language ∈ {python, r, julia, javascript}). Legend labels match? | #### Data Quality (15 pts) | ID | Criterion | Max | Check | @@ -324,21 +333,26 @@ Dark render (plot-dark.png): Legibility verdict: PASS | FAIL (explain if FAIL) EOF -# Criteria checklist as structured JSON +# Criteria checklist as structured JSON. +# Use EXACTLY these six keys — visual_quality, design_excellence, +# spec_compliance, data_quality, code_quality, library_mastery — with these +# maxima (30/20/15/15/10/10, matching step 7). The website's quality tab and +# the stored metadata rely on this exact shape; no other keys, no renames. cat > review_checklist.json << 'EOF' { "visual_quality": { - "score": 36, - "max": 40, + "score": 24, + "max": 30, "items": [ - {"id": "VQ-01", "name": "Text Legibility", "score": 10, "max": 10, "passed": true, "comment": "All text readable"}, - {"id": "VQ-02", "name": "No Overlap", "score": 8, "max": 8, "passed": true, "comment": "No overlapping elements"} + {"id": "VQ-01", "name": "Text Legibility", "score": 7, "max": 8, "passed": true, "comment": "All text readable in both themes"}, + {"id": "VQ-02", "name": "No Overlap", "score": 5, "max": 6, "passed": true, "comment": "No collisions"} ] }, - "spec_compliance": {"score": 23, "max": 25, "items": [...]}, - "data_quality": {"score": 18, "max": 20, "items": [...]}, - "code_quality": {"score": 10, "max": 10, "items": [...]}, - "library_features": {"score": 5, "max": 5, "items": [...]} + "design_excellence": {"score": 12, "max": 20, "items": [...]}, + "spec_compliance": {"score": 13, "max": 15, "items": [...]}, + "data_quality": {"score": 12, "max": 15, "items": [...]}, + "code_quality": {"score": 9, "max": 10, "items": [...]}, + "library_mastery": {"score": 6, "max": 10, "items": [...]} } EOF ``` diff --git a/tests/unit/prompts/test_prompts.py b/tests/unit/prompts/test_prompts.py index c7df1e8fe5..935fe5ecd8 100644 --- a/tests/unit/prompts/test_prompts.py +++ b/tests/unit/prompts/test_prompts.py @@ -14,6 +14,8 @@ import pytest +from core.constants import INTERACTIVE_LIBRARIES, LANGUAGE_FILE_EXTENSIONS, LIBRARIES_METADATA, SUPPORTED_LANGUAGES + # Base paths PROMPTS_DIR = Path(__file__).parent.parent.parent.parent / "prompts" @@ -419,3 +421,71 @@ def test_code_blocks_have_language(self) -> None: f"{filepath.name} has too few labeled code blocks " f"({labeled}/{total} = {labeled_ratio:.0%}, need ≥20%)" ) + + +class TestFifteenLibraryCoverage: + """Review prompts must know the full library registry. + + Guards against the drift the 2026-07-08 audit found (M#12): scoring + prompts still describing the 9-Python-library era while the pipeline + reviews R, Julia, and JavaScript implementations. Expectations derive + from core.constants so a future library lands here as a failing test, + not as silent scoring bias. + """ + + WORKFLOW_PROMPTS_DIR = PROMPTS_DIR / "workflow-prompts" + + SCORING_PROMPTS = [PROMPTS_DIR / "quality-evaluator.md", PROMPTS_DIR / "workflow-prompts" / "ai-quality-review.md"] + + @pytest.mark.parametrize("prompt_path", SCORING_PROMPTS, ids=lambda p: p.name) + def test_interactive_library_list_matches_registry(self, prompt_path: Path) -> None: + """The '**Interactive libraries**' line must cover the canonical set.""" + content = prompt_path.read_text() + line = next((line for line in content.splitlines() if line.startswith("**Interactive libraries**")), None) + assert line is not None, f"{prompt_path.name} has no '**Interactive libraries**' line" + missing = [lib for lib in sorted(INTERACTIVE_LIBRARIES) if lib not in line] + assert not missing, f"{prompt_path.name} interactive list missing: {missing}" + + @pytest.mark.parametrize("prompt_path", SCORING_PROMPTS, ids=lambda p: p.name) + def test_every_file_extension_documented(self, prompt_path: Path) -> None: + """Language defaults plus per-library overrides (.tsx) must all appear.""" + expected = set(LANGUAGE_FILE_EXTENSIONS.values()) | { + lib["file_extension"] for lib in LIBRARIES_METADATA if "file_extension" in lib + } + content = prompt_path.read_text() + missing = [ext for ext in sorted(expected) if f"`{ext}`" not in content] + assert not missing, f"{prompt_path.name} missing file extensions: {missing}" + + @pytest.mark.parametrize( + "prompt_path", + [ + PROMPTS_DIR / "quality-criteria.md", + PROMPTS_DIR / "quality-evaluator.md", + PROMPTS_DIR / "workflow-prompts" / "ai-quality-review.md", + ], + ids=lambda p: p.name, + ) + def test_sc04_title_rule_accepts_every_language(self, prompt_path: Path) -> None: + """SC-04's language set must include every supported language, or every + correct JS/Julia title costs points.""" + content = prompt_path.read_text() + sc04_lines = [line for line in content.splitlines() if "language ∈" in line] + assert sc04_lines, f"{prompt_path.name} has no SC-04 'language ∈' rule" + joined = "\n".join(sc04_lines) + missing = [lang for lang in sorted(SUPPORTED_LANGUAGES) if lang not in joined] + assert not missing, f"{prompt_path.name} SC-04 language set missing: {missing}" + + def test_checklist_example_uses_canonical_six_keys(self) -> None: + """The step-10 review_checklist.json example must show exactly the six + canonical category keys (the website renders this shape directly).""" + content = (self.WORKFLOW_PROMPTS_DIR / "ai-quality-review.md").read_text() + for key in ( + "visual_quality", + "design_excellence", + "spec_compliance", + "data_quality", + "code_quality", + "library_mastery", + ): + assert f'"{key}"' in content, f"checklist example missing key: {key}" + assert "library_features" not in content, "stale 5-category checklist key resurfaced" From 06d67ae7ce59010d467acf6332a72a2522793d03 Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Fri, 10 Jul 2026 08:35:38 +0200 Subject: [PATCH 2/3] docs: append PR ref #9623 to changelog entry Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b2b7976b5..1ecf8f0367 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,7 +30,7 @@ aggregate instead: an italic *Catalog* line at the end of the version section an registry, the fairness rules (plus a JS-harness note for CQ-05) moved into the running prompt, AR-05 gained rows for chartjs/d3/echarts/muix, and new registry-derived tests in `tests/unit/prompts/` fail on any future library-list drift (audit 2026-07-08 Medium#1 + - Medium#12). + Medium#12) (#9623). ### Added From 9c43c027f0fe757beb1fae2890ad04bfb5b3cab3 Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Fri, 10 Jul 2026 08:45:39 +0200 Subject: [PATCH 3/3] test: interactive-list drift guard asserts set equality (review) Co-Authored-By: Claude Fable 5 --- tests/unit/prompts/test_prompts.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/unit/prompts/test_prompts.py b/tests/unit/prompts/test_prompts.py index 935fe5ecd8..839b691ad3 100644 --- a/tests/unit/prompts/test_prompts.py +++ b/tests/unit/prompts/test_prompts.py @@ -439,12 +439,18 @@ class TestFifteenLibraryCoverage: @pytest.mark.parametrize("prompt_path", SCORING_PROMPTS, ids=lambda p: p.name) def test_interactive_library_list_matches_registry(self, prompt_path: Path) -> None: - """The '**Interactive libraries**' line must cover the canonical set.""" + """The '**Interactive libraries**' line must EQUAL the canonical set — + a missing entry biases scoring against that library, an extra entry + makes reviewers expect HTML output the library never produces.""" content = prompt_path.read_text() line = next((line for line in content.splitlines() if line.startswith("**Interactive libraries**")), None) assert line is not None, f"{prompt_path.name} has no '**Interactive libraries**' line" - missing = [lib for lib in sorted(INTERACTIVE_LIBRARIES) if lib not in line] - assert not missing, f"{prompt_path.name} interactive list missing: {missing}" + listed = {entry.strip().rstrip(".") for entry in line.rsplit(":", 1)[1].split(",")} + assert listed == set(INTERACTIVE_LIBRARIES), ( + f"{prompt_path.name} interactive list drifted — " + f"missing: {sorted(set(INTERACTIVE_LIBRARIES) - listed)}, " + f"extra: {sorted(listed - set(INTERACTIVE_LIBRARIES))}" + ) @pytest.mark.parametrize("prompt_path", SCORING_PROMPTS, ids=lambda p: p.name) def test_every_file_extension_documented(self, prompt_path: Path) -> None: