diff --git a/pilot/eval_bootstrap.py b/pilot/eval_bootstrap.py index be831d0..a4cfdac 100644 --- a/pilot/eval_bootstrap.py +++ b/pilot/eval_bootstrap.py @@ -111,6 +111,20 @@ def ensure_score_configs() -> dict: # 2. Dataset from recorded reviews # --------------------------------------------------------------------------- +def item_id(repo: str, pr) -> str: + """A dataset-item id that survives being put in a URL path. + + The obvious `{repo}#{pr}` is unusable: the UI routes items as + `/datasets/{id}/items/{item_id}`, so the `/` in `owner/repo` splits into + extra path segments and everything after the `#` is a fragment the browser + never sends. The item is created fine and then 404s when opened. + + Session ids elsewhere keep the `{repo}#{pr}` form — those are never path + segments, and `feedback_scores` depends on that shape. + """ + return f"{repo.replace('/', '__')}__pr{pr}" + + def read_review_items(db_path: str) -> list[dict]: """One dataset item per (repo, pr) the reviewer has run on. @@ -140,7 +154,7 @@ def read_review_items(db_path: str) -> list[dict]: ).fetchall() items.append( { - "id": f'{row["repo"]}#{row["pr"]}', + "id": item_id(row["repo"], row["pr"]), "input": { "repo": row["repo"], "pr": int(row["pr"]), diff --git a/tests/pilot/test_eval_bootstrap.py b/tests/pilot/test_eval_bootstrap.py new file mode 100644 index 0000000..9185ebc --- /dev/null +++ b/tests/pilot/test_eval_bootstrap.py @@ -0,0 +1,95 @@ +"""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