Fix the collector bugs a retroactive review found

[baz8080]

Slice 2 of the retroactive review, following #48 and #51: a whole-file review of esb_outages/store.py, poll.py, parse.py and client.py, then three reviews of this PR itself. Each commit holds one fix, and each fix's test was checked to fail without it.

The fixes

1. A run's final status was never logged. The raw run line is written before the details start, so it could only say ok. rebuild restored every cut_short, partial, schema_drift or mid-run auth_error run as ok. In esb-data the raw runs say ok 2,470 times and nothing else.
   • Store.finish_run now appends an "event": "end" line before writing the run row, carrying only what can't be derived.
   • Start lines say an end line follows ("ends_logged": true). A flagged start with no end line replays as unfinished, or as in_progress if it is the newest run, because the backup commits raw/ mid-run. A failure the start line already logged (auth_error, unreachable) is kept either way.
   • An end line whose start line was lost replays as a run with no list, counting its own fetches.
   • Older runs replay as before.
   • This reverses a gap notes/storms.md had accepted. The owner approved the reversal, and the note records it.
2. compact could wipe an archived month. Late lines now join the archive as a further gzip member, starting on a fresh line, streamed into a staged file, fsynced and renamed into place before the source is removed. An interrupted compact is safe to repeat, because iter_raw reads a line identical to one already read in the same month (keyed by the month in the file name) once.
3. rebuild and compact ignored the poll lock. Both now take it.
4. A cut-off response, or a 404 on the list, killed the whole run. These are now transient or unreachable. The client has its first tests.
5. Malformed input crashed the run and every rebuild after it. That covers a non-object list item or body, and a non-string timestamp; both are now contained. The logs are read as bytes and each line decoded strictly, so a line torn or corrupted mid-character is one malformed line: never a crash, and never a silently different record. Appends start on a fresh line after a torn one.
6. The storm fetch order:
   • never-captured Restored outages first;
   • then captured ones still waiting for a restore time, including a captured fault that has just flipped to Restored;
   • then everything live;
   • an id listed twice is fetched once.
7. An unreadable restore time marked an outage final. Final now needs the parsed time.
8. Observations that lost their run replayed after all of history. They now replay at their own place in time.
9. The October clock-change docstring pointed the wrong way.
10. Counts:
    • A key rejected mid-run now keeps the errors that came before it.
    • rebuild counts listed and fetched outages the way the live run does, which the site's horizon depends on.

Reviews of this PR

• First review, 10 findings, all fixed. They led to the second half of 1, 2, 5, 6 and 10 above, plus streaming in compact and the orphan-before-run tie.

• Second review of those nine commits, 10 findings. It found my original unfinished rule unsafe. That rule keyed on the earliest end line in the log, so a merged host or a wrong-clock boot would relabel healthy runs. It was replaced by the start-line flag. The same review led to the repeated-id and per-month dedupe fixes, the torn-mid-character and torn-archive fixes, and the flipped-fault rank.

• Third review of the next five commits, 9 findings, all fixed except the limits below. Two were real bugs:

  • errors="replace", added to survive a torn fada, turned a line corrupted on the card into a different valid record ("Dún" read back as "D�n"). It's now a strict per-line decode.
  • A start line's auth_error/unreachable was overridden by the unfinished label.

  Also fixed: the dedupe month key, a lost-start run's fetched count (found by the test the review asked for), and redundancy in the tidy-up.

Known limits, not fixed. The in_progress label goes on the newest start line, and a log snapshot holds nothing better:

• a newest run that really died reads in_progress until the next run logs;
• runs tied on the newest second all read in_progress;
• a later-sorting run from a wrong clock or a merged host pushes the real one to unfinished.

Also not taken: the direction of a same-second orphan tie, which the log can't decide, and making compact itself idempotent, since repeats are already read once.

Effect on the data

Rebuilding esb-data with main and with this branch gives identical outage, outage_change and run tables: 4,954 / 7,121 / 2,556 rows, none differing. Nothing the site publishes moves.

Checks

• ruff check: clean.
• unittest discover with ESB_DATA_DIR set: 333 tests OK.

🤖 Generated with Claude Code

https://claude.ai/code/session_01UqWuH6frirpbQDKnF5rEdT