diff --git a/application/app/components/diagnosis/DiagnosisResult.vue b/application/app/components/diagnosis/DiagnosisResult.vue index 7d3c62be..03ce429d 100644 --- a/application/app/components/diagnosis/DiagnosisResult.vue +++ b/application/app/components/diagnosis/DiagnosisResult.vue @@ -221,6 +221,54 @@ function copyGitApply() { copy(cmd, { toast: 'git apply command copied' }); } +function downloadPatch() { + const patch = details.value?.suggestedFix?.patch as string | undefined; + if (!patch) return; + const body = patch.endsWith('\n') ? patch : patch + '\n'; + const blob = new Blob([body], { type: 'text/x-patch' }); + const url = URL.createObjectURL(blob); + const a = document.createElement('a'); + a.href = url; + a.download = `piwi-diagnosis-${props.diagnosis?.id ?? 'fix'}.patch`; + document.body.appendChild(a); + a.click(); + // Defer cleanup so the download isn't cut short mid-flight in some browsers. + setTimeout(() => { + a.remove(); + URL.revokeObjectURL(url); + }, 1000); +} + +interface PatchValidation { + status: 'applies' | 'applies-with-offset' | 'stale-file' | 'invalid' | 'unchecked'; + filesChecked: number; + filesInPatch: number; + errors: string[]; +} + +const patchValidation = computed(() => { + const v = details.value?.patchValidation; + return v && typeof v === 'object' ? (v as PatchValidation) : null; +}); + +/** Badge describing whether the suggested patch was verified to apply. */ +const patchBadge = computed<{ color: 'success' | 'warning' | 'error' | 'neutral'; icon: string; label: string; title: string } | null>(() => { + const v = patchValidation.value; + if (!v) return null; + switch (v.status) { + case 'applies': + return { color: 'success', icon: 'i-lucide-badge-check', label: 'Applies cleanly', title: 'Verified against the real file at the failing commit' }; + case 'applies-with-offset': + return { color: 'warning', icon: 'i-lucide-badge-check', label: 'Applies with offset', title: 'Context matched at a shifted line — apply should still succeed' }; + case 'stale-file': + return { color: 'error', icon: 'i-lucide-badge-alert', label: 'Does not apply', title: v.errors.join('\n') || 'The file changed since — patch context did not match' }; + case 'invalid': + return { color: 'error', icon: 'i-lucide-badge-alert', label: 'Invalid diff', title: v.errors.join('\n') || 'Could not parse the patch as a unified diff' }; + default: + return { color: 'neutral', icon: 'i-lucide-badge-help', label: 'Unverified', title: 'The source file was not in context, so the patch could not be validated' }; + } +}); + const categoryColors: Record = { 'app-bug': 'error', 'test-bug': 'warning', @@ -466,8 +514,28 @@ const pipeline = computed>(() => {
- patch +
+ patch + + {{ patchBadge.label }} + +
+ >(() => {
+

+ {{ patchValidation.errors[0] || 'This patch could not be verified against the source.' }} +

diff --git a/application/server/utils/ai-context.ts b/application/server/utils/ai-context.ts index c8cfbabc..347733ec 100644 --- a/application/server/utils/ai-context.ts +++ b/application/server/utils/ai-context.ts @@ -102,6 +102,7 @@ const SECTION_ORDER: SectionId[] = [ 'executionError', 'representativeExecution', 'testSource', + 'sourceFiles', 'failingAction', 'failingSteps', 'steps', @@ -1761,6 +1762,162 @@ function formatTopSuspectedChange(top: NonNullable>>; + +type SourceFilesCoverage = NonNullable; + +interface SourceFilesResult { + text: string | null; + files: Array<{ path: string; content: string }>; + coverage: SourceFilesCoverage | null; +} + +/** True when a path is repo-relative (not absolute POSIX or Windows). */ +function isRepoRelativePath(p: string): boolean { + return !p.startsWith('/') && !/^[A-Za-z]:[\\/]/.test(p); +} + +/** Resolve an import specifier against a repo-relative source file's directory. Exported for unit testing. */ +export function resolveImportPath(fromRepoRelPath: string, spec: string): string { + const dir = fromRepoRelPath.replace(/\\/g, '/').split('/').slice(0, -1); + const stack = [...dir]; + for (const part of spec.replace(/\\/g, '/').split('/')) { + if (part === '' || part === '.') continue; + if (part === '..') stack.pop(); + else stack.push(part); + } + return stack.join('/'); +} + +const CODE_EXTENSIONS = ['.ts', '.tsx', '.js', '.jsx', '.mts', '.cts', '.mjs', '.cjs', '.vue']; + +/** + * Candidate on-disk paths for a resolved, extension-less import (TS resolution + * order + index files). When the path already has a code extension, it is the + * only candidate. Exported for unit testing. + */ +export function candidateFilePaths(resolved: string): string[] { + if (CODE_EXTENSIONS.some((e) => resolved.endsWith(e))) return [resolved]; + const out: string[] = []; + for (const e of CODE_EXTENSIONS) out.push(resolved + e); + for (const e of CODE_EXTENSIONS) out.push(`${resolved}/index${e}`); + return out; +} + +/** Number lines `NNNN | text` (same format as the reporter snippet) so hunk headers can be computed. */ +function numberSourceLines(text: string): string { + return text + .split('\n') + .map((l, i) => `${String(i + 1).padStart(4)} | ${l}`) + .join('\n'); +} + +/** Best-effort repo-relative form of the failing test file (uses the SCM tree when the path is absolute). */ +async function resolveRepoRelativeTestPath( + provider: ScmProviderInstance, + ref: string, + testFilePath: string, +): Promise { + const norm = testFilePath.replace(/\\/g, '/'); + if (isRepoRelativePath(norm)) return norm.replace(/^\.\//, ''); + const tree = await provider.fetchTree(ref).catch(() => null); + if (!tree) return null; + let best: string | null = null; + for (const entry of tree) { + if (norm === entry || norm.endsWith('/' + entry)) { + if (!best || entry.length > best.length) best = entry; + } + } + return best; +} + +/** + * Fetch full content of the most-suspect changed files and the failing test's + * local import closure at the commit under test — the grounding that lets the + * model write a patch against code it has actually seen (and that we can then + * validate). Never throws; degrades to an empty result on any SCM failure. + */ +async function buildSourceFiles( + provider: ScmProviderInstance, + ref: string, + scored: ScoredFile[], + signals: RelevanceSignals, + limits: ContextLimits, +): Promise { + const empty: SourceFilesResult = { text: null, files: [], coverage: null }; + if (limits.maxSourceFiles <= 0 || !ref) return empty; + + try { + // Each target is an ordered candidate list; the first candidate that resolves wins. + const targets: string[][] = []; + const seen = new Set(); + const addTarget = (candidates: string[]) => { + const key = candidates[0]!; + if (seen.has(key)) return; + seen.add(key); + targets.push(candidates); + }; + + // 1. Top suspect changed files (repo-relative filenames straight from SCM). + for (const s of scored) { + if (s.score <= 0) break; // sorted desc — stop at the first non-signal file + addTarget([s.file.filename]); + if (targets.length >= limits.maxSourceFiles) break; + } + + // 2. Failing test's local (relative) import closure, one hop. + if (targets.length < limits.maxSourceFiles && signals.testFilePath && signals.testSource) { + const repoTestPath = await resolveRepoRelativeTestPath(provider, ref, signals.testFilePath); + if (repoTestPath) { + for (const spec of extractImportSpecifiers(signals.testSource)) { + if (!spec.startsWith('.')) continue; // only in-repo relative imports + addTarget(candidateFilePaths(resolveImportPath(repoTestPath, spec))); + } + } + } + + // Clip to sourceFileChars at fetch time so the stored content is exactly + // what the model is shown — patch validation must run against that, not the + // full fetched file. A clip counts as truncation for coverage. + const files: Array<{ path: string; content: string; truncated: boolean }> = []; + for (const candidates of targets) { + if (files.length >= limits.maxSourceFiles) break; + for (const path of candidates) { + const fetched = await provider.fetchFileAtRef(path, ref).catch(() => null); + if (fetched) { + const clipped = fetched.content.length > limits.sourceFileChars; + files.push({ + path: fetched.path, + content: clipped ? fetched.content.slice(0, limits.sourceFileChars) : fetched.content, + truncated: clipped || fetched.truncated, + }); + break; // first resolving candidate for this target + } + } + } + + if (files.length === 0) return empty; + + const blocks = files.map( + (f) => `### ${f.path}\n\`\`\`\n${numberSourceLines(f.content)}${f.truncated ? '\n[truncated]' : ''}\n\`\`\``, + ); + const text = + '## Source Files (full content at the failing commit)\n' + + "Full current content of the most-suspect changed files and the failing test's local imports, " + + 'at the commit under test. Line numbers are included so a patch can compute correct hunk headers. ' + + 'Only propose a patch for lines you can quote from these files.\n\n' + + blocks.join('\n\n'); + + return { + text, + files: files.map((f) => ({ path: f.path, content: f.content })), + coverage: { count: files.length, paths: files.map((f) => f.path), truncated: files.some((f) => f.truncated) }, + }; + } catch { + return empty; + } +} + /** * "What changed since last green run" + the actual SCM diff (or a manual-baseline * diff when there is no last green run). Tracks SCM coverage for the UI status line. @@ -1777,8 +1934,10 @@ async function scmInvestigationSections( coverage: ScmCoverage | null; scmChanges: ScmChanges | null; topSuspectedCommit: string | null; + sourceFiles: SourceFilesResult; }> { const sections: string[] = []; + let sourceFilesResult: SourceFilesResult = { text: null, files: [], coverage: null }; const scmCov: ScmCoverage = { hasLastGreen: false, hasCommitRange: false, @@ -1809,7 +1968,8 @@ async function scmInvestigationSections( .from(testRuns) .where(eq(testRuns.id, cluster.firstSeenRunId)); - if (!firstSeenRunRows[0]) return { sections, coverage: null, scmChanges: null, topSuspectedCommit: null }; + if (!firstSeenRunRows[0]) + return { sections, coverage: null, scmChanges: null, topSuspectedCommit: null, sourceFiles: sourceFilesResult }; try { const regression = await computeRegressionContext(db, firstSeenRunRows[0]); @@ -1868,6 +2028,11 @@ async function scmInvestigationSections( // Surface the most-suspect changed file (by relevance) + newest commit const top = getTopSuspectedChange(scored, changes.commits); if (top) sections.push(formatTopSuspectedChange(top)); + + // Full source of the suspect files + test imports at the failing commit. + if (provider) { + sourceFilesResult = await buildSourceFiles(provider, regression.commitRange.toSha, scored, signals, limits); + } } } catch (fetchErr) { scmCov.error = (fetchErr instanceof Error ? fetchErr.message : String(fetchErr)).slice(0, 300); @@ -1921,6 +2086,10 @@ async function scmInvestigationSections( const top = getTopSuspectedChange(scored, changes.commits); if (top) sections.push(formatTopSuspectedChange(top)); + + if (provider) { + sourceFilesResult = await buildSourceFiles(provider, currentCommit, scored, signals, limits); + } } } catch (fetchErr) { scmCov.error = (fetchErr instanceof Error ? fetchErr.message : String(fetchErr)).slice(0, 300); @@ -1996,6 +2165,10 @@ async function scmInvestigationSections( const top = getTopSuspectedChange(scored, changes.commits); if (top) sections.push(formatTopSuspectedChange(top)); + + if (provider) { + sourceFilesResult = await buildSourceFiles(provider, currentCommit, scored, signals, limits); + } } } catch (fetchErr) { scmCov.error = (fetchErr instanceof Error ? fetchErr.message : String(fetchErr)).slice(0, 300); @@ -2007,7 +2180,13 @@ async function scmInvestigationSections( // omit section if regression context fails } - return { sections, coverage: scmReached ? scmCov : null, scmChanges, topSuspectedCommit: null }; + return { + sections, + coverage: scmReached ? scmCov : null, + scmChanges, + topSuspectedCommit: null, + sourceFiles: sourceFilesResult, + }; } /** Diffs for commits the user explicitly picked, sharing one patch budget. */ @@ -2095,6 +2274,7 @@ export async function buildDiagnosisContext( let coverage: DiagnosisContextCoverage = { scm: null }; let scmChanges: ScmChanges | null = null; let images: AiAttachedImage[] | undefined; + let sourceFilesOut: BuiltDiagnosisContext['sourceFiles']; let clusterInfo: BuiltDiagnosisContext['cluster']; const push = (cs: ContextSection | null | undefined) => { @@ -2258,6 +2438,11 @@ export async function buildDiagnosisContext( coverage = { ...coverage, scm: scm.coverage }; scmChanges = scm.scmChanges; + // Full source files (suspect changed files + test imports) — grounds patch suggestions. + if (scm.sourceFiles.text) push(section('sourceFiles', 'Source Files', scm.sourceFiles.text)); + if (scm.sourceFiles.coverage) coverage = { ...coverage, sourceFiles: scm.sourceFiles.coverage }; + if (scm.sourceFiles.files.length > 0) sourceFilesOut = scm.sourceFiles.files; + // Selected commits (network fetch) push( section( @@ -2329,6 +2514,7 @@ export async function buildDiagnosisContext( coverage, scmChanges, images, + sourceFiles: sourceFilesOut, tokenEstimate: textTokenEstimate + imageTokenEstimate, textTokenEstimate, imageTokenEstimate, diff --git a/application/server/utils/ai-context.types.ts b/application/server/utils/ai-context.types.ts index 5640002f..2d0ecd69 100644 --- a/application/server/utils/ai-context.types.ts +++ b/application/server/utils/ai-context.types.ts @@ -31,6 +31,7 @@ export type SectionId = | 'runContext' | 'testAnnotations' | 'testSource' + | 'sourceFiles' | 'steps' | 'failingSteps' | 'console' @@ -70,6 +71,12 @@ export interface BuiltDiagnosisContext { coverage: DiagnosisContextCoverage; scmChanges: ScmChanges | null; images?: AiAttachedImage[]; + /** + * Full content of the source files fetched into the `sourceFiles` section, + * keyed by repo-relative path. Used to validate a suggested patch against the + * exact bytes the model was shown (see `validatePatch`). + */ + sourceFiles?: Array<{ path: string; content: string }>; /** Total estimated input tokens (text + images). */ tokenEstimate: number; /** Estimated tokens from the text context alone (≈ chars / 4). */ diff --git a/application/server/utils/ai-diagnosis.ts b/application/server/utils/ai-diagnosis.ts index 6127b597..61b63915 100644 --- a/application/server/utils/ai-diagnosis.ts +++ b/application/server/utils/ai-diagnosis.ts @@ -2,6 +2,8 @@ import { eq, and } from 'drizzle-orm'; import { failureDiagnoses, failureDiagnosisVersions, failureClusters, projects } from '../database/schema'; import type { FailureDiagnosis, FailureCluster } from '../database/schema'; import { DIAGNOSIS_JSON_SCHEMA, parseDiagnosisJson } from '#shared/ai-diagnosis'; +import { validatePatch } from '#shared/patch'; +import type { BuiltDiagnosisContext } from './ai-context.types'; import type { AiConfig } from '~~/types/api'; import { callAiProvider, resolveAiConfig, streamAiProvider, DEFAULT_ANTHROPIC_MODEL } from './ai-provider'; import type { AiAttachedImage, StreamChunk, StreamResult } from './ai-provider'; @@ -107,6 +109,18 @@ function runningDiagnosisFields(config: AiConfig) { }; } +/** + * Validate a suggested unified-diff patch against the exact source files the + * model was shown (fetched into the `sourceFiles` context section), so the UI + * can badge it as verified rather than taking it on faith. Returns null when + * there is no patch. + */ +function validateSuggestedPatch(ctx: BuiltDiagnosisContext, patch: string | null) { + if (!patch) return null; + const files = new Map((ctx.sourceFiles ?? []).map((f) => [f.path, f.content] as const)); + return validatePatch(patch, files); +} + export async function runClusterDiagnosis( db: DbClient, cluster: FailureCluster, @@ -312,6 +326,7 @@ export async function runClusterDiagnosis( selectedCommitShas: opts?.selectedCommitShas ?? null, additionalContext: opts?.additionalContext ?? null, autoSelectedCommits: ctx.scmChanges?.commits?.slice(0, 3).map((c) => c.sha) ?? null, + patchValidation: validateSuggestedPatch(ctx, diagnosis.suggestedFix.patch), }, error: null, inputTokens: sumTokens('inputTokens'), @@ -598,6 +613,7 @@ export async function streamClusterDiagnosis( selectedCommitShas: opts?.selectedCommitShas ?? null, additionalContext: opts?.additionalContext ?? null, autoSelectedCommits: ctx.scmChanges?.commits?.slice(0, 3).map((c) => c.sha) ?? null, + patchValidation: validateSuggestedPatch(ctx, diagnosis.suggestedFix.patch), }, error: null, inputTokens: sumTokens('inputTokens'), diff --git a/application/server/utils/ai-system-prompt.ts b/application/server/utils/ai-system-prompt.ts index 5b356e82..728ae1ce 100644 --- a/application/server/utils/ai-system-prompt.ts +++ b/application/server/utils/ai-system-prompt.ts @@ -43,10 +43,10 @@ The user message contains diagnostic evidence collected from a CI environment. T ## suggestedFix.patch When you have enough context to determine the exact lines to change, output a standard unified diff that can be applied with \`git apply\`. Rules: - Use the real file paths from the evidence (e.g. \`--- a/tests/foo.spec.ts\`, \`+++ b/tests/foo.spec.ts\`). -- Include correct \`@@ -L,N +L,N @@\` hunk headers. -- For test-bug: the patch should fix the test file using the test source provided. -- For app-bug with a git diff showing the regression: the patch should fix the application file (revert or correct the breaking change). -- Set patch to null if you are not confident in the exact lines, if the fix spans unknown files, or if no source was provided. +- Ground every hunk in a section that shows the actual file content — the \`Source Files\` section (full files, with \`NNNN | \` line numbers) or \`Test Source\`. Only change lines you can quote from one of those sections; compute the \`@@ -L,N +L,N @@\` hunk header from the shown line numbers. If a file you need is not shown, set patch to null. +- For test-bug: the patch should fix the test file using the test source / source files provided. +- For app-bug with a git diff showing the regression: the patch should fix the application file (revert or correct the breaking change), using the full file content in \`Source Files\` to get the surrounding lines and hunk offsets right. +- Set patch to null if you are not confident in the exact lines, if the fix spans files whose content you were not shown, or if no source was provided. A wrong patch is worse than none — the dashboard validates every patch against the real file and will flag one that does not apply. - Do not output a patch and a code snippet for the same fix; prefer patch when possible and set code to null.`; /** diff --git a/application/server/utils/scm/BitbucketProvider.ts b/application/server/utils/scm/BitbucketProvider.ts index 75cfbd1e..35c326bb 100644 --- a/application/server/utils/scm/BitbucketProvider.ts +++ b/application/server/utils/scm/BitbucketProvider.ts @@ -1,10 +1,18 @@ -import { ScmProvider, truncatePatch, MAX_SCM_FILES, MAX_RAW_DIFF_BYTES, FETCH_TIMEOUT_MS } from './ScmProvider'; -import type { ScmCommitDetail, ScmChanges } from './ScmProvider'; +import { + ScmProvider, + truncatePatch, + MAX_SCM_FILES, + MAX_FILE_BYTES, + MAX_RAW_DIFF_BYTES, + FETCH_TIMEOUT_MS, +} from './ScmProvider'; +import type { ScmCommitDetail, ScmChanges, ScmFileContent } from './ScmProvider'; import { TtlCache } from './cache'; const listBranchesCache = new TtlCache(3 * 60 * 1000); const listCommitsCache = new TtlCache(3 * 60 * 1000); const fetchChangesCache = new TtlCache(10 * 60 * 1000); +const fetchFileCache = new TtlCache(30 * 60 * 1000); function parsePatchesByFile(rawDiff: string): Map { const result = new Map(); @@ -44,7 +52,7 @@ export class BitbucketProvider extends ScmProvider { } async listBranches(limit = 100): Promise { - const key = `branches:${this.workspace}/${this.repoSlug}:${limit}`; + const key = `${this.keyPrefix}:branches:${this.workspace}/${this.repoSlug}:${limit}`; const hit = listBranchesCache.get(key); if (hit !== undefined) return hit; @@ -60,7 +68,7 @@ export class BitbucketProvider extends ScmProvider { } async listCommits(limit = 50, branch?: string): Promise { - const key = `${this.workspace}/${this.repoSlug}:${limit}:${branch ?? ''}`; + const key = `${this.keyPrefix}:${this.workspace}/${this.repoSlug}:${limit}:${branch ?? ''}`; const hit = listCommitsCache.get(key); if (hit !== undefined) return hit; @@ -87,7 +95,7 @@ export class BitbucketProvider extends ScmProvider { } async fetchChanges(fromSha: string, toSha: string): Promise { - const key = `${this.workspace}/${this.repoSlug}:${fromSha}:${toSha}`; + const key = `${this.keyPrefix}:${this.workspace}/${this.repoSlug}:${fromSha}:${toSha}`; const hit = fetchChangesCache.get(key); if (hit !== undefined) return hit; @@ -147,7 +155,48 @@ export class BitbucketProvider extends ScmProvider { return this.fetchChanges(`${sha}~1`, sha); } - async probeError(_branch?: string): Promise { + async fetchFileAtRef(path: string, ref: string): Promise { + const cleanPath = path.replace(/^\//, ''); + const key = `${this.keyPrefix}:file:${this.workspace}/${this.repoSlug}:${ref}:${cleanPath}`; + const hit = fetchFileCache.get(key); + if (hit !== undefined) return hit; + + const res = await fetch(`${this.base}/src/${encodeURIComponent(ref)}/${cleanPath}`, { + headers: this.makeHeaders(), + signal: AbortSignal.timeout(FETCH_TIMEOUT_MS), + }); + if (!res.ok) { + fetchFileCache.set(key, null); + return null; + } + const raw = await res.text(); + const result: ScmFileContent = { + path: cleanPath, + content: raw.length > MAX_FILE_BYTES ? raw.slice(0, MAX_FILE_BYTES) : raw, + truncated: raw.length > MAX_FILE_BYTES, + }; + fetchFileCache.set(key, result); + return result; + } + + async fetchTree(_ref: string): Promise { + // Bitbucket's `src` endpoint has no clean recursive listing; skip tree-based + // path normalization for Bitbucket (callers degrade gracefully to null). return null; } + + async probeError(branch?: string): Promise { + try { + const res = await fetch(this.base, { headers: this.makeHeaders(), signal: AbortSignal.timeout(FETCH_TIMEOUT_MS) }); + if (!res.ok) { + if (res.status === 404) return 'Repository not found on Bitbucket. Check the remote URL in your test run metadata.'; + if (res.status === 401 || res.status === 403) + return 'Bitbucket API authentication failed. Check your SCM token in Settings → AI.'; + return `Bitbucket API returned ${res.status}.`; + } + return `No commits found on ${branch ? `branch '${branch}'` : 'the default branch'}.`; + } catch { + return 'Could not reach the Bitbucket API. Check your network connection.'; + } + } } diff --git a/application/server/utils/scm/GitHubProvider.ts b/application/server/utils/scm/GitHubProvider.ts index 9c31c387..ac49c5b3 100644 --- a/application/server/utils/scm/GitHubProvider.ts +++ b/application/server/utils/scm/GitHubProvider.ts @@ -1,11 +1,14 @@ -import { ScmProvider, truncatePatch, MAX_SCM_FILES, FETCH_TIMEOUT_MS } from './ScmProvider'; -import type { ScmCommitDetail, ScmChanges } from './ScmProvider'; +import { ScmProvider, truncatePatch, MAX_SCM_FILES, MAX_FILE_BYTES, FETCH_TIMEOUT_MS } from './ScmProvider'; +import type { ScmCommitDetail, ScmChanges, ScmFileContent } from './ScmProvider'; import { TtlCache } from './cache'; const listBranchesCache = new TtlCache(3 * 60 * 1000); const listCommitsCache = new TtlCache(3 * 60 * 1000); const fetchChangesCache = new TtlCache(10 * 60 * 1000); const fetchCommitDiffCache = new TtlCache(10 * 60 * 1000); +// Content is immutable per SHA, so cache it (incl. negative lookups) for longer. +const fetchFileCache = new TtlCache(30 * 60 * 1000); +const fetchTreeCache = new TtlCache(10 * 60 * 1000); export class GitHubProvider extends ScmProvider { readonly provider = 'github' as const; @@ -26,7 +29,7 @@ export class GitHubProvider extends ScmProvider { } async listBranches(limit = 100): Promise { - const key = `branches:${this.repoPath}:${limit}`; + const key = `${this.keyPrefix}:branches:${this.repoPath}:${limit}`; const hit = listBranchesCache.get(key); if (hit !== undefined) return hit; @@ -42,7 +45,7 @@ export class GitHubProvider extends ScmProvider { } async listCommits(limit = 50, branch?: string): Promise { - const key = `${this.repoPath}:${limit}:${branch ?? ''}`; + const key = `${this.keyPrefix}:${this.repoPath}:${limit}:${branch ?? ''}`; const hit = listCommitsCache.get(key); if (hit !== undefined) return hit; @@ -71,7 +74,7 @@ export class GitHubProvider extends ScmProvider { } async fetchChanges(fromSha: string, toSha: string): Promise { - const key = `${this.repoPath}:${fromSha}:${toSha}`; + const key = `${this.keyPrefix}:${this.repoPath}:${fromSha}:${toSha}`; const hit = fetchChangesCache.get(key); if (hit !== undefined) return hit; @@ -102,7 +105,7 @@ export class GitHubProvider extends ScmProvider { } async fetchCommitDiff(sha: string): Promise { - const key = `${this.repoPath}:${sha}`; + const key = `${this.keyPrefix}:${this.repoPath}:${sha}`; const hit = fetchCommitDiffCache.get(key); if (hit !== undefined) return hit; @@ -131,6 +134,66 @@ export class GitHubProvider extends ScmProvider { return result; } + async fetchFileAtRef(path: string, ref: string): Promise { + const cleanPath = path.replace(/^\//, ''); + const key = `${this.keyPrefix}:file:${this.repoPath}:${ref}:${cleanPath}`; + const hit = fetchFileCache.get(key); + if (hit !== undefined) return hit; + + const url = new URL(`https://api.github.com/repos/${this.repoPath}/contents/${cleanPath}`); + url.searchParams.set('ref', ref); + const res = await fetch(url.toString(), { + headers: { ...this.makeHeaders(), Accept: 'application/vnd.github.raw' }, + signal: AbortSignal.timeout(FETCH_TIMEOUT_MS), + }); + if (!res.ok) { + fetchFileCache.set(key, null); + return null; + } + const raw = await res.text(); + const result: ScmFileContent = { + path: cleanPath, + content: raw.length > MAX_FILE_BYTES ? raw.slice(0, MAX_FILE_BYTES) : raw, + truncated: raw.length > MAX_FILE_BYTES, + }; + fetchFileCache.set(key, result); + return result; + } + + async fetchTree(ref: string): Promise { + const key = `${this.keyPrefix}:tree:${this.repoPath}:${ref}`; + const hit = fetchTreeCache.get(key); + if (hit !== undefined) return hit; + + const treePaths = async (treeish: string): Promise => { + const res = await fetch(`https://api.github.com/repos/${this.repoPath}/git/trees/${treeish}?recursive=1`, { + headers: this.makeHeaders(), + signal: AbortSignal.timeout(FETCH_TIMEOUT_MS), + }); + if (!res.ok) return null; + const data = (await res.json()) as { tree?: Array<{ path: string; type: string }> }; + return (data.tree ?? []).filter((e) => e.type === 'blob').map((e) => e.path); + }; + + // The trees endpoint takes a tree SHA or ref. A commit SHA usually resolves + // too, but when it doesn't, resolve the commit to its tree SHA and retry. + let result = await treePaths(ref); + if (result === null) { + const commitRes = await fetch(`https://api.github.com/repos/${this.repoPath}/commits/${ref}`, { + headers: this.makeHeaders(), + signal: AbortSignal.timeout(FETCH_TIMEOUT_MS), + }); + if (!commitRes.ok) return null; + const commit = (await commitRes.json()) as { commit?: { tree?: { sha?: string } } }; + const treeSha = commit.commit?.tree?.sha; + if (!treeSha) return null; + result = await treePaths(treeSha); + } + if (result === null) return null; + fetchTreeCache.set(key, result); + return result; + } + async probeError(branch?: string): Promise { try { const res = await fetch(`https://api.github.com/repos/${this.repoPath}`, { headers: this.makeHeaders() }); diff --git a/application/server/utils/scm/GitLabProvider.ts b/application/server/utils/scm/GitLabProvider.ts index 8faae042..d5b297fd 100644 --- a/application/server/utils/scm/GitLabProvider.ts +++ b/application/server/utils/scm/GitLabProvider.ts @@ -1,11 +1,24 @@ -import { ScmProvider, truncatePatch, MAX_SCM_FILES, FETCH_TIMEOUT_MS } from './ScmProvider'; -import type { ScmCommitDetail, ScmChanges } from './ScmProvider'; +import { ScmProvider, truncatePatch, MAX_SCM_FILES, MAX_FILE_BYTES, FETCH_TIMEOUT_MS } from './ScmProvider'; +import type { ScmCommitDetail, ScmChanges, ScmFileContent } from './ScmProvider'; import { TtlCache } from './cache'; const listBranchesCache = new TtlCache(3 * 60 * 1000); const listCommitsCache = new TtlCache(3 * 60 * 1000); const fetchChangesCache = new TtlCache(10 * 60 * 1000); const fetchCommitDiffCache = new TtlCache(10 * 60 * 1000); +const fetchFileCache = new TtlCache(30 * 60 * 1000); +const fetchTreeCache = new TtlCache(10 * 60 * 1000); + +/** Count added/removed lines in a unified-diff hunk body (ignores +++/--- headers). */ +function countDiffLines(diff: string): { additions: number; deletions: number } { + let additions = 0; + let deletions = 0; + for (const line of diff.split('\n')) { + if (line.startsWith('+') && !line.startsWith('+++')) additions++; + else if (line.startsWith('-') && !line.startsWith('---')) deletions++; + } + return { additions, deletions }; +} export class GitLabProvider extends ScmProvider { readonly provider = 'gitlab' as const; @@ -19,7 +32,7 @@ export class GitLabProvider extends ScmProvider { } async listBranches(limit = 100): Promise { - const key = `branches:${this.hostname}:${this.repoPath}:${limit}`; + const key = `${this.keyPrefix}:branches:${this.hostname}:${this.repoPath}:${limit}`; const hit = listBranchesCache.get(key); if (hit !== undefined) return hit; @@ -36,7 +49,7 @@ export class GitLabProvider extends ScmProvider { } async listCommits(limit = 50, branch?: string): Promise { - const key = `${this.hostname}:${this.repoPath}:${limit}:${branch ?? ''}`; + const key = `${this.keyPrefix}:${this.hostname}:${this.repoPath}:${limit}:${branch ?? ''}`; const hit = listCommitsCache.get(key); if (hit !== undefined) return hit; @@ -68,7 +81,7 @@ export class GitLabProvider extends ScmProvider { } async fetchChanges(fromSha: string, toSha: string): Promise { - const key = `${this.hostname}:${this.repoPath}:${fromSha}:${toSha}`; + const key = `${this.keyPrefix}:${this.hostname}:${this.repoPath}:${fromSha}:${toSha}`; const hit = fetchChangesCache.get(key); if (hit !== undefined) return hit; @@ -91,20 +104,23 @@ export class GitLabProvider extends ScmProvider { }; const result: ScmChanges = { commits: (data.commits ?? []).map((c) => ({ sha: c.id.slice(0, 7), message: c.message.split('\n')[0] ?? '' })), - files: (data.diffs ?? []).slice(0, MAX_SCM_FILES).map((f) => ({ - filename: f.new_path || f.old_path, - status: f.new_file ? 'added' : f.deleted_file ? 'removed' : f.renamed_file ? 'renamed' : 'modified', - additions: 0, - deletions: 0, - patch: f.diff ? truncatePatch(f.diff) : undefined, - })), + files: (data.diffs ?? []).slice(0, MAX_SCM_FILES).map((f) => { + const { additions, deletions } = countDiffLines(f.diff ?? ''); + return { + filename: f.new_path || f.old_path, + status: f.new_file ? 'added' : f.deleted_file ? 'removed' : f.renamed_file ? 'renamed' : 'modified', + additions, + deletions, + patch: f.diff ? truncatePatch(f.diff) : undefined, + }; + }), }; fetchChangesCache.set(key, result); return result; } async fetchCommitDiff(sha: string): Promise { - const key = `${this.hostname}:${this.repoPath}:${sha}`; + const key = `${this.keyPrefix}:${this.hostname}:${this.repoPath}:${sha}`; const hit = fetchCommitDiffCache.get(key); if (hit !== undefined) return hit; @@ -127,18 +143,69 @@ export class GitLabProvider extends ScmProvider { }>; const result: ScmChanges = { commits: [], - files: data.slice(0, MAX_SCM_FILES).map((f) => ({ - filename: f.new_path || f.old_path, - status: f.new_file ? 'added' : f.deleted_file ? 'removed' : f.renamed_file ? 'renamed' : 'modified', - additions: 0, - deletions: 0, - patch: f.diff ? truncatePatch(f.diff) : undefined, - })), + files: data.slice(0, MAX_SCM_FILES).map((f) => { + const { additions, deletions } = countDiffLines(f.diff ?? ''); + return { + filename: f.new_path || f.old_path, + status: f.new_file ? 'added' : f.deleted_file ? 'removed' : f.renamed_file ? 'renamed' : 'modified', + additions, + deletions, + patch: f.diff ? truncatePatch(f.diff) : undefined, + }; + }), }; fetchCommitDiffCache.set(key, result); return result; } + async fetchFileAtRef(path: string, ref: string): Promise { + const cleanPath = path.replace(/^\//, ''); + const key = `${this.keyPrefix}:file:${this.hostname}:${this.repoPath}:${ref}:${cleanPath}`; + const hit = fetchFileCache.get(key); + if (hit !== undefined) return hit; + + const projectPath = encodeURIComponent(this.repoPath); + const encodedFile = encodeURIComponent(cleanPath); + const res = await fetch( + `https://${this.hostname}/api/v4/projects/${projectPath}/repository/files/${encodedFile}/raw?ref=${encodeURIComponent(ref)}`, + { headers: this.makeHeaders(), signal: AbortSignal.timeout(FETCH_TIMEOUT_MS) }, + ); + if (!res.ok) { + fetchFileCache.set(key, null); + return null; + } + const raw = await res.text(); + const result: ScmFileContent = { + path: cleanPath, + content: raw.length > MAX_FILE_BYTES ? raw.slice(0, MAX_FILE_BYTES) : raw, + truncated: raw.length > MAX_FILE_BYTES, + }; + fetchFileCache.set(key, result); + return result; + } + + async fetchTree(ref: string): Promise { + const key = `${this.keyPrefix}:tree:${this.hostname}:${this.repoPath}:${ref}`; + const hit = fetchTreeCache.get(key); + if (hit !== undefined) return hit; + + const projectPath = encodeURIComponent(this.repoPath); + const paths: string[] = []; + // GitLab paginates the tree; fetch a bounded number of pages for best-effort coverage. + for (let page = 1; page <= 10; page++) { + const res = await fetch( + `https://${this.hostname}/api/v4/projects/${projectPath}/repository/tree?ref=${encodeURIComponent(ref)}&recursive=true&per_page=100&page=${page}`, + { headers: this.makeHeaders(), signal: AbortSignal.timeout(FETCH_TIMEOUT_MS) }, + ); + if (!res.ok) return page === 1 ? null : paths; + const data = (await res.json()) as Array<{ path: string; type: string }>; + for (const e of data) if (e.type === 'blob') paths.push(e.path); + if (data.length < 100) break; + } + fetchTreeCache.set(key, paths); + return paths; + } + async probeError(branch?: string): Promise { try { const projectPath = encodeURIComponent(this.repoPath); diff --git a/application/server/utils/scm/ScmProvider.ts b/application/server/utils/scm/ScmProvider.ts index a1435f50..3edb9c87 100644 --- a/application/server/utils/scm/ScmProvider.ts +++ b/application/server/utils/scm/ScmProvider.ts @@ -26,9 +26,19 @@ export interface ScmChanges { patchesOmitted?: boolean; } +/** Full content of a file at a specific ref (used to ground diagnosis patches). */ +export interface ScmFileContent { + path: string; + content: string; + /** true when the content was truncated to MAX_FILE_BYTES */ + truncated: boolean; +} + export const MAX_SCM_FILES = 30; export const MAX_PATCH_PER_FILE = 100_000; export const MAX_RAW_DIFF_BYTES = 200_000; +/** Cap on a single file's content fetched via fetchFileAtRef. */ +export const MAX_FILE_BYTES = 200_000; export const FETCH_TIMEOUT_MS = 10_000; export function truncatePatch(patch: string): string { @@ -36,12 +46,29 @@ export function truncatePatch(patch: string): string { return patch.slice(0, MAX_PATCH_PER_FILE) + '\n[... patch truncated ...]'; } +/** FNV-1a 32-bit hash → 8 hex chars. Stable, dependency-free, good enough to namespace cache keys by token. */ +export function shortHash(input: string): string { + let h = 0x811c9dc5; + for (let i = 0; i < input.length; i++) { + h ^= input.charCodeAt(i); + h = Math.imul(h, 0x01000193); + } + return (h >>> 0).toString(16).padStart(8, '0'); +} + export abstract class ScmProvider { abstract readonly provider: 'github' | 'gitlab' | 'bitbucket'; protected readonly token: string | null; + /** + * Namespaces module-level cache keys by the token in use so a token-less + * (public, rate-limited) fetch can never serve its result to an authenticated + * caller, or vice-versa. + */ + protected readonly keyPrefix: string; constructor(token: string | null) { this.token = token; + this.keyPrefix = token ? shortHash(token) : 'anon'; } protected makeHeaders(): Record { @@ -55,4 +82,16 @@ export abstract class ScmProvider { abstract fetchChanges(fromSha: string, toSha: string): Promise; abstract fetchCommitDiff(sha: string): Promise; abstract probeError(branch?: string): Promise; + /** + * Full content of a single file at a ref (commit SHA / branch). Returns null + * when the file does not exist at that ref or the fetch fails. Content is + * immutable per SHA, so implementations may cache aggressively. + */ + abstract fetchFileAtRef(path: string, ref: string): Promise; + /** + * Recursive list of repo-relative file paths at a ref. Used to normalize + * reporter paths to repo-relative and to validate patch paths. Returns null + * on failure; may be capped by the provider. + */ + abstract fetchTree(ref: string): Promise; } diff --git a/application/shared/ai-context-limits.ts b/application/shared/ai-context-limits.ts index 94330188..3394307c 100644 --- a/application/shared/ai-context-limits.ts +++ b/application/shared/ai-context-limits.ts @@ -23,6 +23,10 @@ export interface ContextLimits { ariaSnapshotChars: number; /** Max characters of the test source snippet. */ testSourceChars: number; + /** Max number of full source files fetched from SCM to ground patches (0 disables). */ + maxSourceFiles: number; + /** Max characters per fetched full source file. */ + sourceFileChars: number; /** When > 0, parse the Playwright trace ZIP to extract failing action context. */ maxTraceActions: number; /** Max characters for trace-derived DOM/ARIA excerpt. */ @@ -51,6 +55,8 @@ export const DEFAULT_CONTEXT_LIMITS: ContextLimits = { networkRequests: 25, ariaSnapshotChars: 12000, testSourceChars: 8000, + maxSourceFiles: 4, + sourceFileChars: 12000, serverLogEntries: 50, serverLogEntryChars: 1000, maxImages: 5, @@ -145,6 +151,22 @@ export const CONTEXT_LIMIT_FIELDS: ContextLimitField[] = [ min: 0, max: 50000, }, + { + key: 'maxSourceFiles', + label: 'Full source files', + envVar: 'PIWI_AI_MAX_SOURCE_FILES', + description: 'Max full source files fetched from SCM to ground patches (0 disables).', + min: 0, + max: 20, + }, + { + key: 'sourceFileChars', + label: 'Source file characters', + envVar: 'PIWI_AI_MAX_SOURCE_FILE_CHARS', + description: 'Max characters per fetched full source file.', + min: 0, + max: 50000, + }, { key: 'serverLogEntries', label: 'Server log entries', diff --git a/application/shared/diagnosis-sections.ts b/application/shared/diagnosis-sections.ts index 90ca65a7..b0e2708a 100644 --- a/application/shared/diagnosis-sections.ts +++ b/application/shared/diagnosis-sections.ts @@ -22,6 +22,7 @@ export const DIAGNOSIS_SECTIONS: DiagnosisSectionMeta[] = [ { id: 'testAnnotations', label: 'Test annotations (@fixme/@flaky …)', short: 'Annotations' }, { id: 'affectedTests', label: 'Affected tests', short: 'Tests' }, { id: 'testSource', label: 'Test source code', short: 'Source' }, + { id: 'sourceFiles', label: 'Full source files (suspect + imports)', short: 'Files' }, { id: 'steps', label: 'Test steps', short: 'Steps' }, { id: 'failingSteps', label: 'Failing steps', short: 'Steps' }, { id: 'console', label: 'Browser console logs', short: 'Console' }, diff --git a/application/shared/patch.ts b/application/shared/patch.ts new file mode 100644 index 00000000..77d9e4da --- /dev/null +++ b/application/shared/patch.ts @@ -0,0 +1,254 @@ +/** + * Pure unified-diff parser and dry-run applier used to validate an AI-suggested + * `suggestedFix.patch` against the real file content the model was shown. No + * dependencies, no filesystem — safe to run on the server (and, later, the UI). + * + * The goal is a trustworthy signal, not a full `git apply` reimplementation: + * we parse the hunks, then check that each hunk's context/deletion lines can be + * located in the target file (at its stated position, or shifted — "offset"), + * so the UI can badge a patch as verified rather than taking it on faith. + */ + +export interface PatchHunk { + oldStart: number; + oldLines: number; + newStart: number; + newLines: number; + /** Body lines including their leading marker (' ' context, '+' add, '-' delete). */ + lines: string[]; +} + +export interface PatchFile { + /** Path from `--- a/…` (null for /dev/null, i.e. a newly added file). */ + oldPath: string | null; + /** Path from `+++ b/…` (null for /dev/null, i.e. a deleted file). */ + newPath: string | null; + hunks: PatchHunk[]; +} + +export interface ParsedPatch { + files: PatchFile[]; +} + +export type PatchValidationStatus = + | 'applies' + | 'applies-with-offset' + | 'stale-file' + | 'invalid' + | 'unchecked'; + +export interface PatchValidation { + status: PatchValidationStatus; + /** How many target files we actually had content for and could check. */ + filesChecked: number; + /** How many distinct files the patch touches. */ + filesInPatch: number; + /** Human-readable reasons, one per problem file. */ + errors: string[]; +} + +const HUNK_RE = /^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@/; + +/** Strip a leading `a/` or `b/` diff prefix; leave other paths untouched. `/dev/null` → null. */ +export function stripAbPrefix(path: string | null | undefined): string | null { + if (!path) return null; + const p = path.trim(); + if (p === '/dev/null') return null; + return p.replace(/^[ab]\//, ''); +} + +/** + * Parse a unified diff into files and hunks. Tolerant of `diff --git` preamble + * lines and of hunks that omit the count (defaulting to 1). Returns + * `{ files: [] }` when nothing parseable is found. + */ +export function parseUnifiedDiff(text: string): ParsedPatch { + const files: PatchFile[] = []; + const lines = text.split('\n'); + let current: PatchFile | null = null; + let hunk: PatchHunk | null = null; + + const closeHunk = () => { + if (current && hunk) current.hunks.push(hunk); + hunk = null; + }; + const closeFile = () => { + closeHunk(); + if (current && current.hunks.length > 0) files.push(current); + current = null; + }; + + for (let i = 0; i < lines.length; i++) { + const line = lines[i]!; + + if (line.startsWith('--- ')) { + closeFile(); + current = { oldPath: parsePathLine(line.slice(4)), newPath: null, hunks: [] }; + continue; + } + if (line.startsWith('+++ ') && current) { + current.newPath = parsePathLine(line.slice(4)); + continue; + } + + const m = HUNK_RE.exec(line); + if (m && current) { + closeHunk(); + hunk = { + oldStart: parseInt(m[1]!, 10), + oldLines: m[2] != null ? parseInt(m[2], 10) : 1, + newStart: parseInt(m[3]!, 10), + newLines: m[4] != null ? parseInt(m[4], 10) : 1, + lines: [], + }; + continue; + } + + if (hunk) { + const c = line[0]; + if (c === ' ' || c === '+' || c === '-') { + hunk.lines.push(line); + continue; + } + if (c === '\\') continue; // "\ No newline at end of file" + // Anything else ends the hunk (and, if not a new file/hunk marker, the file). + closeHunk(); + } + } + closeFile(); + + return { files }; +} + +/** Extract the path token from a `--- ` / `+++ ` line (drops a trailing tab-timestamp). */ +function parsePathLine(rest: string): string | null { + const token = rest.split('\t')[0]!.trim(); + return token === '/dev/null' ? null : token; +} + +/** Lines of the pre-image (context + deletions), marker stripped. */ +function oldBlock(hunk: PatchHunk): string[] { + return hunk.lines.filter((l) => l[0] === ' ' || l[0] === '-').map((l) => l.slice(1)); +} + +/** Lines of the post-image (context + additions), marker stripped. */ +function newBlock(hunk: PatchHunk): string[] { + return hunk.lines.filter((l) => l[0] === ' ' || l[0] === '+').map((l) => l.slice(1)); +} + +function normalize(line: string): string { + return line.replace(/\r$/, ''); +} + +/** Find `block` in `lines`, preferring the match closest to `expected`. -1 if none. */ +function findBlock(lines: string[], block: string[], expected: number): number { + if (block.length === 0) return Math.max(0, Math.min(expected, lines.length)); + let best = -1; + let bestDist = Infinity; + const last = lines.length - block.length; + for (let i = 0; i <= last; i++) { + let match = true; + for (let j = 0; j < block.length; j++) { + if (normalize(lines[i + j]!) !== normalize(block[j]!)) { + match = false; + break; + } + } + if (!match) continue; + const dist = Math.abs(i - expected); + if (dist < bestDist) { + best = i; + bestDist = dist; + if (dist === 0) break; + } + } + return best; +} + +interface ApplyResult { + ok: boolean; + offset: boolean; + reason?: string; +} + +/** Dry-run apply a file's hunks against its content, tracking whether any hunk shifted. */ +function applyHunks(content: string, hunks: PatchHunk[]): ApplyResult { + let lines = content.split('\n'); + let offset = false; + + for (const hunk of hunks) { + const oldB = oldBlock(hunk); + const newB = newBlock(hunk); + const expected = Math.max(0, hunk.oldStart - 1); + const idx = findBlock(lines, oldB, expected); + if (idx === -1) { + return { ok: false, offset, reason: `hunk @@ -${hunk.oldStart} context did not match the file` }; + } + if (oldB.length > 0 && idx !== expected) offset = true; + lines = [...lines.slice(0, idx), ...newB, ...lines.slice(idx + oldB.length)]; + } + + return { ok: true, offset }; +} + +/** Case-exact lookup with an unambiguous path-suffix fallback (model may drop leading dirs). */ +function lookupContent(files: Map, target: string): string | null { + const direct = files.get(target); + if (direct != null) return direct; + const matches: string[] = []; + for (const key of files.keys()) { + if (key === target || key.endsWith('/' + target) || target.endsWith('/' + key)) matches.push(key); + } + if (matches.length === 1) return files.get(matches[0]!) ?? null; + return null; +} + +/** + * Validate an AI-suggested unified-diff patch against the file content the model + * was shown (repo-relative path → content). Files not present in `available` + * are counted but not judged, so a partially-grounded patch degrades to + * `unchecked` rather than a false failure. + */ +export function validatePatch( + patch: string | null | undefined, + available: Map | Record, +): PatchValidation { + if (!patch || !patch.trim()) { + return { status: 'unchecked', filesChecked: 0, filesInPatch: 0, errors: [] }; + } + + const files = available instanceof Map ? available : new Map(Object.entries(available)); + const parsed = parseUnifiedDiff(patch); + + if (parsed.files.length === 0) { + return { status: 'invalid', filesChecked: 0, filesInPatch: 0, errors: ['Could not parse a unified diff.'] }; + } + + const errors: string[] = []; + let filesChecked = 0; + let sawOffset = false; + let sawStale = false; + + for (const f of parsed.files) { + const target = stripAbPrefix(f.newPath) ?? stripAbPrefix(f.oldPath); + if (!target) continue; // pure deletion of /dev/null etc. — nothing to verify + const content = lookupContent(files, target); + if (content == null) continue; // we don't have this file — leave it unchecked + filesChecked++; + const res = applyHunks(content, f.hunks); + if (!res.ok) { + sawStale = true; + errors.push(`${target}: ${res.reason ?? 'did not apply'}`); + } else if (res.offset) { + sawOffset = true; + } + } + + let status: PatchValidationStatus; + if (filesChecked === 0) status = 'unchecked'; + else if (sawStale) status = 'stale-file'; + else if (sawOffset) status = 'applies-with-offset'; + else status = 'applies'; + + return { status, filesChecked, filesInPatch: parsed.files.length, errors }; +} diff --git a/application/shared/piwi-env-vars.ts b/application/shared/piwi-env-vars.ts index 856f7d6c..256d2390 100644 --- a/application/shared/piwi-env-vars.ts +++ b/application/shared/piwi-env-vars.ts @@ -254,6 +254,14 @@ export const PIWI_ENV_VARS = { description: 'Max characters of the test source snippet.', category: 'ai-limits', }, + PIWI_AI_MAX_SOURCE_FILES: { + description: 'Max full source files fetched from SCM to ground patches (0 disables).', + category: 'ai-limits', + }, + PIWI_AI_MAX_SOURCE_FILE_CHARS: { + description: 'Max characters per fetched full source file.', + category: 'ai-limits', + }, PIWI_AI_MAX_SERVER_LOG_ENTRIES: { description: 'Max backend server log entries (from X-Piwi-Logs header) included.', category: 'ai-limits', diff --git a/application/shared/types.ts b/application/shared/types.ts index 80673929..c8d94bf7 100644 --- a/application/shared/types.ts +++ b/application/shared/types.ts @@ -96,6 +96,8 @@ export interface TestCasePayload { testAnnotations?: TestAnnotation[] | null; /** Per-element locator snapshots with ranked alternatives (transient — not stored as a column). */ locatorSnapshots?: LocatorSnapshot[] | null; + /** Source snippet around the failing line of the spec file (captured on failure only). */ + testSource?: string | null; } // ── Test run counters ───────────────────────────────────────────────────────── @@ -189,6 +191,8 @@ export interface StreamEventPayload { suiteConfig?: SuiteConfigEntry[] | null; testAnnotations?: TestAnnotation[] | null; locatorSnapshots?: LocatorSnapshot[] | null; + /** Source snippet around the failing line of the spec file (captured on failure only). */ + testSource?: string | null; } // ── Finish payload ──────────────────────────────────────────────────────────── diff --git a/application/tests/unit/ai-context-sections.test.ts b/application/tests/unit/ai-context-sections.test.ts index a69a8a10..21e3c10c 100644 --- a/application/tests/unit/ai-context-sections.test.ts +++ b/application/tests/unit/ai-context-sections.test.ts @@ -5,6 +5,8 @@ import { extractPageSnapshotSection, extractLocatorLiterals, findLiteralInPatch, + resolveImportPath, + candidateFilePaths, } from '../../server/utils/ai-context'; import type { ContextLimits } from '#shared/ai-context-limits'; @@ -194,3 +196,27 @@ describe('locator-literal SCM matching (diff-content relevance)', () => { expect(withHit).toBeGreaterThan(pathOnly); }); }); + +describe('resolveImportPath', () => { + test('resolves a sibling import', () => { + expect(resolveImportPath('tests/login.spec.ts', './helpers')).toBe('tests/helpers'); + }); + test('resolves a parent-dir import', () => { + expect(resolveImportPath('tests/e2e/login.spec.ts', '../pages/LoginPage')).toBe('tests/pages/LoginPage'); + }); + test('collapses redundant segments', () => { + expect(resolveImportPath('a/b/c.spec.ts', './../b/./x')).toBe('a/b/x'); + }); +}); + +describe('candidateFilePaths', () => { + test('returns the path as-is when it already has a code extension', () => { + expect(candidateFilePaths('tests/helpers.ts')).toEqual(['tests/helpers.ts']); + }); + test('expands an extension-less path to ts/tsx/... and index files', () => { + const c = candidateFilePaths('tests/pages/LoginPage'); + expect(c).toContain('tests/pages/LoginPage.ts'); + expect(c).toContain('tests/pages/LoginPage.tsx'); + expect(c).toContain('tests/pages/LoginPage/index.ts'); + }); +}); diff --git a/application/tests/unit/patch.test.ts b/application/tests/unit/patch.test.ts new file mode 100644 index 00000000..5416d053 --- /dev/null +++ b/application/tests/unit/patch.test.ts @@ -0,0 +1,113 @@ +import { describe, test, expect } from 'vitest'; +import { parseUnifiedDiff, stripAbPrefix, validatePatch } from '#shared/patch'; + +const SAMPLE = `--- a/src/foo.ts ++++ b/src/foo.ts +@@ -1,3 +1,3 @@ + const a = 1; +-const b = 2; ++const b = 3; + const c = 4; +`; + +describe('stripAbPrefix', () => { + test('strips a/ and b/ prefixes', () => { + expect(stripAbPrefix('a/src/foo.ts')).toBe('src/foo.ts'); + expect(stripAbPrefix('b/src/foo.ts')).toBe('src/foo.ts'); + }); + test('maps /dev/null to null', () => { + expect(stripAbPrefix('/dev/null')).toBeNull(); + }); + test('leaves prefix-less paths untouched', () => { + expect(stripAbPrefix('tests/x.spec.ts')).toBe('tests/x.spec.ts'); + }); +}); + +describe('parseUnifiedDiff', () => { + test('parses a single-file, single-hunk diff', () => { + const parsed = parseUnifiedDiff(SAMPLE); + expect(parsed.files).toHaveLength(1); + const f = parsed.files[0]!; + expect(f.oldPath).toBe('a/src/foo.ts'); + expect(f.newPath).toBe('b/src/foo.ts'); + expect(f.hunks).toHaveLength(1); + expect(f.hunks[0]!.oldStart).toBe(1); + expect(f.hunks[0]!.lines).toHaveLength(4); + }); + + test('tolerates a diff --git preamble and multiple files', () => { + const multi = `diff --git a/x.ts b/x.ts +--- a/x.ts ++++ b/x.ts +@@ -1 +1 @@ +-a ++b +diff --git a/y.ts b/y.ts +--- a/y.ts ++++ b/y.ts +@@ -1 +1 @@ +-c ++d +`; + const parsed = parseUnifiedDiff(multi); + expect(parsed.files.map((f) => f.newPath)).toEqual(['b/x.ts', 'b/y.ts']); + }); + + test('returns no files for non-diff text', () => { + expect(parseUnifiedDiff('just some prose').files).toHaveLength(0); + }); +}); + +describe('validatePatch', () => { + const fooContent = 'const a = 1;\nconst b = 2;\nconst c = 4;\n'; + + test('applies cleanly when context matches at the stated position', () => { + const res = validatePatch(SAMPLE, { 'src/foo.ts': fooContent }); + expect(res.status).toBe('applies'); + expect(res.filesChecked).toBe(1); + expect(res.filesInPatch).toBe(1); + }); + + test('reports offset when the hunk matches at a shifted line', () => { + const shifted = '// header\n// added line\n' + fooContent; + const res = validatePatch(SAMPLE, { 'src/foo.ts': shifted }); + expect(res.status).toBe('applies-with-offset'); + }); + + test('reports stale-file when context does not match', () => { + const diverged = 'const a = 1;\nconst b = 999;\nconst c = 4;\n'; + const res = validatePatch(SAMPLE, { 'src/foo.ts': diverged }); + expect(res.status).toBe('stale-file'); + expect(res.errors.length).toBeGreaterThan(0); + }); + + test('is unchecked when we do not have the target file', () => { + const res = validatePatch(SAMPLE, { 'src/other.ts': 'x' }); + expect(res.status).toBe('unchecked'); + expect(res.filesChecked).toBe(0); + }); + + test('is invalid for unparseable patch text', () => { + const res = validatePatch('not a patch at all', { 'src/foo.ts': fooContent }); + expect(res.status).toBe('invalid'); + }); + + test('is unchecked for a null/empty patch', () => { + expect(validatePatch(null, {}).status).toBe('unchecked'); + expect(validatePatch('', {}).status).toBe('unchecked'); + }); + + test('resolves via unambiguous suffix when the model drops a leading dir', () => { + const patch = `--- a/foo.ts\n+++ b/foo.ts\n@@ -1,3 +1,3 @@\n const a = 1;\n-const b = 2;\n+const b = 3;\n const c = 4;\n`; + const res = validatePatch(patch, { 'src/foo.ts': fooContent }); + expect(res.status).toBe('applies'); + expect(res.filesChecked).toBe(1); + }); + + test('handles a multi-file patch, flagging the stale one', () => { + const patch = `--- a/a.ts\n+++ b/a.ts\n@@ -1 +1 @@\n-x\n+y\n--- a/b.ts\n+++ b/b.ts\n@@ -1 +1 @@\n-old\n+new\n`; + const res = validatePatch(patch, { 'a.ts': 'x\n', 'b.ts': 'DIFFERENT\n' }); + expect(res.filesChecked).toBe(2); + expect(res.status).toBe('stale-file'); + }); +}); diff --git a/application/types/api.ts b/application/types/api.ts index 4166832f..71de6cdc 100644 --- a/application/types/api.ts +++ b/application/types/api.ts @@ -861,6 +861,14 @@ export interface DiagnosisContextCoverage { source: 'prior-run' | 'element-match' | 'fingerprint' | 'aria-snapshot' | 'none'; alternativesCount: number; } | null; + /** Full source files fetched to ground the diagnosis (suspect changed files + test imports). */ + sourceFiles?: { + count: number; + /** Repo-relative paths of the fetched files. */ + paths: string[]; + /** true when at least one file was truncated to the size cap. */ + truncated: boolean; + } | null; /** Sections where data is not applicable (with reason), keyed by section id. Absent in coverage means "no data". */ notApplicable?: Record; } diff --git a/docs/ai-diagnosis.md b/docs/ai-diagnosis.md index 32a71960..d4f446ac 100644 --- a/docs/ai-diagnosis.md +++ b/docs/ai-diagnosis.md @@ -138,6 +138,26 @@ Changed files are ranked by relevance to the failing test before patch text is i Files scoring ≤ 2 are excluded from the "Top Suspected Change" callout (a low-signal hint is worse than none), but they still appear in the full changed-files list. +### Full source files + +A diff shows only the lines that changed. To write a patch the model needs the surrounding code too, so — when SCM is reachable — Piwi also fetches the **full current content** of the most-suspect changed files (top-ranked by the relevance score above) and the failing test's local imports (page objects, helpers, fixtures resolved one hop from the test's `import` statements), at the commit under test. These land in a `Source Files` context section with `NNNN | ` line numbers so the model can compute correct hunk headers. + +Capped by `PIWI_AI_MAX_SOURCE_FILES` (default 4, set to 0 to disable) and `PIWI_AI_MAX_SOURCE_FILE_CHARS` (default 12000). Fetched over the same SCM provider API as the diff (GitHub/GitLab/Bitbucket), cached per commit SHA. The `coverage.sourceFiles` field on the context/diagnosis response lists which files were pulled in. + +### Validated patches + +Every `suggestedFix.patch` is checked server-side, before it reaches you, against the exact source files the model was shown: Piwi parses the unified diff and dry-runs each hunk against the real file content (tolerating line-offset drift). The result is stored on the diagnosis as `details.patchValidation.status` and shown as a badge on the patch: + +| Status | Badge | Meaning | +|--------|-------|---------| +| `applies` | ✅ Applies cleanly | Every hunk matched at its stated position | +| `applies-with-offset` | ⚠️ Applies with offset | Matched, but at a shifted line — `git apply` should still succeed | +| `stale-file` | ❌ Does not apply | The file diverged from what the patch expects | +| `invalid` | ❌ Invalid diff | The text isn't a parseable unified diff | +| `unchecked` | Unverified | The target file wasn't in context, so the patch couldn't be validated | + +A wrong patch is worse than none, so the model is instructed to set `patch` to null unless it can quote the lines it changes from the `Source Files` / `Test Source` sections. The patch card offers **Copy**, **Copy `git apply` command**, and **Download `.patch`**; applying is always manual (the dashboard never writes to your repository). + ## Locator healing When the failure is a broken locator, the context includes an **Alternative Locators** section: ranked replacement locators sourced from a prior passing run (highest confidence — captured against the real DOM), from a fresh match of the renamed/moved element on the failing page, or from the failure-time ARIA snapshot. The section also names a single **recommended fix** — convention-preserving where the original locator style is stable enough — which the model is instructed to use verbatim in `suggestedFix.code` rather than fabricating a locator. When nothing scores as stable, it advises adding a `data-testid` to the application as the durable fix. @@ -154,20 +174,22 @@ Every piece of evidence sent to the model costs tokens. Piwi caps each input so | Environment variable | Default | What it caps | |----------------------|--------:|--------------| -| `PIWI_AI_MAX_SAMPLE_ERROR_CHARS` | 3000 | Characters of raw error text per error block | -| `PIWI_AI_MAX_SCM_PATCH_BUDGET` | 4000 | Total characters of diff patches across changed files | -| `PIWI_AI_MAX_AFFECTED_TESTS` | 15 | Affected tests listed | -| `PIWI_AI_MAX_STEPS` | 30 | Recent test steps included | -| `PIWI_AI_MAX_CONSOLE_ENTRIES` | 15 | Console error/warning entries | -| `PIWI_AI_MAX_CONSOLE_ENTRY_CHARS` | 400 | Characters per console entry | -| `PIWI_AI_MAX_NETWORK_REQUESTS` | 15 | Failed network requests included | -| `PIWI_AI_MAX_ARIA_SNAPSHOT_CHARS` | 4000 | Characters of the page ARIA snapshot | -| `PIWI_AI_MAX_TEST_SOURCE_CHARS` | 3000 | Characters of the test source snippet | -| `PIWI_AI_MAX_SERVER_LOG_ENTRIES` | 30 | Backend server log entries (from the `X-Piwi-Logs` header) | -| `PIWI_AI_MAX_SERVER_LOG_ENTRY_CHARS` | 400 | Characters per server log entry | -| `PIWI_AI_MAX_IMAGES` | 3 | Screenshots auto-included in the context | -| `PIWI_AI_MAX_PASSED_PEERS` | 10 | Passing peer tests in the same file listed | -| `PIWI_AI_MAX_CONSOLE_WINDOW` | 30 | Console entries (any level) in the window before failure | +| `PIWI_AI_MAX_SAMPLE_ERROR_CHARS` | 10000 | Characters of raw error text per error block | +| `PIWI_AI_MAX_SCM_PATCH_BUDGET` | 15000 | Total characters of diff patches across changed files | +| `PIWI_AI_MAX_AFFECTED_TESTS` | 30 | Affected tests listed | +| `PIWI_AI_MAX_STEPS` | 50 | Recent test steps included | +| `PIWI_AI_MAX_CONSOLE_ENTRIES` | 30 | Console error/warning entries | +| `PIWI_AI_MAX_CONSOLE_ENTRY_CHARS` | 1000 | Characters per console entry | +| `PIWI_AI_MAX_NETWORK_REQUESTS` | 25 | Failed network requests included | +| `PIWI_AI_MAX_ARIA_SNAPSHOT_CHARS` | 12000 | Characters of the page ARIA snapshot | +| `PIWI_AI_MAX_TEST_SOURCE_CHARS` | 8000 | Characters of the test source snippet | +| `PIWI_AI_MAX_SOURCE_FILES` | 4 | Full source files fetched from SCM to ground patches (0 disables) | +| `PIWI_AI_MAX_SOURCE_FILE_CHARS` | 12000 | Characters per fetched full source file | +| `PIWI_AI_MAX_SERVER_LOG_ENTRIES` | 50 | Backend server log entries (from the `X-Piwi-Logs` header) | +| `PIWI_AI_MAX_SERVER_LOG_ENTRY_CHARS` | 1000 | Characters per server log entry | +| `PIWI_AI_MAX_IMAGES` | 5 | Screenshots auto-included in the context | +| `PIWI_AI_MAX_PASSED_PEERS` | 20 | Passing peer tests in the same file listed | +| `PIWI_AI_MAX_CONSOLE_WINDOW` | 50 | Console entries (any level) in the window before failure | | `PIWI_AI_SLOW_REQUEST_MS` | 1500 | Duration (ms) above which a network request is flagged as slow | ## Privacy