feat(analysis): bind Rubin loading uncertainty to an analysis-run profile - #374
feat(analysis): bind Rubin loading uncertainty to an analysis-run profile#374seonghobae wants to merge 2 commits into
Conversation
…file Operators still cannot request the already-merged psychometric_core draw-mean OLS loadings and Rubin T combination as a digest-bound analysis-run output. Bind them jointly as rubin_loading_uncertainty_v1 / tepp.rubin_loading_uncertainty.v1 (ADR 0034). Cutoff-filter observations, refuse raw proportions and single-draw inputs, and keep the claim boundary as complete-data OLS combination rather than Mislevy person-level plausible values. Not a new ESEM/DSEM estimator, not CWC, not a Driver p.16 std restore, and not persistence.
|
Warning Review limit reachedNext included review available in 56 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| || !self.within_variance.is_finite() | ||
| || self.within_variance < 0.0 | ||
| || !self.between_variance.is_finite() | ||
| || self.between_variance < 0.0 | ||
| || !self.total_variance.is_finite() | ||
| || self.total_variance < 0.0 |
There was a problem hiding this comment.
🔴 Invalid uncertainty artifacts pass validation
validate accepts any nonnegative total variance without checking Rubin’s equation against its components and draw count. Consumers can accept false uncertainty as valid.
Prompt for agents
Strengthen RubinLoadingUncertaintyArtifact::validate in crates/analysis_engine/src/rubin_loading_artifact.rs so the serialized scientific fields are internally consistent. Recompute Rubin total variance from within_variance, between_variance, and draw_count using the same arithmetic contract and reject mismatches under an explicitly chosen serialization-safe comparison policy. Also test from_json and to_json with finite, nonnegative but inconsistent component values.
Was this helpful? React with 👍 or 👎 to provide feedback.
| #[test] | ||
| fn noiseless_draws_emit_digest_bound_point_mean_and_rubin_t() { | ||
| let request = request(); | ||
| let accepted = accepted(&request); | ||
| let rows = noiseless_rows(); | ||
| let execution = execute( | ||
| &request, | ||
| &accepted, | ||
| "snapshot-rubin-loading", | ||
| cutoff(), | ||
| IndicatorKind::AdditiveLogRatio, | ||
| &rows, | ||
| ) | ||
| .expect("execution"); |
There was a problem hiding this comment.
🔍 Profile recovery evidence remains incomplete
The profile adds cutoff filtering before estimation, but tests cover only one noiseless example. Add profile-level recovery and interval-coverage evidence for filtered data.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if !factor_score.is_finite() | ||
| || indicator_draws.is_empty() | ||
| || indicator_draws.iter().any(|value| !value.is_finite()) |
There was a problem hiding this comment.
🟨 Unbounded draws enable memory exhaustion
RubinLoadingObservation::new accepts unlimited draws. Processing duplicates the full matrix, so oversized untrusted input can exhaust service memory.
Was this helpful? React with 👍 or 👎 to provide feedback.
| let digest = artifact | ||
| .sha256() | ||
| .expect("constructed Rubin artifact is valid and serializable"); |
There was a problem hiding this comment.
🔴 Long identifiers crash completed runs
Identifiers over 256 bytes pass request validation but fail inside expect. A valid accepted run can panic instead of returning an error.
| let digest = artifact | |
| .sha256() | |
| .expect("constructed Rubin artifact is valid and serializable"); | |
| let digest = artifact.sha256()?; |
Was this helpful? React with 👍 or 👎 to provide feedback.
| let combined = | ||
| combine_draw_level_ols_loadings(&eligible.factor_scores, &eligible.indicator_draws, kind)?; | ||
| let point_estimate_mean = combined.mean_loading; |
There was a problem hiding this comment.
🟡 Point estimates bypass robust recovery
Large canceling loadings make combined.mean_loading differ from the robust point-estimate helper. The artifact then reports the wrong point estimate.
Prompt for agents
In crates/analysis_engine/src/rubin_loading_artifact.rs, execute_rubin_loading_uncertainty_run must obtain point_estimate_mean by calling psychometric_core::recover_loading_point_estimate_mean on the eligible factor scores and transposed indicator draws. Keep combine_draw_level_ols_loadings for Rubin Q-bar/U-bar/B/T. Add a regression test with large canceling per-draw slopes to prove the point estimate follows the robust helper rather than combined.mean_loading.
Was this helpful? React with 👍 or 👎 to provide feedback.
| || self.observation_count < 2 | ||
| || self.draw_count < 2 |
There was a problem hiding this comment.
🟡 Unbounded artifact counts are accepted
validate enforces minimum counts but no execution maximum. Imported artifacts can claim more observations and exclusions than any accepted run permits.
Prompt for agents
Make RubinLoadingUncertaintyArtifact::validate enforce the execution bounds represented by the schema. observation_count must not exceed MAX_EVIDENCE_UNITS, and observation_count plus excluded_after_cutoff_count must be checked for overflow and must not exceed MAX_EVIDENCE_UNITS. Define and enforce a bounded draw count as well. Add from_json tests for each oversized count and for overflowing count sums.
Was this helpful? React with 👍 or 👎 to provide feedback.
| pub fn to_json(&self) -> Result<String, AnalysisEngineError> { | ||
| self.validate()?; | ||
| let payload = | ||
| serde_json::to_string(self).map_err(|_| AnalysisEngineError::SerializationFailure)?; | ||
| return Ok(payload); | ||
| } |
There was a problem hiding this comment.
🔍 Serialization skips the declared size bound
from_json enforces 256 KiB, but to_json does not. Keep both public artifact directions symmetric as fields evolve.
Was this helpful? React with 👍 or 👎 to provide feedback.
| fn admit_observations_at_cutoff( | ||
| observations: &[RubinLoadingObservation], | ||
| knowledge_cutoff: KnowledgeCutoff, | ||
| ) -> Result<EligibleRubinRows, AnalysisEngineError> { | ||
| if observations.len() > MAX_EVIDENCE_UNITS { | ||
| return Err(AnalysisEngineError::LimitExceeded); | ||
| } | ||
| let mut eligible = Vec::new(); | ||
| let mut excluded_after_cutoff_count = 0_u64; | ||
| for observation in observations { | ||
| if observation.available_time.instant() <= knowledge_cutoff.instant() { | ||
| eligible.push(observation); | ||
| } else { | ||
| excluded_after_cutoff_count += 1; | ||
| } | ||
| } | ||
| if eligible.is_empty() { | ||
| return Err(AnalysisEngineError::Psychometric( | ||
| PsychometricError::InvalidNumericInput, | ||
| )); | ||
| } | ||
| let draw_count = eligible[0].indicator_draws.len(); | ||
| let mut factor_scores = Vec::with_capacity(eligible.len()); | ||
| let mut indicator_draws = vec![Vec::with_capacity(eligible.len()); draw_count]; | ||
| for observation in eligible { | ||
| if observation.indicator_draws.len() != draw_count { | ||
| return Err(AnalysisEngineError::Psychometric( | ||
| PsychometricError::InvalidNumericInput, | ||
| )); | ||
| } | ||
| factor_scores.push(observation.factor_score); | ||
| for (draw_index, value) in observation.indicator_draws.iter().enumerate() { | ||
| indicator_draws[draw_index].push(*value); | ||
| } | ||
| } | ||
| #[rustfmt::skip] | ||
| let rows = EligibleRubinRows { factor_scores, indicator_draws, excluded_after_cutoff_count }; | ||
| Ok(rows) | ||
| } |
There was a problem hiding this comment.
📝 Info: Cutoff filtering preserves draw alignment
Filtering precedes transposition, so every draw uses the same eligible rows. Unequal retained draw lengths fail before estimation.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
GAP-006 / #169 remaining operator-visible slice: jointly bind the already-merged
psychometric_coreposterior-draw OLS loading mean (recover_loading_point_estimate_mean) and Rubin (1996) total variance (combine_draw_level_ols_loadings) to ananalysis_engineanalysis-run output profile.rubin_loading_uncertainty_v1/ schematepp.rubin_loading_uncertainty.v1(ADR 0034; 0026–0033 remain on other live PRs).available_timeagainst the requestknowledge_cutoff.Q̄/Ū/B/T. Inference status isrubin_combined_ols_loadings_not_mislevy_pv.stdmatrix, duplicate CWC, persist rows, or claim strong invariance.This is not implemented-main. Exact-head Checks on this head only. Predecessor-head evidence does not transfer.
Does not duplicate:
Test plan
cargo test -p analysis_enginecargo clippy -p analysis_engine --all-targets -- -D warningsRUSTDOCFLAGS="-D warnings" cargo doc -p analysis_engine --no-depsMerge bar
Ruleset 18156473: two independent approvals + exact-head Checks. Do not self-approve. Do not
--adminmerge.