#123: Snowflake estimate_query_bytes degrade-first

[wjduenow]

Summary

Super plan for #123 — confirm signalforge generate --estimate degrades cleanly for a Snowflake profile (EstimateNotSupportedError → <unavailable: EstimateNotSupportedError>), with no production code change.

Phase: detailing (awaiting approval)
Stories: 2 implementation (test + doc) + Quality Gate + Patterns & Memory
Decisions: 5 (DEC-001 … DEC-005)

Key finding

This is largely a confirm-and-pin ticket — the degrade path, the inherited ABC raise, and the locked+pinned remediation already exist. The gap is a test exercising a real SnowflakeAdapter through the estimate(...) engine and the --estimate CLI (the existing degrade test uses a fake raising WarehouseAuthError).

Scope decisions

• Keep inheriting the ABC default (zero production change; matches the skeleton convention).
• Verify at engine + in-process CLI levels.
• Defer EXPLAIN-based estimation (Phase 2) — file a follow-up (needs live Snowflake connectivity, feat: Snowflake warehouse adapter (v0.2) — epic + research #118/feat: Snowflake deterministic sampling (sample_rows HASH-mod) + materialise_sample (TEMP TABLE) #122).

Plan document

See plans/super/123-snowflake-estimate-degrade.md.

Next steps

• Review the plan in this PR
• Approve in Claude Code, then say "devolve" to create beads

Closes #123 on merge of the implementation.

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

• Tests

  • Added regression tests to verify graceful degradation when warehouse cost estimation is unavailable.

• Documentation

  • Updated documentation clarifying that --estimate displays unavailable status with zero exit code when warehouse cost calculation cannot proceed, while still computing LLM costs.

[coderabbitai]

Important

Review skipped

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

🗂️ Base branches to auto review (4)

• feature/.*
• bug/.*
• hotfix/.*
• feat/.*

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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f218eff1-df3e-4ac7-99b8-e71fb7661cd1

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

---

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

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[Copilot]

Pull request overview

This PR documents and pins the existing “graceful degrade” behavior for Snowflake query-bytes estimation in signalforge generate --estimate, ensuring EstimateNotSupportedError results in a clean <unavailable: EstimateNotSupportedError> output (exit code 0) while the LLM-cost portion still computes—without changing production behavior.

Changes:

• Add an engine-level test that runs estimate(...) with a real SnowflakeAdapter() and asserts EstimateNotSupportedError is degraded into warehouse_unavailable_reason, plus rendered output contains <unavailable: EstimateNotSupportedError>.
• Add an in-process CLI test for main(["generate", "--estimate", ...]) that injects a real SnowflakeAdapter() and asserts exit 0, correct stdout degrade text, and no traceback leakage; also pin WarehouseAdapter.from_profile dispatch for a Snowflake fixture.
• Update ops docs and the warehouse-adapter rules to record the verified degrade pattern and the EXPLAIN-based estimation deferral.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/cli/test_generate_estimate.py	Extends CLI estimate patch helper to allow injecting a real adapter; adds Snowflake CLI degrade test and a from-profile dispatch pin.
tests/cli/test_estimate_engine.py	Adds an engine-level Snowflake degrade test and asserts rendered output shape.
plans/super/123-snowflake-estimate-degrade.md	Adds the super plan capturing scope, decisions, and verification strategy for #123.
docs/warehouse-adapter-ops.md	Adds a note confirming end-to-end verification of Snowflake degrade and links the EXPLAIN follow-up.
.claude/rules/warehouse-adapters.md	Records the reusable pattern: verify graceful-degrade using the concrete adapter + adapter-specific NotSupported error at engine + CLI levels.

---

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

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/cli/test_estimate_engine.py`:
- Line 35: The test imports SnowflakeAdapter from a private submodule; update
the import in tests/cli/test_estimate_engine.py so SnowflakeAdapter is imported
from the package-level public warehouse API (the re-exported symbol on
signalforge.warehouse) instead of signalforge.warehouse.adapters.snowflake,
ensuring the test uses the public contract boundary and not a private submodule.

In `@tests/cli/test_generate_estimate.py`:
- Line 34: Replace the private-module import of SnowflakeAdapter with the
package's public API import: change the import of SnowflakeAdapter (currently
from signalforge.warehouse.adapters.snowflake) to import SnowflakeAdapter from
the warehouse package's public surface (e.g. from signalforge.warehouse import
SnowflakeAdapter) so tests depend on the re-exported public symbol rather than
the internal adapter path.
- Around line 397-398: Before calling yaml.safe_load(...) check the fixture file
size via fixture.stat().st_size and compare it to the repository YAML
deserialization size limit constant (use the existing constant name used
elsewhere in the repo, e.g. YAML_DESERIALIZATION_MAX_BYTES); if the size exceeds
the constant raise/assert to fail closed. Insert this pre-load guard immediately
above the line that currently calls yaml.safe_load(...) (the code creating raw
and output) so the file size is validated before reading or deserializing.

🪄 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: 195bf8ee-c16f-4ac8-a83b-a7df06de6b63

📥 Commits

Reviewing files that changed from the base of the PR and between 1f8cc51 and 0e94be5.

📒 Files selected for processing (5)

• .claude/rules/warehouse-adapters.md
• docs/warehouse-adapter-ops.md
• plans/super/123-snowflake-estimate-degrade.md
• tests/cli/test_estimate_engine.py
• tests/cli/test_generate_estimate.py

[wjduenow]

PR Review Summary

Fixed (1 item)

File	Line	Issue	Commit
tests/cli/test_generate_estimate.py	49/74	_install_estimate_patches adapter param/local typed object → tightened to WarehouseAdapter | None / WarehouseAdapter (Copilot)	fbecc8d

False Positives (3 items)

File	Line	Issue	Reason
tests/cli/test_estimate_engine.py	35	"Import SnowflakeAdapter from public API signalforge.warehouse" (CodeRabbit)	No such re-export exists — from signalforge.warehouse import SnowflakeAdapter raises ImportError. The skeleton adapters (SnowflakeAdapter/PostgresAdapter) are deliberately not on the public surface (#119); only BigQueryAdapter + the WarehouseAdapter ABC are. Operators reach Snowflake via WarehouseAdapter.from_profile dispatch. The established convention (pre-existing tests/warehouse/test_snowflake_stub.py) imports the skeleton from the private submodule path. The public-API guideline applies to genuinely public symbols.
tests/cli/test_generate_estimate.py	34	Same import suggestion (CodeRabbit)	Same as above.
tests/cli/test_generate_estimate.py	398	"Add a fail-closed YAML size-cap before yaml.safe_load" (CodeRabbit)	This is test code deserializing its own committed, trusted ~1 KB fixture (snowflake_password.yml). The billion-laughs / size-cap defense exists in production readers (signalforge.diff, signalforge.ingest) that load untrusted user-supplied schema. Cargo-culting it into a test reading a fixture it ships adds noise, not safety.

CI green across the 3.11/3.12/3.13 matrix + docs-build. Zero production code changed (DEC-001).