[sijie-Z]

P0. Second of the four research-integrity defects. The class docstring describes the purge gap in "trading days"; the code counted rows.

What was wrong

_prepare_dataset returns one row per (date, asset) pair, but the splitter was handed a row count:

for fold, (train_idx, test_idx) in enumerate(cv.split(len(X_full))):

and split() does range(train_start, train_end) on row positions. On a 50-stock panel:

Setting	Intended	Actual
train_size = 504	~2 years	~10 trading days
gap = 10	10 trading days of purge	less than one cross-section

Measured on 600 dates × 50 assets:

row-based  (before)   train  6 days / test 2 days / SAME-DATE OVERLAP 1
                      every fold shared at least one date
date-based (after)    train 252 days (2020-01-01..2020-12-17)
                      test   63 days (2021-01-01..2021-03-30) / overlap 0

Training and test therefore shared real trading days. For cross-sectional equity prediction that is the worst case: market state, sector rotation and volatility are all date-level, so the model was being evaluated on "a stock I have not seen on a day I have seen" rather than on genuine out-of-sample time. test_ic / test_icir from main.py ml train and POST /api/ml/train were close to in-sample numbers.

Fix

• TimeSeriesCV._folds() holds the fold arithmetic once.
• split() maps it onto rows, unchanged in behaviour.
• New split_by_dates() maps the same arithmetic onto the unique date axis and returns row indices, so callers keep the same shape.
• _prepare_dataset now also returns the per-row date. It has to: row counts per date vary when NaN assets are dropped, so the caller cannot reconstruct the mapping.

A test fixture had to grow

sample_factors used 300 dates against a default train_window of 504. That only "worked" because rows were counted as samples. It is now 800 dates — what the configured window actually requires. Sizing the window down to fit the fixture would have hidden the same class of mistake.

Tests — all three checked against the old behaviour

• test_train_and_test_are_disjoint_on_dates
• test_train_size_is_counted_in_days
• test_train_splits_on_dates_not_rows

The third exists because the first two exercise split_by_dates directly and would still pass if train() reverted to split(len(X_full)). It fails when it does — verified by reverting the call and watching it fail.

1318 passed, 1 skipped.

Impact

Any ML signal produced by train() used date-overlapping folds, so prior test_ic / test_icir figures need re-running. Those results should be marked affected rather than deleted, per the contract's own remediation procedure.

🤖 Generated with Claude Code

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 267763ad-890c-42c2-a8ab-fec1572e9c31

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}