Egor/dev 1066 add an mcp server to gslides api #53
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
📝 WalkthroughWalkthroughAdds a new MCP server for Google Slides with Pydantic models, utilities, CLI entry, tests, docs, and client credential/child-client lifecycle changes to support per-request clients and MCP tool execution (presentation/slide/element operations, thumbnails, read/write, copy/move/delete). Changes
Sequence Diagram(s)sequenceDiagram
participant User as AI Assistant / User
participant MCP as MCP Server (gslides_api.mcp.server)
participant Client as GoogleAPIClient
participant SlidesAPI as Google Slides API
User->>MCP: invoke tool (e.g., get_slide)
activate MCP
MCP->>MCP: parse_presentation_id()
MCP->>Client: get_api_client() (create child from factory)
activate Client
Client-->>MCP: authenticated per-request client
deactivate Client
MCP->>SlidesAPI: fetch_presentation(presentation_id)
activate SlidesAPI
SlidesAPI-->>MCP: presentation JSON
deactivate SlidesAPI
MCP->>MCP: find_slide_by_name()
MCP->>MCP: _get_effective_format(how)
alt format == OUTLINE
MCP->>MCP: build_slide_outline()
else format == DOMAIN
MCP->>MCP: convert_to_domain_model()
else format == RAW
MCP->>MCP: return raw JSON
end
MCP->>MCP: _format_response()
deactivate MCP
MCP-->>User: JSON/metadata (or error)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@docs/MCP_SERVER.md`:
- Line 12: The Table of Contents entry linking to "#configuration" is broken
because there is no corresponding "## Configuration" heading; either remove or
update the TOC link, or add a "## Configuration" section to the document (create
a heading titled "## Configuration" and include the intended configuration
details), then ensure the TOC entry text matches that exact heading text
("Configuration") so the anchor works.
In `@gslides_api/mcp/models.py`:
- Around line 25-33: The ErrorResponse Pydantic model uses a Dict[str, Any] for
the details field which violates repo guidelines; change ErrorResponse.details
to a typed list of structured items (e.g., List[DetailItem]) and add a new
DetailItem model describing key, value, and optional meta fields; update any
corresponding models (e.g., SuccessResponse) that use Dict to use the same
DetailItem list, and update utils/server emitters to produce and return a list
of DetailItem instances instead of raw dicts so schemas remain explicit and
validated (search for class ErrorResponse and its details field and the
analogous model around lines 64-71 to locate all usages).
In `@gslides_api/mcp/server.py`:
- Around line 376-386: The code writes thumbnails to a deterministic temp
filename (filename/file_path) which can collide under concurrent calls; update
the save logic in the block using thumbnail.payload/image_data and
safe_slide_name to create a unique temp file (e.g., use
tempfile.NamedTemporaryFile or mkstemp with a unique suffix or include a
uuid4/timestamp) and write the image to that unique path, then return that
unique file path instead of a fixed filename to avoid overwrites.
- Around line 64-100: Add an asyncio.Lock named api_client_lock alongside the
global api_client and initialize it in initialize_server so concurrent handlers
can serialize access; specifically, declare api_client_lock:
Optional[asyncio.Lock] = None at module scope, set api_client_lock =
asyncio.Lock() inside initialize_server after creating the GoogleAPIClient, and
update docs/comments to instruct handlers to use "async with api_client_lock:
client = get_api_client()" before calling client methods (or refactor to
per-request GoogleAPIClient instances). Ensure references to api_client,
get_api_client, initialize_server, and GoogleAPIClient are preserved so callers
can adopt the lock-based access pattern.
In `@pyproject.toml`:
- Around line 22-30: The entrypoint gslides-mcp imports mcp.server.FastMCP
unconditionally so invoking the CLI without the optional mcp extra raises
ImportError; either make mcp a required dependency in pyproject by moving mcp =
{version = "^1.0.0"} out of optional extras, or modify gslides_api.mcp.server
(the module that defines FastMCP and the CLI main) to guard the import: wrap the
import of mcp.server.FastMCP in a try/except ImportError and raise a clear
runtime error from main (or when importing) that tells the user to install the
[mcp] extra (e.g., "install with poetry install --extras mcp" or similar),
ensuring the entrypoint fails with a helpful message instead of a raw
ImportError.
In `@tests/mcp_tests/test_utils.py`:
- Around line 14-15: Rename the test class to fix the typo: change the class
name TestParsePresenatationId to TestParsePresentationId in
tests/mcp_tests/test_utils.py so it correctly reflects the parse_presentation_id
function and follows naming conventions used by the test suite; update any
references to TestParsePresenatationId within that file if present.
🧹 Nitpick comments (1)
gslides_api/mcp/__init__.py (1)
30-49: Consider sorting__all__alphabetically.Static analysis (RUF022) flags that
__all__is not sorted. While the current categorical grouping (Server, Models, Utils) is readable, the coding guidelines specify using isort for import sorting. You can apply isort-style sorting to__all__for consistency.♻️ Suggested sorted __all__
__all__ = [ - # Server - "mcp", - "main", - "initialize_server", - # Models - "OutputFormat", - "ThumbnailSizeOption", + # Models "ElementOutline", - "SlideOutline", - "PresentationOutline", "ErrorResponse", + "OutputFormat", + "PresentationOutline", + "SlideOutline", "SuccessResponse", + "ThumbnailSizeOption", + # Server + "initialize_server", + "main", + "mcp", # Utils - "parse_presentation_id", - "get_slide_name", - "get_element_name", + "find_element_by_name", "find_slide_by_name", - "find_element_by_name", + "get_element_name", + "get_slide_name", + "parse_presentation_id", ]
| - [Server Startup](#server-startup) | ||
| - [Tool Reference](#tool-reference) | ||
| - [Output Formats](#output-formats) | ||
| - [Configuration](#configuration) |
Contributor
There was a problem hiding this comment.
Broken TOC link: #configuration section does not exist.
The Table of Contents references [Configuration](#configuration) but there is no ## Configuration heading in the document. Either add the missing section or update the TOC entry.
🔧 Suggested fix - remove or rename the link
- [Output Formats](`#output-formats`)
-- [Configuration](`#configuration`)
- [Troubleshooting](`#troubleshooting`)Or add a ## Configuration section if intended.
📝 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
| - [Configuration](#configuration) | |
| - [Output Formats](`#output-formats`) | |
| - [Troubleshooting](`#troubleshooting`) |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
12-12: Link fragments should be valid
(MD051, link-fragments)
🤖 Prompt for AI Agents
In `@docs/MCP_SERVER.md` at line 12, The Table of Contents entry linking to
"#configuration" is broken because there is no corresponding "## Configuration"
heading; either remove or update the TOC link, or add a "## Configuration"
section to the document (create a heading titled "## Configuration" and include
the intended configuration details), then ensure the TOC entry text matches that
exact heading text ("Configuration") so the anchor works.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…s://github.com/ShoggothAI/gslides-api into egor/dev-1066-add-an-mcp-server-to-gslides-api
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.