Support binary files in PR review workflow#18
Conversation
bnavetta
force-pushed
the
ben/pr-review-binary-files
branch
from
860d413 to
d93d11b
Compare
bnavetta
left a comment
bnavetta
left a comment
There was a problem hiding this comment.
Tested in warpdotdev/warp-internal#23397 - the agent was able to leave both a line-level comment on the modified text file and a file-level comment on the modified image.
| } else { | ||
| payload.body = 'Automated review by Oz Agent'; | ||
| } | ||
| if (hasSummary) { |
There was a problem hiding this comment.
These are all indentation changes
| } | ||
|
|
||
| console.log('Review posted successfully.'); | ||
| // Post binary-file comments separately as file-level review comments. |
There was a problem hiding this comment.
File-level comments can't be posted as part of the createReview request unfortunately. Instead, we have to post each file-level comment separately afterwards.
We could extend this to let the agent post file-level comments on text files, but so far letting it post on the first line works well enough.
| /^index / { print; next } | ||
| /^---/ { print; next } | ||
| /^\+\+\+/ { print; next } | ||
| /^Binary files / { print; next } |
There was a problem hiding this comment.
This ensures Binary files xyz and abc differ lines don't get an incorrect line number prefix.
|
|
||
| // Binary files lack a patch property in the listFiles response. | ||
| const binaryPaths = new Set( | ||
| prFiles.filter(f => !f.patch).map(f => f.filename) |
There was a problem hiding this comment.
A little LLM told me that the !f.patch check might be too broad and catch things like submodule changes and file deletions. File deletions are probably less of a concern here but I wonder if there's hardening we can do here. GitHub's docs don't do a good job covering what the response schema looks like in different cases.
There was a problem hiding this comment.
Hmm, yeah it didn't seem like there's an obvious way to detect binary files reliably in their response 😕
I wonder if we want to treat other files without a patch as binary files for commenting purposes though - presumably, the API doesn't allow line-level comments on submodule changes or deleted files either, in which case we ought to leave file-level comments for those too.
There was a problem hiding this comment.
Renamed to reflect that we're using this approach for any file without a patch intentionally
bnavetta
force-pushed
the
ben/pr-review-binary-files
branch
from
d93d11b to
941d4dd
Compare
2 participants
This amends the comment submission and diff formatting logic in the PR review workflow to handle binary files.
Today, if the agent tries to comment on a binary file, we'll generate a malformed request and GitHub will reject the entire review.
As far as I can tell from GitHub's API docs, they support file-level comments on binary files, but not line-level comments.