Skip to content

Wave 19: structural integrity — shift clamps, cache/equals fidelity, formula-record + bytes preservation, lint teeth - #436

Merged
arcaputo3 merged 19 commits into
mainfrom
wave19-structural
Jul 23, 2026
Merged

Wave 19: structural integrity — shift clamps, cache/equals fidelity, formula-record + bytes preservation, lint teeth#436
arcaputo3 merged 19 commits into
mainfrom
wave19-structural

Conversation

@arcaputo3

Copy link
Copy Markdown
Contributor

Burn-down wave 1 of 3 (plan of record 2026-07-22): the corruption/degradation class from the field-gotcha audit, run as five worktree-isolated TDD clusters with pipelined adversarial review (one rework round, structural cluster — 8 findings, all resolved and re-approved).

What's in it

Gates (all green)

  • ./mill __.compile 810/810, 0 errors, 0 non-exhaustive-match warnings
  • scalafmt CI form (reformatAll + checkFormatAll __.sources) — no drift
  • ./mill -i __.test 1028/1028 tasks, 4,818 test cases
  • XL_ROUNDTRIP_MIN_SUCCESS=500 generative round-trip law green
  • ./scripts/test-examples.sh + ./scripts/verify-skill-snippets.sh --local green

Field-repro replays (branch-built CLI, audit fixtures)

  • Full-height CF A1:XFD1048576 + insert-rows 2 2 → stays A1:XFD1048576; xl lint clean; openpyxl loads with zero warnings (previously the MultiCellRange warning)
  • Excel-stamped DV H5:H10H7:H12; autoFilter A5:D10A7:D12; Print_Area $A$1:$D$10$A$1:$D$12 (all previously inert)
  • B4 after insert: <f>A4*2</f><v>4</v> — single = in openpyxl, cache intact (previously ==A4*2, no <v>)
  • Array t="array" ref and dataTable t="dataTable" ref dt2D dtr r1 r2 + cached <v> survive an unrelated put (dataTable previously baked to a constant)
  • Over-max fixture (A144:XFD1048578) → xl lint reports [ref-out-of-bounds], exit 1

Integration notes

Two cross-cluster conflict resolutions (both semantic, both re-verified by the full suite): StructuralEditor.rewriteFormulas combines #427's equals-free/cache-preserving form with #430's FormulaKind threading (two cluster test pins updated to the merged semantics); XlsxWriter sheet-rels passthrough keeps #429's staleTableRels guard on #412's ctx.content accessor. One example updated for the 3-arg Formula pattern.

Follow-ups already filed: #435 (ca/aca attrs on Normal formulas; delete-band data-table degradation fidelity). Reviewer non-gating notes carried in the wave gaps list (delete-side cache staleness candidate, single-cell sqref emission shape).

Closes #412
Closes #413
Closes #414
Closes #427
Closes #428
Closes #429
Closes #430

🤖 Generated with Claude Code

arcaputo3 and others added 17 commits July 22, 2026 16:21
…ched <v>

The structural insert/delete rewrite printed every shifted formula with
includeEquals = true and discarded its cached value. The writer serializes
the stored expression verbatim, so '=A4*2' landed inside <f> (openpyxl
reads it back as '==A4*2') and every formula cell displayed blank in
cached-value viewers until a recalc.

Print with includeEquals = false (the CF path and CLI putf convention;
the reader's canonical model form is equals-free) and carry the cell's
cachedValue through every successful shift — a successful shift means
every reference survived, so the cache is still the Excel-valid display
value. The None branch still degrades to #REF! and drops the cache.

Refs #427

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…sertions

The 'writeStream: single-pass is faster than auto-detect' test compared
wall-clock elapsed times of the two write paths and asserted single-pass
wins within 1.5x — a scheduling race, not a correctness property. Under
2x CPU oversubscription plus disk thrash it failed 3 of 25 iterations
locally (e.g. single-pass 862ms vs auto-detect 412ms), matching the two
field flakes on CI and in wave 18's parallel run.

Replace it with two structural tests that pin what the timing was a
proxy for, with no timing comparison:

- 'single-pass with hint matches auto-detect output': with the hint
  equal to the true bounds, every ZIP entry (worksheet, shared strings,
  statics) is byte-identical between the two paths, and the data
  round-trips through readStream.
- 'hinted single-pass streams to destination with dimension up-front':
  a mid-stream probe (evalTap at row 50 of 100) observes the destination
  file already materializing while the source is still producing —
  writeStreamImpl writes static parts and the worksheet header before
  the body segment pulls any row — and a superset hint (A1:C200 over
  A1:A100 data) is emitted verbatim, which no detection pass could do;
  auto-detect on the same data emits the detected A1:A100 as contrast.

Both single-pass assertions fail when applied to the auto-detect path
(discrimination verified), and each new test passed 50 of 50 iterations
under the same load that broke the old one. Global-tmpdir assertions
were deliberately avoided: xl-cli also creates xl-stream-* temp files
in parallel test JVMs.

Refs #414

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Insert-side shifts had no upper bound: a full-height range (CF sqref
A1:XFD1048576, merges, DV) overflowed to row 1048578 / column XFE on
insert and Excel refused the file as corrupt. Same gap in the formula
shifter, which clamped only the lower bound.

Extract the span algebra into Sheet.shiftSpan — ONE clamp site shared by
merges, cf/dv envelopes, chart data refs (and the GH-429 consumers that
follow): an insert pins the span end at Row.MaxIndex0/Column.MaxIndex0
and drops a span whose start passes the edge. FormulaShifter.shiftPos /
shiftRangePos mirror it (SharedFormula.shiftedIndex bound): a single ref
pushed past the edge -> None -> #REF!; a range end clamps, a range start
past the edge voids the formula. Long intermediate math so pathological
counts cannot overflow.

Refs #428

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
XlsxReader.readFromBytes passed None where the path variant builds a
SourceHandle, so a workbook read from bytes and written back dropped every
preserved-but-unmodeled workbook child (externalReferences,
workbookProtection, pivotCaches, ...) and every unknown part (externalLinks,
pivotCache, media) — the #397 field symptom by other means.

The archive is already resident on a bytes read, so there is no streaming
excuse for the asymmetry:

- SourceContent (xl-core): OnDisk(path) | InMemory(bytes), the physical
  origin a SourceContext preserves from. SourceContext.sourcePath: Path
  becomes content: SourceContent (fromFile keeps its signature; fromContent
  added).
- SourceHandle becomes an enum; readFromBytesWithWarnings wraps a private
  clone of the caller's array (parse and context share the snapshot, so
  caller mutation can never desync them) and fingerprints it like a file.
- XlsxWriter: withZipFile(path) generalizes to withSourceZip(SourceContent)
  — ZipFile random access on disk, ZipInputStream scan in memory — behind a
  SourceZip adapter consumed by every preservation read (structure, styles
  metadata, worksheets, SST, rels, media hashing, copyPreservedPart);
  copyVerbatim verifies the fingerprint over the bytes and writes them.
- StyleIndex.fromWorkbookWithSource opens the archive from either source.

Law (XlsxReaderPassthroughSpec): write(readFromBytes(bytes(F))) is
byte-identical to write(read(F)) over a preservation-rich fixture
(externalReferences + workbookProtection + pivotCaches + externalLink +
pivotCacheDefinition + STORED media), both clean and after an identical
edit; caller mutation after the read cannot corrupt the write. The GH-397
lint acceptance now covers the bytes path too.

Refs #412

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…in, Content_Types registration, O(1) SAX mode, over-max ref detection

Four GH-413 extensions plus the GH-428 corruption-class detector, all
diagnostics-only (no reader/writer behavior changes):

- CT_Chartsheet / CT_Dialogsheet canonical child-order tables; sheet-class
  parts (worksheet/chartsheet/dialogsheet) share one scan pipeline keyed by
  the sheet rel type, including root-content and r:id checks
- externalLink parts: <externalBook r:id> resolved against the part's
  sibling .rels (UnresolvedRelId / WrongRelType), one more hop past the
  workbook-level externalReference check; new XmlUtil.relTypeExternalLinkPath
- MissingContentType: every present-and-referenced part (internal rel target
  of any .rels lint walks, including _rels/.rels) needs an Override or an
  extension Default; absent [Content_Types].xml is a single finding
- RefOutOfBounds: every ref=/sqref= token (dimension, mergeCells,
  conditionalFormatting, dataValidations, autoFilter, shared formulas, table
  parts) checked against row 1048576 / column XFD -- the GH-428
  insert-overflow class Excel refuses as corrupt; boundary-max stays clean so
  post-clamp writer output lints clean (verified against both field repros:
  insert-rows -> A1:XFD1048578 and insert-cols -> A1:XFE1048576)
- lintStream / lintStreamBytes: SAX scanning of sheet-class and table parts,
  O(1) memory in the row count; findings identical to DOM mode, pinned by a
  16-fixture parity suite

Refs #413
Refs #428

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…bles, autoFilter on structural edits

Excel stamps showInputMessage/showErrorMessage on virtually every
validation it writes, so nearly all real-world DVs fell out of the typed
model into Preserved — structurally inert, silently detaching from their
data on every insert/delete. Print areas, tables, and the sheet-level
autoFilter never shifted at all.

- Widen the DV typed model (GH-375 -> GH-429): DvKind gains Custom,
  AnyValue, Bounded(whole/decimal/date/time/textLength x operator x
  formula2); Rules gains DvMessages (prompt/error text, errorStyle) with
  OOXML-default defaults; withPrompt/withError combinators and
  whole/decimal/date/time/textLength/custom/anyValue factories.
- DataValidationCodec: widened whitelist + per-kind parse (operator only
  on bounded kinds; anything else still rides Preserved: xr:uid, imeMode,
  unknown types, corrupt sqref); emission in Excel stamp order with
  schema defaults omitted; message text through the new
  escapeXstringAttr (attr-value normalization eats raw LF/TAB/CR;
  law decodeXstring . escapeXstringAttr = id). planDvWrites unchanged —
  both CLEAN-gate sides use the same widened parser.
- SqrefShift (new): byte-surgical root-sqref rewrite for Preserved
  DV/CF payloads under structural edits — layered all-or-nothing guards
  (start-tag region only, exactly one whitespace-preceded sqref, every
  token parseable), per-token identity fast-path, Excel-shaped printing
  for changed tokens; wired into shiftAxis for both Preserved cases.
- Sheet.shiftAxis: pageSetup printArea/repeatRows shift through the
  GH-428 clamp (collapse clears the field); tables shift and retabulate
  on column edits (drop deleted columns, splice ColumnN-unique fresh
  columns, stamp their header cells; drop sub-minimum tables);
  autoFilter tri-state overlay (Ranged shifts, collapse -> Remove).
- AutoFilterState (new) lift-and-overlay: reader lifts a parseable
  @ref; writer's mergeAutoFilterElem replaces only @ref (children
  verbatim, identity fast-path on equal refs), Remove strips, None
  passthrough — applied in both fromDomainWithMetadata branches.
- StructuralEditor.rewriteDvFormulas: typed DV formulas shift through
  the same engine as CF/cell formulas (inline literals ride verbatim,
  fully-deleted sources -> #REF! text, cross-sheet sources tracked).
- api exports: DvOperator, DvErrorStyle, DvBoundedType, DvMessages,
  AutoFilterState.

Refs #429

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rvive rewrites

CellValue.Formula gains kind: FormulaKind (Normal | ArrayFormula | DataTable),
the GH-430 binding representation. Records are read per cell (no group
inference), re-emitted byte-exactly in CT_CellFormula schema order through the
shared FormulaKindCodec on every writer (DOM, OoxmlCell SAX, DirectSaxEmitter,
streaming), and survive DIRTY sheet regeneration — two-variable Data Table
interiors no longer bake to static grids on put.

- reader: WorksheetReader + SaxStreamingReader + SaxSingleCellReader recognize
  array/dataTable records leniently (junk attrs degrade visibly, never throw)
- writers: dataTable emits a childless <f .../> (new SaxWriter.emptyElement;
  StAX writeEmptyElement override) with the cached <v> seed intact
- evaluator: pinnedCache generalizes the GH-353 seam — DataTable caches pin
  (never parsed, never clobbered); cache write-backs use f.copy so kinds
  survive; DependencyGraph excludes DataTable nodes; StructuralEditor shifts
  ref/r1/r2 and degrades tearing edits (array->Normal text, dataTable->cached
  constant)
- CLI: {=...} braces + additive JSON formulaKind; putf rejects top-level
  TABLE( pointing at GH-419; copy materializes dataTable cells and pastes
  array anchors as plain formulas
- CellValue.dataTable smart constructor + FormulaKind api export prove the
  #419 authoring substrate (scripting-prelude probe)
- new corpus fixture formula-records.xlsx rides every round-trip/parity law;
  property law: forAll genFormulaKind write->read = id on both backends

Refs #430

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rels

Verifying the GH-429 collapse-drop path end-to-end exposed a writer hole:
when a sheet's LAST table vanished (structural collapse or removeTable),
fromDomainWithMetadata's tableParts fallback resurrected the preserved
<tableParts> element and the copied sheet rels kept a table Relationship
whose part no longer ships — our own reader refused the output ('Missing
table file') and Excel would demand repair.

Model-authoritative fix: the preserved tableParts element only rides
while sheet.tables is non-empty, and a staleTableRels guard forces the
rels merge path and drops table-type rels when the model has no tables
left. Pinned by an integration test asserting no tableParts element, no
table rels, no xl/tables/ part, and a clean re-read.

Refs #429

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The all-or-nothing guard in SqrefShift.shiftPayload checked
xml.startsWith("<" + rootLabel), but every real reader-produced
Preserved payload is canonicalized by CfCodec.preservedXml via
XmlUtil.compact, which prepends '<?xml ...?>\n' — so the guard failed
on every real file and DataValidation.Preserved /
ConditionalFormat.Preserved entries stayed structurally inert on
row/column edits (the exact GH-429 defect this rewrite claimed to fix).

shiftPayload now skips one optional '<?...?>' prologue plus following
whitespace, applies every existing guard to the element region behind
it, and splices the prologue back verbatim on rewrite; a mangled
prologue (no '?>') fails the root-label guard and rides unchanged like
any other guard failure. Guard semantics stay all-or-nothing.

Test realism repairs (each red on the prologue-blind guard, verified by
refutation):
- SqrefShiftSpec: reader-shaped shift (exact imeMode payload dumped
  from a field fixture), xmlns/xmlns:xr start-tag shift, identity
  (incl. full-column fixed point), refuse-matrix behind the prologue,
  mangled prologue, and all-drop -> None
- DataValidationSpec / ConditionalFormatSpec / StructuralCfSpec flipped
  tests now use declaration-prefixed payloads and pin that the
  declaration survives the rewrite byte-verbatim
- DataValidationCodecSpec commuting-square property derives payloads
  via CfCodec.preservedXml (the production shape) instead of
  Elem.toString, re-entering shifted payloads through a standalone
  parse like the emission path does
- StructuralWritePreservationSpec: end-to-end pin on a hand-built
  foreign fixture (imeMode DV D5:D6, unknown-type DV E5:E6, xr:uid CF
  block B5:B6): read -> insertRows(1,2) -> Preserved sqrefs move to
  D7:D8/E7:E8/B7:B8 with the rest of each payload byte-intact ->
  write -> re-read clean

Refs #429

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e preservation, shift clamps, SqrefShift for preserved payloads
…ray/dataTable records

Conflict resolution in StructuralEditor.rewriteFormulas: combined GH-427 semantics
(includeEquals=false, cachedValue preserved on successful shift) with GH-430's
FormulaKind threading (DataTable payload-geometry arm + shiftedArrayKind on the
general arm) — formula-records branched pre-#427 and carried the old form.
…eContext

Conflict resolution in XlsxWriter sheet-rels passthrough: kept GH-429's
!staleTableRels guard AND GH-412's ctx.content accessor.
The cluster wrote these pins against its pre-#427 base (equals-prefixed
re-print, cache discarded). Post-merge, shifted formulas re-print
equals-free and keep their caches. Refs #430 #427
@claude

claude Bot commented Jul 22, 2026

Copy link
Copy Markdown

Review — Wave 19: structural integrity

I focused the review on the highest-risk surface area: the byte-surgical SqrefShift rewrite, the edge-clamp span algebra, the merged StructuralEditor.rewriteFormulas (the #427/#430 conflict resolution), and the bytes-fidelity SourceContent path. Overall this is high-quality, unusually well-documented work — the scaladoc on SqrefShift, shiftSpan, and FormulaKind explains why each guard exists, and the test-file-to-source ratio (49 test files touched) is excellent. The commit messages are exemplary root-cause narratives.

Strengths

  • Overflow-safe clamp algebra. Sheet.shiftSpan and FormulaShifter.shiftPos/shiftRangePos do the intermediate arithmetic in Long and only narrow after the axisMax comparison, so a pathological count/delta can't wrap. Verified the full-height CF case by hand: A1:XFD1048576 + insert-rows 2 2 → start 0 < at stays, end 1048575+2 clamps to 1048575, i.e. unchanged — matches the PR's field replay. One shared clamp site (shiftSpan) for merges/CF/DV/print-area/tables is the right factoring.
  • SqrefShift guards are genuinely all-or-nothing. The prologue-skip fix (commit 44246a6) is a real bug the earlier form would have silently no-op'd on every reader-produced payload — good catch, and the refutation-tested specs pin it. The reliance on scala.xml canonicality (> and " escaped everywhere, so the first > ends the start tag) is sound given the payload always comes from CfCodec.preservedXml; the guard degrades safely to unchanged on any non-canonical input.
  • Bytes-fidelity is memory-safe. readFromBytesWithWarnings wraps a private bytes.clone() before unsafeWrapArray, so caller mutation after the read can't desync parse-vs-context, and SourceContent.rawArray only exposes the backing array of that private snapshot. Symmetry with the path read is law-tested byte-identical.
  • FormulaKindCodec is lenient-total — junk t/ref/r1/r2 degrade to None/dropped attrs, never throws, never invents a range. DV message text is XML-escaped by the elemOrdered serializer on top of the xstring layer, so no attribute injection.

Findings / discussion

  1. Data-table band-tear can silently drop cell content (StructuralEditor.scala:105). When a structural edit tears a DataTable interior, the cell degrades to cachedOpt.getOrElse(CellValue.Empty). If the record carried no cached <v>, the formula becomes Empty — the cell's content vanishes entirely on an insert/delete, with no warning surfaced to the caller. The degradation-to-cached-constant behavior is deliberate and documented (and del1/del2 fidelity is filed as ooxml: preserve ca/aca on plain formulas; set del1/del2 when a data-table input is deleted #435), but the no-cache → Empty corner is a quiet data loss. Consider emitting a warning/diagnostic (or a lint finding) when this path drops a formula, so it isn't invisible to a user doing a bulk row delete.

  2. Documented staleness in SqrefShift / AutoFilterState — worth a user-facing note. Absolute refs in un-rewritten Preserved formulas go stale below an edit, and AutoFilterState interior column edits misalign filterColumn@colId. Both are correctly documented in the scaladoc and are strictly better than the prior full-inertness. No change requested — just flagging that these belong in docs/LIMITATIONS.md (already touched in this PR — good).

  3. FormulaKindCodec.toAttrs fixed schema order vs. attribute sorting. The codec emits <f> attributes in CT_CellFormula schema order to reproduce Excel byte-for-byte, but CLAUDE.md notes XmlUtil.elem sorts attributes by name. Assuming the <f> emission bypasses the sorting path (the round-trip parity law would catch a mismatch), this is fine — I couldn't see a conflict, but it's the one spot where the "byte-for-byte" claim depends on emission not re-sorting. The parity suite covers it either way.

Verification note

I reviewed statically and did not independently re-run the suite; the PR reports 1028/1028 tasks / 4,818 cases green plus the generative round-trip law. The clamp and rewrite logic I traced by hand matches the field-repro assertions in the description.

Nice work — the #428 corruption class (A144:XFD1048578) closure and the equals-free/cache-preserving rewrite are meaningful correctness wins. My only substantive ask is surfacing finding #1 (silent no-cache data-table drop) rather than leaving it invisible.

The wave-19 docs described the integration-time cache-preservation
policy; 8c021a9 correctly replaced it with invalidation (shrinking-delete
aggregates and position-sensitive formulas make preserved caches
silently wrong). Refs #427
@arcaputo3
arcaputo3 merged commit aa6de9e into main Jul 23, 2026
4 checks passed
@arcaputo3
arcaputo3 deleted the wave19-structural branch July 23, 2026 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment