test(judge): close accepted-type and ownership gaps - #778
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 change adds cumulative-threshold categorization to the LLM judge, enforces strict validation for category inputs and responses, documents the policy in ADR 0014, updates documentation status governance, and replaces fit-statistics fallback tests with fail-closed compiled-core checks. ChangesLLM judge categorization
Fit-statistics ownership verification
Documentation status governance
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ContextualOrchestratorJudge
participant LLMProvider
participant LLMJudgeResult
ContextualOrchestratorJudge->>LLMProvider: Send method-specific prompt and evaluation payload
LLMProvider-->>ContextualOrchestratorJudge: Return criterion_thresholds
ContextualOrchestratorJudge->>ContextualOrchestratorJudge: Validate thresholds and derive categories
ContextualOrchestratorJudge->>LLMJudgeResult: Preserve category_method and projected results
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 |
|
Exact-head evidence (2026-08-12 Asia/Seoul):
No merge is authorized until all required checks conclude successfully on this exact SHA and an independent approval is present. No self-approval, bypass, or force push. |
|
Exact-head status for fast-mlsirm PR #778: current head ccedd00. Local evidence remains 3471 Python tests passed with 2 optional PyYAML warnings and Rust workspace tests passed with 973 passed and 118 ignored. Remote exact-head checks have no failures; all recorded checks are terminal success/skip except the required opencode-review check, which is still queued. Aggregate reviewDecision is REVIEW_REQUIRED with no formal independent approval, and protection requires one approval with admin enforcement. No self-approval, auto-merge, or merge will be attempted. |
|
Exact-head evidence for the latest fast-mlsirm push:
This comment is tied to the exact HEAD above; a later push invalidates this evidence. No approval or merge is requested. |
|
The exact-head local MLX sweep is documented in contextual-orchestrator commit |
|
Exact-head security hardening evidence for fast-mlsirm PR #778:
This evidence is invalidated by any later push. No self-approval, bypass, or merge will be attempted. |
|
Updated exact-head evidence for PR #778:
No later push has invalidated this evidence. No self-approval, bypass, or merge will be attempted. |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@docs/adr/0014-bounded-llm-judge-category-inputs.md`:
- Around line 41-57: Add a primary-source citation and link for the
cumulative-threshold categorization method described by the ADR, along with a
brief summary of the source’s relevant findings. Place it in the decision or
supporting context near the category interpretation and score-derivation claims,
and explicitly limit the implementation claims to the scope supported by that
source.
- Line 4: Update the ADR’s Date field to the actual decision date, replacing the
future value in the document header before recording the ADR.
In `@docs/documentation_coverage.md`:
- Around line 38-40: Update the documentation around the Canonical
PyO3/public-export registry to describe it as the target architecture and future
source of truth, not the current protected-main implementation. Note that
ADR-0011 remains Proposed and protected main still uses separate native
initializers and package export paths.
In `@python/fast_mlsirm/llm_judge.py`:
- Around line 379-385: Update the category_method validation in the judge method
to reject non-string values before set membership, ensuring unhashable inputs
such as lists or dictionaries raise the existing ValueError rather than
TypeError. Add a regression test covering an unhashable category_method and
preserve the current validation for unsupported strings.
🪄 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: ef8dddcf-f59e-458b-ac21-20f6f385fdc7
📒 Files selected for processing (8)
README.mddocs/adr/0014-bounded-llm-judge-category-inputs.mddocs/adr/README.mddocs/doctoring/fitstats_sx2_person_rust_required.mddocs/documentation_coverage.mdpython/fast_mlsirm/llm_judge.pytests/test_cov_c_fitstats.pytests/test_llm_judge.py
|
Final local exact-head evidence for PR #778:
Remote required checks, including Strix, are still running on this exact SHA. This evidence is invalidated by any later push. No self-approval, bypass, or merge will be attempted. |
|
Exact-head local evidence for fast-mlsirm PR #778:
|
|
@opencode-agent review Perform a fresh read-only semantic review of exact current head 11291fd. Focus on exact built-in type boundaries for score/mode/item-type/text/criterion keys/usage, overflow and conversion-hook behavior, strict model-output parsing, contextual-orchestrator-only Judge routing, and the multi-item dichotomous/polytomous IRT contract. Do not mutate the branch, self-approve, enable auto-merge, or merge. |
|
Local exact-head evidence for c54706c\n\n- Python: 3483 passed, 2 warnings\n- Rust workspace: 973 passed, 118 ignored\n- fast-mlsirm-py release tests: 6 passed\n- The uncommitted user-owned .Jules/palette.md was preserved and not staged.\n\nRemote Strix is still in progress and an independent current-head review is still required; this comment is evidence only, not an approval or merge instruction. |
|
@opencode-agent review |
Pull request was converted to draft
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 head35f6aab194c727e15d7b4b1664efdd71f7e7398d. -
Head SHA:
35f6aab194c727e15d7b4b1664efdd71f7e7398d -
Workflow run: 31574888494
-
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 (3 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (3 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Docs (6 files)"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs (6 files)"]
R2 --> V2["docs review"]
Evidence --> S3["Test (6 files)"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test (6 files)"]
R3 --> V3["targeted test run"]
…pted-type-regression-rebased # Conflicts: # docs/documentation_coverage.md # tests/test_architecture_documentation_contract.py # tests/test_cov_c_fitstats.py # tests/test_documentation_coverage_fitness.py
2a79e97 to
eeaef39
Compare
Rebased onto current main (
|
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 head24e4f11ad4f84602696d743af71b3eeaafd85570. -
Head SHA:
24e4f11ad4f84602696d743af71b3eeaafd85570 -
Workflow run: 31617153931
-
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 (3 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (3 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Docs (12 files)"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs (12 files)"]
R2 --> V2["docs review"]
Evidence --> S3["Test (5 files)"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test (5 files)"]
R3 --> V3["targeted test run"]
Dismissing stale coverage-evidence request; required coverage-evidence is green on current tip.
Summary
Rebase
Rebased onto protected main after #788/#790. Redundant fitstats cov / docs-maturity deltas already on main via #788 were resolved in favor of main.
Validation
tests/test_jmle_optimizer_recovery.py+tests/test_llm_judge.py→ 36 passedAuto-merge when gates + non-author approval clear.