#169: row_count_between 6th test primitive - #176
Merged
Merged
Conversation
Phase 1 discovery + Phase 2 architecture review + Phase 3 refinement (15 DECs) + Phase 4 detailing (12 implementation stories + Quality Gate + Patterns & Memory). Refs #169. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
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 (1)
📝 WalkthroughWalkthroughAdds a model-level CandidateTestRowCountBetween and implements end-to-end support: draft model/prompt, ingest macro recognition and validation, prune compiler SQL and engine routing, diff YAML emission and artifact hashing, rubric text/prompt rotation, docs, fixtures, and comprehensive tests including an engineered e2e. ChangesRow-count-between test primitive end-to-end
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Phase 6 → Phase 7. Plan-doc Beads Manifest filled in. Refs #169. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…del + drift detector Land the 6th first-class CandidateTest variant per #169 DEC-001 / DEC-008 / DEC-013: * New `CandidateTestRowCountBetween` model in `signalforge.draft.models`: - `type: Literal["row_count_between"]` - `column: None = None` (model-level only — matches CandidateTestCustomSQL's posture as the other model-level-capable variant, but hard-coded to None since a row-count is always a model-scoped aggregate) - `minimum: int | None` / `maximum: int | None` with non-negative field validators (DEC-008 — prefix-free naming matching `values` / `to` / `field` precedent; ingest + diff emitter handle the dbt-expectations `min_value` / `max_value` mapping) - `where: str | None` with non-empty-after-strip field validator - `@model_validator(mode="after")` enforces at-least-one-of-(min, max) and `minimum <= maximum` - `frozen=True, extra="ignore"` per the _BASE_CONFIG read-back posture * Extend the `CandidateTest` discriminated union with the new variant (`Field(discriminator="type")` preserved); extend `__all__`. * Drift detector mirror `StrictCandidateTestRowCountBetween(extra="forbid")` in `tests/draft/test_drift_detector.py` + extend the strict union; existing field-set parity test covers the new variant automatically via the discriminated-union walk. * Add one model-level fixture row to `tests/fixtures/draft/candidate_schema_v1.json` exercising all four fields (`minimum`, `maximum`, `where`, `rationale`). No v1→v2 rename — schema-version is forward-compat per DEC-013. * 17 new validator tests in `tests/draft/test_models.py` covering every Pydantic invariant: both bounds none, negative bounds, min>max, empty/ whitespace-only where, frozen-mutation rejection, round-trip byte-stability, discriminated-union resolution, `column != None` rejection, `extra="ignore"` forward-compat. Validation: `uv run pytest tests/draft/test_models.py tests/draft/test_drift_detector.py` green (42 tests). Full suite green (2770 tests, 97.82% coverage). ruff + ruff-format + pyright all green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…etween model + drift detector + fixture row
…trip US-002 of #169. Add "row_count_between" to VALID_TEST_TYPES (six variants now) and refresh the constant + exclude_tests field docstrings to reflect the new sixth member. Tests: - tests/draft/test_config.py — two new round-trip checks: accept exclude_tests=("row_count_between",) and reject the typo ("row_count_betwen",) with the "not a valid test type" error that lists every VALID_TEST_TYPES member (including the new token). - tests/draft/test_exclude_tests.py — bump the pinned VALID_TEST_TYPES set to include "row_count_between" so the canonical-set sentinel test stays green. Traces to: DEC-001 of #169. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… + exclude_tests round-trip
… Snowflake snapshots Implements ``_compile_row_count_between`` in ``signalforge.prune.compiler`` plus the ``_compile_test`` dispatcher arm for the 6th candidate-test variant. Always emits ``SELECT COUNT(*) FROM <quoted-table> [WHERE <where>]`` regardless of ``prune.scope`` (DEC-003) — a sampled COUNT(*) cannot be compared against full-table bounds, so the variant deliberately bypasses ``scope`` / ``sample_size`` / ``sample_bucket`` / ``partition_filter``. Under materialised-sample the orchestrator passes ``table_ref=<temp table>`` so the count lands cheap. Composes the full statement then validates via existing ``validate_test_sql`` (DEC-005); a hostile ``where`` (containing ``;`` / ``--`` / unbalanced parens) routes via ``_InvalidIdentifier`` to ``kept-without-evidence`` (DEC-011). No new ``validate_where_fragment`` helper — reuses the ``custom_sql`` validation surface verbatim. Identifier quoting + case folding read entirely from existing ``Dialect`` fields (``quote_char``, ``identifier_case``, ``quote_qualified_per_component``); no new ``Dialect`` fields, no ``if dialect.name ==`` branches, no new vendor SDK imports under ``signalforge/prune/`` — the import-guard test remains green. Snapshot fixtures cover BigQuery (no-where / with-where / only-min / only-max) and Snowflake (no-where / with-where, per-component double- quoted, UPPER-folded). 14 new tests pin the byte-exact output across dialects, the conservative-bias routing on hostile ``where``, the scope-sample bypass invariant, the partition_filter bypass invariant, and the #116-shaped materialised-sample correctness (compiled SQL references the ``_SESSION._sf_sample_*`` temp table, never the source). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…use type-coherence Extend `_validate_anchor_contract` in `signalforge.draft.parser` with a model-level arm for `CandidateTestRowCountBetween` (the #169 6th variant): - Special-case `row_count_between` AHEAD of the generic `test.column not in model_columns` branch — `column` is always `None` by Pydantic invariant, so the generic check would mishandle it (same precedent as the `custom_sql` arm). - Optional `where` clause validated via a new sibling helper `_check_row_count_between_where`, modelled on `_check_custom_sql_type_coherence` (#159, DEC-003) and reusing the same sqlglot machinery. The helper composes `SELECT 1 FROM __sf_where_placeholder__ WHERE <where>` so sqlglot can parse the freestanding clause, then walks comparison nodes for two violation classes: - Unknown column reference (bare `exp.Column` operand whose name is absent from `model_columns`) — the row_count_between equivalent of the structural `test references nonexistent column` check. - Type incompatibility (bidirectional `COERCES_TO` check on `Column <op> Column` pairs with known types). - Skip-when-uncertain posture preserved per DEC-006: comparisons inside an `exp.Subquery` skip entirely (a `(SELECT ...) > 0` shape does not flag inner column refs); ParseError / annotator failure / unparseable type strings / Cast / Coalesce / function calls / NULL all skip silently. The warehouse adapter remains the safety net for real-SQL breakage via `kept-without-evidence` routing. - `exclude_tests=("row_count_between",)` rejects a drafted row_count_between via the existing model-level dual-defence backstop (prompt-builder filter is the primary defence; this is the parser-side rejection for an LLM that ignores the prompt). - Collect-all preserved: a candidate with both an unknown-column `where` AND a hallucinated CandidateColumn produces BOTH violations in one error. sqlglot imports stay confined to `signalforge.draft.parser` (DEC-008 of #159); no new vendor SDK imports. Traces to: DEC-004, DEC-005, DEC-006, DEC-013 of #169. Tests added (`tests/draft/test_parser.py`): - Valid row_count_between with both bounds + no where → no violations. - `where: "user_id > 100"` referencing a real column → no violations. - `where: "phantom_col > 1"` (unknown column) → violation. - `where: "(SELECT 1 FROM foo) > 0"` (subquery, skip-when-uncertain) → no violations. - `exclude_tests=("row_count_between",)` + drafted row_count_between → violation. - Collect-all preserved with concurrent CandidateColumn violation. Validation: `uv run pytest` 2776 passed, `uv run ruff check` clean, `uv run ruff format --check` clean, `uv run pyright` 0 errors. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…igQuery & Snowflake snapshots
… arm + where-clause sqlglot type-coherence
…tation for row_count_between (US-003) Adds the row_count_between catalogue line to _TEST_CATALOGUE_LINES, illustrating both the no-where (whole-table bound) and with-where (filtered bound) JSON shapes per DEC-012 of #169. Rotates _PROMPT_VERSION from c9e7ee1f6f465933 -> 77e9ee8a6ae7d875; _CACHED_BLOCK_GOLDEN (manifest summary) is unchanged. Extends tests/draft/test_prompts.py with: - system prompt advertises the new type - catalogue illustrates both where forms (two occurrences of the type literal) - default render includes row_count_between - exclude_tests=("row_count_between",) drops the catalogue line + SCOPE entry - two consecutive renders are byte-stable Updates tests/draft/test_exclude_tests.py existing scope-line tests to account for the 5th standard type when excluding everything below custom_sql. Updates tests/llm/test_prompt_cache_stability.py: - _EXPECTED_PROMPT_VERSION to the new hash - rotation-history bullet documenting the #169 rotation Traces to: DEC-012 of #169. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ue + _PROMPT_VERSION rotation
…ween arm Extend signalforge._common.artifact_id.model_test_args_hash with a CandidateTestRowCountBetween arm. Hash domain: type + column (always None for this variant, included for shape-parity) + minimum + maximum + where, serialised as canonical JSON. Identity parity across the three re-exporters (signalforge._common, signalforge.diff, signalforge.grade.engine) holds automatically — the shared seam means the arm propagates everywhere with no per-layer changes; test_cross_stage_parity_is_function_identity continues to pass on `is` equality. Three collision/distinctness tests: - Identical (minimum, maximum, where) -> identical hash - Differing minimum -> different hash - Differing where (None vs "x > 1") -> different hash Traces to: DEC-013 of #169. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…m + cross-stage identity parity
…rage Pin every routing path for `CandidateTestRowCountBetween` through the existing test-type-agnostic `_decide_from_test_result` matrix (DEC-011 of #169). NO engine code change — the matrix dispatches on compiler return shape, warehouse `failure_count`, and raised `WarehouseError`, agnostic to candidate variant. A regression that added a bespoke row_count_between arm would fail loud here. Tests added (8 total, all in tests/prune/test_engine.py): * always-pass on warehouse → `dropped` / `always-passes` * failing rows untrusted → `kept` / `kept` * failing rows trusted model → `dropped` / `failed-on-known-clean-data` * hostile `where` (`;` rejected by compose-then-validate) → compiler `_InvalidIdentifier` → `kept` / `kept-without-evidence` with locked "row_count_between rejected by SQL safety check" `why`. No warehouse call dispatched (fake has zero queued expectations). * `TableNotFoundError` from warehouse → `kept` / `kept-without-evidence` * total-budget exhausted mid-run → remaining tests drain to `kept` / `kept-without-evidence` with locked "Total prune budget" `why` text. Stubs `_now_monotonic_ms` per the established budget-test pattern. * Empty-table carve-out (DEC-010 of #169): a violating bound routes to `kept` / `kept` because that IS what the test exists to catch (real signal). Documented engine posture, NO special-case. * `DropReason` literal still exactly 5 values — closed-set lockdown pin via `typing.get_args`. Cross-checked against the existing drift detector fixture (prune_event_v1.jsonl covers all 5) so the two pins catch regressions independently. Validation: `uv run pytest tests/prune/test_engine.py tests/prune/test_drift_detector.py` → 76 passed. Full suite: 2800 passed. `ruff check` / `ruff format --check` / `pyright` all green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…matrix coverage
…nt_to_be_between (#169 US-005) Promote dbt_expectations.expect_table_row_count_to_be_between from the generic custom-or-generic skip to a first-class supported variant via a new dispatch arm in signalforge.ingest.parser._parse_named_test + helper _parse_row_count_between. Inbound name translation per DEC-008: min_value → minimum, max_value → maximum, where → where. Skip routes (DEC-007), all reusing the existing 'malformed-supported-test' literal (DEC-011 — SkipReason stays the closed 3-value taxonomy): column-scoped usage (variant is model-level only); both bounds missing; non-int or negative bound; bool bound (silent isinstance(True, int) trap); min > max; non-string or whitespace-only where; non-dict body. Sibling dbt-expectations macros (e.g. expect_table_row_count_to_equal) keep falling through to the existing namespaced custom-or-generic-test skip — no behaviour change. The where arg is NOT routed through _extract_args: that helper strips 'where' as a generic dbt test-config key, which is correct for the four standard variants but wrong here (where is a first-class arg of this macro). The new helper reads body directly, honouring the dbt 1.8+ arguments:-nested shape. Files: - src/signalforge/ingest/parser.py: new _ROW_COUNT_BETWEEN_NAME constant + _parse_row_count_between helper + dispatch arm. - tests/ingest/test_parser.py: 17 new tests covering happy paths (inline bounds, arguments-nested, only-min, only-max, with where), every skip route, the sibling-macro fall-through, and the closed 3-value SkipReason invariant. Plus a fixture round-trip test driving every entry in the new schema.yml. - tests/fixtures/ingest/row_count_between_schema.yml: kept + skipped fixture cases exercising every documented path. Traces to DEC-007, DEC-008, DEC-011 of plans/super/169-row-count-between.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ION rotation for vacuous row_count_between bounds (US-009) Extends the existing no-redundant rubric criterion (NOT a 5th criterion — DEC-009 of #169 explicitly forbids that to avoid the +25% LLM cost of an extra per-artifact call) with calibration prose teaching the judge to score vacuous row_count_between bounds low: "...or trivially satisfiable? For tests carrying numeric bounds (e.g. `row_count_between`), is each bound a meaningful guardrail calibrated to the model's expected size, rather than a vacuous floor or ceiling (`minimum=0` with no `maximum`, or a `maximum` so high it cannot fire)?" Rotates the grade-side prompt_version_template a35012b627b8ba6a → 5a70561088930c97 and the _canonical_rubric_hash 280aa6db7fde2b24 → 22a0231690aca6ef. The no-redundant criterion_prompt_hash rotates f89695e3daf7d559 → 60690cb4ef9246ee; the other three criterion hashes are unchanged. The grader's 3-trigger degrade taxonomy (DEC-011) stays locked: LLMError retries / GradeOutputError / total_budget_seconds. Vacuous bounds route through the rubric criterion's low score → existing passed: bool threshold → ship as `flagged` (NOT `kept-uncertain`, which is reserved for prune couldn't-evaluate). NOT a 4th degrade trigger. Adds tests: - no-redundant calibration prose names row_count_between, minimum=0, vacuous, trivially satisfiable - the extension is additive (preserves "semantically identical" + "always-passing" wording) - DEFAULT_RUBRIC stays at exactly four criteria after DEC-009 - vacuous-bound (kept + score=0.2 + passed=False) routes to tier "flagged" via diff.engine._tier_for_kept — NOT "kept-uncertain" - healthy-bound (kept + score=0.9 + passed=True) routes to tier "kept" - 3-trigger degrade taxonomy still has "call failed: " and "grade budget exceeded" reasoning strings in grade.engine Updates rotation-history bullets in test_prompts.py, test_rubric.py, and the DEFAULT_RUBRIC docstring. Traces to: DEC-009, DEC-011, DEC-012 of #169. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…es expect_table_row_count_to_be_between (AC-5)
…on prose + grade _PROMPT_VERSION rotation
US-007 (#169) landed _compile_row_count_between emitting `SELECT COUNT(*) FROM <table> [WHERE <where>]`. The warehouse adapters wrap any compiler output as `SELECT COUNT(*) AS failures FROM (<sql>) AS t` (BigQueryAdapter line 942, SnowflakeAdapter line 852). Under the wrap, the inner returned 1 row (the count value); the outer COUNT(*) over a 1-row result was always 1, so `failures=1` regardless of bounds. The engine's `failures>0 → kept` matrix routed every real row_count_between to `kept` (or `failed-on-known-clean-data` if trusted) without ever checking the bounds. Rewrite _compile_row_count_between to emit the failing-rows CTE shape: SELECT n FROM (SELECT COUNT(*) AS n FROM <table> [WHERE <where>]) AS rc WHERE <bound-violation-predicate> The bound-violation predicate is one of: - only-minimum (maximum is None) → n < <minimum> - only-maximum (minimum is None) → n > <maximum> - both → n < <minimum> OR n > <maximum> When the bound holds, the inner WHERE filters out the count row → outer failures=0 → engine routes `always-passes`. When the bound is violated, the row passes the WHERE → outer failures=1 → engine routes `kept`. The shape now matches the failing-rows-SELECT contract the other 4 built-in tests follow. Changes: - src/signalforge/prune/compiler.py — rewrite _compile_row_count_between; compose-then-validate (DEC-005) preserved. - 6 fixture files regenerated (4 BigQuery + 2 Snowflake). - tests/prune/test_compiler.py — 12 of 14 existing snapshot tests pass automatically against new bytes; 2 tests with shape-invariant assertions (`WHERE not in actual`, `startswith("SELECT COUNT(*) FROM ")`) updated to the new shape. ONE new test (test_compile_row_count_between_adapter_wrapped_failing_rows_contract) pins the failing-rows contract via literal-string assertions on the inner CTE + outer WHERE bound-violation predicate for all three bound variants. - plans/super/169-row-count-between.md — DEC-003 prose updated to describe the CTE+WHERE shape; correction note explains the adapter-wrap interaction bug. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…US-010) Extend ``signalforge.diff._emitter._render_test`` with the ``CandidateTestRowCountBetween`` arm. Emits the dbt-expectations YAML form per DEC-002 of #169: {dbt_expectations.expect_table_row_count_to_be_between: {min_value: <N>, max_value: <M>, where: "..."}} THIS function is the outbound mapping seam — the model carries ``minimum`` / ``maximum`` (DEC-008, prefix-free naming on the model); the dbt-expectations macro uses ``min_value`` / ``max_value``. Maps outbound here. ``None``-valued fields are omitted so the emitted YAML stays minimal. ``proposed_test_files`` stays ``custom_sql``-only (DEC-002). Tier classification is variant-agnostic — a kept ``row_count_between`` flows through the generic ``_tier_for_kept`` classifier as ``tier=kept``, so no engine-level changes are needed. Test pin guards the contract. TDD coverage: * No-where YAML shape (only ``min_value`` / ``max_value``) * With-where YAML shape (verifies the dict carries the ``where`` field) * Only-minimum omits ``max_value``; only-maximum omits ``min_value`` * Hostile ``where`` content (multi-line, embedded quotes, YAML metacharacters) round-trips via ``yaml.safe_dump`` / ``yaml.safe_load`` * Dropped decision filtered out (no model ``tests:`` key) * Variant does NOT appear in ``proposed_test_files`` (custom_sql-only) * Tier-classification pin: kept ``row_count_between`` lands in the kept-table via the generic classifier; proposed_yaml carries the dbt_expectations namespace; proposed_test_files stays empty Traces to DEC-002, DEC-008, DEC-013 of #169. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ng-rows CTE shape for row_count_between
…-expectations YAML shape
…urface parity) Land the 5-surface-parity doc pass for `row_count_between` (the 6th first-class `CandidateTest` variant) plus the 6th-surface SKILL.md update per DEC-015 of #169: - docs/draft-ops.md — new § "Row-count tests (`row_count_between`)" covering the catalogue entry (no-where + with-where), when the drafter proposes it, the `exclude_tests` short-circuit, and the worked example (intuit_airflow `weekly_query_cost` analog from DEC-012). - docs/prune-ops.md — new § "Row-count cost model" covering the failing-rows CTE wrap, sample-mode bypass + materialised-sample substitution, partition-aligned `where`-scan cost guidance, `maximum_bytes_billed` + `total_budget_seconds` safety nets, and the empty-table → `kept` carve-out (DEC-010 — load-bearing: real signal, not a degenerate case). - docs/grade-ops.md — new § "Row-count calibration" naming the `no-redundant` criterion's extended language (DEC-009) and confirming the 3-trigger degrade taxonomy stays locked; vacuous bounds route through `flagged`, not `kept-uncertain` and not a 4th degrade slot. - docs/diff-ops.md — new § "Row-count YAML emission" with the `dbt_expectations.expect_table_row_count_to_be_between` block, `minimum`/`maximum` → `min_value`/`max_value` outbound mapping, null-field omission, no `.sql` fallback (DEC-002), and the operator's responsibility for `dbt-expectations` in `packages.yml`. - docs/ingest-ops.md — new § "Recognition of `expect_table_row_count_to_be_between`" closing AC-5: promotion to the typed variant, inbound mapping table, the closed SkipReason literal preserved at 3 values, and the narrow recognition scope (other `dbt_expectations.*` macros still skip-record). Updated the SkipReason table entry for `custom-or-generic-test` to point at the new section. - CHANGELOG.md — Unreleased Added bullet covering all 5 layers + the ingest promotion + the empty-table semantic + the suppression knob. - src/signalforge/skills/signalforge/SKILL.md — one-paragraph operator-facing description of the 6-variant catalogue and the `exclude_tests` knob. Validation: `uv run pytest tests/cli/test_skill_cli_parity.py` green (no new CLI subcommands/flags/demo-commands introduced); full suite 2842 passed; ruff check + ruff format --check + pyright clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…CHANGELOG + SKILL.md (5-surface + 6th-surface parity)
…t_between (#169 US-012) Adds the gated full-pipeline smoke that pins the row_count_between variant's kept/dropped contract under a real BigQuery warehouse. * tests/cli/test_e2e_row_count_between.py — the @pytest.mark.e2e smoke. Drives signalforge prune-existing against the committed Austin-bikeshare fixture with a hand-crafted external schema.yml carrying two engineered row_count_between tests: - engineered-failure-kept (load-bearing AC-1): min_value: 1, where: "1 = 0". The compiler emits SELECT n FROM (SELECT COUNT(*) AS n FROM <table> WHERE 1 = 0) AS rc WHERE n < 1. Inner COUNT is mathematically 0 on any table; outer WHERE returns one row; adapter wrap yields failures=1; engine routes to decision='kept', reason='kept'. Independent of warehouse state or sample bytes — proves the variant produces signal end-to-end. - engineered-always-pass-and-drop (AC-2): min_value: 0 (no upper bound). Predicate n < 0 vacuously false (COUNT(*) >= 0 always); failures=0; routes to always-passes -> dropped. Mathematically deterministic without LLM cooperation. Gated by SF_RUN_BQ=1 + GOOGLE_CLOUD_PROJECT only (deliberately narrower than the three-env-var bigquery smoke — prune-existing makes no LLM call). The default pytest suite deselects via the e2e marker; missing env vars route through pytest.skip cleanly. * src/signalforge/ingest/anchor.py — adds the row_count_between arm to the model-level test loop. Mirrors the drafter-side exemption in signalforge.draft.parser._validate_anchor_contract: the Pydantic model fixes column=None, so None not in model_columns would otherwise fire a spurious 'references nonexistent column None' violation that blocks the variant through ingest. Without this, the e2e test (and any prune-existing run with a hand-authored expect_table_row_count_to_be_between) is dead on arrival. * src/signalforge/ingest/reader.py — extends _test_dedupe_key with a row_count_between arm. The previous (type, column) fallback collapsed two row_count_between entries on the same model (column=None always) to one, dropping the second silently. The new key includes (minimum, maximum, where) so distinct bound configs survive as separate candidates. Mirrors the existing accepted_values / relationships arm-extension pattern (DEC-008). * tests/ingest/test_anchor.py — pins the model-level row_count_between with column=None passes the anchor validator cleanly. * tests/ingest/test_reader.py — pins (a) distinct row_count_between configs survive dedupe; (b) byte-identical configs across tests:/ data_tests: still collapse. Traces to: AC-1, AC-2, AC-7 of #169. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…m live e2e (bigquery + e2e markers) + 2 ingest bugfixes (anchor exemption, dedupe key arm)
QG Pass 1 (Correctness) HIGH finding: row_count_between returned wrong verdict at default config (scope=sample + sample_strategy=materialised). The engine substituted compile_table_ref = materialised_ref (a temp table with sample_size rows, e.g. 100K), so the compiled COUNT(*) returned the sample size — not the model's true row count. Bounds checked against sample size were semantically meaningless. Fix: in signalforge.prune.engine, add a per-test arm that overrides compile_table_ref to source_table_ref when the test is a CandidateTestRowCountBetween. The COUNT(*) against the source is a single aggregate scan — cheap even on petabyte tables — so there is no cost argument for routing through the temp table. Other variants (not_null / unique / accepted_values / relationships / custom_sql) continue to read row-level data from the materialised sample correctly. The bypass is row_count_between-specific. Plan-doc DEC-003 updated to reflect the corrected behaviour. Pinned by test_prune_tests_row_count_between_under_materialised_references_source_not_temp_table. QG Pass 4 (Docs+UX) HIGH findings F5-F9: 5 doc-vs-code drifts in the #169 surface: - docs/draft-ops.md: VALID_TEST_TYPES "five" -> "six" + row_count_between - docs/prune-ops.md:50: CandidateTest variants "five" -> "six" - docs/prune-ops.md:674: dbt-expectations claim — promoted one macro in #169 - docs/ingest-ops.md: overview omitted expect_table_row_count_to_be_between - src/signalforge/draft/models.py: docstring described pre-US-007a shape Triangulated stale-base findings (Conventions + Docs both flagged it) — will be cleared by a follow-up git merge dev in the same QG turn. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…between # Conflicts: # CHANGELOG.md
…row_count_between Update .claude/rules/business-rule-tests.md to reflect the 2-instance precedent for variant extension (custom_sql #116 + row_count_between #169). Key changes: - Title + intro: "Custom business-rule tests" → "Custom business-rule tests + variant-extension pattern"; describes both variants. - "The variants" § lists both Pydantic models + their validators. - NEW § "The 6 production dispatch sites (was 5; #169 added the 6th ingest anchor exemption)" enumerates the audit list with the new signalforge.ingest.anchor.validate_anchor_contract entry from US-012's inline ingest bugfix; also calls out which sites stay automatic (string-discriminator, not isinstance). - Materialised-sample substitution § now covers TWO directions: - Direction 1 (#116): row-level test bypass-to-temp via FROM-clause rewrite. - Direction 2 (#169): metadata/aggregate test bypass-the-substitution via per_test_table_ref source override at the engine. Pinned by test_prune_tests_row_count_between_under_materialised_references _source_not_temp_table. Decision rule: ask first what the test queries (row-level vs metadata/aggregate); the compiler snapshot can't catch this. - NEW § "Lockstep _PROMPT_VERSION rotation when extending the catalogue (#169 DEC-012)" — two independent _PROMPT_VERSION constants (drafter + grade); both rotate when a variant adds both a catalogue entry AND a rubric criterion refinement. Cross-link to ralph-serialize-shared-registry-beads. - NEW § "where-clause sqlglot type-coherence — reuse, don't fork (#169 DEC-004, DEC-006)" — #169 reuses #159's _check_custom_sql_type_coherence verbatim on the composed SELECT. - Ingest § now describes both recognition paths (custom_sql singular files vs dbt_expectations.expect_table_row_count_to_be_between schema.yml) and the inbound/outbound naming mapping seam (minimum internally, min_value externally). - On-disk artifact § distinguishes per-variant emission form: custom_sql → proposed_test_files; row_count_between → YAML block. Decision rule: default to YAML, promote to proposed_test_files only on a real standalone-artefact need (the 6th fail-closed writer is expensive ceremony). - Testing § adds the engineered-determinism trick for row_count_between (where: false + minimum: 1) and the engine-routing pin pattern (assert source vs _SESSION._sf_sample_*). - Reference § cross-links plans/super/169-row-count-between.md. Also (outside the repo, under ~/.claude/projects/...): new memory signalforge-row-count-between-pattern.md + index entry in MEMORY.md capturing the 6 dispatch sites, the metadata-vs-row-level decision rule for materialised-sample bypass, and the lockstep _PROMPT_VERSION rotation across drafter + grade. No code changes. uv run pytest green (2846 passed, 75 deselected). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…usiness-rule-tests rule + cross-cutting memory
wjduenow
changed the title
wjduenow
marked this pull request as ready for review
June 1, 2026 02:57
There was a problem hiding this comment.
Pull request overview
This PR adds row_count_between as a 6th first-class CandidateTest variant and wires it end-to-end through drafting, ingest (prune-existing), prune compilation/execution, grading rubric guidance, and diff YAML emission (dbt-expectations macro form). It also updates fixtures, drift/prompt stability pins, documentation, and introduces a gated BigQuery e2e test to validate deterministic kept/dropped behavior.
Changes:
- Introduces
CandidateTestRowCountBetween(model-level only) and extends all relevant dispatch/ID/dedupe/emission paths. - Implements prune compilation + engine routing adjustments (including per-test table-ref override) and adds broad unit coverage + SQL snapshot fixtures.
- Updates grading rubric/prompt pins and expands docs/plan/changelog plus a gated BigQuery end-to-end test.
Reviewed changes
Copilot reviewed 47 out of 47 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/prune/test_engine.py | Adds prune routing-matrix tests for row_count_between and locks DropReason literal size. |
| tests/prune/test_compiler.py | Adds compiler tests + dialect snapshots + safety validation behavior for row_count_between. |
| tests/llm/test_prompt_cache_stability.py | Rotates pinned drafter prompt version due to catalogue change. |
| tests/ingest/test_reader.py | Tests dedupe-key changes so distinct row_count_between configs don’t collapse. |
| tests/ingest/test_parser.py | Adds ingest parsing tests for dbt_expectations.expect_table_row_count_to_be_between. |
| tests/ingest/test_anchor.py | Ensures ingest anchor-contract doesn’t treat column=None as missing column for model-level-only variant. |
| tests/grade/test_rubric.py | Updates rubric text pins and adds tiering/calibration-related tests (some currently inconsistent with prune semantics). |
| tests/grade/test_prompts.py | Rotates grade prompt version/hash pins after rubric text change. |
| tests/fixtures/prune/compiled_sql/snowflake/row_count_between.sql | Snowflake compiled SQL snapshot for row_count_between. |
| tests/fixtures/prune/compiled_sql/snowflake/row_count_between_where.sql | Snowflake compiled SQL snapshot with where. |
| tests/fixtures/prune/compiled_sql/row_count_between.sql | BigQuery compiled SQL snapshot for row_count_between. |
| tests/fixtures/prune/compiled_sql/row_count_between_where.sql | BigQuery compiled SQL snapshot with where. |
| tests/fixtures/prune/compiled_sql/row_count_between_only_min.sql | BigQuery snapshot for min-only bound. |
| tests/fixtures/prune/compiled_sql/row_count_between_only_max.sql | BigQuery snapshot for max-only bound. |
| tests/fixtures/ingest/row_count_between_schema.yml | Ingest fixture exercising kept + skipped cases. |
| tests/fixtures/draft/candidate_schema_v1.json | Adds row_count_between row to draft candidate-schema fixture. |
| tests/draft/test_prompts.py | Asserts system prompt advertises/excludes row_count_between properly. |
| tests/draft/test_parser.py | Adds drafter anchor-contract tests for model-level variant + sqlglot checks on where. |
| tests/draft/test_models.py | Adds Pydantic validation tests for CandidateTestRowCountBetween. |
| tests/draft/test_exclude_tests.py | Updates VALID_TEST_TYPES expectations + SCOPE-line rendering assertions. |
| tests/draft/test_drift_detector.py | Adds strict mirror model for drift detection of new variant. |
| tests/draft/test_config.py | Ensures exclude_tests validator accepts row_count_between and rejects typos. |
| tests/diff/test_engine.py | Ensures kept row_count_between renders into kept tier and emits YAML macro block. |
| tests/diff/test_emitter.py | Adds YAML emission tests for row_count_between (min/max/where, yaml-safety, filtering). |
| tests/diff/test_artifact_id.py | Ensures artifact-id args hash includes min/max/where for uniqueness. |
| tests/cli/test_e2e_row_count_between.py | New gated BigQuery e2e for deterministic kept/dropped via engineered predicates. |
| src/signalforge/skills/signalforge/SKILL.md | Documents the new 6th variant and how to suppress it via exclude_tests. |
| src/signalforge/prune/engine.py | Adds per-test table-ref override to ensure row_count_between counts source table, not materialised sample. |
| src/signalforge/prune/compiler.py | Implements _compile_row_count_between and dispatcher arm; adds safety validation on composed SQL. |
| src/signalforge/ingest/reader.py | Extends dedupe key to include (minimum, maximum, where) for row_count_between. |
| src/signalforge/ingest/parser.py | Recognizes dbt_expectations.expect_table_row_count_to_be_between and maps args to the typed variant. |
| src/signalforge/ingest/anchor.py | Exempts row_count_between from model-level column in model_columns check. |
| src/signalforge/grade/rubric.py | Extends no-redundant criterion text with numeric-bound calibration prose. |
| src/signalforge/draft/prompts.py | Adds row_count_between catalogue examples (with/without where) and keeps exclude filtering stable. |
| src/signalforge/draft/parser.py | Adds sqlglot-based validation for row_count_between.where and anchor-contract integration. |
| src/signalforge/draft/models.py | Adds CandidateTestRowCountBetween model and extends discriminated union + exports. |
| src/signalforge/draft/config.py | Adds row_count_between to VALID_TEST_TYPES and updates docs/comments. |
| src/signalforge/diff/_emitter.py | Renders row_count_between as dbt-expectations YAML block with min/max/where mapping. |
| src/signalforge/_common/artifact_id.py | Adds args-hash payload for row_count_between. |
| plans/super/169-row-count-between.md | Adds/updates the super-plan documenting DECs and story breakdown. |
| docs/prune-ops.md | Documents row-count cost model and behavior (currently inconsistent with engine override). |
| docs/ingest-ops.md | Documents ingest promotion of expect_table_row_count_to_be_between. |
| docs/grade-ops.md | Documents calibration intent and rubric change (currently mixes prune-drop vs grade-routing claims). |
| docs/draft-ops.md | Documents variant semantics and drafting guidance (currently claims vacuous bounds ship as flagged). |
| docs/diff-ops.md | Documents YAML emission shape and lack of .sql fallback. |
| CHANGELOG.md | Adds unreleased entry for row_count_between end-to-end support. |
| .claude/rules/business-rule-tests.md | Updates internal rule doc to generalize the “variant extension” pattern to 6 variants. |
💡 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: 3
🤖 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 `@plans/super/169-row-count-between.md`:
- Around line 582-597: The fenced code blocks in the markdown list of user
stories (the triple-backtick blocks showing the US-001…US-014 graph) are missing
language tags and trigger markdownlint MD040; update each untyped fence (the two
blocks shown around the US-001…US-014 diagram and the one at lines 638-648) by
adding a language identifier such as text (e.g., ```text) so the fenced blocks
are annotated and the linter stops flagging them.
- Around line 387-405: The test description and assertions contradict DEC-003's
routing contract for row_count_between: DEC-003 requires compiling against the
source table regardless of prune.scope, but the TDD/US-007 text and the
materialised+sample assertion expect a temp/sample table. Update the TDD and
related assertions so they expect source-table routing (i.e., compiled SQL
should reference the source-qualified table name produced by the Dialect quoting
logic, not _SESSION._sf_sample_*), and ensure any materialised+sample wording no
longer asserts temp-table usage; keep implementation points for
_compile_row_count_between, prune.scope handling, _compile_test dispatcher, and
_InvalidIdentifier behavior unchanged per DEC-003.
In `@src/signalforge/prune/engine.py`:
- Around line 1082-1098: The sample pre-work should be skipped when the run
scope is "sample" and every CandidateTest is a CandidateTestRowCountBetween;
update the engine logic that currently always performs sample setup (materialise
sample table and get_row_count) to first check if all tests are instances of
CandidateTestRowCountBetween and, if so, bypass materialise_sample and
get_row_count and use per_test_table_ref = source_table_ref for all tests;
ensure the same guard is applied where sample setup occurs (the block around
compile_table_ref/materialised substitution and the later lines 1103-1106) so
tests are not routed to kept-without-evidence due to failed sample pre-work.
🪄 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: 4b9e307e-cc2c-41f9-aeaa-c356fd2e36fb
📒 Files selected for processing (47)
.claude/rules/business-rule-tests.mdCHANGELOG.mddocs/diff-ops.mddocs/draft-ops.mddocs/grade-ops.mddocs/ingest-ops.mddocs/prune-ops.mdplans/super/169-row-count-between.mdsrc/signalforge/_common/artifact_id.pysrc/signalforge/diff/_emitter.pysrc/signalforge/draft/config.pysrc/signalforge/draft/models.pysrc/signalforge/draft/parser.pysrc/signalforge/draft/prompts.pysrc/signalforge/grade/rubric.pysrc/signalforge/ingest/anchor.pysrc/signalforge/ingest/parser.pysrc/signalforge/ingest/reader.pysrc/signalforge/prune/compiler.pysrc/signalforge/prune/engine.pysrc/signalforge/skills/signalforge/SKILL.mdtests/cli/test_e2e_row_count_between.pytests/diff/test_artifact_id.pytests/diff/test_emitter.pytests/diff/test_engine.pytests/draft/test_config.pytests/draft/test_drift_detector.pytests/draft/test_exclude_tests.pytests/draft/test_models.pytests/draft/test_parser.pytests/draft/test_prompts.pytests/fixtures/draft/candidate_schema_v1.jsontests/fixtures/ingest/row_count_between_schema.ymltests/fixtures/prune/compiled_sql/row_count_between.sqltests/fixtures/prune/compiled_sql/row_count_between_only_max.sqltests/fixtures/prune/compiled_sql/row_count_between_only_min.sqltests/fixtures/prune/compiled_sql/row_count_between_where.sqltests/fixtures/prune/compiled_sql/snowflake/row_count_between.sqltests/fixtures/prune/compiled_sql/snowflake/row_count_between_where.sqltests/grade/test_prompts.pytests/grade/test_rubric.pytests/ingest/test_anchor.pytests/ingest/test_parser.pytests/ingest/test_reader.pytests/llm/test_prompt_cache_stability.pytests/prune/test_compiler.pytests/prune/test_engine.py
14 of 14 review threads fixed (0 false positives). Categories:
(1) Real new bug — CodeRabbit Thread w2h: when every candidate is
row_count_between, the engine still ran materialise_sample /
_resolve_sample_bucket pre-work before the per-test override fired.
If those adapter calls raised, every test routed to
kept-without-evidence even though they could have run directly
against source. Added all_bypass_to_source short-circuit in
prune.engine.prune_tests that skips the materialised/oneshot setup
entirely when every candidate is row_count_between (the per-test
override would route them all to source anyway). Updated the existing
test_prune_tests_row_count_between_under_materialised_references_source_not_temp_table
to pin the no-pre-work invariant directly.
(2) Real cost-model correction — Copilot Thread u_g: docs claimed
BigQuery COUNT(*) is metadata-cheap; it actually bills as a scan.
Reworded prune-ops.md cost guidance.
(3) Post-QG drift cleanup — Copilot Threads u_S / u_a / u_i / u_k /
u_n / u_q / u_x / u_- / u_0 / u_4 and CodeRabbit Thread w2f. The QG
correction (engine bypasses materialised substitution for
row_count_between) wasn't followed through into:
- compiler.py docstring (still said materialised → temp table)
- prune-ops.md sample-mode section (same)
- test_compiler.py docstring on the materialised-temp-table test
(test still useful as compiler-contract pin, but engine never
actually passes a temp ref for row_count_between)
- plan-doc US-007 TDD bullet (same)
- draft-ops.md / grade-ops.md vacuous-bound routing prose (claimed
minimum=0 maximum=None ships as flagged after grading; actually
dropped by prune as always-passes because failing-rows CTE's
`WHERE n < 0` never matches)
- test_rubric.py docstrings + two test fixtures (constructed
PruneDecision(reason="kept", failures=0) which is unreachable —
the real prune matrix routes failures=0 to always-passes/dropped;
flipped fixtures to failures=1 to match the only reachable kept
state)
(4) Lint nit — CodeRabbit Thread w2g: MD040 markdownlint on plan-doc
fenced code blocks. Added `text` language tag to the two untyped
opening fences (story dep graph and devolve output).
Validation: ruff / ruff format / pyright (0/0/0) clean; full suite
2846 passed, 75 deselected.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Owner
Author
PR Review SummaryFixed (14 items) — commit 103a5feReal new bug (1):
Real cost-model correction (1):
Post-QG drift cleanup (11) — the QG correction (engine routes
Lint nit (1):
False Positives (0)All 14 findings were real. Two reviewer-flagged categories — the unreachable-state test fixtures and the materialised-bypass doc drift — were post-QG follow-through that the QG cycle itself didn't catch (QG fixed the engine but missed the cascading documentation/test implications). Triangulated: the materialised-bypass drift was independently flagged by 3 reviewers across compiler / docs / plan-doc. Validation
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 295-297: The phrase "at-least-one-bound" in the calibration prose
is a typo; replace that hyphenated token with plain prose "at least one bound"
in the docs text (the sentence containing "picked the lowest valid non-vacuous
number to satisfy at-least-one-bound.") so the user-facing documentation reads
naturally and consistently.
🪄 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: 64bd1144-b357-4aab-9866-8df882a7e2e6
📒 Files selected for processing (9)
docs/draft-ops.mddocs/grade-ops.mddocs/prune-ops.mdplans/super/169-row-count-between.mdsrc/signalforge/prune/compiler.pysrc/signalforge/prune/engine.pytests/grade/test_rubric.pytests/prune/test_compiler.pytests/prune/test_engine.py
✅ Files skipped from review due to trivial changes (1)
- plans/super/169-row-count-between.md
🚧 Files skipped from review as they are similar to previous changes (6)
- docs/draft-ops.md
- src/signalforge/prune/compiler.py
- docs/prune-ops.md
- tests/grade/test_rubric.py
- tests/prune/test_engine.py
- tests/prune/test_compiler.py
CodeRabbit Thread ANeH on docs/grade-ops.md:297 — minor wording: 'at-least-one-bound' reads as an accidental typo in user-facing prose. Replaced with 'at least one bound'. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Owner
Author
|
Fixed in 8bf010b — |
wjduenow
added a commit
that referenced
this pull request
Jun 1, 2026
…+ behavioural routing pin Extends the existing row_count_between source-vs-temp conditional in prune/engine.py to include CandidateTestUniqueCombination — composite uniqueness on a sample is semantically approximate (false-negative risk: a duplicate pair may straddle the sampled and unsampled rows), so always-route-to-source mirrors #169 US-007a. Bounded by maximum_bytes_billed. Both sites move in lockstep: * `all_bypass_to_source` short-circuit (CodeRabbit #176 fix) — when EVERY candidate is row_count_between OR unique_combination, skip materialise_sample AND _resolve_sample_bucket pre-work; otherwise a materialisation failure would spuriously route every test to kept-without-evidence. * Per-test `per_test_table_ref` override — when scope="sample" and candidates are mixed, the row_count_between / unique_combination ones still route to source while other variants consume the substituted compile_table_ref. Behavioural routing pin at tests/prune/test_engine.py — mirrors the #169 row_count_between precedent (test_prune_tests_row_count_between_under_materialised_references_source_not_temp_table): * Parametrised across sample_strategy="materialised" AND "oneshot" — the load-bearing pin (snapshot equality from US-005a certifies SQL shape but NOT engine routing per .claude/rules/business-rule-tests.md § "Pin the engine-routing test, not just the compiler snapshot"). * Asserts compiled_sql references the source qualified name AND never references `_SESSION._sf_sample_*`. * Companion scope="full" test as a no-regression belt-and-braces. Traces to #170 DEC-006 (Option (iii): engine override to source via per_test_table_ref). Done when the engine routes unique_combination to source under sample mode, pinned by behavioural test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
wjduenow
added a commit
that referenced
this pull request
Jun 1, 2026
* Add super-plan for #170 — unique_combination 7th test primitive Phase 1 (discovery) — 3 parallel subagents mapped the 6 dispatch sites from the #169 precedent, surfaced the genuinely-new deltas (sample-mode semantics, columns canonical form, grade-side cache-stability gap). Phase 2 (architecture) — 5 parallel reviews (security, performance, data model + API, testing strategy, observability + ops). Cross-review consensus auto-decided sample-mode routing (engine source-override per #169 US-007a), rubric refinement (extend no-redundant, no 5th criterion), ingest strictness, mechanic exhaustiveness gate, and e2e-fixture seeding strategy. Phase 3 (refinement) — 17 DECs across all three earlier phases including three contested decisions resolved by the user: SORT the columns tuple in args_hash, establish the grade-side _PROMPT_VERSION surface, establish __repr__ redaction retroactively across CandidateTestRowCountBetween and CandidateTestCustomSQL. Phase 4 (detailing) — 16 implementation stories + Quality Gate + Patterns & Memory = 18 beads. US-005 split into compiler+snapshots (US-005a) and engine override+behavioural pin (US-005b) per user feedback. Plan doc is the tracking surface; beads land in Phase 7 after plan approval. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Update #170 plan with Phase 7 beads manifest Devolved to 17 beads (1 epic + 14 implementation + Quality Gate + Patterns & Memory). User-approved on 2026-06-01. Ready-to-start beads: US-001 (variant model + plumbing) and US-011 (engineered fixture + manifest seed) — the two independent leaves of the dependency graph. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-0tq.1: US-001 — CandidateTestUniqueCombination variant model + plumbing Add the 7th CandidateTest Pydantic variant + every supporting registration. Foundation for every downstream story in #170. Traces to #170 DEC-001 (variant name), DEC-002 (where shipped v1), DEC-014 (per-column identifier check at anchor-contract arm — NOT Pydantic), DEC-016 (len>=2 cardinality + no-duplicates invariants). Variant shape: type: Literal['unique_combination'] = 'unique_combination' column: None = None (model-level only — third variant after custom_sql and row_count_between to share this constraint) columns: tuple[str, ...] (len>=2, no duplicates — both invariants enforced at Pydantic; raw identifier strings carried through, shape check is US-004) where: str | None = None (non-empty after strip when set, matches the CandidateTestRowCountBetween precedent) rationale: str | None = None Registrations: - Added to CandidateTest discriminated union + __all__ export (src/signalforge/draft/models.py) - VALID_TEST_TYPES frozenset gains 'unique_combination' (src/signalforge/draft/config.py) - StrictCandidateTestUniqueCombination drift mirror + added to _StrictCandidateTest union (tests/draft/test_drift_detector.py) - New row in tests/fixtures/draft/candidate_schema_v1.json (after the row_count_between row, before the closing bracket) - test_valid_test_types_constant_matches_known_set updated to include the new variant (the load-bearing fail-loud gate) Deferred to downstream beads per the plan: - US-002: __repr__ redaction (security review gap) - US-003: drafter prompt catalogue + _PROMPT_VERSION rotation - US-004: anchor-contract arm (identifier shape + per-column check) - US-005a/b: prune compiler arm + sample-mode routing - US-006: artifact_id (SORTED) + diff emitter arm - US-007: ingest parser arm + anchor exemption - US-008: grade rubric no-redundant extension Validation: ruff check + ruff format + pyright + pytest all green (2860 passed, 75 deselected, coverage 97.53%). * bd_1-scaffolding-0tq.12: US-011 — Engineered fixture stg_bikeshare_station_pairs.sql + manifest seed Add a new fixture model with a natural multi-column GROUP BY pattern on (start_station_id, end_station_id, subscriber_type) so the drafter's prompt example reliably steers claude-sonnet-4-6 toward proposing the structured `unique_combination` test (#170 AC-1) instead of freeform `custom_sql GROUP BY HAVING COUNT(*) > 1`. Source-as-model alias trick per .claude/rules/testing-signal.md § 'WHERE the always-pass column must live depends on whether the model is materialised' — the model's `alias` is overridden to `bikeshare_trips` so its relation_name resolves directly to the public source table, no `dbt run` materialisation needed. Real source columns only; no engineered literal/COALESCE columns. Hand-crafted manifest seed per testing-signal.md § 'Hand-crafted manifest seed when workers can't run live tooling': Ralph workers in worktrees can't reach live BigQuery. The committed manifest.json + catalog.json entries mirror the dbt-bigquery 1.8 shape used for `stg_bikeshare_trips`. A future maintainer with live credentials can re-run regenerate.sh to refresh both models in one shot. Loads-only tests in tests/manifest/test_austin_fixture_loads.py verify the seed survives Pydantic parsing through signalforge.manifest.load without env vars / live calls. The existing iter_models test graduated from 'exactly one model' to 'staging models include both'. Demo parity: src/signalforge/_demo/ is mirrored with the same SQL + manifest + catalog updates so tests/test_demo_fixture_parity.py stays green (DEC-015 of #47 — the regenerate.sh's rsync step naturally covers the new file on a real regen). Traces to plans/super/170-unique-combination.md DEC-005, DEC-010, US-011. * bd_1-scaffolding-0tq.9: US-008 — Grade rubric no-redundant extension for grain-meaningfulness Extend the no-redundant criterion (rubric.py:189-198) with sibling calibration prose for unique_combination, naming the vacuously-unique tuple shape (primary_key, anything) — analogous to the row_count_between vacuous-bound extension from #169 DEC-009. Stays at 4 criteria per DEC-007 (no 5th criterion); same routing as the prior extension (low score → existing passed: bool threshold → flagged tier). Rotated three pinned hashes in lockstep (only the no-redundant criterion text changed; clarity/consistency/rationale hashes unchanged): - _canonical_rubric_hash: 22a0231690aca6ef → 30a9fda975b6d45c - prompt_version_template: 5a70561088930c97 → 4dae4421972e9c2d - criterion_prompt_hash[no-redundant]: 60690cb4ef9246ee → 7b96cfdfe63bc8bc Rotation-history comments updated in rubric.py + both pin sites with the #170 DEC-007 rationale. Grader's 3-trigger degrade taxonomy stays locked — a vacuous composite key is a low score, not a 4th degrade trigger. * bd_1-scaffolding-0tq.4: US-004 — Draft parser anchor-contract arm + collect-all matrix Extend _validate_anchor_contract with a unique_combination arm (model-level only): per-column membership check on each entry of test.columns + sqlglot-driven column-existence + type-coherence validation on the optional where clause. Collect-all preserved: every violation surfaces in one LLMOutputAnchorContractError, never short-circuits (DEC-022 of #5; DEC-014/015/016 of #170). Generalised _check_row_count_between_where -> _check_where_clause with a test_type prefix parameter so the same sqlglot machinery serves both where-bearing variants (DEC-005 of #169 'reuse, don't fork'). Existing row_count_between violation messages preserved byte-equal via the test_type='row_count_between' call site. Tests (7 new, all under -k unique_combination): - valid pair (no where) - valid 3-column tuple + where on a coercible-type column - hallucinated column in the columns tuple - hallucinated column in the where clause - type-incoherent where (INT64 vs STRING comparison) - exclude_tests=('unique_combination',) backstop - collect-all multi-violation (CandidateColumn + tuple + where) Validation: ruff/ruff-format/pyright/pytest all green; 2867 passed, coverage 97.68%; the 6 pre-existing row_count_between parser tests still pass byte-equal against the renamed helper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-0tq.7: US-006 — _common.artifact_id arm (SORTED columns) + diff emitter arm Add CandidateTestUniqueCombination arms in two related sites per #170 DEC-011 and DEC-002: - src/signalforge/_common/artifact_id.py: new isinstance arm in model_test_args_hash with sorted(test.columns) (load-bearing — DEC-011). Mirrors accepted_values.values precedent. Cross-stage parity holds by re-export identity through signalforge.diff._artifact_id and signalforge.grade.engine (no per-module change). - src/signalforge/diff/_emitter.py: new isinstance arm in _render_test emitting {dbt_utils.unique_combination_of_columns: {combination_of_columns: [...]}}. Pydantic field 'columns' maps to dbt-utils macro key 'combination_of_columns' on emission only (field-name mapping seam). Emission preserves the LLM's declared order — sorting is for the canonical-hash domain only. Optional 'where' rendered verbatim under 'where' key when set; omitted when None. Tests: - tests/diff/test_artifact_id.py: 7 new unique_combination tests covering sort invariance (a,b) == (b,a), 3-column permutation, distinct columns → distinct hash, distinct where → distinct hash, collision suffix via compute_args_hashes, exact-duplicate ordinal suffix, cross-stage parity. - tests/diff/test_emitter.py: 5 new unique_combination tests covering no-where YAML shape, with-where YAML shape, declared-order preservation (the sort/no-sort load-bearing distinction), dropped-decision filtering, and the contract that unique_combination does NOT flow to emit_proposed_test_files. Validation: uv run ruff check / format / pyright / pytest all green. All 2873 tests pass; coverage 97.54%. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-0tq.10: US-009 — Establish grade-side _PROMPT_VERSION snapshot surface Establishes the grade-side cache-stability surface that .claude/rules/business-rule-tests.md § "Lockstep _PROMPT_VERSION rotation when extending the catalogue (#169 DEC-012)" claims exists but actually doesn't pre-#170. Closes the overstated rule-file claim with reality (#170 DEC-012 / US-009). Changes: - src/signalforge/grade/prompts.py: add module-level _PROMPT_VERSION constant computed at import as prompt_version_template(DEFAULT_RUBRIC). Mirrors the drafter shape (signalforge.draft.prompts._PROMPT_VERSION). Recipe documented inline: blake2b-8 over _SYSTEM_PROMPT + render_rubric_block(DEFAULT_RUBRIC) + envelope tags. Rotates when system prompt, any of the 4 default criterion texts, or envelope tags change. Exported in __all__. - tests/grade/test_prompt_cache_stability.py (new): pin the constant + the rendered rubric-block bytes. Two tests: hash-pin assertion and difflib-diffing byte-equality against an inline golden. Mirrors tests/llm/test_prompt_cache_stability.py shape verbatim. Rotation history documents the current value 4dae4421972e9c2d as the #170 US-009 establishment. - pyproject.toml: per-file-ignore E501 on the new test (inline rubric-block golden carries long single-line criterion texts that render together on the wire; refactoring would change the bytes the test pins). Pinned values (current, also matching the live helpers after US-008): - _PROMPT_VERSION = "4dae4421972e9c2d" - _canonical_rubric_hash(DEFAULT_RUBRIC) = "30a9fda975b6d45c" (pinned elsewhere by US-008; this commit does not touch it). Validation: ruff check, ruff format --check, pyright, pytest all green (2863 passed, 97.54% coverage). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-0tq.8: US-007 — Ingest parser arm + ingest anchor exemption Adds the recognition arm + helper for the ``dbt_utils.unique_combination_of_columns`` macro in ``signalforge.ingest.parser`` (dispatch site #4 per ``.claude/rules/business-rule-tests.md`` § "The 6 production dispatch sites") and the model-level loop exemption in ``signalforge.ingest.anchor.validate_anchor_contract`` (dispatch site #6 — the third model-level-only variant after ``custom_sql`` and ``row_count_between``). Inbound mapping (DEC-002 / DEC-008): YAML ``combination_of_columns: list[str]`` ↔ Pydantic ``columns: tuple[str, ...]``; optional ``where: str`` carried verbatim. The mapping seams are exactly these two functions (inbound here, outbound in ``diff/_emitter.py``) — the dbt_utils macro-name field does NOT bleed into the internal model. ``_UNIQUE_COMBINATION_NAME`` mirrors the ``_ROW_COUNT_BETWEEN_NAME`` precedent. Skip routes (all ``reason="malformed-supported-test"`` — closed 3-value ``SkipReason`` enum stays locked per ``.claude/rules/ingest-layer.md``): column-scoped usage (variant is model-level only); non-dict body; missing ``combination_of_columns`` key; non-list value under the key; empty list; ``len < 2`` (single-column is just ``unique``); non-string items (incl. bool guard mirroring ``_parse_row_count_between``); duplicate items; non-empty non-string or whitespace-only ``where``. Different sibling ``dbt_utils.*`` macros stay ``custom-or-generic-test``. Tests (15 new parser + 1 anchor): * tests/ingest/test_parser.py — happy paths (inline 2-col, with where, 3-col, arguments:-nested), 9 malformed routes, sibling-macro custom-skip, config-keys-ignored. * tests/ingest/test_anchor.py — model-level + ``column=None`` does not raise (mirrors ``test_model_level_row_count_between_with_none_column_does_not_raise``). Fixture updates: ``schema_codegen_shaped.yml`` and ``schema_austin_bikeshare.yml`` (and their consumers ``test_reader.py``, ``test_prune_existing.py``, and the column-scoped custom-skip pin in ``test_parser.py``) switched the example namespaced/custom test from ``dbt_utils.unique_combination_of_columns`` to ``dbt_utils.not_null_proportion`` — the original macro is now a first-class variant and column-scoped usage now correctly routes to malformed-supported-test. Also: pre-existing format drift in ``tests/draft/test_parser.py`` fixed in-passing so the pipeline gate stays green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-0tq.5: US-005a — Prune compiler arm _compile_unique_combination + BQ/Snowflake snapshots Implements the 7th first-class CandidateTest variant's compiler arm (#170 US-005a, traces to DEC-014 / DEC-015). * _compile_unique_combination in src/signalforge/prune/compiler.py emits the composite-grain failing-rows SELECT shape 'SELECT <cols> FROM <table_ref> [WHERE <where>] GROUP BY <cols> HAVING COUNT(*) > 1'. Dialect-driven via the existing _fold_identifier + _quote + _qualified_table_name helpers (no dialect.name branching — load-bearing per .claude/rules/prune-engine.md § 'Compiler is dialect-driven'). * Per-column DEC-014 identifier shape gate (defence-in-depth on top of the anchor-contract arm). Identifier rejection routes via _InvalidIdentifier → kept-without-evidence (conservative-bias). * DEC-015 compose-then-validate: 'where' is interpolated into the full SELECT then routed through validate_test_sql, mirroring _compile_row_count_between (#169 DEC-005 reuse). Hostile 'where' (stray ';', '--', '/* */', unbalanced parens) returns _InvalidIdentifier rather than raising. * Dispatcher arm added at _compile_test after the CandidateTestRowCountBetween branch. 6 new snapshot fixtures (3 BigQuery + 3 Snowflake) pin the byte-exact emitted SQL across both dialects. The 3 Snowflake fixtures are added to the gated sqlglot Snowflake-dialect parse-guard (tests/prune/test_compiler_fakesnow.py::_ALL_SNOWFLAKE_FIXTURES) per the #121 lesson — snapshot equality certifies shape, not validity; a parser-in-the-loop is what catches reserved-keyword / quoting regressions. Sample-mode routing is out of scope here (US-005b). The engine's source-vs-temp override for unique_combination ships in the sibling bead; this arm consumes table_ref as-is. The dispatcher arm comment points at the load-bearing engine-level pin (test_prune_tests_unique_combination_under_*) that US-005b will add. 12 new compiler tests (6 snapshot equality + adversarial column + hostile where x3 + dispatch-arm uniqueness + safety round-trip). The full default suite (2880 tests) passes; the gated -m snowflake suite (36 offline fakesnow/sqlglot tests) passes; pyright clean; ruff check/format clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-0tq.2: US-002 — Custom __repr__ redaction (mixin + retroactive) Establishes redacted __repr__ on the three text-bearing candidate-test variants — CandidateTestCustomSQL, CandidateTestRowCountBetween, CandidateTestUniqueCombination — so the LLM-emitted free-text fields (sql / where / rationale) never reach log sinks via casual repr() or %s-interpolation. Chosen shape: per-class __repr__ overrides + a small shared helper (_scope_repr) for the column/model-level scope segment. Each variant has a distinct identifying surface (custom_sql's column-scope handling, row_count_between's numeric bounds, unique_combination's columns tuple), so per-class bodies stay clearer than a single mixin. The scope helper keeps the column-vs-model-level convention in one place for any future variant. Surface exposed in repr(): - type (the discriminator Literal) - scope (column=<name> for column-scoped, <model-level> otherwise) - per-variant constraint shape: custom_sql shows only scope; row_count_between shows minimum/maximum bounds; unique_combination shows the columns tuple (column NAMES are not value-bearing) Surface redacted: - sql (CandidateTestCustomSQL) - where (CandidateTestRowCountBetween, CandidateTestUniqueCombination) - rationale (all three) Per the rule (prune-engine.md / grade-layer.md § "Custom __repr__"): Pydantic __str__ is reserved for serialisation and stays untouched — only __repr__ is overridden. model_dump_json() round-trip continues to carry every field (3 positive tests pin this). The four non-text-bearing variants (NotNull, Unique, AcceptedValues, Relationships) are untouched — they carry only column names and structured args, no free-text leak surface. Closes the log-hygiene gap surfaced by Phase 2 Security review (#170 DEC-013). Drive-by: ruff format on tests/draft/test_parser.py (pre-existing format drift from US-004 merge that VALIDATE_CMD would otherwise reject). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-0tq.3: US-003 — Drafter prompt catalogue + _PROMPT_VERSION rotation Add unique_combination as the 7th first-class test primitive in the drafter prompt catalogue (issue #170, DEC-002). Two illustrated forms (no-where and with-where) mirror the #169 row_count_between precedent. New _UNIQUE_COMBINATION_SCOPE_INSTRUCTION block carries cautionary prose steering the LLM away from vacuously-unique tuples like (pk, anything) — the prompt-level prevention complements the grade-side no-redundant criterion calibration (US-008). Catalogue + SCOPE wiring: - _TEST_CATALOGUE_LINES gains the unique_combination entry (after row_count_between), illustrating both no-where and with-where shapes with realistic 2-column examples. - _UNIQUE_COMBINATION_SCOPE_INSTRUCTION mirrors _CUSTOM_SQL_SCOPE_INSTRUCTION: emitted only when unique_combination is allowed, omitted when excluded so the prompt never asks for a type the parser would reject. - _render_system_prompt threads unique_combination_scope through the SCOPE template alongside custom_sql_scope. - _SYSTEM_PROMPT_TEMPLATE gets a {unique_combination_scope} format slot at the end of the SCOPE section. _PROMPT_VERSION rotation (77e9ee8a6ae7d875 → 389c8aa970df86cc): - Constant rotates automatically (computed from _SYSTEM_PROMPT bytes). - _EXPECTED_PROMPT_VERSION in tests/llm/test_prompt_cache_stability.py bumped to the new hex. - Rotation history extended with a #170 entry noting the new catalogue line + SCOPE instruction (cached-block golden unchanged — only the system prompt rotated). exclude_tests test fixes: - Four tests in tests/draft/test_exclude_tests.py added unique_combination to their exclusion tuples. They previously enumerated the standard set exhaustively; the 7th variant joining the catalogue means they need to exclude it too to preserve their original intent (testing how SCOPE renders with only some types remaining). Docstring updates name unique_combination + #170. "Five entries" → "six entries" in the _TEST_CATALOGUE_LINES docstring (custom_sql lives separately in _CUSTOM_SQL_CATALOGUE_LINE per the existing convention). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-0tq.6: US-005b — Engine sample-mode source override + behavioural routing pin Extends the existing row_count_between source-vs-temp conditional in prune/engine.py to include CandidateTestUniqueCombination — composite uniqueness on a sample is semantically approximate (false-negative risk: a duplicate pair may straddle the sampled and unsampled rows), so always-route-to-source mirrors #169 US-007a. Bounded by maximum_bytes_billed. Both sites move in lockstep: * `all_bypass_to_source` short-circuit (CodeRabbit #176 fix) — when EVERY candidate is row_count_between OR unique_combination, skip materialise_sample AND _resolve_sample_bucket pre-work; otherwise a materialisation failure would spuriously route every test to kept-without-evidence. * Per-test `per_test_table_ref` override — when scope="sample" and candidates are mixed, the row_count_between / unique_combination ones still route to source while other variants consume the substituted compile_table_ref. Behavioural routing pin at tests/prune/test_engine.py — mirrors the #169 row_count_between precedent (test_prune_tests_row_count_between_under_materialised_references_source_not_temp_table): * Parametrised across sample_strategy="materialised" AND "oneshot" — the load-bearing pin (snapshot equality from US-005a certifies SQL shape but NOT engine routing per .claude/rules/business-rule-tests.md § "Pin the engine-routing test, not just the compiler snapshot"). * Asserts compiled_sql references the source qualified name AND never references `_SESSION._sf_sample_*`. * Companion scope="full" test as a no-regression belt-and-braces. Traces to #170 DEC-006 (Option (iii): engine override to source via per_test_table_ref). Done when the engine routes unique_combination to source under sample mode, pinned by behavioural test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-0tq.11: US-010 — Mechanic exhaustiveness gate (6-site dispatch routing test) Adds a targeted dispatch-site routing test (NOT a full AST scan, per DEC-009 of #170) that constructs a minimal instance of every variant in the CandidateTest discriminated union and asserts each routes through every one of the 6 production dispatch sites without raising. The 6 sites (per .claude/rules/business-rule-tests.md § 'The 6 production dispatch sites'): 1. signalforge.prune.compiler._compile_test 2. signalforge._common.artifact_id.model_test_args_hash 3. signalforge.diff._emitter._render_test 4. signalforge.ingest.parser._parse_named_test (external macro recognition) 5. signalforge.draft.parser._validate_anchor_contract 6. signalforge.ingest.anchor.validate_anchor_contract Each variant × site combination is one parametrize iteration; site 4 skips for variants without an external dbt-macro form (only row_count_between and unique_combination have one). Self-checks pin that sites 5 and 6 raise on real violations so the routing arms can't silently mask dispatch bugs with a tautological green. Variant reflection via typing.get_args means an 8th variant added to the union auto-grows the parametrize without a test edit. A cardinality tripwire asserts the union currently holds exactly 7 variants; bumping the union forces the contributor to update _EXPECTED_VARIANT_COUNT, _make_instance, and _EXTERNAL_MACRO_YAML in lockstep — all gated by this test rather than discovered at runtime on the operator's machine. TDD verified: temporarily removing the unique_combination arm from _render_test (site 3) and the model-level exemption from validate_anchor_contract (site 6) each fail one parametrize iteration loudly with a remediation message pointing at the missing arm. 48 parametrize iterations (43 pass + 5 N/A site-4 skips); no production code change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-0tq.14: US-013 — docs SSOT + README + ops paraphrases + CHANGELOG + mkdocs nav Bundles the documentation tail of #170 (unique_combination as 7th first-class CandidateTest variant). Per Phase 1 S2 + DEC-003, the SSOT lives in the operator-facing docs/ tier (not .claude/rules/). - docs/drafter-catalogue.md (new) — SSOT enumerating the seven first-class primitives (not_null, unique, accepted_values, relationships, custom_sql, row_count_between, unique_combination) with YAML examples, structural slots, scope, ingest signatures, semantics; a 'custom_sql is the catch-all' sub-section; a 'What we do NOT generate today' boundary section (column value range, conditional uniqueness beyond simple where, statistical/distributional anomalies, cross-table reconciliation, time-series anomaly detection). - README.md — new 'What tests SignalForge generates' section sitting between 'What it does' and 'How it works' (compact 7-row table + pointer to the SSOT). - docs/draft-ops.md — new 'Composite uniqueness (unique_combination)' section after the row_count_between block, mirroring its shape: what the variant is, when drafted, worked example, exclude_tests short-circuit. - docs/ingest-ops.md — new 'Recognition of dbt_utils.unique_combination_of_columns' subsection mirroring the expect_table_row_count_to_be_between precedent: inbound mapping, skip-recorded shapes, unchanged-other-dbt_utils-macros boundary. - docs/grade-ops.md — new 'Composite-key calibration (unique_combination)' subsection extending the no-redundant criterion narrative; rubric stays at four criteria. - docs/prune-ops.md — callout in the row-count cost model section for the unique_combination engine source-vs-temp routing override. - mkdocs.yml — adds 'Test Catalogue: drafter-catalogue.md' to the nav between 'Claude Code Skill' and 'Pipeline Stages'. - CHANGELOG.md [Unreleased] — Added (unique_combination variant + dbt_utils.unique_combination_of_columns ingest recognition), Docs (drafter-catalogue.md + README section + four ops doc paraphrases), Changed (drafter _PROMPT_VERSION rotation 77e9ee8a6ae7d875 → 389c8aa970df86cc, grade rubric no-redundant extension, grade-side _PROMPT_VERSION snapshot surface 4dae4421972e9c2d established). Validation: uv run ruff check/format/pyright + uv run pytest (2922 passed, 78 deselected, 97.70% coverage) + uv run --only-group docs mkdocs build green. Pre-existing pre-#170 INFO-level link warnings (e.g. draft-ops.md#row-count-tests-row_count_between anchor missing in rendered HTML — a #169 rendering bug from the BUSINESS RULES code fence) are unaffected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-0tq.13: US-012 — Gated e2e (drafter emits structured unique_combination) New @pytest.mark.e2e-gated test pinning the load-bearing behavioural claim of #170: when the drafter sees a model whose SELECT body advertises a natural composite GROUP BY shape (US-011's engineered fixture model stg_bikeshare_station_pairs), it proposes a structured CandidateTestUniqueCombination candidate — NOT a freeform custom_sql GROUP BY HAVING COUNT(*) > 1. Standard three-env-var gate (mirrors test_e2e_bigquery_smoke.py baseline): SF_RUN_BQ=1 + GOOGLE_CLOUD_PROJECT + ANTHROPIC_API_KEY. Drafter is Anthropic Sonnet 4.6; warehouse is BigQuery; no provider overlay. Engineered determinism via structural (not value-pinning) assertion: AT LEAST ONE PruneDecision must carry test.type == 'unique_combination' with len(test.columns) >= 2. The exact columns tuple is NOT pinned because Sonnet may legitimately propose two- or three-column variants of the fixture's GROUP BY shape; the prune verdict (kept vs. dropped) is NOT pinned because bikeshare data shape is orthogonal to the freeform→structured translation under test. Test reads PruneDecision.test from .signalforge/prune.jsonl via the existing read_prune_decisions helper — the typed CandidateTest discriminated union flows through prune intact (PruneEvent.test: CandidateTest, audit DEC-014). Validation green: ruff + format + pyright + pytest all clean. Test is correctly deselected by default (not e2e) addopts and skips with distinct reason for each missing env var when invoked with -m e2e. AC-1 (drafter proposes structured unique_combination), AC-8 (end-to-end pipeline shape). Maintainer-run command: SF_RUN_BQ=1 ANTHROPIC_API_KEY=sk-... GOOGLE_CLOUD_PROJECT=<project> \ uv run pytest -m e2e -k unique_combination --no-cov Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-0tq.15: US-014 Quality Gate — 6 fixes from 4 reviewer angles Triangulated findings across correctness / conventions / tests / docs+UX reviewers; same finding from 2+ angles upgraded to must-fix per memory qg-diverse-reviewer-angles-catch-cross-surface-drift. Fixes applied: 1. **F1 (Pass 4 must-fix)** — Pre-existing `## BUSINESS RULES` literal inside fenced code block in docs/draft-ops.md:288-300 was parsed as ATX heading by mkdocs anchor generator, breaking 14 of 20 H2 anchors downstream (covers row_count_between + unique_combination — all 8 inbound cross-doc links #170 added go through these). Switched to indented code block (4-space) which defeats the heading scan. 2. **C1 (Pass 1 concern, empirically verified)** — DEC-013 __repr__ redaction was leaking via Pydantic v2's __repr_args__ / __rich_repr__ / __pretty__ hooks (rich.print() / devtools / pprint). Added __repr_args__ overrides on the 3 redacting classes (CandidateTestCustomSQL, CandidateTestRowCountBetween, CandidateTestUniqueCombination). Filters out where / sql / rationale from the structured-debug surface. New test pins the closure across all 3 variants + asserts model_dump_json() still carries the secrets (serialisation contract unchanged). 3. **Pass 3 must-fix (load-bearing routing pin gap)** — US-005b per_test_table_ref override has two conditionals (all_bypass_to_source short-circuit AND per-test override). Single-variant tests only exercised the first; dropping unique_combination from the per-test arm while leaving it in the short-circuit would PASS silently. Added test_prune_tests_mixed_candidates_per_test_override_routes_unique_combination_to_source with not_null + unique_combination mix — exercises the per-test arm directly. Asserts not_null compiles against _SESSION sample, while unique_combination compiles against source — both routings pinned. 4. **F2 (Pass 4 must-fix)** — CHANGELOG Added entry was silent on the dbt-utils install constraint (parallel gap to #169's note on dbt-expectations). Appended the parallel clause. 5. **F3 (Pass 4 should-fix)** — CHANGELOG Changed entries on both _PROMPT_VERSION rotations now name the one-time Anthropic prompt-cache miss operators pay on upgrade. 6. **F4 (Pass 4 should-fix)** — docs/drafter-catalogue.md row_count_between subsection was missing the source-vs-temp routing callout that unique_combination has; added parallel one-paragraph callout naming both variants in lockstep. Deferred to US-015 / follow-up per Pass 2 informational findings: - US-004 #169 pre-existing format drift on tests/draft/test_parser.py (3 workers all "drive-by formatted" the same file; investigate why US-004's own ruff format --check passed) - custom_sql lacks model-level loop exemption in ingest.anchor (benign today, documented gap from US-010 worker) - E501 ignore asymmetry on drafter vs grade snapshot test - _PROMPT_VERSION in grade.prompts __all__ cosmetic - Missing "description" config-key in unique_combination ingest test Validation green across: - uv run ruff check . && uv run ruff format --check . - uv run pyright (0 errors) - uv run pytest (full suite + new tests) - uv run pytest -m cli_subprocess --no-cov (8 pass) - uv run pytest -m wheel_smoke --no-cov (5 pass) - uv run pytest -m snowflake --no-cov (36 pass + 5 live-only skips) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-0tq.16: US-015 Patterns & Memory — rule updates + 4 memory writes Closes #170 Patterns & Memory tail. ## Rule file updates - **`.claude/rules/business-rule-tests.md`** — bumped "2-instance precedent" → "3-instance precedent" (custom_sql / row_count_between / unique_combination). Reference footnote extended with the #170 plan pointer + the durable conventions section "#170 lessons worth carrying forward" enumerating the 5 patterns the next variant-extension should pre-empt: 1. Two engine conditionals (`all_bypass_to_source` short-circuit AND per-test `per_test_table_ref` override) — both grow in lockstep; mixed-candidate test is load-bearing for the per-test arm. 2. Pydantic v2 `__repr_args__` / `__rich_repr__` / `__pretty__` hooks bypass `__repr__` — override `__repr_args__` for redaction parity across rich.print() / devtools / pprint. 3. SORT a tuple-shaped canonical hash arg (`(a,b) ≡ (b,a)` semantics); the diff EMITTER preserves declared order — split contract. 4. mkdocs ATX heading in fenced code block silently corrupts downstream H2 anchors; use indented (4-space) code blocks for examples that need a literal `##`. 5. Drive-by formatting across multiple workers reveals merge-induced format drift the original PR's gates can't see — add a post-merge `ruff format --check .` to the orchestrator's Step 5. - **`.claude/rules/grade-layer.md`** — documented the grade-side `_PROMPT_VERSION` snapshot surface established by #170 DEC-012, closing the asymmetry where `business-rule-tests.md` had historically claimed "two `_PROMPT_VERSION` constants" but only the drafter had one. Rotation policy mirrors the drafter contract — rotate when the grade `_SYSTEM_PROMPT` text changes OR any of the four DEFAULT_RUBRIC criterion texts changes; the new value is computed and pinned in the same commit (US-008 + US-009 paired). ## Memory writes (4 new files + MEMORY.md index) All in `~/.claude/projects/-home-wesd-Projects-SignalForge/memory/`: 1. `pydantic-v2-repr-args-redaction-required.md` — DEC-013 lesson; custom `__repr__` alone leaks via `__rich_repr__` / `__pretty__`. 2. `prune-engine-two-conditional-routing-pattern.md` — US-005b discovery; mixed-candidate test is load-bearing. 3. `mkdocs-atx-in-fenced-block-breaks-anchors.md` — QG Pass 4 finding; indented code blocks are the fix. 4. `drive-by-format-reveals-merge-induced-drift.md` — observation across US-005a / US-007 / US-002; orchestrator can pre-empt with a post-merge `ruff format --check`. ## Deferred to follow-up issues (not blocking #170) - `ingest.anchor.validate_anchor_contract` lacks model-level loop exemption for `custom_sql` (benign today; documented by US-010 worker in the dispatch-exhaustiveness test docstring). - US-004 of #169 pre-existing format drift on `tests/draft/test_parser.py` (each of US-005a / US-007 / US-002 fixed it independently; root cause is merge-time drift not worker carelessness). - E501 ignore asymmetry on grade vs drafter snapshot tests. - `_PROMPT_VERSION` in `signalforge.grade.prompts.__all__` (cosmetic inconsistency with drafter side). Validation green: ruff / pyright / pytest (2967 passed). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #170: Address PR review feedback (CodeRabbit, 5 threads) All 5 review threads addressed: **Major (1):** - `src/signalforge/ingest/anchor.py` — the `unique_combination` model-level loop exemption was skipping `test.columns` validation entirely (CodeRabbit MAJOR; triangulated with QG Pass 1 C2 + Pass 2 informational #2 + US-010 worker docstring — four-way agreement). Extended the arm to iterate `test.columns` and surface per-column violations mirroring the draft-parser side. Pinned by new test `test_model_level_unique_combination_with_hallucinated_column_raises_per_column` asserting collect-all (two hallucinated columns → two distinct violations in one error). Used `isinstance(test, CandidateTestUniqueCombination)` for proper pyright narrowing of the discriminated union. **Minor (4):** - `docs/draft-ops.md` link target → direct `unique_combination` anchor in prune-ops.md (was pointing at `#row-count-cost-model`). - `docs/drafter-catalogue.md` link target → same direct anchor fix. - `plans/super/170-unique-combination.md` DEC-002 row — escaped `|` in `str \| None` so the markdown table parses correctly (markdownlint MD056). - `plans/super/170-unique-combination.md` story-dependency-graph fenced block — added `text` language identifier (markdownlint MD040). Validation green: ruff / pyright / pytest (2968 passed, +1 new test).
This was referenced Jun 1, 2026
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
Super plan for #169 — add
row_count_betweenas a 6th first-classCandidateTestvariant (drafter + prune + grade + diff + ingest).Phase: detailing (awaiting approval)
Stories: 12 implementation + Quality Gate + Patterns & Memory = 14
Decisions: 15 DECs captured
What's being built
A 6th first-class
CandidateTestvariant mirroring thecustom_sql(#116) extension pattern. The drafter proposes it as a structured artifact (not freeformcustom_sql), prune evaluates a full-tableCOUNT(*)against bounds, grade scores calibration, diff emits thedbt_expectations.expect_table_row_count_to_be_betweenYAML block, andprune-existingrecognises operator-authored declarations from external schema.yml.Why it matters now
Survey of
intuit_airflow/plugins/dbt/(Snowflake, 104 models, ~243 tests): 40 % of all declared tests (98 / 243) aredbt_expectations.expect_table_row_count_to_be_between— the single most-used test type, ahead of every column-scoped check combined. A livesignalforge generateagainstweekly_query_costproduced 8not_null+ 11custom_sqltests but zero row-count checks despite the team havingmin=100declared. SignalForge is leaking the single most common dbt-expectations primitive — a direct hit on Architectural Commitment #1 ("signal over volume").Plan document
See
plans/super/169-row-count-between.mdfor the full plan.Key decisions
CandidateTestRowCountBetween— model-level only,minimum/maximum/where/rationaledbt_expectationsYAML block always (no.sqlfallback in v1)COUNT(*), ignoreprune.scopewheresupportwherevalidationvalidate_test_sqlwhere— closes AC-5 in this ticketminimum/maximumon the model;min_value/max_valuein the emitted YAMLflaggedDropReason/SkipReason/audit_schema_versionall stay lockedrow_count_betweenarm(See plan-doc for the full 15.)
Story shape
Stories follow the natural data-flow order: models → config → prompts → parser → ingest → artifact_id → compiler → engine → grade → emitter → docs → e2e → Quality Gate → Patterns & Memory. Most are parallelizable after
US-001(the variant class) lands.Next steps
🤖 Generated with Claude Code
Summary by CodeRabbit