feat(essay): add governed validation evidence reports - #523
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:
📝 WalkthroughWalkthroughAdded a governed essay-validation evidence report API. The API validates provenance and engine scope, delegates metrics to Rust, emits immutable deterministic reports, routes mandatory review triggers, and excludes pass/fail decisions. Documentation, tests, exports, and a one-time workflow were added. ChangesEssay validation reporting
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant build_essay_validation_evidence_report
participant validate_judge
participant EssayValidationEvidenceReport
Caller->>build_essay_validation_evidence_report: provide validation inputs
build_essay_validation_evidence_report->>validate_judge: compute Rust-backed metrics
validate_judge-->>build_essay_validation_evidence_report: return validation outputs
build_essay_validation_evidence_report->>EssayValidationEvidenceReport: create sealed report
EssayValidationEvidenceReport-->>Caller: return deterministic evidence
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/test_scoring_essay_validation_reporting.py (1)
167-179: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the expected metric map from gate names, not gate positions.
Lines 168-172 bind each metric to a fixed index in
expected.gates. This test is the only guard that the adapter maps Rust values to the correct metric identity. If the Rust kernel reordersgates, the expected map pairs the wrong values, and the assertion can still pass when two reordered values are numerically close.Build the expected map from
gate["name"]through the production name map so the test fails on any reordering.♻️ Proposed refactor
- expected_values = { - "quadratic_weighted_kappa": expected.gates[0]["value"], - "pearson_correlation": expected.gates[1]["value"], - "standardized_mean_difference": expected.gates[2]["value"], - "human_machine_degradation": expected.gates[3]["value"], - "worst_subgroup_standardized_mean_difference": expected.gates[4]["value"], - "exact_agreement": expected.exact_agreement, - "adjacent_agreement": expected.adjacent_agreement, - } + expected_values = { + validation_reporting._METRIC_NAME_MAP[gate["name"]]: gate["value"] + for gate in expected.gates + } + expected_values["exact_agreement"] = expected.exact_agreement + expected_values["adjacent_agreement"] = expected.adjacent_agreement + assert set(expected_values) == set(_ALL_METRIC_IDS)🤖 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_scoring_essay_validation_reporting.py` around lines 167 - 179, Update the expected_values construction in the scoring report test to derive gate values by each gate’s "name", using the production name-to-metric-ID map rather than fixed expected.gates indices. Keep the direct exact_agreement and adjacent_agreement expectations, and ensure the resulting keys align with _ALL_METRIC_IDS so gate reordering causes the assertion to fail.
🤖 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 @.github/workflows/one-time-essay-validation-test-fix.yml:
- Around line 45-49: Update the workflow-repair logic around the old/new
assertion block checks to count both patterns before modifying or deleting the
workflow. Proceed only when exactly one legacy block exists and replace it, or
exactly one corrected block exists with no legacy block; otherwise raise an
error. Ensure the later workflow deletion path runs only after the target file
is confirmed to be in the corrected state.
In `@python/fast_mlsirm/scoring/essay/validation_reporting.py`:
- Around line 470-483: Update the category_count validation immediately before
the validate_judge call to reject values outside the inclusive 2–1000 range,
while preserving the existing exact-integer check and
invalid_essay_validation_category_count error. Ensure validate_judge receives
category_count as k only after both validations pass.
In `@tests/test_scoring_essay_validation_reporting.py`:
- Line 195: Strengthen the privacy assertion in the relevant scoring validation
test by checking the full serialized payload rather than only top-level payload
values. Ensure `_AUTOMATED.tolist()` is absent from the serialized
representation, covering nested structures such as metadata and metrics while
preserving the existing no-label-vector requirement.
---
Nitpick comments:
In `@tests/test_scoring_essay_validation_reporting.py`:
- Around line 167-179: Update the expected_values construction in the scoring
report test to derive gate values by each gate’s "name", using the production
name-to-metric-ID map rather than fixed expected.gates indices. Keep the direct
exact_agreement and adjacent_agreement expectations, and ensure the resulting
keys align with _ALL_METRIC_IDS so gate reordering causes the assertion to fail.
🪄 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: 93700b66-8ebc-4a90-af5d-20c1c2b90cb4
📒 Files selected for processing (8)
.github/workflows/one-time-essay-validation-test-fix.ymlCHANGELOG.mddocs/automated_essay_validation_evidence_reports.mddocs/changelog.d/essay-validation-evidence-report.mdpython/fast_mlsirm/scoring/essay/__init__.pypython/fast_mlsirm/scoring/essay/validation_reporting.pytests/test_scoring_essay_contracts.pytests/test_scoring_essay_validation_reporting.py
Summary
Scientific boundary
This slice does not establish construct validity, fairness, reliability, scorer interchangeability, model preference, causal utility, or authorization for consequential deployment. Correlation is descriptive only and no universal metric threshold is asserted.
Closes part of #397.
Summary by CodeRabbit
New Features
Documentation
Tests