Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
e983891
37: super plan — cli --select for multi-model batch + cookbook (plan)
wjduenow May 11, 2026
e7a05e8
37: devolve plan to beads (epic bd_1-scaffolding-4v1, 10 tasks)
wjduenow May 11, 2026
fbb4b39
bd_1-scaffolding-4v1.6: Add multi-model fixture dbt_project_multi/ (U…
wjduenow May 11, 2026
f97ad4e
Merge bead bd_1-scaffolding-4v1.6: US-006 multi-model fixture
wjduenow May 11, 2026
d20eaba
bd_1-scaffolding-4v1.1: Add signalforge.manifest.select (US-001)
wjduenow May 11, 2026
1d8bb8b
Merge bead bd_1-scaffolding-4v1.1: US-001 manifest selector module
wjduenow May 11, 2026
a99a1a3
bd_1-scaffolding-4v1.2: Add CLI selector error classes + exit-code ma…
wjduenow May 11, 2026
29c59a4
Merge bead bd_1-scaffolding-4v1.2: US-002 CLI selector error classes
wjduenow May 11, 2026
64a4661
bd_1-scaffolding-4v1.3: Refactor cmd_generate into _run_single_model …
wjduenow May 11, 2026
a9224f0
Merge bead bd_1-scaffolding-4v1.3: US-003 cmd_generate refactor
wjduenow May 11, 2026
bfa5d1c
bd_1-scaffolding-4v1.4: Wire --select argparse flag + mutex with posi…
wjduenow May 11, 2026
46b3d13
Merge bead bd_1-scaffolding-4v1.4: US-004 --select argparse flag
wjduenow May 11, 2026
c3bfa4b
bd_1-scaffolding-4v1.5: Add batch summary + per-model progress prefix…
wjduenow May 11, 2026
a9eee97
Merge bead bd_1-scaffolding-4v1.5: US-005 batch summary + progress pr…
wjduenow May 11, 2026
7eec706
bd_1-scaffolding-4v1.8: Add multi-model cookbook + --select flag refe…
wjduenow May 11, 2026
c5e944d
bd_1-scaffolding-4v1.7: Add CLI integration tests for --select (US-007)
wjduenow May 11, 2026
7d38b7a
Merge bead bd_1-scaffolding-4v1.8: US-008 docs cookbook + --select re…
wjduenow May 11, 2026
9f592dd
Merge bead bd_1-scaffolding-4v1.7: US-007 CLI integration tests
wjduenow May 11, 2026
56d5809
bd_1-scaffolding-4v1.9: Quality gate — fix bugs from code review (4 p…
wjduenow May 11, 2026
34c345b
bd_1-scaffolding-4v1.10: Patterns & Memory — codify #37 conventions (…
wjduenow May 11, 2026
396a3a8
Merge origin/dev into feature/37-multi-model-select
wjduenow May 11, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 42 additions & 1 deletion .claude/rules/cli-layer.md
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,49 @@ The `--version` flag uses argparse's `action="version"` (which raises `SystemExi

When introducing a new subcommand, match the precedent verbatim. Don't add per-subcommand `try/except` ladders; the single boundary at `cmd_<name>` is the contract.

## Multi-model batch driver pattern (issue #37, v0.2)

Issue #37 lands `--select <expr>` for multi-model batch execution in one `signalforge generate` process. The dispatcher in `cmd_generate` routes to either the single-model path (positional `<model>`) or `_run_batch` (when `--select` is supplied). The two paths are mutex via `add_mutually_exclusive_group(required=True)` — argparse rejects both/neither at parser time, exit 2.

**Dispatcher / driver shape.** Three private helpers in `signalforge.cli.generate`:

- `_SingleModelOutcome` — frozen dataclass with `model_unique_id`, `exit_code`, kept/dropped/flagged counts, `rendered_text` (stdout content for this model), `duration_seconds`, `exception_class_name` (set on failure for the aggregated summary).
- `_BatchOutcome` — frozen dataclass with `per_model: tuple[_SingleModelOutcome, ...]` and `total_exit_code = max(...)` across the four-tier taxonomy.
- `_run_single_model(model, manifest, profile, args, *, project_dir, batch_index=None, batch_count=None) -> _SingleModelOutcome` — runs the full safety → draft → prune → grade → diff pipeline for one model. Constructs its OWN `BigQueryAdapter` via `_make_warehouse_adapter(profile)` so each call gets fresh adapter state. `batch_index` / `batch_count` drive the `[i/N] <unique_id>` progress prefix when both non-None.
- `_run_batch(manifest, profile, args, *, project_dir) -> _BatchOutcome` — calls `select_models(manifest, args.select)`, wraps `SelectorParseError → CliSelectorParseError(cause=...)`, raises `CliSelectorNoMatchError` BEFORE any iteration on empty match.

`cmd_generate` is a thin dispatcher: `if getattr(args, "select", None) is not None: _run_batch(...)` else `_run_single_model(...)`. **Use `is not None`, NOT truthiness** — an empty-string `--select ""` is argparse-accepted (the mutex group treats it as "provided") and MUST route to the parser so it raises `CliSelectorParseError`, not fall through to the single-model branch where `args.model is None`. Pinned by `test_select_empty_string_routes_to_parse_error`.

**Fresh adapter per model** (DEC-010 of #37, generalising `warehouse-adapters.md` DEC-002-of-#22). Stateful adapters carry per-call state in instance fields (`BigQueryAdapter._active_session_id` is the v0.2 instance; v0.3 Snowflake / Postgres will have their own). The batch driver constructs a new adapter inside the per-model loop, NOT once at batch start — otherwise state from model N would leak into model N+1's audit and warehouse session. Adds ~100-500ms BQ client init per model; acceptable vs. state-corruption risk.

**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 on lines +202 to +205
```
Generated <K> kept / <L> dropped / <J> flagged across <M> models in <T>s
```

Plus optional failure block when ≥1 model failed. Summary emits when `(matched ≥ 2 OR failed ≥ 1) AND NOT quiet`. Stdout carries rendered diffs in `unique_id` lex order; stderr carries the summary so operators piping `> diffs.txt` get just the diffs.

**Defence-in-depth: scrub control chars in failure-bullet ids.** The failure bullets are column-padded; a `\n` / `\r` / `\t` in any `model_unique_id` would corrupt the CI-parser-keyable column geometry. Real dbt unique_ids never contain control chars (Pydantic-strict-typed at manifest load), but `format_batch_summary` replaces them with single spaces before measuring + emitting. Pinned by `test_format_batch_summary_sanitises_control_chars_in_unique_id`.

**Per-model `[i/N]` progress prefix** (DEC-014). When the batch driver runs AND `_run_single_model` receives non-None `batch_index`/`batch_count` AND `should_emit_progress(quiet, verbose)` returns True, each iteration emits one stderr line `[i/N] <model_unique_id>` before the model's existing stage progress fires. `--quiet` suppresses; `--verbose` forces on regardless of TTY. Single-model positional path emits NEITHER the prefix NOR the summary — preserves v0.1 output shape byte-for-byte.

**Anthropic prompt cache behaviour in batch** (DEC-015). The drafter's *explicitly cache-marked* block is the per-model manifest summary (`<MODEL_SQL>` + neighbours), which changes each iteration — so the marked block does NOT amortise across siblings. Cost savings within one process come from Anthropic's *automatic* caching of the static system prompt only (once it crosses the auto-cache size threshold). Document this honestly in the operator-facing cookbook; the marked-cache claim looks attractive but doesn't materialise on batch runs.

**Sidecar last-writer-wins** (DEC-003). `.signalforge/grade.json` and `.signalforge/diff.json` are `O_TRUNC` overwrite per `cmd_generate` call (locked by `grade-layer.md` DEC-006/012 and `diff-renderer.md` DEC-009). Multi-model in-process iteration overwrites these per model; only the final model's sidecars persist. The four append-only JSONLs (`safety.audit.jsonl`, `llm_response.jsonl`, `prune.jsonl`, `grade.jsonl`) survive iteration because each record is ≤ 4000 bytes (well under `PIPE_BUF = 4096` on Linux) — POSIX guarantees atomic concurrent appends. Operators who want per-model sidecars use the shell-loop pattern (`docs/cli-ops.md § Running across many models`), one process per `--project-dir`.

**5-surface parity test pattern** (DEC-017). For any new CLI flag whose grammar / examples appear across multiple surfaces, ship a bespoke parity test that reads each surface and asserts the same example tokens appear. `tests/cli/test_5_surface_parity_select.py` is the issue-#37 instance — hard-asserts that `tag:staging`, `path:models/marts/*`, and `tag:staging,path:models/marts/*` appear in argparse help, the cookbook section of `docs/cli-ops.md`, and the plan file. When v0.3 flags ship, copy this test verbatim and re-target. **Don't ship the test with `pytest.skip` branches for surfaces that haven't landed yet** — those become dead code the moment the gating PR merges. Either ship the test gated by a sentinel string check that converts to a hard assert once the surface exists, OR ship the test AFTER all surfaces are committed.

**No new AST scan, no new fail-closed writer.** The batch layer does not introduce a new audit-event class (the per-model writers — safety / draft / prune / grade — already cover the contract; the batch driver just iterates). The 7th AST scan (`test_every_typed_error_is_in_exit_code_mapping_table`) auto-covers the two new errors `CliSelectorParseError` and `CliSelectorNoMatchError` because they live in `cli/errors.py`. Logger grep gate auto-covers new lazy-format calls in `cli/` (6th dir; unchanged).

**Two new exit-code-table entries.** Both `CliSelectorParseError` and `CliSelectorNoMatchError` are tier 2 (input-validation): the operator's selector was syntactically malformed OR resolved to nothing in this project. Mirrors `ModelNotFoundError`'s tier (the positional bare-name case).

When v0.3 introduces parallel batch execution (deferred from #37), the per-model boundary catch needs to coordinate with whatever concurrency primitive lands. The current sequential pattern lays the groundwork: outcome-as-typed-result + max-aggregation generalises cleanly.

## Reference

`plans/super/9-cli-entrypoint.md` — DEC-001 … DEC-027. `src/signalforge/cli/` — current implementation. `docs/cli-ops.md` — operational reference. `tests/cli/` — in-process and subprocess test suite. `tests/test_audit_completeness.py::test_every_typed_error_is_in_exit_code_mapping_table` — 7th AST scan (DEC-024). `tests/llm/test_logger_grep_gate.py` — lazy-format logger gate (6 dirs as of #9). `tests/cli/test_exit_codes.py` — parametrized exception → exit-code contract.
`plans/super/9-cli-entrypoint.md` — DEC-001 … DEC-027. `plans/super/37-multi-model-select.md` — DEC-001 … DEC-017 (multi-model batch additions). `src/signalforge/cli/` — current implementation. `docs/cli-ops.md` — operational reference. `tests/cli/` — in-process and subprocess test suite. `tests/test_audit_completeness.py::test_every_typed_error_is_in_exit_code_mapping_table` — 7th AST scan (DEC-024). `tests/llm/test_logger_grep_gate.py` — lazy-format logger gate (6 dirs as of #9). `tests/cli/test_exit_codes.py` — parametrized exception → exit-code contract. `tests/cli/test_5_surface_parity_select.py` — 5-surface parity for `--select` (issue #37 DEC-017).

See-Also: clauditor's `.claude/rules/llm-cli-exit-code-taxonomy.md` is the source of the four-tier rule; SignalForge ports it as one section inside this file rather than a standalone rule (DEC-009 of #9 — one rule file per pipeline layer).
20 changes: 19 additions & 1 deletion .claude/rules/manifest-readers.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,24 @@ Every typed exception in an external-format reader subclasses a module base (e.g

Reader modules are deterministic JSON-to-typed-objects. They do not emit logs or metrics in v0.1. Observability lives in the stage that *consumes* the data (LLM drafting, prune, grade) — that's where signal-vs-volume tradeoffs surface. Adding logs here just generates noise.

## User-facing string-grammar selectors (issue #37)

When a CLI flag accepts a string grammar that the manifest layer parses (e.g. dbt-style `tag:<name>`, `path:<glob>`, comma-separated unions), the parser + matcher live in a dedicated module under `signalforge.manifest.` rather than in CLI code. Issue #37's `signalforge.manifest.select` ships the pattern:

- **`parse_<grammar>(expr: str) -> tuple[<Atom>, ...]`** — splits + classifies + validates. Returns a tuple of typed value objects. Raises a domain-specific error (`SelectorParseError(ManifestError)`) on malformed input. The CLI layer wraps this error as its own tier-2 typed exception (`CliSelectorParseError`) at the dispatcher boundary; the manifest error itself stays neutral so library callers can consume it directly.
- **`select_<entity>(manifest, expr) -> tuple[<Entity>, ...]`** — parses + matches. Dedupes by primary key (`unique_id`) and sorts deterministically (ASCII-codepoint, NOT locale-aware, so test ordering is stable across environments). Returns an empty tuple on zero match — the CLI layer raises its own `CliSelectorNoMatchError` at orchestrator entry, not the manifest layer.
- **Atom value objects** (e.g. `TagAtom`, `PathAtom`, `BareAtom`) — Pydantic v2 frozen with `extra="forbid"` (deliberately stricter than the `extra="ignore"` default for read-back models; selector atoms are user input where typos must fail loud). Use a discriminated union with `Literal["..."]` `kind` field + `Field(discriminator="kind")` on the union alias.

Three load-bearing conventions:

1. **Whitespace handling is part of the grammar contract.** Document explicitly whether the parser strips whitespace around atoms (`tag:foo , path:bar` → `(TagAtom("foo"), PathAtom("bar"))`) AND whether it preserves whitespace INSIDE payloads (`tag:my tag` → tag named `"my tag"`, exact-match against `Model.tags`). Issue #37 strips around-the-comma whitespace but preserves payload whitespace verbatim — the latter is a deliberate choice (matchers are exact-match, no normalisation), but it means an operator typo like `tag:my tag` matches nothing rather than failing loud. Document this trade-off in the parser's docstring.
2. **Multi-expression union is set-OR, deduped by primary key.** Overlapping atoms (e.g. `tag:staging,path:models/staging/*` both matching the same model) yield ONE entry in the result, not duplicates. The sort order is the only thing tests can pin deterministically.
3. **Bare-value disambiguation.** When the grammar has a "bare" atom that routes to one of multiple targets (issue #37: `model.` prefix → `unique_id` lookup; else `original_file_path` lookup), the disambiguator is a simple prefix check, NOT a regex. Typos like `tags:staging` (one extra char) silently route to the bare branch and produce a zero-match error rather than a grammar error. Acceptable for v0.2; v0.3 could add a typo-detection pass.

The drift-detector pattern (one-off `extra="forbid"` strict model paired with a fixture) does NOT apply to selector atoms — atoms are user-input typed (already `extra="forbid"`), not read-back-from-disk types where forward-compat matters. The standard pattern is mandatory only for `extra="ignore"` reader-shaped models.

When v0.3 adds a sibling grammar (e.g. `--filter <expr>` for column-level filters, `--exclude <expr>` for set subtraction), copy this module shape: parser + matcher + atom-union + domain error in the manifest layer; CLI-tier-2 wrapper in the CLI layer; `extra="forbid"` everywhere on the user-input typed surface.

## Reference

`plans/super/2-manifest-loader.md` — DEC-001, DEC-007, DEC-008, DEC-013, DEC-014, DEC-017. `src/signalforge/manifest/loader.py` — current implementation of all three traps.
`plans/super/2-manifest-loader.md` — DEC-001, DEC-007, DEC-008, DEC-013, DEC-014, DEC-017. `plans/super/37-multi-model-select.md` — DEC-001, DEC-012, DEC-016 (selector grammar additions). `src/signalforge/manifest/loader.py` — current implementation of all three traps. `src/signalforge/manifest/select.py` — issue-#37 selector module (parse_selector / select_models / SelectorAtom).
10 changes: 10 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,16 @@ v0.2 additions (issue #35 — operator-chosen prune disable):

- `PruneConfig.enabled: bool = True` — new `extra="forbid"` config field; default preserves all v0.1 behaviour. When set to `false`, `prune_tests` short-circuits after audit-path resolution + `config_hash` computation and BEFORE `_validate_trusted_models` / `TableRef.from_model` / `with adapter:`; every candidate routes to `kept-without-evidence` with `why="prune disabled in signalforge.yml"` (locked verbatim per DEC-003 of `plans/super/35-prune-enabled-doc-reframe.md`). The fail-closed audit invariant is preserved — one `PruneEvent` per candidate still lands in `prune.jsonl`. The CLI emits one `_LOGGER.info` line at prune-stage entry when the flag fires. Trade-off: disabling prune lets always-pass tests reach the diff (directly counter to Architectural Commitment #1, signal over volume); the field exists as an operator-chosen escape hatch for cases where warehouse contact is unavailable (offline, credentials issue, cost ceiling) but a draft run is still wanted. Documented in `docs/prune-ops.md` § Configuration.

v0.2 additions (issue #37 — `--select` for multi-model batch + cookbook):

- `signalforge.manifest.parse_selector(expr: str) -> tuple[SelectorAtom, ...]` — parses a dbt-style selector expression (comma-separated union of `tag:<name>`, `path:<glob>`, or bare `<unique_id>`/`<file_path>`). Raises `SelectorParseError(ManifestError)` on malformed input. Whitespace is stripped around atoms but preserved verbatim inside payloads. Documented in `docs/cli-ops.md` § Running across many models.
- `signalforge.manifest.select_models(manifest: Manifest, expr: str) -> tuple[Model, ...]` — matches the parsed selector against the manifest, dedupes by `unique_id`, returns deterministic `unique_id`-sorted (ASCII-codepoint) order. Empty match → empty tuple (the CLI layer raises `CliSelectorNoMatchError` at orchestrator entry; manifest layer stays neutral so library callers can consume the empty result directly). Tag match unions `Model.tags` and `Model.config.tags`. Path match uses `fnmatch.fnmatchcase` against `Model.original_file_path` (diverges from dbt's path-prefix convention — documented).
- `signalforge.manifest.SelectorAtom` — discriminated union value object (`TagAtom`, `PathAtom`, `BareAtom`), Pydantic v2 frozen with `extra="forbid"` (user-input typed; typos must fail loud). Re-exported alongside `parse_selector` / `select_models` from `signalforge.manifest`.
- `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.

Internals (`_loader_helpers`, `_sql_safety`, `_path_safety`, `_test_result_repr`, `adapters/_client`, `_classify_column`, `_compute_policy_hash`, `_resolve_redact_patterns`, `_emitter`, `_renderers`, `_sidecar`, `_artifact_id`, `_ansi_safety`, `_markdown_safety`, `cli/_helpers`, etc.) are `_`-prefixed and not part of the public contract.

## Validation
Expand Down
9 changes: 6 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,12 @@ full rendered diff). The committed `.gitignore` covers `.signalforge/`.
| `LLM response did not match the CandidateSchema shape` | Anthropic response shape drifted vs. the parser | Set `ANTHROPIC_LOG=info` and inspect `~/.anthropic-debug/`; file an issue |

Full per-flag reference, exit-code taxonomy, and environment
variables: [docs/cli-ops.md](docs/cli-ops.md). Maintainer-only
walkthrough of the same flow as a gated test
(`pytest -m e2e --no-cov`): [docs/e2e-smoke-test.md](docs/e2e-smoke-test.md).
variables: [docs/cli-ops.md](docs/cli-ops.md). For multi-model dbt
projects, see [Running across many
models](docs/cli-ops.md#running-across-many-models) for the
`--select` flag and shell-loop pattern. Maintainer-only walkthrough
of the same flow as a gated test (`pytest -m e2e --no-cov`):
[docs/e2e-smoke-test.md](docs/e2e-smoke-test.md).

## CLI

Expand Down
Loading
Loading