Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions .claude/rules/cli-layer.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,11 @@ The CLI is the orchestration layer (NOT a stage-0 reader) so it IS allowed to em

## 7th AST scan: every typed exception has an exit-code mapping (DEC-024)

`tests/test_audit_completeness.py::test_every_typed_error_is_in_exit_code_mapping_table` walks every `src/signalforge/*/errors.py` **and every `src/signalforge/*/*/errors.py`** (depth-1 ∪ depth-2 glob — see "Depth-2 glob extension" below), collects each `class <Name>Error(...):` via `ast.ClassDef`, and asserts the class is registered in `_EXCEPTION_TO_EXIT_CODE`. Excludes the twelve per-stage abstract bases (frozenset `_EXCEPTION_MAPPING_EXCLUDED_BASES`: `ManifestError`, `WarehouseError`, `SafetyError`, `LLMError`, `DraftError`, `PruneError`, `GradeError`, `DiffError`, `CliError`, `DemoError`, `IngestError`, `CostError`); subclasses inherit via MRO.
`tests/test_audit_completeness.py::test_every_typed_error_is_in_exit_code_mapping_table` walks every `src/signalforge/*/errors.py` **and every `src/signalforge/*/*/errors.py`** (depth-1 ∪ depth-2 glob — see "Depth-2 glob extension" below), collects each `class <Name>Error(...):` via `ast.ClassDef`, and asserts the class is registered in `_EXCEPTION_TO_EXIT_CODE`. Excludes the fourteen per-stage abstract bases (frozenset `_EXCEPTION_MAPPING_EXCLUDED_BASES`: `ManifestError`, `WarehouseError`, `SafetyError`, `LLMError`, `DraftError`, `PruneError`, `GradeError`, `DiffError`, `CliError`, `DemoError`, `IngestError`, `CostError`, `SkillError`, `AirflowIntegrationError`); subclasses inherit via MRO.

**Dual registration (issue #59).** Ten of the twelve abstract bases are ALSO registered in `_EXCEPTION_TO_EXIT_CODE` at a single fallback tier (`ManifestError`/`DiffError`/`CliError` → 1; `DraftError`/`CostError` → 2; `LLMError`/`WarehouseError`/`GradeError`/`SafetyError`/`PruneError` → 3). Two independent roles: the frozenset excludes bases from the AST scan's required-mapping check; the table entry is a forward-compat safety net so a new concrete subclass that forgets a table entry still gets the parent's tier via the MRO walk rather than dropping to the panic-path tier 1. The AST scan still fails loud on the missing per-class entry — fallback is safety net, not substitute. `DemoError` and `IngestError` (issue #104) are the two deliberate exceptions: their concretes span tiers 1 and 2 (`DemoPathError`/`DemoFixtureMissingError` → 1, `DemoDestExistsError`/`DemoDestUnsafeError` → 2; `IngestSchema*Error` → 1, `IngestModelNotFoundError`/`IngestAnchorContractError` → 2), so no single fallback tier fits — each appears only in the frozenset, and a forgotten table entry falls through to tier 1 (AST scan catches it at test time). `CostError` (issue #157) joins the dual-registration tier-2 camp: its three concretes (`CostRollupAuditMissingError`, `CostRollupMalformedRecordError`, `CostRollupUnknownModelError`) all map to tier 2 (input-validation — the audit corpus IS the input), so the abstract base safely shares that fallback.
**Dual registration (issue #59).** Ten of the fourteen abstract bases are ALSO registered in `_EXCEPTION_TO_EXIT_CODE` at a single fallback tier (`ManifestError`/`DiffError`/`CliError` → 1; `DraftError`/`CostError` → 2; `LLMError`/`WarehouseError`/`GradeError`/`SafetyError`/`PruneError` → 3). Two independent roles: the frozenset excludes bases from the AST scan's required-mapping check; the table entry is a forward-compat safety net so a new concrete subclass that forgets a table entry still gets the parent's tier via the MRO walk rather than dropping to the panic-path tier 1. The AST scan still fails loud on the missing per-class entry — fallback is safety net, not substitute. The remaining four — `DemoError`, `IngestError` (issue #104), `SkillError` (issue #141), and `AirflowIntegrationError` (issue #230) — are deliberate excluded-only exceptions (frozenset only, no dual-registration fallback). `DemoError`/`IngestError`'s concretes span tiers 1 and 2 (`DemoPathError`/`DemoFixtureMissingError` → 1, `DemoDestExistsError`/`DemoDestUnsafeError` → 2; `IngestSchema*Error` → 1, `IngestModelNotFoundError`/`IngestAnchorContractError` → 2), so no single fallback tier fits; each appears only in the frozenset, and a forgotten table entry falls through to tier 1 (AST scan catches it at test time). `CostError` (issue #157) joins the dual-registration tier-2 camp: its three concretes (`CostRollupAuditMissingError`, `CostRollupMalformedRecordError`, `CostRollupUnknownModelError`) all map to tier 2 (input-validation — the audit corpus IS the input), so the abstract base safely shares that fallback. `AirflowIntegrationError` (issue #230) joins the `DemoError`/`IngestError` excluded-only camp: it ships a single tier-2 concrete (`AirflowConfigError`) today, but is deliberately given NO dual-registration fallback entry — airflow errors surface through Airflow's own task runner (`AirflowFailException`), not the `signalforge` CLI panic path, so the registration is scan-7 compliance only; a forgotten future concrete falls through to tier 1 where the AST scan catches it.

Companion test `test_scan_7_discovers_every_per_stage_errors_module` asserts the scan walks exactly twelve `errors.py` files (issue #157 bumped 11 → 12 with `llm/cost/errors.py` — the first sub-stage `errors.py` under `src/signalforge/<stage>/<sub>/`). A new stage or sub-stage's `errors.py` must bump the count AND add its abstract base to the excluded set in lockstep. Sanity test `test_exit_code_mapping_has_at_least_one_entry_per_tier` guards against mass rename/deletion.
Companion test `test_scan_7_discovers_every_per_stage_errors_module` asserts the scan walks exactly fourteen `errors.py` files (issue #157 bumped the count with `llm/cost/errors.py` — the first sub-stage `errors.py` under `src/signalforge/<stage>/<sub>/`; #141 added `skill/errors.py`; #230 added `airflow/errors.py` → 14). A new stage or sub-stage's `errors.py` must bump the count AND add its abstract base to the excluded set in lockstep. Sanity test `test_exit_code_mapping_has_at_least_one_entry_per_tier` guards against mass rename/deletion.

**Depth-2 glob extension (issue #157).** The scan's enumerator graduated from `_SIGNALFORGE_DIR.glob("*/errors.py")` (depth-1 only) to **`_SIGNALFORGE_DIR.glob("*/errors.py") ∪ _SIGNALFORGE_DIR.glob("*/*/errors.py")`** when `signalforge.llm.cost.errors` shipped as the first sub-stage `errors.py`. The generalisation is durable: future sub-stage modules can carry their own `errors.py` and scan-7 picks them up automatically without further glob churn. The depth-1 set keeps covering `cli/errors.py` directly (the CLI is itself a depth-1 stage), so no special-case branch is needed.

Expand Down
10 changes: 10 additions & 0 deletions .claude/rules/python-build.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,16 @@ The canonical dev install is `uv sync --dev`. `uv.lock` is committed; the lockfi

The old "quote the `".[dev]"` — `[dev]` is a glob in zsh" gotcha drops away — `uv sync --dev` takes no glob-fragile argument.

### Deliberate exception: a heavy/isolated optional extra is NOT mirrored into the dev group (issue #230)

The "every optional extra also appears in `[dependency-groups].dev`" convention has ONE deliberate exception: the **`[airflow]` extra** (`apache-airflow>=2.8,<3`, epic #228). It lives in `[project.optional-dependencies]` ONLY — **not** in `[dependency-groups].dev` — so a default `uv sync --dev` (and therefore every default CI lint-test + pyright run) stays Apache-Airflow-free. Three load-bearing reasons:

1. **The acceptance contract is "green WITHOUT Airflow installed."** `signalforge.airflow` types against duck-typed protocols in its one shim (`_airflow_compat`), so `uv run pyright` + the default `uv run pytest` must pass with no airflow present (`tests/airflow` is also excluded from pyright). Mirroring airflow into the dev group would import the heavy, tightly-version-pinned tree into the default env and break that contract.
2. **#229 isolates Airflow on purpose.** Airflow is installed into a separate constraints-pinned `.venv-airflow` (Apache constraints file, certified 2.10.4/py3.11) — NOT through the project's unified resolution. The gated `airflow` pytest marker + the label-gated CI job run there; the gate tests (import-confinement, no-eager-import, wheel-deps) run UNGATED in the default suite and never import airflow.
3. **`uv.lock` still carries the extra (purely additively).** `uv lock` resolves the `[airflow]` extra into the committed lock — verified additive, with NO version downgrades to any default-env package — so the lock stays in sync with `pyproject.toml` while `uv sync --dev` (no `--frozen`) installs none of the airflow tree.

The general rule: when an optional extra is heavy, tightly version-pinned, or must be installable only under a vendor constraints file, keep it OUT of the dev group, document the deviation here, and back it with a no-eager-import gate + a wheel-deps assertion that the base wheel never vendors it. Don't reflexively mirror every extra into `[dependency-groups].dev`.

## Python version: advertised floor matches the tested floor (issue #46, uv migration)

`pyproject.toml` declares `requires-python = ">=3.11"`; `[tool.pyright].pythonVersion` is `"3.11"`; `.github/workflows/ci.yml` runs a `python-version: ["3.11", "3.12"]` matrix. **All three agree on the floor** — what we advertise (`>=3.11`) is what we type-check (`3.11`) is what we test as the *floor of the matrix* (`3.11`). The 3.12 iteration runs pytest only; pyright is gated on `matrix.python-version == '3.11'` so the type-check pins to the advertised floor.
Expand Down
11 changes: 6 additions & 5 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,12 @@ jobs:
CONSTRAINTS="https://raw.githubusercontent.com/apache/airflow/constraints-${{ matrix.airflow-version }}/constraints-${{ matrix.python-version }}.txt"
uv venv
uv pip install "apache-airflow==${{ matrix.airflow-version }}" --constraint "$CONSTRAINTS"
# `-e .` until the skeleton child (epic #228) ships the `[airflow]` extra,
# then `-e ".[airflow]"`. The --constraint is load-bearing: without it uv
# upgrades protobuf 4->5 (+ pydantic) over Airflow 2.10.4's pins and risks
# breaking it; with it, signalforge resolves cleanly inside those pins.
uv pip install -e . --constraint "$CONSTRAINTS"
# `-e ".[airflow]"` — the `[airflow]` optional extra now exists (shipped by
# the epic #228 skeleton child #230). The --constraint is load-bearing:
# without it uv upgrades protobuf 4->5 (+ pydantic) over Airflow 2.10.4's
# pins and risks breaking it; with it, signalforge resolves cleanly inside
# those pins.
uv pip install -e '.[airflow]' --constraint "$CONSTRAINTS"
# Test tooling under the same constraints — the `[airflow]` extra carries no
# test deps, so without pytest-cov the `--no-cov` flag is unrecognised.
uv pip install pytest pytest-cov pytest-asyncio --constraint "$CONSTRAINTS"
Expand Down
1 change: 1 addition & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ The pipeline is a chain of subpackages, each with a distilled rules file in
| `signalforge.cli` | console-script entry, four-tier exit codes | `cli-layer.md` |
| `signalforge.ingest` | external `schema.yml` / `tests/*.sql` reader (prune any generator's tests) | `ingest-layer.md` |
| `signalforge.demo` | bundled Austin demo project (`init-demo`) | — |
| `signalforge.airflow` | Apache Airflow operator integration — **skeleton only** (v0.7, epic #228): one-shim seam (`_airflow_compat`) + lazy `__getattr__` re-exports + stub operators/hooks; ships behind the `[airflow]` extra, zero Airflow weight in the base install | — |

Cross-cutting rules: `business-rule-tests.md` (the `custom_sql` 5th test type, threaded
through every stage), `testing-signal.md`, `python-build.md`, `ci-supply-chain.md`,
Expand Down
Loading
Loading