6: Test prune engine

[wjduenow]

Closes #6.

Drops always-pass and known-clean-fail candidate tests by running each one against the warehouse — the load-bearing differentiator per CLAUDE.md commitment #1 (signal over volume).

What ships

signalforge.prune subpackage (engine, compiler, models, errors, audit, config) plus WarehouseAdapter extension for per-test budget plumbing.

Public API:

• prune_tests(model, adapter, candidates, manifest, *, config=None, project_dir=None, audit_path=None) -> PruneResult
• PruneResult, PruneDecision, PruneConfig, load_prune_config, PruneEvent
• DropReason, Scope literals
• PruneError hierarchy (six classes)

Drop reasons:

Reason	Decision	Trigger
always-passes	dropped	Zero failures on the sample/full set
requires-future-data	dropped	relationships(to: ...) references a manifest-absent parent
failed-on-known-clean-data	dropped	Test failed AND model is in prune.trusted_models
kept	kept	Test failed on an untrusted model
kept-without-evidence	kept	Couldn't evaluate — conservative ship

Implementation summary (16 stories)

Plan + 28 decisions captured in plans/super/6-prune-engine.md. Devolved into 14 implementation stories + Quality Gate + Patterns & Memory. All landed on this branch.

#	Story
US-001	Subpackage scaffold + prune: namespace
US-002	Warehouse adapter timeout_ms plumbing (folded prereq, AR-B2)
US-003	Diagnostic cost probe + ops-doc scaffold (resolves AR-B1)
US-004	PruneResult / PruneDecision / primitives
US-005	PruneConfig + load_prune_config
US-006	PruneError hierarchy (six classes)
US-007	Compiler — failing-rows SQL × 4 test types
US-008	Audit — fail-closed prune.jsonl + AST scan extension
US-009	Engine — prune_tests orchestrator
US-010	Drift detectors (3 Strict mirrors + JSONL fixture)
US-011	Logger grep gate extension
US-012	BQ integration test against bigquery-public-data
US-013	Public re-exports
US-014	docs/prune-ops.md + CLAUDE.md update
QG	Quality Gate — 4-pass code review surfaced 7 bugs, all fixed
PM	.claude/rules/prune-engine.md + memory entries

Quality Gate — bugs caught and fixed before close

Four parallel review passes (security, test-signal, DEC-compliance, API-design) surfaced 7 real bugs. All fixed:

1. PruneResult.__repr__ redacts compiled_sql and sample_failures — Pydantic default would have leaked PII into accidental log lines (DEC-022).
2. Audit path canonicalised via warehouse._path_safety.canonicalise_path — a symlinked .signalforge/prune.jsonl could otherwise redirect writes outside the project (DEC-016).
3. load_prune_config(project_dir, path=None) signature aligned with load_safety_config / load_draft_config — one calling convention across stages.
4. Default audit_path resolves relative to project_dir, not cwd — same Copilot-flagged regression the safety layer fixed.
5. Compiler validates column / field identifier shape via _sql_safety.validate_identifier; new _InvalidIdentifier sentinel routes to kept-without-evidence. Defense-in-depth against an LLM-crafted field like x` AS y UNION SELECT secret ....
6. test_yaml_safe_load_rejects_python_objects strengthened to use a side-effect-detectable gadget (Path.touch on a tmp marker) — the original test would have passed under unsafe yaml.load too.
7. PruneTimeoutError documented as forward-compat for v0.2 (kept in __all__ for stable API).

Architecture review — pivots resolved

• AR-B1 cost-model verification. The deterministic-sample predicate WHERE MOD(ABS(FARM_FINGERPRINT(TO_JSON_STRING(t))), bucket) < 1 reads every column of the row (BQ can't column-prune through a function arg). v0.1 ships Q4=A (one query per test) by default; US-003 is the live diagnostic probe that measures total_bytes_billed and either confirms the cost model or escalates to Q4=C in a v0.1 amendment. Probe is BQ-gated; runs with SF_RUN_BQ=1 pytest -m bigquery.
• AR-B2 adapter prereq. QueryJobConfig.job_timeout_ms plumbing was missing; folded into US-002 as ~10 LOC + tests.

Validation

pip install -e ".[dev]" && ruff check . && ruff format --check . && pyright && pytest — 710 passed, 9 deselected (BQ-marked).

Test plan

• Review plans/super/6-prune-engine.md for the design + 28 DEC entries
• Skim the public API in docs/prune-ops.md
• Run pytest locally — expect 710 passing
• Optional live verification: SF_RUN_BQ=1 SF_BQ_PROJECT=<project> pytest -m bigquery tests/prune/test_integration_bigquery.py tests/warehouse/test_sample_cost_probe.py

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added test pruning engine to automatically evaluate and classify candidate tests as kept or dropped based on warehouse evaluation and configurable budgets.
  • Added support for prune layer configuration in signalforge.yml, including sample scope, timeout settings, budget limits, and trusted model validation.
  • Added fail-closed audit logging in JSONL format for all pruning decisions, enabling operational visibility and debugging.

• Documentation

  • Added comprehensive operations guide for the prune layer with examples, error handling, and schema references.
  • Added timeout mechanism documentation for warehouse operations.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d705a561-75b1-4391-a77c-901268e34609

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

This PR introduces the complete signalforge.prune layer: an end-to-end test pruning engine that evaluates candidate dbt-style tests against BigQuery, compiles tests to SQL, executes them with budget/timeout enforcement, classifies outcomes via a DropReason taxonomy, and writes fail-closed JSONL audit records. The implementation includes strict configuration loading, deterministic error handling, trusted-model validation, and comprehensive test coverage.

Changes

Cohort / File(s)	Summary
Prune Engine Core src/signalforge/prune/__init__.py, src/signalforge/prune/engine.py, src/signalforge/prune/models.py, src/signalforge/prune/errors.py	Orchestrator (prune_tests), typed decision/result schemas with computed views, error hierarchy with remediation support, and public API exports.
Prune Compiler & Config src/signalforge/prune/compiler.py, src/signalforge/prune/config.py, src/signalforge/prune/audit.py	SQL compilation for not_null/unique/relationships/accepted_values tests; configuration loading from signalforge.yml with validation; fail-closed JSONL audit writing with record-size limits and atomic concurrent append.
Warehouse Adapter Timeout Support src/signalforge/warehouse/adapters/_client.py, src/signalforge/warehouse/adapters/bigquery.py	Added optional timeout_ms parameter to make_query_job_config and _default_job_config for server-side job cancellation via BigQuery QueryJobConfig.job_timeout_ms.
Documentation & Planning .claude/rules/prune-engine.md, CLAUDE.md, docs/prune-ops.md, docs/warehouse-adapter-ops.md, plans/super/6-prune-engine.md	Comprehensive specification, operations guides, roadmap updates, and project plan defining architecture decisions, API contracts, and v0.1 design constraints.
Prune Test Fixtures tests/fixtures/prune/compiled_sql/*.sql, tests/fixtures/prune/prune_*.json, tests/fixtures/prune/prune_event_v1.jsonl, tests/fixtures/prune/signalforge_*.yml	Compiled SQL examples, decision/result/event JSON fixtures, and configuration YAML samples for testing.
Prune Unit Tests tests/prune/test_audit.py, tests/prune/test_compiler.py, tests/prune/test_config.py, tests/prune/test_models.py, tests/prune/test_errors.py, tests/prune/test_smoke.py	Comprehensive coverage for audit writing (fail-closed semantics, POSIX mode bits, concurrent appends), SQL compilation with NULL-exclusion validation, config loading with schema enforcement, model frozen/immutable properties, error remediation, and public API surface.
Prune Integration & Infrastructure Tests tests/prune/test_engine.py, tests/prune/test_drift_detector.py, tests/prune/test_integration_bigquery.py, tests/test_audit_completeness.py, tests/llm/test_logger_grep_gate.py	End-to-end engine orchestration including budget/timeout/audit semantics, schema drift detection with strict mirrors, BigQuery integration probe, AST-gated audit construction enforcement, and logger grep-gate extension.
Warehouse Test Updates tests/warehouse/test_bigquery_unit.py, tests/warehouse/test_default_job_config_timeout.py, tests/warehouse/test_sample_cost_probe.py	Updated unit tests for keyword-only stage argument, new timeout plumbing validation, and integration probe for sample-rows billing validation.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant Engine as prune_tests<br/>(Orchestrator)
    participant ConfigLoader as load_prune_config
    participant Compiler as compile_test<br/>(SQL)
    participant Warehouse as BigQueryAdapter
    participant Auditor as _write_prune_event<br/>(JSONL)
    
    Caller->>Engine: prune_tests(model, adapter,<br/>candidates, manifest, ...)
    
    Engine->>ConfigLoader: load_prune_config(project_dir)
    ConfigLoader-->>Engine: PruneConfig
    
    Engine->>Engine: Validate trusted_models<br/>against Manifest<br/>(before any warehouse call)
    
    Engine->>Engine: Create/canonicalise<br/>audit_path
    
    loop For each CandidateTest
        alt Budget Exhausted
            Engine->>Engine: Emit kept-without-evidence<br/>(budget-spent reason)
            Engine->>Auditor: write audit record
        else Budget Available
            Engine->>Compiler: _compile_test(candidate)
            Compiler-->>Engine: compiled_sql or sentinel
            
            alt Sentinel (missing ref / invalid id)
                Engine->>Engine: Map to kept-without-evidence
            else Valid SQL
                Engine->>Warehouse: run_test_sql(compiled_sql,<br/>timeout_ms)
                Warehouse-->>Engine: TestResult
                
                Engine->>Engine: Classify outcome<br/>via DropReason logic
            end
            
            Engine->>Auditor: _write_prune_event(decision)
            alt Write Fails
                Auditor-->>Engine: PruneAuditWriteError
                Engine-->>Caller: raise (fail-closed)
            else Success
                Auditor-->>Engine: ✓
            end
        end
    end
    
    Engine->>Engine: Aggregate decisions<br/>into PruneResult
    Engine-->>Caller: PruneResult

Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

• 5: LLM draft pipeline #19: Extends tests/test_audit_completeness.py AST-scan enforcement to also cover PruneEvent construction gating, paralleling the LLM draft PR's addition of audit completeness checks for LLMResponseEvent.

Poem

🐰 Hops through the test garden, whiskers twitching with glee
A pruning engine blooms! Failed tests shall cease to be,
Budget-blessed, audit-safe, each verdict logged with care—
Six new reasons why tests drop, and we hippity-hop through there! 🌱✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.07% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title '6: Test prune engine' clearly and specifically references the main change: implementation of the prune engine feature (issue #6), which is the primary objective of this comprehensive pull request.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/6-prune-engine

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[wjduenow]

@CodeRabbit review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[Copilot]

Pull request overview

Adds the new signalforge.prune stage to evaluate drafted dbt-style candidate tests against warehouse data and conservatively drop “noise” tests, along with supporting BigQuery adapter plumbing, audits, docs, and a comprehensive test suite.

Changes:

• Introduces signalforge.prune subpackage (engine/compiler/models/config/errors/audit) plus public re-exports.
• Extends BigQuery job-config construction with optional timeout_ms plumbing and adds corresponding tests.
• Adds prune-layer documentation, fixtures, drift/audit completeness gates, and BigQuery-gated integration/probe tests.

Reviewed changes

Copilot reviewed 40 out of 40 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
src/signalforge/prune/__init__.py	Public prune-layer re-exports and __all__ surface.
src/signalforge/prune/audit.py	Fail-closed JSONL audit writer and PruneEvent model.
src/signalforge/prune/compiler.py	CandidateTest → failing-rows SQL compiler + sentinels + hashing.
src/signalforge/prune/config.py	PruneConfig and load_prune_config YAML loader.
src/signalforge/prune/engine.py	prune_tests orchestrator, routing matrix, audit writes, budgeting.
src/signalforge/prune/errors.py	Typed prune error hierarchy with remediation strings.
src/signalforge/prune/models.py	PruneDecision / PruneResult read-back models + literals.
src/signalforge/warehouse/adapters/_client.py	Adds timeout_ms to make_query_job_config (job timeout support).
src/signalforge/warehouse/adapters/bigquery.py	Threads timeout_ms through _default_job_config; updates call sites.
tests/prune/test_audit.py	Unit tests for prune audit JSONL writer semantics.
tests/prune/test_compiler.py	Snapshot + safety/escaping + determinism tests for compiler output.
tests/prune/test_config.py	Tests for prune config resolution/validation and YAML safe_load.
tests/prune/test_drift_detector.py	Strict-mirror drift gates for prune read-back/audit models.
tests/prune/test_engine.py	Orchestrator routing/budget/audit failure semantics tests.
tests/prune/test_errors.py	Tests for prune error hierarchy + log-injection defenses.
tests/prune/test_integration_bigquery.py	BigQuery-gated end-to-end prune integration test on public dataset.
tests/prune/test_models.py	Tests for result/decision shapes and redacted repr behavior.
tests/prune/test_smoke.py	Smoke tests for prune public API exports and __all__.
tests/test_audit_completeness.py	Extends AST audit-completeness scan to gate PruneEvent construction.
tests/warehouse/test_bigquery_unit.py	Updates _default_job_config calls to keyword-only stage=.
tests/warehouse/test_default_job_config_timeout.py	New tests ensuring timeout_ms threads to job_timeout_ms.
tests/warehouse/test_sample_cost_probe.py	BigQuery-gated probe to measure total_bytes_billed for sampling.
tests/llm/test_logger_grep_gate.py	Extends f-string logger grep gate to include signalforge.prune.
tests/fixtures/prune/signalforge_full.yml	Fixture: full prune config example.
tests/fixtures/prune/signalforge_minimal.yml	Fixture: minimal prune config block.
tests/fixtures/prune/signalforge_partition.yml	Fixture: prune partition_filter config.
tests/fixtures/prune/signalforge_typo.yml	Fixture: typo config for schema validation failure.
tests/fixtures/prune/signalforge_with_siblings.yml	Fixture: prune config alongside other stage namespaces.
tests/fixtures/prune/prune_decision_v1.json	Fixture: prune decision read-back schema.
tests/fixtures/prune/prune_result_v1.json	Fixture: prune result read-back schema.
tests/fixtures/prune/prune_event_v1.jsonl	Fixture: prune audit JSONL read-back schema.
tests/fixtures/prune/compiled_sql/not_null.sql	Snapshot: compiled SQL for not_null.
tests/fixtures/prune/compiled_sql/unique.sql	Snapshot: compiled SQL for unique.
tests/fixtures/prune/compiled_sql/accepted_values.sql	Snapshot: compiled SQL for accepted_values.
tests/fixtures/prune/compiled_sql/relationships.sql	Snapshot: compiled SQL for relationships.
docs/prune-ops.md	Operations guide for prune layer (API/config/audit/cost model).
docs/warehouse-adapter-ops.md	Documents adapter cost defaults and new timeout_ms plumbing.
CLAUDE.md	Updates repository status and public API list to include prune layer.
.claude/rules/prune-engine.md	Adds prune-engine rules/memory document for future work.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Actionable comments posted: 11

🧹 Nitpick comments (2)

src/signalforge/prune/models.py (1)

34-36: ⚡ Quick win

sample_failures breaks the transitive-immutability guarantee.

The model is frozen and the outer container is a tuple, but each entry is still a mutable dict. Callers can mutate failure payloads after construction, which undercuts the “read-back-stable immutable result” contract this docstring describes. Consider freezing each row in a validator or storing an immutable mapping shape here.

Also applies to: 109-109

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/signalforge/prune/models.py` around lines 34 - 36, The sample_failures
field currently holds mutable dicts which breaks the transitive immutability
guarantee; change sample_failures to store immutable mappings (e.g.,
Tuple[Mapping[str, Any], ...]) and convert each dict into an immutable mapping
when the model is constructed—either in the model's validator or initializer for
sample_failures (refer to the sample_failures field name and the model class in
models.py); alternatively define a small frozen dataclass for each failure row
and store a Tuple of those instances so callers cannot mutate payloads after
construction.

src/signalforge/prune/compiler.py (1)

124-134: 🏗️ Heavy lift

The compiler still hardcodes BigQuery SQL semantics.

_qualified_table_name() quotes the whole dotted path as one identifier, and accepted_values reaches straight into escape_bq_string_literal(). Both are BigQuery-shaped; they won't generalize to dialects that quote each identifier separately or escape literals differently. If this module is meant to stay adapter-agnostic, the qualified-name and literal-rendering rules need to live behind the dialect/adapter seam instead of in the prune core.

As per coding guidelines, "Use Dialect.quote_char-driven dispatch when compiling dbt-style tests (not_null, unique, accepted_values, relationships) to warehouse-specific SQL to maintain adapter agnosticism" and "Do not bake warehouse-specific logic (e.g., BigQuery-isms) into core modules; keep the adapter seam clean to enable v0.2+ Snowflake/Postgres/Databricks/Redshift support without rework."

Also applies to: 180-207

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/signalforge/prune/compiler.py` around lines 124 - 134, The current
implementation hardcodes BigQuery semantics in _qualified_table_name and
accepted_values by quoting the entire dotted path and calling
escape_bq_string_literal; instead, delegate quoting and literal-escaping to the
dialect/adapter seam. Replace direct uses of _qualified_table_name and
escape_bq_string_literal with calls to dialect methods (e.g.,
Dialect.quote_identifier or Dialect.quote_part for each identifier in TableRef
to render project/dataset/table separately, and Dialect.escape_literal or
Dialect.quote_literal for accepted_values), and update the compiler functions
that build qualified names and value literals (including the code around
accepted_values and the block that previously used escape_bq_string_literal) to
use those dialect APIs so the module remains adapter-agnostic.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.claude/rules/prune-engine.md:
- Around line 116-122: The docs and loader disagree: update the prune config
implementation so the documented top-level keys (audit_path, mode,
total_budget_seconds, test_timeout_seconds, partition_filter, trusted_models,
etc.) match the actual validator instead of rejecting them. In
src/signalforge/prune/config.py adjust PruneConfig to declare the documented
fields (or add aliases for scope/sample_size/capture_failure_rows -> the
documented names), keep extra="forbid" on PruneConfig, ensure the wrapping
_PruneConfigFile uses extra="ignore", and make PruneConfigError be raised on
unknown keys so the loader and docs are aligned.

In `@docs/prune-ops.md`:
- Around line 112-115: Update the docs to reflect that test_timeout_seconds is
now threaded through to the adapter and enforced server-side: replace the
sentence in the test_timeout_seconds bullet that claims it "does NOT yet thread
it through WarehouseAdapter.run_test_sql" and deferred to v0.2 with text stating
that v0.1 now sets QueryJobConfig.job_timeout_ms and the adapter applies this
per-test timeout (i.e., the knob is active and enforced); leave any note about
future refinements but remove the deprecation/deferral claim. Include references
to test_timeout_seconds, WarehouseAdapter.run_test_sql, and
QueryJobConfig.job_timeout_ms so readers can find the implementation.

In `@docs/warehouse-adapter-ops.md`:
- Around line 115-117: The docs callout incorrectly shows
`_default_job_config(timeout_ms=...)` without the required `stage` parameter;
update the text to demonstrate the correct API usage by showing
`_default_job_config(stage=<stage_name>, timeout_ms=...)` (or equivalent named
stage value) so readers include the mandatory `stage=` when setting
`QueryJobConfig.job_timeout_ms`; reference `_default_job_config` and
`QueryJobConfig.job_timeout_ms` in the edited sentence to make the requirement
explicit.

In `@src/signalforge/prune/audit.py`:
- Around line 220-223: The current write using os.write(fd, encoded) can produce
a short write; change the write logic around os.write, fd, and encoded so you
loop until all bytes of encoded are written (update a bytes_written counter and
slice encoded by that offset) or raise an exception if a write returns 0 or
otherwise cannot make progress, only call os.fsync(fd) after verifying total
bytes written == len(encoded), and ensure fd is closed in the existing
try/finally; this preserves the O_APPEND + fsync durable write pattern and fails
closed on partial writes.

In `@src/signalforge/prune/compiler.py`:
- Around line 210-232: The function _resolve_parent_table_ref currently returns
the first manifest.nodes match for parent_name which can silently pick the wrong
model when duplicate Model.name values exist; change it to scan manifest.nodes
and collect all matching parent_model entries, then if zero matches return the
existing _RequiresFutureData sentinel, if exactly one return
TableRef.from_model(that_model) as before, and if more than one return a new or
existing _RequiresFutureData (or raise a clear ambiguity error) indicating
"relationships parent {parent_name!r} ambiguous: multiple models in manifest" so
ambiguity is detected instead of picking an arbitrary match; keep
TableRef.from_model behavior (allow its
ManifestProjectNotFoundError/ManifestSchemaNotFoundError to propagate).

In `@src/signalforge/prune/engine.py`:
- Around line 704-710: The per-test timeout from PruneConfig
(resolved_config.test_timeout_seconds) is not passed through to the adapter
call, so slow queries can overrun the intended budget; update the call site in
engine.py where adapter.run_test_sql(compiled_sql, ...) is invoked to forward
resolved_config.test_timeout_seconds (e.g., add a timeout_seconds or
test_timeout_seconds kwarg) and ensure the adapter implementation and any
job-config seam accept and apply that timeout (adjust adapter.run_test_sql
signature and the underlying job configuration/timeout hook to use the passed
value).
- Around line 584-587: The decision metadata copies resolved_config.scope but
sampling/partition parameters never reach the warehouse path: update the test
compile/run flow so _compile_test and adapter.run_test_sql receive the
resolved_config.scope plus resolved_config.sample_size and
resolved_config.partition_filter (or their local names) instead of the base
TableRef only; specifically ensure TableRef.from_model(model) is transformed or
replaced with a sampled/partitioned TableRef (or pass
scope/sample_size/partition_filter into _compile_test and through to
adapter.run_test_sql) so that adapter.run_test_sql is called with deterministic
hash-mod sampling and explicit partition filters rather than defaulting to
full-table mode (adjust parameter lists where needed to thread scope,
sample_size, partition_filter through _compile_test -> adapter.run_test_sql).
- Around line 145-175: The helper functions (_why_always_passes,
_why_failed_on_known_clean_data, _why_kept) currently coerce sampled_rows None
-> 0 and thus emit misleading "0 rows"; change them to detect sampled_rows is
None and produce wording that omits the numeric count or explicitly states
"unknown" (e.g., "an unknown number of {scope} rows" or simply "rows unknown")
instead of "0 {scope} rows"; update the three functions to build the message
conditionally based on sampled_rows and reuse Scope's stringification unchanged
so audit rationale reflects unknown counts when sampled_rows is None.

In `@src/signalforge/warehouse/adapters/_client.py`:
- Around line 76-82: Rename the internal helper function make_query_job_config
to _make_query_job_config and update all internal references accordingly: change
the function definition name to _make_query_job_config, update every call
site/import that references make_query_job_config within the adapter package,
and adjust any module-level exports (e.g., __all__) or tests that expose the old
name; keep the signature and behavior unchanged so type hints and callers still
work after the rename.

In `@tests/prune/test_integration_bigquery.py`:
- Around line 55-56: The skip gate for test_prune_iowa_liquor_sales is using
os.environ.get("SF_RUN_BQ") which treats "0"/"false" as truthy; change the skip
condition in the pytest.mark.skipif decorator to explicitly check for allowed
enabled values (e.g. read os.environ.get("SF_RUN_BQ", "").lower() and compare
against a whitelist like {"1","true","yes"}) so the test only runs when
SF_RUN_BQ is explicitly enabled; update the decorator on
test_prune_iowa_liquor_sales to use that explicit check and ensure the default
is treated as disabled.

In `@tests/warehouse/test_sample_cost_probe.py`:
- Around line 138-146: The billing lookup SQL in the test (the f-string building
the query against INFORMATION_SCHEMA.JOBS_BY_USER that filters on creation_time
>= TIMESTAMP_SECONDS(`@since`) and the signalforge_stage label) is too coarse and
can pick up concurrent jobs; change the filter to use millisecond precision
and/or a tight end bound or a unique job label: pass millisecond parameters
(e.g. started_after_ms and started_before_ms) and replace
TIMESTAMP_SECONDS(`@since`) with TIMESTAMP_MILLIS(`@started_after_ms`) and add
creation_time <= TIMESTAMP_MILLIS(`@started_before_ms`), or ideally require a
unique job id/version label (e.g. l.key = 'signalforge_job_id' AND l.value =
`@job_id`) so the query (referencing total_bytes_billed, creation_time, labels,
and region_qualifier/INFORMATION_SCHEMA.JOBS_BY_USER) deterministically selects
the exact job; apply the same tightening to the other occurrences noted (lines
~158-162 and ~222-229).

---

Nitpick comments:
In `@src/signalforge/prune/compiler.py`:
- Around line 124-134: The current implementation hardcodes BigQuery semantics
in _qualified_table_name and accepted_values by quoting the entire dotted path
and calling escape_bq_string_literal; instead, delegate quoting and
literal-escaping to the dialect/adapter seam. Replace direct uses of
_qualified_table_name and escape_bq_string_literal with calls to dialect methods
(e.g., Dialect.quote_identifier or Dialect.quote_part for each identifier in
TableRef to render project/dataset/table separately, and Dialect.escape_literal
or Dialect.quote_literal for accepted_values), and update the compiler functions
that build qualified names and value literals (including the code around
accepted_values and the block that previously used escape_bq_string_literal) to
use those dialect APIs so the module remains adapter-agnostic.

In `@src/signalforge/prune/models.py`:
- Around line 34-36: The sample_failures field currently holds mutable dicts
which breaks the transitive immutability guarantee; change sample_failures to
store immutable mappings (e.g., Tuple[Mapping[str, Any], ...]) and convert each
dict into an immutable mapping when the model is constructed—either in the
model's validator or initializer for sample_failures (refer to the
sample_failures field name and the model class in models.py); alternatively
define a small frozen dataclass for each failure row and store a Tuple of those
instances so callers cannot mutate payloads after construction.

🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3e14c813-bcf3-44dd-ae6e-bf1e0da0462e

📥 Commits

Reviewing files that changed from the base of the PR and between 9491f6a and 9a9da06.

📒 Files selected for processing (40)

• .claude/rules/prune-engine.md
• CLAUDE.md
• docs/prune-ops.md
• docs/warehouse-adapter-ops.md
• plans/super/6-prune-engine.md
• src/signalforge/prune/__init__.py
• src/signalforge/prune/audit.py
• src/signalforge/prune/compiler.py
• src/signalforge/prune/config.py
• src/signalforge/prune/engine.py
• src/signalforge/prune/errors.py
• src/signalforge/prune/models.py
• src/signalforge/warehouse/adapters/_client.py
• src/signalforge/warehouse/adapters/bigquery.py
• tests/fixtures/prune/compiled_sql/accepted_values.sql
• tests/fixtures/prune/compiled_sql/not_null.sql
• tests/fixtures/prune/compiled_sql/relationships.sql
• tests/fixtures/prune/compiled_sql/unique.sql
• tests/fixtures/prune/prune_decision_v1.json
• tests/fixtures/prune/prune_event_v1.jsonl
• tests/fixtures/prune/prune_result_v1.json
• tests/fixtures/prune/signalforge_full.yml
• tests/fixtures/prune/signalforge_minimal.yml
• tests/fixtures/prune/signalforge_partition.yml
• tests/fixtures/prune/signalforge_typo.yml
• tests/fixtures/prune/signalforge_with_siblings.yml
• tests/llm/test_logger_grep_gate.py
• tests/prune/test_audit.py
• tests/prune/test_compiler.py
• tests/prune/test_config.py
• tests/prune/test_drift_detector.py
• tests/prune/test_engine.py
• tests/prune/test_errors.py
• tests/prune/test_integration_bigquery.py
• tests/prune/test_models.py
• tests/prune/test_smoke.py
• tests/test_audit_completeness.py
• tests/warehouse/test_bigquery_unit.py
• tests/warehouse/test_default_job_config_timeout.py
• tests/warehouse/test_sample_cost_probe.py

[wjduenow]

PR Review Summary

All 18 review comments addressed across three commits. No false positives — every flagged issue was a real bug or accuracy gap.

Fixed (18 items)

File	Reviewer	Issue	Commit
src/signalforge/prune/compiler.py + engine.py	Copilot + CodeRabbit	scope=sample / sample_size / partition_filter were advisory-only — never reached compiled SQL. Engine now wraps the failing-rows SQL with the deterministic-sample CTE matching the warehouse adapter's sample_rows shape (DEC-006 of issue #3). Sample-mode relationships samples the child only.	e5fb7cf
src/signalforge/prune/compiler.py	CodeRabbit	relationships(to=name) silently picked the first match when multiple manifest models shared Model.name. Now returns _RequiresFutureData with a precise count → routed to requires-future-data.	e5fb7cf
tests/prune/test_compiler.py + test_engine.py	review follow-up	New sample-mode snapshot fixtures ({not_null,unique,accepted_values,relationships}_sample.sql) and 15 new tests pinning the wiring (CTE shape, partition-filter threading, child-only sampling, num_rows-unavailable PruneError).	e5fb7cf
src/signalforge/prune/audit.py	CodeRabbit	os.write() may return fewer bytes than requested (EINTR / short writes). Now loops until full payload lands; raises OSError on persistent zero-byte return. Test pinned via os.write patch.	1ae462f
tests/warehouse/test_sample_cost_probe.py	CodeRabbit	INFORMATION_SCHEMA.JOBS_BY_USER lookup could attribute bytes to the wrong job (label race). Probe now reads total_bytes_billed straight off the QueryJob returned by client.query(...). Race eliminated.	1ae462f
tests/{prune,warehouse}/test_*_bigquery.py, tests/warehouse/test_sample_cost_probe.py	CodeRabbit	not os.environ.get("SF_RUN_BQ") treated SF_RUN_BQ=0 / =false / =no as truthy. Replaced with _bq_runs_enabled() helper restricting truthy values to {"1","true","yes","on"} (case-insensitive). Applied across all three integration test files.	1ae462f
.claude/rules/prune-engine.md	Copilot	(a) requires-future-data mislabeled "Kept" — it's "Dropped". (b) Phantom config-field references (prune.audit_path, prune.mode); replaced with real PruneConfig fields. (c) Fictional engine.py::_classify_decision renamed to actual _decide_from_test_result.	27e2bee
src/signalforge/prune/config.py	Copilot	trusted_models docstring contradicted implementation (claimed "surface as real failures" — actually routes to failed-on-known-clean-data drop). Rewritten to match.	27e2bee
docs/prune-ops.md	Copilot	prune_tests signature missing project_dir parameter; test_timeout_seconds clarified as v0.2 deferral.	27e2bee
docs/warehouse-adapter-ops.md	Copilot	_default_job_config(timeout_ms=...) example missing required stage= kwarg; false claim that prune layer #6 uses it (it doesn't — reserved for v0.2).	27e2bee
src/signalforge/warehouse/adapters/bigquery.py	Copilot	_default_job_config docstring inaccurately stated prune layer is the only non-None caller. Rewritten to honest framing.	27e2bee
src/signalforge/warehouse/adapters/_client.py + callsites	Copilot	Renamed make_query_job_config → _make_query_job_config (privacy-prefix convention; function lives in the underscore-prefixed _client.py SDK seam). Mechanical rename across _client.py, bigquery.py, prune/engine.py docstring, and two test files.	27e2bee

False Positives (0 items)

None — every flagged issue was a real bug or accuracy gap.

Validation

ruff check .          → All checks passed
ruff format --check . → 106 files already formatted
pyright               → 0 errors
pytest                → 725 passed, 9 deselected (BQ-gated)

Test count grew from 710 → 725 (+15 — sample-mode wiring + audit short-write loop + relationships ambiguity + BQ env-var helper coverage).