Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
192 changes: 192 additions & 0 deletions analyses/phase3_log.qmd
Original file line number Diff line number Diff line change
Expand Up @@ -1631,3 +1631,195 @@ result, and a reasonable place to stop reviewing rather than a claim that nothin
**Validation.** Trainer **121 pass**, sizing 20, overrides 22, backfill 4; ruff + black clean.

**Commit.** Feature branch `p3-06-stage2-lora`; squash SHA derived at the phase-exit gate.

## Step P3-07 — 2026-08-03: the recalibration stack, and two numbers it refuses to invent

**Inputs.** `calib/temperature.py::{fit_temperature, apply_temperature, STACK_ORDER}` (P2-13,
the exact convex β = 1/T bisection); the P3-02 `calib` column contract (859 positives + 230
decoys, every one on the `fold_random == "train"` rung, asserted disjoint from val/test at
build time); the Stage-2 binary-head contract `Stage2Model.forward -> "tbox_logit"`, shape
`(B,)`, one raw logit. **Process.** `src/tbox_finder/calib/recalibrate.py` ships the ADR-0005
D11 stack for the Stage-2 head: `temperature_scale` fits T, `prior_shift` applies the
Saerens/Elkan log-odds correction, and `calibrated_posterior` returns **both** the named
posterior (temperature-scaled, pre-shift — the GATE-2-gated object) and the prior-shifted one,
with `gated_posterior_key` naming the former inside the payload. **Outputs.** One module, one
test module (108 tests), no data artifact and no fitted T — see the last paragraph.

**There is one temperature fit in this repo, and this is not a second one.** Stage 2's head is
a single logit; stacking it as `[0, z]` makes P2-13's multi-class fit *identically* the binary
one, because `softmax([0, βz])[1] = σ(βz)`, so its NLL is exactly the binary cross-entropy at
`βz` — the same reduction `stage2/losses.py` already uses to route the binary term through the
audited focal-CE kernel. The adapter therefore touches the *inputs* and never the arithmetic,
and P2-13's two degenerate-limit refusals (perfect separation ⇒ β → ∞; worse-than-uniform ⇒
β → 0) carry through unchanged. The cost of delegating is that "A agrees with B" becomes a
tautology, so the primary test is neither: for `z = [+a, +a, −a]`, `y = [1, 0, 0]` the
stationarity condition `Σ z_i(σ(βz_i) − y_i) = 0` collapses to `3σ(βa) = 2`, giving
**`β* = ln2/a` exactly**. A second case at a second scale (`4σ = 3` ⇒ `β* = ln3/a`), a ternary
search in the *test* that minimises the value rather than rooting the derivative, and a
manufactured-miscalibration draw that recovers `T ≈ 2.5` from 20,000 rows are the independent
confirmations; the delegation check is present and labelled secondary.

**"T fit only on `calib`, never test" is a property of the code, not a claim about the caller.**
`temperature_scale` takes the whole table plus a per-row rung label and selects the `calib`
rows itself, so there is no argument through which a graded row can reach the fit. Three
refusals surround that: an **empty** selection raises with the full census (the all-`False`
calib column filters clean and fits on nothing, which reads exactly like a working filter); an
**unrecognised rung token** raises rather than being skipped (a row that silently vanishes
shrinks the calibration set while every count still looks fine); and a **single-class** calib
rung raises. `assign_rung` re-derives the P3-02 invariant — a calib row lives in the `train`
rung — at the point of use, and tests missingness with `masking.is_missing`, because
`bool(float("nan")) is True` and under the training env's pandas 3 a null cell arrives as NaN,
so the naive guard would promote a row with **no** calibration flag straight into the fit.

**The fixtures are asymmetric on purpose.** The rung arms are 7 calib / 4 train / 3 val / 5
test, and both the calib and test arms are independently fittable, so the inversion test can
run both ways round: swapping the two senses is asserted to change `n_fitted` *and* to produce
exactly the T of the test arm fitted alone. Balanced arms with count-only assertions would
refuse the right number of the wrong rows and pass.

**Two numbers this step declines to produce, and why that is the deliverable rather than a
gap.** (1) **No deployment prior is pinned.** PRD.md:188 and ADR-0005:80 give the genome-scale
prevalence only as prose, `~10³–10⁴:1` (negatives per positive); nothing in the repo encodes a
value. Pinning a scalar would be a new blinded-frozen default needing ADR sign-off, and
defaulting one silently would put a fabricated quantity into every downstream posterior. So
both priors are required keyword arguments with no defaults, a half-specified shift raises, and
`DEPLOYMENT_PRIOR_RANGE` exists only as the band a supplied prior is *reported* against — a
predicate shown to discriminate, since ADR-0005 D7's own `100:1` benchmark prevalence must fall
outside it. (2) **No T is fitted here.** Nothing in the repo reads a Stage-2 checkpoint back —
`stage2_heads.pt` and `lora_adapter/` have write sites and no readers, and `train.py::evaluate`
discards per-row logits — so real `calib`-split logits do not yet exist. This step ships and
certifies the machinery; producing the scores, and with them the first real T, belongs to the
step that first needs a calibrated posterior (P3-08/P3-10). A T invented to fill the gap would
look exactly like a measured one.

**Path drift, recorded not silently taken.** imp.md names
`src/tbox_finder/calibration/recalibrate.py`; the module lands at
`src/tbox_finder/calib/recalibrate.py` because a `calib/` package already exists and two
near-homonymous calibration namespaces is a permanent confusion. imp.md's `calibration/` paths
have drifted once already on their own — `calibration/ece.py::binned_ece` shipped as
`metrics.py::binned_ece` at P0-31 — and P3-02 carries an identically-shaped recorded
correction. The P3-07 block in imp.md now records this one.

**Validation.** `test_recalibrate` **108 pass**, `test_temperature` 81 pass (unchanged), full
unit tier 3,125 pass with the same 12 failures the untouched `main` shows in this runner (a
py3.11/numpy-2.4.2 local env, not the pinned CI stack); ruff 0.15.15 + black 25.11.0 clean.
**15 source sabotages, each RED against its NAMED test, each restored byte-identically** (fold
sense inverted, emptiness guard removed, unknown-token refusal removed, `is_missing` weakened
to `is None`, P3-02 invariant dropped, shift direction inverted, `+` flipped to `−`, gated key
repointed at the shifted posterior, a deployment prior given a default, the half-specified
guard removed, the PRD band widened to swallow D7's `100:1`, the branch-wise sigmoid replaced
by the overflowing one, single-class guard removed, priors of exactly 0/1 admitted,
`prior_from_odds_ratio` inverted).

**Decisions.** No new ADR and no §7 item-2 sign-off: D11 pins the stack order and the named
posterior, both of which this implements verbatim, and the one value that would have needed a
pin is the one the module refuses to supply. **Commit.** Feature branch
`p3-07-calibration-stack`; squash SHA derived at the phase-exit gate.

## Step P3-07 (review r1) — 2026-08-03: two fail-open guards, and a boolean parse that was already written

**Two review sources, deduplicated to six fixes.** CodeRabbit r1 on `88879fd` returned 1
actionable + 1 nitpick; a 44-agent adversarial pass over the same diff returned 20 findings, 10
surviving two independent refuters. The overlap is the interesting part: **both** independently
found the same defect at the `int64` cast, from opposite directions.

**Fix 1 — the "labels must be binary 0/1" guard was vacuous.** It ran *after*
`y.astype(np.int64)`, which truncates toward zero, so the check saw only what the cast had
already produced. Demonstrated, not argued: `temperature_scale(z=[1,1,−1], y=[1.0, 0.4, 0.6])`
**succeeds** and returns `T = 1.4427030259199125`, bit-identical to the fit for `[1,0,0]` — a
target the caller wrote as `0.6` was silently scored a negative. A `NaN` label cast to the
int64 sentinel and surfaced as `range [-9223372036854775808, 1]`, a message pointing at the
wrong problem. The domain check now happens in the original dtype: booleans and integers pass,
floats must be finite and integral, any other dtype is refused by name.

**Fix 2 — `bool(calib)` closed the NaN hole and left the string hole open.** The guard existed
*because* pandas 3 delivers a null as `NaN` and `bool(nan) is True`; one step further out,
`bool("False")` and `bool("0")` are also `True`. A string-typed `calib` column would therefore
promote its explicit **negatives** into the set `T` is fitted on, with every count agreeing.
Reproduced end to end: `rung_labels(calib=["True","False",…], …)` returned `["calib"]*6`.

**And the fix for it was already in the repo, which is the actual lesson.**
`stage2/train.py::_bool_or_none` is exactly this parse — tri-state, `masking.is_missing`-based,
`{"true","1"}/{"false","0"}`, raising on anything else — written for the fold columns and
documented as also reading `calib`. It could not be imported here (that module is
torch-adjacent; this one is deliberately numpy-only), so the parse was **promoted down** to
`masking.py`, beside the `is_missing`/`row_text` it completes, with `NULL_TOKENS` moving with
it. `stage2.heads.NULL_TOKENS` is now a re-export and `stage2.train._bool_or_none` a
delegation: one vocabulary, one parse, three call sites. Writing a fourth copy here would have
been the cheaper diff and the wrong one
([[promote-dont-duplicate-is-a-correctness-rule]]).

**Fixes 3–6, all test-side, all found by sabotage rather than by reading.** (3) The
single-class guard is a **disjunct**, `n_pos == 0 or n_pos == n_fitted`, and only the
all-positive half was tested — with the other half deleted, an all-negative calib carve (the
realistic P3-02 join failure) returns a **cleanly converged, fully certified** `T`. Both halves
are now parametrized. (4) `test_shape_and_domain_violations_raise` used a bare
`pytest.raises(ValueError)`, and both of this module's own guards could be deleted with the
test still green, because the delegated fitter raises its own differently-worded error further
in; it now matches on this module's text. (5) The empty-calib refusal asserted only its first
clause, leaving the census and the `GRADED_RUNGS` count — the only place that constant is read
— unchecked; all three are asserted. (6) The payload's `source_prior`/`target_prior` were
written and never read, so **swapping the two assignments left the suite green** while emitting
an artifact that recorded the deployment prior as the calib prevalence.

**One finding fixed by deletion.** The docstring cited the known-prior closed form as "Elkan
2001, Theorem 1"; a reviewer put it at Theorem 2 (Theorem 1 being the cost-threshold result).
The numbering could not be verified against the paper from here, so the theorem number is
**gone** rather than guessed — a citation pointing at the wrong theorem is worse than one
pointing at the paper, and the DOI'd Saerens reference is the traceable half anyway.

**Not fixed, with reasons.** Ten findings were refuted by their own verifiers and are recorded
rather than acted on — among them "the calib certificate is self-referential and vacuous"
(refuted: the selection is by identity against `CALIB_RUNG`, and the inversion test asserts the
swapped fit equals the test arm's own `T`), "`RUNG_VOCABULARY` is not closed because `.strip()`
admits padded tokens" (`" calib "` is a legitimate parquet spelling of `calib`; `" "` is
refused), and a naming divergence between this module's flat `stack_order` and
`temperature.py`'s nested one (the two artifacts have no coupling path and different
validators).

**Validation.** `test_recalibrate` **138 pass** (was 108); `test_temperature` 81,
`test_masking` + `test_stage2_heads` + `test_stage2_train_smoke` 411 pass together — the
promotion is behaviour-preserving on both sides. Full `unit+golden+ml` tier: the worktree's 14
failures are a **strict subset** of the 31 that untouched `main` produces in this local runner
(py3.11 / numpy 2.4.2, the only local env with pytest), i.e. zero regressions; CI runs the
pinned stack. ruff 0.15.15 + black 25.11.0 clean. **24 source sabotages, each RED against its
NAMED test, each restored byte-identically** (up from 15; the nine new ones cover the strict
boolean parse, all three label-domain branches, both halves of the single-class disjunct,
`GRADED_RUNGS`, the payload prior swap, the payload provenance, and the 1-D logits guard).

**Commit.** Feature branch `p3-07-calibration-stack`; squash SHA derived at the phase-exit gate.

## Step P3-07 (review r2) — 2026-08-03: one finding, and the GitHub check that lied about it

**The gate check said `pass`; no review had run.** After the r1 push, the `CodeRabbit` PR check
went green with the body *"Review rate limited"*, and `pulls/100/reviews` showed exactly one
review, on the **step** commit — `a721d06` was unreviewed. This is the CLAUDE.md §5.1 trap
verbatim: a rate-limit notice is an **absence**, not a pass. Rung 1 of the ladder — the
CodeRabbit **CLI**, a separate path from the throttled GitHub app — ran the round instead,
scoped with `--committed --base-commit 88879fd` to the six files r1 actually touched.

**One minor finding, and it is right.** `masking.bool_or_none` — the parse r1 had just promoted
— stringifies before matching, so a **float** flag becomes `"1.0"`/`"0.0"`, matches no spelling,
and raises. That looked at first like correct fail-closed behaviour on a schema fault, until the
shape was checked: `pd.Series([True, None, False]).astype(float)` and a `boolean`-dtype cast to
float64 **both** produce `1.0 / nan / 0.0`. The NaN was already the missing case; the `1.0` and
`0.0` beside it are a well-formed nullable-boolean column, and refusing them aborts a legitimate
run. So the parse now accepts a **finite Real equal to exactly 0 or 1** before the text path.
`0.5`, `1.5`, `-0.5`, `2` and `-1` still raise — they are schema faults and nothing about this
widening touches them.

**Note the inherited-behaviour point.** This was not introduced by P3-07: it is the behaviour
`stage2/train.py::_bool_or_none` has had since P3-06, now visible because the parse moved to a
shared home and got read by a reviewer. Fixing it once fixes it for the fold readers too, which
is the whole argument for having promoted it rather than forked it.

**Validation.** `test_recalibrate` **146 pass**; `test_masking` + `test_stage2_heads` +
`test_stage2_dataset` + `test_stage2_train_smoke` + `test_temperature` + `test_no_leakage` = 502
pass beside it. Full `unit+golden+ml`: **14 failures, still a strict subset of `main`'s 31** in
this runner — `comm -13` over the two `^FAILED` sets is empty, so zero new failures, asserted
rather than eyeballed. ruff + black clean. **Two more sabotages, both bitten and both
directions**: removing the acceptance turns the float test red, and widening it to *any* finite
real (so `0.5` reads `True`) turns the refusal test red — a one-sided sabotage would have missed
the second, which is the failure that actually matters here.

**Commit.** Feature branch `p3-07-calibration-stack`; squash SHA derived at the phase-exit gate.
11 changes: 10 additions & 1 deletion src/tbox_finder/calib/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,17 @@
(per-class one-vs-rest binned ECE through the frozen ``metrics.binned_ece``, with
cluster-blocked CIs). GATE-2's gated ECE is P3-exit business on the P3-02 ``calib``
carve — nothing here is a calibration claim, and no ``T`` fitted here is shipped.
``recalibrate``
The P3-07 stack for the **Stage-2 binary head**: ``T`` fitted on the P3-02 ``calib``
rung (and structurally on nothing else), then the Saerens/Elkan log-odds prior-shift,
then a producer exposing **both** the D11 named posterior (temperature-scaled,
pre-prior-shift — the GATE-2-gated object) and the prior-shifted one (reported,
non-gated). It **reuses** ``temperature.fit_temperature`` rather than carrying a second
temperature fit: Stage 2's one logit stacked as ``[0, z]`` makes that multi-class fit
identically the binary one. No deployment prior is pinned here — the PRD gives it only
as prose, so both priors are required arguments with no defaults.
"""

from __future__ import annotations

__all__ = ["temperature"]
__all__ = ["recalibrate", "temperature"]
Loading
Loading