feat(docs): add support for review and suggested edits - #919
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR adds Google Docs Developer Preview suggestion and anchored review-thread tools, staged native Markdown table rendering, Drive label APIs and scopes, expanded tab inspection, and updated Docker/Helm deployment configuration and documentation. ChangesContainer deployment configuration
Drive labels access and tools
Docs table and tab workflows
Docs suggestions and review threads
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
Closing this draft while we complete local validation against the Google Docs Developer Preview. The API surface is active, but the enrolled project currently receives the public schema for the preview fields, so the implementation and live end-to-end workflow are still being validated. We’ll return with a focused, fully tested proposal once the review workflow is confirmed. The development branch is being preserved. |
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@gdocs/docs_tools.py`:
- Around line 94-122: The revision-conflict retry path must preserve
caller-supplied required_revision_id values and avoid treating every
FAILED_PRECONDITION as stale. Update _is_stale_revision_error to require a
specific revision-mismatch signal, and update the retry logic around the write
operation to retry only when the required revision was fetched internally;
otherwise propagate the original conflict unchanged.
- Around line 3055-3064: Update the has_native_tables staged-rendering flow
around clear_requests so existing tab content is not permanently cleared before
all prose, table, and refresh operations succeed. Stage rendering in temporary
content/tab or preserve the original and restore it on failure, while retaining
the existing clear behavior only after staged rendering completes successfully.
- Around line 3070-3086: Update markdown_to_docs_requests to use UTF-16 units
for every intra-block cursor, insertion location, and range calculation instead
of Python string lengths, reusing _utf16_length consistently. Preserve the
existing batch insertion flow and add a regression test covering an emoji before
later paragraphs or styles to verify generated Docs indices remain correct.
In `@README.md`:
- Around line 552-554: Remove the duplicated Google Docs suggestion and native
review-thread enrollment sentence in the README, leaving a single introduction
immediately before the Google Workspace Developer Preview Program link.
In `@tests/gdocs/test_docs_review_tools.py`:
- Around line 486-516: The parameterized test’s expected delete response field
is constructed incorrectly as “deleteedSuggestionIds.” Update
test_manage_doc_suggestion_actions to map the delete action to the real
“deletedSuggestionIds” field while preserving the existing accept and reject
mappings, so the test validates the Docs response contract.
🪄 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: 010f11a6-a6b3-4be6-a2ff-6e0247365052
📒 Files selected for processing (23)
DockerfileREADME.mdauth/permissions.pyauth/scopes.pyauth/service_decorator.pycore/tool_tiers.yamlgdocs/docs_markdown_writer.pygdocs/docs_structure.pygdocs/docs_tools.pygdocs/managers/batch_operation_manager.pygdocs/review.pygdrive/drive_tools.pyhelm-chart/workspace-mcp/templates/deployment.yamlskills/managing-google-workspace/references/docs.mdtests/gdocs/golden/docs_tool_schemas.jsontests/gdocs/test_batch_write_mode.pytests/gdocs/test_docs_markdown_writer.pytests/gdocs/test_docs_review_tools.pytests/gdocs/test_insert_doc_tab_response.pytests/gdocs/test_inspect_doc_tabs.pytests/gdrive/test_drive_labels.pytests/test_permissions.pytests/test_scopes.py
| if has_native_tables: | ||
| # Tables require staged writes because Docs assigns cell indices only | ||
| # after insertTable has completed and the document is fetched again. | ||
| clear_requests = all_requests[:1] if replace_existing and tab_end > 2 else [] | ||
| if clear_requests: | ||
| await asyncio.to_thread( | ||
| service.documents() | ||
| .batchUpdate(documentId=document_id, body={"requests": clear_requests}) | ||
| .execute | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Do not delete existing content before staged rendering succeeds.
The first batch permanently clears the tab, while later prose, table, or refresh calls can fail. This regresses the former atomic update into possible empty or partially rendered content. Stage into temporary content/tab or preserve and restore the original before clearing.
🤖 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 `@gdocs/docs_tools.py` around lines 3055 - 3064, Update the has_native_tables
staged-rendering flow around clear_requests so existing tab content is not
permanently cleared before all prose, table, and refresh operations succeed.
Stage rendering in temporary content/tab or preserve the original and restore it
on failure, while retaining the existing clear behavior only after staged
rendering completes successfully.
| for block in markdown_blocks: | ||
| if block["type"] == "markdown": | ||
| requests = markdown_to_docs_requests( | ||
| block["markdown"], tab_id=tab_id, start_index=insertion_index | ||
| ) | ||
| if not requests: | ||
| continue | ||
| await asyncio.to_thread( | ||
| service.documents() | ||
| .batchUpdate(documentId=document_id, body={"requests": requests}) | ||
| .execute | ||
| ) | ||
| insertion_index += sum( | ||
| _utf16_length(request["insertText"]["text"]) | ||
| for request in requests | ||
| if "insertText" in request | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Use UTF-16 units throughout each Markdown request batch.
Line 3082 updates only the next block’s index with UTF-16 lengths, but markdown_to_docs_requests still advances intra-block request locations with Python len(text). Non-BMP characters before later paragraphs or styles therefore shift Docs indices. Convert all request cursors/ranges and add an emoji regression test.
🤖 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 `@gdocs/docs_tools.py` around lines 3070 - 3086, Update
markdown_to_docs_requests to use UTF-16 units for every intra-block cursor,
insertion location, and range calculation instead of Python string lengths,
reusing _utf16_length consistently. Preserve the existing batch insertion flow
and add a regression test covering an emoji before later paragraphs or styles to
verify generated Docs indices remain correct.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
gdocs/docs_markdown_writer.py (1)
23-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsolidate
_utf16_lengthinto a shared utility module.The
_utf16_lengthhelper function is identically duplicated across four files. To adhere to the DRY principle, consider moving it to a common utility file (e.g.,gdocs/docs_utils.py) and importing it where needed.
gdocs/docs_markdown_writer.py#L23-L26: Remove local definition and import the shared utility.gdocs/docs_tables.py#L16-L20: Remove local definition and import the shared utility.gdocs/managers/table_operation_manager.py#L19-L23: Remove local definition and import the shared utility.gdocs/docs_tools.py#L89-L93: Remove local definition and import the shared utility.🤖 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 `@gdocs/docs_markdown_writer.py` around lines 23 - 26, Move the duplicated _utf16_length helper into the shared gdocs/docs_utils.py utility module, preserving its current UTF-16 code-unit behavior. Remove the local definitions and import the shared utility in gdocs/docs_markdown_writer.py (23-26), gdocs/docs_tables.py (16-20), gdocs/managers/table_operation_manager.py (19-23), and gdocs/docs_tools.py (89-93).
🤖 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.
Nitpick comments:
In `@gdocs/docs_markdown_writer.py`:
- Around line 23-26: Move the duplicated _utf16_length helper into the shared
gdocs/docs_utils.py utility module, preserving its current UTF-16 code-unit
behavior. Remove the local definitions and import the shared utility in
gdocs/docs_markdown_writer.py (23-26), gdocs/docs_tables.py (16-20),
gdocs/managers/table_operation_manager.py (19-23), and gdocs/docs_tools.py
(89-93).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e34b1975-adde-4225-bea9-7e363ac24fc0
📒 Files selected for processing (9)
README.mdgdocs/docs_markdown_writer.pygdocs/docs_tables.pygdocs/docs_tools.pygdocs/managers/table_operation_manager.pytests/gdocs/test_docs_markdown_writer.pytests/gdocs/test_docs_review_tools.pytests/gdocs/test_insert_doc_tab_response.pytests/gdocs/test_table_cell_style.py
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/gdocs/test_docs_markdown_writer.py
- README.md
- tests/gdocs/test_insert_doc_tab_response.py
Description
This PR adds Google Docs review and suggested-edit workflows to the Google Workspace MCP, including anchored review comments, suggestion lifecycle operations, staged Markdown/native-table rendering, Drive label operations, and deployment hardening.
Preview requests use the authorized Google client transport directly because the stable discovery schema does not yet expose all Developer Preview fields.
References:
Capabilities
batch_update_docsupportsEDIT|SUGGESTmodes and revision guards.WORKSPACE_MCP_*configuration, with credential storage restricted to mode700.Preview-dependent capabilities are optional. Stable Docs tools and
EDITmode work without Preview enrollment.SUGGESTmode and native review tools require enrollment in the Google Workspace Developer Preview Program; if unavailable, they fail closed and never fall back to direct edits. Deployments can omit them throughWORKSPACE_MCP_TOOLSor the configured tool tier.Type of Change
Testing
Validation completed:
482 passedbefore the latest review-fix tests; focused review/writer tests:59 passed.Checklist
Additional Notes
Native-table replacement with Markdown tables is intentionally rejected before mutation. Google assigns table-cell indices only after table creation, requiring multiple dependent writes without a cross-call transaction. Use
replace_existing=falseto append safely; transactional replacement can be added in a future change.The PR remains draft for maintainer review.
Summary by CodeRabbit