Ship the generator that produces ch26's pinned prediction artifacts - #423
Open
stefan-jansen wants to merge 2 commits into
Open
Ship the generator that produces ch26's pinned prediction artifacts#423stefan-jansen wants to merge 2 commits into
stefan-jansen wants to merge 2 commits into
Conversation
PR #419 took ch26 green with the notebook half of fix/ch26-pinned-prediction-fixture. The half that *generates* the artifacts those notebooks pin never landed, and ch26 is green only because the artifacts are already committed to the test-data repo. The next fixture regeneration drops them and the job goes red for a reason no one will connect to this. This is that half, rebuilt onto current main rather than merged: the original branch is 14 commits behind and merging it would have reverted PR #420. Its notebook and overrides.yaml changes are already upstream, so the whole remaining delta is this one file, and it is purely additive - main's _copy_backtest_artifacts (from #420) and these four functions were a three-way conflict only because both sides inserted at the same point. Both are kept; they share no symbols and main() calls each. ensure_pinned_predictions and preflight_pinned_predictions are idempotent: they decide the destinations and check the layout before recreating any registry, and they project onto the columns the destination schema actually has, so a fixture written before a column was added stays usable instead of failing on it.
Preflight compared (symbol, timestamp) across the pinned prediction artifacts. Two streams can share every key and still carry different 'actual' values for it - a different label, or the same label from a different vintage - and the ch26 notebooks would score model error and realized return against different outcomes with every breadth and key check passing. Not reachable with today's pinned set: the identity assertion above pins all three us_equities_panel hashes to label fwd_ret_1d, and they agree row for row (verified against production, 0 nulls). It is checked because the pinned set is data, and the next hash added to it need not share the label. Verified the check is not vacuous: perturbing one artifact's 'actual' by a constant is rejected, and the key-only comparison it replaces accepted the same perturbation silently. Null targets inside the shared universe are rejected too - a null drops out of every comparison that reaches it rather than failing one.
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.
What this is
PR #419 took
ch26green with the notebook half offix/ch26-pinned-prediction-fixture. The half that generates the artifacts those notebooks pin never landed.ch26is green today only because the artifacts are already committed to the test-data repo — the next fixture regeneration drops them and the job goes red for a reason nobody will connect back to this.This is that missing half.
Why it is a rebuild rather than a merge
The original branch is 14 commits behind
mainand merging it would have reverted PR #420: a 2,731-line deletion against currentmain, taking out the eoa seal gate,input_lineage, and the coverage bar.Its notebook changes and its
tests/overrides.yamlchanges are already upstream via #419 (verified — both are byte-identical tomain). So the entire remaining delta is one file, and it is purely additive: +406 lines, 0 deletions.main's_copy_backtest_artifacts(from #420) and these four functions were a three-way conflict only because both sides inserted at the same point in the file. Both are kept — they share no symbols, andmain()calls each.What it adds
ensure_pinned_predictions/preflight_pinned_predictions— give every pinned hash both a registry row and a materialized artifact, idempotently. Preflight decides every destination and validates the layout before any registry is recreated, so a caller that skips it fails the same way rather than failing after the destination has been mutated._pinned_prediction_universe— one universe shared by every pinned artifact and by the fixture's own data. Reading it off whichever artifact sorted first gave 50 symbols meeting a 56-symbol fixture universe in two names, which would have left two assets per date and made every cross-sectional correlation ±1 by construction._copy_rows_onto_existing_schema— projects onto the columns the destination actually has, so a fixture written before a column was added stays usable instead of failing on it.Review finding, addressed in the second commit
roborev flagged that preflight compared
(symbol, timestamp)only, so two streams could share every key and still carry differentactualvalues for it — the notebooks would then score model error and realized return against different outcomes with every breadth and key check passing.Not reachable with today's pinned set (the identity assertion pins all three
us_equities_panelhashes tofwd_ret_1d, and they agree row for row against production, 0 nulls), but the pinned set is data and the next hash added need not share the label. The comparison now carries the outcome column, and null targets inside the shared universe are rejected.Verified not vacuous: perturbing one artifact's
actualby a constant is rejected; the key-only comparison it replaces accepted the same perturbation silently.Verification
ruff check/ruff formatclean; all pre-commit gates pass.preflight_pinned_predictions('us_equities_panel', ...)runs green against the production registry.