Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions online/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,49 @@ This is the core of the benchmark — a three-step LLM analysis:
- **Recall** = matched actions / total actions ("what % of real issues did the bot catch?")
- **F-beta** = adjustable harmonic mean (F1 when beta=1)

#### Honoring tools' own provenance labels

A code review bot is often more than a code reviewer. The same bot account may
also post check-run results, PR assistant chatter, approvability verdicts, or
release notices. Those are distinct product surfaces from code review, and
scoring them 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.

When a tool tells us which surface produced a comment, the benchmark honors it.
Macroscope stamps every PR comment with a hidden HTML marker carrying a JSON
payload that records the comment's provenance:

```html
<!-- macroscope-meta: {"kind":"code_review","variant":"..."} -->
<!-- macroscope-meta: {"kind":"check_run","config":"...","check":"..."} -->
<!-- macroscope-meta: {"kind":"pr_assistant"} -->
```

The `kind` field distinguishes real review (`code_review`) from non-review
surfaces (`check_run`, `pr_assistant`, `approvability`, `notice`, and any added
later). Any other payload fields are ignored, and a malformed payload (or one
without a `kind`) is treated as untagged so a real review comment is never
wrongly excluded.
The benchmark scores **only `code_review`**. Every other kind is *segmented* — recorded
separately as a custom-check comment and excluded from the review-precision
denominator, not silently dropped (see `custom_check` in
`pipeline/analyze.py::_format_bot_comments`).

Two details matter:

- The marker is detected on the **raw comment body**, before hidden HTML comments
are stripped for the LLM prompt — the marker itself is an HTML comment, so
checking the cleaned body would never see it.
- Exclusion keys on `kind != code_review` rather than an allowlist of known
non-review kinds, so a new non-review surface is excluded automatically without
a benchmark change.

This is a per-tool convention: any bot that labels its own non-review comments can
be scored the same way. The `code_review`-only rule is safe against a tool
labelling its false positives away, because excluded comments are *segmented and
recorded*, not dropped — the exclusions remain auditable.

### 5. Label (optional)

An LLM classifies each PR by language, domain (frontend/backend/infra), PR type (feature/bugfix/refactor), issue severity, and more. These labels power the dashboard filters.
Expand Down
99 changes: 89 additions & 10 deletions online/etl/pipeline/analyze.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
from __future__ import annotations

import asyncio
from dataclasses import dataclass
from dataclasses import field
import json
import logging
import re
Expand All @@ -25,6 +27,22 @@

_HTML_COMMENT_RE = re.compile(r"<!--.*?-->", re.DOTALL)

# Macroscope stamps every PR comment with a hidden provenance marker carrying a
# JSON payload whose `kind` identifies the product surface that produced it:
# `code_review` for real review, and other kinds (check_run, pr_assistant,
# approvability, notice, ...) for non-review surfaces. We score only `code_review`;
# everything else is segmented out of the precision denominator. Keying on
# `kind != code_review` (rather than an allowlist of non-review kinds) means new
# non-review surfaces are excluded automatically.
#
# The marker looks like: <!-- macroscope-meta: {"kind":"code_review",...} -->
# We capture the JSON object between the prefix and the closing "-->" and parse it;
# the regex is whitespace- and newline-tolerant, and the payload may carry extra
# fields (variant, config, check, ...) that we ignore.
_MACROSCOPE_META_RE = re.compile(r"<!--\s*macroscope-meta:\s*(\{.*?\})\s*-->", re.DOTALL)

_CODE_REVIEW_KIND = "code_review"


def _find_bot_review_commit(
reviews: list[dict],
Expand Down Expand Up @@ -150,13 +168,59 @@ def _clean_bot_comment_body(body: str) -> str:
return _HTML_COMMENT_RE.sub("", body).strip()


def _format_bot_comments(events: list[dict], chatbot_username: str) -> str:
"""Format bot's review/review_comment/issue_comment events with full context.
def _macroscope_kind(raw_body: str) -> str | None:
"""Return the macroscope-meta `kind` stamped on a raw comment body, if present.

The marker carries a JSON payload: `<!-- macroscope-meta: {"kind":...} -->`.
We extract the JSON object and read its `kind` field.

The check MUST run on the RAW body, before `_clean_bot_comment_body` strips
HTML comments — the marker lives inside an HTML comment and cleaning would
erase it, silently letting non-review surfaces back into the precision
denominator.

A missing marker, malformed JSON, or a payload without a string `kind` is
treated as untagged (returns None) so the comment keeps the default scored
behavior rather than being wrongly excluded or throwing.
"""
match = _MACROSCOPE_META_RE.search(raw_body or "")
if not match:
return None
try:
payload = json.loads(match.group(1))
except json.JSONDecodeError:
return None
kind = payload.get("kind") if isinstance(payload, dict) else None
return kind if isinstance(kind, str) else None


@dataclass
class BotCommentSegments:
"""Bot comments split by provenance for the analyze step.

Skips review_comment replies (in_reply_to_id set) — these are responses
to other commenters' threads, not original review suggestions.
`review` is the formatted text of `code_review` (and untagged) comments that
feeds EXTRACT_BOT_SUGGESTIONS — i.e. the precision denominator. `custom_check`
holds the comments segmented out by their macroscope-meta kind: recorded here
so they are auditable rather than silently dropped, but excluded from scoring.
"""

review: str
custom_check: list[dict] = field(default_factory=list)


def _format_bot_comments(events: list[dict], chatbot_username: str) -> BotCommentSegments:
"""Split and format the bot's review/review_comment/issue_comment events.

Skips review_comment replies (in_reply_to_id set) — these are responses to
other commenters' threads, not original review suggestions.

Comments carrying a macroscope-meta marker whose kind is not `code_review`
(check runs, PR assistant, approvability, notices, and any future non-review
surface) are segmented into `custom_check` and kept out of the returned review
text, so they never inflate the precision denominator.
"""
lines = []
custom_check = []
comment_num = 1
for e in events:
if not same_github_actor(e.get("actor"), chatbot_username):
Expand All @@ -170,15 +234,21 @@ def _format_bot_comments(events: list[dict], chatbot_username: str) -> str:
continue

ts = e.get("timestamp", "")
raw_body = data.get("body") or ""

# Provenance check on the RAW body, before HTML-comment cleaning.
kind = _macroscope_kind(raw_body)
if kind is not None and kind != _CODE_REVIEW_KIND:
custom_check.append({"kind": kind, "event_type": etype, "timestamp": ts})
continue

body = _clean_bot_comment_body(raw_body)
if etype == "review":
state = data.get("state", "")
body = _clean_bot_comment_body(data.get("body") or "")
lines.append(f"COMMENT C{comment_num} [REVIEW_BODY state={state} timestamp={ts}]")
if body:
lines.append(body)
elif etype in ("review_comment", "issue_comment"):
body = _clean_bot_comment_body(data.get("body") or "")
else:
path = data.get("path") or ""
line = data.get("line") or ""
diff_hunk = data.get("diff_hunk") or ""
Expand All @@ -196,7 +266,7 @@ def _format_bot_comments(events: list[dict], chatbot_username: str) -> str:
lines.append(body)
lines.append("")
comment_num += 1
return "\n".join(lines) if lines else "(no bot comments)"
return BotCommentSegments(review="\n".join(lines) if lines else "(no bot comments)", custom_check=custom_check)


def _format_post_review_activity(
Expand Down Expand Up @@ -342,6 +412,12 @@ def _parse_json_col(col_name: str) -> list[dict]:
# Format inputs for LLM
commits_under_review = _format_commits_with_diffs(pre_commits, details_by_sha)
bot_comments = _format_bot_comments(events, chatbot_username)
if bot_comments.custom_check:
logger.debug(
f"{pr_row['repo_name']}#{pr_row['pr_number']}: segmented "
f"{len(bot_comments.custom_check)} non-review comment(s) out of the precision denominator "
f"(kinds={sorted({c['kind'] for c in bot_comments.custom_check})})"
)
post_review_activity = _format_post_review_activity(post_commits, details_by_sha, events, chatbot_username, hash_x)

pr_title = assembled.get("pr_title", "")
Expand All @@ -355,7 +431,7 @@ def _parse_json_col(col_name: str) -> list[dict]:
pr_author=pr_author,
repo_name=repo_name,
commits_under_review=commits_under_review,
bot_comments=bot_comments,
bot_comments=bot_comments.review,
)
suggestions_resp = await llm.structured_completion(prompt1, BotSuggestionsResponse)
suggestions = [s.model_dump() for s in suggestions_resp.suggestions]
Expand Down Expand Up @@ -447,7 +523,10 @@ async def analyze_prs(
"""
repo = PRRepository(db)
prs = await repo.get_assembled_not_analyzed(
chatbot_id=chatbot_id, limit=limit, since=since, until=until,
chatbot_id=chatbot_id,
limit=limit,
since=since,
until=until,
sort_by=sort_by,
)

Expand Down
2 changes: 1 addition & 1 deletion online/etl/tests/test_actor_normalization.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def test_analyze_includes_graphql_thread_comments_from_bot_slug() -> None:
}
]

formatted = _format_bot_comments(events, "cubic-dev-ai[bot]")
formatted = _format_bot_comments(events, "cubic-dev-ai[bot]").review

assert "Fix the missing null check." in formatted
assert "main.py:42" in formatted
Expand Down
150 changes: 149 additions & 1 deletion online/etl/tests/test_analyze_formatting.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

from __future__ import annotations

import json

from pipeline.analyze import _clean_bot_comment_body
from pipeline.analyze import _format_bot_comments

Expand Down Expand Up @@ -83,9 +85,155 @@ def test_format_bot_comments_labels_and_numbers_comments() -> None:
},
]

formatted = _format_bot_comments(events, "reviewer[bot]")
formatted = _format_bot_comments(events, "reviewer[bot]").review

assert "COMMENT C1 [INLINE_REVIEW_COMMENT path=src/app.py:42 timestamp=2026-07-01T12:00:00Z]" in formatted
assert "Code context:\n```diff\n@@ -1 +1 @@\n-old\n+new\n```" in formatted
assert "COMMENT C2 [REVIEW_BODY state=commented timestamp=2026-07-01T12:02:00Z]" in formatted
assert "Fixed now." not in formatted


def _macroscope_event(kind: str | None, body_text: str, ts: str, **extra: object) -> dict:
"""Build an issue_comment event, optionally stamped with a macroscope-meta marker.

Mirrors how Macroscope emits comments: the provenance marker is a hidden HTML
comment carrying a JSON payload, prepended to the visible body. `kind=None`
produces an untagged comment; `extra` adds sibling payload fields (variant,
config, check, ...) that the parser must ignore.
"""
marker = "" if kind is None else f"<!-- macroscope-meta: {json.dumps({'kind': kind, **extra})} -->\n"
return {
"actor": "macroscopeapp[bot]",
"event_type": "issue_comment",
"timestamp": ts,
"data": {"body": f"{marker}{body_text}"},
}


def test_format_bot_comments_segments_non_code_review_kinds_out_of_review_text() -> None:
"""Requirement: a comment marked with a non-code_review kind must not feed the
precision denominator.

EXTRACT_BOT_SUGGESTIONS is built from the returned `review` text, so a check_run
(or any non-review surface) comment must be absent from it and instead recorded
in `custom_check`. Otherwise convention/style/policy check-run comments — rarely
"fixed" by developers — would drag Macroscope's precision unfairly.
"""
events = [
_macroscope_event("code_review", "Possible null dereference here.", "2026-07-01T12:00:00Z"),
_macroscope_event("check_run", "Naming convention: use snake_case.", "2026-07-01T12:01:00Z"),
]

segments = _format_bot_comments(events, "macroscopeapp[bot]")

assert "Possible null dereference here." in segments.review
assert "Naming convention: use snake_case." not in segments.review
assert [c["kind"] for c in segments.custom_check] == ["check_run"]


def test_format_bot_comments_keys_on_not_code_review_so_future_kinds_are_excluded() -> None:
"""Requirement: exclusion keys on `kind != code_review`, not an allowlist of
known non-review kinds.

A brand-new surface (here `pr_assistant`) that Martian has never heard of must
be excluded automatically, without another benchmark change.
"""
events = [
_macroscope_event("pr_assistant", "Want me to summarize this PR?", "2026-07-01T12:00:00Z"),
_macroscope_event("approvability", "This PR looks safe to merge.", "2026-07-01T12:01:00Z"),
]

segments = _format_bot_comments(events, "macroscopeapp[bot]")

assert segments.review == "(no bot comments)"
assert sorted(c["kind"] for c in segments.custom_check) == ["approvability", "pr_assistant"]


def test_format_bot_comments_keeps_code_review_and_untagged_comments() -> None:
"""Requirement: real review (kind=code_review) and legacy untagged comments are
unaffected — they still feed the precision denominator.

Untagged comments (comments predating the marker, or from bots that never emit
it) must keep the pre-change behavior of being scored.
"""
events = [
_macroscope_event("code_review", "Tagged review finding.", "2026-07-01T12:00:00Z"),
_macroscope_event(None, "Untagged review finding.", "2026-07-01T12:01:00Z"),
]

segments = _format_bot_comments(events, "macroscopeapp[bot]")

assert "Tagged review finding." in segments.review
assert "Untagged review finding." in segments.review
assert segments.custom_check == []
assert "COMMENT C1 " in segments.review
assert "COMMENT C2 " in segments.review


def test_format_bot_comments_detects_marker_on_raw_body_before_html_cleaning() -> None:
"""Requirement (the raw-vs-cleaned gotcha): the marker must be detected on the
RAW body, before `_clean_bot_comment_body` strips HTML comments.

The marker lives inside an HTML comment, which cleaning removes. If detection
ran on the cleaned body the kind would be invisible and the comment would leak
back into the precision denominator. This asserts the marked comment is excluded
even though the marker is exactly the kind of HTML comment cleaning strips.
"""
stripped_marker = '<!-- macroscope-meta: {"kind":"check_run"} -->'
assert _clean_bot_comment_body(stripped_marker) == "" # cleaning erases the marker entirely

events = [_macroscope_event("check_run", "Style nit.", "2026-07-01T12:00:00Z")]

segments = _format_bot_comments(events, "macroscopeapp[bot]")

assert segments.review == "(no bot comments)"
assert [c["kind"] for c in segments.custom_check] == ["check_run"]


def test_format_bot_comments_ignores_extra_payload_fields() -> None:
"""Requirement: the JSON payload may carry sibling fields (variant, config,
check, ...); the parser reads only `kind` and ignores the rest.

Macroscope stamps additional metadata per surface — e.g. check_run comments
carry `config` and `check`. Those must not affect the code_review-only rule.
"""
events = [
_macroscope_event("code_review", "Real finding.", "2026-07-01T12:00:00Z", variant="inline"),
_macroscope_event("check_run", "Style nit.", "2026-07-01T12:01:00Z", config="lint", check="naming"),
]

segments = _format_bot_comments(events, "macroscopeapp[bot]")

assert "Real finding." in segments.review
assert "Style nit." not in segments.review
assert [c["kind"] for c in segments.custom_check] == ["check_run"]


def test_format_bot_comments_treats_malformed_payload_as_untagged() -> None:
"""Requirement: a marker whose payload is not valid JSON (or lacks a string
`kind`) is treated as untagged — the comment stays scored, and parsing never
throws.

Robustness: a truncated or malformed marker must not silently exclude a real
review comment, nor crash the pipeline. Untagged is the safe default.
"""
events = [
{ # malformed JSON payload
"actor": "macroscopeapp[bot]",
"event_type": "issue_comment",
"timestamp": "2026-07-01T12:00:00Z",
"data": {"body": '<!-- macroscope-meta: {"kind": check_run,,,} -->\nReview finding A.'},
},
{ # valid JSON, but no "kind" field
"actor": "macroscopeapp[bot]",
"event_type": "issue_comment",
"timestamp": "2026-07-01T12:01:00Z",
"data": {"body": '<!-- macroscope-meta: {"variant":"inline"} -->\nReview finding B.'},
},
]

segments = _format_bot_comments(events, "macroscopeapp[bot]")

assert "Review finding A." in segments.review
assert "Review finding B." in segments.review
assert segments.custom_check == []
Loading