fix(rubric): redact hostile iterable failures - #801
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:
📝 WalkthroughWalkthroughThe PR adds cumulative-threshold LLM-judge categorization and strict input validation, hardens rubric iterable error boundaries, updates L-BFGS convergence behavior, adds Rust optimizer recovery tests, and expands governance, licensing, and research documentation. ChangesLLM judge validation and categorization
Rubric iterable error boundaries
Native optimizer validation
Documentation governance and research references
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant ContextualOrchestratorJudge
participant Model
participant IRTProjection
Caller->>ContextualOrchestratorJudge: submit category_method and category_count
ContextualOrchestratorJudge->>Model: request direct categories or Boolean thresholds
Model-->>ContextualOrchestratorJudge: return structured judgment
ContextualOrchestratorJudge->>IRTProjection: validate and derive categories
IRTProjection-->>Caller: return serialized LLMJudgeResult
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
ae5c879 to
c678e90
Compare
|
@OpenCode address Bounded current-head repair for exact branch head The branch now correctly contains only the rubric production/test delta plus authoritative fragment |
96951ec to
2a79e97
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
tests/test_cov_c_fitstats.py (1)
191-191: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename the tests to match the fail-closed behavior.
test_sx2_numpy_fallback_realistic,test_sx2_numpy_fallback_spatial_and_dim_floors,test_person_fit_numpy_fallback, andtest_infit_outfit_numpy_fallbacknow patch an incomplete core and expectRuntimeError. They no longer test NumPy fallback behavior. The spatial test also no longer exercises the dimension floors because execution stops at the missing native entry point.Rename the tests to describe incomplete-core fail-closed coverage. Keep separate native-core tests for any retained floor behavior.
Proposed renames
-def test_sx2_numpy_fallback_realistic(monkeypatch): +def test_sx2_incomplete_core_fails_closed_realistic(monkeypatch): -def test_sx2_numpy_fallback_spatial_and_dim_floors(monkeypatch): +def test_sx2_incomplete_core_fails_closed_spatial(monkeypatch): -def test_person_fit_numpy_fallback(monkeypatch): +def test_person_fit_incomplete_core_fails_closed(monkeypatch): -def test_infit_outfit_numpy_fallback(monkeypatch): +def test_infit_outfit_incomplete_core_fails_closed(monkeypatch):Also applies to: 208-208, 221-221, 231-231
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_cov_c_fitstats.py` at line 191, Rename the four tests currently named test_sx2_numpy_fallback_realistic, test_sx2_numpy_fallback_spatial_and_dim_floors, test_person_fit_numpy_fallback, and test_infit_outfit_numpy_fallback to names describing incomplete-core fail-closed behavior and their expected RuntimeError. Remove the misleading NumPy-fallback and dimension-floor wording; preserve any dimension-floor coverage only in separate native-core tests.docs/doctoring/jmle_optimizer_recovery_evidence.md (1)
31-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winState which stopping rule produces the
"converged"status.This PR adds a relative objective-reduction stopping rule to the Rust L-BFGS loop. That rule can set
"converged"while the gradient norm is still above1e-5. Record this in the evidence document, so a reader does not read "convergence remains a mandatory gate" as gradient-norm convergence only.📝 Suggested addition
-The initial 500-iteration evidence reached the configured iteration ceiling for L-BFGS and the hybrid mode rather than satisfying their convergence contract. The follow-up evidence expands the optimizer budget to 2,000 iterations without relaxing the `1e-5` tolerance or any recovery threshold; convergence remains a mandatory gate. +The initial 500-iteration evidence reached the configured iteration ceiling for L-BFGS and the hybrid mode rather than satisfying their convergence contract. The follow-up evidence expands the optimizer budget to 2,000 iterations without relaxing the `1e-5` tolerance or any recovery threshold; convergence remains a mandatory gate. + +The Rust L-BFGS loop reports `converged` from either the gradient-norm rule or the relative objective-reduction rule added in this change. Record which rule terminated each mode when the evidence is regenerated, because the objective-reduction rule can stop the loop while the gradient norm is still above the configured tolerance.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/doctoring/jmle_optimizer_recovery_evidence.md` at line 31, Update the evidence document’s convergence statement to explicitly identify the relative objective-reduction stopping rule in the Rust L-BFGS loop as capable of producing the "converged" status, including when the gradient norm remains above 1e-5; clarify that the mandatory gate uses this rule rather than implying gradient-norm convergence only.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/mlsirm-core/src/jmle_opt.rs`:
- Around line 361-376: Update lbfgs_accepts_relative_objective_convergence to
match the corrected stopping behavior: do not assert a converged status while
stopping at x[0] == 0.5 for an objective whose minimum is x[0] == 1.0. Assert
the distinct status or tolerance-driven outcome introduced by the stopping-rule
change, and make the assertions explicitly validate objective-reduction
convergence rather than reaching the minimum.
- Around line 173-181: Separate the objective-reduction tolerance from the
gradient tolerance in the optimizer’s stopping logic around the L-BFGS iteration
flow, and require the function-reduction criterion to hold on consecutive
iterations before stopping. Return a distinct non-“converged” status for
objective-based termination, while reserving “converged” for the gradient
criterion; update the NumPy L-BFGS reference path to use the same tolerance
semantics, consecutive-iteration rule, and status string, preserving Rust/NumPy
parity.
In `@docs/adr/0013-continuous-execution-and-documentation-governance.md`:
- Around line 68-76: Update the “Current review finding and remediation” section
in ADR-0013 to remove references to the active review and active PR, preserving
only the durable requirement that the documentation matrix separately describe
the target architecture and protected-main behavior until ADR-0011 is
implemented.
In `@docs/papers/README.md`:
- Around line 50-52: Update the paper title in the bibliography entry for Zheng
et al. to hyphenate the compound modifier as “multiple-choice selectors,”
preserving the rest of the citation unchanged.
- Around line 50-62: In the citation-only entries for Zheng et al. and Samejima
in the papers README, add one concise scope-summary sentence to each: describe
Zheng et al. as addressing selector/order sensitivity, and Samejima as defining
ordered cumulative boundaries for graded categories. Preserve the existing
licensing explanations and the stated limits on prompt-fairness and positive-K
claims.
In `@tests/test_jmle_optimizer_recovery.py`:
- Around line 123-146: Add module-level pytest.importorskip for
fast_mlsirm._core with reason “compiled Rust core required” in
tests/test_jmle_optimizer_recovery.py, before tests execute. Keep the explicit
backend="rust" configuration in
test_rust_jmle_optimizer_modes_recover_known_parameters unchanged.
---
Nitpick comments:
In `@docs/doctoring/jmle_optimizer_recovery_evidence.md`:
- Line 31: Update the evidence document’s convergence statement to explicitly
identify the relative objective-reduction stopping rule in the Rust L-BFGS loop
as capable of producing the "converged" status, including when the gradient norm
remains above 1e-5; clarify that the mandatory gate uses this rule rather than
implying gradient-norm convergence only.
In `@tests/test_cov_c_fitstats.py`:
- Line 191: Rename the four tests currently named
test_sx2_numpy_fallback_realistic,
test_sx2_numpy_fallback_spatial_and_dim_floors, test_person_fit_numpy_fallback,
and test_infit_outfit_numpy_fallback to names describing incomplete-core
fail-closed behavior and their expected RuntimeError. Remove the misleading
NumPy-fallback and dimension-floor wording; preserve any dimension-floor
coverage only in separate native-core tests.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0d1772f1-994d-496b-ab1c-97b9bb634b4c
⛔ Files ignored due to path filters (3)
docs/papers/papers/li-2025-evaluating-scoring-bias-llm-as-a-judge.pdfis excluded by!**/*.pdfdocs/papers/papers/pezeshkpour-hruschka-2024-option-order-sensitivity.pdfis excluded by!**/*.pdfdocs/papers/papers/sharma-2023-sycophancy.pdfis excluded by!**/*.pdf
📒 Files selected for processing (21)
CHANGELOG.mdREADME.mdcrates/mlsirm-core/src/jmle_opt.rsdocs/adr/0013-continuous-execution-and-documentation-governance.mddocs/adr/0014-bounded-llm-judge-category-inputs.mddocs/adr/README.mddocs/changelog.d/783-rubric-iterable-error-boundary.mddocs/doctoring/fitstats_sx2_person_rust_required.mddocs/doctoring/jmle_optimizer_recovery_evidence.mddocs/documentation_coverage.mddocs/papers/README.mddocs/papers/directive-irt-coverage.mddocs/papers/oa-pdf-manifest.mdpython/fast_mlsirm/llm_judge.pypython/fast_mlsirm/rubric/models.pytests/test_cov_c_fitstats.pytests/test_fitstats_infit_outfit_allocations.pytests/test_jmle_optimizer_recovery.pytests/test_llm_judge.pytests/test_llm_judge_description_boundary.pytests/test_rubric_iterable_error_boundary.py
💤 Files with no reviewable changes (1)
- tests/test_fitstats_infit_outfit_allocations.py
2a79e97 to
7a19649
Compare
Fail closed on hostile iterable setup/iteration with package-owned messages while preserving MemoryError. Rebuilt on current main after
43245ac to
a03bad3
Compare
![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 heada03bad35a57755394d9eb223951bb263533782c7. -
Head SHA:
a03bad35a57755394d9eb223951bb263533782c7 -
Workflow run: 31632747083
-
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["Changed file (2 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (2 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Docs: 783-rubric-iterable-error-boundary.md"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs: 783-rubric-iterable-error-boundary.md"]
R2 --> V2["docs review"]
Evidence --> S3["Test: test_rubric_iterable_error_boundary.py"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test: test_rubric_iterable_error_boundary.py"]
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 evidence job did not run or did not publish coverage evidence. Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (2 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (2 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Docs: 783-rubric-iterable-error-boundary.md"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs: 783-rubric-iterable-error-boundary.md"]
R2 --> V2["docs review"]
Evidence --> S3["Test: test_rubric_iterable_error_boundary.py"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test: test_rubric_iterable_error_boundary.py"]
R3 --> V3["targeted test run"]
|
Dismissing stale coverage-evidence request; required coverage-evidence is green on current tip.
1 participant
Summary
MemoryErrorresource signals.Supersedes
Test plan
tests/test_rubric_iterable_error_boundary.pySummary by CodeRabbit
New Features
Bug Fixes
Documentation