Skip to content

Commit 8fcbdd1

Browse files
philluiz2323JSONbored
authored andcommitted
feat(miner): capture feasibility-verdict rule-fired signals in attempt-cli (#8543)
AMS's calibration capture was one-sided: eligibility-exclusion reasons were already recorded as rule-fired signals on discovery, but the other deterministic gate an attempt passes through -- the feasibility verdict -- recorded nothing. Wires attempt-cli's existing infeasible branch (ready: false) to record one RuleFiredEvent per avoid/raise reason through createSignalTrackingStore, mirroring discover-cli's recordEligibilityExclusionSignals exactly: same targetKey format, same best-effort discipline (a store-open failure or a single write failure never changes console output, JSON result shape, or exit code), same initSignalTrackingStore seam shape. No pure-module changes, no metadata, no output-format changes -- attempt-cli.ts and its tests only.
1 parent 59faa47 commit 8fcbdd1

2 files changed

Lines changed: 213 additions & 2 deletions

File tree

packages/loopover-miner/lib/attempt-cli.ts

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import {
2020
resolveCodingAgentModeFromConfig,
2121
resolveFirstConfiguredCodingAgentDriverName,
2222
} from "@loopover/engine";
23-
import type { AttemptDbFork, AttemptDbForkConfig, CodingAgentExecutionMode, FeasibilityVerdict, LocalWriteActionSpec } from "@loopover/engine";
23+
import type { AttemptDbFork, AttemptDbForkConfig, CodingAgentExecutionMode, FeasibilityVerdict, LocalWriteActionSpec, SignalStore } from "@loopover/engine";
2424
import { argsWantJson, describeCliError, reportCliFailure } from "./cli-error.js";
2525
import { resolveAttemptDbForkConfig } from "./attempt-db-fork-config.js";
2626
import { constructProductionCodingAgentDriver } from "./coding-agent-construction.js";
@@ -33,8 +33,9 @@ import { resolveMinerGoalSpec } from "./miner-goal-spec.js";
3333
import { resolveClaimConflict } from "./claim-conflict-resolver.js";
3434
import type { ClaimConflictResult, resolveClaimConflict as ResolveClaimConflictFn } from "./claim-conflict-resolver.js";
3535
import { parsePrNumberFromExecResult } from "./pr-number-parse.js";
36-
import { initEventLedger } from "./event-ledger.js";
36+
import { appendEvent, initEventLedger, readEvents } from "./event-ledger.js";
3737
import type { EventLedger } from "./event-ledger.js";
38+
import { createSignalTrackingStore } from "./signal-tracking-store.js";
3839
import { initAttemptLog } from "./attempt-log.js";
3940
import type { AttemptLog } from "./attempt-log.js";
4041
import { initGovernorLedger } from "./governor-ledger.js";
@@ -137,6 +138,7 @@ export type RunAttemptOptions = {
137138
initEventLedger?: () => EventLedger;
138139
initAttemptLog?: () => AttemptLog;
139140
initGovernorLedger?: () => GovernorLedger;
141+
initSignalTrackingStore?: () => SignalStore;
140142
buildAttemptDeps?: typeof buildAttemptDeps;
141143
resolveRejectionSignaled?: typeof ResolveRejectionSignaledFn;
142144
fetchImpl?: SelfReviewContextFetch;
@@ -291,6 +293,44 @@ export function buildAttemptDeps(
291293
};
292294
}
293295

296+
// #8543: the default SignalStore, backed by the miner's own shared local event ledger -- same singleton
297+
// discover-cli.ts's initDefaultSignalTrackingStore already uses, no extra lifecycle management needed here.
298+
function initDefaultSignalTrackingStore(): SignalStore {
299+
return createSignalTrackingStore({ appendEvent, readEvents });
300+
}
301+
302+
// #8543: records each avoid/raise reason a feasibility verdict fired as a rule-fired signal, so AMS can
303+
// precision-score feasibility rules the same way discover-cli.ts's recordEligibilityExclusionSignals already
304+
// does for eligibility exclusions. Best-effort, mirrored verbatim from that sibling: a store-open failure or a
305+
// single write failure never changes the CLI's console output, JSON result shape, or exit code -- the
306+
// infeasible branch still returns 4 either way. Every ready:false occurrence records, no dedup across repeated
307+
// attempts at the same issue -- each attempt is a distinct decision instance.
308+
async function recordFeasibilityVerdictSignals(
309+
feasibility: { avoidReasons: readonly string[]; raiseReasons: readonly string[] },
310+
target: { repoFullName: string; issueNumber: number },
311+
options: Pick<RunAttemptOptions, "initSignalTrackingStore">,
312+
nowMs: number,
313+
): Promise<void> {
314+
let store: SignalStore | null = null;
315+
try {
316+
store = (options.initSignalTrackingStore ?? initDefaultSignalTrackingStore)();
317+
} catch {
318+
store = null;
319+
}
320+
if (!store) return;
321+
const occurredAt = new Date(nowMs).toISOString();
322+
const targetKey = `${target.repoFullName}#issue-${target.issueNumber}`;
323+
const fired: Array<{ ruleId: string; outcome: "avoid" | "raise" }> = [
324+
...feasibility.avoidReasons.map((ruleId) => ({ ruleId, outcome: "avoid" as const })),
325+
...feasibility.raiseReasons.map((ruleId) => ({ ruleId, outcome: "raise" as const })),
326+
];
327+
for (const entry of fired) {
328+
await store
329+
.recordRuleFired({ ruleId: entry.ruleId, targetKey, outcome: entry.outcome, occurredAt })
330+
.catch(() => undefined);
331+
}
332+
}
333+
294334
/**
295335
* Run the `attempt` CLI subcommand end to end: resolveRejectionSignaled (before consuming a worktree slot) ->
296336
* acquire a concurrency slot -> assemble real AttemptDeps -> prepare a REAL git worktree -> fetch a real
@@ -520,6 +560,12 @@ export async function runAttempt(args: string[], options: RunAttemptOptions = {}
520560

521561
if (!codingTaskSpec.ready) {
522562
const reason = `infeasible_${codingTaskSpec.verdict}`;
563+
await recordFeasibilityVerdictSignals(
564+
codingTaskSpec.feasibility,
565+
{ repoFullName: parsed.repoFullName, issueNumber: parsed.issueNumber },
566+
options,
567+
nowMs,
568+
);
523569
attemptLog.appendAttemptLogEvent({
524570
eventType: "attempt_aborted",
525571
attemptId,

test/unit/miner-attempt-cli.test.ts

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,17 @@ function fakeLoopResult(overrides: Record<string, unknown> = {}) {
8787
};
8888
}
8989

90+
/** A no-op SignalStore double (#8543), for tests that reach the infeasible branch but don't themselves assert
91+
* on signal capture -- keeps them off the real on-disk event-ledger fallback under ~/.config, the same leak
92+
* class discover-cli.ts's own initDefaultSignalTrackingStore comment already documents. */
93+
function fakeSignalStore() {
94+
return {
95+
recordRuleFired: vi.fn(async (_event: { ruleId: string; targetKey: string; outcome: string; occurredAt: string }) => undefined),
96+
recordHumanOverride: vi.fn(async () => undefined),
97+
queryRuleHistory: vi.fn(async () => ({ fired: [], overrides: [] })),
98+
};
99+
}
100+
90101
/** The default set of injected options a test needs to reach past every real dependency and into (or
91102
* through) the final runMinerAttempt call, without doing any real network/git/subprocess work. */
92103
function readyPipelineOptions(overrides: Record<string, unknown> = {}) {
@@ -1192,6 +1203,7 @@ describe("runAttempt (#5132)", () => {
11921203
initEventLedger: () => eventLedger,
11931204
initAttemptLog: () => attemptLog,
11941205
initGovernorLedger: () => governorLedger,
1206+
initSignalTrackingStore: () => fakeSignalStore(),
11951207
...readyPipelineOptions({
11961208
buildCodingTaskSpec: () => ({
11971209
ready: false,
@@ -1225,6 +1237,157 @@ describe("runAttempt (#5132)", () => {
12251237
expect(cleanupAttemptWorktreeSpy).toHaveBeenCalledWith(expect.any(String), expect.any(String), true);
12261238
});
12271239

1240+
describe("feasibility-verdict signal capture (#8543)", () => {
1241+
it("records one rule-fired signal per avoid AND raise reason, with the exact ruleId/outcome/targetKey shape", async () => {
1242+
const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers();
1243+
vi.spyOn(console, "log").mockImplementation(() => undefined);
1244+
const store = fakeSignalStore();
1245+
1246+
await runAttempt(["acme/widgets", "7", "--miner-login", "alice", "--json"], {
1247+
env: { MINER_CODING_AGENT_PROVIDER: "noop" },
1248+
openWorktreeAllocator: () => allocator,
1249+
openClaimLedger: () => claimLedger,
1250+
initEventLedger: () => eventLedger,
1251+
initAttemptLog: () => attemptLog,
1252+
initGovernorLedger: () => governorLedger,
1253+
initSignalTrackingStore: () => store,
1254+
...readyPipelineOptions({
1255+
buildCodingTaskSpec: () => ({
1256+
ready: false,
1257+
verdict: "avoid",
1258+
feasibility: {
1259+
verdict: "avoid",
1260+
avoidReasons: ["claim_status_solved", "issue_quality_do_not_use"],
1261+
raiseReasons: ["duplicate_cluster_high"],
1262+
summary: "not feasible",
1263+
},
1264+
}),
1265+
}),
1266+
});
1267+
1268+
expect(store.recordRuleFired).toHaveBeenCalledTimes(3);
1269+
expect(store.recordRuleFired).toHaveBeenCalledWith(
1270+
expect.objectContaining({ ruleId: "claim_status_solved", outcome: "avoid", targetKey: "acme/widgets#issue-7" }),
1271+
);
1272+
expect(store.recordRuleFired).toHaveBeenCalledWith(
1273+
expect.objectContaining({ ruleId: "issue_quality_do_not_use", outcome: "avoid", targetKey: "acme/widgets#issue-7" }),
1274+
);
1275+
expect(store.recordRuleFired).toHaveBeenCalledWith(
1276+
expect.objectContaining({ ruleId: "duplicate_cluster_high", outcome: "raise", targetKey: "acme/widgets#issue-7" }),
1277+
);
1278+
// No metadata (raw-context capture is a separate issue -- not this one) and a well-formed ISO timestamp.
1279+
for (const [event] of store.recordRuleFired.mock.calls) {
1280+
expect(event).not.toHaveProperty("metadata");
1281+
expect(new Date(event.occurredAt).toISOString()).toBe(event.occurredAt);
1282+
}
1283+
});
1284+
1285+
it("records zero fired events on the ready:true path", async () => {
1286+
const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers();
1287+
vi.spyOn(console, "log").mockImplementation(() => undefined);
1288+
const store = fakeSignalStore();
1289+
1290+
await runAttempt(["acme/widgets", "7", "--miner-login", "alice", "--json"], {
1291+
env: { MINER_CODING_AGENT_PROVIDER: "noop" },
1292+
openWorktreeAllocator: () => allocator,
1293+
openClaimLedger: () => claimLedger,
1294+
initEventLedger: () => eventLedger,
1295+
initAttemptLog: () => attemptLog,
1296+
initGovernorLedger: () => governorLedger,
1297+
initSignalTrackingStore: () => store,
1298+
...readyPipelineOptions({ runMinerAttempt: vi.fn(async () => ({ outcome: "submitted", loopResult: fakeLoopResult() }) as never) }),
1299+
});
1300+
1301+
expect(store.recordRuleFired).not.toHaveBeenCalled();
1302+
});
1303+
1304+
it("a throwing initSignalTrackingStore never changes the exit code, console output, or JSON result", async () => {
1305+
const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers();
1306+
const log = vi.spyOn(console, "log").mockImplementation(() => undefined);
1307+
1308+
const exitCode = await runAttempt(["acme/widgets", "7", "--miner-login", "alice", "--json"], {
1309+
env: { MINER_CODING_AGENT_PROVIDER: "noop" },
1310+
attemptId: "infeasible-attempt",
1311+
openWorktreeAllocator: () => allocator,
1312+
openClaimLedger: () => claimLedger,
1313+
initEventLedger: () => eventLedger,
1314+
initAttemptLog: () => attemptLog,
1315+
initGovernorLedger: () => governorLedger,
1316+
initSignalTrackingStore: () => {
1317+
throw new Error("store unavailable");
1318+
},
1319+
...readyPipelineOptions({
1320+
buildCodingTaskSpec: () => ({
1321+
ready: false,
1322+
verdict: "raise",
1323+
feasibility: { verdict: "raise", avoidReasons: [], raiseReasons: ["target_not_found"], summary: "issue not found" },
1324+
}),
1325+
}),
1326+
});
1327+
1328+
expect(exitCode).toBe(4);
1329+
expect(JSON.parse(String(log.mock.calls[0]?.[0]))).toEqual({
1330+
outcome: "blocked_infeasible",
1331+
reason: "infeasible_raise",
1332+
verdict: "raise",
1333+
avoidReasons: [],
1334+
raiseReasons: ["target_not_found"],
1335+
repoFullName: "acme/widgets",
1336+
issueNumber: 7,
1337+
minerLogin: "alice",
1338+
base: "main",
1339+
mode: "dry_run",
1340+
attemptId: "infeasible-attempt",
1341+
});
1342+
});
1343+
1344+
it("a store whose recordRuleFired rejects never changes the exit code, console output, or JSON result", async () => {
1345+
const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers();
1346+
const log = vi.spyOn(console, "log").mockImplementation(() => undefined);
1347+
const rejectingStore = {
1348+
recordRuleFired: vi.fn(async () => {
1349+
throw new Error("write failed");
1350+
}),
1351+
recordHumanOverride: vi.fn(async () => undefined),
1352+
queryRuleHistory: vi.fn(async () => ({ fired: [], overrides: [] })),
1353+
};
1354+
1355+
const exitCode = await runAttempt(["acme/widgets", "7", "--miner-login", "alice", "--json"], {
1356+
env: { MINER_CODING_AGENT_PROVIDER: "noop" },
1357+
attemptId: "infeasible-attempt",
1358+
openWorktreeAllocator: () => allocator,
1359+
openClaimLedger: () => claimLedger,
1360+
initEventLedger: () => eventLedger,
1361+
initAttemptLog: () => attemptLog,
1362+
initGovernorLedger: () => governorLedger,
1363+
initSignalTrackingStore: () => rejectingStore,
1364+
...readyPipelineOptions({
1365+
buildCodingTaskSpec: () => ({
1366+
ready: false,
1367+
verdict: "raise",
1368+
feasibility: { verdict: "raise", avoidReasons: [], raiseReasons: ["target_not_found"], summary: "issue not found" },
1369+
}),
1370+
}),
1371+
});
1372+
1373+
expect(exitCode).toBe(4);
1374+
expect(JSON.parse(String(log.mock.calls[0]?.[0]))).toEqual({
1375+
outcome: "blocked_infeasible",
1376+
reason: "infeasible_raise",
1377+
verdict: "raise",
1378+
avoidReasons: [],
1379+
raiseReasons: ["target_not_found"],
1380+
repoFullName: "acme/widgets",
1381+
issueNumber: 7,
1382+
minerLogin: "alice",
1383+
base: "main",
1384+
mode: "dry_run",
1385+
attemptId: "infeasible-attempt",
1386+
});
1387+
expect(rejectingStore.recordRuleFired).toHaveBeenCalledTimes(1);
1388+
});
1389+
});
1390+
12281391
it("REGRESSION: infeasible WITHOUT --json prints the feasibility verdict on stderr and exits 4", async () => {
12291392
const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers();
12301393
const error = vi.spyOn(console, "error").mockImplementation(() => undefined);
@@ -1236,6 +1399,7 @@ describe("runAttempt (#5132)", () => {
12361399
initEventLedger: () => eventLedger,
12371400
initAttemptLog: () => attemptLog,
12381401
initGovernorLedger: () => governorLedger,
1402+
initSignalTrackingStore: () => fakeSignalStore(),
12391403
...readyPipelineOptions({
12401404
buildCodingTaskSpec: () => ({
12411405
ready: false,
@@ -1879,6 +2043,7 @@ describe("runAttempt: real claim-ledger wiring (#5393)", () => {
18792043
initEventLedger: () => eventLedger,
18802044
initAttemptLog: () => attemptLog,
18812045
initGovernorLedger: () => governorLedger,
2046+
initSignalTrackingStore: () => fakeSignalStore(),
18822047
...readyPipelineOptions({
18832048
buildCodingTaskSpec: () => ({
18842049
ready: false,

0 commit comments

Comments
 (0)