jc: corrective slice — ω sign erasure + R² scale dependence (both P1, reproduced); C2 audit - #888
Conversation
…ct divergence C2 asks for the dichotomous statistics to be named correctly (Pearson->phi, alpha->KR-20, kappa NOT a renamed ICC, Spearman dropped on binaries), because "reporting Pearson while computing phi" is the defect class it exists to prevent. AUDIT RESULT: that defect has ZERO instances today. The jc reliability battery has exactly four consumers, all jc examples, and none feeds dichotomous data: style-table columns (continuous), rung levels 1-10 (ordinal), palette256-squared vs part_of:is_a path distances (continuous + multi-valued discrete, self-labelled in the example's own output), and i4 loci offsets (0..8). Pearson/alpha/ICC are correctly named at every existing call site. Checked rather than assumed: probe_p1.rs and sigma_probe.rs import other jc modules and never touch the battery; g0_graph_loadbearing.rs matched the initial grep only inside a println! string naming jc::reliability as future work. Binary 0/1 codings DO exist (probe_babel_stances.rs:784,792) but feed a NARS belief arena, not a reliability statistic. So C2 is a FORWARD discipline, not a migration — it binds at the first binary-criteria witness, which is D3's. Half of it is already structural: stats.rs's phi takes &[bool], so "compute phi, call it Pearson" cannot be written. What the audit surfaced instead is larger, and is filed as TD-STATS-DEGENERACY-CONTRACT-DIVERGENCE + an EPIPHANIES entry: 47 hand-rolled pearson/spearman/cronbach_alpha definitions return bare f64, 9 return bare f32, 3 return Option — and all three are jc's own. At least one copy collapses EVERY degeneracy (ragged input, n < 2, zero variance) to 0.0, which is byte-identical to a genuine "perfectly uncorrelated" result. The caller cannot distinguish a finding from a bug in its own input. That is the falsifiability rule one level down: not a guard that cannot fire, but a measurement that cannot fail — the artifact has no way to say "no", having spent its whole output range on answers. Second-order finding: reliability.rs's header says callers rolled their own "until now", presenting consolidation as done. It reached the four new callers and none of the 56 pre-existing definitions. A "we consolidated X" claim is an INVENTORY claim, and inventory claims decay silently — true the day written, drifting every day after. They need a census in the same breath, or the word aspiration. Not paid here: 56 definitions across ~26 files is its own wave, each call site owned by its author, and the migration is a decision about what that site should do when the estimate is undefined — exactly the information the current code throws away. Risk-ordered paydown recorded in the TECH_DEBT entry. Board/plan prose only; no code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K3RyLEbuNSHxxB3NTTrGki
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe change hardens incomplete-beta, omega, and multiple-regression R² calculations. It adds binary association and KR-20 APIs with tests. Planning and technical-debt documents record audit findings, deliverable status, and statistical contract differences. ChangesStatistical corrections and measurement APIs
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Poem
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_0a07a470-16cd-45cd-bedf-f9a8d7243083) |
…dence External review of the merged #887 found two P1 numerical defects. Both were REPRODUCED before fixing; both now carry regression tests. 1. omega_total erased loading SIGNS. The triad identity yields lambda_i^2, hence only |lambda_i|, and the code took the positive root for every item. omega depends on (sum lambda)^2, where a negatively-keyed item must SUBTRACT. On an exact signed-congeneric fixture (loadings [+1,-1,+1], equal orthogonal residuals) the true value is 0.25 and the shipped code reported 0.75. Fixed by recovering signs from the covariance row: under one factor sign(sigma_ij) = s_i*s_j, so anchor s_0 = +1 and read the rest off row 0. The anchor is free because the model is identified only up to a global flip and (sum lambda)^2 is invariant to one — asserted as its own test. Sign CONSISTENCY is now also checked, which is a genuine partial one-factor structure test. At k=3 it is provably subsumed by the existing negative- lambda^2 guard (the sign product of the single triad is -1 exactly when the pattern is inconsistent); it only becomes reachable at k>=4 where lambda^2 is averaged. The can-it-fire test therefore uses a k=4 fixture and pre-registers that every lambda^2 and psi is non-negative, so only the sign guard can be what rejects. 2. multiple_r_squared was SCALE-DEPENDENT. Normal equations were built on raw columns and a pivot was called singular below an ABSOLUTE 1e-12 — a statement about units, not rank. Measured: an exact linear fit at 1e-8 magnitude returned None, while the identical relationship at unit scale returned 1.0. Fixed by centering the response and every predictor and scaling predictors to unit norm. R-squared is affine-invariant so no correct answer changes, the rank test becomes relative (diagonal is exactly 1), and centering absorbs the intercept — so "collinear with the intercept" correctly reappears as two identical centered columns. The [0,1] clamp is now bounded to a rounding-scale band: a materially out-of-range value means the solve failed and surfaces as None rather than as a plausible 0 or 1. Also corrected / added: - omega's doc claimed rejection means "the congeneric model does not fit". It checks three NECESSARY conditions and cannot certify a one-factor matrix; the vanishing-tetrad constraints are not tested and k>=4 misfit can still return a number. Doc now states what is and is not verified. - BinaryAssociation + binary_association: counts, BOTH marginals, p_o/p_e alongside kappa and phi. The shipped phi doc said marginals are required to interpret it while the function returned a lone scalar; that was an internal contradiction. - kr20(&[Vec<bool>]): the dichotomous naming surface C2 asked for and C1b did not ship. Enforces binary input by type, delegates arithmetic to cronbach_alpha, does not duplicate it. - betacf now returns Option and reports non-convergence instead of presenting the 300th iterate as a p-value; reg_inc_beta uses ln_1p(-x) near x=1 and clamps only a rounding-scale excursion. Board corrections in the same commit: - The "unblocks D3's fusion falsifier" claim was WRONG and contradicted the plan's own C3 (validity requires an external criterion). kappa is chance- corrected agreement under the observed marginals; it measures overlap, not incremental value. D3 is split: D3a (descriptive overlap, unblocked) and D3b (held-out fusion falsifier, BLOCKED on an external criterion and a criterion-appropriate scoring rule). - C2's Spearman wording: on non-constant binary variables the average-rank transform is affine, so rho is REDUNDANT with phi, not degenerate. - C4's Jirak scope: jc::stats p-values are classical independent-sample p-values; jirak.rs is a fingerprint-specific empirical probe, not a general uncertainty engine. A dependent-cohort claim needs its own justified dependence model. - The exact-zero epiphany was too broad. Narrowed to theoretically non-negative FITTED quantities needing a scale-aware tolerance; it is not a licence to stop testing zero, since a zero determinant or zero within-group variance is a real degeneracy. - New epiphany E-THE-CROSS-IDENTITY-SUITE-INHERITED-MY-BLIND-SPOT-1: five independent cross-identities all passed while both defects were live, because every omega fixture had positive loadings and every R-squared fixture was O(1). A cross-identity suite inherits the blind spots of its fixtures silently, since every check reports green. The practice that would have caught both: name the invariances the estimator should have (affine rescaling, global sign flip) and write one test per dimension. 116 lib tests + 13 doctests green; stats.rs clippy-clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K3RyLEbuNSHxxB3NTTrGki
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 22405280c9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let scale = (cov[0][0].abs() * cov[j][j].abs()).sqrt().max(1.0); | ||
| if cov[0][j] < -1e-9 * scale { |
There was a problem hiding this comment.
Remove the absolute floor from omega sign inference
When signed-congeneric inputs are expressed in smaller units, this .max(1.0)—and the identical floor in the consistency check—turns the relative covariance threshold into an absolute 1e-9 cutoff. Scaling the added signed fixture by 1e-5 makes its covariances approximately ±1.14e-10, so all negative covariances are ignored, every loading remains positive, and ω changes from 0.25 to 0.75 even though this dimensionless reliability must be invariant to a common rescaling. Use a covariance-relative threshold without the unit floor and cover rescaled signed inputs.
Useful? React with 👍 / 👎.
| // A materially-zero covariance means λ_0 or λ_j is ~0; that item | ||
| // contributes ~nothing to Σλ, so either sign is harmless. Keep +1. |
There was a problem hiding this comment.
Choose a nonzero loading as the sign anchor
When item 0 has a zero or thresholded-near-zero factor loading but later items have mixed-sign nonzero loadings, every cov[0][j] carries no sign, so this logic leaves all signs positive and the subsequent consistency check rejects the negative covariances among those later items. This occurs for a valid congeneric model such as loadings [0, +1, -1, +1]; moving a nonzero item to index 0 makes the same model succeed, so ω is incorrectly permutation-dependent. Anchor on a covariance-connected nonzero item or propagate signs through nonzero covariance edges.
Useful? React with 👍 / 👎.
…its own can-it-fire test was firing on the bug) Follow-up to the #888 corrective slice, from re-reading my own diff. THE DEFECT. Signs were read from item 0's covariance row. Under one factor sigma_ij = lambda_i*lambda_j, so if item 0 is weakly loaded its whole row is near zero, every sign defaults to +1, and the new consistency check manufactures conflicts on data that is perfectly one-factor. Measured with lambda = [0,+1,+1,-1]: two false conflicts, valid data REJECTED. THE WORSE PART. The can-it-fire test I wrote to prove that guard alive was passing BECAUSE of this defect. Enumerating its fixture over every anchor: anchor 0 (|lambda|=0.47): 2 conflicts anchor 1 (|lambda|=1.32): 0 conflicts anchor 2 (|lambda|=0.82): 2 conflicts anchor 3 (|lambda|=1.84): 0 conflicts <- strongest A valid sign assignment exists, so the fixture is CONSISTENT and the guard was never shown to fire — what fired was the anchor bug. A test written to prove feature X correct passed because of unnoticed defect Y in the same function; the pair was self-confirming. Root cause: the test reused the implementation's own anchor convention to decide what "inconsistent" means, so it could only ever agree with it. THE FIX, which is principled rather than a patch: anchor on the strongest-loading item. Under a true single factor a near-zero covariance against the strongest item implies the OTHER item's loading is ~0 — whose sign cannot matter, since it contributes ~0 to sum(lambda). So the strongest anchor is correct under the model, not merely more robust. THE TEST, rebuilt on a criterion independent of the implementation: sign-consistency is a property of the PATTERN, not of a chosen reference. The honest question is whether ANY assignment of item signs reproduces every covariance sign; a pattern where none does is frustrated (an odd cycle of negative edges). The new fixture is frustrated from EVERY anchor, so no implementation convention can accidentally satisfy it, and the test still pre-registers that neither the negative-lambda^2 nor the Heywood guard can be the rejecter. Plus a regression test that the lambda = [0,+1,+1,-1] set is accepted. Recorded as E-THE-CAN-IT-FIRE-TEST-WAS-FIRING-ON-THE-BUG-1, with the carry-over rule: a can-it-fire test must define its trigger from the SPECIFICATION, never from the implementation's own decision procedure — and when a guard rejects, ask whether the rejection is a property of the data or of an arbitrary choice the code made along the way. 117 lib + 13 doctests green; stats.rs clippy-clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K3RyLEbuNSHxxB3NTTrGki
|
Post-merge review found two live P1 — ω is still unit-dependentThe new sign cutoffs and both λ² / ψ tolerances use a covariance-scale expression with an absolute floor: ...sqrt().max(1.0)
// and
cov[i][i].abs().max(1.0)Below unit variance this stops being relative. Scaling the PR's own signed-congeneric fixture by The same floor weakens the model guards. A Heywood violation that rejects at unit scale can be scaled below Required shape: tolerances must stay in the quantity's units, e.g. covariance sign decisions from a dimensionless correlation threshold (without P1 — anchoring signs to item 0 rejects a valid zero-loading item
An exact Walsh-basis construction: let f = [ 1.,-1., 1.,-1., 1.,-1., 1.,-1.];
let e0 = [ 1.,-1., 1.,-1.,-1., 1.,-1., 1.];
let e1 = [ 1., 1.,-1.,-1., 1., 1.,-1.,-1.];
let e2 = [ 1.,-1.,-1., 1., 1.,-1.,-1., 1.];
let e3 = [ 1., 1., 1., 1.,-1.,-1.,-1.,-1.];
let items = vec![
e0.to_vec(), // λ0 = 0
(0..8).map(|i| f[i] + e1[i]).collect(), // λ1 = +1
(0..8).map(|i| -f[i] + e2[i]).collect(), // λ2 = -1
(0..8).map(|i| f[i] + e3[i]).collect(), // λ3 = +1
];This is an exact one-factor model with equal orthogonal residuals. True ω is Current code returns Required shape: choose an anchor known to carry a material loading, such as the largest estimated P2 —
|
…sign oracle Addresses all three findings from the #889 review (4856837957) and the #888 post-merge note (5181899915). Each was reproduced before fixing. P1 — omega was still UNIT-DEPENDENT. Four absolute floors survived in omega_total: `.max(1.0)` in the lambda^2 guard, the psi guard, and both sign cutoffs. Below unit variance those degrade to an absolute 1e-9, so on data whose covariances are ~1e-16 every tolerance is enormous, every sign comparison vacuous, and (measured) omega flips 0.25 -> 0.75 purely by rescaling. All four removed; tolerances are now relative to the item's own variance, and covariance cutoffs to sqrt(var_i * var_j). This is the SAME defect class as the R-squared absolute pivot fixed two commits earlier. I fixed the instance and left the class — the floors were in the function I was actively editing. Constant items (zero variance) are now rejected explicitly: they carry no signal, leave the model unidentified, and would make every scale-relative tolerance degenerate. P1 — the regressions blessed the old wrong answer. The zero-loading test asserted only `(0.0..=1.0).contains(&w)`, which 0.75 also satisfies, so it proved "did not reject" rather than "computed the signed omega". Worse, the invariance tests compared against a SELF-COMPUTED baseline, so a consistently wrong implementation would satisfy them at every scale and permutation. Both absolute values are now pinned as constants derived by hand: ZERO_ANCHOR_OMEGA = 0.25 (lambda = [0,+1,-1,+1], psi = [V,0,V,V]) CONGENERIC_OMEGA = 18/19 (exact rational; the 8-digit decimal is 1.05e-9 off and a 1e-9 invariance tolerance correctly rejects it) and the rescale/permutation tests assert against those, not against themselves. Invariance alone never pins a value. P2 — the "independent" frustration oracle mirrored the implementation. It reconstructed signs from each anchor row and looked for conflicts, which is an anchor-shaped decision procedure, not the specification. Replaced with the literal predicate: fix the free global sign and enumerate all 2^(k-1) = 8 assignments, asserting NONE satisfies every materially non-zero covariance edge, plus an anti-vacuity assert that the sign graph carries >= 4 material edges. Also removed (review): the global-observation-sign-flip test. Negating every item leaves the covariance matrix bit-identical, and omega_total consumes only that matrix, so the assertion could not fail for ANY implementation — the workspace's own "assertion implied by the code it tests". Replaced by common rescaling, item permutation (including rotations that move the zero-loading item into index 0), and a single-item flip that must CHANGE omega. 118 lib + 13 doctests green; stats.rs clippy-clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K3RyLEbuNSHxxB3NTTrGki
…ured Horizontverschmelzung
Operator-directed. Wires the whole Bible as ONE 64k SoA bake through
cycle_driver so 64k thought bodies fire per cycle, with the four shipped
stances (Hegel/Nietzsche/Kant/Wittgenstein, already in probe_eyes_opened.rs)
as READS over the sealed version series, and Gadamer's Horizontverschmelzung
as a measured trajectory rather than a label.
THE ONE ARCHITECTURAL DECISION, made rather than offered. The operator gave two
shapes: six parallel SoAs (base + Gadamer + four lenses), or one sealed series
read as a time series. The six-SoA shape is REJECTED, and not on cost grounds:
four lens SoAs would be copies of the same 64k verse rows differing only in
which stance reads them. A stance read is a PROJECTION, not a cross-input
derivation of a higher KIND, so the zero-copy law's ELEVATED carve-out (the
Locus::Quorum precedent) does not cover it — the zero-copy-warden verdict for
that shape is MATERIALIZES. The 6x memory is the smaller objection.
GADAMER, MECHANICALLY. Both readings the operator named are the SAME data:
a priori (Vorurteil — prejudice as the CONDITION of understanding, which is
Gadamer's actual claim) = the prior sealed version Vn read at plan-
evaluation time and fed into Vn+1's thought. Single-version read: FILTER.
hindsight (wirkungsgeschichtliches Bewusstsein) = a version RANGE Vn..Vm
deinterlaced at read time. Range read: CASCADE.
Nothing is chosen at bake time; temporal.rs already distinguishes them per
E-MARKOV-TEMPORAL-STREAM-1. That is why the time-series shape is not merely
cheaper — it is the only one where the a-priori and hindsight readings are the
same object.
FOUR DELIVERABLES, each with its falsifier. The one most likely to fail is
D-BLW-2's discrimination twin: four stances that rank everything identically
carry exactly as much information as one (the closed_class_guess 150/150
defect), and four that agree nowhere are noise rather than perspectives. Both
halves are asserted via jc::stats::binary_association, reporting counts and
BOTH marginals — never bare kappa, which is the reason that type exists.
D-BLW-3 makes Horizontverschmelzung falsifiable: fusion must MOVE. Flat kappa
across the version series kills the Gadamer claim and regrades it to "four
independent stance reads over a shared corpus" — still true, still useful, not
Gadamer. The a-priori and hindsight trajectories must also differ, or the
distinction is not doing work and should be dropped rather than narrated.
CLAIM CEILING carried from the D3a/D3b split: kappa and phi between lens
projections measure OVERLAP, not validity. This is a reliability-class
statement. Calling it validity needs D3b's external criterion, which stays
blocked — #888's board correction exists because that line was crossed once
already. And jc::stats p-values are classical independent-sample p-values;
verses within a book are domain-correlated, so any significance claim here
needs its own justified dependence model (C4, as corrected).
Adds no subsystem: the lens is a thought body in the existing 5.4 seam, the
corpus is the shipped KJV bake, the stances are the shipped B6 panel, and the
fusion read uses temporal.rs's existing surface. Section 7's exclusions hold
verbatim — persist_sink.rs and temporal.rs are consumed, not modified. The jc
additive constraint continues to hold: those statistics are the independent
reference frame this arm is measured against, and are not to be "improved"
while serving as the oracle.
Plan + STATUS_BOARD only; no code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K3RyLEbuNSHxxB3NTTrGki
Two commits. 116 lib + 13 doctests green;
stats.rsclippy-clean.External review of the merged #887 found two P1 numerical defects. I reproduced both before touching anything — they are real, and they were live in merged code.
P1 —
omega_totalerased loading signsThe triad identity yields
λ_i², hence only|λ_i|, and the code took the positive root for every item. ω depends on(Σλ)², where a negatively-keyed item must subtract.Measured on an exact signed-congeneric fixture (λ =
[+1, −1, +1], equal orthogonal residuals):Signs are now read off the covariance row (
sign(σ_ij) = s_i·s_j, anchoreds_0 = +1— free, because the model is identified only up to a global flip and(Σλ)²is invariant to one, which is now its own test).Sign consistency is additionally checked, and that is a genuine partial one-factor structure test. Worth stating precisely: at k = 3 it is provably subsumed by the existing negative-λ² guard — the single triad's sign product is
−1exactly when the pattern is inconsistent — so it only becomes reachable at k ≥ 4, where λ² is averaged. The can-it-fire test therefore uses a searched k = 4 fixture and pre-registers that every λ² and ψ is non-negative, so only the sign guard can be what rejects.P1 —
multiple_r_squaredwas unit-dependentNormal equations on raw columns plus an absolute
1e-12pivot cutoff — a statement about units, not rank:Now the response and predictors are centered and predictors scaled to unit norm. R² is affine-invariant so no correct answer changes; the rank test becomes relative (diagonal exactly 1); centering absorbs the intercept, so "collinear with the intercept" correctly reappears as two identical centered columns. The
[0,1]clamp is bounded to a rounding-scale band — a materially out-of-range value means the solve failed and surfaces asNone, not as a plausible 0 or 1.Also corrected / added
BinaryAssociation+binary_association— counts, both marginals,p_o/p_ealongside κ and φ. The shipped φ doc said marginals are required to interpret it while the function returned a lone scalar; that was an internal contradiction in my own code.kr20(&[Vec<bool>])— the dichotomous naming surface C2 asked for and C1b didn't ship. Binary input enforced by type; arithmetic delegated tocronbach_alpha.betacfreports non-convergence (Option) instead of presenting the 300th iterate as a p-value;ln_1p(-x)nearx = 1; bounded clamp.Board corrections
jc::statsp-values are classical independent-sample p-values;jirak.rsis a fingerprint-specific empirical probe, not a general uncertainty engine. A dependent-cohort claim needs its own justified dependence model.The uncomfortable finding, filed
E-THE-CROSS-IDENTITY-SUITE-INHERITED-MY-BLIND-SPOT-1. All five cross-identities passed while both defects were live — φ vspearson, R² vspearson², η² vs R² on a dummy, η² vst²/(t²+df), ω vs α.They were blind because every ω fixture had all-positive loadings and every R² fixture was O(1). A cross-identity checks that two computations agree, and agreement is only informative over the region the fixtures span.
ω = αunder tau-equivalence holds whether or not signs are handled, because tau-equivalent items are all positively keyed.The practice that would have caught both, and is cheap: name the transformations the estimator should be invariant under (affine rescaling, global sign flip, permutation) and write one test per dimension. Both defects fell out immediately once those existed.
And a correction to #887's own write-up: I concluded there that doc examples catch what unit tests miss. True but incomplete — doc examples covered one uncovered corner. Treating that as the lesson rather than as an instance is why these two corners stayed uncovered until someone else looked.
Also in this PR — the C2 audit (first commit)
C2's rename defect has zero instances: all four
jc-battery consumers feed continuous/ordinal data, so Pearson/α/ICC are correctly named today. Surfaced instead:TD-STATS-DEGENERACY-CONTRACT-DIVERGENCE— 47 baref64+ 9 baref32vs 3Optionhand-rolled statistics, one returning0.0for every degeneracy.Test plan
cargo test -p jc→ 116 lib + 13 doctests.cargo clippy -p jc --lib --tests→ 0 warnings instats.rs. Both P1 fixes carry regression tests using the review's own falsifiers.🤖 Generated with Claude Code
https://claude.ai/code/session_01K3RyLEbuNSHxxB3NTTrGki
Summary by CodeRabbit
New Features
Improvements
Bug Fixes