Code quality improvements: shared data layer, rate limiter, logging#2
Merged
AdamFerguson06 merged 14 commits intomainfrom Feb 17, 2026
Merged
Code quality improvements: shared data layer, rate limiter, logging#2AdamFerguson06 merged 14 commits intomainfrom
AdamFerguson06 merged 14 commits intomainfrom
Conversation
Move hardcoded magic numbers (rate limits, retry settings, pagination limits, MIME depth) into config.py as module-level constants. Each constant can be overridden via environment variable for deployment flexibility without code changes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add validate_query_length() and validate_gmail_id() to queries.py, providing a single source of truth for input validation used by both the CLI and MCP server. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The MCP SDK requires Python 3.11+, so update requires-python to match. Add upper bounds to all dependencies to prevent breaking changes from unexpected major version bumps. Add pytest-cov and pytest-asyncio to test extras. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace stderr prints with logging.getLogger(__name__) for consistent structured logging. Add platform note about POSIX-only fcntl dependency. Log successful token refresh for observability. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace lock-based fixed-interval rate limiter with a token bucket algorithm that allows short bursts while maintaining average rate. Retry on transient 500/502/503 errors in addition to 429. Honor Retry-After header. Reduce base backoff from 10s to 2s (configurable via GMAIL_RETRY_BASE_WAIT). Add operation_name parameter for better log context. Replace print(stderr) with logging module. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add public fetch_message_details(), fetch_message_full_detail(), fetch_thread_details(), and fetch_labels() functions that both the CLI and MCP server can share, eliminating ~73 lines of duplicated message-fetching logic. Fix lambda late-binding closure bugs in loops by using default arguments (lambda mid=msg_id: ...) to capture the current value instead of the loop variable reference. Apply Gmail API field masks (fields= parameter) to all API calls, reducing bandwidth by 40-60%. Add partial failure handling: individual message fetch errors are logged and skipped instead of aborting the entire operation. Replace all print(stderr) calls with structured logging. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace all duplicated message-fetching loops with calls to shared functions from reports.py (fetch_message_details, fetch_message_full_detail, fetch_thread_details, fetch_labels). This eliminates lambda closure bugs and ~70 lines of duplicated code. Wrap call_tool() in try/except for HttpError and generic Exception so API errors return structured error messages instead of crashing the MCP server process. Add input validation via validate_gmail_id() and validate_query_length() from queries.py. Use MCP_EXPORT_LIMIT from config.py instead of a local constant. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add 'test' command that calls getProfile() to verify credentials and API connectivity. Add --verbose/-v flag that enables DEBUG-level logging across all modules. Replace duplicate MAX_QUERY_LENGTH and _GMAIL_ID_PATTERN with shared validators from queries.py. Separate exception handling into HttpError (API errors), EnvironmentError (config), and generic Exception for clearer error messages. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Run the full test suite on push to main and on pull requests. Tests against Python 3.11, 3.12, and 3.13. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Provides build/test commands, architecture overview, key patterns (lambda closures, field masks, shared data layer), security constraints, and configuration guidance. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
- MCP export now fetches full messages with body content instead of metadata-only - threadId sourced from API response instead of input stub (was always empty) - Token bucket releases lock before sleeping to avoid serializing concurrent callers - Added "To" to metadataHeaders so the field is actually populated Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Track token debt as negative balance (-1.0) so concurrent threads see the reservation and compute correct sleep times - Remove unused MESSAGE_LIST_FIELDS import from reports.py - CLI export now uses shared fetch_message_full_detail instead of duplicating the same inline lambda Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
GMAIL_RATE_LIMIT_RPS env var override could be set to 0 or negative, causing division by zero in token bucket. Now floors at 1. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| # Tunable constants with environment variable overrides. | ||
| # Gmail API quota: 250 units/user/second. Default 25 req/sec = 125 units/sec (50% of limit). | ||
| GMAIL_RATE_LIMIT_RPS = max(1, int(os.getenv("GMAIL_RATE_LIMIT_RPS", "25"))) | ||
| GMAIL_MAX_RETRIES = int(os.getenv("GMAIL_MAX_RETRIES", "3")) |
There was a problem hiding this comment.
Negative GMAIL_MAX_RETRIES silently breaks all API calls
Medium Severity
GMAIL_MAX_RETRIES lacks a lower-bound guard, unlike GMAIL_RATE_LIMIT_RPS which uses max(1, ...). If a user sets GMAIL_MAX_RETRIES to a negative value via the env var, range(GMAIL_MAX_RETRIES + 1) produces an empty range, so request_callable() is never invoked. The function falls through to raise RuntimeError("Unexpected retry loop exit"), completely breaking every API call with a confusing error message unrelated to the actual cause.
Additional Locations (1)
| _fetch_all_messages, | ||
| _parse_body, | ||
| _parse_headers, | ||
| _format_date, |
There was a problem hiding this comment.
Private functions used as cross-module shared API
Low Severity
The MCP server imports five _-prefixed "private" functions from reports.py (_fetch_all_message_ids, _fetch_all_messages, _parse_body, _parse_headers, _format_date). These are clearly part of the shared functionality used by both modules, but the underscore prefix signals they're internal implementation details. This contradicts the PR's goal of a shared data layer and creates a fragile cross-module dependency on functions whose contract isn't considered stable.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
fetch_message_details(),fetch_message_full_detail(),fetch_thread_details(), andfetch_labels()into reusable functions in reports.py, eliminating ~70 lines of duplicated code between CLI and MCP serverlambda mid=msg_id: ...) to capture current valuesfields=parameter using existing constants from queries.py, reducing bandwidth 40-60%call_tool()in try/except for HttpError + Exception so API errors return structured messages instead of crashing the serverprint(stderr)calls withlogging.getLogger(__name__)across all modules; add--verbose/-vCLI flagvalidate_query_length()andvalidate_gmail_id()to queries.py, removing duplicates from CLI and MCP serverTest plan
pytest tests/ -v)python -c "from gmail_reader import client, reports, config, queries, auth")testcommand and-vflaggit check-ignore)🤖 Generated with Claude Code
Note
Medium Risk
Touches core Gmail API call/limit/retry behavior and refactors shared fetch paths used by both CLI and MCP server, which could change performance or failure modes despite being read-only.
Overview
Refactors Gmail access to centralize data fetching and validation: common
fetch_*helpers are added toreports.py, CLI and MCP server switch to these helpers, and query/ID validation is moved intoqueries.py(removing duplicated checks).Hardens API interactions by replacing the fixed-interval limiter with a token-bucket rate limiter plus configurable retries (including transient 5xx and honoring
Retry-After), applying field masks to reduce response size, and improving error handling (CLI catchesHttpError/config errors; MCP server returns structured error text instead of crashing).Adds operational polish: new CLI
testcommand and--verboselogging, centralizes tunables inconfig.pywith env overrides, updates packaging to Python >=3.11 with dependency upper bounds and expanded test deps, and introduces GitHub Actions CI while ignoring generated audit reports.Written by Cursor Bugbot for commit f085d04. This will update automatically on new commits. Configure here.