chore: release 0.3.0 - #149
Conversation
) * 93: silence pydantic UserWarning for LLMRequest.schema field shadow The `schema` field name on `LLMRequest` is part of the documented audit-log contract (safety-layer.md DEC-014), but it shadows Pydantic v1's deprecated `BaseModel.schema` method, triggering a UserWarning at class-creation time that surfaces on every fresh import / CLI invocation. Wrap the class definition in `warnings.catch_warnings()` with a targeted filter — narrowest blast radius; no global filter mutation, the contract stays unchanged. Adds a subprocess-import regression test under `-W error::UserWarning` so future shape changes can't silently reintroduce it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * 93: document Pydantic field-name-shadow suppression pattern Compounding update from PR #94: capture the `catch_warnings()` scope-to-class convention so the next field-name-shadow case (audit-log contract + Pydantic v1 BaseModel attribute collision) doesn't get "fixed" by renaming or a global filter. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * 93: address PR review feedback - models.py: relax warning-filter message from exact literal to regex (`Field name "schema".*shadows.*`) so future Pydantic message-wording drift still gets caught. Drops the experimental `module=` qualifier (over-narrowing — filter stopped matching the live warning). - test_models.py: add `timeout=10` to the subprocess import smoke, matching the precedent in `tests/cli/test_subprocess_smoke.py` so a pathological import hang fails fast instead of stalling the suite. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Audited the root README and docs/ ops files against the shipped code: - demo profiles.yml: add maximum_bytes_billed: 1000000000 so the README quick start actually works — the demo's materialised-sample CTAS over the full ~2.27M-row bikeshare_trips source exceeds the adapter's 100 MB default cap. Parity-test docstring updated to document the new rewrite. - README: add the kept-uncertain tier to the expected-output section (four-tier taxonomy was undocumented), complete the generate/lint flag lists in the CLI section, mention lint --model. - docs/audits.md: fix stale audit_schema_version values (safety 1 -> 3, prune 1 -> 2) bumped by issues #54/#55. - docs/e2e-smoke-test.md: fix stale cross-reference claiming the README quick start does a manual profile rewrite — it uses signalforge init-demo. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
) * build: migrate dev tooling to uv + widen Python matrix to 3.11/3.12/3.13 Mirrors clauditor's build setup. uv becomes the canonical dev install (`uv sync --dev`); `uv.lock` is committed; CI runs uv-managed steps across a 3-Python matrix. `pip install -e ".[dev]"` is retained for back-compat — `[project.optional-dependencies].dev` and the new `[dependency-groups].dev` are kept in sync. - `pyproject.toml`: add `[dependency-groups].dev` (PEP 735); only delta vs the pip-extra is `build>=1.2,<2` for the wheel_smoke marker. - `.github/workflows/ci.yml`: replace setup-python + pip install with `astral-sh/setup-uv@v8.1.0` (SHA-pinned per ci-supply-chain.md) + `uv sync --dev`; wrap lint/test steps with `uv run`. Matrix runs ruff + pytest on all three versions; pyright gated to the matrix floor (3.11, matching pyright.pythonVersion); codecov upload gated to the matrix ceiling (3.13) so coverage doesn't double-upload. - `.github/workflows/publish.yml`: `python -m build` → `uv build` in both publish-testpypi and publish-pypi jobs. - `.gitignore`: drop `uv.lock` from the ignore list (bark scaffolding artifact comment is stale — the canonical lock is committed now). - `CLAUDE.md` / `CONTRIBUTING.md` / `README.md` / `docs/cli-ops.md` / `docs/codecov-ops.md` / `docs/manifest-loader-ops.md`: switch copy-pasteable dev commands to `uv sync --dev` + `uv run …`. - `.claude/rules/python-build.md`: replace "Editable install (zsh-safe)" with a uv-managed dev environment section; refresh issue-#46 floor-pinning to describe the matrix. - `.claude/rules/ci-supply-chain.md`: add `astral-sh/setup-uv` to the SHA-pinning example list; graduate DEC-003 from single-Python to a 3-Python matrix with the pyright / codecov gating rationale. Local validation: `uv run ruff check .` / `uv run ruff format --check .` / `uv run pyright` / `uv run pytest` all clean under Python 3.13; `uv run --python 3.11 pytest --no-cov` and `--python 3.12` both 1830/1830 pass. wheel_smoke passes under `uv run`. (Six pre-existing WSL2-only symlink-loop test failures on 3.13 unaffected; pass in GHA.) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: drop 3.13 from matrix; document path-safety follow-up CI exposed six 3.13-only test failures under tests/_common/, tests/diff/, tests/grade/, tests/manifest/. Root cause: Python 3.13 changed Path.resolve() to raise OSError(errno.ELOOP) on cyclic symlinks instead of RuntimeError. SignalForge's _canonicalise_path and the manifest loader's path canonicalisation catch RuntimeError only — on 3.13 the cycle now escapes as OSError or short-circuits to a different typed error before the loop check fires. Fixing it touches production code (signalforge/_common/path_safety.py and signalforge/manifest/loader.py) plus four test fixtures, which is substantial scope creep for a tooling-migration PR. Narrow the matrix to 3.11 / 3.12 (both green: 1830 passed locally and in CI) and track the 3.13 work in a follow-up issue. - ci.yml: matrix python-version: ["3.11", "3.12"]; codecov-upload gate flips from 3.13 → 3.12 (matrix ceiling). Comment block records the reason and points at the follow-up. - CLAUDE.md / CONTRIBUTING.md / docs/cli-ops.md / python-build.md / ci-supply-chain.md: every "3.11 / 3.12 / 3.13" reference now reads "3.11 / 3.12"; ci-supply-chain.md and python-build.md add a "3.13 is deferred" paragraph naming the catch-site fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: address CodeRabbit + Copilot review feedback Five fixes from PR #95 review (all real issues, no false positives): - `.claude/rules/ci-supply-chain.md`: codecov-gate example said `matrix.python-version == '3.13'` (stale from the 3.13 matrix); current CI gates on `'3.12'`. Updated and noted the flip-back when issue #96 lands. [coderabbit/copilot — same finding, threads 1+3] - `.github/workflows/ci.yml`: `actions/checkout` now sets `persist-credentials: false` — CI is read-only; least-privilege for the GITHUB_TOKEN. [coderabbit — thread 4] - `.github/workflows/publish.yml`: same `persist-credentials: false` on both `Checkout` steps; same justification (build job doesn't push to git). [coderabbit — thread 5] - `.github/workflows/publish.yml`: `astral-sh/setup-uv` now pins `python-version: "3.11"` on both jobs for release-build reproducibility. Without it, `uv build` picks whatever `ubuntu-latest` happens to ship. [copilot — thread 2] - `docs/cli-ops.md`: clarify that the `pip install -e ".[dev]"` fallback omits the `build` package (uv-only delta in `[dependency-groups].dev` powering wheel_smoke). Pip users shouldn't assume identical tooling coverage. [coderabbit — thread 6] Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs: publish MkDocs Material site to GitHub Pages on push to main Mirrors clauditor's docs-publishing setup. The published site at https://wjduenow.github.io/SignalForge/ redeploys on every push to main; the gh-pages branch is generated, not authored. - `mkdocs.yml`: Material theme with light/dark toggle, search + include-markdown plugins, mermaid via pymdownx.superfences, exclude_docs: research/ (internal analysis stays off the site), edit_uri: edit/dev/docs/ (doc edits land on dev like every other PR; main is the release line). Nav: Home + CLI Reference + Pipeline Stages (manifest/warehouse/safety/draft/prune/grade/diff) + Audits & Sidecars + e2e + coverage. - `docs/index.md`: 4-line include-markdown stub pulling in the root README. Mirrors clauditor verbatim — README stays the canonical authored home doc; site home auto-syncs on every build. - `.github/workflows/ci.yml`: new `docs:` job gated on `github.ref == 'refs/heads/main' && github.event_name == 'push'` with job-scoped `permissions: contents: write` (workflow default stays `contents: read`). Uses pinned `astral-sh/setup-uv@v8.1.0` with `python-version: "3.11"` (matches publish.yml floor); runs `uv run mkdocs gh-deploy --force --no-history`. Deliberately does NOT set `persist-credentials: false` on the checkout step — `gh-deploy` needs the persisted GITHUB_TOKEN to push to gh-pages. - `pyproject.toml`: add `mkdocs-material>=9.0` and `mkdocs-include-markdown-plugin>=6.0` to `[dependency-groups].dev` only. Pip contributors don't need to build docs; this stays a uv-only delta (alongside `build` for wheel_smoke). - `.gitignore`: ignore `site/` (mkdocs build output). - `README.md`: docs badge top-of-file. - `CLAUDE.md` / `CONTRIBUTING.md`: one-liner pointing at the published site and the deploy contract. - `.claude/rules/docs-publishing.md` (new): full deploy contract — trigger rule, include-markdown pattern, edit_uri choice, persist-credentials exception, exclude_docs convention, local build recipe, when to update mkdocs.yml vs the docs themselves, first-time GH Pages setup the maintainer does once. Local validation: `uv run mkdocs build` succeeds (warnings on repo-internal links to plans/super/ and .claude/rules/ are expected and matched in clauditor — those aren't part of the published site). `uv run ruff check .` / `ruff format --check .` clean. 1823/1823 tests pass on 3.13 (6 pre-existing #96 failures unrelated). Post-merge maintainer step (once): Settings → Pages → source = "Deploy from a branch", branch = gh-pages, folder = / (root). The first push to main after this merges lands the gh-pages branch; toggle Pages on right after. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: address PR #97 review — split docs dep group + PR-time build gate Two Copilot suggestions, both real wins: - Extract `[dependency-groups].docs` (mkdocs-material + mkdocs-include-markdown-plugin only). Both CI docs jobs now `uv run --only-group docs ...` so they pull just MkDocs + plugins, not the heavy dev set (dbt-core, pyright, pytest). `dev` includes `docs` via `{include-group = "docs"}` so `uv sync --dev` still gives contributors everything. [thread: ci.yml deploy pulled --dev] - Add a read-only `docs-build` job that runs on every PR + push (`uv run --only-group docs mkdocs build`, no write perms). Catches a broken mkdocs.yml / plugin config / include-markdown syntax error at PR time instead of silently merging and only failing the post-merge gh-pages deploy. [thread: no pre-merge docs build signal] The third thread (Home page "Edit this page" link points at the docs/index.md include stub rather than README.md) is an accepted limitation of the include-markdown pattern — Material has no per-page edit-button hide without a template override, and clauditor's identical setup accepts it. The stub a contributor lands on literally documents where the real content lives. Documented in the review summary; no code change. `.claude/rules/docs-publishing.md` updated: two-job structure, the docs dependency-group split, and the "new plugin goes in docs not dev" convention. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Branch discipline: test releases are now cut from `dev` (was "any branch") and full releases from `main` (unchanged). The publish.yml prerelease-flag routing is unchanged — it's the mechanism; the branch is the maintainer discipline the skill enforces in pre-flight. dev is the in-development line feeding TestPyPI; main is the released line feeding PyPI. Also refreshes the skill for the uv migration (PRs #95/#97): - validation command → `uv sync --dev && uv run ruff/pyright/pytest` - `python -m build` → `uv build` (both build blocks + PR-body string) - allowed-tools gains `Bash(uv *)`; compatibility frontmatter updated - left the TestPyPI clean-room `pip install` smoke test as-is (it deliberately installs the published artifact in a throwaway venv) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Step 5 TestPyPI smoke test used a bare `python -m venv`, which picks up whatever `python3` is on PATH — on a host where that's 3.10, pip silently ignores the new release (requires-python >=3.11) and only the pre-floor 0.1.0rc1 installs, producing a confusing "Could not find a version that satisfies the requirement" right after a green publish. Surfaced cutting v0.1.0rc3: the first clean-room attempt used the system 3.10 and rejected rc3/rc2 as "Requires-Python >=3.11". Fix: provision the interpreter explicitly via `uv venv --python 3.11` (the requires-python floor — verifies installability on the minimum supported version; uv auto-fetches 3.11 if absent) and `uv pip install --python <venv>`. Also documents how to distinguish this from the separate TestPyPI Fastly-cache propagation lag. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CodeRabbit flagged on PR #98: setup-uv's default persists the uv cache to the GitHub Actions Cache. On a public repo, pull_request runs share a cache scope with the base branch, so a fork PR can poison an entry a later trusted run restores — a cross-trust-boundary cache-poisoning vector. Set `enable-cache: false` explicitly on every setup-uv step: ci.yml (lint-test, docs-build, docs-deploy) + publish.yml (testpypi, pypi). Runs are short enough that the lost cache is negligible; the publish/deploy steps especially must never restore a poisoned cache into a published artifact or the gh-pages deploy. Documents the convention in .claude/rules/ci-supply-chain.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
# Conflicts: # src/signalforge/__init__.py
signalforge-dbt 0.1.0 is on PyPI, so lead the install with `pip install signalforge-dbt` instead of the clone+uv-sync path. - Status line: "Not yet on PyPI" → "Live on PyPI — pip install signalforge-dbt". - Quick start §1: pip install first + `signalforge --version` verify; note the distribution-vs-import name; add `uv tool install` / `pipx` as the isolated-CLI alternative; demote the clone + `uv sync --dev` path to a contributing pointer at CONTRIBUTING.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add super plan for #104: ingest external dbt schema.yml tests Library-only seam (signalforge.ingest.read_schema -> IngestResult). prune-existing CLI subcommand split to fast-follow #105. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Mark #104 plan published (PR #106) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Devolve #104 plan to beads (epic bd_1-scaffolding-ky3, 8 tasks) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ky3.1: scaffold signalforge.ingest package + IngestError hierarchy + exit-code lockstep US-001 of issue #104. Adds the signalforge.ingest subpackage scaffold and its six-class typed-error hierarchy (DEC-001), wired into the CLI four-tier exit-code taxonomy and the 7th AST scan. - src/signalforge/ingest/__init__.py: package init re-exporting the IngestError surface. - src/signalforge/ingest/errors.py: IngestError base (remediation + ↳ Remediation rendering, repr-safe values) plus five concretes — IngestSchemaNotFoundError / IngestSchemaParseError / IngestSchemaTooLargeError (tier 1) and IngestModelNotFoundError / IngestAnchorContractError (tier 2). - cli/_helpers.py: every concrete mapped in _EXCEPTION_TO_EXIT_CODE at its tier; IngestError registered as fallback tier-1 (dual-registration). - tests/test_audit_completeness.py: IngestError added to _EXCEPTION_MAPPING_EXCLUDED_BASES; Scan-7 module count bumped 10 -> 11. - tests/cli/test_exit_codes.py: synthetic-construction branches for the (size, limit) and (violations,) constructor shapes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ky3.2: typed ingest result models (IngestResult, SkippedTest, SkipReason) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ky3.4: anchor-contract validator (fail-loud, collect-all) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ky3.3: dbt test-entry parser (pure mapping) Pure parse_test_entry(entry, *, column) mapping a single dbt schema.yml test entry to a CandidateTest or a structured SkippedTest. Handles bare strings, single-key dicts with inline and arguments:-nested args (DEC-006), config-key tolerance, malformed-supported skips, custom / namespaced skips, and best-effort ref()/source() unwrap in relationships.to (DEC-009). No logging, no I/O, not part of the public surface. 19-test matrix added (TDD). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ky3.5: read_schema orchestrator Implement the public ingest entry point read_schema(schema, model, *, project_dir=None) -> IngestResult tying together the parser, anchor validator, and result models. Steps: str-vs-Path input contract (Path = file, symlink-hardened via _common.path_safety; str = raw YAML), size-cap before yaml.safe_load (DEC-005), model-block selection by name (DEC-006), union+dedupe tests/data_tests (DEC-008), description="" default (DEC-010), whole-file anchor check. No logging (stage-0 reader). Re-export read_schema from the package __init__ as the public surface. Adds the dbt-codegen-shaped fixture and 13 tests: happy path (supported + skipped tests, dedupe, ref() unwrap, both test keys), str-content input, and each error path (model-not-found, malformed YAML, oversize, anchor violation, missing path), plus the disabled-prune acceptance bonus. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ky3.6: docs/ingest-ops.md operational reference + mkdocs nav Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ky3.7: Quality gate — fix bugs from code review - parser: model-level supported test (column=None) now routes to a structured malformed skip instead of raising a Pydantic ValidationError out of read_schema. - reader: accepted_values dedupe key now sorts values so reordered value sets collapse (DEC-008 "sorted-args" contract). - parser: clarify _extract_args docstring for non-dict `arguments`. - tests: regression coverage for both fixes (+3 ingest tests). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ky3.8: Patterns & Memory — ingest-layer rule + CLAUDE.md + cli-layer count - New .claude/rules/ingest-layer.md distilling the #104 conventions (str-vs-Path contract, fail-loud anchor vs skip-record taxonomy, no-logging stage-0, safe-YAML size cap, dbt syntax tolerance, 11th errors.py exit-code lockstep, deferred #105 CLI). - CLAUDE.md: #104 shipped-issue entry + public-API surface entry. - cli-layer.md: scan-7 count 10->11; IngestError joins DemoError as a span-tier base (frozenset-only, no fallback) — and aligned _helpers.py by removing the IngestError tier-1 fallback to match that precedent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #104: Address PR review feedback (Copilot) - errors.py: correct IngestError docstring — base is frozenset-only (no fallback tier), concretes span tiers 1 and 2 like DemoError. - __init__.py: refresh module docstring — the full library seam (reader, parser, anchor, models) ships in #104, not "typed-error surface only". - parser.py: _extract_args now drops the structural `arguments` key from inline args so a non-mapping `arguments:` can't leak (+ regression test). - docs/ingest-ops.md: fix usage snippet to use WarehouseAdapter.from_profile (the public factory; there is no top-level from_profile) and pass an un-entered adapter to prune_tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add super plan for #105: signalforge prune-existing CLI subcommand Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Mark plan phase published (PR #107) * Mark plan devolved; record beads manifest (epic bd_1-scaffolding-ad1) * bd_1-scaffolding-ad1.1: Hoist bare-name model resolver to _helpers #105 US-001 / DEC-008. Move the bare-name model resolver from lint.py to signalforge.cli._helpers as _resolve_model_by_key(manifest, key) -> Model so the future prune-existing subcommand shares one resolver rather than copy-pasting the body. lint.py keeps a thin _resolve_model_for_lint wrapper that delegates. Identical behaviour: unique_id/file-path via Manifest.get_model; bare-name via iter_models with multi-match disambiguation + disabled-model exclusion + the same ModelNotFoundError messages. No behaviour change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ad1.2: Add prune-existing add_parser + stub handler US-002 of #105 — contract surface (DEC-001, DEC-002, DEC-003). Adds src/signalforge/cli/prune_existing.py with add_parser registering the prune-existing subcommand and full flag set (positional <model>, required --schema, --project-dir/--manifest/--profiles-dir, --scope/--sample-strategy/--format choice flags, --dry-run, and the --quiet/--verbose/--no-color observability triad). Drops --mode/--write/ --min-score/--select/--estimate per DEC-002/DEC-003. cmd_prune_existing is a stub returning 0 (full ingest->prune->diff body lands in US-003). Registered in signalforge.cli.__init__._build_parser. Help strings are surface 1 of the 5-surface parity contract. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ad1.5: Add prune-existing --help subprocess smoke Extends the @pytest.mark.cli_subprocess gate to the prune-existing subcommand (#105 US-005), mirroring the existing generate/lint/version/ init-demo --help smokes. Asserts returncode 0, the unique --schema flag in stdout, and the no-traceback floor on stderr. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ad1.3: Implement cmd_prune_existing orchestrator (ingest -> prune -> diff) US-003 of #105. Replaces the US-002 stub with the real ingest -> prune -> diff orchestrator (no draft, no grade, no LLM call), inside one try/except Exception boundary (DEC-016 — no traceback ever leaks). - Resolve project_dir (mirror generate: --project-dir absolute assertion, walk-up default); --no-color/--profiles-dir env mutation (DEC-023). - Resolve <model> via _resolve_model_by_key (bare-name / unique_id / file-path). Canonicalise --schema via canonicalise_user_path (DEC-005). - read_schema(Path, model, project_dir=...) so ingest typed errors fire and route via the pre-registered exit-code table (DEC-006 — no bespoke CliPruneExisting* wrappers). - Skipped-test report to stderr: grouped summary + --verbose per-item detail, suppressed by --quiet (DEC-007). print_stderr, not _LOGGER. - PruneConfig --scope/--sample-strategy overrides via model_validate (DEC-002). _make_warehouse_adapter test seam (DEC-009); prune_tests owns the with-adapter block (un-entered adapter). - Feed the external schema.yml text as render_diff existing_schema with grading_report=None -> kept/kept-uncertain/dropped, never flagged (DEC-004); --dry-run -> write_sidecar=False. render_to_text -> stdout. - 3-stage progress (1/3 ingest, 2/3 prune, 3/3 diff) via the existing emit_progress_* helpers, generalised with a total= kwarg (default 5). Adds an Austin-aligned schema.yml fixture whose four supported test types reference real stg_bikeshare_trips columns plus two unsupported tests for the skip path. Tests cover happy path, skipped report (+ --verbose + --quiet), each error-path exit-code tier with a no-traceback floor, --dry-run no-disk, --format json, and bare-name vs unique_id resolution, via a FakeBigQueryClient-backed BigQueryAdapter on the patch seam. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ad1.4: Add cli-ops prune-existing section + 5-surface parity test US-004 of #105. Documents the `signalforge prune-existing` subcommand in docs/cli-ops.md (synopsis, flag-reference table, four-tier exit codes, skipped-test summary stderr shape, read-only/no-`--write` note, the unified-diff-against-your-file framing + cosmetic-reformatting caveat, the no-grading -> never-flagged note, and a worked example). Bumps the subcommand count from four to six. Adds tests/cli/test_5_surface_parity_prune_existing.py mirroring test_5_surface_parity_select.py: asserts the flag set (--schema, --scope, --sample-strategy, --dry-run, prune-existing) and the read-only intent appear consistently across argparse help, prune_existing.py help/docstrings, docs/cli-ops.md, and the plan's DEC list. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ad1.6: Quality gate — fix bugs from code review - prune engine: short-circuit empty candidate before warehouse contact (all-skipped schema.yml no longer materialises a temp table for zero tests); regression test on the default materialised+sample path. - prune-existing: scrub \n/\r/\t in the --verbose skipped-test report so an operator YAML value can't inject a fake bullet line (mirrors format_batch_summary's control-char scrub). - test: assert the exact kept/dropped mix (7/3) in the --format json test so a prune keep-everything/drop-everything regression fails loudly (testing-signal.md). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ad1.6: Quality gate — fix CodeRabbit finding (plan ref) Correct the prune-existing subprocess-smoke docstring to cite plans/super/105-prune-existing-cli.md (was 104). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ad1.7: Patterns & Memory — record #105 conventions - ingest-layer.md: prune-existing shipped (was deferred); record the no-bespoke-wrappers (DEC-006), --mode-inert (DEC-002), read-only (DEC-003/004) findings. - cli-layer.md: bare-name resolver hoist done in #105; new section on "library errors already mapped -> no per-class wrappers" + no-LLM-stage flag-audit + read-only conventions. - prune-engine.md: empty-candidate short-circuit (no warehouse contact). - CLAUDE.md: add #105 to shipped issues + CLI surface. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #105: Address PR review feedback (Copilot + CodeRabbit) - prune_existing.py: refresh stale module docstring (no longer a "stub"; the orchestrator is implemented) and note the no-bespoke-wrappers seam. - prune/engine.py: call _maybe_emit_kept_rate_warning at the new empty-candidate return site (no-op via total==0 early-return) to honour the "every return site" contract (#51). - docs/cli-ops.md: subcommand count six -> five; avoid a line starting with "#104" (heading parse); trim leading spaces inside the Remediation inline code span. - plans/super/105-prune-existing-cli.md: markdownlint — language on the fenced block, avoid "#104" line start. --profiles-dir non-canonicalisation is intentional (mirrors generate; dbt places profiles.yml outside the project) — documented as a false positive on the thread. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The external-test ingest layer (#104) and the prune-existing CLI (#105) only appeared in their own ops pages; the README — the published home page and product pitch — never mentioned them, and ingest-ops.md still said the CLI didn't exist. README: - "Why this exists" / "What it does": SignalForge now prunes ANY generator's tests, not just its own drafts. - "How it works": add the second (no-LLM) entry path diagram. - New "Prune the tests you already have" cookbook with a runnable example + an availability callout (prune-existing is the v0.2 dev line, not yet on the v0.1 PyPI release; install-from-source command). - CLI list: four -> five subcommands; prune-existing flags; v0.2 marker. - Roadmap: note prune-existing on the v0.2 row. Docs: - ingest-ops.md: fix stale "no prune-existing CLI yet" wording (it shipped in #105); cross-link the CLI reference. - cli-ops.md: cross-link the supported/skipped test taxonomy in ingest-ops.md. Verified: mkdocs build clean; all in-page anchors resolve. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* #96: Python 3.13 compatibility for the path-safety layer Python 3.13 (gh-108958) changed `Path.resolve()` to raise `OSError(errno.ELOOP)` instead of `RuntimeError` on cyclic symlinks, and made `strict=False` resolution stop raising on a cycle entirely (it returns a best-effort unresolved path). The three loop-guard sites caught only `RuntimeError`, so on 3.13 a symlink cycle either escaped as a raw OSError or slipped silently past the containment check. Fix every loop-guard site to resolve the input path `strict=True` first (falling back to `strict=False` only for a genuinely missing target) and to catch both `RuntimeError` (<= 3.12) and `OSError(errno.ELOOP)` (>= 3.13): - `signalforge/_common/path_safety.py` - `signalforge/manifest/loader.py::_canonicalise_path` - `signalforge/demo/__init__.py::copy_demo` The demo test's symlink-cycle case no longer skips on Linux/WSL2 and now asserts the cause is `RuntimeError | OSError`. Re-add 3.13 to the CI matrix as the ceiling; flip the codecov upload gate 3.12 -> 3.13. Update the deferral prose in CLAUDE.md, CONTRIBUTING.md, python-build.md, and ci-supply-chain.md. Validated on 3.11 / 3.12 / 3.13: 1923 passed; ruff + format + pyright clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #96: keep the manifest non-dict-root guard live (pyright unreachable fix) `raw: dict[str, Any] = json.load(fh)` asserted a type json.load doesn't guarantee, so pyright treated the `isinstance(loaded, dict)` root-shape guard as dead code. Load as `Any`, narrow to `dict[str, Any]` after the check so the guard stays live. Add a test (`non_dict_root.json` — valid JSON array root) that exercises the now-reachable branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #96: narrow strict-resolve fallback to missing-target only (PR review) Copilot review: the `except OSError` fallback to `resolve(strict=False)` was too broad — it downgraded ANY non-ELOOP OSError (e.g. PermissionError) to best-effort resolution, contradicting the "fallback only for a missing target" intent and potentially weakening the containment check. Narrow the fallback to `FileNotFoundError` / `NotADirectoryError`; re-raise every other OSError. Applied to all three loop-guard sites (_common/path_safety, manifest/loader, demo). Tighten the demo docstring to state the strict-then-fallback behaviour explicitly. Add a regression test asserting a PermissionError on the input path propagates rather than being swallowed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #96: close codecov patch-coverage gaps on the path-safety fix The codecov upload runs from the 3.13 leg, so the `except RuntimeError` arms (which only execute on Python <= 3.12) and the new non-ELOOP re-raise arms showed as uncovered. Address honestly: - Add regression tests for the non-ELOOP re-raise arms (PermissionError on the input path AND project_dir for path_safety; input path for loader; dest for demo) — these encode the Copilot fix and are reachable on every interpreter. - `# pragma: no cover` the `except RuntimeError` arms (<=3.12-only cycle signal; exercised on the 3.11/3.12 CI legs, structurally unreachable on the 3.13 upload leg) and loader's project_dir handler block (defensive — `load()` passes an already-resolved project_dir, mirroring the tested _common.path_safety helper). Patch coverage on the touched files is now 100% on the 3.13 upload leg. Validated on 3.11 / 3.12 / 3.13: 1928 passed; ruff + format + pyright clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
#110) The gated markers (bigquery / anthropic / cli_subprocess / e2e / wheel_smoke) carry no Python-version pin and run on whatever interpreter `uv run` resolves; no CI job runs them. Following #96 (3.13 is now the matrix ceiling), document that maintainers should run the packaging/entry-point markers (wheel_smoke, cli_subprocess) under `uv run --python 3.13`, while the interpreter-invariant live-service markers stay single-version to avoid multiplying paid API calls. CONTRIBUTING.md § Pre-release coverage audit + testing-signal.md § Known gap. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test: close 0.2.0 patch-coverage gaps in ingest + prune-existing Brings ingest/parser.py and ingest/reader.py to 100% and cli/prune_existing.py to 96% on the default (non-gated) test run, raising the 0.2.0 release PR's codecov patch coverage from 92.44%. - parser: ref()/source() non-quoted + arity edges, non-dict bodies, multi-key + non-str/dict entries, model-level relationships skip - reader: path-containment failure, unreadable file (POSIX, non-root), non-dict / nameless column entries ignored - prune-existing: walk-up project-dir resolution (success + failure), --no-color env mutation, empty skipped-report guard Residual: prune_existing.py:270 (real WarehouseAdapter.from_profile, gated-only) and :447-454 (--profiles-dir resolve error, flaky to trigger) are documented coverage-policy exclusions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: assert diff.json sidecar durability in prune-existing success tests Addresses CodeRabbit review on #112: the three new success-path tests now also assert the default-on .signalforge/diff.json sidecar is written (DEC-016 e2e convention — exit 0 + no traceback + durable sidecar). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… (#113) #3 (ingest reader): enforce the DEC-005 size cap from stat().st_size BEFORE reading file bytes for a Path input, so an oversize schema.yml is rejected without first being slurped into memory. The post-resolve length check stays as the str-path enforcement + backstop. #2 (prune-existing --dry-run docs): correct cli-ops.md — --dry-run suppresses the .signalforge/diff.json sidecar, but the fail-closed .signalforge/prune.jsonl audit is STILL written (every prune run leaves a durable receipt — the cross-stage invariant; mirrors generate). Pinned by strengthening test_dry_run_writes_no_sidecar to assert prune.jsonl is present. (#1, env-var restore, is left as-is — DEC-023 'mutate, don't restore' is the deliberate, consistent pattern across all subcommands.) reader.py back to 100% coverage; full suite 1945 passed. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs: README install instructions for the live PyPI release (#102) signalforge-dbt 0.1.0 is on PyPI, so lead the install with `pip install signalforge-dbt` instead of the clone+uv-sync path. - Status line: "Not yet on PyPI" → "Live on PyPI — pip install signalforge-dbt". - Quick start §1: pip install first + `signalforge --version` verify; note the distribution-vs-import name; add `uv tool install` / `pipx` as the isolated-CLI alternative; demote the clone + `uv sync --dev` path to a contributing pointer at CONTRIBUTING.md. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: release 0.2.0 (#111) * 93: silence pydantic UserWarning for LLMRequest.schema field shadow (#94) * 93: silence pydantic UserWarning for LLMRequest.schema field shadow The `schema` field name on `LLMRequest` is part of the documented audit-log contract (safety-layer.md DEC-014), but it shadows Pydantic v1's deprecated `BaseModel.schema` method, triggering a UserWarning at class-creation time that surfaces on every fresh import / CLI invocation. Wrap the class definition in `warnings.catch_warnings()` with a targeted filter — narrowest blast radius; no global filter mutation, the contract stays unchanged. Adds a subprocess-import regression test under `-W error::UserWarning` so future shape changes can't silently reintroduce it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * 93: document Pydantic field-name-shadow suppression pattern Compounding update from PR #94: capture the `catch_warnings()` scope-to-class convention so the next field-name-shadow case (audit-log contract + Pydantic v1 BaseModel attribute collision) doesn't get "fixed" by renaming or a global filter. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * 93: address PR review feedback - models.py: relax warning-filter message from exact literal to regex (`Field name "schema".*shadows.*`) so future Pydantic message-wording drift still gets caught. Drops the experimental `module=` qualifier (over-narrowing — filter stopped matching the live warning). - test_models.py: add `timeout=10` to the subprocess import smoke, matching the precedent in `tests/cli/test_subprocess_smoke.py` so a pathological import hang fails fast instead of stalling the suite. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: reconcile README + docs with current v0.1 codebase (#90) Audited the root README and docs/ ops files against the shipped code: - demo profiles.yml: add maximum_bytes_billed: 1000000000 so the README quick start actually works — the demo's materialised-sample CTAS over the full ~2.27M-row bikeshare_trips source exceeds the adapter's 100 MB default cap. Parity-test docstring updated to document the new rewrite. - README: add the kept-uncertain tier to the expected-output section (four-tier taxonomy was undocumented), complete the generate/lint flag lists in the CLI section, mention lint --model. - docs/audits.md: fix stale audit_schema_version values (safety 1 -> 3, prune 1 -> 2) bumped by issues #54/#55. - docs/e2e-smoke-test.md: fix stale cross-reference claiming the README quick start does a manual profile rewrite — it uses signalforge init-demo. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * build: migrate dev tooling to uv + widen Python matrix to 3.11/3.12 (#95) * build: migrate dev tooling to uv + widen Python matrix to 3.11/3.12/3.13 Mirrors clauditor's build setup. uv becomes the canonical dev install (`uv sync --dev`); `uv.lock` is committed; CI runs uv-managed steps across a 3-Python matrix. `pip install -e ".[dev]"` is retained for back-compat — `[project.optional-dependencies].dev` and the new `[dependency-groups].dev` are kept in sync. - `pyproject.toml`: add `[dependency-groups].dev` (PEP 735); only delta vs the pip-extra is `build>=1.2,<2` for the wheel_smoke marker. - `.github/workflows/ci.yml`: replace setup-python + pip install with `astral-sh/setup-uv@v8.1.0` (SHA-pinned per ci-supply-chain.md) + `uv sync --dev`; wrap lint/test steps with `uv run`. Matrix runs ruff + pytest on all three versions; pyright gated to the matrix floor (3.11, matching pyright.pythonVersion); codecov upload gated to the matrix ceiling (3.13) so coverage doesn't double-upload. - `.github/workflows/publish.yml`: `python -m build` → `uv build` in both publish-testpypi and publish-pypi jobs. - `.gitignore`: drop `uv.lock` from the ignore list (bark scaffolding artifact comment is stale — the canonical lock is committed now). - `CLAUDE.md` / `CONTRIBUTING.md` / `README.md` / `docs/cli-ops.md` / `docs/codecov-ops.md` / `docs/manifest-loader-ops.md`: switch copy-pasteable dev commands to `uv sync --dev` + `uv run …`. - `.claude/rules/python-build.md`: replace "Editable install (zsh-safe)" with a uv-managed dev environment section; refresh issue-#46 floor-pinning to describe the matrix. - `.claude/rules/ci-supply-chain.md`: add `astral-sh/setup-uv` to the SHA-pinning example list; graduate DEC-003 from single-Python to a 3-Python matrix with the pyright / codecov gating rationale. Local validation: `uv run ruff check .` / `uv run ruff format --check .` / `uv run pyright` / `uv run pytest` all clean under Python 3.13; `uv run --python 3.11 pytest --no-cov` and `--python 3.12` both 1830/1830 pass. wheel_smoke passes under `uv run`. (Six pre-existing WSL2-only symlink-loop test failures on 3.13 unaffected; pass in GHA.) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: drop 3.13 from matrix; document path-safety follow-up CI exposed six 3.13-only test failures under tests/_common/, tests/diff/, tests/grade/, tests/manifest/. Root cause: Python 3.13 changed Path.resolve() to raise OSError(errno.ELOOP) on cyclic symlinks instead of RuntimeError. SignalForge's _canonicalise_path and the manifest loader's path canonicalisation catch RuntimeError only — on 3.13 the cycle now escapes as OSError or short-circuits to a different typed error before the loop check fires. Fixing it touches production code (signalforge/_common/path_safety.py and signalforge/manifest/loader.py) plus four test fixtures, which is substantial scope creep for a tooling-migration PR. Narrow the matrix to 3.11 / 3.12 (both green: 1830 passed locally and in CI) and track the 3.13 work in a follow-up issue. - ci.yml: matrix python-version: ["3.11", "3.12"]; codecov-upload gate flips from 3.13 → 3.12 (matrix ceiling). Comment block records the reason and points at the follow-up. - CLAUDE.md / CONTRIBUTING.md / docs/cli-ops.md / python-build.md / ci-supply-chain.md: every "3.11 / 3.12 / 3.13" reference now reads "3.11 / 3.12"; ci-supply-chain.md and python-build.md add a "3.13 is deferred" paragraph naming the catch-site fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: address CodeRabbit + Copilot review feedback Five fixes from PR #95 review (all real issues, no false positives): - `.claude/rules/ci-supply-chain.md`: codecov-gate example said `matrix.python-version == '3.13'` (stale from the 3.13 matrix); current CI gates on `'3.12'`. Updated and noted the flip-back when issue #96 lands. [coderabbit/copilot — same finding, threads 1+3] - `.github/workflows/ci.yml`: `actions/checkout` now sets `persist-credentials: false` — CI is read-only; least-privilege for the GITHUB_TOKEN. [coderabbit — thread 4] - `.github/workflows/publish.yml`: same `persist-credentials: false` on both `Checkout` steps; same justification (build job doesn't push to git). [coderabbit — thread 5] - `.github/workflows/publish.yml`: `astral-sh/setup-uv` now pins `python-version: "3.11"` on both jobs for release-build reproducibility. Without it, `uv build` picks whatever `ubuntu-latest` happens to ship. [copilot — thread 2] - `docs/cli-ops.md`: clarify that the `pip install -e ".[dev]"` fallback omits the `build` package (uv-only delta in `[dependency-groups].dev` powering wheel_smoke). Pip users shouldn't assume identical tooling coverage. [coderabbit — thread 6] Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: publish MkDocs Material site to GitHub Pages on push to main (#97) * docs: publish MkDocs Material site to GitHub Pages on push to main Mirrors clauditor's docs-publishing setup. The published site at https://wjduenow.github.io/SignalForge/ redeploys on every push to main; the gh-pages branch is generated, not authored. - `mkdocs.yml`: Material theme with light/dark toggle, search + include-markdown plugins, mermaid via pymdownx.superfences, exclude_docs: research/ (internal analysis stays off the site), edit_uri: edit/dev/docs/ (doc edits land on dev like every other PR; main is the release line). Nav: Home + CLI Reference + Pipeline Stages (manifest/warehouse/safety/draft/prune/grade/diff) + Audits & Sidecars + e2e + coverage. - `docs/index.md`: 4-line include-markdown stub pulling in the root README. Mirrors clauditor verbatim — README stays the canonical authored home doc; site home auto-syncs on every build. - `.github/workflows/ci.yml`: new `docs:` job gated on `github.ref == 'refs/heads/main' && github.event_name == 'push'` with job-scoped `permissions: contents: write` (workflow default stays `contents: read`). Uses pinned `astral-sh/setup-uv@v8.1.0` with `python-version: "3.11"` (matches publish.yml floor); runs `uv run mkdocs gh-deploy --force --no-history`. Deliberately does NOT set `persist-credentials: false` on the checkout step — `gh-deploy` needs the persisted GITHUB_TOKEN to push to gh-pages. - `pyproject.toml`: add `mkdocs-material>=9.0` and `mkdocs-include-markdown-plugin>=6.0` to `[dependency-groups].dev` only. Pip contributors don't need to build docs; this stays a uv-only delta (alongside `build` for wheel_smoke). - `.gitignore`: ignore `site/` (mkdocs build output). - `README.md`: docs badge top-of-file. - `CLAUDE.md` / `CONTRIBUTING.md`: one-liner pointing at the published site and the deploy contract. - `.claude/rules/docs-publishing.md` (new): full deploy contract — trigger rule, include-markdown pattern, edit_uri choice, persist-credentials exception, exclude_docs convention, local build recipe, when to update mkdocs.yml vs the docs themselves, first-time GH Pages setup the maintainer does once. Local validation: `uv run mkdocs build` succeeds (warnings on repo-internal links to plans/super/ and .claude/rules/ are expected and matched in clauditor — those aren't part of the published site). `uv run ruff check .` / `ruff format --check .` clean. 1823/1823 tests pass on 3.13 (6 pre-existing #96 failures unrelated). Post-merge maintainer step (once): Settings → Pages → source = "Deploy from a branch", branch = gh-pages, folder = / (root). The first push to main after this merges lands the gh-pages branch; toggle Pages on right after. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: address PR #97 review — split docs dep group + PR-time build gate Two Copilot suggestions, both real wins: - Extract `[dependency-groups].docs` (mkdocs-material + mkdocs-include-markdown-plugin only). Both CI docs jobs now `uv run --only-group docs ...` so they pull just MkDocs + plugins, not the heavy dev set (dbt-core, pyright, pytest). `dev` includes `docs` via `{include-group = "docs"}` so `uv sync --dev` still gives contributors everything. [thread: ci.yml deploy pulled --dev] - Add a read-only `docs-build` job that runs on every PR + push (`uv run --only-group docs mkdocs build`, no write perms). Catches a broken mkdocs.yml / plugin config / include-markdown syntax error at PR time instead of silently merging and only failing the post-merge gh-pages deploy. [thread: no pre-merge docs build signal] The third thread (Home page "Edit this page" link points at the docs/index.md include stub rather than README.md) is an accepted limitation of the include-markdown pattern — Material has no per-page edit-button hide without a template override, and clauditor's identical setup accepts it. The stub a contributor lands on literally documents where the real content lives. Documented in the review summary; no code change. `.claude/rules/docs-publishing.md` updated: two-job structure, the docs dependency-group split, and the "new plugin goes in docs not dev" convention. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * release-manager: cut TestPyPI from dev, PyPI from main + uv refresh Branch discipline: test releases are now cut from `dev` (was "any branch") and full releases from `main` (unchanged). The publish.yml prerelease-flag routing is unchanged — it's the mechanism; the branch is the maintainer discipline the skill enforces in pre-flight. dev is the in-development line feeding TestPyPI; main is the released line feeding PyPI. Also refreshes the skill for the uv migration (PRs #95/#97): - validation command → `uv sync --dev && uv run ruff/pyright/pytest` - `python -m build` → `uv build` (both build blocks + PR-body string) - allowed-tools gains `Bash(uv *)`; compatibility frontmatter updated - left the TestPyPI clean-room `pip install` smoke test as-is (it deliberately installs the published artifact in a throwaway venv) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: bump to 0.1.0rc3 for test release * release-manager: pin clean-room smoke test to Python >=3.11 The Step 5 TestPyPI smoke test used a bare `python -m venv`, which picks up whatever `python3` is on PATH — on a host where that's 3.10, pip silently ignores the new release (requires-python >=3.11) and only the pre-floor 0.1.0rc1 installs, producing a confusing "Could not find a version that satisfies the requirement" right after a green publish. Surfaced cutting v0.1.0rc3: the first clean-room attempt used the system 3.10 and rejected rc3/rc2 as "Requires-Python >=3.11". Fix: provision the interpreter explicitly via `uv venv --python 3.11` (the requires-python floor — verifies installability on the minimum supported version; uv auto-fetches 3.11 if absent) and `uv pip install --python <venv>`. Also documents how to distinguish this from the separate TestPyPI Fastly-cache propagation lag. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: disable setup-uv cache (enable-cache: false) on all steps CodeRabbit flagged on PR #98: setup-uv's default persists the uv cache to the GitHub Actions Cache. On a public repo, pull_request runs share a cache scope with the base branch, so a fork PR can poison an entry a later trusted run restores — a cross-trust-boundary cache-poisoning vector. Set `enable-cache: false` explicitly on every setup-uv step: ci.yml (lint-test, docs-build, docs-deploy) + publish.yml (testpypi, pypi). Runs are short enough that the lost cache is negligible; the publish/deploy steps especially must never restore a poisoned cache into a published artifact or the gh-pages deploy. Documents the convention in .claude/rules/ci-supply-chain.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: begin 0.2.0.dev0 (#100) * docs: README install instructions for the live PyPI release signalforge-dbt 0.1.0 is on PyPI, so lead the install with `pip install signalforge-dbt` instead of the clone+uv-sync path. - Status line: "Not yet on PyPI" → "Live on PyPI — pip install signalforge-dbt". - Quick start §1: pip install first + `signalforge --version` verify; note the distribution-vs-import name; add `uv tool install` / `pipx` as the isolated-CLI alternative; demote the clone + `uv sync --dev` path to a contributing pointer at CONTRIBUTING.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #104: ingest external dbt schema.yml tests into CandidateSchema (#106) * Add super plan for #104: ingest external dbt schema.yml tests Library-only seam (signalforge.ingest.read_schema -> IngestResult). prune-existing CLI subcommand split to fast-follow #105. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Mark #104 plan published (PR #106) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Devolve #104 plan to beads (epic bd_1-scaffolding-ky3, 8 tasks) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ky3.1: scaffold signalforge.ingest package + IngestError hierarchy + exit-code lockstep US-001 of issue #104. Adds the signalforge.ingest subpackage scaffold and its six-class typed-error hierarchy (DEC-001), wired into the CLI four-tier exit-code taxonomy and the 7th AST scan. - src/signalforge/ingest/__init__.py: package init re-exporting the IngestError surface. - src/signalforge/ingest/errors.py: IngestError base (remediation + ↳ Remediation rendering, repr-safe values) plus five concretes — IngestSchemaNotFoundError / IngestSchemaParseError / IngestSchemaTooLargeError (tier 1) and IngestModelNotFoundError / IngestAnchorContractError (tier 2). - cli/_helpers.py: every concrete mapped in _EXCEPTION_TO_EXIT_CODE at its tier; IngestError registered as fallback tier-1 (dual-registration). - tests/test_audit_completeness.py: IngestError added to _EXCEPTION_MAPPING_EXCLUDED_BASES; Scan-7 module count bumped 10 -> 11. - tests/cli/test_exit_codes.py: synthetic-construction branches for the (size, limit) and (violations,) constructor shapes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ky3.2: typed ingest result models (IngestResult, SkippedTest, SkipReason) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ky3.4: anchor-contract validator (fail-loud, collect-all) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ky3.3: dbt test-entry parser (pure mapping) Pure parse_test_entry(entry, *, column) mapping a single dbt schema.yml test entry to a CandidateTest or a structured SkippedTest. Handles bare strings, single-key dicts with inline and arguments:-nested args (DEC-006), config-key tolerance, malformed-supported skips, custom / namespaced skips, and best-effort ref()/source() unwrap in relationships.to (DEC-009). No logging, no I/O, not part of the public surface. 19-test matrix added (TDD). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ky3.5: read_schema orchestrator Implement the public ingest entry point read_schema(schema, model, *, project_dir=None) -> IngestResult tying together the parser, anchor validator, and result models. Steps: str-vs-Path input contract (Path = file, symlink-hardened via _common.path_safety; str = raw YAML), size-cap before yaml.safe_load (DEC-005), model-block selection by name (DEC-006), union+dedupe tests/data_tests (DEC-008), description="" default (DEC-010), whole-file anchor check. No logging (stage-0 reader). Re-export read_schema from the package __init__ as the public surface. Adds the dbt-codegen-shaped fixture and 13 tests: happy path (supported + skipped tests, dedupe, ref() unwrap, both test keys), str-content input, and each error path (model-not-found, malformed YAML, oversize, anchor violation, missing path), plus the disabled-prune acceptance bonus. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ky3.6: docs/ingest-ops.md operational reference + mkdocs nav Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ky3.7: Quality gate — fix bugs from code review - parser: model-level supported test (column=None) now routes to a structured malformed skip instead of raising a Pydantic ValidationError out of read_schema. - reader: accepted_values dedupe key now sorts values so reordered value sets collapse (DEC-008 "sorted-args" contract). - parser: clarify _extract_args docstring for non-dict `arguments`. - tests: regression coverage for both fixes (+3 ingest tests). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ky3.8: Patterns & Memory — ingest-layer rule + CLAUDE.md + cli-layer count - New .claude/rules/ingest-layer.md distilling the #104 conventions (str-vs-Path contract, fail-loud anchor vs skip-record taxonomy, no-logging stage-0, safe-YAML size cap, dbt syntax tolerance, 11th errors.py exit-code lockstep, deferred #105 CLI). - CLAUDE.md: #104 shipped-issue entry + public-API surface entry. - cli-layer.md: scan-7 count 10->11; IngestError joins DemoError as a span-tier base (frozenset-only, no fallback) — and aligned _helpers.py by removing the IngestError tier-1 fallback to match that precedent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #104: Address PR review feedback (Copilot) - errors.py: correct IngestError docstring — base is frozenset-only (no fallback tier), concretes span tiers 1 and 2 like DemoError. - __init__.py: refresh module docstring — the full library seam (reader, parser, anchor, models) ships in #104, not "typed-error surface only". - parser.py: _extract_args now drops the structural `arguments` key from inline args so a non-mapping `arguments:` can't leak (+ regression test). - docs/ingest-ops.md: fix usage snippet to use WarehouseAdapter.from_profile (the public factory; there is no top-level from_profile) and pass an un-entered adapter to prune_tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #105: signalforge prune-existing CLI subcommand (#107) * Add super plan for #105: signalforge prune-existing CLI subcommand Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Mark plan phase published (PR #107) * Mark plan devolved; record beads manifest (epic bd_1-scaffolding-ad1) * bd_1-scaffolding-ad1.1: Hoist bare-name model resolver to _helpers #105 US-001 / DEC-008. Move the bare-name model resolver from lint.py to signalforge.cli._helpers as _resolve_model_by_key(manifest, key) -> Model so the future prune-existing subcommand shares one resolver rather than copy-pasting the body. lint.py keeps a thin _resolve_model_for_lint wrapper that delegates. Identical behaviour: unique_id/file-path via Manifest.get_model; bare-name via iter_models with multi-match disambiguation + disabled-model exclusion + the same ModelNotFoundError messages. No behaviour change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ad1.2: Add prune-existing add_parser + stub handler US-002 of #105 — contract surface (DEC-001, DEC-002, DEC-003). Adds src/signalforge/cli/prune_existing.py with add_parser registering the prune-existing subcommand and full flag set (positional <model>, required --schema, --project-dir/--manifest/--profiles-dir, --scope/--sample-strategy/--format choice flags, --dry-run, and the --quiet/--verbose/--no-color observability triad). Drops --mode/--write/ --min-score/--select/--estimate per DEC-002/DEC-003. cmd_prune_existing is a stub returning 0 (full ingest->prune->diff body lands in US-003). Registered in signalforge.cli.__init__._build_parser. Help strings are surface 1 of the 5-surface parity contract. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ad1.5: Add prune-existing --help subprocess smoke Extends the @pytest.mark.cli_subprocess gate to the prune-existing subcommand (#105 US-005), mirroring the existing generate/lint/version/ init-demo --help smokes. Asserts returncode 0, the unique --schema flag in stdout, and the no-traceback floor on stderr. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ad1.3: Implement cmd_prune_existing orchestrator (ingest -> prune -> diff) US-003 of #105. Replaces the US-002 stub with the real ingest -> prune -> diff orchestrator (no draft, no grade, no LLM call), inside one try/except Exception boundary (DEC-016 — no traceback ever leaks). - Resolve project_dir (mirror generate: --project-dir absolute assertion, walk-up default); --no-color/--profiles-dir env mutation (DEC-023). - Resolve <model> via _resolve_model_by_key (bare-name / unique_id / file-path). Canonicalise --schema via canonicalise_user_path (DEC-005). - read_schema(Path, model, project_dir=...) so ingest typed errors fire and route via the pre-registered exit-code table (DEC-006 — no bespoke CliPruneExisting* wrappers). - Skipped-test report to stderr: grouped summary + --verbose per-item detail, suppressed by --quiet (DEC-007). print_stderr, not _LOGGER. - PruneConfig --scope/--sample-strategy overrides via model_validate (DEC-002). _make_warehouse_adapter test seam (DEC-009); prune_tests owns the with-adapter block (un-entered adapter). - Feed the external schema.yml text as render_diff existing_schema with grading_report=None -> kept/kept-uncertain/dropped, never flagged (DEC-004); --dry-run -> write_sidecar=False. render_to_text -> stdout. - 3-stage progress (1/3 ingest, 2/3 prune, 3/3 diff) via the existing emit_progress_* helpers, generalised with a total= kwarg (default 5). Adds an Austin-aligned schema.yml fixture whose four supported test types reference real stg_bikeshare_trips columns plus two unsupported tests for the skip path. Tests cover happy path, skipped report (+ --verbose + --quiet), each error-path exit-code tier with a no-traceback floor, --dry-run no-disk, --format json, and bare-name vs unique_id resolution, via a FakeBigQueryClient-backed BigQueryAdapter on the patch seam. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ad1.4: Add cli-ops prune-existing section + 5-surface parity test US-004 of #105. Documents the `signalforge prune-existing` subcommand in docs/cli-ops.md (synopsis, flag-reference table, four-tier exit codes, skipped-test summary stderr shape, read-only/no-`--write` note, the unified-diff-against-your-file framing + cosmetic-reformatting caveat, the no-grading -> never-flagged note, and a worked example). Bumps the subcommand count from four to six. Adds tests/cli/test_5_surface_parity_prune_existing.py mirroring test_5_surface_parity_select.py: asserts the flag set (--schema, --scope, --sample-strategy, --dry-run, prune-existing) and the read-only intent appear consistently across argparse help, prune_existing.py help/docstrings, docs/cli-ops.md, and the plan's DEC list. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ad1.6: Quality gate — fix bugs from code review - prune engine: short-circuit empty candidate before warehouse contact (all-skipped schema.yml no longer materialises a temp table for zero tests); regression test on the default materialised+sample path. - prune-existing: scrub \n/\r/\t in the --verbose skipped-test report so an operator YAML value can't inject a fake bullet line (mirrors format_batch_summary's control-char scrub). - test: assert the exact kept/dropped mix (7/3) in the --format json test so a prune keep-everything/drop-everything regression fails loudly (testing-signal.md). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ad1.6: Quality gate — fix CodeRabbit finding (plan ref) Correct the prune-existing subprocess-smoke docstring to cite plans/super/105-prune-existing-cli.md (was 104). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ad1.7: Patterns & Memory — record #105 conventions - ingest-layer.md: prune-existing shipped (was deferred); record the no-bespoke-wrappers (DEC-006), --mode-inert (DEC-002), read-only (DEC-003/004) findings. - cli-layer.md: bare-name resolver hoist done in #105; new section on "library errors already mapped -> no per-class wrappers" + no-LLM-stage flag-audit + read-only conventions. - prune-engine.md: empty-candidate short-circuit (no warehouse contact). - CLAUDE.md: add #105 to shipped issues + CLI surface. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #105: Address PR review feedback (Copilot + CodeRabbit) - prune_existing.py: refresh stale module docstring (no longer a "stub"; the orchestrator is implemented) and note the no-bespoke-wrappers seam. - prune/engine.py: call _maybe_emit_kept_rate_warning at the new empty-candidate return site (no-op via total==0 early-return) to honour the "every return site" contract (#51). - docs/cli-ops.md: subcommand count six -> five; avoid a line starting with "#104" (heading parse); trim leading spaces inside the Remediation inline code span. - plans/super/105-prune-existing-cli.md: markdownlint — language on the fenced block, avoid "#104" line start. --profiles-dir non-canonicalisation is intentional (mirrors generate; dbt places profiles.yml outside the project) — documented as a false positive on the thread. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: surface ingest + prune-existing (#104/#105) in user docs (#108) The external-test ingest layer (#104) and the prune-existing CLI (#105) only appeared in their own ops pages; the README — the published home page and product pitch — never mentioned them, and ingest-ops.md still said the CLI didn't exist. README: - "Why this exists" / "What it does": SignalForge now prunes ANY generator's tests, not just its own drafts. - "How it works": add the second (no-LLM) entry path diagram. - New "Prune the tests you already have" cookbook with a runnable example + an availability callout (prune-existing is the v0.2 dev line, not yet on the v0.1 PyPI release; install-from-source command). - CLI list: four -> five subcommands; prune-existing flags; v0.2 marker. - Roadmap: note prune-existing on the v0.2 row. Docs: - ingest-ops.md: fix stale "no prune-existing CLI yet" wording (it shipped in #105); cross-link the CLI reference. - cli-ops.md: cross-link the supported/skipped test taxonomy in ingest-ops.md. Verified: mkdocs build clean; all in-page anchors resolve. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #96: Python 3.13 compatibility for the path-safety layer (#109) * #96: Python 3.13 compatibility for the path-safety layer Python 3.13 (gh-108958) changed `Path.resolve()` to raise `OSError(errno.ELOOP)` instead of `RuntimeError` on cyclic symlinks, and made `strict=False` resolution stop raising on a cycle entirely (it returns a best-effort unresolved path). The three loop-guard sites caught only `RuntimeError`, so on 3.13 a symlink cycle either escaped as a raw OSError or slipped silently past the containment check. Fix every loop-guard site to resolve the input path `strict=True` first (falling back to `strict=False` only for a genuinely missing target) and to catch both `RuntimeError` (<= 3.12) and `OSError(errno.ELOOP)` (>= 3.13): - `signalforge/_common/path_safety.py` - `signalforge/manifest/loader.py::_canonicalise_path` - `signalforge/demo/__init__.py::copy_demo` The demo test's symlink-cycle case no longer skips on Linux/WSL2 and now asserts the cause is `RuntimeError | OSError`. Re-add 3.13 to the CI matrix as the ceiling; flip the codecov upload gate 3.12 -> 3.13. Update the deferral prose in CLAUDE.md, CONTRIBUTING.md, python-build.md, and ci-supply-chain.md. Validated on 3.11 / 3.12 / 3.13: 1923 passed; ruff + format + pyright clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #96: keep the manifest non-dict-root guard live (pyright unreachable fix) `raw: dict[str, Any] = json.load(fh)` asserted a type json.load doesn't guarantee, so pyright treated the `isinstance(loaded, dict)` root-shape guard as dead code. Load as `Any`, narrow to `dict[str, Any]` after the check so the guard stays live. Add a test (`non_dict_root.json` — valid JSON array root) that exercises the now-reachable branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #96: narrow strict-resolve fallback to missing-target only (PR review) Copilot review: the `except OSError` fallback to `resolve(strict=False)` was too broad — it downgraded ANY non-ELOOP OSError (e.g. PermissionError) to best-effort resolution, contradicting the "fallback only for a missing target" intent and potentially weakening the containment check. Narrow the fallback to `FileNotFoundError` / `NotADirectoryError`; re-raise every other OSError. Applied to all three loop-guard sites (_common/path_safety, manifest/loader, demo). Tighten the demo docstring to state the strict-then-fallback behaviour explicitly. Add a regression test asserting a PermissionError on the input path propagates rather than being swallowed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #96: close codecov patch-coverage gaps on the path-safety fix The codecov upload runs from the 3.13 leg, so the `except RuntimeError` arms (which only execute on Python <= 3.12) and the new non-ELOOP re-raise arms showed as uncovered. Address honestly: - Add regression tests for the non-ELOOP re-raise arms (PermissionError on the input path AND project_dir for path_safety; input path for loader; dest for demo) — these encode the Copilot fix and are reachable on every interpreter. - `# pragma: no cover` the `except RuntimeError` arms (<=3.12-only cycle signal; exercised on the 3.11/3.12 CI legs, structurally unreachable on the 3.13 upload leg) and loader's project_dir handler block (defensive — `load()` passes an already-resolved project_dir, mirroring the tested _common.path_safety helper). Patch coverage on the touched files is now 100% on the 3.13 upload leg. Validated on 3.11 / 3.12 / 3.13: 1928 passed; ruff + format + pyright clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: steer pre-release gated-marker runs to the matrix ceiling (3.13) (#110) The gated markers (bigquery / anthropic / cli_subprocess / e2e / wheel_smoke) carry no Python-version pin and run on whatever interpreter `uv run` resolves; no CI job runs them. Following #96 (3.13 is now the matrix ceiling), document that maintainers should run the packaging/entry-point markers (wheel_smoke, cli_subprocess) under `uv run --python 3.13`, while the interpreter-invariant live-service markers stay single-version to avoid multiplying paid API calls. CONTRIBUTING.md § Pre-release coverage audit + testing-signal.md § Known gap. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: release 0.2.0 * test: close 0.2.0 patch-coverage gaps in ingest + prune-existing (#112) * test: close 0.2.0 patch-coverage gaps in ingest + prune-existing Brings ingest/parser.py and ingest/reader.py to 100% and cli/prune_existing.py to 96% on the default (non-gated) test run, raising the 0.2.0 release PR's codecov patch coverage from 92.44%. - parser: ref()/source() non-quoted + arity edges, non-dict bodies, multi-key + non-str/dict entries, model-level relationships skip - reader: path-containment failure, unreadable file (POSIX, non-root), non-dict / nameless column entries ignored - prune-existing: walk-up project-dir resolution (success + failure), --no-color env mutation, empty skipped-report guard Residual: prune_existing.py:270 (real WarehouseAdapter.from_profile, gated-only) and :447-454 (--profiles-dir resolve error, flaky to trigger) are documented coverage-policy exclusions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: assert diff.json sidecar durability in prune-existing success tests Addresses CodeRabbit review on #112: the three new success-path tests now also assert the default-on .signalforge/diff.json sidecar is written (DEC-016 e2e convention — exit 0 + no traceback + durable sidecar). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: address CodeRabbit findings on ingest + prune-existing (#104/#105) (#113) #3 (ingest reader): enforce the DEC-005 size cap from stat().st_size BEFORE reading file bytes for a Path input, so an oversize schema.yml is rejected without first being slurped into memory. The post-resolve length check stays as the str-path enforcement + backstop. #2 (prune-existing --dry-run docs): correct cli-ops.md — --dry-run suppresses the .signalforge/diff.json sidecar, but the fail-closed .signalforge/prune.jsonl audit is STILL written (every prune run leaves a durable receipt — the cross-stage invariant; mirrors generate). Pinned by strengthening test_dry_run_writes_no_sidecar to assert prune.jsonl is present. (#1, env-var restore, is left as-is — DEC-023 'mutate, don't restore' is the deliberate, consistent pattern across all subcommands.) reader.py back to 100% coverage; full suite 1945 passed. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both inert grade-layer exports re-verified still-reserved against the current design; neither promoted: - GradeBudgetExceededError — v0.1 degrades via aggregate_complete=False; v0.2's pre-first-pair hard-fail is a genuinely distinct category, so the reservation still holds. - GradeThresholds — the BaseModel form (with [0,1] range validation) is the right eventual container, but no grade-layer rework is in flight, so wiring it now is churn for no caller. Updates .claude/rules/grade-layer.md § "Schema-version surfaces" to record verified status and note DiffReport.audit_schema_version graduated in #50. No production code change. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add super plan for #116: custom business-rule test generation 16 implementation stories + Quality Gate + Patterns & Memory; 15 decisions. Phase: detailing (awaiting approval). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: mark #116 plan published (PR #117) * chore: devolve #116 plan to beads (epic bd_1-scaffolding-48o, 18 tasks) * bd_1-scaffolding-48o.3: add CandidateTestCustomSQL variant + union + drift mirror Add a fifth CandidateTest variant CandidateTestCustomSQL (DEC-002) for custom singular SQL business-rule tests: - type: Literal["custom_sql"], sql: str (failing-rows SQL), column: str | None (None = model-level), rationale: str | None = None - frozen Pydantic v2, extra="ignore", non-empty sql validator - added to the CandidateTest discriminated union and __all__ - StrictCandidateTestCustomSQL(extra="forbid") mirror in the draft drift detector + new union member - model-level custom_sql row added to candidate_schema_v1.json fixture Minimal downstream arm to keep pyright/tests green: prune/compiler.py _compile_test now matches CandidateTestRelationships explicitly and raises NotImplementedError for custom_sql (compiler support is a separate bead); behavior for the four existing variants is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-48o.6: backtick-aware SQL-safety strip (DEC-008) Extend `_strip_string_literals` to also neutralise backtick-quoted identifier spans (`` `...` ``) alongside single/double-quoted string literals. Without this, a stray quote inside a backtick identifier (e.g. `` `it's` ``) opened a phantom single-quoted literal that could swallow a real top-level `;`, letting statement-stacking slip past `validate_test_sql`'s cheap-reject checks (DEC-008 of #116). Backtick spans use the same doubled-quote escape handling as the existing quote chars; all prior behaviour (single/double quotes, balanced-paren depth scan, comment-marker detection) is preserved. This stays a cheap-reject checker — no full SQL parser (warehouse-adapters.md / prune-engine.md DEC-024 preserved). Tests: a `;` masked by a stray quote inside backticks is now caught; a top-level `;` after a backtick span containing `;` is caught; benign backtick identifiers (incl. an in-span `;`) are still allowed; comment markers inside quotes/backticks are ignored; balanced-paren logic is unaffected by backtick content. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-48o.9: artifact_id custom_sql args-hash branch (DEC-012) Extend the shared artifact-id seam so the fifth CandidateTest variant CandidateTestCustomSQL gets a stable args-hash and dotted artifact id. - model_test_args_hash: add a custom_sql branch hashing {type, column, sql} as canonical JSON, so distinct SQL -> distinct hash and identical SQL collides deterministically; column (None vs str) keeps a column-scoped and model-level custom_sql with the same SQL apart. - artifact_id_for is generic over test.type and already emits test.column.<col>.custom_sql / test.model.custom_sql with the optional .<args_hash> suffix; no formatter change needed. Tests cover distinct/identical SQL hashing, column distinguishing the hash, both dotted-path shapes, args-hash suffix, collision + ordinal-duplicate disambiguation via compute_args_hashes, and cross-stage byte parity with the grade engine. The cross-stage is-identity parity test stays green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-48o.1: manifest source registry + ref/source/this resolution Expose dbt sources from the manifest and add Jinja-ref relation resolvers (DEC-005 of #116), the manifest-layer foundation for resolving dbt refs in later business-rule-test stories. Stage-0: deterministic, no logging, typed errors with remediation. - models.py: new `Source` read-back model (frozen, extra="ignore", schema_ alias + identifier/name relation_name fallback); `Manifest.sources` field; `Manifest.resolve_ref` / `resolve_source` method wrappers; `Model.resolve_this`. - loader.py: filter `resource_type == "source"` into `Manifest.sources` at load; `resolve_ref(manifest, name, *, package, version)` (matches by Model.name, two-arg package disambiguation, fail-loud on unknown/ambiguous); `resolve_source(manifest, source_name, table_name)` -> TableRef. TableRef imported lazily (deferred) to keep manifest stage-0 import-clean. - errors.py: `RefNotFoundError`, `AmbiguousRefError`, `SourceNotFoundError` (all carry remediation). - __init__.py: re-export `Source`, the two resolvers, and the three errors. - cli/_helpers.py: register the three new errors in the exit-code table (tier 2 input-validation) so the 7th AST scan stays green. - tests: `Source` validate + drift-detector (extra="forbid" mirror) in test_models.py; new test_resolve.py covering ref/source/this resolution, package disambiguation, identifier fallback, and fail-loud unknown/ambiguous. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-48o.11: filename-safety builder + fail-closed .sql writer US-011 of #116 (DEC-010, DEC-014). Ships the two primitives the later `generate --write` CLI write-path will call to emit singular business-rule tests as standalone `.sql` files: - `signalforge.diff._test_file_writer.anchor_to_filename(...)` — injection-safe builder producing a relative `tests/<model>__<descriptor>_<hash>.sql`. Every component slugged to `[A-Za-z0-9_]`; `..`/`/`/`\`/control chars/NUL/absolute paths all collapse to `_` so a crafted manifest model name or LLM-emitted column name cannot escape `tests/`. Exhaustive adversarial unit tests. - `signalforge.diff._test_file_writer.write_test_file(...)` — the project's sixth fail-closed writer, mirroring `diff/_sidecar.py` verbatim: size-check before open -> symlink-hardened canonicalisation -> mkdir -p -> os.open(O_WRONLY|O_CREAT|O_TRUNC, 0o600) -> short-write while-loop -> fsync -> close in try/finally. No except around write/fsync (propagation IS the defence). Prepends a `-- signalforge:generated <hash>` header marker. Two new typed errors (`DiffTestFileWriteError`, `DiffTestFileRecordTooLargeError`) subclass `DiffError`, re-export from the package, and register tier 3 in `_EXCEPTION_TO_EXIT_CODE` (scan 7). Module added to `_FAIL_CLOSED_WRITER_MODULES` (scan 8). CLI does NOT wire `--write` (later bead) — primitives + errors only. Full validation green: ruff, format, pyright (0 errors), pytest (1992 passed). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-48o.4: extend anchor-contract + exclude_tests for custom_sql Add the fifth CandidateTest variant (custom_sql, DEC-002) to the drafter's recognised-type set and anchor-contract validator (US-004). config.py: add "custom_sql" to VALID_TEST_TYPES so it is a recognised type and can be named in exclude_tests. parser.py _validate_anchor_contract: bespoke custom_sql handling, collect-all (no short-circuit), matching the existing style: - sql must be non-empty after strip (whitespace-only is a violation; the model's truthiness validator already rejects "") - exempt from the parent-column-equality rule (a column-scoped custom_sql may reference other columns in its SQL); only membership of the declared column matters when column is not None - column=None is a valid model-level business-rule assertion - the existing exclude_tests gate (keyed on test.type) covers custom_sql Structural validation only — SQL Jinja/safety is the resolver/compiler's job. Tests: valid column-scoped + model-level, references-other-columns-not-rejected, empty-sql (column + model level), unknown declared column (column + model level), and exclude_tests rejection (column + model level). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-48o.5: drafter business-rule reading + custom_sql prompt Teach the drafter to propose custom singular SQL business-rule tests (DEC-001, DEC-015 of #116). Two input paths: 1. meta-driven (primary): read meta.signalforge.business_rules at column level (Column.meta) and model level (Model.config.meta), accepting a natural-language str OR list[str]. Mirrors the safety layer's meta.get("signalforge") dict-guard pattern (strict isinstance(dict) check; scalars/lists under the key are treated as config noise). Rules render into the DYNAMIC (non-cached) block as a fenced ## BUSINESS RULES section, model-level first then per-column (columns sorted for byte-stability), deduped. 2. inferred fallback: the system prompt permits the LLM to infer custom_sql tests from the model SQL + column profile when no rules are supplied. Prompt changes (draft/prompts.py): - Add a custom_sql JSON-shape illustration. It lives in a separate _CUSTOM_SQL_CATALOGUE_LINE appended unconditionally after the filtered four standard types — exclude_tests / VALID_TEST_TYPES stay four-typed and unchanged (config.py is owned by a parallel bead). - SCOPE section describes custom_sql as full singular-test failing-rows SELECTs that may use {{ this }} / {{ ref('m') }}, covering both the meta-driven and inferred paths. - _PROMPT_VERSION rotates 2563a71c5e31f0db -> 2e465018c1f6db22; the exclude_tests prompt-version recipe is intact. Cache-stability snapshot test updated in lockstep (cached block unchanged — business rules are dynamic-block-only). Orchestrator: business rules are read from the Model inside _render_dynamic_block, which render_prompt already receives, so no signature change is needed (both render_prompt callers keep working). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-48o.2: template-ref resolver (no Jinja engine) Add signalforge.manifest.resolve_template_refs(sql, model, manifest) — a bounded regex substituter (NO Jinja engine, DEC-004) that turns dbt-Jinja references in singular-test SQL into qualified table names for the prune compiler (US-007) and ingest reader (US-013) to consume: - {{ this }} -> model.resolve_this().qualified_name - {{ ref('m') }} / {{ ref('pkg','m') }} -> manifest.resolve_ref(...).qualified_name (last positional = name, leading = package) - {{ source('s','t') }} -> manifest.resolve_source(...).qualified_name Whitespace + both quote styles tolerated (mirrors ingest parser's unwrap). Fails loud on {% ... %} blocks, var()/env_var(), macro calls, dynamic ref()/source(), and any residual {{ }} after substitution. Underlying RefNotFoundError / AmbiguousRefError / SourceNotFoundError propagate. Placement keeps layering + AST scans intact: - Resolver lives in the manifest layer (stage-0: deterministic, no logging, typed errors carry remediation); re-exported from signalforge.manifest. - New TemplateResolutionError(ManifestError) + UnsupportedJinjaError go in the EXISTING manifest/errors.py (no new errors.py — scan-7 count stays 11). - Both registered in cli._helpers._EXCEPTION_TO_EXIT_CODE at tier 2; the test_exit_codes catch-all path constructs them via the layer-base shape. Tests: each ref form, source, this, pkg disambiguation, version-kwarg tolerance, whitespace, multi-ref, no-jinja passthrough, all rejection paths, and propagation of the three manifest resolver errors. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-48o.7: prune compiler _compile_custom_sql Replace the NotImplementedError placeholder arm for CandidateTestCustomSQL in prune/compiler.py with a real _compile_custom_sql (DEC-003/006/008/009). - Resolve dbt-Jinja refs ({{ this }} / {{ ref() }} / {{ source() }}) via signalforge.manifest.resolve_template_refs; thread the Model under prune through _compile_test as a keyword-only `model` param (engine call site passes model=model). Built-ins ignore it; existing call sites unchanged. - SQL-safety pre-flight (validate_test_sql) on the RESOLVED sql. - Conservative-bias routing: TemplateResolutionError / UnsupportedJinjaError / QuerySyntaxError all return the existing _InvalidIdentifier sentinel so the engine routes to kept-without-evidence (no 6th DropReason; the compiler never raises for these — DEC-006). - Single-table (no JOIN after literal-stripping) -> sample-CTE wrap mirroring the built-ins (substitute own qualified name with the `sample` alias); multi-table (JOIN survives) -> full-scan, partition filter applied to the model's own table only. Dialect-driven via Dialect.quote_char; no BigQuery-isms. - Returns the resolved failing-rows SELECT (NOT a pre-wrapped count) so the adapter's run_test_sql owns the COUNT(*) envelope, avoiding double-count. Snapshot fixtures pin byte-exact output: custom_sql.sql (single-table full), custom_sql_sample.sql (single-table sample), custom_sql_fullscan.sql (multi-table). Tests cover ref/source resolution, unsupported-Jinja/var/ safety-reject -> sentinel, no-model -> sentinel, full-scan partition wrap, determinism. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-48o.13: ingest tests/*.sql singular-test reader (US-013) Extend the stage-0 ingest layer to read an operator's existing singular dbt tests (.sql files under tests/) into CandidateTestCustomSQL records so prune-existing can later prune them (DEC-013). - parser.classify_singular_test(sql, *, file_name, model, manifest): reuses signalforge.manifest.resolve_template_refs to resolve ref()/source()/this (no regex duplicated). Associated -> CandidateTestCustomSQL(column=None); references a different / unknown / ambiguous model -> None (not included, not skip-recorded); unsupported Jinja ({% %}, {{ var() }}, macros, residual {{ }}) -> SkippedTest(reason="malformed-supported-test"). The closed 3-value SkipReason is unchanged. - reader.read_test_files(tests_dir, model, manifest, *, project_dir=None, existing=None): enumerates *.sql (sorted), size-caps each from stat() before read (5 MB, mirrors read_schema DEC-005), dedupes associated tests by (model, "custom_sql", sql_hash) via blake2b-8 of the SQL body, and seeds the dedupe set from an optional schema.yml-sourced `existing` candidate so the same test from both sources collapses. Returns a model-level-only CandidateSchema (no columns; no anchor check — singular tests are model-level with column=None). - Re-export read_test_files from signalforge.ingest. - Stage-0 discipline preserved: no logging, no audit writer, deterministic, typed errors carry remediation, extra="ignore" read-back models. Fixtures: tests/fixtures/ingest/custom_sql_files/ (orders ref, customers ref, unsupported-macro). Tests: tests/ingest/test_test_files.py (classifier matrix + reader: associate/exclude/skip, dedupe within dir and against existing, oversize-before-read, missing dir, non-sql ignored, sorted determinism). Validation green: ruff check + format, pyright (0 errors), full pytest (2084 passed, 96.58% cov). Logger grep-gate (ingest stays silent) + AST audit-completeness scans pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-48o.10: surface kept custom_sql tests as proposed .sql files US-010 of #116. Teach the diff layer to surface kept singular custom_sql business-rule tests as standalone .sql file proposals (DEC-011) — they are NOT schema.yml blocks. - models.py: add ProposedTestFile frozen model (path + sql); add DiffReport.proposed_test_files tuple; bump audit_schema_version 2 -> 3 in lockstep (sidecar shape evolved). Minimal __repr__ keeps the SQL body out of log sinks. - _emitter.py: _test_args_hash now delegates to the shared _common.artifact_id seam (handles the 5th custom_sql variant); _render_test returns a _SKIP sentinel for custom_sql so the YAML emitter cleanly skips it (never crashes); new emit_proposed_test_files builds one ProposedTestFile per kept custom_sql via the reused anchor_to_filename + _with_marker + shared args-hash. - _renderers.py: AnsiRenderer + MarkdownRenderer render a proposed test-files section (new-file header / fenced sql block) with the same unconditional ANSI-strip + dynamic markdown fence on the SQL content (DEC-007/008). JsonRenderer/sidecar carry it automatically. - engine.py: wire emit_proposed_test_files into DiffReport + INFO log. - Drift detectors (test_drift_detector.py + inline test_models.py) + fixtures (diff_report_v1.json) updated; ProposedTestFile exported on the public surface; snapshot cases + fixtures regenerated; e2e diff.json fixture + docs (diff-ops, audits) bumped to v3. Tier classification + why-cascade unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-48o.8: pin prune engine routing + audit for custom_sql (US-008) Verify and pin (via tests) that the prune ENGINE routes custom_sql business-rule tests through the existing decision matrix unchanged. The engine's per-test routing is deliberately test-type-agnostic — it dispatches on the compiler's return shape (str / _InvalidIdentifier / _RequiresFutureData), the warehouse failure_count, and any raised WarehouseError — so custom_sql flows through the same paths as the four built-ins with NO engine source change. Six new fake-adapter tests in tests/prune/test_engine.py: - always-passes (failure_count=0) -> dropped - real failure on untrusted model (failure_count>0) -> kept - unsupported-Jinja -> _InvalidIdentifier sentinel -> kept-without-evidence (no warehouse call) - WarehouseError during query -> kept-without-evidence (generic per-test why) - multi-table full-scan over maximum_bytes_billed (BytesBilledExceededError) -> kept-without-evidence - fail-closed audit invariant: one PruneEvent per custom_sql candidate, PruneEvent shape unchanged (round-trips through read-back model) DEC-007 why decision (byte-cap case): use the GENERIC per-test handler why ("Test could not be evaluated: BytesBilledExceededError: ...") rather than a bespoke locked string. The typed class name is already in the why for reviewer correlation; a distinct why would require special-casing one WarehouseError subclass in the otherwise error-type-agnostic handler. The locked 5-value DropReason literal stays unchanged; no new DropReason. No audit-shape change -> prune_event_v1.jsonl fixture + drift detector untouched. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-48o.12: generate --write writes proposed .sql + --force (US-012) Wire `signalforge generate --write` to additionally materialise every proposed singular `.sql` business-rule test on `DiffReport.proposed_test_files` to its `tests/` path via `diff._test_file_writer.write_test_file` (DEC-010/ DEC-014 of #116). Add a `--force` flag governing the overwrite policy: - target does not exist -> write - exists + our marker + --force -> overwrite - exists + our marker, no --force -> skip + stderr WARNING (names file) - exists, hand-authored (no marker) -> NEVER overwrite, even --force; skip + WARNING `--dry-run` writes nothing (no schema.yml, no sidecar, no .sql). Paths flow through `canonicalise_user_path` (-> CliPathError); the writer's typed errors (DiffTestFileWriteError / DiffTestFileRecordTooLargeError) are already in the exit-code table and map via the single boundary catch. `ProposedTestFile.sql` ships marker-prefixed, so `_split_marked_sql` recovers (args_hash, bare body) to avoid a doubled marker when `write_test_file` re-prepends it. Lazy-format JSON logging throughout (logger grep-gate clean). 5-surface parity (cli-layer.md): argparse help, handler/helper docstrings, docs/cli-ops.md (flag reference + stderr WARNING shapes), behavioural tests in test_generate.py, and a bespoke test_5_surface_parity_force.py referencing DEC-010/DEC-014/US-012. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-48o.19: _compile_custom_sql catches unresolvable ref/source (no crash) A defect found during US-008: _compile_custom_sql called resolve_template_refs, which can raise RefNotFoundError / AmbiguousRefError / SourceNotFoundError (all ManifestError siblings of TemplateResolutionError, NOT subclasses). The compiler only caught TemplateResolutionError / UnsupportedJinjaError / QuerySyntaxError, so an unresolvable {{ ref('missing_model') }} propagated UNCAUGHT out of prune_tests and crashed the run. Fix in _compile_custom_sql (route to existing sentinels, never raise): - RefNotFoundError / SourceNotFoundError -> _RequiresFutureData sentinel -> requires-future-data drop reason (the target isn't built yet; mirrors the relationships missing-target precedent, DEC-026). - AmbiguousRefError -> _InvalidIdentifier sentinel -> kept-without-evidence (genuine user ambiguity, not future data). DropReason stays the locked 5-value Literal — no 6th added. Compilation stays total (DEC-006). Return type widened to str | _RequiresFutureData | _InvalidIdentifier. Tests: compiler-level (unresolvable ref -> _RequiresFutureData; unresolvable source -> _RequiresFutureData; ambiguous ref -> _InvalidIdentifier) and engine-level end-to-end via fake adapter (unresolvable ref -> requires-future-data with no warehouse call; ambiguous ref -> kept-without-evidence with no warehouse call), confirming prune_tests no longer raises for these. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-48o.14: ingest singular tests/*.sql in prune-existing (US-014) Extend `signalforge prune-existing` (ingest -> prune -> diff, no LLM) so it also ingests the project's singular `tests/*.sql` business-rule tests and prunes them alongside the schema.yml tests (DEC-010, DEC-013). After read_schema, the handler enumerates the singular-test directory (default <project_dir>/tests, override via new --tests-dir) and calls read_test_files(..., existing=<schema candidate>) to merge custom_sql tests, deduped against the schema.yml ones. Both sets are combined into ONE candidate fed to prune_tests so the warehouse tells the operator which of ALL their existing tests add no signal. - Default tests/ dir is optional: silently skipped when absent; an explicit --tests-dir to a missing dir fails loud (IngestSchemaNotFoundError -> exit 1). - Skipped/unsupported singular tests fold into the existing grouped-by-reason stderr summary alongside schema.yml skips. - Read-only preserved (DEC-003 of #105): no --write; diff to stdout + default-on .signalforge/diff.json sidecar (--dry-run suppresses); existing_schema feeds render_diff; grading_report=None (never flagged). No new error classes — the five IngestError concretes are already in the exit-code table. - docs/cli-ops.md prune-existing section only: --tests-dir flag row + a 'Singular tests/*.sql business-rule tests' subsection. generate section untouched. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-48o.15: e2e gated test for custom_sql business-rule tests (US-015) Add a double-gated end-to-end test proving the full pipeline drafts, prunes, and diffs custom singular-SQL business-rule tests (DEC-009 of plans/super/116-business-rule-tests.md). Mirrors the existing test_e2e_bigquery_smoke.py gating exactly: @pytest.mark.e2e (deselected by default addopts) plus a runtime _skip_reason() on the env triple (SF_RUN_BQ=1, ANTHROPIC_API_KEY, GOOGLE_CLOUD_PROJECT). Reuses the committed Austin-bikeshare fixture verbatim and injects engineered meta.signalforge.business_rules into the per-run tmp_path manifest copy via a new inject_model_business_rules helper — keeps the e2e fixture decoupled from the init-demo parity tree (the committed Austin fixture must stay byte-equal to src/signalforge/_demo/), so no demo mirror / regenerate.sh change is needed. Engineered determinism (testing-signal.md): the LLM's exact SQL bytes are non-deterministic, but rule SEMANTICS make the prune outcome mathematically guaranteed on any sample: - always-pass tautology ("duration_minutes >= itself") -> always-passes -> DROPPED (Architectural Commitment #1). - finds-failures rule ("same start/end station") -> guaranteed failing rows in real A->B bikeshare data -> KEPT. Live-run assertions: exit 0, diff.json round-trips, at least one custom_sql PruneDecision dropped with reason='always-passes' AND at least one kept with reason='kept', a proposed_test_files entry exists, no traceback leak. Paired default-suite (no-env) helper tests pin that injected rules survive a real signalforge.manifest.load round-trip onto Model.config.meta where the drafter reads them, and that an unknown unique_id fails loud. Full validation green: ruff check/format, pyright (0 errors), pytest (2147 passed, 21 deselected, 96.61% cov). Both e2e tests collect under `pytest -m e2e --co`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-48o.16: operator docs for custom business-rule tests (US-016) Complete the operator narrative for the custom singular SQL business-rule test feature (custom_sql, issue #116) across the user-facing docs. Extends the lockstep edits already made by implementation beads (diff-ops/audits audit_schema_version 2→3; cli-ops --force/--tests-dir); does not duplicate. - docs/draft-ops.md: new "Custom business-rule tests (custom_sql)" section — authoring meta.signalforge.business_rules (NL str/list, column + model level), inferred-fallback, {{ this }}/{{ ref }}/{{ source }} support with control-flow Jinja unsupported, a worked rule→JSON example; CandidateTest union + CandidateTestCustomSQL in the API table; exclude_tests exemption. - docs/prune-ops.md: new "custom_sql evaluation" section — Jinja-resolution routing (clean / requires-future-data / kept-without-evidence), single- table sampled vs multi-table full-scan, the maximum_bytes_billed cap as the only multi-table guardrail + tuning note; folded into the expected- drop-rate framing; taxonomy table + v0.2-deferrals updated to five types. - docs/ingest-ops.md: new "Singular tests/*.sql tests" section for read_test_files — ref/source/this resolution, association-to-model, unrelated-files-ignored, unsupported-Jinja skip, dedupe + size cap. - README.md: custom business-rule tests in the feature list; a worked rule→generated .sql→kept/dropped example; prune-existing singular .sql ingestion note. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-48o.17: QG fix — custom_sql materialised-table substitution + multi-table test coverage P0 (CRITICAL): _compile_custom_sql single-table scope="full" path now rewrites the model's own qualified name to the substituted table_ref when the engine handed it a DIFFERENT physical table (the materialised temp table under sample_strategy="materialised" + scope="sample"). Previously returned the resolved SQL unchanged, so single-table custom_sql tests silently full-scanned the production source instead of the materialised sample — defeating the cost model and risking maximum_bytes_billed over-cap. Mirrors the built-in compilers, which always FROM table_ref. Multi-table (DEC-006) and oneshot/full-strategy behaviour unchanged. P2: single-table self-reference (correlated subquery / self-UNION) now substitutes ALL occurrences in the sample branch and the full-scope partition branch (dropped count=1). Multi-table partition replace stays count=1 (intentional). Tests (tests/prune/test_engine.py): - Tightened the over-byte-cap multi-table test to require the full-scan JOIN shape and reject a WITH sample CTE; switched to two distinct refs ({{ this }} + {{ ref('other_model') }}) so it genuinely exercises the multi-table classifier. - Added engine-level multi-table full-scan tests (failures=0 → dropped/ always-passes; failures>0 → kept) asserting no sample CTE is dispatched. - Added a P0-fix test: single-table custom_sql under materialised+sample references _SESSION._sf_sample_<run_id> and never the source table. No DropReason added; no .claude/ edits; no fixture regen needed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-48o.18: Patterns & Memory — business-rule-tests rule + CLAUDE.md surface New .claude/rules/business-rule-tests.md distilling the #116 conventions (custom_sql variant, bounded Jinja resolution, materialised-substitution gotcha, fail-closed .sql writer, conservative-bias routing); CLAUDE.md #116 entry + v0.3 public-API block. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-48o.20: close codecov patch-coverage gaps Add targeted tests exercising the 19 feature-introduced lines Codecov flagged uncovered on PR #117, all real behaviour assertions (no src changes, no pragmas): - draft/models.py:157 — CandidateTestCustomSQL.sql empty-string validator. - manifest/loader.py:443 — resolve_source missing-schema/identifier branch. - prune/compiler.py:682 — scope='sample' missing sample_size/bucket guard. - prune/compiler.py:719-722 — single-table scope='full' + partition-filter derived-table wrap of the model's own table. - diff/_emitter.py:313 — dedupe continue for two kept custom_sql decisions resolving to the same path. - diff/_test_file_writer.py:275 — os.write-returns-0 short-write guard (mirrors tests/diff/test_sidecar.py). - cli/generate.py:457 — _split_marked_sql no-marker fallback. - cli/generate.py:482-483 — _existing_file_is_signalforge_generated OSError branch (unreadable path → False). - ingest/reader.py:375-376 — read_test_files PathContainmentError wrap. - ingest/reader.py:396 — non-file *.sql glob match skip. - ingest/reader.py:432-433,441-442 — stat/read OSError → IngestSchemaParseError. Full validation green; coverage 96.90%. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-48o.21: address PR #117 review (CodeRabbit + Copilot) Fix nine review findings across source, tests, and docs: 1. manifest/template.py: _resolve_ref_args split on commas + drop kwargs so ref('m', version='1') resolves model 'm', not '1'. 2. ingest/reader.py: _read_sql_file catches UnicodeDecodeError → typed IngestSchemaParseError instead of an untyped escape. 3. draft/prompts.py: custom_sql now participates in exclude_tests filtering; catalogue line + SCOPE instruction emitted only when allowed; SCOPE phrase reads "..., plus custom_sql". _PROMPT_VERSION rotates to c9e7ee1f6f465933. 4. prune/compiler.py: _compile_custom_sql fails closed (kept-without-evidence) when {{ this }} can't bind to the effective sample/materialised table. 5. tests/manifest/test_models.py: deterministic source select + parenthesised relation_name assertion (was always-truthy). 6. docs/ingest-ops.md: unsupported-Jinja singular .sql skip reason is malformed-supported-test, not custom-or-generic-test. 7. docs/draft-ops.md: public signalforge.manifest.resolve_template_refs. 8. cli/prune_existing.py: --tests-dir help clarifies only the DEFAULT dir is silently skipped; explicit missing --tests-dir fails loud. 9. diff/__init__.py: docstring now lists 4-value Tier (kept-uncertain) and the nine-class DiffError hierarchy. Regression tests added for items 1-4; tests adjusted for item 3/5. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-48o: fix Python-version-fragile stat() monkeypatch (CI 3.12) test_read_test_files_stat_oserror_raises_parse_error keyed on the follow_symlinks kwarg to distinguish is_file()'s stat from the size-check stat — that distinction is 3.13-only, so the patched OSError escaped during is_file() on 3.12. Decouple by forcing is_file() True for the target and letting only the size-check stat() raise. Verified on 3.11/3.12/3.13. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-48o.22: PR #117 round-2 review + codecov gap Item 1 (Major) — ingest/parser.py classify_singular_test: a singular test referencing THIS model AND an unresolvable/ambiguous OTHER ref()/source() was silently dropped when the whole-body resolve raised. Added bounded per-expression heuristic _raw_sql_references_target (reuses template.py regexes + _resolve_ref_args; {{ this }} or a ref() resolving to the target associates). On the resolver-error branch, when this model is referenced we now carry the RAW unresolved SQL into a CandidateTestCustomSQL so the prune compiler routes it (requires-future-data / kept-without-evidence, US-019); only a genuinely-unrelated body returns None. source() is intentionally not checked — a singular test's target is always a model, never a source. Item 2 (Minor) — replaced the raw `target not in resolved` substring check with a word-boundary match (_references_qualified_name) so the target name inside a comment/string or as a fragment of a longer dotted identifier no longer false-associates. Item 3 (Codecov) — added tests/manifest/test_template.py cases for an empty arg-list fragment (trailing / interior comma) in ref(); template.py now 100% (the empty-token continue at line 159 is covered). Item 4 (docs) — docs/draft-ops.md: custom_sql CAN now be excluded via DraftConfig.exclude_tests (US-021; it is in VALID_TEST_TYPES, the prompt omits its catalogue/SCOPE blocks when excluded and the parser rejects it if the LLM defies that). Corrected both the §custom_sql note and the exclude_tests field description + YAML comment. Regression tests added in tests/ingest/test_test_files.py (Items 1+2) and tests/manifest/test_template.py (Item 3). Full suite green (2183 passed), coverage 96.93%, docs build clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…lake_client.py shim (#125) * Add super plan for #119: SnowflakeAdapter skeleton Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: mark #119 plan published (PR #125) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: mark #119 plan devolved (beads epic bd_1-scaffolding-cx3) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-cx3.1: #119 US-001 SNOWFLAKE_DIALECT constant + re-export Add SNOWFLAKE_DIALECT to warehouse/models.py alongside POSTGRES_DIALECT (name='snowflake', quote_char='"', identifier_case='upper', supports_qualify=True, supports_tablesample=True). Docstring notes identifier_case='upper' is the opposite of Postgres and load-bearing for the Snowflake compiler (#121). Added to models.py __all__ and re-exported from the warehouse package (import block + package __all__, sorted between QuerySyntaxError and SamplingError per test_all_is_sorted). Tests pin the dialect values and the top-level package re-export. Traces to #119 DEC-004. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-cx3.2: #119 US-002 _snowflake_client.py shim + [snowflake] dep Add the one-shim-per-vendor SDK seam for snowflake-connector-python, mirroring adapters/_client.py (BigQuery). Confines every snowflake SDK type-ignore to this file (DEC-005); lazy SDK import inside make_real_client so the shim imports cleanly without the connector installed. Adds the [snowflake] optional-dependency extra + keeps the two dev lists in lockstep (DEC-006). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-cx3.3: #119 US-003 SnowflakeAdapter skeleton + factory dispatch Add src/signalforge/warehouse/adapters/snowflake.py defining SnowflakeAdapter (WarehouseAdapter), modelled on the postgres.py stub: forward-compat conn params (DEC-002), credential-redacting __repr__ showing only account + warehouse (DEC-003), no-op context manager, dialect() returning SNOWFLAKE_DIALECT by identity, and three warehouse-op methods raising NotImplementedError naming the epic (#118, DEC-008). materialise_sample / estimate_query_bytes are NOT overridden — the ABC NotSupported defaults are correct v0.2 behaviour. Wire from_profile's snowflake branch (DEC-001, lazy import) + docstring; the no-LLM/no-BQ-SDK dispatch is pinned by a fresh-subprocess test (DEC-007). test_base.py's unknown-type test moves to 'databricks' since snowflake now dispatches. New tests/warehouse/test_snowflake_stub.py pins all four ACs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-cx3.4: fix SNOWFLAKE_DIALECT docstring v0.3→v0.2 (QG code-review) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-cx3.4: Quality gate — fix bugs from code review + CodeRabbit Split _SnowflakeClientProtocol into a cursor protocol (execute/fetchall/close) and a connection protocol (cursor/close) so the shim honestly describes the real snowflake.connector DB-API shape — query execution lives on the cursor, not the connection (CodeRabbit critical x4). Update fakes + docstring in lockstep. Also fixed SNOWFLAKE_DIALECT docstring v0.3->v0.2 slip. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-cx3.5: #119 Patterns & Memory — document Snowflake skeleton Update .claude/rules/warehouse-adapters.md (Snowflake skeleton note: identifier_case='upper' divergence, warehouse-side SDK-confinement test, cursor/connection protocol split, #120 profile-relaxation boundary), CLAUDE.md public-API surface (SnowflakeAdapter + SNOWFLAKE_DIALECT, [snowflake] extra), and docs/warehouse-adapter-ops.md (skeleton + install note). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #119: clarify dataset/schema alias in plan US-003 TDD (PR review) Address CodeRabbit false-positive flag: DbtProfileTarget.dataset carries alias="schema", so the fixture's schema="sch" populates dataset; no mismatch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #119: Address PR review feedback (subprocess timeout + ops-doc consistency) - test_snowflake_stub.py: add timeout=10 + explicit check=False to the no-BigQuery-SDK subprocess (matches repo convention; a hung child would otherwise hang the suite). [Copilot] - warehouse-adapter-ops.md: unwrap the split `uv pip install` code span; reconcile the "bigquery only / else UnsupportedProfileTypeError" text and the error-reference row with the now-live postgres/snowflake dispatch. [Copilot + CodeRabbit] Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add super plan for #120: parse Snowflake target in profiles.yml Unified DbtProfileTarget + cross-field validator for type=snowflake: required account/user/warehouse, password/key-pair/SSO auth, permissive account validator, threads field, IncompleteProfileError (tier 1). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: mark plan #120 phase published (PR #126) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: devolve plan #120 to beads (epic bd_1-scaffolding-ohz) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ohz.4: extend SnowflakeAdapter for key-pair/SSO auth, keep repr redaction Add forward-compat private_key_path / private_key_passphrase / authenticator keyword-only params (DEC-008); #122 consumes them at connection time. __repr__ still renders only account + warehouse — the new auth fields never appear (DEC-003). Pin the redaction + storage with focused tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ohz.1: add validate_snowflake_account permissive account validator DEC-006. Adds `_SF_ACCOUNT_RE` + `validate_snowflake_account` to warehouse/_sql_safety.py: a permissive account-identifier gate (log-injection hygiene, not the strict SQL-identifier rule, since account IDs are never interpolated into SQL). Accepts org-account, region-suffixed legacy locator, and bare forms; rejects empty/whitespace/quoting/SQL-fragment/backtick/over-long input via the existing InvalidIdentifierError. New tests/warehouse/test_sql_safety.py. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ohz.2: add IncompleteProfileError + exit-code registration + snowflake auth remediation US-002 (#120, DEC-004/005): new IncompleteProfileError(WarehouseError) with collect-all missing-key message and repr-safe rendering, a _SNOWFLAKE_DEFERRED_AUTH_REMEDIATION constant for US-003 reuse, tier-1 exit-code registration in _EXCEPTION_TO_EXIT_CODE, and package re-export. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ohz.3: grow DbtProfileTarget with Snowflake fields + cross-field validator Add Snowflake connection fields (account/user/role/warehouse/database/ password/private_key_path/private_key_passphrase/authenticator) plus a shared `threads` field to the unified DbtProfileTarget, with a @model_validator(mode="after") enforcing per-type coherence: required keys (IncompleteProfileError), foreign-field rejection (ValueError → ValidationError), identifier hygiene, and deferred-authenticator rejection. Update the two #119 snowflake-stub dispatch tests that used a BigQuery- shaped placeholder profile (now correctly rejected) to a valid Snowflake target; exact from_profile wiring stays US-005's concern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ohz.5: wire from_profile snowflake branch + fixtures + drift detector Replace the #119 placeholder snowflake branch in from_profile to pass every parsed DbtProfileTarget field (account/user/password/role/warehouse/database/ schema-via-dataset/private_key_path/private_key_passphrase/authenticator) into SnowflakeAdapter; keep the lazy import so the no-BigQuery-SDK-import test holds. Add snowflake_password.yml (load_profile parse fixture) and dbt_snowflake_drift_v1_x.yml (forward-compat drift fixture covering the documented dbt-snowflake 1.8/1.9 target field set). Add StrictSnowflakeModel + test_drift_detector_snowflake_extra_forbid and a load_profile-through-fixture parse test. Tighten test_from_profile_dispatches_snowflake_to_skeleton to assert the full field wiring. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ohz.6: Quality gate — fix bugs from code review - profiles.py: identifier-validate `role` (becomes `USE ROLE <role>` SQL in #122) alongside warehouse/database/schema. - test_profiles.py: tighten snowflake typed-error assertions to the exact type (mode="after" validator propagates WarehouseError subclasses raw, so the ValidationError arm was dead); add a bad-`role` identifier case. CodeRabbit skill not available in this environment. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ohz.7: Patterns & Memory — document unified multi-warehouse profile - warehouse-adapters.md: new section on the unified DbtProfileTarget + per-type cross-field validator pattern (required-by-type, foreign-field rejection, identifier hygiene incl. role, auth scope, drift detector, raw-vs-wrapped validator-error propagation); Reference points to plan #120. - CLAUDE.md: public-API surface updated with the #120 Snowflake profile fields + IncompleteProfileError. - warehouse-adapter-ops.md: Snowflake profile-parsing section + $-identifier deferral + IncompleteProfileError error-reference row. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #120: Address PR review feedback (Copilot + CodeRabbit) - profiles.py: mark password/private_key_passphrase/private_key_path Field(repr=False) so DbtProfileTarget repr()/str() can't leak credentials; add regression test test_snowflake_secrets_excluded_from_repr. - _sql_safety.py: correct validate_snowflake_account docstring — `--`/hyphens are accepted (legal in locators, never reaches SQL); intent is to reject whitespace/quotes/;/backticks/control-chars. - warehouse-adapters.md: fix the validator-exception-wrapping claim (Pydantic v2 wraps only ValueError/TypeError/AssertionError; custom exceptions propagate raw from BOTH field_validator and model_validator), and align the validate_snowflake_account bullet (`--` accepted). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add super plan for #121: prune compiler Snowflake dialect support Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #121: devolve plan to beads (epic bd_1-scaffolding-mfa) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-mfa.1: US-001 extend Dialect with dialect-fragment fields Add four BigQuery-defaulted declarative fields to the frozen Dialect dataclass (DEC-001 of #121): sample_row_hash_expr, timestamp_literal_template, date_literal_template, and quote_qualified_per_component. Defaults reproduce BigQuery SQL byte-for-byte so every existing construction site stays valid unedited. Set SNOWFLAKE_DIALECT's four values (DEC-002): ABS(HASH(*)), '{value}'::TIMESTAMP, '{value}'::DATE, per-component quoting. Leave POSTGRES_DIALECT at the BigQuery defaults with a docstring note (DEC-007) that they are corrected when the Postgres adapter's warehouse ops land. Refresh the Dialect class docstring to describe what the prune compiler reads each new field for. Add unit tests pinning the SNOWFLAKE_DIALECT values, the BIGQUERY_DIALECT defaults, and that constructing a Dialect with only the five original fields still succeeds. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-mfa.2: US-002 consume Dialect fields in compiler Make signalforge.prune.compiler emit warehouse-correct SQL purely from the Dialect value object — no branching on dialect name, no warehouse-SDK import under prune/. - _quote/_qualified_table_name now take the Dialect: fold identifiers per identifier_case (upper/lower/preserve) before quoting (DEC-003); branch on quote_qualified_per_component (Snowflake "DB"."SCH"."T" vs BigQuery whole- path) (DEC-002). - _render_sample_cte renders the hash-mod predicate from dialect.sample_row_hash_expr (Snowflake ABS(HASH(*)) vs BigQuery FARM_FINGERPRINT). - _render_partition_filter renders datetime/date via the dialect's literal templates (Snowflake '...'::TIMESTAMP vs BigQuery TIMESTAMP('...')). - Thread dialect (not bare quote_char) through _wrap_with_sample_or_partition and every _compile_* helper. BigQuery's identifier_case="preserve" + quote_qualified_per_component=False make the refactor a no-op for BigQuery: all 11 compiled_sql/*.sql snapshots stay byte-identical (zero fixture edits) — the regression gate (DEC-001). Adds tests/prune/test_compiler_import_guard.py (DEC-008): AST scan asserting no snowflake / google.cloud import under src/signalforge/prune/, with a planted-violation self-check and a no-false-positive check. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-mfa.3: US-003 Snowflake snapshot fixtures + tests Add byte-exact Snowflake snapshot fixtures (captured from real compiler output with SNOWFLAKE_DIALECT) and snapshot tests covering the four built-in test types (full + sample modes) and custom_sql (single-table full, single-table sample, multi-table full-scan). - 11 new fixtures under tests/fixtures/prune/compiled_sql/snowflake/: "-quoted, per-component qualified names ("FAKE_PROJECT"."DATASET"."ORDERS"), UPPER-folded identifiers, MOD(ABS(HASH(*)), <bucket>) < 1 sample predicate. - Snapshot tests assert compiled == fixture (byte-exact) per variant. - custom_sql single-table sample test confirms the #116 materialised-sample substitution invariant under the Snowflake quote char: the body reads from the `sample` CTE alias and never the source table. - Guard tests: Snowflake fixtures contain `"` and never a backtick; sample fixtures contain HASH(*) and never FARM_FINGERPRINT. The 11 existing BigQuery fixtures are unchanged; compiler logic untouched (US-002). Full validation green (2245 passed). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-mfa.4: US-004 fakesnow gated Snowflake SQL validation Add `fakesnow` as a dev/test dependency behind a NEW gated `@pytest.mark.snowflake` marker and a gated test that feeds the compiler's real emitted Snowflake SQL into an in-memory fakesnow connection. - pyproject.toml: add `fakesnow>=0.9` to `[dependency-groups].dev` and `[project.optional-dependencies].dev` (kept in sync); register the `snowflake` marker; add `and not snowflake` to the default `addopts` deselection so the default `pytest` run does NOT collect it. - tests/prune/test_compiler_fakesnow.py: for each built-in test type (not_null / unique / accepted_values / relationships), compile the failing-rows SELECT with SNOWFLAKE_DIALECT, create a tiny matching fakesnow table, run the SQL wrapped as the adapter does (`SELECT COUNT(*) AS failures FROM (<sql>) AS t`), and assert it executes with the engineered failing-row shape. Determinism engineered by RULE SEMANTICS, not value-equality (testing-signal.md): a NULL row → failures >= 1, a duplicate → failures >= 1, etc. No HASH() value assertions. scope="full" only — fakesnow's DuckDB backend rejects the sample-mode `HASH(*)` predicate and the `sample` CTE name (DEC-005 caveat; sample-mode shape is gated by the US-003 byte-exact snapshots). Confirmed: default `uv run pytest` deselects all 8 snowflake tests; `uv run pytest -m snowflake --no-cov` runs them green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-mfa.5: QG fix — Snowflake sample-CTE reserved-word bug The deterministic-sample CTE was bound to the bare alias `sample`, but `SAMPLE` is a Snowflake reserved keyword (TABLESAMPLE) — `WITH sample AS (...)` is a syntax error on real Snowflake (confirmed via sqlglot). This is on the default `scope=sample` prune path, not an edge case. Fix: add `Dialect.sample_cte_alias` (BigQuery default bare `sample` → BQ snapshots byte-identical; Snowflake `"sample"` quoted to bypass the reserved word). Thread it through `_render_sample_cte`, the built-in `target`, the relationships `child_target`, and the custom_sql sample substitution. Regenerated the 5 Snowflake sample fixtures; added a gated sqlglot parse-guard over every Snowflake fixture (the guard that catches this class of bug, since fakesnow can't execute sample-mode SQL due to HASH(*)). Found during US-005 Quality Gate; surfaced by the US-004 fakesnow validation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-mfa.5: QG review fixes — doc-count drift + import-guard hardening From the 4 code-review passes (no blocking bugs found beyond the already-fixed sample-CTE reserved-word bug): - Doc-count drift: the QG sample-CTE fix added a 5th Dialect field, so the "four fields" wording in the Dialect / POSTGRES docstrings and three test_models docstrings was stale -> "five". Added the missing sample_cte_alias default assertion to test_dialect_constructs_without_new_field_args. Clarified the POSTGRES note (sample_cte_alias="sample" is already Postgres-correct; quote_qualified_per_component + cast templates are what need correcting when Postgres ops land). - Import-guard hardening: the AST detector missed the namespace-split `from google import cloud` form (module='google' alone isn't forbidden but the imported name completes 'google.cloud'). Now flagged; planted self-check extended to 9 violations. Resolved the reviewers' split on string-literal escaping: Snowflake DOES honor backslash escapes (\') in single-quoted string literals, so the shared escape_bq_string_literal is valid for both dialects (no change). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-mfa.6: US-006 Patterns & Memory — Snowflake dialect + identifier_case graduation 5-surface graduation of identifier_case (declared -> active) + Snowflake compiler dialect documentation: - .claude/rules/prune-engine.md: full Dialect-field list the compiler reads; identifier_case graduation; sample_cte_alias reserved-word lesson (snapshots pin shape not validity — keep a parser/executor in the loop); supports_qualify stays forward-compat; new #121 Snowflake subsection + refs. - docs/prune-ops.md: Snowflake compiler dialect section (case-folding + residual, HASH() cross-version reproducibility caveat, QUALIFY not used, validation tiers); updated multi-warehouse deferral (Snowflake landed). - CLAUDE.md: public-API surface — compiler emits Snowflake SQL from SNOWFLAKE_DIALECT; five new Dialect fields; identifier_case graduated. - .claude/rules/warehouse-adapters.md: Dialect now carries prune-compiler SQL-fragment templates; populate them for future vendor dialects. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #121: address PR review (Copilot) — comment typo + plan phase alignment - test_compiler_import_guard.py: fix confusing `googleftover` example in the prefix-match comment -> `google_leftover` (a name that starts with `google` but is not `google.cloud`). - plans/super/121-prune-snowflake-dialect.md: align the Phase line with reality (implementation complete, PR open awaiting review) so it no longer reads as a plan-only review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #121: cover the identifier_case='lower' fold branch (Codecov patch gap) Codecov flagged 1 uncovered patch line — compiler.py:149, the `identifier_case == "lower"` branch of `_fold_identifier`. No compiler test exercised a lower-folding dialect (POSTGRES_DIALECT isn't used in the snapshot suite). Added test_quote_folds_lower_for_postgres_dialect pinning `_quote("CustomerId", POSTGRES_DIALECT) == '"customerid"'`. Patch coverage now 100% for the changeset; the remaining compiler.py:312 miss is pre-existing #116 custom_sql code, outside this PR's diff. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add super plan for #123: Snowflake estimate_query_bytes degrade-first Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Mark #123 plan published (PR #128) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Devolve #123 plan to beads (epic bd_1-scaffolding-dqf) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-dqf.1: Add engine-level Snowflake estimate degrade test #123 US-001: wire a real SnowflakeAdapter() through the estimate(...) engine and assert the warehouse-bytes section degrades on EstimateNotSupportedError (a WarehouseError subclass) while the LLM-cost half still computes. Keys the assertion on the EstimateNotSupportedError class name so a narrowing of the engine's `except WarehouseError` (DEC-005) breaks the test loudly. Test-only; zero production change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-dqf.2: Add CLI --estimate Snowflake degrade test + doc note #123 US-002. Extend _install_estimate_patches with an optional adapter= kwarg (default preserves the BigQueryAdapter behaviour) so a test can inject a real SnowflakeAdapter. New test asserts main(["generate", "--estimate", ...]) exits 0, stdout carries "<unavailable: EstimateNotSupportedError>", and no traceback leaks (DEC-016). Secondary test pins WarehouseAdapter.from_profile dispatch to SnowflakeAdapter from the snowflake_password.yml fixture. One-line doc note added to docs/warehouse-adapter-ops.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-dqf.3: Quality gate — pin count_tokens consumption in Snowflake degrade tests Code-review pass 3 flagged that the two new degrade tests queued count_tokens expectations but never asserted full consumption — a drift to fewer engine calls would leave them unconsumed (extra calls already raise). Added assert_all_expectations_met() to both so the LLM-cost half stays load-bearing. Passes 1/2/4 clean. CodeRabbit skill unavailable in this environment. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-dqf.4: Patterns & Memory — record EXPLAIN deferral (#130) + degrade-verification pattern - Filed follow-up #130 for EXPLAIN-based Snowflake estimation (blocked on #118/#122). - docs/warehouse-adapter-ops.md: pointer to #130 in the non-BQ migration story. - .claude/rules/warehouse-adapters.md: pattern — verify each non-BQ adapter's graceful-degrade with its adapter-specific NotSupported error at engine + CLI level. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #123: Address PR review feedback — tighten _install_estimate_patches adapter type Copilot: type the adapter param/local as WarehouseAdapter | None (not object) to preserve static checking — both BigQueryAdapter and SnowflakeAdapter implement the ABC. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add super plan for #122: Snowflake sampling + materialise_sample Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: mark #122 plan published (PR #129) * chore: devolve #122 plan to beads (epic bd_1-scaffolding-lqr) * bd_1-scaffolding-lqr.1: shared sample-id seam + Snowflake exception mapper (#122 US-001) Foundations for Snowflake sampling — pure plumbing, no adapter behaviour. - Relocate _compute_run_id / _canonical_partition_filter / _hash_session_id VERBATIM out of adapters/bigquery.py into a new shared signalforge/warehouse/_sample_id.py (DEC-008) so BigQuery and Snowflake produce byte-identical run_ids. BigQuery imports them; recipe bytes unchanged (all existing materialise-sample / prune snapshots byte-identical). - Add `description` to _SnowflakeCursorProtocol (DEC-010, DB-API 2.0 column descriptors) so the future adapter builds dict rows without a DictCursor. - Add map_snowflake_exception(exc, *, context) to _snowflake_client.py (DEC-009): lazy snowflake.connector.errors import (one-shim-per-vendor), minimal v0.2 taxonomy — ProgrammingError -> QuerySyntaxError, auth/forbidden -> WarehouseAuthError, else passthrough. Mirrors map_bq_exception shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-lqr.2: FakeSnowflakeClient + SnowflakeAdapter connection lifecycle (#122 US-002) Wire the SnowflakeAdapter connection seam and fail-soft __exit__ cleanup, plus a hand-rolled FakeSnowflakeConnection test double. Sampling / materialise / run_test_sql stay NotImplementedError (US-003/US-004). - tests/warehouse/_fake_snowflake.py: explicit FakeSnowflakeConnection + cursor satisfying the _SnowflakeClientProtocol / _SnowflakeCursorProtocol; expect_execute() queues round-trips, close_raises drives the cleanup-failure path. No MagicMock (testing-signal.md). - adapters/snowflake.py: injectable connection= kwarg + lazy _get_connection() build (DEC-001), connection-bound _active_session state (DEC-002), and a fail-soft _cleanup_active_session() that closes the connection (reaping session-scoped temp tables), swallows-and-warns on failure with an operator-actionable Snowflake-shaped WARNING naming the raw session_id + server-side reap fallback (DEC-003/DEC-014); success logs the hashed id only. __repr__ unchanged (account + warehouse only). - tests: new test_snowflake_lifecycle.py covers injection, lazy build, __repr__ redaction regression, single-close + idempotent second __exit__, hashed-id INFO, and the WARNING failure path; updated the stub's context-manager test for the now-real cleanup. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-lqr.3: implement deterministic SnowflakeAdapter.sample_rows (#122 US-003) Implements deterministic hash-mod sampling for Snowflake, mirroring BigQuery's sample_rows semantics (DEC-005/006/009/010): - _get_num_rows: ROW_COUNT from INFORMATION_SCHEMA.TABLES, case-insensitive, escaped string literals, CURRENT_DATABASE() fallback when project is None. - Fail-loud sizing identical to BigQuery: UnknownTableSizeError / SamplingRequiresPartitionFilterError; bucket=1000 fallback on unknown size with a filter; bucket=max(num_rows//n,1) otherwise. _LARGE_TABLE_THRESHOLD re-declared (100M) to avoid importing the BigQuery adapter. - SQL reuses SNOWFLAKE_DIALECT.sample_row_hash_expr (ABS(HASH(*))) + the timestamp/date literal templates so it stays byte-consistent with the prune compiler's sample CTE; per-component double-quoting; ORDER BY for deterministic LIMIT truncation. - Tuple fetchall() rows shaped into dicts via cursor.description (DEC-010); SDK exceptions routed through map_snowflake_exception (DEC-009). column_stats / run_test_sql / materialise_sample stay NotImplementedError. Removed the now-stale test_sample_rows_raises_not_implemented stub test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-lqr.4: SnowflakeAdapter materialise_sample + run_test_sql (#122 US-004) Implement the two remaining sampling-consumer surfaces on SnowflakeAdapter: - materialise_sample: CTAS into a session-scoped TEMPORARY TABLE (_sf_sample_<run_id>, run_id from the shared _sample_id recipe so it is byte-identical to BigQuery; DEC-008), colocated with + fully-qualified via the source DB/schema (DEC-007), deterministic MOD(ABS(HASH(*)),bucket)<1 + ORDER BY ABS(HASH(*)) read from SNOWFLAKE_DIALECT (DEC-006), pins the live connection as _active_session so a follow-up run_test_sql reaches the temp table (DEC-002). SDK failures route through map_snowflake_exception then wrap in MaterialisationFailedError (DEC-009); INFO log emits the hashed session id only (DEC-003). n<=0 -> ValueError. - run_test_sql: validate_test_sql -> COUNT(*) wrap (ARRAY_AGG(OBJECT_CONSTRUCT(*)) sample capture when capture_failures>0) on the active connection, returns a typed TestResult; case-insensitive alias resolution for Snowflake's upper-folded FAILURES/SAMPLES columns (DEC-004). - Factor the fail-loud sizing pathway into a shared _resolve_sample_bucket helper reused by sample_rows + materialise_sample (DEC-005, no duplicated logic). column_stats stays NotImplementedError (#118); estimate_query_bytes stays the ABC not-supported degrade (#123). Remove the now-stale run_test_sql / materialise_sample NotImplemented stub tests; add tests/warehouse/test_snowflake_materialise.py pinning CTAS shape, fully-qualified temp TableRef, _active_session pinning, run_test_sql reachability on one connection, the #116 substitution AC (compiler emits the temp-table name, NOT the source), and the failure modes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-lqr.5: Quality gate — fix bugs from code review (#122 US-005) - BUG: _cleanup_active_session no longer nulls self._connection (discarded an injected fake / forced a real lazy-rebuild on re-entry; mirrors BigQuery which resets only _active_session, never the client). - #116 substitution test now exercises the bypassable path: CandidateTestCustomSQL with {{ this }} at scope="full" (not_null trivially FROMs table_ref and can never bypass substitution). - Add num_rows==0 sizing test (was untested; mirrors the None pathway). - Fix stale/misleading docstrings (module/class "still raises", _session_started_at "auto-expire" text Snowflake never emits). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-lqr.6: Patterns & Memory — document Snowflake sampling conventions (#122 US-006) - warehouse-adapters.md: new "Snowflake sampling + connection-bound session" section (connection-as-session, dialect-field reuse, INFORMATION_SCHEMA sizing, shared _sample_id hoist, Snowflake-shaped fail-soft cleanup); update the cleanup-boundary forward-note + Reference. - CLAUDE.md: public-API surface note for the implemented sampling methods. - plan: mark complete. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: close patch-coverage gaps on Snowflake adapter (#122) Codecov flagged 15 uncovered lines in adapters/snowflake.py (93%). Add tests for the previously-untested branches — all now 100%: - _execute / _execute_to_dicts / run_test_sql SDK-error mapping: both the mapped (ProgrammingError -> QuerySyntaxError) and unmapped-passthrough (mapped is exc -> raise original) branches, on size + sample + count queries. - _rows_to_dicts dict-row passthrough (DictCursor-style mapping rows). - materialise_sample with a PartitionFilter (CTAS WHERE rendering). - _get_connection lazy real-client build when no connection= is injected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #122: fix invalid CURRENT_DATABASE() namespace in INFORMATION_SCHEMA lookup Copilot PR-review catch: CURRENT_DATABASE().INFORMATION_SCHEMA.TABLES is invalid Snowflake — CURRENT_DATABASE() is a scalar function, not a namespace qualifier, and would fail on a live account (the hand-rolled fake matched it by regex; live execution is deferred to #124). When table.project is None, leave the lookup unqualified (INFORMATION_SCHEMA.TABLES), which Snowflake resolves against the session's current database. Update the test + rule + plan notes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cut the per-issue changelog prose and the Public API / v0.2-v0.3 addition blocks that duplicated CHANGELOG.md, plans/super/ DEC records, and the auto-loaded .claude/rules/ files. Replaced with an architecture map (subpackage -> role -> rules file) and pointers to where history lives. Dropped the stale "Conventions to set when scaffolding lands" section. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
#132) * Add super plan for #130: Snowflake estimate_query_bytes (EXPLAIN-based) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #130: devolve plan to beads (epic bd_1-scaffolding-2wl + 8 tasks) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-2wl.1: #130 US-001 — EstimateUnavailableError typed error + tier-3 registration Add EstimateUnavailableError(WarehouseError) for the "estimation seam ran but produced no usable figure for THIS query" case (DEC-003), distinct from EstimateNotSupportedError ("adapter does no estimation at all"). Keyword-only `detail` rendered repr-safe via _format_value; locked-verbatim default_remediation pointing at the price-only-preview degrade. Exported from warehouse/__init__.py and registered in cli/_helpers._EXCEPTION_TO_EXIT_CODE at tier 3 (external-dep). Tests: subclass check, __str__ renders message + remediation, locked-verbatim remediation, distinct-from-not-supported, map_exception_to_exit_code == 3; scan-7 green with the new concrete. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: prune the four largest .claude/rules files to guidance-only Strip per-issue genealogy (DEC suffixes, issue/PR citations, dated status notes) and multi-sentence retellings of how decisions evolved from the four largest rules files; that history lives in plans/super/ and CHANGELOG.md. Every load-bearing invariant, locked verbatim string, taxonomy, config-field list, function/seam name, and AST-scan / grep-gate / drift-detector contract is preserved. ~287 lines removed across warehouse-adapters, prune-engine, cli-layer, and diff-renderer. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs: move per-layer rules out of .claude/ auto-load into docs/rules/ The 14 .claude/rules/*.md files were auto-loaded into every Claude Code session (~48K tokens of project instructions), dwarfing CLAUDE.md and triggering the large-context warning even after CLAUDE.md was trimmed. They are reference contracts meant to be read per-layer on demand, not held in context globally. Move them to docs/rules/ (out of the auto-loaded .claude/ tree) and add rules/ to mkdocs exclude_docs so they stay off the published site (mirrors research/). Update the ~52 live docstring/comment pointers in src/, tests/, and docs/ to the new path, and correct CLAUDE.md (drop the now-false "auto-loaded into context" claim; the architecture-map table is the layer -> file lookup). plans/super/ ADR pointers are left as historical record. No runtime path dependency existed; all references are prose. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * bd_1-scaffolding-2wl.2: #130 US-002 — pure _parse_explain_json_bytes parser + fixtures Add module-level pure function _parse_explain_json_bytes(cell) -> int in warehouse/adapters/snowflake.py: navigates GlobalStats.bytesAssigned from an EXPLAIN USING JSON result cell (str JSON or pre-parsed dict), returning a non-negative int. Raises typed EstimateUnavailableError (imported from warehouse.errors, US-001) on unparseable JSON, non-object document, missing/ non-mapping GlobalStats, missing bytesAssigned, non-int/bool/negative value — never fabricates a number, never returns 0 for a missing field (DEC-002). Pure: no connection, no logging. Wiring into estimate_query_bytes is US-003. Ship hand-crafted fixtures under tests/fixtures/warehouse/snowflake/ (explain_using_json_sample.json @ 104857600 bytes + explain_using_json_no_stats.json) plus a README documenting they are hand-crafted (workers can't reach live Snowflake) and the maintainer regen command. Engineered determinism: the parse test asserts the int EQUALS the fixture's known bytesAssigned. DEC-001/002/006. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-2wl.3: #130 US-003 — SnowflakeAdapter.estimate_query_bytes EXPLAIN override Override estimate_query_bytes(sql)->int: validate_test_sql first, then EXPLAIN USING JSON <validated-sql> via a new _execute_scalar cursor helper (no TableRef in scope), parse GlobalStats.bytesAssigned via the existing _parse_explain_json_bytes pure fn. SDK exceptions route through map_snowflake_exception (DEC-005); empty result -> EstimateUnavailableError. No snowflake.connector import in the adapter. Docstrings updated (estimate now implemented, no longer inherits ABC degrade). DEC-001/004/005/008. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-2wl.5: #130 US-005 — gated live Snowflake EXPLAIN estimate test + regen note Add tests/warehouse/test_snowflake_estimate_live.py: a @pytest.mark.snowflake- gated live test that drives a real SnowflakeAdapter through estimate_query_bytes against a live warehouse (EXPLAIN USING JSON), certifying the committed fixture's GlobalStats.bytesAssigned shape. Belt-and-suspenders gating: marker (deselected by default addopts) + runtime _skip_reason naming each missing prerequisite (SF_RUN_SNOWFLAKE=1 + SNOWFLAKE_ACCOUNT/USER/PASSWORD/WAREHOUSE). Asserts shape + non-negativity, never an exact planner value (DEC-006). Extend the fixture README regen note with the exact maintainer run command and a pointer to the live test module. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-2wl.4: #130 US-004 — rewrite #123 degrade tests for real EXPLAIN estimate #130 US-003 overrides SnowflakeAdapter.estimate_query_bytes with a real EXPLAIN-based implementation, making the three #123 tests that asserted EstimateNotSupportedError RED. DEC-007: rewrite (not delete) each into a happy path (real EXPLAIN bytes via an injected FakeSnowflakeConnection) plus a degrade path (no-stat EXPLAIN -> EstimateUnavailableError, keyed on the specific class name per the #123 rule note). - test_snowflake_stub.py: replace test_estimate_query_bytes_raises_not_supported with test_estimate_query_bytes_returns_explain_bytes + test_estimate_query_bytes_degrades_on_missing_stat; refresh the stale module docstring claiming the ABC-default inheritance. - test_estimate_engine.py: split the Snowflake degrade test into test_estimate_reports_real_bytes_for_snowflake_explain (no degrade) and test_estimate_degrades_on_snowflake_explain_missing_stat. - test_generate_estimate.py: split the CLI Snowflake test into ..._reports_real_bytes (real estimate) and ..._degrades_to_exit_zero (EstimateUnavailableError); refresh the stale helper docstring. Tests only; no src/ changes. Full suite green (2368 passed). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Revert "docs: move per-layer rules out of .claude/ auto-load into docs/rules/" This reverts commit 5569aa2. * Revert "docs: prune the four largest .claude/rules files to guidance-only" This reverts commit aa0f614. * bd_1-scaffolding-2wl.6: #130 US-006 — 5-surface docs parity for Snowflake EXPLAIN estimate Correct the now-stale "Snowflake inherits the estimate degrade" claims across the two doc surfaces that carried them (DEC-009 of #130): - docs/warehouse-adapter-ops.md § Query-bytes estimation: Snowflake is now a real EXPLAIN USING JSON estimate (GlobalStats.bytesAssigned) alongside BigQuery's dry_run; documents the EstimateUnavailableError degrade (EXPLAIN ran but no parseable figure) and the planner-estimate accuracy caveat. New error-reference row for EstimateUnavailableError; EstimateNotSupportedError row narrowed to the Postgres stub. - .claude/rules/warehouse-adapters.md: #123 note corrected — Snowflake's estimate_query_bytes graduated from the ABC degrade to a real EXPLAIN override in #130 (was deferred-to-#130); the #119/#122 historical notes reframed as past-state with explicit "graduated in #130" pointers. CLAUDE.md (slim version on disk) carries no estimate / public-API enumeration, so no stale claim to correct there. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-2wl.7: #130 Quality Gate — adapter-derived estimate source label + marker description Code-review pass 3 caught the --estimate renderer hardcoding "(BigQuery dryRun)" on the bytes-per-row line; now that SnowflakeAdapter returns a real EXPLAIN estimate it reached that branch and mislabelled the source. Add a warehouse_estimate_source field derived from the adapter class, render it, and pin "Snowflake EXPLAIN" in the happy-path test. Also broaden the stale `snowflake` pytest-marker description to cover the gated live EXPLAIN test, and soften the live-test opt-in docstring. Quality Gate: 4 code-review passes + CodeRabbit (no findings in the #130 changeset; CodeRabbit's 2 findings were in an untracked docs/temp scratch dir left out of this branch). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-2wl.8: #130 Patterns & Memory — generalised graceful-degrade graduation recipe Distil the reusable per-adapter graduation pattern into warehouse-adapters.md (pure-fn parse + hand-crafted fixture + gated live test; rewrite the prior phase's degrade tests rather than deleting them) so the next adapter (Postgres EXPLAIN) inherits it. Memory note added for the orchestrator git-add-all gotcha. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #130: cover _execute_scalar passthrough re-raise (Codecov patch gap) snowflake.py:836 (the `mapped is exc` bare-raise arm of _execute_scalar) was the one #130-introduced line missing patch coverage. Add a test driving estimate_query_bytes with a fake whose EXPLAIN raises a non-connector RuntimeError: map_snowflake_exception returns it unchanged, so the original propagates as-is. snowflake.py now 100%. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #130: address PR review feedback (Copilot) - _execute_scalar: close the cursor in a finally so repeated estimate calls don't leak server-side cursors (the cursor I introduced in this PR). - _parse_explain_json_bytes docstring: clarify an explicit bytesAssigned=0 is a valid estimate; the "never 0" rule only forbids fabricating a 0 fallback when the stat is missing/unparseable (was internally inconsistent with the tests). - semicolon-reject test: narrow pytest.raises(Exception) -> QuerySyntaxError so it can't pass on an unrelated exception. - fixtures README: keep the fully-qualified parser path on one line so the Markdown inline code span renders. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #130: address CodeRabbit review feedback - Promote SnowflakeAdapter to the public signalforge.warehouse surface (mirrors BigQueryAdapter; CLAUDE.md already lists it as public) + __all__ sorted; migrate all 8 Snowflake test files to the public import. - _execute_scalar: normalise a DictCursor-style mapping row to its first cell (was returning the whole row dict → false degrade) + test. - Switch flagged EstimateUnavailableError imports to the package surface. - test_generate_estimate: annotate module-level _SNOWFLAKE_FIXTURES: Path. - live test: keep the genuinely-private _parse_explain_json_bytes white-box import (no public seam) with a justifying comment; SnowflakeAdapter public. - docs: resolve the "v0.2 ships BigQuery override only" contradiction (Snowflake overrides too; Postgres still inherits) + fix MD038 trailing space in code span. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add super plan for #124: Snowflake test harness + gated live e2e + ops docs Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #124: devolve plan to beads — fill manifest, set phase devolved Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-88t.2: fakesnow adapter harness (execute + sqlglot-parse) Drive a real SnowflakeAdapter against an in-memory fakesnow connection so the adapter's OWN emitted SQL is validated offline against a Snowflake- flavoured engine. Mirrors tests/prune/test_compiler_fakesnow.py. EXECUTE (fakesnow can run): run_test_sql over engineered rows for all four rule semantics (not_null / unique / accepted_values / relationships), asserting failing-row SHAPE (>=1 vs ==0), never HASH() value-equality; plus _get_num_rows over INFORMATION_SCHEMA.TABLES.ROW_COUNT (present + absent). PARSE-ONLY (fakesnow cannot run, degraded to sqlglot Snowflake-dialect parse, each with an inline comment naming the gap): the hash-mod sample_rows SQL (variadic HASH(*)), the materialise_sample CTAS (qualified TEMPORARY table + HASH(*)), and the capture_failures ARRAY_AGG(OBJECT_CONSTRUCT(*)) wrap. Module-level pytestmark = pytest.mark.snowflake (deselected by default); runs green under uv run pytest -m snowflake --no-cov. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-88t.1: Full map_snowflake_exception taxonomy + offline tests Expand map_snowflake_exception to mirror map_bq_exception's coverage: split ProgrammingError into object-not-exist (errno 002003 / "does not exist") -> TableNotFoundError, invalid-identifier (errno 000904 / "invalid identifier") -> ColumnNotFoundError, residual -> QuerySyntaxError. Auth (ForbiddenError / auth-marker DatabaseError|OperationalError) -> WarehouseAuthError; everything unmapped -> passthrough. Table/Column split runs before the QuerySyntaxError fallthrough. Reuses existing WarehouseError subclasses only (no new error class), keeps the snowflake-connector import lazy inside the function body (confinement test stays green), and adds a private _extract_invalid_identifier regex helper mirroring _client._extract_unrecognized_column. The #124 full taxonomy drops the (table=...) detail enrichment the #122 minimal mapper appended to QuerySyntaxError; context now only feeds the Table/Column arms (aligns with map_bq_exception). Updated the stale test_sample_id.py assertion accordingly. Offline unit tests construct genuine sfe.ProgrammingError / DatabaseError / OperationalError / ForbiddenError instances and assert each arm. The new test file imports snowflake.connector.errors lazily inside each test (via a _sfe() helper) because a sibling test deletes the module from sys.modules without restoring it, which would stale a module-level import. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-88t.3: TPCH manifest seed + loads-only test #124 US-003. Hand-crafted dbt manifest seed describing one model over SNOWFLAKE_SAMPLE_DATA.TPCH_SF1.CUSTOMER, with engineered always-pass columns ('us' AS region, COALESCE(c_acctbal, 0) AS acctbal_safe) so the US-005 live e2e has a deterministic prune drop signal (mirrors the Austin bikeshare engineered-determinism pattern). - tests/fixtures/snowflake/: dbt_project.yml, profiles.yml, model SQL + sources.yml, hand-crafted target/manifest.json, _gen_manifest.py (idempotent generator), README.md (maintainer-only dbt parse regen note, documents why the seed is hand-crafted per testing-signal.md). - tests/warehouse/test_snowflake_seed_loads.py: loads-only validation in the DEFAULT suite (no marker, no env vars) asserting the seed loads via signalforge.manifest.load and resolves to TPCH_SF1.CUSTOMER. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-88t.5: Full generate-pipeline gated live e2e (TPCH_SF1, oneshot) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-88t.4: Warehouse+prune-only gated live e2e (materialised sample) Add tests/warehouse/test_snowflake_prune_live.py — a @pytest.mark.snowflake gated live e2e that creates a tiny engineered table in a writable Snowflake schema, prunes a hand-crafted not_null candidate under the materialised sample strategy, asserts an always-passes drop, and tears the table down in a finally. Belt-and-suspenders gating: marker (deselected by default addopts) + a _skip_reason() helper returning a distinct reason per missing prerequisite (SF_RUN_SNOWFLAKE=1, SNOWFLAKE_ACCOUNT/USER/PASSWORD/WAREHOUSE, plus the writable SNOWFLAKE_DATABASE/SNOWFLAKE_SCHEMA target). Drives prune_tests directly (no LLM, no generate CLI); engineered determinism via a literal 'austin' region column so not_null is mathematically always-pass. Module docstring carries the cost guidance (resource monitor first, XS warehouse, aggressive auto-suspend, env-var list, run command). Skips cleanly with no traceback in the default suite and under `uv run pytest -m snowflake --no-cov` when env vars are absent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-88t.6: Consolidated Snowflake ops docs + rules distillation Add a "Snowflake adapter (v0.2)" section to docs/warehouse-adapter-ops.md (install, profile keys + auth scope, dialect, connection-bound session + read-only-DB CTAS caveat, fail-soft cleanup, EXPLAIN estimate, full error taxonomy, cost guidance, offline harness) and extend the integration-tests section with the gated Snowflake invocation + env-var matrix. Distil the #124 conventions into .claude/rules/warehouse-adapters.md (full map_snowflake_exception taxonomy reusing existing errors; read-only-DB sample-strategy split; fakesnow-execute + sqlglot-parse adapter harness) with a Reference pointer to the plan. No mkdocs.yml change (file already in nav). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-88t.7: Quality gate — fix US-005 always-passes bug from code review Code review (pass 2 of 4) caught a correctness bug in the TPCH seed + full- pipeline live e2e: the seed model aliased to the read-only source SNOWFLAKE_SAMPLE_DATA.TPCH_SF1.CUSTOMER but declared RENAMED/engineered columns (region, acctbal_safe, customer_id) that do not exist on the real CUSTOMER table. Under oneshot sampling prune queries the source table directly, so every drafted not_null would compile to an "invalid identifier" → kept-without- evidence, never the always-passes drop the e2e asserts — the test could never pass live. Fix: declare only REAL, unrenamed TPCH columns (c_custkey, c_name, c_nationkey, c_phone, c_acctbal, c_mktsegment) and rely on the natural NOT NULL primary key c_custkey for the always-passes signal (mirrors the Austin bikeshare fixture's natural-NOT-NULL pattern, NOT engineered literals). Regenerated manifest.json via _gen_manifest.py (idempotent), updated the .sql, the loads-only test assertions, the smoke-test docstring/comments, and the README. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-88t.8: Patterns & Memory — correct engineered-determinism rule + CHANGELOG Correct the misleading guidance in .claude/rules/testing-signal.md § "Engineered determinism" that caused the #124 QG bug: it claimed the austin fixture ships an engineered literal column ('austin' AS region), but the real fixture (and now the TPCH seed) rely on NATURAL NOT NULL source columns, because under the source-as-model alias trick prune queries the SOURCE table directly — an engineered literal lives only in the never-executed raw_code. Document both fixture shapes (source-as-model → natural NOT NULL; materialised → literal/COALESCE valid). Add the #124 CHANGELOG entry under [Unreleased]. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #124: Address PR review — fix Snowflake adapter case-folding + review nits PRODUCTION FIX (Copilot threads 1-3, real latent #122 bug): SnowflakeAdapter._quote preserved identifier case while the prune compiler folds to UPPER. Under the materialised strategy the adapter CREATEd the temp table quoted-lowercase ("_sf_sample_<hex>") but the compiler REFERENCEd it folded-upper ("_SF_SAMPLE_<HEX>") — case-sensitive mismatch → the failing-rows SQL points at a table the CTAS never created; and a lowercase db/alias never resolves against a conventional uppercase Snowflake object. Fix: _quote (and _get_num_rows's db prefix) now fold-then-quote via a shared _fold helper mirroring the compiler's _fold_identifier. Updated the #122 sampling/materialise assertions accordingly. US-004's gated live materialised e2e is the first thing to exercise this path. Review nits: - close setup/teardown cursors in the prune-live e2e (Copilot) - run-unique engineered table name (uuid suffix) to avoid concurrent-run races (CodeRabbit) - serialize profiles.yml via yaml.safe_dump instead of interpolating env vars (CodeRabbit) - align the BigQuery integration-test command to `uv run pytest` (Copilot) - explicit type annotations on _gen_manifest.py fixture constants (CodeRabbit) - import Manifest from the public signalforge.manifest surface (CodeRabbit) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #124: Certify Snowflake live e2e — fix VARIANT samples parse, scope live tests to full Live-certified the gated @pytest.mark.snowflake suite against a real Snowflake warehouse (37 passed). Two more live-only adapter bugs surfaced + addressed: - run_test_sql capture-failures: ARRAY_AGG(OBJECT_CONSTRUCT(*)) comes back from the connector as a JSON-string VARIANT, not a Python list — json.loads it before building sample-failure dicts (fakesnow returned a list, so offline passed). Production fix in snowflake.py. - Live Snowflake sample-mode prune is non-functional: HASH(*) is invalid in WHERE/ORDER BY (002079; valid only in SELECT projection), and oneshot routes the sample row-count through a BigQuery-only _get_client. Both are #121/#122 bugs (beads bd_1-scaffolding-cdp, bd_1-scaffolding-tft), out of #124 scope. So both live e2e tests now run at safety: schema-only + prune.scope: full — the combination that works on live Snowflake today — and pass: prune-live drops an always-passes not_null; the full generate pipeline against TPCH_SF1 exits 0 with an always-passes drop + aggregate_complete. Documented the known limitations in the ops doc + rule file (live-harness findings: fakes hide vendor-shape bugs); CHANGELOG Fixed entries for the case-folding + VARIANT-samples fixes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #124: Address Copilot review — reconcile scope:full docstrings/docs + representative fakesnow fixture Six Copilot threads, all valid (no false positives) — five are stale text my own scope:full re-scoping + fold fix left behind; one is a real test-quality fix: - fakesnow adapter harness: created the DB unquoted-lowercase but referenced it quoted — passed only via DuckDB's case-insensitivity, not representative of real (case-sensitive) Snowflake. Now creates + references fixture objects in the UPPER-folded namespace the adapter's _quote actually emits; TableRef keeps the lower-cased dbt-style input the adapter folds. - prune-live: _skip_reason + _quoted_table docstrings + env-var notes said "materialised sample"/"CTAS" — the test is scope:full (no materialise). Fixed. - smoke: module docstring + invariant explanation + cost note said "oneshot MANDATORY / pins sample_strategy: oneshot" — the test pins scope: full (sample-mode non-functional live). Fixed. - ops doc: removed the now-contradictory "use prune.sample_strategy: oneshot" recommendation (the same section declares oneshot broken); corrected the temp-table example to the folded "..."."_SF_SAMPLE_<RUN_ID>" shape; flagged the stale v0.2→v0.3 migration note with a Snowflake caveat. Test fixtures + docstrings + docs only (no production change this round). Offline validation green (2389 passed, fakesnow harness 14 passed, ruff+pyright clean). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add super plan for #139: Snowflake projection-subquery sample shape Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #139: devolve plan to beads (epic bd_1-scaffolding-kay) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-kay.1: #139 US-001 Dialect sample-shape fields + render_sample_select helper Add sample_hash_in_projection / sample_hash_alias to the Dialect frozen dataclass (BigQuery/Postgres keep inline defaults; SNOWFLAKE_DIALECT sets projection=True). New stateless warehouse-layer helper render_sample_select switches inline-vs-projection on the boolean flag only (never dialect.name): inline reproduces the prune compiler's current sample-CTE body byte-for-byte; projection-subquery computes HASH(*) in an inner projection and references the alias in WHERE/ORDER BY with SELECT * EXCLUDE. Traces to DEC-001/002/003/004. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-kay.2: #139 US-002 wire compiler sample CTE to render_sample_select + regen Snowflake snapshots Replace _render_sample_cte's inline hash-mod string-building with a call to the shared signalforge.warehouse._sample_sql.render_sample_select helper (order_by_hash=False; the compiler CTE has no ORDER BY). Partition predicate stays rendered by the compiler's own _render_partition_filter and is passed as extra_where. Switches on the boolean Dialect.sample_hash_in_projection — no dialect.name branch, no warehouse-SDK import (import-guard green). BigQuery (inline) output is byte-identical — top-level compiled_sql/*.sql fixtures unchanged (the regression gate). The five Snowflake *_sample.sql fixtures regenerated to the projection-subquery form (SELECT * EXCLUDE (_sf_sample_hash) FROM (SELECT t.*, ABS(HASH(*)) AS _sf_sample_hash ...) WHERE MOD(_sf_sample_hash, n) < 1) so HASH(*) is computed in the projection, never a predicate. sqlglot snowflake-dialect parse guard passes on the new form. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-kay.3: #139 US-003 Snowflake adapter sample_rows + materialise_sample use projection-subquery shape Replace the inline MOD(ABS(HASH(*)), n) < 1 + ORDER BY ABS(HASH(*)) building in SnowflakeAdapter.sample_rows and materialise_sample with the shared render_sample_select(..., order_by_hash=True) helper (US-001). Snowflake's HASH(*) is invalid in a WHERE/ORDER BY predicate (002079); the helper's projection-subquery branch computes the hash once in an inner SELECT t.*, ABS(HASH(*)) AS _sf_sample_hash and the outer clauses reference the alias, with SELECT * EXCLUDE (_sf_sample_hash) stripping the helper column so returned rows / the materialised temp table carry only source columns. Partition filters stay rendered by the adapter's own _render_partition_filter and pass to the helper as extra_where (no name branch, no hard-coded HASH(*)). Update the fakesnow/sqlglot adapter guards plus the test_snowflake_sampling and test_snowflake_materialise unit assertions to the new shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-kay.4: #139 US-004 re-enable live materialised prune e2e at scope=sample + 5-surface docs Flip test_snowflake_prune_live.py to scope=sample + sample_strategy=materialised (exercising the #139 projection-subquery CTAS); rename to test_prune_drops_always_passes_not_null_live_materialised_sample. Two-gate discipline (marker + runtime _skip_reason) preserved; self-skips cleanly offline. DEC-004 primary/fallback note added to the module docstring. 5-surface graduation: warehouse-adapter-ops.md (remove HASH(*)-in-predicate limitation; materialised sample-mode now works), .claude/rules/warehouse-adapters.md (new Dialect fields + mark bd_1-scaffolding-cdp FIXED), .claude/rules/prune-engine.md (compiler dialect field list + render_sample_select delegation). Live certification still pending (maintainer-run with SF_RUN_SNOWFLAKE=1). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-kay.5: #139 Quality gate — fix bugs from code review - harden the name-agnostic helper test with a symmetric inline-direction assertion (pins both branches against a dialect.name-based dispatch) - correct stale docstring/comment in test_e2e_snowflake_smoke.py: the HASH(*)-in-WHERE/ORDER-BY shape bug is FIXED by #139 for both sample strategies; the remaining scope=full requirement here is the read-only SNOWFLAKE_SAMPLE_DATA share (materialised) + oneshot's open row-count seam (bd_1-scaffolding-tft), not the shape bug Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-kay.6: #139 Patterns & Memory — finalise plan + capture dialect-shape lesson - prune-engine.md § "Adding a new vendor dialect": capture the generalised #139 lesson (a single inline SQL-fragment string can't express a clause- POSITION constraint; when the SQL *shape* differs, add a structural Dialect field + shared renderer; sqlglot parses but cannot certify warehouse acceptance, so the live-gated test is the real merge gate) - plans/super/139: add Outcome note (6 stories landed; QG 4 passes fixed E501 + stale e2e docstring; offline green 2394 passed / pyright 0 / -m snowflake 33 passed,4 live-skipped; DEC-006 live cert PENDING/maintainer) - verified US-004's rule/doc graduation already complete + coherent (no re-edit needed there) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #139: record live Snowflake certification (DEC-006 ✅, DEC-004 primary form confirmed) test_prune_drops_always_passes_not_null_live_materialised_sample passed against a real Snowflake warehouse (1 passed in 14.78s) — the materialised scope=sample path executes the SELECT * EXCLUDE projection-subquery CTAS and drops the always-passes test. Snowflake accepts ORDER BY of an EXCLUDE-d column, so the primary form stands (no DEC-004 fallback needed). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #139: address PR review — live-cert docstring + markdownlint fixes - test_snowflake_prune_live.py: DEC-004 note now records the live cert RESOLVED the ORDER-BY-of-EXCLUDE-d-column question in favour of the primary form (Copilot: stale "unresolved decision point" docstring) - docs/warehouse-adapter-ops.md: reword so the line no longer starts with "#139" (CodeRabbit/markdownlint MD heading false-trigger) - plans/super/139: add `sql` language tags to two fenced blocks (CodeRabbit) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* #140: vendor-neutral get_row_count seam for sample-bucket sizing Sample-scope prune sized the deterministic-sample bucket by reaching for a BigQuery-only `getattr(adapter, "_get_client")` crack in `prune.engine._resolve_sample_bucket`, so `prune.scope: sample` + `prune.sample_strategy: oneshot` raised `PruneError` on any non-BigQuery adapter (SnowflakeAdapter exposes `_get_num_rows`, not `_get_client`). Add a vendor-neutral `WarehouseAdapter.get_row_count(table) -> int | None` seam following the established `materialise_sample` / `estimate_query_bytes` pattern: a concrete ABC default raising the new typed `RowCountNotSupportedError` (tier 3), overridden by BigQuery (cached `_get_table().num_rows`) and Snowflake (`_get_num_rows` → INFORMATION_SCHEMA.TABLES.ROW_COUNT). Route `_resolve_sample_bucket` through it; the fail-loud unknown-count `PruneError` and BigQuery snapshot/behaviour parity are preserved. Tests: ABC default-raise; BigQuery + Snowflake overrides (offline fakes); engine vendor-neutral routing regression (a non-BQ adapter with no `_get_client` resolves the bucket); gated live `@pytest.mark.snowflake` oneshot prune e2e. Docs + rules updated to mark bd_1-scaffolding-tft fixed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #140: Address PR review feedback - engine: fail-loud message reports actual observed count (None vs 0) - test stub: type _RowCountOnlyAdapter methods instead of suppressing no-untyped-def - docs: resolve conflicting oneshot guidance (earlier paragraph still said blocked); note prune-live now covers materialised + oneshot Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…145) Add docs/snowflake-e2e-setup.md — a contributor-facing walkthrough mirroring docs/e2e-smoke-test.md: cost guardrails first (resource monitor, XS warehouse, auto-suspend), prerequisites, .env-based local env, a profiles.yml Snowflake target, and the `pytest -m snowflake --no-cov` run command for both the offline fakesnow and live tiers. Add a committed .env.example template (placeholders only) listing the gate vars the live path reads (SF_RUN_SNOWFLAKE + account/user/ password/warehouse, ANTHROPIC_API_KEY for the full-stack e2e, optional role/database/schema). .env stays gitignored. Wire the doc into README (next to the BigQuery e2e pointer) and the MkDocs nav (after End-to-End Smoke Test). mkdocs build stays clean. Closes #138. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…147) * #144: tolerate LLM prose preamble before JSON in draft/grade parsers claude-sonnet-4-6 reproducibly narrates a reasoning preamble before the JSON object on the business-rules drafting path, so the strict parse failed at line 1. The obvious guardrail — an assistant-turn JSON prefill — is unavailable: the API rejects it with HTTP 400 "This model does not support assistant message prefill." The parser is therefore the only place a JSON-only guarantee can live (prompts are advisory). Add signalforge._common.json_payload.extract_json_payload: decode at the FIRST { or [ via json.raw_decode, returning that span and discarding a leading preamble + trailing content. Decode the first candidate ONLY — a truncated outer object must not be silently rescued by a complete inner fragment, so on first-candidate failure the text is returned unchanged and the strict parser raises the normal JSON error with the right excerpt. - draft/parser.py: extract before model_validate_json; error envelopes keep the original raw_text (preamble included) so incident reports are honest. - grade/parser.py: extract after _strip_code_fence (shares the helper). - tests: helper unit matrix (preamble, fence, trailing, array, truncated-not- rescued, no-json passthrough) + draft/grade parser preamble regressions + a pure-prose-still-raises check. - llm-drafter.md / grade-layer.md: document the contract and why prefill (option 1) and prompt hardening (option 3) were not used. Full suite: 2416 passed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #144: address PR review feedback - grade/parser.py: fix stale module docstring that still claimed the parser does not extract JSON from prose (it now does, via extract_json_payload). - tests/_common/test_json_payload.py: mark `unit` (not `draft`) to match CONTRIBUTING.md's unit/integration/error convention + sibling _common tests. - grade-layer.md: clarify the decode rule is "first structural char ({ or [)", not object-only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds Snowflake adapter/dialect, custom_sql business-rule tests and proposed .sql outputs, a new ingest layer, and a read-only prune-existing CLI; refactors compiler for dialects, updates CI to Python 3.13, bumps versions/docs/tests accordingly. Changesv0.3.0 Feature Set
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Manifest
participant Ingest
participant Warehouse
participant Diff
User->>CLI: signalforge prune-existing <model> --schema path
CLI->>Manifest: load manifest
CLI->>Ingest: read_schema + read_test_files
Ingest-->>CLI: candidate, skipped
CLI->>Warehouse: prune_tests(candidate)
Warehouse-->>CLI: PruneResult
CLI->>Diff: render_diff(existing_schema, PruneResult)
Diff-->>CLI: DiffReport (yaml + proposed_test_files)
CLI-->>User: stdout diff, optional sidecar
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related issues
Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
⚔️ Resolve merge conflicts
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
docs/diff-ops.md (1)
80-80:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix
DiffReport.audit_schema_versionin the public API descriptionLine 80 still documents
DiffReport.audit_schema_version: Literal[2], but the sidecar contract is now3. This should be updated to avoid downstream parser/version-gating mistakes.🤖 Prompt for 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. In `@docs/diff-ops.md` at line 80, Update the public API docs to reflect the bumped audit schema version: change the documented type for DiffReport.audit_schema_version from Literal[2] to Literal[3], and ensure the surrounding description (the note about external sidecar consumers gating on >= value) mentions the new contract value 3 so downstream parsers/version-gating use the correct audit_schema_version; update any adjacent examples or mentions of "bumped 1 → 2 in issue `#50`" if necessary to reference the correct bump history for clarity.docs/audits.md (1)
22-22:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate diff sidecar schema version in the artefact table
Line 22 still says
audit_schema_version: 2for.signalforge/diff.json, but Line 214 documents the bump toLiteral[3] = 3. Please align this row to3to avoid consumers gating on the wrong version.🤖 Prompt for 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. In `@docs/audits.md` at line 22, Update the table entry for `.signalforge/diff.json` so its top-level schema indicates `audit_schema_version: 3` (currently `2`); locate the row referencing `.signalforge/diff.json` and change the documented `audit_schema_version` value from 2 to 3 to match the type bump (Literal[3] = 3) described later in the file and referenced by `tests/diff/test_drift_detector.py`.src/signalforge/cli/generate.py (1)
313-320:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the
--estimatehelp text for Snowflake support.This string still says the estimate path uses “BigQuery dryRun only”, but the estimator now also reports Snowflake-backed estimates. The current help text will mislead users trying the new adapter.
Suggested fix
write_group.add_argument( "--estimate", action="store_true", help=( "Print pre-flight cost estimate (uses count_tokens + " - "BigQuery dryRun only; no billable Anthropic or warehouse " + "warehouse estimate primitives such as BigQuery dryRun or " + "Snowflake EXPLAIN; no billable Anthropic or warehouse " "calls); mutually exclusive with --write / --dry-run." ), )🤖 Prompt for 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. In `@src/signalforge/cli/generate.py` around lines 313 - 320, Update the help text for the "--estimate" argument in the write_group.add_argument call so it no longer states "BigQuery dryRun only" and instead references both BigQuery and Snowflake (or "warehouse dry-run for BigQuery and Snowflake") as supported backends; edit the help string assigned to the "--estimate" argument in generate.py (the write_group.add_argument block for "--estimate") to mention Snowflake-backed estimates and keep the rest of the wording about count_tokens, dry-run semantics, and mutual exclusivity with --write/--dry-run unchanged.
🧹 Nitpick comments (1)
src/signalforge/diff/_renderers.py (1)
964-989: ⚡ Quick winEscape
clean_pathbefore Markdown heading emission
_render_test_files_sectionassumesproposed.pathis always slug-safe, but this renderer can receive externally sourcedDiffReportdata. Emitting rawclean_pathinside### `...`can break heading rendering or allow markdown injection for malformed paths.Suggested hardening
for proposed in report.proposed_test_files: clean_path = strip_ansi_escapes(proposed.path) clean_sql = "\n".join( strip_ansi_escapes(sql_line) for sql_line in proposed.sql.splitlines() ) fence = "`" * max(3, _longest_backtick_run(clean_sql) + 1) + escaped_path = escape_markdown_scalar(clean_path, in_table_cell=False) lines.append("") - lines.append(f"### `{clean_path}`") + lines.append(f"### {escaped_path}") lines.append("") lines.append(f"{fence}sql") lines.append(clean_sql) lines.append(fence)🤖 Prompt for 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. In `@src/signalforge/diff/_renderers.py` around lines 964 - 989, The code emits untrusted clean_path directly inside the Markdown inline-code heading in _render_test_files_section; to harden it, sanitize the path after ANSI stripping by converting it to a slug-safe filename (e.g. call the existing anchor_to_filename or a slugify helper) and use that sanitized value in the f"### `...`" line instead of clean_path; ensure you still call strip_ansi_escapes(proposed.path) first, then pass that result through anchor_to_filename (or equivalent) and use the returned safe string when appending the heading.
🤖 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 @.claude/rules/warehouse-adapters.md:
- Line 228: Replace the malformed heading token "`#124` is the epic's closing
test+docs ticket..." so Markdown linter accepts it: either remove the leading
"#" to make it a plain paragraph or insert a space after the "#" to make it a
valid ATX heading (e.g., "# 124 is the epic's closing test+docs ticket...");
locate the exact string "`#124` is the epic's closing test+docs ticket" in the
file and update accordingly.
In `@CLAUDE.md`:
- Around line 14-20: The fenced code block showing the pipeline diagram in
CLAUDE.md (the block beginning with the lines "model.sql + manifest + project
ctx" through "emit graded YAML + diff with per-artifact "why"") is missing a
language tag; update the opening fence to include a language identifier (for
example change ``` to ```text) so markdown linters and renderers treat it as
plain text and avoid lint/render warnings.
In `@docs/warehouse-adapter-ops.md`:
- Line 802: Update the error-table row for MaterialisationNotSupportedError so
it no longer claims “any non-BigQuery adapter in v0.2”; instead state it applies
only to adapters that still inherit the default raising implementation
(WarehouseAdapter.materialise_sample) and therefore do not implement
materialise_sample (exclude Snowflake, which now implements materialise_sample);
keep the suggested mitigations (set prune.sample_strategy: oneshot or wait for
v0.3) unchanged.
In `@plans/super/104-ingest-external-tests.md`:
- Around line 222-225: The fenced code block showing the dependency graph (the
block containing "US-001 ── US-002 ─┬─ US-003 ─┐" and "└─ US-004 ─┴─ US-005 ──
US-006 ── US-007 (Quality Gate) ── US-008 (Patterns & Memory)") lacks a language
tag and triggers MD040; update that fence to include the language identifier
text (i.e., change the opening ``` to ```text) so the block is treated as plain
text and markdown lint no longer flags it.
- Line 247: The second occurrence of the heading "## Detailed breakdown (Phase
4)" duplicates an earlier anchor and causes TOC ambiguity; update the duplicate
heading (the one at the bottom of the Phase 4 section) to a unique title (for
example "## Detailed breakdown (Phase 4) — continued" or "## Additional details
(Phase 4)"), or remove it if redundant, ensuring only one "## Detailed breakdown
(Phase 4)" heading remains.
In `@plans/super/105-prune-existing-cli.md`:
- Around line 99-101: The fenced code blocks containing the message "Skipped 3
unsupported tests: custom-or-generic-test×2, unsupported-test-type×1" are
missing a language tag and will fail MD040; update those triple-backtick fences
to include a language (e.g., use ```text) for the block shown and the other
similar block referenced around lines 173-177 so both code fences are labeled
(use `text` or an appropriate language).
In `@plans/super/124-snowflake-test-docs.md`:
- Around line 214-220: The fenced code block that contains the dependency graph
(the block starting with the triple backticks followed by the lines with US-001,
US-002, US-003, etc.) is missing a language tag; update that opening fence to
include a language identifier such as text (e.g., change ``` to ```text) so
markdownlint MD040 is satisfied and docs checks pass.
In `@README.md`:
- Around line 46-52: The fenced code block containing the ASCII flowchart (the
block that begins with ``` and shows the boxed diagram lines like
"┌──────────────┐ ┌──────────────┐...") lacks a language tag; update its
opening fence to use ```text to satisfy markdownlint MD040 so the flowchart is
treated as plain text and the linter stops flagging it.
In `@src/signalforge/cli/prune_existing.py`:
- Around line 305-317: The _resolve_project_dir path resolution currently uses
Path(override).resolve() which can silently stop on symlink loops; change it to
resolve(strict=True) and handle OSError(ELOOP) (and other resolution errors) by
raising a CliPathError with the same message/remediation used for missing
dbt_project.yml so the CLI surfaces a user-facing error; update the logic around
the override variable in prune_existing._resolve_project_dir to attempt
candidate = Path(override).resolve(strict=True) and convert resolution
exceptions into the existing CliPathError.
In `@src/signalforge/ingest/anchor.py`:
- Around line 67-80: When validating tests, don't assume test.column is always
set — treat None as valid for model-level/custom_sql tests: in the column-scoped
loop (column.tests) only enforce test.column == column.name and membership in
model_columns when test.column is not None, and in the model/candidate loop
(candidate.tests) only check that test.column exists in model_columns when
test.column is not None; this preserves valid model-level custom_sql
(test.column is None) and avoids falsely flagging column-scoped custom_sql.
In `@src/signalforge/prune/compiler.py`:
- Around line 726-734: In _is_multi_table branch where you build partition_sql
via _render_partition_filter and create replacement for own_qualified, change
the current single-occurrence replacement to rewrite all self-references of the
model table in resolved_sql (not just the first). Locate the block using
variables own_qualified, resolved_sql and partition_filter and replace the
.replace(..., 1) behavior with a global replacement (or a regex-based global
replace with appropriate word-boundary anchoring for own_qualified) so every
occurrence of the model table is wrapped with the partitioned subquery.
- Around line 691-710: Those except branches (_RequiresFutureData and
_InvalidIdentifier returns) are currently using only the exception class name in
the reason; change them to include the exception message/details (e.g., str(exc)
or exc.args[0]) so the prune audit shows which ref()/source()/Jinja element
failed. Specifically, update the handlers for
RefNotFoundError/SourceNotFoundError (returning _RequiresFutureData),
AmbiguousRefError (returning _InvalidIdentifier), and TemplateResolutionError
(returning _InvalidIdentifier) to include the exception text alongside the
existing context string (for example: reason=f"...: {type(exc).__name__}:
{str(exc)}"). Ensure each returned reason remains a concise one-line
explanation.
In `@src/signalforge/prune/engine.py`:
- Around line 897-914: The empty-candidate fast path returns before
`_validate_trusted_models`, allowing typos in `trusted_models` to pass silently;
fix by invoking `_validate_trusted_models(...)` (using the same arguments passed
into the surrounding function — e.g. `model` and `trusted_models`) before the
`if not pairs:` early-return and keep the existing
`_maybe_emit_kept_rate_warning` and `PruneResult` return logic unchanged so
validation always runs at entry even when there are zero candidates.
In `@src/signalforge/warehouse/adapters/snowflake.py`:
- Around line 546-555: The _execute and _execute_to_dicts helpers are leaving DB
cursors open; update both functions (the blocks creating cursor =
self._get_connection().cursor()) to ensure the cursor is closed on all paths by
using a try/finally (or a context manager) that calls cursor.close() in the
finally, and keep the existing exception mapping logic
(map_snowflake_exception(...)) intact so you still map and re-raise mapped
exceptions (raise mapped from exc) after closing the cursor; make sure
successful returns (e.g., list(cursor.fetchall()) and whatever _execute_to_dicts
returns) also close the cursor before returning.
- Around line 275-283: The Snowflake auth parameters private_key_path,
private_key_passphrase, and authenticator captured on the adapter are not
forwarded to make_real_client; update the client creation call in the Snowflake
adapter where self._connection is set (the make_real_client(...) invocation) to
pass these parameters (e.g., private_key_path=self._private_key_path,
private_key_passphrase=self._private_key_passphrase,
authenticator=self._authenticator) so profile-based auth modes are honored.
---
Outside diff comments:
In `@docs/audits.md`:
- Line 22: Update the table entry for `.signalforge/diff.json` so its top-level
schema indicates `audit_schema_version: 3` (currently `2`); locate the row
referencing `.signalforge/diff.json` and change the documented
`audit_schema_version` value from 2 to 3 to match the type bump (Literal[3] = 3)
described later in the file and referenced by
`tests/diff/test_drift_detector.py`.
In `@docs/diff-ops.md`:
- Line 80: Update the public API docs to reflect the bumped audit schema
version: change the documented type for DiffReport.audit_schema_version from
Literal[2] to Literal[3], and ensure the surrounding description (the note about
external sidecar consumers gating on >= value) mentions the new contract value 3
so downstream parsers/version-gating use the correct audit_schema_version;
update any adjacent examples or mentions of "bumped 1 → 2 in issue `#50`" if
necessary to reference the correct bump history for clarity.
In `@src/signalforge/cli/generate.py`:
- Around line 313-320: Update the help text for the "--estimate" argument in the
write_group.add_argument call so it no longer states "BigQuery dryRun only" and
instead references both BigQuery and Snowflake (or "warehouse dry-run for
BigQuery and Snowflake") as supported backends; edit the help string assigned to
the "--estimate" argument in generate.py (the write_group.add_argument block for
"--estimate") to mention Snowflake-backed estimates and keep the rest of the
wording about count_tokens, dry-run semantics, and mutual exclusivity with
--write/--dry-run unchanged.
---
Nitpick comments:
In `@src/signalforge/diff/_renderers.py`:
- Around line 964-989: The code emits untrusted clean_path directly inside the
Markdown inline-code heading in _render_test_files_section; to harden it,
sanitize the path after ANSI stripping by converting it to a slug-safe filename
(e.g. call the existing anchor_to_filename or a slugify helper) and use that
sanitized value in the f"### `...`" line instead of clean_path; ensure you still
call strip_ansi_escapes(proposed.path) first, then pass that result through
anchor_to_filename (or equivalent) and use the returned safe string when
appending the heading.
🪄 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: b96de05b-39fe-4a9c-88fa-4ddedb9e61b5
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (191)
.claude/rules/business-rule-tests.md.claude/rules/ci-supply-chain.md.claude/rules/cli-layer.md.claude/rules/grade-layer.md.claude/rules/ingest-layer.md.claude/rules/llm-drafter.md.claude/rules/prune-engine.md.claude/rules/python-build.md.claude/rules/testing-signal.md.claude/rules/warehouse-adapters.md.env.example.github/workflows/ci.ymlCHANGELOG.mdCLAUDE.mdCONTRIBUTING.mdREADME.mddocs/audits.mddocs/cli-ops.mddocs/diff-ops.mddocs/draft-ops.mddocs/ingest-ops.mddocs/prune-ops.mddocs/snowflake-e2e-setup.mddocs/warehouse-adapter-ops.mdmkdocs.ymlplans/super/104-ingest-external-tests.mdplans/super/105-prune-existing-cli.mdplans/super/116-business-rule-tests.mdplans/super/119-snowflake-skeleton.mdplans/super/120-snowflake-profile.mdplans/super/121-prune-snowflake-dialect.mdplans/super/122-snowflake-sampling.mdplans/super/123-snowflake-estimate-degrade.mdplans/super/124-snowflake-test-docs.mdplans/super/130-snowflake-estimate-explain.mdplans/super/139-snowflake-sample-shape.mdpyproject.tomlsrc/signalforge/__init__.pysrc/signalforge/_common/artifact_id.pysrc/signalforge/_common/json_payload.pysrc/signalforge/_common/path_safety.pysrc/signalforge/cli/__init__.pysrc/signalforge/cli/_estimate.pysrc/signalforge/cli/_helpers.pysrc/signalforge/cli/generate.pysrc/signalforge/cli/lint.pysrc/signalforge/cli/prune_existing.pysrc/signalforge/demo/__init__.pysrc/signalforge/demo/errors.pysrc/signalforge/diff/__init__.pysrc/signalforge/diff/_emitter.pysrc/signalforge/diff/_renderers.pysrc/signalforge/diff/_test_file_writer.pysrc/signalforge/diff/engine.pysrc/signalforge/diff/errors.pysrc/signalforge/diff/models.pysrc/signalforge/draft/config.pysrc/signalforge/draft/models.pysrc/signalforge/draft/parser.pysrc/signalforge/draft/prompts.pysrc/signalforge/grade/parser.pysrc/signalforge/ingest/__init__.pysrc/signalforge/ingest/anchor.pysrc/signalforge/ingest/errors.pysrc/signalforge/ingest/models.pysrc/signalforge/ingest/parser.pysrc/signalforge/ingest/reader.pysrc/signalforge/manifest/__init__.pysrc/signalforge/manifest/errors.pysrc/signalforge/manifest/loader.pysrc/signalforge/manifest/models.pysrc/signalforge/manifest/template.pysrc/signalforge/prune/compiler.pysrc/signalforge/prune/engine.pysrc/signalforge/warehouse/__init__.pysrc/signalforge/warehouse/_sample_id.pysrc/signalforge/warehouse/_sample_sql.pysrc/signalforge/warehouse/_sql_safety.pysrc/signalforge/warehouse/adapters/_snowflake_client.pysrc/signalforge/warehouse/adapters/bigquery.pysrc/signalforge/warehouse/adapters/snowflake.pysrc/signalforge/warehouse/base.pysrc/signalforge/warehouse/errors.pysrc/signalforge/warehouse/models.pysrc/signalforge/warehouse/profiles.pytests/_common/test_json_payload.pytests/_common/test_path_safety.pytests/cli/_e2e_helpers.pytests/cli/test_5_surface_parity_force.pytests/cli/test_5_surface_parity_prune_existing.pytests/cli/test_e2e_business_rules.pytests/cli/test_e2e_helpers.pytests/cli/test_e2e_snowflake_smoke.pytests/cli/test_estimate_engine.pytests/cli/test_estimate_render.pytests/cli/test_exit_codes.pytests/cli/test_generate.pytests/cli/test_generate_estimate.pytests/cli/test_prune_existing.pytests/cli/test_subprocess_smoke.pytests/diff/_snapshot_inputs.pytests/diff/test_artifact_id.pytests/diff/test_drift_detector.pytests/diff/test_emitter.pytests/diff/test_engine.pytests/diff/test_models.pytests/diff/test_public_api.pytests/diff/test_renderers.pytests/diff/test_snapshot_fixtures.pytests/diff/test_test_file_writer.pytests/draft/test_drift_detector.pytests/draft/test_exclude_tests.pytests/draft/test_models.pytests/draft/test_parser.pytests/draft/test_prompts.pytests/fixtures/diff/diff_report_v1.jsontests/fixtures/diff/full_with_grade.jsontests/fixtures/diff/proposed_test_files.ansitests/fixtures/diff/proposed_test_files.mdtests/fixtures/draft/candidate_schema_v1.jsontests/fixtures/e2e_helpers/happy/.signalforge/diff.jsontests/fixtures/error_paths/non_dict_root.jsontests/fixtures/ingest/custom_sql_files/assert_customers_have_email.sqltests/fixtures/ingest/custom_sql_files/assert_orders_amount_positive.sqltests/fixtures/ingest/custom_sql_files/assert_orders_with_macro.sqltests/fixtures/ingest/schema_austin_bikeshare.ymltests/fixtures/ingest/schema_codegen_shaped.ymltests/fixtures/profiles/dbt_snowflake_drift_v1_x.ymltests/fixtures/profiles/snowflake_password.ymltests/fixtures/prune/compiled_sql/custom_sql.sqltests/fixtures/prune/compiled_sql/custom_sql_fullscan.sqltests/fixtures/prune/compiled_sql/custom_sql_sample.sqltests/fixtures/prune/compiled_sql/snowflake/accepted_values.sqltests/fixtures/prune/compiled_sql/snowflake/accepted_values_sample.sqltests/fixtures/prune/compiled_sql/snowflake/custom_sql.sqltests/fixtures/prune/compiled_sql/snowflake/custom_sql_fullscan.sqltests/fixtures/prune/compiled_sql/snowflake/custom_sql_sample.sqltests/fixtures/prune/compiled_sql/snowflake/not_null.sqltests/fixtures/prune/compiled_sql/snowflake/not_null_sample.sqltests/fixtures/prune/compiled_sql/snowflake/relationships.sqltests/fixtures/prune/compiled_sql/snowflake/relationships_sample.sqltests/fixtures/prune/compiled_sql/snowflake/unique.sqltests/fixtures/prune/compiled_sql/snowflake/unique_sample.sqltests/fixtures/snowflake/README.mdtests/fixtures/snowflake/_gen_manifest.pytests/fixtures/snowflake/dbt_project.ymltests/fixtures/snowflake/models/staging/sources.ymltests/fixtures/snowflake/models/staging/stg_tpch_customers.sqltests/fixtures/snowflake/profiles.ymltests/fixtures/snowflake/target/manifest.jsontests/fixtures/warehouse/snowflake/README.mdtests/fixtures/warehouse/snowflake/explain_using_json_no_stats.jsontests/fixtures/warehouse/snowflake/explain_using_json_sample.jsontests/grade/test_parser.pytests/ingest/test_anchor.pytests/ingest/test_models.pytests/ingest/test_parser.pytests/ingest/test_reader.pytests/ingest/test_test_files.pytests/llm/test_prompt_cache_stability.pytests/manifest/test_loader.pytests/manifest/test_models.pytests/manifest/test_resolve.pytests/manifest/test_template.pytests/prune/test_compiler.pytests/prune/test_compiler_fakesnow.pytests/prune/test_compiler_import_guard.pytests/prune/test_engine.pytests/test_audit_completeness.pytests/test_demo.pytests/warehouse/_fake_snowflake.pytests/warehouse/test_base.pytests/warehouse/test_bigquery_unit.pytests/warehouse/test_errors.pytests/warehouse/test_models.pytests/warehouse/test_profiles.pytests/warehouse/test_sample_id.pytests/warehouse/test_sample_sql.pytests/warehouse/test_snowflake_adapter_fakesnow.pytests/warehouse/test_snowflake_client.pytests/warehouse/test_snowflake_client_confinement.pytests/warehouse/test_snowflake_estimate.pytests/warehouse/test_snowflake_estimate_live.pytests/warehouse/test_snowflake_exception_mapping.pytests/warehouse/test_snowflake_lifecycle.pytests/warehouse/test_snowflake_materialise.pytests/warehouse/test_snowflake_prune_live.pytests/warehouse/test_snowflake_sampling.pytests/warehouse/test_snowflake_seed_loads.pytests/warehouse/test_snowflake_stub.pytests/warehouse/test_sql_safety.py
|
|
||
| ## Snowflake test harness + full error taxonomy (issue #124) | ||
|
|
||
| #124 is the epic's closing test+docs ticket. Three durable conventions: |
There was a problem hiding this comment.
Fix malformed heading syntax at Line 228.
#124 is the epic's closing test+docs ticket... should be a plain paragraph or a proper heading (# 124 ...) to satisfy markdown linting and consistent rendering.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 228-228: No space after hash on atx style heading
(MD018, no-missing-space-atx)
🤖 Prompt for 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.
In @.claude/rules/warehouse-adapters.md at line 228, Replace the malformed
heading token "`#124` is the epic's closing test+docs ticket..." so Markdown
linter accepts it: either remove the leading "#" to make it a plain paragraph or
insert a space after the "#" to make it a valid ATX heading (e.g., "# 124 is the
epic's closing test+docs ticket..."); locate the exact string "`#124` is the
epic's closing test+docs ticket" in the file and update accordingly.
| ``` | ||
| model.sql + manifest + project ctx | ||
| -> LLM drafts candidate artifacts | ||
| -> run candidates against warehouse samples | ||
| -> drop always-pass tests; drop tests that fail on known-clean data | ||
| -> emit graded YAML + diff with per-artifact "why" | ||
| ``` |
There was a problem hiding this comment.
Add a language tag to the fenced block starting at Line 14.
Use something like ```text for the pipeline diagram block to keep markdown lint clean and rendering consistent.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 14-14: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for 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.
In `@CLAUDE.md` around lines 14 - 20, The fenced code block showing the pipeline
diagram in CLAUDE.md (the block beginning with the lines "model.sql + manifest +
project ctx" through "emit graded YAML + diff with per-artifact "why"") is
missing a language tag; update the opening fence to include a language
identifier (for example change ``` to ```text) so markdown linters and renderers
treat it as plain text and avoid lint/render warnings.
| @@ -559,7 +800,8 @@ on a `↳ Remediation:` line by `__str__`. | |||
| | `UnknownTableSizeError` | `Table.num_rows` is `None`/`0` and no `PartitionFilter` was supplied. | `table` | Provide `partition_filter`, or call `adapter.refresh_table_metadata` once `num_rows` is populated. | | |||
| | `MaterialisationFailedError` (v0.2) | `BigQueryAdapter.materialise_sample` wraps an SDK / network / quota failure during the materialisation query. | `cause` | Inspect `.cause` for the underlying exception; falls back to `prune.sample_strategy: oneshot` to bypass materialisation. | | |||
| | `MaterialisationNotSupportedError` (v0.2)| `WarehouseAdapter.materialise_sample` default impl raised because the concrete adapter doesn't override it (any non-BigQuery adapter in v0.2). | _(none)_ | Set `prune.sample_strategy: oneshot` in `signalforge.yml` to fall back to per-test sampling, or wait for v0.3 multi-warehouse materialisation support. | | |||
There was a problem hiding this comment.
Narrow MaterialisationNotSupportedError scope in error table
Line 802 still says this applies to “any non-BigQuery adapter in v0.2,” but this file now documents Snowflake materialise_sample as implemented. Please update this row so it only references adapters that still inherit the default raise path.
🧰 Tools
🪛 LanguageTool
[grammar] ~802-~802: Ensure spelling is correct
Context: ...oneshotto bypass materialisation. | |MaterialisationNotSupportedError(v0.2)|WarehouseAdapter.materialise_s...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for 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.
In `@docs/warehouse-adapter-ops.md` at line 802, Update the error-table row for
MaterialisationNotSupportedError so it no longer claims “any non-BigQuery
adapter in v0.2”; instead state it applies only to adapters that still inherit
the default raising implementation (WarehouseAdapter.materialise_sample) and
therefore do not implement materialise_sample (exclude Snowflake, which now
implements materialise_sample); keep the suggested mitigations (set
prune.sample_strategy: oneshot or wait for v0.3) unchanged.
| ``` | ||
| US-001 ── US-002 ─┬─ US-003 ─┐ | ||
| └─ US-004 ─┴─ US-005 ── US-006 ── US-007 (Quality Gate) ── US-008 (Patterns & Memory) | ||
| ``` |
There was a problem hiding this comment.
Add a language to this fenced code block.
This unlabeled fence will keep tripping markdown lint (MD040). Use text for the dependency graph block.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 222-222: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for 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.
In `@plans/super/104-ingest-external-tests.md` around lines 222 - 225, The fenced
code block showing the dependency graph (the block containing "US-001 ── US-002
─┬─ US-003 ─┐" and "└─ US-004 ─┴─ US-005 ── US-006 ── US-007 (Quality Gate) ──
US-008 (Patterns & Memory)") lacks a language tag and triggers MD040; update
that fence to include the language identifier text (i.e., change the opening ```
to ```text) so the block is treated as plain text and markdown lint no longer
flags it.
|
|
||
| --- | ||
|
|
||
| ## Detailed breakdown (Phase 4) |
There was a problem hiding this comment.
Avoid duplicate section heading text.
## Detailed breakdown (Phase 4) is already used earlier (Line 147). Rename this second heading (or remove it) to avoid duplicate-anchor/TOC ambiguity.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 247-247: Multiple headings with the same content
(MD024, no-duplicate-heading)
🤖 Prompt for 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.
In `@plans/super/104-ingest-external-tests.md` at line 247, The second occurrence
of the heading "## Detailed breakdown (Phase 4)" duplicates an earlier anchor
and causes TOC ambiguity; update the duplicate heading (the one at the bottom of
the Phase 4 section) to a unique title (for example "## Detailed breakdown
(Phase 4) — continued" or "## Additional details (Phase 4)"), or remove it if
redundant, ensuring only one "## Detailed breakdown (Phase 4)" heading remains.
| except (RefNotFoundError, SourceNotFoundError) as exc: | ||
| # The ref()/source() target is not in the manifest yet — the | ||
| # referenced model/source simply isn't built. Mirror the | ||
| # relationships missing-target precedent (DEC-026): route to | ||
| # requires-future-data so the operator revisits when the | ||
| # dependency lands. NEVER raise — these are ManifestError | ||
| # siblings of TemplateResolutionError, not subclasses, so the | ||
| # broader handler below would not catch them. | ||
| return _RequiresFutureData( | ||
| reason=f"custom_sql references a manifest-absent target: {type(exc).__name__}" | ||
| ) | ||
| except AmbiguousRefError as exc: | ||
| # Genuine user ambiguity (the ref() name matches multiple | ||
| # packages), not future data. Route to kept-without-evidence so a | ||
| # reviewer disambiguates with the two-arg ref('pkg','name') form. | ||
| return _InvalidIdentifier(reason=f"custom_sql ref() is ambiguous: {type(exc).__name__}") | ||
| except TemplateResolutionError as exc: | ||
| # Covers both UnsupportedJinjaError and the residual-{{ }} case. | ||
| return _InvalidIdentifier( | ||
| reason=f"custom_sql Jinja could not be resolved: {type(exc).__name__}" |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Include the actual template-resolution detail in the sentinel reason.
These branches currently collapse the failure down to the exception class name, so the prune diff/audit still does not tell the operator which ref() / source() / Jinja construct needs fixing.
💡 Suggested change
except (RefNotFoundError, SourceNotFoundError) as exc:
+ detail = str(exc).splitlines()[0]
return _RequiresFutureData(
- reason=f"custom_sql references a manifest-absent target: {type(exc).__name__}"
+ reason=f"custom_sql references a manifest-absent target: {detail}"
)
except AmbiguousRefError as exc:
- return _InvalidIdentifier(reason=f"custom_sql ref() is ambiguous: {type(exc).__name__}")
+ detail = str(exc).splitlines()[0]
+ return _InvalidIdentifier(reason=f"custom_sql ref() is ambiguous: {detail}")
except TemplateResolutionError as exc:
+ detail = str(exc).splitlines()[0]
return _InvalidIdentifier(
- reason=f"custom_sql Jinja could not be resolved: {type(exc).__name__}"
+ reason=f"custom_sql Jinja could not be resolved: {detail}"
)Based on learnings: Every kept/dropped artifact must ship with a one-line 'why' explanation; do not add black-box code paths that drop or keep artifacts without recording the reason.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| except (RefNotFoundError, SourceNotFoundError) as exc: | |
| # The ref()/source() target is not in the manifest yet — the | |
| # referenced model/source simply isn't built. Mirror the | |
| # relationships missing-target precedent (DEC-026): route to | |
| # requires-future-data so the operator revisits when the | |
| # dependency lands. NEVER raise — these are ManifestError | |
| # siblings of TemplateResolutionError, not subclasses, so the | |
| # broader handler below would not catch them. | |
| return _RequiresFutureData( | |
| reason=f"custom_sql references a manifest-absent target: {type(exc).__name__}" | |
| ) | |
| except AmbiguousRefError as exc: | |
| # Genuine user ambiguity (the ref() name matches multiple | |
| # packages), not future data. Route to kept-without-evidence so a | |
| # reviewer disambiguates with the two-arg ref('pkg','name') form. | |
| return _InvalidIdentifier(reason=f"custom_sql ref() is ambiguous: {type(exc).__name__}") | |
| except TemplateResolutionError as exc: | |
| # Covers both UnsupportedJinjaError and the residual-{{ }} case. | |
| return _InvalidIdentifier( | |
| reason=f"custom_sql Jinja could not be resolved: {type(exc).__name__}" | |
| except (RefNotFoundError, SourceNotFoundError) as exc: | |
| # The ref()/source() target is not in the manifest yet — the | |
| # referenced model/source simply isn't built. Mirror the | |
| # relationships missing-target precedent (DEC-026): route to | |
| # requires-future-data so the operator revisits when the | |
| # dependency lands. NEVER raise — these are ManifestError | |
| # siblings of TemplateResolutionError, not subclasses, so the | |
| # broader handler below would not catch them. | |
| detail = str(exc).splitlines()[0] | |
| return _RequiresFutureData( | |
| reason=f"custom_sql references a manifest-absent target: {detail}" | |
| ) | |
| except AmbiguousRefError as exc: | |
| # Genuine user ambiguity (the ref() name matches multiple | |
| # packages), not future data. Route to kept-without-evidence so a | |
| # reviewer disambiguates with the two-arg ref('pkg','name') form. | |
| detail = str(exc).splitlines()[0] | |
| return _InvalidIdentifier(reason=f"custom_sql ref() is ambiguous: {detail}") | |
| except TemplateResolutionError as exc: | |
| # Covers both UnsupportedJinjaError and the residual-{{ }} case. | |
| detail = str(exc).splitlines()[0] | |
| return _InvalidIdentifier( | |
| reason=f"custom_sql Jinja could not be resolved: {detail}" | |
| ) |
🤖 Prompt for 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.
In `@src/signalforge/prune/compiler.py` around lines 691 - 710, Those except
branches (_RequiresFutureData and _InvalidIdentifier returns) are currently
using only the exception class name in the reason; change them to include the
exception message/details (e.g., str(exc) or exc.args[0]) so the prune audit
shows which ref()/source()/Jinja element failed. Specifically, update the
handlers for RefNotFoundError/SourceNotFoundError (returning
_RequiresFutureData), AmbiguousRefError (returning _InvalidIdentifier), and
TemplateResolutionError (returning _InvalidIdentifier) to include the exception
text alongside the existing context string (for example: reason=f"...:
{type(exc).__name__}: {str(exc)}"). Ensure each returned reason remains a
concise one-line explanation.
| if _is_multi_table(resolved_sql): | ||
| # Multi-table: full-scan. Apply a partition filter to the model's | ||
| # own table when available; otherwise return the resolved SQL | ||
| # unchanged. Sampling a join is semantically wrong (DEC-006). | ||
| if partition_filter is not None: | ||
| partition_sql = _render_partition_filter(partition_filter, dialect) | ||
| replacement = f"(SELECT * FROM {own_qualified} WHERE {partition_sql})" | ||
| if own_qualified in resolved_sql: | ||
| return resolved_sql.replace(own_qualified, replacement, 1) |
There was a problem hiding this comment.
Partition-rewrite all self-references, not just the first one.
The multi-table path only rewrites the first occurrence of own_qualified. A custom_sql self-join or correlated subquery can still reference the model table multiple times, which leaves later scans unpartitioned and changes the evaluated row set.
🐛 Suggested change
if partition_filter is not None:
partition_sql = _render_partition_filter(partition_filter, dialect)
replacement = f"(SELECT * FROM {own_qualified} WHERE {partition_sql})"
if own_qualified in resolved_sql:
- return resolved_sql.replace(own_qualified, replacement, 1)
+ return resolved_sql.replace(own_qualified, replacement)🤖 Prompt for 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.
In `@src/signalforge/prune/compiler.py` around lines 726 - 734, In _is_multi_table
branch where you build partition_sql via _render_partition_filter and create
replacement for own_qualified, change the current single-occurrence replacement
to rewrite all self-references of the model table in resolved_sql (not just the
first). Locate the block using variables own_qualified, resolved_sql and
partition_filter and replace the .replace(..., 1) behavior with a global
replacement (or a regex-based global replace with appropriate word-boundary
anchoring for own_qualified) so every occurrence of the model table is wrapped
with the partitioned subquery.
| if not pairs: | ||
| # Call the kept-rate WARNING helper at this return site too, per | ||
| # the "called at every prune_tests return site" contract (issue | ||
| # #51) — it's a no-op here (the helper early-returns on | ||
| # ``total == 0``), but routing every return site through it keeps | ||
| # a future change to the empty-candidate path from silently | ||
| # dropping the signal. | ||
| _maybe_emit_kept_rate_warning( | ||
| [], | ||
| model_unique_id=model.unique_id, | ||
| threshold=resolved_config.min_kept_rate_warn, | ||
| ) | ||
| return PruneResult( | ||
| model_unique_id=model.unique_id, | ||
| decisions=(), | ||
| elapsed_ms=max(0, _now_monotonic_ms() - start_ms), | ||
| signalforge_version=_SIGNALFORGE_VERSION, | ||
| ) |
There was a problem hiding this comment.
Keep trusted_models validation ahead of the empty-candidate return.
This fast path exits before _validate_trusted_models, so a typo in trusted_models now passes silently whenever a model has zero candidates. That breaks the documented “validate at entry before any warehouse call” contract.
🔧 Suggested change
- if not pairs:
+ _validate_trusted_models(resolved_config, manifest)
+
+ if not pairs:
_maybe_emit_kept_rate_warning(
[],
model_unique_id=model.unique_id,
threshold=resolved_config.min_kept_rate_warn,
)
return PruneResult(
model_unique_id=model.unique_id,
decisions=(),
elapsed_ms=max(0, _now_monotonic_ms() - start_ms),
signalforge_version=_SIGNALFORGE_VERSION,
)
-
- # Validate trusted_models BEFORE any warehouse call (DEC-008).
- _validate_trusted_models(resolved_config, manifest)🤖 Prompt for 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.
In `@src/signalforge/prune/engine.py` around lines 897 - 914, The empty-candidate
fast path returns before `_validate_trusted_models`, allowing typos in
`trusted_models` to pass silently; fix by invoking
`_validate_trusted_models(...)` (using the same arguments passed into the
surrounding function — e.g. `model` and `trusted_models`) before the `if not
pairs:` early-return and keep the existing `_maybe_emit_kept_rate_warning` and
`PruneResult` return logic unchanged so validation always runs at entry even
when there are zero candidates.
| self._connection = make_real_client( | ||
| account=self._account or "", | ||
| user=self._user or "", | ||
| password=self._password or "", | ||
| role=self._role, | ||
| warehouse=self._warehouse, | ||
| database=self._database, | ||
| schema=self._schema, | ||
| ) |
There was a problem hiding this comment.
Forward Snowflake auth parameters when creating the real client.
private_key_path, private_key_passphrase, and authenticator are captured on the adapter but never passed into make_real_client, so those profile modes are effectively ignored at connection time.
Suggested fix
self._connection = make_real_client(
account=self._account or "",
user=self._user or "",
password=self._password or "",
role=self._role,
warehouse=self._warehouse,
database=self._database,
schema=self._schema,
+ private_key_path=self._private_key_path,
+ private_key_passphrase=self._private_key_passphrase,
+ authenticator=self._authenticator,
)🤖 Prompt for 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.
In `@src/signalforge/warehouse/adapters/snowflake.py` around lines 275 - 283, The
Snowflake auth parameters private_key_path, private_key_passphrase, and
authenticator captured on the adapter are not forwarded to make_real_client;
update the client creation call in the Snowflake adapter where self._connection
is set (the make_real_client(...) invocation) to pass these parameters (e.g.,
private_key_path=self._private_key_path,
private_key_passphrase=self._private_key_passphrase,
authenticator=self._authenticator) so profile-based auth modes are honored.
| cursor = self._get_connection().cursor() | ||
| try: | ||
| cursor.execute(sql) | ||
| return list(cursor.fetchall()) | ||
| except Exception as exc: | ||
| mapped = map_snowflake_exception(exc, context={"table": table.qualified_name}) | ||
| if mapped is exc: | ||
| raise | ||
| raise mapped from exc | ||
|
|
There was a problem hiding this comment.
Close cursors in _execute and _execute_to_dicts to avoid resource leaks.
Both helpers leave cursors open on success and failure paths.
Suggested fix
def _execute(self, sql: str, *, table: TableRef) -> list[Any]:
@@
cursor = self._get_connection().cursor()
try:
cursor.execute(sql)
return list(cursor.fetchall())
except Exception as exc:
mapped = map_snowflake_exception(exc, context={"table": table.qualified_name})
if mapped is exc:
raise
raise mapped from exc
+ finally:
+ cursor.close()
@@
def _execute_to_dicts(self, sql: str, *, table: TableRef) -> list[dict[str, Any]]:
@@
cursor = self._get_connection().cursor()
try:
cursor.execute(sql)
rows = list(cursor.fetchall())
except Exception as exc:
mapped = map_snowflake_exception(exc, context={"table": table.qualified_name})
if mapped is exc:
raise
raise mapped from exc
- return self._rows_to_dicts(cursor, rows)
+ finally:
+ cursor.close()
+ return self._rows_to_dicts(cursor, rows)Also applies to: 565-574
🤖 Prompt for 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.
In `@src/signalforge/warehouse/adapters/snowflake.py` around lines 546 - 555, The
_execute and _execute_to_dicts helpers are leaving DB cursors open; update both
functions (the blocks creating cursor = self._get_connection().cursor()) to
ensure the cursor is closed on all paths by using a try/finally (or a context
manager) that calls cursor.close() in the finally, and keep the existing
exception mapping logic (map_snowflake_exception(...)) intact so you still map
and re-raise mapped exceptions (raise mapped from exc) after closing the cursor;
make sure successful returns (e.g., list(cursor.fetchall()) and whatever
_execute_to_dicts returns) also close the cursor before returning.
main and dev only shared a merge-base at v0.1.0 (0.2.0 release broke ancestry), causing spurious conflicts on the dev->main release PR. dev is the content superset; this -s ours merge records main as a parent without changing the tree, so PR #149 is conflict-free and future releases don't re-conflict.
Cuts v0.3.0 to PyPI. Brings the accumulated
devline (34 commits sincev0.2.0) intomain.Headline features
WarehouseAdapter, graduating the ABC + factory seam through a real vendor.custom_sql) — the 5th dbt test type (feat: custom business-rule test generation (meta.business_rules + LLM-inferred singular SQL tests) #116), drafted frommeta.signalforge.business_rules/ LLM inference, pruned + graded like the built-ins.Notable fixes
get_row_countseam (Snowflake oneshot sample prune: engine routes row-count through BigQuery-only _get_client — needs vendor-neutral get_table_metadata seam #140), drafter/grader prose-preamble JSON tolerance (Business-rules custom_sql drafting reproducibly fails: LLM emits prose preamble before JSON (no assistant prefill) #144).Release mechanics
0.3.0.dev0→0.3.0; CHANGELOG[Unreleased]promoted to[0.3.0]— 2026-05-27 (with the missing feat: custom business-rule test generation (meta.business_rules + LLM-inferred singular SQL tests) #116 entry added).dev-based worktree:ruff/ruff format/pyrightclean, 2416 passed (97% cov);uv build+uvx twine checkPASSED on wheel + sdist (signalforge_dbt-0.3.0).dev(notmain) becausedevis 34 ahead — this PR is the dev→main promotion for the 0.3.0 line.After merge: tag
v0.3.0onmainHEAD → non-prerelease GitHub Release →publish-pypi. Then next-dev bump (0.4.0.dev0) + backmerge todev.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
prune-existingCLI subcommand for read-only evaluation of externally-authored dbt tests without LLM calls.meta.signalforge.business_rulesmetadata..sqltest files for kept custom-rule tests.Improvements