feat(level-factors): prior pushforward + surfaced t2 psense warning (#389 finding 3) - #446
Open
ethanbuckley wants to merge 1 commit into
Open
Conversation
…sense warning (#389 finding 3) The level family lacked the estimand-scale prior_pushforward.csv the ITT/gain families emit, and its key-findings headline stayed unqualified even when power-scaling flagged the t2 term for prior-data conflict (the warning was hidden in a collapsed prior section). - reporting.level_t2_marginal_effect gains group="posterior"|"prior" so the same t2 net-out transform runs on either group (byte-identical default); - reporting.level_prior_pushforward pushes the prior on the t2 group contrast through that transform, returning the same items-scale schema as prior_pushforward; fit_level_factors writes prior_pushforward.csv beside the posterior ROPE card (the level model has a single t2 term, so the pushforward is well-defined here, unlike the curve / matrix families #381 could not cover); - _kf_build_level_factors surfaces a caution bullet right after the headline when psense_summary.csv flags b_grp_time[1], via a new _kf_psense_diagnosis reader (graceful no-op when psense is absent or clear). Tests: level_prior_pushforward reads the prior group and returns the schema; the KF warning appears when the t2 term is flagged, is absent when clear, and the golden level case is unchanged. ruff / format:check / spellcheck clean; lf-001 dev fit emits prior_pushforward.csv. Addresses #389 finding 3 (bar the flagged-outcome prior-grid sensitivity, which is refit-gated). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds level-factors parity with other estimand-reporting families by (1) emitting an estimand-scale prior_pushforward.csv for the t2 contrast and (2) surfacing a power-scaling (psense) caution directly in the level-factors key findings when the t2 term is flagged, rather than leaving it hidden in collapsed sections.
Changes:
- Extend
level_t2_marginal_effectto run the same “net-out then add-back t2” transform on either posterior or prior draws, and addlevel_prior_pushforwardto export the prior-on-estimand check in the standard schema. - Write
prior_pushforward.csvduringfit_level_factorsalongside the t2 ROPE outputs. - Add key-findings logic + tests to surface a t2 psense warning immediately after the headline when
psense_summary.csvflagsb_grp_time[1].
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/language_reading_predictors/statistical_models/reporting.py |
Adds prior-group support for the level t2 estimand transform, implements level_prior_pushforward, and surfaces psense-based warnings in level-factors key findings. |
src/language_reading_predictors/statistical_models/pipeline.py |
Persists prior_pushforward.csv for level-factors fits using the new reporting helper. |
tests/statistical_models/test_reporting.py |
Adds coverage that the level prior pushforward reads from the prior group and returns the expected items-scale schema. |
tests/statistical_models/test_key_findings.py |
Adds coverage for the new level-factors psense warning sentence (present when flagged, absent when clear/absent). |
Comment on lines
+3348
to
+3351
| diag = str(df.loc[term, "diagnosis"]).strip() | ||
| if not diag or diag.lower() in {"-", "nan", "none"}: | ||
| return None | ||
| return diag |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
Drafted by an LLM-based AI tool (Claude Code/Opus 4.8).
Second tranche of #389, stacked on #445 (base
feat/lrp389-level-factors-runplan). Addresses finding 3: the level family lacked the estimand-scaleprior_pushforward.csvthe ITT/gain families emit, and its key-findings headline stayed unqualified even when power-scaling flagged the t2 term for prior-data conflict (the warning was hidden in a collapsed prior section).Prior pushforward (the well-defined half)
The ITT
prior_pushforwardassumes atau/tau_istructure that the level model doesn't have — its t2 effect isb_grp_time[1]computed by the bespokelevel_t2_marginal_effect(which nets the full per-timepoint group vector +group × abilityterm out and adds back only the t2 contrast at mean ability). So:level_t2_marginal_effectgainsgroup="posterior" | "prior"— the same net-out transform now runs on either group (default"posterior", byte-identical);level_prior_pushforwardpushes the prior on the t2 contrast through that transform and returns the same items-scale schema asprior_pushforward, soconfig.jsonand any consumer treat both families' prior checks uniformly;fit_level_factorswritesprior_pushforward.csvbeside the posterior ROPE card.The level model has a single randomised t2 term, so the pushforward is well-defined here — unlike the HSGP-curve / correlation-matrix families that stalled the broader #381 pushforward ask. lf-001 dev fit emits a sane check: prior on the t2 contrast median ≈ 0, ±11 words of 79 (wide but not absurd).
Surfaced t2 psense warning
_kf_build_level_factorsnow surfaces a caution bullet right after the headline whenpsense_summary.csvflagsb_grp_time[1](e.g. "potential prior-data conflict"), reading the diagnosis via a new_kf_psense_diagnosishelper. It's a graceful no-op when psense is absent or the term is clear, so fits that never ran power-scaling — and the synthetic golden test — are unchanged. With the warning the level key-findings box is exactly 5 sentences (headline / warning / confidence / rope / causal), within the cap.Tests
level_prior_pushforwardreads the prior group (a +100-shifted posterior proves it isn't read) and returns the schema;ruff,npm run format:check,npm run spellcheckall clean.Still a follow-up
Finding 3's remaining piece — a direct prior-grid sensitivity for the flagged outcomes (W, L, P, B, N) — is refit-gated (VM) and out of scope here. Findings 1 (t2 estimand decision), 2 (prior recentring) and 5 (SP/RW naming) remain as tracked in #445's description.
Assigning to a human for review and merge.