diff --git a/skillopt_sleep/judges.py b/skillopt_sleep/judges.py index 9f5df1ee..eb5d6471 100644 --- a/skillopt_sleep/judges.py +++ b/skillopt_sleep/judges.py @@ -9,10 +9,24 @@ * max_chars — response length <= n * min_chars — response length >= n * contains — substring present (case-insensitive) + * not_contains — substring absent (case-insensitive) + * no_refusal — the response is not a bare refusal/abstention * tool_called — a tool with was invoked (needs a tool loop; in single-shot replay we approximate via an explicit "TOOL_CALL: " marker the agent emits) +Ops divide into two families, and the distinction is load-bearing: + + * *shape* ops (section_present, max_chars, min_chars) constrain how an answer + is formatted. They are trivially satisfiable by an optimizer — adding a + heading scores 1.0 without the answer improving at all. + * *outcome* ops (contains, not_contains, no_refusal, regex, tool_called) + constrain what the answer actually does. + +A judge built only from shape ops is a formatting checker, not a grader; see +:func:`is_shape_only`. Callers should prefer an outcome-based judge (rubric or +outcome ops) so the gate cannot be won by reformatting. + A task whose judge is {"kind": "rule", "checks": [...]} passes (hard=1.0) iff ALL checks pass; soft = fraction of checks passed. This mirrors gbrain's all-checks-must-pass rule scoring and gives the gate a smooth signal. @@ -35,6 +49,39 @@ def _section_present(response: str, name: str) -> bool: return bool(label.search(response or "")) +_REFUSAL_PREFIXES = ( + "cannot complete", + "i cannot", + "i can't", + "i'm unable", + "i am unable", + "unable to complete", + "sorry, i can't", + "sorry, i cannot", + "no can do", +) + + +def _is_refusal(response: str) -> bool: + """Detect a bare refusal: an abstention with no substantive work reported. + + A refusal that still explains what was searched and what is missing is a + useful answer, so only short responses whose opening is an abstention count. + """ + text = (response or "").strip() + if not text: + return True + # Strip leading markdown markers -- blockquote (>), list bullets (-, *), + # numbered items (1. / 1)), emphasis and headings -- BEFORE bounding the + # head, so a refusal cannot hide behind >160 marker characters. + head = re.sub(r"^(?:[>\-*_#\s]|\d+[.)])+", "", text.lower())[:160] + if not any(head.startswith(p) for p in _REFUSAL_PREFIXES): + return False + # A long response that opens with an abstention still did the work of + # explaining why; only terse dead-ends are refusals. + return len(text) < 600 + + def _check(op: str, arg: Any, response: str, tools_called: List[str]) -> Tuple[bool, str]: """Evaluate one check. @@ -60,6 +107,10 @@ def _check(op: str, arg: Any, response: str, return len(r) >= int(arg), "" if op == "contains": return str(arg).lower() in r.lower(), "" + if op == "not_contains": + return str(arg).lower() not in r.lower(), "" + if op == "no_refusal": + return not _is_refusal(r), "" if op == "tool_called": name = str(arg).lower() if any(name == t.lower() for t in tools_called): @@ -71,9 +122,53 @@ def _check(op: str, arg: Any, response: str, KNOWN_OPS = frozenset({ - "section_present", "regex", "max_chars", "min_chars", "contains", "tool_called", + "section_present", "regex", "max_chars", "min_chars", "contains", + "not_contains", "no_refusal", "tool_called", }) +# Ops that only constrain formatting. An optimizer satisfies these by editing +# the output template, which is why a judge made solely of them is gameable. +SHAPE_OPS = frozenset({"section_present", "max_chars", "min_chars"}) + + +def is_shape_only(judge: Any) -> bool: + """True when every check in ``judge`` merely constrains formatting. + + Such a judge cannot distinguish a better answer from a reformatted one, so + callers should prefer outcome grading (a rubric) instead of trusting it. + """ + if not isinstance(judge, dict): + return False + checks = judge.get("checks", []) or [] + if not isinstance(checks, list) or not checks: + return False + ops = [c.get("op") for c in checks if isinstance(c, dict)] + if not ops or len(ops) != len(checks): + return False + return all(op in SHAPE_OPS for op in ops) + + +def char_bound(arg: Any) -> int: + """Parse a ``max_chars``/``min_chars`` argument, or raise ``ValueError``. + + Shared by :func:`validate_checks` and the LLM miner so the accepted shapes + cannot drift apart: a miner that emits a bound the validator later rejects + produces a tasks file that fails its own validation. ``bool`` is refused + (it is an ``int`` subclass, so ``int(True)`` would silently become 1) and a + non-integral float is refused rather than truncated. + """ + if isinstance(arg, bool): + raise ValueError("bool is not a char bound") + if isinstance(arg, int): + return arg + if isinstance(arg, float): + if not arg.is_integer(): + raise ValueError("non-integral float is not a char bound") + return int(arg) # may raise OverflowError for inf/nan + if isinstance(arg, str) and re.fullmatch(r"[+-]?\d+", arg.strip()): + return int(arg.strip()) + raise ValueError("not a char bound") + def validate_checks(judge: Any) -> Tuple[List[str], List[str]]: """Return ``(errors, warnings)`` for a rule judge's checks. @@ -101,7 +196,7 @@ def validate_checks(judge: Any) -> Tuple[List[str], List[str]]: f"check #{i} op must be a string, got {type(op).__name__}" ) continue - if op in {"regex", "section_present", "contains", "tool_called"} and ( + if op in {"regex", "section_present", "contains", "tool_called", "not_contains"} and ( arg is None or not str(arg).strip() ): errors.append(f"check #{i} {op} needs a non-empty arg") @@ -113,20 +208,7 @@ def validate_checks(judge: Any) -> Tuple[List[str], List[str]]: errors.append(f"check #{i} regex does not compile ({exc}): {arg!r}") elif op in {"max_chars", "min_chars"}: try: - if isinstance(arg, bool): - raise ValueError - if isinstance(arg, int): - bound = arg - elif isinstance(arg, float): - if not arg.is_integer(): - raise ValueError - bound = int(arg) - elif isinstance(arg, str) and re.fullmatch( - r"[+-]?\d+", arg.strip() - ): - bound = int(arg.strip()) - else: - raise ValueError + bound = char_bound(arg) except (OverflowError, TypeError, ValueError): errors.append(f"check #{i} {op} needs an integer arg, got {arg!r}") else: @@ -136,6 +218,11 @@ def validate_checks(judge: Any) -> Tuple[List[str], List[str]]: warnings.append(f"check #{i} min_chars=0 always passes") elif op not in KNOWN_OPS: warnings.append(f"check #{i} has unknown op {op!r} — it always passes") + if not errors and is_shape_only(judge): + warnings.append( + "judge is shape-only (formatting checks); it can be satisfied by " + "reformatting rather than by a better answer" + ) return errors, warnings diff --git a/skillopt_sleep/llm_miner.py b/skillopt_sleep/llm_miner.py index f2d5caf2..6ac7845d 100644 --- a/skillopt_sleep/llm_miner.py +++ b/skillopt_sleep/llm_miner.py @@ -7,9 +7,11 @@ For each recurring intent it extracts: * a clean, generalized `intent` (the reusable task, stripped of one-off specifics) - * a `rubric` (what a good answer must satisfy) -> stored as a rule judge of - `contains`/`regex`/`section_present` checks the local judge can score, OR a - free-text rubric scored by the backend's judge() when no programmatic check fits + * a `rubric` (what a good answer must satisfy). A rubric, when present, is + always stored as the reference and scored by the backend's judge() -- it + resists the reward-hacking that literal/format checks invite. Programmatic + `contains`/`regex`/`section_present`/`tool_called` checks are kept only as a + fallback reference for the intents where the miner supplies no rubric. * a preference signal (was the user satisfied?) to weight failures It is deliberately conservative: it only emits a task when it can name a @@ -18,12 +20,11 @@ """ from __future__ import annotations -import json -import re from typing import Any, Callable, Dict, List from skillopt_sleep import prompts as prompt_registry from skillopt_sleep.backend import Backend, _extract_json +from skillopt_sleep.judges import char_bound from skillopt_sleep.mine import session_skill_hint from skillopt_sleep.types import SessionDigest, TaskRecord @@ -46,32 +47,70 @@ def _mk_task(d: SessionDigest, obj: Dict[str, Any], idx: int) -> TaskRecord | No if len(intent) < 8: return None checks = obj.get("checks") or [] - rubric = str(obj.get("rubric", "")).strip() + # A non-string rubric (e.g. a JSON null) must not become the literal "None" + # and outrank the checks; only a real string counts as a rubric. + rubric_raw = obj.get("rubric") + rubric = rubric_raw.strip() if isinstance(rubric_raw, str) else "" satisfied = bool(obj.get("satisfied", False)) - # keep only well-formed checks + # Keep only well-formed checks: the scorer runs these verbatim during + # replay, so a max_chars/min_chars with a non-integer arg (or an arg-less + # op that needs one) would crash or fail forever. Drop those here, and keep + # the accepted shapes aligned with validate_checks() so a mined tasks file + # never fails validation later (it rejects bools and negative bounds). + _needs_str_arg = {"section_present", "regex", "contains", "not_contains", "tool_called"} + # Trimming a regex would change what it matches (leading/trailing spaces are + # significant in a pattern), so only substring/tool/heading args are stripped. + _strip_arg = _needs_str_arg - {"regex"} clean_checks = [] for c in checks: - if isinstance(c, dict) and c.get("op") in { - "section_present", "regex", "contains", "max_chars", "min_chars", - }: - clean_checks.append({"op": c["op"], "arg": c.get("arg")}) + if not isinstance(c, dict): + continue + op = c.get("op") + arg = c.get("arg") + if op in _needs_str_arg: + # Store the stripped value: stray whitespace would otherwise become + # part of the required substring / tool name. + if isinstance(arg, str) and arg.strip(): + clean_checks.append( + {"op": op, "arg": arg.strip() if op in _strip_arg else arg} + ) + elif op in {"max_chars", "min_chars"}: + # Shared parser with validate_checks() so the two cannot drift: + # rejects bools, non-integral floats and inf/nan (OverflowError). + try: + bound = char_bound(arg) + except (OverflowError, TypeError, ValueError): + continue + if bound < 0: + continue + clean_checks.append({"op": op, "arg": bound}) + elif op == "no_refusal": + clean_checks.append({"op": op, "arg": None}) import hashlib tid = "llm_" + hashlib.sha256((d.project + intent).encode()).hexdigest()[:12] - if clean_checks: + judge = {"kind": "rule", "checks": clean_checks} + # The optimizer edits skill text that is prepended to the model's context, + # so ANY check for a literal string in the response can be satisfied by + # instructing the model to emit that string -- observed twice in practice, + # first with section_present and then with contains. Only semantic grading + # resists that, so a rubric always wins when the miner supplies one. + # Rule judges remain for imported gbrain-style benchmarks, which carry + # checks but no rubric. + if rubric: return TaskRecord( id=tid, project=d.project, intent=intent, - reference_kind="rule", judge={"kind": "rule", "checks": clean_checks}, + reference_kind="rubric", reference=rubric, outcome="success" if satisfied else "fail", tags=["mined:llm"], source_sessions=[d.session_id], skill_hint=session_skill_hint(d), ) - if rubric: + if clean_checks: return TaskRecord( id=tid, project=d.project, intent=intent, - reference_kind="rubric", reference=rubric, + reference_kind="rule", judge=judge, outcome="success" if satisfied else "fail", tags=["mined:llm"], source_sessions=[d.session_id], skill_hint=session_skill_hint(d), diff --git a/skillopt_sleep/prompts.py b/skillopt_sleep/prompts.py index 3553328e..b961465a 100644 --- a/skillopt_sleep/prompts.py +++ b/skillopt_sleep/prompts.py @@ -33,19 +33,29 @@ worth optimizing a skill for. From the session below, extract 0-3 reusable tasks. A good task is something the user asks for repeatedly or had to correct, where a -GENERAL rule would help next time (formatting, structure, tool-use, conventions). -Skip one-off or purely exploratory requests. +GENERAL rule would help next time (correctness, completeness, tool-use, +conventions). Skip one-off or purely exploratory requests. For each task return: - "intent": the reusable request, generalized (no one-off specifics) - "checks": a list of programmatic success checks a grader can run on a future - answer. Each check is one of: - {"op":"section_present","arg":""} + answer. Prefer checks about WHAT THE ANSWER DOES over how it is formatted: + {"op":"contains","arg":""} + {"op":"not_contains","arg":""} + {"op":"no_refusal"} {"op":"regex","arg":""} - {"op":"contains","arg":""} + {"op":"tool_called","arg":""} + Formatting checks are available but weak, because an assistant can satisfy + them by reformatting without answering any better. Use them only alongside + an outcome check, never alone: + {"op":"section_present","arg":""} {"op":"max_chars","arg":} + {"op":"min_chars","arg":} Only include checks you are confident a GOOD answer must satisfy. - - "rubric": a one-sentence description of what a good answer looks like + - "rubric": a one-sentence description of what a GOOD answer achieves — + judged on substance, not on wording or layout. ALWAYS provide this. It is + the primary grader, because an assistant can satisfy any literal-string + check simply by emitting that string. - "satisfied": true/false — did the user seem satisfied with the assistant's answer? Return ONLY a JSON array (possibly empty). No prose. diff --git a/tests/test_judges.py b/tests/test_judges.py index e4aca3d0..1f7e4b4a 100644 --- a/tests/test_judges.py +++ b/tests/test_judges.py @@ -9,7 +9,7 @@ import pytest -from skillopt_sleep.judges import KNOWN_OPS, score_rule_judge, validate_checks +from skillopt_sleep.judges import KNOWN_OPS, SHAPE_OPS, score_rule_judge, validate_checks from skillopt_sleep.tasks_file import load_tasks_file @@ -128,8 +128,9 @@ def test_zero_min_chars_is_flagged_as_toothless(self) -> None: {"checks": [{"op": "min_chars", "arg": 0}]} ) self.assertEqual(errors, []) - self.assertEqual(len(warnings), 1) - self.assertIn("always passes", warnings[0]) + # min_chars is also a shape op, so a lone one now draws a second + # warning; the toothless-bound warning must still be present. + self.assertTrue(any("always passes" in w for w in warnings), warnings) def test_empty_string_operator_arguments_are_errors(self) -> None: for op in ("regex", "section_present", "contains", "tool_called"): @@ -178,7 +179,15 @@ def test_every_known_op_is_accepted(self) -> None: for op in KNOWN_OPS: arg = 1 if op.endswith("_chars") else "x" errors, warnings = validate_checks({"checks": [{"op": op, "arg": arg}]}) - self.assertEqual((errors, warnings), ([], []), op) + self.assertEqual(errors, [], op) + if op in SHAPE_OPS: + # A lone formatting op is still accepted, but is now flagged as + # gameable -- exactly the shape-only warning must fire. + self.assertTrue(warnings, (op, warnings)) + self.assertTrue(all("shape-only" in w for w in warnings), (op, warnings)) + else: + # An outcome op is real signal; it must not warn. + self.assertEqual(warnings, [], (op, warnings)) @pytest.mark.parametrize("bad_judge", [[], "", 0]) diff --git a/tests/test_outcome_judges.py b/tests/test_outcome_judges.py new file mode 100644 index 00000000..ab916074 --- /dev/null +++ b/tests/test_outcome_judges.py @@ -0,0 +1,387 @@ +from __future__ import annotations + +import pytest + +from skillopt_sleep.judges import ( + KNOWN_OPS, + SHAPE_OPS, + char_bound, + is_shape_only, + score_rule_judge, + validate_checks, +) +from skillopt_sleep.llm_miner import _mk_task +from skillopt_sleep.types import SessionDigest + + +def _digest() -> SessionDigest: + return SessionDigest( + session_id="s1", + project=r"C:\proj", + user_prompts=["gather context for a task"], + assistant_finals=["done"], + n_user_turns=1, + n_assistant_turns=1, + ) + + +# --- outcome ops ------------------------------------------------------------- + + +def test_not_contains_passes_when_absent_and_fails_when_present() -> None: + judge = {"kind": "rule", "checks": [{"op": "not_contains", "arg": "TODO"}]} + assert score_rule_judge(judge, "a complete answer")[0] == 1.0 + assert score_rule_judge(judge, "still TODO")[0] == 0.0 + + +@pytest.mark.parametrize( + "response", + [ + "Cannot complete this task. There is no benchmark spec in scope.", + "I cannot help with that.", + "I'm unable to do this.", + "> I cannot help with that.", + "- I cannot help with that.", + "1. I cannot help with that.", + "**Sorry, I can't** do this.", + "> " * 100 + "I cannot help with that.", + "", + ], +) +def test_no_refusal_fails_on_bare_refusals(response: str) -> None: + judge = {"kind": "rule", "checks": [{"op": "no_refusal"}]} + assert score_rule_judge(judge, response)[0] == 0.0 + + +def test_no_refusal_passes_on_substantive_answer() -> None: + judge = {"kind": "rule", "checks": [{"op": "no_refusal"}]} + assert score_rule_judge(judge, "Here is the context you asked for: ...")[0] == 1.0 + + +def test_no_refusal_accepts_a_refusal_that_still_does_the_work() -> None: + # An abstention that explains what was searched and what is missing is a + # useful answer, not a dead end. + response = "Cannot complete this task. " + ( + "I searched the working directory, the session artifacts folder, and " + "the benchmarks docs path, and none of them contain a spec. To proceed " + "I would need the spec file or a path to it. Here is what I checked and " + "what each location contained, so the gap is reproducible. " * 3 + ) + assert len(response) >= 600 + judge = {"kind": "rule", "checks": [{"op": "no_refusal"}]} + assert score_rule_judge(judge, response)[0] == 1.0 + + +def test_new_ops_are_registered_and_validate() -> None: + assert {"not_contains", "no_refusal"} <= KNOWN_OPS + errors, _ = validate_checks( + {"checks": [{"op": "not_contains", "arg": "x"}, {"op": "no_refusal"}]} + ) + assert errors == [] + + +def test_not_contains_requires_an_arg() -> None: + errors, _ = validate_checks({"checks": [{"op": "not_contains", "arg": " "}]}) + assert errors and "not_contains" in errors[0] + + +# --- shape-only detection ---------------------------------------------------- + + +def test_shape_ops_are_the_formatting_ops() -> None: + assert SHAPE_OPS == {"section_present", "max_chars", "min_chars"} + + +def test_is_shape_only_flags_a_formatting_judge() -> None: + assert is_shape_only({"checks": [{"op": "section_present", "arg": "Results"}]}) + assert is_shape_only( + {"checks": [{"op": "section_present", "arg": "Results"}, {"op": "max_chars", "arg": 500}]} + ) + + +def test_is_shape_only_false_when_any_outcome_op_present() -> None: + assert not is_shape_only( + { + "checks": [ + {"op": "section_present", "arg": "Results"}, + {"op": "contains", "arg": "answer"}, + ] + } + ) + assert not is_shape_only({"checks": []}) + assert not is_shape_only(None) + + +def test_validate_warns_on_shape_only_judge() -> None: + _, warnings = validate_checks({"checks": [{"op": "section_present", "arg": "Results"}]}) + assert any("shape-only" in w for w in warnings) + + +def test_validate_does_not_warn_when_an_outcome_op_is_present() -> None: + _, warnings = validate_checks( + {"checks": [{"op": "section_present", "arg": "Results"}, {"op": "no_refusal"}]} + ) + assert not any("shape-only" in w for w in warnings) + + +# --- miner preference: the actual reward-hack regression --------------------- + + +def test_shape_only_checks_lose_to_the_rubric() -> None: + # Regression for the observed hack: a `section_present=Results` judge let + # the optimizer score 1.0 by adding a heading. With a rubric available the + # task must be graded on outcome instead. + task = _mk_task( + _digest(), + { + "intent": "gather context for a task", + "checks": [{"op": "section_present", "arg": "Results"}], + "rubric": "A good answer reports what was found and what is missing.", + "satisfied": False, + }, + 0, + ) + assert task is not None + assert task.reference_kind == "rubric" + assert "what is missing" in task.reference + + +def test_shape_only_checks_still_used_when_no_rubric_offered() -> None: + task = _mk_task( + _digest(), + { + "intent": "gather context for a task", + "checks": [{"op": "section_present", "arg": "Results"}], + "rubric": "", + "satisfied": True, + }, + 0, + ) + assert task is not None + assert task.reference_kind == "rule" + + +def test_outcome_checks_also_lose_to_the_rubric() -> None: + # Second-order regression: after shape checks were demoted, the miner + # produced `contains=DEFAULT_ORGANIZATION` and the optimizer won by + # injecting that literal. Any literal-string check is injectable through + # skill text, so the rubric wins whenever one exists. + task = _mk_task( + _digest(), + { + "intent": "gather context for a task", + "checks": [{"op": "contains", "arg": "DEFAULT_ORGANIZATION"}], + "rubric": "A good answer reports what was found.", + "satisfied": True, + }, + 0, + ) + assert task is not None + assert task.reference_kind == "rubric" + + +def test_outcome_checks_used_when_no_rubric_offered() -> None: + task = _mk_task( + _digest(), + { + "intent": "gather context for a task", + "checks": [{"op": "no_refusal"}], + "rubric": "", + "satisfied": True, + }, + 0, + ) + assert task is not None + assert task.reference_kind == "rule" + assert task.judge["checks"] == [{"op": "no_refusal", "arg": None}] + + +def test_miner_keeps_the_new_outcome_ops() -> None: + task = _mk_task( + _digest(), + { + "intent": "gather context for a task", + "checks": [{"op": "not_contains", "arg": "TODO"}], + "rubric": "", + "satisfied": True, + }, + 0, + ) + assert task is not None + assert task.judge["checks"] == [{"op": "not_contains", "arg": "TODO"}] + + +# --- review-follow-up regressions ------------------------------------------- + + +def test_no_refusal_does_not_flag_a_helpful_sorry() -> None: + # "Sorry, I can help" opens with an apology but is not an abstention; the + # refusal prefixes must not swallow it. + judge = {"kind": "rule", "checks": [{"op": "no_refusal"}]} + assert score_rule_judge(judge, "Sorry, I can help with that. Here it is.")[0] == 1.0 + + +@pytest.mark.parametrize("bad", [[], ["not", "a", "dict"], "string", 7, True]) +def test_is_shape_only_returns_false_for_non_dict(bad) -> None: + # A public helper imported by tests must not raise on a truthy non-dict. + assert is_shape_only(bad) is False + + +def test_miner_keeps_a_tool_called_check() -> None: + # tool_called is advertised by the miner prompt and supported by the local + # judge, so a tool-only task must not be dropped as uncheckable. + task = _mk_task( + _digest(), + { + "intent": "invoke the search tool for a task", + "checks": [{"op": "tool_called", "arg": "search"}], + "rubric": "", + "satisfied": True, + }, + 0, + ) + assert task is not None + assert task.judge["checks"] == [{"op": "tool_called", "arg": "search"}] + + +def test_miner_ignores_a_non_string_rubric() -> None: + # A JSON null rubric must not coerce to the literal "None" and win over the + # real checks. + task = _mk_task( + _digest(), + { + "intent": "gather context for a task", + "checks": [{"op": "not_contains", "arg": "TODO"}], + "rubric": None, + "satisfied": True, + }, + 0, + ) + assert task is not None + assert task.reference_kind == "rule" + assert task.judge["checks"] == [{"op": "not_contains", "arg": "TODO"}] + + +def test_miner_drops_malformed_char_checks() -> None: + # A max_chars with a non-integer arg would crash the scorer during replay; + # it is dropped, and an arg-less contains is dropped too. + task = _mk_task( + _digest(), + { + "intent": "gather context for a task", + "checks": [ + {"op": "max_chars", "arg": "lots"}, + {"op": "contains", "arg": ""}, + {"op": "min_chars", "arg": "50"}, + ], + "rubric": "", + "satisfied": True, + }, + 0, + ) + assert task is not None + # only the coercible min_chars survives, normalized to an int + assert task.judge["checks"] == [{"op": "min_chars", "arg": 50}] + + +def test_miner_strips_string_args() -> None: + # Stray whitespace would otherwise become part of the required substring. + task = _mk_task( + _digest(), + { + "intent": "gather context for a task", + "checks": [{"op": "contains", "arg": " DEFAULT "}], + "rubric": "", + "satisfied": True, + }, + 0, + ) + assert task is not None + assert task.judge["checks"] == [{"op": "contains", "arg": "DEFAULT"}] + + +def test_miner_preserves_regex_whitespace() -> None: + # Trimming a regex changes what it matches, so the pattern is kept verbatim + # even though other string args are stripped. + pattern = r"\bfoo\s+$" + task = _mk_task( + _digest(), + { + "intent": "gather context for a task", + "checks": [{"op": "regex", "arg": pattern}, {"op": "contains", "arg": " x "}], + "rubric": "", + "satisfied": True, + }, + 0, + ) + assert task is not None + assert task.judge["checks"] == [ + {"op": "regex", "arg": pattern}, + {"op": "contains", "arg": "x"}, + ] + errors, _ = validate_checks(task.judge) + assert errors == [] + + + +def test_mined_checks_always_pass_validate_checks() -> None: + # The miner must never emit a judge that validate_checks() later rejects: + # bools (int subclass) and negative bounds are errors there. + task = _mk_task( + _digest(), + { + "intent": "gather context for a task", + "checks": [ + {"op": "max_chars", "arg": True}, + {"op": "min_chars", "arg": -5}, + {"op": "contains", "arg": " ok "}, + ], + "rubric": "", + "satisfied": True, + }, + 0, + ) + assert task is not None + assert task.judge["checks"] == [{"op": "contains", "arg": "ok"}] + errors, _ = validate_checks(task.judge) + assert errors == [] + + +@pytest.mark.parametrize( + "bad", [1.9, float("inf"), float("nan"), float("-inf"), "5_0", "ten", None, True], +) +def test_miner_and_validator_agree_on_bad_char_bounds(bad) -> None: + # Root cause of several drifts: the miner used int(arg) while the validator + # applied stricter rules, so a mined bound could fail its own validation. + # Both now share char_bound(), so anything the miner keeps must validate. + task = _mk_task( + _digest(), + { + "intent": "gather context for a task", + "checks": [{"op": "max_chars", "arg": bad}, {"op": "no_refusal"}], + "rubric": "", + "satisfied": True, + }, + 0, + ) + assert task is not None + # The malformed bound is dropped, never silently truncated (1.9 -> 1). + assert task.judge["checks"] == [{"op": "no_refusal", "arg": None}] + errors, _ = validate_checks(task.judge) + assert errors == [] + + +@pytest.mark.parametrize( + ("arg", "expected"), [(50, 50), (50.0, 50), (" 50 ", 50), ("+50", 50)], +) +def test_char_bound_accepts_integral_forms(arg, expected) -> None: + assert char_bound(arg) == expected + + +@pytest.mark.parametrize("bad", [1.9, True, "5_0", "ten", None, []]) +def test_char_bound_rejects_non_integers(bad) -> None: + with pytest.raises((ValueError, TypeError)): + char_bound(bad) + + + diff --git a/tests/test_sleep_engine.py b/tests/test_sleep_engine.py index c6facdd1..9de9c8ea 100644 --- a/tests/test_sleep_engine.py +++ b/tests/test_sleep_engine.py @@ -942,8 +942,30 @@ def _call(self, prompt, *, max_tokens=1024): miner = make_llm_miner(StubBackend()) tasks = miner([digest]) self.assertEqual(len(tasks), 1) + # A shape-only judge plus a rubric now grades on outcome, not on the + # presence of a heading an optimizer can simply add. + self.assertEqual(tasks[0].reference_kind, "rubric") + self.assertEqual(tasks[0].reference, "has a risks section") + + def test_miner_keeps_rule_judge_when_no_rubric_offered(self): + from skillopt_sleep.backend import Backend + from skillopt_sleep.llm_miner import make_llm_miner + + class StubBackend(Backend): + name = "stub" + + def _call(self, prompt, *, max_tokens=1024): + return ('[{"intent":"write a research brief",' + '"checks":[{"op":"contains","arg":"risk"}],' + '"rubric":"","satisfied":false}]') + + digest = SessionDigest(session_id="s1", project="/p", + user_prompts=["write a brief on X"], + assistant_finals=["a brief"], n_user_turns=1) + tasks = make_llm_miner(StubBackend())([digest]) + self.assertEqual(len(tasks), 1) self.assertEqual(tasks[0].reference_kind, "rule") - self.assertEqual(tasks[0].judge["checks"][0]["op"], "section_present") + self.assertEqual(tasks[0].judge["checks"][0]["op"], "contains") def test_miner_drops_uncheckable(self): from skillopt_sleep.backend import Backend