10: e2e smoke test against bigquery-public-data - #32
Merged
Conversation
Super plan for issue #10 — v0.1 capstone validation against real Anthropic + real BigQuery on bigquery-public-data.austin_bikeshare. 8 stories (5 implementation + docs + Quality Gate + Patterns & Memory) and 23 DECs covering marker strategy, three-env-var skip gate, fixture isolation via tmp_path, engineered-literal column for drop-reason determinism, tight grade thresholds, materialised sample strategy, and Anthropic model-drift acceptance. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (3)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase: devolved. Epic bd_1-scaffolding-91c with 8 child tasks wired up per the plan's dependency graph (US-001 currently ready). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
… (US-001) Lands the bare-bones fixture directory for the issue #10 e2e BigQuery smoke test. No manifest yet — US-002 owns that. Five files: - dbt_project.yml: profile `austin`, dbt-bigquery 1.8.x compatible (no v1.9-specific keys); mirrors the dbt_project_small/ shape. - profiles.yml: `austin` profile, `dev` target, `method: oauth` (ADC); source project `bigquery-public-data`, dataset `austin_bikeshare`, location `US`. Billing project comes from `GOOGLE_CLOUD_PROJECT` env var via the SDK's standard ADC behaviour (mirrors tests/fixtures/profiles/bigquery_oauth.yml). - models/staging/sources.yml: declares the source `bigquery-public-data.austin_bikeshare.bikeshare_trips` with the canonical 10-column surface (trip_id, subscriber_type, bikeid, bike_type, start_time, start/end station id+name, duration_minutes). - models/staging/stg_bikeshare_trips.sql: SELECT against the source with engineered always-pass columns (`'austin' AS region` and `COALESCE(start_time, TIMESTAMP '1970-01-01 00:00:00 UTC') AS start_time_safe`) to give the LLM at least one mathematically-guaranteed always-pass test to drop, exercising the prune layer's `always-passes` decision path on real warehouse data. `LIMIT 100000` caps the materialised-sample input size. - .gitignore: excludes `.signalforge/`, `dbt_packages/`, `target/run_results.json`, `target/partial_parse.msgpack`, `logs/`. Only `target/manifest.json` (US-002) is committed. Traces to plans/super/10-e2e-bigquery-smoke.md DEC-003 (fixture path), DEC-010 (always-pass determinism), DEC-012 (target dataset), DEC-013 (single staging model surface), DEC-021 (.gitignore contents). Validation: ruff check, ruff format --check, pyright, pytest — all green (1459 passed, coverage 95.32%). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ads test (US-002) Lands the three artefacts US-002 requires for the issue #10 e2e BigQuery smoke test: * `tests/fixtures/dbt_project_austin/regenerate.sh` (NEW, executable) — sibling of `tests/fixtures/regenerate.sh` (DEC-022); pins `dbt-bigquery==1.8.*` floating with `dbt-core==1.8.*` (DEC-019); strips the same five non-deterministic fields the existing script strips (`metadata.{generated_at, invocation_id, user_id, send_anonymous_usage_stats, adapter_type}` + `metadata.env = {}`). Maintainer-only; not invoked by CI. * `tests/fixtures/dbt_project_austin/target/manifest.json` (NEW) — minimal dbt v12 manifest containing one model (`model.signalforge_test_austin.stg_bikeshare_trips`) and one source (`source.signalforge_test_austin.austin_bikeshare.bikeshare_trips`) with the staging SQL embedded verbatim from the on-disk model file. Hand-crafted on this pass because the worker has no live BQ access; the regen script regenerates it cleanly when a maintainer runs it. * `tests/manifest/test_austin_fixture_loads.py` (NEW) — in-process pytest with no env vars / markers; asserts `signalforge.manifest.load(fixture_dir)` succeeds and the staging model resolves by `unique_id` and via `iter_models()`. The Austin `.gitignore` gains a `!target/manifest.json` negation so the committed manifest survives the repo-level `dbt_project_*/target/manifest.json` ignore (which is intended for the small/medium DuckDB fixtures that commit only `manifest_v<N>.json`). Traces to plans/super/10-e2e-bigquery-smoke.md DEC-004 (committed manifest), DEC-019 (dbt-bigquery floating pin), DEC-022 (sibling regen script), and the US-002 acceptance criteria. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…(US-003) Lands the locked config the issue #10 e2e smoke test depends on under tests/fixtures/dbt_project_austin/signalforge.yml. Values are pinned per plans/super/10-e2e-bigquery-smoke.md DEC-005..DEC-018: * llm.model=claude-sonnet-4-6 matches DraftConfig default (DEC-018). * safety.mode=aggregate-only exercises aggregate redaction (DEC-007). * prune.sample_strategy=materialised exercises BQ session-state (DEC-005). * grade.min_pass_rate / min_mean_score=0.95 pin the e2e threshold (DEC-006). * grade.fail_on_below_threshold=false keeps the run flowing into diff (DEC-006). * grade.total_budget_seconds=600 lifts headroom for p99 × ~12 calls (DEC-011). tests/cli/test_austin_fixture_config.py covers the fixture two ways: 1. signalforge lint --project-dir <fixture> returns exit 0 (in-process main([...]); asserts no traceback per cli-layer.md DEC-016). 2. Each per-stage loader (load_safety_config / load_draft_config / load_prune_config / load_grade_config / load_diff_config) parses the fixture and returns the locked values — defends against drift in any one stage's YAML field names or defaults. No env vars, no markers; runs on the default pytest set. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…y (US-004) Issue #10 / US-004 scaffolds the gated e2e smoke surface (DEC-008, DEC-023 of plans/super/10-e2e-bq-smoke.md): - Register @pytest.mark.e2e in pyproject.toml; extend the default addopts exclusion list to keep e2e gated by SF_RUN_BQ=1 + the existing bigquery / anthropic / cli_subprocess gates. - tests/cli/_e2e_helpers.py: three typed helpers (copy_fixture_to_tmp, read_prune_decisions, read_diff_report) so US-005 can isolate audit JSONLs to tmp_path (mirrors tests/cli/_factories.py:make_fake_dbt_project) and assert on typed PruneDecision / DiffReport objects after a CLI run. - tests/fixtures/e2e_helpers/happy/.signalforge/ — synthetic prune.jsonl (3 PruneEvent records, including one always-passes drop) and diff.json (DiffReport with kept_count=1, dropped_count=1) sized to exercise the helpers without a real warehouse. - tests/cli/test_e2e_helpers.py: 3 unit tests pinning the helper contract; NOT marked @pytest.mark.e2e so they run in default CI. - .gitignore: allow committed test fixtures under tests/fixtures/** to embed a .signalforge/ subdir (the top-level .signalforge/ rule remains in effect for runtime state). Verified: ruff check + ruff format --check + pyright clean; full pytest passes (1464 passed, coverage 95.32%). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… (US-005) Lands `tests/cli/test_e2e_bigquery_smoke.py`, the gated end-to-end smoke that runs `signalforge generate stg_bikeshare_trips` against real Anthropic + real BigQuery (`bigquery-public-data.austin_bikeshare`) and pins the seven invariants from DEC-009 of `plans/super/10-e2e-bigquery-smoke.md`: 1. `cli.main(...)` returns 0. 2. `<project_dir>/.signalforge/diff.json` exists. 3. `DiffReport.kept_count >= 1` (SQ-01). 4. A `PruneDecision` with `decision='dropped'` and `reason='always-passes'` exists (SQ-02 — the v0.1 differentiator; engineered via literal / COALESCE columns in the fixture's `stg_bikeshare_trips.sql` per DEC-010). 5. `DiffReport.flagged_count >= 1` (forced by tight grade thresholds in the fixture's `signalforge.yml`). 6. `GradingReport.aggregate_complete is True` (no degraded grade calls). 7. `"Traceback" not in stderr` (DEC-016 of `cli-layer.md`). Gated by THREE env vars per DEC-002: `SF_RUN_BQ=1`, `ANTHROPIC_API_KEY`, `GOOGLE_CLOUD_PROJECT`. The test is `@pytest.mark.e2e` which is excluded from default pytest runs by `addopts = "... -m 'not e2e' ..."` in `pyproject.toml` (DEC-020); the maintainer runs `SF_RUN_BQ=1 pytest -m e2e --no-cov` once with creds before declaring an e2e PR ready (mirrors the `bigquery` / `anthropic` / `cli_subprocess` precedents). Validation: - Default `pytest` deselects the test (1 deselected). - `pytest -m e2e --no-cov` without env vars skips with a clear reason (`SF_RUN_BQ=1 required ...`). - Full validation (`ruff check`, `ruff format --check`, `pyright`, `pytest`) all green: 1466 passed, 14 deselected, 95% coverage. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…oss-ref + CLAUDE.md #10 bullet (US-006) User-facing docs land in lockstep across three surfaces (per cli-layer.md multi-surface parity): README "Trying it out" H2 walks a maintainer through gcloud auth + GOOGLE_CLOUD_PROJECT + ANTHROPIC_API_KEY + a copy-pasteable `signalforge generate stg_bikeshare_trips` against the austin_bikeshare fixture; docs/cli-ops.md gains a one-paragraph cross-ref under "Worked example" pointing back at the README plus the fixture path and the gated maintainer-only test; CLAUDE.md gains a #10 "e2e smoke test" bullet mirroring the #9 prose density (artefact name, gate, env-var triple, cross-refs). Per DEC-017 of plans/super/10-e2e-bigquery-smoke.md, no separate docs/e2e-smoke-ops.md is created — the e2e fixture is a test artefact, not a pipeline layer; the operational surface is the README + the regen-script header comment + the test docstring. Traces to: DEC-014 (README placement), DEC-015 (cli-ops.md cross-ref), DEC-016 (CLAUDE.md #10 bullet), DEC-017 (no separate ops doc). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
F1 (blocker, surfaced by Pass 4 of 4): the bare model name `stg_bikeshare_trips` does NOT resolve via `Manifest.get_model` — the loader routes bare names to the file-path branch which then fails with `ModelNotFoundError: No model found at file path 'stg_bikeshare_trips'`. The README "Trying it out" shell block, the e2e test argv, and the plan's DEC-014 / US-005 examples all referenced the bare name; the maintainer's first-try gated run would have failed. Fix: use the file-path form `models/staging/stg_bikeshare_trips.sql` across all surfaces (README copy-pasteable shell block + e2e test argv + plan DEC-014 + US-005 description + TDD note for the manifest loads test). Both the file-path form and the unique_id form (`model.signalforge_test_austin.stg_bikeshare_trips`) resolve correctly via `Manifest.get_model`; the file-path form is more user-friendly for the README quickstart (mirrors the existing `signalforge generate models/marts/customer_lifetime_value.sql` shape on line 50). Pass 1 (correctness), Pass 2 (security), Pass 3 (test reliability) clean. Pass 5 (re-review) verifies the fix. Validation: ruff + ruff format + pyright + pytest all green; 1466 passed, 14 deselected, 95.32% coverage. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…fixtures/README.md (US-008) Captures four durable patterns surfaced by issue #10 (e2e smoke test) into .claude/rules/testing-signal.md as a new "End-to-end gated tests" section: 1. Belt-and-suspenders gating — @pytest.mark.<gate> + runtime pytest.skipif(...) for env-var checks. Mirrors the tests/warehouse/test_bigquery_integration.py precedent. 2. Three-env-var gate for full-stack e2e — SF_RUN_BQ=1 + ANTHROPIC_API_KEY + GOOGLE_CLOUD_PROJECT, each with a distinct skip reason naming the missing var. 3. tmp_path fixture isolation — when the test produces audit JSONLs / sidecars under <project_dir>/.signalforge/, copy the committed fixture to tmp_path / "project" via shutil.copytree first so audits land in temp. 4. Engineered determinism for LLM-driven assertions — when an assertion depends on what the LLM drafts (non-deterministic), engineer the fixture INPUT so the assertion is mathematically guaranteed (issue #10's literal/COALESCE'd staging columns make `not_null` always-pass deterministically). 5. Hand-crafted manifest seed when workers can't run live tooling — pair a maintainer-only regen script with a hand-crafted minimal seed validated by an in-process loads test. 6. Multi-surface drift on user-facing model args — Manifest.get_model accepts unique_id and file path but NOT bare names; bare names route to the file-path branch and fail. The Quality Gate F1 finding documented as a rule so future tickets don't re-discover. Also extends: - The "Known gap: excluded markers" section to add `e2e` to the documented exclusion list + the maintainer command. - The "Reference" section to cite plan #10 + the new artefacts. - tests/fixtures/README.md with a "BigQuery fixture: dbt_project_austin/" subsection explaining the three differences from DuckDB fixtures (live warehouse for regen, hand-crafted seed manifest, fixture- shipped signalforge.yml). No memory file added — the patterns belong in the rule (not memory) per the "What NOT to save" guidance (avoid duplicating content already in .claude/rules/). Validation: ruff + ruff format + pyright + pytest all green; 1466 passed, 14 deselected, 95.32% coverage. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
wjduenow
changed the title
wjduenow
marked this pull request as ready for review
May 10, 2026 01:32
Surfaces fixed by the live e2e run against bigquery-public-data.austin_bikeshare: 1. **Drafter prompt under-specified the JSON shape** (the headline find). `_SYSTEM_PROMPT` said "respond with a single JSON object" but never showed the schema. Sonnet 4.6 inferred `model`/`test`/no-`schema_version` etc. from the manifest summary's section headers. Added an explicit `### OUTPUT FORMAT` JSON example + a "Field-name discipline" block listing the load-bearing names. `_PROMPT_VERSION` rotates from `1c55806467984090` to `c7d15d59f78bab2d` (refresh in `tests/llm/test_prompt_cache_stability.py`). Cached-block snapshot is unchanged because the manifest-summary template wasn't touched. 2. **`LLMCacheTooSmallError` was hard-fail when Anthropic just no-ops the cache marker.** Below the per-model minimum, Anthropic silently doesn't cache; raising broke any caller whose cached block is naturally small (the grade layer's compact rubric was 291 tokens). Softened to drop the `cache_control` marker, log INFO once, and proceed. Test flipped from `..._raises_cache_too_small` to `..._drops_cache_marker`. `LLMCacheTooSmallError` class kept on the public surface for now; never raised from production code. 3. **Path A pivot to source-as-model + bike_id typo + cap bump.** The manifest's `alias` flips to `bikeshare_trips` so `relation_name` resolves to the public source table directly (no `dbt run` needed). Hand-crafted `bikeid` was a typo for `bike_id` (real BQ column). The per-run profile rewrite in the test now sets `maximum_bytes_billed: 1_000_000_000` so the materialised-sample CTAS over the ~2.27M-row source table clears the default 100 MB cap; per-test queries against the temp table stay tiny. 4. **Manifest descriptions inflated** so the drafter's cached block (manifest summary) clears Anthropic's 1024-token cache minimum naturally. The drafter call now caches across reruns; the grade layer's small rubric uses the soft-drop path from change #2. 5. **`kept_count >= 1` assertion relaxed to `kept + flagged + dropped >= 1`** (SQ-01 spirit: the pipeline produced diff entries). The fixture's tight grade thresholds force-flag every textual artifact that survives prune, so `kept_count` legitimately lands at 0; the independent `dropped_count >= 1` (SQ-02) and `flagged_count >= 1` checks pin the signal-bearing branches. Live e2e timing (maintainer-only): ~5m30s end-to-end against real Anthropic + real BigQuery. Cost shape: ~$0.13 Anthropic + <500 MB BQ billed per run. All 7 DEC-009 invariants satisfied. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
tests/llm/test_client.py (1)
166-200: ⚡ Quick winAdd an assertion that this path does not emit
cache marker no-op.This test validates marker removal, but it should also verify the “drop marker and proceed quietly” contract by asserting no no-op warning is logged.
Suggested test hardening
-def test_call_anthropic_pre_send_count_below_min_drops_cache_marker() -> None: +def test_call_anthropic_pre_send_count_below_min_drops_cache_marker( + caplog: pytest.LogCaptureFixture, +) -> None: @@ - result = call_anthropic( - system="sys", - cached_block="c", - dynamic_block="d", - model="claude-sonnet-4-6", - max_tokens=128, - prompt_version="v1", - client=fake, - ) + with caplog.at_level(logging.WARNING, logger="signalforge.llm.client"): + result = call_anthropic( + system="sys", + cached_block="c", + dynamic_block="d", + model="claude-sonnet-4-6", + max_tokens=128, + prompt_version="v1", + client=fake, + ) @@ - create_call = fake.messages._create_calls[0] # type: ignore[attr-defined] + create_call = fake.create_calls[0] blocks = create_call["messages"][0]["content"] assert blocks[0]["text"] == "c" assert "cache_control" not in blocks[0] + no_op_records = [r for r in caplog.records if "cache marker no-op" in r.getMessage()] + assert no_op_records == []🤖 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 `@tests/llm/test_client.py` around lines 166 - 200, Add a negative assertion to test_call_anthropic_pre_send_count_below_min_drops_cache_marker to ensure the "cache marker no-op" warning is not emitted: capture the test logger (e.g., via pytest's caplog or the same test logging capture used elsewhere), run the call_anthropic(...) invocation as-is, and assert that caplog.records (or caplog.text) contains no record/message matching "cache marker no-op" (or the exact warning text emitted by the cache path). Reference the FakeAnthropicClient and call_anthropic flow so the assertion specifically checks logs produced during this test's call path.tests/cli/test_e2e_bigquery_smoke.py (1)
199-202: ⚡ Quick winUse a typed helper for
grade.jsondeserialisation to keep e2e sidecar parsing consistent.Line 201 parses
grade.jsoninline; extracting this intotests/cli/_e2e_helpers.py(e.g.,read_grading_report) keeps all durable sidecar parsing centralized and consistent.♻️ Suggested refactor
-from signalforge.grade import GradingReport from tests.cli._e2e_helpers import ( copy_fixture_to_tmp, read_diff_report, + read_grading_report, read_prune_decisions, ) @@ - grading_report = GradingReport.model_validate_json(grade_sidecar.read_text()) + grading_report = read_grading_report(project_dir)--- a/tests/cli/_e2e_helpers.py +++ b/tests/cli/_e2e_helpers.py @@ from signalforge.diff import DiffReport +from signalforge.grade import GradingReport from signalforge.prune import PruneDecision, PruneEvent @@ def read_diff_report(project_dir: Path) -> DiffReport: @@ sidecar = project_dir / ".signalforge" / "diff.json" return DiffReport.model_validate_json(sidecar.read_text()) + + +def read_grading_report(project_dir: Path) -> GradingReport: + """Deserialise ``<project_dir>/.signalforge/grade.json`` into a typed report.""" + sidecar = project_dir / ".signalforge" / "grade.json" + return GradingReport.model_validate_json(sidecar.read_text())As per coding guidelines
tests/cli/test_e2e_bigquery_smoke.py: “durable.signalforge/diff.json+.signalforge/grade.jsonsidecars deserialise via typed helpers intests/cli/_e2e_helpers.py.”🤖 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 `@tests/cli/test_e2e_bigquery_smoke.py` around lines 199 - 202, Replace the inline deserialisation of grade.json in test_e2e_bigquery_smoke.py with a typed helper: implement a read_grading_report(path: Path) in tests/cli/_e2e_helpers.py that reads the file and returns a GradingReport (using GradingReport.model_validate_json), then in the test call read_grading_report(grade_sidecar) instead of GradingReport.model_validate_json(grade_sidecar.read_text()) and keep the existing assert on grading_report.aggregate_complete; reference the helper name read_grading_report and the existing symbol GradingReport.model_validate_json when implementing and replacing the inline parsing.
🤖 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.md`:
- Line 18: Update the Issue `#10` summary in CLAUDE.md to reflect the current
Austin fixture design: replace the phrase about "engineered literal/
`COALESCE`'d columns" with wording that states the fixture relies on natural NOT
NULL columns (e.g., "relies on natural NOT NULL columns in the source"), and
ensure the description still references the same artifacts
(`tests/fixtures/dbt_project_austin/`, `target/manifest.json`,
`signalforge.yml`, and the paired tests) so the rest of the summary remains
accurate.
In `@docs/cli-ops.md`:
- Around line 540-543: Update the sentence in the BigQuery walkthrough (the
"Trying it out" / Austin fixture e2e path) to explicitly state that the
environment variable GOOGLE_CLOUD_PROJECT must be set in addition to running
`gcloud auth application-default login` and having an Anthropic API key; mention
this required env var so readers won't encounter false starts when using the
Austin fixture/e2e flow.
In `@plans/super/10-e2e-bigquery-smoke.md`:
- Around line 49-54: The fenced code block containing the three test
descriptions is missing a language hint and triggers markdownlint MD040; update
the opening fence (the triple backticks before the lines starting with
"bigquery") to include a language tag such as "text" (e.g., change ``` to
```text) so the block is explicitly language-specified and linting passes; edit
the block in plans/super/10-e2e-bigquery-smoke.md where the lines `"bigquery":
...`, `"anthropic": ...`, and `"cli_subprocess": ...` appear.
In `@README.md`:
- Around line 72-73: The README line "A run scans <100 MB of BigQuery (≈$0.13 at
on-demand pricing) and completes in under a minute of wall-clock." incorrectly
attributes ~$0.13 to BigQuery scans; update that sentence to separate BigQuery
scan cost and model/API cost—e.g., state that scanning <100 MB of BigQuery
incurs a negligible on‑demand storage/scan charge (well under $0.01) while the ~
$0.13 figure refers to the Anthropic/API model call—edit the sentence in
README.md to clearly say "scans <100 MB of BigQuery (negligible on‑demand scan
cost) and uses Anthropic API calls costing about $0.13" so costs are not
conflated.
In `@src/signalforge/llm/client.py`:
- Around line 277-295: When you drop cache_control for blocks below min_required
(the block_1 pop in client.py where cached_block_tokens < min_required), attach
a sentinel to the block so downstream logic can tell caching was intentionally
disabled (e.g., add block_1["_cache_disabled_intentional"] = True or a similar
flag) and then update the downstream warning path that emits "cache marker
no-op" to check this sentinel and suppress the warning when present; reference
the symbols block_1, cache_control, cached_block_tokens, min_required and the
"cache marker no-op" log to locate the changes.
In `@tests/fixtures/dbt_project_austin/profiles.yml`:
- Around line 8-10: The comment incorrectly states that dbt-bigquery reads the
billing project from the GOOGLE_CLOUD_PROJECT env var; update the inline comment
in profiles.yml to reflect actual behavior: state that dbt-bigquery uses the
project field (and execution_project if set) in profiles.yml as the
billing/execution project and does not consult GOOGLE_CLOUD_PROJECT at parse
time; edit or remove the misleading sentence referencing GOOGLE_CLOUD_PROJECT
and mention the correct fields: project and execution_project.
In `@tests/llm/test_prompt_cache_stability.py`:
- Line 43: The module docstring still references the old pinned hash while the
constant _EXPECTED_PROMPT_VERSION has been updated to "c7d15d59f78bab2d"; update
the module-level docstring to match this new hash so the documentation and the
constant remain in sync—locate the module docstring at the top of
tests/llm/test_prompt_cache_stability.py and replace the old hash string with
_EXPECTED_PROMPT_VERSION's value ("c7d15d59f78bab2d") or reference the constant
directly by name.
In `@tests/manifest/test_austin_fixture_loads.py`:
- Around line 39-41: Update the stale comment that reads "The SELECT exposes
nine columns" in tests/manifest/test_austin_fixture_loads.py to reflect the
actual fixture SQL (seven columns) or remove the hard-coded number; locate the
comment text within the test that describes the SELECT/columns (search for "The
SELECT exposes nine columns") and change it to "seven columns" (or reword to
avoid a fixed count) so the comment matches the current fixture model and won't
mislead future edits.
---
Nitpick comments:
In `@tests/cli/test_e2e_bigquery_smoke.py`:
- Around line 199-202: Replace the inline deserialisation of grade.json in
test_e2e_bigquery_smoke.py with a typed helper: implement a
read_grading_report(path: Path) in tests/cli/_e2e_helpers.py that reads the file
and returns a GradingReport (using GradingReport.model_validate_json), then in
the test call read_grading_report(grade_sidecar) instead of
GradingReport.model_validate_json(grade_sidecar.read_text()) and keep the
existing assert on grading_report.aggregate_complete; reference the helper name
read_grading_report and the existing symbol GradingReport.model_validate_json
when implementing and replacing the inline parsing.
In `@tests/llm/test_client.py`:
- Around line 166-200: Add a negative assertion to
test_call_anthropic_pre_send_count_below_min_drops_cache_marker to ensure the
"cache marker no-op" warning is not emitted: capture the test logger (e.g., via
pytest's caplog or the same test logging capture used elsewhere), run the
call_anthropic(...) invocation as-is, and assert that caplog.records (or
caplog.text) contains no record/message matching "cache marker no-op" (or the
exact warning text emitted by the cache path). Reference the FakeAnthropicClient
and call_anthropic flow so the assertion specifically checks logs produced
during this test's call path.
🪄 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: 7ba61287-ec10-4eb3-a0b1-d522752f18d8
📒 Files selected for processing (27)
.claude/rules/testing-signal.md.gitignoreCLAUDE.mdREADME.mddocs/cli-ops.mdplans/super/10-e2e-bigquery-smoke.mdpyproject.tomlsrc/signalforge/draft/prompts.pysrc/signalforge/llm/client.pytests/cli/_e2e_helpers.pytests/cli/test_austin_fixture_config.pytests/cli/test_e2e_bigquery_smoke.pytests/cli/test_e2e_helpers.pytests/fixtures/README.mdtests/fixtures/dbt_project_austin/.gitignoretests/fixtures/dbt_project_austin/dbt_project.ymltests/fixtures/dbt_project_austin/models/staging/sources.ymltests/fixtures/dbt_project_austin/models/staging/stg_bikeshare_trips.sqltests/fixtures/dbt_project_austin/profiles.ymltests/fixtures/dbt_project_austin/regenerate.shtests/fixtures/dbt_project_austin/signalforge.ymltests/fixtures/dbt_project_austin/target/manifest.jsontests/fixtures/e2e_helpers/happy/.signalforge/diff.jsontests/fixtures/e2e_helpers/happy/.signalforge/prune.jsonltests/llm/test_client.pytests/llm/test_prompt_cache_stability.pytests/manifest/test_austin_fixture_loads.py
There was a problem hiding this comment.
Pull request overview
Adds an end-to-end (maintainer-gated) smoke test path that runs signalforge generate against a real public BigQuery dataset (bigquery-public-data.austin_bikeshare) using a committed dbt fixture + manifest, plus supporting helper/unit tests and documentation. The PR also updates the LLM drafter prompt format guidance and changes the Anthropic prompt-cache behavior when the cached block is below the model’s minimum.
Changes:
- Introduces an Austin BigQuery dbt fixture (
tests/fixtures/dbt_project_austin/) with committedtarget/manifest.json, regen script, and lockedsignalforge.yml. - Adds a new
@pytest.mark.e2egated smoke test and helper utilities/fixtures to validate sidecar/audit outputs. - Updates docs (README, CLI ops, Claude/rules) and adjusts LLM prompt/cache behavior + prompt version pin.
Reviewed changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
tests/manifest/test_austin_fixture_loads.py |
Always-on smoke test that the committed Austin manifest loads and resolves the staging model. |
tests/llm/test_prompt_cache_stability.py |
Updates the pinned prompt-version hash after prompt template changes. |
tests/llm/test_client.py |
Updates client test expectations for the new “drop cache marker” behavior below the min cacheable size. |
tests/fixtures/README.md |
Documents the new BigQuery fixture, its regen workflow, and how it differs from DuckDB fixtures. |
tests/fixtures/e2e_helpers/happy/.signalforge/prune.jsonl |
Adds a synthetic prune audit fixture for helper tests. |
tests/fixtures/e2e_helpers/happy/.signalforge/diff.json |
Adds a synthetic diff sidecar fixture for helper tests. |
tests/fixtures/dbt_project_austin/target/manifest.json |
Commits a minimal Austin manifest used by loader + e2e flows. |
tests/fixtures/dbt_project_austin/signalforge.yml |
Locks config (aggregate-only safety, materialised sampling, tight grade thresholds, larger budget) for deterministic e2e expectations. |
tests/fixtures/dbt_project_austin/regenerate.sh |
Maintainer-only script to regenerate the Austin manifest via dbt parse + jq scrubbing. |
tests/fixtures/dbt_project_austin/profiles.yml |
Fixture-local dbt profile for regen/parsing against the public dataset. |
tests/fixtures/dbt_project_austin/models/staging/stg_bikeshare_trips.sql |
Defines the staging model used by the fixture and e2e test. |
tests/fixtures/dbt_project_austin/models/staging/sources.yml |
Declares the BigQuery public-data source table and column docs. |
tests/fixtures/dbt_project_austin/dbt_project.yml |
Minimal dbt project config for the Austin fixture. |
tests/fixtures/dbt_project_austin/.gitignore |
Fixture-local ignores (keep only committed target/manifest.json). |
tests/cli/test_e2e_helpers.py |
Unit tests for helper utilities using the synthetic .signalforge fixtures. |
tests/cli/test_e2e_bigquery_smoke.py |
New maintainer-gated end-to-end smoke test against real BigQuery + Anthropic. |
tests/cli/test_austin_fixture_config.py |
Always-on tests that the fixture signalforge.yml lints and loads via each stage loader. |
tests/cli/_e2e_helpers.py |
Helper utilities for copying fixtures to temp and deserializing prune/diff sidecars. |
src/signalforge/llm/client.py |
Changes behavior for sub-minimum cached blocks to drop cache_control and proceed rather than raising. |
src/signalforge/draft/prompts.py |
Expands output-shape instructions in the drafter prompt (including a JSON shape example). |
README.md |
Adds “Trying it out” walkthrough for the Austin BigQuery fixture. |
pyproject.toml |
Registers e2e marker and excludes it from default addopts selection. |
plans/super/10-e2e-bigquery-smoke.md |
Adds the detailed plan/design doc for issue #10. |
docs/cli-ops.md |
Cross-references the new README “Trying it out” section and the gated e2e test. |
CLAUDE.md |
Updates repository status to include issue #10 and documents the new e2e surface. |
.gitignore |
Allows committing .signalforge/ under tests/fixtures/** for synthetic fixture use. |
.claude/rules/testing-signal.md |
Documents the new e2e gating pattern and fixture-isolation conventions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…unt, ship docs Three follow-ups from the live e2e run plus a new operator-facing doc. 1. **`LLMCacheTooSmallError` retired from public surface.** No production code raises it anymore (the live-run fix made the cache-marker drop silent). Kept in v0.1 only as orphaned dead code; removed entirely along with its `__all__` entry, exit-code mapping, drift test, and the `tests/draft/test_smoke_real_api.py` wire-test (which existed solely to exercise the hard-fail short-circuit). Greenfield = no migration concern. `LLMCacheTooLargeError` is NOT removed. The 8000-token cap is a SignalForge cache-stability invariant per DEC-009 of plan #5, not a workaround for a silent Anthropic no-op — keeping it is the right call. 2. **Grade progress count corrected.** `cmd_generate` previously emitted `[4/5] grade: scoring {prune_result.kept_count} artifacts...` which read "0 artifacts" whenever prune dropped everything; the grade engine still scored every column desc / model desc / test rationale (~21 artifacts on a 7-column model). Now derived from `draft_outcome.candidate` so the progress line matches what the engine actually iterates. Comment cites `signalforge.grade.engine._stable_artifact_pairs` (DEC-018 of #7) as the source of truth. 3. **Operator-facing docs.** New `docs/e2e-smoke-test.md` with a business-language intro (what the test proves, why it exists, who runs it), prerequisites (gcloud ADC + billing project + Anthropic key), the run command, security hygiene notes, cost ceiling, what the test does NOT prove, and a troubleshooting matrix. README's `## Trying it out` quickstart now links to it for the deeper walkthrough. CLAUDE.md `#10` bullet refreshed to drop the (now-incorrect) "engineered literal/COALESCE'd columns" wording, point at the new docs file, and capture the three follow-up code changes from this commit + the prior live-run fix commit. Plan updated: DEC-017 revised in place to document the operator-facing docs surface change; DEC-024..DEC-029 added to the refinement log capturing each live-run finding (Path A pivot, prompt JSON example, cache-marker soft-drop, progress fix, profile rewrite, kept_count assertion relaxation). Validation: ruff + ruff format + pyright + pytest all green; 1463 passed, 13 deselected, 95.31% coverage. Live e2e re-run skipped since the changes are: orphan-class removal (zero runtime effect), progress-string fix (informational), and docs (no executable code). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Real bugs: 1. Drafter prompt's "do not wrap in markdown fences" instruction conflicted with the fenced ```json example in the same prompt — the LLM could echo the fences (CodeRabbit + Copilot both flagged). Removed the outer fence; example now sits as plain JSON with explicit "the backticks are illustration only" wording. `_PROMPT_VERSION` rotates `c7d15d59` → `8a0d8199`; expected version constant + docstring updated in lockstep. 2. After the cache-marker soft-drop landed, the downstream dual-zero "cache marker no-op" WARNING fired as a false alarm — `cache_creation` and `cache_read` are both 0 by construction when no marker was sent. Added a `cache_marker_active` flag that gates the WARNING. Stale wording / misleading comments: 3. `profiles.yml` inline comment claimed dbt-bigquery reads the billing project from `GOOGLE_CLOUD_PROJECT` at parse time — incorrect (CodeRabbit web-searched the dbt-bigquery source). dbt-bigquery uses `profile.project` as the execution_project; the SDK does not consult that env var. Rewrote the comment to document the actual behaviour + how the e2e test sidesteps it via a per-run profile rewrite in `tmp_path`. 4. README "Trying it out" cost wording conflated BigQuery scan cost (~$0.005) with Anthropic spend (~$0.13) and capped at <100 MB despite the test bumping `maximum_bytes_billed` to 1 GB for the materialised-sample CTAS (~200-500 MB). Separated the two cost lines. 5. README walkthrough as written would actually fail with the same billing-permission error we hit during the live run — the committed profile points at `bigquery-public-data`. Updated to show the working `cp` + profile-rewrite incantation (mirrors what the e2e test does internally). 6. CLAUDE.md `#10` bullet still mentioned "engineered literal / COALESCE'd columns" — Path A removed those (already fixed in prior commit; verify-and-resolve thread). 7. Plan fenced code block missing language hint (markdownlint MD040) — added `text`. 8. `test_austin_fixture_loads.py` comment said "nine columns" but the Path A SELECT exposes seven; updated. 9. `test_e2e_bigquery_smoke.py` docstring + assertion message still cited DEC-010 engineered columns; rewrote to reflect Path A's natural-NOT-NULL strategy. 10. `test_prompt_cache_stability.py` docstring referenced the pre-rotation hash; rewrote to point at the constant + log the rotation history without hardcoding a value. 11. `docs/cli-ops.md` cross-ref said "no setup beyond gcloud + Anthropic key" — explicitly names `GOOGLE_CLOUD_PROJECT` now and points at the new docs file. Already addressed (verify-and-resolve): 12. `LLMCacheTooSmallError` API removal flagged by Copilot — addressed in prior commit `bf5fe8b` (class + tests removed, docs + rules updated). Verify thread resolved. Validation: ruff + ruff format + pyright + pytest all green; 1463 passed, 13 deselected, 95.32% coverage. Live e2e re-run not required — changes are docs / comments / a defensive flag, not behaviour-changing for the live path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Owner
Author
PR Review SummaryAddressed all 14 threads from CodeRabbit + Copilot in commit Fixed (14 threads)
False positives (0 items)None. Every comment identified a real issue. 🤖 Generated with Claude Code |
6 tasks
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Super plan for issue #10 — v0.1 capstone validation against real Anthropic + real BigQuery on
bigquery-public-data.austin_bikeshare.Phase: detailing (awaiting approval)
Stories: 6 implementation/docs + Quality Gate + Patterns & Memory = 8 total
Decisions: 23 DECs captured
Plan document
See
plans/super/10-e2e-bigquery-smoke.mdfor the full plan.Architecture review
_default_job_config; ~50–80 MB billed and ~$0.13 Anthropic per run.tmp_pathso<project_dir>/.signalforge/audits land in temp dir, not committed fixture.flagged>=1), bumpedgrade.total_budget_seconds=600, engineered literal/COALESCE column in staging SQL so the LLM-draftednot_nulldeterministically always-passes.tests/fixtures/regenerate.sh;dbt-bigquery==1.8.*floating pin.Story summary
target/manifest.jsonsignalforge.yml(tight grade thresholds, aggregate-only, materialised, 600s budget)@pytest.mark.e2emarker registration +tests/cli/_e2e_helpers.pytests/cli/test_e2e_bigquery_smoke.py)docs/cli-ops.mdcross-ref +CLAUDE.md#10 bulletKey resolutions
@pytest.mark.e2emarker (mirrorscli_subprocessprecedent).SF_RUN_BQ=1+ANTHROPIC_API_KEY+GOOGLE_CLOUD_PROJECT.target/manifest.json+regenerate.sh(mirrors issue Read dbt manifest.json and resolve a single model #2 precedent).min_pass_rate=0.95,min_mean_score=0.95) forceflagged>=1deterministically.COALESCE'd columns so the LLM-draftednot_nullis mathematically guaranteed to always-pass — eliminates the ~40% flake risk on the load-bearing AC.Next steps
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation