diff --git a/CHANGELOG.md b/CHANGELOG.md index 791838e..9f8f0d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ All notable changes to SkillEvaluator are documented in this file. ### Fixed +- `create-eval-dataset --refine` resolves Harbor trial case ids from persisted + `reward.json` `entry_id` metadata, using folder-name parsing only as an + unambiguous legacy fallback. - Windows personal-path PII now flags `C:\Users\...` usernames that start with `s` (for example `steve`), matching the intended whitespace class rather than excluding the letter `s` ([#87](https://github.com/NVIDIA/SkillEvaluator/issues/87)). diff --git a/src/skillevaluator/tier3/generate_dataset.py b/src/skillevaluator/tier3/generate_dataset.py index 24d0206..a85280b 100644 --- a/src/skillevaluator/tier3/generate_dataset.py +++ b/src/skillevaluator/tier3/generate_dataset.py @@ -503,6 +503,45 @@ def _ensure_project_imports(): sys.path.insert(0, src_dir) +def _looks_like_harbor_suffix(suffix: str) -> bool: + """True when a ``__`` tail looks like Harbor's random or attempt suffix.""" + if not suffix: + return False + lowered = suffix.lower() + if lowered.startswith("attempt"): + return True + return len(suffix) <= 12 and suffix.isalnum() and any(ch.isdigit() for ch in suffix) + + +def _read_reward_entry_id(trial_dir: Path) -> str: + for reward_path in (trial_dir / "reward.json", trial_dir / "verifier" / "reward.json"): + if not reward_path.is_file(): + continue + try: + payload = json.loads(reward_path.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError): + continue + if isinstance(payload, dict): + entry_id = str(payload.get("entry_id") or "").strip() + if entry_id: + return entry_id + return "" + + +def _case_id_from_trial_dir(trial_dir: Path) -> str: + """Resolve eval case id from reward metadata, with conservative folder fallback.""" + if entry_id := _read_reward_entry_id(trial_dir): + return entry_id + + name = trial_dir.name + if "__" not in name: + return name + prefix, suffix = name.split("__", 1) + if prefix and _looks_like_harbor_suffix(suffix): + return prefix + return name + + def _discover_trajectories( skill_path: Path, from_results: str | None = None, @@ -541,7 +580,9 @@ def _discover_trajectories( for trial_dir in sorted(trials_dir.iterdir()): if not trial_dir.is_dir(): continue - case_id = trial_dir.name + case_id = _case_id_from_trial_dir(trial_dir) + if not case_id: + continue traj_path = trial_dir / "trajectory.json" traj, meta = load_trajectory_with_fallback(traj_path, logs_dir=trial_dir) if traj and traj.get("steps"): diff --git a/tests/tier3/test_generate_dataset_results.py b/tests/tier3/test_generate_dataset_results.py index 940c00c..5cce8e8 100644 --- a/tests/tier3/test_generate_dataset_results.py +++ b/tests/tier3/test_generate_dataset_results.py @@ -6,6 +6,7 @@ import stat import sys from concurrent.futures import ThreadPoolExecutor +from pathlib import Path import pytest @@ -37,6 +38,114 @@ def test_discover_trajectories_uses_env_results_root(tmp_path, monkeypatch): assert _discover_trajectories(skill) == {"case-001": trajectory} +def test_discover_trajectories_maps_harbor_trial_folder_to_case_id(tmp_path, monkeypatch): + """Harbor persists trials as ``{case_id}__{suffix}``; refine looks up by case id.""" + skill = tmp_path / "demo" + skill.mkdir() + results_root = tmp_path / "results" + run_id = "20260709_120000" + run_dir = results_root / "demo" / run_id + trial = run_dir / "claude-code" / "with-skill" / "trials" / "demo-001__Lmi47iy" + trial.mkdir(parents=True) + trajectory = {"steps": [{"tool_calls": [{"tool": "Read"}]}]} + trial.joinpath("trajectory.json").write_text(json.dumps(trajectory), encoding="utf-8") + (run_dir / "run_config.json").write_text("{}", encoding="utf-8") + (run_dir / "result.json").write_text(json.dumps({"run_id": run_id}), encoding="utf-8") + (results_root / "demo" / "latest").symlink_to(run_id) + + monkeypatch.setenv("SKILLEVALUATOR_RESULTS_DIR", str(results_root)) + + found = _discover_trajectories(skill) + assert "demo-001" in found + assert "demo-001__Lmi47iy" not in found + assert found["demo-001"] == trajectory + + +def _write_results_trial( + tmp_path: Path, + *, + skill_name: str, + trial_folder: str, + trajectory: dict[str, object], + reward: dict[str, object] | None = None, +) -> Path: + skill = tmp_path / skill_name + skill.mkdir(exist_ok=True) + results_root = tmp_path / "results" + run_id = "20260709_120000" + run_dir = results_root / skill_name / run_id + trial = run_dir / "claude-code" / "with-skill" / "trials" / trial_folder + trial.mkdir(parents=True) + trial.joinpath("trajectory.json").write_text(json.dumps(trajectory), encoding="utf-8") + if reward is not None: + trial.joinpath("reward.json").write_text(json.dumps(reward), encoding="utf-8") + (run_dir / "run_config.json").write_text("{}", encoding="utf-8") + (run_dir / "result.json").write_text(json.dumps({"run_id": run_id}), encoding="utf-8") + (results_root / skill_name / "latest").symlink_to(run_id) + return skill + + +def test_discover_trajectories_prefers_reward_entry_id_over_folder_prefix(tmp_path, monkeypatch): + """Stop-on-pass folders must not collapse to the job prefix when entry_id is present.""" + trajectory = {"steps": [{"tool_calls": [{"tool": "Read"}]}]} + skill = _write_results_trial( + tmp_path, + skill_name="demo", + trial_folder="harbor-job__demo-001__Lmi47iy", + trajectory=trajectory, + reward={"entry_id": "demo-001", "overall": 1.0}, + ) + monkeypatch.setenv("SKILLEVALUATOR_RESULTS_DIR", str(tmp_path / "results")) + + found = _discover_trajectories(skill) + assert list(found) == ["demo-001"] + assert found["demo-001"] == trajectory + + +def test_discover_trajectories_keeps_distinct_case_ids_with_double_underscore(tmp_path, monkeypatch): + """Case ids containing ``__`` must not collapse to a shared prefix without metadata.""" + trajectory_a = {"steps": [{"message": "a"}]} + trajectory_b = {"steps": [{"message": "b"}]} + skill = tmp_path / "demo" + skill.mkdir() + results_root = tmp_path / "results" + run_id = "20260709_120000" + run_dir = results_root / "demo" / run_id + trials_dir = run_dir / "claude-code" / "with-skill" / "trials" + for folder, traj, entry_id in ( + ("case__one", trajectory_a, "case__one"), + ("case__two", trajectory_b, "case__two"), + ): + trial = trials_dir / folder + trial.mkdir(parents=True) + trial.joinpath("trajectory.json").write_text(json.dumps(traj), encoding="utf-8") + trial.joinpath("reward.json").write_text(json.dumps({"entry_id": entry_id}), encoding="utf-8") + (run_dir / "run_config.json").write_text("{}", encoding="utf-8") + (run_dir / "result.json").write_text(json.dumps({"run_id": run_id}), encoding="utf-8") + (results_root / "demo" / "latest").symlink_to(run_id) + monkeypatch.setenv("SKILLEVALUATOR_RESULTS_DIR", str(results_root)) + + found = _discover_trajectories(skill) + assert set(found) == {"case__one", "case__two"} + assert found["case__one"] == trajectory_a + assert found["case__two"] == trajectory_b + + +def test_discover_trajectories_ambiguous_folder_without_reward_uses_full_name(tmp_path, monkeypatch): + """Without reward metadata, ambiguous ``__`` folders keep the full directory name.""" + trajectory = {"steps": [{"tool_calls": []}]} + skill = _write_results_trial( + tmp_path, + skill_name="demo", + trial_folder="case__one", + trajectory=trajectory, + ) + monkeypatch.setenv("SKILLEVALUATOR_RESULTS_DIR", str(tmp_path / "results")) + + found = _discover_trajectories(skill) + assert list(found) == ["case__one"] + + def test_discover_trajectories_results_dir_overrides_env(tmp_path, monkeypatch): skill = tmp_path / "my-skill" skill.mkdir()