fix(data): chunk backfill's candle requests too, the last unpaged window site - #295
Merged
Conversation
…indow site #269 chunked `market_feed.poll_once`; #271 swept for the same defect and fixed `repair.py` and `history.py`. `market_feed.backfill` was not reached by either, and has the identical shape: `_missing_ranges` groups absent timestamps into CONTIGUOUS ranges and each range was requested in a single call, so a hole wider than the venue's ~350-candle cap 400s exactly as the poll path did. On an empty repo the entire history window is one such range. LATENT, and stated plainly rather than dressed up: `backfill` has no production caller today -- `keel fetch` goes through `history.ensure_history` -- so nothing is failing on this right now. It is worth closing anyway on #271's own stated grounds, that every candle-request windowing site in the codebase should agree. A public data-layer entry point that 400s the moment it is called from anywhere is a trap left armed for whoever calls it next. The windowing arithmetic is now in one place, `_request_windows`, used by both `backfill` and `_poll_catch_up`. That was the actual reason this site was easy to miss: the `(MAX_CANDLES_PER_REQUEST - 1)` expression existed only inside `_poll_catch_up`, so `backfill` had nothing to be inconsistent WITH. #271 had to correct exactly that off-by-one after finding it duplicated in `history.py` -- the same bug class the shared `MAX_CANDLES_PER_REQUEST` import exists to prevent -- so it now has one definition here. `_poll_catch_up` keeps its own filter and `seen` dedup and is otherwise unchanged; folding it onto the helper also removed a `window_start = window_end + gran_sec` line left stranded when its `while` became a `for`. Tests written first and confirmed failing for the right reason: both cap tests failed on a single 575-candle request against the unfixed code. The third is a regression guard that a window within the cap still costs exactly one request, which passes before and after. Gates: ruff clean, mypy clean (77 source files), pytest 2762 -> 2765 passed / 1 skipped. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Follow-on to #269 / #271. Supersedes #294, which I opened against
29d9b20before those landed and have now closed as redundant.What this is
#269 chunked
market_feed.poll_once. #271 swept for the same defect and fixedrepair.pyandhistory.py.market_feed.backfillwas not reached by either, and has the identical shape —_missing_rangesgroups absent timestamps into contiguous ranges, and each range was requested in one call, so a hole wider than the venue's ~350-candle cap 400s exactly as the poll path did. On an empty repo the entire history window is one such range.It is latent, and I'd rather say so than dress it up
backfillhas no production caller today —keel fetchgoes throughhistory.ensure_history. Nothing is failing on this right now.Worth closing anyway on #271's own stated grounds, that every candle-request windowing site in the codebase should agree. A public data-layer entry point that 400s the moment anything calls it is a trap left armed for whoever calls it next.
Why it was easy to miss
The
(MAX_CANDLES_PER_REQUEST - 1)arithmetic existed only inside_poll_catch_up, sobackfillhad nothing to be inconsistent with — it just looked like ordinary pre-#269 code. #271 had to correct that exact off-by-one after finding it duplicated inhistory.py, which is the same bug class the sharedMAX_CANDLES_PER_REQUESTimport exists to prevent.So the windowing now has one definition,
_request_windows, used by bothbackfilland_poll_catch_up._poll_catch_upkeeps its own filter andseendedup and is otherwise unchanged; folding it onto the helper also removed awindow_start = window_end + gran_secline left stranded when itswhilebecame afor.Tests
Written first and confirmed failing for the right reason — both cap tests failed on a single 575-candle request against the unfixed code:
test_backfill_never_requests_more_than_the_candle_captest_backfill_chunk_windows_are_contiguous_and_cover_the_gaptest_backfill_still_uses_one_request_for_a_gap_within_the_cap— regression guard, passes before and afterGates
pytest— 2762 → 2765 passed, 1 skippedruff check— cleanmypy keel— clean, 77 source files🤖 Generated with Claude Code