Skip to content

feat: factor refactor D5a — freeze the exec baseline, catalogue the eleven runners, probe feasibility - #95

Merged
StackOverFlow11 merged 5 commits into
mainfrom
feat/factor-refactor-d5a-freeze-catalogue-probe
Jul 25, 2026
Merged

feat: factor refactor D5a — freeze the exec baseline, catalogue the eleven runners, probe feasibility#95
StackOverFlow11 merged 5 commits into
mainfrom
feat/factor-refactor-d5a-freeze-catalogue-probe

Conversation

@StackOverFlow11

Copy link
Copy Markdown
Owner

Factor-layer refactor step D5, part A: the three deliverables that must exist before the unified evaluation runner is built — a tamper-evident freeze of the only reconciliation baseline, a catalogue of the eleven runners' differences, and a feasibility probe. Pure addition (+2064/−0), no engine code touched.

The probe is why this is only part A: it found that D4's materializer cannot run the evaluation universe at all, so D5's four-leg reconciliation (C5) is not executable yet. That finding inserts a new migration step (D4b) before the rest of D5. Details below.

What (5 commits)

  1. 6e35d38 C1 — freeze the exec baseline. The 22 exec-basis evaluation artifacts are the reference every later reconciliation compares against; they are gitignored and exist only on this host, and the design's assumed durable copy (scratchpad/baseline_reports/) no longer exists. The new unified runner would write to the same filenames — one run would have destroyed the baseline, and any "reconciliation" afterwards would have compared against files it had just overwritten (the compare_postmerge.py vacuous-reconciliation failure mode this repo has on record). qt/exec_baseline_freeze.py copies them to artifacts/refactor_baseline/exec_baseline/ and commits a sha256 manifest to git (bytes stay gitignored; hashes can only change through a reviewed diff). Reads verify the hash every time; the reader structurally refuses live paths (direct, symlink-alias and .. bypasses all rejected).
  2. 559a27d Feasibility probe (qt/saturation_probe.py, a committed reproducible tool, not a throwaway script) — see the finding below.
  3. 5d2f311 C2 — catalogue the eleven runners' differences (§6.5: "an uncatalogued difference counts as a failure"). Verified complete by AST-level comparison rather than sampling: _build_book_factors / _write_report / _section_payload are character-identical across all 11; the other shared helpers collapse to one group once string constants are stripped. Also records 7 pre-existing bugs (marked, not fixed) and the FactorSpec 16→20 key drift from feat(factors): D1 — factor registry, PanelField requires, FactorSpec contract v1.0 #86/feat: factor refactor D3 — sparse value store, keys/fingerprints, tail-recompute (+R5 fina horizon unification) #91 that must be pre-registered so reconciliation does not chase it as a D5 regression.
  4. d42602e Review fixes (1 HIGH + 2 MEDIUM + 5 LOW/NIT — see below).
  5. 1c92053 NIT-A: tell first-time freezers to pass --source-note.

The finding: D4's materializer cannot run the evaluation universe

Measured, reproduced independently by the reviewer, stk_mins_live_calls=0 throughout:

  • 995 symbols, 84 of them have their first 1min bar at or after emit_start — so the pooled saturation criterion can never terminate early and always expands to the declared floor. D4's disclosed cost ("a symbol that cannot fill its valid-day pool drags the whole universe to the floor") is structurally guaranteed at full scale, not occasional.
  • Disk 8.18 GB (eval window) / 16.43 GB (floor depth); nominal memory 52.7 / 122.2 GB. The per-symbol memory extrapolation is stable within ±1% across n=20/40/60. Peak RSS is 2.2–2.45× the frame (minute_bars concatenates per-symbol parts, so parts and result coexist), which puts the eval window's true peak at ≈115 GB against 56 GB available.
  • Not just pooled factors: the bounded path also loads the whole universe in one frame (materialize.py:262), so the eval window alone already exceeds available memory by roughly 2×.
  • Three cheaper escapes were evaluated and refuted. Caller-side per-symbol batching is not merely slow — factors/service.py::_ensure_coverage computes gaps by date only, not (date, symbol), so a second batch finds "no gap", returns early, and its symbols vanish from the cross-section silently (the I5a red line). Time-chunking only helps the 3 bounded factors (the 8 pooled ones re-expand to the floor every chunk). Shortening the window doesn't move the pooled load's left edge, which is anchored at the floor.
  • intraday_amp_cut closes the last door: it is both pooled and cross-sectional (needs ≥ AMP_CUT_MIN_CROSS_SECTION=10 symbols per day), so per-symbol batching destroys its cross-section and time-chunking cannot bound its depth. Measured: pure factors are bit-identical per-symbol vs whole-universe (max|diff|=0.0 over 728 / 1524 comparable cells), while intraday_amp_cut goes from 1860 finite rows to 0.
  • A design-level gap this exposes, recorded honestly: §8 sized the decision hot path (a 20-day tail) and never sized the evaluation path, which is why a whole-universe single frame passed design review. Meanwhile the old runners' docstrings state outright that "the multi-year all-symbol minute panel is NEVER materialized" — D4's load geometry is a regression relative to what it replaces, invisible at D4's 40-symbol smoke scale.
  • One idea of the lead's was tested and refuted: reusing qt/panel_freeze.py's loading form instead of building D4b. It has no loading form of its own — it imports the 11 runners and calls their private _load_*_panel, i.e. exactly the second data path decision ③ exists to kill. What is reusable is the shape (read one symbol, aggregate to daily, discard), not the call path.

Consequence: a new migration step D4b (per-symbol streaming materialization) is inserted before the rest of D5, with its own four-leg acceptance including an end-to-end cell-by-cell reconciliation of intraday_amp_cut. The design doc records this as implementation-time amendment A1. C3–C6 remain unstarted — deliberately: fixing the materializer changes the factor values path and must carry its own acceptance, and smuggling it in as a "make the run work" change is the exact shape this project keeps catching.

Review (adversarial, 2 rounds)

Round 1 BLOCK, 1 HIGH: freeze() rewrote the manifest unconditionally and did not inherit source_note, so running the command the docs called "idempotent" on an unchanged tree reported pure success (copied: 0 / already ok: 77) while silently wiping the provenance narrative and re-stamping frozen_at_git_head to the re-runner's SHA — an active false claim, in the one file whose whole purpose is provenance, with all 16 tests still green afterwards. Fixed: provenance is inherited (explicit --source-note overrides), nothing is written when content is unchanged, and the always-running manifest test now asserts source_note non-empty, frozen_at_git_head matches ^[0-9a-f]{40}$, every sha256 is valid hex (previously only length), and size_bytes > 0. The reviewer mutated each of the four assertions separately — all four fail as intended.

MEDIUM ×2, both fixed: a vacuous test in this PR's own suite (test_frozen_tree_is_not_a_git_tracked_path never touched git — commenting out .gitignore's artifacts/ left it passing), now checks the git index via git ls-files -- <prefix>; and the C6 blast radius, where three of the four non-test modules importing the 11 runners were uncatalogued — including qt/panel_freeze.py, the tool that produced the D1 frozen baseline that D5's panel leg compares against. Its documented rerun procedure is a hybrid (old SHA + current branch's file) that goes stale under either C6 disposition; that is now registered too.

Two vacuous mutations were written during this work and both are recorded (in the freeze doc, not just the PR): the first tamper test replaced a byte string that did not exist in the file; the git-index mutation's first version used git add -f, which git refused under a symlink, so nothing changed and the test "passed" for the wrong reason. Neither was caught by a test — both were caught by an observation contradicting an expectation. The reviewer added a third: its own suggested git ls-files --error-unmatch returns rc=1 identically for "not tracked" and "path does not exist", so an assertion built on it would have passed vacuously on exactly the machines lacking the frozen bytes. Review suggestions need mutation evidence too.

Lead independent gates: pytest 2117 collected, rc=0 (2116 passed + 1 skipped without the artifacts symlink; the skip is the byte-binding test, whose bytes are gitignored — the manifest self-consistency test runs unconditionally) · ruff clean · phase0 anchor ic 0.9600 / annual 0.8408 unchanged · validate-config 31/31 · secret scan 0 · --verify 77/77 with frozen_at_git_head still 45c14aa after every rerun · artifacts/refactor_baseline/{panels,panels_d2} byte-identical throughout · worktree clean.

Aside, corrected here because it has been repeated in project docs for many sessions: "pytest's summary line gets swallowed by this machine" is a misdiagnosispyproject.toml:53 already sets addopts = "-q", so passing -q again makes it -qq. Drop the flag and the summary line returns. Dot-counting remains a useful cross-check (it caught a 2113-vs-2114 discrepancy here) but it was never necessary for lack of a summary.

Disclosed boundaries

  • NIT-B (accepted, not fixed): deleting the frozen tree and re-freezing from a different live source would carry the old provenance note onto new bytes. Reachability is low (requires deleting the tree first) and the manifest diff would show all 77 hash lines changing, which is loud; the docstring now says to pass --source-note when the source really changed.
  • The byte-binding test skips where the frozen bytes are unreachable. The recommended hard gate belongs in C5's consumption path (verify_all() 77/77 or refuse to run), not in more test plumbing; noted for D5 part B.

No CLAUDE.md/AGENTS.md changes here (docs PR follows). artifacts/refactor_baseline/{panels,panels_d2} untouched.

… anything

The 22 exec-basis eval artifacts (+22 rendered reports, +11 basis-sanity
disclosures, +22 dashboards = 77 files) are the only thing the D5 unified
runner can be judged against, and they are gitignored, single-copy, and not
reproducible from current code: regenerating them needs a pre-D5 tree plus a
~1.5h run. The unified runner writes to those same filenames. Running it once
would destroy the baseline, and any reconciliation performed afterwards would
compare the new output against a copy of itself -- the compare_postmerge.py
empty-reconciliation failure mode already committed once in this repo.

So the freeze lands before any D5 engine code. Bytes go to gitignored
artifacts/refactor_baseline/exec_baseline/; the 77 sha256 hashes go into git.

That split is what gives the tamper check teeth: FrozenExecBaseline verifies
every file it serves against the committed manifest at read time, so a live
artifact copied over a frozen one is a loud failure rather than a silent
substitution -- and whoever overwrites the frozen tree cannot also move the
hashes without a reviewable diff. Reconciliation code must go through this
reader; it may not open artifacts/reports/ itself.

Provenance is stated by evidence strength, not asserted. Observed: all 77 exec
files share one mtime instant (2ms spread) -- a bulk copy, not a 1.5h run --
8 minutes after PR #79 merged. Contrast: the 44 close-basis artifacts carry 11
progressive mtimes matching the recorded #74 rerun window exactly. Attributed
(not observed): the content comes from the #79 exec-basis run. The manifest
field is named frozen_at_git_head, not producing_sha, so the freeze instant is
never misread as the producing commit.

Mutation evidence, run rather than claimed: reader aimed at artifacts/reports/
raises; one flipped byte in a frozen file raises and verify_all reports 76 ok /
1 problem while untouched siblings still read; re-freezing after the live source
changed refuses and leaves the frozen copy byte-identical; a short or
over-full inventory raises. The first draft of the tamper test replaced a byte
string absent from the file, so it mutated nothing and "passed" against a guard
never exercised -- the test now asserts its own mutation is real.
… universe

It cannot, and the margin is not close: a whole-universe floor-depth load needs
an extrapolated 122 GB against 56 GB of available RAM (2.19x). Measured before
committing to a multi-hour run rather than discovered ten minutes into one.

Three findings, all reproducible via python -m qt.saturation_probe (cache-only,
live_calls=0 asserted in the output):

1. Pooled saturation reaching the declared floor is STRUCTURAL, not incidental.
   The termination criterion requires every REQUESTED symbol to accumulate
   lookback_days valid days at or before emit_start, and 84 of the 995 CSI500
   evaluation symbols have their first 1min bar on or after emit_start -- they
   were not listed yet, so no depth of backward expansion can satisfy them. One
   such symbol forces the whole universe to the 2015-01-05 floor, and a
   five-year CSI500 window always contains some: the index keeps admitting new
   listings. Floor depth is 2.01x the evaluation window on disk (16.43 vs 8.18
   GB).

2. The problem is not confined to pooled factors. The bounded path loads the
   whole universe in one frame too (materialize.py:262), and the evaluation
   window alone measures an extrapolated 52.7 GB -- 0.94x available RAM, before
   any factor computation, once per factor. Per-symbol streaming is a structural
   prerequisite for all eleven factors, not a pooled-only optimization. The
   eleven old runners never hit this because they read symbol by symbol and
   aggregate to daily immediately; loading the universe at once is a D4 shape
   that a 40-symbol smoke could not expose.

3. The fix has a right and a wrong incision point, decided by measurement rather
   than assumption. Whole-universe versus per-symbol-concatenated, on real bars:
   ridge_minute_return and volume_peak_count agree bit-for-bit (max|diff| 0.0,
   identical NaN sets), while intraday_amp_cut flips all 1692 rows to NaN,
   because its step-4 cross-sectional z-score needs at least
   AMP_CUT_MIN_CROSS_SECTION=10 finite pairs on a date and a one-symbol
   cross-section is all-NaN by definition. So the per-symbol split must happen
   before that combine, not around it -- which is how the factor module and the
   old runner are already organised.

The accompanying note argues the AMP_CUT coupling explicitly, as required: D4's
warning is against per-symbol TRUNCATION, which really does move the
cross-section; per-symbol SATURATION keeps every symbol's terminal depth
unchanged and only stops forcing the other 911 to travel with the deepest one.
That argument's load-bearing premise is measured for the ten per-symbol-pure
factors and NOT yet measured for intraday_amp_cut, and the note says so: the
equivalence is not to be treated as verified until that reconciliation is run.

No engine code is changed here. This commit measures and documents.
…unifying them

Design v3.2 §6.5 requires the eleven runners' differences to be catalogued
BEFORE they are merged, because after the merge an uncatalogued difference is
indistinguishable from a regression -- the rule is that finding one counts as an
acceptance failure. This table is the difference whitelist C5's reconciliation
will be read against.

Method: AST body comparison across all eleven, a flattened YAML comparison of
all eleven configs, and targeted reads of every diverging region. The headline
is that only FOUR extension points are actually needed; the rest can be a fixed
body, and §5 records the thirteen classes of meaningless drift being normalised
so that normalisation is a decision on the record rather than a silent edit.

The load-bearing finding is that a unified runner cannot assume the per-symbol
reducer returns the factor. Six runners yield a plain series, three yield a
series plus a diagnostics sink, and two (intraday_amp_cut, valley_price_quantile)
do not produce the factor in the loop at all -- they emit a stats frame that a
mandatory post-loop cross-sectional finalizer turns into the factor, and one of
those finalizers also needs the daily close panel. That same cross-sectional
step is why the D5 per-symbol streaming incision has to land before the combine,
which ties this catalogue to the saturation feasibility note.

Also closes the structural half of one of the three items CLAUDE.md lists as
"not independently checked" from PR #79: execution_capacity passthrough is a
single shared code path with no runner variation, and the disclosure dict
deliberately omits the two keys the Tradable axis reads, so NOT_ASSESSED cannot
vary per factor by construction. The other two items remain open.

Seven defects are flagged and deliberately NOT fixed. Two were independently
re-verified here rather than taken on trust:

BUG 1 (CRITICAL, user-visible) -- three runners and the CLI describe a defect
PR #74 already fixed. They claim the frozen layer computes sign*(gross-cost) and
"is not patched"; standard.py:419 reads `sign * gross - base_cost`, with the old
form described in the past tense. The v0.8 fix commit is an ancestor of the
commit that last edited the ridge runner, so the text was already stale when it
was last touched, and it prints to stdout on every PR-K run. This is the
#76/#78/#82 failure mode again: behaviour changed, wording did not.

BUG 2 (HIGH) -- aligned_spread_by_cost is extracted by two runners but no key of
that name exists anywhere in analytics/. Verified against the frozen artifacts
themselves: absent from both return_risk payloads, so both runners always report
{}. Its only guard is `assert isinstance(m[...], dict)`, which {} satisfies
unconditionally -- an impossible-to-fail test of exactly the kind the project
methodology names.

BUG 3 (HIGH) -- peak_ridge_amount_ratio has no runner test at all, leaving 122
lines of coverage-disclosure logic untested while all three sibling summarizers
have counterfactual and empty-frame tests.

BUG 6 is a direct warning to C6: a duplicate test function name across two files
is currently harmless only because tests/__init__.py exists, so a consolidation
that removes it would silently drop a test.

Finally, one artifact difference that is expected and must be pre-registered or
it will be chased as a D5 regression: the frozen spec block has 16 keys and
current code emits 20, the four additions being requires/adjustment/
overnight_boundary/lookback_depth from PR #86 and #91, which reach the JSON
automatically through sanitize_payload(vars(report.spec)).
…venance

Review BLOCK, one HIGH and two MEDIUM, plus five smaller corrections.

HIGH -- freeze() rewrote the manifest unconditionally, regardless of whether it
had copied anything, and source_note came from a CLI argument that defaulted to
empty and was never inherited. So running the command this repo's own
documentation tells you to run, on a completely unchanged tree, printed
`copied: 0 / already ok: 77` while the git diff showed frozen_at_utc and
frozen_at_git_head repointed at whoever re-ran it and the entire provenance
narrative blanked. What broke was the one thing this PR exists to protect, the
documentation actively steered people into it, and all sixteen tests passed
through the destruction. The restamped frozen_at_git_head was also an active
false claim -- it would assert the baseline was frozen at the re-runner's HEAD,
the exact misreading the doc explains the field name was chosen to avoid.

Provenance is now inherited from an existing manifest unless explicitly
overridden, and an unchanged tree writes nothing at all, so the freeze is
byte-idempotent and the doc says "byte-idempotent" rather than "idempotent".
Inheritance only takes what is actually present: a manifest that never carried
provenance gets fresh values rather than an inherited empty string, which would
trade a restamp for a silent blank. The always-run manifest test now asserts
source_note is non-empty, frozen_at_git_head is 40 hex, every sha256 is valid
hex rather than merely 64 characters, and every size is positive.

MEDIUM -- test_frozen_tree_is_not_a_git_tracked_path claimed the bytes stay
untracked while the hashes are committed, but never consulted git: it froze a
fake tree under tmp_path, deleted it, and asserted a file it had just written
was readable. It now queries the git index. The index rather than the
filesystem because artifacts/ is routinely a symlink and every
filesystem-walking git command refuses such paths with "beyond a symbolic
link" -- which is also why the first version of this mutation proved nothing.

MEDIUM -- the catalogue only recorded qt/cli.py as importing the eleven
runners. Three more do, and all three are D1/D2 acceptance tools, including
qt/panel_freeze.py, which produced the very baseline D5's panel leg compares
against. Deleting the runners in C6 would take the ability to regenerate or
re-verify that baseline with them -- the same asymmetric loss that justifies C1.
Recorded with a required disposition, alongside the note that
d1_panel_freeze_manifest.md's "rerun command" section goes stale either way.

Smaller: the close-basis family is 66 files, not 44 -- the miscount dropped the
dashboards, in a document that two paragraphs earlier explains dashboards were
found and frozen on the exec side. The exec mtime spread is 9.46 ms, not ~2 ms,
and the "11 progressive values" is a JSON-only count (27 across all 66), so the
caliber is now stated. The probe printed max|diff|=0.000e+00 next to NOT SAFE
where the two series share no comparable cell at all; it prints "n/a (no
comparable cell)" with an explicit compared-count, which also shows the SAFE
rows agree over 728 and 1524 real comparisons rather than vacuously. Both docs
now state the gitignored-artifacts prerequisite and the nested-symlink trap.

Mutation evidence, re-run against the final code: blanking the manifest
provenance fails the always-run test (rc=1) where all sixteen previously passed;
injecting a frozen path into the git index moves tracked-under-artifacts 0->1->0
and fails the tracking test (rc=1); running freeze on an unchanged tree reports
"manifest: unchanged" and leaves zero porcelain lines.

The second of those mutations was itself empty on the first attempt -- git add
-f refused to traverse the symlink, so nothing changed and the test "passed"
for the wrong reason. That is the second vacuous mutation in this work, both
caught by an observation contradicting the expectation rather than by the test.
Both are recorded in the freeze document.
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