Repository navigation
--limit is a budget of analyses, and the fetch window stays materialised - #24
Merged
Merged
Conversation
jackparnell
approved these changes
Aug 18, 2026
Two review items that look unrelated and are not: fixing the first the obvious way breaks the second. (1) --limit MEANT "posts fetched". Local memory, --days and own-post filters all run AFTER the fetch, so asking for 10 routinely analysed three and said nothing about it. That is the same shape as the bug in Now the scan fetches a candidate window of N * SCAN_FETCH_MULTIPLIER (capped at SCAN_FETCH_CAP) and stops at N ANALYSES. Anything past the budget is left unanalysed AND unmarked, so the next run takes it. When the window runs dry first the run says so, instead of finishing quietly: Analysed 2 of a requested 10 — the 30-post candidate window held no more eligible posts (already analysed, older than --days 7, or the sentinel's own). (2) THE OBVIOUS FIX FOR (1) IS WRONG. Consuming iter_posts lazily until N are analysed would make --limit honest and silently break the scan: _process_post marks each post scanned, which REMOVES it from the sentinel_scanned=false set the server is paginating, and iter_posts pages by offset. Mark k posts and the next page starts k further into a set that just lost k members — skipping k posts that were never seen. So list(...) is load-bearing, not incidental. It now says so at the call site, in the README, and in a test that asserts nothing is pulled off the generator after the first analysis begins. That test is the only thing standing between a future "why materialise? just iterate" and a scanner that skips one post for every post it finishes. Mutation-verified both ways: drop list(...) -> 2 tests fail, both in TestTheWindowIsMaterialised drop the break -> 4 tests fail, incl. the budget and untouched-posts 210 tests pass, ruff clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pxqw6ZyFv5f4rt5HFwtafs
arch-colony
force-pushed
the
fix/limit-means-analysed
branch
from
August 18, 2026 18:40
0815aa2 to
333339a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Review items 1 and 3. They look unrelated and are not: fixing the first the obvious way breaks the second.
1.
--limitmeant "posts fetched"Local memory,
--daysand own-post filters all run after the fetch, so asking for 10 routinely analysed three — and said nothing about it. Same shape as #22: a run that reports success having done less work than it was asked for.Now the scan fetches a candidate window of
N × SCAN_FETCH_MULTIPLIER(capped atSCAN_FETCH_CAP) and stops at N analyses. Anything past the budget is left unanalysed and unmarked, so the next run takes it. When the window runs dry first, the run says so instead of finishing quietly:2. The obvious fix for that is wrong
Consuming
iter_postslazily until N are analysed would make--limithonest and silently break the scan._process_postmarks each post scanned, which removes it from thesentinel_scanned=falseset the server is paginating — anditer_postspages by offset. Mark k posts, and the next page starts k further into a set that just lost k members, skipping k posts that were never seen. No error, no warning, a healthy-looking log.So
list(...)is load-bearing rather than incidental. It now says so at the call site, in the README, and in a test asserting nothing is pulled off the generator after the first analysis begins. That test is the only thing standing between a future "why materialise? just iterate" and a scanner that skips one post for every post it completes.Verification
Mutation-verified both ways:
list(...)TestTheWindowIsMaterialisedbreak210 tests pass (9 new), ruff clean.
Note this branches off
master, so it does not include #23 (the GPU gate) — the two are independent and can merge in either order.🤖 Generated with Claude Code
https://claude.ai/code/session_01Pxqw6ZyFv5f4rt5HFwtafs