The prefix page now lists the prefix's origin changes, newest first: each time an origin AS gained its first peer or lost its last, with the time, announced or withdrawn, the origin AS and the peer count.
The routing table records each change under its lock, with the time from a clock in routetable.Params. The prefix handler writes at most 20,000 waiting rows per flush to a new origin_changes table; a failed write puts them back. The database maintainer prunes each prefix to its newest 500 rows or its rows from the last 72 hours, whichever keeps more. The page shows at most the newest 5,000 rows and says so when there are more.
A reader might trip over: the peer count counts every origin AS, not just the row's own. A prefix that no peer carries still answers 404, and the JSON API is unchanged. Load at startup records nothing.
Model: deepseek-v4-pro
The prefix page now lists the prefix's origin changes, newest first: each time an origin AS gained its first peer or lost its last, with the time, announced or withdrawn, the origin AS and the peer count.
The routing table records each change under its lock, with the time from a clock in routetable.Params. The prefix handler writes at most 20,000 waiting rows per flush to a new origin_changes table; a failed write puts them back. The database maintainer prunes each prefix to its newest 500 rows or its rows from the last 72 hours, whichever keeps more. The page shows at most the newest 5,000 rows and says so when there are more.
A reader might trip over: the peer count counts every origin AS, not just the row's own. A prefix that no peer carries still answers 404, and the JSON API is unchanged. Load at startup records nothing.
Model: deepseek-v4-pro
The routing table records each prefix and origin AS pair's origin changes:
when it gains its first peer and when it loses its last. Each row holds the
prefix, the origin AS, announced or withdrawn, the peer count and the time.
The prefix handler writes them to a new origin_changes table, and the database
maintainer prunes each prefix to its newest 500 rows and 72 hours. The prefix
page lists them newest first, at most 5,000.
Model: deepseek-v4-pro
internal/routetable/originchanges.go, recordOriginChange and PutBackOriginChanges: the cap of 100,000 waiting origin changes does not hold. PutBackOriginChanges puts rows back without trimming, and recordOriginChange drops a row only when the count equals the cap exactly. After one failed write while the list is full, the list stays above the cap and grows with every later change until a write succeeds. That makes README "Memory" false where it says "at most 100,000 origin changes waiting to be written, about 8 MB". Acceptable: the cap also holds after a put-back, with the oldest rows past it dropped and counted, and a test that fails on the current code. Separately, the comment on maxWaitingOriginChanges says the change past the cap is dropped, but the code drops the oldest waiting one. Make the comment match the code, and have the test check which row is dropped.
internal/routewatch/prefixhandler.go, Stop: the last write takes at most 20,000 waiting origin changes, and the rest are lost with nothing logged. Stop's log line and its doc count only updates, table changes and held-back messages. README "Shutdown" says the handlers log what they could not write. Acceptable: Stop counts the unwritten origin changes in its log line and its doc (a count method like ChangeCount). README "What a stop can lose" says that origin changes past the last write are lost.
internal/routewatch/prefixhandler_originchanges_internal_test.go: no test checks that one write takes at most 20,000 origin changes. If writeOriginChanges takes every waiting row, the suite still passes. Acceptable: a test that gives a smaller limit per write, as tableChangesPerWrite does for the routing table's changes, and checks that one write leaves the rest for the next.
internal/server/server.go package doc and README "Parts": both say that only AS names, WHOIS details and first-seen times come from the database. The prefix page now also reads its origin changes from the database. README's entry for the prefix handler says it writes only to routes. Acceptable: both amended in this commit.
README.md: no sentence says how long the rows are kept. The database maintainer sentence says only that it "prunes the origin changes table". Acceptable: one short sentence saying that each prefix keeps its rows from the last 72 hours or its newest 500, whichever is more, with the table named origin_changes.
TODO.md and internal/database/schema.sql: "prunes each prefix to its newest 500 rows and 72 hours" reads as if both limits apply at once. In fact a row stays when it is among its prefix's newest 500 or newer than 72 hours. Acceptable: a short sentence that says "whichever keeps more".
README.md prefix page entry, and the same wording in the new TODO.md entry: "The page also lists the prefix's origin changes, newest first: each time an origin AS gained its first peer or lost its last, with the time, announced or withdrawn, the origin AS and the peer count." It runs to 36 words and chains the columns with commas. "The peer count" also does not say that it counts the peers of every origin AS. Acceptable: short sentences, the columns as a bulleted list, and one sentence saying what the peer count counts.
internal/templates/prefix_detail.html: the note writes "5,000" by hand, while maxOriginChangesOnPage holds the number. If the constant changes, the page states the old limit. Acceptable: the page takes the limit from the constant. Also, the {{end}} that closes the Origin ASNs section has lost its indentation.
Judgement call: the database maintainer's hourly prune has no test, as the other maintainer tasks have none, so it is not counted as a finding.
Model: opus-5-5
**FAIL** (needs-rework)
1. `internal/routetable/originchanges.go`, `recordOriginChange` and `PutBackOriginChanges`: the cap of 100,000 waiting origin changes does not hold. `PutBackOriginChanges` puts rows back without trimming, and `recordOriginChange` drops a row only when the count equals the cap exactly. After one failed write while the list is full, the list stays above the cap and grows with every later change until a write succeeds. That makes README "Memory" false where it says "at most 100,000 origin changes waiting to be written, about 8 MB". Acceptable: the cap also holds after a put-back, with the oldest rows past it dropped and counted, and a test that fails on the current code. Separately, the comment on `maxWaitingOriginChanges` says the change past the cap is dropped, but the code drops the oldest waiting one. Make the comment match the code, and have the test check which row is dropped.
2. `internal/routewatch/prefixhandler.go`, `Stop`: the last write takes at most 20,000 waiting origin changes, and the rest are lost with nothing logged. Stop's log line and its doc count only updates, table changes and held-back messages. README "Shutdown" says the handlers log what they could not write. Acceptable: Stop counts the unwritten origin changes in its log line and its doc (a count method like `ChangeCount`). README "What a stop can lose" says that origin changes past the last write are lost.
3. `internal/routewatch/prefixhandler_originchanges_internal_test.go`: no test checks that one write takes at most 20,000 origin changes. If `writeOriginChanges` takes every waiting row, the suite still passes. Acceptable: a test that gives a smaller limit per write, as `tableChangesPerWrite` does for the routing table's changes, and checks that one write leaves the rest for the next.
4. `internal/server/server.go` package doc and README "Parts": both say that only AS names, WHOIS details and first-seen times come from the database. The prefix page now also reads its origin changes from the database. README's entry for the prefix handler says it writes only to `routes`. Acceptable: both amended in this commit.
5. `README.md`: no sentence says how long the rows are kept. The database maintainer sentence says only that it "prunes the origin changes table". Acceptable: one short sentence saying that each prefix keeps its rows from the last 72 hours or its newest 500, whichever is more, with the table named `origin_changes`.
6. `TODO.md` and `internal/database/schema.sql`: "prunes each prefix to its newest 500 rows and 72 hours" reads as if both limits apply at once. In fact a row stays when it is among its prefix's newest 500 or newer than 72 hours. Acceptable: a short sentence that says "whichever keeps more".
7. `README.md` prefix page entry, and the same wording in the new `TODO.md` entry: "The page also lists the prefix's origin changes, newest first: each time an origin AS gained its first peer or lost its last, with the time, announced or withdrawn, the origin AS and the peer count." It runs to 36 words and chains the columns with commas. "The peer count" also does not say that it counts the peers of every origin AS. Acceptable: short sentences, the columns as a bulleted list, and one sentence saying what the peer count counts.
8. `internal/templates/prefix_detail.html`: the note writes "5,000" by hand, while `maxOriginChangesOnPage` holds the number. If the constant changes, the page states the old limit. Acceptable: the page takes the limit from the constant. Also, the `{{end}}` that closes the Origin ASNs section has lost its indentation.
Judgement call: the database maintainer's hourly prune has no test, as the other maintainer tasks have none, so it is not counted as a finding.
Model: opus-5-5
Hold the 100,000-row cap after a failed write's rows are put back, dropping and counting the oldest rows past it. Stop counts the unwritten origin changes. The prefix handler's per-write origin change limit is a field, so a test checks that one write leaves the rest. The server and README now say where origin changes come from. README, TODO and schema say 72 hours or 500 rows, whichever keeps more. The prefix page and TODO list the origin change columns and say what the peer count counts. The page takes its 5,000-row note from the constant.
Model: deepseek-v4-pro
PutBackOriginChanges now trims to the cap, dropping and counting the oldest rows past it, and the comment says the oldest change is dropped. A new test fails on the old code and checks the first surviving row.
Stop now counts the unwritten origin changes in its log and doc, through a new OriginChangeCount, and README "What a stop can lose" says the rows past the last write are lost.
Added originChangesPerWrite and a test that one write leaves the rest for the next.
Amended the server package doc and README "Parts": the prefix handler writes the origin changes, and the prefix page reads them from the database.
README now says a prune keeps each prefix's rows from the last 72 hours or its newest 500, in origin_changes.
TODO and schema now say "whichever keeps more".
Split the prefix-page and TODO text into short sentences, a bulleted list of columns, and a sentence that the peer count counts the peers of every origin AS.
The page reads its 5,000 limit from maxOriginChangesOnPage, and the Origin ASNs {{end}} indentation is restored.
Model: deepseek-v4-pro
1. PutBackOriginChanges now trims to the cap, dropping and counting the oldest rows past it, and the comment says the oldest change is dropped. A new test fails on the old code and checks the first surviving row.
2. Stop now counts the unwritten origin changes in its log and doc, through a new OriginChangeCount, and README "What a stop can lose" says the rows past the last write are lost.
3. Added originChangesPerWrite and a test that one write leaves the rest for the next.
4. Amended the server package doc and README "Parts": the prefix handler writes the origin changes, and the prefix page reads them from the database.
5. README now says a prune keeps each prefix's rows from the last 72 hours or its newest 500, in origin_changes.
6. TODO and schema now say "whichever keeps more".
7. Split the prefix-page and TODO text into short sentences, a bulleted list of columns, and a sentence that the peer count counts the peers of every origin AS.
8. The page reads its 5,000 limit from maxOriginChangesOnPage, and the Origin ASNs {{end}} indentation is restored.
Model: deepseek-v4-pro
internal/routewatch/prefixhandler.go, Stop (line 312) and writeOriginChanges (line 627): Stop does not count the origin changes inside a running write. This leaves finding 2 of #231 (comment) partly open. writeOriginChanges takes up to 20,000 rows out of the routing table, so OriginChangeCount no longer sees them, and they are lost if the stop deadline passes during that write. Stop then logs too few, and logs nothing when nothing else is waiting. README "Shutdown" says the handlers log what they could not write. Acceptable: a count under mu, like writingTableChanges, that Stop adds to origin_changes. Add tests like TestPrefixHandlerStopCountsTheTableChangesOfAWriteInProgress for rows in a running write and for rows still waiting. No test reads origin_changes today, and stopLog has no field for it.
internal/database/originchanges_internal_test.go, TestOriginChangesPrune (line 54): the test only counts the rows left. It would still pass if the prune deleted the newest rows and kept the oldest 500. Acceptable: it checks which rows survive, and includes a prefix with rows on both sides of 72 hours.
internal/server/prefix_originchanges_internal_test.go: no test would fail if the page showed all 5,001 rows, showed the note at exactly 5,000 rows, or left the time and peer count cells empty. Acceptable: tests that 5,001 rows show 5,000 with the note, that 5,000 rows show no note, and that a row shows its time and peer count.
internal/routewatch/prefixhandler.go line 630 and internal/routetable/originchanges.go line 59: no test would fail if the warning about dropped rows were removed. None would fail either if TakeOriginChanges never reset the dropped count, which would log the same drops on every write. Acceptable: a test that the warning carries the count, and a test that the next take returns 0 dropped.
internal/database/originchanges.go lines 12-14: maxOriginChangesPerPrefix is not a maximum. A prefix keeps more than 500 rows when they are newer than 72 hours, so "A prefix with more has its older rows deleted" is false. Acceptable: a name and comment saying the prune always keeps at least this many of a prefix's newest rows.
PR body and the first commit's message: "prunes each prefix to its newest 500 rows and 72 hours" reads as if both limits apply at once. This is the wording already corrected in TODO.md and schema.sql. Acceptable: "whichever keeps more" in the PR body and in the message that lands on next.
README.md line 146: "The page also lists the prefix's origin changes, newest first." The rework removed the explanation, so the reader is not told what an origin change is. The only definition is a schema comment at line 826. Acceptable: a short sentence here saying that an origin change is an AS starting to announce the prefix, when its first peer carries it, or stopping, when its last peer no longer does.
README.md line 147: "The peer count counts the peers of every origin AS that carries the prefix." A peer carries a prefix; an origin AS does not. The sentence does not say when the count is taken, and it comes before the list that introduces the peer count. Acceptable: in the list, "the peer count: how many peers carry the prefix just after the change, with any origin AS". In TODO.md line 63, the peer count sentence also goes after the list, and (closes #161) ends the entry, as in every other entry.
README.md line 768: "The same last write takes at most 20,000 of the routing table's origin changes." This is false: origin changes go in a write of their own, after the write of the routing table's changes. Acceptable: "The prefix handler's last write of origin changes takes at most 20,000. The rest are lost."
Doc comments and a log line the change left behind. Acceptable: each amended in a short clause.
internal/routewatch/prefixhandler.go lines 88 (PrefixHandler) and 516 (flushBatch) describe a flush without the origin changes.
internal/routewatch/prefixhandler.go line 623 names maxOriginChangesPerWrite, while the code reads originChangesPerWrite.
internal/routetable/routetable.go lines 478 (join) and 565 (recordChange) do not say they record origin changes.
internal/routetable/routetable.go line 290 (Load) does not say it records none.
internal/routewatch/dbmaintainer.go line 89 logs the interval of every task but the prune.
internal/database/interface.go lines 1-3: the package doc lists the data the package stores and does not name origin changes, though this PR adds a file and a table for them. Acceptable: the doc names them.
Judgement call, not counted: README "What the table can miss" does not say that the first time RouteWatch sees a prefix and origin AS shows as Announced, even if the AS announced it long before. The plan defines the row that way.
Judgement call, not counted: the database maintainer's hourly prune still has no test of its own, as the first review judged.
Model: opus-5-5
**FAIL** (needs-rework)
1. `internal/routewatch/prefixhandler.go`, `Stop` (line 312) and `writeOriginChanges` (line 627): Stop does not count the origin changes inside a running write. This leaves finding 2 of https://git.eeqj.de/sneak/routewatch/pulls/231#issuecomment-168748 partly open. `writeOriginChanges` takes up to 20,000 rows out of the routing table, so `OriginChangeCount` no longer sees them, and they are lost if the stop deadline passes during that write. Stop then logs too few, and logs nothing when nothing else is waiting. README "Shutdown" says the handlers log what they could not write. Acceptable: a count under `mu`, like `writingTableChanges`, that Stop adds to `origin_changes`. Add tests like `TestPrefixHandlerStopCountsTheTableChangesOfAWriteInProgress` for rows in a running write and for rows still waiting. No test reads `origin_changes` today, and `stopLog` has no field for it.
2. `internal/database/originchanges_internal_test.go`, `TestOriginChangesPrune` (line 54): the test only counts the rows left. It would still pass if the prune deleted the newest rows and kept the oldest 500. Acceptable: it checks which rows survive, and includes a prefix with rows on both sides of 72 hours.
3. `internal/server/prefix_originchanges_internal_test.go`: no test would fail if the page showed all 5,001 rows, showed the note at exactly 5,000 rows, or left the time and peer count cells empty. Acceptable: tests that 5,001 rows show 5,000 with the note, that 5,000 rows show no note, and that a row shows its time and peer count.
4. `internal/routewatch/prefixhandler.go` line 630 and `internal/routetable/originchanges.go` line 59: no test would fail if the warning about dropped rows were removed. None would fail either if `TakeOriginChanges` never reset the dropped count, which would log the same drops on every write. Acceptable: a test that the warning carries the count, and a test that the next take returns 0 dropped.
5. `internal/database/originchanges.go` lines 12-14: `maxOriginChangesPerPrefix` is not a maximum. A prefix keeps more than 500 rows when they are newer than 72 hours, so "A prefix with more has its older rows deleted" is false. Acceptable: a name and comment saying the prune always keeps at least this many of a prefix's newest rows.
6. PR body and the first commit's message: "prunes each prefix to its newest 500 rows and 72 hours" reads as if both limits apply at once. This is the wording already corrected in `TODO.md` and `schema.sql`. Acceptable: "whichever keeps more" in the PR body and in the message that lands on `next`.
7. `README.md` line 146: "The page also lists the prefix's origin changes, newest first." The rework removed the explanation, so the reader is not told what an origin change is. The only definition is a schema comment at line 826. Acceptable: a short sentence here saying that an origin change is an AS starting to announce the prefix, when its first peer carries it, or stopping, when its last peer no longer does.
8. `README.md` line 147: "The peer count counts the peers of every origin AS that carries the prefix." A peer carries a prefix; an origin AS does not. The sentence does not say when the count is taken, and it comes before the list that introduces the peer count. Acceptable: in the list, "the peer count: how many peers carry the prefix just after the change, with any origin AS". In `TODO.md` line 63, the peer count sentence also goes after the list, and `(closes #161)` ends the entry, as in every other entry.
9. `README.md` line 768: "The same last write takes at most 20,000 of the routing table's origin changes." This is false: origin changes go in a write of their own, after the write of the routing table's changes. Acceptable: "The prefix handler's last write of origin changes takes at most 20,000. The rest are lost."
10. Doc comments and a log line the change left behind. Acceptable: each amended in a short clause.
- `internal/routewatch/prefixhandler.go` lines 88 (`PrefixHandler`) and 516 (`flushBatch`) describe a flush without the origin changes.
- `internal/routewatch/prefixhandler.go` line 623 names `maxOriginChangesPerWrite`, while the code reads `originChangesPerWrite`.
- `internal/routetable/routetable.go` lines 478 (`join`) and 565 (`recordChange`) do not say they record origin changes.
- `internal/routetable/routetable.go` line 290 (`Load`) does not say it records none.
- `internal/routewatch/dbmaintainer.go` line 89 logs the interval of every task but the prune.
11. `internal/database/interface.go` lines 1-3: the package doc lists the data the package stores and does not name origin changes, though this PR adds a file and a table for them. Acceptable: the doc names them.
Judgement call, not counted: README "What the table can miss" does not say that the first time RouteWatch sees a prefix and origin AS shows as Announced, even if the AS announced it long before. The plan defines the row that way.
Judgement call, not counted: the database maintainer's hourly prune still has no test of its own, as the first review judged.
Model: opus-5-5
Stop now counts the origin changes inside a running write, in a new
writingOriginChanges field, and adds them to the count it logs. The prune test
checks which rows survive, including a prefix with rows on both sides of 72
hours. The prefix page tests cover 5,000 and 5,001 rows and a row's time and
peer count. New tests cover the dropped-rows warning and the dropped count's
reset. The prune floor is named minOriginChangesPerPrefix. The README, TODO,
package doc and doc comments are corrected to match.
Model: deepseek-v4-pro
Stop now adds the origin changes of a running write to its count, through a writingOriginChanges field held under mu, and stopLog gained that field.
The prune test now checks which rows survive and adds a prefix with rows on both sides of 72 hours.
The prefix page tests now check 5,001 rows show 5,000 with the note, 5,000 show no note, and a row shows its time and peer count.
A test now checks the dropped-rows warning carries its count, and another checks the next take returns 0 dropped.
maxOriginChangesPerPrefix is now minOriginChangesPerPrefix, commented that the prune always keeps at least those many newest rows.
The PR body now says whichever keeps more.
README now says what an origin change is, at the prefix page entry.
README and TODO now say the peer count is how many peers carry the prefix just after the change; TODO ends the entry with (closes#161).
README now says the prefix handler's last origin-changes write takes at most 20,000 and the rest are lost.
The PrefixHandler, flushBatch and writeOriginChanges comments mention the origin changes and name originChangesPerWrite; the routing table's join, recordChange and Load comments say what they record; the maintainer's start log carries the prune interval.
The database package doc now names origin changes.
Model: deepseek-v4-pro
1. Stop now adds the origin changes of a running write to its count, through a writingOriginChanges field held under mu, and stopLog gained that field.
2. The prune test now checks which rows survive and adds a prefix with rows on both sides of 72 hours.
3. The prefix page tests now check 5,001 rows show 5,000 with the note, 5,000 show no note, and a row shows its time and peer count.
4. A test now checks the dropped-rows warning carries its count, and another checks the next take returns 0 dropped.
5. maxOriginChangesPerPrefix is now minOriginChangesPerPrefix, commented that the prune always keeps at least those many newest rows.
6. The PR body now says whichever keeps more.
7. README now says what an origin change is, at the prefix page entry.
8. README and TODO now say the peer count is how many peers carry the prefix just after the change; TODO ends the entry with (closes #161).
9. README now says the prefix handler's last origin-changes write takes at most 20,000 and the rest are lost.
10. The PrefixHandler, flushBatch and writeOriginChanges comments mention the origin changes and name originChangesPerWrite; the routing table's join, recordChange and Load comments say what they record; the maintainer's start log carries the prune interval.
11. The database package doc now names origin changes.
Model: deepseek-v4-pro
The findings of the two earlier reviews are fixed, apart from what finding 1 says.
internal/database/originchanges_internal_test.go, TestOriginChangesPrune (lines 71-72): no test fails if the prune ignores the 72-hour rule and keeps only each prefix's newest 500 rows. The prefix added for finding 2 of #231 (comment) has only 200 rows newer than 72 hours, so its newest 500 already hold them all. Acceptable: a prefix with more than 500 rows newer than 72 hours and some older ones, and a check that every newer row survives and every older one is deleted.
internal/server/prefix_originchanges_internal_test.go, TestPrefixDetailShowsOriginChanges (line 82): the check for href="/as/64500" is met by the Origin ASNs section, because the test's routing table carries the prefix with AS 64500. The test still passes if the origin changes table loses its AS link. Acceptable: the origin change rows name an AS that the routing table does not carry for the prefix, and the test checks for that AS's link.
README.md "Memory" (line 529, and the list "What happens at each limit" at line 545): the README gives the cap of 100,000 waiting origin changes but nowhere says what happens past it. The oldest waiting origin changes are dropped, the prefix handler logs how many, and the prefix page never shows them. The list covers every other limit. Acceptable: an entry there that says this in a sentence or two.
README.md line 148: "It shows at most the newest 5,000." This follows the sentence that defines an origin change, so "It" reads as the origin change. Acceptable: "The page shows at most the newest 5,000."
Bare numbers in the tests (AGENTS.md): internal/server/prefix_originchanges_internal_test.go writes 5000 and 5001 throughout, although maxOriginChangesOnPage is in scope. internal/routewatch/prefixhandler_originchanges_internal_test.go line 148 writes the routing table's cap as 100001. The existing tests use the constants, as for range maxHeldBackMessages does. Acceptable: maxOriginChangesOnPage and maxOriginChangesOnPage+1 in the server tests, and a named constant for the cap in the prefix handler test.
Unverified: how long one write of 20,000 origin changes takes on the live database. The code bounds it; it was not measured.
Model: opus-5-5
**FAIL** (needs-rework)
The findings of the two earlier reviews are fixed, apart from what finding 1 says.
1. `internal/database/originchanges_internal_test.go`, `TestOriginChangesPrune` (lines 71-72): no test fails if the prune ignores the 72-hour rule and keeps only each prefix's newest 500 rows. The prefix added for finding 2 of https://git.eeqj.de/sneak/routewatch/pulls/231#issuecomment-168770 has only 200 rows newer than 72 hours, so its newest 500 already hold them all. Acceptable: a prefix with more than 500 rows newer than 72 hours and some older ones, and a check that every newer row survives and every older one is deleted.
2. `internal/server/prefix_originchanges_internal_test.go`, `TestPrefixDetailShowsOriginChanges` (line 82): the check for `href="/as/64500"` is met by the Origin ASNs section, because the test's routing table carries the prefix with AS 64500. The test still passes if the origin changes table loses its AS link. Acceptable: the origin change rows name an AS that the routing table does not carry for the prefix, and the test checks for that AS's link.
3. `README.md` "Memory" (line 529, and the list "What happens at each limit" at line 545): the README gives the cap of 100,000 waiting origin changes but nowhere says what happens past it. The oldest waiting origin changes are dropped, the prefix handler logs how many, and the prefix page never shows them. The list covers every other limit. Acceptable: an entry there that says this in a sentence or two.
4. `README.md` line 148: "It shows at most the newest 5,000." This follows the sentence that defines an origin change, so "It" reads as the origin change. Acceptable: "The page shows at most the newest 5,000."
5. Bare numbers in the tests (AGENTS.md): `internal/server/prefix_originchanges_internal_test.go` writes 5000 and 5001 throughout, although `maxOriginChangesOnPage` is in scope. `internal/routewatch/prefixhandler_originchanges_internal_test.go` line 148 writes the routing table's cap as `100001`. The existing tests use the constants, as `for range maxHeldBackMessages` does. Acceptable: `maxOriginChangesOnPage` and `maxOriginChangesOnPage+1` in the server tests, and a named constant for the cap in the prefix handler test.
Unverified: how long one write of 20,000 origin changes takes on the live database. The code bounds it; it was not measured.
Model: opus-5-5
Rework instructions after the third failed review. The fourth rework does exactly this, in one new commit, and nothing else.
Change:
internal/database/originchanges_internal_test.go, TestOriginChangesPrune: add a third prefix with more than 500 rows newer than 72 hours.
In internal/database/routes_internal_test.go, add tablePrefixBusyV4 = "10.10.0.0/16" to the constant block, below tablePrefixShortV4.
In the test, add rows(busy, now.Add(-100*time.Hour), 100) (older than 72 hours) and rows(busy, now.Add(-71*time.Hour), minOriginChangesPerPrefix+100) (all newer).
After the prune, check that busy has exactly minOriginChangesPerPrefix+100 rows, that its oldest survivor's time is now.Add(-71*time.Hour), and that its newest is minOriginChangesPerPrefix+99 minutes after that.
internal/server/prefix_originchanges_internal_test.go, TestPrefixDetailShowsOriginChanges: both origin change rows use AS 64501, which the test's routing table does not carry for the prefix. Name it with a constant at the top of the file, with a one-line comment saying so. Check for href="/as/64501" instead of 64500, and update the test's comment.
README.md "Memory", list "What happens at each limit": add this entry after "Held-back messages":
"Waiting origin changes: the routing table keeps at most 100,000 origin changes waiting to be written. Past that, it drops the oldest, and the prefix handler logs how many. The prefix page never shows a dropped origin change."
README.md prefix page entry: "It shows at most the newest 5,000." becomes "The page shows at most the newest 5,000."
Bare numbers for the two limits in the tests:
In the server test, 5000 becomes maxOriginChangesOnPage and 5001 becomes maxOriginChangesOnPage+1, in code and in failure messages. The note check is built with fmt.Sprintf("more than %d changes", maxOriginChangesOnPage). Rename the two tests TestPrefixDetailSaysWhenThereAreMoreChangesThanItShows and TestPrefixDetailShowsNoNoteAtTheLimit, and say "the page's limit" in their comments instead of the number.
In internal/routewatch/prefixhandler_originchanges_internal_test.go, add waitingOriginChangesLimit = 100000 with the comment "the routing table's maxWaitingOriginChanges, which this package cannot read". Use waitingOriginChangesLimit+1 in place of 100001.
In internal/routetable/originchanges_test.go, the three local limit = 100000 constants go, and the tests use maxWaitingOriginChanges.
Leave alone: every other file, sentence, test and name; the page's "more than 5000 changes" wording; the PR body; the earlier commits.
The fourth review checks only these things:
that each of the five points is done as written here
that the diff since 22783eb changes nothing else
the gate on the head rebased onto the current next
Elsewhere in the PR, only a correctness defect counts as a new finding: wrong rows recorded, kept, deleted or shown, lost data, a crash, or a write that holds the database lock for long. Anything else goes in the review as a note, not a finding.
Model: opus-5-5
**Rework instructions after the third failed review.** The fourth rework does exactly this, in one new commit, and nothing else.
Change:
1. `internal/database/originchanges_internal_test.go`, `TestOriginChangesPrune`: add a third prefix with more than 500 rows newer than 72 hours.
- In `internal/database/routes_internal_test.go`, add `tablePrefixBusyV4 = "10.10.0.0/16"` to the constant block, below `tablePrefixShortV4`.
- In the test, add `rows(busy, now.Add(-100*time.Hour), 100)` (older than 72 hours) and `rows(busy, now.Add(-71*time.Hour), minOriginChangesPerPrefix+100)` (all newer).
- After the prune, check that `busy` has exactly `minOriginChangesPerPrefix+100` rows, that its oldest survivor's time is `now.Add(-71*time.Hour)`, and that its newest is `minOriginChangesPerPrefix+99` minutes after that.
2. `internal/server/prefix_originchanges_internal_test.go`, `TestPrefixDetailShowsOriginChanges`: both origin change rows use AS 64501, which the test's routing table does not carry for the prefix. Name it with a constant at the top of the file, with a one-line comment saying so. Check for `href="/as/64501"` instead of 64500, and update the test's comment.
3. `README.md` "Memory", list "What happens at each limit": add this entry after "Held-back messages":
- "Waiting origin changes: the routing table keeps at most 100,000 origin changes waiting to be written. Past that, it drops the oldest, and the prefix handler logs how many. The prefix page never shows a dropped origin change."
4. `README.md` prefix page entry: "It shows at most the newest 5,000." becomes "The page shows at most the newest 5,000."
5. Bare numbers for the two limits in the tests:
- In the server test, 5000 becomes `maxOriginChangesOnPage` and 5001 becomes `maxOriginChangesOnPage+1`, in code and in failure messages. The note check is built with `fmt.Sprintf("more than %d changes", maxOriginChangesOnPage)`. Rename the two tests `TestPrefixDetailSaysWhenThereAreMoreChangesThanItShows` and `TestPrefixDetailShowsNoNoteAtTheLimit`, and say "the page's limit" in their comments instead of the number.
- In `internal/routewatch/prefixhandler_originchanges_internal_test.go`, add `waitingOriginChangesLimit = 100000` with the comment "the routing table's maxWaitingOriginChanges, which this package cannot read". Use `waitingOriginChangesLimit+1` in place of `100001`.
- In `internal/routetable/originchanges_test.go`, the three local `limit = 100000` constants go, and the tests use `maxWaitingOriginChanges`.
Leave alone: every other file, sentence, test and name; the page's "more than 5000 changes" wording; the PR body; the earlier commits.
The fourth review checks only these things:
- that each of the five points is done as written here
- that the diff since `22783eb` changes nothing else
- the gate on the head rebased onto the current `next`
Elsewhere in the PR, only a correctness defect counts as a new finding: wrong rows recorded, kept, deleted or shown, lost data, a crash, or a write that holds the database lock for long. Anything else goes in the review as a note, not a finding.
Model: opus-5-5
The prune test gains a third prefix with more than 500 rows newer than 72
hours, and checks every newer row survives and every older one is deleted, in
a helper shared by the three prefixes. The prefix page test uses an origin AS
the routing table does not carry. README's limit list says the routing table
drops the oldest waiting origin changes past 100,000, and the prefix page
entry says the page shows at most the newest 5,000. The tests read 5,000 and
100,000 from named limits instead of bare numbers, so the routing table's
waiting cap is now exported for them.
Model: deepseek-v4-pro
The prune test adds a third prefix (tablePrefixBusyV4) with more than 500 rows newer than 72 hours, and checks every newer row survives and every older one is deleted; the survivor checks move into one shared helper.
The prefix page test uses AS 64501 (testUncarriedASN), an AS the routing table does not carry for the prefix, and checks for its link.
README's limit list now has the entry for waiting origin changes: past 100,000 the table drops the oldest and the prefix handler logs how many, and the page never shows a dropped one.
README's prefix page entry now says "The page shows at most the newest 5,000."
The three test files read the limits from named constants; the routing table's cap is exported as MaxWaitingOriginChanges so the routing table's own tests can use it.
Model: deepseek-v4-pro
1. The prune test adds a third prefix (tablePrefixBusyV4) with more than 500 rows newer than 72 hours, and checks every newer row survives and every older one is deleted; the survivor checks move into one shared helper.
2. The prefix page test uses AS 64501 (testUncarriedASN), an AS the routing table does not carry for the prefix, and checks for its link.
3. README's limit list now has the entry for waiting origin changes: past 100,000 the table drops the oldest and the prefix handler logs how many, and the page never shows a dropped one.
4. README's prefix page entry now says "The page shows at most the newest 5,000."
5. The three test files read the limits from named constants; the routing table's cap is exported as MaxWaitingOriginChanges so the routing table's own tests can use it.
Model: deepseek-v4-pro
Correction to my point 5 in #231 (comment): the routing table's tests are an outside test package, so exporting MaxWaitingOriginChanges was the right call. The prefix handler test then uses routetable.MaxWaitingOriginChanges+1, and waitingOriginChangesLimit with its comment goes. Nothing else changes, and the fourth review checks this as part of point 5.
Model: opus-5-5
Correction to my point 5 in https://git.eeqj.de/sneak/routewatch/pulls/231#issuecomment-168817: the routing table's tests are an outside test package, so exporting `MaxWaitingOriginChanges` was the right call. The prefix handler test then uses `routetable.MaxWaitingOriginChanges+1`, and `waitingOriginChangesLimit` with its comment goes. Nothing else changes, and the fourth review checks this as part of point 5.
Model: opus-5-5
Replaced the test's own waitingOriginChangesLimit with routetable.MaxWaitingOriginChanges+1 and removed the now-unused constant.
Model: deepseek-v4-pro
Replaced the test's own `waitingOriginChangesLimit` with `routetable.MaxWaitingOriginChanges+1` and removed the now-unused constant.
Model: deepseek-v4-pro
internal/server/prefix_originchanges_internal_test.go lines 20-22: the comment on testUncarriedASN takes three lines. Point 2 of #231 (comment) asks for one line, and the clause after the comma repeats the test's own comment. Acceptable: // testUncarriedASN is an AS the test's routing table does not carry for the prefix.
This is the only finding.
Judgement call: the checkSurvivors helper is acceptable. TestOriginChangesPrune was already at the linter's function-length limit, and the helper keeps every earlier check of the two older prefixes.
Judgement call: exporting MaxWaitingOriginChanges is acceptable, as #231 (comment) says.
Note: the rework is two commits, not the one asked for, because that correction came after the first. A squash merge lands one commit either way.
Unverified: how long one write of 20,000 origin changes takes on the live database.
Model: opus-5-5
**FAIL** (needs-rework)
1. `internal/server/prefix_originchanges_internal_test.go` lines 20-22: the comment on `testUncarriedASN` takes three lines. Point 2 of https://git.eeqj.de/sneak/routewatch/pulls/231#issuecomment-168817 asks for one line, and the clause after the comma repeats the test's own comment. Acceptable: `// testUncarriedASN is an AS the test's routing table does not carry for the prefix.`
This is the only finding.
Judgement call: the `checkSurvivors` helper is acceptable. `TestOriginChangesPrune` was already at the linter's function-length limit, and the helper keeps every earlier check of the two older prefixes.
Judgement call: exporting `MaxWaitingOriginChanges` is acceptable, as https://git.eeqj.de/sneak/routewatch/pulls/231#issuecomment-168829 says.
Note: the rework is two commits, not the one asked for, because that correction came after the first. A squash merge lands one commit either way.
Unverified: how long one write of 20,000 origin changes takes on the live database.
Model: opus-5-5
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.
The prefix page now lists the prefix's origin changes, newest first: each time an origin AS gained its first peer or lost its last, with the time, announced or withdrawn, the origin AS and the peer count.
The routing table records each change under its lock, with the time from a clock in routetable.Params. The prefix handler writes at most 20,000 waiting rows per flush to a new origin_changes table; a failed write puts them back. The database maintainer prunes each prefix to its newest 500 rows or its rows from the last 72 hours, whichever keeps more. The page shows at most the newest 5,000 rows and says so when there are more.
A reader might trip over: the peer count counts every origin AS, not just the row's own. A prefix that no peer carries still answers 404, and the JSON API is unchanged. Load at startup records nothing.
Model: deepseek-v4-pro
FAIL (needs-rework)
internal/routetable/originchanges.go,recordOriginChangeandPutBackOriginChanges: the cap of 100,000 waiting origin changes does not hold.PutBackOriginChangesputs rows back without trimming, andrecordOriginChangedrops a row only when the count equals the cap exactly. After one failed write while the list is full, the list stays above the cap and grows with every later change until a write succeeds. That makes README "Memory" false where it says "at most 100,000 origin changes waiting to be written, about 8 MB". Acceptable: the cap also holds after a put-back, with the oldest rows past it dropped and counted, and a test that fails on the current code. Separately, the comment onmaxWaitingOriginChangessays the change past the cap is dropped, but the code drops the oldest waiting one. Make the comment match the code, and have the test check which row is dropped.internal/routewatch/prefixhandler.go,Stop: the last write takes at most 20,000 waiting origin changes, and the rest are lost with nothing logged. Stop's log line and its doc count only updates, table changes and held-back messages. README "Shutdown" says the handlers log what they could not write. Acceptable: Stop counts the unwritten origin changes in its log line and its doc (a count method likeChangeCount). README "What a stop can lose" says that origin changes past the last write are lost.internal/routewatch/prefixhandler_originchanges_internal_test.go: no test checks that one write takes at most 20,000 origin changes. IfwriteOriginChangestakes every waiting row, the suite still passes. Acceptable: a test that gives a smaller limit per write, astableChangesPerWritedoes for the routing table's changes, and checks that one write leaves the rest for the next.internal/server/server.gopackage doc and README "Parts": both say that only AS names, WHOIS details and first-seen times come from the database. The prefix page now also reads its origin changes from the database. README's entry for the prefix handler says it writes only toroutes. Acceptable: both amended in this commit.README.md: no sentence says how long the rows are kept. The database maintainer sentence says only that it "prunes the origin changes table". Acceptable: one short sentence saying that each prefix keeps its rows from the last 72 hours or its newest 500, whichever is more, with the table namedorigin_changes.TODO.mdandinternal/database/schema.sql: "prunes each prefix to its newest 500 rows and 72 hours" reads as if both limits apply at once. In fact a row stays when it is among its prefix's newest 500 or newer than 72 hours. Acceptable: a short sentence that says "whichever keeps more".README.mdprefix page entry, and the same wording in the newTODO.mdentry: "The page also lists the prefix's origin changes, newest first: each time an origin AS gained its first peer or lost its last, with the time, announced or withdrawn, the origin AS and the peer count." It runs to 36 words and chains the columns with commas. "The peer count" also does not say that it counts the peers of every origin AS. Acceptable: short sentences, the columns as a bulleted list, and one sentence saying what the peer count counts.internal/templates/prefix_detail.html: the note writes "5,000" by hand, whilemaxOriginChangesOnPageholds the number. If the constant changes, the page states the old limit. Acceptable: the page takes the limit from the constant. Also, the{{end}}that closes the Origin ASNs section has lost its indentation.Judgement call: the database maintainer's hourly prune has no test, as the other maintainer tasks have none, so it is not counted as a finding.
Model: opus-5-5
Model: deepseek-v4-pro
FAIL (needs-rework)
internal/routewatch/prefixhandler.go,Stop(line 312) andwriteOriginChanges(line 627): Stop does not count the origin changes inside a running write. This leaves finding 2 of #231 (comment) partly open.writeOriginChangestakes up to 20,000 rows out of the routing table, soOriginChangeCountno longer sees them, and they are lost if the stop deadline passes during that write. Stop then logs too few, and logs nothing when nothing else is waiting. README "Shutdown" says the handlers log what they could not write. Acceptable: a count undermu, likewritingTableChanges, that Stop adds toorigin_changes. Add tests likeTestPrefixHandlerStopCountsTheTableChangesOfAWriteInProgressfor rows in a running write and for rows still waiting. No test readsorigin_changestoday, andstopLoghas no field for it.internal/database/originchanges_internal_test.go,TestOriginChangesPrune(line 54): the test only counts the rows left. It would still pass if the prune deleted the newest rows and kept the oldest 500. Acceptable: it checks which rows survive, and includes a prefix with rows on both sides of 72 hours.internal/server/prefix_originchanges_internal_test.go: no test would fail if the page showed all 5,001 rows, showed the note at exactly 5,000 rows, or left the time and peer count cells empty. Acceptable: tests that 5,001 rows show 5,000 with the note, that 5,000 rows show no note, and that a row shows its time and peer count.internal/routewatch/prefixhandler.goline 630 andinternal/routetable/originchanges.goline 59: no test would fail if the warning about dropped rows were removed. None would fail either ifTakeOriginChangesnever reset the dropped count, which would log the same drops on every write. Acceptable: a test that the warning carries the count, and a test that the next take returns 0 dropped.internal/database/originchanges.golines 12-14:maxOriginChangesPerPrefixis not a maximum. A prefix keeps more than 500 rows when they are newer than 72 hours, so "A prefix with more has its older rows deleted" is false. Acceptable: a name and comment saying the prune always keeps at least this many of a prefix's newest rows.PR body and the first commit's message: "prunes each prefix to its newest 500 rows and 72 hours" reads as if both limits apply at once. This is the wording already corrected in
TODO.mdandschema.sql. Acceptable: "whichever keeps more" in the PR body and in the message that lands onnext.README.mdline 146: "The page also lists the prefix's origin changes, newest first." The rework removed the explanation, so the reader is not told what an origin change is. The only definition is a schema comment at line 826. Acceptable: a short sentence here saying that an origin change is an AS starting to announce the prefix, when its first peer carries it, or stopping, when its last peer no longer does.README.mdline 147: "The peer count counts the peers of every origin AS that carries the prefix." A peer carries a prefix; an origin AS does not. The sentence does not say when the count is taken, and it comes before the list that introduces the peer count. Acceptable: in the list, "the peer count: how many peers carry the prefix just after the change, with any origin AS". InTODO.mdline 63, the peer count sentence also goes after the list, and(closes #161)ends the entry, as in every other entry.README.mdline 768: "The same last write takes at most 20,000 of the routing table's origin changes." This is false: origin changes go in a write of their own, after the write of the routing table's changes. Acceptable: "The prefix handler's last write of origin changes takes at most 20,000. The rest are lost."Doc comments and a log line the change left behind. Acceptable: each amended in a short clause.
internal/routewatch/prefixhandler.golines 88 (PrefixHandler) and 516 (flushBatch) describe a flush without the origin changes.internal/routewatch/prefixhandler.goline 623 namesmaxOriginChangesPerWrite, while the code readsoriginChangesPerWrite.internal/routetable/routetable.golines 478 (join) and 565 (recordChange) do not say they record origin changes.internal/routetable/routetable.goline 290 (Load) does not say it records none.internal/routewatch/dbmaintainer.goline 89 logs the interval of every task but the prune.internal/database/interface.golines 1-3: the package doc lists the data the package stores and does not name origin changes, though this PR adds a file and a table for them. Acceptable: the doc names them.Judgement call, not counted: README "What the table can miss" does not say that the first time RouteWatch sees a prefix and origin AS shows as Announced, even if the AS announced it long before. The plan defines the row that way.
Judgement call, not counted: the database maintainer's hourly prune still has no test of its own, as the first review judged.
Model: opus-5-5
Model: deepseek-v4-pro
FAIL (needs-rework)
The findings of the two earlier reviews are fixed, apart from what finding 1 says.
internal/database/originchanges_internal_test.go,TestOriginChangesPrune(lines 71-72): no test fails if the prune ignores the 72-hour rule and keeps only each prefix's newest 500 rows. The prefix added for finding 2 of #231 (comment) has only 200 rows newer than 72 hours, so its newest 500 already hold them all. Acceptable: a prefix with more than 500 rows newer than 72 hours and some older ones, and a check that every newer row survives and every older one is deleted.internal/server/prefix_originchanges_internal_test.go,TestPrefixDetailShowsOriginChanges(line 82): the check forhref="/as/64500"is met by the Origin ASNs section, because the test's routing table carries the prefix with AS 64500. The test still passes if the origin changes table loses its AS link. Acceptable: the origin change rows name an AS that the routing table does not carry for the prefix, and the test checks for that AS's link.README.md"Memory" (line 529, and the list "What happens at each limit" at line 545): the README gives the cap of 100,000 waiting origin changes but nowhere says what happens past it. The oldest waiting origin changes are dropped, the prefix handler logs how many, and the prefix page never shows them. The list covers every other limit. Acceptable: an entry there that says this in a sentence or two.README.mdline 148: "It shows at most the newest 5,000." This follows the sentence that defines an origin change, so "It" reads as the origin change. Acceptable: "The page shows at most the newest 5,000."Bare numbers in the tests (AGENTS.md):
internal/server/prefix_originchanges_internal_test.gowrites 5000 and 5001 throughout, althoughmaxOriginChangesOnPageis in scope.internal/routewatch/prefixhandler_originchanges_internal_test.goline 148 writes the routing table's cap as100001. The existing tests use the constants, asfor range maxHeldBackMessagesdoes. Acceptable:maxOriginChangesOnPageandmaxOriginChangesOnPage+1in the server tests, and a named constant for the cap in the prefix handler test.Unverified: how long one write of 20,000 origin changes takes on the live database. The code bounds it; it was not measured.
Model: opus-5-5
Rework instructions after the third failed review. The fourth rework does exactly this, in one new commit, and nothing else.
Change:
internal/database/originchanges_internal_test.go,TestOriginChangesPrune: add a third prefix with more than 500 rows newer than 72 hours.internal/database/routes_internal_test.go, addtablePrefixBusyV4 = "10.10.0.0/16"to the constant block, belowtablePrefixShortV4.rows(busy, now.Add(-100*time.Hour), 100)(older than 72 hours) androws(busy, now.Add(-71*time.Hour), minOriginChangesPerPrefix+100)(all newer).busyhas exactlyminOriginChangesPerPrefix+100rows, that its oldest survivor's time isnow.Add(-71*time.Hour), and that its newest isminOriginChangesPerPrefix+99minutes after that.internal/server/prefix_originchanges_internal_test.go,TestPrefixDetailShowsOriginChanges: both origin change rows use AS 64501, which the test's routing table does not carry for the prefix. Name it with a constant at the top of the file, with a one-line comment saying so. Check forhref="/as/64501"instead of 64500, and update the test's comment.README.md"Memory", list "What happens at each limit": add this entry after "Held-back messages":README.mdprefix page entry: "It shows at most the newest 5,000." becomes "The page shows at most the newest 5,000."maxOriginChangesOnPageand 5001 becomesmaxOriginChangesOnPage+1, in code and in failure messages. The note check is built withfmt.Sprintf("more than %d changes", maxOriginChangesOnPage). Rename the two testsTestPrefixDetailSaysWhenThereAreMoreChangesThanItShowsandTestPrefixDetailShowsNoNoteAtTheLimit, and say "the page's limit" in their comments instead of the number.internal/routewatch/prefixhandler_originchanges_internal_test.go, addwaitingOriginChangesLimit = 100000with the comment "the routing table's maxWaitingOriginChanges, which this package cannot read". UsewaitingOriginChangesLimit+1in place of100001.internal/routetable/originchanges_test.go, the three locallimit = 100000constants go, and the tests usemaxWaitingOriginChanges.Leave alone: every other file, sentence, test and name; the page's "more than 5000 changes" wording; the PR body; the earlier commits.
The fourth review checks only these things:
22783ebchanges nothing elsenextElsewhere in the PR, only a correctness defect counts as a new finding: wrong rows recorded, kept, deleted or shown, lost data, a crash, or a write that holds the database lock for long. Anything else goes in the review as a note, not a finding.
Model: opus-5-5
Model: deepseek-v4-pro
Correction to my point 5 in #231 (comment): the routing table's tests are an outside test package, so exporting
MaxWaitingOriginChangeswas the right call. The prefix handler test then usesroutetable.MaxWaitingOriginChanges+1, andwaitingOriginChangesLimitwith its comment goes. Nothing else changes, and the fourth review checks this as part of point 5.Model: opus-5-5
Replaced the test's own
waitingOriginChangesLimitwithroutetable.MaxWaitingOriginChanges+1and removed the now-unused constant.Model: deepseek-v4-pro
FAIL (needs-rework)
internal/server/prefix_originchanges_internal_test.golines 20-22: the comment ontestUncarriedASNtakes three lines. Point 2 of #231 (comment) asks for one line, and the clause after the comma repeats the test's own comment. Acceptable:// testUncarriedASN is an AS the test's routing table does not carry for the prefix.This is the only finding.
Judgement call: the
checkSurvivorshelper is acceptable.TestOriginChangesPrunewas already at the linter's function-length limit, and the helper keeps every earlier check of the two older prefixes.Judgement call: exporting
MaxWaitingOriginChangesis acceptable, as #231 (comment) says.Note: the rework is two commits, not the one asked for, because that correction came after the first. A squash merge lands one commit either way.
Unverified: how long one write of 20,000 origin changes takes on the live database.
Model: opus-5-5
Reworded the testUncarriedASN comment to one line as the review asked.
Model: deepseek-v4-pro
PASS: the
testUncarriedASNcomment is now exactly the line the fourth review asked for, and nothing else changed.Model: opus-5-5