Files
microsoft-SkillOpt/skillopt/utils/json_utils.py
Yifan Yang 14c045f04f 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>
2026-06-23 19:00:23 +08:00

148 lines
5.0 KiB
Python

"""JSON extraction helpers for LLM responses."""
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:
"""Extract a JSON object from LLM response text.
Tries ```json fences first, then bare {...} patterns.
"""
m = re.search(r"```json\s*(.*?)```", text, re.DOTALL)
if m:
try:
return json.loads(m.group(1))
except json.JSONDecodeError:
pass
m = re.search(r"\{.*\}", text, re.DOTALL)
if m:
try:
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
def extract_json_array(text: str) -> list | None:
"""Extract a JSON array from LLM response text."""
m = re.search(r"```json\s*(.*?)```", text, re.DOTALL)
if m:
try:
return json.loads(m.group(1))
except json.JSONDecodeError:
pass
m = re.search(r"\[.*\]", text, re.DOTALL)
if m:
try:
return json.loads(m.group(0))
except json.JSONDecodeError:
pass
return None