Skip to content

feat(client): return created Event(s) from insert_event/insert_events#104

Closed
TimeToBuildBob wants to merge 1 commit intoActivityWatch:masterfrom
TimeToBuildBob:feat/insert-event-return-value
Closed

feat(client): return created Event(s) from insert_event/insert_events#104
TimeToBuildBob wants to merge 1 commit intoActivityWatch:masterfrom
TimeToBuildBob:feat/insert-event-return-value

Conversation

@TimeToBuildBob
Copy link
Contributor

Summary

The server already returns the created event(s) with server-assigned IDs in the response body for both the POST /buckets/{id}/events endpoint, but the Python client was discarding this data by returning None.

This PR changes insert_event and insert_events to return the created event(s) with their server-assigned IDs:

  • insert_event(bucket_id, event) -> Event — returns the created event with ID
  • insert_events(bucket_id, events) -> List[Event] — returns the list of created events with IDs

Backward compatible: callers that ignore the return value continue to work unchanged.

Motivation

The main use case (raised in #77) is getting the server-assigned event ID immediately after insertion, so you can directly use it in delete_event() or get_event() without a separate lookup.

Before:

client.insert_event(bucket_id, event)
# Have to fetch all events and find it by content to get the ID
fetched = client.get_events(bucket_id)
e = [e for e in fetched if e.data == event.data][0]
client.delete_event(bucket_id, e.id)

After:

inserted = client.insert_event(bucket_id, event)
client.delete_event(bucket_id, inserted.id)  # Clean!

Changes

  • aw_client/client.py: Updated insert_event and insert_events to capture and return the response
  • tests/test_client.py: Updated tests to verify returned events have server-assigned IDs; simplified event deletion to use the ID directly from insert_event

Closes #77

The server already returns the created event(s) with server-assigned IDs
in the response body, but the client was discarding this data.

This makes it possible for callers to get the event ID immediately after
insertion, enabling direct use in delete_event/get_event without a
separate lookup.

Closes ActivityWatch#77
@TimeToBuildBob
Copy link
Contributor Author

@greptileai review

@greptile-apps
Copy link

greptile-apps bot commented Mar 11, 2026

Greptile Summary

This PR changes insert_event and insert_events to capture and return the server-assigned Event(s) from the POST /buckets/{id}/events response, instead of discarding it and returning None. This is a useful and backward-compatible improvement that avoids the need for a separate fetch-and-search just to retrieve a server-assigned event ID.

Key changes:

  • insert_event now returns an Event with the server-assigned ID (falls back to the original input event if the server returns an empty list — see inline comment).
  • insert_events now returns List[Event] with server-assigned IDs (no validation that the returned count matches the input count — see inline comment).
  • Tests updated to assert non-null IDs on returned events and to use the returned ID directly for deletion, simplifying the delete-after-insert pattern.

Issues found:

  • The if created else event fallback in insert_event silently returns the unenriched original event (with id=None) when the server responds with an empty list, breaking the contract that callers can rely on the returned event having a server-assigned ID.
  • insert_events has no guard for a partial or empty server response — it silently returns fewer events than were inserted without raising an error.

Confidence Score: 3/5

  • Mostly safe to merge, but the silent fallback in insert_event and the missing count validation in insert_events are real edge-case bugs worth addressing before merging.
  • The happy-path behavior is correct and the tests pass in normal conditions. However, both new methods have silent failure modes: insert_event returns the input event (no server ID) when the response body is empty rather than raising an error, and insert_events returns a shorter-than-expected list with no warning. Since callers are expected to use the returned IDs (the whole point of this PR), these silent failures could cause confusing downstream errors.
  • aw_client/client.py — specifically the fallback on line 185 in insert_event and the missing length check in insert_events.

Important Files Changed

Filename Overview
aw_client/client.py Changed insert_event and insert_events to capture and return server response. insert_event has a silent fallback that returns the original event (without a server ID) when the server responds with an empty list; insert_events has no count validation and silently returns a shorter list on partial responses.
tests/test_client.py Tests updated to assert non-null IDs on inserted events and to use the returned ID directly for deletion. Changes are correct and improve test coverage for the new return values.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant AWClient
    participant AWServer

    Note over Caller,AWServer: insert_event (before PR) — returns None
    Caller->>AWClient: insert_event(bucket_id, event)
    AWClient->>AWServer: POST /buckets/{id}/events [event]
    AWServer-->>AWClient: 200 OK [created_event_with_id]
    AWClient-->>Caller: None (response discarded)

    Note over Caller,AWServer: insert_event (after PR) — returns Event with server ID
    Caller->>AWClient: insert_event(bucket_id, event)
    AWClient->>AWServer: POST /buckets/{id}/events [event]
    AWServer-->>AWClient: 200 OK [created_event_with_id]
    AWClient-->>Caller: Event(id=server_assigned_id, ...)

    Note over Caller,AWServer: insert_events (after PR) — returns List[Event] with server IDs
    Caller->>AWClient: insert_events(bucket_id, [e1, e2, e3])
    AWClient->>AWServer: POST /buckets/{id}/events [e1, e2, e3]
    AWServer-->>AWClient: 200 OK [e1_created, e2_created, e3_created]
    AWClient-->>Caller: [Event(id=1,...), Event(id=2,...), Event(id=3,...)]
Loading

Last reviewed commit: 666d088

self._post(endpoint, data)
response = self._post(endpoint, data)
created = response.json()
return Event(**created[0]) if created else event
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Silent fallback returns event without server-assigned ID

When the server returns an empty list, insert_event silently falls back to returning the original event object (which has id=None if not pre-assigned). This breaks the stated contract of the function — callers expect an Event with a server-assigned ID, but they silently receive the input event instead.

Any caller that does client.delete_event(bucket_id, client.insert_event(bucket_id, e).id) would then call delete_event(bucket_id, None), causing a confusing error downstream rather than a clear failure at the insertion step.

The insert_events sibling doesn't have this fallback at all — it just returns [] on an empty response — creating an inconsistency in error handling between the two methods.

A more defensive approach would be to raise a clear error if the server unexpectedly returns an empty list, rather than silently swallowing the failure:

def insert_event(self, bucket_id: str, event: Event) -> Event:
    endpoint = f"buckets/{bucket_id}/events"
    data = [event.to_json_dict()]
    response = self._post(endpoint, data)
    created = response.json()
    if not created:
        raise Exception("Server returned empty response when inserting event")
    return Event(**created[0])

Comment on lines +187 to +192
def insert_events(self, bucket_id: str, events: List[Event]) -> List[Event]:
endpoint = f"buckets/{bucket_id}/events"
data = [event.to_json_dict() for event in events]
self._post(endpoint, data)
response = self._post(endpoint, data)
created = response.json()
return [Event(**e) for e in created]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

insert_events silently returns fewer events than inserted

If the server returns fewer events than were passed in (e.g., partial insertion, empty response), the function silently returns a shorter list — including [] for a completely empty response — with no warning or error. The caller has no way to detect that some events were not created.

Consider validating that the returned count matches the input count:

def insert_events(self, bucket_id: str, events: List[Event]) -> List[Event]:
    endpoint = f"buckets/{bucket_id}/events"
    data = [event.to_json_dict() for event in events]
    response = self._post(endpoint, data)
    created = response.json()
    if len(created) != len(events):
        raise Exception(
            f"Server returned {len(created)} events, expected {len(events)}"
        )
    return [Event(**e) for e in created]

@ErikBjare
Copy link
Member

Duplicate of #103

@ErikBjare ErikBjare marked this as a duplicate of #103 Mar 11, 2026
@TimeToBuildBob
Copy link
Contributor Author

Closing as duplicate of #103 (already existed, same change). Thanks @ErikBjare for catching this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Return response data from insert_event(s) methods

2 participants