Repository navigation
Produce explicit carried-finding rechecks in Direct and Strict reviews #180
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| """Validate explicit rechecks without deriving resolution from omission.""" | ||
| from __future__ import annotations | ||
|
|
||
| import re | ||
| from .models import InvalidRequestError | ||
|
|
||
|
|
||
| def validate_carried(rows): | ||
| ids = set() | ||
| if not isinstance(rows, (list, tuple)) or not all(isinstance(row, dict) for row in rows): | ||
| raise InvalidRequestError("carried findings must be a list of objects") | ||
| if len(rows) > 200: | ||
| raise InvalidRequestError("at most 200 carried findings per review") | ||
| for row in rows: | ||
| identity = row.get("finding_id") | ||
| if not isinstance(identity, str) or not identity.strip() or identity in ids: | ||
| raise InvalidRequestError("carried findings require unique nonempty identities") | ||
| if not re.fullmatch(r"[0-9a-fA-F]{40}", str(row.get("source_head_sha") or "")): | ||
| raise InvalidRequestError("carried finding source_head_sha must be a full SHA") | ||
| if not str(row.get("title") or "").strip(): | ||
| raise InvalidRequestError("carried finding title must not be empty") | ||
| ids.add(identity) | ||
| return ids | ||
|
|
||
|
|
||
| def checked_rechecks(carried, records, head_sha): | ||
| """Return only safe records and explicit gaps; duplicates never resolve.""" | ||
| expected = {row["finding_id"] for row in carried} | ||
| missing = [] | ||
| safe = [] | ||
| seen = set() | ||
| duplicates = set() | ||
| if not isinstance(records, (list, tuple)): | ||
| records = () | ||
| for row in records: | ||
| if not isinstance(row, dict): | ||
| missing.append("invalid finding recheck") | ||
| continue | ||
| identity = row.get("finding_id") | ||
| if not isinstance(identity, str) or identity not in expected: | ||
| missing.append("recheck names an unknown carried finding") | ||
| continue | ||
| if identity in seen: | ||
| duplicates.add(identity) | ||
| missing.append("duplicate finding recheck: " + identity) | ||
| seen.add(identity) | ||
| if (row.get("head_sha") != head_sha | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For Direct requests that use uppercase hexadecimal SHAs, Useful? React with 👍 / 👎. |
||
| or not isinstance(row.get("outcome"), str) | ||
| or row.get("outcome") not in {"fixed", "still_affected", "unverified"} | ||
| or not isinstance(row.get("evidence"), str) or not row["evidence"].strip()): | ||
| missing.append("invalid or wrong-head finding recheck: " + identity) | ||
| continue | ||
| safe.append({key: row[key] for key in ("finding_id", "head_sha", "outcome", "evidence")}) | ||
| safe = [row for row in safe if row["finding_id"] not in duplicates] | ||
| answered = {row["finding_id"] for row in safe} | ||
| missing.extend("missing finding recheck: " + identity for identity in sorted(expected - answered)) | ||
| return safe, missing | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the recheck agent returns valid
finding_recheckswith the permitted base statusneeds_review—a likely response when a finding isstill_affected—run_agent_stepsetsrecheck_result.okto false, so this branch discards every answer by passing an empty list tochecked_rechecks. The structured result then reports all carried IDs as missing instead of returning their explicit outcomes; validate the returned records independently of the agent's overall status, while recording any escalation separately.Useful? React with 👍 / 👎.