Skip to content

fix(factors): jump_amount_corr_20 had no 14:50 cutoff — a real look-ahead (correctness fix) - #103

Merged
StackOverFlow11 merged 9 commits into
mainfrom
fix/jump-amount-corr-1450-cutoff
Jul 27, 2026
Merged

fix(factors): jump_amount_corr_20 had no 14:50 cutoff — a real look-ahead (correctness fix)#103
StackOverFlow11 merged 9 commits into
mainfrom
fix/jump-amount-corr-1450-cutoff

Conversation

@StackOverFlow11

Copy link
Copy Markdown
Owner

fix: jump_amount_corr_20 had no 14:50 cutoff — a real look-ahead (correctness fix, standalone)

This is a correctness fix, deliberately NOT part of the factor-layer refactor (design amendment A3): it changes a published factor's values, and the authorization comes from the research side (the owner's standing 2026-07-19 mandate: "全日数据因子一律截断到 14:50……授权直接截断,不必逐因子再问" — this factor predates that mandate and is the only one that missed it).

The defect

jump_amount_corr_20 was the only one of the eleven minute factors with zero references to decision_time / prepare_visible_minute_bars (the others: 4–10). The old runner fed it the whole day's bars (end + 23:59:59). Under the exec_to_exec return basis, day d's entry anchor is the 14:51 bar's VWAP, while the factor's day-d value used d's 14:50–15:00 bars ⇒ its information set crossed its own entry anchor. Reproduced independently by two parties with different symbols/windows: perturbing post-cutoff bars moved 1,373/1,373 and 1,477/1,477 cells for this factor and 0 cells for all other eleven; the sharper cut (perturbing only bar_end >= 14:51) moved 1,363/1,363 — the signal saw the very minute it would have traded in, and the nine minutes after. Its published artifacts state spec.decision_cutoff="14:50:00" / return_basis="exec_to_exec" with verdict Watch, predictive axis PASS — the report described a check it did not perform.

What this PR does

  • Adds the 14:50 cutoff to the factor (same prepare_visible_minute_bars machinery as the other ten), plus a generic intraday-boundary guard covering all 11 minute factors + mmp_ew (coverage closure by directory walk, not a hand-written list).
  • Carries the correction as structured data (JSON top-level corrections, exactly one entry; CORRECTION_FIELD_MAX_CHARS=2000, over-long raises rather than crops), not as capped prose. Eval contract v1.0 → v1.1 (to_version must equal the spec's own version ⇒ spec.version alone discriminates which side of the correction an artifact is on).
  • Lifts the intentional loud block on this factor's exec evaluation (deny-list entry removed; the cross-PR interlock test test_decision_cutoff_visibility.py failed verbatim with the two required actions before that was done).
  • Layout fix (review HIGH): figures.py's definition band now sizes by the branch the dashboard actually renders — meta_y = 0.20 if block is not None else 0.10 means daily capacity 5 lines / intraday capacity 4 lines (measured on the real GridSpec geometry), but the old single DEFINITION_MAX_LINES=5 was measured on the daily branch while the exec path hands the renderer an is_intraday=True spec. Result was 9/11 factors' exec dashboards overlapping. Now DEFINITION_MAX_LINES_DAILY=5 / _INTRADAY=4, and test_definition_band_layout.py is parametrized over both forms — the guard previously only fed the close form, i.e. it protected exactly the half that was not broken. Mutation evidence: INTRADAY back to 5 → 11 tests red; revert → green.
  • Census guard fix (review MEDIUM): test_the_correction_carrier_is_never_truncated_on_disk was vacuous — MUST_NEVER_TRUNCATE={"corrections"} never intersected the leaf paths _leaves() emits (corrections.date, …). Now prefix-matched on corrections., with mutation evidence: truncating corrections[0].superseded turns it red and names the right field; the old exact-set logic stays green on the same tampered corpus.

Restated results (run, not inferred)

Verdicts on both bases are Watch/Watch, unchanged (exec: IC −0.030840 → −0.030539, N_eff 1162 → 1178). Discipline: the unchanged verdict is a measured outcome of the full re-run, not inferred from the pre/post-cutoff pearson r=0.9993. And the old artifacts are not retroactively validated: every emitted cell was contaminated; the aggregate rank-IC moved little only because the polluted bars are ~4.5% of the 20-day pooled window and rank IC caps per day — that is dilution, not harmlessness.

Two review findings the review itself retracted (recorded here so the record is not distorted)

  1. verdict.reasons has no length cap. It goes through sanitize_text, not sanitize_payload; on-disk artifacts contain an untruncated 1,263-char note and 401-char reasons[]. The earlier "16 characters from the 200-char cap" alarm was a false alarm produced by generalizing from a single sample. What stands: six payload fields ARE capped (census hits 44/44 · 44/44 · 44/44 · 44/44 · 22/44 · 20/44); the only genuinely critical one is coverage_bias_bad_vwap (158/200, headroom 42).
  2. The PNG overlap is both a pre-existing defect and a this-PR regression — about different factors. analytics/eval/figures.py had exactly 1 commit in git ⇒ the renderer was always like this (frozen eval_valley_price_quantile_exec_no_book_dashboard.png shows three layers of text stacked). But jump's description growing 323 → 702 chars (3 → 5 lines) was a regression this PR once introduced, then pulled back out via the structured carrier (description now 352 chars). Correct phrasing: the renderer always had the defect; this PR briefly dragged jump into it and then dragged it back out.

Expected downstream signals (pre-registered, not regressions)

  • hand_anchors_d2.json will report all_ok_frozen14: False (70 rows, 5 mismatches, all and only jump: hand-computed is now cut off vs frozen panels_d2 which is not). This is the correct signal; the frozen panels are kept as-is, and artifacts/refactor_baseline/pr_c_cutoff_fix/ holds the "old engine + cut input" reference for the refactor-only comparison.
  • The 9/11 exec-dashboard overlap for the other factors is a pre-existing renderer defect; their PNGs re-render at the C5 full rerun.

Review & gates

  • Branch head 497e50d: cutoff + structured carrier + contract v1.1 (previously reviewed) + merge of main (D4c era, clean) + the two blocker fixes + NIT wording. The post-merge commits were adversarially reviewed by a separate executor: merge face (both sides' semantics in binding.py preserved), independent geometry re-measurement (capacity 5/4 confirmed to the pixel), both mutation evidences re-run, NIT numbers independently verified, no smuggled changes — APPROVE.
  • Gates rerun independently by the lead: pytest 2389 passed / 5 skipped (4 census corpus tests skip in worktrees with no artifacts corpus — verified 4/4 PASS against the real corpus; 1 gitignored-baseline-bytes skip), ruff clean, run-phase0 demo anchor ic 0.9600 / annual 0.8408, validate-config 31/31, secret scan (three-dot diff) 0, exec_baseline_freeze --verify 77/77 @ 45c14aa.
  • Jump's 4 artifact cells regenerated from the branch via a git archive export (cache-only, stk_mins_live_calls=0, zero cache writes): all 9 JSON/MD byte-identical, PNGs unchanged — correct and expected, since jump's 3-line description fits under both the old and new caps (it was one of the two factors never overlapping). Exec dashboards visually inspected: definition band / metadata / minute-block rows clean.

CORRECTNESS FIX, not a refactor step. `compute_jump_amount_corr` was the one
minute factor of the eleven with NO intraday PIT truncation at all: its
`decision_time` / `prepare_visible_minute_bars` reference count was 0 while the
other ten were 4-10. It was built before the 2026-07-19 standing authorization
to truncate every full-day minute factor at 14:50 and never got it applied.

Under the 14:51-VWAP exec_to_exec basis (PR #79) this is a STRICT LOOKAHEAD: the
value at d pooled that day's 14:50-15:00 bars, so the information set crossed its
own entry anchor by ten minutes, in the most volume-concentrated window of the
A-share session. The published exec artifacts declare
`spec.decision_cutoff="14:50:00"` -- a check the factor did not perform.

The fix uses the SAME machinery as its siblings (`visible_minute_frame`, shared
with ideal-amplitude / amp-anomaly / amp-cut). No new mechanism, and the
correlation math is untouched -- only the set of bars fed into it. The two row
filters commute, so the surviving set does not depend on their order.

Blast radius (measured, not assumed):
  * old eval runner + `qt.panel_freeze`  -> defective (the published artifacts);
  * D4 materializer DECISION view        -> ALREADY correct: it applies
    `minute_decision_cutoff`, the same predicate, before the binding, so the
    double filter is idempotent and those values are bit-unchanged;
  * D4 materializer CLOSE view           -> was defective, now consistent.
`binding.py` already documented "the compute functions run with their DEFAULT
decision_time as a redundant-but-consistent internal cutoff" -- true for ten of
eleven. It is now true for all eleven.

Spec version 1.0 -> 1.1 and the description carries the correction, so every
report / dashboard self-identifies which definition produced it.

NET-NEW GUARD (`tests/test_minute_decision_cutoff_leakage.py`, 40 tests): every
existing leakage test guards the CROSS-DAY boundary, and their fixtures are
five-bar morning sessions with no 14:50 in them -- the intraday boundary was
untested everywhere, which is why nothing went red. The new guard covers ALL
eleven minute Factor subclasses (discovered by walking the package, so a new
factor file cannot be added without being covered) plus the `mmp_ew` intraday
aggregate feature, under two knives: every bar the visibility rule excludes, and
the sharper `bar_end >= 14:51` (the execution bar itself and the nine after it).
Each perturbation asserts BEFORE use that it moved every selected row and no
unselected row, and each factor must emit a finite value, so a green run cannot
come from an empty mutation or from NaN == NaN.

Mutation evidence (run, not claimed):
  * pre-fix, jump only: both knives RED -- `invisible` moved 400/400 finite
    values (max|diff| 0.453), `exec_and_after` moved the NaN mask itself; the
    other ten factors and mmp_ew were GREEN on both. Post-fix: 40 passed.
  * dropping one factor from the coverage dict -> the closure test goes RED.
  * making the perturbation a no-op -> 26 tests go RED (the preconditions are
    inside the invariance tests, so a vacuous mutation cannot pass quietly).

`qt/hand_anchors_d2.py` read FULL-day bars for this factor and said so in a
comment -- a hand check that faithfully mirrored the defect. It now truncates
like every sibling.
The D1 frozen panel `artifacts/refactor_baseline/panels/jump_amount_corr_20.parquet`
was produced by the runner loader, so it carries the untruncated definition. It
is left BYTE-UNTOUCHED -- it is the faithful record of what was published, and
D2's bit-for-bit reconciliation still points at it. What D5's panel leg needs is
a SECOND, truncated reference for this one factor.

`--only` (default None = all 14, byte-identical) makes the existing freeze tool
produce it, instead of writing a second freeze implementation for one factor or
re-reading the 279M-row minute cache for the thirteen that did not change. The
shared data plane is still built from the FULL book, so a filtered run computes
on the same daily panel as a full run: the flag decides what is FROZEN, never
what is LOADED. Selection is validated BEFORE the expensive data plane, and an
unknown id is a readable error -- "froze 0 panels" reported as success is the
empty-reconciliation shape this repo has already recorded once.

Why "old engine + truncated input" is a clean refactor-only control: the fix is a
pure INPUT truncation; every line from `amp` to Pearson's closed form is
unchanged. Re-running the SAME pre-D5 runner path therefore isolates exactly one
variable, so D5 can still attribute any residual difference to load geometry /
saturation depth / anchor truncation rather than to a definition change.

`docs/factors/pr_c_cutoff_fix_reference_panel.md` carries the provenance: what
was wrong, which value paths were affected (the D4 decision view was NOT -- it
already applies the same predicate), which panel to use where, the exact rerun
command, the old-vs-new evaluation table, and the explicit warning not to treat
the two panels as two measurements of one quantity. The D1 manifest doc gets a
pointer at the jump row; its numbers are unchanged.
…it is fixed

D5b added a deliberate loud block: `subject_view` derives a factor's information
set from `NOT_DECISION_CUTOFF_SAFE`, `jump_amount_corr_20` was on it, so
(close, exec_to_exec) was refused and its exec evaluation raised. That was
correct -- before the truncation fix there was no honest exec artifact of it, and
a block beats a false `view=decision` claim.

This branch's first commit is the release condition, so the entry is removed HERE
rather than left for a later PR: the intermediate state "factor fixed, evaluation
still blocked" never exists on main.

INDEPENDENT CORROBORATION, and it is stronger than this branch's own assertion.
`tests/test_decision_cutoff_visibility.py` was written by another author with its
own fixture (12 names x 40 days x 240 bars). Run on the merged tree BEFORE
removing the entry, it failed with exactly the two instructions carried out here:

    jump_amount_corr_20 no longer depends on post-14:50:00 bars. If that was
    deliberate, this factor's PUBLISHED values changed: drop it from
    factors.compute.minute.binding.NOT_DECISION_CUTOFF_SAFE, and re-state its
    verdict rather than letting the artifacts drift silently.

The factor now sits in that file's clean parametrization (all ten bars-bound
minute factors, 0 cells moved), so a regression re-introducing the leak is red
again.

The two follow-on sites are updated to statements that are still TRUE rather than
weakened:

  * `test_the_known_exception_is_still_exactly_one_factor_and_still_leaks` ->
    `test_the_deny_list_is_empty_and_that_emptiness_is_a_measurement`. It asserts
    BOTH that the list is empty AND that the measured set equals the bound set:
    "empty" alone would also pass for a list nobody ever measured.
  * `test_a_factor_that_is_not_cutoff_safe_cannot_get_an_exec_identity` keeps its
    property by INJECTING an offender (monkeypatched deny list) instead of naming
    a factor that is no longer broken. Deleting the test would drop the guard on
    the day the next offender appears; leaving the old name in place would be the
    stale-wording failure this repo keeps re-learning.
  * the D5 runner-difference catalogue entry becomes "released, in this PR", and
    points the C5 panel leg at the truncated reference panel.

The D5b review NIT (the block fires late in the runner, after the expensive
per-symbol minute aggregation and after both close_to_close reports are written)
disappears with the entry and needs no separate handling.

STILL OPEN, recorded rather than papered over: two places now measure the same
property -- that file and this branch's `test_minute_decision_cutoff_leakage.py`
(11 Factor subclasses + mmp_ew, two knives, coverage closure). Both being true is
useful today; author-once consolidation is a follow-up, and the leakage file's
coverage is the strict superset.
…ted verdicts

The bulk panel and its machine-readable manifest are gitignored, so the
authoritative numbers live in the doc (same reason the D1 manifest doc carries
its full table). Recorded: producing SHA, canonical/file sha256, row and NaN
counts against the D1 baseline (same grid, 11 more honest NaNs), and the fact
that the panel was frozen twice -- once on the uncommitted tree, once on the
committed one -- with byte-identical hashes.

The first freeze is not kept: its producing_git_sha pointed at the branch base,
which does NOT contain the fix. A provenance line that names a tree the artifact
did not come from is worse than no line, so it was re-run rather than annotated.
SELF-INFLICTED, CAUGHT BY REVIEW. The first version of this branch wrote the
"these values supersede previously published ones" disclosure as prose appended
to `FactorSpec.description`, and the report claimed it reached the Markdown, the
JSON and the dashboard. Two of those three were false:

  * JSON  -- `sanitize_payload` caps every exported string at MAX_VALUE_CHARS
    (200) and appends `...[truncated]`. The disclosure started at character 353,
    so the MACHINE-READABLE copy -- the one a summary layer opens -- contained
    none of it.
  * PNG   -- the longer description grew the FACTOR DEFINITION band from 3 lines
    to 5, and the 5th line printed on the same baseline as the metadata row.

A report that fails to state the thing about its own provenance that it must
state is the same shape as the factor defect that produced the correction. It
landing exactly where the report claimed success is the part worth remembering.

THE CAP IS GENERIC, NOT A QUIRK OF ONE PATH. Measured over the 44 shipped eval
JSONs: 44/44 carry a truncated `spec.description`. So `description` is not a
usable carrier for anything load-bearing, for any factor.

CONTRACT v1.1: the correction becomes `FactorSpec.corrections`, a tuple of
frozen `FactorCorrection` (from_version / to_version / date / defect / effect /
superseded). Every field is required, and OVER-LENGTH RAISES at construction
rather than trimming -- a carrier that can silently lose its content would
rebuild the defect it exists to fix. `to_version` must equal the spec's own
version, so `spec.version` alone tells a reader which side of the correction a
stored artifact is on. Exported as a top-level `corrections` key via
`corrections_record`, outside the capped path (still redacted). The Markdown
provenance row and the dashboard marker derive from the SAME tuple, so the three
surfaces cannot disagree. `description` goes back to being a definition.

The census guard immediately earned itself: with corrections still a spec field,
`vars(spec)` put a SECOND copy into the JSON -- a dataclass repr, cut at 200.
Two carriers for one fact, one of them broken. `corrections` is now excluded
from the spec dump.

DASHBOARD OVERLAP -- fixed generically, and it is NOT this branch's regression.
`_definition_band` anchors the description at y=0.62 and the metadata at
y=0.20/0.10 with nothing in between, so a long description is simply drawn over
the metadata. `analytics/eval/figures.py` has ONE commit in its history: every
dashboard ever produced was drawn this way, including the 22 frozen ones.
Measured on the real geometry, 7 of the 11 minute factors overlap
(valley_price_quantile_20 by 296 px, 25 wrapped lines).

`DEFINITION_MAX_LINES = 5` is MEASURED, not chosen: the gap falls 15 px per line
and goes negative at six (1..5 give +63.8/+48.8/+33.8/+18.8/+3.8). Five keeps
every factor that fits today rendering in FULL -- including the two 5-line ones a
tidier bound of 4 would have needlessly clipped -- and gives the seven that
overflow an explicit elision marker pointing at the Markdown, which carries the
description whole. Elision is disclosed; overprinting loses both texts silently.
No factor's description text is touched: that would be the scope explosion.

GUARDS (three files, none of them a substring match on rendered text -- D5b just
lost a guard to exactly that):

  * tests/test_factor_correction_carrier.py (15) -- round-trips the DECLARED
    object through the real `report_to_dict` + json dumps/loads and asserts
    EQUALITY, plus pins that the fields really exceed the cap, so "it survived"
    is only provable while the carrier is outside the capped path. It also
    demonstrates the original mistake directly: prose past the cap inside
    `description` is shown being cut on export.
  * tests/test_definition_band_layout.py (25) -- renders the band on the REAL
    dashboard figsize/GridSpec and compares matplotlib window extents. Geometric,
    not visual: the failure being guarded is pixels landing on pixels.
  * tests/test_report_json_truncation_census.py (4) -- reads the JSON FROM DISK
    and asserts the WHOLE truncated-field set, not the one field just fixed
    (#82: a guard that scans only what you already fixed can only confirm what
    you already know).

Mutation evidence, all run:
  * DEFINITION_MAX_LINES -> 999: 7 factors go red with measured overruns
    (11.2 / 56.2 / 71.2 / 128.7->191.2 / 206.2 / 296.2 px).
  * corrections=() on the shipped factor: 3 tests red across two files.
  * the census guard failed on the real corpus at `spec.corrections` before the
    double-carrier fix, and passes after regenerating.

INHERITED, RECORDED, NOT FIXED HERE: six fields are truncated across the corpus
(spec.description 44/44; monotonicity_spearman_by_date_ci_note 44/44;
monotonicity_reversed_status 44/44; multiple_testing_note 44/44; vif_status
22/44; forward_return_source 20/44). All six are explanatory caveats cut
mid-sentence -- monotonicity_reversed_status stops at "This evaluator scores ONE
cell and cannot ...", i.e. the half stating what it CANNOT do is the half that is
gone. They predate this branch (the frozen #79 baseline has them), so the set is
RECORDED and asserted exactly rather than fixed under a correctness PR: a seventh
cannot join quietly, and fixing one forces the record to be updated.

Ruled out while measuring: verdict reasons and section notes go through
`sanitize_text` only and are NOT capped -- proved on disk by a 401-char reason
and a 1263-char note surviving whole. The genuine near-miss is
`coverage_bias_bad_vwap` at 158 of 200.
…oard actually renders

DEFINITION_MAX_LINES=5 was measured on the daily branch (metadata row at
axes y=0.10), but the exec-basis dashboard of every shipped minute factor
renders intraday_spec_variant (is_intraday=True), whose metadata row sits
at 0.20 with the minute block below it. Measured on the real geometry
(figsize 15x21.5, the _render GridSpec), the intraday branch holds only
4 wrapped lines (+1.5 px of slack; 5 lines overprints by -13.5 px), so
9 of the 11 exec-form dashboards drew the description over the metadata.

Split the constant into DEFINITION_MAX_LINES_DAILY=5 /
DEFINITION_MAX_LINES_INTRADAY=4, keyed on spec.is_intraday via
definition_max_lines(), and extend the layout guard to BOTH forms: the
geometric no-overlap assertion and the line-budget assertion now run
against the close spec and the exec spec derived by the same
intraday_spec_variant the exec path uses (ExecBasisParams sourced from
IntradayCfg defaults so the test tracks project conventions). A boundary
test pins the split in both directions on volume_peak_count_20, which
wraps to exactly 5 lines: verbatim in the close form, elided to 4 with a
marked tail in the exec form. Mutation evidence: setting the intraday
budget back to 5 turns 11 tests red; reverting restores green.

Also reconcile the numbers the docstrings stated (capacity 5 / 'seven
that genuinely overflow' vs '4-line slot' / 'Six of the eleven' /
'291 px'): both counts were right for different branches, which was the
bug. Post-fix state, all measured: capacity 5 daily / 4 intraday, 7 of
11 minute factors overflow the daily budget and 9 of 11 the intraday
one, and valley_price_quantile_20 unbounded overruns the metadata row by
296 px (daily) / 314 px (intraday).
…atch)

test_the_correction_carrier_is_never_truncated_on_disk could never fire:
_leaves() emits no container path for a list, so the census keys for the
corrections block are corrections.date / corrections.superseded / ...,
and intersecting them with the bare {'corrections'} set was empty by
construction. Mutation proof: truncating corrections[0].superseded in an
on-disk artifact left this test green while its two sibling tests went
red.

Match by prefix instead: a census field is offending when it equals
'corrections' or starts with 'corrections.'. Mutation evidence on a tmp
copy of the real 44-file corpus (real artifacts untouched): intact
corpus -> PASS; corrections[0].superseded truncated -> FAIL naming
['corrections.superseded']; the OLD exact-set logic on the same doctored
corpus -> still PASS, replicating the blind guard. All four census tests
also re-run green against the intact real corpus.
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.

1 participant