From b69b274c862558303dc0f7a743d675f3d688dd24 Mon Sep 17 00:00:00 2001 From: LearningCircuit <185559241+LearningCircuit@users.noreply.github.com> Date: Mon, 14 Sep 2026 08:47:09 +0200 Subject: [PATCH 01/17] feat: configurable commit overview with per-author counts and line totals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Split the commit-history context by token cost. The reviewer now always can be told the situation at a glance — "There are X commits already on this PR" — with per-author commit counts and added/removed line totals, while the fully quoted commit messages (the token-expensive part) get their own, smaller cap: - MAX_SUMMARY_COMMITS (default 15): how many of the PR's most recent commits the overview reads. Line stats are not part of the pulls list response, so each summarized commit costs one extra GitHub API call; the cap bounds that, and 0 keeps the count-only header. - MAX_COMMIT_MESSAGES (default 5, was a hard-coded 15): how many commit messages are fully quoted in the prompt. - INCLUDE_COMMIT_SUMMARY (default true): toggles the overview. The PR commit list is now fetched once and shared by both features (--paginate output slurped into a single array, which also makes the "most recent N" truncation global instead of per-page). Merge commits are excluded from counts and stats; authors fall back to the commit author name when there is no linked GitHub account; failed stats fetches count as zero instead of aborting the review. Offline regression tests cover the overview text, both caps acting independently, merge exclusion, the zero-reads mode, and the singular phrasing. --- README.md | 6 +- ai-reviewer.sh | 132 ++++++++++++++++++++++++-- tests/test_reviewer_context.py | 166 +++++++++++++++++++++++++++++++-- 3 files changed, 289 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index e4ca42a..51b7236 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,7 @@ This guide explains how to set up the automated AI PR review system using OpenRo **Latest Updates:** - **Thinking Model Support**: Now supports advanced reasoning models like Kimi K2 that use `` tags - **Rich Context**: Includes PR descriptions, commit messages, and human comments for comprehensive reviews +- **Commit Overview**: Tells the model how many commits are already on a PR, who authored them, and how many lines each author changed — with caps configured separately from the (token-heavy) fully quoted messages - **Higher Token Limits**: Default 64k tokens for complete reviews without truncation - **Smart Context Management**: Only fetches most recent AI review to save tokens - **Enhanced Error Handling**: Robust parsing of various AI response formats @@ -72,6 +73,9 @@ The workflow is pre-configured with sensible defaults, but you can customize it - For large PRs with thinking models, this prevents cut-off responses - Adjust lower for cost savings on smaller PRs - **MAX_DIFF_SIZE**: Maximum diff size in bytes (default: `800000` / 800KB) +- **MAX_SUMMARY_COMMITS**: How many of the PR's most recent commits the commit overview reads (default: `15`; `0` shows the commit count only). The overview tells the model how many commits are already on the PR, who made them, and each author's added/removed line totals. Each summarized commit costs one extra GitHub API call, but only a handful of numbers enter the prompt, so this cap can stay generous. +- **MAX_COMMIT_MESSAGES**: How many commit messages are fully quoted in the prompt (default: `5`). Fully quoted messages are the token-expensive part of the commit history, hence the separate, smaller cap — the overview (above) still covers many more commits. +- **INCLUDE_COMMIT_SUMMARY**: Include the "There are X commits already on this PR" overview with per-author counts and line totals (default: `true`) - **STRUCTURED_OUTPUT**: Enforce a JSON Schema on the model's output via OpenRouter structured outputs (default: `true`) - Makes the provider emit valid, correctly-escaped JSON instead of the model hand-writing it — the main cause of "Invalid JSON response from AI model" - Requires a model/provider that supports `response_format` json_schema (most modern models do; e.g. Kimi K2, MiniMax M2.5) @@ -187,7 +191,7 @@ If you get a "Diff is too large" error: The workflow fetches and sends these repository elements to the AI: 1. **Code Changes**: Full diff of modified files 2. **PR Description**: Title and description text from the pull request -3. **Commit Messages**: Up to 15 most recent commit messages (excluding merges) +3. **Commit Messages**: Up to `MAX_COMMIT_MESSAGES` most recent commit messages (default 5, excluding merges), plus a compact overview stating how many commits are already on the PR, the per-author commit counts, and each author's added/removed line totals (covering up to `MAX_SUMMARY_COMMITS` most recent commits, default 15) 4. **Human Comments**: Comments from human reviewers on the PR; bot comments are excluded, while human comments quoting a review header or marker are retained 5. **Labels**: All repository labels with descriptions and colors 6. **Previous AI Review**: Most recent bot-authored AI review comment only (limited to 10k chars), identified by its review header or `` marker diff --git a/ai-reviewer.sh b/ai-reviewer.sh index 98c1062..523f15e 100644 --- a/ai-reviewer.sh +++ b/ai-reviewer.sh @@ -48,6 +48,30 @@ INCLUDE_LABELS="${INCLUDE_LABELS:-true}" INCLUDE_PR_DESCRIPTION="${INCLUDE_PR_DESCRIPTION:-true}" INCLUDE_COMMIT_MESSAGES="${INCLUDE_COMMIT_MESSAGES:-true}" +# Commit-history context is split by cost: the overview statistic (how many +# commits are on the PR, who made them, added/removed line totals) is cheap in +# tokens and read for many commits, while the fully quoted commit messages are +# token-heavy and therefore capped separately. +# - MAX_SUMMARY_COMMITS: how many past commits the overview statistic reads +# (per-commit line stats cost one GitHub API call each). 0 keeps the count/ +# author line only. +# - MAX_COMMIT_MESSAGES: how many commit messages are fully quoted in the +# prompt. 0 lists no messages. +# Non-numeric values fall back to the defaults. +MAX_SUMMARY_COMMITS="${MAX_SUMMARY_COMMITS:-15}" +MAX_COMMIT_MESSAGES="${MAX_COMMIT_MESSAGES:-5}" +if ! [[ "$MAX_SUMMARY_COMMITS" =~ ^[0-9]+$ ]]; then + MAX_SUMMARY_COMMITS=15 +fi +if ! [[ "$MAX_COMMIT_MESSAGES" =~ ^[0-9]+$ ]]; then + MAX_COMMIT_MESSAGES=5 +fi + +# Include a short "X commits already on this PR" overview in the prompt, with +# per-author commit counts and added/deleted line totals (see +# MAX_SUMMARY_COMMITS for how many commits those cover). +INCLUDE_COMMIT_SUMMARY="${INCLUDE_COMMIT_SUMMARY:-true}" + # Read diff content from stdin DIFF_CONTENT=$(cat) @@ -151,18 +175,102 @@ if [ "$INCLUDE_PR_DESCRIPTION" = "true" ] && [ -n "$PR_NUMBER" ] && [ -n "$REPO_ fi fi -# Fetch commit messages (limit to 15 most recent, exclude merges) -COMMIT_MESSAGES="" -if [ "$INCLUDE_COMMIT_MESSAGES" = "true" ] && [ -n "$PR_NUMBER" ] && [ -n "$REPO_FULL_NAME" ] && [ -n "$GITHUB_TOKEN" ]; then +# Fetch the PR's commit list once, shared by the commit-message list and the +# commit summary. --paginate emits one JSON array per page back to back, so +# slurp with jq -s and 'add' to merge the pages into a single array (this also +# makes the "most recent N" truncation global instead of per-page). +COMMITS_JSON="[]" +if { [ "$INCLUDE_COMMIT_MESSAGES" = "true" ] || [ "$INCLUDE_COMMIT_SUMMARY" = "true" ]; } && [ -n "$PR_NUMBER" ] && [ -n "$REPO_FULL_NAME" ] && [ -n "$GITHUB_TOKEN" ]; then if [ "$DEBUG_MODE" = "true" ]; then - echo "🔍 Fetching commit messages..." >&2 + echo "🔍 Fetching PR commits..." >&2 fi - COMMIT_MESSAGES=$(gh api "repos/$REPO_FULL_NAME/pulls/$PR_NUMBER/commits" --paginate \ - --jq '[.[] | select(.commit.message | startswith("Merge") | not)] | .[-15:] | .[] | "- " + (.commit.message | split("\n")[0]) + (if (.commit.message | split("\n\n")[1]) then "\n " + (.commit.message | split("\n\n")[1]) else "" end)' 2>/dev/null | head -c 2500 || echo "") + COMMITS_JSON=$(gh api "repos/$REPO_FULL_NAME/pulls/$PR_NUMBER/commits" --paginate 2>/dev/null | jq -s 'add // []' || echo "[]") + + if [ "$DEBUG_MODE" = "true" ]; then + echo "✅ Fetched $(echo "$COMMITS_JSON" | jq 'length') commit(s) from the PR" >&2 + fi +fi + +# Format the commit-message list from the cached commit JSON (limit to the +# MAX_COMMIT_MESSAGES most recent, exclude merges). Fully quoted messages are +# the token-expensive part of the history context, hence the separate, smaller +# cap compared to the overview statistic. +COMMIT_MESSAGES="" +if [ "$INCLUDE_COMMIT_MESSAGES" = "true" ] && [ "$COMMITS_JSON" != "[]" ] && [ "$COMMITS_JSON" != "" ]; then + COMMIT_MESSAGES=$(echo "$COMMITS_JSON" | jq -r --argjson n "$MAX_COMMIT_MESSAGES" \ + '[.[] | select(.commit.message | startswith("Merge") | not)] + | if $n > 0 then .[-$n:] else [] end + | .[] | "- " + (.commit.message | split("\n")[0]) + (if (.commit.message | split("\n\n")[1]) then "\n " + (.commit.message | split("\n\n")[1]) else "" end)' 2>/dev/null | head -c 2500 || echo "") if [ "$DEBUG_MODE" = "true" ] && [ -n "$COMMIT_MESSAGES" ]; then COMMIT_COUNT=$(echo "$COMMIT_MESSAGES" | grep -c "^- " || echo "0") - echo "✅ Successfully fetched $COMMIT_COUNT commit messages" >&2 + echo "✅ Kept $COMMIT_COUNT commit message(s) (limit $MAX_COMMIT_MESSAGES)" >&2 + fi +fi + +# Build the commit overview: how many commits are already on the PR, who made +# them, and how many lines each author added/removed. The total comes from the +# cached list; per-commit line stats are NOT part of that list response, so +# each summarized commit costs one extra API call. MAX_SUMMARY_COMMITS bounds +# that cost (0 skips the per-commit calls entirely). The overview is cheap in +# tokens — a handful of numbers — so it may cover many more commits than the +# fully quoted message list (MAX_COMMIT_MESSAGES). +COMMIT_SUMMARY="" +if [ "$INCLUDE_COMMIT_SUMMARY" = "true" ] && [ -n "$COMMITS_JSON" ] && [ "$COMMITS_JSON" != "[]" ]; then + NONMERGE_COUNT=$(echo "$COMMITS_JSON" | jq '[.[] | select(.commit.message | startswith("Merge") | not)] | length') + MERGE_COUNT=$(echo "$COMMITS_JSON" | jq 'length' ) + MERGE_COUNT=$(( MERGE_COUNT - NONMERGE_COUNT )) + + AUTHOR_LINES="" + if [ "$MAX_SUMMARY_COMMITS" -gt 0 ] && [ "$NONMERGE_COUNT" -gt 0 ]; then + STATS_FILE=$(mktemp) || { echo "Failed to create temporary file for commit stats"; exit 1; } + chmod 600 "$STATS_FILE" + # One "authoradditionsdeletions" row per listed commit; + # a failed stats fetch counts as zero rather than aborting the review. + while IFS=$'\t' read -r author sha; do + [ -n "$sha" ] || continue + line_stats=$(gh api "repos/$REPO_FULL_NAME/commits/$sha" \ + --jq '"\(.stats.additions // 0)\t\(.stats.deletions // 0)"' 2>/dev/null || printf '0\t0') + printf '%s\t%s\n' "$author" "$line_stats" >> "$STATS_FILE" + done < <(echo "$COMMITS_JSON" | jq -r --argjson n "$MAX_SUMMARY_COMMITS" \ + '[.[] | select(.commit.message | startswith("Merge") | not)] + | if $n > 0 then .[-$n:] else [] end + | .[] | [(.author.login // .commit.author.name), .sha] | @tsv') + # Aggregate per author; sort by added lines, then removed, then name. + AUTHOR_LINES=$(awk -F'\t' '{ count[$1]++; add[$1] += $2; del[$1] += $3 } + END { for (who in count) printf "%s\t%d\t%d\t%d\n", who, count[who], add[who], del[who] }' "$STATS_FILE" \ + | LC_ALL=C sort -t$'\t' -k3,3nr -k4,4nr -k1,1) + rm -f "$STATS_FILE" + fi + + if [ "$NONMERGE_COUNT" -gt 0 ]; then + if [ "$NONMERGE_COUNT" -eq 1 ]; then + SUMMARY_HEADER="There is 1 commit already on this PR" + COMMIT_WORD="commit" + else + SUMMARY_HEADER="There are $NONMERGE_COUNT commits already on this PR" + COMMIT_WORD="commits" + fi + [ "$MERGE_COUNT" -gt 0 ] && SUMMARY_HEADER="$SUMMARY_HEADER (excluding $MERGE_COUNT merge commit(s))" + if [ -n "$AUTHOR_LINES" ]; then + LISTED=$(( MAX_SUMMARY_COMMITS < NONMERGE_COUNT ? MAX_SUMMARY_COMMITS : NONMERGE_COUNT )) + [ "$LISTED" -eq "$NONMERGE_COUNT" ] \ + && SCOPE="across all $NONMERGE_COUNT $COMMIT_WORD" \ + || SCOPE="across the $LISTED most recent of $NONMERGE_COUNT $COMMIT_WORD" + SUMMARY_BULLETS=$(printf '%s\n' "$AUTHOR_LINES" | awk -F'\t' \ + '{ word = ($2 == 1 ? "commit" : "commits") + print sprintf("- **%s**: %s %s, +%s/-%s lines", $1, $2, word, $3, $4) }') + COMMIT_SUMMARY="Commit Summary: +$SUMMARY_HEADER. Per-author commit counts and line totals $SCOPE: +$SUMMARY_BULLETS" + else + COMMIT_SUMMARY="Commit Summary: +$SUMMARY_HEADER." + fi + + if [ "$DEBUG_MODE" = "true" ]; then + echo "✅ Commit summary: $SUMMARY_HEADER" >&2 + fi fi fi @@ -213,6 +321,16 @@ $PR_DESCRIPTION " fi +# Add commit summary if available +if [ -n "$COMMIT_SUMMARY" ]; then + PROMPT_PREFIX="${PROMPT_PREFIX} +$COMMIT_SUMMARY + +Use the commit summary to gauge the PR's size and authorship; it records what already changed, not what should change. + +" +fi + # Add commit messages if available if [ -n "$COMMIT_MESSAGES" ]; then PROMPT_PREFIX="${PROMPT_PREFIX} diff --git a/tests/test_reviewer_context.py b/tests/test_reviewer_context.py index 2b47e81..9100df2 100644 --- a/tests/test_reviewer_context.py +++ b/tests/test_reviewer_context.py @@ -35,13 +35,33 @@ def comment(body, login="reviewer[bot]", user_type="Bot"): } +def pull_commit(sha, message, login=None, name=None): + return { + "sha": sha, + "author": {"login": login} if login is not None else None, + "commit": {"author": {"name": name or login or "unknown"}, "message": message}, + } + + class ReviewerRequestTests(unittest.TestCase): def run_reviewer( - self, comments=(), *, previous=True, human=True, response=None, config=None + self, + comments=(), + *, + previous=True, + human=True, + response=None, + config=None, + pull_commits=None, + commit_stats=None, ): with tempfile.TemporaryDirectory() as directory: path = Path(directory) (path / "comments.json").write_text(json.dumps(comments)) + (path / "pull-commits.json").write_text( + json.dumps(pull_commits if pull_commits is not None else []) + ) + (path / "commit-stats.json").write_text(json.dumps(commit_stats or {})) expected = response if response is not None else CLEAN_REVIEW (path / "response.json").write_text(json.dumps({ "choices": [{ @@ -53,14 +73,28 @@ def run_reviewer( "gh": '''import json, os, subprocess, sys from pathlib import Path args = sys.argv[1:] -assert args[:2] == ["api", "repos/example/repo/issues/123/comments"], args -assert args[2] == "--jq" and len(args) == 4, args +assert args[0] == "api", args path = Path(os.environ["FIXTURE_DIR"]) with (path / "gh-calls.jsonl").open("a") as calls: calls.write(json.dumps(args) + "\\n") -result = subprocess.run(["jq", "-r", args[3]], - input=(path / "comments.json").read_text(), text=True) -sys.exit(result.returncode) +parts = args[1].split("?")[0].split("/")[3:] +if parts == ["issues", "123", "comments"]: + assert args[2] == "--jq" and len(args) == 4, args + result = subprocess.run(["jq", "-r", args[3]], + input=(path / "comments.json").read_text(), text=True) + sys.exit(result.returncode) +if parts == ["pulls", "123", "commits"]: + assert args[2:] == ["--paginate"], args + sys.stdout.write((path / "pull-commits.json").read_text()) + sys.exit(0) +if len(parts) == 2 and parts[0] == "commits": + assert args[2] == "--jq" and len(args) == 4, args + stats = json.loads((path / "commit-stats.json").read_text()) + assert parts[1] in stats, parts[1] + result = subprocess.run(["jq", "-r", args[3]], + input=json.dumps(stats[parts[1]]), text=True) + sys.exit(result.returncode) +sys.exit(f"unexpected gh api call: {args}") ''', "curl": '''import os, sys from pathlib import Path @@ -89,6 +123,7 @@ def run_reviewer( "INCLUDE_LABELS": "false", "INCLUDE_PR_DESCRIPTION": "false", "INCLUDE_COMMIT_MESSAGES": "false", + "INCLUDE_COMMIT_SUMMARY": "false", } environment.update(config or {}) result = subprocess.run( @@ -100,9 +135,19 @@ def run_reviewer( self.assertEqual(json.loads(result.stdout), expected) calls_file = path / "gh-calls.jsonl" calls = calls_file.read_text().splitlines() if calls_file.exists() else [] - self.assertEqual(len(calls), int(previous) + int(human)) + self.gh_calls = [json.loads(call) for call in calls] + self.assertEqual( + len(self.gh_calls), int(previous) + int(human) + len(self.commit_calls()) + ) return json.loads((path / "request.json").read_text()) + def commit_calls(self): + """GitHub API calls made for the commit history features.""" + return [ + call for call in getattr(self, "gh_calls", []) + if "pulls/123/commits" in call[1] or "/commits/" in call[1] + ] + def test_concise_instructions_preserve_review_depth_and_protocol(self): request = self.run_reviewer(previous=False, human=False) prompt = request["messages"][0]["content"] @@ -214,6 +259,113 @@ def test_actionable_output_and_custom_budget_are_preserved(self): self.assertEqual(request["max_tokens"], 12345) self.assertNotIn("response_format", request) + def commit_fixture(self): + commits = [ + pull_commit("a1", "feat: first", login="alice"), + pull_commit("a2", "feat: second", login="alice"), + pull_commit("a3", "fix: bob fix", name="Bob B"), + pull_commit("a4", "Merge branch 'x' into main", login="alice"), + pull_commit("a5", "feat: third", login="carol"), + ] + stats = { + "a1": {"stats": {"additions": 10, "deletions": 2}}, + "a2": {"stats": {"additions": 20, "deletions": 3}}, + "a3": {"stats": {"additions": 5, "deletions": 8}}, + "a5": {"stats": {"additions": 1, "deletions": 1}}, + } + return commits, stats + + def test_commit_summary_counts_authors_and_lines(self): + commits, stats = self.commit_fixture() + request = self.run_reviewer( + previous=False, human=False, + pull_commits=commits, commit_stats=stats, + config={"INCLUDE_COMMIT_SUMMARY": "true"}, + ) + prompt = request["messages"][0]["content"] + self.assertIn( + "There are 4 commits already on this PR (excluding 1 merge commit(s))", + prompt, + ) + self.assertIn("across all 4 commits", prompt) + self.assertIn("- **alice**: 2 commits, +30/-5 lines", prompt) + self.assertIn("- **Bob B**: 1 commit, +5/-8 lines", prompt) + self.assertIn("- **carol**: 1 commit, +1/-1 lines", prompt) + # Bullet order follows added lines, so the biggest author leads. + self.assertLess( + prompt.index("- **alice**"), prompt.index("- **carol**"), + ) + # Line stats are fetched per listed non-merge commit only. + stats_urls = [call[1] for call in self.commit_calls() if "/pulls/" not in call[1]] + self.assertEqual(len(stats_urls), 4) + self.assertFalse(any("a4" in url for url in stats_urls), stats_urls) + # Only the summary block is present when messages are disabled. + self.assertNotIn("Commit History (showing development journey):", prompt) + + def test_commit_summary_limit_is_independent_of_message_limit(self): + commits, stats = self.commit_fixture() + request = self.run_reviewer( + previous=False, human=False, + pull_commits=commits, commit_stats=stats, + config={ + "INCLUDE_COMMIT_SUMMARY": "true", + "INCLUDE_COMMIT_MESSAGES": "true", + "MAX_SUMMARY_COMMITS": "1", + }, + ) + prompt = request["messages"][0]["content"] + self.assertIn("across the 1 most recent of 4 commits", prompt) + self.assertIn("- **carol**: 1 commit, +1/-1 lines", prompt) + self.assertNotIn("- **alice**:", prompt) + # The message list keeps its own default cap and still shows the past. + self.assertIn("- feat: first", prompt) + stats_urls = [call[1] for call in self.commit_calls() if "/pulls/" not in call[1]] + self.assertEqual(len(stats_urls), 1) + + def test_commit_message_limit_is_configurable(self): + commits, stats = self.commit_fixture() + request = self.run_reviewer( + previous=False, human=False, + pull_commits=commits, commit_stats=stats, + config={"INCLUDE_COMMIT_MESSAGES": "true", "MAX_COMMIT_MESSAGES": "2"}, + ) + prompt = request["messages"][0]["content"] + self.assertIn("Commit History (showing development journey):", prompt) + self.assertIn("- feat: third", prompt) + self.assertIn("- fix: bob fix", prompt) + self.assertNotIn("- feat: first", prompt) + self.assertNotIn("- feat: second", prompt) + self.assertNotIn("Commit Summary:", prompt) + # No per-commit stats calls when the summary is disabled. + stats_urls = [call[1] for call in self.commit_calls() if "/pulls/" not in call[1]] + self.assertEqual(stats_urls, []) + + def test_commit_summary_zero_reads_no_individual_commits(self): + commits, stats = self.commit_fixture() + request = self.run_reviewer( + previous=False, human=False, + pull_commits=commits, commit_stats=stats, + config={"INCLUDE_COMMIT_SUMMARY": "true", "MAX_SUMMARY_COMMITS": "0"}, + ) + prompt = request["messages"][0]["content"] + self.assertIn("There are 4 commits already on this PR", prompt) + self.assertNotIn("Per-author", prompt) + # One list call only: zero individual commit reads. + urls = [call[1] for call in self.commit_calls()] + self.assertEqual(len(urls), 1) + self.assertIn("pulls/123/commits", urls[0]) + + def test_commit_summary_singular_count_and_fallback_author(self): + commits = [pull_commit("solo", "fix: solo commit", name="Dana D")] + request = self.run_reviewer( + previous=False, human=False, + pull_commits=commits, commit_stats={"solo": {"stats": {"additions": 4, "deletions": 1}}}, + config={"INCLUDE_COMMIT_SUMMARY": "true"}, + ) + prompt = request["messages"][0]["content"] + self.assertIn("There is 1 commit already on this PR", prompt) + self.assertIn("- **Dana D**: 1 commit, +4/-1 lines", prompt) + if __name__ == "__main__": unittest.main() From d4536b09f4476172cb64c810e5fa2314229d188c Mon Sep 17 00:00:00 2001 From: LearningCircuit <185559241+LearningCircuit@users.noreply.github.com> Date: Mon, 14 Sep 2026 08:47:55 +0200 Subject: [PATCH 02/17] feat: default MAX_COMMIT_MESSAGES to 3 Three quoted messages are enough context for how the PR evolved while the overview carries the broader picture; the default stays configurable. --- README.md | 4 ++-- ai-reviewer.sh | 4 ++-- tests/test_reviewer_context.py | 6 ++++-- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 51b7236..378019c 100644 --- a/README.md +++ b/README.md @@ -74,7 +74,7 @@ The workflow is pre-configured with sensible defaults, but you can customize it - Adjust lower for cost savings on smaller PRs - **MAX_DIFF_SIZE**: Maximum diff size in bytes (default: `800000` / 800KB) - **MAX_SUMMARY_COMMITS**: How many of the PR's most recent commits the commit overview reads (default: `15`; `0` shows the commit count only). The overview tells the model how many commits are already on the PR, who made them, and each author's added/removed line totals. Each summarized commit costs one extra GitHub API call, but only a handful of numbers enter the prompt, so this cap can stay generous. -- **MAX_COMMIT_MESSAGES**: How many commit messages are fully quoted in the prompt (default: `5`). Fully quoted messages are the token-expensive part of the commit history, hence the separate, smaller cap — the overview (above) still covers many more commits. +- **MAX_COMMIT_MESSAGES**: How many commit messages are fully quoted in the prompt (default: `3`). Fully quoted messages are the token-expensive part of the commit history, hence the separate, smaller cap — the overview (above) still covers many more commits. - **INCLUDE_COMMIT_SUMMARY**: Include the "There are X commits already on this PR" overview with per-author counts and line totals (default: `true`) - **STRUCTURED_OUTPUT**: Enforce a JSON Schema on the model's output via OpenRouter structured outputs (default: `true`) - Makes the provider emit valid, correctly-escaped JSON instead of the model hand-writing it — the main cause of "Invalid JSON response from AI model" @@ -191,7 +191,7 @@ If you get a "Diff is too large" error: The workflow fetches and sends these repository elements to the AI: 1. **Code Changes**: Full diff of modified files 2. **PR Description**: Title and description text from the pull request -3. **Commit Messages**: Up to `MAX_COMMIT_MESSAGES` most recent commit messages (default 5, excluding merges), plus a compact overview stating how many commits are already on the PR, the per-author commit counts, and each author's added/removed line totals (covering up to `MAX_SUMMARY_COMMITS` most recent commits, default 15) +3. **Commit Messages**: Up to `MAX_COMMIT_MESSAGES` most recent commit messages (default 3, excluding merges), plus a compact overview stating how many commits are already on the PR, the per-author commit counts, and each author's added/removed line totals (covering up to `MAX_SUMMARY_COMMITS` most recent commits, default 15) 4. **Human Comments**: Comments from human reviewers on the PR; bot comments are excluded, while human comments quoting a review header or marker are retained 5. **Labels**: All repository labels with descriptions and colors 6. **Previous AI Review**: Most recent bot-authored AI review comment only (limited to 10k chars), identified by its review header or `` marker diff --git a/ai-reviewer.sh b/ai-reviewer.sh index 523f15e..5e13c01 100644 --- a/ai-reviewer.sh +++ b/ai-reviewer.sh @@ -59,12 +59,12 @@ INCLUDE_COMMIT_MESSAGES="${INCLUDE_COMMIT_MESSAGES:-true}" # prompt. 0 lists no messages. # Non-numeric values fall back to the defaults. MAX_SUMMARY_COMMITS="${MAX_SUMMARY_COMMITS:-15}" -MAX_COMMIT_MESSAGES="${MAX_COMMIT_MESSAGES:-5}" +MAX_COMMIT_MESSAGES="${MAX_COMMIT_MESSAGES:-3}" if ! [[ "$MAX_SUMMARY_COMMITS" =~ ^[0-9]+$ ]]; then MAX_SUMMARY_COMMITS=15 fi if ! [[ "$MAX_COMMIT_MESSAGES" =~ ^[0-9]+$ ]]; then - MAX_COMMIT_MESSAGES=5 + MAX_COMMIT_MESSAGES=3 fi # Include a short "X commits already on this PR" overview in the prompt, with diff --git a/tests/test_reviewer_context.py b/tests/test_reviewer_context.py index 9100df2..78e3e30 100644 --- a/tests/test_reviewer_context.py +++ b/tests/test_reviewer_context.py @@ -317,8 +317,10 @@ def test_commit_summary_limit_is_independent_of_message_limit(self): self.assertIn("across the 1 most recent of 4 commits", prompt) self.assertIn("- **carol**: 1 commit, +1/-1 lines", prompt) self.assertNotIn("- **alice**:", prompt) - # The message list keeps its own default cap and still shows the past. - self.assertIn("- feat: first", prompt) + # The message list keeps its own default cap (3) and still shows + # recent history beyond the overview's single-commit scope. + self.assertIn("- feat: second", prompt) + self.assertNotIn("- feat: first", prompt) stats_urls = [call[1] for call in self.commit_calls() if "/pulls/" not in call[1]] self.assertEqual(len(stats_urls), 1) From d15cb3f544988d6145214e3b9b71105361f14e51 Mon Sep 17 00:00:00 2001 From: LearningCircuit <185559241+LearningCircuit@users.noreply.github.com> Date: Mon, 14 Sep 2026 09:07:52 +0200 Subject: [PATCH 03/17] feat: check-run summary, configurable human-comment caps, label guidance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three context-cost adjustments for large PRs: - Check runs: successful checks collapse into a one-line count ("N of M checks passed"); only non-passing runs (failure, skipped, cancelled, timed out, still running) are listed individually, so green matrix shards no longer flood the prompt. Skipped runs stay visible — they can matter. - Human comments: kept generous but fully configurable via MAX_HUMAN_COMMENTS (default 100), MAX_HUMAN_COMMENT_LENGTH (4000), and MAX_HUMAN_COMMENTS_TOTAL (20000). The newest comments are presented newest-first, so clipping drops the oldest of the selected — the latest feedback always survives — and every clip is marked as truncated so the model knows context was cut. - Labels: the list itself is unchanged and complete (it is important); the prompt now instructs the model to only apply labels that are genuinely useful and to add none when unsure. The full diff is never shortened. --- README.md | 9 ++- ai-reviewer.sh | 84 ++++++++++++++++--- tests/test_reviewer_context.py | 142 +++++++++++++++++++++++++++++++-- 3 files changed, 216 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 378019c..33159ca 100644 --- a/README.md +++ b/README.md @@ -76,6 +76,9 @@ The workflow is pre-configured with sensible defaults, but you can customize it - **MAX_SUMMARY_COMMITS**: How many of the PR's most recent commits the commit overview reads (default: `15`; `0` shows the commit count only). The overview tells the model how many commits are already on the PR, who made them, and each author's added/removed line totals. Each summarized commit costs one extra GitHub API call, but only a handful of numbers enter the prompt, so this cap can stay generous. - **MAX_COMMIT_MESSAGES**: How many commit messages are fully quoted in the prompt (default: `3`). Fully quoted messages are the token-expensive part of the commit history, hence the separate, smaller cap — the overview (above) still covers many more commits. - **INCLUDE_COMMIT_SUMMARY**: Include the "There are X commits already on this PR" overview with per-author counts and line totals (default: `true`) +- **MAX_HUMAN_COMMENTS**: How many of the newest human comments are included (default: `100`). Comments are presented newest-first, so when this or the overall budget clips, the oldest go first — the latest feedback always survives. +- **MAX_HUMAN_COMMENT_LENGTH**: Maximum characters per human comment; longer comments are clipped and marked " […truncated]" (default: `4000`) +- **MAX_HUMAN_COMMENTS_TOTAL**: Overall character budget for the human-comments block; when reached, the block is cut and marked (default: `20000`) - **STRUCTURED_OUTPUT**: Enforce a JSON Schema on the model's output via OpenRouter structured outputs (default: `true`) - Makes the provider emit valid, correctly-escaped JSON instead of the model hand-writing it — the main cause of "Invalid JSON response from AI model" - Requires a model/provider that supports `response_format` json_schema (most modern models do; e.g. Kimi K2, MiniMax M2.5) @@ -192,10 +195,10 @@ The workflow fetches and sends these repository elements to the AI: 1. **Code Changes**: Full diff of modified files 2. **PR Description**: Title and description text from the pull request 3. **Commit Messages**: Up to `MAX_COMMIT_MESSAGES` most recent commit messages (default 3, excluding merges), plus a compact overview stating how many commits are already on the PR, the per-author commit counts, and each author's added/removed line totals (covering up to `MAX_SUMMARY_COMMITS` most recent commits, default 15) -4. **Human Comments**: Comments from human reviewers on the PR; bot comments are excluded, while human comments quoting a review header or marker are retained -5. **Labels**: All repository labels with descriptions and colors +4. **Human Comments**: Comments from human reviewers on the PR, newest first; bot comments are excluded, while human comments quoting a review header or marker are retained. Caps (`MAX_HUMAN_COMMENTS`, `MAX_HUMAN_COMMENT_LENGTH`, `MAX_HUMAN_COMMENTS_TOTAL`) clip the oldest first and mark any truncation. +5. **Labels**: All repository labels with descriptions and colors (kept complete on purpose; the prompt instructs the model to only apply genuinely useful ones) 6. **Previous AI Review**: Most recent bot-authored AI review comment only (limited to 10k chars), identified by its review header or `` marker -7. **CI/CD Status**: GitHub Actions check runs and build statuses +7. **CI/CD Status**: A one-line summary of GitHub Actions check runs ("N of M checks passed") plus only the non-passing runs — failures, skipped, cancelled, timed out, or still running — listed individually; fully green matrix shards no longer flood the prompt 8. **PR Metadata**: Pull request details, head SHA, repository information 9. **Files**: May include sensitive configuration files, keys, or credentials diff --git a/ai-reviewer.sh b/ai-reviewer.sh index 5e13c01..8e76a14 100644 --- a/ai-reviewer.sh +++ b/ai-reviewer.sh @@ -72,6 +72,24 @@ fi # MAX_SUMMARY_COMMITS for how many commits those cover). INCLUDE_COMMIT_SUMMARY="${INCLUDE_COMMIT_SUMMARY:-true}" +# Human comments are high-value context, so these caps are deliberately +# generous and each is configurable: how many of the newest comments are +# kept, how long each may be, and the overall character budget for the +# block. When a cap clips, the oldest of the selected comments go first and +# the clip is marked so the model knows context was cut. +MAX_HUMAN_COMMENTS="${MAX_HUMAN_COMMENTS:-100}" +MAX_HUMAN_COMMENT_LENGTH="${MAX_HUMAN_COMMENT_LENGTH:-4000}" +MAX_HUMAN_COMMENTS_TOTAL="${MAX_HUMAN_COMMENTS_TOTAL:-20000}" +if ! [[ "$MAX_HUMAN_COMMENTS" =~ ^[0-9]+$ ]]; then + MAX_HUMAN_COMMENTS=100 +fi +if ! [[ "$MAX_HUMAN_COMMENT_LENGTH" =~ ^[0-9]+$ ]]; then + MAX_HUMAN_COMMENT_LENGTH=4000 +fi +if ! [[ "$MAX_HUMAN_COMMENTS_TOTAL" =~ ^[0-9]+$ ]]; then + MAX_HUMAN_COMMENTS_TOTAL=20000 +fi + # Read diff content from stdin DIFF_CONTENT=$(cat) @@ -120,24 +138,68 @@ if [ "$INCLUDE_PREVIOUS_REVIEWS" = "true" ] && [ -n "$PR_NUMBER" ] && [ -n "$REP --jq "$COMMENT_CLASSIFIERS"'[.[] | select(is_ai_review)] | last | if . then "### Previous AI Review (" + .created_at + "):\n" + .body + "\n---\n" else "" end' 2>/dev/null | head -c 10000 || echo "") fi -# Fetch human comments for context +# Fetch human comments for context. Human comments are valuable, so the +# defaults are generous and configurable (see MAX_HUMAN_COMMENTS et al.): +# the newest MAX_HUMAN_COMMENTS comments are kept, presented newest-first so +# an overall-budget clip drops the oldest of the selected — never the latest +# feedback. Per-comment and overall clipping are marked as truncated. +# Exclude all bot comments; previous AI reviews have their own context block. HUMAN_COMMENTS="" -if [ "$INCLUDE_HUMAN_COMMENTS" = "true" ] && [ -n "$PR_NUMBER" ] && [ -n "$REPO_FULL_NAME" ] && [ -n "$GITHUB_TOKEN" ]; then - # Exclude all bot comments; previous AI reviews have their own context block. - HUMAN_COMMENTS=$(gh api "repos/$REPO_FULL_NAME/issues/$PR_NUMBER/comments" \ - --jq "$COMMENT_CLASSIFIERS"'[.[] | select(is_bot | not)] | map("**" + .user.login + "** (" + .created_at + "):\n" + .body) | join("\n\n---\n\n")' 2>/dev/null | head -c 20000 || echo "") +if [ "$INCLUDE_HUMAN_COMMENTS" = "true" ] && [ -n "$PR_NUMBER" ] && [ -n "$REPO_FULL_NAME" ] && [ -n "$GITHUB_TOKEN" ] && [ "$MAX_HUMAN_COMMENTS" -gt 0 ] && [ "$MAX_HUMAN_COMMENTS_TOTAL" -gt 0 ]; then + # A count cap of 0 selects nothing; jq's .[-0:] would mean "everything", + # so build the slice expression explicitly. + if [ "$MAX_HUMAN_COMMENTS" -gt 0 ]; then + COMMENT_SLICE=".[-$MAX_HUMAN_COMMENTS:]" + else + COMMENT_SLICE="[]" + fi + HUMAN_COMMENTS_FULL=$(gh api "repos/$REPO_FULL_NAME/issues/$PR_NUMBER/comments" \ + --jq "$COMMENT_CLASSIFIERS"'[.[] | select(is_bot | not)] + | '"$COMMENT_SLICE"' | reverse + | map("**" + (.user.login // "unknown") + "** (" + .created_at + "):\n" + + (if ((.body // "") | length) > '"$MAX_HUMAN_COMMENT_LENGTH"' + then ((.body // "")[0:'"$MAX_HUMAN_COMMENT_LENGTH"'] + " […truncated]") + else (.body // "") end)) + | join("\n\n---\n\n")' 2>/dev/null || echo "") + HUMAN_COMMENTS=$(printf '%s' "$HUMAN_COMMENTS_FULL" | head -c "$MAX_HUMAN_COMMENTS_TOTAL") + if [ "$(printf '%s' "$HUMAN_COMMENTS" | wc -c)" -eq "$MAX_HUMAN_COMMENTS_TOTAL" ]; then + HUMAN_COMMENTS="$HUMAN_COMMENTS +[…truncated at $MAX_HUMAN_COMMENTS_TOTAL characters]" + fi fi -# Fetch GitHub Actions check runs status (if PR_NUMBER and REPO_FULL_NAME are set) +# Fetch GitHub Actions check runs status (if PR_NUMBER and REPO_FULL_NAME are set). +# Successful checks are collapsed into a one-line count; every non-passing run +# (failure, skipped, cancelled, timed out, still running) is listed +# individually — green matrix shards must not flood the prompt, but skipped +# runs can matter, so they stay visible. CHECK_RUNS_STATUS="" if [ "$INCLUDE_CHECK_RUNS" = "true" ] && [ -n "$PR_NUMBER" ] && [ -n "$REPO_FULL_NAME" ] && [ -n "$GITHUB_TOKEN" ]; then # Get the head SHA of the PR HEAD_SHA=$(gh api "repos/$REPO_FULL_NAME/pulls/$PR_NUMBER" --jq '.head.sha' 2>/dev/null || echo "") if [ -n "$HEAD_SHA" ]; then - # Fetch check runs for this commit - CHECK_RUNS_STATUS=$(gh api "repos/$REPO_FULL_NAME/commits/$HEAD_SHA/check-runs" \ - --jq '.check_runs // [] | .[] | "- **\(.name)**: \(.status)\(if .conclusion then " (\(.conclusion))" else "" end)"' 2>/dev/null || echo "") + CHECK_RUNS_SUMMARY=$(gh api "repos/$REPO_FULL_NAME/commits/$HEAD_SHA/check-runs" \ + --jq '(.check_runs // []) + | {total: length, + passed: [.[] | select(.conclusion == "success")] | length, + other: [.[] | select(.conclusion != "success") + | "- **\(.name)**: \(.status)\(if .conclusion then " (\(.conclusion))" else "" end)"]}' 2>/dev/null || echo "") + + if [ -n "$CHECK_RUNS_SUMMARY" ] && [ "$CHECK_RUNS_SUMMARY" != "null" ]; then + TOTAL_CHECKS=$(echo "$CHECK_RUNS_SUMMARY" | jq -r '.total // 0') + PASSED_CHECKS=$(echo "$CHECK_RUNS_SUMMARY" | jq -r '.passed // 0') + OTHER_CHECKS=$(echo "$CHECK_RUNS_SUMMARY" | jq -r 'if .other then .other | join("\n") else "" end') + + if [ "$TOTAL_CHECKS" -gt 0 ]; then + if [ -n "$OTHER_CHECKS" ]; then + CHECK_RUNS_STATUS="$PASSED_CHECKS of $TOTAL_CHECKS checks passed. Non-passing checks: +$OTHER_CHECKS" + else + CHECK_RUNS_STATUS="All $TOTAL_CHECKS checks passed." + fi + fi + fi fi fi @@ -305,7 +367,7 @@ fi if [ -n "$AVAILABLE_LABELS" ]; then PROMPT_PREFIX="${PROMPT_PREFIX} Available Repository Labels: -Please prefer using existing labels from this list over creating new ones: +Prefer existing labels from this list over creating new ones. Only apply labels that are genuinely useful for these changes — when unsure, add none rather than stretching a label to fit: $AVAILABLE_LABELS If none of these labels are appropriate for the changes, you may suggest new ones. @@ -345,7 +407,7 @@ fi # Add human comments context if available if [ -n "$HUMAN_COMMENTS" ]; then PROMPT_PREFIX="${PROMPT_PREFIX} -Human Comments on this PR: +Human Comments on this PR (newest first): $HUMAN_COMMENTS Please consider these human comments when reviewing the code. diff --git a/tests/test_reviewer_context.py b/tests/test_reviewer_context.py index 78e3e30..00841ea 100644 --- a/tests/test_reviewer_context.py +++ b/tests/test_reviewer_context.py @@ -54,6 +54,8 @@ def run_reviewer( config=None, pull_commits=None, commit_stats=None, + check_runs=None, + labels=None, ): with tempfile.TemporaryDirectory() as directory: path = Path(directory) @@ -62,6 +64,13 @@ def run_reviewer( json.dumps(pull_commits if pull_commits is not None else []) ) (path / "commit-stats.json").write_text(json.dumps(commit_stats or {})) + (path / "pr.json").write_text(json.dumps( + {"number": 123, "head": {"sha": "abc"}} + )) + (path / "check-runs.json").write_text(json.dumps( + {"total_count": len(check_runs or []), "check_runs": check_runs or []} + )) + (path / "labels.json").write_text(json.dumps(labels or [])) expected = response if response is not None else CLEAN_REVIEW (path / "response.json").write_text(json.dumps({ "choices": [{ @@ -87,6 +96,21 @@ def run_reviewer( assert args[2:] == ["--paginate"], args sys.stdout.write((path / "pull-commits.json").read_text()) sys.exit(0) +if parts == ["pulls", "123"]: + assert args[2] == "--jq" and len(args) == 4, args + result = subprocess.run(["jq", "-r", args[3]], + input=(path / "pr.json").read_text(), text=True) + sys.exit(result.returncode) +if parts == ["commits", "abc", "check-runs"]: + assert args[2] == "--jq" and len(args) == 4, args + result = subprocess.run(["jq", "-r", args[3]], + input=(path / "check-runs.json").read_text(), text=True) + sys.exit(result.returncode) +if parts == ["labels"]: + assert args[2:4] == ["--paginate", "--jq"] and len(args) == 5, args + result = subprocess.run(["jq", "-r", args[4]], + input=(path / "labels.json").read_text(), text=True) + sys.exit(result.returncode) if len(parts) == 2 and parts[0] == "commits": assert args[2] == "--jq" and len(args) == 4, args stats = json.loads((path / "commit-stats.json").read_text()) @@ -136,16 +160,42 @@ def run_reviewer( calls_file = path / "gh-calls.jsonl" calls = calls_file.read_text().splitlines() if calls_file.exists() else [] self.gh_calls = [json.loads(call) for call in calls] - self.assertEqual( - len(self.gh_calls), int(previous) + int(human) + len(self.commit_calls()) - ) + # Every call must belong to a known context feature, and the + # comment endpoint is still fetched exactly once per enabled + # comment feature (never per comment). + categorized = (self.comment_calls() + self.commit_calls() + + self.check_calls() + self.label_calls()) + self.assertEqual(len(self.gh_calls), len(categorized)) + self.assertEqual(len(self.comment_calls()), int(previous) + int(human)) return json.loads((path / "request.json").read_text()) + def comment_calls(self): + """GitHub API calls made for comment context.""" + return [ + call for call in getattr(self, "gh_calls", []) + if "/issues/123/comments" in call[1] + ] + def commit_calls(self): """GitHub API calls made for the commit history features.""" return [ call for call in getattr(self, "gh_calls", []) - if "pulls/123/commits" in call[1] or "/commits/" in call[1] + if "pulls/123/commits" in call[1] + or ("/commits/" in call[1] and "check-runs" not in call[1]) + ] + + def check_calls(self): + """GitHub API calls made for the check-runs context.""" + return [ + call for call in getattr(self, "gh_calls", []) + if call[1].endswith("/pulls/123") or call[1].endswith("/check-runs") + ] + + def label_calls(self): + """GitHub API calls made for the label context.""" + return [ + call for call in getattr(self, "gh_calls", []) + if call[1].endswith("/labels") ] def test_concise_instructions_preserve_review_depth_and_protocol(self): @@ -199,7 +249,7 @@ def test_humans_quoting_review_identifiers_remain_human(self): for index, body in enumerate(bodies) ]) prompt = request["messages"][0]["content"] - self.assertIn("Human Comments on this PR:", prompt) + self.assertIn("Human Comments on this PR (newest first):", prompt) self.assertNotIn("Previous AI Review (for context", prompt) for body in bodies: self.assertEqual(prompt.count(body), 1) @@ -368,6 +418,88 @@ def test_commit_summary_singular_count_and_fallback_author(self): self.assertIn("There is 1 commit already on this PR", prompt) self.assertIn("- **Dana D**: 1 commit, +4/-1 lines", prompt) + def test_check_runs_summarize_successes_and_list_non_passing(self): + request = self.run_reviewer( + previous=False, human=False, + check_runs=[ + {"name": "lint", "status": "completed", "conclusion": "success"}, + {"name": "unit-tests (3/8)", "status": "completed", "conclusion": "success"}, + {"name": "e2e-playwright", "status": "completed", "conclusion": "failure"}, + {"name": "ui-shard-2", "status": "completed", "conclusion": "skipped"}, + {"name": "docker-build", "status": "in_progress"}, + ], + config={"INCLUDE_CHECK_RUNS": "true"}, + ) + prompt = request["messages"][0]["content"] + self.assertIn("2 of 5 checks passed. Non-passing checks:", prompt) + self.assertIn("- **e2e-playwright**: completed (failure)", prompt) + self.assertIn("- **ui-shard-2**: completed (skipped)", prompt) + self.assertIn("- **docker-build**: in_progress", prompt) + # Successful runs (including matrix shards) collapse into the count. + self.assertNotIn("- **lint**", prompt) + self.assertNotIn("- **unit-tests", prompt) + self.assertEqual(len(self.check_calls()), 2) + + def test_check_runs_all_passed_collapses_to_one_line(self): + request = self.run_reviewer( + previous=False, human=False, + check_runs=[ + {"name": "lint", "status": "completed", "conclusion": "success"}, + {"name": "unit-tests", "status": "completed", "conclusion": "success"}, + ], + config={"INCLUDE_CHECK_RUNS": "true"}, + ) + prompt = request["messages"][0]["content"] + self.assertIn("All 2 checks passed.", prompt) + self.assertNotIn("Non-passing", prompt) + self.assertNotIn("- **lint**", prompt) + + def test_labels_instruction_and_list_stay_complete(self): + request = self.run_reviewer( + previous=False, human=False, + labels=[ + {"name": "bug", "description": "Something is broken", "color": "d73a4a"}, + {"name": "ui", "description": "Touches the web stack", "color": "0366d6"}, + ], + config={"INCLUDE_LABELS": "true"}, + ) + prompt = request["messages"][0]["content"] + self.assertIn("Only apply labels that are genuinely useful", prompt) + self.assertIn("add none rather than stretching a label to fit", prompt) + # The label list itself stays complete — no trimming of data. + self.assertIn("- **bug**: Something is broken (color: #d73a4a)", prompt) + self.assertIn("- **ui**: Touches the web stack (color: #0366d6)", prompt) + + def test_human_comments_keep_newest_within_count_cap(self): + comments = [ + comment("older feedback that must drop", "alice", "User"), + comment("newest feedback that must survive", "bob", "User"), + ] + request = self.run_reviewer(comments, previous=False, + config={"MAX_HUMAN_COMMENTS": "1"}) + prompt = request["messages"][0]["content"] + self.assertIn("Human Comments on this PR (newest first):", prompt) + self.assertIn("newest feedback that must survive", prompt) + self.assertNotIn("older feedback that must drop", prompt) + + def test_human_comment_length_cap_marks_truncation(self): + body = "x" * 30 + request = self.run_reviewer( + [comment(body, "alice", "User")], previous=False, + config={"MAX_HUMAN_COMMENT_LENGTH": "10"}, + ) + prompt = request["messages"][0]["content"] + self.assertIn("x" * 10 + " […truncated]", prompt) + self.assertNotIn("x" * 11, prompt) + + def test_human_comments_total_budget_marks_truncation(self): + request = self.run_reviewer( + [comment("y" * 60, "alice", "User")], previous=False, + config={"MAX_HUMAN_COMMENTS_TOTAL": "50"}, + ) + prompt = request["messages"][0]["content"] + self.assertIn("[…truncated at 50 characters]", prompt) + if __name__ == "__main__": unittest.main() From 3501fc85bc20a81a9669e4a0abc63a1368f19638 Mon Sep 17 00:00:00 2001 From: LearningCircuit <185559241+LearningCircuit@users.noreply.github.com> Date: Mon, 14 Sep 2026 09:19:46 +0200 Subject: [PATCH 04/17] fix: detect human-comment clipping from the source length Measuring the clipped result's byte count is unreliable: command substitution strips trailing newlines, so a comment ending in blank lines could shrink the result below the budget and hide a real cut. Compare the full block's length against the budget instead, so the marker appears exactly when content was lost (and not on exact fill). Also documents why splicing validated numeric values into the gh api --jq filter is safe (gh api has no --arg passthrough). --- ai-reviewer.sh | 9 ++++++++- tests/test_reviewer_context.py | 21 +++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/ai-reviewer.sh b/ai-reviewer.sh index 8e76a14..4b4a0b5 100644 --- a/ai-reviewer.sh +++ b/ai-reviewer.sh @@ -153,6 +153,9 @@ if [ "$INCLUDE_HUMAN_COMMENTS" = "true" ] && [ -n "$PR_NUMBER" ] && [ -n "$REPO_ else COMMENT_SLICE="[]" fi + # Splicing shell variables into a gh api --jq filter is safe here because + # gh api has no --arg passthrough and every value above is validated as a + # non-negative integer before use. HUMAN_COMMENTS_FULL=$(gh api "repos/$REPO_FULL_NAME/issues/$PR_NUMBER/comments" \ --jq "$COMMENT_CLASSIFIERS"'[.[] | select(is_bot | not)] | '"$COMMENT_SLICE"' | reverse @@ -162,7 +165,11 @@ if [ "$INCLUDE_HUMAN_COMMENTS" = "true" ] && [ -n "$PR_NUMBER" ] && [ -n "$REPO_ else (.body // "") end)) | join("\n\n---\n\n")' 2>/dev/null || echo "") HUMAN_COMMENTS=$(printf '%s' "$HUMAN_COMMENTS_FULL" | head -c "$MAX_HUMAN_COMMENTS_TOTAL") - if [ "$(printf '%s' "$HUMAN_COMMENTS" | wc -c)" -eq "$MAX_HUMAN_COMMENTS_TOTAL" ]; then + # Detect clipping from the source length, not the result's byte count: + # command substitution strips trailing newlines, so a comment ending in + # blank lines could otherwise shrink the clipped result below the budget + # and hide a real cut. + if [ "$(printf '%s' "$HUMAN_COMMENTS_FULL" | wc -c)" -gt "$MAX_HUMAN_COMMENTS_TOTAL" ]; then HUMAN_COMMENTS="$HUMAN_COMMENTS […truncated at $MAX_HUMAN_COMMENTS_TOTAL characters]" fi diff --git a/tests/test_reviewer_context.py b/tests/test_reviewer_context.py index 00841ea..97a3861 100644 --- a/tests/test_reviewer_context.py +++ b/tests/test_reviewer_context.py @@ -500,6 +500,27 @@ def test_human_comments_total_budget_marks_truncation(self): prompt = request["messages"][0]["content"] self.assertIn("[…truncated at 50 characters]", prompt) + def test_human_comments_exactly_filling_budget_are_not_marked(self): + # "**alice** (2026-09-12T10:00:00Z):\n" is 34 characters; a 16-char + # body makes the block exactly 50 — no clip, so no marker. + request = self.run_reviewer( + [comment("z" * 16, "alice", "User")], previous=False, + config={"MAX_HUMAN_COMMENTS_TOTAL": "50"}, + ) + prompt = request["messages"][0]["content"] + self.assertIn("z" * 16, prompt) + self.assertNotIn("[…truncated", prompt) + + def test_human_comments_clipped_after_trailing_newlines_still_marked(self): + # The comment body ends in blank lines: command substitution strips + # them from the clipped result, which must not hide the cut. + request = self.run_reviewer( + [comment("w" * 60 + "\n\n\n", "alice", "User")], previous=False, + config={"MAX_HUMAN_COMMENTS_TOTAL": "50"}, + ) + prompt = request["messages"][0]["content"] + self.assertIn("[…truncated at 50 characters]", prompt) + if __name__ == "__main__": unittest.main() From ede8f8466a6bf82ba4e529e5a4d3787bf58970d4 Mon Sep 17 00:00:00 2001 From: LearningCircuit <185559241+LearningCircuit@users.noreply.github.com> Date: Mon, 14 Sep 2026 09:20:46 +0200 Subject: [PATCH 05/17] feat: severity tags, highlighted inferences, and a Should-be-checked section MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The review contract now requires every finding to carry exactly one severity tag — must fix, should fix, or nit — ordered accordingly; inferences to be labeled explicitly as "Inference (not verified):" so assumptions never read as verified facts; and unverifiable but consequential questions to be collected in a final "Should be checked" section instead of being skipped or buried in findings. The section is omitted when there is nothing meaningful to check. --- README.md | 2 +- ai-reviewer.sh | 14 ++++++-------- tests/test_reviewer_context.py | 16 ++++++++++++---- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 33159ca..41cd615 100644 --- a/README.md +++ b/README.md @@ -110,7 +110,7 @@ This will generate a fresh review of the current PR state. ## Review Results -The AI reviews your code across all focus areas and reports actionable findings ordered by severity, with a location, failure scenario, impact, and suggested fix. It omits praise, change summaries, and empty sections. A clean review says "No actionable findings." followed by the verdict. Concise output does not lower the token budget available for reasoning and findings. The review is meant to assist human reviewers, not replace them. +The AI reviews your code across all focus areas and reports actionable findings as bullets tagged **must fix**, **should fix**, or **nit** (in that order), each with a location, failure scenario, impact, and suggested fix. Inferences are highlighted with an explicit "Inference (not verified):" label so they are never mistaken for verified facts, and anything that cannot be verified from the diff but is worth a human look is collected in a final "Should be checked" section. The review omits praise, change summaries, and empty sections; a clean review says "No actionable findings." followed by the verdict. Concise output does not lower the token budget available for reasoning and findings. The review is meant to assist human reviewers, not replace them. ## Cost Estimation diff --git a/ai-reviewer.sh b/ai-reviewer.sh index 4b4a0b5..e73a0fa 100644 --- a/ai-reviewer.sh +++ b/ai-reviewer.sh @@ -357,7 +357,7 @@ PROMPT_PREFIX="Review this code diff thoroughly and report only actionable findi Focus on security, performance, code quality, and best practices. -Keep the review scannable and grouped by importance. Lead with critical issues if any exist. +Keep the review scannable and grouped by severity: must fix first, then should fix, then nits. " # Add GitHub Actions check status if available @@ -443,16 +443,14 @@ PROMPT="You are an expert code reviewer. Analyze this code diff thoroughly and r Focus on security, performance, code quality, and best practices. -Focus on high-value issues. Style suggestions are welcome if impactful, but not minor optimizations. Be concise: omit praise, change summaries, empty sections, and repeated conclusions. For each finding, include its file and line location, concrete failure scenario, impact, and suggested fix. Important: Focus on issues directly visible in the diff. If you cannot verify something from the diff alone (e.g., missing context, unclear defaults, code not shown): -- Default: Skip the issue to avoid spam -- Only ask for clarification if it's critical (security vulnerabilities, breaking bugs, data loss risks): \"Cannot verify [X] from diff - please confirm [specific question]\" -- If making an inference about non-critical issues, explicitly label it: \"Inference (not verified): [observation]\" +Focus on high-value issues. Style suggestions are welcome if impactful, but not minor optimizations. Be concise: omit praise, change summaries, empty sections, and repeated conclusions. For each finding, include its file and line location, concrete failure scenario, impact, and suggested fix. Tag every finding with exactly one severity — \"must fix\" (bugs, security issues, breaking changes that should block merge), \"should fix\" (real problems worth addressing but tolerable to defer), or \"nit\" (minor style or polish) — and order findings must fix first, then should fix, then nits. Never present an assumption as verified fact: label every inference explicitly as \"Inference (not verified): [observation]\" so it stands out from verified findings. If you cannot verify something from the diff alone (e.g., missing context, unclear defaults, code not shown), do not speculate and do not bury the question in a finding; add it to a final \"Should be checked\" section as \"Cannot verify [X] from diff - please confirm [specific question]\", limited to checks that genuinely matter (security vulnerabilities, breaking bugs, data loss risks). Review Structure: 1. Start with the \"## AI Code Review\" header -2. List actionable findings as bullet points, ordered by severity; preserve enough detail to understand and fix each issue -3. If there are no actionable findings, write only \"No actionable findings.\" before the verdict; do not add a summary or empty security section -4. End with one of these verdicts ONLY: +2. List actionable findings as bullet points tagged \"must fix\", \"should fix\", or \"nit\", in that order; preserve enough detail to understand and fix each issue, and highlight inferences with the explicit \"Inference (not verified):\" label +3. If specific things cannot be verified from the diff and are worth a human check, list them in a final \"Should be checked\" section before the verdict; omit the section entirely when there is nothing meaningful to check +4. If there are no actionable findings and nothing to check, write only \"No actionable findings.\" before the verdict; do not add a summary or empty security section +5. End with one of these verdicts ONLY: - \"✅ Approved\" (no issues found) - \"✅ Approved with recommendations\" (minor improvements suggested, but not blocking) - \"❌ Request changes\" (critical issues that must be fixed before merge) diff --git a/tests/test_reviewer_context.py b/tests/test_reviewer_context.py index 97a3861..4ca305b 100644 --- a/tests/test_reviewer_context.py +++ b/tests/test_reviewer_context.py @@ -203,7 +203,12 @@ def test_concise_instructions_preserve_review_depth_and_protocol(self): prompt = request["messages"][0]["content"] self.assertIn("Analyze this code diff thoroughly", prompt) self.assertIn("omit praise, change summaries, empty sections", prompt) - self.assertIn("ordered by severity", prompt) + for tag in ("must fix", "should fix", "nit"): + self.assertIn(f'"{tag}"', prompt) + self.assertIn("order findings must fix first, then should fix, then nits", prompt) + self.assertIn('label every inference explicitly as "Inference (not verified):', prompt) + self.assertIn('add it to a final "Should be checked" section', prompt) + self.assertIn("omit the section entirely when there is nothing meaningful to check", prompt) self.assertIn("file and line location, concrete failure scenario, impact", prompt) self.assertIn('write only "No actionable findings." before the verdict', prompt) self.assertNotIn("Always include a", prompt) @@ -295,9 +300,12 @@ def test_disabling_human_context_keeps_previous_review(self): def test_actionable_output_and_custom_budget_are_preserved(self): findings = { "review": ( - f"{HEADER}\n\n- [High] file.py:12: Passing an empty list raises " - "IndexError, failing the request. Check the list before indexing." - f"\n\n❌ Request changes\n\n{FOOTER}" + f"{HEADER}\n\n- **must fix** — file.py:12: Passing an empty list " + "raises IndexError, failing the request. Check the list before " + "indexing.\n- **nit** — file.py:40: \"Inference (not verified): \" " + "the loop could early-exit.\n\nShould be checked:\n- Cannot verify " + "the migration is reversible from diff - please confirm a " + "downgrade path exists.\n\n❌ Request changes\n\n{FOOTER}" ), "fail_pass_workflow": "fail", "labels_added": ["bug", "tests"], From 2206dcf7e27a35c31ff5212a89c9d878e2ce48c6 Mon Sep 17 00:00:00 2001 From: LearningCircuit <185559241+LearningCircuit@users.noreply.github.com> Date: Mon, 14 Sep 2026 21:18:58 +0200 Subject: [PATCH 06/17] fix: drop the commit-stats temp file; state truncation budget in bytes The per-commit stats rows now pipe straight from the fetch loop into awk/sort, so there is no temp file left behind when a stats fetch goes wrong mid-loop. A bare additional EXIT trap was not an option: this script chains EXIT traps (DIFF_FILE, then DIFF_FILE+PROMPT_FILE), and a new trap silently replaces the previous one. The overall human-comments budget was always enforced by head -c, so its marker and documentation now say bytes; a wc -m check could disagree with the byte cutter (marker missing despite a real cut on multibyte content). --- README.md | 10 +++++----- ai-reviewer.sh | 36 ++++++++++++++++------------------ tests/test_reviewer_context.py | 6 +++--- 3 files changed, 25 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index 41cd615..912901f 100644 --- a/README.md +++ b/README.md @@ -64,7 +64,7 @@ The review is posted as a single concise comment on your pull request with appro The workflow is pre-configured with sensible defaults, but you can customize it by setting repository variables in **Settings** → **Secrets and variables** → **Actions** → **Variables**: -- **AI_MODEL**: Change the AI model (default: `moonshotai/kimi-k2-thinking`) +- **AI_MODEL**: Change the AI model (default: `z-ai/glm-5.3`) - See [OpenRouter models](https://openrouter.ai/models) for options - Recommended: Models with reasoning capabilities (Kimi K2, o1, etc.) - **AI_TEMPERATURE**: Adjust randomness (default: `0.1` for consistent reviews) @@ -78,7 +78,7 @@ The workflow is pre-configured with sensible defaults, but you can customize it - **INCLUDE_COMMIT_SUMMARY**: Include the "There are X commits already on this PR" overview with per-author counts and line totals (default: `true`) - **MAX_HUMAN_COMMENTS**: How many of the newest human comments are included (default: `100`). Comments are presented newest-first, so when this or the overall budget clips, the oldest go first — the latest feedback always survives. - **MAX_HUMAN_COMMENT_LENGTH**: Maximum characters per human comment; longer comments are clipped and marked " […truncated]" (default: `4000`) -- **MAX_HUMAN_COMMENTS_TOTAL**: Overall character budget for the human-comments block; when reached, the block is cut and marked (default: `20000`) +- **MAX_HUMAN_COMMENTS_TOTAL**: Overall byte budget for the human-comments block (`head -c`); when exceeded, the block is cut and marked (default: `20000`) - **STRUCTURED_OUTPUT**: Enforce a JSON Schema on the model's output via OpenRouter structured outputs (default: `true`) - Makes the provider emit valid, correctly-escaped JSON instead of the model hand-writing it — the main cause of "Invalid JSON response from AI model" - Requires a model/provider that supports `response_format` json_schema (most modern models do; e.g. Kimi K2, MiniMax M2.5) @@ -114,7 +114,7 @@ The AI reviews your code across all focus areas and reports actionable findings ## Cost Estimation -Costs with the default Kimi K2 thinking model are very affordable. Based on real usage data: +Costs with the default GLM 5.3 model are very affordable. Based on real usage data: **Typical Costs:** - Small PR (< 1000 lines): $0.01 - $0.02 @@ -127,7 +127,7 @@ Costs with the default Kimi K2 thinking model are very affordable. Based on real - **Total cost: $0.01 - $0.05 per review** **Why So Affordable:** -- Kimi K2 has competitive pricing (~$0.001-$0.003 per 1k tokens) +- GLM 5.3 has competitive pricing (see OpenRouter) - Smart context management (only most recent AI review, limited commit history) - Most PRs are smaller than you think in token count - The 64k token limit is a ceiling, not typical usage @@ -138,7 +138,7 @@ Costs with the default Kimi K2 thinking model are very affordable. Based on real - Number of human comments and commit messages included - OpenRouter provider routing (prices vary slightly by provider) -Check [OpenRouter pricing](https://openrouter.ai/models) for current Kimi K2 rates. +Check [OpenRouter pricing](https://openrouter.ai/models) for current GLM 5.3 rates. ## Customization diff --git a/ai-reviewer.sh b/ai-reviewer.sh index e73a0fa..ec8cfe6 100644 --- a/ai-reviewer.sh +++ b/ai-reviewer.sh @@ -26,7 +26,7 @@ if [ -z "$API_KEY" ]; then fi # Configuration with defaults -AI_MODEL="${AI_MODEL:-minimax/minimax-m2.5}" +AI_MODEL="${AI_MODEL:-z-ai/glm-5.3}" AI_TEMPERATURE="${AI_TEMPERATURE:-0.1}" AI_MAX_TOKENS="${AI_MAX_TOKENS:-64000}" MAX_DIFF_SIZE="${MAX_DIFF_SIZE:-5000000}" # 5MB default limit (allows large PRs while preventing excessive API usage) @@ -171,7 +171,7 @@ if [ "$INCLUDE_HUMAN_COMMENTS" = "true" ] && [ -n "$PR_NUMBER" ] && [ -n "$REPO_ # and hide a real cut. if [ "$(printf '%s' "$HUMAN_COMMENTS_FULL" | wc -c)" -gt "$MAX_HUMAN_COMMENTS_TOTAL" ]; then HUMAN_COMMENTS="$HUMAN_COMMENTS -[…truncated at $MAX_HUMAN_COMMENTS_TOTAL characters]" +[…truncated at $MAX_HUMAN_COMMENTS_TOTAL bytes]" fi fi @@ -292,24 +292,22 @@ if [ "$INCLUDE_COMMIT_SUMMARY" = "true" ] && [ -n "$COMMITS_JSON" ] && [ "$COMMI AUTHOR_LINES="" if [ "$MAX_SUMMARY_COMMITS" -gt 0 ] && [ "$NONMERGE_COUNT" -gt 0 ]; then - STATS_FILE=$(mktemp) || { echo "Failed to create temporary file for commit stats"; exit 1; } - chmod 600 "$STATS_FILE" - # One "authoradditionsdeletions" row per listed commit; - # a failed stats fetch counts as zero rather than aborting the review. - while IFS=$'\t' read -r author sha; do - [ -n "$sha" ] || continue - line_stats=$(gh api "repos/$REPO_FULL_NAME/commits/$sha" \ - --jq '"\(.stats.additions // 0)\t\(.stats.deletions // 0)"' 2>/dev/null || printf '0\t0') - printf '%s\t%s\n' "$author" "$line_stats" >> "$STATS_FILE" - done < <(echo "$COMMITS_JSON" | jq -r --argjson n "$MAX_SUMMARY_COMMITS" \ - '[.[] | select(.commit.message | startswith("Merge") | not)] - | if $n > 0 then .[-$n:] else [] end - | .[] | [(.author.login // .commit.author.name), .sha] | @tsv') - # Aggregate per author; sort by added lines, then removed, then name. - AUTHOR_LINES=$(awk -F'\t' '{ count[$1]++; add[$1] += $2; del[$1] += $3 } - END { for (who in count) printf "%s\t%d\t%d\t%d\n", who, count[who], add[who], del[who] }' "$STATS_FILE" \ + # One "authoradditionsdeletions" row per listed commit, + # piped straight into the aggregation — no temp file to leak if a + # stats fetch goes wrong. A failed stats fetch counts as zero rather + # than aborting the review. + AUTHOR_LINES=$(while IFS=$'\t' read -r author sha; do + [ -n "$sha" ] || continue + line_stats=$(gh api "repos/$REPO_FULL_NAME/commits/$sha" \ + --jq '"\(.stats.additions // 0)\t\(.stats.deletions // 0)"' 2>/dev/null || printf '0\t0') + printf '%s\t%s\n' "$author" "$line_stats" + done < <(echo "$COMMITS_JSON" | jq -r --argjson n "$MAX_SUMMARY_COMMITS" \ + '[.[] | select(.commit.message | startswith("Merge") | not)] + | if $n > 0 then .[-$n:] else [] end + | .[] | [(.author.login // .commit.author.name), .sha] | @tsv') \ + | awk -F'\t' '{ count[$1]++; add[$1] += $2; del[$1] += $3 } + END { for (who in count) printf "%s\t%d\t%d\t%d\n", who, count[who], add[who], del[who] }' \ | LC_ALL=C sort -t$'\t' -k3,3nr -k4,4nr -k1,1) - rm -f "$STATS_FILE" fi if [ "$NONMERGE_COUNT" -gt 0 ]; then diff --git a/tests/test_reviewer_context.py b/tests/test_reviewer_context.py index 4ca305b..95800ed 100644 --- a/tests/test_reviewer_context.py +++ b/tests/test_reviewer_context.py @@ -225,7 +225,7 @@ def test_concise_instructions_preserve_review_depth_and_protocol(self): ["pass", "fail", "uncertain"]) self.assertEqual(request["max_tokens"], 64000) self.assertEqual(request["temperature"], 0.1) - self.assertEqual(request["model"], "minimax/minimax-m2.5") + self.assertEqual(request["model"], "z-ai/glm-5.3") def test_sticky_review_is_only_previous_ai_context(self): body = f"{MARKER}\n## Review results\nSticky AI review content" @@ -506,7 +506,7 @@ def test_human_comments_total_budget_marks_truncation(self): config={"MAX_HUMAN_COMMENTS_TOTAL": "50"}, ) prompt = request["messages"][0]["content"] - self.assertIn("[…truncated at 50 characters]", prompt) + self.assertIn("[…truncated at 50 bytes]", prompt) def test_human_comments_exactly_filling_budget_are_not_marked(self): # "**alice** (2026-09-12T10:00:00Z):\n" is 34 characters; a 16-char @@ -527,7 +527,7 @@ def test_human_comments_clipped_after_trailing_newlines_still_marked(self): config={"MAX_HUMAN_COMMENTS_TOTAL": "50"}, ) prompt = request["messages"][0]["content"] - self.assertIn("[…truncated at 50 characters]", prompt) + self.assertIn("[…truncated at 50 bytes]", prompt) if __name__ == "__main__": From a7fdc093bb9d62b4362e3a1e3ba066e8016bd6fe Mon Sep 17 00:00:00 2001 From: LearningCircuit <185559241+LearningCircuit@users.noreply.github.com> Date: Mon, 14 Sep 2026 21:18:58 +0200 Subject: [PATCH 07/17] feat: default model to z-ai/glm-5.3 Switch the built-in default (script and this repo's own reviewer workflow) from minimax/minimax-m2.5 to z-ai/glm-5.3; slug verified against OpenRouter's model list. AI_MODEL still overrides. --- .github/workflows/ai-code-reviewer.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ai-code-reviewer.yml b/.github/workflows/ai-code-reviewer.yml index 5221244..34c812b 100644 --- a/.github/workflows/ai-code-reviewer.yml +++ b/.github/workflows/ai-code-reviewer.yml @@ -34,7 +34,7 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} OPENROUTER_API_KEY: ${{ secrets.OPENROUTER_API_KEY }} - AI_MODEL: ${{ vars.AI_MODEL || 'minimax/minimax-m2.5' }} + AI_MODEL: ${{ vars.AI_MODEL || 'z-ai/glm-5.3' }} AI_TEMPERATURE: ${{ vars.AI_TEMPERATURE || '0.1' }} AI_MAX_TOKENS: ${{ vars.AI_MAX_TOKENS || '64000' }} MAX_DIFF_SIZE: ${{ vars.MAX_DIFF_SIZE || '800000' }} From 941c39e6e02cc27ed863474c26ae3a79ae1fc2b4 Mon Sep 17 00:00:00 2001 From: LearningCircuit <185559241+LearningCircuit@users.noreply.github.com> Date: Mon, 14 Sep 2026 21:49:01 +0200 Subject: [PATCH 08/17] fix: paginate comment and check-run fetches; mark message-list clips MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The comment fetch sent no per_page/--paginate, so GitHub returned a single default page of 30 comments, oldest first — on busy PRs the newest feedback (the whole point of the newest-first selection) was never fetched. The comment list is now fetched once with --paginate and merged locally (jq -s add), shared by the previous-AI-review and human-comment context; the previous-review selector also finds sticky reviews beyond page one now. The slice runs in local jq (--argjson caps), never inside a gh api --jq filter, because that would apply it per page. This also removes the now-dead COMMENT_SLICE branch. The check-runs fetch had the same first-page-only limit (30 runs per page by default); it paginates and merges locally too, so the pass summary counts every run on matrix-heavy repos. The commit-message list now marks its 2500-byte clip like every other budget, instead of cutting silently. The gh stub in the offline tests emulates real 30-item pages for both endpoints; new regressions cover beyond-page-one selection, sticky reviews past page one, all-page check summaries, and the marked message clip. --- README.md | 2 +- ai-reviewer.sh | 88 ++++++++++++++++++++-------------- tests/test_reviewer_context.py | 80 ++++++++++++++++++++++++++----- 3 files changed, 121 insertions(+), 49 deletions(-) diff --git a/README.md b/README.md index 912901f..2fcf7a2 100644 --- a/README.md +++ b/README.md @@ -195,7 +195,7 @@ The workflow fetches and sends these repository elements to the AI: 1. **Code Changes**: Full diff of modified files 2. **PR Description**: Title and description text from the pull request 3. **Commit Messages**: Up to `MAX_COMMIT_MESSAGES` most recent commit messages (default 3, excluding merges), plus a compact overview stating how many commits are already on the PR, the per-author commit counts, and each author's added/removed line totals (covering up to `MAX_SUMMARY_COMMITS` most recent commits, default 15) -4. **Human Comments**: Comments from human reviewers on the PR, newest first; bot comments are excluded, while human comments quoting a review header or marker are retained. Caps (`MAX_HUMAN_COMMENTS`, `MAX_HUMAN_COMMENT_LENGTH`, `MAX_HUMAN_COMMENTS_TOTAL`) clip the oldest first and mark any truncation. +4. **Human Comments**: Comments from human reviewers on the PR, fetched across all pages (not just the first 30), newest first; bot comments are excluded, while human comments quoting a review header or marker are retained. Caps (`MAX_HUMAN_COMMENTS`, `MAX_HUMAN_COMMENT_LENGTH`, `MAX_HUMAN_COMMENTS_TOTAL`) clip the oldest first and mark any truncation. 5. **Labels**: All repository labels with descriptions and colors (kept complete on purpose; the prompt instructs the model to only apply genuinely useful ones) 6. **Previous AI Review**: Most recent bot-authored AI review comment only (limited to 10k chars), identified by its review header or `` marker 7. **CI/CD Status**: A one-line summary of GitHub Actions check runs ("N of M checks passed") plus only the non-passing runs — failures, skipped, cancelled, timed out, or still running — listed individually; fully green matrix shards no longer flood the prompt diff --git a/ai-reviewer.sh b/ai-reviewer.sh index ec8cfe6..688f20a 100644 --- a/ai-reviewer.sh +++ b/ai-reviewer.sh @@ -130,38 +130,42 @@ def is_ai_review: startswith("## AI Code Review") or contains("")); ' -# Fetch previous AI review (only the most recent one) for context +# Fetch the PR's comment list once, shared by the previous-AI-review and +# human-comment context. --paginate is required: a plain gh api call returns +# only the first page (GitHub default: 30 comments, oldest first), which on +# busy PRs would silently hide the newest feedback. --paginate emits one JSON +# array per page back to back, so slurp with jq -s and 'add' to merge them +# into a single array before any slicing happens (the slice must NOT run +# inside a gh api --jq filter — that would apply it per page). +COMMENTS_JSON="[]" +if { [ "$INCLUDE_PREVIOUS_REVIEWS" = "true" ] || [ "$INCLUDE_HUMAN_COMMENTS" = "true" ]; } && [ -n "$PR_NUMBER" ] && [ -n "$REPO_FULL_NAME" ] && [ -n "$GITHUB_TOKEN" ]; then + COMMENTS_JSON=$(gh api "repos/$REPO_FULL_NAME/issues/$PR_NUMBER/comments" --paginate 2>/dev/null | jq -s 'add // []' || echo "[]") +fi + +# Previous AI review (only the most recent one) for context. Selected from the +# full merged comment list, so a sticky review beyond page one is still found. PREVIOUS_REVIEWS="" -if [ "$INCLUDE_PREVIOUS_REVIEWS" = "true" ] && [ -n "$PR_NUMBER" ] && [ -n "$REPO_FULL_NAME" ] && [ -n "$GITHUB_TOKEN" ]; then - # Fetch only the most recent AI review comment - PREVIOUS_REVIEWS=$(gh api "repos/$REPO_FULL_NAME/issues/$PR_NUMBER/comments" \ - --jq "$COMMENT_CLASSIFIERS"'[.[] | select(is_ai_review)] | last | if . then "### Previous AI Review (" + .created_at + "):\n" + .body + "\n---\n" else "" end' 2>/dev/null | head -c 10000 || echo "") -fi - -# Fetch human comments for context. Human comments are valuable, so the -# defaults are generous and configurable (see MAX_HUMAN_COMMENTS et al.): -# the newest MAX_HUMAN_COMMENTS comments are kept, presented newest-first so -# an overall-budget clip drops the oldest of the selected — never the latest -# feedback. Per-comment and overall clipping are marked as truncated. -# Exclude all bot comments; previous AI reviews have their own context block. +if [ "$INCLUDE_PREVIOUS_REVIEWS" = "true" ] && [ "$COMMENTS_JSON" != "[]" ]; then + PREVIOUS_REVIEWS=$(echo "$COMMENTS_JSON" | jq -r "$COMMENT_CLASSIFIERS"'[.[] | select(is_ai_review)] | last | if . then "### Previous AI Review (" + .created_at + "):\n" + .body + "\n---\n" else "" end' 2>/dev/null | head -c 10000 || echo "") +fi + +# Human comments for context. Human comments are valuable, so the defaults +# are generous and configurable (see MAX_HUMAN_COMMENTS et al.): the newest +# MAX_HUMAN_COMMENTS comments are kept out of the FULL paginated list, +# presented newest-first so an overall-budget clip drops the oldest of the +# selected — never the latest feedback. Per-comment and overall clipping are +# marked as truncated. Exclude all bot comments; previous AI reviews have +# their own context block. HUMAN_COMMENTS="" -if [ "$INCLUDE_HUMAN_COMMENTS" = "true" ] && [ -n "$PR_NUMBER" ] && [ -n "$REPO_FULL_NAME" ] && [ -n "$GITHUB_TOKEN" ] && [ "$MAX_HUMAN_COMMENTS" -gt 0 ] && [ "$MAX_HUMAN_COMMENTS_TOTAL" -gt 0 ]; then - # A count cap of 0 selects nothing; jq's .[-0:] would mean "everything", - # so build the slice expression explicitly. - if [ "$MAX_HUMAN_COMMENTS" -gt 0 ]; then - COMMENT_SLICE=".[-$MAX_HUMAN_COMMENTS:]" - else - COMMENT_SLICE="[]" - fi - # Splicing shell variables into a gh api --jq filter is safe here because - # gh api has no --arg passthrough and every value above is validated as a - # non-negative integer before use. - HUMAN_COMMENTS_FULL=$(gh api "repos/$REPO_FULL_NAME/issues/$PR_NUMBER/comments" \ - --jq "$COMMENT_CLASSIFIERS"'[.[] | select(is_bot | not)] - | '"$COMMENT_SLICE"' | reverse +if [ "$INCLUDE_HUMAN_COMMENTS" = "true" ] && [ "$COMMENTS_JSON" != "[]" ] && [ "$MAX_HUMAN_COMMENTS_TOTAL" -gt 0 ]; then + HUMAN_COMMENTS_FULL=$(echo "$COMMENTS_JSON" | jq -r \ + --argjson n "$MAX_HUMAN_COMMENTS" --argjson c "$MAX_HUMAN_COMMENT_LENGTH" \ + "$COMMENT_CLASSIFIERS"'[.[] | select(is_bot | not)] + | if $n > 0 then .[-$n:] else [] end + | reverse | map("**" + (.user.login // "unknown") + "** (" + .created_at + "):\n" - + (if ((.body // "") | length) > '"$MAX_HUMAN_COMMENT_LENGTH"' - then ((.body // "")[0:'"$MAX_HUMAN_COMMENT_LENGTH"'] + " […truncated]") + + (if ((.body // "") | length) > $c + then ((.body // "")[0:$c] + " […truncated]") else (.body // "") end)) | join("\n\n---\n\n")' 2>/dev/null || echo "") HUMAN_COMMENTS=$(printf '%s' "$HUMAN_COMMENTS_FULL" | head -c "$MAX_HUMAN_COMMENTS_TOTAL") @@ -186,12 +190,15 @@ if [ "$INCLUDE_CHECK_RUNS" = "true" ] && [ -n "$PR_NUMBER" ] && [ -n "$REPO_FULL HEAD_SHA=$(gh api "repos/$REPO_FULL_NAME/pulls/$PR_NUMBER" --jq '.head.sha' 2>/dev/null || echo "") if [ -n "$HEAD_SHA" ]; then - CHECK_RUNS_SUMMARY=$(gh api "repos/$REPO_FULL_NAME/commits/$HEAD_SHA/check-runs" \ - --jq '(.check_runs // []) - | {total: length, - passed: [.[] | select(.conclusion == "success")] | length, - other: [.[] | select(.conclusion != "success") - | "- **\(.name)**: \(.status)\(if .conclusion then " (\(.conclusion))" else "" end)"]}' 2>/dev/null || echo "") + # Paginate (the endpoint returns 30 runs per page by default — big + # matrix repos exceed that) and merge the pages locally; the summary + # must see the full list, not page one. + CHECK_RUNS_JSON=$(gh api "repos/$REPO_FULL_NAME/commits/$HEAD_SHA/check-runs" --paginate 2>/dev/null | jq -s 'map(.check_runs // []) | add // []' || echo "[]") + CHECK_RUNS_SUMMARY=$(echo "$CHECK_RUNS_JSON" | jq \ + '{total: length, + passed: [.[] | select(.conclusion == "success")] | length, + other: [.[] | select(.conclusion != "success") + | "- **\(.name)**: \(.status)\(if .conclusion then " (\(.conclusion))" else "" end)"]}' 2>/dev/null || echo "") if [ -n "$CHECK_RUNS_SUMMARY" ] && [ "$CHECK_RUNS_SUMMARY" != "null" ]; then TOTAL_CHECKS=$(echo "$CHECK_RUNS_SUMMARY" | jq -r '.total // 0') @@ -266,10 +273,17 @@ fi # cap compared to the overview statistic. COMMIT_MESSAGES="" if [ "$INCLUDE_COMMIT_MESSAGES" = "true" ] && [ "$COMMITS_JSON" != "[]" ] && [ "$COMMITS_JSON" != "" ]; then - COMMIT_MESSAGES=$(echo "$COMMITS_JSON" | jq -r --argjson n "$MAX_COMMIT_MESSAGES" \ + COMMIT_MESSAGES_FULL=$(echo "$COMMITS_JSON" | jq -r --argjson n "$MAX_COMMIT_MESSAGES" \ '[.[] | select(.commit.message | startswith("Merge") | not)] | if $n > 0 then .[-$n:] else [] end - | .[] | "- " + (.commit.message | split("\n")[0]) + (if (.commit.message | split("\n\n")[1]) then "\n " + (.commit.message | split("\n\n")[1]) else "" end)' 2>/dev/null | head -c 2500 || echo "") + | .[] | "- " + (.commit.message | split("\n")[0]) + (if (.commit.message | split("\n\n")[1]) then "\n " + (.commit.message | split("\n\n")[1]) else "" end)' 2>/dev/null || echo "") + COMMIT_MESSAGES=$(printf '%s' "$COMMIT_MESSAGES_FULL" | head -c 2500) + # Same contract as the human-comments budget: detect clipping from the + # source length and mark it, so the model knows messages were cut. + if [ "$(printf '%s' "$COMMIT_MESSAGES_FULL" | wc -c)" -gt 2500 ]; then + COMMIT_MESSAGES="$COMMIT_MESSAGES +[…truncated at 2500 bytes]" + fi if [ "$DEBUG_MODE" = "true" ] && [ -n "$COMMIT_MESSAGES" ]; then COMMIT_COUNT=$(echo "$COMMIT_MESSAGES" | grep -c "^- " || echo "0") diff --git a/tests/test_reviewer_context.py b/tests/test_reviewer_context.py index 95800ed..f0300df 100644 --- a/tests/test_reviewer_context.py +++ b/tests/test_reviewer_context.py @@ -88,10 +88,13 @@ def run_reviewer( calls.write(json.dumps(args) + "\\n") parts = args[1].split("?")[0].split("/")[3:] if parts == ["issues", "123", "comments"]: - assert args[2] == "--jq" and len(args) == 4, args - result = subprocess.run(["jq", "-r", args[3]], - input=(path / "comments.json").read_text(), text=True) - sys.exit(result.returncode) + assert args[2:] == ["--paginate"], args + comments = json.loads((path / "comments.json").read_text()) + # Emulate gh api --paginate: one JSON array per page, GitHub's default + # page size of 30, oldest first. + for start in range(0, len(comments), 30): + sys.stdout.write(json.dumps(comments[start:start + 30])) + sys.exit(0) if parts == ["pulls", "123", "commits"]: assert args[2:] == ["--paginate"], args sys.stdout.write((path / "pull-commits.json").read_text()) @@ -102,10 +105,14 @@ def run_reviewer( input=(path / "pr.json").read_text(), text=True) sys.exit(result.returncode) if parts == ["commits", "abc", "check-runs"]: - assert args[2] == "--jq" and len(args) == 4, args - result = subprocess.run(["jq", "-r", args[3]], - input=(path / "check-runs.json").read_text(), text=True) - sys.exit(result.returncode) + assert args[2:] == ["--paginate"], args + document = json.loads((path / "check-runs.json").read_text()) + runs = document["check_runs"] + for start in range(0, len(runs), 30): + sys.stdout.write(json.dumps( + {"total_count": len(runs), "check_runs": runs[start:start + 30]} + )) + sys.exit(0) if parts == ["labels"]: assert args[2:4] == ["--paginate", "--jq"] and len(args) == 5, args result = subprocess.run(["jq", "-r", args[4]], @@ -161,12 +168,12 @@ def run_reviewer( calls = calls_file.read_text().splitlines() if calls_file.exists() else [] self.gh_calls = [json.loads(call) for call in calls] # Every call must belong to a known context feature, and the - # comment endpoint is still fetched exactly once per enabled - # comment feature (never per comment). + # comment list is fetched exactly once (shared by the previous- + # review and human-comment features) whenever either is enabled. categorized = (self.comment_calls() + self.commit_calls() + self.check_calls() + self.label_calls()) self.assertEqual(len(self.gh_calls), len(categorized)) - self.assertEqual(len(self.comment_calls()), int(previous) + int(human)) + self.assertEqual(len(self.comment_calls()), int(bool(previous or human))) return json.loads((path / "request.json").read_text()) def comment_calls(self): @@ -529,6 +536,57 @@ def test_human_comments_clipped_after_trailing_newlines_still_marked(self): prompt = request["messages"][0]["content"] self.assertIn("[…truncated at 50 bytes]", prompt) + def test_human_comments_reach_beyond_the_first_page(self): + # GitHub's default page holds 30 comments, oldest first. The newest + # comments live on later pages; the selection must span all of them. + many = [comment(f"feedback number {index}", f"user-{index}", "User") + for index in range(1, 41)] + request = self.run_reviewer(many, previous=False, + config={"MAX_HUMAN_COMMENTS": "10"}) + prompt = request["messages"][0]["content"] + self.assertIn("feedback number 40", prompt) + self.assertIn("feedback number 31", prompt) + self.assertNotIn("feedback number 30", prompt) + self.assertNotIn("feedback number 1", prompt) + # One shared paginated fetch, not one call per page or per feature. + self.assertEqual(len(self.comment_calls()), 1) + self.assertEqual(self.comment_calls()[0][2:], ["--paginate"]) + + def test_previous_review_found_beyond_the_first_page(self): + many = [comment(f"noise {index}", f"user-{index}", "User") + for index in range(35)] + sticky = comment(f"{MARKER}\nLatest AI review beyond page one") + request = self.run_reviewer(many + [sticky], human=False) + prompt = request["messages"][0]["content"] + self.assertIn("Latest AI review beyond page one", prompt) + self.assertNotIn("noise 0", prompt) + + def test_commit_messages_clip_is_marked(self): + commits = [ + pull_commit(f"c{i}", f"feat: {i}\n\n" + "m" * 1100, login="alice") + for i in range(3) + ] + request = self.run_reviewer( + previous=False, human=False, pull_commits=commits, + config={"INCLUDE_COMMIT_MESSAGES": "true"}, + ) + prompt = request["messages"][0]["content"] + self.assertIn("[…truncated at 2500 bytes]", prompt) + + def test_check_run_summary_spans_all_pages(self): + request = self.run_reviewer( + previous=False, human=False, + check_runs=[ + {"name": f"shard-{index}", "status": "completed", "conclusion": "success"} + for index in range(35) + ] + [{"name": "e2e", "status": "completed", "conclusion": "failure"}], + config={"INCLUDE_CHECK_RUNS": "true"}, + ) + prompt = request["messages"][0]["content"] + self.assertIn("35 of 36 checks passed. Non-passing checks:", prompt) + self.assertIn("- **e2e**: completed (failure)", prompt) + self.assertNotIn("- **shard-0**", prompt) + if __name__ == "__main__": unittest.main() From d4defa86ca1578437715fedec20e64df2ce1cccf Mon Sep 17 00:00:00 2001 From: LearningCircuit <185559241+LearningCircuit@users.noreply.github.com> Date: Mon, 14 Sep 2026 22:05:29 +0200 Subject: [PATCH 09/17] fix: mark prior-review clips, report unavailable line stats, forward knobs - The previous AI review now marks its 10000-byte clip like every other budget, so a cut-off prior review (verdict, Should-be-checked items) is never mistaken for a complete one. - Per-commit line-stat fetch failures are counted and reported in the summary header ("line stats unavailable for K commit(s)") instead of silently reading as verified +0/-0. The stats rows now accumulate in a shell variable with an explicit newline (command substitution strips it; concatenated rows made awk mis-parse author totals). - The workflow's env block forwards the six new configuration variables from repository variables, so the README instructions for them actually take effect; a regression test pins the forwarding. --- .github/workflows/ai-code-reviewer.yml | 7 ++++ ai-reviewer.sh | 54 +++++++++++++++++++------- tests/test_reviewer_context.py | 38 ++++++++++++++++++ 3 files changed, 84 insertions(+), 15 deletions(-) diff --git a/.github/workflows/ai-code-reviewer.yml b/.github/workflows/ai-code-reviewer.yml index 34c812b..eb9f98b 100644 --- a/.github/workflows/ai-code-reviewer.yml +++ b/.github/workflows/ai-code-reviewer.yml @@ -39,6 +39,13 @@ jobs: AI_MAX_TOKENS: ${{ vars.AI_MAX_TOKENS || '64000' }} MAX_DIFF_SIZE: ${{ vars.MAX_DIFF_SIZE || '800000' }} EXCLUDE_FILE_PATTERNS: ${{ vars.EXCLUDE_FILE_PATTERNS || '*.lock,*.min.js,*.min.css,package-lock.json,yarn.lock' }} + MAX_SUMMARY_COMMITS: ${{ vars.MAX_SUMMARY_COMMITS || '15' }} + MAX_COMMIT_MESSAGES: ${{ vars.MAX_COMMIT_MESSAGES || '3' }} + INCLUDE_COMMIT_SUMMARY: ${{ vars.INCLUDE_COMMIT_SUMMARY || 'true' }} + MAX_HUMAN_COMMENTS: ${{ vars.MAX_HUMAN_COMMENTS || '100' }} + MAX_HUMAN_COMMENT_LENGTH: ${{ vars.MAX_HUMAN_COMMENT_LENGTH || '4000' }} + MAX_HUMAN_COMMENTS_TOTAL: ${{ vars.MAX_HUMAN_COMMENTS_TOTAL || '20000' }} + STRUCTURED_OUTPUT: ${{ vars.STRUCTURED_OUTPUT || 'true' }} PR_NUMBER: ${{ github.event.pull_request.number }} REPO_FULL_NAME: ${{ github.repository }} FAIL_ON_REQUESTED_CHANGES: ${{ vars.FAIL_ON_REQUESTED_CHANGES || 'false' }} diff --git a/ai-reviewer.sh b/ai-reviewer.sh index 688f20a..7386fba 100644 --- a/ai-reviewer.sh +++ b/ai-reviewer.sh @@ -146,7 +146,15 @@ fi # full merged comment list, so a sticky review beyond page one is still found. PREVIOUS_REVIEWS="" if [ "$INCLUDE_PREVIOUS_REVIEWS" = "true" ] && [ "$COMMENTS_JSON" != "[]" ]; then - PREVIOUS_REVIEWS=$(echo "$COMMENTS_JSON" | jq -r "$COMMENT_CLASSIFIERS"'[.[] | select(is_ai_review)] | last | if . then "### Previous AI Review (" + .created_at + "):\n" + .body + "\n---\n" else "" end' 2>/dev/null | head -c 10000 || echo "") + PREVIOUS_REVIEWS_FULL=$(echo "$COMMENTS_JSON" | jq -r "$COMMENT_CLASSIFIERS"'[.[] | select(is_ai_review)] | last | if . then "### Previous AI Review (" + .created_at + "):\n" + .body + "\n---\n" else "" end' 2>/dev/null || echo "") + PREVIOUS_REVIEWS=$(printf '%s' "$PREVIOUS_REVIEWS_FULL" | head -c 10000) + # Same truncation contract as every other budget: detect from the source + # length and mark, so a cut-off prior review (verdict, "Should be checked" + # items) is never mistaken for a complete one. + if [ "$(printf '%s' "$PREVIOUS_REVIEWS_FULL" | wc -c)" -gt 10000 ]; then + PREVIOUS_REVIEWS="$PREVIOUS_REVIEWS +[…truncated at 10000 bytes]" + fi fi # Human comments for context. Human comments are valuable, so the defaults @@ -305,22 +313,36 @@ if [ "$INCLUDE_COMMIT_SUMMARY" = "true" ] && [ -n "$COMMITS_JSON" ] && [ "$COMMI MERGE_COUNT=$(( MERGE_COUNT - NONMERGE_COUNT )) AUTHOR_LINES="" + STATS_FAILURES=0 if [ "$MAX_SUMMARY_COMMITS" -gt 0 ] && [ "$NONMERGE_COUNT" -gt 0 ]; then # One "authoradditionsdeletions" row per listed commit, - # piped straight into the aggregation — no temp file to leak if a - # stats fetch goes wrong. A failed stats fetch counts as zero rather - # than aborting the review. - AUTHOR_LINES=$(while IFS=$'\t' read -r author sha; do - [ -n "$sha" ] || continue - line_stats=$(gh api "repos/$REPO_FULL_NAME/commits/$sha" \ - --jq '"\(.stats.additions // 0)\t\(.stats.deletions // 0)"' 2>/dev/null || printf '0\t0') - printf '%s\t%s\n' "$author" "$line_stats" - done < <(echo "$COMMITS_JSON" | jq -r --argjson n "$MAX_SUMMARY_COMMITS" \ - '[.[] | select(.commit.message | startswith("Merge") | not)] - | if $n > 0 then .[-$n:] else [] end - | .[] | [(.author.login // .commit.author.name), .sha] | @tsv') \ - | awk -F'\t' '{ count[$1]++; add[$1] += $2; del[$1] += $3 } - END { for (who in count) printf "%s\t%d\t%d\t%d\n", who, count[who], add[who], del[who] }' \ + # aggregated right after — no temp file to leak if a stats fetch + # goes wrong. A failed fetch counts as zero lines but is tracked + # separately, so "unknown" never masquerades as a verified +0/-0 + # (the summary header says how many commits lack line stats). + # The loop reads from process substitution, so it runs in the + # current shell and the counters below persist. + STATS_ROWS="" + while IFS=$'\t' read -r author sha; do + [ -n "$sha" ] || continue + if line_stats=$(gh api "repos/$REPO_FULL_NAME/commits/$sha" \ + --jq '"\(.stats.additions // 0)\t\(.stats.deletions // 0)"' 2>/dev/null); then + : + else + line_stats=$(printf '0\t0') + STATS_FAILURES=$((STATS_FAILURES + 1)) + fi + # Command substitution strips the trailing newline, so append it + # separately — without it the rows concatenate and awk mis-parses. + STATS_ROWS+=$(printf '%s\t%s' "$author" "$line_stats") + STATS_ROWS+=$'\n' + done < <(echo "$COMMITS_JSON" | jq -r --argjson n "$MAX_SUMMARY_COMMITS" \ + '[.[] | select(.commit.message | startswith("Merge") | not)] + | if $n > 0 then .[-$n:] else [] end + | .[] | [(.author.login // .commit.author.name), .sha] | @tsv') + # Aggregate per author; sort by added lines, then removed, then name. + AUTHOR_LINES=$(printf '%s' "$STATS_ROWS" | awk -F'\t' '{ count[$1]++; add[$1] += $2; del[$1] += $3 } + END { for (who in count) printf "%s\t%d\t%d\t%d\n", who, count[who], add[who], del[who] }' \ | LC_ALL=C sort -t$'\t' -k3,3nr -k4,4nr -k1,1) fi @@ -333,6 +355,8 @@ if [ "$INCLUDE_COMMIT_SUMMARY" = "true" ] && [ -n "$COMMITS_JSON" ] && [ "$COMMI COMMIT_WORD="commits" fi [ "$MERGE_COUNT" -gt 0 ] && SUMMARY_HEADER="$SUMMARY_HEADER (excluding $MERGE_COUNT merge commit(s))" + # Failed line-stat fetches must not read as verified zeros. + [ "$STATS_FAILURES" -gt 0 ] && SUMMARY_HEADER="$SUMMARY_HEADER (line stats unavailable for $STATS_FAILURES commit(s))" if [ -n "$AUTHOR_LINES" ]; then LISTED=$(( MAX_SUMMARY_COMMITS < NONMERGE_COUNT ? MAX_SUMMARY_COMMITS : NONMERGE_COUNT )) [ "$LISTED" -eq "$NONMERGE_COUNT" ] \ diff --git a/tests/test_reviewer_context.py b/tests/test_reviewer_context.py index f0300df..43bc0c4 100644 --- a/tests/test_reviewer_context.py +++ b/tests/test_reviewer_context.py @@ -587,6 +587,44 @@ def test_check_run_summary_spans_all_pages(self): self.assertIn("- **e2e**: completed (failure)", prompt) self.assertNotIn("- **shard-0**", prompt) + def test_previous_review_clip_is_marked(self): + sticky = comment(f"{MARKER}\n" + "r" * 11000 + "\n\n✅ Approved") + request = self.run_reviewer([sticky], human=False) + prompt = request["messages"][0]["content"] + self.assertIn("Previous AI Review (for context", prompt) + self.assertIn("[…truncated at 10000 bytes]", prompt) + + def test_commit_summary_reports_unavailable_line_stats(self): + commits = [ + pull_commit("ok1", "feat: one", login="alice"), + pull_commit("bad1", "feat: two", login="alice"), + ] + # Only ok1 has stats; the fetch for bad1 fails (the stub rejects + # unknown shas), which must be reported instead of reading as +0/-0. + request = self.run_reviewer( + previous=False, human=False, + pull_commits=commits, + commit_stats={"ok1": {"stats": {"additions": 7, "deletions": 2}}}, + config={"INCLUDE_COMMIT_SUMMARY": "true"}, + ) + prompt = request["messages"][0]["content"] + self.assertIn( + "There are 2 commits already on this PR " + "(line stats unavailable for 1 commit(s))", + prompt, + ) + + def test_workflow_forwards_reviewer_configuration(self): + workflow = (Path(__file__).resolve().parents[1] + / ".github" / "workflows" / "ai-code-reviewer.yml").read_text() + for name in ( + "AI_MODEL", "MAX_DIFF_SIZE", "STRUCTURED_OUTPUT", + "MAX_SUMMARY_COMMITS", "MAX_COMMIT_MESSAGES", "INCLUDE_COMMIT_SUMMARY", + "MAX_HUMAN_COMMENTS", "MAX_HUMAN_COMMENT_LENGTH", + "MAX_HUMAN_COMMENTS_TOTAL", + ): + self.assertIn(name + ": ${{ vars." + name, workflow) + if __name__ == "__main__": unittest.main() From a78fe9a6777b5354529dbf8f7fc44012584b9f84 Mon Sep 17 00:00:00 2001 From: LearningCircuit <185559241+LearningCircuit@users.noreply.github.com> Date: Mon, 14 Sep 2026 22:30:08 +0200 Subject: [PATCH 10/17] fix: exact merge predicate, UTF-8-safe clips, marked description cut - Merge detection now uses the exact GitHub signal (.parents | length > 1) centralized in one jq predicate, instead of a startswith("Merge") heuristic that silently miscounted commits like "MergeableHashMap: fix iteration" once the overview made it load-bearing. - Byte-boundary clips strip a trailing incomplete UTF-8 sequence (head -c can split a multibyte character and jq rejects invalid UTF-8 when building the request payload, failing the whole review); applied to the human-comments, previous-review, and commit-message budgets. - The PR-description budget follows the shared truncation contract now: detected from source length and marked. - Test fixture: restore the f-prefix lost on the {FOOTER} literal so the fixture round-trips a realistic footer again. --- ai-reviewer.sh | 42 +++++++++++++++++++++------ tests/test_reviewer_context.py | 52 ++++++++++++++++++++++++++++------ 2 files changed, 77 insertions(+), 17 deletions(-) diff --git a/ai-reviewer.sh b/ai-reviewer.sh index 7386fba..ca602fc 100644 --- a/ai-reviewer.sh +++ b/ai-reviewer.sh @@ -130,6 +130,24 @@ def is_ai_review: startswith("## AI Code Review") or contains("")); ' +# Exact merge-commit predicate for the commit list, shared by the message +# selection and the overview counts: a commit with more than one parent IS a +# merge. Message-prefix heuristics ("Merge...") misfire on subjects like +# "MergeableHashMap: fix iteration". +COMMIT_CLASSIFIERS=' +def is_merge: + (.parents | length) > 1; +' + +# head -c cuts at a byte boundary, which can split a multibyte UTF-8 +# character and leave an invalid sequence at the end of a block — jq (which +# builds the request payload) rejects invalid UTF-8, so the whole review +# would fail over one clipped emoji. Strip a trailing incomplete sequence; +# complete characters are never touched. +strip_partial_utf8() { + perl -pe 's/(?:[\xF0-\xF4][\x80-\xBF]{0,2}|[\xE0-\xEF][\x80-\xBF]?|[\xC2-\xDF])$//' +} + # Fetch the PR's comment list once, shared by the previous-AI-review and # human-comment context. --paginate is required: a plain gh api call returns # only the first page (GitHub default: 30 comments, oldest first), which on @@ -147,7 +165,7 @@ fi PREVIOUS_REVIEWS="" if [ "$INCLUDE_PREVIOUS_REVIEWS" = "true" ] && [ "$COMMENTS_JSON" != "[]" ]; then PREVIOUS_REVIEWS_FULL=$(echo "$COMMENTS_JSON" | jq -r "$COMMENT_CLASSIFIERS"'[.[] | select(is_ai_review)] | last | if . then "### Previous AI Review (" + .created_at + "):\n" + .body + "\n---\n" else "" end' 2>/dev/null || echo "") - PREVIOUS_REVIEWS=$(printf '%s' "$PREVIOUS_REVIEWS_FULL" | head -c 10000) + PREVIOUS_REVIEWS=$(printf '%s' "$PREVIOUS_REVIEWS_FULL" | head -c 10000 | strip_partial_utf8) # Same truncation contract as every other budget: detect from the source # length and mark, so a cut-off prior review (verdict, "Should be checked" # items) is never mistaken for a complete one. @@ -176,7 +194,7 @@ if [ "$INCLUDE_HUMAN_COMMENTS" = "true" ] && [ "$COMMENTS_JSON" != "[]" ] && [ " then ((.body // "")[0:$c] + " […truncated]") else (.body // "") end)) | join("\n\n---\n\n")' 2>/dev/null || echo "") - HUMAN_COMMENTS=$(printf '%s' "$HUMAN_COMMENTS_FULL" | head -c "$MAX_HUMAN_COMMENTS_TOTAL") + HUMAN_COMMENTS=$(printf '%s' "$HUMAN_COMMENTS_FULL" | head -c "$MAX_HUMAN_COMMENTS_TOTAL" | strip_partial_utf8) # Detect clipping from the source length, not the result's byte count: # command substitution strips trailing newlines, so a comment ending in # blank lines could otherwise shrink the clipped result below the budget @@ -251,8 +269,16 @@ if [ "$INCLUDE_PR_DESCRIPTION" = "true" ] && [ -n "$PR_NUMBER" ] && [ -n "$REPO_ if [ "$DEBUG_MODE" = "true" ]; then echo "🔍 Fetching PR title and description..." >&2 fi - PR_DESCRIPTION=$(gh api "repos/$REPO_FULL_NAME/pulls/$PR_NUMBER" \ - --jq '"**PR Title**: " + .title + "\n\n**Description**:\n" + (.body // "No description provided")' 2>/dev/null | head -c 2000 || echo "") + PR_DESCRIPTION_FULL=$(gh api "repos/$REPO_FULL_NAME/pulls/$PR_NUMBER" \ + --jq '"**PR Title**: " + .title + "\n\n**Description**:\n" + (.body // "No description provided")' 2>/dev/null || echo "") + PR_DESCRIPTION=$(printf '%s' "$PR_DESCRIPTION_FULL" | head -c 2000 | strip_partial_utf8) + # Same truncation contract as every other budget: detect from the source + # length and mark, so a cut-off description is never mistaken for the + # complete one. + if [ "$(printf '%s' "$PR_DESCRIPTION_FULL" | wc -c)" -gt 2000 ]; then + PR_DESCRIPTION="$PR_DESCRIPTION +[…truncated at 2000 bytes]" + fi if [ "$DEBUG_MODE" = "true" ] && [ -n "$PR_DESCRIPTION" ]; then echo "✅ Successfully fetched PR description" >&2 @@ -282,10 +308,10 @@ fi COMMIT_MESSAGES="" if [ "$INCLUDE_COMMIT_MESSAGES" = "true" ] && [ "$COMMITS_JSON" != "[]" ] && [ "$COMMITS_JSON" != "" ]; then COMMIT_MESSAGES_FULL=$(echo "$COMMITS_JSON" | jq -r --argjson n "$MAX_COMMIT_MESSAGES" \ - '[.[] | select(.commit.message | startswith("Merge") | not)] + "$COMMIT_CLASSIFIERS"'[.[] | select(is_merge | not)] | if $n > 0 then .[-$n:] else [] end | .[] | "- " + (.commit.message | split("\n")[0]) + (if (.commit.message | split("\n\n")[1]) then "\n " + (.commit.message | split("\n\n")[1]) else "" end)' 2>/dev/null || echo "") - COMMIT_MESSAGES=$(printf '%s' "$COMMIT_MESSAGES_FULL" | head -c 2500) + COMMIT_MESSAGES=$(printf '%s' "$COMMIT_MESSAGES_FULL" | head -c 2500 | strip_partial_utf8) # Same contract as the human-comments budget: detect clipping from the # source length and mark it, so the model knows messages were cut. if [ "$(printf '%s' "$COMMIT_MESSAGES_FULL" | wc -c)" -gt 2500 ]; then @@ -308,7 +334,7 @@ fi # fully quoted message list (MAX_COMMIT_MESSAGES). COMMIT_SUMMARY="" if [ "$INCLUDE_COMMIT_SUMMARY" = "true" ] && [ -n "$COMMITS_JSON" ] && [ "$COMMITS_JSON" != "[]" ]; then - NONMERGE_COUNT=$(echo "$COMMITS_JSON" | jq '[.[] | select(.commit.message | startswith("Merge") | not)] | length') + NONMERGE_COUNT=$(echo "$COMMITS_JSON" | jq "$COMMIT_CLASSIFIERS"'[.[] | select(is_merge | not)] | length') MERGE_COUNT=$(echo "$COMMITS_JSON" | jq 'length' ) MERGE_COUNT=$(( MERGE_COUNT - NONMERGE_COUNT )) @@ -337,7 +363,7 @@ if [ "$INCLUDE_COMMIT_SUMMARY" = "true" ] && [ -n "$COMMITS_JSON" ] && [ "$COMMI STATS_ROWS+=$(printf '%s\t%s' "$author" "$line_stats") STATS_ROWS+=$'\n' done < <(echo "$COMMITS_JSON" | jq -r --argjson n "$MAX_SUMMARY_COMMITS" \ - '[.[] | select(.commit.message | startswith("Merge") | not)] + "$COMMIT_CLASSIFIERS"'[.[] | select(is_merge | not)] | if $n > 0 then .[-$n:] else [] end | .[] | [(.author.login // .commit.author.name), .sha] | @tsv') # Aggregate per author; sort by added lines, then removed, then name. diff --git a/tests/test_reviewer_context.py b/tests/test_reviewer_context.py index 43bc0c4..2086a66 100644 --- a/tests/test_reviewer_context.py +++ b/tests/test_reviewer_context.py @@ -35,9 +35,11 @@ def comment(body, login="reviewer[bot]", user_type="Bot"): } -def pull_commit(sha, message, login=None, name=None): +def pull_commit(sha, message, login=None, name=None, merge=False): + parents = [{"sha": f"{sha}-parent-1"}, {"sha": f"{sha}-parent-2"}] if merge else [{"sha": f"{sha}-parent-1"}] return { "sha": sha, + "parents": parents, "author": {"login": login} if login is not None else None, "commit": {"author": {"name": name or login or "unknown"}, "message": message}, } @@ -56,6 +58,7 @@ def run_reviewer( commit_stats=None, check_runs=None, labels=None, + pr=None, ): with tempfile.TemporaryDirectory() as directory: path = Path(directory) @@ -65,7 +68,7 @@ def run_reviewer( ) (path / "commit-stats.json").write_text(json.dumps(commit_stats or {})) (path / "pr.json").write_text(json.dumps( - {"number": 123, "head": {"sha": "abc"}} + pr if pr is not None else {"number": 123, "head": {"sha": "abc"}} )) (path / "check-runs.json").write_text(json.dumps( {"total_count": len(check_runs or []), "check_runs": check_runs or []} @@ -312,7 +315,7 @@ def test_actionable_output_and_custom_budget_are_preserved(self): "indexing.\n- **nit** — file.py:40: \"Inference (not verified): \" " "the loop could early-exit.\n\nShould be checked:\n- Cannot verify " "the migration is reversible from diff - please confirm a " - "downgrade path exists.\n\n❌ Request changes\n\n{FOOTER}" + f"downgrade path exists.\n\n❌ Request changes\n\n{FOOTER}" ), "fail_pass_workflow": "fail", "labels_added": ["bug", "tests"], @@ -326,13 +329,17 @@ def test_actionable_output_and_custom_budget_are_preserved(self): def commit_fixture(self): commits = [ + # A subject starting with "Merge" on a single-parent commit: the + # exact parents-based predicate must count it. + pull_commit("a0", "MergeableHashMap: fix iteration", login="dana"), pull_commit("a1", "feat: first", login="alice"), pull_commit("a2", "feat: second", login="alice"), pull_commit("a3", "fix: bob fix", name="Bob B"), - pull_commit("a4", "Merge branch 'x' into main", login="alice"), + pull_commit("a4", "Merge branch 'x' into main", login="alice", merge=True), pull_commit("a5", "feat: third", login="carol"), ] stats = { + "a0": {"stats": {"additions": 6, "deletions": 2}}, "a1": {"stats": {"additions": 10, "deletions": 2}}, "a2": {"stats": {"additions": 20, "deletions": 3}}, "a3": {"stats": {"additions": 5, "deletions": 8}}, @@ -349,20 +356,22 @@ def test_commit_summary_counts_authors_and_lines(self): ) prompt = request["messages"][0]["content"] self.assertIn( - "There are 4 commits already on this PR (excluding 1 merge commit(s))", + "There are 5 commits already on this PR (excluding 1 merge commit(s))", prompt, ) - self.assertIn("across all 4 commits", prompt) + self.assertIn("across all 5 commits", prompt) self.assertIn("- **alice**: 2 commits, +30/-5 lines", prompt) self.assertIn("- **Bob B**: 1 commit, +5/-8 lines", prompt) self.assertIn("- **carol**: 1 commit, +1/-1 lines", prompt) + # The merge-sounding subject is counted, not skipped. + self.assertIn("- **dana**: 1 commit, +6/-2 lines", prompt) # Bullet order follows added lines, so the biggest author leads. self.assertLess( prompt.index("- **alice**"), prompt.index("- **carol**"), ) # Line stats are fetched per listed non-merge commit only. stats_urls = [call[1] for call in self.commit_calls() if "/pulls/" not in call[1]] - self.assertEqual(len(stats_urls), 4) + self.assertEqual(len(stats_urls), 5) self.assertFalse(any("a4" in url for url in stats_urls), stats_urls) # Only the summary block is present when messages are disabled. self.assertNotIn("Commit History (showing development journey):", prompt) @@ -379,7 +388,7 @@ def test_commit_summary_limit_is_independent_of_message_limit(self): }, ) prompt = request["messages"][0]["content"] - self.assertIn("across the 1 most recent of 4 commits", prompt) + self.assertIn("across the 1 most recent of 5 commits", prompt) self.assertIn("- **carol**: 1 commit, +1/-1 lines", prompt) self.assertNotIn("- **alice**:", prompt) # The message list keeps its own default cap (3) and still shows @@ -415,7 +424,7 @@ def test_commit_summary_zero_reads_no_individual_commits(self): config={"INCLUDE_COMMIT_SUMMARY": "true", "MAX_SUMMARY_COMMITS": "0"}, ) prompt = request["messages"][0]["content"] - self.assertIn("There are 4 commits already on this PR", prompt) + self.assertIn("There are 5 commits already on this PR", prompt) self.assertNotIn("Per-author", prompt) # One list call only: zero individual commit reads. urls = [call[1] for call in self.commit_calls()] @@ -515,6 +524,31 @@ def test_human_comments_total_budget_marks_truncation(self): prompt = request["messages"][0]["content"] self.assertIn("[…truncated at 50 bytes]", prompt) + def test_multibyte_comment_survives_a_byte_boundary_clip(self): + # The budget lands mid-emoji: without stripping the partial UTF-8 + # sequence, jq rejects the prompt when building the request payload + # and the whole review fails. The header is 34 bytes, so a 39-byte + # budget keeps one complete 4-byte emoji and cuts one byte into the + # next. + request = self.run_reviewer( + [comment("😀" * 20, "alice", "User")], previous=False, + config={"MAX_HUMAN_COMMENTS_TOTAL": "39"}, + ) + prompt = request["messages"][0]["content"] + self.assertIn("😀", prompt) + self.assertIn("[…truncated at 39 bytes]", prompt) + + def test_pr_description_clip_is_marked(self): + request = self.run_reviewer( + previous=False, human=False, + pr={"number": 123, "head": {"sha": "abc"}, + "title": "A change", "body": "d" * 3000}, + config={"INCLUDE_PR_DESCRIPTION": "true"}, + ) + prompt = request["messages"][0]["content"] + self.assertIn("**PR Title**: A change", prompt) + self.assertIn("[…truncated at 2000 bytes]", prompt) + def test_human_comments_exactly_filling_budget_are_not_marked(self): # "**alice** (2026-09-12T10:00:00Z):\n" is 34 characters; a 16-char # body makes the block exactly 50 — no clip, so no marker. From b1b2cb4f693d468a51c4689d82036f0f761d1d57 Mon Sep 17 00:00:00 2001 From: LearningCircuit <185559241+LearningCircuit@users.noreply.github.com> Date: Mon, 14 Sep 2026 22:52:59 +0200 Subject: [PATCH 11/17] fix: count merges directly instead of total-minus-nonmerge The subtraction was correct today but silently wrong if the non-merge filter ever gained more exclusions. Both counts now come straight from the shared is_merge classifier, with 2>/dev/null fallbacks and numeric validation so a jq hiccup degrades to zero rather than propagating an empty string. --- ai-reviewer.sh | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/ai-reviewer.sh b/ai-reviewer.sh index ca602fc..d5cf736 100644 --- a/ai-reviewer.sh +++ b/ai-reviewer.sh @@ -334,9 +334,14 @@ fi # fully quoted message list (MAX_COMMIT_MESSAGES). COMMIT_SUMMARY="" if [ "$INCLUDE_COMMIT_SUMMARY" = "true" ] && [ -n "$COMMITS_JSON" ] && [ "$COMMITS_JSON" != "[]" ]; then - NONMERGE_COUNT=$(echo "$COMMITS_JSON" | jq "$COMMIT_CLASSIFIERS"'[.[] | select(is_merge | not)] | length') - MERGE_COUNT=$(echo "$COMMITS_JSON" | jq 'length' ) - MERGE_COUNT=$(( MERGE_COUNT - NONMERGE_COUNT )) + # Count merges directly with the shared classifier rather than deriving + # them by subtraction — self-consistent if the non-merge filter ever + # gains more exclusions — and guard both counts so a jq hiccup degrades + # to zero instead of propagating an empty string. + NONMERGE_COUNT=$(echo "$COMMITS_JSON" | jq "$COMMIT_CLASSIFIERS"'[.[] | select(is_merge | not)] | length' 2>/dev/null || echo 0) + MERGE_COUNT=$(echo "$COMMITS_JSON" | jq "$COMMIT_CLASSIFIERS"'[.[] | select(is_merge)] | length' 2>/dev/null || echo 0) + [[ "$NONMERGE_COUNT" =~ ^[0-9]+$ ]] || NONMERGE_COUNT=0 + [[ "$MERGE_COUNT" =~ ^[0-9]+$ ]] || MERGE_COUNT=0 AUTHOR_LINES="" STATS_FAILURES=0 From 6a705f350f0831264036c8066592c6de223d09e5 Mon Sep 17 00:00:00 2001 From: LearningCircuit <185559241+LearningCircuit@users.noreply.github.com> Date: Mon, 14 Sep 2026 23:13:47 +0200 Subject: [PATCH 12/17] fix: cap red-shard list, fetch PR object once, forward all knobs - The non-passing check-run list is capped at 20 lines with a "+K more non-passing run(s) not listed" line, so a broadly red matrix (shared dependency failure, mass cancellation) cannot reintroduce the prompt flooding exactly when the diff context is largest. - The PR object is fetched once and shared by the check-runs head-SHA lookup and the PR-description context; both default-on, so the same endpoint was hit twice on every review. - The workflow now forwards every INCLUDE_* toggle the script reads (previously silently ignored as repository variables), and the forwarding test derives the expected knob list from the script's \${VAR:-default} lines so a new knob fails the test until forwarded. - README: document the 0 semantics for the human-comment caps, note the check-list cap, and label the cost ranges as carried-over estimates. --- .github/workflows/ai-code-reviewer.yml | 6 +++ README.md | 8 ++-- ai-reviewer.sh | 34 +++++++++----- tests/test_reviewer_context.py | 62 ++++++++++++++++++++------ 4 files changed, 81 insertions(+), 29 deletions(-) diff --git a/.github/workflows/ai-code-reviewer.yml b/.github/workflows/ai-code-reviewer.yml index eb9f98b..461b12c 100644 --- a/.github/workflows/ai-code-reviewer.yml +++ b/.github/workflows/ai-code-reviewer.yml @@ -42,6 +42,12 @@ jobs: MAX_SUMMARY_COMMITS: ${{ vars.MAX_SUMMARY_COMMITS || '15' }} MAX_COMMIT_MESSAGES: ${{ vars.MAX_COMMIT_MESSAGES || '3' }} INCLUDE_COMMIT_SUMMARY: ${{ vars.INCLUDE_COMMIT_SUMMARY || 'true' }} + INCLUDE_PREVIOUS_REVIEWS: ${{ vars.INCLUDE_PREVIOUS_REVIEWS || 'true' }} + INCLUDE_HUMAN_COMMENTS: ${{ vars.INCLUDE_HUMAN_COMMENTS || 'true' }} + INCLUDE_CHECK_RUNS: ${{ vars.INCLUDE_CHECK_RUNS || 'true' }} + INCLUDE_LABELS: ${{ vars.INCLUDE_LABELS || 'true' }} + INCLUDE_PR_DESCRIPTION: ${{ vars.INCLUDE_PR_DESCRIPTION || 'true' }} + INCLUDE_COMMIT_MESSAGES: ${{ vars.INCLUDE_COMMIT_MESSAGES || 'true' }} MAX_HUMAN_COMMENTS: ${{ vars.MAX_HUMAN_COMMENTS || '100' }} MAX_HUMAN_COMMENT_LENGTH: ${{ vars.MAX_HUMAN_COMMENT_LENGTH || '4000' }} MAX_HUMAN_COMMENTS_TOTAL: ${{ vars.MAX_HUMAN_COMMENTS_TOTAL || '20000' }} diff --git a/README.md b/README.md index 2fcf7a2..fa3aeb9 100644 --- a/README.md +++ b/README.md @@ -76,9 +76,9 @@ The workflow is pre-configured with sensible defaults, but you can customize it - **MAX_SUMMARY_COMMITS**: How many of the PR's most recent commits the commit overview reads (default: `15`; `0` shows the commit count only). The overview tells the model how many commits are already on the PR, who made them, and each author's added/removed line totals. Each summarized commit costs one extra GitHub API call, but only a handful of numbers enter the prompt, so this cap can stay generous. - **MAX_COMMIT_MESSAGES**: How many commit messages are fully quoted in the prompt (default: `3`). Fully quoted messages are the token-expensive part of the commit history, hence the separate, smaller cap — the overview (above) still covers many more commits. - **INCLUDE_COMMIT_SUMMARY**: Include the "There are X commits already on this PR" overview with per-author counts and line totals (default: `true`) -- **MAX_HUMAN_COMMENTS**: How many of the newest human comments are included (default: `100`). Comments are presented newest-first, so when this or the overall budget clips, the oldest go first — the latest feedback always survives. +- **MAX_HUMAN_COMMENTS**: How many of the newest human comments are included (default: `100`; `0` includes none at all). Comments are presented newest-first, so when this or the overall budget clips, the oldest go first — the latest feedback always survives. - **MAX_HUMAN_COMMENT_LENGTH**: Maximum characters per human comment; longer comments are clipped and marked " […truncated]" (default: `4000`) -- **MAX_HUMAN_COMMENTS_TOTAL**: Overall byte budget for the human-comments block (`head -c`); when exceeded, the block is cut and marked (default: `20000`) +- **MAX_HUMAN_COMMENTS_TOTAL**: Overall byte budget for the human-comments block (`head -c`); when exceeded, the block is cut and marked (default: `20000`; `0` omits the block entirely) - **STRUCTURED_OUTPUT**: Enforce a JSON Schema on the model's output via OpenRouter structured outputs (default: `true`) - Makes the provider emit valid, correctly-escaped JSON instead of the model hand-writing it — the main cause of "Invalid JSON response from AI model" - Requires a model/provider that supports `response_format` json_schema (most modern models do; e.g. Kimi K2, MiniMax M2.5) @@ -114,7 +114,7 @@ The AI reviews your code across all focus areas and reports actionable findings ## Cost Estimation -Costs with the default GLM 5.3 model are very affordable. Based on real usage data: +Costs are very affordable. The ranges below are estimates carried over from real usage with the previous default model (Kimi K2) — re-check against current [OpenRouter pricing](https://openrouter.ai/models) for GLM 5.3: **Typical Costs:** - Small PR (< 1000 lines): $0.01 - $0.02 @@ -198,7 +198,7 @@ The workflow fetches and sends these repository elements to the AI: 4. **Human Comments**: Comments from human reviewers on the PR, fetched across all pages (not just the first 30), newest first; bot comments are excluded, while human comments quoting a review header or marker are retained. Caps (`MAX_HUMAN_COMMENTS`, `MAX_HUMAN_COMMENT_LENGTH`, `MAX_HUMAN_COMMENTS_TOTAL`) clip the oldest first and mark any truncation. 5. **Labels**: All repository labels with descriptions and colors (kept complete on purpose; the prompt instructs the model to only apply genuinely useful ones) 6. **Previous AI Review**: Most recent bot-authored AI review comment only (limited to 10k chars), identified by its review header or `` marker -7. **CI/CD Status**: A one-line summary of GitHub Actions check runs ("N of M checks passed") plus only the non-passing runs — failures, skipped, cancelled, timed out, or still running — listed individually; fully green matrix shards no longer flood the prompt +7. **CI/CD Status**: A one-line summary of GitHub Actions check runs ("N of M checks passed") plus only the non-passing runs — failures, skipped, cancelled, timed out, or still running — listed individually (capped at 20 lines with a "+K more" line); fully green matrix shards no longer flood the prompt 8. **PR Metadata**: Pull request details, head SHA, repository information 9. **Files**: May include sensitive configuration files, keys, or credentials diff --git a/ai-reviewer.sh b/ai-reviewer.sh index d5cf736..b5d17ad 100644 --- a/ai-reviewer.sh +++ b/ai-reviewer.sh @@ -205,15 +205,26 @@ if [ "$INCLUDE_HUMAN_COMMENTS" = "true" ] && [ "$COMMENTS_JSON" != "[]" ] && [ " fi fi +# Fetch the PR object once, shared by the check-runs context (head SHA) and +# the PR-description context — both default-on, so a per-feature fetch would +# hit the same endpoint twice on every review. +PR_JSON="" +if { [ "$INCLUDE_CHECK_RUNS" = "true" ] || [ "$INCLUDE_PR_DESCRIPTION" = "true" ]; } && [ -n "$PR_NUMBER" ] && [ -n "$REPO_FULL_NAME" ] && [ -n "$GITHUB_TOKEN" ]; then + PR_JSON=$(gh api "repos/$REPO_FULL_NAME/pulls/$PR_NUMBER" 2>/dev/null || echo "") +fi + # Fetch GitHub Actions check runs status (if PR_NUMBER and REPO_FULL_NAME are set). # Successful checks are collapsed into a one-line count; every non-passing run # (failure, skipped, cancelled, timed out, still running) is listed # individually — green matrix shards must not flood the prompt, but skipped -# runs can matter, so they stay visible. +# runs can matter, so they stay visible. The non-passing list is capped at 20 +# lines with a "+K more" line, so a broadly red matrix (shared dependency +# failure, mass cancellation) cannot trade the green-shard flood for a +# red-shard flood exactly when the diff context is largest. CHECK_RUNS_STATUS="" -if [ "$INCLUDE_CHECK_RUNS" = "true" ] && [ -n "$PR_NUMBER" ] && [ -n "$REPO_FULL_NAME" ] && [ -n "$GITHUB_TOKEN" ]; then - # Get the head SHA of the PR - HEAD_SHA=$(gh api "repos/$REPO_FULL_NAME/pulls/$PR_NUMBER" --jq '.head.sha' 2>/dev/null || echo "") +if [ "$INCLUDE_CHECK_RUNS" = "true" ] && [ -n "$PR_JSON" ]; then + # Get the head SHA from the shared PR object + HEAD_SHA=$(echo "$PR_JSON" | jq -r '.head.sha // empty' 2>/dev/null) if [ -n "$HEAD_SHA" ]; then # Paginate (the endpoint returns 30 runs per page by default — big @@ -223,8 +234,9 @@ if [ "$INCLUDE_CHECK_RUNS" = "true" ] && [ -n "$PR_NUMBER" ] && [ -n "$REPO_FULL CHECK_RUNS_SUMMARY=$(echo "$CHECK_RUNS_JSON" | jq \ '{total: length, passed: [.[] | select(.conclusion == "success")] | length, - other: [.[] | select(.conclusion != "success") - | "- **\(.name)**: \(.status)\(if .conclusion then " (\(.conclusion))" else "" end)"]}' 2>/dev/null || echo "") + other: ([.[] | select(.conclusion != "success") + | "- **\(.name)**: \(.status)\(if .conclusion then " (\(.conclusion))" else "" end)"] + | if length > 20 then .[0:20] + ["+ \(length - 20) more non-passing run(s) not listed"] else . end)}' 2>/dev/null || echo "") if [ -n "$CHECK_RUNS_SUMMARY" ] && [ "$CHECK_RUNS_SUMMARY" != "null" ]; then TOTAL_CHECKS=$(echo "$CHECK_RUNS_SUMMARY" | jq -r '.total // 0') @@ -263,14 +275,14 @@ if [ "$INCLUDE_LABELS" = "true" ] && [ -n "$PR_NUMBER" ] && [ -n "$REPO_FULL_NAM fi fi -# Fetch PR title and description +# Fetch PR title and description (from the shared PR object) PR_DESCRIPTION="" -if [ "$INCLUDE_PR_DESCRIPTION" = "true" ] && [ -n "$PR_NUMBER" ] && [ -n "$REPO_FULL_NAME" ] && [ -n "$GITHUB_TOKEN" ]; then +if [ "$INCLUDE_PR_DESCRIPTION" = "true" ] && [ -n "$PR_JSON" ]; then if [ "$DEBUG_MODE" = "true" ]; then - echo "🔍 Fetching PR title and description..." >&2 + echo "🔍 Extracting PR title and description..." >&2 fi - PR_DESCRIPTION_FULL=$(gh api "repos/$REPO_FULL_NAME/pulls/$PR_NUMBER" \ - --jq '"**PR Title**: " + .title + "\n\n**Description**:\n" + (.body // "No description provided")' 2>/dev/null || echo "") + PR_DESCRIPTION_FULL=$(echo "$PR_JSON" | jq -r \ + '"**PR Title**: " + .title + "\n\n**Description**:\n" + (.body // "No description provided")' 2>/dev/null || echo "") PR_DESCRIPTION=$(printf '%s' "$PR_DESCRIPTION_FULL" | head -c 2000 | strip_partial_utf8) # Same truncation contract as every other budget: detect from the source # length and mark, so a cut-off description is never mistaken for the diff --git a/tests/test_reviewer_context.py b/tests/test_reviewer_context.py index 2086a66..7010635 100644 --- a/tests/test_reviewer_context.py +++ b/tests/test_reviewer_context.py @@ -7,6 +7,7 @@ import json import os from pathlib import Path +import re import subprocess import sys import tempfile @@ -103,10 +104,9 @@ def run_reviewer( sys.stdout.write((path / "pull-commits.json").read_text()) sys.exit(0) if parts == ["pulls", "123"]: - assert args[2] == "--jq" and len(args) == 4, args - result = subprocess.run(["jq", "-r", args[3]], - input=(path / "pr.json").read_text(), text=True) - sys.exit(result.returncode) + assert len(args) == 2, args + sys.stdout.write((path / "pr.json").read_text()) + sys.exit(0) if parts == ["commits", "abc", "check-runs"]: assert args[2:] == ["--paginate"], args document = json.loads((path / "check-runs.json").read_text()) @@ -549,6 +549,34 @@ def test_pr_description_clip_is_marked(self): self.assertIn("**PR Title**: A change", prompt) self.assertIn("[…truncated at 2000 bytes]", prompt) + def test_pr_object_fetched_once_for_description_and_check_runs(self): + request = self.run_reviewer( + previous=False, human=False, + check_runs=[{"name": "lint", "status": "completed", "conclusion": "success"}], + pr={"number": 123, "head": {"sha": "abc"}, "title": "T", "body": "B"}, + config={"INCLUDE_CHECK_RUNS": "true", "INCLUDE_PR_DESCRIPTION": "true"}, + ) + prompt = request["messages"][0]["content"] + self.assertIn("**PR Title**: T", prompt) + self.assertIn("All 1 checks passed.", prompt) + pulls_calls = [call for call in self.gh_calls if call[1].endswith("/pulls/123")] + self.assertEqual(len(pulls_calls), 1, pulls_calls) + + def test_non_passing_check_list_is_capped(self): + request = self.run_reviewer( + previous=False, human=False, + check_runs=[ + {"name": f"fail-{index}", "status": "completed", "conclusion": "failure"} + for index in range(25) + ], + config={"INCLUDE_CHECK_RUNS": "true"}, + ) + prompt = request["messages"][0]["content"] + self.assertIn("0 of 25 checks passed. Non-passing checks:", prompt) + self.assertIn("- **fail-19**:", prompt) + self.assertIn("+ 5 more non-passing run(s) not listed", prompt) + self.assertNotIn("- **fail-20**", prompt) + def test_human_comments_exactly_filling_budget_are_not_marked(self): # "**alice** (2026-09-12T10:00:00Z):\n" is 34 characters; a 16-char # body makes the block exactly 50 — no clip, so no marker. @@ -648,16 +676,22 @@ def test_commit_summary_reports_unavailable_line_stats(self): prompt, ) - def test_workflow_forwards_reviewer_configuration(self): - workflow = (Path(__file__).resolve().parents[1] - / ".github" / "workflows" / "ai-code-reviewer.yml").read_text() - for name in ( - "AI_MODEL", "MAX_DIFF_SIZE", "STRUCTURED_OUTPUT", - "MAX_SUMMARY_COMMITS", "MAX_COMMIT_MESSAGES", "INCLUDE_COMMIT_SUMMARY", - "MAX_HUMAN_COMMENTS", "MAX_HUMAN_COMMENT_LENGTH", - "MAX_HUMAN_COMMENTS_TOTAL", - ): - self.assertIn(name + ": ${{ vars." + name, workflow) + def test_workflow_forwards_every_configurable_script_knob(self): + root = Path(__file__).resolve().parents[1] + script = (root / "ai-reviewer.sh").read_text() + workflow = (root / ".github" / "workflows" / "ai-code-reviewer.yml").read_text() + # Every ${VAR:-default} knob in the script must have a forwarding + # line in the workflow, derived from the script itself so a new knob + # fails this test until it is forwarded. Per-run values that come + # from the event (not repository variables) are excluded. + knobs = sorted(set(re.findall(r"\$\{([A-Z][A-Z_]+):-", script))) + self.assertIn("AI_MODEL", knobs) + self.assertIn("MAX_HUMAN_COMMENTS_TOTAL", knobs) + not_repository_variables = {"REPO_FULL_NAME"} + for name in knobs: + if name in not_repository_variables: + continue + self.assertIn(name + ": ${{ vars." + name, workflow, name) if __name__ == "__main__": From aff45c5beaeb48483df3833b8e1206e5eb4cb484 Mon Sep 17 00:00:00 2001 From: LearningCircuit <185559241+LearningCircuit@users.noreply.github.com> Date: Mon, 14 Sep 2026 23:27:51 +0200 Subject: [PATCH 13/17] fix: reject partial pagination output; pin the UTF-8 guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-1 findings from an independent fresh-context review agent: - A mid-pagination gh failure (rate limit, transient 5xx) after valid pages was invisible: the pipeline's exit status is jq's, so the emitted prefix posed as the full list — the newest comments/commits silently vanished. Every paginated fetch (comments, commits, check runs, labels) now captures gh's exit separately from the merge and degrades to no data (with a stderr note) instead of a stale prefix. - The multibyte rationale was wrong for jq 1.7 (verified: --rawfile exits 0 and substitutes U+FFFD rather than rejecting), so nothing pinned strip_partial_utf8 — removing it kept tests green while the prompt silently gained replacement characters. Tests now assert the absence of U+FFFD at every clip site, and the comments state the actual behavior. Also adds coverage: partial-fetch failure paths, multibyte clips on the previous-review budget, per-comment codepoint slicing, the no-PR-fetch-when-unused invariant, and commit-message body indent. --- ai-reviewer.sh | 49 ++++++++++++++---- tests/test_reviewer_context.py | 92 ++++++++++++++++++++++++++++++---- 2 files changed, 122 insertions(+), 19 deletions(-) diff --git a/ai-reviewer.sh b/ai-reviewer.sh index b5d17ad..0fe33c9 100644 --- a/ai-reviewer.sh +++ b/ai-reviewer.sh @@ -140,10 +140,11 @@ def is_merge: ' # head -c cuts at a byte boundary, which can split a multibyte UTF-8 -# character and leave an invalid sequence at the end of a block — jq (which -# builds the request payload) rejects invalid UTF-8, so the whole review -# would fail over one clipped emoji. Strip a trailing incomplete sequence; -# complete characters are never touched. +# character and leave an invalid sequence at the end of a block. Older jq +# (which builds the request payload) rejects invalid UTF-8 outright; jq 1.7 +# silently substitutes U+FFFD, corrupting the clipped text. Either way the +# trailing incomplete sequence is pure damage — strip it; complete +# characters are never touched. strip_partial_utf8() { perl -pe 's/(?:[\xF0-\xF4][\x80-\xBF]{0,2}|[\xE0-\xEF][\x80-\xBF]?|[\xC2-\xDF])$//' } @@ -157,7 +158,17 @@ strip_partial_utf8() { # inside a gh api --jq filter — that would apply it per page). COMMENTS_JSON="[]" if { [ "$INCLUDE_PREVIOUS_REVIEWS" = "true" ] || [ "$INCLUDE_HUMAN_COMMENTS" = "true" ]; } && [ -n "$PR_NUMBER" ] && [ -n "$REPO_FULL_NAME" ] && [ -n "$GITHUB_TOKEN" ]; then - COMMENTS_JSON=$(gh api "repos/$REPO_FULL_NAME/issues/$PR_NUMBER/comments" --paginate 2>/dev/null | jq -s 'add // []' || echo "[]") + # Capture gh's exit separately from the merge: the pipeline's status + # would be jq's, so a mid-pagination failure (rate limit, transient + # 5xx) after valid pages would otherwise be silent and leave a stale + # prefix posing as the full list. Treat any fetch failure as no data + # rather than a quietly truncated context. + if COMMENTS_RAW=$(gh api "repos/$REPO_FULL_NAME/issues/$PR_NUMBER/comments" --paginate 2>/dev/null); then + COMMENTS_JSON=$(printf '%s' "$COMMENTS_RAW" | jq -s 'add // []') + else + echo "⚠️ Comment list fetch failed; continuing without comment context" >&2 + COMMENTS_JSON="[]" + fi fi # Previous AI review (only the most recent one) for context. Selected from the @@ -229,8 +240,14 @@ if [ "$INCLUDE_CHECK_RUNS" = "true" ] && [ -n "$PR_JSON" ]; then if [ -n "$HEAD_SHA" ]; then # Paginate (the endpoint returns 30 runs per page by default — big # matrix repos exceed that) and merge the pages locally; the summary - # must see the full list, not page one. - CHECK_RUNS_JSON=$(gh api "repos/$REPO_FULL_NAME/commits/$HEAD_SHA/check-runs" --paginate 2>/dev/null | jq -s 'map(.check_runs // []) | add // []' || echo "[]") + # must see the full list, not page one. A failed fetch yields no + # CI context rather than an undercounted summary. + if CHECK_RUNS_RAW=$(gh api "repos/$REPO_FULL_NAME/commits/$HEAD_SHA/check-runs" --paginate 2>/dev/null); then + CHECK_RUNS_JSON=$(printf '%s' "$CHECK_RUNS_RAW" | jq -s 'map(.check_runs // []) | add // []') + else + echo "⚠️ Check-run fetch failed; continuing without CI status" >&2 + CHECK_RUNS_JSON="[]" + fi CHECK_RUNS_SUMMARY=$(echo "$CHECK_RUNS_JSON" | jq \ '{total: length, passed: [.[] | select(.conclusion == "success")] | length, @@ -262,8 +279,12 @@ if [ "$INCLUDE_LABELS" = "true" ] && [ -n "$PR_NUMBER" ] && [ -n "$REPO_FULL_NAM if [ "$DEBUG_MODE" = "true" ]; then echo "🔍 Fetching available labels from repository..." >&2 fi - AVAILABLE_LABELS=$(gh api "repos/$REPO_FULL_NAME/labels" --paginate 2>/dev/null \ - --jq '.[] | "- **\(.name)**: \(.description // "No description") (color: #\(.color))"' || echo "") + AVAILABLE_LABELS="" + if LABELS_RAW=$(gh api "repos/$REPO_FULL_NAME/labels" --paginate 2>/dev/null); then + AVAILABLE_LABELS=$(printf '%s' "$LABELS_RAW" | jq -sr 'add | .[] | "- **\(.name)**: \(.description // "No description") (color: #\(.color))"') + else + echo "⚠️ Label fetch failed; continuing without label context" >&2 + fi if [ "$DEBUG_MODE" = "true" ]; then if [ -n "$AVAILABLE_LABELS" ]; then @@ -306,7 +327,15 @@ if { [ "$INCLUDE_COMMIT_MESSAGES" = "true" ] || [ "$INCLUDE_COMMIT_SUMMARY" = "t if [ "$DEBUG_MODE" = "true" ]; then echo "🔍 Fetching PR commits..." >&2 fi - COMMITS_JSON=$(gh api "repos/$REPO_FULL_NAME/pulls/$PR_NUMBER/commits" --paginate 2>/dev/null | jq -s 'add // []' || echo "[]") + # Same fetch-failure discipline as the comment list: gh's exit is + # checked separately from the page merge, so a mid-pagination failure + # yields no data instead of a stale prefix. + if COMMITS_RAW=$(gh api "repos/$REPO_FULL_NAME/pulls/$PR_NUMBER/commits" --paginate 2>/dev/null); then + COMMITS_JSON=$(printf '%s' "$COMMITS_RAW" | jq -s 'add // []') + else + echo "⚠️ Commit list fetch failed; continuing without commit history" >&2 + COMMITS_JSON="[]" + fi if [ "$DEBUG_MODE" = "true" ]; then echo "✅ Fetched $(echo "$COMMITS_JSON" | jq 'length') commit(s) from the PR" >&2 diff --git a/tests/test_reviewer_context.py b/tests/test_reviewer_context.py index 7010635..b1c0033 100644 --- a/tests/test_reviewer_context.py +++ b/tests/test_reviewer_context.py @@ -60,6 +60,8 @@ def run_reviewer( check_runs=None, labels=None, pr=None, + fail_comments=False, + fail_commits=False, ): with tempfile.TemporaryDirectory() as directory: path = Path(directory) @@ -71,6 +73,10 @@ def run_reviewer( (path / "pr.json").write_text(json.dumps( pr if pr is not None else {"number": 123, "head": {"sha": "abc"}} )) + if fail_comments: + (path / "fail-comments").write_text("") + if fail_commits: + (path / "fail-commits").write_text("") (path / "check-runs.json").write_text(json.dumps( {"total_count": len(check_runs or []), "check_runs": check_runs or []} )) @@ -98,9 +104,15 @@ def run_reviewer( # page size of 30, oldest first. for start in range(0, len(comments), 30): sys.stdout.write(json.dumps(comments[start:start + 30])) + # A mid-stream failure flag: pages already written, then gh exits 1 — + # the partial-payload scenario the fetch discipline must reject. + if (path / "fail-comments").exists(): + sys.exit(1) sys.exit(0) if parts == ["pulls", "123", "commits"]: assert args[2:] == ["--paginate"], args + if (path / "fail-commits").exists(): + sys.exit(1) sys.stdout.write((path / "pull-commits.json").read_text()) sys.exit(0) if parts == ["pulls", "123"]: @@ -117,10 +129,9 @@ def run_reviewer( )) sys.exit(0) if parts == ["labels"]: - assert args[2:4] == ["--paginate", "--jq"] and len(args) == 5, args - result = subprocess.run(["jq", "-r", args[4]], - input=(path / "labels.json").read_text(), text=True) - sys.exit(result.returncode) + assert args[2:] == ["--paginate"], args + sys.stdout.write((path / "labels.json").read_text()) + sys.exit(0) if len(parts) == 2 and parts[0] == "commits": assert args[2] == "--jq" and len(args) == 4, args stats = json.loads((path / "commit-stats.json").read_text()) @@ -525,11 +536,13 @@ def test_human_comments_total_budget_marks_truncation(self): self.assertIn("[…truncated at 50 bytes]", prompt) def test_multibyte_comment_survives_a_byte_boundary_clip(self): - # The budget lands mid-emoji: without stripping the partial UTF-8 - # sequence, jq rejects the prompt when building the request payload - # and the whole review fails. The header is 34 bytes, so a 39-byte - # budget keeps one complete 4-byte emoji and cuts one byte into the - # next. + # The budget lands mid-emoji. Without stripping the partial UTF-8 + # sequence, jq 1.7 substitutes U+FFFD (verified: it exits 0 and + # silently corrupts the clipped text; older jq rejects outright) — + # the assertNotIn below is what pins the guard, since the request + # otherwise builds fine. The header is 34 bytes, so a 39-byte + # budget keeps one complete 4-byte emoji and cuts one byte into + # the next. request = self.run_reviewer( [comment("😀" * 20, "alice", "User")], previous=False, config={"MAX_HUMAN_COMMENTS_TOTAL": "39"}, @@ -537,6 +550,67 @@ def test_multibyte_comment_survives_a_byte_boundary_clip(self): prompt = request["messages"][0]["content"] self.assertIn("😀", prompt) self.assertIn("[…truncated at 39 bytes]", prompt) + self.assertNotIn("\ufffd", prompt) + + def test_partial_comment_fetch_failure_drops_context(self): + # gh fails after emitting valid pages (mid-pagination rate limit): + # the emitted prefix must be discarded, not posing as the full list. + request = self.run_reviewer( + [comment("latest human feedback", "alice", "User")], + previous=False, fail_comments=True, + ) + prompt = request["messages"][0]["content"] + self.assertNotIn("Human Comments on this PR", prompt) + self.assertNotIn("latest human feedback", prompt) + + def test_commits_fetch_failure_skips_history_context(self): + commits, stats = self.commit_fixture() + request = self.run_reviewer( + previous=False, human=False, pull_commits=commits, commit_stats=stats, + fail_commits=True, + config={"INCLUDE_COMMIT_MESSAGES": "true", "INCLUDE_COMMIT_SUMMARY": "true"}, + ) + prompt = request["messages"][0]["content"] + self.assertNotIn("Commit Summary:", prompt) + self.assertNotIn("Commit History", prompt) + + def test_previous_review_multibyte_clip_is_clean(self): + # "### Previous AI Review (ts):\n" + the marker line + 9900 r's put + # the 10000-byte cut 21 bytes into the emoji run: five complete + # emoji survive, the sixth is cut mid-sequence and stripped. + sticky = comment(f"{MARKER}\n" + "r" * 9900 + "😀" * 10 + "\n\n✅ Approved") + request = self.run_reviewer([sticky], human=False) + prompt = request["messages"][0]["content"] + self.assertIn("Previous AI Review (for context", prompt) + self.assertIn("[…truncated at 10000 bytes]", prompt) + self.assertIn("😀", prompt) + self.assertNotIn("\ufffd", prompt) + + def test_per_comment_clip_slices_by_character_not_byte(self): + # jq slices by codepoints: a mixed multibyte body clips at a + # character boundary, never mid-sequence. + request = self.run_reviewer( + [comment("αβγ😀ϵζη", "alice", "User")], previous=False, + config={"MAX_HUMAN_COMMENT_LENGTH": "5"}, + ) + prompt = request["messages"][0]["content"] + self.assertIn("αβγ😀ϵ […truncated]", prompt) + self.assertNotIn("ζη", prompt) + self.assertNotIn("\ufffd", prompt) + + def test_no_pr_fetch_when_no_feature_needs_it(self): + self.run_reviewer(previous=False, human=False) + pulls_calls = [call for call in self.gh_calls if call[1].endswith("/pulls/123")] + self.assertEqual(pulls_calls, []) + + def test_commit_message_body_is_indented_under_subject(self): + commits = [pull_commit("c1", "subject line\n\nbody paragraph", login="alice")] + request = self.run_reviewer( + previous=False, human=False, pull_commits=commits, + config={"INCLUDE_COMMIT_MESSAGES": "true"}, + ) + prompt = request["messages"][0]["content"] + self.assertIn("- subject line\n body paragraph", prompt) def test_pr_description_clip_is_marked(self): request = self.run_reviewer( From 5f6e842430e57862265dbcdb4cfdc6cafa667f8e Mon Sep 17 00:00:00 2001 From: LearningCircuit <185559241+LearningCircuit@users.noreply.github.com> Date: Mon, 14 Sep 2026 23:38:35 +0200 Subject: [PATCH 14/17] fix: warn on PR-object fetch failure; correct byte wording Round-2 findings from a fresh review agent: the PR-object fetch was the only context fetch without a failure warning (operators got no trace when CI status and PR description silently vanished), and the README said the previous-review clip was 10k chars where the code clips 10000 bytes. --- README.md | 2 +- ai-reviewer.sh | 5 ++++- .../test_reviewer_context.cpython-312.pyc | Bin 0 -> 49111 bytes 3 files changed, 5 insertions(+), 2 deletions(-) create mode 100644 tests/__pycache__/test_reviewer_context.cpython-312.pyc diff --git a/README.md b/README.md index fa3aeb9..2878578 100644 --- a/README.md +++ b/README.md @@ -197,7 +197,7 @@ The workflow fetches and sends these repository elements to the AI: 3. **Commit Messages**: Up to `MAX_COMMIT_MESSAGES` most recent commit messages (default 3, excluding merges), plus a compact overview stating how many commits are already on the PR, the per-author commit counts, and each author's added/removed line totals (covering up to `MAX_SUMMARY_COMMITS` most recent commits, default 15) 4. **Human Comments**: Comments from human reviewers on the PR, fetched across all pages (not just the first 30), newest first; bot comments are excluded, while human comments quoting a review header or marker are retained. Caps (`MAX_HUMAN_COMMENTS`, `MAX_HUMAN_COMMENT_LENGTH`, `MAX_HUMAN_COMMENTS_TOTAL`) clip the oldest first and mark any truncation. 5. **Labels**: All repository labels with descriptions and colors (kept complete on purpose; the prompt instructs the model to only apply genuinely useful ones) -6. **Previous AI Review**: Most recent bot-authored AI review comment only (limited to 10k chars), identified by its review header or `` marker +6. **Previous AI Review**: Most recent bot-authored AI review comment only (limited to 10,000 bytes, marked when truncated), identified by its review header or `` marker 7. **CI/CD Status**: A one-line summary of GitHub Actions check runs ("N of M checks passed") plus only the non-passing runs — failures, skipped, cancelled, timed out, or still running — listed individually (capped at 20 lines with a "+K more" line); fully green matrix shards no longer flood the prompt 8. **PR Metadata**: Pull request details, head SHA, repository information 9. **Files**: May include sensitive configuration files, keys, or credentials diff --git a/ai-reviewer.sh b/ai-reviewer.sh index 0fe33c9..75d035c 100644 --- a/ai-reviewer.sh +++ b/ai-reviewer.sh @@ -221,7 +221,10 @@ fi # hit the same endpoint twice on every review. PR_JSON="" if { [ "$INCLUDE_CHECK_RUNS" = "true" ] || [ "$INCLUDE_PR_DESCRIPTION" = "true" ]; } && [ -n "$PR_NUMBER" ] && [ -n "$REPO_FULL_NAME" ] && [ -n "$GITHUB_TOKEN" ]; then - PR_JSON=$(gh api "repos/$REPO_FULL_NAME/pulls/$PR_NUMBER" 2>/dev/null || echo "") + if ! PR_JSON=$(gh api "repos/$REPO_FULL_NAME/pulls/$PR_NUMBER" 2>/dev/null); then + echo "⚠️ PR object fetch failed; continuing without PR description and CI status" >&2 + PR_JSON="" + fi fi # Fetch GitHub Actions check runs status (if PR_NUMBER and REPO_FULL_NAME are set). diff --git a/tests/__pycache__/test_reviewer_context.cpython-312.pyc b/tests/__pycache__/test_reviewer_context.cpython-312.pyc new file mode 100644 index 0000000000000000000000000000000000000000..389a98d319fb8c8bd90bc2726cc89c2e62f95d43 GIT binary patch literal 49111 zcmeIb33wY6xgL-T5?|*&e{8nhe^Rb$av7j`K}6H04gm z+S%Ft{a+m@fCY+@dv^1E>yr3T$E#PbUj484zuuqb{@v*O_`cuNX#O`kkT#>Z z^Qc~{(Y&SMG(#Fr%jqVxLt6H$8`9xdKcSyA3>mb-HRF(xT{8`t*l*TQ7W*|1nc1&p z$fDDD4PNUzXp^?_QsGxwz9SxtmnqK*`<$U1JDf3+tMY zi`V|P=Bs+n%4LsfIolomP(D|{<$O^)RKOK-x%e&AX^v{Ryf11vyVo+_u>J|}=!J7b z#oiJw{|ajG1?aP7yhWw1cuK38)=lf2MUmg{yYA&jd_k`xblL0Ry`BjaSAE`VULHHI zhP}a%!~TP|7;)}LZx7MI2iJUupex(TF-|4ta`U&YC3c| z;O}&}CLLkFFNDh>2j>fp1g_#HSI|Kxf)3YJ>p;Na_lB+o_$$%_gN^|2m<({<2?w4z z5p;NXuVd1~d0VU>QvU~>+Jxb>Cv^Ek`X-NaJwBJz=a%5*NPbzx)hA3m zd$&ZvsK+;YHENNWdTRUJtcQO((K2@{?YFh%ZK)Tk#jHKyg#^fxT#@V(WZ)+}^oT()>}D z!Dd?37>uSzxT+bECYSqieonz}Q*_68hj}PsHMy z0!;fuuJ+_^hcH|Qzh}~$$O`+f_ygDc_!K5j7?{K-vL?O3pl8hMH1I@b5?MkMLB5iX zOf(+85MO`;r-{|ztFX@+<%t9(jFVn|%*$6}KS(c+FM*9GKuBayg(oI3zN`%g&g{Tn z@KqEyHGlT}soTccy4m7cU#zqtmgk)Bi;hIkFF0eJy|Jdgcy52}#aCh;F7`{_nAacs zTp)H~Dt7+Wn@2y$D_AM4SlQkb+rInu?%BS0LG8le%9i>$?!E2L&3jQqr(@MU560pJ z7e2BW=}Q@nCcXqu9l@Wx5g5_-Xb+?7UJdBEW=Lnza2=r0pztaG^qihE+{xyQU(^rj zInx(4Lk2DjrI9nEG;tP`S)3K6naf6L;cO_apt+i%Y*1Xykd3GE|-rok1IfF z=L%8gb44f%xMGxrTnWk|t`uc4SBA2LD@R$%RiG^6Dp8hmRVXXCYLu1S7L--oR+QD8 z1LYR32IW?+7Nvu$Ls`Ssqpan&p{(P!qpat4pxnkapxn+mQSRUxQ8sW*D4kq0%0|wG zvWaU!+03<~ba8DcTexNg?6ymDBPS%I^h}P#qHpBR0F{a1 zj%O)u6L&)v>D)LIC8Q(acSb9|FlF zBQTREJsV#QV&fg%nAWEP$6OIt%$3~H-LTKt$MdDWC4VKn>eXC&PNR7Rn0F)pM!`(( zOg{DeM&V4M8a{EQA!0gg8z*Lxwt=CR%@jb$HAuY#WE852(Oh}5#_NX-z`M6KAz7pz zCkCIkrCcX=m$s2;9U|#~Z8N!2`&d}~vtKn+7+LVTjWD+)={YE#5{_IP^}aFm5y3*)QPw8jpD^w{R57RwZ32wDigd* zZF@&2{YZdPdr9bk^5r|a+1p>NbzwS)={DxiwU@+UV4W4FxYQNa*NK3K3pU6vEj7(v z8t;q`#<^*J+~9Pwm#uU3O@=4Xs$=Z3149f|hxD9oN5JoOJb&)g0N#@KOgpBamBB@C zv#8?P5p;0gQBQaxBsN7C9YJ5j>j;cGI@J&zT-jLuT7~-HwYFYP9qc^Y?P1E-lHYb6{Zj+SO}=0%q7d{5 z*a>0s;uU82NS*AL?*;{G33}CC((lg$nf+SeMH+{%S3|F9lS0DnXTWB$)t# z!JR@(Q0uhFG+0M3wqGLh%H)b>v9Ij{Jb=xEUgJSY_S}MR)NrvDT?Amec$AFoc3!&F zEPp29tEZMe|5P}1K*1r|^Das~xCG!LPIF+84|~(dm(UAvqfnS<`dz10hJex9u-|)q z3KY}JNlb}Ya<}6(>emf0Ur!x$KcWc1M2!u_3qh2qm9+B2_IYNPXz}=3!SY(KwzrO4 z_JmsT9j7L|q-77*vhfoo5=k1NE}{!pFS-<5ne&7^u3<1{ewxPbITsLblk$m7jYK1g zgyxLR30zu)VV|FIZerR~yw4v>GWZm+Egdc(_UXnhCk!KDej=i)t9h)o`u-Y27bzU| zT=BY+BjLK@osQ(QlY-I06$)JO`V)FcC=r|Yx`zg)mG@2s62?)_M9}NZfpw*4@K_>S zA#)`PPMz)>ID6{+VBcAH&*|gt7y4dK*p40_Ja+!Dd+^i?eFF*0>9g*E^Cu7YolWGP z?K^$S-GBbX3HLzH$-YF{@qr^J&iD4YPoM3(aQxKyb8hIniCpR2iJrrKClvSM!M%Owj+{MydhqzEfkdfPk6JrE z=swwZ?p)8&KE(~&6ogMG7K3W*p1pb-{>f0B#e^7(>m&#@WRmG7?$>|O~U8d zAr9FNr*-f{(jYpG2_wT7m`i{YiQEy-6ky-$1|R~95_-t6iL8)s(u)&3&2S^RsRC=n zri2n_eZm~_PEJt|5(R_U3-ExzULOy<3GmZAO~Dh^BqSvags2H4lVuZn(4mCw$ca7- zr!dOs9@-1S^e_!ot@pZjBpf1JQ^GiM8D_SGE`YfJ!FA9(mB@nFNBgK9;>ivKOJKqj zo&qj-6V@bUN$5dZ5+-5P6WIcZ>U%ZpnMhcZxSudVVDR#+AqcPR5TlSV3Gz_FA`7i( zo5XCAtbz&i*kv~p)e||Q7$_b}YG>hMw!jj_14jA9dU_xd*{1>%&I(L#yc44dv)EFQ zClw4|PX#Gic+#)%Wbfcflfvdtd?W2OQ9-&9-i0Dz;iNA>rZ|j{St4Z#V=x3Ws9 z1p{Gz#7mQ^@NgnqoWPJQKqfn)&`_6?S<}rvuu^3%h1lc|osZmq#6AO^5koG0`7XYA}nvWft+>+&-hImfHVou|#=7!dCL_6<$zp!-H z^4o`Q^{(XP-#!)3sav+S#%-vcJ~t0qlufwJQFdcXhtcb{LFdGFAf2mIpBvrD#f^qAxE zoa$wpGj4OY;2(%7q4@{4El z;`uw4?VT|6MGr68cmG*lC7sQU=Wk!Ocf{=-(T*kiF4oa)@tkeTw)VKK{d@J_Z~SiK z`;NM~=ijT}8LeG(blo3%ukU>9-1)fu0v^&SJcM=p_Q`lo&9coEx4G_d?~dOcUnu#P zRrfm?5nw*rH0kZ+N^TJM_055igR!EtDF}wYn1>dRodc~aW-r0AD?hehLh?VVqe{1dB&bOymw(nbU zbgop_uGF@@-`pBCzjJcA`B1$1&|-7X`;D%7?>mjljotCa?#0IaE8ATwwa%5s_LZg% zlpj^*R_5M%VRefpr)=4_C2rd?d-Zo_<_nf=&F>d)ed}{?er_ed^iJWO3vXA(?AumK zURWtEpRbR;@Sx!1EJI1|t^QT3CMSQ{Ru#8Z&0bipZi!d7M77af-#_r(1537^6`NhS zc4zQwLth!1H@%y4H)pYW*MgZYmRH9dZSnH9ThFiT>s#J;BEIj$(!PP)&o37>#ETjh ziyD`Uy5dD$OGUe5gBNb~t#lvyan*Ctda*6T!)n)iRnOh(ef{LB86&h@>55mn7Asq$ z^~)Vc;vGj8JNn|4eJj;9%hj%UwQI4u^&`Eus%@oX*K)_xG{Jp~H#l`lXCEMZTTj1()O=rBObFroiuki!@4|?yHz1MU0!TH7g=N9)2 zF4@il4d3d1vwJo)=Z|mQxxD32e9NK5Ej^2chkyQ>o}ubT0>*x_Hm=uH_h^59MK~=Y z^5Y<>z5nrv?Pv7*f4{Hl^g+}AQhKBs2eE@jlySZBj4f*f@`AjPnwU7tqthsqCb$`R z%D|*T*RVlZsuh)`*-)0k`U(rTo(#-aH9YpA6wP5uQLU^@(_VvDL8MXe9dST-5ZyY+ zrNco=1CcW0SFrE%el!;;Y8A98tq@d5ej^eZe6j{ok4v#CvbIB!-+9W9nN4boC}hJYk`A-bfguBx>9AR zO_5!hhnmzJ^!|`>E<+;$al$bQ#|?-p!4@{`kC`$vC=WU&#ca$Y7WNHr*cuVcy=-pG z|0-UO|A#12Mt=$?GO#1z=>ngAK=%>6tsi`YIQX@5+g z_1}v;JtDH3lhQw+MImFwP%Fddl5nN)cF&itE$3~G=WU&{E#v)@bn4izfj!4PTQ_&YFBQ{@vxvRVcG1OGaD zyndK@oXs%I3h8wjwSr_NT8|ZK!)#f~Yg8XqMwqJGG7M@I;7lH1<4P`or_x4#HTQIq zVT27Sqw$hABca0$9qv^_3Db;xCo>bO)yY!U;mj~O)2GvCXwfpGCtu00>XmoBj!&1N zzExQ-L!InR)Uj=%PR=IkWEyTnQ^n8HB;hZd^7iCZ8Q5 zcOkaT7`Za4`Hl#~$txkdSVg{;e4cu%Xset(yMmRKRE}x5s&8nYc&C=DX7_B_M9rat8%m{H^5_-FZ-?vrV&&StV5#NC3&BC0yhUE2L)Q{gJSt3a<$T{M2@f*Cn-pLIR~Ul%?k4 z3$a*m$;PX4KHO;UnYiYe4q}K#Cc^llp2!vqC`@AHPz~Jy#5gZE>>0Tte#U8jAHjm| z6~9v-wqaN4O&qRCJ0bWbL&3zvZ0LIEvxf_*)>mJLTp&1t3vd!l>g z0p$*2bS6E}@d&?gA$&EAq2&@#Y(gIi+nP7Xlg2Dz@WSE^FJYoX2?Grl+=TJj`H%-* z$JR;DbvIK|27Q{xsF6+TFuWy+sbjzM725vr5d2vI*%J{PK)}~JHZkeyYUy-Z_%BeK zWOXY~ip)e7)SeR_Ja?3C(g(dE{w3_eftDr}beX+) z!h{z9MkMqh&sZW${3QH8L7jD;wEM|%nLL%rsow4LLyHy`u*eM`OIW_IdPQxFoAXYE zF1ulJz?DEKFcO%EoKc;ye3~ZzBX#mWfIQyRtQ3?i7u3ZI>gLZcY>O4tEfzd?D{Cdc zY_=kvUq5#S`jmM~Jip_XX~k@NBj-ywv68(DyOzv{<-@-FSC`BKD}_7PG+Il`ts|Jx zzH#zPCue(RM`q8@)yy`|jXt!wR`s~}fz-sA*twUN%tLY;7h*4cZprMHkHt2n>K9{| zMwZN+d^!F0Q?WBIE}37#^<{He+*}qbZ<@cbWZtPbk<#fekIs%RnYW4UeR=dP|C@eH z@wygE+M{^4?p2+x%!+B8rRmmDXnk_>*$eHM+x5`a2(^i&2}*&2@>qFeJilrFWIVrn zVNX22mkp87fjfn>7v`RiSGO*fw|%efdlQQtN0!XJa(8A+=4|nDSFGi5y!^=h{&@Kd ziSk_^V}$l#goGAmOXsTM7VS9K8LRK+!g2d(Tgm^?>e0h~da{Ex|AauEJ>oRKjo6DzZ* zWCEOu&n;($M^DnXOFu6A;7WZ2<2Q2_sSfi!G?0_Ms>dO`E2WxDz*G@T5Y~`dN2OP& zV@zpBMbwzlkITYFvad-m;bfo5L&#|zh%{LdeGiLQN1!!n2?aVd~%y zJIT{qfe*bTfRklMyM720?wo)Fx~a=rkuxuxY%| zca{-C79PQW4Glp@P5O0yjv8g;&dB_QL|P{VY*!t! zBT_<4jYQylK{qivB7CglZjVp8KXODEA}^mZ*?hW2{!jEY_|H+?)VyC%b{oW?KbCj& zmO&^n z(8a~ZSK{U!w{l{oJ49j_hn9Ex~e^0lh3RLyaVAX#S~ znp@XnFtJ01!5?Es4TDo~8euA#reI0@O+u>ToQ%Z^eAdq(aDrj77_i3hnZ$Q>%t^&A z6?~pa$5NGcIFk}ffj?OTkzDI3NS89IMDndCU=)F?jeIH{F)9Qkf@sH7gb=O-$@lcx zZl{s|I>u6g3v+Zx2!TtC$?M_BL_rJm3|>ZnAWgrRy+tsz2;j}2S*Yrs@Q!&#rrpDV zkOJIA>5SV5yf@tlB8fCXaux)5=a=3#&*sH)8*b^}FRHk`Csyf-<+p$jNjx2(e(rZ) z`QGk@U4L*WR)1u%pf@9oi{MzO zM}I?qLk4clJq?_KIdNtN(!^9$7zSr~GBe<;DRs1%eenoKM+1Kx1wjh?H4-9E=V-5JQKMNz7WvL)6Phvd)A9p= z_R8DF+o6wP8z{QnJKI0kHMb*H(Kz4#(AN1#j|z`8`oip6Sxl15ENl1NhdlbcEbGO} zyBGSG%tuy=DoEPi34^R;U7Ee{wO79K%6vVnOVNBh1m-8p&QBiY;Zcli{a(f49o7_M z1d(Tkv%471as;`zMBLIcCW7f1ri5%&~MX-WPs3e43(%d;xVm&T_F6Y2yQT$6oO#$Ej1#WPf5?OGu7$jCC<6Ih| zNdg-Qt2{mBNth+DF5*)@)|t?>8-f`JH3hk0xre7&aIQPRyIIihMyzYojZ_jo{WEk9 zvrfmrnkLI~R14egt-jlPVWoeo>dmUTzUa%hs}}PQeWWqv9M!&G;g~Ip)$ECt?uBK$ zV#{(xQ@o<--iy(}g|7R~Mc0W~^U1}Ef!kRt<&L?EczN^u8JZqh;^p19O=RoNc{690 zo9kb)H;G5FEr;%xEZL9YaM|vN+a0l*0}EG|?EOE_p$D$A$9|-zp8Z^xJo7k6%91-h z4TtlLf1GDLTxLJ_w~~((!8tS9b%Lf zTGb47%xQIG9xU}4Q!z~LlDr9GnPnhSblwjw8R)(M$m9M|&iE-G%oM?SG33ns7+5;exdF9AgI1z#-_HU`|j=g%HpVw<>7n1oBTHD^D$G zis8K`PcP1A<_w>r*=+>Y6r`h2ro3+}j1@I5+8D6DbBP_+&yCGrU9|05CI0Xv@-GQo ztx-%m_N-?Bl4@l8yh(M|AkX{%Ejp$)YXBg~0EsH%rkD>Tfm8;n^m#vLyl0X?g3bNe zd|wc$ILk(Je=q>$++Uq|>y&kbd32Sv*B2x=HF)IG#}y~O!go#synd%v(21u`nIYn* zQPDZPya0d(0RTyEQ$QgJJtXxDVA#jN5LqkG4H8GBeu#1jh-knYWbioQ^;~fSTqmLN z3k5{*6x|sjLz!Xm)3)7807I-@Txj4oF8t?G5# ztREDWuNpOl^^Y{V!b->{CTO|TWD{|&P&9jW$-G@L@%{f5C>B~zKM?}&Y6X|VCqW=l zKS5xAfIa4?kOa7nG=O75tVGk?SG?Y-U^);+j&26JHqj6zS4agVS1hk(GoX}TB7;)d z2B1VL<=fmWgxWd8gZ9&s-_e~kvxYnPei>M@4yEt1KTq}*ML+U zq-DILyJvleFdZ8cepOf?w42#X8)B2F9@8|c2&Y_%0T{F^cOZNbV`;Uz9F2{V*{rdV zVvS&+VqV4F?H%0+aSH~?!UX-FU~Zd$Lk{AmU^fUlj?B&6qba1H0#W2 z-FV6ok6*UPO|#o4ePfs(!hl3(G~f+pZ9bOShJpkIA8Q9(&OOLE1PY1xu}OyqVJ_GF zxDAFa=3p#D_YqyfnayVe;bVbG_i&hlD;pDRz+RYrh>N_C8K6j#h`i#DumYrelbUigBD@cKe4o7Y-$L_VQr%7F1?{^&#V&hwm_0h z9{nFEfaB<%ReD(1=+`;<{YI*-f(@;+q%5YBJCV+6H%| zLg#6VY&}Uyc}UDTh8TpCo~droi0d$Dla2*AlrV4}zX!~PrhB-fJ(5lSdQ=N>mrV4= z*$%`Av$HU22mJ8p_aL+b?Q~<(IZO>kjEAvv80`>I!}Xn!3if2jFak9Z*SO>Q4rr2K z;U?|MpfriyDpc;09tx8<&mnBU!v`kF=^b%RADU3-LsI$O)g>%2#`~D<7~b+;G2Vth zK{bk~ppX3_$(SD!$_gqbBma>0SE%?iDiT!uISL4M4*C%hjir1dcf|h{CsXk(mnI0l z5IpLylZ`l1r;0zRmCh4hkfsZGVTRxGtbt~XSC?!9@7qde4YQNc-AlH8!oELxbjh|~ z+LzDAwsha$5j%5!$#y|`&uYGf;YEq2sB*q$p(b9nKUQ!6sgUMt7uw@h2Vw;W*Bzu( zlMg=1HsxDy8G)Pj(plrJUyhlp*JF{)Dd+GNGEGToqDC9DtO+U~`WR^&xqT6Y5=ZrE z0sG>70E!;HinWid1M-X^*%YjOsn!9tI+jh;v2LPH_9p7sHc=-ht&W;ZU{F;&$yr5a z)UV6gSyChk74?+#ji3TIiZ0fep4>3P?aRpJ6XIUg2DPteOGi+8=NmLPOiXP?^dW6a zIz!EvBnK(3Af*@iX}91oL^>t5QM#)cQ?HzFNooVFi7I|36G2IzzHvp$(G(Tix*JBQ zwTlP(`R7p|!BU7FylYf2;TJ7?FI+ zU7DhTNpvsLbuo&qpQ0cWdXIma33_+41XaO@RGD~BQeVKr4iLFUjL=Pk79_Gk^EqsS z4x#yJh9=VUsYlmGLwOtpR8-~F%jM0`l`fXId@1)<*6m{1TFAsXs8Z*{D@D!oqp`xS z)hwnBFfh(#&7cjSDcp+f?24`6VvpabLV~-(ur|9~TeJ z)!#3VoxKoy@um3Lm*4Ao8Pm)5-yVw<58bk&kY}0j(HVMs>wL|Uxmj@zuoZoNsp`;T z`Jww2OXd@@Lr$#x*n{m4&1XJYvuH{#YMDa#Pqx|mvJHRIk%ck^;ba+h0b~9Ph4?VU*;4 ziVRaJT*(fu^k0YsXxkxf5`W^4(_tY^A;mCZ7My&1@ErfwbUBSXZ4>zl&C$Vj0xUHM z(ucr5b2Q0lc$(-$YzpGu{W#afB5=CNAr-Rw;~eQ&knsP89%euRGL%Zr7;#D?Vf^2r z=6WKuSB(fw_()`iDTtH?2%IP5kUKEy7CnkVtZ_XuuxX04srG)2$PJtYDL?fR0rRVEL!Tj#eg zncG(yTRsK7d1!9_1d$mM4f*43wj-s6KkmpvnLf|b=PRvc7hRWoD%Ggt=3_E%fm69QMh=|CPfK9t2L?{$fdj9`L9sa+ec$_E7Mdz3U|8zGm6Ac)PHzpeD^unp6 z7b*0B=mpc-jfHvAT0{;$kv?pA{~e+SkMI(kqz5alT@Oa$g=aS{uEa`GJdAG#=0F!( zWV(<6?Iqvb7`$zFEtxxHxTSD;+2%94#Y#R`k2?75X z;g}k~-@duoXvBKj%e#arkT7HW619kV8GId|Fs(4CPbhXOaZ{RWMZA0fR!OATfH{&F zy6>uwLoN>BDw`-rjz1AiH`~<9G? z%k^FH`mV+L-9OITGuw-72xopfE0(wCmH~xa9n${b+vRgTN`s|yerHsBclSec=O>64 zw(n)`XFqJQ9d@N6ofYrJI5ABRH_}KW1$+j7dgM>0IBhLuQ5LLCK4jyi1sI+AmRC#7jRcFM{SID$CnDPN$1 zCh2!Y!~BPI;#*XF8%3I(RIsb0=^MzHhIEItOaK%QOy7WfMiUehB`P5{va39GWB;1) z;3oP8(|aNEUBq_my>DJBJRU1}9-g<7Tdp+Gj+z!qW2eu=e#sL*JRI9I5^Lq2dgdJ~ zNt;&hTe$Gh+`C>QOeB**@i#VL9i;0OET?2Q1++q5WP4fIGqXJl_N;6V_7lpWJ!a%! zF##OgC3^{TP-41Cu7JUDA(hOkN0}(q(S8>lq|?uInbXlKsOdjSOA8Urwl=Ja0DEo46DEp8d6G~L@rlI z9)#GkiEt3cp@^N9R&fzS2s#=%TlY94x!w-13o&ieh%+3!97<$~k%$p%*yqC16FleY zK+{)zQ&V0pVncSpD_~H=VIQ2}v*A~ZNVGASA%kFm^i;kC5XAS>9uWf|XC3X=Q{kjS zO;hXa5GTE4q7K5Ki!5zii(??*M`lgI=8XtJTt zCDN*iZ{&<@rE1F8qKb+ad(153G}w)eBvPYIc?|i#z$-yy{sO%j5hG!WEh`B5SLxjU zgd)}av`1jc^iFII1h>FTk;Msn>?6WWNYH;GHqehyNpTz^CpTcrPv0C(AEn843IY0{ zeCzza`@XpS^h$YC^tIU8i*fraE9K7U#n|bW(|2gi{EX{CHe^g!I+vzl!x z%Z1&k*g3yFR@lB;$h3o2(XOTLDv+{M~H}Q5k4-X>l zqBW|9Ws*6Gv^;u%B9l@s69I%H2^cqsoF#?GSs8f~okBJwaw!fW8D!D)i~wrHDJ3HS z7&uB9sKf!PD*jTFR;WGgU}uflCmfu*6qdYErZ zhk3AC2G(#XMkMQmX9{jCh~h;An3w7OBYm5K^(Qs+AJMj#2kFZPt9-$3N12783Xe;p!xF8XKQF&trl3wGjx*c`mn@8FN z21fN&-!gfJ@V3soIt3c8$3VfaUwAKNpm~zhVDQKcCrA_<`SowQk^e&2M~+eJv2RPP zwfDjGxcwzc$kx&qJN;7J{xT2{T3aCCwmBG?T&tT$z$5oBz$ODuAh41C2bdC`l1)ZO zA3%M3TD~gSYu00<2|ZQ5Aq9V9hHEB{|HrU218{HS3c|F3WXcuMX83OJq<&NA=+t90pGRD2sn z+8j?{Fp}e{5ZcLHVBJEe-IU1+f}JqiL(bKW7|c^N^Ir+Xe;xg~srgHC?HAmpZGFtK zfA>Q1ebb`tD8V&3$eCpaxw&J}@JjP@_v>O!{i}M$XS9XKw70T8*eJn5ny@6NE)cj* ztbES`43RzS0h<7U!0yrifC5Wh3fc(tJbg=sm;hv9Rg^(nOS3&%pzdClbdCWq3z8H( z`UPOlsFTQ60v?!+8gSZ_bpV_>1BoryH{6oY*`Fv|2X*cfeFI04npoG~#y8=0(*&F_ zF_&cXj*CCN`E9tF_$jR_cPWvbymTE?nJ=JnT5uIl0%IB=3UoFFf)M=cA4BdAK%E;Y zbFM(p5uQSmvJE2JH{FOCCWYWQ`T{zeSE_WvMvUR$?C!b##hj*?tr;*^)Dkt2{a*0q zmpn2zlF&P%&|vSAh3wffWVJaCc*>?K2pmZQxT#sS%N&#d^5Wc%`CW@S9Wh(yg7>KbGMU6c5E`GF(0v1a zdNx9b=r|g5Y)m7%J~}QFkxNfiXcSsemZ@Q#0K@EoNU@NGLQD@JNqP!EA|LKLv?3%h zPxDqOKgQC|91*@d)?1b{W=K*KgZb)B08O=c;kl=vG- zW>3*f6M@Yr`UJ2sDZwUI+&n)Py}FolFlIXhWx#y>rnsRbWbKTOE}0L={4fT!5~QxZ z>&coG&!(^Zh|FJ^;6&2(Zr;Mna^t(Z~n6el!=YJAh5lus|z-F4+MQ}xPkth2gM8_WVVSWoQ1mq!$7rwM9qoxFB zg6Dt&&N9Wm5e;B)mWvTqPFt06ZD(yM{omYV5N~t$WQupxPz3Dk%IGs62l>||o_WH(CY z#bF^ZA{oU^@}^iP0ZIub6teVcssU4Sa(IOIl(97wkl6o*kT**O+-9&?twg#h?Vz}5 zDXp_$gcklbIw81n{08=*f@2iPPiOxT6{B=kOu+GZ>?g8W-gqG;f2w6ua0G-)(G)Lo zQp%D`K1SlI0Z_(asTw!*;Tt#tZsz`%2^!9!H(wH$pqdV3rv}sT9trO;gd7=|pNnkLbM zXKAsBkR;<-1d1U@SF)=(-79&LLo(ysm^XrCh)>vvVmwttpCcH#gzg9~H=)?Jz6UQZ z6`qe3Tu67hiH@xlx#q9N3Xy|_DPuRI7|Wz>m5qA;6wls>EC~KI@b+R+%6AOp{{;L#IWOHvOmLdEaVaK0NcLnTZbc=wz%bv2y@1-)pKYIw zz-J@y*$8|#0-ue*XCv_02z)jIpN+t0Bk=cr1fWQ4C%K-Ma7`FPSQ^JGWPUwHXUW|V zGyOW+ucxAk3Kgw=q@=O&^jXIs-A}G)(%c|ig&WqXJ3AEG&h0cKTyG7kWmawI2NW)S z%wR|pLNaK>&wO`gGxmlzj?skg3ObhVZnjcUJ#U;3&;9aBdENXF5+K|6ejp_>u(0`_ zkvVzVHgxiAiNdLDHhvowtn0=nd2t0ZE$2>V@|1k3C;_j2VB zAes!aupc4tR7mlXaWa_VZ<)cjvry@ii8?fOPurlE(u>JuNH0Z+eTv9yM%8_%STZCq zy_6ntd8C&zFeQeHvOa|ArLZ9o7h^>mO9dlNTot-1J#i*1W)7OM{Frhunzev!> zpj;=!BM{g_@1BHsCr)Gr`@B@JAB-?xXoN4N_(ChxX38*tRNWc>qVQnHX3EEfoW z3f)Y%YjsN)5@4zvL6HU}3F0`sw1QF!=1`ab>l2G4h@9FGbDq9IT2CbSFVXRJ7*(?) zy6=H6UiMO~^ko^n?&}e|mv;s$VLa%M7oJ<)JZ>R*2LkM6+!Fn%pHhtncmIov2*GWi zzk(%iByFAz-Tz@-`C*6QhX)KOQ}lTRf&^}lwmbn@6^={tk^u^(E!siK9HcL23k@(= zraOcH;^MaJLIi&C&YvlkuMAEsc}nY81}j@i>|lzr7!m3p67ENgWVUw+aRp_BXcSi< zpc$c@YAiux8%6naAQw{#2C0Vmus9p`jP|`?I+5*7$cZsYOPs;sS`G?=3mtMa(aLMk zb_StFqzL%NMs{CETVo?Q;1|%Fgb^w0#=OY@9h_&V$Wr=zi~9R*6lq?3+ZYlmVsAt& zqbD)D8r;P3#O}=a_N06N0io7+@Z@yVqIHGq<~rVf`EMy4fimyvHB~ip#dBG+Td@ZA z^rHP`tSS7AfeMH3!Ae25X*`VjleQr(jUtQ?N>6wO+9PS3(vC(>Rs3!$)SO0h(0q#IlgV1P zSIvRXdmZ*MEOLa@u81ldlU(_vOy!~kv_Tq`G)KNowfCC@Yh=escZKzg{R%C7<)qX_# zDBNGtHyGa5M`w_vyFNN2!va@`&PYvRsYZo1k!}X&nbdYvq-F&7r`D0B=;V7Tl38ig z7NmPetSU?0scY7%v{6WYIo1{PrtTa8 zdQE;AEkD#Xzav)G5%n(gE$oh-x_{xJ{luyUb#58h&j*qp_*`$aivm~bV@*fpz?I`q z3S5zW*#tR5K9qdjV0%?AjTgg$EpW2zk|TYEK;BkbH?z8ZvAiRSbRCM-FN>DTo8sk7 zi{-9)PaOW3`O$({`OfG})c?@j_sLo*-h+uRZ&V!FmMZ_tM9034=t#)10m(srW+v~; zq(}1Vh2)O~#)#x32~@~{kH9u8bY}djJd0q{V-2t6Qm7&> zQq@&1;y3~HLbf}3b-wi5B3Z(-+TdR5k#X={JTuKx4%d@n`~=mcG?vK;cF=@-2>B%R z6aA2RadAod#R@Q^{>?0RTFOi0%t;9kRD=9iJXU97GTD^@P%A62X@HrG4fKfmYerxvjO7#8U5 zd4hu-JQkMBJxOt`d=9L*>65h_Jd7d8Z&n=Ery>Xe730e9VJDRn=8WbY*v?hL00tUy$jlgFk@Yx9b0!Bd9_l`eD94j?UB7u4f6tU7QU&k|f z=m{p8Pu2?2pupd@_SC2HHxh^#f3spxGifHshc_w6akA*95Dn4J6BNMY-e*GA4nd?# z&)h)@;r0A1Q#OEfMXnm(Oi|=!Q1+%Jfm3(}7r@#Qz>q{tLK>*FEHVi_(lGH%`P)yR zmQnB9)Y(Of-_Bi|QT%Qp1uhyUOCtbHNNuG^>ckYhLVuv(<$sCrU>HT3@^-rb<5ET> zF~8EZJ2>U>BU+JVo*DB7Bl|W0^C#W>fFPa3(oLyvYv;Djl_EV6%b>Jq9}*O}CQYS- zC0fcq9kZYLC`(^{@_U?od+9lhn9{3N@}KRgFsk#L(bAc9de?lVgY^ zF>u4^p=VwX5Q``)5>{jY2!KYSlx!}oWMfRcXfsm@4*)l`q-G6*Z1@2AZ* zj^R=oda#X3h!fuHnqS6m@4X2mK@nbV=b8kPlPHiG*_b2rvUf)_M$2EzL}Y49eJ zxZdEMi6j;%;RG;HIFQf>uCzxif>e(gX_5yg48tByz7)C^5VKm+ESxEW_@C0=^Hea( z_SdwR%BUl!k~v-FB|ud2S~?m?X-k=jkaaKY_gsZGmr@}+V9J&AVbWqF{HA>9rM#=W zcPhZsG7`xaS>7xthw-3I_|R{zp+6^J{370-(wksWq1o4>JC;{i9@o$%D%_WvoZYU3nO5pM)!TPM zu(}+fHnY1O^{>6=xa#49Elw*>&rg`$WMlKX-H9w@s0>VCVL=Mfx zK*xKL_!3!=AsH|;v6yQ)Vc-LS5bCnjV+pg=--PjsKQJ6jlu?Wu;bG7h3LxzP9`7T} z4zi?W>z7sNVq%HZ3b}GzLxRnqo7Oy}#9LV0P)w8V79Q!Q=R}^*ETuecTOScN|0&)X z5ixm%Uq1F5zr38cEuOb+F>l9mUTZwBHL8v0b+G&~5LV&^&gJ}_@%){O`Mbbuippm9 z#*6CbUW*rX#JXOH7rn4tcp+YRVX^R~TV^Z|m|yzk;GK@Ib$_KhwjFsJ7c2W0OOGzv zkKHng=H}bubK8WtIzbEE7RBM7mHM5rx~`bn0fpF_MvwKiv37l4VXTz;vtlp9I`(gz z{L;zU`nl?OMN70OUeOi3e!u9UtseptYBQpMwciTxTFy55?6llEKD*WJo(ynd1_Cy> z`&Hz(7On`hCeq#u%VE4qdps3)sUVKW{~;BBM8%&_@o%a4cU1g)Di*2uD=HpSp#|~b zO;pe%f+tlnUrohUDt1uOOhqRZ-BcW+qKAqjRFJ0_f0ByRR18w_G8LDoAbUGMO2uU= zCaIXBB22|~Dk4<;11kOz6*LxXT-c&Qd@UVPK^}oY>e&}5B|<^bs_~@Gc!Xrmf|8Zo z0<5);kzXk;r~ieew4a@~Qc{Tt*vHkZ=Gx~BMx%8t*M!2(_E=GA+_%YgM^a$ze4f)>3;-YegpG{xy3Z_G&H0?zIvN+AOi+u#6RYW8<1N zi)t5Gj0e|BveB)I9AgJH+_6?-#?3{!#(p%JWo)Ky9$YK4;YyVez0NH#S|2s!q1VF5 z(RdLw(F%udw^J9%k%-eptTBFx{~@kB^#cR_&ay<78*4j{xZV7Doa8CwERjVElTP@C z88PC2jlRkVT^yp~EY&bm9!??&{1@oz7pY(r%1c$gP6ZQ1zd?IsiWS6D2lf)?LELu) zYoZF@_BU}aS)Y9Vv`_!oykEe9gZx{lga4W!f%d9itJQv_GiVKuY#OcYCmPG2Ysygm zM3es$jrAv*JpBL2q_O7Sy8IhkmMvT3maU7Hnq`YKZgI}n#4XLS)@|m@Z{%D{^}_-ZKz=1=Kld* C6U55^ literal 0 HcmV?d00001 From e5fc1e4834b407c81cc3a5e0e516c40f76c07dce Mon Sep 17 00:00:00 2001 From: LearningCircuit <185559241+LearningCircuit@users.noreply.github.com> Date: Mon, 14 Sep 2026 23:38:43 +0200 Subject: [PATCH 15/17] chore: drop stray pyc committed by a local test run --- .../test_reviewer_context.cpython-312.pyc | Bin 49111 -> 0 bytes 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 tests/__pycache__/test_reviewer_context.cpython-312.pyc diff --git a/tests/__pycache__/test_reviewer_context.cpython-312.pyc b/tests/__pycache__/test_reviewer_context.cpython-312.pyc deleted file mode 100644 index 389a98d319fb8c8bd90bc2726cc89c2e62f95d43..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 49111 zcmeIb33wY6xgL-T5?|*&e{8nhe^Rb$av7j`K}6H04gm z+S%Ft{a+m@fCY+@dv^1E>yr3T$E#PbUj484zuuqb{@v*O_`cuNX#O`kkT#>Z z^Qc~{(Y&SMG(#Fr%jqVxLt6H$8`9xdKcSyA3>mb-HRF(xT{8`t*l*TQ7W*|1nc1&p z$fDDD4PNUzXp^?_QsGxwz9SxtmnqK*`<$U1JDf3+tMY zi`V|P=Bs+n%4LsfIolomP(D|{<$O^)RKOK-x%e&AX^v{Ryf11vyVo+_u>J|}=!J7b z#oiJw{|ajG1?aP7yhWw1cuK38)=lf2MUmg{yYA&jd_k`xblL0Ry`BjaSAE`VULHHI zhP}a%!~TP|7;)}LZx7MI2iJUupex(TF-|4ta`U&YC3c| z;O}&}CLLkFFNDh>2j>fp1g_#HSI|Kxf)3YJ>p;Na_lB+o_$$%_gN^|2m<({<2?w4z z5p;NXuVd1~d0VU>QvU~>+Jxb>Cv^Ek`X-NaJwBJz=a%5*NPbzx)hA3m zd$&ZvsK+;YHENNWdTRUJtcQO((K2@{?YFh%ZK)Tk#jHKyg#^fxT#@V(WZ)+}^oT()>}D z!Dd?37>uSzxT+bECYSqieonz}Q*_68hj}PsHMy z0!;fuuJ+_^hcH|Qzh}~$$O`+f_ygDc_!K5j7?{K-vL?O3pl8hMH1I@b5?MkMLB5iX zOf(+85MO`;r-{|ztFX@+<%t9(jFVn|%*$6}KS(c+FM*9GKuBayg(oI3zN`%g&g{Tn z@KqEyHGlT}soTccy4m7cU#zqtmgk)Bi;hIkFF0eJy|Jdgcy52}#aCh;F7`{_nAacs zTp)H~Dt7+Wn@2y$D_AM4SlQkb+rInu?%BS0LG8le%9i>$?!E2L&3jQqr(@MU560pJ z7e2BW=}Q@nCcXqu9l@Wx5g5_-Xb+?7UJdBEW=Lnza2=r0pztaG^qihE+{xyQU(^rj zInx(4Lk2DjrI9nEG;tP`S)3K6naf6L;cO_apt+i%Y*1Xykd3GE|-rok1IfF z=L%8gb44f%xMGxrTnWk|t`uc4SBA2LD@R$%RiG^6Dp8hmRVXXCYLu1S7L--oR+QD8 z1LYR32IW?+7Nvu$Ls`Ssqpan&p{(P!qpat4pxnkapxn+mQSRUxQ8sW*D4kq0%0|wG zvWaU!+03<~ba8DcTexNg?6ymDBPS%I^h}P#qHpBR0F{a1 zj%O)u6L&)v>D)LIC8Q(acSb9|FlF zBQTREJsV#QV&fg%nAWEP$6OIt%$3~H-LTKt$MdDWC4VKn>eXC&PNR7Rn0F)pM!`(( zOg{DeM&V4M8a{EQA!0gg8z*Lxwt=CR%@jb$HAuY#WE852(Oh}5#_NX-z`M6KAz7pz zCkCIkrCcX=m$s2;9U|#~Z8N!2`&d}~vtKn+7+LVTjWD+)={YE#5{_IP^}aFm5y3*)QPw8jpD^w{R57RwZ32wDigd* zZF@&2{YZdPdr9bk^5r|a+1p>NbzwS)={DxiwU@+UV4W4FxYQNa*NK3K3pU6vEj7(v z8t;q`#<^*J+~9Pwm#uU3O@=4Xs$=Z3149f|hxD9oN5JoOJb&)g0N#@KOgpBamBB@C zv#8?P5p;0gQBQaxBsN7C9YJ5j>j;cGI@J&zT-jLuT7~-HwYFYP9qc^Y?P1E-lHYb6{Zj+SO}=0%q7d{5 z*a>0s;uU82NS*AL?*;{G33}CC((lg$nf+SeMH+{%S3|F9lS0DnXTWB$)t# z!JR@(Q0uhFG+0M3wqGLh%H)b>v9Ij{Jb=xEUgJSY_S}MR)NrvDT?Amec$AFoc3!&F zEPp29tEZMe|5P}1K*1r|^Das~xCG!LPIF+84|~(dm(UAvqfnS<`dz10hJex9u-|)q z3KY}JNlb}Ya<}6(>emf0Ur!x$KcWc1M2!u_3qh2qm9+B2_IYNPXz}=3!SY(KwzrO4 z_JmsT9j7L|q-77*vhfoo5=k1NE}{!pFS-<5ne&7^u3<1{ewxPbITsLblk$m7jYK1g zgyxLR30zu)VV|FIZerR~yw4v>GWZm+Egdc(_UXnhCk!KDej=i)t9h)o`u-Y27bzU| zT=BY+BjLK@osQ(QlY-I06$)JO`V)FcC=r|Yx`zg)mG@2s62?)_M9}NZfpw*4@K_>S zA#)`PPMz)>ID6{+VBcAH&*|gt7y4dK*p40_Ja+!Dd+^i?eFF*0>9g*E^Cu7YolWGP z?K^$S-GBbX3HLzH$-YF{@qr^J&iD4YPoM3(aQxKyb8hIniCpR2iJrrKClvSM!M%Owj+{MydhqzEfkdfPk6JrE z=swwZ?p)8&KE(~&6ogMG7K3W*p1pb-{>f0B#e^7(>m&#@WRmG7?$>|O~U8d zAr9FNr*-f{(jYpG2_wT7m`i{YiQEy-6ky-$1|R~95_-t6iL8)s(u)&3&2S^RsRC=n zri2n_eZm~_PEJt|5(R_U3-ExzULOy<3GmZAO~Dh^BqSvags2H4lVuZn(4mCw$ca7- zr!dOs9@-1S^e_!ot@pZjBpf1JQ^GiM8D_SGE`YfJ!FA9(mB@nFNBgK9;>ivKOJKqj zo&qj-6V@bUN$5dZ5+-5P6WIcZ>U%ZpnMhcZxSudVVDR#+AqcPR5TlSV3Gz_FA`7i( zo5XCAtbz&i*kv~p)e||Q7$_b}YG>hMw!jj_14jA9dU_xd*{1>%&I(L#yc44dv)EFQ zClw4|PX#Gic+#)%Wbfcflfvdtd?W2OQ9-&9-i0Dz;iNA>rZ|j{St4Z#V=x3Ws9 z1p{Gz#7mQ^@NgnqoWPJQKqfn)&`_6?S<}rvuu^3%h1lc|osZmq#6AO^5koG0`7XYA}nvWft+>+&-hImfHVou|#=7!dCL_6<$zp!-H z^4o`Q^{(XP-#!)3sav+S#%-vcJ~t0qlufwJQFdcXhtcb{LFdGFAf2mIpBvrD#f^qAxE zoa$wpGj4OY;2(%7q4@{4El z;`uw4?VT|6MGr68cmG*lC7sQU=Wk!Ocf{=-(T*kiF4oa)@tkeTw)VKK{d@J_Z~SiK z`;NM~=ijT}8LeG(blo3%ukU>9-1)fu0v^&SJcM=p_Q`lo&9coEx4G_d?~dOcUnu#P zRrfm?5nw*rH0kZ+N^TJM_055igR!EtDF}wYn1>dRodc~aW-r0AD?hehLh?VVqe{1dB&bOymw(nbU zbgop_uGF@@-`pBCzjJcA`B1$1&|-7X`;D%7?>mjljotCa?#0IaE8ATwwa%5s_LZg% zlpj^*R_5M%VRefpr)=4_C2rd?d-Zo_<_nf=&F>d)ed}{?er_ed^iJWO3vXA(?AumK zURWtEpRbR;@Sx!1EJI1|t^QT3CMSQ{Ru#8Z&0bipZi!d7M77af-#_r(1537^6`NhS zc4zQwLth!1H@%y4H)pYW*MgZYmRH9dZSnH9ThFiT>s#J;BEIj$(!PP)&o37>#ETjh ziyD`Uy5dD$OGUe5gBNb~t#lvyan*Ctda*6T!)n)iRnOh(ef{LB86&h@>55mn7Asq$ z^~)Vc;vGj8JNn|4eJj;9%hj%UwQI4u^&`Eus%@oX*K)_xG{Jp~H#l`lXCEMZTTj1()O=rBObFroiuki!@4|?yHz1MU0!TH7g=N9)2 zF4@il4d3d1vwJo)=Z|mQxxD32e9NK5Ej^2chkyQ>o}ubT0>*x_Hm=uH_h^59MK~=Y z^5Y<>z5nrv?Pv7*f4{Hl^g+}AQhKBs2eE@jlySZBj4f*f@`AjPnwU7tqthsqCb$`R z%D|*T*RVlZsuh)`*-)0k`U(rTo(#-aH9YpA6wP5uQLU^@(_VvDL8MXe9dST-5ZyY+ zrNco=1CcW0SFrE%el!;;Y8A98tq@d5ej^eZe6j{ok4v#CvbIB!-+9W9nN4boC}hJYk`A-bfguBx>9AR zO_5!hhnmzJ^!|`>E<+;$al$bQ#|?-p!4@{`kC`$vC=WU&#ca$Y7WNHr*cuVcy=-pG z|0-UO|A#12Mt=$?GO#1z=>ngAK=%>6tsi`YIQX@5+g z_1}v;JtDH3lhQw+MImFwP%Fddl5nN)cF&itE$3~G=WU&{E#v)@bn4izfj!4PTQ_&YFBQ{@vxvRVcG1OGaD zyndK@oXs%I3h8wjwSr_NT8|ZK!)#f~Yg8XqMwqJGG7M@I;7lH1<4P`or_x4#HTQIq zVT27Sqw$hABca0$9qv^_3Db;xCo>bO)yY!U;mj~O)2GvCXwfpGCtu00>XmoBj!&1N zzExQ-L!InR)Uj=%PR=IkWEyTnQ^n8HB;hZd^7iCZ8Q5 zcOkaT7`Za4`Hl#~$txkdSVg{;e4cu%Xset(yMmRKRE}x5s&8nYc&C=DX7_B_M9rat8%m{H^5_-FZ-?vrV&&StV5#NC3&BC0yhUE2L)Q{gJSt3a<$T{M2@f*Cn-pLIR~Ul%?k4 z3$a*m$;PX4KHO;UnYiYe4q}K#Cc^llp2!vqC`@AHPz~Jy#5gZE>>0Tte#U8jAHjm| z6~9v-wqaN4O&qRCJ0bWbL&3zvZ0LIEvxf_*)>mJLTp&1t3vd!l>g z0p$*2bS6E}@d&?gA$&EAq2&@#Y(gIi+nP7Xlg2Dz@WSE^FJYoX2?Grl+=TJj`H%-* z$JR;DbvIK|27Q{xsF6+TFuWy+sbjzM725vr5d2vI*%J{PK)}~JHZkeyYUy-Z_%BeK zWOXY~ip)e7)SeR_Ja?3C(g(dE{w3_eftDr}beX+) z!h{z9MkMqh&sZW${3QH8L7jD;wEM|%nLL%rsow4LLyHy`u*eM`OIW_IdPQxFoAXYE zF1ulJz?DEKFcO%EoKc;ye3~ZzBX#mWfIQyRtQ3?i7u3ZI>gLZcY>O4tEfzd?D{Cdc zY_=kvUq5#S`jmM~Jip_XX~k@NBj-ywv68(DyOzv{<-@-FSC`BKD}_7PG+Il`ts|Jx zzH#zPCue(RM`q8@)yy`|jXt!wR`s~}fz-sA*twUN%tLY;7h*4cZprMHkHt2n>K9{| zMwZN+d^!F0Q?WBIE}37#^<{He+*}qbZ<@cbWZtPbk<#fekIs%RnYW4UeR=dP|C@eH z@wygE+M{^4?p2+x%!+B8rRmmDXnk_>*$eHM+x5`a2(^i&2}*&2@>qFeJilrFWIVrn zVNX22mkp87fjfn>7v`RiSGO*fw|%efdlQQtN0!XJa(8A+=4|nDSFGi5y!^=h{&@Kd ziSk_^V}$l#goGAmOXsTM7VS9K8LRK+!g2d(Tgm^?>e0h~da{Ex|AauEJ>oRKjo6DzZ* zWCEOu&n;($M^DnXOFu6A;7WZ2<2Q2_sSfi!G?0_Ms>dO`E2WxDz*G@T5Y~`dN2OP& zV@zpBMbwzlkITYFvad-m;bfo5L&#|zh%{LdeGiLQN1!!n2?aVd~%y zJIT{qfe*bTfRklMyM720?wo)Fx~a=rkuxuxY%| zca{-C79PQW4Glp@P5O0yjv8g;&dB_QL|P{VY*!t! zBT_<4jYQylK{qivB7CglZjVp8KXODEA}^mZ*?hW2{!jEY_|H+?)VyC%b{oW?KbCj& zmO&^n z(8a~ZSK{U!w{l{oJ49j_hn9Ex~e^0lh3RLyaVAX#S~ znp@XnFtJ01!5?Es4TDo~8euA#reI0@O+u>ToQ%Z^eAdq(aDrj77_i3hnZ$Q>%t^&A z6?~pa$5NGcIFk}ffj?OTkzDI3NS89IMDndCU=)F?jeIH{F)9Qkf@sH7gb=O-$@lcx zZl{s|I>u6g3v+Zx2!TtC$?M_BL_rJm3|>ZnAWgrRy+tsz2;j}2S*Yrs@Q!&#rrpDV zkOJIA>5SV5yf@tlB8fCXaux)5=a=3#&*sH)8*b^}FRHk`Csyf-<+p$jNjx2(e(rZ) z`QGk@U4L*WR)1u%pf@9oi{MzO zM}I?qLk4clJq?_KIdNtN(!^9$7zSr~GBe<;DRs1%eenoKM+1Kx1wjh?H4-9E=V-5JQKMNz7WvL)6Phvd)A9p= z_R8DF+o6wP8z{QnJKI0kHMb*H(Kz4#(AN1#j|z`8`oip6Sxl15ENl1NhdlbcEbGO} zyBGSG%tuy=DoEPi34^R;U7Ee{wO79K%6vVnOVNBh1m-8p&QBiY;Zcli{a(f49o7_M z1d(Tkv%471as;`zMBLIcCW7f1ri5%&~MX-WPs3e43(%d;xVm&T_F6Y2yQT$6oO#$Ej1#WPf5?OGu7$jCC<6Ih| zNdg-Qt2{mBNth+DF5*)@)|t?>8-f`JH3hk0xre7&aIQPRyIIihMyzYojZ_jo{WEk9 zvrfmrnkLI~R14egt-jlPVWoeo>dmUTzUa%hs}}PQeWWqv9M!&G;g~Ip)$ECt?uBK$ zV#{(xQ@o<--iy(}g|7R~Mc0W~^U1}Ef!kRt<&L?EczN^u8JZqh;^p19O=RoNc{690 zo9kb)H;G5FEr;%xEZL9YaM|vN+a0l*0}EG|?EOE_p$D$A$9|-zp8Z^xJo7k6%91-h z4TtlLf1GDLTxLJ_w~~((!8tS9b%Lf zTGb47%xQIG9xU}4Q!z~LlDr9GnPnhSblwjw8R)(M$m9M|&iE-G%oM?SG33ns7+5;exdF9AgI1z#-_HU`|j=g%HpVw<>7n1oBTHD^D$G zis8K`PcP1A<_w>r*=+>Y6r`h2ro3+}j1@I5+8D6DbBP_+&yCGrU9|05CI0Xv@-GQo ztx-%m_N-?Bl4@l8yh(M|AkX{%Ejp$)YXBg~0EsH%rkD>Tfm8;n^m#vLyl0X?g3bNe zd|wc$ILk(Je=q>$++Uq|>y&kbd32Sv*B2x=HF)IG#}y~O!go#synd%v(21u`nIYn* zQPDZPya0d(0RTyEQ$QgJJtXxDVA#jN5LqkG4H8GBeu#1jh-knYWbioQ^;~fSTqmLN z3k5{*6x|sjLz!Xm)3)7807I-@Txj4oF8t?G5# ztREDWuNpOl^^Y{V!b->{CTO|TWD{|&P&9jW$-G@L@%{f5C>B~zKM?}&Y6X|VCqW=l zKS5xAfIa4?kOa7nG=O75tVGk?SG?Y-U^);+j&26JHqj6zS4agVS1hk(GoX}TB7;)d z2B1VL<=fmWgxWd8gZ9&s-_e~kvxYnPei>M@4yEt1KTq}*ML+U zq-DILyJvleFdZ8cepOf?w42#X8)B2F9@8|c2&Y_%0T{F^cOZNbV`;Uz9F2{V*{rdV zVvS&+VqV4F?H%0+aSH~?!UX-FU~Zd$Lk{AmU^fUlj?B&6qba1H0#W2 z-FV6ok6*UPO|#o4ePfs(!hl3(G~f+pZ9bOShJpkIA8Q9(&OOLE1PY1xu}OyqVJ_GF zxDAFa=3p#D_YqyfnayVe;bVbG_i&hlD;pDRz+RYrh>N_C8K6j#h`i#DumYrelbUigBD@cKe4o7Y-$L_VQr%7F1?{^&#V&hwm_0h z9{nFEfaB<%ReD(1=+`;<{YI*-f(@;+q%5YBJCV+6H%| zLg#6VY&}Uyc}UDTh8TpCo~droi0d$Dla2*AlrV4}zX!~PrhB-fJ(5lSdQ=N>mrV4= z*$%`Av$HU22mJ8p_aL+b?Q~<(IZO>kjEAvv80`>I!}Xn!3if2jFak9Z*SO>Q4rr2K z;U?|MpfriyDpc;09tx8<&mnBU!v`kF=^b%RADU3-LsI$O)g>%2#`~D<7~b+;G2Vth zK{bk~ppX3_$(SD!$_gqbBma>0SE%?iDiT!uISL4M4*C%hjir1dcf|h{CsXk(mnI0l z5IpLylZ`l1r;0zRmCh4hkfsZGVTRxGtbt~XSC?!9@7qde4YQNc-AlH8!oELxbjh|~ z+LzDAwsha$5j%5!$#y|`&uYGf;YEq2sB*q$p(b9nKUQ!6sgUMt7uw@h2Vw;W*Bzu( zlMg=1HsxDy8G)Pj(plrJUyhlp*JF{)Dd+GNGEGToqDC9DtO+U~`WR^&xqT6Y5=ZrE z0sG>70E!;HinWid1M-X^*%YjOsn!9tI+jh;v2LPH_9p7sHc=-ht&W;ZU{F;&$yr5a z)UV6gSyChk74?+#ji3TIiZ0fep4>3P?aRpJ6XIUg2DPteOGi+8=NmLPOiXP?^dW6a zIz!EvBnK(3Af*@iX}91oL^>t5QM#)cQ?HzFNooVFi7I|36G2IzzHvp$(G(Tix*JBQ zwTlP(`R7p|!BU7FylYf2;TJ7?FI+ zU7DhTNpvsLbuo&qpQ0cWdXIma33_+41XaO@RGD~BQeVKr4iLFUjL=Pk79_Gk^EqsS z4x#yJh9=VUsYlmGLwOtpR8-~F%jM0`l`fXId@1)<*6m{1TFAsXs8Z*{D@D!oqp`xS z)hwnBFfh(#&7cjSDcp+f?24`6VvpabLV~-(ur|9~TeJ z)!#3VoxKoy@um3Lm*4Ao8Pm)5-yVw<58bk&kY}0j(HVMs>wL|Uxmj@zuoZoNsp`;T z`Jww2OXd@@Lr$#x*n{m4&1XJYvuH{#YMDa#Pqx|mvJHRIk%ck^;ba+h0b~9Ph4?VU*;4 ziVRaJT*(fu^k0YsXxkxf5`W^4(_tY^A;mCZ7My&1@ErfwbUBSXZ4>zl&C$Vj0xUHM z(ucr5b2Q0lc$(-$YzpGu{W#afB5=CNAr-Rw;~eQ&knsP89%euRGL%Zr7;#D?Vf^2r z=6WKuSB(fw_()`iDTtH?2%IP5kUKEy7CnkVtZ_XuuxX04srG)2$PJtYDL?fR0rRVEL!Tj#eg zncG(yTRsK7d1!9_1d$mM4f*43wj-s6KkmpvnLf|b=PRvc7hRWoD%Ggt=3_E%fm69QMh=|CPfK9t2L?{$fdj9`L9sa+ec$_E7Mdz3U|8zGm6Ac)PHzpeD^unp6 z7b*0B=mpc-jfHvAT0{;$kv?pA{~e+SkMI(kqz5alT@Oa$g=aS{uEa`GJdAG#=0F!( zWV(<6?Iqvb7`$zFEtxxHxTSD;+2%94#Y#R`k2?75X z;g}k~-@duoXvBKj%e#arkT7HW619kV8GId|Fs(4CPbhXOaZ{RWMZA0fR!OATfH{&F zy6>uwLoN>BDw`-rjz1AiH`~<9G? z%k^FH`mV+L-9OITGuw-72xopfE0(wCmH~xa9n${b+vRgTN`s|yerHsBclSec=O>64 zw(n)`XFqJQ9d@N6ofYrJI5ABRH_}KW1$+j7dgM>0IBhLuQ5LLCK4jyi1sI+AmRC#7jRcFM{SID$CnDPN$1 zCh2!Y!~BPI;#*XF8%3I(RIsb0=^MzHhIEItOaK%QOy7WfMiUehB`P5{va39GWB;1) z;3oP8(|aNEUBq_my>DJBJRU1}9-g<7Tdp+Gj+z!qW2eu=e#sL*JRI9I5^Lq2dgdJ~ zNt;&hTe$Gh+`C>QOeB**@i#VL9i;0OET?2Q1++q5WP4fIGqXJl_N;6V_7lpWJ!a%! zF##OgC3^{TP-41Cu7JUDA(hOkN0}(q(S8>lq|?uInbXlKsOdjSOA8Urwl=Ja0DEo46DEp8d6G~L@rlI z9)#GkiEt3cp@^N9R&fzS2s#=%TlY94x!w-13o&ieh%+3!97<$~k%$p%*yqC16FleY zK+{)zQ&V0pVncSpD_~H=VIQ2}v*A~ZNVGASA%kFm^i;kC5XAS>9uWf|XC3X=Q{kjS zO;hXa5GTE4q7K5Ki!5zii(??*M`lgI=8XtJTt zCDN*iZ{&<@rE1F8qKb+ad(153G}w)eBvPYIc?|i#z$-yy{sO%j5hG!WEh`B5SLxjU zgd)}av`1jc^iFII1h>FTk;Msn>?6WWNYH;GHqehyNpTz^CpTcrPv0C(AEn843IY0{ zeCzza`@XpS^h$YC^tIU8i*fraE9K7U#n|bW(|2gi{EX{CHe^g!I+vzl!x z%Z1&k*g3yFR@lB;$h3o2(XOTLDv+{M~H}Q5k4-X>l zqBW|9Ws*6Gv^;u%B9l@s69I%H2^cqsoF#?GSs8f~okBJwaw!fW8D!D)i~wrHDJ3HS z7&uB9sKf!PD*jTFR;WGgU}uflCmfu*6qdYErZ zhk3AC2G(#XMkMQmX9{jCh~h;An3w7OBYm5K^(Qs+AJMj#2kFZPt9-$3N12783Xe;p!xF8XKQF&trl3wGjx*c`mn@8FN z21fN&-!gfJ@V3soIt3c8$3VfaUwAKNpm~zhVDQKcCrA_<`SowQk^e&2M~+eJv2RPP zwfDjGxcwzc$kx&qJN;7J{xT2{T3aCCwmBG?T&tT$z$5oBz$ODuAh41C2bdC`l1)ZO zA3%M3TD~gSYu00<2|ZQ5Aq9V9hHEB{|HrU218{HS3c|F3WXcuMX83OJq<&NA=+t90pGRD2sn z+8j?{Fp}e{5ZcLHVBJEe-IU1+f}JqiL(bKW7|c^N^Ir+Xe;xg~srgHC?HAmpZGFtK zfA>Q1ebb`tD8V&3$eCpaxw&J}@JjP@_v>O!{i}M$XS9XKw70T8*eJn5ny@6NE)cj* ztbES`43RzS0h<7U!0yrifC5Wh3fc(tJbg=sm;hv9Rg^(nOS3&%pzdClbdCWq3z8H( z`UPOlsFTQ60v?!+8gSZ_bpV_>1BoryH{6oY*`Fv|2X*cfeFI04npoG~#y8=0(*&F_ zF_&cXj*CCN`E9tF_$jR_cPWvbymTE?nJ=JnT5uIl0%IB=3UoFFf)M=cA4BdAK%E;Y zbFM(p5uQSmvJE2JH{FOCCWYWQ`T{zeSE_WvMvUR$?C!b##hj*?tr;*^)Dkt2{a*0q zmpn2zlF&P%&|vSAh3wffWVJaCc*>?K2pmZQxT#sS%N&#d^5Wc%`CW@S9Wh(yg7>KbGMU6c5E`GF(0v1a zdNx9b=r|g5Y)m7%J~}QFkxNfiXcSsemZ@Q#0K@EoNU@NGLQD@JNqP!EA|LKLv?3%h zPxDqOKgQC|91*@d)?1b{W=K*KgZb)B08O=c;kl=vG- zW>3*f6M@Yr`UJ2sDZwUI+&n)Py}FolFlIXhWx#y>rnsRbWbKTOE}0L={4fT!5~QxZ z>&coG&!(^Zh|FJ^;6&2(Zr;Mna^t(Z~n6el!=YJAh5lus|z-F4+MQ}xPkth2gM8_WVVSWoQ1mq!$7rwM9qoxFB zg6Dt&&N9Wm5e;B)mWvTqPFt06ZD(yM{omYV5N~t$WQupxPz3Dk%IGs62l>||o_WH(CY z#bF^ZA{oU^@}^iP0ZIub6teVcssU4Sa(IOIl(97wkl6o*kT**O+-9&?twg#h?Vz}5 zDXp_$gcklbIw81n{08=*f@2iPPiOxT6{B=kOu+GZ>?g8W-gqG;f2w6ua0G-)(G)Lo zQp%D`K1SlI0Z_(asTw!*;Tt#tZsz`%2^!9!H(wH$pqdV3rv}sT9trO;gd7=|pNnkLbM zXKAsBkR;<-1d1U@SF)=(-79&LLo(ysm^XrCh)>vvVmwttpCcH#gzg9~H=)?Jz6UQZ z6`qe3Tu67hiH@xlx#q9N3Xy|_DPuRI7|Wz>m5qA;6wls>EC~KI@b+R+%6AOp{{;L#IWOHvOmLdEaVaK0NcLnTZbc=wz%bv2y@1-)pKYIw zz-J@y*$8|#0-ue*XCv_02z)jIpN+t0Bk=cr1fWQ4C%K-Ma7`FPSQ^JGWPUwHXUW|V zGyOW+ucxAk3Kgw=q@=O&^jXIs-A}G)(%c|ig&WqXJ3AEG&h0cKTyG7kWmawI2NW)S z%wR|pLNaK>&wO`gGxmlzj?skg3ObhVZnjcUJ#U;3&;9aBdENXF5+K|6ejp_>u(0`_ zkvVzVHgxiAiNdLDHhvowtn0=nd2t0ZE$2>V@|1k3C;_j2VB zAes!aupc4tR7mlXaWa_VZ<)cjvry@ii8?fOPurlE(u>JuNH0Z+eTv9yM%8_%STZCq zy_6ntd8C&zFeQeHvOa|ArLZ9o7h^>mO9dlNTot-1J#i*1W)7OM{Frhunzev!> zpj;=!BM{g_@1BHsCr)Gr`@B@JAB-?xXoN4N_(ChxX38*tRNWc>qVQnHX3EEfoW z3f)Y%YjsN)5@4zvL6HU}3F0`sw1QF!=1`ab>l2G4h@9FGbDq9IT2CbSFVXRJ7*(?) zy6=H6UiMO~^ko^n?&}e|mv;s$VLa%M7oJ<)JZ>R*2LkM6+!Fn%pHhtncmIov2*GWi zzk(%iByFAz-Tz@-`C*6QhX)KOQ}lTRf&^}lwmbn@6^={tk^u^(E!siK9HcL23k@(= zraOcH;^MaJLIi&C&YvlkuMAEsc}nY81}j@i>|lzr7!m3p67ENgWVUw+aRp_BXcSi< zpc$c@YAiux8%6naAQw{#2C0Vmus9p`jP|`?I+5*7$cZsYOPs;sS`G?=3mtMa(aLMk zb_StFqzL%NMs{CETVo?Q;1|%Fgb^w0#=OY@9h_&V$Wr=zi~9R*6lq?3+ZYlmVsAt& zqbD)D8r;P3#O}=a_N06N0io7+@Z@yVqIHGq<~rVf`EMy4fimyvHB~ip#dBG+Td@ZA z^rHP`tSS7AfeMH3!Ae25X*`VjleQr(jUtQ?N>6wO+9PS3(vC(>Rs3!$)SO0h(0q#IlgV1P zSIvRXdmZ*MEOLa@u81ldlU(_vOy!~kv_Tq`G)KNowfCC@Yh=escZKzg{R%C7<)qX_# zDBNGtHyGa5M`w_vyFNN2!va@`&PYvRsYZo1k!}X&nbdYvq-F&7r`D0B=;V7Tl38ig z7NmPetSU?0scY7%v{6WYIo1{PrtTa8 zdQE;AEkD#Xzav)G5%n(gE$oh-x_{xJ{luyUb#58h&j*qp_*`$aivm~bV@*fpz?I`q z3S5zW*#tR5K9qdjV0%?AjTgg$EpW2zk|TYEK;BkbH?z8ZvAiRSbRCM-FN>DTo8sk7 zi{-9)PaOW3`O$({`OfG})c?@j_sLo*-h+uRZ&V!FmMZ_tM9034=t#)10m(srW+v~; zq(}1Vh2)O~#)#x32~@~{kH9u8bY}djJd0q{V-2t6Qm7&> zQq@&1;y3~HLbf}3b-wi5B3Z(-+TdR5k#X={JTuKx4%d@n`~=mcG?vK;cF=@-2>B%R z6aA2RadAod#R@Q^{>?0RTFOi0%t;9kRD=9iJXU97GTD^@P%A62X@HrG4fKfmYerxvjO7#8U5 zd4hu-JQkMBJxOt`d=9L*>65h_Jd7d8Z&n=Ery>Xe730e9VJDRn=8WbY*v?hL00tUy$jlgFk@Yx9b0!Bd9_l`eD94j?UB7u4f6tU7QU&k|f z=m{p8Pu2?2pupd@_SC2HHxh^#f3spxGifHshc_w6akA*95Dn4J6BNMY-e*GA4nd?# z&)h)@;r0A1Q#OEfMXnm(Oi|=!Q1+%Jfm3(}7r@#Qz>q{tLK>*FEHVi_(lGH%`P)yR zmQnB9)Y(Of-_Bi|QT%Qp1uhyUOCtbHNNuG^>ckYhLVuv(<$sCrU>HT3@^-rb<5ET> zF~8EZJ2>U>BU+JVo*DB7Bl|W0^C#W>fFPa3(oLyvYv;Djl_EV6%b>Jq9}*O}CQYS- zC0fcq9kZYLC`(^{@_U?od+9lhn9{3N@}KRgFsk#L(bAc9de?lVgY^ zF>u4^p=VwX5Q``)5>{jY2!KYSlx!}oWMfRcXfsm@4*)l`q-G6*Z1@2AZ* zj^R=oda#X3h!fuHnqS6m@4X2mK@nbV=b8kPlPHiG*_b2rvUf)_M$2EzL}Y49eJ zxZdEMi6j;%;RG;HIFQf>uCzxif>e(gX_5yg48tByz7)C^5VKm+ESxEW_@C0=^Hea( z_SdwR%BUl!k~v-FB|ud2S~?m?X-k=jkaaKY_gsZGmr@}+V9J&AVbWqF{HA>9rM#=W zcPhZsG7`xaS>7xthw-3I_|R{zp+6^J{370-(wksWq1o4>JC;{i9@o$%D%_WvoZYU3nO5pM)!TPM zu(}+fHnY1O^{>6=xa#49Elw*>&rg`$WMlKX-H9w@s0>VCVL=Mfx zK*xKL_!3!=AsH|;v6yQ)Vc-LS5bCnjV+pg=--PjsKQJ6jlu?Wu;bG7h3LxzP9`7T} z4zi?W>z7sNVq%HZ3b}GzLxRnqo7Oy}#9LV0P)w8V79Q!Q=R}^*ETuecTOScN|0&)X z5ixm%Uq1F5zr38cEuOb+F>l9mUTZwBHL8v0b+G&~5LV&^&gJ}_@%){O`Mbbuippm9 z#*6CbUW*rX#JXOH7rn4tcp+YRVX^R~TV^Z|m|yzk;GK@Ib$_KhwjFsJ7c2W0OOGzv zkKHng=H}bubK8WtIzbEE7RBM7mHM5rx~`bn0fpF_MvwKiv37l4VXTz;vtlp9I`(gz z{L;zU`nl?OMN70OUeOi3e!u9UtseptYBQpMwciTxTFy55?6llEKD*WJo(ynd1_Cy> z`&Hz(7On`hCeq#u%VE4qdps3)sUVKW{~;BBM8%&_@o%a4cU1g)Di*2uD=HpSp#|~b zO;pe%f+tlnUrohUDt1uOOhqRZ-BcW+qKAqjRFJ0_f0ByRR18w_G8LDoAbUGMO2uU= zCaIXBB22|~Dk4<;11kOz6*LxXT-c&Qd@UVPK^}oY>e&}5B|<^bs_~@Gc!Xrmf|8Zo z0<5);kzXk;r~ieew4a@~Qc{Tt*vHkZ=Gx~BMx%8t*M!2(_E=GA+_%YgM^a$ze4f)>3;-YegpG{xy3Z_G&H0?zIvN+AOi+u#6RYW8<1N zi)t5Gj0e|BveB)I9AgJH+_6?-#?3{!#(p%JWo)Ky9$YK4;YyVez0NH#S|2s!q1VF5 z(RdLw(F%udw^J9%k%-eptTBFx{~@kB^#cR_&ay<78*4j{xZV7Doa8CwERjVElTP@C z88PC2jlRkVT^yp~EY&bm9!??&{1@oz7pY(r%1c$gP6ZQ1zd?IsiWS6D2lf)?LELu) zYoZF@_BU}aS)Y9Vv`_!oykEe9gZx{lga4W!f%d9itJQv_GiVKuY#OcYCmPG2Ysygm zM3es$jrAv*JpBL2q_O7Sy8IhkmMvT3maU7Hnq`YKZgI}n#4XLS)@|m@Z{%D{^}_-ZKz=1=Kld* C6U55^ From 8a46c683d7179ca728275c2176f06c35b41bd67d Mon Sep 17 00:00:00 2001 From: LearningCircuit <185559241+LearningCircuit@users.noreply.github.com> Date: Mon, 14 Sep 2026 23:48:33 +0200 Subject: [PATCH 16/17] fix: use -B in documented test command; ignore __pycache__ Round-3 finding: the README's test command lacked -B, so following it recreates the stray-.pyc problem this branch already had to clean up once; CI uses -B for exactly that reason. Also adds a .gitignore backstop and lists GLM 5.3 among the structured-output-capable models (support confirmed via OpenRouter's model metadata). --- .gitignore | 2 ++ README.md | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) create mode 100644 .gitignore diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..7a60b85 --- /dev/null +++ b/.gitignore @@ -0,0 +1,2 @@ +__pycache__/ +*.pyc diff --git a/README.md b/README.md index 2878578..38af4a7 100644 --- a/README.md +++ b/README.md @@ -81,7 +81,7 @@ The workflow is pre-configured with sensible defaults, but you can customize it - **MAX_HUMAN_COMMENTS_TOTAL**: Overall byte budget for the human-comments block (`head -c`); when exceeded, the block is cut and marked (default: `20000`; `0` omits the block entirely) - **STRUCTURED_OUTPUT**: Enforce a JSON Schema on the model's output via OpenRouter structured outputs (default: `true`) - Makes the provider emit valid, correctly-escaped JSON instead of the model hand-writing it — the main cause of "Invalid JSON response from AI model" - - Requires a model/provider that supports `response_format` json_schema (most modern models do; e.g. Kimi K2, MiniMax M2.5) + - Requires a model/provider that supports `response_format` json_schema (most modern models do; e.g. GLM 5.3, Kimi K2, MiniMax M2.5) - Set to `false` only if your chosen model doesn't support structured outputs - **DEBUG_MODE**: Enable debug logging (default: `false`) - ⚠️ Warning: Exposes code diff in workflow logs when enabled @@ -226,4 +226,4 @@ For issues with: ## Development Tests -Run `python3 -m unittest discover -s tests -v` to check the generated request and comment context filters. The tests use local substitutes for GitHub and OpenRouter, with no network requests or model calls. They require Python 3 and the script dependencies (Bash, jq, and Perl). +Run `python3 -B -m unittest discover -s tests -v` to check the generated request and comment context filters. The tests use local substitutes for GitHub and OpenRouter, with no network requests or model calls. They require Python 3 and the script dependencies (Bash, jq, and Perl). The `-B` flag keeps Python from writing `__pycache__` into the tree (CI uses it for the same reason; `.gitignore` covers it as a backstop). From d88b1f4f8600076d6bda82fb77fdf695ad0665d0 Mon Sep 17 00:00:00 2001 From: LearningCircuit <185559241+LearningCircuit@users.noreply.github.com> Date: Mon, 14 Sep 2026 23:57:09 +0200 Subject: [PATCH 17/17] fix: full commit-message bodies, neutral-run wording, stats warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Commit-message extraction now keeps the entire body — all paragraphs, indented per line — and no longer drops bodies from non-conforming messages that lack a blank line after the subject. - The check-status prompt says to treat skipped and neutral runs as informational rather than failures, so a continue-on-error neutral conclusion cannot read as a red check. - Line-stat fetch failures now log to stderr ("N of M listed commits") so rate limiting is distinguishable from one flaky fetch. - Workflow env block documents why the || fallbacks mirror the script defaults (deliberate, keep in sync); the default test PR fixture gains title/body so description tests exercise the real formatting path. Declined from the same review: the claimed tab-in-author column shift cannot occur — jq @tsv escapes tabs (verified: the author field stays intact through read -r and the awk aggregation). --- .github/workflows/ai-code-reviewer.yml | 3 ++ ai-reviewer.sh | 12 ++++++-- tests/test_reviewer_context.py | 42 +++++++++++++++++++++++++- 3 files changed, 54 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ai-code-reviewer.yml b/.github/workflows/ai-code-reviewer.yml index 461b12c..45d1b61 100644 --- a/.github/workflows/ai-code-reviewer.yml +++ b/.github/workflows/ai-code-reviewer.yml @@ -32,6 +32,9 @@ jobs: - name: AI Code Review env: + # The || fallbacks deliberately mirror the script's built-in + # defaults so this block documents every knob at the call site — + # keep them in sync when changing a script default. GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} OPENROUTER_API_KEY: ${{ secrets.OPENROUTER_API_KEY }} AI_MODEL: ${{ vars.AI_MODEL || 'z-ai/glm-5.3' }} diff --git a/ai-reviewer.sh b/ai-reviewer.sh index 75d035c..f19d2d7 100644 --- a/ai-reviewer.sh +++ b/ai-reviewer.sh @@ -354,7 +354,12 @@ if [ "$INCLUDE_COMMIT_MESSAGES" = "true" ] && [ "$COMMITS_JSON" != "[]" ] && [ " COMMIT_MESSAGES_FULL=$(echo "$COMMITS_JSON" | jq -r --argjson n "$MAX_COMMIT_MESSAGES" \ "$COMMIT_CLASSIFIERS"'[.[] | select(is_merge | not)] | if $n > 0 then .[-$n:] else [] end - | .[] | "- " + (.commit.message | split("\n")[0]) + (if (.commit.message | split("\n\n")[1]) then "\n " + (.commit.message | split("\n\n")[1]) else "" end)' 2>/dev/null || echo "") + | .[] | (.commit.message // "") as $message + | ($message | split("\n")[0]) as $subject + | (if ($message | contains("\n")) + then ($message | sub("^[^\n]*\n+"; "") | split("\n") | map(" " + .) | join("\n")) + else "" end) as $body + | "- " + $subject + (if $body != "" then "\n" + $body else "" end)' 2>/dev/null || echo "") COMMIT_MESSAGES=$(printf '%s' "$COMMIT_MESSAGES_FULL" | head -c 2500 | strip_partial_utf8) # Same contract as the human-comments budget: detect clipping from the # source length and mark it, so the model knows messages were cut. @@ -434,6 +439,9 @@ if [ "$INCLUDE_COMMIT_SUMMARY" = "true" ] && [ -n "$COMMITS_JSON" ] && [ "$COMMI [ "$STATS_FAILURES" -gt 0 ] && SUMMARY_HEADER="$SUMMARY_HEADER (line stats unavailable for $STATS_FAILURES commit(s))" if [ -n "$AUTHOR_LINES" ]; then LISTED=$(( MAX_SUMMARY_COMMITS < NONMERGE_COUNT ? MAX_SUMMARY_COMMITS : NONMERGE_COUNT )) + # Operator-visible signal so "rate limited" is distinguishable + # from "one flaky fetch" when the header reports gaps. + [ "$STATS_FAILURES" -gt 0 ] && echo "⚠️ Line stats unavailable for $STATS_FAILURES of $LISTED listed commit(s)" >&2 [ "$LISTED" -eq "$NONMERGE_COUNT" ] \ && SCOPE="across all $NONMERGE_COUNT $COMMIT_WORD" \ || SCOPE="across the $LISTED most recent of $NONMERGE_COUNT $COMMIT_WORD" @@ -477,7 +485,7 @@ if [ -n "$CHECK_RUNS_STATUS" ]; then GitHub Actions Check Status: $CHECK_RUNS_STATUS -Please consider any failed or pending checks in your review. If tests are failing, investigate whether the code changes might be the cause. +Please consider any failed or pending checks in your review, and treat skipped and neutral runs as informational rather than failures. If tests are failing, investigate whether the code changes might be the cause. " fi diff --git a/tests/test_reviewer_context.py b/tests/test_reviewer_context.py index b1c0033..9cbfd18 100644 --- a/tests/test_reviewer_context.py +++ b/tests/test_reviewer_context.py @@ -71,7 +71,9 @@ def run_reviewer( ) (path / "commit-stats.json").write_text(json.dumps(commit_stats or {})) (path / "pr.json").write_text(json.dumps( - pr if pr is not None else {"number": 123, "head": {"sha": "abc"}} + pr if pr is not None + else {"number": 123, "head": {"sha": "abc"}, + "title": "Example PR", "body": "Example body"} )) if fail_comments: (path / "fail-comments").write_text("") @@ -612,6 +614,44 @@ def test_commit_message_body_is_indented_under_subject(self): prompt = request["messages"][0]["content"] self.assertIn("- subject line\n body paragraph", prompt) + def test_commit_message_body_keeps_all_paragraphs(self): + commits = [pull_commit("c1", "subject\n\nfirst paragraph\n\nsecond paragraph", login="alice")] + request = self.run_reviewer( + previous=False, human=False, pull_commits=commits, + config={"INCLUDE_COMMIT_MESSAGES": "true"}, + ) + prompt = request["messages"][0]["content"] + self.assertIn(" first paragraph", prompt) + self.assertIn(" second paragraph", prompt) + + def test_commit_message_body_without_blank_line_is_kept(self): + # Non-conforming message (no blank line after the subject): the body + # must not be silently dropped. + commits = [pull_commit("c1", "fix: thing\nDetails line", login="alice")] + request = self.run_reviewer( + previous=False, human=False, pull_commits=commits, + config={"INCLUDE_COMMIT_MESSAGES": "true"}, + ) + prompt = request["messages"][0]["content"] + self.assertIn("- fix: thing\n Details line", prompt) + + def test_check_status_prompt_marks_neutral_informational(self): + request = self.run_reviewer( + previous=False, human=False, + check_runs=[ + {"name": "advisory", "status": "completed", "conclusion": "neutral"}, + {"name": "lint", "status": "completed", "conclusion": "success"}, + ], + config={"INCLUDE_CHECK_RUNS": "true"}, + ) + prompt = request["messages"][0]["content"] + self.assertIn("1 of 2 checks passed. Non-passing checks:", prompt) + self.assertIn("- **advisory**: completed (neutral)", prompt) + self.assertIn( + "treat skipped and neutral runs as informational rather than failures", + prompt, + ) + def test_pr_description_clip_is_marked(self): request = self.run_reviewer( previous=False, human=False,