Skip to content
Merged
31 changes: 30 additions & 1 deletion scripts/ci/opencode_review_normalize_output.py
Original file line number Diff line number Diff line change
Expand Up @@ -864,6 +864,32 @@ def reject(reason: str) -> int:
return 0


def canonicalize_finding_fields(finding: dict[str, Any]) -> dict[str, Any]:
"""Map known-safe model vocabulary drift onto the canonical finding schema.

Findings only exist on REQUEST_CHANGES control blocks (valid_control rejects
APPROVE blocks that carry findings), so rescuing a drifted finding can only
publish a blocking review — it can never loosen approval evidence. Two
observed drifts from otherwise-complete blocks are repaired: ``priority``
used in place of ``severity``, and a missing ``suggested_diff`` when
``fix_direction`` still states the concrete remedy.
"""

def non_empty(candidate: Any) -> bool:
Comment thread
github-actions[bot] marked this conversation as resolved.
Resolved
"""Return whether a candidate field value is a non-blank string."""
return isinstance(candidate, str) and bool(candidate.strip())

finding = dict(finding)
priority = finding.pop("priority", None)
if not non_empty(finding.get("severity")) and non_empty(priority):
finding["severity"] = priority
if not non_empty(finding.get("suggested_diff")) and non_empty(
finding.get("fix_direction")
):
finding["suggested_diff"] = finding["fix_direction"]
Comment thread
github-actions[bot] marked this conversation as resolved.
Resolved
return finding


def valid_control(
value: Any,
*,
Expand Down Expand Up @@ -944,15 +970,18 @@ def valid_control(
"regression_test_direction",
"suggested_diff",
)
normalized_findings = []
for finding in findings:
if not isinstance(finding, dict):
return None
line = finding.get("line")
if isinstance(line, bool) or not isinstance(line, int) or line <= 0:
return None
finding = canonicalize_finding_fields(finding)
for field in required_finding_fields:
if not isinstance(finding.get(field), str) or not finding[field].strip():
return None
normalized_findings.append(finding)

normalized = {
"head_sha": value["head_sha"],
Expand All @@ -961,7 +990,7 @@ def valid_control(
"result": result,
"reason": reason,
"summary": summary,
"findings": findings,
"findings": normalized_findings,
}
if isinstance(value.get("adversarial_validation"), dict):
normalized["adversarial_validation"] = value["adversarial_validation"]
Expand Down
58 changes: 58 additions & 0 deletions tests/test_opencode_review_normalize_output.py
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,64 @@ def test_valid_control_filters_shape_head_and_review_contract():
assert norm.valid_control(approve_without_findings_key, **kwargs)["findings"] == []


def test_valid_control_canonicalizes_known_safe_finding_field_drift():
kwargs = {
"expected_head_sha": "head",
"expected_run_id": "run",
"expected_run_attempt": "attempt",
}

aliased = finding(priority="P1")
del aliased["severity"]
normalized = norm.valid_control(
control(result="REQUEST_CHANGES", findings=[aliased]), **kwargs
)
assert normalized is not None
assert normalized["findings"][0]["severity"] == "P1"
assert "priority" not in normalized["findings"][0]

diffless = finding()
del diffless["suggested_diff"]
normalized = norm.valid_control(
control(result="REQUEST_CHANGES", findings=[diffless]), **kwargs
)
assert normalized is not None
assert normalized["findings"][0]["suggested_diff"] == "Restore the guard."

blank_diff = finding(suggested_diff=" ")
normalized = norm.valid_control(
control(result="REQUEST_CHANGES", findings=[blank_diff]), **kwargs
)
assert normalized is not None
assert normalized["findings"][0]["suggested_diff"] == "Restore the guard."

canonical_severity_wins = finding(priority="P2")
normalized = norm.valid_control(
control(result="REQUEST_CHANGES", findings=[canonical_severity_wins]),
**kwargs,
)
assert normalized is not None
assert normalized["findings"][0]["severity"] == "HIGH"
assert "priority" not in normalized["findings"][0]

blank_alias = finding(priority=" ")
del blank_alias["severity"]
assert (
norm.valid_control(
control(result="REQUEST_CHANGES", findings=[blank_alias]), **kwargs
)
is None
)

no_remedy = finding(fix_direction="", suggested_diff="")
assert (
norm.valid_control(
control(result="REQUEST_CHANGES", findings=[no_remedy]), **kwargs
)
is None
)


def test_valid_control_repairs_approval_summary_from_bounded_evidence(tmp_path, monkeypatch):
evidence = tmp_path / "bounded-review-evidence.md"
evidence.write_text(
Expand Down
Loading