Skip to content

Commit 6306270

Browse files
snomiaoclaude
andcommitted
fix(gh-test-evidence): add batch limiting and update repo URL
- Update comfyanonymous/ComfyUI to Comfy-Org/ComfyUI - Add batch limit (50 PRs) and time limit (8 min) to avoid CI timeout - Skip PRs that don't need updates (already analyzed + correct comment state) - Return early when limits are reached Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 65b38d8 commit 6306270

1 file changed

Lines changed: 74 additions & 14 deletions

File tree

app/tasks/gh-test-evidence/gh-test-evidence.ts

Lines changed: 74 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,25 @@ import { OpenAI } from "openai";
1010
import { pageFlow } from "sflow";
1111
import z from "zod";
1212

13-
const REPOS = ["https://github.com/Comfy-Org/desktop", "https://github.com/comfyanonymous/ComfyUI"];
13+
const REPOS = ["https://github.com/Comfy-Org/desktop", "https://github.com/Comfy-Org/ComfyUI"];
1414

1515
const BOT_COMMENT_MARKER = "<!-- COMFY_PR_BOT_TEST_EVIDENCE -->";
1616

17+
// Batch limit to avoid CI timeout (10 min limit, ~3s per PR = ~200 max, use 50 for safety)
18+
const MAX_PRS_PER_RUN = 50;
19+
// Time limit in ms (8 minutes to leave buffer before 10 min timeout)
20+
const TIME_LIMIT_MS = 8 * 60 * 1000;
21+
1722
const TestEvidenceSchema = z.object({
18-
isTestExplanationIncluded: z.boolean().describe("true if PR body includes test plan or test explanation"),
19-
isTestScreenshotIncluded: z.boolean().describe("true if PR body includes test screenshots or images"),
20-
isTestVideoIncluded: z.boolean().describe("true if PR body includes test videos or YouTube links"),
23+
isTestExplanationIncluded: z
24+
.boolean()
25+
.describe("true if PR body includes test plan or test explanation"),
26+
isTestScreenshotIncluded: z
27+
.boolean()
28+
.describe("true if PR body includes test screenshots or images"),
29+
isTestVideoIncluded: z
30+
.boolean()
31+
.describe("true if PR body includes test videos or YouTube links"),
2132
});
2233

2334
type TestEvidence = z.infer<typeof TestEvidenceSchema>;
@@ -61,6 +72,8 @@ if (import.meta.main) {
6172

6273
export default async function runGhTestEvidenceTask() {
6374
console.log("Starting test evidence check task...");
75+
const startTime = Date.now();
76+
let processedCount = 0;
6477

6578
for (const repoUrl of REPOS) {
6679
console.log(`Processing repo: ${repoUrl}`);
@@ -81,22 +94,64 @@ export default async function runGhTestEvidenceTask() {
8194
console.log(`Found ${prs.length} open PRs in ${repoUrl}`);
8295

8396
for (const pr of prs) {
97+
// Check time limit
98+
const elapsed = Date.now() - startTime;
99+
if (elapsed >= TIME_LIMIT_MS) {
100+
console.log(
101+
`Time limit reached (${Math.round(elapsed / 1000)}s), stopping. Processed ${processedCount} PRs.`,
102+
);
103+
return;
104+
}
105+
106+
// Check batch limit
107+
if (processedCount >= MAX_PRS_PER_RUN) {
108+
console.log(`Batch limit reached (${MAX_PRS_PER_RUN} PRs), stopping.`);
109+
return;
110+
}
111+
84112
try {
85-
await processPR(pr, repoUrl);
113+
const wasProcessed = await processPR(pr, repoUrl);
114+
if (wasProcessed) processedCount++;
86115
} catch (error) {
87116
console.error(`Error processing PR ${pr.html_url}:`, error);
88117
}
89118
}
90119
}
91120

92-
console.log("Test evidence check task completed");
121+
console.log(`Test evidence check task completed. Processed ${processedCount} PRs.`);
93122
}
94123

95-
async function processPR(pr: Awaited<ReturnType<typeof ghc.pulls.list>>["data"][0], repoUrl: string) {
124+
/** Process a PR and return true if any work was done (API calls made) */
125+
async function processPR(
126+
pr: Awaited<ReturnType<typeof ghc.pulls.list>>["data"][0],
127+
repoUrl: string,
128+
): Promise<boolean> {
96129
// Skip drafts
97130
if (pr.draft) {
98131
console.log(`Skipping draft PR: ${pr.html_url}`);
99-
return;
132+
return false;
133+
}
134+
135+
// Check existing task state
136+
const existingTask = await GithubTestEvidenceTask.findOne({ prUrl: pr.html_url });
137+
138+
// Check if PR was updated since last analysis
139+
const prUpdatedAt = new Date(pr.updated_at);
140+
const needsAnalysis =
141+
!existingTask?.evidenceAnalyzedAt || prUpdatedAt > existingTask.evidenceAnalyzedAt;
142+
143+
// If no analysis needed and we have comment state tracked, skip entirely
144+
if (!needsAnalysis && existingTask?.evidence) {
145+
const hasMissingEvidence =
146+
!existingTask.evidence.isTestExplanationIncluded ||
147+
(!existingTask.evidence.isTestScreenshotIncluded &&
148+
!existingTask.evidence.isTestVideoIncluded);
149+
const hasComment = !!existingTask.commentId;
150+
151+
// If state is consistent (has warning + has comment, or no warning + no comment), skip
152+
if ((hasMissingEvidence && hasComment) || (!hasMissingEvidence && !hasComment)) {
153+
return false;
154+
}
100155
}
101156

102157
// Save basic PR info
@@ -105,13 +160,10 @@ async function processPR(pr: Awaited<ReturnType<typeof ghc.pulls.list>>["data"][
105160
prNumber: pr.number,
106161
prTitle: pr.title,
107162
prBody: pr.body,
108-
prUpdatedAt: new Date(pr.updated_at),
163+
prUpdatedAt,
109164
repoUrl,
110165
});
111166

112-
// Check if we need to re-analyze (PR was updated after last analysis)
113-
const needsAnalysis = !task.evidenceAnalyzedAt || new Date(pr.updated_at) > task.evidenceAnalyzedAt;
114-
115167
if (needsAnalysis) {
116168
console.log(`Analyzing PR: ${pr.html_url}`);
117169
const evidence = await analyzeTestEvidence(pr);
@@ -182,9 +234,13 @@ async function processPR(pr: Awaited<ReturnType<typeof ghc.pulls.list>>["data"][
182234
});
183235
}
184236
}
237+
238+
return true;
185239
}
186240

187-
async function analyzeTestEvidence(pr: Awaited<ReturnType<typeof ghc.pulls.list>>["data"][0]): Promise<TestEvidence> {
241+
async function analyzeTestEvidence(
242+
pr: Awaited<ReturnType<typeof ghc.pulls.list>>["data"][0],
243+
): Promise<TestEvidence> {
188244
const openai = new OpenAI({
189245
apiKey: process.env.OPENAI_API_KEY || DIE("OPENAI_API_KEY not found"),
190246
});
@@ -218,7 +274,11 @@ Be lenient - if there's any indication of testing explanation or visual evidence
218274
isTestScreenshotIncluded: { type: "boolean" },
219275
isTestVideoIncluded: { type: "boolean" },
220276
},
221-
required: ["isTestExplanationIncluded", "isTestScreenshotIncluded", "isTestVideoIncluded"],
277+
required: [
278+
"isTestExplanationIncluded",
279+
"isTestScreenshotIncluded",
280+
"isTestVideoIncluded",
281+
],
222282
additionalProperties: false,
223283
},
224284
},

0 commit comments

Comments
 (0)