fix(cfl): INT-732 detect formatting loss a lossy read hides

tools/cfl/internal/cmd/page/create.go

@@ -183,12 +183,13 @@ func runCreate(ctx context.Context, opts *createOptions) error {
 	// A create writes the same verbatim body an edit does, so it can suffer
 	// the same unseen server-side loss.
 	return verifyStoredBody(ctx, verifyRequest{
-		opts:        opts.Options,
-		client:      client,
-		pageID:      page.ID,
-		bodyFormat:  bodyFormat,
-		sentContent: sentContent,
-		enabled:     !opts.noVerify,
+		opts:          opts.Options,
+		client:        client,
+		pageID:        page.ID,
+		bodyFormat:    bodyFormat,
+		sentContent:   sentContent,
+		storageBefore: "",
+		enabled:       !opts.noVerify,
 	})
 }
 

tools/cfl/internal/cmd/page/edit.go

@@ -215,6 +215,13 @@ func runEdit(ctx context.Context, opts *editOptions) error {
 		req.Body = existingPage.Body
 	}
 
+	// Captured before the write: the storage body is what reveals loss the
+	// caller inherited from a lossy read of their own.
+	var storageBefore string
+	if verificationApplies(bodyFormat, hasNewContent, opts.noVerify) {
+		storageBefore, _ = readStorageBody(ctx, client, opts.pageID)
+	}
+
 	page, err := client.UpdatePage(ctx, opts.pageID, req)
 	if err != nil {
 		return err
@@ -231,12 +238,13 @@ func runEdit(ctx context.Context, opts *editOptions) error {
 	}
 
 	return verifyStoredBody(ctx, verifyRequest{
-		opts:        opts.Options,
-		client:      client,
-		pageID:      opts.pageID,
-		bodyFormat:  bodyFormat,
-		sentContent: sentContent,
-		enabled:     hasNewContent && !opts.noVerify,
+		opts:          opts.Options,
+		client:        client,
+		pageID:        opts.pageID,
+		bodyFormat:    bodyFormat,
+		sentContent:   sentContent,
+		storageBefore: storageBefore,
+		enabled:       hasNewContent && !opts.noVerify,
 	})
 }
 

tools/cfl/internal/cmd/page/verify.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"regexp"
 	"sort"
 	"strings"
 
@@ -355,6 +356,20 @@ type verifyRequest struct {
 	bodyFormat  string
 	sentContent string
 	enabled     bool
+	// storageBefore is the page's storage body read before the write. It is
+	// the representation that survives an ADF round trip, so it is what
+	// reveals loss the caller inherited from their own read.
+	storageBefore string
+}
+
+// verificationApplies reports whether a write will be verified. Markdown is
+// converted before sending, so the stored body is not comparable to what the
+// caller supplied and no read is worth paying for.
+func verificationApplies(bodyFormat string, hasNewContent, noVerify bool) bool {
+	if !hasNewContent || noVerify {
+		return false
+	}
+	return bodyFormat == bodyFormatADF || bodyFormat == bodyFormatXHTML
 }
 
 // verifyStoredBody re-reads a page after a write and reports what Confluence
@@ -385,7 +400,8 @@ func verifyStoredBody(ctx context.Context, req verifyRequest) error {
 	if err != nil {
 		return fmt.Errorf("verifying stored page: %w", err)
 	}
-	if drift.Clean() {
+	lostElements := verifyStorageLoss(ctx, req)
+	if drift.Clean() && len(lostElements) == 0 {
 		return nil
 	}
 
@@ -402,6 +418,7 @@ func verifyStoredBody(ctx context.Context, req verifyRequest) error {
 		AtomChanges:   drift.AtomChanges,
 		DroppedAttrs:  drift.DroppedAttrs,
 		AddedAttrs:    drift.AddedAttrs,
+		LostElements:  lostElements,
 	}
 	if emitErr := cflpresent.Emit(req.opts, cflpresent.PagePresenter{}.PresentWriteDrift(finding)); emitErr != nil {
 		return emitErr
@@ -463,3 +480,64 @@ func readableExcerpt(s string, at int) string {
 	}
 	return string(r[at:end])
 }
+
+// Comparing what was sent against what was stored cannot see loss the caller
+// inherited from their own read. Confluence's atlas_doc_format representation
+// of a page does not always carry marks its storage representation does — em
+// and strong wrapping a code span in a table cell are dropped — so reading a
+// page as ADF and writing it straight back destroys them, and both sides of a
+// sent-versus-stored comparison carry the loss equally.
+//
+// The storage representation is the one that survives the round trip, so it
+// is the one worth watching. Element types whose counts fall across a write
+// are reported: text edits move no element counts, while losing emphasis, a
+// code span, or a link is collateral a caller almost never intends.
+
+var storageElementRE = regexp.MustCompile(`<(\w[\w-]*)[\s/>]`)
+
+// storageProfile counts elements in a storage-format body.
+func storageProfile(body string) map[string]int {
+	profile := map[string]int{}
+	for _, m := range storageElementRE.FindAllStringSubmatch(body, -1) {
+		profile[m[1]]++
+	}
+	return profile
+}
+
+// diffStorageLoss names element types that became less frequent, in sorted
+// order. Additions are not reported: adding content is what an edit is for.
+func diffStorageLoss(before, after map[string]int) []string {
+	var lost []string
+	for name, n := range before {
+		if after[name] < n {
+			lost = append(lost, fmt.Sprintf("%s (%d→%d)", name, n, after[name]))
+		}
+	}
+	sort.Strings(lost)
+	return lost
+}
+
+// verifyStorageLoss compares the page's storage body across the write. An
+// empty result means either nothing was lost or the comparison could not be
+// made; it never reports loss it did not observe.
+func verifyStorageLoss(ctx context.Context, req verifyRequest) []string {
+	if req.storageBefore == "" {
+		return nil
+	}
+	after, err := readStorageBody(ctx, req.client, req.pageID)
+	if err != nil || after == "" {
+		return nil
+	}
+	return diffStorageLoss(storageProfile(req.storageBefore), storageProfile(after))
+}
+
+// readStorageBody fetches a page's storage representation, or "" when it is
+// unavailable. Callers treat that as "cannot compare" rather than as "no
+// loss", so a failure here never manufactures a clean result.
+func readStorageBody(ctx context.Context, client *api.Client, pageID string) (string, error) {
+	page, err := client.GetPage(ctx, pageID, &api.GetPageOptions{BodyFormat: apiBodyFormat(bodyFormatXHTML)})
+	if err != nil {
+		return "", err
+	}
+	return bodyValue(page, bodyFormatXHTML), nil
+}

tools/cfl/internal/cmd/page/verify_test.go

@@ -544,3 +544,83 @@ func TestAttributelessAtomChangeIsNamed(t *testing.T) {
 		t.Errorf("report must name the changed atom when no attributes can:\n%s", report)
 	}
 }
+
+// The loss this exists to catch: a no-op ADF round trip that Confluence
+// stores without the emphasis marks its storage body carried. Sent and
+// stored agree, so only the storage comparison sees it.
+func TestStorageLossDetectedWhenSentAndStoredAgree(t *testing.T) {
+	before := `<p><em><strong><code>KEY</code></strong></em></p><p><strong>bold</strong></p>`
+	after := `<p><code>KEY</code></p><p><strong>bold</strong></p>`
+	lost := diffStorageLoss(storageProfile(before), storageProfile(after))
+	if len(lost) != 2 {
+		t.Fatalf("lost = %v, want em and strong", lost)
+	}
+	joined := strings.Join(lost, " ")
+	if !strings.Contains(joined, "em (1→0)") || !strings.Contains(joined, "strong (2→1)") {
+		t.Errorf("unexpected loss report: %v", lost)
+	}
+}
+
+// Editing text moves no element counts, so an ordinary edit stays quiet.
+func TestStorageLossQuietOnTextOnlyEdit(t *testing.T) {
+	before := `<p>hello <strong>world</strong></p>`
+	after := `<p>goodbye <strong>world</strong></p>`
+	if lost := diffStorageLoss(storageProfile(before), storageProfile(after)); len(lost) != 0 {
+		t.Errorf("text-only edit reported loss: %v", lost)
+	}
+}
+
+// Adding content is what an edit is for; only losses are reported.
+func TestStorageLossIgnoresAdditions(t *testing.T) {
+	before := `<p>hello</p>`
+	after := `<p>hello</p><p><strong>new</strong></p>`
+	if lost := diffStorageLoss(storageProfile(before), storageProfile(after)); len(lost) != 0 {
+		t.Errorf("additions reported as loss: %v", lost)
+	}
+}
+
+// A missing baseline must read as "cannot compare", never as "no loss".
+func TestStorageLossNotClaimedWithoutBaseline(t *testing.T) {
+	req := verifyRequest{storageBefore: ""}
+	if lost := verifyStorageLoss(context.Background(), req); lost != nil {
+		t.Errorf("claimed a comparison with no baseline: %v", lost)
+	}
+}
+
+func TestVerificationApplies(t *testing.T) {
+	tests := []struct {
+		format               string
+		hasNewContent, noVer bool
+		want                 bool
+	}{
+		{bodyFormatADF, true, false, true},
+		{bodyFormatXHTML, true, false, true},
+		{bodyFormatMarkdown, true, false, false},
+		{bodyFormatADF, true, true, false},
+		{bodyFormatADF, false, false, false},
+	}
+	for _, tc := range tests {
+		if got := verificationApplies(tc.format, tc.hasNewContent, tc.noVer); got != tc.want {
+			t.Errorf("verificationApplies(%q,%v,%v) = %v, want %v", tc.format, tc.hasNewContent, tc.noVer, got, tc.want)
+		}
+	}
+}
+
+func TestPresentedStorageLossExplainsTheCause(t *testing.T) {
+	model := cflpresent.PagePresenter{}.PresentWriteDrift(cflpresent.WriteDrift{
+		BodyFormat:   bodyFormatADF,
+		LostElements: []string{"em (3→1)", "strong (3→1)"},
+	})
+	var b strings.Builder
+	for _, sec := range model.Sections {
+		if msg, ok := sec.(*sharedpresent.MessageSection); ok {
+			b.WriteString(msg.Message)
+		}
+	}
+	out := b.String()
+	for _, want := range []string{"lost formatting", "em (3→1)", "writing it back"} {
+		if !strings.Contains(out, want) {
+			t.Errorf("report missing %q:\n%s", want, out)
+		}
+	}
+}

tools/cfl/internal/present/mutation.go

@@ -144,6 +144,9 @@ type WriteDrift struct {
 	AtomChanges  []string
 	DroppedAttrs []string
 	AddedAttrs   []string
+	// LostElements names storage element types that became less frequent
+	// across the write.
+	LostElements []string
 }
 
 // attrLines lists attribute changes, which identify the embedded content
@@ -190,6 +193,13 @@ func describeContentChange(d WriteDrift) string {
 // other means it landed in a document the server tidied.
 func (PagePresenter) PresentWriteDrift(d WriteDrift) *sharedpresent.OutputModel {
 	var lines []string
+	if len(d.LostElements) > 0 {
+		lines = append(lines, "The stored page lost formatting that was present before this write:")
+		for _, e := range d.LostElements {
+			lines = append(lines, "  - "+e)
+		}
+		lines = append(lines, "Reading a page as ADF and writing it back does not always preserve marks its storage form carries. Compare against the storage body before assuming the change was clean.", "")
+	}
 	if d.TextChanged {
 		lines = append(lines,
 			fmt.Sprintf("Stored %s body does not match what was sent: %s.", d.BodyFormat, describeContentChange(d)),