feat(analysis): bind longitudinal ESEM/DSEM composition to an analysis-run profile - #376
feat(analysis): bind longitudinal ESEM/DSEM composition to an analysis-run profile#376seonghobae wants to merge 2 commits into
Conversation
…s-run profile GAP-169 / #169 remaining operator-visible slice: compose recovered construct-class, posterior-draw, strong/strict mean, event-time, membership, and within/between gates as a cutoff-safe analysis-run output. Not an estimator, not a Driver p.16 std restore, and not implemented-main.
|
Warning Review limit reachedNext included review available in 2 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 (11)
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}
| snapshot_id: &str, | ||
| knowledge_cutoff: KnowledgeCutoff, | ||
| design: &LongitudinalEsemDsemDesign, | ||
| observations: &[LongitudinalEsemDsemObservation], |
There was a problem hiding this comment.
🔴 Cross-snapshot draws receive false provenance
Observations carry no snapshot identity, so execute_longitudinal_esem_dsem_run accepts draws from another snapshot. The signed result attributes them to the requested snapshot.
Prompt for agents
Bind every LongitudinalEsemDsemObservation, or an immutable collection containing them, to its source snapshot. In execute_longitudinal_esem_dsem_run, validate that provenance against AnalysisRunRequest.snapshot_id before admitting draws. Ensure the resulting artifact digest covers the validated provenance and add a test that mixes observations from another snapshot.
Was this helpful? React with 👍 or 👎 to provide feedback.
| for observation in observations { | ||
| if observation.available_time.instant() <= knowledge_cutoff.instant() { | ||
| draws.extend_from_slice(&observation.posterior_draws); | ||
| observation_count = observation_count |
There was a problem hiding this comment.
🔴 Unbounded draw copying exhausts memory
A single observation can contain unlimited draws. extend_from_slice duplicates them without a total bound, so a valid run can exhaust process memory.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if request.knowledge_cutoff != knowledge_cutoff.to_rfc3339() | ||
| || request.model_contract_version != LONGITUDINAL_ESEM_DSEM_MODEL_CONTRACT_VERSION | ||
| || request.output_profile != LONGITUDINAL_ESEM_DSEM_OUTPUT_PROFILE | ||
| { | ||
| return Err(AnalysisEngineError::InvalidEvidence); |
There was a problem hiding this comment.
🟡 Equivalent cutoffs reject valid runs
execute_longitudinal_esem_dsem_run compares timestamp text instead of instants. Valid offset timestamps fail when the supplied cutoff uses equivalent canonical UTC text.
Was this helpful? React with 👍 or 👎 to provide feedback.
| construct_class: design.construct_class.as_str().into(), | ||
| membership_design: membership_design_wire_name(design.membership_design)?.into(), | ||
| component_level: design.component_level.wire_name().into(), | ||
| lag_clock: design.lag_clock.as_str().into(), | ||
| invariance_status: design.invariance_status.as_str().into(), | ||
| inference_status: LONGITUDINAL_ESEM_DSEM_INFERENCE_STATUS.into(), |
There was a problem hiding this comment.
🟡 Model provenance disappears from digest
Different model_version or comparison_scope values can produce identical artifacts and digests. Consumers cannot identify which fitted model supplied the invariance claim.
Was this helpful? React with 👍 or 👎 to provide feedback.
| || self.observation_count == 0 | ||
| || self.draw_count < 2 | ||
| || !self.posterior_draw_mean.is_finite() |
There was a problem hiding this comment.
🟡 Impossible artifact counts pass validation
validate accepts 100 observations with only two draws, although every observation requires at least two. Tampered artifacts therefore pass full validation.
Prompt for agents
Validate draw_count against observation_count using checked multiplication: every admitted observation contributes at least two draws. Also enforce the execution's observation bound on reconstructed included and excluded counts with checked addition. Add malformed JSON tests for impossible count combinations and overflow boundaries.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if design.promote_causal { | ||
| claim_causal_effect(CausalHeuristic::TemporalPrecedence)?; | ||
| } | ||
| let _ = claim_causal_effect(CausalHeuristic::TemporalPrecedence); |
There was a problem hiding this comment.
🔍 Discarded causal refusal obscures success
Every successful run calls claim_causal_effect and discards its error. The redundant call obscures which branch actually enforces causal refusal.
Was this helpful? React with 👍 or 👎 to provide feedback.
| let comparison_scope = comparison_scope.into(); | ||
| let model_version = model_version.into(); | ||
| if comparison_scope.is_empty() || model_version.is_empty() { | ||
| return Err(AnalysisEngineError::InvalidEvidence); | ||
| } |
There was a problem hiding this comment.
🔍 Blank provenance labels remain admissible
LongitudinalEsemDsemDesign::new accepts whitespace-only scope and model labels. Such labels provide no usable provenance despite passing the evidence gate.
Was this helpful? React with 👍 or 👎 to provide feedback.
| pub fn to_json(&self) -> Result<String, AnalysisEngineError> { | ||
| self.validate()?; | ||
| serde_json::to_string(self).map_err(|_| AnalysisEngineError::SerializationFailure) |
There was a problem hiding this comment.
📝 Info: Artifact output remains inherently bounded
The removed to_json check cannot admit oversized valid output. Identifiers are capped, timestamps are bounded, and other strings require fixed values.
Was this helpful? React with 👍 or 👎 to provide feedback.
| ) | ||
| .expect("design constructor requires nonempty invariance labels"); |
There was a problem hiding this comment.
📝 Info: Validation assertions preserve error boundaries
Both assertions are unreachable through public inputs. Private design labels are constructor-validated, while evidence counts remain below the summary limit.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
GAP-169 / #169 remaining operator-visible slice: bind recovered construct-class, posterior-draw, strong/strict mean, event-time lag, membership, and within/between gates to an
analysis_engineanalysis-run output profile. Recovery primitives and Driver p.16std-family restores are not this engine composition.longitudinal_esem_dsem_composition_v1/ schematepp.longitudinal_esem_dsem_composition.v1(ADR 0036; 0026–0035 remain on other live PRs or local dirty clones).available_timeagainst the requestknowledge_cutoff.composed_engine_not_estimator.cross_classified.stdmatrix, duplicate CWC/Rubin binds, or claim implemented-main.This is not implemented-main. Exact-head Checks on this head only. Predecessor-head evidence does not transfer.
Does not duplicate:
Test plan
cargo fmt -p analysis_engine -- --checkcargo 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.