Windows robustness for claude/codex backends (+ hardened JSON fallback) (#79)

* Robustness for the claude/codex backends on Windows: argv overflow, subprocess encoding, tolerant JSON, test-eval dirs

Fixes surfaced running SkillOpt end-to-end on the bundled `claude` backend
(local Claude CLI) on Windows. None changes the OpenAI/GPT happy path.

1. skillopt/engine/trainer.py — the final test-eval directory
   (test_eval_final/) is written to before being created; add
   os.makedirs(..., exist_ok=True), matching the two sibling test-eval dirs.
   Without it, summary.json raises FileNotFoundError when a rollout yields
   zero predictions.

2. skillopt/model/claude_backend.py
   a. Pass the prompt via stdin (not argv): on Windows the whole command line
      is capped at ~32 KB and a large optimizer prompt (the success-analyst
      minibatch carrying several report trajectories) overflows it with
      [WinError 206], killing the run after retries.
   b. Pass the system prompt via --append-system-prompt-file (a temp file),
      not argv. The system prompt here is the skill being optimized, which
      SkillOpt grows over training; since the ~32 KB cap applies to the SUM of
      all argv, a grown skill would re-hit [WinError 206] even with the prompt
      on stdin.
   c. Pin the subprocess encoding to utf-8 (errors="replace"). With text=True
      and no encoding=, stdin is encoded with the system codepage; on a zh-CN
      box (cp936/GBK) a prompt containing an emoji or some Latin-1 characters
      raises UnicodeEncodeError before the CLI even starts, failing every retry.

3. skillopt/model/codex_backend.py — the same utf-8 encoding pin on its
   subprocess.run(input=...) call (identical unpinned-encoding pattern).

4. skillopt/utils/json_utils.py — extract_json() returned None for valid-
   looking JSON that strict json.loads rejects (unescaped ASCII quotes inside
   CJK string values, trailing commas), silently dropping the analyst's edits
   on non-schema backends (Claude/Qwen): reflect produces N edits, 0 applied.
   Add a json_repair fallback, but only on a single unambiguous object — a
   balanced-brace extractor plus a refuse-on-multiple-objects guard — so a
   chain-of-thought "scratch + final" response can't make repair silently
   return the wrong (discarded) object, which would be worse than None (None is
   detectable and retryable; a wrong-but-valid edit is applied blind). Declare
   json_repair in requirements.txt and the claude/qwen optional extras so the
   fallback is actually present (it otherwise no-ops, dropping edits silently).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
(cherry picked from commit dca74a683e)

* fix(json_utils): harden tolerant JSON fallback from PR #77

Follow-up fixes on top of the cherry-picked Windows-robustness change:

1. Make _top_level_brace_objects() fully string-aware in its OUTER scan, not
   just inside an object. A '{' inside quoted prose (e.g. '"set it to {x}"')
   no longer starts a candidate object, so extract_json() returns None for
   prose pseudo-JSON instead of repairing it into a bogus dict — which would
   be strictly worse than dropping the edit, since extract_json feeds the
   optimizer's skill edits.

2. Pick the repair candidate BEFORE importing json_repair, so the missing-
   dependency RuntimeWarning only fires when there is genuinely a single
   malformed object that could have been repaired. Ordinary no-JSON / prose
   replies (the common case) now return None silently instead of warning on
   every call.

3. Resolve dependency-metadata inconsistency: json_repair is optional, so add
   it to the `all` extra (it was already in `claude`/`qwen`) and demote it
   from a hard requirement to an optional/commented entry in requirements.txt,
   matching the project's convention for backend-specific deps.

Adds regression tests for prose-with-braces (-> None), no-warning-on-plain-
text, single-object repair, and multi-object ambiguity. Existing 22 json
tests still pass with and without json_repair installed.

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: samuelgoofus-boop <260247789+samuelgoofus-boop@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

pyproject.toml

@@ -37,9 +37,9 @@ dependencies = [
 # Benchmark-specific dependencies
 alfworld = ["alfworld>=0.4.0", "gymnasium>=0.29.0"]
 # Claude model backend
-claude = ["claude-agent-sdk>=0.1.0"]
+claude = ["claude-agent-sdk>=0.1.0", "json_repair>=0.61.0"]
 # Qwen local model backend (via vLLM)
-qwen = ["vllm>=0.4.0"]
+qwen = ["vllm>=0.4.0", "json_repair>=0.61.0"]
 # SearchQA data materialization
 searchqa = ["datasets>=2.18.0"]
 # Documentation site
@@ -53,6 +53,7 @@ all = [
     "alfworld>=0.4.0",
     "gymnasium>=0.29.0",
     "claude-agent-sdk>=0.1.0",
+    "json_repair>=0.61.0",
 ]
 
 [project.scripts]

requirements.txt

@@ -17,6 +17,12 @@ httpx>=0.27.0
 # ── Optional: Qwen local model (via vLLM) ────────
 # vllm>=0.4.0
 
+# ── Optional: tolerant JSON repair for free-form output from non-OpenAI
+#    backends (Claude/Qwen). Without it extract_json() falls back safely and
+#    drops a malformed analyst edit instead of repairing it. Installed by the
+#    `claude`, `qwen`, and `all` extras in pyproject.toml.
+# json_repair>=0.61.0
+
 # ── Optional: WebUI dashboard ────────────────────
 # gradio>=4.0.0
 

skillopt/engine/trainer.py

@@ -2133,6 +2133,7 @@ class ReflACTTrainer:
             )
             print(f"  Test items: {test_n}")
             baseline_test_dir = os.path.join(out_root, "test_eval_baseline")
+            os.makedirs(baseline_test_dir, exist_ok=True)
             baseline_test_results = adapter.rollout(test_env, skill_init, baseline_test_dir)
             baseline_test_hard, baseline_test_soft = compute_score(baseline_test_results)
             baseline_buckets = _compute_task_type_buckets(baseline_test_results, task_types)
@@ -2167,6 +2168,7 @@ class ReflACTTrainer:
             )
             print(f"  Test items: {test_n2}")
             test_dir = os.path.join(out_root, "test_eval")
+            os.makedirs(test_dir, exist_ok=True)
             test_results = adapter.rollout(test_env2, best_skill, test_dir)
             test_hard, test_soft = compute_score(test_results)
             best_buckets = _compute_task_type_buckets(test_results, task_types)
@@ -2230,6 +2232,7 @@ class ReflACTTrainer:
                     )
                     print(f"  Test items: {test_n3}")
                     final_test_dir = os.path.join(out_root, "test_eval_final")
+                    os.makedirs(final_test_dir, exist_ok=True)
                     final_test_results = adapter.rollout(test_env3, current_skill, final_test_dir)
                     final_test_hard, final_test_soft = compute_score(final_test_results)
                     final_buckets = _compute_task_type_buckets(final_test_results, task_types)

skillopt/model/claude_backend.py

@@ -252,13 +252,25 @@ def _run_claude_print(*, system: str, prompt: str, model: str, tools: list[dict[
         if CLAUDE_SETTING_SOURCES:
             cmd.extend(["--setting-sources", CLAUDE_SETTING_SOURCES])
         if system:
-            cmd.extend(["--append-system-prompt", system])
+            # Write the system prompt to a file, not argv: here the skill being
+            # optimized IS the system prompt, and SkillOpt grows it over training,
+            # so past ~30 KB it would re-hit the Windows argv cap (WinError 206).
+            # The CLI reads it via --append-system-prompt-file.
+            system_path = os.path.join(temp_dir, "system_prompt.txt")
+            with open(system_path, "w", encoding="utf-8") as system_fh:
+                system_fh.write(system)
+            cmd.extend(["--append-system-prompt-file", system_path])
         if effort:
             cmd.extend(["--effort", effort])
         structured_output = bool(return_message)
         if structured_output:
             cmd.extend(["--schema", _assistant_message_schema_wrapper()])
-        proc = subprocess.run(cmd + [prompt_for_cli], capture_output=True, text=True, timeout=timeout or 300, cwd=temp_dir)
+        # Feed the prompt via stdin (and the system prompt via a file, above), not
+        # argv: on Windows the whole command line is capped at ~32 KB and large
+        # optimizer prompts / grown skills overflow it → [WinError 206]. Pin UTF-8
+        # so a zh-CN default codepage (cp936) can't raise UnicodeEncodeError on
+        # emoji / non-GBK glyphs before the CLI even starts.
+        proc = subprocess.run(cmd, input=prompt_for_cli, capture_output=True, text=True, encoding="utf-8", errors="replace", timeout=timeout or 300, cwd=temp_dir)
         stderr_text = (proc.stderr or "").strip()
         if proc.returncode != 0:
             _check_claude_error(stderr_text, model)

skillopt/model/codex_backend.py

@@ -328,6 +328,8 @@ def _run_codex_exec(
             command,
             input=prompt,
             text=True,
+            encoding="utf-8",
+            errors="replace",
             capture_output=True,
             timeout=timeout,
             check=False,

skillopt/utils/json_utils.py

@@ -3,6 +3,72 @@ from __future__ import annotations
 
 import json
 import re
+import warnings
+
+
+def _top_level_brace_objects(text: str) -> list[str]:
+    """Return every balanced *top-level* ``{...}`` span in ``text``.
+
+    Fully string/escape aware: braces inside quoted strings are ignored both
+    when scanning for an object start AND while tracking depth inside one, so a
+    ``{`` that appears in prose (e.g. ``'set it to {x}'``) is never mistaken for
+    the start of a JSON object. Used to detect ambiguity: when a response carries
+    more than one top-level object we must not let a repair pass silently pick
+    one — it may pick the wrong (discarded) edit, strictly worse than None.
+    """
+    spans: list[str] = []
+    i, n = 0, len(text)
+    outer_in_str = False
+    outer_esc = False
+    while i < n:
+        ch = text[i]
+        # Skip over braces that live *inside* a quoted string before any object
+        # has started — otherwise a `{` in prose like '"set it to {x}"' is wrongly
+        # treated as an object start, and the repair pass below turns non-JSON
+        # prose into a bogus dict (strictly worse than returning None).
+        if outer_in_str:
+            if outer_esc:
+                outer_esc = False
+            elif ch == "\\":
+                outer_esc = True
+            elif ch == '"':
+                outer_in_str = False
+            i += 1
+            continue
+        if ch == '"':
+            outer_in_str = True
+            i += 1
+            continue
+        if ch != "{":
+            i += 1
+            continue
+        depth = 0
+        in_str = False
+        esc = False
+        start = i
+        while i < n:
+            ch = text[i]
+            if in_str:
+                if esc:
+                    esc = False
+                elif ch == "\\":
+                    esc = True
+                elif ch == '"':
+                    in_str = False
+            elif ch == '"':
+                in_str = True
+            elif ch == "{":
+                depth += 1
+            elif ch == "}":
+                depth -= 1
+                if depth == 0:
+                    spans.append(text[start:i + 1])
+                    i += 1
+                    break
+            i += 1
+        else:
+            break  # unterminated final object
+    return spans
 
 
 def extract_json(text: str) -> dict | None:
@@ -22,6 +88,45 @@ def extract_json(text: str) -> dict | None:
             return json.loads(m.group(0))
         except json.JSONDecodeError:
             pass
+    # Tolerant fallback for non-OpenAI backends (Claude/Qwen, …) whose free-form
+    # JSON strict json.loads rejects — unescaped ASCII quotes inside CJK string
+    # values, trailing commas, etc. Repair so the analyst's edits aren't silently
+    # dropped, but ONLY a single unambiguous object: never feed the greedy `{.*}`
+    # span or the raw text, or json_repair would quietly return one of several
+    # objects (empirically the wrong/last one) — strictly worse than None, which
+    # the caller can detect and retry/skip.
+    #
+    # Pick the candidate FIRST, before importing json_repair, so the optional
+    # dependency only matters (and only warns) when there is genuinely a single
+    # malformed object we could have repaired. Ordinary no-JSON / prose replies
+    # have no candidate and return None silently.
+    candidate = None
+    fenced = re.search(r"```json\s*(.*?)```", text, re.DOTALL)
+    if fenced and len(_top_level_brace_objects(fenced.group(1))) == 1:
+        candidate = fenced.group(1)
+    else:
+        objs = _top_level_brace_objects(text)
+        if len(objs) == 1:
+            candidate = objs[0]
+        # 0 or >1 top-level objects → too ambiguous to repair safely → None
+    if not candidate:
+        return None
+    try:
+        from json_repair import repair_json
+    except ModuleNotFoundError:
+        warnings.warn(
+            "json_repair not installed; malformed-JSON recovery disabled — "
+            "a non-OpenAI analyst edit may be silently dropped. pip install json_repair",
+            RuntimeWarning,
+            stacklevel=2,
+        )
+        return None
+    try:
+        repaired = repair_json(candidate, return_objects=True)
+        if isinstance(repaired, dict) and repaired:
+            return repaired
+    except Exception:  # noqa: BLE001 — repair is best-effort
+        pass
     return None
 
 

tests/test_json_utils.py

@@ -3,7 +3,11 @@ from __future__ import annotations
 
 import pytest
 
-from skillopt.utils.json_utils import extract_json, extract_json_array
+from skillopt.utils.json_utils import (
+    _top_level_brace_objects,
+    extract_json,
+    extract_json_array,
+)
 
 
 class TestExtractJson:
@@ -61,6 +65,52 @@ class TestExtractJson:
         assert extract_json(text) is None
 
 
+class TestTopLevelBraceObjects:
+    """_top_level_brace_objects — string/escape-aware top-level object scan."""
+
+    def test_single_clean_object(self) -> None:
+        assert _top_level_brace_objects('{"a": 1}') == ['{"a": 1}']
+
+    def test_two_top_level_objects(self) -> None:
+        assert _top_level_brace_objects('{"a":1}\n{"b":2}') == ['{"a":1}', '{"b":2}']
+
+    def test_brace_inside_quoted_prose_is_ignored(self) -> None:
+        """A '{' inside a quoted string must NOT start an object (the bug)."""
+        # Brace-shaped content inside a string, with no real object → no spans.
+        assert _top_level_brace_objects('label is "set it to {x: 1}" done') == []
+
+    def test_real_object_after_quoted_brace(self) -> None:
+        """Quoted-prose braces are skipped; a later real object is still found."""
+        text = 'note "{wrong: 1}" then actual {"edit": "right"}'
+        assert _top_level_brace_objects(text) == ['{"edit": "right"}']
+
+
+class TestExtractJsonTolerantFallback:
+    """extract_json — json_repair fallback for malformed non-OpenAI output."""
+
+    def test_prose_pseudo_json_returns_none(self) -> None:
+        """Regression: brace-shaped prose inside quotes must not be 'repaired'
+        into a bogus dict. It returned {'op': 'delete'} before the fix."""
+        text = 'The literal string "{op: delete}" appears in prose, not as JSON.'
+        assert extract_json(text) is None
+
+    def test_no_warning_on_plain_text(self, recwarn: pytest.WarningsRecorder) -> None:
+        """No json_repair warning for ordinary no-JSON replies (no candidate)."""
+        assert extract_json("Just plain text without JSON.") is None
+        assert extract_json("") is None
+        assert [w for w in recwarn.list if issubclass(w.category, RuntimeWarning)] == []
+
+    def test_trailing_comma_repaired_when_available(self) -> None:
+        """With json_repair installed, a single malformed object is repaired."""
+        pytest.importorskip("json_repair")
+        assert extract_json('{"edit": "add", "text": "x",}') == {"edit": "add", "text": "x"}
+
+    def test_two_malformed_objects_too_ambiguous(self) -> None:
+        """Multiple top-level objects are ambiguous → None, never guess."""
+        pytest.importorskip("json_repair")
+        assert extract_json('{"first": true,} noise {"second": true,}') is None
+
+
 class TestExtractJsonArray:
     """extract_json_array — extract a JSON array from LLM response text."""