From 8c1dce95e886bf1f5fecb786b54839561dde880c Mon Sep 17 00:00:00 2001 From: Jeffrey Sperling Date: Sat, 14 Mar 2026 00:38:43 -0700 Subject: [PATCH] Harden local search evaluation harness Isolate eval subprocesses from local yt-dlp config and fix nDCG normalization against the judged pool. Validation: uv run python -m unittest tests.test_evaluate_search_quality --- docs/search-quality-eval.md | 1 + scripts/evaluate_search_quality.py | 49 ++++++++++++++++++++++++--- tests/test_evaluate_search_quality.py | 33 ++++++++++++++++++ 3 files changed, 78 insertions(+), 5 deletions(-) diff --git a/docs/search-quality-eval.md b/docs/search-quality-eval.md index 574ae48..5d9cc84 100644 --- a/docs/search-quality-eval.md +++ b/docs/search-quality-eval.md @@ -43,5 +43,6 @@ Notes: - The script forces a clean env-based auth path when it shells out to `last30days.py`. - It passes `XAI_API_KEY`, `OPENAI_API_KEY`, and `SCRAPECREATORS_API_KEY`, but intentionally does not pass browser-cookie X auth. That keeps evaluation runs on the popup-free path. +- It also strips `node` from the eval `PATH` and wraps `yt-dlp` with `--ignore-config`, so older revisions do not inherit local browser-cookie config either. - `Jaccard` and retention are regression guards, not truth metrics. - `Precision@5` and `nDCG@5` are only as good as the judged pool. They help compare revisions, but they are not a substitute for a larger labeled benchmark. diff --git a/scripts/evaluate_search_quality.py b/scripts/evaluate_search_quality.py index fa86393..909678b 100644 --- a/scripts/evaluate_search_quality.py +++ b/scripts/evaluate_search_quality.py @@ -12,6 +12,7 @@ import argparse import json import math import os +import shlex import shutil import subprocess import sys @@ -68,6 +69,31 @@ def path_without_node(path_value: str) -> str: return os.pathsep.join(parts) +def write_exec_wrapper(path: Path, target: str, fixed_args: List[str]) -> None: + quoted_target = shlex.quote(target) + quoted_args = " ".join(shlex.quote(arg) for arg in fixed_args) + path.write_text(f"#!/bin/sh\nexec {quoted_target} {quoted_args} \"$@\"\n") + path.chmod(0o755) + + +def create_eval_tool_path(eval_home: Path, base_path: str) -> str: + """Create safe wrapper binaries for local evaluation subprocesses.""" + bin_dir = eval_home / "bin" + bin_dir.mkdir(parents=True, exist_ok=True) + + real_ytdlp = shutil.which("yt-dlp") + if real_ytdlp: + write_exec_wrapper( + bin_dir / "yt-dlp", + real_ytdlp, + ["--ignore-config", "--no-cookies-from-browser"], + ) + + if not base_path: + return str(bin_dir) + return os.pathsep.join([str(bin_dir), base_path]) + + def stable_item_key(source: str, item: Dict[str, Any]) -> str: url = str(item.get("url") or "").strip() if url: @@ -142,7 +168,12 @@ def precision_at_k(ranking: List[Dict[str, Any]], judgments: Dict[str, int], k: return hits / len(top) -def ndcg_at_k(ranking: List[Dict[str, Any]], judgments: Dict[str, int], k: int) -> float: +def ndcg_at_k( + ranking: List[Dict[str, Any]], + judgments: Dict[str, int], + k: int, + judged_pool: Optional[List[Dict[str, Any]]] = None, +) -> float: top = ranking[:k] if not top: return 0.0 @@ -154,7 +185,11 @@ def ndcg_at_k(ranking: List[Dict[str, Any]], judgments: Dict[str, int], k: int) return total actual = [judgments.get(item["key"], 0) for item in top] - ideal = sorted(actual, reverse=True) + ideal_candidates = judged_pool or ranking + ideal = sorted( + (judgments.get(item["key"], 0) for item in ideal_candidates), + reverse=True, + )[:len(top)] ideal_score = dcg(ideal) if ideal_score == 0: return 0.0 @@ -181,10 +216,14 @@ def create_eval_env(include_web: bool) -> Tuple[Dict[str, str], Path]: config = envlib.get_config() eval_home = Path(tempfile.mkdtemp(prefix="last30days-eval-home-")) (eval_home / ".config").mkdir(parents=True, exist_ok=True) + safe_path = create_eval_tool_path( + eval_home, + path_without_node(os.environ.get("PATH", "")), + ) passthrough = { "HOME": str(eval_home), "XDG_CONFIG_HOME": str(eval_home / ".config"), - "PATH": path_without_node(os.environ.get("PATH", "")), + "PATH": safe_path, "LANG": os.environ.get("LANG", "en_US.UTF-8"), "LC_ALL": os.environ.get("LC_ALL", ""), "TMPDIR": os.environ.get("TMPDIR", ""), @@ -413,12 +452,12 @@ def summarize_topic( "query_type": query_type, "baseline": { "precision_at_5": precision_at_k(baseline_ranked, judgments, 5), - "ndcg_at_5": ndcg_at_k(baseline_ranked, judgments, 5), + "ndcg_at_5": ndcg_at_k(baseline_ranked, judgments, 5, judged_pool), "source_coverage_recall": source_coverage_recall(baseline_ranked, judged_pool, judgments), }, "candidate": { "precision_at_5": precision_at_k(candidate_ranked, judgments, 5), - "ndcg_at_5": ndcg_at_k(candidate_ranked, judgments, 5), + "ndcg_at_5": ndcg_at_k(candidate_ranked, judgments, 5, judged_pool), "source_coverage_recall": source_coverage_recall(candidate_ranked, judged_pool, judgments), }, "stability": { diff --git a/tests/test_evaluate_search_quality.py b/tests/test_evaluate_search_quality.py index 7471a52..1b94ffa 100644 --- a/tests/test_evaluate_search_quality.py +++ b/tests/test_evaluate_search_quality.py @@ -1,6 +1,7 @@ """Tests for the local search-quality evaluation harness.""" import sys +import tempfile import unittest from pathlib import Path from unittest.mock import patch @@ -35,6 +36,22 @@ class TestMetrics(unittest.TestCase): judgments = {"a": 3, "b": 0, "c": 2} self.assertGreater(evalsq.ndcg_at_k(ranking, judgments, 3), 0.8) + def test_ndcg_at_k_uses_best_items_from_judged_pool(self): + ranking = [ + {"key": "a", "source": "reddit"}, + {"key": "b", "source": "x"}, + {"key": "c", "source": "youtube"}, + ] + judged_pool = ranking + [ + {"key": "d", "source": "reddit"}, + {"key": "e", "source": "x"}, + ] + judgments = {"a": 3, "b": 0, "c": 0, "d": 3, "e": 2} + self.assertLess( + evalsq.ndcg_at_k(ranking, judgments, 3, judged_pool), + 1.0, + ) + def test_source_coverage_recall_uses_union_pool(self): judged_pool = [ {"key": "a", "source": "reddit"}, @@ -70,13 +87,29 @@ class TestRankedItems(unittest.TestCase): class TestPathWithoutNode(unittest.TestCase): def test_removes_node_entries(self): path = "/usr/bin:/tmp/node-bin:/opt/homebrew/bin" + def fake_exists(path_obj): return str(path_obj).endswith("/tmp/node-bin/node") + with patch.object(evalsq.Path, "exists", fake_exists): filtered = evalsq.path_without_node(path) self.assertEqual(filtered, "/usr/bin:/opt/homebrew/bin") +class TestEvalToolPath(unittest.TestCase): + def test_wraps_ytdlp_with_ignore_config(self): + with tempfile.TemporaryDirectory() as tmpdir: + eval_home = Path(tmpdir) + with patch.object(evalsq.shutil, "which", return_value="/opt/homebrew/bin/yt-dlp"): + path_value = evalsq.create_eval_tool_path(eval_home, "/usr/bin") + wrapper = eval_home / "bin" / "yt-dlp" + self.assertTrue(wrapper.exists()) + text = wrapper.read_text() + self.assertIn("--ignore-config", text) + self.assertIn("--no-cookies-from-browser", text) + self.assertEqual(path_value, f"{eval_home / 'bin'}:/usr/bin") + + class TestJudgeKeyResolution(unittest.TestCase): def test_prefers_google_api_key(self): config = {