fix(rubric): harden text schema callback boundary - #992
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughRubric schema validation now rejects caller-defined ChangesRubric text trust boundary
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change narrowly hardens schema validation for hostile string subclasses while preserving exact-string behavior, and no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head9a46ff2f21c62d9d4b7be49af798c0962387a7ed. -
Head SHA:
9a46ff2f21c62d9d4b7be49af798c0962387a7ed -
Workflow run: 32174385733
-
Workflow attempt: 1
Coverage evidence
Coverage evidence job did not run or did not publish coverage evidence.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Docs: 991-rubric-text-trust-boundary.md"]
S1 --> I1["operator or user guidance"]
I1 --> R1["Review risk: Docs: 991-rubric-text-trust-boundary.md"]
R1 --> V1["docs review"]
Evidence --> S2["Changed file: models.py"]
S2 --> I2["repository behavior"]
I2 --> R2["Review risk: Changed file: models.py"]
R2 --> V2["required checks"]
Evidence --> S3["Test (2 files)"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test (2 files)"]
R3 --> V3["targeted test run"]
OpenCode Review Overview
Pull request overviewOpenCode cannot approve yet because required coverage evidence did not pass. Review outcome1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
Coverage evidenceCoverage Decision
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Docs: 991-rubric-text-trust-boundary.md"]
S1 --> I1["operator or user guidance"]
I1 --> R1["Review risk: Docs: 991-rubric-text-trust-boundary.md"]
R1 --> V1["docs review"]
Evidence --> S2["Changed file (4 files)"]
S2 --> I2["repository behavior"]
I2 --> R2["Review risk: Changed file (4 files)"]
R2 --> V2["required checks"]
Evidence --> S3["Test (4 files)"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test (4 files)"]
R3 --> V3["targeted test run"]
|
|
@opencode-agent Please re-review the current exact head |
|
@opencode-agent Please re-review the current exact head |
|
@opencode-agent Please re-review current exact head |
|
@opencode-agent @cwl-noema-review Please independently review exact current head |
|
@opencode-agent review Fresh exact-head re-review requested for |
|
Scheduled review-feedback autofix for this PR head.
|
|
@opencode-agent @cwl-noema-review Re-review the exact current head The prior current-head REQUEST_CHANGES only reported unavailable/failed coverage evidence from an earlier run. Current CI, Security Scan, SAST Semgrep, and CodeQL runs for this exact head are terminal-success. Re-evaluate this head and publish independent approvals only from fresh evidence; do not reuse prior-head reviews. Protected merge policy remains unchanged. |
|
Scheduled review-feedback autofix for this PR head.
|
|
@opencode-agent Please perform a review-only formal review of exact current PR head |
|
Current-head review record: 777a8c8. CodeGraph review confirmed the PR keeps callback-sensitive rubric text and enum normalization at the Python trust boundary, with scoring metadata scalar normalization producing inert built-in values. The only local 100% docstring gap was the new nested hostile text fixture; its class and strip method now have docstrings. Focused behavioral tests: 96 passed; Ruff, compileall, interrogate 100%, and diff clean. Hosted checks must rerun and be read against this exact SHA. Normal squash auto-merge is enabled as the only merge path and still requires the independent formal review gate. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
Added current-head callback-boundary remediation: item-bank lifecycle evidence enums now admit only exact built-in strings or exact enum members, and a hostile-string regression proves no equality/hash callback dispatch. Focused tests: 24 passed; Ruff, compileall, interrogate 100%, and diff checks passed. The synchronized head is now d559e55; waiting only for the normal exact-head protected checks/review path. |
opencode-agent
Bot
added
area: data
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current headd559e55f1e5d5d07d3da47c1b2d67e0ea9a083ca. -
Head SHA:
d559e55f1e5d5d07d3da47c1b2d67e0ea9a083ca -
Workflow run: 32623122947
-
Workflow attempt: 1
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 2
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Docs: 991-rubric-text-trust-boundary.md"]
S1 --> I1["operator or user guidance"]
I1 --> R1["Review risk: Docs: 991-rubric-text-trust-boundary.md"]
R1 --> V1["docs review"]
Evidence --> S2["Changed file (4 files)"]
S2 --> I2["repository behavior"]
I2 --> R2["Review risk: Changed file (4 files)"]
R2 --> V2["required checks"]
Evidence --> S3["Test (4 files)"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test (4 files)"]
R3 --> V3["targeted test run"]
|
Scheduled review-feedback autofix for this PR head.
|
…st (#1324) * fix(ci): pin grm-recovery rust toolchain and align metadata scalar test Two independent contract-test regressions on main, both caught by the repository's own CI (python 3.12/3.14 jobs), unrelated to any in-flight PR's diff: - .github/workflows/statistical-studies.yml: PR #1012 added the grm-recovery job's dtolnay/rust-toolchain step without the `with: toolchain: 1.97.1` pin every sibling step carries, letting it silently float to the default stable channel. Added the missing pin and updated test_rust_toolchain_contract.py's expected step count for statistical-studies.yml from 4 to 5 to reflect the now-fully-pinned, legitimate fifth job. - tests/test_scoring_contract_metadata.py: PR #992 introduced _normalize_metadata_scalar, which safely accepts str/int/float subclasses by copying their value through the inert base-type descriptor (str.__str__, matching the existing int.__int__/ float.__float__ handling in the same function) without invoking any subclass-defined method. The pre-existing test_metadata_rejects_string_subclasses_before_callbacks predates that change and still asserted outright rejection. Updated the test to assert the actual, intentional behavior: zero callback invocations and safe normalization to a plain str. * fix(tests): strengthen hostile metadata subclass check, fix stale changelog ref Addresses two CodeRabbit findings on this PR: - The changelog fragment still named the test's old identifier (test_metadata_rejects_string_subclasses_before_callbacks) after it was renamed to test_metadata_normalizes_string_subclasses_without_callbacks. - _HostileMetadataText only overrode strip/encode, so the test could not detect whether normalization happened through the safe str.__str__ base descriptor or the caller-overridable str(value) path. Added a __str__ override that raises, closing that gap. Confirmed the test still passes: _normalize_metadata_scalar genuinely uses the safe str.__str__ descriptor, not the overridable path. * fix(changelog): resync rendered CHANGELOG.md with the updated fragment The prior commit renamed the test referenced in docs/changelog.d/1300-ci-contract-drift.md but didn't re-run render_changelog_fragments.py --update, leaving the rendered Unreleased block carrying the old test name. Caught by Devin review: render_changelog_fragments.py --check, run by the release-tag workflow, fails closed on this exact mismatch. --------- Co-authored-by: Claude <noreply@anthropic.com>
1 participant
Closes #991.
Closes #996.
Scope
Harden the shared rubric/scoring text and metadata-scalar trust boundary without moving any psychometric/statistical arithmetic out of Rust.
rubric.models._text()admits exact built-in strings before normalization, preventing hostilestrsubclasses from executing overridden text callbacks.rubric.audit._enum_value()applies the same exact-member/exact-built-in-string admission rule toCandidateAuditFinding.severity,CandidateAuditReport.lifecycle_state, and pilot-admission lifecycle state.str/int/floatsubclasses through inert built-in descriptors before generic JSON freezing, so subclassencode, comparison, or numeric-conversion callbacks cannot execute during canonicalization.Evidence lineage
8881e5d...→35dd1fa...→3363717.../6f7fd8b....b424d17...→ecb481a....5e34ccc...→744b9dd....59722b8fb5c0854069d64c06a7f485789222c581.CI root cause and correction
The previous exact-head Python 3.12 and 3.14 failures occurred during pytest collection, not product execution: placing
_HostileText("pilot")directly inpytest.mark.parametrizecaused pytest ID generation to call the intentionally hostile.encode()method. The regression now parametrizes trusted raw values/types and constructs the hostile scalar inside the test body, preserving the public callback-safety assertion without executing caller code during collection.Current exact-head evidence
Current head:
59722b8fb5c0854069d64c06a7f485789222c581against protectedmain@04d0bc21a2a20693bcf16108cd76d394fe844d23.Repository CI, Security Scan, CodeQL, and Semgrep are terminal-success on this exact head, and current inline review threads are empty. Two formal OpenCode
CHANGES_REQUESTEDreviews remain in history: the first evaluated predecessor9a46ff2...; the second evaluated this exact head and failed only because centralcoverage-evidencerun32213304749could not materialize the trusted base Python lock before PR-controlled tests ran. The first causal boundary was central.githubtooling, not this repository's product source. Central protected main has since advanced to9e9f59f3ac1e96a960c021b131d768c238f4c21avia.github#1136, which repairs the trusted-uv target-triple verification boundary. Fresh same-head OpenCode/coverage review has been requested under that corrected central contract. Keep this PR unmerged until the new same-head required evidence is terminal-success; do not transfer either predecessor-head evidence or the pre-fix infrastructure verdict as product approval.This is Python schema validation/canonicalization only. No likelihood, estimator, optimizer, score statistic, or other psychometric arithmetic moves out of Rust.