Conversation
- 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>
Changed "Context Efficiency" section to prioritize LLM usability over brevity. Key points: - LLM's ability to use tools correctly is the top priority - Complex tools require detailed descriptions for correct usage - Token cost is acceptable if it ensures LLM doesn't misuse tools - Avoid verbose phrasing, but never sacrifice clarity for brevity This lesson came from attempting to shorten the sharepoint_excel tool description, which caused LLM to provide incorrect responses. The original detailed description was necessary for proper functionality. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Summary of ChangesHello @k-ibaraki, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! このプルリクエストは、Excelファイル処理の堅牢性と情報提供を向上させることを目的としています。具体的には、指定されたシートが見つからない場合の警告処理を追加し、凍結行の検出ロジックを改善して、ヘッダー検出に関するより詳細な情報を提供するようにしました。また、LLMツール記述に関する内部ガイドラインを更新し、LLMがツールを正しく使用するための明確性を重視するようになりました。 Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
このプルリクエストは、主にExcelの軸範囲を拡張するオプションを追加し、いくつかの改善を行っています。主な変更点として、search_cellsでシートが見つからない場合に警告を返す機能、frozen_rowsが上限を超えて無視された場合に具体的なフィードバックを返す機能の追加、そして_expand_axis_rangeが単一セルを拡張できるようになった点が挙げられます。全体的に良い改善ですが、_parse_sheet関数内にコードの重複が見られたため、リファクタリングを提案するコメントを1件追加しました。
There was a problem hiding this comment.
Pull request overview
SharePoint 上の Excel 解析における利用時の混乱を減らすために、警告情報の付与や expand_axis_range 周辺の説明・判定を拡充している点は良い対応です。Excel 解析結果の「なぜその挙動になったか」を返しやすくする方向性で、全体の可観測性が上がっています。
Changes:
search_cells()で指定シートが存在しない場合に警告（warnings）を返すように変更freeze_panesの DoS 対策で frozen_rows を無視したケースをheader_detection.statusで区別- openpyxl のプライベート属性
_cells利用に関する注意書きと、_expand_axis_range()の説明を追記 - CLAUDE.md の “Context Efficiency” ガイドラインを更新
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/sharepoint_excel.py |
解析・検索結果に警告/ステータス情報を追加し、expand_axis_range/freeze_panes 周辺の説明と挙動を改善 |
CLAUDE.md |
ツール説明の明確さを優先する方針に更新 |
Comments suppressed due to low confidence (1)
src/sharepoint_excel.py:507
- header_detection の追加条件が expand_axis_range フラグの真偽だけで制御されていますが、expand_axis_range=True でも矩形範囲などでは実際に 1行目/A列 が含まれず、警告（header_detection）が抑制されてしまいます。effective_range を計算・拡張した後に、実際に範囲が row=1 / col=A を含むかで header_detection の要否を判定する方が挙動が一貫します。
# frozen_rows=0 かつ cell_range指定時、expand_axis_range=Falseの場合のみ警告
# expand_axis_range=Trueの場合は1行目/A列が含まれるため警告不要
if frozen_rows == 0 and cell_range and not expand_axis_range:
if frozen_rows_ignored:
# 上限超過で無視されたケース
sheet_data["header_detection"] = {
"status": "ignored_due_to_limit",
"frozen_rows": 0,
"note": "This sheet has frozen rows but they exceed the limit and were ignored. Headers are not automatically included.",
"suggestions": [
"Read 'A1:Z5' to check header structure",
"Or retry with expand_axis_range=True to include row 1 (for columns) or column A (for rows)",
],
}
else:
# 元々frozen_rowsが0のケース
sheet_data["header_detection"] = {
"status": "no_frozen_rows",
"frozen_rows": 0,
"note": "This sheet has no frozen rows. Headers are not automatically included.",
"suggestions": [
"If headers are needed, read 'A1:Z5' to check header structure",
"Or retry with expand_axis_range=True to include row 1 (for columns) or column A (for rows)",
],
}
# セル範囲の正規化・拡張（cell_rangeがある場合）
# マージセル情報のキャッシュに使用するため、先に計算する
effective_range_for_merge = None
header_range = None # ヘッダー範囲（再利用のため事前に初期化）
if cell_range:
sheet_data["requested_range"] = cell_range
effective_range = self._normalize_column_range(cell_range, sheet)
if expand_axis_range:
expanded_range = self._expand_axis_range(effective_range)
if expanded_range != effective_range:
logger.info(
"Expanded axis range '%s' -> '%s' (sheet=%s)",
effective_range,
expanded_range,
sheet.title,
)
effective_range = expanded_range
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -76,6 +77,9 @@ def search_cells( | |||
| self._scan_sheet(workbook[sn], sn, query, matches) | |||
| else: | |||
| # sheet_name が存在しない場合は「指定なし」と同じ扱いで全シート検索 | |||
| warnings.append( | |||
| f"Sheet '{sheet_name}' not found. Searching all sheets instead." | |||
| ) | |||
| for sn in workbook.sheetnames: | |||
| self._scan_sheet(workbook[sn], sn, query, matches) | |||
There was a problem hiding this comment.
search_cells() の sheet_name 判定が完全一致のみで、parse_to_json() が使っている _resolve_sheet_name()（trim/casefold 解決や候補提示）と挙動が不整合です。その結果、大小文字/前後空白の違いだけでも「not found」扱いになり、全シート走査にフォールバックして性能面のリスクが出ます。search_cells() 側も _resolve_sheet_name() を使い、解決できない場合は candidates/available_sheets を返す（必要なら cell_range 相当の条件がない限り全走査はしない等）に揃えるのが安全です。
| @@ -85,17 +89,17 @@ def search_cells( | |||
|
|
|||
| logger.info(f"Found {len(matches)} matches for query '{query}'") | |||
|
|
|||
| return json.dumps( | |||
| { | |||
| "file_path": file_path, | |||
| "mode": "search", | |||
| "query": query, | |||
| "match_count": len(matches), | |||
| "matches": matches, | |||
| }, | |||
| ensure_ascii=False, | |||
| indent=2, | |||
| ) | |||
| result = { | |||
| "file_path": file_path, | |||
| "mode": "search", | |||
| "query": query, | |||
| "match_count": len(matches), | |||
| "matches": matches, | |||
| } | |||
| if warnings: | |||
| result["warnings"] = warnings | |||
|
|
|||
| return json.dumps(result, ensure_ascii=False, indent=2) | |||
There was a problem hiding this comment.
sheet_name が存在しない場合に warnings を返す仕様が追加されていますが、この分岐をカバーするテストが現状見当たりません（warnings キーの有無・内容、フォールバック時の match_count など）。既存の tests/test_sharepoint_excel.py に search_cells(sheet_name="NonExistent") のケースを追加して、後方互換（warnings が無い時は出力されない/される等）も含めて固定したいです。
issue
変更内容
確認したこと
スクリーンショット
補足事項
PR時のセルフチェック