Skip to content

The Yahoo fetch and batch paths agree about the current session's placeholder bar - #47

Merged
stefan-jansen merged 3 commits into
mainfrom
fix/w7-reader-data-defects
Sep 8, 2026
Merged

The Yahoo fetch and batch paths agree about the current session's placeholder bar#47
stefan-jansen merged 3 commits into
mainfrom
fix/w7-reader-data-defects

Conversation

@stefan-jansen

Copy link
Copy Markdown
Contributor

Two defects a reader of the book hits through the public API.

The Yahoo single-symbol path raised on the current session's placeholder bar

From the US close until Yahoo consolidates the daily bar, yf.download returns a final row for the current exchange date carrying volume and null OHLC. fetch_ohlcv passed it to _validate_no_nulls, which raised on the whole fetch:

DataValidationError: yahoo: Column 'open' contains 1 null values

fetch_ohlcv_batch had always dropped that row. So the same bar succeeded through batch_load and failed through fetch, load and update in the same run - which is what a reader of 18_data_management saw, its batch_load cell passing and its update cell failing on the same three symbols. update() could not avoid it: StorageManager.update sets fetch_end to now and has no end-date parameter, so an incremental update always asks for the current session.

Both paths now share _drop_priceless_rows, so they cannot drift again. A row with prices and zero volume is a halted session and is kept; a window holding nothing but placeholders raises SymbolNotFoundError rather than reporting an empty fetch as a successful one. _validate_no_nulls stays strict - a row with volume and no prices is not a session, and accepting it would write null prices into storage and into everything computed from them.

Four regression tests reproduce the reported frame exactly (MultiIndex columns, volume present, NaN OHLC, the 2026-09-02/03 AAPL rows from the report). Before the fix three of them fail with the exact error above.

Closes ml4t/agent-workspace#1027.

The continuous-contract builder had no end-to-end roll-gap assertion

Every adjustment assertion on ContinuousContractBuilder.build mocked the adjustment method, so nothing checked that the series a reader gets back has the roll gap removed. The paired-RollEvent rewrite that fixed the adjustment classes was unguarded at the level the reader uses.

Four tests on a two-contract panel in 10-point contango: the unadjusted series moves 11.0 across the roll (10.0 of spread, 1.0 genuine), back-adjustment moves 1.0, ratio adjustment returns 1/103 - the outgoing contract's own return - and both anchor the last bar on its traded price. Checked against a deliberately gap-leaving adjuster: both assertions reject it.

Refs ml4t/agent-workspace#12. The defects that issue reports are already fixed and released; verified against the published 0.1.3 wheel, which reproduces the issue's own expected output ([600, 600, 600, 600, 600], no roll-day return) where the version it was filed against printed [5.5556, 5.5556, 66.6667, 66.6667, 600.0].

Verification

uv run pytest tests/ -q: 3,619 passed, 280 deselected. ruff check, ruff format --check and ty check clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WfX5GbAHc5EVwdUnRSoUPT

Stefan Jansen added 2 commits September 8, 2026 16:04
From the US close until Yahoo consolidates the daily bar, `yf.download` returns a
final row for the current exchange date carrying volume and null OHLC.
`fetch_ohlcv` passed it to `_validate_no_nulls`, which raised on the whole fetch
("yahoo: Column 'open' contains 1 null values"), while `fetch_ohlcv_batch` had
always dropped it. The same bar therefore succeeded through `batch_load` and
failed through `fetch`, `load` and `update` in the same run, and `update()` could
not avoid it because `StorageManager.update` always asks for the current session.

Both paths now share `_drop_priceless_rows`, so they cannot drift again. A row with
prices and zero volume is a halted session and is kept; a window that holds nothing
but placeholders raises `SymbolNotFoundError` rather than reporting an empty fetch
as a successful one. `_validate_no_nulls` stays strict.

Closes ml4t/agent-workspace#1027.
Every adjustment assertion on `ContinuousContractBuilder.build` mocked the
adjustment method, so nothing checked that the series a reader gets back has the
roll gap removed. The paired-`RollEvent` rewrite that fixed the adjustment classes
is therefore unguarded at the level the reader uses.

Four tests on a two-contract panel in 10-point contango: the unadjusted series moves
11.0 across the roll (10.0 of spread, 1.0 genuine), back-adjustment moves 1.0, ratio
adjustment returns 1/103 - the outgoing contract's own return - and both anchor the
last bar on its traded price. Checked against a deliberately gap-leaving adjuster:
it is rejected by both assertions.

Refs ml4t/agent-workspace#12.
Copilot AI lite review requested due to automatic review settings September 8, 2026 20:28

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

The audit on this PR failed on gitpython 3.1.58, which carries CVE-2026-78679 with
eight known vulnerabilities, fixed in 3.1.59. It arrives transitively through
mkdocs-git-revision-date-localized-plugin, so the lock is the only place to move it.
The last main run predates the advisory, which is why main was green.

`uv run pytest tests/ -q`: 3,619 passed, 280 deselected, unchanged.
@stefan-jansen
stefan-jansen merged commit db9b8ef into main Sep 8, 2026
22 checks passed
@stefan-jansen
stefan-jansen deleted the fix/w7-reader-data-defects branch September 8, 2026 20:59
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