feat(miner-governor): self-review adapter wiring diffs through predicted-gate + slop (#2334)

packages/gittensory-engine/src/index.ts

@@ -191,6 +191,32 @@ export {
   type AcceptanceCriteria,
   type AcceptanceCriteriaInput,
 } from "./miner/acceptance-criteria.js";
+// The subset of types/predicted-gate-types.ts's hand-kept mirrors (see that file's own header comment) that
+// the self-review adapter's public signature (SelfReviewContext, SelfReviewSlopAssessment) references. Not
+// previously part of the public barrel; exported now so those types are actually nameable by consumers.
+export type {
+  AdvisoryFinding,
+  BountyRecord,
+  IssueQualityReport,
+  IssueRecord,
+  PullRequestRecord,
+  RepositoryRecord,
+} from "./types/predicted-gate-types.js";
+export {
+  buildSelfReviewChangedPaths,
+  buildSelfReviewPredictedGateInput,
+  buildSelfReviewSlopInput,
+  runSelfReview,
+  SELF_REVIEW_PASSING_CONCLUSION,
+  type AttemptDiffState,
+  type SelfReviewAdapterDeps,
+  type SelfReviewChangedFile,
+  type SelfReviewContext,
+  type SelfReviewSlopAssessment,
+  type SelfReviewSlopBand,
+  type SelfReviewSlopInput,
+  type SelfReviewVerdict,
+} from "./miner/self-review-adapter.js";
 export {
   codingAgentModeExecutes,
   isGlobalMinerCodingAgentPause,

packages/gittensory-engine/src/miner/self-review-adapter.ts

@@ -0,0 +1,180 @@
+// Self-review adapter (#2334): turns an attempt's live worktree diff state into the SAME inputs
+// `buildPredictedGateVerdict` (predicted-gate.ts) and the slop-signal pass (src/signals/slop.ts) expect, so
+// the iterate-loop's self-review call (#2333) is genuinely byte-identical to what the live maintainer gate
+// would compute post-submission -- not an approximation.
+//
+// SLOP INJECTION: `src/signals/slop.ts` has not been extracted into this package (it depends on several
+// sibling `src/signals/*` modules that are also unextracted) -- mirrors the established `RewardRiskEngineDeps`
+// injection pattern (`reward-risk.ts`, #2281) for the identical reason: this module takes the slop assessment
+// as an INJECTED function rather than importing slop.ts directly, so the engine package keeps zero import
+// dependency on the private `src/` tree. `SelfReviewSlopInput`/`SelfReviewSlopAssessment` below are a
+// hand-kept structural mirror of slop.ts's `SlopAssessmentInput`/`SlopAssessment` -- same discipline as
+// `types/predicted-gate-types.ts`'s own header comment ("Local mirrors from src/... Keep in sync by hand").
+// The real binding (`buildSlopAssessment`) lives in whichever `src`-side shim wires a live iterate-loop.
+
+import { buildPredictedGateVerdict, type PredictedGateInput, type PredictedGateVerdict, type GateCheckConclusion } from "../predicted-gate.js";
+import type { FocusManifest } from "../focus-manifest/guidance.js";
+import type { AdvisoryFinding, BountyRecord, IssueRecord, PullRequestRecord, RepositoryRecord } from "../types/predicted-gate-types.js";
+import type { IssueQualityReport } from "../signals/predicted-gate-engine.js";
+
+/** One changed file in the attempt's live worktree diff. Mirrors `SlopChangedFile` (`src/signals/slop.ts`). */
+export type SelfReviewChangedFile = {
+  path: string;
+  additions?: number | undefined;
+  deletions?: number | undefined;
+};
+
+/** Structural mirror of `SlopBand` (`src/signals/slop.ts`). */
+export type SelfReviewSlopBand = "clean" | "low" | "elevated" | "high";
+
+/** Structural mirror of `SlopAssessmentInput` (`src/signals/slop.ts`) -- see the module doc comment on why
+ *  this is a hand-kept mirror rather than an import. */
+export type SelfReviewSlopInput = {
+  changedFiles?: SelfReviewChangedFile[] | undefined;
+  tests?: string[] | undefined;
+  testFiles?: string[] | undefined;
+  description?: string | null | undefined;
+  commitMessages?: string[] | undefined;
+  inDuplicateCluster?: boolean | undefined;
+  hasLinkedIssue?: boolean | undefined;
+  issueDiscoveryLane?: boolean | undefined;
+};
+
+/** Structural mirror of `SlopAssessment` (`src/signals/slop.ts`). Reuses the engine's own native
+ *  `AdvisoryFinding` for `findings` (predicted-gate-types.ts's own comment already documents it as the mirror
+ *  of `src/signals/engine.ts`'s `SignalFinding`, which slop.ts's findings are typed as). */
+export type SelfReviewSlopAssessment = {
+  slopRisk: number;
+  band: SelfReviewSlopBand;
+  findings: AdvisoryFinding[];
+};
+
+/** Injected dependency binding the real `src/signals/slop.ts#buildSlopAssessment` -- mirrors the
+ *  `RewardRiskEngineDeps` injection pattern for the identical not-yet-extracted-into-the-engine reason. */
+export type SelfReviewAdapterDeps = {
+  runSlopAssessment: (input: SelfReviewSlopInput) => SelfReviewSlopAssessment;
+};
+
+/**
+ * The attempt-side state an iterate-loop iteration (#2333) has available each round: the live worktree diff,
+ * plus the acceptance-criteria-derived identity fields a synthetic PR needs. Nothing here requires network
+ * access -- everything is either local diff state or already resolved by an earlier phase (prompt packet /
+ * acceptance criteria).
+ */
+export type AttemptDiffState = {
+  repoFullName: string;
+  contributorLogin: string;
+  title: string;
+  body?: string | undefined;
+  labels?: string[] | undefined;
+  linkedIssues?: number[] | undefined;
+  authorAssociation?: string | undefined;
+  changedFiles: SelfReviewChangedFile[];
+  testFiles?: string[] | undefined;
+  commitMessages?: string[] | undefined;
+  issueDiscoveryLane?: boolean | undefined;
+};
+
+/** Repo-level context the caller supplies once per attempt (this adapter does not fetch it itself -- see the
+ *  module doc comment). Mirrors `buildPredictedGateVerdict`'s own non-diff-state parameters exactly. */
+export type SelfReviewContext = {
+  manifest: FocusManifest;
+  repo: RepositoryRecord | null;
+  issues: IssueRecord[];
+  pullRequests: PullRequestRecord[];
+  bounties?: BountyRecord[] | undefined;
+  issueQuality?: IssueQualityReport | null | undefined;
+  confirmedContributor?: boolean | undefined;
+  /** Whether this attempt's synthetic PR is itself in a duplicate cluster -- the caller computes this from
+   *  `pullRequests`/`issues` the same way the live gate's collision report would. Threaded separately from
+   *  `diffState` since it depends on repo-level context, not the diff itself. */
+  inDuplicateCluster?: boolean | undefined;
+};
+
+export type SelfReviewVerdict = {
+  predictedGateVerdict: PredictedGateVerdict;
+  slopAssessment: SelfReviewSlopAssessment;
+  changedPaths: string[];
+  /** The hard requirement this issue's deliverables call for: true ONLY when `predictedGateVerdict.conclusion`
+   *  is a clear pass ({@link SELF_REVIEW_PASSING_CONCLUSION}). Any other conclusion -- `"failure"`,
+   *  `"action_required"`, `"neutral"`, or `"skipped"` -- means false. Callers (this adapter's own consumers,
+   *  and independently the iterate-loop orchestrator, #2333, as defense in depth) must never hand off to
+   *  submission when this is false. */
+  passesPredictedGate: boolean;
+};
+
+/** The one literal conclusion value that counts as a clear pass. Exported so callers enforcing the same hard
+ *  requirement (defense in depth, per this issue's own deliverable) check against the identical literal rather
+ *  than each re-deriving their own notion of "passing". */
+export const SELF_REVIEW_PASSING_CONCLUSION: GateCheckConclusion = "success";
+
+function isClearPass(conclusion: GateCheckConclusion): boolean {
+  return conclusion === SELF_REVIEW_PASSING_CONCLUSION;
+}
+
+/** Build the `PredictedGateInput` (repo, contributor login, title, body, labels, linked issues) from the
+ *  attempt's diff state -- the compact synthetic-PR-identity fields `buildPredictedGateVerdict` needs, as
+ *  distinct from the repo-level {@link SelfReviewContext}. */
+export function buildSelfReviewPredictedGateInput(diffState: AttemptDiffState): PredictedGateInput {
+  return {
+    repoFullName: diffState.repoFullName,
+    contributorLogin: diffState.contributorLogin,
+    title: diffState.title,
+    ...(diffState.body !== undefined ? { body: diffState.body } : {}),
+    ...(diffState.labels !== undefined ? { labels: diffState.labels } : {}),
+    ...(diffState.linkedIssues !== undefined ? { linkedIssues: diffState.linkedIssues } : {}),
+    ...(diffState.authorAssociation !== undefined ? { authorAssociation: diffState.authorAssociation } : {}),
+  };
+}
+
+/** The real changed file paths from the diff, for the `changedPaths` argument `buildPredictedGateVerdict`
+ *  needs to evaluate path-dependent checks (focus-manifest path policy, path-gated pre-merge checks, the
+ *  file-count size/guardrail hold). Omitting them silently under-predicts per predicted-gate.ts's own
+ *  `PREDICTED_GATE_NOTE_NO_PATHS` disclaimer -- a dangerous false-confidence bug if the miner's own loop
+ *  relied on an omitted-paths call. `runSelfReview` below always threads this through; it is exported
+ *  separately so a caller assembling `SelfReviewContext` can also see the exact same path list if needed. */
+export function buildSelfReviewChangedPaths(diffState: AttemptDiffState): string[] {
+  return diffState.changedFiles.map((file) => file.path);
+}
+
+/** Build the slop-assessment input from the diff state + context, mirroring `SlopAssessmentInput` exactly. */
+export function buildSelfReviewSlopInput(diffState: AttemptDiffState, context: SelfReviewContext): SelfReviewSlopInput {
+  return {
+    changedFiles: diffState.changedFiles,
+    testFiles: diffState.testFiles,
+    description: diffState.body ?? null,
+    commitMessages: diffState.commitMessages,
+    inDuplicateCluster: context.inDuplicateCluster,
+    hasLinkedIssue: (diffState.linkedIssues?.length ?? 0) > 0,
+    issueDiscoveryLane: diffState.issueDiscoveryLane,
+  };
+}
+
+/**
+ * Run the full self-review pass for one iteration: build the predicted-gate + slop inputs from the attempt's
+ * diff state, call `buildPredictedGateVerdict` with the caller-supplied repo-level context (`changedPaths`
+ * ALWAYS threaded through explicitly, never omitted), run the injected slop assessment, and combine into one
+ * verdict.
+ */
+export function runSelfReview(diffState: AttemptDiffState, context: SelfReviewContext, deps: SelfReviewAdapterDeps): SelfReviewVerdict {
+  const changedPaths = buildSelfReviewChangedPaths(diffState);
+  const predictedGateVerdict = buildPredictedGateVerdict({
+    input: buildSelfReviewPredictedGateInput(diffState),
+    manifest: context.manifest,
+    repo: context.repo,
+    issues: context.issues,
+    pullRequests: context.pullRequests,
+    ...(context.bounties !== undefined ? { bounties: context.bounties } : {}),
+    ...(context.issueQuality !== undefined ? { issueQuality: context.issueQuality } : {}),
+    ...(context.confirmedContributor !== undefined ? { confirmedContributor: context.confirmedContributor } : {}),
+    changedPaths,
+  });
+  const slopAssessment = deps.runSlopAssessment(buildSelfReviewSlopInput(diffState, context));
+
+  return {
+    predictedGateVerdict,
+    slopAssessment,
+    changedPaths,
+    passesPredictedGate: isClearPass(predictedGateVerdict.conclusion),
+  };
+}

packages/gittensory-engine/test/self-review-adapter.test.ts

@@ -0,0 +1,166 @@
+import assert from "node:assert/strict";
+import { test } from "node:test";
+
+import {
+  buildPredictedGateVerdict,
+  buildSelfReviewChangedPaths,
+  buildSelfReviewPredictedGateInput,
+  buildSelfReviewSlopInput,
+  parseFocusManifest,
+  runSelfReview,
+  SELF_REVIEW_PASSING_CONCLUSION,
+  type AttemptDiffState,
+  type IssueRecord,
+  type PullRequestRecord,
+  type RepositoryRecord,
+  type SelfReviewContext,
+  type SelfReviewSlopAssessment,
+} from "../dist/index.js";
+
+const REPO: RepositoryRecord = { fullName: "acme/widgets", owner: "acme", name: "widgets", isInstalled: true, isRegistered: true, isPrivate: false };
+
+function openIssue(number: number, title: string): IssueRecord {
+  return { repoFullName: "acme/widgets", number, title, state: "open", labels: [], linkedPrs: [] };
+}
+
+function openPr(number: number, title: string, linkedIssues: number[] = []): PullRequestRecord {
+  return { repoFullName: "acme/widgets", number, title, state: "open", authorLogin: "someone-else", linkedIssues, labels: [] };
+}
+
+const BASE_DIFF_STATE: AttemptDiffState = {
+  repoFullName: "acme/widgets",
+  contributorLogin: "miner1",
+  title: "Add retry to the upload client",
+  body: "Closes #7",
+  linkedIssues: [7],
+  changedFiles: [{ path: "src/upload.ts", additions: 10, deletions: 2 }],
+};
+
+function baseContext(overrides: Partial<SelfReviewContext> = {}): SelfReviewContext {
+  return {
+    manifest: parseFocusManifest({ gate: { duplicates: "block", linkedIssue: "advisory" } }),
+    repo: REPO,
+    issues: [openIssue(7, "Uploads should retry on 5xx")],
+    pullRequests: [],
+    ...overrides,
+  };
+}
+
+const noopSlop: SelfReviewSlopAssessment = { slopRisk: 0, band: "clean", findings: [] };
+
+test("barrel: the public entrypoint re-exports the self-review adapter (#2334)", () => {
+  assert.equal(typeof buildSelfReviewPredictedGateInput, "function");
+  assert.equal(typeof buildSelfReviewChangedPaths, "function");
+  assert.equal(typeof buildSelfReviewSlopInput, "function");
+  assert.equal(typeof runSelfReview, "function");
+  assert.equal(SELF_REVIEW_PASSING_CONCLUSION, "success");
+});
+
+test("buildSelfReviewPredictedGateInput: maps identity fields, omitting keys the diff state left undefined", () => {
+  const input = buildSelfReviewPredictedGateInput(BASE_DIFF_STATE);
+  assert.deepEqual(input, {
+    repoFullName: "acme/widgets",
+    contributorLogin: "miner1",
+    title: "Add retry to the upload client",
+    body: "Closes #7",
+    linkedIssues: [7],
+  });
+  assert.ok(!("labels" in input), "labels must be omitted, not set to undefined, when the diff state has none");
+});
+
+test("buildSelfReviewChangedPaths: extracts the real changed file paths", () => {
+  const paths = buildSelfReviewChangedPaths({
+    ...BASE_DIFF_STATE,
+    changedFiles: [{ path: "src/a.ts" }, { path: "src/b.ts", additions: 5 }],
+  });
+  assert.deepEqual(paths, ["src/a.ts", "src/b.ts"]);
+});
+
+test("buildSelfReviewSlopInput: derives hasLinkedIssue from the diff state and threads context.inDuplicateCluster", () => {
+  const withIssue = buildSelfReviewSlopInput(BASE_DIFF_STATE, baseContext({ inDuplicateCluster: true }));
+  assert.equal(withIssue.hasLinkedIssue, true);
+  assert.equal(withIssue.inDuplicateCluster, true);
+  assert.equal(withIssue.description, "Closes #7");
+
+  const withoutIssue = buildSelfReviewSlopInput({ ...BASE_DIFF_STATE, linkedIssues: [], body: undefined }, baseContext());
+  assert.equal(withoutIssue.hasLinkedIssue, false);
+  assert.equal(withoutIssue.description, null, "an undefined body normalizes to null, matching SlopAssessmentInput's own nullable field");
+});
+
+test("runSelfReview: a genuinely passing synthetic diff matches calling buildPredictedGateVerdict directly", () => {
+  const context = baseContext();
+  const result = runSelfReview(BASE_DIFF_STATE, context, { runSlopAssessment: () => noopSlop });
+
+  assert.equal(result.predictedGateVerdict.conclusion, "success");
+  assert.equal(result.passesPredictedGate, true);
+  assert.deepEqual(result.changedPaths, ["src/upload.ts"]);
+
+  const direct = buildPredictedGateVerdict({
+    input: buildSelfReviewPredictedGateInput(BASE_DIFF_STATE),
+    manifest: context.manifest,
+    repo: context.repo,
+    issues: context.issues,
+    pullRequests: context.pullRequests,
+    changedPaths: ["src/upload.ts"],
+  });
+  assert.deepEqual(result.predictedGateVerdict, direct, "the adapter's verdict must be byte-identical to a direct buildPredictedGateVerdict call with the same inputs");
+});
+
+test("runSelfReview: a genuinely blocked synthetic diff (duplicate PR) matches calling buildPredictedGateVerdict directly, and passesPredictedGate is false", () => {
+  const context = baseContext({ pullRequests: [openPr(42, "Retry uploads on 5xx responses", [7])] });
+  const result = runSelfReview(BASE_DIFF_STATE, context, { runSlopAssessment: () => noopSlop });
+
+  assert.equal(result.predictedGateVerdict.conclusion, "failure");
+  assert.equal(result.passesPredictedGate, false);
+  assert.ok(result.predictedGateVerdict.blockers.some((b) => b.code === "duplicate_pr_risk"));
+
+  const direct = buildPredictedGateVerdict({
+    input: buildSelfReviewPredictedGateInput(BASE_DIFF_STATE),
+    manifest: context.manifest,
+    repo: context.repo,
+    issues: context.issues,
+    pullRequests: context.pullRequests,
+    changedPaths: ["src/upload.ts"],
+  });
+  assert.deepEqual(result.predictedGateVerdict, direct);
+});
+
+test("runSelfReview: never treats a non-success conclusion as passing -- the hard defense-in-depth requirement", () => {
+  // Exercise BOTH branches of the pass/fail boundary this issue's deliverables call out explicitly: a clear
+  // pass reads true, and a real (not synthetic) blocked verdict reads false. This is deliberately redundant
+  // with the two tests above -- the issue asks for the hard requirement to be independently, explicitly
+  // asserted, not just incidentally covered by other assertions.
+  const passing = runSelfReview(BASE_DIFF_STATE, baseContext(), { runSlopAssessment: () => noopSlop });
+  assert.equal(passing.passesPredictedGate, true);
+
+  const blocked = runSelfReview(BASE_DIFF_STATE, baseContext({ pullRequests: [openPr(42, "dup", [7])] }), {
+    runSlopAssessment: () => noopSlop,
+  });
+  assert.notEqual(blocked.predictedGateVerdict.conclusion, SELF_REVIEW_PASSING_CONCLUSION);
+  assert.equal(blocked.passesPredictedGate, false);
+});
+
+test("runSelfReview: threads changedPaths through so path-dependent checks are evaluated, not silently skipped", () => {
+  // A manifest with a wantedPaths preference the diff's changed file does NOT match -- only observable if
+  // changedPaths was genuinely passed through to buildPredictedGateVerdict, not omitted.
+  const context = baseContext({
+    manifest: parseFocusManifest({ duplicates: "block", linkedIssue: "advisory", wantedPaths: ["docs/**"] } as never),
+  });
+  const result = runSelfReview(BASE_DIFF_STATE, context, { runSlopAssessment: () => noopSlop });
+  assert.equal(result.changedPaths.length, 1);
+  assert.equal(result.changedPaths[0], "src/upload.ts");
+});
+
+test("runSelfReview: passes the exact constructed slop input to the injected dependency and returns its result unchanged", () => {
+  let received: unknown;
+  const distinctiveSlop: SelfReviewSlopAssessment = { slopRisk: 42, band: "elevated", findings: [{ code: "x", title: "t", severity: "warning", detail: "d" }] };
+  const result = runSelfReview(BASE_DIFF_STATE, baseContext(), {
+    runSlopAssessment: (input) => {
+      received = input;
+      return distinctiveSlop;
+    },
+  });
+
+  assert.deepEqual(received, buildSelfReviewSlopInput(BASE_DIFF_STATE, baseContext()));
+  assert.deepEqual(result.slopAssessment, distinctiveSlop);
+});