feat(analysis): bind Pareto candidate-K selection to an analysis-run profile - #409
feat(analysis): bind Pareto candidate-K selection to an analysis-run profile#409seonghobae wants to merge 1 commit into
Conversation
…profile GAP-004 leftover / ADR 0053. Bind existing model_selection::select_candidate_k and selected_k_root_mean_square_error to cutoff-safe pareto_candidate_k_v1. LLM votes cannot define the numerical optimum. Not Schwarz fitted selection, not joint Laplace draws, not a Bayesian sampler, and not implemented-main.
|
Warning Review limit reachedNext included review available in 20 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 (9)
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 |
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Operator COMMENT on exact head 7f24e43.
This is not an APPROVE. Author COMMENTED is not an independent review. Ruleset 18156473 still needs two independent current-head APPROVEs from non-author, non-bot reviewers.
Bind is Pareto select_candidate_k / selected-K RMSE as pareto_candidate_k_v1 (ADR 0053). Distinct from Schwarz fitted candidate-K (#404 / ADR 0049), interpreter/verifier (#405 / ADR 0050), topic activity (#407 / ADR 0051), and joint Laplace draws (#408 / ADR 0052). LLM votes cannot define the numerical optimum. Not a Bayesian sampler. Not implemented-main.
Do not self-approve. Do not merge without two independent approvals. Predecessor Checks do not transfer.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| return Err(AnalysisEngineError::InvalidEvidence); | ||
| } | ||
|
|
||
| let selected_k = u64::from(select_candidate_k(input.candidates())?); |
There was a problem hiding this comment.
🔴 Future evidence enters historical selection
With candidates fitted from post-cutoff evidence, select_candidate_k accepts them because the input records no availability provenance. Historical results can therefore use future evidence.
Prompt for agents
Make ParetoCandidateKInput carry validated construction provenance for the exact snapshot and knowledge cutoff, including enough availability information or an eligibility-bound upstream artifact to prove that every diagnostic excludes post-cutoff evidence. In execute_pareto_candidate_k_run, validate that provenance against the request before calling select_candidate_k. Add a test where a candidate built from post-cutoff evidence is rejected.
Was this helpful? React with 👍 or 👎 to provide feedback.
| return Err(AnalysisEngineError::InvalidEvidence); | ||
| } | ||
|
|
||
| let selected_k = u64::from(select_candidate_k(input.candidates())?); |
There was a problem hiding this comment.
🔴 Unbounded selection can exhaust CPU
With a large candidate vector, select_candidate_k performs quadratic dominance comparisons before enforcing any limit. One run can monopolize CPU and stall the service.
Prompt for agents
Define a defensible maximum candidate count for ParetoCandidateKInput and reject oversized inputs before select_candidate_k performs its quadratic Pareto-front scan. Bound selected_replications as well, enforce limits at construction and execution boundaries, and add boundary tests.
Was this helpful? React with 👍 or 👎 to provide feedback.
| let summary = AnalysisResultSummary::new( | ||
| "pareto_candidate_k", | ||
| candidate_count, | ||
| 2, | ||
| PARETO_CANDIDATE_K_INFERENCE_STATUS, | ||
| )?; |
There was a problem hiding this comment.
🟡 Candidate totals masquerade as evidence
AnalysisResultSummary::new records model candidates as evidence units. Clients receive evidence totals unrelated to the source corpus.
Prompt for agents
Carry the cutoff-eligible source evidence count in the validated Pareto input provenance and pass that count as AnalysisResultSummary.evidence_count. Keep candidate_count only in the Pareto artifact. Add an integration assertion where evidence count differs from candidate count.
Was this helpful? React with 👍 or 👎 to provide feedback.
| #[test] | ||
| fn mismatched_replications_record_positive_rmse() { | ||
| let request = request(); | ||
| let mismatched = ParetoCandidateKInput::new( | ||
| vec![ModelCandidate::statistical(2, -30.0, 8.0).expect("k2")], | ||
| vec![4, 4, 4], | ||
| 2, | ||
| ); | ||
| let execution = execute(&request, &mismatched).expect("rmse"); | ||
| assert_eq!(execution.artifact.selected_k, 2); | ||
| assert!((execution.artifact.selected_k_rmse - 2.0).abs() < f64::EPSILON); | ||
| } |
There was a problem hiding this comment.
🔍 Recovery evidence remains narrow
Hand-built RMSE examples do not establish the required recovery, bias, convergence, or failure-rate evidence. Verify whether existing studies cover this exact profile.
Was this helpful? React with 👍 or 👎 to provide feedback.
| pub struct ParetoCandidateKArtifact { | ||
| /// Exact versioned schema identity. | ||
| pub schema_version: String, | ||
| /// Opaque accepted-run identity. | ||
| pub run_id: String, | ||
| /// Immutable source snapshot identity. | ||
| pub snapshot_id: String, | ||
| /// Historical evidence cutoff used by the selection. | ||
| pub knowledge_cutoff: String, | ||
| /// Statistically selected topic count `K`. | ||
| pub selected_k: u64, | ||
| /// Number of candidates offered to the Pareto gate. | ||
| pub candidate_count: u64, | ||
| /// Number of statistically supported candidates. | ||
| pub statistical_count: u64, | ||
| /// Known-truth topic count used for RMSE. | ||
| pub truth_k: u64, | ||
| /// RMSE of selected-`K` replications against known truth. | ||
| pub selected_k_rmse: f64, | ||
| /// Fixed claim boundary for consumer copy. | ||
| pub inference_status: String, | ||
| } |
There was a problem hiding this comment.
🔍 Mutable artifacts weaken invariants
Public fields let callers invalidate completed artifacts after execution. Serialization catches mutations, but in-memory consumers can observe unchecked states.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if request.knowledge_cutoff != knowledge_cutoff.to_rfc3339() | ||
| || request.model_contract_version != PARETO_CANDIDATE_K_MODEL_CONTRACT_VERSION | ||
| || request.output_profile != PARETO_CANDIDATE_K_OUTPUT_PROFILE |
There was a problem hiding this comment.
🔍 Cutoff matching rejects equivalent timestamps
Exact string matching rejects valid RFC 3339 representations of the same instant. Confirm whether profile binding requires textual identity rather than temporal equality.
Was this helpful? React with 👍 or 👎 to provide feedback.
ADR 0053 is already live on #409 (Pareto candidate-K vs main). Rename this stack's decision identity rather than collide.
1 participant
Summary
GAP-004 leftover / ADR 0053. Bind existing
model_selection::select_candidate_kandselected_k_root_mean_square_errorto a cutoff-safepareto_candidate_k_v1analysis-run profile (tepp.pareto_candidate_k.v1).K, candidate/statistical counts, known-truthK, selected-KRMSE, and inference statuspareto_statistical_front_not_fitted_schwarz_sampler.fitted_candidate_k_v1,joint_posterior_draws_v1, andtrsl_topic_lineage_v1.Not Schwarz fitted candidate-
K(#404 / ADR 0049). Not interpreter/verifier (#405 / ADR 0050). Not topic activity/dormancy (#407 / ADR 0051). Not joint Gauss-Newton Laplace draws (#408 / ADR 0052). Not a Bayesian sampler. Not GPU. Not topic birth/split/merge. Not implemented-main.Distinct from live slices
Does not duplicate #408 (joint posterior Laplace draws), #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_enginecargo clippy -p analysis_engine --all-targets -- -D warningspython3 scripts/validate_documentation.pyMerge 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.