Skip to content

37: cli --select for multi-model batch + cookbook - #66

Merged
wjduenow merged 21 commits into
devfrom
feature/37-multi-model-select
May 11, 2026
Merged

37: cli --select for multi-model batch + cookbook#66
wjduenow merged 21 commits into
devfrom
feature/37-multi-model-select

Conversation

@wjduenow

@wjduenow wjduenow commented May 11, 2026

Copy link
Copy Markdown
Owner

Summary

Super plan for #37--select flag for multi-model batch + docs/cli-ops.md cookbook.

Phase: detailing (awaiting approval)
Stories: 8 implementation + Quality Gate + Patterns & Memory
Decisions: 17 (DEC-001 … DEC-017)

Highlights

  • Grammar (DEC-001): comma-separated union of tag:<name>, path:<glob> (fnmatch), or bare value (model./path). Subset of dbt's selector grammar; intersections / exclusions / graph operators deferred to v0.3.
  • Mutex (DEC-002): positional <model> and --select are an add_mutually_exclusive_group(required=True). Backward compat preserved for single-model invocations.
  • Sidecars (DEC-003): .signalforge/grade.json / diff.json last-writer-wins; documented in --help and the cookbook. Append-only JSONLs survive iteration. Operator escape hatch: shell-loop pattern.
  • Errors (DEC-004): continue on per-model failure; exit code = max(per_model_exit_codes) across the four-tier taxonomy. Failed models named in stderr summary (cap 50, then … and K more).
  • Summary (DEC-005): stderr-only; stdout stays clean for tooling. Always emits when ≥2 matched or ≥1 failed.
  • Adapter lifecycle (DEC-010): fresh BigQueryAdapter per model — avoids _active_session_id / _session_started_at bleed across iterations.
  • Cost (DEC-008): no batch-level cap in v0.2. Per-call caps (maximum_bytes_billed, draft retries) + cookbook caveat. cli.max_models_per_run reserved for v0.3.
  • 5-surface parity (DEC-017): new bespoke test asserts argparse help ↔ docs/cli-ops.md ↔ this plan's DEC text reference the same example selectors.

Plan document

See plans/super/37-multi-model-select.md for the full plan: discovery, architecture review (security/perf/data/API/observability/testing), refinement log, and the 10-story breakdown with TDD lists.

Next steps

  • Review the plan in this PR
  • Reply approved in Claude Code to proceed to devolve (create beads tasks)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added --select <expr> flag for multi-model batch execution with dbt-style selector syntax (tag:, path: glob, and bare value matching)
    • Per-model progress indicators for batch runs
    • Aggregated batch summary reporting with failure listing and exit code aggregation across models
    • Continue-on-failure behavior; batch execution proceeds through failures and reports summary

Review Change Stack

17 decisions, 10 stories. Phase A (docs cookbook) + Phase B (--select flag)
ship together. Selector grammar is comma-separated union of tag:/path:/bare
atoms; mutex with positional <model>. Continue-on-failure with max() exit
code across the four-tier taxonomy. Sidecars (grade.json/diff.json) are
last-writer-wins; documented escape hatch is the shell-loop cookbook.
Fresh BigQueryAdapter per model to avoid session-id bleed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 11, 2026

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 82483578-d255-4155-aec8-88793981ce2c

📥 Commits

Reviewing files that changed from the base of the PR and between f7654ab and 396a3a8.

📒 Files selected for processing (29)
  • .claude/rules/cli-layer.md
  • .claude/rules/manifest-readers.md
  • CLAUDE.md
  • README.md
  • docs/cli-ops.md
  • plans/super/37-multi-model-select.md
  • src/signalforge/cli/__init__.py
  • src/signalforge/cli/_helpers.py
  • src/signalforge/cli/errors.py
  • src/signalforge/cli/generate.py
  • src/signalforge/manifest/__init__.py
  • src/signalforge/manifest/errors.py
  • src/signalforge/manifest/select.py
  • tests/cli/test_5_surface_parity_select.py
  • tests/cli/test_batch_emission.py
  • tests/cli/test_exit_codes.py
  • tests/cli/test_format_error_to_stderr.py
  • tests/cli/test_generate_batch.py
  • tests/cli/test_generate_select_flag.py
  • tests/cli/test_select_integration.py
  • tests/fixtures/dbt_project_multi/dbt_project.yml
  • tests/fixtures/dbt_project_multi/models/marts/fct_x.sql
  • tests/fixtures/dbt_project_multi/models/staging/stg_a.sql
  • tests/fixtures/dbt_project_multi/models/staging/stg_b.sql
  • tests/fixtures/dbt_project_multi/regenerate.sh
  • tests/fixtures/dbt_project_multi/signalforge.yml
  • tests/fixtures/dbt_project_multi/target/manifest.json
  • tests/manifest/test_multi_fixture_loads.py
  • tests/manifest/test_select.py

📝 Walkthrough

Walkthrough

This PR implements multi-model batch selection for signalforge generate via a new --select <expr> CLI flag (issue #37). It adds a manifest-layer selector module with typed atoms (tag/path/bare), CLI error wrappers for parse/no-match failures, refactors the generator into single-model and batch pipelines with aggregated exit codes, and includes comprehensive tests and documentation.

Changes

Multi-Model Batch Selection & Execution

Layer / File(s) Summary
Planning & Documentation
plans/super/37-multi-model-select.md, .claude/rules/cli-layer.md, .claude/rules/manifest-readers.md, CLAUDE.md, docs/cli-ops.md, README.md
Epic plan with v0.2 scope, discovery, decisions (DEC-001 through DEC-017), and stories (US-001 through US-010); batch driver rules and sidecar semantics; selector grammar rules and determinism contracts; v0.2 API documentation; user-facing cookbook for in-process --select and shell-loop multi-model runs; troubleshooting reflowed links.
Manifest Selector Module
src/signalforge/manifest/select.py
Immutable Pydantic atom types (TagAtom, PathAtom, BareAtom) with discriminated kind field; parse_selector() splitting and validating comma-separated expressions; select_models() matching against manifest models (tag membership including config.tags, fnmatch glob, unique_id/path routing) with deduplication and lexicographic unique_id ordering.
Manifest Selector Error & Exports
src/signalforge/manifest/errors.py, src/signalforge/manifest/__init__.py
New SelectorParseError(ManifestError) raised on invalid input; re-exports SelectorAtom, parse_selector, select_models, and SelectorParseError.
CLI Selector Errors & Re-Exports
src/signalforge/cli/errors.py, src/signalforge/cli/__init__.py
New CliSelectorParseError(CliInputError) wrapping manifest parse failures with optional cause; new CliSelectorNoMatchError(CliInputError) for zero-match selectors; both re-exported from signalforge.cli.
CLI Helpers: Exit-Code Mapping & Batch Formatting
src/signalforge/cli/_helpers.py
Extended exit-code mapping to register selector errors as tier-2 (exit code 2); added format_batch_summary() for multi-line stderr summary with capped (50-item) failure list and control-char sanitization; added emit_batch_progress_entry() for per-model progress prefix; updated __all__ exports.
Generate Argparse & Outcome Dataclasses
src/signalforge/cli/generate.py
Refactored add_parser() with mutually-exclusive required group (positional <model> or --select <expr>); frozen dataclasses _SingleModelOutcome and _BatchOutcome capturing exit codes, rendered text, exception metadata, counts, and durations.
Single-Model Pipeline Core
src/signalforge/cli/generate.py (_run_single_model)
Extracted pipeline logic: constructs fresh adapter per invocation, runs safety→draft→prune→grade→diff, handles --estimate short-circuit, normalizes output with trailing newline, maps exceptions at boundary, gates batch progress prefix, returns outcome with results and exception metadata.
Batch Driver
src/signalforge/cli/generate.py (_run_batch)
Multi-model orchestration: parses/validates selector expression, wraps manifest errors to CLI errors, raises CliSelectorNoMatchError on zero matches, iterates matched models with fresh adapter per model, aggregates exit code via max(), conditionally emits batch summary to stderr.
Generate Dispatcher
src/signalforge/cli/generate.py (cmd_generate)
Refactored as dispatcher: performs pre-pipeline setup, routes to _run_batch() when args.select is set or _run_single_model() otherwise, streams output to stdout, returns aggregated exit code.
Test Fixture: DBT Project & Manifest
tests/fixtures/dbt_project_multi/
Three-model dbt fixture (stg_a, stg_b staging; fct_x marts) with dbt_project.yml, model SQL, regenerate.sh script (hermetic manifest scrubbing via jq), committed manifest.json, and signalforge.yml config (aggregate-only safety, prune disabled).
Unit Tests: Selector, Errors, Batch, Fixture
tests/manifest/test_select.py, tests/cli/test_format_error_to_stderr.py, tests/cli/test_exit_codes.py, tests/cli/test_batch_emission.py, tests/cli/test_generate_batch.py, tests/manifest/test_multi_fixture_loads.py
Selector grammar parsing/matching, error rendering/exit-codes, batch summary formatting/emission gating, per-model progress prefix, batch driver core (adapter-per-model, failure continuation, error wrapping), fixture loading/tag distribution.
Unit Tests: Argparse Wiring & Integration
tests/cli/test_generate_select_flag.py, tests/cli/test_select_integration.py
Select flag argparse wiring (mutex/required enforcement, routing), end-to-end CLI behavior (selector semantics, progress/summary emission, partial failure, sidecar persistence).
Parity Tests: Documentation Consistency
tests/cli/test_5_surface_parity_select.py
Ensures selector examples appear consistently across argparse help, plan markdown, and docs cookbook section.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related issues

Possibly related PRs

  • wjduenow/SignalForge#26: Both PRs extend the CLI error hierarchy and exit-code mappings in src/signalforge/cli/ for new error types.
  • wjduenow/SignalForge#64: Both PRs modify src/signalforge/cli/generate.py cmd_generate pipeline—this PR refactors it into dispatcher + single-model + batch, while PR #64 adds stage logging.
  • wjduenow/SignalForge#31: Both PRs modify CLI command wiring and exception-to-exit-code mappings.

Poem

🐰 A rabbit hops through selector trees,

With tag: and path: upon the breeze,

Batch models run side by side,

Fresh adapters keep no secrets to hide,

Exit codes dance—max leads the way,

Multi-model magic saves the day! ✨

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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

@codecov-commenter

codecov-commenter commented May 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.04306% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/signalforge/cli/generate.py 97.95% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

wjduenow and others added 19 commits May 11, 2026 11:35
Mark phase devolved, record beads epic + task IDs, capture dependency graph.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…S-006)

Hand-crafted three-model dbt fixture for issue #37's --select integration
tests. Mirrors tests/fixtures/dbt_project_austin/ shape:

  - dbt_project.yml + signalforge.yml (safety.mode: aggregate-only,
    prune.enabled: false so the US-007 integration tests run offline)
  - models/staging/stg_a.sql with engineered determinism: a 'austin' AS
    source literal column mathematically guarantees a not_null test on
    that column always-passes, exercising the prune engine's drop path
  - models/staging/stg_b.sql with varied column shape, also tagged
    staging
  - models/marts/fct_x.sql refs both staging models, tagged marts
  - target/manifest.json hand-crafted, validates via
    signalforge.manifest.load()
  - regenerate.sh mirrors austin's, documents maintainer-only dbt parse

Paired loads test in tests/manifest/test_multi_fixture_loads.py pins
the three unique_ids, the engineered always-passes column on stg_a,
and the tag distribution (2 staging + 1 marts) so the US-007 selector
integration tests have a stable target.

Traces to DEC-013 in plans/super/37-multi-model-select.md.
Adds the typed selector grammar for issue #37's --select flag:

- `SelectorAtom` discriminated union (`TagAtom` / `PathAtom` / `BareAtom`),
  Pydantic v2 frozen with `extra="forbid"` so user-input typos fail loud.
- `parse_selector(expr) -> tuple[SelectorAtom, ...]` — splits on comma,
  strips whitespace, classifies each atom; rejects empty atoms / empty
  payloads with `SelectorParseError(ManifestError)`.
- `select_models(manifest, expr) -> tuple[Model, ...]` — unions atoms,
  dedupes by `unique_id`, sorts by `unique_id`. Tag match: union of
  `Model.tags` and `Model.config.tags`. Path match: `fnmatch.fnmatchcase`
  against `Model.original_file_path` (DEC-016). Bare: `model.` prefix
  routes to `unique_id`; otherwise exact match on `original_file_path`.

Re-exported from `signalforge.manifest`: `parse_selector`, `select_models`,
`SelectorAtom`, `SelectorParseError`. `SelectorParseError` registered in
the CLI exit-code mapping table at tier 2 (input-validation; 7th AST scan
enforces).

Traces to DEC-001 (grammar), DEC-012 (module location), DEC-016 (fnmatch)
of `plans/super/37-multi-model-select.md`.
…pping (US-002)

Adds the two CLI-layer wrapper errors that US-004's cmd_generate dispatcher
will raise around the manifest-layer selector helpers (US-001):

* CliSelectorParseError(CliInputError) — wraps SelectorParseError from
  signalforge.manifest.select.parse_selector. Accepts expr: str and
  cause: Exception | None; message includes cause text when supplied.
* CliSelectorNoMatchError(CliInputError) — raised when a well-formed
  selector resolves to zero models. Mirrors ModelNotFoundError's tier
  (the bare positional <model> equivalent — input-validation).

Both registered in signalforge.cli._helpers._EXCEPTION_TO_EXIT_CODE at
tier 2 (input-validation) per DEC-007 of plans/super/37-multi-model-select.md.
The 7th AST scan in tests/test_audit_completeness.py walks every
src/signalforge/*/errors.py and gates the registration — adding the
classes without the mapping entries would break it loud. Both new
classes also re-exported from signalforge.cli for public-surface
alignment with the other CLI errors.

Tests:
* tests/cli/test_format_error_to_stderr.py (new) — stderr shape contract
  pinning ERROR: <message> + ↳ Remediation: <text> for both new classes,
  cause-text inclusion, and the per-class tier-2 mapping assertions.
* tests/cli/test_exit_codes.py — extends _construct_exception with the
  expr-kwarg constructor shapes so the parametrized exit-code loop
  covers both new classes.

US-004 will wire try/except SelectorParseError as e: raise
CliSelectorParseError(expr=..., cause=e) in cmd_generate. This bead
only adds the classes and registers them.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…+ _run_batch (US-003)

Extract the per-model pipeline body from cmd_generate into a private
_run_single_model(model, manifest, profile, args, *, project_dir,
batch_index=None, batch_count=None) -> _SingleModelOutcome helper, and
add a _run_batch(manifest, profile, args, *, project_dir) -> _BatchOutcome
driver that iterates manifest.select_models matches and calls
_run_single_model per match with a FRESH WarehouseAdapter per iteration
(DEC-010 of plans/super/37-multi-model-select.md — avoids
_active_session_id bleed across in-process iterations).

cmd_generate is now a thin dispatcher: args.select set routes to
_run_batch; else _run_single_model once. This bead does NOT modify
argparse — US-004 wires the --select flag onto the seam. The single-model
observable behaviour is unchanged byte-for-byte: every existing test under
tests/cli/test_generate.py (46 tests, including the load-bearing
test_generate_calls_stages_in_documented_order) passes unchanged.

New tests pin the contracts US-003 introduces:

* test_single_model_path_unchanged_post_refactor — re-affirms the
  positional <model> branch runs end-to-end and prints the rendered
  marker, with the adapter factory called exactly once.
* test_batch_driver_fresh_adapter_per_model — patches _make_warehouse_adapter
  to count calls; tag:staging matches 2 of 3 models in the synthetic
  multi-manifest; asserts call_count == 2 (DEC-010).
* test_batch_driver_continues_after_per_model_failure — first model raises
  LLMRateLimitError (exit 3); other two succeed; total_exit_code == 3.
* test_batch_driver_zero_match_raises_cli_selector_no_match —
  tag:nonexistent → exit 2 via CliSelectorNoMatchError BEFORE any
  iteration (no stage entry is called).
* test_run_batch_zero_match_raises_directly — library-level pinning of the
  raise behaviour (not via the dispatcher's catch).
* test_batch_driver_parse_error_raises_cli_selector_parse_error —
  --select tag: → exit 2 via CliSelectorParseError; no traceback.
* test_run_batch_parse_error_chains_cause — library-level: SelectorParseError
  rides on __cause__ (DEC-007).
* test_run_batch_returns_batch_outcome_with_per_model_entries — pins the
  unique_id-sorted ordering + total_exit_code aggregation.
* test_single_model_outcome_failure_carries_exception_class_name — pins
  exception_class_name == type(exc).__name__ on failure.

_run_single_model carries its own try/except boundary (mirrors cli-layer.md
DEC-016 per-handler boundary catch) and writes the formatted error to
stderr internally; cmd_generate's outer try only catches errors from the
pre-pipeline scaffolding (project-root resolution, manifest load, profile
load, --min-score range check, selector parse / no-match raised by
_run_batch). This preserves stderr ordering in batch mode: each model's
error follows its own progress lines, not a coalesced dump at end-of-batch.

The _SingleModelOutcome and _BatchOutcome dataclasses are frozen and
carry the fields US-005 will read for the [i/N] progress prefix and the
aggregated summary line.

Validation: ruff check . && ruff format --check . && pyright && pytest
(1514 passed, 13 deselected, coverage 95.51%) plus pytest -m cli_subprocess
(1 passed).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tional model (US-004)

Puts positional `<model>` and `--select <expr>` into a
`mutually_exclusive_group(required=True)` on the `generate` subparser so
argparse enforces exactly-one-of at parse time. The dispatcher in
`cmd_generate` (US-003) already reads `args.select`; this bead supplies
the field.

`--select` help text pins the grammar (DEC-001), three examples
(DEC-001), and the sidecar-overwrite caveat (DEC-016) verbatim per the
5-surface parity rule. The positional uses `nargs="?"` so it can be
absent when `--select` is supplied.

Six tests in `tests/cli/test_generate_select_flag.py`:

* `test_positional_model_alone_works_unchanged` — backward compat.
* `test_select_flag_alone_routes_to_batch_driver` — `--select tag:staging`
  reaches the batch handler, runs against two matches, exits 0.
* `test_positional_and_select_mutex_argparse_error` — both supplied →
  exit 2 with `not allowed with` argparse wording.
* `test_neither_positional_nor_select_argparse_error` — neither →
  exit 2 with `required` argparse wording.
* `test_select_parse_failure_returns_exit_2_with_cli_selector_parse_error`
  — `--select tag:` → `CliSelectorParseError` shape, exit 2.
* `test_select_zero_match_returns_exit_2_with_cli_selector_no_match`
  — `--select tag:nonexistent_tag_xyz` → `CliSelectorNoMatchError`
  shape, exit 2.

Traces to DEC-001, DEC-002, DEC-016 of `plans/super/37-multi-model-select.md`.

Validation: `ruff check`, `ruff format --check`, `pyright`, and the
default `pytest` set (1520 passed, 13 deselected) all clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… (US-005)

Two new CLI helpers and their emission wiring in the multi-model batch
driver:

* ``format_batch_summary(outcome)`` — pure formatter for the DEC-005
  stderr summary. Headline always; failure block when ≥1 model failed;
  failure list capped at 50 with ``  ... and <K> more`` overflow line
  (DEC-009).
* ``emit_batch_progress_entry(unique_id, i, N)`` — ``[i/N] <unique_id>``
  stderr line emitted at the head of each ``_run_single_model``
  invocation INSIDE ``_run_batch`` (DEC-014).

Emission rules (DEC-005, DEC-014):

* Summary always emits on stderr when (≥2 models matched OR ≥1 failed),
  suppressed by ``--quiet``. Operator-actionable signal even in CI; the
  failure list is the load-bearing surface for diagnosing batch runs.
* Per-model prefix TTY-gated via ``should_emit_progress`` —
  ``--quiet`` suppresses, ``--verbose`` forces, non-TTY default off.
* Single-model positional path emits NEITHER (batch kwargs default to
  ``None``; the gate fires only when both are non-``None``).

16 new tests in ``tests/cli/test_batch_emission.py`` pin the formatter
shape, emission paths, failure-list cap, single-model preservation, and
TTY / quiet / verbose gating.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
End-to-end tests exercising the full batch driver against the
tests/fixtures/dbt_project_multi/ fixture via in-process main([...]).
LLM seam and warehouse adapter stubbed via stage-entry monkeypatches —
no env vars, no live network. Traces to DEC-001, DEC-002, DEC-004,
DEC-005, DEC-006, DEC-007, DEC-009, DEC-017.

tests/cli/test_select_integration.py (13 tests):

* test_select_tag_routes_to_batch — tag:staging → 2 staging models,
  unique_id-sorted on stdout.
* test_select_path_glob — path:models/staging/* via fnmatch (DEC-016).
* test_select_multi_expression_union — tag:staging,tag:marts → all
  three, deduped + sorted.
* test_select_bare_unique_id_routes_to_batch_with_single_match —
  bare model.<...> through batch driver; single-match suppresses
  summary (DEC-005).
* test_positional_model_still_works — backward-compat, no [i/N], no
  summary.
* test_positional_and_select_mutex_argparse_error_exits_2 —
  argparse mutex error (DEC-002).
* test_select_zero_match_exits_2_with_cli_selector_no_match_error —
  CliSelectorNoMatchError stderr shape (DEC-006).
* test_select_parse_failure_exits_2_with_cli_selector_parse_error —
  CliSelectorParseError stderr shape (DEC-007).
* test_batch_partial_failure_collects_max_exit_code — first model
  raises LLMRateLimitError; other two succeed; exit=3; summary names
  (LLMRateLimitError) (DEC-004 / DEC-009).
* test_batch_summary_shape_to_stderr — DEC-005 locked headline regex.
* test_batch_progress_prefix_emits_under_tty — [1/2]/[2/2] under
  monkeypatched isatty (DEC-014).
* test_batch_quiet_suppresses_progress_but_emits_summary_on_failure —
  --quiet suppresses [i/N]; per-model ERROR lines still surface.
* test_sidecar_last_writer_wins_across_batch — 3-model batch; only
  the alphabetically-last model's .signalforge/{diff,grade}.json
  persist (DEC-003).

tests/cli/test_5_surface_parity_select.py (1 test):

* test_5_surface_parity_for_select_flag — reads argparse help +
  plan DECs and asserts the three example selectors appear
  consistently. Cookbook surface (docs/cli-ops.md) gated by
  pytest.skipif until US-008 merges the cookbook section; the skip
  lifts automatically once the sentinel string lands.

Each test that invokes main(...) copies the committed fixture into
tmp_path via shutil.copytree so .signalforge/ artefacts land per-test
(testing-signal.md DEC-008 of #10). Stage-entry stubs mirror the
make_*_for(model) per-model variation pattern so each model's typed
result carries its own model_unique_id — load-bearing for the
sidecar last-writer-wins test.

All 14 tests pass; 1 skip (parity surface 2 awaiting US-008); full
suite 1549 passed. ruff + pyright clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…asses)

Pass 1: cmd_generate dispatcher used truthiness on `select_expr`; an
empty-string `--select ""` (argparse-accepted as "provided" under the
mutex) fell through to the single-model branch where `args.model is
None`. Fixed to `is not None` + regression test
`test_select_empty_string_routes_to_parse_error`.

Pass 1: 5-surface parity test carried dead `pytest.skip` branches for
the cookbook (US-008 has merged). Converted to hard asserts; removed
unused `pytest` import.

Pass 2: `format_batch_summary` failure-bullet rendering did not
sanitise control chars in `model_unique_id`. Real dbt unique_ids never
contain `\n`/`\r`/`\t`, but the summary is a CI-parser-keyable surface
— defence in depth replaces controls with spaces before column-padding.

Pass 3: docs/cli-ops.md cookbook claimed Anthropic prompt cache
amortises across iterations within one process. Inaccurate: the
explicitly cache-marked block is the per-model manifest summary, which
changes each iteration. Only the static system prompt benefits from
Anthropic's automatic caching. Rewrote the bullet.

Pass 4: added regression test
`test_format_batch_summary_sanitises_control_chars_in_unique_id`
pinning the pass-3 sanitiser so it can't silently regress.

Validation: ruff + ruff format + pyright clean; 1552 default-marker
tests pass (95.54% coverage); cli_subprocess marker green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…US-010)

`.claude/rules/cli-layer.md` — new `## Multi-model batch driver pattern (issue
#37, v0.2)` section covering: dispatcher / driver shape, `is not None` over
truthiness on `select_expr`, fresh adapter per model (cross-link to
warehouse-adapters.md DEC-002-of-#22 generalisation), continue-on-failure with
max() aggregation, stderr summary format, control-char sanitisation in
failure-bullet ids, per-model `[i/N]` progress prefix TTY-gating, Anthropic
prompt-cache honest accounting (system-prompt auto-cache only; per-model marked
block does NOT amortise), sidecar last-writer-wins, 5-surface parity test
pattern, two new tier-2 errors (no new AST scan needed).

`.claude/rules/manifest-readers.md` — new `## User-facing string-grammar
selectors (issue #37)` section covering: `parse_<grammar>` / `select_<entity>`
module pattern, atom discriminated union with `extra="forbid"`, whitespace
handling contract (strip-around-atoms, preserve-inside-payloads), multi-
expression union semantics, bare-value disambiguation, drift-detector
non-applicability for user-input typed atoms.

`CLAUDE.md` — extended `## Public API surface (v0.1 + v0.2 additions)` with
the six new symbols (`parse_selector`, `select_models`, `SelectorAtom`,
`SelectorParseError`, `CliSelectorParseError`, `CliSelectorNoMatchError`)
plus the `signalforge generate --select <expr>` flag bullet.

Validation: ruff + ruff format + pyright clean; 1552 default-marker tests
pass (95.54% coverage). Docs-only commit; no source-code changes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@wjduenow wjduenow changed the title 37: cli --select for multi-model batch + cookbook (plan) 37: cli --select for multi-model batch + cookbook May 11, 2026
@wjduenow
wjduenow marked this pull request as ready for review May 11, 2026 19:41
@wjduenow
wjduenow requested a review from Copilot May 11, 2026 19:48
Picks up #36 (signalforge generate --estimate cost preview) shipped to dev.

Conflict resolution in src/signalforge/cli/generate.py:
- The conflicting region was a docstring inside _run_single_model (HEAD)
  vs cmd_generate (origin/dev). cmd_generate's docstring already carries
  the full --estimate / --scope / --sample-strategy precedence rules at
  the post-#37-refactor location (line ~1043), so the origin/dev chunk
  was duplicated. Kept HEAD's _SingleModelOutcome / _BatchOutcome
  declarations + _run_single_model's docstring; appended a one-line
  cross-reference to cmd_generate for the inherited overrides.

- Merge breakage: origin/dev's --estimate short-circuit returned a bare
  `0` from inside _run_single_model, which post-#37 now expects a
  _SingleModelOutcome. Wrapped the return as
  _SingleModelOutcome(exit_code=0, rendered_text="", ...) so batch mode
  (--select) inherits the estimate short-circuit cleanly via the typed
  outcome aggregation.

Validation: ruff + ruff format + pyright clean; 1605 tests pass
(1552 from #37 + #36's additions), coverage 95.39%.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@wjduenow

Copy link
Copy Markdown
Owner Author

@CodeRabbit review

@coderabbitai

coderabbitai Bot commented May 11, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds multi-model execution to signalforge generate via a new --select flag (issue #37), including a manifest-layer selector grammar, a batch driver with per-model isolation, and extensive fixture + integration test coverage. This fits into the CLI orchestration layer by enabling sequential “batch” runs while keeping stdout clean for tooling and moving aggregated operational output to stderr.

Changes:

  • Introduce signalforge.manifest.select (parse_selector, select_models, SelectorAtom) plus SelectorParseError.
  • Refactor CLI generate into _run_single_model + _run_batch, wire --select as a required mutex with the positional <model>, and add batch summary/progress emission helpers.
  • Add a committed multi-model dbt fixture and end-to-end CLI tests (including 5-surface parity + cookbook docs updates).

Reviewed changes

Copilot reviewed 29 out of 29 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
src/signalforge/cli/generate.py Adds --select argparse wiring; refactors into single-model + batch driver with summary/progress emission.
src/signalforge/cli/_helpers.py Adds batch summary formatter and per-model batch progress emitter; extends exit-code mapping.
src/signalforge/cli/errors.py Adds typed CLI selector wrapper errors for parse/no-match.
src/signalforge/cli/__init__.py Re-exports new selector CLI errors.
src/signalforge/manifest/select.py New selector grammar/parser + matcher (tag:, path:, bare) returning deterministic model tuples.
src/signalforge/manifest/errors.py Adds SelectorParseError to manifest error hierarchy.
src/signalforge/manifest/__init__.py Re-exports selector public surface from the manifest package.
tests/manifest/test_select.py Unit tests for selector parsing/typing and matching semantics.
tests/manifest/test_multi_fixture_loads.py Smoke test ensuring the committed multi-model fixture manifest loads deterministically.
tests/cli/test_generate_batch.py Tests batch driver behavior (fresh adapter per model, continue-on-failure, exit-code aggregation).
tests/cli/test_generate_select_flag.py Tests argparse mutex wiring and --select routing/error shapes.
tests/cli/test_batch_emission.py Tests batch summary/progress prefix formatting and emission rules.
tests/cli/test_select_integration.py End-to-end integration tests running main() against the real multi-model fixture with patched stage seams.
tests/cli/test_format_error_to_stderr.py Pins stderr rendering for new selector CLI errors and exit-code table membership.
tests/cli/test_exit_codes.py Extends exit-code probes for new selector CLI errors.
tests/cli/test_5_surface_parity_select.py Enforces parity of example selectors across argparse help, docs, and plan text.
tests/fixtures/dbt_project_multi/dbt_project.yml New multi-model dbt fixture project definition (paths, tags, materializations).
tests/fixtures/dbt_project_multi/signalforge.yml Fixture SignalForge config for offline-safe integration tests.
tests/fixtures/dbt_project_multi/models/staging/stg_a.sql Fixture model with engineered determinism column.
tests/fixtures/dbt_project_multi/models/staging/stg_b.sql Fixture staging model with varied column shape.
tests/fixtures/dbt_project_multi/models/marts/fct_x.sql Fixture marts model referencing staging models.
tests/fixtures/dbt_project_multi/target/manifest.json Committed manifest for the multi-model fixture.
tests/fixtures/dbt_project_multi/regenerate.sh Maintainer script to regenerate/scrub the fixture manifest via dbt parse.
docs/cli-ops.md Adds --select reference and a “Running across many models” cookbook section.
README.md Links to the new multi-model cookbook section.
plans/super/37-multi-model-select.md Design plan/DEC record for the implementation.
CLAUDE.md Documents new v0.2 public APIs and CLI behavior for --select.
.claude/rules/manifest-readers.md Adds pattern guidance for user-facing selector grammars in the manifest layer.
.claude/rules/cli-layer.md Adds documented batch-driver pattern and parity guidance for CLI changes.

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

"tag:<name>, path:<glob> (shell-style fnmatch), "
"or a bare unique_id / file path. Examples: "
"tag:staging | path:models/marts/* | "
"tag:staging,path:models/marts/*. "
Comment on lines +12 to +20
section. **US-008 lands this surface in a separate PR** running in
parallel; until that PR merges, the section is absent. We
``pytest.skipif`` on a sentinel-string check so this test does NOT
spuriously fail on a merge race. Once US-008 ships the section, the
skip lifts automatically and the parity contract becomes active.
3. **plans/super/37-multi-model-select.md** — the DEC-001 / DEC-016
grammar block plus US-007's TDD bullets reference the same atoms.

The test reads bytes from each surface present at runtime and asserts
Comment thread docs/cli-ops.md
Comment on lines +664 to +670
- `path:<glob>` — shell-style `fnmatch` against
`Model.original_file_path`. dbt's own selector grammar uses a
path-prefix-with-implicit-wildcard convention; v0.2 deliberately
uses fnmatch instead, which means operators write
`path:models/staging/*` (with the trailing wildcard) rather than
`path:models/staging` (dbt-compat semantics are a v0.3 ask;
DEC-016).
Comment thread docs/cli-ops.md
Comment on lines +677 to +683
ordered lexicographically.

Three concrete examples:

```bash
signalforge generate --select tag:staging
signalforge generate --select path:models/marts/*
Comment on lines +202 to +205
**Continue-on-failure with `max()` aggregation** (DEC-004). Each `_run_single_model` lives inside its own `try/except Exception` boundary (mirrors DEC-016). A per-model failure records `exception_class_name`, exit-code (via `map_exception_to_exit_code`), and keeps going. The batch's `total_exit_code = max(per_model_exit_codes)` across the four-tier taxonomy (severity rank = tier integer). Failed models named in the aggregated summary with their tier + exception class; cap 50, overflow `... and <K> more`.

**Aggregated summary → stderr** (DEC-005). `format_batch_summary(outcome) -> str` in `cli/_helpers.py` is the single formatter. Headline format (locked verbatim, pinned by test):

Comment thread CLAUDE.md
- `signalforge.manifest.SelectorParseError` — `ManifestError` subclass raised on grammar violations (empty atoms, empty payloads). Tier 2 in the CLI exit-code mapping table.
- `signalforge.cli.CliSelectorParseError(CliInputError)` — CLI-layer wrapper raised by `cmd_generate`'s dispatcher when `SelectorParseError` fires inside `_run_batch`. Carries `expr` + optional `cause` kwargs; default remediation points operators at `signalforge generate --help`. Tier 2.
- `signalforge.cli.CliSelectorNoMatchError(CliInputError)` — raised by `_run_batch` BEFORE any model iteration when `select_models` returns an empty tuple. Message: `"--select <expr> matched zero models in this project"`. Tier 2.
- `signalforge generate --select <expr>` — new CLI flag, mutex with positional `<model>` (`add_mutually_exclusive_group(required=True)`). Triggers `_run_batch`: sequential per-model execution, fresh `BigQueryAdapter` per model (DEC-010 avoids `_active_session_id` bleed), continue-on-failure with `max(per_model_exit_codes)` aggregation across the four-tier taxonomy. Aggregated stderr summary on completion (`Generated <K> kept / <L> dropped / <J> flagged across <M> models in <T>s`); failed models named with their tier + exception class (cap 50, overflow `... and <K> more`). Per-model `[i/N] <unique_id>` progress prefix on stderr (TTY-gated; `--quiet` suppresses; `--verbose` forces on). Stdout carries rendered diffs in `unique_id` lex order. Sidecar caveat: `.signalforge/grade.json` and `.signalforge/diff.json` are last-writer-wins across iterations (only the final model's sidecars persist on disk); the four append-only JSONLs (`safety.audit.jsonl`, `llm_response.jsonl`, `prune.jsonl`, `grade.jsonl`) survive iteration. Documented in `docs/cli-ops.md` § Running across many models with shell-loop alternative for per-model sidecars.
- **Adapter state risk (CRITICAL):** `BigQueryAdapter._active_session_id` / `_session_started_at` / `_session_ttl_seconds` are per-instance, cleaned in `__exit__`. The current `cmd_generate` instantiates one adapter then enters/exits it inside `prune_tests`. For multi-model in-process iteration, we MUST construct a fresh adapter per model — reusing across iterations would leak session_id and elapsed-TTL state across models.
- **Audit/sidecar overwrite risk (CRITICAL):** every default path is project-shared, not per-model:
- `.signalforge/audit.jsonl` (safety) — append-only, safe.
- `.signalforge/llm_response.jsonl` (draft) — append-only, safe.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants