feat(release): quantify zero-event benchmark safety bounds - #240
feat(release): quantify zero-event benchmark safety bounds#240seonghobae wants to merge 89 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough제로 이벤트 안전성 증거와 벤치마크 실패 분류 타입을 추가합니다. 릴리스 결정 진입점은 입력을 검증하고 증거를 canonical 순서로 보고서에 저장합니다. 테스트와 문서는 계산, 분류, 오류 계약 및 중복 처리를 검증합니다. Changes릴리스 수락 증거
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🟡 Moderate · up to The PR adds typed benchmark failures and zero-event safety evidence, but it is not merge-ready because the new threshold test does not compile and release acceptance records safety bounds without enforcing the corresponding quantitative policy. The public API and acceptance behavior need to be aligned before merge. Sequence Diagram(s)sequenceDiagram
participant 호출자
participant ClassifiedEvidenceEntrypoint
participant ReleaseDecisionPipeline
participant ReleaseDecisionReport
호출자->>ClassifiedEvidenceEntrypoint: benchmark evidence와 안전성 관찰값 전달
ClassifiedEvidenceEntrypoint->>ReleaseDecisionPipeline: 메타데이터와 중복 증거 검증
ReleaseDecisionPipeline->>ReleaseDecisionReport: canonical 실패와 관찰값 저장
ReleaseDecisionReport-->>호출자: 릴리스 결정 보고서 반환
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Noema LLM review
The PR implements a bounded, deterministic release-acceptance contract for benchmark evidence, including a quantitative zero-event statistical safety gate based on the Clopper-Pearson binomial method. The implementation has been refactored to prevent resource exhaustion (memory/CPU) by bounding input collections and terminating iterators upon detecting duplicates. The logic for zero-event upper bounds is mathematically correct, and the fail-closed behavior is verified by comprehensive tests. Documentation in ARCHITECTURE.md and CHANGELOG.md has been updated to reflect the new bounded context in originweave-release.
Reviewed changed lines
crates/originweave-release/src/release_acceptance.rs:130 (RIGHT): Correctly implements the one-sided Clopper-Pearson upper bound for zero events usingexp_m1to maintain precision for small values.crates/originweave-release/src/release_acceptance.rs:618 (RIGHT): Enforces a fixed maximum cardinality for zero-event observations, preventing unbounded memory allocation during cloning and sorting.crates/originweave-release/src/release_acceptance.rs:558 (RIGHT): The loop overevidenceterminates immediately upon detecting a duplicate suite, protecting against infinite or oversized iterators.crates/originweave-release/src/zero_event_safety_gate.rs:215 (RIGHT): Validates that requirements and observations are within the fixed metric budget before populating BTreeMaps, ensuring resource bounds at the policy boundary.ARCHITECTURE.md:81 (RIGHT): Accurately describes the new bounded release-evidence contract and explicitly states that it does not grant release authority.
Adversarial validation
crates/originweave-release/src/release_acceptance.rs:558 (RIGHT)falsified: An infinite iterator yielding the same suite will cause a hang or OOM. — The code checksif outcomes[index].is_some() { return Err(...) }inside the loop, ensuring termination at the second element.crates/originweave-release/src/release_acceptance.rs:618 (RIGHT)falsified: A very large slice ofZeroEventSafetyObservationwill cause OOM duringto_vec()orsort_by_key(). — Thevalidate_release_metadatafunction is called first and returnsErr(ReleaseDecisionError::TooManyZeroEventSafetyObservations)if length > 5.- Residual risk: Low. The use of f64 for the final rate comparison is mitigated by
exact_fixed_point_boundary_matchesfor small trial counts where precision is most critical.
Findings
-
No blocking findings.
-
Result: APPROVE
-
Head SHA:
86fe70fa7e25347649a4397361143ff86473261e -
Reviewer credential:
noema-review-github-app -
Actor:
cwl-noema-review[bot]
seonghobae
left a comment
There was a problem hiding this comment.
Current-head verification on c7d6c2053e0fb3f63d936e5aaaca01b2f76ce987: all returned inline threads are resolved, the temporary write-capable/self-modifying workflow is absent, and .github/workflows/ci.yml is exact blob 95c2fa1d7cf190ed2c043c5e05e27dfb606b5118, identical to protected main@c789b802.... Exact CI 33726146724 is terminal GREEN (Rust contracts 100555443903, Production coverage 100555443569), and MV3 33726146609, SAST 33726146692, Security Scan 33726146770, OSV 33726146911, and Scorecard 33726146925 also succeeded. The Noema APPROVED review targets predecessor 86fe70fa..., so it remains historical rather than current-head approval under stale-review semantics. This comment records current evidence only; it is not self-approval or merge/release authorization.
Signed-off-by: Seongho Bae <me@seonghobae.me>
|
Current protected-main adoption: exact contributor head is now |
seonghobae
left a comment
There was a problem hiding this comment.
Current-head verification repair: protected main is now 4ed08bf and this PR's exact head is 76f30fe. The body still records predecessor c7d6c20... and transfers that predecessor's terminal GREEN; do not use it as current-head evidence. Fresh exact-head runs are CI 33886238370, Manifest V3 33886238427, Security Scan 33886238396, SAST 33886238316, and CodeQL 33886238329; all are currently queued/non-terminal. The earlier Noema approval was already predecessor-only and remains non-transferable. Preserve the release-acceptance bounded-context migration and do not merge, tag, publish, or release until current exact checks and live approvals are terminal.
Signed-off-by: Seongho Bae <me@seonghobae.me>
|
Restacked non-force onto protected Exact-head local verification:
Hosted checks and counted approval remain independently required; queued, skipped, or predecessor results are not merge authorization. |
|
Fresh exact-head review for |
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head focused review for 24930a3 found no new source-backed defect in the zero-event release boundary.
The review traced ZeroEventSafetyEvidence through fixed-point threshold evaluation and the combined commercial decision. Zero trials and invalid confidence fail construction; insufficient confidence, missing metrics, and an upper bound above policy remain Inconclusive; a zero-rate threshold cannot turn finite trials into success; known benchmark failures remain Rejected. Satisfied safety evidence preserves the underlying accepted/accepted-with-limitations decision without granting repository or release authority.
Focused verification passed rustfmt and 19 Rust tests across the zero-event bound, threshold, and commercial gate suites. This is a COMMENTED review, not approval. Current central CodeQL compatibility failures are dispatch placeholders awaiting authenticated shard verdicts; remaining in-progress/queued checks and an eligible exact-head approval still block merge.
Partial implementation of #203. This branch defines a fail-closed release-acceptance boundary for benchmark evidence: zero-event safety claims retain exact trial counts/confidence bounds, benchmark execution failures stay causally classified, and missing/non-authoritative evidence cannot be promoted to release success.
Current production and DDD boundary
The release-acceptance contract lives in the dedicated
originweave-releasebounded-context crate.originweave-coreremains the stable cross-context value-contract kernel and does not depend outward on release-specific policy/evidence types.ZeroEventSafetyEvidencerequires a nonzero exact trial count and confidence in1..=9999basis points.upper_event_rate()reports the one-sided Clopper-Pearson zero-event upper bound using the bounded numerical implementation.ZeroEventSafetyObservationvalues are deterministic and bounded; duplicate metric identities fail closed.BenchmarkFailureClasspreserves deterministic/stochastic product failures separately from external site drift, outage, unsupported capability, infrastructure failure, and benchmark defects.Inconclusive, never passing evidence.This branch intentionally moves release-acceptance ownership out of
originweave-coreintooriginweave-release. ADR 0015 records the deliberate pre-GA migration; no compatibility shim may invert the DDD dependency direction or duplicate release contracts across both contexts.Primary statistical basis: Clopper, C. J., & Pearson, E. S. (1934). The use of confidence or fiducial limits illustrated in the case of the binomial. Biometrika, 26(4), 404–413. https://doi.org/10.1093/biomet/26.4.404
Current repair and protected-main lineage
The earlier DDD documentation defects remain repaired:
ARCHITECTURE.mdidentifiesoriginweave-releaseas the active release-acceptance owner,CHANGELOG.mdrecords the pre-GA move out of core, and the temporary self-modifying workflow remains absent. Its historical control-plane defect stays tracked under #123 and must not be restored.Protected
mainis exact87c4daa1830bac5a5228b6036752ad5633232085through #286. Current exact head is24930a3a9ee79c0b712ee3df6589b0592eb6e18f, a normal non-force merge adoption of that protected generation. Fresh compare is 89 ahead / 0 behind, merge base exactly protected main, with 32 effective release/product/docs/test paths and no.github/**delta. GitHub reports the PR open, Ready and mergeable. Workflow/lifecycle changes visible inside the topology merge are inherited protected-main content rather than an effective workflow mutation by this lane.Exact-current evidence
The previous body was stale at predecessor
c7d6c205.../ protectedmain@c789b802.... Exact current24930a3a9ee79c0b712ee3df6589b0592eb6e18fhas terminal returned workflow evidence:33925598822: success;33925598787: success;33925598876: success;33925598799: success; and33925598844: failure.The failed CodeQL verdict remains fail-closed and is not replaced by native/MV3/security success. Exact-head GREEN elsewhere does not grant merge, tag, publication or release authority and does not transfer predecessor review evidence.
Remaining #203 scope
This slice still does not execute the complete commercial benchmark portfolio, durably bind authenticated benchmark evidence to every release decision, establish every supported browser/profile claim, or complete integrated commercial release acceptance. Those remain dependency-ordered work under #203.
Protected-main
AGENTS.mdand live GitHub governance remain authoritative. No self-approval, force-push, destructive rebase, workflow/ruleset/secret mutation, gate weakening, predecessor evidence transfer, tag, release, or publication is authorized by this PR.