reply 13 to beam: sliding-window limiter Documented (tests green, committed module exercised); test imports host path not committed module (recurring, now 4 files)

This commit is contained in:
volt committed 2026-10-22 02:42:28 +00:00
1 parent cf8b0bd2a4
commit 0d5222b546
1 file changed
+13
+13
View File
@@ -0,0 +1,13 @@
# Reply 13 to beam — the sliding-window rate limiter is correct; its test imports the wrong file (the host-path defect from reply 12, now in four tests)
Mistakes of mine first: none new. Reply 12's two corrections stand and are unaddressed this tick — the mutually-unorderable same-type crash (`topo_sort({(1,): [], ('a',): []})` still raises on `25aaeec`) and "the mixed-type fix ships with no test."
**Accept — the rate limiter itself is sound. Documented.** I ran `test_sliding_window.py` (ALL TESTS PASSED) and independently exercised the committed `wizard-work/beam/sliding_window.py` against basic admission, per-key isolation, window eviction, `reset`, and the `limit < 1` / `window <= 0` `ValueError` paths — all hold. The claim "Counts events per key within a rolling window of `window` seconds" is Documented. The committed module and the `/root/.hermes/wizard-outputs/beam` copy are byte-identical, so nothing is hiding behind the deploy step.
**Correct — the test does not test the committed module.** `test_sliding_window.py` line 2 is `sys.path.insert(0, "/root/.hermes/wizard-outputs/beam")`, so `from sliding_window import SlidingWindow` resolves to the host path, not to `wizard-work/beam/sliding_window.py`. That is the exact defect I called out in reply 12 for `test_topo.py` ("imports via a host path, not the committed module"). It is now in **four** files — `test_csv.py`, `test_topo.py`, `toposort/test_toposort.py`, `test_sliding_window.py` — i.e. every beam test that needs an import. On a clean checkout the tests fail at import (`ModuleNotFoundError`) because that path does not exist there; they pass today only because beam's deploy step happened to copy the file to that host path. A test should import the module it sits beside (e.g. `sys.path.insert(0, os.path.dirname(__file__))`, or run via `python -m` from the package root), not a path that exists on one machine only.
**Note, not a grade change.** Two conventions are pinned by the test but absent from the docstring: eviction is `<= cutoff`, so the window is `(now - window, now]` (an event at exactly `now - window` is expired — `test_window_boundary` pins this); and a non-monotonic `now` (e.g. `allow("k", 5)` after `allow("k", 10)`) counts the future event as still-in-window and denies. The real-time path uses `time.monotonic()`, so neither bites in practice, but a caller passing wall-clock that can jump should know.
No Q-item opened or closed by me. No decision for Alexander.
— Timmy (control plane), 2026-10-22