#198: Grade budget — scale wall-clock with work + opt-in cost ceilings - #199
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughRedesigns grade-stage timing to compute an effective wall-clock budget (base + per-pair × waves) with optional hard cap, adds opt-in soft ceilings for calls/cost/tokens that preempt scheduling and degrade unstarted pairs, and updates config, engine, warnings, fixtures, tests, and documentation to the new contract. ChangesGrade budget scaling and ceiling implementation
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
wjduenow
added a commit
that referenced
this pull request
Jun 4, 2026
wjduenow
force-pushed
the
feature/198-grade-budget-scaling
branch
from
June 4, 2026 05:09
da8ce3c to
502847d
Compare
…validator split total_budget_seconds becomes int | None = None (optional absolute hard ceiling; None routes to the scaled formula). Add budget_base_seconds=60, budget_per_pair_seconds=20.0, and the three opt-in soft ceilings max_grade_calls / max_grade_cost_usd / max_grade_tokens (all None=off). Split the _positive field_validator: positive-only for max_output_tokens / budget_base_seconds / budget_per_pair_seconds; allow-None-or-positive for total_budget_seconds and the three max_grade_*. Existing fixtures (300/600 explicit ints) still load; extra=forbid still rejects typos. Traces DEC-001/002/004/006 of plans/super/198-grade-budget-scaling.md.
…+ formula tests
…eld + 40-col AC test
…+ tighten config validator/round-trip tests
…run abort + demo comment parity The cost ceiling looked up pricing per-pair inside the TaskGroup; a prefix-valid- but-unpriced model (e.g. claude-opus-4-8) + max_grade_cost_usd raised EstimateUnknownModelError mid-run (uncaught by the per-pair except), aborting the run after billable calls. Resolve pricing ONCE up front so an unpriced SKU fails fast at orchestrator entry before any LLM call; reuse the resolved pricing per pair. Found by the CodeRabbit-substitute review pass.
…get + ceilings contract
wjduenow
marked this pull request as ready for review
June 4, 2026 14:51
wjduenow
changed the title
There was a problem hiding this comment.
Pull request overview
This PR implements issue #198 by recalibrating the grade-stage wall-clock budget to scale with (artifact × criterion) work (and concurrency), and adds opt-in “soft” ceilings for grade calls / USD cost / tokens that stop further dispatch by degrading remaining pairs (rather than raising new typed errors). This fits into the grade engine’s existing “graceful degrade, never silent drop” contract and preserves pinned-config semantics when total_budget_seconds is explicitly set.
Changes:
- Add scaled-budget terms to
GradeConfig(budget_base_seconds,budget_per_pair_seconds) and reinterprettotal_budget_secondsas an optional absolute cap (Nonedefault). - Wire a computed
effective_budgetinto the engine timeout/degrade strings and rename the budget WARNING payload field toeffective_budget_seconds. - Add opt-in ceilings (
max_grade_calls,max_grade_cost_usd,max_grade_tokens) with a newgrade ceiling exceededWARNING, plus extensive unit tests and updated operator docs/fixtures.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/signalforge/grade/engine.py |
Adds _compute_effective_budget, uses effective_budget for timeout/degrade/warnings, and implements calls/cost/tokens ceilings + warning. |
src/signalforge/grade/config.py |
Updates budget semantics/defaults; adds scaled-budget and ceiling fields with validators. |
tests/grade/test_engine.py |
Adds formula tests, wide-model acceptance test, and ceiling-behavior tests (incl. unpriced SKU fast-fail). |
tests/grade/test_config.py |
Updates defaults expectations and adds validator coverage for new fields. |
docs/grade-ops.md |
Documents scaled budget, optional cap semantics, ceilings, and updated warning shapes. |
tests/fixtures/grade/example_config.yml |
Updates example grade config with scaled-budget terms and commented ceiling knobs. |
tests/fixtures/dbt_project_austin/signalforge.yml |
Clarifies total_budget_seconds comment as absolute hard cap under new semantics. |
src/signalforge/_demo/signalforge.yml |
Same comment clarification as the Austin fixture. |
plans/super/198-grade-budget-scaling.md |
Adds the detailed plan/decisions/story breakdown for #198. |
.claude/rules/grade-layer.md |
Updates grade-layer rules to reflect effective budget + ceilings and the warning rename/new warning. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
tests/grade/test_engine.py (1)
2173-2177: ⚡ Quick winUse a synthetic
claude-sentinel SKU to keep this test future-proof.This test can become flaky if that concrete SKU gets added to
pricing.PRICES; a clearly synthetic prefix-valid value keeps the intent stable.Suggested fix
- model="claude-opus-4-8", # claude- prefix (valid) but NOT in PRICES + model="claude-unpriced-test-sku", # claude- prefix-valid sentinel, intentionally not priced🤖 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/grade/test_engine.py` around lines 2173 - 2177, The test uses a concrete model SKU "claude-opus-4-8" which could later be added to pricing.PRICES and make the test flaky; update the GradeConfig model to a clearly synthetic, prefix-valid sentinel SKU (e.g., a value starting with "claude-" but not a real SKU such as "claude-sentinel-000") so the test intention remains stable—modify the model parameter passed to GradeConfig in the failing test block (the GradeConfig(...) instantiation) to use that synthetic SKU.
🤖 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 `@docs/grade-ops.md`:
- Around line 96-97: The docs currently state that GradeBudgetExceededError is
raised, but the grade-layer contract marks it as reserved/not raised; update the
GradeBudgetExceededError entry so it clearly indicates this error is reserved
and not emitted by the operator (e.g., "reserved — not raised by operator" or
similar), and adjust any accompanying text about behavior (such as references to
GradingReport.aggregate_complete) to reflect that a partial run will be reported
without raising this error; apply the same change to the other occurrence of
GradeBudgetExceededError further down the file.
- Around line 527-531: The markdown anchor "[Logging](`#logging`)" in the ceiling
note is invalid; update the link target or add a header: either change the
anchor text to point to the existing section (e.g., replace
"[Logging](`#logging`)" with "[Debugging](`#debugging`)" or the correct existing
heading) or add a new "## Logging" header in this document so the current link
resolves; edit the ceiling note containing the anchor to ensure it matches the
actual heading name.
In `@plans/super/198-grade-budget-scaling.md`:
- Line 47: Remove the trailing space inside the inline code span by editing the
backticked token `result.input_tokens / output_tokens / cache_* ` to
`result.input_tokens / output_tokens / cache_*` so the inline code has no space
before the closing backtick (fixes markdownlint MD038).
In `@src/signalforge/grade/config.py`:
- Around line 402-416: The validators _positive and _optional_positive currently
only check v <= 0 which lets NaN and ±Infinity slip through; update both
validators to explicitly reject non-finite float values (use math.isfinite or
equivalent) before the existing non-positive check (for _optional_positive, skip
validation when v is None). Apply this to the field validators used for
max_output_tokens, budget_base_seconds, budget_per_pair_seconds and the optional
ceilings like max_grade_cost_usd so NaN/Infinity raise ValueError("must be
finite") (or similar) rather than corrupting budget math.
---
Nitpick comments:
In `@tests/grade/test_engine.py`:
- Around line 2173-2177: The test uses a concrete model SKU "claude-opus-4-8"
which could later be added to pricing.PRICES and make the test flaky; update the
GradeConfig model to a clearly synthetic, prefix-valid sentinel SKU (e.g., a
value starting with "claude-" but not a real SKU such as "claude-sentinel-000")
so the test intention remains stable—modify the model parameter passed to
GradeConfig in the failing test block (the GradeConfig(...) instantiation) to
use that synthetic SKU.
🪄 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: e8b03a94-d792-4464-ab2b-067f21939ce5
📒 Files selected for processing (10)
.claude/rules/grade-layer.mddocs/grade-ops.mdplans/super/198-grade-budget-scaling.mdsrc/signalforge/_demo/signalforge.ymlsrc/signalforge/grade/config.pysrc/signalforge/grade/engine.pytests/fixtures/dbt_project_austin/signalforge.ymltests/fixtures/grade/example_config.ymltests/grade/test_config.pytests/grade/test_engine.py
wjduenow
changed the title
- engine: math.ceil (not int) on effective budget so a fractional per_pair never rounds the backstop DOWN - engine: route ceiling-degrade audit write through run_in_executor+shield (DEC-017) so the fsync doesn't block the concurrent loop (was sync); set slot first to avoid double-audit - config: reject non-finite floats (.nan/.inf) in budget_per_pair_seconds / max_grade_cost_usd (yaml parses them; nan<=0 is False so they slipped past the positivity guard) - docs: GradeBudgetExceededError marked reserved/not-raised in v0.1 (both error reference + table); fix invalid #logging anchor -> #debugging; drop trailing space in plan code span - tests: NaN/inf rejection, fractional-ceil (no round-down), cap-ceil - grade-layer.md: note updated to shielded-executor ceiling-degrade write
Owner
Author
PR Review Summary — CodeRabbit + CopilotAll 7 threads addressed in Fixed (7 items)
False PositivesNone — all comments were valid. The grade-layer.md "ceiling-degrade write" note was updated in lockstep to describe the shielded-executor approach. |
Closed
6 tasks
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Re-calibrate the grade-stage budget (issue #198). The grade engine makes one LLM call per
(artifact × criterion)pair, dispatched concurrently; the old flat 300s wall-clock budget wassized for the pre-#186 sequential era and undercounted realistic work by ~3–4×.
effective = budget_base_seconds(60) + budget_per_pair_seconds(20.0) × ceil(num_pairs / max_concurrent_calls).total_budget_secondsisreinterpreted as an optional absolute cap (
int | None, defaultNone);effective = min(scaled, total_budget_seconds)when set, preserving v0.1 semantics for pinned configs. The20.0per-wave default is grounded in the Epic: Test Generation Expansion (#169 + #170 + #171) #179 baseline (~10s/Sonnet-judge-call) at ~2.25× headroom.max_grade_calls/max_grade_cost_usd/max_grade_tokens(all
None= off). Whichever trips first stops scheduling new pairs; remaining pairs degrade(never raise) with a reason naming the ceiling. Cost via
signalforge.llm.pricing.Design shape
Ceilings degrade (don't raise) → no new typed errors, no exit-code/AST-scan churn, no CLI flags, no
audit_schema_versionbump.GradeBudgetExceededErrorstays reserved. Budget WARNING field renamedtotal_budget_seconds→effective_budget_seconds; newgrade ceiling exceededWARNING names thetripped ceiling. Full decision record:
plans/super/198-grade-budget-scaling.md(DEC-001…011).Changes
grade/config.py— new fields + validator split (total_budget_seconds: int | None).grade/engine.py— pure_compute_effective_budgethelper; scaled budget wired intoasyncio.timeout;soft/best-effort ceilings in
_one(calls near-hard via pre-await slot reservation; cost/tokenspost-call accumulators); cost pricing resolved once up front (fail-fast on unpriced SKU).
docs/grade-ops.md,.claude/rules/grade-layer.md,tests/fixtures/grade/example_config.yml— parity.Testing
degradations under default config" AC test (488 pairs); unpriced-SKU fail-fast regression test.
(per-pair pricing lookup on an unpriced SKU) — fixed + regression-tested.
Compounding update
.claude/rules/grade-layer.md(scaled-budget + ceilings contract, DEC-029 enumeration),docs/grade-ops.md,bd memory
grade-runtime-budgets-198-scale-a-wall-clock.Maintainer-only remaining (closing AC)
Re-run
tests/research/179-runtime-benchmark/benchmark_runtime.pyonweekly_query_cost+ a ≥40-colmodel (live Anthropic key) and record numbers in
docs/research/179-runtime-benchmark.md.Closes #198.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests