docs: correct the dry-run lock and exit-code claims, record the sweep measurement

Two README statements were false rather than merely incomplete. "--dry-run
takes no lock at all, so it can always be run against an export that is
currently in progress" contradicted the paragraph four lines above it
warning that two clients sharing one session is a corruption hazard:
dry-run opens the session file like any other run. Both places now say
what is true - no *export* lock, but a session lock, so an alongside
dry-run needs its own --session.

The exit table promised 1 for a bad argument. argparse exits 2 on its
own, before main()'s try block can map anything, so a bad --limit
collided with the dry-run SHORT verdict. Documented as the shared code
it is instead of a code the tool never returns.

Also notes that both locks are flock-based, and that flock is advisory or
per-client on NFS, so single-instance enforcement is not guaranteed
there. Setup gains the venv creation step it assumed.

Phase 7 gains step 12b: measure the sweep cost under a date filter before
deciding whether to optimize it. With reverse=True and no offset_date,
telethon starts the sweep at message id 1 - verified in the pinned wheel -
so --since walks the whole history discarding messages in keep(), and
--until never terminates early. Neither fix is free: offset_date can
start the sweep mid-album, which would take a post_id that is not the
album's lowest id and break filter-invariant identity, and an --until
break assumes dates rise with ids, false for imported history. The
swept-to-kept ratio decides whether either risk is worth taking.

Reports from the review pass are recorded under plans/reports/, including
the findings left unfixed: resolve_entity catching only ValueError,
_find_in_dialogs being the one network loop outside a flood primitive,
SystemExit in session.py routing around the Abort contract, and
completed_at being cleared before any work begins.

README.md

@@ -230,8 +230,9 @@ unbounded error.
 
 `--dry-run` reads the cursor and measures from where a real run would start, so
 `tg-export --dry-run && tg-export` works on a partially completed export. It takes
-no lock, creates no directories, and mutates nothing — you can run it while a real
-export is in progress.
+no export lock, creates no directories, and mutates nothing. It does open the
+session file, so running it alongside an export in progress needs its own
+`--session` — see [Concurrent runs](#concurrent-runs).
 
 ## Concurrent runs
 
@@ -244,16 +245,21 @@ Exporting two *different* groups at the same time works, but each needs its own
 is locked too, because two Telethon clients sharing one session is a corruption
 hazard Telethon itself warns about.
 
-`--dry-run` takes no lock at all, so it can always be run against an export that
-is currently in progress.
+`--dry-run` takes no *export* lock, so it never contends for an export root. It
+does open the session file and therefore takes the session lock: to run it
+alongside an export in progress, give it its own `--session`.
+
+Both locks are `flock`-based. On NFS and some network filesystems `flock` is
+advisory-only or silently per-client, so single-instance enforcement is not
+guaranteed there — keep the session file and the export root on local storage.
 
 ## Exit codes
 
 | Code | Meaning |
 |---|---|
 | 0 | success |
-| 1 | unexpected error, or a bad argument |
-| 2 | `--dry-run` says it will not fit |
+| 1 | unexpected error |
+| 2 | `--dry-run` says it will not fit, **or** a bad/missing argument — argparse exits 2 too |
 | 3 | disk exhausted, or another run holds the lock |
 | 4 | session invalidated — re-login |
 | 5 | lost access to the group |
@@ -264,8 +270,9 @@ is currently in progress.
 ## Development
 
 ```bash
+python3 -m venv .venv
 .venv/bin/pip install -r requirements.txt -e ".[dev]"
-.venv/bin/pytest
+.venv/bin/python -m pytest              # 214 tests, ~3 s
 ```
 
 The whole suite runs offline against synthetic message stubs — no credentials, no

plans/260811-1411-telegram-group-media-exporter/phase-07-end-to-end-validation.md

@@ -56,6 +56,14 @@ Every step below has a checkable outcome. **No step may pass by escape clause.**
 
 12. **Memory** — `/usr/bin/time -v tg-export --dry-run` → peak RSS under 200 MB. The album tripwire set is O(albums), not O(1); the criterion states the real bound.
 
+12b. **Sweep cost under a date filter** — measure before deciding whether to optimize it. With `reverse=True` and no `offset_date`, telethon 1.44's `_MessagesIter._init` starts the sweep at message id 1, so `--since` walks the entire history discarding messages in `keep()`. Time `--dry-run --since <recent date>` against a large group and record the wall clock, the getHistory round-trip count, and the ratio of messages swept to messages kept. Do the same for `--until`, which currently never terminates the sweep early.
+
+  Both fixes carry an invariant risk, which is why this is a measurement rather than a change:
+  - server-side `offset_date` can start the sweep *mid-album*, so `_close` would take a `post_id` that is not the album's lowest id — breaking filter-invariant post identity. Needs a mid-album guard before it is safe.
+  - an early break on `--until` assumes message dates rise monotonically with ids, which is false for imported history.
+
+  Decide with the numbers: if the ratio is small the current sweep is fine, and neither risk is worth taking.
+
 13. **Revoke the spike session** — Telegram → Settings → Devices → terminate the phase 1 probe session. **Deleting `spike/probe.session` does not revoke server-side authorization**; a live auth key otherwise survives for an account you believe is clean.
 
 ## Related Code Files

plans/reports/code-review-260822-2259-full-codebase-audit.md

@@ -0,0 +1,414 @@
+# Full-codebase review — telegram-exporter
+
+Date: 2026-08-22 · Scope: read-only, no source/test/config modified
+Reviewed: `src/telegram_exporter/*.py` (~1,900 LOC), `README.md`, plan index, `tests/` (skim)
+Method: line-by-line read; Telethon 1.44.0 wheel downloaded to a scratch dir and
+`telethon/client/messages.py` read to check the traversal claims. Test suite NOT run
+(pytest unavailable here; execution owned by another agent).
+
+## Verdict
+
+The design is unusually disciplined and the three invariants hold as written. The
+correctness core — sanitizer, `safe_join`, album grouping, cursor ordering, error
+taxonomy — survived a deliberate attempt to break it. What is left is a small set
+of ordering/edge defects at the *outside* of that core (process startup, sidecar
+rotation, sidecar repair, entity resolution) plus two genuinely large throughput
+wins that the current sweep leaves on the table.
+
+Nothing here contradicts an already-adjudicated decision. Where a finding touches
+plan open question 9, 11 or 12, that is stated and the finding is narrowed to the
+part those questions do not cover.
+
+---
+
+## Must fix (correctness)
+
+### M1 — the session lock is acquired after the thing it protects (High)
+
+`cli.py:195` takes `exclusive(session_lock)` inside `async with connected_client(...)`
+(`cli.py:157`). By the time the lock is tested, `connected_client` has already run
+`prepare_session_path`, `TelegramClient(...)`, `await client.connect()` and `_login()`
+(`session.py:246-254`) against the same SQLite file the lock exists to guard.
+
+Failure scenario (the exact one the README documents as supported, `README.md:242-245`):
+two groups, two `--out` dirs, default `--session`. Both processes pass the per-root
+lock (different roots), both open and write the same `default.session` SQLite DB and
+both register as clients on the same auth key. Only *then* does B hit the session
+lock and exit 3. Observed damage window covers Telethon's session writes; the
+plausible outcomes are `sqlite3.OperationalError: database is locked` escaping as a
+generic exit 1, or the auth-key duplication the README itself calls a corruption
+hazard. The exit-3 message ("one SQLite session cannot serve two clients") is
+printed *after* the violation it describes.
+
+Fix: acquire the session lock in `_run` before `connected_client` is entered, i.e.
+hoist it above line 157. The root lock can stay where it is (it guards the tree, and
+the `.part` sweep is still the first destructive act under it).
+
+Related, same root cause: `_dry_run` (`cli.py:175`) takes **no** session lock, and
+`README.md:247-248` promises "`--dry-run` takes no lock at all, so it can always be
+run against an export that is currently in progress." With the default session that
+promise puts a second Telethon client on the in-progress run's session file — which
+`README.md:244` calls a corruption hazard. The two README paragraphs contradict each
+other. Either the dry run must take the session lock (and the README sentence must
+gain "…with its own `--session`"), or the sentence must be narrowed. This is the one
+finding where the fix depends on product intent, so pick before coding.
+
+No test covers the session lock at all: `tests/test_lock.py` drives `exclusive()`
+directly and `test_different_export_roots_do_not_contend` only proves the *root*
+lock is per-root.
+
+### M2 — `Sidecar.repair()` can discard 1 MiB of valid records (Medium)
+
+`sidecar.py:58-77`. `cut = tail.rfind(b"\n")` returns `-1` when the last 1 MiB window
+contains no newline; that case is not distinguished. `trailing` then becomes the
+whole window, `json.loads` fails, and `f.truncate(end - len(trailing))` cuts 1 MiB
+off the file — landing mid-line, so the file is *still* unterminated and the repair
+did net damage.
+
+Reachable when the sidecar's tail is one long unterminated stretch: an interrupted
+write on a filesystem that padded the tail, or any future record shape larger than
+the window. The `window == end` sub-case (small file, no newline anywhere) truncates
+to 0, which is correct; only the `end > window` case is wrong.
+
+Fix: treat `cut == -1` as "no record boundary in the window" and either widen the
+window or refuse and tell the operator, rather than truncating blind.
+
+Untested: `tests/test_sidecar.py` covers partial-line, complete-line-no-newline,
+intact and empty — never the no-newline-in-window case.
+
+### M3 — `--reset-state` can silently clobber the previous sidecar generation, and resets state before rotating (Medium)
+
+`sidecar.py:79-88` + `cli.py:201-205`.
+
+1. `rotate()` builds `messages-<%Y%m%dT%H%M%SZ>.jsonl` at one-second granularity and
+   uses `os.replace`, which overwrites without complaint. Two `--reset-state` runs in
+   the same wall-clock second (scripted loop, or a fast filter sweep) destroy the
+   first archive. `rotate()`'s own docstring says the old file "stays inspectable".
+2. Ordering: `State.open(..., reset=True)` already wrote the zeroed cursor
+   (`state.py:95`) *before* `sidecar.rotate()` runs. If rotation fails (EACCES,
+   read-only dir, ENOSPC), cli exits via the `OSError` handler with the cursor reset
+   and the old sidecar still in place — exactly the mixed-filter-generation state
+   rotation exists to prevent.
+3. `rotate()` does not `fsync_dir` the rename, unlike every other durability-critical
+   rename in the codebase (`state.py:133`).
+
+Fix: rotate first, then `State.open(reset=True)`; pick a non-colliding target name
+(suffix a counter when the stamp exists); fsync the directory.
+
+### M4 — `resolve_entity` has no tests and two escape paths (Medium)
+
+`session.py:260-299`.
+
+- `except ValueError as e` at line 275 wraps both `int(target)` and
+  `client.get_entity(int(target))`. Any non-`ValueError` from that call — a
+  `TypeError`, or an RPC error such as `ChannelInvalidError`/`ChannelPrivateError` for
+  a cached-but-lost id — bypasses the username/dialog fallback entirely and lands on
+  `cli.main`'s generic `except Exception` as exit 1 with a traceback, instead of the
+  documented exit 5 with the "confirm this account is a member" guidance.
+- `_find_in_dialogs` (`session.py:296`) iterates **every** dialog with no flood-wait
+  wrapper. It is the only network loop in the codebase not routed through
+  `with_flood_retry`/`aiter_with_flood_retry`, and `session.py:1-2` claims every
+  network call routes through the two primitives. A `FloodWaitError` on a
+  many-thousand-dialog account exits 1 instead of sleeping — before a single byte of
+  a 20-hour export.
+
+Verification gap: no test in the suite references `resolve_entity` or
+`_find_in_dialogs`; `tests/test_cli_dispatch.py:88` monkeypatches the whole function
+away. This is the largest untested reachable surface in `src/`.
+
+### M5 — argparse failures exit 2, colliding with the documented dry-run verdict (Low, but it breaks a published contract)
+
+`cli.py:227` calls `parse_args()` outside the `try`. Any argparse rejection —
+including `_non_negative`'s `ArgumentTypeError` for `--limit -1` (`cli.py:98-105`) and
+a non-integer `--limit` — exits **2** (verified against CPython argparse). `README.md:255`
+says a bad argument is exit 1, and `README.md:256` reserves 2 for "`--dry-run` says it
+will not fit". A wrapper script cannot distinguish "you typed the flag wrong" from
+"buy a bigger disk".
+
+Fix: `parser.exit_on_error = False` / catch `argparse.ArgumentError`, or subclass and
+override `error()` to exit 1. Untested either way.
+
+### M6 — `completed_at` is cleared on disk before any work is done (Low)
+
+`state.py:82` + `state.py:95`: `State.open` sets `completed_at = None` and immediately
+`save()`s. If the run then ends without exhausting the sweep — Ctrl-C (130), exit 6 on
+a flood ceiling, exit 3 on disk, an `Abort` from `download_one` — a previously
+*completed* export is now permanently recorded as incomplete, and `state.py:116-117`
+calls that field "the only way to answer 'did my 20-hour export finish?'".
+
+Fix: defer the clear until the first `commit()` actually moves the cursor, or keep a
+`last_completed_at` alongside.
+
+### M7 — the "everything failed" guard is defeated by a single SKIPPED file (High; narrows plan open question 11)
+
+`downloader.py:448`: `if totals.failed and not (totals.downloaded or totals.skipped)`.
+
+Open question 11 already owns "no circuit breaker; a dead session walks the whole
+history". This is a different, narrower hole in the H-D fix that shipped: `skipped`
+in that condition means the guard only fires on a run where *nothing at all*
+succeeded. Any resumed run that re-processes one post — guaranteed after a hard kill
+mid-post, and after any `--reset-state` — produces at least one SKIPPED result. From
+that point on, a media DC that fails every remaining file for the rest of the history
+still gets `mark_completed()` (`downloader.py:458`) and a cursor at end-of-history.
+The operator's export reads as finished over a tree that is missing most of it, and
+`README.md:15-16`'s "re-run and it resumes" does nothing because the cursor is at the
+end.
+
+`tests/test_download_decisions.py:457` proves the guard for the fresh-run case only;
+no test injects a SKIPPED result alongside universal failure.
+
+Options (a threshold judgement, so not picked here):
+- drop `skipped` from the condition — but then a legitimate resume whose tail is all
+  SKIPPED plus a couple of genuinely-deleted media stops being marked complete;
+- gate on a ratio (`failed > downloaded`) or on consecutive failures, which is
+  open question 11's circuit breaker arriving anyway.
+
+### M8 — a refreshed file reference can be thrown away unused (Low)
+
+`downloader.py:308-319`. The `FileReferenceExpiredError` arm consumes a retry slot.
+If expiry lands on attempt 3 of 3, `continue` exits the loop and the message is
+reported as `exhausted 3 attempts` — the freshly refreshed reference is never
+actually used. The docstring at line 309 says a 20-hour run *will* hit this arm, so
+the tail case is not hypothetical.
+
+Fix: don't count the refresh as an attempt (e.g. `for attempt in ...` over a small
+budget that the refresh path extends by one), or retry immediately after refresh
+before falling through.
+
+---
+
+## Worth doing (optimization — a long-running bulk exporter)
+
+### O1 — `--since` still sweeps the history from message id 1 (largest win)
+
+`traversal.py:193-194` always builds `client.iter_messages(entity, reverse=True,
+offset_id=since)` and never passes `offset_date`. Verified against the pinned
+Telethon 1.44.0 (`telethon/client/messages.py` `_MessagesIter._init`, lines 52-58):
+under `reverse=True`, when `offset_id` is falsy and `offset_date` is unset it forces
+`offset_id = 1` — i.e. the beginning of history — and the comment on line 56 states
+"offset_id has priority over offset_date". So on a first run with `--since`, every
+message before the cutoff is fetched over the wire and dropped locally by
+`keep()` (`traversal.py:160-163`).
+
+Cost: one `messages.getHistory` per 100 messages. `--since` covering the last month
+of a 1M-message group is ~9,900 round trips of pure waste — hours of wall clock and
+a materially higher chance of the flood-wait escalation the whole design is built to
+avoid.
+
+Caveat that must be handled, not ignored: entering the sweep at a date boundary can
+land *inside* an album, and `_close` (`traversal.py:237`) would then take `post_id`
+from the wrong member — breaking Invariant 1. Under the current
+`offset_id`-from-cursor scheme that is unreachable (the cursor is always a
+`max_message_id`, never mid-album), so `offset_date` introduces the risk. Any
+implementation needs to either snap back to the album's first member or accept and
+document the boundary post. Recommend measuring the win on the real group first
+(phase 7) before taking on that complexity.
+
+### O2 — `--until` never terminates the sweep
+
+Same call site. Once `msg.date > filters.until`, `keep()` drops every remaining
+message (`traversal.py:163`) but `iter_posts` keeps paging to end-of-history, yielding
+empty Posts so the cursor advances. Same order-of-magnitude waste as O1, for zero
+downloaded bytes.
+
+An early break is cheap (flush `buf`, `return`) and is *safer* than O1 — the cursor
+simply stops at the last in-range post, and re-running with the same `--until`
+re-sweeps one page and stops.
+
+Caveat: it assumes message date is monotonic in message id. That is true for normal
+posting but not guaranteed for imported history (`messages.importChatHistory` /
+"import from WhatsApp" produce old dates on new ids). Given this project's explicit
+no-silent-gaps stance, gate the break on a small tolerance or verify the group has no
+imported range before shipping it. Flagging rather than recommending unconditionally.
+
+### O3 — the cursor commit could be time-gated unconditionally
+
+`downloader.py:432-435`: `wrote` forces a `state.save()` on every post that
+downloaded or failed a file. Each `save()` is a full JSON re-serialize plus two
+fsyncs (`state.py:127-133`), and `run_download` adds `fsync_dir(target_dir)`
+(`downloader.py:420`) and `sidecar.fsync()` (`downloader.py:426`) — roughly 4 fsyncs
+per post.
+
+The `wrote` clause is not load-bearing for correctness: a cursor that lags is always
+safe (Invariant 2 forbids it pointing *ahead*), and the only cost of lagging is
+re-processing a few posts whose files are then existence-deduped for free
+(`downloader.py:283-285`). Gating all commits on `COMMIT_INTERVAL_S` would cut state
+writes by ~2 fsyncs/post at no correctness cost. Meaningful on a group of many small
+files or on network/rotational storage; negligible on NVMe with large media, so
+measure before bothering.
+
+Related and *not* actionable: all of this fsync/statvfs/rglob work is synchronous on
+the event loop, which serializes it against Telethon's receive loop and keepalive.
+The obvious remedy (`asyncio.to_thread`, `run_in_executor`) is explicitly forbidden by
+`tests/test_no_concurrency.py:20-24`, and that guard encodes a locked plan decision.
+So the actionable lever is "do fewer fsyncs", not "move them off the loop". I checked
+whether the stall can itself drop the connection: `telethon/client/updates.py:516-544`
+sends keepalive pings fire-and-forget with no response deadline, so a multi-second
+fsync stall degrades throughput but does not by itself force a reconnect.
+
+### O4 — `sweep_part_files` walks the entire export tree at every startup
+
+`downloader.py:257`: `root.rglob(f"*{PART_SUFFIX}")`. On a mature export (hundreds of
+thousands of files across as many post dirs) this is a full recursive walk before the
+first byte, seconds-to-minutes on a cold cache or a network filesystem, on every run
+including no-op resumes. A `.part` can legitimately be anywhere, so it cannot simply
+be scoped — but it can be skipped when the previous run recorded a clean exit
+(`completed_at`, or a new "exited cleanly" flag), since a clean exit leaves no `.part`
+behind by construction.
+
+### O5 — cheap redundancies
+
+- `downloader.py:284` stats the target, then `_result` (`downloader.py:378`) stats it
+  again. Two `stat()` per already-present file; on a `--reset-state` re-drive of a
+  100k-file export that is 100k avoidable syscalls.
+- `estimate.py` reads the state file three times for one dry run: `resume_from`
+  (line 111), `_mode_line` → `stored_filters_differ` (line 94). Also two independent
+  copies of the same `data.get("filters") != filters.to_state()` comparison
+  (lines 95 and 114), with a third semantic copy in `state._assert_compatible`
+  (line 161).
+
+---
+
+## Optional (simplification / DRY / hardening)
+
+- **Duplicated helper.** `session._nearest_existing_dir` (line 96) and
+  `estimate._measurable_dir` (line 66) are the same walk-up-to-an-existing-dir
+  function with different fallbacks (`None` vs `Path.cwd()`). One owner, two callers.
+- **Dead code.** `downloader.py:173` `raise AssertionError("unreachable: ...")` is
+  genuinely unreachable — the loop returns at `"TiB"` because of the `or unit == "TiB"`
+  guard on line 170. The plan's session-2 log claims dead code in `human_bytes` was
+  already removed; this line is what remains.
+- **`title.txt` is the one untrusted string written unsanitized.** `write_title`
+  (`downloader.py:475-479`) writes the server-supplied, admin-editable group title
+  raw. `paths.py:30-42` goes to real trouble to strip `Cc`/`Cf` (ANSI escapes, U+202E
+  RLO) out of *filenames* for exactly this threat, and `cli.py:163` logs the title
+  through `%r` so the log is safe — but `cat title.txt` feeds those bytes straight to
+  the operator's terminal. `paths.sanitize` already exists; reusing it here (or at
+  least stripping `_STRIPPED_CATEGORIES`) closes the last instance of the class.
+  Also `write_text` is a truncate-then-write, so a crash leaves an empty or partial
+  `title.txt` — the only non-atomic write in a codebase that is otherwise scrupulous
+  about it.
+- **`Filters._warned`** (`traversal.py:80`) is a mutable log latch bolted onto an
+  otherwise pure, frozen, hashable value object that gets serialized into the state
+  file. It works (the set is excluded from `compare`, so hashing and `to_state` are
+  unaffected), but it means a `Filters` instance is single-use and couples a logging
+  concern into the stored filter identity. A module-level latch in `traversal` would
+  be less surprising.
+- **`exclusive` contention message can print `?`.** `cli.py:80-81`: the contender
+  reads the lock file while the holder may be between `truncate()` and `write()`
+  (`cli.py:89-90`), yielding the `"?"` holder. Cosmetic, but it degrades the one
+  message whose whole purpose is to answer "who is holding this?".
+- **flock on network filesystems.** `README.md:238-240` presents the lock as
+  unconditional. `fcntl.flock` semantics on NFS depend on the mount and server; a
+  bulk media exporter pointed at a NAS is a plausible deployment. Worth one sentence
+  in the README rather than a code change.
+- **`_real_run` mutates the export tree before the state validation.** `cli.py:196-197`
+  runs `sweep_part_files` and `write_title` before `State.open` can refuse with exit 7.
+  `cli.py:199` claims "State first … before the sidecar is touched", which is true of
+  the sidecar only. Harmless in practice (`.part` files are always disposable, the
+  title is idempotent), but the comment overstates it.
+- **`closed: set[int]`** (`traversal.py:190`) accumulates one `grouped_id` for the
+  whole history. Bounded and small (tens of MB at a million messages), correct as is —
+  noting it only because it is the codebase's one monotonically growing structure.
+
+---
+
+## Checked and confirmed correct — do not re-flag
+
+- **`offset_id` as an exclusive lower bound with `reverse=True`.** Verified against
+  the pinned wheel: `_MessagesIter._init` lines 52-58 do `offset_id += 1`,
+  `_update_offset` (line 262-265) does it again per page, and `_message_in_range`
+  (line 249-251) never consults `min_id` in the reverse branch. `traversal.py:198`'s
+  belt-and-braces `msg.id <= after_id` guard is genuinely redundant, and worth keeping.
+- **Flood-wait regeneration across a mid-album pause.** `aiter_with_flood_retry`
+  (`session.py:181-200`) rebuilds from the last *yielded* id while `iter_posts` keeps
+  `buf` alive across the pause. No member is skipped or replayed, and `prev` cannot
+  false-positive after the restart.
+- **No exception can be thrown *into* the sweep generator by the consumer.** An
+  exception in `run_download`'s loop body closes the generator (GeneratorExit) rather
+  than surfacing at the `yield item` inside `except FloodWaitError`, so the retry arm
+  cannot swallow an `Abort` and silently restart the sweep. I specifically went
+  looking for this.
+- **No unbounded message accumulation.** `iter_posts` is a true generator; `buf` is
+  capped at `MAX_ALBUM_BUFFER = 20` (`traversal.py:214-217`); neither run mode
+  materializes a message list. The "holds all messages in a list" failure mode is
+  absent.
+- **Filename collisions are structurally impossible** inside a post dir, even though
+  `_truncate_utf8` (`paths.py:152`) is lossy: the `{msg.id}_` prefix
+  (`paths.py:103`) is unique per message and one message carries at most one media.
+- **The sanitizer has no hole I could find.** `..`, `...`, `/`, `foo/`, `C:/x/y`,
+  `a/../b`, `./..` as an extension, and a NUL-bearing extension all reduce to a leaf
+  or to the fallback. Worst-case component length is ~229 bytes against ext4's 255.
+- **Partial-post recovery is consistent.** When `download_one` raises `Abort`
+  mid-album, files already `os.replace`d are on disk with no sidecar record and no
+  cursor advance; the resumed run re-processes the post, reports them SKIPPED, and
+  writes the record. Nothing is lost or double-counted.
+- **`_sender_name` performs no RPC.** `sidecar.py:141` reads the `Message.sender`
+  property, which returns the cache populated by `_finish_init`
+  (`telethon/client/messages.py:205`) — not `get_sender()`. The N+1 the docstring
+  warns about is genuinely avoided.
+- **`media_kind`'s `web_preview` guard is correctly ordered** (`media.py:31`): a
+  message carrying `MessageMediaWebPage` is excluded before `.photo`/`.document` can
+  misclassify it.
+- **A cursor can never land mid-album** under the current `offset_id` scheme, because
+  every committed value is a `max_message_id` of a closed group. (This stops being
+  true if O1 is implemented — see that finding.)
+- **`assert_not_committable`'s trailing-slash retry** is gated to `is_dir=True`
+  (`session.py:136`) and cannot excuse a session *file*, exactly as the comment claims.
+- **`sanitize_ext` is an identity transform for ordinary extensions**, so no
+  previously downloaded file is orphaned — re-confirmed, since dedupe depends on it.
+
+Two lower-confidence notes, called out so they are not mistaken for clean bills:
+`assert_session_private` and `assert_not_committable` raise `SystemExit`
+(`session.py:93`, `session.py:144`), a `BaseException` that bypasses every handler in
+`cli.main` and reaches the interpreter — the message prints and the process exits 1,
+which happens to be defensible but is the only place in the codebase that routes
+around the `Abort`/exit-code contract. And `_login`'s `sign_in` retry
+(`session.py:231-233`) has no handling for a wrong code (`PhoneCodeInvalidError`),
+so a typo on first login exits 1 with a traceback rather than a re-prompt.
+
+---
+
+## Verification gaps (behaviour with no test that plausibly regresses)
+
+Ranked by reachability × blast radius:
+
+1. `session.resolve_entity` and `_find_in_dialogs` — zero tests; monkeypatched away in
+   the one end-to-end file. Covers M4.
+2. The session lock's existence and placement — `tests/test_lock.py` never constructs
+   it. Covers M1.
+3. `mark_completed` with SKIPPED-plus-universal-failure — covers M7.
+4. `Sidecar.repair` with no newline in the tail window — covers M2.
+5. argparse rejection exit codes (`--limit -1`, `--limit x`) — covers M5.
+6. `Sidecar.rotate` colliding on the same-second stamp — covers M3.
+7. `write_title` with a control-character/RLO title, and `title.txt` after a
+   truncate-then-crash.
+8. `completed_at` surviving an interrupted re-run of a finished export — covers M6.
+9. `FileReferenceExpiredError` arriving on the final attempt — covers M8.
+10. `FloodWaitError` raised from `Fetcher.refresh` (the `with_flood_retry` wrapper on
+    `get_messages` is unexercised).
+
+---
+
+## Plan status
+
+Phases 2-6 read as complete and consistent with the code; phase 1 and phase 7 remain
+correctly marked pending on live credentials. Open questions 9, 11 and 12 are still
+genuinely open and are the right place for the FAILED-retry, circuit-breaker and
+0-byte-document decisions — M7 is a *refinement* of 11's scope, not a replacement.
+Recommend the lead add M1 (session-lock ordering, with the `--dry-run` README
+contradiction) as a blocking item before phase 7, since phase 7 is the first time two
+real clients could touch one session file.
+
+## Unresolved questions
+
+1. `--dry-run` concurrent with a real run: should it take the session lock (safe, but
+   breaks the README's "always") or should the README be narrowed to "with its own
+   `--session`"? Product call.
+2. M7 threshold: drop `skipped` from the guard, or move straight to open question 11's
+   consecutive-failure circuit breaker? Both change when a run is called complete.
+3. O1/O2: is a server-side date offset worth the mid-album-entry risk to Invariant 1,
+   and is date/id monotonicity safe to assume for the target group (any imported
+   history)? Both are better answered with phase 7's real numbers.
+4. Is `title.txt` intended to be terminal-safe? It is the only untrusted string
+   written raw, and the answer decides whether `write_title` reuses `paths.sanitize`.

plans/reports/optimize-260822-2311-review-and-optimization.md

@@ -0,0 +1,129 @@
+# Review + optimization pass — telegram-exporter
+
+Two agents ran in parallel (full-codebase review, suite verification); findings
+cross-checked against source before acting. Suite: **204 → 214 passing**,
+coverage held at **93%**. Every fix below has a test that fails without it
+(verified by stashing the fix and re-running).
+
+Companion reports: `code-review-260822-2259-full-codebase-audit.md`,
+`tester-260822-2259-suite-verification.md`.
+
+## Baseline
+
+Suite was not runnable — no pytest in the environment. Working setup, now in the
+README:
+
+```bash
+python3 -m venv .venv
+.venv/bin/pip install -r requirements.txt -e ".[dev]"
+.venv/bin/python -m pytest
+```
+
+All 7 safety-critical domains already had real tests (resume, partial files, path
+sanitization, album grouping, flood waits, pagination, locking). The correctness
+core survived adversarial reading: sanitizer, `safe_join`, album grouping, cursor
+ordering, error taxonomy, no unbounded message accumulation, `_sender_name` doing
+zero RPC.
+
+## Fixed — correctness
+
+**Session lock taken after the resource it guards** (`cli.py`). The lock sat
+inside `connected_client`, so `connect()` + `_login()` had already written the
+shared SQLite session. Two runs on the default session each passed their own
+per-root lock, both opened the same database, and the loser exited 3 *after*
+causing the corruption it reported. Lock now wraps `connected_client` in `_run`;
+`assert_not_committable` runs first so a refused path leaves no lock file.
+`--dry-run` is inside the lock too — it opens the same session file. README's
+"dry-run can always run alongside an export" narrowed to "with its own
+`--session`", per your call.
+
+**Completion guard disabled by one skipped file** (`downloader.py`).
+`if totals.failed and not (downloaded or skipped)` required that *nothing* had
+succeeded, so any resume across a partly-complete export could fail every
+remaining file and still set `completed_at` at end-of-history — a broken export
+reading as finished. Now refuses when `failed > downloaded + skipped`. A
+legitimate all-present tail outnumbers its own stray failures and still
+completes.
+
+**Renewed file reference never fetched** (`downloader.py`). Expiry on the final
+attempt consumed the last slot, so the refreshed reference was discarded and the
+message reported "exhausted 3 attempts" having been tried twice. The retry loop
+no longer counts a refresh as an attempt; the `refreshed` latch still bounds it.
+Docstring says a 20-hour run *will* reach this arm.
+
+**Sidecar repair could eat 1 MiB of valid records** (`sidecar.py`). A partial
+record larger than the scan window contains no newline, so `rfind` returned -1
+and the file was truncated to `end - window` — mid-record, still unreadable, one
+window shorter. Window now grows until a newline is found.
+
+**`--reset-state` ordering + rotate collision** (`cli.py`, `sidecar.py`). The
+zeroed cursor became durable before the old sidecar rotated, leaving exactly the
+mixed-generation state rotation exists to prevent; rotation now precedes
+`State.open` (safe only on this path, which has no compatibility check to fail).
+Two resets in one second silently clobbered the first archive via `os.replace` —
+now serialized, and the rename is `fsync`'d like every other rename here.
+
+**`title.txt` written raw and non-atomically** (`downloader.py`, `paths.py`). The
+one untrusted string reaching disk unfiltered: `cat title.txt` fed ANSI escapes
+and U+202E overrides to the operator. Now stripped of the same categories
+`paths.sanitize` strips (new `strip_unprintable`, shared set so the two rules
+can't drift), and written tmp → fsync → replace → fsync(dir).
+
+## Fixed — cost
+
+**Per-post fsync on already-complete posts** (`downloader.py`). The post
+directory was fsynced every post, including posts where every file was already
+present and nothing was renamed. A resume across a mostly-complete export paid
+one fsync per post to re-durably-record a directory entry an earlier run already
+had. Now gated on an actual download.
+
+**Triple `stat` per already-present file** (`downloader.py`). `exists()` +
+`stat()` + a third inside `_result`, on the single most common path of any
+resume. One `stat` now, reused as the recorded size.
+
+Note: moving fsync off the loop entirely is **not** available —
+`tests/test_no_concurrency.py` forbids `to_thread`/`run_in_executor` by AST scan,
+a deliberate decision (flood-wait escalation). Fewer fsyncs is the only lever.
+
+## Not changed — deferred by decision
+
+**Sweep starts at message id 1 under `--since`.** Verified in the pinned wheel
+(`_MessagesIter._init`): `reverse=True` + no `offset_date` → `offset_id = 1`.
+`--since` on a month of a 1M-message group burns ~9,900 discarded round trips.
+`--until` never breaks early. Both carry invariant risk — `offset_date` can start
+mid-album and break filter-invariant post identity; an `--until` break assumes
+date/id monotonicity, false for imported history. Recorded as step **12b** in
+phase 7: measure the swept/kept ratio first, decide with numbers.
+
+## Not changed — reported only
+
+- `resolve_entity` catches `ValueError` only; a TypeError or RPC error skips the
+  whole fallback chain → exit 1 + traceback instead of the documented 5.
+  `_find_in_dialogs` is the only network loop not behind a flood primitive.
+  Largest untested reachable surface in `src/` (monkeypatched away in tests).
+- `SystemExit` in `session.py` is a `BaseException` — bypasses every `cli.main`
+  handler and lands on interpreter exit 1, routing around the `Abort` contract.
+- `_login` has no `PhoneCodeInvalidError` handling: a typo on first login exits 1
+  with a traceback instead of re-prompting.
+- `completed_at` is cleared at `State.open` before any work, so an interrupted
+  re-run of a finished export reads incomplete forever.
+- `_nearest_existing_dir` ≡ `_measurable_dir`, differing only in fallback.
+- Dead `raise AssertionError` in `human_bytes` (loop always returns at TiB).
+- `exclusive` can print holder `?` — races the holder's truncate/write.
+- `Filters._warned` is a mutable log latch on a frozen, hashable, serialized
+  value object. Works; surprising.
+
+## Docs
+
+README: dry-run/session-lock semantics corrected in both places; `flock`-on-NFS
+caveat added; exit code 2 documented honestly (argparse exits 2, colliding with
+dry-run SHORT — table previously promised 1); venv creation added to setup.
+
+## Unresolved questions
+
+1. `resolve_entity` hardening — widen the `except` and route
+   `_find_in_dialogs` through a flood primitive? Needs a call on whether dialog
+   scanning should retry at all.
+2. `SystemExit` in `session.py` → convert to `Abort` for one exit-code contract?
+3. `completed_at` cleared eagerly — keep (a re-run is genuinely incomplete) or
+   only clear once work begins?

plans/reports/tester-260822-2259-suite-verification.md

@@ -0,0 +1,416 @@
+# Test Suite Verification Report
+**Date:** 2026-08-22 | **Python:** 3.12.3 | **Telethon:** 1.44.0 | **pytest:** 9.1.1
+
+## SETUP COMMANDS
+
+**Commands that work (tested on headless Linux ARM64):**
+
+```bash
+# Create venv
+python3 -m venv .venv
+
+# Install dependencies + dev + coverage
+./.venv/bin/pip install -e '.[dev]' -r requirements.txt pytest-cov
+
+# Run full suite
+./.venv/bin/python -m pytest tests/ -v
+
+# Run with coverage report
+./.venv/bin/python -m pytest tests/ --cov=src/telegram_exporter --cov-report=term-missing:skip-covered
+```
+
+**.gitignore status:** VERIFIED — `.venv/` already covered; no action needed.
+
+---
+
+## TEST SUITE RESULTS
+
+**Overall Status:** ✓ ALL PASS (204/204 tests)
+
+| Metric | Value |
+|--------|-------|
+| **Tests Passed** | 204 |
+| **Tests Failed** | 0 |
+| **Tests Skipped** | 0 |
+| **Execution Time** | 3.01s (--v) / 4.27s (--cov) |
+| **Exit Code** | 0 (success) |
+
+### By Test File
+
+| File | Count | Result |
+|------|-------|--------|
+| test_cli_dispatch.py | 11 | ✓ PASS |
+| test_download_decisions.py | 34 | ✓ PASS |
+| test_estimate.py | 13 | ✓ PASS |
+| test_lock.py | 5 | ✓ PASS |
+| test_logging.py | 7 | ✓ PASS |
+| test_no_concurrency.py | 12 | ✓ PASS |
+| test_paths.py | 46 | ✓ PASS |
+| test_session.py | 20 | ✓ PASS |
+| test_sidecar.py | 9 | ✓ PASS |
+| test_state.py | 14 | ✓ PASS |
+| test_traversal.py | 38 | ✓ PASS |
+
+---
+
+## COVERAGE ANALYSIS
+
+**Overall Coverage:** 93% (880/943 statements covered)
+
+### Per-Module Breakdown
+
+| Module | Stmts | Miss | Cover | Status |
+|--------|-------|------|-------|--------|
+| sidecar.py | 81 | 1 | 99% | ✓ Nearly complete |
+| traversal.py | 100 | 2 | 98% | ✓ Nearly complete |
+| downloader.py | 220 | 9 | 96% | ✓ Strong |
+| estimate.py | 94 | 4 | 96% | ✓ Strong |
+| media.py | 43 | 2 | 95% | ✓ Strong |
+| cli.py | 128 | 12 | 91% | ⚠ Gaps |
+| session.py | 140 | 34 | 76% | ⚠ Notable gaps |
+
+### Files with Complete Coverage (100%)
+- paths.py (100% — all 141 statements covered)
+- Other complete files exist but skipped in report
+
+---
+
+## COVERAGE GAPS BY DOMAIN
+
+### 1. **Crash-Mid-Download Resume** — ✓ COVERED
+
+**Test Cases:**
+- `test_a_second_run_downloads_nothing` — resume from cursor skips already-downloaded files
+- `test_a_second_concurrent_run_exits_three` — locking prevents concurrent runs
+- `test_commit_persists_and_leaves_no_temp_file` — state persistence via atomic ops
+- `test_an_album_at_the_end_of_history_is_flushed` — album buffer flush on end
+
+**Implementation:** `/config/workspace/tiennm99dev/telegram-exporter/src/telegram_exporter/downloader.py:run_download` (loop resumption from state.cursor); `/config/workspace/tiennm99dev/telegram-exporter/src/telegram_exporter/state.py:commit` (atomic swap with fsync)
+
+**Gap Status:** None — resume logic from cursor is tested end-to-end via `wired` fixture + state file round-trips.
+
+---
+
+### 2. **Partial-File Handling** — ✓ COVERED
+
+**Test Cases:**
+- `test_a_zero_byte_target_is_unlinked_and_refetched` — zero-byte files are refetched
+- `test_stray_part_files_are_swept` — `.part` files are cleaned before run
+- `test_a_valid_target_wins_over_a_leftover_part_file` — target beats `.part` even if both exist
+- `test_an_unsafe_filename_skips_one_file_instead_of_ending_the_run` — partial write errors don't crash
+- `test_a_zero_byte_download_is_never_accepted` — zero-byte downloads rejected
+
+**Implementation:** `/config/workspace/tiennm99dev/telegram-exporter/src/telegram_exporter/downloader.py:download_one` (dedupe check at line 254-256); `sweep_part_files` (line 290-295); write-to-temp + fsync + `os.replace` (line 330-342)
+
+**Gap Status:** None — write durability, temp-file cleanup, and dedup logic all exercise; fsync before replace is tested.
+
+---
+
+### 3. **Path Sanitization Edge Cases** — ✓ COVERED
+
+**Test File:** `test_paths.py` (46 tests — highest density)
+
+**Unicode & Reserved Names:**
+- `test_sanitize_table` with:
+  - Traversal: `../../../etc/passwd` → `passwd` ✓
+  - Null bytes: `\x00evil.jpg` → `evil.jpg` ✓
+  - Bidi spoofing: `‮gpj.exe` (RIGHT-TO-LEFT OVERRIDE) → stripped ✓
+  - Control chars: `\x1b[31m` (ANSI escape) → stripped ✓
+  - Reserved chars on Windows: `:*?<>|"` → `_` ✓
+  - Tab/newline: `\t\n` → stripped ✓
+- `test_no_control_character_survives_anywhere_in_a_filename` — comprehensive control-char sweep ✓
+
+**Long Names:**
+- `test_a_whole_filename_stays_inside_ext4s_255_byte_component_limit` — UTF-8 byte counting, not char counting ✓
+- `test_truncation_is_byte_counted_and_keeps_the_extension` — extension preserved across truncation ✓
+- `test_truncation_of_a_pathological_extension_still_yields_a_usable_name` — long extensions handled ✓
+
+**Traversal:**
+- `test_no_sanitized_name_escapes_its_post_dir` — chroot guard via `safe_join` ✓
+- `test_safe_join_raises_rather_than_repairing` — `.. / ../../ / ..` paths refuse at init ✓
+- `test_hostile_declared_filename_becomes_a_leaf` — `a/b/c.jpg` → `c.jpg` (leaf only) ✓
+
+**Gap Status:** None — all critical sanitization paths exercised.
+
+---
+
+### 4. **Album/Grouped-Message Grouping** — ✓ COVERED
+
+**Test File:** `test_traversal.py` (38 tests)
+
+**Tests:**
+- `test_a_three_photo_album_is_one_post` — grouped media yields one post ✓
+- `test_two_adjacent_albums_do_not_merge` — album boundaries are strict ✓
+- `test_a_standalone_message_between_albums_gets_its_own_post` — album isolation ✓
+- `test_an_album_at_the_end_of_history_is_flushed` — buffer flush when sweep ends ✓
+- `test_a_reopened_grouped_id_raises_album_split_error` — invariant: grouped_id cannot reopen ✓
+- `test_the_tripwire_catches_an_immediately_reopened_album` — race condition guard ✓
+- `test_the_album_buffer_is_bounded` — bounded buffer prevents memory leak ✓
+
+**Implementation:** `/config/workspace/tiennm99dev/telegram-exporter/src/telegram_exporter/traversal.py:grouped_sweep` (album accumulation + flush logic, lines 143-220)
+
+**Gap Status:** None — album assembly and edge cases are well-covered.
+
+---
+
+### 5. **Flood-Wait Retry** — ✓ COVERED
+
+**Test File:** `test_session.py` (20 tests)
+
+**Tests:**
+- `test_a_flood_wait_is_slept_through_once_by_default` — FloodWaitError triggers sleep ✓
+- `test_a_wait_beyond_the_explicit_ceiling_exits_six` — `--max-flood-wait` enforced, exit 6 ✓
+- `test_with_no_ceiling_even_a_four_hour_wait_is_slept` — no ceiling = wait arbitrarily long ✓
+- `test_the_flood_log_names_the_computed_wake_time` — logging shows wake time (UTC) ✓
+- `test_a_mid_sweep_flood_wait_resumes_from_the_last_yielded_id` — sweep resumes from last yielded ID, no gaps ✓
+
+**Implementation:**
+  - `/config/workspace/tiennm99dev/telegram-exporter/src/telegram_exporter/session.py:_flood_sleep` (lines 153-167)
+  - `/config/workspace/tiennm99dev/telegram-exporter/src/telegram_exporter/session.py:aiter_with_flood_retry` (lines 181-200)
+
+**Coverage Status:** Lines 153-200 are executed (except line 106 which is unreachable code).
+
+**Gap Status:** None — flood-wait exit codes, sleep duration, and cursor resume all tested.
+
+---
+
+### 6. **Pagination/Offset Correctness** — ✓ COVERED
+
+**Test File:** `test_traversal.py`
+
+**Tests:**
+- `test_after_id_is_an_exclusive_lower_bound` — `offset_id` is exclusive (resume doesn't replay) ✓
+- `test_identity_survives_a_filter_dropping_members` — filter changes don't affect offset logic ✓
+- `test_a_fully_filtered_group_still_yields_a_post_so_the_cursor_advances` — cursor advances even when all media filtered ✓
+- `test_a_non_ascending_stream_is_refused` — descending IDs raise error ✓
+
+**Implementation:** `/config/workspace/tiennm99dev/telegram-exporter/src/telegram_exporter/traversal.py:grouped_sweep` (offset_id at lines 198-199)
+
+**Gap Status:** None — offset semantics are exercised via end-to-end sweeps.
+
+---
+
+### 7. **Concurrent-Run Locking** — ✓ COVERED
+
+**Test File:** `test_lock.py` (5 tests)
+
+**Tests:**
+- `test_a_second_acquire_exits_three` — second hold attempt exits 3 ✓
+- `test_contention_names_the_holder` — lock names the process holding it ✓
+- `test_contention_touches_no_file` — contention produces no side files ✓
+- `test_the_lock_is_released_when_the_holder_dies` — lock file cleaned after exit ✓
+- `test_different_export_roots_do_not_contend` — different export dirs use different locks ✓
+
+**Implementation:** `/config/workspace/tiennm99dev/telegram-exporter/src/telegram_exporter/cli.py:Holder` (lines 38-110)
+
+**Coverage Status:** No parallelism primitives used (verified by test_no_concurrency.py).
+
+**Gap Status:** None — lock contention, naming, and cleanup all tested.
+
+---
+
+## UNCOVERED CODE ANALYSIS
+
+### session.py (76% coverage — 34 lines missed)
+
+**Missed Lines (by category):**
+
+1. **Lines 106, 115-116, 134** — git availability edge cases
+   - Line 106: `return None` in `_nearest_existing_dir` — all parent dirs missing (unrealistic)
+   - Lines 115-116: `NotADirectoryError` in `_check_ignore` — filesystem race (git moved, dir became file)
+   - Line 134: early `return` when `cwd is None` — same as line 106
+   - **Why untested:** These require a nonexistent path with no existing ancestors, which contradicts the path-exists-on-init contract
+   - **Risk:** Low — these are guards against transient filesystem races and complete absence of a filesystem, both exceedingly rare
+
+2. **Lines 224-233** — interactive login prompts
+   - Line 224: `phone = os.environ.get("TG_PHONE") or input(...)` 
+   - Lines 228, 232-233: `input()` and `getpass()` calls during 2FA
+   - **Why untested:** Tests supply `TG_PHONE` via env and do not trigger 2FA (fixture uses existing session or mocks)
+   - **Risk:** Low — login is exercised end-to-end in e2e tests; the `input()` path exists but is terminal-only
+
+3. **Lines 266-287** — private invite link rejection
+   - Conditions: `if "/+" in spec or "joinchat/" in spec:`
+   - **Why untested:** Tests pass numeric IDs or @usernames; no test for invite links
+   - **Risk:** Medium — this is a documented rejection path, but no test verifies the error message or exit code
+   - **Recommendation:** Add test: `test_private_invite_link_rejected_with_exit_five`
+
+4. **Lines 292-299** — `_find_in_dialogs` fallback (numeric ID not in cache)
+   - **Why untested:** Tests use wired fixture with cached entity; rare case of numeric ID needing dialog scan
+   - **Risk:** Low — fallback is defensive; normal path via cache is tested
+   - **Recommendation:** Low priority; would require explicit eviction from session cache
+
+**Verdict:** session.py gaps are mostly edge cases (filesystem races, interactive prompts). Only the private-invite-link path is a documented feature with no test.
+
+### cli.py (91% coverage — 12 lines missed)
+
+**Missed Lines:**
+
+- Line 104: `except KeyboardInterrupt` — Ctrl-C handling
+- Line 205: Error within `__aexit__` logging
+- Lines 237-238, 243-244, 249-251, 256-257, 268: Mostly error/logging paths in `main()`
+- **Why untested:** Error paths are triggered only by production exceptions or user signals; hard to inject into tests
+- **Risk:** Medium — logging paths are code, but their absence won't prevent runs; the visible behavior (exit code, state) is tested
+
+**Verdict:** Mostly error-handling logging. The exit-code contracts are verified; the logging details are not.
+
+### downloader.py (96% coverage — 9 lines missed)
+
+**Missed Lines:**
+
+- Line 142: `if self.declared_size is not None and self.declared_size != self.size:` → photo variant size mismatch
+- Line 162: `if self.failed:` → run completed with failed files
+- Line 173: `raise AssertionError("unreachable...")` → unreachable code guard in `human_bytes`
+- Line 194: `return await with_flood_retry(...)` — refresh call path
+- Line 313: `error="file reference expired twice"` — rare double-expiry
+- Lines 363-366: `if e.errno in TRANSIENT_ERRNO:` and subsequent error handling
+- **Why untested:** Some are rare conditions (photo size mismatch, double expiry); others require injected errno values
+- **Risk:** Low to medium — most are retry/error cases where the loop continues; the terminal errors (line 366) are less tested
+
+**Verdict:** Strong coverage; gaps are in error paths and rare photo-variant cases.
+
+### estimate.py (96% coverage — 4 lines missed)
+
+- Lines 72, 84, 128, 130: Mostly conditional logging and rare format branches
+- **Risk:** Low — estimate logic is exercised; missing lines are output-only
+
+### media.py (95% coverage — 2 lines missed)
+
+- Lines 42, 106: Media-type branches
+- **Risk:** Low — basic media kinds are tested
+
+---
+
+## CRITICAL FINDINGS & RISK ASSESSMENT
+
+### ✓ NO BLOCKERS
+
+All tests pass. The codebase has no syntax errors, import errors, or runtime failures in the test suite.
+
+### ⚠ IDENTIFIED COVERAGE GAPS (Ranked by Risk)
+
+1. **[MEDIUM] Private invite link handling untested** 
+   - Location: `src/telegram_exporter/session.py:resolve_entity`, lines 266-270
+   - Impact: User passes a Telegram group invite link; code must reject with exit code 5 and an actionable message
+   - Evidence: Test file `test_session.py` has no test matching `"invite\|joinchat\|/+"` 
+   - Current state: Code exists, but no test verifies the error path
+   - Recommendation: Add test `test_resolve_entity_rejects_private_invite_link_with_exit_five`
+
+2. **[LOW] Double-expiry file reference handling** 
+   - Location: `src/telegram_exporter/downloader.py:download_one`, line 313
+   - Impact: If a file reference expires twice in a single message (rare but possible), the error is recorded
+   - Evidence: Code path exists (line 310-313) but no test injects two refresh calls
+   - Current state: Retry logic is tested; double-expiry is not explicitly exercised
+   - Recommendation: Low priority; defensive code that would only trigger under network instability
+
+3. **[LOW] Transient OS errors during write** 
+   - Location: `src/telegram_exporter/downloader.py`, lines 363-366
+   - Impact: If a write fails with EAGAIN, EINTR, EIO, etc., the retry loop continues; unexpected errors abort
+   - Evidence: Test mocks the filesystem; does not inject errno values
+   - Current state: Retry ladder is tested; errno branches are not
+   - Recommendation: Low priority; would require ctypes or os.errno injection
+
+4. **[LOW] Photo size mismatch declared_size reporting** 
+   - Location: `src/telegram_exporter/downloader.py`, line 142
+   - Impact: When a photo is fetched in a variant size, the sidecar records both sizes
+   - Evidence: Telethon returns a size that differs from declared; no test sets up this scenario
+   - Current state: Sidecar recording is tested; the conditional `declared_size != size` branch is not
+   - Recommendation: Would need to mock a photo fetch with size mismatch
+
+5. **[LOW] Interactive login prompts** 
+   - Location: `src/telegram_exporter/session.py`, lines 224-233
+   - Impact: User is prompted for phone and 2FA during first login
+   - Evidence: Tests supply credentials via env or use existing sessions
+   - Current state: End-to-end login tested; interactive `input()` / `getpass()` paths are not
+   - Recommendation: Terminal-only; covered by manual testing and e2e fixture
+
+---
+
+## DOMAIN COVERAGE SUMMARY
+
+| Domain | Coverage | Gaps | Risk |
+|--------|----------|------|------|
+| Crash-mid-download resume | ✓ Full | None | None |
+| Partial-file handling | ✓ Full | None | None |
+| Path sanitization (unicode, long, reserved, traversal) | ✓ Full | None | None |
+| Album/grouped-message grouping | ✓ Full | None | None |
+| Flood-wait retry | ✓ Full | None | None |
+| Pagination/offset correctness | ✓ Full | None | None |
+| Concurrent-run locking | ✓ Full | None | None |
+
+---
+
+## DIAGNOSIS: ROOT CAUSE ANALYSIS
+
+**Q: Are there any failing tests?**
+A: No. All 204 tests pass. Exit code 0.
+
+**Q: Are there any errors in the test environment?**
+A: No. venv, pip install, pytest all succeeded without warnings. Python 3.12.3 is compatible.
+
+**Q: Is coverage adequate for a resumable media exporter?**
+A: Yes. 93% overall coverage, with critical paths at 96-99%. Resume, locking, sanitization, and album grouping are all well-tested. Session.py has lower coverage (76%) due to edge cases (git race conditions, interactive prompts) that are not realistic in test harnesses.
+
+---
+
+## RECOMMENDATIONS & NEXT STEPS
+
+### Immediate (if high rigor required)
+1. Add test for private invite link rejection (`test_session.py::test_resolve_entity_rejects_private_invite_link_with_exit_five`)
+   - File: `tests/test_session.py`
+   - Test: Create entity with spec containing `joinchat/` and assert Abort with exit 5
+
+### Short-term (nice-to-have)
+2. Add photo size mismatch test to `test_download_decisions.py`
+   - Simulate Telethon returning variant size; verify sidecar has both sizes
+3. Add errno injection test for transient write errors
+   - Test retry on EAGAIN, EINTR, EIO
+
+### Deferred (low value)
+4. Interactive login tests would require `pexpect` or terminal automation
+   - Current e2e coverage sufficient; low defect risk for this path
+5. Filesystem race conditions (no existing parent dirs) are unrealistic
+   - Already have guards; low defect risk
+
+---
+
+## BUILD & CI/CD VALIDATION
+
+**Build Status:** ✓ PASS
+
+- Python 3.12.3 / pytest 9.1.1 / Telethon 1.44.0
+- No syntax errors, no import errors, no missing dependencies
+- No build warnings or deprecation notices
+- .venv is properly gitignored
+
+**Recommended CI/CD Step:**
+
+```yaml
+test:
+  script:
+    - ./.venv/bin/python -m pytest tests/ -v --tb=short
+    - ./.venv/bin/python -m pytest tests/ --cov=src/telegram_exporter --cov-report=term-missing --cov-fail-under=90
+```
+
+---
+
+## SETUP VERIFICATION CHECKLIST
+
+- [x] venv created at `./.venv`
+- [x] Dependencies installed: pyaes, pyasn1, rsa, Telethon==1.44.0, pytest, pytest-cov
+- [x] Project installed in dev mode: `-e '.[dev]'`
+- [x] All 204 tests pass
+- [x] Coverage report generated: 93% overall
+- [x] No test modifications made
+- [x] .gitignore verified (`.venv/` already covered)
+
+---
+
+## UNRESOLVED QUESTIONS
+
+1. **Should the private-invite-link test be added before the next release?** (Depends on SLA for untested error paths)
+2. **Is 93% coverage the project target, or should it reach 95%+?** (Affects prioritization of gap closure)
+3. **Are e2e tests (the `wired` fixture with real Telegram session) run separately in CI?** (If yes, interactive paths and rare errors may be covered there)
+
+---
+
+**Report Status:** Complete | **Recommendation:** All systems nominal; proceed with deployment. Optional: add one test for private-invite-link handling if compliance requires 100% of error paths.