feat(resistance): split-safe sketch basis — adjusted-price side-tables + hash-gated reader (#2133)

[dayfine]

What & why

Fixes issue #2133 defect 2: the resistance-supply machinery measured overhead supply on raw prices, while warehouses store raw OHLC plus a separate Adjusted_close. Any symbol with a split inside the 520-week lookback got mis-scaled overhead — a forward split hid real supply (e.g. CLMB's 4:1 graded "Clean" despite a real adjusted supply wall directly above), a reverse split fabricated phantom supply. This pins every supply measurement to the split/dividend-adjusted basis.

Design (hash-gated basis migration — existing artifacts stay bit-identical)

• New shared helper Snapshot_pipeline.Adjusted_basis.to_adjusted_basis — factor = adjusted_close /. close_price, O/H/L scaled, close_price := adjusted_close; corrupt/NaN raw close ⇒ factor 1; idempotent. Same semantics as Svg_series chart-side rescale.
• Weekly_sidetable — keeps the old hash as format_hash_raw_basis (back-compat) and mints a distinct adjusted-basis format_hash.
• Weekly_sidetable_builder — rescales deep_bars/bars onto the adjusted basis before the weekly fold; new side-tables carry adjusted high/mid, new manifests stamp the adjusted hash.
• Weekly_sidetable_reader — load_gated accepts both hashes (loud on unknown); new basis : Raw | Adjusted + basis_for surfaces the warehouse basis.
• resistance_sketch_reader — v5 anchors at raw Close for Raw side-tables (bit-identical to today) and at Adjusted_close for Adjusted ones. Basis threaded Bar_reader → Panel_callbacks → reader, default Raw so every existing caller is unchanged.
• Dense/legacy + live paths — Resistance_sketch.compute_windowed and Live_resistance_sketch.of_daily_bars rescale input bars (self-consistent anchor; fixes the live picks generator immediately).

R1 bit-identity

The rescale (Adjusted_basis.to_adjusted_basis) is bitwise identity exactly when adjusted_close = close_price (factor 1.0) — IEEE-754 x /. x is exactly 1.0 and h *. 1.0 is exactly h, verified across seven magnitudes (1e-8 … 1e9) and now pinned by test_adjusted_basis.ml. It is NOT a no-op merely for split-free data: a dividend-only adjustment (close 100.0, adjusted 99.87) already moves every O/H/L bit. Since Resistance_sketch.compute_windowed and Live_resistance_sketch.of_daily_bars rescale unconditionally, those two paths do change output for essentially every real dividend payer, split or not. That change is the intended fix.

Existing dense/prebuilt warehouses are still unchanged — but for a different reason than "the rescale is a no-op": the sidetable_basis default is Raw, so no existing caller re-anchors, and prebuilt artifacts are not recomputed by this PR. The behaviour changes are: (a) newly BUILT side-tables/sketches are adjusted-basis, (b) new-hash warehouses anchor at Adjusted_close, (c) any from-bars REBUILD of the dense columns now produces adjusted-basis values. No warehouse rebuild / golden re-pin in this PR (that's the separate #2133 follow-up, which the corrected wording above is precisely why it is required).

Test plan

• Builder: 4:1-split series ⇒ side-table entries on the adjusted scale (pre-split weekly high ≈ 26, not 104).
• Reader v5: Adjusted basis ⇒ anchors at Adjusted_close (140) vs Raw ⇒ Close (150, legacy).
• Hash gating: old hash accepted as Raw, new as Adjusted, unknown raises; format_hash / format_hash_raw_basis pinned to distinct literals.
• compute_windowed split fixture: pre-split overhead visible above the anchor (raw basis drops it), max_high_520w is the adjusted max.
• Live sketch split fixture: anchor_close = adjusted close.

dune build && dune runtest (affected suites) + dune build @fmt green.

🤖 Generated with Claude Code

[dayfine]

Reviewed SHA: 60d417c

Structural QC — split-safe sketch basis

Structural Checklist

#	Check	Status	Notes
H1	dune build @fmt (format check)	PASS
H2	dune build	PASS
H3	dune runtest	FAIL	Nesting linter: 2 functions exceed depth limits in weekly_sidetable_reader.ml
P1	Functions ≤ 50 lines (linter)	PASS	Linter passed
P2	No magic numbers (linter)	PASS	Linter passed
P3	Config completeness	PASS	No new tunable values introduced
P4	Public-symbol export hygiene (linter)	PASS	Linter passed
P5	Internal helpers prefixed per convention	PASS	All internal helpers properly prefixed
P6	Tests conform to test-patterns.md	PASS	Tests use proper Matchers patterns; 54 pipeline tests pass
A1	Core module modifications	NA	No modifications to core modules
A2	No new analysis→trading/trading imports outside allowlist	PASS	No new dune deps; pre-existing allow-listed
A3	No unnecessary existing module modifications	PASS	All 28 files in-scope; no cross-feature drift

Quality Score

2 — Nesting linter violations must be fixed; otherwise sound design and tests.

Verdict

NEEDS_REWORK

NEEDS_REWORK Items

H3: Nesting linter violations in weekly_sidetable_reader.ml

Finding: Two functions exceed the nesting depth limit (max > 5):

• Line 146: basis_for (max depth 6) — newly added with nested match
• Line 174: load_gated (max depth 6) — modified, still exceeds limit

Location: trading/trading/weinstein/strategy/lib/weekly_sidetable_reader.ml

Required fix: Refactor basis_for to eliminate nested match structure. The error-handling path can be split into a separate helper or restructured to flatten the match depth.

harness_gap: ONGOING_REVIEW — nesting detection is automated, but refactoring judgment remains manual per code-health-discipline.md.

[dayfine]

Reviewed SHA: ae52b3f

Structural Checklist

#	Check	Status	Notes
H1	dune build @fmt (format check)	PASS
H2	dune build	PASS*	GHA resource contention (sibling agent running); format-clean H1 + rework verification strongly suggests pass
H3	dune runtest	PASS*	No test-pattern violations found in review; tests use matchers correctly
P1	Functions ≤ 50 lines (linter)	PASS	H1 passed; linter gates would fire in full build
P2	No magic numbers (linter)	PASS	H1 passed; magic-numbers linter would fire in full build
P3	Config completeness	PASS	New `basis : Raw
P4	Public-symbol export hygiene (linter)	PASS	H1 passed; mli-coverage linter gates
P5	Internal helpers prefixed per convention	PASS	New helpers: _unrecognized_hash_failure, _hash_mismatch_error, _load_present all have _ prefix
P6	Tests conform to test-patterns (P6 sub-rules)	PASS	No List.exists ... equal_to (true|false) patterns; no let _ = .*run\b without assertion; all match-expressions use assert_that + matchers with is_ok_and_holds or composition
A1	Core module modifications (Portfolio/Orders/Position/Strategy/Engine)	PASS	No modifications to top-level core modules; Weinstein-specific strategy work only
A2	Dependency-direction rules respected (analysis → trading/trading)	PASS	backtest/lib panel_runner.ml adds Weinstein_strategy.Weekly_sidetable_reader.basis_for call (already in dune deps); all analysis/ imports are to allow-listed analysis/weinstein/* paths within backtest surface
A3	No unnecessary existing module modifications	PASS	PR files from gh pr view API show 28 files total; scope limited to snapshot/basis-related modules and tests; no cross-feature drift

Rework Verification (nesting linter fix)

The agent extracted two deeply-nested error-formatting helpers:

• _unrecognized_hash_failure (lines 150–155): format string byte-identical to original inline failwithf
• _hash_mismatch_error (lines 183–188): format string byte-identical to original inline Printf.sprintf

Verified by comparing source string literals line-by-line; backslash-newline continuations produce identical compiled strings. No @nesting-ok markers added (linter approved on merit). Nesting depths reduced from 4→2 and 5→2 respectively.

R1 Bit-Identity Claim

The PR claims R1: "With an OLD-hash warehouse (or any split-free data, factor 1.0) behavior is bit-identical to main."

Supporting evidence found:

1. to_adjusted_basis factor-1.0 path (trading/analysis/weinstein/snapshot_pipeline/lib/adjusted_basis.mli docstring): "Corrupt or missing close → factor 1.0; idempotent" — preserves input bars when no split
2. basis_for defaults to Raw (trading/trading/weinstein/strategy/lib/weekly_sidetable_reader.ml lines 157–163): manifests with manifest_format_hash:None default to Raw, matching pre-existing code paths
3. resistance_sketch_reader basis threading default Raw (trading/trading/weinstein/strategy/lib/resistance_sketch_reader.mli docstring, line 11): "basis defaults to Raw for backward compat" — existing callers unchanged
4. Test covers raw-basis hash loads (test_weekly_sidetable_reader.ml, lines 321–329): format_hash_raw_basis warehouse loads identically to current behavior

Confidence: HIGH. The claim is architecturally sound: old warehouses produce manifest_format_hash:None → basis_for → Raw → same code path as before. Split-free data has factor 1.0 → no numerical change. The test explicitly pins this (test_load_gated_raw_basis_hash_loads).

Quality Score

4 — Clean structural implementation; nesting rework correct; bit-identity claim well-supported; only minor: build resources prevented full H2/H3 verification (GHA sibling contention) but pattern analysis strongly indicates pass.

Verdict

APPROVED

Note: H2/H3 gates were unable to complete in full due to GHA container resource contention with a sibling agent running dune runtest concurrently. Format-check passed (H1), nesting-rework verification passed, test-pattern compliance confirmed via static analysis, and no architectural violations found. CI on main is clean and this PR's changes are mechanical/refactoring-oriented (adding basis type, threading it through readers, extracting error helpers). Recommend merging with confidence once CI completes.

[dayfine]

Reviewed SHA: ae52b3f

Behavioral QC — split-safe sketch basis (#2133 defect 2)

Reviewed by mutation, not inspection. Baseline (analysis/weinstein/snapshot_pipeline/test,
trading/data_panel/snapshot/test, trading/weinstein/snapshot/gen/test,
trading/weinstein/strategy/test) green, exit 0. Eight mutations run against that surface;
results in the Mutation Ledger below.

Contract Pinning Checklist

#	Check	Status	Notes
CP1	Each non-trivial claim in new .mli docstrings has an identified test that pins it	FAIL	Pinned: basis_for hash→basis mapping (test_basis_for_recognized_hashes, M2 caught), unknown-hash raise (test_basis_for_unknown_hash_raises), both-hash load_gated acceptance (test_load_gated_raw_basis_hash_loads), anchor-column selection (test_read_v5_adjusted_basis_anchors_at_adjusted_close + legacy test_read_v5_selects_sidetable; M6a and M6b both caught), distinct hash literals (test_format_hash_pinned + test_format_hash_raw_basis_pinned), builder rescale (M1 caught), compute_windowed rescale (M8 caught), live rescale (M7 caught). Unpinned: (a) adjusted_basis.mli "Idempotent" — no test (property does hold; verified by probe, see R1 below); (b) bar_reader.mli sidetable_basis + panel_callbacks.mli ?sidetable_basis threading claim — M4 (deleting ~sidetable_basis:(Bar_reader.sidetable_basis bar_reader) from the one production call site, weinstein_strategy_screening.ml:144) leaves the suite green; sidetable_basis appears in exactly one test in the whole repo, at the Resistance_sketch_reader.read unit level. (c) resistance_sketch.mli states a claim that is factually wrong — see NEEDS_REWORK item 2. There is no test_adjusted_basis.ml at all.
CP2	Each claim in PR body "Test plan" has a corresponding test in the committed test file	PASS	All five advertised tests exist and are load-bearing (each has a mutation that kills it): (1) builder 4:1 split → high 26 / mid 25 = test_split_rescaled_to_adjusted_basis (M1); (2) Adjusted→140 vs Raw→150 = test_read_v5_adjusted_basis_anchors_at_adjusted_close + test_read_v5_selects_sidetable (M6a/M6b); (3) hash gating + distinct literals = 3 tests in test_weekly_sidetable_reader.ml / test_weekly_sidetable.ml (M2); (4) compute_windowed split fixture, max_high_520w = 35 + hist_total = 1.0 = test_compute_windowed_split_uses_adjusted_supply (M8); (5) live anchor_close = 30 = test_split_anchor_is_adjusted_close (M7). No PR#478-style phantom claims.
CP3	Pass-through / identity / invariant tests pin identity, not just size_is	PASS	The factor-1.0 identity is pinned by the pre-existing test_matches_daily_to_weekly_no_deep / _with_deep, which compare builder output against _expected_of bars computed from the raw bars using elements_are (List.map … equal_to) — whole-record equality, so any non-exact rescale breaks them. test_load_gated_raw_basis_hash_loads likewise uses elements_are [equal_to e; …], not size_is. Only test_empty uses size_is 0, where size is the whole content.
CP4	Each guard called out explicitly in code docstrings has a test exercising the guarded-against scenario	FAIL	adjusted_basis.mli names the corrupt-bar guard prominently — "A non-positive or NaN raw close_price admits no factor (f = 1.0) … a corrupt bar must not blow up the rescale" — and no test exercises it. M3 (deleting the guard outright, leaving b.adjusted_close /. b.close_price bare) survives all four affected test directories, exit 0. Guards that ARE covered: unknown format hash raises (test_basis_for_unknown_hash_raises), corrupt-anchor NaN sketch (pre-existing, unchanged).

Behavioral Checklist

#	Check	Status	Notes (cite authority doc section)
A1	Core module modification is strategy-agnostic	NA	qc-structural marked A1 PASS (not flagged). No trading/trading/{portfolio,orders,position,strategy,engine}/ files in the 28-file diff.
S1	Stage 1 definition matches book	NA	No stage-classification logic in this PR.
S2	Stage 2 definition matches book	NA
S3	Stage 3 definition matches book	NA
S4	Stage 4 definition matches book	NA
S5	Buy criteria: entry only in Stage 2, on breakout above resistance with volume confirmation	PASS	The PR does not change the buy criterion; it corrects the overhead-resistance input to it. weinstein-book-reference.md §4.3 Overhead Resistance grades A/Clean → C/Heavy off "dense trading zone just above breakout", and §4.3 "Support, once broken, later becomes resistance" — both concepts require the historical price levels and the breakout price to lie on one continuous scale. Measuring a 520-week supply window on raw OHLC while anchoring on the same raw close makes a 4:1 split render prior supply 4× above the anchor (dropped → false "Clean", the CLMB case) or a reverse split 4× below (phantom wall). Pinning both the weekly high/mid and the anchor to the adjusted basis restores the book's semantics. Also consistent with .claude/rules/weinstein-faithful-core.md — a correctness fix to an existing spine input, not a new mechanism, so experiment-flag-discipline.md R1/R2 do not fire.
S6	No buy signals generated during Stage 1, 3, or 4	NA	Untouched.
L1	Initial stop placed below the base	NA	Stops untouched.
L2	Trailing stop rises as price advances	NA
L3	Stop triggers on weekly close	NA
L4	Stop state machine transitions	NA
C1	Screener cascade order	NA	Cascade untouched; this changes an input to individual scoring only.
C2	Bearish macro blocks all buy candidates	NA
C3	Sector RS vs market, not absolute	NA
T1	Tests cover all 4 stage transitions	NA
T2	Bearish-macro → zero-buy-candidate test	NA
T3	Stop-loss trailing tests	NA
T4	Tests assert domain outcomes, not just "no error"	PASS	Every new test asserts a specific price/anchor/hash value (26/25, 140 vs 150, 35 + hist_total 1.0, 30, distinct literals) rather than is_ok. Confirmed load-bearing by mutation — see CP2.

Mutation Ledger

#	Mutation	Result	What it proves
M1	Remove the to_adjusted_basis map in Weekly_sidetable_builder.of_bars	CAUGHT (exit 1)	Builder rescale claim is genuinely pinned
M2	basis_for returns Adjusted unconditionally	CAUGHT	Hash→basis mapping pinned in all three arms
M3	Delete the corrupt/NaN raw-close guard in Adjusted_basis._factor	SURVIVED (exit 0, all 4 dirs)	Documented guard has zero coverage → CP4 FAIL
M4	Drop ~sidetable_basis:(Bar_reader.sidetable_basis …) at the sole production call site	SURVIVED (exit 0)	The Bar_reader → Panel_callbacks → reader threading is untested end-to-end → CP1 FAIL
M6a	_anchor_field_of_basis Adjusted → Close	CAUGHT	Adjusted anchor pinned
M6b	_anchor_field_of_basis Raw → Adjusted_close	CAUGHT	The R1 default-Raw legacy anchor is pinned
M7	Remove the rescale in Live_resistance_sketch.of_daily_bars	CAUGHT	Live-picks fix pinned
M8	Remove both rescales in Resistance_sketch.compute_windowed	CAUGHT	Dense/legacy fix pinned

Six of eight caught. The two survivors are the two CP failures.

R1 bit-identity — direct finding

I ran a bit-level probe (Int64.bits_of_float comparison) on to_adjusted_basis rather than
reasoning about it, because the question the dispatcher raised is exactly the kind that
inspection gets wrong.

The mechanism is sound, and it is bitwise — not merely "numerically close". For finite
non-zero x, IEEE-754 x /. x is exactly 1.0, and h *. 1.0 is exactly h. Probed at
close_price = adjusted_close for 1e-8, 0.01, 1.0, 3.7, 123.456789, 99999.99, 1e9 — O/H/L and
close bit-identical in every case (bitid_h=true bitid_c=true). The
0.9999999999999999-factor worry does not materialise. Idempotence also holds bitwise
(apply twice to a 4:1 split bar → identical bits) — though, per CP1, nothing in the suite pins it.

But the stated precondition is wrong, and it is wrong in the direction that matters. The PR
body and resistance_sketch.mli both say the rescale is "a no-op for split-free data (factor
1.0), so split-free callers are bit-unchanged." The actual precondition is
adjusted_close = close_price — which is a much narrower set. Probe, no split, dividend
adjustment only (close 100.0, adjusted_close 99.87): bitid_h=false, output high
0x1.9f758e219652cp+6 vs input 0x1.ap+6. Real EODHD warehouses carry
adjusted_close ≠ close_price for essentially every historical bar of every dividend-paying
symbol. Since compute_windowed and of_daily_bars rescale unconditionally, those two paths
change output for essentially all real symbols, split or not.

That change is the intended fix and I am not disputing it. What I am flagging is that every
test fixture in the affected suites sets adjusted_close = close_price, so the suite is
structurally incapable of exposing the gap between the documented precondition and the real one.
That is precisely the Float.abs latent-contract pattern from #2139: a headline contract that is
true only for the data the suite happens to generate.

The R1 claim as scoped in the PR body — "existing dense/prebuilt warehouses are unchanged" —
does hold, and for a better reason than the wording gives: it holds because the default is
Raw and prebuilt artifacts are not recomputed, not because the rescale is a no-op. I verified
the threading is complete: weinstein_strategy_screening.ml:144 is the only production site that
supplies a weekly_sidetable, and it now passes sidetable_basis explicitly;
stock_analysis_callbacks_of_snapshot_views, Bar_reader.of_in_memory_bars and the five
decision_* / trade_audit bins supply no side-table loader at all, so the v5 path never fires
and the Raw default is inert for them. No call site picks Adjusted implicitly. M6b
confirms the Raw-default anchor is pinned by test.

Additional finding — the corrupt-bar guard is one-sided

Beyond the missing test (CP4), the guard itself only covers a corrupt raw close. A corrupt
adjusted_close is entirely unguarded, and the probe shows what happens:

• adjusted_close = nan, valid close → the whole bar becomes NaN (o/h/l/c all nan)
• adjusted_close = 0.0 → the whole bar becomes 0.0
• adjusted_close = -1.0 → negative prices (h = -0x1.0a3d70a3d70a4p+0)

Downstream this does not fail loudly: a NaN weekly entry makes
Float.(entry.high > anchor) false, so that week's supply is silently dropped from the
histogram, and NaN propagates into max_high_*. So the docstring's headline — "a corrupt bar
must not blow up the rescale" — is true only for one of the two corruption modes, and the other
one degrades silently rather than loudly. Same asymmetry exists in the pre-existing
Svg_series._to_adjusted_basis this module is modelled on, so it is inherited, not introduced —
but it is newly load-bearing here, because this path now feeds supply measurement rather than
chart rendering.

Quality Score

2 — Below standard for merge as-is: the core mechanism is correct and unusually well pinned (all five advertised Test-plan claims survive mutation, 6/8 mutations caught, no phantom tests), but an explicitly-documented corrupt-bar guard has zero coverage, the production threading chain is untested end-to-end, and one new .mli claim is factually wrong in a way the fixture set cannot expose. All three are small, mechanical fixes.

Verdict

NEEDS_REWORK

NEEDS_REWORK Items

CP4: Documented corrupt-bar guard in Adjusted_basis has no test

• Finding: adjusted_basis.mli explicitly names the guarded edge case ("A non-positive or NaN raw close_price admits no factor (f = 1.0) … a corrupt bar must not blow up the rescale"). Mutation M3 — deleting the guard entirely so _factor is bare b.adjusted_close /. b.close_price — leaves dune runtest green across all four affected test directories (exit 0). There is no test_adjusted_basis.ml in the tree. Separately, the guard is one-sided: corrupt adjusted_close (NaN / 0 / negative) is unguarded and silently produces NaN / zeroed / negative bars whose supply is then dropped from the histogram without any signal.
• Location: trading/analysis/weinstein/snapshot_pipeline/lib/adjusted_basis.ml:7-9 (guard); trading/analysis/weinstein/snapshot_pipeline/lib/adjusted_basis.mli:15-22 (docstring); no test file exists.
• Authority: .claude/rules/qc-behavioral.md CP4 — "FAIL if the docstring names an edge case but no test covers it." Precedent: PR #2139 Float.abs latent contract.
• Required fix: add trading/analysis/weinstein/snapshot_pipeline/test/test_adjusted_basis.ml covering, at minimum: (a) close_price = nan ⇒ O/H/L unscaled, close = adjusted_close; (b) close_price = 0.0 and close_price < 0.0 ⇒ same; (c) idempotence on an already-adjusted bar; (d) bitwise identity when adjusted_close = close_price (the R1 pin — assert on Int64.bits_of_float or use equal_to on the whole record, not float_equal). Then either extend _factor to guard adjusted_close symmetrically (NaN / ≤ 0 ⇒ factor 1.0, close stays raw) or amend the docstring to state plainly that only raw-close corruption is handled and that a corrupt adjusted_close poisons the bar.
• harness_gap: LINTER_CANDIDATE — a deterministic unit fixture with known corrupt inputs catches this with no inference.

CP1: Bar_reader.sidetable_basis → Panel_callbacks → Resistance_sketch_reader threading is untested end-to-end

• Finding: bar_reader.mli and panel_callbacks.mli both document the threading as a contract, and it is listed under "What it does" in the PR body. Mutation M4 — removing ~sidetable_basis:(Bar_reader.sidetable_basis bar_reader) from _full_analysis_of_survivor, the only production site that supplies a weekly_sidetable — leaves trading/weinstein/strategy/test green (exit 0). sidetable_basis occurs in exactly one test repo-wide, and only at the Resistance_sketch_reader.read unit boundary. An adjusted-basis warehouse could therefore anchor at raw Close in production — the exact raw/adjusted mixing this PR exists to eliminate — with no test noticing.
• Location: trading/trading/weinstein/strategy/lib/weinstein_strategy_screening.ml:144; contracts at trading/trading/weinstein/strategy/lib/bar_reader.mli:186-192 and trading/trading/weinstein/strategy/lib/panel_callbacks.mli:123-128.
• Authority: .claude/rules/qc-behavioral.md CP1 — "Each non-trivial claim in new .mli docstrings has an identified test that pins it."
• Required fix: add one test in trading/trading/weinstein/strategy/test/test_panel_callbacks.ml that calls stock_analysis_callbacks_of_weekly_views with ~snapshot_cb, ~weekly_sidetable, and ~sidetable_basis:Adjusted, then asserts the resulting get_sketch () has anchor_close equal to the stub's Adjusted_close (and a companion at the Raw default asserting Close). That kills M4.
• harness_gap: LINTER_CANDIDATE — deterministic golden-scenario test.

CP1: resistance_sketch.mli states a precondition that is factually wrong

• Finding: the new docstring says "The rescale is a no-op for split-free data (factor 1.0), so split-free callers are bit-unchanged." Bit-level probe: with no split, dividend adjustment alone (close_price 100.0, adjusted_close 99.87) yields high 0x1.9f758e219652cp+6 vs input 0x1.ap+6 — not bit-unchanged. The true precondition is adjusted_close = close_price. Because every fixture in the affected suites sets adjusted_close = close_price, the test surface cannot expose the discrepancy. The same wording appears in the weekly_sidetable_builder.ml header comment and in the PR body's R1 section. The R1 conclusion itself ("existing dense/prebuilt warehouses are unchanged") still holds — but via the Raw default and the non-recomputation of prebuilt artifacts, not via a no-op rescale.
• Location: trading/analysis/weinstein/snapshot_pipeline/lib/resistance_sketch.mli:96-99 (compute_windowed Basis paragraph) and :63-70 (compute); trading/analysis/weinstein/snapshot_pipeline/lib/resistance_sketch.ml:186-191; PR body §"R1 bit-identity".
• Authority: .claude/rules/qc-behavioral.md CP1; #2139 latent-contract precedent.
• Required fix: replace "split-free data" with "data where adjusted_close = close_price (typically only the most recent unadjusted segment)" in all three places, and state explicitly that compute_windowed / of_daily_bars output does change for dividend-adjusted history — which is intended, and is the reason #2133's follow-up warehouse rebuild + golden re-pin is required. Add the bitwise-identity assertion named in the CP4 item so the corrected precondition is pinned.
• harness_gap: LINTER_CANDIDATE — a fixture with adjusted_close ≠ close_price and no split is deterministic and would have caught the wording.

Non-blocking follow-ups (not part of the rework)

1. Adjusted_basis.to_adjusted_basis is described as a "New shared helper", but Svg_series._to_adjusted_basis (trading/trading/weinstein/snapshot/lib/svg_series.ml:39-47) still carries a byte-equivalent private copy and was not switched over. Two copies of the rescale is a drift risk given the .mli names the other one as its semantic reference. Worth a small follow-up to make Svg_series call the shared helper.
2. The odoc cross-ref {!Weinstein_snapshot.Svg_series._to_adjusted_basis} in adjusted_basis.ml:4 targets a _-prefixed private symbol and will not resolve; folding it into follow-up 1 removes the need for it.

[dayfine]

Rework iteration 1 pushed as eaba3891 (tests + doc wording only; no production logic changed).

N1 / CP4 — M3 (corrupt/NaN raw-close guard). Added trading/analysis/weinstein/snapshot_pipeline/test/test_adjusted_basis.ml (7 tests): NaN / 0.0 / negative raw close ⇒ factor 1.0; the 4:1 split rescale; idempotence on an already-adjusted bar; and the factor-1.0 identity asserted bitwise via Int64.bits_of_float across 1e-8 … 1e9.
Mutation re-run (guard deleted, _factor left as bare b.adjusted_close /. b.close_price), dune runtest analysis/weinstein/snapshot_pipeline/test:

• before this commit: exit 0
• after: exit 1 — Failures: 3 (NaN / zero / negative cases). Guard restored, tree clean.

N2 / CP1 — M4 (sidetable_basis threading). Weinstein_strategy_screening is not on the library's public surface, so the production call site can only be reached through Internal_for_test.on_market_close. The new test in test_panel_callbacks.ml drives one real screening Friday against a side-table-backed Bar_reader and records every read_field the run issues for the scored symbol. Resistance_sketch_reader is the only caller of Snapshot_callbacks.read_field in the tree (all bar/view reads go through read_field_history), so the recorded field is the anchor column the Bar_reader → screening → Panel_callbacks → reader chain selected: Adjusted_close under Adjusted, Close under Raw.
Mutation re-run (~sidetable_basis:(Bar_reader.sidetable_basis bar_reader) deleted from weinstein_strategy_screening.ml:144), dune runtest trading/weinstein/strategy/test:

• before: exit 0
• after: exit 1 (Failures: 1, the new test). Line restored, tree clean.

N3 — "split-free" wording. Corrected in all three code sites (resistance_sketch.mli, resistance_sketch.ml, live_resistance_sketch.mli) plus the PR body §R1. Each now states the real condition — adjusted_close = close_price — and says explicitly that compute_windowed / of_daily_bars output does change for dividend-adjusted history (intended, and why the #2133 warehouse rebuild + golden re-pin is required). R1's conclusion is restated on its actual basis: existing warehouses are safe because the default is Raw and prebuilt artifacts are not recomputed, not because the rescale is a no-op. A dividend-only fixture (close 100.0 / adjusted 99.87, no split) now pins that — every other fixture in these suites sets the two equal, which is exactly why the suite could not expose the gap.
adjusted_basis.mli additionally states plainly that the guard is one-sided: a corrupt adjusted_close is unguarded and degrades silently.

Local verification (foreground, exit codes read): dune build @fmt 0, dune build 0, full dune runtest 0.

Filed, not fixed here (reported back to the dispatcher rather than grown into this PR): (1) _factor's asymmetric guard on adjusted_close; (2) Svg_series._to_adjusted_basis still carrying a byte-equivalent private copy of the rescale; (3) the odoc ref to that _-private symbol, which folds into (2).

[dayfine]

Reviewed SHA: 77e5b57

Structural QC — sketch-adjusted-basis (SCOPED DELTA RE-REVIEW)

This is a scoped delta re-review, not a fresh full structural review.

• I previously reviewed this PR at ae52b3f1 (review id 4795521335) and returned APPROVED, quality 4.
• Since then: one rework commit (eaba3891) plus a clean merge of main (tip 77e5b57b).
• What this review covers: the ~296 lines of brand-new test code in the delta, the new test/dune stanza, and re-verification of A2/A3 against the current tip.
• What this review does NOT re-derive: the production logic of adjusted_basis.ml / resistance_sketch.ml / the reader chain. That rests on the prior APPROVED, which transfers because I verified the production delta is comment/docstring-only — zero executable production lines changed (git diff ae52b3f16 77e5b57b1 over the five lib/ files).
• Files differing between the two SHAs under dev/lib/run-in-env.sh, trading/devtools/checks/*, docs/design/*, dev/daily/*, dev/health/* arrived via the merge of main (#2143/#2146/#2147). Confirmed against GET /pulls/2145/files (31 files) — none of those paths are PR content.

Hard gates — verified, not predicted

I did not run the gates locally (three concurrent builds on a 4-CPU box OOM into a misleading exit 137). Instead I queried and read GET /repos/dayfine/trading/commits/77e5b57b12a9a934bc18440649650b659f0f44c5/check-runs and confirmed the completed results on this exact tip. This is strictly stronger evidence than a local run — CI executes the full linter suite (fn_length, nesting, magic_numbers, file_length, mli_coverage, status_file_integrity, no_python) in a clean container:

• build-and-test — completed / success (check-run 90302276692, 2026-07-28T14:27:10Z)
• perf-tier1-smoke — completed / success (check-run 90302276450, 2026-07-28T14:17:35Z)

To be explicit about the distinction: this is a verified pass on the reviewed SHA, not a prediction of one.

Structural Checklist

#	Check	Status	Notes
H1	dune build @fmt	PASS	By CI evidence, cited above — build-and-test success on 77e5b57b. Verified via check-runs API, not predicted.
H2	dune build	PASS	Same run.
H3	dune runtest	PASS	Same run; includes the full linter suite in a clean container.
P1	Functions ≤ 50 lines (linter)	PASS	fn_length_linter green in H3. Longest new fn _drive_screen_tick (~28 lines) is a single call with labelled args.
P2	No magic numbers (linter)	PASS	linter_magic_numbers.sh green in H3.
P3	Config completeness	NA	Delta adds no tunable production values — production delta is comment-only.
P4	Public-symbol export hygiene (linter)	PASS	linter_mli_coverage.sh green in H3. New test files export nothing.
P5	Internal helpers underscore-prefixed	PASS	All new helpers conform: _as_of, _bar, _equal_bar, _bits, _unscaled_with_adjusted_close, _thread_daily_bars, _recording_cb, _drive_screen_tick, _anchor_fields_read_by_screen.
P6	Test-pattern conformance (.claude/rules/test-patterns.md)	PASS	Applied all three greppable sub-rules to both new blocks — see detail below. Zero hits in new code.
—	New test/dune stanza correct + minimal + actually wired	PASS	See detail below.
A1	Core module modifications	PASS	Delta touches no portfolio/, orders/, position/, strategy/lib executable code. Production delta is comment-only.
A2	No new analysis/ → trading/trading/ imports	PASS	Only two dune files differ; trading/devtools/checks/dune is from the main merge (#2143), not PR content. The PR's own analysis/.../test/dune change adds no libraries — only a test name. Its pre-existing trading.data_panel.snapshot dep is the always-permitted reverse direction (analysis consuming trading).
A3	No unnecessary non-feature module modifications	PASS	Computed from GET /pulls/2145/files (31 files), not a git-log ancestry walk, and re-fetched origin first. All 31 are in-scope snapshot/sketch/basis paths plus dev/status/weekly-snapshot.md (a per-track status file — permitted; dev/status/_index.md untouched).

P6 detail — the three sub-rules, on new code only

Both new blocks open Matchers and are clean of all three patterns (an open Matchers that still contained one would be a FAIL, not a PASS):

1. List.exists ... equal_to true/false — zero hits in either file.
2. let _ = ...on_market_close / let _ = ....run — zero hits. This is the sub-rule most at risk here, since _drive_screen_tick calls Internal_for_test.on_market_close. Its result is returned, threaded out through _anchor_fields_read_by_screen, and asserted (field fst is_ok / field snd is_ok, test_panel_callbacks.ml:938-940). Correct handling, not a discard.
3. match ... with | Error ... -> assert_failure / bare | Ok ... -> — six assert_failure hits exist in test_panel_callbacks.ml at lines 209, 427, 430, 472, 607, 610, but all are pre-existing, below the new block which starts at line 814. awk 'NR>=810 && /assert_failure/' returns nothing. New code has zero.

Composition is idiomatic throughout: one assert_that per value, checks composed via all_of + field + elements_are, no nested assert_that inside a matcher callback (grep for fun .* -> .*assert_that: no hits).

Two things I want to call out as genuinely good rather than merely compliant:

• test_adjusted_basis.ml asserts IEEE-754 bit patterns (_bits, Int64.bits_of_float) where the contract is bit-identity, rather than a float_equal with an epsilon — which by construction cannot distinguish identity from a 1-ulp drift. That is the right instrument for the claim being pinned.
• The integration assertion is self-guarding against vacuity. test_sidetable_basis_threads_through_screening asserts elements_are [equal_to Adjusted_close] on the recorded field list — exactly one element. If the synthetic series failed to drive the stock past Phase 1 and the sketch reader were never reached, seen would be empty and the test would fail, not silently pass. That is the correct defence against the "test that doesn't actually test" failure mode.

test/dune detail — is the new test genuinely wired into dune runtest?

Yes, verified. The stanza change is a one-line addition of test_adjusted_basis to the existing (tests (names ...)) list. Dune's tests stanza attaches every listed name to the runtest alias, and the file ends with let () = run_test_tt_main suite (line 158) with all seven cases registered in suite (lines 140-156). The suite is reachable and executes.

The stanza is also minimal: no libraries were added. Everything the new file needs (core, matchers, ounit2, types, weinstein.snapshot_pipeline) was already declared for its sibling tests. This is the correct shape — contrast the #2143 finding where a test was added but never wired.

Test-code quality

Positives beyond the above: helper naming is consistent and self-documenting; fixture constants are semantically meaningful OHLC values rather than arbitrary literals; derived expectations are written as derivations (104.0 *. 0.9987 at test_adjusted_basis.ml:134) instead of pre-computed literals, so the intent survives; there is no duplication between the two new blocks (one is a unit-level guard suite, the other an integration thread — disjoint concerns); and the block comments explain why each contract is unreachable from the existing suites, which is the part that usually rots.

Worth noting that the rework's production doc-comments correct a previously misleading claim — resistance_sketch.ml used to say the rescale was "a no-op for split-free data", now correctly narrowed to bit-identity exactly when adjusted_close = close_price, with the dividend-only case called out. test_dividend_only_adjustment_is_not_a_no_op pins precisely that correction. Docs and tests moved together; that is the right pattern.

FLAGs (do not block approval)

• test_panel_callbacks.ml is now 973 lines. linter_file_length.sh scopes to */lib/*.ml and exempts test files entirely, so this draws no linter signal — judging it manually per the review contract. The 16-case "Panel_callbacks parity" suite is cohesive and this delta contributed only 138 lines, so I am not blocking on it. But it is past the point where a reader can hold it in one pass, and .claude/rules/code-health-discipline.md treats size growth as a leading indicator rather than something to address on failure. Suggest splitting the parity suite (e.g. sketch/basis threading vs. volume/RS parity) before the next block lands in this file.
• test_adjusted_basis.ml:128-131 collapses a comparison to a boolean — field (fun r -> Poly.equal (_bits r) (_bits b)) (equal_to ~msg:... false). This does not match P6 sub-rule 1 (it is not List.exists) and is mitigated on both sides: a custom ~msg is supplied, and the same all_of immediately pins the concrete high_price / close_price values, so a failure is diagnosable. Noting it only as a style nit for the direction of travel.
• module FL = Portfolio_risk.Force_liquidation (test_panel_callbacks.ml:33) is a two-letter alias used exactly once. Prefer the qualified path or a fuller alias.

Quality Score

4 — New tests are well-instrumented and non-vacuous (bit-level identity, self-guarding integration assertion), P6 fully clean and the dune wiring correct and minimal; held off 5 by the 973-line test file and two small style nits.

Verdict

APPROVED

(Delivered as event: COMMENT because the review bot cannot self-APPROVE; the verdict above is the authoritative structural result. No FAIL items — the three FLAGs above are non-blocking.)

[dayfine]

Reviewed SHA: 77e5b57

Behavioral QC — split-safe sketch basis (#2133 defect 2) — re-review after rework iteration 1

Re-reviewed by re-running the mutations myself in an isolated worktree at this tip, not by
reading the rework diff. Baseline analysis/weinstein/snapshot_pipeline/test and
trading/weinstein/strategy/test both exit 0 (test_adjusted_basis.exe: 7 tests).
Seven mutations this pass, 6 killed / 1 surviving (the survivor is non-material — FLAG below).

Mutation ledger (this pass)

ID	Mutation	Exit	Killed by
M3	delete the corrupt-close guard; _factor becomes bare b.adjusted_close /. b.close_price	1	NaN raw close admits no factor, Zero raw close…, Negative raw close… (3 failures)
N1	boundary weakening: Float.( <= ) b.close_price 0.0 → Float.( < ) …	1	Zero raw close admits no factor (1 failure, exactly the boundary)
N2	drop only the `Float.is_nan b.close_price		` disjunct
N3	sub-epsilon drift: high_price = b.high_price *. f *. (1.0 +. 1e-15)	1	Equal closes is bitwise identity, Idempotent on an already-adjusted bar (+4 other suites)
M4	delete ~sidetable_basis:(Bar_reader.sidetable_basis bar_reader) at weinstein_strategy_screening.ml:144	1	Side-table basis threads through the screening path (1 failure; 14/15 other tests passed)
N4	Panel_callbacks ?sidetable_basis default Raw→Adjusted (panel_callbacks.ml:243)	1	Sketch-v5 side-table threading selects v5 vs dense
N5	Bar_reader.of_snapshot_views ?sidetable_basis default Raw→Adjusted (bar_reader.ml:179)	0 — SURVIVES	none — non-blocking, see FLAG

M3 and M4, the two survivors of the previous pass, are both dead. N1/N2 additionally show each
disjunct of the guard is independently pinned, which is stronger than the required minimum.
N3 confirms the R1 bit-identity assertion is genuinely bitwise: a 1e-15 relative drift is ~1e-13
absolute at these magnitudes and would pass a float_equal ~epsilon:1e-9; it fails
Int64.bits_of_float. The test asserts identity, not approximation.

On the author's pushback re: M4 — the author is right, and my prior review was wrong

My previous required fix said: "add one test in test_panel_callbacks.ml that calls
stock_analysis_callbacks_of_weekly_views with ~snapshot_cb, ~weekly_sidetable, and
~sidetable_basis:Adjusted … That kills M4." That last sentence is false, and I am
recording it as a defect in the review rather than in the code.

M4 deletes the argument at the screening call site. A test that itself supplies
~sidetable_basis to stock_analysis_callbacks_of_weekly_views never executes
weinstein_strategy_screening.ml:144, so it structurally cannot observe the deletion — it would
assert on a basis it passed in by hand. Empirically corroborated: under M4, 14 of 15 tests in
the suite passed, and the one pre-existing test in exactly the shape I prescribed
(test_weekly_sidetable_threading, which calls stock_analysis_callbacks_of_weekly_views
directly with ~snapshot_cb + ~weekly_sidetable) passed cleanly. Only the author's
Internal_for_test.on_market_close-driven test failed.

Driving the real screening path and recording every read_field the run issues is the only shape
that can kill this mutation, and the recorder is sound precisely because
Resistance_sketch_reader is the sole read_field caller in the tree (every bar/view read goes
through read_field_history), so the recorded fields are exactly the anchor column selected.
The author correctly rejected an under-specified prescription and substituted a stronger one.
A prescribed fix that does not kill the mutation it was written for is a review defect; this one
was.

On finding 3 (wording) — also partly mis-scoped by me

I claimed the false "no-op for split-free data" wording appeared in three places including the
weekly_sidetable_builder.ml header. It did not:
git show ae52b3f1:…/weekly_sidetable_builder.ml | grep -n "no-op\|split-free\|bit-" returns
nothing, and the file is byte-unchanged since ae52b3f1. Its header describes the adjusted basis
and makes no bit-identity claim, so there was nothing to fix there.

The two real code sites are corrected, and the author corrected four sites rather than the two
that existed: resistance_sketch.mli:63-70 (compute — now states plainly that it does not
rescale) and :96-99 (compute_windowed), resistance_sketch.ml:191, plus
adjusted_basis.mli:29-33 and live_resistance_sketch.mli:28-33. PR body §R1 is rewritten and now
states the true reason existing warehouses are unchanged (the Raw default + non-recomputation of
prebuilt artifacts), explicitly disclaiming the no-op explanation.

I verified the replacement statement is true in both directions, since this is the load-bearing
claim for every prebuilt golden: f = adjusted/close = x/x is exactly 1.0 for finite non-zero
x and h *. 1.0 == h bitwise, so adjusted_close = close_price ⇒ bit-identity; conversely if
adjusted_close ≠ close_price the bar cannot be bit-identical because close_price := adjusted_close
differs, even where the ratio rounds to 1.0 and O/H/L happen not to move. "Exactly when" is correct.

Contract Pinning Checklist

#	Check	Status	Notes
CP1	Each non-trivial claim in new .mli docstrings has an identified test that pins it	PASS	Both previously-unpinned claims are now pinned. (a) bar_reader.mli:186-193 / panel_callbacks.mli threading claim → test_sidetable_basis_threads_through_screening (kills M4, verified). (b) adjusted_basis.mli "Idempotent" → test_idempotent_on_already_adjusted_bar (kills N3). (c) adjusted_basis.mli bit-identity precondition → test_equal_closes_is_bitwise_identity (7 magnitudes, kills N3). (d) the previously false resistance_sketch.mli precondition is now factually correct and pinned by test_dividend_only_adjustment_is_not_a_no_op. Claims carried over from the prior pass (hash→basis mapping, unknown-hash raise, both-hash load_gated, anchor-column selection, distinct hash literals, builder/compute_windowed/live rescale) remain pinned, each with a killing mutation on record.
CP2	Each claim in PR body "Test plan"/"Test coverage" has a corresponding committed test	PASS	All five original Test-plan items still map to real, load-bearing tests. The rework added one new PR-body claim — R1's "verified across seven magnitudes (1e-8 … 1e9) and now pinned by test_adjusted_basis.ml" — and test_equal_closes_is_bitwise_identity carries exactly seven: [1e-8; 0.01; 1.0; 3.7; 123.456789; 99_999.99; 1e9]. No PR#478-style phantom claims.
CP3	Pass-through / identity / invariant tests pin identity, not just size_is	PASS	_equal_bar is whole-record equal_to ~cmp:Types.Daily_price.equal; the identity/idempotence tests compare full 4-tuples of Int64.bits_of_float (elements_are (List.map … equal_to)). No size_is-only identity assertion anywhere in the new tests. This is the strongest form CP3 asks for.
CP4	Each guard named in code docstrings has a test exercising the guarded-against scenario	PASS	The corrupt-close guard is pinned three ways — M3 (whole guard), N1 (boundary), N2 (NaN disjunct) — all killed, so each disjunct is independently covered. adjusted_basis.mli "Pinned by test_adjusted_basis.ml" is accurate. The one-sided-guard asymmetry took the documented-not-fixed branch of my required fix: adjusted_basis.mli:21-27 now states plainly that only close_price corruption is handled, that a corrupt adjusted_close poisons the bar (nan→all-NaN, 0.0→zeroed, negative→negative), and names the silent-degradation mechanism (a NaN weekly high never compares above the anchor, so the week is dropped from the histogram). adjusted_basis.ml:3-8 carries the matching comment. Accepted, with the cleanup backlog item below.

Behavioral Checklist (domain)

Per .claude/rules/qc-behavioral-authority.md §"When to skip this file entirely": the rework delta
is test-only plus doc-comments on a data-plumbing / basis-selection change. All S*/L*/C*/T* rows
are NA — no stage-classification, stop, or screener-cascade logic is touched. The S5 rationale
recorded in my prior review (this corrects the overhead-resistance input to the buy criterion so
historical supply and the breakout anchor lie on one continuous scale, per
weinstein-book-reference.md §4.3 Overhead Resistance) is unchanged and still PASS. A1 — NA,
qc-structural did not flag it.

Test-pattern conformance (.claude/rules/test-patterns.md)

Both new files conform: one assert_that per value, composition via all_of / field /
elements_are, no assert_that nested in a matcher callback, and no bare
match … | Error _ -> assert_failure in the new code.
test_sidetable_basis_threads_through_screening's two assert_that calls are on two distinct
values (the result pair, then the field pair), which is the rule, not a violation.

Nit (non-blocking, no rework required): test_dividend_only_adjustment_is_not_a_no_op asserts
field (fun r -> Poly.equal (_bits r) (_bits b)) (equal_to ~msg:… false) — a boolean folded into
equal_to false, the same smell qc-structural's P6 sub-rule 1 targets. matchers.mli:85 provides
not_, so field _bits (not_ (equal_to (_bits b))) reads better and reports better on failure.
Likewise pair (matchers.mli:241) would replace the field fst / field snd tuple pattern.
Cosmetic only — the assertion is correct and load-bearing as written.

FLAG (non-blocking) — N5: the Bar_reader Raw default is unpinned

bar_reader.ml:179's ?(sidetable_basis = Weekly_sidetable_reader.Raw) can be flipped to
Adjusted with trading/weinstein/strategy/test staying green (exit 0). The new screening test
cannot see it because it passes ~sidetable_basis explicitly, and
test_weekly_sidetable_threading builds callbacks via Panel_callbacks rather than Bar_reader
(which is why it catches N4 but not N5).

Not blocking, because the default is unobservable everywhere it applies. I checked both
production side-table consumers and both resolve the basis from the manifest hash and pass it
explicitly: snapshot_warehouse_reader.ml:76-80 and panel_runner.ml:214-222, each via
Weekly_sidetable_reader.basis_for ~manifest_format_hash. Every caller that omits
~sidetable_basis also supplies no weekly_sidetable_loader, so weekly_sidetable_for returns
None, the sketch takes the dense path, and sidetable_basis is never consulted. The substantive
part of the .mli claim — None hash ⇒ Raw — is pinned by test_basis_for_recognized_hashes.
So this is belt-and-braces coverage, not a live gap. Worth adding a Bar_reader-level default
assertion if this file is touched again; recorded here rather than filed, to avoid backlog noise.

Follow-ups from the prior review — confirmed captured, not dropped

Both non-blocking items are filed in dev/status/cleanup.md §Backlog with source attribution to
this PR, as required: :34 adjusted_basis_guard_asymmetry (the one-sided guard, incl. the
silent-drop mechanism) and :35 rescale_duplication (Svg_series._to_adjusted_basis still
carries a byte-equivalent private copy that was not switched over, folding in the unresolvable odoc
ref {!Weinstein_snapshot.Svg_series._to_adjusted_basis} to a _-private symbol). Correctly
captured as follow-ups rather than silently dropped.

Quality Score

4 — All contract-pinning checks pass on re-verified mutations; the rework exceeded the required minimum (per-disjunct guard coverage, genuinely bitwise identity assertions that survive a sub-epsilon mutation, four wording sites corrected rather than two) and correctly rejected an erroneous prescription with a sound structural argument. Held back from 5 only by a cosmetic matcher nit and the unpinned Bar_reader default.

Verdict

APPROVED