test(data): pin that a partially-failed window is never recorded absent - #274
Merged
Conversation
Follow-up from the independent review of #271. The rule "a gap window may be recorded absent-at-source only if EVERY chunk completed" is the most dangerous thing in `repair.py` to get wrong -- getting it wrong permanently writes off a hole the venue was never fully asked about -- and the multi-chunk path had no test that could see a regression in it. `test_one_bad_chunk_is_not_recorded_absent_and_a_later_window_still_repairs` fails the SECOND chunk. That means the first chunk lands 299 bars, so the surviving gap window's key shifts from (BASE+1d, BASE+553d) to (BASE+300d, BASE+553d). `probed_keys` in `repair.py` matches by EXACT key, so that window is skipped no matter what `probed_ok` holds: its `windows_absent_at_source == 0` and `get_gap_probes() == []` assertions are vacuous in their own setup. Failing the FIRST chunk instead upserts nothing, so the remaining window keeps its original key and the gate is genuinely exercised. Verified discriminating: with `probed_ok.append(window)` added to the failure path, this test FAILS and the pre-existing single-chunk `test_a_failed_fetch_is_NOT_recorded_as_absent` alone would not have covered the multi-chunk case. No production code changes -- the shipped behaviour was already correct. This only stops a plausible future refactor ("partial progress means the window was probed") from passing the suite. 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-up from the independent adversarial review of #271. No production code changes — the shipped behaviour is correct. This closes a hole in the tests.
The gap
repair.py's most dangerous rule is "a gap window may be recorded absent-at-source only if EVERY chunk completed". Getting it wrong permanently writes off a hole the venue was never fully asked about.test_one_bad_chunk_is_not_recorded_absent_and_a_later_window_still_repairsclaims to cover this. It doesn't. It fails the second chunk, so the first chunk lands 299 bars and the surviving window's key shifts from(BASE+1d, BASE+553d)to(BASE+300d, BASE+553d).probed_keysmatches by exact key, so the window is skipped regardless of whatprobed_okcontains — itswindows_absent_at_source == 0andget_gap_probes() == []assertions are vacuous in their own setup.The fix
Fail the first chunk instead. Nothing is upserted, the remaining window keeps its original key, and the
probed_keysgate is genuinely exercised.Verified discriminating
With
probed_ok.append(window)added to the failure path:Note the pre-existing single-chunk
test_a_failed_fetch_is_NOT_recorded_as_absentdoes defend this rule for the one-chunk case — so the review's "not pinned by any test" was slightly overstated. The real gap was the multi-chunk path, which is what this covers.Gates
🤖 Generated with Claude Code