Skip to content

Fixes the Claude Code Exec backend for issue #233 - #238

Open
Nuplum (45lab95) wants to merge 1 commit into
microsoft:mainfrom
45lab95:fix/claude-exec-reflection
Open

Fixes the Claude Code Exec backend for issue #233#238
Nuplum (45lab95) wants to merge 1 commit into
microsoft:mainfrom
45lab95:fix/claude-exec-reflection

Conversation

@45lab95

Copy link
Copy Markdown

Summary

Fixes the Claude Code Exec backend for issue #233: with --backend claude_code_exec, both optimizer and target roles now
default to Claude Code (symmetric default), and the agent's SDK session trace is parsed into structured claude_trace_steps.txt files that are injected into the reflection
prompt — addressing context-length truncation and trajectory loss.

Implementation

  1. Backend registration + symmetric defaultbackend_config.py, model/__init__.py, scripts/train.py, scripts/eval_only.py register claude_code_exec; both roles
    default to it (a role pinned to a non-default value like minimax_chat still overrides).
  2. SDK trace → structured stepscodex_harness.py adds parse/format/persist_claude_trace_steps: extracts text / tool_call / tool_result steps from the SDK message
    stream, drops init / thinking_tokens bookkeeping, caps tool_result at 200 chars, and persists one file per prediction.
  3. Gated injectionreflect.py injects #### Claude Trace Steps only when REFLACT_CLAUDE_TRACE_TO_OPTIMIZER=1, which trainer.py sets only for claude_code_exec
    targets with model.claude_trace_to_optimizer: true (default; mirrors the existing codex gate).
  4. reasoning_effort wiring — the previously-dead REASONING_EFFORT module global is now consumed via run_claude_code_chat(..., effort=...) and forwarded by the
    dispatcher.
  5. Tests + docs — new tests/test_claude_code_backend.py; configs/_base_/default.yaml, docs/reference/config.md, and _FLATTEN_MAP extended.

Results

  • Unit tests: tests/test_claude_code_backend.py + tests/test_role_backend_resolution.py58 passed (11 new cases all green).
  • Integration smoke (small searchqa run): exit 0, accept=1, best-on-val 0.7500 → 0.9375, 80 claude_trace_steps.txt written.
  • Artifacts intact: merged_patch.json / config.json valid; skill v0000v0001.

How to verify

PYTHONUTF8=1 uv run --extra dev pytest tests/test_claude_code_backend.py tests/test_role_backend_resolution.py -q
# → 58 passed

Run a small searchqa training with --backend claude_code_exec --optimizer_backend claude_code_exec, then check:

  1. Reflection does not raise ValueError: Unsupported optimizer backend.
  2. Every prediction directory contains claude_trace_steps.txt.
  3. The analyst prompt contains #### Claude Trace Steps.
  4. All output files are valid UTF-8 (use PYTHONUTF8=1 on Windows).

…ace support

Register claude_code_exec as a full optimizer/target backend (issue microsoft#233).
--backend claude_code_exec now defaults both roles to claude_code_exec so
reflection sees the agent's complete session, and the SDK message stream is
parsed into structured trace steps persisted as claude_trace_steps.txt and
injected into the analyst prompt.

- model/claude_code_backend.py (new): chat_optimizer/chat_optimizer_messages on
  run_claude_code_chat, reasoning_effort threaded through, retry loop that
  surfaces non-JSON structured replies as RuntimeError, token tracking.
- model/codex_harness.py: parse/format/persist claude trace steps (text,
  tool_call, tool_result; drops init/thinking_tokens; 200-char tool_result cap;
  total truncation) + effort override on run_claude_code_chat.
- trainer.py/reflect.py: inject Claude Trace Steps gated behind
  REFLACT_CLAUDE_TRACE_TO_OPTIMIZER, set by the trainer only for claude_code_exec
  targets with model.claude_trace_to_optimizer (mirrors codex gate; default true).
- config.py/default.yaml/docs: model.claude_trace_to_optimizer key + flatten
  mapping + config.md rows.
- backend_config.py + model/__init__.py: register backend, route chat dispatch,
  token summary, reasoning effort, deployments.
- scripts/train.py, eval_only.py: symmetric default + accurate comments.
- tests: tests/test_claude_code_backend.py (10 tests: parsing, dispatch, effort,
  retry, trainer/reflect gating); test_role_backend_resolution.py updated to the
  symmetric default.

Verified: 58 unit tests pass; integration smoke on searchqa improved best-on-val
0.7500 -> 0.9375 with 80 claude_trace_steps.txt written; all output files valid
UTF-8 (no GBK mojibake).
@45lab95 Nuplum (45lab95) changed the title feat(claude_code_exec): add claude code optimizer backend with SDK tr… Fixes the Claude Code Exec backend for issue #233 Aug 20, 2026
@45lab95

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

@Yif-Yang

Copy link
Copy Markdown
Contributor

Thanks for tackling #233. I rechecked the current head (b4ae1d1). The focused tests pass (58/58), but three runtime blockers remain:

  1. With the shipped default config, --backend claude_code_exec resolves to optimizer_backend=claude_code_exec while leaving optimizer_model=gpt-5.5; only the target model becomes claude-sonnet-4-6. Both scripts/train.py and scripts/eval_only.py normalize the optimizer model only for claude_chat, not claude_code_exec, and the trainer then passes gpt-5.5 to the Claude optimizer. Please apply default_model_for_backend("claude_code_exec") when the optimizer model was not explicitly overridden, and add train/eval config-resolution regressions asserting both role backends and models.

  2. The CLI structured-output path builds --schema in skillopt/model/codex_harness.py. Claude Code 2.1.220 exposes --json-schema; I reproduced error: unknown option '--schema' with exit 1. This breaks calls using tools or return_message=True whenever CLI mode is forced or the SDK is unavailable. Please switch the flag and add an argv/parser-level CLI contract test.

  3. The optimizer chat path does not actually disable Claude Code's built-in tools. In the SDK path, allowed_tools=[] only controls auto-approval; it does not restrict tool availability, and bypassPermissions removes the remaining permission check. Please set tools=[], which the SDK emits as --tools "". The CLI fallback likewise needs explicit --tools ""; --permission-mode dontAsk is not a tool restriction. Please add regressions for both paths that assert the optimizer cannot access built-in tools.

The branch also conflicts with current main; please rebase and resolve those conflicts after the runtime fixes.

@Yif-Yang Yifan Yang (Yif-Yang) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tackling #233 — the diagnosis is right and the shape of the fix (drive Claude Code as the optimizer so reflection sees the full trajectory) is what I'd want. A few things need fixing before merge.

Note up front: I checked the REFLACT_ spelling and it is correct, not a typoREFLACT_CODEX_TRACE_TO_OPTIMIZER already exists on main (reflect.py:192) from the project's former name. Matching it was the right call. Likewise claude_code_backend.py is not duplicating claude_backend.py; it reuses _build_prompt_from_messages and wraps a genuinely different transport. Both fine.

Blocker 1 — --schema is not a real Claude CLI flag

codex_harness.py:1120 emits cmd.extend(["--schema", ...]). On the installed CLI:

$ claude -p --schema '{"type":"object"}' 'hi'
error: unknown option '--schema'

The actual flag is --json-schema. Since claude-agent-sdk isn't installed in a default environment, use_sdk: auto falls through to exactly this CLI path — so structured-output calls fail at runtime for anyone without the SDK. No test covers it, which is why it's green.

Blocker 2 — the tool_result payload is dropped, defeating the point of the PR

parse_claude_trace_steps reads part.get("content") for tool_result blocks, but Anthropic content blocks carry their payload under text. Reproduced on your branch:

raw = {"messages":[{"role":"user","content":[{"type":"tool_result","tool_use_id":"tu_1",
       "content":[{"type":"text","text":"THE ACTUAL RESULT PAYLOAD"}]}]}]}
parse_claude_trace_steps(json.dumps(raw))
# -> [{'type': 'tool_result', 'summary': '', 'index': 1}]

summary is empty. The reflector gets the shape of the trajectory but none of the observations — which is the specific thing #233 asked to preserve.

Should fix — stale claude_trace_steps.txt across turns

_persist_claude_artifacts writes the steps file with mode "w" from the current call's raw, while _persist_artifacts concatenates claude_raw.txt across calls with a turn separator. In the spreadsheetbench multi-turn repair loop (codegen_agent.py:604), which reuses one work_dir, the steps file ends up describing only the last turn while raw holds all of them. And when parsing yields "" the write is skipped entirely, leaving the previous turn's file for the reflector to read as if it described this prediction. Suggest formatting from combined_raw and writing unconditionally.

Please reconsider — silent optimizer-backend flip

trainer.py / scripts/train.py / scripts/eval_only.py now force optimizer_backend = "claude_code_exec" whenever --backend claude_code_exec. Your own comment states the consequence: an explicit --optimizer_backend openai_chat is silently ignored because it happens to equal a base-config default. A flag the user typed should never be silently discarded. Either track provenance, or leave the optimizer default alone and let people opt in.

Also worth noting this changes cost characteristics for existing claude_code_exec users without them asking.

Housekeeping

  • The .gitignore entry for .mcp.json is good practice; I confirmed no secret was actually committed.
  • configs/_base_/default.yaml gains claude_trace_to_optimizer: true — please document it in docs/reference/config.md alongside codex_trace_to_optimizer.
  • Conflicts with main in skillopt/engine/trainer.py; needs a rebase.

Suite on your branch: 1112 passed, 10 skipped, 130 subtests.

The two blockers are both narrow and testable — a test that feeds a realistic Anthropic message stream through parse_claude_trace_steps would have caught the second one and would be good to have regardless.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants