From dfced6b226a99a887166fe4bf4effc290976901b Mon Sep 17 00:00:00 2001 From: rsnetworkinginc <272110387+rsnetworkinginc@users.noreply.github.com> Date: Fri, 24 Jul 2026 18:16:56 +0300 Subject: [PATCH] fix(miner): resolve metrics-cli correct/incorrect via the calibration outcome-join --- packages/loopover-miner/lib/calibration.ts | 2 +- packages/loopover-miner/lib/metrics-cli.ts | 105 ++++++++++++++--- test/unit/miner-metrics-cli.test.ts | 130 ++++++++++++++++++--- 3 files changed, 206 insertions(+), 31 deletions(-) diff --git a/packages/loopover-miner/lib/calibration.ts b/packages/loopover-miner/lib/calibration.ts index c50a8f8e59..31ef9af6d9 100644 --- a/packages/loopover-miner/lib/calibration.ts +++ b/packages/loopover-miner/lib/calibration.ts @@ -27,7 +27,7 @@ export { isCalibrationReport, isCalibrationRow, isObservedOutcomeRecord, isPredi * unrecognized. `value` is always the already-validated non-empty string field of a record (the type guards run * first), so no non-string handling is needed here. Accepts both the predicted (`merge`/`close`/`hold`) and the * realized (`merged`/`closed`) forms. */ -function normalizeDecision(value: string): "merge" | "close" | "hold" | "" { +export function normalizeDecision(value: string): "merge" | "close" | "hold" | "" { const decision = value.trim().toLowerCase(); if (decision === "merge" || decision === "merged") return "merge"; if (decision === "close" || decision === "closed") return "close"; diff --git a/packages/loopover-miner/lib/metrics-cli.ts b/packages/loopover-miner/lib/metrics-cli.ts index a94bb75bc6..995152f42b 100644 --- a/packages/loopover-miner/lib/metrics-cli.ts +++ b/packages/loopover-miner/lib/metrics-cli.ts @@ -1,35 +1,96 @@ import { renderMinerPredictionMetrics } from "@loopover/engine"; import type { MinerPredictionMetricRow } from "@loopover/engine"; -import { initPredictionLedger } from "./prediction-ledger.js"; +import { toOutcomeRecords, toPredictionRecords } from "./calibration-cli.js"; +import { normalizeDecision } from "./calibration.js"; +import { initEventLedger, resolveEventLedgerDbPath } from "./event-ledger.js"; +import type { EventLedger, LedgerEntry } from "./event-ledger.js"; +import { initPredictionLedger, resolvePredictionLedgerDbPath } from "./prediction-ledger.js"; import type { PredictionLedger } from "./prediction-ledger.js"; import { argsWantJson, describeCliError, reportCliFailure } from "./cli-error.js"; // `metrics` (#4838): render the miner's prediction-calibration counters as Prometheus text-exposition to stdout, // for a scrape wrapper or cron redirect. The counters are produced by the engine's already-built // renderMinerPredictionMetrics (packages/loopover-engine/src/miner-prediction-metrics.ts) -- this command only -// reads the local prediction ledger and feeds it in, never touching the renderer itself. Strictly local + offline: -// no network, no writes. +// reads the local prediction ledger + realized `pr_outcome` event stream and feeds them in, never touching the +// renderer itself. Strictly local + offline: no network, no writes. const METRICS_USAGE = "Usage: loopover-miner metrics"; +// Key an outcome/prediction by its (project, targetId) exactly as calibration.ts's own join does, so metrics +// resolves each prediction against the identical (project, targetId) pairing buildCalibrationReport uses. +function recordKey(project: string, targetId: string): string { + return `${project} ${targetId}`; +} + +// Resolve one prediction's realized-outcome pairing against its observed decision (`undefined` when none was +// observed for its (project, targetId)), mirroring buildCalibrationReport's own classification: a prediction is +// scored only when it itself normalizes to `merge`/`close` AND its observed outcome also normalizes to +// `merge`/`close` -- `true` when they match, `false` when they differ. A `hold`/unrecognized prediction, a +// still-pending row (no outcome), or an unclassifiable outcome all stay unresolved (`correct` left unset); an +// undecided row is never fabricated as `false`. +function resolveCorrect(predictedDecision: string, observedDecision: string | undefined): boolean | undefined { + const predicted = normalizeDecision(predictedDecision); + if (predicted !== "merge" && predicted !== "close") return undefined; + if (observedDecision === undefined) return undefined; + const observed = normalizeDecision(observedDecision); + if (observed !== "merge" && observed !== "close") return undefined; + return predicted === observed; +} + /** - * Project prediction-ledger rows onto the engine renderer's metric-row shape -- the predicted `conclusion` only. - * The realized-outcome pairing (`correct`) is intentionally left unset: the miner has no outcome-join yet, so the - * correct/incorrect counters stay zero and only `predictions_total{conclusion}` moves -- exactly how the renderer - * is designed to degrade before outcome-pairing exists (see its header comment). + * Project prediction-ledger rows onto the engine renderer's metric-row shape: the predicted `conclusion`, plus the + * realized-outcome pairing (`correct`) resolved through the SAME join `calibration-cli.ts` already uses (#8315). + * Each prediction's outcome is looked up by its `(project, targetId)` among the latest `pr_outcome` events + * (`toOutcomeRecords`) and compared against the prediction (`toPredictionRecords`), both normalized through + * calibration.ts's `normalizeDecision` exactly as `buildCalibrationReport` does. A row is left unresolved + * (`correct` omitted) when it is still pending (no realized outcome), when its own conclusion is `hold` (which has + * no `merged`/`closed` counterpart), or when the realized outcome is unclassifiable -- never fabricating `false` + * for an undecided row. Resolved rows carry `correct: true` when the normalized prediction matches the realized + * decision, `false` when they differ. */ -export function collectPredictionMetricRows(ledger: PredictionLedger): MinerPredictionMetricRow[] { - return ledger.readPredictions().map((entry) => ({ conclusion: entry.conclusion })); +export function collectPredictionMetricRows( + ledger: PredictionLedger, + events: LedgerEntry[] = [], +): MinerPredictionMetricRow[] { + const outcomeByKey = new Map(); + for (const outcome of toOutcomeRecords(events)) { + outcomeByKey.set(recordKey(outcome.project, outcome.targetId), outcome.outcomeDecision); + } + return toPredictionRecords(ledger.readPredictions()).map((prediction) => { + const correct = resolveCorrect(prediction.predictedDecision, outcomeByKey.get(recordKey(prediction.project, prediction.targetId))); + return correct === undefined + ? { conclusion: prediction.predictedDecision } + : { conclusion: prediction.predictedDecision, correct }; + }); } // Open the local prediction ledger (or a test-injected one) for the duration of `run`, closing it only when we // opened it -- an injected ledger is owned by the caller. Mirrors event-ledger-cli.js's withEventLedger. function withPredictionLedger( options: { initPredictionLedger?: () => PredictionLedger }, + env: Record, run: (ledger: PredictionLedger) => T, ): T { const ownsLedger = options.initPredictionLedger === undefined; - const ledger = (options.initPredictionLedger ?? initPredictionLedger)(); + const ledger = (options.initPredictionLedger ?? (() => initPredictionLedger(resolvePredictionLedgerDbPath(env))))(); + try { + return run(ledger); + } finally { + if (ownsLedger) ledger.close(); + } +} + +// Open the local event ledger (or a test-injected one) for the duration of `run`, closing it only when we opened +// it. Mirrors withPredictionLedger above and calibration-cli.ts's bare `calibration` command: open via +// initEventLedger(resolveEventLedgerDbPath(env)), read once, close in a finally -- keeping `metrics` strictly +// read-only and offline with no behavior change to its zero-argument usage contract. +function withEventLedger( + options: { initEventLedger?: () => EventLedger }, + env: Record, + run: (ledger: EventLedger) => T, +): T { + const ownsLedger = options.initEventLedger === undefined; + const ledger = (options.initEventLedger ?? (() => initEventLedger(resolveEventLedgerDbPath(env))))(); try { return run(ledger); } finally { @@ -37,18 +98,28 @@ function withPredictionLedger( } } -export function runMetrics(args: string[], options: { initPredictionLedger?: () => PredictionLedger } = {}): number { +export function runMetrics( + args: string[], + options: { + initPredictionLedger?: () => PredictionLedger; + initEventLedger?: () => EventLedger; + env?: Record; + } = {}, +): number { if (args.length > 0) { return reportCliFailure(argsWantJson(args), METRICS_USAGE); } + const env = options.env ?? process.env; try { - return withPredictionLedger(options, (ledger) => { - // renderMinerPredictionMetrics returns a newline-terminated document; console.log re-adds the terminator, so - // trim it to emit exactly one trailing newline. - console.log(renderMinerPredictionMetrics(collectPredictionMetricRows(ledger)).trimEnd()); - return 0; - }); + return withPredictionLedger(options, env, (ledger) => + withEventLedger(options, env, (eventLedger) => { + // renderMinerPredictionMetrics returns a newline-terminated document; console.log re-adds the terminator, + // so trim it to emit exactly one trailing newline. + console.log(renderMinerPredictionMetrics(collectPredictionMetricRows(ledger, eventLedger.readEvents())).trimEnd()); + return 0; + }), + ); } catch (error) { return reportCliFailure(argsWantJson(args), describeCliError(error)); } diff --git a/test/unit/miner-metrics-cli.test.ts b/test/unit/miner-metrics-cli.test.ts index 9fbf1b8000..d8919b0d35 100644 --- a/test/unit/miner-metrics-cli.test.ts +++ b/test/unit/miner-metrics-cli.test.ts @@ -2,7 +2,15 @@ import { mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; -import { initPredictionLedger } from "../../packages/loopover-miner/lib/prediction-ledger.js"; +import { + initEventLedger, + resolveEventLedgerDbPath, +} from "../../packages/loopover-miner/lib/event-ledger.js"; +import type { EventLedger, LedgerEntry } from "../../packages/loopover-miner/lib/event-ledger.js"; +import { + initPredictionLedger, + resolvePredictionLedgerDbPath, +} from "../../packages/loopover-miner/lib/prediction-ledger.js"; import { collectPredictionMetricRows, runMetrics, @@ -20,16 +28,56 @@ function tempLedger(): PredictionLedger { return ledger; } -function tempDbPath() { +function tempEventLedger(): EventLedger { + const root = mkdtempSync(join(tmpdir(), "loopover-miner-metrics-cli-evt-")); + roots.push(root); + const ledger = initEventLedger(join(root, "event-ledger.sqlite3")); + ledgers.push(ledger); + return ledger; +} + +function tempDir() { const root = mkdtempSync(join(tmpdir(), "loopover-miner-metrics-cli-")); roots.push(root); - return join(root, "prediction-ledger.sqlite3"); + return root; +} + +/** Env pointing both local stores at a fresh temp config dir (mirrors miner-calibration-cli.test.ts). */ +function tempConfigEnv(): Record { + return { LOOPOVER_MINER_CONFIG_DIR: tempDir() }; } function appendPrediction(ledger: PredictionLedger, targetId: number, conclusion: string) { ledger.appendPrediction({ repoFullName: "acme/widgets", targetId, conclusion, pack: "gittensor", engineVersion: "0.2.0" }); } +function seedPredictionEnv(env: Record, targetId: number, conclusion: string) { + const store = initPredictionLedger(resolvePredictionLedgerDbPath(env)); + store.appendPrediction({ repoFullName: "acme/widgets", targetId, conclusion, pack: "gittensor", engineVersion: "0.2.0" }); + store.close(); +} + +function seedOutcomeEnv(env: Record, prNumber: number, decision: string) { + const ledger = initEventLedger(resolveEventLedgerDbPath(env)); + ledger.appendEvent({ type: "pr_outcome", repoFullName: "acme/widgets", payload: { prNumber, decision } }); + ledger.close(); +} + +// Build a raw `pr_outcome` ledger row (or a deliberately malformed one) for the in-process join, exactly as +// toOutcomeRecords would read it off the event ledger. +let seq = 0; +function outcomeEvent(prNumber: unknown, decision: unknown, repoFullName: string | null = "acme/widgets"): LedgerEntry { + seq += 1; + return { + id: seq, + seq, + type: "pr_outcome", + repoFullName, + payload: { prNumber, decision } as Record, + createdAt: new Date(seq * 1000).toISOString(), + }; +} + afterEach(() => { for (const ledger of ledgers.splice(0)) ledger.close(); vi.restoreAllMocks(); @@ -37,7 +85,7 @@ afterEach(() => { }); describe("loopover-miner metrics CLI (#4838)", () => { - it("collectPredictionMetricRows projects ledger rows onto the renderer's conclusion-only shape", () => { + it("collectPredictionMetricRows leaves `correct` unset when no outcome events are supplied", () => { const ledger = tempLedger(); appendPrediction(ledger, 1, "merge"); appendPrediction(ledger, 2, "close"); @@ -51,34 +99,41 @@ describe("loopover-miner metrics CLI (#4838)", () => { appendPrediction(ledger, 3, "merge"); const log = vi.spyOn(console, "log").mockImplementation(() => undefined); - expect(runMetrics([], { initPredictionLedger: () => ledger })).toBe(0); + // Inject an empty event ledger: no realized outcomes, so the correct/incorrect counters stay zero. + expect(runMetrics([], { initPredictionLedger: () => ledger, initEventLedger: () => tempEventLedger() })).toBe(0); const text = String(log.mock.calls[0]?.[0]); expect(text).toContain("# TYPE loopover_miner_predictions_total counter"); // Series are emitted in sorted conclusion order, so "close" precedes "merge". expect(text).toContain('loopover_miner_predictions_total{conclusion="close"} 1'); expect(text).toContain('loopover_miner_predictions_total{conclusion="merge"} 2'); - // No outcome-join exists yet, so both the correct and incorrect counters stay zero. + // No realized outcomes joined, so both the correct and incorrect counters stay zero. expect(text).toContain("loopover_miner_prediction_correct_total 0"); expect(text).toContain("loopover_miner_prediction_incorrect_total 0"); // The output is a single, once-terminated document (no doubled trailing blank line). expect(text.endsWith("\n")).toBe(false); }); - it("runMetrics opens and closes its own default ledger when none is injected", () => { - const dbPath = tempDbPath(); - const seed = initPredictionLedger(dbPath); + it("runMetrics opens and closes its own default ledgers when none are injected", () => { + const dir = tempDir(); + const predictionDbPath = join(dir, "prediction-ledger.sqlite3"); + const eventDbPath = join(dir, "event-ledger.sqlite3"); + const seed = initPredictionLedger(predictionDbPath); appendPrediction(seed, 1, "hold"); seed.close(); - const prev = process.env.LOOPOVER_MINER_PREDICTION_LEDGER_DB; - process.env.LOOPOVER_MINER_PREDICTION_LEDGER_DB = dbPath; + const prevPrediction = process.env.LOOPOVER_MINER_PREDICTION_LEDGER_DB; + const prevEvent = process.env.LOOPOVER_MINER_EVENT_LEDGER_DB; + process.env.LOOPOVER_MINER_PREDICTION_LEDGER_DB = predictionDbPath; + process.env.LOOPOVER_MINER_EVENT_LEDGER_DB = eventDbPath; const log = vi.spyOn(console, "log").mockImplementation(() => undefined); try { expect(runMetrics([])).toBe(0); } finally { - if (prev === undefined) delete process.env.LOOPOVER_MINER_PREDICTION_LEDGER_DB; - else process.env.LOOPOVER_MINER_PREDICTION_LEDGER_DB = prev; + if (prevPrediction === undefined) delete process.env.LOOPOVER_MINER_PREDICTION_LEDGER_DB; + else process.env.LOOPOVER_MINER_PREDICTION_LEDGER_DB = prevPrediction; + if (prevEvent === undefined) delete process.env.LOOPOVER_MINER_EVENT_LEDGER_DB; + else process.env.LOOPOVER_MINER_EVENT_LEDGER_DB = prevEvent; } expect(String(log.mock.calls[0]?.[0])).toContain('loopover_miner_predictions_total{conclusion="hold"} 1'); }); @@ -122,4 +177,53 @@ describe("loopover-miner metrics CLI (#4838)", () => { ).toBe(2); expect(error).toHaveBeenCalledWith("prediction-ledger-unavailable"); }); + + describe("outcome-join (#8315)", () => { + it("resolves each prediction's `correct` against the realized pr_outcome via the calibration join", () => { + const ledger = tempLedger(); + appendPrediction(ledger, 1, "merge"); // realized merged -> correct + appendPrediction(ledger, 2, "merge"); // realized closed -> incorrect + appendPrediction(ledger, 3, "close"); // realized closed -> correct + appendPrediction(ledger, 4, "close"); // realized merged -> incorrect + appendPrediction(ledger, 5, "hold"); // hold has no realized counterpart -> unresolved + appendPrediction(ledger, 6, "merge"); // no outcome yet (pending) -> unresolved + appendPrediction(ledger, 7, "merge"); // outcome present but unclassifiable -> unresolved + + const events: LedgerEntry[] = [ + outcomeEvent(1, "merged"), + outcomeEvent(2, "closed"), + outcomeEvent(3, "closed"), + outcomeEvent(4, "merged"), + outcomeEvent(5, "merged"), + // no event for prediction 6 (still pending) + outcomeEvent(7, "reopened"), // a well-formed pr_outcome whose decision is neither merged nor closed + outcomeEvent("not-a-number", "merged"), // malformed: non-integer prNumber -> skipped by toOutcomeRecords + ]; + + expect(collectPredictionMetricRows(ledger, events)).toEqual([ + { conclusion: "merge", correct: true }, + { conclusion: "merge", correct: false }, + { conclusion: "close", correct: true }, + { conclusion: "close", correct: false }, + { conclusion: "hold" }, + { conclusion: "merge" }, + { conclusion: "merge" }, + ]); + }); + + it("runMetrics opens the event ledger by env path and moves the correct/incorrect counters", () => { + const env = tempConfigEnv(); + seedPredictionEnv(env, 1, "merge"); + seedPredictionEnv(env, 2, "close"); + seedOutcomeEnv(env, 1, "merged"); // merge predicted, merged realized -> correct + seedOutcomeEnv(env, 2, "merged"); // close predicted, merged realized -> incorrect + + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + expect(runMetrics([], { env })).toBe(0); + + const text = String(log.mock.calls[0]?.[0]); + expect(text).toContain("loopover_miner_prediction_correct_total 1"); + expect(text).toContain("loopover_miner_prediction_incorrect_total 1"); + }); + }); });