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

README.md

@@ -147,7 +147,9 @@ CLI flags shown in `python3 sentinel.py {scan,webhook,webhook-register} --help`.
 | `DEFAULT_MODEL` | `qwen3.5:9b-q4_k_m` | `SENTINEL_MODEL` | Ollama model. Must be an installed tag — Ollama lowercases tags on pull, and a startup preflight fails fast (with `ollama pull …`) if it's missing. Also overridable per-run via `--model`. |
 | `REQUIRE_GPU` | `1` (on) | `SENTINEL_REQUIRE_GPU` | Refuse to run if the model is answering from CPU. See **GPU requirement** below. Per-run escape hatch: `--allow-cpu`. |
 | `GPU_MIN_OFFLOAD` | `0.9` | `SENTINEL_GPU_MIN_OFFLOAD` | Fraction of the model that must sit in VRAM before the run is considered healthy. Below it: a warning. At zero: abort. |
-| `DEFAULT_LIMIT` | 20 | — (`--limit`) | Posts per scan run |
+| `DEFAULT_LIMIT` | 20 | — (`--limit`) | Posts **analysed** per scan run — a budget, not a fetch size. See **How `--limit` is counted** below. |
+| `SCAN_FETCH_MULTIPLIER` | 3 | `SCAN_FETCH_MULTIPLIER` | Candidates fetched per post the run intends to analyse, so post-fetch skips don't shrink the batch |
+| `SCAN_FETCH_CAP` | 200 | `SCAN_FETCH_CAP` | Hard ceiling on that candidate window |
 | `MAX_COMMENTS` | 6 | — | Top comments included in analysis |
 | `DEFAULT_DAYS` | 7 | — (`--days`) | Only analyze posts newer than this (scan mode) |
 | `OLLAMA_TIMEOUT` | 180 | `OLLAMA_TIMEOUT` | Seconds before a single Ollama generation is cut off (the runaway bound — a wedged model can't hang the scan) |
@@ -216,6 +218,44 @@ Three outcomes:
 To run on CPU deliberately — a box with no GPU where a slow scan is
 acceptable — pass `--allow-cpu`, or set `SENTINEL_REQUIRE_GPU=0`.
 
+## How `--limit` is counted
+
+`--limit N` means **analyse up to N posts**, not "fetch N and analyse whatever
+survives".
+
+Several filters run *after* the fetch — posts already in local memory, posts
+older than `--days`, and the sentinel's own posts. Fetching exactly N therefore
+routinely analysed fewer than N, and said nothing about it. `--limit 10`
+quietly delivering three is the same shape as the bug where the scanner fetched
+the newest ten and skipped all ten: a run that reports success having done less
+work than it was asked for.
+
+So the scan fetches a **candidate window** of `N × SCAN_FETCH_MULTIPLIER`
+(capped at `SCAN_FETCH_CAP`), and stops once N posts have actually been
+analysed. Anything past the budget is left unanalysed *and unmarked*, so the
+next run picks it up. If the window runs dry first, the run says so explicitly
+rather than 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).
+```
+
+### Why the window is read all at once
+
+The fetch is deliberately materialised (`list(...)`) before the first post is
+processed, and that is load-bearing rather than incidental.
+
+Processing a post marks it scanned, which **removes it** from the
+`sentinel_scanned=false` set the server is paginating — and `iter_posts` pages
+by offset. Consuming the fetch lazily would have the window slide underneath
+the scan: 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. Reading the
+whole window before the first mark is what keeps the offsets stable.
+
+`tests/test_scan_batch_semantics.py` pins this by asserting no post is pulled
+off the generator after the first analysis begins.
+
 ## Vote-exemption lists
 
 Two optional files on the sentinel's own filesystem name users this instance must never vote on:

sentinel.py

@@ -141,6 +141,19 @@ def _env_int(name: str, default: int) -> int:
 # the next run re-burns the model over all of them. The write is atomic.
 SCAN_SAVE_EVERY = _env_int("SCAN_SAVE_EVERY", 10)
 
+# --limit means "analyse up to N posts", not "fetch N and analyse whatever
+# survives". Several filters run AFTER the fetch — local memory, --days, and
+# the sentinel's own posts — so a straight fetch of N routinely analysed fewer
+# than N and said nothing about it. That is the same shape as the bug where
+# the scanner fetched the newest ten and skipped all ten: a run that reports
+# success having done less work than asked.
+#
+# So over-fetch, then stop at N analyses. The multiplier is a candidate
+# budget, not a promise; SCAN_FETCH_CAP bounds it so a large --limit cannot
+# turn into an enormous request.
+SCAN_FETCH_MULTIPLIER = _env_int("SCAN_FETCH_MULTIPLIER", 3)
+SCAN_FETCH_CAP = _env_int("SCAN_FETCH_CAP", 200)
+
 DEFAULT_USERNAME = "qwen-jorwhol-analyzer"
 DEFAULT_DISPLAY_NAME = "Qwen 3.5 Jorwhol Analyzer"
 DEFAULT_BIO = (
@@ -1416,17 +1429,31 @@ def _checkpoint() -> None:
                 new_analyses += 1
         else:
             scanned_filter = scanned_filter_for(args)
+            fetch_n = min(args.limit * SCAN_FETCH_MULTIPLIER, SCAN_FETCH_CAP)
             try:
+                # MATERIALISED ON PURPOSE — do not make this lazy.
+                #
+                # _process_post marks each post scanned as it goes, which
+                # REMOVES it from the sentinel_scanned=false set the server is
+                # paginating. iter_posts pages by offset, so a lazily-consumed
+                # generator would have the window slide underneath it and skip
+                # one unseen post for every post already marked. Collecting the
+                # whole window before the first mark is what makes the offsets
+                # stable. Pinned by tests/test_scan_batch_semantics.py.
                 posts = list(client.iter_posts(
                     sort=args.sort,
-                    max_results=args.limit,
+                    max_results=fetch_n,
                     sentinel_scanned=scanned_filter,
                 ))
             except ColonyAPIError as e:
                 logger.error("Failed to fetch posts: %s", e)
                 sys.exit(1)
 
             for post in posts:
+                if new_analyses >= args.limit:
+                    # --limit is a budget of ANALYSES. Everything past this is
+                    # left unmarked, so the next run picks it up.
+                    break
                 post_id = post.get("id")
                 title = (post.get("title") or "")[:70]
                 created_at = post.get("created_at")
@@ -1461,6 +1488,17 @@ def _checkpoint() -> None:
         # Always persist what we finished — even on Ctrl-C / crash / sys.exit.
         _checkpoint()
 
+    # Say so when the run could not fill its budget. Silence here is what let
+    # "analysed 3 of a requested 10" read as a healthy scan.
+    if not args.post_id and new_analyses < args.limit:
+        logger.info(
+            "Analysed %d of a requested %d — the %d-post candidate window held "
+            "no more eligible posts (already analysed, older than --days %d, or "
+            "the sentinel's own). Raise --limit, widen --days, or raise "
+            "SCAN_FETCH_MULTIPLIER if you expected more.",
+            new_analyses, args.limit, fetch_n, args.days,
+        )
+
     if not args.dry_run:
         logger.info("Memory updated: %d posts (added %d new)", len(memory), new_analyses)
     else:

tests/test_scan_batch_semantics.py

@@ -0,0 +1,175 @@
+"""`--limit N` means "analyse up to N posts", and the fetch is materialised.
+
+Two properties that look unrelated and are not: fixing the first naively
+breaks the second.
+
+**--limit is a budget of ANALYSES.** Several filters run AFTER the fetch —
+local memory, `--days`, and the sentinel's own posts — so fetching exactly N
+routinely analysed fewer than N and said nothing about it. `--limit 10`
+delivering three is the same shape as the bug in #22: a run that reports
+success having done less work than asked. So the scan over-fetches a
+candidate window and stops at N analyses.
+
+**The window is materialised before the first mark.** The obvious way to make
+--limit honest is to consume `iter_posts` lazily until N are analysed. That is
+wrong here, and quietly so. `_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. A lazy generator would have the window slide
+underneath it: mark k posts, and the next page starts k posts further into a
+set that just lost k members, skipping k posts that were never seen. Collecting
+the whole window before the first mark is what keeps the offsets stable.
+
+That is why `list(...)` is load-bearing rather than incidental, and why
+`test_the_window_is_fully_read_before_anything_is_marked` exists — it is the
+only thing standing between a future "optimisation" and a scanner that
+silently skips a post for every post it completes.
+"""
+from __future__ import annotations
+
+from types import SimpleNamespace
+
+import pytest
+
+import sentinel as s
+
+
+def _args(**over):
+    base = dict(
+        dry_run=True, no_vote=True, no_pii=True, model="m", username=None,
+        force=False, post_id=None, sort="new", limit=3, days=7, confirm=False,
+        include_scanned=False, allow_cpu=True,
+    )
+    base.update(over)
+    return SimpleNamespace(**base)
+
+
+def _post(i, *, author="someone", created=None):
+    from datetime import datetime, timedelta, timezone
+    when = created or (datetime.now(timezone.utc) - timedelta(hours=1)).isoformat()
+    return {"id": f"post-{i:03d}", "title": f"Post {i}",
+            "created_at": when, "author": {"username": author}}
+
+
+@pytest.fixture
+def harness(monkeypatch, tmp_path):
+    """Drive cmd_scan with every side effect stubbed except the batch logic."""
+    monkeypatch.setattr(s, "MEMORY_FILE", tmp_path / "mem.json")
+    monkeypatch.setattr(s, "ensure_model_available", lambda m: None)
+    monkeypatch.setattr(s, "ensure_gpu_available", lambda m: None, raising=False)
+    monkeypatch.setattr(s, "log_vote_exemptions", lambda: None)
+
+    state = SimpleNamespace(requested=[], analysed=[], yielded=[], events=[])
+
+    client = SimpleNamespace()
+
+    def iter_posts(**kw):
+        state.requested.append(kw)
+        for p in state.available:
+            state.yielded.append(p["id"])
+            state.events.append(("yield", p["id"]))
+            yield p
+
+    client.iter_posts = iter_posts
+    monkeypatch.setattr(s, "get_or_register_client",
+                        lambda u: (client, {"username": "sentinel-bot"}))
+    monkeypatch.setattr(s, "fetch_post_with_comments",
+                        lambda c, pid: {"post": {"id": pid}})
+
+    def process(client_, data, args, memory, results):
+        pid = data["post"]["id"]
+        state.analysed.append(pid)
+        state.events.append(("analyse", pid))
+        memory[pid] = {"score": 5}
+        return True
+
+    monkeypatch.setattr(s, "_process_post", process)
+    state.available = []
+    return state
+
+
+class TestLimitIsABudgetOfAnalyses:
+    def test_it_stops_at_the_limit(self, harness):
+        harness.available = [_post(i) for i in range(10)]
+        s.cmd_scan(_args(limit=3))
+        assert harness.analysed == ["post-000", "post-001", "post-002"]
+
+    def test_it_over_fetches_so_skips_do_not_shrink_the_batch(self, harness):
+        """The whole point: ask for more candidates than the budget, because
+        some will be filtered out after the fetch."""
+        harness.available = [_post(i) for i in range(30)]
+        s.cmd_scan(_args(limit=5))
+        assert harness.requested[0]["max_results"] == 5 * s.SCAN_FETCH_MULTIPLIER
+
+    def test_the_over_fetch_is_capped(self, harness):
+        """A large --limit must not turn into an enormous request."""
+        harness.available = []
+        s.cmd_scan(_args(limit=10_000))
+        assert harness.requested[0]["max_results"] == s.SCAN_FETCH_CAP
+
+    def test_skipped_posts_do_not_consume_the_budget(self, harness):
+        """Three of the first four are the sentinel's own; a budget of 3 must
+        still analyse 3, not 1. This is the behaviour that was broken."""
+        harness.available = [
+            _post(0, author="sentinel-bot"),
+            _post(1),
+            _post(2, author="sentinel-bot"),
+            _post(3),
+            _post(4),
+            _post(5),
+        ]
+        s.cmd_scan(_args(limit=3))
+        assert harness.analysed == ["post-001", "post-003", "post-004"]
+
+    def test_posts_beyond_the_budget_are_left_untouched(self, harness):
+        """Anything past the limit must stay unanalysed AND unmarked, so the
+        next run picks it up rather than it being silently consumed."""
+        harness.available = [_post(i) for i in range(10)]
+        s.cmd_scan(_args(limit=2))
+        assert "post-002" not in harness.analysed
+        assert len(harness.analysed) == 2
+
+    def test_a_short_batch_is_reported(self, harness, caplog):
+        """Silence here is what let 'analysed 3 of a requested 10' read as a
+        healthy scan."""
+        harness.available = [_post(0), _post(1)]
+        with caplog.at_level("INFO"):
+            s.cmd_scan(_args(limit=10))
+        assert any("Analysed 2 of a requested 10" in r.message for r in caplog.records)
+
+    def test_a_full_batch_is_not_reported_as_short(self, harness, caplog):
+        harness.available = [_post(i) for i in range(10)]
+        with caplog.at_level("INFO"):
+            s.cmd_scan(_args(limit=3))
+        assert not any("of a requested" in r.message for r in caplog.records)
+
+
+class TestTheWindowIsMaterialised:
+    def test_the_window_is_fully_read_before_anything_is_marked(self, harness):
+        """The load-bearing one.
+
+        Every post must be pulled off the generator BEFORE the first analysis
+        (and therefore before the first mark_post_scanned). If this ever
+        interleaves, the server-side offset window shifts under the scanner
+        and it skips one unseen post per post completed — silently, with a
+        healthy-looking log.
+        """
+        harness.available = [_post(i) for i in range(6)]
+        s.cmd_scan(_args(limit=6))
+
+        kinds = [k for k, _ in harness.events]
+        first_analyse = kinds.index("analyse")
+        assert "yield" not in kinds[first_analyse:], (
+            "the fetch is being consumed lazily — posts were still being "
+            "pulled after marking began, so the offset window will slide and "
+            "the scan will skip work. Keep the list(...) around iter_posts."
+        )
+        assert harness.yielded == [f"post-{i:03d}" for i in range(6)]
+
+    def test_the_whole_window_is_read_even_when_the_budget_is_smaller(self, harness):
+        """A consequence worth stating: over-fetching means we read more than
+        we analyse. That is the price of stable offsets, and it is cheap — the
+        fetch is one request, the analysis is an LLM call per post."""
+        harness.available = [_post(i) for i in range(9)]
+        s.cmd_scan(_args(limit=3))
+        assert len(harness.yielded) == 9
+        assert len(harness.analysed) == 3