From bd3d2bcd35538b63bfc69916a0e95d3f2960e89c Mon Sep 17 00:00:00 2001 From: Chris Feijoo Date: Thu, 27 Aug 2026 03:25:43 +0200 Subject: [PATCH 01/11] FE-1514: Highlight arch-docs changes against a base ref on preview builds --- apps/petrinaut-docs/README.md | 14 + apps/petrinaut-docs/astro.config.mjs | 80 +++++- apps/petrinaut-docs/src/styles/chrome.css | 39 +++ apps/petrinaut-docs/vercel-build.sh | 9 + libs/@local/petrinaut-arch-docs/README.md | 47 ++++ libs/@local/petrinaut-arch-docs/src/cli.ts | 105 +++++++- .../petrinaut-arch-docs/src/diff/annotate.ts | 76 ++++++ .../petrinaut-arch-docs/src/diff/base-tree.ts | 114 ++++++++ .../src/diff/blocks.test.ts | 144 ++++++++++ .../petrinaut-arch-docs/src/diff/blocks.ts | 181 +++++++++++++ .../src/diff/bundle-diff.test.ts | 229 ++++++++++++++++ .../src/diff/bundle-diff.ts | 247 ++++++++++++++++++ .../src/diff/normalize.test.ts | 76 ++++++ .../petrinaut-arch-docs/src/diff/normalize.ts | 47 ++++ .../src/emit/bundle-outputs.ts | 16 ++ .../src/emit/components/diff-marker.css | 58 ++++ .../src/emit/components/diff-marker.tsx | 27 ++ .../petrinaut-arch-docs/src/emit/mdx.ts | 6 +- .../src/emit/shipped-components.ts | 5 + libs/@local/petrinaut-arch-docs/turbo.json | 4 + 20 files changed, 1516 insertions(+), 8 deletions(-) create mode 100644 libs/@local/petrinaut-arch-docs/src/diff/annotate.ts create mode 100644 libs/@local/petrinaut-arch-docs/src/diff/base-tree.ts create mode 100644 libs/@local/petrinaut-arch-docs/src/diff/blocks.test.ts create mode 100644 libs/@local/petrinaut-arch-docs/src/diff/blocks.ts create mode 100644 libs/@local/petrinaut-arch-docs/src/diff/bundle-diff.test.ts create mode 100644 libs/@local/petrinaut-arch-docs/src/diff/bundle-diff.ts create mode 100644 libs/@local/petrinaut-arch-docs/src/diff/normalize.test.ts create mode 100644 libs/@local/petrinaut-arch-docs/src/diff/normalize.ts create mode 100644 libs/@local/petrinaut-arch-docs/src/emit/components/diff-marker.css create mode 100644 libs/@local/petrinaut-arch-docs/src/emit/components/diff-marker.tsx diff --git a/apps/petrinaut-docs/README.md b/apps/petrinaut-docs/README.md index 0d88a2da978..855669d547d 100644 --- a/apps/petrinaut-docs/README.md +++ b/apps/petrinaut-docs/README.md @@ -47,6 +47,20 @@ Copying sidesteps that, and matches what an embedding host does anyway. The copied directories (`src/content/docs`, `src/content/diagrams`) are gitignored, as is the bundle they come from. Nothing generated is versioned. +## Preview deployments highlight changes + +On a Vercel preview, [`vercel-build.sh`](vercel-build.sh) sets +`PETRINAUT_ARCH_DOCS_DIFF_BASE=main`, so the generator builds the bundle in +diff mode: the sidebar badges pages as `new`, `changed` or `removed` (with +roll-up counts on collapsed groups, so a change deep in a subtree is visible +from the top), and changed pages mark their differing blocks — green for +added, blue for edited, red collapsed blocks for removed content. Removed +pages keep a struck-through entry linking to a stub that carries the removed +source. Set the variable in the Vercel project to compare against a different +ref; production builds of `main` never diff. What counts as a change (and what +noise is filtered out) is the generator's contract — see its README section +"Highlighting changes against a base ref". + ## Notes on configuration Authored pages are optional. When `content/` in the generator package is empty diff --git a/apps/petrinaut-docs/astro.config.mjs b/apps/petrinaut-docs/astro.config.mjs index 9c397b468cf..499d387de4a 100644 --- a/apps/petrinaut-docs/astro.config.mjs +++ b/apps/petrinaut-docs/astro.config.mjs @@ -26,11 +26,71 @@ const manifestPath = fileURLToPath( const manifest = JSON.parse(readFileSync(manifestPath, "utf8")); /** - * @typedef {{ label: string, link: string }} SidebarLink - * @typedef {{ label: string, collapsed?: boolean, items: SidebarItem[] }} SidebarGroup + * @typedef {{ text: string, variant: "success" | "note" | "danger" }} SidebarBadge + * @typedef {{ label: string, link: string, badge?: SidebarBadge, attrs?: Record }} SidebarLink + * @typedef {{ label: string, collapsed?: boolean, items: SidebarItem[], badge?: SidebarBadge }} SidebarGroup * @typedef {SidebarLink | SidebarGroup} SidebarItem */ +/** + * Change statuses from a diff build (a preview built against a base ref). + * Empty on a plain build, so everything below renders as before. + * + * @type {Record} + */ +const diffPages = manifest.diff?.pages ?? {}; + +const DIFF_BADGES = /** @type {const} */ ({ + added: { text: "new", variant: "success" }, + changed: { text: "changed", variant: "note" }, + removed: { text: "removed", variant: "danger" }, +}); + +/** + * Badge and class for one page's link. + * + * @param {string} slug + */ +const diffLinkProps = (slug) => { + const status = diffPages[slug]; + return status === undefined + ? {} + : { + badge: DIFF_BADGES[status], + attrs: { class: `pnd-diff-${status}` }, + }; +}; + +/** + * Roll-up badge for a group: how many pages at or beneath `slug` changed, so + * a collapsed group still shows that something inside it did. Colored by the + * one status when they all match, neutral when mixed. + * + * @param {string} slug + */ +const diffGroupProps = (slug) => { + const statuses = Object.entries(diffPages) + .filter( + ([pageSlug]) => pageSlug === slug || pageSlug.startsWith(`${slug}/`), + ) + .map(([, status]) => status); + + if (statuses.length === 0) { + return {}; + } + + const uniform = statuses.every((status) => status === statuses[0]) + ? statuses[0] + : null; + + return { + badge: { + text: String(statuses.length), + variant: uniform === null ? "note" : DIFF_BADGES[uniform].variant, + }, + }; +}; + /** * Builds Starlight's nested sidebar from the manifest. * @@ -154,14 +214,19 @@ const buildSidebar = () => { // Named "Overview" rather than " overview", matching the // Architecture group and avoiding a label that repeats the // group it sits directly beneath. - items: [{ label: "Overview", link }, ...itemsUnder(page.slug)], + items: [ + { label: "Overview", link, ...diffLinkProps(page.slug) }, + ...itemsUnder(page.slug), + ], + ...diffGroupProps(page.slug), } - : { label: page.title, link }; + : { label: page.title, link, ...diffLinkProps(page.slug) }; }), ...implied.map((slug) => ({ label: labelFromSlug(slug), collapsed: true, items: itemsUnder(slug), + ...diffGroupProps(slug), })), ]; }; @@ -180,9 +245,14 @@ const buildSidebar = () => { label: "Architecture", collapsed: false, items: [ - { label: "Overview", link: `/${architectureRoot.slug}` }, + { + label: "Overview", + link: `/${architectureRoot.slug}`, + ...diffLinkProps(architectureRoot.slug), + }, ...itemsFrom(pages)(architectureRoot.slug), ], + ...diffGroupProps(architectureRoot.slug), }, ] : []), diff --git a/apps/petrinaut-docs/src/styles/chrome.css b/apps/petrinaut-docs/src/styles/chrome.css index d546473c029..b9233f65a7c 100644 --- a/apps/petrinaut-docs/src/styles/chrome.css +++ b/apps/petrinaut-docs/src/styles/chrome.css @@ -210,3 +210,42 @@ border: 1px solid color-mix(in srgb, currentColor 22%, transparent); border-radius: 0.5rem; } + +/* Diff badges ------------------------------------------------------------ */ + +/* + * On a diff build (a preview built against a base ref) the sidebar carries + * change badges from `manifest.diff`; see astro.config.mjs. The classes come + * in through the sidebar links' `attrs`. Removed pages link to tombstone + * stubs, and the strike-through is what says "gone" before the reader clicks. + */ +.pnd-diff-removed { + text-decoration: line-through; + opacity: 0.75; +} + +/* + * A row that carries a badge lays out as a flex row so the badge sits beside + * the label instead of under it — the label rule above makes every span a + * block for clipping, which would otherwise push the badge to its own line. + * The label keeps `min-width: 0` so it still clips instead of the badge. + */ +.sidebar-content a:has(> .sl-badge), +.sidebar-content .group-label:has(> .sl-badge) { + display: flex; + align-items: center; + gap: 0.4rem; +} + +.sidebar-content a:has(> .sl-badge) > span:first-child, +.sidebar-content .group-label:has(> .sl-badge) > span:first-child { + flex: 0 1 auto; + min-width: 0; +} + +.sidebar-content .sl-badge { + flex: none; + padding: 0 0.3rem; + font-size: 0.6rem; + line-height: 1.4; +} diff --git a/apps/petrinaut-docs/vercel-build.sh b/apps/petrinaut-docs/vercel-build.sh index d36257177bf..8cf361002c1 100755 --- a/apps/petrinaut-docs/vercel-build.sh +++ b/apps/petrinaut-docs/vercel-build.sh @@ -15,6 +15,15 @@ cd ../.. # See: https://linear.app/hash/issue/H-3212/clean-up-env-files rm -f .env +# Preview deployments highlight what the branch changes against main: pages and +# blocks that differ get badges and markers, driven by the generator's diff +# mode. Production builds (main itself) never diff. An already-set variable +# wins, so the Vercel project can point previews at a different base. +if [[ "${VERCEL_ENV:-}" == "preview" && -z "${PETRINAUT_ARCH_DOCS_DIFF_BASE:-}" ]]; then + export PETRINAUT_ARCH_DOCS_DIFF_BASE="main" + echo "Preview build: highlighting changes against main" +fi + # Run through Turborepo rather than `yarn workspace ... build`: the package # script alone skips `sync:bundle`, and would build whatever content happened to # be on disk. diff --git a/libs/@local/petrinaut-arch-docs/README.md b/libs/@local/petrinaut-arch-docs/README.md index 9556560c95a..bd73a29d7b9 100644 --- a/libs/@local/petrinaut-arch-docs/README.md +++ b/libs/@local/petrinaut-arch-docs/README.md @@ -173,6 +173,53 @@ local build at another ref. A preview deployment of `apps/petrinaut-docs` reads Vercel's variables, so its links point at the commit it was built from instead of at a file `main` may not have yet. +### Highlighting changes against a base ref + +A build can compare itself against another version of the repository and mark +what differs, which is how a PR preview shows a reviewer where to look: + +```sh +# Locally +yarn workspace @local/petrinaut-arch-docs doc:architecture --diff-base main + +# In CI (what vercel-build.sh sets on preview deployments) +PETRINAUT_ARCH_DOCS_DIFF_BASE=main turbo build --filter '@apps/petrinaut-docs' +``` + +The build extracts the covered package trees at the base ref with `git archive` +and runs the _same_ generator over them — same config, same emitter, same +source-URL prefix — so nothing but a real change can differ between the two +sides. No install and no `node_modules` are needed for the base tree: workspace +imports resolve through aliases derived from each package's `exports` map. + +What it produces: + +- `manifest.json` gains a `diff` section mapping page slugs to `added`, + `changed` or `removed`, for a host to build navigation badges from. +- Changed pages carry `<DiffMarker>` elements between blocks; the shipped + `diff-marker` component and its stylesheet render a green bar on added + blocks, a blue bar on edited ones, and removed content as a red, collapsed + block in place. +- A removed page becomes a tombstone stub at its old slug, carrying the + removed source, so navigation can show it (struck through) instead of it + silently disappearing. + +**What counts as a change** is deliberately narrower than a byte diff, because +a generated page embeds facts that shift whenever _neighbouring_ code moves. +Masked before comparison: import counts, file and line totals, sidebar +positions, and the whole "depended on by" list — an incoming edge is the +importing layer's change and is flagged there, on its `dependsOn` side. A page +is `changed` when anything else differs: its role, prose, name, declaring +file, outgoing edges, sub-layers, attached guides — or when the layer's file +membership moved, the one structural change the masked content cannot show. +Editing code inside a layer without moving files or edges changes nothing the +architecture describes, and flags nothing. + +The diff decorates the bundle, it never gates it: when the base ref cannot be +resolved (say, a shallow clone with no way to fetch it) the build warns and +writes a plain bundle. Production builds of `main` never set the variable, so +they never diff. + ### Embedding the bundle elsewhere A host reads `manifest.json`, maps each page's `slug` onto its own URL space, and diff --git a/libs/@local/petrinaut-arch-docs/src/cli.ts b/libs/@local/petrinaut-arch-docs/src/cli.ts index 891f4cd326a..a565c58f615 100644 --- a/libs/@local/petrinaut-arch-docs/src/cli.ts +++ b/libs/@local/petrinaut-arch-docs/src/cli.ts @@ -6,6 +6,7 @@ * violated rule — so the map cannot quietly stop matching the code. */ +import { existsSync } from "node:fs"; import { chmod, mkdir, rm, writeFile } from "node:fs/promises"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; @@ -13,6 +14,8 @@ import { fileURLToPath } from "node:url"; import { config } from "../architecture.config"; import { buildBundle, bundleTextFiles, type BuiltBundle } from "./build"; import { countErrors, type Diagnostic } from "./diagnostics"; +import { materializeBaseTree, type BaseTree } from "./diff/base-tree"; +import { applyBundleDiff } from "./diff/bundle-diff"; import { canRenderDiagrams, renderD2 } from "./emit/d2"; const repoRoot = fileURLToPath(new URL("../../../../", import.meta.url)); @@ -61,6 +64,98 @@ const summarise = (bundle: BuiltBundle): void => { ); }; +/** + * The ref to diff the bundle against, or null for a plain build. + * + * `--diff-base <ref>` (or `--diff-base=<ref>`) wins over the + * `PETRINAUT_ARCH_DOCS_DIFF_BASE` environment variable, which is how CI turns + * the diff on for preview builds without touching the command line. + */ +const resolveDiffBase = (args: string[]): string | null => { + const flagIndex = args.findIndex( + (arg) => arg === "--diff-base" || arg.startsWith("--diff-base="), + ); + + if (flagIndex !== -1) { + const flag = args[flagIndex] ?? ""; + const value = flag.includes("=") + ? flag.slice(flag.indexOf("=") + 1) + : (args[flagIndex + 1] ?? ""); + return value.trim() === "" ? null : value.trim(); + } + + const fromEnvironment = process.env.PETRINAUT_ARCH_DOCS_DIFF_BASE?.trim(); + return fromEnvironment === undefined || fromEnvironment === "" + ? null + : fromEnvironment; +}; + +/** + * Builds the base ref's bundle and applies the diff to the head one. + * + * The base build runs the current generator over the base sources, with the + * current config and source-URL prefix, so the comparison never sees emitter + * or prefix drift. Any failure — an unfetchable ref, a base tree the + * generator cannot process — degrades to the plain bundle with a warning: a + * broken diff must not take the docs down with it. + */ +const withDiffAgainst = async ( + bundle: BuiltBundle, + ref: string, + includeDiagrams: boolean, +): Promise<BuiltBundle> => { + let baseTree: BaseTree | null = null; + try { + const tree = await materializeBaseTree({ + repoRoot, + ref, + paths: [ + ...new Set([ + ...config.packages.map((pkg) => pkg.path), + // For the base tree's authored content and dependency-cruiser + // tsconfig; the generator itself still runs from this checkout. + "libs/@local/petrinaut-arch-docs", + ]), + ], + }); + baseTree = tree; + + const baseBundle = await buildBundle({ + repoRoot: tree.root, + includeDiagrams, + overrides: { + // A package added since the base ref has no directory to scan there. + packages: config.packages.filter((pkg) => + existsSync(join(tree.root, pkg.path)), + ), + }, + }); + + const diffed = applyBundleDiff(bundle, baseBundle, { + baseRef: ref, + baseSha: tree.sha, + }); + + const statuses = Object.values(diffed.manifest.diff?.pages ?? {}); + const count = (status: string) => + statuses.filter((entry) => entry === status).length; + process.stdout.write( + dim( + `changes vs ${ref} (${tree.sha.slice(0, 10)}): ${count("added")} added · ${count("changed")} changed · ${count("removed")} removed\n`, + ), + ); + + return diffed; + } catch (cause) { + process.stderr.write( + `${yellow("warning")} building without change highlighting: could not compare against \`${ref}\`\n ${cause instanceof Error ? cause.message : String(cause)}\n`, + ); + return bundle; + } finally { + await baseTree?.dispose(); + } +}; + /** Returns false when a diagram the pages already reference failed to render. */ const writeBundle = async ( bundle: BuiltBundle, @@ -152,7 +247,15 @@ const main = async (): Promise<number> => { return 1; } - if (!(await writeBundle(bundle, diagramsAvailable))) { + // Applied only to a bundle that already passed the checks: the diff decorates + // the output, it never gates it. + const diffBase = resolveDiffBase(process.argv.slice(3)); + const written = + diffBase === null + ? bundle + : await withDiffAgainst(bundle, diffBase, diagramsAvailable); + + if (!(await writeBundle(written, diagramsAvailable))) { return 1; } diff --git a/libs/@local/petrinaut-arch-docs/src/diff/annotate.ts b/libs/@local/petrinaut-arch-docs/src/diff/annotate.ts new file mode 100644 index 00000000000..9f34dba6bd5 --- /dev/null +++ b/libs/@local/petrinaut-arch-docs/src/diff/annotate.ts @@ -0,0 +1,76 @@ +/** + * Rewrites a changed page's MDX so each differing block carries a marker. + * + * Markers are emitted *between* blocks rather than wrapping them: the shipped + * `DiffMarker` component renders an invisible element and its stylesheet + * decorates the next sibling, so the markdown itself is never nested inside + * JSX and heading extraction, anchors and the table of contents behave exactly + * as they do on an unannotated page. A removed run has no head block to mark, + * so its marker is visible: it renders the removed source, collapsed. + */ + +import { assetPathFrom } from "../emit/mdx"; +import { DIFF_MARKER_MODULE } from "../emit/shipped-components"; + +import type { BlockDiff } from "./blocks"; + +/** + * Splits a page into its frontmatter (empty when absent) and body, so blocks + * can be diffed without the frontmatter counting as one of them. + */ +export const splitFrontmatter = ( + contents: string, +): { frontmatter: string; body: string } => { + const match = /^---\n[\s\S]*?\n---\n/u.exec(contents); + return match === null + ? { frontmatter: "", body: contents } + : { frontmatter: match[0], body: contents.slice(match[0].length) }; +}; + +const marker = (status: "added" | "changed"): string => + `<DiffMarker status="${status}" />`; + +const removedMarker = (baseBlocks: string[]): string => + `<DiffMarker status="removed" content={${JSON.stringify(baseBlocks.join("\n\n"))}} />`; + +/** + * Reassembles a page from its parts with markers inserted. + * + * Blocks are rejoined with single blank lines, which can differ from the + * original spacing byte-for-byte but not in what MDX renders. Only pages that + * actually get markers pass through here; an unchanged page keeps its + * original bytes. + */ +export const annotatePageBlocks = (options: { + slug: string; + frontmatter: string; + blocks: string[]; + diff: BlockDiff; +}): string => { + const { slug, frontmatter, blocks, diff } = options; + + const importLine = `import { DiffMarker } from "${assetPathFrom(slug, `components/${DIFF_MARKER_MODULE}`)}";`; + + const parts: string[] = []; + + blocks.forEach((block, index) => { + const removedHere = diff.removed.get(index); + if (removedHere !== undefined) { + parts.push(removedMarker(removedHere)); + } + + const status = diff.headStatuses[index] ?? "unchanged"; + if (status !== "unchanged") { + parts.push(marker(status)); + } + + parts.push(block); + }); + + const removedAtEnd = diff.removed.get(blocks.length); + if (removedAtEnd !== undefined) { + parts.push(removedMarker(removedAtEnd)); + } + + return `${frontmatter}\n${importLine}\n\n${parts.join("\n\n")}\n`; +}; diff --git a/libs/@local/petrinaut-arch-docs/src/diff/base-tree.ts b/libs/@local/petrinaut-arch-docs/src/diff/base-tree.ts new file mode 100644 index 00000000000..83c87d5007d --- /dev/null +++ b/libs/@local/petrinaut-arch-docs/src/diff/base-tree.ts @@ -0,0 +1,114 @@ +/** + * Materializes the covered source trees at a base ref, for a diff build. + * + * `git archive` extracts only the directories the generator scans rather than + * checking out the whole monorepo, and the *current* generator then runs over + * the extracted tree — dependency-cruiser resolves workspace imports through + * aliases derived from each package's `exports` map, so the tree needs no + * `node_modules` and no install step. + * + * Everything here can fail in a legitimate environment — a shallow CI clone + * without the base ref, no network to fetch it — so callers treat a throw as + * "build without the diff", never as a build failure. + */ + +import { execFileSync } from "node:child_process"; +import { mkdtemp, realpath, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +export interface BaseTree { + /** Absolute root of the extracted tree, laid out repo-relative. */ + root: string; + ref: string; + /** The commit the ref resolved to. */ + sha: string; + dispose: () => Promise<void>; +} + +const git = (repoRoot: string, args: string[]): string => + execFileSync("git", args, { + cwd: repoRoot, + encoding: "utf8", + stdio: ["ignore", "pipe", "pipe"], + }).trim(); + +/** + * Resolves a ref to a commit, fetching it when the local clone lacks it. + * + * A CI clone is typically shallow and checked out at the head commit only, so + * the base branch resolves neither bare nor as `origin/<ref>` — the fetch is + * the path most CI builds actually take. + */ +const resolveCommit = (repoRoot: string, ref: string): string => { + for (const candidate of [ref, `origin/${ref}`]) { + try { + return git(repoRoot, [ + "rev-parse", + "--verify", + "--quiet", + `${candidate}^{commit}`, + ]); + } catch { + // Try the next form. + } + } + + git(repoRoot, ["fetch", "--depth=1", "origin", ref]); + return git(repoRoot, [ + "rev-parse", + "--verify", + "--quiet", + "FETCH_HEAD^{commit}", + ]); +}; + +export const materializeBaseTree = async (options: { + repoRoot: string; + ref: string; + /** Repo-relative directories to extract; ones absent at the ref are skipped. */ + paths: string[]; +}): Promise<BaseTree> => { + const sha = resolveCommit(options.repoRoot, options.ref); + // Canonicalised because macOS puts the temp directory behind a symlink + // (`/var` → `/private/var`): dependency-cruiser realpaths what it resolves, + // and against a symlinked root every resolved file appears to escape the + // tree, which silently drops every TypeScript edge from the base model. + const root = await realpath(await mkdtemp(join(tmpdir(), "arch-docs-base-"))); + const archive = join(root, ".base.tar"); + + let extracted = 0; + for (const path of options.paths) { + try { + // Written to a file and extracted in a second step: a pipe would need a + // shell with `pipefail` for `git archive`'s failure to be visible at all. + execFileSync( + "git", + ["archive", "--format=tar", "-o", archive, sha, "--", path], + { cwd: options.repoRoot, stdio: ["ignore", "ignore", "pipe"] }, + ); + } catch { + // The path does not exist at the ref, e.g. a package added since. + continue; + } + execFileSync("tar", ["-xf", archive, "-C", root], { + stdio: ["ignore", "ignore", "pipe"], + }); + extracted += 1; + } + await rm(archive, { force: true }); + + if (extracted === 0) { + await rm(root, { recursive: true, force: true }); + throw new Error( + `none of the covered directories exist at \`${options.ref}\``, + ); + } + + return { + root, + ref: options.ref, + sha, + dispose: () => rm(root, { recursive: true, force: true }), + }; +}; diff --git a/libs/@local/petrinaut-arch-docs/src/diff/blocks.test.ts b/libs/@local/petrinaut-arch-docs/src/diff/blocks.test.ts new file mode 100644 index 00000000000..d53c8a520d8 --- /dev/null +++ b/libs/@local/petrinaut-arch-docs/src/diff/blocks.test.ts @@ -0,0 +1,144 @@ +import { describe, expect, it } from "vitest"; + +import { diffBlocks, splitBlocks } from "./blocks"; + +/** + * Splitting decides where a marker may legally be inserted, so the cases that + * matter are the ones where a naive blank-line split would land a marker + * inside a construct: a fenced code block with blank lines, a JSX element with + * markdown children. Either would fail the consuming site's MDX compile. + */ +describe("splitBlocks", () => { + it("splits on blank lines", () => { + expect(splitBlocks("one\n\ntwo\n\n\nthree\n")).toEqual([ + "one", + "two", + "three", + ]); + }); + + it("keeps a fenced code block with blank lines as one block", () => { + const body = "before\n\n```ts\nconst a = 1;\n\nconst b = 2;\n```\n\nafter"; + expect(splitBlocks(body)).toEqual([ + "before", + "```ts\nconst a = 1;\n\nconst b = 2;\n```", + "after", + ]); + }); + + it("keeps a JSX element with blank-lined children as one block", () => { + const body = + "<Sequence>\n\nSome **markdown** inside.\n\n</Sequence>\n\nafter"; + expect(splitBlocks(body)).toEqual([ + "<Sequence>\n\nSome **markdown** inside.\n\n</Sequence>", + "after", + ]); + }); + + it("treats a multi-line self-closing element as one closed block", () => { + const body = '<LayerFacts\n files={3}\n layerId={"core"}\n/>\n\nafter'; + expect(splitBlocks(body)).toEqual([ + '<LayerFacts\n files={3}\n layerId={"core"}\n/>', + "after", + ]); + }); + + it("does not merge on JSX mentioned in inline code", () => { + const body = "Use `<Sequence>` for timelines.\n\nafter"; + expect(splitBlocks(body)).toEqual([ + "Use `<Sequence>` for timelines.", + "after", + ]); + }); +}); + +const identity = (block: string): string => block; + +describe("diffBlocks", () => { + it("reports nothing on equal blocks", () => { + const diff = diffBlocks({ + baseBlocks: ["a", "b"], + headBlocks: ["a", "b"], + normalize: identity, + }); + + expect(diff.headStatuses).toEqual(["unchanged", "unchanged"]); + expect(diff.removed.size).toBe(0); + }); + + it("marks an inserted block added", () => { + const diff = diffBlocks({ + baseBlocks: ["a", "c"], + headBlocks: ["a", "b", "c"], + normalize: identity, + }); + + expect(diff.headStatuses).toEqual(["unchanged", "added", "unchanged"]); + expect(diff.removed.size).toBe(0); + }); + + it("records a removed block at the head position it preceded", () => { + const diff = diffBlocks({ + baseBlocks: ["a", "b", "c"], + headBlocks: ["a", "c"], + normalize: identity, + }); + + expect(diff.headStatuses).toEqual(["unchanged", "unchanged"]); + expect(diff.removed).toEqual(new Map([[1, ["b"]]])); + }); + + it("records a run removed from the end after the last head block", () => { + const diff = diffBlocks({ + baseBlocks: ["a", "b", "c"], + headBlocks: ["a"], + normalize: identity, + }); + + expect(diff.removed).toEqual(new Map([[1, ["b", "c"]]])); + }); + + it("pairs a replaced block as changed rather than removed plus added", () => { + const diff = diffBlocks({ + baseBlocks: ["a", "old", "c"], + headBlocks: ["a", "new", "c"], + normalize: identity, + }); + + expect(diff.headStatuses).toEqual(["unchanged", "changed", "unchanged"]); + expect(diff.removed.size).toBe(0); + }); + + it("splits an uneven replace run into changed, added and removed", () => { + const grew = diffBlocks({ + baseBlocks: ["a", "old", "z"], + headBlocks: ["a", "new-1", "new-2", "z"], + normalize: identity, + }); + expect(grew.headStatuses).toEqual([ + "unchanged", + "changed", + "added", + "unchanged", + ]); + + const shrank = diffBlocks({ + baseBlocks: ["a", "old-1", "old-2", "z"], + headBlocks: ["a", "new", "z"], + normalize: identity, + }); + expect(shrank.headStatuses).toEqual(["unchanged", "changed", "unchanged"]); + expect(shrank.removed).toEqual(new Map([[2, ["old-2"]]])); + }); + + it("compares normalized forms but reports raw base blocks", () => { + const diff = diffBlocks({ + baseBlocks: ["count 1", "gone 1"], + headBlocks: ["count 2"], + normalize: (block) => (block.startsWith("count") ? "count N" : block), + }); + + expect(diff.headStatuses).toEqual(["unchanged"]); + expect(diff.removed).toEqual(new Map([[1, ["gone 1"]]])); + }); +}); diff --git a/libs/@local/petrinaut-arch-docs/src/diff/blocks.ts b/libs/@local/petrinaut-arch-docs/src/diff/blocks.ts new file mode 100644 index 00000000000..9f52a76099d --- /dev/null +++ b/libs/@local/petrinaut-arch-docs/src/diff/blocks.ts @@ -0,0 +1,181 @@ +/** + * Block-level diffing of page bodies. + * + * A "block" is what a reader perceives as one unit on the rendered page: a + * paragraph, a heading, a table, a fenced code block, one JSX card. Diffing at + * that granularity is deliberate — a per-word diff of generated MDX would + * highlight serialization details no reader cares about, while a per-page diff + * cannot say *where* a page changed. + */ + +export type BlockStatus = "unchanged" | "added" | "changed"; + +export interface BlockDiff { + /** One status per head block, aligned with the input's head blocks. */ + headStatuses: BlockStatus[]; + /** + * Base blocks with no head counterpart, keyed by the head block index they + * precede. A run removed at the end of the page keys `headBlocks.length`. + */ + removed: Map<number, string[]>; +} + +/** + * Counts JSX element openings a text leaves unclosed. + * + * Only capitalised tags count — those are the component elements MDX treats as + * JSX, and lowercase `<` in prose ("a < b") must not look like markup. Inline + * code is stripped first so a page *documenting* JSX does not appear to open + * elements. + */ +const openJsxElements = (text: string): number => { + const withoutInlineCode = text.replace(/`[^`\n]*`/gu, ""); + const opens = withoutInlineCode.match(/<[A-Z][\w.]*/gu)?.length ?? 0; + const closes = + (withoutInlineCode.match(/\/>/gu)?.length ?? 0) + + (withoutInlineCode.match(/<\/[A-Z][\w.]*>/gu)?.length ?? 0); + return opens - closes; +}; + +/** + * Splits a page body (frontmatter already removed) into blocks. + * + * Blocks are blank-line separated, with two exceptions that keep multi-part + * constructs whole: blank lines inside a fenced code block do not split, and a + * segment that leaves a JSX element open absorbs the following segments until + * the element closes. Splitting inside either would let a marker be inserted + * into the middle of a fence or an element, which fails the consuming site's + * MDX compile. + */ +export const splitBlocks = (body: string): string[] => { + const segments: string[] = []; + let current: string[] = []; + let insideFence = false; + + const flush = () => { + if (current.length > 0) { + segments.push(current.join("\n")); + current = []; + } + }; + + for (const line of body.split("\n")) { + if (/^ {0,3}(?:```|~~~)/u.test(line)) { + insideFence = !insideFence; + current.push(line); + continue; + } + + if (!insideFence && line.trim() === "") { + flush(); + continue; + } + + current.push(line); + } + flush(); + + const blocks: string[] = []; + for (const segment of segments) { + const previous = blocks.at(-1); + if (previous !== undefined && openJsxElements(previous) > 0) { + blocks[blocks.length - 1] = `${previous}\n\n${segment}`; + } else { + blocks.push(segment); + } + } + + return blocks; +}; + +/** Longest-common-subsequence table over normalized block equality. */ +const lcsTable = (base: string[], head: string[]): number[][] => { + const table: number[][] = Array.from({ length: base.length + 1 }, () => + Array.from({ length: head.length + 1 }, () => 0), + ); + + for (let row = base.length - 1; row >= 0; row -= 1) { + for (let column = head.length - 1; column >= 0; column -= 1) { + table[row]![column] = + base[row] === head[column] + ? table[row + 1]![column + 1]! + 1 + : Math.max(table[row + 1]![column]!, table[row]![column + 1]!); + } + } + + return table; +}; + +/** + * Diffs two block lists, comparing by their *normalized* forms. + * + * `normalize` masks derived noise (counts, orders) before comparison, while + * statuses stay aligned with the raw head blocks so the caller can annotate + * the real content. Within a run where the diff both removes and inserts, + * blocks pair positionally and report as `changed` — a paragraph rewritten in + * place reads as an edit, not as an unrelated removal plus addition. Excess + * inserts are `added`, excess removals join `removed`. + */ +export const diffBlocks = (options: { + baseBlocks: string[]; + headBlocks: string[]; + normalize: (block: string) => string; +}): BlockDiff => { + const base = options.baseBlocks.map(options.normalize); + const head = options.headBlocks.map(options.normalize); + const table = lcsTable(base, head); + + const headStatuses: BlockStatus[] = []; + const removed = new Map<number, string[]>(); + + let baseIndex = 0; + let headIndex = 0; + let pendingRemoved: string[] = []; + let pendingAdded = 0; + + const flushRun = () => { + const paired = Math.min(pendingRemoved.length, pendingAdded); + for (let offset = 0; offset < pendingAdded; offset += 1) { + headStatuses.push(offset < paired ? "changed" : "added"); + } + const unpaired = pendingRemoved.slice(paired); + if (unpaired.length > 0) { + removed.set(headStatuses.length, [ + ...(removed.get(headStatuses.length) ?? []), + ...unpaired, + ]); + } + pendingRemoved = []; + pendingAdded = 0; + }; + + while (baseIndex < base.length || headIndex < head.length) { + if ( + baseIndex < base.length && + headIndex < head.length && + base[baseIndex] === head[headIndex] + ) { + flushRun(); + headStatuses.push("unchanged"); + baseIndex += 1; + headIndex += 1; + continue; + } + + if ( + headIndex < head.length && + (baseIndex === base.length || + table[baseIndex]![headIndex + 1]! >= table[baseIndex + 1]![headIndex]!) + ) { + pendingAdded += 1; + headIndex += 1; + continue; + } + + pendingRemoved.push(options.baseBlocks[baseIndex]!); + baseIndex += 1; + } + flushRun(); + + return { headStatuses, removed }; +}; diff --git a/libs/@local/petrinaut-arch-docs/src/diff/bundle-diff.test.ts b/libs/@local/petrinaut-arch-docs/src/diff/bundle-diff.test.ts new file mode 100644 index 00000000000..18b152e26a8 --- /dev/null +++ b/libs/@local/petrinaut-arch-docs/src/diff/bundle-diff.test.ts @@ -0,0 +1,229 @@ +import { describe, expect, it } from "vitest"; + +import { diffBundlePages, type DiffPage, type DiffSide } from "./bundle-diff"; + +import type { Layer } from "../model"; + +/** + * The classification is the contract this feature stands on: a reviewer must + * see the pages this change actually touched, and must NOT see every + * neighbour lit up because counts drifted. The transitive-noise case pins the + * second half explicitly. + */ + +const layer = (id: string, files: string[]): Layer => ({ + id, + name: id, + parent: id.includes(".") ? id.slice(0, id.lastIndexOf(".")) : null, + package: "@test/pkg", + role: `role of ${id}`, + declaredIn: `src/${id}/index.ts`, + prose: null, + references: [], + files, + fileCount: files.length, + lineCount: files.length * 10, +}); + +const generatedPage = (options: { + slug: string; + title?: string; + role?: string; + order?: number; + lines?: number; + dependsOn?: { id: string; imports: number }[]; + dependedOnBy?: { id: string; imports: number }[]; +}): DiffPage => { + const title = options.title ?? "Core"; + const role = options.role ?? "Headless engine."; + + const relationsProp = ( + key: string, + entries: { id: string; imports: number }[], + ): string => + entries.length === 0 + ? ` ${key}={[]}` + : [ + ` ${key}={[`, + ...entries.map((entry) => ` ${JSON.stringify(entry)},`), + " ]}", + ].join("\n"); + + const contents = [ + "---", + `title: ${JSON.stringify(title)}`, + `description: ${JSON.stringify(role)}`, + `sidebar_order: ${options.order ?? 1001}`, + "---", + "", + 'import { LayerRelations } from "../components/layer-relations";', + "", + role, + "", + "<LayerFacts", + " files={3}", + ` lines={${options.lines ?? 120}}`, + "/>", + "", + "<LayerRelations", + relationsProp("dependsOn", options.dependsOn ?? []), + relationsProp("dependedOnBy", options.dependedOnBy ?? []), + "/>", + "", + ].join("\n"); + + return { + slug: options.slug, + title, + description: role, + order: options.order ?? 1001, + kind: "generated", + contents, + }; +}; + +const authoredPage = (slug: string, body: string): DiffPage => ({ + slug, + title: "Guide", + description: "", + order: 10, + kind: "authored", + contents: `---\ntitle: "Guide"\n---\n\n${body}\n`, +}); + +const side = (pages: DiffPage[], layers: Layer[] = []): DiffSide => ({ + pages, + layers, +}); + +describe("diffBundlePages", () => { + it("reports nothing when the sides are identical", () => { + const pages = [generatedPage({ slug: "architecture/core" })]; + + const result = diffBundlePages({ + base: side(pages), + head: side(pages), + baseRef: "main", + }); + + expect(result.statuses).toEqual({}); + expect(result.annotated.size).toBe(0); + expect(result.tombstones).toEqual([]); + }); + + it("ignores transitive drift: counts, order, and incoming edges", () => { + const base = generatedPage({ + slug: "architecture/core", + order: 1001, + lines: 120, + dependsOn: [{ id: "core.hir", imports: 2 }], + dependedOnBy: [], + }); + const head = generatedPage({ + slug: "architecture/core", + order: 1004, + lines: 260, + dependsOn: [{ id: "core.hir", imports: 9 }], + dependedOnBy: [{ id: "ui.panels", imports: 7 }], + }); + + const result = diffBundlePages({ + base: side([base]), + head: side([head]), + baseRef: "main", + }); + + expect(result.statuses).toEqual({}); + }); + + it("flags a role change and marks the changed block", () => { + const result = diffBundlePages({ + base: side([generatedPage({ slug: "architecture/core" })]), + head: side([ + generatedPage({ slug: "architecture/core", role: "New role." }), + ]), + baseRef: "main", + }); + + expect(result.statuses).toEqual({ "architecture/core": "changed" }); + + const annotated = result.annotated.get("architecture/core"); + expect(annotated).toContain( + 'import { DiffMarker } from "../../components/diff-marker";', + ); + expect(annotated).toContain('<DiffMarker status="changed" />\n\nNew role.'); + }); + + it("flags a new outgoing dependency", () => { + const result = diffBundlePages({ + base: side([generatedPage({ slug: "architecture/core" })]), + head: side([ + generatedPage({ + slug: "architecture/core", + dependsOn: [{ id: "core.hir", imports: 1 }], + }), + ]), + baseRef: "main", + }); + + expect(result.statuses).toEqual({ "architecture/core": "changed" }); + }); + + it("classifies a page only the head has as added, without markers", () => { + const result = diffBundlePages({ + base: side([]), + head: side([generatedPage({ slug: "architecture/core" })]), + baseRef: "main", + }); + + expect(result.statuses).toEqual({ "architecture/core": "added" }); + expect(result.annotated.size).toBe(0); + }); + + it("emits a tombstone carrying the removed page's source", () => { + const result = diffBundlePages({ + base: side([ + generatedPage({ slug: "architecture/core/old", title: "Old" }), + ]), + head: side([]), + baseRef: "main", + }); + + expect(result.statuses).toEqual({ "architecture/core/old": "removed" }); + + const [tombstone] = result.tombstones; + expect(tombstone?.title).toBe("Old"); + expect(tombstone?.contents).toContain('title: "Old"'); + expect(tombstone?.contents).toContain('<DiffMarker status="removed"'); + // The removed body travels as a string prop, inert to MDX. + expect(tombstone?.contents).toContain("LayerFacts"); + }); + + it("flags moved file membership even though the content is identical", () => { + const contents = generatedPage({ slug: "architecture/core" }); + + const result = diffBundlePages({ + base: side([contents], [layer("core", ["src/a.ts"])]), + head: side([contents], [layer("core", ["src/a.ts", "src/b.ts"])]), + baseRef: "main", + }); + + expect(result.statuses).toEqual({ "architecture/core": "changed" }); + expect(result.annotated.size).toBe(0); + }); + + it("diffs authored pages verbatim, marking removed blocks", () => { + const result = diffBundlePages({ + base: side([ + authoredPage("guides/setup", "Intro.\n\nDropped paragraph."), + ]), + head: side([authoredPage("guides/setup", "Intro.")]), + baseRef: "main", + }); + + expect(result.statuses).toEqual({ "guides/setup": "changed" }); + expect(result.annotated.get("guides/setup")).toContain( + '<DiffMarker status="removed" content={"Dropped paragraph."} />', + ); + }); +}); diff --git a/libs/@local/petrinaut-arch-docs/src/diff/bundle-diff.ts b/libs/@local/petrinaut-arch-docs/src/diff/bundle-diff.ts new file mode 100644 index 00000000000..59dd142e348 --- /dev/null +++ b/libs/@local/petrinaut-arch-docs/src/diff/bundle-diff.ts @@ -0,0 +1,247 @@ +/** + * Classifies every page against a base build and rewrites the changed ones. + * + * Both sides come from the *same* generator run over two source trees, so the + * emitter, the config and the source-URL prefix cannot differ between them — + * what survives normalization is a real change. The classification is per + * page: `added` and `removed` by slug presence, `changed` when the normalized + * content differs or the layer's own file membership moved. Removed pages get + * a stub ("tombstone") page carrying the removed source, so navigation has + * something to point at. + */ + +import { buildManifest } from "../emit/bundle-outputs"; +import { assetPathFrom, frontmatter, layerSlug } from "../emit/mdx"; +import { DIFF_MARKER_MODULE } from "../emit/shipped-components"; +import { annotatePageBlocks, splitFrontmatter } from "./annotate"; +import { diffBlocks, splitBlocks } from "./blocks"; +import { normalizeGeneratedBlock } from "./normalize"; + +import type { BuiltBundle } from "../build"; +import type { Layer } from "../model"; + +export type PageChange = "added" | "changed" | "removed"; + +/** The slice of a page the diff needs, common to generated and authored. */ +export interface DiffPage { + slug: string; + title: string; + description: string; + order: number; + kind: "generated" | "authored"; + contents: string; +} + +export interface DiffSide { + pages: DiffPage[]; + layers: Layer[]; +} + +export interface PageDiffResult { + /** Page slug → how it differs from the base. Unchanged slugs are absent. */ + statuses: Record<string, PageChange>; + /** Changed pages rewritten with block markers, by slug. */ + annotated: Map<string, string>; + /** Stub pages standing in for removed ones, so navigation can show them. */ + tombstones: DiffPage[]; +} + +/** + * Layers whose file membership changed between the two models. + * + * The one structural change a page's normalized content cannot show: counts + * are masked, and the page never lists its files. Everything else about a + * layer — role, prose, name, parent, declaring file, outgoing edges, + * references — already appears in the content and is compared there. + */ +const layersWithMovedFiles = ( + baseLayers: Layer[], + headLayers: Layer[], +): Set<string> => { + const baseFiles = new Map( + baseLayers.map((layer) => [layer.id, [...layer.files].sort().join("\n")]), + ); + + return new Set( + headLayers + .filter((layer) => { + const base = baseFiles.get(layer.id); + return ( + base !== undefined && base !== [...layer.files].sort().join("\n") + ); + }) + .map((layer) => layer.id), + ); +}; + +const identity = (block: string): string => block; + +const tombstonePage = (page: DiffPage, baseRef: string): DiffPage => { + const { body } = splitFrontmatter(page.contents); + + const contents = [ + frontmatter({ + title: page.title, + description: `Removed since ${baseRef}.`, + sidebar_order: page.order, + }), + `import { DiffMarker } from "${assetPathFrom(page.slug, `components/${DIFF_MARKER_MODULE}`)}";`, + "", + `> **Removed** — this page existed at \`${baseRef}\` and was removed by this change.`, + "", + `<DiffMarker status="removed" content={${JSON.stringify(body.trim())}} />`, + "", + ].join("\n"); + + return { + slug: page.slug, + title: page.title, + description: `Removed since ${baseRef}.`, + order: page.order, + kind: "generated", + contents, + }; +}; + +export const diffBundlePages = (options: { + base: DiffSide; + head: DiffSide; + baseRef: string; +}): PageDiffResult => { + const baseBySlug = new Map( + options.base.pages.map((page) => [page.slug, page]), + ); + const headSlugs = new Set(options.head.pages.map((page) => page.slug)); + + const movedFileSlugs = new Set( + [...layersWithMovedFiles(options.base.layers, options.head.layers)].map( + layerSlug, + ), + ); + + const statuses: Record<string, PageChange> = {}; + const annotated = new Map<string, string>(); + + for (const page of options.head.pages) { + const basePage = baseBySlug.get(page.slug); + + if (basePage === undefined) { + statuses[page.slug] = "added"; + continue; + } + + const normalize = + page.kind === "generated" ? normalizeGeneratedBlock : identity; + + const head = splitFrontmatter(page.contents); + const base = splitFrontmatter(basePage.contents); + + const diff = diffBlocks({ + baseBlocks: splitBlocks(base.body), + headBlocks: splitBlocks(head.body), + normalize, + }); + + const bodyChanged = + diff.removed.size > 0 || + diff.headStatuses.some((status) => status !== "unchanged"); + const frontmatterChanged = + normalize(base.frontmatter) !== normalize(head.frontmatter); + + if (!bodyChanged && !frontmatterChanged && !movedFileSlugs.has(page.slug)) { + continue; + } + + statuses[page.slug] = "changed"; + + if (bodyChanged) { + annotated.set( + page.slug, + annotatePageBlocks({ + slug: page.slug, + frontmatter: head.frontmatter, + blocks: splitBlocks(head.body), + diff, + }), + ); + } + } + + const tombstones = options.base.pages + .filter((page) => !headSlugs.has(page.slug)) + .map((page) => tombstonePage(page, options.baseRef)); + + for (const tombstone of tombstones) { + statuses[tombstone.slug] = "removed"; + } + + return { statuses, annotated, tombstones }; +}; + +const toDiffPages = (bundle: BuiltBundle): DiffPage[] => [ + ...bundle.generated.map((page) => ({ + slug: page.slug, + title: page.title, + description: page.description, + order: page.order, + kind: "generated" as const, + contents: page.contents, + })), + ...bundle.authored.map((page) => ({ + slug: page.slug, + title: page.title, + description: page.description, + order: page.order, + kind: "authored" as const, + contents: page.contents, + })), +]; + +/** + * Returns the head bundle with the diff applied: changed pages carry block + * markers, tombstones stand in for removed pages, and the manifest records + * every page's status for a host to build navigation from. + */ +export const applyBundleDiff = ( + head: BuiltBundle, + base: BuiltBundle, + info: { baseRef: string; baseSha: string }, +): BuiltBundle => { + const { statuses, annotated, tombstones } = diffBundlePages({ + base: { pages: toDiffPages(base), layers: base.model.layers }, + head: { pages: toDiffPages(head), layers: head.model.layers }, + baseRef: info.baseRef, + }); + + const generated = [ + ...head.generated.map((page) => { + const contents = annotated.get(page.slug); + return contents === undefined ? page : { ...page, contents }; + }), + ...tombstones.map((page) => ({ + path: `pages/${page.slug}.mdx`, + slug: page.slug, + title: page.title, + description: page.description, + order: page.order, + contents: page.contents, + })), + ]; + + const authored = head.authored.map((page) => { + const contents = annotated.get(page.slug); + return contents === undefined ? page : { ...page, contents }; + }); + + return { + ...head, + generated, + authored, + manifest: buildManifest({ + generator: head.manifest.generator, + generated, + authored, + diff: { baseRef: info.baseRef, baseSha: info.baseSha, pages: statuses }, + }), + }; +}; diff --git a/libs/@local/petrinaut-arch-docs/src/diff/normalize.test.ts b/libs/@local/petrinaut-arch-docs/src/diff/normalize.test.ts new file mode 100644 index 00000000000..fe9ffffbd43 --- /dev/null +++ b/libs/@local/petrinaut-arch-docs/src/diff/normalize.test.ts @@ -0,0 +1,76 @@ +import { describe, expect, it } from "vitest"; + +import { normalizeGeneratedBlock } from "./normalize"; + +/** + * Every mask exists to keep a *transitive* change from flagging a page, so + * each case pins one source of drift: counts that move when neighbouring code + * moves, orders that shift when a layer is inserted, and the incoming-edge + * list that changes when some other layer adds an import. + */ +describe("normalizeGeneratedBlock", () => { + it("masks the sidebar order", () => { + expect( + normalizeGeneratedBlock('---\ntitle: "Core"\nsidebar_order: 1042\n---\n'), + ).toBe('---\ntitle: "Core"\nsidebar_order: 0\n---\n'); + }); + + it("masks file and line counts in facts props", () => { + expect(normalizeGeneratedBlock(" files={13}\n lines={2200}")).toBe( + " files={0}\n lines={0}", + ); + }); + + it("masks import counts in relation entries", () => { + const entry = ' {"id":"core.hir","imports":41,"crossesPackage":false},'; + expect(normalizeGeneratedBlock(entry)).toBe( + ' {"id":"core.hir","imports":0,"crossesPackage":false},', + ); + }); + + it("collapses the depended-on-by list to the empty form", () => { + const block = [ + "<LayerRelations", + " dependsOn={[", + ' {"id":"core.hir","imports":2},', + " ]}", + " dependedOnBy={[", + ' {"id":"ui.panels","imports":7},', + ' {"id":"ui.canvas","imports":3},', + " ]}", + "/>", + ].join("\n"); + + expect(normalizeGeneratedBlock(block)).toBe( + [ + "<LayerRelations", + " dependsOn={[", + ' {"id":"core.hir","imports":0},', + " ]}", + " dependedOnBy={[]}", + "/>", + ].join("\n"), + ); + }); + + it("keeps the depends-on entries themselves", () => { + const block = [ + " dependsOn={[", + ' {"id":"core.hir","imports":2},', + " ]}", + ].join("\n"); + + expect(normalizeGeneratedBlock(block)).toContain('"id":"core.hir"'); + }); + + it("masks the file-count column of the overview table", () => { + expect( + normalizeGeneratedBlock("| [Core](core) | Headless engine | 214 |"), + ).toBe("| [Core](core) | Headless engine | 0 |"); + }); + + it("leaves ordinary prose and numbers alone", () => { + const prose = "The buffer holds 64 bytes per token."; + expect(normalizeGeneratedBlock(prose)).toBe(prose); + }); +}); diff --git a/libs/@local/petrinaut-arch-docs/src/diff/normalize.ts b/libs/@local/petrinaut-arch-docs/src/diff/normalize.ts new file mode 100644 index 00000000000..46332567f3a --- /dev/null +++ b/libs/@local/petrinaut-arch-docs/src/diff/normalize.ts @@ -0,0 +1,47 @@ +/** + * Masks the derived noise in generated pages before diffing. + * + * A generated page embeds facts that shift whenever *neighbouring* code moves: + * import counts, file and line totals, the sidebar position, the list of layers + * that depend on this one. Diffing raw content would flag every neighbour of a + * real change — precisely the transitive noise a reviewer opens the diff to + * avoid. Everything masked here still renders with its current value; it just + * no longer counts as a difference. + * + * Normalization runs on blocks that were already split from the raw page, so + * statuses computed on normalized blocks map back to the raw ones by index. + */ + +/** + * What is deliberately ignored, and why: + * + * - `sidebar_order` — index-based, so an inserted layer shifts every page + * after it. + * - `files={N}` / `lines={N}` — code volume, not architecture. A layer whose + * file *membership* changed is flagged separately from the model. + * - `"imports":N` in relation entries — edge weight. The edge appearing or + * disappearing still counts; its count drifting does not. + * - the `dependedOnBy` list — an incoming edge is the importing layer's + * change, and it is flagged there, on the `dependsOn` side. + * - the file-count column of the overview's layer table — same reason as + * `files={N}`. + */ +const masks: [RegExp, string][] = [ + [/^sidebar_order: \d+$/gmu, "sidebar_order: 0"], + [/\b(files|lines)=\{\d+\}/gu, "$1={0}"], + [/"imports":\d+/gu, '"imports":0'], + // Collapsed to the same form the emitter uses for an empty list, so a layer + // gaining its first dependent (or losing its last) is masked like any other. + [/^ {2}dependedOnBy=\{\[\n(?: {4}.*\n)+? {2}\]\}$/gmu, " dependedOnBy={[]}"], + [/^(\| \[.*) \| \d+ \|$/gmu, "$1 | 0 |"], +]; + +/** + * The normalized form of one generated block (or frontmatter). Authored pages + * are compared verbatim — they carry no derived facts to mask. + */ +export const normalizeGeneratedBlock = (block: string): string => + masks.reduce( + (text, [pattern, replacement]) => text.replace(pattern, replacement), + block, + ); diff --git a/libs/@local/petrinaut-arch-docs/src/emit/bundle-outputs.ts b/libs/@local/petrinaut-arch-docs/src/emit/bundle-outputs.ts index 4a50bcf9591..c2212ad0b69 100644 --- a/libs/@local/petrinaut-arch-docs/src/emit/bundle-outputs.ts +++ b/libs/@local/petrinaut-arch-docs/src/emit/bundle-outputs.ts @@ -29,6 +29,19 @@ export interface ManifestPage { order: number; } +/** + * How this bundle differs from the one a base ref would produce. Present only + * on diff builds; a host without diff styling can ignore it entirely. + */ +export interface ManifestDiff { + /** The ref the build compared against, e.g. `main`. */ + baseRef: string; + /** The commit that ref resolved to at build time. */ + baseSha: string; + /** Page slug → change. Unchanged pages are absent. */ + pages: Record<string, "added" | "changed" | "removed">; +} + export interface BundleManifest { manifestVersion: number; modelVersion: number; @@ -36,12 +49,14 @@ export interface BundleManifest { /** Relative path to the machine-readable model. */ model: string; pages: ManifestPage[]; + diff?: ManifestDiff; } export const buildManifest = (options: { generator: string; generated: GeneratedPage[]; authored: AuthoredPage[]; + diff?: ManifestDiff; }): BundleManifest => { const pages: ManifestPage[] = [ ...options.generated.map((page) => ({ @@ -71,6 +86,7 @@ export const buildManifest = (options: { generator: options.generator, model: "architecture.json", pages, + ...(options.diff === undefined ? {} : { diff: options.diff }), }; }; diff --git a/libs/@local/petrinaut-arch-docs/src/emit/components/diff-marker.css b/libs/@local/petrinaut-arch-docs/src/emit/components/diff-marker.css new file mode 100644 index 00000000000..ee98e2f7181 --- /dev/null +++ b/libs/@local/petrinaut-arch-docs/src/emit/components/diff-marker.css @@ -0,0 +1,58 @@ +/* + * Styling for diff markers on pages built against a base ref. + * + * Self-contained plain CSS like the layer cards: the three status hues are + * fixed because green/blue/red must stay green/blue/red on any theme, while + * everything structural derives from `currentColor`. The marker itself is + * invisible; the adjacent-sibling selector paints the block it precedes, so + * the marked markdown needs no wrapper of its own. + */ + +.arch-diff-marker { + display: none; + + --arch-diff-added: #2f9e44; + --arch-diff-changed: #3676b8; +} + +.arch-diff-marker[data-status="added"] + * { + border-inline-start: 3px solid var(--arch-diff-added, #2f9e44); + padding-block: 0.25rem; + padding-inline-start: 0.75rem; + background: color-mix(in srgb, #2f9e44 7%, transparent); +} + +.arch-diff-marker[data-status="changed"] + * { + border-inline-start: 3px solid var(--arch-diff-changed, #3676b8); + padding-block: 0.25rem; + padding-inline-start: 0.75rem; + background: color-mix(in srgb, #3676b8 7%, transparent); +} + +.arch-diff-removed { + --arch-diff-removed: #c92a2a; + + margin: 1rem 0; + padding: 0.25rem 0.75rem; + border-inline-start: 3px solid var(--arch-diff-removed); + background: color-mix(in srgb, #c92a2a 7%, transparent); + font-size: 0.85rem; +} + +.arch-diff-removed > summary { + cursor: pointer; + color: var(--arch-diff-removed); + font-weight: 600; +} + +/* Two classes outrank Starlight's sibling-margin rule, as in the cards. */ +.arch-diff-removed .arch-diff-removed-source { + margin: 0.5rem 0 0.25rem; + padding: 0.5rem 0.75rem; + border: 1px solid color-mix(in srgb, currentColor 15%, transparent); + border-radius: 0.4rem; + overflow-x: auto; + font-size: 0.75rem; + line-height: 1.5; + white-space: pre; +} diff --git a/libs/@local/petrinaut-arch-docs/src/emit/components/diff-marker.tsx b/libs/@local/petrinaut-arch-docs/src/emit/components/diff-marker.tsx new file mode 100644 index 00000000000..7abb325f5aa --- /dev/null +++ b/libs/@local/petrinaut-arch-docs/src/emit/components/diff-marker.tsx @@ -0,0 +1,27 @@ +/** + * Marks a block that differs from the base build, on pages emitted by a diff + * build. For `added` and `changed` the marker renders nothing itself — the + * stylesheet decorates the element that follows it, so the marked markdown is + * never nested inside JSX. A `removed` marker has no following block to + * decorate, so it is the visible element: the removed source, collapsed. + */ + +import "./diff-marker.css"; + +export interface DiffMarkerProps { + status: "added" | "changed" | "removed"; + /** Source of the removed content, shown collapsed on `removed` markers. */ + content?: string; +} + +export const DiffMarker = ({ status, content }: DiffMarkerProps) => + status === "removed" ? ( + <details className="arch-diff-removed"> + <summary>Removed content</summary> + {content === undefined ? null : ( + <pre className="arch-diff-removed-source">{content}</pre> + )} + </details> + ) : ( + <span className="arch-diff-marker" data-status={status} /> + ); diff --git a/libs/@local/petrinaut-arch-docs/src/emit/mdx.ts b/libs/@local/petrinaut-arch-docs/src/emit/mdx.ts index 46f8e552569..053e8d58ea3 100644 --- a/libs/@local/petrinaut-arch-docs/src/emit/mdx.ts +++ b/libs/@local/petrinaut-arch-docs/src/emit/mdx.ts @@ -60,7 +60,9 @@ const escapeTableCell = (text: string): string => text.replace(/\\/gu, "\\\\").replace(/\|/gu, "\\|").replace(/\n/gu, " "); /** Serialises frontmatter by hand so the output stays byte-stable. */ -const frontmatter = (fields: Record<string, string | number>): string => { +export const frontmatter = ( + fields: Record<string, string | number>, +): string => { const lines = Object.entries(fields).map( ([key, value]) => `${key}: ${typeof value === "number" ? value : JSON.stringify(value)}`, @@ -135,7 +137,7 @@ const relativeTo = (fromSlug: string, toSlug: string): string => { * slug-relative form here produced `diagrams/x.svg` from `pages/architecture.mdx`, * which points at a `pages/diagrams/` directory that does not exist. */ -const assetPathFrom = (slug: string, assetPath: string): string => +export const assetPathFrom = (slug: string, assetPath: string): string => posix.relative(posix.dirname(`pages/${slug}`), assetPath); /** diff --git a/libs/@local/petrinaut-arch-docs/src/emit/shipped-components.ts b/libs/@local/petrinaut-arch-docs/src/emit/shipped-components.ts index 2775c7600f6..7651daf6c16 100644 --- a/libs/@local/petrinaut-arch-docs/src/emit/shipped-components.ts +++ b/libs/@local/petrinaut-arch-docs/src/emit/shipped-components.ts @@ -24,11 +24,15 @@ export const LAYER_SOURCE_MODULE = "layer-source"; /** Module name generated pages import the links card from. */ export const LAYER_LINKS_MODULE = "layer-links"; +/** Module name diff-annotated pages import the block marker from. */ +export const DIFF_MARKER_MODULE = "diff-marker"; + const shippedComponentFiles = [ `${LAYER_FACTS_MODULE}.tsx`, `${LAYER_RELATIONS_MODULE}.tsx`, `${LAYER_SOURCE_MODULE}.tsx`, `${LAYER_LINKS_MODULE}.tsx`, + `${DIFF_MARKER_MODULE}.tsx`, // Styling the cards share, imported by the components above. // // Resolution via `import.meta.url` works because this package always runs @@ -36,6 +40,7 @@ const shippedComponentFiles = [ // from compilation, so no build ever places these files next to a compiled // output. If the package ever ships compiled, copy them into the output. "layer-cards.css", + `${DIFF_MARKER_MODULE}.css`, ]; export const readShippedComponents = async (): Promise<AuthoredComponent[]> => diff --git a/libs/@local/petrinaut-arch-docs/turbo.json b/libs/@local/petrinaut-arch-docs/turbo.json index 786118fdb00..d9f89c99c63 100644 --- a/libs/@local/petrinaut-arch-docs/turbo.json +++ b/libs/@local/petrinaut-arch-docs/turbo.json @@ -17,8 +17,12 @@ // env mode hides anything unlisted, which would leave a preview build // emitting `main` links. Mirrors REF_VARIABLES in `src/source-url.ts`; // keep the two lists identical. + // + // PETRINAUT_ARCH_DOCS_DIFF_BASE is separate: when set, the build + // compares against that git ref and annotates what changed. "env": [ "PETRINAUT_ARCH_DOCS_SOURCE_REF", + "PETRINAUT_ARCH_DOCS_DIFF_BASE", "VERCEL_GIT_COMMIT_SHA", "GITHUB_SHA", "VERCEL_GIT_COMMIT_REF", From 52d0779b71078f9ca3228b4dd285e72093326ee2 Mon Sep 17 00:00:00 2001 From: Chris Feijoo <chris@kube.io> Date: Thu, 27 Aug 2026 03:47:39 +0200 Subject: [PATCH 02/11] FE-1514: Apply review findings to the diff mode Sort masked depends-on entries so count-driven reordering cannot flag a page; scope every mask to the block shape it targets, keeping embedded README prose untouched. Clean up the scratch tree on any extraction failure and only swallow git-archive pathspec misses; refuse refs with a leading dash. Warn on an empty --diff-base, drop dead custom properties, and mark sidebar links with a data attribute instead of a second class. --- apps/petrinaut-docs/astro.config.mjs | 5 +- apps/petrinaut-docs/src/styles/chrome.css | 9 +- libs/@local/petrinaut-arch-docs/README.md | 5 +- libs/@local/petrinaut-arch-docs/src/cli.ts | 13 ++- .../petrinaut-arch-docs/src/diff/base-tree.ts | 77 +++++++++++------ .../src/diff/bundle-diff.test.ts | 25 ++++++ .../src/diff/bundle-diff.ts | 5 +- .../src/diff/normalize.test.ts | 85 +++++++++++++------ .../petrinaut-arch-docs/src/diff/normalize.ts | 77 ++++++++++++----- .../src/emit/components/diff-marker.css | 5 +- 10 files changed, 220 insertions(+), 86 deletions(-) diff --git a/apps/petrinaut-docs/astro.config.mjs b/apps/petrinaut-docs/astro.config.mjs index 499d387de4a..840b429ce93 100644 --- a/apps/petrinaut-docs/astro.config.mjs +++ b/apps/petrinaut-docs/astro.config.mjs @@ -57,7 +57,10 @@ const diffLinkProps = (slug) => { ? {} : { badge: DIFF_BADGES[status], - attrs: { class: `pnd-diff-${status}` }, + // A data attribute rather than `class`: Starlight already merges + // `attrs.class` into the link's computed class and then re-spreads + // `attrs`, which would emit a second `class` attribute. + attrs: { "data-pnd-diff": status }, }; }; diff --git a/apps/petrinaut-docs/src/styles/chrome.css b/apps/petrinaut-docs/src/styles/chrome.css index b9233f65a7c..cfad20c0a19 100644 --- a/apps/petrinaut-docs/src/styles/chrome.css +++ b/apps/petrinaut-docs/src/styles/chrome.css @@ -215,11 +215,12 @@ /* * On a diff build (a preview built against a base ref) the sidebar carries - * change badges from `manifest.diff`; see astro.config.mjs. The classes come - * in through the sidebar links' `attrs`. Removed pages link to tombstone - * stubs, and the strike-through is what says "gone" before the reader clicks. + * change badges from `manifest.diff`; see astro.config.mjs. Each link's + * status comes in through its `attrs` as a data attribute. Removed pages + * link to tombstone stubs, and the strike-through is what says "gone" before + * the reader clicks. */ -.pnd-diff-removed { +.sidebar-content [data-pnd-diff="removed"] { text-decoration: line-through; opacity: 0.75; } diff --git a/libs/@local/petrinaut-arch-docs/README.md b/libs/@local/petrinaut-arch-docs/README.md index bd73a29d7b9..323bab887c9 100644 --- a/libs/@local/petrinaut-arch-docs/README.md +++ b/libs/@local/petrinaut-arch-docs/README.md @@ -206,8 +206,9 @@ What it produces: **What counts as a change** is deliberately narrower than a byte diff, because a generated page embeds facts that shift whenever _neighbouring_ code moves. -Masked before comparison: import counts, file and line totals, sidebar -positions, and the whole "depended on by" list — an incoming edge is the +Masked before comparison: import counts and the count-driven ordering of the +depends-on list, file and line totals, sidebar positions, and the whole +"depended on by" list — an incoming edge is the importing layer's change and is flagged there, on its `dependsOn` side. A page is `changed` when anything else differs: its role, prose, name, declaring file, outgoing edges, sub-layers, attached guides — or when the layer's file diff --git a/libs/@local/petrinaut-arch-docs/src/cli.ts b/libs/@local/petrinaut-arch-docs/src/cli.ts index a565c58f615..8b9beb0b68b 100644 --- a/libs/@local/petrinaut-arch-docs/src/cli.ts +++ b/libs/@local/petrinaut-arch-docs/src/cli.ts @@ -81,7 +81,15 @@ const resolveDiffBase = (args: string[]): string | null => { const value = flag.includes("=") ? flag.slice(flag.indexOf("=") + 1) : (args[flagIndex + 1] ?? ""); - return value.trim() === "" ? null : value.trim(); + if (value.trim() === "") { + // The flag was typed deliberately, so an empty value gets a message + // rather than a silent plain build. + process.stderr.write( + `${yellow("warning")} --diff-base given without a ref; building without change highlighting\n`, + ); + return null; + } + return value.trim(); } const fromEnvironment = process.env.PETRINAUT_ARCH_DOCS_DIFF_BASE?.trim(); @@ -152,7 +160,8 @@ const withDiffAgainst = async ( ); return bundle; } finally { - await baseTree?.dispose(); + // A failed cleanup must not reject out of the build the diff decorates. + await baseTree?.dispose().catch(() => {}); } }; diff --git a/libs/@local/petrinaut-arch-docs/src/diff/base-tree.ts b/libs/@local/petrinaut-arch-docs/src/diff/base-tree.ts index 83c87d5007d..c076af50ea6 100644 --- a/libs/@local/petrinaut-arch-docs/src/diff/base-tree.ts +++ b/libs/@local/petrinaut-arch-docs/src/diff/base-tree.ts @@ -9,7 +9,8 @@ * * Everything here can fail in a legitimate environment — a shallow CI clone * without the base ref, no network to fetch it — so callers treat a throw as - * "build without the diff", never as a build failure. + * "build without the diff", never as a build failure. A throw always cleans + * the scratch directory up behind itself. */ import { execFileSync } from "node:child_process"; @@ -63,46 +64,72 @@ const resolveCommit = (repoRoot: string, ref: string): string => { ]); }; +/** + * Whether a `git archive` failure means the pathspec matched nothing at the + * ref — the one failure that is a normal state (a package added since the + * base) rather than a defect. Anything else must propagate: swallowing it + * would drop a whole package from the base tree and render every layer in it + * as `added`, a confidently wrong diff instead of a degraded one. + */ +const isPathspecMismatch = (cause: unknown): boolean => + cause instanceof Error && + /did not match any files/u.test( + String((cause as { stderr?: unknown }).stderr ?? ""), + ); + export const materializeBaseTree = async (options: { repoRoot: string; ref: string; /** Repo-relative directories to extract; ones absent at the ref are skipped. */ paths: string[]; }): Promise<BaseTree> => { + // Refused rather than escaped: git would parse a leading dash as an option, + // in `rev-parse`, `fetch` and `archive` alike. + if (options.ref.startsWith("-")) { + throw new Error(`\`${options.ref}\` is not a usable ref`); + } + const sha = resolveCommit(options.repoRoot, options.ref); // Canonicalised because macOS puts the temp directory behind a symlink // (`/var` → `/private/var`): dependency-cruiser realpaths what it resolves, // and against a symlinked root every resolved file appears to escape the // tree, which silently drops every TypeScript edge from the base model. const root = await realpath(await mkdtemp(join(tmpdir(), "arch-docs-base-"))); - const archive = join(root, ".base.tar"); - let extracted = 0; - for (const path of options.paths) { - try { - // Written to a file and extracted in a second step: a pipe would need a - // shell with `pipefail` for `git archive`'s failure to be visible at all. - execFileSync( - "git", - ["archive", "--format=tar", "-o", archive, sha, "--", path], - { cwd: options.repoRoot, stdio: ["ignore", "ignore", "pipe"] }, - ); - } catch { - // The path does not exist at the ref, e.g. a package added since. - continue; + try { + const archive = join(root, ".base.tar"); + let extracted = 0; + + for (const path of options.paths) { + try { + // Written to a file and extracted in a second step: a pipe would need + // a shell with `pipefail` for `git archive`'s failure to be visible. + execFileSync( + "git", + ["archive", "--format=tar", "-o", archive, sha, "--", path], + { cwd: options.repoRoot, stdio: ["ignore", "ignore", "pipe"] }, + ); + } catch (cause) { + if (isPathspecMismatch(cause)) { + continue; + } + throw cause; + } + execFileSync("tar", ["-xf", archive, "-C", root], { + stdio: ["ignore", "ignore", "pipe"], + }); + extracted += 1; } - execFileSync("tar", ["-xf", archive, "-C", root], { - stdio: ["ignore", "ignore", "pipe"], - }); - extracted += 1; - } - await rm(archive, { force: true }); + await rm(archive, { force: true }); - if (extracted === 0) { + if (extracted === 0) { + throw new Error( + `none of the covered directories exist at \`${options.ref}\``, + ); + } + } catch (cause) { await rm(root, { recursive: true, force: true }); - throw new Error( - `none of the covered directories exist at \`${options.ref}\``, - ); + throw cause; } return { diff --git a/libs/@local/petrinaut-arch-docs/src/diff/bundle-diff.test.ts b/libs/@local/petrinaut-arch-docs/src/diff/bundle-diff.test.ts index 18b152e26a8..164d79454ba 100644 --- a/libs/@local/petrinaut-arch-docs/src/diff/bundle-diff.test.ts +++ b/libs/@local/petrinaut-arch-docs/src/diff/bundle-diff.test.ts @@ -136,6 +136,31 @@ describe("diffBundlePages", () => { expect(result.statuses).toEqual({}); }); + it("ignores dependencies reordering when only their counts moved", () => { + const base = generatedPage({ + slug: "architecture/core", + dependsOn: [ + { id: "core.hir", imports: 9 }, + { id: "core.types", imports: 2 }, + ], + }); + const head = generatedPage({ + slug: "architecture/core", + dependsOn: [ + { id: "core.types", imports: 8 }, + { id: "core.hir", imports: 3 }, + ], + }); + + const result = diffBundlePages({ + base: side([base]), + head: side([head]), + baseRef: "main", + }); + + expect(result.statuses).toEqual({}); + }); + it("flags a role change and marks the changed block", () => { const result = diffBundlePages({ base: side([generatedPage({ slug: "architecture/core" })]), diff --git a/libs/@local/petrinaut-arch-docs/src/diff/bundle-diff.ts b/libs/@local/petrinaut-arch-docs/src/diff/bundle-diff.ts index 59dd142e348..cff0ecf063c 100644 --- a/libs/@local/petrinaut-arch-docs/src/diff/bundle-diff.ts +++ b/libs/@local/petrinaut-arch-docs/src/diff/bundle-diff.ts @@ -135,10 +135,11 @@ export const diffBundlePages = (options: { const head = splitFrontmatter(page.contents); const base = splitFrontmatter(basePage.contents); + const headBlocks = splitBlocks(head.body); const diff = diffBlocks({ baseBlocks: splitBlocks(base.body), - headBlocks: splitBlocks(head.body), + headBlocks, normalize, }); @@ -160,7 +161,7 @@ export const diffBundlePages = (options: { annotatePageBlocks({ slug: page.slug, frontmatter: head.frontmatter, - blocks: splitBlocks(head.body), + blocks: headBlocks, diff, }), ); diff --git a/libs/@local/petrinaut-arch-docs/src/diff/normalize.test.ts b/libs/@local/petrinaut-arch-docs/src/diff/normalize.test.ts index fe9ffffbd43..42dceeb7067 100644 --- a/libs/@local/petrinaut-arch-docs/src/diff/normalize.test.ts +++ b/libs/@local/petrinaut-arch-docs/src/diff/normalize.test.ts @@ -5,27 +5,59 @@ import { normalizeGeneratedBlock } from "./normalize"; /** * Every mask exists to keep a *transitive* change from flagging a page, so * each case pins one source of drift: counts that move when neighbouring code - * moves, orders that shift when a layer is inserted, and the incoming-edge - * list that changes when some other layer adds an import. + * moves, orders that shift with those counts, and the incoming-edge list that + * changes when some other layer adds an import. The masks are scoped to the + * block shapes that carry the derived facts, so the prose cases pin that an + * embedded README is never rewritten. */ describe("normalizeGeneratedBlock", () => { - it("masks the sidebar order", () => { + it("masks the sidebar order in frontmatter", () => { expect( normalizeGeneratedBlock('---\ntitle: "Core"\nsidebar_order: 1042\n---\n'), ).toBe('---\ntitle: "Core"\nsidebar_order: 0\n---\n'); }); - it("masks file and line counts in facts props", () => { - expect(normalizeGeneratedBlock(" files={13}\n lines={2200}")).toBe( - " files={0}\n lines={0}", - ); + it("masks file and line counts in the facts card", () => { + expect( + normalizeGeneratedBlock("<LayerFacts\n files={13}\n lines={2200}\n/>"), + ).toBe("<LayerFacts\n files={0}\n lines={0}\n/>"); }); it("masks import counts in relation entries", () => { - const entry = ' {"id":"core.hir","imports":41,"crossesPackage":false},'; - expect(normalizeGeneratedBlock(entry)).toBe( - ' {"id":"core.hir","imports":0,"crossesPackage":false},', - ); + const block = [ + "<LayerRelations", + " dependsOn={[", + ' {"id":"core.hir","imports":41},', + " ]}", + " dependedOnBy={[]}", + "/>", + ].join("\n"); + + expect(normalizeGeneratedBlock(block)).toContain('"imports":0'); + expect(normalizeGeneratedBlock(block)).toContain('"id":"core.hir"'); + }); + + it("sorts depends-on entries, so count-driven reordering masks out", () => { + const entries = (rows: string[]): string => + [ + "<LayerRelations", + " dependsOn={[", + ...rows.map((row) => ` ${row}`), + " ]}", + " dependedOnBy={[]}", + "/>", + ].join("\n"); + + const base = entries([ + '{"id":"a","imports":9},', + '{"id":"b","imports":2},', + ]); + const head = entries([ + '{"id":"b","imports":8},', + '{"id":"a","imports":3},', + ]); + + expect(normalizeGeneratedBlock(base)).toBe(normalizeGeneratedBlock(head)); }); it("collapses the depended-on-by list to the empty form", () => { @@ -53,24 +85,25 @@ describe("normalizeGeneratedBlock", () => { ); }); - it("keeps the depends-on entries themselves", () => { - const block = [ - " dependsOn={[", - ' {"id":"core.hir","imports":2},', - " ]}", + it("masks the file-count column of the overview table", () => { + const table = [ + "| Layer | Responsibility | Files |", + "| --- | --- | --- |", + "| [Core](core) | Headless engine | 214 |", ].join("\n"); - expect(normalizeGeneratedBlock(block)).toContain('"id":"core.hir"'); - }); - - it("masks the file-count column of the overview table", () => { - expect( - normalizeGeneratedBlock("| [Core](core) | Headless engine | 214 |"), - ).toBe("| [Core](core) | Headless engine | 0 |"); + expect(normalizeGeneratedBlock(table)).toContain( + "| [Core](core) | Headless engine | 0 |", + ); }); - it("leaves ordinary prose and numbers alone", () => { - const prose = "The buffer holds 64 bytes per token."; - expect(normalizeGeneratedBlock(prose)).toBe(prose); + it("leaves prose alone, even when it resembles a masked shape", () => { + for (const prose of [ + "The buffer holds 64 bytes per token.", + "Set `files={12}` on the card to override the count.", + "| [A guide](link) | prose table | 7 |", + ]) { + expect(normalizeGeneratedBlock(prose)).toBe(prose); + } }); }); diff --git a/libs/@local/petrinaut-arch-docs/src/diff/normalize.ts b/libs/@local/petrinaut-arch-docs/src/diff/normalize.ts index 46332567f3a..ad233346eaf 100644 --- a/libs/@local/petrinaut-arch-docs/src/diff/normalize.ts +++ b/libs/@local/petrinaut-arch-docs/src/diff/normalize.ts @@ -8,40 +8,75 @@ * avoid. Everything masked here still renders with its current value; it just * no longer counts as a difference. * + * Each mask is scoped to the block shape it targets, matched by the block's + * first line, so authored prose embedded in a page (a declaring README's + * `## Notes` body) is never rewritten even when it happens to contain + * `files={12}` or a table row ending in a number. + * * Normalization runs on blocks that were already split from the raw page, so * statuses computed on normalized blocks map back to the raw ones by index. */ /** + * Entry lines sorted after their counts are masked: the emitter orders + * `dependsOn` by import count, so two dependencies swapping rank — with no + * edge added or removed — would otherwise reorder the lines and defeat the + * count mask. + */ +const sortRelationEntries = (block: string): string => + block.replace( + /^( {2}dependsOn=\{\[\n)((?: {4}.*\n)+?)( {2}\]\})$/gmu, + (_, open: string, entries: string, close: string) => + `${open}${entries + .split("\n") + .filter((line) => line !== "") + .sort() + .join("\n")}\n${close}`, + ); + +/** + * The normalized form of one generated block (or frontmatter). Authored pages + * are compared verbatim — they carry no derived facts to mask. + * * What is deliberately ignored, and why: * * - `sidebar_order` — index-based, so an inserted layer shifts every page * after it. * - `files={N}` / `lines={N}` — code volume, not architecture. A layer whose * file *membership* changed is flagged separately from the model. - * - `"imports":N` in relation entries — edge weight. The edge appearing or - * disappearing still counts; its count drifting does not. + * - `"imports":N` in relation entries, and the count-driven order of the + * `dependsOn` list — edge weight. The edge appearing or disappearing still + * counts; its count drifting does not. * - the `dependedOnBy` list — an incoming edge is the importing layer's - * change, and it is flagged there, on the `dependsOn` side. + * change, and it is flagged there, on the `dependsOn` side. Collapsed to + * the same form the emitter uses for an empty list, so a layer gaining its + * first dependent (or losing its last) is masked like any other. * - the file-count column of the overview's layer table — same reason as * `files={N}`. */ -const masks: [RegExp, string][] = [ - [/^sidebar_order: \d+$/gmu, "sidebar_order: 0"], - [/\b(files|lines)=\{\d+\}/gu, "$1={0}"], - [/"imports":\d+/gu, '"imports":0'], - // Collapsed to the same form the emitter uses for an empty list, so a layer - // gaining its first dependent (or losing its last) is masked like any other. - [/^ {2}dependedOnBy=\{\[\n(?: {4}.*\n)+? {2}\]\}$/gmu, " dependedOnBy={[]}"], - [/^(\| \[.*) \| \d+ \|$/gmu, "$1 | 0 |"], -]; +export const normalizeGeneratedBlock = (block: string): string => { + if (block.startsWith("---\n")) { + return block.replace(/^sidebar_order: \d+$/gmu, "sidebar_order: 0"); + } -/** - * The normalized form of one generated block (or frontmatter). Authored pages - * are compared verbatim — they carry no derived facts to mask. - */ -export const normalizeGeneratedBlock = (block: string): string => - masks.reduce( - (text, [pattern, replacement]) => text.replace(pattern, replacement), - block, - ); + if (block.startsWith("<LayerFacts") || block.startsWith("<LayerSource")) { + return block.replace(/\b(files|lines)=\{\d+\}/gu, "$1={0}"); + } + + if (block.startsWith("<LayerRelations")) { + return sortRelationEntries( + block + .replace(/"imports":\d+/gu, '"imports":0') + .replace( + /^ {2}dependedOnBy=\{\[\n(?: {4}.*\n)+? {2}\]\}$/gmu, + " dependedOnBy={[]}", + ), + ); + } + + if (block.startsWith("| Layer | Responsibility | Files |")) { + return block.replace(/^(\| \[.*) \| \d+ \|$/gmu, "$1 | 0 |"); + } + + return block; +}; diff --git a/libs/@local/petrinaut-arch-docs/src/emit/components/diff-marker.css b/libs/@local/petrinaut-arch-docs/src/emit/components/diff-marker.css index ee98e2f7181..457cbc70825 100644 --- a/libs/@local/petrinaut-arch-docs/src/emit/components/diff-marker.css +++ b/libs/@local/petrinaut-arch-docs/src/emit/components/diff-marker.css @@ -8,11 +8,10 @@ * the marked markdown needs no wrapper of its own. */ +/* The status hues are read with a fallback so a host may re-theme them by + defining the custom properties on an ancestor (e.g. `:root`). */ .arch-diff-marker { display: none; - - --arch-diff-added: #2f9e44; - --arch-diff-changed: #3676b8; } .arch-diff-marker[data-status="added"] + * { From 871c556c3ffe169447beeb5c2af032fbf9948a55 Mon Sep 17 00:00:00 2001 From: Chris Feijoo <chris@kube.io> Date: Thu, 27 Aug 2026 13:58:19 +0200 Subject: [PATCH 03/11] FE-1514: Fetch the diff base anonymously when the local clone cannot supply it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Vercel build gets a snapshot of the sources with no usable git clone behind it, so resolving the base ref locally fails and every preview degraded to a plain bundle. When local resolution fails, fetch the ref from the public repository into a scratch repo — blobless, with a sparse checkout of only the covered directories — and log which strategy ran. Verified against github.com from a directory with no clone: 3s to materialize, and the base model matches a local build. --- libs/@local/petrinaut-arch-docs/README.md | 14 +- libs/@local/petrinaut-arch-docs/src/cli.ts | 8 + .../petrinaut-arch-docs/src/diff/base-tree.ts | 217 +++++++++++++----- libs/@local/petrinaut-arch-docs/turbo.json | 8 +- 4 files changed, 185 insertions(+), 62 deletions(-) diff --git a/libs/@local/petrinaut-arch-docs/README.md b/libs/@local/petrinaut-arch-docs/README.md index 323bab887c9..4ebbabfa9ad 100644 --- a/libs/@local/petrinaut-arch-docs/README.md +++ b/libs/@local/petrinaut-arch-docs/README.md @@ -216,10 +216,18 @@ membership moved, the one structural change the masked content cannot show. Editing code inside a layer without moving files or edges changes nothing the architecture describes, and flags nothing. +The build extracts the base tree from the local clone when it can. When it +cannot — a Vercel build gets a snapshot of the sources with no usable clone +behind it — it fetches the ref anonymously from the public repository +(`VERCEL_GIT_REPO_OWNER`/`VERCEL_GIT_REPO_SLUG` when set, `hashintel/hash` +otherwise) into a scratch repository: a blobless fetch plus a sparse checkout +of only the covered directories, so the transfer stays small. The build log +says `base tree fetched from …` when this path ran. + The diff decorates the bundle, it never gates it: when the base ref cannot be -resolved (say, a shallow clone with no way to fetch it) the build warns and -writes a plain bundle. Production builds of `main` never set the variable, so -they never diff. +resolved either way (no network, a repository that is not public) the build +warns and writes a plain bundle. Production builds of `main` never set the +variable, so they never diff. ### Embedding the bundle elsewhere diff --git a/libs/@local/petrinaut-arch-docs/src/cli.ts b/libs/@local/petrinaut-arch-docs/src/cli.ts index 8b9beb0b68b..fb7b4b58c1b 100644 --- a/libs/@local/petrinaut-arch-docs/src/cli.ts +++ b/libs/@local/petrinaut-arch-docs/src/cli.ts @@ -128,6 +128,14 @@ const withDiffAgainst = async ( }); baseTree = tree; + // Logged because it names the strategy: a Vercel build has no usable + // clone, and this line is how its logs show the fallback fetch worked. + if (tree.fetchedFrom !== undefined) { + process.stdout.write( + dim(`base tree fetched from ${tree.fetchedFrom}\n`), + ); + } + const baseBundle = await buildBundle({ repoRoot: tree.root, includeDiagrams, diff --git a/libs/@local/petrinaut-arch-docs/src/diff/base-tree.ts b/libs/@local/petrinaut-arch-docs/src/diff/base-tree.ts index c076af50ea6..a703e141b15 100644 --- a/libs/@local/petrinaut-arch-docs/src/diff/base-tree.ts +++ b/libs/@local/petrinaut-arch-docs/src/diff/base-tree.ts @@ -1,19 +1,30 @@ /** * Materializes the covered source trees at a base ref, for a diff build. * - * `git archive` extracts only the directories the generator scans rather than - * checking out the whole monorepo, and the *current* generator then runs over - * the extracted tree — dependency-cruiser resolves workspace imports through - * aliases derived from each package's `exports` map, so the tree needs no - * `node_modules` and no install step. + * Two strategies, tried in order: * - * Everything here can fail in a legitimate environment — a shallow CI clone - * without the base ref, no network to fetch it — so callers treat a throw as - * "build without the diff", never as a build failure. A throw always cleans - * the scratch directory up behind itself. + * 1. The local clone: resolve the ref and `git archive` the covered + * directories out of it. This is what a developer machine and a GitHub + * Actions checkout take. + * 2. An anonymous fetch from the public repository: a Vercel build gets a + * snapshot of the sources with no usable git clone behind it, so the ref + * is fetched into a scratch repository instead — blobless + * (`--filter=blob:none`) with a sparse checkout of only the covered + * directories, which keeps the transfer to the trees plus the blobs the + * generator actually scans. + * + * Either way the extracted tree needs no `node_modules` and no install step: + * dependency-cruiser resolves workspace imports through aliases derived from + * each package's `exports` map. + * + * Everything here can fail in a legitimate environment — no network, a + * repository that is not public — so callers treat a throw as "build without + * the diff", never as a build failure. A throw always cleans the scratch + * directory up behind itself. */ import { execFileSync } from "node:child_process"; +import { existsSync, rmSync } from "node:fs"; import { mkdtemp, realpath, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -24,24 +35,28 @@ export interface BaseTree { ref: string; /** The commit the ref resolved to. */ sha: string; + /** The remote URL the ref was fetched from, when the local clone could not supply it. */ + fetchedFrom?: string; dispose: () => Promise<void>; } -const git = (repoRoot: string, args: string[]): string => +const git = (cwd: string, args: string[]): string => execFileSync("git", args, { - cwd: repoRoot, + cwd, encoding: "utf8", stdio: ["ignore", "pipe", "pipe"], }).trim(); /** - * Resolves a ref to a commit, fetching it when the local clone lacks it. + * Resolves a ref against the local clone, or null when it cannot. * * A CI clone is typically shallow and checked out at the head commit only, so - * the base branch resolves neither bare nor as `origin/<ref>` — the fetch is - * the path most CI builds actually take. + * the base branch resolves neither bare nor as `origin/<ref>`; the fetch + * still succeeds where the clone has a credentialed remote (a developer + * machine, a GitHub Actions checkout). Null covers the rest — most notably a + * Vercel build, whose sources come as a snapshot with no fetchable clone. */ -const resolveCommit = (repoRoot: string, ref: string): string => { +const resolveLocalCommit = (repoRoot: string, ref: string): string | null => { for (const candidate of [ref, `origin/${ref}`]) { try { return git(repoRoot, [ @@ -55,13 +70,17 @@ const resolveCommit = (repoRoot: string, ref: string): string => { } } - git(repoRoot, ["fetch", "--depth=1", "origin", ref]); - return git(repoRoot, [ - "rev-parse", - "--verify", - "--quiet", - "FETCH_HEAD^{commit}", - ]); + try { + git(repoRoot, ["fetch", "--quiet", "--depth=1", "origin", ref]); + return git(repoRoot, [ + "rev-parse", + "--verify", + "--quiet", + "FETCH_HEAD^{commit}", + ]); + } catch { + return null; + } }; /** @@ -77,6 +96,102 @@ const isPathspecMismatch = (cause: unknown): boolean => String((cause as { stderr?: unknown }).stderr ?? ""), ); +/** Extracts the covered directories from the local clone into `root`. */ +const extractFromLocalClone = (options: { + repoRoot: string; + sha: string; + paths: string[]; + root: string; +}): void => { + const archive = join(options.root, ".base.tar"); + let extracted = 0; + + for (const path of options.paths) { + try { + // Written to a file and extracted in a second step: a pipe would need + // a shell with `pipefail` for `git archive`'s failure to be visible. + execFileSync( + "git", + ["archive", "--format=tar", "-o", archive, options.sha, "--", path], + { cwd: options.repoRoot, stdio: ["ignore", "ignore", "pipe"] }, + ); + } catch (cause) { + if (isPathspecMismatch(cause)) { + continue; + } + throw cause; + } + execFileSync("tar", ["-xf", archive, "-C", options.root], { + stdio: ["ignore", "ignore", "pipe"], + }); + extracted += 1; + } + rmSync(archive, { force: true }); + + if (extracted === 0) { + throw new Error("none of the covered directories exist at the base ref"); + } +}; + +/** + * The repository to fetch the base ref from when the local clone cannot + * supply it. Vercel names the repository being built; outside Vercel the + * fallback is the same repository `source-url.ts` already assumes for source + * links. The values are validated because they end up on a git command line. + */ +const remoteRepoUrl = (env: NodeJS.ProcessEnv): string => { + const owner = env.VERCEL_GIT_REPO_OWNER; + const slug = env.VERCEL_GIT_REPO_SLUG; + const wellFormed = /^[\w.-]+$/u; + + return owner !== undefined && + slug !== undefined && + wellFormed.test(owner) && + wellFormed.test(slug) + ? `https://github.com/${owner}/${slug}` + : "https://github.com/hashintel/hash"; +}; + +/** + * Fetches the ref anonymously into a scratch repository at `root` and + * materializes the covered directories with a sparse checkout. Returns the + * commit it resolved to. Anonymous access works because the repository is + * public; on a private one the fetch fails and the caller degrades. + */ +const fetchSparseTree = (options: { + url: string; + ref: string; + paths: string[]; + root: string; +}): string => { + const { root } = options; + + git(root, ["init", "--quiet"]); + git(root, ["remote", "add", "origin", options.url]); + git(root, [ + "fetch", + "--quiet", + "--depth=1", + "--no-tags", + "--filter=blob:none", + "origin", + options.ref, + ]); + const sha = git(root, ["rev-parse", "--verify", "--quiet", "FETCH_HEAD^{commit}"]); + + // Sparse patterns are set before the checkout so only the covered + // directories materialize — the checkout is also what batch-fetches their + // blobs from the promisor remote. + git(root, ["sparse-checkout", "set", ...options.paths]); + git(root, ["-c", "advice.detachedHead=false", "checkout", "--quiet", sha]); + + if (!options.paths.some((path) => existsSync(join(root, path)))) { + throw new Error("none of the covered directories exist at the base ref"); + } + + return sha; +}; + export const materializeBaseTree = async (options: { repoRoot: string; ref: string; @@ -89,7 +204,6 @@ export const materializeBaseTree = async (options: { throw new Error(`\`${options.ref}\` is not a usable ref`); } - const sha = resolveCommit(options.repoRoot, options.ref); // Canonicalised because macOS puts the temp directory behind a symlink // (`/var` → `/private/var`): dependency-cruiser realpaths what it resolves, // and against a symlinked root every resolved file appears to escape the @@ -97,45 +211,34 @@ export const materializeBaseTree = async (options: { const root = await realpath(await mkdtemp(join(tmpdir(), "arch-docs-base-"))); try { - const archive = join(root, ".base.tar"); - let extracted = 0; - - for (const path of options.paths) { - try { - // Written to a file and extracted in a second step: a pipe would need - // a shell with `pipefail` for `git archive`'s failure to be visible. - execFileSync( - "git", - ["archive", "--format=tar", "-o", archive, sha, "--", path], - { cwd: options.repoRoot, stdio: ["ignore", "ignore", "pipe"] }, - ); - } catch (cause) { - if (isPathspecMismatch(cause)) { - continue; - } - throw cause; - } - execFileSync("tar", ["-xf", archive, "-C", root], { - stdio: ["ignore", "ignore", "pipe"], + const localSha = resolveLocalCommit(options.repoRoot, options.ref); + + if (localSha !== null) { + extractFromLocalClone({ + repoRoot: options.repoRoot, + sha: localSha, + paths: options.paths, + root, }); - extracted += 1; + return { + root, + ref: options.ref, + sha: localSha, + dispose: () => rm(root, { recursive: true, force: true }), + }; } - await rm(archive, { force: true }); - if (extracted === 0) { - throw new Error( - `none of the covered directories exist at \`${options.ref}\``, - ); - } + const url = remoteRepoUrl(process.env); + const sha = fetchSparseTree({ url, ref: options.ref, paths: options.paths, root }); + return { + root, + ref: options.ref, + sha, + fetchedFrom: url, + dispose: () => rm(root, { recursive: true, force: true }), + }; } catch (cause) { await rm(root, { recursive: true, force: true }); throw cause; } - - return { - root, - ref: options.ref, - sha, - dispose: () => rm(root, { recursive: true, force: true }), - }; }; diff --git a/libs/@local/petrinaut-arch-docs/turbo.json b/libs/@local/petrinaut-arch-docs/turbo.json index d9f89c99c63..4ee54f83906 100644 --- a/libs/@local/petrinaut-arch-docs/turbo.json +++ b/libs/@local/petrinaut-arch-docs/turbo.json @@ -19,7 +19,9 @@ // keep the two lists identical. // // PETRINAUT_ARCH_DOCS_DIFF_BASE is separate: when set, the build - // compares against that git ref and annotates what changed. + // compares against that git ref and annotates what changed. The + // VERCEL_GIT_REPO_* pair names the repository the base ref is fetched + // from when the local clone cannot supply it. "env": [ "PETRINAUT_ARCH_DOCS_SOURCE_REF", "PETRINAUT_ARCH_DOCS_DIFF_BASE", @@ -27,7 +29,9 @@ "GITHUB_SHA", "VERCEL_GIT_COMMIT_REF", "GITHUB_HEAD_REF", - "GITHUB_REF_NAME" + "GITHUB_REF_NAME", + "VERCEL_GIT_REPO_OWNER", + "VERCEL_GIT_REPO_SLUG" ] }, "lint:arch-docs": { From 1573021e057f49c6b80792c801c02ce72a59a273 Mon Sep 17 00:00:00 2001 From: Chris Feijoo <chris@kube.io> Date: Thu, 27 Aug 2026 14:18:36 +0200 Subject: [PATCH 04/11] FE-1514: Apply repo formatting --- libs/@local/petrinaut-arch-docs/src/cli.ts | 4 +--- .../petrinaut-arch-docs/src/diff/base-tree.ts | 14 ++++++++++++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/libs/@local/petrinaut-arch-docs/src/cli.ts b/libs/@local/petrinaut-arch-docs/src/cli.ts index fb7b4b58c1b..e15cb717095 100644 --- a/libs/@local/petrinaut-arch-docs/src/cli.ts +++ b/libs/@local/petrinaut-arch-docs/src/cli.ts @@ -131,9 +131,7 @@ const withDiffAgainst = async ( // Logged because it names the strategy: a Vercel build has no usable // clone, and this line is how its logs show the fallback fetch worked. if (tree.fetchedFrom !== undefined) { - process.stdout.write( - dim(`base tree fetched from ${tree.fetchedFrom}\n`), - ); + process.stdout.write(dim(`base tree fetched from ${tree.fetchedFrom}\n`)); } const baseBundle = await buildBundle({ diff --git a/libs/@local/petrinaut-arch-docs/src/diff/base-tree.ts b/libs/@local/petrinaut-arch-docs/src/diff/base-tree.ts index a703e141b15..437649bb458 100644 --- a/libs/@local/petrinaut-arch-docs/src/diff/base-tree.ts +++ b/libs/@local/petrinaut-arch-docs/src/diff/base-tree.ts @@ -177,7 +177,12 @@ const fetchSparseTree = (options: { "origin", options.ref, ]); - const sha = git(root, ["rev-parse", "--verify", "--quiet", "FETCH_HEAD^{commit}"]); + const sha = git(root, [ + "rev-parse", + "--verify", + "--quiet", + "FETCH_HEAD^{commit}", + ]); // Sparse patterns are set before the checkout so only the covered // directories materialize — the checkout is also what batch-fetches their @@ -229,7 +234,12 @@ export const materializeBaseTree = async (options: { } const url = remoteRepoUrl(process.env); - const sha = fetchSparseTree({ url, ref: options.ref, paths: options.paths, root }); + const sha = fetchSparseTree({ + url, + ref: options.ref, + paths: options.paths, + root, + }); return { root, ref: options.ref, From fdaca3b28d3ce83e7f97233b7e57ba3b075a9628 Mon Sep 17 00:00:00 2001 From: Chris Feijoo <chris@kube.io> Date: Thu, 27 Aug 2026 15:20:36 +0200 Subject: [PATCH 05/11] FE-1514: Document diff builds on a dedicated page with a flow diagram --- .../content/diagrams/diff-build-flow.d2 | 23 +++++++ .../content/maintaining.mdx | 3 + .../maintaining/previewing-changes.mdx | 65 +++++++++++++++++++ 3 files changed, 91 insertions(+) create mode 100644 libs/@local/petrinaut-arch-docs/content/diagrams/diff-build-flow.d2 create mode 100644 libs/@local/petrinaut-arch-docs/content/maintaining/previewing-changes.mdx diff --git a/libs/@local/petrinaut-arch-docs/content/diagrams/diff-build-flow.d2 b/libs/@local/petrinaut-arch-docs/content/diagrams/diff-build-flow.d2 new file mode 100644 index 00000000000..0c40bbfca16 --- /dev/null +++ b/libs/@local/petrinaut-arch-docs/content/diagrams/diff-build-flow.d2 @@ -0,0 +1,23 @@ +# Hand-written; rendered by the arch-docs build. Palette matches src/emit/d2.ts. +direction: right + +branch: "branch checkout" {style.fill: "#f2f2f2"; style.stroke: "#777777"} +base-ref: "base ref\n(main)" {style.fill: "#f2f2f2"; style.stroke: "#777777"} +base-tree: "base source tree\n(scratch directory)" {style.fill: "#f2f2f2"; style.stroke: "#777777"} + +build-branch: "generator" {style.fill: "#fbe3e8"; style.stroke: "#ad3a5b"} +build-base: "generator\n(same version, same config)" {style.fill: "#fbe3e8"; style.stroke: "#ad3a5b"} +compare: "normalize + compare\nmask counts, ordering,\nincoming edges; diff per block" {style.fill: "#fbe3e8"; style.stroke: "#ad3a5b"} + +branch-bundle: "branch bundle" {style.fill: "#dcecff"; style.stroke: "#3676b8"} +base-bundle: "base bundle\n(in memory only)" {style.fill: "#dcecff"; style.stroke: "#3676b8"} +output: "annotated bundle\nbadges, block markers,\ntombstones" {style.fill: "#dcecff"; style.stroke: "#3676b8"} + +base-ref -> base-tree: git archive, or an\nanonymous sparse fetch {style.stroke-dash: 3} +branch -> build-branch +base-tree -> build-base +build-branch -> branch-bundle +build-base -> base-bundle +branch-bundle -> compare +base-bundle -> compare +compare -> output diff --git a/libs/@local/petrinaut-arch-docs/content/maintaining.mdx b/libs/@local/petrinaut-arch-docs/content/maintaining.mdx index 9fc2da45973..d6a49f8309c 100644 --- a/libs/@local/petrinaut-arch-docs/content/maintaining.mdx +++ b/libs/@local/petrinaut-arch-docs/content/maintaining.mdx @@ -19,6 +19,9 @@ Two things follow from that, and they are what these pages cover. to accept. - **[Running locally](doc:maintaining/running-locally)** — building the docs, reading them without a server, and what to do when a page looks stale. +- **[Previewing changes](doc:maintaining/previewing-changes)** — how a PR + preview highlights the pages and blocks the branch changed, and how that + comparison is built. Pages like this one are hand-written. They live in `libs/@local/petrinaut-arch-docs/content/` and are optional: the generator diff --git a/libs/@local/petrinaut-arch-docs/content/maintaining/previewing-changes.mdx b/libs/@local/petrinaut-arch-docs/content/maintaining/previewing-changes.mdx new file mode 100644 index 00000000000..541aa7ad85a --- /dev/null +++ b/libs/@local/petrinaut-arch-docs/content/maintaining/previewing-changes.mdx @@ -0,0 +1,65 @@ +--- +title: Previewing changes +description: How a PR preview highlights what the branch changed in these docs, and how the comparison is built. +sidebar_order: 3 +--- + +A preview deployment of these docs highlights what the branch changed: + +- The sidebar badges every page the branch **added**, **changed**, or + **removed**. A collapsed group carries a count of the flagged pages inside + it, so a change deep in a subtree is visible from the top. +- Inside a changed page, an added block carries a green bar on its left edge, + an edited block a blue one, and removed content appears in place as a red, + collapsed block. +- A removed page keeps a struck-through sidebar entry, linking to a stub that + carries the removed source. + +Production builds of `main` never highlight anything: the comparison runs only +where a base to compare against exists, which is a preview. + +## How the comparison is built + +![How a diff build compares the branch against the base ref](@diagrams/diff-build-flow.svg) + +The build runs the generator twice. Once over the branch checkout — that +produces the bundle the site renders — and once over the covered source +directories extracted at the base ref into a scratch directory. The base tree +comes out of the local clone with `git archive` when the clone can resolve the +ref; a Vercel build cannot (its sources arrive without a usable clone), so it +fetches the ref anonymously from the public repository instead, materialising +only the covered directories. Either way the _same_ generator version and +configuration run over both trees, so the two bundles can differ only where +the sources differ. + +The two bundles are then compared page by page and block by block. Raw +comparison would flag every neighbour of a real change, because generated +pages embed facts that shift whenever nearby code moves. Those are masked +first: + +- import counts, and the count-driven ordering of the depends-on list; +- file and line totals, and sidebar positions; +- the whole "depended on by" list — an incoming edge is the importing layer's + change, and it is flagged there, on its depends-on side. + +A page is flagged when anything else differs: its role, prose, name, declaring +file, outgoing edges, sub-layers, attached guides — or when the layer's file +membership moved, the one structural change the masked content cannot show. +Editing code inside a layer without moving files or edges changes nothing the +architecture describes, and flags nothing. + +The result is written into the bundle itself: the manifest records each page's +status for the sidebar, changed pages carry markers between their blocks, and +each removed page becomes a stub at its old address. + +## Running it locally + +```sh +yarn workspace @local/petrinaut-arch-docs doc:architecture --diff-base main +``` + +Any ref works as the base — `main~20`, a tag, a commit. Then build or serve +the site as usual (see [Running locally](doc:maintaining/running-locally)). +The build prints a summary line, `changes vs main (…): 2 added · 3 changed · +1 removed`, and warns instead of failing when it cannot resolve the base ref: +a build that cannot compare still writes a complete, unhighlighted bundle. From 864e40b8e2f7e05c32413fa9c3d8af8871e68460 Mon Sep 17 00:00:00 2001 From: Chris Feijoo <chris@kube.io> Date: Thu, 27 Aug 2026 16:09:37 +0200 Subject: [PATCH 06/11] FE-1514: Diff previews against the PR's target branch, with a cached base Which ref to compare against is the deployment's decision, so the docs app's CI script now resolves it: the PR's target branch via the GitHub API, so a stacked PR shows only its own delta, falling back to main. The generator stays ref-in, diff-out and never knows about PRs. The built base side is cached under node_modules/.cache keyed by the base commit and a hash of the generator's own inputs, so pushing to a PR whose base has not moved skips the base build. Each side's source-URL prefix is masked before comparison, which both removes derived noise and makes a cached base from an earlier build comparable. --- .../scripts/resolve-diff-base.mjs | 62 ++++++++ apps/petrinaut-docs/vercel-build.sh | 17 ++- libs/@local/petrinaut-arch-docs/README.md | 17 ++- .../maintaining/previewing-changes.mdx | 10 +- libs/@local/petrinaut-arch-docs/src/cli.ts | 103 +++++++++----- .../src/diff/base-cache.test.ts | 132 ++++++++++++++++++ .../src/diff/base-cache.ts | Bin 0 -> 4348 bytes .../petrinaut-arch-docs/src/diff/base-tree.ts | 64 +++++++-- .../src/diff/bundle-diff.test.ts | 43 +++++- .../src/diff/bundle-diff.ts | 94 +++++++++---- 10 files changed, 457 insertions(+), 85 deletions(-) create mode 100644 apps/petrinaut-docs/scripts/resolve-diff-base.mjs create mode 100644 libs/@local/petrinaut-arch-docs/src/diff/base-cache.test.ts create mode 100644 libs/@local/petrinaut-arch-docs/src/diff/base-cache.ts diff --git a/apps/petrinaut-docs/scripts/resolve-diff-base.mjs b/apps/petrinaut-docs/scripts/resolve-diff-base.mjs new file mode 100644 index 00000000000..251f6c87a4a --- /dev/null +++ b/apps/petrinaut-docs/scripts/resolve-diff-base.mjs @@ -0,0 +1,62 @@ +/** + * Prints the ref a preview build should highlight changes against. + * + * This is the policy half of the diff feature: *which* ref to compare with is + * a deployment question, so it lives here in the CI wrapper, while turning a + * ref into a comparable tree is the generator's job (`--diff-base`). + * + * For a pull-request deployment the base is the PR's target branch, so a + * stacked PR shows only its own delta rather than the whole stack's diff + * against `main`. Vercel does not expose the target branch, so it is read + * from the GitHub API — anonymously, which works because the repository is + * public. Anything short of a definite answer falls back to `main`, and a + * non-preview build prints nothing at all. + */ + +const out = (ref) => process.stdout.write(`${ref}\n`); + +const preset = process.env.PETRINAUT_ARCH_DOCS_DIFF_BASE?.trim(); +if (preset) { + out(preset); + process.exit(0); +} + +if (process.env.VERCEL_ENV !== "preview") { + process.exit(0); +} + +const { + VERCEL_GIT_REPO_OWNER, + VERCEL_GIT_REPO_SLUG, + VERCEL_GIT_PULL_REQUEST_ID, +} = process.env; + +const wellFormed = /^[\w.-]+$/u; +const prNumber = /^\d+$/u.test(VERCEL_GIT_PULL_REQUEST_ID ?? "") + ? VERCEL_GIT_PULL_REQUEST_ID + : null; + +if ( + prNumber === null || + !wellFormed.test(VERCEL_GIT_REPO_OWNER ?? "") || + !wellFormed.test(VERCEL_GIT_REPO_SLUG ?? "") +) { + // A preview without a PR (e.g. a branch deployment) has no target branch. + out("main"); + process.exit(0); +} + +try { + const response = await fetch( + `https://api.github.com/repos/${VERCEL_GIT_REPO_OWNER}/${VERCEL_GIT_REPO_SLUG}/pulls/${prNumber}`, + { + headers: { accept: "application/vnd.github+json" }, + signal: AbortSignal.timeout(10_000), + }, + ); + const baseRef = response.ok ? (await response.json()).base?.ref : undefined; + out(typeof baseRef === "string" && baseRef !== "" ? baseRef : "main"); +} catch { + // Rate limiting or a network failure: `main` still gives a useful diff. + out("main"); +} diff --git a/apps/petrinaut-docs/vercel-build.sh b/apps/petrinaut-docs/vercel-build.sh index 8cf361002c1..9848a42084c 100755 --- a/apps/petrinaut-docs/vercel-build.sh +++ b/apps/petrinaut-docs/vercel-build.sh @@ -15,13 +15,16 @@ cd ../.. # See: https://linear.app/hash/issue/H-3212/clean-up-env-files rm -f .env -# Preview deployments highlight what the branch changes against main: pages and -# blocks that differ get badges and markers, driven by the generator's diff -# mode. Production builds (main itself) never diff. An already-set variable -# wins, so the Vercel project can point previews at a different base. -if [[ "${VERCEL_ENV:-}" == "preview" && -z "${PETRINAUT_ARCH_DOCS_DIFF_BASE:-}" ]]; then - export PETRINAUT_ARCH_DOCS_DIFF_BASE="main" - echo "Preview build: highlighting changes against main" +# Preview deployments highlight what the branch changes against its base: +# the PR's target branch (so a stacked PR shows only its own delta), or `main` +# when the target cannot be determined. Which ref to use is decided here, in +# the deployment layer — the generator only ever receives a ref. Production +# builds print nothing and never diff. An already-set variable wins, so the +# Vercel project can pin a different base. +diff_base="$(node apps/petrinaut-docs/scripts/resolve-diff-base.mjs)" +if [[ -n "${diff_base}" ]]; then + export PETRINAUT_ARCH_DOCS_DIFF_BASE="${diff_base}" + echo "Preview build: highlighting changes against ${diff_base}" fi # Run through Turborepo rather than `yarn workspace ... build`: the package diff --git a/libs/@local/petrinaut-arch-docs/README.md b/libs/@local/petrinaut-arch-docs/README.md index 4ebbabfa9ad..fa13f1a1447 100644 --- a/libs/@local/petrinaut-arch-docs/README.md +++ b/libs/@local/petrinaut-arch-docs/README.md @@ -183,9 +183,16 @@ what differs, which is how a PR preview shows a reviewer where to look: yarn workspace @local/petrinaut-arch-docs doc:architecture --diff-base main # In CI (what vercel-build.sh sets on preview deployments) -PETRINAUT_ARCH_DOCS_DIFF_BASE=main turbo build --filter '@apps/petrinaut-docs' +PETRINAUT_ARCH_DOCS_DIFF_BASE=<ref> turbo build --filter '@apps/petrinaut-docs' ``` +Which ref to compare against is the caller's decision, not this package's: the +generator takes any ref and never knows about pull requests. On Vercel +previews, `apps/petrinaut-docs/scripts/resolve-diff-base.mjs` resolves the +PR's _target branch_ (via the GitHub API, anonymously) and sets the variable — +so a stacked PR is compared against the PR below it and shows only its own +delta, and a PR targeting `main` is compared against `main`. + The build extracts the covered package trees at the base ref with `git archive` and runs the _same_ generator over them — same config, same emitter, same source-URL prefix — so nothing but a real change can differ between the two @@ -224,6 +231,14 @@ otherwise) into a scratch repository: a blobless fetch plus a sparse checkout of only the covered directories, so the transfer stays small. The build log says `base tree fetched from …` when this path ran. +The built base side is cached under `node_modules/.cache/petrinaut-arch-docs`, +keyed by the base commit and a hash of the generator's own sources, config and +pinned dependencies — CI providers persist that directory between builds, so +pushing to a PR whose base has not moved skips the base build entirely (the +log says `base bundle from cache (…)`). Source-link URLs carry the built +commit and are masked per side before comparison, which is what makes a base +side built by an earlier build comparable at all. + The diff decorates the bundle, it never gates it: when the base ref cannot be resolved either way (no network, a repository that is not public) the build warns and writes a plain bundle. Production builds of `main` never set the diff --git a/libs/@local/petrinaut-arch-docs/content/maintaining/previewing-changes.mdx b/libs/@local/petrinaut-arch-docs/content/maintaining/previewing-changes.mdx index 541aa7ad85a..4ca1c16daff 100644 --- a/libs/@local/petrinaut-arch-docs/content/maintaining/previewing-changes.mdx +++ b/libs/@local/petrinaut-arch-docs/content/maintaining/previewing-changes.mdx @@ -15,8 +15,10 @@ A preview deployment of these docs highlights what the branch changed: - A removed page keeps a struck-through sidebar entry, linking to a stub that carries the removed source. -Production builds of `main` never highlight anything: the comparison runs only -where a base to compare against exists, which is a preview. +The base of the comparison is the PR's _target branch_: a PR against `main` +is compared with `main`, and a PR stacked on another PR is compared with the +branch below it, so each PR in a stack shows only its own delta. Production +builds of `main` never highlight anything. ## How the comparison is built @@ -48,6 +50,10 @@ membership moved, the one structural change the masked content cannot show. Editing code inside a layer without moving files or edges changes nothing the architecture describes, and flags nothing. +The base side is cached between builds, keyed by the base commit and the +generator's own sources, so pushing again while the base has not moved skips +the base build. + The result is written into the bundle itself: the manifest records each page's status for the sidebar, changed pages carry markers between their blocks, and each removed page becomes a stub at its old address. diff --git a/libs/@local/petrinaut-arch-docs/src/cli.ts b/libs/@local/petrinaut-arch-docs/src/cli.ts index e15cb717095..fff312359e6 100644 --- a/libs/@local/petrinaut-arch-docs/src/cli.ts +++ b/libs/@local/petrinaut-arch-docs/src/cli.ts @@ -14,8 +14,18 @@ import { fileURLToPath } from "node:url"; import { config } from "../architecture.config"; import { buildBundle, bundleTextFiles, type BuiltBundle } from "./build"; import { countErrors, type Diagnostic } from "./diagnostics"; -import { materializeBaseTree, type BaseTree } from "./diff/base-tree"; -import { applyBundleDiff } from "./diff/bundle-diff"; +import { + generatorInputsHash, + readCachedBaseSide, + writeCachedBaseSide, +} from "./diff/base-cache"; +import { + materializeBaseTree, + remoteRepoUrl, + resolveBaseSha, + type BaseTree, +} from "./diff/base-tree"; +import { applyBundleDiff, diffSideOfBundle } from "./diff/bundle-diff"; import { canRenderDiagrams, renderD2 } from "./emit/d2"; const repoRoot = fileURLToPath(new URL("../../../../", import.meta.url)); @@ -114,40 +124,69 @@ const withDiffAgainst = async ( ): Promise<BuiltBundle> => { let baseTree: BaseTree | null = null; try { - const tree = await materializeBaseTree({ - repoRoot, - ref, - paths: [ - ...new Set([ - ...config.packages.map((pkg) => pkg.path), - // For the base tree's authored content and dependency-cruiser - // tsconfig; the generator itself still runs from this checkout. - "libs/@local/petrinaut-arch-docs", - ]), - ], + const url = remoteRepoUrl(process.env); + const cacheDir = join(repoRoot, "node_modules/.cache/petrinaut-arch-docs"); + const inputsHash = generatorInputsHash({ + packageRoot: join(repoRoot, "libs/@local/petrinaut-arch-docs"), + includeDiagrams, }); - baseTree = tree; - // Logged because it names the strategy: a Vercel build has no usable - // clone, and this line is how its logs show the fallback fetch worked. - if (tree.fetchedFrom !== undefined) { - process.stdout.write(dim(`base tree fetched from ${tree.fetchedFrom}\n`)); + // The base side is deterministic in (base commit, generator inputs), so a + // build against an unmoved base reuses the last one instead of extracting + // and building the base tree again. + const knownSha = resolveBaseSha(repoRoot, ref, url); + let base = + knownSha === null + ? null + : readCachedBaseSide({ cacheDir, sha: knownSha, inputsHash }); + let baseSha = knownSha ?? ""; + + if (base !== null) { + process.stdout.write( + dim(`base bundle from cache (${baseSha.slice(0, 10)})\n`), + ); + } else { + const tree = await materializeBaseTree({ + repoRoot, + ref, + paths: [ + ...new Set([ + ...config.packages.map((pkg) => pkg.path), + // For the base tree's authored content and dependency-cruiser + // tsconfig; the generator itself still runs from this checkout. + "libs/@local/petrinaut-arch-docs", + ]), + ], + }); + baseTree = tree; + baseSha = tree.sha; + + // Logged because it names the strategy: a Vercel build has no usable + // clone, and this line is how its logs show the fallback fetch worked. + if (tree.fetchedFrom !== undefined) { + process.stdout.write( + dim(`base tree fetched from ${tree.fetchedFrom}\n`), + ); + } + + const baseBundle = await buildBundle({ + repoRoot: tree.root, + includeDiagrams, + overrides: { + // A package added since the base ref has no directory to scan there. + packages: config.packages.filter((pkg) => + existsSync(join(tree.root, pkg.path)), + ), + }, + }); + base = diffSideOfBundle(baseBundle, config.sourceUrlPrefix); + writeCachedBaseSide({ cacheDir, sha: baseSha, inputsHash, side: base }); } - const baseBundle = await buildBundle({ - repoRoot: tree.root, - includeDiagrams, - overrides: { - // A package added since the base ref has no directory to scan there. - packages: config.packages.filter((pkg) => - existsSync(join(tree.root, pkg.path)), - ), - }, - }); - - const diffed = applyBundleDiff(bundle, baseBundle, { + const diffed = applyBundleDiff(bundle, base, { baseRef: ref, - baseSha: tree.sha, + baseSha, + sourceUrlPrefix: config.sourceUrlPrefix, }); const statuses = Object.values(diffed.manifest.diff?.pages ?? {}); @@ -155,7 +194,7 @@ const withDiffAgainst = async ( statuses.filter((entry) => entry === status).length; process.stdout.write( dim( - `changes vs ${ref} (${tree.sha.slice(0, 10)}): ${count("added")} added · ${count("changed")} changed · ${count("removed")} removed\n`, + `changes vs ${ref} (${baseSha.slice(0, 10)}): ${count("added")} added · ${count("changed")} changed · ${count("removed")} removed\n`, ), ); diff --git a/libs/@local/petrinaut-arch-docs/src/diff/base-cache.test.ts b/libs/@local/petrinaut-arch-docs/src/diff/base-cache.test.ts new file mode 100644 index 00000000000..b47ff6f0036 --- /dev/null +++ b/libs/@local/petrinaut-arch-docs/src/diff/base-cache.test.ts @@ -0,0 +1,132 @@ +import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import { + generatorInputsHash, + readCachedBaseSide, + writeCachedBaseSide, +} from "./base-cache"; + +import type { DiffSide } from "./bundle-diff"; + +/** + * The cache's one hazard is serving a stale base: a side built by older + * generator code, or for another commit. Both key components are pinned here + * — a round trip only succeeds on an exact (sha, inputs) match, and the + * inputs hash moves when any generator source moves. + */ + +const sha = "a".repeat(40); + +const sideFixture: DiffSide = { + layers: [], + sourceUrlPrefix: "https://github.com/x/y/blob/aaaa/", + pages: [ + { + slug: "architecture", + title: "Architecture", + description: "", + order: 1000, + kind: "generated", + contents: '---\ntitle: "Architecture"\n---\n\nBody.\n', + }, + ], +}; + +let scratch: string; + +beforeEach(async () => { + scratch = await mkdtemp(join(tmpdir(), "base-cache-test-")); +}); + +afterEach(async () => { + await rm(scratch, { recursive: true, force: true }); +}); + +describe("base cache", () => { + it("round-trips a base side on an exact key match", () => { + writeCachedBaseSide({ + cacheDir: scratch, + sha, + inputsHash: "hash-1", + side: sideFixture, + }); + + expect( + readCachedBaseSide({ cacheDir: scratch, sha, inputsHash: "hash-1" }), + ).toEqual(sideFixture); + }); + + it("misses when the generator inputs moved", () => { + writeCachedBaseSide({ + cacheDir: scratch, + sha, + inputsHash: "hash-1", + side: sideFixture, + }); + + expect( + readCachedBaseSide({ cacheDir: scratch, sha, inputsHash: "hash-2" }), + ).toBeNull(); + }); + + it("misses on an absent or unreadable entry", async () => { + expect( + readCachedBaseSide({ cacheDir: scratch, sha, inputsHash: "hash-1" }), + ).toBeNull(); + + await writeFile(join(scratch, `base-${sha}.json`), "not json", "utf8"); + expect( + readCachedBaseSide({ cacheDir: scratch, sha, inputsHash: "hash-1" }), + ).toBeNull(); + }); +}); + +describe("generatorInputsHash", () => { + const packageFixture = async (): Promise<string> => { + const root = join(scratch, "pkg"); + await mkdir(join(root, "src"), { recursive: true }); + await writeFile(join(root, "architecture.config.ts"), "export {};"); + await writeFile(join(root, "dependency-cruiser.tsconfig.json"), "{}"); + await writeFile(join(root, "package.json"), "{}"); + await writeFile(join(root, "src/build.ts"), "export const a = 1;"); + await writeFile(join(root, "src/build.test.ts"), "test file"); + return root; + }; + + it("moves when a source file or a flag moves, and only then", async () => { + const root = await packageFixture(); + const before = generatorInputsHash({ + packageRoot: root, + includeDiagrams: true, + }); + + expect( + generatorInputsHash({ packageRoot: root, includeDiagrams: true }), + ).toBe(before); + expect( + generatorInputsHash({ packageRoot: root, includeDiagrams: false }), + ).not.toBe(before); + + await writeFile(join(root, "src/build.ts"), "export const a = 2;"); + expect( + generatorInputsHash({ packageRoot: root, includeDiagrams: true }), + ).not.toBe(before); + }); + + it("ignores test files", async () => { + const root = await packageFixture(); + const before = generatorInputsHash({ + packageRoot: root, + includeDiagrams: true, + }); + + await writeFile(join(root, "src/build.test.ts"), "changed test file"); + expect( + generatorInputsHash({ packageRoot: root, includeDiagrams: true }), + ).toBe(before); + }); +}); diff --git a/libs/@local/petrinaut-arch-docs/src/diff/base-cache.ts b/libs/@local/petrinaut-arch-docs/src/diff/base-cache.ts new file mode 100644 index 0000000000000000000000000000000000000000..76948133501523f240e2609197d845e2880157a2 GIT binary patch literal 4348 zcmb7HVQ<{H5$)&vin&E_)f#K<wYVOxHrzv+_>dI1B#3i|0!bQkCCbZKOQ|H~^}<=8 zKcc^|zoc)5qNLqRi}s7X63LmznKy4n%afBwbV5&PrF}TnRn@pag(}vXnu=DPsRON) z*W^v9{epZ$PPbYGT~cYPxTW-@Zj`Y;kg{y9vzz7)JLF`h@3h-d+xazZ*QVBFf_5qx zdqa&SMX%q?$={lm^nGxukOgH+HpBo*-WUK#tOchvX&a2w^)AOA-gaq&Ga4wIrdz#Z z&%5zR(QGy*&<Sjwz};E_2rehE!a7=4f&5yv+Os)!cB3tWY}{`>HC<@CK>zy3KLOn% z2AwN77)4_%bA$h`GhRCiKD~h}YK^s8Zc!p)w$!#@jBEu_R8*{$z0nk!Lw{uKovKZV z5c`OcuZsWz&04#NPg#P)JI1bJT>4b)BrDqC2;h}Er?<FFOlWQHV6p=ev2ty({&CZk zU9J5xmp#`DPEoO@r!S~=%^i~NJhk{^kld9HTdi%(3c^w`at7qpM)Uqpx(Mg6fISGf zK|)rZku(l&omQn<A^jX)Nh}{Jv}XEvG8N9SvC@=en*cVzCuey1=#km9Ow0#_nz{F! zQ@E$fH5;1oHO>mRYeO?TA9g+>+T51LUF~eKzyo_fdi)Vw9F{lfmk%nW-&+T&j@u4A zR(|4*J~hT3B5IK?>_~vZuGPT*3^j9ws1AGc<q8?8^(j{q?+1kt>ZebhKEM3<%jKJ^ z7q4E@1^o^y%af7mE4^#LG0@l3M%kTEzjYlLx1gp*tp=)$LjXB`(dJK2{`&Fq<=Zze zF0WwmTgGNliIpleJ>`-w|BOl(bf`oy){k%>5JHr(H;p|b+ih0bo#Pd1=8Qaw&))E( zv8WTz*%{yNBXEvlPrf6*x2m{RD1`00fzALbT&HT3dW+>Zxrn%l1}`Y<&%bG!ki5>* zk9QyF;!nijV+bliaY66!hG?c-u||fA&^euB2dX&_e#U&k>QJ`3)4sy7*zfw}8{6+& zF6mh+@LdX0)FOQkIb$J+$|>0@RLkx>M`45NZ$NSO)?XtZEPu#1s?9QnJm*_5#io~C zAi-FgBai8NTUFm8wHb(xOEqqn;(&nMHK?mT7(~!{%3@3qZKw;1c~T;yg#}2vHDS%+ zehUu!GYYN)f9G?FD`Ss#nDB5eFb4pOC|<dRLI5f622Co;#A|N+vjjDp7$oAG;Id&o z9NLqv!H?-##+1$J>#ymn<@+3Z8Q$mbip_@~g1`Ua@v@Uwy(a-MLPi8|*yr4pj{FG7 zJ^t<)uK@XV=x5#)vp)7tBcZl8=%G^?K;tkMEDPEztNW&d3OXb|H<vBMQtF1<x)WOT z>R8q4hM&=!Vy|FhTYhX&FJ4LoT^AC+ZIUK)s!0F6Q?<CnE@*7op~6?YpiiMZXq|<s z+0@wAg$^vsBINQuHq9u%`d-vGrg><LFNR`g$~A3(Hh>*mY^+pIR=%mbpvj>y`(812 zRa`-;UaFeX)3k;;_{J7>SL$a*-8i*@@m15*T3Lj5DDlMUh)$aO#iUJSGicQB|M1;R zh{Vfz*K%{suJ0Z{9LVhcyiATBKlJy>k09*Nuf@=$ewavnzfqM^F!G5s4eXvm;G;dk zr>|$fiD*f3nDBCWGUK^9MOFAON14ApFU4|cz?hKD*7}}pGdm~M|2md#CNlskGRX$! zy;0z&5r=iDiF9B=*UXyJ#}7EL7Y*Y$Ny~bh29SOp<VjKphY?67Gc+uRo>JE`4x*Rc z8niY<bYr|1f*cT#<8aijj`^2hrF$@!CO3)H`a-M16^91>IYR0clO>Xwoy0DR<^}!b z>eWl6*?FDGY8*6%G|4uO?m&qI4VR7;lLOd%L9;GY-_Pc;^~`x7I=rK(e3Z`KQB=^r zDbrUM7e_<G=g$Wl;YK!1+|tpOC_~SAxMFi@=lEI-jw0F<j8n%Ug<^6m8i*I;MO3RD zCWo`W6U3)@PcD4$GA+qNT8UHk&#r#$bY<?-o}<@sw<z@<V{X9c8r>dY+b2P$EA&Ok zBF5&!kDGI<l&PgN^Jj#I@Yl^2pAtN<vo=cRWT=G<+3pL>lyUJeZNHMnl>h&en&2<W zW~66Sz-dn(89L0pUm)EBRH5iF$KAnL-^hDvP4wXGh<vz3O6D*G9!TeqG?t6R8!#@t zA1O30kMGj1JkN)-xa}mj=)==dJk{F&TwZ-8NVUds)l|DU8T2KG2}~+5I%Vx*CXX@U zPq&MM5h+1U8J+h|a;Q4S>MWCygWJ>cZz7KGzB&D)I;}oDeEZG*y=dexqr~YX8IZYe zIGv@?6Y;wgEPUStvq2Wq2TLZ3e2i)#hvxHKcK_tlam;VYos2A5t*fvgXKvO(#!dMZ zPthsyYJ5ncnRGxPh%J!wfk5THHigb6-y1{sgE+{S)_(#krsA~7lP8=MQ=)QCWHdXd gbxhzdoA@OkXLd|gc2ct|%^%9lApZX3d0XV*-_d`9(f|Me literal 0 HcmV?d00001 diff --git a/libs/@local/petrinaut-arch-docs/src/diff/base-tree.ts b/libs/@local/petrinaut-arch-docs/src/diff/base-tree.ts index 437649bb458..d8aaffdab80 100644 --- a/libs/@local/petrinaut-arch-docs/src/diff/base-tree.ts +++ b/libs/@local/petrinaut-arch-docs/src/diff/base-tree.ts @@ -47,16 +47,8 @@ const git = (cwd: string, args: string[]): string => stdio: ["ignore", "pipe", "pipe"], }).trim(); -/** - * Resolves a ref against the local clone, or null when it cannot. - * - * A CI clone is typically shallow and checked out at the head commit only, so - * the base branch resolves neither bare nor as `origin/<ref>`; the fetch - * still succeeds where the clone has a credentialed remote (a developer - * machine, a GitHub Actions checkout). Null covers the rest — most notably a - * Vercel build, whose sources come as a snapshot with no fetchable clone. - */ -const resolveLocalCommit = (repoRoot: string, ref: string): string | null => { +/** The ref's commit per the local clone alone — no fetch, no side effects. */ +const localShaOf = (repoRoot: string, ref: string): string | null => { for (const candidate of [ref, `origin/${ref}`]) { try { return git(repoRoot, [ @@ -69,6 +61,56 @@ const resolveLocalCommit = (repoRoot: string, ref: string): string | null => { // Try the next form. } } + return null; +}; + +/** + * Resolves a ref to its commit without materializing anything, so a cache can + * be consulted before any tree is fetched or extracted. Null when only a full + * materialization could resolve it (say, `main~3` on a clone-less build). + */ +export const resolveBaseSha = ( + repoRoot: string, + ref: string, + url: string, +): string | null => { + if (/^[0-9a-f]{40}$/u.test(ref)) { + return ref; + } + + const local = localShaOf(repoRoot, ref); + if (local !== null) { + return local; + } + + try { + const listed = git(repoRoot, [ + "ls-remote", + url, + `refs/heads/${ref}`, + `refs/tags/${ref}`, + ]); + const sha = listed.split(/\s/u)[0] ?? ""; + return /^[0-9a-f]{40}$/u.test(sha) ? sha : null; + } catch { + return null; + } +}; + +/** + * Resolves a ref against the local clone, or null when it cannot. + * + * A CI clone is typically shallow and checked out at the head commit only, so + * the base branch resolves neither bare nor as `origin/<ref>`; the fetch + * still succeeds where the clone has a credentialed remote (a developer + * machine, a GitHub Actions checkout). Null covers the rest — most notably a + * Vercel build, whose sources come as a snapshot with no fetchable clone. + */ +const resolveLocalCommit = (repoRoot: string, ref: string): string | null => { + const local = localShaOf(repoRoot, ref); + if (local !== null) { + return local; + } try { git(repoRoot, ["fetch", "--quiet", "--depth=1", "origin", ref]); @@ -139,7 +181,7 @@ const extractFromLocalClone = (options: { * fallback is the same repository `source-url.ts` already assumes for source * links. The values are validated because they end up on a git command line. */ -const remoteRepoUrl = (env: NodeJS.ProcessEnv): string => { +export const remoteRepoUrl = (env: NodeJS.ProcessEnv): string => { const owner = env.VERCEL_GIT_REPO_OWNER; const slug = env.VERCEL_GIT_REPO_SLUG; const wellFormed = /^[\w.-]+$/u; diff --git a/libs/@local/petrinaut-arch-docs/src/diff/bundle-diff.test.ts b/libs/@local/petrinaut-arch-docs/src/diff/bundle-diff.test.ts index 164d79454ba..cb0b553016e 100644 --- a/libs/@local/petrinaut-arch-docs/src/diff/bundle-diff.test.ts +++ b/libs/@local/petrinaut-arch-docs/src/diff/bundle-diff.test.ts @@ -91,9 +91,14 @@ const authoredPage = (slug: string, body: string): DiffPage => ({ contents: `---\ntitle: "Guide"\n---\n\n${body}\n`, }); -const side = (pages: DiffPage[], layers: Layer[] = []): DiffSide => ({ +const side = ( + pages: DiffPage[], + layers: Layer[] = [], + sourceUrlPrefix = "", +): DiffSide => ({ pages, layers, + sourceUrlPrefix, }); describe("diffBundlePages", () => { @@ -161,6 +166,42 @@ describe("diffBundlePages", () => { expect(result.statuses).toEqual({}); }); + it("ignores each side's own source-URL prefix in generated pages", () => { + const pageWithPrefix = (prefix: string): DiffPage => ({ + slug: "architecture/core", + title: "Core", + description: "Headless engine.", + order: 1001, + kind: "generated", + contents: [ + "---", + 'title: "Core"', + "---", + "", + "<LayerFacts", + ` declaredInUrl={${JSON.stringify(`${prefix}libs/core/README.md`)}}`, + "/>", + "", + ].join("\n"), + }); + + const result = diffBundlePages({ + base: side( + [pageWithPrefix("https://github.com/x/y/blob/aaaa/")], + [], + "https://github.com/x/y/blob/aaaa/", + ), + head: side( + [pageWithPrefix("https://github.com/x/y/blob/bbbb/")], + [], + "https://github.com/x/y/blob/bbbb/", + ), + baseRef: "main", + }); + + expect(result.statuses).toEqual({}); + }); + it("flags a role change and marks the changed block", () => { const result = diffBundlePages({ base: side([generatedPage({ slug: "architecture/core" })]), diff --git a/libs/@local/petrinaut-arch-docs/src/diff/bundle-diff.ts b/libs/@local/petrinaut-arch-docs/src/diff/bundle-diff.ts index cff0ecf063c..3ef58e6d81b 100644 --- a/libs/@local/petrinaut-arch-docs/src/diff/bundle-diff.ts +++ b/libs/@local/petrinaut-arch-docs/src/diff/bundle-diff.ts @@ -35,8 +35,21 @@ export interface DiffPage { export interface DiffSide { pages: DiffPage[]; layers: Layer[]; + /** + * The source-URL prefix this side's generated pages embed. Each side's own + * prefix is replaced with one placeholder before comparison: the prefix + * carries the built commit, so it legitimately differs between a head build + * and a base side built earlier (a cached one), without a single page + * having changed. + */ + sourceUrlPrefix: string; } +const SOURCE_PREFIX_PLACEHOLDER = "https://source.invalid/"; + +const stripSourcePrefix = (text: string, prefix: string): string => + prefix === "" ? text : text.split(prefix).join(SOURCE_PREFIX_PLACEHOLDER); + export interface PageDiffResult { /** Page slug → how it differs from the base. Unchanged slugs are absent. */ statuses: Record<string, PageChange>; @@ -130,16 +143,26 @@ export const diffBundlePages = (options: { continue; } - const normalize = - page.kind === "generated" ? normalizeGeneratedBlock : identity; - - const head = splitFrontmatter(page.contents); - const base = splitFrontmatter(basePage.contents); - const headBlocks = splitBlocks(head.body); + const generated = page.kind === "generated"; + const normalize = generated ? normalizeGeneratedBlock : identity; + + // Compared on prefix-stripped copies; the raw head text is what gets + // annotated. The prefix holds no newline, so both split identically and + // block statuses map across by index. + const head = splitFrontmatter( + generated + ? stripSourcePrefix(page.contents, options.head.sourceUrlPrefix) + : page.contents, + ); + const base = splitFrontmatter( + generated + ? stripSourcePrefix(basePage.contents, options.base.sourceUrlPrefix) + : basePage.contents, + ); const diff = diffBlocks({ baseBlocks: splitBlocks(base.body), - headBlocks, + headBlocks: splitBlocks(head.body), normalize, }); @@ -156,12 +179,13 @@ export const diffBundlePages = (options: { statuses[page.slug] = "changed"; if (bodyChanged) { + const raw = splitFrontmatter(page.contents); annotated.set( page.slug, annotatePageBlocks({ slug: page.slug, - frontmatter: head.frontmatter, - blocks: headBlocks, + frontmatter: raw.frontmatter, + blocks: splitBlocks(raw.body), diff, }), ); @@ -179,24 +203,32 @@ export const diffBundlePages = (options: { return { statuses, annotated, tombstones }; }; -const toDiffPages = (bundle: BuiltBundle): DiffPage[] => [ - ...bundle.generated.map((page) => ({ - slug: page.slug, - title: page.title, - description: page.description, - order: page.order, - kind: "generated" as const, - contents: page.contents, - })), - ...bundle.authored.map((page) => ({ - slug: page.slug, - title: page.title, - description: page.description, - order: page.order, - kind: "authored" as const, - contents: page.contents, - })), -]; +/** The slice of a built bundle the diff consumes — also what a cache stores. */ +export const diffSideOfBundle = ( + bundle: BuiltBundle, + sourceUrlPrefix: string, +): DiffSide => ({ + layers: bundle.model.layers, + sourceUrlPrefix, + pages: [ + ...bundle.generated.map((page) => ({ + slug: page.slug, + title: page.title, + description: page.description, + order: page.order, + kind: "generated" as const, + contents: page.contents, + })), + ...bundle.authored.map((page) => ({ + slug: page.slug, + title: page.title, + description: page.description, + order: page.order, + kind: "authored" as const, + contents: page.contents, + })), + ], +}); /** * Returns the head bundle with the diff applied: changed pages carry block @@ -205,12 +237,12 @@ const toDiffPages = (bundle: BuiltBundle): DiffPage[] => [ */ export const applyBundleDiff = ( head: BuiltBundle, - base: BuiltBundle, - info: { baseRef: string; baseSha: string }, + base: DiffSide, + info: { baseRef: string; baseSha: string; sourceUrlPrefix: string }, ): BuiltBundle => { const { statuses, annotated, tombstones } = diffBundlePages({ - base: { pages: toDiffPages(base), layers: base.model.layers }, - head: { pages: toDiffPages(head), layers: head.model.layers }, + base, + head: diffSideOfBundle(head, info.sourceUrlPrefix), baseRef: info.baseRef, }); From bf9713d413d39f2e4fd13d10efda3728a1e395e1 Mon Sep 17 00:00:00 2001 From: Chris Feijoo <chris@kube.io> Date: Thu, 27 Aug 2026 16:17:24 +0200 Subject: [PATCH 07/11] FE-1514: Show what a preview compares in the header Two linked chips beside the title on diff builds: the deployed PR (or branch) and the base it is diffed against, each with the commit built. The base's PR is looked up by head branch, anonymously; the context is resolved once in astro.config.mjs and injected as a compile-time constant, since a bundled component has no stable path to the manifest. --- apps/petrinaut-docs/README.md | 27 ++-- apps/petrinaut-docs/astro.config.mjs | 16 +++ .../scripts/resolve-diff-base.mjs | 1 + .../src/components/DiffBadges.astro | 38 +++++ .../src/components/SiteTitle.astro | 12 +- apps/petrinaut-docs/src/diff-context.ts | 136 ++++++++++++++++++ apps/petrinaut-docs/src/styles/chrome.css | 57 ++++++++ 7 files changed, 275 insertions(+), 12 deletions(-) create mode 100644 apps/petrinaut-docs/src/components/DiffBadges.astro create mode 100644 apps/petrinaut-docs/src/diff-context.ts diff --git a/apps/petrinaut-docs/README.md b/apps/petrinaut-docs/README.md index 855669d547d..2e7f2ceecb0 100644 --- a/apps/petrinaut-docs/README.md +++ b/apps/petrinaut-docs/README.md @@ -50,15 +50,24 @@ gitignored, as is the bundle they come from. Nothing generated is versioned. ## Preview deployments highlight changes On a Vercel preview, [`vercel-build.sh`](vercel-build.sh) sets -`PETRINAUT_ARCH_DOCS_DIFF_BASE=main`, so the generator builds the bundle in -diff mode: the sidebar badges pages as `new`, `changed` or `removed` (with -roll-up counts on collapsed groups, so a change deep in a subtree is visible -from the top), and changed pages mark their differing blocks — green for -added, blue for edited, red collapsed blocks for removed content. Removed -pages keep a struck-through entry linking to a stub that carries the removed -source. Set the variable in the Vercel project to compare against a different -ref; production builds of `main` never diff. What counts as a change (and what -noise is filtered out) is the generator's contract — see its README section +`PETRINAUT_ARCH_DOCS_DIFF_BASE` to the PR's target branch — resolved by +[`scripts/resolve-diff-base.mjs`](scripts/resolve-diff-base.mjs) via the +GitHub API, falling back to `main` — so the generator builds the bundle in +diff mode against the right base, and a stacked PR shows only its own delta. + +The sidebar badges pages as `new`, `changed` or `removed` (with roll-up counts +on collapsed groups, so a change deep in a subtree is visible from the top), +and changed pages mark their differing blocks — green for added, blue for +edited, red collapsed blocks for removed content. Removed pages keep a +struck-through entry linking to a stub that carries the removed source. The +header carries two linked chips saying what is being compared — the deployed +PR and its base, each with the commit built +(`src/components/DiffBadges.astro`, resolved at config time and injected as a +compile-time constant). + +Set the variable in the Vercel project to pin a different base; production +builds of `main` never diff. What counts as a change (and what noise is +filtered out) is the generator's contract — see its README section "Highlighting changes against a base ref". ## Notes on configuration diff --git a/apps/petrinaut-docs/astro.config.mjs b/apps/petrinaut-docs/astro.config.mjs index 840b429ce93..33853d50532 100644 --- a/apps/petrinaut-docs/astro.config.mjs +++ b/apps/petrinaut-docs/astro.config.mjs @@ -6,6 +6,8 @@ import react from "@astrojs/react"; import starlight from "@astrojs/starlight"; import { defineConfig, fontProviders } from "astro/config"; +import { resolveDiffCompareContext } from "./src/diff-context"; + /** * Renders the architecture bundle produced by `@local/petrinaut-arch-docs`. * @@ -273,9 +275,23 @@ const hasAuthoredIndex = manifest.pages.some( (page) => page.kind === "authored" && page.slug === "index", ); +/** + * Resolved here rather than in the component that renders it: this file + * already owns reading the manifest, and after bundling a component has no + * stable relative path to the bundle. Injected as a compile-time constant + * below (`vite.define`). + */ +const diffCompareContext = await resolveDiffCompareContext(manifest); + export default defineConfig({ site: "https://docs.petrinaut.org", + vite: { + define: { + __PND_DIFF_COMPARE__: JSON.stringify(diffCompareContext), + }, + }, + ...(hasAuthoredIndex ? {} : { redirects: { "/": "/architecture" } }), // The bundle's inter-page links are relative and assume slugs map to URLs diff --git a/apps/petrinaut-docs/scripts/resolve-diff-base.mjs b/apps/petrinaut-docs/scripts/resolve-diff-base.mjs index 251f6c87a4a..e50cf476de3 100644 --- a/apps/petrinaut-docs/scripts/resolve-diff-base.mjs +++ b/apps/petrinaut-docs/scripts/resolve-diff-base.mjs @@ -13,6 +13,7 @@ * non-preview build prints nothing at all. */ +/** @param {string} ref */ const out = (ref) => process.stdout.write(`${ref}\n`); const preset = process.env.PETRINAUT_ARCH_DOCS_DIFF_BASE?.trim(); diff --git a/apps/petrinaut-docs/src/components/DiffBadges.astro b/apps/petrinaut-docs/src/components/DiffBadges.astro new file mode 100644 index 00000000000..02975bf9531 --- /dev/null +++ b/apps/petrinaut-docs/src/components/DiffBadges.astro @@ -0,0 +1,38 @@ +--- +/** + * On a diff build, two linked chips in the header say what is being compared: + * the deployed PR (or branch) and the base it is diffed against, each with the + * commit built. The arrow reads in the merge direction. Hidden entirely on a + * plain build, and on viewports too narrow to carry them. + * + * The context is injected by `astro.config.mjs` as a compile-time constant; + * see `src/diff-context.ts`. + */ + +import type { DiffCompareContext } from "../diff-context"; + +declare const __PND_DIFF_COMPARE__: DiffCompareContext | null | undefined; + +const context: DiffCompareContext | null = + typeof __PND_DIFF_COMPARE__ === "undefined" + ? null + : (__PND_DIFF_COMPARE__ ?? null); +--- + +{ + context !== null && ( + <span class="pnd-diff-compare"> + <a class="pnd-diff-chip" href={context.head.href} title={context.head.title}> + <span class="pnd-diff-chip-name">{context.head.label}</span> + {context.head.sha !== null && <code>{context.head.sha}</code>} + </a> + <span class="pnd-diff-compare-arrow" aria-hidden="true"> + → + </span> + <a class="pnd-diff-chip" href={context.base.href} title={context.base.title}> + <span class="pnd-diff-chip-name">{context.base.label}</span> + {context.base.sha !== null && <code>{context.base.sha}</code>} + </a> + </span> + ) +} diff --git a/apps/petrinaut-docs/src/components/SiteTitle.astro b/apps/petrinaut-docs/src/components/SiteTitle.astro index bd149a9ed1b..732e74cbca7 100644 --- a/apps/petrinaut-docs/src/components/SiteTitle.astro +++ b/apps/petrinaut-docs/src/components/SiteTitle.astro @@ -1,14 +1,18 @@ --- import Default from "@astrojs/starlight/components/SiteTitle.astro"; +import DiffBadges from "./DiffBadges.astro"; + /** - * Adds the desktop sidebar collapse toggle and resize handle beside the title. + * Adds the desktop sidebar collapse toggle and resize handle beside the title, + * and — on diff builds — the badges saying what is being compared. * * They hang off `SiteTitle` because it is the header's leftmost slot, next to * the rail's edge where the controls act, and the fixed header is the only * chrome still on screen once the sidebar pane itself is collapsed away. Both - * are display: none unless the page has a sidebar (see `chrome.css`), so the - * splash-template 404 page does not grow controls with nothing to control. + * controls are display: none unless the page has a sidebar (see `chrome.css`), + * so the splash-template 404 page does not grow controls with nothing to + * control. */ --- @@ -36,6 +40,8 @@ import Default from "@astrojs/starlight/components/SiteTitle.astro"; <Default><slot /></Default> +<DiffBadges /> + <div class="pnd-sidebar-resize" role="separator" diff --git a/apps/petrinaut-docs/src/diff-context.ts b/apps/petrinaut-docs/src/diff-context.ts new file mode 100644 index 00000000000..145f7898f59 --- /dev/null +++ b/apps/petrinaut-docs/src/diff-context.ts @@ -0,0 +1,136 @@ +/** + * What a diff build is comparing, for the header badges: the deployed side + * and the base side, each as a PR number when one exists (the branch name + * otherwise) plus the commit built. Null on a plain build, which is what + * hides the badges. + * + * Resolved once, from `astro.config.mjs`, and injected into the pages as a + * compile-time constant — a component resolving it itself would run after + * bundling, where file-relative paths to the bundle no longer hold. + * + * This is deployment chrome, so it lives in the site rather than the bundle: + * the bundle records refs and commits (`manifest.diff`), and mapping those to + * pull requests is GitHub knowledge the generator deliberately does not have. + */ + +import { execSync } from "node:child_process"; + +import type { BundleManifest } from "@local/petrinaut-arch-docs"; + +export interface DiffChip { + /** `#1234` when a PR exists, the branch name otherwise. */ + label: string; + /** Short commit id, when known. */ + sha: string | null; + href: string; + title: string; +} + +export interface DiffCompareContext { + head: DiffChip; + base: DiffChip; +} + +const wellFormed = /^[\w.-]+$/u; + +const gitFallback = (args: string): string | null => { + try { + return execSync(`git ${args}`, { encoding: "utf8" }).trim() || null; + } catch { + return null; + } +}; + +/** The one open PR whose head is `branch`, or null (anonymous API, best effort). */ +const openPrNumberFor = async ( + owner: string, + slug: string, + branch: string, +): Promise<string | null> => { + try { + const response = await fetch( + `https://api.github.com/repos/${owner}/${slug}/pulls?head=${owner}:${encodeURIComponent(branch)}&state=open`, + { + headers: { accept: "application/vnd.github+json" }, + signal: AbortSignal.timeout(10_000), + }, + ); + if (!response.ok) { + return null; + } + const pulls = (await response.json()) as { number?: number }[]; + const number = Array.isArray(pulls) ? pulls[0]?.number : undefined; + return typeof number === "number" ? String(number) : null; + } catch { + return null; + } +}; + +const chip = (options: { + repoUrl: string; + prNumber: string | null; + branch: string | null; + sha: string | null; + role: string; +}): DiffChip => { + const where = + options.prNumber !== null + ? `PR #${options.prNumber}` + : (options.branch ?? "an unknown ref"); + const at = options.sha === null ? "" : ` at ${options.sha.slice(0, 7)}`; + + return { + label: + options.prNumber !== null + ? `#${options.prNumber}` + : (options.branch ?? "?"), + sha: options.sha?.slice(0, 7) ?? null, + href: + options.prNumber !== null + ? `${options.repoUrl}/pull/${options.prNumber}` + : `${options.repoUrl}/tree/${options.branch ?? ""}`, + title: `${options.role}: ${where}${at}`, + }; +}; + +export const resolveDiffCompareContext = async ( + manifest: BundleManifest, +): Promise<DiffCompareContext | null> => { + const diff = manifest.diff; + if (diff === undefined) { + return null; + } + + const env = process.env; + const owner = wellFormed.test(env.VERCEL_GIT_REPO_OWNER ?? "") + ? env.VERCEL_GIT_REPO_OWNER! + : "hashintel"; + const slug = wellFormed.test(env.VERCEL_GIT_REPO_SLUG ?? "") + ? env.VERCEL_GIT_REPO_SLUG! + : "hash"; + const repoUrl = `https://github.com/${owner}/${slug}`; + + const headBranch = + env.VERCEL_GIT_COMMIT_REF ?? gitFallback("rev-parse --abbrev-ref HEAD"); + const headSha = env.VERCEL_GIT_COMMIT_SHA ?? gitFallback("rev-parse HEAD"); + const headPr = /^\d+$/u.test(env.VERCEL_GIT_PULL_REQUEST_ID ?? "") + ? env.VERCEL_GIT_PULL_REQUEST_ID! + : null; + + return { + head: chip({ + repoUrl, + prNumber: headPr, + branch: headBranch, + sha: headSha, + role: "This preview", + }), + base: chip({ + repoUrl, + prNumber: await openPrNumberFor(owner, slug, diff.baseRef), + branch: diff.baseRef, + sha: diff.baseSha, + role: "Compared against", + }), + }; +}; diff --git a/apps/petrinaut-docs/src/styles/chrome.css b/apps/petrinaut-docs/src/styles/chrome.css index cfad20c0a19..66a389c5c7f 100644 --- a/apps/petrinaut-docs/src/styles/chrome.css +++ b/apps/petrinaut-docs/src/styles/chrome.css @@ -250,3 +250,60 @@ font-size: 0.6rem; line-height: 1.4; } + +/* Compare badges (diff builds) ------------------------------------------- */ + +/* + * What this preview is compared against, next to the title: the deployed PR + * (or branch) and its base, each linking to GitHub. Desktop only — the mobile + * header has no room, and the information is a nicety, not navigation. + */ +.pnd-diff-compare { + display: none; +} + +@media (min-width: 50rem) { + .pnd-diff-compare { + display: inline-flex; + flex: none; + align-items: center; + gap: 0.4rem; + margin-inline-start: 1rem; + font-size: var(--sl-text-xs); + white-space: nowrap; + } +} + +.pnd-diff-compare .pnd-diff-chip { + display: inline-flex; + align-items: center; + gap: 0.35rem; + padding: 0.05rem 0.55rem; + border: 1px solid var(--sl-color-gray-5); + border-radius: 999px; + color: var(--sl-color-gray-2); + text-decoration: none; +} + +.pnd-diff-compare .pnd-diff-chip:hover { + border-color: var(--sl-color-gray-3); + color: var(--sl-color-white); +} + +.pnd-diff-compare .pnd-diff-chip-name { + max-width: 10rem; + overflow: hidden; + font-weight: 600; + text-overflow: ellipsis; +} + +.pnd-diff-compare .pnd-diff-chip > code { + padding: 0; + background: none; + font-size: 0.9em; + color: var(--sl-color-gray-3); +} + +.pnd-diff-compare .pnd-diff-compare-arrow { + color: var(--sl-color-gray-4); +} From 58de7508c9a01dd531e93f5feaafe6cc7fd1bfda Mon Sep 17 00:00:00 2001 From: Chris Feijoo <chris@kube.io> Date: Thu, 27 Aug 2026 16:27:15 +0200 Subject: [PATCH 08/11] FE-1514: Share the base cache across branches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cache entries are keyed in the file name by the generator-inputs hash then the base commit, so different generator versions keep separate entries instead of overwriting one slot. Every clean build also stores its own side as a future base: the CI cache falls back to the production deployment's, so the entry a main build writes is what a PR targeting main finds on its first build — the common case computes no base at all, and entries shared across branches are only ever written by protected-branch builds. --- libs/@local/petrinaut-arch-docs/README.md | 16 +++- .../maintaining/previewing-changes.mdx | 3 +- libs/@local/petrinaut-arch-docs/src/cli.ts | 75 +++++++++++++++--- .../src/diff/base-cache.test.ts | 29 ++++++- .../src/diff/base-cache.ts | Bin 4348 -> 4970 bytes 5 files changed, 108 insertions(+), 15 deletions(-) diff --git a/libs/@local/petrinaut-arch-docs/README.md b/libs/@local/petrinaut-arch-docs/README.md index fa13f1a1447..ca4cad678c6 100644 --- a/libs/@local/petrinaut-arch-docs/README.md +++ b/libs/@local/petrinaut-arch-docs/README.md @@ -232,10 +232,18 @@ of only the covered directories, so the transfer stays small. The build log says `base tree fetched from …` when this path ran. The built base side is cached under `node_modules/.cache/petrinaut-arch-docs`, -keyed by the base commit and a hash of the generator's own sources, config and -pinned dependencies — CI providers persist that directory between builds, so -pushing to a PR whose base has not moved skips the base build entirely (the -log says `base bundle from cache (…)`). Source-link URLs carry the built +keyed in the entry's file name by a hash of the generator's own sources, +config and pinned dependencies, then the base commit — CI providers persist +that directory between builds, so any later commit whose base has not moved +skips the base build entirely (the log says `base bundle from cache (…)`), +and builds running different generator versions against one base keep +separate entries. Every clean build also stores its _own_ side as a future +base: the CI cache is restored per branch with a fallback to the production +deployment, so the entry a `main` build writes is what a PR targeting `main` +finds on its first build — the common case computes no base at all. A cached entry is trusted exactly as far as the build that +wrote it — its pages are compiled as MDX — which is sound while only this +project's own builds write the directory; a shared remote cache would need +this analysis redone. Source-link URLs carry the built commit and are masked per side before comparison, which is what makes a base side built by an earlier build comparable at all. diff --git a/libs/@local/petrinaut-arch-docs/content/maintaining/previewing-changes.mdx b/libs/@local/petrinaut-arch-docs/content/maintaining/previewing-changes.mdx index 4ca1c16daff..d6497202a90 100644 --- a/libs/@local/petrinaut-arch-docs/content/maintaining/previewing-changes.mdx +++ b/libs/@local/petrinaut-arch-docs/content/maintaining/previewing-changes.mdx @@ -52,7 +52,8 @@ architecture describes, and flags nothing. The base side is cached between builds, keyed by the base commit and the generator's own sources, so pushing again while the base has not moved skips -the base build. +the base build. A build of `main` stores its own pages as the entry every PR +targeting `main` starts from, so the common case computes no base at all. The result is written into the bundle itself: the manifest records each page's status for the sidebar, changed pages carry markers between their blocks, and diff --git a/libs/@local/petrinaut-arch-docs/src/cli.ts b/libs/@local/petrinaut-arch-docs/src/cli.ts index fff312359e6..f5c0c6b03b7 100644 --- a/libs/@local/petrinaut-arch-docs/src/cli.ts +++ b/libs/@local/petrinaut-arch-docs/src/cli.ts @@ -6,6 +6,7 @@ * violated rule — so the map cannot quietly stop matching the code. */ +import { execFileSync } from "node:child_process"; import { existsSync } from "node:fs"; import { chmod, mkdir, rm, writeFile } from "node:fs/promises"; import { dirname, join } from "node:path"; @@ -108,6 +109,63 @@ const resolveDiffBase = (args: string[]): string | null => { : fromEnvironment; }; +const cacheDir = join(repoRoot, "node_modules/.cache/petrinaut-arch-docs"); + +/** Repo-relative directories whose content the generator reads. */ +const coveredPaths = (): string[] => [ + ...new Set([ + ...config.packages.map((pkg) => pkg.path), + "libs/@local/petrinaut-arch-docs", + ]), +]; + +/** + * Stores this build's own side as a future diff base. + * + * This is what shares the cache across branches: the CI cache is restored per + * branch with a fallback to the production deployment, so the entry a `main` + * build writes here is what every PR targeting `main` finds on its first + * build — no PR recomputes the base the production build already produced. + * It also means only protected-branch builds ever write the entries other + * branches read. + * + * Skipped when the checkout's commit is unknown, and locally when the covered + * sources have uncommitted changes — an entry must describe its commit, not a + * dirty tree. + */ +const seedBaseCache = (bundle: BuiltBundle, includeDiagrams: boolean): void => { + let sha = process.env.VERCEL_GIT_COMMIT_SHA ?? ""; + + if (!/^[0-9a-f]{40}$/u.test(sha)) { + try { + sha = execFileSync("git", ["rev-parse", "HEAD"], { + cwd: repoRoot, + encoding: "utf8", + }).trim(); + const dirty = execFileSync( + "git", + ["status", "--porcelain", "--", ...coveredPaths()], + { cwd: repoRoot, encoding: "utf8" }, + ).trim(); + if (!/^[0-9a-f]{40}$/u.test(sha) || dirty !== "") { + return; + } + } catch { + return; + } + } + + writeCachedBaseSide({ + cacheDir, + sha, + inputsHash: generatorInputsHash({ + packageRoot: join(repoRoot, "libs/@local/petrinaut-arch-docs"), + includeDiagrams, + }), + side: diffSideOfBundle(bundle, config.sourceUrlPrefix), + }); +}; + /** * Builds the base ref's bundle and applies the diff to the head one. * @@ -125,7 +183,6 @@ const withDiffAgainst = async ( let baseTree: BaseTree | null = null; try { const url = remoteRepoUrl(process.env); - const cacheDir = join(repoRoot, "node_modules/.cache/petrinaut-arch-docs"); const inputsHash = generatorInputsHash({ packageRoot: join(repoRoot, "libs/@local/petrinaut-arch-docs"), includeDiagrams, @@ -146,17 +203,13 @@ const withDiffAgainst = async ( dim(`base bundle from cache (${baseSha.slice(0, 10)})\n`), ); } else { + // The list includes the arch-docs package for the base tree's authored + // content and dependency-cruiser tsconfig; the generator itself still + // runs from this checkout. const tree = await materializeBaseTree({ repoRoot, ref, - paths: [ - ...new Set([ - ...config.packages.map((pkg) => pkg.path), - // For the base tree's authored content and dependency-cruiser - // tsconfig; the generator itself still runs from this checkout. - "libs/@local/petrinaut-arch-docs", - ]), - ], + paths: coveredPaths(), }); baseTree = tree; baseSha = tree.sha; @@ -301,6 +354,10 @@ const main = async (): Promise<number> => { return 1; } + // Every clean build stores its own side as a future diff base — a `main` + // build's entry is what spares each PR from rebuilding the base itself. + seedBaseCache(bundle, diagramsAvailable); + // Applied only to a bundle that already passed the checks: the diff decorates // the output, it never gates it. const diffBase = resolveDiffBase(process.argv.slice(3)); diff --git a/libs/@local/petrinaut-arch-docs/src/diff/base-cache.test.ts b/libs/@local/petrinaut-arch-docs/src/diff/base-cache.test.ts index b47ff6f0036..8e38d03b4b2 100644 --- a/libs/@local/petrinaut-arch-docs/src/diff/base-cache.test.ts +++ b/libs/@local/petrinaut-arch-docs/src/diff/base-cache.test.ts @@ -73,12 +73,39 @@ describe("base cache", () => { ).toBeNull(); }); + it("keeps entries for different generator versions side by side", () => { + const otherSide = { ...sideFixture, sourceUrlPrefix: "other/" }; + writeCachedBaseSide({ + cacheDir: scratch, + sha, + inputsHash: "hash-1", + side: sideFixture, + }); + writeCachedBaseSide({ + cacheDir: scratch, + sha, + inputsHash: "hash-2", + side: otherSide, + }); + + expect( + readCachedBaseSide({ cacheDir: scratch, sha, inputsHash: "hash-1" }), + ).toEqual(sideFixture); + expect( + readCachedBaseSide({ cacheDir: scratch, sha, inputsHash: "hash-2" }), + ).toEqual(otherSide); + }); + it("misses on an absent or unreadable entry", async () => { expect( readCachedBaseSide({ cacheDir: scratch, sha, inputsHash: "hash-1" }), ).toBeNull(); - await writeFile(join(scratch, `base-${sha}.json`), "not json", "utf8"); + await writeFile( + join(scratch, `base-hash-1-${sha}.json`), + "not json", + "utf8", + ); expect( readCachedBaseSide({ cacheDir: scratch, sha, inputsHash: "hash-1" }), ).toBeNull(); diff --git a/libs/@local/petrinaut-arch-docs/src/diff/base-cache.ts b/libs/@local/petrinaut-arch-docs/src/diff/base-cache.ts index 76948133501523f240e2609197d845e2880157a2..16da90ebd81cbd632877be0b4a14692f13654e78 100644 GIT binary patch delta 1014 zcmYk5&uSDw5XRZ$5LnS11cP`fgXnHpH-8Qyi3(x_FNz02Pm-FRnw?H&yO-`7*Fl!` z<k_&kf={q7<5iD>;8{Fa)w?l)fq|~-Z|eKiH-B&by#4vxjUOMsfOWtnL18NbheHBA z%$n2R4_cksPV4cs3x%&LKc-xcQ*VZm!1&?NavxsF<pDVg$UZ=cv4n~+j}Sdz8@4VK zO^I$mFtjmJ1S4&(YI=Y!*XFFYm0Sz8bJh(Yw?#pL+?3B1RPG&xXuXSo1GFwOct^0D zh~8T{&{LTJd;wm%$HB4+BE%AdxQLbKzDm9DTxw;B;Dnk)ASROwe~{&*Ejk?U%cu=h z$P||LAy6$|R7@d~<IupkRF;<GVl1_%#dAoJlV!G5rE|zL``)@X`_tOHu1Xp4n8=+0 z5gT%S_H<=$nyQKkM!_2*!G#fQAs-W7IXKUd81&%@no_)ua3H9~whla!xpKYn{P2zJ zQq^=YBQ)!wx{Q{omlo%iUaY_fweZYEc#4LrMg<BC`qJBJZ3`)67zfYlJ*5bZp%G&y zG%_+Mq-0qlAk?EeI3~k)BKYxtuevKzu~JsQNQQi)0fP>(ng&^GqX`j5ndN{&+xVRN z@Y?2-Vv`K1cM*=pkCIv^qU4MvkY^bPG}J+jc6MRxYhk*KCzs*_9ac@W@c?qKSana^ z^RFv*ZI)j?oKCv?5IHEbhmYC-$KJY*yaD`(HJ$yWd}u>2ogQgG@80@wR<&+ab9^R0 zOFvfDP-mwHyZ5@23oR1XN&h%{ccc>CgLZp+Yb*V22IscD)|!91w7YsWMeZeX*;#6L dd$V6xHr8OX=HmY5Y-9cH>h8VC{AB&K^$&TuUf%!! delta 446 zcmZvXF-rqM5QRA}AOsZx8Z1=CAQD47G+2n(2v!y<Xf4^?&gE8;-LShka)@YWYdHVO z{TUk@t+chZaBgF?af@Mx`QDp1ADdt6kI&1U6l{#pVFn>o{l2i!V3Ie+pNFN_`zOeD zyrVlPentU<FlMrgV3kvKQKDv6X%I4DJ;9kp1DK#r0i~r%(-?Ib8pJaR26ZB2myF!P z&QSC$Py{Zb2sKG41IGX^DN@Knsy&fl*}zb8XOq3pw6Zn7>XT)*=eO!mcyJhK6(Vv2 z%z%hM*kUCY9Hg><BVqEFlA2w0p2tETfU%l_G{MAd+GOLo#g!@1gmw{ed*g<P+u+(L z+0MQBC9fL%A!=ZSk|rP|uP*HR?J~IMZa=5f>r>)i%~tb=Ke;$N4JL_1l-8a6cUT+R QXumc=vr*+Lf2%xr-yh7D6aWAK From 3478adde6e0656f1b2eb613940afbc686cc2ebc7 Mon Sep 17 00:00:00 2001 From: Chris Feijoo <chris@kube.io> Date: Thu, 27 Aug 2026 16:56:55 +0200 Subject: [PATCH 09/11] FE-1514: Apply self-review to the diff mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Base-side acquisition (cache lookup, materialization, build, seeding) moves from cli.ts into diff/base-side.ts, leaving the CLI as wiring and logging. Ref validation runs at every entry point that takes a ref, not only in materialization, so no call order lets an unvalidated ref reach a git command line; the credentialed origin-fetch fallback is removed — local resolution either succeeds or the anonymous fetch takes over. ls-remote resolution peels annotated tags to the commit the cache and materialization both key on. The CI resolver's crash costs the highlighting instead of the deploy, and it only prints branch-name shaped output; branch names are URL-encoded in the header chips. --- .../scripts/resolve-diff-base.mjs | 8 +- apps/petrinaut-docs/src/diff-context.ts | 5 +- apps/petrinaut-docs/vercel-build.sh | 3 +- libs/@local/petrinaut-arch-docs/src/cli.ts | 158 +++-------------- .../petrinaut-arch-docs/src/diff/base-side.ts | 164 ++++++++++++++++++ .../petrinaut-arch-docs/src/diff/base-tree.ts | 85 +++++---- 6 files changed, 243 insertions(+), 180 deletions(-) create mode 100644 libs/@local/petrinaut-arch-docs/src/diff/base-side.ts diff --git a/apps/petrinaut-docs/scripts/resolve-diff-base.mjs b/apps/petrinaut-docs/scripts/resolve-diff-base.mjs index e50cf476de3..4b0efef2275 100644 --- a/apps/petrinaut-docs/scripts/resolve-diff-base.mjs +++ b/apps/petrinaut-docs/scripts/resolve-diff-base.mjs @@ -56,7 +56,13 @@ try { }, ); const baseRef = response.ok ? (await response.json()).base?.ref : undefined; - out(typeof baseRef === "string" && baseRef !== "" ? baseRef : "main"); + // Branch-name shaped only: the value becomes an environment variable that + // ends up on a git command line, so anything surprising falls back. + out( + typeof baseRef === "string" && /^[\w][\w.@+/-]*$/u.test(baseRef) + ? baseRef + : "main", + ); } catch { // Rate limiting or a network failure: `main` still gives a useful diff. out("main"); diff --git a/apps/petrinaut-docs/src/diff-context.ts b/apps/petrinaut-docs/src/diff-context.ts index 145f7898f59..58c07ac58f4 100644 --- a/apps/petrinaut-docs/src/diff-context.ts +++ b/apps/petrinaut-docs/src/diff-context.ts @@ -88,7 +88,10 @@ const chip = (options: { href: options.prNumber !== null ? `${options.repoUrl}/pull/${options.prNumber}` - : `${options.repoUrl}/tree/${options.branch ?? ""}`, + : `${options.repoUrl}/tree/${(options.branch ?? "") + .split("/") + .map(encodeURIComponent) + .join("/")}`, title: `${options.role}: ${where}${at}`, }; }; diff --git a/apps/petrinaut-docs/vercel-build.sh b/apps/petrinaut-docs/vercel-build.sh index 9848a42084c..cdbd7acfa1b 100755 --- a/apps/petrinaut-docs/vercel-build.sh +++ b/apps/petrinaut-docs/vercel-build.sh @@ -21,7 +21,8 @@ rm -f .env # the deployment layer — the generator only ever receives a ref. Production # builds print nothing and never diff. An already-set variable wins, so the # Vercel project can pin a different base. -diff_base="$(node apps/petrinaut-docs/scripts/resolve-diff-base.mjs)" +# `|| true`: a crash in the resolver must cost the highlighting, not the deploy. +diff_base="$(node apps/petrinaut-docs/scripts/resolve-diff-base.mjs || true)" if [[ -n "${diff_base}" ]]; then export PETRINAUT_ARCH_DOCS_DIFF_BASE="${diff_base}" echo "Preview build: highlighting changes against ${diff_base}" diff --git a/libs/@local/petrinaut-arch-docs/src/cli.ts b/libs/@local/petrinaut-arch-docs/src/cli.ts index f5c0c6b03b7..e504f72c9bc 100644 --- a/libs/@local/petrinaut-arch-docs/src/cli.ts +++ b/libs/@local/petrinaut-arch-docs/src/cli.ts @@ -6,8 +6,6 @@ * violated rule — so the map cannot quietly stop matching the code. */ -import { execFileSync } from "node:child_process"; -import { existsSync } from "node:fs"; import { chmod, mkdir, rm, writeFile } from "node:fs/promises"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; @@ -15,18 +13,8 @@ import { fileURLToPath } from "node:url"; import { config } from "../architecture.config"; import { buildBundle, bundleTextFiles, type BuiltBundle } from "./build"; import { countErrors, type Diagnostic } from "./diagnostics"; -import { - generatorInputsHash, - readCachedBaseSide, - writeCachedBaseSide, -} from "./diff/base-cache"; -import { - materializeBaseTree, - remoteRepoUrl, - resolveBaseSha, - type BaseTree, -} from "./diff/base-tree"; -import { applyBundleDiff, diffSideOfBundle } from "./diff/bundle-diff"; +import { obtainBaseSide, seedBaseCacheWithSelf } from "./diff/base-side"; +import { applyBundleDiff } from "./diff/bundle-diff"; import { canRenderDiagrams, renderD2 } from "./emit/d2"; const repoRoot = fileURLToPath(new URL("../../../../", import.meta.url)); @@ -109,136 +97,37 @@ const resolveDiffBase = (args: string[]): string | null => { : fromEnvironment; }; -const cacheDir = join(repoRoot, "node_modules/.cache/petrinaut-arch-docs"); - -/** Repo-relative directories whose content the generator reads. */ -const coveredPaths = (): string[] => [ - ...new Set([ - ...config.packages.map((pkg) => pkg.path), - "libs/@local/petrinaut-arch-docs", - ]), -]; - /** - * Stores this build's own side as a future diff base. - * - * This is what shares the cache across branches: the CI cache is restored per - * branch with a fallback to the production deployment, so the entry a `main` - * build writes here is what every PR targeting `main` finds on its first - * build — no PR recomputes the base the production build already produced. - * It also means only protected-branch builds ever write the entries other - * branches read. + * Applies the diff against the base ref to the head bundle. * - * Skipped when the checkout's commit is unknown, and locally when the covered - * sources have uncommitted changes — an entry must describe its commit, not a - * dirty tree. - */ -const seedBaseCache = (bundle: BuiltBundle, includeDiagrams: boolean): void => { - let sha = process.env.VERCEL_GIT_COMMIT_SHA ?? ""; - - if (!/^[0-9a-f]{40}$/u.test(sha)) { - try { - sha = execFileSync("git", ["rev-parse", "HEAD"], { - cwd: repoRoot, - encoding: "utf8", - }).trim(); - const dirty = execFileSync( - "git", - ["status", "--porcelain", "--", ...coveredPaths()], - { cwd: repoRoot, encoding: "utf8" }, - ).trim(); - if (!/^[0-9a-f]{40}$/u.test(sha) || dirty !== "") { - return; - } - } catch { - return; - } - } - - writeCachedBaseSide({ - cacheDir, - sha, - inputsHash: generatorInputsHash({ - packageRoot: join(repoRoot, "libs/@local/petrinaut-arch-docs"), - includeDiagrams, - }), - side: diffSideOfBundle(bundle, config.sourceUrlPrefix), - }); -}; - -/** - * Builds the base ref's bundle and applies the diff to the head one. - * - * The base build runs the current generator over the base sources, with the - * current config and source-URL prefix, so the comparison never sees emitter - * or prefix drift. Any failure — an unfetchable ref, a base tree the - * generator cannot process — degrades to the plain bundle with a warning: a - * broken diff must not take the docs down with it. + * The base side comes from `obtainBaseSide` — the cache, the local clone, or + * an anonymous fetch — always built by the current generator with the current + * config, so the comparison never sees emitter or prefix drift. Any failure + * degrades to the plain bundle with a warning: a broken diff must not take + * the docs down with it. */ const withDiffAgainst = async ( bundle: BuiltBundle, ref: string, includeDiagrams: boolean, ): Promise<BuiltBundle> => { - let baseTree: BaseTree | null = null; try { - const url = remoteRepoUrl(process.env); - const inputsHash = generatorInputsHash({ - packageRoot: join(repoRoot, "libs/@local/petrinaut-arch-docs"), - includeDiagrams, - }); + const base = await obtainBaseSide({ repoRoot, ref, includeDiagrams }); - // The base side is deterministic in (base commit, generator inputs), so a - // build against an unmoved base reuses the last one instead of extracting - // and building the base tree again. - const knownSha = resolveBaseSha(repoRoot, ref, url); - let base = - knownSha === null - ? null - : readCachedBaseSide({ cacheDir, sha: knownSha, inputsHash }); - let baseSha = knownSha ?? ""; - - if (base !== null) { + if (base.fromCache) { process.stdout.write( - dim(`base bundle from cache (${baseSha.slice(0, 10)})\n`), + dim(`base bundle from cache (${base.sha.slice(0, 10)})\n`), ); - } else { - // The list includes the arch-docs package for the base tree's authored - // content and dependency-cruiser tsconfig; the generator itself still - // runs from this checkout. - const tree = await materializeBaseTree({ - repoRoot, - ref, - paths: coveredPaths(), - }); - baseTree = tree; - baseSha = tree.sha; - - // Logged because it names the strategy: a Vercel build has no usable - // clone, and this line is how its logs show the fallback fetch worked. - if (tree.fetchedFrom !== undefined) { - process.stdout.write( - dim(`base tree fetched from ${tree.fetchedFrom}\n`), - ); - } - - const baseBundle = await buildBundle({ - repoRoot: tree.root, - includeDiagrams, - overrides: { - // A package added since the base ref has no directory to scan there. - packages: config.packages.filter((pkg) => - existsSync(join(tree.root, pkg.path)), - ), - }, - }); - base = diffSideOfBundle(baseBundle, config.sourceUrlPrefix); - writeCachedBaseSide({ cacheDir, sha: baseSha, inputsHash, side: base }); + } + // Logged because it names the strategy: a Vercel build has no usable + // clone, and this line is how its logs show the fallback fetch worked. + if (base.fetchedFrom !== undefined) { + process.stdout.write(dim(`base tree fetched from ${base.fetchedFrom}\n`)); } - const diffed = applyBundleDiff(bundle, base, { + const diffed = applyBundleDiff(bundle, base.side, { baseRef: ref, - baseSha, + baseSha: base.sha, sourceUrlPrefix: config.sourceUrlPrefix, }); @@ -247,7 +136,7 @@ const withDiffAgainst = async ( statuses.filter((entry) => entry === status).length; process.stdout.write( dim( - `changes vs ${ref} (${baseSha.slice(0, 10)}): ${count("added")} added · ${count("changed")} changed · ${count("removed")} removed\n`, + `changes vs ${ref} (${base.sha.slice(0, 10)}): ${count("added")} added · ${count("changed")} changed · ${count("removed")} removed\n`, ), ); @@ -257,9 +146,6 @@ const withDiffAgainst = async ( `${yellow("warning")} building without change highlighting: could not compare against \`${ref}\`\n ${cause instanceof Error ? cause.message : String(cause)}\n`, ); return bundle; - } finally { - // A failed cleanup must not reject out of the build the diff decorates. - await baseTree?.dispose().catch(() => {}); } }; @@ -356,7 +242,11 @@ const main = async (): Promise<number> => { // Every clean build stores its own side as a future diff base — a `main` // build's entry is what spares each PR from rebuilding the base itself. - seedBaseCache(bundle, diagramsAvailable); + seedBaseCacheWithSelf({ + repoRoot, + bundle, + includeDiagrams: diagramsAvailable, + }); // Applied only to a bundle that already passed the checks: the diff decorates // the output, it never gates it. diff --git a/libs/@local/petrinaut-arch-docs/src/diff/base-side.ts b/libs/@local/petrinaut-arch-docs/src/diff/base-side.ts new file mode 100644 index 00000000000..248d10773c1 --- /dev/null +++ b/libs/@local/petrinaut-arch-docs/src/diff/base-side.ts @@ -0,0 +1,164 @@ +/** + * Obtains the base side of a diff build, and seeds the cache for future ones. + * + * One entry point per direction: `obtainBaseSide` serves a diff build — from + * the cache when the base commit and generator match a stored entry, else by + * materializing the base tree and building it, writing the result back — and + * `seedBaseCacheWithSelf` stores a finished build's own side so a later build + * can use this commit as its base without rebuilding it. + */ + +import { execFileSync } from "node:child_process"; +import { existsSync } from "node:fs"; +import { join } from "node:path"; + +import { config } from "../../architecture.config"; +import { buildBundle, type BuiltBundle } from "../build"; +import { + generatorInputsHash, + readCachedBaseSide, + writeCachedBaseSide, +} from "./base-cache"; +import { + materializeBaseTree, + remoteRepoUrl, + resolveBaseSha, +} from "./base-tree"; +import { diffSideOfBundle, type DiffSide } from "./bundle-diff"; + +/** Repo-relative root of this generator package. */ +const GENERATOR_PACKAGE = "libs/@local/petrinaut-arch-docs"; + +/** + * Repo-relative directories whose content the generator reads: the covered + * packages, plus this package for the authored content and the + * dependency-cruiser tsconfig. + */ +export const coveredPaths = (): string[] => [ + ...new Set([...config.packages.map((pkg) => pkg.path), GENERATOR_PACKAGE]), +]; + +/** Where cache entries live; `node_modules/.cache` is what CI persists. */ +const baseCacheDir = (repoRoot: string): string => + join(repoRoot, "node_modules/.cache/petrinaut-arch-docs"); + +const inputsHashFor = (repoRoot: string, includeDiagrams: boolean): string => + generatorInputsHash({ + packageRoot: join(repoRoot, GENERATOR_PACKAGE), + includeDiagrams, + }); + +export interface ObtainedBaseSide { + side: DiffSide; + sha: string; + fromCache: boolean; + /** The remote URL the tree came from, when the local clone could not supply it. */ + fetchedFrom?: string; +} + +/** + * The base side for a ref. Throws when the ref cannot be resolved or its tree + * cannot be built — the caller degrades to an unhighlighted bundle. + */ +export const obtainBaseSide = async (options: { + repoRoot: string; + ref: string; + includeDiagrams: boolean; +}): Promise<ObtainedBaseSide> => { + const { repoRoot, ref, includeDiagrams } = options; + const cacheDir = baseCacheDir(repoRoot); + const inputsHash = inputsHashFor(repoRoot, includeDiagrams); + + // The base side is deterministic in (base commit, generator inputs), so a + // build against an unmoved base reuses the stored one instead of extracting + // and building the base tree again. + const knownSha = resolveBaseSha(repoRoot, ref, remoteRepoUrl(process.env)); + if (knownSha !== null) { + const cached = readCachedBaseSide({ cacheDir, sha: knownSha, inputsHash }); + if (cached !== null) { + return { side: cached, sha: knownSha, fromCache: true }; + } + } + + const tree = await materializeBaseTree({ + repoRoot, + ref, + paths: coveredPaths(), + }); + + try { + const bundle = await buildBundle({ + repoRoot: tree.root, + includeDiagrams, + overrides: { + // A package added since the base ref has no directory to scan there. + packages: config.packages.filter((pkg) => + existsSync(join(tree.root, pkg.path)), + ), + }, + }); + const side = diffSideOfBundle(bundle, config.sourceUrlPrefix); + writeCachedBaseSide({ cacheDir, sha: tree.sha, inputsHash, side }); + + return { + side, + sha: tree.sha, + fromCache: false, + ...(tree.fetchedFrom === undefined + ? {} + : { fetchedFrom: tree.fetchedFrom }), + }; + } finally { + await tree.dispose().catch(() => { + // A failed cleanup must not reject out of the build the diff decorates. + }); + } +}; + +/** + * Stores a finished build's own side as a future diff base. + * + * This is what shares the cache across branches: the CI cache is restored per + * branch with a fallback to the production deployment, so the entry a `main` + * build writes here is what every PR targeting `main` finds on its first + * build. It also means only protected-branch builds ever write the entries + * other branches read. + * + * Skipped when the checkout's commit is unknown, and locally when the covered + * sources have uncommitted changes — an entry must describe its commit, not a + * dirty tree. + */ +export const seedBaseCacheWithSelf = (options: { + repoRoot: string; + bundle: BuiltBundle; + includeDiagrams: boolean; +}): void => { + const { repoRoot } = options; + let sha = process.env.VERCEL_GIT_COMMIT_SHA ?? ""; + + if (!/^[0-9a-f]{40}$/u.test(sha)) { + try { + sha = execFileSync("git", ["rev-parse", "HEAD"], { + cwd: repoRoot, + encoding: "utf8", + }).trim(); + const dirty = execFileSync( + "git", + ["status", "--porcelain", "--", ...coveredPaths()], + { cwd: repoRoot, encoding: "utf8" }, + ).trim(); + if (!/^[0-9a-f]{40}$/u.test(sha) || dirty !== "") { + return; + } + } catch { + return; + } + } + + writeCachedBaseSide({ + cacheDir: baseCacheDir(repoRoot), + sha, + inputsHash: inputsHashFor(repoRoot, options.includeDiagrams), + side: diffSideOfBundle(options.bundle, config.sourceUrlPrefix), + }); +}; diff --git a/libs/@local/petrinaut-arch-docs/src/diff/base-tree.ts b/libs/@local/petrinaut-arch-docs/src/diff/base-tree.ts index d8aaffdab80..3f4f64b320f 100644 --- a/libs/@local/petrinaut-arch-docs/src/diff/base-tree.ts +++ b/libs/@local/petrinaut-arch-docs/src/diff/base-tree.ts @@ -4,14 +4,13 @@ * Two strategies, tried in order: * * 1. The local clone: resolve the ref and `git archive` the covered - * directories out of it. This is what a developer machine and a GitHub - * Actions checkout take. - * 2. An anonymous fetch from the public repository: a Vercel build gets a - * snapshot of the sources with no usable git clone behind it, so the ref - * is fetched into a scratch repository instead — blobless - * (`--filter=blob:none`) with a sparse checkout of only the covered - * directories, which keeps the transfer to the trees plus the blobs the - * generator actually scans. + * directories out of it. This is what a developer machine takes. + * 2. An anonymous fetch from the public repository: a CI build often cannot + * resolve the ref locally — Vercel provides a snapshot of the sources with + * no usable clone at all — so the ref is fetched into a scratch repository + * instead: blobless (`--filter=blob:none`) with a sparse checkout of only + * the covered directories, which keeps the transfer to the trees plus the + * blobs the generator actually scans. * * Either way the extracted tree needs no `node_modules` and no install step: * dependency-cruiser resolves workspace imports through aliases derived from @@ -47,6 +46,17 @@ const git = (cwd: string, args: string[]): string => stdio: ["ignore", "pipe", "pipe"], }).trim(); +/** + * Whether a ref can safely appear on a git command line: a leading dash + * parses as an option, and whitespace or control characters split arguments. + * Checked at every entry point that takes a ref, so no call order lets an + * unvalidated ref reach a subprocess. + */ +const isUsableRef = (ref: string): boolean => + ref !== "" && !ref.startsWith("-") && !/[\s\u0000-\u001f]/u.test(ref); + +const isCommitSha = (text: string): boolean => /^[0-9a-f]{40}$/u.test(text); + /** The ref's commit per the local clone alone — no fetch, no side effects. */ const localShaOf = (repoRoot: string, ref: string): string | null => { for (const candidate of [ref, `origin/${ref}`]) { @@ -68,13 +78,20 @@ const localShaOf = (repoRoot: string, ref: string): string | null => { * Resolves a ref to its commit without materializing anything, so a cache can * be consulted before any tree is fetched or extracted. Null when only a full * materialization could resolve it (say, `main~3` on a clone-less build). + * + * Tags are peeled: the `^{}` line names the commit an annotated tag points + * at, which is also what a later materialization resolves — reporting the tag + * object instead would key the cache under a sha no other step produces. */ export const resolveBaseSha = ( repoRoot: string, ref: string, url: string, ): string | null => { - if (/^[0-9a-f]{40}$/u.test(ref)) { + if (!isUsableRef(ref)) { + return null; + } + if (isCommitSha(ref)) { return ref; } @@ -84,42 +101,26 @@ export const resolveBaseSha = ( } try { - const listed = git(repoRoot, [ + const lines = git(repoRoot, [ "ls-remote", url, `refs/heads/${ref}`, `refs/tags/${ref}`, - ]); - const sha = listed.split(/\s/u)[0] ?? ""; - return /^[0-9a-f]{40}$/u.test(sha) ? sha : null; - } catch { - return null; - } -}; + `refs/tags/${ref}^{}`, + ]) + .split("\n") + .filter((line) => line !== "") + .map((line) => line.split(/\s+/u)); -/** - * Resolves a ref against the local clone, or null when it cannot. - * - * A CI clone is typically shallow and checked out at the head commit only, so - * the base branch resolves neither bare nor as `origin/<ref>`; the fetch - * still succeeds where the clone has a credentialed remote (a developer - * machine, a GitHub Actions checkout). Null covers the rest — most notably a - * Vercel build, whose sources come as a snapshot with no fetchable clone. - */ -const resolveLocalCommit = (repoRoot: string, ref: string): string | null => { - const local = localShaOf(repoRoot, ref); - if (local !== null) { - return local; - } + const shaOf = (name: string): string | undefined => + lines.find(([, refName]) => refName === name)?.[0]; - try { - git(repoRoot, ["fetch", "--quiet", "--depth=1", "origin", ref]); - return git(repoRoot, [ - "rev-parse", - "--verify", - "--quiet", - "FETCH_HEAD^{commit}", - ]); + const sha = + shaOf(`refs/heads/${ref}`) ?? + shaOf(`refs/tags/${ref}^{}`) ?? + shaOf(`refs/tags/${ref}`) ?? + ""; + return isCommitSha(sha) ? sha : null; } catch { return null; } @@ -245,9 +246,7 @@ export const materializeBaseTree = async (options: { /** Repo-relative directories to extract; ones absent at the ref are skipped. */ paths: string[]; }): Promise<BaseTree> => { - // Refused rather than escaped: git would parse a leading dash as an option, - // in `rev-parse`, `fetch` and `archive` alike. - if (options.ref.startsWith("-")) { + if (!isUsableRef(options.ref)) { throw new Error(`\`${options.ref}\` is not a usable ref`); } @@ -258,7 +257,7 @@ export const materializeBaseTree = async (options: { const root = await realpath(await mkdtemp(join(tmpdir(), "arch-docs-base-"))); try { - const localSha = resolveLocalCommit(options.repoRoot, options.ref); + const localSha = localShaOf(options.repoRoot, options.ref); if (localSha !== null) { extractFromLocalClone({ From f87f1f056f99dc7ccc9dd1f31dc4256549a756bb Mon Sep 17 00:00:00 2001 From: Chris Feijoo <chris@kube.io> Date: Thu, 27 Aug 2026 16:59:01 +0200 Subject: [PATCH 10/11] FE-1514: Let CI relocate the base cache The default stays node_modules/.cache, which CI providers persist without configuration and local builds can always write; a CI whose persisted location differs sets PETRINAUT_ARCH_DOCS_CACHE_DIR instead of the code guessing its layout. --- libs/@local/petrinaut-arch-docs/README.md | 4 +++- .../petrinaut-arch-docs/src/diff/base-side.ts | 20 +++++++++++++++---- libs/@local/petrinaut-arch-docs/turbo.json | 1 + 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/libs/@local/petrinaut-arch-docs/README.md b/libs/@local/petrinaut-arch-docs/README.md index ca4cad678c6..a22d021a01f 100644 --- a/libs/@local/petrinaut-arch-docs/README.md +++ b/libs/@local/petrinaut-arch-docs/README.md @@ -231,7 +231,9 @@ otherwise) into a scratch repository: a blobless fetch plus a sparse checkout of only the covered directories, so the transfer stays small. The build log says `base tree fetched from …` when this path ran. -The built base side is cached under `node_modules/.cache/petrinaut-arch-docs`, +The built base side is cached under `node_modules/.cache/petrinaut-arch-docs` +(`PETRINAUT_ARCH_DOCS_CACHE_DIR` relocates it, for a CI whose persisted +location differs), keyed in the entry's file name by a hash of the generator's own sources, config and pinned dependencies, then the base commit — CI providers persist that directory between builds, so any later commit whose base has not moved diff --git a/libs/@local/petrinaut-arch-docs/src/diff/base-side.ts b/libs/@local/petrinaut-arch-docs/src/diff/base-side.ts index 248d10773c1..ec713ee2ccb 100644 --- a/libs/@local/petrinaut-arch-docs/src/diff/base-side.ts +++ b/libs/@local/petrinaut-arch-docs/src/diff/base-side.ts @@ -10,7 +10,7 @@ import { execFileSync } from "node:child_process"; import { existsSync } from "node:fs"; -import { join } from "node:path"; +import { isAbsolute, join } from "node:path"; import { config } from "../../architecture.config"; import { buildBundle, type BuiltBundle } from "../build"; @@ -38,9 +38,21 @@ export const coveredPaths = (): string[] => [ ...new Set([...config.packages.map((pkg) => pkg.path), GENERATOR_PACKAGE]), ]; -/** Where cache entries live; `node_modules/.cache` is what CI persists. */ -const baseCacheDir = (repoRoot: string): string => - join(repoRoot, "node_modules/.cache/petrinaut-arch-docs"); +/** + * Where cache entries live. The default is `node_modules/.cache`, which CI + * providers persist without configuration and local builds can always write; + * a CI whose persisted location differs sets `PETRINAUT_ARCH_DOCS_CACHE_DIR` + * rather than the code guessing its layout. A relative override resolves + * against the repository root. Whoever controls the variable controls what + * the diff trusts as its base, which is the standing for any CI-set variable. + */ +const baseCacheDir = (repoRoot: string): string => { + const override = process.env.PETRINAUT_ARCH_DOCS_CACHE_DIR?.trim(); + if (override === undefined || override === "") { + return join(repoRoot, "node_modules/.cache/petrinaut-arch-docs"); + } + return isAbsolute(override) ? override : join(repoRoot, override); +}; const inputsHashFor = (repoRoot: string, includeDiagrams: boolean): string => generatorInputsHash({ diff --git a/libs/@local/petrinaut-arch-docs/turbo.json b/libs/@local/petrinaut-arch-docs/turbo.json index 4ee54f83906..5d800233f1e 100644 --- a/libs/@local/petrinaut-arch-docs/turbo.json +++ b/libs/@local/petrinaut-arch-docs/turbo.json @@ -25,6 +25,7 @@ "env": [ "PETRINAUT_ARCH_DOCS_SOURCE_REF", "PETRINAUT_ARCH_DOCS_DIFF_BASE", + "PETRINAUT_ARCH_DOCS_CACHE_DIR", "VERCEL_GIT_COMMIT_SHA", "GITHUB_SHA", "VERCEL_GIT_COMMIT_REF", From 3a85f2342ac714723900e12f0aa3812bc39b36ee Mon Sep 17 00:00:00 2001 From: Chris Feijoo <chris@kube.io> Date: Thu, 27 Aug 2026 17:18:08 +0200 Subject: [PATCH 11/11] FE-1514: Apply verification-review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two of these were invisible on GitHub: base-cache.ts carried raw NUL bytes as hash separators, so git rendered it as a binary file — now the escape spelling, with identical hash input — and the ref guard's control-character class tripped oxlint's no-control-regex; it is now an allow-list, which also subsumes the empty and leading-dash checks. The prune could evict exactly the shared base entry: eviction now orders by last use (a hit refreshes the timestamp), the cap rises to 16, and seeding runs after the diff has read its base. Cache keys hash package-relative paths, so relocated checkouts sharing a cache directory hit. Seeding is wrapped whole — no failure in a bonus write may fail a passed build. Network git calls get a timeout and prompt suppression; the marker stylesheet reads every hue through its custom property so re-theming works as documented; empty Vercel variables no longer produce empty header chips. --- apps/petrinaut-docs/src/diff-context.ts | 18 +++++--- libs/@local/petrinaut-arch-docs/src/cli.ts | 18 ++++---- .../src/diff/base-cache.ts | Bin 4970 -> 5760 bytes .../petrinaut-arch-docs/src/diff/base-side.ts | 41 +++++++++++------- .../petrinaut-arch-docs/src/diff/base-tree.ts | 17 +++++--- .../src/emit/components/diff-marker.css | 37 ++++++++++------ 6 files changed, 82 insertions(+), 49 deletions(-) diff --git a/apps/petrinaut-docs/src/diff-context.ts b/apps/petrinaut-docs/src/diff-context.ts index 58c07ac58f4..df45b28016f 100644 --- a/apps/petrinaut-docs/src/diff-context.ts +++ b/apps/petrinaut-docs/src/diff-context.ts @@ -13,7 +13,7 @@ * pull requests is GitHub knowledge the generator deliberately does not have. */ -import { execSync } from "node:child_process"; +import { execFileSync } from "node:child_process"; import type { BundleManifest } from "@local/petrinaut-arch-docs"; @@ -33,14 +33,20 @@ export interface DiffCompareContext { const wellFormed = /^[\w.-]+$/u; -const gitFallback = (args: string): string | null => { +const gitFallback = (args: string[]): string | null => { try { - return execSync(`git ${args}`, { encoding: "utf8" }).trim() || null; + return execFileSync("git", args, { encoding: "utf8" }).trim() || null; } catch { return null; } }; +/** A trimmed, non-empty environment value, or null. */ +const envString = (value: string | undefined): string | null => { + const trimmed = value?.trim() ?? ""; + return trimmed === "" ? null : trimmed; +}; + /** The one open PR whose head is `branch`, or null (anonymous API, best effort). */ const openPrNumberFor = async ( owner: string, @@ -114,8 +120,10 @@ export const resolveDiffCompareContext = async ( const repoUrl = `https://github.com/${owner}/${slug}`; const headBranch = - env.VERCEL_GIT_COMMIT_REF ?? gitFallback("rev-parse --abbrev-ref HEAD"); - const headSha = env.VERCEL_GIT_COMMIT_SHA ?? gitFallback("rev-parse HEAD"); + envString(env.VERCEL_GIT_COMMIT_REF) ?? + gitFallback(["rev-parse", "--abbrev-ref", "HEAD"]); + const headSha = + envString(env.VERCEL_GIT_COMMIT_SHA) ?? gitFallback(["rev-parse", "HEAD"]); const headPr = /^\d+$/u.test(env.VERCEL_GIT_PULL_REQUEST_ID ?? "") ? env.VERCEL_GIT_PULL_REQUEST_ID! : null; diff --git a/libs/@local/petrinaut-arch-docs/src/cli.ts b/libs/@local/petrinaut-arch-docs/src/cli.ts index e504f72c9bc..a05a933a1b7 100644 --- a/libs/@local/petrinaut-arch-docs/src/cli.ts +++ b/libs/@local/petrinaut-arch-docs/src/cli.ts @@ -240,14 +240,6 @@ const main = async (): Promise<number> => { return 1; } - // Every clean build stores its own side as a future diff base — a `main` - // build's entry is what spares each PR from rebuilding the base itself. - seedBaseCacheWithSelf({ - repoRoot, - bundle, - includeDiagrams: diagramsAvailable, - }); - // Applied only to a bundle that already passed the checks: the diff decorates // the output, it never gates it. const diffBase = resolveDiffBase(process.argv.slice(3)); @@ -256,6 +248,16 @@ const main = async (): Promise<number> => { ? bundle : await withDiffAgainst(bundle, diffBase, diagramsAvailable); + // Every clean build stores its own side as a future diff base — a `main` + // build's entry is what spares each PR from rebuilding the base itself. + // Seeded after the diff has read its base, so at the cache's capacity this + // write can never evict the entry the diff was about to use. + seedBaseCacheWithSelf({ + repoRoot, + bundle, + includeDiagrams: diagramsAvailable, + }); + if (!(await writeBundle(written, diagramsAvailable))) { return 1; } diff --git a/libs/@local/petrinaut-arch-docs/src/diff/base-cache.ts b/libs/@local/petrinaut-arch-docs/src/diff/base-cache.ts index 16da90ebd81cbd632877be0b4a14692f13654e78..1c20d8b9d27d85f8c9d55a2204b640afaaa67f24 100644 GIT binary patch delta 845 zcmZXSzlsz=5XNC~H`tRy!Nja0_fGbo2nQ+#ieey$h~XtYGqu~z_D+x8H8+f+d$F00 z4<hUf_!1YG8j26$*E754ft?I=*Zk_M@2lT;&L`hb`-4)<nnUxpe=;1CXRE}Var3#K z_J-8_>5n#+RS>+SoQtig44l!zc-;Lu|3-?mks;kh!NPOU97yGaQeTe(wQN!AVobr2 zXr`c*2Ov>_yDH){_B9n{&|rWbBF&Xzp#y{sesVcaf_6fIISW*IBf>c)98AV!WJ=G` z(4gFxIeof6pVP~y`<4-xs)7uZCE$|frAdr6Qj=S>qB4W=#QAzmdF*1|^Ax|$hN;V& z(cot9-pTgCpz3dCGkUGkm0I#-Z&lZ(a+GRLSKx;|HK;Wc2#^Y9oEvVfxQ@OJ2?sQ- zf;RcGRiTpLY$;5I5>1sWZAR}(!_oDO7p&w30O;#dCGap+XbGJ-5i>1|4EINSVMneI z<MJ>qI>m3ClQELBeBAsvxYawIHx~yF@!8(Ee$m??F#6EKT47IOu2({h*<GgD$}QkE zqVXB~KnbkdCERA;rTPzdCEgfBq}jMu;%Mo}D_LmvxW?|ptx9-rinAG`E=JCm#<dC~ zqus;K0*Y3nGItd{rkpE!7Tq|0v=h<o`PakSuTT+lu69o!VWgz`|KXuV@RrAwNHD!C U;dd702Qk)mt3TcTJp6L)FD*A6uK)l5 delta 117 zcmZqBeWkV`hIMiltKQ^AtjUu(*wobWb5c@^OBB*Fi;7G16twiYlJoO`d|$_icvruW zAWzp|1zQD^&Gzi&tg0D_#Tj~~1u2OosTygSIjNe~Tna#dEUd&Zc@nP`Bg5u%yjPhv J{}yaz0RXpXCBgsz diff --git a/libs/@local/petrinaut-arch-docs/src/diff/base-side.ts b/libs/@local/petrinaut-arch-docs/src/diff/base-side.ts index ec713ee2ccb..737edf1088c 100644 --- a/libs/@local/petrinaut-arch-docs/src/diff/base-side.ts +++ b/libs/@local/petrinaut-arch-docs/src/diff/base-side.ts @@ -31,11 +31,17 @@ const GENERATOR_PACKAGE = "libs/@local/petrinaut-arch-docs"; /** * Repo-relative directories whose content the generator reads: the covered - * packages, plus this package for the authored content and the - * dependency-cruiser tsconfig. + * packages, the authored content, and this package for the dependency-cruiser + * tsconfig. The content directory is listed from config even though it sits + * inside this package today — were it moved, the base side would otherwise + * silently lose every authored page. */ export const coveredPaths = (): string[] => [ - ...new Set([...config.packages.map((pkg) => pkg.path), GENERATOR_PACKAGE]), + ...new Set([ + ...config.packages.map((pkg) => pkg.path), + config.contentDirectory, + GENERATOR_PACKAGE, + ]), ]; /** @@ -145,11 +151,14 @@ export const seedBaseCacheWithSelf = (options: { bundle: BuiltBundle; includeDiagrams: boolean; }): void => { - const { repoRoot } = options; - let sha = process.env.VERCEL_GIT_COMMIT_SHA ?? ""; + // One try around everything: seeding is a bonus, and no failure in it — a + // git oddity, a file swapped out mid-hash — may fail a build that already + // passed its checks. + try { + const { repoRoot } = options; + let sha = process.env.VERCEL_GIT_COMMIT_SHA ?? ""; - if (!/^[0-9a-f]{40}$/u.test(sha)) { - try { + if (!/^[0-9a-f]{40}$/u.test(sha)) { sha = execFileSync("git", ["rev-parse", "HEAD"], { cwd: repoRoot, encoding: "utf8", @@ -162,15 +171,15 @@ export const seedBaseCacheWithSelf = (options: { if (!/^[0-9a-f]{40}$/u.test(sha) || dirty !== "") { return; } - } catch { - return; } - } - writeCachedBaseSide({ - cacheDir: baseCacheDir(repoRoot), - sha, - inputsHash: inputsHashFor(repoRoot, options.includeDiagrams), - side: diffSideOfBundle(options.bundle, config.sourceUrlPrefix), - }); + writeCachedBaseSide({ + cacheDir: baseCacheDir(repoRoot), + sha, + inputsHash: inputsHashFor(repoRoot, options.includeDiagrams), + side: diffSideOfBundle(options.bundle, config.sourceUrlPrefix), + }); + } catch { + return; + } }; diff --git a/libs/@local/petrinaut-arch-docs/src/diff/base-tree.ts b/libs/@local/petrinaut-arch-docs/src/diff/base-tree.ts index 3f4f64b320f..7f319cb1454 100644 --- a/libs/@local/petrinaut-arch-docs/src/diff/base-tree.ts +++ b/libs/@local/petrinaut-arch-docs/src/diff/base-tree.ts @@ -44,16 +44,21 @@ const git = (cwd: string, args: string[]): string => cwd, encoding: "utf8", stdio: ["ignore", "pipe", "pipe"], + // A hung network call must degrade the diff, not stall the build until + // the CI-wide timeout; suppressing the prompt keeps a credential dialog + // from ever holding a connection open. + timeout: 120_000, + env: { ...process.env, GIT_TERMINAL_PROMPT: "0" }, }).trim(); /** - * Whether a ref can safely appear on a git command line: a leading dash - * parses as an option, and whitespace or control characters split arguments. - * Checked at every entry point that takes a ref, so no call order lets an - * unvalidated ref reach a subprocess. + * Whether a ref can safely appear on a git command line. An allow-list of + * word characters plus the separators and rev syntax refs use (`origin/main`, + * `v1.2.3`, `main~3`, `HEAD^`, `v1^{}`), anchored on a word character so + * nothing option-shaped passes. Checked at every entry point that takes a + * ref, so no call order lets an unvalidated ref reach a subprocess. */ -const isUsableRef = (ref: string): boolean => - ref !== "" && !ref.startsWith("-") && !/[\s\u0000-\u001f]/u.test(ref); +const isUsableRef = (ref: string): boolean => /^[\w][\w.@+/~^{}-]*$/u.test(ref); const isCommitSha = (text: string): boolean => /^[0-9a-f]{40}$/u.test(text); diff --git a/libs/@local/petrinaut-arch-docs/src/emit/components/diff-marker.css b/libs/@local/petrinaut-arch-docs/src/emit/components/diff-marker.css index 457cbc70825..93cd65e5c82 100644 --- a/libs/@local/petrinaut-arch-docs/src/emit/components/diff-marker.css +++ b/libs/@local/petrinaut-arch-docs/src/emit/components/diff-marker.css @@ -1,15 +1,14 @@ /* * Styling for diff markers on pages built against a base ref. * - * Self-contained plain CSS like the layer cards: the three status hues are - * fixed because green/blue/red must stay green/blue/red on any theme, while - * everything structural derives from `currentColor`. The marker itself is - * invisible; the adjacent-sibling selector paints the block it precedes, so - * the marked markdown needs no wrapper of its own. + * Self-contained plain CSS like the layer cards. Every status hue is read + * through a custom property with a fixed fallback — green, blue and red must + * stay themselves on any theme, and a host re-themes them by defining the + * variables on an ancestor. The marker itself is invisible; the + * adjacent-sibling selector paints the block it precedes, so the marked + * markdown needs no wrapper of its own. */ -/* The status hues are read with a fallback so a host may re-theme them by - defining the custom properties on an ancestor (e.g. `:root`). */ .arch-diff-marker { display: none; } @@ -18,29 +17,39 @@ border-inline-start: 3px solid var(--arch-diff-added, #2f9e44); padding-block: 0.25rem; padding-inline-start: 0.75rem; - background: color-mix(in srgb, #2f9e44 7%, transparent); + background: color-mix( + in srgb, + var(--arch-diff-added, #2f9e44) 7%, + transparent + ); } .arch-diff-marker[data-status="changed"] + * { border-inline-start: 3px solid var(--arch-diff-changed, #3676b8); padding-block: 0.25rem; padding-inline-start: 0.75rem; - background: color-mix(in srgb, #3676b8 7%, transparent); + background: color-mix( + in srgb, + var(--arch-diff-changed, #3676b8) 7%, + transparent + ); } .arch-diff-removed { - --arch-diff-removed: #c92a2a; - margin: 1rem 0; padding: 0.25rem 0.75rem; - border-inline-start: 3px solid var(--arch-diff-removed); - background: color-mix(in srgb, #c92a2a 7%, transparent); + border-inline-start: 3px solid var(--arch-diff-removed, #c92a2a); + background: color-mix( + in srgb, + var(--arch-diff-removed, #c92a2a) 7%, + transparent + ); font-size: 0.85rem; } .arch-diff-removed > summary { cursor: pointer; - color: var(--arch-diff-removed); + color: var(--arch-diff-removed, #c92a2a); font-weight: 600; }