[no-ticket] Add review prompt eval set and dry run mode - #36
Draft
bockets wants to merge 2 commits into
Draft
Conversation
The prompts had no regression set, so a prompt change could only be judged by reading it. evals/ holds cases: a real pull request whose correct review outcome is known, an expected verdict, and rows of acceptable phrasings that separate finding the actual defect from requesting changes over something else. evals/score.sh scores one verdict against one case and treats a missing verdict as a failure, so a crashed review cannot score as a pass. The first two cases are the review-app sweeper: the replay of the PR whose too-narrow permission was approved, and the fix for it, kept as the approve case so a prompt that blocks everything does not score well. dry_run on the reusable review workflow runs the real pipeline and uploads the verdict as an artifact instead of posting a review, which is what lets the set run against pull requests without leaving reviews on them.
Each scored case goes out twice: an LLM Obs evaluation metric joined to the review's Claude span, so a case result reads next to the summary it scored and groups by prompt_version with everything else the pipeline reports, and a biggiepockets.review_eval.case_passed series, because evaluation metrics cannot be alerted on and a dashboard needs a metric. The span join needs span_id in decimal while the review workflow allocates hex, so the ids move to their own step and the dry run records them for the scorer. A case whose review died has no span to join to; it still publishes the metric and says why the join is missing rather than inventing a key. A publishing failure warns with the HTTP status and body instead of failing the eval run.
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
1 participant
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.
[no-ticket]
The review prompts have no regression set, so a prompt change can only be judged by reading it. This adds one, seeded with the case that motivated it.
evals/
Each case is a real pull request whose correct review outcome is known — usually because the review that actually ran got it wrong and the defect surfaced later. A case names the PR, the verdict a correct review must reach, and rows of acceptable phrasings that separate found this defect from requested changes over something else.
evals/score.shscores one verdict against one case. Rows are ANDed, entries within a row ORed, matched case-insensitively. A review that produced no verdict scores as a failure, never a pass, so a crashed pipeline can't read as green.Two cases to start:
review-app-sweeper-permission-scope— a replay of BiggerPockets/biggerpockets#30384 at its original head, where the added workflow declares onlyissues: readwhile its/cwacheck needspull-requests: read. The real review approved it and recorded the scope as correctly declared.review-app-sweeper-permission-fix— the fix for it, expected to be approved. Scored on verdict alone, and kept deliberately: an eval set of defect cases only rewards a reviewer that never approves.The replay exists because the original PR now carries a comment explaining the defect, and the prompts weigh PR discussion — pointing a case at it would be a reading exercise, not a review.
evals/README.mdrecords that constraint along with how to add a case.dry_run on the reusable review workflow
Runs the real pipeline and uploads the verdict as a
review-verdict-pr<n>artifact instead of posting a review, so the set can run against pull requests without leaving reviews on them. Everything upstream of the submit step is untouched on purpose — an eval that runs a different pipeline measures a different pipeline. The artifact uploads even on failure, since a missing verdict is itself a result.The runner that consumes this is BiggerPockets/biggerpockets#30505; it needs this merged first.
Matching against a summary can only tell that a review talked about the right thing, never that its reasoning was sound. That limit is documented in both the scorer and the README rather than papered over — if it turns out to matter, the next step is an LLM judge scoring against the case's
why.