fix: review rubric knows all 15 libraries#9623
Merged
Merged
Conversation
…ss, checklist shape 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 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the workflow-active quality review rubric and related documentation/tests so it accurately reflects the current 15-library / 4-language registry (Python, R, Julia, JavaScript) and avoids future “rubric drift” that would silently bias review scores.
Changes:
- Updates the reviewer prompts and scoring criteria to recognize JavaScript/TSX implementations (extensions, interactive-library list, SC-04 language set) and to include interactive fairness rules in the workflow-consumed prompt.
- Fixes the
review_checklist.jsonexample to the canonical 6-key shape and updated max-score distribution. - Adds unit drift-guard tests that derive expectations from
core.constantsso future registry changes cause test failures rather than silent rubric skew.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
tests/unit/prompts/test_prompts.py |
Adds registry-derived drift-guard tests for interactive library lists, file extensions, SC-04 language set, and checklist example keys. |
prompts/workflow-prompts/ai-quality-review.md |
Updates extension/language guidance, adds interactive+JS fairness rules, fixes SC-04 language set, and corrects checklist example shape/instructions. |
prompts/quality-evaluator.md |
Updates language/library era statements, extension guidance, SC-04 language set, and interactive library list to include JS/TSX libraries. |
prompts/quality-criteria.md |
Makes AR-01 verification language-generic, expands AR-05 “must use” rows for JS libs, updates AR-08 static-library list, and fixes SC-04 language set. |
CHANGELOG.md |
Adds an Unreleased “Fixed” entry documenting the rubric corrections and drift-guard tests. |
.github/workflows/impl-review.yml |
Passes LANGUAGE/EXT values (from the existing lang step) into the reviewer prompt variables. |
Comments suppressed due to low confidence (1)
prompts/quality-evaluator.md:323
- This section says “DO NOT penalize” interactive features including “Animations”, but the JS render harness and JS library prompts require entrance animations to be disabled so the PNG screenshot is deterministic. Keeping “Animations” unqualified here can mislead offline reviewers into treating animated-on-load charts as acceptable.
**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
- Zoom, pan, selection tools
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts: # CHANGELOG.md
MarkusNeusinger
force-pushed
the
fix/rubric-15-libraries
branch
from
97b46c0 to
8ddbdf8
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
CHANGELOG.md:25
- This section still contains unresolved Git merge-conflict markers (<<<<<<< / =======). As-is, the changelog is invalid Markdown and will break parsing/rendering; resolve the conflict by removing the markers and keeping both entries.
- **Global keyboard shortcuts no longer hijack focused elements on `/plots`** — the
window-level Space/Enter/Backspace handler now bails out when the keystroke targets an
interactive element (button, link, focused card/chip/toggle), so keyboard-activating a card
no longer double-fires with a random-plot jump (audit 2026-07-08 Quick Win 1) (#9620).
- **Dark-mode contrast for stock MUI components** — the MUI palette is locked to light-mode
CHANGELOG.md:65
- There is still an unresolved Git merge-conflict end marker here (>>>>>>> origin/main). Remove it so the changelog renders correctly.
|
|
||
| WORKFLOW_PROMPTS_DIR = PROMPTS_DIR / "workflow-prompts" | ||
|
|
||
| SCORING_PROMPTS = [PROMPTS_DIR / "quality-evaluator.md", PROMPTS_DIR / "workflow-prompts" / "ai-quality-review.md"] |
| | 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 | |
|
|
||
| 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 |
| ) | ||
|
|
||
|
|
||
| class TestFifteenLibraryCoverage: |
Comment on lines
+40
to
42
| | 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? | |
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
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.
Summary
core/constants.py):ai-quality-review.md+quality-evaluator.md:.js(chartjs, d3, echarts, highcharts) and.tsx(muix) added; highcharts no longer listed as Python.{python, r, julia}(andquality-criteria.mdeven only{python, r}) — every correct… · javascript · …title silently lost points. Now{python, r, julia, javascript}in all three files.impl-review.ymlnow passesLANGUAGE/EXT(already computed in thelangstep) to the reviewer prompt — the prompt referenced them but never received them.impl-generate.yml/impl-repair.ymlalready did this.quality-evaluator.md, which no workflow reads. They now live in the running prompt as section 5e, with the canonical 10-library interactive list and a JS-harness note for CQ-05 (JS snippets never save files — the harness captures; check the mount-node contract instead ofsavefig).review_checklist.jsonexample still showedlibrary_featuresand 40/25/20/10/5 maxima — the shape that made ~6% of shipped review checklists inconsistent. Now the canonical six keys (30/20/15/15/10/10) with an "exactly these six keys" instruction.TestFifteenLibraryCoverageintests/unit/prompts/test_prompts.pyderives expectations fromINTERACTIVE_LIBRARIES,LANGUAGE_FILE_EXTENSIONS,LIBRARIES_METADATA, andSUPPORTED_LANGUAGES— the next library addition fails these tests instead of silently biasing scores.Verification limits
prompts/and.github/workflows/changes have no local verify loop — they are only observable on real pipeline runs (known gap per CLAUDE.md). What was verified: YAML parses, the full unit suite (1518 passed, incl. 90 prompt tests with the 5 new drift guards), ruff clean. Theimpl-review.ymlchange is additive (two extra prompt variables); the first real review run after merge is the live check — worth watching the next daily-regen or bulk-generate review.Plan
agentic/audits/2026-07-08-product-ux.md (Medium#1, Medium#12)
Test plan
uv run pytest tests/unit(1518 passed; new drift-guard tests fail before the fix, pass after)uv run ruff check+format --checkclean; workflow YAML parsesjavascripttitles, and the storedreview_checklist.jsonshould carry the six canonical keys