feat(cfl): INT-725 verify page writes by reading back what was stored

[piekstra]

[INT-725]

cfl page edit reported success from the update response alone. For an exact body format cfl transmits the caller's content verbatim (bodyForInput passes the string through with no re-serialization), so when Confluence stores something different, nothing surfaced it.

The failure this came from

Editing a real page with --body-format adf silently lost __confluenceMetadata on eight link marks. cfl sent them correctly; Confluence dropped them server-side. The command printed Version: 6 and exited zero.

It was only found later by re-fetching and diffing by hand — by which point those intra-page anchor links (see also A11) had degraded to full expanded URLs. Worth noting the loss is not recoverable through the API: sending the attributes back gets them stripped again, so the damage outlives the discovery.

What this adds

After a successful write with --body-format adf or xhtml, re-read the page in that format and compare what was stored against what was sent.

The two kinds of difference are reported separately because they carry different consequences:

Difference	Meaning	Result
Text content differs	the caller's content did not land	error, non-zero exit
Attributes differ	server normalized around content that survived	reported, tolerated

Conflating them would either cry wolf on every normalized write or hide a real one.

Markdown is converted to ADF before sending, so the stored body isn't comparable to what was supplied — verification is skipped there rather than reporting a false mismatch. An unparseable body is an error, not a pass: an unverifiable write is not a verified one.

--no-verify opts out of the extra read.

Note on the default

This is on by default for exact formats, which is a behavior change: one extra GET per write, and a new way for the command to exit non-zero. I think that's right — verification you have to remember to ask for is the failure mode this is fixing — but it's your call and easy to flip to opt-in.

Test doubles

Six edit tests failed immediately on this change, which was itself informative: their fakes returned the original body on every GET regardless of what was PUT, so verification correctly saw drift on writes that had succeeded. Real Confluence returns what it stored, so mockEditBodyServer and five inline handlers now do too. They previously asserted nothing about the server's response.

Validation

• make test and make lint green across all three modules (0 issues).
• Unit tests cover: identical documents, silent text change, dropped attribute with text intact, text reordering, XHTML markup rewritten vs text lost, markdown rejection, unparseable body, and that the report names the dropped attribute and locates the first text difference.
• Exercised against live Confluence: a re-save of an unchanged page passes clean with no false positive.

[monit-reviewer]

Automated PR Review

Reviewed commit: c78535ee89c0
Profile: claude-monit-reviewer - Posting as: monit-reviewer

Summary

Reviewer	Findings
go:implementation-tests	1
policies:conventions	0
architecture:solid-reviewer-agnostic	3
structure:harness-engineering	1

go:implementation-tests (1 finding)

Major - tools/cfl/internal/cmd/page/edit.go:233

verifyStoredBody is the new integration point that turns a drift finding into actual command behavior (non-zero exit on TextChanged, a stderr warning on dropped/added attrs, an extra GET, --no-verify bypass), but no test exercises it through runEdit. verify_test.go only unit-tests the pure helpers (compareStoredBody, describeDrift) with hand-built strings; edit_test.go's changes are limited to making existing fakes echo back the PUT body (storedPageJSON) so the new verify GET doesn't spuriously fail those unrelated tests — every one of them therefore hits the Clean() early-return and never touches the error-returning or warning-emitting branches. Nothing asserts: (1) runEdit returns a non-nil error and the correct message when the readback shows text loss, (2) runEdit succeeds (exit 0) while still emitting the stderr warning when only attributes were dropped, (3) --no-verify actually skips the extra GET, or (4) a failure/empty-body response from the verify GET is surfaced as the wrapped 'verifying stored page' error. Given this is the exact regression class the PR exists to catch (silent success on a lossy write), add at least one runEdit-level test with a fake server whose GET deliberately returns a body that differs from the PUT payload (drop an attr, or change the text) to prove the command wiring — not just the diff algorithm — reports it correctly, plus one asserting --no-verify suppresses the read entirely (e.g. assert GET is not called a second time).

architecture:solid-reviewer-agnostic (3 findings)

Major - tools/cfl/internal/cmd/page/verify.go:302

Presentation ownership is inverted here: the command package composes the user-facing wording (describeDrift, verify.go:193-219, returning []string of finished display text) and then constructs the presenter-owned DTO itself — stderrLines builds an OutputModel with a MessageSection, its Kind, and its Stream.

This is exactly what ARCHITECTURE.md forbids for new rendering work: hard rule 1 ("Do not build user-facing strings in commands"), hard rule 5 ("Do not encode presentation intent as [][]string/[]string in new code"), and hard rule 7 ("Do not make commands construct presenter-owned DTOs, fields, rows, labels, or section ordering"). tools/cfl/internal/present/README.md repeats it verbatim for cfl and assigns "empty-state messages… mutation wording… stream destination for messages, warnings, and diagnostics" to presenters.

The house pattern already exists two files away and covers the identical case: the legacy-editor advisory in PagePresenter.PresentEdit (internal/present/mutation.go:38-47) is a stderr MessageWarning owned by the presenter. verify.go is currently the only production file under internal/cmd that constructs an OutputModel (rg 'OutputModel' internal/cmd --glob '!**/*_test.go' returns only verify.go:302-303), so this is a fresh drift, not an existing convention. The AST gate in internal/cmd/root/presenter_boundary_test.go only catches direct writes and legacy v.* helpers, so it passes — the rule is doc-enforced at this seam, which is why it needs to be caught in review.

Suggested fix: keep the comparison and the Emit call in verify.go, and move the wording/stream/section decisions into internal/present as something like func (PagePresenter) PresentWriteDrift(d writeDrift, bodyFormat string) *sharedpresent.OutputModel (with writeDrift — or a small presentation-facing projection of it — passed in as the domain value). That also lets the report be pinned with an exact-OutputModel presenter test; the current coverage asserts wording via strings.Contains (verify_test.go:77-83, 159-165), which ARCHITECTURE.md lists under "Review smells".

Minor - tools/cfl/internal/cmd/page/verify.go:207

The tolerated/error split is documented as "losing attributes means the document was normalized around content that did [survive]" (verify.go:26-27, and the printed line "Text content is intact"), but the detector behind it cannot support that claim for attribute-bearing leaf nodes. adfText only concatenates type == "text" nodes, so if Confluence drops an entire inlineCard, media, mention, emoji, or status node, the text comparison is unchanged and the loss shows up only as media.attrs.id (1→0) in DroppedAttrs — i.e. real content loss is reported under the heading that asserts content survived, and the command exits zero (verify.go:288-299).

That is a documented-semantics mismatch (U-L1) in the contract this PR is establishing, and it is the same failure shape the PR exists to catch — an operator reading "Text content is intact" will stop looking.

Suggested fix: profile node type occurrences alongside the attribute profile (the walk in adfAttrProfile already visits every node, so it is a second counter, not a second traversal), and treat a decrease in non-text node counts as content loss rather than normalization. If classifying node drops as an error is too aggressive for now, at minimum stop asserting "Text content is intact" when a node type disappeared, and say the document lost nodes.

Minor - tools/cfl/internal/cmd/page/edit.go:114

New user-facing surface without the matching documented contract (U-G1). --no-verify is registered here and explained in the command's Long help, but two repo-designated contract documents are not updated:

• tools/cfl/README.md documents every cfl page edit flag in a table (--title, --parent, --file, --editor, --body-format, --legacy); --no-verify is missing, so the only place a user learns the default-on readback exists is --help.
• tools/cfl/internal/cmd/OUTPUT_SPEC.md:248-257 states the page edit contract as a success block only. This PR adds a second output shape for that command (a multi-line stderr drift report) and a new non-zero exit path after success output has already been emitted. OUTPUT_SPEC calls itself "the authoritative declaration of the target cfl output contract" and does spec stderr shapes elsewhere (empty-result prose lines, pagination hints), so a new stderr shape belongs in it.

Suggested fix: add the flag row to the README page edit table and a short stderr/exit-status subsection to the page edit entry in OUTPUT_SPEC.md, in this PR, since the on-by-default behavior change is the part users most need documented.

structure:harness-engineering (1 finding)

Major - tools/cfl/internal/cmd/page/verify.go:249

verifyRequest's doc comment claims "edit and create share one verification path," but page create never calls verifyStoredBody — grep confirms the only caller is edit.go. create.go builds ADF/XHTML bodies through the same verbatim bodyForInput path (create.go:131) that motivated this PR (the INT-725 incident was a silent server-side attribute drop on a write reported as success), so a cfl page create --body-format adf can suffer the identical unseen corruption this PR was written to catch, with no --no-verify escape hatch to even acknowledge the gap. Either wire verifyStoredBody into create.go now (the comment already asserts the path is shared) or correct the comment to state create is intentionally out of scope and left for a follow-up, so the asymmetry isn't discovered the same way the original bug was — by hand, after data loss.

Reviewer Coverage

• go:implementation-tests — complete (broad); skipped: none; constraints: none
• policies:conventions — complete (broad); inspected 2 assigned files (4 inspected across reviewers): tools/cfl/internal/cmd/page/edit.go, tools/cfl/internal/cmd/page/verify.go; skipped: none; constraints: Sibling checkouts for shared standards (../cli-common/docs, ../.github) are not present in this workbench, so cross-repo shared conventions could not be verified beyond what's linked from tools/cfl/CLAUDE.md.
• architecture:solid-reviewer-agnostic — complete (broad); inspected 2 assigned files (4 inspected across reviewers): tools/cfl/internal/cmd/page/edit.go, tools/cfl/internal/cmd/page/verify.go; skipped: none; constraints: Scope limited to the two assigned files; edit_test.go and verify_test.go were read for context only and are not reported on. Verification run: go test ./internal/cmd/page/... ./internal/cmd/root/... in tools/cfl — both packages pass, including the presenter-boundary gate in internal/cmd/root.
• structure:harness-engineering — complete (broad); inspected 2 assigned files (4 inspected across reviewers): tools/cfl/internal/cmd/page/edit.go, tools/cfl/internal/cmd/page/verify.go; skipped: none; constraints: none

Inspected files (4)

• tools/cfl/internal/cmd/page/edit.go
• tools/cfl/internal/cmd/page/edit_test.go
• tools/cfl/internal/cmd/page/verify.go
• tools/cfl/internal/cmd/page/verify_test.go

0 PR discussion threads considered. 0 summarized; 0 resolved.

---

Completed in 6m 31s | ~$5.22 (est.) | claude-sonnet-5, claude-opus-5 | cr 0.10.286

Field	Value
Model	claude-sonnet-5, claude-opus-5
Reviewers	go:implementation-tests, policies:conventions, architecture:solid-reviewer-agnostic, structure:harness-engineering
Engine	claude_cli · claude-sonnet-5, claude-opus-5
Reviewed by	cr · monit-reviewer
Duration	6m 31s wall · 13m 40s compute
Cost	~$5.22 (est.)
Tokens	141 in / 38.9k out

Per-workstream usage

• orchestrator-selection — claude-sonnet-5
  • In: 4
  • Out: 4.5k
  • Cache read: 31.8k
  • Cache create: 76.9k
  • Cost: ~$0.37 (est.)
  • Duration: 52s
• go:implementation-tests — claude-sonnet-5
  • In: 42
  • Out: 7.9k
  • Cache read: 1.6M
  • Cache create: 99.3k
  • Cost: ~$0.97 (est.)
  • Duration: 2m 46s
• policies:conventions — claude-sonnet-5
  • In: 20
  • Out: 4.0k
  • Cache read: 670.8k
  • Cache create: 93.4k
  • Cost: ~$0.61 (est.)
  • Duration: 1m 48s
• architecture:solid-reviewer-agnostic — claude-opus-5
  • In: 49
  • Out: 16.7k
  • Cache read: 2.1M
  • Cache create: 123.7k
  • Cost: ~$2.24 (est.)
  • Duration: 4m 54s
• structure:harness-engineering — claude-sonnet-5
  • In: 20
  • Out: 3.6k
  • Cache read: 641.4k
  • Cache create: 87.1k
  • Cost: ~$0.57 (est.)
  • Duration: 2m 45s
• orchestrator-rollup — claude-sonnet-5
  • In: 6
  • Out: 2.2k
  • Cache read: 112.7k
  • Cache create: 105.9k
  • Cost: ~$0.46 (est.)
  • Duration: 32s

[piekstra]

All five taken, in fd33305.

The atom-loss finding was the important one, and it was a real hole. adfText only concatenated type == "text", so a dropped inlineCard/media/mention/emoji/status left the text identical and surfaced only as media.attrs.id (1→0) — classified as normalization, the one bucket that tells the operator to ignore it. Silent content loss reported as benign is worse than no check.

Replaced with a content fingerprint: text as itself, plus a marker per content-bearing atom including its identifying attrs, so substitution is caught too — swapping one card's URL for another keeps every count identical and would otherwise have passed. Pinned by TestContentAtomLossCountsAsContentChange and TestContentAtomSubstitutionDetected.

Presentation ownership — right, this violated hard rules 1 and 5. describeDrift and stderrLines are gone from the command package. Commands now hand a present.WriteDrift finding to PagePresenter.PresentWriteDrift, which owns the wording and the OutputModel. The unit tests assert against the presenter's output rather than hand-built strings, so they check what an operator actually reads.

page create was unverified while my comment claimed edit and create shared the path. The comment was the lie, the gap was real — create.go builds bodies through the same verbatim bodyForInput. Wired it in rather than softening the comment.

Integration tests through runEdit — content drift fails, --no-verify skips both the check and the extra GET (asserted on request count), normalization is tolerated, and markdown isn't verified at all.

Worth noting the create fakes had no GET-by-id branch, so a create that succeeded answered 404 on readback. Same shape as the edit fakes: they asserted nothing about what the server returns. Both now report the stored body.

Docs — --no-verify added to both README flag tables, plus a paragraph on the readback; OUTPUT_SPEC.md documents the stderr blocks for both the tolerated and failing cases.

make test (61 packages) and make lint (all three modules) clean.

[monit-reviewer]

Automated PR Review

Reviewed commit: fd33305c7974
Profile: claude-monit-reviewer - Posting as: monit-reviewer

Summary

Reviewer	Findings
go:implementation-tests	1
policies:conventions	0
architecture:solid-reviewer-agnostic	4
structure:harness-engineering	0

go:implementation-tests (1 finding)

Minor - tools/cfl/internal/cmd/page/create.go:178

runCreate now wires the same verifyStoredBody path as runEdit (page.ID from the create response, sentContent, opts.noVerify), but only edit got runCommand-level integration tests for it: verify_test.go added TestRunEditFailsWhenStoredContentDiffers, TestRunEditNoVerifySkipsReadback, TestRunEditToleratesNormalization, and TestRunEditSkipsVerificationForMarkdown, all driving runEdit through a fake server. create_test.go's only change is fixture plumbing (storedCreatedPageJSON added to GET handlers so existing tests don't 404 on the new readback) — nothing exercises runCreate itself failing on stored-content drift, tolerating attribute normalization, respecting --no-verify, or skipping verification for markdown. Since compareStoredBody/verifyStoredBody are shared and already well covered, the residual risk is narrower than edit's was, but it's the same wiring (page.ID sourcing, enabled flag, sentContent capture) newly introduced in this file and it is currently proven only by inspection. Add at least a TestRunCreateFailsWhenStoredContentDiffers analogous to the edit case (reuse driftServer's pattern against the create fixture's GET/POST server) so create's own enable/disable and error-propagation logic is verified, not just inferred from edit's coverage.

architecture:solid-reviewer-agnostic (4 findings)

Minor - tools/cfl/internal/cmd/OUTPUT_SPEC.md:259

The readback contract is spec'd under page edit only, but this revision wires the identical behavior into page create (create.go:176-192: same stderr drift block, same non-zero exit on content loss, same --no-verify). The page create section at OUTPUT_SPEC.md:238-246 still declares a success block as the whole contract, so the document that calls itself "the authoritative declaration of the target cfl output contract" is now incomplete for the command that just gained the surface (U-G1: new public surface, documented contract).

A third shape emitted by the presenter is also unspec'd: Confluence added attributes that were not sent: followed by + <node>.attrs.<name> (<before>→<after>) (mutation.go:158-163).

Suggested fix: add one cross-reference line under page create ("With --body-format adf or xhtml the readback contract described under page edit applies identically") rather than duplicating the blocks, and add the added-attributes block to the page edit section alongside the dropped-attributes one.

Minor - tools/cfl/internal/cmd/page/verify.go:328

The wording move to PagePresenter.PresentWriteDrift fixed the bulk of the presenter-boundary problem, but two user-facing values are still composed here from the internal fingerprint, and the fingerprint is not something an operator should ever see.

firstTextDifference returns finished prose (at offset %d — sent %q, stored %q) that the presenter splices in verbatim (mutation.go:152), and SentLen/StoredLen (verify.go:281-282) are lengths of the fingerprint string, not of the document's content. Reproduced on a 7-character paragraph containing one inlineCard:

sentLen=46 storedLen=7
diff=at offset 4 — sent "\x00inlineCard(url=https://example.test/x) ", stored " ok"

So the operator is told "content differs (46 chars sent, 7 stored)" for a document of seven characters, and the excerpt shows a NUL escape and the internal marker syntax. tools/cfl/internal/cmd/OUTPUT_SPEC.md:271 documents that number as <n> chars, which it no longer is. This is ARCHITECTURE.md hard rule 1 ("Do not build user-facing strings in commands") and hard rule 3 ("Do not normalize… display content in the renderer if that decision belongs to presentation logic") applied to the residue: the command owns a display sentence and leaks a comparison artifact into it.

Suggested fix: keep the fingerprint internal and hand the presenter structured facts — e.g. DiffOffset int, SentExcerpt/StoredExcerpt derived from the text (or an atom description such as inlineCard url=… when the divergence is at an atom marker), and counts taken from the text portion only — letting PresentWriteDrift phrase them. Alternatively drop the char counts when the fingerprint contains atom markers rather than reporting a number that means nothing to the reader.

Minor - tools/cfl/internal/cmd/page/verify.go:89

The atom fingerprint is an allowlist, so it fails open on the node types it does not name — and the doc comment above it makes a stronger claim than the code supports: "Two documents with the same fingerprint hold the same content for a reader" (verify.go:106). hardBreak and rule are attribute-less atoms, so a server-side drop moves neither the text (no text child) nor the attribute profile (no attrs), and the write is reported as verified.

Reproduced against this checkout with a scratch test calling compareStoredBody:

hardBreak drop: clean=true textChanged=false dropped=[]
rule drop:      clean=true

That is a silent pass on real content loss (page edit exits zero and prints nothing) in the feature whose purpose is to make silent loss visible — U-L1, documented semantics not honored by the implementation. The same hole covers any atom type Atlassian adds later, since a new type is invisible until someone remembers to edit this map.

Suggested fix: make the fingerprint fail closed instead of enumerating types — in adfContent, emit a marker for any node that has neither a text value nor a content array (i.e. any non-text leaf), keeping atomIdentity for the attrs it does carry. That covers hardBreak, rule, and unknown future atoms without reacting to container restructuring, and lets contentAtoms go away. If the allowlist is preferred, add hardBreak and rule and soften the comment at verify.go:104-106 to say which node classes the fingerprint covers.

Nits - tools/cfl/README.md:444

The Writes are read back. paragraph — the one that states the non-zero exit and the markdown skip — lives only in the cfl page create section (README.md:355). The cfl page edit section (README.md:395-449) gets the flag row and nothing else, so a reader who lands on page edit (the command the INT-725 incident came from) learns the behavior exists but not that it can fail the command. One sentence or a pointer back to the create section closes it (U-G1).

Reviewer Coverage

• go:implementation-tests — complete (constrained); inspected 7 assigned files (9 inspected across reviewers): tools/cfl/internal/cmd/page/create.go, tools/cfl/internal/cmd/page/create_test.go, tools/cfl/internal/cmd/page/edit.go, tools/cfl/internal/cmd/page/edit_test.go, tools/cfl/internal/cmd/page/verify.go, tools/cfl/internal/cmd/page/verify_test.go, tools/cfl/internal/present/mutation.go; skipped: none; constraints: none
• policies:conventions — complete (constrained); inspected 2 assigned files (9 inspected across reviewers): tools/cfl/internal/cmd/page/edit.go, tools/cfl/internal/cmd/page/verify.go; skipped: none; constraints: Sibling checkouts for shared standards (../cli-common/docs, ../.github) are not present in this workbench, so cross-repo shared conventions could not be verified beyond what's linked from tools/cfl/CLAUDE.md. This pass reviews only the assigned files (edit.go, verify.go); the doc updates (README.md, OUTPUT_SPEC.md) and test files that resolved prior findings are out of scope here but were consulted for context.
• architecture:solid-reviewer-agnostic — complete (constrained); inspected 4 assigned files (9 inspected across reviewers): tools/cfl/README.md, tools/cfl/internal/cmd/OUTPUT_SPEC.md, tools/cfl/internal/cmd/page/edit.go, tools/cfl/internal/cmd/page/verify.go; skipped: none; constraints: Behavioral claims in findings 1 and 2 were reproduced with a temporary test in internal/cmd/page (removed afterwards); outputs are quoted verbatim. Findings are restricted to the four assigned files. create.go, present/mutation.go, and the test files were read as context for the revision but are not reported on. Prior-round findings on the presenter boundary, create wiring, and the --no-verify docs are confirmed addressed and are not re-reported. Verified in the checkout: go build ./... and go test ./internal/... under tools/cfl are green at fd33305.
• structure:harness-engineering — complete (constrained); inspected 2 assigned files (9 inspected across reviewers): tools/cfl/internal/cmd/page/edit.go, tools/cfl/internal/cmd/page/verify.go; skipped: none; constraints: none

Inspected files (9)

• tools/cfl/README.md
• tools/cfl/internal/cmd/OUTPUT_SPEC.md
• tools/cfl/internal/cmd/page/create.go
• tools/cfl/internal/cmd/page/create_test.go
• tools/cfl/internal/cmd/page/edit.go
• tools/cfl/internal/cmd/page/edit_test.go
• tools/cfl/internal/cmd/page/verify.go
• tools/cfl/internal/cmd/page/verify_test.go
• tools/cfl/internal/present/mutation.go

0 PR discussion threads considered. 0 summarized; 0 resolved.

---

Completed in 4m 35s | ~$6.20 (est.) | claude-sonnet-5, claude-opus-5 | cr 0.10.286

Field	Value
Model	claude-sonnet-5, claude-opus-5
Reviewers	go:implementation-tests, policies:conventions, architecture:solid-reviewer-agnostic, structure:harness-engineering
Engine	claude_cli · claude-sonnet-5, claude-opus-5
Reviewed by	cr · monit-reviewer
Duration	4m 35s wall · 8m 40s compute
Cost	~$6.20 (est.)
Tokens	109 in / 30.2k out

Per-workstream usage

• go:implementation-tests — claude-sonnet-5
  • In: 36
  • Out: 9.0k
  • Cache read: 2.0M
  • Cache create: 172.3k
  • Cost: ~$1.38 (est.)
  • Duration: 2m 23s
• policies:conventions — claude-sonnet-5
  • In: 22
  • Out: 3.4k
  • Cache read: 1.0M
  • Cache create: 147.0k
  • Cost: ~$0.90 (est.)
  • Duration: 1m 07s
• architecture:solid-reviewer-agnostic — claude-opus-5
  • In: 27
  • Out: 13.7k
  • Cache read: 1.8M
  • Cache create: 231.5k
  • Cost: ~$2.67 (est.)
  • Duration: 3m 49s
• structure:harness-engineering — claude-sonnet-5
  • In: 18
  • Out: 3.2k
  • Cache read: 723.8k
  • Cache create: 126.8k
  • Cost: ~$0.74 (est.)
  • Duration: 58s
• orchestrator-rollup — claude-sonnet-5
  • In: 6
  • Out: 925
  • Cache read: 121.2k
  • Cache create: 122.4k
  • Cost: ~$0.51 (est.)
  • Duration: 20s

[piekstra]

All five taken, in 9778c06.

Attribute-less atoms — a genuine hole in the fix I'd just written. hardBreak and rule carry neither text nor attributes, so dropping one moved neither half of the comparison. Both named now. And you're right that the doc comment overclaimed: an allowlist can't support "same fingerprint means same content". It now states what it actually delivers — a differing fingerprint always means content changed; an identical one means nothing outside the allowlist moved, and a node type ADF adds later goes unnoticed until it's added here.

Worth noting the test caught me: my first attempt to extend the set silently no-op'd (the replace targeted pre-gofmt alignment), and TestAttributelessAtomLossDetected failed on both subtests rather than letting it through.

Fingerprint leaking to the operator — correct, and a UX bug on top of the boundary one: the NUL separators exist so atom markers can't collide with document text, which is precisely the kind of internal detail nobody should be shown. Excerpts are scrubbed, and the command now hands the presenter a DiffOffset plus two excerpts instead of a finished sentence, so the wording lives where ARCHITECTURE.md puts it. Pinned by TestExcerptHidesFingerprintSeparators.

page create untested — fair; I wired the behavior and tested only edit. It now has the same three: content drift fails, --no-verify skips the readback (asserted on request count), normalization tolerated.

Docs — OUTPUT_SPEC.md documents the readback under page create and points at the page edit section for the stderr blocks rather than duplicating them. The README note now appears in the page edit section too; it had landed only under page create, which is backwards given page edit is where the incident came from.

make test and make lint clean.

[monit-reviewer]

Automated PR Review

Reviewed commit: 9778c0643169
Profile: claude-monit-reviewer - Posting as: monit-reviewer

Summary

Reviewer	Findings
go:implementation-tests	0
policies:conventions	0
architecture:solid-reviewer-agnostic	2
structure:harness-engineering	0

architecture:solid-reviewer-agnostic (2 findings)

Minor - tools/cfl/internal/cmd/page/verify.go:290

The structured DiffOffset/SentExcerpt/StoredExcerpt split fixed the NUL leak, but the quantities handed to the presenter are still raw byte positions in the internal fingerprint, presented to the operator as facts about their document. Two distinct effects, both reproduced at this head by rendering through PresentWriteDrift:

# paragraph whose text is 7 characters, containing one inlineCard
content differs (46 chars sent, 7 stored)

# paragraph of ASCII + Japanese text, one word changed
content differs (89 chars sent, 89 stored)
  first difference at offset 0 — sent "aaaaa日本語テキストです、こ\xe3\x82", stored "bbbbb日本語テキストです、こ\xe3\x82"

SentLen/StoredLen (verify.go:290-291) are len() over the fingerprint, so they count atom markers such as \x00inlineCard(url=…) and count bytes rather than characters — 46 and 89 for documents of 7 and 32 characters. readableExcerpt (verify.go:345-355) slices on the same byte index, so a 40-byte window ending mid-rune emits \xe3\x82 into the message. OUTPUT_SPEC.md:276 documents that number as <n> chars, which it is not for any page containing non-ASCII text — which is most real Confluence content.

This is the residue of the fingerprint-leak point rather than a repeat of it: the excerpt half landed, the numeric half did not. ARCHITECTURE.md hard rule 1 still applies to the values the command derives for display.

Suggested fix: make both quantities describe the document, not the fingerprint — count with utf8.RuneCountInString over the text portion (markers excluded), advance readableExcerpt to a rune boundary before slicing (or range over runes for a fixed rune budget), and report the offset in runes to match. If keeping counts honest is more work than it is worth, dropping the counts entirely and relying on the excerpt is a defensible alternative — an inaccurate number is worse than none.

While in this literal: diffOffset(drift.SentText, drift.StoredText) is evaluated three times across verify.go:292-294; hoisting it to a local reads better and makes the shared-offset invariant explicit.

Minor - tools/cfl/internal/cmd/OUTPUT_SPEC.md:278

The spec and the emitted text diverged in this revision. OUTPUT_SPEC.md:278 declares:

  first difference: at offset <n> — sent "<excerpt>", stored "<excerpt>"

while the presenter now emits first difference at offset %d — sent %q, stored %q (mutation.go:156) — the colon moved out when the Difference string was replaced by DiffOffset/excerpts. For a document that calls itself "the authoritative declaration of the target cfl output contract", a wording change in the same commit should carry the spec with it (U-G1).

Also still missing from this section: the third shape the presenter can emit, Confluence added attributes that were not sent: followed by + <node>.attrs.<name> (<before>→<after>) (mutation.go:166-169). The dropped-attributes block above it is spec'd; its sibling is not.

Suggested fix: correct line 278 to match the emitted wording and add the added-attributes block alongside the dropped-attributes one.

Reviewer Coverage

• go:implementation-tests — complete (constrained); inspected 7 assigned files (9 inspected across reviewers): tools/cfl/internal/cmd/page/create.go, tools/cfl/internal/cmd/page/create_test.go, tools/cfl/internal/cmd/page/edit.go, tools/cfl/internal/cmd/page/edit_test.go, tools/cfl/internal/cmd/page/verify.go, tools/cfl/internal/cmd/page/verify_test.go, tools/cfl/internal/present/mutation.go; skipped: none; constraints: none
• policies:conventions — complete (constrained); inspected 2 assigned files (9 inspected across reviewers): tools/cfl/internal/cmd/page/edit.go, tools/cfl/internal/cmd/page/verify.go; skipped: none; constraints: Sibling checkouts for shared standards (../cli-common/docs, ../.github) are not present in this workbench, so cross-repo shared conventions could not be verified beyond what's linked from tools/cfl/CLAUDE.md.
• architecture:solid-reviewer-agnostic — complete (constrained); inspected 4 assigned files (9 inspected across reviewers): tools/cfl/README.md, tools/cfl/internal/cmd/OUTPUT_SPEC.md, tools/cfl/internal/cmd/page/edit.go, tools/cfl/internal/cmd/page/verify.go; skipped: none; constraints: Findings restricted to the four assigned files; create.go, present/mutation.go and the test files were read as context only. Output claims were reproduced with a temporary test in internal/cmd/page rendering through PresentWriteDrift; the file was removed and the tree is clean. Prior-round findings are confirmed fixed: hardBreak/rule are now in the fingerprint, the allowlist comment no longer overclaims, the excerpt no longer leaks NUL, README covers both commands, and OUTPUT_SPEC cross-references page create. Verified at 9778c06: go build ./... and go test ./internal/... under tools/cfl are green. edit.go is unchanged since c78535e and carries no findings this round.
• structure:harness-engineering — complete (constrained); inspected 2 assigned files (9 inspected across reviewers): tools/cfl/internal/cmd/page/edit.go, tools/cfl/internal/cmd/page/verify.go; skipped: none; constraints: none

Inspected files (9)

• tools/cfl/README.md
• tools/cfl/internal/cmd/OUTPUT_SPEC.md
• tools/cfl/internal/cmd/page/create.go
• tools/cfl/internal/cmd/page/create_test.go
• tools/cfl/internal/cmd/page/edit.go
• tools/cfl/internal/cmd/page/edit_test.go
• tools/cfl/internal/cmd/page/verify.go
• tools/cfl/internal/cmd/page/verify_test.go
• tools/cfl/internal/present/mutation.go

0 PR discussion threads considered. 0 summarized; 0 resolved.

---

Completed in 3m 57s | ~$7.01 (est.) | claude-sonnet-5, claude-opus-5 | cr 0.10.286

Field	Value
Model	claude-sonnet-5, claude-opus-5
Reviewers	go:implementation-tests, policies:conventions, architecture:solid-reviewer-agnostic, structure:harness-engineering
Engine	claude_cli · claude-sonnet-5, claude-opus-5
Reviewed by	cr · monit-reviewer
Duration	3m 57s wall · 8m 02s compute
Cost	~$7.01 (est.)
Tokens	84 in / 26.8k out

Per-workstream usage

• go:implementation-tests — claude-sonnet-5
  • In: 20
  • Out: 5.6k
  • Cache read: 1.3M
  • Cache create: 239.3k
  • Cost: ~$1.36 (est.)
  • Duration: 1m 43s
• policies:conventions — claude-sonnet-5
  • In: 18
  • Out: 5.1k
  • Cache read: 975.9k
  • Cache create: 199.7k
  • Cost: ~$1.12 (est.)
  • Duration: 1m 32s
• architecture:solid-reviewer-agnostic — claude-opus-5
  • In: 20
  • Out: 8.9k
  • Cache read: 1.5M
  • Cache create: 307.0k
  • Cost: ~$2.89 (est.)
  • Duration: 2m 46s
• structure:harness-engineering — claude-sonnet-5
  • In: 22
  • Out: 4.9k
  • Cache read: 1.2M
  • Cache create: 177.4k
  • Cost: ~$1.09 (est.)
  • Duration: 1m 22s
• orchestrator-rollup — claude-sonnet-5
  • In: 4
  • Out: 2.3k
  • Cache read: 31.8k
  • Cache create: 136.4k
  • Cost: ~$0.56 (est.)
  • Duration: 37s

[piekstra]

Both taken, in a7a6add.

The counts were the same class of bug this PR exists to fix — internal representation presented to the operator as fact about their document. Your two reproductions were exact: an atom marker inflating a 7-character paragraph to "46 chars sent", and a multibyte edit reporting equal byte lengths as though nothing moved.

Counts now come from text nodes only, so they're the characters a reader sees. Offsets and excerpts are rune-based over the scrubbed rendering. And the case your first example exposed — visible text identical, only the embedded card changed — now says so explicitly instead of printing two equal numbers and leaving the reader to work it out:

Stored adf body does not match what was sent: visible text is unchanged at 7 characters, but embedded content differs.

Pinned by TestReportedCountsDescribeTheDocument (atom doesn't inflate the count; multibyte counts runes and the offset is 3 not 9).

OUTPUT_SPEC — right, the colon moved when Difference became DiffOffset, and the spec kept the old shape. Synced, and it now documents all three first-line shapes rather than the one, plus a note that the counts and offset are document measurements.

make test (61 packages) and make lint clean.

[monit-reviewer]

Automated PR Review

Reviewed commit: a7a6addb2b27
Profile: claude-monit-reviewer - Posting as: monit-reviewer

Summary

Reviewer	Findings
go:implementation-tests	0
policies:conventions	0
architecture:solid-reviewer-agnostic	2
structure:harness-engineering	1

architecture:solid-reviewer-agnostic (2 findings)

Major - tools/cfl/internal/cmd/page/verify.go:64

The visible-character counts introduced in this revision are populated only by compareADF; the XHTML branch returns a writeDrift with VisibleSent/VisibleStored left at zero. PresentWriteDrift cannot tell "zero characters" from "not measured", so it takes the equal-lengths branch of describeContentChange and states something false about the document. Reproduced at this head:

# sent "<p>hello world, this is what we sent</p>", stored "<p>hello</p>"
Stored xhtml body does not match what was sent: content differs at the same length of 0 characters.
  first difference at offset 5 — sent " world, this is what we sent", stored ""

The excerpt line is right; the sentence above it tells an operator the length did not change, on a write that lost 30 of 35 visible characters. The previous revision reported byte lengths here, which were at least directionally true, so this is a regression on the xhtml half of the feature introduced by the fix to the adf half — writeDrift now has two producers and only one honors the documented field semantics ("count the characters a reader sees", verify.go:29-31). That is U-L1, and it also falsifies the sentence added at OUTPUT_SPEC.md:288-289 ("Counts are characters a reader sees … measured on the document") for one of the two supported exact formats.

Suggested fix: populate the fields in the XHTML branch — xhtmlText already returns exactly the reader-visible text, so VisibleSent: len([]rune(sentText)), VisibleStored: len([]rune(storedText)) completes the contract with no new traversal. AtomsChanged stays false, which is correct: storage format has no atom markers.

The gap survived because TestReportedCountsDescribeTheDocument (verify_test.go:461-493) exercises ADF only, while TestRunEditFailsWhenStoredContentDiffers drives XHTML but asserts on the error string, never on the report. A table case with a storage-format body would have caught it (U-T1).

Minor - tools/cfl/internal/cmd/page/verify.go:369

diffOffset now counts runes rather than bytes, but it still counts them in the fingerprint, which carries an atom marker such as inlineCard(url=https://example.test/x) inline with the text. So the reported offset is a position in the compared representation, not in the document — while OUTPUT_SPEC.md:288-289 now asserts "the offset is a character position … measured on the document rather than on any internal representation".

Reproduced: a paragraph ab + inlineCard + cd where only the final character changed (d→Z) — document character 3 — reports first difference at offset 41, the 38-character marker having displaced it. The excerpts ("d" vs "Z") are correct, so the reader is pointed at the right content by a number that does not locate it in anything they can see.

This is the last of the fingerprint-as-document-fact residue (ARCHITECTURE.md hard rule 1, on values the command derives for display). Two defensible fixes: compute the offset over visibleText and report it only when the divergence lies in text (falling back to the atom description otherwise), or leave the computation alone and reword OUTPUT_SPEC to say the offset locates the difference within the compared body including embedded-content markers. Either is fine; what should not stand is the spec claiming a document position that the code does not produce.

structure:harness-engineering (1 finding)

Minor - tools/cfl/internal/cmd/page/verify.go:338

Second round flagging the same gap: the AddedAttrs stderr shape the presenter emits ("Confluence added attributes that were not sent:" / " + .attrs. (→)", mutation.go's PresentWriteDrift, fed from drift.AddedAttrs at verify.go:338) still has no entry in OUTPUT_SPEC.md. The current page edit section documents the success block, the DroppedAttrs normalization block, and all three TextChanged wordings in detail, but grep -n "added" tools/cfl/internal/cmd/OUTPUT_SPEC.md still matches nothing shape-related. Since this round's diff shows the spec is actively being kept current with the presenter (the three describeContentChange wordings and the character/offset semantics were just added), the sibling shape is a one-block omission rather than an intentional scope cut. Add the added-attributes block next to the dropped-attributes one so OUTPUT_SPEC.md covers every stderr shape PresentWriteDrift can produce.

Reviewer Coverage

• go:implementation-tests — complete (constrained); inspected 7 assigned files (9 inspected across reviewers): tools/cfl/internal/cmd/page/create.go, tools/cfl/internal/cmd/page/create_test.go, tools/cfl/internal/cmd/page/edit.go, tools/cfl/internal/cmd/page/edit_test.go, tools/cfl/internal/cmd/page/verify.go, tools/cfl/internal/cmd/page/verify_test.go, tools/cfl/internal/present/mutation.go; skipped: none; constraints: none
• policies:conventions — complete (constrained); inspected 2 assigned files (9 inspected across reviewers): tools/cfl/internal/cmd/page/edit.go, tools/cfl/internal/cmd/page/verify.go; skipped: none; constraints: Sibling checkouts for shared standards (../cli-common/docs, ../.github) are not present in this workbench, so cross-repo shared conventions could not be verified beyond what's linked from tools/cfl/CLAUDE.md.
• architecture:solid-reviewer-agnostic — complete (constrained); inspected 4 assigned files (9 inspected across reviewers): tools/cfl/README.md, tools/cfl/internal/cmd/OUTPUT_SPEC.md, tools/cfl/internal/cmd/page/edit.go, tools/cfl/internal/cmd/page/verify.go; skipped: none; constraints: Confirmed fixed this round: rune-based offsets and excerpts, atom markers no longer inflating counts for ADF, and the OUTPUT_SPEC first-difference wording. Findings restricted to the four assigned files; create.go, present/mutation.go and the tests were read as context only. Output claims were reproduced with temporary tests in internal/cmd/page rendering through PresentWriteDrift; the probes were deleted afterwards. Verified at a7a6add: go build ./... and go test ./internal/... under tools/cfl are green; working tree clean. edit.go and README.md are unchanged since earlier revisions and carry no findings this round.
• structure:harness-engineering — complete (constrained); inspected 2 assigned files (9 inspected across reviewers): tools/cfl/internal/cmd/page/edit.go, tools/cfl/internal/cmd/page/verify.go; skipped: none; constraints: none

Inspected files (9)

• tools/cfl/README.md
• tools/cfl/internal/cmd/OUTPUT_SPEC.md
• tools/cfl/internal/cmd/page/create.go
• tools/cfl/internal/cmd/page/create_test.go
• tools/cfl/internal/cmd/page/edit.go
• tools/cfl/internal/cmd/page/edit_test.go
• tools/cfl/internal/cmd/page/verify.go
• tools/cfl/internal/cmd/page/verify_test.go
• tools/cfl/internal/present/mutation.go

0 PR discussion threads considered. 0 summarized; 0 resolved.

---

Completed in 3m 34s | ~$7.93 (est.) | claude-sonnet-5, claude-opus-5 | cr 0.10.286

Field	Value
Model	claude-sonnet-5, claude-opus-5
Reviewers	go:implementation-tests, policies:conventions, architecture:solid-reviewer-agnostic, structure:harness-engineering
Engine	claude_cli · claude-sonnet-5, claude-opus-5
Reviewed by	cr · monit-reviewer
Duration	3m 34s wall · 6m 09s compute
Cost	~$7.93 (est.)
Tokens	72 in / 19.8k out

Per-workstream usage

• go:implementation-tests — claude-sonnet-5
  • In: 18
  • Out: 3.7k
  • Cache read: 1.3M
  • Cache create: 288.4k
  • Cost: ~$1.53 (est.)
  • Duration: 1m 17s
• policies:conventions — claude-sonnet-5
  • In: 14
  • Out: 2.2k
  • Cache read: 831.3k
  • Cache create: 245.5k
  • Cost: ~$1.20 (est.)
  • Duration: 48s
• architecture:solid-reviewer-agnostic — claude-opus-5
  • In: 20
  • Out: 9.4k
  • Cache read: 1.7M
  • Cache create: 366.6k
  • Cost: ~$3.40 (est.)
  • Duration: 2m 41s
• structure:harness-engineering — claude-sonnet-5
  • In: 16
  • Out: 3.3k
  • Cache read: 944.9k
  • Cache create: 230.1k
  • Cost: ~$1.20 (est.)
  • Duration: 1m 00s
• orchestrator-rollup — claude-sonnet-5
  • In: 4
  • Out: 1.1k
  • Cache read: 31.8k
  • Cache create: 153.6k
  • Cost: ~$0.60 (est.)
  • Duration: 21s

[piekstra]

All three taken, in 72ddbf7 + d5f0a2e. Two were regressions I introduced in the previous round, and both were this PR's own failure mode — the tool stating something untrue.

XHTML never populated the visible counts, so a storage-format drift reported "the same length of 0 characters" for a body it had measured in full. Measured now, with TestXHTMLDriftReportsRealCounts asserting the report can't claim zero for a counted body.

The offset was a position in the fingerprint — which carries atom markers inline — while I'd just changed OUTPUT_SPEC to assert it was a document position. So the spec and the value disagreed, and the spec was the one making the promise. Positions are measured on the reader-visible text now.

That exposed a follow-on: a change confined to embedded content has no position in the visible text. It previously pointed at an offset that meant nothing; it now reports none, and naming what moved falls to the attribute list, which content loss reports too rather than only the normalization case. TestAtomOnlyChangeReportsNoTextOffset pins that no text position is claimed.

Added-attributes shape — fair, flagged twice and missed twice. Documented, along with the attribute lines that now appear under content loss and the note that the offset line is omitted for embedded-only changes.

Also dropped scrubFingerprint, which lost its last caller once excerpts came from visible text.

make test (61 packages) and make lint (3 modules) clean.

[monit-reviewer]

Automated PR Review

Reviewed commit: dcde645c1e51
Profile: claude-monit-reviewer - Posting as: monit-reviewer

Summary

Reviewer	Findings
go:implementation-tests	0
policies:conventions	1
architecture:solid-reviewer-agnostic	1
structure:harness-engineering	0

policies:conventions (1 finding)

Nits - tools/cfl/internal/cmd/page/verify.go:399

readableExcerpt's doc comment is now stale: it says the function 'renders part of a fingerprint' and strips NUL atom separators, but this revision (dcde645, dropping scrubFingerprint as unused) changed both call sites to pass drift.SentVisible/StoredVisible — reader-visible text with no NUL markers — not the fingerprint. The comment misdescribes the function's current input and invites a future edit to re-add NUL-scrubbing that's no longer needed. Reword to describe slicing a rune window of the document's visible text, matching what diffOffset (verify.go:379-382) now documents.

architecture:solid-reviewer-agnostic (1 finding)

Minor - tools/cfl/internal/cmd/page/verify.go:339

Measuring the offset on the visible text is the right call, but it leaves the AtomsChanged path with nothing to say in the two cases the atom fingerprint was specifically built for, and OUTPUT_SPEC.md:300-301 (added this revision) promises otherwise: "the attribute lines then identify what moved." They only do when an attribute name or count moved. Reproduced at this head:

# inlineCard url .../a -> .../b   (dropped=[] added=[])
Stored adf body does not match what was sent: visible text is unchanged at 4 characters, but embedded content differs.
The page was updated, but it does not hold the content supplied. Re-read the page before treating the change as applied.

# hardBreak dropped               (dropped=[] added=[])
Stored adf body does not match what was sent: visible text is unchanged at 2 characters, but embedded content differs.
The page was updated, but it does not hold the content supplied. Re-read the page before treating the change as applied.

# media dropped                   (attribute lines present — this case works)
  attributes dropped:
    - media.attrs.id (1→0)

A card swap keeps inlineCard.attrs.url at count 1 on both sides, and hardBreak/rule carry no attrs at all, so diffAttrProfiles yields nothing. The operator is told the write failed and is given no way to find where — on a substitution the fingerprint comment at verify.go:129-131 explicitly calls out ("Identity, not just presence: swapping one card for another keeps the count identical").

The information exists and is already computed: drift.SentText/StoredText hold the fingerprint, whose divergence renders as inlineCard(url=…/a) vs inlineCard(url=…/b). Suggested fix: when AtomsChanged, take diffOffset/readableExcerpt over the fingerprint pair and pass the two atom descriptions as separate presenter fields (e.g. AtomSent/AtomStored) so the presenter can phrase an "embedded content: sent … stored …" line — no document offset implied, so the earlier round's objection does not return. At minimum, correct OUTPUT_SPEC.md:300-301, which currently claims a level of detail the report cannot always deliver (U-G1: documented contract must match emitted behavior).

Reviewer Coverage

• go:implementation-tests — complete (constrained); inspected 7 assigned files (9 inspected across reviewers): tools/cfl/internal/cmd/page/create.go, tools/cfl/internal/cmd/page/create_test.go, tools/cfl/internal/cmd/page/edit.go, tools/cfl/internal/cmd/page/edit_test.go, tools/cfl/internal/cmd/page/verify.go, tools/cfl/internal/cmd/page/verify_test.go, tools/cfl/internal/present/mutation.go; skipped: none; constraints: none
• policies:conventions — complete (constrained); inspected 2 assigned files (9 inspected across reviewers): tools/cfl/internal/cmd/page/edit.go, tools/cfl/internal/cmd/page/verify.go; skipped: none; constraints: Sibling checkouts for shared standards (../cli-common/docs, ../.github) are not present in this workbench, so cross-repo shared conventions could not be verified beyond what's linked from tools/cfl/CLAUDE.md.
• architecture:solid-reviewer-agnostic — complete (constrained); inspected 4 assigned files (9 inspected across reviewers): tools/cfl/README.md, tools/cfl/internal/cmd/OUTPUT_SPEC.md, tools/cfl/internal/cmd/page/edit.go, tools/cfl/internal/cmd/page/verify.go; skipped: none; constraints: Confirmed fixed this round: XHTML visible counts, offsets/excerpts measured on visible text, scrubFingerprint removed, and the added-attributes block now spec'd. Findings restricted to the four assigned files; create.go, present/mutation.go and the tests were read as context only. Report shapes were reproduced by rendering writeDrift values through PagePresenter.PresentWriteDrift in a temporary test, since the wording lives in present/ which is outside my anchorable files. Verified at dcde645: go build ./... and go test ./internal/... under tools/cfl are green; working tree clean after removing the probe. edit.go and README.md are unchanged since earlier revisions and carry no findings.
• structure:harness-engineering — complete (constrained); inspected 2 assigned files (9 inspected across reviewers): tools/cfl/internal/cmd/page/edit.go, tools/cfl/internal/cmd/page/verify.go; skipped: none; constraints: none

Inspected files (9)

• tools/cfl/README.md
• tools/cfl/internal/cmd/OUTPUT_SPEC.md
• tools/cfl/internal/cmd/page/create.go
• tools/cfl/internal/cmd/page/create_test.go
• tools/cfl/internal/cmd/page/edit.go
• tools/cfl/internal/cmd/page/edit_test.go
• tools/cfl/internal/cmd/page/verify.go
• tools/cfl/internal/cmd/page/verify_test.go
• tools/cfl/internal/present/mutation.go

0 PR discussion threads considered. 0 summarized; 0 resolved.

---

Completed in 3m 38s | ~$8.98 (est.) | claude-sonnet-5, claude-opus-5 | cr 0.10.286

Field	Value
Model	claude-sonnet-5, claude-opus-5
Reviewers	go:implementation-tests, policies:conventions, architecture:solid-reviewer-agnostic, structure:harness-engineering
Engine	claude_cli · claude-sonnet-5, claude-opus-5
Reviewed by	cr · monit-reviewer
Duration	3m 38s wall · 5m 48s compute
Cost	~$8.98 (est.)
Tokens	68 in / 19.5k out

Per-workstream usage

• go:implementation-tests — claude-sonnet-5
  • In: 16
  • Out: 4.0k
  • Cache read: 1.3M
  • Cache create: 336.7k
  • Cost: ~$1.70 (est.)
  • Duration: 1m 06s
• policies:conventions — claude-sonnet-5
  • In: 14
  • Out: 3.1k
  • Cache read: 957.4k
  • Cache create: 296.6k
  • Cost: ~$1.45 (est.)
  • Duration: 56s
• architecture:solid-reviewer-agnostic — claude-opus-5
  • In: 18
  • Out: 8.7k
  • Cache read: 1.7M
  • Cache create: 428.8k
  • Cost: ~$3.77 (est.)
  • Duration: 2m 34s
• structure:harness-engineering — claude-sonnet-5
  • In: 14
  • Out: 2.7k
  • Cache read: 926.6k
  • Cache create: 280.3k
  • Cost: ~$1.37 (est.)
  • Duration: 45s
• orchestrator-rollup — claude-sonnet-5
  • In: 6
  • Out: 1.0k
  • Cache read: 143.5k
  • Cache create: 168.1k
  • Cost: ~$0.69 (est.)
  • Duration: 24s

[piekstra]

Both taken, in 85c7f0a.

The atoms-only path had nothing to say about exactly the nodes I'd extended the fingerprint to catch. hardBreak and rule carry no attributes, so a change confined to them left the visible text identical, produced no attribute lines, and printed "embedded content differs" with no detail — while OUTPUT_SPEC promised the attribute lines would identify what moved. The spec was writing a cheque the code couldn't cash, for the one case the previous round added.

Atom types whose counts changed are now named directly:

  embedded content changed:
    ~ hardBreak (2→1)

TestAttributelessAtomChangeIsNamed asserts the precondition (hardBreak contributes no attributes) and then that the report still names it — so the test fails if this regresses to attribute-only reporting.

readableExcerpt's doc comment still described reading from the fingerprint and stripping NUL separators, after both call sites moved to visible text. Corrected. Fitting that a stale comment surfaced in this PR of all of them.

make test (61 packages) and make lint (3 modules) clean.