From 652508a929ea8ece4ab760e9227a6b48a2b789f2 Mon Sep 17 00:00:00 2001
From: Rudy Mizrahi Celekli <47457359+rudycelekli@users.noreply.github.com>
Date: Sun, 18 Oct 2026 01:11:57 -0400
Subject: [PATCH] fix(salary): skip chart-only workbook tabs (#590)

* fix(salary): skip chart-only workbook tabs

* docs(changelog): record fix for #590
---
 CHANGELOG.md                        |  2 +
 tests/test_salary_chartsheet_cli.py | 67 +++++++++++++++++++++++++++++
 tools/convert_salary_excel.py       |  7 ++-
 3 files changed, 72 insertions(+), 4 deletions(-)
 create mode 100644 tests/test_salary_chartsheet_cli.py

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8d48cae6..01bda175 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -218,6 +218,8 @@ per-file diff commands.
   has been aligned to `scale=0.80`, preventing tighter margins and unwanted page overflow during
   CV generation, and a test now guards that the two files stay in sync.
 
+- **Salary workbook conversion skips chart-only tabs** (#590) - chartsheets no longer cause iteration failures while worksheet data continues to be converted.
+
 - **`.claude/settings.local.json` is ignored in `.gitignore` and pinned in security guards** (#551,
   `.gitignore`, `tools/security_guards.py`) - Claude Code stores machine-local settings and
   ad-hoc tool approvals in `.claude/settings.local.json`. The file is now excluded from version
diff --git a/tests/test_salary_chartsheet_cli.py b/tests/test_salary_chartsheet_cli.py
new file mode 100644
index 00000000..f9d702ab
--- /dev/null
+++ b/tests/test_salary_chartsheet_cli.py
@@ -0,0 +1,67 @@
+"""Run the converter on workbooks with both tabular and chart-only sheets."""
+import json
+from pathlib import Path
+import subprocess
+import sys
+from tempfile import TemporaryDirectory
+import unittest
+
+try:
+    import openpyxl
+    from openpyxl.chart import BarChart, Reference
+except ImportError:
+    openpyxl = None
+
+
+TOOL = Path(__file__).resolve().parent.parent / "tools" / "convert_salary_excel.py"
+
+
+@unittest.skipUnless(openpyxl, "requires optional openpyxl (installed in CI)")
+class ChartsheetCLI(unittest.TestCase):
+    def convert(self, with_chart):
+        with TemporaryDirectory() as directory:
+            root = Path(directory)
+            workbook = root / "salary report.xlsx"
+            output = root / "salary output.json"
+            book = openpyxl.Workbook()
+            try:
+                first = book.active
+                first.title = "Salaries"
+                first.append(["Company", "Count", "Index"])
+                first.append(["Acme Corp", 12, 105])
+                second = book.create_sheet("Engineering")
+                second.append(["Company", "Engineering Count", "Engineering Index"])
+                second.append(["Other Corp", 20, 108])
+                if with_chart:
+                    chart = BarChart()
+                    chart.add_data(Reference(first, min_col=2, max_col=3,
+                                             min_row=1, max_row=2), titles_from_data=True)
+                    chart.set_categories(Reference(first, min_col=1, min_row=2, max_row=2))
+                    overview = book.create_chartsheet("Overview", 0)
+                    overview.add_chart(chart)
+                book.save(workbook)
+            finally:
+                book.close()
+
+            result = subprocess.run(
+                [sys.executable, str(TOOL), str(workbook), "--output", str(output)],
+                cwd=root, capture_output=True, text=True,
+            )
+            self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
+            data = json.loads(output.read_text(encoding="utf-8"))
+            self.assertEqual(data["companies"], [
+                {"company": "Acme Corp", "city": "", "categories": {
+                    "all_employees": {"count": 12, "index": 105.0}}},
+                {"company": "Other Corp", "city": "", "categories": {
+                    "engineering": {"count": 20, "index": 108.0}}},
+            ])
+
+    def test_chart_only_tab_does_not_abort_salary_conversion(self):
+        self.convert(with_chart=True)
+
+    def test_all_data_worksheets_are_preserved_without_chart_tabs(self):
+        self.convert(with_chart=False)
+
+
+if __name__ == "__main__":
+    unittest.main()
diff --git a/tools/convert_salary_excel.py b/tools/convert_salary_excel.py
index eda8d92c..7c02ad2e 100644
--- a/tools/convert_salary_excel.py
+++ b/tools/convert_salary_excel.py
@@ -382,10 +382,9 @@ def main():
     wb = openpyxl.load_workbook(excel_path, read_only=True, data_only=True)
 
     all_companies = []
-    for sheet_name in wb.sheetnames:
-        print(f"  Parsing sheet: {sheet_name}")
-        ws = wb[sheet_name]
-        companies = parse_sheet(ws, sheet_label=sheet_name)
+    for ws in wb.worksheets:
+        print(f"  Parsing sheet: {ws.title}")
+        companies = parse_sheet(ws, sheet_label=ws.title)
         all_companies.extend(companies)
 
     wb.close()