fix(sleep): grade mined tasks on outcome instead of literal strings

[lufen]

The gate could be won by reformatting. A mined task graded by section_present=Results let the optimizer score held-out 0.25 -> 1.00 simply by adding a '## Results' heading. The rubric judge that grades substance already existed, but _mk_task preferred any programmatic check over it, so it was almost never selected.

Demoting formatting checks alone is not sufficient. With them demoted the miner emitted contains=DEFAULT_ORGANIZATION and the optimizer scored 1.00 again, now by injecting that literal. The root cause is that the optimizer edits skill text which is prepended to the model's context, so ANY check for a literal string in the response is satisfiable by instructing the model to emit it. Only semantic grading resists that, so a rubric now wins whenever the miner supplies one. Rule judges remain for imported gbrain-style benchmarks, which carry checks but no rubric.

Also adds outcome ops (not_contains, no_refusal), an is_shape_only() detector that warns when a judge only constrains formatting, and re-steers the miner prompt away from "formatting, structure" toward what an answer achieves.

[Copilot]

Pull request overview

This PR hardens SkillOpt-Sleep’s mined-task grading against “reward hacking” by ensuring tasks are primarily graded on semantic outcome (rubrics) rather than gameable literal/format checks, and extends the rule-judge operator set with additional outcome-oriented ops.

Changes:

• Prefer rubric-based grading whenever the miner provides a rubric; keep rule-judges only as a fallback when no rubric is supplied.
• Add outcome ops (not_contains, no_refusal) plus “shape-only” (formatting-only) judge detection and warnings.
• Update miner prompt + tests to reflect the new preference and new ops.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/test_sleep_engine.py	Updates miner tests to assert rubric wins over shape-only checks; keeps rule judge when no rubric provided.
tests/test_outcome_judges.py	Adds new unit tests for not_contains, no_refusal, shape-only detection, and miner preference regression coverage.
tests/test_judges.py	Adjusts validation tests to account for new shape-only warning behavior.
skillopt_sleep/prompts.py	Re-steers miner instructions toward outcome checks and mandates providing a rubric.
skillopt_sleep/llm_miner.py	Changes task construction to prefer reference_kind="rubric" when rubric exists; extends accepted ops list.
skillopt_sleep/judges.py	Implements not_contains, no_refusal, refusal detection heuristic, shape-only detection, and validation warnings.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (2)

skillopt_sleep/prompts.py:53

• The miner prompt lists available formatting checks but omits min_chars, even though the code supports it (and it’s classified as a shape op). This makes the prompt/documentation inconsistent with the implemented judge ops.

     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":"<heading text>"}
        {"op":"max_chars","arg":<int>}
     Only include checks you are confident a GOOD answer must satisfy.

skillopt_sleep/llm_miner.py:68

• This branch treats any truthy rubric as authoritative, but _mk_task currently coerces obj["rubric"] with str(...). If the miner emits a non-string rubric (e.g. null), it becomes the literal text "None" and will now win over checks, creating a bogus rubric-scored task. Also, when falling back to a rule judge, the check list is not validated here; malformed *_chars args or missing not_contains args can crash or permanently fail scoring during replay.

    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

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (2)

skillopt_sleep/judges.py:76

• _is_refusal strips only *_# from the start of the response head. A bare refusal formatted as a blockquote (> I cannot ...) or list item (- I cannot ..., 1. I cannot ...) will currently not be detected, causing the new no_refusal op to incorrectly pass. Consider stripping common markdown quote/list prefixes before checking _REFUSAL_PREFIXES.

    text = (response or "").strip()
    if not text:
        return True
    head = text[:160].lower().lstrip("*_# ")
    if not any(head.startswith(p) for p in _REFUSAL_PREFIXES):
        return False

tests/test_judges.py:185

• test_every_known_op_is_accepted currently uses all("shape-only" in w for w in warnings), which passes even when warnings is empty (vacuous truth). That means the test won’t fail if an unexpected warning is introduced for non-shape ops like contains/regex/no_refusal. Make the assertion conditional so non-shape ops require warnings == [], while shape ops require exactly the shape-only warning.

    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, [], op)
            # A lone formatting op is still accepted, but is now flagged as
            # gameable; no other warning should fire.
            self.assertTrue(all("shape-only" in w for w in warnings), (op, warnings))

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (1)

skillopt_sleep/judges.py:77

• _is_refusal() slices to text[:160] before stripping markdown markers. A refusal can bypass detection by prefixing >160 marker characters (e.g. many >), making head empty and causing no_refusal to incorrectly pass. Strip leading markers first, then apply the length limit.

    head = re.sub(r"^(?:[>\-*_#\s]|\d+[.)])+", "", text[:160].lower())

[lufen]

Follow-up in 453abbe addresses the re-review's suppressed suggestions: _is_refusal now strips leading markdown markers (blockquote/list/numbered/emphasis) so a formatted refusal is still caught by no_refusal, and test_every_known_op_is_accepted no longer passes vacuously (shape ops must emit exactly the shape-only warning, outcome ops none). Earlier 1f34187 also guards a non-string rubric and validates mined checks. All suites green.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (1)

skillopt_sleep/llm_miner.py:74

• In _mk_task(), string-arg checks are validated with arg.strip() but the unstripped value is stored. That can make contains/not_contains/tool_called/section_present brittle (leading/trailing whitespace becomes part of the required substring/tool name). Also, max_chars/min_chars currently accept booleans and negative bounds via int(arg), which can later cause validate_checks() to reject the mined tasks file as unusable (e.g., negative bounds are errors). Strip/normalize string args and align int parsing with validate_checks (reject bool and negative).

    _needs_str_arg = {"section_present", "regex", "contains", "not_contains", "tool_called"}
    clean_checks = []
    for c in checks:
        if not isinstance(c, dict):
            continue
        op = c.get("op")
        arg = c.get("arg")
        if op in _needs_str_arg:
            if isinstance(arg, str) and arg.strip():
                clean_checks.append({"op": op, "arg": arg})
        elif op in {"max_chars", "min_chars"}:
            try:
                clean_checks.append({"op": op, "arg": int(arg)})
            except (TypeError, ValueError):
                continue
        elif op == "no_refusal":
            clean_checks.append({"op": op, "arg": None})

[lufen]

Second re-review follow-up in 85a45d1: the refusal-marker strip now runs on the full text before the 160-char bound, so a refusal padded with >160 leading markers (e.g. many >) can no longer empty the head and slip past no_refusal. Regression added; suites green.

[lufen]

Third re-review follow-up in 53d9d91, on _mk_task():

• String args were validated with .strip() but stored unstripped, so stray whitespace became part of the required substring / tool name. The stripped value is now stored.
• max_chars/min_chars accepted bools (bool is an int subclass, so int(True) == 1) and negative bounds, both of which validate_checks() rejects as errors — the miner could emit a tasks file that fails its own validation. Both are now rejected at construction.
• Added a regression asserting mined judges always pass validate_checks() with no errors.

Suites green.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (1)

skillopt_sleep/llm_miner.py:82

• _mk_task() normalizes max_chars/min_chars via int(arg), but this can (a) silently truncate non-integral floats (e.g. 1.9 -> 1), and (b) raise OverflowError for values like float('inf') (possible if JSON contains a huge exponent like 1e309). Both diverge from validate_checks() and can lead to unexpected behavior or miner crashes. Reject non-integral floats up front and catch OverflowError when converting.

            try:
                bound = int(arg)
            except (TypeError, ValueError):
                continue

[lufen]

Fourth re-review follow-up in f060117. The suggestion was correct — _mk_task() used int(arg), which silently truncated a non-integral float (1.9 -> 1) and raised an uncaught OverflowError for float('inf') (reachable from JSON like 1e309), both diverging from validate_checks().

Since this is the third time the miner and the validator drifted apart on argument shapes, I fixed the root cause rather than the symptom: extracted judges.char_bound() and used it in both places, so the accepted shapes cannot diverge again. Bools, non-integral floats, inf/nan and underscore-separated strings ("5_0", which int() accepts but the validator's regex rejects) are now refused consistently.

Added a property-style regression asserting anything the miner keeps passes validate_checks() with no errors, plus direct char_bound() coverage. Suites green (515 passed).

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (1)

skillopt_sleep/llm_miner.py:72

• _mk_task() strips whitespace from all string-arg ops, including regex. Trimming a regex pattern can change its meaning (leading/trailing spaces can be significant), so this normalization should be limited to substring/tool/heading ops (e.g., contains/not_contains/tool_called/section_present) while preserving the regex text as-provided.

        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()})

[lufen]

Fifth re-review follow-up in 7124efe: correct catch — the whitespace strip applied to every string-arg op, including regex, but leading/trailing spaces are significant in a pattern, so trimming changes what it matches. Stripping is now limited to the substring/tool/heading ops (contains, not_contains, tool_called, section_present) and the regex text is stored verbatim. Regression added; suites green (516 passed).

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.