diff --git a/prisma/migrations/20261003000000_add_issue_groomed_search_queries/migration.sql b/prisma/migrations/20261003000000_add_issue_groomed_search_queries/migration.sql new file mode 100644 index 00000000..e4b80f5f --- /dev/null +++ b/prisma/migrations/20261003000000_add_issue_groomed_search_queries/migration.sql @@ -0,0 +1,6 @@ +-- Preserve empty code-search queries so freshness checks can verify whether +-- repo-wide negative evidence still holds after a default-branch commit. +ALTER TABLE "Issue" + ADD COLUMN IF NOT EXISTS "groomedSearchCodeQueries" TEXT[] NOT NULL DEFAULT ARRAY[]::TEXT[]; + +COMMENT ON COLUMN "Issue"."groomedSearchCodeQueries" IS 'Successful empty search_code queries backing global grooming evidence'; diff --git a/prisma/schema.prisma b/prisma/schema.prisma index f338fc12..7c5e8a06 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -72,6 +72,7 @@ model Issue { groomedEvidenceCapturedAt DateTime? groomedEvidenceScope String? groomedEvidencePaths String[] @default([]) + groomedSearchCodeQueries String[] @default([]) groomedDependencyKeys String[] @default([]) groomedOpenBlockerKeys String[] @default([]) groomedRelatedWork Json? diff --git a/src/lib/github-ci.ts b/src/lib/github-ci.ts index 9e5ddd01..b4554f67 100644 --- a/src/lib/github-ci.ts +++ b/src/lib/github-ci.ts @@ -160,6 +160,27 @@ export async function fetchLatestCommit(repoFullName: string, branch: string): P return response.json(); } +/** + * Committer timestamp (ISO string) for an existing commit sha, or null when + * the ref does not resolve. Used by the grooming freshness pass to judge + * whether the code-search index has had time to catch up with a new head. + * GitHub always returns a committer date for a commit; the author date is + * deliberately not used as a fallback (it can make a rebased head look + * older than it is). + */ +export async function fetchCommitDate(repoFullName: string, ref: string): Promise { + const response = await fetchWithRetry(`${GITHUB_API}/repos/${repoFullName}/commits/${encodeURIComponent(ref)}`, { + headers: await getHeadersAsync(), + }); + if (!response.ok) { + if (response.status === 404) return null; + const text = await response.text(); + throw new Error(`Failed to fetch commit ${ref} for ${repoFullName}: ${response.status} ${text}`); + } + const data = (await response.json()) as { commit?: { committer?: { date?: string } } }; + return data.commit?.committer?.date ?? null; +} + export function jobIdFromCheckRunUrl(url: string | undefined | null): string | null { const m = /\/job\/(\d+)/.exec(url ?? ""); return m ? m[1] : null; diff --git a/src/lib/github-code-search.test.ts b/src/lib/github-code-search.test.ts index 9cb8b740..30e3681a 100644 --- a/src/lib/github-code-search.test.ts +++ b/src/lib/github-code-search.test.ts @@ -137,6 +137,14 @@ describe("github-code-search: searchRepositoryCode (the groomer repo-exploration expect(fetchSpy).toHaveBeenCalledTimes(1); }); + it("fails an incomplete search with no items instead of reporting no matches", async () => { + fetchSpy.mockResolvedValueOnce(mockResponse({ total_count: 0, incomplete_results: true, items: [] })); + + await expect(searchRepositoryCode("org/repo", "prisma", 5)).rejects.toThrow( + /^Code search failed for org\/repo: search timed out with incomplete results and no matches$/, + ); + }); + // (c) HTTP 429 retried with backoff. it("retries a 429 rate-limit response with backoff and succeeds on the next attempt", async () => { fetchSpy @@ -306,10 +314,17 @@ describe("github-code-search: compareCommits (grooming freshness, #1064)", () => mockResponse({ status: "ahead", files: [{ filename: "src/new.ts", previous_filename: "src/old.ts" }, { filename: "docs/a.md" }], + commits: [{ commit: { committer: { date: "2026-09-28T00:00:00Z" } } }], }), ); const result = await compareCommits("org/repo", "base1", "head1"); - expect(result).toEqual({ ok: true, status: "ahead", files: ["src/new.ts", "src/old.ts", "docs/a.md"], truncated: false }); + expect(result).toEqual({ + ok: true, + status: "ahead", + files: ["src/new.ts", "src/old.ts", "docs/a.md"], + truncated: false, + firstCommitDate: "2026-09-28T00:00:00Z", + }); expect(fetchSpy).toHaveBeenCalledTimes(1); expect(String(fetchSpy.mock.calls[0][0])).toContain("/repos/org/repo/compare/base1...head1?per_page=1"); }); diff --git a/src/lib/github-code-search.ts b/src/lib/github-code-search.ts index 5c3b0729..e1a0fe8c 100644 --- a/src/lib/github-code-search.ts +++ b/src/lib/github-code-search.ts @@ -57,11 +57,19 @@ export async function searchRepositoryCode( // so a search with more matches than fit on one page is not silently // truncated at the first page. Each page is fetched through fetchWithRetry, // so transient 429/5xx responses are retried here as well. - const items = await fetchPaginated<{ path?: string; html_url?: string }>( - url, - limit, - (data) => (data as { items?: { path?: string; html_url?: string }[] }).items ?? [], - ); + let incomplete = false; + const items = await fetchPaginated<{ path?: string; html_url?: string }>(url, limit, (data) => { + const page = data as { items?: { path?: string; html_url?: string }[]; incomplete_results?: boolean }; + if (page.incomplete_results === true) incomplete = true; + return page.items ?? []; + }); + // Code search can time out and still answer 200 with incomplete_results + // and no items. That is not "no matches": the groomer treats an empty + // result as proof of absence and the freshness recheck advances on it + // (#1091), so surface it as a failure instead. + if (incomplete && items.length === 0) { + throw new Error("search timed out with incomplete results and no matches"); + } return items.map((item) => ({ path: item.path ?? "", url: item.html_url ?? "", @@ -161,6 +169,12 @@ export type CommitComparison = files: string[]; /** True when GitHub's file cap was reached, so `files` may be incomplete. */ truncated: boolean; + /** + * Committer timestamp of the first (oldest) commit in base...head, when + * the response carried one. Lets callers bound how long a recheck may + * stay deferred on an unverified range (#1091). + */ + firstCommitDate?: string | null; } | { ok: false; @@ -196,6 +210,7 @@ export async function compareCommits( const data = (await response.json()) as { status?: string; files?: Array<{ filename?: string; previous_filename?: string }>; + commits?: Array<{ commit?: { committer?: { date?: string } } }>; }; const rawFiles = Array.isArray(data.files) ? data.files : []; const files = new Set(); @@ -203,11 +218,14 @@ export async function compareCommits( if (typeof file.filename === "string" && file.filename) files.add(file.filename); if (typeof file.previous_filename === "string" && file.previous_filename) files.add(file.previous_filename); } + // per_page=1 still returns the first (oldest) commit of the range. + const firstCommitDate = Array.isArray(data.commits) ? data.commits[0]?.commit?.committer?.date ?? null : null; return { ok: true, status: typeof data.status === "string" ? data.status : "unknown", files: [...files], truncated: rawFiles.length >= COMPARE_MAX_FILES, + firstCommitDate, }; } catch (err) { return { diff --git a/src/lib/github-facades.test.ts b/src/lib/github-facades.test.ts index d54eb211..9741d5fa 100644 --- a/src/lib/github-facades.test.ts +++ b/src/lib/github-facades.test.ts @@ -34,6 +34,7 @@ describe("github domain modules expose expected exports", () => { it("github-ci exports CI/workflows/runs/jobs/releases/packages/commits/logs symbols", () => { expect(Object.keys(Ci).sort()).toEqual([ "extractLogExcerpt", + "fetchCommitDate", "fetchFailedJobLogExcerpt", "fetchLatestCommit", "fetchPackages", diff --git a/src/lib/groomer/freshness-invalidation.test.ts b/src/lib/groomer/freshness-invalidation.test.ts index a5d35e0c..9f5076b0 100644 --- a/src/lib/groomer/freshness-invalidation.test.ts +++ b/src/lib/groomer/freshness-invalidation.test.ts @@ -34,6 +34,7 @@ function row(overrides: Partial = {}): FreshnessIssueRow { groomedEvidenceCapturedAt: CAPTURED, groomedEvidenceScope: "paths", groomedEvidencePaths: ["src/a.ts"], + groomedSearchCodeQueries: [], groomedDependencyKeys: [], groomedOpenBlockerKeys: [], groomedRelatedWork: null, @@ -86,9 +87,12 @@ function fakeStore(rows: FreshnessIssueRow[]): FakeStore { function fakeGitHub() { return { fetchHeadSha: vi.fn(async (): Promise => "sha-1"), + searchCode: vi.fn(async (): Promise<{ path: string }[]> => []), compareCommits: vi.fn( async (): Promise => ({ ok: true, status: "ahead", files: [], truncated: false }), ), + // Old enough that the code-search index is assumed caught up (#1091). + fetchCommitDate: vi.fn(async (): Promise => new Date(Date.now() - 3_600_000).toISOString()), fetchRecentComments: vi.fn(async (): Promise> => []), fetchIssueState: vi.fn(async (): Promise<"open" | "closed" | null> => "open"), fetchPullRequestState: vi.fn(async (): Promise<"open" | "closed" | "merged" | null> => "open"), @@ -263,12 +267,190 @@ describe("runGroomingFreshnessPass", () => { expect(store.stale.get("issue-1")?.detail).toContain("src/a.ts"); }); - it("conservatively stales global (negative) evidence on any commit", async () => { + it("conservatively stales global evidence without saved search queries", async () => { github.fetchHeadSha.mockResolvedValue("sha-2"); github.compareCommits.mockResolvedValue({ ok: true, status: "ahead", files: ["docs/readme.md"], truncated: false }); const store = fakeStore([{ ...row(), groomedEvidenceScope: "global", groomedEvidencePaths: [] }]); await pass(store, github); expect(store.stale.get("issue-1")?.reasons).toEqual(["global_evidence_commit"]); + expect(github.searchCode).not.toHaveBeenCalled(); + }); + + it("advances global evidence when saved empty queries remain empty", async () => { + github.fetchHeadSha.mockResolvedValue("sha-2"); + github.compareCommits.mockResolvedValue({ ok: true, status: "ahead", files: ["docs/readme.md"], truncated: false }); + const store = fakeStore([{ ...row(), groomedEvidenceScope: "global", groomedEvidencePaths: [], groomedSearchCodeQueries: ["new symbol", "missing call"] }]); + await pass(store, github); + expect(store.stale.size).toBe(0); + expect(store.advanced).toEqual([{ id: "issue-1", data: { groomingVerifiedSha: "sha-2" } }]); + expect(github.searchCode).toHaveBeenCalledTimes(2); + }); + + it("stales global evidence when a saved query now matches", async () => { + github.fetchHeadSha.mockResolvedValue("sha-2"); + github.compareCommits.mockResolvedValue({ ok: true, status: "ahead", files: [], truncated: false }); + github.searchCode.mockResolvedValueOnce([]).mockResolvedValueOnce([{ path: "src/new.ts" }]); + const store = fakeStore([{ ...row(), groomedEvidenceScope: "global", groomedEvidencePaths: [], groomedSearchCodeQueries: ["first", "specific missing query"] }]); + await pass(store, github); + expect(store.stale.get("issue-1")?.reasons).toEqual(["global_evidence_commit"]); + expect(store.stale.get("issue-1")?.detail).toContain("specific missing query"); + }); + + it("defers instead of staling when the search budget runs out mid-recheck", async () => { + github.fetchHeadSha.mockResolvedValue("sha-2"); + github.compareCommits.mockResolvedValue({ + ok: true, + status: "ahead", + files: [], + truncated: false, + firstCommitDate: new Date(Date.now() - 5 * 60_000).toISOString(), + }); + const store = fakeStore([{ ...row(), groomedEvidenceScope: "global", groomedEvidencePaths: [], groomedSearchCodeQueries: ["first", "second"] }]); + await pass(store, github, { ...DEFAULT_FRESHNESS_BUDGET, maxSearchCodeRechecks: 2 }); + // One budget unit goes to the commit-date check, one to the first query; + // the second query exhausts the budget, which defers rather than stales. + expect(store.stale.size).toBe(0); + expect(store.advanced).toEqual([]); + expect(github.searchCode).toHaveBeenCalledTimes(1); + }); + + it("stales a persistently exhausted issue once the oldest unverified commit passes the defer limit", async () => { + github.fetchHeadSha.mockResolvedValue("sha-2"); + github.compareCommits.mockResolvedValue({ + ok: true, + status: "ahead", + files: [], + truncated: false, + firstCommitDate: new Date(Date.now() - 3 * 3_600_000).toISOString(), + }); + const store = fakeStore([{ ...row(), groomedEvidenceScope: "global", groomedEvidencePaths: [], groomedSearchCodeQueries: ["first", "second"] }]); + await pass(store, github, { ...DEFAULT_FRESHNESS_BUDGET, maxSearchCodeRechecks: 2 }); + expect(store.stale.get("issue-1")?.reasons).toEqual(["global_evidence_commit"]); + expect(store.advanced).toEqual([]); + }); + + it("defers within the bound when the budget is exhausted before the date fetch", async () => { + github.fetchHeadSha.mockResolvedValue("sha-2"); + github.compareCommits.mockResolvedValue({ + ok: true, + status: "ahead", + files: [], + truncated: false, + firstCommitDate: new Date(Date.now() - 5 * 60_000).toISOString(), + }); + const store = fakeStore([{ ...row(), groomedEvidenceScope: "global", groomedEvidencePaths: [], groomedSearchCodeQueries: ["query"] }]); + const result = await pass(store, github, { ...DEFAULT_FRESHNESS_BUDGET, maxSearchCodeRechecks: 0 }); + expect(store.stale.size).toBe(0); + expect(store.advanced).toEqual([]); + expect(github.searchCode).not.toHaveBeenCalled(); + expect(result.deferred).toBe(1); + }); + + it("completes the recheck for an issue with the maximum saved queries", async () => { + github.fetchHeadSha.mockResolvedValue("sha-2"); + github.compareCommits.mockResolvedValue({ + ok: true, + status: "ahead", + files: [], + truncated: false, + firstCommitDate: new Date(Date.now() - 5 * 60_000).toISOString(), + }); + const queries = Array.from({ length: 10 }, (_, index) => `missing ${index}`); + const store = fakeStore([{ ...row(), groomedEvidenceScope: "global", groomedEvidencePaths: [], groomedSearchCodeQueries: queries }]); + await pass(store, github); + // Commit-date fetch (1) plus all ten queries fit the default budget. + expect(store.stale.size).toBe(0); + expect(store.advanced).toEqual([{ id: "issue-1", data: { groomingVerifiedSha: "sha-2" } }]); + expect(github.searchCode).toHaveBeenCalledTimes(10); + expect(github.fetchCommitDate).toHaveBeenCalledTimes(1); + }); + + it("defers the recheck while the new head is younger than the index grace window", async () => { + github.fetchHeadSha.mockResolvedValue("sha-2"); + github.compareCommits.mockResolvedValue({ + ok: true, + status: "ahead", + files: [], + truncated: false, + firstCommitDate: new Date(Date.now() - 5 * 60_000).toISOString(), + }); + github.fetchCommitDate.mockResolvedValue(new Date(Date.now() - 60_000).toISOString()); + const store = fakeStore([{ ...row(), groomedEvidenceScope: "global", groomedEvidencePaths: [], groomedSearchCodeQueries: ["query"] }]); + const result = await pass(store, github); + expect(store.stale.size).toBe(0); + expect(store.advanced).toEqual([]); + expect(github.searchCode).not.toHaveBeenCalled(); + expect(result.deferred).toBe(1); + }); + + it("bounds the deferral: stales once the oldest unverified commit outlives the defer limit", async () => { + github.fetchHeadSha.mockResolvedValue("sha-2"); + github.compareCommits.mockResolvedValue({ + ok: true, + status: "ahead", + files: [], + truncated: false, + firstCommitDate: new Date(Date.now() - 3 * 3_600_000).toISOString(), + }); + github.fetchCommitDate.mockResolvedValue(new Date(Date.now() - 60_000).toISOString()); + const store = fakeStore([{ ...row(), groomedEvidenceScope: "global", groomedEvidencePaths: [], groomedSearchCodeQueries: ["query"] }]); + const result = await pass(store, github); + expect(store.stale.get("issue-1")?.reasons).toEqual(["global_evidence_commit"]); + expect(store.advanced).toEqual([]); + expect(result.deferred).toBe(0); + }); + + it("stales instead of deferring when the oldest unverified commit age is unknown", async () => { + github.fetchHeadSha.mockResolvedValue("sha-2"); + github.compareCommits.mockResolvedValue({ ok: true, status: "ahead", files: [], truncated: false }); + github.fetchCommitDate.mockResolvedValue(new Date(Date.now() - 60_000).toISOString()); + const store = fakeStore([{ ...row(), groomedEvidenceScope: "global", groomedEvidencePaths: [], groomedSearchCodeQueries: ["query"] }]); + await pass(store, github); + expect(store.stale.get("issue-1")?.reasons).toEqual(["global_evidence_commit"]); + }); + + it("falls back to global staleness when the commit date is missing or unavailable", async () => { + github.fetchHeadSha.mockResolvedValue("sha-2"); + github.compareCommits.mockResolvedValue({ ok: true, status: "ahead", files: [], truncated: false }); + const issue = { ...row(), groomedEvidenceScope: "global", groomedEvidencePaths: [], groomedSearchCodeQueries: ["query"] }; + github.fetchCommitDate.mockResolvedValue(null); + const missing = fakeStore([issue]); + await pass(missing, github); + expect(missing.stale.get("issue-1")?.reasons).toEqual(["global_evidence_commit"]); + + github.fetchCommitDate.mockRejectedValueOnce(new Error("boom")); + const failed = fakeStore([issue]); + await pass(failed, github); + expect(failed.stale.get("issue-1")?.reasons).toEqual(["global_evidence_commit"]); + }); + + it("stales a global result whose relied-on read path the commit touched, without rechecking", async () => { + github.fetchHeadSha.mockResolvedValue("sha-2"); + github.compareCommits.mockResolvedValue({ ok: true, status: "ahead", files: ["src/a.ts"], truncated: false }); + const store = fakeStore([ + { ...row(), groomedEvidenceScope: "global", groomedEvidencePaths: ["src/a.ts"], groomedSearchCodeQueries: ["query"] }, + ]); + await pass(store, github); + expect(store.stale.get("issue-1")?.reasons).toEqual(["global_evidence_commit"]); + expect(store.stale.get("issue-1")?.detail).toContain("src/a.ts"); + expect(github.searchCode).not.toHaveBeenCalled(); + }); + + it("falls back to global staleness when search is missing or fails", async () => { + github.fetchHeadSha.mockResolvedValue("sha-2"); + github.compareCommits.mockResolvedValue({ ok: true, status: "ahead", files: [], truncated: false }); + const issue = { ...row(), groomedEvidenceScope: "global", groomedEvidencePaths: [], groomedSearchCodeQueries: ["query"] }; + const missing = fakeStore([issue]); + const withoutSearch = { ...github } as { searchCode?: typeof github.searchCode } & Omit, "searchCode">; + delete withoutSearch.searchCode; + await pass(missing, withoutSearch as ReturnType); + expect(missing.stale.get("issue-1")?.reasons).toEqual(["global_evidence_commit"]); + + github.searchCode.mockRejectedValueOnce(new Error("rate limited")); + const failed = fakeStore([issue]); + await pass(failed, github); + expect(failed.stale.get("issue-1")?.reasons).toEqual(["global_evidence_commit"]); + expect(failed.stale.get("issue-1")?.detail).toBe("default branch moved sha-1...sha-2 and the result relied on repo-wide evidence"); }); it("ignores commits for a result that used no repository evidence", async () => { diff --git a/src/lib/groomer/freshness-invalidation.ts b/src/lib/groomer/freshness-invalidation.ts index 036d070a..4103b9af 100644 --- a/src/lib/groomer/freshness-invalidation.ts +++ b/src/lib/groomer/freshness-invalidation.ts @@ -16,8 +16,8 @@ import { prisma } from "@/lib/prisma"; import { fetchIssue, fetchIssueComments } from "@/lib/github-issues"; import { fetchPullRequestState } from "@/lib/github-prs"; -import { fetchLatestCommit } from "@/lib/github-ci"; -import { compareCommits, type CommitComparison } from "@/lib/github-code-search"; +import { fetchLatestCommit, fetchCommitDate } from "@/lib/github-ci"; +import { compareCommits, searchRepositoryCode, type CommitComparison } from "@/lib/github-code-search"; import { dependencyKey } from "@/lib/issue-dependencies"; import { findOpenIssueKeys } from "@/lib/issue-dependency-annotation"; import { isAutomationAuthor } from "./context"; @@ -52,6 +52,7 @@ export interface FreshnessIssueRow { groomedEvidenceCapturedAt: Date | null; groomedEvidenceScope: string | null; groomedEvidencePaths: string[]; + groomedSearchCodeQueries: string[]; groomedDependencyKeys: string[]; groomedOpenBlockerKeys: string[]; groomedRelatedWork: unknown; @@ -79,6 +80,9 @@ export interface FreshnessStore { export interface FreshnessGitHub { fetchHeadSha(repoFullName: string, branch: string): Promise; + searchCode?(repoFullName: string, query: string, limit: number): Promise<{ path: string }[]>; + /** Committer timestamp for a sha; used to judge code-search index catch-up. */ + fetchCommitDate?(repoFullName: string, sha: string): Promise; compareCommits(repoFullName: string, base: string, head: string): Promise; fetchRecentComments( repoFullName: string, @@ -98,6 +102,8 @@ export interface FreshnessBudget { maxCommentFetches: number; /** Related issue/PR state reads per pass (tracked issues come from the cache, free). */ maxRelatedFetches: number; + /** Saved negative code-search queries rechecked per pass. */ + maxSearchCodeRechecks: number; /** Comments read per comment check. */ commentWindow: number; } @@ -107,9 +113,26 @@ export const DEFAULT_FRESHNESS_BUDGET: FreshnessBudget = { maxCompares: 10, maxCommentFetches: 10, maxRelatedFetches: 10, + maxSearchCodeRechecks: 20, commentWindow: 30, }; +/** + * GitHub code search runs against an index that can lag the default branch. + * A saved empty query only counts as "still absent" once the new head commit + * is at least this old; younger heads defer the recheck to a later pass + * (#1091). Failures and missing timestamps stay conservative instead. + */ +export const SEARCH_RECHECK_INDEX_GRACE_MS = 30 * 60 * 1000; + +/** + * How long a recheck may stay deferred because the code-search index has not + * demonstrably caught up. Once the oldest unverified commit in base...head is + * older than this, the deferral is bounded and the result goes stale + * conservatively instead of starving forever in a busy repo (#1091 review). + */ +export const SEARCH_RECHECK_DEFER_LIMIT_MS = 2 * 60 * 60 * 1000; + export interface FreshnessPassResult { issuesChecked: number; markedStale: Array<{ repo: string; issueNumber: number; reasons: GroomingStaleReason[] }>; @@ -160,6 +183,7 @@ export async function runGroomingFreshnessPass( compares: budget.maxCompares, comments: budget.maxCommentFetches, related: budget.maxRelatedFetches, + searchCode: budget.maxSearchCodeRechecks, }; for (const repo of repos) { @@ -177,7 +201,7 @@ async function evaluateRepo( store: FreshnessStore, github: FreshnessGitHub, budget: FreshnessBudget, - remaining: { compares: number; comments: number; related: number }, + remaining: { compares: number; comments: number; related: number; searchCode: number }, result: FreshnessPassResult, now: () => Date, ): Promise { @@ -284,7 +308,7 @@ async function evaluateRepo( } // 5. Default-branch commits since the last verified SHA. - await evaluateCommits(repo.fullName, live(), github, remaining, result); + await evaluateCommits(repo.fullName, live(), github, remaining, result, now); // Persist. const at = now(); @@ -342,8 +366,9 @@ async function evaluateCommits( repoFullName: string, evaluations: Evaluation[], github: FreshnessGitHub, - remaining: { compares: number }, + remaining: { compares: number; searchCode: number }, result: FreshnessPassResult, + now: () => Date, ): Promise { const sensitive = evaluations.filter( (evaluation) => @@ -399,19 +424,22 @@ async function evaluateCommits( remaining.compares--; result.githubCalls++; const comparison = await github.compareCommits(repoFullName, base, head); - applyComparison(members, comparison, base, head, repoFullName, result); + await applyComparison(members, comparison, base, head, repoFullName, github, remaining, result, now); } } } -function applyComparison( +async function applyComparison( members: Evaluation[], comparison: CommitComparison, base: string, head: string, repoFullName: string, + github: FreshnessGitHub, + remaining: { searchCode: number }, result: FreshnessPassResult, -): void { + now: () => Date, +): Promise { const range = `${base.slice(0, 12)}...${head.slice(0, 12)}`; if (!comparison.ok) { if (comparison.definitive) { @@ -433,8 +461,118 @@ function applyComparison( for (const evaluation of members) stale(evaluation, "compare_unreliable", `${range}: ${why}`); return; } + // The head commit timestamp is shared by every member of the comparison + // (they compare against the same head), so it is fetched at most once and + // costs one budget unit per comparison, not per issue (#1091 review). + // "exhausted" (budget ran out) defers within the defer limit like every + // other unverified case; "failed" (fetch errored / unusable date) is + // conservative and stales. + let headCommittedAt: number | null = null; + let headDateState: "unresolved" | "ok" | "failed" | "exhausted" = "unresolved"; + const resolveHeadDate = async ( + fetchCommitDate: NonNullable, + ): Promise<{ at: number | null; state: "ok" | "failed" | "exhausted" }> => { + if (headDateState === "unresolved") { + if (remaining.searchCode <= 0) { + headDateState = "exhausted"; + } else { + remaining.searchCode--; + result.githubCalls++; + try { + const date = await fetchCommitDate(repoFullName, head); + const parsed = date ? Date.parse(date) : Number.NaN; + if (Number.isNaN(parsed)) { + headDateState = "failed"; + } else { + headCommittedAt = parsed; + headDateState = "ok"; + } + } catch { + headDateState = "failed"; + } + } + } + return { at: headCommittedAt, state: headDateState }; + }; + + // A deferral is only safe while it is bounded: once the oldest unverified + // commit has waited past the limit, the code-search index cannot be + // trusted to ever confirm "still absent" for this range, so the result + // goes stale conservatively instead of starving forever (#1091 review). + const deferWithinBound = (evaluation: Evaluation): void => { + const firstAt = comparison.firstCommitDate ? Date.parse(comparison.firstCommitDate) : Number.NaN; + const firstAge = Number.isNaN(firstAt) ? Number.POSITIVE_INFINITY : now().getTime() - firstAt; + if (Number.isNaN(firstAt) || firstAge > SEARCH_RECHECK_DEFER_LIMIT_MS) { + stale(evaluation, "global_evidence_commit", `default branch moved ${range} and the result relied on repo-wide evidence`); + return; + } + evaluation.deferred = true; + }; + for (const evaluation of members) { if (evaluation.issue.groomedEvidenceScope === "global") { + const queries = evaluation.issue.groomedSearchCodeQueries ?? []; + // A commit that touches a relied-on read path invalidates the result + // regardless of what the saved searches say (#1091: other global + // evidence keeps the conservative behaviour). + const pathHits = intersectEvidencePaths(evaluation.issue.groomedEvidencePaths, comparison.files); + if (pathHits.length > 0) { + const shown = pathHits.slice(0, 5).join(", ") + (pathHits.length > 5 ? `, +${pathHits.length - 5} more` : ""); + stale(evaluation, "global_evidence_commit", `default branch moved ${range}; commit touched relied-on evidence paths: ${shown}`); + continue; + } + if (queries.length > 0 && github.searchCode && github.fetchCommitDate) { + const fetchCommitDate = github.fetchCommitDate; + const resolved = await resolveHeadDate(fetchCommitDate); + if (resolved.state === "ok" && resolved.at !== null) { + if (now().getTime() - resolved.at < SEARCH_RECHECK_INDEX_GRACE_MS) { + // The head is too recent for the code-search index to have caught + // up; an empty recheck now would not mean "still absent". + deferWithinBound(evaluation); + continue; + } + let matchedQuery: string | null = null; + let exhausted = false; + let failed = false; + for (const query of queries) { + if (remaining.searchCode <= 0) { + exhausted = true; + break; + } + remaining.searchCode--; + result.githubCalls++; + try { + if ((await github.searchCode(repoFullName, query, 1)).length > 0) { + matchedQuery = query; + break; + } + } catch { + failed = true; + break; + } + } + if (matchedQuery) { + stale(evaluation, "global_evidence_commit", `default branch moved ${range}; previously empty search query now matches: ${matchedQuery}`); + continue; + } + if (failed) { + stale(evaluation, "global_evidence_commit", `default branch moved ${range} and the result relied on repo-wide evidence`); + continue; + } + if (exhausted) { + // Out of budget, not evidence of change: leave unverified for a + // later pass, bounded by the defer limit like the young-head case. + deferWithinBound(evaluation); + continue; + } + evaluation.advance.groomingVerifiedSha = head; + continue; + } + if (resolved.state === "exhausted") { + deferWithinBound(evaluation); + continue; + } + } stale(evaluation, "global_evidence_commit", `default branch moved ${range} and the result relied on repo-wide evidence`); continue; } @@ -562,6 +700,7 @@ const FRESHNESS_SELECT = { groomedEvidenceCapturedAt: true, groomedEvidenceScope: true, groomedEvidencePaths: true, + groomedSearchCodeQueries: true, groomedDependencyKeys: true, groomedOpenBlockerKeys: true, groomedRelatedWork: true, @@ -634,6 +773,8 @@ export const defaultFreshnessGitHub: FreshnessGitHub = { return (await fetchLatestCommit(repoFullName, branch))?.sha ?? null; }, compareCommits, + searchCode: searchRepositoryCode, + fetchCommitDate, async fetchRecentComments(repoFullName, issueNumber, max) { const comments = await fetchIssueComments(repoFullName, issueNumber, max, "desc"); return comments.map((comment) => ({ author: comment.user?.login ?? "unknown", createdAt: comment.created_at ?? "" })); diff --git a/src/lib/groomer/freshness.test.ts b/src/lib/groomer/freshness.test.ts index daee31d4..460d3412 100644 --- a/src/lib/groomer/freshness.test.ts +++ b/src/lib/groomer/freshness.test.ts @@ -6,6 +6,7 @@ import { deriveEvidenceScope, deriveGroomingFreshness, dependencyKeysForIssue, + explorationCallsForFreshness, hasNegativeSearchResult, intersectEvidencePaths, isFreshnessTrackedStatus, @@ -259,11 +260,95 @@ describe("buildGroomingFreshnessBaseline", () => { expect(baseline.groomedEvidencePaths).toEqual([]); }); - it("marks a negative code search as global evidence", async () => { + it("records trimmed, deduplicated empty-search queries only for global evidence", async () => { + const calls = [ + { name: "search_code", ok: true, bytes: 0, arguments: { query: " missing symbol " } }, + { name: "search_code", ok: true, bytes: 0, arguments: { query: "missing symbol" } }, + { name: "search_code", ok: true, bytes: 0, arguments: { query: "x".repeat(200) } }, + { name: "search_code", ok: false, bytes: 0, arguments: { query: "failed" } }, + ]; + const global = await buildGroomingFreshnessBaseline(input({ explorationToolCalls: calls })); + expect(global.groomedEvidenceScope).toBe("global"); + expect(global.groomedSearchCodeQueries).toEqual(["missing symbol", "x".repeat(200)]); + + const paths = await buildGroomingFreshnessBaseline( + input({ + evidence: { ...evidence, sources: [{ path: "src/a.ts", provenance: "repository", via: "read", ref: "sha-1" }] }, + explorationToolCalls: [{ name: "search_code", ok: false, bytes: 0, arguments: { query: "missing symbol" } }], + citations: [{ id: "repo:src/a.ts", subject: "repository", state: null }], + }), + ); + expect(paths.groomedEvidenceScope).toBe("paths"); + expect(paths.groomedSearchCodeQueries).toEqual([]); + }); + + it("does not save queries for a global scope caused by a surfaced path", async () => { + const baseline = await buildGroomingFreshnessBaseline( + input({ + citations: [{ id: "repo:src/hit.ts", subject: "repository", state: null }], + explorationToolCalls: [{ name: "search_code", ok: true, bytes: 0, arguments: { query: "missing symbol" } }], + }), + ); + expect(baseline.groomedEvidenceScope).toBe("global"); + expect(baseline.groomedSearchCodeQueries).toEqual([]); + }); + + it("does not save queries for a no-read-path global when repository-context queries ran", async () => { + const baseline = await buildGroomingFreshnessBaseline( + input({ + evidence: { ...evidence, sources: [] }, + repositoryQueries: ["sslmode"], + explorationToolCalls: [{ name: "search_code", ok: true, bytes: 0, arguments: { query: "missing symbol" } }], + }), + ); + expect(baseline.groomedEvidenceScope).toBe("global"); + expect(baseline.groomedSearchCodeQueries).toEqual([]); + }); + + it("does not save queries for a no-read-path global when list_directory surfaced evidence", async () => { const baseline = await buildGroomingFreshnessBaseline( - input({ explorationToolCalls: [{ name: "search_code", ok: true, bytes: 0 }] }), + input({ + evidence: { ...evidence, sources: [] }, + explorationToolCalls: [ + { name: "list_directory", ok: true, bytes: 50, arguments: { path: "src" } }, + { name: "search_code", ok: true, bytes: 0, arguments: { query: "missing symbol" } }, + ], + }), ); expect(baseline.groomedEvidenceScope).toBe("global"); + expect(baseline.groomedSearchCodeQueries).toEqual([]); + }); + + it("explorationCallsForFreshness keeps arguments and drops other fields", () => { + expect( + explorationCallsForFreshness([ + { name: "search_code", arguments: { query: "q" }, ok: true, bytes: 0, preview: "No matches" }, + ]), + ).toEqual([{ name: "search_code", arguments: { query: "q" }, ok: true, bytes: 0 }]); + }); + + const emptySearches = (queries: unknown[]) => + queries.map((query) => ({ name: "search_code", ok: true, bytes: 0, arguments: { query } })); + + it("saves up to ten empty-search queries, below the pass search budget", async () => { + const queries = Array.from({ length: 10 }, (_, index) => `missing ${index}`); + const baseline = await buildGroomingFreshnessBaseline(input({ explorationToolCalls: emptySearches(queries) })); + expect(baseline.groomedSearchCodeQueries).toEqual(queries); + }); + + it("saves none when the empty searches exceed the cap, so a subset is never rechecked", async () => { + const queries = Array.from({ length: 11 }, (_, index) => `missing ${index}`); + const baseline = await buildGroomingFreshnessBaseline(input({ explorationToolCalls: emptySearches(queries) })); + expect(baseline.groomedSearchCodeQueries).toEqual([]); + }); + + it("saves none when an empty search can't be saved whole", async () => { + const tooLong = await buildGroomingFreshnessBaseline( + input({ explorationToolCalls: emptySearches(["short", "x".repeat(201)]) }), + ); + expect(tooLong.groomedSearchCodeQueries).toEqual([]); + const unreadable = await buildGroomingFreshnessBaseline(input({ explorationToolCalls: emptySearches(["short", 42]) })); + expect(unreadable.groomedSearchCodeQueries).toEqual([]); }); }); diff --git a/src/lib/groomer/freshness.ts b/src/lib/groomer/freshness.ts index 77195fd9..ae911b37 100644 --- a/src/lib/groomer/freshness.ts +++ b/src/lib/groomer/freshness.ts @@ -47,6 +47,14 @@ export const MAX_BASELINE_PATHS = 60; export const MAX_BASELINE_RELATED_WORK = 20; /** Dependency keys kept on the baseline. */ export const MAX_BASELINE_DEPENDENCIES = 20; +/** + * Empty code-search queries retained to recheck global evidence. Capped well + * below the freshness pass's search budget (20) so a single issue can always + * complete its recheck — commit-date fetch plus every saved query — even + * when it is first in the pass (#1091 review). + */ +export const MAX_BASELINE_SEARCH_CODE_QUERIES = 10; +export const MAX_BASELINE_SEARCH_CODE_QUERY_CHARS = 200; /** * Statuses a worker owns. The freshness pass does not evaluate these (a claim @@ -95,6 +103,36 @@ export interface ExplorationToolCallLike { name: string; ok: boolean; bytes: number; + arguments?: Record; +} + +/** + * Map exploration tool records to the freshness input shape, keeping the + * call arguments so saved empty search queries can be recovered later. + */ +export function explorationCallsForFreshness( + toolCalls: Array<{ name: string; arguments: Record; ok: boolean; bytes: number; preview?: string }>, +): ExplorationToolCallLike[] { + return toolCalls.map(({ name, arguments: args, ok, bytes }) => ({ name, arguments: args, ok, bytes })); +} + +function emptySearchCodeQueries(toolCalls: ExplorationToolCallLike[]): string[] { + const queries: string[] = []; + for (const call of toolCalls) { + if (call.name !== "search_code" || !call.ok || call.bytes !== 0) continue; + // Every empty search is part of the negative evidence. One that can't be + // saved whole (unreadable, too long, or past the cap) would leave the + // recheck verifying the result on a subset, so save none and keep the + // conservative stale-on-commit behaviour instead. + const raw = call.arguments?.query; + if (typeof raw !== "string") return []; + const query = raw.trim(); + if (!query) return []; + if (queries.includes(query)) continue; + if (query.length > MAX_BASELINE_SEARCH_CODE_QUERY_CHARS || queries.length >= MAX_BASELINE_SEARCH_CODE_QUERIES) return []; + queries.push(query); + } + return queries; } /** @@ -320,6 +358,7 @@ export interface GroomingFreshnessBaseline { groomedEvidenceCapturedAt: Date; groomedEvidenceScope: GroomingEvidenceScope; groomedEvidencePaths: string[]; + groomedSearchCodeQueries: string[]; groomedDependencyKeys: string[]; groomedOpenBlockerKeys: string[]; groomedRelatedWork: RelatedWorkBaselineEntry[]; @@ -340,6 +379,7 @@ export const UNKNOWN_FRESHNESS: Record = { groomedEvidenceCapturedAt: null, groomedEvidenceScope: null, groomedEvidencePaths: [], + groomedSearchCodeQueries: [], groomedDependencyKeys: [], groomedOpenBlockerKeys: [], groomedRelatedWork: null, @@ -407,6 +447,21 @@ export async function buildGroomingFreshnessBaseline(input: GroomingFreshnessInp groomedEvidenceCapturedAt: input.evidenceWindowStart, groomedEvidenceScope: scope, groomedEvidencePaths: reliance.repositoryPaths, + // Only negative-search globals may be rechecked later (#1091). The other + // global cases — a relied-on path that was only surfaced, or repository + // access with no read path — keep the conservative stale-on-commit + // behaviour even when an empty search also happened during the run. + // A no-read-path global may still save its queries when exploration + // searches were its ONLY repository evidence: no repository-context + // queries ran and nothing else (e.g. list_directory) surfaced paths. + groomedSearchCodeQueries: + scope === "global" && + !reliance.reliesOnSurfacedPath && + (reliance.repositoryPaths.length > 0 || + (input.repositoryQueries.length === 0 && + !input.explorationToolCalls.some((call) => call.name === "list_directory"))) + ? emptySearchCodeQueries(input.explorationToolCalls) + : [], groomedDependencyKeys: dependencyKeys, groomedOpenBlockerKeys: dependencyKeys.filter((key) => openKeys.has(key)).sort(), groomedRelatedWork: reliance.relatedWork, diff --git a/src/lib/groomer/run.test.ts b/src/lib/groomer/run.test.ts index 4258a731..fd885878 100644 --- a/src/lib/groomer/run.test.ts +++ b/src/lib/groomer/run.test.ts @@ -1632,6 +1632,25 @@ Investigate session handling in auth module.`; expect(issueUpdateData()).toMatchObject({ groomedEvidenceScope: "global", groomedEvidencePaths: [] }); }); + it("saves empty exploration search queries in the freshness baseline (#1091)", async () => { + mocks.getHostedGroomerConfig.mockReturnValue({ ...mockConfig, toolLoopEnabled: true }); + mocks.exploreRepository.mockResolvedValue({ + ...mockExploration, + sources: [], + readSources: [], + toolCalls: [ + { name: "search_code", arguments: { query: "missing symbol" }, ok: true, bytes: 0, preview: "No matches" }, + { name: "search_code", arguments: { query: "also missing" }, ok: true, bytes: 0, preview: "No matches" }, + { name: "search_code", arguments: { query: "found it" }, ok: true, bytes: 400, preview: "src/a.ts" }, + ], + }); + await runHostedGroomer(); + expect(issueUpdateData()).toMatchObject({ + groomedEvidenceScope: "global", + groomedSearchCodeQueries: ["missing symbol", "also missing"], + }); + }); + it("does not record a baseline for a skipped in-flight run, leaving the prior one untouched", async () => { mocks.selectGroomingCandidate.mockResolvedValue({ ...mockCandidate, labels: ["status/in-progress", "priority/p0"] }); await runHostedGroomer(); diff --git a/src/lib/groomer/run.ts b/src/lib/groomer/run.ts index 172730a1..6d34ecaa 100644 --- a/src/lib/groomer/run.ts +++ b/src/lib/groomer/run.ts @@ -20,6 +20,7 @@ import { } from "./evidence-snapshot"; import type { RepositoryContextInput, RepositoryContextConfig } from "./repository-context"; import { createGroomingRunRecord, completeGroomingRunRecord, updateGroomingRunRecord } from "./history"; +import { explorationCallsForFreshness } from "./freshness"; import { freshnessBaselineIssueData } from "./freshness-invalidation"; import { compareCommits } from "@/lib/github-code-search"; import { validateApplyPreconditions, type LiveComment, type PreconditionReader } from "./mutation-validator"; @@ -676,7 +677,7 @@ async function executeGroomerRun( plan, repositoryQueries: repositoryContext.queries, explorationRan: exploration !== null, - explorationToolCalls: exploration?.toolCalls ?? [], + explorationToolCalls: exploration ? explorationCallsForFreshness(exploration.toolCalls) : [], }, reader, ); @@ -913,7 +914,7 @@ async function executeGroomerRun( evidenceWindowStart, repositoryQueries: repositoryContext.queries, explorationRan: exploration !== null, - explorationToolCalls: exploration?.toolCalls ?? [], + explorationToolCalls: exploration ? explorationCallsForFreshness(exploration.toolCalls) : [], citations: plan.citations, }), );