| 1 | # frozen_string_literal: true |
| 2 | |
| 3 | require "rails_helper" |
| 4 | |
| 5 | RSpec.describe GithubReviewPrompt do |
| 6 | def file(name, patch: "@@ -1,1 +1,2 @@\n a\n+b", **extra) |
| 7 | { "filename" => name, "status" => "modified", "additions" => 1, "deletions" => 0, |
| 8 | "patch" => patch }.merge(extra) |
| 9 | end |
| 10 | |
| 11 | describe ".select_files" do |
| 12 | it "keeps normal source files and reports nothing skipped" do |
| 13 | selected, skipped = described_class.select_files([ file("app/a.rb") ]) |
| 14 | expect(selected.map { |f| f["filename"] }).to eq([ "app/a.rb" ]) |
| 15 | expect(skipped).to be_empty |
| 16 | end |
| 17 | |
| 18 | it "excludes lockfiles, vendored paths, minified assets, and db/schema.rb" do |
| 19 | files = [ |
| 20 | file("Gemfile.lock"), file("sub/dir/Cargo.lock"), file("vendor/lib.rb"), |
| 21 | file("node_modules/x/index.js"), file("app.min.js"), file("db/schema.rb"), |
| 22 | file("app/real.rb") |
| 23 | ] |
| 24 | selected, skipped = described_class.select_files(files) |
| 25 | |
| 26 | expect(selected.map { |f| f["filename"] }).to eq([ "app/real.rb" ]) |
| 27 | expect(skipped.map { |f, reason| [ f["filename"], reason ] }) |
| 28 | .to all(satisfy { |_name, reason| reason == "generated or vendored" }) |
| 29 | expect(skipped.size).to eq(6) |
| 30 | end |
| 31 | |
| 32 | it "skips binary files (no patch)" do |
| 33 | _, skipped = described_class.select_files([ file("logo.png", patch: nil) ]) |
| 34 | expect(skipped).to eq([ [ file("logo.png", patch: nil), "no text diff" ] ]) |
| 35 | end |
| 36 | |
| 37 | it "skips a single file over the per-file budget" do |
| 38 | big = file("big.rb", patch: "@@ -1,1 +1,2 @@\n a\n+#{"x" * described_class::MAX_FILE_PATCH_BYTES}") |
| 39 | _, skipped = described_class.select_files([ big ]) |
| 40 | expect(skipped.first.last).to eq("diff too large") |
| 41 | end |
| 42 | |
| 43 | it "drops the largest files first when the total budget is exceeded" do |
| 44 | chunk = "@@ -1,1 +1,2 @@\n a\n+#{"y" * 11_000}" |
| 45 | files = (1..9).map { |i| file("f#{i}.rb", patch: chunk) } |
| 46 | selected, skipped = described_class.select_files(files) |
| 47 | |
| 48 | expect(selected.sum { |f| f["patch"].bytesize }).to be <= described_class::MAX_TOTAL_PATCH_BYTES |
| 49 | expect(selected.size + skipped.size).to eq(9) |
| 50 | expect(skipped.map(&:last)).to all(eq("over the total diff budget")) |
| 51 | end |
| 52 | |
| 53 | it "preserves GitHub's file ordering in the selection" do |
| 54 | files = [ file("z.rb"), file("a.rb") ] |
| 55 | selected, = described_class.select_files(files) |
| 56 | expect(selected.map { |f| f["filename"] }).to eq([ "z.rb", "a.rb" ]) |
| 57 | end |
| 58 | end |
| 59 | |
| 60 | describe ".too_large?" do |
| 61 | it "is true over the file-count cap" do |
| 62 | files = (0..described_class::MAX_FILES).map { |i| file("f#{i}.rb") } |
| 63 | selected, = described_class.select_files(files) |
| 64 | expect(described_class.too_large?(files, selected)).to be true |
| 65 | end |
| 66 | |
| 67 | it "is true when nothing survives selection" do |
| 68 | files = [ file("Gemfile.lock") ] |
| 69 | expect(described_class.too_large?(files, [])).to be true |
| 70 | end |
| 71 | |
| 72 | it "is false for a reviewable PR" do |
| 73 | files = [ file("a.rb") ] |
| 74 | expect(described_class.too_large?(files, files)).to be false |
| 75 | end |
| 76 | end |
| 77 | |
| 78 | describe ".payload" do |
| 79 | let(:pull_request) do |
| 80 | { "title" => "Fix login", "body" => "Corrects the token check.", |
| 81 | "base" => { "ref" => "main" }, "head" => { "ref" => "fix-login", "sha" => "abc1234" } } |
| 82 | end |
| 83 | |
| 84 | it "builds a non-streaming chat payload with annotated diffs" do |
| 85 | payload = described_class.payload( |
| 86 | pull_request: pull_request, files: [ file("app/a.rb") ], |
| 87 | skipped: [ [ file("Gemfile.lock"), "generated or vendored" ] ], model: "onde-large" |
| 88 | ) |
| 89 | |
| 90 | expect(payload["model"]).to eq("onde-large") |
| 91 | expect(payload["stream"]).to be false |
| 92 | system_message, user_message = payload["messages"] |
| 93 | expect(system_message["content"]).to include("siGit Code") |
| 94 | expect(user_message["content"]).to include("Fix login") |
| 95 | .and include("main <- fix-login") |
| 96 | .and include("Gemfile.lock (generated or vendored)") |
| 97 | .and include(" 2 +b") # annotated new-file line number |
| 98 | end |
| 99 | end |
| 100 | |
| 101 | describe ".parse_response" do |
| 102 | let(:valid_json) do |
| 103 | '{"summary": "Adds a thing.", "findings": [{"path": "a.rb", "line": 2, "severity": "warning", "comment": "Check nil."}]}' |
| 104 | end |
| 105 | |
| 106 | it "parses a clean JSON reply" do |
| 107 | parsed = described_class.parse_response(valid_json) |
| 108 | expect(parsed[:summary]).to eq("Adds a thing.") |
| 109 | expect(parsed[:findings]).to eq([ { path: "a.rb", line: 2, severity: "warning", comment: "Check nil." } ]) |
| 110 | end |
| 111 | |
| 112 | it "parses a fenced reply" do |
| 113 | parsed = described_class.parse_response("```json\n#{valid_json}\n```") |
| 114 | expect(parsed[:summary]).to eq("Adds a thing.") |
| 115 | end |
| 116 | |
| 117 | it "parses a prose-wrapped reply" do |
| 118 | parsed = described_class.parse_response("Here is my review:\n#{valid_json}\nHope that helps!") |
| 119 | expect(parsed[:summary]).to eq("Adds a thing.") |
| 120 | end |
| 121 | |
| 122 | it "raises ParseError on garbage" do |
| 123 | expect { described_class.parse_response("no json here") } |
| 124 | .to raise_error(described_class::ParseError) |
| 125 | end |
| 126 | |
| 127 | it "raises ParseError when the summary is missing" do |
| 128 | expect { described_class.parse_response('{"findings": []}') } |
| 129 | .to raise_error(described_class::ParseError, /summary/) |
| 130 | end |
| 131 | |
| 132 | it "drops malformed findings, coerces string lines, defaults bad severities" do |
| 133 | reply = { |
| 134 | "summary" => "ok", |
| 135 | "findings" => [ |
| 136 | { "path" => "a.rb", "line" => "3", "severity" => "silly", "comment" => "x" }, |
| 137 | { "path" => "", "line" => 1, "comment" => "no path" }, |
| 138 | { "path" => "a.rb", "line" => 0, "comment" => "bad line" }, |
| 139 | { "path" => "a.rb", "line" => 1 }, |
| 140 | "not even a hash" |
| 141 | ] |
| 142 | }.to_json |
| 143 | |
| 144 | findings = described_class.parse_response(reply)[:findings] |
| 145 | expect(findings).to eq([ { path: "a.rb", line: 3, severity: "warning", comment: "x" } ]) |
| 146 | end |
| 147 | |
| 148 | it "caps findings at MAX_FINDINGS" do |
| 149 | many = (1..20).map { |i| { "path" => "a.rb", "line" => i, "comment" => "c#{i}" } } |
| 150 | findings = described_class.parse_response({ "summary" => "ok", "findings" => many }.to_json)[:findings] |
| 151 | expect(findings.size).to eq(described_class::MAX_FINDINGS) |
| 152 | end |
| 153 | end |
| 154 | |
| 155 | describe ".summary_footer" do |
| 156 | it "brands the review and cites the short SHA" do |
| 157 | footer = described_class.summary_footer("abcdef0123456789") |
| 158 | expect(footer).to include("siGit Code").and include("abcdef0") |
| 159 | end |
| 160 | end |
| 161 | end |