Skip to content

fix(longitudinal): require both marginals for lagged correlation - #441

Closed
seonghobae wants to merge 10 commits into
mainfrom
agent/psychometric-trait-plus-state-expected-autocorrelation
Closed

fix(longitudinal): require both marginals for lagged correlation#441
seonghobae wants to merge 10 commits into
mainfrom
agent/psychometric-trait-plus-state-expected-autocorrelation

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Root-cause repair

The original head beba8ea6dd979b7355077f12110e04ea00426af0 exposed a one-sided covariance/earlier-variance ratio as expected_autocorrelation. That ratio can exceed one under nonstationary marginals and is not printed or validated as an autocorrelation by Driver, Oud, and Voelkle (2017). That predecessor remains RED/scientific-failure lineage only; its public ratio API and psychometric_core additions are retired.

The replacement belongs to the Longitudinal Modeling bounded context. longitudinal_core::recover_event_time_lagged_correlation requires lagged covariance, both occasion-specific marginal variances, and a positive event-time interval. It does not infer process noise, marginal state variance, or a psychometric response kernel. Reusable static/dependence psychometric kernels remain owned by ContextualWisdomLab/fast-mlsirm; TEPP owns temporal/event composition.

Numerical contract

The Cauchy–Schwarz bound is checked exactly for the represented binary64 inputs by decomposing each positive finite value into an integer significand and a power-of-two exponent and comparing covariance^2 <= variance_t * variance_t_plus_delta without rounded square-root products or floating-point multiplication overflow.

After that exact gate, standardization divides by the smaller marginal standard deviation first and then by the larger one. The proven covariance bound guarantees the intermediate magnitude cannot exceed the remaining scale while avoiding avoidable underflow for highly unequal marginals.

Exact-head CI-driven repair

Hosted checks on predecessor exact head 6f483224b3a03e8237c6f4f098a8b0e85e0a91f5 terminated rather than remaining queued:

  • Documentation Quality: GREEN;
  • SAST Semgrep: GREEN;
  • Security Scan: FAILED because the dependency-review job failed its repository-support preflight before the actual dependency-review action; OSV and Trivy jobs were GREEN;
  • Rust Foundation CI: FAILED at cargo fmt --check and at the 100% production branch-coverage gate.

The uploaded llvm-cov branch artifact identified six uncovered branches in crates/longitudinal_core/src/association.rs: three untested non-finite argument positions, later-marginal non-positive validation, the reverse marginal-scale ordering, and a defensive post-division !correlation.is_finite() branch. Current commit 23959d1199f84554f4a5090cea2b9e9d70b55dd8 repairs those causes:

  • adds the missing final newline that made the new source fail cargo fmt --check;
  • covers every non-finite argument position independently;
  • covers both earlier and later non-positive marginal variance;
  • covers the earlier_scale > later_scale ordering with symmetric recovery;
  • removes the mathematically unreachable post-division non-finite branch: finite positive marginals plus the exact covariance bound already imply finite standardized divisions, so manufacturing an impossible test would only hide dead code.

This is a source/test repair, not a waiver or exclusion. New exact-head hosted checks on 23959d1199f84554f4a5090cea2b9e9d70b55dd8 are the only landing evidence; predecessor successes/failures do not transfer.

Recovery / capability boundary

RED/GREEN lineage covers nonstationary marginals (cov=1.5, variances 1 and 4 -> 0.75), exact ±1 boundaries at ordinary/f64::MAX/minimum-subnormal scales, one-ULP over-bound rejection, gross subnormal violation before division, huge representable marginals, unequal marginal scales in both orderings, and every fail-closed input branch.

docs/research/trait-state-lagged-association.md traces Pearson normalization, boundedness, nonstationary continuous-time semantics, and the distinction between an arithmetic standardizer and future model-specific recovery. These arithmetic tests do not promote a DSEM/ctsem estimator. A supported model-derived temporal correlation still requires explicit state equations, process-noise/marginal recovery, identification, uncertainty, RMSE/bias/coverage and leakage-safe rolling-origin recovery.

Remaining merge gate

Current exact head: 23959d1199f84554f4a5090cea2b9e9d70b55dd8.

Do not merge until all current-head Rust/documentation/security checks terminate GREEN, every blocking conversation remains resolved on this head, and a qualifying independent non-author APPROVE exists under live ruleset 18156473. If the dependency-review support preflight fails again on this exact head, verify whether it is repository-owned configuration or centrally owned governance before changing source; do not mislabel a support/preflight failure as a vulnerability finding. No self-approval, predecessor evidence transfer, coverage exclusion, or bypass.

…ocorrelation

Name the covariance ratio (trait + e^{aΔt} p + added) / (trait + p + added)
as a scalar map distinct from unstandardised discreteDRIFT and from
discreteDRIFTstd. Form lagged covariance first, then contemporaneous
total, then addedTIPREDVAR, then the ratio. Claim-boundary tests refuse
substituting either discrete auto-effect for that named quantity.
@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 93d4c836-5cb8-4d3a-a6a9-e7c80923645a

📥 Commits

Reviewing files that changed from the base of the PR and between c1aeed3 and 6f48322.

📒 Files selected for processing (4)
  • ARCHITECTURE.md
  • CHANGELOG.md
  • crates/longitudinal_core/src/association.rs
  • docs/research/trait-state-lagged-association.md

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

시간 지연 공분산을 두 시점의 주변 분산으로 표준화하는 공개 함수가 추가되었습니다. 정확한 binary64 공분산 경계 검사와 입력 오류가 추가되었습니다. 관련 테스트와 연구·패키지 문서가 갱신되었습니다.

Changes

시간 지연 연관 표준화

Layer / File(s) Summary
연관 API 계약과 오류 모델
crates/longitudinal_core/src/association.rs, crates/longitudinal_core/src/error.rs, crates/longitudinal_core/src/lib.rs
recover_event_time_lagged_correlation 함수와 공개 내보내기가 추가되었습니다. 비유한 입력, 비양수 분산 및 간격, 공분산 경계 위반을 위한 오류가 추가되었습니다.
정확한 경계 검사와 표준화 검증
crates/longitudinal_core/src/association.rs
binary64 significand와 지수의 정수 비교로 공분산 경계를 검사합니다. 작은 표준편차를 먼저 사용해 나눗셈 순서를 조정합니다. 극단값, subnormal, ULP 초과, 불균등 스케일을 검증합니다.
연구 경계와 패키지 문서
crates/longitudinal_core/Cargo.toml, crates/longitudinal_core/src/lib.rs, docs/research/trait-state-lagged-association.md, ARCHITECTURE.md, CHANGELOG.md
시간 지연 연관 표준화의 계약과 연구 범위를 문서화했습니다. 기존 expected_autocorrelation API 제거와 모델별 검증 기준을 기록했습니다.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: 🟡 Moderate · up to 6f483

The PR replaces the one-sided ratio with a bounded lagged-correlation calculation and adds validation for extreme numeric inputs. Merge readiness remains moderate because current documentation still contains unresolved scientific terminology, verification traceability, ownership, and identifier inconsistencies that could mislead users or weaken the published contract until corrected or explicitly accepted.

Suggested reviewers: cursoragent

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant recover_event_time_lagged_correlation
  participant Binary64BoundCheck
  participant CorrelationResult
  Caller->>recover_event_time_lagged_correlation: 지연 공분산과 두 주변 분산 전달
  recover_event_time_lagged_correlation->>Binary64BoundCheck: Cauchy–Schwarz 경계 검사
  Binary64BoundCheck-->>recover_event_time_lagged_correlation: 경계 통과 또는 CovarianceBoundViolation
  recover_event_time_lagged_correlation->>CorrelationResult: 표준화 및 [-1, 1] 제한
  CorrelationResult-->>Caller: 복구된 시간 지연 상관 반환
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 93.94% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 33 functions across 7 files. (3 skipped: 3 …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 지연 상관 계산에 두 주변분산을 요구하는 핵심 변경을 정확하고 간결하게 설명합니다.
Full details: Docstring Coverage

Explanation

Docstring coverage is 93.94% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 33 functions across 7 files. (3 skipped: 3 unsupported.)

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/psychometric-trait-plus-state-expected-autocorrelation

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

devin-ai-integration[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

Replace the unmerged psychometric_core covariance/initial-variance ratio with a Longitudinal Modeling boundary that only standardizes lagged covariance when both marginal variances are known. Preserve the invalid predecessor commit as RED lineage, remove its public autocorrelation API and docs from the final diff, and add nonstationary/covariance-bound/overflow-safe regressions.
@seonghobae seonghobae changed the title feat(psychometric): recover Driver §7.1 trait-plus-state expected autocorrelation fix(longitudinal): require both marginals for lagged correlation Sep 1, 2026
devin-ai-integration[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

Put the ARCHITECTURE.md and CHANGELOG.md records on the reviewed
branch and remove the one-shot writer workflow that lacked Workflows
permission and mixed repository-write credentials into verification.
seonghobae added a commit that referenced this pull request Sep 1, 2026
Re-read GitHub at 2026-09-01T10:19:29Z: 136 open PRs (91 draft, 45
non-draft) on protected main 1bc02f5. #356 is closed without merge.
#441 exact head is 6f48322. #444 is a fold candidate, not a new
bounded context. Queued checks remain non-passing.
@seonghobae
seonghobae enabled auto-merge (squash) September 1, 2026 12:29
@opencode-agent
opencode-agent Bot disabled auto-merge September 1, 2026 14:24

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

Devin Review

Comment thread crates/longitudinal_core/src/association.rs

Copy link
Copy Markdown
Contributor Author

Queue consolidation executed after strict ancestry verification. #441@23959d1199f84554f4a5090cea2b9e9d70b55dd8 is the merge base and exact ancestor of the current Longitudinal Modeling landing vehicle #310; compare reported ahead_by=13, behind_by=0 before the subsequent #310 repairs. #310 preserves this lagged-correlation source, tests, scientific-failure lineage and resolved review evidence, then adds typed EventTimeInterval, the discreteDRIFTstd slice, overflow repair, and governance amendments. Closing #441 as superseded-by-fold; no unique source or review evidence is discarded and no predecessor check is transferred as passing evidence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant