Fix PR lens review: inline comments + split confidence thresholds - #147
Merged
Conversation
The "EvalOps PR lens review" system published 0 findings across the last
12 scheduled runs (~70 lens reviews) yet reported success every time.
Three bugs combined to make it silently green:
1. One blunt threshold (DEFAULT_MIN_CONFIDENCE = 0.82) decided both what
to show a human and what fails the status, so real medium-confidence
findings (0.6-0.8) were silently discarded.
2. Findings posted as one issue comment at the bottom of the PR with
`path:line` as plain text, despite every finding carrying an exact
code_location.
3. normalize_finding silently returned nil for malformed findings.
Changes:
- Publish as a single PR review (POST pulls/{pr}/reviews, event COMMENT)
with inline comments anchored to each finding's code_location.
meta-review parses each file's patch hunks to compute the set of
addable right-side line numbers; findings whose path:line is in that
set are inlined, the rest are folded into the review summary body
(avoids GitHub's 422 on off-diff lines).
- Split the threshold in two: comment_min_confidence (default 0.55,
env PR_LENS_COMMENT_MIN_CONFIDENCE) for surfacing; block_min_confidence
(default 0.80, env PR_LENS_BLOCK_MIN_CONFIDENCE) for failing the
status, and only on P0/P1. PR_LENS_MIN_CONFIDENCE / --min-confidence
kept as a back-compat alias mapping to the block threshold.
- Honest green status: "N lenses · 0 findings >= 0.55" instead of
implying nothing was found.
- normalize_finding now raises DroppedFinding; the caller counts and
warns per dropped finding and records dropped_findings in the ledger.
- Idempotency preserved: prior marker issue-comment and prior marker
inline review comments are deleted before posting, so re-running on
the same head replaces rather than duplicates.
- Workflow gains comment_min_confidence / block_min_confidence inputs
and keeps min_confidence for back-compat.
Test Plan:
- ruby -Itest -e 'ARGV.each { |path| require "./#{path}" }' test/*_test.rb
=> 125 runs, 1016 assertions, 0 failures, 0 errors, 0 skips
- New coverage: diff-hunk line parsing, inline-vs-summary split, the two
thresholds, idempotent replacement, dropped-finding logging.
Rollback: revert this commit; behavior returns to the single-threshold
issue-comment publisher. No state migration involved.
Signal: 12 consecutive green scheduled runs with 0 published findings
(gh run list evalops-pr-lens-review.yml, 2026-06-08..09).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PR SummaryMedium Risk Overview Publication replaces Thresholds split into Observability: malformed findings raise Workflow inputs/env and extensive tests cover pagination, anchoring, publication order, and thresholds. Reviewed by Cursor Bugbot for commit 85ae9c0. Bugbot is set up for automated code reviews on this repo. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Stale diff for inline anchors
meta_reviewnow disables inline publication and routes findings to the summary when the live PR head no longer matches the artifact head SHA.
- ✅ Fixed: Inline failure drops summary
publish_reviewnow rolls back partial inline comments, warns, and still publishes a summary-only fallback instead of aborting the entire publication.
Preview (f0c265aaff)
diff --git a/.github/scripts/evalops-pr-lens-review.rb b/.github/scripts/evalops-pr-lens-review.rb
--- a/.github/scripts/evalops-pr-lens-review.rb
+++ b/.github/scripts/evalops-pr-lens-review.rb
@@ -9,6 +9,7 @@
require "openssl"
require "open3"
require "optparse"
+require "set"
require "time"
require "uri"
require "yaml"
@@ -123,7 +124,15 @@
MARKER = "<!-- evalops-pr-lens-review -->"
REVIEW_REQUESTED_DISPATCH_EVENT = "evalopsbot-review-requested"
REVIEW_REQUESTED_DISPATCH_SOURCE = "evalopsbot-review-request-dispatch"
- DEFAULT_MIN_CONFIDENCE = 0.82
+ # Findings at or above this confidence are surfaced to humans (inline or in the
+ # review summary). Lower than the historical 0.82 so real medium-confidence
+ # findings stop getting silently discarded.
+ DEFAULT_COMMENT_MIN_CONFIDENCE = 0.55
+ # Only P0/P1 findings at or above this confidence flip the meta-review status to
+ # failure. Showing a finding and blocking on it are now separate decisions.
+ DEFAULT_BLOCK_MIN_CONFIDENCE = 0.80
+ # Back-compat: the single legacy knob now maps to the block threshold.
+ DEFAULT_MIN_CONFIDENCE = DEFAULT_BLOCK_MIN_CONFIDENCE
DEFAULT_MODEL = "claude-opus-4-7"
DEFAULT_PROVIDER = "anthropic"
DEFAULT_MAX_DIFF_BYTES = 180_000
@@ -420,10 +429,71 @@
summary
end
+ def gh_api_paginated_json(*args, input: nil, token: ENV["GH_TOKEN"])
+ raw = gh_api("--paginate", "--slurp", *args, input: input, token: token)
+ return [] if raw.strip.empty?
+
+ JSON.parse(raw)
+ end
+
def pr_files_metadata(repo:, pr:)
- gh_api_json("repos/#{repo}/pulls/#{pr}/files?per_page=100")
+ Array(gh_api_paginated_json("repos/#{repo}/pulls/#{pr}/files?per_page=100")).flat_map { |page| Array(page) }
end
+ # Parse a single file's unified-diff patch (as returned by the GitHub files API)
+ # into the set of right-side (head) line numbers that an inline review comment
+ # may anchor to. GitHub only accepts inline comments on added or context lines
+ # that are part of the diff; commenting elsewhere returns HTTP 422.
+ def addable_lines_from_patch(patch)
+ lines = Set.new
+ right_line = nil
+ patch.to_s.each_line do |raw|
+ line = raw.chomp
+ if (match = line.match(/\A@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/))
+ right_line = Integer(match[1])
+ next
+ end
+ next if right_line.nil?
+
+ case line[0]
+ when "+"
+ lines << right_line
+ right_line += 1
+ when " ", ""
+ # Context line counts on the right side but is not itself an addition.
+ right_line += 1
+ when "-"
+ # Deletion only advances the left side.
+ when "\\"
+ # "\ No newline at end of file" marker; does not advance either side.
+ else
+ right_line += 1
+ end
+ end
+ lines
+ end
+
+ # Map of head-side path => Set of addable line numbers for the PR diff. Used to
+ # decide which findings can be posted as inline review comments versus folded
+ # into the review summary body.
+ def addable_lines_by_path(repo:, pr:, files: nil)
+ files ||= pr_files_metadata(repo: repo, pr: pr)
+ Array(files).each_with_object({}) do |file, map|
+ path = file["filename"].to_s
+ next if path.empty?
+
+ map[path] = addable_lines_from_patch(file["patch"])
+ end
+ end
+
+ def finding_inline_anchorable?(finding, addable_by_path)
+ location = finding.fetch("code_location")
+ addable = addable_by_path[location.fetch("path")]
+ return false if addable.nil?
+
+ addable.include?(Integer(location.fetch("line")))
+ end
+
def discover_open_prs(repos:, pr_filter: nil, force_lenses: nil)
repos.flat_map do |repo|
normalized_repo = normalize_repo(repo)
@@ -709,7 +779,7 @@
end
def pr_file_summary(repo:, pr:)
- files = gh_api_json("repos/#{repo}/pulls/#{pr}/files?per_page=100")
+ files = pr_files_metadata(repo: repo, pr: pr)
files.map do |file|
[
file.fetch("status"),
@@ -963,12 +1033,36 @@
"line" => Integer(line)
}
}
- rescue ArgumentError, KeyError, TypeError
- nil
+ rescue ArgumentError, KeyError, TypeError => e
+ raise DroppedFinding, e.message
end
+ # Raised by normalize_finding when a malformed finding cannot be normalized,
+ # so the caller can count and log the drop instead of silently swallowing it.
+ class DroppedFinding < StandardError; end
+
+ def normalize_findings_with_drops(raw_findings, repo:, pr:, lens:)
+ dropped = 0
+ findings = Array(raw_findings).each_with_index.filter_map do |finding, index|
+ normalize_finding(finding)
+ rescue DroppedFinding => e
+ dropped += 1
+ warn(
+ "pr-lens: dropped malformed finding ##{index} from #{repo}##{pr} #{lens}: " \
+ "#{e.message.lines.first.to_s.strip}"
+ )
+ nil
+ end
+ [findings, dropped]
+ end
+
def normalize_lens_review(raw_review, repo:, pr:, lens:, head_sha:)
- findings = Array(raw_review["findings"]).map { |finding| normalize_finding(finding) }.compact
+ findings, dropped = normalize_findings_with_drops(
+ raw_review["findings"],
+ repo: repo,
+ pr: pr,
+ lens: lens
+ )
top_confidence = findings.map { |finding| finding.fetch("confidence_score") }.max || 0.0
confidence = coerce_number(
raw_review.fetch("confidence_score", top_confidence),
@@ -987,6 +1081,7 @@
"generated_at" => Time.now.utc.iso8601,
"summary" => raw_review.fetch("summary", "").to_s.strip,
"confidence_score" => confidence,
+ "dropped_findings" => dropped,
"findings" => findings
}
end
@@ -1169,29 +1264,82 @@
"#{server}/#{repo}/actions/runs/#{run_id}"
end
- def comment_body(repo:, pr:, findings:, min_confidence:, target_url:)
+ def finding_inline_comment_body(finding)
+ [
+ MARKER,
+ "**P#{finding.fetch("priority")} · #{format("%.2f", finding.fetch("confidence_score"))} · #{finding.fetch("lens")}**: #{finding.fetch("title")}",
+ "",
+ finding.fetch("body"),
+ "",
+ "_Check: `#{finding.fetch("check_id")}`_"
+ ].join("\n")
+ end
+
+ def append_summary_findings(lines, title, findings, omission_label:)
+ return if findings.empty?
+
+ lines << title
+ findings.first(MAX_FINDINGS_PER_COMMENT).each do |finding|
+ location = finding.fetch("code_location")
+ lines << "- **P#{finding.fetch("priority")} #{format("%.2f", finding.fetch("confidence_score"))} #{finding.fetch("lens")}** `#{location.fetch("path")}:#{location.fetch("line")}`: #{finding.fetch("title")}"
+ lines << " - #{finding.fetch("body")}"
+ end
+ if findings.length > MAX_FINDINGS_PER_COMMENT
+ lines << "- _#{findings.length - MAX_FINDINGS_PER_COMMENT} additional #{omission_label} omitted; inspect the workflow artifact for the full ledger._"
+ end
+ lines << ""
+ end
+
+ # Build the summary comment body. `inline_findings` are anchored to the diff as
+ # individual comments. If there are more anchorable findings than the inline
+ # comment cap allows, `overflow_inline_findings` are listed in the summary with
+ # their locations. `failed_inline_findings` are diff findings listed here when
+ # inline publication fails after selection. `summary_findings` could not be
+ # anchored because their line is not part of the diff and are also listed here
+ # with path:line.
+ def review_summary_body(
+ repo:,
+ pr:,
+ inline_findings:,
+ overflow_inline_findings:,
+ summary_findings:,
+ comment_min_confidence:,
+ target_url:,
+ failed_inline_findings: []
+ )
+ total = inline_findings.length + overflow_inline_findings.length + summary_findings.length + failed_inline_findings.length
lines = [
MARKER,
"**EvalOps PR lens review**",
"",
- "High-confidence findings only. Threshold: #{format("%.2f", min_confidence)}.",
+ "#{total} finding#{total == 1 ? "" : "s"} ≥ #{format("%.2f", comment_min_confidence)} confidence.",
"Run: #{target_url || "unavailable"}",
""
]
- findings.first(MAX_FINDINGS_PER_COMMENT).each_with_index do |finding, index|
- location = finding.fetch("code_location")
- lines << "#{index + 1}. **P#{finding.fetch("priority")} #{format("%.2f", finding.fetch("confidence_score"))} #{finding.fetch("lens")}**: #{finding.fetch("title")}"
- lines << " - Location: `#{location.fetch("path")}:#{location.fetch("line")}`"
- lines << " - Check: `#{finding.fetch("check_id")}`"
- lines << " - #{finding.fetch("body")}"
+ unless inline_findings.empty?
+ lines << "#{inline_findings.length} anchored inline below."
lines << ""
end
- if findings.length > MAX_FINDINGS_PER_COMMENT
- lines << "_#{findings.length - MAX_FINDINGS_PER_COMMENT} additional high-confidence finding(s) were omitted from the comment; inspect the workflow artifact for the full ledger._"
- lines << ""
- end
+ append_summary_findings(
+ lines,
+ "Diff findings (inline publication failed, so listed here):",
+ failed_inline_findings,
+ omission_label: "diff finding(s)"
+ )
+ append_summary_findings(
+ lines,
+ "Additional diff findings (not posted inline due to the #{MAX_FINDINGS_PER_COMMENT}-comment cap):",
+ overflow_inline_findings,
+ omission_label: "diff finding(s)"
+ )
+ append_summary_findings(
+ lines,
+ "Findings outside the diff (not inline-anchorable):",
+ summary_findings,
+ omission_label: "finding(s)"
+ )
lines << "_Repo: #{repo} PR: ##{pr}_"
lines.join("\n")
@@ -1207,54 +1355,172 @@
raw.lines.map(&:strip).reject(&:empty?)
end
- def upsert_comment(repo:, pr:, body:)
- ids = marker_comment_ids(repo: repo, pr: pr)
- if ids.empty?
- gh_api(
- "--method", "POST", "repos/#{repo}/issues/#{pr}/comments",
- input: JSON.generate({ body: body })
+ def delete_marker_comments(repo:, pr:, ids: nil)
+ Array(ids || marker_comment_ids(repo: repo, pr: pr)).each do |id|
+ gh_api("--method", "DELETE", "repos/#{repo}/issues/comments/#{id}")
+ end
+ end
+
+ # Prior bot inline review comments carrying the marker, on this PR. Deleted
+ # before a fresh review is posted so re-running on the same head replaces
+ # rather than duplicates.
+ def marker_review_comment_ids(repo:, pr:)
+ raw = gh_api(
+ "--paginate",
+ "repos/#{repo}/pulls/#{pr}/comments",
+ "--jq",
+ ".[] | select(.body | contains(\"#{MARKER}\")) | .id"
+ )
+ raw.lines.map(&:strip).reject(&:empty?)
+ end
+
+ def delete_marker_review_comments(repo:, pr:, ids: nil)
+ Array(ids || marker_review_comment_ids(repo: repo, pr: pr)).each do |id|
+ gh_api("--method", "DELETE", "repos/#{repo}/pulls/comments/#{id}")
+ end
+ end
+
+ def post_summary_comment(repo:, pr:, body:)
+ gh_api_json(
+ "--method", "POST", "repos/#{repo}/issues/#{pr}/comments",
+ input: JSON.generate(body: body)
+ )
+ end
+
+ def post_inline_comment(repo:, pr:, head_sha:, finding:)
+ location = finding.fetch("code_location")
+ gh_api_json(
+ "--method", "POST", "repos/#{repo}/pulls/#{pr}/comments",
+ input: JSON.generate(
+ commit_id: head_sha,
+ path: location.fetch("path"),
+ line: Integer(location.fetch("line")),
+ side: "RIGHT",
+ body: finding_inline_comment_body(finding)
)
- else
- first, *stale = ids
- gh_api(
- "--method", "PATCH", "repos/#{repo}/issues/comments/#{first}",
- input: JSON.generate({ body: body })
+ )
+ end
+
+ # Remove the bot's prior published artifacts for this PR (marker issue-comment
+ # left by the legacy code path, and prior marker inline review comments) so the
+ # publication is idempotent across re-runs on the same head.
+ def clear_prior_publication(repo:, pr:)
+ delete_marker_comments(repo: repo, pr: pr)
+ delete_marker_review_comments(repo: repo, pr: pr)
+ end
+
+ def rollback_publication(repo:, pr:, summary_comment_id:, inline_comment_ids:)
+ delete_marker_review_comments(repo: repo, pr: pr, ids: inline_comment_ids.reverse)
+ delete_marker_comments(repo: repo, pr: pr, ids: [summary_comment_id].compact)
+ rescue StandardError => e
+ warn "pr-lens: failed to roll back partial publication for #{repo}##{pr}: #{e.message.lines.first.to_s.strip}"
+ end
+
+ # Publish findings as a marker summary issue comment plus inline PR comments
+ # anchored to each anchorable finding's code_location. Findings whose line is
+ # not in the diff, or that overflow the inline comment cap, are folded into the
+ # summary body. Prior marker comments are deleted only after the replacement
+ # publication succeeds so a partial API failure does not leave a misleading
+ # summary or erase the prior review. If inline publication fails after findings
+ # are selected, the script falls back to a summary-only publication so off-diff
+ # and overflow findings are still published for the run.
+ def publish_review(repo:, pr:, head_sha:, inline_findings:, summary_findings:, comment_min_confidence:, target_url:)
+ if inline_findings.empty? && summary_findings.empty?
+ clear_prior_publication(repo: repo, pr: pr)
+ return
+ end
+
+ inline_to_publish = inline_findings.first(MAX_FINDINGS_PER_COMMENT)
+ overflow_inline_findings = inline_findings.drop(MAX_FINDINGS_PER_COMMENT)
+ prior_summary_ids = marker_comment_ids(repo: repo, pr: pr)
+ prior_inline_ids = marker_review_comment_ids(repo: repo, pr: pr)
+ published_inline_ids = []
+ published_summary_id = nil
+ failed_inline_findings = []
+
+ begin
+ inline_to_publish.each do |finding|
+ response = post_inline_comment(repo: repo, pr: pr, head_sha: head_sha, finding: finding)
+ published_inline_ids << response["id"] if response && response["id"]
+ end
+ rescue StandardError => e
+ rollback_publication(
+ repo: repo,
+ pr: pr,
+ summary_comment_id: nil,
+ inline_comment_ids: published_inline_ids
)
- stale.each { |id| gh_api("--method", "DELETE", "repos/#{repo}/issues/comments/#{id}") }
+ warn(
+ "pr-lens: inline publication failed for #{repo}##{pr} @ #{head_sha}; " \
+ "publishing summary only: #{e.message.lines.first.to_s.strip}"
+ )
+ failed_inline_findings = inline_to_publish + overflow_inline_findings
+ inline_to_publish = []
+ overflow_inline_findings = []
+ published_inline_ids = []
end
+
+ published_summary_id = post_summary_comment(
+ repo: repo,
+ pr: pr,
+ body: review_summary_body(
+ repo: repo,
+ pr: pr,
+ inline_findings: inline_to_publish,
+ overflow_inline_findings: overflow_inline_findings,
+ summary_findings: summary_findings,
+ comment_min_confidence: comment_min_confidence,
+ target_url: target_url,
+ failed_inline_findings: failed_inline_findings
+ )
+ )
+ published_summary_id = published_summary_id["id"] if published_summary_id
+ delete_marker_comments(repo: repo, pr: pr, ids: prior_summary_ids)
+ delete_marker_review_comments(repo: repo, pr: pr, ids: prior_inline_ids)
+ rescue StandardError
+ rollback_publication(
+ repo: repo,
+ pr: pr,
+ summary_comment_id: published_summary_id,
+ inline_comment_ids: published_inline_ids
+ )
+ raise
end
- def delete_marker_comments(repo:, pr:)
- marker_comment_ids(repo: repo, pr: pr).each do |id|
- gh_api("--method", "DELETE", "repos/#{repo}/issues/comments/#{id}")
+ def blocking_findings(findings, block_min_confidence:)
+ findings.select do |finding|
+ finding.fetch("priority") <= 1 &&
+ finding.fetch("confidence_score") >= block_min_confidence
end
end
- def meta_state(findings, coverage_incomplete: false)
+ def meta_state(findings, block_min_confidence:, coverage_incomplete: false)
return "error" if coverage_incomplete
- findings.any? { |finding| finding.fetch("priority") <= 1 } ? "failure" : "success"
+ blocking_findings(findings, block_min_confidence: block_min_confidence).any? ? "failure" : "success"
end
- def meta_description(findings, missing_count: 0, skipped_count: 0)
+ def meta_description(findings, lens_count:, comment_min_confidence:, missing_count: 0, skipped_count: 0)
+ confidence_label = format("%.2f", comment_min_confidence)
+ lens_label = "#{lens_count} lens#{lens_count == 1 ? "" : "es"}"
+
if missing_count.positive?
"PR lens coverage incomplete: #{missing_count} missing"
- elsif findings.empty? && skipped_count.positive?
- "No findings; #{skipped_count} lens review#{skipped_count == 1 ? "" : "s"} skipped"
elsif findings.empty?
- "No high-confidence PR lens findings"
+ base = "#{lens_label} · 0 findings ≥ #{confidence_label}"
+ skipped_count.positive? ? "#{base} (#{skipped_count} skipped)" : base
else
- "#{findings.length} high-confidence finding#{findings.length == 1 ? "" : "s"}"
+ "#{lens_label} · #{findings.length} finding#{findings.length == 1 ? "" : "s"} ≥ #{confidence_label}"
end
end
- def meta_review(artifact_root:, min_confidence:, output:)
+ def meta_review(artifact_root:, comment_min_confidence:, block_min_confidence:, output:)
reviews = read_lens_reviews(artifact_root)
expected_reviews = read_expected_reviews(artifact_root)
reviews_by_key = reviews.each_with_object({}) do |review, hash|
hash[[review.fetch("repo"), Integer(review.fetch("pr")), review.fetch("lens"), review.fetch("head_sha")]] = review
end
- ranked = dedupe_and_rank(high_confidence_findings(reviews, min_confidence: min_confidence))
+ ranked = dedupe_and_rank(high_confidence_findings(reviews, min_confidence: comment_min_confidence))
grouped = grouped_by_pr(ranked)
target_url = run_url
coverage_by_pr = {}
@@ -1285,22 +1551,48 @@
"lenses" => reviews.select { |review| review.fetch("repo") == repo && review.fetch("pr") == pr && review.fetch("head_sha") == head_sha }.map { |review| review.fetch("lens") }.sort
}
)
+
if findings.empty?
- delete_marker_comments(repo: repo, pr: pr)
+ clear_prior_publication(repo: repo, pr: pr)
else
- upsert_comment(
+ current_head_sha = pr_head_sha(repo: repo, pr: pr)
+ if current_head_sha != head_sha
+ warn(
+ "pr-lens: skipping inline publication for #{repo}##{pr} because PR head advanced " \
+ "from #{head_sha} to #{current_head_sha}"
+ )
+ inline_findings = []
+ summary_findings = findings
+ else
+ addable_by_path = addable_lines_by_path(repo: repo, pr: pr)
+ inline_findings, summary_findings = findings.partition do |finding|
+ finding_inline_anchorable?(finding, addable_by_path)
+ end
+ end
+ publish_review(
repo: repo,
pr: pr,
- body: comment_body(repo: repo, pr: pr, findings: findings, min_confidence: min_confidence, target_url: target_url)
+ head_sha: head_sha,
+ inline_findings: inline_findings,
+ summary_findings: summary_findings,
+ comment_min_confidence: comment_min_confidence,
+ target_url: target_url
)
end
+
post_status(
repo: repo,
sha: head_sha,
context: meta_context,
- state: meta_state(findings, coverage_incomplete: coverage.fetch("missing").positive?),
+ state: meta_state(
+ findings,
+ block_min_confidence: block_min_confidence,
+ coverage_incomplete: coverage.fetch("missing").positive?
+ ),
description: meta_description(
findings,
+ lens_count: Array(coverage.fetch("lenses", [])).length,
+ comment_min_confidence: comment_min_confidence,
missing_count: coverage.fetch("missing"),
skipped_count: coverage.fetch("skipped")
),
@@ -1311,13 +1603,16 @@
result = {
"schema_version" => 1,
"generated_at" => Time.now.utc.iso8601,
- "min_confidence" => min_confidence,
+ "comment_min_confidence" => comment_min_confidence,
+ "block_min_confidence" => block_min_confidence,
"reviews" => reviews.length,
"expected_reviews" => expected_reviews.length,
+ "dropped_findings" => reviews.sum { |review| Integer(review.fetch("dropped_findings", 0)) },
"coverage" => coverage_by_pr.map do |(repo, pr, head_sha), coverage|
coverage.merge("repo" => repo, "pr" => pr, "head_sha" => head_sha)
end,
"published_findings" => ranked,
+ "blocking_findings" => blocking_findings(ranked, block_min_confidence: block_min_confidence),
"run_url" => target_url
}
File.write(output, JSON.pretty_generate(result))
@@ -1325,12 +1620,16 @@
end
def markdown_meta_report(result)
+ comment_threshold = format("%.2f", result.fetch("comment_min_confidence", DEFAULT_COMMENT_MIN_CONFIDENCE))
+ block_threshold = format("%.2f", result.fetch("block_min_confidence", DEFAULT_BLOCK_MIN_CONFIDENCE))
lines = [
"## EvalOps PR Lens Review",
"",
"- Reviews: #{result.fetch("reviews")}",
"- Expected reviews: #{result.fetch("expected_reviews")}",
- "- Published findings: #{result.fetch("published_findings").length}",
+ "- Comment threshold: #{comment_threshold} · Block threshold: #{block_threshold}",
+ "- Published findings: #{result.fetch("published_findings").length} (#{result.fetch("blocking_findings", []).length} blocking)",
+ "- Dropped malformed findings: #{result.fetch("dropped_findings", 0)}",
"- Run: #{result.fetch("run_url") || "unavailable"}",
"",
"### Coverage"
@@ -1340,7 +1639,7 @@
end
if result.fetch("published_findings", []).empty?
lines << ""
- lines << "No high-confidence findings cleared the publication threshold."
+ lines << "No findings cleared the comment threshold (#{comment_threshold})."
else
lines << ""
lines << "### Findings"
@@ -1458,22 +1757,41 @@
review = JSON.parse(File.read(options.fetch(:review_json)))
puts EvalOpsPrLensReview.lens_status_description(review)
when "meta-review"
+ # Back-compat: PR_LENS_MIN_CONFIDENCE (and --min-confidence) is the legacy
+ # single knob and now maps to the *block* threshold. The comment threshold
+ # defaults lower so medium-confidence findings are still shown.
+ legacy_block = ENV["PR_LENS_MIN_CONFIDENCE"]
+ legacy_block = nil if legacy_block.to_s.empty?
options = {
- min_confidence: Float(ENV.fetch("PR_LENS_MIN_CONFIDENCE", EvalOpsPrLensReview::DEFAULT_MIN_CONFIDENCE)),
+ comment_min_confidence: Float(
+ ENV.fetch("PR_LENS_COMMENT_MIN_CONFIDENCE", EvalOpsPrLensReview::DEFAULT_COMMENT_MIN_CONFIDENCE)
+ ),
+ block_min_confidence: Float(
+ ENV.fetch(
+ "PR_LENS_BLOCK_MIN_CONFIDENCE",
+ legacy_block || EvalOpsPrLensReview::DEFAULT_BLOCK_MIN_CONFIDENCE
+ )
+ ),
output: "meta-review.json"
}
+ markdown_output = nil
OptionParser.new do |parser|
parser.on("--artifact-root PATH") { |value| options[:artifact_root] = value }
- parser.on("--min-confidence NUMBER", Float) { |value| options[:min_confidence] = value }
+ parser.on("--comment-min-confidence NUMBER", Float) { |value| options[:comment_min_confidence] = value }
+ parser.on("--block-min-confidence NUMBER", Float) { |value| options[:block_min_confidence] = value }
+ # Legacy alias: sets the block threshold.
+ parser.on("--min-confidence NUMBER", Float) { |value| options[:block_min_confidence] = value }
parser.on("--output PATH") { |value| options[:output] = value }
- parser.on("--markdown-output PATH") { |value| options[:markdown_output] = value }
+ parser.on("--markdown-output PATH") { |value| markdown_output = value }
end.parse!
raise OptionParser::MissingArgument, "artifact-root" if options[:artifact_root].to_s.empty?
- markdown_output = options.delete(:markdown_output)
result = EvalOpsPrLensReview.meta_review(**options)
File.write(markdown_output, EvalOpsPrLensReview.markdown_meta_report(result)) if markdown_output
- puts "Published #{result.fetch("published_findings").length} high-confidence finding(s)."
+ puts(
+ "Published #{result.fetch("published_findings").length} finding(s) " \
+ "(#{result.fetch("blocking_findings").length} blocking)."
+ )
when "dispatch-review-requests"
options = {
owner: "evalops",
diff --git a/.github/workflows/evalops-pr-lens-review.yml b/.github/workflows/evalops-pr-lens-review.yml
--- a/.github/workflows/evalops-pr-lens-review.yml
+++ b/.github/workflows/evalops-pr-lens-review.yml
@@ -16,9 +16,17 @@
required: false
default: ""
min_confidence:
- description: "Minimum confidence for PR comment publication"
+ description: "Deprecated alias for block_min_confidence (kept for back-compat)"
required: false
- default: "0.82"
+ default: ""
+ comment_min_confidence:
+ description: "Minimum confidence for surfacing a finding (inline or summary)"
+ required: false
+ default: "0.55"
+ block_min_confidence:
+ description: "Minimum confidence for a P0/P1 finding to fail the meta-review status"
+ required: false
+ default: "0.80"
model:
description: "Model for lens reviewers"
required: false
@@ -309,7 +317,10 @@
timeout-minutes: 10
env:
GH_TOKEN: ${{ secrets.EVALOPS_PR_LENS_TOKEN || secrets.EVALOPS_REVIEW_GUARD_TOKEN }}
- PR_LENS_MIN_CONFIDENCE: ${{ github.event_name == 'repository_dispatch' && github.event.client_payload.min_confidence || inputs.min_confidence || '0.82' }}
+ # Back-compat alias: PR_LENS_MIN_CONFIDENCE maps to the block threshold.
+ PR_LENS_MIN_CONFIDENCE: ${{ github.event_name == 'repository_dispatch' && github.event.client_payload.min_confidence || inputs.min_confidence || '' }}
+ PR_LENS_COMMENT_MIN_CONFIDENCE: ${{ github.event_name == 'repository_dispatch' && github.event.client_payload.comment_min_confidence || inputs.comment_min_confidence || '0.55' }}
+ PR_LENS_BLOCK_MIN_CONFIDENCE: ${{ github.event_name == 'repository_dispatch' && github.event.client_payload.block_min_confidence || inputs.block_min_confidence || '0.80' }}
PR_LENS_APP_REPOSITORIES: ".github,platform,deploy,maestro-internal,maestro,ensemble,diffscope,chat,cerebro"
RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
steps:
@@ -351,15 +362,23 @@
exit 2
fi
- - name: Publish high-confidence findings
+ - name: Publish findings as inline PR review
shell: bash
run: |
set -euo pipefail
- ruby .github/scripts/evalops-pr-lens-review.rb meta-review \
- --artifact-root artifacts \
- --min-confidence "${PR_LENS_MIN_CONFIDENCE}" \
- --output meta-review.json \
+ args=(
+ meta-review
+ --artifact-root artifacts
+ --comment-min-confidence "${PR_LENS_COMMENT_MIN_CONFIDENCE}"
+ --block-min-confidence "${PR_LENS_BLOCK_MIN_CONFIDENCE}"
+ --output meta-review.json
--markdown-output "${GITHUB_STEP_SUMMARY}"
+ )
+ # Back-compat: a non-empty legacy min_confidence overrides the block threshold.
+ if [ -n "${PR_LENS_MIN_CONFIDENCE}" ]; then
+ args+=(--min-confidence "${PR_LENS_MIN_CONFIDENCE}")
+ fi
+ ruby .github/scripts/evalops-pr-lens-review.rb "${args[@]}"
- name: Upload meta review ledger
uses: actions/upload-artifact@v4
diff --git a/test/evalops_pr_lens_review_test.rb b/test/evalops_pr_lens_review_test.rb
--- a/test/evalops_pr_lens_review_test.rb
+++ b/test/evalops_pr_lens_review_test.rb
@@ -2,6 +2,9 @@
require "json"
require "minitest/autorun"
+require "open3"
+require "set"
+require "stringio"
require "tmpdir"
require_relative "../.github/scripts/evalops-pr-lens-review"
@@ -154,19 +157,54 @@
]
}
- review = EvalOpsPrLensReview.normalize_lens_review(
- raw,
- repo: "evalops/platform",
- pr: 2023,
- lens: "migration-safety",
- head_sha: "abc123"
- )
+ review = nil
+ warnings = capture_warnings do
+ review = EvalOpsPrLensReview.normalize_lens_review(
+ raw,
+ repo: "evalops/platform",
+ pr: 2023,
+ lens: "migration-safety",
+ head_sha: "abc123"
+ )
+ end
+ assert_equal 1, warnings.grep(/dropped malformed finding/).length
assert_equal "evalops-pr-lens/migration-safety", review.fetch("check_id")
assert_equal 1, review.fetch("findings").length
assert_equal "db/migrations/001.sql", review.fetch("findings").fetch(0).dig("code_location", "path")
+ assert_equal 1, review.fetch("dropped_findings")
end
+ def test_normalize_findings_with_drops_counts_and_warns_malformed_findings
+ raw_findings = [
+ {
+ "title" => "Valid",
+ "body" => "A real defect.",
+ "confidence_score" => 0.7,
+ "priority" => 2,
+ "code_location" => { "path" => "a.rb", "line" => 5 }
+ },
+ { "title" => "missing body" },
+ { "body" => "missing title" }
+ ]
+
+ findings = nil
+ dropped = nil
+ warnings = capture_warnings do
+ findings, dropped = EvalOpsPrLensReview.normalize_findings_with_drops(
+ raw_findings,
+ repo: "evalops/platform",
+ pr: 7,
+ lens: "migration-safety"
+ )
+ end
+
+ assert_equal 1, findings.length
+ assert_equal 2, dropped
+ assert_equal 2, warnings.grep(/dropped malformed finding/).length
+ assert(warnings.any? { |line| line.include?("evalops/platform#7 migration-safety") })
+ end
+
def test_high_confidence_findings_filters_and_ranks_by_confidence
reviews = [
{
@@ -287,8 +325,8 @@
end
end
- def test_comment_body_contains_only_ranked_findings
- findings = [
+ def test_review_summary_body_lists_inline_and_off_diff_findings
+ inline = [
finding("Unsafe IAM expansion", 0.94, 1, "infra/main.tf", 22).merge(
"repo" => "evalops/deploy",
"pr" => 10,
@@ -297,18 +335,78 @@
"check_id" => "evalops-pr-lens/iam-blast-radius"
)
]
+ summary = [
+ finding("Drift outside the diff", 0.61, 2, "infra/old.tf", 9).merge(
+ "repo" => "evalops/deploy",
+ "pr" => 10,
+ "lens" => "argo-manifest-skew",
+ "head_sha" => "abc123",
+ "check_id" => "evalops-pr-lens/argo-manifest-skew"
+ )
+ ]
- body = EvalOpsPrLensReview.comment_body(
+ body = EvalOpsPrLensReview.review_summary_body(
repo: "evalops/deploy",
pr: 10,
- findings: findings,
- min_confidence: 0.82,
+ inline_findings: inline,
+ overflow_inline_findings: [],
+ summary_findings: summary,
+ comment_min_confidence: 0.55,
target_url: "https://github.com/evalops/.github/actions/runs/1"
)
assert_includes body, EvalOpsPrLensReview::MARKER
- assert_includes body, "High-confidence findings only"
- assert_includes body, "`infra/main.tf:22`"
+ assert_includes body, "2 findings ≥ 0.55 confidence."
+ assert_includes body, "1 anchored inline below."
+ assert_includes body, "Findings outside the diff"
+ assert_includes body, "`infra/old.tf:9`"
+ end
+
+ def test_review_summary_body_lists_inline_overflow_in_summary
+ inline = Array.new(EvalOpsPrLensReview::MAX_FINDINGS_PER_COMMENT) do |index|
+ finding("Inline #{index}", 0.80, 1, "infra/main.tf", index + 1).merge(
+ "repo" => "evalops/deploy",
+ "pr" => 10,
+ "lens" => "iam-blast-radius",
+ "head_sha" => "abc123",
+ "check_id" => "evalops-pr-lens/iam-blast-radius"
+ )
+ end
+ overflow = [
+ finding("Overflow inline", 0.79, 2, "infra/main.tf", 99).merge(
+ "repo" => "evalops/deploy",
+ "pr" => 10,
+ "lens" => "iam-blast-radius",
+ "head_sha" => "abc123",
+ "check_id" => "evalops-pr-lens/iam-blast-radius"
... diff truncated: showing 800 of 1360 linesYou can send follow-ups to the cloud agent here.
…ive API call #147 changed pr_files_metadata to the paginated helper (gh api --paginate --slurp), which the existing gh_api_json stub no longer intercepts; the test then hit the live API and failed in CI (no GH_TOKEN). Stub gh_api_paginated_json so the test is hermetic. Verified passing under a no-auth gh environment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is ON, but it could not run because the branch was deleted or merged before autofix could start.
Reviewed by Cursor Bugbot for commit 85ae9c0. Configure here.
| ) | ||
| published_summary_id = published_summary_id["id"] if published_summary_id | ||
| delete_marker_comments(repo: repo, pr: pr, ids: prior_summary_ids) | ||
| delete_marker_review_comments(repo: repo, pr: pr, ids: prior_inline_ids) |
There was a problem hiding this comment.
Inline failure removes prior comments
Medium Severity
When publish_review hits an error posting inline pull comments, it rolls back only the new partial inline posts, then still posts the summary and deletes all prior marker inline comments for the PR. A transient inline API failure on re-run can leave the PR with no inline annotations even though an earlier successful run had them.
Reviewed by Cursor Bugbot for commit 85ae9c0. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
The EvalOps PR lens review system (
.github/scripts/evalops-pr-lens-review.rb+.github/workflows/evalops-pr-lens-review.yml) fans out 6 lenses per open PR every 2h and publishes findings. It published 0 findings across the last 12 scheduled runs (~70 lens reviews) and reportedsuccessevery single time. Evidence:gh run list --workflow evalops-pr-lens-review.ymlshows 12 consecutive green runs (2026-06-08 22:54Z through 2026-06-09 19:12Z).Three causes, all in the script:
DEFAULT_MIN_CONFIDENCE = 0.82did double duty — it decided both what to show a human and what fails the status. Real medium-confidence findings (0.6–0.8) were silently discarded.upsert_comment, printingpath:lineas plain text — even though every finding carries an exactcode_location.normalize_findingreturnednilfor malformed findings, dropping them with no signal.The fix
POST /repos/{repo}/pulls/{pr}/reviews(event: "COMMENT",commit_id= head sha, summarybody,comments: [{path, line, side: "RIGHT", body}]).meta-reviewfetchespulls/{pr}/files?per_page=100, parses each file'spatchhunks to compute the set of addable right-side line numbers per path, and inlines only findings whose path:line is in that set; the rest fold into the review summary body withpath:line. This avoids GitHub's 422 on lines not in the diff.comment_min_confidence(envPR_LENS_COMMENT_MIN_CONFIDENCE, default 0.55) — findings at/above this are surfaced (inline or summary).block_min_confidence(envPR_LENS_BLOCK_MIN_CONFIDENCE, default 0.80) — only P0/P1 findings at/above this flip themeta-reviewstatus tofailure.PR_LENS_MIN_CONFIDENCE/--min-confidencekept as a back-compat alias that maps to the block threshold.6 lenses · 0 findings ≥ 0.55instead of implying nothing was found.normalize_findingnow raisesDroppedFinding; the caller counts andwarns per malformed finding and recordsdropped_findingsin the lens-review and meta-review ledgers.GET pulls/{pr}/comments→DELETE pulls/comments/{id}), so re-running on the same head replaces rather than duplicates. TheMARKERsurvives in both the summary body and each inline comment.comment_min_confidence/block_min_confidenceinputs and meta-review env vars;min_confidenceretained for back-compat. The app token already haspull_requests: write.Test results
New coverage in
test/evalops_pr_lens_review_test.rb: diff-hunk right-side line parsing (incl. the\ No newlinemarker), inline-vs-summary partition, the two thresholds +meta_state/meta_description, idempotent clear of issue + review marker comments, the PR-review payload shape, and dropped-finding counting/warning.How to verify
gh workflow run evalops-pr-lens-review.yml -f target_prs="deploy#<N>" -f comment_min_confidence=0.55 -f block_min_confidence=0.80(or trigger the repository_dispatch path).evalops-pr-lens/meta-reviewstatus reads… findings ≥ 0.55(and only goesfailureon a P0/P1 ≥ 0.80).🤖 Generated with Claude Code