fix: Address code review feedback

Apply all suggested improvements from code review:

1. Remove duplicate dict initialization in merged_cell_handler.py
   - Remove redundant None initialization (lines 39-40)
   - Add type hints at actual initialization (lines 73-74)

2. Narrow exception handling in range_calculator.py
   - Change `except Exception:` to `except ValueError:` for better specificity

3. Restore backward compatibility in calculate_range_size
   - Add try-except block to return (0, 0) on error
   - Maintain original behavior for invalid ranges
   - Add logging for debugging

4. Add defensive check for negative frozen_rows
   - Prevent negative values in pane_manager.py
   - Return (True, 0) for negative input

5. Restore duplicate normalization check in tests
   - Add mock-based call count verification
   - Ensure ExcelRangeCalculator methods called only once
   - Verify no performance regression from refactoring

All 220 tests pass. Quality checks pass.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Author: k-ibaraki

src/excel/merged_cell_handler.py

@@ -36,8 +36,6 @@ def build_merged_cell_cache(
             - merged_anchor_value_map: 結合範囲 -> アンカー値のマップ
             - merged_ranges: 結合範囲情報のリスト
         """
-        merged_cell_map: dict[str, str] | None = None
-        merged_anchor_value_map: dict[str, Any] | None = None
         merged_ranges: list[dict[str, Any]] = []
 
         # 今回返す予定の範囲（結合情報の部分展開に使用）
@@ -70,8 +68,8 @@ def build_merged_cell_cache(
         target_min_col = min(start_col_idx, end_col_idx)
         target_max_col = max(start_col_idx, end_col_idx)
 
-        merged_cell_map = {}
-        merged_anchor_value_map = {}
+        merged_cell_map: dict[str, str] = {}
+        merged_anchor_value_map: dict[str, Any] = {}
 
         for merged_range in sheet.merged_cells.ranges:
             merged_range_str = str(merged_range)

src/excel/pane_manager.py

@@ -67,9 +67,12 @@ def validate_frozen_rows(frozen_rows: int, max_limit: int) -> tuple[bool, int]:
 
         Returns:
             (is_valid, validated_frozen_rows)のタプル
-            - is_valid: 上限以内ならTrue、超過ならFalse
-            - validated_frozen_rows: 超過時は0、それ以外は元の値
+            - is_valid: 上限以内ならTrue、超過または負の場合はFalse
+            - validated_frozen_rows: 無効時は0、それ以外は元の値
         """
+        # 負の値は無効として0に丸める
+        if frozen_rows < 0:
+            return (True, 0)
         if frozen_rows > max_limit:
             return (False, 0)
         return (True, frozen_rows)

src/excel/range_calculator.py

@@ -4,9 +4,13 @@
 セル範囲の計算・変換・検証を担当するヘルパークラス
 """
 
+import logging
+
 from openpyxl.utils import column_index_from_string, get_column_letter
 from openpyxl.utils.cell import coordinate_from_string
 
+logger = logging.getLogger(__name__)
+
 
 class ExcelRangeCalculator:
     """セル範囲の計算・変換・検証（全て staticmethod）"""
@@ -127,7 +131,7 @@ def expand_axis_range(range_str: str) -> str:
             try:
                 col, row = coordinate_from_string(raw.replace("$", ""))
                 return f"{col}1:{col}{row}"
-            except Exception:
+            except ValueError:
                 return range_str
 
         start_cell, end_cell = raw.split(":", 1)
@@ -160,33 +164,36 @@ def calculate_range_size(range_str: str) -> tuple[int, int]:
             range_str: セル範囲（例: "A1:D10" または "A1:XFD1048576"）
 
         Returns:
-            (rows, cols)のタプル
-
-        Raises:
-            ValueError: 逆順序の範囲を検出した場合
+            (rows, cols)のタプル。
+            エラー時は (0, 0) を返す。
         """
-        if ":" not in range_str:
-            # 単一セルの場合
-            return (1, 1)
-
-        start_cell, end_cell = range_str.split(":")
-        start_col, start_row = coordinate_from_string(start_cell)
-        end_col, end_row = coordinate_from_string(end_cell)
-
-        start_col_idx = column_index_from_string(start_col)
-        end_col_idx = column_index_from_string(end_col)
-
-        # 逆順序の範囲を検出（セキュリティ対策）
-        if end_row < start_row or end_col_idx < start_col_idx:
-            raise ValueError(
-                f"無効なセル範囲: '{range_str}'。"
-                f"範囲は正しい順序で指定してください（例: 'A1:Z100'）"
-            )
-
-        rows = end_row - start_row + 1
-        cols = end_col_idx - start_col_idx + 1
-
-        return (rows, cols)
+        try:
+            if ":" not in range_str:
+                # 単一セルの場合
+                return (1, 1)
+
+            start_cell, end_cell = range_str.split(":")
+            start_col, start_row = coordinate_from_string(start_cell)
+            end_col, end_row = coordinate_from_string(end_cell)
+
+            start_col_idx = column_index_from_string(start_col)
+            end_col_idx = column_index_from_string(end_col)
+
+            # 逆順序の範囲を検出（セキュリティ対策）
+            if end_row < start_row or end_col_idx < start_col_idx:
+                raise ValueError(
+                    f"無効なセル範囲: '{range_str}'。"
+                    f"範囲は正しい順序で指定してください（例: 'A1:Z100'）"
+                )
+
+            rows = end_row - start_row + 1
+            cols = end_col_idx - start_col_idx + 1
+
+            return (rows, cols)
+        except Exception as e:
+            # 元の実装との互換性維持: エラー時は (0, 0) を返す
+            logger.warning(f"Failed to calculate range size '{range_str}': {e}")
+            return (0, 0)
 
     @staticmethod
     def normalize_column_range(cell_range: str, max_row: int) -> str:

tests/excel/test_excel_pane_manager.py

@@ -175,7 +175,7 @@ def test_validate_frozen_rows_zero(self):
         assert validated == 0
 
     def test_validate_frozen_rows_negative_value(self):
-        """負の値の場合はTrueと元の値が返ること（実際には発生しない）"""
+        """負の値の場合はTrueと0が返ること（防御的処理）"""
         is_valid, validated = ExcelPaneManager.validate_frozen_rows(-1, 100)
         assert is_valid is True
-        assert validated == -1
+        assert validated == 0

tests/excel/test_excel_range_calculator.py

@@ -127,22 +127,22 @@ def test_calculate_range_size_single_column(self):
         assert cols == 1
 
     def test_calculate_range_size_reverse_order_raises(self):
-        """逆順序の範囲でValueErrorが発生すること"""
-        with pytest.raises(ValueError) as exc_info:
-            ExcelRangeCalculator.calculate_range_size("D10:A1")
-        assert "無効なセル範囲" in str(exc_info.value)
+        """逆順序の範囲で(0, 0)が返ること（互換性維持）"""
+        rows, cols = ExcelRangeCalculator.calculate_range_size("D10:A1")
+        assert rows == 0
+        assert cols == 0
 
     def test_calculate_range_size_reverse_column_raises(self):
-        """逆順序の列でValueErrorが発生すること"""
-        with pytest.raises(ValueError) as exc_info:
-            ExcelRangeCalculator.calculate_range_size("D1:A10")
-        assert "無効なセル範囲" in str(exc_info.value)
+        """逆順序の列で(0, 0)が返ること（互換性維持）"""
+        rows, cols = ExcelRangeCalculator.calculate_range_size("D1:A10")
+        assert rows == 0
+        assert cols == 0
 
     def test_calculate_range_size_reverse_row_raises(self):
-        """逆順序の行でValueErrorが発生すること"""
-        with pytest.raises(ValueError) as exc_info:
-            ExcelRangeCalculator.calculate_range_size("A10:D1")
-        assert "無効なセル範囲" in str(exc_info.value)
+        """逆順序の行で(0, 0)が返ること（互換性維持）"""
+        rows, cols = ExcelRangeCalculator.calculate_range_size("A10:D1")
+        assert rows == 0
+        assert cols == 0
 
     # normalize_column_range のテスト
 

tests/test_sharepoint_excel.py

@@ -861,10 +861,15 @@ def test_normalize_column_range_already_normalized(self):
 
     def test_no_duplicate_range_normalization(self):
         """
-        列範囲指定と拡張が正しく動作することを確認
+        セル範囲の正規化・拡張が重複して実行されないことを確認
 
-        課題3-2の対応：範囲計算がヘルパークラスで一元化されたことを検証
+        課題3-2の対応：_parse_sheetと_build_merged_cell_cacheで
+        重複していた計算が1回のみになったことを検証
         """
+        from unittest.mock import patch
+
+        from src.excel import ExcelRangeCalculator
+
         # テスト用Excelを作成（結合セルあり）
         wb = Workbook()
         ws = wb.active
@@ -888,11 +893,25 @@ def test_no_duplicate_range_normalization(self):
 
         parser = SharePointExcelParser(self.mock_download_client)
 
-        # 列範囲指定で解析（expand_axis_range=Trueで拡張を有効化）
-        result = parser.parse_to_json(
-            "/test/file.xlsx", cell_range="A:B", expand_axis_range=True
-        )
-        result_data = json.loads(result)
+        # 範囲計算メソッドの呼び出し回数を監視
+        with patch.object(
+            ExcelRangeCalculator,
+            "normalize_column_range",
+            wraps=ExcelRangeCalculator.normalize_column_range,
+        ) as mock_normalize, patch.object(
+            ExcelRangeCalculator,
+            "expand_axis_range",
+            wraps=ExcelRangeCalculator.expand_axis_range,
+        ) as mock_expand:
+            # 列範囲指定で解析（expand_axis_range=Trueで拡張を有効化）
+            result = parser.parse_to_json(
+                "/test/file.xlsx", cell_range="A:B", expand_axis_range=True
+            )
+            result_data = json.loads(result)
+
+            # 呼び出し回数が1回であることを確認（重複なし）
+            assert mock_normalize.call_count == 1
+            assert mock_expand.call_count == 1
 
         # 結果が正しいことを確認
         assert "sheets" in result_data