| 1 | # frozen_string_literal: true |
| 2 | |
| 3 | require "json" |
| 4 | |
| 5 | # Builds the chat-completions payload for a GitHub PR review and parses the |
| 6 | # model's reply. Pure functions — all state comes in as arguments — so the |
| 7 | # budgets, exclusions, and parsing are unit-testable without HTTP. |
| 8 | # |
| 9 | # The model's output is untrusted and the comments it produces are posted |
| 10 | # publicly on github.com, so two rules hold throughout: the system prompt |
| 11 | # carries the same identity/non-disclosure constraints as the cloud |
| 12 | # assistant (see ChatCompletionsController::SIGIT_IDENTITY_PROMPT), and |
| 13 | # nothing is ever posted that didn't survive strict JSON parsing. |
| 14 | class GithubReviewPrompt |
| 15 | class ParseError < StandardError; end |
| 16 | |
| 17 | MAX_FILES = 50 |
| 18 | MAX_TOTAL_PATCH_BYTES = 80_000 |
| 19 | MAX_FILE_PATCH_BYTES = 12_000 |
| 20 | MAX_FINDINGS = 12 |
| 21 | MAX_PR_BODY_CHARS = 4_000 |
| 22 | |
| 23 | SEVERITIES = %w[critical warning nit].freeze |
| 24 | |
| 25 | # Generated/vendored artifacts that add bulk but no review signal. Excluded |
| 26 | # files still show up in the walkthrough's file list with a note. |
| 27 | EXCLUDED_PATTERNS = [ |
| 28 | %r{\A(?:.*/)?(?:Gemfile\.lock|package-lock\.json|pnpm-lock\.yaml|yarn\.lock|Cargo\.lock|composer\.lock|poetry\.lock)\z}, |
| 29 | /\.lock\z/, |
| 30 | %r{(?:\A|/)(?:vendor|node_modules|dist|build|target)/}, |
| 31 | /\.min\.(?:js|css)\z/, |
| 32 | %r{\Adb/schema\.rb\z}, |
| 33 | /\.(?:map|svg\.gz|woff2?)\z/ |
| 34 | ].freeze |
| 35 | |
| 36 | SYSTEM_PROMPT = <<~PROMPT |
| 37 | You are siGit Code, the automated code-review assistant from siGit (sigit.si). |
| 38 | If you are asked what model you are, who built, made, or trained you, or what |
| 39 | you are based on, answer only that you are siGit Code. Never state or imply |
| 40 | that you are any third-party model, and never name or hint at the underlying |
| 41 | model, provider, vendor, or infrastructure. |
| 42 | |
| 43 | You are reviewing a GitHub pull request. Each changed file's diff is shown |
| 44 | with the new-file line number prefixed to every added (+) and unchanged |
| 45 | ( ) line. Review only what changed. |
| 46 | |
| 47 | Rules: |
| 48 | - Report real problems: bugs, correctness risks, security issues, broken |
| 49 | edge cases, misleading names or comments. No praise-only comments, no |
| 50 | restating the diff, no style nitpicks a formatter would catch. |
| 51 | - Each finding must point at an added or context line of the diff, using the |
| 52 | prefixed new-file line number of that file. |
| 53 | - Severity is one of: "critical" (likely bug or security problem), |
| 54 | "warning" (probable problem or risky pattern), "nit" (minor, take or leave). |
| 55 | - At most #{MAX_FINDINGS} findings. Fewer, well-chosen findings beat many |
| 56 | shallow ones. Zero findings is a valid answer. |
| 57 | - Keep every comment terse: one or two sentences, no headings or lists. |
| 58 | - The summary is a short markdown walkthrough of the change: what it does, |
| 59 | how the pieces fit, anything reviewers should look at first. No preamble. |
| 60 | |
| 61 | Respond with exactly one JSON object and nothing else — no markdown fences, |
| 62 | no prose before or after — in this shape: |
| 63 | {"summary": "markdown walkthrough of the change", |
| 64 | "findings": [{"path": "path/to/file", "line": 42, "severity": "warning", "comment": "…"}]} |
| 65 | PROMPT |
| 66 | |
| 67 | class << self |
| 68 | # Splits the PR's files into the set to review and the set to mention as |
| 69 | # skipped: hard exclusions first, then a greedy per-file/total byte budget |
| 70 | # (smallest patches first, so one huge file can't crowd out ten small ones). |
| 71 | # Each entry in `skipped` is [file, reason]. |
| 72 | def select_files(files) |
| 73 | candidates, skipped = Array(files).partition { |f| reviewable?(f) } |
| 74 | skipped = skipped.map { |f| [ f, skip_reason(f) ] } |
| 75 | |
| 76 | selected = [] |
| 77 | budget = MAX_TOTAL_PATCH_BYTES |
| 78 | candidates.sort_by { |f| f["patch"].bytesize }.each do |file| |
| 79 | if file["patch"].bytesize <= budget |
| 80 | selected << file |
| 81 | budget -= file["patch"].bytesize |
| 82 | else |
| 83 | skipped << [ file, "over the total diff budget" ] |
| 84 | end |
| 85 | end |
| 86 | |
| 87 | # Preserve GitHub's file ordering for the prompt and the walkthrough. |
| 88 | selected = files.select { |f| selected.include?(f) } |
| 89 | [ selected, skipped ] |
| 90 | end |
| 91 | |
| 92 | # True when the PR can't be reviewed at all (the job posts a summary-only |
| 93 | # note and marks the review skipped/too_large). |
| 94 | def too_large?(files, selected) |
| 95 | Array(files).size > MAX_FILES || (files.present? && selected.empty?) |
| 96 | end |
| 97 | |
| 98 | def payload(pull_request:, files:, skipped:, model:) |
| 99 | { |
| 100 | "model" => model, |
| 101 | "messages" => [ |
| 102 | { "role" => "system", "content" => SYSTEM_PROMPT }, |
| 103 | { "role" => "user", "content" => user_message(pull_request, files, skipped) } |
| 104 | ], |
| 105 | "stream" => false, |
| 106 | "temperature" => 0.2, |
| 107 | "max_tokens" => 4_096, |
| 108 | # Sent but not depended on — parse_response handles fenced/wrapped JSON. |
| 109 | "response_format" => { "type" => "json_object" } |
| 110 | } |
| 111 | end |
| 112 | |
| 113 | # Parses the model reply into {summary:, findings:}. Malformed individual |
| 114 | # findings are dropped; an unparseable reply raises ParseError (the job |
| 115 | # posts nothing rather than garbled output). |
| 116 | def parse_response(content) |
| 117 | data = lenient_json_parse(content.to_s) |
| 118 | summary = data["summary"] |
| 119 | raise ParseError, "missing summary" unless summary.is_a?(String) && summary.present? |
| 120 | |
| 121 | findings = Array(data["findings"]).filter_map { |finding| normalize_finding(finding) } |
| 122 | { summary: summary, findings: findings.first(MAX_FINDINGS) } |
| 123 | end |
| 124 | |
| 125 | def summary_footer(head_sha) |
| 126 | "\n\n---\n*Automated review by [siGit Code](https://sigit.si/code) · commit #{head_sha.to_s.first(7)}*" |
| 127 | end |
| 128 | |
| 129 | # The summary-only comment for PRs over budget. |
| 130 | def too_large_body(files_count) |
| 131 | "This pull request changes #{files_count} files, which is over the size " \ |
| 132 | "limit for an automated review, so siGit Code skipped it. Splitting " \ |
| 133 | "the change into smaller pull requests will get each part a review." |
| 134 | end |
| 135 | |
| 136 | # ── internals ────────────────────────────────────────────────────────────── |
| 137 | |
| 138 | def reviewable?(file) |
| 139 | file["patch"].present? && !excluded_path?(file["filename"]) && |
| 140 | file["patch"].bytesize <= MAX_FILE_PATCH_BYTES |
| 141 | end |
| 142 | |
| 143 | def excluded_path?(path) |
| 144 | EXCLUDED_PATTERNS.any? { |pattern| pattern.match?(path.to_s) } |
| 145 | end |
| 146 | |
| 147 | def skip_reason(file) |
| 148 | if file["patch"].blank? |
| 149 | "no text diff" |
| 150 | elsif excluded_path?(file["filename"]) |
| 151 | "generated or vendored" |
| 152 | else |
| 153 | "diff too large" |
| 154 | end |
| 155 | end |
| 156 | |
| 157 | def user_message(pull_request, files, skipped) |
| 158 | parts = [] |
| 159 | parts << "Pull request: #{pull_request["title"]}" |
| 160 | parts << "Branches: #{pull_request.dig("base", "ref")} <- #{pull_request.dig("head", "ref")}" |
| 161 | body = pull_request["body"].to_s.strip |
| 162 | parts << "Description:\n#{body.truncate(MAX_PR_BODY_CHARS)}" if body.present? |
| 163 | |
| 164 | if skipped.any? |
| 165 | notes = skipped.map { |file, reason| "- #{file["filename"]} (#{reason})" } |
| 166 | parts << "Changed but not shown:\n#{notes.join("\n")}" |
| 167 | end |
| 168 | |
| 169 | diffs = files.map do |file| |
| 170 | header = "#{file["filename"]} (#{file["status"]}, +#{file["additions"]}/-#{file["deletions"]})" |
| 171 | "### #{header}\n```diff\n#{GithubPatchIndex.annotate_patch(file["patch"])}\n```" |
| 172 | end |
| 173 | parts << "Changed files:\n\n#{diffs.join("\n\n")}" |
| 174 | |
| 175 | parts.join("\n\n") |
| 176 | end |
| 177 | |
| 178 | def lenient_json_parse(content) |
| 179 | text = content.strip |
| 180 | text = text.delete_prefix("```json").delete_prefix("```").delete_suffix("```").strip |
| 181 | |
| 182 | begin |
| 183 | data = JSON.parse(text) |
| 184 | rescue JSON::ParserError |
| 185 | start, finish = text.index("{"), text.rindex("}") |
| 186 | raise ParseError, "no JSON object in model reply" if start.nil? || finish.nil? || finish <= start |
| 187 | |
| 188 | begin |
| 189 | data = JSON.parse(text[start..finish]) |
| 190 | rescue JSON::ParserError => e |
| 191 | raise ParseError, "invalid JSON in model reply: #{e.message}" |
| 192 | end |
| 193 | end |
| 194 | |
| 195 | raise ParseError, "model reply is not a JSON object" unless data.is_a?(Hash) |
| 196 | |
| 197 | data |
| 198 | end |
| 199 | |
| 200 | def normalize_finding(finding) |
| 201 | return nil unless finding.is_a?(Hash) |
| 202 | |
| 203 | path = finding["path"] |
| 204 | comment = finding["comment"] |
| 205 | line = finding["line"] |
| 206 | line = Integer(line, exception: false) if line.is_a?(String) |
| 207 | return nil unless path.is_a?(String) && path.present? |
| 208 | return nil unless comment.is_a?(String) && comment.present? |
| 209 | return nil unless line.is_a?(Integer) && line.positive? |
| 210 | |
| 211 | severity = SEVERITIES.include?(finding["severity"]) ? finding["severity"] : "warning" |
| 212 | { path: path, line: line, severity: severity, comment: comment } |
| 213 | end |
| 214 | |
| 215 | private :reviewable?, :excluded_path?, :skip_reason, :user_message, |
| 216 | :lenient_json_parse, :normalize_finding |
| 217 | end |
| 218 | end |