feat(api): carry scientific acceptance only on terminal results - #358
feat(api): carry scientific acceptance only on terminal results#358seonghobae wants to merge 2 commits into
Conversation
GAP-003A second slice: AnalysisRunRequest and AnalysisRunAccepted stay metric-free receipts. tepp.scientific_acceptance.v1 is admitted only on a succeeded terminal result with profile scientific_acceptance_v1. Receipt RMSE/bias/coverage keys, missing artifacts, failed-terminal artifacts, all-zero binding digests, and digest mismatch fail closed. Does not promote implemented-main. analysis_engine library binding remains on live #356. Persistence remains GAP-003B (issue #166).
|
Warning Review limit reachedNext included review available in 31 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 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}
There was a problem hiding this comment.
🔍 Required coverage evidence is absent
Repository rules require 100% production line and branch coverage. The listed verification omits coverage for the new validation and detector branches.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if !is_canonical_sha256(&self.binding_sha256) | ||
| || self.binding_sha256.bytes().all(|byte| byte == b'0') | ||
| { |
There was a problem hiding this comment.
🔴 Arbitrary digests pass binding validation
Any nonzero hexadecimal binding_sha256 passes without derivation from the artifact’s run, snapshot, cutoff, model, seed, backend, or precision. Invalid provenance appears verified.
Prompt for agents
Define a canonical, versioned preimage for ScientificAcceptanceArtifact.binding_sha256 and verify the field against a SHA-256 computed from the intended run-binding inputs. The encoding must be deterministic and unambiguous, and construction plus deserialization must use the same function. Add tests proving that changing each bound input independently invalidates the digest.
Was this helpful? React with 👍 or 👎 to provide feedback.
| impl Eq for ScientificAcceptanceReport {} | ||
|
|
||
| impl Eq for ScientificAcceptanceArtifact {} |
There was a problem hiding this comment.
🔍 Equality promise excludes constructible NaNs
Public f64 fields permit NaN-bearing values, while manual Eq promises reflexivity. Terminal results and statuses inherit the same invalid generic contract.
Was this helpful? React with 👍 or 👎 to provide feedback.
| || artifact.snapshot_id != self.snapshot_id | ||
| || artifact.knowledge_cutoff != self.knowledge_cutoff | ||
| || artifact.output_profile != self.output_profile | ||
| || artifact.sha256()?.as_str() != digest |
There was a problem hiding this comment.
📝 Info: Nested artifact limit remains enforced
Terminal validation hashes through the artifact’s bounded serializer. Construction, parsing, and serialization therefore retain the independent 16 KiB artifact limit.
Was this helpful? React with 👍 or 👎 to provide feedback.
GAP-003A Devin repair on #358. Terminal artifacts now fail closed on negative RMSE/SEs, coverage/Wilson/temporal-order outside [0, 1], inverted Wilson bounds, se_gate_accepted inconsistent with |RMSE| <= k * SE(RMSE), k > MAX_SE_GATE_K, a model that does not match the request, a future or malformed cutoff, and a run_id that is not tepp-validation-{first 32 hex of binding_sha256}. Receipt metric detection covers both standard errors, Wilson upper, and temporal-order accuracy. Not implemented-main. Engine binding remains #356. Persistence remains GAP-003B.
| result_artifact_id: Some(result_artifact_id.into()), | ||
| result_sha256: Some(digest), | ||
| result_schema_version: Some(SCIENTIFIC_ACCEPTANCE_SCHEMA_VERSION.into()), | ||
| completed_at: completed_at.into(), |
There was a problem hiding this comment.
🟡 Completion can predate evidence cutoff
succeeded_scientific_acceptance accepts completed_at earlier than the knowledge cutoff. The result can include evidence unavailable when it supposedly completed.
Prompt for agents
AnalysisRunTerminalResult::succeeded_scientific_acceptance and terminal-result validation parse completed_at and knowledge_cutoff independently but do not enforce temporal ordering. Reject terminal results whose system completion instant precedes their knowledge cutoff, so the eligible evidence horizon cannot extend beyond completion. Apply the invariant in shared terminal validation so parsed JSON and both succeeded constructors behave consistently, and add a test with completed_at before knowledge_cutoff.
Was this helpful? React with 👍 or 👎 to provide feedback.
| fn se_gate_accepts(rmse: f64, rmse_standard_error: f64, k: f64) -> Result<bool, ApiError> { | ||
| if ![rmse, rmse_standard_error, k] | ||
| .iter() | ||
| .all(|value| value.is_finite()) | ||
| { | ||
| return Err(ApiError::InvalidWirePayload); | ||
| } | ||
| if k < 0.0 || rmse_standard_error < 0.0 || rmse < 0.0 { | ||
| return Err(ApiError::InvalidWirePayload); | ||
| } | ||
| if rmse_standard_error == 0.0 { | ||
| return Ok(rmse == 0.0); | ||
| } | ||
| let scale = rmse.max(rmse_standard_error).max(1.0); | ||
| Ok((rmse / scale) <= k * (rmse_standard_error / scale)) |
There was a problem hiding this comment.
📝 Info: SE gate matches existing arithmetic
se_gate_accepts reproduces the existing zero-target acceptance rule. Scaling before multiplication preserves valid comparisons for very large finite inputs.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Hour-20 exact-head review request. Current head GAP-003A tepp_api terminal-wire. Metric-free request/accepted; tepp.scientific_acceptance.v1 only on succeeded terminal with profile scientific_acceptance_v1. Do not duplicate this DTO slice. Do not self-approve. Do not --admin merge. Checks/reviews are not a reason to weaken fail-closed gates. |
|
Hour-21 exact-head review request. Current head @opencode-agent review |
|
Hour-23 exact-head review request. Current head @opencode-agent review |
1 participant
Consolidation decision — closed after invalid scientific authority was retired
This PR is not merged. Its exact head
96a34a41136bfd9ab405acd808645a5f422652edis preserved as transport-contract lineage, but its purpose is to carrytepp.scientific_acceptance.v1produced by #356. PR #356 has now been closed as scientifically unsafe because its generic RMSE/SE gate is scale-invariant, its recovery provenance is caller-declared, and it conflates Validation Evidence with Scientific Claim Promotion.A terminal-result contract remains useful, but it must be rebuilt/folded into the coherent Analysis Run / Validation landing vehicle under #166 and #435 with these boundaries:
Do not reopen this exact vehicle merely by renaming
scientific_acceptance_v1; the authority model must change first. Branch, commits, tests, review history and doctoring remain available as fold evidence.