Skip to content

#187: Faster grade defaults — Haiku + per-provider fast models (plan) - #193

Merged
wjduenow merged 23 commits into
devfrom
feature/187-fast-grade-defaults
Jun 3, 2026
Merged

#187: Faster grade defaults — Haiku + per-provider fast models (plan)#193
wjduenow merged 23 commits into
devfrom
feature/187-fast-grade-defaults

Conversation

@wjduenow

@wjduenow wjduenow commented Jun 2, 2026

Copy link
Copy Markdown
Owner

Summary

Super plan for #187 — switch the default grader to a fast/cheap model, and (per the requested addendum) make the OpenAI and Gemini grade providers resolve to a fast default too.

Phase: detailing (awaiting approval)
Stories: 6 implementation + Quality Gate + Patterns & Memory
Decisions: DEC-001 … DEC-009

Headline decisions

  • Per-provider fast default via sentinel. GradeConfig.modelstr | None = None, resolved at config-load from PROVIDER_FAST_MODELS = {anthropic: claude-haiku-4-5, openai: gpt-4o-mini, gemini: gemini-2.5-flash} (all exact pricing keys). Explicit model: still honoured.
  • Frozen-safe resolution via @model_validator(mode="before") (not mode="after"GradeConfig is frozen).
  • max_output_tokens default 256 → 1024 so the new Gemini default doesn't truncate out of the box (supersedes Gemini MAX_TOKENS swallowed; live smoke flakes; per-provider full-pipeline e2e gap #155's 256 for the production default only).
  • Model↔provider compat validator — reject provider: openai, model: claude-… at config-load instead of at runtime; single prefix-map source shared with cost/_rollup.py.
  • Calibration is a gated, maintainer-run story — re-grade the Epic: Test Generation Expansion (#169 + #170 + #171) #179 Sonnet baseline with Haiku, ≥85% pass/fail concordance ships it as default.
  • DraftConfig.cheap_model dated id → bare SKU claude-haiku-4-5 (lockstep). Drafter stays on Sonnet.

Plan document

See plans/super/187-fast-grade-defaults.md.

Next steps

  • Review the plan in this PR.
  • Approve in Claude Code to proceed to devolve (beads creation).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Per-provider fast-grade defaults: omitting grade.model auto-resolves to a provider-specific default (Anthropic Sonnet, OpenAI gpt-4o-mini, Gemini gemini-2.5-flash).
  • Configuration Changes

    • Default max_output_tokens raised from 256 to 1024.
  • Validation Improvements

    • Fail-fast provider↔model compatibility checks and clearer errors when mismatched or missing defaults.
  • Documentation

    • Updated ops, provider, draft, and changelog guidance to reflect new defaults and behaviors.
  • Tests

    • Added calibration and coverage tests plus unit tests exercising default resolution and rollup logic.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR implements issue #187: switch grader default model resolution to per-provider fast defaults via provider tables, make GradeConfig model a sentinel-resolved field with SKU-prefix compatibility validation, raise grade max_output_tokens to 1024, add runtime assertions and estimate checks, update docs/rules/CHANGELOG, and add gated calibration and Gemini truncation tests and fixtures.

Changes

Fast-grade defaults rollout

Layer / File(s) Summary
Plan and roadmap
plans/super/187-fast-grade-defaults.md
Super Plan document defines the behavioral change goals, discovery findings about pricing and risk, architectural decisions (sentinel-based resolution, provider tables, compatibility validation, 1024-token default, gated calibration), and eight implementation stories with dependency graph.
Architecture rules and conventions
.claude/rules/grade-layer.md, .claude/rules/llm-drafter.md, .claude/rules/testing-signal.md
Updates rubric parsing guidance to reflect Sonnet default with Haiku opt-in; documents fast-model sentinel resolution and provider mapping tables; and adds gated calibration/concordance harness and denominator-hygiene patterns.
Provider registry and shared defaults
src/signalforge/llm/providers.py, src/signalforge/draft/config.py, src/signalforge/llm/cost/_rollup.py, tests/llm/test_providers.py
New PROVIDER_DEFAULT_MODELS and PROVIDER_SKU_PREFIXES exported from providers module. DraftConfig.cheap_model updated to bare claude-haiku-4-5 SKU. Cost rollup derives provider-prefix dispatch from the shared table. Tests verify constant structure, pricing key existence, and SKU prefix alignment.
GradeConfig model resolution and compatibility validation
src/signalforge/grade/config.py
GradeConfig.model changed to `str
GradeConfig tests and fixtures
tests/grade/test_config.py, tests/grade/test_provider_neutrality.py, tests/fixtures/grade/example_config.yml, tests/cli/test_generate_estimate.py
Updated defaults assertions and added #187 test suite covering per-provider fast-model resolution when model omitted, explicit model overrides, non-None guarantee, unknown-provider error behavior, prefix-mismatch rejection, loader-side error wrapping, and updated fixture comments.
Cost rollup SKU-to-provider dispatch tests
tests/llm/cost/test_rollup.py
Parametrized tests verify _provider_for_model returns expected provider for known SKU prefixes and assert _PROVIDER_PREFIXES equals inversion of PROVIDER_SKU_PREFIXES.
Runtime assertions and estimate output updates
src/signalforge/cli/_estimate.py, src/signalforge/grade/engine.py
Added fail-fast assert config.model is not None in grading engine and assert checks in estimate token/count helpers before pricing to ensure resolved config always has concrete model.
Operational documentation and changelog
docs/draft-ops.md, docs/grade-ops.md, docs/llm-providers-ops.md, CHANGELOG.md
Updated examples to use bare claude-haiku-4-5 and 1024-token defaults, documented config-load auto-resolution per provider, added cost guidance table and capability matrix updates, and added CHANGELOG entry for #187.
Calibration harness substrate and fixtures
tests/research/187-haiku-calibration/_substrate.py, tests/research/187-haiku-calibration/real_candidate.json, tests/research/187-haiku-calibration/sonnet_baseline_sample.json
Substrate provides pinned deterministic model, lazy-loaded frozen candidate JSON, empty prune result helper, expected-artifact-ids helper, and Sonnet baseline fixture recording provenance and per-(artifact_id,criterion_id) verdicts.
Sonnet baseline capture script
tests/research/187-haiku-calibration/capture_sonnet_baseline.py
CLI script drafts schema-only candidates, freezes artifacts to real_candidate.json, grades with claude-sonnet-4-6, excludes degraded pairs, writes provenance-rich baseline JSON, and prints next-step guidance.
Haiku concordance calibration gate
tests/research/187-haiku-calibration/test_haiku_calibration.py
Maintainer-run Anthropic-gated concordance test comparing Haiku vs Sonnet baseline enforcing ≥85% per-criterion agreement after excluding degraded results; validates baseline coverage, prints diagnostics, and is gated by pytest marker and API key checks.
Gemini 1024-token no-truncation empirical check
tests/research/187-haiku-calibration/test_gemini_1024_no_truncation.py
Maintainer-run Gemini-gated test that verifies no truncation-degradation at 1024-token setting; constructs verbose input/rubric, asserts no degraded results and aggregate_complete True, and validates JSON sidecar roundtrip.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

  • wjduenow/SignalForge#187: This PR directly implements issue #187 (grader-defaults, provider tables, validators, and calibration harness).

Possibly related PRs

Poem

🐇 I hop through configs, tidy and spry,
Per-provider defaults now waving hi.
Sonnet stands firm, Haiku takes a peek,
Tokens expanded — no truncation leak.
Calibration printed — maintainers cheer, hip-hip, high-five!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: implementing per-provider fast grader defaults with Haiku as an explicit option, matching the plan documentation and code changes throughout the PR.
Docstring Coverage ✅ Passed Docstring coverage is 92.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

wjduenow and others added 2 commits June 2, 2026 11:53
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@wjduenow
wjduenow marked this pull request as ready for review June 2, 2026 16:04

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@plans/super/187-fast-grade-defaults.md`:
- Line 74: The markdown table header line "| Provider | Cheapest known SKU |
input $/MTok | output $/MTok |" is breaking lint/rendering and there is a stray
literal "259" and inconsistent code-fence around the ASCII diagram; remove the
stray "259", ensure the table header is either a valid markdown table with a
separator row (e.g., add the --- row after the header) or convert the entire
section to a fenced code block, and make sure the ASCII diagram uses matching
```text opening and closing fences; apply the same cleanup to the related block
covering lines 251-259 so the table/code-block formatting is consistent and
lint-safe.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3fe8f193-7b9a-4292-9a9d-7da2bf82c4da

📥 Commits

Reviewing files that changed from the base of the PR and between b07f1b2 and 0c08043.

📒 Files selected for processing (1)
  • plans/super/187-fast-grade-defaults.md

Comment thread plans/super/187-fast-grade-defaults.md
wjduenow and others added 15 commits June 2, 2026 12:10
… claude-haiku-4-5

The dated id claude-haiku-4-5-20251001 is not a key in
signalforge.llm.pricing.PRICES (which uses bare SKUs). Change the
unused (declared-not-consumed) cheap_model default to the bare SKU
so it matches pricing and the new grade default; update the DEC-017
reference comment and field docstring in lockstep. Lockstep
consistency fix only — not wired into any code path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…XES constants

Add two provider->string mapping constants to signalforge.llm.providers,
keyed by the three registered provider names (anthropic/openai/gemini),
and export both in __all__:

- PROVIDER_FAST_MODELS — cheap/fast judge SKU per provider; every value is
  an exact key in signalforge.llm.pricing.PRICES (claude-haiku-4-5 /
  gpt-4o-mini / gemini-2.5-flash) so lookup()/--estimate never raise.
- PROVIDER_SKU_PREFIXES — SKU-string prefix per provider.

Refactor signalforge.llm.cost._rollup so its SKU-prefix dispatch derives
_PROVIDER_PREFIXES by inverting PROVIDER_SKU_PREFIXES instead of duplicating
the claude-/gpt-/gemini- literals. Classification behaviour is unchanged;
existing rollup tests stay green and a new parametrised test pins the
per-SKU classification plus the derived-from-source-of-truth invariant.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ver + compat validator + 1024 cap

Make the grade-judge model default a per-provider fast model resolved at
config-load, plus a model<->provider compat validator and a raised
max_output_tokens cap.

- model: str -> str | None = None sentinel. A @model_validator(mode="before")
  resolves the sentinel to PROVIDER_FAST_MODELS[provider] (frozen-safe; no
  mutation of the constructed instance). Unknown provider is NOT injected so
  the existing provider field-validator still raises UnknownProviderError
  rather than being masked. After construction model is always concrete.
- _model_non_empty field-validator now passes None through cleanly (the only
  None-survival path is the unknown-provider case that raises downstream).
- New @model_validator(mode="after") _validate_model_provider_compat rejects a
  SKU-prefix/provider mismatch (e.g. provider=openai + claude-* model), reusing
  PROVIDER_SKU_PREFIXES. Only the three known-prefix providers participate;
  custom/plugin providers (not in the table) may use any model name.
- max_output_tokens default 256 -> 1024 (DEC-004) to avoid one-line
  gemini-2.5-flash grade-JSON truncation. It is a cap, not a target.
- Module + class docstrings and the DEC-023..027 locked-defaults list updated.
- tests/grade/test_config.py: updated the defaults regression test to the new
  resolved defaults (haiku, 1024) and added coverage for per-provider
  resolution, explicit-honour, compat reject/accept, whitespace guard,
  unknown-provider-not-masked, and the loader round-trips.
- grade/engine.py + cli/_estimate.py: read-only assert narrows for the now
  str|None field (runtime-guaranteed concrete) to keep pyright green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ures for Haiku default

The #187 US-002 grade default flip (anthropic resolves to claude-haiku-4-5,
max_output_tokens 256 -> 1024) drifted two estimate fixtures pinned to the
old claude-sonnet-4-6 grade default. Re-baseline in lockstep:

- tests/fixtures/estimate/anthropic_byte_identity_golden.txt: regenerated.
  Only diff is the grade-model id (sonnet -> haiku) and the corresponding
  grade USD figures (per-criterion $0.0292 -> $0.0078, grade $0.1170 ->
  $0.0312, total $0.1814 -> $0.0956). Verified the draft section, token
  counts, call counts, artifact counts, and warehouse section are unchanged.

- tests/cli/test_estimate_engine.py::test_estimate_total_llm_usd_matches_hand_calculation:
  the hand calc now keys the grade half on claude-haiku-4-5 pricing
  ($0.80/MTok input, $4/MTok output) instead of reusing the draft model's
  pricing. The grade output-token figure stays the fixed
  _GRADE_OUTPUT_TOKENS_PER_CALL (50) — the 256->1024 max_output_tokens bump
  is a response cap, not the estimate's per-call output projection.

- tests/fixtures/grade/grade_event_v1.jsonl: cosmetic model id sonnet ->
  haiku for consistency with the new default (drift detector validates
  shape only; still green).

No production source changed. Full suite green: 3329 passed, 8 skipped.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Build the maintainer-run empirical gate for the #187 Haiku grade default.

- tests/research/187-haiku-calibration/_substrate.py: pinned Model +
  CandidateSchema (engineered-deterministic, spans the rubric calibration
  space) + curated Sonnet-baseline loader. Artifact-id set derived from
  the grade engine itself so it can't drift from the formatter.
- sonnet_baseline_sample.json: curated 44-verdict Sonnet baseline
  (11 artifacts x 4 DEFAULT_RUBRIC criteria), NOT the un-committed #179
  Phase-B dump; provenance documented.
- test_haiku_calibration.py (@pytest.mark.anthropic + runtime skip on
  ANTHROPIC_API_KEY): re-grades the sample with the resolved Haiku default
  (claude-haiku-4-5, max_output_tokens=1024), joins to the baseline by
  (artifact_id, criterion_id), asserts >=85% per-criterion concordance,
  prints the breakdown for the writeup.
- test_gemini_1024_no_truncation.py (@pytest.mark.gemini + runtime skip on
  SF_RUN_GEMINI/GOOGLE_API_KEY): grades a verbose artifact on
  gemini-2.5-flash @ 1024 tokens, asserts no score=None truncation degrade
  (verifies DEC-004).
- docs/research/187-haiku-calibration.md: writeup mirroring #179's
  structure with a clearly-marked "Result (maintainer-filled)" TODO.

Gating is belt-and-suspenders (existing anthropic/gemini markers excluded
in addopts + runtime skip). No production default changed; src/ untouched;
no new marker registered. Default `uv run pytest` stays fully green
(3329 passed, gated tests deselected) at 97% coverage.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…u grade default

Update every non-code surface to agree with the #187 shipped behaviour:
grade default model resolves per-provider (anthropic -> claude-haiku-4-5,
openai -> gpt-4o-mini, gemini -> gemini-2.5-flash) via PROVIDER_FAST_MODELS;
grade max_output_tokens default 256 -> 1024; model<->provider compat
validation via PROVIDER_SKU_PREFIXES.

- docs/grade-ops.md: model + max_output_tokens defaults; per-provider
  fast-default cost table (haiku/gpt-4o-mini/gemini-2.5-flash) with
  pricing.py figures; config example shows model auto-resolve.
- docs/llm-providers-ops.md: PROVIDER_FAST_MODELS table + gemini-2.5-flash
  1024-cap truncation note; default grader-model matrix row.
- docs/draft-ops.md: cheap_model shown as bare SKU claude-haiku-4-5.
- .claude/rules/grade-layer.md: locked-defaults section (per-provider
  sentinel resolution + compat validator).
- .claude/rules/llm-drafter.md: PROVIDER_FAST_MODELS / PROVIDER_SKU_PREFIXES
  source note (reusable by a future draft --cheap).
- CHANGELOG.md: Unreleased "Changed" entry.

README/CLAUDE.md unchanged (neither enumerates the grade default model;
the slim CLAUDE.md delegates API surfaces to CHANGELOG/docs by design).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Real bug (passes 1 & 2): GradeConfig.model could stay None for a
registered-but-not-in-PROVIDER_FAST_MODELS provider (the custom/plugin
growth path), contradicting the engine/_estimate `assert model is not None`.
Now fails loud at config-load requiring an explicit grade.model, restoring
the post-construction invariant. Updated the #135 neutrality test contract
+ the divergent-providers estimate test to supply an explicit model.

Accuracy fixes (passes 3 & 4):
- Soften the "gemini-2.5-flash JSON is not truncated at 1024" claim across
  config docstring, grade-ops.md, CHANGELOG — 1024 reduces but doesn't
  eliminate Gemini truncation at full-fixture scale (floors table wants 4096).
- grade-ops.md: gpt-4o example reframed as explicit override (default resolves
  to gpt-4o-mini).
- hand-calc estimate test: pin the resolved literal claude-haiku-4-5 so a
  resolver regression fails there instead of drifting into a tautology.
- calibration gate: reject degraded-dominated runs (comparable >= degraded).
- round-trip test docstring + example_config.yml comments corrected (fixture
  pins explicit overrides, not the new defaults).

CodeRabbit: unavailable in this environment (skipped). Full validation green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…gistry-table-invariant + calibration-gate patterns

Distil the durable, generalisable conventions surfaced by epic #187 into
the rule files so future work benefits:

- .claude/rules/grade-layer.md — new "Reusable conventions distilled from
  #187" subsection: (1) frozen-config default-from-sibling-field resolves in
  @model_validator(mode="before") not mode="after" (frozen=True forbids the
  after-mutation); (2) the load-bearing lesson — a default looked up in a
  table keyed by a registry-growable field must FAIL LOUD on a
  registered-but-absent key rather than leak a None/sentinel, citing the
  #187 QG bug as the cautionary example, generalised for any per-X default
  table.
- .claude/rules/testing-signal.md — two additions cross-referencing the
  e2e-gated conventions: gated calibration/concordance harness as a
  research-test pattern (tests/research/187-haiku-calibration/ — pinned
  baseline + marker + runtime skip + maintainer-run decision gate); and
  concordance-gate denominator hygiene (exclude degraded score=None pairs
  but fail loud when degraded dominates: comparable >= degraded).

Docs-only; no src/ or tests/ behaviour change. Full suite green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@wjduenow

wjduenow commented Jun 2, 2026

Copy link
Copy Markdown
Owner Author

Implementation complete (Ralph run)

All 8 beads of epic SignalForge-dpy landed on this branch; full validation green (ruff + pyright + 3329 passed, 97.24% cov; gated calibration/e2e tests deselected by default).

Shipped:

  • GradeConfig.model → per-provider sentinel resolved at config-load (PROVIDER_FAST_MODELS: anthropic→claude-haiku-4-5, openai→gpt-4o-mini, gemini→gemini-2.5-flash); explicit model: still honoured. Frozen-safe mode="before" resolver.
  • New model↔provider compatibility validation (fails loud at config-load on a SKU/provider mismatch).
  • GradeConfig.max_output_tokens default 256→1024.
  • DraftConfig.cheap_model aligned to bare SKU claude-haiku-4-5 (drafter still Sonnet).
  • Gated Haiku calibration harness + writeup under tests/research/187-haiku-calibration/.
  • 5-surface docs/rules/CHANGELOG lockstep.

Quality Gate (4 review passes): found + fixed one real bug — a registered-but-untabled custom provider left model=None, contradicting the engine's assert; now fails loud requiring an explicit grade.model. Plus accuracy fixes (notably softening the overstated "Gemini not truncated at 1024" claim — the floors table wants 4096 at full-fixture scale). CodeRabbit unavailable in this environment (skipped).

⚠️ Required before merge (maintainer gate)

Per DEC-005, run the calibration eval and confirm ≥85% Haiku/Sonnet concordance before merging the default flip:

ANTHROPIC_API_KEY=... uv run pytest -m anthropic --no-cov tests/research/187-haiku-calibration/

Note the pinned baseline is a curated sample, not the raw #179 dump (documented in the writeup).

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
tests/research/187-haiku-calibration/test_haiku_calibration.py (1)

110-115: ⚡ Quick win

Baseline already covers all (artifact_id, criterion_id) pairs; keep guard future-proof

The committed sonnet_baseline_sample.json contains all 44 unique (artifact_id, criterion_id) combinations (11 × 4). So the “denominator shrink” failure mode won’t happen with the current baseline data.

Lines 110-115 still only assert artifact-id coverage, though; if the baseline ever drops a single pair in the future, the test won’t fail loudly. Add a pair-level coverage assertion (check set(baseline) against the graded (artifact_id, criterion_id) keys) to keep the enforcement aligned with the documented intent.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/research/187-haiku-calibration/test_haiku_calibration.py` around lines
110 - 115, The test currently only asserts artifact-level coverage using
engine_ids and baseline_ids; change it to assert pair-level coverage: build the
set of graded pairs (artifact_id, criterion_id) that the engine will grade
(instead of engine_ids) and compare that to set(baseline); compute graded_pairs
= {...} from the same source you used to derive engine_ids (referencing
expected_artifact_ids usage) and then do baseline_pairs = set(baseline); let
missing = graded_pairs - baseline_pairs and assert not missing with a clear
message so the test fails if any (artifact_id, criterion_id) pair is missing.
src/signalforge/llm/providers.py (1)

359-378: ⚡ Quick win

Make the exported provider tables actually immutable.

These are documented as read-only source-of-truth tables, but plain dicts can still be mutated by any importer and silently change resolver / rollup behavior at runtime. Wrapping them in an immutable mapping would make the runtime contract match the docs.

♻️ Proposed change
+from types import MappingProxyType
-from typing import TYPE_CHECKING, Any, ClassVar
+from typing import TYPE_CHECKING, Any, ClassVar, Mapping
...
-PROVIDER_FAST_MODELS: dict[str, str] = {
+PROVIDER_FAST_MODELS: Mapping[str, str] = MappingProxyType({
     "anthropic": "claude-haiku-4-5",
     "openai": "gpt-4o-mini",
     "gemini": "gemini-2.5-flash",
-}
+})
...
-PROVIDER_SKU_PREFIXES: dict[str, str] = {
+PROVIDER_SKU_PREFIXES: Mapping[str, str] = MappingProxyType({
     "anthropic": "claude-",
     "openai": "gpt-",
     "gemini": "gemini-",
-}
+})
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/signalforge/llm/providers.py` around lines 359 - 378,
PROVIDER_FAST_MODELS and PROVIDER_SKU_PREFIXES are declared as plain dicts but
should be immutable; replace the mutable dicts with immutable mappings (e.g.,
wrap them in types.MappingProxyType) and update their type annotations to
typing.Mapping[str, str] (or the concrete MappingProxyType if preferred), import
types/typing as needed, and export the MappingProxyType instances so callers
cannot mutate these tables at runtime.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/signalforge/llm/providers.py`:
- Around line 359-378: PROVIDER_FAST_MODELS and PROVIDER_SKU_PREFIXES are
declared as plain dicts but should be immutable; replace the mutable dicts with
immutable mappings (e.g., wrap them in types.MappingProxyType) and update their
type annotations to typing.Mapping[str, str] (or the concrete MappingProxyType
if preferred), import types/typing as needed, and export the MappingProxyType
instances so callers cannot mutate these tables at runtime.

In `@tests/research/187-haiku-calibration/test_haiku_calibration.py`:
- Around line 110-115: The test currently only asserts artifact-level coverage
using engine_ids and baseline_ids; change it to assert pair-level coverage:
build the set of graded pairs (artifact_id, criterion_id) that the engine will
grade (instead of engine_ids) and compare that to set(baseline); compute
graded_pairs = {...} from the same source you used to derive engine_ids
(referencing expected_artifact_ids usage) and then do baseline_pairs =
set(baseline); let missing = graded_pairs - baseline_pairs and assert not
missing with a clear message so the test fails if any (artifact_id,
criterion_id) pair is missing.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9787ed31-cad5-4342-adb6-22718600f07a

📥 Commits

Reviewing files that changed from the base of the PR and between 0c08043 and 34847b3.

📒 Files selected for processing (28)
  • .claude/rules/grade-layer.md
  • .claude/rules/llm-drafter.md
  • .claude/rules/testing-signal.md
  • CHANGELOG.md
  • docs/draft-ops.md
  • docs/grade-ops.md
  • docs/llm-providers-ops.md
  • docs/research/187-haiku-calibration.md
  • src/signalforge/cli/_estimate.py
  • src/signalforge/draft/config.py
  • src/signalforge/grade/config.py
  • src/signalforge/grade/engine.py
  • src/signalforge/llm/cost/_rollup.py
  • src/signalforge/llm/providers.py
  • tests/cli/test_estimate_engine.py
  • tests/cli/test_generate_estimate.py
  • tests/draft/test_config.py
  • tests/fixtures/estimate/anthropic_byte_identity_golden.txt
  • tests/fixtures/grade/example_config.yml
  • tests/fixtures/grade/grade_event_v1.jsonl
  • tests/grade/test_config.py
  • tests/grade/test_provider_neutrality.py
  • tests/llm/cost/test_rollup.py
  • tests/llm/test_providers.py
  • tests/research/187-haiku-calibration/_substrate.py
  • tests/research/187-haiku-calibration/sonnet_baseline_sample.json
  • tests/research/187-haiku-calibration/test_gemini_1024_no_truncation.py
  • tests/research/187-haiku-calibration/test_haiku_calibration.py
✅ Files skipped from review due to trivial changes (6)
  • tests/research/187-haiku-calibration/sonnet_baseline_sample.json
  • docs/draft-ops.md
  • CHANGELOG.md
  • .claude/rules/llm-drafter.md
  • tests/fixtures/grade/example_config.yml
  • docs/grade-ops.md

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements the #187 plan to make grading faster/cheaper by switching the grade layer to per-provider “fast default” models (Haiku / 4o-mini / Gemini flash), raising the grade output cap to reduce Gemini truncation risk, and adding config-time provider↔model compatibility validation plus supporting docs/tests/fixtures.

Changes:

  • Add PROVIDER_FAST_MODELS and PROVIDER_SKU_PREFIXES as single sources of truth and refactor cost rollup to use them.
  • Update GradeConfig to resolve model=None to a provider fast default at config-load, validate model/provider compatibility, and bump max_output_tokens default 256 → 1024.
  • Update estimates/fixtures/docs and add maintainer-gated calibration harnesses under tests/research/.

Reviewed changes

Copilot reviewed 29 out of 29 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
src/signalforge/llm/providers.py Adds exported per-provider fast model + SKU-prefix constants.
src/signalforge/llm/cost/_rollup.py Derives prefix dispatch table from PROVIDER_SKU_PREFIXES to avoid duplicated literals.
src/signalforge/grade/config.py Implements sentinel model resolution + provider/model compatibility validation; raises default output cap.
src/signalforge/grade/engine.py Narrows config.model to a concrete string where required.
src/signalforge/draft/config.py Aligns DraftConfig.cheap_model with priced bare SKU (claude-haiku-4-5).
src/signalforge/cli/_estimate.py Updates estimate path to use resolved grade model (assert non-None) for token counting and pricing lookup.
tests/llm/test_providers.py Adds unit tests pinning fast-model/prefix tables and export surface.
tests/llm/cost/test_rollup.py Adds tests ensuring rollup provider classification remains identical post-refactor.
tests/grade/test_config.py Updates default assertions and adds coverage for resolver + compatibility validator + new token cap default.
tests/grade/test_provider_neutrality.py Updates plugin-provider behavior to require explicit model when no built-in default exists.
tests/draft/test_config.py Updates default cheap_model assertion to bare SKU.
tests/cli/test_generate_estimate.py Adjusts custom-provider estimate test to include explicit grade model.
tests/cli/test_estimate_engine.py Updates expected USD math and explicitly pins resolved Haiku grade default in the test.
tests/fixtures/grade/grade_event_v1.jsonl Updates fixture model field to new default (claude-haiku-4-5).
tests/fixtures/grade/example_config.yml Clarifies explicit overrides vs auto-resolved defaults in the example config fixture.
tests/fixtures/estimate/anthropic_byte_identity_golden.txt Updates estimate golden to reflect new grade default + cost changes.
tests/research/187-haiku-calibration/_substrate.py Adds pinned calibration substrate builders + baseline loader.
tests/research/187-haiku-calibration/sonnet_baseline_sample.json Adds committed baseline verdict sample for calibration gate.
tests/research/187-haiku-calibration/test_haiku_calibration.py Adds maintainer-gated Haiku-vs-baseline concordance harness.
tests/research/187-haiku-calibration/test_gemini_1024_no_truncation.py Adds maintainer-gated Gemini “1024 doesn’t truncate” check.
docs/research/187-haiku-calibration.md Documents calibration method, substrate, and maintainer-run instructions/result placeholder.
docs/llm-providers-ops.md Updates provider ops docs for per-provider grade defaults and token-cap note.
docs/grade-ops.md Updates grade ops docs for new default model resolution and 1024 cap guidance.
docs/draft-ops.md Updates draft ops docs to reflect bare cheap_model SKU.
CHANGELOG.md Adds changelog entry describing per-provider grade defaults, cap bump, and compat validation.
.claude/rules/testing-signal.md Adds documented pattern for maintainer-run concordance harnesses and denominator hygiene.
.claude/rules/llm-drafter.md Documents new provider mapping constants and intended reuse pattern.
.claude/rules/grade-layer.md Updates grade-layer rules for new defaults/resolution/compat validator patterns.
plans/super/187-fast-grade-defaults.md Adds/updates the #187 “super plan” document describing decisions and story breakdown.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/research/187-haiku-calibration/test_haiku_calibration.py Outdated
Comment thread tests/research/187-haiku-calibration/test_haiku_calibration.py
Comment thread tests/research/187-haiku-calibration/test_haiku_calibration.py
Comment thread tests/research/187-haiku-calibration/sonnet_baseline_sample.json Outdated
Comment thread docs/llm-providers-ops.md Outdated
Comment thread docs/llm-providers-ops.md Outdated
Comment thread docs/grade-ops.md Outdated
Comment thread plans/super/187-fast-grade-defaults.md Outdated
Replaces the hand-authored US-005 calibration baseline with a genuine one,
per request:
- capture_sonnet_baseline.py drafts artifacts for the real intuit_airflow
  model calendar_hour (schema-only, no warehouse), freezes them to
  real_candidate.json, and grades them with claude-sonnet-4-6.
- _substrate.py: build_model() is now the real model (deterministic);
  build_candidate() loads the frozen drafted artifacts.
- sonnet_baseline_sample.json: 80 live Sonnet verdicts (4 degraded excluded).

RESULT — the gate FAILS: Haiku concordance 81.8% / 77.0% on two runs, below
the 85% DEC-005 bar. Divergence is systematic (Haiku stricter than Sonnet,
concentrated on no-redundant + clarity). Per DEC-005 this points to shipping
Haiku as opt-in, NOT the default. Full analysis in
docs/research/187-haiku-calibration.md § Result / Disposition.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@wjduenow

wjduenow commented Jun 2, 2026

Copy link
Copy Markdown
Owner Author

⚠️ Real calibration result: Haiku is BELOW the 85% bar — reconsider the default

The hand-authored calibration baseline has been replaced with a real one (drafted from the intuit_airflow model calendar_hour, schema-only; graded live with claude-sonnet-4-6 = 80 verdicts, 63 pass / 17 fail). Re-grading the same frozen artifacts with the Haiku default:

Run Comparable Agreement Verdict
1 77 81.8% FAIL (<85%)
2 74 77.0% FAIL (<85%)

Both runs miss the DEC-005 ≥85% bar, and the divergence is systematic, not noise: ~80% of discordances are sonnet=pass → haiku=failHaiku grades stricter than Sonnet, concentrated on no-redundant (8) and clarity (4). A Haiku default would flag more artifacts than the Sonnet baseline operators calibrate against.

Per DEC-005 (<85% → opt-in, not default), the recommendation is to NOT ship Haiku as the resolved Anthropic grade default — keep Sonnet the default and document grade.model: claude-haiku-4-5 as the opt-in fast mode. The resolver, compat validator, and 1024 cap (US-001..US-006) all stand; only the Anthropic default target would change. OpenAI/Gemini fast defaults are unaffected (explicit operator choices, never calibrated against Sonnet).

Full analysis + the three disposition options: docs/research/187-haiku-calibration.md § Result / Disposition. The gated test deliberately still asserts ≥85% (it fails on Haiku) so the signal isn't silently lost. This is a maintainer product decision.

The empirical calibration (real Sonnet baseline from intuit_airflow) found
claude-haiku-4-5 grades stricter than Sonnet (~77-82% concordance, below the
85% DEC-005 bar). Per the decision rule, Haiku does NOT ship as the default.

- Rename PROVIDER_FAST_MODELS -> PROVIDER_DEFAULT_MODELS (Sonnet isn't "fast");
  anthropic -> claude-sonnet-4-6 (was claude-haiku-4-5). OpenAI/Gemini fast
  defaults unchanged (gpt-4o-mini / gemini-2.5-flash).
- GradeConfig() now resolves to claude-sonnet-4-6; claude-haiku-4-5 is the
  documented opt-in (grade.model: claude-haiku-4-5).
- Revert estimate goldens + grade_event fixture + hand-calc to Sonnet
  (estimate is independent of max_output_tokens, so origin/dev values apply).
- Kept: the per-provider resolver, model<->provider compat validator, the
  1024 max_output_tokens cap (still justified by the gemini-2.5-flash default).
- Calibration gate now selects Haiku explicitly and still asserts >=85% (fails)
  as the durable record that Haiku is the stricter opt-in.
- Docs/rules/CHANGELOG/writeup updated across all surfaces.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@wjduenow

wjduenow commented Jun 3, 2026

Copy link
Copy Markdown
Owner Author

Decision implemented: Sonnet stays the grade default, Haiku is opt-in

Per the calibration finding (Haiku ~77-82% concordance vs Sonnet, below the 85% bar), Option 1 is implemented:

  • PROVIDER_FAST_MODELS → renamed PROVIDER_DEFAULT_MODELS (Sonnet isn't "fast"); anthropicclaude-sonnet-4-6. OpenAI/Gemini keep their fast defaults (gpt-4o-mini / gemini-2.5-flash).
  • GradeConfig() resolves to Sonnet; grade.model: claude-haiku-4-5 is the documented opt-in (faster/cheaper, but stricter).
  • Estimate goldens + grade fixture + hand-calc reverted to Sonnet (estimate is independent of max_output_tokens).
  • Kept the grade: switch default grader to Haiku-4-5 (Sonnet's reasoning depth is wasted on rubric scoring) #187 machinery: per-provider resolver (fixes the provider: openai/gemini leak), model↔provider compat validator, and the 1024 cap (still justified by the gemini-2.5-flash default).
  • The gated calibration test now selects Haiku explicitly and still asserts ≥85% (so it fails) — the durable record that Haiku is the stricter opt-in.

ruff + pyright + 3329 passed. All docs/rules/CHANGELOG/writeup surfaces updated.

wjduenow and others added 3 commits June 2, 2026 22:31
gemini-2.5-flash graded a verbose artifact at the new max_output_tokens=1024
default with no score=None truncation degrade — DEC-004 holds at the
single-artifact scale (full-fixture runs may still want 4096 per #158).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- test_haiku_calibration: lazy _substrate import inside the test (no
  import-time sys.path mutation when deselected) — Copilot.
- docstrings: 'never collects' -> 'deselected'; 'Haiku default' -> 'Haiku
  opt-in' across the calibration tests — Copilot.
- llm-providers-ops: drafter row now honest (no per-provider drafter
  default; switching provider requires llm.model) with a footnote — Copilot.
- plan doc: ```text fence on the dependency graph + blank line before the
  SKU table (markdownlint) — CodeRabbit; Phase -> complete.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@wjduenow

wjduenow commented Jun 3, 2026

Copy link
Copy Markdown
Owner Author

PR Review Summary (CodeRabbit + Copilot)

Also resolved the dev merge conflict (CHANGELOG.md — kept both #185 and #187 entries); branch is now MERGEABLE and the combined tree is green (3373 passed).

Fixed

File Issue Reviewer
tests/research/187-haiku-calibration/test_haiku_calibration.py Import-time sys.path mutation leaked at collection (test deselected but still imported) → moved sys.path insert + _substrate import lazily inside the test, after the skip Copilot
same + test_gemini_1024_no_truncation.py Docstring "never collects" was inaccurate (module IS imported during collection) → "deselected … body never runs" Copilot
test_haiku_calibration.py Stale "Haiku default" framing → "Haiku opt-in" (the gate now selects Haiku explicitly; Sonnet is the default) contextual
docs/llm-providers-ops.md "Default drafter model" row implied per-provider drafter defaults that don't exist → made honest (claude-sonnet-4-6 regardless of provider; switching provider requires llm.model) + footnote Copilot
plans/super/187-fast-grade-defaults.md ```text fence on the dependency graph; blank line before the SKU table (MD058); Phase → complete CodeRabbit + Copilot

Addressed by subsequent work (valid when filed, since fixed)

File Comment Resolution
sonnet_baseline_sample.json "Sonnet baseline verdicts are hand-assigned → should be a real Sonnet run" Done — the baseline is now a live claude-sonnet-4-6 run drafted from the real intuit_airflow calendar_hour model (capture_sonnet_baseline.py). This is exactly what surfaced the result that kept Haiku opt-in.
docs/llm-providers-ops.md / docs/grade-ops.md "PROVIDER_FAST_MODELS isn't the cheapest SKU; don't call it cheapest" Done — renamed PROVIDER_FAST_MODELSPROVIDER_DEFAULT_MODELS and reworded to "default judge model" (anthropic → Sonnet) in the Option-1 reversal.

No false positives, no deferrals — every thread is either fixed or addressed above.

@wjduenow
wjduenow merged commit 4a70799 into dev Jun 3, 2026
6 checks passed
@wjduenow
wjduenow deleted the feature/187-fast-grade-defaults branch June 3, 2026 04:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants