Skip to content

feat(core): add fail-closed benchmark release decision contract - #218

Open
seonghobae wants to merge 63 commits into
mainfrom
feat/release-benchmark-decision-contract
Open

feat(core): add fail-closed benchmark release decision contract#218
seonghobae wants to merge 63 commits into
mainfrom
feat/release-benchmark-decision-contract

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Buyer-visible gap

Issue #203 requires a deterministic release decision that cannot promote missing, inconclusive, duplicated, failed, oversized, ambiguously represented, or non-canonical mandatory-suite evidence into an accepted commercial release. This bounded slice owns only the pure decision contract; benchmark execution, thresholds, corpus licensing/statistics, signed artifact binding, and suite runners remain outside this PR.

Implemented boundary

originweave_core::release_acceptance provides the five mandatory suite identities, deterministic accepted / accepted_with_declared_limitations / rejected / inconclusive aggregation, exact failed/inconclusive/missing evidence, fail-closed duplicate-suite rejection, bounded buyer-visible limitations, Unicode 17.0.0 presentation-safety validation, NFC and canonical surrounding-whitespace rejection, duplicate limitation-claim identity rejection, and no model/status/author override path.

A known threshold failure remains rejected even if other evidence is incomplete. Missing or inconclusive mandatory evidence is never promoted to acceptance.

Test-first repair lineage

Historical failing/predecessor heads below are regression lineage only, never current passing evidence.

Duplicate limitation identity

  • RED exact head 955d24b8a3eca9819e219932dfad3aaece61707f: CI 32675693535, Rust job 97283280184; repository contracts/format/check passed and conflicting_consequences_for_one_limitation_claim_fail_closed failed against the old production behavior.
  • Production repair b71d6ba3948b799155d177874ab00fa8e2a9d526: decide_release rejects repeated unsupported_claim identity with typed ReleaseDecisionError::DuplicateLimitationClaim before report emission.
  • Test strengthening 5c0478985edccdda33851903282145056ddc2ba6 pins both conflicting- and identical-consequence duplicate-claim rejection and preserves the 64-item resource ceiling with unique claims.

Canonical limitation text and exact coverage

The release contract rejects non-NFC text and leading/trailing whitespace in both unsupported_claim and buyer_consequence, preventing presentation variants from bypassing one-authoritative-claim identity. Exact head 51c8ee82e6fd2fa0baf9e8ad76870f3c4e375580 then exposed a coverage-evidence defect: CI 32764379012, Production coverage job 97550390176 reported 542/544 branches, 3831/3833 lines, and 4763/4765 regions. The repair at 0a9bef98dc5a1e866207d8cf3999c0defeb9a83f added leading-claim and trailing-consequence resource-bounds regressions without changing production behavior and restored exact four-axis coverage.

Current-head canonical-format repair

An intervening contributor move produced exact head ad149e357c94a4ec050e5452be2f81b82f9b4b2e. CI 32776557846, Rust job 97588928011 failed at cargo fmt --all -- --check because crates/originweave-core/tests/release_acceptance_resource_bounds.rs lacked its canonical final newline. No production behavior failed. The canonical owner branch was repaired with the smallest formatting-only change; no product code was mutated merely to retrigger CI.

Exact-current state and evidence

Protected main is exact b05d5acca82b9d916ada2c8e82f59f92a89817e1. This PR is open, non-draft, mergeable, and currently exact head cd33b3befabf2cf2efdd8c375d4293dc4408119b against that base.

On unchanged exact head cd33b3befabf2cf2efdd8c375d4293dc4408119b:

  • CI 32782969262: success;
  • Rust contracts job 97608806449: exact-head checkout; repository contracts, canonical formatting, locked workspace/all-target check, full tests, strict Clippy, and rustdoc success;
  • Production coverage job 97608806271: exact-head checkout; owned production functions, lines, regions, and branches all exactly 100%;
  • exact coverage artifact 9540469189, exact-coverage-cd33b3befabf2cf2efdd8c375d4293dc4408119b, ZIP SHA-256 0da6085f95a395c5142c0bdf183782bd6ec1add9737726fce485f7d923582f3a;
  • SAST Semgrep 32782969238: success;
  • Manifest V3 Compatibility 32782969224: success; and
  • Security Scan 32782969166: success after bounded non-busy wait and exact-head refetch; no source mutation was used to retrigger the lane.

Formal review inventory contains automated COMMENTED evidence only; GitHub records no qualifying counted independent APPROVED review. Two current inline threads remain unresolved and intentionally stay unresolved because they are informational contract notes rather than repaired findings: one notes that the presentation filter also rejects U+2028/U+2029 beyond the documented Default_Ignorable set, and one records that duplicate-claim identity relies on the constructor's NFC/whitespace canonicalization. No predecessor-head check, review, scanner result, status, synthetic merge, queued/skipped/cancelled evidence, or model output is promoted as current-head proof or independent approval.

Scope and authority boundary

This PR does not claim release readiness and does not implement #201 signed-artifact authority, #202 enterprise operability, browser/security/recovery suite runners, benchmark persistence, model routing, or benchmark statistics. It introduces no database and no workflow mutation.

Protected-main AGENTS.md remains authoritative. Scheduled repository work does not merge, self-approve, tag, publish, mutate workflows, add secrets, or weaken gates.

@coderabbitai

coderabbitai Bot commented Aug 22, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

새로운 공개 release_acceptance 모듈이 필수 벤치마크 결과와 선언된 제한사항을 검증하고 릴리스 결정을 생성합니다. 테스트는 fail-closed 처리, 입력 경계, 유니코드 규칙 및 결과 보존을 검증합니다.

Changes

릴리스 수용

Layer / File(s) Summary
수용 계약과 입력 검증
crates/originweave-core/src/release_acceptance.rs, crates/originweave-core/src/root.rs
공개 벤치마크 스위트, 제한사항, 결정, 오류 및 보고서 형식을 추가합니다. 제한사항 입력의 텍스트와 자원 한도를 정의합니다.
결정 집계와 보고서 생성
crates/originweave-core/src/release_acceptance.rs, crates/originweave-core/tests/release_acceptance.rs
decide_release가 필수 스위트 증거와 제한사항을 집계합니다. 누락, 실패, 불충분, 중복 입력을 fail-closed로 처리하고 결정 보고서에 세부 증거를 보존합니다.
수용 동작 검증과 릴리스 기록
crates/originweave-core/tests/release_acceptance*.rs
빈 값, 주변 공백, 제어 문자, 모호한 유니코드 서식 문자, UTF-8 바이트 초과 및 제한사항 개수 초과를 검증합니다. 허용되는 국제 문자와 공백의 원문 보존도 검증합니다.

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

Merge Risk: 🟡 Moderate · up to 28cd9

The release-decision contract still accepts limitation text with surrounding spaces, which can let equivalent limitations evade duplicate detection and create ambiguous buyer-visible release reports. This is localized but affects fail-closed correctness; merge should wait for the input-validation fix and documentation follow-up.

Sequence Diagram(s)

sequenceDiagram
  participant BenchmarkEvidence
  participant decide_release
  participant DeclaredLimitation
  participant ReleaseDecisionReport
  BenchmarkEvidence->>decide_release: 결과 전달
  DeclaredLimitation->>decide_release: 제한사항 전달
  decide_release->>decide_release: 입력과 필수 스위트 검증
  decide_release->>ReleaseDecisionReport: 릴리스 결정 및 증거 반환
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.
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 제목은 fail-closed 벤치마크 릴리스 판정 계약 추가라는 변경의 핵심을 정확하고 간결하게 설명합니다.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/release-benchmark-decision-contract

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.

@seonghobae
seonghobae marked this pull request as ready for review August 22, 2026 16:17

@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: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

devin-ai-integration[bot]

This comment was marked as resolved.

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.

devin-ai-integration[bot]

This comment was marked as resolved.

seonghobae added a commit that referenced this pull request Aug 24, 2026
- Record 158 open PRs (44 ready, 114 draft) with refreshed exact base/head
  evidence for the #208-#222 release, enterprise-approval, BAP, and
  WARC/PROV chains while retaining the 2026-08-21 rows as regression anchors.
- Add governance issues #212 and #215 to the operational signal table and
  note issue #206 closure between snapshots.
- Record the required-check provider-failure RCA for the fail-closed Strix
  re-dispatches on #208, #218, and #220 without weakening the gate.
- Tighten the completion-gap contract so superseded inventory counts cannot
  pass as current evidence.
devin-ai-integration[bot]

This comment was marked as resolved.

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

Open in Devin Review

| 0x17b4..=0x17b5
| 0x180b..=0x180f
| 0x200b..=0x200f
| 0x2028..=0x202e

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📝 Info: Filter is stricter than the documented ignorable set

disallowed_release_limitation_character at release_acceptance.rs rejects 0x2028..=0x202e, two code points wider than the reviewed set documented in the unicode17 test and doctoring (0x202a..=0x202e, 4,174 points). This only rejects U+2028/U+2029 (line/paragraph separators) as well, so behavior is stricter, not wrong, and no test breaks. Align code and docs on the intended set.

Open in Devin Review

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

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

Open in Devin Review

Comment on lines +315 to +320
let mut limitation_claims = std::collections::BTreeSet::new();
for limitation in declared_limitations {
if !limitation_claims.insert(limitation.unsupported_claim()) {
return Err(ReleaseDecisionError::DuplicateLimitationClaim);
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📝 Info: Duplicate-claim dedup depends on constructor canonicalization

decide_release dedups claims by raw string identity (release_acceptance.rs:315-320). This holds only because DeclaredLimitation::new already enforces NFC and rejects surrounding whitespace. Relaxing that constructor would silently weaken the duplicate-claim guarantee.

Open in 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