Skip to content

Add file version-history (Drive revisions) and spreadsheet audit/diff tools - #936

Open
Carlo-SR wants to merge 1 commit into
taylorwilsdon:mainfrom
Carlo-SR:feat/file-revisions-and-sheet-audit
Open

Add file version-history (Drive revisions) and spreadsheet audit/diff tools#936
Carlo-SR wants to merge 1 commit into
taylorwilsdon:mainfrom
Carlo-SR:feat/file-revisions-and-sheet-audit

Conversation

@Carlo-SR

@Carlo-SR Carlo-SR commented Jul 17, 2026

Copy link
Copy Markdown

Summary

Adds file version-history tools (Drive revisions) and two spreadsheet auditing tools. All changes are additive; existing tools and behavior are unchanged.

Motivation: there was no way to inspect a file's revision history or recover how a spreadsheet looked at an earlier point in time (e.g. to find how a formula was built before a #REF! was introduced). These tools fill that gap, plus quick error/diff auditing for spreadsheets.

New tools

Drive (gdrive/drive_tools.py) — work for any Drive file, incl. Sheets/Docs/Slides:

Tool Purpose
list_file_revisions List a file's Drive revision history (revision id, modified time, last editor).
export_file_revision Download a specific historical revision. Google-native files are exported (Sheets → xlsx default / csv / pdf); other files return the original revision bytes. Saved via the existing attachment storage (local path in stdio mode, download URL in HTTP mode).

Sheets (gsheets/sheets_tools.py):

Tool Purpose
audit_spreadsheet_errors Scan a spreadsheet (all sheets or one) for formula error cells (#REF!, #DIV/0!, #N/A, #VALUE!, #NAME?, #NUM!, #ERROR!, #NULL!); returns per-sheet counts and sample cells. Reuses the existing _fetch_detailed_sheet_errors grid helper.
diff_spreadsheets Compare the same sheet across two spreadsheets on the formula level: frozen (formula→static value), changed formulas, cleared, and added cells. Useful to verify/reverse-engineer a transformation between a master and a derived copy.

Implementation notes

  • Follows the existing conventions: @server.tool + @handle_http_errors + @require_google_service, asyncio.to_thread around the blocking Google client calls, human-readable string returns.
  • export_file_revision mirrors get_drive_file_download_url's storage/return handling (stateless / stdio path / HTTP URL). For Google-native files it downloads the per-revision exportLinks via the service's already-authorized transport; for binary files it uses revisions().get_media.
  • Uses the read-only Drive scope group (drive_read) already resolved by the service decorator — sufficient for reading revisions of files the user can access.
  • Registered in core/tool_tiers.yaml: revisions under drive.extended, audit/diff under sheets.complete.

Testing

  • uv sync + import of the modified modules succeeds (decorators register without error).
  • server.list_tools() reports all four new tools among the registered set (35 total in the complete tier).
  • The underlying Drive-revision / error-grid / formula-diff logic was validated against the live Google APIs in a standalone harness; end-to-end validation of these tools inside the server against a live account is in progress and I'm happy to attach output.

Summary by CodeRabbit

  • New Features
    • Added tools to view Google Drive file revision history, including revision IDs, timestamps, and editors.
    • Added support for exporting or downloading specific Drive file revisions in suitable formats.
    • Added spreadsheet error auditing with per-sheet counts and example error cells.
    • Added spreadsheet comparison tools to identify changed, added, cleared, or converted formulas across matching sheets.

New Drive tools (version history for any Drive file, incl. Sheets/Docs):
- list_file_revisions: list a file's Drive revision history (id, time, editor)
- export_file_revision: download a specific historical revision (Sheets ->
  xlsx/csv/pdf, others -> original bytes), e.g. to inspect how a spreadsheet's
  formulas looked before a change was introduced

New Sheets tools:
- audit_spreadsheet_errors: scan a spreadsheet for formula error cells
  (#REF!, #DIV/0!, #N/A, ...); reuses existing grid-error helper
- diff_spreadsheets: compare the same sheet across two spreadsheets on the
  formula level (frozen/changed/cleared/added)

Registered in core/tool_tiers.yaml (drive: extended, sheets: complete).
All additive; existing tools unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Four MCP tools were added: two for Google Drive revision history and exports, and two for Google Sheets error auditing and spreadsheet formula diffs. The tools were registered under the corresponding Drive and Sheets tiers.

Changes

Drive revision tools

Layer / File(s) Summary
Drive revision listing and export
gdrive/drive_tools.py, core/tool_tiers.yaml
Adds paginated revision listing, native-file export and direct media download, attachment or transport-specific output, and Drive tier registrations.

Sheets analysis tools

Layer / File(s) Summary
Spreadsheet error and formula analysis
gsheets/sheets_tools.py, core/tool_tiers.yaml
Adds per-sheet formula error auditing, cross-spreadsheet formula comparison, categorized differences, bounded examples, and Sheets tier registrations.

Estimated code review effort: 4 (Complex) | ~45 minutes

Suggested reviewers: taylorwilsdon

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the added Drive revision-history and spreadsheet audit/diff tools.
Description check ✅ Passed The description covers summary, new tools, implementation notes, and testing, with only template checklist sections omitted.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@gdrive/drive_tools.py`:
- Around line 2831-2839: Update the revision download flow around the revision
metadata fetch and blob-download handling to reject revisions whose declared
size exceeds the configured attachment limit, enforce the limit while receiving
data, and stream content directly to storage instead of creating full in-memory
copies. Change each affected persistence call, including the paths around the
referenced ranges, to pass the bound save_attachment method itself to
asyncio.to_thread with its arguments, ensuring save_attachment executes off the
event loop.
- Around line 2831-2839: Update the revision retrieval and subsequent saved-file
naming/labeling flow around the revision variable to request and use the
revision’s mimeType and originalFilename, rather than current file metadata.
Ensure binary revisions retain the revision-specific MIME type and original
filename when determining the output name and file labels.
- Around line 2842-2847: Update the export-format selection logic in the
native_defaults branch to validate and normalize any provided export_format
before choosing a target. Reject unknown values such as "csvv" instead of
falling back to default_mime/default_ext, while preserving the existing default
behavior when export_format is absent and valid mapped formats continue using
fmt_map.
- Around line 2757-2759: Update the revision-listing flow and
export_file_revision to distinguish downloadable blob revisions: include
keepForever and the current file’s headRevisionId, mark or filter revision IDs
that are not downloadable, and validate the requested revision before calling
revisions.get_media. Fail fast with a clear error for unavailable revisions, and
revise the tip so it does not imply every revision can be downloaded.
- Around line 2744-2749: Update the revision summary label in the
version-history output built from total and shown so it explicitly calls the
count the Drive-visible revision count, avoiding wording that implies a complete
archive. Preserve the existing counts and oldest-to-newest ordering text.

In `@gsheets/sheets_tools.py`:
- Around line 2561-2625: Update the comparison logic around grab, grid_a/grid_b,
and a1 to parse each response’s returned range and map every value to absolute
row and column coordinates before comparing, including offset ranges and
differing data bounding boxes; generate addresses from those absolute
coordinates rather than assuming A1 origins. Also add a distinct counter and
output category for changed literals when both cells are non-formulas with
different values, reserving same_literal for equal literal values.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ad120263-aa4d-4731-b949-ae3e201f6696

📥 Commits

Reviewing files that changed from the base of the PR and between 3f19c7d and f1d3e52.

📒 Files selected for processing (3)
  • core/tool_tiers.yaml
  • gdrive/drive_tools.py
  • gsheets/sheets_tools.py

Comment thread gdrive/drive_tools.py
Comment on lines +2744 to +2749
total = len(revisions)
shown = revisions[-max_results:] if max_results and max_results > 0 else revisions

lines = [
f'Version history for "{file_name}" (ID: {file_id}) — '
f"{total} revision(s), showing {len(shown)} (oldest to newest):"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the relevant file region with line numbers.
sed -n '2680,2795p' gdrive/drive_tools.py

printf '\n--- SEARCH ---\n'

# Find revision listing helpers/usages.
rg -n "list_file_revisions|export_file_revision|revisions =|pageToken|nextPageToken|revisions" gdrive/drive_tools.py

Repository: taylorwilsdon/google_workspace_mcp

Length of output: 7340


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '2680,2795p' gdrive/drive_tools.py

printf '\n--- SEARCH ---\n'
rg -n "list_file_revisions|export_file_revision|revisions =|pageToken|nextPageToken|revisions" gdrive/drive_tools.py

Repository: taylorwilsdon/google_workspace_mcp

Length of output: 7340


🌐 Web query:

Google Drive API revisions list older revisions automatically deleted 30 days 100 revisions manage revisions

💡 Result:

In Google Drive, revisions for files with binary content (such as images, videos, or PDFs) are subject to automatic purging [1][2]. By default, these revisions are automatically deleted 30 days after newer content is uploaded [1][2][3]. Additionally, revisions may be purged earlier if a file reaches a limit of 100 revisions that are not designated to be kept [1][3]. To prevent a revision from being automatically deleted, you can set the keepForever property to true when using the Drive API (or select "Keep forever" in the Google Drive web interface) [2][3]. This property is only applicable to files with binary content in Drive [2]. You can set this "Keep forever" status on a maximum of 200 revisions per file [2]. These automatic deletion rules do not apply in the same way to native Google Workspace files (such as Google Docs, Sheets, and Slides), which manage their revision history differently [1]. Note that the Drive API revisions resource allows you to list, get, update, and delete individual revisions programmatically [4][5].

Citations:


Call this the Drive-visible revision count. len(revisions) is only the revisions returned by Drive, so “revision history” can imply a complete archive when older revisions may have been pruned. Reword the label to make the scope explicit.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@gdrive/drive_tools.py` around lines 2744 - 2749, Update the revision summary
label in the version-history output built from total and shown so it explicitly
calls the count the Drive-visible revision count, avoiding wording that implies
a complete archive. Preserve the existing counts and oldest-to-newest ordering
text.

Comment thread gdrive/drive_tools.py
Comment on lines +2757 to +2759
lines.append(
"\nTip: use export_file_revision(file_id, revision_id=...) to download a "
"specific revision (e.g. as xlsx) and inspect its contents/formulas."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

python3 - <<'PY'
from pathlib import Path
p = Path('gdrive/drive_tools.py')
text = p.read_text()
for needle in ['export_file_revision', 'keepForever', 'headRevisionId', 'get_media', 'revisions']:
    print(f'\n## {needle}')
    for i, line in enumerate(text.splitlines(), 1):
        if needle in line:
            start = max(1, i-3); end = min(len(text.splitlines()), i+6)
            for j in range(start, end+1):
                print(f'{j}: {text.splitlines()[j-1]}')
            print('---')
PY

Repository: taylorwilsdon/google_workspace_mcp

Length of output: 12462


🌐 Web query:

Google Drive API revisions keepForever headRevisionId blob revision download get_media documentation

💡 Result:

In the Google Drive API (v3), blob file revisions are managed through the revisions resource [1][2]. The following technical details address your specific terms: Keep Forever The keepForever property is a boolean field in the Revision resource that determines whether a specific blob file revision is preserved [1][3]. Any blob revision that is not designated as "Keep Forever" is considered purgeable and may be automatically deleted after 30 days or sooner if the file exceeds 100 non-permanent revisions [4]. You must explicitly set a revision to "Keep Forever" if you intend to download it at a later time [5][6]. Download Revisions To download the content of a blob file revision, use the revisions.get method with the fileId, revisionId, and the alt=media query parameter [5][6]. Example request: GET https://www.googleapis.com/drive/v3/files/{fileId}/revisions/{revisionId}?alt=media Alternatively, you can use the files.download method, which supports long-running operations and allows you to specify a revisionId for blob files, Google Docs, and Google Sheets [6][7]. Head Revision ID The headRevisionId represents the most current version of a file and is available for blob files in the Drive API [4]. It is accessed as a property of the File resource (via the files resource), not the revisions resource [4]. Summary of Key Methods - revisions.list: Retrieve a list of all revisions for a file to find the necessary revisionId [2]. - revisions.get: Use with alt=media to download the specific content of a revision [5][6]. - revisions.update: Use this method to modify the keepForever property [2][8]. - files.download: An alternative method for downloading files and specific revisions [6][7].

Citations:


🌐 Web query:

site:developers.google.com Drive revisions.get_media keepForever headRevisionId current revision blob download

💡 Result:

To download a specific revision blob from Google Drive using the Drive API, you must use the revisions.get method [1][2]. Below are the key requirements and steps: 1. Prerequisite (Keep Forever): You can only download blob file revisions that are explicitly marked with the keepForever property set to true [1][2]. If a revision is not marked as "Keep Forever," it is subject to automatic purging (typically after 30 days) [3][4]. You can update this property using the revisions.update method [3][5]. 2. Identifying the Revision: While you can retrieve the headRevisionId from the file resource [6], you must use the revisions.list method to retrieve the specific revisionId you wish to download [3]. 3. Downloading the Blob: Use the revisions.get method with the fileId and revisionId, and include the alt=media query parameter to request the content body rather than metadata [1][7]. Example request (cURL): curl -L "https://www.googleapis.com/drive/v3/files/{FILE_ID}/revisions/{REVISION_ID}?alt=media" \ --header "Authorization: Bearer {ACCESS_TOKEN}" \ --output "{FILE_NAME}" Note: This process is only applicable to files with binary content (blob files) in Google Drive [3][2][5]. The head revision is automatically preserved and does not require the keepForever setting to be enabled [2][4]. You can set a maximum of 200 revisions to "Keep Forever" per file [3][4]. Alternatively, you may use the files.download method for certain operations, which also supports specifying a revisionId [7][8].

Citations:


Don’t imply every revision is downloadable
For blob files, only the head revision or revisions with keepForever=true can be fetched. Include keepForever (and headRevisionId for the current file) in the revision listing, mark or filter the returned IDs accordingly, and fail fast in export_file_revision when a revision can’t be downloaded instead of calling revisions.get_media and surfacing a backend error.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@gdrive/drive_tools.py` around lines 2757 - 2759, Update the revision-listing
flow and export_file_revision to distinguish downloadable blob revisions:
include keepForever and the current file’s headRevisionId, mark or filter
revision IDs that are not downloadable, and validate the requested revision
before calling revisions.get_media. Fail fast with a clear error for unavailable
revisions, and revise the tip so it does not imply every revision can be
downloaded.

Comment thread gdrive/drive_tools.py
Comment on lines +2831 to +2839
revision = await asyncio.to_thread(
service.revisions()
.get(
fileId=file_id,
revisionId=revision_id,
fields="id,modifiedTime,mimeType,exportLinks",
)
.execute
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Bound blob downloads and move persistence off the event loop.

This path creates several full-size in-memory copies without checking revision size, then performs synchronous disk writes. Enforce the configured attachment-size limit before and during download; otherwise stream directly to storage. Also run save_attachment in asyncio.to_thread.

Offload the blocking save
-        result = storage.save_attachment(
+        result = await asyncio.to_thread(
+            storage.save_attachment,
             base64_data=base64_data,
             filename=output_filename,
             mime_type=output_mime_type,
         )

Based on learnings, pass the bound method itself to asyncio.to_thread, not its result.

Also applies to: 2872-2878, 2883-2904

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@gdrive/drive_tools.py` around lines 2831 - 2839, Update the revision download
flow around the revision metadata fetch and blob-download handling to reject
revisions whose declared size exceeds the configured attachment limit, enforce
the limit while receiving data, and stream content directly to storage instead
of creating full in-memory copies. Change each affected persistence call,
including the paths around the referenced ranges, to pass the bound
save_attachment method itself to asyncio.to_thread with its arguments, ensuring
save_attachment executes off the event loop.

Source: Learnings


🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the relevant function and nearby callers.
rg -n -C 4 "revisions\(\)\.get|originalFilename|mimeType|exportLinks|revision_id|revisionId" gdrive/drive_tools.py

# Show a focused slice around the reported lines.
sed -n '2800,2895p' gdrive/drive_tools.py

# Find other revision-download paths for comparison.
rg -n -C 3 "revision.*mimeType|originalFilename|download.*revision|revisions" gdrive

Repository: taylorwilsdon/google_workspace_mcp

Length of output: 30132


🌐 Web query:

Google Drive API revisions resource originalFilename mimeType exportLinks get revision fields

💡 Result:

In the Google Drive API (v3), the revisions resource provides metadata about file revisions [1][2]. You can retrieve a specific revision using the revisions.get method by providing the required fileId and revisionId [3][4]. The revisions resource includes several fields, including those you mentioned: exportLinks: A map of links for exporting Docs Editors files to specific formats [1][2]. mimeType: The MIME type of the revision [1][2]. originalFilename: The original filename used to create the revision; this field is only applicable to files with binary content in Drive [1][2]. When using the revisions.get or revisions.list methods, you can use the fields system parameter to specify which of these fields to return in the API response [5]. If you omit the fields parameter, the server returns a default set of fields, which for revisions.list typically includes only id, mimeType, kind, and modifiedTime [5].

Citations:


Use the revision’s MIME type and original filename.
For binary revisions, the current file metadata can be stale after renames or retypes; use the revision’s mimeType and originalFilename when naming and labeling the saved file.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@gdrive/drive_tools.py` around lines 2831 - 2839, Update the revision
retrieval and subsequent saved-file naming/labeling flow around the revision
variable to request and use the revision’s mimeType and originalFilename, rather
than current file metadata. Ensure binary revisions retain the revision-specific
MIME type and original filename when determining the output name and file
labels.

Comment thread gdrive/drive_tools.py
Comment on lines +2842 to +2847
if mime_type in native_defaults:
default_mime, default_ext = native_defaults[mime_type]
if export_format and export_format in fmt_map:
target_mime, ext = fmt_map[export_format], export_format
else:
target_mime, ext = default_mime, default_ext

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Reject unknown export_format values instead of silently defaulting.

A typo such as export_format="csvv" currently returns XLSX/PDF. Validate and normalize the value before selecting the default.

Proposed fix
-        if export_format and export_format in fmt_map:
-            target_mime, ext = fmt_map[export_format], export_format
+        if export_format:
+            export_format = export_format.lower()
+            if export_format not in fmt_map:
+                return f"Unsupported export format: '{export_format}'."
+            target_mime, ext = fmt_map[export_format], export_format
         else:
             target_mime, ext = default_mime, default_ext
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if mime_type in native_defaults:
default_mime, default_ext = native_defaults[mime_type]
if export_format and export_format in fmt_map:
target_mime, ext = fmt_map[export_format], export_format
else:
target_mime, ext = default_mime, default_ext
if mime_type in native_defaults:
default_mime, default_ext = native_defaults[mime_type]
if export_format:
export_format = export_format.lower()
if export_format not in fmt_map:
return f"Unsupported export format: '{export_format}'."
target_mime, ext = fmt_map[export_format], export_format
else:
target_mime, ext = default_mime, default_ext
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@gdrive/drive_tools.py` around lines 2842 - 2847, Update the export-format
selection logic in the native_defaults branch to validate and normalize any
provided export_format before choosing a target. Reject unknown values such as
"csvv" instead of falling back to default_mime/default_ext, while preserving the
existing default behavior when export_format is absent and valid mapped formats
continue using fmt_map.

Comment thread gsheets/sheets_tools.py
Comment on lines +2561 to +2625
async def grab(sid: str) -> List[List]:
result = await asyncio.to_thread(
service.spreadsheets()
.values()
.get(spreadsheetId=sid, range=full_range, valueRenderOption="FORMULA")
.execute
)
return result.get("values", [])

grid_a = await grab(spreadsheet_id_a)
grid_b = await grab(spreadsheet_id_b)

def a1(row_idx: int, col_idx: int) -> str:
letters = ""
n = col_idx + 1
while n > 0:
n, rem = divmod(n - 1, 26)
letters = chr(65 + rem) + letters
return f"{letters}{row_idx + 1}"

frozen = changed = cleared = added = same_formula = same_literal = 0
examples = {"frozen": [], "changed": [], "cleared": [], "added": []}
n_rows = max(len(grid_a), len(grid_b))
for i in range(n_rows):
row_a = grid_a[i] if i < len(grid_a) else []
row_b = grid_b[i] if i < len(grid_b) else []
n_cols = max(len(row_a), len(row_b))
for j in range(n_cols):
av = row_a[j] if j < len(row_a) else ""
bv = row_b[j] if j < len(row_b) else ""
a_has = av not in (None, "")
b_has = bv not in (None, "")
a_form = isinstance(av, str) and av.startswith("=")
b_form = isinstance(bv, str) and bv.startswith("=")
addr = a1(i, j)
if a_form and b_has and not b_form:
frozen += 1
if len(examples["frozen"]) < max_examples:
examples["frozen"].append(f" {addr}: {str(av)[:40]} -> {str(bv)[:30]}")
elif a_form and b_form:
if av == bv:
same_formula += 1
else:
changed += 1
if len(examples["changed"]) < max_examples:
examples["changed"].append(f" {addr}: {str(av)[:34]} -> {str(bv)[:34]}")
elif a_has and not b_has:
cleared += 1
if len(examples["cleared"]) < max_examples:
examples["cleared"].append(f" {addr}: {str(av)[:40]}")
elif b_has and not a_has:
added += 1
if len(examples["added"]) < max_examples:
examples["added"].append(f" {addr}: {str(bv)[:40]}")
elif a_has and b_has:
same_literal += 1

out = [
f"Diff of sheet '{sheet}' ({'range ' + range_name if range_name else 'whole sheet'}):",
f"- frozen (formula -> value in B): {frozen}",
f"- changed formulas: {changed}",
f"- cleared in B: {cleared}",
f"- added in B: {added}",
f"- unchanged formulas: {same_formula} | unchanged literals: {same_literal}",
]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🔴 Critical | 🏗️ Heavy lift

Data bounding box misalignment and incorrect cell addressing.

The spreadsheets.values.get API bounds the returned arrays to the existing data footprint. If grid_a and grid_b possess differing leading empty cells, or if an offset range_name (e.g., B2:D5) is supplied, grid_a and grid_b can start at entirely different row/column coordinates. Iterating from [0][0] aligns them incorrectly by their relative top-left bounds rather than their absolute positions. Furthermore, the a1 helper incorrectly assumes zero-indices always correspond to A1.

Additionally, the tool categorizes cells with different literal values (e.g., 1 -> 2) as same_literal which is misleading for users interpreting the diff output.

Parse the starting coordinates directly from the returned range API field to map the grids to absolute cell coordinates before comparing, and distinguish changed literals.

🛠️ Proposed fix using absolute coordinate mapping
-    async def grab(sid: str) -> List[List]:
+    async def grab(sid: str) -> dict:
         result = await asyncio.to_thread(
             service.spreadsheets()
             .values()
             .get(spreadsheetId=sid, range=full_range, valueRenderOption="FORMULA")
             .execute
         )
-        return result.get("values", [])
+        return result
 
-    grid_a = await grab(spreadsheet_id_a)
-    grid_b = await grab(spreadsheet_id_b)
+    resp_a = await grab(spreadsheet_id_a)
+    resp_b = await grab(spreadsheet_id_b)
 
     def a1(row_idx: int, col_idx: int) -> str:
         letters = ""
         n = col_idx + 1
         while n > 0:
             n, rem = divmod(n - 1, 26)
             letters = chr(65 + rem) + letters
         return f"{letters}{row_idx + 1}"
 
-    frozen = changed = cleared = added = same_formula = same_literal = 0
+    import re
+    def to_map(resp: dict) -> dict:
+        grid = resp.get("values", [])
+        r_str = resp.get("range", "")
+        m = re.search(r'!([a-zA-Z]+)(\d+)', r_str)
+        start_r, start_c = 0, 0
+        if m:
+            col_str = m.group(1).upper()
+            for c in col_str:
+                start_c = start_c * 26 + (ord(c) - 64)
+            start_c -= 1
+            start_r = int(m.group(2)) - 1
+            
+        abs_map = {}
+        for i, row in enumerate(grid):
+            for j, val in enumerate(row):
+                if val not in (None, ""):
+                    abs_map[(start_r + i, start_c + j)] = val
+        return abs_map
+
+    map_a = to_map(resp_a)
+    map_b = to_map(resp_b)
+    all_coords = set(map_a.keys()) | set(map_b.keys())
+
+    frozen = changed = cleared = added = same_formula = same_literal = changed_literal = 0
     examples = {"frozen": [], "changed": [], "cleared": [], "added": []}
-    n_rows = max(len(grid_a), len(grid_b))
-    for i in range(n_rows):
-        row_a = grid_a[i] if i < len(grid_a) else []
-        row_b = grid_b[i] if i < len(grid_b) else []
-        n_cols = max(len(row_a), len(row_b))
-        for j in range(n_cols):
-            av = row_a[j] if j < len(row_a) else ""
-            bv = row_b[j] if j < len(row_b) else ""
-            a_has = av not in (None, "")
-            b_has = bv not in (None, "")
-            a_form = isinstance(av, str) and av.startswith("=")
-            b_form = isinstance(bv, str) and bv.startswith("=")
-            addr = a1(i, j)
-            if a_form and b_has and not b_form:
-                frozen += 1
-                if len(examples["frozen"]) < max_examples:
-                    examples["frozen"].append(f"    {addr}: {str(av)[:40]} -> {str(bv)[:30]}")
-            elif a_form and b_form:
-                if av == bv:
-                    same_formula += 1
-                else:
-                    changed += 1
-                    if len(examples["changed"]) < max_examples:
-                        examples["changed"].append(f"    {addr}: {str(av)[:34]} -> {str(bv)[:34]}")
-            elif a_has and not b_has:
-                cleared += 1
-                if len(examples["cleared"]) < max_examples:
-                    examples["cleared"].append(f"    {addr}: {str(av)[:40]}")
-            elif b_has and not a_has:
-                added += 1
-                if len(examples["added"]) < max_examples:
-                    examples["added"].append(f"    {addr}: {str(bv)[:40]}")
-            elif a_has and b_has:
-                same_literal += 1
+
+    for r, c in sorted(all_coords):
+        av = map_a.get((r, c), "")
+        bv = map_b.get((r, c), "")
+        a_has = av != ""
+        b_has = bv != ""
+        a_form = isinstance(av, str) and av.startswith("=")
+        b_form = isinstance(bv, str) and bv.startswith("=")
+        addr = a1(r, c)
+        if a_form and b_has and not b_form:
+            frozen += 1
+            if len(examples["frozen"]) < max_examples:
+                examples["frozen"].append(f"    {addr}: {str(av)[:40]} -> {str(bv)[:30]}")
+        elif a_form and b_form:
+            if av == bv:
+                same_formula += 1
+            else:
+                changed += 1
+                if len(examples["changed"]) < max_examples:
+                    examples["changed"].append(f"    {addr}: {str(av)[:34]} -> {str(bv)[:34]}")
+        elif a_has and not b_has:
+            cleared += 1
+            if len(examples["cleared"]) < max_examples:
+                examples["cleared"].append(f"    {addr}: {str(av)[:40]}")
+        elif b_has and not a_has:
+            added += 1
+            if len(examples["added"]) < max_examples:
+                examples["added"].append(f"    {addr}: {str(bv)[:40]}")
+        elif a_has and b_has:
+            if av == bv:
+                same_literal += 1
+            else:
+                changed_literal += 1
 
     out = [
         f"Diff of sheet '{sheet}' ({'range ' + range_name if range_name else 'whole sheet'}):",
         f"- frozen (formula -> value in B): {frozen}",
         f"- changed formulas: {changed}",
         f"- cleared in B: {cleared}",
         f"- added in B: {added}",
-        f"- unchanged formulas: {same_formula} | unchanged literals: {same_literal}",
+        f"- unchanged formulas: {same_formula} | unchanged literals: {same_literal} | changed literals: {changed_literal}",
     ]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async def grab(sid: str) -> List[List]:
result = await asyncio.to_thread(
service.spreadsheets()
.values()
.get(spreadsheetId=sid, range=full_range, valueRenderOption="FORMULA")
.execute
)
return result.get("values", [])
grid_a = await grab(spreadsheet_id_a)
grid_b = await grab(spreadsheet_id_b)
def a1(row_idx: int, col_idx: int) -> str:
letters = ""
n = col_idx + 1
while n > 0:
n, rem = divmod(n - 1, 26)
letters = chr(65 + rem) + letters
return f"{letters}{row_idx + 1}"
frozen = changed = cleared = added = same_formula = same_literal = 0
examples = {"frozen": [], "changed": [], "cleared": [], "added": []}
n_rows = max(len(grid_a), len(grid_b))
for i in range(n_rows):
row_a = grid_a[i] if i < len(grid_a) else []
row_b = grid_b[i] if i < len(grid_b) else []
n_cols = max(len(row_a), len(row_b))
for j in range(n_cols):
av = row_a[j] if j < len(row_a) else ""
bv = row_b[j] if j < len(row_b) else ""
a_has = av not in (None, "")
b_has = bv not in (None, "")
a_form = isinstance(av, str) and av.startswith("=")
b_form = isinstance(bv, str) and bv.startswith("=")
addr = a1(i, j)
if a_form and b_has and not b_form:
frozen += 1
if len(examples["frozen"]) < max_examples:
examples["frozen"].append(f" {addr}: {str(av)[:40]} -> {str(bv)[:30]}")
elif a_form and b_form:
if av == bv:
same_formula += 1
else:
changed += 1
if len(examples["changed"]) < max_examples:
examples["changed"].append(f" {addr}: {str(av)[:34]} -> {str(bv)[:34]}")
elif a_has and not b_has:
cleared += 1
if len(examples["cleared"]) < max_examples:
examples["cleared"].append(f" {addr}: {str(av)[:40]}")
elif b_has and not a_has:
added += 1
if len(examples["added"]) < max_examples:
examples["added"].append(f" {addr}: {str(bv)[:40]}")
elif a_has and b_has:
same_literal += 1
out = [
f"Diff of sheet '{sheet}' ({'range ' + range_name if range_name else 'whole sheet'}):",
f"- frozen (formula -> value in B): {frozen}",
f"- changed formulas: {changed}",
f"- cleared in B: {cleared}",
f"- added in B: {added}",
f"- unchanged formulas: {same_formula} | unchanged literals: {same_literal}",
]
async def grab(sid: str) -> dict:
result = await asyncio.to_thread(
service.spreadsheets()
.values()
.get(spreadsheetId=sid, range=full_range, valueRenderOption="FORMULA")
.execute
)
return result
resp_a = await grab(spreadsheet_id_a)
resp_b = await grab(spreadsheet_id_b)
def a1(row_idx: int, col_idx: int) -> str:
letters = ""
n = col_idx + 1
while n > 0:
n, rem = divmod(n - 1, 26)
letters = chr(65 + rem) + letters
return f"{letters}{row_idx + 1}"
import re
def to_map(resp: dict) -> dict:
grid = resp.get("values", [])
r_str = resp.get("range", "")
m = re.search(r'!([a-zA-Z]+)(\d+)', r_str)
start_r, start_c = 0, 0
if m:
col_str = m.group(1).upper()
for c in col_str:
start_c = start_c * 26 + (ord(c) - 64)
start_c -= 1
start_r = int(m.group(2)) - 1
abs_map = {}
for i, row in enumerate(grid):
for j, val in enumerate(row):
if val not in (None, ""):
abs_map[(start_r + i, start_c + j)] = val
return abs_map
map_a = to_map(resp_a)
map_b = to_map(resp_b)
all_coords = set(map_a.keys()) | set(map_b.keys())
frozen = changed = cleared = added = same_formula = same_literal = changed_literal = 0
examples = {"frozen": [], "changed": [], "cleared": [], "added": []}
for r, c in sorted(all_coords):
av = map_a.get((r, c), "")
bv = map_b.get((r, c), "")
a_has = av != ""
b_has = bv != ""
a_form = isinstance(av, str) and av.startswith("=")
b_form = isinstance(bv, str) and bv.startswith("=")
addr = a1(r, c)
if a_form and b_has and not b_form:
frozen += 1
if len(examples["frozen"]) < max_examples:
examples["frozen"].append(f" {addr}: {str(av)[:40]} -> {str(bv)[:30]}")
elif a_form and b_form:
if av == bv:
same_formula += 1
else:
changed += 1
if len(examples["changed"]) < max_examples:
examples["changed"].append(f" {addr}: {str(av)[:34]} -> {str(bv)[:34]}")
elif a_has and not b_has:
cleared += 1
if len(examples["cleared"]) < max_examples:
examples["cleared"].append(f" {addr}: {str(av)[:40]}")
elif b_has and not a_has:
added += 1
if len(examples["added"]) < max_examples:
examples["added"].append(f" {addr}: {str(bv)[:40]}")
elif a_has and b_has:
if av == bv:
same_literal += 1
else:
changed_literal += 1
out = [
f"Diff of sheet '{sheet}' ({'range ' + range_name if range_name else 'whole sheet'}):",
f"- frozen (formula -> value in B): {frozen}",
f"- changed formulas: {changed}",
f"- cleared in B: {cleared}",
f"- added in B: {added}",
f"- unchanged formulas: {same_formula} | unchanged literals: {same_literal} | changed literals: {changed_literal}",
]
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@gsheets/sheets_tools.py` around lines 2561 - 2625, Update the comparison
logic around grab, grid_a/grid_b, and a1 to parse each response’s returned range
and map every value to absolute row and column coordinates before comparing,
including offset ranges and differing data bounding boxes; generate addresses
from those absolute coordinates rather than assuming A1 origins. Also add a
distinct counter and output category for changed literals when both cells are
non-formulas with different values, reserving same_literal for equal literal
values.

@taylorwilsdon taylorwilsdon self-assigned this Jul 27, 2026
@taylorwilsdon

Copy link
Copy Markdown
Owner

Interesting idea and I can definitely see the use, looks like a number of things to address first though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants