#136: OpenAI grading provider (plan) - #152
Merged
Merged
Conversation
Phase: detailing (awaiting approval). 13 DECs captured: Chat Completions API, no provider/model cross-validation, fully wire --estimate for OpenAI (new estimate_input_tokens ABC), default judge gpt-4o, scope both grade + draft, server-side response_format=json_object, four pricing SKUs (gpt-4o, -mini, 4.1, 4-turbo), live smoke covers grade + estimate, .messages adapter wraps chat.completions, AST Scan 3 extension, [openai] extra + lazy tiktoken, Anthropic estimate byte-identity floor. 9 stories: shim + AST confinement, OpenAIProvider + registration, FakeOpenAIClient + neutrality test, pricing entries, --estimate provider-aware counting, live gated smokes, docs, Quality Gate, Patterns & Memory. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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 (4)
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:
📝 WalkthroughWalkthroughThis PR implements OpenAI as a second pluggable LLM provider for grading and drafting, adding a provider-neutral seam (LLMProvider ABC + registry), OpenAI SDK adapter shim, provider-aware token estimation for ChangesOpenAI Pluggable Provider Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes This PR introduces a substantial provider-neutral architectural seam with multiple interacting components: provider ABC and registry, OpenAI SDK adapter, token-estimation refactoring, pricing updates, and comprehensive test coverage spanning unit/integration/confinement/audit/live-API layers. The changes are heterogeneous (spanning architecture, CLI, pricing, tests, and docs) and require reasoning about provider dispatch patterns, token-counting byte-identity contracts, SDK isolation via AST audits, and capability-flag-gated behavior. While individual ranges follow a consistent pattern, the overall complexity comes from cross-cutting concerns (provider dispatch in estimate, capability flags in grading/drafting, SDK confinement in audit) that demand careful integration review. Possibly related issues
Possibly related PRs
Poem
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Cross-review against #137 (Gemini grading) plan surfaced 5 substantive gaps in the OpenAI plan: 1. DEC-010 — fix wording: a NEW 9th AST scan (Scan 3 is Anthropic-specific), not an extension. Reuse _QualifiedNameCallFinder + three-pattern bypass regression test per testing-signal.md. 2. DEC-012 — fix pyproject.toml listing: three slots in lockstep ([project.optional-dependencies].openai + .dev + [dependency-groups].dev), mirroring Snowflake precedent per python-build.md. Plan previously named only two. 3. DEC-014 (new) — codify the _load_openai_exception_classes empty-tuple fallback on ImportError (mirrors AnthropicProvider). Includes refusal / content-filter symmetry note explaining why OpenAI needs no Gemini-style safety_filter typed-degrade DEC. 4. US-006 — add a third live test (draft_schema) honouring DEC-005's "scope both stages" commitment at live level too. DEC-008 updated to match (three tests, not two). 5. New "Open notes for implementation" section — pragmatic SDK-class-name verification, .messages adapter shape confirmation, tiktoken fallback table, response_format prompt-requirement check, Anthropic byte-identity snapshot capture protocol. Plus a new "Worker-writability routing" section codifying that US-009 (Patterns & Memory) is orchestrator-only because it edits .claude/rules/, mirroring #137's same routing. Plan grows from 319 → 343 lines (+40/-17). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
wjduenow
added a commit
that referenced
this pull request
May 28, 2026
DEC-019 was documentary only; bd_1-scaffolding-txe.7 (US-007) showed as bd-ready despite the plan saying "wait for #136 to merge to dev." A Ralph worker running `bd ready` would have picked it up and rebase-fought #136 on providers.py / pricing.py / cli/_estimate.py exactly as DEC-019 warns against. Created sentinel bead bd_1-scaffolding-41a ("#136 OpenAI grading PR #152 merged to dev") and wired bd_1-scaffolding-txe.7 to depend on it. `bd ready` no longer surfaces US-007 until the sentinel closes. Close the sentinel the moment #136 merges to dev → US-007 unblocks automatically. Updated DEC-019 + the beads manifest entry on US-007 to point at the sentinel. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a planning document for implementing OpenAI as a second LLM provider behind the provider-neutral seam introduced in #135. The plan covers shim creation, provider registration, pricing entries, --estimate integration via tiktoken, gated live tests, and documentation updates across 7 implementation stories plus Quality Gate and Patterns & Memory stories.
Changes:
- Adds a single new plan document under
plans/super/describing the OpenAI grading + drafting provider work.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wjduenow
marked this pull request as ready for review
May 28, 2026 06:26
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plans/super/136-openai-grading-provider.md (1)
103-103: 💤 Low valueConsider clarifying "dual-listed across all three dev slots" terminology.
The phrase "dual-listed across all three dev slots" might momentarily confuse readers. It means two packages (openai + tiktoken) each listed in three pyproject.toml locations. Consider rephrasing to "two packages (openai + tiktoken) listed in three pyproject.toml slots" for immediate clarity, or accept as-is since line 119 in US-001 makes the implementation clear.
🤖 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/136-openai-grading-provider.md` at line 103, Update the DEC-012 sentence to clarify that two packages are listed in three locations: replace "dual-listed across all three dev slots" with a clearer phrase such as "two packages (openai + tiktoken) listed in three pyproject.toml slots" in the DEC-012 paragraph (the block that also references `_count_openai_tokens(model, text)`), ensuring the text explicitly conveys that both openai and tiktoken appear in the three locations mentioned.
🤖 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.
Nitpick comments:
In `@plans/super/136-openai-grading-provider.md`:
- Line 103: Update the DEC-012 sentence to clarify that two packages are listed
in three locations: replace "dual-listed across all three dev slots" with a
clearer phrase such as "two packages (openai + tiktoken) listed in three
pyproject.toml slots" in the DEC-012 paragraph (the block that also references
`_count_openai_tokens(model, text)`), ensuring the text explicitly conveys that
both openai and tiktoken appear in the three locations mentioned.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9151cf89-59c6-4a16-8229-5e0063b3de33
📒 Files selected for processing (1)
plans/super/136-openai-grading-provider.md
Fills in the Beads manifest section. Epic bd_1-scaffolding-4tw with 9 child tasks (US-001 … US-009) created with dep edges per the plan's "Depends on:" lines: US-001 → none (READY) US-002 → US-001 US-003 → US-002 US-004 → none (parallel-safe) (READY) US-005 → US-002, US-004 US-006 → US-002, US-005 US-007 → US-001, US-002, US-005 US-008 → US-001..US-007 (QG) US-009 → US-008 (P&M; orchestrator-only) Cross-epic gate wired downstream: #137's sentinel bd_1-scaffolding-41a now DEPENDS ON US-009, so #137 US-007 stays mechanically blocked until this epic completes + sentinel closes after PR #152 merges to dev. Parallel-safe at the entry points: US-001 and US-004 edit disjoint files (shim/pyproject/AST vs pricing) — ralph-serialize-shared-registry does not apply. Run concurrently. bd ready (post-devolve) surfaces .1 + .4 of this epic + the two epics themselves + the unrelated Airflow integration. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… siblings) Adds four OpenAI SKUs to _PRICES_MUTABLE per DEC-007: gpt-4o (default judge per DEC-004), gpt-4o-mini (budget), gpt-4.1 (newer flagship), gpt-4-turbo (back-compat). Cache fields = 0.0 (OpenAI has no equivalent cache discount). PRICE_TABLE_VERSION bumped to 2026-05-28. Anthropic SKUs unchanged (byte-identical) — Anthropic estimate byte-identity floor (DEC-013) preserved at the pricing layer. Tests assert: non-zero input/output rates + zero cache fields for all four; unknown model still raises EstimateUnknownModelError; Anthropic SKUs unchanged; version bump pinned. OpenAI per-MTok rates are calibration figures captured at PR-prep time pending operator verification against https://openai.com/api/pricing/; the figures are sanity-check baselines, not billing guarantees. Traces: DEC-003, DEC-004, DEC-007. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…i] extra + 9th AST scan Implements the per-vendor shim confining every openai SDK type ignore. Adds [openai] optional-dep extra (openai + tiktoken) in three-slot lockstep across [project.optional-dependencies].openai, .dev, and [dependency-groups].dev. Adds the new 9th AST scan in tests/test_audit_completeness.py reusing _QualifiedNameCallFinder (NOT an extension of Scan 3 — Scan 3 is Anthropic-specific) and a companion per-file confinement test mirroring Snowflake's. Bumps the docstring tally 8→9. DEC-014 empty-tuple ImportError fallback on _load_openai_exception_classes mirrors AnthropicProvider; tiktoken cl100k_base fallback for unknown model ids. Traces: DEC-001, DEC-009, DEC-010, DEC-012, DEC-014. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…im + [openai] extra + 9th AST scan
… config-validator coverage
Implements OpenAIProvider(LLMProvider) with both capability flags False
(no prompt caching, no pre-send token count) — mirrors FakeNoCacheProvider
end-to-end shape. build_create_kwargs attaches response_format=
{type: json_object} per DEC-006 (server-side JSON enforcement); no
cache_control / extra_headers. extract_text_blocks reads
response.choices[0].message.content; extract_usage maps usage.prompt_tokens
+ completion_tokens → UsageMetrics with cache fields 0. classify_exception
covers all five ExceptionCategory branches via _load_openai_exception_classes
from the US-001 shim.
register_provider(OpenAIProvider()) at module scope so GradeConfig/
DraftConfig validators accept provider='openai'. Exported from
signalforge.llm.__init__.
Tests pin: each ABC method's contract; config validator acceptance for
both stages; UnknownProviderError lists both anthropic and openai.
Traces: DEC-001, DEC-005, DEC-006, DEC-009, DEC-011.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…istration + config-validator coverage
…lity e2e Adds tests/llm/_fake_openai.py with FakeOpenAIClient + expect_messages_create API mirroring FakeAnthropicClient verbatim. Support dataclasses match the real chat-completions response shape (choices[0].message.content; usage with prompt_tokens + completion_tokens). Adds tests/grade/test_provider_neutrality_openai.py — the OpenAI analogue of the FakeNoCacheProvider neutrality proof. Asserts cache_*=0 in JSONL audit, blake2b-8 reproducibility hashes present, sidecar drift-detector round-trip, no dual-zero cache-anomaly WARNING in caplog, all expectations consumed. Traces: DEC-001, DEC-005, DEC-006, DEC-009, DEC-011. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rade neutrality e2e
… counting Adds LLMProvider.estimate_input_tokens(model, text, *, client=None) -> int as an abstract method on the ABC (DEC-003). AnthropicProvider impl delegates to the SDK's messages.count_tokens with a single user-message text envelope; OpenAIProvider impl delegates to _count_openai_tokens (tiktoken with cl100k_base fallback per DEC-012); FakeNoCacheProvider and _DummyProvider impls return trivial deterministic answers so the existing neutrality + registry tests still pass. Refactors cli/_estimate.py: _count_draft_tokens and the grader-side equivalent dispatch through provider_for(config.provider).estimate_input_tokens — no more hard-coded anthropic_client.messages.count_tokens calls. The engine signature relaxes anthropic_client to 'object | None' so the OpenAI path can pass None (tiktoken is local). Lifts the '--estimate currently supports only provider=anthropic' gate in cli/generate.py; the divergent-providers check stays (the engine still takes one optional client). Pins DEC-013 Anthropic byte-identity floor via tests/cli/test_estimate.py::test_estimate_anthropic_byte_identity_golden and tests/fixtures/estimate/anthropic_byte_identity_golden.txt (captured 2026-05-28 before the refactor; reproduced verbatim after). New companion tests prove the OpenAI path produces non-zero token counts + non-zero USD, ignores the threaded anthropic_client, and goes through _count_openai_tokens (patched-helper assertion). Traces: DEC-003, DEC-007, DEC-012, DEC-013. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…aware token counting
…er-writable portion) Updates operator-facing documentation surfaces for OpenAI grading + drafting: - docs/grade-ops.md — OpenAI provider section (config snippet, OPENAI_API_KEY, no-cache caveat, live smoke gating link) - docs/draft-ops.md — equivalent for llm.provider: openai - docs/cost-estimate-ops.md — tiktoken note, [openai] extra install, four pricing SKUs - CHANGELOG.md — [Unreleased] Added entry for #136 - README.md — provider enumeration extended (if applicable) The .claude/rules/llm-drafter.md update is deferred to US-009 (Patterns & Memory) per the project's worker-writability rule (orchestrator-only edits under .claude/). Traces: DEC-001 through DEC-014. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…marker + 3 tests) Adds @pytest.mark.openai marker + three gated live-API tests: - tests/grade/test_smoke_real_api_openai.py — grade_artifacts(provider=openai) - tests/draft/test_smoke_real_api_openai.py — draft_schema(provider=openai) (honors DEC-005 both-stages at live level) - tests/cli/test_e2e_estimate_openai.py — generate --estimate openai All three env-gated on SF_RUN_OPENAI=1 + OPENAI_API_KEY via belt-and- suspenders _skip_reason() helper per testing-signal.md. Marker registered in pyproject.toml + added to addopts exclusion so default pytest doesn't collect them. CONTRIBUTING.md documents the maintainer-only run pattern. Traces: DEC-001, DEC-004, DEC-005, DEC-008. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ts (openai marker + 3 tests)
…om code review x4 Code-review pass 3 surfaced two correctness MAJORS + pass 4 surfaced a doc MINOR; all fixed inline per /ralph-run Step 3b. Pass 1, 2, 4 (other surfaces) reported no correctness bugs. MAJOR 1 — EstimateUnknownModelError.default_remediation enumerated only the three Anthropic SKUs. An operator typo on an OpenAI SKU got pointed at Anthropic options (violates "errors carry remediation" in manifest-readers.md). Updated to enumerate all 7 SKUs (claude-sonnet-4-6, claude-opus-4-7, claude-haiku-4-5, gpt-4o, gpt-4o-mini, gpt-4.1, gpt-4-turbo) + refreshed the locked-text test that pinned the stale string. MAJOR 2 — AnthropicProvider.estimate_input_tokens dropped the pre-refactor `system=` kwarg, concatenating system+cached+dynamic into a single user-content string. Anthropic's server-side tokenizer counts the system block with its own envelope tokens; without the kwarg, real-API counts under-report by the system-envelope size. The fake-driven byte-identity test passed only because the fake returned canned input_tokens regardless of kwargs (so rendered-output identity held, but DEC-013's "byte-identity vs the pre-refactor Anthropic count_tokens call" spirit broke at the real-API level). Fix extends the ABC signature with a keyword-only `system: str = ""` parameter. AnthropicProvider passes it via `system=` kwarg when non-empty (matches pre-refactor real-API call shape). OpenAIProvider concatenates `system + text` before tiktoken (tiktoken has no system-envelope distinction; the total still counts every token). FakeNoCacheProvider concatenates for the word-count proxy. _DummyProvider gains the new kwarg for ABC parity. cli/_estimate.py callers (_count_draft_tokens, _count_grade_criterion_tokens) thread `system=` separately; grade-side preserves the pre-existing double-count of the rubric (passed as both system= AND in user content) to keep byte-identity with the pre-refactor shape. MINOR — docs/grade-ops.md:118 config-snippet comment said "only anthropic registered today", contradicting the same file's later "## OpenAI provider" section and docs/draft-ops.md's correct wording. Updated to mirror draft-ops's form. Validation: ruff/pyright/pytest all green (2498 passed, 62 deselected, 97.34% coverage); wheel_smoke green (2 passed). Note: CodeRabbit skill not available in this environment, skipped per Step 3b "(if available)". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…onventions Orchestrator-only commit (workers can't write under .claude/ in worktrees per ralph-worker-claude-dir-perms.md). Updates .claude/rules/llm-drafter.md to capture three durable patterns from #136: 1. Provider-neutral seam — adds OpenAI as the second registered provider (name="openai", both capability flags False) and extends the LLMProvider ABC surface enumeration with the new estimate_input_tokens(model, text, *, system="", client=None) method (#136 US-005). 2. "OpenAI provider shape" subsection — codifies the .messages.create façade adapter pattern (SimpleNamespace delegating to chat.completions.create), the response_format={type:json_object} server-side JSON enforcement (with the cross-ref to issue #144's tolerant parser as fallback), and the tiktoken cl100k_base fallback for unknown model ids. This becomes the canonical precedent for #137 Gemini's response_mime_type=application/json equivalent. 3. estimate_input_tokens(*, system=...) — documents the load-bearing reason the system envelope is threaded separately (Anthropic's server-side tokenizer applies system-block envelope tokens; dropping the kwarg under-reports real-API counts silently because fake-driven byte-identity tests can't catch call-shape drift). Also pins the deliberate double-count of system_and_rubric in _count_grade_criterion_tokens as pre-existing behaviour that must be preserved. Bumps AST-scan tally section from "four" → "five" (adds the openai.OpenAI 9th-project-scan); flags #137 Gemini as the 10th. Updates the References block with #136 plan + new files. Memory entry: ~/.claude/projects/.../memory/fake-driven-byte-identity- blind-spot.md captures the QG lesson (fakes return canned values regardless of kwargs → call-shape drift slips past rendered-output snapshots → needs explicit kwargs-shape assertion OR live test). Indexed in MEMORY.md. Validation: all four canonical gates green (2498 passed, 97.34% coverage). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@CONTRIBUTING.md`:
- Around line 60-64: The pytest marker expression used for the gated coverage
run currently reads 'bigquery or anthropic or openai or e2e or cli_subprocess or
wheel_smoke' and omits the snowflake marker; update that expression to include
'snowflake' (e.g., 'bigquery or anthropic or openai or e2e or cli_subprocess or
wheel_smoke or snowflake') so the SF_RUN_BQ / SF_RUN_OPENAI gated audit covers
tests excluded-by-default for snowflake as listed earlier in the document.
In `@tests/llm/_fake_provider.py`:
- Around line 223-234: The token-count proxy currently concatenates system and
text as (system + text).split(), which can merge boundary words and undercount;
update the proxy to join with a delimiter (e.g., use f"{system} {text}" or "
".join(filter(None, [system, text]))) before splitting so boundary words remain
separate—locate the return expression in the token proxy method (the line
returning len((system + text).split())) and replace it with a safe join that
preserves spacing.
In `@tests/llm/test_openai_client_confinement.py`:
- Around line 43-47: The loop over _LLM_DIR currently uses glob("*.py") which
only finds top-level files; change it to recursively traverse subdirectories
(e.g., use _LLM_DIR.rglob("*.py") or equivalent recursive glob) so nested
modules are also checked, keeping the rest of the logic intact (the conditional
skip of _SHIM_FILENAME and the call to _openai_type_ignore_lines(py) that
appends to offenders).
In `@tests/test_audit_completeness.py`:
- Around line 1044-1080: The test currently covers three import patterns but
misses the "call-before-import" alias case; add a fourth subtest in
test_attribute_call_finder_catches_all_three_openai_bypass_patterns that
constructs source where the call to the alias O appears before the import alias
(e.g. "def make():\n return O(api_key='x')\nfrom openai import OpenAI as
O\n"), create an _AttributeCallFinder("openai","OpenAI"), visit ast.parse(...)
on that source, and assert len(calls) == 1 with a clear failure message
referencing the late-import alias pattern so the suite detects this bypass
scenario.
🪄 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: ceb184c1-a900-4bc2-9f0a-dade3393974c
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (36)
.claude/rules/llm-drafter.mdCHANGELOG.mdCONTRIBUTING.mdREADME.mddocs/cost-estimate-ops.mddocs/draft-ops.mddocs/grade-ops.mdmkdocs.ymlplans/super/136-openai-grading-provider.mdpyproject.tomlsrc/signalforge/cli/_estimate.pysrc/signalforge/cli/generate.pysrc/signalforge/llm/__init__.pysrc/signalforge/llm/_openai_client.pysrc/signalforge/llm/errors.pysrc/signalforge/llm/pricing.pysrc/signalforge/llm/providers.pytests/cli/test_e2e_estimate_openai.pytests/cli/test_estimate.pytests/cli/test_estimate_engine.pytests/cli/test_generate_estimate.pytests/draft/test_config.pytests/draft/test_schema.pytests/draft/test_smoke_real_api_openai.pytests/fixtures/estimate/anthropic_byte_identity_golden.txttests/grade/test_config.pytests/grade/test_provider_neutrality_openai.pytests/grade/test_smoke_real_api_openai.pytests/llm/_fake_openai.pytests/llm/_fake_provider.pytests/llm/test_client_shim.pytests/llm/test_openai_client_confinement.pytests/llm/test_pricing.pytests/llm/test_providers.pytests/llm/test_public_api.pytests/test_audit_completeness.py
✅ Files skipped from review due to trivial changes (9)
- src/signalforge/llm/errors.py
- CHANGELOG.md
- README.md
- docs/draft-ops.md
- tests/draft/test_schema.py
- docs/cost-estimate-ops.md
- src/signalforge/llm/init.py
- docs/grade-ops.md
- plans/super/136-openai-grading-provider.md
Copilot: - MAJOR _estimate.py:359 — OpenAI grade-side over-counted rubric. The first QG fix preserved a pre-existing Anthropic double-count of system_and_rubric (passed as both system= AND in user content), which triple-counted on OpenAI (system→system+text concat → rubric prefix in text). Corrected to match the runtime grader call: rubric in system= once, artifact envelope in user content. Anthropic real- API counts drop one rubric copy from the buggy pre-refactor bytes; OpenAI counts each input once. Fake-driven byte-identity golden still passes (canned token counts are call-shape-agnostic). CHANGELOG documents the calibration shift. - NIT _estimate.py:421 — renamed anthropic_client → client. Type was already object | None and forwarded to whichever provider strategy is active; old name implied Anthropic-only and would mislead a future #137 Gemini wiring. CLI in generate.py already passed None for non-Anthropic; rename surfaces the contract without behaviour change. - META plan/PR description — addressed by updating the PR body separately (not part of this commit). CodeRabbit: - CONTRIBUTING.md:64 — added snowflake to the gated-marker audit command + the corresponding SNOWFLAKE_* env vars. - tests/llm/_fake_provider.py:234 — boundary-word undercount fix in the FakeNoCacheProvider word-count proxy (f"{system} {text}" with a delimiter; was system+text which merged the last word of system with the first word of text under .split()). - tests/llm/test_openai_client_confinement.py:47 — glob("*.py") → rglob("*.py") so the confinement scan catches openai-mentioning ignore directives in any future nested signalforge/llm/ subpackage, not just top-level files. Path display becomes relative-to-_LLM_DIR. - tests/test_audit_completeness.py:1080 — closed the "call-before- import alias" bypass on _AttributeCallFinder by adding a two-pass visit_Module that pre-collects every alias module-wide before visiting any Call node. Mirrors the same fix already shipped on _QualifiedNameCallFinder (PR #69 / DEC-013). Pattern-4 regression test added. Validation: ruff/format/pyright/pytest all green (2498 passed, 97.34% coverage); wheel_smoke green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Owner
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
|
Actionable comments posted: 0 |
Codecov flagged 91.82% patch coverage with 9 lines missing across 3 files. Closed all 9 with 6 focused unit tests: src/signalforge/llm/_openai_client.py (was 85%, now 100%): - _OpenAIClientAdapter.__init__ + ._messages_create — no production caller exercises the adapter (orchestrator drives via FakeOpenAIClient which has its own .messages.create); added test_openai_client_adapter_messages_create_delegates_to_chat_completions that builds a SimpleNamespace raw client and pins the delegation + kwargs forwarding. - _count_openai_tokens cl100k_base fallback (DEC-012 unknown-model branch) — added test_count_openai_tokens_falls_back_to_cl100k_base_ for_unknown_model. src/signalforge/llm/providers.py (was 98%, now 100%): - AnthropicProvider.estimate_input_tokens no-system-kwarg arm (the else branch added in QG) — added test_anthropic_provider_estimate_ input_tokens_skips_system_kwarg_when_empty, which ALSO pins the load-bearing invariant that an empty system MUST be omitted from the SDK kwargs (otherwise the count carries a spurious system="" block). - LLMResponseFormatError raise on missing input_tokens — added test_anthropic_provider_estimate_input_tokens_raises_on_missing_ input_tokens. - OpenAIProvider.extract_text_blocks missing-message arm — added test_openai_provider_extract_text_blocks_missing_message_attr_ raises (distinct from the existing content=None and empty-choices cases — those exercise different arms). src/signalforge/cli/generate.py:846 (was 66%, now covered): - The non-Anthropic `client = None` branch in cmd_generate's --estimate short-circuit — exercised only by the @pytest.mark.openai live smoke before. Added test_generate_ estimate_openai_provider_passes_client_none_to_engine that drives cmd_generate with llm.provider: openai + grade.provider: openai in a tmp signalforge.yml, spies on AnthropicProvider.make_client to confirm it's NEVER called on the openai path, and captures the client kwarg into estimate(...) to pin client=None. The remaining 7 uncovered lines in generate.py are pre-PR and out of scope. Total: 2504 passed (was 2498), 97.47% coverage (was 97.34%), no new gated tests added (all unit tests, default-CI-included). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
wjduenow
added a commit
that referenced
this pull request
May 28, 2026
#136 PR #152 merged at 27daba5. Pulls in LLMProvider.estimate_input_tokens ABC method (unblocks US-007), OpenAIProvider + _openai_client.py shim, OpenAI pricing SKUs + PRICE_TABLE_VERSION bump, cli/_estimate.py provider-aware token counting + Anthropic byte-identity golden. Conflicts resolved to preserve both providers' work; sentinel bd_1-scaffolding-41a closes after this lands so US-007 unblocks. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> # Conflicts: # CHANGELOG.md # CONTRIBUTING.md # docs/draft-ops.md # docs/grade-ops.md # pyproject.toml # src/signalforge/llm/pricing.py # src/signalforge/llm/providers.py # tests/draft/test_config.py # tests/grade/test_config.py # tests/llm/test_pricing.py # tests/llm/test_providers.py # tests/llm/test_public_api.py # tests/test_audit_completeness.py # uv.lock
wjduenow
added a commit
that referenced
this pull request
May 28, 2026
* #137: Gemini grading provider (plan) Super plan for #137 (Gemini grading provider) — depends on #135 (merged). 9 stories: shim + provider + extra + fake/unit + neutrality + live + docs + QG + P&M. Caching deferred (both capability flags False); safety-filter no-content surfaces as LLMResponseFormatError → grade GradeLLMError degrade. Both drafter and grader covered. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #137: extend plan after comparison with #136 OpenAI plan Closes the three gaps surfaced by the #136 comparison: - Add --estimate integration (DEC-016): GeminiProvider.estimate_input_tokens via native client.models.count_tokens (cleaner than #136's tiktoken path — Gemini has a first-party count endpoint). - Add 3 Gemini pricing SKUs to pricing.py (DEC-017): gemini-2.5-pro, gemini-2.5-flash, gemini-2.0-flash; bump PRICE_TABLE_VERSION. - Add server-side JSON enforcement (DEC-018): response_mime_type= application/json — mirrors #136 DEC-006. - Add CHANGELOG entry to docs story (US-009). - Add wheel_smoke to QG (new [gemini] extra changes packaging). - Sequence after #136 (DEC-019): inherit ABC extension + estimate refactor. New stories: US-006 (pricing, parallel-safe), US-007 (estimate impl, depends on #136). Old US-006/007 renumbered to US-008/009. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #137: apply 3 plan refinements from #136 cross-review Closes the smaller gaps surfaced by re-reviewing #137 against #136 after the #136 plan was tightened (520b36c): 1. DEC-009 — merge-order-conditional AST scan tally. Currently said "8 → 9" unconditionally, but DEC-019 specifies #137 ships AFTER #136. Realistic bump is 9 → 10 (since #136's openai.OpenAI scan bumps 8 → 9 first); added the fallback wording for the slipped-sequencing case. 2. Dedicated "Worker-writability routing" top-level section codifying that Patterns & Memory is orchestrator-only (.claude/rules/ writes). Was a one-line architecture-review row; now a durable section that mirrors #136's codification — future #138+ provider plans copy the pattern. 3. Open notes for implementation — two additions: - response_mime_type requires NO prompt keyword (contrast with OpenAI's response_format which fails without "json" in prompt). Prevents a future maintainer adding defensive prompt text. - pricing.lookup zero cache fields math validation (symmetric to #136 verification item). Major gaps from the prior cross-review (--estimate, pricing, dual-listing, JSON enforcement, CHANGELOG, three live tests, sequencing) were already addressed in b28c0b5. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #137: devolve plan to beads (phase=devolved) Epic: bd_1-scaffolding-txe Tasks: 11 (US-001 through US-009 + Quality Gate + Patterns & Memory) Dependencies: 19 edges per the plan's dependency graph Ready queue: US-001 (shim) + US-006 (pricing, parallel-safe) US-007 (--estimate integration) is blocked on US-002 + US-006 AND carries a cross-epic gate on #136 landing first per DEC-019 — that sequencing is documented in the task's description rather than wired as a bead dep since #136's beads live in a different epic. US-011 (Patterns & Memory) is flagged orchestrator-only because it edits .claude/rules/ which Ralph workers can't write under in worktrees (per the Worker-writability routing section of the plan). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #137: adjust plan now that #136 is being implemented first Folds devolve-time annotations (phase=devolved, beads epic bd_1-scaffolding-txe, per-task status + cross-epic blocker on US-007) together with five #136-driven adjustments: 1. AST-scan tally locked to 10th (not 9th) — #136 owns the 8→9 bump for openai.OpenAI; #137 takes 9→10 for genai.Client. Updated in DEC-009, US-001, P&M, and Open notes. 2. DEC-019 sharpened: #136 plan → #136 implementation; explicit rebase guidance (rebase on dev after #136 merges) rather than contingency wording. 3. DEC-005 gains a refusal/content-filter symmetry note cross-ref'ing #136 DEC-014 — explains why OpenAI deliberately ships no safety-filter typed-degrade and why Gemini deliberately does. 4. New "Worker-writability routing" section mirroring #136's, codifying that P&M is orchestrator-only because it edits .claude/rules/. 5. Open notes extended with two pragmatic items parallel to #136's: - google-genai count_tokens response field-name verification (.total_tokens vs .total_token_count) at US-007 implementation. - Anthropic byte-identity snapshot ownership: #136 owns it; #137 inherits it; the wiring is wrong if it moves the snapshot. Plan grows 762 → 825 lines (+70/-30). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-txe.6: #137 US-006 — add Gemini pricing SKUs (gemini-2.5-pro, gemini-2.5-flash, gemini-2.0-flash) Adds three Gemini SKUs to _PRICES_MUTABLE per DEC-017, each with positive input_per_mtok / output_per_mtok per Google's public Gemini API price page and cache fields = 0.0 (v0.3 ships Gemini without an Anthropic-equivalent prompt-cache discount). Bumps PRICE_TABLE_VERSION to "2026-05-27". Tests parametrise the three SKUs, pin Anthropic SKU byte-identity (additive-only), and assert lookup("gemini-unknown") still raises EstimateUnknownModelError. cli/_estimate.py reads only input_per_mtok and output_per_mtok in the USD math (lines 468, 511), so zero cache fields are safe — no divide-by-zero or NaN risk. The EstimateUnknownModelError.default_remediation in llm/errors.py still lists only the three Claude SKUs by name; per the story scope ("pricing-only; DO NOT touch any other file") this is left for a follow-up to broaden once the wider Gemini surface (provider class + --estimate integration in US-007) lands. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-txe.1: #137 US-001 — _gemini_client.py shim + new AST confinement scan Land the per-vendor Gemini SDK shim and the 9th AST audit-completeness scan that confines genai.Client(...) constructions to it. Mirrors the Anthropic shim shape verbatim — GeminiClientProtocol (with .messages façade for US-002 and .models for US-007), lazy-import factory, frozen _GeminiExceptionClasses dataclass with empty-tuple fallback per DEC-015 so the module imports cleanly without the [gemini] extra installed. The new scan extends _AttributeCallFinder with an optional parent_module parameter so the namespace-package shape `from google import genai; genai.Client(...)` is caught alongside `from google.genai import Client` and its alias variant — five planted-violation regression tests pin the three bypass patterns plus a negative case and an Anthropic-path-unchanged guard. A line-based confinement test (tests/llm/test_gemini_client_confinement.py) rejects any google.genai-mentioning `# type: ignore` outside the shim, mirroring tests/warehouse/test_snowflake_client_confinement.py. The existing Anthropic shim's regex-level confinement test narrowed from "any ignore" to "anthropic-mentioning ignore" so the new Gemini shim's own SDK ignores don't trip it. All four canonical validation steps pass; 2474 tests; coverage 97.33%. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-txe.3: #137 US-003 — pyproject.toml [gemini] extra + dev-group sync google-genai>=0.5,<1 listed in three pyproject slots in lockstep per DEC-010 (operator install, pip dev back-compat, uv dev group). uv.lock regenerated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-txe.2: #137 US-002 — GeminiProvider(LLMProvider) + registration GeminiProvider concrete strategy registered at module import; both capability flags False per DEC-003. build_create_kwargs sets response_mime_type= application/json (DEC-018) and maps system → system_instruction + single user turn (DEC-004). Safety-blocked / no-content responses → LLMResponseFormatError per DEC-005. Exception taxonomy per DEC-006 — verified against google-genai==0.8.0. Notes from verification against the installed SDK: - google.genai.errors.APIError (the shared parent of ClientError + ServerError) stores the HTTP code on .code (an int), not .status_code — the classifier reads .code. ClientError covers 4xx (incl. 401/403/429); ServerError covers 5xx. ServerError is checked BEFORE ClientError so the narrower 5xx bucket cannot be shadowed by a future shared parent class. - The SDK's models.generate_content(config=) accepts both GenerateContentConfig and the plain-dict GenerateContentConfigDict form. build_create_kwargs returns the plain-dict form so providers.py never imports google.genai.types at any scope — keeping test_gemini_client_confinement.py green and base-install module-import clean. - The .messages.create façade required by the orchestrator is wired in GeminiProvider.make_client via a small _GeminiClientAdapter / _GeminiMessagesAdapter pair (the shim returns the bare client; US-002 owns the adapter per the shim's docstring). The adapter forwards **kwargs straight to client.models.generate_content / count_tokens. - Connection-flavoured exceptions (httpx.ConnectError / httpx.TimeoutException) leak through the SDK on hard network failures; the classifier handles them with a lazy httpx import and an ImportError-safe fallback to NO_RETRY. Updates the pinned signalforge.llm __all__ surface in tests/llm/test_public_api.py and tests/draft/test_schema.py to include the new GeminiProvider re-export; adds DraftConfig/GradeConfig provider="gemini" acceptance tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-txe.9: #137 US-009 — operator-facing docs + CHANGELOG for Gemini docs/grade-ops.md + docs/draft-ops.md register gemini as a provider, name the [gemini] install extra + GOOGLE_API_KEY env var, and document the v0.3 no-caching cost note (DEC-013). README provider list deferred (no list exists to extend). CHANGELOG entry under [Unreleased]. .claude/rules/* deferred to Patterns & Memory (orchestrator-only). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-txe.4: #137 US-004 — FakeGeminiClient + offline provider integration tests Hand-rolled FakeGeminiClient mirrors FakeAnthropicClient's expect_* API (DEC-011). New tests/llm/test_fake_gemini.py proves the fake's contract; tests/llm/test_gemini_provider_via_fake.py drives GeminiProvider through the fake end-to-end (call_llm round-trip, safety-blocked branch, retry exhaustion). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-txe.5: #137 US-005 — provider-neutrality e2e tests (draft + grade) tests/grade/test_gemini_neutrality.py drives grade_artifacts end-to-end through FakeGeminiClient: cache_*=0 in JSONL, blake2b-8 hashes, sidecar round-trip, no dual-zero WARNING (DEC-003), safety-blocked → GradeLLMError degrade (DEC-005). tests/draft/test_gemini_neutrality.py drives draft_schema end-to-end likewise. DEC-014 two-stage scope satisfied at the offline-test layer. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-txe.8: #137 US-008 — gemini marker + live tests (raw + draft + grade) + CONTRIBUTING @pytest.mark.gemini marker registered; addopts excludes it from default CI per DEC-012. Three live smokes gated SF_RUN_GEMINI=1 + GOOGLE_API_KEY: test_gemini_live.py (raw call_llm), test_gemini_draft_live.py (draft_schema), test_gemini_grade_live.py (grade_artifacts 1-criterion × 1-artifact). CONTRIBUTING.md adds the maintainer 'uv run pytest -m gemini --no-cov' entry alongside the existing snowflake/anthropic equivalents. --estimate live test deferred to US-007 (gated on #136). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #137: harden DEC-019 cross-epic gate via bd sentinel bead DEC-019 was documentary only; bd_1-scaffolding-txe.7 (US-007) showed as bd-ready despite the plan saying "wait for #136 to merge to dev." A Ralph worker running `bd ready` would have picked it up and rebase-fought #136 on providers.py / pricing.py / cli/_estimate.py exactly as DEC-019 warns against. Created sentinel bead bd_1-scaffolding-41a ("#136 OpenAI grading PR #152 merged to dev") and wired bd_1-scaffolding-txe.7 to depend on it. `bd ready` no longer surfaces US-007 until the sentinel closes. Close the sentinel the moment #136 merges to dev → US-007 unblocks automatically. Updated DEC-019 + the beads manifest entry on US-007 to point at the sentinel. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-txe.7: #137 US-007 — GeminiProvider.estimate_input_tokens + --estimate integration Replaces the merge-resolution NotImplementedError stub with a real implementation via Gemini's native client.models.count_tokens (first-party; no tiktoken equivalent). The --estimate cost-preview path now works end-to-end for grade.provider: gemini and llm.provider: gemini. Anthropic byte-identity golden unchanged (verified against tests/fixtures/estimate/anthropic_byte_identity_golden.txt). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-txe.10: #137 Quality Gate — fix bugs from 4-reviewer pass Four parallel code-review passes surfaced 2 substantive code findings and 5 documentation findings; all fixed below. CodeRabbit run skipped (no skill available locally); maintainer can request post-merge. Code fixes (reviewer 2 — tests): - tests/test_audit_completeness.py: added the missing planted-violation regression test for the new parent_module codepath in _AttributeCallFinder. Covers Patterns 5-8 (namespace-package via from-google-import-genai, namespace alias, dotted-from-import, dotted-import-as) plus a negative check pinning that Pattern 7 requires parent_module to be set. Mandated by testing-signal.md § "AST single-construction-seam scans must catch all three bypass patterns" — without this, a refactor of the parent_module branches could silently break Scan 10 at the exact moment a real Gemini-SDK construction was added outside _gemini_client.py. - tests/grade/test_gemini_grade_live.py: added cache_*=0 assertion on every GradeEvent record (DEC-003 of #137). The docstring claimed this was tested but the assertion was missing — a grade-path bookkeeping regression would have slipped through the live smoke. Doc fixes (reviewer 3 — docs): - docs/grade-ops.md + docs/draft-ops.md: rewrote the "--estimate integration (deferred)" sections to reflect that US-007 SHIPPED in this PR. Both now describe the active behaviour (native client.models.count_tokens round-trip; failures surface as <unavailable>) instead of the pre-US-007 deferred state. - CHANGELOG.md: updated the Gemini Unreleased bullet to advertise --estimate support as part of the #137 deliverable; reserved the "follow-up" framing for explicit Gemini context caching only. - docs/cost-estimate-ops.md: added Gemini coverage in three places — the provider-aware token counting bullet list, a parallel "Gemini provider — [gemini] install extra" section (with the three registered SKUs in a table), and the maintainer live smoke set. File had ZERO Gemini coverage before this fix. - README.md: added Google Gemini to the supported-providers section alongside Anthropic + OpenAI; moved the Gemini roadmap row from "Planned v0.4" into "Shipped v0.4" since it's landing in this PR. Reviewer 1 (providers.py) and reviewer 4 (packaging + integration) reported 0 bugs. Reviewer 2's LOW-priority findings (flake-prone aggregate_complete assertion, loose substring on confinement scan, missing 5xx-retry integration test) deferred — not correctness risks. Validation after fixes: - ruff check: passed - ruff format --check: passed - pyright: 0 errors, 0 warnings, 0 informations - pytest: 2555 passed, 65 deselected, 97.51% coverage - wheel_smoke: 2/2 passed (new [gemini] extra packaging intact) - Anthropic byte-identity golden: passed (unchanged) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-txe.11: #137 Patterns & Memory — durable rules for Gemini Orchestrator-only commit (Ralph workers can't write under .claude/ in worktrees per memory ralph-worker-claude-dir-perms.md). .claude/rules/llm-drafter.md: - AST scan tally bumped 5 → 6 (Scan 10 = genai.Client confinement via _AttributeCallFinder(parent_module="google")). - New § "Gemini provider shape (#137 — the third concrete provider, no-cache via a namespace-package SDK)" with the four load-bearing patterns: .messages-over-.models.generate_content adapter, response_mime_type=application/json server-side JSON enforcement, safety-filter typed LLMResponseFormatError → grade degrade, and native models.count_tokens for --estimate (distinct from OpenAI's local tiktoken path). - New § "Namespace-package SDKs (#137 generalisation)" documenting the parent_module parameter that catches the four namespace-package import shapes the no-parent path misses. - Reference block extended with the #137 plan + the three new fakes + the two new neutrality test suites. - "new vendor lands" wording generalised (#137 is no longer "next", it's "shipped"); Anthropic→OpenAI→Gemini lineage noted explicitly. .claude/rules/grade-layer.md: - Conservative degrade taxonomy DEC-002/DEC-015 entry for GradeLLMError extended with a sentence explaining Gemini's safety-filter / no-content response routes through the same path via the typed LLMResponseFormatError that GeminiProvider raises (DEC-005 of #137). Locks in the provider-neutral contract: a future vendor with a content-filter surface MUST route through LLMResponseFormatError, not a provider-specific switch in grade_artifacts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #137: Address PR review feedback (Copilot + CodeRabbit) Five real issues fixed; three outdated threads resolved as-is. Fixed: 1. src/signalforge/llm/_gemini_client.py — Copilot flagged that GeminiClientProtocol's docstring claimed the bare SDK client satisfies it, and _make_gemini_client was annotated to return GeminiClientProtocol despite returning the raw google.genai.Client (which has no .messages namespace). The protocol is satisfied only by the wrapped _GeminiClientAdapter (in providers.py) and by the test fake. Rewrote both docstrings; relaxed _make_gemini_client's return type to `Any` and explained the wrapper-on-top contract explicitly. Replaces the misleading "satisfies structurally" claim with the honest "bare SDK does NOT satisfy; wrapper does." 2. plans/super/137-gemini-grading.md (Discovery, line 98) — CodeRabbit flagged the stale PRICE_TABLE_VERSION = "2026-05-11". Annotated the reference as "at discovery time" and noted that US-006 and #136 both bump it. 3. plans/super/137-gemini-grading.md (Worker-writability routing) — CodeRabbit flagged a duplicated "## Worker-writability routing" heading. The duplicate must have crept in during the cross-review pass. Removed the second copy; kept the first canonical version. 4. tests/llm/test_pricing.py (test_lookup_raises_estimateunknownmodelerror_for_unknown_gemini_model) — CodeRabbit flagged that the test only validates `.model`, leaving the operator-facing remediation text drifting silently from the pricing table. Added a structural pin that the rendered exception names every Gemini SKU. Future SKU additions force a lockstep remediation update. Outdated threads (3) — content already fixed by earlier commits; the threads are kept resolved for hygiene: 5. tests/test_audit_completeness.py docstring "node.module == genai" note (Copilot, two threads). The actual scan handles the dotted- form via parent_module; the comments in the implementation already reflect this since the QG pass. 6. CONTRIBUTING.md pre-release excluded-marker list (CodeRabbit). The list was already extended to include `gemini` (and `snowflake`) during the QG fix in commit f15a469. Validation after fixes: - ruff check: passed - ruff format --check: 279 files formatted - pyright: 0 errors, 0 warnings, 0 informations - pytest: 2555 passed, 65 deselected, 97.51% coverage Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #137: Address second-round PR review (Copilot, 4 new comments) Two real findings fixed; two false positives documented. Fixed: 1. pyproject.toml gemini marker description — Copilot flagged that the help text named only GOOGLE_API_KEY but the live tests also gate on SF_RUN_GEMINI=1. Updated to "requires SF_RUN_GEMINI=1 + GOOGLE_API_KEY" — mirrors the OpenAI marker's shape exactly. 2. src/signalforge/llm/providers.py::GeminiProvider.extract_usage — Copilot flagged that getattr(usage, "prompt_token_count", 0) or 0 silently swallows missing/malformed SDK response shapes and feeds misleading 0/0 figures into the audit JSONL + --estimate math. Switched to the shared _extract_usage_field helper which raises LLMResponseFormatError on missing/non-int fields. Matches the Anthropic precedent (the OpenAI provider already uses the same helper). Added test_geminiprovider_extract_usage_missing_inner_ field_raises pinning both the missing-field and non-int-type paths to keep them durable. False positives (replied in resolve-threads): 3. _gemini_client.py:116 (`_make_gemini_client` return annotation): Copilot is reviewing the pre-commit-00dc673 diff. The previous round of fixes (commit 00dc673) already changed the return type to Any and rewrote the docstring to be honest that the bare SDK client does NOT satisfy GeminiClientProtocol; only the _GeminiClientAdapter wrapper does. Current file matches. 4. plans/super/137-gemini-grading.md:777 (duplicate Worker- writability routing): Same — the duplicate was already removed in 00dc673. Only one section now exists at line 757. Validation: - ruff check: passed - ruff format --check: passed - pyright: 0 errors, 0 warnings, 0 informations - pytest: 2556 passed, 65 deselected, 97.55% coverage Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
wjduenow
added a commit
that referenced
this pull request
May 30, 2026
* chore: begin 0.4.0.dev0
* #135: provider-neutral LLM seam (plan) (#148)
* Add super plan for #135: provider-neutral LLM seam
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Mark #135 plan published (PR #148)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Mark #135 plan devolved (beads epic bd_1-scaffolding-j2c)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-j2c.1: US-001 provider foundation (ExceptionCategory, UsageMetrics, LLMProvider ABC, registry)
Adds the provider-neutral LLM seam foundation (DEC-001/002/003 of #135):
- src/signalforge/llm/providers.py: ExceptionCategory (5-member enum),
UsageMetrics (frozen pydantic value object, cache fields default 0),
LLMProvider ABC, and the process-level registry (register_provider /
provider_for). No vendor behaviour wired; registry starts empty.
- UnknownProviderError(LLMError) in errors.py — lists available registered
provider names, repr-safe message, default_remediation; registered at
CLI exit-code tier 2 (looked-up-identifier-not-in-table input failure).
- Exported the new public names from signalforge.llm.__all__; updated the
documented-surface lists in test_public_api.py / test_schema.py and the
errors count in test_errors.py.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-j2c.2: US-002 AnthropicProvider strategy + _anthropic_client rename + AST scan
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-j2c.3: US-003 generic call_llm orchestrator (rename, capability gating, strategy dispatch)
Collapse call_anthropic into a provider-neutral call_llm orchestrator:
- Resolve strategy via provider_for(provider); build client via
strategy.make_client() when client is None (DEC-006).
- Pre-send count gate now gated on strategy.supports_token_count; cache
marker / beta header + dual-zero anomaly WARNING gated on
supports_prompt_caching (DEC-008). Anthropic = both True ⇒ byte-identical.
- Retry loop dispatches on strategy.classify_exception -> ExceptionCategory
(DEC-001) instead of catching Anthropic SDK classes directly; per-class
budgets, backoff math, WARNING shape, exhaustion raises preserved.
- Response assembled via strategy.extract_text_blocks/extract_usage.
- Neutral _LLMClientProtocol narrows the resolved client so no vendor-SDK
type / type-checker suppression leaks into llm.client (DEC-012 confinement).
Drop call_anthropic (name + __all__); add call_llm. Migrate draft_from_request
and grade._grade_one call sites + all client/retry/public-api/schema tests.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-j2c.4: US-004 provider config field + stage threading + CLI client-construction migration
DEC-006/007 of #135. Adds a registry-validated `provider: str = "anthropic"`
field to DraftConfig and GradeConfig, each with a field_validator that
delegates to providers.provider_for (raises UnknownProviderError listing
available names on an unknown value — fails loud at config load). Threads
provider=config.provider into call_llm from draft_from_request and
grade._grade_one.
Migrates the CLI generate path off the in-CLI _make_anthropic_client helper
(removed): the real-run draft/grade stages now pass client=None so call_llm
lazy-builds the client via the configured provider. The --estimate
short-circuit (Anthropic-specific count_tokens) builds its concrete client via
provider_for(draft_config.provider).make_client(). Migrates the 7 CLI
monkeypatch tests: 5 batch/select tests drop the orphaned make_anthropic_client
mock; the estimate test patches AnthropicProvider.make_client; test_lint
unchanged (it patches the LLM shim, which still exists).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-j2c.5: US-005 no-cache fake provider neutrality proof (AC #2/#3)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-j2c.6: US-006 docs + surface parity (docs/ portion)
Update operator-facing docs for the provider-neutral LLM seam (#135):
- call_anthropic -> call_llm; llm._client -> llm._anthropic_client
- document the provider config knob (llm.provider / grade.provider),
default "anthropic", registry-validated, forward-looking plugin seam
- note prompt caching + count_tokens are now provider capabilities
(supports_prompt_caching / supports_token_count); Anthropic unchanged
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-j2c.6: US-006 .claude/rules seam-rename + provider-seam accuracy
Orchestrator-only rule-file edits (workers can't write .claude/ in worktrees).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-j2c.7: Quality gate — fix bugs from code review
- providers.build_count_tokens_kwargs: correct docstring (probe omits the
cache_control marker because it doesn't affect the token count; the old
claim of "matching the inline call_anthropic probe" was inaccurate).
- call_llm: gate cache_marker_active on BOTH supports_prompt_caching AND
supports_token_count, so a future caching=True/token_count=False provider
degrades to no-caching rather than sending an unvalidated cache_control
marker (no sub-minimum drop / no oversize cap). Anthropic is True/True so
the default path is a no-op. (4-pass review latent-trap finding.)
CodeRabbit: skill not available in this environment — skipped.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-j2c.8: Patterns & Memory — provider-seam convention + QG lesson
- llm-drafter.md: capability-gating lesson (gate the cache marker on BOTH
supports_prompt_caching AND supports_token_count — the #135 QG latent-trap
finding) for future #136/#137 provider authors.
- Added `provider` to the documented `llm:` config block.
- Reference section now cites plans/super/135 + the no-cache neutrality proof.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* #135: close Codecov patch gaps — count_tokens probe error mapping + classify fallthrough
The provider-neutral refactor rewrote the count_tokens pre-send probe's
error-mapping as new lines (auth/rate-limit/conn/5xx/other -> typed LLMError,
no retry on the probe) that the existing retry tests (messages.create path)
never exercised. Add 6 probe-failure tests + a missing-input_tokens case
(client.py 320-348 now covered, 87% -> 96%).
Also cover AnthropicProvider.classify_exception's defensive fallthrough for a
non-5xx/non-4xx-non-auth APIStatusError (3xx) -> NO_RETRY (providers.py -> 100%).
Remaining uncovered client.py helper branches (_extract_text_blocks /
_extract_usage_field malformed-response raises) are pre-existing and not part
of the #135 diff.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* #135: address PR review (CodeRabbit/Copilot)
- Stale-symbol docstrings: rename every `call_anthropic` :func: reference to
`call_llm` across llm/{__init__,models,_anthropic_client,providers}.py,
draft/schema.py, grade/{engine,config,errors}.py, cli/_estimate.py; reword
the AnthropicProvider "preparatory refactor" note (US-003 already collapsed
the duplication); rewrite the llm package docstring to the provider-neutral
seam.
- client.py: count_tokens NO_RETRY fallback message no longer claims a "status"
(it also covers non-status exceptions) — now "failed with a non-retryable error".
- generate.py --estimate: fail fast with CliInputError when draft.provider !=
grade.provider or the provider isn't 'anthropic' (the estimate engine drives a
single Anthropic-cast count_tokens client across both configs). + 2 regression
tests.
- plan SD-1: drop the stale `Literal` wording (DEC-007 settled it as a
registry-validated str).
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(readme): refresh status, generalize config, redraft roadmap (#153)
* docs(readme): refresh status, generalize config, redraft roadmap
The v0.1 "Status:" line was four releases stale and BigQuery-only.
Replace with a Supported-warehouses block covering BigQuery (v0.1) and
Snowflake (v0.3 — incl. the deferred aggregate-only / column_stats
limitation), generalize Configuration to dispatch on dbt profile type,
and redraft the Roadmap into Shipped (v0.1/v0.2/v0.3 with release dates)
+ Planned (Gemini/OpenAI → installable skill → Airflow → GitHub Action
→ rubric customization → dbt Fusion). Drop the now-stale "prune-existing
is dev-branch only" and "first four ship; prune-existing is v0.2 dev"
callouts.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(readme): add custom-business-logic callout, strip non-roadmap version markers
- Add a **Custom business logic** paragraph alongside Supported warehouses,
explaining `meta.signalforge.business_rules` → `custom_sql` drafting + the
ownership-marker write semantics + always-pass drop behaviour.
- Remove every version marker outside the Roadmap section: "since v0.1/v0.3"
in the warehouses block, "*(v0.2)*" on the prune-existing bullet, the "v0.2
default" config comment, the "(v0.1)"/"(v0.3)" subsection headers, and the
"pre-alpha … v0.1 milestone" line in Contributing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(readme): reorganize scope callouts; promote Supported warehouses to its own H2
The two bolded standalone paragraphs at the top of the README (Supported
warehouses, Custom business logic) sat before the "Why this exists" pitch
and front-loaded dense capability detail. The custom-business-logic block
was also triple-covered (standalone block + "What it does" bullet +
Quick-start worked example).
- Drop both standalone bolded paragraphs from the top.
- Promote Supported warehouses to a proper H2 between "How it works"
and the "Live on PyPI" callout — brief, points to Configuration for
per-warehouse setup; Snowflake's known limitation stays in its single
home under Configuration > Snowflake.
- Strengthen the custom-business-rule bullet in "What it does" with the
"fifth test type" framing and a direct link to the worked example, so
the feature stays prominent without a redundant top-of-readme banner.
New flow: pitch → features → architecture → scope → quickstart → reference.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* #136: OpenAI grading provider (plan) (#152)
* #136: super plan for OpenAI grading provider
Phase: detailing (awaiting approval).
13 DECs captured: Chat Completions API, no provider/model cross-validation,
fully wire --estimate for OpenAI (new estimate_input_tokens ABC), default
judge gpt-4o, scope both grade + draft, server-side response_format=json_object,
four pricing SKUs (gpt-4o, -mini, 4.1, 4-turbo), live smoke covers grade +
estimate, .messages adapter wraps chat.completions, AST Scan 3 extension,
[openai] extra + lazy tiktoken, Anthropic estimate byte-identity floor.
9 stories: shim + AST confinement, OpenAIProvider + registration, FakeOpenAIClient
+ neutrality test, pricing entries, --estimate provider-aware counting,
live gated smokes, docs, Quality Gate, Patterns & Memory.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* #136: link plan to PR #152 (phase=published)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* #136: apply 5 plan revisions from #137 cross-review
Cross-review against #137 (Gemini grading) plan surfaced 5 substantive gaps
in the OpenAI plan:
1. DEC-010 — fix wording: a NEW 9th AST scan (Scan 3 is Anthropic-specific),
not an extension. Reuse _QualifiedNameCallFinder + three-pattern bypass
regression test per testing-signal.md.
2. DEC-012 — fix pyproject.toml listing: three slots in lockstep
([project.optional-dependencies].openai + .dev + [dependency-groups].dev),
mirroring Snowflake precedent per python-build.md. Plan previously named
only two.
3. DEC-014 (new) — codify the _load_openai_exception_classes empty-tuple
fallback on ImportError (mirrors AnthropicProvider). Includes refusal /
content-filter symmetry note explaining why OpenAI needs no Gemini-style
safety_filter typed-degrade DEC.
4. US-006 — add a third live test (draft_schema) honouring DEC-005's
"scope both stages" commitment at live level too. DEC-008 updated to
match (three tests, not two).
5. New "Open notes for implementation" section — pragmatic SDK-class-name
verification, .messages adapter shape confirmation, tiktoken fallback
table, response_format prompt-requirement check, Anthropic byte-identity
snapshot capture protocol.
Plus a new "Worker-writability routing" section codifying that US-009
(Patterns & Memory) is orchestrator-only because it edits .claude/rules/,
mirroring #137's same routing.
Plan grows from 319 → 343 lines (+40/-17).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* #136: devolve plan into bd beads (epic bd_1-scaffolding-4tw + 9 tasks)
Fills in the Beads manifest section. Epic bd_1-scaffolding-4tw with 9
child tasks (US-001 … US-009) created with dep edges per the plan's
"Depends on:" lines:
US-001 → none (READY)
US-002 → US-001
US-003 → US-002
US-004 → none (parallel-safe) (READY)
US-005 → US-002, US-004
US-006 → US-002, US-005
US-007 → US-001, US-002, US-005
US-008 → US-001..US-007 (QG)
US-009 → US-008 (P&M; orchestrator-only)
Cross-epic gate wired downstream: #137's sentinel bd_1-scaffolding-41a
now DEPENDS ON US-009, so #137 US-007 stays mechanically blocked until
this epic completes + sentinel closes after PR #152 merges to dev.
Parallel-safe at the entry points: US-001 and US-004 edit disjoint
files (shim/pyproject/AST vs pricing) — ralph-serialize-shared-registry
does not apply. Run concurrently.
bd ready (post-devolve) surfaces .1 + .4 of this epic + the two epics
themselves + the unrelated Airflow integration.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-4tw.4: #136 US-004 — OpenAI pricing SKUs (gpt-4o + 3 siblings)
Adds four OpenAI SKUs to _PRICES_MUTABLE per DEC-007: gpt-4o (default
judge per DEC-004), gpt-4o-mini (budget), gpt-4.1 (newer flagship),
gpt-4-turbo (back-compat). Cache fields = 0.0 (OpenAI has no equivalent
cache discount). PRICE_TABLE_VERSION bumped to 2026-05-28.
Anthropic SKUs unchanged (byte-identical) — Anthropic estimate
byte-identity floor (DEC-013) preserved at the pricing layer.
Tests assert: non-zero input/output rates + zero cache fields for all
four; unknown model still raises EstimateUnknownModelError; Anthropic
SKUs unchanged; version bump pinned.
OpenAI per-MTok rates are calibration figures captured at PR-prep time
pending operator verification against https://openai.com/api/pricing/;
the figures are sanity-check baselines, not billing guarantees.
Traces: DEC-003, DEC-004, DEC-007.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-4tw.1: #136 US-001 — _openai_client.py shim + [openai] extra + 9th AST scan
Implements the per-vendor shim confining every openai SDK type ignore.
Adds [openai] optional-dep extra (openai + tiktoken) in three-slot
lockstep across [project.optional-dependencies].openai, .dev, and
[dependency-groups].dev. Adds the new 9th AST scan in
tests/test_audit_completeness.py reusing _QualifiedNameCallFinder
(NOT an extension of Scan 3 — Scan 3 is Anthropic-specific) and a
companion per-file confinement test mirroring Snowflake's. Bumps the
docstring tally 8→9.
DEC-014 empty-tuple ImportError fallback on _load_openai_exception_classes
mirrors AnthropicProvider; tiktoken cl100k_base fallback for unknown
model ids.
Traces: DEC-001, DEC-009, DEC-010, DEC-012, DEC-014.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-4tw.2: #136 US-002 — OpenAIProvider + registration + config-validator coverage
Implements OpenAIProvider(LLMProvider) with both capability flags False
(no prompt caching, no pre-send token count) — mirrors FakeNoCacheProvider
end-to-end shape. build_create_kwargs attaches response_format=
{type: json_object} per DEC-006 (server-side JSON enforcement); no
cache_control / extra_headers. extract_text_blocks reads
response.choices[0].message.content; extract_usage maps usage.prompt_tokens
+ completion_tokens → UsageMetrics with cache fields 0. classify_exception
covers all five ExceptionCategory branches via _load_openai_exception_classes
from the US-001 shim.
register_provider(OpenAIProvider()) at module scope so GradeConfig/
DraftConfig validators accept provider='openai'. Exported from
signalforge.llm.__init__.
Tests pin: each ABC method's contract; config validator acceptance for
both stages; UnknownProviderError lists both anthropic and openai.
Traces: DEC-001, DEC-005, DEC-006, DEC-009, DEC-011.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-4tw.3: #136 US-003 — FakeOpenAIClient + grade neutrality e2e
Adds tests/llm/_fake_openai.py with FakeOpenAIClient + expect_messages_create
API mirroring FakeAnthropicClient verbatim. Support dataclasses match the
real chat-completions response shape (choices[0].message.content; usage
with prompt_tokens + completion_tokens).
Adds tests/grade/test_provider_neutrality_openai.py — the OpenAI analogue
of the FakeNoCacheProvider neutrality proof. Asserts cache_*=0 in JSONL
audit, blake2b-8 reproducibility hashes present, sidecar drift-detector
round-trip, no dual-zero cache-anomaly WARNING in caplog, all expectations
consumed.
Traces: DEC-001, DEC-005, DEC-006, DEC-009, DEC-011.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-4tw.5: #136 US-005 — --estimate provider-aware token counting
Adds LLMProvider.estimate_input_tokens(model, text, *, client=None) -> int
as an abstract method on the ABC (DEC-003). AnthropicProvider impl
delegates to the SDK's messages.count_tokens with a single user-message
text envelope; OpenAIProvider impl delegates to _count_openai_tokens
(tiktoken with cl100k_base fallback per DEC-012); FakeNoCacheProvider
and _DummyProvider impls return trivial deterministic answers so the
existing neutrality + registry tests still pass.
Refactors cli/_estimate.py: _count_draft_tokens and the grader-side
equivalent dispatch through provider_for(config.provider).estimate_input_tokens
— no more hard-coded anthropic_client.messages.count_tokens calls. The
engine signature relaxes anthropic_client to 'object | None' so the
OpenAI path can pass None (tiktoken is local). Lifts the
'--estimate currently supports only provider=anthropic' gate in
cli/generate.py; the divergent-providers check stays (the engine still
takes one optional client).
Pins DEC-013 Anthropic byte-identity floor via
tests/cli/test_estimate.py::test_estimate_anthropic_byte_identity_golden
and tests/fixtures/estimate/anthropic_byte_identity_golden.txt (captured
2026-05-28 before the refactor; reproduced verbatim after). New
companion tests prove the OpenAI path produces non-zero token counts
+ non-zero USD, ignores the threaded anthropic_client, and goes through
_count_openai_tokens (patched-helper assertion).
Traces: DEC-003, DEC-007, DEC-012, DEC-013.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-4tw.7: #136 US-007 — operator docs + CHANGELOG (worker-writable portion)
Updates operator-facing documentation surfaces for OpenAI grading +
drafting:
- docs/grade-ops.md — OpenAI provider section (config snippet,
OPENAI_API_KEY, no-cache caveat, live smoke gating link)
- docs/draft-ops.md — equivalent for llm.provider: openai
- docs/cost-estimate-ops.md — tiktoken note, [openai] extra install,
four pricing SKUs
- CHANGELOG.md — [Unreleased] Added entry for #136
- README.md — provider enumeration extended (if applicable)
The .claude/rules/llm-drafter.md update is deferred to US-009
(Patterns & Memory) per the project's worker-writability rule
(orchestrator-only edits under .claude/).
Traces: DEC-001 through DEC-014.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-4tw.6: #136 US-006 — live gated smoke tests (openai marker + 3 tests)
Adds @pytest.mark.openai marker + three gated live-API tests:
- tests/grade/test_smoke_real_api_openai.py — grade_artifacts(provider=openai)
- tests/draft/test_smoke_real_api_openai.py — draft_schema(provider=openai)
(honors DEC-005 both-stages at live level)
- tests/cli/test_e2e_estimate_openai.py — generate --estimate openai
All three env-gated on SF_RUN_OPENAI=1 + OPENAI_API_KEY via belt-and-
suspenders _skip_reason() helper per testing-signal.md. Marker registered
in pyproject.toml + added to addopts exclusion so default pytest doesn't
collect them. CONTRIBUTING.md documents the maintainer-only run pattern.
Traces: DEC-001, DEC-004, DEC-005, DEC-008.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-4tw.8: #136 Quality Gate — fix 2 majors + 1 minor from code review x4
Code-review pass 3 surfaced two correctness MAJORS + pass 4 surfaced
a doc MINOR; all fixed inline per /ralph-run Step 3b. Pass 1, 2, 4
(other surfaces) reported no correctness bugs.
MAJOR 1 — EstimateUnknownModelError.default_remediation enumerated
only the three Anthropic SKUs. An operator typo on an OpenAI SKU
got pointed at Anthropic options (violates "errors carry remediation"
in manifest-readers.md). Updated to enumerate all 7 SKUs
(claude-sonnet-4-6, claude-opus-4-7, claude-haiku-4-5, gpt-4o,
gpt-4o-mini, gpt-4.1, gpt-4-turbo) + refreshed the locked-text test
that pinned the stale string.
MAJOR 2 — AnthropicProvider.estimate_input_tokens dropped the
pre-refactor `system=` kwarg, concatenating system+cached+dynamic
into a single user-content string. Anthropic's server-side tokenizer
counts the system block with its own envelope tokens; without the
kwarg, real-API counts under-report by the system-envelope size.
The fake-driven byte-identity test passed only because the fake
returned canned input_tokens regardless of kwargs (so rendered-output
identity held, but DEC-013's "byte-identity vs the pre-refactor
Anthropic count_tokens call" spirit broke at the real-API level).
Fix extends the ABC signature with a keyword-only `system: str = ""`
parameter. AnthropicProvider passes it via `system=` kwarg when
non-empty (matches pre-refactor real-API call shape). OpenAIProvider
concatenates `system + text` before tiktoken (tiktoken has no
system-envelope distinction; the total still counts every token).
FakeNoCacheProvider concatenates for the word-count proxy.
_DummyProvider gains the new kwarg for ABC parity. cli/_estimate.py
callers (_count_draft_tokens, _count_grade_criterion_tokens) thread
`system=` separately; grade-side preserves the pre-existing
double-count of the rubric (passed as both system= AND in user content)
to keep byte-identity with the pre-refactor shape.
MINOR — docs/grade-ops.md:118 config-snippet comment said "only
anthropic registered today", contradicting the same file's later
"## OpenAI provider" section and docs/draft-ops.md's correct
wording. Updated to mirror draft-ops's form.
Validation: ruff/pyright/pytest all green (2498 passed, 62 deselected,
97.34% coverage); wheel_smoke green (2 passed).
Note: CodeRabbit skill not available in this environment, skipped
per Step 3b "(if available)".
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-4tw.9: #136 Patterns & Memory — codify OpenAI seam conventions
Orchestrator-only commit (workers can't write under .claude/ in
worktrees per ralph-worker-claude-dir-perms.md).
Updates .claude/rules/llm-drafter.md to capture three durable patterns
from #136:
1. Provider-neutral seam — adds OpenAI as the second registered
provider (name="openai", both capability flags False) and extends
the LLMProvider ABC surface enumeration with the new
estimate_input_tokens(model, text, *, system="", client=None)
method (#136 US-005).
2. "OpenAI provider shape" subsection — codifies the .messages.create
façade adapter pattern (SimpleNamespace delegating to
chat.completions.create), the response_format={type:json_object}
server-side JSON enforcement (with the cross-ref to issue #144's
tolerant parser as fallback), and the tiktoken cl100k_base
fallback for unknown model ids. This becomes the canonical
precedent for #137 Gemini's response_mime_type=application/json
equivalent.
3. estimate_input_tokens(*, system=...) — documents the load-bearing
reason the system envelope is threaded separately (Anthropic's
server-side tokenizer applies system-block envelope tokens; dropping
the kwarg under-reports real-API counts silently because
fake-driven byte-identity tests can't catch call-shape drift). Also
pins the deliberate double-count of system_and_rubric in
_count_grade_criterion_tokens as pre-existing behaviour that must
be preserved.
Bumps AST-scan tally section from "four" → "five" (adds the openai.OpenAI
9th-project-scan); flags #137 Gemini as the 10th. Updates the
References block with #136 plan + new files.
Memory entry: ~/.claude/projects/.../memory/fake-driven-byte-identity-
blind-spot.md captures the QG lesson (fakes return canned values
regardless of kwargs → call-shape drift slips past rendered-output
snapshots → needs explicit kwargs-shape assertion OR live test).
Indexed in MEMORY.md.
Validation: all four canonical gates green (2498 passed, 97.34%
coverage).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* #136: address PR #152 review (6 actionable + meta)
Copilot:
- MAJOR _estimate.py:359 — OpenAI grade-side over-counted rubric. The
first QG fix preserved a pre-existing Anthropic double-count of
system_and_rubric (passed as both system= AND in user content),
which triple-counted on OpenAI (system→system+text concat → rubric
prefix in text). Corrected to match the runtime grader call: rubric
in system= once, artifact envelope in user content. Anthropic real-
API counts drop one rubric copy from the buggy pre-refactor bytes;
OpenAI counts each input once. Fake-driven byte-identity golden
still passes (canned token counts are call-shape-agnostic).
CHANGELOG documents the calibration shift.
- NIT _estimate.py:421 — renamed anthropic_client → client. Type was
already object | None and forwarded to whichever provider strategy
is active; old name implied Anthropic-only and would mislead a
future #137 Gemini wiring. CLI in generate.py already passed None
for non-Anthropic; rename surfaces the contract without behaviour
change.
- META plan/PR description — addressed by updating the PR body
separately (not part of this commit).
CodeRabbit:
- CONTRIBUTING.md:64 — added snowflake to the gated-marker audit
command + the corresponding SNOWFLAKE_* env vars.
- tests/llm/_fake_provider.py:234 — boundary-word undercount fix in
the FakeNoCacheProvider word-count proxy (f"{system} {text}" with
a delimiter; was system+text which merged the last word of system
with the first word of text under .split()).
- tests/llm/test_openai_client_confinement.py:47 — glob("*.py") →
rglob("*.py") so the confinement scan catches openai-mentioning
ignore directives in any future nested signalforge/llm/ subpackage,
not just top-level files. Path display becomes relative-to-_LLM_DIR.
- tests/test_audit_completeness.py:1080 — closed the "call-before-
import alias" bypass on _AttributeCallFinder by adding a two-pass
visit_Module that pre-collects every alias module-wide before
visiting any Call node. Mirrors the same fix already shipped on
_QualifiedNameCallFinder (PR #69 / DEC-013). Pattern-4 regression
test added.
Validation: ruff/format/pyright/pytest all green (2498 passed,
97.34% coverage); wheel_smoke green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* #136: close PR #152 codecov gaps (9 lines → 0)
Codecov flagged 91.82% patch coverage with 9 lines missing across
3 files. Closed all 9 with 6 focused unit tests:
src/signalforge/llm/_openai_client.py (was 85%, now 100%):
- _OpenAIClientAdapter.__init__ + ._messages_create — no production
caller exercises the adapter (orchestrator drives via
FakeOpenAIClient which has its own .messages.create); added
test_openai_client_adapter_messages_create_delegates_to_chat_completions
that builds a SimpleNamespace raw client and pins the delegation
+ kwargs forwarding.
- _count_openai_tokens cl100k_base fallback (DEC-012 unknown-model
branch) — added test_count_openai_tokens_falls_back_to_cl100k_base_
for_unknown_model.
src/signalforge/llm/providers.py (was 98%, now 100%):
- AnthropicProvider.estimate_input_tokens no-system-kwarg arm (the
else branch added in QG) — added test_anthropic_provider_estimate_
input_tokens_skips_system_kwarg_when_empty, which ALSO pins the
load-bearing invariant that an empty system MUST be omitted from
the SDK kwargs (otherwise the count carries a spurious system=""
block).
- LLMResponseFormatError raise on missing input_tokens — added
test_anthropic_provider_estimate_input_tokens_raises_on_missing_
input_tokens.
- OpenAIProvider.extract_text_blocks missing-message arm — added
test_openai_provider_extract_text_blocks_missing_message_attr_
raises (distinct from the existing content=None and empty-choices
cases — those exercise different arms).
src/signalforge/cli/generate.py:846 (was 66%, now covered):
- The non-Anthropic `client = None` branch in cmd_generate's
--estimate short-circuit — exercised only by the
@pytest.mark.openai live smoke before. Added test_generate_
estimate_openai_provider_passes_client_none_to_engine that drives
cmd_generate with llm.provider: openai + grade.provider: openai
in a tmp signalforge.yml, spies on AnthropicProvider.make_client
to confirm it's NEVER called on the openai path, and captures the
client kwarg into estimate(...) to pin client=None. The remaining
7 uncovered lines in generate.py are pre-PR and out of scope.
Total: 2504 passed (was 2498), 97.47% coverage (was 97.34%), no
new gated tests added (all unit tests, default-CI-included).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* #137: Gemini grading provider (plan) (#151)
* #137: Gemini grading provider (plan)
Super plan for #137 (Gemini grading provider) — depends on #135 (merged).
9 stories: shim + provider + extra + fake/unit + neutrality + live +
docs + QG + P&M. Caching deferred (both capability flags False);
safety-filter no-content surfaces as LLMResponseFormatError →
grade GradeLLMError degrade. Both drafter and grader covered.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* #137: extend plan after comparison with #136 OpenAI plan
Closes the three gaps surfaced by the #136 comparison:
- Add --estimate integration (DEC-016): GeminiProvider.estimate_input_tokens
via native client.models.count_tokens (cleaner than #136's tiktoken path —
Gemini has a first-party count endpoint).
- Add 3 Gemini pricing SKUs to pricing.py (DEC-017): gemini-2.5-pro,
gemini-2.5-flash, gemini-2.0-flash; bump PRICE_TABLE_VERSION.
- Add server-side JSON enforcement (DEC-018): response_mime_type=
application/json — mirrors #136 DEC-006.
- Add CHANGELOG entry to docs story (US-009).
- Add wheel_smoke to QG (new [gemini] extra changes packaging).
- Sequence after #136 (DEC-019): inherit ABC extension + estimate refactor.
New stories: US-006 (pricing, parallel-safe), US-007 (estimate impl,
depends on #136). Old US-006/007 renumbered to US-008/009.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* #137: apply 3 plan refinements from #136 cross-review
Closes the smaller gaps surfaced by re-reviewing #137 against #136 after
the #136 plan was tightened (520b36c):
1. DEC-009 — merge-order-conditional AST scan tally. Currently said "8 → 9"
unconditionally, but DEC-019 specifies #137 ships AFTER #136. Realistic
bump is 9 → 10 (since #136's openai.OpenAI scan bumps 8 → 9 first);
added the fallback wording for the slipped-sequencing case.
2. Dedicated "Worker-writability routing" top-level section codifying that
Patterns & Memory is orchestrator-only (.claude/rules/ writes). Was a
one-line architecture-review row; now a durable section that mirrors
#136's codification — future #138+ provider plans copy the pattern.
3. Open notes for implementation — two additions:
- response_mime_type requires NO prompt keyword (contrast with OpenAI's
response_format which fails without "json" in prompt). Prevents a
future maintainer adding defensive prompt text.
- pricing.lookup zero cache fields math validation (symmetric to #136
verification item).
Major gaps from the prior cross-review (--estimate, pricing, dual-listing,
JSON enforcement, CHANGELOG, three live tests, sequencing) were already
addressed in b28c0b5.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* #137: devolve plan to beads (phase=devolved)
Epic: bd_1-scaffolding-txe
Tasks: 11 (US-001 through US-009 + Quality Gate + Patterns & Memory)
Dependencies: 19 edges per the plan's dependency graph
Ready queue: US-001 (shim) + US-006 (pricing, parallel-safe)
US-007 (--estimate integration) is blocked on US-002 + US-006 AND
carries a cross-epic gate on #136 landing first per DEC-019 — that
sequencing is documented in the task's description rather than wired
as a bead dep since #136's beads live in a different epic.
US-011 (Patterns & Memory) is flagged orchestrator-only because it
edits .claude/rules/ which Ralph workers can't write under in
worktrees (per the Worker-writability routing section of the plan).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* #137: adjust plan now that #136 is being implemented first
Folds devolve-time annotations (phase=devolved, beads epic
bd_1-scaffolding-txe, per-task status + cross-epic blocker on US-007)
together with five #136-driven adjustments:
1. AST-scan tally locked to 10th (not 9th) — #136 owns the 8→9 bump
for openai.OpenAI; #137 takes 9→10 for genai.Client. Updated in
DEC-009, US-001, P&M, and Open notes.
2. DEC-019 sharpened: #136 plan → #136 implementation; explicit rebase
guidance (rebase on dev after #136 merges) rather than contingency
wording.
3. DEC-005 gains a refusal/content-filter symmetry note cross-ref'ing
#136 DEC-014 — explains why OpenAI deliberately ships no
safety-filter typed-degrade and why Gemini deliberately does.
4. New "Worker-writability routing" section mirroring #136's, codifying
that P&M is orchestrator-only because it edits .claude/rules/.
5. Open notes extended with two pragmatic items parallel to #136's:
- google-genai count_tokens response field-name verification
(.total_tokens vs .total_token_count) at US-007 implementation.
- Anthropic byte-identity snapshot ownership: #136 owns it; #137
inherits it; the wiring is wrong if it moves the snapshot.
Plan grows 762 → 825 lines (+70/-30).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-txe.6: #137 US-006 — add Gemini pricing SKUs (gemini-2.5-pro, gemini-2.5-flash, gemini-2.0-flash)
Adds three Gemini SKUs to _PRICES_MUTABLE per DEC-017, each with positive
input_per_mtok / output_per_mtok per Google's public Gemini API price page
and cache fields = 0.0 (v0.3 ships Gemini without an Anthropic-equivalent
prompt-cache discount). Bumps PRICE_TABLE_VERSION to "2026-05-27". Tests
parametrise the three SKUs, pin Anthropic SKU byte-identity (additive-only),
and assert lookup("gemini-unknown") still raises EstimateUnknownModelError.
cli/_estimate.py reads only input_per_mtok and output_per_mtok in the USD
math (lines 468, 511), so zero cache fields are safe — no divide-by-zero
or NaN risk. The EstimateUnknownModelError.default_remediation in
llm/errors.py still lists only the three Claude SKUs by name; per the
story scope ("pricing-only; DO NOT touch any other file") this is left
for a follow-up to broaden once the wider Gemini surface (provider class
+ --estimate integration in US-007) lands.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-txe.1: #137 US-001 — _gemini_client.py shim + new AST confinement scan
Land the per-vendor Gemini SDK shim and the 9th AST audit-completeness
scan that confines genai.Client(...) constructions to it. Mirrors the
Anthropic shim shape verbatim — GeminiClientProtocol (with .messages
façade for US-002 and .models for US-007), lazy-import factory, frozen
_GeminiExceptionClasses dataclass with empty-tuple fallback per DEC-015
so the module imports cleanly without the [gemini] extra installed.
The new scan extends _AttributeCallFinder with an optional parent_module
parameter so the namespace-package shape `from google import genai;
genai.Client(...)` is caught alongside `from google.genai import Client`
and its alias variant — five planted-violation regression tests pin the
three bypass patterns plus a negative case and an Anthropic-path-unchanged
guard. A line-based confinement test
(tests/llm/test_gemini_client_confinement.py) rejects any
google.genai-mentioning `# type: ignore` outside the shim, mirroring
tests/warehouse/test_snowflake_client_confinement.py. The existing
Anthropic shim's regex-level confinement test narrowed from "any
ignore" to "anthropic-mentioning ignore" so the new Gemini shim's own
SDK ignores don't trip it. All four canonical validation steps pass;
2474 tests; coverage 97.33%.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-txe.3: #137 US-003 — pyproject.toml [gemini] extra + dev-group sync
google-genai>=0.5,<1 listed in three pyproject slots in lockstep per DEC-010
(operator install, pip dev back-compat, uv dev group). uv.lock regenerated.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-txe.2: #137 US-002 — GeminiProvider(LLMProvider) + registration
GeminiProvider concrete strategy registered at module import; both capability
flags False per DEC-003. build_create_kwargs sets response_mime_type=
application/json (DEC-018) and maps system → system_instruction + single user
turn (DEC-004). Safety-blocked / no-content responses → LLMResponseFormatError
per DEC-005. Exception taxonomy per DEC-006 — verified against
google-genai==0.8.0.
Notes from verification against the installed SDK:
- google.genai.errors.APIError (the shared parent of ClientError +
ServerError) stores the HTTP code on .code (an int), not .status_code —
the classifier reads .code. ClientError covers 4xx (incl. 401/403/429);
ServerError covers 5xx. ServerError is checked BEFORE ClientError so the
narrower 5xx bucket cannot be shadowed by a future shared parent class.
- The SDK's models.generate_content(config=) accepts both
GenerateContentConfig and the plain-dict GenerateContentConfigDict form.
build_create_kwargs returns the plain-dict form so providers.py never
imports google.genai.types at any scope — keeping
test_gemini_client_confinement.py green and base-install module-import
clean.
- The .messages.create façade required by the orchestrator is wired in
GeminiProvider.make_client via a small _GeminiClientAdapter /
_GeminiMessagesAdapter pair (the shim returns the bare client; US-002
owns the adapter per the shim's docstring). The adapter forwards
**kwargs straight to client.models.generate_content / count_tokens.
- Connection-flavoured exceptions (httpx.ConnectError /
httpx.TimeoutException) leak through the SDK on hard network failures;
the classifier handles them with a lazy httpx import and an
ImportError-safe fallback to NO_RETRY.
Updates the pinned signalforge.llm __all__ surface in
tests/llm/test_public_api.py and tests/draft/test_schema.py to include the
new GeminiProvider re-export; adds DraftConfig/GradeConfig provider="gemini"
acceptance tests.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-txe.9: #137 US-009 — operator-facing docs + CHANGELOG for Gemini
docs/grade-ops.md + docs/draft-ops.md register gemini as a provider, name the
[gemini] install extra + GOOGLE_API_KEY env var, and document the v0.3
no-caching cost note (DEC-013). README provider list deferred (no list exists
to extend). CHANGELOG entry under [Unreleased]. .claude/rules/* deferred to
Patterns & Memory (orchestrator-only).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-txe.4: #137 US-004 — FakeGeminiClient + offline provider integration tests
Hand-rolled FakeGeminiClient mirrors FakeAnthropicClient's expect_* API
(DEC-011). New tests/llm/test_fake_gemini.py proves the fake's contract;
tests/llm/test_gemini_provider_via_fake.py drives GeminiProvider through
the fake end-to-end (call_llm round-trip, safety-blocked branch, retry
exhaustion).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-txe.5: #137 US-005 — provider-neutrality e2e tests (draft + grade)
tests/grade/test_gemini_neutrality.py drives grade_artifacts end-to-end through
FakeGeminiClient: cache_*=0 in JSONL, blake2b-8 hashes, sidecar round-trip, no
dual-zero WARNING (DEC-003), safety-blocked → GradeLLMError degrade (DEC-005).
tests/draft/test_gemini_neutrality.py drives draft_schema end-to-end likewise.
DEC-014 two-stage scope satisfied at the offline-test layer.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-txe.8: #137 US-008 — gemini marker + live tests (raw + draft + grade) + CONTRIBUTING
@pytest.mark.gemini marker registered; addopts excludes it from default CI per
DEC-012. Three live smokes gated SF_RUN_GEMINI=1 + GOOGLE_API_KEY:
test_gemini_live.py (raw call_llm), test_gemini_draft_live.py (draft_schema),
test_gemini_grade_live.py (grade_artifacts 1-criterion × 1-artifact).
CONTRIBUTING.md adds the maintainer 'uv run pytest -m gemini --no-cov' entry
alongside the existing snowflake/anthropic equivalents. --estimate live test
deferred to US-007 (gated on #136).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* #137: harden DEC-019 cross-epic gate via bd sentinel bead
DEC-019 was documentary only; bd_1-scaffolding-txe.7 (US-007) showed
as bd-ready despite the plan saying "wait for #136 to merge to dev."
A Ralph worker running `bd ready` would have picked it up and
rebase-fought #136 on providers.py / pricing.py / cli/_estimate.py
exactly as DEC-019 warns against.
Created sentinel bead bd_1-scaffolding-41a ("#136 OpenAI grading
PR #152 merged to dev") and wired bd_1-scaffolding-txe.7 to depend on
it. `bd ready` no longer surfaces US-007 until the sentinel closes.
Close the sentinel the moment #136 merges to dev → US-007 unblocks
automatically.
Updated DEC-019 + the beads manifest entry on US-007 to point at the
sentinel.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-txe.7: #137 US-007 — GeminiProvider.estimate_input_tokens + --estimate integration
Replaces the merge-resolution NotImplementedError stub with a real
implementation via Gemini's native client.models.count_tokens
(first-party; no tiktoken equivalent). The --estimate cost-preview
path now works end-to-end for grade.provider: gemini and
llm.provider: gemini. Anthropic byte-identity golden unchanged
(verified against tests/fixtures/estimate/anthropic_byte_identity_golden.txt).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-txe.10: #137 Quality Gate — fix bugs from 4-reviewer pass
Four parallel code-review passes surfaced 2 substantive code findings
and 5 documentation findings; all fixed below. CodeRabbit run skipped
(no skill available locally); maintainer can request post-merge.
Code fixes (reviewer 2 — tests):
- tests/test_audit_completeness.py: added the missing planted-violation
regression test for the new parent_module codepath in
_AttributeCallFinder. Covers Patterns 5-8 (namespace-package via
from-google-import-genai, namespace alias, dotted-from-import,
dotted-import-as) plus a negative check pinning that Pattern 7
requires parent_module to be set. Mandated by testing-signal.md
§ "AST single-construction-seam scans must catch all three bypass
patterns" — without this, a refactor of the parent_module branches
could silently break Scan 10 at the exact moment a real Gemini-SDK
construction was added outside _gemini_client.py.
- tests/grade/test_gemini_grade_live.py: added cache_*=0 assertion on
every GradeEvent record (DEC-003 of #137). The docstring claimed
this was tested but the assertion was missing — a grade-path
bookkeeping regression would have slipped through the live smoke.
Doc fixes (reviewer 3 — docs):
- docs/grade-ops.md + docs/draft-ops.md: rewrote the "--estimate
integration (deferred)" sections to reflect that US-007 SHIPPED in
this PR. Both now describe the active behaviour (native
client.models.count_tokens round-trip; failures surface as
<unavailable>) instead of the pre-US-007 deferred state.
- CHANGELOG.md: updated the Gemini Unreleased bullet to advertise
--estimate support as part of the #137 deliverable; reserved the
"follow-up" framing for explicit Gemini context caching only.
- docs/cost-estimate-ops.md: added Gemini coverage in three places —
the provider-aware token counting bullet list, a parallel "Gemini
provider — [gemini] install extra" section (with the three
registered SKUs in a table), and the maintainer live smoke set.
File had ZERO Gemini coverage before this fix.
- README.md: added Google Gemini to the supported-providers section
alongside Anthropic + OpenAI; moved the Gemini roadmap row from
"Planned v0.4" into "Shipped v0.4" since it's landing in this PR.
Reviewer 1 (providers.py) and reviewer 4 (packaging + integration)
reported 0 bugs. Reviewer 2's LOW-priority findings (flake-prone
aggregate_complete assertion, loose substring on confinement scan,
missing 5xx-retry integration test) deferred — not correctness risks.
Validation after fixes:
- ruff check: passed
- ruff format --check: passed
- pyright: 0 errors, 0 warnings, 0 informations
- pytest: 2555 passed, 65 deselected, 97.51% coverage
- wheel_smoke: 2/2 passed (new [gemini] extra packaging intact)
- Anthropic byte-identity golden: passed (unchanged)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-txe.11: #137 Patterns & Memory — durable rules for Gemini
Orchestrator-only commit (Ralph workers can't write under .claude/ in
worktrees per memory ralph-worker-claude-dir-perms.md).
.claude/rules/llm-drafter.md:
- AST scan tally bumped 5 → 6 (Scan 10 = genai.Client confinement
via _AttributeCallFinder(parent_module="google")).
- New § "Gemini provider shape (#137 — the third concrete provider,
no-cache via a namespace-package SDK)" with the four load-bearing
patterns: .messages-over-.models.generate_content adapter,
response_mime_type=application/json server-side JSON enforcement,
safety-filter typed LLMResponseFormatError → grade degrade, and
native models.count_tokens for --estimate (distinct from OpenAI's
local tiktoken path).
- New § "Namespace-package SDKs (#137 generalisation)" documenting
the parent_module parameter that catches the four namespace-package
import shapes the no-parent path misses.
- Reference block extended with the #137 plan + the three new fakes
+ the two new neutrality test suites.
- "new vendor lands" wording generalised (#137 is no longer "next",
it's "shipped"); Anthropic→OpenAI→Gemini lineage noted explicitly.
.claude/rules/grade-layer.md:
- Conservative degrade taxonomy DEC-002/DEC-015 entry for
GradeLLMError extended with a sentence explaining Gemini's
safety-filter / no-content response routes through the same path
via the typed LLMResponseFormatError that GeminiProvider raises
(DEC-005 of #137). Locks in the provider-neutral contract: a
future vendor with a content-filter surface MUST route through
LLMResponseFormatError, not a provider-specific switch in
grade_artifacts.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* #137: Address PR review feedback (Copilot + CodeRabbit)
Five real issues fixed; three outdated threads resolved as-is.
Fixed:
1. src/signalforge/llm/_gemini_client.py — Copilot flagged that
GeminiClientProtocol's docstring claimed the bare SDK client
satisfies it, and _make_gemini_client was annotated to return
GeminiClientProtocol despite returning the raw google.genai.Client
(which has no .messages namespace). The protocol is satisfied only
by the wrapped _GeminiClientAdapter (in providers.py) and by the
test fake. Rewrote both docstrings; relaxed _make_gemini_client's
return type to `Any` and explained the wrapper-on-top contract
explicitly. Replaces the misleading "satisfies structurally" claim
with the honest "bare SDK does NOT satisfy; wrapper does."
2. plans/super/137-gemini-grading.md (Discovery, line 98) — CodeRabbit
flagged the stale PRICE_TABLE_VERSION = "2026-05-11". Annotated
the reference as "at discovery time" and noted that US-006 and
#136 both bump it.
3. plans/super/137-gemini-grading.md (Worker-writability routing) —
CodeRabbit flagged a duplicated "## Worker-writability routing"
heading. The duplicate must have crept in during the cross-review
pass. Removed the second copy; kept the first canonical version.
4. tests/llm/test_pricing.py
(test_lookup_raises_estimateunknownmodelerror_for_unknown_gemini_model)
— CodeRabbit flagged that the test only validates `.model`,
leaving the operator-facing remediation text drifting silently
from the pricing table. Added a structural pin that the rendered
exception names every Gemini SKU. Future SKU additions force a
lockstep remediation update.
Outdated threads (3) — content already fixed by earlier commits; the
threads are kept resolved for hygiene:
5. tests/test_audit_completeness.py docstring "node.module == genai"
note (Copilot, two threads). The actual scan handles the dotted-
form via parent_module; the comments in the implementation
already reflect this since the QG pass.
6. CONTRIBUTING.md pre-release excluded-marker list (CodeRabbit). The
list was already extended to include `gemini` (and `snowflake`)
during the QG fix in commit f15a469.
Validation after fixes:
- ruff check: passed
- ruff format --check: 279 files formatted
- pyright: 0 errors, 0 warnings, 0 informations
- pytest: 2555 passed, 65 deselected, 97.51% coverage
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* #137: Address second-round PR review (Copilot, 4 new comments)
Two real findings fixed; two false positives documented.
Fixed:
1. pyproject.toml gemini marker description — Copilot flagged that
the help text named only GOOGLE_API_KEY but the live tests also
gate on SF_RUN_GEMINI=1. Updated to "requires SF_RUN_GEMINI=1 +
GOOGLE_API_KEY" — mirrors the OpenAI marker's shape exactly.
2. src/signalforge/llm/providers.py::GeminiProvider.extract_usage —
Copilot flagged that getattr(usage, "prompt_token_count", 0) or 0
silently swallows missing/malformed SDK response shapes and feeds
misleading 0/0 figures into the audit JSONL + --estimate math.
Switched to the shared _extract_usage_field helper which raises
LLMResponseFormatError on missing/non-int fields. Matches the
Anthropic precedent (the OpenAI provider already uses the same
helper). Added test_geminiprovider_extract_usage_missing_inner_
field_raises pinning both the missing-field and non-int-type
paths to keep them durable.
False positives (replied in resolve-threads):
3. _gemini_client.py:116 (`_make_gemini_client` return annotation):
Copilot is reviewing the pre-commit-00dc673 diff. The previous
round of fixes (commit 00dc673) already changed the return type
to Any and rewrote the docstring to be honest that the bare SDK
client does NOT satisfy GeminiClientProtocol; only the
_GeminiClientAdapter wrapper does. Current file matches.
4. plans/super/137-gemini-grading.md:777 (duplicate Worker-
writability routing): Same — the duplicate was already removed
in 00dc673. Only one section now exists at line 757.
Validation:
- ruff check: passed
- ruff format --check: passed
- pyright: 0 errors, 0 warnings, 0 informations
- pytest: 2556 passed, 65 deselected, 97.55% coverage
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* #155: Gemini truncation + per-provider e2e gap (#156)
* #155: super-plan — Gemini truncation + per-provider e2e gap
12 DECs covering:
- DEC-001/002/005: provider-neutral is_clean_completion ABC method
- DEC-003/011: 2 new e2e siblings + BQ smoke parametrize
- DEC-008/009: per-provider max_output_tokens floor table (1024/1024/2048)
- DEC-010: pre-release-only cadence (~\$0.30 / suite run)
- DEC-012: apply_provider_override helper
10 stories sized for Ralph contexts. Architecture review: 1 concern (seam
design — resolved), 8 pass. Regression risk green (no fixture pins old
reasoning string; existing test_gemini_neutrality.py:381 already pins the
post-fix shape).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* #155: devolved — epic + 10 tasks live in bd
Epic: bd_1-scaffolding-eu0
Ready set: US-001 (.2), US-003 (.4), US-004 (.5) — three parallel-safe.
16 dep links wired.
Serialization callouts captured (US-005/6/7 share _e2e_helpers + BQ smoke;
US-002 + US-010 edit .claude/rules/, orchestrator-only).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-eu0.4: #155 US-003 — bump Gemini live fixture to 2048 + per-provider max_output_tokens floor docs
* bd_1-scaffolding-eu0.5: #155 US-004 — apply_provider_override helper + BQ smoke uses it
Adds the canonical per-test grade-provider overlay helper. Multi-provider
e2e smokes (BigQuery+Anthropic / +OpenAI / +Gemini) share the committed
Austin fixture and swap only grade.provider/model/max_output_tokens via
this seam — no near-duplicate fixtures.
The helper is non-destructive: unset knobs left alone, sibling top-level
blocks (llm:/safety:/prune:) round-trip via yaml.safe_dump(sort_keys=False).
Missing signalforge.yml raises FileNotFoundError rather than silently
creating one (masks misconfigured tests).
Unit-tested in tests/cli/test_e2e_helpers.py — runs in the default
pytest set (no marker), so a regression in the YAML overlay plumbing the
real e2e smokes depend on trips immediately.
BigQuery smoke refactored to call apply_provider_override(project_dir,
grade_provider='anthropic') as a no-op proof-of-use; OpenAI (US-005) and
Gemini (US-006) sibling smokes will pass non-default values through the
same seam.
Traces to: DEC-012 in plans/super/155-gemini-truncation-e2e-gap.md
* bd_1-scaffolding-eu0.2: #155 US-001 — LLMProvider.is_clean_completion ABC + 3 concretes + wire-in
Add provider-neutral allowlist gate for response finish-reason / stop-reason
to prevent silent pass-through of truncated / safety-filtered / tool-use
responses (DEC-001/002/005/006/007 of plans/super/155).
- LLMProvider.is_clean_completion(response) -> bool — abstract
- LLMProvider.unclean_finish_reason_message(response) -> str — default + per-vendor overrides
- AnthropicProvider._CLEAN_STOP_REASONS = {end_turn, stop_sequence}; tool_use is UNCLEAN (DEC-006)
- OpenAIProvider._CLEAN_STOP_REASONS = {stop}
- GeminiProvider._CLEAN_STOP_REASONS = {STOP}
- call_llm wires the gate immediately before strategy.extract_text_blocks,
raising LLMResponseFormatError (typed, non-retryable response-shape error)
with the provider-specific diagnostic when the gate returns False.
- _DummyProvider + FakeNoCacheProvider satisfy the new abstract by
returning True (no finish-reason concept on the canned shapes).
TDD: 4 happy-path tests added (one per concrete provider + 2 for Anthropic
covering both clean stop reasons) and confirmed failing before implementation,
then green. Canonical validation quad (ruff/format/pyright/pytest) all-green;
2560 tests pass (no regressions); AST scans 3/9/10 pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-eu0.6: #155 US-005 — new tests/cli/test_e2e_openai_smoke.py
Full-pipeline live e2e gated by @pytest.mark.e2e + @pytest.mark.openai
markers and a three-env-var skip gate (SF_RUN_OPENAI=1, OPENAI_API_KEY,
GOOGLE_CLOUD_PROJECT — BigQuery stays the warehouse). Mirrors
tests/cli/test_e2e_bigquery_smoke.py verbatim and only swaps the grader
via apply_provider_override(grade_provider='openai', grade_model='gpt-4o')
per DEC-011/DEC-012 of plans/super/155-gemini-truncation-e2e-gap.md;
drafter stays Anthropic Sonnet per the fixture's llm.model pin and the
cost-table rationale (DEC-009).
Pins the seven invariants from the BQ smoke (DEC-009 of #10): exit 0,
sidecar present, kept+flagged+dropped>=1, always-passes drop present
(warehouse-side, provider-independent), flagged_count>=1 (tight grade
thresholds), GradingReport.aggregate_complete=True (the cross-provider
contract the in-isolation grade smokes can't pin), and no traceback in
stderr (cli-layer.md DEC-016).
Test is deselected by default addopts; maintainer runs once pre-release.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-eu0.7: #155 US-006 — new tests/cli/test_e2e_gemini_smoke.py with max_output_tokens=2048 overlay
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-eu0.3: #155 US-002 — per-provider unclean-path tests + call_llm integration (rule edit deferred to orchestrator)
Per-provider unclean-path tests pinning the LLMProvider.is_clean_completion
contract (#155 DEC-001/DEC-002/DEC-005/DEC-006/DEC-007):
- tests/llm/test_anthropic_provider_via_fake.py: + max_tokens (with partial
text), tool_use, unclean_finish_reason_message naming stop_reason.
- tests/llm/test_openai_provider_via_fake.py: + length (with partial text),
content_filter, tool_calls, unclean_finish_reason_message naming
finish_reason.
- tests/llm/test_gemini_provider_via_fake.py: + MAX_TOKENS-with-partial-text
(the LOAD-BEARING #155 Finding 1 regression pin, with full context comment),
SAFETY/RECITATION/OTHER parametrized, unclean_finish_reason_message naming
finish_reason.
- tests/llm/test_client.py: + call_llm integration test asserting
LLMResponseFormatError raises at the is_clean_completion gate
(post-messages.create, pre-extract_text_blocks, no retry).
Existing contract pin tests/grade/test_gemini_neutrality.py:381 continues
to pass unmodified — the safety-blocked Gemini path now routes through the
same orchestrator gate as MAX_TOKENS, both landing at
'call failed: GradeLLMError' degrade.
Note: .claude/rules/llm-drafter.md edit deferred — per memory
ralph-worker-claude-dir-perms, .claude/ writes are orchestrator-only and
will land in a separate commit after this merge.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-eu0.8: #155 US-007 — parametrize tests/cli/test_e2e_bigquery_smoke.py over grade.provider
Adds @pytest.mark.parametrize over grade_provider ∈ [anthropic, openai,
gemini] to the BigQuery e2e smoke test. Per #155 DEC-003 this covers
the cross-provider diff-sidecar rendering contract the in-isolation
grade smokes (tests/grade/test_*_grade_live.py) cannot pin: those
smokes never exercise the diff-sidecar evidence/reasoning cascade with
a non-Anthropic judge.
Per #155 DEC-011 the drafter stays Anthropic Sonnet across all three
variants (fixture stability — the LLM payload the drafter sees is
unchanged, so the always-passes column the LLM proposes is
reproducibly the same). Only the grader varies, via the canonical
apply_provider_override helper (US-004 / DEC-012).
Per-variant env-var gates layered on top of the existing baseline:
- anthropic: baseline only (SF_RUN_BQ + ANTHROPIC_API_KEY + GOOGLE_CLOUD_PROJECT)
- openai: baseline + SF_RUN_OPENAI + OPENAI_API_KEY
- gemini: baseline + SF_RUN_GEMINI + GOOGLE_API_KEY
Gemini variant pins max_output_tokens=2048 per #155 Finding 2 — Gemini
2.5-flash's verbose reasoning field truncates mid-string at the
default 512/1024 floors and would flake assertion #6 (aggregate_complete).
Sibling files (test_e2e_openai_smoke.py / test_e2e_gemini_smoke.py)
remain per DEC-011 for per-provider failure ergonomics and cost
transparency; this parametrize is internal to the BQ smoke.
Pytest IDs: test_*[anthropic] / test_*[openai] / test_*[gemini]. All
three are deselected by default addopts (gated by the e2e marker);
the maintainer runs them per pre-release live suite (US-008).
Validation: uv sync --dev && ruff check && ruff format --check &&
pyright && pytest — all green (2566 passed, 69 deselected, coverage
97.40%).
* bd_1-scaffolding-eu0.3: #155 US-002 (orchestrator portion) — clarify llm-drafter.md DEC-005 for the is_clean_completion seam
Updates the Gemini-section DEC-005 contract to reflect the post-#155 generalisation:
- Rule now applies to ALL providers (Anthropic stop_reason, OpenAI choices[0].finish_reason,
Gemini candidates[0].finish_reason.name) — not just Gemini
- Enforcement moved from "no text at all" check buried in extract_text_blocks to the new
LLMProvider.is_clean_completion(response) -> bool ABC method (#155 DEC-005)
- Closes the Finding-1 gap: MAX_TOKENS with partial text now surfaces as
LLMResponseFormatError → GradeLLMError, not GradeOutputError(json_parse)
- Per-provider _CLEAN_STOP_REASONS enumerated; tool_use deliberately UNCLEAN per DEC-006
- unclean_finish_reason_message override (DEC-007) keeps vendor-native field names visible
Test-side pins (worker portion of US-002, commit 9a23e3f):
- tests/llm/test_{anthropic,openai,gemini}_provider_via_fake.py — unclean-path coverage
- tests/llm/test_client.py — call_llm gate integration
Worker (commit 9a23e3f) deferred this rule edit per memory ralph-worker-claude-dir-perms;
this commit closes the bead's orchestrator-only deliverable.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-eu0.9: #155 US-008 — CONTRIBUTING.md live-suite pre-release cadence + env-var block
Add a 'Live e2e suite (pre-release only)' subsection to CONTRIBUTING.md
per DEC-010 of plans/super/155-gemini-truncation-e2e-gap.md.
Documents the full pre-release maintainer audit:
- 5 paid e2e tests (BQ smoke parametrized over 3 grader providers per
US-007, OpenAI sibling, Gemini sibling with max_output_tokens=2048
floor per DEC-008, Snowflake sibling)
- 6 grade-only / draft-only live-API smokes gated by anthropic /
openai / gemini markers
- One-shot invocation with -m 'e2e or anthropic or openai or gemini'
--no-cov and the full env-var stack (SF_RUN_*, ANTHROPIC_API_KEY,
OPENAI_API_KEY, GOOGLE_API_KEY, GOOGLE_CLOUD_PROJECT, SNOWFLAKE_*)
Frames the cadence as pre-release only — NOT per-PR, NOT CI-gated. The
addopts exclusion in pyproject.toml already keeps these out of default
runs; this section just documents the maintainer-side invocation when
cutting a release. Cost ceiling: ~$0.30/run × ~2–3 audits/month =
~$0.60–1.00/month.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bd_1-scaffolding-eu0.10: #155 US-009 — Quality Gate fixes (4-pass code review)
Pass 1 (correctness): 0 real bugs. 2 doc-drift fixes:
- docs/grade-ops.md floor table: pre-fix GradeOutputError narrative
rewritten to post-fix LLMResponseFormatError → GradeLLMError per #155 DEC-005
- tests/cli/test_e2e_gemini_smoke.py invariant-#6 comment: same update
Pass 2 (simplification): 0 refactors needed.
Pass 3 (test coverage): 0 critical gaps. 3 nice-to-have safety nets added:
- tests/llm/test_gemini_provider_via_fake.py — new orchestrator wire-in pin
for MAX_TOKENS+partial-text via call_llm (distinct from existing
SAFETY-only call_llm test)
- tests/llm/test_openai_provider_via_fake.py — new orchestrator wire-in pin
for length+partial-text via call_llm (no prior call_llm coverage)
- tests/grade/test_gemini_neutrality.py:381 — docstring comment binding the
existing safety-blocked pin to the new #155 MAX_TOKENS routing path
Pass 4 (rules compliance): 1 real rule violation + 1 multi-surface drift
(same root cause). Fixed:
- tests/cli/test_e2e_openai_smoke.py::_skip_reason() expanded from 3 to 5
env-va…
Merged
wjduenow
added a commit
that referenced
this pull request
May 30, 2026
* chore: begin 0.4.0.dev0
* #135: provider-neutral LLM seam (plan) (#148)
* Add super plan for #135: provider-neutral LLM seam
* Mark #135 plan published (PR #148)
* Mark #135 plan devolved (beads epic bd_1-scaffolding-j2c)
* bd_1-scaffolding-j2c.1: US-001 provider foundation (ExceptionCategory, UsageMetrics, LLMProvider ABC, registry)
Adds the provider-neutral LLM seam foundation (DEC-001/002/003 of #135):
- src/signalforge/llm/providers.py: ExceptionCategory (5-member enum),
UsageMetrics (frozen pydantic value object, cache fields default 0),
LLMProvider ABC, and the process-level registry (register_provider /
provider_for). No vendor behaviour wired; registry starts empty.
- UnknownProviderError(LLMError) in errors.py — lists available registered
provider names, repr-safe message, default_remediation; registered at
CLI exit-code tier 2 (looked-up-identifier-not-in-table input failure).
- Exported the new public names from signalforge.llm.__all__; updated the
documented-surface lists in test_public_api.py / test_schema.py and the
errors count in test_errors.py.
* bd_1-scaffolding-j2c.2: US-002 AnthropicProvider strategy + _anthropic_client rename + AST scan
* bd_1-scaffolding-j2c.3: US-003 generic call_llm orchestrator (rename, capability gating, strategy dispatch)
Collapse call_anthropic into a provider-neutral call_llm orchestrator:
- Resolve strategy via provider_for(provider); build client via
strategy.make_client() when client is None (DEC-006).
- Pre-send count gate now gated on strategy.supports_token_count; cache
marker / beta header + dual-zero anomaly WARNING gated on
supports_prompt_caching (DEC-008). Anthropic = both True ⇒ byte-identical.
- Retry loop dispatches on strategy.classify_exception -> ExceptionCategory
(DEC-001) instead of catching Anthropic SDK classes directly; per-class
budgets, backoff math, WARNING shape, exhaustion raises preserved.
- Response assembled via strategy.extract_text_blocks/extract_usage.
- Neutral _LLMClientProtocol narrows the resolved client so no vendor-SDK
type / type-checker suppression leaks into llm.client (DEC-012 confinement).
Drop call_anthropic (name + __all__); add call_llm. Migrate draft_from_request
and grade._grade_one call sites + all client/retry/public-api/schema tests.
* bd_1-scaffolding-j2c.4: US-004 provider config field + stage threading + CLI client-construction migration
DEC-006/007 of #135. Adds a registry-validated `provider: str = "anthropic"`
field to DraftConfig and GradeConfig, each with a field_validator that
delegates to providers.provider_for (raises UnknownProviderError listing
available names on an unknown value — fails loud at config load). Threads
provider=config.provider into call_llm from draft_from_request and
grade._grade_one.
Migrates the CLI generate path off the in-CLI _make_anthropic_client helper
(removed): the real-run draft/grade stages now pass client=None so call_llm
lazy-builds the client via the configured provider. The --estimate
short-circuit (Anthropic-specific count_tokens) builds its concrete client via
provider_for(draft_config.provider).make_client(). Migrates the 7 CLI
monkeypatch tests: 5 batch/select tests drop the orphaned make_anthropic_client
mock; the estimate test patches AnthropicProvider.make_client; test_lint
unchanged (it patches the LLM shim, which still exists).
* bd_1-scaffolding-j2c.5: US-005 no-cache fake provider neutrality proof (AC #2/#3)
* bd_1-scaffolding-j2c.6: US-006 docs + surface parity (docs/ portion)
Update operator-facing docs for the provider-neutral LLM seam (#135):
- call_anthropic -> call_llm; llm._client -> llm._anthropic_client
- document the provider config knob (llm.provider / grade.provider),
default "anthropic", registry-validated, forward-looking plugin seam
- note prompt caching + count_tokens are now provider capabilities
(supports_prompt_caching / supports_token_count); Anthropic unchanged
* bd_1-scaffolding-j2c.6: US-006 .claude/rules seam-rename + provider-seam accuracy
Orchestrator-only rule-file edits (workers can't write .claude/ in worktrees).
* bd_1-scaffolding-j2c.7: Quality gate — fix bugs from code review
- providers.build_count_tokens_kwargs: correct docstring (probe omits the
cache_control marker because it doesn't affect the token count; the old
claim of "matching the inline call_anthropic probe" was inaccurate).
- call_llm: gate cache_marker_active on BOTH supports_prompt_caching AND
supports_token_count, so a future caching=True/token_count=False provider
degrades to no-caching rather than sending an unvalidated cache_control
marker (no sub-minimum drop / no oversize cap). Anthropic is True/True so
the default path is a no-op. (4-pass review latent-trap finding.)
CodeRabbit: skill not available in this environment — skipped.
* bd_1-scaffolding-j2c.8: Patterns & Memory — provider-seam convention + QG lesson
- llm-drafter.md: capability-gating lesson (gate the cache marker on BOTH
supports_prompt_caching AND supports_token_count — the #135 QG latent-trap
finding) for future #136/#137 provider authors.
- Added `provider` to the documented `llm:` config block.
- Reference section now cites plans/super/135 + the no-cache neutrality proof.
* #135: close Codecov patch gaps — count_tokens probe error mapping + classify fallthrough
The provider-neutral refactor rewrote the count_tokens pre-send probe's
error-mapping as new lines (auth/rate-limit/conn/5xx/other -> typed LLMError,
no retry on the probe) that the existing retry tests (messages.create path)
never exercised. Add 6 probe-failure tests + a missing-input_tokens case
(client.py 320-348 now covered, 87% -> 96%).
Also cover AnthropicProvider.classify_exception's defensive fallthrough for a
non-5xx/non-4xx-non-auth APIStatusError (3xx) -> NO_RETRY (providers.py -> 100%).
Remaining uncovered client.py helper branches (_extract_text_blocks /
_extract_usage_field malformed-response raises) are pre-existing and not part
of the #135 diff.
* #135: address PR review (CodeRabbit/Copilot)
- Stale-symbol docstrings: rename every `call_anthropic` :func: reference to
`call_llm` across llm/{__init__,models,_anthropic_client,providers}.py,
draft/schema.py, grade/{engine,config,errors}.py, cli/_estimate.py; reword
the AnthropicProvider "preparatory refactor" note (US-003 already collapsed
the duplication); rewrite the llm package docstring to the provider-neutral
seam.
- client.py: count_tokens NO_RETRY fallback message no longer claims a "status"
(it also covers non-status exceptions) — now "failed with a non-retryable error".
- generate.py --estimate: fail fast with CliInputError when draft.provider !=
grade.provider or the provider isn't 'anthropic' (the estimate engine drives a
single Anthropic-cast count_tokens client across both configs). + 2 regression
tests.
- plan SD-1: drop the stale `Literal` wording (DEC-007 settled it as a
registry-validated str).
---------
* docs(readme): refresh status, generalize config, redraft roadmap (#153)
* docs(readme): refresh status, generalize config, redraft roadmap
The v0.1 "Status:" line was four releases stale and BigQuery-only.
Replace with a Supported-warehouses block covering BigQuery (v0.1) and
Snowflake (v0.3 — incl. the deferred aggregate-only / column_stats
limitation), generalize Configuration to dispatch on dbt profile type,
and redraft the Roadmap into Shipped (v0.1/v0.2/v0.3 with release dates)
+ Planned (Gemini/OpenAI → installable skill → Airflow → GitHub Action
→ rubric customization → dbt Fusion). Drop the now-stale "prune-existing
is dev-branch only" and "first four ship; prune-existing is v0.2 dev"
callouts.
* docs(readme): add custom-business-logic callout, strip non-roadmap version markers
- Add a **Custom business logic** paragraph alongside Supported warehouses,
explaining `meta.signalforge.business_rules` → `custom_sql` drafting + the
ownership-marker write semantics + always-pass drop behaviour.
- Remove every version marker outside the Roadmap section: "since v0.1/v0.3"
in the warehouses block, "*(v0.2)*" on the prune-existing bullet, the "v0.2
default" config comment, the "(v0.1)"/"(v0.3)" subsection headers, and the
"pre-alpha … v0.1 milestone" line in Contributing.
* docs(readme): reorganize scope callouts; promote Supported warehouses to its own H2
The two bolded standalone paragraphs at the top of the README (Supported
warehouses, Custom business logic) sat before the "Why this exists" pitch
and front-loaded dense capability detail. The custom-business-logic block
was also triple-covered (standalone block + "What it does" bullet +
Quick-start worked example).
- Drop both standalone bolded paragraphs from the top.
- Promote Supported warehouses to a proper H2 between "How it works"
and the "Live on PyPI" callout — brief, points to Configuration for
per-warehouse setup; Snowflake's known limitation stays in its single
home under Configuration > Snowflake.
- Strengthen the custom-business-rule bullet in "What it does" with the
"fifth test type" framing and a direct link to the worked example, so
the feature stays prominent without a redundant top-of-readme banner.
New flow: pitch → features → architecture → scope → quickstart → reference.
---------
* #136: OpenAI grading provider (plan) (#152)
* #136: super plan for OpenAI grading provider
Phase: detailing (awaiting approval).
13 DECs captured: Chat Completions API, no provider/model cross-validation,
fully wire --estimate for OpenAI (new estimate_input_tokens ABC), default
judge gpt-4o, scope both grade + draft, server-side response_format=json_object,
four pricing SKUs (gpt-4o, -mini, 4.1, 4-turbo), live smoke covers grade +
estimate, .messages adapter wraps chat.completions, AST Scan 3 extension,
[openai] extra + lazy tiktoken, Anthropic estimate byte-identity floor.
9 stories: shim + AST confinement, OpenAIProvider + registration, FakeOpenAIClient
+ neutrality test, pricing entries, --estimate provider-aware counting,
live gated smokes, docs, Quality Gate, Patterns & Memory.
* #136: link plan to PR #152 (phase=published)
* #136: apply 5 plan revisions from #137 cross-review
Cross-review against #137 (Gemini grading) plan surfaced 5 substantive gaps
in the OpenAI plan:
1. DEC-010 — fix wording: a NEW 9th AST scan (Scan 3 is Anthropic-specific),
not an extension. Reuse _QualifiedNameCallFinder + three-pattern bypass
regression test per testing-signal.md.
2. DEC-012 — fix pyproject.toml listing: three slots in lockstep
([project.optional-dependencies].openai + .dev + [dependency-groups].dev),
mirroring Snowflake precedent per python-build.md. Plan previously named
only two.
3. DEC-014 (new) — codify the _load_openai_exception_classes empty-tuple
fallback on ImportError (mirrors AnthropicProvider). Includes refusal /
content-filter symmetry note explaining why OpenAI needs no Gemini-style
safety_filter typed-degrade DEC.
4. US-006 — add a third live test (draft_schema) honouring DEC-005's
"scope both stages" commitment at live level too. DEC-008 updated to
match (three tests, not two).
5. New "Open notes for implementation" section — pragmatic SDK-class-name
verification, .messages adapter shape confirmation, tiktoken fallback
table, response_format prompt-requirement check, Anthropic byte-identity
snapshot capture protocol.
Plus a new "Worker-writability routing" section codifying that US-009
(Patterns & Memory) is orchestrator-only because it edits .claude/rules/,
mirroring #137's same routing.
Plan grows from 319 → 343 lines (+40/-17).
* #136: devolve plan into bd beads (epic bd_1-scaffolding-4tw + 9 tasks)
Fills in the Beads manifest section. Epic bd_1-scaffolding-4tw with 9
child tasks (US-001 … US-009) created with dep edges per the plan's
"Depends on:" lines:
US-001 → none (READY)
US-002 → US-001
US-003 → US-002
US-004 → none (parallel-safe) (READY)
US-005 → US-002, US-004
US-006 → US-002, US-005
US-007 → US-001, US-002, US-005
US-008 → US-001..US-007 (QG)
US-009 → US-008 (P&M; orchestrator-only)
Cross-epic gate wired downstream: #137's sentinel bd_1-scaffolding-41a
now DEPENDS ON US-009, so #137 US-007 stays mechanically blocked until
this epic completes + sentinel closes after PR #152 merges to dev.
Parallel-safe at the entry points: US-001 and US-004 edit disjoint
files (shim/pyproject/AST vs pricing) — ralph-serialize-shared-registry
does not apply. Run concurrently.
bd ready (post-devolve) surfaces .1 + .4 of this epic + the two epics
themselves + the unrelated Airflow integration.
* bd_1-scaffolding-4tw.4: #136 US-004 — OpenAI pricing SKUs (gpt-4o + 3 siblings)
Adds four OpenAI SKUs to _PRICES_MUTABLE per DEC-007: gpt-4o (default
judge per DEC-004), gpt-4o-mini (budget), gpt-4.1 (newer flagship),
gpt-4-turbo (back-compat). Cache fields = 0.0 (OpenAI has no equivalent
cache discount). PRICE_TABLE_VERSION bumped to 2026-05-28.
Anthropic SKUs unchanged (byte-identical) — Anthropic estimate
byte-identity floor (DEC-013) preserved at the pricing layer.
Tests assert: non-zero input/output rates + zero cache fields for all
four; unknown model still raises EstimateUnknownModelError; Anthropic
SKUs unchanged; version bump pinned.
OpenAI per-MTok rates are calibration figures captured at PR-prep time
pending operator verification against https://openai.com/api/pricing/;
the figures are sanity-check baselines, not billing guarantees.
Traces: DEC-003, DEC-004, DEC-007.
* bd_1-scaffolding-4tw.1: #136 US-001 — _openai_client.py shim + [openai] extra + 9th AST scan
Implements the per-vendor shim confining every openai SDK type ignore.
Adds [openai] optional-dep extra (openai + tiktoken) in three-slot
lockstep across [project.optional-dependencies].openai, .dev, and
[dependency-groups].dev. Adds the new 9th AST scan in
tests/test_audit_completeness.py reusing _QualifiedNameCallFinder
(NOT an extension of Scan 3 — Scan 3 is Anthropic-specific) and a
companion per-file confinement test mirroring Snowflake's. Bumps the
docstring tally 8→9.
DEC-014 empty-tuple ImportError fallback on _load_openai_exception_classes
mirrors AnthropicProvider; tiktoken cl100k_base fallback for unknown
model ids.
Traces: DEC-001, DEC-009, DEC-010, DEC-012, DEC-014.
* bd_1-scaffolding-4tw.2: #136 US-002 — OpenAIProvider + registration + config-validator coverage
Implements OpenAIProvider(LLMProvider) with both capability flags False
(no prompt caching, no pre-send token count) — mirrors FakeNoCacheProvider
end-to-end shape. build_create_kwargs attaches response_format=
{type: json_object} per DEC-006 (server-side JSON enforcement); no
cache_control / extra_headers. extract_text_blocks reads
response.choices[0].message.content; extract_usage maps usage.prompt_tokens
+ completion_tokens → UsageMetrics with cache fields 0. classify_exception
covers all five ExceptionCategory branches via _load_openai_exception_classes
from the US-001 shim.
register_provider(OpenAIProvider()) at module scope so GradeConfig/
DraftConfig validators accept provider='openai'. Exported from
signalforge.llm.__init__.
Tests pin: each ABC method's contract; config validator acceptance for
both stages; UnknownProviderError lists both anthropic and openai.
Traces: DEC-001, DEC-005, DEC-006, DEC-009, DEC-011.
* bd_1-scaffolding-4tw.3: #136 US-003 — FakeOpenAIClient + grade neutrality e2e
Adds tests/llm/_fake_openai.py with FakeOpenAIClient + expect_messages_create
API mirroring FakeAnthropicClient verbatim. Support dataclasses match the
real chat-completions response shape (choices[0].message.content; usage
with prompt_tokens + completion_tokens).
Adds tests/grade/test_provider_neutrality_openai.py — the OpenAI analogue
of the FakeNoCacheProvider neutrality proof. Asserts cache_*=0 in JSONL
audit, blake2b-8 reproducibility hashes present, sidecar drift-detector
round-trip, no dual-zero cache-anomaly WARNING in caplog, all expectations
consumed.
Traces: DEC-001, DEC-005, DEC-006, DEC-009, DEC-011.
* bd_1-scaffolding-4tw.5: #136 US-005 — --estimate provider-aware token counting
Adds LLMProvider.estimate_input_tokens(model, text, *, client=None) -> int
as an abstract method on the ABC (DEC-003). AnthropicProvider impl
delegates to the SDK's messages.count_tokens with a single user-message
text envelope; OpenAIProvider impl delegates to _count_openai_tokens
(tiktoken with cl100k_base fallback per DEC-012); FakeNoCacheProvider
and _DummyProvider impls return trivial deterministic answers so the
existing neutrality + registry tests still pass.
Refactors cli/_estimate.py: _count_draft_tokens and the grader-side
equivalent dispatch through provider_for(config.provider).estimate_input_tokens
— no more hard-coded anthropic_client.messages.count_tokens calls. The
engine signature relaxes anthropic_client to 'object | None' so the
OpenAI path can pass None (tiktoken is local). Lifts the
'--estimate currently supports only provider=anthropic' gate in
cli/generate.py; the divergent-providers check stays (the engine still
takes one optional client).
Pins DEC-013 Anthropic byte-identity floor via
tests/cli/test_estimate.py::test_estimate_anthropic_byte_identity_golden
and tests/fixtures/estimate/anthropic_byte_identity_golden.txt (captured
2026-05-28 before the refactor; reproduced verbatim after). New
companion tests prove the OpenAI path produces non-zero token counts
+ non-zero USD, ignores the threaded anthropic_client, and goes through
_count_openai_tokens (patched-helper assertion).
Traces: DEC-003, DEC-007, DEC-012, DEC-013.
* bd_1-scaffolding-4tw.7: #136 US-007 — operator docs + CHANGELOG (worker-writable portion)
Updates operator-facing documentation surfaces for OpenAI grading +
drafting:
- docs/grade-ops.md — OpenAI provider section (config snippet,
OPENAI_API_KEY, no-cache caveat, live smoke gating link)
- docs/draft-ops.md — equivalent for llm.provider: openai
- docs/cost-estimate-ops.md — tiktoken note, [openai] extra install,
four pricing SKUs
- CHANGELOG.md — [Unreleased] Added entry for #136
- README.md — provider enumeration extended (if applicable)
The .claude/rules/llm-drafter.md update is deferred to US-009
(Patterns & Memory) per the project's worker-writability rule
(orchestrator-only edits under .claude/).
Traces: DEC-001 through DEC-014.
* bd_1-scaffolding-4tw.6: #136 US-006 — live gated smoke tests (openai marker + 3 tests)
Adds @pytest.mark.openai marker + three gated live-API tests:
- tests/grade/test_smoke_real_api_openai.py — grade_artifacts(provider=openai)
- tests/draft/test_smoke_real_api_openai.py — draft_schema(provider=openai)
(honors DEC-005 both-stages at live level)
- tests/cli/test_e2e_estimate_openai.py — generate --estimate openai
All three env-gated on SF_RUN_OPENAI=1 + OPENAI_API_KEY via belt-and-
suspenders _skip_reason() helper per testing-signal.md. Marker registered
in pyproject.toml + added to addopts exclusion so default pytest doesn't
collect them. CONTRIBUTING.md documents the maintainer-only run pattern.
Traces: DEC-001, DEC-004, DEC-005, DEC-008.
* bd_1-scaffolding-4tw.8: #136 Quality Gate — fix 2 majors + 1 minor from code review x4
Code-review pass 3 surfaced two correctness MAJORS + pass 4 surfaced
a doc MINOR; all fixed inline per /ralph-run Step 3b. Pass 1, 2, 4
(other surfaces) reported no correctness bugs.
MAJOR 1 — EstimateUnknownModelError.default_remediation enumerated
only the three Anthropic SKUs. An operator typo on an OpenAI SKU
got pointed at Anthropic options (violates "errors carry remediation"
in manifest-readers.md). Updated to enumerate all 7 SKUs
(claude-sonnet-4-6, claude-opus-4-7, claude-haiku-4-5, gpt-4o,
gpt-4o-mini, gpt-4.1, gpt-4-turbo) + refreshed the locked-text test
that pinned the stale string.
MAJOR 2 — AnthropicProvider.estimate_input_tokens dropped the
pre-refactor `system=` kwarg, concatenating system+cached+dynamic
into a single user-content string. Anthropic's server-side tokenizer
counts the system block with its own envelope tokens; without the
kwarg, real-API counts under-report by the system-envelope size.
The fake-driven byte-identity test passed only because the fake
returned canned input_tokens regardless of kwargs (so rendered-output
identity held, but DEC-013's "byte-identity vs the pre-refactor
Anthropic count_tokens call" spirit broke at the real-API level).
Fix extends the ABC signature with a keyword-only `system: str = ""`
parameter. AnthropicProvider passes it via `system=` kwarg when
non-empty (matches pre-refactor real-API call shape). OpenAIProvider
concatenates `system + text` before tiktoken (tiktoken has no
system-envelope distinction; the total still counts every token).
FakeNoCacheProvider concatenates for the word-count proxy.
_DummyProvider gains the new kwarg for ABC parity. cli/_estimate.py
callers (_count_draft_tokens, _count_grade_criterion_tokens) thread
`system=` separately; grade-side preserves the pre-existing
double-count of the rubric (passed as both system= AND in user content)
to keep byte-identity with the pre-refactor shape.
MINOR — docs/grade-ops.md:118 config-snippet comment said "only
anthropic registered today", contradicting the same file's later
"## OpenAI provider" section and docs/draft-ops.md's correct
wording. Updated to mirror draft-ops's form.
Validation: ruff/pyright/pytest all green (2498 passed, 62 deselected,
97.34% coverage); wheel_smoke green (2 passed).
Note: CodeRabbit skill not available in this environment, skipped
per Step 3b "(if available)".
* bd_1-scaffolding-4tw.9: #136 Patterns & Memory — codify OpenAI seam conventions
Orchestrator-only commit (workers can't write under .claude/ in
worktrees per ralph-worker-claude-dir-perms.md).
Updates .claude/rules/llm-drafter.md to capture three durable patterns
from #136:
1. Provider-neutral seam — adds OpenAI as the second registered
provider (name="openai", both capability flags False) and extends
the LLMProvider ABC surface enumeration with the new
estimate_input_tokens(model, text, *, system="", client=None)
method (#136 US-005).
2. "OpenAI provider shape" subsection — codifies the .messages.create
façade adapter pattern (SimpleNamespace delegating to
chat.completions.create), the response_format={type:json_object}
server-side JSON enforcement (with the cross-ref to issue #144's
tolerant parser as fallback), and the tiktoken cl100k_base
fallback for unknown model ids. This becomes the canonical
precedent for #137 Gemini's response_mime_type=application/json
equivalent.
3. estimate_input_tokens(*, system=...) — documents the load-bearing
reason the system envelope is threaded separately (Anthropic's
server-side tokenizer applies system-block envelope tokens; dropping
the kwarg under-reports real-API counts silently because
fake-driven byte-identity tests can't catch call-shape drift). Also
pins the deliberate double-count of system_and_rubric in
_count_grade_criterion_tokens as pre-existing behaviour that must
be preserved.
Bumps AST-scan tally section from "four" → "five" (adds the openai.OpenAI
9th-project-scan); flags #137 Gemini as the 10th. Updates the
References block with #136 plan + new files.
Memory entry: ~/.claude/projects/.../memory/fake-driven-byte-identity-
blind-spot.md captures the QG lesson (fakes return canned values
regardless of kwargs → call-shape drift slips past rendered-output
snapshots → needs explicit kwargs-shape assertion OR live test).
Indexed in MEMORY.md.
Validation: all four canonical gates green (2498 passed, 97.34%
coverage).
* #136: address PR #152 review (6 actionable + meta)
Copilot:
- MAJOR _estimate.py:359 — OpenAI grade-side over-counted rubric. The
first QG fix preserved a pre-existing Anthropic double-count of
system_and_rubric (passed as both system= AND in user content),
which triple-counted on OpenAI (system→system+text concat → rubric
prefix in text). Corrected to match the runtime grader call: rubric
in system= once, artifact envelope in user content. Anthropic real-
API counts drop one rubric copy from the buggy pre-refactor bytes;
OpenAI counts each input once. Fake-driven byte-identity golden
still passes (canned token counts are call-shape-agnostic).
CHANGELOG documents the calibration shift.
- NIT _estimate.py:421 — renamed anthropic_client → client. Type was
already object | None and forwarded to whichever provider strategy
is active; old name implied Anthropic-only and would mislead a
future #137 Gemini wiring. CLI in generate.py already passed None
for non-Anthropic; rename surfaces the contract without behaviour
change.
- META plan/PR description — addressed by updating the PR body
separately (not part of this commit).
CodeRabbit:
- CONTRIBUTING.md:64 — added snowflake to the gated-marker audit
command + the corresponding SNOWFLAKE_* env vars.
- tests/llm/_fake_provider.py:234 — boundary-word undercount fix in
the FakeNoCacheProvider word-count proxy (f"{system} {text}" with
a delimiter; was system+text which merged the last word of system
with the first word of text under .split()).
- tests/llm/test_openai_client_confinement.py:47 — glob("*.py") →
rglob("*.py") so the confinement scan catches openai-mentioning
ignore directives in any future nested signalforge/llm/ subpackage,
not just top-level files. Path display becomes relative-to-_LLM_DIR.
- tests/test_audit_completeness.py:1080 — closed the "call-before-
import alias" bypass on _AttributeCallFinder by adding a two-pass
visit_Module that pre-collects every alias module-wide before
visiting any Call node. Mirrors the same fix already shipped on
_QualifiedNameCallFinder (PR #69 / DEC-013). Pattern-4 regression
test added.
Validation: ruff/format/pyright/pytest all green (2498 passed,
97.34% coverage); wheel_smoke green.
* #136: close PR #152 codecov gaps (9 lines → 0)
Codecov flagged 91.82% patch coverage with 9 lines missing across
3 files. Closed all 9 with 6 focused unit tests:
src/signalforge/llm/_openai_client.py (was 85%, now 100%):
- _OpenAIClientAdapter.__init__ + ._messages_create — no production
caller exercises the adapter (orchestrator drives via
FakeOpenAIClient which has its own .messages.create); added
test_openai_client_adapter_messages_create_delegates_to_chat_completions
that builds a SimpleNamespace raw client and pins the delegation
+ kwargs forwarding.
- _count_openai_tokens cl100k_base fallback (DEC-012 unknown-model
branch) — added test_count_openai_tokens_falls_back_to_cl100k_base_
for_unknown_model.
src/signalforge/llm/providers.py (was 98%, now 100%):
- AnthropicProvider.estimate_input_tokens no-system-kwarg arm (the
else branch added in QG) — added test_anthropic_provider_estimate_
input_tokens_skips_system_kwarg_when_empty, which ALSO pins the
load-bearing invariant that an empty system MUST be omitted from
the SDK kwargs (otherwise the count carries a spurious system=""
block).
- LLMResponseFormatError raise on missing input_tokens — added
test_anthropic_provider_estimate_input_tokens_raises_on_missing_
input_tokens.
- OpenAIProvider.extract_text_blocks missing-message arm — added
test_openai_provider_extract_text_blocks_missing_message_attr_
raises (distinct from the existing content=None and empty-choices
cases — those exercise different arms).
src/signalforge/cli/generate.py:846 (was 66%, now covered):
- The non-Anthropic `client = None` branch in cmd_generate's
--estimate short-circuit — exercised only by the
@pytest.mark.openai live smoke before. Added test_generate_
estimate_openai_provider_passes_client_none_to_engine that drives
cmd_generate with llm.provider: openai + grade.provider: openai
in a tmp signalforge.yml, spies on AnthropicProvider.make_client
to confirm it's NEVER called on the openai path, and captures the
client kwarg into estimate(...) to pin client=None. The remaining
7 uncovered lines in generate.py are pre-PR and out of scope.
Total: 2504 passed (was 2498), 97.47% coverage (was 97.34%), no
new gated tests added (all unit tests, default-CI-included).
---------
* #137: Gemini grading provider (plan) (#151)
* #137: Gemini grading provider (plan)
Super plan for #137 (Gemini grading provider) — depends on #135 (merged).
9 stories: shim + provider + extra + fake/unit + neutrality + live +
docs + QG + P&M. Caching deferred (both capability flags False);
safety-filter no-content surfaces as LLMResponseFormatError →
grade GradeLLMError degrade. Both drafter and grader covered.
* #137: extend plan after comparison with #136 OpenAI plan
Closes the three gaps surfaced by the #136 comparison:
- Add --estimate integration (DEC-016): GeminiProvider.estimate_input_tokens
via native client.models.count_tokens (cleaner than #136's tiktoken path —
Gemini has a first-party count endpoint).
- Add 3 Gemini pricing SKUs to pricing.py (DEC-017): gemini-2.5-pro,
gemini-2.5-flash, gemini-2.0-flash; bump PRICE_TABLE_VERSION.
- Add server-side JSON enforcement (DEC-018): response_mime_type=
application/json — mirrors #136 DEC-006.
- Add CHANGELOG entry to docs story (US-009).
- Add wheel_smoke to QG (new [gemini] extra changes packaging).
- Sequence after #136 (DEC-019): inherit ABC extension + estimate refactor.
New stories: US-006 (pricing, parallel-safe), US-007 (estimate impl,
depends on #136). Old US-006/007 renumbered to US-008/009.
* #137: apply 3 plan refinements from #136 cross-review
Closes the smaller gaps surfaced by re-reviewing #137 against #136 after
the #136 plan was tightened (520b36c):
1. DEC-009 — merge-order-conditional AST scan tally. Currently said "8 → 9"
unconditionally, but DEC-019 specifies #137 ships AFTER #136. Realistic
bump is 9 → 10 (since #136's openai.OpenAI scan bumps 8 → 9 first);
added the fallback wording for the slipped-sequencing case.
2. Dedicated "Worker-writability routing" top-level section codifying that
Patterns & Memory is orchestrator-only (.claude/rules/ writes). Was a
one-line architecture-review row; now a durable section that mirrors
#136's codification — future #138+ provider plans copy the pattern.
3. Open notes for implementation — two additions:
- response_mime_type requires NO prompt keyword (contrast with OpenAI's
response_format which fails without "json" in prompt). Prevents a
future maintainer adding defensive prompt text.
- pricing.lookup zero cache fields math validation (symmetric to #136
verification item).
Major gaps from the prior cross-review (--estimate, pricing, dual-listing,
JSON enforcement, CHANGELOG, three live tests, sequencing) were already
addressed in b28c0b5.
* #137: devolve plan to beads (phase=devolved)
Epic: bd_1-scaffolding-txe
Tasks: 11 (US-001 through US-009 + Quality Gate + Patterns & Memory)
Dependencies: 19 edges per the plan's dependency graph
Ready queue: US-001 (shim) + US-006 (pricing, parallel-safe)
US-007 (--estimate integration) is blocked on US-002 + US-006 AND
carries a cross-epic gate on #136 landing first per DEC-019 — that
sequencing is documented in the task's description rather than wired
as a bead dep since #136's beads live in a different epic.
US-011 (Patterns & Memory) is flagged orchestrator-only because it
edits .claude/rules/ which Ralph workers can't write under in
worktrees (per the Worker-writability routing section of the plan).
* #137: adjust plan now that #136 is being implemented first
Folds devolve-time annotations (phase=devolved, beads epic
bd_1-scaffolding-txe, per-task status + cross-epic blocker on US-007)
together with five #136-driven adjustments:
1. AST-scan tally locked to 10th (not 9th) — #136 owns the 8→9 bump
for openai.OpenAI; #137 takes 9→10 for genai.Client. Updated in
DEC-009, US-001, P&M, and Open notes.
2. DEC-019 sharpened: #136 plan → #136 implementation; explicit rebase
guidance (rebase on dev after #136 merges) rather than contingency
wording.
3. DEC-005 gains a refusal/content-filter symmetry note cross-ref'ing
#136 DEC-014 — explains why OpenAI deliberately ships no
safety-filter typed-degrade and why Gemini deliberately does.
4. New "Worker-writability routing" section mirroring #136's, codifying
that P&M is orchestrator-only because it edits .claude/rules/.
5. Open notes extended with two pragmatic items parallel to #136's:
- google-genai count_tokens response field-name verification
(.total_tokens vs .total_token_count) at US-007 implementation.
- Anthropic byte-identity snapshot ownership: #136 owns it; #137
inherits it; the wiring is wrong if it moves the snapshot.
Plan grows 762 → 825 lines (+70/-30).
* bd_1-scaffolding-txe.6: #137 US-006 — add Gemini pricing SKUs (gemini-2.5-pro, gemini-2.5-flash, gemini-2.0-flash)
Adds three Gemini SKUs to _PRICES_MUTABLE per DEC-017, each with positive
input_per_mtok / output_per_mtok per Google's public Gemini API price page
and cache fields = 0.0 (v0.3 ships Gemini without an Anthropic-equivalent
prompt-cache discount). Bumps PRICE_TABLE_VERSION to "2026-05-27". Tests
parametrise the three SKUs, pin Anthropic SKU byte-identity (additive-only),
and assert lookup("gemini-unknown") still raises EstimateUnknownModelError.
cli/_estimate.py reads only input_per_mtok and output_per_mtok in the USD
math (lines 468, 511), so zero cache fields are safe — no divide-by-zero
or NaN risk. The EstimateUnknownModelError.default_remediation in
llm/errors.py still lists only the three Claude SKUs by name; per the
story scope ("pricing-only; DO NOT touch any other file") this is left
for a follow-up to broaden once the wider Gemini surface (provider class
+ --estimate integration in US-007) lands.
* bd_1-scaffolding-txe.1: #137 US-001 — _gemini_client.py shim + new AST confinement scan
Land the per-vendor Gemini SDK shim and the 9th AST audit-completeness
scan that confines genai.Client(...) constructions to it. Mirrors the
Anthropic shim shape verbatim — GeminiClientProtocol (with .messages
façade for US-002 and .models for US-007), lazy-import factory, frozen
_GeminiExceptionClasses dataclass with empty-tuple fallback per DEC-015
so the module imports cleanly without the [gemini] extra installed.
The new scan extends _AttributeCallFinder with an optional parent_module
parameter so the namespace-package shape `from google import genai;
genai.Client(...)` is caught alongside `from google.genai import Client`
and its alias variant — five planted-violation regression tests pin the
three bypass patterns plus a negative case and an Anthropic-path-unchanged
guard. A line-based confinement test
(tests/llm/test_gemini_client_confinement.py) rejects any
google.genai-mentioning `# type: ignore` outside the shim, mirroring
tests/warehouse/test_snowflake_client_confinement.py. The existing
Anthropic shim's regex-level confinement test narrowed from "any
ignore" to "anthropic-mentioning ignore" so the new Gemini shim's own
SDK ignores don't trip it. All four canonical validation steps pass;
2474 tests; coverage 97.33%.
* bd_1-scaffolding-txe.3: #137 US-003 — pyproject.toml [gemini] extra + dev-group sync
google-genai>=0.5,<1 listed in three pyproject slots in lockstep per DEC-010
(operator install, pip dev back-compat, uv dev group). uv.lock regenerated.
* bd_1-scaffolding-txe.2: #137 US-002 — GeminiProvider(LLMProvider) + registration
GeminiProvider concrete strategy registered at module import; both capability
flags False per DEC-003. build_create_kwargs sets response_mime_type=
application/json (DEC-018) and maps system → system_instruction + single user
turn (DEC-004). Safety-blocked / no-content responses → LLMResponseFormatError
per DEC-005. Exception taxonomy per DEC-006 — verified against
google-genai==0.8.0.
Notes from verification against the installed SDK:
- google.genai.errors.APIError (the shared parent of ClientError +
ServerError) stores the HTTP code on .code (an int), not .status_code —
the classifier reads .code. ClientError covers 4xx (incl. 401/403/429);
ServerError covers 5xx. ServerError is checked BEFORE ClientError so the
narrower 5xx bucket cannot be shadowed by a future shared parent class.
- The SDK's models.generate_content(config=) accepts both
GenerateContentConfig and the plain-dict GenerateContentConfigDict form.
build_create_kwargs returns the plain-dict form so providers.py never
imports google.genai.types at any scope — keeping
test_gemini_client_confinement.py green and base-install module-import
clean.
- The .messages.create façade required by the orchestrator is wired in
GeminiProvider.make_client via a small _GeminiClientAdapter /
_GeminiMessagesAdapter pair (the shim returns the bare client; US-002
owns the adapter per the shim's docstring). The adapter forwards
**kwargs straight to client.models.generate_content / count_tokens.
- Connection-flavoured exceptions (httpx.ConnectError /
httpx.TimeoutException) leak through the SDK on hard network failures;
the classifier handles them with a lazy httpx import and an
ImportError-safe fallback to NO_RETRY.
Updates the pinned signalforge.llm __all__ surface in
tests/llm/test_public_api.py and tests/draft/test_schema.py to include the
new GeminiProvider re-export; adds DraftConfig/GradeConfig provider="gemini"
acceptance tests.
* bd_1-scaffolding-txe.9: #137 US-009 — operator-facing docs + CHANGELOG for Gemini
docs/grade-ops.md + docs/draft-ops.md register gemini as a provider, name the
[gemini] install extra + GOOGLE_API_KEY env var, and document the v0.3
no-caching cost note (DEC-013). README provider list deferred (no list exists
to extend). CHANGELOG entry under [Unreleased]. .claude/rules/* deferred to
Patterns & Memory (orchestrator-only).
* bd_1-scaffolding-txe.4: #137 US-004 — FakeGeminiClient + offline provider integration tests
Hand-rolled FakeGeminiClient mirrors FakeAnthropicClient's expect_* API
(DEC-011). New tests/llm/test_fake_gemini.py proves the fake's contract;
tests/llm/test_gemini_provider_via_fake.py drives GeminiProvider through
the fake end-to-end (call_llm round-trip, safety-blocked branch, retry
exhaustion).
* bd_1-scaffolding-txe.5: #137 US-005 — provider-neutrality e2e tests (draft + grade)
tests/grade/test_gemini_neutrality.py drives grade_artifacts end-to-end through
FakeGeminiClient: cache_*=0 in JSONL, blake2b-8 hashes, sidecar round-trip, no
dual-zero WARNING (DEC-003), safety-blocked → GradeLLMError degrade (DEC-005).
tests/draft/test_gemini_neutrality.py drives draft_schema end-to-end likewise.
DEC-014 two-stage scope satisfied at the offline-test layer.
* bd_1-scaffolding-txe.8: #137 US-008 — gemini marker + live tests (raw + draft + grade) + CONTRIBUTING
@pytest.mark.gemini marker registered; addopts excludes it from default CI per
DEC-012. Three live smokes gated SF_RUN_GEMINI=1 + GOOGLE_API_KEY:
test_gemini_live.py (raw call_llm), test_gemini_draft_live.py (draft_schema),
test_gemini_grade_live.py (grade_artifacts 1-criterion × 1-artifact).
CONTRIBUTING.md adds the maintainer 'uv run pytest -m gemini --no-cov' entry
alongside the existing snowflake/anthropic equivalents. --estimate live test
deferred to US-007 (gated on #136).
* #137: harden DEC-019 cross-epic gate via bd sentinel bead
DEC-019 was documentary only; bd_1-scaffolding-txe.7 (US-007) showed
as bd-ready despite the plan saying "wait for #136 to merge to dev."
A Ralph worker running `bd ready` would have picked it up and
rebase-fought #136 on providers.py / pricing.py / cli/_estimate.py
exactly as DEC-019 warns against.
Created sentinel bead bd_1-scaffolding-41a ("#136 OpenAI grading
PR #152 merged to dev") and wired bd_1-scaffolding-txe.7 to depend on
it. `bd ready` no longer surfaces US-007 until the sentinel closes.
Close the sentinel the moment #136 merges to dev → US-007 unblocks
automatically.
Updated DEC-019 + the beads manifest entry on US-007 to point at the
sentinel.
* bd_1-scaffolding-txe.7: #137 US-007 — GeminiProvider.estimate_input_tokens + --estimate integration
Replaces the merge-resolution NotImplementedError stub with a real
implementation via Gemini's native client.models.count_tokens
(first-party; no tiktoken equivalent). The --estimate cost-preview
path now works end-to-end for grade.provider: gemini and
llm.provider: gemini. Anthropic byte-identity golden unchanged
(verified against tests/fixtures/estimate/anthropic_byte_identity_golden.txt).
* bd_1-scaffolding-txe.10: #137 Quality Gate — fix bugs from 4-reviewer pass
Four parallel code-review passes surfaced 2 substantive code findings
and 5 documentation findings; all fixed below. CodeRabbit run skipped
(no skill available locally); maintainer can request post-merge.
Code fixes (reviewer 2 — tests):
- tests/test_audit_completeness.py: added the missing planted-violation
regression test for the new parent_module codepath in
_AttributeCallFinder. Covers Patterns 5-8 (namespace-package via
from-google-import-genai, namespace alias, dotted-from-import,
dotted-import-as) plus a negative check pinning that Pattern 7
requires parent_module to be set. Mandated by testing-signal.md
§ "AST single-construction-seam scans must catch all three bypass
patterns" — without this, a refactor of the parent_module branches
could silently break Scan 10 at the exact moment a real Gemini-SDK
construction was added outside _gemini_client.py.
- tests/grade/test_gemini_grade_live.py: added cache_*=0 assertion on
every GradeEvent record (DEC-003 of #137). The docstring claimed
this was tested but the assertion was missing — a grade-path
bookkeeping regression would have slipped through the live smoke.
Doc fixes (reviewer 3 — docs):
- docs/grade-ops.md + docs/draft-ops.md: rewrote the "--estimate
integration (deferred)" sections to reflect that US-007 SHIPPED in
this PR. Both now describe the active behaviour (native
client.models.count_tokens round-trip; failures surface as
<unavailable>) instead of the pre-US-007 deferred state.
- CHANGELOG.md: updated the Gemini Unreleased bullet to advertise
--estimate support as part of the #137 deliverable; reserved the
"follow-up" framing for explicit Gemini context caching only.
- docs/cost-estimate-ops.md: added Gemini coverage in three places —
the provider-aware token counting bullet list, a parallel "Gemini
provider — [gemini] install extra" section (with the three
registered SKUs in a table), and the maintainer live smoke set.
File had ZERO Gemini coverage before this fix.
- README.md: added Google Gemini to the supported-providers section
alongside Anthropic + OpenAI; moved the Gemini roadmap row from
"Planned v0.4" into "Shipped v0.4" since it's landing in this PR.
Reviewer 1 (providers.py) and reviewer 4 (packaging + integration)
reported 0 bugs. Reviewer 2's LOW-priority findings (flake-prone
aggregate_complete assertion, loose substring on confinement scan,
missing 5xx-retry integration test) deferred — not correctness risks.
Validation after fixes:
- ruff check: passed
- ruff format --check: passed
- pyright: 0 errors, 0 warnings, 0 informations
- pytest: 2555 passed, 65 deselected, 97.51% coverage
- wheel_smoke: 2/2 passed (new [gemini] extra packaging intact)
- Anthropic byte-identity golden: passed (unchanged)
* bd_1-scaffolding-txe.11: #137 Patterns & Memory — durable rules for Gemini
Orchestrator-only commit (Ralph workers can't write under .claude/ in
worktrees per memory ralph-worker-claude-dir-perms.md).
.claude/rules/llm-drafter.md:
- AST scan tally bumped 5 → 6 (Scan 10 = genai.Client confinement
via _AttributeCallFinder(parent_module="google")).
- New § "Gemini provider shape (#137 — the third concrete provider,
no-cache via a namespace-package SDK)" with the four load-bearing
patterns: .messages-over-.models.generate_content adapter,
response_mime_type=application/json server-side JSON enforcement,
safety-filter typed LLMResponseFormatError → grade degrade, and
native models.count_tokens for --estimate (distinct from OpenAI's
local tiktoken path).
- New § "Namespace-package SDKs (#137 generalisation)" documenting
the parent_module parameter that catches the four namespace-package
import shapes the no-parent path misses.
- Reference block extended with the #137 plan + the three new fakes
+ the two new neutrality test suites.
- "new vendor lands" wording generalised (#137 is no longer "next",
it's "shipped"); Anthropic→OpenAI→Gemini lineage noted explicitly.
.claude/rules/grade-layer.md:
- Conservative degrade taxonomy DEC-002/DEC-015 entry for
GradeLLMError extended with a sentence explaining Gemini's
safety-filter / no-content response routes through the same path
via the typed LLMResponseFormatError that GeminiProvider raises
(DEC-005 of #137). Locks in the provider-neutral contract: a
future vendor with a content-filter surface MUST route through
LLMResponseFormatError, not a provider-specific switch in
grade_artifacts.
* #137: Address PR review feedback (Copilot + CodeRabbit)
Five real issues fixed; three outdated threads resolved as-is.
Fixed:
1. src/signalforge/llm/_gemini_client.py — Copilot flagged that
GeminiClientProtocol's docstring claimed the bare SDK client
satisfies it, and _make_gemini_client was annotated to return
GeminiClientProtocol despite returning the raw google.genai.Client
(which has no .messages namespace). The protocol is satisfied only
by the wrapped _GeminiClientAdapter (in providers.py) and by the
test fake. Rewrote both docstrings; relaxed _make_gemini_client's
return type to `Any` and explained the wrapper-on-top contract
explicitly. Replaces the misleading "satisfies structurally" claim
with the honest "bare SDK does NOT satisfy; wrapper does."
2. plans/super/137-gemini-grading.md (Discovery, line 98) — CodeRabbit
flagged the stale PRICE_TABLE_VERSION = "2026-05-11". Annotated
the reference as "at discovery time" and noted that US-006 and
#136 both bump it.
3. plans/super/137-gemini-grading.md (Worker-writability routing) —
CodeRabbit flagged a duplicated "## Worker-writability routing"
heading. The duplicate must have crept in during the cross-review
pass. Removed the second copy; kept the first canonical version.
4. tests/llm/test_pricing.py
(test_lookup_raises_estimateunknownmodelerror_for_unknown_gemini_model)
— CodeRabbit flagged that the test only validates `.model`,
leaving the operator-facing remediation text drifting silently
from the pricing table. Added a structural pin that the rendered
exception names every Gemini SKU. Future SKU additions force a
lockstep remediation update.
Outdated threads (3) — content already fixed by earlier commits; the
threads are kept resolved for hygiene:
5. tests/test_audit_completeness.py docstring "node.module == genai"
note (Copilot, two threads). The actual scan handles the dotted-
form via parent_module; the comments in the implementation
already reflect this since the QG pass.
6. CONTRIBUTING.md pre-release excluded-marker list (CodeRabbit). The
list was already extended to include `gemini` (and `snowflake`)
during the QG fix in commit f15a469.
Validation after fixes:
- ruff check: passed
- ruff format --check: 279 files formatted
- pyright: 0 errors, 0 warnings, 0 informations
- pytest: 2555 passed, 65 deselected, 97.51% coverage
* #137: Address second-round PR review (Copilot, 4 new comments)
Two real findings fixed; two false positives documented.
Fixed:
1. pyproject.toml gemini marker description — Copilot flagged that
the help text named only GOOGLE_API_KEY but the live tests also
gate on SF_RUN_GEMINI=1. Updated to "requires SF_RUN_GEMINI=1 +
GOOGLE_API_KEY" — mirrors the OpenAI marker's shape exactly.
2. src/signalforge/llm/providers.py::GeminiProvider.extract_usage —
Copilot flagged that getattr(usage, "prompt_token_count", 0) or 0
silently swallows missing/malformed SDK response shapes and feeds
misleading 0/0 figures into the audit JSONL + --estimate math.
Switched to the shared _extract_usage_field helper which raises
LLMResponseFormatError on missing/non-int fields. Matches the
Anthropic precedent (the OpenAI provider already uses the same
helper). Added test_geminiprovider_extract_usage_missing_inner_
field_raises pinning both the missing-field and non-int-type
paths to keep them durable.
False positives (replied in resolve-threads):
3. _gemini_client.py:116 (`_make_gemini_client` return annotation):
Copilot is reviewing the pre-commit-00dc673 diff. The previous
round of fixes (commit 00dc673) already changed the return type
to Any and rewrote the docstring to be honest that the bare SDK
client does NOT satisfy GeminiClientProtocol; only the
_GeminiClientAdapter wrapper does. Current file matches.
4. plans/super/137-gemini-grading.md:777 (duplicate Worker-
writability routing): Same — the duplicate was already removed
in 00dc673. Only one section now exists at line 757.
Validation:
- ruff check: passed
- ruff format --check: passed
- pyright: 0 errors, 0 warnings, 0 informations
- pytest: 2556 passed, 65 deselected, 97.55% coverage
---------
* #155: Gemini truncation + per-provider e2e gap (#156)
* #155: super-plan — Gemini truncation + per-provider e2e gap
12 DECs covering:
- DEC-001/002/005: provider-neutral is_clean_completion ABC method
- DEC-003/011: 2 new e2e siblings + BQ smoke parametrize
- DEC-008/009: per-provider max_output_tokens floor table (1024/1024/2048)
- DEC-010: pre-release-only cadence (~\$0.30 / suite run)
- DEC-012: apply_provider_override helper
10 stories sized for Ralph contexts. Architecture review: 1 concern (seam
design — resolved), 8 pass. Regression risk green (no fixture pins old
reasoning string; existing test_gemini_neutrality.py:381 already pins the
post-fix shape).
* #155: devolved — epic + 10 tasks live in bd
Epic: bd_1-scaffolding-eu0
Ready set: US-001 (.2), US-003 (.4), US-004 (.5) — three parallel-safe.
16 dep links wired.
Serialization callouts captured (US-005/6/7 share _e2e_helpers + BQ smoke;
US-002 + US-010 edit .claude/rules/, orchestrator-only).
* bd_1-scaffolding-eu0.4: #155 US-003 — bump Gemini live fixture to 2048 + per-provider max_output_tokens floor docs
* bd_1-scaffolding-eu0.5: #155 US-004 — apply_provider_override helper + BQ smoke uses it
Adds the canonical per-test grade-provider overlay helper. Multi-provider
e2e smokes (BigQuery+Anthropic / +OpenAI / +Gemini) share the committed
Austin fixture and swap only grade.provider/model/max_output_tokens via
this seam — no near-duplicate fixtures.
The helper is non-destructive: unset knobs left alone, sibling top-level
blocks (llm:/safety:/prune:) round-trip via yaml.safe_dump(sort_keys=False).
Missing signalforge.yml raises FileNotFoundError rather than silently
creating one (masks misconfigured tests).
Unit-tested in tests/cli/test_e2e_helpers.py — runs in the default
pytest set (no marker), so a regression in the YAML overlay plumbing the
real e2e smokes depend on trips immediately.
BigQuery smoke refactored to call apply_provider_override(project_dir,
grade_provider='anthropic') as a no-op proof-of-use; OpenAI (US-005) and
Gemini (US-006) sibling smokes will pass non-default values through the
same seam.
Traces to: DEC-012 in plans/super/155-gemini-truncation-e2e-gap.md
* bd_1-scaffolding-eu0.2: #155 US-001 — LLMProvider.is_clean_completion ABC + 3 concretes + wire-in
Add provider-neutral allowlist gate for response finish-reason / stop-reason
to prevent silent pass-through of truncated / safety-filtered / tool-use
responses (DEC-001/002/005/006/007 of plans/super/155).
- LLMProvider.is_clean_completion(response) -> bool — abstract
- LLMProvider.unclean_finish_reason_message(response) -> str — default + per-vendor overrides
- AnthropicProvider._CLEAN_STOP_REASONS = {end_turn, stop_sequence}; tool_use is UNCLEAN (DEC-006)
- OpenAIProvider._CLEAN_STOP_REASONS = {stop}
- GeminiProvider._CLEAN_STOP_REASONS = {STOP}
- call_llm wires the gate immediately before strategy.extract_text_blocks,
raising LLMResponseFormatError (typed, non-retryable response-shape error)
with the provider-specific diagnostic when the gate returns False.
- _DummyProvider + FakeNoCacheProvider satisfy the new abstract by
returning True (no finish-reason concept on the canned shapes).
TDD: 4 happy-path tests added (one per concrete provider + 2 for Anthropic
covering both clean stop reasons) and confirmed failing before implementation,
then green. Canonical validation quad (ruff/format/pyright/pytest) all-green;
2560 tests pass (no regressions); AST scans 3/9/10 pass.
* bd_1-scaffolding-eu0.6: #155 US-005 — new tests/cli/test_e2e_openai_smoke.py
Full-pipeline live e2e gated by @pytest.mark.e2e + @pytest.mark.openai
markers and a three-env-var skip gate (SF_RUN_OPENAI=1, OPENAI_API_KEY,
GOOGLE_CLOUD_PROJECT — BigQuery stays the warehouse). Mirrors
tests/cli/test_e2e_bigquery_smoke.py verbatim and only swaps the grader
via apply_provider_override(grade_provider='openai', grade_model='gpt-4o')
per DEC-011/DEC-012 of plans/super/155-gemini-truncation-e2e-gap.md;
drafter stays Anthropic Sonnet per the fixture's llm.model pin and the
cost-table rationale (DEC-009).
Pins the seven invariants from the BQ smoke (DEC-009 of #10): exit 0,
sidecar present, kept+flagged+dropped>=1, always-passes drop present
(warehouse-side, provider-independent), flagged_count>=1 (tight grade
thresholds), GradingReport.aggregate_complete=True (the cross-provider
contract the in-isolation grade smokes can't pin), and no traceback in
stderr (cli-layer.md DEC-016).
Test is deselected by default addopts; maintainer runs once pre-release.
* bd_1-scaffolding-eu0.7: #155 US-006 — new tests/cli/test_e2e_gemini_smoke.py with max_output_tokens=2048 overlay
* bd_1-scaffolding-eu0.3: #155 US-002 — per-provider unclean-path tests + call_llm integration (rule edit deferred to orchestrator)
Per-provider unclean-path tests pinning the LLMProvider.is_clean_completion
contract (#155 DEC-001/DEC-002/DEC-005/DEC-006/DEC-007):
- tests/llm/test_anthropic_provider_via_fake.py: + max_tokens (with partial
text), tool_use, unclean_finish_reason_message naming stop_reason.
- tests/llm/test_openai_provider_via_fake.py: + length (with partial text),
content_filter, tool_calls, unclean_finish_reason_message naming
finish_reason.
- tests/llm/test_gemini_provider_via_fake.py: + MAX_TOKENS-with-partial-text
(the LOAD-BEARING #155 Finding 1 regression pin, with full context comment),
SAFETY/RECITATION/OTHER parametrized, unclean_finish_reason_message naming
finish_reason.
- tests/llm/test_client.py: + call_llm integration test asserting
LLMResponseFormatError raises at the is_clean_completion gate
(post-messages.create, pre-extract_text_blocks, no retry).
Existing contract pin tests/grade/test_gemini_neutrality.py:381 continues
to pass unmodified — the safety-blocked Gemini path now routes through the
same orchestrator gate as MAX_TOKENS, both landing at
'call failed: GradeLLMError' degrade.
Note: .claude/rules/llm-drafter.md edit deferred — per memory
ralph-worker-claude-dir-perms, .claude/ writes are orchestrator-only and
will land in a separate commit after this merge.
* bd_1-scaffolding-eu0.8: #155 US-007 — parametrize tests/cli/test_e2e_bigquery_smoke.py over grade.provider
Adds @pytest.mark.parametrize over grade_provider ∈ [anthropic, openai,
gemini] to the BigQuery e2e smoke test. Per #155 DEC-003 this covers
the cross-provider diff-sidecar rendering contract the in-isolation
grade smokes (tests/grade/test_*_grade_live.py) cannot pin: those
smokes never exercise the diff-sidecar evidence/reasoning cascade with
a non-Anthropic judge.
Per #155 DEC-011 the drafter stays Anthropic Sonnet across all three
variants (fixture stability — the LLM payload the drafter sees is
unchanged, so the always-passes column the LLM proposes is
reproducibly the same). Only the grader varies, via the canonical
apply_provider_override helper (US-004 / DEC-012).
Per-variant env-var gates layered on top of the existing baseline:
- anthropic: baseline only (SF_RUN_BQ + ANTHROPIC_API_KEY + GOOGLE_CLOUD_PROJECT)
- openai: baseline + SF_RUN_OPENAI + OPENAI_API_KEY
- gemini: baseline + SF_RUN_GEMINI + GOOGLE_API_KEY
Gemini variant pins max_output_tokens=2048 per #155 Finding 2 — Gemini
2.5-flash's verbose reasoning field truncates mid-string at the
default 512/1024 floors and would flake assertion #6 (aggregate_complete).
Sibling files (test_e2e_openai_smoke.py / test_e2e_gemini_smoke.py)
remain per DEC-011 for per-provider failure ergonomics and cost
transparency; this parametrize is internal to the BQ smoke.
Pytest IDs: test_*[anthropic] / test_*[openai] / test_*[gemini]. All
three are deselected by default addopts (gated by the e2e marker);
the maintainer runs them per pre-release live suite (US-008).
Validation: uv sync --dev && ruff check && ruff format --check &&
pyright && pytest — all green (2566 passed, 69 deselected, coverage
97.40%).
* bd_1-scaffolding-eu0.3: #155 US-002 (orchestrator portion) — clarify llm-drafter.md DEC-005 for the is_clean_completion seam
Updates the Gemini-section DEC-005 contract to reflect the post-#155 generalisation:
- Rule now applies to ALL providers (Anthropic stop_reason, OpenAI choices[0].finish_reason,
Gemini candidates[0].finish_reason.name) — not just Gemini
- Enforcement moved from "no text at all" check buried in extract_text_blocks to the new
LLMProvider.is_clean_completion(response) -> bool ABC method (#155 DEC-005)
- Closes the Finding-1 gap: MAX_TOKENS with partial text now surfaces as
LLMResponseFormatError → GradeLLMError, not GradeOutputError(json_parse)
- Per-provider _CLEAN_STOP_REASONS enumerated; tool_use deliberately UNCLEAN per DEC-006
- unclean_finish_reason_message override (DEC-007) keeps vendor-native field names visible
Test-side pins (worker portion of US-002, commit 9a23e3f):
- tests/llm/test_{anthropic,openai,gemini}_provider_via_fake.py — unclean-path coverage
- tests/llm/test_client.py — call_llm gate integration
Worker (commit 9a23e3f) deferred this rule edit per memory ralph-worker-claude-dir-perms;
this commit closes the bead's orchestrator-only deliverable.
* bd_1-scaffolding-eu0.9: #155 US-008 — CONTRIBUTING.md live-suite pre-release cadence + env-var block
Add a 'Live e2e suite (pre-release only)' subsection to CONTRIBUTING.md
per DEC-010 of plans/super/155-gemini-truncation-e2e-gap.md.
Documents the full pre-release maintainer audit:
- 5 paid e2e tests (BQ smoke parametrized over 3 grader providers per
US-007, OpenAI sibling, Gemini sibling with max_output_tokens=2048
floor per DEC-008, Snowflake sibling)
- 6 grade-only / draft-only live-API smokes gated by anthropic /
openai / gemini markers
- One-shot invocation with -m 'e2e or anthropic or openai or gemini'
--no-cov and the full env-var stack (SF_RUN_*, ANTHROPIC_API_KEY,
OPENAI_API_KEY, GOOGLE_API_KEY, GOOGLE_CLOUD_PROJECT, SNOWFLAKE_*)
Frames the cadence as pre-release only — NOT per-PR, NOT CI-gated. The
addopts exclusion in pyproject.toml already keeps these out of default
runs; this section just documents the maintainer-side invocation when
cutting a release. Cost ceiling: ~$0.30/run × ~2–3 audits/month =
~$0.60–1.00/month.
* bd_1-scaffolding-eu0.10: #155 US-009 — Quality Gate fixes (4-pass code review)
Pass 1 (correctness): 0 real bugs. 2 doc-drift fixes:
- docs/grade-ops.md floor table: pre-fix GradeOutputError narrative
rewritten to post-fix LLMResponseFormatError → GradeLLMError per #155 DEC-005
- tests/cli/test_e2e_gemini_smoke.py invariant-#6 comment: same update
Pass 2 (simplification): 0 refactors needed.
Pass 3 (test coverage): 0 critical gaps. 3 nice-to-have safety nets added:
- tests/llm/test_gemini_provider_via_fake.py — new orchestrator wire-in pin
for MAX_TOKENS+partial-text via call_llm (distinct from existing
SAFETY-only call_llm test)
- tests/llm/test_openai_provider_via_fake.py — new orchestrator wire-in pin
for length+partial-text via call_llm (no prior call_llm coverage)
- tests/grade/test_gemini_neutrality.py:381 — docstring comment binding the
existing safety-blocked pin to the new #155 MAX_TOKENS routing path
Pass 4 (rules compliance): 1 real rule violation + 1 multi-surface drift
(same root cause). Fixed:
- tests/cli/test_e2e_openai_smoke.py::_skip_reason() expanded from 3 to 5
env-va…
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 30, 2026
Closed
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
#136 OpenAI grading provider — implementation complete + QG. Registers OpenAI as the second concrete LLM provider behind the #135 provider-neutral seam, selectable via
grade.provider: openai/llm.provider: openai. Anthropic stays the default (byte-identical at the rendered-output layer); the seam is now genuinely vendor-pluggable, validating that #137 Gemini can slot in as the third provider with the same pattern.This PR contains the full US-001…US-007 implementation chain + QG fixes + Patterns & Memory — NOT plan-only. The plan doc at
plans/super/136-openai-grading-provider.mdstays in-tree as the canonical ADR-style record (DEC-001 … DEC-014 + the worker-writability and open-notes sections).What lands
src/signalforge/llm/_openai_client.py— the per-vendor shim where everyopenaiSDK# type: ignorelives._OpenAIClientAdapterexposes a.messages.createfaçade delegating tochat.completions.create;.messages.count_tokensraisesNotImplementedErrordefensively._load_openai_exception_classesempty-tupleImportErrorfallback (DEC-014);_count_openai_tokenswithcl100k_basefallback for unknown ids (DEC-012).OpenAIProviderinsrc/signalforge/llm/providers.py— both capability flagsFalse(no prompt caching, no pre-send token count);response_format={"type": "json_object"}server-side JSON enforcement (DEC-006); registered at module scope.src/signalforge/llm/pricing.py—gpt-4o,gpt-4o-mini,gpt-4.1,gpt-4-turbo;PRICE_TABLE_VERSIONbumped.LLMProvider.estimate_input_tokens(model, text, *, system="", client=None)— new ABC method (DEC-003) generalising the--estimatetoken-counting path. AnthropicProvider preserves byte-identity via SDKcount_tokenswithsystem=kwarg; OpenAIProvider counts locally viatiktoken.cli/_estimate.pyrefactored to dispatch through the provider strategy;cli/generate.pylifted the anthropic-only--estimategate.[openai]optional extra inpyproject.toml(DEC-012) — three-slot lockstep:[project.optional-dependencies].openai,[project.optional-dependencies].dev,[dependency-groups].dev. Operator install:pip install signalforge-dbt[openai].tests/test_audit_completeness.py—openai.OpenAI(...)constructions only in_openai_client.py(NEW scan, NOT an extension of Scan 3 which is Anthropic-specific). Reuses_AttributeCallFinder; closes the late-import alias bypass via a two-passvisit_Module. Companiontests/llm/test_openai_client_confinement.pyline-based scan for# type: ignoreconfinement.tests/llm/_fake_openai.py::FakeOpenAIClientmirrorsFakeAnthropicClient'sexpect_*API.tests/grade/test_provider_neutrality_openai.pyprovesgrade_artifacts(provider="openai")end-to-end withcache_*=0JSONL/sidecar round-trip, blake2b-8 reproducibility hashes intact, no dual-zero WARNING.@pytest.mark.openaismokes — grade, draft, AND--estimate(per DEC-005/DEC-008), env-gated onSF_RUN_OPENAI=1+OPENAI_API_KEY.docs/grade-ops.md+docs/draft-ops.mdOpenAI provider sections; newdocs/cost-estimate-ops.md(tiktoken note,[openai]install, pricing SKUs);CONTRIBUTING.mdenv-var documentation;README.mdprovider mention;CHANGELOG.md[Unreleased]entry..claude/rules/llm-drafter.mdupdated with the OpenAI shim sub-section +estimate_input_tokens(*, system=...)byte-identity discussion + 9th AST scan note. Memory:fake-driven-byte-identity-blind-spot.mdcaptures the QG lesson.Quality Gate
Code-review ×4 + CodeRabbit + Copilot. 2 MAJORS + 4 MINORS fixed:
EstimateUnknownModelError.default_remediationenumerated only Anthropic SKUs — refreshed to all 7.AnthropicProvider.estimate_input_tokensdroppedsystem=kwarg (fake snapshot couldn't catch real-API call-shape drift) — extended ABC withsystem: str = ""; provider re-threads.system=once, artifact envelope in user content. CHANGELOG documents the calibration shift.anthropic_client→clientrename on theestimate(...)engine signature.CONTRIBUTING.mdgated-marker audit missingsnowflake.FakeNoCacheProviderword-count proxy boundary-undercount;test_openai_client_confinement.pynon-recursive glob;_AttributeCallFinderlate-import alias bypass.Validation
uv sync --dev && uv run ruff check . && uv run ruff format --check . && uv run pyright && uv run pytest✓ (2498 passed, 62 deselected, 97.34% coverage)uv run pytest -m wheel_smoke --no-cov✓uv run pytest -m openai --no-covrequiresSF_RUN_OPENAI=1+OPENAI_API_KEY.Cross-epic coordination (#137 Gemini)
#137 sentinel
bd_1-scaffolding-41a("#136 OpenAI grading PR #152 merged to dev") gates #137 US-007 (GeminiProvider.estimate_input_tokens+--estimateintegration). When this PR merges todev: runbd close bd_1-scaffolding-41aand #137's US-007 unblocks for the next/ralph-run.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
signalforge.yml(llm.providerandgrade.provider).--estimateflag.[openai]installation extra with required dependencies.Bug Fixes
--estimategrader-side rubric token count double-counting issue.estimate()CLI parameter from Anthropic-specific to provider-agnostic.Documentation