Skip to content

Commit e807082

Browse files
Merge pull request #6 from nonlinear-xyz/feat/factory-verification-score
Feat/factory verification score
2 parents b5ca4ca + 67d37f0 commit e807082

26 files changed

Lines changed: 1099 additions & 47 deletions

.factory-check.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"ignorePaths": ["**/__tests__/**", "check/rules/**"]
3+
}
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
# Factory conformance — the kit dogfooding its own checker.
2+
#
3+
# This is NOT the shipped template (templates/factory-conformance.yml, which
4+
# npx's the published package). The kit IS the package, so it runs the checker
5+
# from source: npm ci → build → node bin/factory-kit-check.js. Same contract —
6+
# sticky scorecard, gate only on a new critical — proven against itself.
7+
name: Factory conformance
8+
9+
on:
10+
pull_request:
11+
12+
permissions:
13+
contents: read
14+
pull-requests: write
15+
16+
jobs:
17+
conformance:
18+
runs-on: ubuntu-latest
19+
steps:
20+
- uses: actions/checkout@v4
21+
with:
22+
fetch-depth: 0 # full history so the base ref resolves for the delta
23+
24+
- uses: actions/setup-node@v4
25+
with:
26+
node-version: 20
27+
cache: npm
28+
29+
- run: npm ci
30+
- run: npm run build
31+
32+
- name: Run factory-kit-check on itself
33+
run: |
34+
set -o pipefail
35+
node bin/factory-kit-check.js . \
36+
--base "origin/${{ github.base_ref }}" --md > scorecard.md \
37+
&& echo "GATE=pass" >> "$GITHUB_ENV" \
38+
|| echo "GATE=block" >> "$GITHUB_ENV"
39+
cat scorecard.md
40+
41+
- name: Post sticky scorecard
42+
uses: actions/github-script@v7
43+
with:
44+
script: |
45+
const fs = require('fs');
46+
const body = fs.readFileSync('scorecard.md', 'utf8');
47+
const marker = '<!-- factory-kit-check:scorecard -->';
48+
const { owner, repo } = context.repo;
49+
const issue_number = context.issue.number;
50+
const { data: comments } = await github.rest.issues.listComments({ owner, repo, issue_number });
51+
const existing = comments.find((c) => c.body && c.body.includes(marker));
52+
if (existing) {
53+
await github.rest.issues.updateComment({ owner, repo, comment_id: existing.id, body });
54+
} else {
55+
await github.rest.issues.createComment({ owner, repo, issue_number, body });
56+
}
57+
58+
- name: Enforce delta gate
59+
if: env.GATE == 'block'
60+
run: |
61+
echo "::error::This PR introduces a new critical-severity finding. See the conformance scorecard on the PR."
62+
exit 1

CLAUDE.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ Skills auto-load on the `factory-*` namespace:
3535
- **factory-ci** — single `ci.yml` merge gate, ephemeral PR DB, coverage floor, `anthropics/claude-code-action@v1` as required check, branch protection
3636
- **factory-commits** — Conventional Commits + required Linear issue ID; commitlint config, Husky hook, opencommit wiring
3737
- **factory-pitfalls** — flat cross-skill index of Failure mode blocks + process-level pitfalls without a skill home
38+
- **factory-verification** — four-tier eval spectrum (CLI rule / test / agent / human gate), evals-graduate-downward promotion pipeline, banded conformance score with severity-aware coverage disclosure, delta-gated GitHub Action that guards the PR boundary not the inner loop
3839

3940
## Specialist subagents (callable via Agent tool)
4041

@@ -49,6 +50,7 @@ Skills auto-load on the `factory-*` namespace:
4950
- **llm-workflow-engineer** — LangGraph workflows, RAG, structured output, streaming
5051
- **security-engineer** — threat modeling, AI-code review, sensitive-data handling
5152
- **code-reviewer** — PR review against factory-pitfalls digest
53+
- **verification-engineer** — designs the verification strategy for a change (blast radius → required eval tiers → gaps); sister to `code-reviewer` (finds defects) and generalization of `db-migration-engineer`'s verify-stage to all changes
5254

5355

5456
## Slash commands (auto-loaded into `~/.claude/commands/`)

README.md

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ factory-kit-check # check the current directory
6060
factory-kit-check ../some-repo # check another repo
6161
```
6262

63-
It is **read-only** — it reads and judges, it never writes to your code. It exits non-zero on any `critical`/`high` finding, so it drops straight into a pre-push hook or CI step.
63+
It is **read-only** — it reads and judges, it never writes to your code. By default it exits non-zero on any `critical`/`high` finding; with `--base <ref>` it gates on the *delta* instead (see the conformance gate below). Run it on demand locally, or as the PR-boundary Action — not as a blocking inner-loop hook (see `factory-verification.md §Guardrails at the boundary`).
6464

6565
Why deterministic (not an LLM): the rules are cheap, reproducible, and cost nothing per run, so you can run them on every save without thinking about it. Each rule is greppable code with a documented heuristic and a citation — no black box.
6666

@@ -77,15 +77,34 @@ Why deterministic (not an LLM): the rules are cheap, reproducible, and cost noth
7777

7878
Detection is regex/line-heuristic in v0 — precision-first, tuned against real repos. Rules are language-tagged (TS today; the seam for a Python `ast` sidecar is in place) and the report footer lists how many known pitfalls are not yet covered, so the tool never implies full coverage.
7979

80+
### Score & coverage
81+
82+
The report leads with a **banded verdict**`pass` / `warn` / `fail`, severity-gated (one critical ⇒ fail; one high ⇒ warn). The precision of the verdict matches the precision of the instrument: a band, never a false-precision number. Alongside it, **coverage** — how many of the ~40 named pitfalls are machine-checked — with a severity-aware caveat naming any critical-class pitfall that has *no* rule, so a passing grade can't impersonate "nothing critical is wrong." The model lives in `check/score.ts` (pure, tested); the doctrine is `factory-verification.md`.
83+
84+
### Conformance gate (GitHub Action)
85+
86+
```sh
87+
factory-kit-check . --base origin/main --md # render the PR scorecard
88+
npx @nonlinear-labs/factory-kit add-ci # drop the Action into a repo
89+
```
90+
91+
The `Factory conformance` Action posts one sticky scorecard comment per PR and fails the check **only on a newly-introduced critical** — it gates the *delta*, not the absolute, so pre-existing debt never blocks a PR and the developer's inner loop is never interrupted. Tighten to new-highs with `"gateOnHigh": true` in `.factory-check.json`. With `--base`, the CLI exit code follows this delta gate; without it, the legacy whole-repo critical/high gate applies, so it still drops into a simple CI step.
92+
8093
### Configure
8194

82-
Drop a `.factory-check.json` in the repo to disable a rule:
95+
Drop a `.factory-check.json` in the repo to disable a rule, ignore paths, or tighten the gate:
8396

8497
```json
85-
{ "disabledRules": ["update-delete-no-where"] }
98+
{
99+
"disabledRules": ["update-delete-no-where"],
100+
"ignorePaths": ["**/__tests__/**"],
101+
"gateOnHigh": false
102+
}
86103
```
87104

88-
The report prints how many rules are disabled. If you find yourself disabling more than a handful, the rule design is wrong — open an issue, don't paper over it.
105+
- `disabledRules` blinds a rule across the whole repo. The report prints how many are disabled; if you disable more than a handful, the rule design is wrong — open an issue, don't paper over it.
106+
- `ignorePaths` (fast-glob, relative to repo root) excludes paths from the walk — e.g. a rule suite's own test files, fixtures, and rule-definition sources, which contain the very patterns they detect (as test bait or as detection heuristics). Path exclusion scopes *where* rules apply without blinding the rule itself. The kit ships this exact config to skip its own `__tests__/` tree and `check/rules/`.
107+
- `gateOnHigh` tightens the PR delta gate to also block a newly-introduced high (default: new-critical only).
89108

90109
### From source
91110

@@ -122,6 +141,7 @@ Synthesized cross-build conventions. Auto-loaded by Claude Code from `~/.claude/
122141
| `factory-ci` | Single `ci.yml` merge gate, ephemeral PR DB, coverage floor, Claude Code reviewer as required check |
123142
| `factory-commits` | Conventional Commits + required Linear-ID; commitlint config |
124143
| `factory-pitfalls` | Flat cross-skill index of Failure mode blocks + process-level pitfalls without a skill home |
144+
| `factory-verification` | Four-tier eval spectrum, evals-graduate-downward pipeline, banded conformance score with coverage disclosure, delta-gated conformance Action |
125145

126146
### Agents (specialist subagents)
127147

@@ -139,7 +159,8 @@ Each is a Claude Code subagent file (YAML frontmatter + markdown body). Callable
139159
| `data-pipeline-engineer` | CSV ingestion, Python services, simulation envelopes |
140160
| `llm-workflow-engineer` | LangGraph workflows, RAG, structured output, streaming |
141161
| `security-engineer` | Threat-model a feature, audit AI-generated code, sensitive-data handling |
142-
| `code-reviewer` | PR review against `factory-pitfalls.md` checklist |
162+
| `code-reviewer` | PR review against `factory-pitfalls.md` checklist — finds defects |
163+
| `verification-engineer` | Designs the verification strategy for a change (blast radius → eval tiers → gaps); sister to `code-reviewer`, generalizes the migration verify-stage |
143164

144165
### Slash commands
145166

agents/verification-engineer.md

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
---
2+
name: verification-engineer
3+
description: Use to design the verification strategy for a change — what would prove it correct, given its blast radius, and what is currently unverifiable. Read-only — outputs a verification plan and a gap list, not a review and not code. Sister to `code-reviewer` (which finds defects in a diff) and the generalization of `db-migration-engineer`'s verify-stage discipline to all changes. Carries the four-tier eval spectrum and the score model from `factory-verification.md`. Invoke before merging a nontrivial change, when onboarding a risky area, or when asking "how would we know this is right?"
4+
tools: Read, Grep, Glob, Bash
5+
model: sonnet
6+
---
7+
8+
You are the **verification-engineer** subagent. Your job is to design *how a change would be proven correct* — and to name what currently can't be. You **do not write or edit code, and you do not hunt for defects** (that's `code-reviewer`). Your output is a verification strategy: which tiers of the eval spectrum apply, which are satisfied, and where the gaps are. Read `~/.claude/skills/factory-verification.md` first.
9+
10+
The distinction from `code-reviewer` is load-bearing:
11+
- `code-reviewer` answers *"what's wrong in this diff?"* — it finds defects.
12+
- **You** answer *"what would establish this diff is right, and what part of that is missing?"* — you design the proof.
13+
14+
A change can pass a defect review and still be unverified — no test exercises the new branch, no human can explain the tricky part, nothing checks the convention it relies on. That gap is your subject.
15+
16+
## How to think (in order)
17+
18+
1. **What's the scope and the blast radius?** Identify the change (files / diff range / feature). Then classify its blast radius — this sets how much verification is warranted:
19+
- **Cosmetic** — copy, styling, docs. Reversible, no behavior change.
20+
- **Behavioral** — logic, control flow, a new code path.
21+
- **Data-shape** — schema, migration, anything that changes persisted state.
22+
- **Destructive / irreversible** — prod data mutation, deletion, anything you can't trivially undo.
23+
24+
If scope is ambiguous, ask. Don't strategize the whole repo by default.
25+
26+
2. **Map the change onto the four-tier spectrum.** For each tier, state what it *would* take to verify this change, and whether that's present:
27+
- **CLI rule** (static inspection): does `factory-kit-check` cover the conventions this change touches? Run it (`--diff`/`--base`) and read the band + coverage. Note any relevant pitfall that sits in `UNCOVERED` — that's a known blind spot for this change.
28+
- **Test** (execution): is there a test that exercises the new behavior — not just that it compiles, but that it does the right thing? New branch with no new test = a gap. Check `__tests__/` co-location.
29+
- **Agent** (judgment): is there a question here only judgment answers — right abstraction, missing case, security reasoning — that no rule or test will catch?
30+
- **Human gate** (comprehension): is there a part of this change a human must be able to explain to own it? Flag the spots where "the checks passed" is not enough.
31+
32+
3. **Match required tiers to blast radius.** Cosmetic needs tiers 1–2 at most. Behavioral needs 1–3. Data-shape and destructive need all four, human gate last — for destructive prod writes, defer to `db-migration-engineer`'s preflight/mutate/verify/rollback runbook; your job there is to confirm that discipline is being followed, not to re-derive it.
33+
34+
4. **Find the gaps — this is the core output.** The hardest and most valuable findings are about verification that *should* exist and doesn't:
35+
- New behavior with no test exercising it.
36+
- A convention the change depends on that nothing enforces (a silent verification gap — name it; it may be a rule-promotion candidate).
37+
- A tricky section no human has signed off on comprehending.
38+
- A critical-class pitfall in the change's area that `factory-kit-check` doesn't cover.
39+
40+
5. **Name rule-promotion candidates.** If a gap is decidable by static inspection and likely to recur, say so explicitly: it should graduate into a `check/rules/` rule (see `factory-verification.md §Evals graduate downward`). This feeds the factory's backlog.
41+
42+
6. **Don't gold-plate.** Match verification cost to blast radius. Demanding an E2E test for a copy change is the same failure as shipping a migration with no rollback — verification spent out of proportion to the risk. Say what's *enough*, not the maximum.
43+
44+
## Output format
45+
46+
```
47+
## Change under verification
48+
- Scope: <files / diff range; count>
49+
- Blast radius: <cosmetic | behavioral | data-shape | destructive>
50+
51+
## Verification strategy (by tier)
52+
- CLI rule: <covered / partial / gap> — <what factory-kit-check says; relevant UNCOVERED items>
53+
- Test: <covered / partial / gap> — <what exists; what behavior is unexercised>
54+
- Agent: <needed / not needed> — <the judgment question, if any>
55+
- Human gate: <needed / not needed> — <the section that must be comprehended, if any>
56+
57+
## Gaps (what is currently unverifiable)
58+
1. <gap> — <why it matters> — <which tier would close it>
59+
60+
## Rule-promotion candidates
61+
- <gap decidable by static inspection that should become a check/rules/ rule>, citing the factory-pitfalls.md / skill section it would enforce
62+
63+
## Verdict
64+
<the minimum verification this change needs before merge, proportional to blast radius — and what's missing from it today>
65+
```
66+
67+
## What you do NOT do
68+
69+
- **Don't write or edit code, tests, or rules.** You design the strategy; another agent or the contributor implements it.
70+
- **Don't hunt for defects.** Bugs in the diff are `code-reviewer`'s job; if you spot one in passing, note it and hand it off — don't pivot into a line-by-line review.
71+
- **Don't strategize the whole repo by default.** Honor scope.
72+
- **Don't demand maximum verification.** Proportional to blast radius — over-verifying a cosmetic change is as wrong as under-verifying a destructive one.
73+
- **Don't treat green checks as sufficient.** Your value is naming what the passing checks *don't* prove.
74+
- **Don't run non-read-only commands.** `factory-kit-check`, `git diff`, test runs are fine; never mutate the repo or any database.
75+
76+
## When the request is too small for this framework
77+
78+
If the user asks "does this one-line copy fix need anything?" answer directly: no, tier 1 covers it. The framework is for behavioral-and-above changes where the verification strategy is a real decision.

bin/factory-kit-check.js

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,26 @@ async function main() {
2424
return;
2525
}
2626

27-
// First positional arg is the target repo; default to cwd.
28-
const target = process.argv[2] ? path.resolve(process.argv[2]) : process.cwd();
29-
const exitCode = await run({ targetDir: target });
27+
// Parse: one optional positional (target repo, default cwd) plus flags.
28+
// --base <ref> diff against <ref>: adds the PR delta + new-critical gate
29+
// --diff scope reported findings to files changed vs --base
30+
// --json emit machine-readable score+delta
31+
// --md emit the Markdown scorecard (for the PR comment)
32+
const argv = process.argv.slice(2);
33+
let target = null;
34+
let base;
35+
let diffOnly = false;
36+
let format = "term";
37+
for (let i = 0; i < argv.length; i++) {
38+
const a = argv[i];
39+
if (a === "--base") base = argv[++i];
40+
else if (a === "--diff") diffOnly = true;
41+
else if (a === "--json") format = "json";
42+
else if (a === "--md") format = "md";
43+
else if (!a.startsWith("--") && target === null) target = a;
44+
}
45+
const targetDir = target ? path.resolve(target) : process.cwd();
46+
const exitCode = await run({ targetDir, base, diffOnly, format });
3047
process.exit(exitCode);
3148
}
3249

bin/factory-kit.js

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,9 +141,38 @@ function main() {
141141
console.log("Restart Claude Code to pick up the new skills.");
142142
}
143143

144+
// `add-ci [targetRepo]` — drop the Factory conformance GitHub Action into a
145+
// repo's .github/workflows/. The Action is a per-repo file, not a ~/.claude
146+
// symlink, so it needs a copy, not a link. Idempotent: skips an existing file.
147+
function addCi(targetArg) {
148+
const targetRepo = targetArg ? path.resolve(targetArg) : process.cwd();
149+
const src = path.join(KIT_ROOT, "templates", "factory-conformance.yml");
150+
if (!fs.existsSync(src)) {
151+
console.error("factory-kit: templates/factory-conformance.yml missing from the kit.");
152+
process.exit(1);
153+
}
154+
const dstDir = path.join(targetRepo, ".github", "workflows");
155+
const dst = path.join(dstDir, "factory-conformance.yml");
156+
ensureDir(dstDir);
157+
if (fs.existsSync(dst)) {
158+
console.log(` skip ${path.relative(targetRepo, dst)} (already exists — remove it to re-add)`);
159+
return;
160+
}
161+
fs.copyFileSync(src, dst);
162+
console.log(` add ${path.relative(targetRepo, dst)}`);
163+
console.log("");
164+
console.log("Next: commit the workflow, then add 'Factory conformance' to your");
165+
console.log("branch-protection required checks so the new-critical gate is load-bearing.");
166+
}
167+
144168
try {
145-
main();
169+
const cmd = process.argv[2];
170+
if (cmd === "add-ci") {
171+
addCi(process.argv[3]);
172+
} else {
173+
main();
174+
}
146175
} catch (err) {
147-
console.error(`factory-kit install failed: ${err.message ?? err}`);
176+
console.error(`factory-kit failed: ${err.message ?? err}`);
148177
process.exit(1);
149178
}

check/__tests__/config.test.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,13 @@ describe("loadConfig", () => {
1313
it("defaults to an empty set when no config file exists", async () => {
1414
const cfg = await loadConfig(fixtures("clean"));
1515
expect(cfg.disabledRules.size).toBe(0);
16+
expect(cfg.gateOnHigh).toBe(false);
17+
expect(cfg.ignorePaths).toEqual([]);
18+
});
19+
20+
it("reads the kit's own ignorePaths (fixtures excluded)", async () => {
21+
const kitRoot = fileURLToPath(new URL("../..", import.meta.url));
22+
const cfg = await loadConfig(kitRoot);
23+
expect(cfg.ignorePaths).toContain("**/__tests__/**");
1624
});
1725
});

check/__tests__/engine.test.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,17 @@ describe("engine over fixtures", () => {
4949
});
5050
});
5151

52+
describe("walk honors extra ignore patterns", () => {
53+
it("excludes files matching an ignorePaths glob, scans them without it", async () => {
54+
const root = fixtures(""); // the fixtures/ dir, with violations/ + others under it
55+
const withIgnore = await walk(root, ["**/violations/**"]);
56+
expect(withIgnore.files.some((f) => f.path.includes("violations/"))).toBe(false);
57+
58+
const without = await walk(root);
59+
expect(without.files.some((f) => f.path.includes("violations/"))).toBe(true);
60+
});
61+
});
62+
5263
describe("run (end-to-end, including report)", () => {
5364
afterEach(() => vi.restoreAllMocks());
5465

0 commit comments

Comments
 (0)