From 427706e1441bb7786ab7ab59325e3e640ee9e8d6 Mon Sep 17 00:00:00 2001 From: Michael Yong Date: Thu, 20 Aug 2026 16:35:08 -0700 Subject: [PATCH 1/3] Add full-file context to plugin diffs --- .../app/src/components/code/DiffHost.test.tsx | 92 ++++++++++++++++--- apps/app/src/components/code/DiffHost.tsx | 30 +++--- .../app/src/components/code/code-rendering.ts | 6 +- .../components/git-diff/GitDiffCardBody.tsx | 62 ++++++++----- apps/app/src/components/plugin/PluginDiff.tsx | 34 ++++++- .../git-diff/DiffFileCard.test.tsx | 66 ++++++++++++- .../bb-plugin-authoring/SKILL.md | 16 +++- .../plugins/plugin-authoring-docs.test.ts | 1 + docs/api_to_audit.md | 31 +++++-- packages/plugin-sdk/src/app-contract.ts | 38 +++++++- packages/plugin-sdk/src/testing/app.tsx | 4 + .../src/templates/bb-guide-plugins.md | 5 +- 12 files changed, 311 insertions(+), 74 deletions(-) diff --git a/apps/app/src/components/code/DiffHost.test.tsx b/apps/app/src/components/code/DiffHost.test.tsx index ad0f229a22..5bb3451a1e 100644 --- a/apps/app/src/components/code/DiffHost.test.tsx +++ b/apps/app/src/components/code/DiffHost.test.tsx @@ -3,7 +3,10 @@ import { cleanup, render, screen } from "@testing-library/react"; import { createStore, Provider as JotaiProvider } from "jotai"; import { act } from "react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import type { PluginDiffRendererProps } from "@get-bb/plugin-sdk"; +import type { + ExperimentalDiffFullFileContents, + PluginDiffRendererProps, +} from "@get-bb/plugin-sdk"; import { defaultResolvedCodeTheme } from "@bb/domain"; import { applyResolvedCodeTheme } from "@/lib/code-theme"; import { @@ -58,6 +61,29 @@ const PATCH = [ "", ].join("\n"); +const FULL_FILE_CONTENTS = { + old: { + path: "src/app.ts", + content: [ + "const a = 1;", + "const b = 2;", + "const c = 4;", + "const oldTail = true;", + "", + ].join("\n"), + }, + new: { + path: "src/app.ts", + content: [ + "const a = 1;", + "const b = 3;", + "const c = 4;", + "const newTail = true;", + "", + ].join("\n"), + }, +} satisfies ExperimentalDiffFullFileContents; + function parseFixture() { const file = parseGitDiffFiles(PATCH)[0]; if (file === undefined) throw new Error("fixture patch did not parse"); @@ -77,9 +103,7 @@ function registerDiffRenderer( sidebarFooterActions: [], fileOpeners: [], messageDirectives: [], - diffRenderers: [ - { id: "diffs", title: "Demo diffs", component }, - ], + diffRenderers: [{ id: "diffs", title: "Demo diffs", component }], }); } @@ -106,7 +130,12 @@ describe("DiffHost", () => { }); render( - , + , ); expect(await screen.findByTestId("plugin-diff")).toBeDefined(); @@ -128,6 +157,7 @@ describe("DiffHost", () => { { expect(props?.view).toBe("split"); expect(props?.overflow).toBe("wrap"); expect(props?.showLineNumbers).toBe(false); + expect(props?.experimental_fullFileContents).toBe(FULL_FILE_CONTENTS); expect(Object.keys(props ?? {})).not.toContain("onSelectionAddToChat"); expect(Object.keys(props ?? {})).not.toContain("file"); }); @@ -152,7 +183,7 @@ describe("DiffHost", () => { return
plugin diff
; }); - render(); + render(); await screen.findByTestId("plugin-diff"); const patch = receivedProps.at(-1)?.patch ?? ""; @@ -173,7 +204,13 @@ describe("DiffHost", () => { path.endsWith(".ts") ? :
plugin diff
, ); - render(); + render( + , + ); expect(await screen.findByTestId("bb-diff")).toBeDefined(); expect(bbDiff.loaded).toBe(true); @@ -191,7 +228,11 @@ describe("DiffHost", () => { render( - + , ); @@ -230,7 +271,11 @@ describe("DiffHost", () => { render( - + , ); @@ -245,13 +290,19 @@ describe("DiffHost", () => { throw new Error("replacement exploded"); }); - render(); + render( + , + ); expect(await screen.findByTestId("bb-diff")).toBeDefined(); }); it("uses BB's renderer with resolved presentation defaults when nothing is registered", async () => { - render(); + render(); await screen.findByTestId("bb-diff"); expect(bbDiff.lastProps?.view).toBe("unified"); @@ -271,6 +322,7 @@ describe("experimental_Diff", () => { await screen.findByTestId("plugin-diff"); expect(receivedProps.at(-1)?.path).toBe("src/app.ts"); + expect(receivedProps.at(-1)?.experimental_fullFileContents).toBeNull(); expect(bbDiff.loaded).toBe(false); }); @@ -296,6 +348,24 @@ describe("experimental_Diff", () => { expect(patch).not.toContain("\r"); }); + it("enriches BB's renderer with complete file contents for context expansion", async () => { + render( + , + ); + + await screen.findByTestId("bb-diff"); + const file = bbDiff.lastProps?.file as ReturnType< + typeof parseFixture + > | null; + expect(file?.isPartial).toBe(false); + expect(file?.additionLines).toContain("const newTail = true;\n"); + expect(bbDiff.lastProps?.expansionLineCount).toBe(30); + }); + it("degrades to plain text instead of an empty diff when the patch will not parse", () => { render(); diff --git a/apps/app/src/components/code/DiffHost.tsx b/apps/app/src/components/code/DiffHost.tsx index be4cc1a269..f52781d4c6 100644 --- a/apps/app/src/components/code/DiffHost.tsx +++ b/apps/app/src/components/code/DiffHost.tsx @@ -1,4 +1,5 @@ import { Suspense, lazy, useMemo, type ReactNode } from "react"; +import type { ExperimentalDiffFullFileContents } from "@get-bb/plugin-sdk"; import { PluginReplacementSlot } from "@/components/plugin/PluginReplacementSlot"; import type { ParsedGitDiffFile } from "@/components/git-diff/git-diff-parsing"; import { buildFileDiffPatchText } from "@/components/git-diff/git-diff-patch-text"; @@ -14,11 +15,14 @@ const DIFF_RENDERER_SLOT_KIND = "diffRenderer"; const BbDiff = lazy(() => import("./BbDiff")); +/** Unchanged lines revealed by one built-in expand-context action. */ +const DEFAULT_DIFF_EXPANSION_LINE_COUNT = 30; + interface DiffHostProps extends Partial { /** * The parsed diff to render. Callers parse it anyway for their own header, - * and the diff panel additionally enriches it with full file contents so the - * renderer can expand context between hunks. + * and callers with full file contents additionally enrich it so the renderer + * can expand context between hunks. */ file: ParsedGitDiffFile; /** @@ -27,13 +31,9 @@ interface DiffHostProps extends Partial { * reconstructs an equivalent single-file patch from `file`. */ patchText?: string; + /** Resolved semantic context forwarded to renderer replacements. */ + fullFileContents: ExperimentalDiffFullFileContents | null; className?: string; - /** - * Forwarded to BB's renderer; see {@link BbDiffProps.expansionLineCount}. - * Never reaches a plugin replacement — context expansion is a BB renderer - * capability, not part of the semantic contract. - */ - expansionLineCount?: number; /** Rendered while BB's renderer chunk loads. */ fallback?: ReactNode; onSelectionAddToChat?: (text: string) => void; @@ -45,6 +45,8 @@ interface DiffHostProps extends Partial { * the environment diff panel's file bodies — and every plugin that calls * `experimental_Diff` renders through here, so one * `experimental_diffRenderer` registration replaces them all at once. + * Resolved full-file text is semantic input: the built-in renderer uses the + * enriched parsed file, while a replacement receives the plain text sides. * * BB's own renderer sits behind `lazy()`. A plugin replacement that never * delegates therefore never downloads it, and `experimental_Original` costs @@ -53,11 +55,11 @@ interface DiffHostProps extends Partial { export function DiffHost({ file, patchText, + fullFileContents, view = DEFAULT_DIFF_VIEW, overflow = DEFAULT_CODE_OVERFLOW, showLineNumbers = true, className, - expansionLineCount, fallback = null, onSelectionAddToChat, }: DiffHostProps) { @@ -66,8 +68,7 @@ export function DiffHost({ // Only reconstructed when a replacement will actually read it: the walk is // proportional to the rendered hunks, and BB's own renderer never needs it. const semanticPatch = useMemo( - () => - isReplaced ? (patchText ?? buildFileDiffPatchText(file)) : "", + () => (isReplaced ? (patchText ?? buildFileDiffPatchText(file)) : ""), [file, isReplaced, patchText], ); @@ -79,7 +80,11 @@ export function DiffHost({ overflow={overflow} showLineNumbers={showLineNumbers} className={className} - expansionLineCount={expansionLineCount} + expansionLineCount={ + fullFileContents !== null && file.isPartial === false + ? DEFAULT_DIFF_EXPANSION_LINE_COUNT + : undefined + } onSelectionAddToChat={onSelectionAddToChat} /> @@ -99,6 +104,7 @@ export function DiffHost({ view={view} overflow={overflow} showLineNumbers={showLineNumbers} + experimental_fullFileContents={fullFileContents} experimental_Original={BoundOriginal} /> diff --git a/apps/app/src/components/code/code-rendering.ts b/apps/app/src/components/code/code-rendering.ts index ba71cf0fb2..dc9b13de83 100644 --- a/apps/app/src/components/code/code-rendering.ts +++ b/apps/app/src/components/code/code-rendering.ts @@ -65,9 +65,9 @@ export interface BbDiffProps extends DiffPresentation { className?: string; /** * How many unchanged lines each expand-context click reveals. Set ONLY by a - * caller that can attach full file contents to `file`: pierre renders an - * empty diff when it is given an expansion budget for a hunk-only patch, - * which is what the timeline supplies. + * host that received complete file contents and a successfully enriched + * `file`: pierre renders an empty diff when it is given an expansion budget + * for a hunk-only patch, which is what the timeline supplies. */ expansionLineCount?: number; onSelectionAddToChat?: (text: string) => void; diff --git a/apps/app/src/components/git-diff/GitDiffCardBody.tsx b/apps/app/src/components/git-diff/GitDiffCardBody.tsx index 147c0dc5c6..bfd276483a 100644 --- a/apps/app/src/components/git-diff/GitDiffCardBody.tsx +++ b/apps/app/src/components/git-diff/GitDiffCardBody.tsx @@ -9,6 +9,7 @@ import { } from "react"; import type { FileContents } from "@pierre/diffs"; import type { GitDiffFileChangeKind } from "@bb/server-contract"; +import type { ExperimentalDiffFullFileContents } from "@get-bb/plugin-sdk"; import { useIntersectionObserver } from "usehooks-ts"; import { Button } from "@bb/shared-ui/button"; import { usePointerCoarse } from "@bb/shared-ui/hooks/use-pointer-coarse"; @@ -57,14 +58,6 @@ export interface DiffImageSizeStat { export type GitDiffCardSvgDisplayMode = "preview" | "raw"; -/** - * Unchanged lines revealed per expand-up / expand-down click; the library - * default of 100 is too aggressive for our compact cards. Only sent for a - * card that can actually fetch full file contents — see - * {@link BbDiffProps.expansionLineCount}. - */ -const DIFF_EXPANSION_LINE_COUNT = 30; - const GIT_DIFF_CARD_BODY_STYLE: CSSProperties = { contain: "layout paint style", contentVisibility: "auto", @@ -84,7 +77,11 @@ interface DiffFileContentPlan { type DiffFileEnrichmentState = | { status: "idle" } | { status: "loading" } - | { status: "ready"; fileDiff: ParsedGitDiffFile } + | { + status: "ready"; + fileDiff: ParsedGitDiffFile; + fullFileContents: ExperimentalDiffFullFileContents; + } | { status: "ready-image"; oldImageUrl: string | null; @@ -95,6 +92,7 @@ type DiffFileEnrichmentState = | { status: "ready-svg"; fileDiff: ParsedGitDiffFile; + fullFileContents: ExperimentalDiffFullFileContents; oldImageUrl: string | null; newImageUrl: string | null; } @@ -218,6 +216,16 @@ function resolveDiffFileContentSource( return fetcher(source.path, source.side); } +function toDiffFullFileContents( + oldFile: FileContents, + newFile: FileContents, +): ExperimentalDiffFullFileContents { + return { + old: { path: oldFile.name, content: oldFile.contents }, + new: { path: newFile.name, content: newFile.contents }, + }; +} + /** * An image change is conveyed as a single-binary swap rather than text hunks, * so the card renders inline `` previews instead of a ``. A card @@ -279,6 +287,8 @@ interface GitDiffCardBodyState { imageSizeStat: DiffImageSizeStat | null; /** On-demand full-file context for text cards; see {@link DiffContextExpansionState}. */ contextExpansion: DiffContextExpansionState; + /** Resolved text sides forwarded through the public renderer contract. */ + fullFileContents: ExperimentalDiffFullFileContents | null; /** * The raw per-file patch the caller supplied, forwarded to the host diff * boundary so a plugin replacement gets the caller's own bytes instead of a @@ -438,11 +448,16 @@ export function useGitDiffCardBody({ newFile: newResult.file, patchText, }); + const fullFileContents = toDiffFullFileContents( + oldResult.file, + newResult.file, + ); if (isSvgCard) { enrichmentStatusRef.current = "ready-svg"; setEnrichment({ status: "ready-svg", fileDiff: enrichedFileDiff, + fullFileContents, oldImageUrl: svgTextToDataUrl(oldResult.file.contents), newImageUrl: svgTextToDataUrl(newResult.file.contents), }); @@ -452,6 +467,7 @@ export function useGitDiffCardBody({ setEnrichment({ status: "ready", fileDiff: enrichedFileDiff, + fullFileContents, }); }) .catch(() => { @@ -490,7 +506,10 @@ export function useGitDiffCardBody({ enrichmentStatus: enrichment.status, }); const contextExpansion = useMemo( - () => ({ status: contextExpansionStatus, request: requestContextExpansion }), + () => ({ + status: contextExpansionStatus, + request: requestContextExpansion, + }), [contextExpansionStatus, requestContextExpansion], ); @@ -500,6 +519,10 @@ export function useGitDiffCardBody({ } return enrichment.fileDiff; }, [fileDiff, enrichment]); + const fullFileContents = + enrichment.status === "ready" || enrichment.status === "ready-svg" + ? enrichment.fullFileContents + : null; const loadDeletedDiff = useCallback(() => { setHasLoadedDeletedDiff(true); @@ -523,6 +546,7 @@ export function useGitDiffCardBody({ loadDeletedDiff, imageSizeStat, contextExpansion, + fullFileContents, patchText, }; } @@ -768,8 +792,8 @@ interface GitDiffCardSvgBodyProps { fileDiff: ParsedGitDiffFile; fileDiffLabel: string; patchText: string | undefined; + fullFileContents: ExperimentalDiffFullFileContents | null; presentation: DiffPresentation; - expansionLineCount: number | undefined; onSelectionAddToChat?: (text: string) => void; } @@ -779,8 +803,8 @@ function GitDiffCardSvgBody({ fileDiff, fileDiffLabel, patchText, + fullFileContents, presentation, - expansionLineCount, onSelectionAddToChat, }: GitDiffCardSvgBodyProps) { return displayMode === "preview" ? ( @@ -793,8 +817,8 @@ function GitDiffCardSvgBody({ ); @@ -841,15 +865,9 @@ export function GitDiffCardBody({ shouldRenderDiffView, loadDeletedDiff, contextExpansion, + fullFileContents, patchText, } = state; - // pierre renders an empty diff when it gets an expansion budget for a - // hunk-only patch, so only a card that can fetch full contents sends one. - // The timeline never can; the diff panel can, through its fetcher. - const expansionLineCount = - contextExpansion.status === "unavailable" - ? undefined - : DIFF_EXPANSION_LINE_COUNT; return (
) : ( @@ -898,8 +916,8 @@ export function GitDiffCardBody({ } onSelectionAddToChat={onSelectionAddToChat} /> diff --git a/apps/app/src/components/plugin/PluginDiff.tsx b/apps/app/src/components/plugin/PluginDiff.tsx index b56c8a48ed..668285edb5 100644 --- a/apps/app/src/components/plugin/PluginDiff.tsx +++ b/apps/app/src/components/plugin/PluginDiff.tsx @@ -1,15 +1,19 @@ import { useMemo } from "react"; import type { DiffProps } from "@get-bb/plugin-sdk"; import { DiffHost } from "@/components/code/DiffHost"; -import { normalizeFilePatch } from "@/components/git-diff/git-diff-parsing"; +import { + enrichGitDiffFileForContext, + normalizeFilePatch, +} from "@/components/git-diff/git-diff-parsing"; import { cn } from "@bb/shared-ui/lib/utils"; /** * The public `experimental_Diff` component. It normalizes whatever patch shape * the caller has (a `git diff` patch, a GitHub REST patch, a single `@@` hunk) - * into one the renderer understands, then hands it to the host boundary. - * Content that does not parse as a patch degrades to plain monospace text - * rather than to an empty diff. + * into one the renderer understands, enriches it when the caller supplied both + * complete text sides, then hands it to the host boundary. Content that does + * not parse as a patch degrades to plain monospace text rather than to an empty + * diff. */ export function PluginDiff({ patch, @@ -17,12 +21,30 @@ export function PluginDiff({ view, overflow, showLineNumbers, + experimental_fullFileContents: fullFileContents, className, }: DiffProps) { const normalized = useMemo( () => normalizeFilePatch({ patch, path }), [patch, path], ); + const file = useMemo(() => { + if (normalized === null || fullFileContents === undefined) { + return normalized?.file ?? null; + } + return enrichGitDiffFileForContext({ + fileDiff: normalized.file, + oldFile: { + name: fullFileContents.old.path, + contents: fullFileContents.old.content, + }, + newFile: { + name: fullFileContents.new.path, + contents: fullFileContents.new.content, + }, + patchText: normalized.patch, + }); + }, [fullFileContents, normalized]); if (normalized === null) { return (
     );
   }
+  if (file === null) return null;
   return (
      {
     expect(seen.at(-1)?.view).toBe("unified");
   });
 
+  it("forwards lazily resolved text sides to a replacement renderer", async () => {
+    const seen: PluginDiffRendererProps["experimental_fullFileContents"][] = [];
+    setPluginSlotRegistrations("demo", {
+      homepageSections: [],
+      settingsSections: [],
+      navPanels: [],
+      threadPanelActions: [],
+      sidebarFooterActions: [],
+      fileOpeners: [],
+      messageDirectives: [],
+      diffRenderers: [
+        {
+          id: "diffs",
+          title: "Demo diffs",
+          component: ({ experimental_fullFileContents }) => {
+            seen.push(experimental_fullFileContents);
+            return 
plugin diff
; + }, + }, + ], + }); + const onRequestFileContents = vi.fn( + async (path, side) => ({ + kind: "text", + file: { + name: path, + contents: + side === "old" + ? "const b = 2;\nconst tail = true;\n" + : "const b = 3;\nconst tail = true;\n", + }, + }), + ); + + renderCard({ + entry: buildEntry(), + patchState: { status: "loaded", patch: TEXT_PATCH }, + onRequestFileContents, + }); + + fireEvent.click( + await screen.findByRole("button", { name: "Expand context" }), + ); + await waitFor(() => { + expect(seen.at(-1)).toEqual({ + old: { + path: "src/file.ts", + content: "const b = 2;\nconst tail = true;\n", + }, + new: { + path: "src/file.ts", + content: "const b = 3;\nconst tail = true;\n", + }, + }); + }); + }); + it("falls back to the load gate when an image-looking path is not previewable", async () => { const onLoadPatch = vi.fn(); const onRequestFileContents = vi.fn( diff --git a/apps/server/src/services/skills/builtin-skills/bb-plugin-authoring/SKILL.md b/apps/server/src/services/skills/builtin-skills/bb-plugin-authoring/SKILL.md index 91392d49b8..3b92f40eca 100644 --- a/apps/server/src/services/skills/builtin-skills/bb-plugin-authoring/SKILL.md +++ b/apps/server/src/services/skills/builtin-skills/bb-plugin-authoring/SKILL.md @@ -1802,7 +1802,10 @@ projectId }` (nullable fields) and `path` follows the source (workspace: belongs in the component. Source props: `{ content, path, overflow, highlightedLines, experimental_Original }`; diff props: - `{ patch, path, view, overflow, showLineNumbers, experimental_Original }`. + `{ patch, path, view, overflow, showLineNumbers, experimental_fullFileContents, +experimental_Original }`. `experimental_fullFileContents` is either + `{ old: { path, content }, new: { path, content } }` or `null`; a replacement + can use those complete UTF-8 sides to implement context expansion. Every value is already resolved. Render `experimental_Original` (bb's renderer, bound to this call) to delegate without re-entering resolution — behind a plugin setting, by language, over a size threshold: @@ -1918,12 +1921,17 @@ className?, leadingContent?, messageActions? }` — and `highlightedLines` is a 1-based inclusive `{ start, end }` (default null). bb owns syntax highlighting, gutters, and the live code theme. - `experimental_Diff` — bb's diff viewer. Props: - `{ patch, path, view?, overflow?, showLineNumbers?, className? }` — + `{ patch, path, view?, overflow?, showLineNumbers?, experimental_fullFileContents?, +className? }` — `patch` is a unified patch for exactly ONE file and `view` is `"unified"` (default) or `"split"`. bb normalizes the patch, so a GitHub REST patch or a bare `@@` hunk works without synthesizing a `diff --git` header - yourself; unparseable content degrades to plain monospace text. Reference: - `plugins/github/app.tsx`. + yourself; unparseable content degrades to plain monospace text. + `experimental_fullFileContents` is + `{ old: { path, content }, new: { path, content } }`; when supplied and + consistent with the patch, bb enables expand-context controls between + hunks. The caller owns loading those complete UTF-8 sides and omits the prop + while it has only the patch. Reference: `plugins/github/app.tsx`. Alias both on import — JSX reads a lowercase-initial name as an intrinsic element: diff --git a/apps/server/test/services/plugins/plugin-authoring-docs.test.ts b/apps/server/test/services/plugins/plugin-authoring-docs.test.ts index f427adb5af..304c53b5ff 100644 --- a/apps/server/test/services/plugins/plugin-authoring-docs.test.ts +++ b/apps/server/test/services/plugins/plugin-authoring-docs.test.ts @@ -258,6 +258,7 @@ const FRONTEND_SLOT_PROP_FIELDS = { "view", "overflow", "showLineNumbers", + "experimental_fullFileContents", "experimental_Original", ], messageDirective: ["attributes", "source", "message", "openWorkspaceFile"], diff --git a/docs/api_to_audit.md b/docs/api_to_audit.md index b6bcce73b0..fbba58caaf 100644 --- a/docs/api_to_audit.md +++ b/docs/api_to_audit.md @@ -924,11 +924,14 @@ files, thread-storage files, and project files that use the primary host. **What it does.** Two host-owned renderers for supplied code content. `experimental_SourceCode` takes source text plus a path and owns syntax highlighting, gutters, wrapping, highlighted-line presentation, and the live BB -code theme. `experimental_Diff` takes a single-file patch plus a path and owns -patch normalization (a patch without a `diff --git` header is completed from -`path`, which is what makes GitHub's REST patches and bare `@@` hunks render), +code theme. `experimental_Diff` takes a single-file patch plus a path and +optional `experimental_fullFileContents` for both text sides, and owns patch normalization +(a patch without a `diff --git` header is completed from `path`, which is what +makes GitHub's REST patches and bare `@@` hunks render), context enrichment, syntax highlighting, unified/split presentation, gutters, and the same live -theme. Patch content that will not parse degrades to plain monospace text. +theme. Patch content that will not parse degrades to plain monospace text. The +caller still owns loading file contents; omission means a patch-only render +without context expansion. These are the same components BB's own file preview, timeline file diffs, and environment diff panel render through, so an active @@ -939,8 +942,9 @@ behavior deliberately stay with the caller. **Audit before stabilizing.** -1. **Prop surface.** Confirm content + path + presentation is the right minimal - contract, and decide whether `className` belongs in it at all — a +1. **Prop surface.** Confirm content + path + presentation plus optional full + diff sides is the right minimal contract, and decide whether `className` + belongs in it at all — a replacement never receives it today, so a `className` that only styles BB's renderer is a quiet inconsistency. 2. **Diff input shape.** Confirm single-file patch text is the right currency. @@ -959,6 +963,12 @@ behavior deliberately stay with the caller. that through `useComposer()` rather than a renderer prop. 6. **Size and virtualization.** Neither component caps input size or virtualizes. Audit against a plugin that renders a very large file or patch. +7. **Resolved (Aug 2026): context expansion takes resolved semantic data, not + a loader callback.** `experimental_fullFileContents` carries required `old` and `new` + `{ path, content }` objects. This keeps lazy loading, retries, and viewport + policy with the caller while letting BB's renderer and a replacement consume + complete UTF-8 sides without exposing Pierre's `FileContents` type. A + replacement always receives the resolved field as an object or `null`. ## `app.slots.experimental_sourceCodeRenderer` / `app.slots.experimental_diffRenderer` (`@get-bb/plugin-sdk/app`) @@ -1005,10 +1015,11 @@ is temporarily unavailable renders BB's renderer without erasing the pin. its users out. No first-party-only or own-surfaces-only scope. Audit this as precedent rather than as a fact about these two slots: no other slot lets a plugin reach into another plugin's rendered output. -4. **Capability parity.** A replacement cannot implement context expansion, - selection-to-chat, or the deleted-file gate, because those inputs are - host-only. Confirm that asymmetry is acceptable, or promote the ones that - should be part of the contract. +4. **Capability parity. Resolved for context expansion (Aug 2026):** a + replacement receives `experimental_fullFileContents` as an object or `null`, matching the + public host component and first-party diff cards. Selection-to-chat and the + deleted-file gate remain host-only; confirm that remaining asymmetry is + acceptable, or promote either capability before stabilization. 5. **Two slots or one.** Confirm source and diff should stay separately replaceable rather than one "code renderer" registration. diff --git a/packages/plugin-sdk/src/app-contract.ts b/packages/plugin-sdk/src/app-contract.ts index 6c31320689..10eed4c204 100644 --- a/packages/plugin-sdk/src/app-contract.ts +++ b/packages/plugin-sdk/src/app-contract.ts @@ -208,6 +208,20 @@ export interface SourceCodeLineRange { end: number; } +/** One complete text side of a diff, resolved by the caller. */ +export interface ExperimentalDiffFileContent { + /** File path for this side. May differ between `old` and `new` for a rename. */ + path: string; + /** Complete UTF-8 file contents, including unchanged lines outside the patch. */ + content: string; +} + +/** Complete text contents for both sides of a diff. */ +export interface ExperimentalDiffFullFileContents { + old: ExperimentalDiffFileContent; + new: ExperimentalDiffFileContent; +} + /** * Props of the host-owned `experimental_SourceCode` component — BB's source * viewer. The host owns syntax highlighting, gutters, wrapping, line-selection @@ -234,8 +248,9 @@ export interface SourceCodeProps { * Props of the host-owned `experimental_Diff` component — BB's diff viewer. * The host owns patch normalization (a patch without a `diff --git` header is * completed from `path`), syntax highlighting, unified/split presentation, - * gutters, line-selection presentation, and the live BB code theme. Content - * that cannot be parsed as a patch degrades to plain monospace text. + * gutters, line-selection presentation, optional full-file context expansion, + * and the live BB code theme. Content that cannot be parsed as a patch + * degrades to plain monospace text. */ export interface DiffProps { /** Unified patch text for exactly ONE file. */ @@ -252,6 +267,12 @@ export interface DiffProps { overflow?: CodeOverflowMode; /** Whether the gutter shows line numbers. Defaults to `true`. */ showLineNumbers?: boolean; + /** + * Complete text for both file sides. When present and consistent with the + * patch, BB enables expand-context controls between hunks. The caller owns + * loading these contents; omit the field to render from the patch alone. + */ + experimental_fullFileContents?: ExperimentalDiffFullFileContents; /** Applied to the renderer's root element. */ className?: string; } @@ -276,7 +297,8 @@ export interface PluginSourceCodeRendererProps { /** * Props passed to an `experimental_diffRenderer` component. `patch` is always - * a complete single-file unified patch, whatever shape the caller supplied. + * a complete single-file unified patch, whatever shape the caller supplied, + * and optional full-file context is resolved to an object or `null`. */ export interface PluginDiffRendererProps { patch: string; @@ -284,6 +306,11 @@ export interface PluginDiffRendererProps { view: DiffViewMode; overflow: CodeOverflowMode; showLineNumbers: boolean; + /** + * Complete resolved text for both sides, or `null` when the caller supplied + * only the patch. A replacement can use this to implement context expansion. + */ + experimental_fullFileContents: ExperimentalDiffFullFileContents | null; /** * BB's diff renderer, bound to this request. Render it to delegate * conditionally without re-entering plugin replacement resolution. @@ -1852,8 +1879,9 @@ export interface PluginSdkApp { experimental_SourceCode: ComponentType; /** * The host-owned diff viewer (see {@link DiffProps}). Renders supplied patch - * content with BB's normalization, syntax highlighting, unified/split - * presentation, and live code theme, and honours an active + * content with BB's normalization, optional full-file context expansion, + * syntax highlighting, unified/split presentation, and live code theme, and + * honours an active * `experimental_diffRenderer` replacement. Experimental: see * docs/api_to_audit.md. */ diff --git a/packages/plugin-sdk/src/testing/app.tsx b/packages/plugin-sdk/src/testing/app.tsx index 53e2fd4e3a..699601834a 100644 --- a/packages/plugin-sdk/src/testing/app.tsx +++ b/packages/plugin-sdk/src/testing/app.tsx @@ -535,6 +535,7 @@ function TestDiff({ view = "unified", overflow = "scroll", showLineNumbers = true, + experimental_fullFileContents, className, }: DiffProps) { return ( @@ -544,6 +545,9 @@ function TestDiff({ data-view={view} data-overflow={overflow} data-show-line-numbers={showLineNumbers ? "true" : "false"} + data-has-full-file-contents={ + experimental_fullFileContents === undefined ? "false" : "true" + } className={className} > {patch} diff --git a/packages/templates/src/templates/bb-guide-plugins.md b/packages/templates/src/templates/bb-guide-plugins.md index 075a1ca4d1..c2161af641 100644 --- a/packages/templates/src/templates/bb-guide-plugins.md +++ b/packages/templates/src/templates/bb-guide-plugins.md @@ -579,7 +579,10 @@ class-variance-authority libraries are runtime-shimmed (never bundled) — though source and diffs should go through the host's own experimental_SourceCode / experimental_Diff components rather than @pierre/diffs directly, so bb owns patch normalization, syntax -highlighting, and the live code theme. +highlighting, and the live code theme. A Diff caller that has loaded complete +old/new UTF-8 file contents can pass them through +`experimental_fullFileContents` to enable +expand-context controls without exposing Pierre types. Everything else (zod included) bundles from the plugin's node_modules (`npm install` for authors; BB installs release packages with their declared production dependencies). A crashing slot collapses to a "plugin crashed" chip without From 94a0418c4c69052afb243e43da8d577d78fa30b9 Mon Sep 17 00:00:00 2001 From: Michael Yong Date: Fri, 21 Aug 2026 11:04:31 -0700 Subject: [PATCH 2/3] Bump plugin SDK to 0.4.12 --- packages/domain/src/plugin-sdk-version.ts | 2 +- packages/plugin-sdk/package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/domain/src/plugin-sdk-version.ts b/packages/domain/src/plugin-sdk-version.ts index 1cb75dabe4..d3d061645d 100644 --- a/packages/domain/src/plugin-sdk-version.ts +++ b/packages/domain/src/plugin-sdk-version.ts @@ -16,7 +16,7 @@ // PLUGIN_SDK_MAJOR is 0, so the major-only artifact gate cannot distinguish // 0.x releases and is intentionally vacuous for them until a future 1.0. // Rebuildable artifacts still rebuild on the exact sdkVersion-differs trigger. -export const PLUGIN_SDK_VERSION = "0.4.11"; +export const PLUGIN_SDK_VERSION = "0.4.12"; /** Major of {@link PLUGIN_SDK_VERSION} — the plugin API compatibility number. */ export const PLUGIN_SDK_MAJOR = Number(PLUGIN_SDK_VERSION.split(".", 1)[0]); diff --git a/packages/plugin-sdk/package.json b/packages/plugin-sdk/package.json index dec2ece2ca..6314583303 100644 --- a/packages/plugin-sdk/package.json +++ b/packages/plugin-sdk/package.json @@ -1,6 +1,6 @@ { "name": "@get-bb/plugin-sdk", - "version": "0.4.11", + "version": "0.4.12", "homepage": "https://github.com/get-bb/bb#readme", "bugs": { "url": "https://github.com/get-bb/bb/issues" From e8840cee74bab7721b553d314b2d0b8ea8ae2351 Mon Sep 17 00:00:00 2001 From: Michael Yong Date: Fri, 21 Aug 2026 11:27:08 -0700 Subject: [PATCH 3/3] Validate and lazily enrich diff context --- apps/app/src/components/code/BbDiff.test.tsx | 118 +++++++++++++++++- apps/app/src/components/code/BbDiff.tsx | 43 ++++++- .../app/src/components/code/DiffHost.test.tsx | 16 ++- apps/app/src/components/code/DiffHost.tsx | 19 ++- .../app/src/components/code/code-rendering.ts | 17 ++- .../components/git-diff/GitDiffCardBody.tsx | 27 +--- .../git-diff/git-diff-parsing.test.ts | 55 ++++++++ .../components/git-diff/git-diff-parsing.ts | 106 ++++++++++++++-- apps/app/src/components/plugin/PluginDiff.tsx | 34 ++--- docs/api_to_audit.md | 10 +- packages/plugin-sdk/src/app-contract.ts | 7 +- .../src/templates/bb-guide-plugins.md | 4 +- 12 files changed, 351 insertions(+), 105 deletions(-) diff --git a/apps/app/src/components/code/BbDiff.test.tsx b/apps/app/src/components/code/BbDiff.test.tsx index 258a70eb01..ad5b4b049a 100644 --- a/apps/app/src/components/code/BbDiff.test.tsx +++ b/apps/app/src/components/code/BbDiff.test.tsx @@ -18,12 +18,32 @@ interface RenderedOptions { const pierre = vi.hoisted(() => ({ lastOptions: null as RenderedOptions | null, + lastFileDiff: null as object | null, + processFileCalls: 0, })); +vi.mock("@pierre/diffs", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + processFile: (...args: Parameters) => { + pierre.processFileCalls += 1; + return actual.processFile(...args); + }, + }; +}); + vi.mock("@pierre/diffs/react", async () => { const React = await import("react"); return { - FileDiff: ({ options }: { options: RenderedOptions }) => { + FileDiff: ({ + fileDiff, + options, + }: { + fileDiff: object; + options: RenderedOptions; + }) => { + pierre.lastFileDiff = fileDiff; pierre.lastOptions = options; return React.createElement("div", { "data-testid": "pierre-file-diff" }); }, @@ -34,12 +54,37 @@ const PATCH = [ "diff --git a/src/app.ts b/src/app.ts", "--- a/src/app.ts", "+++ b/src/app.ts", - "@@ -1,2 +1,2 @@", + "@@ -1,3 +1,3 @@", + " const a = 1;", "-const b = 2;", "+const b = 3;", + " const c = 4;", "", ].join("\n"); +const FULL_FILE_CONTENTS = { + old: { + path: "src/app.ts", + content: [ + "const a = 1;", + "const b = 2;", + "const c = 4;", + "const oldTail = true;", + "", + ].join("\n"), + }, + new: { + path: "src/app.ts", + content: [ + "const a = 1;", + "const b = 3;", + "const c = 4;", + "const newTail = true;", + "", + ].join("\n"), + }, +}; + function fixture() { const file = parseGitDiffFiles(PATCH)[0]; if (file === undefined) throw new Error("fixture patch did not parse"); @@ -48,6 +93,8 @@ function fixture() { beforeEach(() => { pierre.lastOptions = null; + pierre.lastFileDiff = null; + pierre.processFileCalls = 0; applyResolvedCodeTheme(defaultResolvedCodeTheme); }); @@ -65,6 +112,7 @@ describe("BbDiff", () => { view="unified" overflow="scroll" showLineNumbers + fullFileContents={null} />, ); await screen.findByTestId("pierre-file-diff"); @@ -95,6 +143,7 @@ describe("BbDiff", () => { view="unified" overflow="scroll" showLineNumbers + fullFileContents={null} />, ); await screen.findByTestId("pierre-file-diff"); @@ -103,19 +152,79 @@ describe("BbDiff", () => { expect("expansionLineCount" in (pierre.lastOptions ?? {})).toBe(false); }); - it("passes the expansion budget through when the caller supplies one", async () => { + it("enriches matching full contents and enables context expansion", async () => { render( , ); await screen.findByTestId("pierre-file-diff"); expect(pierre.lastOptions?.expansionLineCount).toBe(30); + expect(pierre.lastFileDiff).toMatchObject({ + isPartial: false, + additionLines: expect.arrayContaining(["const newTail = true;\n"]), + }); + }); + + it("rejects full contents that do not match the patch", async () => { + const file = fixture(); + render( + , + ); + await screen.findByTestId("pierre-file-diff"); + + expect(pierre.lastFileDiff).toBe(file); + expect(pierre.lastOptions).not.toHaveProperty("expansionLineCount"); + }); + + it("does not reparse when a new wrapper carries the same primitive contents", async () => { + const file = fixture(); + const { rerender } = render( + , + ); + await screen.findByTestId("pierre-file-diff"); + const firstResolvedFile = pierre.lastFileDiff; + expect(pierre.processFileCalls).toBe(1); + + rerender( + , + ); + + expect(pierre.lastFileDiff).toBe(firstResolvedFile); + expect(pierre.processFileCalls).toBe(1); }); it("maps semantic presentation onto the renderer's options", async () => { @@ -125,6 +234,7 @@ describe("BbDiff", () => { view="split" overflow="wrap" showLineNumbers={false} + fullFileContents={null} />, ); await screen.findByTestId("pierre-file-diff"); diff --git a/apps/app/src/components/code/BbDiff.tsx b/apps/app/src/components/code/BbDiff.tsx index d3fa9df652..ae3851437a 100644 --- a/apps/app/src/components/code/BbDiff.tsx +++ b/apps/app/src/components/code/BbDiff.tsx @@ -6,9 +6,11 @@ import { PierreWorkerPoolBoundary } from "@/lib/pierre-worker-pool-boundary"; import { useRequirePierreWorkerPool } from "@/lib/pierre-worker-pool-gate"; import { usePierreStrictModeRecoveryOptions } from "@/lib/pierre-strict-mode-recovery"; import { + buildFileDiffPatchText, buildDiffDomSelectionText, buildDiffLineSelectionText, } from "@/components/git-diff/git-diff-patch-text"; +import { enrichGitDiffFileForContext } from "@/components/git-diff/git-diff-parsing"; import { useResolvedCodeThemePair } from "@/lib/code-theme"; import { usePreferredTheme } from "@/hooks/useTheme"; import { Skeleton } from "@bb/shared-ui/skeleton"; @@ -20,6 +22,9 @@ const DIFF_VIEW_STYLE = { "--diffs-line-height": "18px", } as CSSProperties; +/** Unchanged lines revealed by one built-in expand-context action. */ +const DEFAULT_DIFF_EXPANSION_LINE_COUNT = 30; + function BbDiffSkeleton() { return (
@@ -41,13 +46,38 @@ function BbDiffSkeleton() { */ export function BbDiff({ file, + patchText, + fullFileContents, view, overflow, showLineNumbers, className, - expansionLineCount, onSelectionAddToChat, }: BbDiffProps) { + const oldPath = fullFileContents?.old.path; + const oldContent = fullFileContents?.old.content; + const newPath = fullFileContents?.new.path; + const newContent = fullFileContents?.new.content; + const resolvedFile = useMemo(() => { + if ( + oldPath === undefined || + oldContent === undefined || + newPath === undefined || + newContent === undefined + ) { + return file; + } + return enrichGitDiffFileForContext({ + fileDiff: file, + oldFile: { name: oldPath, contents: oldContent }, + newFile: { name: newPath, contents: newContent }, + patchText: patchText ?? buildFileDiffPatchText(file), + }); + }, [file, newContent, newPath, oldContent, oldPath, patchText]); + const expansionLineCount = + resolvedFile !== file && resolvedFile.isPartial === false + ? DEFAULT_DIFF_EXPANSION_LINE_COUNT + : undefined; const containerRef = useRef(null); const codeTheme = useResolvedCodeThemePair(); const themeType = usePreferredTheme(); @@ -55,10 +85,10 @@ export function BbDiff({ (range: SelectedLineRange) => buildDiffLineSelectionText({ displayStyle: view, - fileDiff: file, + fileDiff: resolvedFile, range, }), - [file, view], + [resolvedFile, view], ); const buildFallbackSelectionText = useCallback( ({ @@ -66,8 +96,9 @@ export function BbDiff({ }: { containerElement: HTMLElement | null; range: SelectedLineRange; - }) => buildDiffDomSelectionText({ containerElement, fileDiff: file }), - [file], + }) => + buildDiffDomSelectionText({ containerElement, fileDiff: resolvedFile }), + [resolvedFile], ); const lineSelectionActions = usePierreLineSelectionActions({ buildFallbackSelectionText, @@ -135,7 +166,7 @@ export function BbDiff({
diff --git a/apps/app/src/components/code/DiffHost.test.tsx b/apps/app/src/components/code/DiffHost.test.tsx index 5bb3451a1e..640f9fe896 100644 --- a/apps/app/src/components/code/DiffHost.test.tsx +++ b/apps/app/src/components/code/DiffHost.test.tsx @@ -123,7 +123,7 @@ afterEach(() => { }); describe("DiffHost", () => { - it("keeps BB's renderer chunk unloaded when a replacement never delegates", async () => { + it("skips BB's renderer and full-file enrichment when a replacement never delegates", async () => { registerDiffRenderer((props) => { receivedProps.push(props); return
plugin diff
; @@ -133,7 +133,7 @@ describe("DiffHost", () => { , ); @@ -145,6 +145,9 @@ describe("DiffHost", () => { await Promise.resolve(); }); expect(bbDiff.loaded).toBe(false); + expect(receivedProps.at(-1)?.experimental_fullFileContents).toBe( + FULL_FILE_CONTENTS, + ); }); it("hands the replacement resolved semantic props, not BB's host-only inputs", async () => { @@ -348,7 +351,7 @@ describe("experimental_Diff", () => { expect(patch).not.toContain("\r"); }); - it("enriches BB's renderer with complete file contents for context expansion", async () => { + it("defers complete-file enrichment to BB's lazy renderer", async () => { render( { const file = bbDiff.lastProps?.file as ReturnType< typeof parseFixture > | null; - expect(file?.isPartial).toBe(false); - expect(file?.additionLines).toContain("const newTail = true;\n"); - expect(bbDiff.lastProps?.expansionLineCount).toBe(30); + expect(file?.isPartial).toBe(true); + expect(bbDiff.lastProps?.patchText).toBe(PATCH); + expect(bbDiff.lastProps?.fullFileContents).toBe(FULL_FILE_CONTENTS); + expect(bbDiff.lastProps).not.toHaveProperty("expansionLineCount"); }); it("degrades to plain text instead of an empty diff when the patch will not parse", () => { diff --git a/apps/app/src/components/code/DiffHost.tsx b/apps/app/src/components/code/DiffHost.tsx index f52781d4c6..4de868f29a 100644 --- a/apps/app/src/components/code/DiffHost.tsx +++ b/apps/app/src/components/code/DiffHost.tsx @@ -15,14 +15,11 @@ const DIFF_RENDERER_SLOT_KIND = "diffRenderer"; const BbDiff = lazy(() => import("./BbDiff")); -/** Unchanged lines revealed by one built-in expand-context action. */ -const DEFAULT_DIFF_EXPANSION_LINE_COUNT = 30; - interface DiffHostProps extends Partial { /** * The parsed diff to render. Callers parse it anyway for their own header, - * and callers with full file contents additionally enrich it so the renderer - * can expand context between hunks. + * while the built-in renderer lazily enriches it if full contents are + * available and consistent with the patch. */ file: ParsedGitDiffFile; /** @@ -45,8 +42,9 @@ interface DiffHostProps extends Partial { * the environment diff panel's file bodies — and every plugin that calls * `experimental_Diff` renders through here, so one * `experimental_diffRenderer` registration replaces them all at once. - * Resolved full-file text is semantic input: the built-in renderer uses the - * enriched parsed file, while a replacement receives the plain text sides. + * Resolved full-file text is semantic input: a replacement receives the plain + * text sides, while the built-in renderer validates and parses them only if it + * actually mounts. * * BB's own renderer sits behind `lazy()`. A plugin replacement that never * delegates therefore never downloads it, and `experimental_Original` costs @@ -76,15 +74,12 @@ export function DiffHost({ diff --git a/apps/app/src/components/code/code-rendering.ts b/apps/app/src/components/code/code-rendering.ts index dc9b13de83..532b278871 100644 --- a/apps/app/src/components/code/code-rendering.ts +++ b/apps/app/src/components/code/code-rendering.ts @@ -1,6 +1,7 @@ import type { CodeOverflowMode, DiffViewMode, + ExperimentalDiffFullFileContents, SourceCodeLineRange, } from "@get-bb/plugin-sdk"; import type { ParsedGitDiffFile } from "@/components/git-diff/git-diff-parsing"; @@ -57,18 +58,14 @@ export interface BbSourceCodeProps extends SourceCodePresentation { /** Props BB's default diff renderer receives from {@link DiffHost}. */ export interface BbDiffProps extends DiffPresentation { /** - * The diff to draw. Already parsed — and possibly enriched with full file - * contents for context expansion — by the caller, which also needs it for - * its own header. + * The raw parsed diff to draw. The built-in renderer enriches it lazily when + * complete file contents agree with the patch. */ file: ParsedGitDiffFile; + /** Original patch text, when the caller still has it. */ + patchText?: string; + /** Caller-resolved full text sides, or null when context is unavailable. */ + fullFileContents: ExperimentalDiffFullFileContents | null; className?: string; - /** - * How many unchanged lines each expand-context click reveals. Set ONLY by a - * host that received complete file contents and a successfully enriched - * `file`: pierre renders an empty diff when it is given an expansion budget - * for a hunk-only patch, which is what the timeline supplies. - */ - expansionLineCount?: number; onSelectionAddToChat?: (text: string) => void; } diff --git a/apps/app/src/components/git-diff/GitDiffCardBody.tsx b/apps/app/src/components/git-diff/GitDiffCardBody.tsx index bfd276483a..71328c455f 100644 --- a/apps/app/src/components/git-diff/GitDiffCardBody.tsx +++ b/apps/app/src/components/git-diff/GitDiffCardBody.tsx @@ -23,7 +23,6 @@ import { import { Skeleton } from "@bb/shared-ui/skeleton"; import { formatGitDiffFileLabel, - enrichGitDiffFileForContext, isPreviewableImagePath, isSvgGitDiffFile, normalizeGitDiffPath, @@ -79,7 +78,6 @@ type DiffFileEnrichmentState = | { status: "loading" } | { status: "ready"; - fileDiff: ParsedGitDiffFile; fullFileContents: ExperimentalDiffFullFileContents; } | { @@ -91,7 +89,6 @@ type DiffFileEnrichmentState = } | { status: "ready-svg"; - fileDiff: ParsedGitDiffFile; fullFileContents: ExperimentalDiffFullFileContents; oldImageUrl: string | null; newImageUrl: string | null; @@ -276,7 +273,7 @@ interface UseGitDiffCardBodyArgs { interface GitDiffCardBodyState { bodySentinelRef: RefCallback; enrichment: DiffFileEnrichmentState; - enrichedFileDiff: ParsedGitDiffFile; + fileDiff: ParsedGitDiffFile; fileDiffLabel: string; isImageCard: boolean; isSvgPreviewCard: boolean; @@ -442,12 +439,6 @@ export function useGitDiffCardBody({ setEnrichment({ status: "unavailable" }); return; } - const enrichedFileDiff = enrichGitDiffFileForContext({ - fileDiff, - oldFile: oldResult.file, - newFile: newResult.file, - patchText, - }); const fullFileContents = toDiffFullFileContents( oldResult.file, newResult.file, @@ -456,7 +447,6 @@ export function useGitDiffCardBody({ enrichmentStatusRef.current = "ready-svg"; setEnrichment({ status: "ready-svg", - fileDiff: enrichedFileDiff, fullFileContents, oldImageUrl: svgTextToDataUrl(oldResult.file.contents), newImageUrl: svgTextToDataUrl(newResult.file.contents), @@ -466,7 +456,6 @@ export function useGitDiffCardBody({ enrichmentStatusRef.current = "ready"; setEnrichment({ status: "ready", - fileDiff: enrichedFileDiff, fullFileContents, }); }) @@ -513,12 +502,6 @@ export function useGitDiffCardBody({ [contextExpansionStatus, requestContextExpansion], ); - const enrichedFileDiff = useMemo(() => { - if (enrichment.status !== "ready" && enrichment.status !== "ready-svg") { - return fileDiff; - } - return enrichment.fileDiff; - }, [fileDiff, enrichment]); const fullFileContents = enrichment.status === "ready" || enrichment.status === "ready-svg" ? enrichment.fullFileContents @@ -537,7 +520,7 @@ export function useGitDiffCardBody({ return { bodySentinelRef, enrichment, - enrichedFileDiff, + fileDiff, fileDiffLabel, isImageCard, isSvgPreviewCard: isSvgCard, @@ -857,7 +840,7 @@ export function GitDiffCardBody({ const { bodySentinelRef, enrichment, - enrichedFileDiff, + fileDiff, fileDiffLabel, isImageCard, isSvgPreviewCard, @@ -904,7 +887,7 @@ export function GitDiffCardBody({ { expect(secondHunk.deletionLineIndex).toBe(7); expect(secondHunk.collapsedBefore).toBe(5); }); + + it.each([ + { + name: "empty contents", + oldFile: { name: "src/context.ts", contents: "" }, + newFile: { name: "src/context.ts", contents: "" }, + }, + { + name: "unrelated contents", + oldFile: { + name: "src/context.ts", + contents: "not the old file\n", + }, + newFile: { + name: "src/context.ts", + contents: "not the new file\n", + }, + }, + { + name: "swapped contents", + oldFile: { + name: "src/context.ts", + contents: `${NEW_CONTEXT_CONTENT}\n`, + }, + newFile: { + name: "src/context.ts", + contents: `${OLD_CONTEXT_CONTENT}\n`, + }, + }, + { + name: "mismatched paths", + oldFile: { + name: "src/another.ts", + contents: `${OLD_CONTEXT_CONTENT}\n`, + }, + newFile: { + name: "src/another.ts", + contents: `${NEW_CONTEXT_CONTENT}\n`, + }, + }, + ])("keeps the patch partial for $name", ({ oldFile, newFile }) => { + const [file] = parseGitDiffFiles(MULTI_HUNK_DIFF); + expect(file).toBeDefined(); + if (!file) throw new Error("expected parsed file"); + + const result = enrichGitDiffFileForContext({ + fileDiff: file, + oldFile, + newFile, + patchText: MULTI_HUNK_DIFF, + }); + + expect(result).toBe(file); + expect(result.isPartial).toBe(true); + }); }); diff --git a/apps/app/src/components/git-diff/git-diff-parsing.ts b/apps/app/src/components/git-diff/git-diff-parsing.ts index 09ab500a23..687c1faa88 100644 --- a/apps/app/src/components/git-diff/git-diff-parsing.ts +++ b/apps/app/src/components/git-diff/git-diff-parsing.ts @@ -70,9 +70,11 @@ interface GitDiffContextEnrichmentInput { } /** - * Reparses a card's raw file patch with both full file sides attached. The - * diff renderer only exposes expand-context controls when `isPartial` is false - * and `additionLines` / `deletionLines` contain complete file contents. + * Reparses a raw file patch with both full file sides attached. The returned + * file is only enriched when the supplied paths and every hunk line agree with + * the original patch. `@pierre/diffs` trusts caller-supplied contents, so this + * check prevents unrelated, empty, or swapped files from being presented as + * expandable context. */ export function enrichGitDiffFileForContext({ fileDiff, @@ -80,20 +82,100 @@ export function enrichGitDiffFileForContext({ newFile, patchText, }: GitDiffContextEnrichmentInput): ParsedGitDiffFile { - if (!patchText) return fileDiff; + if (!patchText || !doFullFilePathsMatch(fileDiff, oldFile, newFile)) { + return fileDiff; + } + + // Do not inherit the partial diff's cache key. Pierre treats matching cache + // keys as semantic equality, but callers may refresh the full contents while + // retaining the same patch identity. + const enrichedFile = processFile(patchText, { oldFile, newFile }); + if ( + enrichedFile === undefined || + enrichedFile.isPartial || + !doFullFileHunksMatch(fileDiff, enrichedFile) + ) { + return fileDiff; + } + return enrichedFile; +} +function doFullFilePathsMatch( + fileDiff: ParsedGitDiffFile, + oldFile: FileContents, + newFile: FileContents, +): boolean { + const expectedNewPath = normalizeGitDiffPath(fileDiff.name); + const expectedOldPath = + normalizeGitDiffPath(fileDiff.prevName) ?? expectedNewPath; return ( - processFile(patchText, { - oldFile, - newFile, - cacheKey: - fileDiff.cacheKey === undefined - ? undefined - : `${fileDiff.cacheKey}:context`, - }) ?? fileDiff + expectedOldPath !== undefined && + expectedNewPath !== undefined && + normalizeGitDiffPath(oldFile.name) === expectedOldPath && + normalizeGitDiffPath(newFile.name) === expectedNewPath ); } +function doFullFileHunksMatch( + partialFile: ParsedGitDiffFile, + fullFile: ParsedGitDiffFile, +): boolean { + return partialFile.hunks.every( + (hunk) => + doLineRangesMatch({ + expectedLines: partialFile.deletionLines, + expectedStart: hunk.deletionLineIndex, + actualLines: fullFile.deletionLines, + actualStart: hunk.deletionStart - 1, + count: hunk.deletionCount, + }) && + doLineRangesMatch({ + expectedLines: partialFile.additionLines, + expectedStart: hunk.additionLineIndex, + actualLines: fullFile.additionLines, + actualStart: hunk.additionStart - 1, + count: hunk.additionCount, + }), + ); +} + +function doLineRangesMatch({ + expectedLines, + expectedStart, + actualLines, + actualStart, + count, +}: { + expectedLines: string[]; + expectedStart: number; + actualLines: string[]; + actualStart: number; + count: number; +}): boolean { + if ( + expectedStart < 0 || + actualStart < 0 || + count < 0 || + expectedStart + count > expectedLines.length || + actualStart + count > actualLines.length + ) { + return false; + } + for (let index = 0; index < count; index += 1) { + if ( + normalizeComparableDiffLine(expectedLines[expectedStart + index]) !== + normalizeComparableDiffLine(actualLines[actualStart + index]) + ) { + return false; + } + } + return true; +} + +function normalizeComparableDiffLine(line: string | undefined): string { + return line?.endsWith("\r\n") ? `${line.slice(0, -2)}\n` : (line ?? ""); +} + export function summarizeGitDiffFile( file: ParsedGitDiffFile, ): Pick { diff --git a/apps/app/src/components/plugin/PluginDiff.tsx b/apps/app/src/components/plugin/PluginDiff.tsx index 668285edb5..8061eb8cb9 100644 --- a/apps/app/src/components/plugin/PluginDiff.tsx +++ b/apps/app/src/components/plugin/PluginDiff.tsx @@ -1,19 +1,17 @@ import { useMemo } from "react"; import type { DiffProps } from "@get-bb/plugin-sdk"; import { DiffHost } from "@/components/code/DiffHost"; -import { - enrichGitDiffFileForContext, - normalizeFilePatch, -} from "@/components/git-diff/git-diff-parsing"; +import { normalizeFilePatch } from "@/components/git-diff/git-diff-parsing"; import { cn } from "@bb/shared-ui/lib/utils"; /** * The public `experimental_Diff` component. It normalizes whatever patch shape * the caller has (a `git diff` patch, a GitHub REST patch, a single `@@` hunk) - * into one the renderer understands, enriches it when the caller supplied both - * complete text sides, then hands it to the host boundary. Content that does - * not parse as a patch degrades to plain monospace text rather than to an empty - * diff. + * into one the renderer understands, then hands it to the host boundary. + * Content that does not parse as a patch degrades to plain monospace text + * rather than to an empty diff. Full-file enrichment stays behind the lazy + * built-in renderer so a replacement that never delegates pays none of its + * parsing cost. */ export function PluginDiff({ patch, @@ -28,23 +26,6 @@ export function PluginDiff({ () => normalizeFilePatch({ patch, path }), [patch, path], ); - const file = useMemo(() => { - if (normalized === null || fullFileContents === undefined) { - return normalized?.file ?? null; - } - return enrichGitDiffFileForContext({ - fileDiff: normalized.file, - oldFile: { - name: fullFileContents.old.path, - contents: fullFileContents.old.content, - }, - newFile: { - name: fullFileContents.new.path, - contents: fullFileContents.new.content, - }, - patchText: normalized.patch, - }); - }, [fullFileContents, normalized]); if (normalized === null) { return (
     );
   }
-  if (file === null) return null;
   return (
      crashed" chip without