From 6a95acf58d0c8fbc1ae766c707664e8528ed094f Mon Sep 17 00:00:00 2001 From: Matt Alonso Date: Mon, 20 Jul 2026 16:25:31 -0500 Subject: [PATCH 1/8] Define provider-neutral review-history contracts Introduce forge-neutral revision, range, comparison, evolution, version, and review-plan types plus the constructors and plan-resolution helpers that operate on them. Keeping this domain model independent gives Codiff Web and provider adapters a stable contract before any shared review UI is introduced. --- core/__tests__/review-history.test.ts | 113 +++++++ core/lib/review-history.ts | 181 +++++++++++ core/types.ts | 414 +++++++++++++++++++++++++- 3 files changed, 693 insertions(+), 15 deletions(-) create mode 100644 core/__tests__/review-history.test.ts create mode 100644 core/lib/review-history.ts diff --git a/core/__tests__/review-history.test.ts b/core/__tests__/review-history.test.ts new file mode 100644 index 00000000..ab5a1b4b --- /dev/null +++ b/core/__tests__/review-history.test.ts @@ -0,0 +1,113 @@ +import { expect, test } from 'vite-plus/test'; +import { + commitRevisionLabel, + diffComparison, + diffRange, + resolveReviewPlan, + reviewableUnits, + revisionRef, + reviewVersionOption, + versionOptionHeadCommitId, + versionRevisionLabel, +} from '../lib/review-history.ts'; +import type { ReviewEvolutionUnit } from '../types.ts'; + +const base = revisionRef('a'.repeat(40), commitRevisionLabel('base')); +const headOld = revisionRef('b'.repeat(40), versionRevisionLabel('v1')); +const headNew = revisionRef('c'.repeat(40), versionRevisionLabel('v2')); + +const before = diffRange(base, headOld); +const after = diffRange(base, headNew); +const comparison = diffComparison(before, after); + +const units: ReadonlyArray = [ + { + after: { + authoredAt: '2026-01-01T00:00:00.000Z', + authorName: 'A', + parentIds: [base.commitId], + sha: headNew.commitId, + shortSha: headNew.commitId.slice(0, 7), + subject: 'feat: add', + }, + confidence: 'high', + id: 'introduced-1', + kind: 'introduced', + order: 0, + reviewable: true, + }, + { + after: { + authoredAt: '2026-01-01T00:00:00.000Z', + authorName: 'A', + parentIds: [base.commitId], + sha: headOld.commitId, + shortSha: headOld.commitId.slice(0, 7), + subject: 'same', + }, + before: { + authoredAt: '2026-01-01T00:00:00.000Z', + authorName: 'A', + parentIds: [base.commitId], + sha: headOld.commitId, + shortSha: headOld.commitId.slice(0, 7), + subject: 'same', + }, + confidence: 'exact', + id: 'retained-1', + kind: 'retained', + order: 1, + reviewable: false, + }, +]; + +test('projects version options through DiffRange identity', () => { + const version = reviewVersionOption({ + createdAt: '2026-01-02T00:00:00.000Z', + id: '2', + number: 2, + range: after, + }); + expect(versionOptionHeadCommitId(version)).toBe(headNew.commitId); + expect(version.range.base.commitId).toBe(base.commitId); +}); + +test('filters non-reviewable evolution markers from unit plans', () => { + expect(reviewableUnits(units).map((unit) => unit.kind)).toEqual(['introduced']); +}); + +test('resolves whole-diff and unit plans from recommendations', () => { + expect( + resolveReviewPlan({ + comparison, + recommendation: { + rationale: 'Low pairing confidence.', + suggestedStructure: 'whole-diff', + }, + units, + }), + ).toMatchObject({ structure: 'whole-diff' }); + + expect( + resolveReviewPlan({ + comparison, + recommendation: { + rationale: 'Review introduced units.', + suggestedStructure: 'commit-by-commit', + }, + structure: 'auto', + units, + }), + ).toMatchObject({ + structure: 'units', + units: [{ kind: 'introduced' }], + }); + + expect( + resolveReviewPlan({ + comparison, + structure: 'whole-diff', + units, + }), + ).toMatchObject({ structure: 'whole-diff' }); +}); diff --git a/core/lib/review-history.ts b/core/lib/review-history.ts new file mode 100644 index 00000000..754b74f7 --- /dev/null +++ b/core/lib/review-history.ts @@ -0,0 +1,181 @@ +import type { + DiffComparison, + DiffComparisonAnalysis, + DiffComparisonView, + DiffRange, + ReviewCommitEvolution, + ReviewEvolutionUnit, + ReviewPlan, + ReviewStructureRecommendation, + ReviewUnit, + ReviewVersionOption, + RevisionLabel, + RevisionRef, +} from '../types.ts'; + +/** Create a commit-scoped revision label. */ +export const commitRevisionLabel = (text: string, url?: string): RevisionLabel => ({ + kind: 'commit', + text, + ...(url ? { url } : {}), +}); + +/** Create a version-scoped revision label. */ +export const versionRevisionLabel = (text: string, url?: string): RevisionLabel => ({ + kind: 'version', + text, + ...(url ? { url } : {}), +}); + +export const revisionRef = ( + commitId: string, + label: RevisionLabel, + aliases?: ReadonlyArray, +): RevisionRef => ({ + commitId, + label, + ...(aliases?.length ? { aliases } : {}), +}); + +export const diffRange = (base: RevisionRef, head: RevisionRef): DiffRange => ({ base, head }); + +export const diffComparison = (before: DiffRange, after: DiffRange): DiffComparison => ({ + after, + before, +}); + +export const reviewVersionOption = ({ + createdAt, + diffStat, + id, + isHead, + number, + previousCreatedAt, + previousNumber, + range, +}: { + createdAt: string; + diffStat?: ReviewVersionOption['diffStat']; + id: string; + isHead?: boolean; + number?: number; + previousCreatedAt?: string; + previousNumber?: number; + range: DiffRange; +}): ReviewVersionOption => ({ + createdAt, + id, + range, + ...(diffStat ? { diffStat } : {}), + ...(isHead != null ? { isHead } : {}), + ...(number != null ? { number } : {}), + ...(previousCreatedAt ? { previousCreatedAt } : {}), + ...(previousNumber != null ? { previousNumber } : {}), +}); + +export const isReviewableUnit = (unit: ReviewEvolutionUnit): unit is ReviewUnit => + unit.reviewable === true; + +export const reviewableUnits = ( + units: ReadonlyArray, +): ReadonlyArray => units.filter(isReviewableUnit); + +export const resolveReviewPlan = ({ + analysis, + comparison, + recommendation, + structure, + units, +}: { + analysis?: DiffComparisonAnalysis; + comparison?: DiffComparison; + recommendation?: ReviewStructureRecommendation; + structure?: 'commit-by-commit' | 'units' | 'whole-diff' | 'auto'; + units?: ReadonlyArray; +}): ReviewPlan => { + const reviewUnits = units ? reviewableUnits(units) : []; + const resolved = + structure === 'whole-diff' + ? 'whole-diff' + : structure === 'commit-by-commit' || structure === 'units' + ? 'units' + : recommendation?.suggestedStructure === 'commit-by-commit' && reviewUnits.length > 0 + ? 'units' + : 'whole-diff'; + + if (resolved === 'units' && reviewUnits.length > 0) { + return { + structure: 'units', + units: reviewUnits, + ...(analysis ? { analysis } : {}), + ...(comparison ? { comparison } : {}), + }; + } + + return { + structure: 'whole-diff', + ...(analysis ? { analysis } : {}), + ...(comparison ? { comparison } : {}), + }; +}; + +export const diffComparisonView = ({ + analysis, + comparison, + files, + from, + to, +}: { + analysis: DiffComparisonAnalysis; + comparison: DiffComparison; + files: DiffComparisonView['files']; + from: ReviewVersionOption; + to: ReviewVersionOption; +}): DiffComparisonView => ({ + analysis, + comparison, + files, + from, + to, +}); + +export const reviewCommitEvolution = (evolution: ReviewCommitEvolution): ReviewCommitEvolution => + evolution; + +export const versionOptionHeadCommitId = (version: ReviewVersionOption) => + version.range.head.commitId; + +export const versionOptionBaseCommitId = (version: ReviewVersionOption) => + version.range.base.commitId; + +export const versionOptionLabelText = (version: ReviewVersionOption) => + version.number != null + ? version.number === 0 + ? 'Base' + : `v${version.number}` + : version.range.head.label.text; + +export const evolutionUnitCommit = (unit: ReviewEvolutionUnit) => { + if (unit.kind === 'commit') { + return unit.commit; + } + if (unit.kind === 'introduced') { + return unit.after; + } + if (unit.kind === 'removed') { + return unit.before; + } + if (unit.kind === 'revised') { + return unit.after ?? unit.before; + } + if (unit.kind === 'ambiguous') { + return unit.after ?? unit.before; + } + if (unit.kind === 'absorbed-into-base') { + return unit.baseCommit ?? unit.before ?? unit.after; + } + return unit.after ?? unit.before; +}; + +export const evolutionUnitRebaseDrivers = (unit: ReviewEvolutionUnit) => + unit.kind === 'revised' ? (unit.rebaseDrivers ?? []) : []; diff --git a/core/types.ts b/core/types.ts index 2682715f..161ccd1c 100644 --- a/core/types.ts +++ b/core/types.ts @@ -125,16 +125,8 @@ export type ReviewSource = type: 'branch-diff'; } | { - /** - * Resolved base commit for the branch part of the comparison. Optional - * because the CLI can construct this source from just a branch name - * (`codiff main`), before merge-base resolution happens; the resolved - * state's `source` always carries a concrete value. - */ baseRef?: string; - /** Resolved head commit for the branch part of the comparison. See {@link baseRef}. */ headRef?: string; - /** Target branch the current branch was compared against. */ ref: string; type: 'branch-working-tree'; } @@ -365,8 +357,6 @@ export type SharedPlanSnapshot = { version: 1; }; -export type WalkthroughShareManifestV1 = SharedWalkthroughSnapshot; - export type ShareResult = | { status: 'uploaded'; @@ -378,8 +368,330 @@ export type ShareResult = }; export type SharePlanResult = ShareResult; + +export type WalkthroughShareManifestV1 = SharedWalkthroughSnapshot; + +export type ReviewPreferences = Pick< + CodiffPreferences, + 'codeFontFamily' | 'codeFontSize' | 'diffStyle' | 'showWhitespace' | 'theme' | 'wordWrap' +>; export type ShareWalkthroughResult = ShareResult; +/** + * Mutable display label for a revision. + * + * Labels can move or be renumbered (bookmarks, version tags). They are for + * display and navigation only and must not be treated as durable identity. + */ +export type RevisionLabel = { + kind: 'bookmark' | 'branch' | 'commit' | 'review-marker' | 'tag' | 'version'; + text: string; + /** Optional provider web URL for this label (for example a commit page). */ + url?: string; +}; + +/** + * Provider-neutral revision identity for this iteration of Codiff. + * + * `commitId` is the full Git commit SHA. Provider-specific extras such as a + * GitLab `startSha` stay inside the provider adapter and are projected into + * this shape at the host boundary. + */ +export type RevisionRef = { + /** Additional display labels that resolve to the same commit. */ + aliases?: ReadonlyArray; + /** Full Git commit SHA for this iteration. */ + commitId: string; + label: RevisionLabel; +}; + +/** + * Provider-neutral base/head review range. + * + * Contains only review semantics shared across hosts. GitLab retains its full + * `{ baseSha, startSha, headSha }` identity inside the GitLab adapter for + * historical fetches, cache keys, and comment positions. `startSha` is not + * authoring input. + */ +export type DiffRange = { + base: RevisionRef; + head: RevisionRef; +}; + +/** Identity of a before/after review comparison. */ +export type DiffComparison = { + after: DiffRange; + before: DiffRange; +}; + +export type DiffComparisonCommentAssociation = { + commentId: string; + filePath?: string; + status: 'newly-anchored' | 'outdated' | 'resolved-by-change' | 'still-valid'; +}; + +export type DiffComparisonBaseMovementCommit = { + authoredAt: string; + authorName: string; + sha: string; + shortSha: string; + subject: string; + webUrl?: string; +}; + +export type DiffComparisonBaseMovement = { + changed: boolean; + commits?: ReadonlyArray; + commitsBetween: number | null; + commitTimestampDeltaMs: number | null; + diffStat: { additions: number; deletions: number; filesChanged: number } | null; + from: { committedAt: string | null; sha: string; shortSha: string; webUrl?: string }; + relationship: 'forward' | 'backward' | 'divergent' | 'unknown'; + to: { committedAt: string | null; sha: string; shortSha: string; webUrl?: string }; + truncated: boolean; + warning?: string; +}; + +export type DiffComparisonSummary = { + addedLines: number; + baseMoved: boolean; + commentsAffected: number; + conflictFiles: number; + deletedLines: number; + empty: boolean; + filesChanged: number; + intentionalFiles: number; + noiseFiles: number; +}; + +export type ReviewCommitSummary = { + authoredAt: string; + authorName: string; + diffStat?: { additions: number; deletions: number; filesChanged: number }; + parentIds: ReadonlyArray; + sha: string; + shortSha: string; + subject: string; + webUrl?: string; +}; + +export type ReviewRebaseDriverCommit = { + authoredAt: string; + authorName: string; + overlappingPaths: ReadonlyArray; + sha: string; + shortSha: string; + subject: string; + webUrl?: string; +}; + +/** + * Discriminated reviewable work unit. + * + * Units are generation targets only. Non-reviewable stack markers such as + * retained or absorbed commits live on {@link ReviewEvolutionUnit}. + */ +export type ReviewUnit = + | { + commit: ReviewCommitSummary; + id: string; + /** Parent-to-commit range for an ordinary MR commit. */ + kind: 'commit'; + order: number; + reviewable: true; + } + | { + after: ReviewCommitSummary; + confidence: 'exact' | 'high' | 'unmatched'; + id: string; + /** Newly added range between comparison endpoints. */ + kind: 'introduced'; + matchReasons?: ReadonlyArray; + matchScore?: number; + order: number; + reviewable: true; + } + | { + before: ReviewCommitSummary; + confidence: 'exact' | 'high' | 'unmatched'; + id: string; + /** Reverse materialization of an earlier range removed from the stack. */ + kind: 'removed'; + matchReasons?: ReadonlyArray; + matchScore?: number; + order: number; + reviewable: true; + } + | { + after: ReviewCommitSummary; + before: ReviewCommitSummary; + confidence: 'exact' | 'high' | 'unmatched'; + id: string; + /** Comparison of earlier and later commit ranges for one logical change. */ + kind: 'revised'; + matchReasons?: ReadonlyArray; + matchScore?: number; + order: number; + rebaseDrivers?: ReadonlyArray; + reviewable: true; + } + | { + after?: ReviewCommitSummary; + before?: ReviewCommitSummary; + confidence: 'exact' | 'high' | 'unmatched'; + id: string; + /** Authoritative fallback range when pairing is uncertain. */ + kind: 'ambiguous'; + matchReasons?: ReadonlyArray; + matchScore?: number; + order: number; + reviewable: true; + }; + +/** Non-reviewable evolution markers retained for stack display. */ +export type ReviewEvolutionMarkerUnit = { + after?: ReviewCommitSummary; + baseCommit?: ReviewCommitSummary; + before?: ReviewCommitSummary; + confidence: 'exact' | 'high' | 'unmatched'; + id: string; + kind: 'retained' | 'rewritten-same-patch' | 'absorbed-into-base'; + matchReasons?: ReadonlyArray; + matchScore?: number; + order: number; + reviewable: false; +}; + +export type ReviewEvolutionUnit = ReviewUnit | ReviewEvolutionMarkerUnit; + +export type ReviewEvolutionSummary = { + absorbedIntoBase: number; + added: number; + ambiguous: number; + pairingCoverage: number; + removed: number; + retained: number; + reviewable: number; + revised: number; + rewrittenSamePatch: number; +}; + +/** + * Provider recommendation only. Hosts may override before resolving a + * {@link ReviewPlan}. + */ +export type ReviewStructureRecommendation = { + confidence?: number; + rationale: string; + suggestedStructure: 'commit-by-commit' | 'whole-diff'; +}; + +export type ReviewCommitEvolution = { + recommendation: ReviewStructureRecommendation; + summary: ReviewEvolutionSummary; + units: ReadonlyArray; + warnings?: ReadonlyArray; +}; + +/** + * Derived comparison data kept separate from {@link DiffComparison} identity. + */ +export type DiffComparisonAnalysis = { + baseMovement?: DiffComparisonBaseMovement; + commentAssociations?: ReadonlyArray; + commitEvolution?: ReviewCommitEvolution; + summary: DiffComparisonSummary; + warnings?: ReadonlyArray; +}; + +/** + * Resolved generation plan. Distinct from {@link ReviewStructureRecommendation}. + * + * - `whole-diff` materializes one {@link RepositoryState} + * - `units` materializes one {@link RepositoryState} per {@link ReviewUnit} + */ +export type ReviewPlan = + | { + analysis?: DiffComparisonAnalysis; + comparison?: DiffComparison; + structure: 'whole-diff'; + } + | { + analysis?: DiffComparisonAnalysis; + comparison?: DiffComparison; + structure: 'units'; + units: ReadonlyArray; + }; + +/** + * Host-facing version row for review history pickers. + * + * Durable identity is `id` plus the projected {@link DiffRange}. Provider + * internals such as GitLab `startSha` remain in the adapter. + */ +export type ReviewVersionOption = { + createdAt: string; + diffStat?: { additions: number; deletions: number; filesChanged: number }; + id: string; + isHead?: boolean; + number?: number; + previousCreatedAt?: string; + previousNumber?: number; + range: DiffRange; +}; + +/** Materialized comparison view consumed by shared review UI. */ +export type DiffComparisonView = { + analysis: DiffComparisonAnalysis; + comparison: DiffComparison; + files: ReadonlyArray; + from: ReviewVersionOption; + to: ReviewVersionOption; +}; + +export type ReviewCommitListEntry = { + authoredAt: string; + authorName: string; + role?: string; + sha: string; + shortSha: string; + subject: string; + webUrl?: string; +}; + +export type ReviewStrategySummary = { + confidence: number; + mode: 'commit-by-commit' | 'whole-diff'; + reason: string; +}; + +/** + * Top-level walkthrough authoring context. + * + * A range target can use a whole-diff or ordinary commit-unit plan. A + * comparison target requires comparison analysis and can use a whole-diff or + * evolution-unit plan. + */ +export type WalkthroughGenerationInput = + | { + kind: 'range'; + plan: ReviewPlan; + range: DiffRange; + /** Present for whole-diff plans and as the aggregate state for composition. */ + state: RepositoryState; + /** Present for unit plans: one materialized state per review unit. */ + unitStates?: ReadonlyArray<{ state: RepositoryState; unit: ReviewUnit }>; + } + | { + analysis: DiffComparisonAnalysis; + comparison: DiffComparison; + kind: 'comparison'; + plan: ReviewPlan; + /** Present for whole-diff comparison plans. */ + state?: RepositoryState; + unitStates?: ReadonlyArray<{ state: RepositoryState; unit: ReviewUnit }>; + }; + export type WalkthroughContext = { changedFiles?: ReadonlyArray<{ path: string; @@ -525,7 +837,19 @@ export type WalkthroughHunkGroup = { }; /** One stop in the main walkthrough path. */ +export type WalkthroughCommentReference = { + authorName: string; + body: string; + filePath: string; + id: string; + lineNumber?: number; + status: 'newly-anchored' | 'outdated' | 'resolved-by-change' | 'still-valid'; + url?: string; +}; + export type WalkthroughStop = WalkthroughHunkGroup & { + /** Review comments whose anchored code region overlaps this stop. */ + commentReferences?: ReadonlyArray; importance: 'critical' | 'normal' | 'context'; /** Agent narration (markdown / inline code). */ prose: string; @@ -541,6 +865,28 @@ export type WalkthroughSupportGroup = WalkthroughHunkGroup & { /** A named chapter in the walkthrough. */ export type WalkthroughChapter = { blurb: string; + /** Commit boundary metadata for commit-by-commit walkthroughs. */ + commit?: { + gitSha?: string; + /** + * Base-branch commits that likely forced this MR commit rewrite on rebase. + * Present for version-comparison commit-by-commit units when attribution exists. + */ + rebaseDrivers?: ReadonlyArray<{ + authoredAt?: string; + authorName?: string; + overlappingPaths?: ReadonlyArray; + sha?: string; + shortSha: string; + subject: string; + webUrl?: string; + }>; + revisionCause?: string; + sha: string; + shortSha: string; + subject: string; + webUrl?: string; + }; icon: WalkthroughIcon; id: string; stops: ReadonlyArray; @@ -572,6 +918,8 @@ export type NarrativeWalkthrough = { * composer at the end of the walkthrough. Stripped unless `source` is a working tree. */ commit?: WalkthroughCommit; + /** Commit-scoped diff files used to resolve per-commit walkthrough hunk anchors. */ + commitFiles?: ReadonlyArray; /** The originating conversation, embedded for in-app Q&A. */ context?: WalkthroughContext; /** 1–2 sentence summary of the change. */ @@ -754,11 +1102,6 @@ export type CodiffPreferences = { wordWrap: boolean; }; -export type ReviewPreferences = Pick< - CodiffPreferences, - 'codeFontFamily' | 'codeFontSize' | 'diffStyle' | 'showWhitespace' | 'theme' | 'wordWrap' ->; - export type PullRequestReviewComment = { anchor?: 'file' | 'line'; body: string; @@ -780,8 +1123,49 @@ export type PullRequestExistingReviewComment = PullRequestReviewComment & { id: string; isOutdated?: boolean; isThreadResolved?: boolean; + /** Diff identity the comment was positioned against (provider SHAs). */ + positionIdentity?: { + baseSha: string; + headSha: string; + startSha: string; + }; + resolution?: { + confidence: 'approximate' | 'exact'; + nearbyHunkContext?: { + after?: string; + before?: string; + }; + versions: ReadonlyArray<{ + fromLabel: string; + toLabel: string; + }>; + }; + submittedAt?: string; + url?: string; + versionAssociation?: 'exact' | 'unmatched'; + versionHeadSha?: string; + versionId?: string; + versionLabel?: string; +}; + +export type PullRequestAIReviewDecision = + | 'approved' + | 'approved-with-comments' + | 'changes-requested' + | 'unknown'; + +export type PullRequestAIReview = { + body: string; + decision: PullRequestAIReviewDecision; + id: string; + reviewedHeadSha?: string; + reviewer: ReviewAuthor & { id: string }; submittedAt?: string; url?: string; + versionAssociation?: 'exact' | 'unmatched'; + versionHeadSha?: string; + versionId?: string; + versionLabel?: string; }; export type PullRequestGeneralComment = { From 85420d66fef138585062e96c20e2f6da4b3af8b5 Mon Sep 17 00:00:00 2001 From: Matt Alonso Date: Mon, 20 Jul 2026 16:25:46 -0500 Subject: [PATCH 2/8] Add shared walkthrough authoring primitives Centralize draft parsing, prompt construction, hunk indexing, normalization, comment attachment, and unit composition in Core. This gives Codiff Web and Electron one deterministic authoring pipeline while leaving model execution, retries, caching, and storage in their respective hosts. --- core/__tests__/walkthrough-authoring.test.ts | 289 +++++ core/lib/walkthrough-authoring.ts | 1158 ++++++++++++++++++ core/package.json | 17 +- core/tsconfig.build.json | 1 + core/walkthrough-authoring.ts | 29 + 5 files changed, 1493 insertions(+), 1 deletion(-) create mode 100644 core/__tests__/walkthrough-authoring.test.ts create mode 100644 core/lib/walkthrough-authoring.ts create mode 100644 core/walkthrough-authoring.ts diff --git a/core/__tests__/walkthrough-authoring.test.ts b/core/__tests__/walkthrough-authoring.test.ts new file mode 100644 index 00000000..dd17e493 --- /dev/null +++ b/core/__tests__/walkthrough-authoring.test.ts @@ -0,0 +1,289 @@ +import { expect, test } from 'vite-plus/test'; +import { + attachVersionCommentReferences, + buildWalkthroughPrompt, + buildWalkthroughPromptInput, + composeUnitWalkthroughs, + indexWalkthroughHunks, + normalizeWalkthroughDraft, + parseWalkthroughDraft, +} from '../lib/walkthrough-authoring.ts'; +import type { RepositoryState } from '../types.ts'; + +const state = { + branch: 'feature/walkthrough', + files: [ + { + fingerprint: 'fingerprint', + path: 'src/app.ts', + sections: [ + { + binary: false, + id: 'src/app.ts:pull-request:42', + kind: 'pull-request', + patch: + 'diff --git a/src/app.ts b/src/app.ts\n--- a/src/app.ts\n+++ b/src/app.ts\n@@ -1 +1 @@\n-old();\n+newCall();\n@@ -10,0 +11 @@\n+test();\n', + }, + ], + status: 'modified', + }, + ], + generatedAt: Date.parse('2026-06-26T00:00:00.000Z'), + launchPath: 'cloudflare/voidzero/codiff', + root: 'cloudflare/voidzero/codiff', + source: { + headSha: 'head-sha', + host: 'gitlab.example.com', + number: 42, + projectPath: 'cloudflare/voidzero/codiff', + provider: 'gitlab', + type: 'pull-request', + url: 'https://gitlab.example.com/cloudflare/voidzero/codiff/-/merge_requests/42', + }, +} satisfies RepositoryState; + +test('indexes stable hunk ids but sends compact aliases in the prompt', () => { + const index = indexWalkthroughHunks(state.files); + expect(index.hunks.map(({ id }) => id)).toEqual([ + 'src/app.ts:pull-request:42:h1', + 'src/app.ts:pull-request:42:h2', + ]); + expect(index.hunkIdByAlias.get('h1')).toBe('src/app.ts:pull-request:42:h1'); + const prompt = buildWalkthroughPrompt(state); + expect(prompt).toContain('"id":"h1"'); + expect(prompt).not.toContain('"id":"src/app.ts:pull-request:42:h1"'); + expect(prompt).toContain('compact request-local aliases'); + expect(prompt).toContain('Every stop must have a concise semantic title'); +}); + +test('includes commit-by-commit strategy guidance in the walkthrough prompt', () => { + const prompt = buildWalkthroughPrompt(state, { + reviewStrategy: { + commits: [ + { + role: 'feature', + shortSha: 'aaaaaaaa', + subject: 'Add feature', + }, + ], + confidence: 0.9, + mode: 'commit-by-commit', + reason: 'stacked-subjects', + }, + }); + expect(prompt).toContain('Review strategy is commit-by-commit'); + expect(prompt).toContain('Add feature'); +}); + +test('normalizes draft aliases back onto live hunk ids and fills support', () => { + const index = indexWalkthroughHunks(state.files); + const walkthrough = normalizeWalkthroughDraft( + { + chapters: [ + { + blurb: 'Core change', + icon: 'path', + id: 'c1', + stops: [ + { + hunkIds: ['h1'], + id: 's1', + importance: 'critical', + prose: 'Explain the new call path.', + title: 'New call path', + }, + ], + title: 'Core', + }, + ], + focus: 'Review the feature.', + kind: 'narrative', + title: 'Feature walkthrough', + version: 4, + }, + state, + 'codex', + ); + expect(walkthrough.chapters[0]?.stops[0]?.hunkIds).toEqual([index.hunks[0]!.id]); + expect(walkthrough.support.length).toBeGreaterThan(0); + expect(walkthrough.support.some((item) => item.hunkIds.includes(index.hunks[1]!.id))).toBe(true); +}); + +test('accepts compact and legacy nullable draft shapes', () => { + const compact = parseWalkthroughDraft({ + chapters: [ + { + blurb: 'Core', + icon: 'path', + id: 'c1', + stops: [ + { + hunkIds: ['h1'], + id: 's1', + importance: 'normal', + prose: 'Details', + title: 'Title here', + }, + ], + title: 'Core', + }, + ], + focus: 'Focus', + kind: 'narrative', + title: 'Title', + version: 4, + }); + expect(compact.chapters[0]?.stops[0]?.title).toBe('Title here'); + + const legacy = parseWalkthroughDraft({ + chapters: [ + { + blurb: 'Core', + icon: 'path', + id: 'c1', + stops: [ + { + changeType: null, + commitNote: null, + hunkIds: ['h1'], + id: 's1', + importance: 'normal', + notes: null, + prose: 'Details', + summary: null, + title: null, + }, + ], + title: 'Core', + }, + ], + focus: 'Focus', + kind: 'narrative', + support: null, + title: 'Title', + version: 4, + }); + expect(legacy.chapters[0]?.stops[0]?.hunkIds).toEqual(['h1']); + expect(legacy.support).toBeUndefined(); +}); + +test('attaches overlapping version comment references to stops', () => { + const index = indexWalkthroughHunks(state.files); + const walkthrough = normalizeWalkthroughDraft( + { + chapters: [ + { + blurb: 'Core change', + icon: 'path', + id: 'c1', + stops: [ + { + hunkIds: [index.hunks[0]!.id], + id: 's1', + importance: 'critical', + prose: 'Explain the new call path.', + title: 'New call path', + }, + ], + title: 'Core', + }, + ], + focus: 'Review the feature.', + kind: 'narrative', + title: 'Feature walkthrough', + version: 4, + }, + state, + 'codex', + ); + const withComments = attachVersionCommentReferences(walkthrough, [ + { + authorName: 'Ada', + body: 'Please rename this.', + filePath: 'src/app.ts', + id: 'c1', + lineNumber: 1, + status: 'still-valid', + }, + ]); + expect(withComments.chapters[0]?.stops[0]?.commentReferences?.[0]?.id).toBe('c1'); +}); + +test('includes version-commit guidance and composes unit walkthroughs', () => { + const prompt = buildWalkthroughPrompt(state, { + versionCommitContext: { + after: { shortSha: 'bbbbbbb', subject: 'Later' }, + before: { shortSha: 'aaaaaaa', subject: 'Earlier' }, + evolutionKind: 'revised', + kind: 'version-commit', + range: { fromLabel: 'v1', toLabel: 'v2' }, + unitId: 'unit-1', + }, + versionCompareRange: { + fromLabel: 'v1', + structure: 'commit-by-commit', + toLabel: 'v2', + }, + }); + expect(prompt).toContain('logical commit between v1 and v2'); + expect(prompt).toContain('Earlier'); + expect(prompt).toContain('Later'); + + const unitWalkthrough = normalizeWalkthroughDraft( + { + chapters: [ + { + blurb: 'Unit', + icon: 'path', + id: 'c1', + stops: [ + { + hunkIds: ['h1'], + id: 's1', + importance: 'normal', + prose: 'Unit prose', + title: 'Unit stop', + }, + ], + title: 'Unit', + }, + ], + focus: 'Unit focus', + kind: 'narrative', + title: 'Unit title', + version: 4, + }, + state, + 'codex', + ); + const composed = composeUnitWalkthroughs({ + agent: 'codex', + entries: [ + { + context: { + after: { + sha: 'b'.repeat(40), + shortSha: 'bbbbbbb', + subject: 'Later', + }, + kind: 'version-commit', + range: { fromLabel: 'v1', toLabel: 'v2' }, + unitId: 'unit-1', + }, + state, + walkthrough: unitWalkthrough, + }, + ], + state, + }); + expect(composed.chapters[0]?.id.startsWith('unit-1:')).toBe(true); + expect(composed.commitFiles?.length).toBe(1); + expect(composed.title).toContain('v1'); +}); + +test('exposes prompt digest sizing and patch budgets', () => { + const { digest, patchBudgets, size } = buildWalkthroughPromptInput(state); + expect(size.hunkCount).toBe(2); + expect(digest.files[0]?.sections[0]?.hunks[0]?.id).toBe('h1'); + expect(patchBudgets.total).toBeGreaterThan(0); +}); diff --git a/core/lib/walkthrough-authoring.ts b/core/lib/walkthrough-authoring.ts new file mode 100644 index 00000000..aac60902 --- /dev/null +++ b/core/lib/walkthrough-authoring.ts @@ -0,0 +1,1158 @@ +/** + * Deterministic walkthrough authoring: draft parsing, hunk indexing, prompt + * construction, normalization, and unit composition. + * + * Hosts own model IDs, retries, and cache policy. This module owns shared + * authoring semantics used by local Codiff and Codiff Web. + */ +import { + array, + boolean, + type InferOutput, + literal, + looseObject, + maxLength, + maxValue, + minLength, + minValue, + null_, + number, + object, + optional, + parse, + picklist, + pipe, + safeParse, + string, + union, +} from 'valibot'; +import { + getSectionWalkthroughHunks, + isGeneratedWalkthroughPath, +} from './narrative-walkthrough-diff.js'; +import type { + ChangedFile, + DiffSection, + NarrativeWalkthrough, + RepositoryState, + WalkthroughCommentReference, + WalkthroughHunk, +} from '../types.ts'; + + +export const maxProseChars = 4000; +export const maxPatchExcerpt = 2500; +export const maxTotalPatchExcerpt = 60_000; +export const maxLargePatchExcerpt = 700; +export const maxLargeTotalPatchExcerpt = 35_000; +export const maxWalkthroughChapters = 20; +export const maxWalkthroughStops = 14; +export const maxHunksPerGroup = 14; + +const boundedString = (maximum: number) => pipe(string(), maxLength(maximum)); +const nonEmptyString = (maximum: number) => pipe(string(), minLength(1), maxLength(maximum)); +const positiveInt = pipe(number(), minValue(1), maxValue(1_000_000_000)); + +const noteSchema = object({ + body: nonEmptyString(500), + hunkId: nonEmptyString(200), +}); + +const reviewCommentSchema = looseObject({ + anchor: optional(picklist(['file', 'line'])), + author: looseObject({ + avatarUrl: optional(string()), + login: nonEmptyString(200), + url: optional(string()), + }), + body: nonEmptyString(maxProseChars * 8), + filePath: nonEmptyString(4096), + id: nonEmptyString(200), + isOutdated: optional(boolean()), + lineNumber: optional(positiveInt), + side: optional(picklist(['additions', 'deletions'])), + startLineNumber: optional(positiveInt), + startSide: optional(picklist(['additions', 'deletions'])), + submittedAt: optional(string()), + url: optional(string()), +}); + +const diffSectionSchema = looseObject({ + binary: boolean(), + id: nonEmptyString(500), + kind: picklist(['commit', 'pull-request', 'staged', 'unstaged']), + patch: string(), +}); + +const changedFileSchema = looseObject({ + fingerprint: nonEmptyString(256), + oldPath: optional(string()), + path: nonEmptyString(4096), + sections: array(diffSectionSchema), + status: picklist(['added', 'deleted', 'modified', 'renamed', 'untracked']), +}); + +const pullRequestSourceSchema = looseObject({ + type: literal('pull-request'), + url: nonEmptyString(2048), +}); + +const genericSourceSchema = looseObject({ + type: picklist([ + 'working-tree', + 'commit', + 'branch', + 'branch-diff', + 'branch-working-tree', + 'range', + 'pull-request', + ]), +}); + +export const repositoryStateSchema = looseObject({ + branch: union([string(), null_()]), + files: array(changedFileSchema), + generatedAt: number(), + launchPath: string(), + reviewComments: optional(array(reviewCommentSchema)), + root: string(), + source: union([pullRequestSourceSchema, genericSourceSchema]), +}); + +const changeTypeSchema = picklist([ + 'fix', + 'feature', + 'refactor', + 'test', + 'generated', + 'lockfile', + 'snapshot', + 'i18n', + 'docs', +]); + +const groupFields = { + changeType: optional(changeTypeSchema), + commitNote: optional(boundedString(1000)), + hunkIds: pipe(array(nonEmptyString(200)), minLength(1), maxLength(maxHunksPerGroup)), + id: nonEmptyString(100), + notes: optional(pipe(array(noteSchema), maxLength(maxHunksPerGroup))), + summary: optional(boundedString(1000)), + title: optional(boundedString(200)), +}; + +const stopSchema = object({ + ...groupFields, + importance: picklist(['critical', 'normal', 'context']), + prose: nonEmptyString(4000), +}); + +const supportSchema = object({ + ...groupFields, + note: optional(boundedString(2000)), + reason: nonEmptyString(200), +}); + +export const walkthroughDraftSchema = object({ + chapters: pipe( + array( + object({ + blurb: boundedString(1000), + icon: picklist(['bug', 'wrench', 'path', 'flask', 'beaker', 'doc', 'gear']), + id: nonEmptyString(100), + stops: pipe(array(stopSchema), maxLength(maxWalkthroughStops)), + title: nonEmptyString(16), + }), + ), + minLength(1), + maxLength(maxWalkthroughChapters), + ), + focus: nonEmptyString(2000), + kind: literal('narrative'), + support: optional(pipe(array(supportSchema), maxLength(30))), + title: nonEmptyString(200), + version: literal(4), +}); + +export type WalkthroughDraft = InferOutput; + +/** @deprecated Prefer WalkthroughDraft. */ +export type AuthoredWalkthrough = WalkthroughDraft; + +const nullableGroupFields = { + changeType: optional(union([changeTypeSchema, null_()])), + commitNote: optional(union([boundedString(1000), null_()])), + hunkIds: groupFields.hunkIds, + id: groupFields.id, + notes: optional(union([pipe(array(noteSchema), maxLength(maxHunksPerGroup)), null_()])), + summary: optional(union([boundedString(1000), null_()])), + title: optional(union([boundedString(200), null_()])), +}; + +const legacyWalkthroughDraftSchema = object({ + chapters: pipe( + array( + object({ + blurb: boundedString(1000), + icon: picklist(['bug', 'wrench', 'path', 'flask', 'beaker', 'doc', 'gear']), + id: nonEmptyString(100), + stops: pipe( + array( + object({ + ...nullableGroupFields, + importance: picklist(['critical', 'normal', 'context']), + prose: nonEmptyString(4000), + }), + ), + maxLength(maxWalkthroughStops), + ), + title: nonEmptyString(16), + }), + ), + minLength(1), + maxLength(maxWalkthroughChapters), + ), + focus: nonEmptyString(2000), + kind: literal('narrative'), + support: optional( + union([ + pipe( + array( + object({ + ...nullableGroupFields, + note: optional(union([boundedString(2000), null_()])), + reason: nonEmptyString(200), + }), + ), + maxLength(30), + ), + null_(), + ]), + ), + title: nonEmptyString(200), + version: literal(4), +}); + +const compactWalkthroughDraftSchema = object({ + chapters: pipe( + array( + object({ + blurb: boundedString(1000), + icon: picklist(['bug', 'wrench', 'path', 'flask', 'beaker', 'doc', 'gear']), + id: nonEmptyString(100), + stops: pipe( + array( + object({ + hunkIds: groupFields.hunkIds, + id: groupFields.id, + importance: picklist(['critical', 'normal', 'context']), + prose: nonEmptyString(4000), + title: nonEmptyString(80), + }), + ), + maxLength(maxWalkthroughStops), + ), + title: nonEmptyString(16), + }), + ), + minLength(1), + maxLength(maxWalkthroughChapters), + ), + focus: nonEmptyString(2000), + kind: literal('narrative'), + title: nonEmptyString(200), + version: literal(4), +}); + +/** Agent structured-output schema shape for hosts that need a JSON schema later. */ +export const walkthroughDraftAgentOutputSchema = compactWalkthroughDraftSchema; + +const compactNullableGroup = (group: { + changeType?: string | null; + commitNote?: string | null; + hunkIds: ReadonlyArray; + id: string; + notes?: ReadonlyArray<{ body: string; hunkId: string }> | null; + summary?: string | null; + title?: string | null; +}) => ({ + ...(group.changeType + ? { changeType: group.changeType as NonNullable } + : {}), + ...(group.commitNote ? { commitNote: group.commitNote } : {}), + hunkIds: [...group.hunkIds], + id: group.id, + ...(group.notes ? { notes: [...group.notes] } : {}), + ...(group.summary ? { summary: group.summary } : {}), + ...(group.title ? { title: group.title } : {}), +}); + +export const parseWalkthroughDraft = (value: unknown): WalkthroughDraft => { + const authored = safeParse(walkthroughDraftSchema, value); + if (authored.success) { + return authored.output; + } + + const legacyOutput = safeParse(legacyWalkthroughDraftSchema, value); + if (legacyOutput.success) { + return { + chapters: legacyOutput.output.chapters.map((chapter) => ({ + blurb: chapter.blurb, + icon: chapter.icon, + id: chapter.id, + stops: chapter.stops.map((stop) => ({ + ...compactNullableGroup(stop), + importance: stop.importance, + prose: stop.prose, + })), + title: chapter.title, + })), + focus: legacyOutput.output.focus, + kind: legacyOutput.output.kind, + ...(Array.isArray(legacyOutput.output.support) + ? { + support: legacyOutput.output.support.map((item) => ({ + ...compactNullableGroup(item), + ...(item.note ? { note: item.note } : {}), + reason: item.reason, + })), + } + : {}), + title: legacyOutput.output.title, + version: legacyOutput.output.version, + }; + } + + const output = parse(compactWalkthroughDraftSchema, value); + return { + chapters: output.chapters.map((chapter) => ({ + blurb: chapter.blurb, + icon: chapter.icon, + id: chapter.id, + stops: chapter.stops.map((stop) => ({ + hunkIds: [...stop.hunkIds], + id: stop.id, + importance: stop.importance, + prose: stop.prose, + title: stop.title, + })), + title: chapter.title, + })), + focus: output.focus, + kind: output.kind, + title: output.title, + version: output.version, + }; +}; + +/** @deprecated Prefer parseWalkthroughDraft. */ +export const parseAuthoredWalkthrough = parseWalkthroughDraft; + +export const parseRepositoryState = (value: unknown): RepositoryState => + parse(repositoryStateSchema, value) as RepositoryState; + +export type WalkthroughReviewStrategy = + | { + commits: ReadonlyArray<{ + role: string; + shortSha: string; + subject: string; + }>; + confidence: number; + mode: 'commit-by-commit'; + reason: string; + } + | { + confidence: number; + mode: 'whole-diff' | 'whole-mr'; + reason: string; + }; + + +type IndexedHunk = WalkthroughHunk & { + sectionId: string; + sectionKind: 'pull-request'; +}; + +type SectionWalkthroughHunk = ReturnType[number]; + +const defaultSideForStatus = (status: ChangedFile['status']) => + status === 'added' || status === 'untracked' + ? ('additions' as const) + : status === 'deleted' + ? ('deletions' as const) + : ('both' as const); + +const createIndexedHunk = ( + file: ChangedFile, + section: DiffSection, + hunk: SectionWalkthroughHunk, +): IndexedHunk => { + const patchHunk = 'additionStart' in hunk ? hunk : null; + const startLine = patchHunk + ? patchHunk.added > 0 + ? patchHunk.additionStart + : patchHunk.deletionStart + : undefined; + const endLine = patchHunk + ? patchHunk.added > 0 + ? patchHunk.additionEnd + : patchHunk.deletionEnd + : undefined; + const display = + startLine == null + ? file.path + : startLine === endLine + ? `${file.path}:${startLine}` + : `${file.path}:${startLine}-${endLine}`; + + return { + added: hunk.added, + ...(patchHunk ? { additionEnd: patchHunk.additionEnd } : {}), + ...(patchHunk ? { additionStart: patchHunk.additionStart } : {}), + anchor: { + display, + ...(endLine != null ? { endLine } : {}), + sectionId: section.id, + sectionKind: section.kind, + side: defaultSideForStatus(file.status), + ...(startLine != null ? { startLine } : {}), + }, + deleted: hunk.deleted, + ...(patchHunk ? { deletionEnd: patchHunk.deletionEnd } : {}), + ...(patchHunk ? { deletionStart: patchHunk.deletionStart } : {}), + id: hunk.id, + kind: patchHunk ? 'patch' : 'synthetic', + ...(file.oldPath ? { oldPath: file.oldPath } : {}), + path: file.path, + sectionId: section.id, + sectionKind: 'pull-request', + status: file.status, + }; +}; + +const indexFileHunks = (file: ChangedFile): Array => + file.sections.flatMap((section) => + getSectionWalkthroughHunks(file, section).map((hunk) => createIndexedHunk(file, section, hunk)), + ); + +export const indexWalkthroughHunks = (files: ReadonlyArray) => { + const hunks = files.flatMap(indexFileHunks); + const byId = new Map(); + const aliasByHunkId = new Map(); + const hunkIdByAlias = new Map(); + hunks.forEach((hunk, index) => { + const alias = `h${index + 1}`; + byId.set(hunk.id, hunk); + byId.set(alias, hunk); + aliasByHunkId.set(hunk.id, alias); + hunkIdByAlias.set(alias, hunk.id); + }); + return { + aliasByHunkId, + byId, + hunkIdByAlias, + hunks, + }; +}; + +const lineCounts = (hunks: ReadonlyArray) => + hunks.reduce( + (total, hunk) => ({ + added: total.added + hunk.added, + deleted: total.deleted + hunk.deleted, + }), + { added: 0, deleted: 0 }, + ); + +const clean = (value: string, fallback = '') => value.trim() || fallback; + +export const normalizeWalkthroughDraft = ( + value: unknown, + state: RepositoryState, + agent: NarrativeWalkthrough['agent'], +): NarrativeWalkthrough => { + const authored = parseWalkthroughDraft(value); + const index = indexWalkthroughHunks(state.files); + const used = new Set(); + const itemIds = new Set(); + + const resolveGroup = (group: WalkthroughDraft["chapters"][number]["stops"][number] | NonNullable[number]) => { + if (itemIds.has(group.id)) { + return null; + } + const groupHunkIds = new Set(); + const hunks = group.hunkIds.flatMap((id) => { + const hunk = index.byId.get(id); + if (!hunk || used.has(hunk.id) || groupHunkIds.has(hunk.id)) { + return []; + } + groupHunkIds.add(hunk.id); + return [hunk]; + }); + if (hunks.length === 0) { + return null; + } + hunks.forEach((hunk) => used.add(hunk.id)); + itemIds.add(group.id); + const counts = lineCounts(hunks); + const hunkIds = hunks.map((hunk) => hunk.id); + return { + ...counts, + ...(group.changeType + ? { changeType: group.changeType as NonNullable } + : {}), + ...(group.commitNote ? { commitNote: clean(group.commitNote) } : {}), + hunkIds, + hunks, + id: group.id, + ...(group.notes + ? { + notes: group.notes + .map((note) => ({ + ...note, + hunkId: index.byId.get(note.hunkId)?.id ?? note.hunkId, + })) + .filter( + (note, noteIndex, notes) => + hunkIds.includes(note.hunkId) && + notes.findIndex((candidate) => candidate.hunkId === note.hunkId) === noteIndex, + ), + } + : {}), + ...(group.summary ? { summary: clean(group.summary) } : {}), + ...(group.title ? { title: clean(group.title) } : {}), + }; + }; + + const chapters = authored.chapters.flatMap((chapter) => { + const stops = chapter.stops.flatMap((stop) => { + const group = resolveGroup(stop); + return group + ? [ + { + ...group, + importance: stop.importance, + prose: clean(stop.prose), + }, + ] + : []; + }); + return stops.length > 0 + ? [ + { + blurb: clean(chapter.blurb), + icon: chapter.icon, + id: chapter.id, + stops, + title: clean(chapter.title, 'Review'), + }, + ] + : []; + }); + if (chapters.length === 0) { + throw new Error('The generated walkthrough did not reference any current diff hunks.'); + } + + const support = (authored.support ?? []).flatMap((item) => { + const group = resolveGroup(item); + return group + ? [ + { + ...group, + ...(item.note ? { note: clean(item.note) } : {}), + reason: clean(item.reason, 'Other changes'), + }, + ] + : []; + }); + const remainingByPath = new Map>(); + for (const hunk of index.hunks) { + if (!used.has(hunk.id)) { + remainingByPath.set(hunk.path, [...(remainingByPath.get(hunk.path) ?? []), hunk]); + } + } + for (const [path, hunks] of remainingByPath) { + for (let start = 0; start < hunks.length; start += maxHunksPerGroup) { + const chunk = hunks.slice(start, start + maxHunksPerGroup); + const counts = lineCounts(chunk); + let supportId = `support-${support.length + 1}`; + while (itemIds.has(supportId)) { + supportId = `support-${Number(supportId.slice('support-'.length)) + 1}`; + } + support.push({ + ...counts, + hunkIds: chunk.map((hunk) => hunk.id), + hunks: chunk, + id: supportId, + reason: 'Other changes', + title: path, + }); + itemIds.add(supportId); + } + } + const stopCount = chapters.reduce((count, chapter) => count + chapter.stops.length, 0); + return { + agent, + chapters, + focus: clean(authored.focus, 'Walk through the merge request.'), + generatedAt: new Date().toISOString(), + kind: 'narrative', + meta: `${stopCount} stops · ${chapters.length} chapters`, + repo: { + branch: state.branch, + root: state.root, + }, + source: state.source, + support, + title: clean(authored.title, 'Merge request walkthrough'), + version: 4, + }; +}; + +const truncate = (value: string, maxLength: number) => + value.length <= maxLength + ? value + : maxLength <= 1 + ? value.slice(0, maxLength) + : `${value.slice(0, maxLength - 1)}…`; + +const getPromptPatchBudgets = (fileCount: number) => + fileCount > 32 + ? { section: maxLargePatchExcerpt, total: maxLargeTotalPatchExcerpt } + : { section: maxPatchExcerpt, total: maxTotalPatchExcerpt }; + +const buildPatchExcerpt = ( + section: DiffSection, + remainingBudget: number, + sectionBudget: number, +) => { + const summary = section.summary?.reason ? `Summary: ${section.summary.reason}\n` : ''; + const patch = `${summary}${section.patch || ''}`; + if (!patch) { + return '[patch omitted: no text patch available]'; + } + const maxLength = Math.max(0, Math.min(sectionBudget, remainingBudget)); + return truncate(patch, maxLength); +}; + +const formatPromptLineRange = (start: number, end: number) => + start === end ? `${start}` : `${start}-${end}`; + +const buildPromptHunk = (hunk: IndexedHunk, id: string) => + hunk.kind === 'synthetic' + ? { + added: hunk.added, + deleted: hunk.deleted, + id, + kind: 'synthetic' as const, + } + : { + added: hunk.added, + deleted: hunk.deleted, + id, + kind: 'patch' as const, + newLines: formatPromptLineRange(hunk.additionStart ?? 0, hunk.additionEnd ?? 0), + oldLines: formatPromptLineRange(hunk.deletionStart ?? 0, hunk.deletionEnd ?? 0), + }; + +type WalkthroughSize = { + fileCount: number; + hunkCount: number; +}; + +const buildWalkthroughSizingGuidance = ( + { fileCount, hunkCount }: WalkthroughSize, + { independentCommit = false }: { independentCommit?: boolean } = {}, +) => { + const targetStops = + fileCount <= 2 + ? hunkCount <= 4 + ? '1-2' + : '2-3' + : fileCount <= 4 && hunkCount <= 4 + ? '1-2' + : fileCount <= 4 && hunkCount <= 8 + ? '1-3' + : fileCount <= 16 + ? '5-9' + : '6-9'; + const targetChapters = fileCount <= 2 ? '1' : fileCount <= 4 && hunkCount <= 8 ? '1-2' : '2-6'; + + return `Coverage contract: +- The digest has ${fileCount} files and ${hunkCount} reviewable hunks. Put only the highest-leverage review path in chapters[]; Codiff preserves everything else as support. +- Digest hunk ids are compact request-local aliases like h1 and h2. Return those aliases exactly; Codiff maps them back to stable live-diff ids. +- Define chapters[] and stops[] in display order. Use stable stop ids like s1, s2, and never invent hunk ids. +${ + independentCommit + ? '- Choose as many conceptual chapters as this commit needs. Do not collapse distinct review ideas into one chapter.' + : `- Target ${targetStops} main-path stops and at most ${maxWalkthroughStops}. Use ${targetChapters} conceptual chapters.` +} +- Chapter titles render in a compact top bar: keep each title to 1-2 short words and at most 16 characters. +- Default to one review idea per stop. Group multiple hunkIds when they implement the same invariant, behavior, or repeated pattern. +- Every stop must have a concise semantic title that names the review idea in roughly 2-6 words, e.g. "Prevent duplicate payments" or "Preserve offline drafts". Never use a filename or path as a stop title. +- A stop may contain at most ${maxHunksPerGroup} hunkIds, listed in the exact display order Codiff should render. +- Generated-like files have "generated":true and one synthetic hunk per changed section. Never split them; main-path them only when they explain behavior. +- Leave secondary, mechanical, docs-only, generated, styling, fixture, lockfile, snapshot, and repeated-pattern hunks out of chapters[]. Codiff automatically places every unreferenced hunk in support. +- Do not provide support, added/deleted counts, status, paths, section ids, repo, source, generatedAt, agent, meta, notes, changeType, summary, or commitNote for stops; Codiff computes display metadata from the live diff.`; +}; + +// Prompt shaping for commit-aware / version-comparison walkthroughs. +// Options flow: fate generateWalkthrough → walkthrough-agent.generate → buildWalkthroughPrompt. +export type WalkthroughPromptOptions = { + /** When set, the digest contains exactly one commit and is authored independently. */ + commitContext?: { + sha: string; + subject: string; + } | null; + reviewStrategy?: WalkthroughReviewStrategy | null; + versionBaseContext?: { + absorbedCommits: ReadonlyArray<{ + baseShortSha?: string; + shortSha: string; + subject: string; + }>; + commits: ReadonlyArray<{ shortSha: string; subject: string }>; + relationship: 'forward' | 'backward' | 'divergent' | 'unknown'; + } | null; + versionCommentReferences?: ReadonlyArray | null; + /** One changed logical commit unit inside a selected version range. */ + versionCommitContext?: { + after?: { shortSha: string; subject: string }; + before?: { shortSha: string; subject: string }; + evolutionKind: 'likely-revised' | 'added' | 'removed' | 'ambiguous'; + kind: 'version-commit'; + range: { fromLabel: string; toLabel: string }; + rebaseDrivers?: ReadonlyArray<{ + authorName: string; + overlappingPaths: ReadonlyArray; + shortSha: string; + subject: string; + }>; + unitId: string; + } | null; + /** When set, author a walkthrough of MR version evolution, not the whole net MR. */ + versionCompareRange?: { + fromLabel: string; + structure?: 'commit-by-commit' | 'whole-diff'; + toLabel: string; + } | null; +}; + +const commentTouchesHunk = (comment: WalkthroughCommentReference, hunk: WalkthroughHunk) => { + if (comment.filePath !== hunk.path && comment.filePath !== hunk.oldPath) { + return false; + } + if (comment.lineNumber == null) { + return true; + } + const line = comment.lineNumber; + return ( + (hunk.additionStart != null && + hunk.additionEnd != null && + line >= hunk.additionStart - 2 && + line <= hunk.additionEnd + 2) || + (hunk.deletionStart != null && + hunk.deletionEnd != null && + line >= hunk.deletionStart - 2 && + line <= hunk.deletionEnd + 2) + ); +}; + +export const attachVersionCommentReferences = ( + walkthrough: NarrativeWalkthrough, + comments: ReadonlyArray | null | undefined, +): NarrativeWalkthrough => + !comments?.length + ? walkthrough + : { + ...walkthrough, + chapters: walkthrough.chapters.map((chapter) => ({ + ...chapter, + stops: chapter.stops.map((stop) => { + const commentReferences = comments.filter((comment) => + stop.hunks.some((hunk) => commentTouchesHunk(comment, hunk)), + ); + return commentReferences.length ? { ...stop, commentReferences } : stop; + }), + })), + }; + +const buildReviewStrategyDigest = (strategy: WalkthroughReviewStrategy | null | undefined) => { + if (!strategy) { + return null; + } + if (strategy.mode === 'commit-by-commit') { + return { + commits: strategy.commits.map((commit) => ({ + role: commit.role, + sha: commit.shortSha, + subject: truncate(commit.subject, 120), + })), + confidence: strategy.confidence, + mode: strategy.mode, + reason: strategy.reason, + }; + } + return { + confidence: strategy.confidence, + mode: strategy.mode === 'whole-mr' ? 'whole-diff' : strategy.mode, + reason: strategy.reason, + }; +}; + +const buildCommitStructureGuidance = (strategy: WalkthroughReviewStrategy | null | undefined) => { + if (!strategy || strategy.mode !== 'commit-by-commit') { + return `- Prefer conceptual chapters across the net merge-request diff. +- A commit list may be present as weak author history context; do not structure the walkthrough around fixups or review-response commits. +- Do not invent commit metadata that is not in the digest.`; + } + return `- Review strategy is commit-by-commit (${strategy.reason}). +- Preserve distinct review ideas as separate chapters; there is no one-chapter-per-commit limit. +- Stop titles must stay semantic, but may reference the related commit subject. +- Digest hunks still come from the live whole-MR diff for stable anchors; optionally mention commit subjects in prose when they clarify chapter boundaries. +- Include every non-merge commit in its own boundary; do not group commits.`; +}; + +const buildVersionCompareStructureGuidance = ( + versionCompareRange: WalkthroughPromptOptions['versionCompareRange'], +) => { + if (!versionCompareRange) { + return ''; + } + const structureLabel = + versionCompareRange.structure === 'commit-by-commit' + ? 'commit-by-commit' + : versionCompareRange.structure === 'whole-diff' + ? 'whole-diff' + : null; + return `- This walkthrough covers the version comparison from ${versionCompareRange.fromLabel} to ${versionCompareRange.toLabel}${structureLabel ? ` as a ${structureLabel} walkthrough` : ''}. +- Focus on how the merge request itself evolved: intentional edits, conflict-resolution fallout, and newly added/removed behavior. +- Do not narrate pure rebase noise. Prefer chapters that answer "what changed since the earlier version?" for a returning reviewer. +- Hunks still come from the supplied digest; treat them as the version-comparison surface, not the full historical MR net diff.${ + structureLabel === 'commit-by-commit' + ? '\n- This request is one unit inside a commit-by-commit version walkthrough; stay scoped to the supplied unit.' + : structureLabel === 'whole-diff' + ? '\n- This is a whole-diff version walkthrough across the intentional version-comparison surface.' + : '' + }`; +}; + +const buildVersionBaseGuidance = (context: WalkthroughPromptOptions['versionBaseContext']) => { + if (!context) { + return ''; + } + const absorbed = context.absorbedCommits + .map( + (commit) => + ` - ${commit.shortSha}: ${commit.subject}${commit.baseShortSha ? ` (now in base as ${commit.baseShortSha})` : ''}`, + ) + .join('\n'); + const baseCommits = context.commits + .slice(0, 12) + .map((commit) => ` - ${commit.shortSha}: ${commit.subject}`) + .join('\n'); + return `- The target base changed between these versions (${context.relationship}). +${ + absorbed + ? `- These earlier MR commits are now supplied by the target base. Do not describe their behavior as removed: +${absorbed}` + : '- No earlier MR commits were confidently identified as moving into the target base.' +} +- Base commits are context only. Mention them only when the supplied version diff demonstrates an adaptation: +${baseCommits || ' - none available'}`; +}; + +const buildVersionCommentGuidance = ( + comments: WalkthroughPromptOptions['versionCommentReferences'], +) => { + if (!comments?.length) { + return ''; + } + return `- The digest includes review comments anchored to this version comparison. +- Mention a comment when its code region is part of the reviewer path. +- Say a comment was addressed only when its status is resolved-by-change; otherwise describe it as related context.`; +}; + +const buildSingleCommitGuidance = (commit: WalkthroughPromptOptions['commitContext']) => { + if (!commit) { + return ''; + } + return `- This is an independent walkthrough for commit ${commit.sha}: ${commit.subject}. +- Explain only the changes introduced by this commit. Do not summarize the merge request as a whole. +- Never refer to earlier or later commits, the commit stack, a rebase, or cumulative merge-request history. +- Do not infer behavior from code outside this commit's supplied diff. +- Build the best reviewer path through this commit's own diff.`; +}; + +const buildVersionCommitGuidance = (context: WalkthroughPromptOptions['versionCommitContext']) => { + if (!context) { + return ''; + } + const before = context.before ? `${context.before.shortSha}: ${context.before.subject}` : 'none'; + const after = context.after ? `${context.after.shortSha}: ${context.after.subject}` : 'none'; + if (context.evolutionKind === 'added') { + return `- This is one commit added between ${context.range.fromLabel} and ${context.range.toLabel}: ${after}. +- Explain only this new commit's own contribution. Do not summarize the complete version range.`; + } + if (context.evolutionKind === 'removed') { + return `- This commit was removed from the MR stack between ${context.range.fromLabel} and ${context.range.toLabel}: ${before}. +- The supplied diff is intentionally inverted. Explain what was removed from the MR, never as newly authored implementation.`; + } + if (context.evolutionKind === 'ambiguous') { + return `- GitLab commit changes could not be paired confidently between ${context.range.fromLabel} and ${context.range.toLabel}. +- This fallback contains authoritative whole-range effects not safely attributable to one logical commit. +- Explain these as other stack changes and do not invent commit identity.`; + } + const drivers = context.rebaseDrivers ?? []; + if (drivers.length === 0) { + return `- This is the change to one logical commit between ${context.range.fromLabel} and ${context.range.toLabel}. +- Earlier commit: ${before}. Later commit: ${after}. +- Explain only the supplied patch evolution, not the complete later commit or changes in other commits. +- This pairing is heuristic. Never claim the two SHAs are exactly the same commit. +- If the patch looks like conflict resolution or context-line churn, say the rewrite may be rebase fallout, but only when the supplied hunks support that reading. +- Do not invent base-branch commits that are not listed in the digest. +- Treat the supplied hunks as the source of truth.`; + } + const primary = drivers[0]!; + const driverLines = drivers + .map( + (driver) => + ` - ${driver.shortSha}: ${driver.subject} (by ${driver.authorName}; overlaps ${driver.overlappingPaths.slice(0, 4).join(', ')}${driver.overlappingPaths.length > 4 ? ', …' : ''})`, + ) + .join('\n'); + return `- This is a REBASE-REVISED logical commit between ${context.range.fromLabel} and ${context.range.toLabel}. +- Earlier commit: ${before}. Later commit: ${after}. +- Opening requirement: the first sentence of the first chapter blurb MUST start from the base-branch update that this rebase pulled in, using this shape: + "Because this rebase brought in ${primary.shortSha} (${primary.subject}) from the base branch, this MR commit was revised to …" +- Preferred primary driver: ${primary.shortSha} — ${primary.subject}. +- All attributed base-branch commits brought in by the rebase: +${driverLines} +- After that opening sentence, explain only the supplied patch evolution for this logical commit. +- Distinguish clearly: + 1) adaptations required because the rebase moved the MR onto newer base-branch commits, versus + 2) intentional new MR behavior not explained by those base-branch commits. +- Do not narrate the entire base branch; only the listed base commits that the rebase introduced under this MR commit. +- Prefer wording like "brought in by the rebase", "now present on the updated base", or "required after rebasing onto newer base commits". Avoid "landed by rebase". +- This pairing is heuristic. Never claim the two SHAs are exactly the same commit. +- Treat the supplied hunks as the source of truth. +`; +}; + +export const buildWalkthroughPromptInput = ( + state: RepositoryState, + options: WalkthroughPromptOptions = {}, +) => { + const index = indexWalkthroughHunks(state.files); + const patchBudgets = getPromptPatchBudgets(state.files.length); + const size = { + fileCount: state.files.length, + hunkCount: index.hunks.length, + }; + let remainingPatchBudget = patchBudgets.total; + const digest = { + branch: state.branch, + commitContext: options.commitContext ?? null, + files: state.files.map((file) => ({ + ...(isGeneratedWalkthroughPath(file.path) + ? { + generated: true, + generatedReason: + 'Generated-like file; Codiff exposes each changed section as one synthetic hunk.', + } + : {}), + oldPath: file.oldPath, + path: file.path, + sections: file.sections.map((section) => { + const patchExcerpt = buildPatchExcerpt(section, remainingPatchBudget, patchBudgets.section); + remainingPatchBudget = Math.max(0, remainingPatchBudget - patchExcerpt.length); + return { + binary: section.binary, + hunks: index.hunks + .filter((hunk) => hunk.sectionId === section.id) + .map((hunk) => buildPromptHunk(hunk, index.aliasByHunkId.get(hunk.id) ?? hunk.id)), + id: section.id, + kind: section.kind, + loadState: section.loadState, + patchExcerpt, + summary: section.summary?.reason, + }; + }), + status: file.status, + })), + reviewStrategy: buildReviewStrategyDigest(options.reviewStrategy), + source: + (options.commitContext || options.versionCommitContext) && + state.source.type === 'pull-request' + ? { + ...state.source, + description: undefined, + title: + options.commitContext?.subject ?? + options.versionCommitContext?.after?.subject ?? + options.versionCommitContext?.before?.subject ?? + state.source.title, + } + : state.source.type === 'pull-request' && typeof state.source.description === 'string' + ? { ...state.source, description: truncate(state.source.description, maxProseChars) } + : state.source, + versionBaseContext: options.versionBaseContext ?? null, + versionCommentReferences: (options.versionCommentReferences ?? []).map((comment) => ({ + ...comment, + body: truncate(comment.body, 500), + })), + versionCommitContext: options.versionCommitContext ?? null, + versionCompareRange: options.versionCompareRange ?? null, + }; + return { digest, patchBudgets, size }; +}; + +export const buildWalkthroughPrompt = ( + state: RepositoryState, + options: WalkthroughPromptOptions = {}, +) => { + const { digest, size } = buildWalkthroughPromptInput(state, options); + return `Author a Codiff narrative walkthrough for this GitLab merge request. + +Use only the supplied digest. Return the required structured object. If source.description is present, treat it as author-written intent and orientation, not proof of behavior; patches and hunk data remain the source of truth. + +${buildWalkthroughSizingGuidance(size, { independentCommit: Boolean(options.commitContext || options.versionCommitContext) })} + +Product rules: +- Write all user-visible text in English. +- Order the review by conceptual leverage, not file path. +- Keep prose concise, concrete, and evidence-based. Inline code is allowed, but no headings or lists. +- Do not claim tests, risks, or behavior that the diff does not support. +${buildCommitStructureGuidance(options.reviewStrategy)} +${buildVersionCompareStructureGuidance(options.versionCompareRange)} +${buildVersionBaseGuidance(options.versionBaseContext)} +${buildSingleCommitGuidance(options.commitContext)} +${buildVersionCommitGuidance(options.versionCommitContext)} +${buildVersionCommentGuidance(options.versionCommentReferences)} + +Repository digest: +${JSON.stringify(digest)}`; +}; + + + +/** @deprecated Prefer normalizeWalkthroughDraft. */ +export const normalizeAuthoredWalkthrough = normalizeWalkthroughDraft; + +export type UnitWalkthroughEntry = { + context: + | { + kind: 'mr-commit'; + commit: { + sha: string; + shortSha: string; + subject: string; + webUrl?: string; + }; + } + | { + kind: 'version-commit'; + after?: { + sha: string; + shortSha: string; + subject: string; + webUrl?: string; + }; + before?: { + sha: string; + shortSha: string; + subject: string; + webUrl?: string; + }; + commentReferences?: ReadonlyArray; + range: { fromLabel: string; toLabel: string }; + rebaseDrivers?: ReadonlyArray<{ + authoredAt?: string; + authorName?: string; + overlappingPaths?: ReadonlyArray; + sha?: string; + shortSha: string; + subject: string; + webUrl?: string; + }>; + unitId: string; + }; + state: RepositoryState; + walkthrough: NarrativeWalkthrough | null; +}; + +/** + * Deterministically compose completed unit walkthroughs into one narrative. + */ +export const composeUnitWalkthroughs = ({ + agent, + entries, + state, +}: { + agent: NarrativeWalkthrough['agent']; + entries: ReadonlyArray; + state: RepositoryState; +}): NarrativeWalkthrough => { + const chapters = entries.flatMap((entry) => { + const context = entry.context; + const identity = context.kind === 'mr-commit' ? context.commit.sha : context.unitId; + const summary = + context.kind === 'mr-commit' ? context.commit : (context.after ?? context.before); + const entryWalkthrough = + context.kind === 'version-commit' && entry.walkthrough + ? attachVersionCommentReferences(entry.walkthrough, context.commentReferences) + : entry.walkthrough; + const rebaseDrivers = context.kind === 'version-commit' ? (context.rebaseDrivers ?? []) : []; + return (entryWalkthrough?.chapters ?? []).map((chapter, chapterIndex) => ({ + ...chapter, + ...(chapterIndex === 0 && summary + ? { + commit: { + gitSha: summary.sha, + sha: identity, + shortSha: summary.shortSha, + subject: summary.subject, + webUrl: summary.webUrl, + ...(rebaseDrivers.length > 0 + ? { + rebaseDrivers: rebaseDrivers.map((driver) => ({ + authoredAt: driver.authoredAt, + authorName: driver.authorName, + overlappingPaths: driver.overlappingPaths, + sha: driver.sha, + shortSha: driver.shortSha, + subject: driver.subject, + webUrl: driver.webUrl, + })), + } + : {}), + }, + } + : {}), + id: `${identity}:${chapter.id}`, + stops: chapter.stops.map((stop) => ({ ...stop, id: `${identity}:${stop.id}` })), + })); + }); + const versionContext = entries.find((entry) => entry.context.kind === 'version-commit')?.context; + const isVersionComparison = versionContext?.kind === 'version-commit'; + return { + agent, + chapters, + commitFiles: entries.flatMap((entry) => entry.state.files), + focus: isVersionComparison + ? `Review ${entries.length} changed commit units in stack order.` + : `Review ${entries.length} commits in topological order, from oldest to newest.`, + generatedAt: new Date().toISOString(), + kind: 'narrative', + repo: { branch: state.branch, root: state.root }, + source: state.source, + support: [], + title: isVersionComparison + ? `Commit-by-commit changes from ${versionContext.range.fromLabel} to ${versionContext.range.toLabel}` + : 'Commit walkthroughs', + version: 4, + }; +}; + +/** @deprecated Prefer composeUnitWalkthroughs. */ +export const combineCommitWalkthroughs = composeUnitWalkthroughs; diff --git a/core/package.json b/core/package.json index e9113900..84ff0e6d 100644 --- a/core/package.json +++ b/core/package.json @@ -47,10 +47,25 @@ "types": "./dist/lib/narrative-walkthrough-diff.d.ts", "@nkzw/codiff-source": "./lib/narrative-walkthrough-diff.js", "default": "./dist/lib/narrative-walkthrough-diff.mjs" + }, + "./types": { + "types": "./dist/types.d.ts", + "@nkzw/codiff-source": "./types.ts", + "default": "./dist/types.d.ts" + }, + "./narrative-walkthrough-diff": { + "types": "./dist/lib/narrative-walkthrough-diff.d.ts", + "@nkzw/codiff-source": "./lib/narrative-walkthrough-diff.js", + "default": "./dist/lib/narrative-walkthrough-diff.mjs" + }, + "./walkthrough-authoring": { + "types": "./dist/walkthrough-authoring.d.ts", + "@nkzw/codiff-source": "./walkthrough-authoring.ts", + "default": "./dist/walkthrough-authoring.mjs" } }, "scripts": { - "build": "rm -rf dist && vp pack -d dist --target=node24 index.ts react.ts share.ts lib/narrative-walkthrough-diff.js App.css && vp exec tsc -p tsconfig.build.json" + "build": "rm -rf dist && vp pack -d dist --target=node24 index.ts react.ts share.ts walkthrough-authoring.ts lib/narrative-walkthrough-diff.js App.css && vp exec tsc -p tsconfig.build.json" }, "dependencies": { "@nkzw/mdx-editor": "^0.2.2", diff --git a/core/tsconfig.build.json b/core/tsconfig.build.json index 9ed75c10..c4db1239 100644 --- a/core/tsconfig.build.json +++ b/core/tsconfig.build.json @@ -16,6 +16,7 @@ "index.ts", "react.ts", "share.ts", + "walkthrough-authoring.ts", "types.ts", "SharedPlanApp.tsx", "SharedWalkthroughApp.tsx", diff --git a/core/walkthrough-authoring.ts b/core/walkthrough-authoring.ts new file mode 100644 index 00000000..e97c4876 --- /dev/null +++ b/core/walkthrough-authoring.ts @@ -0,0 +1,29 @@ +export { + attachVersionCommentReferences, + buildWalkthroughPrompt, + buildWalkthroughPromptInput, + combineCommitWalkthroughs, + composeUnitWalkthroughs, + indexWalkthroughHunks, + maxHunksPerGroup, + maxLargePatchExcerpt, + maxLargeTotalPatchExcerpt, + maxPatchExcerpt, + maxProseChars, + maxTotalPatchExcerpt, + maxWalkthroughChapters, + maxWalkthroughStops, + normalizeAuthoredWalkthrough, + normalizeWalkthroughDraft, + parseAuthoredWalkthrough, + parseRepositoryState, + parseWalkthroughDraft, + repositoryStateSchema, + walkthroughDraftAgentOutputSchema, + walkthroughDraftSchema, + type AuthoredWalkthrough, + type UnitWalkthroughEntry, + type WalkthroughDraft, + type WalkthroughPromptOptions, + type WalkthroughReviewStrategy, +} from './lib/walkthrough-authoring.ts'; From c4ea0164ece46510318ed5897322c9ee82cd8b7f Mon Sep 17 00:00:00 2001 From: Matt Alonso Date: Mon, 20 Jul 2026 16:25:48 -0500 Subject: [PATCH 3/8] Add forge-neutral commit stack matching Implement patch-signature generation and confidence-based pairing for commits across rewritten review stacks. The matcher lives in Core so GitLab and GitHub can share retained, revised, removed, ambiguous, and absorbed-into-base classification without duplicating provider logic. --- core/lib/commit-stack-evolution.ts | 830 +++++++++++++++++++++++++++++ core/lib/crypto.ts | 5 + 2 files changed, 835 insertions(+) create mode 100644 core/lib/commit-stack-evolution.ts create mode 100644 core/lib/crypto.ts diff --git a/core/lib/commit-stack-evolution.ts b/core/lib/commit-stack-evolution.ts new file mode 100644 index 00000000..f2376202 --- /dev/null +++ b/core/lib/commit-stack-evolution.ts @@ -0,0 +1,830 @@ +/** + * Forge-neutral commit-stack evolution: patch signatures + stack matching. + * + * Used by GitLab MR versions and GitHub force-push head comparisons so both + * forges share one pairing algorithm. + */ +import type { ChangedFile } from '../types.ts'; +import { sha256 } from './crypto.ts'; + +export const versionCommitSignatureAlgorithmVersion = 'patch-signature-v1'; +export const versionCommitEvolutionAlgorithmVersion = 'monotonic-v1'; +export const versionCommitStackLimit = 40; +export const versionCommitDiffConcurrency = 4; + +export type VersionCommitMatchKind = + | 'retained' + | 'rewritten-same-patch' + | 'likely-revised' + | 'absorbed-into-base' + | 'added' + | 'removed' + | 'ambiguous'; + +export type VersionCommitSummary = { + authoredAt: string; + authorName: string; + diffStat?: { + additions: number; + deletions: number; + filesChanged: number; + }; + parentIds: ReadonlyArray; + sha: string; + shortSha: string; + subject: string; + webUrl: string; +}; + +export type CommitPatchSignature = { + additions: number; + changedPaths: ReadonlyArray; + changeTokenSketch: ReadonlyArray; + commitSha: string; + deletions: number; + exactPatchId: string; + filesChanged: number; + subjectKey: string; +}; + +export type VersionRebaseDriverCommit = { + authoredAt: string; + authorName: string; + /** Paths this base commit shares with the revised MR commit interdiff. */ + overlappingPaths: ReadonlyArray; + sha: string; + shortSha: string; + subject: string; + webUrl: string; +}; + +export type VersionCommitEvolutionUnit = { + after?: VersionCommitSummary; + baseCommit?: VersionCommitSummary; + before?: VersionCommitSummary; + confidence: 'exact' | 'high' | 'unmatched'; + id: string; + kind: VersionCommitMatchKind; + matchReasons?: ReadonlyArray; + matchScore?: number; + order: number; + /** + * Base-branch commits that likely forced this commit rewrite during rebase. + * Present only for revised units when base movement can be attributed. + */ + rebaseDrivers?: ReadonlyArray; + reviewable: boolean; +}; + +/** Minimal endpoint identity needed for evolution range bookkeeping. */ +export type DiffEndpointRef = { + baseSha: string; + headSha: string; + id?: string; + label?: string; + startSha?: string; + createdAt?: string; +}; + +export type CommitStackEvolutionRange = { + from: DiffEndpointRef; + to: DiffEndpointRef; + paths?: ReadonlyArray; +}; + +export type CommitStackEvolution = { + range: CommitStackEvolutionRange; + recommendation: { + reason: string; + structure: 'commit-by-commit' | 'whole-diff'; + }; + summary: { + absorbedIntoBase: number; + added: number; + ambiguous: number; + pairingCoverage: number; + removed: number; + retained: number; + reviewable: number; + revised: number; + rewrittenSamePatch: number; + }; + units: ReadonlyArray; + warnings?: ReadonlyArray; +}; + +/** @deprecated Prefer CommitStackEvolution. */ +export type MergeRequestVersionCommitEvolution = CommitStackEvolution; + +type CommitLike = { + authoredDate: string; + authorName: string; + message: string; + parentIds: ReadonlyArray; + sha: string; + shortSha: string; + title: string; + webUrl: string; +}; + +const normalizedSubject = (value: string) => + value + .toLowerCase() + .replaceAll(/^(?:fixup!|squash!|amend!)\s*/g, '') + .replaceAll(/[^a-z0-9]+/g, ' ') + .trim(); + +const lineCount = (files: ReadonlyArray) => { + let additions = 0; + let deletions = 0; + for (const file of files) { + for (const section of file.sections) { + for (const line of section.patch.split('\n')) { + if (line.startsWith('+') && !line.startsWith('+++')) { + additions += 1; + } + if (line.startsWith('-') && !line.startsWith('---')) { + deletions += 1; + } + } + } + } + return { additions, deletions }; +}; + +const normalizePatch = (files: ReadonlyArray) => + files + .map((file) => { + const changedLines = file.sections + .flatMap((section) => section.patch.split('\n')) + .filter( + (line) => + (line.startsWith('+') || line.startsWith('-')) && + !line.startsWith('+++') && + !line.startsWith('---'), + ) + .map((line) => `${line[0]}${line.slice(1).trim().replaceAll(/\s+/g, ' ')}`); + return [file.oldPath ?? file.path, file.path, file.status, ...changedLines].join('\n'); + }) + .toSorted() + .join('\n--file--\n'); + +const sketchHash = (value: string) => { + let hash = 2_166_136_261; + for (let index = 0; index < value.length; index += 1) { + hash ^= value.charCodeAt(index); + hash = Math.imul(hash, 16_777_619); + } + return (hash >>> 0).toString(16).padStart(8, '0'); +}; + +export const createCommitPatchSignature = async ( + commit: Pick, + files: ReadonlyArray, +): Promise => { + const { additions, deletions } = lineCount(files); + const tokens = files + .flatMap((file) => file.sections) + .flatMap((section) => section.patch.split('\n')) + .filter( + (line) => + (line.startsWith('+') || line.startsWith('-')) && + !line.startsWith('+++') && + !line.startsWith('---'), + ) + .flatMap( + (line) => + line + .slice(1) + .toLowerCase() + .match(/[a-z_][a-z0-9_]*|\d+|[^\s\w]/g) ?? [], + ) + .map(sketchHash); + return { + additions, + changedPaths: [ + ...new Set(files.flatMap((file) => [file.oldPath, file.path]).filter(Boolean)), + ].toSorted() as Array, + changeTokenSketch: [...new Set(tokens)].toSorted().slice(0, 128), + commitSha: commit.sha, + deletions, + exactPatchId: await sha256(normalizePatch(files)), + filesChanged: files.length, + subjectKey: normalizedSubject(commit.title), + }; +}; + +export const toVersionCommitSummary = ( + commit: CommitLike, + signature?: CommitPatchSignature, +): VersionCommitSummary => + ({ + authoredAt: commit.authoredDate, + authorName: commit.authorName, + ...(signature + ? { + diffStat: { + additions: signature.additions, + deletions: signature.deletions, + filesChanged: signature.filesChanged, + }, + } + : {}), + parentIds: commit.parentIds, + sha: commit.sha, + shortSha: commit.shortSha || commit.sha.slice(0, 8), + subject: commit.title || commit.message.split('\n')[0] || 'Commit', + webUrl: commit.webUrl, + }) as VersionCommitSummary; + +const jaccard = (first: ReadonlyArray, second: ReadonlyArray) => { + const left = new Set(first); + const right = new Set(second); + const union = new Set([...left, ...right]); + if (union.size === 0) { + return 1; + } + let intersection = 0; + for (const value of left) { + if (right.has(value)) { + intersection += 1; + } + } + return intersection / union.size; +}; + +const sizeSimilarity = (first: CommitPatchSignature, second: CommitPatchSignature) => { + const left = first.additions + first.deletions; + const right = second.additions + second.deletions; + return Math.max(left, right) === 0 ? 1 : Math.min(left, right) / Math.max(left, right); +}; + +const scoreCandidate = ( + oldCommit: CommitLike, + newCommit: CommitLike, + oldSignature: CommitPatchSignature, + newSignature: CommitPatchSignature, + oldPosition: number, + newPosition: number, + oldLength: number, + newLength: number, +) => { + const subject = jaccard(oldSignature.subjectKey.split(' '), newSignature.subjectKey.split(' ')); + const author = oldCommit.authorName.toLowerCase() === newCommit.authorName.toLowerCase() ? 1 : 0; + const paths = jaccard(oldSignature.changedPaths, newSignature.changedPaths); + const tokens = jaccard(oldSignature.changeTokenSketch, newSignature.changeTokenSketch); + const size = sizeSimilarity(oldSignature, newSignature); + const relativePosition = + 1 - Math.min(1, Math.abs(oldPosition / oldLength - newPosition / newLength)); + const score = + subject * 0.22 + + author * 0.08 + + paths * 0.24 + + tokens * 0.32 + + size * 0.1 + + relativePosition * 0.04; + const reasons = [ + subject >= 0.7 ? 'similar subject' : null, + author ? 'same author' : null, + paths >= 0.6 ? 'overlapping paths' : null, + tokens >= 0.6 ? 'similar changed lines' : null, + size >= 0.7 ? 'similar patch size' : null, + ].filter((reason): reason is string => reason != null); + return { reasons, score }; +}; + +const unitId = async ( + range: CommitStackEvolutionRange, + kind: VersionCommitMatchKind, + before?: string, + after?: string, +) => + `vcu-${( + await sha256( + [ + range.from.baseSha, + range.from.headSha, + range.to.baseSha, + range.to.headSha, + kind, + before, + after, + ].join(':'), + ) + ).slice(0, 20)}`; + +export const recommendVersionWalkthroughStructure = ({ + ambiguous, + failedUnitDiffs = 0, + pairingCoverage, + reviewable, + tinyUnits = 0, +}: { + ambiguous: number; + failedUnitDiffs?: number; + pairingCoverage: number; + reviewable: number; + tinyUnits?: number; +}): CommitStackEvolution['recommendation'] => { + if (reviewable <= 1) { + return { + reason: 'A whole diff is clearer for zero or one changed commit.', + structure: 'whole-diff', + }; + } + if (failedUnitDiffs > 0) { + return { reason: 'Some commit-unit diffs could not be materialized.', structure: 'whole-diff' }; + } + if (pairingCoverage < 0.8) { + return { reason: 'Commit pairing confidence is below 80%.', structure: 'whole-diff' }; + } + if (ambiguous > Math.max(1, Math.floor(reviewable * 0.2))) { + return { reason: 'Too many stack changes are ambiguous.', structure: 'whole-diff' }; + } + if (tinyUnits > reviewable / 2) { + return { + reason: 'The stack is dominated by tiny mechanical changes.', + structure: 'whole-diff', + }; + } + return { + reason: `Review ${reviewable} changed commit units in stack order.`, + structure: 'commit-by-commit', + }; +}; + +export const matchVersionCommitStacks = async ({ + baseCommits = [], + baseStackComplete = true, + from, + newCommits, + oldCommits, + signatures, + stackCompleteness = { new: true, old: true }, + to, + warnings = [], +}: { + baseCommits?: ReadonlyArray; + baseStackComplete?: boolean; + from: DiffEndpointRef; + newCommits: ReadonlyArray; + oldCommits: ReadonlyArray; + signatures: ReadonlyMap; + stackCompleteness?: { new: boolean; old: boolean }; + to: DiffEndpointRef; + warnings?: ReadonlyArray; +}): Promise => { + const range = { from, to }; + const matches: Array<{ + after?: CommitLike; + baseCommit?: CommitLike; + before?: CommitLike; + confidence: VersionCommitEvolutionUnit['confidence']; + kind: VersionCommitMatchKind; + reasons?: ReadonlyArray; + score?: number; + }> = []; + const usedOld = new Set(); + const usedNew = new Set(); + const usedBase = new Set(); + const newBySha = new Map(newCommits.map((commit) => [commit.sha, commit])); + + for (const commit of oldCommits) { + const after = newBySha.get(commit.sha); + if (!after) { + continue; + } + usedOld.add(commit.sha); + usedNew.add(after.sha); + matches.push({ after, before: commit, confidence: 'exact', kind: 'retained' }); + } + + const oldByPatch = new Map>(); + const newByPatch = new Map>(); + const ambiguousOld = new Set(); + const ambiguousNew = new Set(); + for (const commit of oldCommits.filter((entry) => !usedOld.has(entry.sha))) { + const signature = signatures.get(commit.sha); + if (signature) { + oldByPatch.set(signature.exactPatchId, [ + ...(oldByPatch.get(signature.exactPatchId) ?? []), + commit, + ]); + } + } + for (const commit of newCommits.filter((entry) => !usedNew.has(entry.sha))) { + const signature = signatures.get(commit.sha); + if (signature) { + newByPatch.set(signature.exactPatchId, [ + ...(newByPatch.get(signature.exactPatchId) ?? []), + commit, + ]); + } + } + for (const [patchId, oldMatches] of oldByPatch) { + const newMatches = newByPatch.get(patchId) ?? []; + if (newMatches.length > 0 && (oldMatches.length > 1 || newMatches.length > 1)) { + oldMatches.forEach((commit) => ambiguousOld.add(commit.sha)); + newMatches.forEach((commit) => ambiguousNew.add(commit.sha)); + continue; + } + if (oldMatches.length !== 1 || newMatches.length !== 1) { + continue; + } + const before = oldMatches[0]!; + const after = newMatches[0]!; + usedOld.add(before.sha); + usedNew.add(after.sha); + matches.push({ after, before, confidence: 'exact', kind: 'rewritten-same-patch' }); + } + + const exactAnchors = matches + .filter((match) => match.before && match.after) + .map((match) => ({ + newIndex: newCommits.findIndex((commit) => commit.sha === match.after!.sha), + oldIndex: oldCommits.findIndex((commit) => commit.sha === match.before!.sha), + })); + const respectsExactAnchors = (oldIndex: number, newIndex: number) => + exactAnchors.every( + (anchor) => + (oldIndex < anchor.oldIndex && newIndex < anchor.newIndex) || + (oldIndex > anchor.oldIndex && newIndex > anchor.newIndex), + ); + + let lastNewIndex = -1; + for (let oldIndex = 0; oldIndex < oldCommits.length; oldIndex += 1) { + const before = oldCommits[oldIndex]!; + if (usedOld.has(before.sha) || ambiguousOld.has(before.sha)) { + continue; + } + const oldSignature = signatures.get(before.sha); + if (!oldSignature || before.parentIds.length > 1) { + continue; + } + const candidates = newCommits + .map((after, newIndex) => ({ after, newIndex })) + .filter( + ({ after, newIndex }) => + !usedNew.has(after.sha) && + !ambiguousNew.has(after.sha) && + newIndex > lastNewIndex && + respectsExactAnchors(oldIndex, newIndex) && + after.parentIds.length <= 1, + ) + .map(({ after, newIndex }) => { + const signature = signatures.get(after.sha); + return signature + ? { + after, + newIndex, + ...scoreCandidate( + before, + after, + oldSignature, + signature, + oldIndex, + newIndex, + Math.max(1, oldCommits.length - 1), + Math.max(1, newCommits.length - 1), + ), + } + : null; + }) + .filter((candidate): candidate is NonNullable => candidate != null) + .toSorted((first, second) => second.score - first.score); + const best = candidates[0]; + const runnerUp = candidates[1]; + if (!best || best.score < 0.72 || (runnerUp && best.score - runnerUp.score < 0.12)) { + if (best && best.score >= 0.55) { + ambiguousOld.add(before.sha); + ambiguousNew.add(best.after.sha); + } + continue; + } + usedOld.add(before.sha); + usedNew.add(best.after.sha); + lastNewIndex = best.newIndex; + matches.push({ + after: best.after, + before, + confidence: 'high', + kind: 'likely-revised', + reasons: best.reasons, + score: Number(best.score.toFixed(3)), + }); + } + + const unmatchedOld = () => oldCommits.filter((commit) => !usedOld.has(commit.sha)); + const baseBySha = new Map(baseCommits.map((commit) => [commit.sha, commit])); + for (const before of unmatchedOld()) { + const baseCommit = baseBySha.get(before.sha); + if (!baseCommit || usedBase.has(baseCommit.sha)) { + continue; + } + usedOld.add(before.sha); + usedBase.add(baseCommit.sha); + matches.push({ + baseCommit, + before, + confidence: 'exact', + kind: 'absorbed-into-base', + reasons: ['Commit is now present in the later target base'], + }); + } + + const baseByPatch = new Map>(); + for (const commit of baseCommits.filter((entry) => !usedBase.has(entry.sha))) { + const signature = signatures.get(commit.sha); + if (signature) { + baseByPatch.set(signature.exactPatchId, [ + ...(baseByPatch.get(signature.exactPatchId) ?? []), + commit, + ]); + } + } + for (const before of unmatchedOld()) { + const signature = signatures.get(before.sha); + if (!signature) { + continue; + } + const candidates = baseByPatch + .get(signature.exactPatchId) + ?.filter((commit) => !usedBase.has(commit.sha)); + if (candidates?.length !== 1) { + continue; + } + const baseCommit = candidates[0]!; + usedOld.add(before.sha); + usedBase.add(baseCommit.sha); + matches.push({ + baseCommit, + before, + confidence: 'exact', + kind: 'absorbed-into-base', + reasons: ['Equivalent patch is now present in the later target base'], + }); + } + + for (let oldIndex = 0; oldIndex < oldCommits.length; oldIndex += 1) { + const before = oldCommits[oldIndex]!; + if (usedOld.has(before.sha) || ambiguousOld.has(before.sha)) { + continue; + } + const oldSignature = signatures.get(before.sha); + if (!oldSignature || before.parentIds.length > 1) { + continue; + } + const candidates = baseCommits + .map((baseCommit, baseIndex) => ({ baseCommit, baseIndex })) + .filter(({ baseCommit }) => !usedBase.has(baseCommit.sha) && baseCommit.parentIds.length <= 1) + .map(({ baseCommit, baseIndex }) => { + const signature = signatures.get(baseCommit.sha); + return signature + ? { + baseCommit, + ...scoreCandidate( + before, + baseCommit, + oldSignature, + signature, + oldIndex, + baseIndex, + Math.max(1, oldCommits.length - 1), + Math.max(1, baseCommits.length - 1), + ), + } + : null; + }) + .filter((candidate): candidate is NonNullable => candidate != null) + .toSorted((first, second) => second.score - first.score); + const best = candidates[0]; + const runnerUp = candidates[1]; + if (!best || best.score < 0.72 || (runnerUp && best.score - runnerUp.score < 0.12)) { + continue; + } + usedOld.add(before.sha); + usedBase.add(best.baseCommit.sha); + matches.push({ + baseCommit: best.baseCommit, + before, + confidence: 'high', + kind: 'absorbed-into-base', + reasons: [...best.reasons, 'Logical commit is now present in the later target base'], + score: Number(best.score.toFixed(3)), + }); + } + + const canClassifyUnpaired = stackCompleteness.old && stackCompleteness.new; + const canClassifyRemoved = canClassifyUnpaired && baseStackComplete; + for (const commit of oldCommits) { + if (!usedOld.has(commit.sha)) { + const classificationIsIncomplete = + !canClassifyRemoved || !signatures.has(commit.sha) || ambiguousOld.has(commit.sha); + matches.push({ + before: commit, + confidence: 'unmatched', + kind: classificationIsIncomplete ? 'ambiguous' : 'removed', + ...(classificationIsIncomplete + ? { + reasons: ['Insufficient evidence to classify this commit as removed'], + } + : {}), + }); + } + } + for (const commit of newCommits) { + if (!usedNew.has(commit.sha)) { + const classificationIsIncomplete = + !canClassifyUnpaired || !signatures.has(commit.sha) || ambiguousNew.has(commit.sha); + matches.push({ + after: commit, + confidence: 'unmatched', + kind: classificationIsIncomplete ? 'ambiguous' : 'added', + ...(classificationIsIncomplete + ? { + reasons: ['Insufficient evidence to classify this commit as new'], + } + : {}), + }); + } + } + + const oldOrder = new Map(oldCommits.map((commit, index) => [commit.sha, index])); + const newOrder = new Map(newCommits.map((commit, index) => [commit.sha, index])); + const units = await Promise.all( + matches.map(async (match) => { + const beforeSignature = match.before ? signatures.get(match.before.sha) : undefined; + const afterSignature = match.after ? signatures.get(match.after.sha) : undefined; + const baseSignature = match.baseCommit ? signatures.get(match.baseCommit.sha) : undefined; + const oldIndex = match.before + ? (oldOrder.get(match.before.sha) ?? oldCommits.length) + : oldCommits.length; + const nextMatched = match.baseCommit + ? matches + .filter( + (candidate) => + candidate.before && + candidate.after && + (oldOrder.get(candidate.before.sha) ?? oldCommits.length) > oldIndex, + ) + .toSorted( + (first, second) => + (oldOrder.get(first.before!.sha) ?? oldCommits.length) - + (oldOrder.get(second.before!.sha) ?? oldCommits.length), + )[0] + : undefined; + const order = match.after + ? (newOrder.get(match.after.sha) ?? newCommits.length) + : match.baseCommit && nextMatched?.after + ? (newOrder.get(nextMatched.after.sha) ?? 0) - + (oldOrder.get(nextMatched.before!.sha)! - oldIndex) / (oldCommits.length + 1) + : newCommits.length + oldIndex / (oldCommits.length + 1); + return { + ...(match.after ? { after: toVersionCommitSummary(match.after, afterSignature) } : {}), + ...(match.baseCommit + ? { baseCommit: toVersionCommitSummary(match.baseCommit, baseSignature) } + : {}), + ...(match.before ? { before: toVersionCommitSummary(match.before, beforeSignature) } : {}), + confidence: match.confidence, + id: await unitId( + range, + match.kind, + match.before?.sha, + match.after?.sha ?? match.baseCommit?.sha, + ), + kind: match.kind, + ...(match.reasons ? { matchReasons: match.reasons } : {}), + ...(match.score != null ? { matchScore: match.score } : {}), + order, + reviewable: + (match.kind === 'likely-revised' || match.kind === 'added' || match.kind === 'removed') && + (match.before?.parentIds.length ?? 0) <= 1 && + (match.after?.parentIds.length ?? 0) <= 1, + } satisfies VersionCommitEvolutionUnit; + }), + ); + units.sort((first, second) => first.order - second.order || first.id.localeCompare(second.id)); + const count = (kind: VersionCommitMatchKind) => units.filter((unit) => unit.kind === kind).length; + const revised = count('likely-revised'); + const absorbedIntoBase = count('absorbed-into-base'); + const removed = count('removed'); + const added = count('added'); + const ambiguous = count('ambiguous'); + const pairingDenominator = revised + Math.min(removed, added); + const pairingCoverage = pairingDenominator === 0 ? 1 : revised / pairingDenominator; + const reviewable = units.filter((unit) => unit.reviewable).length; + return { + range, + recommendation: recommendVersionWalkthroughStructure({ + ambiguous, + pairingCoverage, + reviewable, + }), + summary: { + absorbedIntoBase, + added, + ambiguous, + pairingCoverage, + removed, + retained: count('retained'), + reviewable, + revised, + rewrittenSamePatch: count('rewritten-same-patch'), + }, + units, + ...(warnings.length ? { warnings } : {}), + }; +}; + +const pathTokens = (paths: ReadonlyArray) => + paths + .flatMap((path) => path.toLowerCase().split(/[^a-z0-9]+/g)) + .filter((token) => token.length > 2); + +export const scoreBaseCommitAsRebaseDriver = ({ + baseSignature, + unitSignature, +}: { + baseSignature: Pick< + CommitPatchSignature, + 'changedPaths' | 'changeTokenSketch' | 'additions' | 'deletions' + >; + unitSignature: Pick< + CommitPatchSignature, + 'changedPaths' | 'changeTokenSketch' | 'additions' | 'deletions' + >; +}) => { + const overlappingPaths = unitSignature.changedPaths.filter((path) => + baseSignature.changedPaths.includes(path), + ); + const pathOverlap = + unitSignature.changedPaths.length === 0 + ? 0 + : overlappingPaths.length / unitSignature.changedPaths.length; + const tokens = jaccard(unitSignature.changeTokenSketch, baseSignature.changeTokenSketch); + const pathNameTokens = jaccard( + pathTokens(unitSignature.changedPaths), + pathTokens(baseSignature.changedPaths), + ); + const left = unitSignature.additions + unitSignature.deletions; + const right = baseSignature.additions + baseSignature.deletions; + const size = Math.max(left, right) === 0 ? 1 : Math.min(left, right) / Math.max(left, right); + // Prefer path overlap heavily: a base commit that touches the same files is the + // strongest signal that the MR commit was revised due to rebase conflict fallout. + const score = pathOverlap * 0.62 + tokens * 0.28 + pathNameTokens * 0.06 + size * 0.04; + return { + overlappingPaths, + score: Number(score.toFixed(3)), + }; +}; + +export const attributeRebaseDrivers = ({ + baseCommits, + baseSignatures, + limit = 3, + unitSignature, +}: { + baseCommits: ReadonlyArray<{ + authoredAt: string; + authorName: string; + sha: string; + shortSha: string; + subject: string; + webUrl: string; + }>; + baseSignatures: ReadonlyMap; + limit?: number; + unitSignature: CommitPatchSignature | null | undefined; +}): Array => { + if (!unitSignature || unitSignature.changedPaths.length === 0 || baseCommits.length === 0) { + return []; + } + return baseCommits + .map((commit) => { + const signature = baseSignatures.get(commit.sha); + if (!signature) { + return null; + } + const { overlappingPaths, score } = scoreBaseCommitAsRebaseDriver({ + baseSignature: signature, + unitSignature, + }); + if (score < 0.22 || overlappingPaths.length === 0) { + return null; + } + return { + authoredAt: commit.authoredAt, + authorName: commit.authorName, + overlappingPaths, + score, + sha: commit.sha, + shortSha: commit.shortSha, + subject: commit.subject, + webUrl: commit.webUrl, + }; + }) + .filter((entry): entry is NonNullable => entry != null) + .toSorted((first, second) => second.score - first.score || first.sha.localeCompare(second.sha)) + .slice(0, limit) + .map(({ score: _score, ...commit }) => commit); +}; diff --git a/core/lib/crypto.ts b/core/lib/crypto.ts new file mode 100644 index 00000000..c0e06c34 --- /dev/null +++ b/core/lib/crypto.ts @@ -0,0 +1,5 @@ +const bytesToHex = (buffer: ArrayBuffer) => + [...new Uint8Array(buffer)].map((byte) => byte.toString(16).padStart(2, '0')).join(''); + +export const sha256 = async (value: string) => + bytesToHex(await crypto.subtle.digest('SHA-256', new TextEncoder().encode(value))); From e60b4c25e562f3d20945cc17cb21f958d2d09500 Mon Sep 17 00:00:00 2001 From: Matt Alonso Date: Mon, 20 Jul 2026 16:25:48 -0500 Subject: [PATCH 4/8] Add GitLab review-history transport and analysis Introduce the GitLab transport abstraction and read-side provider implementation for versions, commit diffs, comparisons, review strategy, and commit evolution. Build the provider on Core review-history contracts and the shared matcher so Codiff Web can supply authenticated HTTP transport without carrying provider logic in its host. --- core/index.ts | 86 + gitlab/__tests__/history-transport.test.ts | 113 ++ gitlab/package.json | 36 + gitlab/src/crypto.ts | 5 + gitlab/src/history.ts | 1778 ++++++++++++++++++++ gitlab/src/index.ts | 5 + gitlab/src/review-strategy.ts | 335 ++++ gitlab/src/transport.ts | 139 ++ gitlab/src/version-commit-evolution.ts | 29 + gitlab/src/version-compare.ts | 1341 +++++++++++++++ gitlab/tsconfig.build.json | 13 + gitlab/vite.config.ts | 8 + package.json | 2 +- pnpm-lock.yaml | 12 +- pnpm-workspace.yaml | 1 + vite.config.ts | 7 + 16 files changed, 3903 insertions(+), 7 deletions(-) create mode 100644 gitlab/__tests__/history-transport.test.ts create mode 100644 gitlab/package.json create mode 100644 gitlab/src/crypto.ts create mode 100644 gitlab/src/history.ts create mode 100644 gitlab/src/index.ts create mode 100644 gitlab/src/review-strategy.ts create mode 100644 gitlab/src/transport.ts create mode 100644 gitlab/src/version-commit-evolution.ts create mode 100644 gitlab/src/version-compare.ts create mode 100644 gitlab/tsconfig.build.json create mode 100644 gitlab/vite.config.ts diff --git a/core/index.ts b/core/index.ts index 0a5d9053..e1f8bd16 100644 --- a/core/index.ts +++ b/core/index.ts @@ -1,4 +1,43 @@ export { defaultReviewPreferences } from './defaults.ts'; +export { + commitRevisionLabel, + diffComparison, + diffComparisonView, + diffRange, + evolutionUnitCommit, + evolutionUnitRebaseDrivers, + isReviewableUnit, + resolveReviewPlan, + reviewableUnits, + reviewCommitEvolution, + reviewVersionOption, + revisionRef, + versionOptionBaseCommitId, + versionOptionHeadCommitId, + versionOptionLabelText, + versionRevisionLabel, +} from './lib/review-history.ts'; + +export { + attributeRebaseDrivers, + createCommitPatchSignature, + matchVersionCommitStacks, + recommendVersionWalkthroughStructure, + scoreBaseCommitAsRebaseDriver, + toVersionCommitSummary, + versionCommitDiffConcurrency, + versionCommitEvolutionAlgorithmVersion, + versionCommitSignatureAlgorithmVersion, + versionCommitStackLimit, + type CommitPatchSignature, + type CommitStackEvolution, + type CommitStackEvolutionRange, + type DiffEndpointRef, + type VersionCommitEvolutionUnit, + type VersionCommitMatchKind, + type VersionCommitSummary, + type VersionRebaseDriverCommit, +} from './lib/commit-stack-evolution.ts'; export { parsePlanShareManifest, parsePlanShareUpload, @@ -9,9 +48,19 @@ export type { ChangedFile, CodiffFeatureFlags, CodiffPreferences, + DiffComparison, + DiffComparisonAnalysis, + DiffComparisonBaseMovement, + DiffComparisonBaseMovementCommit, + DiffComparisonCommentAssociation, + DiffComparisonSummary, + DiffComparisonView, + DiffRange, DiffSection, GitIdentity, NarrativeWalkthrough, + PullRequestAIReview, + PullRequestAIReviewDecision, PlanCommentThread, PullRequestCodeQualityFinding, PullRequestGeneralComment, @@ -23,13 +72,50 @@ export type { PullRequestReviewEvent, PullRequestReviewStatus, PullRequestReviewer, + ReviewCommitEvolution, + ReviewCommitListEntry, + ReviewCommitSummary, + ReviewEvolutionMarkerUnit, + ReviewEvolutionSummary, + ReviewEvolutionUnit, + ReviewPlan, ReviewPreferences, + ReviewRebaseDriverCommit, + ReviewStrategySummary, + ReviewStructureRecommendation, + ReviewUnit, + ReviewVersionOption, RepositoryState, ReviewSource, + RevisionLabel, + RevisionRef, SharePlanResult, ShareWalkthroughResult, SharedPlanSnapshot, SharedWalkthroughSnapshot, + WalkthroughCommentReference, + WalkthroughGenerationInput, WalkthroughShareManifestV1, WalkthroughHunk, } from './types.ts'; +export { + formatVersionElapsedDuration, + MergeRequestReviewApp, + ReadOnlyGeneralCommentCard, + ReviewSurface, + type MergeRequestCommitListEntry, + type MergeRequestReviewAppProps, + type MergeRequestReviewMode, + type MergeRequestReviewStrategySummary, + type MergeRequestVersionBaseMovement, + type MergeRequestVersionBaseMovementCommit, + type MergeRequestVersionCommitEvolution, + type MergeRequestVersionCommitEvolutionUnit, + type MergeRequestVersionCommitSummary, + type MergeRequestVersionCompareCommentAssociation, + type MergeRequestVersionCompareSummary, + type MergeRequestVersionCompareView, + type MergeRequestVersionOption, + type MergeRequestVersionRebaseDriverCommit, + type MergeRequestWalkthroughStatus, +} from './SharedWalkthroughApp.tsx'; diff --git a/gitlab/__tests__/history-transport.test.ts b/gitlab/__tests__/history-transport.test.ts new file mode 100644 index 00000000..3fc167cb --- /dev/null +++ b/gitlab/__tests__/history-transport.test.ts @@ -0,0 +1,113 @@ +import { expect, test } from 'vite-plus/test'; +import { + createFakeGitLabTransport, + fetchGitLabMergeRequestVersions, + projectCommitEvolution, + projectReviewPlan, + projectVersionCompare, + toGitLabDiffIdentity, +} from '../src/index.ts'; +import { matchVersionCommitStacks, createCommitPatchSignature } from '../src/version-commit-evolution.ts'; + +test('loads merge request versions through the injected transport', async () => { + const transport = createFakeGitLabTransport([ + { + path: '/api/v4/projects/group%2Fproject/merge_requests/7/versions', + response: [ + { + id: 2, + head_commit_sha: 'b'.repeat(40), + base_commit_sha: 'a'.repeat(40), + start_commit_sha: 'a'.repeat(40), + created_at: '2026-01-02T00:00:00.000Z', + }, + { + id: 1, + head_commit_sha: 'c'.repeat(40), + base_commit_sha: 'a'.repeat(40), + start_commit_sha: 'a'.repeat(40), + created_at: '2026-01-01T00:00:00.000Z', + }, + ], + }, + ]); + + const versions = await fetchGitLabMergeRequestVersions({ + iid: 7, + projectPath: 'group/project', + transport, + }); + + expect(versions).toHaveLength(2); + expect(versions[0]?.id).toBe('2'); + expect(versions[0]?.label).toContain('v2'); + expect(toGitLabDiffIdentity(versions[0]!).headSha).toBe('b'.repeat(40)); + expect(transport.calls[0]?.path).toContain('/merge_requests/7/versions'); +}); + +test('projects algorithm evolution into Core review plans', async () => { + const from = { + baseSha: 'a'.repeat(40), + createdAt: '2026-01-01T00:00:00.000Z', + headSha: 'b'.repeat(40), + id: '1', + label: 'v1', + startSha: 'a'.repeat(40), + }; + const to = { + baseSha: 'a'.repeat(40), + createdAt: '2026-01-02T00:00:00.000Z', + headSha: 'c'.repeat(40), + id: '2', + label: 'v2', + startSha: 'a'.repeat(40), + }; + const oldCommit = { + authoredDate: '2026-01-01T00:00:00.000Z', + authorName: 'Ada', + message: 'feat: one\n', + parentIds: [from.baseSha], + sha: 'd'.repeat(40), + shortSha: 'ddddddd', + title: 'feat: one', + webUrl: 'https://example.test/d', + }; + const newCommit = { + ...oldCommit, + sha: 'e'.repeat(40), + shortSha: 'eeeeeee', + }; + const files = [ + { + fingerprint: 'f', + path: 'a.ts', + sections: [ + { + binary: false, + id: 'a.ts:commit:1', + kind: 'commit' as const, + patch: 'diff --git a/a.ts b/a.ts\n--- a/a.ts\n+++ b/a.ts\n@@ -1 +1 @@\n-old\n+new\n', + }, + ], + status: 'modified' as const, + }, + ]; + const oldSig = await createCommitPatchSignature(oldCommit, files); + const newSig = await createCommitPatchSignature(newCommit, files); + const evolution = await matchVersionCommitStacks({ + from, + newCommits: [newCommit], + oldCommits: [oldCommit], + signatures: new Map([ + [oldCommit.sha, oldSig], + [newCommit.sha, newSig], + ]), + to, + }); + const projected = projectCommitEvolution(evolution); + expect(projected.units.some((unit) => unit.kind === 'revised' || unit.kind === 'rewritten-same-patch' || unit.kind === 'retained')).toBe( + true, + ); + const plan = projectReviewPlan({ evolution, structure: 'auto' }); + expect(plan.structure === 'whole-diff' || plan.structure === 'units').toBe(true); +}); diff --git a/gitlab/package.json b/gitlab/package.json new file mode 100644 index 00000000..646baf3c --- /dev/null +++ b/gitlab/package.json @@ -0,0 +1,36 @@ +{ + "name": "@nkzw/codiff-gitlab", + "version": "0.1.0", + "description": "Read-side GitLab review-history adapter for Codiff.", + "license": "MIT", + "author": { + "name": "Christoph Nakazawa", + "email": "christoph.pojer@gmail.com" + }, + "repository": { + "type": "git", + "url": "https://github.com/nkzw-tech/codiff.git", + "directory": "gitlab" + }, + "files": [ + "dist", + "src" + ], + "type": "module", + "main": "./dist/index.mjs", + "types": "./src/index.ts", + "exports": { + ".": { + "types": "./src/index.ts", + "@nkzw/codiff-source": "./src/index.ts", + "default": "./dist/index.mjs" + } + }, + "scripts": { + "build": "rm -rf dist && vp pack -d dist --target=node24 src/index.ts && vp exec tsc -p tsconfig.build.json", + "typecheck": "vp exec tsc --noEmit -p tsconfig.build.json" + }, + "dependencies": { + "@nkzw/codiff-core": "workspace:*" + } +} diff --git a/gitlab/src/crypto.ts b/gitlab/src/crypto.ts new file mode 100644 index 00000000..c0e06c34 --- /dev/null +++ b/gitlab/src/crypto.ts @@ -0,0 +1,5 @@ +const bytesToHex = (buffer: ArrayBuffer) => + [...new Uint8Array(buffer)].map((byte) => byte.toString(16).padStart(2, '0')).join(''); + +export const sha256 = async (value: string) => + bytesToHex(await crypto.subtle.digest('SHA-256', new TextEncoder().encode(value))); diff --git a/gitlab/src/history.ts b/gitlab/src/history.ts new file mode 100644 index 00000000..ce84c515 --- /dev/null +++ b/gitlab/src/history.ts @@ -0,0 +1,1778 @@ +/** + * Read-side GitLab review-history adapter over {@link GitLabTransport}. + */ +import type { + ChangedFile, + DiffComparisonAnalysis, + DiffComparisonView, + ReviewCommitEvolution, + ReviewCommitSummary, + ReviewEvolutionUnit, + ReviewPlan, + ReviewVersionOption, +} from '@nkzw/codiff-core/types'; +import { + commitRevisionLabel, + diffComparison, + diffComparisonView, + diffRange, + resolveReviewPlan, + reviewVersionOption, + revisionRef, + versionRevisionLabel, +} from '@nkzw/codiff-core'; +import type { GitLabTransport } from './transport.ts'; +import { + attributeRebaseDrivers, + createCommitPatchSignature, + matchVersionCommitStacks, + versionCommitDiffConcurrency, + versionCommitStackLimit, + type CommitPatchSignature, + type MergeRequestVersionCommitEvolution, + type VersionCommitEvolutionUnit, + type VersionRebaseDriverCommit, +} from './version-commit-evolution.ts'; +import { + computeVersionComparePreferringReplay, + type CommentAnchor, + type MergeRequestVersionCompare, + type MergeRequestVersionRef, + type VersionBaseMovement, + type VersionCompareEndpoint, + type VersionPatchFile, +} from './version-compare.ts'; + +export type { GitLabTransport } from './transport.ts'; +export type { + CommentAnchor, + MergeRequestVersionCompare, + MergeRequestVersionRef, + VersionCompareEndpoint, + VersionPatchFile, +} from './version-compare.ts'; +export type { + CommitPatchSignature, + MergeRequestVersionCommitEvolution, + VersionCommitEvolutionUnit, + VersionCommitMatchKind, + VersionCommitSummary, + VersionRebaseDriverCommit, +} from './version-commit-evolution.ts'; +export { + attributeRebaseDrivers, + createCommitPatchSignature, + matchVersionCommitStacks, + recommendVersionWalkthroughStructure, + toVersionCommitSummary, + versionCommitDiffConcurrency, + versionCommitEvolutionAlgorithmVersion, + versionCommitSignatureAlgorithmVersion, + versionCommitStackLimit, +} from './version-commit-evolution.ts'; +export { + applyUnifiedPatchBody, + computeApproximatePatchTextVersionCompare, + computeLineDiff, + computeRebaseReplayVersionCompare, + computeVersionComparePreferringReplay, + isMergeRequestVersionRef, + materializeRebaseReplayTrees, + versionCompareAlgorithmVersion, +} from './version-compare.ts'; +export { + classifyGitLabCommit, + classifyMergeRequestReviewStrategy, + orderCommitsTopologically, + overrideMergeRequestReviewStrategy, + reviewStructureFromStrategy, + versionCompareReviewStructureKey, + type ClassifiedCommit, + type ClassifiedCommitRole, + type GitLabMergeRequestCommitLike, + type MergeRequestReviewStrategy, +} from './review-strategy.ts'; +import { orderCommitsTopologically } from './review-strategy.ts'; + +const maxPages = 20; + +const gitLabOrigin = 'https://gitlab.cfdata.org'; + +export type GitLabDiffIdentity = { + baseSha: string; + headSha: string; + startSha: string; +}; + +export type GitLabMergeRequestCommit = { + authoredDate: string; + authorEmail: string; + authorName: string; + committedDate: string; + committerName: string; + message: string; + parentIds: ReadonlyArray; + sha: string; + shortSha: string; + title: string; + webUrl: string; +}; + +type JsonRecord = Record; +const isRecord = (value: unknown): value is JsonRecord => + Boolean(value) && typeof value === 'object' && !Array.isArray(value); +const asRecord = (value: unknown): JsonRecord => (isRecord(value) ? value : {}); +const asArray = (value: unknown): Array => (Array.isArray(value) ? value : []); +const asString = (value: unknown, fallback = '') => (typeof value === 'string' ? value : fallback); +const asNumber = (value: unknown) => + typeof value === 'number' && Number.isFinite(value) ? value : null; +const trimmedString = (value: unknown) => { + if (typeof value !== 'string') return null; + const trimmed = value.trim(); + return trimmed ? trimmed : null; +}; + +export const validateProjectPath = (projectPath: string) => { + const normalized = projectPath.trim().replaceAll(/^\/+|\/+$/g, ''); + if ( + !normalized || + normalized.length > 500 || + !normalized.includes('/') || + normalized.split('/').some((segment) => !segment || segment === '.' || segment === '..') + ) { + throw new Error('Invalid GitLab project path.'); + } + return normalized; +}; + +const mergeRequestEndpoint = (projectPath: string, iid: number, suffix = '') => + `${gitLabOrigin}/api/v4/projects/${encodeURIComponent(projectPath)}/merge_requests/${iid}${suffix}`; + +const mergeRequestCommitsEndpoint = (projectPath: string, iid: number) => + mergeRequestEndpoint(projectPath, iid, '/commits'); + +const mergeRequestVersionsEndpoint = (projectPath: string, iid: number) => + mergeRequestEndpoint(projectPath, iid, '/versions'); + +const mergeRequestVersionEndpoint = (projectPath: string, iid: number, versionId: string) => + mergeRequestEndpoint(projectPath, iid, `/versions/${encodeURIComponent(versionId)}`); + +const repositoryCompareEndpoint = (projectPath: string, from: string, to: string) => { + const url = new URL( + `${gitLabOrigin}/api/v4/projects/${encodeURIComponent(projectPath)}/repository/compare`, + ); + url.searchParams.set('from', from); + url.searchParams.set('to', to); + url.searchParams.set('straight', 'true'); + return url.toString(); +}; + +const repositoryFileRawEndpoint = (projectPath: string, filePath: string, ref: string) => { + const url = new URL( + `${gitLabOrigin}/api/v4/projects/${encodeURIComponent(projectPath)}/repository/files/${encodeURIComponent(filePath)}/raw`, + ); + url.searchParams.set('ref', ref); + return url.toString(); +}; + +const repositoryCommitDiffEndpoint = (projectPath: string, sha: string) => + `${gitLabOrigin}/api/v4/projects/${encodeURIComponent(projectPath)}/repository/commits/${encodeURIComponent(sha)}/diff`; + +const repositoryCommitEndpoint = (projectPath: string, sha: string) => + `${gitLabOrigin}/api/v4/projects/${encodeURIComponent(projectPath)}/repository/commits/${encodeURIComponent(sha)}`; + + + + + + + +const createRequest = ( + pathOrUrl: string, + init: { method?: string; headers?: HeadersInit; body?: unknown } = {}, +) => { + // Accept absolute URLs from legacy helpers and reduce to path+query. + let path = pathOrUrl; + let query: Record | undefined; + if (pathOrUrl.startsWith('http://') || pathOrUrl.startsWith('https://')) { + const url = new URL(pathOrUrl); + path = url.pathname; + if (url.searchParams.toString()) { + query = Object.fromEntries(url.searchParams.entries()); + } + } else if (pathOrUrl.includes('?')) { + const url = new URL(pathOrUrl, 'https://gitlab.local'); + path = url.pathname; + query = Object.fromEntries(url.searchParams.entries()); + } + return { body: init.body, method: (init.method as any) ?? 'GET', path, query }; +}; + +const readJson = async ( + transport: GitLabTransport, + request: { path: string; query?: Record; method?: any; body?: unknown }, + _unavailableMessage: string, +) => transport.request({ + body: request.body, + method: request.method, + path: request.path, + query: request.query, +}); + +const readText = async ( + transport: GitLabTransport, + request: { path: string; query?: Record }, +) => { + if (!transport.requestText) { + throw new Error('GitLabTransport.requestText is required for raw blob reads.'); + } + return transport.requestText({ path: request.path, query: request.query }); +}; + +const readPages = async ( + transport: GitLabTransport, + url: string, +): Promise> => { + const first = createRequest(url); + if (transport.requestPages) { + return transport.requestPages({ path: first.path, query: first.query }); + } + const values: Array = []; + let page = 1; + while (page <= maxPages) { + const result = await transport.request({ + path: first.path, + query: { ...(first.query ?? {}), page, per_page: 100 }, + }); + const pageValues = asArray(result); + values.push(...pageValues); + if (pageValues.length < 100) break; + page += 1; + } + if (page > maxPages) { + throw new Error('GitLab merge request data exceeded the pagination limit.'); + } + return values; +}; + + + + + + + + + +const createPatch = (diff: JsonRecord) => { + const oldPath = asString(diff.old_path); + const newPath = asString(diff.new_path); + const body = asString(diff.diff); + const oldHeader = diff.new_file === true ? '/dev/null' : `a/${oldPath}`; + const newHeader = diff.deleted_file === true ? '/dev/null' : `b/${newPath}`; + return `diff --git a/${oldPath} b/${newPath}\n--- ${oldHeader}\n+++ ${newHeader}\n${body}${ + body.endsWith('\n') ? '' : '\n' + }`; +}; + +const normalizeDiff = ( + diffValue: unknown, + iid: number, + headSha: string, + index: number, + sectionKind: 'commit' | 'pull-request' = 'pull-request', + sectionScope: string = String(iid), +): ChangedFile | null => { + const diff = asRecord(diffValue); + const oldPath = asString(diff.old_path); + const newPath = asString(diff.new_path); + if (!oldPath || !newPath) { + return null; + } + const status = diff.new_file + ? 'added' + : diff.deleted_file + ? 'deleted' + : diff.renamed_file + ? 'renamed' + : 'modified'; + const rawPatch = asString(diff.diff); + const unavailable = !rawPatch.trim(); + const sectionId = `${newPath}:${sectionKind}:${sectionScope}`; + return { + fingerprint: `${headSha}:${index}:${status}:${oldPath}:${newPath}:${rawPatch.length}`, + ...(oldPath !== newPath ? { oldPath } : {}), + path: newPath, + sections: [ + { + binary: false, + id: sectionId, + kind: sectionKind, + loadState: unavailable ? 'too-large' : 'ready', + patch: unavailable ? '' : createPatch(diff), + ...(unavailable + ? { + summary: { + canLoad: false, + reason: + diff.too_large === true + ? 'GitLab marked this diff as too large to display.' + : 'GitLab did not return a text patch for this file.', + }, + } + : {}), + }, + ], + status, + }; +}; + +const normalizeMergeRequestCommit = ( + value: unknown, + projectPath: string, +): GitLabMergeRequestCommit | null => { + const commit = asRecord(value); + const sha = asString(commit.id ?? commit.sha); + if (!sha) { + return null; + } + const title = asString(commit.title, asString(commit.message).split('\n')[0] || sha.slice(0, 8)); + const message = asString(commit.message, title); + const shortSha = asString(commit.short_id, sha.slice(0, 8)); + const parentIds = asArray(commit.parent_ids) + .map((parent) => asString(parent)) + .filter(Boolean); + const authorName = + trimmedString(commit.author_name) ?? trimmedString(asRecord(commit.author).name) ?? 'Unknown'; + const authorEmail = + trimmedString(commit.author_email) ?? trimmedString(asRecord(commit.author).email) ?? ''; + const authoredDate = + asString(commit.authored_date) || + asString(commit.created_at) || + asString(commit.committed_date) || + new Date(0).toISOString(); + const committerName = trimmedString(commit.committer_name) ?? authorName; + const committedDate = asString(commit.committed_date) || authoredDate; + return { + authoredDate, + authorEmail, + authorName, + committedDate, + committerName, + message, + parentIds, + sha, + shortSha, + title, + webUrl: + asString(commit.web_url) || + `${gitLabOrigin}/${projectPath}/-/commit/${encodeURIComponent(sha)}`, + }; +}; + +const normalizeVersionPatchFile = (value: unknown): VersionPatchFile | null => { + const diff = asRecord(value); + const oldPath = asString(diff.old_path); + const newPath = asString(diff.new_path); + if (!oldPath || !newPath) { + return null; + } + const status = diff.new_file + ? 'added' + : diff.deleted_file + ? 'deleted' + : diff.renamed_file + ? 'renamed' + : 'modified'; + return { + newPath, + oldPath, + patchBody: asString(diff.diff), + status, + }; +}; + +const normalizeMergeRequestVersion = ( + value: unknown, + index: number, +): MergeRequestVersionRef | null => { + const version = asRecord(value); + const rawId = version.id; + const id = + (typeof rawId === 'string' && rawId) || + (typeof rawId === 'number' && Number.isFinite(rawId) ? String(rawId) : String(index + 1)); + const baseSha = asString(version.base_commit_sha); + const startSha = asString(version.start_commit_sha, baseSha); + const headSha = asString(version.head_commit_sha); + if (!baseSha || !headSha) { + return null; + } + const createdAt = asString(version.created_at, new Date(0).toISOString()); + const shortHead = headSha.slice(0, 7); + return { + baseSha, + createdAt, + headSha, + id, + label: `v${id} · ${shortHead}`, + startSha, + }; +}; + +// --- Commit / version / version-comparison GitLab surface (Fate root queries call these) --- +// Commits list → client commits mode. Commit diff → lazy onLoadCommitDiff. +// Versions + version comparison → version picker + version-comparison view (algorithm in version-compare.ts). +export const fetchGitLabMergeRequestCommits = async ({ + transport, + iid, + projectPath: rawProjectPath, +}: { + transport: GitLabTransport; + iid: number; + projectPath: string; +}): Promise> => { + const projectPath = validateProjectPath(rawProjectPath); + if (!Number.isInteger(iid) || iid <= 0) { + throw new Error('Invalid GitLab merge request IID.'); + } + const values = await readPages( + transport, + mergeRequestCommitsEndpoint(projectPath, iid), + ); + return values + .map((value) => normalizeMergeRequestCommit(value, projectPath)) + .filter((commit): commit is GitLabMergeRequestCommit => commit != null); +}; + +export const fetchGitLabCommitDiff = async ({ + transport, + projectPath: rawProjectPath, + sha, +}: { + transport: GitLabTransport; + projectPath: string; + sha: string; +}): Promise> => { + const projectPath = validateProjectPath(rawProjectPath); + const normalizedSha = sha.trim(); + if (!normalizedSha) { + throw new Error('A commit SHA is required.'); + } + const diffs = await readPages( + transport, + repositoryCommitDiffEndpoint(projectPath, normalizedSha), + ); + return diffs + .map((diff, index) => + normalizeDiff(diff, 0, normalizedSha, index, 'commit', normalizedSha.slice(0, 12)), + ) + .filter((file): file is ChangedFile => file != null) + .toSorted((first, second) => first.path.localeCompare(second.path)); +}; + +export type MergeRequestVersionDiffStat = { + additions: number; + deletions: number; + filesChanged: number; +}; + +/** + * MR-local version history. GitLab's `id` is deliberately retained only for + * server-side endpoint resolution; URLs use `number` (0 is the MR base). + */ +export type MergeRequestVersionHistoryEntry = { + createdAt: string | null; + diffStat: MergeRequestVersionDiffStat; + headSha: string; + id: string | null; + isHead: boolean; + number: number; + previousCreatedAt?: string; + previousNumber?: number; +}; + +type VersionStatCache = { + read?: (range: { + next: MergeRequestVersionRef; + previous: MergeRequestVersionRef; + }) => Promise | MergeRequestVersionDiffStat | null; + write?: ( + range: { next: MergeRequestVersionRef; previous: MergeRequestVersionRef }, + diffStat: MergeRequestVersionDiffStat, + ) => Promise | void; +}; + +const readGitLabMergeRequestVersions = async ({ + transport, + iid, + projectPath: rawProjectPath, +}: { + transport: GitLabTransport; + iid: number; + projectPath: string; +}): Promise> => { + const projectPath = validateProjectPath(rawProjectPath); + if (!Number.isInteger(iid) || iid <= 0) { + throw new Error('Invalid GitLab merge request IID.'); + } + const values = await readPages( + transport, + mergeRequestVersionsEndpoint(projectPath, iid), + ); + return values + .map((value, index) => normalizeMergeRequestVersion(value, index)) + .filter((version): version is MergeRequestVersionRef => version != null) + .toSorted((first, second) => { + const timeDifference = Date.parse(first.createdAt) - Date.parse(second.createdAt); + return timeDifference || first.id.localeCompare(second.id); + }); +}; + +/** Versions ordered newest → oldest for existing versionCompare endpoint callers. */ +export const fetchGitLabMergeRequestVersions = async (args: { + transport: GitLabTransport; + iid: number; + projectPath: string; +}): Promise> => { + const versions = await readGitLabMergeRequestVersions(args); + return versions.toReversed().map((version, index) => ({ + ...version, + label: `v${versions.length - index} · ${version.headSha.slice(0, 7)}`, + })); +}; + +export const fetchGitLabMergeRequestVersionHistory = async ({ + cache, + transport, + iid, + projectPath, +}: { + cache?: VersionStatCache; + transport: GitLabTransport; + iid: number; + projectPath: string; +}): Promise> => { + const versions = await readGitLabMergeRequestVersions({ + transport, + iid, + projectPath, + }); + if (versions.length === 0) { + return []; + } + const stats = await Promise.all( + versions.map(async (version, index) => { + const previous = + versions[index - 1] ?? + ({ + ...version, + headSha: version.baseSha, + id: 'base', + label: 'MR base', + startSha: version.baseSha, + } satisfies MergeRequestVersionRef); + const cached = await cache?.read?.({ next: version, previous }); + if (cached) { + return cached; + } + // GitLab's version endpoint returns the whole MR patch for that version, + // not the delta from the preceding version. Compare the two endpoint + // SHAs so this is genuinely v(N-1) → vN (and base → v1). + const files = await readCompareFiles({ + from: previous.headSha, + transport, + projectPath, + to: version.headSha, + }); + const diffStat = { + additions: files.reduce( + (total, file) => + total + + file.patchBody + .split('\n') + .filter((line) => line.startsWith('+') && !line.startsWith('+++')).length, + 0, + ), + deletions: files.reduce( + (total, file) => + total + + file.patchBody + .split('\n') + .filter((line) => line.startsWith('-') && !line.startsWith('---')).length, + 0, + ), + filesChanged: files.length, + }; + await cache?.write?.({ next: version, previous }, diffStat); + return diffStat; + }), + ); + const first = versions[0]; + return [ + { + createdAt: null, + diffStat: stats[0]!, + headSha: first.baseSha, + id: null, + isHead: false, + number: 0, + }, + ...versions.map((version, index) => ({ + createdAt: version.createdAt, + diffStat: stats[index]!, + headSha: version.headSha, + id: version.id, + isHead: index === versions.length - 1, + number: index + 1, + ...(index > 0 + ? { previousCreatedAt: versions[index - 1]!.createdAt, previousNumber: index } + : {}), + })), + ]; +}; + +const readMergeRequestVersionFiles = async ({ + transport, + iid, + projectPath, + versionId, +}: { + transport: GitLabTransport; + iid: number; + projectPath: string; + versionId: string; +}): Promise> => { + const value = await readJson( + transport, + createRequest(mergeRequestVersionEndpoint(projectPath, iid, versionId)), + 'Unable to load the merge request version.', + ); + const version = asRecord(value); + return asArray(version.diffs ?? version.changes) + .map(normalizeVersionPatchFile) + .filter((file): file is VersionPatchFile => file != null); +}; +const readRepositoryCompare = async ({ + from, + transport, + projectPath, + to, +}: { + from: string; + transport: GitLabTransport; + projectPath: string; + to: string; +}): Promise => + asRecord( + await readJson( + transport, + createRequest(repositoryCompareEndpoint(projectPath, from, to)), + 'Unable to compare GitLab revisions.', + ), + ); + +const readCompareFiles = async (args: { + from: string; + transport: GitLabTransport; + projectPath: string; + to: string; +}): Promise> => { + const value = await readRepositoryCompare(args); + return asArray(value.diffs) + .map(normalizeVersionPatchFile) + .filter((file): file is VersionPatchFile => file != null); +}; + +const getPatchDiffStat = (files: ReadonlyArray) => ({ + additions: files.reduce( + (total, file) => + total + + file.patchBody.split('\n').filter((line) => line.startsWith('+') && !line.startsWith('+++')) + .length, + 0, + ), + deletions: files.reduce( + (total, file) => + total + + file.patchBody.split('\n').filter((line) => line.startsWith('-') && !line.startsWith('---')) + .length, + 0, + ), + filesChanged: files.length, +}); + +const commitGraphReaches = ( + targetSha: string, + ancestorSha: string, + candidates: ReadonlyArray, +) => { + const bySha = new Map(candidates.map((commit) => [commit.sha, commit])); + const visit = (sha: string, visited = new Set()): boolean => { + if (sha === ancestorSha) { + return true; + } + if (visited.has(sha)) { + return false; + } + visited.add(sha); + return (bySha.get(sha)?.parentIds ?? []).some((parent) => visit(parent, visited)); + }; + return visit(targetSha); +}; + +const toBaseMovementCommit = (commit: GitLabMergeRequestCommit) => ({ + authoredAt: commit.authoredDate, + authorName: commit.authorName, + sha: commit.sha, + shortSha: commit.shortSha, + subject: commit.title, + webUrl: commit.webUrl, +}); + +const readBaseMovement = async ({ + fromSha, + transport, + projectPath, + toSha, +}: { + fromSha: string; + transport: GitLabTransport; + projectPath: string; + toSha: string; +}): Promise => { + const baseRef = (sha: string, value?: unknown) => { + const commit = asRecord(value); + return { + committedAt: trimmedString(commit.committed_date) ?? null, + sha, + shortSha: trimmedString(commit.short_id) ?? sha.slice(0, 7), + webUrl: + trimmedString(commit.web_url) ?? + `${gitLabOrigin}/${projectPath}/-/commit/${encodeURIComponent(sha)}`, + }; + }; + if (fromSha === toSha) { + return { + changed: false, + commits: [], + commitsBetween: 0, + commitTimestampDeltaMs: null, + diffStat: { additions: 0, deletions: 0, filesChanged: 0 }, + from: baseRef(fromSha), + relationship: 'forward', + to: baseRef(toSha), + truncated: false, + }; + } + try { + const [fromValue, toValue, compare] = await Promise.all([ + readJson( + transport, + createRequest(repositoryCommitEndpoint(projectPath, fromSha)), + 'Unable to load the old base commit.', + ), + readJson( + transport, + createRequest(repositoryCommitEndpoint(projectPath, toSha)), + 'Unable to load the new base commit.', + ), + readRepositoryCompare({ from: fromSha, transport, projectPath, to: toSha }), + ]); + const from = baseRef(fromSha, fromValue); + const to = baseRef(toSha, toValue); + const commits = asArray(compare.commits) + .map((value) => normalizeMergeRequestCommit(value, projectPath)) + .filter((commit): commit is GitLabMergeRequestCommit => commit != null); + const forwardTruncated = compare.compare_timeout === true || compare.overflow === true; + let relationship: VersionBaseMovement['relationship'] = commitGraphReaches( + toSha, + fromSha, + commits, + ) + ? 'forward' + : 'unknown'; + // Prefer the new-base-facing commit list for UI expansion. For a pure + // backward base move, fall back to the reverse compare list. + let movementCommits = commits.map(toBaseMovementCommit); + let commitsBetween = relationship === 'forward' && !forwardTruncated ? commits.length : null; + let truncated = forwardTruncated; + if (relationship === 'unknown' && !forwardTruncated) { + try { + const reverse = await readRepositoryCompare({ + from: toSha, + transport, + projectPath, + to: fromSha, + }); + const reverseCommits = asArray(reverse.commits) + .map((value) => normalizeMergeRequestCommit(value, projectPath)) + .filter((commit): commit is GitLabMergeRequestCommit => commit != null); + const reverseTruncated = reverse.compare_timeout === true || reverse.overflow === true; + truncated = reverseTruncated; + if (commitGraphReaches(fromSha, toSha, reverseCommits)) { + relationship = 'backward'; + movementCommits = reverseCommits.map(toBaseMovementCommit); + commitsBetween = reverseTruncated ? null : reverseCommits.length; + } else if (!reverseTruncated) { + relationship = 'divergent'; + // From→to still describes the new base tip relative to the old one. + // Count those commits even when the histories diverged, so the UI can + // show base-diff stats and an expandable commit list. + commitsBetween = commits.length; + } + } catch { + relationship = 'unknown'; + } + } else if (relationship === 'unknown' && forwardTruncated && commits.length > 0) { + // GitLab may still return a partial commit list when compare overflows. + commitsBetween = commits.length; + } + const files = asArray(compare.diffs) + .map(normalizeVersionPatchFile) + .filter((file): file is VersionPatchFile => file != null); + const fromTimestamp = from.committedAt ? Date.parse(from.committedAt) : Number.NaN; + const toTimestamp = to.committedAt ? Date.parse(to.committedAt) : Number.NaN; + return { + changed: true, + commits: movementCommits, + commitsBetween, + commitTimestampDeltaMs: + Number.isFinite(fromTimestamp) && Number.isFinite(toTimestamp) + ? toTimestamp - fromTimestamp + : null, + diffStat: getPatchDiffStat(files), + from, + relationship, + to, + truncated, + }; + } catch (error) { + return { + changed: true, + commits: [], + commitsBetween: null, + commitTimestampDeltaMs: null, + diffStat: null, + from: baseRef(fromSha), + relationship: 'unknown', + to: baseRef(toSha), + truncated: false, + warning: error instanceof Error ? error.message : 'Base movement details are unavailable.', + }; + } +}; + +const resolveVersionCompareEndpoint = ({ + comments, + endpoint, + lastReviewed, + versions, +}: { + comments?: ReadonlyArray; + endpoint: VersionCompareEndpoint; + lastReviewed?: MergeRequestVersionRef | null; + versions: ReadonlyArray; +}): MergeRequestVersionRef => { + if (endpoint.kind === 'mr-base') { + const oldest = versions.at(-1); + if (!oldest) { + throw new Error('No merge request base is available.'); + } + return { + ...oldest, + headSha: oldest.baseSha, + id: 'base', + label: 'MR base', + startSha: oldest.baseSha, + }; + } + if (endpoint.kind === 'mr-version') { + const match = versions.find((version) => version.id === endpoint.versionId); + if (!match) { + throw new Error(`Unknown merge request version: ${endpoint.versionId}`); + } + return match; + } + if (endpoint.kind === 'diff-identity') { + return { + baseSha: endpoint.baseSha, + createdAt: new Date(0).toISOString(), + headSha: endpoint.headSha, + id: `identity:${endpoint.headSha}`, + label: endpoint.headSha.slice(0, 7), + startSha: endpoint.startSha, + }; + } + if (endpoint.kind === 'head-sha') { + const match = versions.find((version) => version.headSha === endpoint.headSha); + if (match) { + return match; + } + return { + baseSha: versions[0]?.baseSha ?? endpoint.headSha, + createdAt: new Date(0).toISOString(), + headSha: endpoint.headSha, + id: `head:${endpoint.headSha}`, + label: endpoint.headSha.slice(0, 7), + startSha: versions[0]?.startSha ?? versions[0]?.baseSha ?? endpoint.headSha, + }; + } + if (endpoint.kind === 'last-reviewed') { + if (!lastReviewed) { + throw new Error('No last-reviewed identity is available for this merge request.'); + } + return lastReviewed; + } + if (endpoint.kind === 'comment-position') { + const match = (comments ?? []).find((comment) => comment.commentId === endpoint.commentId); + if (!match) { + throw new Error(`Unknown comment position for version comparison: ${endpoint.commentId}`); + } + const byHead = versions.find((version) => version.headSha === match.position.headSha); + if (byHead) { + return byHead; + } + return { + baseSha: match.position.baseSha, + createdAt: new Date(0).toISOString(), + headSha: match.position.headSha, + id: `comment:${endpoint.commentId}`, + label: `comment ${endpoint.commentId} · ${match.position.headSha.slice(0, 7)}`, + startSha: match.position.startSha, + }; + } + throw new Error('Unsupported version-comparison endpoint.'); +}; + +const collectCommentAnchorsFromDiscussions = ( + discussions: ReadonlyArray, +): Array => + discussions.flatMap((discussionValue) => { + const discussion = asRecord(discussionValue); + return asArray(discussion.notes).flatMap((noteValue) => { + const note = asRecord(noteValue); + if (note.system === true) { + return []; + } + const position = asRecord(note.position ?? note.original_position); + const filePath = asString(position.new_path ?? position.old_path); + const baseSha = asString(position.base_sha); + const startSha = asString(position.start_sha); + const headSha = asString(position.head_sha); + const id = asNumber(note.id); + if (!filePath || !baseSha || !startSha || !headSha || id == null || !Number.isInteger(id)) { + return []; + } + const rawLine = asNumber(position.new_line ?? position.old_line); + const anchor: CommentAnchor = { + commentId: `gitlab:${id}`, + filePath, + position: { baseSha, headSha, startSha }, + }; + if (rawLine != null && Number.isInteger(rawLine) && rawLine > 0) { + anchor.lineNumber = rawLine; + } + return [anchor]; + }); + }); + +export const fetchGitLabMergeRequestVersionCompare = async ({ + comments = [], + from: fromEndpoint, + transport, + iid, + lastReviewed = null, + paths, + projectPath: rawProjectPath, + readCached, + to: toEndpoint, + writeCached, +}: { + comments?: ReadonlyArray; + from: VersionCompareEndpoint; + transport: GitLabTransport; + iid: number; + lastReviewed?: MergeRequestVersionRef | null; + paths?: ReadonlyArray; + projectPath: string; + readCached?: (range: { + from: MergeRequestVersionRef; + to: MergeRequestVersionRef; + }) => Promise | MergeRequestVersionCompare | null; + to: VersionCompareEndpoint; + writeCached?: (versionCompare: MergeRequestVersionCompare) => Promise | void; +}): Promise => { + const projectPath = validateProjectPath(rawProjectPath); + if (!Number.isInteger(iid) || iid <= 0) { + throw new Error('Invalid GitLab merge request IID.'); + } + const needsCommentAnchors = + comments.length === 0 || + fromEndpoint.kind === 'comment-position' || + toEndpoint.kind === 'comment-position'; + const [versions, discussionAnchors] = await Promise.all([ + fetchGitLabMergeRequestVersions({ + transport, + iid, + projectPath, + }), + needsCommentAnchors + ? readPages( + transport, + mergeRequestEndpoint(projectPath, iid, '/discussions'), + ).then(collectCommentAnchorsFromDiscussions) + : Promise.resolve([] as Array), + ]); + if (versions.length === 0) { + throw new Error('GitLab did not return merge request versions for version comparison.'); + } + const resolvedComments = comments.length > 0 ? comments : discussionAnchors; + const from = resolveVersionCompareEndpoint({ + comments: resolvedComments, + endpoint: fromEndpoint, + lastReviewed, + versions, + }); + const to = resolveVersionCompareEndpoint({ + comments: resolvedComments, + endpoint: toEndpoint, + lastReviewed, + versions, + }); + if (from.headSha === to.headSha && from.baseSha === to.baseSha) { + throw new Error('Version comparison requires distinct from and to endpoints.'); + } + if (readCached) { + const cached = await readCached({ from, to }); + if (cached) { + return cached; + } + } + + const baseMovementPromise = readBaseMovement({ + fromSha: from.baseSha, + transport, + projectPath, + toSha: to.baseSha, + }); + + const loadVersionFiles = async (version: MergeRequestVersionRef) => { + const listed = versions.find((candidate) => candidate.id === version.id); + if (listed) { + try { + return await readMergeRequestVersionFiles({ + transport, + iid, + projectPath, + versionId: version.id, + }); + } catch { + // Fall through to repository compare. + } + } + return readCompareFiles({ + from: version.baseSha, + transport, + projectPath, + to: version.headSha, + }); + }; + + const [fromFiles, toFiles] = await Promise.all([loadVersionFiles(from), loadVersionFiles(to)]); + const blobCache = new Map(); + const readBlob = async (filePath: string, ref: string) => { + const key = `${ref}:${filePath}`; + if (blobCache.has(key)) { + return blobCache.get(key) ?? null; + } + try { + const content = await readText( + transport, + createRequest(repositoryFileRawEndpoint(projectPath, filePath, ref), { + headers: { accept: 'text/plain' }, + }), + ); + blobCache.set(key, content); + return content; + } catch { + blobCache.set(key, null); + return null; + } + }; + const computedVersionCompare = await computeVersionComparePreferringReplay({ + comments: resolvedComments, + from, + fromFiles, + paths, + readBlob, + to, + toFiles, + }); + const versionCompare: MergeRequestVersionCompare = { + ...computedVersionCompare, + baseMovement: await baseMovementPromise, + }; + if (writeCached) { + try { + await writeCached(versionCompare); + } catch { + // Cache writes are best-effort. + } + } + return versionCompare; +}; + +const resolveVersionCommitRange = async ({ + from, + transport, + iid, + projectPath, + to, +}: { + from: VersionCompareEndpoint; + transport: GitLabTransport; + iid: number; + projectPath: string; + to: VersionCompareEndpoint; +}) => { + const versions = + from.kind === 'diff-identity' && to.kind === 'diff-identity' + ? [] + : await fetchGitLabMergeRequestVersions({ + transport, + iid, + projectPath, + }); + if (versions.length === 0 && (from.kind !== 'diff-identity' || to.kind !== 'diff-identity')) { + throw new Error('GitLab did not return merge request versions.'); + } + return { + from: resolveVersionCompareEndpoint({ endpoint: from, versions }), + to: resolveVersionCompareEndpoint({ endpoint: to, versions }), + }; +}; + +export const fetchGitLabHistoricalCommitStack = async ({ + baseSha, + transport, + headSha, + projectPath: rawProjectPath, +}: { + baseSha: string; + transport: GitLabTransport; + headSha: string; + projectPath: string; +}): Promise> => { + if (baseSha === headSha) { + return []; + } + const projectPath = validateProjectPath(rawProjectPath); + const compare = await readRepositoryCompare({ + from: baseSha, + transport, + projectPath, + to: headSha, + }); + if (compare.compare_timeout === true || compare.overflow === true) { + throw new Error('GitLab truncated the historical commit stack.'); + } + const commits = asArray(compare.commits) + .map((value) => normalizeMergeRequestCommit(value, projectPath)) + .filter( + (commit): commit is GitLabMergeRequestCommit => commit != null && commit.sha !== baseSha, + ); + return [...orderCommitsTopologically(commits)]; +}; + +const readCommitPatchFiles = async ({ + transport, + projectPath, + sha, +}: { + transport: GitLabTransport; + projectPath: string; + sha: string; +}) => { + const values = await readPages( + transport, + repositoryCommitDiffEndpoint(projectPath, sha), + ); + return values + .map(normalizeVersionPatchFile) + .filter((file): file is VersionPatchFile => file != null); +}; + +const scopeVersionCommitFiles = ( + files: ReadonlyArray, + unitId: string, +): Array => + files.map((file, fileIndex) => ({ + ...file, + fingerprint: `${unitId}:${fileIndex}:${file.fingerprint}`, + sections: file.sections.map((section, sectionIndex) => ({ + ...section, + id: `${file.path}:version-commit:${unitId}:${sectionIndex}`, + })), + })); + +type VersionCommitSignatureCache = { + read?: (sha: string) => Promise | CommitPatchSignature | null; + write?: (signature: CommitPatchSignature) => Promise | void; +}; + +export const fetchGitLabMergeRequestVersionCommitEvolution = async ({ + cache, + from: fromEndpoint, + transport, + iid, + projectPath: rawProjectPath, + to: toEndpoint, +}: { + cache?: VersionCommitSignatureCache; + from: VersionCompareEndpoint; + transport: GitLabTransport; + iid: number; + projectPath: string; + to: VersionCompareEndpoint; +}): Promise => { + const projectPath = validateProjectPath(rawProjectPath); + const range = await resolveVersionCommitRange({ + from: fromEndpoint, + transport, + iid, + projectPath, + to: toEndpoint, + }); + const warnings: Array = []; + const [oldStackResult, newStackResult, baseStackResult] = await Promise.allSettled([ + fetchGitLabHistoricalCommitStack({ + baseSha: range.from.baseSha, + transport, + headSha: range.from.headSha, + projectPath, + }), + fetchGitLabHistoricalCommitStack({ + baseSha: range.to.baseSha, + transport, + headSha: range.to.headSha, + projectPath, + }), + range.from.baseSha === range.to.baseSha + ? Promise.resolve([]) + : fetchGitLabHistoricalCommitStack({ + baseSha: range.from.baseSha, + transport, + headSha: range.to.baseSha, + projectPath, + }), + ]); + const stackCompleteness = { + new: newStackResult.status === 'fulfilled', + old: oldStackResult.status === 'fulfilled', + }; + let baseStackComplete = baseStackResult.status === 'fulfilled'; + let oldCommits = oldStackResult.status === 'fulfilled' ? oldStackResult.value : []; + let newCommits = newStackResult.status === 'fulfilled' ? newStackResult.value : []; + let baseCommits = baseStackResult.status === 'fulfilled' ? baseStackResult.value : []; + if (!stackCompleteness.old) { + warnings.push( + 'The earlier commit stack is unavailable. Visible commits remain unclassified rather than being called new.', + ); + } + if (!stackCompleteness.new) { + warnings.push( + 'The later commit stack is unavailable. Visible commits remain unclassified rather than being called removed.', + ); + } + if (baseStackResult.status !== 'fulfilled') { + warnings.push( + 'Target-base movement could not be analyzed. Earlier commits that moved into the base remain unclassified.', + ); + } + if (oldCommits.length > versionCommitStackLimit) { + oldCommits = oldCommits.slice(-versionCommitStackLimit); + stackCompleteness.old = false; + warnings.push( + `Only the latest ${versionCommitStackLimit} commits from the earlier version were analyzed; unmatched commits remain unclassified.`, + ); + } + if (newCommits.length > versionCommitStackLimit) { + newCommits = newCommits.slice(-versionCommitStackLimit); + stackCompleteness.new = false; + warnings.push( + `Only the latest ${versionCommitStackLimit} commits from the later version were analyzed; unmatched commits remain unclassified.`, + ); + } + if (baseCommits.length > versionCommitStackLimit) { + baseCommits = baseCommits.slice(-versionCommitStackLimit); + baseStackComplete = false; + warnings.push( + `Only the latest ${versionCommitStackLimit} target-base commits were analyzed; earlier commits that moved into the base may remain unclassified.`, + ); + } + const sameShas = new Set( + oldCommits + .map((commit) => commit.sha) + .filter( + (sha) => + newCommits.some((commit) => commit.sha === sha) || + baseCommits.some((commit) => commit.sha === sha), + ), + ); + const commitsNeedingSignatures = [...oldCommits, ...newCommits, ...baseCommits].filter( + (commit, index, commits) => + !sameShas.has(commit.sha) && + commits.findIndex((candidate) => candidate.sha === commit.sha) === index, + ); + const signatures = new Map(); + const baseCommitShas = new Set(baseCommits.map((commit) => commit.sha)); + let failedBaseSignatureCount = 0; + let failedSignatureCount = 0; + for ( + let index = 0; + index < commitsNeedingSignatures.length; + index += versionCommitDiffConcurrency + ) { + const batch = commitsNeedingSignatures.slice(index, index + versionCommitDiffConcurrency); + await Promise.all( + batch.map(async (commit) => { + try { + const cached = await cache?.read?.(commit.sha); + if (cached) { + signatures.set(commit.sha, cached); + return; + } + } catch { + // A missing/stale cache must not prevent direct analysis. + } + try { + const files = await fetchGitLabCommitDiff({ + transport, + projectPath, + sha: commit.sha, + }); + const signature = await createCommitPatchSignature(commit, files); + signatures.set(commit.sha, signature); + try { + await cache?.write?.(signature); + } catch { + // Immutable signature cache writes are best-effort. + } + } catch { + if (baseCommitShas.has(commit.sha)) { + failedBaseSignatureCount += 1; + } else { + failedSignatureCount += 1; + } + } + }), + ); + } + if (failedSignatureCount > 0) { + warnings.push( + `Patch details were unavailable for ${failedSignatureCount} ${failedSignatureCount === 1 ? 'commit' : 'commits'}; they remain unclassified rather than being called new or removed.`, + ); + stackCompleteness.old = false; + stackCompleteness.new = false; + } + if (failedBaseSignatureCount > 0) { + warnings.push( + `Patch details were unavailable for ${failedBaseSignatureCount} target-base ${failedBaseSignatureCount === 1 ? 'commit' : 'commits'}; earlier MR commits are only marked as removed when base evidence is complete.`, + ); + baseStackComplete = false; + } + return matchVersionCommitStacks({ + baseCommits, + baseStackComplete, + from: range.from, + newCommits, + oldCommits, + signatures, + stackCompleteness, + to: range.to, + warnings, + }); +}; + +export const attributeVersionCommitRebaseDrivers = async ({ + baseCommits, + transport, + projectPath, + unit, + unitFiles, +}: { + baseCommits: ReadonlyArray<{ + authoredAt: string; + authorName: string; + sha: string; + shortSha: string; + subject: string; + webUrl: string; + }>; + transport: GitLabTransport; + projectPath: string; + unit: VersionCommitEvolutionUnit; + unitFiles: ReadonlyArray; +}): Promise> => { + if (unit.kind !== 'likely-revised' || baseCommits.length === 0 || unitFiles.length === 0) { + return unit.rebaseDrivers ?? []; + } + const unitSignature = await createCommitPatchSignature( + { + sha: unit.after?.sha ?? unit.before?.sha ?? unit.id, + title: unit.after?.subject ?? unit.before?.subject ?? unit.id, + }, + unitFiles, + ); + const signatures = new Map(); + // Cap base-commit signature work; large base moves are common on revived MRs. + const candidates = baseCommits.slice(0, 40); + for (let index = 0; index < candidates.length; index += versionCommitDiffConcurrency) { + const batch = candidates.slice(index, index + versionCommitDiffConcurrency); + await Promise.all( + batch.map(async (commit) => { + try { + const files = await fetchGitLabCommitDiff({ + transport, + projectPath, + sha: commit.sha, + }); + signatures.set( + commit.sha, + await createCommitPatchSignature({ sha: commit.sha, title: commit.subject }, files), + ); + } catch { + // Skip unreadable base commits rather than failing the unit walkthrough. + } + }), + ); + } + return attributeRebaseDrivers({ + baseCommits: candidates, + baseSignatures: signatures, + unitSignature, + }); +}; + +export const fetchGitLabVersionCommitUnitDiff = async ({ + transport, + projectPath: rawProjectPath, + unit, +}: { + transport: GitLabTransport; + projectPath: string; + unit: VersionCommitEvolutionUnit; +}): Promise> => { + const projectPath = validateProjectPath(rawProjectPath); + if (!unit.reviewable) { + throw new Error('This commit evolution unit is not reviewable.'); + } + if (unit.kind === 'added' && unit.after) { + return scopeVersionCommitFiles( + await fetchGitLabCommitDiff({ transport, projectPath, sha: unit.after.sha }), + unit.id, + ); + } + if (unit.kind === 'removed' && unit.before) { + const parent = unit.before.parentIds[0]; + if (!parent) { + throw new Error('The removed commit parent is unavailable.'); + } + const compare = await readRepositoryCompare({ + from: unit.before.sha, + transport, + projectPath, + to: parent, + }); + return scopeVersionCommitFiles( + asArray(compare.diffs) + .map((diff, index) => normalizeDiff(diff, 0, parent, index, 'commit', unit.id)) + .filter((file): file is ChangedFile => file != null), + unit.id, + ); + } + if (unit.kind === 'likely-revised' && unit.before && unit.after) { + const oldParent = unit.before.parentIds[0]; + const newParent = unit.after.parentIds[0]; + if (!oldParent || !newParent) { + throw new Error('A revised commit parent is unavailable.'); + } + const [fromFiles, toFiles] = await Promise.all([ + readCommitPatchFiles({ transport, projectPath, sha: unit.before.sha }), + readCommitPatchFiles({ transport, projectPath, sha: unit.after.sha }), + ]); + const blobs = new Map(); + const readBlob = async (filePath: string, ref: string) => { + const key = `${ref}:${filePath}`; + if (blobs.has(key)) { + return blobs.get(key) ?? null; + } + try { + const content = await readText( + transport, + createRequest(repositoryFileRawEndpoint(projectPath, filePath, ref), { + headers: { accept: 'text/plain' }, + }), + ); + blobs.set(key, content); + return content; + } catch { + blobs.set(key, null); + return null; + } + }; + const comparison = await computeVersionComparePreferringReplay({ + from: { + baseSha: oldParent, + createdAt: unit.before.authoredAt, + headSha: unit.before.sha, + id: unit.before.sha, + label: unit.before.shortSha, + startSha: oldParent, + }, + fromFiles, + readBlob, + to: { + baseSha: newParent, + createdAt: unit.after.authoredAt, + headSha: unit.after.sha, + id: unit.after.sha, + label: unit.after.shortSha, + startSha: newParent, + }, + toFiles, + }); + return scopeVersionCommitFiles( + comparison.files.map((file) => file.file), + unit.id, + ); + } + throw new Error('Unsupported commit evolution unit.'); +}; + +export const projectMergeRequestVersionRef = ( + version: MergeRequestVersionRef & { number?: number; createdAt?: string; diffStat?: ReviewVersionOption['diffStat']; isHead?: boolean; previousCreatedAt?: string; previousNumber?: number }, +): ReviewVersionOption => + reviewVersionOption({ + createdAt: version.createdAt, + id: version.id, + range: diffRange( + revisionRef(version.baseSha, commitRevisionLabel(version.baseSha.slice(0, 7))), + revisionRef( + version.headSha, + versionRevisionLabel(version.label, undefined), + ), + ), + ...(version.diffStat ? { diffStat: version.diffStat } : {}), + ...(version.isHead != null ? { isHead: version.isHead } : {}), + ...(version.number != null ? { number: version.number } : {}), + ...(version.previousCreatedAt ? { previousCreatedAt: version.previousCreatedAt } : {}), + ...(version.previousNumber != null ? { previousNumber: version.previousNumber } : {}), + }); + +const projectCommitSummary = (commit: Algorithmish | undefined): ReviewCommitSummary | undefined => { + if (!commit) return undefined; + return { + authorName: commit.authorName, + authoredAt: commit.authoredAt, + parentIds: commit.parentIds ?? [], + sha: commit.sha, + shortSha: commit.shortSha, + subject: commit.subject, + webUrl: commit.webUrl, + ...(commit.diffStat ? { diffStat: commit.diffStat } : {}), + }; +}; + +type Algorithmish = { + authorName: string; + authoredAt: string; + diffStat?: { additions: number; deletions: number; filesChanged: number }; + parentIds?: ReadonlyArray; + sha: string; + shortSha: string; + subject: string; + webUrl?: string; +}; + +export const projectEvolutionUnit = (unit: { + after?: Algorithmish; + baseCommit?: Algorithmish; + before?: Algorithmish; + confidence: 'exact' | 'high' | 'unmatched'; + id: string; + kind: string; + matchReasons?: ReadonlyArray; + matchScore?: number; + order: number; + rebaseDrivers?: ReadonlyArray<{ + authorName: string; + authoredAt: string; + overlappingPaths: ReadonlyArray; + sha: string; + shortSha: string; + subject: string; + webUrl?: string; + }>; + reviewable: boolean; +}): ReviewEvolutionUnit => { + const common = { + confidence: unit.confidence, + id: unit.id, + order: unit.order, + ...(unit.matchReasons ? { matchReasons: unit.matchReasons } : {}), + ...(unit.matchScore != null ? { matchScore: unit.matchScore } : {}), + } as const; + if (unit.kind === 'added' || unit.kind === 'introduced') { + return { + ...common, + after: projectCommitSummary(unit.after)!, + kind: 'introduced', + reviewable: true, + }; + } + if (unit.kind === 'removed') { + return { + ...common, + before: projectCommitSummary(unit.before)!, + kind: 'removed', + reviewable: true, + }; + } + if (unit.kind === 'likely-revised' || unit.kind === 'revised') { + return { + ...common, + after: projectCommitSummary(unit.after)!, + before: projectCommitSummary(unit.before)!, + kind: 'revised', + reviewable: true, + ...(unit.rebaseDrivers + ? { + rebaseDrivers: unit.rebaseDrivers.map((driver) => ({ + authorName: driver.authorName, + authoredAt: driver.authoredAt, + overlappingPaths: driver.overlappingPaths, + sha: driver.sha, + shortSha: driver.shortSha, + subject: driver.subject, + webUrl: driver.webUrl, + })), + } + : {}), + }; + } + if (unit.kind === 'ambiguous') { + return { + ...common, + kind: 'ambiguous', + reviewable: true, + ...(projectCommitSummary(unit.after) ? { after: projectCommitSummary(unit.after) } : {}), + ...(projectCommitSummary(unit.before) ? { before: projectCommitSummary(unit.before) } : {}), + }; + } + if (unit.kind === 'absorbed-into-base') { + return { + ...common, + kind: 'absorbed-into-base', + reviewable: false, + ...(projectCommitSummary(unit.after) ? { after: projectCommitSummary(unit.after) } : {}), + ...(projectCommitSummary(unit.baseCommit) ? { baseCommit: projectCommitSummary(unit.baseCommit) } : {}), + ...(projectCommitSummary(unit.before) ? { before: projectCommitSummary(unit.before) } : {}), + }; + } + // retained / rewritten-same-patch + return { + ...common, + kind: unit.kind === 'rewritten-same-patch' ? 'rewritten-same-patch' : 'retained', + reviewable: false, + ...(projectCommitSummary(unit.after) ? { after: projectCommitSummary(unit.after) } : {}), + ...(projectCommitSummary(unit.before) ? { before: projectCommitSummary(unit.before) } : {}), + }; +}; + +export const projectCommitEvolution = ( + evolution: MergeRequestVersionCommitEvolution, +): ReviewCommitEvolution => ({ + recommendation: { + rationale: evolution.recommendation.reason, + suggestedStructure: evolution.recommendation.structure, + }, + summary: evolution.summary, + units: evolution.units.map(projectEvolutionUnit), + ...(evolution.warnings ? { warnings: evolution.warnings } : {}), +}); + +export const projectVersionCompare = ( + compare: MergeRequestVersionCompare, + files: ReadonlyArray = compare.files.map((file) => file.file), +): DiffComparisonView => { + const from = projectMergeRequestVersionRef(compare.range.from); + const to = projectMergeRequestVersionRef(compare.range.to); + const analysis: DiffComparisonAnalysis = { + summary: compare.summary, + ...(compare.baseMovement ? { baseMovement: compare.baseMovement } : {}), + ...(compare.commentAssociations + ? { commentAssociations: compare.commentAssociations } + : {}), + ...(compare.warnings ? { warnings: compare.warnings } : {}), + }; + return diffComparisonView({ + analysis, + comparison: diffComparison(from.range, to.range), + files, + from, + to, + }); +}; + +export const projectReviewPlan = ({ + evolution, + structure, + versionCompare, +}: { + evolution?: MergeRequestVersionCommitEvolution | ReviewCommitEvolution | null; + structure?: 'auto' | 'commit-by-commit' | 'whole-diff' | 'units'; + versionCompare?: MergeRequestVersionCompare | DiffComparisonView | null; +}): ReviewPlan => { + const projectedEvolution = + evolution && 'recommendation' in evolution && 'reason' in (evolution as MergeRequestVersionCommitEvolution).recommendation + ? projectCommitEvolution(evolution as MergeRequestVersionCommitEvolution) + : (evolution as ReviewCommitEvolution | undefined); + const view = + versionCompare && 'range' in versionCompare + ? projectVersionCompare(versionCompare) + : (versionCompare as DiffComparisonView | undefined); + const analysis = view + ? { + ...view.analysis, + ...(projectedEvolution ? { commitEvolution: projectedEvolution } : {}), + } + : projectedEvolution + ? { + commitEvolution: projectedEvolution, + summary: { + addedLines: 0, + baseMoved: false, + commentsAffected: 0, + conflictFiles: 0, + deletedLines: 0, + empty: false, + filesChanged: 0, + intentionalFiles: 0, + noiseFiles: 0, + }, + } + : undefined; + return resolveReviewPlan({ + analysis, + comparison: view?.comparison, + recommendation: projectedEvolution?.recommendation, + structure, + units: projectedEvolution?.units, + }); +}; + +export const toGitLabDiffIdentity = (version: MergeRequestVersionRef): GitLabDiffIdentity => ({ + baseSha: version.baseSha, + headSha: version.headSha, + startSha: version.startSha, +}); diff --git a/gitlab/src/index.ts b/gitlab/src/index.ts new file mode 100644 index 00000000..1632c45a --- /dev/null +++ b/gitlab/src/index.ts @@ -0,0 +1,5 @@ +export * from './history.ts'; +export * from './review-strategy.ts'; +export * from './transport.ts'; +export * from './version-commit-evolution.ts'; +export * from './version-compare.ts'; diff --git a/gitlab/src/review-strategy.ts b/gitlab/src/review-strategy.ts new file mode 100644 index 00000000..5818aa1c --- /dev/null +++ b/gitlab/src/review-strategy.ts @@ -0,0 +1,335 @@ +// Review-structure classifier for GitLab MRs. +// Entry: classifyMergeRequestReviewStrategy() — used by merge-request snapshot load. +// Consumers: walkthrough prompt guidance, walkthrough cache identity (reviewStructure), +// and the client commits/strategy summary. See PLAN.md §G1. + +export type ClassifiedCommitRole = + | 'chore' + | 'docs' + | 'feature' + | 'fixup' + | 'merge' + | 'refactor' + | 'revert' + | 'review-response' + | 'test' + | 'unknown'; + +export type ClassifiedCommit = { + authoredAt: string; + authorName: string; + body: string; + isMerge: boolean; + parents: ReadonlyArray; + role: ClassifiedCommitRole; + sha: string; + shortSha: string; + subject: string; + webUrl?: string; +}; + +export type MergeRequestReviewStrategy = + | { + commits: ReadonlyArray; + confidence: number; + mode: 'commit-by-commit'; + reason: 'chapter-shaped' | 'explicit-description' | 'stacked-subjects' | 'user-override'; + } + | { + confidence: number; + mode: 'whole-mr'; + reason: + | 'default' + | 'explicit-whole' + | 'fixup-style' + | 'review-response-style' + | 'single-commit' + | 'too-many-commits' + | 'user-override'; + }; + +export type GitLabMergeRequestCommitLike = { + authoredDate: string; + authorName: string; + message: string; + parentIds: ReadonlyArray; + sha: string; + shortSha: string; + title: string; + webUrl?: string; +}; + +const explicitCommitByCommitPattern = + /\b(?:review\s+commit(?:s)?(?:\s+by\s+commit|[- ]by[- ]commit)|please\s+review\s+each\s+commit|commit[- ]wise\s+review|stacked\s+diff|stacked\s+commits)\b/i; + +const explicitWholePattern = + /\b(?:review\s+as\s+a\s+whole|ignore\s+commits|squash\s+on\s+merge)\b/i; + +const fixupSubjectPattern = + /^(?:fixup!|squash!|amend!|wip\b|tmp\b|try\b|rework\b|review\s+feedback|pr\s+feedback|mr\s+feedback|nits?\b|typo\b|oops\b|cleanup\s+after|follow[- ]?up\b)/i; + +const reviewResponseSubjectPattern = + /^(?:address(?:ing)?\s+(?:review|comments?|feedback)|respond(?:ing)?\s+to\s+(?:review|comments?|feedback)|review\s+comments?)\b/i; + +const conventionalCommitPattern = + /^(?:feat|fix|refactor|test|docs|chore|perf|build|ci|style|revert)(?:\(.+\))?!?:/i; + +const numberedStepPattern = /^(?:step\s+)?\d+[.):\-\s]/i; + +const shortShaPattern = /\b[0-9a-f]{7,40}\b/gi; + +const classifyCommitRole = ( + subject: string, + parentIds: ReadonlyArray, +): ClassifiedCommitRole => { + if (parentIds.length > 1 || /^merge\b/i.test(subject)) { + return 'merge'; + } + if (fixupSubjectPattern.test(subject)) { + return 'fixup'; + } + if (reviewResponseSubjectPattern.test(subject)) { + return 'review-response'; + } + if (/^revert\b/i.test(subject) || subject.startsWith('Revert "')) { + return 'revert'; + } + if (/^(?:test|tests?)(?:\b|[:(\s])/i.test(subject) || /^test(?:\(.+\))?!?:/i.test(subject)) { + return 'test'; + } + if (/^(?:docs?)(?:\b|[:(\s])/i.test(subject) || /^docs?(?:\(.+\))?!?:/i.test(subject)) { + return 'docs'; + } + if (/^refactor(?:\b|[:(\s])/i.test(subject) || /^refactor(?:\(.+\))?!?:/i.test(subject)) { + return 'refactor'; + } + if (/^(?:chore|build|ci)(?:\b|[:(\s])/i.test(subject)) { + return 'chore'; + } + if ( + /^(?:feat|feature|add|implement)(?:\b|[:(\s])/i.test(subject) || + /^feat(?:\(.+\))?!?:/i.test(subject) + ) { + return 'feature'; + } + return 'unknown'; +}; + +const splitMessage = (message: string, title: string) => { + const normalized = message.trim() || title.trim(); + const [subjectLine = title.trim() || 'Commit', ...rest] = normalized.split('\n'); + return { + body: rest.join('\n').trim(), + subject: subjectLine.trim() || title.trim() || 'Commit', + }; +}; + +export const classifyGitLabCommit = (commit: GitLabMergeRequestCommitLike): ClassifiedCommit => { + const { body, subject } = splitMessage(commit.message, commit.title); + const role = classifyCommitRole(subject, commit.parentIds); + return { + authoredAt: commit.authoredDate, + authorName: commit.authorName, + body, + isMerge: role === 'merge' || commit.parentIds.length > 1, + parents: commit.parentIds, + role, + sha: commit.sha, + shortSha: commit.shortSha || commit.sha.slice(0, 8), + subject, + ...(commit.webUrl ? { webUrl: commit.webUrl } : {}), + }; +}; + +/** + * Orders the commits in an MR from its base toward its head. GitLab's commits + * endpoint does not promise the direction we need for a reviewer walkthrough, + * so never use its response order as chronology. + */ +export const orderCommitsTopologically = < + Commit extends { + parentIds?: ReadonlyArray; + parents?: ReadonlyArray; + sha: string; + }, +>( + commits: ReadonlyArray, +): ReadonlyArray => { + const bySha = new Map(commits.map((commit) => [commit.sha, commit])); + const inputIndex = new Map(commits.map((commit, index) => [commit.sha, index])); + const children = new Map>(); + const remainingParents = new Map(); + for (const commit of commits) { + const parents = (commit.parents ?? commit.parentIds ?? []).filter((parent) => + bySha.has(parent), + ); + remainingParents.set(commit.sha, parents.length); + for (const parent of parents) { + const siblings = children.get(parent) ?? []; + siblings.push(commit.sha); + children.set(parent, siblings); + } + } + const ready = commits.filter((commit) => remainingParents.get(commit.sha) === 0); + const result: Array = []; + while (ready.length > 0) { + ready.sort((first, second) => inputIndex.get(first.sha)! - inputIndex.get(second.sha)!); + const commit = ready.shift()!; + result.push(commit); + for (const childSha of children.get(commit.sha) ?? []) { + const remaining = (remainingParents.get(childSha) ?? 1) - 1; + remainingParents.set(childSha, remaining); + if (remaining === 0) { + ready.push(bySha.get(childSha)!); + } + } + } + // A malformed/cyclic response should remain reviewable rather than dropping commits. + return result.length === commits.length + ? result + : [...result, ...commits.filter((commit) => !result.some((entry) => entry.sha === commit.sha))]; +}; + +const descriptionListsCommits = (description: string, commits: ReadonlyArray) => { + if (!description.trim() || commits.length < 2) { + return false; + } + const shortShas = new Set(commits.map((commit) => commit.shortSha.toLowerCase())); + const fullShas = new Set(commits.map((commit) => commit.sha.toLowerCase())); + const matches = description.toLowerCase().match(shortShaPattern) ?? []; + const matched = new Set( + matches.filter( + (value) => + shortShas.has(value) || + fullShas.has(value) || + [...fullShas].some((sha) => sha.startsWith(value)), + ), + ); + if (matched.size >= 2) { + return true; + } + const numberedLines = description + .split('\n') + .map((line) => line.trim()) + .filter((line) => numberedStepPattern.test(line)); + if (numberedLines.length < 2) { + return false; + } + const subjectHits = numberedLines.filter((line) => + commits.some((commit) => + line.toLowerCase().includes(commit.subject.toLowerCase().slice(0, 24)), + ), + ).length; + return subjectHits >= 2; +}; + +const looksChapterShaped = (commits: ReadonlyArray) => { + if (commits.length < 2) { + return false; + } + const intentional = commits.filter( + (commit) => + commit.role === 'feature' || + commit.role === 'refactor' || + commit.role === 'test' || + commit.role === 'docs' || + conventionalCommitPattern.test(commit.subject) || + numberedStepPattern.test(commit.subject), + ); + const uniqueSubjects = new Set(commits.map((commit) => commit.subject.toLowerCase())); + // GitLab commit subjects are often descriptive without using Conventional + // Commits. Treat a short, distinct, non-fixup history as chapter-shaped too. + return ( + uniqueSubjects.size >= 2 && + (intentional.length >= Math.ceil(commits.length * 0.5) || + uniqueSubjects.size === commits.length) + ); +}; + +export const classifyMergeRequestReviewStrategy = (input: { + commits: ReadonlyArray; + description?: string; + title?: string; +}): MergeRequestReviewStrategy => { + const classified = input.commits.map(classifyGitLabCommit); + const nonMerge = classified.filter((commit) => !commit.isMerge); + const text = `${input.title ?? ''}\n${input.description ?? ''}`; + const fixupDensity = + nonMerge.filter((commit) => commit.role === 'fixup' || commit.role === 'review-response') + .length / Math.max(nonMerge.length, 1); + + if (explicitWholePattern.test(text) && !explicitCommitByCommitPattern.test(text)) { + return { confidence: 0.95, mode: 'whole-mr', reason: 'explicit-whole' }; + } + if (explicitCommitByCommitPattern.test(text) || descriptionListsCommits(text, nonMerge)) { + return { + commits: nonMerge, + confidence: 0.95, + mode: 'commit-by-commit', + reason: 'explicit-description', + }; + } + if (nonMerge.length <= 1) { + return { confidence: 0.99, mode: 'whole-mr', reason: 'single-commit' }; + } + if (fixupDensity >= 0.4) { + const reviewResponseOnly = + nonMerge.filter((commit) => commit.role === 'review-response').length / + Math.max(nonMerge.length, 1) >= + 0.4; + return { + confidence: 0.85, + mode: 'whole-mr', + reason: reviewResponseOnly ? 'review-response-style' : 'fixup-style', + }; + } + if (nonMerge.length > 20) { + return { confidence: 0.7, mode: 'whole-mr', reason: 'too-many-commits' }; + } + if (looksChapterShaped(nonMerge)) { + return { + commits: nonMerge, + confidence: 0.75, + mode: 'commit-by-commit', + reason: conventionalCommitPattern.test(nonMerge[0]?.subject ?? '') + ? 'stacked-subjects' + : 'chapter-shaped', + }; + } + return { confidence: 0.55, mode: 'whole-mr', reason: 'default' }; +}; + +export const reviewStructureFromStrategy = ( + strategy: MergeRequestReviewStrategy, +): 'commit-by-commit' | 'whole-mr' => strategy.mode; + +export const overrideMergeRequestReviewStrategy = ( + strategy: MergeRequestReviewStrategy, + mode: 'commit-by-commit' | 'whole-mr', + sourceCommits: ReadonlyArray = [], +): MergeRequestReviewStrategy => { + if (mode === 'whole-mr') { + return { + confidence: 1, + mode: 'whole-mr', + reason: 'user-override', + }; + } + const commits = + strategy.mode === 'commit-by-commit' + ? strategy.commits + : sourceCommits.map(classifyGitLabCommit).filter((commit) => !commit.isMerge); + return { + commits, + confidence: 1, + mode: 'commit-by-commit', + reason: 'user-override', + }; +}; + +/** Cache identity segment for a version-comparison walkthrough. */ +export const versionCompareReviewStructureKey = ( + fromId: string, + toId: string, + structure: 'commit-by-commit' | 'whole-diff' = 'whole-diff', +) => `version-compare:${fromId}:${toId}:${structure}`; diff --git a/gitlab/src/transport.ts b/gitlab/src/transport.ts new file mode 100644 index 00000000..02b9341b --- /dev/null +++ b/gitlab/src/transport.ts @@ -0,0 +1,139 @@ +/** + * Host-injected GitLab transport. + * + * The host authenticates and executes HTTP. This package owns endpoint + * construction, pagination policy, and response parsing. + */ +export type GitLabTransport = { + request(request: { + method?: 'GET' | 'POST' | 'PUT' | 'DELETE'; + path: string; + query?: Readonly>; + body?: unknown; + }): Promise; + /** + * Optional paginated reader. When omitted, {@link request} is called with + * `page` / `per_page` until a short page is returned. + */ + requestPages?(request: { + path: string; + query?: Readonly>; + }): Promise>; + /** Optional raw text reader for repository file blobs. */ + requestText?(request: { + path: string; + query?: Readonly>; + }): Promise; +}; + +export type FakeGitLabTransportRoute = { + body?: unknown; + method?: 'GET' | 'POST' | 'PUT' | 'DELETE'; + path: string; + query?: Readonly>; + response: unknown | ((request: { + method: string; + path: string; + query?: Readonly>; + }) => unknown | Promise); + text?: string | ((request: { + method: string; + path: string; + query?: Readonly>; + }) => string | Promise); +}; + +const queryKey = (query?: Readonly>) => + query + ? Object.entries(query) + .toSorted(([left], [right]) => left.localeCompare(right)) + .map(([key, value]) => `${key}=${String(value)}`) + .join('&') + : ''; + +/** + * Deterministic transport for package tests. + */ +export const createFakeGitLabTransport = ( + routes: ReadonlyArray, +): GitLabTransport & { calls: Array<{ method: string; path: string; query?: Record }> } => { + const calls: Array<{ + method: string; + path: string; + query?: Record; + }> = []; + + const matchRoute = (method: string, path: string, query?: Readonly>) => { + const key = queryKey(query); + return ( + routes.find( + (route) => + route.path === path && + (route.method ?? 'GET') === method && + queryKey(route.query) === key, + ) ?? + routes.find( + (route) => route.path === path && (route.method ?? 'GET') === method && route.query == null, + ) + ); + }; + + return { + calls, + async request(request) { + const method = request.method ?? 'GET'; + calls.push({ + method, + path: request.path, + ...(request.query ? { query: { ...request.query } } : {}), + }); + const route = matchRoute(method, request.path, request.query); + if (!route) { + throw new Error(`No fake GitLab route for ${method} ${request.path}?${queryKey(request.query)}`); + } + const response = + typeof route.response === 'function' + ? await route.response({ method, path: request.path, query: request.query }) + : route.response; + return response as never; + }, + async requestPages(request) { + // Collect all pages if fake routes include page query variants; else one shot. + const values: Array = []; + let page = 1; + while (page < 50) { + const pageQuery = { ...(request.query ?? {}), page, per_page: 100 }; + const route = matchRoute('GET', request.path, pageQuery) ?? (page === 1 ? matchRoute('GET', request.path, request.query) : undefined); + if (!route) { + break; + } + calls.push({ method: 'GET', path: request.path, query: pageQuery }); + const response = + typeof route.response === 'function' + ? await route.response({ method: 'GET', path: request.path, query: pageQuery }) + : route.response; + const pageValues = Array.isArray(response) ? response : []; + values.push(...pageValues); + if (pageValues.length < 100) { + break; + } + page += 1; + } + return values; + }, + async requestText(request) { + calls.push({ + method: 'GET', + path: request.path, + ...(request.query ? { query: { ...request.query } } : {}), + }); + const route = matchRoute('GET', request.path, request.query); + if (!route || route.text == null) { + throw new Error(`No fake GitLab text route for GET ${request.path}`); + } + return typeof route.text === 'function' + ? route.text({ method: 'GET', path: request.path, query: request.query }) + : route.text; + }, + }; +}; diff --git a/gitlab/src/version-commit-evolution.ts b/gitlab/src/version-commit-evolution.ts new file mode 100644 index 00000000..c2efd218 --- /dev/null +++ b/gitlab/src/version-commit-evolution.ts @@ -0,0 +1,29 @@ +/** + * Re-export forge-neutral commit-stack evolution from Core. + * Kept as a stable GitLab package path for existing imports. + */ +export { + attributeRebaseDrivers, + createCommitPatchSignature, + matchVersionCommitStacks, + recommendVersionWalkthroughStructure, + scoreBaseCommitAsRebaseDriver, + toVersionCommitSummary, + versionCommitDiffConcurrency, + versionCommitEvolutionAlgorithmVersion, + versionCommitSignatureAlgorithmVersion, + versionCommitStackLimit, + type CommitPatchSignature, + type CommitStackEvolution, + type CommitStackEvolutionRange, + type DiffEndpointRef, + type VersionCommitEvolutionUnit, + type VersionCommitMatchKind, + type VersionCommitSummary, + type VersionRebaseDriverCommit, +} from '@nkzw/codiff-core'; + +import type { CommitStackEvolution } from '@nkzw/codiff-core'; + +/** @deprecated Prefer CommitStackEvolution. */ +export type MergeRequestVersionCommitEvolution = CommitStackEvolution; diff --git a/gitlab/src/version-compare.ts b/gitlab/src/version-compare.ts new file mode 100644 index 00000000..2d660537 --- /dev/null +++ b/gitlab/src/version-compare.ts @@ -0,0 +1,1341 @@ +// Merge-request version-comparison algorithm. +// Orchestration entry: computeVersionComparePreferringReplay() +// → materializeRebaseReplayTrees + computeRebaseReplayVersionCompare (preferred) +// → computeApproximatePatchTextVersionCompare (fallback when blobs missing) +// GitLab I/O + endpoint resolution live in merge-request.ts (fetchGitLabMergeRequestVersionCompare). +// Fate query: gitLabMergeRequestVersionCompare in server/src/fate/server.ts. +// See PLAN.md §G2. + +// Version-comparison control flow inspired by Jujutsu (jj) rebase_to_dest_parent + +// show_inter_diff (Apache-2.0). Clean-room TypeScript reimplementation; +// no jj source is vendored. + +// === Localized line diff (bounded Myers O(ND) algorithm) === + +const MAX_DIFF_D = 1000; + +type DiffEdit = { + kind: 'delete' | 'equal' | 'insert'; + line: string; +}; + +/** + * Compute the shortest edit script between two line arrays using Myers' O(ND) algorithm. + * Returns null if the edit distance exceeds maxD (pathological input). + */ +const myersEditScript = ( + a: ReadonlyArray, + b: ReadonlyArray, + maxD = MAX_DIFF_D, +): ReadonlyArray | null => { + const N = a.length; + const M = b.length; + + if (N === 0 && M === 0) { + return []; + } + if (N === 0) { + return b.map((line) => ({ kind: 'insert' as const, line })); + } + if (M === 0) { + return a.map((line) => ({ kind: 'delete' as const, line })); + } + // Minimum possible edit distance is |N-M|; bail early if already too large. + if (Math.abs(N - M) > maxD) { + return null; + } + + const MAX = Math.min(N + M, maxD); + const offset = MAX; + const size = 2 * MAX + 1; + const v = new Int32Array(size); + const trace: Array = []; + let solved = false; + + for (let d = 0; d <= MAX; d++) { + trace.push(v.slice()); + for (let k = -d; k <= d; k += 2) { + let x: number; + if (k === -d || (k !== d && v[k - 1 + offset]! < v[k + 1 + offset]!)) { + x = v[k + 1 + offset]!; + } else { + x = v[k - 1 + offset]! + 1; + } + let y = x - k; + while (x < N && y < M && a[x] === b[y]) { + x++; + y++; + } + v[k + offset] = x; + if (x >= N && y >= M) { + solved = true; + break; + } + } + if (solved) { + break; + } + } + + if (!solved) { + return null; + } + + // Backtrack to build edit script. + let x = N; + let y = M; + const edits: Array = []; + + for (let d = trace.length - 1; d > 0; d--) { + const vPrev = trace[d]!; + const k = x - y; + + let prevK: number; + if (k === -d || (k !== d && vPrev[k - 1 + offset]! < vPrev[k + 1 + offset]!)) { + prevK = k + 1; + } else { + prevK = k - 1; + } + + const prevX = vPrev[prevK + offset]!; + const prevY = prevX - prevK; + + // Diagonal (equal) moves from after-edit position to current (x, y). + const afterEditX = prevK === k + 1 ? prevX : prevX + 1; + while (x > afterEditX) { + x--; + y--; + edits.push({ kind: 'equal', line: a[x]! }); + } + + // The edit itself. + if (prevK === k + 1) { + edits.push({ kind: 'insert', line: b[prevY]! }); + } else { + edits.push({ kind: 'delete', line: a[prevX]! }); + } + + x = prevX; + y = prevY; + } + + // Initial snake (d=0): remaining diagonals from (0,0). + while (x > 0) { + x--; + y--; + edits.push({ kind: 'equal', line: a[x]! }); + } + + edits.reverse(); + return edits; +}; + +/** + * Format an edit script as unified diff hunks with limited context lines. + * Returns an empty string when the edit script contains no changes. + */ +const formatUnifiedHunks = ( + edits: ReadonlyArray, + leftEndsWithNewline: boolean, + rightEndsWithNewline: boolean, + contextLines = 3, +): string => { + // Find positions of changes. + const changePositions: Array = []; + for (let i = 0; i < edits.length; i++) { + if (edits[i]!.kind !== 'equal') { + changePositions.push(i); + } + } + if (changePositions.length === 0) { + return ''; + } + + // Group changes into hunk ranges [start, end) with context. + const hunkRanges: Array<[number, number]> = []; + let hunkStart = Math.max(0, changePositions[0]! - contextLines); + let hunkEnd = changePositions[0]! + 1; + + for (let i = 1; i < changePositions.length; i++) { + const pos = changePositions[i]!; + if (pos <= hunkEnd + 2 * contextLines) { + hunkEnd = pos + 1; + } else { + hunkRanges.push([hunkStart, Math.min(edits.length, hunkEnd + contextLines)]); + hunkStart = Math.max(0, pos - contextLines); + hunkEnd = pos + 1; + } + } + hunkRanges.push([hunkStart, Math.min(edits.length, hunkEnd + contextLines)]); + + // Pre-compute old/new line numbers at each edit position. + const oldLineAt: Array = []; + const newLineAt: Array = []; + let oln = 1; + let nln = 1; + for (let i = 0; i < edits.length; i++) { + oldLineAt.push(oln); + newLineAt.push(nln); + const kind = edits[i]!.kind; + if (kind === 'equal' || kind === 'delete') { + oln++; + } + if (kind === 'equal' || kind === 'insert') { + nln++; + } + } + const totalOldLines = oln - 1; + const totalNewLines = nln - 1; + + // Format each hunk. + const output: Array = []; + for (const [start, end] of hunkRanges) { + const hunkEdits = edits.slice(start, end); + const oldStart = oldLineAt[start]!; + const newStart = newLineAt[start]!; + let oldCount = 0; + let newCount = 0; + const body: Array = []; + + for (let i = 0; i < hunkEdits.length; i++) { + const edit = hunkEdits[i]!; + if (edit.kind === 'equal') { + body.push(` ${edit.line}`); + oldCount++; + newCount++; + // Check for no-newline marker on the last line of both files. + if ( + oldLineAt[start + i]! === totalOldLines && + newLineAt[start + i]! === totalNewLines && + !leftEndsWithNewline && + !rightEndsWithNewline + ) { + body.push(String.raw`\ No newline at end of file`); + } + } else if (edit.kind === 'delete') { + body.push(`-${edit.line}`); + oldCount++; + // No-newline marker for last old line. + if (oldLineAt[start + i]! === totalOldLines && !leftEndsWithNewline) { + body.push(String.raw`\ No newline at end of file`); + } + } else { + body.push(`+${edit.line}`); + newCount++; + // No-newline marker for last new line. + if (newLineAt[start + i]! === totalNewLines && !rightEndsWithNewline) { + body.push(String.raw`\ No newline at end of file`); + } + } + } + + output.push(`@@ -${oldStart},${oldCount} +${newStart},${newCount} @@`); + output.push(...body); + } + + return output.join('\n') + '\n'; +}; + +/** + * Compute a localized unified diff between two file contents. + * Returns `{ patchBody, incomplete }`. + * `incomplete` is true when the edit distance exceeds the bounded cap, + * in which case `patchBody` is empty and the caller should fall back. + */ +export const computeLineDiff = ( + left: string, + right: string, + contextLines = 3, +): { incomplete: boolean; patchBody: string } => { + if (left === right) { + return { incomplete: false, patchBody: '' }; + } + + const leftLines = left.length === 0 ? [] : left.replace(/\n$/, '').split('\n'); + const rightLines = right.length === 0 ? [] : right.replace(/\n$/, '').split('\n'); + + const editScript = myersEditScript(leftLines, rightLines); + if (!editScript) { + return { incomplete: true, patchBody: '' }; + } + + const leftEndsWithNewline = left.length > 0 && left.endsWith('\n'); + const rightEndsWithNewline = right.length > 0 && right.endsWith('\n'); + const patchBody = formatUnifiedHunks( + editScript, + leftEndsWithNewline, + rightEndsWithNewline, + contextLines, + ); + + return { incomplete: false, patchBody }; +}; + +// === Bounded concurrency pool === + +const poolMap = async ( + items: ReadonlyArray, + concurrency: number, + fn: (item: T) => Promise, +): Promise => { + let nextIndex = 0; + const worker = async () => { + while (nextIndex < items.length) { + const index = nextIndex++; + await fn(items[index]!); + } + }; + await Promise.all(Array.from({ length: Math.min(concurrency, items.length) }, () => worker())); +}; + +import type { ChangedFile } from '@nkzw/codiff-core/types'; + +export type MergeRequestVersionRef = { + baseSha: string; + createdAt: string; + headSha: string; + id: string; + label: string; + startSha: string; +}; + +export type VersionCompareEndpoint = + | { kind: 'mr-base' } + | { commentId: string; kind: 'comment-position' } + | { baseSha: string; headSha: string; kind: 'diff-identity'; startSha: string } + | { headSha: string; kind: 'head-sha' } + | { kind: 'last-reviewed' } + | { kind: 'mr-version'; versionId: string }; + +export type VersionCompareRange = { + from: MergeRequestVersionRef; + paths?: ReadonlyArray; + to: MergeRequestVersionRef; +}; + +export type VersionCompareHunkClass = + | 'comment-anchored' + | 'conflict-resolution' + | 'intentional' + | 'rebase-noise'; + +export type VersionCompareFile = { + classes: ReadonlyArray; + file: ChangedFile; + oldPath?: string; + path: string; + relatedCommentIds: ReadonlyArray; + status: 'added' | 'deleted' | 'modified' | 'renamed' | 'unchanged-noise'; +}; + +export type VersionBaseRef = { + committedAt: string | null; + sha: string; + shortSha: string; + webUrl?: string; +}; + +export type VersionBaseMovementCommit = { + authoredAt: string; + authorName: string; + sha: string; + shortSha: string; + subject: string; + webUrl: string; +}; + +export type VersionBaseMovement = { + changed: boolean; + commits: ReadonlyArray; + commitsBetween: number | null; + commitTimestampDeltaMs: number | null; + diffStat: { + additions: number; + deletions: number; + filesChanged: number; + } | null; + from: VersionBaseRef; + relationship: 'forward' | 'backward' | 'divergent' | 'unknown'; + to: VersionBaseRef; + truncated: boolean; + warning?: string; +}; + +export type MergeRequestVersionCompare = { + algorithm: 'approximate-patch-text' | 'jj-rebase-replay'; + baseMovement?: VersionBaseMovement; + commentAssociations: ReadonlyArray<{ + commentId: string; + filePath?: string; + status: 'newly-anchored' | 'outdated' | 'resolved-by-change' | 'still-valid'; + }>; + files: ReadonlyArray; + range: VersionCompareRange; + summary: { + addedLines: number; + baseMoved: boolean; + commentsAffected: number; + conflictFiles: number; + deletedLines: number; + empty: boolean; + filesChanged: number; + intentionalFiles: number; + noiseFiles: number; + }; + warnings?: ReadonlyArray; +}; + +const getVersionCompareLineStats = (files: ReadonlyArray) => { + let addedLines = 0; + let deletedLines = 0; + for (const file of files) { + for (const section of file.file.sections) { + for (const line of section.patch.split('\n')) { + if (line.startsWith('+') && !line.startsWith('+++')) { + addedLines += 1; + } else if (line.startsWith('-') && !line.startsWith('---')) { + deletedLines += 1; + } + } + } + } + return { addedLines, deletedLines }; +}; + +export type VersionPatchFile = { + newPath: string; + oldPath: string; + patchBody: string; + status: 'added' | 'deleted' | 'modified' | 'renamed'; +}; + +export type CommentAnchor = { + commentId: string; + filePath: string; + lineNumber?: number; + position: { + baseSha: string; + headSha: string; + startSha: string; + }; +}; + +const isRecord = (value: unknown): value is Record => + Boolean(value) && typeof value === 'object' && !Array.isArray(value); + +const hashString = (value: string) => { + let hash = 0; + for (let index = 0; index < value.length; index += 1) { + hash = (hash * 31 + value.charCodeAt(index)) >>> 0; + } + return hash.toString(16); +}; + +const normalizePatchBody = (patchBody: string) => + patchBody + .split('\n') + .filter((line) => line.startsWith('+') || line.startsWith('-') || line.startsWith(' ')) + .map((line) => line.replace(/^./, (prefix) => prefix)) + .join('\n') + .trim(); + +const changeRegionFingerprint = (patchBody: string) => { + const changes = patchBody + .split('\n') + .filter( + (line) => + (line.startsWith('+') || line.startsWith('-')) && + !line.startsWith('+++') && + !line.startsWith('---'), + ) + .map((line) => line.slice(1).trimEnd()) + .filter(Boolean); + return hashString(changes.join('\n')); +}; + +const createChangedFile = ( + path: string, + oldPath: string | undefined, + status: ChangedFile['status'], + patchBody: string, + headSha: string, + kind: 'version-compare' | 'conflict' = 'version-compare', +): ChangedFile => { + const sectionId = `${path}:commit:version-compare`; + const headerOld = status === 'added' ? '/dev/null' : `a/${oldPath ?? path}`; + const headerNew = status === 'deleted' ? '/dev/null' : `b/${path}`; + const patch = `diff --git a/${oldPath ?? path} b/${path}\n--- ${headerOld}\n+++ ${headerNew}\n${patchBody}${ + patchBody.endsWith('\n') ? '' : '\n' + }`; + return { + fingerprint: `${headSha}:${kind}:${status}:${oldPath ?? path}:${path}:${patch.length}`, + ...(oldPath && oldPath !== path ? { oldPath } : {}), + path, + sections: [ + { + binary: false, + id: sectionId, + kind: 'commit', + loadState: 'ready', + patch, + }, + ], + status, + }; +}; + +const pathKey = (file: VersionPatchFile) => file.newPath || file.oldPath; + +const pairFiles = ( + fromFiles: ReadonlyArray, + toFiles: ReadonlyArray, +) => { + const fromByPath = new Map(fromFiles.map((file) => [pathKey(file), file])); + const toByPath = new Map(toFiles.map((file) => [pathKey(file), file])); + const paths = new Set([...fromByPath.keys(), ...toByPath.keys()]); + return [...paths] + .toSorted((first, second) => first.localeCompare(second)) + .map((path) => ({ + from: fromByPath.get(path) ?? null, + path, + to: toByPath.get(path) ?? null, + })); +}; + +type PatchRegion = { + body: string; + newEnd: number; + newStart: number; + oldEnd: number; + oldStart: number; +}; + +const patchRegions = (patch: string): ReadonlyArray => { + const matches = [...patch.matchAll(/^@@\s+-(\d+)(?:,(\d+))?\s+\+(\d+)(?:,(\d+))?\s+@@.*$/gm)]; + return matches.map((match, index) => { + const oldStart = Number(match[1]); + const oldCount = Number(match[2] ?? 1); + const newStart = Number(match[3]); + const newCount = Number(match[4] ?? 1); + return { + body: patch.slice(match.index! + match[0].length, matches[index + 1]?.index ?? patch.length), + newEnd: newStart + Math.max(0, newCount - 1), + newStart, + oldEnd: oldStart + Math.max(0, oldCount - 1), + oldStart, + }; + }); +}; + +const regionAtLine = (patch: string, lineNumber: number) => + patchRegions(patch).find( + (region) => + (lineNumber >= region.newStart - 2 && lineNumber <= region.newEnd + 2) || + (lineNumber >= region.oldStart - 2 && lineNumber <= region.oldEnd + 2), + ); + +const commentRegionChanged = ( + fromPatch: string | undefined, + toPatch: string | undefined, + lineNumber: number, +) => { + const before = fromPatch ? regionAtLine(fromPatch, lineNumber) : undefined; + const after = toPatch ? regionAtLine(toPatch, lineNumber) : undefined; + if (!before && !after) { + return false; + } + if (before && !after && toPatch) { + const beforeBody = normalizePatchBody(before.body); + return !patchRegions(toPatch).some( + (candidate) => normalizePatchBody(candidate.body) === beforeBody, + ); + } + if (after && !before && fromPatch) { + const afterBody = normalizePatchBody(after.body); + return !patchRegions(fromPatch).some( + (candidate) => normalizePatchBody(candidate.body) === afterBody, + ); + } + if (!before || !after) { + return true; + } + return normalizePatchBody(before.body) !== normalizePatchBody(after.body); +}; + +const commentContentWindowChanged = ( + before: string | undefined, + after: string | undefined, + lineNumber: number, +) => { + if (before == null || after == null) { + return before !== after; + } + const start = Math.max(0, lineNumber - 3); + const end = lineNumber + 2; + return ( + before.split('\n').slice(start, end).join('\n') !== + after.split('\n').slice(start, end).join('\n') + ); +}; + +const classifyCommentAssociations = ( + comments: ReadonlyArray, + intentionalPaths: ReadonlySet, + from: MergeRequestVersionRef, + to: MergeRequestVersionRef, + addressedCommentIds: ReadonlySet = new Set(), +) => + comments.map((comment) => { + const onFrom = + comment.position.headSha === from.headSha || + comment.position.baseSha === from.baseSha || + comment.position.startSha === from.startSha; + const pathTouched = intentionalPaths.has(comment.filePath); + if (!onFrom) { + return { + commentId: comment.commentId, + filePath: comment.filePath, + status: pathTouched ? ('newly-anchored' as const) : ('still-valid' as const), + }; + } + if (addressedCommentIds.has(comment.commentId)) { + return { + commentId: comment.commentId, + filePath: comment.filePath, + status: 'resolved-by-change' as const, + }; + } + if (pathTouched) { + return { + commentId: comment.commentId, + filePath: comment.filePath, + status: 'outdated' as const, + }; + } + if (comment.position.headSha === to.headSha) { + return { + commentId: comment.commentId, + filePath: comment.filePath, + status: 'still-valid' as const, + }; + } + return { + commentId: comment.commentId, + filePath: comment.filePath, + status: 'still-valid' as const, + }; + }); + +/** + * Approximate jj version comparison when only patch text is available: + * compare the logical change regions of each version's MR patch. + * Pure rebases (identical change regions) produce an empty intentional set. + */ +export const computeApproximatePatchTextVersionCompare = ({ + comments = [], + from, + fromFiles, + paths, + to, + toFiles, +}: { + comments?: ReadonlyArray; + from: MergeRequestVersionRef; + fromFiles: ReadonlyArray; + paths?: ReadonlyArray; + to: MergeRequestVersionRef; + toFiles: ReadonlyArray; +}): MergeRequestVersionCompare => { + const pathFilter = paths?.length ? new Set(paths) : null; + const pairs = pairFiles(fromFiles, toFiles).filter( + (pair) => pathFilter == null || pathFilter.has(pair.path), + ); + const files: Array = []; + const warnings: Array = []; + const addressedCommentIds = new Set(); + const baseMoved = from.baseSha !== to.baseSha; + + for (const pair of pairs) { + for (const comment of comments) { + if ( + comment.filePath === pair.path && + comment.lineNumber != null && + commentRegionChanged(pair.from?.patchBody, pair.to?.patchBody, comment.lineNumber) + ) { + addressedCommentIds.add(comment.commentId); + } + } + if (!pair.from && pair.to) { + files.push({ + classes: ['intentional'], + file: createChangedFile( + pair.to.newPath, + pair.to.oldPath, + pair.to.status, + pair.to.patchBody, + to.headSha, + ), + oldPath: pair.to.oldPath !== pair.to.newPath ? pair.to.oldPath : undefined, + path: pair.path, + relatedCommentIds: comments + .filter((comment) => comment.filePath === pair.path) + .map((comment) => comment.commentId), + status: pair.to.status, + }); + continue; + } + if (pair.from && !pair.to) { + files.push({ + classes: ['intentional'], + file: createChangedFile( + pair.from.newPath, + pair.from.oldPath, + 'deleted', + pair.from.patchBody, + to.headSha, + ), + oldPath: pair.from.oldPath !== pair.from.newPath ? pair.from.oldPath : undefined, + path: pair.path, + relatedCommentIds: comments + .filter((comment) => comment.filePath === pair.path) + .map((comment) => comment.commentId), + status: 'deleted', + }); + continue; + } + if (!pair.from || !pair.to) { + continue; + } + + const fromFingerprint = changeRegionFingerprint(pair.from.patchBody); + const toFingerprint = changeRegionFingerprint(pair.to.patchBody); + if (fromFingerprint === toFingerprint) { + // Pure rebase / identical logical patch — hide by default. + continue; + } + + // Prefer the newer version's patch body as the visible versionCompare surface. + // When both exist and differ, mark intentional; if markers look conflicted, + // flag conflict-resolution. + const conflictLike = + pair.to.patchBody.includes('<<<<<<<') || + pair.to.patchBody.includes('>>>>>>>') || + pair.from.patchBody.includes('<<<<<<<'); + const classes: Array = conflictLike + ? ['conflict-resolution', 'intentional'] + : ['intentional']; + if ( + baseMoved && + normalizePatchBody(pair.from.patchBody) === normalizePatchBody(pair.to.patchBody) + ) { + // Safety net: identical normalized text after base move. + continue; + } + if (baseMoved && fromFingerprint !== toFingerprint && !conflictLike) { + // Approximate path only — note degraded fidelity for consumers. + warnings.push( + `Approximate version comparison for ${pair.path} (could not replay onto new base).`, + ); + } + files.push({ + classes, + file: createChangedFile( + pair.to.newPath, + pair.to.oldPath, + pair.to.status, + pair.to.patchBody, + to.headSha, + conflictLike ? 'conflict' : 'version-compare', + ), + oldPath: pair.to.oldPath !== pair.to.newPath ? pair.to.oldPath : undefined, + path: pair.path, + relatedCommentIds: comments + .filter((comment) => comment.filePath === pair.path) + .map((comment) => comment.commentId), + status: pair.to.status, + }); + } + + const intentionalPaths = new Set( + files + .filter( + (file) => + file.classes.includes('intentional') || file.classes.includes('conflict-resolution'), + ) + .map((file) => file.path), + ); + const commentAssociations = classifyCommentAssociations( + comments, + intentionalPaths, + from, + to, + addressedCommentIds, + ); + const intentionalFiles = files.filter((file) => file.classes.includes('intentional')).length; + const conflictFiles = files.filter((file) => file.classes.includes('conflict-resolution')).length; + + return { + algorithm: 'approximate-patch-text', + commentAssociations, + files, + range: { + from, + ...(paths?.length ? { paths } : {}), + to, + }, + summary: { + ...getVersionCompareLineStats(files), + baseMoved, + commentsAffected: commentAssociations.filter((item) => item.status !== 'still-valid').length, + conflictFiles, + empty: files.length === 0, + filesChanged: files.length, + intentionalFiles, + noiseFiles: 0, + }, + ...(warnings.length > 0 ? { warnings: [...new Set(warnings)] } : {}), + }; +}; + +/** + * jj-faithful control flow when left/right trees (path→content) are available: + * left = from tree when bases match, else apply from-patch onto to-base; + * versionCompare = diff(left, right=to tree). + */ +export const computeRebaseReplayVersionCompare = ({ + comments = [], + from, + fromTree, + paths, + to, + toTree, +}: { + comments?: ReadonlyArray; + from: MergeRequestVersionRef; + fromTree: ReadonlyMap; + paths?: ReadonlyArray; + to: MergeRequestVersionRef; + toTree: ReadonlyMap; +}): MergeRequestVersionCompare & { incompleteDiffPaths: ReadonlyArray } => { + const pathFilter = paths?.length ? new Set(paths) : null; + const allPaths = new Set([...fromTree.keys(), ...toTree.keys()]); + const files: Array = []; + const incompleteDiffPaths: Array = []; + const addressedCommentIds = new Set(); + const baseMoved = from.baseSha !== to.baseSha; + + for (const path of [...allPaths].toSorted((a, b) => a.localeCompare(b))) { + if (pathFilter && !pathFilter.has(path)) { + continue; + } + const left = fromTree.get(path); + const right = toTree.get(path); + if (left === right) { + continue; + } + for (const comment of comments) { + if ( + comment.filePath === path && + comment.lineNumber != null && + commentContentWindowChanged(left, right, comment.lineNumber) + ) { + addressedCommentIds.add(comment.commentId); + } + } + if (left == null && right != null) { + files.push({ + classes: ['intentional'], + file: createChangedFile( + path, + undefined, + 'added', + `@@ -0,0 +1,${right.split('\n').length} @@\n${right + .split('\n') + .map((line) => `+${line}`) + .join('\n')}\n`, + to.headSha, + ), + path, + relatedCommentIds: comments + .filter((comment) => comment.filePath === path) + .map((comment) => comment.commentId), + status: 'added', + }); + continue; + } + if (left != null && right == null) { + files.push({ + classes: ['intentional'], + file: createChangedFile( + path, + path, + 'deleted', + `@@ -1,${left.split('\n').length} +0,0 @@\n${left + .split('\n') + .map((line) => `-${line}`) + .join('\n')}\n`, + to.headSha, + ), + path, + relatedCommentIds: comments + .filter((comment) => comment.filePath === path) + .map((comment) => comment.commentId), + status: 'deleted', + }); + continue; + } + if (left == null || right == null) { + continue; + } + const conflictLike = left.includes('<<<<<<<') || right.includes('<<<<<<<'); + const diff = computeLineDiff(left, right); + if (diff.incomplete) { + incompleteDiffPaths.push(path); + continue; + } + if (!diff.patchBody) { + // computeLineDiff determined the files are identical after line splitting. + continue; + } + files.push({ + classes: conflictLike ? ['conflict-resolution', 'intentional'] : ['intentional'], + file: createChangedFile( + path, + path, + 'modified', + diff.patchBody, + to.headSha, + conflictLike ? 'conflict' : 'version-compare', + ), + path, + relatedCommentIds: comments + .filter((comment) => comment.filePath === path) + .map((comment) => comment.commentId), + status: 'modified', + }); + } + + const intentionalPaths = new Set(files.map((file) => file.path)); + const commentAssociations = classifyCommentAssociations( + comments, + intentionalPaths, + from, + to, + addressedCommentIds, + ); + + return { + algorithm: 'jj-rebase-replay', + commentAssociations, + files, + incompleteDiffPaths, + range: { + from, + ...(paths?.length ? { paths } : {}), + to, + }, + summary: { + ...getVersionCompareLineStats(files), + baseMoved, + commentsAffected: commentAssociations.filter((item) => item.status !== 'still-valid').length, + conflictFiles: files.filter((file) => file.classes.includes('conflict-resolution')).length, + empty: files.length === 0, + filesChanged: files.length, + intentionalFiles: files.length, + noiseFiles: 0, + }, + }; +}; + +export const versionCompareAlgorithmVersion = 'jj-rebase-replay-v4'; + +export type BlobLookup = (path: string, ref: string) => Promise | string | null; + +const parseHunkHeader = (line: string) => { + const match = /^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@/.exec(line); + if (!match) { + return null; + } + return { + newCount: match[4] == null ? 1 : Number(match[4]), + newStart: Number(match[3]), + oldCount: match[2] == null ? 1 : Number(match[2]), + oldStart: Number(match[1]), + }; +}; + +/** Apply a unified-diff body (no git headers) onto source text. Returns conflict markers on mismatch. */ +export const applyUnifiedPatchBody = ( + source: string, + patchBody: string, +): { conflict: boolean; text: string } => { + const sourceLines = source.length === 0 ? [] : source.replace(/\n$/, '').split('\n'); + const patchLines = patchBody.replace(/\n$/, '').split('\n'); + const output: Array = []; + let sourceIndex = 0; + let cursor = 0; + let conflict = false; + + while (cursor < patchLines.length) { + const header = parseHunkHeader(patchLines[cursor] ?? ''); + if (!header) { + cursor += 1; + continue; + } + cursor += 1; + const oldStart = Math.max(0, header.oldStart - 1); + while (sourceIndex < oldStart && sourceIndex < sourceLines.length) { + output.push(sourceLines[sourceIndex] ?? ''); + sourceIndex += 1; + } + while (cursor < patchLines.length) { + const line = patchLines[cursor] ?? ''; + if (line.startsWith('@@')) { + break; + } + cursor += 1; + if (line.startsWith('\\')) { + continue; + } + const prefix = line[0] ?? ' '; + const content = line.slice(1); + if (prefix === ' ' || prefix === '-') { + const expected = sourceLines[sourceIndex]; + if (expected !== content) { + conflict = true; + // Materialize a conflict block and skip remaining hunk application for this file. + const remainingSource = sourceLines.slice(sourceIndex).join('\n'); + const remainingPatch = patchLines.slice(cursor - 1).join('\n'); + return { + conflict: true, + text: `${output.join('\n')}${output.length ? '\n' : ''}<<<<<<< source\n${remainingSource}\n=======\n${remainingPatch}\n>>>>>>> patch\n`, + }; + } + if (prefix === ' ') { + output.push(content); + } + sourceIndex += 1; + } else if (prefix === '+') { + output.push(content); + } + } + } + while (sourceIndex < sourceLines.length) { + output.push(sourceLines[sourceIndex] ?? ''); + sourceIndex += 1; + } + const text = output.join('\n'); + return { + conflict, + text: + source.endsWith('\n') || text.length === 0 + ? `${text}${text.length ? '\n' : ''}` + : `${text}\n`, + }; +}; + +const collectVersionComparePaths = ({ + comments = [], + fromFiles, + paths, + toFiles, +}: { + comments?: ReadonlyArray; + fromFiles: ReadonlyArray; + paths?: ReadonlyArray; + toFiles: ReadonlyArray; +}) => { + if (paths?.length) { + return [...new Set(paths)]; + } + return [ + ...new Set([ + ...fromFiles.map(pathKey), + ...toFiles.map(pathKey), + ...comments.map((comment) => comment.filePath), + ]), + ].toSorted((a, b) => a.localeCompare(b)); +}; + +/** + * Materialize left/right trees for jj-style rebase-then-diff. + * left = headA when bases match, else apply(from-patch, onto baseB). + * right = headB tree for the same path set. + */ +export const materializeRebaseReplayTrees = async ({ + from, + fromFiles, + paths, + readBlob, + to, + toFiles, +}: { + from: MergeRequestVersionRef; + fromFiles: ReadonlyArray; + paths?: ReadonlyArray; + readBlob: BlobLookup; + to: MergeRequestVersionRef; + toFiles: ReadonlyArray; +}): Promise<{ + fromTree: Map; + incompletePaths: Array; + toTree: Map; + warnings: Array; +}> => { + const targetPaths = collectVersionComparePaths({ fromFiles, paths, toFiles }); + const fromByPath = new Map(fromFiles.map((file) => [pathKey(file), file])); + const toByPath = new Map(toFiles.map((file) => [pathKey(file), file])); + const fromTree = new Map(); + const toTree = new Map(); + const incompletePaths: Array = []; + const warnings: Array = []; + const basesMatch = from.baseSha === to.baseSha; + + await poolMap(targetPaths, 8, async (path) => { + const fromFile = fromByPath.get(path); + const toFile = toByPath.get(path); + try { + const rightBlob = await readBlob(path, to.headSha); + if (rightBlob != null) { + toTree.set(path, rightBlob); + } else if (toFile?.status === 'deleted') { + // deleted in to → absent from right tree + } else if (toFile) { + // Reconstruct right from baseB + to-patch when head blob missing. + const toBaseBlob = (await readBlob(path, to.baseSha)) ?? ''; + if (toFile.status === 'added' && !toBaseBlob) { + const applied = applyUnifiedPatchBody('', toFile.patchBody); + if (applied.conflict) { + incompletePaths.push(path); + warnings.push(`Conflict reconstructing ${path} at head ${to.headSha.slice(0, 7)}.`); + return; + } + toTree.set(path, applied.text); + } else if (toBaseBlob || toFile.patchBody) { + const applied = applyUnifiedPatchBody(toBaseBlob, toFile.patchBody); + if (applied.conflict) { + incompletePaths.push(path); + warnings.push(`Conflict reconstructing ${path} at head ${to.headSha.slice(0, 7)}.`); + return; + } + toTree.set(path, applied.text); + } else { + incompletePaths.push(path); + return; + } + } + + if (basesMatch) { + const leftBlob = await readBlob(path, from.headSha); + if (leftBlob != null) { + fromTree.set(path, leftBlob); + } else if (fromFile?.status === 'deleted') { + // absent + } else if (fromFile) { + const fromBaseBlob = (await readBlob(path, from.baseSha)) ?? ''; + const applied = applyUnifiedPatchBody(fromBaseBlob, fromFile.patchBody); + if (applied.conflict) { + incompletePaths.push(path); + warnings.push(`Conflict replaying ${path} onto ${from.baseSha.slice(0, 7)}.`); + return; + } + fromTree.set(path, applied.text); + } else if (rightBlob != null) { + // Path only in to; left equals shared base/head-from content if present. + const shared = await readBlob(path, from.headSha); + if (shared != null) { + fromTree.set(path, shared); + } + } + } else { + // Rebase from-patch onto to.base. + const onto = (await readBlob(path, to.baseSha)) ?? ''; + if (fromFile) { + let replayOk = false; + if (fromFile.status === 'added' && !onto) { + const applied = applyUnifiedPatchBody('', fromFile.patchBody); + if (!applied.conflict) { + fromTree.set(path, applied.text); + replayOk = true; + } + } else { + const applied = applyUnifiedPatchBody(onto, fromFile.patchBody); + if (!applied.conflict) { + fromTree.set(path, applied.text); + replayOk = true; + } + } + if (!replayOk) { + // Replay produced a conflict — the base changed in a way that + // affects this file's patch. Fall back to the actual v11 head + // blob. The resulting diff (head-A vs head-B) may include some + // base-change noise alongside intentional changes, but is still + // localized and far more useful than dropping the path. + const headABlob = await readBlob(path, from.headSha); + if (headABlob != null) { + fromTree.set(path, headABlob); + warnings.push( + `Replay conflict for ${path} — comparing v${from.label} head directly (may include base changes).`, + ); + } else { + incompletePaths.push(path); + warnings.push(`Conflict replaying ${path} onto ${to.baseSha.slice(0, 7)}.`); + } + } + } else if (onto) { + // Unchanged in from MR; left is just the new base content. + fromTree.set(path, onto); + } + } + } catch (error) { + incompletePaths.push(path); + warnings.push( + `Missing blobs for ${path}: ${error instanceof Error ? error.message : String(error)}`, + ); + } + }); + + return { fromTree, incompletePaths, toTree, warnings }; +}; + +/** + * Prefer jj rebase-replay when trees can be materialized; fall back per-file + * to approximate patch-text comparison for incomplete paths only. + */ +export const computeVersionComparePreferringReplay = async ({ + comments = [], + from, + fromFiles, + paths, + readBlob, + to, + toFiles, +}: { + comments?: ReadonlyArray; + from: MergeRequestVersionRef; + fromFiles: ReadonlyArray; + paths?: ReadonlyArray; + readBlob: BlobLookup; + to: MergeRequestVersionRef; + toFiles: ReadonlyArray; +}): Promise => { + const targetPaths = collectVersionComparePaths({ comments, fromFiles, paths, toFiles }); + const materialization = await materializeRebaseReplayTrees({ + from, + fromFiles, + paths, + readBlob, + to, + toFiles, + }); + const replayableCount = targetPaths.length - materialization.incompletePaths.length; + const canReplay = + targetPaths.length === 0 + ? false + : materialization.incompletePaths.length === 0 || + replayableCount >= Math.ceil(targetPaths.length / 2); + + if (canReplay && (materialization.fromTree.size > 0 || materialization.toTree.size > 0)) { + const replay = computeRebaseReplayVersionCompare({ + comments, + from, + fromTree: materialization.fromTree, + paths, + to, + toTree: materialization.toTree, + }); + const incomplete = new Set([...materialization.incompletePaths, ...replay.incompleteDiffPaths]); + if (incomplete.size === 0) { + return { + ...replay, + ...(materialization.warnings.length + ? { warnings: [...new Set([...(replay.warnings ?? []), ...materialization.warnings])] } + : {}), + }; + } + // Fill gaps with approximate for incomplete paths only. + const approx = computeApproximatePatchTextVersionCompare({ + comments, + from, + fromFiles: fromFiles.filter((file) => incomplete.has(pathKey(file))), + paths: [...incomplete], + to, + toFiles: toFiles.filter((file) => incomplete.has(pathKey(file))), + }); + const files = [...replay.files, ...approx.files].toSorted((a, b) => + a.path.localeCompare(b.path), + ); + const commentAssociations = comments.flatMap((comment) => { + const source = incomplete.has(comment.filePath) ? approx : replay; + const association = source.commentAssociations.find( + (candidate) => candidate.commentId === comment.commentId, + ); + return association ? [association] : []; + }); + return { + algorithm: 'jj-rebase-replay', + commentAssociations, + files, + range: { + from, + ...(paths?.length ? { paths } : {}), + to, + }, + summary: { + ...getVersionCompareLineStats(files), + baseMoved: from.baseSha !== to.baseSha, + commentsAffected: commentAssociations.filter((item) => item.status !== 'still-valid') + .length, + conflictFiles: files.filter((file) => file.classes.includes('conflict-resolution')).length, + empty: files.length === 0, + filesChanged: files.length, + intentionalFiles: files.filter((file) => file.classes.includes('intentional')).length, + noiseFiles: 0, + }, + warnings: [ + ...new Set([ + ...(replay.warnings ?? []), + ...(approx.warnings ?? []), + ...materialization.warnings, + ...[...incomplete].map( + (path) => + `Approximate version comparison for ${path} (could not replay onto new base).`, + ), + ]), + ], + }; + } + + const approx = computeApproximatePatchTextVersionCompare({ + comments, + from, + fromFiles, + paths, + to, + toFiles, + }); + return { + ...approx, + warnings: [ + ...new Set([ + ...(approx.warnings ?? []), + ...materialization.warnings, + 'Fell back to approximate patch-text version comparison (insufficient blobs for rebase-replay).', + ]), + ], + }; +}; + +export const isMergeRequestVersionRef = (value: unknown): value is MergeRequestVersionRef => { + if (!isRecord(value)) { + return false; + } + return ( + typeof value.id === 'string' && + typeof value.baseSha === 'string' && + typeof value.startSha === 'string' && + typeof value.headSha === 'string' && + typeof value.createdAt === 'string' && + typeof value.label === 'string' + ); +}; diff --git a/gitlab/tsconfig.build.json b/gitlab/tsconfig.build.json new file mode 100644 index 00000000..bddab4a7 --- /dev/null +++ b/gitlab/tsconfig.build.json @@ -0,0 +1,13 @@ +{ + "extends": "../tsconfig.json", + "compilerOptions": { + "declaration": true, + "declarationMap": false, + "emitDeclarationOnly": true, + "incremental": false, + "noEmit": false, + "outDir": "./dist", + "rootDir": "./src" + }, + "include": ["src/**/*.ts"] +} diff --git a/gitlab/vite.config.ts b/gitlab/vite.config.ts new file mode 100644 index 00000000..ef7d6468 --- /dev/null +++ b/gitlab/vite.config.ts @@ -0,0 +1,8 @@ +import { defineConfig } from 'vite-plus'; + +export default defineConfig({ + pack: { + copy: [], + dts: false, + }, +}); diff --git a/package.json b/package.json index a0cf2ef6..3d746647 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,7 @@ "type": "module", "main": "./electron/main.cjs", "scripts": { - "build": "vp run --filter '@nkzw/codiff-core' build && vp run --filter '@nkzw/codiff-service' build && vp run --filter '@nkzw/codiff-web' build && vp build", + "build": "vp run --filter '@nkzw/codiff-core' build && vp run --filter '@nkzw/codiff-gitlab' build && vp run --filter '@nkzw/codiff-service' build && vp run --filter '@nkzw/codiff-web' build && vp build", "codiff": "node ./bin/codiff.js", "dev": "vp dev --host 127.0.0.1", "dev:app": "ELECTRON_RENDERER_URL=http://127.0.0.1:5173 node ./bin/codiff.js", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 552c8262..f89224ff 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -6,15 +6,9 @@ settings: catalogs: default: - vite: - specifier: npm:@voidzero-dev/vite-plus-core@^0.2.4 - version: 0.2.4 vite-plus: specifier: ^0.2.4 version: 0.2.4 - vitest: - specifier: 4.1.10 - version: 4.1.10 overrides: extract-zip>yauzl: ^3.4.0 @@ -153,6 +147,12 @@ importers: specifier: ^1.4.2 version: 1.4.2(typescript@7.0.2) + gitlab: + dependencies: + '@nkzw/codiff-core': + specifier: workspace:* + version: link:../core + service: dependencies: '@nkzw/codiff-core': diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 7bd77c5a..ebafdf45 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -2,6 +2,7 @@ packages: - core - service - web + - gitlab allowBuilds: '@google/genai': false diff --git a/vite.config.ts b/vite.config.ts index 2f5e327e..1bbdfe22 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -8,6 +8,12 @@ import { defineConfig } from 'vite-plus'; const testWorkers = Math.max(1, Math.min(4, Math.floor(availableParallelism() / 6))); export default defineConfig({ + // Added by jjk so Vite/Vitest does not watch jj internals (see jj FAQ). + server: { + watch: { + ignored: ['**/.jj/**'], + }, + }, base: './', build: { chunkSizeWarningLimit: 10 * 1024, @@ -107,6 +113,7 @@ export default defineConfig({ test: { include: [ 'core/**/*.test.{ts,tsx}', + 'gitlab/**/*.test.ts', 'electron/**/*.test.ts', 'service/**/*.test.ts', 'web/**/*.test.{ts,tsx}', From 9a36ff677965f49c8a2553b68a84563dfb29895c Mon Sep 17 00:00:00 2001 From: Matt Alonso Date: Mon, 20 Jul 2026 16:25:48 -0500 Subject: [PATCH 5/8] Exercise GitLab comparison and evolution cases Add focused fixtures for pure rebases, edited rebases, conflict resolutions, strategy classification, and commit pairing. Keeping the high-volume scenario coverage separate makes the provider implementation reviewable while preserving regression coverage for the analysis behavior Codiff Web will depend on. --- gitlab/__tests__/review-strategy.test.ts | 160 +++++ .../version-commit-evolution.test.ts | 366 +++++++++++ gitlab/__tests__/version-compare.test.ts | 602 ++++++++++++++++++ gitlab/fixtures/version-compare-cases.ts | 83 +++ 4 files changed, 1211 insertions(+) create mode 100644 gitlab/__tests__/review-strategy.test.ts create mode 100644 gitlab/__tests__/version-commit-evolution.test.ts create mode 100644 gitlab/__tests__/version-compare.test.ts create mode 100644 gitlab/fixtures/version-compare-cases.ts diff --git a/gitlab/__tests__/review-strategy.test.ts b/gitlab/__tests__/review-strategy.test.ts new file mode 100644 index 00000000..1495a041 --- /dev/null +++ b/gitlab/__tests__/review-strategy.test.ts @@ -0,0 +1,160 @@ +import { expect, test } from 'vite-plus/test'; +import { + classifyGitLabCommit, + classifyMergeRequestReviewStrategy, + versionCompareReviewStructureKey, + orderCommitsTopologically, + overrideMergeRequestReviewStrategy, + type GitLabMergeRequestCommitLike, +} from '../src/review-strategy.ts'; + +const commit = ( + overrides: Partial & + Pick, +): GitLabMergeRequestCommitLike => ({ + authoredDate: '2026-07-01T00:00:00.000Z', + authorName: 'Ada', + message: overrides.title, + parentIds: ['parent'], + shortSha: overrides.sha.slice(0, 8), + ...overrides, +}); + +test('classifies fixup and review-response subjects', () => { + expect(classifyGitLabCommit(commit({ sha: 'aaaaaaaa', title: 'fixup! add gate' })).role).toBe( + 'fixup', + ); + expect( + classifyGitLabCommit(commit({ sha: 'bbbbbbbb', title: 'Address review comments' })).role, + ).toBe('review-response'); + expect(classifyGitLabCommit(commit({ sha: 'cccccccc', title: 'feat: add gate' })).role).toBe( + 'feature', + ); +}); + +test('prefers whole-mr for single commits and fixup-heavy histories', () => { + expect( + classifyMergeRequestReviewStrategy({ + commits: [commit({ sha: 'aaaaaaaa', title: 'feat: only one' })], + description: '', + title: 'One change', + }), + ).toMatchObject({ mode: 'whole-mr', reason: 'single-commit' }); + + expect( + classifyMergeRequestReviewStrategy({ + commits: [ + commit({ sha: 'aaaaaaaa', title: 'feat: start' }), + commit({ sha: 'bbbbbbbb', title: 'fixup! start' }), + commit({ sha: 'cccccccc', title: 'Address review comments' }), + commit({ sha: 'dddddddd', title: 'nits' }), + ], + description: '', + title: 'WIP', + }), + ).toMatchObject({ mode: 'whole-mr', reason: 'fixup-style' }); +}); + +test('detects explicit commit-by-commit requests and chapter-shaped stacks', () => { + expect( + classifyMergeRequestReviewStrategy({ + commits: [ + commit({ sha: 'aaaaaaaa', title: 'feat: model' }), + commit({ sha: 'bbbbbbbb', title: 'test: cover model' }), + ], + description: 'Please review commit by commit.', + title: 'Stack', + }), + ).toMatchObject({ mode: 'commit-by-commit', reason: 'explicit-description' }); + + expect( + classifyMergeRequestReviewStrategy({ + commits: [ + commit({ sha: 'aaaaaaaa', title: 'feat: add endpoint' }), + commit({ sha: 'bbbbbbbb', title: 'test: cover endpoint' }), + commit({ sha: 'cccccccc', title: 'docs: document endpoint' }), + ], + description: '', + title: 'Endpoint stack', + }), + ).toMatchObject({ mode: 'commit-by-commit' }); +}); +test('treats a short distinct descriptive history as commit-by-commit', () => { + expect( + classifyMergeRequestReviewStrategy({ + commits: [ + commit({ sha: 'aaaaaaaa', title: 'Add the request path' }), + commit({ sha: 'bbbbbbbb', title: 'Handle retries' }), + ], + description: '', + title: 'Request handling', + }), + ).toMatchObject({ mode: 'commit-by-commit', reason: 'chapter-shaped' }); +}); + +test('honors explicit whole-mr language over chapter shape', () => { + expect( + classifyMergeRequestReviewStrategy({ + commits: [ + commit({ sha: 'aaaaaaaa', title: 'feat: a' }), + commit({ sha: 'bbbbbbbb', title: 'feat: b' }), + ], + description: 'Please review as a whole; squash on merge.', + title: 'Two features', + }), + ).toMatchObject({ mode: 'whole-mr', reason: 'explicit-whole' }); +}); + +test('supports user overrides for walkthrough structure', () => { + const base = classifyMergeRequestReviewStrategy({ + commits: [ + commit({ sha: 'aaaaaaaa', title: 'feat: a' }), + commit({ sha: 'bbbbbbbb', title: 'test: a' }), + ], + description: '', + title: 'Stack', + }); + expect(overrideMergeRequestReviewStrategy(base, 'whole-mr')).toMatchObject({ + mode: 'whole-mr', + reason: 'user-override', + }); + expect(overrideMergeRequestReviewStrategy(base, 'commit-by-commit')).toMatchObject({ + mode: 'commit-by-commit', + reason: 'user-override', + }); + const forcedCommitStrategy = overrideMergeRequestReviewStrategy( + { confidence: 1, mode: 'whole-mr', reason: 'default' }, + 'commit-by-commit', + [commit({ sha: 'cccccccc', title: 'Add routing' })], + ); + expect(forcedCommitStrategy).toMatchObject({ + commits: [expect.objectContaining({ sha: 'cccccccc' })], + mode: 'commit-by-commit', + reason: 'user-override', + }); +}); + +test('builds version-comparison walkthrough cache structure keys', () => { + expect(versionCompareReviewStructureKey('1', '2')).toBe('version-compare:1:2:whole-diff'); + expect(versionCompareReviewStructureKey('1', '2', 'commit-by-commit')).toBe( + 'version-compare:1:2:commit-by-commit', + ); +}); + +test('orders commits topologically from the merge-request base toward its head', () => { + const oldest = classifyGitLabCommit( + commit({ parentIds: ['base'], sha: 'aaaaaaaa', title: 'First' }), + ); + const middle = classifyGitLabCommit( + commit({ parentIds: [oldest.sha], sha: 'bbbbbbbb', title: 'Second' }), + ); + const newest = classifyGitLabCommit( + commit({ parentIds: [middle.sha], sha: 'cccccccc', title: 'Third' }), + ); + + expect(orderCommitsTopologically([newest, middle, oldest]).map((entry) => entry.sha)).toEqual([ + oldest.sha, + middle.sha, + newest.sha, + ]); +}); diff --git a/gitlab/__tests__/version-commit-evolution.test.ts b/gitlab/__tests__/version-commit-evolution.test.ts new file mode 100644 index 00000000..6be8a169 --- /dev/null +++ b/gitlab/__tests__/version-commit-evolution.test.ts @@ -0,0 +1,366 @@ +import type { ChangedFile } from '@nkzw/codiff-core/types'; +import { describe, expect, test } from 'vite-plus/test'; +import { + attributeRebaseDrivers, + createCommitPatchSignature, + matchVersionCommitStacks, + recommendVersionWalkthroughStructure, + scoreBaseCommitAsRebaseDriver, + type CommitPatchSignature, +} from '../src/version-commit-evolution.ts'; + +const endpoint = (id: string, headSha: string) => ({ + baseSha: `base-${id}`, + createdAt: '2026-07-15T00:00:00.000Z', + headSha, + id, + label: `v${id}`, + startSha: `base-${id}`, +}); + +const commit = (index: number, generation: 'new' | 'old' = 'old') => ({ + authoredDate: `2026-07-15T00:${String(index).padStart(2, '0')}:00.000Z`, + authorName: 'Ada', + message: `Change logical unit ${index}`, + parentIds: index === 1 ? [`${generation}-base`] : [`${generation}-${index - 1}`], + sha: `${generation}-${index}`, + shortSha: `${generation[0]}${index}`, + title: `Change logical unit ${index}`, + webUrl: `https://gitlab.example/commit/${generation}-${index}`, +}); + +const signature = ( + sha: string, + index: number, + patchId: string, + revision = 'same', +): CommitPatchSignature => ({ + additions: 10 + index, + changedPaths: [`src/unit-${index}.ts`], + changeTokenSketch: [`token-${index}`, `revision-${revision}`], + commitSha: sha, + deletions: index, + exactPatchId: patchId, + filesChanged: 1, + subjectKey: `change logical unit ${index}`, +}); + +const changedFile = (offset: number): ChangedFile => ({ + fingerprint: String(offset), + path: 'src/app.ts', + sections: [ + { + binary: false, + id: String(offset), + kind: 'commit', + loadState: 'ready', + patch: `diff --git a/src/app.ts b/src/app.ts\n--- a/src/app.ts\n+++ b/src/app.ts\n@@ -${offset},1 +${offset},1 @@\n-old value\n+new value\n`, + }, + ], + status: 'modified', +}); + +describe('version commit evolution', () => { + test('returns only revised logical commits 2, 4, and 6 as reviewable after a rebase', async () => { + const oldCommits = Array.from({ length: 10 }, (_, index) => commit(index + 1)); + const newCommits = Array.from({ length: 10 }, (_, index) => commit(index + 1, 'new')); + const revised = new Set([2, 4, 6]); + const signatures = new Map(); + for (let index = 1; index <= 10; index += 1) { + signatures.set( + `old-${index}`, + signature(`old-${index}`, index, `patch-${index}`, revised.has(index) ? 'old' : 'same'), + ); + signatures.set( + `new-${index}`, + signature( + `new-${index}`, + index, + revised.has(index) ? `revised-patch-${index}` : `patch-${index}`, + revised.has(index) ? 'new' : 'same', + ), + ); + } + const evolution = await matchVersionCommitStacks({ + from: endpoint('2', 'old-10'), + newCommits, + oldCommits, + signatures, + to: endpoint('6', 'new-10'), + }); + + expect( + evolution.units.filter((unit) => unit.reviewable).map((unit) => unit.before?.sha), + ).toEqual(['old-2', 'old-4', 'old-6']); + expect(evolution.summary).toMatchObject({ + reviewable: 3, + revised: 3, + rewrittenSamePatch: 7, + }); + expect(evolution.recommendation.structure).toBe('commit-by-commit'); + }); + + test('keeps duplicate patch IDs unmatched instead of forcing identity', async () => { + const oldCommits = [commit(1), commit(2)]; + const newCommits = [commit(1, 'new'), commit(2, 'new')]; + const signatures = new Map([ + ['old-1', signature('old-1', 1, 'duplicate')], + ['old-2', signature('old-2', 2, 'duplicate')], + ['new-1', signature('new-1', 1, 'duplicate')], + ['new-2', signature('new-2', 2, 'duplicate')], + ]); + const evolution = await matchVersionCommitStacks({ + from: endpoint('1', 'old-2'), + newCommits, + oldCommits, + signatures, + to: endpoint('2', 'new-2'), + }); + expect(evolution.summary.rewrittenSamePatch).toBe(0); + }); + + test('keeps commits unclassified when patch evidence or one stack is unavailable', async () => { + const oldCommits = [commit(1)]; + const newCommits = [commit(1, 'new')]; + const withoutPatches = await matchVersionCommitStacks({ + from: endpoint('1', 'old-1'), + newCommits, + oldCommits, + signatures: new Map(), + to: endpoint('2', 'new-1'), + }); + expect(withoutPatches.summary).toMatchObject({ added: 0, ambiguous: 2, removed: 0 }); + expect(withoutPatches.units.every((unit) => !unit.reviewable)).toBe(true); + + const withOnlyLaterStack = await matchVersionCommitStacks({ + from: endpoint('1', 'old-1'), + newCommits, + oldCommits: [], + signatures: new Map([['new-1', signature('new-1', 1, 'new-patch')]]), + stackCompleteness: { new: true, old: false }, + to: endpoint('2', 'new-1'), + }); + expect(withOnlyLaterStack.summary).toMatchObject({ added: 0, ambiguous: 1, removed: 0 }); + expect(withOnlyLaterStack.units[0]?.matchReasons).toContain( + 'Insufficient evidence to classify this commit as new', + ); + + const unrelatedNewCommit = { + ...newCommits[0]!, + authorName: 'Lin', + message: 'Replace an unrelated subsystem', + title: 'Replace an unrelated subsystem', + }; + const unrelatedNewSignature = { + ...signature('new-1', 1, 'unrelated-patch'), + additions: 1000, + changedPaths: ['unrelated/file.ts'], + changeTokenSketch: ['unrelated-token'], + subjectKey: 'replace an unrelated subsystem', + }; + const unrelatedChanges = await matchVersionCommitStacks({ + from: endpoint('1', 'old-1'), + newCommits: [unrelatedNewCommit], + oldCommits, + signatures: new Map([ + ['old-1', signature('old-1', 1, 'old-patch')], + ['new-1', unrelatedNewSignature], + ]), + to: endpoint('2', 'new-1'), + }); + expect(unrelatedChanges.summary).toMatchObject({ added: 1, ambiguous: 0, removed: 1 }); + }); + + test('classifies earlier MR commits rewritten into the later target base', async () => { + const oldCommits = [commit(1), commit(2), commit(3)]; + const newCommits = [commit(3, 'new'), commit(4, 'new'), commit(5, 'new')]; + const baseCommits = [ + { ...commit(1, 'new'), sha: 'base-1', shortSha: 'b1' }, + { ...commit(2, 'new'), sha: 'base-2', shortSha: 'b2' }, + ]; + const signatures = new Map([ + ['old-1', signature('old-1', 1, 'old-patch-1', 'old')], + ['old-2', signature('old-2', 2, 'old-patch-2', 'old')], + ['old-3', signature('old-3', 3, 'old-patch-3', 'old')], + ['base-1', signature('base-1', 1, 'base-patch-1', 'new')], + ['base-2', signature('base-2', 2, 'base-patch-2', 'new')], + ['new-3', signature('new-3', 3, 'new-patch-3', 'new')], + ['new-4', signature('new-4', 4, 'new-patch-4')], + ['new-5', signature('new-5', 5, 'new-patch-5')], + ]); + + const evolution = await matchVersionCommitStacks({ + baseCommits, + from: endpoint('1', 'old-3'), + newCommits, + oldCommits, + signatures, + to: endpoint('2', 'new-5'), + }); + + expect(evolution.summary).toMatchObject({ + absorbedIntoBase: 2, + added: 2, + ambiguous: 0, + reviewable: 3, + revised: 1, + }); + expect( + evolution.units + .filter((unit) => unit.kind === 'absorbed-into-base') + .map((unit) => [unit.before?.sha, unit.baseCommit?.sha]), + ).toEqual([ + ['old-1', 'base-1'], + ['old-2', 'base-2'], + ]); + expect(evolution.recommendation.structure).toBe('commit-by-commit'); + }); + + test('normalizes hunk offsets and stores only hashed changed-line tokens', async () => { + const first = await createCommitPatchSignature({ sha: 'a', title: 'Update app' }, [ + changedFile(1), + ]); + const second = await createCommitPatchSignature({ sha: 'b', title: 'Update app' }, [ + changedFile(100), + ]); + expect(first.exactPatchId).toBe(second.exactPatchId); + expect(JSON.stringify(first.changeTokenSketch)).not.toContain('value'); + }); + + test('recommends structure from content confidence without an upper commit-count bound', () => { + expect( + recommendVersionWalkthroughStructure({ ambiguous: 0, pairingCoverage: 1, reviewable: 1 }) + .structure, + ).toBe('whole-diff'); + expect( + recommendVersionWalkthroughStructure({ ambiguous: 0, pairingCoverage: 0.5, reviewable: 3 }) + .structure, + ).toBe('whole-diff'); + expect( + recommendVersionWalkthroughStructure({ ambiguous: 0, pairingCoverage: 1, reviewable: 30 }) + .structure, + ).toBe('commit-by-commit'); + }); +}); + +describe('rebase driver attribution', () => { + test('scores overlapping base commits as likely drivers', () => { + const unitSignature = { + additions: 12, + changedPaths: ['src/auth.ts', 'src/session.ts'], + changeTokenSketch: ['token', 'session', 'refresh'], + commitSha: 'unit', + deletions: 4, + exactPatchId: 'unit-patch', + filesChanged: 2, + subjectKey: 'preserve empty fields', + } satisfies CommitPatchSignature; + const overlapping = scoreBaseCommitAsRebaseDriver({ + baseSignature: { + additions: 20, + changedPaths: ['src/auth.ts', 'src/login.ts'], + changeTokenSketch: ['token', 'login'], + deletions: 3, + }, + unitSignature, + }); + const unrelated = scoreBaseCommitAsRebaseDriver({ + baseSignature: { + additions: 8, + changedPaths: ['docs/readme.md'], + changeTokenSketch: ['docs', 'readme'], + deletions: 1, + }, + unitSignature, + }); + expect(overlapping.overlappingPaths).toEqual(['src/auth.ts']); + expect(overlapping.score).toBeGreaterThan(unrelated.score); + expect(overlapping.score).toBeGreaterThanOrEqual(0.22); + }); + + test('returns ranked overlapping base commits only', () => { + const unitSignature = { + additions: 10, + changedPaths: ['src/auth.ts'], + changeTokenSketch: ['token'], + commitSha: 'unit', + deletions: 2, + exactPatchId: 'unit-patch', + filesChanged: 1, + subjectKey: 'preserve empty fields', + } satisfies CommitPatchSignature; + const baseSignatures = new Map([ + [ + 'base-strong', + { + additions: 15, + changedPaths: ['src/auth.ts', 'src/token.ts'], + changeTokenSketch: ['token', 'auth'], + commitSha: 'base-strong', + deletions: 2, + exactPatchId: 'base-strong', + filesChanged: 2, + subjectKey: 'harden auth tokens', + }, + ], + [ + 'base-weak', + { + additions: 4, + changedPaths: ['src/auth.ts'], + changeTokenSketch: ['format'], + commitSha: 'base-weak', + deletions: 1, + exactPatchId: 'base-weak', + filesChanged: 1, + subjectKey: 'format auth file', + }, + ], + [ + 'base-unrelated', + { + additions: 30, + changedPaths: ['package.json'], + changeTokenSketch: ['deps'], + commitSha: 'base-unrelated', + deletions: 5, + exactPatchId: 'base-unrelated', + filesChanged: 1, + subjectKey: 'bump deps', + }, + ], + ]); + const drivers = attributeRebaseDrivers({ + baseCommits: [ + { + authoredAt: '2026-07-01T00:00:00.000Z', + authorName: 'Ada', + sha: 'base-strong', + shortSha: 'strong', + subject: 'Harden auth tokens', + webUrl: 'https://gitlab.example/base-strong', + }, + { + authoredAt: '2026-07-02T00:00:00.000Z', + authorName: 'Grace', + sha: 'base-weak', + shortSha: 'weak', + subject: 'Format auth file', + webUrl: 'https://gitlab.example/base-weak', + }, + { + authoredAt: '2026-07-03T00:00:00.000Z', + authorName: 'Lin', + sha: 'base-unrelated', + shortSha: 'unrel', + subject: 'Bump deps', + webUrl: 'https://gitlab.example/base-unrelated', + }, + ], + baseSignatures, + unitSignature, + }); + expect(drivers.map((driver) => driver.sha)).toEqual(['base-strong', 'base-weak']); + expect(drivers[0]?.overlappingPaths).toEqual(['src/auth.ts']); + }); +}); diff --git a/gitlab/__tests__/version-compare.test.ts b/gitlab/__tests__/version-compare.test.ts new file mode 100644 index 00000000..f429937d --- /dev/null +++ b/gitlab/__tests__/version-compare.test.ts @@ -0,0 +1,602 @@ +import { expect, test } from 'vite-plus/test'; +import { + conflictResolutionFiles, + pureRebaseFiles, + pureRebaseVersions, + rebasePlusEditFiles, +} from '../fixtures/version-compare-cases.ts'; +import { + applyUnifiedPatchBody, + computeApproximatePatchTextVersionCompare, + computeLineDiff, + computeVersionComparePreferringReplay, + computeRebaseReplayVersionCompare, + materializeRebaseReplayTrees, + type MergeRequestVersionRef, + type VersionPatchFile, +} from '../src/version-compare.ts'; + +const version = (id: string, headSha: string, baseSha: string): MergeRequestVersionRef => ({ + baseSha, + createdAt: '2026-07-01T00:00:00.000Z', + headSha, + id, + label: `v${id}`, + startSha: baseSha, +}); + +const patchFile = ( + path: string, + body: string, + status: VersionPatchFile['status'] = 'modified', +): VersionPatchFile => ({ + newPath: path, + oldPath: path, + patchBody: body, + status, +}); + +test('approximate version comparison is empty for pure rebase (identical change regions)', () => { + const body = '@@ -1 +1 @@\n-old\n+new\n'; + const result = computeApproximatePatchTextVersionCompare({ + from: version('1', 'head-a', 'base-a'), + fromFiles: [patchFile('src/app.ts', body)], + to: version('2', 'head-b', 'base-b'), + toFiles: [patchFile('src/app.ts', body)], + }); + expect(result.summary.empty).toBe(true); + expect(result.summary.intentionalFiles).toBe(0); + expect(result.summary.baseMoved).toBe(true); + expect(result.files).toEqual([]); +}); + +test('approximate version comparison surfaces intentional edits after rebase', () => { + const result = computeApproximatePatchTextVersionCompare({ + comments: [ + { + commentId: '42', + filePath: 'src/app.ts', + lineNumber: 2, + position: { + baseSha: 'base-a', + headSha: 'head-a', + startSha: 'base-a', + }, + }, + ], + from: version('1', 'head-a', 'base-a'), + fromFiles: [patchFile('src/app.ts', '@@ -1 +1 @@\n-old\n+new\n')], + to: version('2', 'head-b', 'base-b'), + toFiles: [patchFile('src/app.ts', '@@ -1 +1 @@\n-old\n+new and safer\n')], + }); + expect(result.summary.empty).toBe(false); + expect(result.summary.addedLines).toBe(1); + expect(result.summary.deletedLines).toBe(1); + expect(result.summary.intentionalFiles).toBe(1); + expect(result.files[0]?.path).toBe('src/app.ts'); + expect(result.commentAssociations[0]?.status).toBe('resolved-by-change'); +}); + +test('does not call a comment addressed when only another hunk in the file changed', () => { + const comments = [ + { + commentId: 'comment-100', + filePath: 'src/app.ts', + lineNumber: 100, + position: { baseSha: 'base-a', headSha: 'head-a', startSha: 'base-a' }, + }, + ]; + const result = computeApproximatePatchTextVersionCompare({ + comments, + from: version('1', 'head-a', 'base-a'), + fromFiles: [ + patchFile('src/app.ts', '@@ -1 +1 @@\n-old\n+new\n@@ -100 +100 @@\n-keep\n+kept\n'), + ], + to: version('2', 'head-b', 'base-a'), + toFiles: [ + patchFile('src/app.ts', '@@ -1 +1 @@\n-old\n+newer\n@@ -100 +100 @@\n-keep\n+kept\n'), + ], + }); + expect(result.commentAssociations[0]?.status).toBe('outdated'); +}); + +test('rebase-replay version comparison diffs left vs right trees', () => { + const from = version('1', 'head-a', 'base-shared'); + const to = version('2', 'head-b', 'base-shared'); + const result = computeRebaseReplayVersionCompare({ + from, + fromTree: new Map([ + ['src/app.ts', 'const value = 1;\n'], + ['README.md', 'hello\n'], + ]), + to, + toTree: new Map([ + ['src/app.ts', 'const value = 2;\n'], + ['README.md', 'hello\n'], + ]), + }); + expect(result.algorithm).toBe('jj-rebase-replay'); + expect(result.summary.filesChanged).toBe(1); + expect(result.files[0]?.path).toBe('src/app.ts'); + expect(result.summary.baseMoved).toBe(false); +}); + +test('conflict markers are classified as conflict-resolution', () => { + const result = computeApproximatePatchTextVersionCompare({ + from: version('1', 'head-a', 'base-a'), + fromFiles: [patchFile('src/app.ts', '@@ -1 +1 @@\n-old\n+new\n')], + to: version('2', 'head-b', 'base-b'), + toFiles: [ + patchFile('src/app.ts', '@@ -1,3 +1,5 @@\n<<<<<<<\n-old\n=======\n+resolved\n>>>>>>>\n'), + ], + }); + expect(result.files[0]?.classes).toContain('conflict-resolution'); + expect(result.summary.conflictFiles).toBe(1); +}); + +test('fixture: pure rebase yields empty intentional set', () => { + const result = computeApproximatePatchTextVersionCompare({ + from: pureRebaseVersions.from, + fromFiles: pureRebaseFiles.from, + to: pureRebaseVersions.to, + toFiles: pureRebaseFiles.to, + }); + expect(result.summary.empty).toBe(true); + expect(result.summary.intentionalFiles).toBe(0); +}); + +test('fixture: rebase plus edit surfaces only changed files', () => { + const result = computeApproximatePatchTextVersionCompare({ + from: pureRebaseVersions.from, + fromFiles: rebasePlusEditFiles.from, + to: pureRebaseVersions.to, + toFiles: rebasePlusEditFiles.to, + }); + expect(result.summary.empty).toBe(false); + expect(result.files.map((file) => file.path).toSorted()).toEqual(['README.md', 'src/app.ts']); +}); + +test('fixture: conflict resolution is high signal', () => { + const result = computeApproximatePatchTextVersionCompare({ + from: pureRebaseVersions.from, + fromFiles: conflictResolutionFiles.from, + to: pureRebaseVersions.to, + toFiles: conflictResolutionFiles.to, + }); + expect(result.summary.conflictFiles).toBe(1); + expect(result.files[0]?.classes).toContain('conflict-resolution'); +}); + +test('applyUnifiedPatchBody applies a simple hunk', () => { + const result = applyUnifiedPatchBody( + 'const value = 1;\nold();\n', + '@@ -1,2 +1,2 @@\n const value = 1;\n-old();\n+newCall();\n', + ); + expect(result.conflict).toBe(false); + expect(result.text).toBe('const value = 1;\nnewCall();\n'); +}); + +test('applyUnifiedPatchBody reports conflicts on context mismatch', () => { + const result = applyUnifiedPatchBody( + 'const value = 1;\nother();\n', + '@@ -1,2 +1,2 @@\n const value = 1;\n-old();\n+newCall();\n', + ); + expect(result.conflict).toBe(true); + expect(result.text).toContain('<<<<<<<'); +}); + +test('preferring replay uses jj algorithm when blobs are available', async () => { + const from = version('1', 'head-a', 'base-a'); + const to = version('2', 'head-b', 'base-b'); + const blobs = new Map([ + ['base-b:src/app.ts', 'const value = 1;\nold();\n'], + ['head-b:src/app.ts', 'const value = 1;\nnewCall();\nguard();\n'], + ]); + const result = await computeVersionComparePreferringReplay({ + from, + fromFiles: [ + patchFile('src/app.ts', '@@ -1,2 +1,2 @@\n const value = 1;\n-old();\n+newCall();\n'), + ], + readBlob: (path, ref) => blobs.get(`${ref}:${path}`) ?? null, + to, + toFiles: [ + patchFile( + 'src/app.ts', + '@@ -1,2 +1,3 @@\n const value = 1;\n-old();\n+newCall();\n+guard();\n', + ), + ], + }); + expect(result.algorithm).toBe('jj-rebase-replay'); + expect(result.summary.empty).toBe(false); + expect(result.files[0]?.path).toBe('src/app.ts'); + // left = apply(from-patch onto base-b) => newCall only; right has guard + expect(result.files[0]?.file.sections[0]?.patch).toContain('+guard();'); +}); + +test('preferring replay falls back to approximate when patch reconstruction conflicts', async () => { + const result = await computeVersionComparePreferringReplay({ + from: version('1', 'head-a', 'base-a'), + fromFiles: [patchFile('src/app.ts', '@@ -1 +1 @@\n-old\n+new\n')], + readBlob: async () => null, + to: version('2', 'head-b', 'base-b'), + toFiles: [patchFile('src/app.ts', '@@ -1 +1 @@\n-old\n+new and safer\n')], + }); + // Without blobs, patch reconstruction onto empty source conflicts. + // The path is reported incomplete and falls back to approximate comparison. + expect(result.algorithm).toBe('approximate-patch-text'); + expect(result.summary.empty).toBe(false); +}); + +test('preferring replay falls back when materialization cannot recover paths', async () => { + const result = await computeVersionComparePreferringReplay({ + from: version('1', 'head-a', 'base-a'), + fromFiles: [], + readBlob: async () => null, + to: version('2', 'head-b', 'base-b'), + // Binary / empty patch with no blobs and no reconstructable body. + toFiles: [ + { + newPath: 'src/binary.bin', + oldPath: 'src/binary.bin', + patchBody: '', + status: 'modified', + }, + ], + }); + expect(result.algorithm).toBe('approximate-patch-text'); + expect(result.warnings?.some((warning) => warning.includes('Fell back'))).toBe(true); +}); + +test('materialize trees rebases from-patch onto new base', async () => { + const from = version('1', 'head-a', 'base-a'); + const to = version('2', 'head-b', 'base-b'); + const trees = await materializeRebaseReplayTrees({ + from, + fromFiles: [ + patchFile('src/app.ts', '@@ -1,2 +1,2 @@\n const value = 1;\n-old();\n+newCall();\n'), + ], + readBlob: (path, ref) => { + if (ref === 'base-b' && path === 'src/app.ts') { + return 'const value = 1;\nold();\n'; + } + if (ref === 'head-b' && path === 'src/app.ts') { + return 'const value = 1;\nnewCall();\n'; + } + return null; + }, + to, + toFiles: [ + patchFile('src/app.ts', '@@ -1,2 +1,2 @@\n const value = 1;\n-old();\n+newCall();\n'), + ], + }); + expect(trees.fromTree.get('src/app.ts')).toBe('const value = 1;\nnewCall();\n'); + expect(trees.toTree.get('src/app.ts')).toBe('const value = 1;\nnewCall();\n'); + expect(trees.incompletePaths).toEqual([]); +}); + +test('pure rebase with blobs yields empty versionCompare', async () => { + const from = version('1', 'head-a', 'base-a'); + const to = version('2', 'head-b', 'base-b'); + const patch = '@@ -1,2 +1,2 @@\n const value = 1;\n-old();\n+newCall();\n'; + const result = await computeVersionComparePreferringReplay({ + from, + fromFiles: [patchFile('src/app.ts', patch)], + readBlob: (path, ref) => { + if (path !== 'src/app.ts') { + return null; + } + if (ref === 'base-b') { + return 'const value = 1;\nold();\n'; + } + if (ref === 'head-b') { + return 'const value = 1;\nnewCall();\n'; + } + return null; + }, + to, + toFiles: [patchFile('src/app.ts', patch)], + }); + expect(result.algorithm).toBe('jj-rebase-replay'); + expect(result.summary.empty).toBe(true); + expect(result.files).toEqual([]); +}); + +// === Regression tests (plan.md §4) === + +test('computeLineDiff: unchanged function in a changed file is excluded (Test A)', () => { + // Two file versions where one hunk differs and an unchanged region + // (apply_target_strategy) is identical. The localized diff must exclude + // the unchanged region entirely. + const left = + [ + 'fn setup() {', + ' init();', + '}', + '', + 'fn apply_target_strategy() {', + ' // complex logic', + ' let x = compute();', + ' validate(x);', + '}', + '', + 'fn teardown() {', + ' cleanup();', + '}', + ].join('\n') + '\n'; + + const right = + [ + 'fn setup() {', + ' init();', + ' extra_setup();', + '}', + '', + 'fn apply_target_strategy() {', + ' // complex logic', + ' let x = compute();', + ' validate(x);', + '}', + '', + 'fn teardown() {', + ' cleanup();', + '}', + ].join('\n') + '\n'; + + const result = computeLineDiff(left, right); + expect(result.incomplete).toBe(false); + expect(result.patchBody).not.toBe(''); + // The diff should contain the added line. + expect(result.patchBody).toContain('+ extra_setup();'); + // The unchanged function must NOT appear as a changed line. + expect(result.patchBody).not.toContain('+fn apply_target_strategy'); + expect(result.patchBody).not.toContain('-fn apply_target_strategy'); + expect(result.patchBody).not.toContain('+ let x = compute'); + expect(result.patchBody).not.toContain('- let x = compute'); + expect(result.patchBody).not.toContain('+ validate(x)'); + expect(result.patchBody).not.toContain('- validate(x)'); +}); + +test('rebase-replay produces localized hunks for modified files (Test A end-to-end)', () => { + const from = version('1', 'head-a', 'base-shared'); + const to = version('2', 'head-b', 'base-shared'); + const unchanged = [ + '', + 'fn apply_target_strategy() {', + ' // complex logic', + ' let x = compute();', + ' validate(x);', + '}', + ].join('\n'); + + const result = computeRebaseReplayVersionCompare({ + from, + fromTree: new Map([['authority.rs', `fn setup() {\n init();\n}${unchanged}\n`]]), + to, + toTree: new Map([ + ['authority.rs', `fn setup() {\n init();\n extra_setup();\n}${unchanged}\n`], + ]), + }); + expect(result.algorithm).toBe('jj-rebase-replay'); + expect(result.summary.filesChanged).toBe(1); + const patch = result.files[0]?.file.sections[0]?.patch ?? ''; + expect(patch).toContain('+ extra_setup();'); + // apply_target_strategy must not appear as a change. + expect(patch).not.toContain('-fn apply_target_strategy'); + expect(patch).not.toContain('+fn apply_target_strategy'); + expect(patch).not.toContain('- let x = compute'); + expect(patch).not.toContain('+ let x = compute'); +}); + +test('large path count uses rebase-replay, not approximate fallback (Test B)', async () => { + const from = version('1', 'head-a', 'base-a'); + const to = version('2', 'head-b', 'base-b'); + + // Create 20 files — well above the old 12-path cutoff. + const fromFiles: Array> = []; + const toFiles: Array> = []; + const blobs = new Map(); + for (let i = 0; i < 20; i++) { + const path = `src/file-${i}.ts`; + fromFiles.push(patchFile(path, `@@ -1 +1 @@\n-old${i}\n+new${i}\n`)); + toFiles.push(patchFile(path, `@@ -1 +1 @@\n-old${i}\n+new${i} revised\n`)); + blobs.set(`base-b:${path}`, `old${i}\n`); + blobs.set(`head-b:${path}`, `new${i} revised\n`); + } + + const result = await computeVersionComparePreferringReplay({ + from, + fromFiles, + readBlob: (path, ref) => blobs.get(`${ref}:${path}`) ?? null, + to, + toFiles, + }); + + // Must use rebase-replay, NOT approximate-patch-text. + expect(result.algorithm).toBe('jj-rebase-replay'); + // Should contain localized hunks (not whole-file replacements). + expect(result.summary.filesChanged).toBe(20); + // No warning about "patch-text version comparison because this range contains N files". + expect( + result.warnings?.some((w) => w.includes('patch-text version comparison because')), + ).toBeFalsy(); +}); + +test('likely-revised interdiff produces localized hunks (Test C)', async () => { + // A likely-revised commit pair where one function changed and another did not. + const from = version('old-sha', 'old-sha', 'old-parent'); + const to = version('new-sha', 'new-sha', 'new-parent'); + + const unchangedFn = '\nfn untouched() {\n stable_code();\n}\n'; + const fromPatch = `@@ -1,2 +1,2 @@\n fn main() {\n- v1();\n+ v1_impl();\n`; + const toPatch = `@@ -1,2 +1,2 @@\n fn main() {\n- v1();\n+ v2_impl();\n`; + + const blobs = new Map(); + blobs.set('new-parent:src/lib.rs', `fn main() {\n v1();\n}${unchangedFn}`); + blobs.set('new-sha:src/lib.rs', `fn main() {\n v2_impl();\n}${unchangedFn}`); + + const result = await computeVersionComparePreferringReplay({ + from, + fromFiles: [patchFile('src/lib.rs', fromPatch)], + readBlob: (path, ref) => blobs.get(`${ref}:${path}`) ?? null, + to, + toFiles: [patchFile('src/lib.rs', toPatch)], + }); + + expect(result.algorithm).toBe('jj-rebase-replay'); + expect(result.summary.filesChanged).toBe(1); + const patch = result.files[0]?.file.sections[0]?.patch ?? ''; + // Should show the actual change. + expect(patch).toContain('- v1_impl();'); + expect(patch).toContain('+ v2_impl();'); + // The unchanged function must NOT appear as a change (+ or - prefix). + // It may appear as context (space prefix) — that is acceptable per plan.md. + expect(patch).not.toContain('-fn untouched'); + expect(patch).not.toContain('+fn untouched'); + expect(patch).not.toContain('- stable_code'); + expect(patch).not.toContain('+ stable_code'); +}); + +test('computeLineDiff handles identical files', () => { + const result = computeLineDiff('hello\nworld\n', 'hello\nworld\n'); + expect(result.incomplete).toBe(false); + expect(result.patchBody).toBe(''); +}); + +test('computeLineDiff handles empty-to-content', () => { + const result = computeLineDiff('', 'new line\n'); + expect(result.incomplete).toBe(false); + expect(result.patchBody).toContain('+new line'); +}); + +test('computeLineDiff handles content-to-empty', () => { + const result = computeLineDiff('old line\n', ''); + expect(result.incomplete).toBe(false); + expect(result.patchBody).toContain('-old line'); +}); + +test('computeLineDiff produces multiple independent hunks', () => { + // Two changes separated by many unchanged lines. + const shared = Array.from({ length: 20 }, (_, i) => `line ${i + 2}`).join('\n'); + const left = `first\n${shared}\nlast\n`; + const right = `FIRST\n${shared}\nLAST\n`; + const result = computeLineDiff(left, right); + expect(result.incomplete).toBe(false); + // Should produce two separate hunks. + const hunkHeaders = result.patchBody.match(/@@ /g); + expect(hunkHeaders?.length).toBe(2); +}); + +test('rebase-replay: added file shows full addition patch (Test D)', () => { + const from = version('1', 'head-a', 'base-shared'); + const to = version('2', 'head-b', 'base-shared'); + const result = computeRebaseReplayVersionCompare({ + from, + fromTree: new Map(), + to, + toTree: new Map([['new-file.ts', 'export const x = 1;\n']]), + }); + expect(result.summary.filesChanged).toBe(1); + expect(result.files[0]?.status).toBe('added'); + const patch = result.files[0]?.file.sections[0]?.patch ?? ''; + expect(patch).toContain('+export const x = 1;'); +}); + +test('rebase-replay: removed file shows full deletion patch (Test D)', () => { + const from = version('1', 'head-a', 'base-shared'); + const to = version('2', 'head-b', 'base-shared'); + const result = computeRebaseReplayVersionCompare({ + from, + fromTree: new Map([['old-file.ts', 'export const y = 2;\n']]), + to, + toTree: new Map(), + }); + expect(result.summary.filesChanged).toBe(1); + expect(result.files[0]?.status).toBe('deleted'); + const patch = result.files[0]?.file.sections[0]?.patch ?? ''; + expect(patch).toContain('-export const y = 2;'); +}); + +test('rebase-replay reports incompleteDiffPaths for pathological diffs', () => { + const from = version('1', 'head-a', 'base-shared'); + const to = version('2', 'head-b', 'base-shared'); + // Create two completely different files that exceed the Myers cap. + const left = Array.from({ length: 1500 }, (_, i) => `unique-old-${i}`).join('\n') + '\n'; + const right = Array.from({ length: 1500 }, (_, i) => `unique-new-${i}`).join('\n') + '\n'; + const result = computeRebaseReplayVersionCompare({ + from, + fromTree: new Map([['big.ts', left]]), + to, + toTree: new Map([['big.ts', right]]), + }); + // The pathological diff should be reported as incomplete, not as a whole-file replacement. + expect(result.incompleteDiffPaths).toContain('big.ts'); + expect(result.files.find((f) => f.path === 'big.ts')).toBeUndefined(); +}); + +test('conflict during patch replay falls back to direct head blob comparison', async () => { + // When replaying v11's patch onto v20's base produces a conflict (because + // the base changed), we should fall back to comparing the actual v11 head + // blob against the v20 head blob. This produces a real diff that may include + // some base-change noise but never shows raw +/- patch syntax. + const from = version('1', 'head-a', 'base-a'); + const to = version('2', 'head-b', 'base-b'); + + // The from-patch expects "old line" at line 1, but the new base has "different base line". + const fromPatch = '@@ -1,2 +1,2 @@\n old line\n-remove me\n+add me\n'; + const toPatch = '@@ -1,2 +1,2 @@\n different base line\n-other\n+result\n'; + + const blobs = new Map(); + blobs.set('base-b:src/app.rs', 'different base line\nother\n'); + blobs.set('head-a:src/app.rs', 'old line\nadd me\n'); + blobs.set('head-b:src/app.rs', 'different base line\nresult\n'); + + const result = await computeVersionComparePreferringReplay({ + from, + fromFiles: [patchFile('src/app.rs', fromPatch)], + readBlob: (path, ref) => blobs.get(`${ref}:${path}`) ?? null, + to, + toFiles: [patchFile('src/app.rs', toPatch)], + }); + + // Should still use rebase-replay algorithm (with blob fallback), not approximate. + expect(result.algorithm).toBe('jj-rebase-replay'); + expect(result.summary.filesChanged).toBe(1); + const patch = result.files[0]?.file.sections[0]?.patch ?? ''; + // Must never contain conflict markers. + expect(patch).not.toContain('<<<<<<< source'); + expect(patch).not.toContain('>>>>>>> patch'); + // Should show the actual diff between head-a and head-b content. + expect(patch).toContain('-old line'); + expect(patch).toContain('+different base line'); + // Warning should mention the replay conflict fallback. + expect(result.warnings?.some((w) => w.includes('Replay conflict'))).toBe(true); +}); + +test('conflict during replay with no head blob falls back to approximate', async () => { + const from = version('1', 'head-a', 'base-a'); + const to = version('2', 'head-b', 'base-b'); + + const fromPatch = '@@ -1,2 +1,2 @@\n old line\n-remove me\n+add me\n'; + const toPatch = '@@ -1,2 +1,2 @@\n different base line\n-other\n+result\n'; + + const blobs = new Map(); + blobs.set('base-b:src/app.rs', 'different base line\nother\n'); + blobs.set('head-b:src/app.rs', 'different base line\nresult\n'); + // head-a blob NOT available — can't do direct comparison + + const result = await computeVersionComparePreferringReplay({ + from, + fromFiles: [patchFile('src/app.rs', fromPatch)], + readBlob: (path, ref) => blobs.get(`${ref}:${path}`) ?? null, + to, + toFiles: [patchFile('src/app.rs', toPatch)], + }); + + // Without head-a blob, path is incomplete → approximate fallback. + // Must still never contain conflict markers. + for (const file of result.files) { + const patch = file.file.sections[0]?.patch ?? ''; + expect(patch).not.toContain('<<<<<<< source'); + expect(patch).not.toContain('>>>>>>> patch'); + } +}); diff --git a/gitlab/fixtures/version-compare-cases.ts b/gitlab/fixtures/version-compare-cases.ts new file mode 100644 index 00000000..c10b9971 --- /dev/null +++ b/gitlab/fixtures/version-compare-cases.ts @@ -0,0 +1,83 @@ +import type { MergeRequestVersionRef, VersionPatchFile } from '../version-compare.ts'; + +/** Synthetic fixtures used for version-comparison algorithm coverage without live GitLab. */ + +export const pureRebaseVersions = { + from: { + baseSha: 'base-a', + createdAt: '2026-06-01T00:00:00.000Z', + headSha: 'head-a', + id: '1', + label: 'v1 · head-a', + startSha: 'base-a', + }, + to: { + baseSha: 'base-b', + createdAt: '2026-06-02T00:00:00.000Z', + headSha: 'head-b', + id: '2', + label: 'v2 · head-b', + startSha: 'base-b', + }, +} satisfies { from: MergeRequestVersionRef; to: MergeRequestVersionRef }; + +const sameLogicalPatch = '@@ -1,2 +1,2 @@\n const value = 1;\n-old();\n+newCall();\n'; + +export const pureRebaseFiles = { + from: [ + { + newPath: 'src/app.ts', + oldPath: 'src/app.ts', + patchBody: sameLogicalPatch, + status: 'modified', + }, + ], + to: [ + { + newPath: 'src/app.ts', + oldPath: 'src/app.ts', + // Context lines shifted after rebase, same change regions. + patchBody: '@@ -10,2 +10,2 @@\n const value = 1;\n-old();\n+newCall();\n', + status: 'modified', + }, + ], +} satisfies { + from: ReadonlyArray; + to: ReadonlyArray; +}; + +export const rebasePlusEditFiles = { + from: pureRebaseFiles.from, + to: [ + { + newPath: 'src/app.ts', + oldPath: 'src/app.ts', + patchBody: '@@ -10,2 +10,3 @@\n const value = 1;\n-old();\n+newCall();\n+guard();\n', + status: 'modified', + }, + { + newPath: 'README.md', + oldPath: 'README.md', + patchBody: '@@ -1 +1 @@\n-hello\n+hello world\n', + status: 'modified', + }, + ], +} satisfies { + from: ReadonlyArray; + to: ReadonlyArray; +}; + +export const conflictResolutionFiles = { + from: pureRebaseFiles.from, + to: [ + { + newPath: 'src/app.ts', + oldPath: 'src/app.ts', + patchBody: '@@ -1,5 +1,7 @@\n<<<<<<< HEAD\n-old();\n=======\n+resolved();\n>>>>>>> feature\n', + status: 'modified', + }, + ], +} satisfies { + from: ReadonlyArray; + to: ReadonlyArray; +}; From 0f7f467116042d5b09f3d14cf46ea83ae28445fd Mon Sep 17 00:00:00 2001 From: Matt Alonso Date: Mon, 20 Jul 2026 16:25:50 -0500 Subject: [PATCH 6/8] Document Codiff Web review-history package boundaries Describe the shared Core and GitLab APIs, walkthrough-authoring ownership, generation inputs, and the Codiff Web migration checklist. Keep local Electron authentication and IPC guidance out of this document so the foundation MR remains scoped to reusable Web-facing packages. --- docs/review-history-packages.md | 96 +++++++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 docs/review-history-packages.md diff --git a/docs/review-history-packages.md b/docs/review-history-packages.md new file mode 100644 index 00000000..73a9ed7f --- /dev/null +++ b/docs/review-history-packages.md @@ -0,0 +1,96 @@ +# Review-history package notes + +These packages are the Codiff-side extraction target for Codiff Web. + +## Packages + +| Package | Notes | +| --------------------- | -------------------------------------------------------------------------------- | +| `@nkzw/codiff-core` | Review-history contracts + `walkthrough-authoring` + `generateReviewWalkthrough` | +| `@nkzw/codiff-gitlab` | Read-side GitLab history adapter over `GitLabTransport` | + +## New Core contracts + +Import from `@nkzw/codiff-core` / `@nkzw/codiff-core/types`: + +- `RevisionLabel`, `RevisionRef`, `DiffRange`, `DiffComparison` +- `DiffComparisonAnalysis`, `DiffComparisonView` +- `ReviewUnit`, `ReviewEvolutionUnit`, `ReviewCommitEvolution` +- `ReviewStructureRecommendation`, `ReviewPlan` +- `WalkthroughGenerationInput` +- helpers: `resolveReviewPlan`, `reviewVersionOption`, `project`-style builders in `lib/review-history` + +Shared review UI still exports compatibility aliases such as +`MergeRequestVersionOption`, but those now alias the Core contracts above. + +## Walkthrough authoring + +Import from `@nkzw/codiff-core/walkthrough-authoring`: + +- `parseWalkthroughDraft` / `parseAuthoredWalkthrough` +- `parseRepositoryState` +- `indexWalkthroughHunks` +- `buildWalkthroughPrompt` / `buildWalkthroughPromptInput` +- `normalizeWalkthroughDraft` / `normalizeAuthoredWalkthrough` +- `attachVersionCommentReferences` +- `composeUnitWalkthroughs` / `combineCommitWalkthroughs` + +This module owns draft validation, hunk aliasing, prompt construction, +normalization, and unit composition. Hosts keep model IDs, AI Gateway routing, +retries, Durable Object orchestration, and cache policy. + +Generation input rule: + +- whole-diff plan -> one `RepositoryState` +- units plan -> one `RepositoryState` per `ReviewUnit` + +## GitLab package + +Import from `@nkzw/codiff-gitlab`. + +Host implements: + +```ts +type GitLabTransport = { + request(request: { + method?: 'GET' | 'POST' | 'PUT' | 'DELETE'; + path: string; + query?: Readonly>; + body?: unknown; + }): Promise; + requestPages?(request: { + path: string; + query?: Readonly>; + }): Promise>; + requestText?(request: { + path: string; + query?: Readonly>; + }): Promise; +}; +``` + +Package owns: + +- endpoint construction and pagination +- version list / historical commit+diff loading +- version comparison materialization +- patch signatures, commit evolution, review-structure classification +- projection helpers: `projectVersionCompare`, `projectCommitEvolution`, + `projectReviewPlan`, `toGitLabDiffIdentity` + +Not migrated in this package extraction: + +- baseline current-MR snapshot loading +- write-side comments/reviews/merge mutations +- host authentication + +## Codiff Web migration checklist + +1. Depend on `@nkzw/codiff-core` and `@nkzw/codiff-gitlab`. +2. Implement Worker `GitLabTransport` over `GITLAB_VPC_BRIDGE`. +3. Replace local version-compare / evolution / review-strategy copies with package APIs. +4. Replace local walkthrough draft/prompt/normalize/combine helpers with + `@nkzw/codiff-core/walkthrough-authoring`. +5. Keep D1/R2, Durable Objects, Think/AI Gateway, and Fate orchestration in Web. +6. Feed Core `MergeRequestReviewApp` with projected Core contracts, not GitLab + wire shapes. From 0064afc8314f2f1fd957d2e1e37682cd0dd2ef8a Mon Sep 17 00:00:00 2001 From: Matt Alonso Date: Mon, 20 Jul 2026 17:10:51 -0500 Subject: [PATCH 7/8] Add shared walkthrough generation orchestration Resolve review plans into whole-diff or per-unit generation, invoke a host-supplied model runner, normalize each authored result, and compose unit walkthroughs in Core. This lets Codiff Web share one orchestration contract while retaining ownership of model selection, queues, retries, and persistence. --- .../generate-review-walkthrough.test.ts | 153 +++++++++ core/index.ts | 7 + core/lib/generate-review-walkthrough.ts | 301 ++++++++++++++++++ core/walkthrough-authoring.ts | 8 + 4 files changed, 469 insertions(+) create mode 100644 core/__tests__/generate-review-walkthrough.test.ts create mode 100644 core/lib/generate-review-walkthrough.ts diff --git a/core/__tests__/generate-review-walkthrough.test.ts b/core/__tests__/generate-review-walkthrough.test.ts new file mode 100644 index 00000000..c38098cd --- /dev/null +++ b/core/__tests__/generate-review-walkthrough.test.ts @@ -0,0 +1,153 @@ +import { expect, test, vi } from 'vite-plus/test'; +import { generateReviewWalkthrough } from '../lib/generate-review-walkthrough.ts'; +import type { RepositoryState, ReviewCommitEvolution } from '../types.ts'; +import { createChangedFile } from './helpers/fixtures.ts'; + +const baseState = { + branch: 'feature', + files: [createChangedFile('src/app.ts')], + generatedAt: 1, + launchPath: '/repo', + root: '/repo', + source: { + number: 1, + provider: 'github', + type: 'pull-request', + url: 'https://github.com/nkzw-tech/codiff/pull/1', + }, +} satisfies RepositoryState; + +const draft = { + chapters: [ + { + blurb: 'Review the change.', + icon: 'gear', + id: 'c1', + stops: [ + { + hunkIds: ['h1'], + id: 's1', + importance: 'critical', + prose: 'Check this.', + title: 'Main change', + }, + ], + title: 'Core', + }, + ], + focus: 'Focus', + kind: 'narrative', + title: 'Walkthrough', + version: 4, +}; + +test('generateReviewWalkthrough whole-diff path prompts once and normalizes', async () => { + const runModel = vi.fn(async () => ({ draft })); + const result = await generateReviewWalkthrough({ + agent: 'codex', + runModel, + states: { whole: baseState }, + structure: 'whole-diff', + }); + expect(result.status).toBe('ready'); + if (result.status !== 'ready') { + return; + } + expect(runModel).toHaveBeenCalledTimes(1); + expect(result.walkthrough.kind).toBe('narrative'); + expect(result.plan.structure).toBe('whole-diff'); +}); + +test('generateReviewWalkthrough units path fans out and composes', async () => { + const evolution = { + recommendation: { + rationale: 'Multiple units changed.', + suggestedStructure: 'commit-by-commit', + }, + summary: { + absorbedIntoBase: 0, + added: 2, + ambiguous: 0, + pairingCoverage: 0, + removed: 0, + retained: 0, + reviewable: 2, + revised: 0, + rewrittenSamePatch: 0, + }, + units: [ + { + after: { + authoredAt: '2026-01-01T00:00:00.000Z', + authorName: 'Ada', + parentIds: [], + sha: 'a'.repeat(40), + shortSha: 'aaaaaaa', + subject: 'one', + }, + confidence: 'exact', + id: 'introduced:a', + kind: 'introduced', + order: 0, + reviewable: true, + }, + { + after: { + authoredAt: '2026-01-01T01:00:00.000Z', + authorName: 'Ada', + parentIds: [], + sha: 'b'.repeat(40), + shortSha: 'bbbbbbb', + subject: 'two', + }, + confidence: 'exact', + id: 'introduced:b', + kind: 'introduced', + order: 1, + reviewable: true, + }, + ], + } satisfies ReviewCommitEvolution; + + const runModel = vi.fn(async () => ({ draft })); + const unitState = { + ...baseState, + files: [createChangedFile('src/unit.ts')], + } satisfies RepositoryState; + + const result = await generateReviewWalkthrough({ + agent: 'codex', + evolution, + runModel, + states: { + byUnitId: { + 'introduced:a': unitState, + 'introduced:b': unitState, + }, + whole: baseState, + }, + structure: 'units', + }); + + expect(result.status).toBe('ready'); + if (result.status !== 'ready') { + return; + } + expect(runModel).toHaveBeenCalledTimes(2); + expect(result.plan.structure).toBe('units'); + expect(result.walkthrough.chapters.length).toBeGreaterThan(0); + expect(result.walkthrough.title).toContain('Commit-by-commit'); +}); + +test('generateReviewWalkthrough fails clearly without whole state', async () => { + const result = await generateReviewWalkthrough({ + agent: 'codex', + runModel: async () => ({ draft }), + states: {}, + structure: 'whole-diff', + }); + expect(result).toEqual({ + reason: 'Whole-diff walkthrough generation requires a whole RepositoryState.', + status: 'failed', + }); +}); diff --git a/core/index.ts b/core/index.ts index e1f8bd16..f84f04a1 100644 --- a/core/index.ts +++ b/core/index.ts @@ -38,6 +38,13 @@ export { type VersionCommitSummary, type VersionRebaseDriverCommit, } from './lib/commit-stack-evolution.ts'; +export { + generateReviewWalkthrough, + type GenerateReviewWalkthroughInput, + type GenerateReviewWalkthroughResult, + type ReviewWalkthroughModelResult, + type ReviewWalkthroughRunModel, +} from './lib/generate-review-walkthrough.ts'; export { parsePlanShareManifest, parsePlanShareUpload, diff --git a/core/lib/generate-review-walkthrough.ts b/core/lib/generate-review-walkthrough.ts new file mode 100644 index 00000000..1cbc47d3 --- /dev/null +++ b/core/lib/generate-review-walkthrough.ts @@ -0,0 +1,301 @@ +import type { + NarrativeWalkthrough, + RepositoryState, + ReviewCommitEvolution, + ReviewPlan, + ReviewUnit, +} from '../types.ts'; +import { resolveReviewPlan, reviewableUnits } from './review-history.ts'; +import { + buildWalkthroughPrompt, + composeUnitWalkthroughs, + normalizeWalkthroughDraft, + type UnitWalkthroughEntry, + type WalkthroughPromptOptions, +} from './walkthrough-authoring.ts'; + +export type ReviewWalkthroughModelResult = { + /** Raw model JSON/text payload to normalize. */ + draft: unknown; +}; + +export type ReviewWalkthroughRunModel = (input: { + agent: NarrativeWalkthrough['agent']; + prompt: string; + state: RepositoryState; +}) => Promise; + +export type GenerateReviewWalkthroughInput = { + agent: NarrativeWalkthrough['agent']; + /** + * Optional evolution used when `plan` is omitted and unit generation is desired. + * Hosts typically pass the projected Core evolution for the active compare. + */ + evolution?: ReviewCommitEvolution | null; + /** + * Optional explicit plan. When omitted, resolved from evolution recommendation + * (units if recommended and unit states exist; otherwise whole-diff). + */ + plan?: ReviewPlan | null; + /** Extra prompt options applied to every generation call. */ + promptOptions?: WalkthroughPromptOptions; + /** Host-provided model runner (local agent CLI, Think, etc.). */ + runModel: ReviewWalkthroughRunModel; + /** + * Whole-diff state and/or per-unit states. + * - whole-diff plan: provide `whole` + * - units plan: provide `byUnitId` for each reviewable unit (missing units are skipped) + */ + states: { + byUnitId?: Readonly>; + whole?: RepositoryState; + }; + /** Force whole-diff even when a units plan is available. */ + structure?: 'auto' | 'commit-by-commit' | 'whole-diff' | 'units'; +}; + +export type GenerateReviewWalkthroughResult = + | { + plan: ReviewPlan; + status: 'ready'; + walkthrough: NarrativeWalkthrough; + } + | { + reason: string; + status: 'failed'; + }; + +const toEvolutionKind = ( + kind: ReviewUnit['kind'], +): 'likely-revised' | 'added' | 'removed' | 'ambiguous' => { + switch (kind) { + case 'introduced': + return 'added'; + case 'removed': + return 'removed'; + case 'revised': + return 'likely-revised'; + default: + return 'ambiguous'; + } +}; + +const unitAfter = (unit: ReviewUnit) => + unit.kind === 'commit' + ? unit.commit + : unit.kind === 'introduced' || unit.kind === 'revised' || unit.kind === 'ambiguous' + ? unit.after + : undefined; + +const unitBefore = (unit: ReviewUnit) => + unit.kind === 'removed' || unit.kind === 'revised' || unit.kind === 'ambiguous' + ? unit.before + : undefined; + +const toPromptOptionsForUnit = ( + unit: ReviewUnit, + range: { fromLabel: string; toLabel: string }, + base: WalkthroughPromptOptions | undefined, +): WalkthroughPromptOptions => { + const after = unitAfter(unit); + const before = unitBefore(unit); + return { + ...base, + versionCommitContext: { + evolutionKind: toEvolutionKind(unit.kind), + kind: 'version-commit', + range, + unitId: unit.id, + ...(after ? { after: { shortSha: after.shortSha, subject: after.subject } } : {}), + ...(before ? { before: { shortSha: before.shortSha, subject: before.subject } } : {}), + ...('rebaseDrivers' in unit && unit.rebaseDrivers + ? { + rebaseDrivers: unit.rebaseDrivers.map((driver) => ({ + authorName: driver.authorName, + overlappingPaths: driver.overlappingPaths ?? [], + shortSha: driver.shortSha, + subject: driver.subject, + })), + } + : {}), + }, + versionCompareRange: { + fromLabel: range.fromLabel, + structure: 'commit-by-commit', + toLabel: range.toLabel, + }, + }; +}; + +/** + * Host-agnostic walkthrough generation orchestrator. + * + * Owns plan resolution + whole-diff vs unit fanout + composition. + * Does **not** own model IDs, process execution, caching, or auth — hosts inject + * `runModel` for that. + * + * ## Local vs Web + * - Local: `runModel` shells out to codex/claude/opencode/pi and returns parsed JSON. + * - Web (later): `runModel` calls Think/AI Gateway; Durable Object fanout can still + * pre-build per-unit states and call this helper once per unit or once for whole-diff. + * + * Prompt construction, draft normalization, and unit composition stay inside + * `walkthrough-authoring` so both hosts cannot drift. + */ +export async function generateReviewWalkthrough( + input: GenerateReviewWalkthroughInput, +): Promise { + const plan = + input.plan ?? + resolveReviewPlan({ + recommendation: input.evolution?.recommendation, + structure: input.structure === 'commit-by-commit' ? 'units' : input.structure, + units: input.evolution?.units, + }); + + if (plan.structure === 'whole-diff') { + const state = input.states.whole; + if (!state) { + return { + reason: 'Whole-diff walkthrough generation requires a whole RepositoryState.', + status: 'failed', + }; + } + if (state.files.length === 0) { + return { + reason: 'No changed files are available for walkthrough generation.', + status: 'failed', + }; + } + try { + const prompt = buildWalkthroughPrompt(state, input.promptOptions); + const { draft } = await input.runModel({ + agent: input.agent, + prompt, + state, + }); + const walkthrough = normalizeWalkthroughDraft(draft, state, input.agent); + return { plan, status: 'ready', walkthrough }; + } catch (error: unknown) { + return { + reason: error instanceof Error ? error.message : String(error), + status: 'failed', + }; + } + } + + const units = plan.units?.length + ? plan.units + : input.evolution + ? reviewableUnits(input.evolution.units) + : []; + if (units.length === 0) { + return { + reason: 'Unit walkthrough generation requires reviewable evolution units.', + status: 'failed', + }; + } + + const wholeState = input.states.whole; + if (!wholeState) { + return { + reason: 'Unit composition requires the parent whole RepositoryState.', + status: 'failed', + }; + } + + const range = { + fromLabel: input.promptOptions?.versionCompareRange?.fromLabel ?? 'before', + toLabel: input.promptOptions?.versionCompareRange?.toLabel ?? 'after', + }; + + const entries: Array = []; + const failures: Array = []; + + for (const unit of units) { + const unitState = input.states.byUnitId?.[unit.id]; + if (!unitState || unitState.files.length === 0) { + failures.push(`${unit.id}: missing unit diff state`); + continue; + } + try { + const prompt = buildWalkthroughPrompt( + unitState, + toPromptOptionsForUnit(unit, range, input.promptOptions), + ); + const { draft } = await input.runModel({ + agent: input.agent, + prompt, + state: unitState, + }); + const walkthrough = normalizeWalkthroughDraft(draft, unitState, input.agent); + const after = unitAfter(unit); + const before = unitBefore(unit); + const context: UnitWalkthroughEntry['context'] = { + kind: 'version-commit', + range, + unitId: unit.id, + ...(after + ? { + after: { + sha: after.sha, + shortSha: after.shortSha, + subject: after.subject, + ...(after.webUrl ? { webUrl: after.webUrl } : {}), + }, + } + : {}), + ...(before + ? { + before: { + sha: before.sha, + shortSha: before.shortSha, + subject: before.subject, + ...(before.webUrl ? { webUrl: before.webUrl } : {}), + }, + } + : {}), + ...('rebaseDrivers' in unit && unit.rebaseDrivers + ? { + rebaseDrivers: unit.rebaseDrivers.map((driver) => ({ + ...(driver.authoredAt ? { authoredAt: driver.authoredAt } : {}), + ...(driver.authorName ? { authorName: driver.authorName } : {}), + ...(driver.overlappingPaths + ? { overlappingPaths: [...driver.overlappingPaths] } + : {}), + ...(driver.sha ? { sha: driver.sha } : {}), + shortSha: driver.shortSha, + subject: driver.subject, + ...(driver.webUrl ? { webUrl: driver.webUrl } : {}), + })), + } + : {}), + }; + entries.push({ + context, + state: unitState, + walkthrough, + }); + } catch (error: unknown) { + failures.push(`${unit.id}: ${error instanceof Error ? error.message : String(error)}`); + } + } + + if (entries.length === 0) { + return { + reason: + failures.length > 0 + ? `All unit walkthroughs failed. ${failures.join('; ')}` + : 'No unit walkthroughs were generated.', + status: 'failed', + }; + } + + const walkthrough = composeUnitWalkthroughs({ + agent: input.agent, + entries, + state: wholeState, + }); + + return { plan, status: 'ready', walkthrough }; +} diff --git a/core/walkthrough-authoring.ts b/core/walkthrough-authoring.ts index e97c4876..b18b36cb 100644 --- a/core/walkthrough-authoring.ts +++ b/core/walkthrough-authoring.ts @@ -27,3 +27,11 @@ export { type WalkthroughPromptOptions, type WalkthroughReviewStrategy, } from './lib/walkthrough-authoring.ts'; + +export { + generateReviewWalkthrough, + type GenerateReviewWalkthroughInput, + type GenerateReviewWalkthroughResult, + type ReviewWalkthroughModelResult, + type ReviewWalkthroughRunModel, +} from './lib/generate-review-walkthrough.ts'; From 6967d11160050979c4c45fd89fcc42ededacae02 Mon Sep 17 00:00:00 2001 From: Matt Alonso Date: Mon, 20 Jul 2026 18:05:17 -0500 Subject: [PATCH 8/8] Centralize provider-neutral evolution projection Move commit-evolution projection and shared unit helpers into Core so provider adapters produce the same review-history model. Update the GitLab adapter to consume that shared projection instead of owning provider-specific copies, preparing the API used by Codiff Web and later GitHub integration. --- core/index.ts | 23 +---- core/lib/commit-stack-evolution.ts | 152 ++++++++++++++++++++++++++++- gitlab/src/history.ts | 135 +------------------------ 3 files changed, 153 insertions(+), 157 deletions(-) diff --git a/core/index.ts b/core/index.ts index f84f04a1..f3066731 100644 --- a/core/index.ts +++ b/core/index.ts @@ -22,6 +22,8 @@ export { attributeRebaseDrivers, createCommitPatchSignature, matchVersionCommitStacks, + projectCommitEvolution, + projectEvolutionUnit, recommendVersionWalkthroughStructure, scoreBaseCommitAsRebaseDriver, toVersionCommitSummary, @@ -105,24 +107,3 @@ export type { WalkthroughShareManifestV1, WalkthroughHunk, } from './types.ts'; -export { - formatVersionElapsedDuration, - MergeRequestReviewApp, - ReadOnlyGeneralCommentCard, - ReviewSurface, - type MergeRequestCommitListEntry, - type MergeRequestReviewAppProps, - type MergeRequestReviewMode, - type MergeRequestReviewStrategySummary, - type MergeRequestVersionBaseMovement, - type MergeRequestVersionBaseMovementCommit, - type MergeRequestVersionCommitEvolution, - type MergeRequestVersionCommitEvolutionUnit, - type MergeRequestVersionCommitSummary, - type MergeRequestVersionCompareCommentAssociation, - type MergeRequestVersionCompareSummary, - type MergeRequestVersionCompareView, - type MergeRequestVersionOption, - type MergeRequestVersionRebaseDriverCommit, - type MergeRequestWalkthroughStatus, -} from './SharedWalkthroughApp.tsx'; diff --git a/core/lib/commit-stack-evolution.ts b/core/lib/commit-stack-evolution.ts index f2376202..bb621731 100644 --- a/core/lib/commit-stack-evolution.ts +++ b/core/lib/commit-stack-evolution.ts @@ -4,7 +4,12 @@ * Used by GitLab MR versions and GitHub force-push head comparisons so both * forges share one pairing algorithm. */ -import type { ChangedFile } from '../types.ts'; +import type { + ChangedFile, + ReviewCommitEvolution, + ReviewCommitSummary, + ReviewEvolutionUnit, +} from '../types.ts'; import { sha256 } from './crypto.ts'; export const versionCommitSignatureAlgorithmVersion = 'patch-signature-v1'; @@ -79,17 +84,17 @@ export type VersionCommitEvolutionUnit = { /** Minimal endpoint identity needed for evolution range bookkeeping. */ export type DiffEndpointRef = { baseSha: string; + createdAt?: string; headSha: string; id?: string; label?: string; startSha?: string; - createdAt?: string; }; export type CommitStackEvolutionRange = { from: DiffEndpointRef; - to: DiffEndpointRef; paths?: ReadonlyArray; + to: DiffEndpointRef; }; export type CommitStackEvolution = { @@ -828,3 +833,144 @@ export const attributeRebaseDrivers = ({ .slice(0, limit) .map(({ score: _score, ...commit }) => commit); }; + +type ProjectableCommit = { + authoredAt: string; + authorName: string; + diffStat?: { additions: number; deletions: number; filesChanged: number }; + parentIds?: ReadonlyArray; + sha: string; + shortSha: string; + subject: string; + webUrl?: string; +}; + +const projectCommitSummary = ( + commit: ProjectableCommit | undefined, +): ReviewCommitSummary | undefined => { + if (!commit) { + return undefined; + } + return { + authoredAt: commit.authoredAt, + authorName: commit.authorName, + parentIds: commit.parentIds ?? [], + sha: commit.sha, + shortSha: commit.shortSha, + subject: commit.subject, + webUrl: commit.webUrl, + ...(commit.diffStat ? { diffStat: commit.diffStat } : {}), + }; +}; + +/** + * Project forge-internal evolution units onto Core {@link ReviewEvolutionUnit}. + * Maps GitLab-ish kinds (`added` / `likely-revised`) onto host UI kinds + * (`introduced` / `revised`). + */ +export const projectEvolutionUnit = (unit: { + after?: ProjectableCommit; + baseCommit?: ProjectableCommit; + before?: ProjectableCommit; + confidence: 'exact' | 'high' | 'unmatched'; + id: string; + kind: string; + matchReasons?: ReadonlyArray; + matchScore?: number; + order: number; + rebaseDrivers?: ReadonlyArray<{ + authoredAt: string; + authorName: string; + overlappingPaths: ReadonlyArray; + sha: string; + shortSha: string; + subject: string; + webUrl?: string; + }>; + reviewable: boolean; +}): ReviewEvolutionUnit => { + const common = { + confidence: unit.confidence, + id: unit.id, + order: unit.order, + ...(unit.matchReasons ? { matchReasons: unit.matchReasons } : {}), + ...(unit.matchScore != null ? { matchScore: unit.matchScore } : {}), + } as const; + if (unit.kind === 'added' || unit.kind === 'introduced') { + return { + ...common, + after: projectCommitSummary(unit.after)!, + kind: 'introduced', + reviewable: true, + }; + } + if (unit.kind === 'removed') { + return { + ...common, + before: projectCommitSummary(unit.before)!, + kind: 'removed', + reviewable: true, + }; + } + if (unit.kind === 'likely-revised' || unit.kind === 'revised') { + return { + ...common, + after: projectCommitSummary(unit.after)!, + before: projectCommitSummary(unit.before)!, + kind: 'revised', + reviewable: true, + ...(unit.rebaseDrivers + ? { + rebaseDrivers: unit.rebaseDrivers.map((driver) => ({ + authoredAt: driver.authoredAt, + authorName: driver.authorName, + overlappingPaths: driver.overlappingPaths, + sha: driver.sha, + shortSha: driver.shortSha, + subject: driver.subject, + webUrl: driver.webUrl, + })), + } + : {}), + }; + } + if (unit.kind === 'ambiguous') { + return { + ...common, + kind: 'ambiguous', + reviewable: true, + ...(projectCommitSummary(unit.after) ? { after: projectCommitSummary(unit.after) } : {}), + ...(projectCommitSummary(unit.before) ? { before: projectCommitSummary(unit.before) } : {}), + }; + } + if (unit.kind === 'absorbed-into-base') { + return { + ...common, + kind: 'absorbed-into-base', + reviewable: false, + ...(projectCommitSummary(unit.after) ? { after: projectCommitSummary(unit.after) } : {}), + ...(projectCommitSummary(unit.baseCommit) + ? { baseCommit: projectCommitSummary(unit.baseCommit) } + : {}), + ...(projectCommitSummary(unit.before) ? { before: projectCommitSummary(unit.before) } : {}), + }; + } + return { + ...common, + kind: unit.kind === 'rewritten-same-patch' ? 'rewritten-same-patch' : 'retained', + reviewable: false, + ...(projectCommitSummary(unit.after) ? { after: projectCommitSummary(unit.after) } : {}), + ...(projectCommitSummary(unit.before) ? { before: projectCommitSummary(unit.before) } : {}), + }; +}; + +/** Project forge-internal stack evolution onto Core {@link ReviewCommitEvolution}. */ +export const projectCommitEvolution = (evolution: CommitStackEvolution): ReviewCommitEvolution => ({ + recommendation: { + rationale: evolution.recommendation.reason, + suggestedStructure: evolution.recommendation.structure, + }, + summary: evolution.summary, + units: evolution.units.map(projectEvolutionUnit), + ...(evolution.warnings ? { warnings: evolution.warnings } : {}), +}); diff --git a/gitlab/src/history.ts b/gitlab/src/history.ts index ce84c515..8d99dfd0 100644 --- a/gitlab/src/history.ts +++ b/gitlab/src/history.ts @@ -6,8 +6,6 @@ import type { DiffComparisonAnalysis, DiffComparisonView, ReviewCommitEvolution, - ReviewCommitSummary, - ReviewEvolutionUnit, ReviewPlan, ReviewVersionOption, } from '@nkzw/codiff-core/types'; @@ -16,6 +14,7 @@ import { diffComparison, diffComparisonView, diffRange, + projectCommitEvolution, resolveReviewPlan, reviewVersionOption, revisionRef, @@ -1569,137 +1568,7 @@ export const projectMergeRequestVersionRef = ( ...(version.previousNumber != null ? { previousNumber: version.previousNumber } : {}), }); -const projectCommitSummary = (commit: Algorithmish | undefined): ReviewCommitSummary | undefined => { - if (!commit) return undefined; - return { - authorName: commit.authorName, - authoredAt: commit.authoredAt, - parentIds: commit.parentIds ?? [], - sha: commit.sha, - shortSha: commit.shortSha, - subject: commit.subject, - webUrl: commit.webUrl, - ...(commit.diffStat ? { diffStat: commit.diffStat } : {}), - }; -}; - -type Algorithmish = { - authorName: string; - authoredAt: string; - diffStat?: { additions: number; deletions: number; filesChanged: number }; - parentIds?: ReadonlyArray; - sha: string; - shortSha: string; - subject: string; - webUrl?: string; -}; - -export const projectEvolutionUnit = (unit: { - after?: Algorithmish; - baseCommit?: Algorithmish; - before?: Algorithmish; - confidence: 'exact' | 'high' | 'unmatched'; - id: string; - kind: string; - matchReasons?: ReadonlyArray; - matchScore?: number; - order: number; - rebaseDrivers?: ReadonlyArray<{ - authorName: string; - authoredAt: string; - overlappingPaths: ReadonlyArray; - sha: string; - shortSha: string; - subject: string; - webUrl?: string; - }>; - reviewable: boolean; -}): ReviewEvolutionUnit => { - const common = { - confidence: unit.confidence, - id: unit.id, - order: unit.order, - ...(unit.matchReasons ? { matchReasons: unit.matchReasons } : {}), - ...(unit.matchScore != null ? { matchScore: unit.matchScore } : {}), - } as const; - if (unit.kind === 'added' || unit.kind === 'introduced') { - return { - ...common, - after: projectCommitSummary(unit.after)!, - kind: 'introduced', - reviewable: true, - }; - } - if (unit.kind === 'removed') { - return { - ...common, - before: projectCommitSummary(unit.before)!, - kind: 'removed', - reviewable: true, - }; - } - if (unit.kind === 'likely-revised' || unit.kind === 'revised') { - return { - ...common, - after: projectCommitSummary(unit.after)!, - before: projectCommitSummary(unit.before)!, - kind: 'revised', - reviewable: true, - ...(unit.rebaseDrivers - ? { - rebaseDrivers: unit.rebaseDrivers.map((driver) => ({ - authorName: driver.authorName, - authoredAt: driver.authoredAt, - overlappingPaths: driver.overlappingPaths, - sha: driver.sha, - shortSha: driver.shortSha, - subject: driver.subject, - webUrl: driver.webUrl, - })), - } - : {}), - }; - } - if (unit.kind === 'ambiguous') { - return { - ...common, - kind: 'ambiguous', - reviewable: true, - ...(projectCommitSummary(unit.after) ? { after: projectCommitSummary(unit.after) } : {}), - ...(projectCommitSummary(unit.before) ? { before: projectCommitSummary(unit.before) } : {}), - }; - } - if (unit.kind === 'absorbed-into-base') { - return { - ...common, - kind: 'absorbed-into-base', - reviewable: false, - ...(projectCommitSummary(unit.after) ? { after: projectCommitSummary(unit.after) } : {}), - ...(projectCommitSummary(unit.baseCommit) ? { baseCommit: projectCommitSummary(unit.baseCommit) } : {}), - ...(projectCommitSummary(unit.before) ? { before: projectCommitSummary(unit.before) } : {}), - }; - } - // retained / rewritten-same-patch - return { - ...common, - kind: unit.kind === 'rewritten-same-patch' ? 'rewritten-same-patch' : 'retained', - reviewable: false, - ...(projectCommitSummary(unit.after) ? { after: projectCommitSummary(unit.after) } : {}), - ...(projectCommitSummary(unit.before) ? { before: projectCommitSummary(unit.before) } : {}), - }; -}; - -export const projectCommitEvolution = ( - evolution: MergeRequestVersionCommitEvolution, -): ReviewCommitEvolution => ({ - recommendation: { - rationale: evolution.recommendation.reason, - suggestedStructure: evolution.recommendation.structure, - }, - summary: evolution.summary, - units: evolution.units.map(projectEvolutionUnit), - ...(evolution.warnings ? { warnings: evolution.warnings } : {}), -}); +export { projectCommitEvolution, projectEvolutionUnit } from '@nkzw/codiff-core'; export const projectVersionCompare = ( compare: MergeRequestVersionCompare,