diff --git a/coverage-service/.env.example b/coverage-service/.env.example index 6d9a25c..d4fcb4f 100644 --- a/coverage-service/.env.example +++ b/coverage-service/.env.example @@ -38,7 +38,13 @@ WORKER_FORCE_SHUTDOWN=false MAX_MEMORY_MB=2048 MAX_CPU_CORES=2 -# Coverage workflow -# TEST_THRESHOLD accepts 80 (percent) or 0.8 (fraction). Diff coverage is preferred; overall coverage is the fallback. +# Coverage workflow / optimization +# TARGET_DIFF_COVERAGE accepts 80 (percent) or 0.8 (fraction). Falls back to TEST_THRESHOLD. +TARGET_DIFF_COVERAGE=80 TEST_THRESHOLD=80 +MAX_OPTIMIZATION_ITERATIONS=8 MAX_GENERATION_ATTEMPTS=3 +MAX_REPAIR_ATTEMPTS=3 +MIN_COVERAGE_GAIN=1 +ENABLE_REPAIR_LOOP=true +MAX_FILES_PER_ITERATION=10 diff --git a/coverage-service/api/prisma/migrations/20250629000000_add_coverage_optimization/migration.sql b/coverage-service/api/prisma/migrations/20250629000000_add_coverage_optimization/migration.sql new file mode 100644 index 0000000..6ba748d --- /dev/null +++ b/coverage-service/api/prisma/migrations/20250629000000_add_coverage_optimization/migration.sql @@ -0,0 +1,63 @@ +-- CreateEnum +CREATE TYPE "GeneratedTestStatus" AS ENUM ('PENDING', 'PASSING', 'FAILED'); + +-- CreateTable +CREATE TABLE "CoverageIteration" ( + "id" TEXT NOT NULL, + "prRunId" TEXT NOT NULL, + "iteration" INTEGER NOT NULL, + "coverageBefore" DOUBLE PRECISION NOT NULL, + "coverageAfter" DOUBLE PRECISION, + "coverageGain" DOUBLE PRECISION, + "generatedTests" INTEGER NOT NULL DEFAULT 0, + "failedTests" INTEGER NOT NULL DEFAULT 0, + "stopReason" TEXT, + "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + + CONSTRAINT "CoverageIteration_pkey" PRIMARY KEY ("id") +); + +-- CreateTable +CREATE TABLE "CoverageGap" ( + "id" TEXT NOT NULL, + "iterationId" TEXT NOT NULL, + "filePath" TEXT NOT NULL, + "coverage" DOUBLE PRECISION NOT NULL, + "missingLines" INTEGER[], + "priority" DOUBLE PRECISION NOT NULL, + "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + + CONSTRAINT "CoverageGap_pkey" PRIMARY KEY ("id") +); + +-- AlterTable +ALTER TABLE "GeneratedTestArtifact" ADD COLUMN "coverageIterationId" TEXT, +ADD COLUMN "status" "GeneratedTestStatus" NOT NULL DEFAULT 'PENDING', +ADD COLUMN "repairAttempts" INTEGER NOT NULL DEFAULT 0, +ADD COLUMN "coverageContribution" DOUBLE PRECISION, +ADD COLUMN "failureReason" TEXT; + +-- CreateIndex +CREATE UNIQUE INDEX "CoverageIteration_prRunId_iteration_key" ON "CoverageIteration"("prRunId", "iteration"); + +-- CreateIndex +CREATE INDEX "CoverageIteration_prRunId_idx" ON "CoverageIteration"("prRunId"); + +-- CreateIndex +CREATE INDEX "CoverageGap_iterationId_idx" ON "CoverageGap"("iterationId"); + +-- CreateIndex +CREATE INDEX "GeneratedTestArtifact_coverageIterationId_idx" ON "GeneratedTestArtifact"("coverageIterationId"); + +-- AddForeignKey +ALTER TABLE "CoverageIteration" ADD CONSTRAINT "CoverageIteration_prRunId_fkey" FOREIGN KEY ("prRunId") REFERENCES "PullRequestRun"("id") ON DELETE CASCADE ON UPDATE CASCADE; + +-- AddForeignKey +ALTER TABLE "CoverageGap" ADD CONSTRAINT "CoverageGap_iterationId_fkey" FOREIGN KEY ("iterationId") REFERENCES "CoverageIteration"("id") ON DELETE CASCADE ON UPDATE CASCADE; + +-- AddForeignKey +ALTER TABLE "GeneratedTestArtifact" ADD CONSTRAINT "GeneratedTestArtifact_coverageIterationId_fkey" FOREIGN KEY ("coverageIterationId") REFERENCES "CoverageIteration"("id") ON DELETE SET NULL ON UPDATE CASCADE; + +-- Backfill status from passed column +UPDATE "GeneratedTestArtifact" SET "status" = 'PASSING' WHERE "passed" = true; +UPDATE "GeneratedTestArtifact" SET "status" = 'FAILED' WHERE "passed" = false; diff --git a/coverage-service/api/prisma/schema.prisma b/coverage-service/api/prisma/schema.prisma index 2907f33..4ea51bd 100644 --- a/coverage-service/api/prisma/schema.prisma +++ b/coverage-service/api/prisma/schema.prisma @@ -51,6 +51,7 @@ model PullRequestRun { completedAt DateTime? coverageResult CoverageResult? generatedTests GeneratedTestArtifact[] + coverageIterations CoverageIteration[] executionLogs ExecutionLog[] llmUsage LlmUsageRecord[] @@ -110,17 +111,63 @@ model TestGenerationLog { createdAt DateTime @default(now()) } +enum GeneratedTestStatus { + PENDING + PASSING + FAILED +} + +model CoverageIteration { + id String @id @default(cuid()) + prRunId String + prRun PullRequestRun @relation(fields: [prRunId], references: [id], onDelete: Cascade) + iteration Int + coverageBefore Float + coverageAfter Float? + coverageGain Float? + generatedTests Int @default(0) + failedTests Int @default(0) + stopReason String? + createdAt DateTime @default(now()) + gaps CoverageGap[] + testArtifacts GeneratedTestArtifact[] + + @@unique([prRunId, iteration]) + @@index([prRunId]) +} + +model CoverageGap { + id String @id @default(cuid()) + iterationId String + iteration CoverageIteration @relation(fields: [iterationId], references: [id], onDelete: Cascade) + filePath String + coverage Float + missingLines Int[] + priority Float + createdAt DateTime @default(now()) + + @@index([iterationId]) +} + model GeneratedTestArtifact { - id String @id @default(cuid()) - prRunId String? - prRun PullRequestRun? @relation(fields: [prRunId], references: [id], onDelete: Cascade) - testGenerationRunId String? @unique - testGenerationRun TestGenerationRun? @relation(fields: [testGenerationRunId], references: [id], onDelete: Cascade) - filePath String - targetFile String - content String @db.Text - passed Boolean? - createdAt DateTime @default(now()) + id String @id @default(cuid()) + prRunId String? + prRun PullRequestRun? @relation(fields: [prRunId], references: [id], onDelete: Cascade) + coverageIterationId String? + coverageIteration CoverageIteration? @relation(fields: [coverageIterationId], references: [id], onDelete: SetNull) + testGenerationRunId String? @unique + testGenerationRun TestGenerationRun? @relation(fields: [testGenerationRunId], references: [id], onDelete: Cascade) + filePath String + targetFile String + content String @db.Text + passed Boolean? + status GeneratedTestStatus @default(PENDING) + repairAttempts Int @default(0) + coverageContribution Float? + failureReason String? @db.Text + createdAt DateTime @default(now()) + + @@index([coverageIterationId]) } model ExecutionLog { diff --git a/coverage-service/lib/src/coverage/coverage-config.ts b/coverage-service/lib/src/coverage/coverage-config.ts index f34b322..87fe389 100644 --- a/coverage-service/lib/src/coverage/coverage-config.ts +++ b/coverage-service/lib/src/coverage/coverage-config.ts @@ -1,4 +1,16 @@ -/** Parse TEST_THRESHOLD — accepts 80 (percent) or 0.8 (fraction). Default 80. */ +/** Parse TARGET_DIFF_COVERAGE or TEST_THRESHOLD — accepts 80 (percent) or 0.8 (fraction). Default 80. */ +export function getTargetDiffCoveragePercent(): number { + const targetRaw = process.env.TARGET_DIFF_COVERAGE?.trim(); + if (targetRaw) { + const value = parseFloat(targetRaw); + if (!Number.isNaN(value)) { + return value <= 1 ? value * 100 : value; + } + } + return getTestThresholdPercent(); +} + +/** @deprecated Use getTargetDiffCoveragePercent — kept for backward compatibility. */ export function getTestThresholdPercent(): number { const raw = process.env.TEST_THRESHOLD?.trim(); if (!raw) return 80; @@ -17,3 +29,46 @@ export function getMaxGenerationAttempts(): number { const value = parseInt(raw, 10); return Number.isNaN(value) || value < 1 ? 3 : value; } + +/** Parse MAX_OPTIMIZATION_ITERATIONS. Default 8. */ +export function getMaxOptimizationIterations(): number { + const raw = process.env.MAX_OPTIMIZATION_ITERATIONS?.trim(); + if (!raw) return 8; + + const value = parseInt(raw, 10); + return Number.isNaN(value) || value < 1 ? 8 : value; +} + +/** Parse MAX_REPAIR_ATTEMPTS. Default 3. */ +export function getMaxRepairAttempts(): number { + const raw = process.env.MAX_REPAIR_ATTEMPTS?.trim(); + if (!raw) return 3; + + const value = parseInt(raw, 10); + return Number.isNaN(value) || value < 1 ? 3 : value; +} + +/** Parse MIN_COVERAGE_GAIN (percentage points). Default 1. */ +export function getMinCoverageGain(): number { + const raw = process.env.MIN_COVERAGE_GAIN?.trim(); + if (!raw) return 1; + + const value = parseFloat(raw); + return Number.isNaN(value) || value < 0 ? 1 : value; +} + +/** Parse ENABLE_REPAIR_LOOP. Default true. */ +export function isRepairLoopEnabled(): boolean { + const raw = process.env.ENABLE_REPAIR_LOOP?.trim()?.toLowerCase(); + if (!raw) return true; + return raw !== 'false' && raw !== '0' && raw !== 'no'; +} + +/** Max files to process per optimization iteration. Default 10. */ +export function getMaxFilesPerIteration(): number { + const raw = process.env.MAX_FILES_PER_ITERATION?.trim(); + if (!raw) return 10; + + const value = parseInt(raw, 10); + return Number.isNaN(value) || value < 1 ? 10 : value; +} diff --git a/coverage-service/lib/src/coverage/coverage-gap-analysis.ts b/coverage-service/lib/src/coverage/coverage-gap-analysis.ts new file mode 100644 index 0000000..70ea0d3 --- /dev/null +++ b/coverage-service/lib/src/coverage/coverage-gap-analysis.ts @@ -0,0 +1,121 @@ +import type { ChangedFile, DiffCoverageReport } from '../types'; +import { pathsMatch } from './cobertura-parser'; + +export interface CoverageGapEntry { + file: string; + coverage: number; + uncoveredLines: number[]; + uncoveredPercentage: number; + /** Estimated file complexity (line count of source). */ + complexity: number; + /** Priority score assigned by CoveragePriorityService. */ + priority?: number; +} + +export interface CoverageGapsReport { + files: CoverageGapEntry[]; + totalUncoveredLines: number; +} + +/** Extract per-file coverage gaps from a diff-cover report. */ +export function getCoverageGaps( + report: DiffCoverageReport, + thresholdPercent: number, +): CoverageGapsReport { + const files: CoverageGapEntry[] = []; + + for (const entry of report.fileCoverage) { + const effective = entry.diffCoveragePercent ?? entry.lineCoveragePercent; + const uncoveredLines = + entry.uncoveredLines.length > 0 + ? entry.uncoveredLines + : report.uncoveredLines + .filter((l) => pathsMatch(l.file, entry.file)) + .map((l) => l.line); + + if (effective >= thresholdPercent && uncoveredLines.length === 0) { + continue; + } + + const uncoveredPercentage = Math.max(0, 100 - effective); + + files.push({ + file: entry.file, + coverage: effective, + uncoveredLines: [...new Set(uncoveredLines)].sort((a, b) => a - b), + uncoveredPercentage, + complexity: 0, + }); + } + + // Files with uncovered lines but no fileCoverage entry + const coveredFiles = new Set(files.map((f) => f.file)); + for (const { file, line } of report.uncoveredLines) { + if (coveredFiles.has(file)) continue; + const existing = files.find((f) => pathsMatch(f.file, file)); + if (existing) { + if (!existing.uncoveredLines.includes(line)) { + existing.uncoveredLines.push(line); + existing.uncoveredLines.sort((a, b) => a - b); + } + } else { + files.push({ + file, + coverage: 0, + uncoveredLines: [line], + uncoveredPercentage: 100, + complexity: 0, + }); + coveredFiles.add(file); + } + } + + const totalUncoveredLines = files.reduce( + (sum, f) => sum + f.uncoveredLines.length, + 0, + ); + + return { files, totalUncoveredLines }; +} + +/** Map coverage gaps to changed source files eligible for test generation. */ +export function gapsToChangedFiles( + gaps: CoverageGapEntry[], + sourceFiles: ChangedFile[], +): ChangedFile[] { + const result: ChangedFile[] = []; + for (const gap of gaps) { + const match = sourceFiles.find((f) => pathsMatch(f.path, gap.file)); + if (match && !result.some((r) => pathsMatch(r.path, match.path))) { + result.push(match); + } + } + return result; +} + +/** Detect stagnation: two consecutive iterations with gain below minimum. */ +export function shouldStopForStagnation( + recentGains: number[], + minGain: number, +): boolean { + if (recentGains.length < 2) return false; + const lastTwo = recentGains.slice(-2); + return lastTwo.every((g) => g < minGain); +} + +export type OptimizationStopReason = + | 'target_reached' + | 'max_iterations' + | 'plateau' + | 'no_gaps' + | 'unrecoverable_failure' + | 'no_llm'; + +export function formatGapSummary(gaps: CoverageGapEntry[]): string { + return gaps + .map( + (g) => + `- ${g.file}: ${g.uncoveredLines.slice(0, 20).join(',')}${g.uncoveredLines.length > 20 ? '…' : ''}`, + ) + .join('\n'); +} diff --git a/coverage-service/lib/src/coverage/coverage-priority.ts b/coverage-service/lib/src/coverage/coverage-priority.ts new file mode 100644 index 0000000..d4be47f --- /dev/null +++ b/coverage-service/lib/src/coverage/coverage-priority.ts @@ -0,0 +1,39 @@ +import type { CoverageGapEntry } from './coverage-gap-analysis'; + +export interface PriorityContext { + /** Number of previous failed generation/repair attempts per file. */ + failureCounts: Map; + /** Source line counts per file for complexity scoring. */ + sourceLineCounts: Map; +} + +/** + * Score and sort coverage gaps for the next optimization iteration. + * + * priority = missingDiffLines * 10 + uncoveredPercentage - failedAttempts * 5 + */ +export function prioritizeCoverageGaps( + gaps: CoverageGapEntry[], + context: PriorityContext, +): CoverageGapEntry[] { + const scored = gaps.map((gap) => { + const missingLines = gap.uncoveredLines.length; + const failedAttempts = context.failureCounts.get(gap.file) ?? 0; + const lineCount = context.sourceLineCounts.get(gap.file) ?? 0; + const complexityPenalty = lineCount > 500 ? 5 : lineCount > 200 ? 2 : 0; + + const priority = + missingLines * 10 + + gap.uncoveredPercentage - + failedAttempts * 5 - + complexityPenalty; + + return { + ...gap, + complexity: lineCount, + priority, + }; + }); + + return scored.sort((a, b) => (b.priority ?? 0) - (a.priority ?? 0)); +} diff --git a/coverage-service/lib/src/coverage/coverage-workflow.ts b/coverage-service/lib/src/coverage/coverage-workflow.ts index 605650b..cc2ebdc 100644 --- a/coverage-service/lib/src/coverage/coverage-workflow.ts +++ b/coverage-service/lib/src/coverage/coverage-workflow.ts @@ -11,8 +11,10 @@ import type { FileCoverage, UncoveredLine, } from '../types'; +import type { CoverageProvider } from '../providers/coverage-provider'; -import { pathsMatch } from './cobertura-parser'; +import { parseCoberturaXml, pathsMatch } from './cobertura-parser'; +import { computeDiffCoverageFromGit } from './git-diff-coverage'; export function getEffectiveCoverage( diffCoverage: number | null, @@ -78,6 +80,17 @@ export function selectFilesForGeneration( const thresholded = applyCoverageThreshold(report, thresholdPercent); if (thresholded.filesWithPoorCoverage.length === 0) { + const effective = getEffectiveCoverage( + report.diffCoveragePercent, + report.totalCoveragePercent, + ); + if ( + sourceFiles.length > 0 && + report.fileCoverage.length === 0 && + !meetsThreshold(effective, thresholdPercent) + ) { + return sourceFiles; + } return []; } @@ -88,6 +101,38 @@ export function selectFilesForGeneration( ); } +/** Prefer diff-cover when Cobertura has data; fall back to git diff when it does not. */ +export async function resolveDiffCoverageReport(params: { + coverageXmlPath: string; + repoDir: string; + compareRef: string; + headBranch: string; + targetFiles: string[]; + thresholdPercent: number; + coverageProvider: CoverageProvider; +}): Promise { + const cobertura = params.coverageXmlPath + ? await parseCoberturaXml(params.coverageXmlPath) + : { totalCoveragePercent: 0, files: [] }; + + if (cobertura.files.length === 0 && params.targetFiles.length > 0) { + return computeDiffCoverageFromGit( + params.coverageXmlPath, + params.repoDir, + params.compareRef, + params.headBranch, + params.targetFiles, + params.thresholdPercent, + ); + } + + return params.coverageProvider.runDiffCoverage( + params.coverageXmlPath, + params.compareRef, + params.repoDir, + ); +} + function effectiveFileCoverage(entry: FileCoverage | undefined): number { if (!entry) return 0; return entry.diffCoveragePercent ?? entry.lineCoveragePercent; @@ -314,6 +359,8 @@ export function buildWorkflowSummary(params: { coverageBefore: BaselineMetrics; coverageAfter: BaselineMetrics; blockers: CoverageBlockerEntry[]; + optimizationIterations?: CoverageWorkflowSummary['optimizationIterations']; + stopReason?: string | null; }): CoverageWorkflowSummary { const testsPassing = params.generatedTests.filter((t) => t.passed === true) .length; @@ -334,5 +381,7 @@ export function buildWorkflowSummary(params: { params.coverageAfter.diffCoverage, params.coverageAfter.overallCoverage, ), + optimizationIterations: params.optimizationIterations, + stopReason: params.stopReason, }; } diff --git a/coverage-service/lib/src/coverage/source-file-classifier.ts b/coverage-service/lib/src/coverage/source-file-classifier.ts new file mode 100644 index 0000000..9c257da --- /dev/null +++ b/coverage-service/lib/src/coverage/source-file-classifier.ts @@ -0,0 +1,36 @@ +/** Files exporting constants, prompts, schemas, or config should never be skipped. */ +export function isConfigOrPromptExportFile(source: string): boolean { + const patterns = [ + /export\s+(?:const|let)\s+\w*(?:PROMPT|SCHEMA|CONFIG|CONSTANTS?)\w*\s*=/i, + /export\s+(?:const|let)\s+(?:SYSTEM_|USER_|DEFAULT_)\w+/i, + /export\s+(?:const|let)\s+\w+\s*=\s*[\[{`'"]/, + /export\s+(?:const|let)\s+\w+\s*:\s*(?:z\.|Schema|Record|string\[\])/i, + /export\s+default\s+\{[\s\S]*(?:prompt|schema|config)/i, + ]; + return patterns.some((p) => p.test(source)); +} + +/** Service files with retry logic, LLM calls, HTTP clients, etc. need richer test context. */ +export function isComplexServiceFile(source: string): boolean { + const patterns = [ + /\b(retry|retries|backoff|exponential)\b/i, + /\b(openai|anthropic|llm|chat\.completions|generateText)\b/i, + /\b(fetch\s*\(|axios|httpx|requests\.|HttpClient|http\.get|http\.post)\b/i, + /\b(pagination|paginate|nextPage|cursor|offset|limit)\b/i, + /\b(chunk|batch|splitIntoChunks)\b/i, + /\bProvider\b|\bAbstractProvider\b|\bcreateClient\b/, + ]; + return patterns.filter((p) => p.test(source)).length >= 2; +} + +/** Suggest smoke test exports for config/prompt files. */ +export function suggestSmokeTestExports(source: string): string[] { + const exports: string[] = []; + const constExports = source.matchAll( + /export\s+(?:const|let)\s+(\w+)/g, + ); + for (const match of constExports) { + exports.push(match[1]); + } + return exports.slice(0, 8); +} diff --git a/coverage-service/lib/src/index.ts b/coverage-service/lib/src/index.ts index f605b6b..289c3c9 100644 --- a/coverage-service/lib/src/index.ts +++ b/coverage-service/lib/src/index.ts @@ -3,6 +3,9 @@ export * from './coverage/cobertura-parser'; export * from './coverage/git-diff-coverage'; export * from './coverage/coverage-config'; export * from './coverage/coverage-workflow'; +export * from './coverage/coverage-gap-analysis'; +export * from './coverage/coverage-priority'; +export * from './coverage/source-file-classifier'; export * from './providers/coverage-provider'; export * from './providers/diff-cover-provider'; export * from './providers/covpeek-provider'; diff --git a/coverage-service/lib/src/prompts/test-generation.ts b/coverage-service/lib/src/prompts/test-generation.ts index f72c40b..7f4ab0c 100644 --- a/coverage-service/lib/src/prompts/test-generation.ts +++ b/coverage-service/lib/src/prompts/test-generation.ts @@ -1,5 +1,5 @@ import { inferSourceImportPath, inferTestFilePath, sourceFileExtension } from '../test-paths'; -import type { TestGenerationContext } from '../types'; +import { GenerationMode, type TestGenerationContext } from '../types'; function formatRepoPackages(packages: string[]): string { if (packages.length === 0) return '(none detected)'; @@ -46,6 +46,29 @@ function formatTestCountGuidance(ctx: TestGenerationContext): string { function formatTestOutputMode(ctx: TestGenerationContext): string { const path = outputTestPath(ctx); const countGuidance = formatTestCountGuidance(ctx); + + if (ctx.generationMode === GenerationMode.COVERAGE_GAP) { + return `## Mode: COVERAGE GAP — append incremental tests +- Output path: \`${path}\` +- DO NOT rewrite existing tests in Existing Tests or Previous Generated Tests. +- DO NOT duplicate test names or behaviors already present. +- Generate ONLY additional test cases that execute these uncovered diff lines: ${ctx.uncoveredLines} +- Reuse existing mocks and fixtures from Existing Tests. +- Return the **complete updated test file** (all existing + new tests only).`; + } + + if (ctx.isConfigExportFile) { + return `## Mode: CONFIG / PROMPT EXPORT smoke tests +- Output path: \`${path}\` +- This file exports constants, prompts, schemas, or configuration — never skip it. +- Write lightweight smoke tests that import exported symbols and assert basic invariants: + - String prompts contain expected keywords (e.g. expect(SYSTEM_PROMPT).toContain("extract")) + - Schemas/objects have expected keys or non-zero length + - Constants are defined and non-empty +- Suggested exports to test: ${ctx.smokeTestExports?.join(', ') || '(detect from source)'} +- Do NOT call external APIs.`; + } + if (ctx.isUpdatingExistingTest) { return `## Mode: UPDATE existing test file - Output path: \`${path}\` (this file already exists — do NOT create a new file elsewhere). @@ -59,6 +82,39 @@ ${countGuidance} ${countGuidance}`; } +function formatComplexServiceSection(ctx: TestGenerationContext): string { + if (!ctx.isComplexServiceFile) return ''; + + return `## Complex service file (mandatory) +This file contains retry logic, provider abstractions, LLM calls, HTTP clients, pagination, or chunking. + +Requirements: +- Mock ALL external services — never call real APIs or LLMs. +- Verify retry/backoff behavior with controlled mock failures. +- Verify pagination/chunking processes all items. +- Verify fallback logic when primary provider fails. +- Include dependency interfaces and mock at the import boundary used by production code. +${ctx.similarTestExamples ? `\nSimilar repository tests (follow these patterns):\n${ctx.similarTestExamples}` : ''} + +`; +} + +function formatPreviousGeneratedSection(ctx: TestGenerationContext): string { + if (!ctx.previousGeneratedTests?.trim()) return ''; + return `Previous Generated Tests (do NOT duplicate): +${ctx.previousGeneratedTests} + +`; +} + +function formatCoverageReportSection(ctx: TestGenerationContext): string { + if (!ctx.coverageReport?.trim()) return ''; + return `Coverage Report (this file): +${ctx.coverageReport} + +`; +} + function buildPythonPrompt(ctx: TestGenerationContext, symbols: string): string { return `You are writing Python unit tests for ONE production file. Tests must pass on first run with no manual fixes. @@ -67,7 +123,7 @@ function buildPythonPrompt(ctx: TestGenerationContext, symbols: string): string ${formatTestOutputMode(ctx)} - Test the file: ${ctx.file} -${formatFullSourceHeader(ctx)}## Context +${formatFullSourceHeader(ctx)}${formatComplexServiceSection(ctx)}## Context Repository Language: ${ctx.language} Testing Framework: ${ctx.framework} @@ -84,7 +140,7 @@ ${ctx.source} Existing Tests: ${ctx.existingTests || '(none found)'} -Uncovered Changed Lines: +${formatPreviousGeneratedSection(ctx)}${formatCoverageReportSection(ctx)}Uncovered Changed Lines: ${ctx.uncoveredLines} All Symbols in File: @@ -206,7 +262,7 @@ Do NOT use @jest/globals, jest, vitest, mocha, describe, it, expect, or jest.fn. ${formatTestOutputMode(ctx)} - Production file: ${ctx.file} -${formatFullSourceHeader(ctx)}## Context +${formatFullSourceHeader(ctx)}${formatComplexServiceSection(ctx)}## Context Repository Language: ${ctx.language} Testing Framework: ${ctx.framework} @@ -226,7 +282,7 @@ ${formatExportedSymbols(ctx)} Existing Tests: ${ctx.existingTests || '(none found)'} -Uncovered Changed Lines: +${formatPreviousGeneratedSection(ctx)}${formatCoverageReportSection(ctx)}Uncovered Changed Lines: ${ctx.uncoveredLines} All Symbols in File: @@ -296,7 +352,7 @@ function buildRepairSection(ctx: TestGenerationContext): string { return ` ## Repair (attempt ${ctx.attemptNumber ?? 2}) -The previous generated test failed when executed. Fix the test file so it compiles, runs, and passes. +The generated test failed when executed. Modify ONLY the failing test — do not rewrite unrelated tests. Failure output: ${ctx.failureLogs} diff --git a/coverage-service/lib/src/types/index.ts b/coverage-service/lib/src/types/index.ts index 421c1ca..bf1d7d7 100644 --- a/coverage-service/lib/src/types/index.ts +++ b/coverage-service/lib/src/types/index.ts @@ -60,7 +60,18 @@ export interface CoverageBlockerEntry { export type CoverageWorkflowStatus = | 'threshold_met' | 'success' - | 'threshold_not_reached'; + | 'threshold_not_reached' + | 'plateau_reached'; + +export interface CoverageIterationSummary { + iteration: number; + coverageBefore: number; + coverageAfter: number | null; + coverageGain: number | null; + generatedTests: number; + failedTests: number; + stopReason?: string | null; +} export interface CoverageWorkflowSummary { status: CoverageWorkflowStatus; @@ -75,6 +86,13 @@ export interface CoverageWorkflowSummary { blockers: CoverageBlockerEntry[]; targetCoverage: number; currentCoverage: number; + optimizationIterations?: CoverageIterationSummary[]; + stopReason?: string | null; +} + +export enum GenerationMode { + NEW_TEST_FILE = 'NEW_TEST_FILE', + COVERAGE_GAP = 'COVERAGE_GAP', } export interface FileCoverage { @@ -124,6 +142,20 @@ export interface TestGenerationContext { testOutputPath?: string; /** True when updating an existing test file instead of creating a new one. */ isUpdatingExistingTest?: boolean; + /** Generation mode: full file or gap-targeted incremental tests. */ + generationMode?: GenerationMode; + /** Previously generated test content for this target (avoid duplication). */ + previousGeneratedTests?: string; + /** Coverage report excerpt for this file. */ + coverageReport?: string; + /** True for config/prompt/schema export files requiring smoke tests. */ + isConfigExportFile?: boolean; + /** True for complex service files (retry, LLM, HTTP, pagination). */ + isComplexServiceFile?: boolean; + /** Similar test examples from the repository. */ + similarTestExamples?: string; + /** Suggested export names for smoke tests. */ + smokeTestExports?: string[]; } export interface PrAnalysisJobData { diff --git a/coverage-service/worker/src/lib/coverage-optimization-service.ts b/coverage-service/worker/src/lib/coverage-optimization-service.ts new file mode 100644 index 0000000..b16a60b --- /dev/null +++ b/coverage-service/worker/src/lib/coverage-optimization-service.ts @@ -0,0 +1,423 @@ +import type { + ChangedFile, + DiffCoverageReport, + GeneratedTest, +} from '@openreview/coverage-lib'; +import type { OptimizationStopReason } from '@openreview/coverage-lib'; +import { + extractBaselineMetrics, + formatGapSummary, + gapsToChangedFiles, + getCoverageGaps, + getEffectiveCoverage, + getMaxFilesPerIteration, + getMaxOptimizationIterations, + getMinCoverageGain, + getTargetDiffCoveragePercent, + meetsThreshold, + pathsMatch, + prioritizeCoverageGaps, + shouldStopForStagnation, +} from '@openreview/coverage-lib'; + +import { prisma } from './prisma'; + +export interface OptimizationRunContext { + prRunId: string; + runDir: string; + sourceFiles: ChangedFile[]; + sourcePaths: string[]; + baseRef: string; + headBranch: string; + testCommand: string; + repoPackages: string[]; + useCoveragePackageOnly: boolean; + useAutoJsCoverage: boolean; + pythonTestPaths: string[]; + jsTestPaths: string[]; + hasLlm: boolean; +} + +export interface GenerateTestOutcome { + test: GeneratedTest | null; + passed: boolean; + attempts: number; + declaredDeps: string[]; + repairAttempts: number; + failureReason?: string; +} + +export interface OptimizationCallbacks { + log: (level: string, message: string) => Promise; + updateStatus: (status: string) => Promise; + runCoverage: ( + coverageCommand: string, + report: DiffCoverageReport, + ) => Promise<{ report: DiffCoverageReport; coverageXml: string }>; + buildPostCoverageCommand: (passingTestPaths: string[]) => string; + generateTestForFile: ( + file: ChangedFile, + report: DiffCoverageReport, + options: { + generationMode: 'NEW_TEST_FILE' | 'COVERAGE_GAP'; + iterationNumber: number; + previousGeneratedContent?: string; + }, + ) => Promise; + getSourceLineCount: (filePath: string) => Promise; + isConfigExportFile: (filePath: string) => Promise; + installTestDeps: ( + tests: GeneratedTest[], + declaredDeps: string[], + ) => Promise; +} + +export interface OptimizationResult { + afterReport: DiffCoverageReport; + generatedTests: GeneratedTest[]; + generatedTestResults: { filePath: string; passed: boolean | null }[]; + totalGenerationAttempts: number; + stopReason: OptimizationStopReason | null; + iterationSummaries: Array<{ + iteration: number; + coverageBefore: number; + coverageAfter: number | null; + coverageGain: number | null; + generatedTests: number; + failedTests: number; + stopReason?: string | null; + }>; +} + +export class CoverageOptimizationService { + private readonly targetPercent = getTargetDiffCoveragePercent(); + private readonly maxIterations = getMaxOptimizationIterations(); + private readonly minGain = getMinCoverageGain(); + private readonly maxFilesPerIteration = getMaxFilesPerIteration(); + + async run( + ctx: OptimizationRunContext, + beforeReport: DiffCoverageReport, + callbacks: OptimizationCallbacks, + ): Promise { + const baselineMetrics = extractBaselineMetrics(beforeReport); + let currentReport = beforeReport; + let currentCoverage = getEffectiveCoverage( + baselineMetrics.diffCoverage, + baselineMetrics.overallCoverage, + ); + + const generatedTests: GeneratedTest[] = []; + const generatedTestResults: { filePath: string; passed: boolean | null }[] = + []; + const failureCounts = new Map(); + const sourceLineCounts = new Map(); + const recentGains: number[] = []; + const iterationSummaries: OptimizationResult['iterationSummaries'] = []; + let totalGenerationAttempts = 0; + let stopReason: OptimizationStopReason | null = null; + let iteration = 0; + + if (!ctx.hasLlm) { + return { + afterReport: currentReport, + generatedTests, + generatedTestResults, + totalGenerationAttempts: 0, + stopReason: 'no_llm', + iterationSummaries, + }; + } + + while ( + !meetsThreshold(currentCoverage, this.targetPercent) && + iteration < this.maxIterations + ) { + const gapsReport = getCoverageGaps(currentReport, this.targetPercent); + + // Include config/prompt export files even when baseline import coverage looks complete + const configExportFiles: ChangedFile[] = []; + for (const file of ctx.sourceFiles) { + if (await callbacks.isConfigExportFile(file.path)) { + const inGaps = gapsReport.files.some((g) => + pathsMatch(g.file, file.path), + ); + if (!inGaps) { + gapsReport.files.push({ + file: file.path, + coverage: 100, + uncoveredLines: [], + uncoveredPercentage: 0, + complexity: 0, + }); + configExportFiles.push(file); + } + } + } + + if ( + gapsReport.totalUncoveredLines === 0 && + configExportFiles.length === 0 + ) { + stopReason = 'no_gaps'; + break; + } + + for (const gap of gapsReport.files) { + if (!sourceLineCounts.has(gap.file)) { + sourceLineCounts.set( + gap.file, + await callbacks.getSourceLineCount(gap.file), + ); + } + } + + const prioritized = prioritizeCoverageGaps(gapsReport.files, { + failureCounts, + sourceLineCounts, + }); + + let filesToProcess = gapsToChangedFiles(prioritized, ctx.sourceFiles); + for (const cf of configExportFiles) { + if (!filesToProcess.some((f) => pathsMatch(f.path, cf.path))) { + filesToProcess.push(cf); + } + } + filesToProcess = filesToProcess.slice(0, this.maxFilesPerIteration); + + if (filesToProcess.length === 0) { + stopReason = 'no_gaps'; + break; + } + + const coverageBefore = currentCoverage; + const iterationRecord = await prisma.coverageIteration.create({ + data: { + prRunId: ctx.prRunId, + iteration, + coverageBefore, + }, + }); + + for (const gap of prioritized.slice(0, this.maxFilesPerIteration)) { + await prisma.coverageGap.create({ + data: { + iterationId: iterationRecord.id, + filePath: gap.file, + coverage: gap.coverage, + missingLines: gap.uncoveredLines, + priority: gap.priority ?? 0, + }, + }); + } + + const selectedNames = filesToProcess.map((f) => f.path).join(', '); + await callbacks.log( + 'info', + [ + `Iteration ${iteration}`, + `Coverage Before: ${coverageBefore.toFixed(1)}%`, + `Target: ${this.targetPercent}%`, + `Files Selected: ${selectedNames}`, + `Missing Lines:`, + formatGapSummary( + prioritized.filter((g) => + filesToProcess.some((f) => pathsMatch(f.path, g.file)), + ), + ), + ].join('\n'), + ); + + await callbacks.updateStatus('GENERATING_TESTS'); + + let iterGenerated = 0; + let iterFailed = 0; + const iterDeclaredDeps: string[] = []; + + for (const file of filesToProcess) { + const gap = prioritized.find((g) => pathsMatch(g.file, file.path)); + const hasPriorArtifact = generatedTests.some((t) => + pathsMatch(t.targetFile, file.path), + ); + const existingArtifact = await prisma.generatedTestArtifact.findFirst({ + where: { prRunId: ctx.prRunId, targetFile: file.path }, + orderBy: { createdAt: 'desc' }, + }); + + const generationMode = + hasPriorArtifact || existingArtifact + ? 'COVERAGE_GAP' + : 'NEW_TEST_FILE'; + + await callbacks.log( + 'info', + `Generating tests for ${file.path} (mode: ${generationMode}, diff coverage ${gap?.coverage?.toFixed(1) ?? 'n/a'}%, uncovered: ${gap?.uncoveredLines.join(',') ?? 'config export'})`, + ); + + try { + const outcome = await callbacks.generateTestForFile( + file, + currentReport, + { + generationMode, + iterationNumber: iteration, + previousGeneratedContent: existingArtifact?.content, + }, + ); + + totalGenerationAttempts += outcome.attempts; + iterDeclaredDeps.push(...outcome.declaredDeps); + + if (outcome.test) { + iterGenerated++; + const existingIdx = generatedTests.findIndex( + (t) => t.targetFile === outcome.test!.targetFile, + ); + if (existingIdx >= 0) { + generatedTests[existingIdx] = outcome.test; + } else { + generatedTests.push(outcome.test); + } + + const resultIdx = generatedTestResults.findIndex( + (r) => r.filePath === outcome.test!.filePath, + ); + const resultEntry = { + filePath: outcome.test.filePath, + passed: outcome.passed, + }; + if (resultIdx >= 0) { + generatedTestResults[resultIdx] = resultEntry; + } else { + generatedTestResults.push(resultEntry); + } + + if (!outcome.passed) { + iterFailed++; + failureCounts.set( + file.path, + (failureCounts.get(file.path) ?? 0) + 1, + ); + } + } else { + iterFailed++; + failureCounts.set( + file.path, + (failureCounts.get(file.path) ?? 0) + 1, + ); + } + } catch (err) { + iterFailed++; + failureCounts.set( + file.path, + (failureCounts.get(file.path) ?? 0) + 1, + ); + await callbacks.log( + 'warn', + `Test generation failed for ${file.path}: ${(err as Error).message}`, + ); + } + } + + await callbacks.log( + 'info', + `Generated Tests: ${iterGenerated}\nPassing: ${iterGenerated - iterFailed}\nFailed: ${iterFailed}`, + ); + + const passingPaths = generatedTestResults + .filter((t) => t.passed) + .map((t) => t.filePath); + + if (passingPaths.length > 0) { + await callbacks.updateStatus('RECALCULATING'); + await callbacks.log( + 'info', + 'Recalculating coverage after generated tests (passing tests only)', + ); + + if (iterDeclaredDeps.length > 0) { + await callbacks.installTestDeps(generatedTests, iterDeclaredDeps); + } + + const postCommand = callbacks.buildPostCoverageCommand(passingPaths); + const postResult = await callbacks.runCoverage( + postCommand, + currentReport, + ); + currentReport = postResult.report; + currentCoverage = getEffectiveCoverage( + extractBaselineMetrics(currentReport).diffCoverage, + extractBaselineMetrics(currentReport).overallCoverage, + ); + } + + const coverageGain = currentCoverage - coverageBefore; + recentGains.push(coverageGain); + + await prisma.coverageIteration.update({ + where: { id: iterationRecord.id }, + data: { + coverageAfter: currentCoverage, + coverageGain, + generatedTests: iterGenerated, + failedTests: iterFailed, + }, + }); + + iterationSummaries.push({ + iteration, + coverageBefore, + coverageAfter: currentCoverage, + coverageGain, + generatedTests: iterGenerated, + failedTests: iterFailed, + }); + + await callbacks.log( + 'info', + `Coverage After: ${currentCoverage.toFixed(1)}%\nGain: ${coverageGain >= 0 ? '+' : ''}${coverageGain.toFixed(1)}%`, + ); + + if (meetsThreshold(currentCoverage, this.targetPercent)) { + stopReason = 'target_reached'; + await prisma.coverageIteration.update({ + where: { id: iterationRecord.id }, + data: { stopReason: 'target_reached' }, + }); + break; + } + + if (shouldStopForStagnation(recentGains, this.minGain)) { + stopReason = 'plateau'; + await prisma.coverageIteration.update({ + where: { id: iterationRecord.id }, + data: { stopReason: 'plateau' }, + }); + await callbacks.log( + 'info', + 'Coverage plateau reached — stopping optimization', + ); + break; + } + + iteration++; + } + + if ( + !stopReason && + iteration >= this.maxIterations && + !meetsThreshold(currentCoverage, this.targetPercent) + ) { + stopReason = 'max_iterations'; + } + + return { + afterReport: currentReport, + generatedTests, + generatedTestResults, + totalGenerationAttempts, + stopReason, + iterationSummaries, + }; + } +} diff --git a/coverage-service/worker/src/lib/js-coverage.ts b/coverage-service/worker/src/lib/js-coverage.ts index 8ddf692..5c466ab 100644 --- a/coverage-service/worker/src/lib/js-coverage.ts +++ b/coverage-service/worker/src/lib/js-coverage.ts @@ -1,4 +1,4 @@ -import { existsSync, readFileSync, readdirSync, statSync } from 'fs'; +import { existsSync, readFileSync, readdirSync, statSync, writeFileSync } from 'fs'; import { join, relative } from 'path'; import { sourceFileExtension, type ChangedFile, type RepositoryProvider } from '@openreview/coverage-lib'; @@ -30,6 +30,31 @@ function normalizePath(p: string): string { return p.replace(/\\/g, '/'); } +/** Escape glob metacharacters so c8/node --test match literal paths (e.g. [matchId] routes). */ +export function escapeGlobPathForC8(path: string): string { + return normalizePath(path) + .split('/') + .map((segment) => + [...segment] + .map((char) => (char === '[' ? '[[]' : char === ']' ? '[]]' : char)) + .join(''), + ) + .join('/'); +} + +/** Write tsx/CSS loader hooks before running coverage or tests (avoids sh -c setup chains). */ +export function prepareJsTestHarness( + repoDir: string, + sourcePaths: string[], + testPaths: string[], +): void { + const runtimePaths = [...testPaths, ...sourcePaths]; + if (!needsRuntimeRegister(runtimePaths)) return; + + writeFileSync(join(repoDir, TEST_REGISTER_PATH), TEST_REGISTER_CONTENT); + writeFileSync(join(repoDir, TEST_LOADER_PATH), TEST_LOADER_CONTENT); +} + function isTestFile(path: string): boolean { const base = path.split('/').pop() ?? path; return ( @@ -61,13 +86,8 @@ function needsRuntimeRegister(paths: string[]): boolean { return needsJsTranspileLoader(paths); } -function runtimeRegisterSetupCommand(paths: string[]): string | null { - if (!needsRuntimeRegister(paths)) return null; - const body = [ - `require('fs').writeFileSync(${JSON.stringify(TEST_REGISTER_PATH)}, ${JSON.stringify(TEST_REGISTER_CONTENT)})`, - `require('fs').writeFileSync(${JSON.stringify(TEST_LOADER_PATH)}, ${JSON.stringify(TEST_LOADER_CONTENT)})`, - ].join(';'); - return `node -e ${shellQuote(body)}`; +function runtimeRegisterSetupCommand(_paths: string[]): string | null { + return null; } function nodeLoaderPrefix(paths: string[]): string { @@ -127,7 +147,7 @@ function buildRunTarget( const loader = nodeLoaderPrefix(runtimePaths); return withRuntimeRegister( runtimePaths, - `node ${loader}--test ${testPaths.map(shellQuote).join(' ')}`, + `node ${loader}--test ${testPaths.map((p) => shellQuote(escapeGlobPathForC8(p))).join(' ')}`, ); } @@ -140,7 +160,9 @@ function buildRunTarget( function coverageToolPrefix(sourcePaths: string[]): string { const includes = sourcePaths.length > 0 - ? sourcePaths.map((p) => `--include=${shellQuote(normalizePath(p))}`) + ? sourcePaths.map( + (p) => `--include=${shellQuote(escapeGlobPathForC8(p))}`, + ) : ["--include='**/*'"]; // c8 uses V8 native coverage and works with ESM; nyc/istanbul often reports 0% for "type":"module" repos. diff --git a/coverage-service/worker/src/lib/repo-packages.ts b/coverage-service/worker/src/lib/repo-packages.ts index e1224ff..9224882 100644 --- a/coverage-service/worker/src/lib/repo-packages.ts +++ b/coverage-service/worker/src/lib/repo-packages.ts @@ -2,6 +2,8 @@ import { existsSync } from 'fs'; import { readFile } from 'fs/promises'; import { join } from 'path'; +import { findJsPackageRoots } from './repo-setup.js'; + const PYTHON_STDLIB = new Set([ 'abc', 'argparse', @@ -91,8 +93,8 @@ async function collectFromPyproject(repoDir: string): Promise { return packages; } -async function collectFromPackageJson(repoDir: string): Promise { - const path = join(repoDir, 'package.json'); +async function collectFromPackageJson(packageDir: string): Promise { + const path = join(packageDir, 'package.json'); if (!existsSync(path)) return []; const pkg = JSON.parse(await readFile(path, 'utf-8')) as { @@ -122,7 +124,13 @@ export async function collectRepoPackages(repoDir: string): Promise { for (const source of [ await collectFromRequirements(repoDir), await collectFromPyproject(repoDir), - await collectFromPackageJson(repoDir), + ...(await Promise.all( + findJsPackageRoots(repoDir).map((root) => + collectFromPackageJson( + root === '.' ? repoDir : join(repoDir, root), + ), + ), + )), ]) { for (const pkg of source) add(pkg); } diff --git a/coverage-service/worker/src/lib/repo-setup.ts b/coverage-service/worker/src/lib/repo-setup.ts index f8dad57..a47f4e1 100644 --- a/coverage-service/worker/src/lib/repo-setup.ts +++ b/coverage-service/worker/src/lib/repo-setup.ts @@ -9,14 +9,93 @@ import { wrapWithVenvActivate, } from './python-venv'; +const JS_PACKAGE_DIR_CANDIDATES = [ + '.', + 'backend', + 'frontend', + 'web', + 'client', + 'server', + 'app', + 'packages/web', + 'packages/app', +]; + export interface RepoSetup { isPython: boolean; isJavaScript: boolean; + /** Repo-relative path containing package.json (e.g. "backend"), or "." at root. */ + jsPackageRoot: string | null; venvDir: string | null; installCommand: string | null; wrapCommand: (command: string) => string; } +export function hasJsSourcePaths(paths: string[]): boolean { + return paths.some((p) => /\.(ts|tsx|js|jsx|mjs|cjs)$/i.test(p)); +} + +export function findJsPackageRoot(repoDir: string): string | null { + for (const sub of JS_PACKAGE_DIR_CANDIDATES) { + const dir = sub === '.' ? repoDir : join(repoDir, sub); + if (existsSync(join(dir, 'package.json'))) { + return sub; + } + } + return null; +} + +export function findJsPackageRoots(repoDir: string): string[] { + const roots: string[] = []; + for (const sub of JS_PACKAGE_DIR_CANDIDATES) { + const dir = sub === '.' ? repoDir : join(repoDir, sub); + if (existsSync(join(dir, 'package.json'))) { + roots.push(sub); + } + } + return roots; +} + +export function defaultJsToolsInstallCommand(): string { + return 'npm install --no-save --legacy-peer-deps c8 check-code-coverage tsx'; +} + +function buildJsInstallCommand(repoDir: string, jsPackageRoot: string): string { + const jsCoverageDeps = 'c8 check-code-coverage tsx'; + const pkgDir = jsPackageRoot === '.' ? repoDir : join(repoDir, jsPackageRoot); + const hasPackageLock = existsSync(join(pkgDir, 'package-lock.json')); + const hasYarnLock = existsSync(join(pkgDir, 'yarn.lock')); + const hasPnpmLock = existsSync(join(pkgDir, 'pnpm-lock.yaml')); + + let pkgInstall: string; + if (jsPackageRoot === '.') { + if (hasPnpmLock) pkgInstall = 'pnpm install --frozen-lockfile'; + else if (hasYarnLock) pkgInstall = 'yarn install --frozen-lockfile'; + else if (hasPackageLock) pkgInstall = 'npm ci'; + else pkgInstall = 'npm install'; + return `${pkgInstall} && npm install --no-save --legacy-peer-deps ${jsCoverageDeps}`; + } + + const cdPrefix = `(cd ${jsPackageRoot} && `; + if (hasPnpmLock) pkgInstall = `${cdPrefix}pnpm install --frozen-lockfile)`; + else if (hasYarnLock) pkgInstall = `${cdPrefix}yarn install --frozen-lockfile)`; + else if (hasPackageLock) pkgInstall = `${cdPrefix}npm ci)`; + else pkgInstall = `${cdPrefix}npm install)`; + + return `${pkgInstall} && npm install --no-save --legacy-peer-deps ${jsCoverageDeps}`; +} + +function buildJsRepoSetup(repoDir: string, jsPackageRoot: string): RepoSetup { + return { + isPython: false, + isJavaScript: true, + jsPackageRoot, + venvDir: null, + installCommand: buildJsInstallCommand(repoDir, jsPackageRoot), + wrapCommand: (cmd) => cmd, + }; +} + export async function setupPythonRepo( repoDir: string, setup: RepoSetup, @@ -45,9 +124,11 @@ export function detectRepoSetup(repoDir: string): RepoSetup { const coverageDeps = 'coverage pytest pytest-asyncio'; const jsCoverageDeps = 'c8 check-code-coverage tsx'; + const jsPackageRoot = findJsPackageRoot(repoDir); const noVenv: RepoSetup = { isPython: false, isJavaScript: false, + jsPackageRoot: null, venvDir: null, installCommand: null, wrapCommand: (cmd) => cmd, @@ -56,7 +137,8 @@ export function detectRepoSetup(repoDir: string): RepoSetup { if (hasUvLock) { return { isPython: true, - isJavaScript: false, + isJavaScript: jsPackageRoot !== null, + jsPackageRoot, venvDir: null, installCommand: [ `UV_PROJECT_ENVIRONMENT=${PR_COVERAGE_VENV} uv sync --all-groups --all-extras`, @@ -69,7 +151,8 @@ export function detectRepoSetup(repoDir: string): RepoSetup { if (hasPoetryLock) { return { isPython: true, - isJavaScript: false, + isJavaScript: jsPackageRoot !== null, + jsPackageRoot, venvDir: null, installCommand: [ 'POETRY_VIRTUALENVS_CREATE=false poetry install --no-interaction', @@ -90,38 +173,31 @@ export function detectRepoSetup(repoDir: string): RepoSetup { return { isPython: true, - isJavaScript: false, + isJavaScript: jsPackageRoot !== null, + jsPackageRoot, venvDir: null, installCommand: installParts.join(' && '), wrapCommand: (cmd) => cmd, }; } - const jsInstallBase = hasPackageLock ? 'npm ci' : 'npm install'; - const jsInstallCommand = `${jsInstallBase} && npm install --no-save --legacy-peer-deps ${jsCoverageDeps}`; - if (hasPnpmLock) { - return { - ...noVenv, - isJavaScript: true, - installCommand: `pnpm install --frozen-lockfile && pnpm add -D ${jsCoverageDeps}`, - }; + return buildJsRepoSetup(repoDir, '.'); } if (hasYarnLock) { return { - ...noVenv, - isJavaScript: true, + ...buildJsRepoSetup(repoDir, '.'), installCommand: `yarn install --frozen-lockfile && yarn add -D ${jsCoverageDeps}`, }; } if (hasPackageLock || hasPackageJson) { - return { - ...noVenv, - isJavaScript: true, - installCommand: jsInstallCommand, - }; + return buildJsRepoSetup(repoDir, '.'); + } + + if (jsPackageRoot) { + return buildJsRepoSetup(repoDir, jsPackageRoot); } return noVenv; diff --git a/coverage-service/worker/src/lib/test-repair-service.ts b/coverage-service/worker/src/lib/test-repair-service.ts new file mode 100644 index 0000000..60853f2 --- /dev/null +++ b/coverage-service/worker/src/lib/test-repair-service.ts @@ -0,0 +1,79 @@ +import type { GeneratedTestWithUsage } from '@openreview/coverage-lib'; +import { + getMaxRepairAttempts, + isRepairLoopEnabled, +} from '@openreview/coverage-lib'; + +export interface TestRepairParams { + maxAttempts?: number; + runTest: () => Promise<{ passed: boolean; output: string }>; + repair: (failureLogs: string, previousContent: string, attempt: number) => Promise; + writeTest: (content: string) => Promise; + log: (message: string) => Promise; +} + +export interface TestRepairResult { + passed: boolean; + repairAttempts: number; + failureReason?: string; + finalContent?: string; +} + +/** + * Repair loop for failing generated tests. + * Sends failure output back to the LLM up to MAX_REPAIR_ATTEMPTS times. + */ +export class TestRepairService { + async repairUntilPassing(params: TestRepairParams): Promise { + if (!isRepairLoopEnabled()) { + const result = await params.runTest(); + return { + passed: result.passed, + repairAttempts: 0, + failureReason: result.passed ? undefined : result.output.slice(-2000), + }; + } + + const maxAttempts = params.maxAttempts ?? getMaxRepairAttempts(); + let repairAttempts = 0; + let lastOutput = ''; + let previousContent = ''; + + while (repairAttempts < maxAttempts) { + const result = await params.runTest(); + if (result.passed) { + return { passed: true, repairAttempts, finalContent: previousContent || undefined }; + } + + lastOutput = result.output; + repairAttempts++; + + await params.log( + `Repair attempt ${repairAttempts}/${maxAttempts}`, + ); + + const repaired = await params.repair( + lastOutput.slice(-4000), + previousContent, + repairAttempts + 1, + ); + + if (!repaired?.content) { + break; + } + + previousContent = repaired.content; + await params.writeTest(repaired.content); + } + + const finalResult = await params.runTest(); + return { + passed: finalResult.passed, + repairAttempts, + failureReason: finalResult.passed + ? undefined + : (finalResult.output || lastOutput).slice(-2000), + finalContent: previousContent || undefined, + }; + } +} diff --git a/coverage-service/worker/src/processors/pr-analysis.processor.ts b/coverage-service/worker/src/processors/pr-analysis.processor.ts index 670ef1e..45ea9a8 100644 --- a/coverage-service/worker/src/processors/pr-analysis.processor.ts +++ b/coverage-service/worker/src/processors/pr-analysis.processor.ts @@ -11,7 +11,8 @@ import type { CoverageWorkflowSummary, DiffCoverageReport, LlmUsage, - GitHubProvider} from '@openreview/coverage-lib'; + GitHubProvider, +} from '@openreview/coverage-lib'; import { pathsMatch, detectFramework, @@ -20,16 +21,21 @@ import { extractExportedSymbols, computeDiffCoverageFromGit, parseCoberturaXml, - getTestThresholdPercent, + getTargetDiffCoveragePercent, getMaxGenerationAttempts, + getMaxRepairAttempts, extractBaselineMetrics, getEffectiveCoverage, meetsThreshold, applyCoverageThreshold, - selectFilesForGeneration, + resolveDiffCoverageReport, classifyCoverageBlockers, buildWorkflowSummary, prepareTestFileContext, + isConfigOrPromptExportFile, + isComplexServiceFile, + suggestSmokeTestExports, + GenerationMode, } from '@openreview/coverage-lib'; import type { Prisma } from '@prisma/client'; import type { Job } from 'bullmq'; @@ -38,13 +44,13 @@ import { buildJsCoverageCommand, buildJsTestCommand, collectJsTestPaths, + prepareJsTestHarness, } from '../lib/js-coverage'; import { prisma } from '../lib/prisma'; import { createCoverageProviderFromEnv, createLLMProvider, createRepositoryProvider, - resolveTestGenerationModel, } from '../lib/providers'; import { buildPythonCoverageCommand, @@ -59,16 +65,24 @@ import { parseGeneratedTestContent, } from '../lib/repo-packages'; import type { RepoSetup} from '../lib/repo-setup'; -import { detectRepoSetup, setupPythonRepo } from '../lib/repo-setup'; +import { detectRepoSetup, setupPythonRepo, hasJsSourcePaths, defaultJsToolsInstallCommand } from '../lib/repo-setup'; import { cleanupDir, findCoverageXml, runCommand } from '../lib/shell'; +import { + CoverageOptimizationService, + type GenerateTestOutcome, +} from '../lib/coverage-optimization-service'; +import { TestRepairService } from '../lib/test-repair-service'; export class PrAnalysisProcessor { private readonly repoProvider = createRepositoryProvider(); private readonly coverageProvider = createCoverageProviderFromEnv(); private readonly llmProvider = createLLMProvider(); + private readonly optimizationService = new CoverageOptimizationService(); + private readonly testRepairService = new TestRepairService(); private readonly workDir = process.env.WORK_DIR ?? '/tmp/pr-coverage-runs'; - private readonly testThresholdPercent = getTestThresholdPercent(); + private readonly testThresholdPercent = getTargetDiffCoveragePercent(); private readonly maxGenerationAttempts = getMaxGenerationAttempts(); + private readonly maxRepairAttempts = getMaxRepairAttempts(); private activeJob?: Job; async process( @@ -151,11 +165,46 @@ export class PrAnalysisProcessor { await this.log(data.prRunId, 'info', 'Python virtualenv ready'); } + await this.updateStatus(data.prRunId, 'ANALYZING'); + await this.log(data.prRunId, 'info', 'Analyzing changed files in PR'); + const changedFiles = await this.repoProvider.getChangedFiles( + runDir, + baseRef, + headBranch, + ); + + const sourceFiles = changedFiles.filter( + (f) => + f.status !== 'deleted' && + /\.(ts|tsx|js|jsx|py)$/.test(f.path) && + !f.path.includes('.test.') && + !f.path.includes('.spec.') && + !f.path.startsWith('test_'), + ); + + await this.log( + data.prRunId, + 'info', + `Found ${changedFiles.length} changed file(s), ${sourceFiles.length} source file(s) eligible for analysis`, + ); + + const sourcePaths = sourceFiles.map((f) => f.path); + const hasJsSourceChanges = hasJsSourcePaths(sourcePaths); + const hasPySourceChanges = sourceFiles.some((f) => f.path.endsWith('.py')); + const useAutoJsCoverage = repoSetup.isJavaScript || hasJsSourceChanges; + const useCoveragePackageOnly = + repoSetup.isPython && hasPySourceChanges && !hasJsSourceChanges; + const rawInstallCommand = repository.installCommand?.trim() || repoSetup.installCommand; - const installCommand = rawInstallCommand + let installCommand = rawInstallCommand ? repoSetup.wrapCommand(rawInstallCommand) : null; + if (!installCommand && useAutoJsCoverage) { + installCommand = repoSetup.wrapCommand( + repoSetup.installCommand ?? defaultJsToolsInstallCommand(), + ); + } if (installCommand) { await this.log( data.prRunId, @@ -165,7 +214,7 @@ export class PrAnalysisProcessor { const installResult = await this.withProgressHeartbeat( data.prRunId, 'Installing dependencies', - () => runCommand(installCommand, runDir), + () => runCommand(installCommand!, runDir), ); if (installResult.exitCode !== 0) { const output = `${installResult.stdout}\n${installResult.stderr}`.slice( @@ -187,33 +236,6 @@ export class PrAnalysisProcessor { ); } - await this.updateStatus(data.prRunId, 'ANALYZING'); - await this.log(data.prRunId, 'info', 'Analyzing changed files in PR'); - const changedFiles = await this.repoProvider.getChangedFiles( - runDir, - baseRef, - headBranch, - ); - - const sourceFiles = changedFiles.filter( - (f) => - f.status !== 'deleted' && - /\.(ts|tsx|js|jsx|py)$/.test(f.path) && - !f.path.includes('.test.') && - !f.path.includes('.spec.') && - !f.path.startsWith('test_'), - ); - - await this.log( - data.prRunId, - 'info', - `Found ${changedFiles.length} changed file(s), ${sourceFiles.length} source file(s) eligible for analysis`, - ); - - const sourcePaths = sourceFiles.map((f) => f.path); - const useCoveragePackageOnly = repoSetup.isPython; - const useAutoJsCoverage = repoSetup.isJavaScript; - let pythonTestPaths: string[] = []; let jsTestPaths: string[] = []; let coverageCommand: string; @@ -281,6 +303,10 @@ export class PrAnalysisProcessor { : `Running initial coverage: ${coverageCommand}`, ); + if (useAutoJsCoverage) { + prepareJsTestHarness(runDir, sourcePaths, jsTestPaths); + } + beforeCoverageResult = await this.withProgressHeartbeat( data.prRunId, 'Coverage command running', @@ -357,94 +383,144 @@ export class PrAnalysisProcessor { return; } - const filesNeedingTests = selectFilesForGeneration( - beforeCoverageResult.report, - sourceFiles, - this.testThresholdPercent, - ).slice(0, 10); - - await this.log( - data.prRunId, - 'info', - `${filesNeedingTests.length} file(s) with coverage gaps selected for test generation`, - ); - - const generatedTests: GeneratedTest[] = []; - const generatedTestResults: { filePath: string; passed: boolean | null }[] = - []; - const declaredTestDeps: string[] = []; - let totalGenerationAttempts = 0; - - if (filesNeedingTests.length > 0 && this.hasLlmConfigured()) { - await this.updateStatus(data.prRunId, 'GENERATING_TESTS'); - const llmProviderName = (process.env.LLM_PROVIDER ?? 'openai') as - | 'openai' - | 'anthropic' - | 'local'; + if (this.hasLlmConfigured()) { await this.log( data.prRunId, 'info', - `Test generation: ${llmProviderName} / ${resolveTestGenerationModel(llmProviderName)}`, + `Starting coverage optimization (target: ${this.testThresholdPercent}% diff coverage)`, ); + } - for (const file of filesNeedingTests) { - const fileDiff = - beforeCoverageResult.report.fileCoverage.find((f) => - pathsMatch(f.file, file.path), - )?.diffCoveragePercent ?? null; - await this.log( - data.prRunId, - 'info', - `Generating tests for ${file.path} (diff coverage ${fileDiff ?? 'n/a'}%, full PR branch source)`, - ); - try { - const framework = detectFramework(runDir, detectLanguage(file.path), testCommand); - const testCtx = await prepareTestFileContext( - this.repoProvider, - runDir, - file.path, - framework, + const optimizationResult = await this.optimizationService.run( + { + prRunId: data.prRunId, + runDir, + sourceFiles, + sourcePaths, + baseRef, + headBranch, + testCommand, + repoPackages, + useCoveragePackageOnly, + useAutoJsCoverage, + pythonTestPaths, + jsTestPaths, + hasLlm: this.hasLlmConfigured(), + }, + beforeCoverageResult.report, + { + log: (level, message) => this.log(data.prRunId, level, message), + updateStatus: (status) => + this.updateStatus(data.prRunId, status as PrRunStatus), + runCoverage: async (coverageCommand, _prevReport) => { + const result = await this.withProgressHeartbeat( + data.prRunId, + 'Post-test coverage command running', + () => + this.runCoverage( + data.prRunId, + runDir, + coverageCommand, + baseRef, + headBranch, + sourcePaths, + useCoveragePackageOnly, + logs, + ), + ); + result.report = applyCoverageThreshold( + result.report, + this.testThresholdPercent, ); - if (testCtx.isUpdatingExistingTest) { - await this.log( - data.prRunId, - 'info', - `Updating existing test file: ${testCtx.testOutputPath}`, - ); + return result; + }, + buildPostCoverageCommand: (passingTestPaths) => { + const postTestPaths = useCoveragePackageOnly + ? passingTestPaths.length > 0 + ? passingTestPaths + : pythonTestPaths + : useAutoJsCoverage + ? passingTestPaths.length > 0 + ? passingTestPaths + : jsTestPaths + : passingTestPaths; + if (useAutoJsCoverage) { + prepareJsTestHarness(runDir, sourcePaths, postTestPaths); } - - const outcome = await this.generateAndValidateTest({ + return useCoveragePackageOnly + ? repoSetup.wrapCommand( + buildPythonCoverageCommand(sourcePaths, postTestPaths, runDir), + ) + : useAutoJsCoverage + ? repoSetup.wrapCommand( + buildJsCoverageCommand(sourcePaths, postTestPaths, runDir), + ) + : repoSetup.wrapCommand(repository.coverageCommand); + }, + generateTestForFile: async (file, report, options) => + this.generateAndValidateTest({ prRunId: data.prRunId, runDir, file, baseRef, headBranch, testCommand, - report: beforeCoverageResult.report, + report, repoPackages, repoSetup, sourcePaths, useCoveragePackageOnly, useAutoJsCoverage, logs, - }); - - totalGenerationAttempts += outcome.attempts; - - if (outcome.test) { - generatedTests.push(outcome.test); - generatedTestResults.push({ - filePath: outcome.test.filePath, - passed: outcome.passed, + generationMode: options.generationMode, + previousGeneratedContent: options.previousGeneratedContent, + }), + getSourceLineCount: async (filePath) => { + const content = await this.repoProvider.getFileContent( + runDir, + filePath, + ); + return content ? content.split('\n').length : 0; + }, + isConfigExportFile: async (filePath) => { + const content = await this.repoProvider.getFileContent( + runDir, + filePath, + ); + return content ? isConfigOrPromptExportFile(content) : false; + }, + installTestDeps: async (tests, declaredDeps) => { + if (repoSetup.isPython || repoSetup.isJavaScript) { + await this.installGeneratedTestDependencies({ + prRunId: data.prRunId, + runDir, + repoSetup, + rawInstallCommand: rawInstallCommand, + generatedTests: tests, + declaredTestDeps: declaredDeps, + repoPackages, + logs, }); - declaredTestDeps.push(...outcome.declaredDeps); } - } catch (err) { - const msg = `Test generation failed for ${file.path}: ${(err as Error).message}`; - logs.push(msg); - await this.log(data.prRunId, 'warn', msg); - } - } + }, + }, + ); + + const generatedTests = optimizationResult.generatedTests; + const generatedTestResults = optimizationResult.generatedTestResults; + const totalGenerationAttempts = optimizationResult.totalGenerationAttempts; + let afterCoverageResult = { + report: optimizationResult.afterReport, + coverageXml: beforeCoverageResult.coverageXml, + }; + + if (generatedTests.length > 0) { + await this.updateStatus(data.prRunId, 'RUNNING_TESTS'); + await this.log( + data.prRunId, + 'info', + `Optimization complete: ${generatedTestResults.filter((t) => t.passed).length} passing, ${generatedTestResults.filter((t) => !t.passed).length} failed`, + ); } else if (!this.hasLlmConfigured()) { await this.log( data.prRunId, @@ -454,56 +530,8 @@ export class PrAnalysisProcessor { } let executionStatus: 'PASS' | 'FAIL' | 'SKIPPED' | 'PARTIAL' = 'SKIPPED'; - let afterCoverageResult = beforeCoverageResult; - - if (generatedTests.length > 0) { - await this.updateStatus(data.prRunId, 'RUNNING_TESTS'); - - if (repoSetup.isPython || repoSetup.isJavaScript) { - await this.installGeneratedTestDependencies({ - prRunId: data.prRunId, - runDir, - repoSetup, - rawInstallCommand, - generatedTests, - declaredTestDeps, - repoPackages, - logs, - }); - } - - if (!useCoveragePackageOnly && !useAutoJsCoverage) { - const runTestsCommand = repoSetup.wrapCommand(testCommand); - await this.log( - data.prRunId, - 'info', - `Running generated tests: ${runTestsCommand}`, - ); - const testResult = await runCommand(runTestsCommand, runDir); - logs.push(testResult.stdout, testResult.stderr); - const passed = testResult.exitCode === 0; - - await prisma.generatedTestArtifact.updateMany({ - where: { prRunId: data.prRunId }, - data: { passed }, - }); - - for (const entry of generatedTestResults) { - entry.passed = passed; - } - - if (!passed) { - const output = `${testResult.stdout}\n${testResult.stderr}`.slice( - -2000, - ); - await this.log( - data.prRunId, - 'error', - `Generated tests failed execution (exit ${testResult.exitCode}): ${output}`, - ); - } - } + if (generatedTestResults.length > 0) { const passingTests = generatedTestResults.filter((t) => t.passed).length; const failingTests = generatedTestResults.filter((t) => !t.passed).length; @@ -520,59 +548,13 @@ export class PrAnalysisProcessor { `Generated test results: ${passingTests} passed, ${failingTests} failed`, ); - const generatedTestPaths = generatedTests.map((t) => t.filePath); - await this.updateStatus(data.prRunId, 'RECALCULATING'); - await this.log( - data.prRunId, - 'info', - 'Recalculating coverage after generated tests', - ); - - // Run only generated tests for post-coverage — pre-existing broken tests - // (e.g. src/**/__test__ with bad ESM imports) must not fail the recalc. - const postTestPaths = useCoveragePackageOnly - ? generatedTestPaths.length > 0 - ? generatedTestPaths - : pythonTestPaths - : useAutoJsCoverage - ? generatedTestPaths.length > 0 - ? generatedTestPaths - : jsTestPaths - : generatedTestPaths; - const postCoverageCommand = useCoveragePackageOnly - ? repoSetup.wrapCommand( - buildPythonCoverageCommand(sourcePaths, postTestPaths, runDir), - ) - : useAutoJsCoverage - ? repoSetup.wrapCommand( - buildJsCoverageCommand(sourcePaths, postTestPaths, runDir), - ) - : coverageCommand; - - afterCoverageResult = await this.withProgressHeartbeat( - data.prRunId, - 'Post-test coverage command running', - () => - this.runCoverage( - data.prRunId, - runDir, - postCoverageCommand, - baseRef, - headBranch, - sourcePaths, - useCoveragePackageOnly, - logs, - ), - ); - afterCoverageResult.report = applyCoverageThreshold( - afterCoverageResult.report, - this.testThresholdPercent, - ); - await this.log( - data.prRunId, - 'info', - `Final coverage: ${afterCoverageResult.report.totalCoveragePercent}% total, ${afterCoverageResult.report.diffCoveragePercent ?? 'n/a'}% diff`, - ); + if (afterCoverageResult.report !== beforeCoverageResult.report) { + await this.log( + data.prRunId, + 'info', + `Final coverage: ${afterCoverageResult.report.totalCoveragePercent}% total, ${afterCoverageResult.report.diffCoveragePercent ?? 'n/a'}% diff`, + ); + } } const afterMetrics = extractBaselineMetrics(afterCoverageResult.report); @@ -593,12 +575,15 @@ export class PrAnalysisProcessor { ) { workflowStatus = 'success'; thresholdReached = true; + } else if (optimizationResult.stopReason === 'plateau') { + workflowStatus = 'plateau_reached'; } else { workflowStatus = 'threshold_not_reached'; } const blockers = - workflowStatus === 'threshold_not_reached' + workflowStatus === 'threshold_not_reached' || + workflowStatus === 'plateau_reached' ? await classifyCoverageBlockers( runDir, afterMetrics.uncoveredLines, @@ -606,11 +591,15 @@ export class PrAnalysisProcessor { ) : []; - if (workflowStatus === 'threshold_not_reached' && blockers.length > 0) { + if ( + (workflowStatus === 'threshold_not_reached' || + workflowStatus === 'plateau_reached') && + blockers.length > 0 + ) { await this.log( data.prRunId, 'info', - `Classified ${blockers.length} remaining uncovered line(s) after ${totalGenerationAttempts} generation attempt(s)`, + `Classified ${blockers.length} remaining uncovered line(s) after ${totalGenerationAttempts} generation attempt(s) across ${optimizationResult.iterationSummaries.length} iteration(s)`, ); } @@ -623,6 +612,8 @@ export class PrAnalysisProcessor { coverageBefore: baselineMetrics, coverageAfter: afterMetrics, blockers, + optimizationIterations: optimizationResult.iterationSummaries, + stopReason: optimizationResult.stopReason, }); await this.persistCoverageResult({ @@ -766,11 +757,15 @@ export class PrAnalysisProcessor { this.testThresholdPercent, ) : applyCoverageThreshold( - await this.coverageProvider.runDiffCoverage( - coverageXml, - compareRef, + await resolveDiffCoverageReport({ + coverageXmlPath: coverageXml, repoDir, - ), + compareRef, + headBranch, + targetFiles: changedSourcePaths, + thresholdPercent: this.testThresholdPercent, + coverageProvider: this.coverageProvider, + }), this.testThresholdPercent, ); @@ -918,18 +913,17 @@ export class PrAnalysisProcessor { useCoveragePackageOnly: boolean; useAutoJsCoverage: boolean; logs: string[]; - }): Promise<{ - test: GeneratedTest | null; - passed: boolean; - attempts: number; - declaredDeps: string[]; - }> { + generationMode?: 'NEW_TEST_FILE' | 'COVERAGE_GAP'; + previousGeneratedContent?: string; + }): Promise { let lastFailureLogs = ''; let previousContent = ''; let lastTest: GeneratedTest | null = null; const declaredDeps: string[] = []; + let generationAttempts = 0; for (let attempt = 1; attempt <= this.maxGenerationAttempts; attempt++) { + generationAttempts = attempt; const generated = await this.generateTestForFile( params.runDir, params.file.path, @@ -945,13 +939,22 @@ export class PrAnalysisProcessor { attemptNumber: attempt, } : undefined, + { + generationMode: params.generationMode, + previousGeneratedContent: params.previousGeneratedContent, + }, ); if (!generated) { - return { test: null, passed: false, attempts: attempt, declaredDeps }; + return { + test: null, + passed: false, + attempts: attempt, + declaredDeps, + repairAttempts: 0, + }; } - // Persist LLM usage for this attempt (including repair rounds) if (generated.usage) { await this.persistUsage(params.prRunId, null, generated.usage); } @@ -983,7 +986,11 @@ export class PrAnalysisProcessor { if (existingArtifact) { await prisma.generatedTestArtifact.update({ where: { id: existingArtifact.id }, - data: { content: generated.content, filePath: generated.filePath }, + data: { + content: generated.content, + filePath: generated.filePath, + status: 'PENDING', + }, }); } else { await prisma.generatedTestArtifact.create({ @@ -992,12 +999,16 @@ export class PrAnalysisProcessor { filePath: generated.filePath, targetFile: generated.targetFile, content: generated.content, + status: 'PENDING', }, }); } const canRunIsolated = - params.useCoveragePackageOnly || params.useAutoJsCoverage; + params.useCoveragePackageOnly || + params.useAutoJsCoverage || + params.repoSetup.isJavaScript || + hasJsSourcePaths(params.sourcePaths); if (!canRunIsolated) { await this.log( @@ -1010,79 +1021,240 @@ export class PrAnalysisProcessor { passed: false, attempts: attempt, declaredDeps, + repairAttempts: 0, }; } - if (params.repoSetup.isPython || params.repoSetup.isJavaScript) { - await this.installGeneratedTestDependencies({ - prRunId: params.prRunId, - runDir: params.runDir, - repoSetup: params.repoSetup, - rawInstallCommand: null, - generatedTests: [generated], - declaredTestDeps: parsed.declaredDeps, - repoPackages: params.repoPackages, - logs: params.logs, - }); - } + const runSingleTest = async (): Promise<{ + passed: boolean; + output: string; + }> => { + if (params.repoSetup.isPython || params.repoSetup.isJavaScript) { + await this.installGeneratedTestDependencies({ + prRunId: params.prRunId, + runDir: params.runDir, + repoSetup: params.repoSetup, + rawInstallCommand: null, + generatedTests: [generated], + declaredTestDeps: parsed.declaredDeps, + repoPackages: params.repoPackages, + logs: params.logs, + }); + } - const runTestCommand = params.repoSetup.wrapCommand( - params.useCoveragePackageOnly - ? buildPythonTestCommand( - params.sourcePaths, - [generated.filePath], - params.runDir, - ) - : buildJsTestCommand( - params.sourcePaths, - [generated.filePath], - params.runDir, - ), - ); + const runTestCommand = params.repoSetup.wrapCommand( + params.useCoveragePackageOnly + ? buildPythonTestCommand( + params.sourcePaths, + [generated.filePath], + params.runDir, + ) + : buildJsTestCommand( + params.sourcePaths, + [generated.filePath], + params.runDir, + ), + ); - await this.log( - params.prRunId, - 'info', - `Running generated test (attempt ${attempt}/${this.maxGenerationAttempts}): ${generated.filePath}`, - ); - const testResult = await runCommand(runTestCommand, params.runDir); - params.logs.push(testResult.stdout, testResult.stderr); + if (params.useAutoJsCoverage || params.repoSetup.isJavaScript) { + prepareJsTestHarness(params.runDir, params.sourcePaths, [ + generated.filePath, + ]); + } - const passed = testResult.exitCode === 0; - await prisma.generatedTestArtifact.updateMany({ - where: { - prRunId: params.prRunId, - filePath: generated.filePath, - }, - data: { passed }, - }); + const testResult = await runCommand(runTestCommand, params.runDir); + params.logs.push(testResult.stdout, testResult.stderr); + const testOutput = `${testResult.stdout}\n${testResult.stderr}`; + const noTestsRan = + (params.useAutoJsCoverage || params.repoSetup.isJavaScript) && + /# tests 0\b/m.test(testOutput) && + testResult.exitCode === 0; + + const passed = testResult.exitCode === 0 && !noTestsRan; + return { passed, output: testOutput }; + }; - if (passed) { + const initialResult = await runSingleTest(); + + if (initialResult.passed) { + await this.updateArtifactStatus( + params.prRunId, + generated.filePath, + true, + 0, + ); await this.log( params.prRunId, 'info', `Generated test passed: ${generated.filePath}`, ); - return { test: generated, passed: true, attempts: attempt, declaredDeps }; + return { + test: generated, + passed: true, + attempts: attempt, + declaredDeps, + repairAttempts: 0, + }; } - lastFailureLogs = - `${testResult.stdout}\n${testResult.stderr}`.slice(-4000); + const noTestsRan = + (params.useAutoJsCoverage || params.repoSetup.isJavaScript) && + /# tests 0\b/m.test(initialResult.output) && + !initialResult.output.includes('fail'); + + if (noTestsRan) { + lastFailureLogs = `node --test did not discover any tests: ${generated.filePath}\n${initialResult.output.slice(-2000)}`; + await this.log( + params.prRunId, + 'error', + `Generated test reported pass but ran 0 tests for ${generated.filePath} (attempt ${attempt})`, + ); + continue; + } + + lastFailureLogs = initialResult.output.slice(-4000); + break; + } + + if (!lastTest) { + return { + test: null, + passed: false, + attempts: generationAttempts, + declaredDeps, + repairAttempts: 0, + }; + } + + const repairResult = await this.testRepairService.repairUntilPassing({ + maxAttempts: this.maxRepairAttempts, + runTest: async () => { + const runTestCommand = params.repoSetup.wrapCommand( + params.useCoveragePackageOnly + ? buildPythonTestCommand( + params.sourcePaths, + [lastTest!.filePath], + params.runDir, + ) + : buildJsTestCommand( + params.sourcePaths, + [lastTest!.filePath], + params.runDir, + ), + ); + if (params.useAutoJsCoverage || params.repoSetup.isJavaScript) { + prepareJsTestHarness(params.runDir, params.sourcePaths, [ + lastTest!.filePath, + ]); + } + const testResult = await runCommand(runTestCommand, params.runDir); + params.logs.push(testResult.stdout, testResult.stderr); + const testOutput = `${testResult.stdout}\n${testResult.stderr}`; + const noTestsRan = + (params.useAutoJsCoverage || params.repoSetup.isJavaScript) && + /# tests 0\b/m.test(testOutput) && + testResult.exitCode === 0; + return { + passed: testResult.exitCode === 0 && !noTestsRan, + output: testOutput, + }; + }, + repair: async (failureLogs, prevContent, repairAttempt) => { + const repaired = await this.generateTestForFile( + params.runDir, + params.file.path, + params.baseRef, + params.headBranch, + params.testCommand, + params.report, + params.repoPackages, + { + failureLogs, + previousTestContent: prevContent, + attemptNumber: repairAttempt, + }, + { + generationMode: params.generationMode ?? 'COVERAGE_GAP', + previousGeneratedContent: params.previousGeneratedContent, + }, + ); + if (repaired?.usage) { + await this.persistUsage(params.prRunId, null, repaired.usage); + } + if (repaired) { + lastTest = { ...repaired, content: parseGeneratedTestContent(repaired.content).content }; + } + return repaired; + }, + writeTest: async (content) => { + await writeFile( + join(params.runDir, lastTest!.filePath), + content, + 'utf-8', + ); + await prisma.generatedTestArtifact.updateMany({ + where: { + prRunId: params.prRunId, + filePath: lastTest!.filePath, + }, + data: { content }, + }); + }, + log: async (message) => { + await this.log(params.prRunId, 'info', message); + }, + }); + + await this.updateArtifactStatus( + params.prRunId, + lastTest.filePath, + repairResult.passed, + repairResult.repairAttempts, + repairResult.failureReason, + ); + + if (repairResult.passed) { + await this.log( + params.prRunId, + 'info', + `Generated test passed after ${repairResult.repairAttempts} repair attempt(s): ${lastTest.filePath}`, + ); + } else { await this.log( params.prRunId, 'error', - `Generated test failed for ${generated.filePath} (attempt ${attempt}, exit ${testResult.exitCode}): ${lastFailureLogs.slice(-500)}`, + `Generated test failed after ${repairResult.repairAttempts} repair attempt(s): ${lastTest.filePath}`, ); } return { test: lastTest, - passed: false, - attempts: this.maxGenerationAttempts, + passed: repairResult.passed, + attempts: generationAttempts, declaredDeps, + repairAttempts: repairResult.repairAttempts, + failureReason: repairResult.failureReason, }; } + private async updateArtifactStatus( + prRunId: string, + filePath: string, + passed: boolean, + repairAttempts: number, + failureReason?: string, + ) { + await prisma.generatedTestArtifact.updateMany({ + where: { prRunId, filePath }, + data: { + passed, + status: passed ? 'PASSING' : 'FAILED', + repairAttempts, + failureReason: failureReason ?? null, + }, + }); + } + private async generateTestForFile( repoDir: string, filePath: string, @@ -1096,6 +1268,10 @@ export class PrAnalysisProcessor { previousTestContent: string; attemptNumber: number; }, + options?: { + generationMode?: 'NEW_TEST_FILE' | 'COVERAGE_GAP'; + previousGeneratedContent?: string; + }, ): Promise { const source = await this.repoProvider.getFileContent(repoDir, filePath); if (!source) return null; @@ -1116,9 +1292,13 @@ export class PrAnalysisProcessor { pathsMatch(f.file, filePath), ); const fileDiffCoverage = fileCoverageEntry?.diffCoveragePercent ?? null; + const fileUncoveredLines = + fileCoverageEntry?.uncoveredLines.length + ? fileCoverageEntry.uncoveredLines + : uncoveredForFile; const analyzer = getAnalyzerForFile(filePath); - const symbols = await analyzer.extractSymbols(source, filePath, []); + const symbols = await analyzer.extractSymbols(source, filePath, fileUncoveredLines); const exportedSymbols = extractExportedSymbols(source, filePath); const testFile = await prepareTestFileContext( @@ -1128,6 +1308,21 @@ export class PrAnalysisProcessor { framework, ); + const isConfigExport = isConfigOrPromptExportFile(source); + const isComplexService = isComplexServiceFile(source); + const smokeExports = isConfigExport ? suggestSmokeTestExports(source) : []; + + const generationMode = + options?.generationMode === 'COVERAGE_GAP' + ? GenerationMode.COVERAGE_GAP + : testFile.isUpdatingExistingTest || options?.previousGeneratedContent + ? GenerationMode.COVERAGE_GAP + : GenerationMode.NEW_TEST_FILE; + + const coverageReport = fileCoverageEntry + ? `File: ${filePath}\nDiff coverage: ${fileDiffCoverage?.toFixed(1) ?? 'n/a'}%\nUncovered lines: ${fileUncoveredLines.join(', ')}` + : undefined; + const result = await this.llmProvider.generateTests({ language, framework, @@ -1135,7 +1330,7 @@ export class PrAnalysisProcessor { diff, source, existingTests: testFile.existingTests, - uncoveredLines: uncoveredForFile.join(', ') || 'unknown', + uncoveredLines: fileUncoveredLines.join(', ') || 'unknown', symbols, repoPackages, useFullSource: true, @@ -1146,6 +1341,12 @@ export class PrAnalysisProcessor { attemptNumber: repair?.attemptNumber, testOutputPath: testFile.testOutputPath, isUpdatingExistingTest: testFile.isUpdatingExistingTest, + generationMode, + previousGeneratedTests: options?.previousGeneratedContent, + coverageReport, + isConfigExportFile: isConfigExport, + isComplexServiceFile: isComplexService, + smokeTestExports: smokeExports, }); return result; } diff --git a/coverage-service/worker/src/processors/test-generation.processor.ts b/coverage-service/worker/src/processors/test-generation.processor.ts index 9dfd2cf..019a241 100644 --- a/coverage-service/worker/src/processors/test-generation.processor.ts +++ b/coverage-service/worker/src/processors/test-generation.processor.ts @@ -18,6 +18,7 @@ import { parseCoberturaXml, applyCoverageThreshold, pickTargetFileForTestGeneration, + resolveDiffCoverageReport, getTestThresholdPercent, pathsMatch, prepareTestFileContext, @@ -27,6 +28,7 @@ import type { Job } from 'bullmq'; import { buildJsCoverageCommand, collectJsTestPaths, + prepareJsTestHarness, } from '../lib/js-coverage'; import { prisma } from '../lib/prisma'; import { @@ -43,7 +45,7 @@ import { collectRepoPackages, parseGeneratedTestContent, } from '../lib/repo-packages'; -import { detectRepoSetup, setupPythonRepo } from '../lib/repo-setup'; +import { detectRepoSetup, setupPythonRepo, hasJsSourcePaths } from '../lib/repo-setup'; import { cleanupDir, findCoverageXml, runCommand } from '../lib/shell'; export class TestGenerationProcessor { @@ -312,10 +314,15 @@ export class TestGenerationProcessor { job?: Job; }): Promise { const sourcePaths = params.sourceFiles.map((f) => f.path); - const useCoveragePackageOnly = params.repoSetup.isPython; - const useAutoJsCoverage = params.repoSetup.isJavaScript; + const useCoveragePackageOnly = + params.repoSetup.isPython && + params.sourceFiles.some((f) => f.path.endsWith('.py')) && + !hasJsSourcePaths(sourcePaths); + const useAutoJsCoverage = + params.repoSetup.isJavaScript || hasJsSourcePaths(sourcePaths); let coverageCommand: string; + let jsTestPaths: string[] = []; if (useCoveragePackageOnly) { const pythonTestPaths = await collectPythonTestPaths( @@ -328,7 +335,7 @@ export class TestGenerationProcessor { buildPythonCoverageCommand(sourcePaths, pythonTestPaths, params.runDir), ); } else if (useAutoJsCoverage) { - const jsTestPaths = await collectJsTestPaths( + jsTestPaths = await collectJsTestPaths( params.runDir, params.changedFiles, sourcePaths, @@ -363,6 +370,10 @@ export class TestGenerationProcessor { `Coverage command: ${coverageCommand}`, ); + if (useAutoJsCoverage) { + prepareJsTestHarness(params.runDir, sourcePaths, jsTestPaths); + } + const result = await this.withProgressHeartbeat( params.runId, 'Coverage command running', @@ -423,11 +434,15 @@ export class TestGenerationProcessor { sourcePaths, this.testThresholdPercent, ) - : await this.coverageProvider.runDiffCoverage( - coverageXml, - params.baseRef, - params.runDir, - ); + : await resolveDiffCoverageReport({ + coverageXmlPath: coverageXml, + repoDir: params.runDir, + compareRef: params.baseRef, + headBranch: params.headBranch, + targetFiles: sourcePaths, + thresholdPercent: this.testThresholdPercent, + coverageProvider: this.coverageProvider, + }); return applyCoverageThreshold(report, this.testThresholdPercent); }