Skip to content

--limit is a budget of analyses, and the fetch window stays materialised - #24

Merged
jackparnell merged 1 commit into
masterfrom
fix/limit-means-analysed
Aug 18, 2026
Merged

jackparnell merged 1 commit into
masterfrom
fix/limit-means-analysed

Conversation

@arch-colony

Copy link
Copy Markdown
Collaborator

Review items 1 and 3. They 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. 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 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 that 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. 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:

Mutation Result
drop list(...) 2 fail, both in TestTheWindowIsMaterialised
drop the budget break 4 fail, incl. the budget and untouched-posts checks

210 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

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
arch-colony force-pushed the fix/limit-means-analysed branch from 0815aa2 to 333339a Compare August 18, 2026 18:40
@jackparnell
jackparnell merged commit 850eeb3 into master Aug 18, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants