Skip to content

Commit 5d34f85

Browse files
authored
test(engine): make self-review-adapter coverage visible to Codecov (#8466)
`packages/loopover-engine/src/miner/self-review-adapter.ts` (#2334) turns an attempt's live worktree diff into the same inputs `buildPredictedGateVerdict` and an injected slop-assessment function expect, so the miner's self-review pass is byte-identical in shape to the live maintainer gate. It exports `buildSelfReviewPredictedGateInput`, `buildSelfReviewChangedPaths`, `buildSelfReviewSlopInput`, `runSelfReview`, and `SELF_REVIEW_PASSING_CONCLUSION`. It is fully exercised by the engine package's own `node --test` suite, but that runner is not part of the root vitest run Codecov reads `codecov/patch` from, so it reports as ~0% covered despite being genuinely tested (same blind spot as #6250). Add a root-level vitest twin importing the adapter functions via the engine barrel and mirroring every scenario the package suite covers: each optional identity field present/omitted (both arms of every conditional spread), the `?? null` description fallback and `?.length ?? 0 > 0` hasLinkedIssue derivation across undefined/empty/non-empty linkedIssues, changedPaths always threaded through, the optional context fields (bounties/issueQuality/confirmedContributor) forwarded, the injected runSlopAssessment called with the built input and its result passed through verbatim, and passesPredictedGate true only for a `"success"` conclusion. Uses `buildPredictedGateVerdict` + `parseFocusManifest` and an INJECTED slop fake exactly as the package suite does; the real `src/signals/slop.ts` is never imported. 100% line + branch of the source locally. Test-only: no change to any file under `packages/loopover-engine/src/**` or `packages/loopover-engine/test/**`. Also document the root mirror in the engine README — the doc touch keeps this a full-coverage CI run so the sharded coverage blobs reach the merge step (a root-`test/**`-only diff is otherwise treated as a scoped, artifact-free run). Closes #8348
1 parent 04d38b6 commit 5d34f85

2 files changed

Lines changed: 243 additions & 0 deletions

File tree

packages/loopover-engine/README.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -687,6 +687,11 @@ root mirror at `test/unit/miner-governor-action-mode.test.ts` (#8345) — `codec
687687
vitest suite (see [Test](#test)), so this safety-adjacent write-execution gate is gradeable there as well as by
688688
the package's own `node:test` suite, alongside the existing `test/unit/miner-governor-kill-switch.test.ts` mirror.
689689

690+
Similarly, the miner self-review adapter (`src/miner/self-review-adapter.ts`, which builds the predicted-gate +
691+
slop inputs the miner's self-review pass runs) has a Codecov-visible root mirror at
692+
`test/unit/self-review-adapter.test.ts` (#8348) — again because `codecov/patch` only reads the root vitest suite
693+
(see [Test](#test)), not the package's own `node:test` suite.
694+
690695
## Governor ledger
691696

692697
`normalizeGovernorLedgerEvent` validates append-only governor decision rows before the local miner persists them.
Lines changed: 238 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,238 @@
1+
// Root-level vitest coverage twin for `packages/loopover-engine/src/miner/self-review-adapter.ts` (#8348).
2+
//
3+
// The self-review adapter (#2334) turns an attempt's live worktree diff state into the same inputs
4+
// `buildPredictedGateVerdict` and an injected slop-assessment function expect, so the miner's self-review pass
5+
// is byte-identical in shape to the live maintainer gate. It is fully exercised by the engine package's own
6+
// `node --test` suite (`packages/loopover-engine/test/self-review-adapter.test.ts`), but that runner is not part
7+
// of the root vitest run Codecov reads `codecov/patch` from, so it reports as ~0% covered despite being
8+
// genuinely tested (same blind spot as #6250). This twin imports the adapter functions via the engine barrel
9+
// and mirrors every scenario the package suite covers — matching the sibling pattern in
10+
// `test/unit/calibration-dashboard.test.ts`. It uses `buildPredictedGateVerdict` + `parseFocusManifest` and an
11+
// INJECTED `runSlopAssessment` fake exactly as the package suite does; the real `src/signals/slop.ts` is never
12+
// imported, and no source file is modified.
13+
import { describe, expect, it } from "vitest";
14+
import {
15+
buildPredictedGateVerdict,
16+
buildSelfReviewChangedPaths,
17+
buildSelfReviewPredictedGateInput,
18+
buildSelfReviewSlopInput,
19+
parseFocusManifest,
20+
runSelfReview,
21+
SELF_REVIEW_PASSING_CONCLUSION,
22+
} from "../../packages/loopover-engine/src/index";
23+
import type {
24+
AttemptDiffState,
25+
IssueRecord,
26+
PullRequestRecord,
27+
RepositoryRecord,
28+
SelfReviewContext,
29+
SelfReviewSlopAssessment,
30+
} from "../../packages/loopover-engine/src/index";
31+
32+
const REPO: RepositoryRecord = { fullName: "acme/widgets", owner: "acme", name: "widgets", isInstalled: true, isRegistered: true, isPrivate: false };
33+
34+
function openIssue(number: number, title: string): IssueRecord {
35+
return { repoFullName: "acme/widgets", number, title, state: "open", labels: [], linkedPrs: [] };
36+
}
37+
38+
function openPr(number: number, title: string, linkedIssues: number[] = []): PullRequestRecord {
39+
return { repoFullName: "acme/widgets", number, title, state: "open", authorLogin: "someone-else", linkedIssues, labels: [] };
40+
}
41+
42+
const BASE_DIFF_STATE: AttemptDiffState = {
43+
repoFullName: "acme/widgets",
44+
contributorLogin: "miner1",
45+
title: "Add retry to the upload client",
46+
body: "Closes #7",
47+
linkedIssues: [7],
48+
changedFiles: [{ path: "src/upload.ts", additions: 10, deletions: 2 }],
49+
};
50+
51+
function baseContext(overrides: Partial<SelfReviewContext> = {}): SelfReviewContext {
52+
return {
53+
manifest: parseFocusManifest({ gate: { duplicates: "block", linkedIssue: "advisory" } }),
54+
repo: REPO,
55+
issues: [openIssue(7, "Uploads should retry on 5xx")],
56+
pullRequests: [],
57+
...overrides,
58+
};
59+
}
60+
61+
const noopSlop: SelfReviewSlopAssessment = { slopRisk: 0, band: "clean", findings: [] };
62+
63+
describe("barrel: the self-review adapter is re-exported from the engine entrypoint (#2334)", () => {
64+
it("exposes the adapter functions and the passing-conclusion literal", () => {
65+
expect(typeof buildSelfReviewPredictedGateInput).toBe("function");
66+
expect(typeof buildSelfReviewChangedPaths).toBe("function");
67+
expect(typeof buildSelfReviewSlopInput).toBe("function");
68+
expect(typeof runSelfReview).toBe("function");
69+
expect(SELF_REVIEW_PASSING_CONCLUSION).toBe("success");
70+
});
71+
});
72+
73+
describe("buildSelfReviewPredictedGateInput — conditional spreads on optional identity fields", () => {
74+
it("maps identity fields, omitting keys the diff state left undefined (present body/linkedIssues, omitted labels/authorAssociation)", () => {
75+
const input = buildSelfReviewPredictedGateInput(BASE_DIFF_STATE);
76+
expect(input).toEqual({
77+
repoFullName: "acme/widgets",
78+
contributorLogin: "miner1",
79+
title: "Add retry to the upload client",
80+
body: "Closes #7",
81+
linkedIssues: [7],
82+
});
83+
expect("labels" in input).toBe(false);
84+
expect("authorAssociation" in input).toBe(false);
85+
});
86+
87+
it("includes labels and authorAssociation when the diff state sets them", () => {
88+
const input = buildSelfReviewPredictedGateInput({
89+
...BASE_DIFF_STATE,
90+
labels: ["gittensor:feature"],
91+
authorAssociation: "CONTRIBUTOR",
92+
});
93+
expect(input.labels).toEqual(["gittensor:feature"]);
94+
expect(input.authorAssociation).toBe("CONTRIBUTOR");
95+
});
96+
97+
it("omits body and linkedIssues when the diff state leaves them undefined", () => {
98+
const input = buildSelfReviewPredictedGateInput({
99+
repoFullName: "acme/widgets",
100+
contributorLogin: "miner1",
101+
title: "Add retry to the upload client",
102+
changedFiles: [],
103+
});
104+
expect("body" in input).toBe(false);
105+
expect("linkedIssues" in input).toBe(false);
106+
});
107+
});
108+
109+
describe("buildSelfReviewChangedPaths", () => {
110+
it("extracts the real changed file paths in order", () => {
111+
const paths = buildSelfReviewChangedPaths({
112+
...BASE_DIFF_STATE,
113+
changedFiles: [{ path: "src/a.ts" }, { path: "src/b.ts", additions: 5 }],
114+
});
115+
expect(paths).toEqual(["src/a.ts", "src/b.ts"]);
116+
});
117+
118+
it("returns an empty array for an empty changedFiles list", () => {
119+
expect(buildSelfReviewChangedPaths({ ...BASE_DIFF_STATE, changedFiles: [] })).toEqual([]);
120+
});
121+
});
122+
123+
describe("buildSelfReviewSlopInput — `??` description fallback and hasLinkedIssue derivation", () => {
124+
it("derives hasLinkedIssue from a non-empty linkedIssues array, threads inDuplicateCluster, and keeps a present body as the description", () => {
125+
const withIssue = buildSelfReviewSlopInput(BASE_DIFF_STATE, baseContext({ inDuplicateCluster: true }));
126+
expect(withIssue.hasLinkedIssue).toBe(true);
127+
expect(withIssue.inDuplicateCluster).toBe(true);
128+
expect(withIssue.description).toBe("Closes #7");
129+
});
130+
131+
it("an empty linkedIssues array yields hasLinkedIssue false, and an undefined body normalizes to null via `?? null`", () => {
132+
const withoutIssue = buildSelfReviewSlopInput({ ...BASE_DIFF_STATE, linkedIssues: [], body: undefined }, baseContext());
133+
expect(withoutIssue.hasLinkedIssue).toBe(false);
134+
expect(withoutIssue.description).toBe(null);
135+
});
136+
137+
it("an entirely undefined linkedIssues exercises the `?.length ?? 0` fallback chain distinctly from the empty-array case", () => {
138+
const undefinedIssues = buildSelfReviewSlopInput({ ...BASE_DIFF_STATE, linkedIssues: undefined }, baseContext());
139+
expect(undefinedIssues.hasLinkedIssue).toBe(false);
140+
});
141+
});
142+
143+
describe("runSelfReview", () => {
144+
it("a genuinely passing synthetic diff matches calling buildPredictedGateVerdict directly, and passesPredictedGate is true", () => {
145+
const context = baseContext();
146+
const result = runSelfReview(BASE_DIFF_STATE, context, { runSlopAssessment: () => noopSlop });
147+
148+
expect(result.predictedGateVerdict.conclusion).toBe("success");
149+
expect(result.passesPredictedGate).toBe(true);
150+
expect(result.changedPaths).toEqual(["src/upload.ts"]);
151+
152+
const direct = buildPredictedGateVerdict({
153+
input: buildSelfReviewPredictedGateInput(BASE_DIFF_STATE),
154+
manifest: context.manifest,
155+
repo: context.repo,
156+
issues: context.issues,
157+
pullRequests: context.pullRequests,
158+
changedPaths: ["src/upload.ts"],
159+
});
160+
expect(result.predictedGateVerdict).toEqual(direct);
161+
});
162+
163+
it("a genuinely blocked synthetic diff (duplicate PR) is a failure conclusion and passesPredictedGate is false", () => {
164+
const context = baseContext({ pullRequests: [openPr(42, "Retry uploads on 5xx responses", [7])] });
165+
const result = runSelfReview(BASE_DIFF_STATE, context, { runSlopAssessment: () => noopSlop });
166+
167+
expect(result.predictedGateVerdict.conclusion).toBe("failure");
168+
expect(result.passesPredictedGate).toBe(false);
169+
expect(result.predictedGateVerdict.blockers.some((b) => b.code === "duplicate_pr_risk")).toBe(true);
170+
171+
const direct = buildPredictedGateVerdict({
172+
input: buildSelfReviewPredictedGateInput(BASE_DIFF_STATE),
173+
manifest: context.manifest,
174+
repo: context.repo,
175+
issues: context.issues,
176+
pullRequests: context.pullRequests,
177+
changedPaths: ["src/upload.ts"],
178+
});
179+
expect(result.predictedGateVerdict).toEqual(direct);
180+
});
181+
182+
it("never treats a non-success conclusion as passing — the hard defense-in-depth requirement (both boundary arms)", () => {
183+
const passing = runSelfReview(BASE_DIFF_STATE, baseContext(), { runSlopAssessment: () => noopSlop });
184+
expect(passing.passesPredictedGate).toBe(true);
185+
186+
const blocked = runSelfReview(BASE_DIFF_STATE, baseContext({ pullRequests: [openPr(42, "dup", [7])] }), {
187+
runSlopAssessment: () => noopSlop,
188+
});
189+
expect(blocked.predictedGateVerdict.conclusion).not.toBe(SELF_REVIEW_PASSING_CONCLUSION);
190+
expect(blocked.passesPredictedGate).toBe(false);
191+
});
192+
193+
it("threads changedPaths through so path-dependent checks are evaluated, not silently skipped", () => {
194+
const context = baseContext({
195+
manifest: parseFocusManifest({ duplicates: "block", linkedIssue: "advisory", wantedPaths: ["docs/**"] } as never),
196+
});
197+
const result = runSelfReview(BASE_DIFF_STATE, context, { runSlopAssessment: () => noopSlop });
198+
expect(result.changedPaths).toEqual(["src/upload.ts"]);
199+
});
200+
201+
it("forwards the optional context fields (bounties, issueQuality, confirmedContributor) through to buildPredictedGateVerdict", () => {
202+
const context = baseContext({ confirmedContributor: true, bounties: [], issueQuality: null });
203+
const result = runSelfReview(BASE_DIFF_STATE, context, { runSlopAssessment: () => noopSlop });
204+
205+
expect(result.predictedGateVerdict.confirmedContributor).toBe(true);
206+
207+
const direct = buildPredictedGateVerdict({
208+
input: buildSelfReviewPredictedGateInput(BASE_DIFF_STATE),
209+
manifest: context.manifest,
210+
repo: context.repo,
211+
issues: context.issues,
212+
pullRequests: context.pullRequests,
213+
bounties: [],
214+
issueQuality: null,
215+
confirmedContributor: true,
216+
changedPaths: ["src/upload.ts"],
217+
});
218+
expect(result.predictedGateVerdict).toEqual(direct);
219+
});
220+
221+
it("passes the exact constructed slop input to the injected dependency and returns its result unchanged", () => {
222+
let received: unknown;
223+
const distinctiveSlop: SelfReviewSlopAssessment = {
224+
slopRisk: 42,
225+
band: "elevated",
226+
findings: [{ code: "x", title: "t", severity: "warning", detail: "d" }],
227+
};
228+
const result = runSelfReview(BASE_DIFF_STATE, baseContext(), {
229+
runSlopAssessment: (input) => {
230+
received = input;
231+
return distinctiveSlop;
232+
},
233+
});
234+
235+
expect(received).toEqual(buildSelfReviewSlopInput(BASE_DIFF_STATE, baseContext()));
236+
expect(result.slopAssessment).toEqual(distinctiveSlop);
237+
});
238+
});

0 commit comments

Comments
 (0)