fix: apply --limit to the write, not to the selection #22

Merged
claude merged 1 commits from fix/limit-bounds-the-write into main 2026-08-10 13:21:55 +00:00
Collaborator

Closes #18.

pull.py's docstring said "the limit is on the write, not on the selection" and the code did the opposite: _gitea.list_issues truncated the payload list to limit, and pull.py dropped the closed ones after that. A milestone whose first issues are closed answered --limit 20 with twelve files, and the only statement about the behavior anywhere was the false one.

What changed

  • --limit counts what the run leaves in the store. list_issues takes a keep predicate; pages keep arriving until limit payloads satisfy it. Payloads that did not count are still returned, so pull.py can keep reporting "N closed issue(s) enumerated, not stored". What keep means stays the caller's business — the transport only counts.
  • pull.lands_in_store is the predicate, and it is the same test the walk itself applies: a closed issue counts only when the store already has it, because that one is refreshed rather than dropped. A --cached skip counts too — the store holds it when the run ends.
  • paginate is now a thin wrapper over a new pages generator. The page after the one that fills the budget is never requested.
  • The scan is bounded. "Fetch until N are kept" is "fetch the whole tracker" on a filter that matches mostly closed issues, so a keep-bounded read scans at most _gitea.PAGE_SLACK (4) times the pages the limit would need if nothing were dropped, then warns on stderr and returns short. Raising --limit raises that ceiling with it. --deps is outside the count: a dependency is followed because an issue named it, not because the filter selected it.
  • remote.py keeps the old meaning, now said out loud. It writes nothing, so there is no write for a limit to bound and its --limit caps the listing, closed issues included. The divergence is documented in both scripts and in the /tea:sync command table.
  • --limit 0 is refused instead of dividing by the page size and raising ZeroDivisionError.

Acceptance criteria

  • pull.py --limit N writes N issues when at least N non-closed ones match the filter
  • pagination keeps requesting pages until N are stored or the pages run out
  • no page beyond the one that filled the budget is requested
  • the docstring, skills/sync/SKILL.md and the behavior say the same thing; the remote.py divergence is stated explicitly
  • test: a selection that is half closed gives exactly N files under --limit N

Tests

tests/test_pull_limit.py stubs the transport at _gitea.api with a fake that serves page= / limit= itself, so the request pattern is observed rather than assumed. It covers exactly N files out of a half-closed selection, the second page being fetched and the third not, the scan stopping at the budget with a warning, a closed issue already on disk spending the budget, and remote.py's listing being unchanged. Stdlib unittest, a throwaway store per test, no network.

python3 -m unittest discover -s tests
Ran 251 tests in 2.7s — OK

Reverting pull.py and _gitea.py while keeping the new file fails 8 of its tests.

🤖 Generated with Claude Code

Closes #18. `pull.py`'s docstring said "the limit is on the write, not on the selection" and the code did the opposite: `_gitea.list_issues` truncated the payload list to `limit`, and `pull.py` dropped the closed ones after that. A milestone whose first issues are closed answered `--limit 20` with twelve files, and the only statement about the behavior anywhere was the false one. ## What changed - **`--limit` counts what the run leaves in the store.** `list_issues` takes a `keep` predicate; pages keep arriving until `limit` payloads satisfy it. Payloads that did not count are still returned, so `pull.py` can keep reporting "N closed issue(s) enumerated, not stored". What `keep` means stays the caller's business — the transport only counts. - **`pull.lands_in_store` is the predicate**, and it is the same test the walk itself applies: a closed issue counts only when the store already has it, because that one is refreshed rather than dropped. A `--cached` skip counts too — the store holds it when the run ends. - **`paginate` is now a thin wrapper over a new `pages` generator.** The page after the one that fills the budget is never requested. - **The scan is bounded.** "Fetch until N are kept" is "fetch the whole tracker" on a filter that matches mostly closed issues, so a `keep`-bounded read scans at most `_gitea.PAGE_SLACK` (4) times the pages the limit would need if nothing were dropped, then warns on stderr and returns short. Raising `--limit` raises that ceiling with it. `--deps` is outside the count: a dependency is followed because an issue named it, not because the filter selected it. - **`remote.py` keeps the old meaning, now said out loud.** It writes nothing, so there is no write for a limit to bound and its `--limit` caps the listing, closed issues included. The divergence is documented in both scripts and in the `/tea:sync` command table. - **`--limit 0` is refused** instead of dividing by the page size and raising `ZeroDivisionError`. ## Acceptance criteria - [x] `pull.py --limit N` writes N issues when at least N non-closed ones match the filter - [x] pagination keeps requesting pages until N are stored or the pages run out - [x] no page beyond the one that filled the budget is requested - [x] the docstring, `skills/sync/SKILL.md` and the behavior say the same thing; the `remote.py` divergence is stated explicitly - [x] test: a selection that is half closed gives exactly N files under `--limit N` ## Tests `tests/test_pull_limit.py` stubs the transport at `_gitea.api` with a fake that serves `page=` / `limit=` itself, so the request pattern is observed rather than assumed. It covers exactly N files out of a half-closed selection, the second page being fetched and the third not, the scan stopping at the budget with a warning, a closed issue already on disk spending the budget, and `remote.py`'s listing being unchanged. Stdlib `unittest`, a throwaway store per test, no network. ``` python3 -m unittest discover -s tests Ran 251 tests in 2.7s — OK ``` Reverting `pull.py` and `_gitea.py` while keeping the new file fails 8 of its tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
claude added 1 commit 2026-08-10 12:25:14 +00:00
pull.py's docstring said "the limit is on the write, not on the
selection", and the code did the opposite: `list_issues` truncated the
payload list to `limit`, and pull.py dropped the closed ones after that.
A milestone whose first issues are closed therefore answered `--limit 20`
with twelve files, and the only statement about the behavior anywhere was
the false one.

The limit now counts what the run leaves in the store. `list_issues`
takes a `keep` predicate, pages keep arriving until `limit` payloads have
satisfied it, and the ones that did not are still returned — they were
enumerated, and pull.py still reports them as "N closed, not stored".
What `keep` means stays the caller's business; the transport only counts.
pull.py hands it `lands_in_store`, which is the same test the walk itself
applies: a closed issue counts only when the store already has it, since
that one is refreshed rather than dropped.

Pagination is the other half, and it cuts both ways. `paginate` is now a
thin wrapper over a new `pages` generator, so the page after the one that
fills the budget is never requested. In the other direction "fetch until
N are kept" is "fetch the whole tracker" on a filter that matches mostly
closed issues, so a keep-bounded read scans at most PAGE_SLACK times the
pages the limit would need if nothing were dropped, then warns on stderr
and returns short. Raising --limit raises that ceiling with it. --deps is
outside the count: a dependency is followed because an issue named it.

remote.py keeps the old meaning and now says so in as many words — it
writes nothing, so there is no write for a limit to bound, and its
--limit caps the listing, closed issues included. Same flag, two jobs,
documented in both scripts and in the skill's command table.

Also refuses `--limit 0` instead of dividing by the page size and
raising ZeroDivisionError.

tests/test_pull_limit.py stubs the transport with a fake that serves
`page=`/`limit=` itself, so the request pattern is observed rather than
assumed: exactly N files out of a half-closed selection, the second page
fetched and the third not, the scan stopping at the budget with a
warning, and remote.py's listing unchanged. 251 tests, no network.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
claude merged commit cfbd6e5ddb into main 2026-08-10 13:21:55 +00:00
Sign in to join this conversation.