Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .claude/skills/contributing-to-loopover/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,12 @@ This is where most PRs fail Codecov. The bar is **every changed line AND every c
`createTestEnv()`), `test/workers/` (Cloudflare pool, separate config), plus `test/contract/`.
Use the `createTestEnv()` / `TestD1Database` helpers (in-memory SQLite, applies all migrations) and
`vi.stubGlobal("fetch", …)` for GitHub calls. Patterns + snippets in `reference.md`.
- **Never close a coverage gap with a new `*-coverage.test.ts` / `*-branch-coverage.test.ts` bolt-on
file.** Add the missing branch case(s) to the module's own existing test file instead. A CI check
(`coverage-boltons:check`, part of `test:ci`) blocks this filename pattern — a whole family of these
(`predicted-gate-engine-coverage.test.ts`, `reward-risk-engine-branch-coverage.test.ts`,
`focus-manifest-engine-branch-coverage.test.ts`, `signals-coverage.test.ts`) accumulated in the repo
and each needed its own dedicated consolidation PR to undo (epic #8574).
- **Branch coverage is the trap.** Every `if/else`, ternary `? :`, `&&`/`||`, and especially every
nullish fallback `?? 0` / `?? []` is **two branches** — you must exercise *both* sides. The classic
miss: a `SUM(...)` over an empty set returns **NULL**, so `count ?? 0` needs a test where the value
Expand Down
1 change: 1 addition & 0 deletions .claude/skills/contributing-to-loopover/reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ validate the real publishable artifact still compiles and packs cleanly.
| docs → drift | doc/code claim checker | `npm run docs:drift-check` | a doc makes a claim the mechanical lint can verify is now false |
| docs → command-reference | generated CLI reference drift | `npm run command-reference:check` | committed command-reference doc is stale (run `npm run command-reference`) |
| manifest drift | `.loopover.yml` vs bundled fallback YAML | `npm run manifest:drift-check` | `src/config/loopover-repo-focus-manifest.ts`'s bundled YAML diverges from the real root `.loopover.yml` |
| coverage bolt-on filenames | blocks new `*-coverage.test.ts` / `*-branch-coverage.test.ts` files | `npm run coverage-boltons:check` | a new test file matches the bolt-on pattern instead of extending its module's existing suite (see Phase 3) |
| engine-parity drift | `src/{review,settings,signals}` vs `loopover-engine` twins | `npm run engine-parity:drift-check` | a hand-duplicated twin file pair diverges, or the installed `@loopover/engine` semver skews from the monorepo package |
| branding drift | "gittensory" leaking into runtime source | `npm run branding-drift:check` | a file's gittensory-string hit count rises above the recorded baseline (`scripts/branding-drift-baseline.json`) |
| engines/.nvmrc sync | every `engines.node` (root + each workspace that declares one) vs `.nvmrc`'s pinned major | `npm run engines-nvmrc:check` | any declared `engines.node` still allows the major above `.nvmrc`'s pin (paired with the root `.npmrc`'s `engine-strict=true`, which turns that mismatch into a hard local `npm install`/`npm run` error instead of a silently-ignored warning) |
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
"ui:version-audit": "node --experimental-strip-types scripts/check-ui-mcp-version-copy.ts",
"ui:version-audit:sync": "node --experimental-strip-types scripts/check-ui-mcp-version-copy.ts --write",
"docs:drift-check": "node --experimental-strip-types scripts/check-docs-drift.ts",
"coverage-boltons:check": "node --experimental-strip-types scripts/check-coverage-bolt-on-filenames.ts",
"roadmap:drift-check": "node --experimental-strip-types scripts/check-roadmap-issue-drift.ts",
"branding-drift:check": "node --experimental-strip-types scripts/check-branding-drift.ts",
"branding-drift:update": "node --experimental-strip-types scripts/check-branding-drift.ts --update",
Expand Down Expand Up @@ -111,7 +112,7 @@
"test:smoke:browser:install": "playwright install chromium",
"test:smoke:browser": "node --experimental-strip-types scripts/smoke-ui-browser.ts",
"pretest:ci": "npm run check-node-version",
"test:ci": "git diff --check && npm run actionlint && npm run lint:composite-actions && npm run db:migrations:check && npm run db:schema-drift:check && npm run selfhost:env-reference:check && npm run miner:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run build --workspace @loopover/engine && npm run build --workspace @loopover/discovery-index && npm run build:mcp && npm run build:miner && npm run typecheck && npm run test:coverage && npm run test:engine-parity && npm run test:live-gate-parity && npm run test:driver-parity && npm run test --workspace @loopover/engine && npm run test:workers && npm run test:mcp-pack && npm run test:miner-pack && npm run test:miner-deployment-docs-audit && npm run rees:test && npm run ui:openapi:check && npm run ui:openapi:settings-parity && npm run ui:version-audit && npm run docs:drift-check && npm run branding-drift:check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run engines-nvmrc:check && npm run release-manifest:sync:check && npm run command-reference:check && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build",
"test:ci": "git diff --check && npm run actionlint && npm run lint:composite-actions && npm run db:migrations:check && npm run db:schema-drift:check && npm run selfhost:env-reference:check && npm run miner:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run build --workspace @loopover/engine && npm run build --workspace @loopover/discovery-index && npm run build:mcp && npm run build:miner && npm run typecheck && npm run test:coverage && npm run test:engine-parity && npm run test:live-gate-parity && npm run test:driver-parity && npm run test --workspace @loopover/engine && npm run test:workers && npm run test:mcp-pack && npm run test:miner-pack && npm run test:miner-deployment-docs-audit && npm run rees:test && npm run ui:openapi:check && npm run ui:openapi:settings-parity && npm run ui:version-audit && npm run docs:drift-check && npm run coverage-boltons:check && npm run branding-drift:check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run engines-nvmrc:check && npm run release-manifest:sync:check && npm run command-reference:check && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build",
"test:release": "npm run test:ci && npm run changelog:check",
"test:release:mcp": "npm run test:ci",
"test:watch": "vitest",
Expand Down
57 changes: 57 additions & 0 deletions scripts/check-coverage-bolt-on-filenames.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
#!/usr/bin/env node
// Blocks the "-coverage.test.ts" / "-branch-coverage.test.ts" bolt-on pattern from recurring (#8580,
// epic #8574). A contributor's cheapest path to the 99% Codecov patch-coverage bar is a new file of
// generic "exercises X branches" tests instead of extending the module's existing suite -- that's how
// predicted-gate-engine-coverage.test.ts, reward-risk-engine-branch-coverage.test.ts,
// focus-manifest-engine-branch-coverage.test.ts, and signals-coverage.test.ts all came to exist, each
// requiring a dedicated consolidation PR to undo (#8575/#8577/#8578/#8576). Extend the module's own
// test file instead; if a name below is a genuine exception, add it to ALLOWED_FILENAMES with a reason.
import { readdirSync } from "node:fs";
import { fileURLToPath } from "node:url";

const TEST_ROOT = "test";
const BOLT_ON_PATTERN = /-(branch-)?coverage\.test\.ts$/;

/** rees-coverage-script.test.ts tests scripts/rees-coverage.js (real coverage-report tooling) -- the
* name is a coincidence, not the bolt-on pattern; the "coverage" in its subject IS the point of the file. */
const ALLOWED_FILENAMES = new Set(["rees-coverage-script.test.ts"]);

function defaultListTestFiles(root: string): string[] {
return readdirSync(root, { recursive: true })
.map(String)
.filter((entry) => entry.endsWith(".test.ts"));
}

/** Pure: given the repo's test filenames, returns the bolt-on-named ones that aren't allowlisted.
* `listTestFiles` is injectable so tests can simulate a fresh offender without touching the real tree. */
export function findCoverageBoltOnFilenames(options: {
root?: string;
listTestFiles?: (root: string) => string[];
} = {}): string[] {
const { root = TEST_ROOT, listTestFiles = defaultListTestFiles } = options;
return listTestFiles(root)
.filter((entry) => BOLT_ON_PATTERN.test(entry.split("/").pop() ?? entry))
.filter((entry) => !ALLOWED_FILENAMES.has(entry.split("/").pop() ?? entry))
.sort();
}

function main() {
const offenders = findCoverageBoltOnFilenames();

if (offenders.length > 0) {
console.error(`Found ${offenders.length} "-coverage.test.ts" / "-branch-coverage.test.ts" bolt-on file(s):`);
for (const offender of offenders) console.error(` test/${offender}`);
console.error(
"\nExtend the module's existing test file instead of adding a new coverage-named bolt-on file " +
"(see .claude/skills/contributing-to-loopover/reference.md's test-patterns section). If this is a " +
"genuine exception, add its filename to ALLOWED_FILENAMES in scripts/check-coverage-bolt-on-filenames.ts " +
"with a one-line reason.",
);
process.exit(1);
}

console.log('Coverage bolt-on filename check ok: no new "-coverage.test.ts" / "-branch-coverage.test.ts" files.');
}

// Guard so importing this module for its pure export (tests) never triggers the exit side effect.
if (process.argv[1] === fileURLToPath(import.meta.url)) main();
62 changes: 62 additions & 0 deletions test/unit/check-coverage-bolt-on-filenames-script.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import { execFileSync } from "node:child_process";
import { describe, expect, it } from "vitest";
import { findCoverageBoltOnFilenames } from "../../scripts/check-coverage-bolt-on-filenames.js";

describe("check-coverage-bolt-on-filenames script", () => {
it("flags a new *-coverage.test.ts file", () => {
const offenders = findCoverageBoltOnFilenames({
listTestFiles: () => ["unit/foo.test.ts", "unit/foo-coverage.test.ts"],
});
expect(offenders).toEqual(["unit/foo-coverage.test.ts"]);
});

it("flags a new *-branch-coverage.test.ts file", () => {
const offenders = findCoverageBoltOnFilenames({
listTestFiles: () => ["unit/bar-branch-coverage.test.ts"],
});
expect(offenders).toEqual(["unit/bar-branch-coverage.test.ts"]);
});

it("does not flag a file that merely contains 'coverage' mid-name, not as a bolt-on suffix", () => {
const offenders = findCoverageBoltOnFilenames({
listTestFiles: () => ["unit/coverage-report-parser.test.ts", "unit/rees-coverage-script.test.ts"],
});
expect(offenders).toEqual([]);
});

it("allowlists rees-coverage-script.test.ts (tests scripts/rees-coverage.js, real coverage tooling)", () => {
const offenders = findCoverageBoltOnFilenames({
listTestFiles: () => ["unit/rees-coverage-script.test.ts"],
});
expect(offenders).toEqual([]);
});

it("sorts the offenders and matches regardless of subdirectory nesting", () => {
const offenders = findCoverageBoltOnFilenames({
listTestFiles: () => ["integration/z-coverage.test.ts", "unit/a-branch-coverage.test.ts"],
});
expect(offenders).toEqual(["integration/z-coverage.test.ts", "unit/a-branch-coverage.test.ts"]);
});

it("passes cleanly when nothing matches the bolt-on pattern", () => {
const offenders = findCoverageBoltOnFilenames({
listTestFiles: () => ["unit/foo.test.ts", "unit/bar.test.ts"],
});
expect(offenders).toEqual([]);
});

// Most important regression test in this file: the real repo, post-consolidation (#8574), must have
// zero bolt-on-named test files. If this fails, a new one has landed -- extend its module's existing
// suite instead of weakening this check.
it("the real repo has zero coverage bolt-on filenames (regression guard)", () => {
const offenders = findCoverageBoltOnFilenames();
expect(offenders).toEqual([]);
});

it("prints a clean summary and exits 0 for the real repo state when run as a subprocess", () => {
const output = execFileSync("node", ["--experimental-strip-types", "scripts/check-coverage-bolt-on-filenames.ts"], {
encoding: "utf8",
});
expect(output).toMatch(/Coverage bolt-on filename check ok/);
});
});
Loading