Skip to content

feat(analysis): bind joint posterior Laplace draws to an analysis-run profile - #408

Draft
seonghobae wants to merge 1 commit into
mainfrom
feat/joint-posterior-draws-analysis-run-gap-004
Draft

feat(analysis): bind joint posterior Laplace draws to an analysis-run profile#408
seonghobae wants to merge 1 commit into
mainfrom
feat/joint-posterior-draws-analysis-run-gap-004

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Summary

GAP-004 leftover / ADR 0052. Bind existing JointCoordinatePrecision::draw_joint_gaussian (Philox4x32-10, Box-Muller, Cholesky) to a cutoff-safe joint_posterior_draws_v1 analysis-run profile (tepp.joint_posterior_draws.v1).

  • Fits the CPU f64 TRSL-TM reference, builds the identified joint Gauss-Newton Laplace precision, and draws through the library already on protected main.
  • Artifact carries draw_set_id, algorithm version, seed, draw/document/topic counts, approximation joint_gauss_newton_laplace, and inference status joint_gaussian_laplace_plausible_values_not_mcmc.
  • Draw coordinates stay off the operator artifact; the draw-set digest already binds them.
  • Zero-draw, non-convergence, snapshot/profile/cutoff mismatch, and trsl_topic_lineage_v1 / fitted_candidate_k_v1 profile reuse fail closed.

Not MCMC. Not Schwarz candidate-K (#404 / ADR 0049). Not interpreter/verifier (#405 / ADR 0050). Not topic activity/dormancy (#407 / ADR 0051). Not GPU. Not topic birth/split/merge. Not implemented-main.

Distinct from live slices

Does not duplicate #407 (topic activity), #406 (wait CLI), #405 (interpreter/verifier), #404 (fitted candidate-K), #403 (retry-lineage CLI), #398 (membership-posterior ICC), #376 (ESEM/DSEM), #374 (Rubin), #372 (CWC), #389 (irregular event-time), #364 (TDT/CHRONOS), #356/#358/#359 (GAP-003A), #351 (Leiden), or Driver p.16 std-family micro-PRs.

Verification

  • cargo test -p analysis_engine
  • cargo clippy -p analysis_engine --all-targets -- -D warnings
  • python3 scripts/validate_documentation.py

Merge gate

Two independent current-head APPROVEs required. Author/bot COMMENTED is not independent APPROVE. Exact-head Checks on this SHA only. Predecessor Checks do not transfer. Do not self-approve. Do not merge without two independent approvals.


Devin Review

… profile

GAP-004 remaining slice (ADR 0051): operators request cutoff-safe
joint_posterior_draws_v1 which fits the CPU f64 TRSL-TM reference, builds
the identified Gauss-Newton Laplace precision, and materializes Philox/
Box-Muller/Cholesky plausible values. Not MCMC, not Schwarz candidate-K
(#404), not GPU, not topic birth/split/merge. Persistence remains later.
@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 24 minutes.

View limit details

Limit 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.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8c179d4e-92aa-47f1-8690-f11ee52470ff

📥 Commits

Reviewing files that changed from the base of the PR and between 1bc02f5 and fd3e85a.

📒 Files selected for processing (9)
  • CHANGELOG.md
  • DOCUMENTATION.md
  • crates/analysis_engine/src/joint_posterior_draws_artifact.rs
  • crates/analysis_engine/src/lib.rs
  • crates/analysis_engine/tests/joint_posterior_draws_execution_contract.rs
  • docs/TRACEABILITY.md
  • docs/adr/0052-joint-posterior-draws-analysis-run.md
  • docs/adr/README.md
  • docs/doctoring/joint-posterior-draws-analysis-run.md

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 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 4 potential issues.

Devin Review

Comment on lines +157 to +160
if request.snapshot_id != snapshot_id {
return Err(AnalysisEngineError::SnapshotMismatch);
}
if request.knowledge_cutoff != knowledge_cutoff.to_rfc3339()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Input provenance is self-asserted

An input built from another snapshot or later cutoff passes execute_joint_posterior_draws_run when separate metadata matches the request. The artifact can include future evidence.

Prompt for agents
The new execute_joint_posterior_draws_run API accepts ReferenceTopicInput separately from snapshot_id and knowledge_cutoff, but ReferenceTopicInput does not retain either provenance value after construction. The executor therefore validates only caller assertions and can bind input built from a different snapshot or a later cutoff to the request. Redesign the boundary so the model input, or a validated wrapper around it, carries immutable snapshot identity and construction cutoff. Validate those retained values against AnalysisRunRequest before fitting, and add tests that build input under a later cutoff or different snapshot and verify rejection.
Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +206 to +209
Ok(JointPosteriorDrawsExecution {
artifact,
terminal_result,
})

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Generated draws are discarded

Every successful execute_joint_posterior_draws_run returns only draw metadata and a digest. Callers cannot retrieve or reconstruct the generated coordinates.

Prompt for agents
execute_joint_posterior_draws_run materializes a JointPosteriorDrawSet, extracts metadata, then drops the set and returns only JointPosteriorDrawsArtifact plus AnalysisRunTerminalResult. No persistence API or retrieval identifier backed by storage exists, so the requested coordinates are unavailable after success. Define the intended data boundary and either return the bounded JointPosteriorDrawSet with the execution, persist it through an explicit artifact store referenced by the terminal result, or include a reconstructible and available payload. Keep the digest binding and size limits, and test that a successful caller can obtain the exact generated coordinates.
Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +106 to +107
|| !valid_identifier(&self.draw_set_id)
|| self.draw_set_id.len() != 64

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Malformed draw digests pass validation

from_json accepts any 64-byte identifier as draw_set_id, including non-hex values. Corrupted artifacts therefore masquerade as SHA-256-bound draw sets.

Suggested change
|| !valid_identifier(&self.draw_set_id)
|| self.draw_set_id.len() != 64
|| !valid_identifier(&self.draw_set_id)
|| self.draw_set_id.len() != 64
|| !self
.draw_set_id
.bytes()
.all(|byte| byte.is_ascii_digit() || (b'a'..=b'f').contains(&byte))
Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +165 to +166
#[test]
fn fitted_precision_emits_digest_bound_laplace_draws() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 Scientific recovery evidence is absent

The new estimator path adds contract tests but no synthetic RMSE, bias, coverage, convergence, or failure-rate study. Repository acceptance rules require that evidence.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

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