Exclude non-review Macroscope comments from precision denominator - #60
Open
kalmanm wants to merge 2 commits into
Open
Exclude non-review Macroscope comments from precision denominator#60kalmanm wants to merge 2 commits into
kalmanm wants to merge 2 commits into
Conversation
The online benchmark treats every comment from a bot account as a review suggestion, so precision = fixed / total. But a code review bot is often more than a code reviewer: the same account may post check-run results, PR assistant chatter, approvability verdicts, or notices. Scoring those as review suggestions is a category error — a style/convention check run is rarely "fixed", so it unfairly drags the tool's precision. Honor the tool's own provenance label. Macroscope stamps every PR comment with a hidden `<!-- macroscope-meta kind=... -->` marker. In `_format_bot_comments` (the function feeding EXTRACT_BOT_SUGGESTIONS, i.e. the precision denominator), detect the marker and segment comments whose kind is not `code_review` into a separate `custom_check` bucket, kept out of the review text but recorded (not silently dropped). Two details: - Detection runs on the RAW body, before `_clean_bot_comment_body` strips HTML comments — the marker is itself an HTML comment. - Exclusion keys on `kind != code_review`, so future non-review surfaces are excluded automatically without another benchmark change. Add unit tests covering exclusion, the not-code_review key, code_review / untagged pass-through, the raw-vs-cleaned gotcha, and marker attribute tolerance. Document the convention in online/README.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The hidden Macroscope marker is changing from attribute style
(`<!-- macroscope-meta kind=... -->`) to a JSON payload
(`<!-- macroscope-meta: {"kind":"...","..."} -->`). Update the parser to
match, or the exclusion silently stops matching once back#14633 lands.
_macroscope_kind now captures the JSON object between the `macroscope-meta:`
prefix and the closing `-->` (whitespace/newline tolerant, DOTALL) and
json.loads it, reading the `kind` field. Extra payload fields (variant,
config, check, ...) are ignored. A missing marker, malformed JSON, or a
payload without a string `kind` is treated as untagged — the comment stays
scored rather than being wrongly excluded, and parsing never throws.
The segmentation logic and BotCommentSegments(review, custom_check) return
are unchanged — only extraction changed. Detection still runs on the RAW
body before HTML-comment cleaning. Tests updated to the JSON format (valid
extraction, extra-field tolerance, malformed-payload robustness, raw-vs-
cleaned gotcha) and README shows the new tag shape.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
kalmanm
marked this pull request as ready for review
August 26, 2026 21:27
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
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.
Summary
The online benchmark treats every comment from a bot account as a review
suggestion, so precision =
fixed / total. But a code review bot is often morethan a code reviewer. The same bot account can also post check-run results, PR
assistant chatter, approvability verdicts, or release notices — distinct
product surfaces from code review. Scoring those as review suggestions is a
category error: a style/convention check run is rarely "fixed" by a
developer, so counting it in the precision denominator unfairly drags the tool's
precision.
This PR makes the benchmark honor a tool's own provenance label and score
only real code review.
The convention
Macroscope stamps every PR comment with a hidden HTML marker carrying a JSON
payload that records which surface produced it:
kinddistinguishes real review (code_review) from non-review surfaces(
check_run,pr_assistant,approvability,notice, …). The benchmark shouldscore only
code_review.The change
In
online/etl/pipeline/analyze.py,_format_bot_comments()(the function thatbuilds the bot-comment list feeding
EXTRACT_BOT_SUGGESTIONS, i.e. the precisiondenominator):
kind != code_revieware moved into aseparate
custom_checkbucket on the returnedBotCommentSegments— recordedand auditable, kept out of the review text, and excluded from the precision
denominator. They are not silently discarded. This is deliberate: segmenting
(not dropping) is what keeps the rule safe against a tool "labelling its false
positives away" — the exclusions remain visible and countable.
_clean_bot_comment_bodystrips HTMLcomments. The marker is itself an HTML comment, so checking the cleaned body
would never see it. (A regression test pins this gotcha.)
kind != code_review, not an allowlist of known non-review kinds,so any future non-review surface is excluded automatically without another
benchmark change.
Extraction grabs the JSON payload between the marker prefix and
-->, thenjson.loadsit and readskind(whitespace/newline tolerant, extra fieldsignored). A malformed payload or one lacking a string
kindis treated asuntagged — the comment stays scored and parsing never throws:
Two approaches considered
BotCommentSegments(review, custom_check);reviewfeeds the LLM,custom_checkrecords the excluded comments. Onecaller updated; the number of segmented comments is logged per PR.
continuepast non-code_reviewcomments in the loop —fewer lines, but the excluded comments vanish with no record, which is exactly
the transparency property we want to keep.
We lead with segment because it directly answers the obvious objection to any
provenance-based exclusion ("couldn't a tool just relabel its misses?"): excluded
comments are recorded, not dropped, so the exclusions stay auditable. The segment
change is small and localized — a dataclass return plus one caller — so there was
no need to defer it to a follow-up.
Generality
This is implemented as a per-tool convention, but it is tool-agnostic in spirit:
any bot that labels its own non-review comments can be scored the same way. Other
tools with check/gate features hit the same conflation; a companion Discussion
invites them to adopt a comparable marker. Nothing here requires action from
other tools — the marker is self-contained and the benchmark simply honors it.
Tests
online/etl/tests/test_analyze_formatting.pyadds coverage for:check_runcomment is absent from the review text and lands incustom_check;kind != code_review(a novelpr_assistantkind is excluded);code_reviewand untagged comments still feed the denominator unchanged;Docs:
online/README.mddocuments the convention under Analyze.