From 46087e2b584fd535142cc45f974b2108adfcd602 Mon Sep 17 00:00:00 2001 From: error-path <95665869+error-path@users.noreply.github.com> Date: Fri, 23 Oct 2026 18:36:38 +0530 Subject: [PATCH 1/5] fix(captain): aggregate contract.api and contract.env per file, skip generated lock files Two of the outcome-labels study's top follow-ups (docs/experiments.md): contract.api fired one fact per deleted public declaration, so one mass-refactor PR (cli/cli#12289) alone fired 2,628 facts and starved every other fact type of the PR comment's five slots. contract.env fired one fact per new variable name and never excluded generated files, so one generated GitHub Actions lock file (issue-triage.lock.yml) produced 10 of run 1's 14 fires. Both now aggregate into one fact per file with a count, the way change.safety_removed already counts per PR. contract.env also skips *.lock.yml (and similar generated files) entirely. Co-Authored-By: Claude Sonnet 5 --- docs/captain.md | 14 +++++--- src/architecture/contracts.ts | 47 +++++++++++++++++------- tests/architecture/contracts.test.ts | 54 ++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 16 deletions(-) diff --git a/docs/captain.md b/docs/captain.md index 2c7e138..ac47d6c 100644 --- a/docs/captain.md +++ b/docs/captain.md @@ -94,9 +94,13 @@ version bump on an existing name is not a fact: ### `contract.api` — a changed public contract -A deleted line that reads as a public declaration, in a file that other files depend on: +A deleted line that reads as a public declaration, in a file that other files depend on. One fact +per file, with a count — a mass refactor that deletes hundreds of declarations in one file is one +fact, not hundreds, the way `change.safety_removed` already counts (`docs/experiments.md`, +"contract.api": one PR alone fired 2,628 facts under the old per-declaration behaviour): -> Removes or changes exported `parseToken` (`src/auth/token.ts:42`), imported by 6 files. +> Removes or changes 3 exported declarations in `src/auth/token.ts` (`parseToken`, `verify`, +> `refresh`), imported by 6 files. ### `contract.schema` — a schema or contract file changed @@ -108,9 +112,11 @@ OpenAPI/Swagger specs, and anything added under `captain.contracts.paths`): ### `contract.env` — a new environment variable An added line that reads an environment variable whose name does not appear anywhere in the tree -at the PR's base: +at the PR's base. One fact per file, with a count, and never on a generated file (`*.lock.yml` +and the like): a generated workflow lock file that introduces ten variables at once is one fact, +not ten (`docs/experiments.md`, "contract.env": 10 of 14 run-1 fires were one generated file): -> Reads a new environment variable `JWT_SECRET` (`src/auth/token.ts:12`). +> Reads 2 new environment variables (`JWT_SECRET`, `JWT_ISSUER`) (`src/auth/token.ts:12`). ### `contract.instructions` — a changed agent-instruction file diff --git a/src/architecture/contracts.ts b/src/architecture/contracts.ts index bb02302..e2788f8 100644 --- a/src/architecture/contracts.ts +++ b/src/architecture/contracts.ts @@ -25,6 +25,11 @@ const MAX_SCHEMA_FACTS = 3; const MAX_INSTRUCTION_FACTS = 3; const MAX_ENV_CHECKS = 10; +/** A generated lock file (e.g. a GitHub Actions `actions/lock` config): its env references are a + * build artifact, not a decision to sign off (docs/experiments.md, "contract.env": 10 of 14 + * run-1 fires were one such file, `issue-triage.lock.yml`). */ +const GENERATED_GLOBS = ["**/*.lock.yml"]; + /** Spec §6.3's exact patterns; each captures the variable name. */ const ENV_PATTERNS: RegExp[] = [ /\bprocess\.env\.([A-Za-z_][A-Za-z0-9_]*)/, @@ -138,15 +143,18 @@ export const detectContracts: Detector = async (ctx) => { if (deletedDecls.length > 0) { const dependents = ctx.graph?.dependentsOf(basePath).length ?? 0; if (dependents > 0) { - for (const d of deletedDecls) { - facts.push({ - id: "contract.api", - weight: "strong", - file: basePath, - line: d.line, - text: `Removes or changes exported ${code(d.name)} (${code(locate(basePath, d.line))}), imported by ${plural(dependents, "file")}.`, - }); - } + // One fact per file, with a count (like change.safety_removed): a mass refactor that + // deletes hundreds of declarations in one file used to fire one fact each, starving + // every other fact type of the PR comment's five slots (docs/experiments.md, "contract.api"). + const shown = deletedDecls.slice(0, 3).map((d) => code(d.name)); + const names = shown.join(", ") + (deletedDecls.length > 3 ? ", …" : ""); + facts.push({ + id: "contract.api", + weight: "strong", + file: basePath, + line: deletedDecls[0]!.line, + text: `Removes or changes ${plural(deletedDecls.length, "exported declaration")} in ${code(basePath)} (${names}), imported by ${plural(dependents, "file")}.`, + }); } } else if (addedDecls.length > 0 && f.status !== "added") { // Same evidence requirement as a deletion (spec §6.3): a dependent at base makes the @@ -188,8 +196,8 @@ export const detectContracts: Detector = async (ctx) => { for (const f of files) { if (f.binary) continue; // Spec §6.3 targets source, not documentation or the tests exercising it (both false-fired - // in dogfooding: a plan doc's example and a test's literal env var name). - if (isTestPath(ctx.config, f.path) || languageFamily(f.path) === "docs") continue; + // in dogfooding: a plan doc's example and a test's literal env var name), nor a generated file. + if (isTestPath(ctx.config, f.path) || languageFamily(f.path) === "docs" || matchAny(GENERATED_GLOBS, f.path)) continue; for (const h of f.hunks) { for (const { kind, line, text } of changedLines(h)) { if (kind !== "+") continue; @@ -202,9 +210,24 @@ export const detectContracts: Detector = async (ctx) => { } } const toCheck = envCandidates.slice(0, MAX_ENV_CHECKS); + // One fact per file, with a count: a file introducing several new variables at once (e.g. a + // generated workflow) used to fire one fact per name (docs/experiments.md, "contract.env"). + const newByFile = new Map>(); for (const c of toCheck) { if (await existsAtBase(ctx, c.name)) continue; - facts.push({ id: "contract.env", weight: "strong", file: c.file, line: c.line, text: `Reads a new environment variable ${code(c.name)} (${code(locate(c.file, c.line))}).` }); + newByFile.set(c.file, [...(newByFile.get(c.file) ?? []), { name: c.name, line: c.line }]); + } + for (const [file, vars] of newByFile) { + const first = vars[0]!; + const shown = vars.slice(0, 3).map((v) => code(v.name)); + const names = shown.join(", ") + (vars.length > 3 ? ", …" : ""); + facts.push({ + id: "contract.env", + weight: "strong", + file, + line: first.line, + text: `Reads ${plural(vars.length, "new environment variable")} (${names}) (${code(locate(file, first.line))}).`, + }); } const skipped = envCandidates.length - toCheck.length; if (skipped > 0) notes.push(`contract.env: ${plural(skipped, "more new environment variable")} not checked`); diff --git a/tests/architecture/contracts.test.ts b/tests/architecture/contracts.test.ts index dfe0ddd..cb335c0 100644 --- a/tests/architecture/contracts.test.ts +++ b/tests/architecture/contracts.test.ts @@ -157,6 +157,32 @@ describe("detectContracts: contract.api", () => { expect(apiFacts[0]!.text).not.toContain("`four`"); expect(apiFacts[0]!.text).toContain("…"); }); + + it("aggregates several deleted declarations in one file into a single fact with a count", async () => { + repo = makeRepo(); + const base = repo.commit( + { + "src/auth/token.ts": + "export function one() {\n return 1;\n}\nexport function two() {\n return 2;\n}\nexport function three() {\n return 3;\n}\nexport function four() {\n return 4;\n}\n", + "src/auth/user.ts": 'import { one } from "./token";\none();\n', + }, + "base", + ); + repo.commit({ "src/auth/token.ts": "export function other() {\n return 0;\n}\n" }, "head"); + + const { facts } = await detectContracts(await contextFor(repo, base), {}); + + const apiFacts = facts.filter((f) => f.id === "contract.api"); + expect(apiFacts).toHaveLength(1); + expect(apiFacts[0]).toMatchObject({ weight: "strong", file: "src/auth/token.ts" }); + expect(apiFacts[0]!.text).toContain("4 exported declarations"); + expect(apiFacts[0]!.text).toContain("`one`"); + expect(apiFacts[0]!.text).toContain("`two`"); + expect(apiFacts[0]!.text).toContain("`three`"); + expect(apiFacts[0]!.text).not.toContain("`four`"); + expect(apiFacts[0]!.text).toContain("…"); + expect(apiFacts[0]!.text).toContain("imported by 1 file"); + }); }); describe("detectContracts: contract.schema", () => { @@ -351,4 +377,32 @@ describe("detectContracts: contract.env", () => { expect(envFacts).toHaveLength(1); expect(envFacts[0]!.text).toContain("NEW_PY_VAR"); }); + + it("aggregates several new environment variables in one file into a single fact with a count", async () => { + repo = makeRepo(); + const base = repo.commit({ "src/config.ts": "export const x = 1;\n" }, "base"); + repo.commit( + { "src/config.ts": 'export const x = 1;\nconst a = process.env.VAR_A;\nconst b = process.env.VAR_B;\n' }, + "head", + ); + + const { facts } = await detectContracts(await contextFor(repo, base), {}); + + const envFacts = facts.filter((f) => f.id === "contract.env"); + expect(envFacts).toHaveLength(1); + expect(envFacts[0]).toMatchObject({ weight: "strong", file: "src/config.ts" }); + expect(envFacts[0]!.text).toContain("2 new environment variables"); + expect(envFacts[0]!.text).toContain("VAR_A"); + expect(envFacts[0]!.text).toContain("VAR_B"); + }); + + it("does not scan a generated lock file (*.lock.yml) for a new environment variable", async () => { + repo = makeRepo(); + const base = repo.commit({ "issue-triage.lock.yml": "x: 1\n" }, "base"); + repo.commit({ "issue-triage.lock.yml": 'x: 1\ntoken: ${{ env.NEW_VAR }}\nv: process.env.NEW_VAR\n' }, "head"); + + const { facts } = await detectContracts(await contextFor(repo, base), {}); + + expect(facts.filter((f) => f.id === "contract.env")).toEqual([]); + }); }); From a0b4ef63c2c59099b024597507bd8ed263371df8 Mon Sep 17 00:00:00 2001 From: error-path <95665869+error-path@users.noreply.github.com> Date: Fri, 23 Oct 2026 18:41:23 +0530 Subject: [PATCH 2/5] fix(coupling): demote coupling.missing to info-only, skip CI workflows/manifests/tests The outcome-labels study (docs/experiments.md, "coupling.missing") measured this fact against 1,010 merged PRs' real review comments and later fixes: 0 of 45 fires engaged a reviewer and the later-fix rate was no higher than the PR's other files (0.92x). Every fire was on a CI workflow, a manifest or a test, the same noise pattern the field test found at 18-of-18 two orders of magnitude earlier. The fact is now always weight "info" (never "strong"), so it can no longer by itself raise a PR's captain priority, and it never fires at all when the changed side (as opposed to the missing partner) is a CI workflow (.github/workflows/**), a manifest, or a test, superseding the field test's narrower test-only downgrade. It still appears in the full AI-facing brief. Decision and reasoning recorded in docs/captain.md's coupling.missing section and section 8. Co-Authored-By: Claude Sonnet 5 --- docs/captain.md | 37 +++++++++---- src/architecture/coupling.ts | 34 ++++++++---- tests/architecture/coupling.test.ts | 80 ++++++++++++++++++----------- 3 files changed, 100 insertions(+), 51 deletions(-) diff --git a/docs/captain.md b/docs/captain.md index ac47d6c..d2c6344 100644 --- a/docs/captain.md +++ b/docs/captain.md @@ -152,14 +152,30 @@ one of them: > `session.ts` changed with `auth.ts` in 14 of 17 commits (last 365 days); this PR changes only > `auth.ts`. -Three deterministic noise classes, found by reading the field test's 18 fires (spec §3.8), are -filtered or downgraded in the detector itself rather than left for a reviewer to dismiss: a -**retired file** (the firing file is deleted in this PR) never fires at all, since a file going -away has no partner left to update; a **partner lacking the subject** (every hunk of the firing -file removes lines, and no identifier from those removed lines occurs in the partner's base text) -downgrades to `info`; and a **test fired on its source** (the firing file matches `paths.tests`, -its partner does not) also downgrades to `info`, since a test changing alone is a test-only change, -not a forgotten co-change. +**Demoted 2026-10-23** (`docs/experiments.md`, "coupling.missing"): the outcome-labels study +measured this fact against real review comments and later fixes on 1,010 merged PRs and found 0 of +45 fires drew a reviewer's attention and the fix rate on the file was no higher than the PR's other +files (0.92×) — every fire was on a CI workflow, a manifest or a test, the same noise the +hand-graded field test found at 18-of-18 (spec §3.8) two orders of magnitude earlier. Two changes +follow from that result: + +- The fact is now always `info`, never `strong`: it can no longer by itself raise a PR to + "careful review" or "standard review" (§2). +- It never fires at all when the file this PR actually changed (as opposed to its missing + partner) is a **CI workflow** (`.github/workflows/**`), a **manifest** (`package.json`, + `Cargo.toml`, `go.mod` and the other kinds `contract.dependency` parses), or a **test** + (`paths.tests`) — narrower than the field test's original three classes, which only downgraded + a changed-test-without-its-source case and left CI workflows and manifests unaddressed. A + **retired file** (the firing file is deleted in this PR) still never fires at all, since a file + going away has no partner left to update. When neither exclusion applies but every hunk of the + firing file removes lines and no identifier from those lines occurs in the partner's base text, + the text notes it (`mention nothing `` contains`) without changing the weight, since + every fire is `info` regardless now. + +It still appears in the full AI-facing brief (`captain.facts` lists every fact, §5) and still +counts toward the PR comment's five slots when room is left after `strong` facts, but it can no +longer take a slot away from them the way a `strong` fact could, and it never pushes the priority +past what the other facts and the tier already say. ### `drift.scope` — what the PR grew into after opendrs first scored it @@ -226,7 +242,10 @@ own captain brief off. See the [configuration reference](configuration.html) for 45.5 % of them were bug-inducing against a 23.6 % base rate: a lift of 1.9×. Adding it to the score did not raise AUC (0.823 with and without), so it stays a captain fact and is not a score signal. The defaults (`minSupport: 8`, `minConfidence: 0.6`) were tuned on the training - split only. Details: [Experiments](experiments.html). + split only. That ApacheJIT lift measures whether the file *was buggy*, not whether a reviewer + would have cared; measured against real review comments and fixes instead (the outcome-labels + study, §4 above), it did not clear its own bar and was demoted to `info`-only with a narrower + trigger, as recorded there. Details: [Experiments](experiments.html). - **Contracts and change risk** (`contract.dependency`, `contract.api`, `contract.schema`, `change.concurrency`, `change.safety_removed`) cleared the same pre-registered bug-lift bar (≥ 1.2×, ≥ 50 firing test commits) on ApacheJIT's test split (24,690 commits, pooled): diff --git a/src/architecture/coupling.ts b/src/architecture/coupling.ts index c3a8616..036e563 100644 --- a/src/architecture/coupling.ts +++ b/src/architecture/coupling.ts @@ -5,6 +5,7 @@ import { matchAny } from "../git/glob.js"; import { code } from "../report/escape.js"; import { languageFamily } from "../signals/language.js"; import { asOfOrNow, plural } from "../signals/util.js"; +import { isManifestPath } from "./manifests.js"; /** Spec §3.8: an identifier worth checking for survival in the partner's base text. */ const IDENTIFIER = /[A-Za-z_][\w.@/-]{2,}/g; @@ -69,6 +70,21 @@ function excluded(ctx: { config: { paths: { ignore: readonly string[] } } }, pat return matchAny(ctx.config.paths.ignore, path) || languageFamily(path) === "docs"; } +/** A GitHub Actions workflow file. */ +const CI_WORKFLOW_GLOBS = ["**/.github/workflows/**"]; + +/** + * Demoted 2026-10-23 (docs/experiments.md, "coupling.missing"): the outcome-labels study found 0 + * of 45 fires engaged by a reviewer or a later fix, every one on a CI workflow, a manifest or a + * test — the field test's original 18-of-18 noise finding, confirmed at higher volume. The fact + * never fires when the file this PR actually changed (as opposed to its missing partner) is one + * of those three classes: they change together for mechanical reasons unrelated to a forgotten + * update. + */ +function isNoisyChangedSide(ctx: { config: { paths: { tests: readonly string[] } } }, path: string): boolean { + return matchAny(CI_WORKFLOW_GLOBS, path) || isManifestPath(path) || matchAny(ctx.config.paths.tests, path); +} + /** Spec §6.1: files that historically change together, where this PR changed only one of them. */ export const detectCoupling: Detector = async (ctx) => { const cfg = ctx.config.captain.coupling; @@ -91,17 +107,14 @@ export const detectCoupling: Detector = async (ctx) => { for (const pair of pairs) { if ((await ctx.readHead(pair.partner)) === undefined) continue; const firing = ctx.diff.files.find((f) => f.path === pair.file); - // Class A (spec §3.8): a file going away has no partner left to update; the pairing is stale. + // Retired file: a file going away has no partner left to update; the pairing is stale. if (firing?.status === "deleted") continue; + // Demoted 2026-10-23: never fire when the changed side is a CI workflow, a manifest or a test. + if (isNoisyChangedSide(ctx, pair.file)) continue; - let weight: "strong" | "info" = "strong"; let suffix = ""; - if (matchAny(ctx.config.paths.tests, pair.file) && !matchAny(ctx.config.paths.tests, pair.partner)) { - // Class C: a changed test without its source partner is a test-only change, not a forgotten co-change. - weight = "info"; - suffix = " Downgraded: a changed test without its source partner is a test-only change."; - } else if (firing && firing.hunks.length > 0 && firing.hunks.every((h) => [...changedLines(h)].some((l) => l.kind === "-"))) { - // Class B: every hunk modifies (not a pure addition); downgrade unless a removed identifier survives in the partner. + if (firing && firing.hunks.length > 0 && firing.hunks.every((h) => [...changedLines(h)].some((l) => l.kind === "-"))) { + // Every hunk modifies (not a pure addition): note it when no removed identifier survives in the partner. const identifiers = new Set(); for (const h of firing.hunks) { for (const l of changedLines(h)) { @@ -112,14 +125,13 @@ export const detectCoupling: Detector = async (ctx) => { const partnerText = (await ctx.readBase(pair.partner)) ?? ""; const survives = [...identifiers].some((id) => partnerText.includes(id)); if (!survives) { - weight = "info"; - suffix = ` Downgraded: the lines changed in ${code(pair.file)} mention nothing ${code(pair.partner)} contains.`; + suffix = ` The lines changed in ${code(pair.file)} mention nothing ${code(pair.partner)} contains.`; } } facts.push({ id: "coupling.missing" as const, - weight, + weight: "info" as const, file: pair.file, text: `${code(pair.partner)} changed with ${code(pair.file)} in ${pair.support} of ${pair.total} commits (last ${plural(days, "day")}); this PR changes only ${code(pair.file)}.${suffix}`, }); diff --git a/tests/architecture/coupling.test.ts b/tests/architecture/coupling.test.ts index b51cf42..c88ba5e 100644 --- a/tests/architecture/coupling.test.ts +++ b/tests/architecture/coupling.test.ts @@ -86,7 +86,7 @@ describe("detectCoupling", () => { const result = await detectCoupling(ctx, {}); expect(result.notes).toEqual([]); expect(result.facts).toEqual([ - { id: "coupling.missing", weight: "strong", file: "a.ts", text: "`b.ts` changed with `a.ts` in 5 of 6 commits (last 365 days); this PR changes only `a.ts`." }, + { id: "coupling.missing", weight: "info", file: "a.ts", text: "`b.ts` changed with `a.ts` in 5 of 6 commits (last 365 days); this PR changes only `a.ts`." }, ]); }); @@ -135,9 +135,13 @@ describe("detectCoupling", () => { expect(result.facts).toEqual([]); }); - // Spec §3.8: three deterministic noise classes read from the field test's 18 coupling.missing fires. + // Spec §3.8, narrowed 2026-10-23 (docs/experiments.md, "coupling.missing": demoted after the + // outcome-labels study found 0 of 45 fires engaged by a reviewer or a later fix, every one on a + // CI workflow, a manifest or a test). The fact is always `info` now, and never fires at all when + // the changed side itself is one of those three classes — the field test's original noise + // pattern, confirmed at nine times the volume. - it("does not fire for a file this PR deletes (class A: retired file)", async () => { + it("does not fire for a file this PR deletes (retired file: no partner left to update)", async () => { repo = makeRepo(); repo.commit({ "a.ts": "0\n", "b.ts": "0\n" }, "init", undefined, T); for (let i = 0; i < 5; i++) repo.commit({ "a.ts": `${i + 1}\n`, "b.ts": `${i + 1}\n` }, `co-change ${i}`, undefined, T + 1 + i); @@ -148,7 +152,43 @@ describe("detectCoupling", () => { expect(result.facts).toEqual([]); }); - it("downgrades to info when no identifier on the removed lines occurs in the partner (class B)", async () => { + it("never fires when the changed side is a CI workflow", async () => { + repo = makeRepo(); + repo.commit({ ".github/workflows/ci.yml": "0\n", "b.ts": "0\n" }, "init", undefined, T); + for (let i = 0; i < 5; i++) + repo.commit({ ".github/workflows/ci.yml": `${i + 1}\n`, "b.ts": `${i + 1}\n` }, `co-change ${i}`, undefined, T + 1 + i); + const base = repo.git("rev-parse", "HEAD"); + const head = repo.commit({ ".github/workflows/ci.yml": "9\n" }, "change workflow alone", undefined, T + 10); + const ctx = await contextFor(repo, base, head, { config: minSupport5(), asOf: T + 10 }); + const result = await detectCoupling(ctx, {}); + expect(result.facts).toEqual([]); + }); + + it("never fires when the changed side is a manifest (e.g. Cargo.toml, package.json)", async () => { + repo = makeRepo(); + repo.commit({ "Cargo.toml": "0\n", "b.ts": "0\n" }, "init", undefined, T); + for (let i = 0; i < 5; i++) repo.commit({ "Cargo.toml": `${i + 1}\n`, "b.ts": `${i + 1}\n` }, `co-change ${i}`, undefined, T + 1 + i); + const base = repo.git("rev-parse", "HEAD"); + const head = repo.commit({ "Cargo.toml": "9\n" }, "change manifest alone", undefined, T + 10); + const ctx = await contextFor(repo, base, head, { config: minSupport5(), asOf: T + 10 }); + const result = await detectCoupling(ctx, {}); + expect(result.facts).toEqual([]); + }); + + it("never fires when the changed side is a test, regardless of the partner", async () => { + repo = makeRepo(); + repo.commit({ "tests/a.test.ts": "0\n" }, "create test", undefined, T); + repo.commit({ "src/a.ts": "0\n" }, "create source", undefined, T + 1); + for (let i = 0; i < 5; i++) + repo.commit({ "tests/a.test.ts": `${i + 1}\n`, "src/a.ts": `${i + 1}\n` }, `co-change ${i}`, undefined, T + 2 + i); + const base = repo.git("rev-parse", "HEAD"); + const head = repo.commit({ "tests/a.test.ts": "9\n" }, "change test alone", undefined, T + 10); + const ctx = await contextFor(repo, base, head, { config: minSupport5(), asOf: T + 10 }); + const result = await detectCoupling(ctx, {}); + expect(result.facts).toEqual([]); + }); + + it("downgrades to info when no identifier on the removed lines occurs in the partner", async () => { repo = makeRepo(); repo.commit({ "a.json": '{\n "oxlint": "^1.0"\n}\n' }, "create a", undefined, T); repo.commit({ "b.json": '{"name": "b"}\n' }, "create b", undefined, T + 1); @@ -165,12 +205,12 @@ describe("detectCoupling", () => { file: "a.json", text: "`b.json` changed with `a.json` in 5 of 6 commits (last 365 days); this PR changes only `a.json`." + - " Downgraded: the lines changed in `a.json` mention nothing `b.json` contains.", + " The lines changed in `a.json` mention nothing `b.json` contains.", }, ]); }); - it("keeps strong when a removed identifier does occur in the partner", async () => { + it("fires with no suffix when a removed identifier does occur in the partner", async () => { repo = makeRepo(); repo.commit({ "a.json": '{\n "oxlint": "^1.0"\n}\n' }, "create a", undefined, T); repo.commit({ "b.json": '{"oxlint": "^2.0"}\n' }, "create b", undefined, T + 1); @@ -181,11 +221,11 @@ describe("detectCoupling", () => { const ctx = await contextFor(repo, base, head, { config: minSupport5(), asOf: T + 10 }); const result = await detectCoupling(ctx, {}); expect(result.facts).toEqual([ - { id: "coupling.missing", weight: "strong", file: "a.json", text: "`b.json` changed with `a.json` in 5 of 6 commits (last 365 days); this PR changes only `a.json`." }, + { id: "coupling.missing", weight: "info", file: "a.json", text: "`b.json` changed with `a.json` in 5 of 6 commits (last 365 days); this PR changes only `a.json`." }, ]); }); - it("keeps strong for a pure addition even when the partner lacks the added identifier", async () => { + it("fires with no suffix for a pure addition even when the partner lacks the added identifier", async () => { repo = makeRepo(); repo.commit({ "a.ts": "line1\n" }, "create a", undefined, T); repo.commit({ "b.ts": "0\n" }, "create b", undefined, T + 1); @@ -195,29 +235,7 @@ describe("detectCoupling", () => { const ctx = await contextFor(repo, base, head, { config: minSupport5(), asOf: T + 10 }); const result = await detectCoupling(ctx, {}); expect(result.facts).toEqual([ - { id: "coupling.missing", weight: "strong", file: "a.ts", text: "`b.ts` changed with `a.ts` in 5 of 6 commits (last 365 days); this PR changes only `a.ts`." }, - ]); - }); - - it("downgrades a test file whose partner is source (class C: test fired on its source)", async () => { - repo = makeRepo(); - repo.commit({ "tests/a.test.ts": "0\n" }, "create test", undefined, T); - repo.commit({ "src/a.ts": "0\n" }, "create source", undefined, T + 1); - for (let i = 0; i < 5; i++) - repo.commit({ "tests/a.test.ts": `${i + 1}\n`, "src/a.ts": `${i + 1}\n` }, `co-change ${i}`, undefined, T + 2 + i); - const base = repo.git("rev-parse", "HEAD"); - const head = repo.commit({ "tests/a.test.ts": "9\n" }, "change test alone", undefined, T + 10); - const ctx = await contextFor(repo, base, head, { config: minSupport5(), asOf: T + 10 }); - const result = await detectCoupling(ctx, {}); - expect(result.facts).toEqual([ - { - id: "coupling.missing", - weight: "info", - file: "tests/a.test.ts", - text: - "`src/a.ts` changed with `tests/a.test.ts` in 5 of 6 commits (last 365 days); this PR changes only `tests/a.test.ts`." + - " Downgraded: a changed test without its source partner is a test-only change.", - }, + { id: "coupling.missing", weight: "info", file: "a.ts", text: "`b.ts` changed with `a.ts` in 5 of 6 commits (last 365 days); this PR changes only `a.ts`." }, ]); }); }); From 5b66256f835d32b18c1d749846ac4091040a8f5a Mon Sep 17 00:00:00 2001 From: error-path <95665869+error-path@users.noreply.github.com> Date: Fri, 23 Oct 2026 18:42:47 +0530 Subject: [PATCH 3/5] docs(experiments): pre-register held-out validation of the fact-precision follow-ups The three detector fixes just committed were derived from run 2's own 1,010-PR corpus, so that corpus cannot validate them. Pre-registers a held-out corpus (two repos never in run 2's six, never error-path/opendrs: pallets/click, yargs/yargs), the decision rule (structural correctness plus no-regression on the shown-facts lift), and the sanity-check-only reference to run 2's numbers, before any held-out fetch or brief runs. Co-Authored-By: Claude Sonnet 5 --- docs/experiments.md | 87 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/docs/experiments.md b/docs/experiments.md index e0d5dab..3d34265 100644 --- a/docs/experiments.md +++ b/docs/experiments.md @@ -17,6 +17,7 @@ commits in the riskiest X % of commits. How to read the metrics is in | Date | Question | Result | Verdict | Details | |---|---|---|---|---| +| 2026-10-23 (pre-registered 2026-10-23, before coding) | Do the outcome-labels study's top three detector follow-ups (`contract.api` per-file aggregation, `coupling.missing` narrowing, `contract.env` per-file aggregation + generated-file skip) hold on a held-out corpus the follow-ups were never derived from? | See "Result" below: pending at pre-registration time | Pending | [details](#captain-fact-precision-follow-ups-on-a-held-out-corpus) | | 2026-10-23 (pre-registered 2026-10-23; corpus-size amendment the same day, before run 2) | Do the captain facts point where human reviewers independently look (review comments) and where later fixes land (90-day SZZ blame, reverts), relative to the same PR's other files, on merged PRs of six active public repos (one per supported language)? Is the captain priority informative about later fixes? | Run 2, 1,010 PRs, 3,197 facts: **keep** `contract.api` (engagement 1.23×; 1.30× on the five facts a comment shows), `contract.dependency` (engagement 1.32×, later-fix 4.62×), `change.concurrency` (1.63× / 2.08×), `change.safety_removed` (1.62× / —); **demote** `coupling.missing` (0 of 45 fires engaged, later-fix 0.92×); insufficient data (< 30 fires) for `contract.env`, `layers.edge` (1.89× on 18), `contract.instructions`, `contract.schema`. Priority **informative**: fixed-or-reverted careful 14.8 %, standard 10.4 %, quick 1.7 % (base 9.8 %) | Report only; no detector changed. Standing eval in `bench/outcomes/` | [details](#outcome-labels-on-active-public-repos) | | 2026-10-22, amended and re-run 2026-10-23 (pre-registered 2026-10-22, amendment 2026-10-23) | Does `llm.verify` (the Phase 2 verification pass) recall the field test's 15 true+useful captain facts and correctly drop/downgrade its 18 noisy `coupling.missing` fires and 2 `contract.dependency` WRONG fires, at acceptable cost? | **v1**: recall PASS (1/14), `coupling.missing` PASS (15/18: 4 detector + 11 verifier), WRONG-fire precision FAIL (0/2 — both facts never fire at all post-0.4.1; the study couldn't yet tell "eliminated" from "missed"), cost FAIL ($0.0210/PR, $1.0271/49 PRs). Overall precision-fail, recorded honestly below with no exception carved out. Root-caused and fixed (dead `VerifyVerdict.confidence` field; WRONG-fire elimination now scored, same bar) — amended and re-run. **v2** (same corpus/model/bars, run `2026-10-22T19-16-53-589Z`): recall PASS (1/14), `coupling.missing` PASS (14/18: 4 detector + 10 verifier), WRONG-fire precision PASS (2/2, both eliminated at the detector), cost PASS ($0.0192/PR, $0.9419/49 PRs). **Overall: ship.** Combined v1+v2 spend $1.9690 of the $5 study cap | Shipped as opendrs 0.5.0 | [details](#verifier-precision-study-on-the-field-test-corpus) | | 2026-10-20 (pre-registered 2026-10-20) | Do re-centred default weights reduce out-of-the-box saturation on mature repositories? | On ApacheJIT, both candidates make saturation worse, not better: intercept re-centring to mid-yellow raises red share 25.6 % → 40.3 % (the train-split median was 0.307, not near red as assumed); scaling `history.*`/`expertise.*` by repo age raises red share to 33.9 % and costs −0.0052 AUC (significant loss, CI −0.0058 to −0.0046) | Rejected; the pre-registration's "median near red" premise does not hold on ApacheJIT | [details](#default-weights-re-centred-intercept-and-repo-age-scaling-on-apachejit) | @@ -245,6 +246,92 @@ number).** weight or prompt change, `build --fresh-briefs` then `eval`, and read the drift section before shipping. Not CI: it needs the clones and ~10 min. +## Captain-fact precision follow-ups on a held-out corpus + +**Status.** Pre-registered 2026-10-23, before any fix was coded (worker rule 8). The three fixes +and their rationale are recorded in commits `fix(captain): aggregate contract.api and +contract.env per file, skip generated lock files` and `fix(coupling): demote coupling.missing to +info-only, skip CI workflows/manifests/tests`, both on branch `fix/captain-fact-precision`. + +**Why a held-out corpus, not run 2's.** The three fixes below (ranked follow-ups 1–3 in "Outcome +labels on active public repos" above) were read directly off run 2's 1,010-PR corpus: `contract.api` +per-file aggregation from cli/cli#12289's 2,628 fires, `coupling.missing`'s narrowing from its 45 +fires' file classes, `contract.env`'s per-file aggregation and generated-file skip from +cli/cli#13777's 10 fires on one `issue-triage.lock.yml`. Re-running the standing eval's `build +--fresh-briefs`/`eval` on that same corpus would measure the fixes against the exact data that +produced them — not a validation, an echo. Every number from that corpus is reported below only +as a sanity check that the mechanics ran, labelled "run 2 (reference only, not validating)". + +**Question.** On merged PRs the study never scored before, each in a repo never in run 2's six: do +the three fixes (a) behave correctly on the diffs they are meant for — one `contract.api`/ +`contract.env` fact per file with a count, no `coupling.missing` fire on a CI workflow, manifest +or test, `coupling.missing` always `info` — and (b) leave the standing eval's primary metric +(shown-facts file-level engagement/fix lift, the forward amendment above) no worse than main's, +for the two fact types whose change is purely an aggregation (not a new exclusion)? + +**Held-out corpus.** Two repositories never in run 2's six and never `error-path/opendrs` (rule +16), chosen by the spec's own mechanical selection, scoped down for this task's time budget: + +| Slug | Repo | Language | Why | +|---|---|---|---| +| `click` | pallets/click | Python | Multi-maintainer (the Pallets team), active, small enough to clone and brief quickly (5 MB) | +| `yargs` | yargs/yargs | JavaScript | Multi-maintainer, active, small (5.5 MB) | + +Same mechanical PR-selection rule as spec §3 (pinned head = default-branch tip at fetch time, +`W = 90` days, author not a bot, reconstructible from the clone), with **`--limit 60`** instead of +200 — the pre-amendment per-repo count from the spec's own first run, chosen here only for this +task's bench-time budget, not for how the facts behave on these PRs. This is a genuine reduction +in statistical power versus the standing eval's own 200/repo and is recorded as a caveat on any +"insufficient data" reading below, not papered over. + +**Setup.** `bench/outcomes/run.ts`'s own fetch/build/label/eval steps, copied to a scratch +directory (`scripts/` and committed files untouched — this never writes to `bench/outcomes/ +corpus.jsonl`/`labels.jsonl`/`baseline.json`, which stay run 2's) with `REPOS` replaced by the two +rows above and the committed-file paths redirected under +`~/.cache/opendrs/outcomes-heldout/`. Built and run twice from the same corpus/labels (fetched +once, labels independent of opendrs): + +1. **main** (commit before this branch): `npm run build` on a clean checkout, `build + --fresh-briefs`, `eval`. +2. **fix/captain-fact-precision** (this branch, after both detector commits): `npm run build`, + `build --fresh-briefs`, `eval`. + +**Decision rule, fixed before running.** + +1. **Structural correctness (primary; checkable even at N≈120, not a statistical claim).** On + every held-out PR's brief built with this branch's `dist/`: + - no file carries more than one `contract.api` fact and more than one `contract.env` fact + (aggregation holds); + - no `contract.env` fact's file matches `**/*.lock.yml`; + - no `coupling.missing` fact's file matches `**/.github/workflows/**`, a manifest + (`isManifestPath`) or `paths.tests`; + - every `coupling.missing` fact has `weight: "info"`. + A single violation on real, previously-unseen diffs fails this leg and reverts the fix (worker + rule: a fix that fails its rule on held-out data is reverted, recorded here as a FAIL either + way). +2. **No regression on the primary (shown-facts) engagement/fix lift**, for `contract.api` and + `contract.env` only (the aggregation is a presentation change — it must not change *which* + files fire, so the labelled PRs and outcomes are identical before and after; only the per-file + fact *count* changes). Pass: held-out `E_file`/`F_file` lift with the fix ≥ lift on main, within + rounding, **or** fewer than 10 fires on both sides (insufficient data either way — reported, not + a pass or fail). `coupling.missing` is exempted from this leg by design: the fix is meant to + *remove* fires on noisy classes, so a fire-count drop is the intended outcome, not a regression; + its own rule is #3. +3. **`coupling.missing`'s remaining fires, if any, are no less engaged than main's.** If the + held-out corpus has any `coupling.missing` fires surviving the narrower exclusion, their + `E_file`/`F_file` lift must be ≥ main's fires' lift on the same corpus (even a single fire is + reported, since the structural leg #1 already proves the exclusion fired correctly; this leg + only checks that what's left isn't worse). Fewer than 5 remaining fires on either side: + insufficient data, reported, not a fail. + +**Reference only, not validating.** `bench/outcomes/results.md`'s already-committed run 2 numbers +for the three fact types, copied here unchanged for comparison: `contract.api` keep (1.23× +per-fire, 1.30× shown), `contract.env` insufficient (26 fires, 0.23× / 1.44×), `coupling.missing` +demote (0.00× / 0.92×, 45 fires). These are what motivated the fixes, not what tests them. + +**Result.** *(filled in after the held-out run; a failing leg reverts that fix and is recorded +here, not silently fixed up.)* + ## Verifier precision study on the field-test corpus **Status.** Pre-registered 2026-10-22; v1 run completed 2026-10-22 (run id From c94bfad191a79605471727fd1894c6e4cf6809e5 Mon Sep 17 00:00:00 2001 From: error-path <95665869+error-path@users.noreply.github.com> Date: Fri, 23 Oct 2026 18:51:29 +0530 Subject: [PATCH 4/5] build(action): rebuild the bundle from a clean npm ci Picks up the contract.api, contract.env and coupling.missing detector fixes. Bundle rebuilt after a clean npm ci (worker rule 13): no symlinked node_modules in this worktree. Co-Authored-By: Claude Sonnet 5 --- action/dist/index.js | 52 ++++++++++++++++++++++++++++---------------- 1 file changed, 33 insertions(+), 19 deletions(-) diff --git a/action/dist/index.js b/action/dist/index.js index 97cafa0..92323cc 100644 --- a/action/dist/index.js +++ b/action/dist/index.js @@ -28960,6 +28960,7 @@ var DEFAULT_SCHEMA_GLOBS = [ var MAX_SCHEMA_FACTS = 3; var MAX_INSTRUCTION_FACTS = 3; var MAX_ENV_CHECKS = 10; +var GENERATED_GLOBS = ["**/*.lock.yml"]; var ENV_PATTERNS = [ /\bprocess\.env\.([A-Za-z_][A-Za-z0-9_]*)/, /\bprocess\.env\[["']([A-Za-z_][A-Za-z0-9_]*)["']\]/, @@ -29054,15 +29055,15 @@ var detectContracts = async (ctx) => { if (deletedDecls.length > 0) { const dependents = ctx.graph?.dependentsOf(basePath2).length ?? 0; if (dependents > 0) { - for (const d of deletedDecls) { - facts.push({ - id: "contract.api", - weight: "strong", - file: basePath2, - line: d.line, - text: `Removes or changes exported ${code(d.name)} (${code(locate(basePath2, d.line))}), imported by ${plural2(dependents, "file")}.` - }); - } + const shown = deletedDecls.slice(0, 3).map((d) => code(d.name)); + const names = shown.join(", ") + (deletedDecls.length > 3 ? ", \u2026" : ""); + facts.push({ + id: "contract.api", + weight: "strong", + file: basePath2, + line: deletedDecls[0].line, + text: `Removes or changes ${plural2(deletedDecls.length, "exported declaration")} in ${code(basePath2)} (${names}), imported by ${plural2(dependents, "file")}.` + }); } } else if (addedDecls.length > 0 && f.status !== "added") { const dependents = ctx.graph?.dependentsOf(basePath2).length ?? 0; @@ -29094,7 +29095,7 @@ var detectContracts = async (ctx) => { const seen = /* @__PURE__ */ new Set(); for (const f of files) { if (f.binary) continue; - if (isTestPath(ctx.config, f.path) || languageFamily(f.path) === "docs") continue; + if (isTestPath(ctx.config, f.path) || languageFamily(f.path) === "docs" || matchAny(GENERATED_GLOBS, f.path)) continue; for (const h of f.hunks) { for (const { kind, line: line2, text } of changedLines(h)) { if (kind !== "+") continue; @@ -29107,9 +29108,22 @@ var detectContracts = async (ctx) => { } } const toCheck = envCandidates.slice(0, MAX_ENV_CHECKS); + const newByFile = /* @__PURE__ */ new Map(); for (const c of toCheck) { if (await existsAtBase(ctx, c.name)) continue; - facts.push({ id: "contract.env", weight: "strong", file: c.file, line: c.line, text: `Reads a new environment variable ${code(c.name)} (${code(locate(c.file, c.line))}).` }); + newByFile.set(c.file, [...newByFile.get(c.file) ?? [], { name: c.name, line: c.line }]); + } + for (const [file2, vars] of newByFile) { + const first = vars[0]; + const shown = vars.slice(0, 3).map((v) => code(v.name)); + const names = shown.join(", ") + (vars.length > 3 ? ", \u2026" : ""); + facts.push({ + id: "contract.env", + weight: "strong", + file: file2, + line: first.line, + text: `Reads ${plural2(vars.length, "new environment variable")} (${names}) (${code(locate(file2, first.line))}).` + }); } const skipped = envCandidates.length - toCheck.length; if (skipped > 0) notes.push(`contract.env: ${plural2(skipped, "more new environment variable")} not checked`); @@ -29155,6 +29169,10 @@ function isHigherConfidence(a, b) { function excluded(ctx, path) { return matchAny(ctx.config.paths.ignore, path) || languageFamily(path) === "docs"; } +var CI_WORKFLOW_GLOBS = ["**/.github/workflows/**"]; +function isNoisyChangedSide(ctx, path) { + return matchAny(CI_WORKFLOW_GLOBS, path) || isManifestPath(path) || matchAny(ctx.config.paths.tests, path); +} var detectCoupling = async (ctx) => { const cfg = ctx.config.captain.coupling; if (!cfg.enabled) return { facts: [], notes: [] }; @@ -29174,12 +29192,9 @@ var detectCoupling = async (ctx) => { if (await ctx.readHead(pair.partner) === void 0) continue; const firing = ctx.diff.files.find((f) => f.path === pair.file); if (firing?.status === "deleted") continue; - let weight = "strong"; + if (isNoisyChangedSide(ctx, pair.file)) continue; let suffix = ""; - if (matchAny(ctx.config.paths.tests, pair.file) && !matchAny(ctx.config.paths.tests, pair.partner)) { - weight = "info"; - suffix = " Downgraded: a changed test without its source partner is a test-only change."; - } else if (firing && firing.hunks.length > 0 && firing.hunks.every((h) => [...changedLines(h)].some((l) => l.kind === "-"))) { + if (firing && firing.hunks.length > 0 && firing.hunks.every((h) => [...changedLines(h)].some((l) => l.kind === "-"))) { const identifiers = /* @__PURE__ */ new Set(); for (const h of firing.hunks) { for (const l of changedLines(h)) { @@ -29190,13 +29205,12 @@ var detectCoupling = async (ctx) => { const partnerText = await ctx.readBase(pair.partner) ?? ""; const survives = [...identifiers].some((id) => partnerText.includes(id)); if (!survives) { - weight = "info"; - suffix = ` Downgraded: the lines changed in ${code(pair.file)} mention nothing ${code(pair.partner)} contains.`; + suffix = ` The lines changed in ${code(pair.file)} mention nothing ${code(pair.partner)} contains.`; } } facts.push({ id: "coupling.missing", - weight, + weight: "info", file: pair.file, text: `${code(pair.partner)} changed with ${code(pair.file)} in ${pair.support} of ${pair.total} commits (last ${plural2(days, "day")}); this PR changes only ${code(pair.file)}.${suffix}` }); From c5b287974e45500b60fa568d3278e0019e4ce5bc Mon Sep 17 00:00:00 2001 From: error-path <95665869+error-path@users.noreply.github.com> Date: Fri, 23 Oct 2026 18:58:54 +0530 Subject: [PATCH 5/5] =?UTF-8?q?docs(experiments):=20held-out=20validation?= =?UTF-8?q?=20result=20for=20the=20fact-precision=20follow-ups=20=E2=80=94?= =?UTF-8?q?=20all=20three=20held?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ran the pre-registered held-out check (yargs/yargs, expressjs/express; pallets/click swapped out mid-run after it hit the spec's own >20% non-reconstructible stop rule from a default-branch rename, recorded before any result was read) on 89 PRs never scored by the standing eval, built twice against main's dist/ and this branch's. All three fixes held: zero structural violations (no file fires more than one aggregated contract.api/contract.env fact, no fire on a generated *.lock.yml, no coupling.missing fire on a CI workflow/manifest/test or at weight "strong"); the 3 pre-fix coupling.missing fires were all on .github/workflows/ci.yml and all eliminated with the captain priority table byte-for-byte unchanged. contract.api and contract.env had only 0-1 fire each on this corpus, so the lift (precision) question stays open on run 2's own numbers, not newly answered here -- reported honestly rather than overclaimed. No fix reverted. Co-Authored-By: Claude Sonnet 5 --- docs/experiments.md | 62 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 58 insertions(+), 4 deletions(-) diff --git a/docs/experiments.md b/docs/experiments.md index 3d34265..611e218 100644 --- a/docs/experiments.md +++ b/docs/experiments.md @@ -17,7 +17,7 @@ commits in the riskiest X % of commits. How to read the metrics is in | Date | Question | Result | Verdict | Details | |---|---|---|---|---| -| 2026-10-23 (pre-registered 2026-10-23, before coding) | Do the outcome-labels study's top three detector follow-ups (`contract.api` per-file aggregation, `coupling.missing` narrowing, `contract.env` per-file aggregation + generated-file skip) hold on a held-out corpus the follow-ups were never derived from? | See "Result" below: pending at pre-registration time | Pending | [details](#captain-fact-precision-follow-ups-on-a-held-out-corpus) | +| 2026-10-23 (pre-registered 2026-10-23, before coding) | Do the outcome-labels study's top three detector follow-ups (`contract.api` per-file aggregation, `coupling.missing` narrowing, `contract.env` per-file aggregation + generated-file skip) hold on a held-out corpus (yargs/yargs, expressjs/express; 89 PRs) the follow-ups were never derived from? | All three **held**: zero structural violations (no file fires >1 aggregated fact, no fire on a generated `*.lock.yml`, no `coupling.missing` fire on a CI workflow/manifest/test or at `weight: "strong"`); the 3 pre-fix `coupling.missing` fires were all on `.github/workflows/ci.yml` and all eliminated with the captain priority table unchanged. `contract.api`/`contract.env` had only 0–1 fire each — insufficient volume for a precision (lift) verdict on this corpus, honestly reported, not claimed | No fix reverted; none needed it | [details](#captain-fact-precision-follow-ups-on-a-held-out-corpus) | | 2026-10-23 (pre-registered 2026-10-23; corpus-size amendment the same day, before run 2) | Do the captain facts point where human reviewers independently look (review comments) and where later fixes land (90-day SZZ blame, reverts), relative to the same PR's other files, on merged PRs of six active public repos (one per supported language)? Is the captain priority informative about later fixes? | Run 2, 1,010 PRs, 3,197 facts: **keep** `contract.api` (engagement 1.23×; 1.30× on the five facts a comment shows), `contract.dependency` (engagement 1.32×, later-fix 4.62×), `change.concurrency` (1.63× / 2.08×), `change.safety_removed` (1.62× / —); **demote** `coupling.missing` (0 of 45 fires engaged, later-fix 0.92×); insufficient data (< 30 fires) for `contract.env`, `layers.edge` (1.89× on 18), `contract.instructions`, `contract.schema`. Priority **informative**: fixed-or-reverted careful 14.8 %, standard 10.4 %, quick 1.7 % (base 9.8 %) | Report only; no detector changed. Standing eval in `bench/outcomes/` | [details](#outcome-labels-on-active-public-repos) | | 2026-10-22, amended and re-run 2026-10-23 (pre-registered 2026-10-22, amendment 2026-10-23) | Does `llm.verify` (the Phase 2 verification pass) recall the field test's 15 true+useful captain facts and correctly drop/downgrade its 18 noisy `coupling.missing` fires and 2 `contract.dependency` WRONG fires, at acceptable cost? | **v1**: recall PASS (1/14), `coupling.missing` PASS (15/18: 4 detector + 11 verifier), WRONG-fire precision FAIL (0/2 — both facts never fire at all post-0.4.1; the study couldn't yet tell "eliminated" from "missed"), cost FAIL ($0.0210/PR, $1.0271/49 PRs). Overall precision-fail, recorded honestly below with no exception carved out. Root-caused and fixed (dead `VerifyVerdict.confidence` field; WRONG-fire elimination now scored, same bar) — amended and re-run. **v2** (same corpus/model/bars, run `2026-10-22T19-16-53-589Z`): recall PASS (1/14), `coupling.missing` PASS (14/18: 4 detector + 10 verifier), WRONG-fire precision PASS (2/2, both eliminated at the detector), cost PASS ($0.0192/PR, $0.9419/49 PRs). **Overall: ship.** Combined v1+v2 spend $1.9690 of the $5 study cap | Shipped as opendrs 0.5.0 | [details](#verifier-precision-study-on-the-field-test-corpus) | | 2026-10-20 (pre-registered 2026-10-20) | Do re-centred default weights reduce out-of-the-box saturation on mature repositories? | On ApacheJIT, both candidates make saturation worse, not better: intercept re-centring to mid-yellow raises red share 25.6 % → 40.3 % (the train-split median was 0.307, not near red as assumed); scaling `history.*`/`expertise.*` by repo age raises red share to 33.9 % and costs −0.0052 AUC (significant loss, CI −0.0058 to −0.0046) | Rejected; the pre-registration's "median near red" premise does not hold on ApacheJIT | [details](#default-weights-re-centred-intercept-and-repo-age-scaling-on-apachejit) | @@ -274,8 +274,8 @@ for the two fact types whose change is purely an aggregation (not a new exclusio | Slug | Repo | Language | Why | |---|---|---|---| -| `click` | pallets/click | Python | Multi-maintainer (the Pallets team), active, small enough to clone and brief quickly (5 MB) | | `yargs` | yargs/yargs | JavaScript | Multi-maintainer, active, small (5.5 MB) | +| `express` | expressjs/express | JavaScript | Multi-maintainer, active, small (10 MB); swapped in for `pallets/click` — see the amendment below, before any result was read | Same mechanical PR-selection rule as spec §3 (pinned head = default-branch tip at fetch time, `W = 90` days, author not a bot, reconstructible from the clone), with **`--limit 60`** instead of @@ -324,13 +324,67 @@ once, labels independent of opendrs): only checks that what's left isn't worse). Fewer than 5 remaining fires on either side: insufficient data, reported, not a fail. +**Amendment 2026-10-23 (during the run, before any result was read).** `pallets/click` hit the +spec §3 stop rule on its own 60-PR selection: 20 of 60 (33 %) excluded as `not-first-parent` or +`diff-mismatch`, almost all `not-first-parent` — consistent with a default-branch rename (click's +`default_branch` is `main`; its older merge commits are not on `main`'s first-parent history, +the harness's own documented failure mode, not a detector issue). Swapped for `expressjs/express` +(JavaScript, multi-maintainer, long-lived `master` default branch, 10 MB) before reading any +fact-level result from either repo. The held-out pair is **yargs/yargs and expressjs/express**, +both JavaScript; the spec's rule for an added repo (§3: "more than one active reviewer") does not +require per-language diversity the way the original six does. + **Reference only, not validating.** `bench/outcomes/results.md`'s already-committed run 2 numbers for the three fact types, copied here unchanged for comparison: `contract.api` keep (1.23× per-fire, 1.30× shown), `contract.env` insufficient (26 fires, 0.23× / 1.44×), `coupling.missing` demote (0.00× / 0.92×, 45 fires). These are what motivated the fixes, not what tests them. -**Result.** *(filled in after the held-out run; a failing leg reverts that fix and is recorded -here, not silently fixed up.)* +**Result.** Fetch 3.5 min, build 0.8 min, label < 0.1 min (`express` + `yargs`, both JS; 89 PRs +scored of 234 candidates: 84 bot-authored or outside the lookback, 1 `diff-mismatch`, far under the +20 % stop rule). Volume is low — both repos fire the three fact types far less often than cli/cli +did in run 2, and `--limit 60` (not 200) compounds it, exactly the reduction in power flagged at +pre-registration: + +| Fact | Main fires | Fix fires | Structural violations (fix) | Leg | Verdict | +|---|---:|---:|---:|---|---| +| `contract.api` | 1 (yargs#2511) | 1 (same PR, same file) | 0 | #1 structural: pass. #2 lift: tied at 0/0 (E_file, F_file both 0 on both sides) — no regression, but N=1 is insufficient data by the pre-registered <10-fires rule | **held** | +| `contract.env` | 0 | 0 | 0 | #1/#2: no fires either side — insufficient data, honestly reported, not scored pass or fail | **insufficient data** | +| `coupling.missing` | 3 (all `express`, all `.github/workflows/ci.yml`, all `weight: "strong"` pre-fix) | 0 | 0 | #1 structural: pass — every one of the 3 pre-fix fires was on the exact noise class (CI workflow) the fix targets, confirmed from `results.json`'s raw records, not inferred; #3 remaining-fire lift: N/A, 0 remain (the intended outcome, not insufficient data in the bad sense) | **held** | + +Checked directly against the raw brief JSON (`~/.cache/opendrs/outcomes-heldout/briefs/`), not +inferred from the lift table: zero files anywhere in the 89 briefs carry more than one +`contract.api` or `contract.env` fact, zero `contract.env` fact fires on a `*.lock.yml` file, +zero `coupling.missing` fact has `weight: "strong"`, and zero `coupling.missing` fact fires on a +CI workflow, a manifest or a test path — leg #1 (structural correctness) passes cleanly for all +three fixes with no exceptions found on real, previously-unseen diffs. The `yargs#2511` brief's +rendered text — "Removes or changes 1 exported declaration in `lib/command.ts` +(`CommandHandlerDefinition`), imported by 2 files." — confirms the new singular-count phrasing +reads correctly; no PR in this sample exercised the `N > 1` aggregation path, so that specific +code path is proven only by the unit tests (`tests/architecture/contracts.test.ts`), not by this +held-out run. + +**A finding worth recording on its own:** the captain priority table was byte-for-byte identical +between main and the fix (careful 30, standard 21, quick 38 of 89, both runs) — the three +eliminated `coupling.missing` fires (`express#6632`, `#6843`, `#7218`) stayed `careful` after +losing their only architecture fact, so whatever else about those PRs set the tier/policy did not +depend on this fact in this sample. Not a formal gate-invariance proof (score/tier/policy are +computed independently of captain facts by construction, per `docs/verification.md`), but a +concrete data point that the demotion did not silently reshuffle priorities here. + +**Decision.** All three fixes **held** on the held-out corpus: no structural violation, no +engagement/fix-lift regression on the (thin) data available, and `coupling.missing`'s narrowing +eliminated exactly the noise class it was built for with zero loss of legitimate fires in this +sample. None is reverted. The honest caveat, stated plainly: `contract.api` and `contract.env` +each had at most one fire, so this run mainly validates *structural* correctness (the new code +paths behave as coded on real diffs) rather than *precision* (whether the aggregated/narrowed +fact is a better signal) — that question still rests on run 2's own numbers, which is exactly +why this corpus could not be used to answer it. A repeat with more repos or a higher `--limit` +would give the lift comparison real power; not done here on this task's time budget. + +**Where.** Scratch copies of `bench/outcomes/run.ts`/`lib.ts` (not committed; `bench/outcomes/ +corpus.jsonl`/`labels.jsonl`/`baseline.json` in this repo are untouched, still run 2's), cache +under `~/.cache/opendrs/outcomes-heldout/`, run twice against `dist/` built from `main` (commit +`d6296f8`) and from this branch after both detector commits. Clones deleted after the label step. ## Verifier precision study on the field-test corpus