Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .claude/rules/business-rule-tests.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ Established by issue #116. Apply to any code touching the `custom_sql` candidate

`meta.signalforge.business_rules` (NL `str` or `list[str]`, column- and model-level) is read with the **safety-layer `meta.get("signalforge")` dict-guard pattern** (strict `isinstance(dict)`; a scalar/list under the key is config noise, dropped — never fail loud). Rules render into the drafter's **dynamic (non-cached) block**, NOT the cached system prompt — so they don't perturb the prompt-cache golden. When no rules are declared, the inferred-fallback prompt still permits `custom_sql`. `_PROMPT_VERSION` rotated when the `custom_sql` catalogue entry landed in the cached system prompt; the cache-stability snapshot moved in lockstep (`llm-drafter.md`). `exclude_tests` can name `"custom_sql"` (it's in `VALID_TEST_TYPES`).

**Numbered envelope + parser cardinality gate (#163).** Drafter instruction-following on `business_rules` was fragile on `claude-sonnet-4-6` — a plain bulleted list let the model silently skip rules and hallucinate unrelated ones (issue #163, observed in the live e2e). The fix is a **gate-over-prompt** pair (testing-signal.md §):

- **Dynamic-block envelope (DEC-009 of #163).** Each rule renders inside `<BUSINESS_RULE id="N">…</BUSINESS_RULE>` (IDs 1-indexed, body indented 2 spaces, scope prefix `(model)` / `(column X)` preserved). The envelope mirrors `<MODEL_SQL>` exactly — clearer reference targets for the LLM AND a fence-shaped surface for the prompt-injection breach guard. A rule containing the literal `</BUSINESS_RULE>` substring raises `PromptEnvelopeBreachError(envelope="BUSINESS_RULE", rule_index=N)` BEFORE any LLM call (boring substring match — no whitespace / case normalisation; opening tag alone is allowed, truncated fragments like `</BUSINESS_RUL` are allowed; only the exact closing tag breaks the fence). The error class is the same one used for `</MODEL_SQL>` — **parameterised, not subclassed** (DEC-005 of #163). Future envelopes follow the same shape: extend with a new `envelope=` value, never a new error class.
- **Parser cardinality gate (DEC-002 / DEC-006 of #163).** `_validate_anchor_contract` enforces **at-least-one-`custom_sql`-per-declared-rule**: when `business_rules` is non-empty AND `"custom_sql"` is NOT in `exclude_tests`, `custom_sql_count < len(business_rules)` appends one violation to the existing collect-all `violations` list (surfaced via `LLMOutputAnchorContractError`, CLI tier 2). At-least, not exact-equality — the LLM may legitimately decompose a complex rule into two SELECTs. The violation message names every declared rule verbatim (`repr()`-quoted with the existing `(model)` / `(column X)` prefix) so the operator sees exactly what was declared. Gate is a no-op on `business_rules=()` (preserves all existing parser-test call sites + the inferred-fallback path) and on `"custom_sql" in exclude_tests` (DEC-008 of #163). The threading mirrors #159's `model_columns_by_type` (single source of truth: built once in `signalforge.draft.schema._draft_from_request`, passed keyword-only through `parse_draft_response` → `_validate_anchor_contract`).
- **`exclude_tests=("custom_sql",)` short-circuits both surfaces.** The renderer returns `""` (don't tell the LLM to draft tests the operator forbade); the parser gate becomes a no-op. The two layers are orthogonal — the per-test `exclude_tests` filter at the parser still catches any defying `custom_sql` the LLM emits.

**No `_PROMPT_VERSION` rotation.** Everything #163 changes is dynamic-block-side or parser-side; the cached system prompt is untouched. `tests/llm/test_prompt_cache_stability.py` is the load-bearing gate that pins this — touch the cached system prompt and it fails loudly.

## Bounded Jinja resolution — NO Jinja engine (DEC-004, DEC-005)

`signalforge.manifest.resolve_template_refs(sql, model, manifest) -> str` substitutes `{{ this }}` (→ `model.resolve_this()`), `{{ ref('m') }}` / `{{ ref('pkg','m') }}` / version forms, and `{{ source('s','t') }}` to qualified names via the manifest source registry + `resolve_ref`/`resolve_source` (also #116). Control-flow Jinja (`{% … %}`, `{{ var() }}`, `{{ env_var() }}`, macros) and any residual `{{ }}` after substitution are **rejected loudly** (`UnsupportedJinjaError`/`TemplateResolutionError`, in the existing `manifest/errors.py` — do NOT add a new `errors.py`, scan-7 asserts exactly 11). The resolver lives in the **manifest layer** (stage-0, no logging) so both prune and ingest consume it without a cross-stage import.
Expand Down
10 changes: 7 additions & 3 deletions .claude/rules/cli-layer.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,13 @@ The CLI is the orchestration layer (NOT a stage-0 reader) so it IS allowed to em

## 7th AST scan: every typed exception has an exit-code mapping (DEC-024)

`tests/test_audit_completeness.py::test_every_typed_error_is_in_exit_code_mapping_table` walks every `src/signalforge/*/errors.py`, collects each `class <Name>Error(...):` via `ast.ClassDef`, and asserts the class is registered in `_EXCEPTION_TO_EXIT_CODE`. Excludes the eleven per-stage abstract bases (frozenset `_EXCEPTION_MAPPING_EXCLUDED_BASES`: `ManifestError`, `WarehouseError`, `SafetyError`, `LLMError`, `DraftError`, `PruneError`, `GradeError`, `DiffError`, `CliError`, `DemoError`, `IngestError`); subclasses inherit via MRO.
`tests/test_audit_completeness.py::test_every_typed_error_is_in_exit_code_mapping_table` walks every `src/signalforge/*/errors.py` **and every `src/signalforge/*/*/errors.py`** (depth-1 ∪ depth-2 glob — see "Depth-2 glob extension" below), collects each `class <Name>Error(...):` via `ast.ClassDef`, and asserts the class is registered in `_EXCEPTION_TO_EXIT_CODE`. Excludes the twelve per-stage abstract bases (frozenset `_EXCEPTION_MAPPING_EXCLUDED_BASES`: `ManifestError`, `WarehouseError`, `SafetyError`, `LLMError`, `DraftError`, `PruneError`, `GradeError`, `DiffError`, `CliError`, `DemoError`, `IngestError`, `CostError`); subclasses inherit via MRO.

**Dual registration (issue #59).** Nine of the eleven abstract bases are ALSO registered in `_EXCEPTION_TO_EXIT_CODE` at a single fallback tier (`ManifestError`/`DiffError`/`CliError` → 1; `DraftError` → 2; `LLMError`/`WarehouseError`/`GradeError`/`SafetyError`/`PruneError` → 3). Two independent roles: the frozenset excludes bases from the AST scan's required-mapping check; the table entry is a forward-compat safety net so a new concrete subclass that forgets a table entry still gets the parent's tier via the MRO walk rather than dropping to the panic-path tier 1. The AST scan still fails loud on the missing per-class entry — fallback is safety net, not substitute. `DemoError` and `IngestError` (issue #104) are the two deliberate exceptions: their concretes span tiers 1 and 2 (`DemoPathError`/`DemoFixtureMissingError` → 1, `DemoDestExistsError`/`DemoDestUnsafeError` → 2; `IngestSchema*Error` → 1, `IngestModelNotFoundError`/`IngestAnchorContractError` → 2), so no single fallback tier fits — each appears only in the frozenset, and a forgotten table entry falls through to tier 1 (AST scan catches it at test time).
**Dual registration (issue #59).** Ten of the twelve abstract bases are ALSO registered in `_EXCEPTION_TO_EXIT_CODE` at a single fallback tier (`ManifestError`/`DiffError`/`CliError` → 1; `DraftError`/`CostError` → 2; `LLMError`/`WarehouseError`/`GradeError`/`SafetyError`/`PruneError` → 3). Two independent roles: the frozenset excludes bases from the AST scan's required-mapping check; the table entry is a forward-compat safety net so a new concrete subclass that forgets a table entry still gets the parent's tier via the MRO walk rather than dropping to the panic-path tier 1. The AST scan still fails loud on the missing per-class entry — fallback is safety net, not substitute. `DemoError` and `IngestError` (issue #104) are the two deliberate exceptions: their concretes span tiers 1 and 2 (`DemoPathError`/`DemoFixtureMissingError` → 1, `DemoDestExistsError`/`DemoDestUnsafeError` → 2; `IngestSchema*Error` → 1, `IngestModelNotFoundError`/`IngestAnchorContractError` → 2), so no single fallback tier fits — each appears only in the frozenset, and a forgotten table entry falls through to tier 1 (AST scan catches it at test time). `CostError` (issue #157) joins the dual-registration tier-2 camp: its three concretes (`CostRollupAuditMissingError`, `CostRollupMalformedRecordError`, `CostRollupUnknownModelError`) all map to tier 2 (input-validation — the audit corpus IS the input), so the abstract base safely shares that fallback.

Companion test `test_scan_7_discovers_every_per_stage_errors_module` asserts the scan walks exactly eleven `errors.py` files (the eleventh is `signalforge.ingest.errors`, issue #104). A new stage's `errors.py` must bump the count AND add its abstract base to the excluded set in lockstep. Sanity test `test_exit_code_mapping_has_at_least_one_entry_per_tier` guards against mass rename/deletion.
Companion test `test_scan_7_discovers_every_per_stage_errors_module` asserts the scan walks exactly twelve `errors.py` files (issue #157 bumped 11 → 12 with `llm/cost/errors.py` — the first sub-stage `errors.py` under `src/signalforge/<stage>/<sub>/`). A new stage or sub-stage's `errors.py` must bump the count AND add its abstract base to the excluded set in lockstep. Sanity test `test_exit_code_mapping_has_at_least_one_entry_per_tier` guards against mass rename/deletion.

**Depth-2 glob extension (issue #157).** The scan's enumerator graduated from `_SIGNALFORGE_DIR.glob("*/errors.py")` (depth-1 only) to **`_SIGNALFORGE_DIR.glob("*/errors.py") ∪ _SIGNALFORGE_DIR.glob("*/*/errors.py")`** when `signalforge.llm.cost.errors` shipped as the first sub-stage `errors.py`. The generalisation is durable: future sub-stage modules can carry their own `errors.py` and scan-7 picks them up automatically without further glob churn. The depth-1 set keeps covering `cli/errors.py` directly (the CLI is itself a depth-1 stage), so no special-case branch is needed.

If a new module legitimately needs to declare an `*Error` subclass without an exit-code mapping (e.g. an abstract intermediate), update `_EXCEPTION_MAPPING_EXCLUDED_BASES` AND document the addition. Don't suppress the test.

Expand Down Expand Up @@ -122,6 +124,8 @@ A behaviour change in the CLI touches **five surfaces**, all updated in the same

When introducing a new flag, write surfaces 1–3 first, then the test against those, then back-fill the DEC.

**The bundled Claude Code skill is a sixth parity surface** (#141, see `skill-parity.md`). `src/signalforge/skills/signalforge/SKILL.md` teaches the CLI surface to operators driving Claude Code; a subcommand / flag / demo-command change updates the skill body in the same commit. Enforcement is a gate, not a prompt: `tests/cli/test_skill_cli_parity.py` parses the live argparse subparser registry plus the locked demo-command list and asserts every token appears in `SKILL.md`. The gate runs inside the canonical `VALIDATE_CMD` (`uv run pytest`), so a `/ralph-run` bead that drifts the CLI from the skill fails validation until the skill is fixed. The gate is mechanical (subcommand / flag / demo-command presence); semantic freshness is the clauditor self-grade plus reviewer attention.

## API alignment with adjacent stages

`add_parser(subparsers) -> None` and `cmd_<name>(args) -> int` for every subcommand; `main(argv: list[str] | None = None) -> int` at the top. No top-level `try/except` in `main()` — typed errors flow up; `cmd_<name>` does the explicit catch and returns the right exit code. **One layer's exception → one CLI handler → one exit code.**
Expand Down
2 changes: 1 addition & 1 deletion .claude/rules/grade-layer.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ The grade layer sits between the prune engine (#6) and the diff renderer (#8). I

- **Scored:** `score: float ∈ [0.0, 1.0]` + `passed: bool`. The judge ran, the response parsed, the anchor contract held.
- **Degraded:** `score: None, passed: False, evidence: "", reasoning: "<failure reason>"`. Three causes route here:
1. `LLMError` retries exhausted → `reasoning="call failed: GradeLLMError"`.
1. `LLMError` retries exhausted → `reasoning="call failed: GradeLLMError"`. **Also covers a provider-specific safety-filter / no-content response** (Gemini's `finish_reason ∈ {SAFETY, RECITATION, OTHER, ...}` with empty parts is the v0.3 example — `GeminiProvider.extract_text_blocks` raises a typed `LLMResponseFormatError` per DEC-005 of #137, which propagates as an `LLMError` and lands here). The contract is provider-neutral: a future vendor with a content-filter surface MUST route through `LLMResponseFormatError` so the conservative degrade fires uniformly — `grade-artifacts` does NOT switch on provider name.
2. `GradeOutputError` (parser failure / anchor-contract failure) → `reasoning="call failed: GradeOutputError"`.
3. `total_budget_seconds` exceeded → `reasoning="grade budget exceeded ..."`.

Expand Down
Loading
Loading