feat: add Google Calendar tool#134
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Google Calendar integration: a new Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Example as Example Script
participant Agentor as Agentor
participant CalendarTool as CalendarTool
participant GoogleAPI as Google Calendar API
User->>Example: run script / trigger query
Example->>Example: get_calendar_credentials()
Note over Example: load credentials.json or run OAuth flow
Example->>Agentor: instantiate with CalendarTool(credentials)
User->>Agentor: "What events do I have tomorrow?"
Agentor->>CalendarTool: list_events(start_time, end_time, ...)
CalendarTool->>CalendarTool: normalize times, clamp limits
CalendarTool->>GoogleAPI: events.list(timeMin, timeMax, calendarId, ...)
GoogleAPI-->>CalendarTool: events JSON
CalendarTool->>Agentor: return events as JSON string
Agentor->>User: present final_output
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/agentor/tools/google_calendar.py`:
- Around line 57-64: The _normalize_datetime function must not silently append
'Z' to timezone-naive strings; instead, detect naive datetimes in
_normalize_datetime and raise a clear error (e.g., ValueError) instructing
callers to provide an explicit timezone offset or UTC 'Z' suffix; update callers
or documentation to validate/convert inputs before calling this function so
functions like GoogleCalendarTool._normalize_datetime (and any callers of it) no
longer reinterpret local wall-clock times as UTC.
- Around line 49-55: find_free_slots() currently requests many events but is
silently limited by _clamp_limit (cap 100), causing later busy blocks to be
dropped; update find_free_slots / list_events call to page through the Google
Calendar API (use events.list with pageToken / nextPageToken and aggregate items
until no nextPageToken or until a configured overall max) rather than relying on
_clamp_limit, or add an explicit bypass flag to list_events to avoid clamping
for internal callers; apply the same paging/bypass fix to the second occurrence
of this path referenced around the other call site (the alternate call that also
uses _clamp_limit).
- Around line 248-253: The loop that appends guests to event attendees can add
duplicates when guest_emails contains repeated addresses because existing_emails
is computed once and never updated; inside the loop in google_calendar.py where
existing_emails, attendees, and guest_emails are used, update the dedupe set as
you append (e.g., add the email to existing_emails after
attendees.append({"email": email})) or build a new set of added_emails and check
against that so each new email is only appended once.
- Around line 134-172: In find_free_slots, validate inputs before computing:
ensure meeting_minutes is an integer > 0 and that end_time (after normalizing
via _normalize_datetime and parsing to datetime) is strictly greater than
start_time; if either check fails return an error string starting with "Error:"
(consistent with list_events error style) such as "Error: invalid
meeting_minutes" or "Error: end_time must be after start_time". Perform these
checks using the same datetime parsing logic (replace("Z", "+00:00") then
datetime.fromisoformat) and short-circuit before calling list_events or
computing busy/free ranges.
🪄 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: 531f5fd9-5457-464e-bbe1-6b6940bdca17
📒 Files selected for processing (4)
examples/tools/google_calendar.pysrc/agentor/tools/__init__.pysrc/agentor/tools/google_calendar.pytests/tools/test_google_calendar.py
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/agentor/tools/google_calendar.py (1)
233-233: Use precise type hint forguest_emails.The parameter
guest_emails: listshould beguest_emails: list[str]for better type safety and IDE support. As per coding guidelines, type hints should be used where applicable.Proposed fix
def add_guests( self, event_id: str, - guest_emails: list, + guest_emails: list[str], calendar_id: str = "primary", send_notifications: bool = True, ) -> str:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agentor/tools/google_calendar.py` at line 233, The parameter type hint for guest_emails is too generic; update the function/method signature that declares guest_emails (look for the function in google_calendar.py that has "guest_emails: list") to use a precise type hint "guest_emails: list[str]" and, if the project targets Python <3.9, import and use "List[str]" from typing instead; update any corresponding docstrings or type annotations in that function (and related callers) to match.tests/tools/test_google_calendar.py (2)
123-145: Consider adding edge case tests forfind_free_slots.The test covers a happy path but doesn't verify behavior for:
meeting_minutes <= 0end_time <= start_time- All-day events (events with
dateinstead ofdateTime)These are known gaps in the implementation; adding tests would document expected behavior and catch regressions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/tools/test_google_calendar.py` around lines 123 - 145, Add unit tests in tests/tools/test_google_calendar.py that exercise CalendarTool.find_free_slots for the edge cases: (1) meeting_minutes <= 0 — call find_free_slots with meeting_minutes=0 and assert it returns an error JSON or empty "free_slots" and/or an appropriate error message; (2) end_time <= start_time — call with end_time equal to or before start_time and assert the result indicates invalid window (error JSON or empty "free_slots"); and (3) all-day events — mock the calendar service response to return events with "start": {"date": "..."} / "end": {"date": "..."} and assert find_free_slots skips or handles these events (returns valid free_slots or documents expected behavior). Use the existing _mock_calendar_service and mock_build pattern and assert on parsed JSON keys ("free_slots" or error message) so failures detect regressions in CalendarTool.find_free_slots.
180-204: Test doesn't actually verify deduplication logic.The test passes
["existing@example.com", "new@example.com"]but doesn't verify thatexisting@example.comwas not re-added. The assertion on line 200-203 only checks the final email list, which comes from the mock's fixed response—not from the actual dedup logic in the implementation.To truly test deduplication, assert on the
body["attendees"]sent to theupdatecall before it hits the mock.Proposed improvement
update_kwargs = service.events.return_value.update.call_args.kwargs attendees = update_kwargs["body"]["attendees"] emails = sorted([a["email"] for a in attendees]) + # Verify existing@example.com wasn't duplicated (should appear only once) self.assertEqual( emails, ["existing@example.com", "new@example.com"], ) + # Verify exactly 2 attendees were sent (1 existing + 1 new) + self.assertEqual(len(attendees), 2) self.assertEqual(update_kwargs["sendUpdates"], "none")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/tools/test_google_calendar.py` around lines 180 - 204, The test test_add_guests_success_and_dedup currently reads the attendee list from the mock's preset response rather than asserting what was sent to the Calendar API, so update the test to inspect the actual arguments passed into service.events().update() (use service.events.return_value.update.call_args.kwargs) and assert that the "body"->"attendees" only contains the deduplicated set (e.g., no duplicate "existing@example.com"); ensure the mock service (from _mock_calendar_service) is set up so the original event contains the existing attendee and then verify the update call's "body" and "sendUpdates" fields reflect the correct deduped emails and "none" respectively.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/agentor/tools/google_calendar.py`:
- Around line 93-94: Remove the orphaned standalone docstring line ("""Run a
sample calendar query using authenticated credentials.""") that is inside the
try block immediately before the return statement that calls
json.dumps(events_result.get("items", [])); simply delete that docstring so the
try block contains only executable code (e.g., the return) and no stray string
literal in the function that returns the JSON-serialized calendar events.
- Around line 162-166: The loop over events only reads start/end from
"dateTime", so all-day events using "date" are ignored; update the iteration
over events (where busy_ranges is built) to fallback to event.get("start",
{}).get("date") and event.get("end", {}).get("date") when "dateTime" is absent,
and convert those all-day "date" values into explicit datetime ranges (e.g.,
start at midnight and end at the next midnight or end-of-day) preserving
timezone information if available before appending to busy_ranges; ensure you
still prefer "dateTime" when present and normalize both kinds to the same
datetime format.
- Around line 269-276: The success message currently uses len(guest_emails)
which reports input count rather than actual additions; change it to compute the
real number added by comparing the attendees before and after the update (e.g.,
use the pre-update attendees list (existing attendees from the original event,
often named event or original_event) and the post-update list
updated.get("attendees", [])) and set the message to something like f"Added
{num_added} guest(s) to event." where num_added = len(post_attendees) -
len(pre_attendees) or by counting new emails not present in pre_attendees;
update the return JSON construction (the variables guest_emails, updated, and
the original event/attendees variable) accordingly so the message reflects
actual additions.
---
Nitpick comments:
In `@src/agentor/tools/google_calendar.py`:
- Line 233: The parameter type hint for guest_emails is too generic; update the
function/method signature that declares guest_emails (look for the function in
google_calendar.py that has "guest_emails: list") to use a precise type hint
"guest_emails: list[str]" and, if the project targets Python <3.9, import and
use "List[str]" from typing instead; update any corresponding docstrings or type
annotations in that function (and related callers) to match.
In `@tests/tools/test_google_calendar.py`:
- Around line 123-145: Add unit tests in tests/tools/test_google_calendar.py
that exercise CalendarTool.find_free_slots for the edge cases: (1)
meeting_minutes <= 0 — call find_free_slots with meeting_minutes=0 and assert it
returns an error JSON or empty "free_slots" and/or an appropriate error message;
(2) end_time <= start_time — call with end_time equal to or before start_time
and assert the result indicates invalid window (error JSON or empty
"free_slots"); and (3) all-day events — mock the calendar service response to
return events with "start": {"date": "..."} / "end": {"date": "..."} and assert
find_free_slots skips or handles these events (returns valid free_slots or
documents expected behavior). Use the existing _mock_calendar_service and
mock_build pattern and assert on parsed JSON keys ("free_slots" or error
message) so failures detect regressions in CalendarTool.find_free_slots.
- Around line 180-204: The test test_add_guests_success_and_dedup currently
reads the attendee list from the mock's preset response rather than asserting
what was sent to the Calendar API, so update the test to inspect the actual
arguments passed into service.events().update() (use
service.events.return_value.update.call_args.kwargs) and assert that the
"body"->"attendees" only contains the deduplicated set (e.g., no duplicate
"existing@example.com"); ensure the mock service (from _mock_calendar_service)
is set up so the original event contains the existing attendee and then verify
the update call's "body" and "sendUpdates" fields reflect the correct deduped
emails and "none" respectively.
🪄 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: 18ecbd33-e17b-4e9c-9857-6e27122dd5ae
📒 Files selected for processing (3)
examples/tools/google_calendar.pysrc/agentor/tools/google_calendar.pytests/tools/test_google_calendar.py
✅ Files skipped from review due to trivial changes (1)
- examples/tools/google_calendar.py
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/agentor/tools/google_calendar.py (3)
133-146: Consider validating thatend_timeis afterstart_time.The method validates that
titleis non-empty but doesn't validate thatend_time > start_time. Invalid time ranges will cause a Google API error, but early validation would provide a clearer error message.Suggested validation
if not title: return "Error: title is required." start_time = self._normalize_datetime(start_time) end_time = self._normalize_datetime(end_time) + try: + start_dt = datetime.fromisoformat(start_time.replace("Z", "+00:00")) + end_dt = datetime.fromisoformat(end_time.replace("Z", "+00:00")) + if end_dt <= start_dt: + return "Error: end_time must be after start_time." + except ValueError as e: + return f"Error: Invalid datetime format - {e}" + try: event = {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agentor/tools/google_calendar.py` around lines 133 - 146, In the calendar event creation method, after calling self._normalize_datetime(start_time) and self._normalize_datetime(end_time) add a validation that the normalized end time is strictly after the normalized start time and return a clear error (e.g., "Error: end_time must be after start_time") if not; update the logic around the event dict construction in the same method (the block that builds "event" with "start": {"dateTime": start_time} and "end": {"dateTime": end_time}) so it only proceeds when this check passes.
250-250: Improve type hint forguest_emails.The parameter is typed as
listbut should belist[str]for better type checking and documentation.- guest_emails: list, + guest_emails: list[str],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agentor/tools/google_calendar.py` at line 250, Update the type hint for the function parameter named guest_emails from a bare list to a typed sequence: change guest_emails: list to guest_emails: list[str]; if the project supports Python versions older than 3.9, instead import List from typing and use guest_emails: List[str] so the parameter is explicitly typed as a list of strings.
22-31: Add type hint forcredentialsparameter.The
credentialsparameter lacks a type annotation. Consider adding a type hint for clarity and better IDE/type-checker support.- def __init__(self, credentials, api_key: Optional[str] = None): + def __init__(self, credentials: "Credentials", api_key: Optional[str] = None):Alternatively, use
from typing import TYPE_CHECKINGwith a conditional import to avoid runtime dependency on google-auth.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agentor/tools/google_calendar.py` around lines 22 - 31, The __init__ signature in Calendar (def __init__(self, credentials, api_key: Optional[str] = None)) lacks a type annotation for credentials; update it to an explicit type (e.g., google.oauth2.credentials.Credentials) or use a forward/conditional import with typing.TYPE_CHECKING to avoid runtime dependency, then annotate the parameter (e.g., credentials: Credentials). Modify the __init__ in the Calendar class in src/agentor/tools/google_calendar.py and add the conditional import block so the annotation is available for type checkers/IDEs but doesn't import google-auth at runtime.examples/tools/google_calendar.py (2)
71-73: Credentials file written with default permissions.The credentials file is written without explicit permissions, which may result in world-readable credentials on some systems. For production use, consider setting restrictive permissions.
Suggested fix (for production scenarios)
# Save credentials for future use - with open(CREDS_FILE, "w") as f: - f.write(creds.to_json()) + import stat + with open(CREDS_FILE, "w") as f: + f.write(creds.to_json()) + os.chmod(CREDS_FILE, stat.S_IRUSR | stat.S_IWUSR) # 600 permissionsThis is less critical for an example script but worth considering if this pattern is copied to production code.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/tools/google_calendar.py` around lines 71 - 73, The credentials file is created with default (potentially world-readable) permissions; update the save logic around CREDS_FILE and creds.to_json() to ensure restrictive permissions (e.g., owner read/write only) by either creating the file with explicit mode 0o600 or writing then calling os.chmod(CREDS_FILE, 0o600); ensure this change is applied where CREDS_FILE is opened/written in examples/tools/google_calendar.py so saved credentials are not world-readable.
14-14: Add return type hint.The function lacks a return type annotation.
-def get_calendar_credentials(): +def get_calendar_credentials() -> Credentials:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/tools/google_calendar.py` at line 14, The function get_calendar_credentials is missing a return type annotation; update its signature to include an explicit return type (e.g., -> google.oauth2.credentials.Credentials or -> Optional[Credentials] if it can return None) and add the corresponding import (from google.oauth2.credentials import Credentials) or use typing.Any/Optional if you prefer a looser type; ensure the annotation matches the actual return values inside get_calendar_credentials.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/tools/google_calendar.py`:
- Around line 29-34: Wrap the call to creds.refresh(Request()) in a try/except
that catches google.auth.exceptions.RefreshError (import that symbol) so a
revoked/invalid refresh token doesn't crash; on catching RefreshError remove or
invalidate the existing credentials (e.g. delete CREDS_FILE or set creds = None)
so the code will fall through to the OAuth flow, and log a clear message before
proceeding; reference creds.refresh, RefreshError, CREDS_FILE and Request when
making changes.
---
Nitpick comments:
In `@examples/tools/google_calendar.py`:
- Around line 71-73: The credentials file is created with default (potentially
world-readable) permissions; update the save logic around CREDS_FILE and
creds.to_json() to ensure restrictive permissions (e.g., owner read/write only)
by either creating the file with explicit mode 0o600 or writing then calling
os.chmod(CREDS_FILE, 0o600); ensure this change is applied where CREDS_FILE is
opened/written in examples/tools/google_calendar.py so saved credentials are not
world-readable.
- Line 14: The function get_calendar_credentials is missing a return type
annotation; update its signature to include an explicit return type (e.g., ->
google.oauth2.credentials.Credentials or -> Optional[Credentials] if it can
return None) and add the corresponding import (from google.oauth2.credentials
import Credentials) or use typing.Any/Optional if you prefer a looser type;
ensure the annotation matches the actual return values inside
get_calendar_credentials.
In `@src/agentor/tools/google_calendar.py`:
- Around line 133-146: In the calendar event creation method, after calling
self._normalize_datetime(start_time) and self._normalize_datetime(end_time) add
a validation that the normalized end time is strictly after the normalized start
time and return a clear error (e.g., "Error: end_time must be after start_time")
if not; update the logic around the event dict construction in the same method
(the block that builds "event" with "start": {"dateTime": start_time} and "end":
{"dateTime": end_time}) so it only proceeds when this check passes.
- Line 250: Update the type hint for the function parameter named guest_emails
from a bare list to a typed sequence: change guest_emails: list to guest_emails:
list[str]; if the project supports Python versions older than 3.9, instead
import List from typing and use guest_emails: List[str] so the parameter is
explicitly typed as a list of strings.
- Around line 22-31: The __init__ signature in Calendar (def __init__(self,
credentials, api_key: Optional[str] = None)) lacks a type annotation for
credentials; update it to an explicit type (e.g.,
google.oauth2.credentials.Credentials) or use a forward/conditional import with
typing.TYPE_CHECKING to avoid runtime dependency, then annotate the parameter
(e.g., credentials: Credentials). Modify the __init__ in the Calendar class in
src/agentor/tools/google_calendar.py and add the conditional import block so the
annotation is available for type checkers/IDEs but doesn't import google-auth at
runtime.
🪄 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: add588db-7315-4b2a-90c3-2ba38b639d0c
📒 Files selected for processing (3)
examples/tools/google_calendar.pysrc/agentor/tools/google_calendar.pytests/tools/test_google_calendar.py
✅ Files skipped from review due to trivial changes (1)
- tests/tools/test_google_calendar.py
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/agentor/tools/google_calendar.py (2)
181-210:⚠️ Potential issue | 🟡 MinorValidate free-slot inputs before computing availability.
Line 205–209 computes slots even for invalid requests. If
end_time <= start_timeormeeting_minutes <= 0, the method should return anError:response immediately instead of producing a normal payload.💡 Suggested patch
try: start_time = self._normalize_datetime(start_time) end_time = self._normalize_datetime(end_time) + start_dt = datetime.fromisoformat(start_time.replace("Z", "+00:00")) + end_dt = datetime.fromisoformat(end_time.replace("Z", "+00:00")) + if end_dt <= start_dt: + return "Error: end_time must be after start_time." + if not isinstance(meeting_minutes, int) or meeting_minutes <= 0: + return "Error: meeting_minutes must be a positive integer." raw = self.list_events( start_time=start_time, end_time=end_time, calendar_id=calendar_id, limit=250, ) @@ - start_dt = datetime.fromisoformat(start_time.replace("Z", "+00:00")) - end_dt = datetime.fromisoformat(end_time.replace("Z", "+00:00")) free_slots = [] cursor = start_dt needed = timedelta(minutes=meeting_minutes)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agentor/tools/google_calendar.py` around lines 181 - 210, The method that computes free slots should validate inputs early: after normalizing start_time and end_time (see _normalize_datetime usage) and before calling list_events or computing availability, check that end_time > start_time (convert to datetimes using the same fromisoformat(replace("Z", "+00:00")) logic used later) and that meeting_minutes > 0; if either check fails return an "Error: ..." string (consistent with the function's error convention) immediately instead of proceeding to list_events, busy_ranges, or free slot calculation.
286-290:⚠️ Potential issue | 🟡 MinorFix dedupe for repeated new guest emails in the same request.
Line 286–290 checks against
existing_emailsbut never updates it after append, so duplicate entries inguest_emailscan still be added.💡 Suggested patch
for email in guest_emails: if email not in existing_emails: attendees.append({"email": email}) + existing_emails.add(email) added_count += 1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agentor/tools/google_calendar.py` around lines 286 - 290, The loop over guest_emails checks only existing_emails and never updates it, so repeated emails in the same request get added multiple times; inside the loop (where attendees.append({"email": email}) and added_count += 1 occur) update the membership set by adding the new email to existing_emails (or maintain a local set like seen_new_emails and add to it) immediately after appending so subsequent iterations will skip duplicates; ensure you use the same normalized form of email (the same variable used in the membership test) when adding to the set.
🧹 Nitpick comments (3)
tests/tools/test_google_calendar.py (2)
42-288: Prefer functional-style tests instead ofunittest.TestCaseclass grouping.This suite is valid, but it diverges from the project’s preferred testing style for new tests.
As per coding guidelines:
tests/**/*.py: "Prefer functional style tests over other approaches".
167-212: Add regression tests for two edge cases currently unprotected.Please add cases for:
- duplicate values inside
guest_emails(same new email repeated), and- invalid
find_free_slotsrequests (meeting_minutes <= 0,end_time <= start_time).These are easy-to-miss regressions and align with the logic paths in
src/agentor/tools/google_calendar.py.Also applies to: 247-272
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/tools/test_google_calendar.py` around lines 167 - 212, Add regression tests in tests/tools/test_google_calendar.py covering duplicate guest emails and invalid find_free_slots inputs: add a test that calls the CalendarTool method that constructs events (referencing guest_emails) with duplicate email addresses and assert duplicates are deduplicated or handled per implementation (parse the returned JSON or assert behavior used elsewhere in tests), and add tests for CalendarTool.find_free_slots that pass meeting_minutes <= 0 and end_time <= start_time (use the same call pattern as existing tests that parse json result) and assert the function signals failure appropriately (either raises the expected exception with assertRaises or returns an error JSON / empty free_slots consistent with current implementation); use the same mocking of build/service used in nearby tests to isolate calendar API calls.src/agentor/tools/google_calendar.py (1)
22-22: Tighten parameter type hints for public API methods.Line 22 and Line 265 use broad typing where concrete hints are available (
credentials,guest_emails). This will improve static checks and schema clarity.💡 Suggested patch
-from typing import Optional +from typing import Any, Optional @@ - def __init__(self, credentials, api_key: Optional[str] = None): + def __init__(self, credentials: Any, api_key: Optional[str] = None): @@ - guest_emails: list, + guest_emails: list[str],As per coding guidelines:
src/**/*.py: "Use type hints where applicable throughout Python code".Also applies to: 265-265
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agentor/tools/google_calendar.py` at line 22, The public API uses broad/untyped parameters—tighten the hints by changing the __init__ signature parameter credentials to a concrete Google credentials type (e.g., google.auth.credentials.Credentials or google.oauth2.service_account.Credentials) and tighten guest_emails to a sequence of strings (e.g., Sequence[str] or list[str]) in the method that accepts guest_emails; import the appropriate types (from google.auth.credentials import Credentials and typing.Sequence or list) and update the type annotations for the __init__ (credentials) and the method parameter named guest_emails so static checks and schema clarity improve.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/agentor/tools/google_calendar.py`:
- Around line 181-210: The method that computes free slots should validate
inputs early: after normalizing start_time and end_time (see _normalize_datetime
usage) and before calling list_events or computing availability, check that
end_time > start_time (convert to datetimes using the same
fromisoformat(replace("Z", "+00:00")) logic used later) and that meeting_minutes
> 0; if either check fails return an "Error: ..." string (consistent with the
function's error convention) immediately instead of proceeding to list_events,
busy_ranges, or free slot calculation.
- Around line 286-290: The loop over guest_emails checks only existing_emails
and never updates it, so repeated emails in the same request get added multiple
times; inside the loop (where attendees.append({"email": email}) and added_count
+= 1 occur) update the membership set by adding the new email to existing_emails
(or maintain a local set like seen_new_emails and add to it) immediately after
appending so subsequent iterations will skip duplicates; ensure you use the same
normalized form of email (the same variable used in the membership test) when
adding to the set.
---
Nitpick comments:
In `@src/agentor/tools/google_calendar.py`:
- Line 22: The public API uses broad/untyped parameters—tighten the hints by
changing the __init__ signature parameter credentials to a concrete Google
credentials type (e.g., google.auth.credentials.Credentials or
google.oauth2.service_account.Credentials) and tighten guest_emails to a
sequence of strings (e.g., Sequence[str] or list[str]) in the method that
accepts guest_emails; import the appropriate types (from google.auth.credentials
import Credentials and typing.Sequence or list) and update the type annotations
for the __init__ (credentials) and the method parameter named guest_emails so
static checks and schema clarity improve.
In `@tests/tools/test_google_calendar.py`:
- Around line 167-212: Add regression tests in
tests/tools/test_google_calendar.py covering duplicate guest emails and invalid
find_free_slots inputs: add a test that calls the CalendarTool method that
constructs events (referencing guest_emails) with duplicate email addresses and
assert duplicates are deduplicated or handled per implementation (parse the
returned JSON or assert behavior used elsewhere in tests), and add tests for
CalendarTool.find_free_slots that pass meeting_minutes <= 0 and end_time <=
start_time (use the same call pattern as existing tests that parse json result)
and assert the function signals failure appropriately (either raises the
expected exception with assertRaises or returns an error JSON / empty free_slots
consistent with current implementation); use the same mocking of build/service
used in nearby tests to isolate calendar API calls.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 78d21b0e-b5b2-4f4f-bb29-161fcc948e0c
📒 Files selected for processing (2)
src/agentor/tools/google_calendar.pytests/tools/test_google_calendar.py
|
@codex please review this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9778e36f95
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
To use Codex here, create a Codex account and connect to github. |
This pull request adds a new Google Calendar integration to the project, providing a robust
CalendarToolfor interacting with Google Calendar via OAuth credentials. The integration includes capabilities for listing, creating, and deleting events, finding free time slots, and managing event guests. The PR also introduces comprehensive tests and an example usage script.Key changes include:
Google Calendar Tool Integration:
CalendarToolinsrc/agentor/tools/google_calendar.pythat supports listing events, creating events, finding free slots, deleting events, and adding guests to events, all via Google Calendar API. The tool handles OAuth credentials, input validation, and error handling.CalendarToolin thesrc/agentor/tools/__init__.pyfor use throughout the project. [1] [2]Example and Documentation:
examples/tools/google_calendar.pythat demonstrates how to authenticate with Google, instantiate the agent with the calendar tool, and run a sample query for upcoming events.Testing:
tests/tools/test_google_calendar.py, covering initialization, all tool capabilities, error handling, and OpenAI function registration, using mocks for the Google API client.Summary by CodeRabbit
Greptile Summary
This PR adds a
CalendarToolbacked by the Google Calendar API, covering listing events, creating events, finding free slots, deleting events, and managing guests. The tool follows the existingBaseTool/@capabilitypattern well, and the test suite covers the main code paths with clean mocks.There are two correctness bugs in
find_free_slotsand a credential-lifecycle issue in the example that should be fixed before merge:find_free_slots100-event cap — the internal call tolist_events(limit=250)is silently clamped to 100 by_clamp_limit. For a busy calendar this causes later busy periods to be invisible, producing false free-slot results.find_free_slots— Google returns all-day events with adatekey, notdateTime. The current code skips them, so an "Out of Office" all-day block is not treated as busy.creds.expiredor callingcreds.refresh(Request()), so the example will fail with a 401 roughly 1 hour after the initial OAuth flow.add_guestssuccess message — the message always reports the full number of emails passed, not the number actually appended after deduplication.Confidence Score: 2/5
Not safe to merge — two logic bugs in the primary find_free_slots capability can produce silently wrong results in normal usage
The 100-event cap and the all-day-event blind spot in find_free_slots are reproducible in ordinary use and return incorrect data without any error signal to the caller. These are not edge cases; any user with a moderately busy calendar or any all-day event in the window would be affected. The credential refresh omission makes the example non-functional after an hour.
src/agentor/tools/google_calendar.py (find_free_slots method, lines 147–198) and examples/tools/google_calendar.py (credential loading, lines 21–25)
Important Files Changed
Sequence Diagram
sequenceDiagram participant Agent participant CalendarTool participant GoogleCalendarAPI Agent->>CalendarTool: find_free_slots(start, end, meeting_minutes) CalendarTool->>CalendarTool: _normalize_datetime(start/end) CalendarTool->>CalendarTool: list_events(start, end, limit=250) Note over CalendarTool: _clamp_limit(250) → 100 (bug!) CalendarTool->>GoogleCalendarAPI: events.list(maxResults=100) GoogleCalendarAPI-->>CalendarTool: items[] (only first 100 events) CalendarTool->>CalendarTool: parse busy_ranges (skips all-day events, bug!) CalendarTool->>CalendarTool: compute free_slots from gaps CalendarTool-->>Agent: JSON {free_slots} Agent->>CalendarTool: create_event(title, start, end, ...) CalendarTool->>CalendarTool: validate + normalize datetimes CalendarTool->>GoogleCalendarAPI: events.insert(body) GoogleCalendarAPI-->>CalendarTool: created event JSON CalendarTool-->>Agent: JSON event Agent->>CalendarTool: add_guests(event_id, guest_emails) CalendarTool->>GoogleCalendarAPI: events.get(eventId) GoogleCalendarAPI-->>CalendarTool: existing event with attendees CalendarTool->>CalendarTool: dedup + append new attendees CalendarTool->>GoogleCalendarAPI: events.update(body, sendUpdates) GoogleCalendarAPI-->>CalendarTool: updated event JSON CalendarTool-->>Agent: JSON {status, attendees}Reviews (1): Last reviewed commit: "feat: add Google Calendar tool" | Re-trigger Greptile
(2/5) Greptile learns from your feedback when you react with thumbs up/down!