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
2 changes: 2 additions & 0 deletions .claude/rules/cli-layer.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ A behaviour change in the CLI touches **five surfaces**, all updated in the same

When introducing a new flag, write surfaces 1–3 first, then the test against those, then back-fill the DEC.

**The bundled Claude Code skill is a sixth parity surface** (#141, see `skill-parity.md`). `src/signalforge/skills/signalforge/SKILL.md` teaches the CLI surface to operators driving Claude Code; a subcommand / flag / demo-command change updates the skill body in the same commit. Enforcement is a gate, not a prompt: `tests/cli/test_skill_cli_parity.py` parses the live argparse subparser registry plus the locked demo-command list and asserts every token appears in `SKILL.md`. The gate runs inside the canonical `VALIDATE_CMD` (`uv run pytest`), so a `/ralph-run` bead that drifts the CLI from the skill fails validation until the skill is fixed. The gate is mechanical (subcommand / flag / demo-command presence); semantic freshness is the clauditor self-grade plus reviewer attention.

## API alignment with adjacent stages

`add_parser(subparsers) -> None` and `cmd_<name>(args) -> int` for every subcommand; `main(argv: list[str] | None = None) -> int` at the top. No top-level `try/except` in `main()` — typed errors flow up; `cmd_<name>` does the explicit catch and returns the right exit code. **One layer's exception → one CLI handler → one exit code.**
Expand Down
52 changes: 52 additions & 0 deletions .claude/rules/skill-parity.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# Skill parity (the bundled Claude Code skill is a CLI-parity surface)

Established by [#141](https://github.com/wjduenow/SignalForge/issues/141) (ship a SignalForge Claude Code skill + install command). The shipped artefacts:

- `src/signalforge/skills/signalforge/SKILL.md` — the user-facing skill that teaches Claude to drive `signalforge generate` / `lint` / `prune-existing` / `init-demo` / `install-skill` / `version` against a user's dbt project, including the zero-credential demo and the gated live e2e flow.
- `signalforge install-skill [<dest>]` — the CLI subcommand that copies the bundled skill out of the wheel into `<dest>/.claude/skills/signalforge/`. Lib seam at `signalforge.skill.install_skill(...)`; CLI handler at `signalforge.cli.install_skill`.
- `tests/cli/test_skill_cli_parity.py` — the parity gate that closes the loop.

Because SKILL.md documents the CLI surface, it is a **parity surface**: it must stay in lockstep with the actual CLI.

## The skill is the 6th parity surface (extends `cli-layer.md`)

A behaviour change to the CLI subcommand/flag surface — adding, renaming, or removing a subcommand; changing the flags or demo commands the skill names — updates `src/signalforge/skills/signalforge/SKILL.md` in the **same change**. The bundled skill is one more entry in `cli-layer.md`'s "a behaviour change touches N surfaces" rule; treat it exactly like the help string / ops doc / test surfaces already listed there.

## Enforcement is a gate, not a prompt (load-bearing)

Do **not** rely on the model remembering to update the skill during a `/ralph-run` (or any) session. `tests/cli/test_skill_cli_parity.py` is the **parity gate** — it parses the live CLI (`signalforge.cli._build_parser()` → walks the `_SubParsersAction.choices` mapping) plus the locked `_CANONICAL_DEMO_COMMANDS` tuple and asserts every token appears verbatim in `SKILL.md`. Three categories scanned (per #141 DEC-015):

1. **Every subcommand name** from the live argparse parser. Auto-grows when a new subcommand lands — the gate iterates the parser, never a hardcoded set, so adding `signalforge foo` and forgetting `SKILL.md` fails the gate without anyone editing the test.
2. **Four canonical demo command lines** (hardcoded in the test, mirrors the demo flow taught in SKILL.md): `signalforge init-demo`, `signalforge generate <model> --write`, `signalforge prune-existing <model> --schema <path>`, `signalforge install-skill`. Plain substring match; no whitespace / case normalisation (mirrors the envelope-breach guard pattern from `business-rule-tests.md`).
3. **The install-skill bootstrap line** — covered by category 2's fourth entry but documented as a separate concern in the test docstring.

The third test in the file (`test_parity_gate_catches_missing_subcommand_planted_violation`) is the **planted-violation self-check** required by `testing-signal.md` § "AST source-scan gates": it writes a synthetic SKILL.md missing one subcommand to `tmp_path`, drives the same factored-out helper the real gate uses, and asserts an `AssertionError` is raised. Without it, a refactor that broke the scan visitor would silently disable the gate at the exact moment a real violation needed catching.

Because `/ralph-run` runs `VALIDATE_CMD` on every bead, a change that drifts the CLI from the skill **fails validation until SKILL.md is updated** — the skill stays current automatically. The gate also encodes "**when appropriate**": it fires only on a relevant surface change, never on unrelated work. This is the same gate-over-prompt philosophy as the AST scans, drift detectors, and grep gates in `testing-signal.md`.

## Worker-writability — keep the skill in `src/`, never `.claude/`

Ralph workers **cannot write to `.claude/` in worktrees** (orchestrator-only — see the user memory `ralph-worker-claude-dir-perms`). The shipped skill therefore lives in `src/signalforge/skills/` and the parity gate in `tests/` — **both worker-writable** — so a worker that trips the gate fixes `SKILL.md` in `src/` itself. The maintainer-only skills (`release-manager`, `review-agentskills-spec`) stay under repo-root `.claude/skills/` and are excluded from the wheel + the install command:

- They live at repo-root `.claude/skills/`, outside `src/`, so the Hatch `include = ["src/signalforge/skills"]` cannot reach them by construction (DEC-022 of #141).
- `signalforge install-skill` enumerates from `importlib.resources.files("signalforge").joinpath("skills")` — the package-data tree only — so there's no code path that could install them.
- A defensive **negative assertion** in `tests/test_wheel_packaging.py` (`test_wheel_excludes_maintainer_only_claude_skills`) documents this intent: no `.claude/skills/*` paths appear in the built wheel.

Never move the shipped skill under `.claude/`: that would make it un-updatable by workers and defeat this rule.

## The two-name convention (load-bearing)

Two paths, one each side of the seam — easy to confuse, deliberately distinct:

- **Package-data tree:** `src/signalforge/skills/signalforge/SKILL.md` — plural `skills/` parent matches the install destination shape (`.claude/skills/signalforge/SKILL.md`) and allows future sibling skills (e.g. `skills/signalforge-grade/`) without restructuring. NOT a Python package — no `__init__.py` under `skills/` or `skills/signalforge/`. Mirrors `src/signalforge/_demo/`'s posture (package-data, not a Python package).
- **Python lib package:** `src/signalforge/skill/` — singular `skill/`, a real Python package with `__init__.py` + `errors.py`. Owns `install_skill(...)` and the `SkillError` hierarchy. Mirrors `signalforge.demo` exactly.

When adding a v0.2 sibling skill, add `src/signalforge/skills/<other-skill>/SKILL.md` (recursive Hatch include picks it up); the parity gate auto-grows for `<other-skill>`'s subcommands; the Python lib stays a single `signalforge.skill` module.

## What the gate cannot catch

The gate enforces the **mechanical** surface (subcommands / flags / demo commands present). It cannot judge whether the skill's **prose** is still accurate after a behaviour change. Back that with the optional clauditor self-grade (`clauditor grade src/signalforge/skills/signalforge/SKILL.md`, see #141 DEC-014 + US-008 — pre-release manual run, pinned in `assets/SKILL.eval.json`, surfaced via shields.io README badge) and reviewer attention — the gate is necessary, not sufficient.

## Reference

`#141` — the skill, the `install-skill` command, and the parity gate. `plans/super/141-claude-skill-install.md` — the full plan (24 DECs). `cli-layer.md` § "Multi-surface parity for behaviour changes" — the N-surface parity rule the skill extends + the exit-code taxonomy the install command follows. `python-build.md` — wheel packaging of the skill (`include` + `wheel_smoke`). `testing-signal.md` — the gate-over-prompt philosophy + planted-violation self-check requirement. `tests/cli/test_skill_cli_parity.py` — the gate. `tests/cli/test_5_surface_parity_*.py` — the per-subcommand parity-test precedent (orthogonal to this gate — that one pins ONE subcommand across five surfaces; this one scans the FULL CLI surface against ONE skill body).
16 changes: 15 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,19 @@ All notable changes to SignalForge are documented here. The format is loosely ba

_Nothing yet — entries land here on `dev` and get promoted to a dated section at release time._

## [0.5.0] — 2026-05-30

### Added

- **SignalForge skill for Claude Code + `install-skill` subcommand (#141).** Ships a bundled Claude Code skill at `src/signalforge/skills/signalforge/SKILL.md` that teaches Claude to drive the SignalForge CLI against a user's dbt project — pointing at a project, running the zero-cred demo, drafting + pruning real models, reading diffs, and the gated live e2e flow. New `signalforge install-skill [<dest>]` subcommand copies the bundled skill out of the wheel into `<dest>/.claude/skills/signalforge/`; library seam at `signalforge.skill.install_skill(...)` with a three-class typed-error hierarchy (`SkillDestPathError` tier 1, `SkillDestUnsafeError` tier 2, `SkillPackageDataMissingError` tier 1) mirroring `signalforge.demo.copy_demo` verbatim for symlink-cycle defence. Symlink protection covers every bundled path (SKILL.md AND every `assets/` file), not just the top-level destination — a symlinked ancestor directory or sibling asset cannot smuggle writes through copytree.
- **SKILL ↔ CLI parity gate (`tests/cli/test_skill_cli_parity.py`).** The bundled skill is the project's **6th parity surface** (`.claude/rules/skill-parity.md`). The gate parses the live argparse subparser registry, four canonical demo command lines, and `signalforge <subcommand> --<flag>` patterns from the SKILL body, then asserts every token appears verbatim in `SKILL.md`. Runs inside the canonical `uv run pytest` so a CLI change that drifts from the skill fails validation until the skill is updated in the same commit — gate-over-prompt, not a reviewer-discretion item. Planted-violation self-check per `.claude/rules/testing-signal.md` § AST source-scan gates.
- **`docs/skills.md` documentation + README skill pointer.** New MkDocs page documenting the bundled skill, the `install-skill` subcommand, the two demo paths (zero-cred default + opt-in live e2e), the parity gate, and the maintainer-only-skill wheel exclusion. README Quick-start gains a one-sentence pointer.

### Fixed

- **`install-skill` symlink defence extended to every bundled path (#141 CodeRabbit/Copilot).** The pre-review version protected only `<dest>/.claude/skills/signalforge/SKILL.md`; a symlinked `assets/SKILL.eval.json` (or symlinked `assets/` directory) would have smuggled writes through copytree. The seam now enumerates every relative path under the bundled source tree via `rglob` and refuses to overwrite any of them through a symlink, with `mkdir(parents=True)` wrapped to raise `SkillDestUnsafeError` (not raw `OSError`) when a non-directory component sits along the install chain (e.g. `<dest>/.claude` is a regular file).
- **`install-skill` `existed_before` probe now catches broken symlinks (#141 CodeRabbit).** The DEC-017 stdout contract is "True for files and symlinks (both shapes are replaced from the operator's POV)." `.exists()` alone follows symlinks AND returns False for broken symlinks; probe now ORs `.is_symlink()` so a broken-symlink destination is honestly reported as "replaced" (even though the lib seam then refuses to write through it).

## [0.4.0] — 2026-05-30

### Added
Expand Down Expand Up @@ -90,7 +103,8 @@ signalforge --version
- OSS-first, Core-friendly — no dbt Cloud dependency; runs against any dbt-core project, locally or in CI.
- Explainable diffs — every kept/dropped/flagged artifact ships with a one-line "why"; every run produces a sidecar JSON with reproducibility hashes.

[Unreleased]: https://github.com/wjduenow/SignalForge/compare/v0.4.0...HEAD
[Unreleased]: https://github.com/wjduenow/SignalForge/compare/v0.5.0...HEAD
[0.5.0]: https://github.com/wjduenow/SignalForge/releases/tag/v0.5.0
[0.4.0]: https://github.com/wjduenow/SignalForge/releases/tag/v0.4.0
[0.3.0]: https://github.com/wjduenow/SignalForge/releases/tag/v0.3.0
[0.2.0]: https://github.com/wjduenow/SignalForge/releases/tag/v0.2.0
Expand Down
6 changes: 5 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
[![codecov](https://codecov.io/gh/wjduenow/SignalForge/branch/dev/graph/badge.svg)](https://codecov.io/gh/wjduenow/SignalForge) [![docs](https://img.shields.io/badge/docs-signalforge-blue?logo=materialformkdocs)](https://wjduenow.github.io/SignalForge/)
[![codecov](https://codecov.io/gh/wjduenow/SignalForge/branch/dev/graph/badge.svg)](https://codecov.io/gh/wjduenow/SignalForge) [![docs](https://img.shields.io/badge/docs-signalforge-blue?logo=materialformkdocs)](https://wjduenow.github.io/SignalForge/) [![clauditor-graded](https://img.shields.io/badge/clauditor-pending-lightgrey)](src/signalforge/skills/signalforge/assets/SKILL.eval.json)

# SignalForge

Expand Down Expand Up @@ -113,6 +113,10 @@ without adding it to a project environment.
**Working from a clone (contributing)?** Install the dev toolchain with
`uv sync --dev` — see [CONTRIBUTING.md](CONTRIBUTING.md) for the full workflow.

Run `signalforge install-skill` to drop the [Claude Code skill](docs/skills.md)
into your project's `.claude/skills/signalforge/` and let Claude drive
SignalForge end-to-end.

### 2. Authenticate to BigQuery and your LLM provider

```bash
Expand Down
101 changes: 97 additions & 4 deletions docs/cli-ops.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,10 @@ After install, the `signalforge` console script is registered via

## Subcommands

The CLI exposes five subcommands: `generate`, `init-demo`, `lint`,
`prune-existing`, `version`. `signalforge --help` prints the
top-level help; each subcommand has its own `--help` page (e.g.
`signalforge generate --help`).
The CLI exposes six subcommands: `generate`, `init-demo`,
`install-skill`, `lint`, `prune-existing`, `version`. `signalforge
--help` prints the top-level help; each subcommand has its own
`--help` page (e.g. `signalforge generate --help`).

### `signalforge generate <model>`

Expand Down Expand Up @@ -335,6 +335,99 @@ signalforge lint
signalforge generate models/staging/stg_bikeshare_trips.sql --dry-run
```

### `signalforge install-skill [<dest>]`

Copy the bundled SignalForge Claude Code skill into
`<dest>/.claude/skills/signalforge/`. With the skill installed, a
Claude Code session in `<dest>` recognises requests like "draft tests
for `dim_customers`" or "prune my existing `schema.yml`," picks the
right `signalforge` subcommand and flags, and explains the resulting
kept / dropped / flagged diff back to the user. See
[docs/skills.md](skills.md) for the skill catalog entry and the body
sections it covers (DEC-021 of
[`plans/super/141-claude-skill-install.md`](../plans/super/141-claude-skill-install.md)).

Wraps the public library entry point
`signalforge.skill.install_skill(dest) -> Path`; the CLI re-raises
the lower-level `SkillError` subclasses as `CliInstallSkill*Error`
wrappers at the handler boundary so the four-tier exit-code taxonomy
stays homogeneous (DEC-008).

Positional argument:

- `<dest>` — Destination directory. Optional; default `.` (the
current working directory), so the common invocation from a dbt
project root is just `signalforge install-skill`. Relative paths
resolve against the current working directory; `~` expands.
Symlink-cycle defence applies (resolves via `.resolve(strict=True)`,
falling back to `.resolve(strict=False)` on
`FileNotFoundError` / `NotADirectoryError`) and raises
`CliInstallSkillPathError` on a cycle on every supported Python
version (gh-108958). **No `--project-dir` containment gate applies**
— `install-skill` is the second subcommand that *creates* a project
context rather than operating *inside* one (the first is
`init-demo`), so the `canonicalise_user_path(...)` containment
helper used by every other CLI flag is deliberately bypassed
(DEC-006).

Flags: none. There is **no `--force` flag** (DEC-003): the library
seam always overwrites every file SignalForge ships and preserves
every other file in the destination tree, so the
`--force`-against-symlink-dest hazard `init-demo --force` defends
against does not apply here.

Install path: `<dest>/.claude/skills/signalforge/SKILL.md`. The
companion `assets/` subtree (also part of the bundled skill) lands
alongside it.

Exit codes (four-tier taxonomy; see § Four-tier exit-code taxonomy
for the full table):

- `0` — install succeeded; INFO line printed to stdout.
- `1` — `CliInstallSkillPathError` (symlink cycle on `<dest>`) or
`CliInstallSkillPackageDataMissingError` (broken wheel install:
the bundled skill tree could not be located via
`importlib.resources` — practically unreachable on a clean
`pip install signalforge-dbt` run).
- `2` — `CliInstallSkillDestUnsafeError`: `<dest>` exists as a
regular file (not a directory), OR the existing `SKILL.md` is a
symlink (writing would follow the link and clobber an arbitrary
destination).
- `3` — n/a. `install-skill` makes no network, warehouse, or LLM
call.

Stdout shapes:

- New install (no existing `SKILL.md` at the target):
```text
Installed SignalForge skill to <abs path>
```
- Upgrade-in-place (existing `SKILL.md` was overwritten — detected
via `Path.exists()` BEFORE the copy, DEC-017):
```text
Installed SignalForge skill to <abs path> (replaced existing SKILL.md)
```

The `(replaced existing SKILL.md)` suffix surfaces the lib seam's
upgrade-in-place overwrite policy so operators know their
hand-edited `SKILL.md` was replaced. The operator can `git diff` if
they had the file under version control.

Stderr shapes: standard `ERROR: <message>` + optional
`↳ Remediation: <text>` per tier (see § Stderr message shape per
tier); no multi-violation header / bullet form fires from this
subcommand.

Example:

```bash
cd /repo/dbt/analytics
signalforge install-skill
# stdout: Installed SignalForge skill to /repo/dbt/analytics/.claude/skills/signalforge/SKILL.md
echo $?
# 0
```

### `signalforge lint`

Validate the five existing `signalforge.yml` config blocks (`safety:`,
Expand Down
Loading
Loading