From bf1f94b33096760b0b27009cb4fda969af1e194c Mon Sep 17 00:00:00 2001 From: Taylor Wilsdon Date: Sun, 15 Mar 2026 17:52:13 -0400 Subject: [PATCH] refac --- gsheets/sheets_helpers.py | 95 +++++++++++++++++++++++++++++++++++++++ gsheets/sheets_tools.py | 84 ++++------------------------------ 2 files changed, 104 insertions(+), 75 deletions(-) diff --git a/gsheets/sheets_helpers.py b/gsheets/sheets_helpers.py index c4c128f..588204c 100644 --- a/gsheets/sheets_helpers.py +++ b/gsheets/sheets_helpers.py @@ -7,11 +7,15 @@ conditional formatting helpers. import asyncio import json +import logging import re from typing import List, Optional, Union from core.utils import UserInputError +logger = logging.getLogger(__name__) + +MAX_GRID_METADATA_CELLS = 5000 A1_PART_REGEX = re.compile(r"^([A-Za-z]*)(\d*)$") SHEET_TITLE_SAFE_RE = re.compile(r"^[A-Za-z0-9_]+$") @@ -955,3 +959,94 @@ def _format_sheet_notes_section( else "" ) return f"\n\nCell notes in range '{range_label}':\n" + "\n".join(lines) + suffix + + +async def _fetch_grid_metadata( + service, + spreadsheet_id: str, + resolved_range: str, + values: List[List[object]], + include_hyperlinks: bool = False, + include_notes: bool = False, +) -> tuple[str, str]: + """Fetch hyperlinks and/or notes for a range via a single spreadsheets.get call. + + Computes tight range bounds, enforces the cell-count cap, builds a combined + ``fields`` selector so only one API round-trip is needed when both flags are + ``True``, then parses the response into formatted output sections. + + Returns: + (hyperlink_section, notes_section) — each is an empty string when the + corresponding flag is ``False`` or no data was found. + """ + if not include_hyperlinks and not include_notes: + return "", "" + + tight_range = _a1_range_for_values(resolved_range, values) + if not tight_range: + logger.info( + "[read_sheet_values] Skipping grid metadata fetch for range '%s': " + "unable to determine tight bounds", + resolved_range, + ) + return "", "" + + cell_count = _a1_range_cell_count(tight_range) or sum( + len(row) for row in values + ) + if cell_count > MAX_GRID_METADATA_CELLS: + logger.info( + "[read_sheet_values] Skipping grid metadata fetch for large range " + "'%s' (%d cells > %d limit)", + tight_range, + cell_count, + MAX_GRID_METADATA_CELLS, + ) + return "", "" + + # Build a combined fields selector so we hit the API at most once. + value_fields: list[str] = [] + if include_hyperlinks: + value_fields.extend(["hyperlink", "textFormatRuns(format(link(uri)))"]) + if include_notes: + value_fields.append("note") + + fields = ( + "sheets(properties(title),data(startRow,startColumn," + f"rowData(values({','.join(value_fields)}))))" + ) + + try: + response = await asyncio.to_thread( + service.spreadsheets() + .get( + spreadsheetId=spreadsheet_id, + ranges=[tight_range], + includeGridData=True, + fields=fields, + ) + .execute + ) + except Exception as exc: + logger.warning( + "[read_sheet_values] Failed fetching grid metadata for range '%s': %s", + tight_range, + exc, + ) + return "", "" + + hyperlink_section = "" + if include_hyperlinks: + hyperlinks = _extract_cell_hyperlinks_from_grid(response) + hyperlink_section = _format_sheet_hyperlink_section( + hyperlinks=hyperlinks, range_label=tight_range + ) + + notes_section = "" + if include_notes: + notes = _extract_cell_notes_from_grid(response) + notes_section = _format_sheet_notes_section( + notes=notes, range_label=tight_range + ) + + return hyperlink_section, notes_section diff --git a/gsheets/sheets_tools.py b/gsheets/sheets_tools.py index 5f6da4e..90b54db 100644 --- a/gsheets/sheets_tools.py +++ b/gsheets/sheets_tools.py @@ -15,18 +15,14 @@ from core.server import server from core.utils import handle_http_errors, UserInputError from core.comments import create_comment_tools from gsheets.sheets_helpers import ( - _a1_range_cell_count, CONDITION_TYPES, _a1_range_for_values, _build_boolean_rule, _build_gradient_rule, _fetch_detailed_sheet_errors, - _fetch_sheet_hyperlinks, - _fetch_sheet_notes, + _fetch_grid_metadata, _fetch_sheets_with_rules, _format_conditional_rules_section, - _format_sheet_hyperlink_section, - _format_sheet_notes_section, _format_sheet_error_section, _parse_a1_range, _parse_condition_values, @@ -38,7 +34,6 @@ from gsheets.sheets_helpers import ( # Configure module logger logger = logging.getLogger(__name__) -MAX_HYPERLINK_FETCH_CELLS = 5000 @server.tool() @@ -216,75 +211,14 @@ async def read_sheet_values( resolved_range = result.get("range", range_name) detailed_range = _a1_range_for_values(resolved_range, values) or resolved_range - hyperlink_section = "" - if include_hyperlinks: - # Use a tight A1 range for includeGridData fetches to avoid expensive - # open-ended requests (e.g., A:Z). - hyperlink_range = _a1_range_for_values(resolved_range, values) - if not hyperlink_range: - logger.info( - "[read_sheet_values] Skipping hyperlink fetch for range '%s': unable to determine tight bounds", - resolved_range, - ) - else: - cell_count = _a1_range_cell_count(hyperlink_range) or sum( - len(row) for row in values - ) - if cell_count <= MAX_HYPERLINK_FETCH_CELLS: - try: - hyperlinks = await _fetch_sheet_hyperlinks( - service, spreadsheet_id, hyperlink_range - ) - hyperlink_section = _format_sheet_hyperlink_section( - hyperlinks=hyperlinks, range_label=hyperlink_range - ) - except Exception as exc: - logger.warning( - "[read_sheet_values] Failed fetching hyperlinks for range '%s': %s", - hyperlink_range, - exc, - ) - else: - logger.info( - "[read_sheet_values] Skipping hyperlink fetch for large range '%s' (%d cells > %d limit)", - hyperlink_range, - cell_count, - MAX_HYPERLINK_FETCH_CELLS, - ) - - notes_section = "" - if include_notes: - notes_range = _a1_range_for_values(resolved_range, values) - if not notes_range: - logger.info( - "[read_sheet_values] Skipping notes fetch for range '%s': unable to determine tight bounds", - resolved_range, - ) - else: - cell_count = _a1_range_cell_count(notes_range) or sum( - len(row) for row in values - ) - if cell_count <= MAX_HYPERLINK_FETCH_CELLS: - try: - notes = await _fetch_sheet_notes( - service, spreadsheet_id, notes_range - ) - notes_section = _format_sheet_notes_section( - notes=notes, range_label=notes_range - ) - except Exception as exc: - logger.warning( - "[read_sheet_values] Failed fetching notes for range '%s': %s", - notes_range, - exc, - ) - else: - logger.info( - "[read_sheet_values] Skipping notes fetch for large range '%s' (%d cells > %d limit)", - notes_range, - cell_count, - MAX_HYPERLINK_FETCH_CELLS, - ) + hyperlink_section, notes_section = await _fetch_grid_metadata( + service, + spreadsheet_id, + resolved_range, + values, + include_hyperlinks=include_hyperlinks, + include_notes=include_notes, + ) detailed_errors_section = "" if _values_contain_sheets_errors(values):