From ff60fd4f8d245e175fa9d27204669e73ade96dab Mon Sep 17 00:00:00 2001
From: Arnav Goel <arnavgoel2@gmail.com>
Date: Mon, 24 Aug 2026 23:59:18 -0400
Subject: [PATCH 1/4] fix(parser): keep style on strikethrough-only and
 underline-only cells
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

_extract_style() decided whether a cell was styled from a hand-written
attribute list that omitted font.strikethrough and font.underline, so a
cell whose only formatting was one of those had its entire CellStyle
discarded — taking the correctly-parsed font with it.

Compare against a default FontStyle instead of enumerating attributes.
Each _extract_* helper already returns None when nothing is set, so the
enumeration was redundant as well as lossy; dropping it also fixes fills
with only bg_color and alignments with only text_rotation/indent.

Also expose font_strikethrough on chunk cells in to_json(). Strikethrough
commonly marks deprecated or void rows, so downstream RAG consumers need
it alongside font_color/fill_color.

Fixes #17

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 src/excel_parser/parsers/cell_parser.py | 17 ++++++++++-----
 src/excel_parser/pipeline.py            |  7 +++++-
 tests/conftest.py                       | 21 ++++++++++++++++++
 tests/test_parsers.py                   | 29 +++++++++++++++++++++++++
 tests/test_pipeline.py                  | 25 +++++++++++++++++++++
 5 files changed, 93 insertions(+), 6 deletions(-)

diff --git a/src/excel_parser/parsers/cell_parser.py b/src/excel_parser/parsers/cell_parser.py
index c2d5bdb..30af848 100644
--- a/src/excel_parser/parsers/cell_parser.py
+++ b/src/excel_parser/parsers/cell_parser.py
@@ -30,6 +30,9 @@
 
 logger = logging.getLogger(__name__)
 
+# Sentinel used to detect fonts with no non-default attributes.
+_DEFAULT_FONT = FontStyle()
+
 # Regex to extract cell and range references from formulas
 # Matches: A1, $A$1, Sheet1!A1, Sheet1!$A$1:$B$10, 'Sheet Name'!A1
 _REF_RE = re.compile(
@@ -279,12 +282,16 @@ def _extract_style(self, cell: OpenpyxlCell) -> CellStyle | None:
         alignment = self._extract_alignment(cell)
         number_format = cell.number_format if cell.number_format != "General" else None
 
-        # Check if any style is non-default
+        # Check if any style is non-default. Compare against the default DTO
+        # instead of enumerating attributes: an attribute-by-attribute check
+        # silently drops cells whose only formatting is an unlisted attribute
+        # (e.g. strikethrough-only or underline-only cells). The _extract_*
+        # helpers already return None when nothing is set.
         has_style = any([
-            font and (font.bold or font.italic or font.name or font.size or font.color),
-            fill and fill.fg_color,
-            border and any([border.left, border.right, border.top, border.bottom]),
-            alignment and (alignment.horizontal or alignment.vertical or alignment.wrap_text),
+            font is not None and font != _DEFAULT_FONT,
+            fill is not None,
+            border is not None,
+            alignment is not None,
             number_format,
         ])
 
diff --git a/src/excel_parser/pipeline.py b/src/excel_parser/pipeline.py
index 797cd04..c4fcc9f 100644
--- a/src/excel_parser/pipeline.py
+++ b/src/excel_parser/pipeline.py
@@ -106,9 +106,13 @@ def _chunk_cells(chunk: ChunkDTO, workbook: WorkbookDTO) -> list[dict[str, Any]]
             # colors: font and fill from style
             font_color = None
             fill_color = None
+            # strikethrough commonly marks deprecated/void rows, so downstream
+            # consumers need it alongside the colors.
+            font_strikethrough = False
             if cell.style:
-                if cell.style.font and cell.style.font.color:
+                if cell.style.font:
                     font_color = cell.style.font.color
+                    font_strikethrough = cell.style.font.strikethrough
                 if cell.style.fill and cell.style.fill.fg_color:
                     fill_color = cell.style.fill.fg_color
             cells.append({
@@ -117,6 +121,7 @@ def _chunk_cells(chunk: ChunkDTO, workbook: WorkbookDTO) -> list[dict[str, Any]]
                 "formula": formula,
                 "font_color": font_color,
                 "fill_color": fill_color,
+                "font_strikethrough": font_strikethrough,
             })
     return cells
 
diff --git a/tests/conftest.py b/tests/conftest.py
index 28842ec..3fa2df5 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -41,6 +41,7 @@
     "freeze_panes_workbook",
     "wide_workbook",
     "styled_workbook",
+    "strikethrough_workbook",
     "assumptions_workbook",
     "hyperlink_workbook",
     "two_tables_vertical",
@@ -455,6 +456,26 @@ def wide_workbook(tmp_dir) -> Path:
     return path
 
 
+@pytest.fixture
+def strikethrough_workbook(tmp_dir) -> Path:
+    """Workbook whose only formatting on some cells is strikethrough/underline."""
+    path = tmp_dir / "strikethrough.xlsx"
+    wb = Workbook()
+    ws = wb.active
+    ws.title = "Sheet1"
+
+    ws["A1"] = "Normal"
+    ws["B1"] = "Strike"
+    ws["B1"].font = Font(strike=True)
+    ws["C1"] = "BoldStrike"
+    ws["C1"].font = Font(bold=True, strike=True, color="FF0000")
+    ws["D1"] = "Underline"
+    ws["D1"].font = Font(underline="single")
+
+    wb.save(path)
+    return path
+
+
 @pytest.fixture
 def styled_workbook(tmp_dir) -> Path:
     """Workbook with rich formatting: borders, fills, fonts, alignment."""
diff --git a/tests/test_parsers.py b/tests/test_parsers.py
index 836d15e..8572e69 100644
--- a/tests/test_parsers.py
+++ b/tests/test_parsers.py
@@ -289,6 +289,35 @@ def test_border_extracted(self, styled_workbook):
         assert a1.style.border is not None
 
 
+class TestStrikethroughOnlyStyles:
+    """Cells whose only non-default font attribute must keep their style."""
+
+    def test_strikethrough_only_cell_keeps_style(self, strikethrough_workbook):
+        result = WorkbookParser(path=strikethrough_workbook).parse()
+        b1 = result.sheets[0].get_cell(1, 2)
+        assert b1.style is not None
+        assert b1.style.font is not None
+        assert b1.style.font.strikethrough is True
+
+    def test_underline_only_cell_keeps_style(self, strikethrough_workbook):
+        result = WorkbookParser(path=strikethrough_workbook).parse()
+        d1 = result.sheets[0].get_cell(1, 4)
+        assert d1.style is not None
+        assert d1.style.font is not None
+        assert d1.style.font.underline == "single"
+
+    def test_strikethrough_with_other_attributes(self, strikethrough_workbook):
+        result = WorkbookParser(path=strikethrough_workbook).parse()
+        c1 = result.sheets[0].get_cell(1, 3)
+        assert c1.style.font.strikethrough is True
+        assert c1.style.font.bold is True
+
+    def test_unstruck_cell_reports_false(self, strikethrough_workbook):
+        result = WorkbookParser(path=strikethrough_workbook).parse()
+        a1 = result.sheets[0].get_cell(1, 1)
+        assert a1.style.font.strikethrough is False
+
+
 class TestWideSheet:
     """Test wide sheets with many columns."""
 
diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py
index 60ad4f8..8b196cb 100644
--- a/tests/test_pipeline.py
+++ b/tests/test_pipeline.py
@@ -108,6 +108,31 @@ def test_serializer_records(self, simple_workbook):
             assert "metadata" in entry
 
 
+class TestChunkCellStrikethrough:
+    """Strikethrough is exposed on chunk cells in to_json output."""
+
+    @staticmethod
+    def _cells_by_address(result):
+        data = result.to_json()
+        return {
+            cell["address"]: cell
+            for chunk in data["chunks"]
+            for cell in chunk.get("cells", [])
+        }
+
+    def test_struck_cell_flagged(self, strikethrough_workbook):
+        result = parse_workbook(path=strikethrough_workbook)
+        cells = self._cells_by_address(result)
+        assert cells["B1"]["font_strikethrough"] is True
+        assert cells["C1"]["font_strikethrough"] is True
+
+    def test_unstruck_cell_not_flagged(self, strikethrough_workbook):
+        result = parse_workbook(path=strikethrough_workbook)
+        cells = self._cells_by_address(result)
+        assert cells["A1"]["font_strikethrough"] is False
+        assert cells["D1"]["font_strikethrough"] is False
+
+
 class TestAssumptionsPipeline:
     """Test the pipeline on an assumptions/results workbook."""
 

From 108212c3b403299397ca8c6b3ae219ac24e681e1 Mon Sep 17 00:00:00 2001
From: Arnav Goel <arnavgoel2@gmail.com>
Date: Wed, 2 Sep 2026 14:41:07 -0400
Subject: [PATCH 2/4] fix(parser): keep merge invariants intact when regions
 overlap

Excel forbids overlapping merged regions, but malformed and
machine-generated files contain them, and _build_merge_lookup mapped
every cell of every region into one flat dict. A cell in an intersection
was written twice and the last region silently won, leaving the parse
output self-contradictory in two ways:

  * the loser's master was a slave whose master was never flagged, so
    every other cell in its region pointed at an unflagged master (M2)
  * a cell that was master of one region and slave of another came back
    flagged both (M4) and slave-with-no-master (M1), because CellParser
    pre-flags any openpyxl MergedCell as a slave

Resolve overlaps up front: accept regions first-come in reading order
and drop any that intersect an accepted one, reporting each drop as a
WARNING instead of losing it silently. Reading order makes the survivor
independent of the order the file happens to list ranges in, so the
workbook hash stays deterministic, and surviving regions keep their
original ordering so well-formed workbooks are bit-identical.

Make the merge lookup the single authority for the flags while here: it
now sets master/slave exclusively and clears CellParser's provisional
slave flag for merges we do not track, which also covers stale ranges
that openpyxl reports outside any declared region.

Found by pointing the corpus robustness suite at real workbooks: 6 of
112 violated the merge invariants, all of them from this cause.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 src/excel_parser/parsers/sheet_parser.py | 103 +++++++++++++++++++++--
 tests/conftest.py                        |  33 ++++++++
 tests/test_parsers.py                    |  72 ++++++++++++++++
 3 files changed, 203 insertions(+), 5 deletions(-)

diff --git a/src/excel_parser/parsers/sheet_parser.py b/src/excel_parser/parsers/sheet_parser.py
index 2819eb3..841db45 100644
--- a/src/excel_parser/parsers/sheet_parser.py
+++ b/src/excel_parser/parsers/sheet_parser.py
@@ -97,8 +97,10 @@ def parse(self) -> SheetDTO:
         # Extract properties first
         sheet.properties = self._extract_properties()
 
-        # Extract merged regions
-        sheet.merged_regions = self._extract_merges()
+        # Extract merged regions. Overlapping regions cannot be represented
+        # consistently (a cell would be master of one and slave of another),
+        # so drop the losers before anything downstream sees them.
+        sheet.merged_regions = self._resolve_overlapping_merges(sheet, self._extract_merges())
 
         # Build merge lookup for cell parsing
         merge_masters = self._build_merge_lookup(sheet.merged_regions)
@@ -195,17 +197,29 @@ def _extract_cells(
 
             cell_dto = self._cell_parser.parse(cell, computed_value)
 
-            # Annotate merge info
+            # Annotate merge info. CellParser pre-flags any openpyxl MergedCell
+            # as a slave without knowing its master, so the lookup is the single
+            # authority here: it must set master/slave exclusively and clear the
+            # provisional flag for merges we ended up not tracking.
             key = (cell.row, cell.column)
-            if key in merge_masters:
-                master_coord, row_span, col_span = merge_masters[key]
+            merge_info = merge_masters.get(key)
+            if merge_info is not None:
+                master_coord, row_span, col_span = merge_info
                 if master_coord.row == cell.row and master_coord.col == cell.column:
                     cell_dto.is_merged_master = True
+                    cell_dto.is_merged_slave = False
+                    cell_dto.merge_master = None
                     cell_dto.merge_extent = row_span
                     cell_dto.merge_col_extent = col_span
                 else:
                     cell_dto.is_merged_slave = True
+                    cell_dto.is_merged_master = False
                     cell_dto.merge_master = master_coord
+            elif cell_dto.is_merged_slave:
+                # openpyxl reported a MergedCell for a region we do not track
+                # (a dropped overlap, or a stale range): not a slave of anything.
+                cell_dto.is_merged_slave = False
+                cell_dto.merge_master = None
 
             if not cell_dto.is_empty or cell_dto.is_merged_slave or cell_dto.is_merged_master:
                 sheet.set_cell(cell_dto)
@@ -225,6 +239,85 @@ def _extract_merges(self) -> list[MergedRegion]:
             regions.append(MergedRegion(range=cell_range, master=master))
         return regions
 
+    @staticmethod
+    def _ranges_overlap(a: CellRange, b: CellRange) -> bool:
+        """True when two rectangular ranges share at least one cell."""
+        return not (
+            a.bottom_right.row < b.top_left.row
+            or b.bottom_right.row < a.top_left.row
+            or a.bottom_right.col < b.top_left.col
+            or b.bottom_right.col < a.top_left.col
+        )
+
+    def _resolve_overlapping_merges(
+        self, sheet: SheetDTO, regions: list[MergedRegion]
+    ) -> list[MergedRegion]:
+        """
+        Drop merged regions that overlap an already-accepted region.
+
+        Excel forbids overlapping merges, but malformed and machine-generated
+        files contain them. They cannot be represented consistently: a cell in
+        the intersection would be the master of one region and a slave of
+        another, so ``_build_merge_lookup``'s flat dict silently kept whichever
+        region came last and left the cell flagged both master and slave (or a
+        slave whose master was never flagged).
+
+        Resolution is first-come in reading order (top-left row, then column),
+        which is deterministic regardless of the order the file lists them in.
+        Dropped regions are reported as warnings rather than dropped silently.
+        """
+        if len(regions) < 2:
+            return regions
+
+        ordered = sorted(
+            range(len(regions)),
+            key=lambda i: (
+                regions[i].range.top_left.row,
+                regions[i].range.top_left.col,
+                regions[i].range.bottom_right.row,
+                regions[i].range.bottom_right.col,
+            ),
+        )
+
+        kept: list[int] = []
+        dropped: list[tuple[MergedRegion, MergedRegion]] = []
+        for i in ordered:
+            clash = next(
+                (k for k in kept if self._ranges_overlap(regions[i].range, regions[k].range)),
+                None,
+            )
+            if clash is None:
+                kept.append(i)
+            else:
+                dropped.append((regions[i], regions[clash]))
+
+        if not dropped:
+            return regions
+
+        for loser, winner in dropped:
+            sheet.errors.append(
+                ParseError(
+                    severity=Severity.WARNING,
+                    stage="parse",
+                    message=(
+                        f"Overlapping merged regions: {loser.range.to_a1()} overlaps "
+                        f"{winner.range.to_a1()}; {loser.range.to_a1()} was dropped"
+                    ),
+                    sheet_name=self._sheet_name,
+                )
+            )
+        logger.warning(
+            "Sheet %s: dropped %d of %d merged regions for overlapping",
+            self._sheet_name,
+            len(dropped),
+            len(regions),
+        )
+
+        # Preserve the file's original ordering among the surviving regions so
+        # non-overlapping workbooks are completely unaffected by this pass.
+        survivors = set(kept)
+        return [r for i, r in enumerate(regions) if i in survivors]
+
     def _build_merge_lookup(
         self, regions: list[MergedRegion]
     ) -> dict[tuple[int, int], tuple[CellCoord, int, int]]:
diff --git a/tests/conftest.py b/tests/conftest.py
index 3fa2df5..f6684db 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -42,6 +42,7 @@
     "wide_workbook",
     "styled_workbook",
     "strikethrough_workbook",
+    "overlapping_merges_workbook",
     "assumptions_workbook",
     "hyperlink_workbook",
     "two_tables_vertical",
@@ -476,6 +477,38 @@ def strikethrough_workbook(tmp_dir) -> Path:
     return path
 
 
+@pytest.fixture
+def overlapping_merges_workbook(tmp_dir) -> Path:
+    """
+    Workbook with merged regions that overlap each other.
+
+    Excel forbids this, but machine-generated files contain it. Both shapes
+    that broke the merge invariants are represented:
+
+    * ``B2:C5`` / ``C5:E8`` — C5 is the master of one region and a slave of
+      the other, so the region that lost the race left C5 flagged as a slave
+      whose master was never flagged (``M2``).
+    * ``H2:J5`` / ``I4:L7`` — I4 is a master and a slave simultaneously,
+      leaving it flagged both (``M4``) and slave-with-no-master (``M1``).
+    """
+    path = tmp_dir / "overlapping_merges.xlsx"
+    wb = Workbook()
+    ws = wb.active
+    ws.title = "Sheet1"
+
+    ws["A1"] = "Overlapping merge regions"
+    ws["B2"] = "first"
+    ws["C5"] = "second"
+    ws["H2"] = "third"
+    ws["I4"] = "fourth"
+
+    for rng in ("B2:C5", "C5:E8", "H2:J5", "I4:L7"):
+        ws.merge_cells(rng)
+
+    wb.save(path)
+    return path
+
+
 @pytest.fixture
 def styled_workbook(tmp_dir) -> Path:
     """Workbook with rich formatting: borders, fills, fonts, alignment."""
diff --git a/tests/test_parsers.py b/tests/test_parsers.py
index 8572e69..2b5d059 100644
--- a/tests/test_parsers.py
+++ b/tests/test_parsers.py
@@ -7,7 +7,11 @@
 """
 
 
+from openpyxl import Workbook
+
+from excel_parser.models.common import Severity
 from excel_parser.parsers import WorkbookParser
+from tests.helpers.invariant_checker import check_invariants
 
 
 class TestSimpleWorkbook:
@@ -87,6 +91,74 @@ def test_merge_slave_annotated(self, merged_cells_workbook):
         assert b1.is_merged_slave is True
 
 
+class TestOverlappingMerges:
+    """
+    Overlapping merged regions must not corrupt the merge invariants.
+
+    A flat (row, col) -> master lookup let a later region overwrite an
+    earlier one, so a cell in the intersection came out flagged both master
+    and slave, or a slave pointing at a master that was never flagged.
+    """
+
+    def test_invariants_hold(self, overlapping_merges_workbook):
+        result = WorkbookParser(path=overlapping_merges_workbook).parse()
+        assert check_invariants(result) == []
+
+    def test_no_cell_is_both_master_and_slave(self, overlapping_merges_workbook):
+        result = WorkbookParser(path=overlapping_merges_workbook).parse()
+        for sheet in result.sheets:
+            for cell in sheet.cells.values():
+                assert not (cell.is_merged_master and cell.is_merged_slave), (
+                    f"{cell.a1_ref} is both master and slave"
+                )
+
+    def test_every_slave_points_at_a_flagged_master(self, overlapping_merges_workbook):
+        result = WorkbookParser(path=overlapping_merges_workbook).parse()
+        for sheet in result.sheets:
+            for cell in sheet.cells.values():
+                if not cell.is_merged_slave:
+                    continue
+                assert cell.merge_master is not None, f"{cell.a1_ref} slave with no master"
+                master = sheet.get_cell(cell.merge_master.row, cell.merge_master.col)
+                assert master is not None and master.is_merged_master
+
+    def test_overlaps_are_dropped_not_silently_kept(self, overlapping_merges_workbook):
+        result = WorkbookParser(path=overlapping_merges_workbook).parse()
+        sheet = result.sheets[0]
+        # Four regions declared, two of them overlapping an earlier one.
+        assert len(sheet.merged_regions) == 2
+        kept = {r.range.to_a1() for r in sheet.merged_regions}
+        assert kept == {"B2:C5", "H2:J5"}
+
+    def test_dropped_overlaps_are_reported(self, overlapping_merges_workbook):
+        result = WorkbookParser(path=overlapping_merges_workbook).parse()
+        sheet = result.sheets[0]
+        warnings = [e for e in sheet.errors if "Overlapping merged regions" in e.message]
+        assert len(warnings) == 2
+        assert all(w.severity == Severity.WARNING for w in warnings)
+
+    def test_resolution_is_reading_order_not_file_order(self, tmp_dir):
+        """The surviving region is the upper-left one regardless of declaration order."""
+        path = tmp_dir / "reversed_overlap.xlsx"
+        wb = Workbook()
+        ws = wb.active
+        # Declared bottom-right first; B2:C5 must still be the survivor.
+        for rng in ("C5:E8", "B2:C5"):
+            ws.merge_cells(rng)
+        wb.save(path)
+
+        result = WorkbookParser(path=path).parse()
+        kept = {r.range.to_a1() for r in result.sheets[0].merged_regions}
+        assert kept == {"B2:C5"}
+
+    def test_non_overlapping_merges_are_untouched(self, merged_cells_workbook):
+        """The overlap pass must not perturb well-formed workbooks."""
+        result = WorkbookParser(path=merged_cells_workbook).parse()
+        sheet = result.sheets[0]
+        assert not [e for e in sheet.errors if "Overlapping" in e.message]
+        assert len(sheet.merged_regions) >= 2
+
+
 class TestEmptyMasterRecovery:
     """Test OOXML recovery of values from empty merge masters."""
 

From 8341e91834c6cf2dea1be258d876cf80acc0d3a9 Mon Sep 17 00:00:00 2001
From: Arnav Goel <arnavgoel2@gmail.com>
Date: Wed, 2 Sep 2026 14:41:07 -0400
Subject: [PATCH 3/4] test(corpus): make the corpus suite actually exercise the
 parser
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Every corpus source was dead, and the download tests asserted only
isinstance(files, list) while download_and_extract_xlsx swallowed
RequestException into a log warning. So they reported green for months
having fetched nothing:

  * 3x SheetJS test_files -> the submodule repo is blocked by GitHub
  * openpyxl genuine/empty.xlsx -> 404 (openpyxl is not on GitHub)
  * Zenodo EUSES.zip -> 404, record withdrawn
  * Enron xls dump -> 200, but 321 MB for ~0 .xlsx, as its own docstring
    predicted

tests/fixtures/corpus/ therefore stayed empty, and because corpus_files
is computed at import time the eight robustness tests that depend on it
skipped silently. The suite spent 5.5 minutes verifying nothing.

Repoint at sources chosen for durability over volume: sdists of
Excel-handling PyPI packages (published artifacts are immutable, so
these cannot rot) and upstream parser fixture directories enumerated
through the GitHub contents API, so a rename upstream costs one file
rather than the whole corpus. 80 files in 7.5s, down from 0 in 325s.

Assert on what arrived. A genuinely offline machine skips; a reachable
source returning nothing usable now fails. Members that are not real
XLSX containers are skipped at extraction: upstream suites ship a
zero-byte file and an encrypted OLE workbook under an .xlsx name, which
belong in targeted unit tests, not in a corpus asserting that
well-formed workbooks parse.

Split the two contracts the corpus was conflating. Upstream fixture
suites include files that are valid ZIPs but not valid OOXML packages
(missing [Content_Types].xml, missing sharedStrings.xml, a style
attribute openpyxl rejects). openpyxl cannot open them either, so
test_has_sheets was asserting the impossible; it now skips them, and
test_unloadable_files_report_an_error covers the contract that does
apply — degrade with a recorded ERROR rather than crash or come back
silently empty. test_success_rate's 95% threshold likewise measures the
loadable subset, so it tracks this parser's health rather than how many
negative fixtures upstream happens to ship.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 tests/helpers/corpus_downloader.py | 244 ++++++++++++++++++++---------
 tests/test_corpus_robustness.py    | 140 +++++++++++++----
 2 files changed, 284 insertions(+), 100 deletions(-)

diff --git a/tests/helpers/corpus_downloader.py b/tests/helpers/corpus_downloader.py
index 9cdb130..db37b98 100644
--- a/tests/helpers/corpus_downloader.py
+++ b/tests/helpers/corpus_downloader.py
@@ -1,16 +1,26 @@
 """
 Download real-world Excel corpora for robustness testing.
 
-Supports downloading .xlsx files from:
-- EUSES Spreadsheet Corpus (Zenodo)
-- Enron Spreadsheet Corpus (SheetJS GitHub)
-- Additional GitHub repositories with public xlsx samples
+Sources are chosen for durability over volume:
+
+- **PyPI sdists** of libraries that bundle .xlsx test fixtures. A published
+  PyPI artifact is immutable, so these downloads cannot rot; only a yanked
+  release would break them, and the version is resolved at run time.
+- **Upstream test-fixture directories** on GitHub, enumerated through the
+  contents API rather than a hand-written file list, so individual renames
+  upstream do not silently reduce the corpus to nothing.
+
+An earlier revision pointed at the SheetJS ``test_files`` submodule (now
+blocked by GitHub), the EUSES archive on Zenodo (record withdrawn) and the
+Enron ``.xls`` dump (321 MB, ~0 .xlsx). All four URLs 404'd or yielded
+nothing, which is why every downloader here returns the paths it actually
+obtained and the tests assert on that.
 """
 
 
-
 import io
 import logging
+import tarfile
 import zipfile
 from pathlib import Path
 
@@ -21,6 +31,57 @@
 # Timeout for HTTP requests
 _TIMEOUT = 60
 
+# Upstream fixture directories: (repo, ref, path). Enumerated via the GitHub
+# contents API. These are deliberately parser test-suites — they concentrate
+# the malformed, edge-case and adversarial workbooks that a robustness corpus
+# wants (missing dimensions, empty shared strings, merged ranges, pivots,
+# rich text, shared formulas, non-standard XML namespaces).
+_GITHUB_FIXTURE_DIRS: tuple[tuple[str, str, str], ...] = (
+    ("pandas-dev/pandas", "main", "pandas/tests/io/data/excel"),
+    ("tafia/calamine", "master", "tests"),
+)
+
+# PyPI packages whose sdists ship .xlsx fixtures.
+_PYPI_PACKAGES: tuple[str, ...] = (
+    "python-calamine",
+    "xlsx2csv",
+    "pyexcel-xlsx",
+    "excelrd",
+    "tablib",
+    "pyexcel",
+)
+
+
+def _is_safe_member(name: str) -> bool:
+    """Reject archive members that are not plain, in-tree .xlsx files."""
+    if not name.lower().endswith(".xlsx"):
+        return False
+    if name.startswith(("/", "__MACOSX", ".")):
+        return False
+    # Path traversal guard: no member may escape the extraction directory.
+    return ".." not in Path(name).parts
+
+
+def _write_member(target_dir: Path, name: str, data: bytes) -> Path | None:
+    """
+    Write one archive member into target_dir under its basename.
+
+    Members that are not real XLSX containers are skipped. Upstream test
+    suites ship negative fixtures under an .xlsx name — a zero-byte file, an
+    encrypted OLE workbook — which belong in targeted unit tests, not in a
+    corpus whose whole purpose is asserting that well-formed workbooks parse.
+    """
+    safe_name = Path(name).name
+    if not safe_name:
+        return None
+    if not zipfile.is_zipfile(io.BytesIO(data)):
+        logger.debug("Skipping %s: not an XLSX (ZIP) container", name)
+        return None
+    dest = target_dir / safe_name
+    if not dest.exists():
+        dest.write_bytes(data)
+    return dest
+
 
 def download_and_extract_xlsx(
     url: str,
@@ -28,45 +89,47 @@ def download_and_extract_xlsx(
     max_files: int = 50,
 ) -> list[Path]:
     """
-    Download a ZIP archive and extract .xlsx files.
+    Download a ZIP or tar.gz archive and extract the .xlsx files it contains.
 
-    Returns list of extracted .xlsx paths.
+    Returns the list of extracted .xlsx paths (empty if the download failed).
     """
     target_dir.mkdir(parents=True, exist_ok=True)
     files: list[Path] = []
 
     try:
         logger.info("Downloading %s ...", url)
-        resp = requests.get(url, timeout=_TIMEOUT, stream=True)
+        resp = requests.get(url, timeout=_TIMEOUT)
         resp.raise_for_status()
         content = resp.content
-
-        if not zipfile.is_zipfile(io.BytesIO(content)):
-            logger.warning("URL did not return a ZIP file: %s", url)
-            return files
-
-        with zipfile.ZipFile(io.BytesIO(content)) as zf:
-            xlsx_names = [
-                n for n in zf.namelist()
-                if n.lower().endswith(".xlsx")
-                and not n.startswith("__MACOSX")
-                and not n.startswith(".")
-            ]
-            logger.info("Found %d .xlsx files in archive", len(xlsx_names))
-
-            for name in xlsx_names[:max_files]:
-                safe_name = Path(name).name
-                if not safe_name:
-                    continue
-                dest = target_dir / safe_name
-                if not dest.exists():
-                    dest.write_bytes(zf.read(name))
-                files.append(dest)
-
     except requests.RequestException as e:
         logger.warning("Failed to download %s: %s", url, e)
-    except zipfile.BadZipFile as e:
-        logger.warning("Bad ZIP from %s: %s", url, e)
+        return files
+
+    buf = io.BytesIO(content)
+    try:
+        if zipfile.is_zipfile(buf):
+            buf.seek(0)
+            with zipfile.ZipFile(buf) as zf:
+                names = [n for n in zf.namelist() if _is_safe_member(n)]
+                logger.info("Found %d .xlsx files in %s", len(names), url)
+                for name in names[:max_files]:
+                    dest = _write_member(target_dir, name, zf.read(name))
+                    if dest is not None:
+                        files.append(dest)
+        else:
+            buf.seek(0)
+            with tarfile.open(fileobj=buf, mode="r:*") as tf:
+                members = [m for m in tf.getmembers() if m.isfile() and _is_safe_member(m.name)]
+                logger.info("Found %d .xlsx files in %s", len(members), url)
+                for member in members[:max_files]:
+                    handle = tf.extractfile(member)
+                    if handle is None:
+                        continue
+                    dest = _write_member(target_dir, member.name, handle.read())
+                    if dest is not None:
+                        files.append(dest)
+    except (zipfile.BadZipFile, tarfile.TarError) as e:
+        logger.warning("Unreadable archive from %s: %s", url, e)
 
     return files
 
@@ -87,70 +150,111 @@ def download_single_xlsx(
     try:
         resp = requests.get(url, timeout=_TIMEOUT)
         resp.raise_for_status()
-        dest.write_bytes(resp.content)
-        return dest
     except requests.RequestException as e:
         logger.warning("Failed to download %s: %s", url, e)
         return None
 
+    if not zipfile.is_zipfile(io.BytesIO(resp.content)):
+        logger.debug("Skipping %s: not an XLSX (ZIP) container", url)
+        return None
+    dest.write_bytes(resp.content)
+    return dest
+
+
+def _list_github_xlsx(repo: str, ref: str, path: str) -> list[tuple[str, str]]:
+    """
+    List .xlsx files in a GitHub directory as (raw_url, filename) pairs.
+
+    Uses the contents API so an upstream rename shrinks the corpus by one file
+    instead of breaking a hard-coded URL list.
+    """
+    api = f"https://api.github.com/repos/{repo}/contents/{path}?ref={ref}"
+    try:
+        resp = requests.get(api, timeout=_TIMEOUT, headers={"Accept": "application/vnd.github+json"})
+        resp.raise_for_status()
+        entries = resp.json()
+    except (requests.RequestException, ValueError) as e:
+        logger.warning("Could not list %s/%s: %s", repo, path, e)
+        return []
+
+    if not isinstance(entries, list):
+        # A submodule or a file resolves to a dict, not a listing.
+        logger.warning("%s/%s is not a directory listing", repo, path)
+        return []
+
+    out: list[tuple[str, str]] = []
+    for entry in entries:
+        name = entry.get("name", "")
+        download_url = entry.get("download_url")
+        if entry.get("type") == "file" and name.lower().endswith(".xlsx") and download_url:
+            out.append((download_url, name))
+    return out
+
 
 def download_github_xlsx_samples(
     target_dir: Path,
     max_files: int = 50,
 ) -> list[Path]:
-    """
-    Download diverse .xlsx samples from known public GitHub repos.
-
-    These are individual files from repos known to contain xlsx samples.
-    """
+    """Download .xlsx fixtures from upstream parser test suites on GitHub."""
     target_dir.mkdir(parents=True, exist_ok=True)
     files: list[Path] = []
 
-    # Known public xlsx sample URLs (raw GitHub links)
-    sample_urls = [
-        # SheetJS test files
-        ("https://raw.githubusercontent.com/SheetJS/sheetjs/master/test_files/comments_stress_test.xlsx", "comments_stress.xlsx"),
-        ("https://raw.githubusercontent.com/SheetJS/sheetjs/master/test_files/merge_cells.xlsx", "merge_cells.xlsx"),
-        ("https://raw.githubusercontent.com/SheetJS/sheetjs/master/test_files/number_format.xlsx", "number_format.xlsx"),
-        # openpyxl test fixtures
-        ("https://raw.githubusercontent.com/openpyxl/openpyxl/master/openpyxl/tests/data/genuine/empty.xlsx", "genuine_empty.xlsx"),
-    ]
-
-    for url, fname in sample_urls:
+    for repo, ref, path in _GITHUB_FIXTURE_DIRS:
         if len(files) >= max_files:
             break
-        path = download_single_xlsx(url, target_dir, fname)
-        if path:
-            files.append(path)
+        for url, fname in _list_github_xlsx(repo, ref, path):
+            if len(files) >= max_files:
+                break
+            # Namespace by repo: pandas and calamine both ship merge_cells.xlsx.
+            prefix = repo.split("/")[-1]
+            dest = download_single_xlsx(url, target_dir, f"{prefix}_{fname}")
+            if dest is not None:
+                files.append(dest)
 
     return files
 
 
-def download_euses_corpus(
+def download_pypi_corpus(
     target_dir: Path,
     max_files: int = 50,
 ) -> list[Path]:
     """
-    Download EUSES corpus from Zenodo and extract .xlsx files.
+    Download .xlsx fixtures from the sdists of Excel-handling PyPI packages.
 
-    Note: EUSES is mostly .xls files. The .xlsx subset may be small.
+    PyPI artifacts are immutable, making this the most durable of the sources.
     """
-    url = "https://zenodo.org/records/581673/files/EUSES.zip"
-    return download_and_extract_xlsx(url, target_dir, max_files)
+    target_dir.mkdir(parents=True, exist_ok=True)
+    files: list[Path] = []
 
+    for package in _PYPI_PACKAGES:
+        if len(files) >= max_files:
+            break
+        sdist_url = _resolve_pypi_sdist(package)
+        if sdist_url is None:
+            continue
+        files.extend(
+            download_and_extract_xlsx(sdist_url, target_dir, max_files=max_files - len(files))
+        )
 
-def download_enron_corpus(
-    target_dir: Path,
-    max_files: int = 50,
-) -> list[Path]:
-    """
-    Download Enron spreadsheets from SheetJS GitHub repo.
+    return files
 
-    Note: Enron files are almost entirely .xls (pre-2007).
-    The .xlsx subset will likely be empty or very small.
-    """
-    url = "https://github.com/SheetJS/enron_xls/archive/refs/heads/master.zip"
-    return download_and_extract_xlsx(url, target_dir, max_files)
+
+def _resolve_pypi_sdist(package: str) -> str | None:
+    """Resolve the sdist download URL for a package's current release."""
+    try:
+        resp = requests.get(f"https://pypi.org/pypi/{package}/json", timeout=_TIMEOUT)
+        resp.raise_for_status()
+        payload = resp.json()
+    except (requests.RequestException, ValueError) as e:
+        logger.warning("Could not resolve sdist for %s: %s", package, e)
+        return None
+
+    for url_entry in payload.get("urls", []):
+        if url_entry.get("packagetype") == "sdist":
+            return url_entry.get("url")
+
+    logger.warning("No sdist published for %s", package)
+    return None
 
 
 def get_corpus_files(corpus_dir: Path) -> list[Path]:
diff --git a/tests/test_corpus_robustness.py b/tests/test_corpus_robustness.py
index 974b19b..f7de3a0 100644
--- a/tests/test_corpus_robustness.py
+++ b/tests/test_corpus_robustness.py
@@ -11,16 +11,18 @@
 
 
 import json
+import zipfile
 from pathlib import Path
 
+import openpyxl
 import pytest
+import requests
 
 from excel_parser.models.common import Severity
 from excel_parser.pipeline import parse_workbook
 from tests.helpers.corpus_downloader import (
-    download_enron_corpus,
-    download_euses_corpus,
     download_github_xlsx_samples,
+    download_pypi_corpus,
     get_corpus_files,
 )
 from tests.helpers.invariant_checker import check_invariants
@@ -40,6 +42,30 @@ def _collect_corpus_files() -> list[Path]:
 
 corpus_files = _collect_corpus_files()
 
+_loadable_cache: dict[Path, bool] = {}
+
+
+def _is_loadable(path: Path) -> bool:
+    """
+    True when openpyxl can open the file at all.
+
+    Used to separate "the parser mishandled a workbook" from "this file is not
+    a workbook any reader can open". Cached because the corpus tests re-parse
+    each file per assertion.
+    """
+    cached = _loadable_cache.get(path)
+    if cached is not None:
+        return cached
+
+    loadable = True
+    try:
+        wb = openpyxl.load_workbook(path, read_only=True)
+        wb.close()
+    except Exception:
+        loadable = False
+    _loadable_cache[path] = loadable
+    return loadable
+
 
 # ---------------------------------------------------------------------------
 # Corpus download tests (require network)
@@ -48,23 +74,46 @@ def _collect_corpus_files() -> list[Path]:
 
 @pytest.mark.corpus
 class TestCorpusDownload:
-    """Download corpus files. Run once, then corpus tests use the files."""
+    """
+    Download corpus files. Run once, then corpus tests use the files.
+
+    These assert that files actually arrived. An earlier revision asserted
+    only ``isinstance(files, list)``, which stayed green for months while
+    every upstream URL 404'd and the corpus directory sat empty — silently
+    skipping the eight robustness tests that depend on it. A genuinely
+    offline machine is a skip; a reachable source returning nothing usable
+    is a failure.
+    """
+
+    @staticmethod
+    def _require_network() -> None:
+        try:
+            requests.head("https://pypi.org/simple/", timeout=15)
+        except requests.RequestException as e:
+            pytest.skip(f"network unavailable: {e}")
+
+    def test_download_pypi_corpus(self):
+        self._require_network()
+        target = CORPUS_DIR / "pypi"
+        files = download_pypi_corpus(target, max_files=40)
+        assert files, "no .xlsx fixtures extracted from any PyPI sdist"
+        assert all(f.exists() and f.stat().st_size > 0 for f in files)
 
     def test_download_github_samples(self):
+        self._require_network()
         target = CORPUS_DIR / "github_samples"
-        files = download_github_xlsx_samples(target, max_files=20)
-        # Some URLs may fail; just ensure we tried
-        assert isinstance(files, list)
+        files = download_github_xlsx_samples(target, max_files=40)
+        if not files:
+            pytest.skip("GitHub fixture listing unavailable (rate limit or upstream move)")
+        assert all(f.exists() and f.stat().st_size > 0 for f in files)
 
-    def test_download_euses(self):
-        target = CORPUS_DIR / "euses"
-        files = download_euses_corpus(target, max_files=50)
-        assert isinstance(files, list)
-
-    def test_download_enron(self):
-        target = CORPUS_DIR / "enron"
-        files = download_enron_corpus(target, max_files=50)
-        assert isinstance(files, list)
+    def test_downloaded_files_are_parseable_xlsx(self):
+        """A download that yields unopenable bytes is worse than no download."""
+        self._require_network()
+        files = download_pypi_corpus(CORPUS_DIR / "pypi", max_files=5)
+        assert files, "no .xlsx fixtures extracted from any PyPI sdist"
+        for path in files[:5]:
+            assert zipfile.is_zipfile(path), f"{path.name} is not a valid xlsx container"
 
 
 # ---------------------------------------------------------------------------
@@ -91,11 +140,34 @@ def test_no_unhandled_exception(self, xlsx_path):
             pytest.fail(f"Parser crashed on {xlsx_path.name}: {e}")
 
     def test_has_sheets(self, xlsx_path):
+        """
+        A workbook that can be opened at all must yield at least one sheet.
+
+        Upstream parser test-suites contribute files that are valid ZIPs but
+        not valid OOXML packages (a missing ``[Content_Types].xml``, a missing
+        ``sharedStrings.xml``, a style attribute openpyxl rejects). No reader
+        can open those, so demanding a sheet from them asserts the impossible;
+        the contract that actually matters for them — degrade with a recorded
+        error instead of crashing — is covered by
+        ``test_unloadable_files_report_an_error``.
+        """
+        if not _is_loadable(xlsx_path):
+            pytest.skip(f"{xlsx_path.name} is not a loadable OOXML package")
         result = parse_workbook(path=xlsx_path)
         assert len(result.workbook.sheets) >= 1, (
             f"{xlsx_path.name}: no sheets parsed"
         )
 
+    def test_unloadable_files_report_an_error(self, xlsx_path):
+        """An unopenable workbook must say so, not come back silently empty."""
+        if _is_loadable(xlsx_path):
+            pytest.skip(f"{xlsx_path.name} loads fine")
+        result = parse_workbook(path=xlsx_path)
+        assert result.workbook.errors, (
+            f"{xlsx_path.name}: failed to load but recorded no error"
+        )
+        assert any(e.severity == Severity.ERROR for e in result.workbook.errors)
+
     def test_workbook_hash_present(self, xlsx_path):
         result = parse_workbook(path=xlsx_path)
         assert result.workbook.workbook_hash, (
@@ -132,25 +204,33 @@ class TestCorpusAggregateStats:
     """Aggregate statistics across the whole corpus."""
 
     def test_success_rate(self):
-        """At least 95% of corpus files should parse without ERROR-level errors."""
-        total = len(corpus_files)
-        errors = 0
-        for path in corpus_files:
+        """
+        At least 95% of *loadable* corpus files parse without ERROR-level errors.
+
+        The denominator is deliberately the loadable subset. Sources include
+        upstream parser test-suites, which seed the corpus with files that are
+        intentionally broken; counting those as parser failures would make the
+        threshold a measure of how many negative fixtures upstream happens to
+        ship rather than of this parser's health.
+        """
+        loadable = [p for p in corpus_files if _is_loadable(p)]
+        total = len(loadable)
+        if total == 0:
+            pytest.skip("no loadable corpus files")
+
+        failures = []
+        for path in loadable:
             try:
                 result = parse_workbook(path=path)
-                has_errors = any(
-                    e.severity == Severity.ERROR
-                    for e in result.workbook.errors
-                )
-                if has_errors:
-                    errors += 1
-            except Exception:
-                errors += 1
+                if any(e.severity == Severity.ERROR for e in result.workbook.errors):
+                    failures.append(path.name)
+            except Exception as e:
+                failures.append(f"{path.name} ({type(e).__name__}: {e})")
 
-        rate = (total - errors) / total if total > 0 else 1.0
+        rate = (total - len(failures)) / total
         assert rate >= 0.95, (
-            f"Success rate {rate:.1%} ({total - errors}/{total}) "
-            f"below 95% threshold"
+            f"Success rate {rate:.1%} ({total - len(failures)}/{total}) "
+            f"below 95% threshold; failures: {failures[:10]}"
         )
 
     def test_aggregate_stats(self):

From 5d748f00acc176d5ae95714cf16f57f3e517e0e2 Mon Sep 17 00:00:00 2001
From: Arnav Goel <arnavgoel2@gmail.com>
Date: Wed, 2 Sep 2026 14:59:57 -0400
Subject: [PATCH 4/4] fix(parser): preserve formatting on cells that hold no
 value
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two defects compounded here, and each hid the other.

_has_meaningful_style() enumerated three font attributes, so it missed
strikethrough, underline, font size and font name — the same blind spot
just fixed in _extract_style, in the one place where a false negative
discards data outright. It was also wrong in the opposite direction:
building a partial Font leaves `color` as None while the inherited
default font has one, so an explicitly formatted cell looked *less*
styled than an untouched neighbour, and every untouched cell reported
as styled.

Ask openpyxl instead of re-deriving it. A cell's format is recorded as
non-zero style ids for exactly the aspects that differ from the workbook
default, which is both precise and immune to growing another blind spot
as style support widens. Verified against one cell per aspect plus an
untouched control: correct on all ten, where comparing resolved style
objects to fresh defaults is not (a loaded default font does not equal
Font()).

The predicate was moot regardless: it spared a valueless cell from the
early `continue`, and then the gate that stores cells dropped anything
is_empty. So styling on empty cells was parsed and thrown away. That
gate now keeps a cell that carries a style. Strikethrough or a fill on
an otherwise blank row is precisely the "deprecated / void" marker that
motivated #17, and dropping it silently is the same data loss one layer
down.

Correcting the predicate also removed an accident the old one was
holding up. A merge master arrives as an ordinary Cell, not a
MergedCell, so the empty-cell skip never exempted it; an empty master
survived only because the broken predicate called every untouched cell
styled. Reporting those cells honestly started dropping empty masters,
stranding every slave in the region with a merge_master that no longer
existed — 348 M2 violations across 44 corpus workbooks, none of which
the default suite could see. The skip now spares any cell taking part
in a merge, via the merge lookup for the master.

Costs 3.6% more cells across the 192-workbook corpus (9,013 on
251,376). Cells kept this way still report is_empty, so nothing
mistakes them for data.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 src/excel_parser/parsers/sheet_parser.py |  65 ++++++++++----
 tests/conftest.py                        |  30 +++++++
 tests/test_parsers.py                    | 104 +++++++++++++++++++++++
 3 files changed, 182 insertions(+), 17 deletions(-)

diff --git a/src/excel_parser/parsers/sheet_parser.py b/src/excel_parser/parsers/sheet_parser.py
index 841db45..2412916 100644
--- a/src/excel_parser/parsers/sheet_parser.py
+++ b/src/excel_parser/parsers/sheet_parser.py
@@ -38,6 +38,11 @@
 
 logger = logging.getLogger(__name__)
 
+# openpyxl style-array slots that hold a non-zero id when the cell's format
+# differs from the workbook default. These are the five aspects the parser
+# turns into a CellStyle; see SheetParser._has_meaningful_style.
+_STYLED_STYLE_SLOTS = ("fontId", "fillId", "borderId", "numFmtId", "alignmentId")
+
 
 class SheetParser:
     """
@@ -175,10 +180,14 @@ def _extract_cells(
 
             # Skip truly empty cells (no value, no formula, no style worth capturing)
             if cell.value is None and cell.data_type != "f" and not self._has_meaningful_style(cell):
-                # But still capture merged slaves
+                # But never drop a cell taking part in a merge. Slaves arrive as
+                # openpyxl MergedCell instances; the master is an ordinary Cell
+                # and can be empty, so it has to be recognised via the merge
+                # lookup. Dropping it strands every slave in the region with a
+                # merge_master that does not exist.
                 from openpyxl.cell.cell import MergedCell as MergedCellType
 
-                if not isinstance(cell, MergedCellType):
+                if not isinstance(cell, MergedCellType) and (cell.row, cell.column) not in merge_masters:
                     continue
 
             # Get computed value. Prefer the calamine-provided dict (populated
@@ -221,7 +230,18 @@ def _extract_cells(
                 cell_dto.is_merged_slave = False
                 cell_dto.merge_master = None
 
-            if not cell_dto.is_empty or cell_dto.is_merged_slave or cell_dto.is_merged_master:
+            # Keep a valueless cell when it carries formatting. Without this
+            # the _has_meaningful_style skip above was inert: it spared the
+            # cell from the early `continue` only for this gate to drop it,
+            # so styling on empty cells was parsed and then thrown away.
+            # Strikethrough or a fill on an otherwise blank row is exactly the
+            # "this is deprecated" marker downstream consumers need to see.
+            if (
+                not cell_dto.is_empty
+                or cell_dto.is_merged_slave
+                or cell_dto.is_merged_master
+                or cell_dto.style is not None
+            ):
                 sheet.set_cell(cell_dto)
                 cell_count += 1
 
@@ -761,17 +781,28 @@ def _update_master_cell(sheet: SheetDTO, master_coord: CellCoord, value: Any) ->
 
     @staticmethod
     def _has_meaningful_style(cell) -> bool:
-        """Check if a cell has non-default styling worth preserving."""
-        try:
-            if cell.font and (cell.font.bold or cell.font.italic or cell.font.color):
-                return True
-            if cell.fill and cell.fill.patternType and cell.fill.patternType != "none":
-                return True
-            if cell.border:
-                for side in ("left", "right", "top", "bottom"):
-                    s = getattr(cell.border, side, None)
-                    if s and s.style:
-                        return True
-        except Exception:
-            pass
-        return False
+        """
+        Whether a valueless cell carries formatting worth keeping.
+
+        This gates the skip for empty cells, so a false negative discards
+        formatting outright. The previous implementation enumerated three font
+        attributes (bold/italic/color) and was wrong in both directions: it
+        dropped cells whose only formatting was strikethrough, underline, a
+        font size or a font name, and it kept every *untouched* cell, because
+        an explicitly-built partial ``Font`` leaves ``color`` as None while the
+        inherited default font has one.
+
+        openpyxl already records which aspects of a cell's format differ from
+        the workbook default, as non-zero style ids, so read that instead of
+        re-deriving it attribute by attribute — the enumeration is what grew
+        the blind spot. The slots checked are exactly the five aspects
+        ``_extract_style`` can turn into a CellStyle; protection and
+        quotePrefix are excluded because they carry nothing for an empty cell.
+        """
+        # openpyxl exposes ``has_style`` but not the ids behind it, hence
+        # ``_style``; guarded so a future rename degrades to "not styled"
+        # rather than raising mid-parse.
+        style = getattr(cell, "_style", None)
+        if style is None:
+            return False
+        return any(getattr(style, slot, 0) for slot in _STYLED_STYLE_SLOTS)
diff --git a/tests/conftest.py b/tests/conftest.py
index f6684db..9dd7c65 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -43,6 +43,7 @@
     "styled_workbook",
     "strikethrough_workbook",
     "overlapping_merges_workbook",
+    "styled_empty_cells_workbook",
     "assumptions_workbook",
     "hyperlink_workbook",
     "two_tables_vertical",
@@ -477,6 +478,35 @@ def strikethrough_workbook(tmp_dir) -> Path:
     return path
 
 
+@pytest.fixture
+def styled_empty_cells_workbook(tmp_dir) -> Path:
+    """
+    Workbook whose formatting lives on cells that hold no value.
+
+    One cell per style aspect, so a predicate that enumerates attributes
+    instead of asking openpyxl fails on whichever aspect it forgot. A6 is the
+    control: touched by nothing, and must stay out of the parse output.
+    """
+    path = tmp_dir / "styled_empty.xlsx"
+    wb = Workbook()
+    ws = wb.active
+    ws.title = "Sheet1"
+
+    ws["A1"] = "anchor"  # keeps the sheet's used range honest
+    ws["B1"].font = Font(strike=True)
+    ws["B2"].font = Font(underline="single")
+    ws["B3"].font = Font(bold=True)
+    ws["B4"].font = Font(size=20)
+    ws["B5"].font = Font(name="Courier New")
+    ws["C1"].number_format = "0.00%"
+    ws["C2"].alignment = Alignment(horizontal="center")
+    ws["C3"].fill = PatternFill("solid", fgColor="FFFF00")
+    ws["C4"].border = Border(left=Side(style="thin"))
+
+    wb.save(path)
+    return path
+
+
 @pytest.fixture
 def overlapping_merges_workbook(tmp_dir) -> Path:
     """
diff --git a/tests/test_parsers.py b/tests/test_parsers.py
index 2b5d059..d8a55c5 100644
--- a/tests/test_parsers.py
+++ b/tests/test_parsers.py
@@ -7,10 +7,14 @@
 """
 
 
+import openpyxl
+import pytest
 from openpyxl import Workbook
+from openpyxl.styles import Font
 
 from excel_parser.models.common import Severity
 from excel_parser.parsers import WorkbookParser
+from excel_parser.parsers.sheet_parser import SheetParser
 from tests.helpers.invariant_checker import check_invariants
 
 
@@ -91,6 +95,106 @@ def test_merge_slave_annotated(self, merged_cells_workbook):
         assert b1.is_merged_slave is True
 
 
+class TestStyledEmptyCells:
+    """
+    Formatting on a valueless cell must survive the parse.
+
+    Two bugs met here. ``_has_meaningful_style`` enumerated three font
+    attributes and so missed strikethrough, underline, size and name, while
+    reporting *untouched* cells as styled (a partial ``Font`` leaves ``color``
+    None; the inherited default font has one). And the gate that stores cells
+    dropped anything empty regardless, which made the predicate inert — the
+    styling was parsed and then discarded.
+    """
+
+    @pytest.mark.parametrize(
+        ("coord", "attr", "expected"),
+        [
+            ((1, 2), "strikethrough", True),
+            ((2, 2), "underline", "single"),
+            ((3, 2), "bold", True),
+            ((4, 2), "size", 20.0),
+            ((5, 2), "name", "Courier New"),
+        ],
+    )
+    def test_font_only_empty_cell_kept(
+        self, styled_empty_cells_workbook, coord, attr, expected
+    ):
+        result = WorkbookParser(path=styled_empty_cells_workbook).parse()
+        cell = result.sheets[0].get_cell(*coord)
+        assert cell is not None, f"empty cell with {attr} was discarded"
+        assert cell.style is not None and cell.style.font is not None
+        assert getattr(cell.style.font, attr) == expected
+
+    @pytest.mark.parametrize(
+        ("coord", "attr"),
+        [
+            ((1, 3), "number_format"),
+            ((2, 3), "alignment"),
+            ((3, 3), "fill"),
+            ((4, 3), "border"),
+        ],
+    )
+    def test_non_font_only_empty_cell_kept(self, styled_empty_cells_workbook, coord, attr):
+        result = WorkbookParser(path=styled_empty_cells_workbook).parse()
+        cell = result.sheets[0].get_cell(*coord)
+        assert cell is not None, f"empty cell with {attr} was discarded"
+        assert cell.style is not None
+        assert getattr(cell.style, attr) is not None
+
+    def test_untouched_empty_cell_still_dropped(self, styled_empty_cells_workbook):
+        """The fix must not start hoarding genuinely blank cells."""
+        result = WorkbookParser(path=styled_empty_cells_workbook).parse()
+        sheet = result.sheets[0]
+        assert sheet.get_cell(6, 1) is None
+        assert sheet.get_cell(20, 20) is None
+
+    def test_styled_empty_cells_are_still_empty(self, styled_empty_cells_workbook):
+        """Kept for their style, not misreported as carrying data."""
+        result = WorkbookParser(path=styled_empty_cells_workbook).parse()
+        cell = result.sheets[0].get_cell(1, 2)
+        assert cell.is_empty is True
+        assert cell.raw_value is None
+
+    def test_unstyled_empty_merge_master_is_kept(self, tmp_dir):
+        """
+        A merge master that is empty *and* unstyled must survive the skip.
+
+        It arrives as an ordinary Cell, not a MergedCell, so the empty-cell
+        skip only spares it via the merge lookup. Dropping it strands every
+        slave in the region with a merge_master that does not exist — which
+        the old predicate hid by calling every untouched cell styled.
+        """
+        path = tmp_dir / "empty_unstyled_master.xlsx"
+        wb = Workbook()
+        ws = wb.active
+        ws["A1"] = "anchor"
+        ws.merge_cells("B2:D2")  # master B2 left empty and unformatted
+        wb.save(path)
+
+        result = WorkbookParser(path=path).parse()
+        sheet = result.sheets[0]
+        master = sheet.get_cell(2, 2)
+        assert master is not None, "empty unstyled merge master was discarded"
+        assert master.is_merged_master is True
+        assert check_invariants(result) == []
+
+    def test_predicate_rejects_unstyled_cell(self, tmp_dir):
+        """_has_meaningful_style must not call every default cell styled."""
+        path = tmp_dir / "one_styled.xlsx"
+        wb = Workbook()
+        ws = wb.active
+        ws["A1"] = "anchor"
+        ws["B1"].font = Font(strike=True)
+        wb.save(path)
+
+        loaded = openpyxl.load_workbook(path).active
+        assert SheetParser._has_meaningful_style(loaded["B1"]) is True
+        # A1 carries a value but no styling; C1 was never touched at all.
+        assert SheetParser._has_meaningful_style(loaded["A1"]) is False
+        assert SheetParser._has_meaningful_style(loaded["C1"]) is False
+
+
 class TestOverlappingMerges:
     """
     Overlapping merged regions must not corrupt the merge invariants.