feat(EVO-2170): add edit_calendar_event tool (edit / reschedule) - #43
Conversation
Reviewer's GuideAdds a new Google Calendar edit/reschedule tool (edit_calendar_event) to the ADK toolset, wires it into the tool builder, and covers it with unit tests including credential reloading and event-disambiguation behavior. Sequence diagram for the new edit_calendar_event tool behaviorsequenceDiagram
actor Agent
participant edit_calendar_event
participant GoogleCalendarClient
participant Postgres
participant GoogleCalendarAPI
Agent->>edit_calendar_event: call with start_date end_date optional_changes
edit_calendar_event->>edit_calendar_event: _credentials_have_secrets
alt credentials sanitized
edit_calendar_event->>Postgres: _load_full_credentials_from_db
Postgres-->>edit_calendar_event: full_credentials
edit_calendar_event->>edit_calendar_event: _credentials_have_secrets
alt still incomplete
edit_calendar_event-->>Agent: status error (incomplete credentials)
end
end
edit_calendar_event->>GoogleCalendarClient: get_calendar_service
GoogleCalendarClient-->>edit_calendar_event: service
alt event_id provided
edit_calendar_event->>GoogleCalendarAPI: events().get
GoogleCalendarAPI-->>edit_calendar_event: target_event
else search by time range
edit_calendar_event->>GoogleCalendarClient: check_availability
GoogleCalendarClient-->>edit_calendar_event: events
alt zero events
edit_calendar_event-->>Agent: status error (no meeting found)
else multiple events
edit_calendar_event-->>Agent: status needs_clarification with _event_label list
else single event
edit_calendar_event->>edit_calendar_event: build target_event
end
end
edit_calendar_event->>edit_calendar_event: build patch body
edit_calendar_event->>GoogleCalendarAPI: events().patch sendUpdates=all
GoogleCalendarAPI-->>edit_calendar_event: updated_event
edit_calendar_event-->>Agent: status success with _event_label and link
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
_load_full_credentials_from_dbhelper bypasses the injecteddbdependency and opens a new psycopg2 connection using an env DSN, which makes the tool harder to test and configure; consider reusing the existing DB abstraction /dbhandle instead of creating a raw connection here. - The async
edit_calendar_eventfunction performs blocking operations (psycopg2 connect/query and Google API calls) directly, which may block the event loop under load; if this pattern isn't already established, consider offloading these to a sync context or threadpool helper for better async behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `_load_full_credentials_from_db` helper bypasses the injected `db` dependency and opens a new psycopg2 connection using an env DSN, which makes the tool harder to test and configure; consider reusing the existing DB abstraction / `db` handle instead of creating a raw connection here.
- The async `edit_calendar_event` function performs blocking operations (psycopg2 connect/query and Google API calls) directly, which may block the event loop under load; if this pattern isn't already established, consider offloading these to a sync context or threadpool helper for better async behavior.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…ting) Completes the calendar toolset: the agent could create and cancel, but not edit / reschedule an existing meeting. - edit_event.py: new edit_calendar_event tool. Locates the event by event_id (events().get) or by the search window (client.check_availability) + optional title. Applies only the provided fields (new_start_date/new_end_date to reschedule, new_title, new_description) via events().patch(sendUpdates="all"). When only new_start_date is given, the original duration is preserved. Requires at least one change; disambiguates on multiple matches; informative message on none. Same sanitized-credentials DB reload and default calendar as the others. - __init__.py / tool_builder.py: register the tool (log lists edit_event). - tests: reschedule preserves duration; title-only; explicit start+end; no-change error; multiple -> disambiguation; none -> informative; event_id via get; sanitized creds reloaded. Validated in the client's production (14:00-15:00 -> rescheduled to 16:30 -> Google confirmed 16:30-17:30, 1h preserved, title changed). Independent of EVO-2169 (#42, cancel): both add their own tool file. When both merge, the only overlap is the tools-registered log line — a trivial resolution.
eb376cf to
92f7ad0
Compare
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- The
_load_full_credentials_from_dbhelper bypasses the existingdbdependency and opens a new psycopg2 connection from an env var each time; consider wiring this through the existing DB abstraction (or the injecteddbparameter) so connection management, configuration, and pooling remain consistent with the rest of the service. - The
ToolContextparameter ofedit_calendar_eventis currently unused; either remove it from the signature or leverage it (e.g., for logging/trace information) to avoid confusion about its purpose.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `_load_full_credentials_from_db` helper bypasses the existing `db` dependency and opens a new psycopg2 connection from an env var each time; consider wiring this through the existing DB abstraction (or the injected `db` parameter) so connection management, configuration, and pooling remain consistent with the rest of the service.
- The `ToolContext` parameter of `edit_calendar_event` is currently unused; either remove it from the signature or leverage it (e.g., for logging/trace information) to avoid confusion about its purpose.
## Individual Comments
### Comment 1
<location path="src/services/adk/tools/google_calendar/edit_event.py" line_range="140-144" />
<code_context>
+ "message": "Informe ao menos uma alteração (novo horário, novo título ou nova descrição).",
+ }
+
+ # Load full credentials from DB when the ones passed are sanitized.
+ effective_credentials = credentials_config
+ if not _credentials_have_secrets(effective_credentials):
+ full_creds = _load_full_credentials_from_db(effective_agent_id)
+ if _credentials_have_secrets(full_creds):
+ effective_credentials = full_creds
+ logger.info("Loaded full Google Calendar credentials from database")
</code_context>
<issue_to_address>
**issue (performance):** Blocking DB calls inside async tool can stall the event loop.
This async tool calls `_load_full_credentials_from_db` (psycopg2) directly, which will block the event loop under load. Please either load credentials before entering this async context and pass them in, run the DB call in a thread executor, or make this tool synchronous if the framework supports it.
</issue_to_address>
### Comment 2
<location path="src/services/adk/tools/google_calendar/edit_event.py" line_range="38-47" />
<code_context>
+ body["end"] = {"dateTime": _parse(new_end_date).isoformat(), "timeZone": timezone}
+ else:
+ # keep original duration
+ try:
+ o_start = _parse(target["start"].get("dateTime"))
+ o_end = _parse(target["end"].get("dateTime"))
+ duration = o_end - o_start
+ except Exception:
+ duration = timedelta(hours=1)
</code_context>
<issue_to_address>
**suggestion:** Handling all-day events when preserving original duration could be more precise.
When `dateTime` is absent for all-day events (only `start['date']`/`end['date']` present), this `try` will always fail and we fall back to a 1‑hour duration. Consider parsing the date-only fields to compute the real duration so all‑day events aren’t reduced to 1 hour when rescheduling with only `new_start_date`.
</issue_to_address>
### Comment 3
<location path="tests/unit/test_google_calendar_edit.py" line_range="140-149" />
<code_context>
+ svc.events.return_value.patch.assert_called_once()
+
+
+def test_sanitized_credentials_are_reloaded_from_db():
+ svc = _mock_service()
+ gcs = MagicMock(return_value=svc)
+ with patch(
+ "src.services.adk.tools.google_calendar.edit_event._load_full_credentials_from_db",
+ return_value=FULL_CREDS,
+ ) as load_mock, patch.object(
+ GoogleCalendarClient, "get_calendar_service", new=gcs
+ ), patch.object(
+ GoogleCalendarClient,
+ "check_availability",
+ new=AsyncMock(return_value={"status": "success", "available": False, "events": [TARGET]}),
+ ):
+ res = asyncio.run(
+ _tool(SANITIZED_CREDS)(
+ start_date="2026-07-20T00:00:00",
+ end_date="2026-07-20T23:59:59",
+ new_title="X",
+ )
+ )
+ load_mock.assert_called_once_with(AGENT_ID)
+ assert res["status"] == "success"
+ gcs.assert_called_once_with(FULL_CREDS)
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test for the case where credentials are incomplete and the DB also fails to provide full secrets
The implementation also handles the case where `_credentials_have_secrets` is still `False` after `_load_full_credentials_from_db`. Please add a test where `credentials_config` is sanitized and `_load_full_credentials_from_db` returns `None` (or creds without secrets), and assert that the tool returns `status == 'error'` with a message about incomplete/missing OAuth secrets, and that `GoogleCalendarClient.get_calendar_service` is not called. This locks in the failure behaviour and prevents regressions that might start using unusable credentials.
</issue_to_address>
### Comment 4
<location path="tests/unit/test_google_calendar_edit.py" line_range="61-68" />
<code_context>
+ )
+
+
+def test_reschedule_new_start_only_preserves_duration():
+ svc = _mock_service()
+ res = _run_search(_tool(), [TARGET], svc, new_start_date="2026-07-20T16:30:00-03:00")
+ assert res["status"] == "success"
+ body = svc.events.return_value.patch.call_args.kwargs["body"]
+ assert "16:30:00" in body["start"]["dateTime"]
+ assert "17:30:00" in body["end"]["dateTime"] # original 1h duration preserved
+ assert "summary" not in body # title untouched
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test that exercises the fallback duration logic for events without `dateTime` (all-day events)
Since the duration logic falls back to a hard-coded `timedelta(hours=1)` when parsing `start`/`end` fails (e.g., all-day events with only `date`), please add a test where the target event uses only `start['date']`/`end['date']` (or another unparsable structure), calls the tool with `new_start_date`, and asserts that `end` is exactly 1 hour after `start` to cover this fallback path.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| # Load full credentials from DB when the ones passed are sanitized. | ||
| effective_credentials = credentials_config | ||
| if not _credentials_have_secrets(effective_credentials): | ||
| full_creds = _load_full_credentials_from_db(effective_agent_id) | ||
| if _credentials_have_secrets(full_creds): |
There was a problem hiding this comment.
issue (performance): Blocking DB calls inside async tool can stall the event loop.
This async tool calls _load_full_credentials_from_db (psycopg2) directly, which will block the event loop under load. Please either load credentials before entering this async context and pass them in, run the DB call in a thread executor, or make this tool synchronous if the framework supports it.
| try: | ||
| import psycopg2 | ||
|
|
||
| conn = psycopg2.connect(dsn) | ||
| try: | ||
| cur = conn.cursor() | ||
| cur.execute( | ||
| "SELECT config FROM evo_core_agent_integrations " | ||
| "WHERE agent_id = %s AND provider = %s LIMIT 1", | ||
| (str(agent_id), "google_calendar_credentials"), |
There was a problem hiding this comment.
suggestion: Handling all-day events when preserving original duration could be more precise.
When dateTime is absent for all-day events (only start['date']/end['date'] present), this try will always fail and we fall back to a 1‑hour duration. Consider parsing the date-only fields to compute the real duration so all‑day events aren’t reduced to 1 hour when rescheduling with only new_start_date.
| def test_sanitized_credentials_are_reloaded_from_db(): | ||
| svc = _mock_service() | ||
| gcs = MagicMock(return_value=svc) | ||
| with patch( | ||
| "src.services.adk.tools.google_calendar.edit_event._load_full_credentials_from_db", | ||
| return_value=FULL_CREDS, | ||
| ) as load_mock, patch.object( | ||
| GoogleCalendarClient, "get_calendar_service", new=gcs | ||
| ), patch.object( | ||
| GoogleCalendarClient, |
There was a problem hiding this comment.
suggestion (testing): Add a test for the case where credentials are incomplete and the DB also fails to provide full secrets
The implementation also handles the case where _credentials_have_secrets is still False after _load_full_credentials_from_db. Please add a test where credentials_config is sanitized and _load_full_credentials_from_db returns None (or creds without secrets), and assert that the tool returns status == 'error' with a message about incomplete/missing OAuth secrets, and that GoogleCalendarClient.get_calendar_service is not called. This locks in the failure behaviour and prevents regressions that might start using unusable credentials.
| def test_reschedule_new_start_only_preserves_duration(): | ||
| svc = _mock_service() | ||
| res = _run_search(_tool(), [TARGET], svc, new_start_date="2026-07-20T16:30:00-03:00") | ||
| assert res["status"] == "success" | ||
| body = svc.events.return_value.patch.call_args.kwargs["body"] | ||
| assert "16:30:00" in body["start"]["dateTime"] | ||
| assert "17:30:00" in body["end"]["dateTime"] # original 1h duration preserved | ||
| assert "summary" not in body # title untouched |
There was a problem hiding this comment.
suggestion (testing): Add a test that exercises the fallback duration logic for events without dateTime (all-day events)
Since the duration logic falls back to a hard-coded timedelta(hours=1) when parsing start/end fails (e.g., all-day events with only date), please add a test where the target event uses only start['date']/end['date'] (or another unparsable structure), calls the tool with new_start_date, and asserts that end is exactly 1 hour after start to cover this fallback path.
…ty + create_event The tools defaulted calendar_id to 'primary' and never read the calendar the user picks in the UI (settings.selectedCalendarId), so the agent operated on the OAuth account's primary calendar instead of the configured one. Resolve the calendar from config (config selection is authoritative; fall back to the tool arg / primary) and use it in every API call. Tests assert the selected calendar is used and that an empty/whitespace selection falls back to primary. Part of EVO-2171 (also applied to cancel_event #42 and edit_event #43).
Resolve the calendar from config (selectedCalendarId, else the tool arg / primary) so edit locates (events().get / search) and patches the event on the calendar the event lives on, not always primary. Test asserts search + patch target the selected calendar.
…event-tool # Conflicts: # src/services/adk/tool_builder.py # src/services/adk/tools/google_calendar/__init__.py
EVO-2170 — nova tool
edit_calendar_event(editar/remarcar reunião)PR independente, base
develop(completa o toolset junto da #42 cancel).O que faz
event_id(events().get) ou pela janelastart_date/end_date(viaclient.check_availability) + filtrotitle.new_start_date/new_end_date(remarcar),new_title,new_description—patchsó dos campos informados. Exige ≥1 alteração.new_start_dateé dado.service.events().patch(sendUpdates="all")(notifica). Desambiguação em >1; mensagem informativa em 0 (não alucina). Recarrega credenciais do banco quando sanitizadas.Testes (
tests/unit/test_google_calendar_edit.py, 8) — 8 passedremarca preservando duração · só título · start+end explícitos · sem alteração→erro · >1→desambiguação · 0→informa ·
event_idviaget· credenciais sanitizadas recarregam.Validado em produção
14:00–15:00 → remarcado para 16:30 → Google confirmou 16:30–17:30 (1h preservada) + título trocado.
Relação com a #42 (cancel)
As duas são independentes (cada uma adiciona seu próprio arquivo de tool). A única sobreposição é a linha de log que lista as tools registradas em
tool_builder.py— quando a segunda mergear, é um conflito trivial de 1 linha (juntar os nomes). Nenhuma depende da outra.Follow-up
create/check/cancel/edit usam calendário
primary, não oselectedCalendarIdda UI — bug separado.Summary by Sourcery
Add a Google Calendar tool for editing or rescheduling existing events and wire it into the agent toolset.
New Features:
Tests: