From f13105333abb287f7d32b3e4305f20854cc4c160 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Sat, 4 Jul 2026 15:17:07 +0300 Subject: [PATCH 1/2] =?UTF-8?q?feat(client):=20intraline=20diff=20highligh?= =?UTF-8?q?ting=20in=20the=20suggestion=20before=E2=86=92after=20block=20(?= =?UTF-8?q?#331)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The suggestion block (#315) struck the whole `selection` red and showed the whole `suggestedText` green, so a one-letter edit (заведем→заведём) highlighted the entire line. Now only the CHANGED fragments are emphasized intraline, git-style. Pure, render-only — nothing changes in the DB/backend/MCP/IComment/mutations/ Apply/Badge. New pure `computeSuggestionDiff(old, new) => { old: Segment[], new: Segment[] }` (Segment = {text, changed}) in suggestion.ts: hybrid word+char — `diffWordsWithSpace` for the word skeleton, then `diffChars` inside an adjacent removed+added pair so only the differing letters (not the whole word) are flagged; a lone insertion/deletion is wholly changed; equal parts are common on both sides. Concatenating each side reproduces the input (lossless). Wrapped in `useMemo` on [selection, suggestedText]. comment-list-item.tsx renders per-segment spans instead of two whole ; changed segments get `.suggestionChanged` (a stronger currentColor tint + bold, NO text-decoration so the old block's inherited line-through survives on the changed letters — the whole old line still reads removed, new as added). `diff@8.0.3` (jsdiff, already in the root package.json) added to apps/client/package.json (+ lockfile, additive) so the workspace resolves it; it bundles its own types. Tests: new suggestion.test.ts (one-letter ё/е; word replacement keeping the shared word common with no per-letter noise; word insertion/deletion; identical) — asserts segment text + changed flags, non-vacuous. Two pre-existing comment-list-item.test assertions switched from getByText (a single text node) to container.textContent (the new line is now multiple spans) — adapts to the intended DOM change, not a weakening. Verified: tsc --noEmit clean; client vitest 892 passed | 1 expected-fail. Visual/pixel check of the tint at the 390px comment panel needs a human (no screenshot tooling in-repo). Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/client/package.json | 1 + .../components/comment-list-item.test.tsx | 14 ++- .../comment/components/comment-list-item.tsx | 39 ++++++- .../comment/components/comment.module.css | 15 +++ .../features/comment/utils/suggestion.test.ts | 102 +++++++++++++++++ .../src/features/comment/utils/suggestion.ts | 103 ++++++++++++++++++ pnpm-lock.yaml | 3 + 7 files changed, 267 insertions(+), 10 deletions(-) create mode 100644 apps/client/src/features/comment/utils/suggestion.test.ts diff --git a/apps/client/package.json b/apps/client/package.json index 010cb5e4..9262d7a4 100644 --- a/apps/client/package.json +++ b/apps/client/package.json @@ -40,6 +40,7 @@ "axios": "1.16.0", "blueimp-load-image": "5.16.0", "clsx": "2.1.1", + "diff": "8.0.3", "dompurify": "3.4.1", "file-saver": "2.0.5", "highlightjs-sap-abap": "0.3.0", diff --git a/apps/client/src/features/comment/components/comment-list-item.test.tsx b/apps/client/src/features/comment/components/comment-list-item.test.tsx index 8b75b337..ceb9cbc0 100644 --- a/apps/client/src/features/comment/components/comment-list-item.test.tsx +++ b/apps/client/src/features/comment/components/comment-list-item.test.tsx @@ -108,10 +108,12 @@ describe("CommentListItem — suggested edit (#315)", () => { }); it("renders the было→стало diff and an Apply button when canEdit and not applied/resolved", () => { - renderItem(suggestion(), true); - // Old text appears both as the selection quote and as the struck diff row. + const { container } = renderItem(suggestion(), true); + // Old text appears as the selection quote (a single unsplit Text node). expect(screen.getAllByText("old wording here").length).toBeGreaterThan(0); - expect(screen.getByText("new wording here")).toBeDefined(); + // The new line is now rendered as per-fragment spans (intraline diff, #331), + // so it is no longer a single text node — assert the concatenated content. + expect(container.textContent).toContain("new wording here"); // Apply button is present. expect(screen.getByRole("button", { name: "Apply" })).toBeDefined(); // No Applied badge yet. @@ -119,9 +121,9 @@ describe("CommentListItem — suggested edit (#315)", () => { }); it("hides the Apply button when canEdit is false", () => { - renderItem(suggestion(), false); - // Diff still renders... - expect(screen.getByText("new wording here")).toBeDefined(); + const { container } = renderItem(suggestion(), false); + // Diff still renders (as per-fragment spans, #331)... + expect(container.textContent).toContain("new wording here"); // ...but no Apply button. expect(screen.queryByRole("button", { name: "Apply" })).toBeNull(); }); diff --git a/apps/client/src/features/comment/components/comment-list-item.tsx b/apps/client/src/features/comment/components/comment-list-item.tsx index 0d4b5e02..0e397245 100644 --- a/apps/client/src/features/comment/components/comment-list-item.tsx +++ b/apps/client/src/features/comment/components/comment-list-item.tsx @@ -1,6 +1,6 @@ import { Group, Text, Box, Badge, Button } from "@mantine/core"; import { AgentAvatarStack } from "@/components/ui/agent-avatar-stack.tsx"; -import React, { useEffect, useRef, useState } from "react"; +import React, { useEffect, useMemo, useRef, useState } from "react"; import classes from "./comment.module.css"; import { useAtom, useAtomValue } from "jotai"; import { useTimeAgo } from "@/hooks/use-time-ago"; @@ -17,7 +17,10 @@ import { useUpdateCommentMutation, } from "@/features/comment/queries/comment-query"; import { IComment } from "@/features/comment/types/comment.types"; -import { canShowApply } from "@/features/comment/utils/suggestion"; +import { + canShowApply, + computeSuggestionDiff, +} from "@/features/comment/utils/suggestion"; import { CustomAvatar } from "@/components/ui/custom-avatar.tsx"; import { currentUserAtom } from "@/features/user/atoms/current-user-atom.ts"; import { useTranslation } from "react-i18next"; @@ -54,6 +57,18 @@ function CommentListItem({ const [currentUser] = useAtom(currentUserAtom); const createdAtAgo = useTimeAgo(comment.createdAt); + // Intraline "before -> after" diff (#331) for a suggested edit: only the + // fragments that actually changed get emphasised inside the red/green block, + // instead of striking through / greening the whole line. Memoised on the + // (selection, suggestedText) pair so it recomputes only when they change. + const suggestionDiff = useMemo( + () => + comment.suggestedText != null + ? computeSuggestionDiff(comment.selection ?? "", comment.suggestedText) + : null, + [comment.selection, comment.suggestedText], + ); + useEffect(() => { setContent(comment.content); }, [comment]); @@ -236,12 +251,28 @@ function CommentListItem({ {!comment.parentCommentId && comment.suggestedText && ( {comment.selection && ( + // Old line: read as removed as a whole (line-through/red); only the + // changed fragments carry the extra intraline emphasis. - {comment.selection} + {suggestionDiff?.old.map((segment, index) => ( + + {segment.text} + + ))} )} - {comment.suggestedText} + {suggestionDiff?.new.map((segment, index) => ( + + {segment.text} + + ))} {comment.suggestionAppliedAt ? ( diff --git a/apps/client/src/features/comment/components/comment.module.css b/apps/client/src/features/comment/components/comment.module.css index 2a4b9397..f3e7f04f 100644 --- a/apps/client/src/features/comment/components/comment.module.css +++ b/apps/client/src/features/comment/components/comment.module.css @@ -53,6 +53,21 @@ margin-top: 4px; } +/* Intraline diff (#331): the fragment that actually changed within the + red "before" / green "after" block. It inherits the surrounding red/green + framing and adds a stronger tint plus an underline so the eye lands on the + changed letters/words (git/GitHub-style) rather than the whole line. The + container's line-through (old) / green (new) still marks the full line. */ +.suggestionChanged { + /* Stronger tint of the surrounding red/green so the changed fragment pops + within the block. `currentColor` follows the parent's red (old) or green + (new) text colour. No `text-decoration` here on purpose: the old block's + inherited line-through must survive on the changed letters too. */ + background: color-mix(in srgb, currentColor 22%, transparent); + border-radius: 2px; + font-weight: 700; +} + .commentEditor { &[data-editable][data-surface="muted"] .ProseMirror:not(.focused) { diff --git a/apps/client/src/features/comment/utils/suggestion.test.ts b/apps/client/src/features/comment/utils/suggestion.test.ts new file mode 100644 index 00000000..890e9625 --- /dev/null +++ b/apps/client/src/features/comment/utils/suggestion.test.ts @@ -0,0 +1,102 @@ +import { describe, it, expect } from "vitest"; +import { computeSuggestionDiff, Segment } from "@/features/comment/utils/suggestion"; + +// Reconstruct the plain string from a segment stream — the diff must be +// lossless (concatenating every fragment yields the original input). +const join = (segments: Segment[]): string => + segments.map((s) => s.text).join(""); + +// The subset of segments (in order) that the UI would emphasise. +const changed = (segments: Segment[]): string[] => + segments.filter((s) => s.changed).map((s) => s.text); + +// Find the segment that contains a substring, to assert its `changed` flag. +const segmentWith = (segments: Segment[], needle: string): Segment | undefined => + segments.find((s) => s.text.includes(needle)); + +describe("computeSuggestionDiff", () => { + it("highlights only the single changed letter in a one-letter edit", () => { + const { old, new: neu } = computeSuggestionDiff("заведем", "заведём"); + + // Lossless. + expect(join(old)).toBe("заведем"); + expect(join(neu)).toBe("заведём"); + + // Old side: exactly the `е` is changed, the rest is common. + expect(changed(old)).toEqual(["е"]); + expect(old).toEqual([ + { text: "завед", changed: false }, + { text: "е", changed: true }, + { text: "м", changed: false }, + ]); + + // New side: exactly the `ё` is changed. + expect(changed(neu)).toEqual(["ё"]); + expect(neu).toEqual([ + { text: "завед", changed: false }, + { text: "ё", changed: true }, + { text: "м", changed: false }, + ]); + }); + + it("marks the differing words changed but keeps the shared word common", () => { + const { old, new: neu } = computeSuggestionDiff( + "привет мир", + "здравствуй мир", + ); + + // Lossless. + expect(join(old)).toBe("привет мир"); + expect(join(neu)).toBe("здравствуй мир"); + + // The shared trailing word stays common on both sides (no per-letter noise + // leaking across the differing words into `мир`). + expect(segmentWith(old, "мир")?.changed).toBe(false); + expect(segmentWith(neu, "мир")?.changed).toBe(false); + + // The differing words are emphasised somewhere on each side. + expect(changed(old).length).toBeGreaterThan(0); + expect(changed(neu).length).toBeGreaterThan(0); + expect(changed(old).join("")).toContain("п"); // from `привет` + expect(changed(neu).join("")).toContain("зд"); // from `здравствуй` + + // No changed fragment on either side touches the word `мир`. + expect(changed(old).some((t) => t.includes("мир"))).toBe(false); + expect(changed(neu).some((t) => t.includes("мир"))).toBe(false); + }); + + it("marks a whole inserted word changed and leaves the old line common", () => { + const { old, new: neu } = computeSuggestionDiff("a c", "a b c"); + + expect(join(old)).toBe("a c"); + expect(join(neu)).toBe("a b c"); + + // Old line has no changed fragment (nothing was removed). + expect(changed(old)).toEqual([]); + // The inserted word is the only changed fragment on the new side. + expect(neu).toContainEqual({ text: "b ", changed: true }); + expect(changed(neu)).toEqual(["b "]); + }); + + it("marks a whole deleted word changed and leaves the new line common", () => { + const { old, new: neu } = computeSuggestionDiff("a b c", "a c"); + + expect(join(old)).toBe("a b c"); + expect(join(neu)).toBe("a c"); + + // The deleted word is the only changed fragment on the old side. + expect(old).toContainEqual({ text: "b ", changed: true }); + expect(changed(old)).toEqual(["b "]); + // New line has no changed fragment (nothing was added). + expect(changed(neu)).toEqual([]); + }); + + it("marks everything common for identical strings", () => { + const { old, new: neu } = computeSuggestionDiff("hello", "hello"); + + expect(old).toEqual([{ text: "hello", changed: false }]); + expect(neu).toEqual([{ text: "hello", changed: false }]); + expect(changed(old)).toEqual([]); + expect(changed(neu)).toEqual([]); + }); +}); diff --git a/apps/client/src/features/comment/utils/suggestion.ts b/apps/client/src/features/comment/utils/suggestion.ts index d14dea6e..b353b876 100644 --- a/apps/client/src/features/comment/utils/suggestion.ts +++ b/apps/client/src/features/comment/utils/suggestion.ts @@ -1,3 +1,4 @@ +import { diffWordsWithSpace, diffChars } from "diff"; import { IComment } from "@/features/comment/types/comment.types"; // Whether the suggested-edit (#315) "Apply" button should be shown for a @@ -12,3 +13,105 @@ export function canShowApply(comment: IComment, canEdit?: boolean): boolean { !comment.parentCommentId, ); } + +// One contiguous run of text within a suggestion's "before" or "after" line. +// `changed` marks the fragment that actually differs from the other side, so +// the UI can emphasise only the intraline delta (git/GitHub-style) instead of +// the whole line. +export interface Segment { + text: string; + changed: boolean; +} + +// A pure "before -> after" intraline diff (#331): the old line split into +// common vs. removed-and-changed fragments, and the new line split into common +// vs. added-and-changed fragments. Concatenating each side's `text` reproduces +// the original strings. +export interface SuggestionDiff { + old: Segment[]; + new: Segment[]; +} + +// Push a segment, coalescing runs of the same `changed` flag on the same side +// so the render emits as few spans as possible and tests stay predictable. +function pushSegment(segments: Segment[], text: string, changed: boolean): void { + if (text === "") return; + const last = segments[segments.length - 1]; + if (last && last.changed === changed) { + last.text += text; + } else { + segments.push({ text, changed }); + } +} + +// Compute an intraline diff between the old `selection` and the new +// `suggestedText` of a suggestion. PURE — no React, no DOM, no I/O. +// +// Hybrid word + char algorithm (per #331): +// 1. `diffWordsWithSpace` yields word-granular parts [{value, added, removed}]. +// 2. An ADJACENT removed+added pair (a word replacement) is refined with +// `diffChars`: shared characters stay common, differing characters are +// marked `changed` on their respective side. This is what keeps a +// one-letter edit (заведем -> заведём) from highlighting the whole word. +// 3. A lone `added` (insertion) or lone `removed` (deletion) marks the whole +// fragment `changed`. +// 4. An unchanged part is `common` on both sides. +// +// Rejected alternatives: pure `diffChars` is noisy on word swaps; pure +// `diffWordsWithSpace` highlights the whole word rather than the changed letter. +export function computeSuggestionDiff( + oldStr: string, + newStr: string, +): SuggestionDiff { + const oldSegments: Segment[] = []; + const newSegments: Segment[] = []; + + const parts = diffWordsWithSpace(oldStr, newStr); + + for (let i = 0; i < parts.length; i++) { + const part = parts[i]; + const next = parts[i + 1]; + + // A word replacement: a removed part immediately followed by an added part + // (or the reverse). Refine it character-by-character so only the differing + // letters are highlighted while shared letters stay common. + const isReplacementPair = + next && + ((part.removed && next.added) || (part.added && next.removed)); + + if (isReplacementPair) { + const removedPart = part.removed ? part : next; + const addedPart = part.added ? part : next; + + const charParts = diffChars(removedPart.value, addedPart.value); + for (const cp of charParts) { + if (cp.added) { + pushSegment(newSegments, cp.value, true); + } else if (cp.removed) { + pushSegment(oldSegments, cp.value, true); + } else { + // Shared character: common on both sides. + pushSegment(oldSegments, cp.value, false); + pushSegment(newSegments, cp.value, false); + } + } + + i++; // consume the paired part as well + continue; + } + + if (part.added) { + // Lone insertion: only present in the new line, wholly changed. + pushSegment(newSegments, part.value, true); + } else if (part.removed) { + // Lone deletion: only present in the old line, wholly changed. + pushSegment(oldSegments, part.value, true); + } else { + // Unchanged: common on both sides. + pushSegment(oldSegments, part.value, false); + pushSegment(newSegments, part.value, false); + } + } + + return { old: oldSegments, new: newSegments }; +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 7540bafe..a9642cf2 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -335,6 +335,9 @@ importers: clsx: specifier: 2.1.1 version: 2.1.1 + diff: + specifier: 8.0.3 + version: 8.0.3 dompurify: specifier: 3.4.1 version: 3.4.1 -- 2.52.0 From 94f60cf0ec4fd21cf0758e12a4332f115f127c13 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Sat, 4 Jul 2026 16:21:25 +0300 Subject: [PATCH 2/2] =?UTF-8?q?docs(client):=20fix=20.suggestionChanged=20?= =?UTF-8?q?comment=20=E2=80=94=20bold=20weight,=20not=20underline=20(#331?= =?UTF-8?q?=20review=20F1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The header comment claimed the rule adds 'an underline'; it does not — it adds a color-mix tint + font-weight:700, and the inner comment already notes text- decoration is omitted on purpose. Aligned the header comment with the rule. Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/client/src/features/comment/components/comment.module.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/client/src/features/comment/components/comment.module.css b/apps/client/src/features/comment/components/comment.module.css index f3e7f04f..0f6e4c5e 100644 --- a/apps/client/src/features/comment/components/comment.module.css +++ b/apps/client/src/features/comment/components/comment.module.css @@ -55,7 +55,7 @@ /* Intraline diff (#331): the fragment that actually changed within the red "before" / green "after" block. It inherits the surrounding red/green - framing and adds a stronger tint plus an underline so the eye lands on the + framing and adds a stronger tint plus bold weight so the eye lands on the changed letters/words (git/GitHub-style) rather than the whole line. The container's line-through (old) / green (new) still marks the full line. */ .suggestionChanged { -- 2.52.0