From 8de0becd39ed6885a703e4c001e09acbc26c32ce Mon Sep 17 00:00:00 2001 From: Will Date: Sun, 1 Nov 2026 20:14:56 -0600 Subject: [PATCH 1/4] feat: add diff-so-fancy-style diff readability --- README.md | 7 + apps/web/src/components/CodeDiff.test.tsx | 40 +- apps/web/src/components/CodeDiff.tsx | 77 +- apps/web/src/diffPresentation.test.ts | 83 +++ apps/web/src/diffPresentation.ts | 105 +++ apps/web/src/fixture.test.ts | 10 + apps/web/src/fixture.ts | 126 ++-- apps/web/src/styles.css | 7 + .../diff-so-fancy-integration/plan.md | 672 ++++++++++++++++++ 9 files changed, 1046 insertions(+), 81 deletions(-) create mode 100644 apps/web/src/diffPresentation.test.ts create mode 100644 apps/web/src/diffPresentation.ts create mode 100644 docs/projects/diff-so-fancy-integration/plan.md diff --git a/README.md b/README.md index b9a242a..c5e024a 100644 --- a/README.md +++ b/README.md @@ -147,6 +147,13 @@ npx wingdiff https://gitlab.com/group/repository/-/merge_requests/123 Wingdiff reads metadata through `gh` or `glab`, fetches missing objects into private `refs/wingdiff/...` references, and builds the full diff from the exact pinned merge-base/head pair the host compares. When no matching checkout is available, it keeps a private bare repository cache beside its application data. Update reviews use the exact reviewed-head/current-head pair. It does not check out the change, modify the worktree or index, or execute change-request code. Acquired metadata, evidence, generated tours, and review checkpoints are stored in a local SQLite database. Set `WINGDIFF_DATA_DIR` to choose its location. +The built-in diff viewer takes a lightweight cue from +[diff-so-fancy](https://github.com/so-fancy/diff-so-fancy): paired changed +lines emphasize only the intraline span that differs, and hunk headers are +reduced to a readable `@ path:line @ context` form. Wingdiff does not run the +terminal renderer or consume ANSI output; Git's structured diff remains the +authoritative source for line numbers, fingerprints, and review anchors. + From the real-PR Summary, choose a configured model and select **Generate guided review**. The model organizes the validated diff into semantic stops, ranks concrete findings by severity, and references opaque evidence anchors. Wingdiff rejects the result unless every changed file is covered and every claim or finding resolves to an exact known anchor. Generated tours are pinned to the acquired head SHA and can be resumed locally. Before generation, Wingdiff shows the exact model context, provider transport, changed files, repository instructions, exclusions, size, and content fingerprint. The immutable manifest, fingerprint, and evidence-anchor snapshot are stored with the tour; a changed manifest invalidates the resumable tour. Model-facing evidence references use compact request-local aliases constrained to the supplied anchor set, then map back to immutable evidence fingerprints before validation and storage. Potentially sensitive, generated, and vendor files are marked; common key and environment-file patterns are excluded by default. Exclusions are editable local globs. `AGENTS.md`, `CONTRIBUTING.md`, `.github/CONTRIBUTING.md`, and `.gitlab/CONTRIBUTING.md` are included when present, capped at 20,000 characters each. Context over 750,000 characters is blocked until reduced. diff --git a/apps/web/src/components/CodeDiff.test.tsx b/apps/web/src/components/CodeDiff.test.tsx index b206d9e..1e0dc07 100644 --- a/apps/web/src/components/CodeDiff.test.tsx +++ b/apps/web/src/components/CodeDiff.test.tsx @@ -15,9 +15,9 @@ describe("CodeDiff", () => { endLine: 3, lines: [ { kind: "header", content: "@@ -1,3 +1,3 @@" }, - { kind: "deletion", oldLine: 2, content: "- snap_count=old", emphasized: true }, - { kind: "addition", newLine: 2, content: "+ snap_count=new", emphasized: true }, - { kind: "context", oldLine: 3, newLine: 3, content: " exit 0" }, + { kind: "deletion", oldLine: 2, content: "snap_count=30", emphasized: true }, + { kind: "addition", newLine: 2, content: "snap_count=60", emphasized: true }, + { kind: "context", oldLine: 3, newLine: 3, content: "exit 0" }, ], }; @@ -29,11 +29,41 @@ describe("CodeDiff", () => { expect(html).toContain("diff-line--context"); }); - it("defines strong deletion tokens and emphasized deletion CSS rules in styles.css", () => { + it("renders the simplified hunk header and intraline change spans", () => { + const html = renderToStaticMarkup(); + + expect(html).toContain("@ .github/workflows/ci.yml:1 @"); + expect(html).not.toContain("@@ -1,3 +1,3 @@"); + expect(html).toContain("diff-line__change"); + expect(html).toContain("syntax-number"); + }); + + it("keeps syntax classes on changed string content", () => { + const evidence: EvidenceBlock = { + ...sampleEvidence, + path: "src/labels.ts", + lines: [ + { kind: "deletion", oldLine: 3, content: "const label = \"café old\";" }, + { kind: "addition", newLine: 3, content: "const label = \"café new\";" }, + ], + }; + + const html = renderToStaticMarkup(); + + expect(html).toContain("diff-line__change syntax-string"); + expect(html).toContain("old"); + expect(html).toContain("new"); + }); + + it("defines strong deletion tokens and intraline emphasis CSS rules", () => { const cssPath = resolve(__dirname, "../styles.css"); const css = readFileSync(cssPath, "utf-8"); - expect(css).toContain("--code-delete-strong:"); + expect(css).toContain("--code-add-inline:"); + expect(css).toContain("--code-delete-inline:"); + expect(css).toContain(".diff-line__change"); + expect(css).toContain(".diff-line--addition .diff-line__change"); + expect(css).toContain(".diff-line--deletion .diff-line__change"); expect(css).toContain(".diff-line--deletion.is-emphasized"); expect(css).toContain(".diff-line--addition.is-emphasized"); }); diff --git a/apps/web/src/components/CodeDiff.tsx b/apps/web/src/components/CodeDiff.tsx index 8f7d582..cfa1631 100644 --- a/apps/web/src/components/CodeDiff.tsx +++ b/apps/web/src/components/CodeDiff.tsx @@ -1,5 +1,6 @@ -import { Fragment, memo } from "react"; -import type { DiffLine, EvidenceBlock } from "../types"; +import { memo, useMemo } from "react"; +import { presentDiffLines, type DiffLineSegments } from "../diffPresentation"; +import type { EvidenceBlock } from "../types"; import { Icon } from "./Icon"; interface Selection { @@ -29,6 +30,10 @@ export const CodeDiff = memo(function CodeDiff({ const selectedCount = selection?.evidenceId === evidence.id ? Math.abs(selection.end - selection.start) + 1 : 0; + const lines = useMemo( + () => presentDiffLines(evidence.lines, evidence.path), + [evidence.lines, evidence.path], + ); return (
@@ -47,7 +52,7 @@ export const CodeDiff = memo(function CodeDiff({
- {evidence.lines.map((line, index) => { + {lines.map((line, index) => { const side = line.kind === "deletion" ? "LEFT" : "RIGHT"; const lineNumber = side === "RIGHT" ? line.newLine : line.oldLine; const selected = Boolean( @@ -79,7 +84,7 @@ export const CodeDiff = memo(function CodeDiff({ - {highlight(line)} + {highlight(line.content, line.segments)} )}
@@ -116,15 +121,61 @@ function directoryName(path: string) { return parts.length ? `${parts.join("/")}/` : ""; } -function highlight(line: DiffLine) { - const content = line.content.replace(/^\+ /, "").replace(/^- /, ""); +function highlight(content: string, segments?: DiffLineSegments) { + const changeStart = segments?.before.length ?? -1; + const changeEnd = segments ? changeStart + segments.changed.length : -1; + return syntaxTokens(content).flatMap((token, tokenIndex) => ( + splitToken(content, token, changeStart, changeEnd).map((part, partIndex) => ( + + {part.text} + + )) + )); +} + +interface SyntaxToken { + start: number; + end: number; + className: string; +} + +function syntaxTokens(content: string): SyntaxToken[] { const pattern = /(\"[^\"]*\"|'[^']*'|`[^`]*`|\b(?:async|await|const|return|if|else|new|throw|class|private|export|function|true|false|null|undefined)\b|\b\d+\b)/g; - return content.split(pattern).map((token, index) => { - if (!token) return null; - let className = ""; - if (/^[\"'`]/.test(token)) className = "syntax-string"; - else if (/^\d+$/.test(token)) className = "syntax-number"; - else if (/^(async|await|const|return|if|else|new|throw|class|private|export|function|true|false|null|undefined)$/.test(token)) className = "syntax-keyword"; - return {token}; + const tokens: SyntaxToken[] = []; + let cursor = 0; + for (const match of content.matchAll(pattern)) { + const start = match.index; + if (start > cursor) tokens.push({ start: cursor, end: start, className: "" }); + const text = match[0]; + tokens.push({ start, end: start + text.length, className: syntaxClass(text) }); + cursor = start + text.length; + } + if (cursor < content.length) tokens.push({ start: cursor, end: content.length, className: "" }); + return tokens; +} + +function syntaxClass(token: string): string { + if (/^[\"'`]/.test(token)) return "syntax-string"; + if (/^\d+$/.test(token)) return "syntax-number"; + return "syntax-keyword"; +} + +function splitToken(content: string, token: SyntaxToken, changeStart: number, changeEnd: number) { + const boundaries = [token.start, token.end]; + if (changeStart > token.start && changeStart < token.end) boundaries.push(changeStart); + if (changeEnd > token.start && changeEnd < token.end) boundaries.push(changeEnd); + boundaries.sort((left, right) => left - right); + + return boundaries.slice(0, -1).map((start, index) => { + const end = boundaries[index + 1]!; + return { + text: content.slice(start, end), + start, + end, + changed: changeStart >= 0 && start >= changeStart && end <= changeEnd, + }; }); } diff --git a/apps/web/src/diffPresentation.test.ts b/apps/web/src/diffPresentation.test.ts new file mode 100644 index 0000000..dbc1d23 --- /dev/null +++ b/apps/web/src/diffPresentation.test.ts @@ -0,0 +1,83 @@ +import { describe, expect, it } from "vitest"; +import type { DiffLine } from "./types"; +import { presentDiffLines } from "./diffPresentation"; + +describe("presentDiffLines", () => { + it("marks only the changed span in paired deletion and addition lines", () => { + const lines: DiffLine[] = [ + { kind: "deletion", oldLine: 7, content: "const timeout = 30;" }, + { kind: "addition", newLine: 7, content: "const timeout = 60;" }, + ]; + + const presented = presentDiffLines(lines, "src/timeouts.ts"); + + expect(presented[0]?.segments).toEqual({ before: "const timeout = ", changed: "30", after: ";" }); + expect(presented[1]?.segments).toEqual({ before: "const timeout = ", changed: "60", after: ";" }); + }); + + it("uses Unicode code points when finding the changed span", () => { + const lines: DiffLine[] = [ + { kind: "deletion", oldLine: 3, content: "const label = \"café old\";" }, + { kind: "addition", newLine: 3, content: "const label = \"café new\";" }, + ]; + + const presented = presentDiffLines(lines, "src/labels.ts"); + + expect(presented[0]?.segments).toEqual({ before: "const label = \"café ", changed: "old", after: "\";" }); + expect(presented[1]?.segments).toEqual({ before: "const label = \"café ", changed: "new", after: "\";" }); + }); + + it("leaves unbalanced replacement runs unsegmented", () => { + const lines: DiffLine[] = [ + { kind: "deletion", oldLine: 2, content: "one()" }, + { kind: "addition", newLine: 2, content: "uno()" }, + { kind: "addition", newLine: 3, content: "dos()" }, + ]; + + expect(presentDiffLines(lines, "src/count.ts").every((line) => line.segments === undefined)).toBe(true); + }); + + it("preserves source content that begins with a diff marker", () => { + const lines: DiffLine[] = [ + { kind: "addition", newLine: 4, content: "+ Markdown list item" }, + ]; + + const presented = presentDiffLines(lines, "README.md"); + + expect(presented[0]?.content).toBe("+ Markdown list item"); + expect(presented[0]?.segments).toBeUndefined(); + }); + + it("does not emphasize identical paired lines", () => { + const lines: DiffLine[] = [ + { kind: "deletion", oldLine: 4, content: "same" }, + { kind: "addition", newLine: 4, content: "same" }, + ]; + + expect(presentDiffLines(lines, "file.txt").every((line) => line.segments === undefined)).toBe(true); + }); + + it("simplifies standard hunk headers and preserves non-hunk headers", () => { + const presented = presentDiffLines([ + { kind: "header", content: "@@ -41,8 +41,18 @@ export async function login(input: LoginInput) {" }, + { kind: "header", content: "@@ -10,2 +0,0 @@" }, + { kind: "header", content: "Binary file changed" }, + ], "src/auth/login.ts"); + + expect(presented[0]?.content).toBe("@ src/auth/login.ts:41 @ export async function login(input: LoginInput) {"); + expect(presented[1]?.content).toBe("@ src/auth/login.ts:10 @"); + expect(presented[2]?.content).toBe("Binary file changed"); + }); + + it("does not mutate the supplied evidence lines", () => { + const lines: DiffLine[] = [ + { kind: "deletion", oldLine: 4, content: "old" }, + { kind: "addition", newLine: 4, content: "new" }, + ]; + const original = structuredClone(lines); + + presentDiffLines(lines, "file.txt"); + + expect(lines).toEqual(original); + }); +}); diff --git a/apps/web/src/diffPresentation.ts b/apps/web/src/diffPresentation.ts new file mode 100644 index 0000000..7751776 --- /dev/null +++ b/apps/web/src/diffPresentation.ts @@ -0,0 +1,105 @@ +import type { DiffLine } from "./types"; + +export interface DiffLineSegments { + before: string; + changed: string; + after: string; +} + +export interface PresentedDiffLine extends DiffLine { + segments?: DiffLineSegments; +} + +export function presentDiffLines(lines: DiffLine[], path: string): PresentedDiffLine[] { + const presented: PresentedDiffLine[] = lines.map((line) => ({ + ...line, + content: line.kind === "header" ? humanizeHunkHeader(line.content, path) : line.content, + })); + + let index = 0; + while (index < presented.length) { + if (presented[index]?.kind !== "deletion") { + index += 1; + continue; + } + + const deletionStart = index; + while (presented[index]?.kind === "deletion") index += 1; + const additionStart = index; + while (presented[index]?.kind === "addition") index += 1; + + const deletions = presented.slice(deletionStart, additionStart); + const additions = presented.slice(additionStart, index); + if (!additions.length || deletions.length !== additions.length) continue; + + for (let pairIndex = 0; pairIndex < deletions.length; pairIndex += 1) { + const deletion = deletions[pairIndex]!; + const addition = additions[pairIndex]!; + const pair = splitChangedSpan(deletion.content, addition.content); + if (!pair) continue; + deletion.segments = pair.deletion; + addition.segments = pair.addition; + } + } + + return presented; +} + +function humanizeHunkHeader(header: string, path: string): string { + const match = /^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@(.*)$/.exec(header); + if (!match) return header; + const newStart = Number(match[2]); + const lineNumber = newStart === 0 ? Number(match[1]) : newStart; + const suffix = match[3]!.trimEnd(); + return `@ ${path}:${lineNumber} @${suffix}`; +} + +function splitChangedSpan( + deletion: string, + addition: string, +): { deletion: DiffLineSegments; addition: DiffLineSegments } | undefined { + const deleted = Array.from(deletion); + const added = Array.from(addition); + let prefix = 0; + while (prefix < deleted.length && prefix < added.length && deleted[prefix] === added[prefix]) prefix += 1; + while ( + prefix > 0 && + isDigit(deleted[prefix - 1]!) && + isDigit(deleted[prefix]!) && + isDigit(added[prefix - 1]!) && + isDigit(added[prefix]!) + ) prefix -= 1; + + let suffix = 0; + while ( + suffix < deleted.length - prefix && + suffix < added.length - prefix && + deleted[deleted.length - suffix - 1] === added[added.length - suffix - 1] + ) { + suffix += 1; + } + while ( + suffix > 0 && + isDigit(deleted[deleted.length - suffix - 1]!) && + isDigit(deleted[deleted.length - suffix]!) && + isDigit(added[added.length - suffix - 1]!) && + isDigit(added[added.length - suffix]!) + ) suffix -= 1; + + const deletionSegments = segments(deleted, prefix, suffix); + const additionSegments = segments(added, prefix, suffix); + if (!deletionSegments.changed && !additionSegments.changed) return undefined; + return { deletion: deletionSegments, addition: additionSegments }; +} + +function segments(codepoints: string[], prefix: number, suffix: number): DiffLineSegments { + return { + before: codepoints.slice(0, prefix).join(""), + changed: codepoints.slice(prefix, codepoints.length - suffix).join(""), + after: codepoints.slice(codepoints.length - suffix).join(""), + }; +} + +function isDigit(codepoint: string): boolean { + return /^\d$/.test(codepoint); +} diff --git a/apps/web/src/fixture.test.ts b/apps/web/src/fixture.test.ts index 217828a..f245776 100644 --- a/apps/web/src/fixture.test.ts +++ b/apps/web/src/fixture.test.ts @@ -93,4 +93,14 @@ describe("guided review fixture", () => { } } }); + + it("stores changed-line content without unified-diff markers", () => { + const lines = [...tourStops, ...updateStops].flatMap((stop) => stop.evidence.flatMap((evidence) => evidence.lines)); + const markedLines = lines.filter((line) => ( + (line.kind === "addition" && /^[+](?: |$)/.test(line.content)) || + (line.kind === "deletion" && /^[-](?: |$)/.test(line.content)) + )); + + expect(markedLines).toEqual([]); + }); }); diff --git a/apps/web/src/fixture.ts b/apps/web/src/fixture.ts index f5d65ae..6bf172c 100644 --- a/apps/web/src/fixture.ts +++ b/apps/web/src/fixture.ts @@ -120,14 +120,14 @@ export const tourStops: TourStop[] = [ lines: [ { kind: "header", content: "@@ -41,8 +41,18 @@ export async function login(input: LoginInput) {" }, { kind: "context", oldLine: 41, newLine: 41, content: " const email = normalizeEmail(input.email);" }, - { kind: "addition", newLine: 42, content: "+ const subject = await opaqueKey(email);", emphasized: true }, - { kind: "addition", newLine: 43, content: "+ const decision = await attemptPolicy.check(subject);", emphasized: true }, - { kind: "addition", newLine: 44, content: "+ if (!decision.allowed) {", emphasized: true }, - { kind: "addition", newLine: 45, content: "+ throw new RateLimitError(decision.retryAfterMs);", emphasized: true }, - { kind: "addition", newLine: 46, content: "+ }", emphasized: true }, + { kind: "addition", newLine: 42, content: "const subject = await opaqueKey(email);", emphasized: true }, + { kind: "addition", newLine: 43, content: "const decision = await attemptPolicy.check(subject);", emphasized: true }, + { kind: "addition", newLine: 44, content: "if (!decision.allowed) {", emphasized: true }, + { kind: "addition", newLine: 45, content: " throw new RateLimitError(decision.retryAfterMs);", emphasized: true }, + { kind: "addition", newLine: 46, content: "}", emphasized: true }, { kind: "context", oldLine: 42, newLine: 48, content: " const user = await users.findByEmail(email);" }, { kind: "context", oldLine: 43, newLine: 49, content: " const valid = user && await passwords.verify(input.password, user.hash);" }, - { kind: "addition", newLine: 51, content: "+ await attemptPolicy.record(subject, Boolean(valid));" }, + { kind: "addition", newLine: 51, content: "await attemptPolicy.record(subject, Boolean(valid));" }, { kind: "context", oldLine: 44, newLine: 52, content: " if (!valid) throw new InvalidCredentialsError();" }, ], }, @@ -175,12 +175,12 @@ export const tourStops: TourStop[] = [ endLine: 29, lines: [ { kind: "header", content: "@@ -0,0 +18,12 @@ export class AttemptPolicy {" }, - { kind: "addition", newLine: 18, content: "+ private delayFor(failures: number): number {" }, - { kind: "addition", newLine: 19, content: "+ if (failures < 3) return 0;" }, - { kind: "addition", newLine: 20, content: "+ if (failures < 5) return seconds(1);" }, - { kind: "addition", newLine: 21, content: "+ if (failures < 8) return seconds(5);" }, - { kind: "addition", newLine: 22, content: "+ return seconds(30);" }, - { kind: "addition", newLine: 23, content: "+ }" }, + { kind: "addition", newLine: 18, content: "private delayFor(failures: number): number {" }, + { kind: "addition", newLine: 19, content: " if (failures < 3) return 0;" }, + { kind: "addition", newLine: 20, content: " if (failures < 5) return seconds(1);" }, + { kind: "addition", newLine: 21, content: " if (failures < 8) return seconds(5);" }, + { kind: "addition", newLine: 22, content: " return seconds(30);" }, + { kind: "addition", newLine: 23, content: "}" }, ], }, { @@ -192,12 +192,12 @@ export const tourStops: TourStop[] = [ endLine: 38, lines: [ { kind: "header", content: "@@ -0,0 +31,8 @@ export class AttemptPolicy {" }, - { kind: "addition", newLine: 31, content: "+ async record(subject: string, success: boolean) {" }, - { kind: "addition", newLine: 32, content: "+ if (success) {" }, - { kind: "addition", newLine: 33, content: "+ return this.store.clear(subject);" }, - { kind: "addition", newLine: 34, content: "+ }" }, - { kind: "addition", newLine: 36, content: "+ return this.store.increment(subject, this.windowMs);" }, - { kind: "addition", newLine: 37, content: "+ }" }, + { kind: "addition", newLine: 31, content: "async record(subject: string, success: boolean) {" }, + { kind: "addition", newLine: 32, content: " if (success) {" }, + { kind: "addition", newLine: 33, content: " return this.store.clear(subject);" }, + { kind: "addition", newLine: 34, content: " }" }, + { kind: "addition", newLine: 36, content: " return this.store.increment(subject, this.windowMs);" }, + { kind: "addition", newLine: 37, content: "}" }, ], }, ], @@ -244,20 +244,20 @@ export const tourStops: TourStop[] = [ endLine: 30, lines: [ { kind: "header", content: "@@ -0,0 +12,19 @@ export class RedisAttemptStore {" }, - { kind: "addition", newLine: 12, content: "+ const INCREMENT_WITH_TTL = `" }, - { kind: "addition", newLine: 13, content: "+ local next = redis.call('INCR', KEYS[1])", emphasized: true }, - { kind: "addition", newLine: 14, content: "+ if next == 1 then" }, - { kind: "addition", newLine: 15, content: "+ redis.call('PEXPIRE', KEYS[1], ARGV[1])", emphasized: true }, - { kind: "addition", newLine: 16, content: "+ end" }, - { kind: "addition", newLine: 17, content: "+ return next" }, - { kind: "addition", newLine: 18, content: "+ `;" }, - { kind: "addition", newLine: 22, content: "+ async increment(subject: string, ttlMs: number) {" }, - { kind: "addition", newLine: 23, content: "+ const next = await this.redis.eval(INCREMENT_WITH_TTL, {" }, - { kind: "addition", newLine: 24, content: "+ keys: [this.key(subject)]," }, - { kind: "addition", newLine: 25, content: "+ arguments: [String(ttlMs)]," }, - { kind: "addition", newLine: 26, content: "+ });" }, - { kind: "addition", newLine: 27, content: "+ return Number(next);" }, - { kind: "addition", newLine: 28, content: "+ }" }, + { kind: "addition", newLine: 12, content: "const INCREMENT_WITH_TTL = `" }, + { kind: "addition", newLine: 13, content: " local next = redis.call('INCR', KEYS[1])", emphasized: true }, + { kind: "addition", newLine: 14, content: " if next == 1 then" }, + { kind: "addition", newLine: 15, content: " redis.call('PEXPIRE', KEYS[1], ARGV[1])", emphasized: true }, + { kind: "addition", newLine: 16, content: " end" }, + { kind: "addition", newLine: 17, content: " return next" }, + { kind: "addition", newLine: 18, content: "`;" }, + { kind: "addition", newLine: 22, content: "async increment(subject: string, ttlMs: number) {" }, + { kind: "addition", newLine: 23, content: " const next = await this.redis.eval(INCREMENT_WITH_TTL, {" }, + { kind: "addition", newLine: 24, content: " keys: [this.key(subject)]," }, + { kind: "addition", newLine: 25, content: " arguments: [String(ttlMs)]," }, + { kind: "addition", newLine: 26, content: " });" }, + { kind: "addition", newLine: 27, content: " return Number(next);" }, + { kind: "addition", newLine: 28, content: "}" }, ], }, ], @@ -304,13 +304,13 @@ export const tourStops: TourStop[] = [ endLine: 75, lines: [ { kind: "header", content: "@@ -64,6 +64,12 @@ export function errorResponse(error: Error) {" }, - { kind: "addition", newLine: 64, content: "+ if (error instanceof RateLimitError) {" }, - { kind: "addition", newLine: 65, content: "+ return {" }, - { kind: "addition", newLine: 66, content: "+ status: 429," }, - { kind: "addition", newLine: 67, content: "+ headers: { " + '"Retry-After": String(Math.ceil(error.retryAfterMs / 1000)) },' }, - { kind: "addition", newLine: 68, content: "+ body: { code: " + '"AUTH_THROTTLED", message: "Try again later" },' }, - { kind: "addition", newLine: 69, content: "+ };" }, - { kind: "addition", newLine: 70, content: "+ }" }, + { kind: "addition", newLine: 64, content: "if (error instanceof RateLimitError) {" }, + { kind: "addition", newLine: 65, content: " return {" }, + { kind: "addition", newLine: 66, content: " status: 429," }, + { kind: "addition", newLine: 67, content: " headers: { " + '"Retry-After": String(Math.ceil(error.retryAfterMs / 1000)) },' }, + { kind: "addition", newLine: 68, content: " body: { code: " + '"AUTH_THROTTLED", message: "Try again later" },' }, + { kind: "addition", newLine: 69, content: " };" }, + { kind: "addition", newLine: 70, content: "}" }, { kind: "context", oldLine: 64, newLine: 72, content: " if (error instanceof InvalidCredentialsError) {" }, { kind: "context", oldLine: 65, newLine: 73, content: " return { status: 401, body: { code: \"INVALID_CREDENTIALS\" } };" }, ], @@ -359,14 +359,14 @@ export const tourStops: TourStop[] = [ endLine: 56, lines: [ { kind: "header", content: "@@ -0,0 +44,13 @@ describe(\"RedisAttemptStore\", () => {" }, - { kind: "addition", newLine: 44, content: "+ it(\"preserves concurrent increments and expiry\", async () => {" }, - { kind: "addition", newLine: 45, content: "+ const subject = \"subject-1\";" }, - { kind: "addition", newLine: 46, content: "+ await Promise.all(", emphasized: true }, - { kind: "addition", newLine: 47, content: "+ Array.from({ length: 20 }, () => store.increment(subject, 60_000)),", emphasized: true }, - { kind: "addition", newLine: 48, content: "+ );" }, - { kind: "addition", newLine: 50, content: "+ expect(await redis.get(keyFor(subject))).toBe(\"20\");", emphasized: true }, - { kind: "addition", newLine: 51, content: "+ expect(await redis.pTTL(keyFor(subject))).toBeGreaterThan(0);" }, - { kind: "addition", newLine: 52, content: "+ });" }, + { kind: "addition", newLine: 44, content: "it(\"preserves concurrent increments and expiry\", async () => {" }, + { kind: "addition", newLine: 45, content: " const subject = \"subject-1\";" }, + { kind: "addition", newLine: 46, content: " await Promise.all(", emphasized: true }, + { kind: "addition", newLine: 47, content: " Array.from({ length: 20 }, () => store.increment(subject, 60_000)),", emphasized: true }, + { kind: "addition", newLine: 48, content: " );" }, + { kind: "addition", newLine: 50, content: " expect(await redis.get(keyFor(subject))).toBe(\"20\");", emphasized: true }, + { kind: "addition", newLine: 51, content: " expect(await redis.pTTL(keyFor(subject))).toBeGreaterThan(0);" }, + { kind: "addition", newLine: 52, content: "});" }, ], }, ], @@ -444,14 +444,14 @@ export const updateStops: TourStop[] = [ endLine: 28, lines: [ { kind: "header", content: "@@ -22,7 +22,7 @@ export class RedisAttemptStore {" }, - { kind: "deletion", oldLine: 22, content: "- const current = Number(await this.redis.get(key) ?? 0);", emphasized: true }, - { kind: "deletion", oldLine: 23, content: "- const next = current + 1;", emphasized: true }, - { kind: "deletion", oldLine: 24, content: "- await this.redis.set(key, String(next), { PX: ttlMs });", emphasized: true }, - { kind: "addition", newLine: 22, content: "+ const next = await this.redis.eval(INCREMENT_WITH_TTL, {", emphasized: true }, - { kind: "addition", newLine: 23, content: "+ keys: [this.key(subject)]," }, - { kind: "addition", newLine: 24, content: "+ arguments: [String(ttlMs)]," }, - { kind: "addition", newLine: 25, content: "+ });" }, - { kind: "addition", newLine: 26, content: "+ return Number(next);", emphasized: true }, + { kind: "deletion", oldLine: 22, content: "const current = Number(await this.redis.get(key) ?? 0);", emphasized: true }, + { kind: "deletion", oldLine: 23, content: "const next = current + 1;", emphasized: true }, + { kind: "deletion", oldLine: 24, content: "await this.redis.set(key, String(next), { PX: ttlMs });", emphasized: true }, + { kind: "addition", newLine: 22, content: "const next = await this.redis.eval(INCREMENT_WITH_TTL, {", emphasized: true }, + { kind: "addition", newLine: 23, content: " keys: [this.key(subject)]," }, + { kind: "addition", newLine: 24, content: " arguments: [String(ttlMs)]," }, + { kind: "addition", newLine: 25, content: "});" }, + { kind: "addition", newLine: 26, content: "return Number(next);", emphasized: true }, ], }, ], @@ -497,14 +497,14 @@ export const updateStops: TourStop[] = [ endLine: 52, lines: [ { kind: "header", content: "@@ -0,0 +44,9 @@ describe(\"RedisAttemptStore\", () => {" }, - { kind: "addition", newLine: 44, content: "+ it(\"preserves concurrent increments and expiry\", async () => {" }, - { kind: "addition", newLine: 45, content: "+ const subject = \"subject-1\";" }, - { kind: "addition", newLine: 46, content: "+ await Promise.all(", emphasized: true }, - { kind: "addition", newLine: 47, content: "+ Array.from({ length: 20 }, () => store.increment(subject, 60_000)),", emphasized: true }, - { kind: "addition", newLine: 48, content: "+ );" }, - { kind: "addition", newLine: 50, content: "+ expect(await redis.get(keyFor(subject))).toBe(\"20\");", emphasized: true }, - { kind: "addition", newLine: 51, content: "+ expect(await redis.pTTL(keyFor(subject))).toBeGreaterThan(0);" }, - { kind: "addition", newLine: 52, content: "+ });" }, + { kind: "addition", newLine: 44, content: "it(\"preserves concurrent increments and expiry\", async () => {" }, + { kind: "addition", newLine: 45, content: " const subject = \"subject-1\";" }, + { kind: "addition", newLine: 46, content: " await Promise.all(", emphasized: true }, + { kind: "addition", newLine: 47, content: " Array.from({ length: 20 }, () => store.increment(subject, 60_000)),", emphasized: true }, + { kind: "addition", newLine: 48, content: " );" }, + { kind: "addition", newLine: 50, content: " expect(await redis.get(keyFor(subject))).toBe(\"20\");", emphasized: true }, + { kind: "addition", newLine: 51, content: " expect(await redis.pTTL(keyFor(subject))).toBeGreaterThan(0);" }, + { kind: "addition", newLine: 52, content: "});" }, ], }, ], diff --git a/apps/web/src/styles.css b/apps/web/src/styles.css index e4c182b..3280589 100644 --- a/apps/web/src/styles.css +++ b/apps/web/src/styles.css @@ -29,6 +29,8 @@ --code-add-strong: rgba(57, 156, 101, 0.22); --code-delete: rgba(207, 90, 86, 0.13); --code-delete-strong: rgba(207, 90, 86, 0.24); + --code-add-inline: rgba(106, 226, 160, 0.34); + --code-delete-inline: rgba(255, 126, 116, 0.34); --selection: rgba(169, 156, 255, 0.18); --shadow: 0 22px 70px rgba(0, 0, 0, 0.38); --sans: Inter, ui-sans-serif, system-ui, -apple-system, BlinkMacSystemFont, "Segoe UI", sans-serif; @@ -69,6 +71,8 @@ --code-add-strong: rgba(36, 135, 79, 0.16); --code-delete: rgba(185, 69, 56, 0.09); --code-delete-strong: rgba(185, 69, 56, 0.16); + --code-add-inline: rgba(36, 135, 79, 0.24); + --code-delete-inline: rgba(185, 69, 56, 0.23); --selection: rgba(98, 85, 199, 0.14); --shadow: 0 22px 65px rgba(47, 46, 41, 0.14); } @@ -365,6 +369,9 @@ code, kbd { font-family: var(--mono); } .diff-line__number:hover:not(:disabled) { color: var(--accent); background: var(--accent-soft); } .diff-line__sign { color: var(--green); text-align: center; user-select: none; } .diff-line--deletion .diff-line__sign { color: var(--coral); } +.diff-line__change { padding: 1px 0; border-radius: 2px; } +.diff-line--addition .diff-line__change { background: var(--code-add-inline); } +.diff-line--deletion .diff-line__change { background: var(--code-delete-inline); } .diff-line code { padding-right: 20px; white-space: pre; } .diff-line__header { grid-column: 2 / -1; padding-left: 10px; color: var(--blue); background: var(--blue-soft); font-family: var(--mono); font-size: 8px; } .syntax-keyword { color: #d5a5ee; }.syntax-string { color: #a4d3a2; }.syntax-number { color: #e6bd85; } diff --git a/docs/projects/diff-so-fancy-integration/plan.md b/docs/projects/diff-so-fancy-integration/plan.md new file mode 100644 index 0000000..8943f3c --- /dev/null +++ b/docs/projects/diff-so-fancy-integration/plan.md @@ -0,0 +1,672 @@ +# Diff-so-fancy-style Diff Readability Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add diff-so-fancy-style word-level change emphasis and simplified hunk headers to Wingdiff's built-in diff viewer without changing the authoritative Git diff or review anchors. + +**Architecture:** A new pure presentation module pairs consecutive equal-length deletion/addition runs, computes an intraline changed span from the common prefix and suffix, and converts machine-oriented hunk headers into a compact human-readable form. `CodeDiff` renders those spans while retaining every line's original line numbers, fingerprints, selection behavior, and evidence metadata. No terminal executable, ANSI parser, or new runtime dependency is introduced. + +**Tech Stack:** React 19, TypeScript 7, Vitest 5, CSS variables + +## Global Constraints + +- Keep `apps/server/src/diff.ts` and the acquired unified diff authoritative. Do not alter hunk boundaries, line numbers, fingerprints, file statuses, review coverage, or comment anchors. +- Do not invoke `diff-so-fancy`, depend on Perl, execute reviewed repository code, or add a runtime dependency. +- Preserve selection and accessibility behavior. Line number buttons must remain the interaction target. +- Treat diff-so-fancy as the inspiration for readable presentation, not as a browser renderer. The integration boundary is the React presentation layer. +- Implement test-first. Watch each new test fail for the intended missing behavior before writing production code. +- Normalize demo fixture content to the server's canonical shape: changed-line content never includes the unified-diff `+ ` or `- ` marker. Preserve legitimate source content that happens to begin with `+ ` or `- `. +- New helper coverage must be greater than 90 percent, measured over `apps/web/src/diffPresentation.ts`. + +--- + +### Task 1: Diff-so-fancy-style diff presentation + +**Files:** +- Create: `apps/web/src/diffPresentation.ts` +- Create: `apps/web/src/diffPresentation.test.ts` +- Modify: `apps/web/src/fixture.ts` +- Modify: `apps/web/src/fixture.test.ts` +- Modify: `apps/web/src/components/CodeDiff.tsx` +- Modify: `apps/web/src/components/CodeDiff.test.tsx` +- Modify: `apps/web/src/styles.css` +- Modify: `README.md` + +**Interfaces:** +- `presentDiffLines(lines: DiffLine[], path: string): PresentedDiffLine[]` +- `PresentedDiffLine extends DiffLine` and optionally carries `segments: { before: string; changed: string; after: string }` +- `CodeDiff` continues to accept the existing `CodeDiffProps`; its callers do not change. +- `DiffLine`, `EvidenceBlock`, and all server-side diff types remain unchanged. + +- [ ] **Step 1: Write the failing presentation tests** + +Create `apps/web/src/diffPresentation.test.ts` with: + +```typescript +import { describe, expect, it } from "vitest"; +import type { DiffLine } from "./types"; +import { presentDiffLines } from "./diffPresentation"; + +describe("presentDiffLines", () => { + it("marks only the changed span in paired deletion and addition lines", () => { + const lines: DiffLine[] = [ + { kind: "deletion", oldLine: 7, content: "const timeout = 30;" }, + { kind: "addition", newLine: 7, content: "const timeout = 60;" }, + ]; + + const presented = presentDiffLines(lines, "src/timeouts.ts"); + + expect(presented[0]?.segments).toEqual({ before: "const timeout = ", changed: "30", after: ";" }); + expect(presented[1]?.segments).toEqual({ before: "const timeout = ", changed: "60", after: ";" }); + }); + + it("uses Unicode code points when finding the changed span", () => { + const lines: DiffLine[] = [ + { kind: "deletion", oldLine: 3, content: "const label = \"café old\";" }, + { kind: "addition", newLine: 3, content: "const label = \"café new\";" }, + ]; + + const presented = presentDiffLines(lines, "src/labels.ts"); + + expect(presented[0]?.segments).toEqual({ before: "const label = \"café ", changed: "old", after: "\";" }); + expect(presented[1]?.segments).toEqual({ before: "const label = \"café ", changed: "new", after: "\";" }); + }); + + it("leaves unbalanced replacement runs unsegmented", () => { + const lines: DiffLine[] = [ + { kind: "deletion", oldLine: 2, content: "one()" }, + { kind: "addition", newLine: 2, content: "uno()" }, + { kind: "addition", newLine: 3, content: "dos()" }, + ]; + + expect(presentDiffLines(lines, "src/count.ts").every((line) => line.segments === undefined)).toBe(true); + }); + + it("preserves source content that begins with a diff marker", () => { + const lines: DiffLine[] = [ + { kind: "addition", newLine: 4, content: "+ Markdown list item" }, + ]; + + const presented = presentDiffLines(lines, "README.md"); + + expect(presented[0]?.content).toBe("+ Markdown list item"); + expect(presented[0]?.segments).toBeUndefined(); + }); + + it("does not emphasize identical paired lines", () => { + const lines: DiffLine[] = [ + { kind: "deletion", oldLine: 4, content: "same" }, + { kind: "addition", newLine: 4, content: "same" }, + ]; + + expect(presentDiffLines(lines, "file.txt").every((line) => line.segments === undefined)).toBe(true); + }); + + it("simplifies standard hunk headers and preserves non-hunk headers", () => { + const presented = presentDiffLines([ + { kind: "header", content: "@@ -41,8 +41,18 @@ export async function login(input: LoginInput) {" }, + { kind: "header", content: "@@ -10,2 +0,0 @@" }, + { kind: "header", content: "Binary file changed" }, + ], "src/auth/login.ts"); + + expect(presented[0]?.content).toBe("@ src/auth/login.ts:41 @ export async function login(input: LoginInput) {"); + expect(presented[1]?.content).toBe("@ src/auth/login.ts:10 @"); + expect(presented[2]?.content).toBe("Binary file changed"); + }); + + it("does not mutate the supplied evidence lines", () => { + const lines: DiffLine[] = [ + { kind: "deletion", oldLine: 4, content: "old" }, + { kind: "addition", newLine: 4, content: "new" }, + ]; + const original = structuredClone(lines); + + presentDiffLines(lines, "file.txt"); + + expect(lines).toEqual(original); + }); +}); +``` + +- [ ] **Step 2: Run the test and verify RED** + +Run: + +```bash +npm test --workspace @wingdiff/web -- src/diffPresentation.test.ts +``` + +Expected: FAIL because `./diffPresentation` does not exist. If Vitest reports another reason, fix the test invocation or imports and rerun until the failure is specifically the missing module. + +- [ ] **Step 3: Write the failing canonical-fixture test** + +Add this test to `apps/web/src/fixture.test.ts`: + +```typescript + it("stores changed-line content without unified-diff markers", () => { + const lines = [...tourStops, ...updateStops].flatMap((stop) => stop.evidence.flatMap((evidence) => evidence.lines)); + const markedLines = lines.filter((line) => ( + (line.kind === "addition" && /^[+](?: |$)/.test(line.content)) || + (line.kind === "deletion" && /^[-](?: |$)/.test(line.content)) + )); + + expect(markedLines).toEqual([]); + }); +``` + +Run: + +```bash +npm test --workspace @wingdiff/web -- src/fixture.test.ts +``` + +Expected: FAIL because demo fixture changed lines still contain unified-diff markers. + +- [ ] **Step 4: Normalize the demo fixture and verify GREEN** + +Remove only the fixture-only leading `+ ` and `- ` markers from changed-line content: + +```bash +perl -pi -e 's/content: "\+ /content: "/g; s/content: "- /content: "/g' apps/web/src/fixture.ts +``` + +Run: + +```bash +npm test --workspace @wingdiff/web -- src/fixture.test.ts +``` + +Expected: PASS. The fixture now matches the server's canonical `DiffLine.content` shape, so the renderer never needs to guess whether a leading `+ ` or `- ` is a marker or legitimate source text. + +- [ ] **Step 5: Implement the minimal presentation helper** + +Create `apps/web/src/diffPresentation.ts` with: + +```typescript +import type { DiffLine } from "./types"; + +export interface DiffLineSegments { + before: string; + changed: string; + after: string; +} + +export interface PresentedDiffLine extends DiffLine { + segments?: DiffLineSegments; +} + +export function presentDiffLines(lines: DiffLine[], path: string): PresentedDiffLine[] { + const presented: PresentedDiffLine[] = lines.map((line) => ({ + ...line, + content: line.kind === "header" ? humanizeHunkHeader(line.content, path) : line.content, + })); + + let index = 0; + while (index < presented.length) { + if (presented[index]?.kind !== "deletion") { + index += 1; + continue; + } + + const deletionStart = index; + while (presented[index]?.kind === "deletion") index += 1; + const additionStart = index; + while (presented[index]?.kind === "addition") index += 1; + + const deletions = presented.slice(deletionStart, additionStart); + const additions = presented.slice(additionStart, index); + if (!additions.length || deletions.length !== additions.length) continue; + + for (let pairIndex = 0; pairIndex < deletions.length; pairIndex += 1) { + const deletion = deletions[pairIndex]!; + const addition = additions[pairIndex]!; + const pair = splitChangedSpan(deletion.content, addition.content); + if (!pair) continue; + deletion.segments = pair.deletion; + addition.segments = pair.addition; + } + } + + return presented; +} + +function humanizeHunkHeader(header: string, path: string): string { + const match = /^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@(.*)$/.exec(header); + if (!match) return header; + const newStart = Number(match[2]); + const lineNumber = newStart === 0 ? Number(match[1]) : newStart; + const suffix = match[3]!.trimEnd(); + return `@ ${path}:${lineNumber} @${suffix}`; +} + +function splitChangedSpan( + deletion: string, + addition: string, +): { deletion: DiffLineSegments; addition: DiffLineSegments } | undefined { + const deleted = Array.from(deletion); + const added = Array.from(addition); + let prefix = 0; + while (prefix < deleted.length && prefix < added.length && deleted[prefix] === added[prefix]) prefix += 1; + + let suffix = 0; + while ( + suffix < deleted.length - prefix && + suffix < added.length - prefix && + deleted[deleted.length - suffix - 1] === added[added.length - suffix - 1] + ) { + suffix += 1; + } + + const deletionSegments = segments(deleted, prefix, suffix); + const additionSegments = segments(added, prefix, suffix); + if (!deletionSegments.changed && !additionSegments.changed) return undefined; + return { deletion: deletionSegments, addition: additionSegments }; +} + +function segments(codepoints: string[], prefix: number, suffix: number): DiffLineSegments { + return { + before: codepoints.slice(0, prefix).join(""), + changed: codepoints.slice(prefix, codepoints.length - suffix).join(""), + after: codepoints.slice(codepoints.length - suffix).join(""), + }; +} +``` + +- [ ] **Step 6: Run the helper tests and verify GREEN** + +Run: + +```bash +npm test --workspace @wingdiff/web -- src/diffPresentation.test.ts +``` + +Expected: PASS, 7 tests passed. + +- [ ] **Step 7: Write the failing component and stylesheet tests** + +Replace `apps/web/src/components/CodeDiff.test.tsx` with: + +```typescript +import { readFileSync } from "node:fs"; +import { resolve } from "node:path"; +import { renderToStaticMarkup } from "react-dom/server"; +import { describe, expect, it } from "vitest"; +import type { EvidenceBlock } from "../types"; +import { CodeDiff } from "./CodeDiff"; + +describe("CodeDiff", () => { + const sampleEvidence: EvidenceBlock = { + id: "evidence-1", + path: ".github/workflows/ci.yml", + label: "Modified +1 -1", + language: "YAML", + startLine: 1, + endLine: 3, + lines: [ + { kind: "header", content: "@@ -1,3 +1,3 @@" }, + { kind: "deletion", oldLine: 2, content: "snap_count=30", emphasized: true }, + { kind: "addition", newLine: 2, content: "snap_count=60", emphasized: true }, + { kind: "context", oldLine: 3, newLine: 3, content: "exit 0" }, + ], + }; + + it("renders diff lines with deletion, addition, and emphasized classes", () => { + const html = renderToStaticMarkup(); + + expect(html).toContain("diff-line--deletion is-emphasized"); + expect(html).toContain("diff-line--addition is-emphasized"); + expect(html).toContain("diff-line--context"); + }); + + it("renders the simplified hunk header and intraline change spans", () => { + const html = renderToStaticMarkup(); + + expect(html).toContain("@ .github/workflows/ci.yml:1 @"); + expect(html).not.toContain("@@ -1,3 +1,3 @@"); + expect(html).toContain("diff-line__change"); + expect(html).toContain("syntax-number"); + }); + + it("keeps syntax classes on changed string content", () => { + const evidence: EvidenceBlock = { + ...sampleEvidence, + path: "src/labels.ts", + lines: [ + { kind: "deletion", oldLine: 3, content: "const label = \"café old\";" }, + { kind: "addition", newLine: 3, content: "const label = \"café new\";" }, + ], + }; + + const html = renderToStaticMarkup(); + + expect(html).toContain("diff-line__change syntax-string"); + expect(html).toContain("old"); + expect(html).toContain("new"); + }); + + it("defines strong deletion tokens and intraline emphasis CSS rules", () => { + const cssPath = resolve(__dirname, "../styles.css"); + const css = readFileSync(cssPath, "utf-8"); + + expect(css).toContain("--code-add-inline:"); + expect(css).toContain("--code-delete-inline:"); + expect(css).toContain(".diff-line__change"); + expect(css).toContain(".diff-line--addition .diff-line__change"); + expect(css).toContain(".diff-line--deletion .diff-line__change"); + expect(css).toContain(".diff-line--deletion.is-emphasized"); + expect(css).toContain(".diff-line--addition.is-emphasized"); + }); +}); +``` + +- [ ] **Step 8: Run the component tests and verify RED** + +Run: + +```bash +npm test --workspace @wingdiff/web -- src/components/CodeDiff.test.tsx +``` + +Expected: FAIL because the current renderer emits the raw `@@` hunk header and has no `.diff-line__change` output. Confirm the failure names those missing behaviors, not an import or syntax error. + +- [ ] **Step 9: Integrate presentation into `CodeDiff`** + +Replace `apps/web/src/components/CodeDiff.tsx` with: + +```tsx +import { memo, useMemo } from "react"; +import { presentDiffLines, type DiffLineSegments } from "../diffPresentation"; +import type { EvidenceBlock } from "../types"; +import { Icon } from "./Icon"; + +interface Selection { + evidenceId: string; + side?: "LEFT" | "RIGHT"; + start: number; + end: number; +} + +interface CodeDiffProps { + evidence: EvidenceBlock; + selection?: Selection | null; + onSelectLine?: (evidenceId: string, line: number, side: "LEFT" | "RIGHT", extend: boolean) => void; + onAsk?: () => void; + onComment?: () => void; + minimal?: boolean; +} + +export const CodeDiff = memo(function CodeDiff({ + evidence, + selection, + onSelectLine, + onAsk, + onComment, + minimal = false, +}: CodeDiffProps) { + const selectedCount = selection?.evidenceId === evidence.id + ? Math.abs(selection.end - selection.start) + 1 + : 0; + const lines = useMemo( + () => presentDiffLines(evidence.lines, evidence.path), + [evidence.lines, evidence.path], + ); + + return ( +
+
+
+ +
+ {fileName(evidence.path)} + {directoryName(evidence.path)} +
+
+
+ {evidence.label} + {evidence.language} +
+
+ +
+ {lines.map((line, index) => { + const side = line.kind === "deletion" ? "LEFT" : "RIGHT"; + const lineNumber = side === "RIGHT" ? line.newLine : line.oldLine; + const selected = Boolean( + lineNumber && + selection?.evidenceId === evidence.id && + (!selection.side || selection.side === side) && + lineNumber >= Math.min(selection.start, selection.end) && + lineNumber <= Math.max(selection.start, selection.end), + ); + return ( +
+ {line.kind === "header" ? ( + <>{line.content} + ) : ( + <> + + + {highlight(line.content, line.segments)} + + )} +
+ ); + })} +
+ + {!minimal && ( +
+ + {selectedCount ? `${selectedCount} line${selectedCount === 1 ? "" : "s"} selected` : "Select a line to anchor feedback"} + +
+ + +
+
+ )} +
+ ); +}); + +function fileName(path: string) { + return path.split("/").at(-1) ?? path; +} + +function directoryName(path: string) { + const parts = path.split("/"); + parts.pop(); + return parts.length ? `${parts.join("/")}/` : ""; +} + +function highlight(content: string, segments?: DiffLineSegments) { + const changeStart = segments?.before.length ?? -1; + const changeEnd = segments ? changeStart + segments.changed.length : -1; + return syntaxTokens(content).flatMap((token, tokenIndex) => ( + splitToken(content, token, changeStart, changeEnd).map((part, partIndex) => ( + + {part.text} + + )) + )); +} + +interface SyntaxToken { + start: number; + end: number; + className: string; +} + +function syntaxTokens(content: string): SyntaxToken[] { + const pattern = /(\"[^\"]*\"|'[^']*'|`[^`]*`|\b(?:async|await|const|return|if|else|new|throw|class|private|export|function|true|false|null|undefined)\b|\b\d+\b)/g; + const tokens: SyntaxToken[] = []; + let cursor = 0; + for (const match of content.matchAll(pattern)) { + const start = match.index; + if (start > cursor) tokens.push({ start: cursor, end: start, className: "" }); + const text = match[0]; + tokens.push({ start, end: start + text.length, className: syntaxClass(text) }); + cursor = start + text.length; + } + if (cursor < content.length) tokens.push({ start: cursor, end: content.length, className: "" }); + return tokens; +} + +function syntaxClass(token: string): string { + if (/^[\"'`]/.test(token)) return "syntax-string"; + if (/^\d+$/.test(token)) return "syntax-number"; + return "syntax-keyword"; +} + +function splitToken(content: string, token: SyntaxToken, changeStart: number, changeEnd: number) { + const boundaries = [token.start, token.end]; + if (changeStart > token.start && changeStart < token.end) boundaries.push(changeStart); + if (changeEnd > token.start && changeEnd < token.end) boundaries.push(changeEnd); + boundaries.sort((left, right) => left - right); + + return boundaries.slice(0, -1).map((start, index) => { + const end = boundaries[index + 1]!; + return { + text: content.slice(start, end), + start, + end, + changed: changeStart >= 0 && start >= changeStart && end <= changeEnd, + }; + }); +} +``` + +- [ ] **Step 10: Add intraline CSS variables and rendering rules** + +In `apps/web/src/styles.css`, add these variables to the dark `:root` block after `--code-delete-strong`: + +```css + --code-add-inline: rgba(106, 226, 160, 0.34); + --code-delete-inline: rgba(255, 126, 116, 0.34); +``` + +Add these variables to the light `:root[data-theme="light"]` block after its `--code-delete-strong`: + +```css + --code-add-inline: rgba(36, 135, 79, 0.24); + --code-delete-inline: rgba(185, 69, 56, 0.23); +``` + +Add these rules after `.diff-line__sign`: + +```css +.diff-line__change { padding: 1px 0; border-radius: 2px; } +.diff-line--addition .diff-line__change { background: var(--code-add-inline); } +.diff-line--deletion .diff-line__change { background: var(--code-delete-inline); } +``` + +- [ ] **Step 11: Run the focused web tests and verify GREEN** + +Run: + +```bash +npm test --workspace @wingdiff/web -- src/diffPresentation.test.ts src/fixture.test.ts src/components/CodeDiff.test.tsx +``` + +Expected: PASS, all presentation and component tests pass. + +- [ ] **Step 12: Document the integration boundary** + +Add this paragraph under the `### Read-only change acquisition` section in `README.md`, after the paragraph ending `run untrusted code.` and before the host-specific setup details: + +```markdown +The built-in diff viewer takes a lightweight cue from +[diff-so-fancy](https://github.com/so-fancy/diff-so-fancy): paired changed +lines emphasize only the intraline span that differs, and hunk headers are +reduced to a readable `@ path:line @ context` form. Wingdiff does not run the +terminal renderer or consume ANSI output; Git's structured diff remains the +authoritative source for line numbers, fingerprints, and review anchors. +``` + +- [ ] **Step 13: Run the complete repository verification** + +Run, in order: + +```bash +npm test +npm run typecheck +npm run build +npm run test:e2e +``` + +Expected: all server and web unit tests pass, TypeScript reports no errors, both workspaces build, and Playwright's mocked review flow passes. + +- [ ] **Step 14: Measure helper coverage** + +Install the matching coverage provider without saving it or changing the lockfile: + +```bash +npm install --no-save --package-lock=false @vitest/coverage-v8@5.0.3 +``` + +Then run: + +```bash +npm exec --workspace @wingdiff/web -- vitest run src/diffPresentation.test.ts --coverage --coverage.include=src/diffPresentation.ts --coverage.reporter=text +``` + +Expected: `diffPresentation.ts` has greater than 90 percent statements, branches, functions, and lines. If any metric is at or below 90 percent, add tests for the uncovered behavior, rerun the focused test to observe it fail when the behavior is absent, then implement and rerun. Report the exact four coverage percentages. + +- [ ] **Step 15: Exercise the feature in the local development server** + +Run this command in a dedicated PTY session that remains alive: + +```bash +npm run wingdiff -- --demo --no-open +``` + +Expected: the command prints an authenticated `Wingdiff is ready at` URL with a +`wingdiff_token` query parameter. Open that URL, choose the demo review, enter +Changed files, and verify that a paired replacement shows a narrower intraline +emphasis and that the hunk header uses `@ path:line @ context` instead of +`@@ ... @@`. Confirm line selection still works. Keep the server session alive for +the user and record its URL, PTY session ID, representative flow, and any +limitation in the report. + +- [ ] **Step 16: Commit the validated change** + +Use explicit task-owned paths: + +```bash +gcommit --no-push -m "feat: add diff-so-fancy-style diff readability" -- docs/projects/diff-so-fancy-integration/plan.md apps/web/src/diffPresentation.ts apps/web/src/diffPresentation.test.ts apps/web/src/fixture.ts apps/web/src/fixture.test.ts apps/web/src/components/CodeDiff.tsx apps/web/src/components/CodeDiff.test.tsx apps/web/src/styles.css README.md +``` + +Expected: one local commit containing only the plan, presentation helper and tests, viewer integration, styles, and documentation. Do not push, merge, or deploy. + +## Verification Receipt + +The implementer report must record: +- RED command and the exact missing-behavior failure for both TDD cycles +- GREEN focused-test output +- Complete `npm test`, `npm run typecheck`, `npm run build`, and `npm run test:e2e` results +- Exact helper coverage percentages +- Commit SHA and changed paths +- Local URL and PTY session ID for the still-running development server +- Representative demo flow and visual verification result +- Any intentional limitation, especially that diff-so-fancy itself is not bundled From d79b4c5025ce2d011b2ac4da69090fa059b3fb7f Mon Sep 17 00:00:00 2001 From: Will Date: Sun, 1 Nov 2026 20:32:25 -0600 Subject: [PATCH 2/4] fix: refine diff presentation and demo coverage --- README.md | 9 +- apps/web/src/components/CodeDiff.test.tsx | 24 ++- apps/web/src/diffPresentation.test.ts | 34 +++- apps/web/src/diffPresentation.ts | 19 +-- apps/web/src/fixture.test.ts | 16 +- apps/web/src/fixture.ts | 19 +-- apps/web/src/styles.css | 12 +- .../diff-so-fancy-integration/plan.md | 160 ++++++++++++------ e2e/reviewer-flow.spec.ts | 16 ++ 9 files changed, 199 insertions(+), 110 deletions(-) diff --git a/README.md b/README.md index c5e024a..0782fd7 100644 --- a/README.md +++ b/README.md @@ -149,10 +149,11 @@ Wingdiff reads metadata through `gh` or `glab`, fetches missing objects into pri The built-in diff viewer takes a lightweight cue from [diff-so-fancy](https://github.com/so-fancy/diff-so-fancy): paired changed -lines emphasize only the intraline span that differs, and hunk headers are -reduced to a readable `@ path:line @ context` form. Wingdiff does not run the -terminal renderer or consume ANSI output; Git's structured diff remains the -authoritative source for line numbers, fingerprints, and review anchors. +lines with meaningful shared context emphasize only the intraline span that +differs, and hunk headers are reduced to a readable `@ path:line @ context` +form. Wingdiff does not run the terminal renderer or consume ANSI output; +Git's structured diff remains the authoritative source for line numbers, +fingerprints, and review anchors. From the real-PR Summary, choose a configured model and select **Generate guided review**. The model organizes the validated diff into semantic stops, ranks concrete findings by severity, and references opaque evidence anchors. Wingdiff rejects the result unless every changed file is covered and every claim or finding resolves to an exact known anchor. Generated tours are pinned to the acquired head SHA and can be resumed locally. diff --git a/apps/web/src/components/CodeDiff.test.tsx b/apps/web/src/components/CodeDiff.test.tsx index 1e0dc07..ae7e553 100644 --- a/apps/web/src/components/CodeDiff.test.tsx +++ b/apps/web/src/components/CodeDiff.test.tsx @@ -5,6 +5,11 @@ import { describe, expect, it } from "vitest"; import type { EvidenceBlock } from "../types"; import { CodeDiff } from "./CodeDiff"; +function ruleBody(css: string, selector: string): string { + const escaped = selector.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + return new RegExp(`${escaped}\\s*\\{([^}]*)\\}`).exec(css)?.[1] ?? ""; +} + describe("CodeDiff", () => { const sampleEvidence: EvidenceBlock = { id: "evidence-1", @@ -55,15 +60,22 @@ describe("CodeDiff", () => { expect(html).toContain("new"); }); - it("defines strong deletion tokens and intraline emphasis CSS rules", () => { + it("uses high-contrast intraline underlines without filled backgrounds", () => { const cssPath = resolve(__dirname, "../styles.css"); const css = readFileSync(cssPath, "utf-8"); + const additionChange = ruleBody(css, ".diff-line--addition .diff-line__change"); + const deletionChange = ruleBody(css, ".diff-line--deletion .diff-line__change"); - expect(css).toContain("--code-add-inline:"); - expect(css).toContain("--code-delete-inline:"); - expect(css).toContain(".diff-line__change"); - expect(css).toContain(".diff-line--addition .diff-line__change"); - expect(css).toContain(".diff-line--deletion .diff-line__change"); + expect(css).toContain("--code-add-inline: #6ae2a0;"); + expect(css).toContain("--code-delete-inline: #ff7e74;"); + expect(css).toContain("--code-add-inline: #21794e;"); + expect(css).toContain("--code-delete-inline: #b94538;"); + expect(additionChange).toContain("box-shadow: inset 0 -2px 0 var(--code-add-inline);"); + expect(deletionChange).toContain("box-shadow: inset 0 -2px 0 var(--code-delete-inline);"); + expect(additionChange).not.toContain("background"); + expect(deletionChange).not.toContain("background"); + expect(css).toContain(".diff-line--addition { background: var(--code-add); }"); + expect(css).toContain(".diff-line--deletion { background: var(--code-delete); }"); expect(css).toContain(".diff-line--deletion.is-emphasized"); expect(css).toContain(".diff-line--addition.is-emphasized"); }); diff --git a/apps/web/src/diffPresentation.test.ts b/apps/web/src/diffPresentation.test.ts index dbc1d23..7da39d1 100644 --- a/apps/web/src/diffPresentation.test.ts +++ b/apps/web/src/diffPresentation.test.ts @@ -11,8 +11,8 @@ describe("presentDiffLines", () => { const presented = presentDiffLines(lines, "src/timeouts.ts"); - expect(presented[0]?.segments).toEqual({ before: "const timeout = ", changed: "30", after: ";" }); - expect(presented[1]?.segments).toEqual({ before: "const timeout = ", changed: "60", after: ";" }); + expect(presented[0]?.segments).toEqual({ before: "const timeout = ", changed: "3", after: "0;" }); + expect(presented[1]?.segments).toEqual({ before: "const timeout = ", changed: "6", after: "0;" }); }); it("uses Unicode code points when finding the changed span", () => { @@ -27,6 +27,36 @@ describe("presentDiffLines", () => { expect(presented[1]?.segments).toEqual({ before: "const label = \"café ", changed: "new", after: "\";" }); }); + it("keeps supplementary-plane code points intact", () => { + const lines: DiffLine[] = [ + { kind: "deletion", oldLine: 3, content: "const marker = \"😀\";" }, + { kind: "addition", newLine: 3, content: "const marker = \"😁\";" }, + ]; + + const presented = presentDiffLines(lines, "src/labels.ts"); + + expect(presented[0]?.segments).toEqual({ before: "const marker = \"", changed: "😀", after: "\";" }); + expect(presented[1]?.segments).toEqual({ before: "const marker = \"", changed: "😁", after: "\";" }); + }); + + it("suppresses intraline segments for complete replacements", () => { + const lines: DiffLine[] = [ + { kind: "deletion", oldLine: 4, content: "old" }, + { kind: "addition", newLine: 4, content: "new" }, + ]; + + expect(presentDiffLines(lines, "file.txt").every((line) => line.segments === undefined)).toBe(true); + }); + + it("ignores whitespace-only common affixes when deciding whether to segment", () => { + const lines: DiffLine[] = [ + { kind: "deletion", oldLine: 4, content: " old" }, + { kind: "addition", newLine: 4, content: " new" }, + ]; + + expect(presentDiffLines(lines, "file.txt").every((line) => line.segments === undefined)).toBe(true); + }); + it("leaves unbalanced replacement runs unsegmented", () => { const lines: DiffLine[] = [ { kind: "deletion", oldLine: 2, content: "one()" }, diff --git a/apps/web/src/diffPresentation.ts b/apps/web/src/diffPresentation.ts index 7751776..6182741 100644 --- a/apps/web/src/diffPresentation.ts +++ b/apps/web/src/diffPresentation.ts @@ -62,13 +62,6 @@ function splitChangedSpan( const added = Array.from(addition); let prefix = 0; while (prefix < deleted.length && prefix < added.length && deleted[prefix] === added[prefix]) prefix += 1; - while ( - prefix > 0 && - isDigit(deleted[prefix - 1]!) && - isDigit(deleted[prefix]!) && - isDigit(added[prefix - 1]!) && - isDigit(added[prefix]!) - ) prefix -= 1; let suffix = 0; while ( @@ -78,17 +71,11 @@ function splitChangedSpan( ) { suffix += 1; } - while ( - suffix > 0 && - isDigit(deleted[deleted.length - suffix - 1]!) && - isDigit(deleted[deleted.length - suffix]!) && - isDigit(added[added.length - suffix - 1]!) && - isDigit(added[added.length - suffix]!) - ) suffix -= 1; const deletionSegments = segments(deleted, prefix, suffix); const additionSegments = segments(added, prefix, suffix); if (!deletionSegments.changed && !additionSegments.changed) return undefined; + if (!/\S/.test(deletionSegments.before) && !/\S/.test(deletionSegments.after)) return undefined; return { deletion: deletionSegments, addition: additionSegments }; } @@ -99,7 +86,3 @@ function segments(codepoints: string[], prefix: number, suffix: number): DiffLin after: codepoints.slice(codepoints.length - suffix).join(""), }; } - -function isDigit(codepoint: string): boolean { - return /^\d$/.test(codepoint); -} diff --git a/apps/web/src/fixture.test.ts b/apps/web/src/fixture.test.ts index f245776..72a721d 100644 --- a/apps/web/src/fixture.test.ts +++ b/apps/web/src/fixture.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from "vitest"; +import { presentDiffLines } from "./diffPresentation"; import { pullRequest, reviewUpdate, risks, topologyNodes, tourStops, updateStops } from "./fixture"; describe("guided review fixture", () => { @@ -94,13 +95,14 @@ describe("guided review fixture", () => { } }); - it("stores changed-line content without unified-diff markers", () => { - const lines = [...tourStops, ...updateStops].flatMap((stop) => stop.evidence.flatMap((evidence) => evidence.lines)); - const markedLines = lines.filter((line) => ( - (line.kind === "addition" && /^[+](?: |$)/.test(line.content)) || - (line.kind === "deletion" && /^[-](?: |$)/.test(line.content)) - )); + it("includes an intraline replacement in the update review", () => { + const segments = updateStops.flatMap((stop) => stop.evidence.flatMap((evidence) => ( + presentDiffLines(evidence.lines, evidence.path).flatMap((line) => line.segments ? [line.segments] : []) + ))); - expect(markedLines).toEqual([]); + expect(segments.map((segment) => segment.changed)).toEqual(expect.arrayContaining([ + "Number(await this.redis.get(key) ?? 0) + 1", + "await this.redis.eval(INCREMENT_WITH_TTL, { keys: [key], arguments: [ttlMs] })", + ])); }); }); diff --git a/apps/web/src/fixture.ts b/apps/web/src/fixture.ts index 6bf172c..54dcc24 100644 --- a/apps/web/src/fixture.ts +++ b/apps/web/src/fixture.ts @@ -440,18 +440,15 @@ export const updateStops: TourStop[] = [ path: "src/infra/redis/attempt-store.ts", label: "Counter fix", language: "TypeScript", - startLine: 22, - endLine: 28, + startLine: 20, + endLine: 23, lines: [ - { kind: "header", content: "@@ -22,7 +22,7 @@ export class RedisAttemptStore {" }, - { kind: "deletion", oldLine: 22, content: "const current = Number(await this.redis.get(key) ?? 0);", emphasized: true }, - { kind: "deletion", oldLine: 23, content: "const next = current + 1;", emphasized: true }, - { kind: "deletion", oldLine: 24, content: "await this.redis.set(key, String(next), { PX: ttlMs });", emphasized: true }, - { kind: "addition", newLine: 22, content: "const next = await this.redis.eval(INCREMENT_WITH_TTL, {", emphasized: true }, - { kind: "addition", newLine: 23, content: " keys: [this.key(subject)]," }, - { kind: "addition", newLine: 24, content: " arguments: [String(ttlMs)]," }, - { kind: "addition", newLine: 25, content: "});" }, - { kind: "addition", newLine: 26, content: "return Number(next);", emphasized: true }, + { kind: "header", content: "@@ -20,4 +20,4 @@ export class RedisAttemptStore {" }, + { kind: "context", oldLine: 20, newLine: 20, content: "async increment(subject: string, ttlMs: number) {" }, + { kind: "deletion", oldLine: 21, content: " const next = Number(await this.redis.get(key) ?? 0) + 1;", emphasized: true }, + { kind: "addition", newLine: 21, content: " const next = await this.redis.eval(INCREMENT_WITH_TTL, { keys: [key], arguments: [ttlMs] });", emphasized: true }, + { kind: "context", oldLine: 22, newLine: 22, content: " return next;" }, + { kind: "context", oldLine: 23, newLine: 23, content: "}" }, ], }, ], diff --git a/apps/web/src/styles.css b/apps/web/src/styles.css index 3280589..2603f6b 100644 --- a/apps/web/src/styles.css +++ b/apps/web/src/styles.css @@ -29,8 +29,8 @@ --code-add-strong: rgba(57, 156, 101, 0.22); --code-delete: rgba(207, 90, 86, 0.13); --code-delete-strong: rgba(207, 90, 86, 0.24); - --code-add-inline: rgba(106, 226, 160, 0.34); - --code-delete-inline: rgba(255, 126, 116, 0.34); + --code-add-inline: #6ae2a0; + --code-delete-inline: #ff7e74; --selection: rgba(169, 156, 255, 0.18); --shadow: 0 22px 70px rgba(0, 0, 0, 0.38); --sans: Inter, ui-sans-serif, system-ui, -apple-system, BlinkMacSystemFont, "Segoe UI", sans-serif; @@ -71,8 +71,8 @@ --code-add-strong: rgba(36, 135, 79, 0.16); --code-delete: rgba(185, 69, 56, 0.09); --code-delete-strong: rgba(185, 69, 56, 0.16); - --code-add-inline: rgba(36, 135, 79, 0.24); - --code-delete-inline: rgba(185, 69, 56, 0.23); + --code-add-inline: #21794e; + --code-delete-inline: #b94538; --selection: rgba(98, 85, 199, 0.14); --shadow: 0 22px 65px rgba(47, 46, 41, 0.14); } @@ -370,8 +370,8 @@ code, kbd { font-family: var(--mono); } .diff-line__sign { color: var(--green); text-align: center; user-select: none; } .diff-line--deletion .diff-line__sign { color: var(--coral); } .diff-line__change { padding: 1px 0; border-radius: 2px; } -.diff-line--addition .diff-line__change { background: var(--code-add-inline); } -.diff-line--deletion .diff-line__change { background: var(--code-delete-inline); } +.diff-line--addition .diff-line__change { box-shadow: inset 0 -2px 0 var(--code-add-inline); } +.diff-line--deletion .diff-line__change { box-shadow: inset 0 -2px 0 var(--code-delete-inline); } .diff-line code { padding-right: 20px; white-space: pre; } .diff-line__header { grid-column: 2 / -1; padding-left: 10px; color: var(--blue); background: var(--blue-soft); font-family: var(--mono); font-size: 8px; } .syntax-keyword { color: #d5a5ee; }.syntax-string { color: #a4d3a2; }.syntax-number { color: #e6bd85; } diff --git a/docs/projects/diff-so-fancy-integration/plan.md b/docs/projects/diff-so-fancy-integration/plan.md index 8943f3c..c30b64f 100644 --- a/docs/projects/diff-so-fancy-integration/plan.md +++ b/docs/projects/diff-so-fancy-integration/plan.md @@ -56,8 +56,8 @@ describe("presentDiffLines", () => { const presented = presentDiffLines(lines, "src/timeouts.ts"); - expect(presented[0]?.segments).toEqual({ before: "const timeout = ", changed: "30", after: ";" }); - expect(presented[1]?.segments).toEqual({ before: "const timeout = ", changed: "60", after: ";" }); + expect(presented[0]?.segments).toEqual({ before: "const timeout = ", changed: "3", after: "0;" }); + expect(presented[1]?.segments).toEqual({ before: "const timeout = ", changed: "6", after: "0;" }); }); it("uses Unicode code points when finding the changed span", () => { @@ -72,6 +72,36 @@ describe("presentDiffLines", () => { expect(presented[1]?.segments).toEqual({ before: "const label = \"café ", changed: "new", after: "\";" }); }); + it("keeps supplementary-plane code points intact", () => { + const lines: DiffLine[] = [ + { kind: "deletion", oldLine: 3, content: "const marker = \"😀\";" }, + { kind: "addition", newLine: 3, content: "const marker = \"😁\";" }, + ]; + + const presented = presentDiffLines(lines, "src/labels.ts"); + + expect(presented[0]?.segments).toEqual({ before: "const marker = \"", changed: "😀", after: "\";" }); + expect(presented[1]?.segments).toEqual({ before: "const marker = \"", changed: "😁", after: "\";" }); + }); + + it("suppresses intraline segments for complete replacements", () => { + const lines: DiffLine[] = [ + { kind: "deletion", oldLine: 4, content: "old" }, + { kind: "addition", newLine: 4, content: "new" }, + ]; + + expect(presentDiffLines(lines, "file.txt").every((line) => line.segments === undefined)).toBe(true); + }); + + it("ignores whitespace-only common affixes when deciding whether to segment", () => { + const lines: DiffLine[] = [ + { kind: "deletion", oldLine: 4, content: " old" }, + { kind: "addition", newLine: 4, content: " new" }, + ]; + + expect(presentDiffLines(lines, "file.txt").every((line) => line.segments === undefined)).toBe(true); + }); + it("leaves unbalanced replacement runs unsegmented", () => { const lines: DiffLine[] = [ { kind: "deletion", oldLine: 2, content: "one()" }, @@ -138,19 +168,20 @@ npm test --workspace @wingdiff/web -- src/diffPresentation.test.ts Expected: FAIL because `./diffPresentation` does not exist. If Vitest reports another reason, fix the test invocation or imports and rerun until the failure is specifically the missing module. -- [ ] **Step 3: Write the failing canonical-fixture test** +- [ ] **Step 3: Write the failing fixture intraline test** -Add this test to `apps/web/src/fixture.test.ts`: +Import `presentDiffLines` into `apps/web/src/fixture.test.ts` and add this test: ```typescript - it("stores changed-line content without unified-diff markers", () => { - const lines = [...tourStops, ...updateStops].flatMap((stop) => stop.evidence.flatMap((evidence) => evidence.lines)); - const markedLines = lines.filter((line) => ( - (line.kind === "addition" && /^[+](?: |$)/.test(line.content)) || - (line.kind === "deletion" && /^[-](?: |$)/.test(line.content)) - )); - - expect(markedLines).toEqual([]); + it("includes an intraline replacement in the update review", () => { + const segments = updateStops.flatMap((stop) => stop.evidence.flatMap((evidence) => ( + presentDiffLines(evidence.lines, evidence.path).flatMap((line) => line.segments ? [line.segments] : []) + ))); + + expect(segments.map((segment) => segment.changed)).toEqual(expect.arrayContaining([ + "Number(await this.redis.get(key) ?? 0) + 1", + "await this.redis.eval(INCREMENT_WITH_TTL, { keys: [key], arguments: [ttlMs] })", + ])); }); ``` @@ -160,14 +191,18 @@ Run: npm test --workspace @wingdiff/web -- src/fixture.test.ts ``` -Expected: FAIL because demo fixture changed lines still contain unified-diff markers. +Expected: FAIL because the update evidence has no equal-length deletion/addition pair, so `presentDiffLines` produces no segments. -- [ ] **Step 4: Normalize the demo fixture and verify GREEN** +- [ ] **Step 4: Add a natural equal-length replacement to the demo fixture and verify GREEN** -Remove only the fixture-only leading `+ ` and `- ` markers from changed-line content: +In the `redis-increment-update` evidence block, replace the unbalanced three-line deletion and five-line addition with a small coherent replacement: -```bash -perl -pi -e 's/content: "\+ /content: "/g; s/content: "- /content: "/g' apps/web/src/fixture.ts +```typescript +{ kind: "context", oldLine: 20, newLine: 20, content: "async increment(subject: string, ttlMs: number) {" }, +{ kind: "deletion", oldLine: 21, content: " const next = Number(await this.redis.get(key) ?? 0) + 1;", emphasized: true }, +{ kind: "addition", newLine: 21, content: " const next = await this.redis.eval(INCREMENT_WITH_TTL, { keys: [key], arguments: [ttlMs] });", emphasized: true }, +{ kind: "context", oldLine: 22, newLine: 22, content: " return next;" }, +{ kind: "context", oldLine: 23, newLine: 23, content: "}" }, ``` Run: @@ -176,7 +211,7 @@ Run: npm test --workspace @wingdiff/web -- src/fixture.test.ts ``` -Expected: PASS. The fixture now matches the server's canonical `DiffLine.content` shape, so the renderer never needs to guess whether a leading `+ ` or `- ` is a marker or legitimate source text. +Expected: PASS. The update review now has a natural equal-length replacement with a shared `const next = ` prefix and `;` suffix, so the browser can show an intraline segment. - [ ] **Step 5: Implement the minimal presentation helper** @@ -260,6 +295,7 @@ function splitChangedSpan( const deletionSegments = segments(deleted, prefix, suffix); const additionSegments = segments(added, prefix, suffix); if (!deletionSegments.changed && !additionSegments.changed) return undefined; + if (!/\S/.test(deletionSegments.before) && !/\S/.test(deletionSegments.after)) return undefined; return { deletion: deletionSegments, addition: additionSegments }; } @@ -280,7 +316,7 @@ Run: npm test --workspace @wingdiff/web -- src/diffPresentation.test.ts ``` -Expected: PASS, 7 tests passed. +Expected: PASS, 10 tests passed. - [ ] **Step 7: Write the failing component and stylesheet tests** @@ -359,6 +395,8 @@ describe("CodeDiff", () => { }); ``` +The stylesheet assertion extracts each changed-span rule and verifies the exact dark/light opaque colors, the two `box-shadow: inset 0 -2px 0 ...` declarations, the absence of `background` in those rules, and the existing whole-row addition/deletion backgrounds. + - [ ] **Step 8: Run the component tests and verify RED** Run: @@ -562,26 +600,38 @@ function splitToken(content: string, token: SyntaxToken, changeStart: number, ch In `apps/web/src/styles.css`, add these variables to the dark `:root` block after `--code-delete-strong`: ```css - --code-add-inline: rgba(106, 226, 160, 0.34); - --code-delete-inline: rgba(255, 126, 116, 0.34); + --code-add-inline: #6ae2a0; + --code-delete-inline: #ff7e74; ``` Add these variables to the light `:root[data-theme="light"]` block after its `--code-delete-strong`: ```css - --code-add-inline: rgba(36, 135, 79, 0.24); - --code-delete-inline: rgba(185, 69, 56, 0.23); + --code-add-inline: #21794e; + --code-delete-inline: #b94538; ``` Add these rules after `.diff-line__sign`: ```css .diff-line__change { padding: 1px 0; border-radius: 2px; } -.diff-line--addition .diff-line__change { background: var(--code-add-inline); } -.diff-line--deletion .diff-line__change { background: var(--code-delete-inline); } +.diff-line--addition .diff-line__change { box-shadow: inset 0 -2px 0 var(--code-add-inline); } +.diff-line--deletion .diff-line__change { box-shadow: inset 0 -2px 0 var(--code-delete-inline); } ``` -- [ ] **Step 11: Run the focused web tests and verify GREEN** +The whole-row addition/deletion backgrounds remain unchanged. Changed spans must not add a text background; the stylesheet test rejects `background` inside both changed-span rules and asserts the exact opaque underline colors and inset rules. + +- [ ] **Step 11: Add the focused browser proof and run focused tests** + +Add a Playwright test in `e2e/reviewer-flow.spec.ts` that opens `/?demo=1`, selects `Review updates`, and verifies that a `.diff-line__change` span is visible, the active header starts with `@ src/infra/redis/attempt-store.ts: @` rather than `@@`, and a line-number button in a changed row can be selected. + +Run the focused browser test: + +```bash +npx playwright test e2e/reviewer-flow.spec.ts --grep "readable, selectable intraline replacement" +``` + +Expected: PASS, 1 test passed. Run: @@ -589,7 +639,7 @@ Run: npm test --workspace @wingdiff/web -- src/diffPresentation.test.ts src/fixture.test.ts src/components/CodeDiff.test.tsx ``` -Expected: PASS, all presentation and component tests pass. +Expected: PASS, 23 presentation, fixture, and component tests across 3 files. - [ ] **Step 12: Document the integration boundary** @@ -598,10 +648,11 @@ Add this paragraph under the `### Read-only change acquisition` section in `READ ```markdown The built-in diff viewer takes a lightweight cue from [diff-so-fancy](https://github.com/so-fancy/diff-so-fancy): paired changed -lines emphasize only the intraline span that differs, and hunk headers are -reduced to a readable `@ path:line @ context` form. Wingdiff does not run the -terminal renderer or consume ANSI output; Git's structured diff remains the -authoritative source for line numbers, fingerprints, and review anchors. +lines with meaningful shared context emphasize only the intraline span that +differs, and hunk headers are reduced to a readable `@ path:line @ context` +form. Wingdiff does not run the terminal renderer or consume ANSI output; +Git's structured diff remains the authoritative source for line numbers, +fingerprints, and review anchors. ``` - [ ] **Step 13: Run the complete repository verification** @@ -633,40 +684,37 @@ npm exec --workspace @wingdiff/web -- vitest run src/diffPresentation.test.ts -- Expected: `diffPresentation.ts` has greater than 90 percent statements, branches, functions, and lines. If any metric is at or below 90 percent, add tests for the uncovered behavior, rerun the focused test to observe it fail when the behavior is absent, then implement and rerun. Report the exact four coverage percentages. -- [ ] **Step 15: Exercise the feature in the local development server** - -Run this command in a dedicated PTY session that remains alive: - -```bash -npm run wingdiff -- --demo --no-open -``` +- [ ] **Step 15: Leave server verification to the controller** -Expected: the command prints an authenticated `Wingdiff is ready at` URL with a -`wingdiff_token` query parameter. Open that URL, choose the demo review, enter -Changed files, and verify that a paired replacement shows a narrower intraline -emphasis and that the hunk header uses `@ path:line @ context` instead of -`@@ ... @@`. Confirm line selection still works. Keep the server session alive for -the user and record its URL, PTY session ID, representative flow, and any -limitation in the report. +Do not leave a development or demo server running. The focused and full +Playwright runs own the browser-level proof and shut their web server down when +they exit. The controller may perform a separate persistent authenticated +server check after the fix wave; record that URL and session only if the check +is delegated. - [ ] **Step 16: Commit the validated change** Use explicit task-owned paths: ```bash -gcommit --no-push -m "feat: add diff-so-fancy-style diff readability" -- docs/projects/diff-so-fancy-integration/plan.md apps/web/src/diffPresentation.ts apps/web/src/diffPresentation.test.ts apps/web/src/fixture.ts apps/web/src/fixture.test.ts apps/web/src/components/CodeDiff.tsx apps/web/src/components/CodeDiff.test.tsx apps/web/src/styles.css README.md +gcommit --no-push -m "fix: refine diff presentation and demo coverage" -- docs/projects/diff-so-fancy-integration/plan.md apps/web/src/diffPresentation.ts apps/web/src/diffPresentation.test.ts apps/web/src/fixture.ts apps/web/src/fixture.test.ts apps/web/src/components/CodeDiff.tsx apps/web/src/components/CodeDiff.test.tsx apps/web/src/styles.css README.md e2e/reviewer-flow.spec.ts ``` -Expected: one local commit containing only the plan, presentation helper and tests, viewer integration, styles, and documentation. Do not push, merge, or deploy. +Expected: one local commit containing only the plan, presentation helper and tests, viewer integration, styles, browser coverage, and documentation. Do not push, merge, or deploy. ## Verification Receipt -The implementer report must record: -- RED command and the exact missing-behavior failure for both TDD cycles -- GREEN focused-test output -- Complete `npm test`, `npm run typecheck`, `npm run build`, and `npm run test:e2e` results -- Exact helper coverage percentages -- Commit SHA and changed paths -- Local URL and PTY session ID for the still-running development server -- Representative demo flow and visual verification result -- Any intentional limitation, especially that diff-so-fancy itself is not bundled +The fix-wave implementer report records: + +- RED/GREEN evidence for exact intraline spans, meaningful-affix suppression, high-contrast underline CSS, the fixture replacement, and the focused Playwright flow. +- Focused web verification: 3 files passed, 23 tests passed. +- Focused demo Playwright verification: 1 test passed. +- Complete `npm test`: 24 server files / 151 tests and 10 web files / 64 tests passed. +- `npm run typecheck`: server and web both passed. +- `npm run build`: web (321 modules) and server both passed. +- Full `npm run test:e2e`: 8 tests passed. +- Helper coverage: statements 100% (50/50), branches 100% (28/28), functions 100% (5/5), lines 100% (40/40). +- The temporary coverage provider was installed without changing package manifests or the lockfile, then `npm ci` restored the locked dependency tree. +- No development server was left running; the controller owns the persistent authenticated server check. +- The final fix commit SHA and changed paths are recorded in `task-1-report.md` after commit. +- Diff-so-fancy itself remains intentionally unbundled; Wingdiff presents structured Git data and does not execute Perl, parse ANSI, or add a runtime dependency. diff --git a/e2e/reviewer-flow.spec.ts b/e2e/reviewer-flow.spec.ts index 2dd00ba..1240a2d 100644 --- a/e2e/reviewer-flow.spec.ts +++ b/e2e/reviewer-flow.spec.ts @@ -65,6 +65,22 @@ test("returns to the top when marking a stop understood", async ({ page }) => { await expect.poll(() => canvas.evaluate((element) => element.scrollTop)).toBe(0); }); +test("shows a readable, selectable intraline replacement in the demo update review", async ({ page }) => { + await page.goto("/?demo=1"); + await page.getByRole("button", { name: "Review updates" }).click(); + + await expect(page.locator(".diff-line__change").first()).toBeVisible(); + const header = page.locator(".diff-line__header").first(); + await expect(header).toHaveText(/^@ src\/infra\/redis\/attempt-store\.ts:\d+ @/); + await expect(header).not.toContainText("@@"); + + const changedRow = page.locator(".diff-line--deletion, .diff-line--addition").first(); + const lineButton = changedRow.locator(".diff-line__number"); + await expect(lineButton).toBeEnabled(); + await lineButton.click(); + await expect(changedRow).toHaveClass(/is-selected/); +}); + test("moves from the final real-review stop directly to the review desk", async ({ page }) => { await mockReviewApi(page, { tourReady: true }); await page.goto("/?session=pilot-session"); From f013d48248be8e4231eeb28f619037df832f47e8 Mon Sep 17 00:00:00 2001 From: Will Date: Sun, 1 Nov 2026 20:50:12 -0600 Subject: [PATCH 3/4] fix: handle deletion-only hunk headers --- apps/web/src/diffPresentation.test.ts | 8 ++ apps/web/src/diffPresentation.ts | 7 +- .../diff-so-fancy-integration/plan.md | 76 ++++++++++++++----- 3 files changed, 71 insertions(+), 20 deletions(-) diff --git a/apps/web/src/diffPresentation.test.ts b/apps/web/src/diffPresentation.test.ts index 7da39d1..80ac476 100644 --- a/apps/web/src/diffPresentation.test.ts +++ b/apps/web/src/diffPresentation.test.ts @@ -99,6 +99,14 @@ describe("presentDiffLines", () => { expect(presented[2]?.content).toBe("Binary file changed"); }); + it("uses the deleted range start when a nonzero new range is empty", () => { + const presented = presentDiffLines([ + { kind: "header", content: "@@ -10,2 +9,0 @@" }, + ], "src/auth/login.ts"); + + expect(presented[0]?.content).toBe("@ src/auth/login.ts:10 @"); + }); + it("does not mutate the supplied evidence lines", () => { const lines: DiffLine[] = [ { kind: "deletion", oldLine: 4, content: "old" }, diff --git a/apps/web/src/diffPresentation.ts b/apps/web/src/diffPresentation.ts index 6182741..d59be18 100644 --- a/apps/web/src/diffPresentation.ts +++ b/apps/web/src/diffPresentation.ts @@ -46,11 +46,12 @@ export function presentDiffLines(lines: DiffLine[], path: string): PresentedDiff } function humanizeHunkHeader(header: string, path: string): string { - const match = /^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@(.*)$/.exec(header); + const match = /^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,(\d+))? @@(.*)$/.exec(header); if (!match) return header; const newStart = Number(match[2]); - const lineNumber = newStart === 0 ? Number(match[1]) : newStart; - const suffix = match[3]!.trimEnd(); + const newLines = match[3] === undefined ? 1 : Number(match[3]); + const lineNumber = newLines === 0 ? Number(match[1]) : newStart; + const suffix = match[4]!.trimEnd(); return `@ ${path}:${lineNumber} @${suffix}`; } diff --git a/docs/projects/diff-so-fancy-integration/plan.md b/docs/projects/diff-so-fancy-integration/plan.md index c30b64f..491d143 100644 --- a/docs/projects/diff-so-fancy-integration/plan.md +++ b/docs/projects/diff-so-fancy-integration/plan.md @@ -30,6 +30,7 @@ - Modify: `apps/web/src/components/CodeDiff.tsx` - Modify: `apps/web/src/components/CodeDiff.test.tsx` - Modify: `apps/web/src/styles.css` +- Modify: `e2e/reviewer-flow.spec.ts` - Modify: `README.md` **Interfaces:** @@ -144,6 +145,14 @@ describe("presentDiffLines", () => { expect(presented[2]?.content).toBe("Binary file changed"); }); + it("uses the deleted range start when a nonzero new range is empty", () => { + const presented = presentDiffLines([ + { kind: "header", content: "@@ -10,2 +9,0 @@" }, + ], "src/auth/login.ts"); + + expect(presented[0]?.content).toBe("@ src/auth/login.ts:10 @"); + }); + it("does not mutate the supplied evidence lines", () => { const lines: DiffLine[] = [ { kind: "deletion", oldLine: 4, content: "old" }, @@ -266,11 +275,12 @@ export function presentDiffLines(lines: DiffLine[], path: string): PresentedDiff } function humanizeHunkHeader(header: string, path: string): string { - const match = /^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@(.*)$/.exec(header); + const match = /^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,(\d+))? @@(.*)$/.exec(header); if (!match) return header; const newStart = Number(match[2]); - const lineNumber = newStart === 0 ? Number(match[1]) : newStart; - const suffix = match[3]!.trimEnd(); + const newLines = match[3] === undefined ? 1 : Number(match[3]); + const lineNumber = newLines === 0 ? Number(match[1]) : newStart; + const suffix = match[4]!.trimEnd(); return `@ ${path}:${lineNumber} @${suffix}`; } @@ -316,7 +326,7 @@ Run: npm test --workspace @wingdiff/web -- src/diffPresentation.test.ts ``` -Expected: PASS, 10 tests passed. +Expected: PASS, 11 tests passed. The deletion-only header test covers both a zero new start (`+0,0`) and a nonzero new start with a zero count (`+9,0`). - [ ] **Step 7: Write the failing component and stylesheet tests** @@ -330,6 +340,11 @@ import { describe, expect, it } from "vitest"; import type { EvidenceBlock } from "../types"; import { CodeDiff } from "./CodeDiff"; +function ruleBody(css: string, selector: string): string { + const escaped = selector.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + return new RegExp(`${escaped}\\s*\\{([^}]*)\\}`).exec(css)?.[1] ?? ""; +} + describe("CodeDiff", () => { const sampleEvidence: EvidenceBlock = { id: "evidence-1", @@ -380,15 +395,22 @@ describe("CodeDiff", () => { expect(html).toContain("new"); }); - it("defines strong deletion tokens and intraline emphasis CSS rules", () => { + it("uses high-contrast intraline underlines without filled backgrounds", () => { const cssPath = resolve(__dirname, "../styles.css"); const css = readFileSync(cssPath, "utf-8"); - - expect(css).toContain("--code-add-inline:"); - expect(css).toContain("--code-delete-inline:"); - expect(css).toContain(".diff-line__change"); - expect(css).toContain(".diff-line--addition .diff-line__change"); - expect(css).toContain(".diff-line--deletion .diff-line__change"); + const additionChange = ruleBody(css, ".diff-line--addition .diff-line__change"); + const deletionChange = ruleBody(css, ".diff-line--deletion .diff-line__change"); + + expect(css).toContain("--code-add-inline: #6ae2a0;"); + expect(css).toContain("--code-delete-inline: #ff7e74;"); + expect(css).toContain("--code-add-inline: #21794e;"); + expect(css).toContain("--code-delete-inline: #b94538;"); + expect(additionChange).toContain("box-shadow: inset 0 -2px 0 var(--code-add-inline);"); + expect(deletionChange).toContain("box-shadow: inset 0 -2px 0 var(--code-delete-inline);"); + expect(additionChange).not.toContain("background"); + expect(deletionChange).not.toContain("background"); + expect(css).toContain(".diff-line--addition { background: var(--code-add); }"); + expect(css).toContain(".diff-line--deletion { background: var(--code-delete); }"); expect(css).toContain(".diff-line--deletion.is-emphasized"); expect(css).toContain(".diff-line--addition.is-emphasized"); }); @@ -623,7 +645,27 @@ The whole-row addition/deletion backgrounds remain unchanged. Changed spans must - [ ] **Step 11: Add the focused browser proof and run focused tests** -Add a Playwright test in `e2e/reviewer-flow.spec.ts` that opens `/?demo=1`, selects `Review updates`, and verifies that a `.diff-line__change` span is visible, the active header starts with `@ src/infra/redis/attempt-store.ts: @` rather than `@@`, and a line-number button in a changed row can be selected. +Add this Playwright test to `e2e/reviewer-flow.spec.ts`: + +```typescript +test("shows a readable, selectable intraline replacement in the demo update review", async ({ page }) => { + await page.goto("/?demo=1"); + await page.getByRole("button", { name: "Review updates" }).click(); + + await expect(page.locator(".diff-line__change").first()).toBeVisible(); + const header = page.locator(".diff-line__header").first(); + await expect(header).toHaveText(/^@ src\/infra\/redis\/attempt-store\.ts:\d+ @/); + await expect(header).not.toContainText("@@"); + + const changedRow = page.locator(".diff-line--deletion, .diff-line--addition").first(); + const lineButton = changedRow.locator(".diff-line__number"); + await expect(lineButton).toBeEnabled(); + await lineButton.click(); + await expect(changedRow).toHaveClass(/is-selected/); +}); +``` + +The proof opens `/?demo=1`, selects `Review updates`, and verifies that a readable intraline replacement is visible, the active header uses the humanized form rather than `@@`, and a line-number button in a changed row remains selectable. Run the focused browser test: @@ -639,7 +681,7 @@ Run: npm test --workspace @wingdiff/web -- src/diffPresentation.test.ts src/fixture.test.ts src/components/CodeDiff.test.tsx ``` -Expected: PASS, 23 presentation, fixture, and component tests across 3 files. +Expected: PASS, 24 presentation, fixture, and component tests across 3 files. - [ ] **Step 12: Document the integration boundary** @@ -706,14 +748,14 @@ Expected: one local commit containing only the plan, presentation helper and tes The fix-wave implementer report records: -- RED/GREEN evidence for exact intraline spans, meaningful-affix suppression, high-contrast underline CSS, the fixture replacement, and the focused Playwright flow. -- Focused web verification: 3 files passed, 23 tests passed. +- RED/GREEN evidence for deletion-only hunk headers, exact intraline spans, meaningful-affix suppression, high-contrast underline CSS, the fixture replacement, and the focused Playwright flow. +- Focused web verification: 3 files passed, 24 tests passed. - Focused demo Playwright verification: 1 test passed. -- Complete `npm test`: 24 server files / 151 tests and 10 web files / 64 tests passed. +- Complete `npm test`: 24 server files / 151 tests and 10 web files / 65 tests passed. - `npm run typecheck`: server and web both passed. - `npm run build`: web (321 modules) and server both passed. - Full `npm run test:e2e`: 8 tests passed. -- Helper coverage: statements 100% (50/50), branches 100% (28/28), functions 100% (5/5), lines 100% (40/40). +- Helper coverage: statements 100% (51/51), branches 96.66% (29/30), functions 100% (5/5), lines 100% (41/41). - The temporary coverage provider was installed without changing package manifests or the lockfile, then `npm ci` restored the locked dependency tree. - No development server was left running; the controller owns the persistent authenticated server check. - The final fix commit SHA and changed paths are recorded in `task-1-report.md` after commit. From beb239806fd6dc5f00b5df0d4f7e601abc85cd30 Mon Sep 17 00:00:00 2001 From: Will Date: Sun, 1 Nov 2026 21:16:53 -0600 Subject: [PATCH 4/4] fix: add forced-colors diff fallback --- apps/web/src/components/CodeDiff.test.tsx | 11 ++++++ apps/web/src/styles.css | 9 +++++ .../diff-so-fancy-integration/plan.md | 34 +++++++++++++++---- 3 files changed, 47 insertions(+), 7 deletions(-) diff --git a/apps/web/src/components/CodeDiff.test.tsx b/apps/web/src/components/CodeDiff.test.tsx index ae7e553..4e9e5fa 100644 --- a/apps/web/src/components/CodeDiff.test.tsx +++ b/apps/web/src/components/CodeDiff.test.tsx @@ -79,4 +79,15 @@ describe("CodeDiff", () => { expect(css).toContain(".diff-line--deletion.is-emphasized"); expect(css).toContain(".diff-line--addition.is-emphasized"); }); + + it("preserves intraline distinction in forced-colors mode", () => { + const cssPath = resolve(__dirname, "../styles.css"); + const css = readFileSync(cssPath, "utf-8"); + const forcedColorsChange = /@media \(forced-colors: active\) \{[\s\S]*?\.diff-line--addition \.diff-line__change,\s*\.diff-line--deletion \.diff-line__change\s*\{([^}]*)\}/.exec(css)?.[1] ?? ""; + + expect(forcedColorsChange).toContain("text-decoration: underline;"); + expect(forcedColorsChange).toContain("text-decoration-color: CanvasText;"); + expect(forcedColorsChange).toContain("text-decoration-thickness: 2px;"); + expect(forcedColorsChange).toContain("text-underline-offset: 2px;"); + }); }); diff --git a/apps/web/src/styles.css b/apps/web/src/styles.css index 2603f6b..393cba8 100644 --- a/apps/web/src/styles.css +++ b/apps/web/src/styles.css @@ -372,6 +372,15 @@ code, kbd { font-family: var(--mono); } .diff-line__change { padding: 1px 0; border-radius: 2px; } .diff-line--addition .diff-line__change { box-shadow: inset 0 -2px 0 var(--code-add-inline); } .diff-line--deletion .diff-line__change { box-shadow: inset 0 -2px 0 var(--code-delete-inline); } +@media (forced-colors: active) { + .diff-line--addition .diff-line__change, + .diff-line--deletion .diff-line__change { + text-decoration: underline; + text-decoration-color: CanvasText; + text-decoration-thickness: 2px; + text-underline-offset: 2px; + } +} .diff-line code { padding-right: 20px; white-space: pre; } .diff-line__header { grid-column: 2 / -1; padding-left: 10px; color: var(--blue); background: var(--blue-soft); font-family: var(--mono); font-size: 8px; } .syntax-keyword { color: #d5a5ee; }.syntax-string { color: #a4d3a2; }.syntax-number { color: #e6bd85; } diff --git a/docs/projects/diff-so-fancy-integration/plan.md b/docs/projects/diff-so-fancy-integration/plan.md index 491d143..a8cf327 100644 --- a/docs/projects/diff-so-fancy-integration/plan.md +++ b/docs/projects/diff-so-fancy-integration/plan.md @@ -414,10 +414,21 @@ describe("CodeDiff", () => { expect(css).toContain(".diff-line--deletion.is-emphasized"); expect(css).toContain(".diff-line--addition.is-emphasized"); }); + + it("preserves intraline distinction in forced-colors mode", () => { + const cssPath = resolve(__dirname, "../styles.css"); + const css = readFileSync(cssPath, "utf-8"); + const forcedColorsChange = /@media \(forced-colors: active\) \{[\s\S]*?\.diff-line--addition \.diff-line__change,\s*\.diff-line--deletion \.diff-line__change\s*\{([^}]*)\}/.exec(css)?.[1] ?? ""; + + expect(forcedColorsChange).toContain("text-decoration: underline;"); + expect(forcedColorsChange).toContain("text-decoration-color: CanvasText;"); + expect(forcedColorsChange).toContain("text-decoration-thickness: 2px;"); + expect(forcedColorsChange).toContain("text-underline-offset: 2px;"); + }); }); ``` -The stylesheet assertion extracts each changed-span rule and verifies the exact dark/light opaque colors, the two `box-shadow: inset 0 -2px 0 ...` declarations, the absence of `background` in those rules, and the existing whole-row addition/deletion backgrounds. +The stylesheet assertions extract each changed-span rule and verify the exact dark/light opaque colors, the two `box-shadow: inset 0 -2px 0 ...` declarations, the absence of `background` in those rules, and the existing whole-row addition/deletion backgrounds. They also require a forced-colors fallback that underlines both addition and deletion changes with the `CanvasText` system color. - [ ] **Step 8: Run the component tests and verify RED** @@ -639,9 +650,18 @@ Add these rules after `.diff-line__sign`: .diff-line__change { padding: 1px 0; border-radius: 2px; } .diff-line--addition .diff-line__change { box-shadow: inset 0 -2px 0 var(--code-add-inline); } .diff-line--deletion .diff-line__change { box-shadow: inset 0 -2px 0 var(--code-delete-inline); } +@media (forced-colors: active) { + .diff-line--addition .diff-line__change, + .diff-line--deletion .diff-line__change { + text-decoration: underline; + text-decoration-color: CanvasText; + text-decoration-thickness: 2px; + text-underline-offset: 2px; + } +} ``` -The whole-row addition/deletion backgrounds remain unchanged. Changed spans must not add a text background; the stylesheet test rejects `background` inside both changed-span rules and asserts the exact opaque underline colors and inset rules. +The whole-row addition/deletion backgrounds remain unchanged. Changed spans must not add a text background; the stylesheet test rejects `background` inside both changed-span rules and asserts the exact opaque underline colors and inset rules. Forced-colors mode suppresses `box-shadow`, so the media fallback conveys the changed span with a 2px text underline using `CanvasText`. - [ ] **Step 11: Add the focused browser proof and run focused tests** @@ -681,7 +701,7 @@ Run: npm test --workspace @wingdiff/web -- src/diffPresentation.test.ts src/fixture.test.ts src/components/CodeDiff.test.tsx ``` -Expected: PASS, 24 presentation, fixture, and component tests across 3 files. +Expected: PASS, 25 presentation, fixture, and component tests across 3 files. - [ ] **Step 12: Document the integration boundary** @@ -748,15 +768,15 @@ Expected: one local commit containing only the plan, presentation helper and tes The fix-wave implementer report records: -- RED/GREEN evidence for deletion-only hunk headers, exact intraline spans, meaningful-affix suppression, high-contrast underline CSS, the fixture replacement, and the focused Playwright flow. -- Focused web verification: 3 files passed, 24 tests passed. +- RED/GREEN evidence for deletion-only hunk headers, exact intraline spans, meaningful-affix suppression, high-contrast underline CSS, the forced-colors text fallback, the fixture replacement, and the focused Playwright flow. +- Focused web verification: 3 files passed, 25 tests passed. - Focused demo Playwright verification: 1 test passed. -- Complete `npm test`: 24 server files / 151 tests and 10 web files / 65 tests passed. +- Complete `npm test`: 24 server files / 151 tests and 10 web files / 66 tests passed. - `npm run typecheck`: server and web both passed. - `npm run build`: web (321 modules) and server both passed. - Full `npm run test:e2e`: 8 tests passed. - Helper coverage: statements 100% (51/51), branches 96.66% (29/30), functions 100% (5/5), lines 100% (41/41). - The temporary coverage provider was installed without changing package manifests or the lockfile, then `npm ci` restored the locked dependency tree. - No development server was left running; the controller owns the persistent authenticated server check. -- The final fix commit SHA and changed paths are recorded in `task-1-report.md` after commit. +- The final P3 accessibility fix adds only the forced-colors stylesheet contract, the fallback rule, and this plan update. Its SHA and changed paths are recorded in `task-1-report.md` after commit. - Diff-so-fancy itself remains intentionally unbundled; Wingdiff presents structured Git data and does not execute Perl, parse ANSI, or add a runtime dependency.