feat: factor refactor D4 — FactorService + decision-view materializer, saturation-correct pooled fills - #93
Merged
Conversation
The D3 tail-recompute engine HARD-REQUIRES spec.lookback_depth to size its incremental overlap window. Declare the transitive lookback depth (design §六.18: headline + nested baseline, NOT the headline window) on every closing factor plus the remaining registered daily factors, each derived from its own code constants so a re-parameterized window yields the matching depth: * book: value_ep/value_bp = 1, volatility_w = w + 1 (pct_change adds a day) * minute, no baseline: jump/amp_marginal/minute_ideal/intraday_amp_cut = lookback_days * minute, nested baseline: volume_peak_count/peak_interval_kurtosis/valley_*/ ridge_minute_return/peak_ridge_amount_ratio = lookback_days + VOLUME_PRV_BASELINE_DAYS (20 + 20 = 40) * daily: momentum/reversal/overnight = w + 1, liquidity = w, financial = 1 Parameterized test pins all 14 values and proves the declaration tracks the constructor param (a non-default window cannot silently keep the default depth). This is a pre-registration: changing a value is a definition-adjacent change.
… (D4) The single path that turns injected data-layer inputs into RAW factor values for a view over a date range (design §3.5). factors.service read-through will route every store miss here; nothing else computes factor values. * factors/view_lag.py — PURE availability-lag transforms (R18): daily prev-day shift with the field-level ``open`` same-day exception (the same-day set is DERIVED from the policy table, not a literal), the minute ``available_time <= d 14:50`` cutoff, and the ex-date BOOLEAN (``af(d) != af(d-1)`` — the adj_factor numeric never enters the frame, a type boundary per §1.3 note 4). * factors/materialize.py — the engine: reads ``requires`` to pick daily vs minute input, pulls from INJECTED providers (layering: factors never touches a feed/token/qt), applies the (source, view) lag, trims to EXACTLY the factor's ``lookback_depth`` trailing trading days before the emit window (the P8 single==batch correctness floor), computes, and emits. ``masked`` factors have ex-date rows NaN'd (the 14 closing factors are non-masked -> no-op). ``build_horizon_config`` wires CacheHorizonConfig from the LIVE cache config with NO default fallback (R5). ``make_recompute_fn`` yields a D3-compatible RecomputeFn. * factors/compute/minute/binding.py — binds each of 10 minute factors to its free ``compute_*`` (parameterized by the instance's lookback + name, all other definition constants at their single-source module defaults); valley_price_quantile is deferred (needs the daily panel too) with a readable error, never a silent mis-compute. Tests (network-free): view-lag transforms (open-only same-day, prev-day shift, cutoff, ex-date bool); R18 lag coverage (close[d] not visible / open[d] is — reverse teeth / daily_basic prev-day); minute cutoff no-leak; binding == direct compute_* (float-reorder tolerance, §五); ex-date masking fixture; recompute-fn parity; live-config wiring refuses a missing attr.
…tion/panel (D4) A thin read-through module (design §3.5, R25): two functions and a frozen DecisionPoint, not a behavior-class stack. Both cross_section and panel read-through the value store; a miss triggers the ONE materializer engine to fill the gap and append, then re-reads — there is no second implementation path. Contract (structural): only ``available_time <= decision`` data is computed (the materializer applies the lag); NEVER touches execution prices (raw VALUES only, forward returns are born at the alpha/eval boundary); returns RAW values; the view x return-basis pairing is enforced at CALL time via the D0/D1 require_legal_pairing (decision<->exec_to_exec / close<->close_to_close). The data access (store + providers) is INJECTED — factors never carries a feed, token, or qt import. Tests (network-free), the §3.5 P8 acceptance: * single-fill (repeated per-date cross_section) == batch-fill (one panel): momentum_20 is EXACTLY bit-identical (window-local, depth 21 > headline 20 — the warmup trim uses the transitive depth); volatility_20 and the NESTED volume_peak_count_20 (depth 40) match up to an attributable pandas-rolling float-reorder (<= 1e-12); * MUTATION: forcing the warmup below the factor window makes per-date fills under-warm while batch fills do not -> the stores diverge (committed test asserts the divergence; a halved-warmup engine edit turns both single==batch tests red, restored -> green); * panel[d] == cross_section(d) on the same store; read-through hit does not re-hit the provider; universe slicing; the pairing gate rejects the two illegal (view, basis) combinations; panel requires a uniform cutoff.
…ss (D4 R9) drop_missing + winsorize + neutralize + zscore orchestration moves from qt.pipeline._process_factors into factors.process.orchestrate, so the D5 unified eval runner (analytics layer, which may not import qt, red line #10) and the backtest path share ONE processing truth instead of copying it (the P3-6 drop_missing-across-columns lesson) or breaking layering. * factors/process/orchestrate.py — process_factor_panel (constructs the SAME ProcessingPipeline with the SAME arguments and transforms) + covariates_from_panel (the industry/market_cap extraction the retired inline logic did). Covariates are INJECTED by the caller (factors never fetches, red line #3); the store stays raw-only (processing is a consumer-side decision on the raw panel). * qt.pipeline._process_factors becomes a THIN delegate — same covariate extraction, same pipeline args, verbatim behavior. Behavior preserved: phase0 anchor ic 0.9600 / annual 0.8408 unchanged; the orchestrator matches a directly-constructed ProcessingPipeline byte-for-byte (no-neutralize + neutralize), never mutates its input, and extracts covariates identically to the retired inline logic.
…4 R20/§八) qt/factor_hotpath_smoke.py measures, on the REAL intraday cache (cache-only, stk_mins_live_calls=0), the wall-time gap between NAIVE per-factor loading (each materialize_range loads its own bars — the D4 service's current behavior) and SHARED loading (read + normalize + cutoff once, then every factor computed from the shared bars — the primitives-layer amortization §3.2 calls the load-bearing hot-path mechanism). The budget is measured, not claimed (§八), with a caliber caveat that it does NOT extrapolate to full-A (~6x the CSI500 read). A standalone dev tool (not in qt/cli, like qt.panel_freeze); run manually after symlinking the main checkout's artifacts (or --cache-root the absolute path). Measured (CSI500 sample, 40 symbols, window 2024-03-01..2024-04-05, 753,710 post-cutoff 1min bar rows, cache-only stk_mins_live_calls=0): 6 factors : NAIVE 8.90s (6 loads) / SHARED 4.36s (1 load) = 2.04x 10 factors: NAIVE 14.22s (10 loads) / SHARED 7.13s (1 load) = 2.00x Shared-read roughly halves the wall time on this sample — the direction of §八 (load amortizes across factors); the plateau at ~2x reflects the nested factors' heavy per-factor compute relative to the shared read on this short window.
…(D4 review HIGH) Valid-day-POOLED factors (ridge_minute_return / valley_ridge_vwap_ratio / peak_ridge_amount_ratio and their structural siblings) count VALID days in the trailing window (invalid days do not occupy a slot -> roll over a valid-day-indexed series), so the CALENDAR lookback depth of a lookback_days-valid-day pool is DATA-DEPENDENT and UNBOUNDED, and loading more history can re-classify boundary days. A fixed lookback_depth trim therefore truncated sparse-valid-day symbols' pools, making the stored value depend on LOAD GEOMETRY: a single-date fill and a batch fill diverged finite<->NaN (review repro ridge_minute_return_20 (2021-03-18, 000002.SZ): single=1.6976..., batch=NaN). This DISPROVES design §3.3 R4's "W_dep = sum of lookbacks" assumption (it treated valid-day slack as bounded; it is unbounded) — a design correction. Fix (red line #6: the value must be f(factor, params, view, data), never load geometry): * factors/compute/minute/binding.py: a CLOSED partition of the minute surface — VALID_DAY_POOLED_FACTORS (8, code-inspected by the rolling mechanism, NOT by observed divergence: only 3 diverged on the review's data, the other 5 are the "happens-to-be-clean" siblings, #82 lesson; valley_price_quantile declared though deferred so D5 cannot bind it with the defect) vs _BOUNDED_MINUTE_FACTORS (3); a minute factor in neither is a readable error. * factors/materialize.py: pooled factors load by SATURATION-EXPANSION — expand the load backward in chunks and recompute the emit-range values until they stop changing (saturated) or the provider returns no earlier bars (the real data start = the structural terminal); bounded factors keep the fixed trim. The chunk is generous vs baseline_days so one no-change implies saturation. NOT load-from-data-start: the real cache holds minute bars from 2015-01-05, so that would be an ~11-year load per pooled fill. * Single-fill and batch-fill now agree: the finite<->NaN divergence is gone (NaN mask load-geometry-free); the residual is only the pandas-accumulation float-reorder (JC1, <= 1e-12; on a short window both fills reach the data start and are bit-identical). D3 linkage (review point 5): make_recompute_fn delegates to materialize_range, so the tail-recompute path saturates pooled factors through the SAME code path — no second implementation. Tests: P8 expansion (the 3 divergent factors, single==batch on a sparse-valid window) + mutation (saturation OFF via monkeypatch -> divergence returns) + classification pinned (8 pooled / 3 bounded, unclassified raises). The hot-path smoke's naive path now uses a bounded per-factor load (never the pooled expansion) so it measures pure read amortization (2.0x, cache-only) without the saturation confound.
…floor (D4) CORRECTION to the previous commit's overclaim: it said the single/batch divergence "is gone". Its real reach was narrower — moderate sparsity was fixed, but a LONG no-bar gap (a suspension) still broke it, in two ways the review reproduced: 1. the value-stability fixed point false-positives: one expansion chunk landing ENTIRELY inside the gap adds no bars, so the emit values are unchanged and the loop terminated early; 2. an unchanged row count inside the gap was read as "provider exhausted = data start", so a mid-history gap masqueraded as the terminal. Result: ridge_minute_return 2021-11-04 single=3.2032634 vs batch=3.6674902 — a silent finite-vs-finite WRONG value, not float noise. The old chunk-size argument covered baseline re-classification only, not pool-count completeness: two failure modes, one guarded. This commit replaces the fixed point with a STRUCTURAL criterion (value-stability checking deleted entirely) and an explicit data floor: * LOCKING ARGUMENT (in the docstring): expanding backward can only change the classification of days within ``baseline_days`` trading days of the loaded window's start (the rolling baseline takes the most recent strictly-prior same-slot observations; at full depth earlier history cannot move it). Locking is POSITION-MONOTONE: drop the first ``baseline_days`` loaded trading days and every later day's classification is FINAL. * CRITERION: saturated iff the LOCKED sub-window holds >= ``lookback_days`` FINAL valid days at or before the earliest emit date — the pool only takes the most recent ``lookback_days`` valid days, so it can never reach the unlocked head. Sufficient, structural, deterministic, no iteration to a fixed point. Valid days are counted as the factor's own OUTPUT dates (a pooled factor emits one row per valid day), so a gap contributes none, the count stalls, and expansion continues — gap-immune by construction. * ``baseline_days`` is DERIVED per factor from its own parameter (peak family = VOLUME_PRV_BASELINE_DAYS; intraday_amp_cut = 0, within-day classification), not hardcoded. * DECLARED data floor: ``MinuteBarProvider.earliest_available()``; row-count inference is refused (a provider without it raises readably for a pooled factor). The chunk size is now only a search STEP, never a correctness input. Reach, stated precisely (no repeat of the overclaim): the short fixtures are bit-identical because both fills reach the declared floor; on long data the claim is exact NaN-mask agreement + finite values within 1e-12 (each fill terminates at its own saturated start, and by the criterion's sufficiency both agree with a common deeper start) — this reasoning is in the docstring. Tests: the review's counterexample committed (200-trading-day gap > one chunk, emit after the gap, pool spanning it) with a non-vacuity assertion; the earlier thin-day fixture kept (it covers the moderate-sparsity regime); providers declare their floor. MUTATION rc recorded in the test docstring: reverting to the value-stability fixed point FAILS it (rc=1, both fills emit all-NaN), restored passes (rc=0); dropping the locking offset does NOT fail on these fixtures and is recorded honestly as an unisolated correctness margin.
…ew HIGH)
The structural criterion iterated the symbols PRESENT IN THE OUTPUT, so a symbol
producing ZERO rows in the current load window (a long suspension, thin coverage)
had NO VETO over termination — and it is precisely the symbol a deeper load would
unlock. The consequence was worse than a wrong value: a single-date fill dropped
such a name from the cross-section entirely while a batch fill kept it. Measured
on the review's 2-symbol fixture (A with bars throughout; B = an ancient block +
a recent block with a long suspension between): volume_peak_count_20 single-fill
``600519.SH rows=0`` vs batch-fill ``rows=10 finite=6``; the criterion's
per-symbol counts were ``{'600000.SH': 67}`` with 600519 absent entirely. Silent
name-dropping is a sample-coverage bias the I5a red line forbids.
Fix: iterate the REQUESTED universe; an absent symbol counts 0 valid days, vetoes
termination, and the loop expands until it too saturates or the declared floor is
reached. After the fix both fills give ``rows=50`` with identical symbol sets.
COST, stated in the docstring rather than discovered later: any requested symbol
that cannot accumulate ``lookback_days`` valid days before ``emit_start`` (recent
listing, long suspension, coverage hole) drags the WHOLE universe's load to the
declared floor. That cost already existed for symbols with SOME output but too
few valid days; this only extends the same conservative behaviour to zero-output
symbols. A per-symbol saturation start is the D5 optimization — and NOT a pure
one: ``intraday_amp_cut`` z-scores across the loaded cross-section, so changing
per-symbol load depth changes its cross-section composition (noted for D5).
Also in this commit:
* LOCKING-OFFSET isolation test (the review's construction): a sparse recent
block of 33 every-other-day bar-days puts an UNLOCKED band (classifiable but
not yet final) inside the criterion's margin. With the offset the loop expands
and reproduces the whole-history truth; without it the criterion stops a chunk
early and returns a silently wrong value. This replaces last commit's honest
note that the offset was not isolated by the gap fixtures.
* LOW: the gap test's docstring said "95 trading days" while the fixture uses
``_GAP_TRADING_DAYS`` = 200 (stale after the geometry correction, #76 shape) —
now references the constant instead of restating a number.
* HARDENING: exhausting ``_MAX_SATURATION_CHUNKS`` silently returned an
UNSATURATED result; it now raises with the factor, the depth reached, the
symbol count and the declared floor (red line #9 — no silent degradation).
Unreachable in practice (500 chunks is >160 years), which is exactly why a
silent path there was indefensible.
MUTATION rc: restricting the criterion to output symbols FAILS the new test
(rc=1, "600519.SH silently dropped by the single-date fill"), restored passes
(rc=0); forcing ``baseline_days=0`` FAILS the locking-offset test (rc=1),
restored passes (rc=0).
… veto mutation (D4) Two review NITs, test-file only (materialize.py untouched). NIT 1 — the ``_MAX_SATURATION_CHUNKS`` exhaustion RuntimeError had no test. It is unreachable in practice (500 chunks is >160 years), which is exactly why it needs one: a raise that never executes is the next AttributeError's host. Recipe: cap the budget at 2 chunks, use the thin symbol that never accumulates lookback_days valid days, and declare a floor too distant to reach — so neither termination condition fires and the loop must fall through to the raise. A second test asserts the message is actionable (names the factor, the declared floor, and the chunk count) so an operator can tell WHICH factor stalled and how deep it looked. NIT 2 — the veto mutation test's disjunction ``syms_a != syms_b or _ZO_B not in syms_a`` is trivially true when BOTH stores are empty, so it could pass without exercising the mutation. Added non-vacuity guards: both stores must be non-empty and the batch fill must still carry the thin symbol before the contrast is read. MUTATION rc: reverting the raise to a silent return FAILS both new tests (rc=1, "DID NOT RAISE"); restored passes (rc=0).
StackOverFlow11
added a commit
that referenced
this pull request
Jul 25, 2026
docs: record refactor step D4 (PR #93)
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
1 participant
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.
Factor-layer refactor step D4 (design v3.2 §3.5/§9 D4 row): the FactorService (DecisionPoint + read-through
cross_section/panel), the decision-view materializer (the ONE vectorized engine that fills the D3 store), the processing-orchestration home infactors/process(R9), and the pre-registeredlookback_depthdeclarations for the 14 closing factors. Purely additive + behavior-preserving: no existing runner changes behavior in this PR (consumers switch in D6); the only shared-path edit is_process_factorsbecoming a thin, verbatim delegate.What (9 commits)
b70487alookback_depth pre-registration: all 14 closing factors declare transitive lookback depth (headline + nested baseline), derived from their own constants (parameterized, evidence in each spec), pinned by tests.ca1e892Decision-view materializer: readsregistry.requirements(), applies the full §1.3 policy-table decision-view lags (R18) — daily non-open columns ≤ d−1 with the field-levelopen[d]exception derived from the policy table (not re-listed), daily_basic ≤ d−1, finaann_date≤ d−1, minuteavailable_time ≤ 14:50; ex-date exposed to the engine as a boolean only (af(d)'s value never enters the decision frame — a type boundary, not a docstring rule);CacheHorizonConfigbuilt from live config (missing attrs raise; no 400/14 default fallback).c36b0feFactorService: frozenDecisionPoint;cross_section/panelboth read-through the D3 store, misses trigger the one engine (no second fill path); view×basis pairing legality reuses D0'srequire_legal_pairingat construction.44a8ee1Processing orchestration →factors/process(R9):drop_missing + neutralize + zscoreorchestration now lives infactors/process/orchestrate.py;qt/pipeline._process_factorsis a thin verbatim delegate (same covariate extraction, same pipeline args) — eval and backtest share one truth withoutanalytics → qt.e61f446Hot-path timing smoke (R20): real cache, cache-only (stk_mins_live_calls=0), CSI500 40-symbol sample: shared read ≈2.0x over naive per-factor reads. Honest scoping: smaller than §8's 900-symbol measurement; not extrapolated to full-A. No hardcoded machine paths (repo-relative default +--cache-root).29d0205→f1934d9→decbdc9Review-driven hardening (see below — the real story of this PR).The review story (4 rounds, 2 HIGHs found and fixed — and a design correction)
The P8 acceptance criterion (single-fill ≡ batch-fill, §3.5) caught three successive layers of real defects:
lookback_depth=40trim truncated sparse symbols' pools → single-fill vs batch-fill disagreed (finite↔NaN) at the same (date,symbol). This disproves design §3.3 R4's assumption that W_dep is a bounded sum of lookbacks — a genuine design correction, recorded here. Fix: saturation-expanding load for pooled factors (classification by code-inspected rolling mechanism, all 8 declared — not just the 3 observed to diverge; fix(config): sweep every remaining stale description of the intraday gate #82 lesson).baseline_daystrading days; everything after is FINAL because rolling baselines use the most recent strictly-prior same-slot observations) + count the factor's own output dates (one row per valid day ⇒ gap-immune) ≥lookback_daysbefore the emit range; plus a provider-declared earliest-available floor (earliest_available; nbars inference deleted; mid-gap can never be mistaken for data start). The prior commit's "divergence is gone" overclaim is explicitly corrected in6e9486d's message.f1934d9): iterate the requested symbol list (absent ⇒ count 0 ⇒ veto ⇒ keep expanding to the floor). The reviewer also constructed a fixture proving the locking offset is load-bearing (offset ON = 416.0 = full-history truth; offset removed = 438.0 silent wrong value) — both fixtures are now committed tests with source-edit mutation witnesses (rc=1/rc=0).decbdc9.Honest costs, disclosed in source docstrings: any symbol that cannot accumulate
lookback_daysvalid days before the emit start drags the whole universe's load to the declared floor (this cost already existed for insufficient-output symbols; the fix extends the same conservatism to zero-output ones). Saturation currently uses one sharedload_startper universe._MAX_SATURATION_CHUNKSexhaustion now raises loudly (red line #9) instead of silently returning an unsaturated value.Acceptance (dual-track)
Adversarial review: 4 rounds as above; final verdict APPROVE (0 CRIT/HIGH/MEDIUM), all mutations independently re-run by the reviewer from its own scripts. Earlier rounds also verified: behavior preservation of
_process_factors(read line-by-line +assert_frame_equalagainst direct construction), single-engine property, R18 lag coverage with two-directional teeth (perturb close[d]/post-14:50 bars ⇒ bit-identical; perturb open[d]/pre-cutoff bars ⇒ changes), pairing enforcement, 8/3 pooled/bounded classification (per-mechanism code inspection), layering (no qt/analytics imports in factors/), hot-path honesty.Lead independent gates: pytest 2098 passed, rc=0 (dot-count 2098 = 2031 baseline + 67 new; FAILED/ERROR 0) · ruff clean · phase0 anchor
ic 0.9600 / annual 0.8408unchanged · validate-config 31/31 · provisional D2 anchor green · secret scan ongit diff main...HEAD: 0 · worktree clean, no artifacts symlink residue.Disclosures & judgment calls
valley_price_quantileminute binding deferred (needs the daily close panel; loud NotImplementedError) but already declared pooled — D5 binds it inheriting saturation; do not bypass with a fixed trim.D5/D6 notes (reviewer's final list)
CACHE_MINUTE_DATA_START=2015-01-05, same for all symbols); D5 should derive per-symbol floors from the minute coverage ledger. Risk is asymmetric: a floor later than real data silently truncates; earlier only wastes loads. A floor after emit_start currently returns an empty series silently — same item.intraday_amp_cut's value is a cross-sectional z-score over the loaded panel, so per-symbol depths change its composition (definition-adjacent; separate decision).No CLAUDE.md/AGENTS.md changes here (docs PR follows). Frozen baseline
artifacts/refactor_baseline/untouched.