From 0996073debbbf2643c9432ce138dda180cfe6dfa Mon Sep 17 00:00:00 2001 From: Lukas May Date: Fri, 6 Mar 2026 19:51:04 +0100 Subject: [PATCH] feat: add in-memory diff cache with TTL and commit-hash invalidation Adds DiffCache module, extends BranchManager with getHeadCommitHash, and wires phase-level caching into getPhaseReviewDiff and getFileDiff. Cache is invalidated in ExecutionOrchestrator after each task merges into the phase branch, ensuring stale diffs are never served after new commits. Co-Authored-By: Claude Sonnet 4.6 --- apps/server/execution/orchestrator.test.ts | 1 + apps/server/execution/orchestrator.ts | 5 ++ apps/server/git/branch-manager.ts | 5 ++ apps/server/git/simple-git-branch-manager.ts | 6 ++ apps/server/review/diff-cache.ts | 67 ++++++++++++++++++++ apps/server/trpc/routers/phase.ts | 32 ++++++++-- 6 files changed, 112 insertions(+), 4 deletions(-) create mode 100644 apps/server/review/diff-cache.ts diff --git a/apps/server/execution/orchestrator.test.ts b/apps/server/execution/orchestrator.test.ts index ebf6ea2..3bbf1a1 100644 --- a/apps/server/execution/orchestrator.test.ts +++ b/apps/server/execution/orchestrator.test.ts @@ -48,6 +48,7 @@ function createMocks() { diffBranches: vi.fn().mockResolvedValue(''), diffBranchesStat: vi.fn().mockResolvedValue([]), diffFileSingle: vi.fn().mockResolvedValue(''), + getHeadCommitHash: vi.fn().mockResolvedValue('deadbeef00000000000000000000000000000000'), deleteBranch: vi.fn(), branchExists: vi.fn().mockResolvedValue(true), remoteBranchExists: vi.fn().mockResolvedValue(false), diff --git a/apps/server/execution/orchestrator.ts b/apps/server/execution/orchestrator.ts index 5b8c521..452d017 100644 --- a/apps/server/execution/orchestrator.ts +++ b/apps/server/execution/orchestrator.ts @@ -23,6 +23,7 @@ import type { ConflictResolutionService } from '../coordination/conflict-resolut import { phaseBranchName, taskBranchName } from '../git/branch-naming.js'; import { ensureProjectClone } from '../git/project-clones.js'; import { createModuleLogger } from '../logger/index.js'; +import { phaseMetaCache, fileDiffCache } from '../review/diff-cache.js'; const log = createModuleLogger('execution-orchestrator'); @@ -249,6 +250,10 @@ export class ExecutionOrchestrator { log.info({ taskId, taskBranch, phaseBranch, project: project.name }, 'task branch merged into phase branch'); } + // Invalidate diff cache — phase branch HEAD has advanced after merges + phaseMetaCache.invalidateByPrefix(`${phaseId}:`); + fileDiffCache.invalidateByPrefix(`${phaseId}:`); + // Emit task:merged event const mergedEvent: TaskMergedEvent = { type: 'task:merged', diff --git a/apps/server/git/branch-manager.ts b/apps/server/git/branch-manager.ts index dc7d174..fd94ebb 100644 --- a/apps/server/git/branch-manager.ts +++ b/apps/server/git/branch-manager.ts @@ -45,6 +45,11 @@ export interface BranchManager { */ diffFileSingle(repoPath: string, baseBranch: string, headBranch: string, filePath: string): Promise; + /** + * Returns the current HEAD commit hash (40-char SHA) for the given branch in the repo. + */ + getHeadCommitHash(repoPath: string, branch: string): Promise; + /** * Delete a branch. No-op if the branch doesn't exist. */ diff --git a/apps/server/git/simple-git-branch-manager.ts b/apps/server/git/simple-git-branch-manager.ts index 7e96848..74a1e83 100644 --- a/apps/server/git/simple-git-branch-manager.ts +++ b/apps/server/git/simple-git-branch-manager.ts @@ -267,6 +267,12 @@ export class SimpleGitBranchManager implements BranchManager { return result.trim(); } + async getHeadCommitHash(repoPath: string, branch: string): Promise { + const git = simpleGit(repoPath); + const result = await git.raw(['rev-parse', branch]); + return result.trim(); + } + async pushBranch(repoPath: string, branch: string, remote = 'origin'): Promise { const git = simpleGit(repoPath); try { diff --git a/apps/server/review/diff-cache.ts b/apps/server/review/diff-cache.ts new file mode 100644 index 0000000..312734d --- /dev/null +++ b/apps/server/review/diff-cache.ts @@ -0,0 +1,67 @@ +/** + * DiffCache — in-memory cache with TTL and prefix-based invalidation. + * Used to avoid re-running expensive git diff subprocesses on repeated requests. + */ + +import type { FileStatEntry } from '../git/types.js'; + +interface CacheEntry { + value: T; + expiresAt: number; +} + +export class DiffCache { + private store = new Map>(); + private ttlMs: number; + + constructor(ttlMs: number) { + this.ttlMs = ttlMs; + } + + get(key: string): T | undefined { + const entry = this.store.get(key); + if (!entry) return undefined; + if (Date.now() > entry.expiresAt) { + this.store.delete(key); + return undefined; + } + return entry.value; + } + + set(key: string, value: T): void { + this.store.set(key, { value, expiresAt: Date.now() + this.ttlMs }); + } + + invalidateByPrefix(prefix: string): void { + for (const key of this.store.keys()) { + if (key.startsWith(prefix)) this.store.delete(key); + } + } +} + +// --------------------------------------------------------------------------- +// Response shapes (mirror the return types of getPhaseReviewDiff / getFileDiff) +// --------------------------------------------------------------------------- + +export interface PhaseMetaResponse { + phaseName: string; + sourceBranch: string; + targetBranch: string; + files: FileStatEntry[]; + totalAdditions: number; + totalDeletions: number; +} + +export interface FileDiffResponse { + binary: boolean; + rawDiff: string; +} + +// --------------------------------------------------------------------------- +// Singleton instances — TTL is read once at module load time +// --------------------------------------------------------------------------- + +const TTL = parseInt(process.env.REVIEW_DIFF_CACHE_TTL_MS ?? '300000', 10); + +export const phaseMetaCache = new DiffCache(TTL); +export const fileDiffCache = new DiffCache(TTL); diff --git a/apps/server/trpc/routers/phase.ts b/apps/server/trpc/routers/phase.ts index 0d9c999..8e84beb 100644 --- a/apps/server/trpc/routers/phase.ts +++ b/apps/server/trpc/routers/phase.ts @@ -11,6 +11,7 @@ import { requirePhaseRepository, requireTaskRepository, requireBranchManager, re import { phaseBranchName } from '../../git/branch-naming.js'; import { ensureProjectClone } from '../../git/project-clones.js'; import type { FileStatEntry } from '../../git/types.js'; +import { phaseMetaCache, fileDiffCache } from '../../review/diff-cache.js'; export function phaseProcedures(publicProcedure: ProcedureBuilder) { return { @@ -237,6 +238,16 @@ export function phaseProcedures(publicProcedure: ProcedureBuilder) { const projects = await projectRepo.findProjectsByInitiativeId(phase.initiativeId); const files: FileStatEntry[] = []; + if (projects.length === 0) { + return { phaseName: phase.name, sourceBranch: phBranch, targetBranch: initBranch, files: [], totalAdditions: 0, totalDeletions: 0 }; + } + + const firstClone = await ensureProjectClone(projects[0], ctx.workspaceRoot!); + const headHash = await branchManager.getHeadCommitHash(firstClone, phBranch); + const cacheKey = `${input.phaseId}:${headHash}`; + const cached = phaseMetaCache.get(cacheKey); + if (cached) return cached; + for (const project of projects) { const clonePath = await ensureProjectClone(project, ctx.workspaceRoot!); const entries = await branchManager.diffBranchesStat(clonePath, diffBase, phBranch); @@ -255,7 +266,7 @@ export function phaseProcedures(publicProcedure: ProcedureBuilder) { const totalAdditions = files.reduce((sum, f) => sum + f.additions, 0); const totalDeletions = files.reduce((sum, f) => sum + f.deletions, 0); - return { + const result = { phaseName: phase.name, sourceBranch: phBranch, targetBranch: initBranch, @@ -263,6 +274,8 @@ export function phaseProcedures(publicProcedure: ProcedureBuilder) { totalAdditions, totalDeletions, }; + phaseMetaCache.set(cacheKey, result); + return result; }), getFileDiff: publicProcedure @@ -297,6 +310,13 @@ export function phaseProcedures(publicProcedure: ProcedureBuilder) { const decodedPath = decodeURIComponent(input.filePath); const projects = await projectRepo.findProjectsByInitiativeId(phase.initiativeId); + + const firstClone = await ensureProjectClone(projects[0], ctx.workspaceRoot!); + const headHash = await branchManager.getHeadCommitHash(firstClone, phBranch); + const cacheKey = `${input.phaseId}:${headHash}:${input.filePath}`; + const cached = fileDiffCache.get(cacheKey); + if (cached) return cached; + let clonePath: string; if (input.projectId) { const project = projects.find((p) => p.id === input.projectId); @@ -305,18 +325,22 @@ export function phaseProcedures(publicProcedure: ProcedureBuilder) { } clonePath = await ensureProjectClone(project, ctx.workspaceRoot!); } else { - clonePath = await ensureProjectClone(projects[0], ctx.workspaceRoot!); + clonePath = firstClone; } const git = simpleGit(clonePath); // Binary files appear as "-\t-\t" in --numstat output const numstatOut = await git.raw(['diff', '--numstat', `${diffBase}...${phBranch}`, '--', decodedPath]); if (numstatOut.trim() && numstatOut.startsWith('-\t-\t')) { - return { binary: true, rawDiff: '' }; + const binaryResult = { binary: true, rawDiff: '' }; + fileDiffCache.set(cacheKey, binaryResult); + return binaryResult; } const rawDiff = await branchManager.diffFileSingle(clonePath, diffBase, phBranch, decodedPath); - return { binary: false, rawDiff }; + const result = { binary: false, rawDiff }; + fileDiffCache.set(cacheKey, result); + return result; }), approvePhaseReview: publicProcedure