add diff coverage preflight for PR review submissions (#544)

* add one-time diff coverage preflight for PR reviews track diff read coverage from agent tool-use events and run a one-time pre-flight before review submission, with explicit coverage skip reasons for low-value files like lockfiles. Made-with: Cursor * add manual dispatch fallback for preview deploy workflow allow preview repo and preview sync jobs to be run via workflow_dispatch with explicit PR number and branch inputs, so preview provisioning can be retriggered when pull_request events fail to fire. Made-with: Cursor * fix manual preview dispatch PR input wiring use normalized PR number and branch env values for comment creation and script env wiring so workflow_dispatch preview runs can create and update PR-specific preview resources. Made-with: Cursor * remove obsolete snapshots invalidated by checkout instructions change * fix diff coverage read offset handling and add local sanity-check guidance normalize read offset semantics for diff coverage tracking, reuse shared range counting in review preflight, add focused diff coverage unit tests, and document the local play.ts testing workflow in AGENTS.md. Made-with: Cursor * add regenerated mcp test snapshots capture snapshot files generated by the review comment and checkout formatting tests during pre-push validation so the branch remains clean and reproducible. Made-with: Cursor * add diff coverage preflight instrumentation logs log diff coverage initialization in checkout_pr and emit preflight state/breakdown diagnostics in create_pull_request_review to debug missing coverage enforcement in preview e2e runs. Made-with: Cursor * add env override to force local cli execution in action runtime support explicit local-cli execution via PULLFROG_FORCE_LOCAL_CLI so preview workflows can run branch action code instead of the npm fallback package during e2e debugging. Made-with: Cursor * add preview e2e debugging learnings for action runtime validation capture the preview execution-path gotchas and one-time preflight verification pattern in AGENTS.md so future investigations validate the real runtime and avoid npm fallback confusion. Made-with: Cursor * reduce diff coverage log noise while preserving failure visibility downgrade verbose diff coverage lifecycle diagnostics to debug, keep a concise info-level pre-flight failure signal, and document preview runtime debugging learnings in AGENTS.md. Made-with: Cursor * WIP * tune sync.md: ff override + softer overlap verification Made-with: Cursor * chore: bump models snapshot for claude-opus-4-7 Made-with: Cursor * rip out coverage_skips waiver from diff coverage pre-flight Made-with: Cursor --------- Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com>
2026-04-16 21:51:44 +00:00
parent 5e6ff67623
commit 56a5d29598
11 changed files with 653 additions and 5 deletions
@@ -3,6 +3,7 @@ import { join } from "node:path";
 import type { Octokit, RestEndpointMethodTypes } from "@octokit/rest";
 import { type } from "arktype";
 import { log } from "../utils/cli.ts";
+import { countLines, createDiffCoverageState } from "../utils/diffCoverage.ts";
 import { $git } from "../utils/gitAuth.ts";
 import { executeLifecycleHook } from "../utils/lifecycle.ts";
 import { computeIncrementalDiff } from "../utils/rangeDiff.ts";
@@ -504,6 +505,14 @@ export function CheckoutPrTool(ctx: ToolContext) {
      const diffPath = join(tempDir, `pr-${pull_number}-${headShort}.diff`);
      writeFileSync(diffPath, formatResult.content);
      log.debug(`wrote diff to ${diffPath} (${formatResult.content.length} bytes)`);
+      ctx.toolState.diffCoverage = createDiffCoverageState({
+        diffPath,
+        totalLines: countLines({ content: formatResult.content }),
+        toc: formatResult.toc,
+      });
+      log.debug(
+        `» diff coverage initialized: diffPath=${diffPath}, totalLines=${ctx.toolState.diffCoverage.totalLines}, tocEntries=${ctx.toolState.diffCoverage.tocEntries.length}`
+      );

      const incrementalInstructions = incrementalDiffPath
        ? ` IMPORTANT: incrementalDiffPath contains ONLY the changes since the last reviewed version ` +
@@ -527,9 +536,10 @@ export function CheckoutPrTool(ctx: ToolContext) {
        toc: formatResult.toc,
        instructions:
          `the diff file at diffPath contains a table of contents (TOC) at the top listing every changed file with its line range. ` +
-          `use the line ranges to read specific files from the diff instead of reading the entire file. ` +
+          `use the TOC line ranges as your checklist and read specific files from the diff instead of reading the entire file. ` +
          `for example, if the TOC says "src/foo.ts → lines 5-42", read lines 5-42 from diffPath to see that file's changes. ` +
-          `review files selectively based on relevance rather than reading everything sequentially. ` +
+          `before your review is submitted, a one-time coverage pre-flight may error listing unread TOC regions. ` +
+          `retry the same create_pull_request_review call to proceed — optionally after reading the listed ranges. the pre-flight will not block again this session. ` +
          `the local branch is 'localBranch' (pr-{number}), not the remote branch name. ` +
          `when pushing, omit branchName to use the current branch. do not use remoteBranch as a local branch name.` +
          incrementalInstructions,
@@ -4,6 +4,11 @@ import { formatMcpToolRef } from "../external.ts";
 import { getApiUrl } from "../utils/apiUrl.ts";
 import { buildPullfrogFooter } from "../utils/buildPullfrogFooter.ts";
 import { log } from "../utils/cli.ts";
+import {
+  countLinesInRanges,
+  getDiffCoverageBreakdown,
+  renderDiffCoverageBreakdown,
+} from "../utils/diffCoverage.ts";
 import { fixDoubleEscapedString } from "../utils/fixDoubleEscapedString.ts";
 import { patchWorkflowRunFields } from "../utils/patchWorkflowRunFields.ts";
 import type { ToolContext } from "./server.ts";
@@ -75,6 +80,7 @@ export function CreatePullRequestReviewTool(ctx: ToolContext) {
      "IMPORTANT: 95%+ of feedback should be in 'comments' array with file paths and line numbers. " +
      "Only use 'body' for a 1-2 sentence summary with urgency and critical callouts. " +
      "Use 'suggestion' to propose replacement code - MUST preserve exact indentation of original code. " +
+      "The first submission may error once with a one-time diff-coverage nudge listing unread TOC regions — retry with the same arguments and the pre-flight will not block again. " +
      "Example replacing lines 42-44 (3 lines) with 5 lines: " +
      `{ path: 'src/api.ts', start_line: 42, line: 44, suggestion: '    const result = await fetch(url);\\n    if (!result.ok) {\\n      log.error(result.status);\\n      throw new Error("request failed");\\n    }' }` +
      " CONSTRAINT: Inline comments can ONLY target files and lines that appear in the PR diff." +
@@ -131,6 +137,9 @@ export function CreatePullRequestReviewTool(ctx: ToolContext) {
          );
        }
      }
+
+      runDiffCoveragePreflight({ ctx });
+
      type ReviewComment = NonNullable<typeof params.comments>[number];
      const reviewComments = comments.map((comment) => {
        let commentBody = fixDoubleEscapedString(comment.body || "");
@@ -248,6 +257,61 @@ export function CreatePullRequestReviewTool(ctx: ToolContext) {
  });
 }

+function runDiffCoveragePreflight(params: { ctx: ToolContext }): void {
+  const coverageState = params.ctx.toolState.diffCoverage;
+  if (!coverageState) {
+    log.debug("diff coverage pre-flight skipped: no diffCoverage state present in toolState");
+    return;
+  }
+  if (coverageState.coveragePreflightRan) {
+    log.debug("diff coverage pre-flight skipped: already ran in this session");
+    return;
+  }
+
+  coverageState.coveragePreflightRan = true;
+  log.debug(
+    `diff coverage pre-flight start: diffPath=${coverageState.diffPath}, totalLines=${coverageState.totalLines}, tocEntries=${coverageState.tocEntries.length}, coveredRanges=${coverageState.coveredRanges.length}`
+  );
+  const breakdown = getDiffCoverageBreakdown({ state: coverageState });
+  const unread: Array<{ path: string; ranges: string; unreadLines: number }> = [];
+  let unreadLines = 0;
+  for (const file of breakdown.files) {
+    if (file.unreadRanges.length === 0) continue;
+    const rangesText = file.unreadRanges
+      .map((range) => `${range.startLine}-${range.endLine}`)
+      .join(", ");
+    const fileUnreadLines = countLinesInRanges({ ranges: file.unreadRanges });
+    unread.push({ path: file.filename, ranges: rangesText, unreadLines: fileUnreadLines });
+    unreadLines += fileUnreadLines;
+  }
+  coverageState.lastBreakdown = renderDiffCoverageBreakdown({
+    diffPath: coverageState.diffPath,
+    breakdown,
+  });
+  log.debug(
+    `diff coverage pre-flight breakdown: coveredLines=${breakdown.coveredLines}, unreadLines=${unreadLines}`
+  );
+
+  if (unreadLines === 0) {
+    log.debug("diff coverage pre-flight passed: no unread regions");
+    return;
+  }
+
+  log.info(
+    `diff coverage pre-flight nudge: unread lines=${unreadLines}, unread files=${unread.length}`
+  );
+  const unreadText = unread
+    .map((entry) => `- ${entry.path} (${entry.unreadLines} lines, ${entry.ranges})`)
+    .join("\n");
+  throw new Error(
+    `diff coverage pre-flight: some TOC regions were not read before review submission. ` +
+      `this is a one-time nudge — optionally read the ranges below from ${coverageState.diffPath}, then call create_pull_request_review again with the same arguments. ` +
+      `this pre-flight will not block again in this review session.\n\n` +
+      `unread TOC regions:\n${unreadText}\n\n` +
+      `${coverageState.lastBreakdown}`
+  );
+}
+
 type FooterOpts = { body: string; approved: boolean; hasComments: boolean };

 async function createAndSubmitWithFooter(
@@ -9,6 +9,7 @@ import type { Mode } from "../modes.ts";
 import type { PrepResult } from "../prep/index.ts";
 import { closeBrowserDaemon } from "../utils/browser.ts";
 import { log } from "../utils/cli.ts";
+import type { DiffCoverageState } from "../utils/diffCoverage.ts";
 import type { OctokitWithPlugins } from "../utils/github.ts";
 import type { ResolvedPayload } from "../utils/payload.ts";
 import type { RunContextData } from "../utils/runContextData.ts";
@@ -107,6 +108,7 @@ export interface ToolState {
  usageEntries: AgentUsage[];
  model?: string | undefined;
  todoTracker?: TodoTracker | undefined;
+  diffCoverage?: DiffCoverageState | undefined;
 }

 interface InitToolStateParams {