open-swe/tests/test_reviewer_outcomes.py
Johannes du Plessis 4a55145bb1
feat: outcomes dataset + bootstrap/continual split via skills (#1365)
* fix: reset stale sandbox creation sentinel

Co-authored-by: Johannes du Plessis <51395795+johannes117@users.noreply.github.com>

* fix: treat SANDBOX_CREATING as a timestamped cross-process lock

Only reset the sentinel when proven stale (older than the creation
timeout); otherwise wait for the worker that holds the lock so a
concurrent run does not create a duplicate sandbox.

* feat(analyzer): outcomes dataset + bootstrap/continual split via skills

Rename the review_style_analyzer graph to `analyzer` and split it into two
modes, plus capture reviewer finding outcomes for continual learning.

- Outcomes dataset: upsert resolved-by-commit (positive), dismissed (false
  positive), and GitHub/Slack thumbs findings into a single LangSmith dataset
  (openswe-reviewer-outcomes), keyed deterministically per finding+source.
  Emit points wired into update_finding, resolve_finding_thread, and the
  GitHub/Slack reaction handlers.
- Two playbooks delivered as deepagents skills (bootstrap-repo-analysis,
  continual-learning), served as virtual files via a CompositeBackend /skills/
  route + StateBackend (seeded into the run files channel at invoke time, never
  written to the sandbox). Mode is set by the launcher; continual runs fall
  back to the GitHub App installation token.
- Split launcher into start_bootstrap_analysis + start_continual_run; register
  a per-repo nightly continual-learning cron when bootstrap completes.
- New read_finding_outcomes tool feeds confirmed/dismissed findings back to the
  continual playbook.

Tests for outcome label mapping, skills helper, and cron idempotency.

* fix(analyzer): anchor continual cron runs to a real thread_id

The nightly continual-learning cron is threadless, and get_analyzer
early-returns an empty agent when configurable.thread_id is missing — so
every cron-launched run no-op'd before reading outcomes or saving a refined
prompt. Include the repo's deterministic analyzer thread_id in the continual
run configurable so the run executes; the threadless run carries no message
history, so nightly runs don't accumulate context.

* refactor(analyzer): move cron lifecycle calls out of the review-styles store

Drop the inline `analyzer_cron` imports from review_styles.py (added only to
dodge a circular import) by relocating the cron-trigger calls to the layer
above the store: registration to the save_review_style tool (after a prompt is
saved) and removal to the dashboard delete route. review_styles.py is now a
pure store again with top-level imports only.

* refactor: hoist reviewer_outcomes imports to module level

Move the two inline emit_finding_status_outcome imports introduced in this PR
(update_finding, resolve_finding_thread) to top-level imports. reviewer_outcomes
only depends on langsmith, so there is no circular import to avoid.

---------

Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
2026-06-01 13:25:12 -07:00

181 lines
5.7 KiB
Python

from __future__ import annotations
from typing import Any
from agent.utils import reviewer_outcomes
from agent.utils.reviewer_outcomes import (
FALSE_POSITIVE,
TRUE_POSITIVE,
emit_finding_status_outcome,
outcome_from_score,
outcome_from_status,
upsert_finding_outcome,
)
def test_outcome_from_status_resolved_by_commit() -> None:
assert outcome_from_status("resolved", first_seen_sha="aaa", head_sha="bbb") == (
TRUE_POSITIVE,
"resolved_by_commit",
)
def test_outcome_from_status_resolved_same_sha() -> None:
assert outcome_from_status("resolved", first_seen_sha="aaa", head_sha="aaa") == (
TRUE_POSITIVE,
"resolved_same_sha",
)
def test_outcome_from_status_dismissed() -> None:
assert outcome_from_status("dismissed", first_seen_sha="aaa", head_sha="bbb") == (
FALSE_POSITIVE,
"dismissed",
)
def test_outcome_from_status_open_is_none() -> None:
assert outcome_from_status("open", first_seen_sha="aaa", head_sha="bbb") is None
def test_outcome_from_score() -> None:
assert outcome_from_score(1.0, source="github") == (TRUE_POSITIVE, "github_thumbs_up")
assert outcome_from_score(0.0, source="github") == (FALSE_POSITIVE, "github_thumbs_down")
assert outcome_from_score(1.0, source="slack") == (TRUE_POSITIVE, "slack_thumbs_up")
assert outcome_from_score(None, source="github") is None
def test_example_id_is_deterministic() -> None:
a = reviewer_outcomes._example_id("o/r", "f_1", "dismissed")
b = reviewer_outcomes._example_id("o/r", "f_1", "dismissed")
c = reviewer_outcomes._example_id("o/r", "f_1", "resolved_by_commit")
assert a == b
assert a != c
class _FakeDataset:
id = "ds_123"
class _FakeClient:
def __init__(self) -> None:
self.created: list[dict[str, Any]] = []
self.updated: list[dict[str, Any]] = []
self.conflict_once = False
def list_datasets(self, dataset_name: str): # noqa: ANN001
return iter([_FakeDataset()])
def create_example(self, **kwargs: Any) -> None:
if self.conflict_once:
self.conflict_once = False
raise RuntimeError("already exists")
self.created.append(kwargs)
def update_example(self, **kwargs: Any) -> None:
self.updated.append(kwargs)
def _finding() -> dict[str, Any]:
return {
"id": "f_abc",
"file": "app/x.rb",
"start_line": 10,
"end_line": 12,
"side": "RIGHT",
"diff_hunk": "@@ -1 +1 @@\n-foo\n+bar",
"title": "NoMethodError on nil",
"description": "body",
"severity": "high",
"confidence": "high",
"category": "correctness",
"first_seen_sha": "aaa",
"github_review_run_id": "run_1",
"resolution_note": "fixed in def",
}
def test_upsert_finding_outcome_builds_payload(monkeypatch) -> None: # noqa: ANN001
fake = _FakeClient()
monkeypatch.setattr(reviewer_outcomes, "_outcomes_client", lambda: fake)
ok = upsert_finding_outcome(
_finding(),
label=TRUE_POSITIVE,
label_source="resolved_by_commit",
repo="o/r",
pr_number=7,
pr_url="https://github.com/o/r/pull/7",
base_sha="aaa",
head_sha="bbb",
thread_id="t1",
)
assert ok
assert len(fake.created) == 1
call = fake.created[0]
assert call["dataset_id"] == "ds_123"
assert call["inputs"]["repo"] == "o/r"
assert call["inputs"]["file"] == "app/x.rb"
assert call["inputs"]["diff_hunk"].startswith("@@")
assert call["outputs"]["label"] == TRUE_POSITIVE
assert call["outputs"]["label_source"] == "resolved_by_commit"
assert call["outputs"]["finding"]["severity"] == "high"
assert call["metadata"]["granularity"] == "finding"
assert call["metadata"]["repo"] == "o/r"
assert call["metadata"]["run_id"] == "run_1"
def test_upsert_finding_outcome_updates_on_conflict(monkeypatch) -> None: # noqa: ANN001
fake = _FakeClient()
fake.conflict_once = True
monkeypatch.setattr(reviewer_outcomes, "_outcomes_client", lambda: fake)
ok = upsert_finding_outcome(
_finding(), label=FALSE_POSITIVE, label_source="dismissed", repo="o/r"
)
assert ok
assert not fake.created
assert len(fake.updated) == 1
def test_upsert_no_client_is_noop(monkeypatch) -> None: # noqa: ANN001
monkeypatch.setattr(reviewer_outcomes, "_outcomes_client", lambda: None)
assert (
upsert_finding_outcome(_finding(), label=TRUE_POSITIVE, label_source="x", repo="o/r")
is False
)
def test_emit_finding_status_outcome_maps_and_calls(monkeypatch) -> None: # noqa: ANN001
captured: dict[str, Any] = {}
def fake_upsert(finding, **kwargs: Any) -> bool: # noqa: ANN001
captured.update(kwargs)
captured["finding_id"] = finding["id"]
return True
monkeypatch.setattr(reviewer_outcomes, "upsert_finding_outcome", fake_upsert)
configurable = {
"repo": {"owner": "o", "name": "r"},
"pr_number": 7,
"pr_url": "https://github.com/o/r/pull/7",
"base_sha": "aaa",
"head_sha": "bbb",
}
assert emit_finding_status_outcome(
_finding(), "resolved", configurable=configurable, thread_id="t1"
)
assert captured["repo"] == "o/r"
assert captured["label"] == TRUE_POSITIVE
assert captured["label_source"] == "resolved_by_commit"
assert captured["pr_number"] == 7
def test_emit_finding_status_outcome_no_repo_is_noop(monkeypatch) -> None: # noqa: ANN001
monkeypatch.setattr(reviewer_outcomes, "upsert_finding_outcome", lambda *a, **k: pytest_fail())
assert emit_finding_status_outcome(_finding(), "dismissed", configurable={}) is False
def pytest_fail() -> bool:
raise AssertionError("upsert should not be called without a repo")