#197: grade cache default off (can't speed up generate re-runs)

[wjduenow]

Summary

Closes the actionable part of #197: the #189 grade cache provides no speed-up on signalforge generate re-runs, because its key mixes a hash of the drafted artefact text and the drafter is a live, non-deterministic LLM — so every re-draft rotates the key and misses on every pair (measured: 370 entries written, 0 read back; docs/research/179-runtime-benchmark.md).

Decision (per the #197 thread): flip GradeConfig.cache_enabled default True → False, defer the delete.

Why flip rather than delete

• The cache is cross-invocation only — all lookups run in grade_artifacts's sync prefix before any write of the current run, so there's zero intra-run reuse. Single-run speed is the grade: parallelise per-(artifact × criterion) calls via asyncio.gather (deferred-to-v0.2 graduation) #186 asyncio fan-out, not the cache.
• Left on by default it silently wrote hundreds of never-hit .signalforge/grade-cache/*.json files on the common path and implied a "re-run is fast" UX the architecture can't deliver.
• The keying is correct (changed text should re-grade), so the cache stays in the code, opt-in via grade.cache_enabled: true for the narrow cross-run paths where candidate text is identical: pinned-candidate CI, --no-grade draft-then-grade, a resumed grade.
• Option 3 (re-key off stable identity) rejected — it would replay stale verdicts against changed text. A future "fast re-run" UX needs a draft cache; this grade cache is the already-correct second half of that.

Changes

• src/signalforge/grade/config.py — cache_enabled default True → False + rewritten docstring (cross-run-only, non-determinism caveat, opt-in guidance).
• docs/grade-ops.md — corrected the overclaiming "Grade cache" section (the "re-run hits cache on every pair" claim was the false one), the field reference, the yml example, and the hits/misses guidance.
• src/signalforge/skills/signalforge/SKILL.md — same correction on the parity surface.
• tests/grade/test_config.py — default-assertion flipped (..._defaults_false + the DEC-defaults block).
• tests/grade/test_engine.py — new _config_cache_on() opt-in helper; the ~11 cache-behaviour tests (which relied on the old default-on) now opt in.

Validation

uv run pytest tests/grade/ tests/cli/test_generate.py tests/cli/test_cache.py tests/cli/test_skill_cli_parity.py --no-cov → 532 passed. ruff check / ruff format --check clean on changed files.

Follow-on (separate ticket, only if "fast re-run" is wanted)

A draft cache keyed on stable draft inputs (model raw_code hash, neighbour manifest summary, prompt_version, exclude_tests, provider/model) that composes with this grade cache — identical re-drafted text → grade cache hits for free.

Refs #197.

Summary by CodeRabbit

• Bug Fixes

  • Grade cache is now disabled by default; enable via configuration to use cross-invocation caching.

• Documentation

  • Updated grade cache documentation with clarified content-addressing behavior and cache hit/miss scenarios (pinned candidates, committed drafts, resumed grading).

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e2e9ea91-d75c-4cbc-bf6a-56cb22b59aee

📥 Commits

Reviewing files that changed from the base of the PR and between f770c5e and 6cd2121.

📒 Files selected for processing (5)

• docs/grade-ops.md
• src/signalforge/grade/config.py
• src/signalforge/skills/signalforge/SKILL.md
• tests/grade/test_config.py
• tests/grade/test_engine.py

---

📝 Walkthrough

Walkthrough

The PR changes the grade cache behavior from enabled by default to disabled by default (issue #197), updates documentation to reflect opt-in usage, and adapts tests to explicitly enable cache where needed to exercise cache paths under the new default.

Changes

Grade cache off-by-default (issue #197)

Layer / File(s)	Summary
Config default and docstring src/signalforge/grade/config.py	GradeConfig.cache_enabled default flips from True to False. Docstring rewritten to describe cache as cross-invocation only, content-addressed via blake2b-8, and opt-in via cache_enabled: true in config.
Operator documentation updates docs/grade-ops.md, src/signalforge/skills/signalforge/SKILL.md	Operator guides clarified that cache is off by default (issue #197), cross-invocation only (does not accelerate generate reruns due to artifact text rotation), and enabled via cache_enabled: true. Configuration examples and cache-hit scenarios updated to remove prior default-on expectations and document when hits occur (pinned/committed candidates, draft-once-then-grade, resumed grading over unchanged artifact).
Config default tests tests/grade/test_config.py	Tests updated to assert GradeConfig.cache_enabled defaults to False and DEC-023..DEC-027 defaults regression-guard expects cache_enabled=False.
Engine tests cache opt-in tests/grade/test_engine.py	New _config_cache_on() helper provides cache-enabled test fixtures. Cache-related tests (hit, miss, invalidation, degraded, write-failure, dispatch, symlink, provenance, sweep) updated to opt-in to cache via _config_cache_on() or explicit cache_enabled=True so grader cache lookup/write paths are exercised deterministically under the new default-off posture.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

• Grade cache (#189) keyed on artifact_text_hash → no benefit on full generate re-runs #197: Main issue addressed. PR directly flips GradeConfig.cache_enabled default to False and updates all documentation and tests to reflect cache as opt-in rather than always-on, eliminating cache misses on every generate rerun that was described in the issue.

Poem

A cache that defaults to sleep,
Now operators opt-in the keep,
No misses on reruns today,
Just tests that show the better way,
Off by default, fast when needed here. 🐰✨

🚥 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 PR title accurately summarizes the main change: flipping the grade cache default from on to off, with a brief clarification of the key limitation (can't speed up re-runs because cache key includes LLM-drafted text).
Docstring Coverage	✅ Passed	Docstring coverage is 100.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.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

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