diff --git a/CATALOG.md b/CATALOG.md index 20aced4..178b6d3 100644 --- a/CATALOG.md +++ b/CATALOG.md @@ -7,6 +7,7 @@ Current focus, validated against ~630 merged PRs across the four main lecture re - **Few, high-frequency skills** over many niche ones; every skill is validated against actual PR history. - **Focal point is PR management**: consistent review results, plus the same checks run by authors on a working copy before opening a PR. - **Report first, fix on request.** Skills produce a structured report and offer fixes; they never silently edit. Safe in CI, authors stay in control. +- **Cited claims; computed scores.** Every finding carries a citation (rule ID + `file:line`, or a number + its source). Skills whose output is a findings list need nothing more. Skills that aggregate judgements into a scored verdict use the evidence-file pattern from the benchmark plugin: judgement recorded as cited answers, every score computed by a deterministic engine, never typed by hand (see [docs/developing-skills.md](docs/developing-skills.md)). ## 1. Style skill family — flagship diff --git a/README.md b/README.md index c01cce3..7b3fed6 100644 --- a/README.md +++ b/README.md @@ -13,6 +13,14 @@ Each plugin bundles one area of work — a skill (the instructions Claude follow The `qe` plugin is the author-facing surface: one memorable prefix for the skills authors use while editing lectures and preparing PRs. `check-style` is the umbrella (whole lecture, optional category filter, e.g. `/qe:check-style lectures/aiyagari.md figures math`); the per-category sub-skills run the same shared rules individually. `benchmark` is a specialist family for maintainers evaluating accelerated implementations. See [CATALOG.md](CATALOG.md) for the plan and [FUTURE-IDEAS.md](FUTURE-IDEAS.md) for parked candidates. +## Documentation + +| Guide | For | +|---|---| +| [docs/using-skills.md](docs/using-skills.md) | Authors/reviewers: setup, invoking skills, what to expect | +| [docs/developing-skills.md](docs/developing-skills.md) | Contributors: layout, conventions, dev loop, versioning, PR flow | +| [benchmark/README.md](benchmark/README.md) | The evaluation skill: review mode, triage mode, report format, manual pipeline | + ## Installation ### Automatic (lecture repos) diff --git a/benchmark/README.md b/benchmark/README.md new file mode 100644 index 0000000..bef83cd --- /dev/null +++ b/benchmark/README.md @@ -0,0 +1,77 @@ +# benchmark plugin + +Evaluation tooling for QuantEcon lecture code rewrites — the question it answers is never "is JAX faster?" but **"does this implementation earn its place in this lecture?"** Lectures are teaching materials first and programs second; the plugin's rubric weights readability (0.25) above efficiency (0.15) on purpose. + +One skill, two modes: + +| Mode | Question | Needs | Produces | +|---|---|---|---| +| **Review** | Did this conversion PR improve the lecture? | baseline + candidate implementations | A scored report with a merge recommendation | +| **Triage** | Is this lecture worth converting at all? | the existing lecture only | A predicted verdict band with the binding constraint named | + +Status: evaluation system landed (v0.2.0); skill wiring tracked in [skills#4](https://github.com/QuantEcon/skills/issues/4). The system was developed and validated by [@xuanguang-li](https://github.com/xuanguang-li) on [lecture-python.myst#717](https://github.com/QuantEcon/lecture-python.myst/pull/717) and [#654](https://github.com/QuantEcon/lecture-python.myst/pull/654). + +## Using the skill + +``` +/benchmark:review-acceleration # review mode +/benchmark:review-acceleration should we convert ? # triage mode +``` + +### Review mode — what you get + +The skill follows the measure → record-evidence → score contract ([scripts/README.md](scripts/README.md)): it extracts both implementations verbatim from the lecture's code cells, adapts the measurement templates, runs them, fills `evidence.json` with cited answers, and lets the engine compute the verdict — **no score is ever typed by hand**. The session shows the engine's derivation table (every score with the measured number and threshold band that produced it), and the final report follows the worked examples' format: + +1. **TL;DR** — weighted score, verdict band, the decisive facts in one paragraph +2. **Dimension table** — weight / score / weighted contribution / one-line driver each +3. **What changed** — before/after implementation shape +4. **Evidence by dimension** — `max|Δ|` in both dtype regimes, prerequisite-concept and docstring deltas, the as-used vs warm timing table, crossover-n, recompile audit +5. **Recommendation** — a must-fix list where each item is tagged with the dimension it lifts, plus where the score lands after fixes + +See [references/examples/ge_arrow/ge_arrow_REPORT.md](references/examples/ge_arrow/ge_arrow_REPORT.md) (2.85/5, mixed/wash) and [references/examples/markov_asset/markov_asset_REPORT.md](references/examples/markov_asset/markov_asset_REPORT.md) (2.25/5, net regression) for complete real reports. Verdict bands, weights, and scoring anchors: [references/EVALUATION_FRAMEWORK.md](references/EVALUATION_FRAMEWORK.md) §1–2. + +**The one rule to remember:** warm-only speedups are never the headline. The ge_arrow case measured 1.4–4.8× faster warm and **45× slower as-used** — the as-used number (fresh process, actual problem sizes, compile time included) decides the efficiency score. + +### Triage mode — before any code is written + +Four checks, using only the existing lecture: + +1. **Baseline as-used total** — replay the lecture's real call sequence (the NumPy half of an `as_used_total.py` template). This bounds the entire possible win: a lecture whose compute totals 30ms has nothing to give. +2. **Workload-pattern match** — against the two calibrated poles: **aiyagari-shaped** (large fixed-shape arrays, many re-solves, stable static args → measured ~24× as-used win) vs **ge_arrow-shaped** (tiny models, fresh static args per call → measured ~45× as-used loss). +3. **Crossover comparison** — the lecture's problem sizes vs the warm crossover-n from the scaling data. +4. **Readability-cost forecast** — which concepts the conversion would force on readers (static args, `lax` carries, checkify, the float32/x64 distinction), against the prerequisite-concept bands. + +Then the decision rule that falls out of the rubric weights: efficiency (0.15) can gain at most +0.30 weighted (band 3→5), while readability (0.25) losing two bands costs −0.50 — **a conversion that costs meaningful readability cannot break even on speed alone**; it must also win on logic & design and ergonomics, and those structural wins are usually achievable in plain NumPy. + +**Validation (2026-07-21):** triage applied blind (baseline-side data only) to the three known cases reproduces every known verdict: + +| Case | Baseline total | Pattern | Triage says | Full evaluation said | +|---|---|---|---|---| +| ge_arrow | 0.028 s | n=2/3, fresh static args | don't convert | 2.85 wash; 45× slower as-used | +| markov_asset | 0.087 s | n=5/25, LAPACK-bound | don't convert | 2.25 net regression | +| aiyagari pattern | 54.3 s | 200×7 fixed, 20 re-solves | convert | 23.8× as-used win | + +Scope limit, confirmed by the same test: triage predicts whether the prize is worth pursuing — it cannot predict conversion-quality outcomes (markov_asset's masked `err.throw()` defect was a property of the PR, invisible to triage). Note also that this validation is **in-sample** — the three cases are the ones the thresholds were calibrated on; out-of-sample validation accumulates as fresh lectures are triaged. + +## Manual usage (no skill) + +The full recipe is in [scripts/README.md](scripts/README.md) ("Evaluating a new lecture"); quickstart from this directory: + +```bash +conda activate quantecon +python references/examples//scripts/run_all.py # measure + provenance stamp +# fill references/examples//evidence.json (numbers + cited yes/no answers) +python scripts/scoring/score.py references/examples/ +``` + +Sanity anchors: re-running either worked example must reproduce **2.85** / **2.25**. + +## Map + +| Path | What | +|---|---| +| [skills/review-acceleration/](skills/review-acceleration/SKILL.md) | The skill (procedure, both modes) | +| [scripts/README.md](scripts/README.md) | Deterministic engine (`scripts/scoring/`): rubric, scorer, evidence template, provenance stamp | +| [scripts/calibration/](scripts/calibration/bellman_bench.py) | The shared HIGH-efficiency anchor (~24× ⇒ score 5) | +| [references/EVALUATION_FRAMEWORK.md](references/EVALUATION_FRAMEWORK.md) | The standard in prose — weights, anchors, checklists, verdict bands | +| [references/examples/](references/examples/README.md) | Two complete worked evaluations + the logic-check/provenance audit; the regression baseline | diff --git a/benchmark/references/examples/README.md b/benchmark/references/examples/README.md index 2355fc5..92a3e24 100644 --- a/benchmark/references/examples/README.md +++ b/benchmark/references/examples/README.md @@ -66,7 +66,7 @@ Lucas-tree, consol, and call-option pricing over a Markov chain (default: 25-sta ### The bug and the near-critical precision finding -- `smoke_test.py` demonstrates the crash exactly as the lecture calls it: `call_option_jit(...)` → `NameError: name 'err' is not defined`. Two lecture cells (consol/call-option cell, Exercise 1) depend on it → **the lecture does not build as shipped** → correctness 1 by the does-not-build override. +- `smoke_test.py` demonstrates the failure in a clean namespace: `call_option_jit(...)` → `NameError: name 'err' is not defined` → correctness 1 by the does-not-build override, under the system's fresh-process measurement regime. **Corrected 2026-07-21:** the notebook itself executes in cell order — earlier cells bind a global `err` that the stray `err.throw()` silently resolves to, which means the checkify stability validation is *never actually performed* in the shipped lecture (a masked failure rather than a crash); a reader copying the function into a clean namespace hits the `NameError`. See the REPORT's erratum. - `check_equivalence.py` additionally compares a **bug-patched copy** (verified line-identical to shipped logic minus the stray `err.throw()`) to establish that the *intended* logic is right: under x64 every asset matches NumPy to ≈1e-11. As shipped (float32) drift reaches **1.02e-2** on the exercise model — and that model's spectral radius (1.0618) sits **0.002 below** the stability bound 1/β = 1.0638, so float32 is not merely imprecise but close to flipping the stability check itself. Regimes are stored separately (`equivalence_x64_{True,False}.json`) with the x64 flag stamped in `_meta` — the pattern ge_arrow's template should adopt (caveat m3). ### The measurements @@ -102,7 +102,7 @@ correctness 1 (does not build) + readability 2 + efficiency 2 + logic 3 (capped) 2. **Scorecard reproduction:** `score.py` regenerates both committed scorecards **byte-identically** from `evidence.json` alone. 3. **Evidence↔results cross-check (scripted):** every quantitative evidence slot matches its results-file source (Δprereq, docstring coverage, max\|Δ\| shipped, crash record, statements). 4. **Rubric edge audit:** brute force over all 5⁷ score combinations found FP band-edge misclassifications (797 cases), fixed by computing the verdict from the rounded total; neither reference case was affected. -5. **Fairness audit:** `block_until_ready` on every JAX timing; fresh processes for as-used; medians over repeats; identical call sequences per side; disclosed patches only where timing is otherwise impossible. +5. **Fairness audit:** `block_until_ready` on every JAX timing; fresh processes for as-used; medians over repeats in the warm/scaling benchmarks (**correction:** the as-used totals themselves are single passes per side — a known limitation, tracked for v2); identical call sequences per side at the level the replay scripts encode (**correction:** later review found both replays deviate from the lecture in construction patterns — see the design-review documents in `reviews/`); disclosed patches only where timing is otherwise impossible. ## Known caveats (recorded, deliberate, or pending upstream) @@ -111,5 +111,5 @@ correctness 1 (does not build) + readability 2 + efficiency 2 + logic 3 (capped) | **M1** | `n_prerequisite_concepts` (readability driver, weight 0.25) and `statements_for_one_result` (ergonomics) are **hand-curated judgements encoded in the measurement scripts**, disclosed as such — not AST measurements. When the skill adapts templates to a new lecture it authors these lists, so they need the same citation discipline as the structural checklists. Proposed: move them into `evidence.json` as cited judgement slots. | Raised with the system's author (@xuanguang-li) on skills PR #5 | | m3 | ge_arrow's `equivalence.json` does not stamp the x64 regime and is overwritten between regimes; markov_asset's split-file pattern (`equivalence_x64_{bool}.json` + `_meta`) is the better template | Adopt on next template revision | | n6 | `sweep_bench` asymmetry: the old sweep computes only α (constructor + `wealth_distribution`), while the new one-call API forcibly computes everything — faithful to each API as used, but part of the measured sweep disadvantage is API-induced | Documented; by design (the API's cost is real) | -| — | markov_asset's as-used timing requires the bug-patched `call_option`; the shipped code cannot complete at all | Disclosed in script, output record, and report | +| — | markov_asset's as-used timing requires the bug-patched `call_option`; the shipped code cannot complete in a clean namespace (in notebook order it runs with the validation silently masked — see the REPORT erratum) | Disclosed in script, output record, and report | | — | Benchmarks are CPU-only; timings vary ±~15% run-to-run — the rubric keys on orders of magnitude | Framework limitation note | diff --git a/benchmark/references/examples/markov_asset/markov_asset_REPORT.md b/benchmark/references/examples/markov_asset/markov_asset_REPORT.md index b52967f..7d903b7 100644 --- a/benchmark/references/examples/markov_asset/markov_asset_REPORT.md +++ b/benchmark/references/examples/markov_asset/markov_asset_REPORT.md @@ -1,5 +1,7 @@ # Evaluation Report — `markov_asset.md`: NumPy (`main`) → JAX (`update_markov_asset`) +> **Erratum (2026-07-21).** This report states that "the lecture does not build as shipped." Design review later established this is false *as worded*: executing the notebook's cells **in order** completes cleanly (and the PR's preview CI passes), because earlier cells bind a global `err` immediately before `call_option_jit` is first traced, so the stray `err.throw()` inside `call_option` resolves to that stale, already-checked object and silently does nothing. The defect is real and arguably worse than a crash: the stale-global masking means the checkify stability validation inside `call_option` is **never actually performed** in the shipped lecture — on the model whose spectral radius sits 0.002 below the stability bound — and any reader copying the function into a clean namespace hits the `NameError` this report describes. The evaluation's `builds: false` refers to fresh-process/clean-namespace execution of the extracted implementation (the system's declared measurement regime); the verdict and must-fix list stand. Nothing has been posted to [lecture-python.myst#654](https://github.com/QuantEcon/lecture-python.myst/pull/654) — the PR will receive one authoritative evaluation once the rubric-v2 revision lands and the skill has run the full protocol. See the design-review documents in `reviews/`. + Applies the system in [`../../EVALUATION_FRAMEWORK.md`](../../EVALUATION_FRAMEWORK.md) to the NumPy→JAX conversion of `markov_asset.md`. All numbers are reproduced by `scripts/run_all.py` (CPU, jax 0.4.35, numpy 2.1.3, quantecon) into `results/`. Every dimension score is **computed from [`evidence.json`](evidence.json) by the shared rubric** (`../../../scripts/scoring/rubric.py`) — see `results/scorecard.json` for the derivation. ## TL;DR — weighted score **2.25 / 5** → *net regression as shipped: do not merge until fixed* diff --git a/benchmark/skills/review-acceleration/SKILL.md b/benchmark/skills/review-acceleration/SKILL.md index d1476f1..3d527af 100644 --- a/benchmark/skills/review-acceleration/SKILL.md +++ b/benchmark/skills/review-acceleration/SKILL.md @@ -23,12 +23,23 @@ Given a baseline implementation (usually `main`) and a candidate (usually a PR b Never present warm-only speedups as the headline — the ge_arrow case measured 1.4–4.8× faster warm and ~45× slower as-used. +## Triage mode (no candidate yet) + +When the question is "should this lecture be converted at all," run the prospective subset — only the existing lecture is needed: + +1. **Baseline as-used total**: adapt just the baseline half of an `as_used_total.py` template and replay the lecture's real call sequence — this bounds the maximum possible win (a 30 ms lecture has nothing to give). +2. **Pattern-match** against the calibrated poles: aiyagari-shaped (large fixed shapes, many re-solves, stable static args → ~24× win) vs ge_arrow-shaped (tiny models, fresh static args per call → ~45× loss). +3. **Crossover check**: the lecture's problem sizes vs warm crossover-n. +4. **Readability-cost forecast**: which prerequisite concepts the conversion would force. + +Decision rule from the weights: efficiency (0.15) gains at most +0.30 weighted; readability (0.25) losing two bands costs −0.50 — a conversion that costs meaningful readability cannot break even on speed alone, and structural wins are usually achievable in the baseline library. Report a predicted verdict band with the binding constraint named, not a scorecard. Validated 2026-07-21: blind triage on ge_arrow (0.028 s → don't convert), markov_asset (0.087 s → don't convert), and the aiyagari pattern (54.3 s → convert) reproduced all three known verdicts; triage cannot predict conversion-quality defects (markov_asset's build bug), and must say so. + ## Calibration baseline (regression anchors) The two worked evaluations in `references/examples/` are the validation baseline — re-running their pipelines must reproduce these verdicts: - **`ge_arrow`** ([#717](https://github.com/QuantEcon/lecture-python.myst/pull/717)): **2.85/5 — mixed/wash.** Tiny 2×2/3×3 economies, fresh static args per call → ~45× slower as-used despite warm wins. -- **`markov_asset`** ([#654](https://github.com/QuantEcon/lecture-python.myst/pull/654)): **2.25/5 — net regression.** Build-breaking `NameError` (stray `err.throw()`), float32 drift near a critical stability margin. +- **`markov_asset`** ([#654](https://github.com/QuantEcon/lecture-python.myst/pull/654)): **2.25/5 — net regression.** A stray `err.throw()` that crashes in any clean namespace and, in notebook order, silently disables the checkify stability validation (a masked failure — see the REPORT erratum); float32 drift near a critical stability margin. - **HIGH anchor:** the aiyagari Bellman pattern (`scripts/calibration/bellman_bench.py`) — large fixed-shape arrays, many re-solves; ~25× faster as-used → the "score 5" calibration. The rubric will also be distilled into the QuantEcon manual as the companion to the JAX style page ([QuantEcon.manual#104](https://github.com/QuantEcon/QuantEcon.manual/issues/104)). diff --git a/docs/developing-skills.md b/docs/developing-skills.md new file mode 100644 index 0000000..4b1fdd8 --- /dev/null +++ b/docs/developing-skills.md @@ -0,0 +1,53 @@ +# Developing skills + +For contributors adding or modifying plugins in this repo. (Using them: [using-skills.md](using-skills.md).) + +## Repo layout + +``` +.claude-plugin/marketplace.json # the catalogue — every plugin registers here +scripts/validate.py # manifest + frontmatter validation (CI runs this) +docs/ # these guides +/ # one directory per plugin + .claude-plugin/plugin.json # name, description, version + README.md # the plugin's user guide + skills//SKILL.md # one directory per skill + scripts/ # deterministic scripts the skills drive + references/ # rule/rubric content the skills read +``` + +The two live plugins show the two shapes: `qe` (umbrella skill + thin per-category sub-skills sharing plugin-level rules and scripts) and `benchmark` (one skill driving a deterministic engine with worked examples as its regression baseline). + +## Conventions + +- **Verb-first skill names** (`check-style`, `review-acceleration`) — they read as commands. +- **Description quality matters**: the SKILL.md frontmatter `description` is what natural-language invocation matches against. State what the skill does, what it measures, and when to use it. `validate.py` rejects descriptions too short to trigger reliably. +- **Report first, fix on request** — skills never silently edit; anything `build_risk` or output-changing (RNG streams) is presented, never auto-applied. +- **Deterministic before LLM**: put everything mechanical in `scripts/` (checkable, testable, zero-false-positive bar); reserve the skill's judgement for what genuinely needs it. The discipline scales with what the skill outputs: + 1. *Every skill*: claims carry citations — rule ID + `file:line`, or a number + its source. A findings list needs nothing more; don't add ceremony to simple skills. + 2. *Skills that judge*: record judgement as discrete answers (true/false per criterion, each cited), not free prose — so it's checkable. + 3. *Skills that score*: when multiple judgements aggregate into a verdict with stakes, use the benchmark plugin's evidence-file pattern — judgement lives only in an evidence file, a deterministic engine computes every score, and no score is ever typed by hand. Aggregation is where hand-waving hides; the engine eliminates it. +- **Don't duplicate content across docs** — one canonical location, pointers elsewhere. Rule text, weights, and thresholds especially: restated copies drift. +- **Self-contained plugins**: an installed plugin ships only its own directory. No relative links or paths that escape the plugin root; use absolute GitHub URLs for repo-level files, and anchor runtime paths for the installed context (issue #4 tracks the `${CLAUDE_PLUGIN_ROOT}` pattern). + +## Development loop + +```bash +# try a plugin against a real project without installing it +claude --plugin-dir ./benchmark + +# validate everything the marketplace serves +python scripts/validate.py +``` + +`validate.py` checks: every catalogue entry resolves to a real directory; `plugin.json` agrees with `marketplace.json` on name/version/description; every SKILL.md has frontmatter whose `name` matches its directory. Negative-test your changes (break something on purpose; the validator must fail loudly) — a malformed manifest breaks installation silently in every consuming repo. + +## Versioning and releases + +Bump the version in **both** `plugin.json` and the plugin's `marketplace.json` entry — the validator enforces they match. Scaffolding → first usable content is a minor bump (the benchmark plugin's evaluation-system landing was 0.1.0 → 0.2.0). + +## PR flow + +- Branch, PR, CI must be green. This repo **squash-merges** — stacked branches need `git rebase --onto origin/main ` after the base PR merges (already-upstream commits drop automatically). +- External contributions land with the contributor as git author (`--author`, GitHub noreply address unless they prefer otherwise) and integration fixes as separate commits — see PR #5 for the pattern. +- Plans live in [CATALOG.md](../CATALOG.md) (active) and [FUTURE-IDEAS.md](../FUTURE-IDEAS.md) (parked); per-plugin work items live in issues ([#3](https://github.com/QuantEcon/skills/issues/3) style, [#4](https://github.com/QuantEcon/skills/issues/4) benchmark). The style-guide rule content is authored in `QuantEcon/style-guide`, never here — this repo's `qe` plugin consumes a rendered snapshot ([project-style-guide#6](https://github.com/QuantEcon/project-style-guide/issues/6)). diff --git a/docs/using-skills.md b/docs/using-skills.md new file mode 100644 index 0000000..253b9bb --- /dev/null +++ b/docs/using-skills.md @@ -0,0 +1,45 @@ +# Using QuantEcon skills + +For lecture authors, reviewers, and maintainers who want to *use* the skills. (Building new ones: [developing-skills.md](developing-skills.md).) + +## Setup + +**In a lecture repo that has opted in** — nothing to do. Repos that check the marketplace into `.claude/settings.json` (see the [repo README](../README.md)) install everything automatically when you open the repo in Claude Code and trust it. + +**Anywhere else** — three commands in a Claude Code session (the marketplace first, then the plugins you want): + +``` +/plugin marketplace add QuantEcon/skills +/plugin install qe@quantecon # author-facing base skills +/plugin install benchmark@quantecon # lecture-evaluation tooling +``` + +**In CI** — `anthropics/claude-code-action@v1` accepts `plugin_marketplaces` and `plugins` inputs directly; see the repo README for the workflow snippet. + +## Invoking a skill + +Three ways, all equivalent: + +1. **Slash command** — type `/` and pick from the menu, e.g. `/benchmark:review-acceleration 717`. Trailing words are passed to the skill as arguments. +2. **Natural language** — describe what you want ("check this lecture's figures against the style guide"; "is this JAX conversion actually an improvement?") and the matching skill triggers on its description. +3. **Category entry points** — some plugins expose thin sub-skills (`/qe:check-figures`, `/qe:check-math`, …) so a narrow check is one keystroke and shows up in autocomplete. + +## What to expect + +- **Report first, fix on request.** Skills produce a structured report and *offer* changes; they never silently edit your files. Risky fixes (anything that breaks builds or changes published figures, e.g. RNG-stream changes) are presented but never auto-applied. +- **Evidence, not vibes.** Reports cite rule IDs, `file:line` locations, and measured numbers. The benchmark plugin goes further: its scores are computed by a deterministic engine from recorded evidence — the session shows the full derivation. +- **The same skill works pre-PR and in review.** Run it on your working copy before opening a PR (catch issues early), or point it at an open PR (consistent review). + +## The plugins + +| Plugin | Skills | What they do | Status | +|---|---|---|---| +| `qe` | `/qe:check-style` + `check-{writing,math,code,figures,jax,refs}` | Style-guide compliance for lecture source, by rule ID | scaffolding — [skills#3](https://github.com/QuantEcon/skills/issues/3) | +| `benchmark` | `/benchmark:review-acceleration` | Score a NumPy→JAX/Numba conversion (review mode) or assess whether a lecture is worth converting (triage mode) | system landed — [guide](../benchmark/README.md), [skills#4](https://github.com/QuantEcon/skills/issues/4) | + +## Updating and troubleshooting + +- **Update**: `/plugin` → marketplace → update, or reinstall; repos with the settings.json opt-in track the marketplace automatically. +- **Skill not in the menu?** Check the plugin is installed and enabled (`/plugin`), and that you trusted the repo. In settings-managed repos, `enabledPlugins` must list it. +- **A skill reports "not yet operational"** — it's scaffolding; its issue link says what's pending. +- **Version pinning**: plugin versions live in the marketplace catalogue; CI validates that every manifest is consistent, so a broken install is a bug — please open an issue. diff --git a/reviews/benchmark-design-2026-07-21-independent.md b/reviews/benchmark-design-2026-07-21-independent.md new file mode 100644 index 0000000..954df3f --- /dev/null +++ b/reviews/benchmark-design-2026-07-21-independent.md @@ -0,0 +1,159 @@ +# First-principles design review — `benchmark/` lecture-evaluation system + +**Date:** 2026-07-21 +**Branch reviewed:** `docs-skills-usage` +**Scope:** the full evaluation system in `benchmark/` — rubric dimensions and weights, metrics, aggregation and verdict method, thresholds and calibration, measurement architecture. Everything treated as open to revision; the goal is the best possible evaluation system for QuantEcon, where lectures are teaching materials first. +**Method:** read every design document ([README.md](benchmark/README.md), [EVALUATION_FRAMEWORK.md](benchmark/references/EVALUATION_FRAMEWORK.md), [scripts/README.md](benchmark/scripts/README.md), [SKILL.md](benchmark/skills/review-acceleration/SKILL.md), [examples README](benchmark/references/examples/README.md)), the scoring engine ([rubric.py](benchmark/scripts/scoring/rubric.py), [score.py](benchmark/scripts/scoring/score.py)), both worked evaluations (evidence, results, reports), and the measurement scripts; then ran the rubric against synthetic edge cases to test the aggregation empirically. The numbered edge-case results below (A–D) were produced by executing `rubric.score_all` directly on constructed evidence. + +## Bottom line + +The system's core measurement doctrine — **as-used, fresh-process, compile-time-counted** — is genuinely right and well-executed, but the scoring layer on top of it has structural flaws that can produce wrong verdicts. Three were confirmed empirically: + +- A lecture that **does not build** can score 3.9–4.2 → "merge" territory (compensatory aggregation, no gates). +- A **no-op conversion** (candidate byte-identical to baseline) scores 3.40 → "net positive" (the scale's zero is displaced). +- Moving a **single hand-counted concept** flips markov_asset's verdict band (2.25 "net regression" → 2.50 "wash"). + +The critique is ranked by decision impact; each item carries evidence and a recommended replacement design. + +--- + +## What the system gets right (keep these) + +- **The as-used doctrine.** Fresh process, real call sequence, real sizes, compile time counted, warm numbers never the headline. The ge_arrow case (1.4–4.8× faster warm, 45× slower as-used) proves this rule earns its keep. This is the system's central insight and should survive any redesign. +- **Evidence → score determinism.** Scores computed, never typed; every score carries its derivation; scorecards reproduce byte-identically; the 5⁷ brute-force band-edge audit found and fixed real FP bugs. This auditability discipline is rare and valuable. +- **Verbatim extraction with disclosed deviations**, including keeping markov_asset's build-breaking bug in `model_new.py`. +- **The caveat register** (M1/m3/n6 in the examples README) is unusually honest — M1 already anticipates part of finding 4. +- **Triage's bounding logic** — "the baseline as-used total bounds the entire possible win" — is elegant, cheap, and correct. + +--- + +## 1. The verdict is compensatory with no gates — a lecture that doesn't build can be told "merge" + +The weighted mean lets any dimension buy off any other. Running the rubric on a candidate with `builds: false` and strong scores elsewhere: + +| Synthetic case | Total | Verdict (`rubric.py:75-82`) | +|---|--:|---| +| Does not build, all else max, bug-override set | **3.90** | "net positive with fixable regressions — merge after addressing them" | +| Same, evaluator forgets the override flag | **4.20** | "clear improvement — merge" | +| Every dimension at its "wash" anchor (all 3s) | **3.00** | "net positive with fixable regressions" | + +markov_asset landed at 2.25 only because its *other* dimensions were also weak — the rubric has no mechanism guaranteeing that outcome. Note also that the build-breaking bug must be recorded twice (`builds: false` in correctness *and* `introduces_correctness_bug: true` in logic_design) with nothing enforcing consistency; forgetting the duplicate is worth +0.30 and a band. + +The all-3s case exposes a band mislabeling: every dimension's 3 is defined as "wash" (efficiency 0.8–1.3× is literally labeled "wash" in the anchors), yet a total of 3.0 maps to "net positive — merge after addressing." The wash band (2.5–2.9) sits *below* the scale's center. The ge_arrow report feels this: it calls 2.85 "net mixed, *slightly negative*" while the band says "mixed/wash." + +**Recommended design.** Non-compensatory gates checked *before* any weighted total: + +- **G1 — executes end-to-end** on the shipped configuration. Fail → "do not merge," full stop, whatever the total. +- **G2 — published numbers preserved** (within tolerance under the shipped dtype config) *or* the change is explicitly flagged and justified in the PR. Silent changes to printed numbers fail the gate — this is the framework's own stated principle ("silently changes the numbers"), which the current scoring only *discounts* rather than blocks. + +Then re-center the bands so 3.0 is a wash, and make band labels *descriptive* ("net improvement" / "wash" / "net regression") rather than imperative ("merge after addressing them" asserts fixability the score cannot know — the must-fix list is where fixability belongs). + +## 2. The scale's zero is displaced — a no-op conversion scores 3.40 "net positive" + +The verdict claims to answer "did this change improve the lecture?" but roughly 40% of the weight scores the *candidate in absolute terms*: the four structural checklists (0.45 combined weight) ask whether the new code is pure, global-free, idiomatic, testable; the docstring column of readability is `docstring_cov_new`, an absolute; ergonomics counts the candidate's statements. + +The identity test — candidate byte-identical to ge_arrow's baseline, evidence filled per the committed conventions — yields **3.40, "net positive with fixable regressions — merge after addressing them"** for a PR that changes nothing. + +The converse also holds: converting an already-pristine baseline (lake_model-style) inherits structural 4s–5s from virtues the baseline already had, and `fixes_prior_bugs` means a conversion of a *clean* lecture caps logic at 4 — the candidate is punished for the baseline's quality. The checklists even mix framings internally: `fixes_prior_bugs` is a delta, `pure_no_order_dependence` is an absolute. + +**Recommended design.** Score **both implementations on the same absolute anchors** and derive the verdict from the delta profile. The machinery barely changes: `static_metrics.py` already measures both sides; the checklists just get answered twice. Output becomes two absolute scorecards plus a per-dimension delta table; the verdict comes from the weighted delta (identity = 0.0 by construction) with the gates from finding 1. The dual scorecards are independently useful for QuantEcon: "the baseline itself scores 2.4 — the fix is a rewrite, not necessarily a JAX rewrite" is exactly the recommendation the ge_arrow report reached by hand. + +## 3. Measurement architecture: the system measures a hand-built reconstruction, not the lecture + +Each evaluation hand-adapts `as_used_total.py` to re-enact the lecture's call sequence. This puts the headline metric at the mercy of adaptation choices: the ge_arrow script implements the λ-sweep as a 100-iteration Python loop on the NumPy side and one jitted `fori_loop` on the JAX side (`as_used_total.py:50-54` vs `:89-99`) — defensible (caveat n6 documents it), but it's an evaluator's judgement sitting inside the decisive number, and nothing verifies the reconstruction against the lecture. markov_asset needed a hand-patched copy just to produce a timing. And the whole thing is timed **once** — a single pass per side feeding threshold cliffs at 0.8×/1.3×/3×. + +Beyond that: the deliverable is the *lecture* (prose + code + outputs), but only extracted code is evaluated — markov_asset's prose edits went unassessed, and prose is where pedagogy actually lives. + +**Recommended design.** Measure the lecture itself. The lectures are executable MyST documents: for each branch, convert with jupytext and execute with nbclient in a fresh kernel, recording **per-cell wall time** (repeat K≥5, take medians). As-used time is then the real reader/CI wait, by definition — no reconstruction, no asymmetry class, no per-lecture timing scripts. Equivalence becomes an **executed-output diff**: compare the printed numbers between branches numerically — which is *directly* "were the published numbers preserved?" (gate G2), and it catches build breaks natively (markov_asset's `NameError` fails execution — no `smoke_test.py` needed). The current micro-benchmarks (crossover-n, recompile audit, cold-start) remain valuable as *diagnosis* feeding the must-fix list, not as verdict inputs. This is also the only architecture that scales across ~200 QuantEcon lectures, and it makes triage nearly free (baseline-side timing = execute the current lecture). + +## 4. Readability — the heaviest weight rides on the weakest instrument + +`n_prerequisite_concepts` drives the 0.25-weight dimension, and it is a hand-curated list embedded in each lecture's script (`static_metrics.py:41-52`) — caveat M1 admits this. The counting granularity is arbitrary ("jax.jit & tracing" and "static_argnames & recompilation" are two concepts; "NumPy arrays & slicing" is one), and the verdict is exquisitely sensitive to it: **recounting markov_asset's +5 as +4 — one concept — moves the total from 2.25 ("net regression — do not merge") to 2.50 ("mixed/wash")**, verified against the committed evidence. + +Further problems: the delta treats concepts as fungible (7 OOP concepts out, 7 JAX concepts in = +0 = band 5, though the audience already knows the former from the series and none of the latter); `docstring_cov_new` is absolute, not a delta, and is a weak proxy for lecture code whose real documentation is the surrounding prose; and the "math-to-code distance" tie-breaker promised in EVALUATION_FRAMEWORK.md §2 never appears in `score_readability` (`rubric.py:104-111`). + +**Recommended design.** Curate the concept inventory **once at series level, not per lecture**: a versioned `concepts.yml` mapping detection patterns → canonical concepts → the lecture where the series first teaches each. The existing `CONCEPTS` regex dict shows most JAX concepts have syntactic signatures (`jax.jit`, `static_argnames`, `.at[].set`, `lax.fori_loop`) — the fragile step was per-lecture deduplication into "ideas," which a shared map eliminates. The metric becomes mechanical: concepts used by the candidate that the series has *not taught at or before this lecture*, minus the same for baseline. Pair it with a prose check — for each new concept, does the PR's added prose explain it near first use? (A concept explained in prose is pedagogy; one that appears bare is burden.) Judgement that remains moves into `evidence.json` as cited slots — the M1 fix already proposed on skills PR #5, endorsed and extended here. + +## 5. Efficiency: ratio-only scoring with a saturating floor and no materiality test + +Three problems in `score_efficiency` (`rubric.py:114-124`): + +1. Everything below 0.8× collapses to one band: 0.75× slower and 45× slower both score 2. +2. `correct_or_fixable` is near-vacuous (all code is "fixable"; both committed cases say so), so score 1 is practically unreachable — it's a 4-point scale in disguise. +3. Most important, **the ratio ignores absolute materiality**: ge_arrow's "45× slower" is 0.035 s → 1.56 s — 1.5 wall-clock seconds a reader would never notice. Triage mode already knows this ("a lecture whose compute totals 30 ms has nothing to give") but review mode doesn't: it hands ge_arrow a −2-band penalty for an imperceptible cost, while the actual crimes (readability, precision) are elsewhere. Ratios of tiny denominators are also noise-dominated — a single unrepeated pass deciding a banded score. + +**Recommended design.** Two-dimensional scoring: a **materiality zone** first (|Δtotal| below a threshold — say 2 s of reader/CI wait — is automatically a 3/wash, whatever the ratio), then **log-symmetric ratio bands** outside it, so 3× faster and 3× slower are equidistant from wash. Under this, ge_arrow's efficiency is a 3 (correct: the case against it is pedagogy, not seconds), aiyagari's −52 s is a decisive 5, and the headline stops sounding catastrophic ("45×!") for immaterial stakes. Require K fresh-process repeats with the band assignment stable across the spread before it's recorded. + +## 6. Calibration is two in-sample points, and triage's validation is circular + +The thresholds claim calibration "against two measured end points" — 25× and 0.022×. Two points pin the two extreme bands; every interior boundary (0.8, 1.3, 3× for efficiency; +1–2/+3–4 for concepts; the docstring cuts) has **zero observed cases** — both real evaluations landed in band 2 of readability and band 2 of efficiency. + +The triage validation table in the plugin README "reproduces every known verdict" on exactly the three cases the thresholds were built from — in-sample prediction presented as validation. + +The HIGH anchor's provenance is inconsistent: EVALUATION_FRAMEWORK.md (§2, dimension 3) cites 1664 ms / 29.3 s while the committed `bellman_bench.json` says 2955 ms / 54.3 s — same ~24× ratio, visibly different run, in a system whose brand is citation fidelity. + +**Recommended design.** Label all interior thresholds *provisional* in the framework; pre-register a recalibration protocol (after every N evaluations, re-fit band edges against the accumulated evidence files, version the rubric, re-run the regression anchors); treat the next several real evaluations as out-of-sample tests of triage and report hits/misses. Reconcile the anchor numbers now — either re-run and update the doc, or cite the JSON. + +## 7. Cross-dimension double counting quietly rewrites the weights + +Single root causes score in multiple dimensions: + +- **float32** hits correctness (the entire band-3 rung), maintainability (`dtype_precision_safe`), and readability (the "float32/x64 flag" concept). +- **Purity** appears in logic (`pure_no_order_dependence`), maintainability (`pure_unit_testable`), and ergonomics (`fragile_protocol`). +- **Vectorisation** appears in logic (`good_algorithmic_choices`), style (`vectorised_where_natural` *and* `correct_control_flow_primitive`), efficiency, and readability (the `fori_loop`/carry concepts). + +So the effective weight of these facts exceeds any nominal number, and the headline claim "readability (0.25) outranks efficiency (0.15)" isn't reliably true of the system's actual behavior. Seven dimensions also carry overlap costs: maintainability at 0.05 can swing the total by at most 0.20 — ceremony without leverage. + +**Recommended design.** Either assign each observable to exactly one home dimension and publish the assignment, or — better — consolidate to four orthogonal dimensions re-derived from the teaching-first principle: + +| Dimension | Weight (proposed) | Contents | +|---|:--:|---| +| Fidelity | ~0.25 | gates + precision policy | +| Pedagogy | ~0.40 | concept burden, math-to-code, prose explanation, exercise integrity | +| Cost | ~0.15 | as-used time with materiality, dependency/install burden | +| Code quality | ~0.20 | purity / idiom / API / maintainability merged | + +Fewer, cleaner dimensions make the weights mean what they say. + +## 8. No reliability engineering around the human/AI judgement slots + +The system's motto — "no score is ever typed by hand" — is true but subtly overstated: the *scores* are computed, but the checklist booleans, `fragile_protocol`, `correct_or_fixable`, concept lists, and `statements_for_one_result` are hand-typed judgements that map deterministically to scores. Determinism relocated the subjectivity; it didn't remove it. There is no inter-rater data — no evidence that two independent evaluators (or two AI-skill runs) fill `evidence.json` the same way, on an instrument where finding 4 shows one boolean or one concept can flip a verdict. + +**Recommended design.** Since the skill automates the fill, reliability testing is nearly free: run the evidence-fill twice in independent sessions, diff the judgement slots, and surface disagreements for human adjudication rather than silently keeping one. Track agreement rates across evaluations — that number, not the determinism claim, is what makes the structural dimensions trustworthy. + +## 9. Smaller defects (fix opportunistically) + +- `score_correctness` (`rubric.py:92`): the x64-divergence guard fires only when `d > 1e-8` — divergent logic with small shipped drift slips into bands 4–5. Also the band-3 reason string hardcodes "ships float32" as the explanation regardless of actual cause. +- Correctness thresholds are **absolute** `max|Δ|`; for lectures whose published objects are large-magnitude, 1e-3 absolute may be ~1e-6 relative and still scores 2. Use relative error (or per-object normalization). +- `score_ergonomics` (`rubric.py:129`) gives base 2 for any n≥4, but the prose anchor reserves 2 for "ordered, side-effecting" protocols — code and prose disagree. +- Dependency cost is unmeasured: adding `jax`/`jaxlib` to a lecture is a real installability burden for students (platform wheels, Windows) and belongs in the Cost dimension. +- Hardware policy should be pinned in the framework: "as-used" = the target series' actual build environment (CPU runners for lecture-python.myst, GPU for lecture-jax), recorded by `env_stamp.py`. A conversion verdict is environment-relative and should say so. +- Anchors were measured on jax 0.4.35 (old by now); compile costs and defaults drift across JAX releases. The recalibration trigger lives only in a script docstring — promote it to the framework. + +--- + +## Summary + +The measurement doctrine (as-used, fresh-process) and the auditability discipline are the right foundation — keep both. The redesign priorities: + +1. **Gates before any weighted total** (builds; published numbers preserved), with re-centered, descriptive bands. +2. **Symmetric dual scoring** so the verdict measures the *change*, with identity = 0. +3. **Execute the actual lecture on both branches** (per-cell timing + output diff) in place of hand-adapted replay scripts — simultaneously fixes gate G2, kills the reconstruction-fidelity risk, and scales to the whole series. +4. **Series-level concept inventory** with mechanical detection in place of per-lecture hand lists. +5. **Materiality zone plus log-symmetric bands** for efficiency. + +Items 1, 2, and 5 are pure `rubric.py` changes that could land quickly and be validated against the two committed evidence files; item 3 is the one real piece of new engineering; item 4 is a natural companion to the QuantEcon.manual style-page work already tracked in QuantEcon.manual#104. + +--- + +## Appendix — empirical verification runs + +All produced by calling `rubric.score_all` directly (engine at `benchmark/scripts/scoring/rubric.py`, evidence conventions as in the two committed `evidence.json` files): + +| Case | Construction | Total | Verdict | +|---|---|--:|---| +| A | No-op conversion: candidate ≡ ge_arrow baseline; quantitative slots take the baseline's own measured values (Δ=0, speedup 1.0×, docstrings 0.90, 4 ordered statements); checklists describe the baseline code (order-dependent methods, module globals, typo) | **3.40** | net positive with fixable regressions — merge after addressing them | +| B | `builds: false`, every other slot maximal, `introduces_correctness_bug: true` (logic capped at 3) | **3.90** | net positive with fixable regressions — merge after addressing them | +| B2 | Same as B but the duplicate override flag left false | **4.20** | clear improvement — merge | +| C | Every dimension at its "wash" anchor (score 3 on all seven) | **3.00** | net positive with fixable regressions — merge after addressing them | +| D | markov_asset committed evidence with `delta_prereq_concepts` 5 → 4 (one fewer hand-counted concept; docstrings 0.75 → band 4, so the concept column pins the readability score) | 2.25 → **2.50** | net regression → mixed / wash | diff --git a/reviews/benchmark-design-2026-07-21-merged.md b/reviews/benchmark-design-2026-07-21-merged.md new file mode 100644 index 0000000..b283cf1 --- /dev/null +++ b/reviews/benchmark-design-2026-07-21-merged.md @@ -0,0 +1,73 @@ +# Benchmark evaluation system — merged design review (2026-07-21) + +Synthesis of two independent design critiques of the evaluation system, run deliberately in isolation from each other as a bias control: + +- **Review A** ([benchmark-design-2026-07-21-independent.md](benchmark-design-2026-07-21-independent.md)) — a fresh session with an unframed prompt; 9 findings, empirically verified against the engine. +- **Review B** — a 36-agent adversarial workflow: six critics attacking from independent angles, every non-minor critique then passed to a **steelman defender** instructed to save the original design; only critiques surviving the strongest defense are reported. Two critiques survived outright (CRITIQUE_STANDS); most were PARTIALLY_DEFENDED — the residuals below are what remains after the best defense. + +Findings present in **both** independent runs are the most robust. All demonstrations referenced here were produced by executing `rubric.py`/the notebooks, not by inspection alone. The scope was the *design*; the system's architecture (evidence → deterministic engine, verbatim extraction, the as-used doctrine) was validated previously and both reviews independently concluded it should survive any redesign. + +--- + +## 1. Corrections of record (already applied) + +The review process falsified three claims our own documents made — corrected via erratum and rewording on this branch. (Evaluation findings briefly posted to [lecture-python.myst#654](https://github.com/QuantEcon/lecture-python.myst/pull/654) were withdrawn; that PR will receive one authoritative evaluation after the v2 revision, rather than a comment-and-correction trail.) + +1. **"markov_asset's lecture does not build as shipped" — false as worded.** Executing all cells of the PR branch's notebook in order completes cleanly (preview CI passes): earlier cells bind a global `err` that the stray `err.throw()` inside `call_option` silently resolves to. The true finding is *subtler and worse*: the stale-global masking means **the checkify stability validation never actually runs in the shipped lecture**, on the model whose spectral radius sits 0.002 below the stability bound; a reader copying the function into a clean namespace hits the `NameError`. `builds: false` remains a true measurement under the system's declared fresh-process regime; the sentence about the lecture was wrong. +2. **"The replayed sequence mirrors the lecture exactly" — false for both reference cases.** ge_arrow's replay constructs 12 model objects where the lecture constructs 6 and reuses them across initial states; markov_asset's replay builds a fresh model per γ where the lecture mutates one object, and omits two calls. The reconstruction-fidelity risk the review process flagged abstractly had already occurred, undetected. +3. **"Medians over repeats" — false for the headline metric.** The as-used totals are single passes per side; only the warm/scaling benchmarks use medians. The one metric that solely decides a 0.15-weight dimension is the least-replicated measurement in the system. + +## 2. Findings that survive the steelman defense + +### 2.1 The verdict's safety couplings exist only by convention (both reviews; residual after defense) + +The defense established something the critiques missed: **with the documented evidence convention followed** (build failure recorded in both `builds` and the logic-design bug flag), a non-building lecture's ceiling is exactly **3.90 < 4.0** — the two overrides jointly form a designed soft gate on the unconditional-merge band. But that coupling is enforced by reviewer discipline, not code: one forgotten boolean yields **4.20 "clear improvement — merge" for a lecture that crashes** — in a system whose stated contract is that scores are deterministic functions of evidence. Worse, Review B closed the "requires dishonest evidence" escape: `builds: true`, `matches_under_x64: true`, `max|Δ| > 1e-1` (a float32 catastrophe with no logic bug) reaches **4.2 with fully honest evidence**. + +### 2.2 The readability instrument inverts its own ground truth (CRITIQUE_STANDS) + +`score_readability` = worse-of(Δprereq-concepts, docstring coverage). Executed against the framework's own labeled exemplars: **odu.py — the framework's LOW-readability example — measures 0.86 docstring coverage**, and code written in the flagship aiyagari style (inline shape comments, undocumented closures/NamedTuples) measures ~0.41–0.55 and is **mechanically capped at readability 2** regardless of concept count. The metric anti-correlates with the framework's own judgements at both ends, and the construct the prose says defines the dimension ("math-to-code distance") has no encoding in the scorer at all. + +### 2.3 Review mode and triage mode contradict each other (residual after defense) + +A fully-polished ge_arrow — every fix from its own report applied, the intrinsic 0.022× as-used unchanged — scores **4.0 "clear improvement — merge"** while the (blind-validated) triage rule says *don't convert* at a 0.028 s baseline. The band vocabulary cannot express "no-conversion" — a verdict the system's own authors needed twice and both times delivered in prose outside the score. + +### 2.4 Band labels use delta language an absolute-hybrid total cannot license (both reviews; narrowed by defense) + +The defense partly refuted Review A's "3 is neutral" (only efficiency anchors 3 as wash; correctness/readability anchor *no-change at 5*). What survives: ~40% of the weight scores the candidate absolutely, so a no-op rewrite of a ge_arrow-quality baseline scores ~3.35–3.55 — rewrites landing in [3.0, 3.55) are *worse than doing nothing* yet labeled "net positive." The homogeneous-population defense (baselines share a known-bad house style, so absolute criteria are deltas in disguise) is honest but population-bound — a clean-baseline conversion scores 4.30 "clear improvement" for virtues the baseline already had. + +### 2.5 Judgement noise exceeds band resolution (both reviews; narrowed by defense) + +One hand-counted concept flips markov_asset 2.25→2.50 across a band; one contestable checklist boolean flips ge_arrow 2.85→3.00; ~27% of ordering-preserving weight vectors flip ge_arrow's band. The defense's strongest point: every demonstrated flip crosses a *deliberation* boundary (the operational next step is identical), never the merge/reject gates, which sit ≥0.35 away. What survives: the 2-decimal total communicates precision the instrument lacks; the concept-count grain rule exists only by example; a one-flip sensitivity stamp (~20 lines in score.py) would make fragility visible. + +### 2.6 Fact-level fan-out is undocumented (both reviews; substantially defended) + +The per-consequence billing defense is strong — a decision harming readers *and* callers *and* editors *should* cost multiply, and most of the demonstrated 1.35-point checkify swing decomposes into legitimately distinct harms. What survives: two near-verbatim duplicate criteria across logic/style (`good_algorithmic_choices` glossed as "vectorised where natural" vs style's `vectorised_where_natural`); and no document states that a root cause's total influence is the sum over its manifestations — the weight table invites misreading. + +### 2.7 "Calibrated" overclaims (both reviews) + +The two measured anchors pin the efficiency scale's *extremes*; every interior edge (0.8/1.3/3×; the Δ bands; the concept and coverage cuts; the 4.0/3.0/2.5 verdict cutoffs) has zero observed cases and no recorded derivation. The defense showed the wash band is deliberately wider than the noise floor (a real design rationale) — but nothing in the repo records it. Same class: the triage "3/3 validation" is in-sample (now noted in the docs). + +## 3. Defended — no change recommended + +- **The efficiency ratio-only form** (Review A's materiality critique): the ratio is the right construct for "did the conversion meet its stated goal"; log-rescoring changes no committed verdict; absolute reader-seconds belong in *triage* (where they already are) and in the report prose (where they already are). +- **The weighted total per se**: it provides a real total order for programme-level triage and a distance-to-merge trajectory; the four-gate alternative was shown to be a lossy projection of the rubric fitted on its own calibration set — its "3/3 agreement" validates the rubric, not the gates. +- **min() aggregation in readability** (direction): the gameable input (docstrings) has no upward power under min() — Goodhart-resistant by shape. The problem is the input (2.2), not the aggregation. +- **Per-consequence multi-counting as a principle** (2.6's core). + +## 4. Recommended v2 changes + +Ordered by (impact ÷ effort); items 1–5 are engine/doc changes validatable against the committed evidence files; item 6 needs @xuanguang-li's design input. + +| # | Change | Effort | +|---|---|---| +| 1 | **Enforce the couplings in code**: derive the logic-design cap from `builds`/x64-divergence in `score_all`; gate the verdict — correctness 1 (any cause) caps the verdict at "net regression," correctness 2 caps at "mixed/wash" | ~5 lines | +| 2 | **Add a "no-conversion" verdict**: when the efficiency evidence shows the triage don't-convert profile (immaterial baseline total + slower as-used), the verdict says so instead of scoring the polish | ~10 lines | +| 3 | **Sensitivity stamp**: score.py perturbs each boolean and band-adjacent value, marks the scorecard `robust` or `fragile (deciding flips listed)`; report totals at the precision the instrument supports | ~20 lines | +| 4 | **K-repeat as-used** (median of ≥3 fresh-process runs; contested-band annotation when the spread crosses an edge) | script change | +| 5 | **Documentation honesty pass**: thresholds labeled policy choices with derivation notes; fan-out paragraph; concept-grain rule stated (one item per reader-facing API surface, symmetric old/new); "calibrated" → "anchored" | prose | +| 6 | **Readability instrument v2**: replace docstring coverage with equation-traceability (per numbered equation: can a reviewer cite the single implementing expression? fraction traceable, old vs new — same citation discipline as the checklists); move concept lists into evidence.json as cited slots (extends the M1 proposal) | design + rubric | +| 7 | **Extraction/replay verification**: a mechanical step diffing extracted code against the lecture's cells, and the replay's call sequence against the lecture's — closing the fidelity gap that produced §1.2. Longer-term: evaluate executing the lecture itself at both refs (nbclient per-cell timings; meta#335 telemetry) as the as-used source, with the current scripts as the diagnostic layer | design | + +## 5. What both reviews agree must survive + +The as-used, fresh-process, compile-counted doctrine; evidence → deterministic scoring with printed derivations; verbatim extraction with disclosed deviations; the calibration-anchor discipline; the caveat register; triage's bounding logic. The redesign is of the scoring superstructure, not the measurement foundation.