perf: Skip session stats update during batch upload

[evansenter]

Summary

• Add update_stats param to upload_entries (default: False)
• Add finalize_sync tool to update stats once at end
• Add progress logging with time estimates to push command
• Increase HTTP timeout to 120s
• Fix timezone comparison in incremental sync

Problem

The heavy update_session_stats query was running after every batch, causing timeouts. A single batch of 50 entries was taking 2+ minutes because the stats query scans all events.

Solution

Skip stats update during batch uploads, call finalize_sync once at the end.

Test plan

• Test push with --days 1 completes without timeout
• Verify session stats are updated after finalize_sync

🤖 Generated with Claude Code

[claude]

Prompt: evansenter/dotfiles/.../claude-review.md

Code Review

Summary

This PR adds a performance optimization for the push command by deferring session stats updates until after all batches are uploaded. A new finalize_sync MCP tool is introduced, HTTP timeout is increased to 120s, timezone handling is improved for incremental sync, and progress logging with time estimates is added.

Issues Found

Critical

None

Important

• tests/test_server.py - Missing test for finalize_sync tool. The new MCP tool is not imported or tested. Following the project pattern, there should be at least a basic test similar to test_upload_entries() that verifies finalize_sync.fn() returns {"status": "ok", "sessions_updated": ...}.
• src/agent_session_analytics/guide.md - finalize_sync is not documented. Per CLAUDE.md "Adding Endpoints" guidelines, new MCP tools should be documented in guide.md. The tool should be listed in the "Remote Sync (Multi-Machine)" table alongside get_sync_status and upload_entries.

Suggestions

• cli.py:1681 - The upload_entries call doesn't pass the new update_stats parameter. While the default is False which is correct for batch uploads, explicitly passing update_stats=False would make the intent clearer and document that this is a deliberate choice.
• cli.py:1476 - The benchmark comment mentioning skipped tools should be updated to include finalize_sync alongside upload_entries and get_sync_status for completeness: "- upload_entries, get_sync_status, finalize_sync (remote sync tools - modify DB or require client context)".

Verdict

REQUEST_CHANGES - Missing test and documentation for the new finalize_sync MCP tool. The code changes look correct, but the project's conventions require tests and guide.md updates for new endpoints.

---

Automated review by Claude Code

[claude]

Prompt: evansenter/dotfiles/.../claude-review.md

Code Review

Summary

This PR optimizes the push command by deferring expensive session stats updates until after all batch uploads are complete. It introduces a new finalize_sync MCP tool, increases HTTP timeout to 120s, fixes timezone-aware comparison in incremental sync, adds progress logging with time estimates, and trusts localhost connections to bypass Tailscale auth.

Issues Found

Critical

None

Important

• tests/test_server.py - Missing test for finalize_sync tool. The new MCP tool should be imported and tested following the existing pattern (e.g., test_upload_entries()). A basic test should verify finalize_sync.fn() returns {"status": "ok", "sessions_updated": ...}.
• src/agent_session_analytics/guide.md - finalize_sync is not documented. Per CLAUDE.md "Adding Endpoints" guidelines, new MCP tools must be documented in guide.md. It should be added to the "Remote Sync (Multi-Machine)" table (lines 25-28) alongside get_sync_status and upload_entries.

Suggestions

• cli.py:1681 - The upload_entries call does not pass the new update_stats parameter. While the default False is correct, explicitly passing update_stats=False would make the intent clearer and document that this is deliberate batch behavior.
• cli.py:1476 - The benchmark skip comment should include finalize_sync for completeness: "- upload_entries, get_sync_status, finalize_sync (remote sync tools - modify DB or require client context)".

Verdict

REQUEST_CHANGES - The new finalize_sync MCP tool needs test coverage and documentation in guide.md per project conventions. The performance optimization logic itself looks correct.

---

Automated review by Claude Code

[claude]

Prompt: evansenter/dotfiles/.../claude-review.md

Code Review

Summary

This PR optimizes the push command by deferring expensive session stats updates until all batch uploads complete. It adds a new finalize_sync MCP tool, increases HTTP timeout to 120s, fixes timezone-aware timestamp comparison for incremental sync, adds progress logging with time estimates, and trusts localhost connections to bypass Tailscale auth.

Issues Found

Critical

None

Important

• tests/test_server.py - Missing test for finalize_sync tool. The new MCP tool is not imported or tested. Following the existing pattern (e.g., test_upload_entries), there should be a basic test that imports finalize_sync and verifies finalize_sync.fn() returns {"status": "ok", "sessions_updated": ...}.
• src/agent_session_analytics/guide.md:21-28 - finalize_sync is not documented. Per CLAUDE.md "Adding Endpoints" guidelines, new MCP tools must be documented in guide.md. It should be added to the "Remote Sync (Multi-Machine)" table alongside get_sync_status and upload_entries, and the workflow explanation below should mention calling it after batch uploads complete.

Suggestions

• cli.py:1681 - The upload_entries call does not pass the new update_stats parameter. While the default False is correct for batch uploads, explicitly passing update_stats=False would make the intent clearer and document that this is deliberate batch behavior.
• cli.py:1476 - The benchmark skip comment should include finalize_sync for completeness: "- upload_entries, get_sync_status, finalize_sync (remote sync tools - modify DB or require client context)".

Verdict

REQUEST_CHANGES - The new finalize_sync MCP tool needs test coverage and documentation in guide.md per project conventions (CLAUDE.md "Adding Endpoints" section). The performance optimization logic itself is sound.

---

Automated review by Claude Code

[claude]

Prompt: evansenter/dotfiles/.../claude-review.md

Code Review

Summary

This PR optimizes the push command by deferring expensive session stats updates (update_session_stats) until all batch uploads complete. It introduces a new finalize_sync MCP tool, increases HTTP timeout to 120s, fixes timezone-aware timestamp comparison for incremental sync, adds progress logging with time estimates, and trusts localhost connections to bypass Tailscale auth.

Issues Found

Critical

None

Important

• tests/test_server.py - Missing test for finalize_sync tool. The new MCP tool is not imported or tested. Following the existing pattern (e.g., test_upload_entries), there should be a basic test that verifies finalize_sync.fn() returns {"status": "ok", "sessions_updated": ...}.
• src/agent_session_analytics/guide.md:21-28 - finalize_sync is not documented. Per CLAUDE.md "Adding Endpoints" guidelines, new MCP tools must be documented in guide.md. It should be added to the "Remote Sync (Multi-Machine)" table alongside get_sync_status and upload_entries, with a note explaining it should be called once after all batch uploads complete.

Suggestions

• cli.py:1681 - The upload_entries call does not pass the new update_stats parameter. While the default False is correct for batch uploads, explicitly passing update_stats=False would make the intent clearer and document that this is deliberate batch behavior.
• cli.py:1476 - The benchmark skip comment should include finalize_sync for completeness: "- upload_entries, get_sync_status, finalize_sync (remote sync tools - modify DB or require client context)".

Verdict

REQUEST_CHANGES - The new finalize_sync MCP tool needs test coverage and documentation in guide.md per project conventions (CLAUDE.md "Adding Endpoints" section). The performance optimization logic itself is sound.

---

Automated review by Claude Code

[evansenter]

Feedback Addressed

Implemented

• [Important] Added test for finalize_sync MCP tool (tests/test_server.py)
• [Important] Documented finalize_sync in guide.md Remote Sync table
• [Suggestion] Updated benchmark skip comment to include finalize_sync

Skipped

• [Suggestion] Explicit update_stats=False param - default is already False, adds noise without clarity

[claude]

Prompt: evansenter/dotfiles/.../claude-review.md

Code Review

Summary

This PR optimizes the push command by deferring expensive session stats updates until all batch uploads complete. It introduces a new finalize_sync MCP tool, increases HTTP timeout to 120s, adds SSE response parsing, fixes timezone-aware timestamp comparison for incremental sync, adds progress logging with time estimates, and trusts localhost connections to bypass Tailscale auth.

Issues Found

Critical

None

Important

None

Suggestions

None

Previously Addressed (Filtered)

• tests/test_server.py - Missing test for finalize_sync tool (Implemented)
• guide.md - finalize_sync not documented (Implemented)
• cli.py:1476 - Benchmark skip comment missing finalize_sync (Implemented)
• cli.py:1681 - Explicit update_stats=False param (Skipped: adds noise without clarity)

4 items from prior feedback rounds were not re-raised.

Verdict

APPROVE - All previously raised issues have been addressed. The performance optimization is well-implemented with proper test coverage and documentation.

---

Automated review by Claude Code