diff --git a/src/ai/claude.ts b/src/ai/claude.ts index cc2e3c7..9c4af40 100644 --- a/src/ai/claude.ts +++ b/src/ai/claude.ts @@ -1,5 +1,5 @@ import Anthropic from '@anthropic-ai/sdk'; -import type { AIProvider } from './provider.js'; +import type { AIProvider, AIResponse, AIRequestOptions } from './provider.js'; export class ClaudeProvider implements AIProvider { private client: Anthropic; @@ -14,17 +14,24 @@ export class ClaudeProvider implements AIProvider { this.model = model; } - async complete(prompt: string): Promise { + async complete(prompt: string, options?: AIRequestOptions): Promise { const response = await this.client.messages.create({ model: this.model, - max_tokens: 1024, + max_tokens: options?.maxTokens ?? 1024, messages: [{ role: 'user', content: prompt }], }); const block = response.content[0]; - if (block.type === 'text') { - return block.text; + if (block.type !== 'text') { + throw new Error('Unexpected response type from Claude API'); } - throw new Error('Unexpected response type from Claude API'); + + return { + text: block.text, + usage: { + inputTokens: response.usage.input_tokens, + outputTokens: response.usage.output_tokens, + }, + }; } } diff --git a/src/ai/index.ts b/src/ai/index.ts index 4457c4d..2c96fed 100644 --- a/src/ai/index.ts +++ b/src/ai/index.ts @@ -4,7 +4,7 @@ import { ClaudeProvider } from './claude.js'; import { OpenAIProvider } from './openai.js'; import { OllamaProvider } from './ollama.js'; -export type { AIProvider } from './provider.js'; +export type { AIProvider, AIResponse, AIUsage, AIRequestOptions } from './provider.js'; export function createAIProvider(config: RepoKeeperConfig['ai']): AIProvider { switch (config.provider) { diff --git a/src/ai/ollama.ts b/src/ai/ollama.ts index 809403a..98813f6 100644 --- a/src/ai/ollama.ts +++ b/src/ai/ollama.ts @@ -1,4 +1,10 @@ -import type { AIProvider } from './provider.js'; +import type { AIProvider, AIResponse, AIRequestOptions } from './provider.js'; + +interface OllamaResponse { + response: string; + prompt_eval_count?: number; + eval_count?: number; +} export class OllamaProvider implements AIProvider { private url: string; @@ -9,7 +15,7 @@ export class OllamaProvider implements AIProvider { this.model = model; } - async complete(prompt: string): Promise { + async complete(prompt: string, options?: AIRequestOptions): Promise { const response = await fetch(`${this.url}/api/generate`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, @@ -17,6 +23,7 @@ export class OllamaProvider implements AIProvider { model: this.model, prompt, stream: false, + ...(options?.maxTokens ? { options: { num_predict: options.maxTokens } } : {}), }), }); @@ -24,7 +31,13 @@ export class OllamaProvider implements AIProvider { throw new Error(`Ollama API error: ${response.status} ${response.statusText}`); } - const data = (await response.json()) as { response: string }; - return data.response; + const data = (await response.json()) as OllamaResponse; + return { + text: data.response, + usage: { + inputTokens: data.prompt_eval_count ?? 0, + outputTokens: data.eval_count ?? 0, + }, + }; } } diff --git a/src/ai/openai.ts b/src/ai/openai.ts index 1e877d2..bc5e1f0 100644 --- a/src/ai/openai.ts +++ b/src/ai/openai.ts @@ -1,5 +1,5 @@ import OpenAI from 'openai'; -import type { AIProvider } from './provider.js'; +import type { AIProvider, AIResponse, AIRequestOptions } from './provider.js'; export class OpenAIProvider implements AIProvider { private client: OpenAI; @@ -15,17 +15,24 @@ export class OpenAIProvider implements AIProvider { this.model = model; } - async complete(prompt: string): Promise { + async complete(prompt: string, options?: AIRequestOptions): Promise { const response = await this.client.chat.completions.create({ model: this.model, messages: [{ role: 'user', content: prompt }], - max_tokens: 1024, + max_tokens: options?.maxTokens ?? 1024, }); const content = response.choices[0]?.message?.content; if (!content) { throw new Error('Empty response from OpenAI API'); } - return content; + + return { + text: content, + usage: { + inputTokens: response.usage?.prompt_tokens ?? 0, + outputTokens: response.usage?.completion_tokens ?? 0, + }, + }; } } diff --git a/src/ai/provider.ts b/src/ai/provider.ts index 453e0f4..d5c75ff 100644 --- a/src/ai/provider.ts +++ b/src/ai/provider.ts @@ -1,3 +1,17 @@ +export interface AIUsage { + inputTokens: number; + outputTokens: number; +} + +export interface AIResponse { + text: string; + usage: AIUsage; +} + +export interface AIRequestOptions { + maxTokens?: number; +} + export interface AIProvider { - complete(prompt: string): Promise; + complete(prompt: string, options?: AIRequestOptions): Promise; } diff --git a/src/config.ts b/src/config.ts index 5ba0769..747ea1e 100644 --- a/src/config.ts +++ b/src/config.ts @@ -41,6 +41,8 @@ export interface RepoKeeperConfig { focus: string[]; maxContextFiles: number; minDiffLines: number; + ignore?: string[]; + commitStatus?: boolean; }; port: number; } @@ -71,6 +73,8 @@ const defaults: RepoKeeperConfig = { focus: ['security', 'performance', 'test-coverage', 'breaking-changes'], maxContextFiles: 5, minDiffLines: 10, + ignore: [], + commitStatus: false, }, port: 3001, }; diff --git a/src/github/client.ts b/src/github/client.ts index 3fd5caa..2576213 100644 --- a/src/github/client.ts +++ b/src/github/client.ts @@ -125,6 +125,23 @@ export class GitHubClient { })); } + async createCommitStatus( + sha: string, + state: 'success' | 'failure' | 'pending', + description: string, + context: string = 'repokeeper/review', + ): Promise { + await this.octokit.repos.createCommitStatus({ + owner: this.owner, + repo: this.repo, + sha, + state, + description, + context, + }); + log('info', `Set commit status ${state} on ${sha.slice(0, 7)}: ${description}`); + } + private async ensureLabelsExist(labels: string[]): Promise { const existing = await this.octokit.paginate(this.octokit.issues.listLabelsForRepo, { owner: this.owner, diff --git a/src/pr/summariser.ts b/src/pr/summariser.ts index d65c8fd..6394783 100644 --- a/src/pr/summariser.ts +++ b/src/pr/summariser.ts @@ -80,7 +80,7 @@ export async function handlePullRequest( .replace('{description}', body || '(no description)') .replace('{diff}', truncatedDiff); - const summary = await ai.complete(prompt); + const { text: summary } = await ai.complete(prompt); // Build the comment const fileList = files @@ -122,7 +122,7 @@ export async function handlePullRequestMerged( .replace('{description}', body || '(no description)') .replace('{fileSummary}', fileSummary); - const releaseNotes = await ai.complete(prompt); + const { text: releaseNotes } = await ai.complete(prompt); const comment = `## Release Notes\n\n${releaseNotes}\n\n` + diff --git a/src/review/reviewer.ts b/src/review/reviewer.ts index 4437ead..8dc1b6c 100644 --- a/src/review/reviewer.ts +++ b/src/review/reviewer.ts @@ -9,6 +9,48 @@ import { isAlreadyReviewed, markReviewed, parseDiffHunks, cleanupPR } from './hu import { getAcceptedPatterns, formatAcceptedPatternsPrompt, learnFromMergedPR } from './memory.js'; import type { PRReviewPayload, ReviewResult, ReviewFinding, CodeReviewConfig, EnrichedFile } from './types.js'; +export function matchesIgnorePattern(file: string, pattern: string): boolean { + // Escape regex metacharacters first (except glob chars * ? which we handle separately) + const regexStr = pattern + .replace(/[.+^${}()|[\]\\]/g, '\\$&') + .replace(/\*\*\//g, '{{GLOBSTAR_SLASH}}') + .replace(/\*\*/g, '{{GLOBSTAR}}') + .replace(/\*/g, '[^/]*') + .replace(/\?/g, '[^/]') + .replace(/{{GLOBSTAR_SLASH}}/g, '(?:.+/)?') + .replace(/{{GLOBSTAR}}/g, '.*'); + return new RegExp(`^${regexStr}$`).test(file); +} + +export function filterIgnoredFiles(files: string[], ignorePatterns: string[]): string[] { + if (!ignorePatterns || ignorePatterns.length === 0) return files; + return files.filter((file) => !ignorePatterns.some((pattern) => matchesIgnorePattern(file, pattern))); +} + +function filterDiffByFiles(diff: string, allowedFiles: string[]): string { + const allowedSet = new Set(allowedFiles); + const sections = diff.split(/(?=^diff --git )/m); + return sections + .filter((section) => { + const match = section.match(/^diff --git a\/(.+?) b\//); + if (!match) return false; + return allowedSet.has(match[1]); + }) + .join(''); +} + +export function buildCommitStatus(findings: ReviewFinding[]): { state: 'success' | 'failure'; description: string } { + const hasBlocking = findings.some((f) => f.severity === 'BLOCKING'); + if (hasBlocking) { + const count = findings.filter((f) => f.severity === 'BLOCKING').length; + return { state: 'failure', description: `${count} blocking issue(s) found` }; + } + if (findings.length > 0) { + return { state: 'success', description: `${findings.length} non-blocking finding(s)` }; + } + return { state: 'success', description: 'No issues found' }; +} + const DEFAULT_REVIEW_CONFIG: CodeReviewConfig = { enabled: true, focus: ['security', 'performance', 'test-coverage', 'breaking-changes'], @@ -154,11 +196,18 @@ export async function handleCodeReview( // Get changed file names from hunks const changedFiles = [...new Set(hunks.map((h) => h.file))]; + // Filter out ignored files + const reviewableFiles = filterIgnoredFiles(changedFiles, reviewConfig.ignore ?? []); + if (reviewableFiles.length === 0) { + log('info', `PR #${prNumber}: all changed files match ignore patterns, skipping review`); + return; + } + // Build codebase context let enrichedFiles: EnrichedFile[] = []; try { enrichedFiles = await buildContext( - changedFiles, + reviewableFiles, repository.clone_url, owner, repo, @@ -174,21 +223,28 @@ export async function handleCodeReview( // Get accepted patterns from memory const acceptedPatterns = formatAcceptedPatternsPrompt(getAcceptedPatterns()); - // Truncate diff for AI + // Filter diff to only include reviewable files and truncate for AI + const filteredDiff = filterDiffByFiles(diff, reviewableFiles); const maxDiffChars = 30_000; - const truncatedDiff = diff.length > maxDiffChars - ? diff.slice(0, maxDiffChars) + '\n\n... (diff truncated)' - : diff; + const truncatedDiff = filteredDiff.length > maxDiffChars + ? filteredDiff.slice(0, maxDiffChars) + '\n\n... (diff truncated)' + : filteredDiff; // Build prompt and call AI const prompt = buildReviewPrompt(truncatedDiff, enrichedFiles, reviewConfig, acceptedPatterns); - const aiResponse = await ai.complete(prompt); - const result = parseAIResponse(aiResponse); + const { text: aiResponseText } = await ai.complete(prompt); + const result = parseAIResponse(aiResponseText); // Post review via GitHub API const octokit = new Octokit({ auth: config.github.token }); await postReview(octokit, owner, repo, prNumber, headSha, result); + // Post commit status if enabled + if (reviewConfig.commitStatus) { + const { state, description } = buildCommitStatus(result.findings); + await github.createCommitStatus(headSha, state, description); + } + // Mark this SHA as reviewed markReviewed(owner, repo, prNumber, headSha); diff --git a/src/review/types.ts b/src/review/types.ts index d364c54..433649c 100644 --- a/src/review/types.ts +++ b/src/review/types.ts @@ -36,6 +36,8 @@ export interface CodeReviewConfig { focus: string[]; maxContextFiles: number; minDiffLines: number; + ignore?: string[]; + commitStatus?: boolean; } export interface PRReviewPayload { diff --git a/src/triage/classifier.ts b/src/triage/classifier.ts index b77649f..0d72f8a 100644 --- a/src/triage/classifier.ts +++ b/src/triage/classifier.ts @@ -51,7 +51,7 @@ export async function classifyIssue( } const prompt = CLASSIFY_PROMPT.replace('{title}', title).replace('{body}', body || '(empty)'); - const result = (await ai.complete(prompt)).trim().toLowerCase(); + const result = (await ai.complete(prompt)).text.trim().toLowerCase(); const valid: IssueCategory[] = ['bug', 'feature', 'question', 'duplicate', 'docs', 'invalid']; if (valid.includes(result as IssueCategory)) { diff --git a/src/triage/duplicate.ts b/src/triage/duplicate.ts index 4a04a52..9d80012 100644 --- a/src/triage/duplicate.ts +++ b/src/triage/duplicate.ts @@ -87,7 +87,7 @@ async function findDuplicatesWithAI( .replace('{existingTitle}', candidate.title) .replace('{existingBody}', candidate.body || '(empty)'); - const response = (await ai.complete(prompt)).trim(); + const response = (await ai.complete(prompt)).text.trim(); const score = parseFloat(response); if (!isNaN(score) && score >= 0 && score <= 1 && score >= threshold) { diff --git a/src/triage/responder.ts b/src/triage/responder.ts index 610e620..ce13664 100644 --- a/src/triage/responder.ts +++ b/src/triage/responder.ts @@ -110,7 +110,7 @@ async function generateComment( .replace('{category}', category); try { - const response = (await ai.complete(prompt)).trim(); + const response = (await ai.complete(prompt)).text.trim(); // Sanity check: if AI returns something too short or suspicious, use fallback if (response.length < 20) { return buildFallbackComment(category, title); diff --git a/tests/classifier.test.ts b/tests/classifier.test.ts index 48450fb..288248d 100644 --- a/tests/classifier.test.ts +++ b/tests/classifier.test.ts @@ -3,7 +3,7 @@ import { classifyIssue, categoryToLabel, isVagueIssue } from '../src/triage/clas import type { AIProvider } from '../src/ai/provider.js'; function mockAI(response: string): AIProvider { - return { complete: async () => response }; + return { complete: async () => ({ text: response, usage: { inputTokens: 0, outputTokens: 0 } }) }; } describe('isVagueIssue', () => { diff --git a/tests/duplicate.test.ts b/tests/duplicate.test.ts index bf89797..582ccf5 100644 --- a/tests/duplicate.test.ts +++ b/tests/duplicate.test.ts @@ -14,17 +14,19 @@ const installIssues = [ { number: 4, title: 'Add dark mode support', body: 'Please add dark mode theme to the settings page' }, ]; +const usage = { inputTokens: 0, outputTokens: 0 }; + function mockAI(response: string): AIProvider { - return { complete: async () => response }; + return { complete: async () => ({ text: response, usage }) }; } function mockAIDynamic(responses: Map): AIProvider { return { complete: async (prompt: string) => { for (const [key, value] of responses) { - if (prompt.includes(key)) return value; + if (prompt.includes(key)) return { text: value, usage }; } - return '0.1'; + return { text: '0.1', usage }; }, }; } @@ -168,7 +170,7 @@ describe('findDuplicates with AI', () => { const countingAI: AIProvider = { complete: async () => { aiCallCount++; - return '0.3'; + return { text: '0.3', usage }; }, }; diff --git a/tests/responder.test.ts b/tests/responder.test.ts index a8cb281..29045ed 100644 --- a/tests/responder.test.ts +++ b/tests/responder.test.ts @@ -4,16 +4,18 @@ import type { AIProvider } from '../src/ai/provider.js'; import type { GitHubClient } from '../src/github/client.js'; import type { RepoKeeperConfig } from '../src/config.js'; +const usage = { inputTokens: 0, outputTokens: 0 }; + function createMockAI(classifyResponse: string, commentResponse: string): AIProvider { let callCount = 0; return { complete: async (prompt: string) => { // Duplicate detection calls come first, then classify, then comment - if (prompt.includes('duplicate issue detector')) return '0.1'; + if (prompt.includes('duplicate issue detector')) return { text: '0.1', usage }; callCount++; // Even calls = classify, odd calls = comment (roughly) - if (prompt.includes('classifier') || prompt.includes('Classify')) return classifyResponse; - return commentResponse; + if (prompt.includes('classifier') || prompt.includes('Classify')) return { text: classifyResponse, usage }; + return { text: commentResponse, usage }; }, }; } @@ -116,8 +118,8 @@ describe('handleIssueOpened', () => { it('flags duplicates with possible-duplicate label instead of closing', async () => { const ai: AIProvider = { complete: async (prompt: string) => { - if (prompt.includes('duplicate issue detector')) return '0.85'; - return 'comment'; + if (prompt.includes('duplicate issue detector')) return { text: '0.85', usage }; + return { text: 'comment', usage }; }, }; const github = createMockGithub(); diff --git a/tests/reviewer.test.ts b/tests/reviewer.test.ts index bbf1d3b..aeb3f82 100644 --- a/tests/reviewer.test.ts +++ b/tests/reviewer.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { parseImports } from '../src/review/context-builder.js'; import { parseDiffHunks, isAlreadyReviewed, markReviewed, getReviewedShas, cleanupPR } from '../src/review/hunk-tracker.js'; import { getAcceptedPatterns, addAcceptedPattern, learnFromMergedPR, formatAcceptedPatternsPrompt } from '../src/review/memory.js'; +import { matchesIgnorePattern, filterIgnoredFiles, buildCommitStatus } from '../src/review/reviewer.js'; import { existsSync, unlinkSync, mkdirSync } from 'node:fs'; // --- context-builder tests --- @@ -270,6 +271,132 @@ describe('comment-poster', () => { }); }); +// --- review file filtering tests --- + +describe('matchesIgnorePattern', () => { + it('matches exact file paths', () => { + expect(matchesIgnorePattern('package-lock.json', 'package-lock.json')).toBe(true); + }); + + it('matches single wildcard patterns', () => { + expect(matchesIgnorePattern('src/utils.test.ts', 'src/*.test.ts')).toBe(true); + expect(matchesIgnorePattern('src/deep/utils.test.ts', 'src/*.test.ts')).toBe(false); + }); + + it('matches globstar patterns', () => { + expect(matchesIgnorePattern('src/deep/nested/file.test.ts', '**/*.test.ts')).toBe(true); + expect(matchesIgnorePattern('file.test.ts', '**/*.test.ts')).toBe(true); + }); + + it('matches directory globstar patterns', () => { + expect(matchesIgnorePattern('dist/index.js', 'dist/**')).toBe(true); + expect(matchesIgnorePattern('dist/sub/file.js', 'dist/**')).toBe(true); + expect(matchesIgnorePattern('src/dist/file.js', 'dist/**')).toBe(false); + }); + + it('matches single character wildcard', () => { + expect(matchesIgnorePattern('src/a.ts', 'src/?.ts')).toBe(true); + expect(matchesIgnorePattern('src/ab.ts', 'src/?.ts')).toBe(false); + }); + + it('does not match unrelated files', () => { + expect(matchesIgnorePattern('src/index.ts', '**/*.test.ts')).toBe(false); + expect(matchesIgnorePattern('README.md', 'dist/**')).toBe(false); + }); + + it('handles dots in patterns correctly', () => { + expect(matchesIgnorePattern('.env', '.env')).toBe(true); + expect(matchesIgnorePattern('xenv', '.env')).toBe(false); + }); + + it('escapes regex metacharacters in patterns', () => { + expect(matchesIgnorePattern('src/file[1].ts', 'src/file[1].ts')).toBe(true); + expect(matchesIgnorePattern('src/file(test).ts', 'src/file(test).ts')).toBe(true); + expect(matchesIgnorePattern('src/file+extra.ts', 'src/file+extra.ts')).toBe(true); + expect(matchesIgnorePattern('src/fileX.ts', 'src/file[1].ts')).toBe(false); + }); +}); + +describe('filterIgnoredFiles', () => { + const files = [ + 'src/index.ts', + 'src/utils.test.ts', + 'dist/index.js', + 'package-lock.json', + 'docs/README.md', + ]; + + it('returns all files when no ignore patterns', () => { + expect(filterIgnoredFiles(files, [])).toEqual(files); + }); + + it('filters files matching a single pattern', () => { + const result = filterIgnoredFiles(files, ['dist/**']); + expect(result).not.toContain('dist/index.js'); + expect(result).toHaveLength(4); + }); + + it('filters files matching multiple patterns', () => { + const result = filterIgnoredFiles(files, ['dist/**', 'package-lock.json']); + expect(result).not.toContain('dist/index.js'); + expect(result).not.toContain('package-lock.json'); + expect(result).toHaveLength(3); + }); + + it('filters test files with globstar', () => { + const result = filterIgnoredFiles(files, ['**/*.test.ts']); + expect(result).not.toContain('src/utils.test.ts'); + expect(result).toContain('src/index.ts'); + }); + + it('returns all files when patterns match nothing', () => { + const result = filterIgnoredFiles(files, ['**/*.xyz']); + expect(result).toEqual(files); + }); + + it('handles undefined/empty ignore patterns gracefully', () => { + expect(filterIgnoredFiles(files, undefined as unknown as string[])).toEqual(files); + }); +}); + +// --- commit status tests --- + +describe('buildCommitStatus', () => { + it('returns failure when BLOCKING findings exist', () => { + const result = buildCommitStatus([ + { file: 'a.ts', line: 1, severity: 'BLOCKING', message: 'Security issue' }, + { file: 'b.ts', line: 2, severity: 'WARNING', message: 'Code smell' }, + ]); + expect(result.state).toBe('failure'); + expect(result.description).toBe('1 blocking issue(s) found'); + }); + + it('counts multiple BLOCKING findings', () => { + const result = buildCommitStatus([ + { file: 'a.ts', line: 1, severity: 'BLOCKING', message: 'SQL injection' }, + { file: 'b.ts', line: 2, severity: 'BLOCKING', message: 'Hardcoded secret' }, + { file: 'c.ts', line: 3, severity: 'WARNING', message: 'Missing error handling' }, + ]); + expect(result.state).toBe('failure'); + expect(result.description).toBe('2 blocking issue(s) found'); + }); + + it('returns success with count for non-blocking findings only', () => { + const result = buildCommitStatus([ + { file: 'a.ts', line: 1, severity: 'WARNING', message: 'Code smell' }, + { file: 'b.ts', line: 2, severity: 'SUGGESTION', message: 'Style' }, + ]); + expect(result.state).toBe('success'); + expect(result.description).toBe('2 non-blocking finding(s)'); + }); + + it('returns success with clean message when no findings', () => { + const result = buildCommitStatus([]); + expect(result.state).toBe('success'); + expect(result.description).toBe('No issues found'); + }); +}); + // --- integration-style test for the reviewer module --- describe('reviewer module', () => { diff --git a/tests/summariser.test.ts b/tests/summariser.test.ts index cdbefa4..7a7a832 100644 --- a/tests/summariser.test.ts +++ b/tests/summariser.test.ts @@ -4,7 +4,7 @@ import type { AIProvider } from '../src/ai/provider.js'; import type { RepoKeeperConfig } from '../src/config.js'; function mockAI(response: string): AIProvider { - return { complete: vi.fn().mockResolvedValue(response) }; + return { complete: vi.fn().mockResolvedValue({ text: response, usage: { inputTokens: 0, outputTokens: 0 } }) }; } function mockGitHub() {