diff --git a/apps/review-desktop/code-oss/src/vs/review/common/reviewLens.test.ts b/apps/review-desktop/code-oss/src/vs/review/common/reviewLens.test.ts index 4470324ed..2c6efa5c4 100644 --- a/apps/review-desktop/code-oss/src/vs/review/common/reviewLens.test.ts +++ b/apps/review-desktop/code-oss/src/vs/review/common/reviewLens.test.ts @@ -24,6 +24,20 @@ test('structural folds within the lens survive, overlapping folds do not', () => assert.ok(!gaps.includes(outside)); }); +test('an exact document lens (tour step, code peek) clips an added file to its +/-3 window; without it, #502 fills the whole open run', () => { + const diff = { + ...plain, + sourceLineAlignment: Array.from({ length: 300 }, (_, i) => [null, i] as const), + contextScopes: { original: [], modified: [[0, 300] as const] }, + }; + const ranges = [{ side: 'head' as const, file: 'new.ts', fromLine: 80, toLine: 87 }]; + // #502 behavior (Diff tab): the added file is one open run, so contextScopes fills it whole. + assert.deepEqual(lensContextGaps(diff, 0, 300, ranges), []); + // exact document lens: withLens strips contextScopes before calling lensContextGaps. + const clipped = lensContextGaps({ ...diff, contextScopes: undefined }, 0, 300, ranges); + assert.deepEqual(clipped.map(gap => [gap.modifiedStart, gap.modifiedCount]), [[1, 76], [91, 210]]); +}); + test('viewed folds never hide an unread counterpart, and do not overlap structural folds', async () => { const { viewedContextGaps } = await import('./reviewLens.js'); const range = (side: 'base' | 'head', fromLine: number, toLine: number) => ({ side, file: 'a.ts', fromLine, toLine }); diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewApiSourceService.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewApiSourceService.ts index c71b01890..963eb375d 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/reviewApiSourceService.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewApiSourceService.ts @@ -281,7 +281,7 @@ export class ReviewApiSourceService extends Disposable implements IReviewApiSour const current = spec.pins ? reviewSourceAnchor(view(), spec.pins) : reviewSourceComparison(view()); const lens: ReviewDiffLens = { id: "document:" + JSON.stringify([spec.path, spec.ranges, spec.pins]), title: spec.path, - reviewId: current.reviewId, version: current.version, + reviewId: current.reviewId, version: current.version, exact: true, ranges: spec.ranges.map(range => ({ file: spec.path, side: range.side ?? spec.side, fromLine: range.startLine, toLine: range.endLine })), }; return { lens, source: makeSource(() => current) }; diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewLens.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewLens.ts index 8cc8a33d2..1b73282bc 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/reviewLens.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewLens.ts @@ -59,7 +59,10 @@ export function withLens(instantiation: IInstantiationService, entries: readonly }) ? { ...gap, collapsed: false } : gap) }; const section = progress()?.sections?.find(section => section.id === entry.sectionId); - return lens && !lens.wholeFiles ? { ...diff, contextGaps: lensContextGaps(diff, original.getLineCount(), modified.getLineCount(), lensRanges(section ? { ...lens, ranges: section.sources } : lens, entry)) } : diff; + // An exact lens (a tour step, a code peek) is authored to show one chunk; + // diffr's structural scope expansion is right for the Diff tab, not here. + const scopedDiff = lens?.exact ? { ...diff, contextScopes: undefined } : diff; + return lens && !lens.wholeFiles ? { ...diff, contextGaps: lensContextGaps(scopedDiff, original.getLineCount(), modified.getLineCount(), lensRanges(section ? { ...lens, ranges: section.sources } : lens, entry)) } : diff; }, }; }, diff --git a/packages/review-protocol/src/contracts.ts b/packages/review-protocol/src/contracts.ts index b9335aa6d..91021b612 100644 --- a/packages/review-protocol/src/contracts.ts +++ b/packages/review-protocol/src/contracts.ts @@ -192,6 +192,8 @@ export interface ReviewInlineEditorFactory { export interface ReviewDiffLens { /** Filter files while retaining ordinary diff context/folding within them. */ wholeFiles?: boolean; + /** Show only the pinned range, ignoring diffr's structural scope expansion. */ + exact?: boolean; id: string; title: string; reviewId: string; diff --git a/packages/review/app/src/CodePeek.browser.test.tsx b/packages/review/app/src/CodePeek.browser.test.tsx index 2000a3fd6..2cd0ee3f0 100644 --- a/packages/review/app/src/CodePeek.browser.test.tsx +++ b/packages/review/app/src/CodePeek.browser.test.tsx @@ -7,7 +7,12 @@ import { createRoot } from "react-dom/client"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { selectSource, sourceAnchors } from "../../src/lens-selection"; -import { CodePeek, CodePeekCard, CodePeekGroup } from "./CodePeek"; +import { + CodePeek, + CodePeekCard, + CodePeekGroup, + CodePeekStack, +} from "./CodePeek"; import { type ReviewSession, ReviewSessionProvider, @@ -216,6 +221,52 @@ describe("CodePeek native editor", () => { }); }); + it("shows same-file chunks in one editor and other files in their own", async () => { + const chunk = (file: string, fromLine: number, toLine: number) => + selectSource({ side: "head", file, fromLine, toLine }); + + const container = document.createElement("div"); + document.body.append(container); + root = createRoot(container); + + await act(async () => + renderWithSession( + , + ), + ); + + await vi.waitFor(() => { + expect(created).toMatchObject([ + { + path: "src/a.ts", + title: "src/a.ts:1-3, 20-22", + ranges: [ + { startLine: 1, endLine: 3 }, + { startLine: 20, endLine: 22 }, + ], + }, + { path: "src/b.ts", title: "src/b.ts:5" }, + ]); + }); + + await act(async () => { + created[0]?.onDidOpen?.(); + }); + + expect(posted.at(-1)).toMatchObject({ + name: "reveal", + args: { path: "src/a.ts", startLine: 1, endLine: 3 }, + }); + }); + it("gives range side peeks a source title and content height policy", async () => { const input = { side: "head", diff --git a/packages/review/app/src/CodePeek.tsx b/packages/review/app/src/CodePeek.tsx index 193df3e3f..9569cabb8 100644 --- a/packages/review/app/src/CodePeek.tsx +++ b/packages/review/app/src/CodePeek.tsx @@ -9,6 +9,7 @@ import { type DiffSelection, selectionKey, sourceAnchor, + sourcePinsKey, } from "../../src/lens-selection"; import type { ReviewComponentProps } from "../../src/review-document-data"; import { type FileLineRange, codePeekSource } from "../../src/source"; @@ -116,28 +117,82 @@ export function ReviewCodePeek({ anchor }: ReviewComponentProps<"CodePeek">) { return ; } +interface CodePeekCardOptions { + active?: boolean; + heightMode?: ReviewInlineEditorHeightMode; + onNativeFocus?: () => void; + lenses?: ReviewLensView; + /** Report resolution telemetry. Only the panel the user opened sets this, + * so an authored document with many inline peeks sends one event, not N. */ + reportOutcome?: boolean; +} + /** A document peek is an interval of the same alignment used by diff lenses. */ export function CodePeekCard({ source, + ...options +}: CodePeekCardOptions & { source: DiffSelection }) { + const sources = useMemo(() => [source], [source]); + + return ; +} + +/** Several chunks, one card per file: chunks in the same file share a card so + * its lens folds the code between them. */ +export function CodePeekStack({ + sources, + ...options +}: CodePeekCardOptions & { sources: readonly DiffSelection[] }) { + const groups = useMemo(() => codePeekFileGroups(sources), [sources]); + + return ( +
+ {groups.map((group) => ( + + ))} +
+ ); +} + +/** Chunks group by file, diff side and pins, in the order each group first + * appears. */ +export function codePeekFileGroups( + sources: readonly DiffSelection[], +): DiffSelection[][] { + const groups = new Map(); + + for (const source of sources) { + const key = JSON.stringify([ + source.file, + sourceAnchor(source).side, + source.pins ? sourcePinsKey(source.pins) : null, + ]); + + groups.set(key, [...(groups.get(key) ?? []), source]); + } + + return [...groups.values()]; +} + +/** Every source names the same file, side and pins; the first one anchors + * the card's jump-to-source. */ +function CodePeekFileCard({ + sources, active = false, heightMode = "capped", onNativeFocus, lenses: lensesOverride, reportOutcome = false, -}: { - source: DiffSelection; - active?: boolean; - heightMode?: ReviewInlineEditorHeightMode; - onNativeFocus?: () => void; - lenses?: ReviewLensView; - /** Report resolution telemetry. Only the panel the user opened sets this, - * so an authored document with many inline peeks sends one event, not N. */ - reportOutcome?: boolean; -}) { +}: CodePeekCardOptions & { sources: readonly DiffSelection[] }) { const session = useReviewSession(); const contextLenses = useReviewLenses(); const lenses = lensesOverride ?? contextLenses; - const resolved = lenses?.resolve([source]) ?? []; + const resolved = lenses?.resolve(sources) ?? []; + const source = sources[0]!; const anchor = sourceAnchor(source); const ranges = resolved.map((range) => ({ @@ -146,12 +201,14 @@ export function CodePeekCard({ endLine: range.toLine, })); - const key = selectionKey(source); + const key = sources.map(selectionKey).join("\n"); const outcome = peekResolutionOutcome({ resolvedCount: ranges.length, complete: Boolean(lenses?.progress?.complete), - unavailable: Boolean(lenses?.progress?.unavailableSelections?.[key]), + unavailable: sources.some( + (item) => lenses?.progress?.unavailableSelections?.[selectionKey(item)], + ), error: Boolean(lenses?.error), }); @@ -181,15 +238,7 @@ export function CodePeekCard({
source.start.side !== source.end.side)) + return file; + + const lines = sources.map(({ start, end }) => + start.line === end.line ? `${start.line}` : `${start.line}-${end.line}`, + ); + + return `${file}:${lines.join(", ")}`; +} + function FileSnippetCard({ source, active = false, diff --git a/packages/review/app/src/code-peek-groups.test.ts b/packages/review/app/src/code-peek-groups.test.ts new file mode 100644 index 000000000..d2030650d --- /dev/null +++ b/packages/review/app/src/code-peek-groups.test.ts @@ -0,0 +1,48 @@ +import { describe, expect, it } from "vitest"; + +import type { DiffSelection } from "../../src/lens-selection"; +import { codePeekFileGroups } from "./CodePeek"; + +const chunk = ( + file: string, + line: number, + extra: Partial = {}, +): DiffSelection => ({ + file, + start: { side: "head", line }, + end: { side: "head", line }, + ...extra, +}); + +describe("code peek file groups", () => { + it("puts chunks from one file in one card and other files in their own", () => { + const first = chunk("src/a.ts", 10); + const other = chunk("src/b.ts", 3); + const second = chunk("src/a.ts", 40); + + expect(codePeekFileGroups([first, other, second])).toEqual([ + [first, second], + [other], + ]); + }); + + it("keeps chunks apart when their side or pins differ", () => { + const head = chunk("src/a.ts", 10); + + const base = chunk("src/a.ts", 10, { + start: { side: "base", line: 10 }, + end: { side: "base", line: 10 }, + pins: { repositoryId: "repo", base: "b1", head: "h1" }, + }); + + const pinned = chunk("src/a.ts", 20, { + pins: { repositoryId: "repo", head: "h2" }, + }); + + expect(codePeekFileGroups([head, base, pinned])).toEqual([ + [head], + [base], + [pinned], + ]); + }); +}); diff --git a/packages/review/app/src/diagrams.tsx b/packages/review/app/src/diagrams.tsx index 308bdd1d4..12db820fb 100644 --- a/packages/review/app/src/diagrams.tsx +++ b/packages/review/app/src/diagrams.tsx @@ -30,7 +30,11 @@ import { DiagramTourOverlay, useDiagramTourShell } from "./diagram-tour"; import { useMotionPhase } from "./draw-queue-provider"; import { useReviewSession } from "./host/review-session"; import { useReviewPanel } from "./review-panel"; -import type { GuidedTour, PeekAnchor } from "./review-panel-model"; +import type { + GuidedTour, + PeekAnchor, + ReviewPeekContent, +} from "./review-panel-model"; import { useTourPersist, useTourRestore } from "./review-view-state"; import { captureUiEvent } from "./ui-telemetry"; @@ -90,7 +94,10 @@ export interface SequenceMessage { to: SequenceParticipant; label: string; style: Step["style"]; + /** The first chunk, where a single anchor is needed. */ source?: Step["source"]; + /** Every chunk the step shows, in authored order. */ + sources?: NonNullable[]; code?: Step["code"]; explanation?: string; } @@ -120,7 +127,12 @@ export function sequenceView(block: SequenceDiagramProps): SequenceView { style: step.style, }; - if (step.source) message.source = step.source; + const sources = step.sources ?? (step.source ? [step.source] : []); + + if (sources.length) { + message.source = sources[0]; + message.sources = sources; + } if (step.code) message.code = step.code; @@ -159,15 +171,22 @@ export function createSequenceTourEntry(sequence: SequenceView): GuidedTour { anchor: panelAnchor(message), label: message.label, detail: `${message.from.label} -> ${message.to.label}`, - content: message.code - ? { kind: "inline-code" as const, ...message.code } - : message.source - ? { kind: "source" as const, source: message.source } - : { kind: "explanation" as const, text: message.explanation }, + content: sequenceStopContent(message), })), }; } +function sequenceStopContent(message: SequenceMessage): ReviewPeekContent { + if (message.code) return { kind: "inline-code", ...message.code }; + + if (message.sources && message.sources.length > 1) + return { kind: "sources", sources: message.sources }; + + if (message.source) return { kind: "source", source: message.source }; + + return { kind: "explanation", text: message.explanation }; +} + function participantsForMessages( messages: SequenceMessage[], ): SequenceParticipant[] { diff --git a/packages/review/app/src/review-components.tsx b/packages/review/app/src/review-components.tsx index 2c092e66c..e981e4591 100644 --- a/packages/review/app/src/review-components.tsx +++ b/packages/review/app/src/review-components.tsx @@ -9,7 +9,7 @@ import { useEffect, useEffectEvent, useMemo, useRef, useState } from "react"; import type { ReviewComponentProps } from "../../src/review-document-data"; import { AuthoredCodeSurface } from "./authored-code-surface"; -import { CodePeekCard } from "./CodePeek"; +import { CodePeekCard, CodePeekStack } from "./CodePeek"; import { findWhitespaceNormalizedSpan } from "./highlighted-text"; import { useOptionalReviewSession, @@ -613,7 +613,7 @@ function CodeReviewPeekPanel({ anchor: PeekAnchor; content: Extract< ReviewPeekContent, - { kind: "source" | "inline-code" | "explanation" } + { kind: "source" | "sources" | "inline-code" | "explanation" } >; onClose: () => void; }) { @@ -1038,6 +1038,18 @@ function ReviewPeekContentView({ ); } + if (content.kind === "sources") { + return ( + + ); + } + if (content.kind === "inline-code") { return ( { ).toEqual(["request", "request--sequence-use-2"]); }); + it("shows every chunk of a multi-chunk step in one tour stop", () => { + const chunk = (file: string, line: number) => ({ + file, + start: { side: "head" as const, line }, + end: { side: "head" as const, line: line + 2 }, + }); + + const chunks = [chunk("src/a.ts", 10), chunk("src/a.ts", 40)]; + + const sequence = sequenceView({ + id: "diagram-1", + title: "Save", + actors: { app: "App", db: "Database" }, + steps: [ + { + id: "step-1", + type: "step", + from: "app", + to: "db", + label: "write", + style: "call", + sources: chunks, + }, + { + id: "step-2", + type: "step", + from: "db", + to: "app", + label: "ack", + style: "return", + sources: [chunk("src/b.ts", 5)], + }, + ], + }); + + const [write, ack] = createSequenceTourEntry(sequence).stops; + + expect(write).toMatchObject({ + anchor: { id: "step-1", peek: chunks[0] }, + content: { kind: "sources", sources: chunks }, + }); + expect(ack).toMatchObject({ + anchor: { id: "step-2", peek: chunk("src/b.ts", 5) }, + content: { kind: "source", source: chunk("src/b.ts", 5) }, + }); + }); + it("calculates scroll targets that reveal the active message participants", () => { const actors = defineActors({ reviewer: { label: "Reviewer" }, diff --git a/packages/review/app/src/styles.css b/packages/review/app/src/styles.css index 4539c2fb0..eb869e077 100644 --- a/packages/review/app/src/styles.css +++ b/packages/review/app/src/styles.css @@ -3299,6 +3299,13 @@ button { min-width: 0; } +.code-peek-stack { + display: flex; + flex-direction: column; + gap: 14px; + min-width: 0; +} + .code-peek { min-width: 0; max-width: 100%; diff --git a/packages/review/instructions/authoring.md b/packages/review/instructions/authoring.md index 78e7ded6f..f431affe0 100644 --- a/packages/review/instructions/authoring.md +++ b/packages/review/instructions/authoring.md @@ -35,3 +35,5 @@ follow these first six steps exactly, without any extraneous tool calls. - keep whiteboards short and sweet when possible (esp. for small changes.) feel free to omit sections. - when something (a phrase in the prose, diagram node, etc.) describes actual code in the codebase, always default to attaching/hyperlink code. - Link repository code as `[label](review-source:head/src/file.ts#L10-L24)`; use `base` for old code. Use repository-relative paths and verified line numbers. +- point each code reference (diagram step, call-stack frame, `code_peek`) at the smallest range that shows the claim, usually 3-15 lines: the call, the branch, the assignment. not the whole function. tour steps and peeks show only that range. +- when a sequence step's story spans several places (a setting, its gate, its effect), give it `sources` instead of one wide range. list them in reading order. chunks in one file share a card with the gap folded; chunks in other files stack. diff --git a/packages/review/instructions/scratchpad.md b/packages/review/instructions/scratchpad.md index ffcdf62f6..4ff415d51 100644 --- a/packages/review/instructions/scratchpad.md +++ b/packages/review/instructions/scratchpad.md @@ -14,7 +14,7 @@ Answer in chat when one sentence or one code line does it. Do not use the scratc ## How 1. `session_capabilities({})`. Draw only when `desktopAvailable` and `scratchpadEnabled` are true. Otherwise answer in chat; if the pad is off, say once that it can be turned on in Whiteboard Settings. -2. Every source reference names its own pins. For each repository you will quote, `session_register_repository({path})`, then `session_resolve_pins({repositoryId, base: "HEAD", head: "HEAD"})` (or the commit the user is looking at) to get commit ids. Put `pins: {repositoryId, head}` on each `code_peek` source, sequence step source, flow attachment source, call-stack frame source and database operation source; add `base` only when the block compares two commits. Put the same `pins` on a `markdown` block so its `review-source:` links resolve there. A reference without pins is rejected, since the scratchpad has none to lend. +2. Every source reference names its own pins. For each repository you will quote, `session_register_repository({path})`, then `session_resolve_pins({repositoryId, base: "HEAD", head: "HEAD"})` (or the commit the user is looking at) to get commit ids. Put `pins: {repositoryId, head}` on each `code_peek` source, sequence step source (or each of its `sources`), flow attachment source, call-stack frame source and database operation source; add `base` only when the block compares two commits. Put the same `pins` on a `markdown` block so its `review-source:` links resolve there. A reference without pins is rejected, since the scratchpad has none to lend. 3. Read what you cite with `session_file`, `session_tree` or `session_source` at those pins: pass `repositoryId` and `head` (and `base`) to `session_file` and `session_tree`, or `source.pins` to `session_source`. 4. Begin `session_activity({sessionId: "scratchpad", action: "begin", leaseId})` with a fresh UUID, then append blocks with `session_edit({sessionId: "scratchpad", edit: {type: "insert", content}})`. Omitted placement puts the block at the top of the pad, and the response's `targetId` names it. A thought that spans several blocks reads top-down only if you insert it bottom-up (last block first) or give each following block `afterId` of the block you just inserted. Draw a diagram whole in one insert; fix one you drew earlier by patching its nodes, edges or steps by ID, and `replace` a block only when the whole picture was wrong. The block shapes are the same as a session's; follow the edit tool's description for them, and read `session_get_instructions({topic:"authoring"})` for choosing components. End the lease when finished. 5. After your first insert in a session, `session_open({sessionId: "scratchpad"})` once so the pad is showing. Do not call it again for later edits; they appear live. diff --git a/packages/review/src/review-api/authoring-pitfalls.test.ts b/packages/review/src/review-api/authoring-pitfalls.test.ts index 029295f5b..070e942e2 100644 --- a/packages/review/src/review-api/authoring-pitfalls.test.ts +++ b/packages/review/src/review-api/authoring-pitfalls.test.ts @@ -254,10 +254,10 @@ describe("diagram rules", () => { "Unknown component name: cache", )); - it("rejects a step with none or two of source, explanation and code", async () => { + it("rejects a step with none or two of source, sources, explanation and code", async () => { await expectRejected( () => insert(sequence([{ from: "app", to: "db", label: "Write" }])), - "A step needs exactly one of source, explanation, or code.", + "A step needs exactly one of source, sources, explanation, or code.", ); await expectRejected( () => @@ -272,7 +272,39 @@ describe("diagram rules", () => { }, ]), ), - "A step needs exactly one of source, explanation, or code.", + "A step needs exactly one of source, sources, explanation, or code.", + ); + await expectRejected( + () => + insert( + sequence([ + { + from: "app", + to: "db", + label: "Write", + source: selectSource(head("src/store.ts", 1)), + sources: [selectSource(head("src/store.ts", 2))], + }, + ]), + ), + "A step needs exactly one of source, sources, explanation, or code.", + ); + }); + + it("stores a step that carries several code chunks", async () => { + const sources = [ + selectSource(head("src/store.ts", 1)), + selectSource(head("src/store.ts", 3)), + selectSource(head("order.ts", 1)), + ]; + + const result = await insert( + sequence([{ from: "app", to: "db", label: "Write", sources }]), + ); + + expect(result.status).toBe(200); + expect(elements(local.store.read(reviewId).document)).toContainEqual( + expect.objectContaining({ type: "step", sources }), ); }); @@ -343,6 +375,26 @@ describe("source rules in every peek position", () => { }), message, ); + await expectRejected( + () => + insert({ + type: "sequence", + title: "Save", + actors: { app: "App" }, + steps: [ + { + from: "app", + to: "app", + label: "Write", + sources: [ + selectSource(head("src/store.ts", 1)), + selectSource(blank), + ], + }, + ], + }), + message, + ); await expectRejected( () => insert({ diff --git a/packages/review/src/review-api/authoring-tools.ts b/packages/review/src/review-api/authoring-tools.ts index 50880d34f..b0c1132d7 100644 --- a/packages/review/src/review-api/authoring-tools.ts +++ b/packages/review/src/review-api/authoring-tools.ts @@ -25,7 +25,7 @@ export function authoringTools( 'Create a review of saved working files, immutable commits or a GitHub PR. Revisions are resolved on acceptance. Omitted commits base means source at head with no diff; supply the parent to review introduced changes. For a GitHub PR, pullRequestUrl alone is enough: target and title become optional, and the host fetches the PR into a registered checkout of its repository and pins the current PR head and GitHub diff base, titled from the PR. When a review for that PR exists, it is returned instead, reporting whether its head moved and whether another session owns it; update it in place, move its target with review_set_target, or create a separate review with reuseExisting. kind:"scratchpad" names the one scratchpad, which the host creates itself. The result carries review, the review as review_list shows it: its target with resolved commits, origin (its PR), repositoryName and repositoryPath, so no follow-up read is needed before diffing. When Desktop is available the review opens there and the result reports opened, softwareMapEnabled and environmentIssues, as review_open does; set open:false to author in the background without taking over Desktop.', set_target: "Change the review target, preserving document and component IDs. Returns warnings for source references needing repair. Earlier versions keep their retained source.", - edit: 'Edit a document component. Common content shapes: section `{"type":"section","title":"…","children":[]}`; prose `{"type":"markdown","markdown":"…"}`. Sections require `children` (not `blocks`); Markdown uses `markdown` (not `text`). To populate a section later, insert content with `edit.parentId` set to its returned ID. Flow diagram: `{"type":"flow_diagram","title":"Flow","nodes":[{"key":"a","label":"Start"},{"key":"b","label":"Finish"}],"edges":[{"from":"a","to":"b"}]}`. Use nodes and edges arrays (not children); at least one node is required, edges may be empty, and edge endpoints name unique node keys. Code peek: `{"type":"code_peek","source":{"file":"src/app.ts","start":{"side":"head","line":10},"end":{"side":"head","line":20}}}`. Call stack diff: `{"type":"call_stack_diff","title":"Request flow","base":[{"label":"Entry","source":{"file":"src/app.ts","start":{"side":"base","line":10},"end":{"side":"base","line":20}}}],"head":[{"label":"Entry","source":{"file":"src/app.ts","start":{"side":"head","line":10},"end":{"side":"head","line":20}}}]}`. Both base and head arrays are required; either may be empty. Each frame requires source; optional key/parentKey express nesting, and parentKey must name an earlier frame on that side. Sequence: `{"type":"sequence","title":"Request","actors":{"a":"Client","b":"Server"},"steps":[{"from":"a","to":"b","label":"Send","explanation":"Client sends a request."}]}`. actors is a key-to-label object; steps reference actor keys and each needs exactly one of source, explanation, or code. Database lens: `{"type":"database_lens","title":"Read items","actors":{"app":"App"},"stores":{"db":{"label":"DB","storage":"relational","collections":{"items":{"label":"Items","fields":{"id":{"label":"ID","dataType":"integer"}}}}}},"useCases":[{"label":"Load","operations":[{"kind":"read","actor":"app","store":"db","collection":"items","label":"Fetch items","source":{"file":"src/app.ts","start":{"side":"head","line":10},"end":{"side":"head","line":20}}}]}]}`. actors, stores, collections, and fields are keyed objects; useCases and operations are arrays. Include at least one store and use case, with at least one operation per use case; each operation requires source and must reference declared keys. Source paths are repository-relative; line numbers are 1-based and inclusive. Replace example paths and lines with verified source ranges. The host assigns short durable IDs; use returned IDs to edit components in place. The result identifies the edited component and, for an insert or replace, its first-level children, so they can be edited without a follow-up read. Accepted edits are saved immediately. Omitted placement appends; on the scratchpad it lands at the top, so insert a multi-block thought bottom-up or chain each block with afterId. null removes an optional field in a patch. While a reader may be watching, write small and often: one paragraph per edit, so the document draws itself as you go. Insert a new diagram whole, with all its nodes and edges or steps; the board traces it in one quick pass. Change a diagram already on the board one unit at a time: insert, update or remove a flow_node, flow_edge or step by ID (parentId names the diagram). Link each added flow_node to a node already drawn, so it arrives attached; a separate flow_edge is only for two nodes that already exist. Removing a flow_node removes its edges.', + edit: 'Edit a document component. Common content shapes: section `{"type":"section","title":"…","children":[]}`; prose `{"type":"markdown","markdown":"…"}`. Sections require `children` (not `blocks`); Markdown uses `markdown` (not `text`). To populate a section later, insert content with `edit.parentId` set to its returned ID. Flow diagram: `{"type":"flow_diagram","title":"Flow","nodes":[{"key":"a","label":"Start"},{"key":"b","label":"Finish"}],"edges":[{"from":"a","to":"b"}]}`. Use nodes and edges arrays (not children); at least one node is required, edges may be empty, and edge endpoints name unique node keys. Code peek: `{"type":"code_peek","source":{"file":"src/app.ts","start":{"side":"head","line":10},"end":{"side":"head","line":20}}}`. Call stack diff: `{"type":"call_stack_diff","title":"Request flow","base":[{"label":"Entry","source":{"file":"src/app.ts","start":{"side":"base","line":10},"end":{"side":"base","line":20}}}],"head":[{"label":"Entry","source":{"file":"src/app.ts","start":{"side":"head","line":10},"end":{"side":"head","line":20}}}]}`. Both base and head arrays are required; either may be empty. Each frame requires source; optional key/parentKey express nesting, and parentKey must name an earlier frame on that side. Sequence: `{"type":"sequence","title":"Request","actors":{"a":"Client","b":"Server"},"steps":[{"from":"a","to":"b","label":"Send","explanation":"Client sends a request."}]}`. actors is a key-to-label object; steps reference actor keys and each needs exactly one of source, sources, explanation, or code. Use `sources` (an array of up to 10 ranges) when one step spans several code chunks; its tour stop shows them all, with chunks from one file in one card. Database lens: `{"type":"database_lens","title":"Read items","actors":{"app":"App"},"stores":{"db":{"label":"DB","storage":"relational","collections":{"items":{"label":"Items","fields":{"id":{"label":"ID","dataType":"integer"}}}}}},"useCases":[{"label":"Load","operations":[{"kind":"read","actor":"app","store":"db","collection":"items","label":"Fetch items","source":{"file":"src/app.ts","start":{"side":"head","line":10},"end":{"side":"head","line":20}}}]}]}`. actors, stores, collections, and fields are keyed objects; useCases and operations are arrays. Include at least one store and use case, with at least one operation per use case; each operation requires source and must reference declared keys. Source paths are repository-relative; line numbers are 1-based and inclusive. Replace example paths and lines with verified source ranges. The host assigns short durable IDs; use returned IDs to edit components in place. The result identifies the edited component and, for an insert or replace, its first-level children, so they can be edited without a follow-up read. Accepted edits are saved immediately. Omitted placement appends; on the scratchpad it lands at the top, so insert a multi-block thought bottom-up or chain each block with afterId. null removes an optional field in a patch. While a reader may be watching, write small and often: one paragraph per edit, so the document draws itself as you go. Insert a new diagram whole, with all its nodes and edges or steps; the board traces it in one quick pass. Change a diagram already on the board one unit at a time: insert, update or remove a flow_node, flow_edge or step by ID (parentId names the diagram). Link each added flow_node to a node already drawn, so it arrives attached; a separate flow_edge is only for two nodes that already exist. Removing a flow_node removes its edges.', lens: 'Edit one Diff-view lens. Lenses partition the review\'s change for the Diff view; they sit beside the document (never in it) and version with it. The host assigns durable lens IDs; updates replace only the fields supplied. Write one lens per call while a reader may be watching; each draws in on the Diffs page. Requires the lenses lease: review_activity with scope:"lenses", which another agent can hold while the document lease is held elsewhere. The result identifies the lens and reports uncategorized: changed lines no lens selects yet, grouped by file. Keep adding lenses until it is empty or what remains is deliberate. review_lens_get reads the current lenses and gaps.', rename: "Change the review title.", repin: diff --git a/packages/review/src/review-api/blocks/sequence.ts b/packages/review/src/review-api/blocks/sequence.ts index 4c3061c4a..524988e28 100644 --- a/packages/review/src/review-api/blocks/sequence.ts +++ b/packages/review/src/review-api/blocks/sequence.ts @@ -11,6 +11,8 @@ import { text, } from "./definition.js"; +const maxStepSources = 10; + export const stepSchema = z .strictObject({ ...identity, @@ -20,14 +22,23 @@ export const stepSchema = z label, style: z.enum(["call", "return", "async"]).default("call"), source: lensSourceSchema.optional(), + sources: z + .array(lensSourceSchema) + .min(1) + .max(maxStepSources) + .optional() + .describe( + "Several code chunks for one step, shown together in its tour stop. Chunks in the same file share one card.", + ), explanation: label.optional(), code: z.strictObject(codeFields).optional(), }) .refine( (s) => - [s.source, s.explanation, s.code].filter((v) => v !== undefined) - .length === 1, - "A step needs exactly one of source, explanation, or code.", + [s.source, s.sources, s.explanation, s.code].filter( + (v) => v !== undefined, + ).length === 1, + "A step needs exactly one of source, sources, explanation, or code.", ); export type Step = z.infer; diff --git a/packages/review/src/review-api/document-text.ts b/packages/review/src/review-api/document-text.ts index 0912932f3..2d0561429 100644 --- a/packages/review/src/review-api/document-text.ts +++ b/packages/review/src/review-api/document-text.ts @@ -87,6 +87,8 @@ export function documentText( if (element.source) detail(sourceText(element.source)); + if (element.sources) detail(element.sources.map(sourceText).join(", ")); + if (detailed && element.explanation) detail(element.explanation); if (detailed && element.code) code(depth + 1, element.code); diff --git a/packages/review/src/review-api/document.ts b/packages/review/src/review-api/document.ts index 71d9913b3..1c6346d54 100644 --- a/packages/review/src/review-api/document.ts +++ b/packages/review/src/review-api/document.ts @@ -286,6 +286,16 @@ function documentReferences( ), ); + // Each chunk of a multi-chunk step is its own reference, so one stale + // chunk marks only itself. + if (element.type === "step" && element.sources) + return element.sources.map((source, index) => ({ + id: `${element.id}:${index}`, + source, + label: element.label, + peek: true, + })); + if (element.type === "call_stack_diff") return [...element.base, ...element.head].flatMap((frame) => [ { ...frame, id: frame.id!, peek: true },