fix: keep ctx.last_seen_uid in sync during IDLE mode#53
Conversation
In IDLE mode, fetch_new_emails reused a stale FetchContext across notification cycles. The UID was persisted to DB via save_uid but the in-memory ctx.last_seen_uid was never updated, causing all previously processed emails to be re-fetched from IMAP on every IDLE notification. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
|
||
| **Verification:** All 222 existing tests pass. Added 3 new tests in `tests/test_fetch_new_emails.py` covering: (1) ctx updates after processing emails, (2) ctx updates when emails are skipped by routing, (3) ctx unchanged when no new emails. Full suite: 225 passed. | ||
|
|
||
| **Risk:** None identified. The `FetchContext` is a non-frozen dataclass, mutation is safe. The `ctx` is only used within the `fetch_new_emails` → `idle_loop` call chain. Polling mode already creates fresh contexts and is unaffected. |
There was a problem hiding this comment.
This file duplicates information already in the PR description and commit messages. It will go stale quickly and adds maintenance burden. Consider removing it — git history and the PR are the appropriate places for this context.
| """Tests for fetch_new_emails keeping ctx.last_seen_uid in sync.""" | ||
|
|
||
| import pytest | ||
| from unittest.mock import AsyncMock, MagicMock, patch |
There was a problem hiding this comment.
MagicMock and WorkerState (line 11) are imported but never used. Remove them to keep the test file clean.
| imap = AsyncMock() | ||
| uid_str = " ".join(str(u) for u in uids).encode() | ||
| imap.uid_search = AsyncMock(return_value=("OK", [uid_str] if uids else [b""])) | ||
| imap.uid = AsyncMock(return_value=("OK", [bytearray(email_bytes)])) |
There was a problem hiding this comment.
Suggestion (test gap): This mock returns the same email for every UID and doesn't validate the search criteria string. Consider adding an assertion that uid_search is called with the expected UID {last_seen_uid + 1}:* criteria — that's the core invariant this fix protects. Without it, these tests would still pass even if the ctx.last_seen_uid update were removed (the mock doesn't change behavior based on UID range).
Also: for test_ctx_last_seen_uid_updated_after_processing, consider testing with a second call to fetch_new_emails on the same ctx to verify that the updated last_seen_uid actually affects the next IDLE cycle's search range. That would be a true integration-style test of the fix.
1 participant
Summary
fetch_new_emailspersisted the last processed UID to the database viasave_uid()but never updated the in-memoryctx.last_seen_uid. Since IDLE mode reuses the sameFetchContextacross notification cycles, every IDLE notification caused all previously-processed emails to be re-fetched from the IMAP server and re-checked against the dedup table. The overhead grew linearly with each new email during a session.ctx.last_seen_uid = uidafter eachsave_uid()call (2 lines). Polling mode was already unaffected since it creates a fresh context each cycle.Test plan
🤖 Generated with Claude Code