Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 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.
Comment thread
wjduenow marked this conversation as resolved.
Resolved
- **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
4 changes: 3 additions & 1 deletion .claude/rules/llm-drafter.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,14 @@ Mirrors safety's fail-closed audit at the LLM-output boundary. Three load-bearin

`LLMResponseEvent` carries `sent_sql_hash` (blake2b-8 of `Model.raw_code`), `parsed_schema_hash` (blake2b-8 of `candidate.model_dump_json` with sorted keys), `response_text_hash` (blake2b-8 of raw LLM text), plus `prompt_version`, cache-token economics, model id, and `signalforge_version`.

## `<MODEL_SQL>` prompt-injection envelope (DEC-007)
## `<MODEL_SQL>` prompt-injection envelope (DEC-007), parameterised in #163

`Model.raw_code` is user-authored SQL. A comment like `-- IGNORE PREVIOUS INSTRUCTIONS` could flip the LLM's output without the envelope. `_render_dynamic_block` wraps `raw_code` in `<MODEL_SQL>...</MODEL_SQL>` tags; the system message's anchor contract instructs the LLM to treat anything between as data.

**Envelope-breach guard.** `_render_dynamic_block` raises `PromptEnvelopeBreachError(model_unique_id)` if `raw_code` contains the literal `</MODEL_SQL>` — refuses to render the prompt, never reaches the LLM. Don't downgrade to a warning; the envelope is the only defence between malicious manifest content and the LLM.

**`PromptEnvelopeBreachError` is envelope-parameterised, not subclassed (#163, DEC-005).** When the drafter shipped a second envelope `<BUSINESS_RULE id="N">…</BUSINESS_RULE>` around operator-supplied `meta.signalforge.business_rules` content (see `business-rule-tests.md` § "Two input paths"), the breach guard pattern repeated verbatim — but `PromptEnvelopeBreachError.__init__` was extended with keyword-only `envelope: str = "MODEL_SQL"` + `rule_index: int | None = None` rather than a new error subclass. Default kwargs preserve byte-equality of the existing `</MODEL_SQL>` message; `envelope="BUSINESS_RULE"` + 1-indexed `rule_index` renders the BUSINESS_RULE-shaped message. `default_remediation` text was deliberately generalised to name both envelopes (pinned by `tests/draft/test_errors.py::test_prompt_envelope_breach_default_remediation_covers_both_envelopes`). **Future envelopes follow this shape — extend with a new `envelope=` value, never a new error class** (one class, two raise sites today, N tomorrow; no taxonomy growth; no `_EXCEPTION_TO_EXIT_CODE` churn — already tier 2 via the existing entry). The breach scan is **boring substring match**: literal `in` check on the exact closing tag (no whitespace / case normalisation; opening tag alone is allowed; truncated fragments like `</BUSINESS_RUL` are allowed) — mirror this for every envelope (mirrors safety / grade envelope-breach contract precedents too).

## Cached-block scope (DEC-009)

The cached block contains **only** the model under draft + its direct `refs` and `depends_on` neighbours from `Manifest`. NOT the full project manifest. Hard cap at 8000 input tokens via pre-send `messages.count_tokens` (DEC-024); above 8000 raises `LLMCacheTooLargeError` **before** any `messages.create`. Below the model's minimum cacheable size (1024 Sonnet/Opus, 2048 Haiku), the `cache_control` marker is **dropped** and an INFO line is logged — Anthropic silently no-ops a sub-minimum marker; dropping explicitly avoids paying the count-tokens round-trip twice and silences the dual-zero cache-anomaly WARNING. (Behaviour changed under issue #10 — previously raised `LLMCacheTooSmallError`, removed from the public surface.)
Expand Down
74 changes: 74 additions & 0 deletions docs/draft-ops.md
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,80 @@ Business-rule reading is **best-effort, never fail-loud.** A
inferred-fallback path below covers the gap. Whitespace-only strings
collapse to nothing, so an empty `meta` value emits no section.

### Numbered envelope shape (`<BUSINESS_RULE id="N">…</BUSINESS_RULE>`)

As of #163, each rule renders inside a numbered envelope rather than a
bare bullet:

```text
## BUSINESS RULES

Operator-supplied business rules for this model. Draft one custom_sql
test per rule below, using the rule ID as a reference:

<BUSINESS_RULE id="1">
(model) total_amount must never be negative
</BUSINESS_RULE>
<BUSINESS_RULE id="2">
(column discount_pct) discount_pct stays between 0 and 100 inclusive
</BUSINESS_RULE>
```

IDs start at 1; bodies are indented 2 spaces and carry the existing
`(model) ` / `(column X) ` scope prefix. The envelope gives the LLM
unambiguous reference targets and parallels the existing `<MODEL_SQL>`
Comment thread
wjduenow marked this conversation as resolved.
Resolved
fence around the model's raw SQL.

**Envelope-breach guard.** A rule body containing the literal
`</BUSINESS_RULE>` substring would terminate the fence early and let
downstream content escape the data block. Before rendering, the
drafter scans every rule for that exact substring (boring substring
match — no whitespace / case normalisation, mirrors the `</MODEL_SQL>`
precedent) and raises `PromptEnvelopeBreachError(envelope="BUSINESS_RULE",
rule_index=N)` if found. **The opening tag `<BUSINESS_RULE>` alone
is fine** (only the closing tag breaks the fence), as is any truncated
fragment like `</BUSINESS_RUL`. The error class is the same one used
for `</MODEL_SQL>` — extended in #163 with `envelope=` / `rule_index=`
kwargs rather than subclassed. Future envelopes follow the same shape.

### Cardinality contract (at-least-one-per-rule)

The drafter's anchor-contract validator enforces a hard contract on
the LLM's output: when `meta.signalforge.business_rules` declares N
rules AND `custom_sql` is NOT excluded via `DraftConfig.exclude_tests`,
the response MUST carry **at least N `custom_sql` tests** (counted
across both model-level `tests:` and per-column `columns[*].tests`).
Fewer is rejected loudly via `LLMOutputAnchorContractError` with a
violation message that lists every declared rule verbatim:

```text
Expected ≥2 custom_sql test(s) (one per declared business rule), got 1.
Declared rules: '(model) total_amount must never be negative',
'(column discount_pct) discount_pct stays between 0 and 100 inclusive'.
```

The gate is **at-least, not exact-equality** — the LLM may legitimately
decompose a complex rule into two SELECTs, and the excess is allowed.
The collect-all invariant is preserved: a candidate with both a
hallucinated column AND a cardinality miss surfaces both violations in
one error. The gate is a no-op when no rules are declared (the
inferred-fallback path stays open) and a no-op when `custom_sql` is in
`DraftConfig.exclude_tests` (see below).

### `exclude_tests` short-circuit

When `draft.exclude_tests` in `signalforge.yml` contains `"custom_sql"`,
both surfaces no-op:

- `_render_business_rules_section` returns `""` — the drafter does not
send rules to the LLM (no point asking for tests the operator forbade).
- The parser's cardinality gate is skipped — no violation fires even
when rules are declared.

The per-test `exclude_tests` filter at the parser still catches any
`custom_sql` an LLM defies-the-prompt to emit. The two layers are
orthogonal — together they preserve the operator's choice.

### Inferred fallback

You do **not** have to declare any rules. When no
Expand Down
Loading
Loading