From dbdcb8521eeec7c9eea86367c721554fdafa0ce8 Mon Sep 17 00:00:00 2001 From: Sahil Ahuja Date: Fri, 26 Jun 2026 17:02:04 +0530 Subject: [PATCH 1/4] =?UTF-8?q?feat:=20detect=20&=20fill=20test=5Fpaths=20?= =?UTF-8?q?at=20setup=20from=20on-disk=20markers=20+=202.7.1=E2=86=922.8.0?= =?UTF-8?q?=20migration?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/memory/_shared/configuration.md | 2 +- docs/memory/distribution/index.md | 4 +- docs/memory/distribution/migrations.md | 10 +- docs/memory/distribution/setup.md | 19 +- docs/specs/skills/SPEC-fab-setup.md | 13 ++ .../.history.jsonl | 13 ++ .../.status.yaml | 55 +++++ .../intake.md | 136 +++++++++++++ .../plan.md | 192 ++++++++++++++++++ src/kit/VERSION | 2 +- src/kit/migrations/2.7.1-to-2.8.0.md | 138 +++++++++++++ src/kit/scaffold/fab/project/config.yaml | 16 +- src/kit/skills/fab-setup.md | 28 ++- 13 files changed, 615 insertions(+), 13 deletions(-) create mode 100644 fab/changes/260626-5qf5-detect-fill-test-paths/.history.jsonl create mode 100644 fab/changes/260626-5qf5-detect-fill-test-paths/.status.yaml create mode 100644 fab/changes/260626-5qf5-detect-fill-test-paths/intake.md create mode 100644 fab/changes/260626-5qf5-detect-fill-test-paths/plan.md create mode 100644 src/kit/migrations/2.7.1-to-2.8.0.md diff --git a/docs/memory/_shared/configuration.md b/docs/memory/_shared/configuration.md index 70edb69c..ff9e596a 100644 --- a/docs/memory/_shared/configuration.md +++ b/docs/memory/_shared/configuration.md @@ -82,7 +82,7 @@ Optional top-level field (7t5a). A YAML sequence of glob/pathspec patterns ident **Purpose — attribution, NOT exclusion.** `test_paths` does NOT strip lines from the impact universe — it *attributes* the already-scaffolding-excluded lines to tests vs. implementation, so reviewers see the split (e.g. "140 impl / 400 test") instead of one conflated number. This is orthogonal to `true_impact_exclude`, which removes scaffolding/doc *noise* from the universe entirely. Tests are first-class deliverables (constitution principle VII, Test Integrity), not scaffolding noise — adding test patterns to `true_impact_exclude` was explicitly rejected because it would conflate the two axes and destroy the test-coverage signal. -**No kit default (Portability, constitution principle V).** fab-kit is language-agnostic; there is no universal test-file pattern (`*_test.go`, `test_*.py`, `*.spec.ts` all differ by language). The kit MUST NOT ship a default `test_paths` list — each project declares its own. The scaffold (`src/kit/scaffold/fab/project/config.yaml`) carries only a commented-out `# test_paths: []` placeholder so users discover the knob. This repo dogfoods it with `test_paths: ["**/*_test.go"]`. +**No kit default (Portability, constitution principle V).** fab-kit is language-agnostic; there is no universal test-file pattern (`*_test.go`, `test_*.py`, `*.spec.ts` all differ by language), so the kit MUST NOT ship a hardcoded default `test_paths` list. Instead, the value is **detected per project at setup**: the scaffold (`src/kit/scaffold/fab/project/config.yaml`) ships a standing annotated example comment block (Go/Python/JS-TS/Java-Kotlin patterns + the `:(glob)`/anti-substring note) above a `{TEST_PATHS}` placeholder, and `/fab-setup config` create-mode auto-detects the project's ecosystem from on-disk marker files (`go.mod`→Go, `pyproject.toml`→Python, etc.) and fills the anchored pattern **non-interactively**. Unrecognized or inline-test stacks (e.g. Rust) are left empty — the breakdown then collapses to a single total. Existing projects are backfilled by the `2.7.1-to-2.8.0` migration (same detection; skips projects that already set `test_paths`). This repo dogfoods it with `test_paths: ["**/*_test.go"]`. **Glob/pathspec format.** Include patterns are applied as `:(glob)` magic pathspecs in the test-only `git diff --shortstat` pass, so wildcards behave like `.gitignore`-style globs — notably `**` matches across directory boundaries, so `**/*_test.go` matches both root-level `foo_test.go` and nested `pkg/foo_test.go`. (Without `:(glob)`, plain git pathspec rules treat `*` as not crossing `/`, silently missing root-level test files and under-counting tests.) The pass combines these includes with the same `:(exclude)` args derived from `true_impact_exclude`, so tests are counted within the scaffolding-excluded universe; when `true_impact_exclude` is empty, the pass runs with the includes alone (tests attributed within the raw universe). diff --git a/docs/memory/distribution/index.md b/docs/memory/distribution/index.md index 33cad446..9f8d3413 100644 --- a/docs/memory/distribution/index.md +++ b/docs/memory/distribution/index.md @@ -9,5 +9,5 @@ description: "How the kit is packaged, shipped & configured — .kit/ structure, |------|-------------| | [distribution](distribution.md) | How `src/kit/` is distributed — Homebrew formula (2 binaries direct + 2 via `depends_on`), `fab` router (always-route policy), `fab-kit` lifecycle, `fab init` bootstrap, `fab upgrade-repo` (offline-first default = installed binary's `systemVersion`; `--latest` opts into the GitHub-API newest-release path; explicit arg wins; `dev`/unstamped → network fallback — 1hmj), release workflow (3 binaries, 12 cross-compiled, `SHA256SUMS` for kit-* archives, `just test` gate before tag/build + `go-version-file` single-sourcing — tb6f), hardened auto-download (bounded HTTP timeouts, version-keyed flock + atomic rename, digest verification) + fail-loud lifecycle exit contracts (`init`/`update`/`upgrade-repo`/`sync`), `wt shell-setup` wrapper; the `shll.ai/fab-kit` public docs site — README-slice pull (`ReadmeSlice.astro`) + producer-side README-conformance obligation (tail boundary at `## Development`, diagram SVGs by absolute raw URL, absolute external slice links — except README→`docs/site/` links kept repo-relative for the site rewrite) + `docs/` audience-axis layout (pull surface is exactly `README.md` + `docs/site/**`; `docs/site/**` pulled + rendered one page per file at `/tools/fab-kit/`, §9 ACTIVE) | | [kit-architecture](kit-architecture.md) | `src/kit/` structure (binary-free; `spec.md` template removed in j6cs; `schemas/` removed in c5tr — scaffold no longer seeds `stage_directives`), three-binary architecture (fab router + fab-kit + fab-go), router always-route policy + shared `LifecycleCommands` allowlist table with contract/collision drift tests (ye8r), `fab-kit sync` (post-state version guard, fail-loud deployment writes, version threading from init/upgrade, stamp-after-success upgrade), agent integration, versioning, monorepos, underscore file ecosystem, `fab pane` command group, `fab shell-init`, hidden `fab help-dump`, shared `internal/lines` + `internal/atomicfile` helpers (hv7t), widened `internal/config` single config.yaml parser (ye8r); tb6f: Go 1.26 + cobra v1.10 toolchain, fab-kit `sync.go` split (semver/prereqs/scaffold/skills + orchestrator), golden byte-stability suite as the standing yaml parity arbiter, yaml.v3-stays-pinned decision, `src/benchmark/` tombstoned; frlo: new `reference/` shipped content dir (read via `$(fab kit-path)/reference/...`, holds the `fkf.md` normative extract) + the whole-`src/kit/`-copied-verbatim packaging invariant (new kit content ships with no Go/packaging change); 2fm8: `templates/memory.md` added — canonical FKF memory-file template, the third artifact template, read on demand via `$(fab kit-path)/templates/memory.md` (ships verbatim, no Go/packaging/migration change) | -| [migrations](migrations.md) | Migration system — dual-version model, migration file format, binary-owned discovery (`fab migrations-status [--json]` / `DiscoverMigrations`), `/fab-setup migrations` subcommand (delegates discovery, applies LLM-driven), brew-install migration, `1.8.0-to-1.9.0` migration (tasks-stage collapse + plan.md), `1.9.1-to-1.9.2` migration (`true_impact_exclude` config field), `1.9.7-to-1.10.0` migration (spec-stage collapse, four-state spec.md→plan.md case table), `2.1.6-to-2.2.0` migration (drops dead `stage_directives` + defensive `model_tiers`; preserves `stage_hooks`), `2.2.0-to-2.3.0` migration (fully-commented `agent.tiers` reference block, comment-sentinel idempotency), `2.5.5-to-2.6.0` migration (freeze-on-write `log.md` re-baseline — `fab memory-index --rebuild` + commit, `--rebuild` binary pre-check, no `fab/`/`.status.yaml` change), `2.6.6-to-2.7.0` migration (drop the index `Last Updated` column → two-column index re-baseline — `fab memory-index` + commit, rendered-output binary pre-check, no `fab/`/`.status.yaml` change, VERSION bump to `2.7.0`), version drift detection (`upgrade-repo` mechanical detection + silent self-stamp + TTY-gated styled reminder), `fab/.kit-migration-version` creation | -| [setup](setup.md) | `/fab-setup` skill — structural bootstrap (sync-first order since szxd: doctor → config → constitution → `fab sync`), subcommand architecture (config, constitution, migrations — version handling delegated to a single `fab migrations-status --json` run), delegation pattern with `fab-kit sync`; stage_directives editor removed, semver one-liner restored at the three-way branch, fab_version fresh-create fallback, Next Steps re-aligned to the State Table (c5tr); scaffold fragment merge fails loudly (jznd) and its `.gitignore` dedup is gitignore-aware — variant coverage + negation hard-stop, `.envrc` stays literal (mqiq) | +| [migrations](migrations.md) | Migration system — dual-version model, migration file format, binary-owned discovery (`fab migrations-status [--json]` / `DiscoverMigrations`), `/fab-setup migrations` subcommand (delegates discovery, applies LLM-driven), brew-install migration, `1.8.0-to-1.9.0` migration (tasks-stage collapse + plan.md), `1.9.1-to-1.9.2` migration (`true_impact_exclude` config field), `1.9.7-to-1.10.0` migration (spec-stage collapse, four-state spec.md→plan.md case table), `2.1.6-to-2.2.0` migration (drops dead `stage_directives` + defensive `model_tiers`; preserves `stage_hooks`), `2.2.0-to-2.3.0` migration (fully-commented `agent.tiers` reference block, comment-sentinel idempotency), `2.5.5-to-2.6.0` migration (freeze-on-write `log.md` re-baseline — `fab memory-index --rebuild` + commit, `--rebuild` binary pre-check, no `fab/`/`.status.yaml` change), `2.6.6-to-2.7.0` migration (drop the index `Last Updated` column → two-column index re-baseline — `fab memory-index` + commit, rendered-output binary pre-check, no `fab/`/`.status.yaml` change, VERSION bump to `2.7.0`), `2.7.1-to-2.8.0` migration (detect + fill `test_paths` from on-disk markers + refresh the scaffold example comment block, sentinel-guarded, preserves non-empty user values, config-only, VERSION bump to `2.8.0`), version drift detection (`upgrade-repo` mechanical detection + silent self-stamp + TTY-gated styled reminder), `fab/.kit-migration-version` creation | +| [setup](setup.md) | `/fab-setup` skill — structural bootstrap (sync-first order since szxd: doctor → config → constitution → `fab sync`), subcommand architecture (config, constitution, migrations — version handling delegated to a single `fab migrations-status --json` run), delegation pattern with `fab-kit sync`; stage_directives editor removed, semver one-liner restored at the three-way branch, fab_version fresh-create fallback, Next Steps re-aligned to the State Table (c5tr); scaffold fragment merge fails loudly (jznd) and its `.gitignore` dedup is gitignore-aware — variant coverage + negation hard-stop, `.envrc` stays literal (mqiq); Config Create-Mode non-interactively detects + fills `test_paths` from on-disk markers via the `{TEST_PATHS}` placeholder, with a visible note (5qf5) | diff --git a/docs/memory/distribution/migrations.md b/docs/memory/distribution/migrations.md index f7f7571d..de36f618 100644 --- a/docs/memory/distribution/migrations.md +++ b/docs/memory/distribution/migrations.md @@ -1,6 +1,6 @@ --- type: memory -description: "Migration system — dual-version model, migration file format, binary-owned discovery (`fab migrations-status [--json]` / `DiscoverMigrations`), `/fab-setup migrations` subcommand (delegates discovery, applies LLM-driven), brew-install migration, `1.8.0-to-1.9.0` migration (tasks-stage collapse + plan.md), `1.9.1-to-1.9.2` migration (`true_impact_exclude` config field), `1.9.7-to-1.10.0` migration (spec-stage collapse, four-state spec.md→plan.md case table), `2.1.6-to-2.2.0` migration (drops dead `stage_directives` + defensive `model_tiers`; preserves `stage_hooks`), `2.2.0-to-2.3.0` migration (fully-commented `agent.tiers` reference block, comment-sentinel idempotency), `2.5.5-to-2.6.0` migration (freeze-on-write `log.md` re-baseline — `fab memory-index --rebuild` + commit, `--rebuild` binary pre-check, no `fab/`/`.status.yaml` change), `2.6.6-to-2.7.0` migration (drop the index `Last Updated` column → two-column index re-baseline — `fab memory-index` + commit, rendered-output binary pre-check, no `fab/`/`.status.yaml` change, VERSION bump to `2.7.0`), version drift detection (`upgrade-repo` mechanical detection + silent self-stamp + TTY-gated styled reminder), `fab/.kit-migration-version` creation" +description: "Migration system — dual-version model, migration file format, binary-owned discovery (`fab migrations-status [--json]` / `DiscoverMigrations`), `/fab-setup migrations` subcommand (delegates discovery, applies LLM-driven), brew-install migration, `1.8.0-to-1.9.0` migration (tasks-stage collapse + plan.md), `1.9.1-to-1.9.2` migration (`true_impact_exclude` config field), `1.9.7-to-1.10.0` migration (spec-stage collapse, four-state spec.md→plan.md case table), `2.1.6-to-2.2.0` migration (drops dead `stage_directives` + defensive `model_tiers`; preserves `stage_hooks`), `2.2.0-to-2.3.0` migration (fully-commented `agent.tiers` reference block, comment-sentinel idempotency), `2.5.5-to-2.6.0` migration (freeze-on-write `log.md` re-baseline — `fab memory-index --rebuild` + commit, `--rebuild` binary pre-check, no `fab/`/`.status.yaml` change), `2.6.6-to-2.7.0` migration (drop the index `Last Updated` column → two-column index re-baseline — `fab memory-index` + commit, rendered-output binary pre-check, no `fab/`/`.status.yaml` change, VERSION bump to `2.7.0`), `2.7.1-to-2.8.0` migration (detect + fill `test_paths` from on-disk markers + refresh the scaffold example comment block, sentinel-guarded, preserves non-empty user values, config-only, VERSION bump to `2.8.0`), version drift detection (`upgrade-repo` mechanical detection + silent self-stamp + TTY-gated styled reminder), `fab/.kit-migration-version` creation" --- # Migrations @@ -118,6 +118,14 @@ A migration file for the transition to the system shim model. The migration: - **Idempotent (Constitution III).** Re-running `fab memory-index` + commit on an already two-column tree is a no-op diff (nothing to commit), and the two-column pre-check still passes. After the baseline, `fab memory-index --check` exits **0 or 1, never 2** (a re-baselined tree is provably never destructive-loss); the `--check` exit-code contract is unchanged. - **Version bump.** `src/kit/VERSION` is bumped to `2.7.0` (the migration's target version) — a behavior change to a shipped CLI warrants a minor bump, matching the catalog's `2.5.5-to-2.6.0` / `2.4.2-to-2.5.0` feature-migration convention. +### `2.7.1-to-2.8.0` Backfill Migration (detect & fill `test_paths` — 5qf5) + +`src/kit/migrations/2.7.1-to-2.8.0.md` backfills `test_paths` in existing projects' `fab/project/config.yaml`, mirroring the new Config Create-Mode detection (see [setup.md](/distribution/setup.md) § Config Create-Mode Detects & Fills `test_paths`). `test_paths` drives the `/git-pr` impact breakdown's test/impl split, ships language-specific with no kit default, and most projects never set it — so the split silently does nothing. The migration has two effects: (a) **refresh the scaffold's `test_paths` example comment block** so users keep an editing reference even when the key stays empty, and (b) **detect + fill `test_paths`** from on-disk marker files via the same anchored marker→ecosystem table the create-mode skill uses (Go/Python/JS-TS/Java-Kotlin/.NET; Rust & unrecognized → empty, since inline `#[cfg(test)]` tests are not glob-addressable). + +- **Config-only, no binary/`.status.yaml` change.** Unlike the re-baseline migrations above (`2.5.5-to-2.6.0`, `2.6.6-to-2.7.0`), this one needs **no binary capability pre-check** — `impact.go` already consumes any non-empty `test_paths` verbatim, and detection is pure prompt logic (Constitution I). It is the config-field-add shape, like `1.9.1-to-1.9.2` (`true_impact_exclude`) and `2.2.0-to-2.3.0` (`agent.tiers`): Summary / Pre-check / Changes / Verification, atomic write. +- **Idempotent + value-preserving (Constitution III).** Pre-check skips entirely when `config.yaml` is absent. The comment-block refresh is **sentinel-guarded** on the `# Examples (uncomment/adapt the line for your stack):` line (re-run no-op). The fill happens **only when `test_paths` is absent or empty** — a user's hand-set non-empty value is preserved unchanged (only the comment block refreshes). Report lines mirror the create-mode notes (detected ecosystem + patterns, or "no test convention detected → left empty"). +- **Version bump.** `src/kit/VERSION` is bumped to `2.8.0` (the migration's target version) — an additive config-feature change is a minor bump, matching the catalog's feature-migration convention. + ### Version Drift Detection - **`fab upgrade-repo`**: after sync, runs `DiscoverMigrations` against the target version's cached `migrations/` dir and the current `fab/.kit-migration-version` (mechanical relevance check, not string inequality). Three terminal cases: diff --git a/docs/memory/distribution/setup.md b/docs/memory/distribution/setup.md index 0d6de241..a7fc6e0b 100644 --- a/docs/memory/distribution/setup.md +++ b/docs/memory/distribution/setup.md @@ -1,6 +1,6 @@ --- type: memory -description: "`/fab-setup` skill — structural bootstrap (sync-first order since szxd: doctor → config → constitution → `fab sync`), subcommand architecture (config, constitution, migrations — version handling delegated to a single `fab migrations-status --json` run), delegation pattern with `fab-kit sync`; stage_directives editor removed, semver one-liner restored at the three-way branch, fab_version fresh-create fallback, Next Steps re-aligned to the State Table (c5tr); scaffold fragment merge fails loudly (jznd) and its `.gitignore` dedup is gitignore-aware — variant coverage + negation hard-stop, `.envrc` stays literal (mqiq)" +description: "`/fab-setup` skill — structural bootstrap (sync-first order since szxd: doctor → config → constitution → `fab sync`), subcommand architecture (config, constitution, migrations — version handling delegated to a single `fab migrations-status --json` run), delegation pattern with `fab-kit sync`; stage_directives editor removed, semver one-liner restored at the three-way branch, fab_version fresh-create fallback, Next Steps re-aligned to the State Table (c5tr); scaffold fragment merge fails loudly (jznd) and its `.gitignore` dedup is gitignore-aware — variant coverage + negation hard-stop, `.envrc` stays literal (mqiq); Config Create-Mode non-interactively detects + fills `test_paths` from on-disk markers via the `{TEST_PATHS}` placeholder, with a visible note (5qf5)" --- # Setup @@ -30,6 +30,23 @@ description: "`/fab-setup` skill — structural bootstrap (sync-first order sinc - Creates `.gitignore` entries - Safe to re-run (idempotent — skips existing files) +### Config Create-Mode Detects & Fills `test_paths` (5qf5) + +As of 260626-5qf5, `/fab-setup config` **create mode** auto-detects and fills `test_paths` from on-disk marker files — **non-interactively** (no prompt). After asking for project name/description/source_paths (step 2), it reads markers and derives an **anchored** test-path pattern from a marker→ecosystem table: + +| Detected marker | Ecosystem | `test_paths` | +|---|---|---| +| `go.mod` | Go | `**/*_test.go` | +| `pytest.ini` / `pyproject.toml` / `setup.cfg` | Python (pytest) | `**/test_*.py`, `**/*_test.py` | +| `package.json` (jest/vitest), or `*.spec`/`*.test` `.ts`/`.js` present | JS/TS | `**/*.spec.ts`, `**/*.test.ts`, `**/*.spec.js`, `**/*.test.js` | +| `pom.xml` / `build.gradle` | Java/Kotlin | `**/src/test/**` | +| `*.csproj` (test SDK) | .NET | `**/*Tests.cs`, `**/*Test.cs` | +| `Cargo.toml` (Rust) / *(no marker)* | — | leave empty (Rust tests are inline `#[cfg(test)]`, not glob-addressable) | + +The derived value is substituted as the new `{TEST_PATHS}` placeholder (step 4, alongside `{PROJECT_NAME}`/`{PROJECT_DESCRIPTION}`/`{SOURCE_PATHS}`), **preserving the scaffold's standing example comment block** and replacing only the active key line. Config Output surfaces a visible note: `Detected {ecosystem} — set test_paths to {patterns}. Edit fab/project/config.yaml if wrong.` when filled, or `No test convention detected — test_paths left empty (impact breakdown will show a single total). Set it later if desired.` for Rust/unrecognized stacks. Multi-marker repos take the union of pattern sets. + +**Why anchored, not a substring**: `test_paths` drives the `/git-pr` impact breakdown's test/impl split (`impl = total − tests`). A bare substring (`**/*test*`) miscounts production code like `attestation.go`/`latest.go` — a confidently-wrong number is worse than the absent (collapsed-to-single-total) breakdown, so unrecognized stacks are left empty. The `2.7.1-to-2.8.0` migration backfills the same detection for existing repos (see [migrations.md](/distribution/migrations.md)). + ### Subcommands `/fab-setup` accepts three subcommands: `config [section]`, `constitution`, and `migrations [file]`. These provide ongoing management of initialization artifacts and version migrations without requiring separate commands. Validation is built into the `config` and `constitution` flows rather than exposed as a standalone subcommand. diff --git a/docs/specs/skills/SPEC-fab-setup.md b/docs/specs/skills/SPEC-fab-setup.md index 1e294c15..0624b831 100644 --- a/docs/specs/skills/SPEC-fab-setup.md +++ b/docs/specs/skills/SPEC-fab-setup.md @@ -11,6 +11,8 @@ Bootstraps a new project or manages config/constitution/migrations. Creates `fab **Prose optimization** (260620-skop): skill content trimmed to remove re-explanation of partial-owned concepts and consolidate the seven Migrations Output Format blocks to one canonical block plus exact-string variant notes, and a `## Contents` TOC added; no behavioral change (Flow / Tools / Sub-agents unchanged). +**test_paths detection** (260626-5qf5): Config Create-Mode gains a **non-interactive** detection sub-step (step 2) that reads on-disk marker files (`go.mod`, `pyproject.toml`/`pytest.ini`, jest/vitest deps, `pom.xml`/`build.gradle`, `*.csproj`) and derives an anchored `test_paths` pattern, substituted as the new `{TEST_PATHS}` placeholder (step 4, alongside `{PROJECT_NAME}`/`{PROJECT_DESCRIPTION}`/`{SOURCE_PATHS}`) while preserving the scaffold's standing example comment block. It never prompts; Config Output surfaces the detected ecosystem+patterns (or "no test convention detected → left empty" for Rust/unrecognized stacks). The `2.7.1-to-2.8.0` migration backfills the same detection + comment refresh for existing repos. + ## Flow ``` @@ -31,6 +33,17 @@ User invokes /fab-setup [subcommand] │ │ ├─ Read: README, package.json (project context) │ │ ├─ Read: src/kit/scaffold/fab/project/config.yaml │ │ ├─ (interactive: ask name, description, source_paths) +│ │ ├─ (NON-INTERACTIVE test_paths detection, 5qf5 — read on-disk +│ │ │ marker files → derive an anchored {TEST_PATHS} pattern from +│ │ │ the marker→ecosystem table: go.mod→**/*_test.go, +│ │ │ pyproject.toml/pytest.ini→**/test_*.py+**/*_test.py, +│ │ │ jest/vitest→**/*.spec|test.ts|js, pom.xml/build.gradle→ +│ │ │ **/src/test/**, *.csproj→**/*Tests.cs+**/*Test.cs; +│ │ │ Rust/unrecognized → leave empty. NO prompt. Substituted as +│ │ │ the {TEST_PATHS} placeholder in step 4, preserving the +│ │ │ scaffold's standing example comment block; visible note in +│ │ │ Config Output: detected ecosystem+patterns, or "no convention +│ │ │ detected → empty") │ │ └─ Write: fab/project/config.yaml │ │ (preserves an existing fab_version key; on a fresh create │ │ with no prior key, stamps the engine version from diff --git a/fab/changes/260626-5qf5-detect-fill-test-paths/.history.jsonl b/fab/changes/260626-5qf5-detect-fill-test-paths/.history.jsonl new file mode 100644 index 00000000..ce4db174 --- /dev/null +++ b/fab/changes/260626-5qf5-detect-fill-test-paths/.history.jsonl @@ -0,0 +1,13 @@ +{"action":"enter","driver":"fab-new","event":"stage-transition","stage":"intake","ts":"2026-06-26T11:08:50Z"} +{"args":"detect and fill test_paths at setup with migration and scaffold examples","cmd":"fab-new","event":"command","ts":"2026-06-26T11:08:50Z"} +{"delta":"+4.8","event":"confidence","score":4.8,"trigger":"calc-score","ts":"2026-06-26T11:10:03Z"} +{"delta":"+0.0","event":"confidence","score":4.8,"trigger":"calc-score","ts":"2026-06-26T11:10:06Z"} +{"cmd":"fab-fff","event":"command","ts":"2026-06-26T11:12:31Z"} +{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"apply","ts":"2026-06-26T11:12:39Z"} +{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"review","ts":"2026-06-26T11:18:58Z"} +{"event":"review","result":"failed","ts":"2026-06-26T11:23:45Z"} +{"action":"re-entry","driver":"fab-fff","event":"stage-transition","stage":"apply","ts":"2026-06-26T11:23:45Z"} +{"action":"re-entry","driver":"fab-fff","event":"stage-transition","stage":"review","ts":"2026-06-26T11:24:38Z"} +{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"hydrate","ts":"2026-06-26T11:29:09Z"} +{"event":"review","result":"passed","ts":"2026-06-26T11:29:09Z"} +{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"ship","ts":"2026-06-26T11:31:14Z"} diff --git a/fab/changes/260626-5qf5-detect-fill-test-paths/.status.yaml b/fab/changes/260626-5qf5-detect-fill-test-paths/.status.yaml new file mode 100644 index 00000000..493075ad --- /dev/null +++ b/fab/changes/260626-5qf5-detect-fill-test-paths/.status.yaml @@ -0,0 +1,55 @@ +id: 5qf5 +name: 260626-5qf5-detect-fill-test-paths +created: 2026-06-26T11:08:50Z +created_by: sahil-noon +change_type: feat +issues: [] +progress: + intake: done + apply: done + review: done + hydrate: done + ship: active + review-pr: pending +plan: + generated: true + task_count: 8 + acceptance_count: 21 + acceptance_completed: 0 +confidence: + certain: 4 + confident: 2 + tentative: 0 + unresolved: 0 + score: 4.8 + fuzzy: true + dimensions: + signal: 80.0 + reversibility: 79.2 + competence: 82.5 + disambiguation: 77.5 +stage_metrics: + intake: {started_at: "2026-06-26T11:08:50Z", driver: fab-new, iterations: 1, completed_at: "2026-06-26T11:12:39Z"} + apply: {started_at: "2026-06-26T11:23:45Z", driver: fab-fff, iterations: 2, completed_at: "2026-06-26T11:24:38Z"} + review: {started_at: "2026-06-26T11:24:38Z", driver: fab-fff, iterations: 2, completed_at: "2026-06-26T11:29:09Z"} + hydrate: {started_at: "2026-06-26T11:29:09Z", driver: fab-fff, iterations: 1, completed_at: "2026-06-26T11:31:14Z"} + ship: {started_at: "2026-06-26T11:31:14Z", driver: fab-fff, iterations: 1} +prs: [] +change_type_source: explicit +true_impact: + added: 0 + deleted: 0 + net: 0 + excluding: + added: 0 + deleted: 0 + net: 0 + tests: + added: 0 + deleted: 0 + net: 0 + computed_at: "2026-06-26T11:31:14Z" + computed_at_stage: hydrate +summary: Detect & fill test_paths at setup from on-disk markers (anchored table) + 2.7.1-to-2.8.0 backfill migration; refresh scaffold example block. +# true_impact: lazily created on first apply-finish (no placeholder here). +last_updated: 2026-06-26T11:31:14Z diff --git a/fab/changes/260626-5qf5-detect-fill-test-paths/intake.md b/fab/changes/260626-5qf5-detect-fill-test-paths/intake.md new file mode 100644 index 00000000..6b6d2c3f --- /dev/null +++ b/fab/changes/260626-5qf5-detect-fill-test-paths/intake.md @@ -0,0 +1,136 @@ +# Intake: Detect & Fill test_paths at Setup + +**Change**: 260626-5qf5-detect-fill-test-paths +**Created**: 2026-06-26 + +## Origin + +> detect and fill test_paths at setup with migration and scaffold examples + +Conversational origin. The discussion traced how `/git-pr`'s impact breakdown splits test vs. non-test code: it is **purely pathspec-driven** (`src/go/fab/internal/impact/impact.go` `Compute` runs a third `git diff --shortstat` pass with the `test_paths` globs as `:(glob)` includes combined with the `true_impact_exclude` patterns). `test_paths` is **language-specific and ships empty** — the scaffold has it commented out (`# test_paths: []`), and the Go config struct has no hardcoded default. When empty, the breakdown collapses to a single total line (no `impl`/`tests` split). + +Key decisions reached in discussion: + +1. **Rejected a case-insensitive `**/*test*` default.** A bare substring miscounts production code that contains "test" (`attestation.go`, `latest.go`, a `TestModeBanner` component) — inflating the test count and deflating `impl`, the opposite of the table's purpose. Case-insensitivity makes it worse (`Latest`, `Contestant`). The classification's reliability comes from *anchoring* to a language's test convention (`*_test.go` suffix, `test_*.py` prefix, `*.spec.ts` infix). +2. **Chosen approach: anchored language detection at setup, non-interactive.** Infer the right anchored pattern from on-disk marker files, fill `test_paths` automatically, surface a visible note rather than prompting. Zero-friction without the substring trap. +3. **Scaffold examples must persist as a standing comment** above the active key (annotated by ecosystem + convention name), so the user retains an editing reference even after a value is written. +4. **A migration backfills existing repos** — runs the same detection and fills `test_paths`, and refreshes the scaffold's comment block, for projects already on fab-kit. + +## Why + +**Problem.** The `/git-pr` impact breakdown is one of the most-read signals in a fab-kit PR body, but its test/impl split silently does nothing unless a project has hand-set `test_paths`. Since the key ships commented-out with no default and setup never asks for it, the overwhelming majority of fab-kit projects get the collapsed single-line breakdown — the richer `impl`/`tests` taxonomy is invisible. fab-kit itself only has the split because a human hand-edited the config. + +**Consequence if unfixed.** The feature stays effectively opt-in-by-expert-knowledge. New projects never discover it; existing projects never gain it. The impact table under-delivers on its stated taxonomy (`raw / true / impl / tests / excluded`) for nearly everyone. + +**Why this approach over alternatives.** +- *A hardcoded default glob* (e.g. `**/*test*`) was rejected — it can't be anchored to a language at config-write time without knowing the language, and an unanchored substring produces a confidently-wrong number (see Origin §1). A wrong-but-confident impact number is worse than an absent one. +- *Asking the user interactively* adds a setup prompt for a value most users won't have an opinion on. The user explicitly asked for non-interactive detection "if possible," and the marker-file signal is strong enough to auto-fill safely. +- *Detection at setup, anchored to the detected ecosystem*, gets zero-config correctness for recognized stacks while leaving the value trivially editable (and the migration re-runnable) if detection guesses wrong. Unrecognized stacks fall back to the standing examples — never a wrong value, just the (current) collapsed breakdown. + +## What Changes + +Four coordinated edits — a skill + scaffold + migration + their doc mirrors. No Go binary change (detection is prompt/skill logic, honoring Constitution I "Pure Prompt Play"; the `impact.go` consumer already handles any non-empty `test_paths` verbatim). + +### 1. Scaffold — persistent example comment block + +`src/kit/scaffold/fab/project/config.yaml`: replace the current `test_paths` block (lines 14–18) so the examples live as a **standing comment above the active key** (so they survive whether the key is filled or left empty), one example per line in YAML-list form, annotated by ecosystem and convention, plus the load-bearing `:(glob)` / anti-substring note: + +```yaml +# Glob/pathspec patterns identifying test files. Used by the true-impact +# breakdown to attribute lines to tests vs. implementation (impl = total − tests). +# Language-specific — no kit default. Patterns are :(glob) magic pathspecs, so +# `**` matches across directories and `*` does NOT match `/`. Anchor to your +# language's test convention rather than a bare substring (a substring like +# `*test*` miscounts production code such as `attestation.go` or `latest.go`). +# When absent/empty, the breakdown collapses to a single total line. +# +# Examples (uncomment/adapt the line for your stack): +# - "**/*_test.go" # Go — `_test.go` suffix +# - "**/test_*.py" # Python (pytest) — `test_` prefix +# - "**/*.spec.ts" # JS/TS (Jest/Vitest) — `.spec` infix +# - "**/*.test.ts" # JS/TS — `.test` infix +# - "**/src/test/**" # Java/Kotlin (Maven/Gradle) — test source root +# test_paths: [] +``` + +When create-mode setup detects and fills a value, it MUST preserve this comment block intact and replace only the `# test_paths: []` line (so the examples persist as an editing reference). The placeholder substitution introduces a new `{TEST_PATHS}` token (see §2 for the create-mode flow). + +### 2. fab-setup skill — non-interactive detection in create mode + +`src/kit/skills/fab-setup.md`, **Config Create-Mode** (current steps at lines 157–166). Detection slots between reading root files (step 1) and writing the config: + +- **Step 2** (currently "Ask the user: project name, description, source paths") gains a **detection sub-step**: after reading root files, apply the language→pattern table below to on-disk marker files and derive `{TEST_PATHS}` automatically. Do NOT prompt for it — the user explicitly wants this non-interactive. +- **Step 4** (placeholder substitution) gains `{TEST_PATHS}` alongside `{PROJECT_NAME}`, `{PROJECT_DESCRIPTION}`, `{SOURCE_PATHS}`. +- **Detection writes the value AND surfaces a note.** When detection fills `test_paths`, the create-mode output (step 7 / Config Output) adds a visible line, e.g.: + `Detected {ecosystem} — set test_paths to {patterns}. Edit via /fab-setup config source_paths if wrong.` + When no ecosystem is recognized, leave the key commented (`# test_paths: []`), the breakdown collapses to one line (today's behavior), and note: `No test convention detected — test_paths left empty (impact breakdown will show a single total). Set it later if desired.` + +**Detection table** (marker file on disk → anchored pattern). Multi-marker repos take the union: + +| Detected marker | Ecosystem | `test_paths` | +|---|---|---| +| `go.mod` | Go | `**/*_test.go` | +| `pytest.ini` / `pyproject.toml` / `setup.cfg` | Python (pytest) | `**/test_*.py`, `**/*_test.py` | +| `package.json` with jest/vitest dep, or `*.spec.ts`/`*.test.ts`/`*.spec.js`/`*.test.js` present | JS/TS | `**/*.spec.ts`, `**/*.test.ts`, `**/*.spec.js`, `**/*.test.js` | +| `pom.xml` / `build.gradle` | Java/Kotlin (Maven/Gradle) | `**/src/test/**` | +| `*.csproj` referencing a test SDK | .NET | `**/*Tests.cs`, `**/*Test.cs` | +| `Cargo.toml` | Rust | *(none — Rust tests are inline `#[cfg(test)]`; not glob-addressable)* → leave empty, note why | +| *(no marker / unrecognized)* | — | leave empty; standing examples remain the reference | + +The Rust row is deliberate: a substring match would "work" there yet be doubly wrong (matches `attestation.rs`, misses the actual inline tests) — so the honest behavior is to leave it empty. + +### 3. Migration — backfill existing repos + +New file `src/kit/migrations/2.7.1-to-2.8.0.md` (next minor; bump `src/kit/VERSION` 2.7.1 → 2.8.0 as part of this change). Follows the established migration shape (cf. `2.2.0-to-2.3.0.md`): markdown instruction file, idempotent, applied by `/fab-setup migrations`. Two effects: + +- **Refresh the scaffold comment block** in the user's existing `fab/project/config.yaml` to match §1 (the annotated example comment + `:(glob)`/anti-substring note), so existing repos get the editing reference even if they keep `test_paths` empty. +- **Detect and fill `test_paths`** by running the §2 detection table against the user's repo — *only when the key is currently absent or empty*. + +**Pre-check / idempotency (Constitution III):** +1. If `fab/project/config.yaml` is absent → `Skipped: fab/project/config.yaml not present.` +2. If `test_paths` is already present and **non-empty** → do NOT overwrite the user's value; still refresh the comment block. (A user who hand-set patterns keeps them.) +3. Use a sentinel comment marker (e.g. the `# Examples (uncomment/adapt the line for your stack):` line) to detect whether the refreshed comment block is already present, so re-running is a no-op on the comment refresh. +4. If detection recognizes no ecosystem → leave `test_paths` empty, refresh the comment only, and report it. + +Report lines mirror the create-mode notes in §2 (detected ecosystem + patterns, or "no convention detected"). + +### 4. Doc mirrors (sweep class) + +Constitution-required and review-must-fix: + +- `docs/specs/skills/SPEC-fab-setup.md` — mirror the create-mode detection sub-step + `{TEST_PATHS}` placeholder (Constitution Additional Constraints: skill change MUST carry SPEC mirror). +- `docs/memory/distribution/setup.md` — document that create-mode now detects/fills `test_paths`. +- `docs/memory/distribution/migrations.md` — note the new `2.7.1-to-2.8.0` migration if that file enumerates migrations (verify at apply). +- Re-run `fab memory-index` after any memory write (byte-stable index). + +## Affected Memory + +- `distribution/setup`: (modify) create-mode now auto-detects and fills `test_paths` from on-disk marker files (non-interactive), with a visible note; unrecognized stacks leave it empty. +- `distribution/migrations`: (modify) record the new `2.7.1-to-2.8.0` backfill migration (detection + comment refresh, idempotent). + +## Impact + +- **Skill**: `src/kit/skills/fab-setup.md` (create-mode steps 2 + 4 + 7 / Config Output). +- **Scaffold**: `src/kit/scaffold/fab/project/config.yaml` (`test_paths` comment block + `{TEST_PATHS}` placeholder). +- **Migration**: new `src/kit/migrations/2.7.1-to-2.8.0.md`; `src/kit/VERSION` bump 2.7.1 → 2.8.0. +- **SPEC mirror**: `docs/specs/skills/SPEC-fab-setup.md`. +- **Memory**: `docs/memory/distribution/setup.md`, `docs/memory/distribution/migrations.md`; regen indexes. +- **No Go change**: `impact.go` already consumes any non-empty `test_paths` verbatim; detection is skill/migration prompt logic per Constitution I. (Therefore no `_cli-fab.md` / Go-test obligation is triggered.) +- **Downstream effect**: new projects (and migrated existing ones) get the `impl`/`tests` impact split automatically for recognized stacks; the impact table delivers its full taxonomy without expert hand-config. + +## Open Questions + +- Confirm at apply whether `docs/memory/distribution/migrations.md` enumerates individual migrations (→ needs the new entry) or only describes the migration *mechanism* (→ no per-migration edit needed). Resolve by reading the file at apply; does not block planning. + +## Assumptions + +| # | Grade | Decision | Rationale | Scores | +|---|-------|----------|-----------|--------| +| 1 | Confident | Detection at setup is non-interactive (auto-fill + visible note), not a prompt | User explicitly asked "non-interactively if possible"; marker-file signal is strong; value is trivially editable and the migration is re-runnable | S:90 R:80 A:80 D:80 | +| 2 | Confident | Anchored language→pattern detection table (Go/Python/JS-TS/Java-Kotlin/.NET; Rust & unknown → empty) | Direction approved in discussion; anchoring to convention is what makes the classification reliable; rejected the unanchored substring | S:80 R:75 A:75 D:75 | +| 3 | Certain | Migration must skip repos with a non-empty `test_paths` and be sentinel-guarded for the comment refresh | Constitution III (idempotent operations); established migration pattern (`2.2.0-to-2.3.0` pre-check shape) | S:85 R:85 A:95 D:90 | +| 4 | Confident | New migration file `2.7.1-to-2.8.0.md` + `VERSION` bump to 2.8.0 (next minor) | Current VERSION is 2.7.1; additive config-only change is a minor bump per the existing migration cadence | S:75 R:80 A:85 D:80 | +| 5 | Confident | No Go binary change — detection is skill/migration prompt logic | Constitution I (Pure Prompt Play: workflow logic in markdown/scripts); `impact.go` already handles any non-empty `test_paths` | S:80 R:75 A:90 D:85 | +| 6 | Tentative | Unrecognized/inline-test stacks (e.g. Rust) leave `test_paths` empty rather than guessing | Rust tests aren't glob-addressable; a guess would be doubly wrong; empty = today's safe collapsed breakdown | S:70 R:80 A:70 D:55 | + +6 assumptions (1 certain, 4 confident, 1 tentative, 0 unresolved). diff --git a/fab/changes/260626-5qf5-detect-fill-test-paths/plan.md b/fab/changes/260626-5qf5-detect-fill-test-paths/plan.md new file mode 100644 index 00000000..22743bec --- /dev/null +++ b/fab/changes/260626-5qf5-detect-fill-test-paths/plan.md @@ -0,0 +1,192 @@ +# Plan: Detect & Fill test_paths at Setup + +**Change**: 260626-5qf5-detect-fill-test-paths +**Intake**: `intake.md` + +## Requirements + +### Scaffold: Standing test_paths Example Comment Block + +#### R1: Persistent annotated example comment above the active key +The scaffold `src/kit/scaffold/fab/project/config.yaml` SHALL present `test_paths` examples as a **standing comment block above the active key** — one example per line in YAML-list form, annotated by ecosystem and convention — plus a `:(glob)` magic-pathspec note and an anti-substring warning. The block SHALL persist whether the key is filled or left empty. A `{TEST_PATHS}` placeholder token SHALL be introduced so create-mode setup can substitute a detected value while preserving the comment block. + +- **GIVEN** a fresh project scaffolded from the kit template +- **WHEN** the user opens `fab/project/config.yaml` +- **THEN** the `test_paths` section shows the annotated examples (Go/Python/JS-TS/Java-Kotlin), the `:(glob)`/anti-substring note, and an active line carrying the `{TEST_PATHS}` placeholder + +#### R2: Filling preserves the comment block +When create-mode substitutes a value for `{TEST_PATHS}`, the standing example comment block MUST remain intact and only the active key line is replaced. + +- **GIVEN** create-mode has detected an ecosystem +- **WHEN** it writes the detected `test_paths` value +- **THEN** the example comment block above the key is preserved verbatim and only the active key line carries the detected patterns + +### Skill: Non-Interactive Detection in fab-setup Create Mode + +#### R3: Non-interactive ecosystem detection derives test_paths +`src/kit/skills/fab-setup.md` Config Create-Mode SHALL gain a detection sub-step (in step 2) that reads on-disk marker files and derives an anchored `test_paths` pattern from a language→pattern table, WITHOUT prompting the user. Multi-marker repos take the union of patterns. + +- **GIVEN** a project being set up that has a `go.mod` on disk +- **WHEN** create-mode runs detection +- **THEN** `{TEST_PATHS}` is derived as `**/*_test.go` with no prompt to the user +- **AND** a project with no recognized marker leaves `test_paths` empty + +#### R4: {TEST_PATHS} placeholder substitution +`src/kit/skills/fab-setup.md` step 4 (placeholder substitution) SHALL include `{TEST_PATHS}` alongside `{PROJECT_NAME}`, `{PROJECT_DESCRIPTION}`, `{SOURCE_PATHS}`. When a value is filled, substitution MUST preserve the standing comment block and replace only the active key line. + +- **GIVEN** detection has derived a pattern set +- **WHEN** step 4 substitutes placeholders +- **THEN** `{TEST_PATHS}` is replaced by the detected patterns and the example comment block is preserved + +#### R5: Visible detection note in Config Output +Detection SHALL surface a visible note in step 7 / Config Output: when filled, `Detected {ecosystem} — set test_paths to {patterns}. Edit via /fab-setup config source_paths if wrong.`; when no convention is recognized, leave the key commented and note `No test convention detected — test_paths left empty (impact breakdown will show a single total). Set it later if desired.` + +- **GIVEN** create-mode detected Python (pytest) +- **WHEN** Config Output is shown +- **THEN** it includes the detected-ecosystem note with the patterns +- **AND** an unrecognized stack shows the "no convention detected" note + +### Migration: Backfill Existing Repos (2.7.1 → 2.8.0) + +#### R6: New migration file refreshes comment block and fills test_paths +A new migration `src/kit/migrations/2.7.1-to-2.8.0.md` SHALL (a) refresh the scaffold comment block in the user's existing `fab/project/config.yaml` to match R1, and (b) detect + fill `test_paths` using the same detection table — ONLY when the key is absent or empty. It MUST follow the established migration shape (Summary / Pre-check / Changes / Verification) per `2.2.0-to-2.3.0.md`. + +- **GIVEN** an existing fab-kit project with `test_paths` absent or empty and a `pom.xml` on disk +- **WHEN** `/fab-setup migrations` applies `2.7.1-to-2.8.0` +- **THEN** the comment block is refreshed and `test_paths` is filled with `**/src/test/**` + +#### R7: Migration idempotency and value preservation (Constitution III) +The migration MUST be idempotent and sentinel-guarded: skip entirely when `config.yaml` is absent; never overwrite a non-empty user `test_paths` (still refresh the comment); use a sentinel comment marker to make the comment refresh a re-run no-op; leave `test_paths` empty when no ecosystem is recognized. Report lines mirror the create-mode notes. + +- **GIVEN** a project that already has a hand-set non-empty `test_paths` +- **WHEN** the migration runs +- **THEN** the user's value is preserved unchanged and only the comment block is refreshed +- **AND** re-running the migration is a complete no-op on the comment refresh (sentinel detected) + +#### R8: VERSION bump to 2.8.0 +`src/kit/VERSION` SHALL be bumped from `2.7.1` to `2.8.0` (next minor) to match the new migration's target version. + +- **GIVEN** the current VERSION is `2.7.1` +- **WHEN** this change ships +- **THEN** `src/kit/VERSION` reads `2.8.0` and a `2.7.1-to-2.8.0.md` migration targets it + +### Doc Mirrors (Sweep Class — Constitution-Required) + +#### R9: SPEC mirror reflects detection + placeholder +`docs/specs/skills/SPEC-fab-setup.md` SHALL mirror the create-mode detection sub-step and the `{TEST_PATHS}` placeholder (Constitution Additional Constraints: a skill change MUST carry its SPEC mirror). + +- **GIVEN** the fab-setup skill gained create-mode detection +- **WHEN** the SPEC mirror is read +- **THEN** the create-mode flow documents reading marker files → deriving `{TEST_PATHS}` (non-interactive) and the `{TEST_PATHS}` placeholder in the substitution list + +#### R10: Memory documents the new behavior and migration +`docs/memory/distribution/setup.md` SHALL document that create-mode now detects/fills `test_paths` (non-interactive, with a visible note; unrecognized stacks left empty). `docs/memory/distribution/migrations.md` SHALL record the new `2.7.1-to-2.8.0` migration (it enumerates individual migrations — confirmed at apply). After any `docs/memory/` write, the byte-stable index SHALL be regenerated via `fab memory-index` and verified with `fab memory-index --check`. + +- **GIVEN** the change touched create-mode and added a migration +- **WHEN** the distribution memory files are read +- **THEN** `setup.md` describes the detection/fill behavior and `migrations.md` lists the `2.7.1-to-2.8.0` migration +- **AND** `fab memory-index --check` passes + +### Non-Goals + +- **No Go binary change** — detection is skill/migration prompt logic (Constitution I); `impact.go` already consumes any non-empty `test_paths` verbatim. No `_cli-fab.md` update and no Go-test obligation are triggered. +- **No interactive prompt for test_paths** — detection is non-interactive by design (user explicitly requested this). +- **No hardcoded default glob** — an unanchored substring (`**/*test*`) was rejected; unrecognized stacks leave the value empty rather than guess. + +### Design Decisions + +1. **Anchored language detection over a default glob**: derive the pattern from on-disk markers anchored to each language's test convention — *Why*: an unanchored substring produces a confidently-wrong impact number (worse than absent); anchoring is what makes the classification reliable — *Rejected*: a case-insensitive `**/*test*` default (miscounts `attestation.go`, `latest.go`). +2. **Standing comment block survives a filled value**: examples live as a comment above the active key, not inline on the key line — *Why*: the user keeps an editing reference even after detection writes a value — *Rejected*: inline-only examples on the key line (lost once the value is written). +3. **Rust / unrecognized → empty, not guessed**: Rust tests are inline `#[cfg(test)]`, not glob-addressable — *Why*: a guess would be doubly wrong (matches `attestation.rs`, misses the real inline tests) — *Rejected*: a substring fallback for unrecognized stacks. + +## Tasks + +### Phase 1: Scaffold + Skill (independent files) + +- [x] T001 [P] Replace the `test_paths` block (lines ~14-18) in `src/kit/scaffold/fab/project/config.yaml` with the standing annotated comment block (verbatim from intake §1) above an active key line carrying the `{TEST_PATHS}` placeholder +- [x] T002 [P] In `src/kit/skills/fab-setup.md` Config Create-Mode: add the non-interactive detection sub-step to step 2 (reproduce the detection table marker→ecosystem→patterns from intake §2), add `{TEST_PATHS}` to step 4's substitution list with the preserve-comment-block note, and add the detected/empty note to step 7 / Config Output + +### Phase 2: Migration + VERSION + +- [x] T003 Create `src/kit/migrations/2.7.1-to-2.8.0.md` following the `2.2.0-to-2.3.0.md` shape (Summary / Pre-check / Changes / Verification): refresh the scaffold comment block (sentinel-guarded on the `# Examples (uncomment/adapt the line for your stack):` line), detect+fill `test_paths` via the §2 table only when absent/empty, preserve a non-empty user value, idempotent, report lines mirroring create-mode +- [x] T004 Bump `src/kit/VERSION` from `2.7.1` to `2.8.0` + +### Phase 3: Doc Mirrors (sweep class) + +- [x] T005 [P] Update `docs/specs/skills/SPEC-fab-setup.md` to mirror the create-mode detection sub-step (read marker files → derive `{TEST_PATHS}`, non-interactive) and the `{TEST_PATHS}` placeholder in the substitution list +- [x] T006 [P] Update `docs/memory/distribution/setup.md` to document create-mode auto-detects/fills `test_paths` (non-interactive, visible note, unrecognized → empty) +- [x] T007 [P] Update `docs/memory/distribution/migrations.md` to record the new `2.7.1-to-2.8.0` migration (enumerated catalog: add the description-frontmatter entry + a dedicated subsection) + +### Phase 4: Index Regeneration + +- [x] T008 Run `fab memory-index` then `fab memory-index --check` to regenerate and verify the byte-stable memory index + +## Execution Order + +- T001 and T002 are independent (different files) — parallelizable. +- T003 depends conceptually on T001 (the migration's comment-block refresh must match the scaffold block) — write T001 first. +- T005, T006, T007 are independent files; T008 must run after T006 and T007 (it regenerates the index over the memory writes). + +## Acceptance + +### Functional Completeness + +- [ ] A-001 R1: The scaffold's `test_paths` section is a standing annotated comment block (Go/Python/JS-TS/Java-Kotlin examples, `:(glob)`/anti-substring note) above an active key line with the `{TEST_PATHS}` placeholder +- [ ] A-002 R2: Substituting `{TEST_PATHS}` preserves the comment block and replaces only the active key line +- [ ] A-003 R3: fab-setup create-mode step 2 has a non-interactive detection sub-step with the marker→ecosystem→pattern table; no test_paths prompt is added +- [ ] A-004 R4: `{TEST_PATHS}` is listed in step 4's placeholder substitution alongside the existing three placeholders, with the preserve-comment-block note +- [ ] A-005 R5: step 7 / Config Output has the detected-ecosystem note and the "no convention detected → empty" note +- [ ] A-006 R6: `src/kit/migrations/2.7.1-to-2.8.0.md` exists with Summary/Pre-check/Changes/Verification, refreshing the comment block and filling test_paths via the detection table +- [ ] A-007 R8: `src/kit/VERSION` reads `2.8.0` +- [ ] A-008 R9: `docs/specs/skills/SPEC-fab-setup.md` mirrors the detection sub-step and `{TEST_PATHS}` placeholder +- [ ] A-009 R10: `docs/memory/distribution/setup.md` and `docs/memory/distribution/migrations.md` document the new behavior and migration + +### Behavioral Correctness + +- [ ] A-010 R3: Detection is non-interactive — the skill derives the value from markers and never prompts for test_paths +- [ ] A-011 R7: The migration preserves a non-empty user test_paths and only refreshes the comment block in that case + +### Scenario Coverage + +- [ ] A-012 R3: Each ecosystem row in the detection table maps a concrete marker to an anchored pattern set (Go→`**/*_test.go`, Python→`**/test_*.py`+`**/*_test.py`, JS/TS→spec/test infixes, Java/Kotlin→`**/src/test/**`, .NET→`**/*Tests.cs`+`**/*Test.cs`, Rust/unknown→empty) +- [ ] A-013 R6: A repo with a recognized marker and absent/empty test_paths gets the pattern filled by the migration + +### Edge Cases & Error Handling + +- [ ] A-014 R7: Migration is idempotent — absent config.yaml → skip; sentinel present → comment-refresh no-op; Rust/unrecognized → empty + note +- [ ] A-015 R3: Multi-marker repos take the union of patterns; Rust (inline tests) is deliberately left empty with a note explaining why + +### Code Quality + +- [ ] A-016 Pattern consistency: The migration follows the existing migration file shape and the scaffold comment matches intake §1 verbatim; canonical sources only (`src/kit/`), never `.claude/skills/` +- [ ] A-017 No unnecessary duplication: The detection table is the single reference reproduced in the skill and referenced by the migration; the scaffold comment block is authored once +- [ ] A-018 Mirror sweep: The skill change carries its SPEC mirror and the two distribution memory files in the same change (Constitution Additional Constraints; code-quality § Sibling & Mirror Sweeps) +- [ ] A-019 Markdown-only / CommonMark: all artifacts are plain markdown/YAML; no Go change, so no `_cli-fab.md`/Go-test obligation + +### Documentation Accuracy + +- [ ] A-020 R9: The SPEC mirror and memory files accurately describe the shipped create-mode detection and migration behavior (no drift from the skill/migration) + +### Cross References + +- [ ] A-021 R10: `fab memory-index --check` passes after the memory writes; memory↔memory links use the bundle-relative `/...` form + +## Notes + +- Check items as you review: `- [x]` +- All acceptance items must pass before `/fab-continue` (hydrate) +- If an item is not applicable, mark checked and prefix with **N/A**: `- [x] A-NNN **N/A**: {reason}` + +## Assumptions + +| # | Grade | Decision | Rationale | Scores | +|---|-------|----------|-----------|--------| +| 1 | Confident | Detection is non-interactive (auto-fill + visible note), not a prompt | User explicitly asked "non-interactively if possible"; marker signal is strong; value trivially editable and migration re-runnable | S:90 R:80 A:80 D:80 | +| 2 | Confident | Anchored language→pattern detection table (Go/Python/JS-TS/Java-Kotlin/.NET; Rust & unknown → empty) | Direction approved in discussion; anchoring is what makes classification reliable | S:80 R:75 A:75 D:75 | +| 3 | Certain | Migration skips non-empty test_paths and is sentinel-guarded for the comment refresh | Constitution III; established `2.2.0-to-2.3.0` / `1.9.1-to-1.9.2` pre-check shape | S:85 R:85 A:95 D:90 | +| 4 | Confident | New migration `2.7.1-to-2.8.0.md` + VERSION bump to 2.8.0 (next minor) | Current VERSION is 2.7.1; additive config-only change is a minor bump per the migration cadence | S:75 R:80 A:85 D:80 | +| 5 | Confident | No Go binary change — detection is skill/migration prompt logic | Constitution I; `impact.go` already handles any non-empty `test_paths` | S:80 R:75 A:90 D:85 | +| 6 | Certain | `migrations.md` enumerates individual migrations → the new entry is required (Open Question resolved) | Read at apply: the file's `description:` lists each migration by range and carries dedicated per-migration subsections (e.g. `### 2.6.6-to-2.7.0`) | S:95 R:80 A:95 D:90 | +| 7 | Tentative | Unrecognized/inline-test stacks (e.g. Rust) leave test_paths empty rather than guessing | Rust tests aren't glob-addressable; a guess would be doubly wrong; empty = today's safe collapsed breakdown | S:70 R:80 A:70 D:55 | + +7 assumptions (2 certain, 4 confident, 1 tentative). diff --git a/src/kit/VERSION b/src/kit/VERSION index 860487ca..834f2629 100644 --- a/src/kit/VERSION +++ b/src/kit/VERSION @@ -1 +1 @@ -2.7.1 +2.8.0 diff --git a/src/kit/migrations/2.7.1-to-2.8.0.md b/src/kit/migrations/2.7.1-to-2.8.0.md new file mode 100644 index 00000000..01b07afc --- /dev/null +++ b/src/kit/migrations/2.7.1-to-2.8.0.md @@ -0,0 +1,138 @@ +# Migration: 2.7.1 to 2.8.0 + +## Summary + +Make the `/git-pr` impact breakdown's test/impl split work for existing +projects by **detecting and filling `test_paths`** in +`fab/project/config.yaml`, and by **refreshing the scaffold's `test_paths` +example comment block** so users keep an editing reference even when the key +stays empty. + +Background: the `/git-pr` impact table attributes lines to tests vs. +implementation (`impl = total − tests`) using the `test_paths` `:(glob)` +patterns. The key ships **language-specific with no kit default**, so most +projects never set it and get the collapsed single-line breakdown. As of +2.8.0, create-mode setup auto-detects the right anchored pattern from on-disk +marker files (`src/kit/skills/fab-setup.md`); this migration backfills the +same detection for projects already on fab-kit. + +This is config-only and additive: there is **no `.status.yaml` schema change** +and **no Go binary change** — `impact.go` already consumes any non-empty +`test_paths` verbatim; detection is pure prompt logic (Constitution I). +Existing non-empty values are preserved; the key is filled only when absent or +empty, and only for a recognized ecosystem. Unrecognized stacks (and inline-test +languages like Rust) are left empty — a confidently-wrong number is worse than +an absent one. + +## Pre-check + +1. Check if `fab/project/config.yaml` exists — if not, skip this migration + entirely. Print: `Skipped: fab/project/config.yaml not present.` + +## Changes + +### 1. Refresh the `test_paths` example comment block (sentinel-guarded) + +Read `fab/project/config.yaml`. The refreshed block carries the sentinel line +`# Examples (uncomment/adapt the line for your stack):`. + +- **If that sentinel line is already present**, the comment block is already + refreshed — skip this step (re-run no-op). Print: + `Skipped: test_paths example comment block already refreshed.` +- **Otherwise**, replace the existing `test_paths` comment/key region (the + contiguous run of `# ...`/`# test_paths:` lines that documents the key, + typically a short note like + `# Glob/pathspec patterns identifying test files. ... # test_paths: []`) + with the standing annotated block below. Preserve the active key line as-is: + if `test_paths` is currently a live key with a value, keep that value line + and place the comment block immediately above it; if it is only the commented + `# test_paths: []` placeholder, that placeholder line is the active line and + is handled by Change 2. + + Insert exactly this comment block (it matches the scaffold at + `$(fab kit-path)/scaffold/fab/project/config.yaml`): + + ```yaml + # Glob/pathspec patterns identifying test files. Used by the true-impact + # breakdown to attribute lines to tests vs. implementation (impl = total − tests). + # Language-specific — no kit default. Patterns are :(glob) magic pathspecs, so + # `**` matches across directories and `*` does NOT match `/`. Anchor to your + # language's test convention rather than a bare substring (a substring like + # `*test*` miscounts production code such as `attestation.go` or `latest.go`). + # When absent/empty, the breakdown collapses to a single total line. + # + # Examples (uncomment/adapt the line for your stack): + # - "**/*_test.go" # Go — `_test.go` suffix + # - "**/test_*.py" # Python (pytest) — `test_` prefix + # - "**/*.spec.ts" # JS/TS (Jest/Vitest) — `.spec` infix + # - "**/*.test.ts" # JS/TS — `.test` infix + # - "**/src/test/**" # Java/Kotlin (Maven/Gradle) — test source root + ``` + + All other top-level sections — `project:`, `source_paths:`, + `true_impact_exclude:`, `checklist:`, `agent:`, `fab_version:`, and any + formatting/comments — SHALL be preserved verbatim. + +### 2. Detect and fill `test_paths` (only when absent or empty) + +After refreshing the comment block: + +1. **Skip the fill when the user already has a value.** If `test_paths` is + present and **non-empty** (a live key with one or more patterns — including a + user-customized list), do NOT overwrite it. Print: + `Skipped: test_paths already set — preserved user value.` + (The comment refresh from Change 1 still applies.) + +2. **Otherwise** (`test_paths` is absent, the commented `# test_paths: []` + placeholder, or an empty `[]`/`null`), **detect the ecosystem** by reading + the on-disk marker files and derive an anchored pattern set from this table. + Multi-marker repos take the **union** of matched pattern sets: + + | Detected marker | Ecosystem | `test_paths` | + |---|---|---| + | `go.mod` | Go | `**/*_test.go` | + | `pytest.ini` / `pyproject.toml` / `setup.cfg` | Python (pytest) | `**/test_*.py`, `**/*_test.py` | + | `package.json` with jest/vitest dep, or `*.spec.ts`/`*.test.ts`/`*.spec.js`/`*.test.js` present | JS/TS | `**/*.spec.ts`, `**/*.test.ts`, `**/*.spec.js`, `**/*.test.js` | + | `pom.xml` / `build.gradle` | Java/Kotlin (Maven/Gradle) | `**/src/test/**` | + | `*.csproj` referencing a test SDK | .NET | `**/*Tests.cs`, `**/*Test.cs` | + | `Cargo.toml` | Rust | *(none — Rust tests are inline `#[cfg(test)]`; not glob-addressable)* → leave empty, note why | + | *(no marker / unrecognized)* | — | leave empty; standing examples remain the reference | + + - **Recognized ecosystem**: replace the active key line (the commented + `# test_paths: []` placeholder, or an empty `test_paths: []`) with a live + YAML-list key carrying the detected patterns, placed directly under the + comment block. Example for a Go repo: + + ```yaml + test_paths: + - "**/*_test.go" + ``` + + Print: `Detected {ecosystem} — set test_paths to {patterns}.` + + - **Unrecognized / inline-test stack (Rust)**: leave `test_paths` empty (keep + the commented `# test_paths: []` placeholder), refresh the comment only. + Print: `No test convention detected — test_paths left empty (impact breakdown will show a single total).` + +Use an atomic write (write to a temp file in the same directory, then `rename` +to `fab/project/config.yaml`) to prevent partial writes if the user has the +file open in an editor. + +## Verification + +1. `fab/project/config.yaml` contains the refreshed `test_paths` example comment + block, including the `# Examples (uncomment/adapt the line for your stack):` + sentinel line. +2. For a project with a recognized marker and a previously absent/empty + `test_paths`, the key is now a live YAML list with the detected anchored + patterns (e.g. `**/*_test.go` for a `go.mod` repo). +3. A project whose `test_paths` was already **non-empty** keeps its value + unchanged (only the comment block was refreshed). +4. A project with no recognized marker (or an inline-test stack like Rust) + keeps `test_paths` empty (the commented `# test_paths: []` placeholder), with + the comment block refreshed. +5. All other top-level keys (`project`, `source_paths`, `true_impact_exclude`, + `checklist`, `agent`, `fab_version`) and their contents are unchanged. +6. Re-running this migration is a complete no-op: Change 1's sentinel trips + (`Skipped: test_paths example comment block already refreshed.`) and Change 2 + sees a now-set or deliberately-empty key. diff --git a/src/kit/scaffold/fab/project/config.yaml b/src/kit/scaffold/fab/project/config.yaml index b7542d24..1710cc8d 100644 --- a/src/kit/scaffold/fab/project/config.yaml +++ b/src/kit/scaffold/fab/project/config.yaml @@ -13,9 +13,19 @@ true_impact_exclude: [fab/, docs/] # Glob/pathspec patterns identifying test files. Used by the true-impact # breakdown to attribute lines to tests vs. implementation (impl = total − tests). -# Language-specific — no kit default. When absent/empty, the breakdown collapses -# to a single total line. Examples: "**/*_test.go", "test_*.py", "**/*.spec.ts". -# test_paths: [] +# Language-specific — no kit default. Patterns are :(glob) magic pathspecs, so +# `**` matches across directories and `*` does NOT match `/`. Anchor to your +# language's test convention rather than a bare substring (a substring like +# `*test*` miscounts production code such as `attestation.go` or `latest.go`). +# When absent/empty, the breakdown collapses to a single total line. +# +# Examples (uncomment/adapt the line for your stack): +# - "**/*_test.go" # Go — `_test.go` suffix +# - "**/test_*.py" # Python (pytest) — `test_` prefix +# - "**/*.spec.ts" # JS/TS (Jest/Vitest) — `.spec` infix +# - "**/*.test.ts" # JS/TS — `.test` infix +# - "**/src/test/**" # Java/Kotlin (Maven/Gradle) — test source root +{TEST_PATHS} # The review stage validates code against the plan.md ## Acceptance section. # Built-in categories: functional_completeness, behavioral_correctness, diff --git a/src/kit/skills/fab-setup.md b/src/kit/skills/fab-setup.md index 8f6a3614..4f50279e 100644 --- a/src/kit/skills/fab-setup.md +++ b/src/kit/skills/fab-setup.md @@ -157,12 +157,32 @@ Create a new `fab/project/config.yaml` interactively or update specific sections When that trigger holds: 1. Read the project's README, package.json, or other root-level files for context -2. Ask the user: project name, description, source paths +2. Ask the user: project name, description, source paths. Then **detect `test_paths` non-interactively** (do NOT prompt for it — the value is inferred from on-disk marker files): + - **Detection sub-step**: read the on-disk marker files and derive an anchored `test_paths` pattern from the table below. Multi-marker repos take the **union** of matched pattern sets. The anchoring (suffix/prefix/infix/source-root) is what makes the test/impl classification reliable — never substitute a bare substring like `**/*test*` (it miscounts production code such as `attestation.go` or `latest.go`). + + | Detected marker | Ecosystem | `test_paths` | + |---|---|---| + | `go.mod` | Go | `**/*_test.go` | + | `pytest.ini` / `pyproject.toml` / `setup.cfg` | Python (pytest) | `**/test_*.py`, `**/*_test.py` | + | `package.json` with jest/vitest dep, or `*.spec.ts`/`*.test.ts`/`*.spec.js`/`*.test.js` present | JS/TS | `**/*.spec.ts`, `**/*.test.ts`, `**/*.spec.js`, `**/*.test.js` | + | `pom.xml` / `build.gradle` | Java/Kotlin (Maven/Gradle) | `**/src/test/**` | + | `*.csproj` referencing a test SDK | .NET | `**/*Tests.cs`, `**/*Test.cs` | + | `Cargo.toml` | Rust | *(none — Rust tests are inline `#[cfg(test)]`; not glob-addressable)* → leave empty, note why | + | *(no marker / unrecognized)* | — | leave empty; standing examples remain the reference | + + Record the detected ecosystem + pattern set (or "no convention detected") for the substitution in step 4 and the note in step 7. 3. Read `$(fab kit-path)/scaffold/fab/project/config.yaml` as the starting template -4. Substitute placeholders with user-provided values: `{PROJECT_NAME}`, `{PROJECT_DESCRIPTION}`, `{SOURCE_PATHS}` +4. Substitute placeholders with user-provided values: `{PROJECT_NAME}`, `{PROJECT_DESCRIPTION}`, `{SOURCE_PATHS}`, `{TEST_PATHS}`. **For `{TEST_PATHS}`**: the scaffold carries a standing example comment block above the active key — preserve it intact and replace only the `{TEST_PATHS}` line. When detection matched an ecosystem, substitute the active key with the detected list, e.g.: + ```yaml + test_paths: + - "**/*_test.go" + ``` + When no ecosystem was recognized (or the stack uses inline tests like Rust), substitute the commented placeholder line `# test_paths: []` so the key stays empty and the breakdown collapses to a single total (today's behavior). 5. **Preserve `fab_version`**: if the existing config.yaml has a `fab_version` key (e.g., written by `fab init`), carry it into the new file unchanged — the scaffold template lacks it and the fab router errors without it. **Fallback (fresh create)**: if no `fab_version` key exists (no prior config.yaml at all), stamp the engine version into the new file — `fab_version: "$(cat "$(fab kit-path)/VERSION")"` — so the guarantee that `fab_version` exists after step 1a holds on every path 6. Write the result to `fab/project/config.yaml` -7. Output: `Created fab/project/config.yaml` +7. Output: `Created fab/project/config.yaml`, then a **test_paths detection note**: + - **Detected**: `Detected {ecosystem} — set test_paths to {patterns}. Edit fab/project/config.yaml if wrong.` + - **Not detected**: `No test convention detected — test_paths left empty (impact breakdown will show a single total). Set it later if desired.` ### Config Update Mode — Menu Flow @@ -201,7 +221,7 @@ If no changes made, output: `No changes made. config.yaml unchanged.` ### Config Output -Show `Created fab/project/config.yaml` (create mode), `{N} sections updated in fab/project/config.yaml` (update mode), or `No changes made` (no-op). Next steps: `/fab-new` after create. +Show `Created fab/project/config.yaml` (create mode), `{N} sections updated in fab/project/config.yaml` (update mode), or `No changes made` (no-op). In create mode, follow `Created` with the **test_paths detection note** (per Config Create Mode step 7 — detected ecosystem + patterns, or "no test convention detected → left empty"). Next steps: `/fab-new` after create. ### Config Error Handling From dc121ef6ebff0f156cb3c20b5300f932265cb7e0 Mon Sep 17 00:00:00 2001 From: Sahil Ahuja Date: Fri, 26 Jun 2026 17:02:50 +0530 Subject: [PATCH 2/4] Update ship status and record PR URL --- .../.history.jsonl | 1 + .../260626-5qf5-detect-fill-test-paths/.status.yaml | 12 +++++++----- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/fab/changes/260626-5qf5-detect-fill-test-paths/.history.jsonl b/fab/changes/260626-5qf5-detect-fill-test-paths/.history.jsonl index ce4db174..e1faa076 100644 --- a/fab/changes/260626-5qf5-detect-fill-test-paths/.history.jsonl +++ b/fab/changes/260626-5qf5-detect-fill-test-paths/.history.jsonl @@ -11,3 +11,4 @@ {"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"hydrate","ts":"2026-06-26T11:29:09Z"} {"event":"review","result":"passed","ts":"2026-06-26T11:29:09Z"} {"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"ship","ts":"2026-06-26T11:31:14Z"} +{"action":"enter","driver":"git-pr","event":"stage-transition","stage":"review-pr","ts":"2026-06-26T11:32:46Z"} diff --git a/fab/changes/260626-5qf5-detect-fill-test-paths/.status.yaml b/fab/changes/260626-5qf5-detect-fill-test-paths/.status.yaml index 493075ad..b29d08d8 100644 --- a/fab/changes/260626-5qf5-detect-fill-test-paths/.status.yaml +++ b/fab/changes/260626-5qf5-detect-fill-test-paths/.status.yaml @@ -9,8 +9,8 @@ progress: apply: done review: done hydrate: done - ship: active - review-pr: pending + ship: done + review-pr: active plan: generated: true task_count: 8 @@ -33,8 +33,10 @@ stage_metrics: apply: {started_at: "2026-06-26T11:23:45Z", driver: fab-fff, iterations: 2, completed_at: "2026-06-26T11:24:38Z"} review: {started_at: "2026-06-26T11:24:38Z", driver: fab-fff, iterations: 2, completed_at: "2026-06-26T11:29:09Z"} hydrate: {started_at: "2026-06-26T11:29:09Z", driver: fab-fff, iterations: 1, completed_at: "2026-06-26T11:31:14Z"} - ship: {started_at: "2026-06-26T11:31:14Z", driver: fab-fff, iterations: 1} -prs: [] + ship: {started_at: "2026-06-26T11:31:14Z", driver: fab-fff, iterations: 1, completed_at: "2026-06-26T11:32:46Z"} + review-pr: {started_at: "2026-06-26T11:32:46Z", driver: git-pr, iterations: 1} +prs: + - https://github.com/sahil87/fab-kit/pull/450 change_type_source: explicit true_impact: added: 0 @@ -52,4 +54,4 @@ true_impact: computed_at_stage: hydrate summary: Detect & fill test_paths at setup from on-disk markers (anchored table) + 2.7.1-to-2.8.0 backfill migration; refresh scaffold example block. # true_impact: lazily created on first apply-finish (no placeholder here). -last_updated: 2026-06-26T11:31:14Z +last_updated: 2026-06-26T11:32:46Z From 94065da1e3074b61665577a8cab2bb2e6280c934 Mon Sep 17 00:00:00 2001 From: Sahil Ahuja Date: Fri, 26 Jun 2026 17:08:40 +0530 Subject: [PATCH 3/4] fix: address review feedback from @Copilot --- docs/memory/_shared/configuration.md | 2 +- docs/specs/skills/SPEC-fab-setup.md | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/memory/_shared/configuration.md b/docs/memory/_shared/configuration.md index ff9e596a..200040fa 100644 --- a/docs/memory/_shared/configuration.md +++ b/docs/memory/_shared/configuration.md @@ -82,7 +82,7 @@ Optional top-level field (7t5a). A YAML sequence of glob/pathspec patterns ident **Purpose — attribution, NOT exclusion.** `test_paths` does NOT strip lines from the impact universe — it *attributes* the already-scaffolding-excluded lines to tests vs. implementation, so reviewers see the split (e.g. "140 impl / 400 test") instead of one conflated number. This is orthogonal to `true_impact_exclude`, which removes scaffolding/doc *noise* from the universe entirely. Tests are first-class deliverables (constitution principle VII, Test Integrity), not scaffolding noise — adding test patterns to `true_impact_exclude` was explicitly rejected because it would conflate the two axes and destroy the test-coverage signal. -**No kit default (Portability, constitution principle V).** fab-kit is language-agnostic; there is no universal test-file pattern (`*_test.go`, `test_*.py`, `*.spec.ts` all differ by language), so the kit MUST NOT ship a hardcoded default `test_paths` list. Instead, the value is **detected per project at setup**: the scaffold (`src/kit/scaffold/fab/project/config.yaml`) ships a standing annotated example comment block (Go/Python/JS-TS/Java-Kotlin patterns + the `:(glob)`/anti-substring note) above a `{TEST_PATHS}` placeholder, and `/fab-setup config` create-mode auto-detects the project's ecosystem from on-disk marker files (`go.mod`→Go, `pyproject.toml`→Python, etc.) and fills the anchored pattern **non-interactively**. Unrecognized or inline-test stacks (e.g. Rust) are left empty — the breakdown then collapses to a single total. Existing projects are backfilled by the `2.7.1-to-2.8.0` migration (same detection; skips projects that already set `test_paths`). This repo dogfoods it with `test_paths: ["**/*_test.go"]`. +**No kit default (Portability, constitution principle V).** fab-kit is language-agnostic; there is no universal test-file pattern (`*_test.go`, `test_*.py`, `*.spec.ts` all differ by language), so the kit MUST NOT ship a hardcoded default `test_paths` list. Instead, the value is **detected per project at setup**: the scaffold (`$(fab kit-path)/scaffold/fab/project/config.yaml`) ships a standing annotated example comment block (Go/Python/JS-TS/Java-Kotlin patterns + the `:(glob)`/anti-substring note) above a `{TEST_PATHS}` placeholder, and `/fab-setup config` create-mode auto-detects the project's ecosystem from on-disk marker files (`go.mod`→Go, `pyproject.toml`→Python, etc.) and fills the anchored pattern **non-interactively**. Unrecognized or inline-test stacks (e.g. Rust) are left empty — the breakdown then collapses to a single total. Existing projects are backfilled by the `2.7.1-to-2.8.0` migration (same detection; skips projects that already set `test_paths`). This repo dogfoods it with `test_paths: ["**/*_test.go"]`. **Glob/pathspec format.** Include patterns are applied as `:(glob)` magic pathspecs in the test-only `git diff --shortstat` pass, so wildcards behave like `.gitignore`-style globs — notably `**` matches across directory boundaries, so `**/*_test.go` matches both root-level `foo_test.go` and nested `pkg/foo_test.go`. (Without `:(glob)`, plain git pathspec rules treat `*` as not crossing `/`, silently missing root-level test files and under-counting tests.) The pass combines these includes with the same `:(exclude)` args derived from `true_impact_exclude`, so tests are counted within the scaffolding-excluded universe; when `true_impact_exclude` is empty, the pass runs with the includes alone (tests attributed within the raw universe). diff --git a/docs/specs/skills/SPEC-fab-setup.md b/docs/specs/skills/SPEC-fab-setup.md index 0624b831..ce86ffce 100644 --- a/docs/specs/skills/SPEC-fab-setup.md +++ b/docs/specs/skills/SPEC-fab-setup.md @@ -37,8 +37,9 @@ User invokes /fab-setup [subcommand] │ │ │ marker files → derive an anchored {TEST_PATHS} pattern from │ │ │ the marker→ecosystem table: go.mod→**/*_test.go, │ │ │ pyproject.toml/pytest.ini→**/test_*.py+**/*_test.py, -│ │ │ jest/vitest→**/*.spec|test.ts|js, pom.xml/build.gradle→ -│ │ │ **/src/test/**, *.csproj→**/*Tests.cs+**/*Test.cs; +│ │ │ jest/vitest→**/*.spec.ts+**/*.test.ts+**/*.spec.js+**/*.test.js, +│ │ │ pom.xml/build.gradle→**/src/test/**, +│ │ │ *.csproj→**/*Tests.cs+**/*Test.cs; │ │ │ Rust/unrecognized → leave empty. NO prompt. Substituted as │ │ │ the {TEST_PATHS} placeholder in step 4, preserving the │ │ │ scaffold's standing example comment block; visible note in From 038abbc625fe20036f3cc69a1436b587b6d8cecc Mon Sep 17 00:00:00 2001 From: Sahil Ahuja Date: Fri, 26 Jun 2026 17:09:19 +0530 Subject: [PATCH 4/4] Update review-pr status --- .../260626-5qf5-detect-fill-test-paths/.history.jsonl | 1 + fab/changes/260626-5qf5-detect-fill-test-paths/.status.yaml | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/fab/changes/260626-5qf5-detect-fill-test-paths/.history.jsonl b/fab/changes/260626-5qf5-detect-fill-test-paths/.history.jsonl index e1faa076..eb18cfa3 100644 --- a/fab/changes/260626-5qf5-detect-fill-test-paths/.history.jsonl +++ b/fab/changes/260626-5qf5-detect-fill-test-paths/.history.jsonl @@ -12,3 +12,4 @@ {"event":"review","result":"passed","ts":"2026-06-26T11:29:09Z"} {"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"ship","ts":"2026-06-26T11:31:14Z"} {"action":"enter","driver":"git-pr","event":"stage-transition","stage":"review-pr","ts":"2026-06-26T11:32:46Z"} +{"event":"review","result":"passed","ts":"2026-06-26T11:39:09Z"} diff --git a/fab/changes/260626-5qf5-detect-fill-test-paths/.status.yaml b/fab/changes/260626-5qf5-detect-fill-test-paths/.status.yaml index b29d08d8..20ab87c3 100644 --- a/fab/changes/260626-5qf5-detect-fill-test-paths/.status.yaml +++ b/fab/changes/260626-5qf5-detect-fill-test-paths/.status.yaml @@ -10,7 +10,7 @@ progress: review: done hydrate: done ship: done - review-pr: active + review-pr: done plan: generated: true task_count: 8 @@ -34,7 +34,7 @@ stage_metrics: review: {started_at: "2026-06-26T11:24:38Z", driver: fab-fff, iterations: 2, completed_at: "2026-06-26T11:29:09Z"} hydrate: {started_at: "2026-06-26T11:29:09Z", driver: fab-fff, iterations: 1, completed_at: "2026-06-26T11:31:14Z"} ship: {started_at: "2026-06-26T11:31:14Z", driver: fab-fff, iterations: 1, completed_at: "2026-06-26T11:32:46Z"} - review-pr: {started_at: "2026-06-26T11:32:46Z", driver: git-pr, iterations: 1} + review-pr: {started_at: "2026-06-26T11:32:46Z", driver: git-pr, iterations: 1, completed_at: "2026-06-26T11:39:09Z"} prs: - https://github.com/sahil87/fab-kit/pull/450 change_type_source: explicit @@ -54,4 +54,4 @@ true_impact: computed_at_stage: hydrate summary: Detect & fill test_paths at setup from on-disk markers (anchored table) + 2.7.1-to-2.8.0 backfill migration; refresh scaffold example block. # true_impact: lazily created on first apply-finish (no placeholder here). -last_updated: 2026-06-26T11:32:46Z +last_updated: 2026-06-26T11:39:09Z