Skip to content

Complete XLSX style fidelity and reduce checkpoint overhead after Issue #8 #11

Description

@rastagan-git

Summary

Issue #8 was closed by #9 and #10, and the main failure mode from v0.3.1 is substantially improved: worksheets, formulas, cached formula values, notes, widths, and visible formatting survive the focused preservation fixture.

However, regression testing the merged main branch against a 271-row real-world workbook found one remaining XLSX fidelity defect, plus checkpoint-performance and provenance-version inconsistencies. The style defect means the workbook is still not strictly preserved and should be fixed before publishing the merged code as the next release.

Related work:

Tested commit: 7cbe48515f31c3c608f0ffc1879d626d0a151284

Environment

  • Windows
  • Python 3.11.9
  • pandas 3.0.3
  • openpyxl 3.1.5
  • installed from a wheel built from the tested commit into a clean virtual environment
  • 87 passed; Ruff lint and format checks passed

1. Original cells can silently acquire the wrong base style

Observed behavior

The output preserved row count, row order, original values, worksheets, formulas, fills, and other visible highlights. Nevertheless, 169 original cells changed from unspecified vertical alignment to vertical='center'.

Example for cell A3:

Source worksheet XML:

<c r="A3" s="4" t="s"><v>4</v></c>

Output worksheet XML:

<c r="A3" t="inlineStr"><is><t>n-Hexane</t></is></c>

The source and output xl/styles.xml files were byte-identical (same SHA-256: 45ebdbe98867191c00ea588fe15186bf6d33c8de98e05b8cddec4538ef2f720a). Because the rewritten cell no longer has an s attribute, Excel/openpyxl resolves it through style index 0, which has vertical centering in this workbook.

This is visually mild in the tested file, but it violates the guarantee that untouched original cells retain their formatting. Other workbooks may have a more consequential style at cellXfs[0].

Likely cause

_write_preserved_xlsx() saves the workbook through openpyxl, after which _build_preserved_package() places the rewritten worksheet XML into the original package while retaining the original styles.xml:

The worksheet style IDs produced by the openpyxl round-trip are therefore not always semantically compatible with the original style table. The existing ColumnDimension workaround protects new columns but does not preserve style references for every untouched source cell.

Expected behavior

  • Every untouched original cell should retain its effective font, fill, border, number format, protection, and alignment.
  • A cell with an explicit source style must not silently fall back to cellXfs[0] after the worksheet XML is replaced.
  • Preservation tests should include a workbook where the effective default-looking cell style is not index 0 and where index 0 has visibly different formatting.

2. Full-workbook checkpoints add substantial avoidable overhead

To isolate serialization cost, all 271 rows were skipped with --skip-pattern '^.*$'; therefore no provider or network request occurred.

--checkpoint-every 0:   10.319 seconds
--checkpoint-every 25: 31.522 seconds

The default checkpoint configuration was approximately 3.1 times slower. Each checkpoint currently calls write_table(), which performs another full fidelity-preserving XLSX round-trip.

On the normal cached PubChem regression run, disabling checkpoints completed in about 68.5 seconds. A default-checkpoint run had only reached the row-250 partial workbook when the external test harness stopped it after 124 seconds. It was progressing rather than deadlocked, but the checkpoint cost is large enough to resemble a hang in interactive use.

Expected behavior

  • Checkpoints should remain crash-safe without repeatedly performing the entire expensive package-preservation operation.
  • Add a serialization-only benchmark or regression fixture so checkpoint cost is visible.
  • Consider a lightweight progress journal or incremental checkpoint representation, followed by one final fidelity-preserving workbook write.

3. --no-odor still reports PUG REST + PUG-View

With --no-odor, no PUG-View odor request is made and the odor columns are correctly omitted. The resulting PubChem Version field still says PUG REST + PUG-View because PUBCHEM_VERSION is unconditional:

This overstates which PubChem interface contributed to the row's result.

Expected behavior

  • Report PUG REST when odor lookup is disabled or was not attempted.
  • Report PUG REST + PUG-View only when PUG-View actually contributed or was attempted, depending on the intended semantics.
  • Add explicit tests for both include_odor=True and include_odor=False provenance values.

Real-workbook regression results that passed

The same 271-row run otherwise behaved correctly:

  • statuses: ok=250, skipped=14, not_found=7
  • CAS resolution: unique=157, multiple=76, missing=16, skipped=14, not_evaluated=7, input_cas_confirmed=1
  • all C6 through C19 structural rows were skipped
  • skipped rows had blank retrieval timestamps
  • non-skipped rows had retrieval timestamps
  • --no-odor added no odor columns
  • original rows, order, values, and worksheet order were preserved
  • all resolved/candidate CAS values had valid checksums
  • candidate counts matched the parsed candidate lists
  • n-Hexane retained and confirmed its existing CAS 110-54-3

Suggested acceptance criteria

  • Untouched source-cell style semantics are identical before and after output, including workbooks with a non-trivial cellXfs[0].
  • Regression tests compare all original cells' values/formulas and effective styles, not only selected hand-built styled cells.
  • Existing worksheets, formulas, cached values, dimensions, panes, and relationships continue to pass the Preserve XLSX workbook fidelity #9 preservation tests.
  • Checkpointing no longer requires a complete expensive preserved-XLSX rewrite for every interval, or its cost is otherwise substantially reduced and covered by a benchmark.
  • --no-odor provenance does not claim PUG-View participation.
  • The project version is bumped from 0.3.1 before publishing these post-Preserve XLSX workbook fidelity and tighten CLI/provenance semantics #8 changes as a new release.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions