Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
ae974a5
#137: Gemini grading provider (plan)
wjduenow May 28, 2026
b28c0b5
#137: extend plan after comparison with #136 OpenAI plan
wjduenow May 28, 2026
dbfff41
#137: apply 3 plan refinements from #136 cross-review
wjduenow May 28, 2026
50167a2
#137: devolve plan to beads (phase=devolved)
wjduenow May 28, 2026
b459360
#137: adjust plan now that #136 is being implemented first
wjduenow May 28, 2026
660d74b
bd_1-scaffolding-txe.6: #137 US-006 — add Gemini pricing SKUs (gemini…
wjduenow May 28, 2026
dc5e010
Merge bead bd_1-scaffolding-txe.6: #137 US-006 Gemini pricing SKUs
wjduenow May 28, 2026
8ed9aed
bd_1-scaffolding-txe.1: #137 US-001 — _gemini_client.py shim + new AS…
wjduenow May 28, 2026
40db0aa
Merge bead bd_1-scaffolding-txe.1: #137 US-001 _gemini_client.py shim…
wjduenow May 28, 2026
43c70d1
bd_1-scaffolding-txe.3: #137 US-003 — pyproject.toml [gemini] extra +…
wjduenow May 28, 2026
6634bde
Merge bead bd_1-scaffolding-txe.3: #137 US-003 pyproject [gemini] ext…
wjduenow May 28, 2026
b6bb73d
bd_1-scaffolding-txe.2: #137 US-002 — GeminiProvider(LLMProvider) + r…
wjduenow May 28, 2026
f40e110
Merge bead bd_1-scaffolding-txe.2: #137 US-002 GeminiProvider + regis…
wjduenow May 28, 2026
1f731d3
bd_1-scaffolding-txe.9: #137 US-009 — operator-facing docs + CHANGELO…
wjduenow May 28, 2026
2ce09b6
Merge bead bd_1-scaffolding-txe.9: #137 US-009 docs + CHANGELOG
wjduenow May 28, 2026
9367194
bd_1-scaffolding-txe.4: #137 US-004 — FakeGeminiClient + offline prov…
wjduenow May 28, 2026
0e25898
Merge bead bd_1-scaffolding-txe.4: #137 US-004 FakeGeminiClient + int…
wjduenow May 28, 2026
a3e4b16
bd_1-scaffolding-txe.5: #137 US-005 — provider-neutrality e2e tests (…
wjduenow May 28, 2026
abde849
Merge bead bd_1-scaffolding-txe.5: #137 US-005 provider-neutrality e2…
wjduenow May 28, 2026
f45d166
bd_1-scaffolding-txe.8: #137 US-008 — gemini marker + live tests (raw…
wjduenow May 28, 2026
3ec024f
Merge bead bd_1-scaffolding-txe.8: #137 US-008 live tests + marker + …
wjduenow May 28, 2026
6dd86d1
#137: harden DEC-019 cross-epic gate via bd sentinel bead
wjduenow May 28, 2026
a1c8c0f
Merge dev (#153 README refresh) into feature/137-gemini-grading
wjduenow May 28, 2026
b5a226f
Merge dev (#136 OpenAI grading) into feature/137-gemini-grading
wjduenow May 28, 2026
098c1f6
bd_1-scaffolding-txe.7: #137 US-007 — GeminiProvider.estimate_input_t…
wjduenow May 28, 2026
ac2f4c3
Merge bead bd_1-scaffolding-txe.7: #137 US-007 GeminiProvider.estimat…
wjduenow May 28, 2026
f15a469
bd_1-scaffolding-txe.10: #137 Quality Gate — fix bugs from 4-reviewer…
wjduenow May 28, 2026
430566c
bd_1-scaffolding-txe.11: #137 Patterns & Memory — durable rules for G…
wjduenow May 28, 2026
00dc673
#137: Address PR review feedback (Copilot + CodeRabbit)
wjduenow May 28, 2026
cbcfe4b
#137: Address second-round PR review (Copilot, 4 new comments)
wjduenow May 28, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .claude/rules/grade-layer.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ The grade layer sits between the prune engine (#6) and the diff renderer (#8). I

- **Scored:** `score: float ∈ [0.0, 1.0]` + `passed: bool`. The judge ran, the response parsed, the anchor contract held.
- **Degraded:** `score: None, passed: False, evidence: "", reasoning: "<failure reason>"`. Three causes route here:
1. `LLMError` retries exhausted → `reasoning="call failed: GradeLLMError"`.
1. `LLMError` retries exhausted → `reasoning="call failed: GradeLLMError"`. **Also covers a provider-specific safety-filter / no-content response** (Gemini's `finish_reason ∈ {SAFETY, RECITATION, OTHER, ...}` with empty parts is the v0.3 example — `GeminiProvider.extract_text_blocks` raises a typed `LLMResponseFormatError` per DEC-005 of #137, which propagates as an `LLMError` and lands here). The contract is provider-neutral: a future vendor with a content-filter surface MUST route through `LLMResponseFormatError` so the conservative degrade fires uniformly — `grade-artifacts` does NOT switch on provider name.
2. `GradeOutputError` (parser failure / anchor-contract failure) → `reasoning="call failed: GradeOutputError"`.
3. `total_budget_seconds` exceeded → `reasoning="grade budget exceeded ..."`.

Expand Down
22 changes: 18 additions & 4 deletions .claude/rules/llm-drafter.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ Every `# pyright: ignore[...]` and `# type: ignore[...]` comment for the Anthrop

**Gate the cache marker on BOTH capability flags, not just `supports_prompt_caching` (#135 QG lesson).** `call_llm` sets `cache_marker_active = supports_prompt_caching AND supports_token_count`. The pre-send count gate is what enforces the sub-minimum drop + the 8000-token oversize cap; attaching a `cache_control` marker without that gate having run would send an *unvalidated* marker (a sub-minimum block silently no-ops the marker — paying the input premium with no discount; an oversize block bypasses `LLMCacheTooLargeError`). Anthropic is `True/True` so the default path is unaffected, but a future provider that caches yet has no token-count API (`True/False`) must degrade to no-caching rather than send an unguarded marker. A new provider's capability flags are load-bearing — set them honestly, and don't assume "supports caching" alone is sufficient to attach a marker.

When a new vendor lands (#137 Gemini next), add a `_<vendor>_client.py` shim + a `LLMProvider` subclass + `register_provider`; don't pool SDK ignores into a generic util module, and don't reach into `call_llm` — extend via the strategy.
When a new vendor lands, add a `_<vendor>_client.py` shim + a `LLMProvider` subclass + `register_provider`; don't pool SDK ignores into a generic util module, and don't reach into `call_llm` — extend via the strategy. The pattern is now well-trodden: Anthropic (#135 baseline), OpenAI (#136), Gemini (#137) — each landed as a self-contained slice without touching `call_llm`.

### OpenAI provider shape (#136 — the second concrete provider, the no-cache precedent)

Expand All @@ -41,6 +41,19 @@ The `--estimate` path generalised in US-005 via `LLMProvider.estimate_input_toke

A new provider added under #137+ MUST implement `estimate_input_tokens` (it's `@abstractmethod`; missing impls fail at instantiation time). The implementation can ignore `system` (FakeNoCacheProvider stub concatenates for word-count; `_DummyProvider` returns `0`); but if the vendor's API distinguishes system from user tokens (like Anthropic), thread it separately or the same drift surfaces.

### Gemini provider shape (#137 — the third concrete provider, no-cache via a namespace-package SDK)

`GeminiProvider` ships under `provider="gemini"` for both stages; the shim at `src/signalforge/llm/_gemini_client.py` confines every `# pyright: ignore` / `# type: ignore` for the `google-genai` SDK. Both capability flags are `False` (mirrors OpenAI for the orchestrator path; v0.3 deliberately ships without Anthropic-style prompt caching — explicit Gemini context caching is a tracked follow-up). Four load-bearing #137 patterns the next vendor can mirror:

1. **`.messages.create` façade adapter — applies even when the SDK doesn't ship a `.messages` namespace at all.** The orchestrator hard-calls `llm_client.messages.create(**kwargs)`, but `google-genai`'s native surface is `client.models.generate_content(...)`. `_GeminiClientAdapter.messages` is a `_GeminiMessagesAdapter` instance whose `.create(**kwargs)` forwards to `models.generate_content(**kwargs)` and whose `.count_tokens(**kwargs)` forwards to `models.count_tokens(**kwargs)`. The pattern generalises #136's adapter shape to any vendor whose SDK exposes a differently-named namespace; the orchestrator stays vendor-neutral by construction.
2. **Server-side JSON via `response_mime_type="application/json"` (DEC-018 of #137).** `GeminiProvider.build_create_kwargs` attaches it inside the `config=...` dict that `models.generate_content` accepts. Mirrors OpenAI's `response_format={"type":"json_object"}` exactly per the precedent above — server-side enforcement eliminates the prose-preamble drift class; the tolerant `extract_json_payload` parser remains the fallback.
3. **Safety-filter / no-content → typed `LLMResponseFormatError` (DEC-005 of #137).** Gemini blocks responses via `finish_reason ∈ {SAFETY, RECITATION, OTHER, MAX_TOKENS, ...}` and surfaces a candidate with no text parts. `GeminiProvider.extract_text_blocks` walks `response.candidates[*].content.parts[*].text`; when no candidate yields a non-empty text part it raises `LLMResponseFormatError(f"Gemini response produced no text (finish_reason={fr!r}).")` with the first candidate's reason name. `call_llm` propagates as an `LLMError`; `grade_artifacts` wraps to `GradeLLMError` → conservative degrade per `grade-layer.md`. **The typed degrade is the contract — a future vendor with an analogous safety surface (e.g. content-filter responses) MUST route through the same `LLMResponseFormatError` → typed-degrade path rather than returning empty text or a sentinel.** Don't add a Gemini-specific error class — the existing `LLMError` hierarchy already covers it (Open notes for implementation in `plans/super/137-gemini-grading.md`).
4. **Native `models.count_tokens` for `estimate_input_tokens` (DEC-016 of #137).** Distinct from OpenAI's local `tiktoken` path: Gemini has a first-party server-side count API, so `GeminiProvider.estimate_input_tokens(model, text, *, system="", client=None)` calls `client.models.count_tokens(model=model, contents=[system + text])` and reads `response.total_tokens`. One extra API round-trip per estimate (comparable in shape to the Anthropic path). `system + text` is concatenated into a single `contents` entry — Gemini's count endpoint doesn't distinguish a system envelope from regular tokens (unlike Anthropic), so every token contributes to the same total and the figure matches what `generate_content` will bill at runtime. **Don't reach for `tiktoken` for a vendor that ships its own count API.** Verify the response field name against the installed SDK version — `total_tokens` is correct for `google-genai>=0.5,<1`; a future SDK rev may rename it.

### Namespace-package SDKs (#137 generalisation)

`google-genai` ships as a namespace-package — `from google import genai` rather than `import genai`. The AST confinement scan (Scan 10) handles this via the new `_AttributeCallFinder(parent_module="google")` parameter — that adds detection for four namespace-package shapes (bare `from google import genai; genai.Client(...)`, alias `from google import genai as g`, dotted-from `from google.genai import Client; Client(...)`, dotted-import-as `import google.genai as g; g.Client(...)`) on top of the four no-parent patterns the OpenAI scan covers. Pinned by `test_attribute_call_finder_catches_all_namespace_package_bypass_patterns_for_gemini` per `testing-signal.md`. **A future namespace-package SDK MUST pass its parent module name to the finder; the no-parent code path won't catch the dotted-from shape.**

## Module-level `_sleep` / `_rand_uniform` aliases (DEC-004)

`signalforge.llm.client` declares `_sleep = time.sleep` and `_rand_uniform = random.uniform` at module scope so tests can reassign to deterministic stand-ins without monkey-patching `time.sleep` globally (which would break pytest's own timeouts and any other concurrent test).
Expand Down Expand Up @@ -117,15 +130,16 @@ Same rule as the other layers (`safety-layer.md` DEC-022 / `prune-engine.md` DEC

## AST audit-completeness scans (DEC-013)

`tests/test_audit_completeness.py` runs five AST scans relevant to the LLM seam:
`tests/test_audit_completeness.py` runs six AST scans relevant to the LLM seam:

- `LLMRequest` constructed only in `signalforge.safety.request` (existing from #4).
- `AuditEvent` constructed only in `signalforge.safety.request`.
- `anthropic.Anthropic(...)` constructed only in `signalforge.llm._anthropic_client` (DEC-012 — the SDK seam; module renamed from `_client` by #135).
- `LLMResponseEvent` constructed only in `signalforge.draft.audit` — every event flows through `_build_response_event`.
- **`openai.OpenAI(...)` constructed only in `signalforge.llm._openai_client`** (the 9th project AST scan, #136 US-001/DEC-010). Mirrors Scan 3 shape exactly — reuses `_AttributeCallFinder`, catches all three bypass patterns (bare via `from openai import OpenAI`, import-alias via `from openai import OpenAI as O`, module-attribute via `import openai; openai.OpenAI(...)`).
- **`genai.Client(...)` constructed only in `signalforge.llm._gemini_client`** (the 10th project AST scan, #137 US-001/DEC-009). The Gemini SDK ships as a namespace-package, so the scan instantiates `_AttributeCallFinder` with `parent_module="google"` — that parameter activates four additional detection branches for the namespace-package shapes (see § "Namespace-package SDKs" above for the full enumeration).

When a new vendor lands (#137 Gemini bumps the tally to 10 for `genai.Client(...)`), add a **new** scan rather than extending an existing one — Scan 3 is Anthropic-specific (it hunts `anthropic.Anthropic`), so the OpenAI / Gemini scans are siblings, not extensions. The companion per-vendor `tests/llm/test_<vendor>_client_confinement.py` mirrors the Snowflake-shaped line-based scan for `# type: ignore` / `# pyright: ignore` confinement.
When a new vendor lands, add a **new** scan rather than extending an existing one — Scan 3 is Anthropic-specific (it hunts `anthropic.Anthropic`), so OpenAI / Gemini / future scans are siblings, not extensions. The companion per-vendor `tests/llm/test_<vendor>_client_confinement.py` mirrors the Snowflake-shaped line-based scan for `# type: ignore` / `# pyright: ignore` confinement.

If a new module legitimately needs to construct one of these gated names, update the scan's exclusion list AND document the audit-write seam. Don't suppress the test.

Expand All @@ -135,4 +149,4 @@ The drafter's config block is `{ llm: { provider, model, cheap_model, max_output

## Reference

`plans/super/5-llm-draft-pipeline.md` — DEC-001 … DEC-027. `plans/super/135-provider-neutral-llm-seam.md` — DEC-001 … DEC-012 (the provider-neutral seam: `call_llm`, `LLMProvider` ABC + registry, capability flags, `provider` config field). `plans/super/136-openai-grading-provider.md` — DEC-001 … DEC-014 (OpenAI as the second concrete provider: shim + `OpenAIProvider` + four pricing SKUs + `--estimate` strategy refactor + JSON-mode enforcement; the no-cache real-vendor precedent for #137 Gemini). `src/signalforge/llm/` (incl. `providers.py` + `_anthropic_client.py` + `_openai_client.py`), `src/signalforge/draft/` — current implementation. `tests/llm/_fake.py::FakeAnthropicClient` + `tests/llm/_fake_openai.py::FakeOpenAIClient` — `expect_*` API; `tests/llm/_fake_provider.py::FakeNoCacheProvider` + `tests/grade/test_provider_neutrality.py` + `tests/grade/test_provider_neutrality_openai.py` — the no-cache provider-neutrality proofs (synthetic + real OpenAI). `docs/draft-ops.md` / `docs/grade-ops.md` / `docs/cost-estimate-ops.md` — operational references. `tests/fixtures/draft/llm_response_*.json` / `tests/fixtures/estimate/anthropic_byte_identity_golden.txt` — fixture sets exercising happy + each error path + the DEC-013 Anthropic byte-identity floor.
`plans/super/5-llm-draft-pipeline.md` — DEC-001 … DEC-027. `plans/super/135-provider-neutral-llm-seam.md` — DEC-001 … DEC-012 (the provider-neutral seam: `call_llm`, `LLMProvider` ABC + registry, capability flags, `provider` config field). `plans/super/136-openai-grading-provider.md` — DEC-001 … DEC-014 (OpenAI as the second concrete provider: shim + `OpenAIProvider` + four pricing SKUs + `--estimate` strategy refactor + JSON-mode enforcement). `plans/super/137-gemini-grading.md` — DEC-001 … DEC-019 (Gemini as the third concrete provider: shim + `GeminiProvider` + `.messages`-over-`.models.generate_content` adapter + three pricing SKUs + native `models.count_tokens` for `--estimate` + safety-filter typed degrade + namespace-package AST confinement). `src/signalforge/llm/` (incl. `providers.py` + `_anthropic_client.py` + `_openai_client.py` + `_gemini_client.py`), `src/signalforge/draft/` — current implementation. `tests/llm/_fake.py::FakeAnthropicClient` + `tests/llm/_fake_openai.py::FakeOpenAIClient` + `tests/llm/_fake_gemini.py::FakeGeminiClient` — `expect_*` API; `tests/llm/_fake_provider.py::FakeNoCacheProvider` + `tests/grade/test_provider_neutrality.py` + `tests/grade/test_provider_neutrality_openai.py` + `tests/grade/test_gemini_neutrality.py` + `tests/draft/test_gemini_neutrality.py` — the no-cache provider-neutrality proofs (synthetic + real OpenAI + real Gemini). `docs/draft-ops.md` / `docs/grade-ops.md` / `docs/cost-estimate-ops.md` — operational references. `tests/fixtures/draft/llm_response_*.json` / `tests/fixtures/estimate/anthropic_byte_identity_golden.txt` — fixture sets exercising happy + each error path + the DEC-013 Anthropic byte-identity floor.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ All notable changes to SignalForge are documented here. The format is loosely ba
### Added

- **OpenAI as a grading + drafting provider (#136).** Set `grade.provider: openai` or `llm.provider: openai` in `signalforge.yml`; requires the `[openai]` install extra and `OPENAI_API_KEY`. Ships four pricing SKUs (`gpt-4o`, `gpt-4o-mini`, `gpt-4.1`, `gpt-4-turbo`); `--estimate` works via tiktoken (no extra API round-trip). Server-side JSON enforcement via `response_format={"type": "json_object"}`. v0.3 ships without prompt caching (no Anthropic-style cache discount); follow-up to evaluate OpenAI prompt caching.
- **Google Gemini as a grading + drafting provider (#137).** Set `grade.provider: gemini` or `llm.provider: gemini` in `signalforge.yml`; requires the `[gemini]` install extra (`pip install signalforge-dbt[gemini]`) and `GOOGLE_API_KEY`. Recommended SKU for both drafter and judge is `gemini-2.5-flash` (also registered: `gemini-2.5-pro`, `gemini-2.0-flash`). Server-side JSON enforcement via `response_mime_type="application/json"`. `--estimate` cost-preview is wired through Gemini's native `client.models.count_tokens` (US-007 of #137; DEC-016) — first-party token counter, one extra API round-trip per estimate, comparable to the Anthropic shape. Ships **without prompt caching** in v0.3 — `LLMProvider` strategy reports `supports_prompt_caching=False` / `supports_token_count=False`, so `call_llm` skips the `cache_control` marker, the `extended-cache-ttl` beta header, and the pre-send `count_tokens` gate; budget per-call cost accordingly under `provider: gemini` (especially for the grader's 4-criterion × ~12-artifact fan-out). Explicit Gemini context caching is tracked as a follow-up.

### Fixed

Expand Down
31 changes: 26 additions & 5 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ uv run ruff check . && uv run ruff format --check . && uv run pyright && uv run

The default `pytest` run — and therefore the coverage badge — measures only the
default marker set. Tests gated behind `bigquery`, `anthropic`, `openai`,
`cli_subprocess`, `e2e`, `snowflake`, and `wheel_smoke` are filtered out by
`addopts` (see `.claude/rules/testing-signal.md` § "Known gap: excluded
`gemini`, `cli_subprocess`, `e2e`, `snowflake`, and `wheel_smoke` are filtered
out by `addopts` (see `.claude/rules/testing-signal.md` § "Known gap: excluded
markers"), so the real-network and packaging paths are not instrumented in the
badge number.

Expand All @@ -57,14 +57,35 @@ uv run pytest
# --cov-append combines with run 1 so the term report shows the COMBINED total.
# --cov-fail-under=0 overrides the 80% gate inherited from addopts — gated
# markers alone never clear it, and this is a measurement, not a gate.
# (bigquery/anthropic/openai/snowflake/e2e need creds; cli_subprocess/wheel_smoke do not.)
# (bigquery/anthropic/openai/gemini/snowflake/e2e need creds; cli_subprocess/wheel_smoke do not.)
SF_RUN_BQ=1 ANTHROPIC_API_KEY=sk-... OPENAI_API_KEY=sk-... \
SF_RUN_OPENAI=1 SF_RUN_SNOWFLAKE=1 GOOGLE_CLOUD_PROJECT=<billing-project> \
SF_RUN_OPENAI=1 SF_RUN_SNOWFLAKE=1 SF_RUN_GEMINI=1 GOOGLE_API_KEY=... \
GOOGLE_CLOUD_PROJECT=<billing-project> \
SNOWFLAKE_ACCOUNT=... SNOWFLAKE_USER=... SNOWFLAKE_PASSWORD=... SNOWFLAKE_WAREHOUSE=... \
uv run pytest -m 'bigquery or anthropic or openai or snowflake or e2e or cli_subprocess or wheel_smoke' \
uv run pytest -m 'bigquery or anthropic or openai or gemini or snowflake or e2e or cli_subprocess or wheel_smoke' \
--cov=signalforge --cov-append --cov-fail-under=0 --cov-report=term
```

## Gemini live smoke

Three tests gated by `@pytest.mark.gemini` exercise the Gemini provider
end-to-end against the real Google Gemini API:

- `tests/llm/test_gemini_live.py` — raw `call_llm(provider="gemini", ...)` round-trip.
- `tests/draft/test_gemini_draft_live.py` — `draft_schema` against a small in-test manifest fixture.
- `tests/grade/test_gemini_grade_live.py` — `grade_artifacts` 1-criterion × 1-artifact.

All three are deselected from default CI (`addopts -m 'not gemini'`) and
additionally self-skip via a runtime gate if either env var is missing:

```bash
SF_RUN_GEMINI=1 GOOGLE_API_KEY=... uv run pytest -m gemini --no-cov
```

Recommended SKU is `gemini-2.5-flash` (cheapest of the three registered
SKUs); per-call cost is dominated by the no-caching posture (DEC-013 of
#137) so each rubric criterion ships the full system + rubric prompt.

The combined total from step 2 minus the default badge number from step 1 is
the coverage the gated paths add — typically 5–10%. Interpreting the delta: if
the default badge number drops by M% but the combined total holds steady, that
Expand Down
Loading
Loading