[0xbrayo]

Reduce repeated datastore work on watcher and dashboard requests, and stream large exports with bounded event memory.

Changes:

• Reuse bucket handles and direct existence checks instead of deserializing all bucket metadata per request.
• Merge heartbeats with a bucket-scoped UPDATE by event ID. Preserve IDs returned by inserts, invalidate heartbeat state on other writes (create/update/delete/import), and serialize API writes so event mutations cannot race a merge. Merge invalidation of the query cache is scoped to exactly the replaced event's old and new ranges.
• Fetch bucket metadata and last-update times in one disk-backend query.
• Stream JSON exports through datastore cursors in roughly 64 KiB chunks. Preserve the export schema and event ordering, omit event IDs, return missing-bucket errors before streaming, and close cursors on interrupted responses.
• Fix update_bucket passing type= instead of the datastore's type_id= argument.
• Defer event/heartbeat debug formatting, with regression tests for disabled logging.

This PR no longer adds its own query-result cache: it was rebased onto the finished-period query cache that landed in #174, which it builds on (the write serialization and invalidation hooks are shared).

This PR depends on ActivityWatch/aw-core#159. Both pyproject.toml and poetry.lock pin the tested aw-core commit while these coordinated changes are reviewed. The release dependency can replace that pin after the companion changes ship.

Validation:

• Server tests (tests/test_server.py, profile, query cache, and the new tests/test_performance.py): 92 passed, 2 deliberate skips for disk backends in the API-only concurrency test.
• Regression coverage spans all three storage backends, lifecycle changes, failed writes, concurrent API heartbeats, export equivalence/cancellation, and lazy logging.
• scripts/profile-api-query.py reproduces the baseline query profile against a disposable 10,000-event bucket.

[0xbrayo]

🤖 Claude, on behalf of @0xbrayo

@greptile review

[greptile-apps]

Confidence Score: 5/5

[High impact] The PR appears safe to merge based on the changes since the last review.

Summary

This PR reduces repeated database work for heartbeats and bucket lookups, and streams exports instead of loading every event at once.

• Bucket requests use direct datastore lookups.
• Heartbeat merges keep the stored event ID.
• JSON exports stream bucket events in bounded batches.
• Debug logs skip formatting when debug output is off.
• A profiling script compares query work on disposable event data.

_{Reviews (4) · Last reviewed commit: "fix(rest): drop duplicate flask imports ..." · Reviewed by Greptile}

[0xbrayo]

🤖 Claude, on behalf of @0xbrayo

@greptile review

[TimeToBuildBob]

Thanks @0xbrayo, this is a solid change and the write-serialization + _forget_last_event invalidation is a good way to make the ID-based merge safe. I read the full diff; non-blocking notes first, then what I'd want before merge.

Looks right

• update_bucket(type= → type_id= is a genuine bug fix, worth calling out in the changelog.
• Streaming export validates the bucket before returning the generator (404 instead of a broken stream), uses closing() on both the generator and the datastore cursor, and stream_with_context keeps the request context (and so fix(storage): close request database connections #171's teardown connection release) alive until the stream is consumed.
• Merge-by-ID falls back to a plain insert and drops cached state when replace reports no row, so a row deleted by another datastore user can't be silently lost.

Questions / requests

1. Event ordering in exports. The old path used get(limit=-1) (newest first). Please confirm iter_events() yields the same order, and that the "export equivalence" test asserts order, not only set equality, so import/export round-trips don't change.
2. Heartbeat merge semantics vs. before. replace_last always replaced the stored last event; replace(merged.id, …) replaces the event the cache held. These differ when the cached last_event is not the stored last (out-of-order heartbeat). Is that covered by a test, and does it match aw-server-rust, which merges only into the newest event? The deleted FIXME comment described exactly this case, so a test pinning the new behaviour would be good.
3. json.dumps(metadata)[:-1] + ',"events":[' is correct for any non-empty metadata dict but breaks on {}. Probably unreachable, but {**metadata, "events": …} split differently, or an assert, would make the assumption explicit.
4. Benchmark. The PR ships a profile script but no before/after numbers. A short table (heartbeat p50/p99, GET /buckets, 100k-event export peak RSS) on sqlite and peewee would justify the +477 lines and the new locking.
5. Dependency pin. aw-core is pinned to a personal fork commit, so this can't merge until perf(datastore): stream exports and remove redundant event reads aw-core#159 ships and the pin returns to a release range. Worth stating in the PR as a hard merge gate.
6. Nit: from contextlib import closing lands after pathlib/socket imports; isort would place it earlier.

Not testing the TCC-free parts myself beyond reading; happy to re-check after a benchmark and answers on 1–2.

[0xbrayo]

🤖 Claude, on behalf of @0xbrayo

@greptile review

[TimeToBuildBob]

Rechecked the rebased head d25c5e2. The failing lint job is a real rebase regression: aw_server/rest.py imports both Response and stream_with_context twice. Remove their second occurrences from the Flask import block (keep request and one of each); CI reports exactly those two F811 errors. The three platform test jobs, typecheck and format passed. I cannot push the two-line repair to the author fork with my current permissions.

One correction to my earlier dependency note: ActivityWatch/aw-core#159 is now merged, so that merge is no longer a blocker. This PR still pins the personal-fork SHA; before merging, replace it with the appropriate released dependency or an explicitly agreed upstream pin and regenerate the lockfile.

My earlier correctness checks remain unproven by the current tests: export equivalence uses only one event, so it cannot detect an ordering regression, and the heartbeat tests do not exercise the cached out-of-order-event case. Those are author follow-ups, not failures demonstrated by the passing suite. I have not changed the branch or retriggered review.

[0xbrayo]

🤖 Claude, on behalf of @0xbrayo

@greptile review

[TimeToBuildBob]

Confirmed the duplicate-import fix in d4c58f5: the commit removes exactly the extra Response and stream_with_context imports. All eight checks on that head now pass, including lint and all three platform test jobs. That resolves the lint failure I reported; the dependency-pin and test-evidence follow-ups in my previous comment remain separate. No additional code change or review retrigger from me.