Fix eta calculation

[8nt0n]

Fixes #85

Description

This PR resolves multiple accuracy, stability, and edge-case issues in the ETA calculation logic (moon_engine.py and app.js):

• ETA Clamp Fix: Updated raw_eta >= 7200 to return None (instead of the 7200 sentinel), allowing app.js to properly render --.
• Direct Remaining-Bytes Calculation: Replaced early-run average-file-size skew by calculating remaining bytes using known file_bytes for in-flight files and applying average estimates only to pending files without known sizes.
• Jitter Reduction: Decoupled the live UI speed display (3-second window) from the ETA calculation speed (10-second smoothed window).
• Terminal State Filter: Excluded non-active states (ok, fail, aborted, stopped) from _tracked so failed or aborted partial downloads no longer contribute ghost remaining bytes to the ETA.
• Code Cleanup & Tests: Cleaned up dead code (t_start), updated window comments, and added unit tests in test_snapshot_speed.py covering clamp behavior and terminal-state handling.

Type of change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that would change existing behavior)
• Documentation update
• Refactor / code cleanup
• Other:

Checklist

• I have tested my changes locally
• If this affects shared logic (extraction, download engine), I also applied the equivalent change to moon_cli.py
• I have kept the single-file architecture (no package split)
• I have not added new dependencies without justification in the PR description

[LeyckerS]

@8nt0n — a lot of this is right, and the parts that are right are the parts that were hard. Decoupling the ETA rate from the live-speed window, returning None instead of the 7200 sentinel, teaching fmtEta to render --, and excluding terminal states so a failed partial download stops contributing ghost bytes — all correct, and the terminal-state filter is something I had not thought of.

Two things block, and I verified both by running them rather than by reading.

---

1. self._tracked is iterated outside the lock

Both new loops run after the with self._lock: block has closed:

for record in self._tracked.values():
    if record.file_bytes > 0:

_track() writes to that dict from the asyncio worker thread (moon_engine.py:519), and snapshot() is called from the caller's thread about twelve times a second. Iterating a dict while another thread inserts into it raises. Reproduced on your branch — one thread calling _track() in a loop, one calling snapshot(0):

RESULT: RuntimeError: dictionary changed size during iteration

The same probe on main ran to completion with no error, because main never iterates the live dict.

The fix is already in this file, sixty lines above yours. _files_payload() does exactly the right thing:

def _files_payload(self) -> list[dict]:
    with self._lock:
        tracked = list(self._tracked.items())
    # ...everything else works on the copy

Take a copy inside the existing lock block — snapshot() already holds it a few lines earlier to read the counters — and iterate the copy.

---

2. Files that have not been picked up yet contribute nothing

dl_size_left sums only records in _tracked, and a link enters _tracked when a worker picks it up (moon_engine.py:156 and :235), not when it is queued. So everything still waiting counts as zero work remaining.

Measured on your branch — 100 files of 100 MB, ten picked up, none finished:

speed reported : 6.4 MB/s
eta reported   : 111.1 s
files          : 0/100

The real figure is 10,000 MB at 6.4 MB/s, roughly 1,560 seconds. The ETA is short by about fourteen times, and it will climb steadily through the run rather than falling — the estimate going backwards is the failure mode users notice most.

main avoided this by multiplying files_remaining = dl_tot - dl_done, which counts queued files even though its per-file average was wrong. Your avg_file is computed correctly but is only applied to tracked records whose size is unknown; the untracked ones are missing entirely. The formula needs its second term:

dl_size_left = Σ (file_bytes − done_bytes)   over tracked, non-terminal records
             + (dl_total − dl_done − tracked_non_terminal_count) × avg_file

That is the shape I described on #85 and I should have been clearer that the second line was load-bearing rather than a footnote.

---

Two smaller things, neither blocking:

The new test uses now = time.time() while the engine filters on time.monotonic() — the test in the same file directly above it uses monotonic. It passes today only because wall-clock values are enormous compared to monotonic ones, so every sample lands inside the window by accident. Match the existing test and it will keep testing what it says.

Two unrelated blank lines are removed, after self._proxy_status = "empty_file" and after self._last_proxy_check = now. Harmless, but they are not part of #85.

---

None of this is a rewrite: the first is a two-line change, the second is one added term. The structure you built is the right one, and the terminal-state filter is a genuine improvement I want to keep. Push when ready and I will re-run both probes against it.

[LeyckerS]

@8nt0n — as promised, I re-ran both probes from the last review against 7c25563.

Probe 1 (thread safety) — one thread hammering _track() while another calls snapshot() in a loop: no error. The copy taken inside the lock (tracked_records = list(self._tracked.values()), moon_engine.py:691 on your branch) holds; the old branch raised RuntimeError: dictionary changed size during iteration under the same load.

Probe 2 (queued files) — 100 files of 100 MiB, ten picked up, none finished, ~6.4 MiB/s: eta reported 1562.5 s against a hand-computed ~1560 s. The old branch said 111.1 s. The queued_files term is exactly right.

So the engine code is now correct and both blockers are gone. What keeps this from merging is the tests, not the code:

1. test_snapshot_eta_ignores_terminal_states is vacuous. It sets _dl_total = 1 and _dl_done = 1, so files_remaining is 0 and the ETA branch exits before your terminal-state filter ever runs — eta is None for any status. I ran the test verbatim against unmodified main (which has no filter at all): it passes there too, and its assertion also holds with the non-terminal status "downloading". As it stands, reverting your filter would keep CI green. Fix: _dl_total = 2, _dl_done = 1 (or similar) so the branch executes, and assert the ETA excludes the terminal record's remaining bytes rather than in (0, None).

2. Nothing exercises raw_eta >= 7200 → None — the literal headline of #85. test_snapshot_eta_clamp_and_none is a genuine regression test for the not-meaningful branch (I checked: it fails on main, eta_s=0.0 vs None), but a fresh engine has mbs_eta = 0, so the clamp line is never reached. Add one case: plausible speed in _bytes_acc, enormous remaining bytes, assert eta_s is None.

3. The body bullet "Cleaned up dead code (t_start)" describes nothing in the final diff. The t_start your later commit removed was introduced by this PR's own earlier commit; main's t_start (moon_engine.py:226) is live — it feeds queue_wait_s and extract_s. The changelog quotes PR bodies, so please drop or reword that bullet.

4. Small, none blocking on its own: the blank line after the proxy-status branch in snapshot() is still removed, and a new stray blank line appeared inside _get_proxy_status — it looks like the restore landed in the wrong function. And in fmtEta the new guard returns "--" while the line just below still returns "—" (em dash) for 0 < s < 0.5 — two different unknown markers — plus the new lines are indented 8/4 spaces in a 2-space file. Worth unifying while you are in there.

Items 1 and 2 block only because the body claims tests "covering clamp behavior" and terminal-state handling — with those two tests made real, this merges. The terminal-state filter itself remains the part of this PR I most want to keep.