The interval menu cleared each host's history, and the reporter builds every report from that history, so the samples recorded since the last delivered report were never sent.
The menu now resets each host with HostState.reset(), which moves the history's entries to clearedHistory, empties the history and leaves the host pending, as the menu did before. buildReport reads clearedHistory before history, with the same filter as before: non-paused samples newer than the host's mark. The marks, and when they advance, are unchanged, so a sample a delivered report had is not sent again, and samples whose report fails are sent at the next interval, as they would be with no interval change.
What the diff does not show:
The first round after a change is discarded, so a host stays pending for at least a round. A report sent then carries only checks from before the change, and gives the host the status of the last of them, by the rule pushSample uses (statusFromError, now shared).
clearedHistory is never emptied. Entries a delivered report has sent stay in it and are skipped by the mark, as they are in history. It keeps the last 100 entries the menu has cleared (CONFIG.maxHistoryPoints), so repeated changes with no backend cannot grow it.
Sending a report at the moment of the change was not used: a report still pending then, a failed delivery or a paused page would still lose the samples.
Reporter is now exported, for the unit tests.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/netwatch/issues/124.
The interval menu cleared each host's history, and the reporter builds every report from that history, so the samples recorded since the last delivered report were never sent.
The menu now resets each host with `HostState.reset()`, which moves the history's entries to `clearedHistory`, empties the history and leaves the host `pending`, as the menu did before. `buildReport` reads `clearedHistory` before `history`, with the same filter as before: non-paused samples newer than the host's mark. The marks, and when they advance, are unchanged, so a sample a delivered report had is not sent again, and samples whose report fails are sent at the next interval, as they would be with no interval change.
What the diff does not show:
- The first round after a change is discarded, so a host stays `pending` for at least a round. A report sent then carries only checks from before the change, and gives the host the status of the last of them, by the rule `pushSample` uses (`statusFromError`, now shared).
- `clearedHistory` is never emptied. Entries a delivered report has sent stay in it and are skipped by the mark, as they are in `history`. It keeps the last 100 entries the menu has cleared (`CONFIG.maxHistoryPoints`), so repeated changes with no backend cannot grow it.
- Sending a report at the moment of the change was not used: a report still pending then, a failed delivery or a paused page would still lose the samples.
- `Reporter` is now exported, for the unit tests.
Model: opus-5-5
test/unit/main.test.js:386: the new test calls clearHistory() itself (line 398) and never goes through the interval menu. If the menu at src/main.js:1519 went back to host.history = [], the defect #124 reports, no test would fail. The definition of done asks for a test that changes the interval. Nothing tests the 100-entry limit on clearedHistory either. Acceptable: the interval menu's per-host reset is a function that both the menu and the test call, so the test fails when an interval change loses unreported samples; and a test covers the limit.
src/main.js:1521 with src/main.js:471: after an interval change, a host whose unreported samples all come from before the change is sent with status pending next to those samples. No report sent that value before this change, and it contradicts the samples it comes with. The definition of done says the samples go out "as if the interval had not changed", and without a change that host's status would be the one its last check gave it. Acceptable: such a host is reported with the status of its last check, not pending.
src/main.js:268: the comment says clearedHistory holds the entries the history had before the menu last cleared it. But clearHistory adds to it each time, so after several changes it holds entries from all of them, up to 100. Acceptable: the comment says it holds the last 100 entries the interval menu has cleared from the history.
Judgement call: I read the PR body's disclosed pending status as a defect under the definition of done, not as a design decision. Nothing on record from the owner covers it.
Model: opus-5-5
FAIL: needs-rework
1. `test/unit/main.test.js:386`: the new test calls `clearHistory()` itself (line 398) and never goes through the interval menu. If the menu at `src/main.js:1519` went back to `host.history = []`, the defect https://git.eeqj.de/sneak/netwatch/issues/124 reports, no test would fail. The definition of done asks for a test that changes the interval. Nothing tests the 100-entry limit on `clearedHistory` either. Acceptable: the interval menu's per-host reset is a function that both the menu and the test call, so the test fails when an interval change loses unreported samples; and a test covers the limit.
2. `src/main.js:1521` with `src/main.js:471`: after an interval change, a host whose unreported samples all come from before the change is sent with status `pending` next to those samples. No report sent that value before this change, and it contradicts the samples it comes with. The definition of done says the samples go out "as if the interval had not changed", and without a change that host's status would be the one its last check gave it. Acceptable: such a host is reported with the status of its last check, not `pending`.
3. `src/main.js:268`: the comment says `clearedHistory` holds the entries the history had before the menu last cleared it. But `clearHistory` adds to it each time, so after several changes it holds entries from all of them, up to 100. Acceptable: the comment says it holds the last 100 entries the interval menu has cleared from the history.
Judgement call: I read the PR body's disclosed `pending` status as a defect under the definition of done, not as a design decision. Nothing on record from the owner covers it.
Model: opus-5-5
HostState.reset() is now the menu's whole per-host reset, and the menu and both tests call it; a second test resets twice after 60 checks each and finds only the last 100 in the report.
Fixed: a host still pending after a reset is sent with the status its last check gives it; the test now sends a report before the next check and expects error from the timeout.
Fixed: the comment now says clearedHistory holds the last 100 entries the interval menu has cleared from the history.
Model: opus-5-5
1. `HostState.reset()` is now the menu's whole per-host reset, and the menu and both tests call it; a second test resets twice after 60 checks each and finds only the last 100 in the report.
2. Fixed: a host still `pending` after a reset is sent with the status its last check gives it; the test now sends a report before the next check and expects `error` from the timeout.
3. Fixed: the comment now says `clearedHistory` holds the last 100 entries the interval menu has cleared from the history.
Model: opus-5-5
The interval menu cleared every host's history, and the reporter builds
each report from that history, so the samples recorded since the last
delivered report were never sent. The menu now resets each host with
HostState.reset, which moves the history's entries to clearedHistory,
keeping the last 100 it has cleared, and buildReport reads both. A host
still pending after the reset is sent with the status of its last check.
The per-host marks are unchanged, so a sample a delivered report had is
not sent again, and the page's history still starts over. Reporter is
exported for the new unit tests.
Model: opus-5-5
PASS: #128 fixes #124 and all three findings of the earlier review, and can be merged into next.
Model: opus-5-5
PASS: https://git.eeqj.de/sneak/netwatch/pulls/128 fixes https://git.eeqj.de/sneak/netwatch/issues/124 and all three findings of the earlier review, and can be merged into `next`.
Model: opus-5-5
clawbot
merged commit 03bc282871 into next2026-10-14 12:23:13 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #124.
The interval menu cleared each host's history, and the reporter builds every report from that history, so the samples recorded since the last delivered report were never sent.
The menu now resets each host with
HostState.reset(), which moves the history's entries toclearedHistory, empties the history and leaves the hostpending, as the menu did before.buildReportreadsclearedHistorybeforehistory, with the same filter as before: non-paused samples newer than the host's mark. The marks, and when they advance, are unchanged, so a sample a delivered report had is not sent again, and samples whose report fails are sent at the next interval, as they would be with no interval change.What the diff does not show:
pendingfor at least a round. A report sent then carries only checks from before the change, and gives the host the status of the last of them, by the rulepushSampleuses (statusFromError, now shared).clearedHistoryis never emptied. Entries a delivered report has sent stay in it and are skipped by the mark, as they are inhistory. It keeps the last 100 entries the menu has cleared (CONFIG.maxHistoryPoints), so repeated changes with no backend cannot grow it.Reporteris now exported, for the unit tests.Model: opus-5-5
FAIL: needs-rework
test/unit/main.test.js:386: the new test callsclearHistory()itself (line 398) and never goes through the interval menu. If the menu atsrc/main.js:1519went back tohost.history = [], the defect #124 reports, no test would fail. The definition of done asks for a test that changes the interval. Nothing tests the 100-entry limit onclearedHistoryeither. Acceptable: the interval menu's per-host reset is a function that both the menu and the test call, so the test fails when an interval change loses unreported samples; and a test covers the limit.src/main.js:1521withsrc/main.js:471: after an interval change, a host whose unreported samples all come from before the change is sent with statuspendingnext to those samples. No report sent that value before this change, and it contradicts the samples it comes with. The definition of done says the samples go out "as if the interval had not changed", and without a change that host's status would be the one its last check gave it. Acceptable: such a host is reported with the status of its last check, notpending.src/main.js:268: the comment saysclearedHistoryholds the entries the history had before the menu last cleared it. ButclearHistoryadds to it each time, so after several changes it holds entries from all of them, up to 100. Acceptable: the comment says it holds the last 100 entries the interval menu has cleared from the history.Judgement call: I read the PR body's disclosed
pendingstatus as a defect under the definition of done, not as a design decision. Nothing on record from the owner covers it.Model: opus-5-5
c6f325cff5to85ac1e08ceHostState.reset()is now the menu's whole per-host reset, and the menu and both tests call it; a second test resets twice after 60 checks each and finds only the last 100 in the report.pendingafter a reset is sent with the status its last check gives it; the test now sends a report before the next check and expectserrorfrom the timeout.clearedHistoryholds the last 100 entries the interval menu has cleared from the history.Model: opus-5-5
85ac1e08ceto4d33e484ffPASS: #128 fixes #124 and all three findings of the earlier review, and can be merged into
next.Model: opus-5-5