diff --git a/DESIGN.md b/DESIGN.md index cf6d712b..50ec028d 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -342,7 +342,15 @@ such detail into a token. New layout spacing must use the scale. Comment status badges match the same control height. - Resolved comments collapse to their header by default. A labeled chevron expands the body and actions; reopening restores the expanded open state. -- Comment views default to open comments and offer Open, Resolved, and All filters. +- Comment views default to open comments and offer Open, Resolved, Outdated, and + All filters. The Comments panel lists Current comments, then an Outdated section + for comments whose code changed or left the diff. Outdated comments carry an + "Outdated" badge, a muted placement note, and their original code; they stay + out of code lines and render in the file's lineNumber 0 slot. File headers and + the file tree show muted outdated counts beside accent current counts. A + snapshot-notice-style banner points to open outdated comments. "Keep open" + moves an outdated comment to its file on the latest snapshot, still labeled + outdated. Export uses one primary `Copy review` action plus a menu for broader scopes. - Keep `Save comment` enabled when idle; empty submission focuses the labeled editor and shows an inline error. Disable it while saving. Deleting a saved diff --git a/README.md b/README.md index c79c107f..b35ae5c2 100644 --- a/README.md +++ b/README.md @@ -275,8 +275,11 @@ agent-ready XML. Observations are immutable. New content creates another observation; matching submissions associate new provenance with an existing review without changing -its captured diff. Comments and reviewed marks are -stored in SQLite. Display preferences remain in the browser. +its captured diff. Comments and reviewed marks belong to the checkout's stream +(or one piped capture) and are stored in SQLite: later snapshots re-anchor +comments to moved code, flag outdated ones, and keep marks on files whose patch +is unchanged. Deleting one snapshot keeps them; they end with the stream's last +snapshot. Display preferences remain in the browser. ## Piped diffs @@ -327,7 +330,7 @@ WebMCP while the page is open; see [docs/webmcp.md](docs/webmcp.md). Local data lives in `$XDG_STATE_HOME/diffx/state.db`, or `~/.local/state/diffx/state.db`. `--state memory` disables persistence. -Current schema 10 state persists across restarts. Older, newer or unrecognized +Current schema 11 state persists across restarts. Older, newer or unrecognized databases are refused without an automatic reset. To start fresh, stop the server, back up its state directory, and explicitly choose a new database path. Legacy commands and pending producer payloads are retired. diff --git a/apps/web/src/App.tsx b/apps/web/src/App.tsx index ec3cc157..32135182 100644 --- a/apps/web/src/App.tsx +++ b/apps/web/src/App.tsx @@ -1,4 +1,5 @@ import { useReviewState } from "./app-state.tsx"; +import { OutdatedCommentsNotice } from "./CommentsPanel.tsx"; import { DiffWorkspace } from "./DiffWorkspace.tsx"; import { FileExplorerNav, SidebarResizer } from "./FileExplorerNav.tsx"; import { Header } from "./Header.tsx"; @@ -14,6 +15,7 @@ export function App() {
+
diff --git a/apps/web/src/CommentsPanel.tsx b/apps/web/src/CommentsPanel.tsx index d7ccc54f..d7f19532 100644 --- a/apps/web/src/CommentsPanel.tsx +++ b/apps/web/src/CommentsPanel.tsx @@ -1,5 +1,9 @@ import { useState } from "react"; -import { ChevronDownIcon, Trash2Icon } from "lucide-react"; +import { + ChevronDownIcon, + MessageSquareWarningIcon, + Trash2Icon, +} from "lucide-react"; import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; import { @@ -25,32 +29,33 @@ import { useReviewState, } from "./app-state.tsx"; import { DraftComment, ReviewCommentCard } from "./review.tsx"; +import { capabilityEnabled, useSession } from "./session-context.tsx"; import { - anchored, - commentApplicability, + commentStale, + placedInline, + placementFor, type ReviewComment, } from "./review-model.ts"; -type CommentState = "open" | "resolved" | "stale"; -type CommentFilter = CommentState | "all"; +type CommentFilter = "open" | "resolved" | "outdated" | "all"; const COMMENT_FILTERS: readonly CommentFilter[] = [ "open", "resolved", - "stale", + "outdated", "all", ]; const DELETE_FILTERS: readonly CommentFilter[] = [ "all", "open", "resolved", - "stale", + "outdated", ]; function filterLabel(filter: CommentFilter) { if (filter === "open") return "Open"; if (filter === "resolved") return "Resolved"; - if (filter === "stale") return "Stale"; + if (filter === "outdated") return "Outdated"; return "All"; } @@ -63,15 +68,17 @@ export function CommentsPanel() { const { draft } = useDraft(); const review = useReviewState(); const { navigateComment } = useNavigation(); - const state = (comment: ReviewComment): CommentState => - commentApplicability(comment, repository) === "stale" - ? "stale" - : comment.status; + // Lifecycle filters by status; Outdated selects comments written against earlier code. + const matches = (comment: ReviewComment, value: CommentFilter) => + value === "all" || + (value === "outdated" + ? commentStale(comment, repository) + : comment.status === value); const counts = { - open: review.comments.filter((comment) => state(comment) === "open").length, - resolved: review.comments.filter((comment) => state(comment) === "resolved") + open: review.comments.filter((comment) => matches(comment, "open")).length, + resolved: review.comments.filter((comment) => matches(comment, "resolved")) .length, - stale: review.comments.filter((comment) => state(comment) === "stale") + outdated: review.comments.filter((comment) => matches(comment, "outdated")) .length, }; const activeCount = @@ -82,14 +89,11 @@ export function CommentsPanel() { : deleteSelection === "all" ? review.comments.length : counts[deleteSelection]; - const currentRound = review.rounds.find((round) => round.current); - const earlierRounds = review.rounds.filter((round) => !round.current); - const visible = (comment: ReviewComment) => - filter === "all" || filter === state(comment); - const currentComments = currentRound?.comments.filter(visible) ?? []; - const visibleEarlierComments = earlierRounds.reduce( - (count, round) => count + round.comments.filter(visible).length, - 0, + const currentComments = review.currentComments.filter((comment) => + matches(comment, filter), + ); + const outdatedComments = review.outdatedComments.filter((comment) => + matches(comment, filter), ); return ( {draft && - (!anchored(draft, repository) ? ( + (!placedInline(placementFor(draft, repository).applicability) ? ( ) : ( + + )} +
+ ); +} diff --git a/apps/web/src/DiffWorkspace.tsx b/apps/web/src/DiffWorkspace.tsx index 7ffef837..cdb4ab1d 100644 --- a/apps/web/src/DiffWorkspace.tsx +++ b/apps/web/src/DiffWorkspace.tsx @@ -30,7 +30,14 @@ import { DiffToolbar } from "./DiffToolbar.tsx"; import { DiffComparison } from "./DiffComparison.tsx"; import { themesFor } from "./display-options.ts"; import { DraftComment, ReviewCommentCard } from "./review.tsx"; -import type { CommentAnnotation } from "./review-model.ts"; +import { + commentInFile, + commentStale, + placedInline, + placedOutdated, + placementFor, + type CommentAnnotation, +} from "./review-model.ts"; import { ServerMetrics } from "./ServerMetrics.tsx"; import { WorkspaceFeedback } from "./WorkspaceFeedback.tsx"; import { capabilityEnabled, useSession } from "./session-context.tsx"; @@ -234,19 +241,33 @@ export function DiffWorkspace() { ) .filter( (comment) => - comment.path === file.path && + commentInFile(comment, file) && comment.scope === mode && - comment.fingerprint === file.fingerprint && comment.id !== draft?.id, ) - .map((comment) => ({ - side: - comment.target === "file" && file.status === "D" - ? "deletions" - : comment.side, - lineNumber: comment.end, - metadata: { kind: "saved", comment }, - })); + .flatMap((comment): DiffLineAnnotation[] => { + const placement = placementFor(comment, repository); + const fileSide = file.status === "D" ? "deletions" : "additions"; + // Inline comments sit on their (possibly moved) lines; outdated + // ones stay at file level with their original code. + if (placedInline(placement.applicability)) + return [ + { + side: comment.target === "file" ? fileSide : placement.side, + lineNumber: comment.target === "file" ? 0 : placement.end, + metadata: { kind: "saved", comment }, + }, + ]; + return placedOutdated(placement.applicability) + ? [ + { + side: fileSide, + lineNumber: 0, + metadata: { kind: "saved", comment }, + }, + ] + : []; + }); if ( draft && draft.path === file.path && @@ -296,6 +317,7 @@ export function DiffWorkspace() { files, diff.items, review.comments, + repository, draft, mode, collapsed, @@ -553,8 +575,17 @@ export function DiffWorkspace() { const currentDraft = actions.current.draft; const ranges = (context.item.annotations ?? []).flatMap( (annotation): CommentLineRange[] => { - if (annotation.metadata.kind === "saved") - return [annotation.metadata.comment]; + if (annotation.metadata.kind === "saved") { + const { comment } = annotation.metadata; + const placement = placementFor( + comment, + actions.current.repository, + ); + return comment.target !== "file" && + placedInline(placement.applicability) + ? [placement] + : []; + } return currentDraft && currentDraft.path === context.item.id ? [currentDraft] : []; @@ -709,8 +740,25 @@ export function DiffWorkspace() { )} renderHeaderMetadata={(item) => { const file = allFiles.find((file) => file.path === item.id); + const outdated = + commentsEnabled && file + ? review.comments.filter( + (comment) => + comment.scope === mode && + commentInFile(comment, file) && + commentStale(comment, repository), + ).length + : 0; return file ? (
+ {outdated > 0 && ( + + {outdated} outdated + + )} diff --git a/apps/web/src/review-model.ts b/apps/web/src/review-model.ts index a28a6457..0834fd6c 100644 --- a/apps/web/src/review-model.ts +++ b/apps/web/src/review-model.ts @@ -1,20 +1,12 @@ -import type { ReviewComment } from "@diffx/shared"; +import type { ChangedFile, RepositoryDiff, ReviewComment } from "@diffx/shared"; export { - anchored, - commentApplicability, commentContext, formatComments, lineContext, parseComments, - reviewRounds, -} from "@diffx/shared"; -export type { - CommentApplicability, - ReviewComment, - ReviewOrigin, - ReviewRound, } from "@diffx/shared"; +export type { ReviewComment, ReviewOrigin } from "@diffx/shared"; export type CommentAnnotation = | { kind: "saved"; comment: ReviewComment } @@ -27,3 +19,60 @@ export function createCommentId( "", ); } + +export type CommentPlacement = NonNullable; +export type Applicability = CommentPlacement["applicability"]; + +/** + * Server placement in the viewed snapshot. Comments saved since the last + * comment read lack one; they belong to the viewed snapshot by construction, + * and anything else unplaced is conservatively outdated. + */ +export function placementFor( + comment: ReviewComment, + repository: RepositoryDiff | null, +): CommentPlacement { + if (comment.placement) return comment.placement; + return { + applicability: + repository === null + ? "unknown" + : comment.scope !== repository.mode + ? "other-scope" + : comment.versionId === repository.versionId + ? "anchored" + : "outdated", + side: comment.side, + start: comment.start, + end: comment.end, + }; +} + +/** Anchored and moved comments render on their lines. */ +export function placedInline(applicability: Applicability) { + return applicability === "anchored" || applicability === "moved"; +} + +/** Outdated comments stay visible at file level, never on lines. */ +export function placedOutdated(applicability: Applicability) { + return applicability === "outdated" || applicability === "unchanged"; +} + +/** Written against earlier code: outdated now, or kept open from an outdated state. */ +export function commentStale( + comment: ReviewComment, + repository: RepositoryDiff | null, +) { + return ( + comment.carriedFrom !== undefined || + placedOutdated(placementFor(comment, repository).applicability) + ); +} + +/** Stream comments follow renames: the viewed file may carry the comment's path as its old path. */ +export function commentInFile( + comment: ReviewComment, + file: Pick, +) { + return comment.path === file.path || comment.path === file.oldPath; +} diff --git a/apps/web/src/review.css b/apps/web/src/review.css index c2e56089..4ba2e94d 100644 --- a/apps/web/src/review.css +++ b/apps/web/src/review.css @@ -222,6 +222,15 @@ font: var(--text-xs) var(--font-sans); white-space: nowrap; } +.file-comment-count.outdated, +.file-outdated-count { + color: var(--muted); +} +.file-outdated-count { + flex-shrink: 0; + font-size: var(--text-xs); + white-space: nowrap; +} .file-comment-count svg { width: var(--icon-sm); height: var(--icon-sm); @@ -426,12 +435,15 @@ .review-round h2 { margin: 0 0 var(--space-2); } -.earlier-review { +.outdated-review { border-top: 1px solid var(--border); padding-top: var(--space-3); } -.earlier-review summary { - cursor: pointer; +.review-round-hint { + margin: 0 0 var(--space-2); + color: var(--muted); + font-size: var(--text-xs); + line-height: var(--leading-copy); } .comment-card, .comment-editor { @@ -652,7 +664,8 @@ border-radius: var(--radius-sm); } #comments-panel .comment-actions { - flex-wrap: nowrap; + /* Outdated comments add Keep open; wrap instead of overflowing the card. */ + flex-wrap: wrap; min-height: var(--control-height); gap: var(--space-1); padding: var(--space-0-5) var(--space-1-5); diff --git a/apps/web/src/review.tsx b/apps/web/src/review.tsx index e93674c5..07f55c94 100644 --- a/apps/web/src/review.tsx +++ b/apps/web/src/review.tsx @@ -5,6 +5,7 @@ import { CircleDotIcon, CopyIcon, PencilIcon, + PinIcon, RotateCcwIcon, SendIcon, Trash2Icon, @@ -38,11 +39,49 @@ import { } from "./app-state.tsx"; import { useAppearance } from "./appearance-context.tsx"; import { CopyPathButton } from "./CopyPathButton.tsx"; -import { commentApplicability, type ReviewComment } from "./review-model.ts"; +import { + commentInFile, + commentStale, + placedInline, + placedOutdated, + placementFor, + type CommentPlacement, + type ReviewComment, +} from "./review-model.ts"; + +function lines(start: number, end: number) { + return `${start}${end !== start ? `–${end}` : ""}`; +} -function location(comment: ReviewComment) { +// Inline comments name their current lines; outdated ones their original lines. +function location(comment: ReviewComment, placement: CommentPlacement) { if (comment.target === "file") return comment.path; - return `${comment.path}:${comment.start}${comment.end !== comment.start ? `–${comment.end}` : ""}`; + return placedInline(placement.applicability) + ? `${comment.path}:${lines(placement.start, placement.end)}` + : `${comment.path}:${lines(comment.start, comment.end)}`; +} + +function draftLocation(draft: ReviewComment) { + return draft.target === "file" + ? draft.path + : `${draft.path}:${lines(draft.start, draft.end)}`; +} + +function placementNote(comment: ReviewComment, placement: CommentPlacement) { + if (comment.carriedFrom) + return `Outdated — kept open from line ${lines(comment.carriedFrom.start, comment.carriedFrom.end)} of earlier code`; + switch (placement.applicability) { + case "moved": + return `Moved from line ${lines(comment.start, comment.end)}`; + case "outdated": + return "Outdated — the commented code changed or left the diff"; + case "unchanged": + return "Outdated — the commented code is no longer changed"; + case "unknown": + return "Original code preserved"; + default: + return ""; + } } // Both the diff annotations and sidebar edit the same page-level draft. @@ -91,6 +130,7 @@ export function ReviewCommentCard({ if (review.edit(comment)) navigateComment(comment); }} onToggle={review.toggle} + onCarry={review.carry} onDelete={review.remove} onCopy={sidebar ? review.copySidebarComment : review.copyComment} /> @@ -144,7 +184,7 @@ export function CommentEditor({ - {location(draft)} + {draftLocation(draft)} void; onEdit: (comment: ReviewComment) => void; onToggle: (comment: ReviewComment) => void; + onCarry: (comment: ReviewComment) => void; onDelete: (comment: ReviewComment) => void; onCopy: (comment: ReviewComment) => void; }) { const resolved = comment.status === "resolved"; - const applicability = commentApplicability(comment, repository); - const stale = applicability === "stale"; + const placement = placementFor(comment, repository); + const { applicability } = placement; + const stale = commentStale(comment, repository); + const note = + applicability === "other-scope" && sidebar + ? `From ${comment.scope} changes` + : placementNote(comment, placement); + // Keep open re-targets the comment to its file in the viewed snapshot. + const carryable = + !resolved && + !comment.carriedFrom && + placedOutdated(applicability) && + !!repository?.files.some((file) => commentInFile(comment, file)); const [expanded, setExpanded] = useState(!resolved && !stale); const [confirmDelete, setConfirmDelete] = useState(false); const contentId = useId(); @@ -262,7 +315,7 @@ export function CommentCard({ size="icon-xs" aria-controls={contentId} aria-expanded={expanded} - aria-label={`${expanded ? "Collapse" : "Expand"} ${stale ? "stale " : resolved ? "resolved " : ""}comment at ${location(comment)}`} + aria-label={`${expanded ? "Collapse" : "Expand"} ${stale ? "outdated " : resolved ? "resolved " : ""}comment at ${location(comment, placement)}`} onClick={() => setExpanded((current) => !current)} > @@ -314,28 +369,12 @@ export function CommentCard({ >

{comment.body}

- {sidebar && ( - <> - {applicability !== "anchored" && ( -

- {applicability === "other-scope" - ? `From ${comment.scope} changes` - : applicability === "stale" - ? comment.target === "file" - ? "Stale — file changed" - : "Stale — file changed; original code preserved" - : comment.target === "file" - ? "File comment from an earlier diff" - : "Original code preserved"} -

- )} - {comment.target !== "file" && ( -
- Code context -
{comment.code}
-
- )} - + {note &&

{note}

} + {(sidebar || stale) && comment.code && ( +
+ {stale ? "Original code" : "Code context"} +
{comment.code}
+
)}
@@ -344,7 +383,7 @@ export function CommentCard({ className="copy-comment" variant="ghost" size={sidebar ? "icon-xs" : "icon-sm"} - aria-label={`Copy comment at ${location(comment)}`} + aria-label={`Copy comment at ${location(comment, placement)}`} title="Copy comment" onClick={() => onCopy(comment)} > @@ -375,6 +414,18 @@ export function CommentCard({ )} {resolved ? "Reopen" : "Resolve"} + {carryable && ( + + )}