refactor(qt): source phase0/phase2 factor values from the factor service - #118
Merged
Conversation
The daily runners got their factor values from `factor.compute(panel)` in the runner, which is the second factor-sourcing path the refactor exists to retire (design decision 3). phase0 and phase2_baseline now read them from `factors.service` -- the store read-through onto the one materializer engine. Close view x close-to-close basis (the other legal pairing), under which the materializer applies no availability lag: a change of PATH, not of VALUES. Measured on a real phase2 run captured before and after the switch: 16,388 cells, max|diff| 0.0, NaN mask identical, and every headline metric identical at full precision (ic_mean, ic_ir, annual_return, max_drawdown, volatility, sharpe, avg_turnover, cost_drag, per-factor coverage, combo IC). The phase0 anchor (0.9600 / 0.8408) is unchanged. A SYNTHETIC data source is refused a durable store, structurally. The store key is (factor_id, params, code, view) plus a fingerprint over the PIT/schema modules; nothing in it says WHICH DATA produced the value. For demo data that is false and demonstrably so: `demo_feed` builds each price path from `arange(n)` counted off `data.start`, so the same (date, symbol) changes value when the window start changes. A value whose identity the key cannot capture is not storable, so the answer is refusal rather than a wider key -- a synthetic source gets an ephemeral per-run store, and `factor_store_root` raises rather than hand one a persistent root. Cost: a demo run recomputes every time, which is exactly what it did before. Already live, not hypothetical: `config/example.yaml`'s demo universe is spelled with real A-share tickers over real 2024 dates, and `test_provisional_anchor_d2` runs it against `artifacts/`. The demo run and the real phase2 run derive the IDENTICAL key (`7816ca515a5b9a1a__ca4d351df0bf40f6__close.parquet`, measured); they miss each other today only because SSE50 is Shanghai-only while the demo tickers are Shenzhen -- CSI300, which D6b's runners use over 2024, has 000001.SZ. The persistent root is derived from `output.root_dir` rather than configured separately, so a run that redirects its outputs takes its factor store with it (the 16 test call sites of run_phase0 did not, before this). The service answers a (date, symbol) question and returns a row for every cell it was asked about, carrying NaN where the factor has no value (D4c's fill footprint). An index universe's symbol list is the union of historical constituents, so that grid is larger than the loaded panel (measured: +2.2% for CSI300, +4.0% for CSI500). Every extra row is all-NaN so no value changes, but `per_factor[...]["coverage"]` counts rows -- so the result is reindexed back to the panel's own grid and the number of rows that drops is logged (red line #8: a published diagnostic must not move because the values arrived by another route). The phase2 panel is exactly dense, so that class is empty on the real run; the sparse geometry is exercised by unit test instead. `_compute_factor_panel` keeps its signature and body for the runners D6a does not migrate (oos_stability, subset_validation); the migrated pair calls the new `_serve_factor_panel`. Two names rather than one function with an optional store, so "which runners are still on the old path" is a grep and the old path is never a silent default. An earlier draft of this change altered the shared signature under those two runners and the whole suite stayed green -- they need real tushare data, so nothing in the suite executes their call. A guard now parses their call sites and binds them against the live signature. `DailyEvalPanelProvider` moves into the new module and `qt.factor_eval_providers` re-exports it (identity pinned by test): the daily runners need it too, and that module imports `qt.pipeline`, so `qt.pipeline` cannot import back from it.
Five items D6a found and did not do, each with the reason and the step that owns it. A deferral nobody recorded is indistinguishable from a defect nobody noticed. Four are deferrals: the provenance dimension on the store key (a real-vintage question, pre-existing in D3, orthogonal to routing runners through the service); the `isinstance` enrichment dispatch that red line #5 forbids while the `requires` declaration it should read already exists and `registry.requirements()` has zero production callers; three runners that do not pass the shared cache to three enrichments, so `fina_indicator` / `daily_basic` / `index_member_all` are fetched live every run; and `phase2_real_baseline.yaml` having no cache block. Each is deferred for the same reason: D6a's value is that it is a path switch provable to be a no-op. Any of these would have changed something else at the same time, so a moved number would have had more than one candidate explanation. The fifth is an observation, not a deferral: the phase2 baseline's archived headline numbers no longer reproduce on `main`. Captured before touching anything -- annual_return -10.19% archived vs -9.41% measured, sharpe -0.5703 vs -0.5168, turnover 1.0818 vs 1.0727 -- while the factor IC is unchanged, which points at the neutralization covariates rather than the panel or the factor. The consequence for a reviewer is that the archived numbers are not a valid baseline for a change made today; compare a capture taken on the tree being changed, immediately before changing it.
…ndent Review LOW-2. The store key is (factor_id, params, code, view) and has no universe dimension -- deliberately, and it cannot have one: a PIT universe over a date range is a time-varying SET, not a value a key dimension could hold (design revision A2). That is sound only while a stored value is universe-independent. The minute plane learned this the hard way and it was measured: `intraday_amp_cut` ran its cross-sectional z-score during materialization, so its stored value WAS a function of the loaded universe while the key said otherwise -- fill with 12 names, read asking for 24, and 24 wrong cells were served with zero recompute and zero error. D4c fixed it by storing the per-symbol intermediate and combining at read-assembly. THE DAILY PLANE NEVER INHERITED THAT FIX, because until D6a no daily factor value was ever stored. D6a is the step that puts them in the shared store, so the property that keeps the daily plane sound becomes load-bearing here. It held before by accident of what happened to be written; from here it holds on purpose. Adding the guard in the same step that makes it load-bearing is new protection, not a behaviour change -- the path switch stays provably no-op. The census is BEHAVIOURAL: compute every registered daily factor over a 5-name universe and a 3-name subset, require the shared symbols bit-identical. A source scan for `groupby(level="date")` / `.rank(` would be both too strict (`groupby(level="symbol").rank()` is a per-symbol time-series rank, a false positive) and too loose. Mutation evidence for the second half: a cross-sectional demean written `x.unstack().sub(x.unstack().mean(axis=1), axis=0).stack()` turns the census RED while containing ZERO of those tokens (measured by the harness, which greps the mutated file and reports the count). The enumeration is DERIVED from the registry's dispatch tables, not a hand-kept list -- a newly registered daily factor lands in the census without anyone remembering to add it, which is the case the guard exists for. Two anti-vacuity tests back it: an enumeration returning nothing fails (mutation: red), and a fixture short enough to make every value NaN fails (mutation: red). A red here is STOP-AND-REPORT. The fix is D4c's, not an exemption list -- an exemption would rebuild exactly the silent-wrong-value failure D4c closed, and the assertion message says so. Recorded because it cost a wrong conclusion for a minute: the first fixture keyed each symbol's synthetic prices to its position in the PASSED list, so dropping two names shifted the survivors' own data and all ten daily factors "failed". The census caught it immediately -- which is the only reason we know it has power -- but a probe reporting that everything is broken is usually the broken thing.
… figures
Review LOW-1 and NIT.
LOW-1 -- phase2's neutralization covariates are fetched LIVE on this path (the
runner does not pass the shared cache; D6a-3), so the before/after captures sit
minutes apart against a revisable upstream. That puts a condition on the
comparison which neither side had written down:
the reconciliation is valid only if those covariates were not revised
between the two captures.
Nothing of the sort happened -- the factor panel matched cell-for-cell and every
headline metric matched at full precision, which is itself strong evidence the
covariates were stable across the window. But that is evidence collected after
the fact, not a control designed in, and an upstream revision could equally have
MASKED a real difference as invented one. Recorded in D6a-5 and in the
reconciliation script's own docstring, with the instruction a future run needs:
on any small movement, re-capture the legacy side and check legacy-vs-legacy
before attributing anything to the code.
NIT -- the "+2.2% CSI300 / +4.0% CSI500" grid-inflation figures in
`factor_values`' docstring cannot be reproduced from anything this change emits.
They are scouting-phase measurements off two local gitignored daily panels; the
docstring now names the panels, their row/date/symbol counts, the arithmetic
(`dates * symbols / rows - 1`), and says plainly that they size the effect
rather than being evidence about this code. It also states that the phase2 panel
actually reconciled on is exactly dense (68 x 241 = 16,388), so its footprint
count is zero and the non-zero case is covered by unit test instead.
Also registers D6a-0: the daily universe-independence invariant now guarded by
tests/test_daily_factor_universe_independence.py, and why it is a different
question from the provenance dimension in D6a-1 (whether a value depends on WHO
ELSE was loaded, vs telling two real vintages apart).
…and pin
the reduction it was mistaken for
Review S1. `test_the_reindex_preserves_every_value_the_service_served` never
tested that: both sides came from the same already-reindexed `factor_values`
call, so for that property the comparison reduced to `frame == frame`. What it
could actually catch was a difference between the two STORES, which is worth
keeping -- a factor value must be a function of (factor, panel, universe) alone,
and that is the assumption the read-through rests on. Renamed to
`test_the_served_values_do_not_depend_on_which_store_they_were_filled_into`,
tightened to `check_exact=True`. Mutation: making a served value depend on the
store root turns it red.
RENAMING ALONE WOULD HAVE LEFT A REAL GAP, so the assertion the old name
promised is now added where it belongs -- inside the sparse-panel test, next to
the reduction it constrains. Checking the file rather than trusting the earlier
summary of it:
* the sparse test asserted the INDEX only (`index.equals`);
* the value-vs-direct-compute test runs on a DENSE panel, where the reduction
is the identity.
So nothing could see a reduction that lands the correct index carrying the wrong
rows. Mutation, run against three tests to show the coverage is new and not a
duplicate: replacing the label-based reindex with a positional one
(`served.head(len(panel.index)).set_axis(panel.index)`) leaves the dense value
test GREEN and the index+count assertions GREEN, and turns only the added
assertion RED.
Worth recording how the gap survived: the earlier self-review reported "the
reindex path is covered" -- true of the path, not of value preservation -- and
that report was then relied on when deciding this was a naming issue only. A
test whose name overstates it does not just fail to cover; it actively suppresses
the coverage someone would otherwise add, and the first person it misleads is
whoever wrote it. The new name states the narrower thing it really pins, and its
docstring points at where the wider claim is now enforced.
…ert the grid the end-to-end test was named for Review LOW-3 and NIT-2. LOW-3 -- the previous guard named its two files. The reviewer showed what that costs: add a third caller in a file the list does not name, change the shared signature, and dutifully update everything the list DOES name, and all 21 tests stayed green. Replaced by `tests/test_legacy_factor_panel_callers.py`, which derives the caller set from `git ls-files` UNION the untracked-but-not-ignored files (tracked-only would miss a caller added and not yet staged; a directory walk would drag in gitignored artifacts and, in a worktree, follow the `artifacts` symlink into another checkout). It resolves direct calls, attribute calls, and local import aliases, and REFUSES a `getattr`-by-name reference rather than skipping the form it cannot bind. What it still cannot see -- a name assembled at runtime -- is stated in its docstring rather than left implied. Mutations, all measured: a third caller in `qt/robustness.py` turns it red; the full MS4 scenario above turns it red; an aliased import with the wrong arity turns it red; a `getattr` reference turns it red; an empty file derivation turns the anti-vacuity test red. ON MS2, WHERE THE BRIEF AND THE EVIDENCE DISAGREE, AND THE EVIDENCE WINS. MS2 (rewriting a listed runner's call to attribute form) turned the OLD guard red, and the instruction was to keep that red. It should NOT stay red: the old guard only matched `ast.Name`, so attribute form made it find zero calls and fail its own `assert calls` -- red from a blind spot, not from anything being wrong. The derived guard handles attribute form, so that mutation is now correctly GREEN (measured), and the property MS2 was standing in for is pinned directly instead: attribute form PLUS a signature change turns the binding test red (measured). Preserving the old red would have been preserving an accident. The census deliberately includes `tests/test_factor_source.py`, which calls the legacy entry point as its only execution coverage -- derived means derived, not "derived except the inconvenient one".⚠️ This module is SUPPOSED to go red in D6b and be deleted in D6d. D6b empties the expected set; the function itself then goes away. Written into the docstring so the next reader does not quietly "fix" it by emptying the set while the function still has callers. NIT-2 -- `test_the_persisted_factor_panel_keeps_the_market_panel_grid` asserted `len(stored) == len(market)`. The reviewer flagged it from reading and marked it UNVERIFIED. Verified before changing anything: under a reduction that keeps the length and shifts every date by one day, that test PASSES while three others go red. Confirmed, so it now compares the actual (date, symbol) index. The row count was the weakest observable the name could have meant. Also recorded: the mutation harness for this round backed up each file per EDIT rather than per FILE, so a two-edit mutation restored a half-mutated file. It was caught by the `git status` the harness prints after itself, not by any test -- the harness was fixed and every mutation above re-run from a clean tree.
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.
D6a（D6 四子步的第一步，R17 拆分）：phase0 / phase2_baseline 的因子取值改走
FactorService。只切这一步——D6b（oos/subset/robustness）、D6c（I5 框架）、D6d（删 shim + 新锚转正）都不在本 PR。按 R10 锚时间线，旧锚
ic 0.9600 / annual 0.8408在本步必须不动——实测未动。这一步的全部价值：一次可证明为 no-op 的路径切换
因此凡是会让"某个数动了"变成多选题的改动，一律登记而不夹带（见文末）。
真实 phase2 逐格对账（
config/phase2_real_baseline_cached.yaml，cache-only）：max_abs_diff = 0.0/ NaN mask 失配 0 / 索引集合差 0ic_mean 0.008274778101139676、annual_return −0.09409370539082629、sharpe、turnover、coverage、combo_ic…）；21 个叶只elapsed_seconds不同main上、切换之后在冻结树上重跑。切完就没有 legacy 可比——这是 D5a 抓到过的形态（唯一基线被自己的重跑毁掉）。B1（CRITICAL，勘察期发现，已经在发生）：demo 值正在写进真实 store
StoreKey = (factor_id, params_hash, code_hash, view)，没有任何一维说得出这批值是哪份数据算的。而config/example.yaml的 demo universe 是真实 A 股代码（000001.SZ…）、真实日期（2024）。⇒ 每跑一次
pytest（test_provisional_anchor_d2调run_phase0），DemoFeed 编造的momentum_20就被写进共享 store。而 D6b 的 oos/subset 正是 CSI300 2024–2026 + momentum_20 —— 重叠 (date, symbol) 上会命中 store、零重算、零报错地拿到编造值（红线 #6 点名的失效模式）。修法不是加固 key，是拒绝存储。 demo 价格由
_close_path的arange(n)从data.start起算 ⇒ 同一 (date, symbol) 的值随data.start而变（实测 210.0 / 187.0 / 166.0），而 key 看不见data.start：qt/factor_source.py：合成源 ⇒ ephemeral store（TemporaryDirectory，退出即删）；持久 store 配合成源 ⇒ 构造期 raise；未分类的新 source 也 raise（不假设是真数据）。策略与切换在同一个 commit，不存在"先切后隔离"的窗口。三方独立证据：① 实现方与评审各自算出同一个 key
momentum_20/7816ca515a5b9a1a__ca4d351df0bf40f6__close.parquet（demo 与 real 两配置同名）；② 评审从 PITindex_weight实测 2024 年 CSI300 与 demo 五票交集 = {000001.SZ, 000002.SZ}（今天没撞上只因 SSE50/CSI500 交集为空）；③ 最硬的一条——真实 phase2 已把该 key 写到盘上（11:33，68 只全 .SH、零 demo 票），随后跑 demo 派生同一个 key 而该文件 mtime 纹丝未动。(b) 给 key 加 provenance 维登记为 follow-up（它多买到的是真实源之间的 vintage 漂移，属 D3 既有性质、非本步引入）。
其余两条裁定
cfg.output.root_dir派生，不新增 config 旋钮。此前 16 个run_phase0测试把output.*重定向到tmp_path而 store 根不跟着走 —— 那本身就是测试隔离缺陷，与 demo 无关。coverage的分母。按红线 docs: mark PR #7 (P2-4) merged in progress docs #8（重构改实现、不改定义） reindex 回panel.index复原旧口径，并把丢掉的行数记进日志（复原可以，隐形不行）；面板有而服务没返回的 cell ⇒ValueError（红线 feat(factors): P3-1 multi-factor pipeline + first real price+financial baseline #9）。评审揪出的两条 + 实现方自审的两条
LOW-2（评审）：D6a 是第一次把日频因子放进共享 store，而 key 没有 universe 维。 今天安全只因为每个日频因子都是 per-symbol —— 但这条事实此前无测试、无登记。将来任何一个截面日频因子会在 universe-盲的 key 下静默复用错值（D5b 那个 CRITICAL 的形态；minute 面已由 D4c 修掉，日频面没有继承那个不变量）。
⇒ 新增行为式守卫：每个日频因子在 5 票 universe vs 3 票子集上各算一次，共有票必须逐位相同。为什么不做源码扫描——是测出来的：截面去均值写成
x.unstack().sub(...).stack()不含.rank(也不含groupby(level="date")，扫描 token 数 0 而 census 变红。红了是 STOP-AND-REPORT（修法是 D4c 的：存 per-symbol 中间量、combine 移到读出装配），不是加豁免名单。LOW-3（评审）：守卫的⚠️ "D6b 会让它变红并被删掉"写进 docstring，并明写不许在函数还有调用者时清空期望集来"修"它。
UNMIGRATED_RUNNERS是手敲清单。评审的 MS4 是决定性证据：第三个未登记调用者 ＋ 改签名 ＋ 两个已登记 runner 与可见测试全部尽职更新 ⇒ 21 passed，全绿，完全静默。改成从全仓派生（git ls-files∪ untracked-not-ignored；认直接调用/属性调用/别名；getattr响亮拒绝；看不见的运行时拼名写进 docstring）。三层反空过 +S1（实现方自审）：一条名为
..._preserves_every_value_...的测试实际测的是确定性。它逐行核后发现比自己上一轮报的更糟：稀疏测试只断言索引、断言值的那条跑在稠密面板（reindex 是恒等）⇒ "索引对、行错"的 reduction 全仓无人看得见。⇒ 改名 ＋ 把缺失的值断言补到它该在的地方。M16 三向（按位置 reindex：稠密值测试绿 / 稀疏索引与计数绿 / 新增稀疏值断言红，旧套件 21 passed 完全失明）把"新覆盖 vs 重复"从主张变成测量。NIT-2（评审提出、标注未验证）：
..._keeps_the_market_panel_grid只断言len(stored) == len(market)。实现方先验证再修：施加"长度不变、每 date 平移一天" ⇒ 该测试 PASS、另外三条红 ⇒ 坐实，改为比对真实(date, symbol)索引。一处实现方拒绝指令并给了测量（lead 与评审都判它对）
lead 要求「清单内文件改
ast.Attribute形态必须仍然红」。实现方拒绝：旧守卫只认ast.Name⇒ Attribute 形态让它找到 0 个调用、栽在自己的assert calls上，报出 "no longer calls _compute_factor_panel"——而这句是假的。新守卫认识该形态所以正确地绿；MS2 真正代理的性质由 M17 更强地钉住：Attribute ＋ 改签名 → 红 ⇒ Attribute 调用是被 bind 的，旧守卫连看都看不见。
评审复核后判 实现方对、lead 的指令错，并认领了源头（是它上一轮把这个红写成"一条好性质"）。
登记而不夹带（
docs/factors/d6_deferred_register.md）isinstance(FinancialFactor/ValueFactor)分派（红线 feat(neutralize): P2-3 point-in-time SW industry covariate (replace current-tag) #5；registry.requirements()零生产调用者）_maybe_enrich_covariates漏传cache⇒ 协变量每跑实时抓main上已不复现（annual −10.19% → −9.41%、sharpe/turnover 亦动，而因子 IC 逐位不变 ⇒ 动的不是行情/因子，指向 D6a-3 那条实时抓的协变量）。⇒ 拿档案数字当今天的基线是错的，本 PR 当场捕获 legacy 正是为此。并写明该对账的未言明前提：其有效性依赖协变量在两次捕获之间未被上游修订（双向危险：既能凭空造差异，也能掩盖真差异）。Gates（lead 独立重跑，未采信 subagent 自报）
pytest -p no:warnings2705 passed / 1 skipped ·ruffclean · phase0 锚 ic 0.9600 / annual 0.8408 未动 ·validate-config32/32 ·exec_baseline_freeze --verify77/77 @ 45c14aa · secret 0 · 未碰CLAUDE.md/AGENTS.md· 冻结基线 自 2026-07-25 起 0 写入 · 跑完 phase0 demo 后真实 store 中momentum_20的 mtime 仍停在 11:33（真实 phase2 写的）⇒ demo 零写入node-ID：2691 → 2706（dropped 3 / added 18）。⚠️ dropped 3 正常是危险信号，逐条有取代物且经评审独立核：两条手敲清单版 → 派生普查 4 条（MS3/MS4 旧版全绿、新版红）；一条 S1 改名后以新名字存在（主体保留 +
check_exact=True）。