Skip to content

Commit d6421f9

Browse files
authored
Merge pull request #173 from vedmalex/fix/rule-judge-silent-failures
fix(sleep): surface silent rule-judge and edit failures
2 parents 566c0ae + 5e3270f commit d6421f9

9 files changed

Lines changed: 431 additions & 18 deletions

File tree

‎skillopt_sleep/consolidate.py‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
from typing import List, Optional, Tuple
1414

1515
from skillopt_sleep.backend import Backend
16-
from skillopt_sleep.memory import apply_edits
16+
from skillopt_sleep.memory import apply_edits_detailed
1717
from skillopt_sleep.replay import aggregate_scores, replay_batch
1818
from skillopt_sleep.types import EditRecord, ReplayResult, TaskRecord
1919

@@ -37,6 +37,11 @@ class ConsolidationResult:
3737
holdout_baseline: float
3838
holdout_candidate: float
3939
# ── observability (so a 0.0->0.0 night is self-diagnosing, not a black box) ──
40+
# Edits that changed nothing (anchor not found, or a duplicate add). They are
41+
# neither applied nor gate-rejected, so without this list they would vanish
42+
# from the report and the night would look like the optimizer produced less
43+
# than it did.
44+
unmatched_edits: List[EditRecord] = field(default_factory=list)
4045
holdout_detail: List[dict] = field(default_factory=list) # per val task: hard/soft/resp/why
4146
reflect_raw: str = "" # the optimizer's last raw reply (empty => reflect produced nothing)
4247
call_error: str = "" # backend's last call error (timeout/auth/empty)
@@ -143,6 +148,7 @@ def consolidate(
143148
cand_skill, cand_memory = skill, memory
144149
all_applied: List[EditRecord] = []
145150
all_rejected: List[EditRecord] = []
151+
all_unmatched: List[EditRecord] = []
146152

147153
def _edits_payload(edits: List[EditRecord]) -> List[dict]:
148154
return [{"op": e.op, "content": e.content, "anchor": e.anchor,
@@ -155,7 +161,12 @@ def _gate_apply(doc: str, edits: List[EditRecord], which: str) -> str:
155161
n_edits=len(edits), edits=_edits_payload(edits))
156162
if not edits:
157163
return doc
158-
new_doc, applied = apply_edits(doc, edits)
164+
new_doc, applied, unmatched = apply_edits_detailed(doc, edits)
165+
if unmatched:
166+
all_unmatched.extend(unmatched)
167+
if ev is not None:
168+
ev.log("reflect", "edits_unmatched", target=which,
169+
n_edits=len(unmatched), edits=_edits_payload(unmatched))
159170
if not applied:
160171
return doc
161172
# gate OFF: accept greedily with NO val scoring (the daily-use path)
@@ -277,6 +288,13 @@ def _gate_apply(doc: str, edits: List[EditRecord], which: str) -> str:
277288
else:
278289
action = "accept" if final_score > base_gate_score else "reject"
279290
accepted = bool(all_applied) and final_score > base_gate_score
291+
# The gate scores documents, not edit bookkeeping: when every proposed
292+
# edit was dropped during the per-target trials, `all_applied` is empty
293+
# and nothing changed, yet the score comparison can still yield an
294+
# accept-flavoured action. Reporting that as "accept_new_best" while
295+
# `accepted` is False makes the headline contradict the outcome.
296+
if not accepted and action in {"accept", "accept_new_best"}:
297+
action = "reject"
280298

281299
if ev is not None:
282300
w = max(0.0, min(1.0, float(gate_mixed_weight)))
@@ -297,7 +315,8 @@ def _gate_apply(doc: str, edits: List[EditRecord], which: str) -> str:
297315
candidate_hard=final_hard, candidate_soft=final_soft,
298316
metric=gate_metric, mixed_weight=gate_mixed_weight,
299317
formula=formula, n_applied=len(all_applied),
300-
n_rejected=len(all_rejected), night=night)
318+
n_rejected=len(all_rejected),
319+
n_unmatched=len(all_unmatched), night=night)
301320

302321
return ConsolidationResult(
303322
accepted=accepted,
@@ -308,6 +327,7 @@ def _gate_apply(doc: str, edits: List[EditRecord], which: str) -> str:
308327
new_memory=cand_memory if accepted else memory,
309328
applied_edits=all_applied,
310329
rejected_edits=all_rejected,
330+
unmatched_edits=all_unmatched,
311331
holdout_baseline=base_hard,
312332
holdout_candidate=final_hard,
313333
holdout_detail=holdout_detail,

‎skillopt_sleep/cycle.py‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,15 @@ def _render_report_md(report: SleepReport, cfg: SleepConfig) -> str:
124124
for e in report.rejected_edits:
125125
lines.append(f"- [{e.target}/{e.op}] {e.content}")
126126
lines.append("")
127+
if report.unmatched_edits:
128+
lines.append("## Proposed but changed nothing (never reached the gate)")
129+
lines.append(
130+
"_Anchor not found, duplicate/empty add, or an unknown op. "
131+
"These were never scored — check the anchor text if a rule you expected is missing._")
132+
for e in report.unmatched_edits:
133+
anchor = f" \n _anchor: `{e.anchor}`_" if e.anchor else ""
134+
lines.append(f"- [{e.target}/{e.op}] {e.content}{anchor}")
135+
lines.append("")
127136
if report.notes:
128137
lines.append("## Notes")
129138
for n in report.notes:
@@ -367,7 +376,8 @@ def run_sleep_cycle(
367376
_progress(
368377
cfg,
369378
f"consolidate done: gate={result.gate_action} accepted={result.accepted} "
370-
f"edits={len(result.applied_edits)} rejected={len(result.rejected_edits)}",
379+
f"edits={len(result.applied_edits)} rejected={len(result.rejected_edits)}"
380+
+ (f" unmatched={len(result.unmatched_edits)}" if result.unmatched_edits else ""),
371381
)
372382

373383
report.n_replayed = len(tasks)
@@ -378,6 +388,7 @@ def run_sleep_cycle(
378388
report.no_edits_reason = getattr(result, "no_edits_reason", "")
379389
report.edits = result.applied_edits
380390
report.rejected_edits = result.rejected_edits
391+
report.unmatched_edits = result.unmatched_edits
381392
report.tokens_used = backend.tokens_used()
382393
report.ended_at = _now_iso(clock)
383394

@@ -424,6 +435,7 @@ def run_sleep_cycle(
424435
"accepted": result.accepted,
425436
"n_applied_edits": len(result.applied_edits),
426437
"n_rejected_edits": len(result.rejected_edits),
438+
"n_unmatched_edits": len(result.unmatched_edits),
427439
"call_error": redact_secrets(getattr(result, "call_error", "")),
428440
"reflect_raw_head": redact_secrets(
429441
(getattr(result, "reflect_raw", "") or "")[:1200]
@@ -452,6 +464,7 @@ def run_sleep_cycle(
452464
candidate_score=report.candidate_score,
453465
n_applied_edits=len(report.edits),
454466
n_rejected_edits=len(report.rejected_edits),
467+
n_unmatched_edits=len(report.unmatched_edits),
455468
tokens_used=report.tokens_used, adopted=adopted)
456469

457470
return CycleOutcome(report, staging_dir, adopted, adopted_paths)

‎skillopt_sleep/judges.py‎

Lines changed: 67 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -35,29 +35,80 @@ def _section_present(response: str, name: str) -> bool:
3535
return bool(label.search(response or ""))
3636

3737

38-
def _check(op: str, arg: Any, response: str, tools_called: List[str]) -> bool:
38+
def _check(op: str, arg: Any, response: str,
39+
tools_called: List[str]) -> Tuple[bool, str]:
40+
"""Evaluate one check.
41+
42+
Returns ``(passed, problem)``. ``problem`` is non-empty only when the check
43+
itself is malformed (e.g. an unparseable regex) rather than simply unmet —
44+
the two need opposite fixes, so they must not look alike in the rationale.
45+
"""
3946
r = response or ""
4047
if op == "section_present":
41-
return _section_present(r, str(arg))
48+
return _section_present(r, str(arg)), ""
4249
if op == "regex":
4350
try:
44-
return bool(re.search(str(arg), r))
45-
except re.error:
46-
return False
51+
return bool(re.search(str(arg), r)), ""
52+
except re.error as exc:
53+
# A malformed pattern can never match, so it would fail every
54+
# rollout forever and read exactly like a model that never
55+
# complies. Surface it instead of hiding it behind a False.
56+
return False, f"invalid regex ({exc})"
4757
if op == "max_chars":
48-
return len(r) <= int(arg)
58+
return len(r) <= int(arg), ""
4959
if op == "min_chars":
50-
return len(r) >= int(arg)
60+
return len(r) >= int(arg), ""
5161
if op == "contains":
52-
return str(arg).lower() in r.lower()
62+
return str(arg).lower() in r.lower(), ""
5363
if op == "tool_called":
5464
name = str(arg).lower()
5565
if any(name == t.lower() for t in tools_called):
56-
return True
66+
return True, ""
5767
# single-shot approximation: the agent emits an explicit marker
58-
return bool(re.search(r"(?i)\btool_call\s*:\s*%s\b" % re.escape(name), r))
68+
return bool(re.search(r"(?i)\btool_call\s*:\s*%s\b" % re.escape(name), r)), ""
5969
# unknown op: do not block
60-
return True
70+
return True, ""
71+
72+
73+
KNOWN_OPS = frozenset({
74+
"section_present", "regex", "max_chars", "min_chars", "contains", "tool_called",
75+
})
76+
77+
78+
def validate_checks(judge: Dict[str, Any]) -> Tuple[List[str], List[str]]:
79+
"""Return ``(errors, warnings)`` for a rule judge's checks.
80+
81+
An *error* means the check can never behave as written — a regex that does
82+
not compile always scores 0.0, which is indistinguishable from a model that
83+
never complies. A *warning* means the check is accepted but toothless, e.g.
84+
an unknown op, which :func:`_check` deliberately lets pass.
85+
"""
86+
errors: List[str] = []
87+
warnings: List[str] = []
88+
if judge and not isinstance(judge, dict):
89+
return [f"judge must be an object, got {type(judge).__name__}"], warnings
90+
checks = (judge or {}).get("checks", []) or []
91+
if not isinstance(checks, list):
92+
return [f"judge 'checks' must be an array, got {type(checks).__name__}"], warnings
93+
for i, c in enumerate(checks):
94+
if not isinstance(c, dict):
95+
errors.append(f"check #{i} is not an object")
96+
continue
97+
op = c.get("op", "")
98+
arg = c.get("arg")
99+
if op == "regex":
100+
try:
101+
re.compile(str(arg))
102+
except re.error as exc:
103+
errors.append(f"check #{i} regex does not compile ({exc}): {arg!r}")
104+
elif op in {"max_chars", "min_chars"}:
105+
try:
106+
int(arg)
107+
except (TypeError, ValueError):
108+
errors.append(f"check #{i} {op} needs an integer arg, got {arg!r}")
109+
elif op not in KNOWN_OPS:
110+
warnings.append(f"check #{i} has unknown op {op!r} — it always passes")
111+
return errors, warnings
61112

62113

63114
def score_rule_judge(
@@ -73,11 +124,14 @@ def score_rule_judge(
73124
passed = 0
74125
failed_desc: List[str] = []
75126
for c in checks:
76-
ok = _check(c.get("op", ""), c.get("arg"), response, tools_called)
127+
ok, problem = _check(c.get("op", ""), c.get("arg"), response, tools_called)
77128
if ok:
78129
passed += 1
79130
else:
80-
failed_desc.append(f"{c.get('op')}={c.get('arg')}")
131+
desc = f"{c.get('op')}={c.get('arg')}"
132+
if problem:
133+
desc += f" [{problem}]"
134+
failed_desc.append(desc)
81135
soft = passed / len(checks)
82136
hard = 1.0 if passed == len(checks) else 0.0
83137
rationale = "all checks passed" if hard else "failed: " + ", ".join(failed_desc)

‎skillopt_sleep/memory.py‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,15 +76,41 @@ def apply_edits(doc: str, edits: List[EditRecord]) -> Tuple[str, List[EditRecord
7676
Returns (new_doc, applied_edits). Dedups: an `add` whose content already
7777
exists (normalized) is skipped. `delete`/`replace` match on normalized
7878
anchor substring.
79+
80+
See :func:`apply_edits_detailed` when the caller also needs the edits that
81+
matched nothing.
82+
"""
83+
new_doc, applied, _unmatched = apply_edits_detailed(doc, edits)
84+
return new_doc, applied
85+
86+
87+
def apply_edits_detailed(
88+
doc: str, edits: List[EditRecord]
89+
) -> Tuple[str, List[EditRecord], List[EditRecord]]:
90+
"""Apply edits and also report the ones that matched nothing.
91+
92+
Returns ``(new_doc, applied, unmatched)``. An edit lands in ``unmatched``
93+
whenever it left the document unchanged:
94+
95+
* ``delete``/``replace`` whose anchor matches no existing line;
96+
* ``add`` whose content duplicates an existing line (normalized), or is
97+
empty/whitespace;
98+
* any unrecognized op.
99+
100+
Without this list such edits are invisible — they appear in neither the
101+
applied nor the gate-rejected set, so a night can report zero edits while
102+
the optimizer actually produced several.
79103
"""
80104
lines = current_learned_lines(doc)
81105
norm_set = {_norm(line) for line in lines}
82106
applied: List[EditRecord] = []
107+
unmatched: List[EditRecord] = []
83108

84109
for e in edits:
85110
op = (e.op or "add").lower()
86111
if op == "add":
87112
if _norm(e.content) in norm_set or not e.content.strip():
113+
unmatched.append(e)
88114
continue
89115
lines.append(e.content.strip())
90116
norm_set.add(_norm(e.content))
@@ -96,6 +122,8 @@ def apply_edits(doc: str, edits: List[EditRecord]) -> Tuple[str, List[EditRecord
96122
lines = keep
97123
norm_set = {_norm(line) for line in lines}
98124
applied.append(e)
125+
else:
126+
unmatched.append(e)
99127
elif op == "replace":
100128
anchor = _norm(e.anchor)
101129
new_lines = []
@@ -110,8 +138,12 @@ def apply_edits(doc: str, edits: List[EditRecord]) -> Tuple[str, List[EditRecord
110138
lines = new_lines
111139
norm_set = {_norm(line) for line in lines}
112140
applied.append(e)
141+
else:
142+
unmatched.append(e)
143+
else:
144+
unmatched.append(e)
113145

114-
return set_learned(doc, lines), applied
146+
return set_learned(doc, lines), applied, unmatched
115147

116148

117149
def ensure_skill_scaffold(doc: str, *, name: str, description: str) -> str:

‎skillopt_sleep/tasks_file.py‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33

44
import json
55
import os
6+
import sys
67
from typing import Any, Dict, List, Tuple
78

9+
from skillopt_sleep.judges import validate_checks
810
from skillopt_sleep.mine import assign_splits, normalize_legacy_split
911
from skillopt_sleep.types import TaskRecord
1012

@@ -78,4 +80,21 @@ def load_tasks_file(
7880
if not isinstance(item, dict):
7981
raise ValueError("each task entry must be a JSON object")
8082
tasks.append(TaskRecord.from_dict(item))
83+
84+
# Fail loudly on malformed rule judges. A check that cannot work as written
85+
# scores 0.0 on every rollout, which reads exactly like a model that never
86+
# complies — the run looks legitimate while one dimension is dead.
87+
errors: List[str] = []
88+
for task in tasks:
89+
if task.reference_kind != "rule" or not task.judge:
90+
continue
91+
task_errors, task_warnings = validate_checks(task.judge)
92+
errors.extend(f"task {task.id}: {e}" for e in task_errors)
93+
for w in task_warnings:
94+
print(f"[sleep] warning: task {task.id}: {w}", file=sys.stderr)
95+
if errors:
96+
raise ValueError(
97+
"tasks file contains unusable rule checks:\n " + "\n ".join(errors)
98+
)
99+
81100
return _normalize_tasks(tasks, holdout_fraction=holdout_fraction, seed=seed), meta

‎skillopt_sleep/types.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,11 @@ class SleepReport:
138138
no_edits_reason: str = ""
139139
edits: List[EditRecord] = field(default_factory=list)
140140
rejected_edits: List[EditRecord] = field(default_factory=list)
141+
# Proposed edits that changed nothing (anchor absent, duplicate/empty add,
142+
# unknown op). They were never scored by the gate, so they belong in neither
143+
# list above — without them a night can report no edits while the optimizer
144+
# actually produced several.
145+
unmatched_edits: List[EditRecord] = field(default_factory=list)
141146
tokens_used: int = 0
142147
notes: List[str] = field(default_factory=list)
143148

0 commit comments

Comments
 (0)