fix(etfs): rebuild combined file from partitions, not stale aggregate - #46
Conversation
|
Accepted as the issue record under the ecosystem external-contribution policy. I reproduced the defect through the public Focused verification on the submitted commit passes: 37 ETF/regression tests, Ruff check and format, and ty. Full repository qualification is now running before review approval. |
stefan-jansen
left a comment
There was a problem hiding this comment.
Verified at ce0e7e8. Reproduced through ETFDataManager.update(): main advanced the partition but left the combined dataset stale, while this head advances both. Full repository qualification passed: Ruff, formatting, ty, 3,611 tests, wheel and sdist installation verification, strict docs build, dependency audit, and pre-commit.
Summary
Fixes ETFDataManager._regenerate_combined() so it actually rebuilds etf_universe.parquet from the individual ticker partitions.
Problem
_regenerate_combined() currently calls:
all_data = self.load_all()
but load_all() prefers the existing etf_universe.parquet when that file exists. This means _regenerate_combined() can read the stale combined file and write it straight back out, even after the individual ticker partitions have been updated successfully.
In practice, I reproduced this with the ML4T Chapter 25 ETF deployment loop:
individual ticker partitions advanced through 2026-08-20
ETFDataManager.update() reported new rows
_regenerate_combined() ran
the combined aggregate still ended at 2025-12-31
Deleting the combined file caused load_all() to fall back to load_symbols(...), after which the aggregate rebuilt correctly. That isolated the issue to the cache-preferring behavior of load_all().
Fix
Change _regenerate_combined() to load the configured ticker partitions directly:
all_data = self.load_symbols(self.config.get_all_symbols())
This matches the method docstring (“Regenerate combined file from individual ticker files”) and uses the same partition-loading path that load_all() already falls back to when the aggregate is absent.
Tests
Added regression coverage for:
rebuilding when an existing aggregate is stale
restoring symbols present in ticker partitions but missing from the aggregate
Targeted test run:
37 passed, 2 warnings
End-to-end verification
I installed this branch into the machine-learning-for-trading project via a SHA-pinned uv Git source and reran the Chapter 25 ETF deployment loop with the existing frozen etf_universe.parquet still present.
Before the fix, the combined file remained at:
470,662 rows, ending 2025-12-31
After the fix, the normal refresh path rebuilt it successfully without deleting the aggregate first:
ml4t-data update: 318 new rows across 2 symbols
Prices: 486,552 rows x 100 symbols, 2006-01-03 to 2026-08-20
No changes were made to the deployment logic itself; this PR only fixes aggregate regeneration inside ml4t-data.