fix: omit unchanged revision formatting details

src/components/RevisionPanel.test.tsx

@@ -2,7 +2,7 @@ import { describe, it, expect, vi } from 'vitest'
 import { render, screen } from '@testing-library/react'
 import userEvent from '@testing-library/user-event'
 import { RevisionPanel } from './RevisionPanel'
-import type { DocxDiffRevision as Revision, RevisionListEntry } from 'docxodus/core'
+import type { DocxDiffRevision as Revision, RevisionListEntry, FormatChangeDetails } from 'docxodus/core'
 
 const mockRevisions: Revision[] = [
   {
@@ -45,12 +45,18 @@ describe('RevisionPanel', () => {
     expect(onAccept).toHaveBeenCalledWith('rev2-native-1');
   });
 
-  it('retains structural diagnostics and disables unsafe resolution', async () => {
+  it.each([
+    { type: 'structure', family: 'cell_merge', filter: 'structural', label: 'cell merge' },
+    { type: 'format', family: 'properties_change', filter: 'formatting', label: 'Formatted' },
+  ] as const)('retains $type diagnostics and disables unsafe resolution', async ({ type, family, filter, label }) => {
     const onAccept = vi.fn();
-    render(<RevisionPanel revisions={[native({ type: 'structure', family: 'cell_merge', resolutionStatus: 'ambiguous', diagnostic: { code: 'ambiguous_pair', message: 'The cell merge has conflicting markers.' } })]} onAccept={onAccept} onAcceptAll={vi.fn()} />);
-    await userEvent.setup().selectOptions(screen.getByRole('combobox'), 'structural');
-    expect(screen.getByText('cell merge')).toBeInTheDocument();
-    expect(screen.getByText('The cell merge has conflicting markers.')).toBeInTheDocument();
+    const revision = native({ type, family, resolutionStatus: 'ambiguous', diagnostic: { code: 'ambiguous_pair', message: 'This change has conflicting markers.' } });
+    render(<RevisionPanel revisions={[revision]} onAccept={onAccept} onAcceptAll={vi.fn()} formatDetails={{ [revision.id]: {
+      oldProperties: { bold: 'false' }, newProperties: { bold: 'false' },
+    } }} />);
+    await userEvent.setup().selectOptions(screen.getByRole('combobox'), filter);
+    expect(screen.getByText(label)).toBeInTheDocument();
+    expect(screen.getByText('This change has conflicting markers.')).toBeInTheDocument();
     expect(screen.getByRole('button', { name: 'Accept' })).toBeDisabled();
     expect(screen.getByRole('button', { name: 'Accept all' })).toBeDisabled();
     expect(onAccept).not.toHaveBeenCalled();
@@ -61,6 +67,69 @@ describe('RevisionPanel', () => {
     expect(screen.getByText('No tracked changes found in this document.')).toBeInTheDocument()
   })
 
+  it.each(['comparison', 'native'] as const)('shows only actual formatting differences for %s revisions', async (source) => {
+    const unchanged: FormatChangeDetails = {
+      oldProperties: { justification: 'Left', spacingAfter: '0' },
+      newProperties: { justification: 'Left', spacingAfter: '0' },
+    }
+    const changed: FormatChangeDetails = {
+      oldProperties: { ...unchanged.oldProperties, bold: 'false', fontSize: '20', underline: 'single' },
+      newProperties: { ...unchanged.newProperties, bold: 'true', fontSize: '24', italic: 'true' },
+    }
+    const entries = [
+      { text: 'Unchanged heading', details: unchanged },
+      { text: 'Changed heading', details: changed },
+    ]
+    const revisions = entries.map(({ text, details }) => source === 'native'
+      ? native({ id: text, type: 'format', family: 'properties_change', text })
+      : { ...mockRevisions[0], revisionType: 'FormatChanged', text, formatChange: details })
+    const { container } = render(<RevisionPanel
+      revisions={[...revisions, { ...mockRevisions[0], formatChange: unchanged }]}
+      formatDetails={Object.fromEntries(entries.map(({ text, details }) => [text, details]))}
+    />)
+
+    expect(Array.from(container.querySelectorAll('.rdv-format-change'), row => row.textContent)).toEqual([
+      'BoldNo→Yes', 'Font Size20→24', 'UnderlineSingle(removed)', 'ItalicYes(added)',
+    ])
+    expect(screen.queryByText('Unchanged heading')).not.toBeInTheDocument()
+    expect(screen.getByText('This is inserted text')).toBeInTheDocument()
+    expect(screen.getByText('2 changes')).toBeInTheDocument()
+    expect(screen.getByRole('option', { name: 'Formatting (1)' })).toBeInTheDocument()
+    await userEvent.setup().selectOptions(screen.getByRole('combobox'), 'formatting')
+    expect(screen.getByText('Changed heading')).toBeInTheDocument()
+    expect(screen.queryByText('This is inserted text')).not.toBeInTheDocument()
+  })
+
+  it('keeps changes with incomplete evidence or unprintable properties', () => {
+    const entries: Array<{ text: string; formatChange?: FormatChangeDetails }> = [
+      { text: 'No formatting evidence' },
+      { text: 'Missing old snapshot', formatChange: { newProperties: {} } },
+      { text: 'Missing new snapshot', formatChange: { oldProperties: {} } },
+      { text: 'Empty snapshots', formatChange: { oldProperties: {}, newProperties: {} } },
+      { text: 'Unmodeled run change', formatChange: { oldProperties: {}, newProperties: {}, changedPropertyNames: [] } },
+      { text: 'Unmodeled change beside equal bold', formatChange: { oldProperties: { bold: 'true' }, newProperties: { bold: 'true' }, changedPropertyNames: [] } },
+      { text: 'Table shell changed', formatChange: { oldProperties: {}, newProperties: {}, changedPropertyNames: ['shell'], scope: 'table' } },
+      { text: 'Border XML changed', formatChange: { oldProperties: { border: '<w:top w:val="single"/>' }, newProperties: { border: '<w:top w:val="double"/>' } } },
+    ]
+    const { container } = render(<RevisionPanel revisions={entries.map(entry => ({
+      ...mockRevisions[0], revisionType: 'FormatChanged', ...entry,
+    }))} />)
+    for (const { text } of entries) expect(screen.getByText(text)).toBeInTheDocument()
+    expect(screen.getByText('8 changes')).toBeInTheDocument()
+    expect(container.querySelector('.rdv-format-change')).not.toBeInTheDocument()
+  })
+
+  it('shows the empty state when arriving evidence proves the only revision unchanged', () => {
+    const revision = native({ type: 'format', family: 'properties_change', text: 'Same paragraph' })
+    const { rerender } = render(<RevisionPanel revisions={[revision]} />)
+    expect(screen.getByText('Same paragraph')).toBeInTheDocument()
+    rerender(<RevisionPanel revisions={[revision]} formatDetails={{ [revision.id]: {
+      oldProperties: { keepNext: 'false' }, newProperties: { keepNext: 'false' },
+    } }} />)
+    expect(screen.getByText('No tracked changes found in this document.')).toBeInTheDocument()
+    expect(screen.queryByText('Same paragraph')).not.toBeInTheDocument()
+  })
+
   it('displays revision count in stats', () => {
     render(<RevisionPanel revisions={mockRevisions} />)
     expect(screen.getByText('3 changes')).toBeInTheDocument()

src/components/RevisionPanel.tsx

@@ -12,7 +12,8 @@ export interface RevisionPanelProps {
   onReject?: (id: string) => ReturnType<Resolution>;
   onAcceptAll?: Resolution;
   onRejectAll?: Resolution;
-  /** Optional formatting evidence keyed by native revision id. Comparison records carry their own. */
+  /** Formatting snapshots by native revision id. Equal snapshots without changedPropertyNames
+   * are treated as unchanged; keep that metadata for unmodeled changes. Comparison records carry their own. */
   formatDetails?: Record<string, FormatChangeDetails>;
 }
 
@@ -62,7 +63,7 @@ function truncateText(text: string, maxLength: number = 150): string {
 }
 
 // Filter out raw XML values and clean up property names
-function isValidPropertyValue(value: string): boolean {
+function isValidPropertyValue(value: string | undefined): value is string {
   if (!value || typeof value !== 'string') return false;
   // Filter out raw XML data
   if (value.includes('<') || value.includes('xmlns') || value.includes('Unid=')) return false;
@@ -99,37 +100,24 @@ interface FormatChangeItem {
 function getFormatChanges(details?: FormatChangeDetails): FormatChangeItem[] {
   if (!details) return [];
   const { oldProperties, newProperties } = details;
-  const changes: FormatChangeItem[] = [];
-  const processedKeys = new Set<string>();
-
-  // Process old properties
-  if (oldProperties) {
-    for (const [key, value] of Object.entries(oldProperties)) {
-      if (!isValidPropertyValue(value)) continue;
-      processedKeys.add(key);
-      const newValue = newProperties?.[key];
-      changes.push({
-        property: formatPropertyName(key),
-        oldValue: formatPropertyValue(value),
-        newValue: newValue && isValidPropertyValue(newValue) ? formatPropertyValue(newValue) : undefined,
-      });
-    }
-  }
-
-  // Process new properties not in old
-  if (newProperties) {
-    for (const [key, value] of Object.entries(newProperties)) {
-      if (processedKeys.has(key)) continue;
-      if (!isValidPropertyValue(value)) continue;
-      changes.push({
-        property: formatPropertyName(key),
-        oldValue: undefined,
-        newValue: formatPropertyValue(value),
-      });
-    }
-  }
+  const keys = new Set([...Object.keys(oldProperties ?? {}), ...Object.keys(newProperties ?? {})]);
+  return [...keys].flatMap(key => {
+    const before = oldProperties?.[key], after = newProperties?.[key];
+    if (before === after) return [];
+    const oldValue = isValidPropertyValue(before) ? formatPropertyValue(before) : undefined;
+    const newValue = isValidPropertyValue(after) ? formatPropertyValue(after) : undefined;
+    return oldValue || newValue ? [{ property: formatPropertyName(key), oldValue, newValue }] : [];
+  });
+}
 
-  return changes;
+// Engine records carry changedPropertyNames, even [] for unmodeled changes.
+// Only infer a no-op from nonempty, equal snapshots supplied without that metadata.
+function isUnchangedFormat(details?: FormatChangeDetails): boolean {
+  if (!details?.oldProperties || !details.newProperties || details.changedPropertyNames !== undefined) return false;
+  const { oldProperties, newProperties } = details;
+  const keys = new Set([...Object.keys(oldProperties), ...Object.keys(newProperties)]);
+  return keys.size > 0 && [...keys].every(key => Object.hasOwn(oldProperties, key) && Object.hasOwn(newProperties, key)
+    && oldProperties[key] === newProperties[key]);
 }
 
 export function RevisionPanel({ revisions, onSelect, onAccept, onReject, onAcceptAll, onRejectAll, formatDetails }: RevisionPanelProps) {
@@ -148,10 +136,17 @@ export function RevisionPanel({ revisions, onSelect, onAccept, onReject, onAccep
   // Pair each revision with its stable index in the original array so that
   // filtering and expansion state stay tied to the revision itself, not to a
   // shifting position within the filtered subset.
+  const displayedRevisions = useMemo(() => revisions.flatMap((revision, index) => {
+    const details = 'family' in revision ? formatDetails?.[revision.id] : revision.formatChange;
+    const hasDiagnostic = 'family' in revision && revision.resolutionStatus !== 'supported';
+    if (isFormatChange(revision) && !hasDiagnostic && isUnchangedFormat(details)) return [];
+    const id = 'family' in revision ? revision.id : `${revision.leftAnchor ?? ''}:${revision.rightAnchor ?? ''}:${revision.revisionType}:${index}`;
+    return [{ revision, id, formatChanges: getFormatChanges(details) }];
+  }), [revisions, formatDetails]);
+
   const filteredRevisions = useMemo(() => {
-    const withIds = revisions.map((revision, index) => ({ revision, id: 'family' in revision ? revision.id : `${revision.leftAnchor ?? ''}:${revision.rightAnchor ?? ''}:${revision.revisionType}:${index}` }));
-    if (filter === 'all') return withIds;
-    return withIds.filter(({ revision }) => {
+    if (filter === 'all') return displayedRevisions;
+    return displayedRevisions.filter(({ revision }) => {
       switch (filter) {
         case 'insertions': return isInsertion(revision);
         case 'deletions': return isDeletion(revision);
@@ -161,16 +156,19 @@ export function RevisionPanel({ revisions, onSelect, onAccept, onReject, onAccep
         default: return true;
       }
     });
-  }, [revisions, filter]);
+  }, [displayedRevisions, filter]);
 
-  const stats = useMemo(() => ({
-    total: revisions.length,
-    insertions: revisions.filter(isInsertion).length,
-    deletions: revisions.filter(isDeletion).length,
-    moves: revisions.filter(isMove).length,
-    formatting: revisions.filter(isFormatChange).length,
-    structural: revisions.filter(isStructural).length,
-  }), [revisions]);
+  const stats = useMemo(() => {
+    const visible = displayedRevisions.map(({ revision }) => revision);
+    return {
+      total: visible.length,
+      insertions: visible.filter(isInsertion).length,
+      deletions: visible.filter(isDeletion).length,
+      moves: visible.filter(isMove).length,
+      formatting: visible.filter(isFormatChange).length,
+      structural: visible.filter(isStructural).length,
+    };
+  }, [displayedRevisions]);
 
   const toggleExpanded = (index: string) => {
     setExpandedIds((prev) => {
@@ -184,7 +182,7 @@ export function RevisionPanel({ revisions, onSelect, onAccept, onReject, onAccep
     });
   };
 
-  if (revisions.length === 0) {
+  if (displayedRevisions.length === 0) {
     return (
       <div className="rdv-revision-panel">
         <div className="rdv-revision-empty">
@@ -244,10 +242,9 @@ export function RevisionPanel({ revisions, onSelect, onAccept, onReject, onAccep
       </div>
 
       <div className="rdv-revision-list">
-        {filteredRevisions.map(({ revision, id }) => {
+        {filteredRevisions.map(({ revision, id, formatChanges }) => {
           const isExpanded = expandedIds.has(id);
           const needsTruncation = revision.text.length > 150;
-          const formatChanges = getFormatChanges('family' in revision ? formatDetails?.[revision.id] : revision.formatChange);
 
           return (
             <div