|
| 1 | +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. |
| 2 | +# SPDX-License-Identifier: Apache-2.0 |
| 3 | + |
| 4 | +"""Refuse to score an Insight suite whose tasks were never filled in. |
| 5 | +
|
| 6 | +Insight-driven mode has Eval Author write tasks from production traces by filling |
| 7 | +the placeholders in `dataset/task-template/`. Nothing upstream requires it to |
| 8 | +succeed: `fill_task_template` is instructed to leave unfillable placeholders |
| 9 | +as-is, and `InsightSuite.validate` only checks that `instruction.md` is *non-empty* |
| 10 | +-- which a file still containing `<QUESTION>` is. |
| 11 | +
|
| 12 | +An unfilled task asks a question no agent can parse and expects an answer nothing |
| 13 | +produces, so it scores 0 whether or not the weakness was repaired. A run made of |
| 14 | +those reads as "the Experimentalist failed to fix it" while measuring nothing at |
| 15 | +all -- a confident wrong answer, which is the one output a fixture must never give. |
| 16 | +
|
| 17 | +These assertions turn that into a named failure. They do not make Mode 1 work; they |
| 18 | +make it honest about which component fell over. |
| 19 | +
|
| 20 | +Point SMOKE_EXPERIMENT_DIR at an experiment directory to run them; they skip |
| 21 | +otherwise. |
| 22 | +
|
| 23 | + SMOKE_EXPERIMENT_DIR=/tmp/smoke-insight-g1 uv run pytest \\ |
| 24 | + plugins/nemo-experimentalist/tests/experimentalist/test_smoke_agent_insight_suite.py -v |
| 25 | +""" |
| 26 | + |
| 27 | +from __future__ import annotations |
| 28 | + |
| 29 | +import json |
| 30 | +import os |
| 31 | +import re |
| 32 | +from pathlib import Path |
| 33 | + |
| 34 | +import pytest |
| 35 | + |
| 36 | +_EXPERIMENT_DIR = Path(os.environ.get("SMOKE_EXPERIMENT_DIR", "/nonexistent")) |
| 37 | +_TEMPLATE_DIR = Path(__file__).resolve().parents[2] / "examples" / "smoke-agent" / "dataset" / "task-template" |
| 38 | + |
| 39 | +# `<QUESTION>`, `<FIELD>`, `<EXPECTED>`. Read from the template rather than listed |
| 40 | +# here, so adding a placeholder extends this guard instead of silently escaping it. |
| 41 | +_PLACEHOLDER = re.compile(r"<[A-Z][A-Z0-9_]*>") |
| 42 | + |
| 43 | +# Files a placeholder can hide in. tests/test.sh is excluded on purpose: it is synced |
| 44 | +# from dataset/_shared and carries no placeholders, and its `<key>=<value>` prose |
| 45 | +# would false-positive. |
| 46 | +_FILLABLE = ("instruction.md", "task.toml", "tests/expected.txt") |
| 47 | + |
| 48 | + |
| 49 | +def _require_experiment_dir() -> Path: |
| 50 | + if not _EXPERIMENT_DIR.is_dir(): |
| 51 | + pytest.skip(f"set SMOKE_EXPERIMENT_DIR to an experiment directory (got {_EXPERIMENT_DIR})") |
| 52 | + return _EXPERIMENT_DIR |
| 53 | + |
| 54 | + |
| 55 | +def _template_placeholders() -> set[str]: |
| 56 | + """Every placeholder token the committed template declares.""" |
| 57 | + found: set[str] = set() |
| 58 | + for name in _FILLABLE: |
| 59 | + path = _TEMPLATE_DIR / name |
| 60 | + if path.is_file(): |
| 61 | + found.update(_PLACEHOLDER.findall(path.read_text(encoding="utf-8"))) |
| 62 | + return found |
| 63 | + |
| 64 | + |
| 65 | +def _suite_dirs(experiment_dir: Path) -> list[Path]: |
| 66 | + """Materialized Insight suites, located by their manifest rather than by guessing. |
| 67 | +
|
| 68 | + The path is `eval-and-optimize/eval_author/<insight-slug>/insight-suite/`, but the |
| 69 | + slug is derived from the insight id, so searching for the manifest is both simpler |
| 70 | + and robust to that scheme changing. |
| 71 | + """ |
| 72 | + root = experiment_dir / "eval-and-optimize" / "eval_author" |
| 73 | + if not root.is_dir(): |
| 74 | + return [] |
| 75 | + return sorted(manifest.parent for manifest in root.rglob("insight-suite/manifest.json")) |
| 76 | + |
| 77 | + |
| 78 | +def _materialized_tasks(suite_dir: Path) -> list[Path]: |
| 79 | + """Task directories the manifest claims, so a stray directory is not mistaken for one.""" |
| 80 | + manifest = json.loads((suite_dir / "manifest.json").read_text(encoding="utf-8")) |
| 81 | + tasks = manifest.get("tasks") |
| 82 | + if not isinstance(tasks, list): |
| 83 | + return [] |
| 84 | + return [suite_dir / entry["path"] for entry in tasks if isinstance(entry, dict) and entry.get("path")] |
| 85 | + |
| 86 | + |
| 87 | +def test_the_template_still_declares_placeholders() -> None: |
| 88 | + """Guards the guard: with no placeholders to find, everything below passes vacuously.""" |
| 89 | + declared = _template_placeholders() |
| 90 | + assert declared, ( |
| 91 | + f"{_TEMPLATE_DIR} declares no <PLACEHOLDER> tokens, so the checks in this module " |
| 92 | + "cannot fail. Either the template changed shape or this pattern is wrong." |
| 93 | + ) |
| 94 | + |
| 95 | + |
| 96 | +def test_a_suite_was_materialized() -> None: |
| 97 | + """Mode 1 with trace_refs must produce a suite; an empty one is not a passing run.""" |
| 98 | + experiment_dir = _require_experiment_dir() |
| 99 | + suites = _suite_dirs(experiment_dir) |
| 100 | + assert suites, ( |
| 101 | + f"no Insight suite under {experiment_dir}/eval-and-optimize/eval_author/. Either this " |
| 102 | + "was a Mode 2 run, or Eval Author produced nothing -- check that the Insight's " |
| 103 | + "trace_refs resolve in the target workspace." |
| 104 | + ) |
| 105 | + for suite in suites: |
| 106 | + assert _materialized_tasks(suite), f"{suite}/manifest.json lists no tasks" |
| 107 | + |
| 108 | + |
| 109 | +def test_no_materialized_task_still_contains_a_placeholder() -> None: |
| 110 | + """The failure this module exists for.""" |
| 111 | + experiment_dir = _require_experiment_dir() |
| 112 | + declared = _template_placeholders() |
| 113 | + unfilled: list[str] = [] |
| 114 | + |
| 115 | + for suite in _suite_dirs(experiment_dir): |
| 116 | + for task_dir in _materialized_tasks(suite): |
| 117 | + for name in _FILLABLE: |
| 118 | + path = task_dir / name |
| 119 | + if not path.is_file(): |
| 120 | + continue |
| 121 | + remaining = sorted(set(_PLACEHOLDER.findall(path.read_text(encoding="utf-8"))) & declared) |
| 122 | + if remaining: |
| 123 | + unfilled.append(f"{task_dir.name}/{name}: {', '.join(remaining)}") |
| 124 | + |
| 125 | + assert not unfilled, ( |
| 126 | + "Eval Author did not fill the task template:\n " |
| 127 | + + "\n ".join(unfilled) |
| 128 | + + "\n\nEvery task in this suite scores 0 regardless of the agent, so this run cannot " |
| 129 | + "measure whether the weakness was fixed. Read the result as a broken test, not as a " |
| 130 | + "failed repair." |
| 131 | + ) |
| 132 | + |
| 133 | + |
| 134 | +def test_expected_answers_are_not_empty() -> None: |
| 135 | + """A blank expectation compares equal to a blank answer, which would score 1.0. |
| 136 | +
|
| 137 | + Distinct from the placeholder check: a template can be filled with nothing at all, |
| 138 | + and the verifier's fail-closed guard only covers an *unreadable* fixture, not an |
| 139 | + empty one. |
| 140 | + """ |
| 141 | + experiment_dir = _require_experiment_dir() |
| 142 | + empty = [ |
| 143 | + f"{task_dir.name}/tests/expected.txt" |
| 144 | + for suite in _suite_dirs(experiment_dir) |
| 145 | + for task_dir in _materialized_tasks(suite) |
| 146 | + if (task_dir / "tests" / "expected.txt").is_file() |
| 147 | + and not (task_dir / "tests" / "expected.txt").read_text(encoding="utf-8").strip() |
| 148 | + ] |
| 149 | + assert not empty, "materialized tasks have an empty expected answer: " + ", ".join(empty) |
| 150 | + |
| 151 | + |
| 152 | +def test_every_suite_task_was_actually_scored() -> None: |
| 153 | + """A task that never ran is worse than one that scored 0: it leaves no trace at all. |
| 154 | +
|
| 155 | + In the first Mode 1 run the generated tasks referenced a stale image tag, so their |
| 156 | + containers never started -- `pull access denied for smoke-agent-env`. No |
| 157 | + `reward.json` was written, the tasks were dropped from the aggregate, and the run |
| 158 | + reported a perfectly ordinary baseline over the three *real* tasks. The Insight |
| 159 | + suite contributed nothing and nothing said so. |
| 160 | +
|
| 161 | + A missing metric and a legitimate zero are treated very differently by the loop; |
| 162 | + this is the check that tells them apart. |
| 163 | + """ |
| 164 | + experiment_dir = _require_experiment_dir() |
| 165 | + results = experiment_dir / "eval-and-optimize" / "results" |
| 166 | + if not results.is_dir(): |
| 167 | + pytest.skip("no results/ directory; the run did not reach evaluation") |
| 168 | + |
| 169 | + expected_slugs = {task.name for suite in _suite_dirs(experiment_dir) for task in _materialized_tasks(suite)} |
| 170 | + if not expected_slugs: |
| 171 | + pytest.skip("no materialized suite tasks to check") |
| 172 | + |
| 173 | + # Harbor names a trial `<task-name-truncated>__<suffix>`, and the truncation length |
| 174 | + # is its business, not ours. Strip the suffix and ask whether what remains prefixes |
| 175 | + # a slug, rather than guessing how much survived -- an earlier version assumed 40 |
| 176 | + # characters, matched nothing against the real 32, and passed while the tasks it |
| 177 | + # was meant to catch had never run. |
| 178 | + trials_by_slug: dict[str, list[Path]] = {slug: [] for slug in expected_slugs} |
| 179 | + for trial in results.rglob("*"): |
| 180 | + if not trial.is_dir(): |
| 181 | + continue |
| 182 | + base = re.sub(r"__[A-Za-z0-9]+$", "", trial.name) |
| 183 | + if len(base) < 12: |
| 184 | + continue |
| 185 | + for slug in expected_slugs: |
| 186 | + if slug.startswith(base): |
| 187 | + trials_by_slug[slug].append(trial) |
| 188 | + |
| 189 | + unscored = sorted( |
| 190 | + slug |
| 191 | + for slug, trials in trials_by_slug.items() |
| 192 | + if trials and not any((trial / "verifier" / "reward.json").is_file() for trial in trials) |
| 193 | + ) |
| 194 | + |
| 195 | + assert not unscored, ( |
| 196 | + "materialized tasks produced no reward.json, so they never ran:\n " |
| 197 | + + "\n ".join(unscored) |
| 198 | + + "\n\nCheck the trial log for a container failure -- a stale [environment].docker_image " |
| 199 | + "is the usual cause. These tasks were silently dropped from the aggregate, so the run's " |
| 200 | + "scores describe only the tasks that did run." |
| 201 | + ) |
| 202 | + |
| 203 | + |
| 204 | +_GRAMMAR = ( |
| 205 | + re.compile(r"what is the \w+ of ", re.IGNORECASE), |
| 206 | + re.compile(r"how many .* in the \w+ department", re.IGNORECASE), |
| 207 | + re.compile(r"what is the total \w+ in the \w+ (?:department|role)", re.IGNORECASE), |
| 208 | +) |
| 209 | + |
| 210 | + |
| 211 | +def test_the_grammar_matches_the_committed_tasks() -> None: |
| 212 | + """Keep the patterns tied to the tasks, not to prose about them. |
| 213 | +
|
| 214 | + This runs without an experiment directory, because it is the check that would have |
| 215 | + caught the mistake it exists for: the first version of the list came from the |
| 216 | + template README, which still described the pre-rewording `total ... for the` |
| 217 | + phrasing. It then flagged correctly-generated questions as off-grammar -- a guard |
| 218 | + accusing the component it was written to exonerate. |
| 219 | + """ |
| 220 | + groups = _TEMPLATE_DIR.parent / "groups" |
| 221 | + if not groups.is_dir(): |
| 222 | + pytest.skip("no committed groups to check against") |
| 223 | + unmatched = [ |
| 224 | + f"{path.parent.parent.parent.name}/{path.parent.name}: {path.read_text(encoding='utf-8').splitlines()[0]}" |
| 225 | + for path in sorted(groups.rglob("instruction.md")) |
| 226 | + if not any(pattern.search(path.read_text(encoding="utf-8")) for pattern in _GRAMMAR) |
| 227 | + ] |
| 228 | + assert not unmatched, ( |
| 229 | + "committed tasks fall outside the grammar this module enforces, so it would " |
| 230 | + "reject correctly-generated questions:\n " + "\n ".join(unmatched) |
| 231 | + ) |
| 232 | + |
| 233 | + |
| 234 | +def test_generated_questions_stay_inside_the_agent_grammar() -> None: |
| 235 | + """A question outside the three parsed forms fails for the wrong reason. |
| 236 | +
|
| 237 | + It looks exactly like the weakness under test -- the agent returns its fallback -- |
| 238 | + but the cause is phrasing the agent was never able to parse, so the run measures the |
| 239 | + template rather than the Experimentalist. Warned about in the template README; this |
| 240 | + is that warning enforced. |
| 241 | + """ |
| 242 | + experiment_dir = _require_experiment_dir() |
| 243 | + off_grammar: list[str] = [] |
| 244 | + |
| 245 | + for suite in _suite_dirs(experiment_dir): |
| 246 | + for task_dir in _materialized_tasks(suite): |
| 247 | + instruction = task_dir / "instruction.md" |
| 248 | + if not instruction.is_file(): |
| 249 | + continue |
| 250 | + text = instruction.read_text(encoding="utf-8") |
| 251 | + if not any(pattern.search(text) for pattern in _GRAMMAR): |
| 252 | + off_grammar.append(f"{task_dir.name}: {text.strip().splitlines()[0][:80]!r}") |
| 253 | + |
| 254 | + assert not off_grammar, ( |
| 255 | + "generated questions fall outside the grammar the agent parses:\n " |
| 256 | + + "\n ".join(off_grammar) |
| 257 | + + "\n\nThese fail on the baseline for the wrong reason, which is indistinguishable " |
| 258 | + "from the weakness under test. See dataset/task-template/README.md." |
| 259 | + ) |
0 commit comments