Manual multi-model AI code review (agentic swarm + native Codex/Claude) - #671
Conversation
|
/ai-review standard |
AI Review (standard)PR #671 · 18 changed files Findings
Status column reflects the verdict from the verifier: deepseek-verifier (openrouter/deepseek/deepseek-v4-pro). AI-001: Agentic lanes receive the full CI environment including unused provider secrets
Claim run_opencode_agent passes Evidence Workflow exports OPENROUTER_API_KEY plus ANTHROPIC_API_KEY, OPENAI_API_KEY, MOONSHOT_API_KEY, MINIMAX_API_KEY at lines 151–157 and 295–302. ai_review.py line 461 copies the entire environment, and the review-ro agent only disables bash/edit/write; it still runs code under the invoking user and any network-capable tool can leak the environment. Suggested fix Build an explicit allow-list environment for the subprocess containing only the key the lane actually needs (OPENROUTER_API_KEY for OpenRouter lanes, etc.) plus required non-sensitive variables such as PATH, HOME, AI_REVIEW_OUT. AI-002: OpenRouter model names include provider prefix causing API failures
Claim The matrix.json specifies model names with 'openrouter/' prefix (e.g., 'openrouter/z-ai/glm-5.2'), but openrouter_payload() sends this directly to the OpenRouter API which expects model slugs without the provider prefix (e.g., 'z-ai/glm-5.2'). Evidence In matrix.json lines 6, 12, 18, 24, 30, 38, 52, 58, 64, 70, 76, 82, 89: all model values have 'openrouter/' prefix. In openrouter_payload() line 956, lane["model"] is used directly without stripping the prefix. However, llm_dedup_candidates() line 1081 correctly uses .removeprefix("openrouter/"). Suggested fix Strip the 'openrouter/' prefix in openrouter_payload() before sending to API: model = lane["model"].removeprefix("openrouter/") AI-003: parse_name_status vulnerable to IndexError on malformed git output
Claim parse_name_status assumes git diff --name-status output for rename/copy (R/C) has at least 3 tab-separated parts (status, old_path, new_path) without bounds checking. Evidence Lines 1500-1501: accesses parts[1] and parts[2] directly. If git output is malformed or a line has fewer tabs, this will raise IndexError. Suggested fix Add bounds checking: if len(parts) >= 3 for R/C status, else treat as regular status with parts[-1] as path. AI-004: git_file_text returns empty string for zero budget, treated as valid content
Claim When max_chars <= 0, git_file_text returns ("", True) - an empty string with truncated=True. The caller in cmd_context checks 'if head_content is not None:' which treats empty string as valid content, causing empty file content to be included in context. Evidence Line 1518-1519: returns ("", True) for max_chars <= 0. Line 222-224: caller checks 'if head_content is not None:' and subtracts len(head_content) from remaining. Empty string has len 0, so remaining doesn't decrease, but empty content is still added to file_context. Suggested fix Return (None, False) for max_chars <= 0 to signal no content available, consistent with the subprocess.CalledProcessError case. AI-005: Agentic review/verifier lanes check out default branch, not the PR, for /ai-review comment triggers
Claim The openrouter-review and openrouter-verify jobs use Evidence Lines 116-117 (review) and 254-255 (verify): Suggested fix Check out the PR head/merge explicitly in both agentic jobs, e.g. AI-006: CI installs and executes remote opencode installer without checksum verification
Claim The workflow pipes Evidence Lines 137–146 and the identical block at 281–290 run the installer from the network. The preceding harden-runner step uses Suggested fix Download a pinned release archive from a trusted source (e.g. GitHub releases) and verify its SHA-256 checksum before installing. Alternatively vendor the installer in the repository. AI-008: cmd_context budget allocation divides remaining by 2 per file incorrectly
Claim The remaining budget is divided by 2 for each file's head content, then the (already reduced) remaining is divided by 2 again for base content. This causes unequal allocation and can starve base content for later files. Evidence Lines 222-227: head_content gets remaining//2, then remaining decreases by len(head_content). Then base_content gets max(0, remaining//2). For first file with large head, base gets very little. The per-file budget should be calculated upfront. Suggested fix Calculate per_file_budget = max_file_chars // max(1, len(non_deleted_files)) upfront, then allocate per_file_budget // 2 to head and base for each file. AI-009: Agent timeouts can drop findings that were already written by the model
Claim When Evidence In Suggested fix In AI-013: JSON extraction from model responses is O(n²) over response length
Claim When the model response is not fenced in triple backticks, extract_json iterates over every character and calls decoder.raw_decode from each Evidence Lines 1578–1586: Suggested fix For non-fenced text, scan for the first valid JSON value and use its end index to continue scanning, or use a single pass with a streaming decoder. AI-015: post_or_update_comment crashes if the GitHub comment-listing response body is empty
Claim
Evidence
Suggested fix Coerce Reviewer Lanes
Verification Lanes
Discarded candidates (7) — rejected by the verifier
Raw lane outputs, candidates, final issues, and model metrics are uploaded as workflow artifacts. |
1b27986 to
52633cb
Compare
response_format={type: json_object} was added in the hardening commit and
turned out to be the cause of empty model responses: it routes to
structured-output providers and makes reasoning models (minimax-m3, glm,
mimo) reason until truncated at max_tokens without ever emitting content
(observed reasoning_tokens=33989, completion_tokens=32000, findings=0).
Make response_format opt-in per lane and rely on the existing extract_json
parser, matching the request shape that works locally. Also capture
finish_reason in the lane result so truncation is visible in the report.
Without forced JSON mode the model occasionally emits invalid JSON (e.g. unescaped quotes when a finding quotes code), which strict json.loads rejects all-or-nothing, dropping a whole review to zero findings. Add an optional json-repair fallback in extract_json: try strict parsing first, and only on failure fall back to repair, flagging it as a parse warning so invalid output stays visible. Install json-repair in the review/verifier lane steps. Verified against real lane output: recovers all 6 findings that strict parsing dropped.
Applied these changes
|
A previous commit put a literal ${{ }} inside a comment in the lane run blocks.
GitHub evaluates expressions everywhere in a workflow file (including comments),
and an empty ${{ }} is invalid -> startup_failure, so no run could be created
(the label trigger silently produced nothing). Reword the comment to drop the
token.
Codex Code ReviewFinding High - Verification Ran |
Codex (correctly) flagged that pr_is_from_fork() runs inside ai_review.py, which on the pull_request (label) arm is checked out FROM the PR merge commit — so a fork PR could replace prepare and bypass the gate, emitting should_run=true with arbitrary matrix outputs. The check was in the wrong (untrusted) layer for that arm. Fix: gate the pull_request arm in the workflow `if` using the trusted event context (head.repo.full_name == base.repo.full_name), evaluated before any checkout, so a fork PR's prepare job never starts. The issue_comment arm runs prepare from the default branch (trusted), so its pr_is_from_fork check is trustworthy there; the Python check stays as that arm's gate + defense-in-depth. Docs/comments updated to explain the layering.
An independent opus security review confirmed the pwn-request hole is closed but flagged hardening worth doing: - F1: the trusted same-repo gate was enforced in only one place (prepare.if); downstream jobs that hold provider secrets / the write token and run PR-controlled ai_review.py were protected only transitively. Replicate the same-repo if-gate on openrouter-review, candidates, openrouter-verify, and final-report so it is no longer a single point of failure. - F2: model-supplied finding text (claim/evidence/suggested_fix/title) is now HTML-escaped before going into the posted comment, preventing markup/link injection into the bot comment (md_escape routes through html_escape). - F4: submit_findings/submit_verifications refuse to write unless AI_REVIEW_OUT is the expected lane-*.submit.json basename. Skipped F6 (SHA-pinning the first-party org reusable workflows) — it mainly adds update-management friction for marginal benefit when the same org owns both repos. Tests pass (incl. existing fork/lane-id guards).
- Replace the stale 'Multiple Prompts Versus One Prompt' section (and its per-model multi-prompt 'Initial policy' table listing models not in the matrix) with a short note: one generic general.md for all reviewers. - Add-a-model playbook: 'tier' -> review_lanes/verifier_lanes; 'run the tier' -> 'run the review'. - Update the example provenance lane ids to current ones (nemotron/glm/ deepseek-verifier instead of minimax-correctness/glm-standard/qwen-standard). - Document the operational caveat: native Claude + the /ai-review comment trigger only activate after merge to the default branch (claude-code-action's default-branch guard; issue_comment uses the default-branch workflow).
…acing surface There is one flow, so the standard/critical naming was vestigial where users see it: - Docs: present a single `/ai-review` command and `ai-review` label; the old `/ai-review standard|critical` forms and `ai-review-standard/-critical` labels still work (tolerant parser + allowlist) but are no longer advertised as a choice. - Report title: `## AI Review (critical)` -> `## AI Review` (the marker stays `<!-- ai-review:critical -->`, invisible, so existing comments still update). - Created the canonical `ai-review` label. Parser, label allowlist, and the internal matrix key (`critical`) are unchanged — back-compat preserved, just not surfaced as two options.
#5: the opencode installer was fetched unpinned (curl|bash) and run in a step holding all provider secrets. Now fetch it to a file, verify a pinned sha256 (fail-closed if the script changes), then run it. #4: harden-runner egress-policy audit only logged egress. Switch the lane jobs to 'block' with an allowlist harvested from a real run's harden-runner audit (GitHub Actions infra, opencode install/binary/catalog at opencode.ai + *.github usercontent + models.dev, pip + npm, and the model APIs openrouter.ai + api.minimax.io). A compromised dep/installer can no longer exfiltrate to an arbitrary host. Trade-off: adding a new direct provider requires adding its host to allowed-endpoints, or that lane is blocked. Validating with a run next.
Codex Code ReviewNo substantive issues found in the PR diff. I reviewed the new AI review workflow, Python orchestration, opencode tools, prompts, docs, and removed workflows. The fork/same-repo secret boundary is explicitly handled, lane IDs are constrained before shell/path use, model text is mostly escaped before comments, and the runner avoids obvious shell injection in the lane paths. Verification run: |
#2, #3) The single-shot review/verify path is unreachable — the workflow only runs agentic-lane (+ prepare/context/candidates/lane-error/report). Remove it: - run-lane/verify-lane subparsers + dispatch, cmd_run_lane/cmd_verify_lane, run_review_lane/run_verifier_lane (-161 lines). openrouter_chat, lane_base_result, and infer_tier_from_lane stay (the deduper + agentic path + lane-error use them). - Drop the 5 tests that covered the dead path (they were inflating apparent coverage of code the workflow no longer runs). 34 tests remain, all live paths. - general.md now flags dead/unreachable code under simplicity, so future PRs get called out for it. Not adding agentic-path unit tests: cmd_agentic_lane shells out to opencode and is impractical to test in isolation; its parsing/salvage helpers (read_submission, extract_json, dedup) are already covered.
Codex Code ReviewFindings:
Tests run: |
Fixed |
High (reviewer): json-repair was pip-installed unpinned, then imported in the lane step that holds the provider keys — a hijacked release could run import-time code with the secrets. Pin it to ==0.61.0 with sha256 hashes via a requirements file + --require-hashes (pip only honors --hash there, not on the CLI; verified locally incl. a wrong-hash negative test). Same in both lane installs. Low (reviewer): format_location(issue) was interpolated raw inside markdown code-spans in two detail sections, and file comes from model/tool output — a backtick or newline could break out and inject markdown. Add format_location_code (strips backticks/newlines; HTML is already literal inside a code span) and use it at both sites. The table cell already used md_escape.
Codex Code ReviewFindings
Verification Ran |
The validation run's minimax lane (and the new dead-code prompt) caught leftovers from the earlier single-shot removal: format_review_prompt / format_verification_prompt were only called by the deleted run_*_lane, and format_changed_files / format_file_context only by those — all now dead. Removed the cluster (-81 lines) plus the now-unused textwrap import. Full unused-function scan confirms no remaining orphans; 34 tests pass.
Codex Code ReviewFindings
I didn’t find safety/security issues in the changed automation beyond that. The added unit tests pass locally with |
…reviewer) cmd_agentic_lane left status=success when opencode failed (auth/outage/402/crash) but no findings were submitted — masking reviewer failures as 'success with 0 findings' (exactly what the OpenRouter 402 lanes did last run). Add opencode_failed() and, when nothing was submitted, mark the lane status=error if opencode reported a failure — either a non-zero exit OR an 'error' event (a 402 exits 0 but emits an error event, so the return-code check alone misses it). Applied to both the review and verify not-submitted branches; a valid submit_* result still keeps success. (The dead single-shot formatters the same review flagged were already removed in b7fb33a.) +1 test; 35 pass.
…riage Triaging ALL lane findings across the experiment runs surfaced a real regression I introduced: the dead-code commit b7fb33a swept away the module-level DEDUP_SYSTEM constant (it sat between format_file_context and the next def, so the 'delete to next def' boundary took it). llm_dedup_candidates references it inside a try/except Exception: return candidates, so every run NameError'd and silently returned candidates unchanged — the LLM dedup has been a no-op (this is the early '61 -> 61, merged nothing'). Restored the constant; added a regression test that fails if it's missing or the dedup no-ops. Also from the same triage: - Remove unused 'import urllib.parse' (dead import). - Remove MOONSHOT_API_KEY from the lane env — kimi goes via OpenRouter, the /kimi command was retired, so it was dead config (and an unstripped key). - Add a concurrency group (cancel-in-progress) so rapid re-triggers can't race and post duplicate report comments. 36 tests pass. Lesson: name-anchored 'delete to next def' is unsafe for module constants between functions — audited both dead-code commits; DEDUP_SYSTEM was the only collateral.
Codex Code ReviewHigh: secret-bearing lane jobs execute PR-controlled code. In If same-repo write access is not intentionally equivalent to provider-secret access, this is a secret exfiltration path. Use a trusted runner checkout for orchestration code and tools in the secret-bearing jobs, and keep the PR checkout only as review data, e.g. run Verification: |
cmd_context fetched base content using the new path, which doesn't exist at the base ref for a rename/copy — so renamed files silently lost their base-side context in the review. Use old_path for the base fetch when present.
- id-token (OIDC) scoped to only the native Claude job (it's the only one that needs it); removed from workflow-wide permissions so the internal jobs don't carry it. Codex job gets contents/PR/issues write only. - post_or_update_comment now paginates all comment pages, so it finds the existing report on busy PRs (>100 comments) instead of posting a duplicate. - apply_dedup_clusters keeps the richest evidence/suggested_fix across merged duplicates instead of always discarding the others'. - clean_path tolerates a trailing slash in GITHUB_WORKSPACE. Deliberately left (design/graceful/rare, per review): scoped_provider_env unknown- provider fallback, cmd_context per-file budget (graceful + agent explores), extract_json fallback heuristic, parse_name_status git-quoting, submit-unset.
After collapsing standard/critical into a single flow, 'critical' lingered as
internal naming (matrix key, tier output, the tier=='critical' gate, job names,
the comment marker). There are no tiers, so remove the concept entirely:
- matrix.json flattened to {review_lanes, verifier_lanes, deduper} (no tier key).
- prepare reads the flat matrix; parse_review_trigger returns the PR number (or
None); parse_tier_command/label -> is_review_command/is_review_label (bool).
- Drop tier from lane_base_result/build_candidates/build_final_issues, remove
infer_tier_from_lane, and the comment marker is a fixed REVIEW_COMMENT_MARKER
('<!-- ai-review -->'), not tier-keyed.
- Native jobs renamed codex-critical-review/claude-critical-review ->
codex-review/claude-review and no longer gated on tier (they run on the one
flow); dropped the tier workflow output + the tier in the artifact name.
- The native-reviews note in the report now always shows.
Note: the comment marker changed, so the next run posts a fresh report comment
on #671 once (the old marker won't match); harmless. 36 tests pass.
…privilege - Report shows a loud 'all N reviewers failed' banner when review lanes ran but none succeeded, instead of implying a clean PR (high finding). - Remove the now-dead moonshotai/ PROVIDER_KEYS mapping (MOONSHOT_API_KEY is gone and no lane uses that prefix). - Least-privilege: default workflow permissions are now read-only; only final-report (posts the comment) and the native review jobs request write/id-token. The internal prepare/context/candidates jobs no longer carry issues/PR write.
From the local opus reviewers' findings: - name_status now uses --find-renames --find-copies, matching the diff body, so rename/copy detection is consistent (the copy branch in parse_name_status was otherwise unreachable, and a heavy-edit rename could mismatch the diff). - The context job now carries the same same-repo if-gate as the other downstream jobs (defense-in-depth consistency; it checks out and runs PR code). Reviewers found no critical/high regressions, no dangling tier refs, and verified the supply-chain pins (incl. json-repair hashes vs PyPI). Remaining notes: pin the native reusable workflows to a SHA (already tracked in the PR), and openrouter_chat is now deduper-only (harmless defensive generality).
Codex Code ReviewFindings
Verification Ran |
From the in-flight run's findings (glm + Codex): - docs: drop the removed <tier> artifact-path segment; the matrix is flat now, not 'keyed critical for backward compatibility' (both stale after the de-tier). - workflow label gate: use startsWith(label, 'ai-review') instead of an exact-list contains(), matching is_review_label's prefix behavior in ai_review.py (the list rejected ad-hoc ai-review* labels the Python parser accepts). Noted (deferred): Codex flagged that a *partial* reviewer outage (some lane artifacts missing) isn't banner-flagged — only a total outage is; the per-lane Reviewer Lanes table still shows it. Fuller expected-vs-present check is a follow-up.
AI ReviewPR #671 · 14 changed files Findings
Status column reflects the verdict from the verifier: deepseek-verifier (openrouter/deepseek/deepseek-v4-pro). AI-003: extract_json ignores bare (non-fenced) JSON whenever any fenced block is present
Claim When the model output contains at least one ```json fenced block, the bare-JSON parsing branch (the Evidence In extract_json: Suggested fix Always also scan the raw AI-004: cmd_lane_error hard-requires the context file, so a missing context download cascades into no lane result
Claim cmd_lane_error reads the context.json (line 282) before writing any result. If the context download failed earlier in the lane job, the lane-error fallback itself raises FileNotFoundError, the bash script's Evidence Suggested fix AI-006: getattr on argparse attribute is unnecessary
Claim Line 295 uses Evidence Suggested fix AI-007: scoped_provider_env only strips the four hardcoded provider secrets
Claim scoped_provider_env removes OPENROUTER_API_KEY, ANTHROPIC_API_KEY, OPENAI_API_KEY, and MINIMAX_API_KEY. If a new direct provider is added (e.g. Google/Gemini, a new minimax key, etc.) with a different environment variable, that secret is not stripped and leaks into the opencode subprocess env. Evidence .github/scripts/ai_review.py lines 455-474 define PROVIDER_KEYS with exactly four prefixes; unknown providers keep the full environment. Suggested fix Document that adding a provider requires updating PROVIDER_KEYS, or centralize provider->env-var mapping in matrix.json so scoped_provider_env can strip every non-needed secret automatically. AI-010: Binary file detection only checks the first 4096 bytes for null
Claim git_file_text checks b'\x00' in result.stdout[:4096] to decide a file is binary. A binary file with no early null byte will be partially decoded and included in the review context, wasting budget and possibly producing mojibake. Evidence .github/scripts/ai_review.py line 1397: if b"\x00" in result.stdout[:4096]:. This misses nulls beyond 4 KiB. Suggested fix Check the whole content up to the requested max_chars for null bytes, or rely on git's own binary detection (e.g. git diff --numstat output starting with '-') to skip binary files before reading them. AI-011: Deduper JSON is interpolated into a single-quoted shell argument
Claim The workflow passes the deduper JSON to the candidates step as a single shell-quoted argument. Although the current matrix JSON contains no single quotes, this pattern is fragile: any future single quote or shell metacharacter in the matrix would break the command line and could inject shell tokens. Evidence .github/workflows/pr_ai_review.yaml line 276: --deduper '${{ needs.prepare.outputs.deduper }}'. The output is produced by json.dumps(..., separators=(',', ':')). Suggested fix Write the deduper JSON to a file (or use a heredoc/env var) and have ai_review.py read it from a filename, rather than embedding it directly into the shell command. AI-012: docs/ai-review.md artifact path references removed `<tier>` segment
Claim The 'Evaluation Artifacts' section lists Evidence Workflow pr_ai_review.yaml final-report upload step: Suggested fix Update the artifact path in docs/ai-review.md to Reviewer Lanes
Verification Lanes
Native Codex and Claude reviews run separately and post their own comments. They are not included in this structured provenance report. Discarded candidates (6) — rejected by the verifier
Raw lane outputs, candidates, final issues, and model metrics are uploaded as workflow artifacts. |
- Drop the unnecessary getattr(args, 'deduper', None) -> args.deduper (the candidates subparser always defines it). Pure cleanup. - Pass the deduper JSON via a DEDUPER_JSON env var instead of single-quote shell interpolation, matching the LANE_JSON/LANE_ID pattern (defense-in-depth; the source is matrix.json so not exploitable, but consistent). Left by design/risk: scoped_provider_env unknown-provider (design), extract_json bare-JSON selection (tested fallback), cmd_lane_error context dep (edge), binary null-scan window (heuristic). The <tier> doc path was already fixed in 2dec6f4.
3 participants
Summary
Adds a single, manually triggered multi-model AI code-review system for PRs.
Comment
/ai-review(or apply theai-reviewlabel) and several models reviewthe PR; a structured, de-duplicated report is posted as a PR comment, alongside
independent native Codex and Claude reviews.
This replaces the previous always-on per-model workflows
(
pr_review_{claude,codex,kimi}.yaml), which ran automatically on every PR —review is now opt-in.
How it works
Swarm → dedup → verifier → report. Open-weight finder models review the PR
in parallel (
review_lanes); their findings are merged, de-duplicated (apath/text heuristic plus a conservative single-shot LLM dedup), and a verifier
model confirms/rejects each (
verifier_lanes). A structured report is postedwith per-finding provenance (which models found it) and per-model
cost/token metrics.
Agentic via opencode. Each lane runs as an opencode agent in a read-only
sandbox (
review-ro: read/grep/glob only; bash/edit/write/patch/webfetchdenied,
external_directory: deny), so it explores the repo (unchanged files,call sites, tests) rather than judging a stuffed diff. Findings/verdicts are
returned through custom
submit_findings/submit_verificationstools — a toolcall converges far more reliably than asking an agent to hand-write final JSON.
Plus native reviews. The same trigger also runs Codex (GPT) and
Claude (opus) in the vendors' own agentic harnesses (the org's reusable
pr_review_{codex,claude}.ymlactions). They post their own independentcomments, outside the structured report.
Trigger (manual only)
/ai-reviewcomment, or theai-reviewlabel, from an OWNER / MEMBER /COLLABORATOR. Never runs automatically on PR open.
Models
Structured swarm (open-weight; OpenRouter + direct MiniMax): GLM, Kimi,
Nemotron, MiniMax → MiniMax-M3 deduper → DeepSeek verifier. Native:
Codex (GPT) and Claude (opus). One generic prompt (
general.md) forevery reviewer;
lanes/verify.mdfor the verifier.Security (pwn-request hardening)
The lane jobs hold provider secrets and execute repo code, so:
preparechecks head repo == base repo, so onlysame-repo branches (which require write access) reach the secret-bearing,
code-executing steps.
[A-Za-z0-9._-]) and passed via env, closing amatrix → shellinjection./proc/self/environto leakkeys); harden-runner egress audit; malformed model JSON recovered via
json-repair; OpenRouter calls retry transient failures.
The native Codex/Claude actions enforce their own protection — they only run
from a workflow that matches the repository's default branch.
Known gaps / notes
correctness and security, not soundness bugs (under-constrained AIRs,
Fiat-Shamir/commitment/witness-soundness mistakes). A buzzword list does not
help a model find those; real soundness review needs dedicated tooling and is
deferred.
/ai-reviewcomment trigger only activate once thismerges to
main.claude-code-actionrefuses to run from an unmergedworkflow, and
issue_commentalways uses the default-branch workflow. Thelabel trigger works pre-merge for the swarm + Codex.
mutable action tags (
step-security/harden-runner@v2,yetanotherco/actions/...@v1.0.0,actions/checkout@v4, …); a force-movedtag would run attacker code with the job's secrets. The egress allowlist now
limits exfil even then, but pinning to full commit SHAs is the standard fix —
deferred so it can be applied as a repo-wide convention.
PR only as read-only review data) is an optional future step; fork-blocking
already lands on the same effective trust boundary.
Key files
.github/scripts/ai_review.py— orchestrator (prepare / context / agentic-lane/ candidates / final-report)
.github/workflows/pr_ai_review.yaml— trigger, swarm matrix jobs, nativereview calls
.github/ai-review/matrix.json— finders / verifier / deduper.github/ai-review/prompts/—general.md,lanes/verify.md.opencode/agent/review-ro.md,.opencode/tools/submit_*.tsdocs/ai-review.md— full design + lessons learned.github/scripts/test_ai_review.py— offline unit tests (CI:pr_ai_review_tests.yaml)