mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-12 23:12:23 -06:00
fix(eval): address Copilot review — checkout-agnostic docs + skill validation
- Docstrings/help said the treatment skill 'composes into the system message'. This harness runs on both checkouts (system on main, a context turn on the placement-refactor branch), so the wording now describes the natural set_skill composition path without asserting a placement. - Validate each skill-bearing case's 'skill' shape up front (driver + CLI) so a malformed dataset fails with a clear error, not a mid-run KeyError. Pinned by test_rejects_malformed_skill.
This commit is contained in:
@@ -147,6 +147,32 @@ class TestAdherenceLift:
|
||||
assert row["n_runs"] == 3
|
||||
assert result["mean_lift"] == pytest.approx(2.0 / 3.0)
|
||||
|
||||
def test_rejects_malformed_skill(self) -> None:
|
||||
# A skill missing 'content' (or 'name') fails fast with a clear error,
|
||||
# not a KeyError mid-run (Copilot review). Validation raises before any
|
||||
# arm runs, so no _run_iteration stub is needed.
|
||||
cases = [
|
||||
{
|
||||
"id": "bad-skill",
|
||||
"skill": {"name": "x"}, # missing 'content'
|
||||
"user_prompt": "do x",
|
||||
"expected_actions": [{"tool": "search"}],
|
||||
}
|
||||
]
|
||||
with pytest.raises(ValueError, match="non-empty 'name' and 'content'"):
|
||||
run_skill_adherence(
|
||||
client=None,
|
||||
base_url="http://localhost:9/v1",
|
||||
api_key="dummy",
|
||||
model="test-model",
|
||||
cases=cases,
|
||||
n_runs=1,
|
||||
temperature=0.7,
|
||||
max_tokens=1024,
|
||||
reasoning_effort="medium",
|
||||
context_window=8192,
|
||||
)
|
||||
|
||||
def test_skipped_when_no_skill(self, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
# A case with no skill is not measurable — it must be skipped, not
|
||||
# crash, and must not contribute to the mean.
|
||||
|
||||
+10
-2
@@ -48,6 +48,14 @@ def _run_skill_adherence_cli(
|
||||
raise SystemExit(f"Test case {i} missing required 'id' field")
|
||||
if "user_prompt" not in case:
|
||||
raise SystemExit(f"Test case '{case.get('id', i)}' missing 'user_prompt'")
|
||||
skill = case.get("skill")
|
||||
if skill is not None and (
|
||||
not isinstance(skill, dict) or not skill.get("name") or not skill.get("content")
|
||||
):
|
||||
raise SystemExit(
|
||||
f"Test case '{case['id']}' has a malformed 'skill' — it must be an "
|
||||
"object with non-empty 'name' and 'content'"
|
||||
)
|
||||
if not any(c.get("skill") for c in cases):
|
||||
raise SystemExit("No cases carry a 'skill' — nothing to measure for adherence")
|
||||
|
||||
@@ -179,8 +187,8 @@ def main() -> None:
|
||||
action="store_true",
|
||||
help=(
|
||||
"Measure skill adherence: for each case carrying a 'skill', run a "
|
||||
"treatment arm (skill composed into the system message) vs a control "
|
||||
"arm (no skill) and report the pass-rate lift"
|
||||
"treatment arm (skill applied via the real set_skill composition "
|
||||
"path) vs a control arm (no skill) and report the pass-rate lift"
|
||||
),
|
||||
)
|
||||
parser.add_argument(
|
||||
|
||||
+20
-5
@@ -452,8 +452,11 @@ def _run_single_test(
|
||||
When ``skill_mode`` is True the session is built WITHOUT a system-prompt
|
||||
override so the model runs under turnstone's natural prompt composition
|
||||
(the base identity under test). If ``skill`` is given it is seeded into
|
||||
the temp DB and activated via the real ``set_skill`` path, so the skill
|
||||
body composes into the system message exactly as it would in production;
|
||||
the temp DB and activated via the real ``set_skill`` path — the skill
|
||||
flows through turnstone's natural composition exactly as in production,
|
||||
landing wherever THAT checkout places a named skill (the system message,
|
||||
or a separate context turn). This harness measures adherence regardless
|
||||
of placement, which is the whole point of comparing across checkouts.
|
||||
``skill`` None is the control arm (natural default, no skill). When
|
||||
``skill_mode`` is False behaviour is unchanged — the system prompt is
|
||||
overridden as before.
|
||||
@@ -519,7 +522,8 @@ def _run_single_test(
|
||||
client=run_client,
|
||||
model=model,
|
||||
# skill_mode uses turnstone's natural composition (no override)
|
||||
# so the skill can fold into the system message under test.
|
||||
# so the skill folds in wherever the checkout under test places
|
||||
# a named skill (system message, or a separate context turn).
|
||||
system_prompt_override=None if skill_mode else system_prompt,
|
||||
instructions=None,
|
||||
temperature=temperature,
|
||||
@@ -1254,8 +1258,9 @@ def run_skill_adherence(
|
||||
For every case that carries a ``skill`` this runs two arms ``n_runs``
|
||||
times each, scoring both against the case's ``expected_actions``:
|
||||
|
||||
* **treatment** — the skill is composed into the system message via the
|
||||
real ``set_skill`` path (``skill_mode=True, skill=<case skill>``);
|
||||
* **treatment** — the skill is applied via the real ``set_skill``
|
||||
composition path (``skill_mode=True, skill=<case skill>``); where its
|
||||
body lands (system message or a context turn) depends on the checkout;
|
||||
* **control** — the same base identity with no skill
|
||||
(``skill_mode=True, skill=None``).
|
||||
|
||||
@@ -1269,6 +1274,16 @@ def run_skill_adherence(
|
||||
case_results: list[dict[str, Any]] = []
|
||||
skill_cases = [c for c in cases if c.get("skill")]
|
||||
|
||||
# Validate skill shape up front so a malformed dataset fails with a clear
|
||||
# message instead of a KeyError mid-run (after arms have already started).
|
||||
for case in skill_cases:
|
||||
s = case["skill"]
|
||||
if not isinstance(s, dict) or not s.get("name") or not s.get("content"):
|
||||
raise ValueError(
|
||||
f"case {case.get('id', '?')!r}: 'skill' must be an object with "
|
||||
"non-empty 'name' and 'content'"
|
||||
)
|
||||
|
||||
for ci, case in enumerate(skill_cases):
|
||||
skill = case["skill"]
|
||||
# Drop the skill key from the case handed to the runner — it is
|
||||
|
||||
Reference in New Issue
Block a user