Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
32f9dce
Add super plan for #154: dbt-expectations prune+grade adapter via man…
wjduenow Jul 4, 2026
ac6d43f
Thread follow-up issues #267/#268 into #154 plan; mark phase published
wjduenow Jul 4, 2026
1ffb642
Devolve #154 plan to beads: epic bd_1-scaffolding-ajm + 10 tasks
wjduenow Jul 4, 2026
07e566d
bd_1-scaffolding-ajm.1: manifest GenericTest read-back model + Manife…
wjduenow Jul 5, 2026
42b7864
Merge bead bd_1-scaffolding-ajm.1: US-001 manifest GenericTest + Mani…
wjduenow Jul 5, 2026
f8a8073
bd_1-scaffolding-ajm.2: sqlglot-AST analysis helpers for dbt-compiled…
wjduenow Jul 5, 2026
fc19c4b
bd_1-scaffolding-ajm.6: compiled dbt-expectations fixture + regen scr…
wjduenow Jul 5, 2026
da09659
Merge bead bd_1-scaffolding-ajm.2: US-002 sqlglot-AST analysis helpers
wjduenow Jul 5, 2026
e3f4286
Merge bead bd_1-scaffolding-ajm.6: US-006 dbt-expectations compiled f…
wjduenow Jul 5, 2026
47cf081
bd_1-scaffolding-ajm.3: ingest bridge manifest test node -> Candidate…
wjduenow Jul 5, 2026
791e8be
Merge bead bd_1-scaffolding-ajm.3: US-003 ingest bridge (manifest tes…
wjduenow Jul 5, 2026
c2320ab
bd_1-scaffolding-ajm.5: diff macro-`why` + ingested tests on the tabl…
wjduenow Jul 5, 2026
9aa99ea
Merge bead bd_1-scaffolding-ajm.5: US-005 diff macro-why + read-only …
wjduenow Jul 5, 2026
2d03fcd
bd_1-scaffolding-ajm.4: prune wiring for ingested tests — full-scope …
wjduenow Jul 5, 2026
6a5a278
Merge bead bd_1-scaffolding-ajm.4: US-004 prune full-scope routing + …
wjduenow Jul 5, 2026
5206f5d
bd_1-scaffolding-ajm.7: prune-existing --from-manifest + --grade flag…
wjduenow Jul 5, 2026
75f4732
Merge bead bd_1-scaffolding-ajm.7: US-007 CLI --from-manifest + --gra…
wjduenow Jul 5, 2026
8082202
bd_1-scaffolding-ajm.8: docs + worked example for the dbt-expectation…
wjduenow Jul 5, 2026
96a5a4b
Merge bead bd_1-scaffolding-ajm.8: US-008 docs + worked example
wjduenow Jul 5, 2026
68b5cf7
bd_1-scaffolding-ajm.9: Quality gate — fix bugs from 4-pass code review
wjduenow Jul 6, 2026
0fc5c67
bd_1-scaffolding-ajm.10: Patterns & Memory — rule-file updates for #154
wjduenow Jul 6, 2026
31c606a
#154: Address PR review feedback (CodeRabbit + Copilot)
wjduenow Jul 6, 2026
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
28 changes: 28 additions & 0 deletions .claude/rules/business-rule-tests.md
Original file line number Diff line number Diff line change
Expand Up @@ -190,3 +190,31 @@ Issue #184 closed a model-vs-column-scope mis-steering bug for `row_count_anomal
4. **WARNING shape: positional `%s` + `json.dumps({...})` — and watch the AST grep gate's nested-`JoinedStr` recursion.** The `tests/llm/test_logger_grep_gate.py` AST visitor recursively walks every arg of `_LOGGER.<method>(...)` calls — so a nested f-string INSIDE a `json.dumps({...})` dict literal that is itself a `_LOGGER.warning` arg trips the gate. Use `"column=" + repr(value)` instead of `f"column={value!r}"` to construct identifier-quoted strings inside the dict. (#184 US-003 implementation finding — the spec's verbatim f-string template would have failed validation.) Mirror the cache-anomaly WARNING precedent from `signalforge.llm.client`: payload dict has 5 keys (`test_type`, `from_scope`, `to_scope`, `model_unique_id`, `reason`); fires unconditionally per re-attach; not gated by `--quiet` or any config flag.

5. **Audit-event field bump for cross-stage forensic visibility.** When a parser carve-out mutates the LLM's output, `LLMResponseEvent.parsed_schema_hash` reflects the rebuilt candidate, not the LLM's original emission. To close that visibility gap: add a new field on the audit event (here, `parser_reshaped: tuple[ReshapeRecord, ...] = ()`), bump `audit_schema_version` ONE step (`1 → 2`), keep the field typed `int` (not `Literal`) so prior-version records still round-trip, default to empty so the no-mutation happy path stays byte-equal with the prior version's fixture. Drift detector validates BOTH the prior-version fixture (field absent → default) AND a new fixture exercising the populated path. The threading is end-to-end: parser appends ReshapeRecord to a caller-supplied list → `parse_draft_response` exposes the list via keyword-only kwarg → `draft_from_request` builds the list, threads it into the parse call, then passes `parser_reshaped=tuple(list)` to `_build_response_event(...)` (which is the SINGLE AST-gated `LLMResponseEvent` construction seam — do NOT instantiate from the orchestrator).

## `custom_sql` now has a THIRD source: manifest-ingested dbt tests (issue #154)

`custom_sql` (the 5th test type, #116) originally had two input paths (`meta.signalforge.business_rules`
NL rules + LLM inference). Issue #154 added a THIRD: dbt-compiled test nodes read from
`manifest.json` (`read_manifest_tests`, see `ingest-layer.md`). It reuses the `CandidateTestCustomSQL`
variant WITHOUT a 7th-type extension — the differentiator is a per-candidate
`from_manifest: bool = Field(default=False, exclude=True)` marker:

- **`exclude=True` is load-bearing** — keeps `from_manifest` out of `model_dump_json` so the diff
`candidate_hash`, proposed YAML, and every committed `custom_sql` fixture stay byte-identical
(no DEC-016 schema/audit bump). This is the "carry the category as a typed field set at the source"
discriminator (`grade-layer.md`), NOT a serialized-shape change. **Guard the invariant with an
explicit test** (a mutation removing `exclude=True` passed the whole suite — the drift mirror
carries a default so it can't catch it).
- **Ingested-vs-drafted must be scoped consistently across layers.** The QG caught two cross-bead
drifts: (1) the diff macro-`why` was gated on `type=="custom_sql"` (ALL custom_sql) instead of
`from_manifest`, leaking the drafter rationale into `generate`'s dropped/kept-uncertain `why` and
re-breaking the #50 carve-out; (2) `prune-existing --grade` graded the whole merged candidate
instead of only the `from_manifest` tests, risking flipping the operator's own kept built-ins to
`flagged`. **Rule: anywhere ingested-vs-drafted behaviour diverges (prune routing, diff `why`,
grade input), gate on `from_manifest`, never on `type=="custom_sql"`.**
- **Ingested tests are READ-ONLY** — they appear on the kept/dropped/flagged table, NEVER as
`proposed_test_files` (we didn't author them). `prune-existing` passes `render_diff(emit_test_files=False)`
unconditionally. Macro identity rides the synthesized rationale → diff `why` (DEC-015 of #154).
- **Full-scope only in pass 1** (`scope=sample` deferred, #268); aggregate/bare-`COUNT(*)` macros
skip-recorded (source-routing deferred, #267); dbt-expectations' `validation_errors` wrapper makes
even its `expect_table_row_count` macro row-returning/prunable.
34 changes: 34 additions & 0 deletions .claude/rules/ingest-layer.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,37 @@ The bare-name model resolver was hoisted to `signalforge.cli._helpers._resolve_m
## Reference

`plans/super/104-ingest-external-tests.md` — DEC-001 … DEC-011. `src/signalforge/ingest/` — current implementation. `docs/ingest-ops.md` — operational reference. `tests/ingest/` — test suite (`test_parser.py` variant matrix, `test_anchor.py` collect-all, `test_reader.py` orchestrator + the disabled-prune acceptance check, `test_models.py` with the no-drift-detector note). `tests/fixtures/ingest/schema_codegen_shaped.yml` — dbt-codegen-shaped fixture. See-Also: `manifest-readers.md` (the reader precedent), `prune-engine.md` (what `prune_tests` accepts), `cli-layer.md` (exit-code lockstep, the deferred CLI).

## Manifest-compiled-SQL source: `read_manifest_tests` (issue #154)

A SECOND ingest source alongside `read_schema` (schema.yml) and `read_test_files`
(`tests/*.sql`): `signalforge.ingest.read_manifest_tests(manifest, model, *, project_dir=None)
-> IngestResult` reads dbt-compiled test nodes (`Manifest.tests`, see `manifest-readers.md`
§ GenericTest) and turns each into a prunable candidate. It closes the un-graded half of the
prune gate for dbt-expectations / dbt-utils / in-house generic tests (Architectural Commitment #1).

- **Reuses `CandidateTestCustomSQL`, no 7th variant (DEC-001).** A row-returning + deterministic
test with populated `compiled_code` becomes a model-level `CandidateTestCustomSQL(sql=compiled_code,
column=None, rationale=<synthesized>, from_manifest=True)`. The `from_manifest` marker
(`Field(default=False, exclude=True)`) distinguishes ingested-from-drafted downstream WITHOUT a
schema/audit bump (`exclude=True` keeps `candidate_hash` byte-identical — guard that invariant
with an explicit test; a mutation dropping it passed the whole suite at QG time).
- **Four-gate classification, all reusing `signalforge.ingest._compiled_sql` (sqlglot-AST):**
presence (`compiled_code` non-null) → `is_row_returning` (skip bare-scalar `SELECT COUNT(*)`;
DEC-004) → `is_deterministic_sql` (skip `TABLESAMPLE`/`RAND`/`CURRENT_TIMESTAMP`/…; DEC-012) →
`validate_ingested_sql` (comment-tolerant safety scan; DEC-013). **Regex/substring is unsafe
here — a column named `random_id` false-positives; use AST.** NOTE: dbt-expectations wraps
EVERY macro (incl. `expect_table_row_count_to_be_between`) in a row-returning `validation_errors`
shell, so those ARE prunable; the aggregate-skip fires only on a BARE top-level `SELECT COUNT(*)`.
- **`SkipReason` stays the closed 3-value Literal (DEC-014).** No-`compiled_code` / aggregate /
non-deterministic / unparseable → an existing reason (`custom-or-generic-test` /
`malformed-supported-test`), never a 4th. When EVERY associated node lacks `compiled_code`, emit
ONE summary `SkippedTest` with a "run `dbt compile`" remediation — the AC's "not a silent skip"
surface (soft, never a hard abort).
- **Envelope-safe rationale (DEC-011).** The synthesized rationale (macro name + arg summary) is set
at frozen-model CONSTRUCTION (never mutated) and stripped of `</ARTIFACT>` (+ the `<\s*/ARTIFACT>`
whitespace-split variant) so a hostile macro arg can't fail-close the whole grade run. The macro
identity rides on the rationale → the diff `why` cascade surfaces it (DEC-015).
- **Stage-0 preserved.** No logging, no warehouse/LLM calls, no SQL building, no `bigquery` import.
The `_compiled_sql` helpers are self-contained (they do NOT import the warehouse `_strip_string_literals`
private — a QG fix; a stage-0 reader must not couple to another layer's `_`-internal).
15 changes: 15 additions & 0 deletions .claude/rules/llm-drafter.md
Original file line number Diff line number Diff line change
Expand Up @@ -215,3 +215,18 @@ The drafter's config block is `{ llm: { provider, model, cheap_model, max_output
## Reference

`plans/super/5-llm-draft-pipeline.md` — DEC-001 … DEC-027. `plans/super/135-provider-neutral-llm-seam.md` — DEC-001 … DEC-012 (the provider-neutral seam: `call_llm`, `LLMProvider` ABC + registry, capability flags, `provider` config field). `plans/super/136-openai-grading-provider.md` — DEC-001 … DEC-014 (OpenAI as the second concrete provider: shim + `OpenAIProvider` + four pricing SKUs + `--estimate` strategy refactor + JSON-mode enforcement). `plans/super/137-gemini-grading.md` — DEC-001 … DEC-019 (Gemini as the third concrete provider: shim + `GeminiProvider` + `.messages`-over-`.models.generate_content` adapter + three pricing SKUs + native `models.count_tokens` for `--estimate` + safety-filter typed degrade + namespace-package AST confinement). `plans/super/188-bulk-cache-prefix.md` — DEC-001 … DEC-015 (project-scope shared cached prefix for `--select` batches: `DraftConfig.cache_scope`, `_render_project_summary` + `<PROJECT_MANIFEST>` envelope, dual `_PROMPT_VERSION` + `_prompt_version_for` dispatch, oversize catch-and-retry vs breach fail-closed, two cache-stability goldens). `src/signalforge/llm/` (incl. `providers.py` + `_anthropic_client.py` + `_openai_client.py` + `_gemini_client.py`), `src/signalforge/draft/` — current implementation. `tests/llm/_fake.py::FakeAnthropicClient` + `tests/llm/_fake_openai.py::FakeOpenAIClient` + `tests/llm/_fake_gemini.py::FakeGeminiClient` — `expect_*` API; `tests/llm/_fake_provider.py::FakeNoCacheProvider` + `tests/grade/test_provider_neutrality.py` + `tests/grade/test_provider_neutrality_openai.py` + `tests/grade/test_gemini_neutrality.py` + `tests/draft/test_gemini_neutrality.py` — the no-cache provider-neutrality proofs (synthetic + real OpenAI + real Gemini). `docs/draft-ops.md` / `docs/grade-ops.md` / `docs/cost-estimate-ops.md` — operational references. `tests/fixtures/draft/llm_response_*.json` / `tests/fixtures/estimate/anthropic_byte_identity_golden.txt` — fixture sets exercising happy + each error path + the DEC-013 Anthropic byte-identity floor.

## sqlglot confinement — second/third importer (issue #154)

The `#159` sqlglot-confinement convention ("confined to `signalforge.draft.parser`; future scan
when a second module reaches for sqlglot") graduated in #154. sqlglot now has a SECOND importer —
`signalforge.ingest._compiled_sql` (the `is_deterministic_sql` / `is_row_returning` /
`validate_ingested_sql` helpers for dbt-compiled test SQL; #154 DEC-006) — and a third CONSUMER,
`signalforge.prune.compiler`, which imports those helpers (not sqlglot directly). The rationale:
dbt's `compiled_code` is FOREIGN-rendered SQL, so the `#116` string-substitution / substring-validation
machinery is safe-but-wrong-tool — AST parsing is the only sound approach for a relation SignalForge
did not render itself (regex false-positives on a `random_id` column; a scalar wrapped in
`COUNT(*) AS failures` is `failures=1`-always). No AST confinement-scan was added (the convention
remains documented, not gated); `tests/prune/test_compiler_import_guard.py` still passes (it forbids
`google.cloud`/`snowflake` under `prune/`, not sqlglot). If a FOURTH sqlglot importer lands, promote
the convention to a real confinement scan.
33 changes: 33 additions & 0 deletions .claude/rules/manifest-readers.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,36 @@ When the operator wants types but does NOT run `dbt docs generate` (e.g. a dbt-p
## Reference

`plans/super/2-manifest-loader.md` — DEC-001, DEC-007, DEC-008, DEC-013, DEC-014, DEC-017. `plans/super/37-multi-model-select.md` — DEC-001, DEC-012, DEC-016 (selector grammar additions). `plans/super/159-drafter-column-types.md` — DEC-001, DEC-002, DEC-007, DEC-009, DEC-010 (catalog.json sibling merge). `src/signalforge/manifest/loader.py` — current implementation of all three traps + the `_apply_catalog_overlay` helper. `src/signalforge/manifest/select.py` — issue-#37 selector module (parse_selector / select_models / SelectorAtom).

## Test-node read surface: `GenericTest` + `Manifest.tests` sibling filter (issue #154)

`signalforge.manifest.load` now also surfaces dbt `resource_type == "test"` nodes so the
prune pipeline can grade externally-authored dbt-expectations / dbt-utils / generic tests
from their manifest `compiled_code`. The read surface is a **sibling filter**, mirroring the
`sources` precedent — NOT a repurpose of the model-only `nodes` map.

- **`GenericTest`** — frozen `extra="ignore", populate_by_name=True` read-back model carrying
`unique_id`, `compiled_code: str | None`, `test_metadata` (name/namespace/kwargs),
`column_name`, `depends_on`, `attached_node`, `file_key_name`. Surfaced into
`Manifest.tests: dict[str, GenericTest] = Field(default_factory=dict)` via a
`resource_type == "test"` arm in `_load` parallel to `filtered_sources`. Test nodes already
flowed through the model-only filter and were discarded — the arm keeps them instead.
**`Manifest.nodes` stays model-only** (invariant intact); `tests` defaulting empty breaks no
`Manifest(...)` construction site; the frozen `model_copy` catalog overlay passes `tests`
through untouched. Mandatory `StrictGenericTest(extra="forbid")` drift detector + committed
fixture (`tests/fixtures/manifest/generic_test_nodes.json`, both v9/v10 shapes).

- **Test→model association FEATURE-DETECTS, never version-branches (DEC-009).** No single field
is authoritative across manifest v9–v12 (`attached_node` is dbt 1.6+/manifest v10+, ABSENT in
v9) AND the loader discards the detected version. `signalforge.manifest.associate_test_model`
uses a precedence ladder: `attached_node` → else `file_key_name` + `depends_on.nodes`
disambiguated by `column_name` / `test_metadata.kwargs.model` (bounded `ref('…')` scan).
Ambiguous → `None` (the ingest bridge simply doesn't associate it — matches the source-registry
"return None rather than guess" posture).

- **`compiled_code` availability is the exact `data_type` / catalog.json analogue.** `dbt parse`
does NOT populate it; `dbt compile` / `dbt run` / `dbt docs generate` does. The reader stays
stage-0 silent on absent/null `compiled_code` (like `Column.data_type=None`, no drift detector
needed — an existing-field null). The AC's "not a silent skip" is satisfied DOWNSTREAM (the
ingest bridge skip-records a no-`compiled_code` node with a "run `dbt compile`" remediation).
No `_PROMPT_VERSION` change (manifest is not a prompt template).
27 changes: 27 additions & 0 deletions .claude/rules/prune-engine.md
Original file line number Diff line number Diff line change
Expand Up @@ -225,3 +225,30 @@ New ABC method on `WarehouseAdapter`: `run_stats_query(sql: str) -> tuple[dict[s
## Reference

`plans/super/6-prune-engine.md` — DEC-001 … DEC-028. `plans/super/22-temp-table-sample.md` — v0.2 materialised-sample additions. `plans/super/35-prune-enabled-doc-reframe.md` — operator-disable additions. `plans/super/51-kept-rate-warn-doc.md` — kept-rate WARNING + drop-rate doc. `plans/super/55-normalise-hash-recipe.md` — hash recipe normalisation. `plans/super/121-prune-snowflake-dialect.md` — Snowflake compiler dialect (DEC-001…008). `plans/super/171-row-count-anomaly.md` — `_test_requires_source_table` helper (DEC-009), DEC-010 stricter-bypass behavior change, two-query split + cold-start (DEC-008), `AnomalyTestStats` (DEC-005), `--as-of` reproducibility carve-out (DEC-001), `_PRUNE_AUDIT_SCHEMA_VERSION: 2 → 3` + serializer (DEC-013), `StatsQueryNotSupportedError` ABC graceful degrade. `src/signalforge/prune/` — current implementation. `docs/prune-ops.md` — operational reference. `tests/prune/test_drift_detector.py` — schema-drift gate. `tests/prune/test_compiler_import_guard.py` — `prune/` SDK-import confinement (DEC-008 of #121). `tests/prune/test_compiler_fakesnow.py` — gated `@pytest.mark.snowflake` fakesnow/sqlglot validation. `tests/test_audit_completeness.py` — AST-scan suite. `tests/llm/test_logger_grep_gate.py` — lazy-format logger gate. `tests/fixtures/prune/prune_event_v1.jsonl` — committed audit fixture (v3 as of #171).

## Ingested manifest-compiled tests: full-scope routing + determinism fallback (issue #154)

Manifest-ingested `custom_sql` candidates (`from_manifest=True`, see `ingest-layer.md`
§ `read_manifest_tests`) route differently from drafted `custom_sql`:

- **`scope=full` regardless of `--scope` (DEC-007).** dbt's `compiled_code` renders the model
relation with dbt's own dialect-specific quoting, which the `custom_sql` string-substitution
CANNOT match — under `scope=sample` every ingested test would silently degrade to
`kept-without-evidence` (the fail-closed guard holds — NOT a prod full-scan — but sampling is
inert). So `_test_requires_source_table` returns `True` for `from_manifest` custom_sql under any
sample strategy (joins the metadata-aggregate bypass set → `source_table_ref`, both the
`all_bypass_to_source` short-circuit AND the per-test `per_test_table_ref` arm, in lockstep — the
#170 two-conditional rule). One INFO fires when `--scope=sample` was requested. The compiler's
ingested branch returns the compiled body VERBATIM (dbt's quoted relation already points at the
real table — no substitution). sqlglot AST relation-rewriting for true sampling is deferred (#268).
- **Comment-tolerant validation on the compiled body (DEC-013).** The ingested compile path uses
`validate_ingested_sql` (strips `--`//`* */` before the safety scan) NOT the #116 `validate_test_sql`
— dbt-compiled SQL routinely carries comments the #116 validator rejects wholesale. Plus a
belt-and-braces `is_deterministic_sql → _InvalidIdentifier → kept-without-evidence` fallback at
the compiler (the primary determinism gate is in ingest).
- **`DropReason` stays the 5-value LOCK.** Non-deterministic / unparseable / can't-evaluate ingested
tests route through the existing `kept-without-evidence` per the conservative-bias template — never
a 6th reason. No new `PruneEvent` field / no `_PRUNE_AUDIT_SCHEMA_VERSION` bump.
- **sqlglot extension.** The compiler consumes `signalforge.ingest._compiled_sql` (the 3rd sqlglot
consumer after `draft/parser` and `ingest/_compiled_sql` itself); the `test_compiler_import_guard`
(no `google.cloud`/`snowflake` under `prune/`) is unaffected — sqlglot is dialect-neutral parsing.
Loading
Loading