Skip to content

Commit 98ed299

Browse files
k-ibarakiclaude
andcommitted
fix: address AI review comments for Excel parser improvements
- Distinguish frozen_rows reset cases in header_detection - Add 'ignored_due_to_limit' status for frozen_rows exceeding limit - Keep 'no_frozen_rows' status for sheets without frozen rows - Add warning when sheet_name not found in search_cells - Consistent with parse_to_json warning behavior - Improve documentation and comments - Add single cell behavior to _expand_axis_range docstring - Add compatibility notes for _cells private attribute usage These changes improve error clarity and maintainability without affecting the core functionality. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent f630ded commit 98ed299

1 file changed

Lines changed: 44 additions & 20 deletions

File tree

src/sharepoint_excel.py

Lines changed: 44 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ def search_cells(
6262
workbook = load_workbook(file_stream, data_only=False, rich_text=True)
6363

6464
matches = []
65+
warnings = []
6566

6667
# sheet_name 指定がある場合はそのシートを優先して検索
6768
if sheet_name:
@@ -76,6 +77,9 @@ def search_cells(
7677
self._scan_sheet(workbook[sn], sn, query, matches)
7778
else:
7879
# sheet_name が存在しない場合は「指定なし」と同じ扱いで全シート検索
80+
warnings.append(
81+
f"Sheet '{sheet_name}' not found. Searching all sheets instead."
82+
)
7983
for sn in workbook.sheetnames:
8084
self._scan_sheet(workbook[sn], sn, query, matches)
8185
else:
@@ -85,17 +89,17 @@ def search_cells(
8589

8690
logger.info(f"Found {len(matches)} matches for query '{query}'")
8791

88-
return json.dumps(
89-
{
90-
"file_path": file_path,
91-
"mode": "search",
92-
"query": query,
93-
"match_count": len(matches),
94-
"matches": matches,
95-
},
96-
ensure_ascii=False,
97-
indent=2,
98-
)
92+
result = {
93+
"file_path": file_path,
94+
"mode": "search",
95+
"query": query,
96+
"match_count": len(matches),
97+
"matches": matches,
98+
}
99+
if warnings:
100+
result["warnings"] = warnings
101+
102+
return json.dumps(result, ensure_ascii=False, indent=2)
99103

100104
except Exception as e:
101105
logger.error(f"Failed to search cells in Excel file: {str(e)}")
@@ -273,6 +277,8 @@ def _scan_sheet(
273277
# 空シートを避ける意図
274278
if sheet.dimensions:
275279
# パフォーマンスのため_cellsを優先し、無い場合は公開APIにフォールバック
280+
# 注意: _cellsはopenpyxlのプライベート属性のため、将来のバージョンで変更される可能性があります。
281+
# その場合はiter_rows()を使用するフォールバックロジックが動作します。
276282
if hasattr(sheet, "_cells"):
277283
# 実在セルのみを走査(高速)
278284
for cell in sheet._cells.values():
@@ -434,6 +440,7 @@ def _parse_sheet(
434440

435441
# frozen_rows検証(DoS対策)
436442
# frozen_rowsは補助的なメタ情報なので、上限超過時はリセットして処理を続行
443+
frozen_rows_ignored = False
437444
if frozen_rows > config.excel_max_frozen_rows:
438445
logger.warning(
439446
"固定行数が上限(%d)を超えたため、freeze_panes情報を無視します。"
@@ -443,6 +450,7 @@ def _parse_sheet(
443450
frozen_rows,
444451
sheet.title,
445452
)
453+
frozen_rows_ignored = True
446454
frozen_rows = 0
447455
frozen_cols = 0 # freeze_panes全体を無視
448456

@@ -456,15 +464,28 @@ def _parse_sheet(
456464
# frozen_rows=0 かつ cell_range指定時、expand_axis_range=Falseの場合のみ警告
457465
# expand_axis_range=Trueの場合は1行目/A列が含まれるため警告不要
458466
if frozen_rows == 0 and cell_range and not expand_axis_range:
459-
sheet_data["header_detection"] = {
460-
"status": "no_frozen_rows",
461-
"frozen_rows": 0,
462-
"note": "This sheet has no frozen rows. Headers are not automatically included.",
463-
"suggestions": [
464-
"If headers are needed, read 'A1:Z5' to check header structure",
465-
"Or retry with expand_axis_range=True to include row 1 (for columns) or column A (for rows)",
466-
],
467-
}
467+
if frozen_rows_ignored:
468+
# 上限超過で無視されたケース
469+
sheet_data["header_detection"] = {
470+
"status": "ignored_due_to_limit",
471+
"frozen_rows": 0,
472+
"note": "This sheet has frozen rows but they exceed the limit and were ignored. Headers are not automatically included.",
473+
"suggestions": [
474+
"Read 'A1:Z5' to check header structure",
475+
"Or retry with expand_axis_range=True to include row 1 (for columns) or column A (for rows)",
476+
],
477+
}
478+
else:
479+
# 元々frozen_rowsが0のケース
480+
sheet_data["header_detection"] = {
481+
"status": "no_frozen_rows",
482+
"frozen_rows": 0,
483+
"note": "This sheet has no frozen rows. Headers are not automatically included.",
484+
"suggestions": [
485+
"If headers are needed, read 'A1:Z5' to check header structure",
486+
"Or retry with expand_axis_range=True to include row 1 (for columns) or column A (for rows)",
487+
],
488+
}
468489

469490
# セル範囲の正規化・拡張(cell_rangeがある場合)
470491
# マージセル情報のキャッシュに使用するため、先に計算する
@@ -701,6 +722,8 @@ def _build_merged_cell_cache(
701722

702723
# 実在セル(sheet._cells)だけから、結合範囲内の最小(row,col)の値を選ぶ
703724
# 互換性のため_cellsの有無をチェックしてフォールバック
725+
# 注意: _cellsはopenpyxlのプライベート属性のため、将来のバージョンで変更される可能性があります。
726+
# その場合は公開APIを使用するフォールバックロジックが動作します。
704727
if hasattr(sheet, "_cells"):
705728
# プライベート属性を使った高速版
706729
for (r, c), cell_obj in sheet._cells.items():
@@ -755,6 +778,7 @@ def _build_merged_cell_cache(
755778
def _expand_axis_range(self, range_str: str) -> str:
756779
"""
757780
指定されたセル範囲を「枠分離」ではなく「方向に拡張」する。
781+
- 単一セル (例: C5) -> C1:C5
758782
- 単一列 (例: Z100:Z200) -> Z1:Z200
759783
- 単一行 (例: D200:Z200) -> A200:Z200
760784
- それ以外(矩形など)はそのまま

0 commit comments

Comments
 (0)