chore: begin 0.6.0.dev0 + backmerge main into dev (post-0.5.0) - #174
Conversation
* 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…
* #141: SignalForge skill + install-skill (plan) Phase 4 detailing complete. 24 decisions locked across: - skill source path (`src/signalforge/skills/signalforge/` package-data tree; `src/signalforge/skill/` Python lib) - destination policy (always overwrite SKILL.md, preserve siblings, no --force) - symlink/cycle defence (mirrors copy_demo verbatim) - error hierarchy (SkillError base + 3 concretes; spans tiers 1+2 → excluded base) - wheel packaging + wheel_smoke gate - AST scan #7 bump (12→13) - SKILL ↔ CLI parity gate (new test scans live argparse + key demo commands) - 5-surface parity for install-skill - self-grade ops (pre-release manual; pinned in eval.json + README badge) - e2e demo paths (zero-cred default + opt-in live) - skill-parity.md rule + cli-layer.md update (orchestrator-only) 11 stories laid out: US-001…US-009 implementation + US-010 Quality Gate + US-011 Patterns & Memory. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #141: mark plan phase=published, link PR #166 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(#141): add skill-parity rule documenting CLI/skill parity gate Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #141: devolved to beads (epic bd_1-scaffolding-ezn, 11 tasks) Approved + devolved. US-001 is at the front of the ready queue. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ezn.1: US-001 bootstrap skills tree + wheel packaging Lays down src/signalforge/skills/signalforge/{SKILL.md,assets/SKILL.eval.json} as structural placeholders. Wires [tool.hatch.build.targets.wheel].include for the skills tree. Extends wheel_smoke with _EXPECTED_SKILL_FILES (positive) and a negative assertion that no .claude/skills/* paths appear in the wheel. Plan: plans/super/141-claude-skill-install.md US-001 / DEC-001, 010, 011, 022. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ezn.2: US-002 signalforge.skill lib + typed errors + AST scan #7 bump Adds signalforge.skill subpackage with install_skill(dest) -> Path and the three-class typed-error hierarchy (SkillDestPathError tier 1, SkillDestUnsafeError tier 2, SkillPackageDataMissingError tier 1). Mirrors signalforge.demo.copy_demo verbatim for symlink-cycle defence and importlib.resources lookup; never rmtree. Registers the three lib concretes in _EXCEPTION_TO_EXIT_CODE; adds SkillError to _EXCEPTION_MAPPING_EXCLUDED_BASES; bumps AST scan #7 count 12 -> 13. Plan: plans/super/141-claude-skill-install.md US-002 / DEC-002, 003, 005, 006, 007, 008, 009. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ezn.3: US-003 CLI install-skill subcommand + handler + exit-code mapping + subprocess smoke Adds 'signalforge install-skill [<dest>]' wired via add_parser/cmd_install_skill mirroring init_demo shape. Three CliInstallSkill*Error wrappers (Path/DestUnsafe/ PackageDataMissing) registered in _EXCEPTION_TO_EXIT_CODE (tier 1/2/1). Stdout success line + (replaced existing SKILL.md) on overwrite per DEC-017. Subprocess --help smoke under cli_subprocess marker. Per-class construction branches added to test_exit_codes.py for the three new wrappers. Plan: plans/super/141-claude-skill-install.md US-003 / DEC-002, 003, 004, 008, 009, 017, 024. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ezn.7: US-007 author SKILL.md prose (frontmatter + 7 body sections) Replaces the US-001 placeholder with the real user-facing workflow per DEC-020 (frontmatter) and DEC-021 (seven sections). Body teaches the full pipeline: point-at-project, zero-cred demo, real draft+prune, prune-existing, diff-reading, gated live e2e, troubleshooting. Contains every canonical token the US-004 parity gate scans for (subcommand names + demo command lines + install-skill bootstrap). Plan: plans/super/141-claude-skill-install.md US-007 / DEC-012, 013, 020, 021. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ezn.4: US-004 SKILL ↔ CLI parity gate NEW tests/cli/test_skill_cli_parity.py scans src/signalforge/skills/signalforge/ SKILL.md for three token categories per DEC-015: 1. Every subcommand name from the live argparse parser (auto-grows) 2. Four canonical demo command lines (signalforge init-demo / generate <model> --write / prune-existing <model> --schema <path> / install-skill) 3. The install-skill bootstrap line (covered by category 2's fourth entry) Plain substring match; no normalisation. Planted-violation self-check proves the gate can fail loud — per testing-signal.md AST-source-scan-gate philosophy. The gate runs inside the canonical VALIDATE_CMD (uv run pytest) so /ralph-run keeps the skill current automatically without relying on the model remembering. Plan: plans/super/141-claude-skill-install.md US-004 / DEC-015, 016, 019. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ezn.9: US-009 skill-parity.md + cli-layer.md update Replaces the anticipatory skill-parity.md with the post-implementation contract: names the actual shipped artefacts (src/signalforge/skill/, src/signalforge/skills/signalforge/SKILL.md, signalforge install-skill, tests/cli/test_skill_cli_parity.py), documents the two-name convention (skills/ plural for package-data vs skill/ singular for the Python lib), the planted-violation self-check, and the wheel exclusion defence for maintainer-only .claude/skills/. cli-layer.md § Multi-surface parity gains a paragraph naming the bundled skill as the 6th parity surface, cross-linking to skill-parity.md and the test_skill_cli_parity.py gate. ORCHESTRATOR-ONLY commit per ralph-worker-claude-dir-perms memory — workers cannot Write under .claude/ in worktrees. Plan: plans/super/141-claude-skill-install.md US-009 / DEC-018, 019. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ezn.6: US-006 docs (skills.md + mkdocs nav + cli-ops + README pointer) NEW docs/skills.md catalog page describing the bundled SignalForge skill, the install-skill subcommand, the two demo paths (zero-cred default + opt-in live e2e), the parity gate, and the maintainer-only-skill exclusion. mkdocs.yml nav gains 'Claude Code Skill' entry. docs/cli-ops.md gains the install-skill subcommand entry with stderr shapes + exit codes. README Quick start gains a one-sentence pointer after pip install. clauditor badge intentionally NOT added here — US-008 owns that surface. Plan: plans/super/141-claude-skill-install.md US-006 / DEC-021, 023. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ezn.5: US-005 5-surface parity test for install-skill Mirrors tests/cli/test_5_surface_parity_init_demo.py for the install-skill subcommand. v0.1 canonical token: 'install-skill' (no flags). Pins the token across argparse help, handler docstring, docs/cli-ops.md, plan document, and the test docstring itself. Orthogonal to test_skill_cli_parity.py (US-004): that scans the FULL CLI surface against ONE skill body; this pins ONE subcommand across FIVE surfaces. Plan: plans/super/141-claude-skill-install.md US-005 / DEC-024. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ezn.8: US-008 clauditor self-grade + README badge Adds clauditor-eval to [dependency-groups].dev (PyPI dist name; provides the 'clauditor' CLI entry point — the upstream LLM-as-judge framework SignalForge's own grading layer shares its methodology with). Grade pending — clauditor-eval installs cleanly and 'uv run clauditor grade' runs, but a meaningful score requires a maintainer-crafted EvalSpec (SignalForge-specific assertions + grading criteria); the auto-scaffolded 'clauditor init' template is generic boilerplate that would grade noise-against-noise. Refined assets/SKILL.eval.json to a 'pending-first-grade' shape that pins the current signalforge.__version__, names the grader and regen command, and explains why the maintainer must hand-tune an EvalSpec before the first real grade. README shields.io badge surfaces the pending state ('clauditor: pending', lightgrey). New 'Self-grade' section in docs/skills.md documents the regen flow for the maintainer's pre-release workflow per DEC-014. VALIDATE_CMD green: 2745 passed, all four checks. wheel_smoke + cli_subprocess gated markers also green. Plan: plans/super/141-claude-skill-install.md US-008 / DEC-014. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bd_1-scaffolding-ezn.10: Quality Gate — fix bugs from code review Four code-review passes surfaced 8 real findings; this commit addresses them. CORRECTNESS - src/signalforge/__init__.py: revert __version__ from 0.4.0.dev0 back to 0.5.0.dev0 — the dev branch is the 0.5.0 development line; the previous value was a stale-rebase artifact (Review 1). - src/signalforge/skill/__init__.py: extend symlink defence (DEC-005) to cover every install-tree ancestor (.claude/, .claude/skills/, .claude/skills/signalforge/) — not just SKILL.md itself. A symlinked ancestor dir would otherwise smuggle writes through copytree (Review 1). - tests/skill/test_install.py: new test_install_skill_refuses_when_install_dir_ancestor_is_symlink pins the ancestor-symlink defence with a concrete attacker-elsewhere repro. SKILL.md PROSE (drift between skill and live CLI surface) - Remove the "--force" example (DEC-003 explicitly forbids the flag; re-running install-skill overwrites SKILL.md by default and preserves siblings). - Frontmatter signalforge-version: "0.X.Y" → "0.5.0.dev0" (matches __version__; was a literal placeholder shipped to operators). - Section 2: "no warehouse, no API keys, no dbt profile" was misleading — the drafter always calls Anthropic. Reword to "no dbt profile / no warehouse credentials of your own" + name the ANTHROPIC_API_KEY requirement. - LLMCacheTooLargeError mis-labelled tier-3 → corrected to tier-2 (the cache-too-large check is a pre-LLM-call input-validation gate per cli-layer.md's four-tier taxonomy). docs/skills.md DRIFT - Canonical-commands list said "signalforge --version" — DEC-015's hardcoded list is init-demo / generate <model> --write / prune-existing <model> --schema <path> / install-skill (no --version variant; the parity gate scans for `version` subcommand separately via category 1). - "signalforge --version is the first thing it runs" → "signalforge version (the subcommand)" — flag vs subcommand mismatch. PARITY GATE EXTENSION (the gate would have caught the --force bug) - tests/cli/test_skill_cli_parity.py: add fourth category that scans SKILL.md for `signalforge <subcommand> --<flag>` patterns and asserts each flag exists on the live subparser. Pinned with the same planted-violation philosophy (verified manually: reinstating --force fails the gate loud). Closes the "skill prose teaches a flag that doesn't exist" failure mode the original gate could not catch. Validation: - uv run ruff check . — clean - uv run ruff format --check . — clean - uv run pyright — 0 errors - uv run pytest — 2747 passed (was 2745 baseline; +2 new tests) - uv run pytest -m wheel_smoke --no-cov — 5/5 - uv run pytest -m cli_subprocess --no-cov — 8/8 Plan: plans/super/141-claude-skill-install.md US-010. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #141: plan phase=implemented; record Ralph run completion + QG findings Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #141: close codecov patch-coverage gaps (5 lines, 3 files → 100%) codecov flagged 5 missed lines in the PR diff: - src/signalforge/cli/install_skill.py:177-178 — except OSError around the existed_before probe (silently downgrades on probe failure; the install should still proceed) - src/signalforge/skill/__init__.py:150 — resolve(strict=False) fallback for a dest that doesn't exist yet (the common fresh-project case) - src/signalforge/skill/__init__.py:157 — non-ELOOP OSError raw re-raise (narrow ELOOP-only routing is load-bearing — a PermissionError must NOT be mis-attributed as a symlink cycle) - src/signalforge/skill/errors.py:66 — SkillError.__str__'s no-footer branch (when neither remediation kwarg nor default_remediation is set) Per the qg-pass-3-defer-defensive-tests-fails-codecov memory, codecov holds patch coverage to project standard regardless of "is this a real bug today" — defensive branches need test coverage even when they're fallbacks. Adds 4 tests: - test_skill_error_str_omits_footer_when_remediation_is_none (errors.py:66) - test_install_skill_propagates_non_eloop_oserror_unchanged (skill:157) - test_install_skill_resolves_nonexistent_dest_via_strict_false_fallback (skill:150) - test_install_skill_handles_oserror_in_existed_before_probe (cli:177-178) Coverage: install_skill.py 95% → 100%, skill/__init__.py 95% → 100%, skill/errors.py 95% → 100%. Full pytest: 2751 passed (was 2747). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * #141: address CodeRabbit + Copilot PR review (13 threads) Trivial: - assets/SKILL.eval.json: version 0.4.0.dev0 → 0.5.0.dev0 (matches the __version__ revert in the previous QG commit; flagged by both reviewers). - SKILL.md fenced ASCII pipeline diagram: add `text` language tag (MD040). - plans/super/141: drop the leftover duplicate "_Pending Phase 3/4._" placeholders at the end of the doc. Substantive — symlink defence broadened to EVERY bundled path: - src/signalforge/skill/__init__.py: the symlink defence used to cover only `<dest>/.claude/skills/signalforge/SKILL.md`. A symlinked `assets/SKILL.eval.json` (or symlinked `assets/` directory) would smuggle writes through copytree. Now enumerates every relative path under the bundled source tree via `rglob` and refuses to overwrite any of them through a symlink. Pinned by `test_install_skill_refuses_when_assets_dir_is_symlink`. - Also wrap `mkdir(parents=True)` in try/except NotADirectoryError → raise SkillDestUnsafeError so a non-dir component along the install chain (e.g. `<dest>/.claude` is a regular file) yields a typed remediation-bearing message instead of a raw OSError. Pinned by `test_install_skill_wraps_notadirectoryerror_from_mkdir_chain`. Substantive — CLI existed_before probe: - src/signalforge/cli/install_skill.py: probe was `.exists()` which follows symlinks AND returns False for broken symlinks. The DEC-017 contract says "True for files and symlinks (both shapes are replaced from the operator's POV)" — a broken symlink is a third shape that the probe silently downgrades. Probe now OR's `.is_symlink()` to catch the broken-symlink case (semantics stay honest even though the lib seam then raises SkillDestUnsafeError on the same path). Substantive — SKILL.md ADC contradiction: - The frontmatter `compatibility:` field and Section 2 body claimed "zero-credential demo (no warehouse needed)" while simultaneously describing the demo as sampling a public BigQuery dataset under ADC (which requires `gcloud auth application-default login` + GOOGLE_CLOUD_PROJECT). Both surfaces rewritten to honestly describe the demo's real posture: removes the dbt-project setup cost, but needs ANTHROPIC_API_KEY + ADC + GOOGLE_CLOUD_PROJECT. Section 2 also surfaces `signalforge lint --model <name>` as the truly-offline fallback (manifest-only, no LLM, no warehouse). Substantive — dbt parse invocation: - SKILL.md Section 1 suggested running `dbt parse` but the `allowed-tools` frontmatter does NOT include `Bash(dbt *)`. Reworded to "ask the user to run dbt parse themselves" rather than implying the skill runs it — preserves the narrow tool grant. Substantive — parity gate (`test_skill_cli_parity.py`): - Broaden `_SKILL_FLAG_USAGE_RE` to match `signalforge <subcommand> [<positional> ...] --<flag>` so canonical shapes like `generate <model> --write` and `prune-existing <model> --schema <path>` are no longer skipped (CodeRabbit + Copilot). Constrain to same-line `[ \t]` (not `\s`) so the match cannot span newlines — without this, prose like "signalforge installed (pip install ...)" plus "signalforge lint --model" two paragraphs later yields a spurious `installed --model` capture (caught during validation). - Unknown-subcommand branch now FAILS instead of skipping (Copilot). A typo like `signalforge instal-skill --force` would previously skip silently; now it surfaces in the assertion message. - Verified via planted-violation: `signalforge generate <model> --xyzbogus` injection trips the gate; restoring SKILL.md returns the gate to green. Validation: - uv run ruff check . — clean - uv run ruff format --check . — clean - uv run pyright — 0 errors - uv run pytest — 2753 passed (was 2751; +2 new tests) - uv run pytest -m wheel_smoke --no-cov — 5/5 - Patch coverage: install_skill.py 100%, skill/__init__.py 100%, skill/errors.py 100% — codecov-clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cuts v0.5.0 to PyPI. Bumps version 0.4.0 → 0.5.0 and adds a new [0.5.0] CHANGELOG section honestly scoped to #141 (the SignalForge Claude Code skill + install-skill subcommand). Also bumps the SKILL.md frontmatter and assets/SKILL.eval.json version strings 0.5.0.dev0 → 0.5.0 so the shipped skill metadata matches the release. Branch shape: release/0.5.0 is based on main + cherry-pick of 709f147 (#141's squash-merge commit on dev). Main's content-equivalence to dev means the cherry-pick reproduces dev's tree modulo this commit; no intervening dev work is lost. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chore: release 0.5.0
Combined post-release housekeeping per the "dev ahead of main" pattern: 1. Bump `__version__` 0.5.0.dev0 → 0.6.0.dev0 2. Bump skill metadata version strings (SKILL.md frontmatter + assets/ SKILL.eval.json) to match 3. Backmerge origin/main (v0.5.0) into dev so dev = main + future work 4. Clear dev's stale `[Unreleased]` CHANGELOG carryover from v0.4.0 by taking main's post-release CHANGELOG shape verbatim (empty placeholder + correct version-descending order) The CHANGELOG cleanup is the load-bearing fix that prevents the v0.4.0 → v0.5.0 release-PR conflict from repeating: dev's `[Unreleased]` had been carrying the v0.4.0 entries since the v0.4.0 release didn't clear them, which then duplicated into the proposed v0.5.0 release notes via the release skill's CHANGELOG promotion. Taking main's CHANGELOG resets dev to the correct empty-placeholder shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughVersion 0.5.0 is released with changelog documentation for features and fixes, and package version constants are advanced to 0.6.0.dev0 across Python source, skill metadata, and skill configuration files. ChangesRelease and version advancement
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Combined post-0.5.0 housekeeping per the documented "dev ahead of main" pattern (one PR instead of separate next-dev-bump + backmerge).
What this does:
__version__0.5.0.dev0 → 0.6.0.dev0Why the CHANGELOG cleanup is load-bearing:
The release-PR conflict on #172 was caused by dev's
[Unreleased]carrying the v0.4.0 entries since the v0.4.0 release didn't clear them — when the release-manager skill promoted[Unreleased]→[0.5.0], it duplicated the v0.4.0 content into the v0.5.0 release notes. Then the merge into main conflicted because main's[0.5.0]and dev's[Unreleased]both contained the same bullets in different positions.Taking main's CHANGELOG resets dev to the correct empty-placeholder shape, so the next v0.6.0 release cycle starts clean: bullets accumulate under
[Unreleased]on dev, the release skill promotes them to[0.6.0]honestly, no carryover ambiguity.Validation:
uv sync --dev && uv run ruff check . && uv run ruff format --check . && uv run pyright && uv run pytest— all PASS (2753 passed, 97.75% coverage).🤖 Generated with Claude Code
Summary by CodeRabbit