Land the lecture evaluation system (benchmark plugin 0.3.0: rubric v2, skill wired) - #5
Conversation
There was a problem hiding this comment.
Pull request overview
This PR lands the benchmark plugin’s lecture-acceleration evaluation framework, including a prose rubric plus a deterministic scoring engine and two worked evaluation baselines (ge_arrow, markov_asset) that can be regenerated from evidence files.
Changes:
- Adds a deterministic scoring engine (
scripts/scoring/) that computes dimension scores + weighted totals from per-lectureevidence.json. - Adds a shared efficiency calibration benchmark (
scripts/calibration/bellman_bench.*) and updates plugin/marketplace versions to0.2.0. - Adds the evaluation framework document plus two fully-worked evaluation example directories (scripts, results, evidence, reports) used as regression anchors.
Reviewed changes
Copilot reviewed 42 out of 43 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates plugin status messaging for benchmark. |
| benchmark/skills/review-acceleration/SKILL.md | Replaces placeholder procedure with the new 3-step evidence→score workflow and calibration anchors. |
| benchmark/scripts/scoring/score.py | Adds the scoring CLI that reads <lecture>/evidence.json and writes <lecture>/results/scorecard.json. |
| benchmark/scripts/scoring/rubric.py | Implements the rubric as code (weights, thresholds, checklists, verdict bands). |
| benchmark/scripts/scoring/EVIDENCE_TEMPLATE.json | Introduces the evidence schema template for new evaluations. |
| benchmark/scripts/README.md | Documents the scripts layout and end-to-end evaluation workflow. |
| benchmark/scripts/calibration/bellman_bench.py | Adds the shared high-end efficiency calibration benchmark. |
| benchmark/scripts/calibration/bellman_bench.json | Commits the benchmark output pinning the “score 5” efficiency anchor. |
| benchmark/references/examples/markov_asset/scripts/static_metrics.py | Adds markov_asset static metrics extraction used in scoring evidence. |
| benchmark/references/examples/markov_asset/scripts/smoke_test.py | Adds a runnable smoke test for old/new markov_asset implementations. |
| benchmark/references/examples/markov_asset/scripts/run_all.py | Adds markov_asset pipeline orchestration + provenance stamp + shared scoring invocation. |
| benchmark/references/examples/markov_asset/scripts/model_old.py | Adds extracted baseline NumPy implementation under evaluation. |
| benchmark/references/examples/markov_asset/scripts/model_new.py | Adds extracted candidate JAX implementation under evaluation (verbatim). |
| benchmark/references/examples/markov_asset/scripts/check_equivalence.py | Adds markov_asset equivalence checking (float32 vs x64) and patched-call-option comparison. |
| benchmark/references/examples/markov_asset/scripts/benchmark.py | Adds markov_asset warm scaling benchmark. |
| benchmark/references/examples/markov_asset/scripts/as_used_total.py | Adds markov_asset as-used (cold-inclusive) end-to-end timing script. |
| benchmark/references/examples/markov_asset/results/static_metrics.json | Adds captured metrics output used by the evidence/scorecard. |
| benchmark/references/examples/markov_asset/results/scorecard.json | Adds computed scorecard output for markov_asset baseline. |
| benchmark/references/examples/markov_asset/results/scaling.json | Adds captured scaling results for markov_asset baseline. |
| benchmark/references/examples/markov_asset/results/equivalence_x64_True.json | Adds captured x64 equivalence results for markov_asset baseline. |
| benchmark/references/examples/markov_asset/results/equivalence_x64_False.json | Adds captured float32-as-shipped equivalence results for markov_asset baseline. |
| benchmark/references/examples/markov_asset/markov_asset_REPORT.md | Adds the worked markov_asset evaluation report. |
| benchmark/references/examples/markov_asset/evidence.json | Adds the worked markov_asset evidence file consumed by the scorer. |
| benchmark/references/examples/ge_arrow/scripts/sweep_bench.py | Adds ge_arrow sweep benchmark used by the evaluation pipeline. |
| benchmark/references/examples/ge_arrow/scripts/static_metrics.py | Adds ge_arrow static metrics extraction used in scoring evidence. |
| benchmark/references/examples/ge_arrow/scripts/run_all.py | Adds ge_arrow pipeline orchestration + provenance stamp + shared scoring invocation. |
| benchmark/references/examples/ge_arrow/scripts/model_old.py | Adds extracted baseline NumPy implementation under evaluation. |
| benchmark/references/examples/ge_arrow/scripts/model_new.py | Adds extracted candidate JAX implementation under evaluation (verbatim). |
| benchmark/references/examples/ge_arrow/scripts/cold_start.py | Adds cold-start latency measurement for ge_arrow evaluation. |
| benchmark/references/examples/ge_arrow/scripts/check_equivalence.py | Adds ge_arrow equivalence checking across all lecture examples. |
| benchmark/references/examples/ge_arrow/scripts/benchmark.py | Adds ge_arrow benchmark (as-used, warm, scaling) used in scoring evidence. |
| benchmark/references/examples/ge_arrow/scripts/as_used_total.py | Adds ge_arrow as-used (cold-inclusive) end-to-end timing script. |
| benchmark/references/examples/ge_arrow/results/sweep.json | Adds captured sweep benchmark results for ge_arrow baseline. |
| benchmark/references/examples/ge_arrow/results/static_metrics.json | Adds captured metrics output used by the evidence/scorecard. |
| benchmark/references/examples/ge_arrow/results/scorecard.json | Adds computed scorecard output for ge_arrow baseline. |
| benchmark/references/examples/ge_arrow/results/equivalence.json | Adds captured equivalence results for ge_arrow baseline. |
| benchmark/references/examples/ge_arrow/results/benchmark.json | Adds captured benchmark results for ge_arrow baseline. |
| benchmark/references/examples/ge_arrow/ge_arrow_REPORT.md | Adds the worked ge_arrow evaluation report. |
| benchmark/references/examples/ge_arrow/evidence.json | Adds the worked ge_arrow evidence file consumed by the scorer. |
| benchmark/references/EVALUATION_FRAMEWORK.md | Adds the evaluation framework specification, anchors, and workflow. |
| benchmark/.claude-plugin/plugin.json | Bumps benchmark plugin version to 0.2.0. |
| .gitignore | Adds standard Python/macOS ignore patterns. |
| .claude-plugin/marketplace.json | Updates marketplace version to 0.2.0. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…/template Addresses Copilot review on #5: - rubric.py computes the verdict band from the rounded total, so the band always agrees with the number displayed (raw FP sums can land at 2.4999999999999996 for combinations that are exactly 2.50 in exact arithmetic; 797/78125 score combinations were affected) - rubric.py docstring points at ../../references/EVALUATION_FRAMEWORK.md - EVIDENCE_TEMPLATE.json _how cites the actual CLI form (scripts/scoring/score.py <lecture-dir> from the plugin root) Both committed scorecards regenerate unchanged (neither sits at a band edge). The x64-divergence guard in score_correctness is deliberately left as authored — rubric semantics stay with the standard's author. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Detailed logic review completed — all 17 scripts line-by-line, plus scripted cross-checks of every evidence value against its results-file source. Full write-up now lives in the PR as Verdict: logic sound, methodology fair, every traceable number consistent. Fairness properties verified: Fixed in
For @xuanguang-li's consideration (rubric/design territory — deliberately not changed here):
Also still open from the earlier Copilot round: the 🤖 Generated with Claude Code |
|
Final review completed (31-agent adversarially-verified pass: integration correctness traced path-by-path, measured duplication analysis, doc-overlap mapping, installed-plugin packaging, fresh-eyes diff review, and an execution agent that ran every documented command). Fixes landed in Two additional items for @xuanguang-li (found by the fresh-eyes pass; your files, your call):
Deferred simplification bundle (measured, verified outcome-preserving, but touching your scripts' structure — proposed for whenever a third example lands or the next re-measurement, whichever comes first):
Deliberately not proposed: sharing the median-timer/results-path/allclose helpers — measured as a net negative (couples the templates the adaptation model wants independent). 🤖 Generated with Claude Code |
Three guards from the PR #5 review, all closing the same failure class — a contract that was documented but not enforced: - validate_evidence() (review B3, B4): score.py now refuses evidence that omits a scored input the verdict gates read, or marks a structural criterion met without a citation. Both new v2 fields failed open: a missing baseline_as_used_seconds silently disarmed the no-conversion verdict, and stripping every citation left the score unchanged. as_used_runs must now be present; [] remains legal as an explicit single-run declaration. Both evidence files gain the key (score-neutral: same single-run path). The check is a separate pass over authored evidence, never inside a scorer, so score_all stays a pure function of evidence and the perturbation search never silently drops mutants that trip authoring checks. - Honest perturbation count (review E5): tested increments only after a successful scoring call, and perturbations that raise are recorded in perturbations_skipped with the exception instead of being silently counted in the denominator the stamp is judged on. - robust-at-floor (review C1, floor half): a verdict already in the bottom band cannot be perturbed downward, so zero deciding flips there is partly the band's geometry, not evidence strength. markov_asset now stamps robust-at-floor with the reason attached; SKILL.md carries the stamp verbatim into reports, so plain "robust" was asserting support the run never demonstrated. Scorecards regenerated: totals, verdicts, and gates unchanged; the diff is the new fields plus markov_asset's stamp wording. The rubric's floor comment also stops restating the measured baselines (review E2) and points at evidence.json as the value the gate reads. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Applied the review (items referenced by its numbering) in seven commits, Invariant held throughout: neither published evaluation moved — ge_arrow is still 2.85 / no-conversion / fragile, markov_asset still 2.25 / no-conversion / gated. All scorecards regenerate byte-identically (md5-checked across repeated runs); the only scorecard diffs are additive fields plus markov_asset's stamp wording (see C1 below).
Deferred, deliberately: C2 (floor keying on the baseline alone — a policy decision worth making explicitly, though note triage already decides from the baseline alone, so the asymmetry is a live review/triage disagreement), C5 (pin the run-pairing definition before any |
The 2026-07-25 external review of this PR, committed verbatim alongside the repo's other review records so the item numbers cited on #7 and #4 (C2, C4, C5, D, E1, E6, E7) resolve without the PR comment. A labeled disposition note maps items to what was applied (4fffbc9..8388e86) and where the remainder is tracked; the text is otherwise as received. Every checkable claim in it was reproduced against the branch before the fixes landed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Skill names here are objects because the plugin is the verb. That is the general form of the repo's convention — an invocation reads as a command — rather than a deviation from it: where the plugin is a namespace or domain (qe, benchmark) the verb has nowhere to live but the skill; where the plugin is the verb, the skill is the object. Also records why audit beat review. Review is already the per-item word here, so a /review:prs sweeping every open PR would sit beside /review assessing one — erasing the bulk-vs-single-item line the family is built on. The convention amendment itself belongs in docs/developing-skills.md, which arrives with #5; this is the rationale in the meantime. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
e7b6c18 to
53dee5b
Compare
…t:issues (#11) * Audit plugin: the bulk-audit family and its shared doctrine A third plugin for maintainer-facing work: portfolio-wide, read-only sweeps of one repository that deliver a report bundle. Membership is gated on three tests together — bulk, read-only, bundle output — which keeps single-item review outside the family and stops the plugin becoming a general runbook dump. The method is authored once at plugin level so the four planned skills share it rather than restating it: doctrine.md (trust rules, evidence classes, the read-only boundary, phased checkpointing, the coverage self-audit), quantecon-context.md (repo types, label ownership, the cross-repo graph, notes-system discovery, access), and deliverables.md (the bundle contract and where bundles may land). Read-only is structural rather than cautious. It is what makes the family safe to point at any repo and safe to run headlessly, and it keeps a report honest: an audit that half-applied its own findings would describe a repo that no longer exists. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Audit: deterministic tracker snapshot with coverage reconciliation Phase 1 of every audit is mechanical, so it runs as a script rather than as model judgement. fetch_tracker.py captures issues and PRs in any state with full comment threads in two round trips, which is what makes reading closed threads free — the one gap the first execution of the issue runbook found. The snapshot also freezes the audit's point in time, so every later phase reads one instant and "events after the snapshot" becomes a stated property of the report rather than an unnoticed gap. coverage.json does the mechanical half of the self-audit: items against the number sequence 1..max, thread counts split open/closed, and a truncation flag for a stream that returns exactly at the fetch limit — indistinguishable from truncation, so it is surfaced, not swallowed. Preflight refuses to start without gh and auth, because the anonymous API is 60 req/h per IP and returns nothing for the org's private repos. Validated against QuantEcon/action-translation: 111 issues + 110 PRs, numbers 1..221 fully accounted, 123 comments across 60 threads. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Audit: the issue-triage runbook as /audit:issues The runbook's four paste-in fields become optional arguments with documented discovery and stated fallbacks, so the common invocation is just the repo. Its numbered sections split along the doctrine boundary: the trust rules and coverage self-audit move to plugin level, and the skill keeps what is specific to issues — per-item verification, the finding categories to hunt, GC/T0-T3 tiering, and the link graph. QuantEcon adaptations over the generic runbook: tier by repo type, since a build break in a lecture repo and a consumer-visible change in an action repo outrank their thread activity; check siblings before concluding, because "resolved in a sibling" and "one step of a rollout" are the two wrong conclusions a single-repo audit reaches here; leave label application to qe gh labels; and keep GitHub closing keywords out of drafted cross-repo references, which would otherwise close the upstream item when the text lands. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Docs: register the audit plugin in the marketplace, README, and catalog Marketplace entry at 0.1.0, catalogue minor-bumped for the new plugin. CATALOG.md §3 records the family design and the evidence behind it, which is breadth rather than per-repo frequency: a tracker audit is a once-a-year event for one repo, but the org has ~245 of them and three of the four procedures have already been run by hand. audit is deliberately left out of the lecture-repo auto-install block. The plugin is the enable unit, so including it would put maintainer tooling in every author's command list — the same audience separation that keeps benchmark out of the qe prefix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Audit: state the naming rule, not a local exception Skill names here are objects because the plugin is the verb. That is the general form of the repo's convention — an invocation reads as a command — rather than a deviation from it: where the plugin is a namespace or domain (qe, benchmark) the verb has nowhere to live but the skill; where the plugin is the verb, the skill is the object. Also records why audit beat review. Review is already the per-item word here, so a /review:prs sweeping every open PR would sit beside /review assessing one — erasing the bulk-vs-single-item line the family is built on. The convention amendment itself belongs in docs/developing-skills.md, which arrives with #5; this is the rationale in the meantime. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Audit: harden the snapshot's provenance and capture claims Copilot review on #11. The `authenticated` field was derived by substring-matching `gh auth status` output, which preflight has already gated on — so it could only ever be true, but would silently record false if gh reworded or localized. A provenance field that can lie undermines the thing the snapshot exists for. preflight now returns True on its only non-exiting path and meta records that. Thread fields are shape-asserted at capture. Everything downstream assumes `comments`/`reviews` hold lists of objects; a gh build returning counts would have crashed later with a bare TypeError, and a differently-shaped payload would have under-reported threads while the report still claimed thread-completeness. Both now fail by name at the point of capture. The missing-gh error pointed at "the unauthenticated route documented in the skill", which read as an offer of a fallback this script does not implement. It now names the file, and states the two constraints that make it a real choice rather than a shrug. Negative-tested: counts and lists-of-non-objects rejected, empty and absent threads pass through. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Audit: re-home the HTML-thread caveat to the doc that owns it Copilot review on #11. Phase 3 of the issues runbook warned that a thread reconstructed from HTML can start mid-conversation — a caveat inherited from the source runbook's unauthenticated route, which phase 1 never takes now that the snapshot comes from gh. Left there it muddied the trust model: a reader could not tell which capture path the warning applied to. It moves to quantecon-context.md beside the fallbacks it describes, gaining the two things it was missing — that such claims are [stated] at best, and that an audit taking that route must say so in its coverage statement. Phase 3 keeps the range-reference caveat, which is about link parsing and applies to every path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Audit: count PR reviews in coverage, not just comments (review 1) The snapshot fetched `reviews` and shape-asserted them, then dropped them from every tally: `thread_stats` counted `comments` alone, so a PR whose whole conversation happened in review bodies reported as unread. On the runbook's own example repo that is not a corner case — closed PRs carry 374 reviews against 28 comments, so phase 5 was measuring 7% of the PR discussion from a number that read like all of it. Counters are now per-field, PRs split open/closed the way issues already were, and the terminal summary prints the PR rows it previously computed and discarded. Paying this before `/audit:prs` is written keeps one definition of captured discussion rather than two. The residue wording tightens with it. Review *bodies* are captured, so the gap left to disclose is inline line comments, which the list call does not return — named in scripts/README.md with the call that fetches them. * Audit: write the snapshot in number order (review 2) Items were written in whatever order `gh` returned them. That order is stable enough in practice, but it is undocumented and not ours — and the snapshot is the evidence every later phase and every cited claim rests on, so its layout should not be a property of the CLI version that produced it. Sorting by number makes two runs over an unchanged tracker byte-identical in issues.json, prs.json, and coverage.json, so a re-fetch diffs down to what actually changed on the tracker. Verified against the runbook's example repo. Not sorting JSON keys: `gh` already emits `--json` fields in a fixed order, so it would add a diff without adding a guarantee. * Audit: state the method as advice, not as contract Three pieces of scaffolding were written as requirements after a single execution of a single audit: the four-document bundle, the five phases, and "produces a bundle" as a membership test. One worked example is not enough to generalise from, and each would have forced the next skill to fit a shape derived from issue triage — a debt audit has no link graph to put in `03-links.md`, and a small audit should be free to be one document. So the shape becomes a worked example and the obligations stay. What an audit owes its reader is now its own short section — a coverage statement, an evidence tag per claim, recommendations marked as proposals, drafted comments marked unsent, a date and a named snapshot — and none of it presumes a file count. Doctrine §4 keeps the rule that earns its place (checkpoint each phase before starting the next, because long runs lose sessions) and demotes the five-phase division to what `/audit:issues` happened to need. Membership drops to two tests, bulk and read-only. Read-only is the one doing real work: it is a permission boundary, not a shape preference. The general half of the naming note goes too, now that the repo's rule is deliberately permissive; what stays is the choice specific to this plugin, `audit` over `review`. Candidate skills move from CATALOG.md to #12, and are marked candidate rather than planned — none has the evidence a skill here normally carries before it is written. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
… cases The quantitative evaluation system for lecture code rewrites developed and validated on QuantEcon/lecture-python.myst#717 and #654: - references/EVALUATION_FRAMEWORK.md — the standard in prose: 7 weighted dimensions, numeric scoring anchors, structural checklists, verdict bands, worked HIGH/LOW examples - scripts/scoring/ — the standard as code: rubric.py (deterministic evidence -> score), score.py (engine/CLI), EVIDENCE_TEMPLATE.json (the judgement contract: measured numbers + cited yes/no answers) - scripts/calibration/ — the shared aiyagari Bellman benchmark pinning the "25x as-used = score 5" efficiency anchor - references/examples/{ge_arrow,markov_asset}/ — two complete worked evaluations (measurement scripts, results, evidence, reports): ge_arrow 2.85/5 mixed/wash; markov_asset 2.25/5 net regression (build-breaking bug) Content as delivered 2026-07-21; placed at plugin-convention paths. Path/link integration follows in a separate commit.
Integration on top of the landed package (content authored by @xuanguang-li; this commit is path/plumbing only plus docs): - score.py takes a lecture directory path (works from any cwd) instead of a name resolved against the old package root - ge_arrow scripts use local imports (import model_old), matching the markov_asset idiom, so every script runs directly from its directory - run_all.py (both examples): scoring call updated to the new layout, lecture dir derived not hardcoded, and a provenance stamp written to results/env.json (python/platform/numpy/jax/quantecon versions) -- the seed of the QuantEcon/meta#335 shared result schema - All relative links in EVALUATION_FRAMEWORK.md and the two reports rewritten for the new layout (verified: no dangling references) - scripts/README.md rewritten: engine layout, the three-step scoring contract, the evaluate-a-new-lecture recipe - SKILL.md updated: system landed, operational procedure now points at the real engine/templates, worked cases become regression anchors - benchmark plugin 0.1.0 -> 0.2.0 (marketplace kept in sync) Verified: both example scorecards regenerate byte-identically from the new layout; scripts/validate.py green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…/template Addresses Copilot review on #5: - rubric.py computes the verdict band from the rounded total, so the band always agrees with the number displayed (raw FP sums can land at 2.4999999999999996 for combinations that are exactly 2.50 in exact arithmetic; 797/78125 score combinations were affected) - rubric.py docstring points at ../../references/EVALUATION_FRAMEWORK.md - EVIDENCE_TEMPLATE.json _how cites the actual CLI form (scripts/scoring/score.py <lecture-dir> from the plugin root) Both committed scorecards regenerate unchanged (neither sits at a band edge). The x64-divergence guard in score_correctness is deliberately left as authored — rubric semantics stay with the standard's author. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
From the detailed logic review of the evaluation scripts: - run_all.py (both examples) now captures the JSON lines printed by the fresh-process scripts (as_used_total, cold_start) into results/as_used.json / results/cold_start.json, with the derived as_used_speedup - the headline metric previously lived only on the console, though the docstrings already claimed aggregation - ge_arrow static_metrics: remove the duplicated "@" pattern that double-counted concept token hits (informational metric only; regenerated results: old.concept_token_hits 110 -> 105) - markov_asset static_metrics: rename statements_for_one_asset -> statements_for_one_result, matching EVIDENCE_TEMPLATE.json and the ge_arrow template vocabulary (values unchanged; results regenerated) Both scorecards regenerate byte-identically - no scored value changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
benchmark/references/examples/README.md explains both canonical cases in detail - the economics, the two implementations, where every evidence number comes from, and why each verdict is what it is - and records the 2026-07-21 line-by-line verification (scorecard byte reproduction, evidence-results cross-checks, rubric edge audit, fairness audit) plus the known caveats (M1 hand-curated readability inputs, m3 x64 stamping, n6 sweep asymmetry). These examples are the regression baseline for the skill; their accuracy is now auditable rather than asserted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
From the adversarially-verified final review (31-agent pass over the full PR): Robustness (both run_all.py files — all four confirmed by execution): - guard against JSON-scalar stdout lines (json.loads succeeds on `42`/ `null`; .get then raised AttributeError and aborted the pipeline) - track per-step returncodes; failed step titles now go into the provenance stamp so a partial run cannot claim full provenance for stale results - symmetric total_s guard in the as_used_speedup derivation (bare numpy-side index could KeyError) - warn on duplicate mode keys instead of silently overwriting Simplification (integrator-authored code only): - the byte-identical 18-line write_env block duplicated in both run_all.py files becomes shared scripts/scoring/env_stamp.py, invoked like score.py (-26 LOC net; the shared meta#335 schema now has one definition) Consistency: - gitignore the per-run generated results (as_used.json, cold_start.json, env.json) and annotate their doc citations as generated-not-committed (committing them faithfully is impossible here: the local env is jax 0.10.1 vs the reports' 0.4.35) - examples/README: fix 'logic 5' -> 4 (scorecard and its own arithmetic say 4; with 5 the listed scores sum to 3.00, not 2.85) - REPORT link labels updated to match their (already-correct) targets; markov REPORT's stale statements_for_one_asset key renamed - evidence.json _how strings and score.py's scorecard _note now cite scripts/scoring/... (scorecards regenerated; only the _note changed) - SKILL.md no longer restates the weight vector and verdict bands -- it points at EVALUATION_FRAMEWORK.md sections 1-2 and rubric.py, so recalibration cannot drift the copies - scripts/README: commands documented as running from the plugin root Both scorecards regenerate byte-identically under the updated engine; validate.py green; env_stamp smoke-tested including steps_failed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Docs: plugin guide, skill usage, repo setup — with triage mode validated
- benchmark/README.md: the plugin's user guide — review mode (session
walkthrough, report format), triage mode ("should this lecture be
converted?"), manual pipeline quickstart, plugin map
- SKILL.md: triage-mode section (the prospective subset: baseline
as-used total, pattern match against the calibrated poles, crossover
check, readability-cost forecast, and the weight-algebra decision
rule)
- docs/using-skills.md: consumer guide — setup paths, invocation forms,
report-first expectations, troubleshooting
- docs/developing-skills.md: contributor guide — layout, conventions,
dev loop, versioning, squash-merge/stacking and external-author
attribution patterns
- README.md: documentation index
Triage mode is empirically validated before being documented: blind
triage using only baseline-side data (fresh runs: ge_arrow 0.028s,
markov_asset 0.087s; committed calibration: aiyagari pattern 54.3s)
reproduces all three known verdicts (don't convert / don't convert /
convert), and correctly cannot predict conversion-quality defects
(markov_asset's build bug) — that scope limit is documented with it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Docs: fix command count and map-table link text
Addresses Copilot review on #6: the manual-install snippet is three
commands, not two; the benchmark map row's link text now matches its
target (scripts/README.md) with the engine path named in the
description instead.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Docs: scale the evidence discipline by output tier, not blanket
The evidence-file + engine pattern applies to skills that aggregate
judgements into scored verdicts; findings-list skills need only cited
claims. developing-skills.md gets the three-tier discipline; CATALOG
gains the principle.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Design review: corrections of record + merged three-way synthesis
Two independent design critiques of the evaluation system (a fresh
unframed session; a 36-agent adversarial workflow with steelman
defense) were merged in reviews/. Three of our own claims were
falsified by execution and are corrected here:
- markov_asset's lecture DOES build in notebook order: a stale global
err masks the stray err.throw(), silently disabling the checkify
stability validation (worse than a crash, but not a build failure).
Erratum prepended to the REPORT; wording corrected in examples
README, SKILL.md, plugin README; correction posted on
lecture-python.myst#654
- "mirrors the lecture exactly": both reference replays deviate from
the lectures' construction patterns; certification corrected
- "medians over repeats": false for the as-used totals (single pass
per side); fairness-audit wording corrected; triage validation
noted as in-sample
reviews/ holds the independent report and the merged synthesis:
which critiques survived the steelman defense (convention-only safety
couplings; readability instrument inverting its own exemplars; the
review/triage mode contradiction; band-label semantics; noise vs
resolution) and which were defended (ratio form, weighted total,
min() shape, per-consequence billing), plus the ranked v2 plan.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Update references after withdrawal of the #654 comments
The evaluation findings briefly posted to lecture-python.myst#654 were
withdrawn; the PR will receive one authoritative evaluation after the
rubric-v2 revision and a full skill run, rather than a
comment-and-correction trail. Erratum and merged review updated.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…mp, K-repeat Implements items 1-4 of #7 (the changes that survived the three-way design review), validated against both committed evidence files: - score_all derives the logic&design bug-cap from the correctness evidence (builds / x64-divergence) instead of trusting a hand-set boolean, and gates the verdict: correctness 1 caps at net regression, correctness 2 at mixed/wash. The review's honest-evidence 4.2 hole (float32 catastrophe, no logic bug -> "merge") now gates to net regression. - no-conversion verdict: baseline as-used under the 1 s materiality floor (a labeled policy choice) + slower as-used candidate -> the scorecard says don't convert instead of scoring the polish. Reconciles review with triage. - sensitivity stamp in score.py: every scored input perturbed one at a time (bools flipped, counts +/-1, floats +/-10%); scorecard stamped robust/fragile with deciding flips listed. - K-repeat as-used: run_all.py repeats each as-used side 3x in fresh processes; the headline speedup is a median, per-run speedups feed a contested-band annotation in the engine. Re-validation: ge_arrow re-scores 2.85 (unchanged), verdict now no-conversion (candidate band mixed/wash), stamped fragile with exactly the review's demonstrated flips. markov_asset re-scores 2.25 (unchanged), no-conversion + gated net regression, stamped robust across all 29 perturbations - the gate absorbs the one-concept band flip the review demonstrated. Both band movements are deliberate v2 changes, noted in the reports. Benchmark plugin bumped to 0.3.0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Operationalizes /benchmark:review-acceleration for installed-plugin runs (#4 item 3): evaluations are built under <workspace>/benchmark-eval/<lecture>/ with the plugin read-only at CLAUDE_PLUGIN_ROOT (run_all.py already resolves the shared engine from that env var); preconditions stated up front; extraction/replay diff check added to scaffold; the v2 verdict outputs (gates, no-conversion, sensitivity stamp) carried through the procedure, triage decision rule, and calibration anchors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tplace Adds a Testing locally section to the contributor guide: --plugin-dir for skill iteration, a local-path marketplace for full install simulation before merging (test from a consuming project; the checkout's branch is what gets served; the marketplace name collides with production), and the remove/add/install sequence to return to the GitHub source afterwards. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…reproduced The skills#8 dry run, targeting the motivating PR the system was first developed on (lecture-python.myst#717) rather than the reserved #654 acceptance case. Fresh partial clone at base 8cfba4c / head 8c2d0d7, wired workspace procedure (benchmark-eval/<lecture>/ with CLAUDE_PLUGIN_ROOT), jax 0.10.1 vs the reference 0.4.35: reproduces 2.85 / no-conversion / fragile with the same three deciding flips; every measured quantity moved only within its band. Full cross-comparison in reviews/validation-run-ge_arrow-2026-07-22.md. Fixes surfaced by the run: - ge_arrow check_equivalence.py now writes equivalence_x64.json under JAX_ENABLE_X64 instead of clobbering the as-shipped results (the markov_asset template already did this per-regime) - model_old.py fidelity note discloses the cosmetic whitespace normalisation found by diffing against a fresh extraction - both evidence files now record source_pr + base/head SHAs (provenance was previously PR-number-and-branch only) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Walks the review-acceleration procedure end-to-end by hand - checkout at the recorded SHAs, workspace scaffold, K-repeat measurement, the two precision regimes and why each exists, evidence, scoring, band-based cross-comparison - with every command and number taken from the recorded validation run so readers can check their results against a committed reference. Linked from the README docs table and the benchmark guide. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… truth Adds the repo-level instructions file for AI agents and contributors, following the QuantEcon.manual convention: AGENTS.md is canonical and CLAUDE.md imports it. Its governing principle is @jstac's — skills point to existing documentation in the manual wherever possible instead of repeating what the manual says — worked out concretely for this repo: rule text stays upstream in style-guide, numbers live once, every topic has an owning doc, and cross-boundary references are links rather than copies. The rest of the file is a doc map plus the conventions that aren't written down anywhere else (commit subjects, scratch notes, writing for the GitHub renderer). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Three install-surface bugs surfaced by @xuanguang-li while testing the benchmark plugin (#10): - marketplace.json omitted the required top-level `owner` object, so `/plugin marketplace add` failed schema validation for every user. - Both plugins declared a remote github source pointing back at this repo, forcing an install-time re-clone over SSH (ED25519 failure) to reach a subdirectory already present in the added marketplace copy. Switched to the documented co-located pattern — relative-path sources (`./qe`, `./benchmark`) — so install uses the local copy: no SSH, no auth prerequisite. - validate.py never checked `owner` and assumed an object source, so it passed a manifest the installer rejects. It now requires `owner.name`, resolves the relative-path source form, and hard-fails any plugin whose source points back at this repo. Also documents the version-gated `/plugin:skill` slash form (v2.1.216+) and the natural-language fallback in docs/using-skills.md. Marketplace version 0.1.0 -> 0.1.1. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The resolve_source refactor removed the `path` local from check_plugin, but three error strings still referenced it — so the name-mismatch, no-skills-dir, and empty-skills branches raised NameError instead of printing the diagnostic the validator exists to print. CI stayed green only because a healthy tree never enters those branches (review A1). Hoist the repo-relative path once after resolve_source returns and reuse it everywhere. Both reachable branches verified by hand: deleting benchmark/skills/ and emptying it now yield the intended one-line diagnostics with exit 1. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
EVALUATION_FRAMEWORK.md and ge_arrow_REPORT.md were CRLF in an otherwise-LF repo, so any future edit of either would render as a whole-file diff burying the real change (review E3). Pure normalization — zero content change, verified with `git diff --ignore-cr-at-eol`. The .gitattributes makes the normalization structural instead of a contributor convention. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three guards from the PR #5 review, all closing the same failure class — a contract that was documented but not enforced: - validate_evidence() (review B3, B4): score.py now refuses evidence that omits a scored input the verdict gates read, or marks a structural criterion met without a citation. Both new v2 fields failed open: a missing baseline_as_used_seconds silently disarmed the no-conversion verdict, and stripping every citation left the score unchanged. as_used_runs must now be present; [] remains legal as an explicit single-run declaration. Both evidence files gain the key (score-neutral: same single-run path). The check is a separate pass over authored evidence, never inside a scorer, so score_all stays a pure function of evidence and the perturbation search never silently drops mutants that trip authoring checks. - Honest perturbation count (review E5): tested increments only after a successful scoring call, and perturbations that raise are recorded in perturbations_skipped with the exception instead of being silently counted in the denominator the stamp is judged on. - robust-at-floor (review C1, floor half): a verdict already in the bottom band cannot be perturbed downward, so zero deciding flips there is partly the band's geometry, not evidence strength. markov_asset now stamps robust-at-floor with the reason attached; SKILL.md carries the stamp verbatim into reports, so plain "robust" was asserting support the run never demonstrated. Scorecards regenerated: totals, verdicts, and gates unchanged; the diff is the new fields plus markov_asset's stamp wording. The rubric's floor comment also stops restating the measured baselines (review E2) and points at evidence.json as the value the gate reads. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The x64-divergence cap required max_delta_shipped > 1e-8 as well, in both score_correctness and the derived logic&design bug cap. That conjunct made the guard structurally unable to fire when the shipped float32 delta is small — which is exactly the "wrong economics masked by low precision" case the framework's correctness section names as the thing this dimension guards. A candidate with divergent logic and a lucky 1e-12 shipped delta scored correctness 5 and total 3.25; it now scores correctness 1, logic_design capped at 3, total 2.30 gated to net regression. The flag's semantics come from the repo's own usage: the worked examples record TRUE for x64-noise residuals (~1e-14 to ~1e-11), so FALSE asserts the economics genuinely differ — not a failure of bitwise identity. Under that reading, agreement as shipped is luck, not correctness, and the cap needs no second condition. No committed scorecard changes: both worked examples have max_delta_shipped > 1e-8, so they were already caught by the old conjunct — verified byte-identical regeneration under both engines. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Neither worked example exercises the v2 headline features: both take the single-run efficiency fallback (no as_used_runs), and markov_asset hand-sets the correctness-bug flag, so the derived-cap path never runs on committed data. The newest scoring behaviour was tested only by hand-probing. references/fixtures/rubric_v2 is a synthetic evidence file — not an evaluation — whose one job is to make five untested paths execute on every scoring run: the unconditional x64 cap, the derived bug cap firing against a hand-set FALSE, the as_used_runs median, the contested-band annotation (runs straddle the 1.3x edge), and a verdict gate reporting the ungated total. The baseline sits above the 1 s floor on purpose so no-conversion does not mask the paths under test; every source string starts SYNTHETIC: so nobody cites the numbers as evidence about a lecture. Kept out of references/examples/ because an example records what was measured about a real PR and a fixture is a test input — conflating them invites citing synthetic data. Its own sensitivity line documents the stake: flipping matches_under_x64 alone swings the outcome from gated net regression to 4.70 clear improvement. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The system's central claim — no score is ever written by hand — was verified only in reviewers' terminals. CI now regenerates all three committed scorecards (both worked examples and the v2 fixture) and fails on any diff, which catches a hand-edited scorecard, an unintended scoring change, and — the important case — an intended scoring change whose baselines were not regenerated. There the failure is desirable: the fix is to re-run and commit, which forces the verdict-moving diff into the PR where a reviewer sees it. Stdlib only, so no install step. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review E2/E4 plus the doc side of the engine changes: - The 0.028 s / 0.087 s vs 0.035 s / 0.18 s discrepancy was two honest measurements of the same quantity presented as one. The README table now labels its column triage-time (2026-07-21) and says the gate reads each lecture's own baseline_as_used_seconds; the framework and SKILL.md stop restating the numbers and point at evidence.json (review E2). - README quotes markov_asset's verdict as the scorecard emits it — no-conversion with the banded quality alongside — instead of the stale "2.25 net regression" (review E4). - Framework Sec. 1 states the unconditional x64 cap with its rationale, the three stamp values including robust-at-floor, and records the measured-vs-adjudicated conflation in the perturbation walk as a known limit for v3: read the deciding-flip list, not the stamp alone. - SKILL.md step 5 forbids reporting robust-at-floor as robust; sanity anchors and the tutorial's quoted output line updated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The 2026-07-25 external review of this PR, committed verbatim alongside the repo's other review records so the item numbers cited on #7 and #4 (C2, C4, C5, D, E1, E6, E7) resolve without the PR comment. A labeled disposition note maps items to what was applied (4fffbc9..8388e86) and where the remainder is tracked; the text is otherwise as received. Every checkable claim in it was reproduced against the branch before the fixes landed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
"Verb-first skill names" reads as a rule about skills, but the thing it was protecting is that the invocation scans as an imperative — and the whole `/plugin:skill` string is what a user types. `/audit:issues` satisfies that with the verb in the plugin, yet fails the rule as written, so the rule had to move. It moves by getting weaker, not by growing a taxonomy. Both shapes are shown, neither is preferred, and the absence of a preference is stated so a later contributor reads it as undecided rather than unspecified. Ranking them now would mean generalising from three plugins and no usage; the question is parked in FUTURE-IDEAS.md with the specific thing worth watching — whether mixing shapes inside one plugin actually causes trouble or merely looks untidy. Depends on #11 for its example: `audit` must land before this does.
Two changes, one idea: this repo is three plugins old, and its docs were describing more certainty than it has. CATALOG.md becomes a list of what is merged and runnable, with each plugin's state stated honestly (qe is scaffolding and says so) and a tracking issue beside it. It was previously the active plan, which meant it described skills nobody could run and drifted from the repo every time work moved. Plans now live in the issues — #3, #4, and #12 for the audit plugin — where they can change without anyone mistaking them for a description of what exists. README.md drops its duplicate plugin table and points here; the qe sub-skills point at #3 rather than at a catalogue entry that no longer carries their plan. The conventions are reframed as guidance. developing-skills.md now says so at the top, says only SKILL.md is required — a skill with nothing mechanical to run should be one file, not a directory tree — and marks the three conventions that are genuinely load-bearing, each with its reason. The rest is what one or two examples happened to need, and a contributor with a reason to depart should depart and say so in the PR, because a second example is how any of this becomes a real convention. The test that separates the two: a rule earns firmness when it keeps a skill's output checkable by someone who will not re-run it. Report-first, cited claims, and the plugin-root constraint pass it. Report shapes, phase divisions and naming forms do not.
Post-#11 reconciliation. README gains audit in the plugin table and the documentation index, and its layout note drops "bundle contract", which the plugin no longer has. The marketplace entry's move to the co-located `./audit` form is not here — it belongs in "Install fix: co-located plugin sources", which is the commit that established that form, and the rebase carried it there.
fe0ab47 to
ac29cbb
Compare
|
@xuanguang-li I am going to merge this PR to keep working on this in the context of the repo in general. The only change may be repointing to |
Lands @xuanguang-li's complete evaluation system for lecture code rewrites — the deliverable #4 was waiting on, developed and validated on QuantEcon/lecture-python.myst#717 and QuantEcon/lecture-python.myst#654. Two commits, deliberately split: commit 1 is the package as delivered, authored by @xuanguang-li; commit 2 is path/plumbing integration plus docs.
What lands
references/EVALUATION_FRAMEWORK.mdscripts/scoring/rubric.py(evidence → score, deterministically — never hand-typed),score.py(CLI),EVIDENCE_TEMPLATE.json(the judgement contract: measured numbers + cited true/false answers)scripts/calibration/references/examples/ge_arrow/references/examples/markov_asset/The two worked cases double as the regression baseline: the skill must reproduce their verdicts. Verified in this PR — both scorecards regenerate byte-identically from the new layout via
python scripts/scoring/score.py references/examples/<lecture>.Integration (commit 2)
score.pytakes a lecture directory path (runs from any cwd); ge_arrow scripts converted to local imports matching the markov_asset idiomrun_all.pywrites a provenance stamp (results/env.json: python/platform/library versions) — the seed of the PROJECT: QuantEcon benchmarking programme (code performance & execution) meta#335 shared result + environment-descriptor schemascripts/README.mdrewritten around the three-step contract (measure → record evidence → score); SKILL.md procedure now points at the real engine and templates, with the worked cases as calibration anchors0.1.0 → 0.2.0, marketplace kept in sync;validate.pygreenDesign note
Per-lecture measurement scripts are adapted templates, not a fixed harness — adapting them to each lecture's examples and call sequence is exactly the step the skill automates, while scoring stays deterministic. This supersedes the "generalised CLI" idea in #4 item 2.
🤖 Generated with Claude Code
Update 2026-07-22 — docs merged in, rubric v2, skill wired (plugin 0.3.0)
This PR now also carries three follow-ups so the whole system can be tested from this branch before landing on
main:2aea122): user/developer docs, the benchmark user guide, validated triage mode, the design-review record (reviews/), and the markov_asset corrections of record.719e825) — the four engine quick-wins from PLAN: evaluation rubric v2 — enforce couplings, verdict vocabulary, instrument fixes (from the three-way design review) #7 that survived the three-way design review, each validated against the committed evidence files: safety couplings enforced inscore_all(derived logic&design bug-cap; correctness 1/2 gates the verdict band — the review's honest-evidence 4.2 hole now gates to net regression), the no-conversion verdict (reconciles review mode with triage), a one-flip sensitivity stamp (robust/fragile with deciding flips), and K-repeat as-used (median of 3 fresh-process runs with contested-band annotation).a36a744): evaluations run in the user's workspace (benchmark-eval/<lecture>/) with the plugin read-only atCLAUDE_PLUGIN_ROOT; preconditions and the extraction/replay diff check added to the procedure.Re-validation: both totals are unchanged (ge_arrow 2.85, markov_asset 2.25); both verdicts now lead with no-conversion, markov_asset's additionally gated at net regression. ge_arrow stamps fragile (the review's demonstrated flips, listed in the scorecard); markov_asset stamps robust — the v2 gate absorbs the one-concept band flip the review demonstrated. Band movements are documented as deliberate v2 changes in both reports. Plugin
0.2.0 → 0.3.0, marketplace in sync,validate.pygreen.Update 2026-07-25 — review hardening (7 commits)
An item-by-item external review of this PR was applied in
4fffbc9..8388e86; the full mapping from review items to changes is in the response comment. Headlines: the validator reports manifest errors instead of raisingNameError; CI now regenerates all committed scorecards from evidence and fails on any diff;score.pyvalidates evidence before scoring (a missing gate input or an uncited met-criterion is an error, closing the fail-open paths);matches_under_x64caps correctness at 1 unconditionally (the Copilot x64 thread, now resolved there); a synthetic fixture (references/fixtures/rubric_v2) pins the five previously-untested v2 paths on every CI run.One correction to the 2026-07-22 update above: markov_asset's stamp is now robust-at-floor, not robust — zero deciding flips at the bottom band is partly the band's geometry (nothing can push a floored verdict lower), and the scorecard now says so rather than overclaiming. Totals and verdicts are otherwise unchanged: ge_arrow 2.85, markov_asset 2.25, both leading with no-conversion. Deferred methodology items are filed on #7, remaining housekeeping on #4.