2e1ad817e7
The filter bar matches on metadata only — not on input and not on the item id — so a dataset seeded with repo/pr in `input` alone could not be sliced by repo at all. Every facet worth filtering on is now a flat primitive in `metadata`: repo, owner, repo_name, pr, head_sha, finding_count, has_findings, max_severity, reviews_run and the review timestamp both ways. `owner` is split out because a filter on the joined repo matches one repo, never a whole org, and `max_severity` is "none" rather than absent because an absent key matches no filter. `eval_experiment.py` links reviews that already ran into a dataset run, one run per model, so the Experiments tab is populated without re-running the reviewer. One trace per (run, item), the most recent: a PR re-reviewed on every push has many traces and a run is one output per input. It posts to the deprecated /api/public/dataset-run-items — the notice exempts self-hosted v3 from the cutoff date and the pilot is stdlib-only by design. Revisit at v4. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
159 lines
5.0 KiB
Python
159 lines
5.0 KiB
Python
"""Tests for the eval bootstrap's dataset-item construction."""
|
|
import os
|
|
import sqlite3
|
|
import sys
|
|
import urllib.parse
|
|
|
|
sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "..", "pilot"))
|
|
|
|
import eval_bootstrap as eb # noqa: E402
|
|
|
|
|
|
# --- item_id --------------------------------------------------------------
|
|
|
|
def test_item_id_has_no_path_separator():
|
|
"""A `/` would split the UI's item route into extra path segments."""
|
|
assert "/" not in eb.item_id("netcracker/interview", 29)
|
|
|
|
|
|
def test_item_id_has_no_fragment_marker():
|
|
"""Everything after a `#` is a fragment the browser never sends."""
|
|
assert "#" not in eb.item_id("netcracker/interview", 29)
|
|
|
|
|
|
def test_item_id_survives_a_url_round_trip():
|
|
"""The id must appear verbatim in a path, needing no percent-encoding."""
|
|
ident = eb.item_id("netcracker/interview", 29)
|
|
assert urllib.parse.quote(ident, safe="") == ident
|
|
|
|
|
|
def test_item_id_keeps_repo_and_pr_readable():
|
|
assert eb.item_id("netcracker/interview", 29) == "netcracker__interview__pr29"
|
|
|
|
|
|
def test_item_id_is_unique_per_pr():
|
|
assert eb.item_id("o/r", 1) != eb.item_id("o/r", 2)
|
|
|
|
|
|
def test_item_id_is_unique_per_repo():
|
|
assert eb.item_id("o/one", 1) != eb.item_id("o/two", 1)
|
|
|
|
|
|
def test_item_id_accepts_a_string_pr():
|
|
assert eb.item_id("o/r", "29") == eb.item_id("o/r", 29)
|
|
|
|
|
|
# --- read_review_items ----------------------------------------------------
|
|
|
|
def _db(tmp_path, rows, findings=()):
|
|
path = str(tmp_path / "feedback.db")
|
|
conn = sqlite3.connect(path)
|
|
conn.execute(
|
|
"CREATE TABLE review (repo TEXT, pr INTEGER, posted_at INTEGER, head_sha TEXT)"
|
|
)
|
|
conn.execute(
|
|
"CREATE TABLE inline_finding (repo TEXT, pr INTEGER, path TEXT, line INTEGER,"
|
|
" severity TEXT, problem TEXT, fix TEXT)"
|
|
)
|
|
conn.executemany("INSERT INTO review VALUES (?,?,?,?)", rows)
|
|
conn.executemany("INSERT INTO inline_finding VALUES (?,?,?,?,?,?,?)", findings)
|
|
conn.commit()
|
|
conn.close()
|
|
return path
|
|
|
|
|
|
def test_items_use_url_safe_ids(tmp_path):
|
|
path = _db(tmp_path, [("netcracker/interview", 29, 100, "abc")])
|
|
items = eb.read_review_items(path)
|
|
assert [i["id"] for i in items] == ["netcracker__interview__pr29"]
|
|
|
|
|
|
def test_item_input_keeps_the_real_repo_name(tmp_path):
|
|
"""The id is mangled for the URL; the payload must stay faithful."""
|
|
path = _db(tmp_path, [("netcracker/interview", 29, 100, "abc")])
|
|
item = eb.read_review_items(path)[0]
|
|
assert item["input"]["repo"] == "netcracker/interview"
|
|
assert item["input"]["pr"] == 29
|
|
|
|
|
|
def test_one_item_per_pr_not_per_review(tmp_path):
|
|
path = _db(
|
|
tmp_path,
|
|
[
|
|
("o/r", 1, 100, "a"),
|
|
("o/r", 1, 200, "b"),
|
|
("o/r", 2, 300, "c"),
|
|
],
|
|
)
|
|
items = eb.read_review_items(path)
|
|
assert [i["id"] for i in items] == ["o__r__pr1", "o__r__pr2"]
|
|
assert items[0]["metadata"]["reviews_run"] == 2
|
|
|
|
|
|
def test_items_are_not_flagged_as_human_labelled(tmp_path):
|
|
path = _db(tmp_path, [("o/r", 1, 100, "a")])
|
|
assert eb.read_review_items(path)[0]["metadata"]["labelled_by_human"] is False
|
|
|
|
|
|
# --- metadata facets ------------------------------------------------------
|
|
|
|
def _md(findings=(), repo="netcracker/interview", pr=29):
|
|
return eb._item_metadata(
|
|
repo=repo, pr=pr, head_sha="abc", reviews_run=2, last_seen=1788189422,
|
|
findings=[{"severity": s} for s in findings],
|
|
)
|
|
|
|
|
|
def test_metadata_carries_the_repo_for_filtering():
|
|
assert _md()["repo"] == "netcracker/interview"
|
|
|
|
|
|
def test_metadata_splits_owner_from_repo_name():
|
|
"""A filter on the joined repo can match one repo; owner matches an org."""
|
|
md = _md()
|
|
assert md["owner"] == "netcracker"
|
|
assert md["repo_name"] == "interview"
|
|
|
|
|
|
def test_owner_falls_back_when_the_repo_is_unqualified():
|
|
md = _md(repo="standalone")
|
|
assert md["owner"] == "standalone"
|
|
assert md["repo_name"] == "standalone"
|
|
|
|
|
|
def test_metadata_values_are_filterable_primitives():
|
|
"""Nested objects and lists are not reachable from the filter bar."""
|
|
for key, value in _md(["high"]).items():
|
|
assert isinstance(value, (str, int, float, bool)), key
|
|
|
|
|
|
def test_max_severity_is_the_worst_finding():
|
|
assert _md(["low", "critical", "medium"])["max_severity"] == "critical"
|
|
|
|
|
|
def test_max_severity_is_none_not_absent_for_a_silent_review():
|
|
md = _md([])
|
|
assert md["max_severity"] == "none"
|
|
assert md["has_findings"] is False
|
|
|
|
|
|
def test_unknown_severity_does_not_win_the_max():
|
|
assert _md(["banana", "low"])["max_severity"] == "low"
|
|
|
|
|
|
def test_severity_comparison_ignores_case():
|
|
assert _md(["HIGH"])["max_severity"] == "high"
|
|
|
|
|
|
def test_finding_count_matches_the_findings():
|
|
md = _md(["low", "low"])
|
|
assert md["finding_count"] == 2
|
|
assert md["has_findings"] is True
|
|
|
|
|
|
def test_last_reviewed_is_exposed_both_ways():
|
|
"""The epoch sorts; the ISO string is what a human reads in a filter."""
|
|
md = _md()
|
|
assert md["last_reviewed_at"] == 1788189422
|
|
assert md["last_reviewed_iso"].startswith("2026-08-31T")
|