From d0ad8949b8b67bad21edcc332fdd5d911c9efc13 Mon Sep 17 00:00:00 2001 From: crisperity Date: Mon, 9 Nov 2026 21:33:03 +0100 Subject: [PATCH] code-walkthrough: stub generated files in prompts, recover from safeguard flags, bump to 1.4.1 A live-mode batch with a golden fixture (lines up to 43 KB of escaped XML) was rejected by Sonnet 5.5 safeguards. The failure was deterministic. The escalate retry re-sent the same prompt. - Prompt copy stubs binary, golden, snapshot, lock and huge-line/huge files, keeping hunk headers. Validation and merge still use the real diff. - Batches are also capped by characters (batch_max_chars, default 120000). - A flagged error is its own kind, skips escalate, and retries with the largest file stubbed, up to 5 rounds. Withheld files get a fixed note the validator accepts and are listed in meta and walkthrough_get. - Small route and routed retries get the same handling. - A batch that is still flagged with everything stubbed fails with the batch number and files named. --- .claude-plugin/marketplace.json | 2 +- skills/code-walkthrough/scripts/cw_llm.py | 6 + skills/code-walkthrough/scripts/cw_run.py | 216 ++++++++++++------ skills/code-walkthrough/scripts/cw_server.py | 2 +- skills/code-walkthrough/scripts/cw_store.py | 1 + skills/code-walkthrough/scripts/fanout.py | 65 +++++- .../code-walkthrough/scripts/test_cw_llm.py | 3 + .../code-walkthrough/scripts/test_cw_run.py | 167 +++++++++++++- .../code-walkthrough/scripts/test_fanout.py | 46 +++- .../scripts/test_validate_analysis.py | 8 + .../scripts/validate_analysis.py | 4 +- 11 files changed, 439 insertions(+), 81 deletions(-) diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index e32c8c9..f182593 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -8,7 +8,7 @@ { "name": "code-walkthrough", "description": "code-walkthrough: turn a diff, branch, PR, or existing code into a top-down walkthrough page: overview, story map, call graph, annotated diff, and line comments you hand back to the agent", - "version": "1.4.0", + "version": "1.4.1", "source": "./", "author": { "name": "crisperity" diff --git a/skills/code-walkthrough/scripts/cw_llm.py b/skills/code-walkthrough/scripts/cw_llm.py index fc69991..8d85891 100644 --- a/skills/code-walkthrough/scripts/cw_llm.py +++ b/skills/code-walkthrough/scripts/cw_llm.py @@ -45,6 +45,10 @@ _CLAUDE_AUTH_MARKERS = ("not logged in", "unauthorized", "authentication required") _CLAUDE_RATE_MARKERS = ("usage limit", "rate limit", "rate_limit") _CLAUDE_CONTEXT_MARKERS = ("prompt is too long", "context length") +_CLAUDE_FLAGGED_MARKERS = ("safeguards flagged",) +_CLAUDE_FLAGGED_REMEDY = ( + "the content of this batch was flagged by model safeguards; retrying or switching model will not help" +) _semaphore_lock = threading.Lock() _semaphore = threading.BoundedSemaphore(4) @@ -260,6 +264,8 @@ def _classify_claude_error(parsed, stderr, profile): return "rate", _CLAUDE_RATE_REMEDY if any(marker in text for marker in _CLAUDE_CONTEXT_MARKERS): return "context", f"this batch is too big for {profile.get('model')}: lower batch_max_lines" + if any(marker in text for marker in _CLAUDE_FLAGGED_MARKERS): + return "flagged", _CLAUDE_FLAGGED_REMEDY if parsed.get("subtype") == "error_max_turns": return "rounds", None return "error", None diff --git a/skills/code-walkthrough/scripts/cw_run.py b/skills/code-walkthrough/scripts/cw_run.py index 6e1e989..89e4fb2 100644 --- a/skills/code-walkthrough/scripts/cw_run.py +++ b/skills/code-walkthrough/scripts/cw_run.py @@ -38,7 +38,7 @@ import symdelta as symdelta_mod # noqa: E402 import visuals # noqa: E402 from symdelta import add_worktree, remove_worktree # noqa: E402 -from validate_analysis import EMPTY_NOTE_FLOOR, HUNK_PREFIX, parse_hunks, validate # noqa: E402 +from validate_analysis import EMPTY_NOTE_FLOOR, HUNK_PREFIX, WITHHELD_NOTE, parse_hunks, validate # noqa: E402 SCRIPTS_DIR = Path(__file__).resolve().parent SKILL_DIR = SCRIPTS_DIR.parent @@ -592,10 +592,49 @@ def _request_render(d, fragments=None): renderer.request(fragments or []) -def _submit_fragment_handler(d, n, batch_diff, seed, state): +def _withhold(fragment, paths): + for f in fragment["files"]: + if f["path"] in paths: + for h in f["hunks"]: + h["note"] = WITHHELD_NOTE + + +MAX_STUB_ROUNDS = 5 + + +def _record_withheld(d, n, withheld): + def fn(m): + if withheld: + m.setdefault("withheld", {})[str(n)] = withheld + else: + m.get("withheld", {}).pop(str(n), None) + cw_store.update_meta(d, fn) + + +def _withheld_of(fragment): + return sorted(f["path"] for f in fragment["files"] + if any(h.get("note") == WITHHELD_NOTE for h in f["hunks"])) + + +def _retry_flagged(attempt, diff, stub_paths): + """Run `attempt()` -> (..., err); on a flagged error stub the largest file not yet withheld + and run again, up to MAX_STUB_ROUNDS files. `attempt` reads `stub_paths`, which this fills.""" + sizes = {path: len(chunk) for path, chunk in fanout.split_chunks(diff)} + while True: + result = attempt() + if getattr(result[-1], "kind", None) != "flagged" or len(stub_paths) >= MAX_STUB_ROUNDS: + return result + left = [p for p in sizes if p not in fanout.stub_diff(diff, stub_paths)[1]] + if not left: + return result + stub_paths.add(max(left, key=sizes.get)) + + +def _submit_fragment_handler(d, n, batch_diff, seed, state, withheld=()): def handler(args): state["attempt"] += 1 fragment = _copy_into_seed(seed, args) + _withhold(fragment, withheld) problems = validate(batch_diff, fragment, fragment=True) state["fragment"] = fragment state["problems"] = problems @@ -612,7 +651,7 @@ def handler(args): return handler -def _run_fragment_conversation(d, n, entry, config, on_event, role, extra_lines=None, hint=None): +def _run_fragment_conversation(d, n, entry, config, on_event, role, extra_lines=None, hint=None, stub_paths=()): profile = cw_store.role_profile(config, role, agent=_agent_of(d)) if profile is None: return False, None, [f"no profile for role {role}"], None @@ -624,7 +663,8 @@ def _run_fragment_conversation(d, n, entry, config, on_event, role, extra_lines= tools = tools + [_fragment_submit_tool()] batch_prompt = (SKILL_DIR / "prompts" / "batch.md").read_text() system_prompt = batch_prompt + DELIVERY_SUFFIX.format(tool="submit_fragment") - user_text = f"```diff\n{batch_diff}\n```\n\n```json\n{json.dumps(seed, indent=2)}\n```" + shown, withheld = fanout.stub_diff(batch_diff, stub_paths) + user_text = f"```diff\n{shown}\n```\n\n```json\n{json.dumps(seed, indent=2)}\n```" if extra_lines: user_text += ("\n\nThe central gate reported these problems with your files:\n" + "\n".join(extra_lines)) @@ -635,7 +675,7 @@ def _run_fragment_conversation(d, n, entry, config, on_event, role, extra_lines= state = {"attempt": 0, "ok": False, "fragment": None, "problems": []} local_handlers = dict(handlers) - local_handlers["submit_fragment"] = _submit_fragment_handler(d, n, batch_diff, seed, state) + local_handlers["submit_fragment"] = _submit_fragment_handler(d, n, batch_diff, seed, state, withheld) messages = [{"role": "system", "content": system_prompt}, {"role": "user", "content": user_text}] error = None exc = None @@ -660,6 +700,7 @@ def _run_batch(d, n, entry, config, on_event): if fragment_path.exists(): frag = cw_store.read_json(fragment_path, default=None) if frag is not None and not validate(batch_diff, frag, fragment=True): + _record_withheld(d, n, _withheld_of(frag)) cw_store.update_meta( d, lambda m: (m.setdefault("batches_done", []).append(n) if n not in m.get("batches_done", []) else None)) _emit_step(d, on_event, name, "ok", reused=True) @@ -671,6 +712,7 @@ def _run_batch(d, n, entry, config, on_event): frag = _copy_into_seed(json.loads(Path(entry["seed"]).read_text()), {}) if not validate(batch_diff, frag, fragment=True): fragment_path.write_text(json.dumps(frag, indent=2)) + _record_withheld(d, n, _withheld_of(frag)) cw_store.update_meta( d, lambda m: (m.setdefault("batches_done", []).append(n) if n not in m.get("batches_done", []) else None)) _emit_step(d, on_event, name, "ok", reused=True, carried=True) @@ -683,14 +725,17 @@ def _run_batch(d, n, entry, config, on_event): fragment = None problems = [] last_error = None + stub_paths = set() for role in ("analysis", "escalate"): profile = cw_store.role_profile(config, role, agent=_agent_of(d)) if profile is None: continue attempts += 1 model_used = profile.get("model") - ok, frag, probs, err = _run_fragment_conversation(d, n, entry, config, on_event, role, - hint=carry.get("pending")) + ok, frag, probs, err = _retry_flagged( + lambda: _run_fragment_conversation( + d, n, entry, config, on_event, role, hint=carry.get("pending"), stub_paths=stub_paths), + batch_diff, stub_paths) if ok: passed, fragment = True, frag break @@ -698,13 +743,20 @@ def _run_batch(d, n, entry, config, on_event): fragment, problems = frag, probs if err is not None: last_error = err + if getattr(err, "kind", None) == "flagged": + break + withheld = sorted(fanout.stub_diff(batch_diff, stub_paths)[1]) if passed: - _emit_step(d, on_event, name, "ok", attempts=attempts, model=model_used) + _record_withheld(d, n, withheld) + _emit_step(d, on_event, name, "ok", attempts=attempts, model=model_used, withheld=withheld or None) return True if not problems and last_error is not None: + remedy = last_error.remedy + if getattr(last_error, "kind", None) == "flagged": + remedy = f"batch {n} ({', '.join(entry['files'])}): {remedy}" _emit_step(d, on_event, name, "failed", attempts=attempts, model=model_used, - error=str(last_error), remedy=last_error.remedy) + error=str(last_error), remedy=remedy) return False _emit_step(d, on_event, name, "failed", attempts=attempts, model=model_used, lines=problems) return False @@ -1159,19 +1211,27 @@ def _route_and_finalize(d, meta, config, on_event, batch_count, manifest, other_ batches_to_retry.add(n) _emit_step(d, on_event, "routed", "running") - for n in batches_to_retry: + withheld = (cw_store.read_meta(d) or {}).get("withheld") or {} + flagged = [] + for n in sorted(batches_to_retry): entry = manifest[n - 1] lines = file_lines_by_batch.get(n, []) + floor_lines - _run_fragment_conversation(d, n, entry, config, on_event, "analysis", extra_lines=lines) + *_, err = _run_fragment_conversation( + d, n, entry, config, on_event, "analysis", extra_lines=lines, + stub_paths=set(withheld.get(str(n), []))) + if getattr(err, "kind", None) == "flagged": + flagged.append(f"batch {n} ({', '.join(entry['files'])})") + note = ("model safeguards flagged the routed retry of " + "; ".join(flagged) + + ": kept the earlier fragment") if flagged else None prose = cw_store.read_json(d / "prose.json", default={}) fragment_paths = _fragment_paths(d, batch_count) merged, problems = _merge_and_validate(d, fragment_paths, prose) if not problems: (d / "analysis.json").write_text(json.dumps(merged, indent=2)) - _emit_step(d, on_event, "routed", "ok") + _emit_step(d, on_event, "routed", "ok", remedy=note) return True, [] - _emit_step(d, on_event, "routed", "failed", lines=problems) + _emit_step(d, on_event, "routed", "failed", remedy=note, lines=problems) return False, problems @@ -1186,8 +1246,6 @@ def _run_small(d, meta, config, on_event): batch_prompt = (SKILL_DIR / "prompts" / "batch.md").read_text() prose_prompt = (SKILL_DIR / "prompts" / "prose.md").read_text() system_prompt = batch_prompt + "\n\n" + prose_prompt + DELIVERY_SUFFIX.format(tool="submit_analysis") - user_text = (f"```diff\n{raw_diff}\n```\n\n```json\n{json.dumps(seed, indent=2)}\n```" - f"\n\nNumstat:\n{numstat}") attempts = 0 model_used = None @@ -1196,75 +1254,96 @@ def _run_small(d, meta, config, on_event): prose_json = None problems = [] last_exc = None + stub_paths = set() + entry_paths = [f["path"] for f in seed["files"]] for role in ("prose", "escalate"): profile = cw_store.role_profile(config, role, agent=_agent_of(d)) if profile is None: continue attempts += 1 model_used = profile.get("model") - state = {"attempt": 0, "ok": False, "analysis": None, "prose": None, "problems": []} - - def handler(args, state=state): - state["attempt"] += 1 - args = args if isinstance(args, dict) else {} - fragment = _copy_into_seed(seed, args) - if not validate(raw_diff, fragment, fragment=True): - (d / "batches" / "fragment-1.json").write_text(json.dumps(fragment, indent=2)) - cw_store.update_meta( - d, lambda m: (m.setdefault("batches_done", []).append(1) if 1 not in m.get("batches_done", []) else None)) - _request_render(d, [1]) - - prose = { - "target": meta["target"], - "overview": args.get("overview") if isinstance(args.get("overview"), str) else "", - "verdict": args.get("verdict") if isinstance(args.get("verdict"), str) else "", - } - if isinstance(args.get("groups"), list): - prose["groups"] = args["groups"] - full = {**{k: v for k, v in prose.items() if k != "target"}, "target": prose["target"], - "files": fragment["files"]} - full_problems = validate(raw_diff, full) - if _blank(full.get("verdict")): - full_problems.append("verdict is empty") - state["analysis"] = full - state["prose"] = prose - state["problems"] = full_problems - retry = state["attempt"] < 3 - if not full_problems: - state["ok"] = True - return cw_llm.Done({"ok": True}) - if not retry: - return cw_llm.Done({"ok": False, "problems": full_problems}) - return "gate: " + "\n".join(full_problems) + "\nfix and call submit_analysis again" - - local_handlers = dict(handlers) - local_handlers["submit_analysis"] = handler - messages = [{"role": "system", "content": system_prompt}, {"role": "user", "content": user_text}] - error = None - try: - cw_llm.run_tools( - profile, messages, tools, local_handlers, max_rounds=10, - max_tokens=config["max_conversation_tokens"], timeout=config["timeout_s"], - on_usage=_make_on_usage(d, role, on_event, config), cwd=d / "head", - ) - except cw_llm.LLMError as e: - error = str(e) - last_exc = e - _write_transcript(d, role, profile, messages, result=state["ok"], error=error) + + def attempt(role=role, profile=profile): + shown, withheld = fanout.stub_diff(raw_diff, stub_paths) + user_text = (f"```diff\n{shown}\n```\n\n```json\n{json.dumps(seed, indent=2)}\n```" + f"\n\nNumstat:\n{numstat}") + state = {"attempt": 0, "ok": False, "analysis": None, "prose": None, "problems": []} + + def handler(args, state=state): + state["attempt"] += 1 + args = args if isinstance(args, dict) else {} + fragment = _copy_into_seed(seed, args) + _withhold(fragment, withheld) + if not validate(raw_diff, fragment, fragment=True): + (d / "batches" / "fragment-1.json").write_text(json.dumps(fragment, indent=2)) + cw_store.update_meta( + d, lambda m: (m.setdefault("batches_done", []).append(1) if 1 not in m.get("batches_done", []) else None)) + _request_render(d, [1]) + + prose = { + "target": meta["target"], + "overview": args.get("overview") if isinstance(args.get("overview"), str) else "", + "verdict": args.get("verdict") if isinstance(args.get("verdict"), str) else "", + } + if isinstance(args.get("groups"), list): + prose["groups"] = args["groups"] + full = {**{k: v for k, v in prose.items() if k != "target"}, "target": prose["target"], + "files": fragment["files"]} + full_problems = validate(raw_diff, full) + if _blank(full.get("verdict")): + full_problems.append("verdict is empty") + state["analysis"] = full + state["prose"] = prose + state["problems"] = full_problems + retry = state["attempt"] < 3 + if not full_problems: + state["ok"] = True + return cw_llm.Done({"ok": True}) + if not retry: + return cw_llm.Done({"ok": False, "problems": full_problems}) + return "gate: " + "\n".join(full_problems) + "\nfix and call submit_analysis again" + + local_handlers = dict(handlers) + local_handlers["submit_analysis"] = handler + messages = [{"role": "system", "content": system_prompt}, {"role": "user", "content": user_text}] + error = None + exc = None + try: + cw_llm.run_tools( + profile, messages, tools, local_handlers, max_rounds=10, + max_tokens=config["max_conversation_tokens"], timeout=config["timeout_s"], + on_usage=_make_on_usage(d, role, on_event, config), cwd=d / "head", + ) + except cw_llm.LLMError as e: + error = str(e) + exc = e + _write_transcript(d, role, profile, messages, result=state["ok"], error=error) + return state, exc + + state, exc = _retry_flagged(attempt, raw_diff, stub_paths) + if exc is not None: + last_exc = exc if state["ok"]: passed, analysis, prose_json = True, state["analysis"], state["prose"] break if state.get("analysis") is not None: analysis, prose_json, problems = state["analysis"], state["prose"], state["problems"] + if getattr(exc, "kind", None) == "flagged": + break if passed: (d / "prose.json").write_text(json.dumps(prose_json, indent=2)) (d / "analysis.json").write_text(json.dumps(analysis, indent=2)) - _emit_step(d, on_event, name, "ok", attempts=attempts, model=model_used) + withheld = sorted(fanout.stub_diff(raw_diff, stub_paths)[1]) + _record_withheld(d, 1, withheld) + _emit_step(d, on_event, name, "ok", attempts=attempts, model=model_used, withheld=withheld or None) return True if not problems and last_exc is not None: + remedy = last_exc.remedy + if getattr(last_exc, "kind", None) == "flagged": + remedy = f"files {', '.join(entry_paths)}: {remedy}" _emit_step(d, on_event, name, "failed", attempts=attempts, model=model_used, - error=str(last_exc), remedy=last_exc.remedy) + error=str(last_exc), remedy=remedy) cw_store.update_meta(d, lambda m: m.update({"gate": []})) return False _emit_step(d, on_event, name, "failed", attempts=attempts, model=model_used, lines=problems) @@ -1701,11 +1780,12 @@ def _split(d, meta, config): if total <= config["small_diff_lines"]: route = "small" args = ["split", "--diff", str(d / "raw.diff"), "--out", str(batches_dir), - "--max-batches", "1", "--max-lines", "1000000000"] + "--max-batches", "1", "--max-lines", "1000000000", "--max-chars", "1000000000"] else: route = "fanout" args = ["split", "--diff", str(d / "raw.diff"), "--out", str(batches_dir), - "--max-lines", str(config["batch_max_lines"])] + "--max-lines", str(config["batch_max_lines"]), + "--max-chars", str(config["batch_max_chars"])] result = subprocess.run([sys.executable, str(SCRIPTS_DIR / "fanout.py"), *args], capture_output=True, text=True) if result.returncode != 0: diff --git a/skills/code-walkthrough/scripts/cw_server.py b/skills/code-walkthrough/scripts/cw_server.py index 74e9d2f..b584ccd 100644 --- a/skills/code-walkthrough/scripts/cw_server.py +++ b/skills/code-walkthrough/scripts/cw_server.py @@ -335,7 +335,7 @@ def _tool_walkthrough_get(daemon, args): result["meta"] = { "target": meta.get("target"), "base": meta.get("base"), "head": meta.get("head"), "route": meta.get("route"), "batches": meta.get("batches"), - "batches_done": meta.get("batches_done"), "steps": _steps_public(meta), + "batches_done": meta.get("batches_done"), "withheld": meta.get("withheld"), "steps": _steps_public(meta), "usage": meta.get("usage", {}), "total": cw_store.usage_total(meta), "created_at": meta.get("created_at"), "updated_at": meta.get("updated_at"), "dir": str(d), } diff --git a/skills/code-walkthrough/scripts/cw_store.py b/skills/code-walkthrough/scripts/cw_store.py index 5f4e996..17554fa 100644 --- a/skills/code-walkthrough/scripts/cw_store.py +++ b/skills/code-walkthrough/scripts/cw_store.py @@ -86,6 +86,7 @@ def strip_ref_fences(text): "max_conversation_tokens": 200000, "small_diff_lines": 1500, "batch_max_lines": 400, + "batch_max_chars": 120000, "trust": {"repos": [], "orgs": []}, "repos": {}, "port": 27183, diff --git a/skills/code-walkthrough/scripts/fanout.py b/skills/code-walkthrough/scripts/fanout.py index 2ff1515..f09ab0c 100644 --- a/skills/code-walkthrough/scripts/fanout.py +++ b/skills/code-walkthrough/scripts/fanout.py @@ -1,7 +1,7 @@ #!/usr/bin/env python3 """Split raw.diff into per-file batches for parallel analysis, then merge the results. - python3 fanout.py split --diff raw.diff --out [--max-lines 400] [--max-batches 8] + python3 fanout.py split --diff raw.diff --out [--max-lines 400] [--max-chars 120000] [--max-batches 8] python3 fanout.py merge --diff raw.diff --prose prose.json \ --fragments /fragment-1.json ... --out analysis.json @@ -66,19 +66,71 @@ def split_chunks(text): return [(order[i], text[bounds[i] : bounds[i + 1]]) for i in range(len(order))] -def batch(chunks, max_lines, max_batches): +STUB_NAME = re.compile(r"\.golden\.|\.snap$|\.lock$|(^|/)(package-lock\.json|pnpm-lock\.yaml)$") +STUB_LINE_CHARS = 2000 +STUB_FILE_CHARS = 60000 + + +def should_stub(path, chunk): + return ( + bool(STUB_NAME.search(path)) + or "GIT binary patch" in chunk + or re.search(r"^Binary files .* differ$", chunk, re.M) is not None + or len(chunk) > STUB_FILE_CHARS + or any(len(line) > STUB_LINE_CHARS for line in chunk.split("\n")) + ) + + +def stub_chunk(chunk): + """Keep the file header and every `@@` line, drop the hunk bodies: seed entries and + validation key on the headers.""" + out, body, in_body = [], 0, False + + def flush(): + if body: + out.append(f"<{-(-body // 1000)} KB omitted: golden/generated/binary>\n") + + for line in chunk.splitlines(keepends=True): + if line.startswith("@@"): + flush() + out.append(line) + body, in_body = 0, True + elif in_body or line.startswith("GIT binary patch"): + body, in_body = body + len(line), True + else: + out.append(line) + flush() + return "".join(out) + + +def stub_diff(text, extra=()): + """The diff as a model should see it, plus the paths whose bodies were withheld.""" + parts, withheld = [], set() + for path, chunk in split_chunks(text): + if path in extra or should_stub(path, chunk): + parts.append(stub_chunk(chunk)) + withheld.add(path) + else: + parts.append(chunk) + return "".join(parts), withheld + + +def batch(chunks, max_lines, max_batches, max_chars=None): """Greedy contiguous fill. A file never straddles two batches: its notes read better written against the whole file's diff.""" total = sum(chunk.count("\n") for _, chunk in chunks) budget = max(max_lines, -(-total // max_batches)) if max_batches > 0 else max_lines - batches, current, current_lines = [], [], 0 + batches, current, current_lines, current_chars = [], [], 0, 0 for path, chunk in chunks: lines = chunk.count("\n") - if current and current_lines + lines > budget: + size = len(stub_chunk(chunk) if should_stub(path, chunk) else chunk) + if current and (current_lines + lines > budget + or (max_chars and current_chars + size > max_chars)): batches.append(current) - current, current_lines = [], 0 + current, current_lines, current_chars = [], 0, 0 current.append((path, chunk)) current_lines += lines + current_chars += size if current: batches.append(current) return batches @@ -109,7 +161,7 @@ def do_split(args): _, diff_files = parse_hunks(text) manifest = [] - for i, group in enumerate(batch(split_chunks(text), args.max_lines, args.max_batches), 1): + for i, group in enumerate(batch(split_chunks(text), args.max_lines, args.max_batches, args.max_chars), 1): batch_path = out_dir / f"batch-{i}.diff" seed_path = out_dir / f"fragment-{i}.seed.json" body = "".join(chunk for _, chunk in group) @@ -195,6 +247,7 @@ def main(): split_parser.add_argument("--diff", required=True) split_parser.add_argument("--out", required=True) split_parser.add_argument("--max-lines", type=int, default=400) + split_parser.add_argument("--max-chars", type=int, default=120000) split_parser.add_argument("--max-batches", type=int, default=8) split_parser.set_defaults(func=do_split) diff --git a/skills/code-walkthrough/scripts/test_cw_llm.py b/skills/code-walkthrough/scripts/test_cw_llm.py index 531d450..0382b5e 100644 --- a/skills/code-walkthrough/scripts/test_cw_llm.py +++ b/skills/code-walkthrough/scripts/test_cw_llm.py @@ -354,6 +354,7 @@ def test_claude_call_error_kinds(): "rate": [cw_testlib.claude_result(is_error=True, result="rate limit hit", api_error_status=429)], "ctx": [cw_testlib.claude_result(is_error=True, result="Prompt is too long")], "nf": [not_found], + "flag": [cw_testlib.claude_result(is_error=True, result="API Error: Sonnet 5.5's safeguards flagged this message")], "raw": [cw_testlib.claude_raw("not json", exit=1, stderr="boom")], "long": [cw_testlib.claude_result(is_error=True, result="z" * 500)], } @@ -370,6 +371,8 @@ def failure(model, **extra): assert error.kind == "context" and "ctx" in error.remedy error = failure("nf", name="prof-nf") assert error.kind == "notfound" and "prof-nf" in error.remedy + error = failure("flag") + assert error.kind == "flagged" and "safeguards" in error.remedy assert failure("raw").kind == "protocol" error = failure("long") assert error.kind == "error" and len(str(error)) == 300 diff --git a/skills/code-walkthrough/scripts/test_cw_run.py b/skills/code-walkthrough/scripts/test_cw_run.py index aff854e..b9c9abf 100644 --- a/skills/code-walkthrough/scripts/test_cw_run.py +++ b/skills/code-walkthrough/scripts/test_cw_run.py @@ -21,6 +21,7 @@ import cw_store # noqa: E402 import cw_testlib # noqa: E402 import fanout # noqa: E402 +from validate_analysis import WITHHELD_NOTE # noqa: E402 def _config(**overrides): @@ -41,7 +42,8 @@ def _prepare_batches(tmp, repo, base, head, max_lines=1): buf = io.StringIO() with contextlib.redirect_stdout(buf): fanout.do_split(types.SimpleNamespace( - diff=str(d / "raw.diff"), out=str(batches_dir), max_lines=max_lines, max_batches=8)) + diff=str(d / "raw.diff"), out=str(batches_dir), max_lines=max_lines, max_batches=8, + max_chars=1000000000)) manifest = json.loads(buf.getvalue()) (batches_dir / "manifest.json").write_text(json.dumps(manifest, indent=2)) return d, manifest @@ -121,6 +123,169 @@ def test_failing_fragment_gate_escalates(kind): assert all(Path(e["cwd"]).resolve() == (d / "head").resolve() for e in log()) +def test_stubbed_long_line_file_reaches_the_model_headers_only_and_passes_with_the_canned_note(): + big = "x" * 5000 + with tempfile.TemporaryDirectory() as tmp: + repo, base, head = cw_testlib.make_repo(tmp, {"foo.py": "a\nb\nc\n"}, {"foo.py": f"a\n{big}\nc\n"}) + d, manifest = _prepare_batches(tmp, repo, base, head, max_lines=50) + (d / "head").mkdir() + entry = manifest[0] + seed = json.loads(Path(entry["seed"]).read_text()) + with cw_testlib.StubLLM({"m": [cw_testlib.tool_call("submit_fragment", _passing_fragment(seed))]}) as stub: + config = _config(profiles={"a": stub.profile("m")}, roles={"analysis": "a", "escalate": "a"}) + assert cw_run._run_batch(d, 1, entry, config, None) is True + prompt = json.dumps(stub.requests) + assert big not in prompt and "KB omitted" in prompt and seed["files"][0]["hunks"][0]["header"] in prompt + frag = json.loads((d / "batches" / "fragment-1.json").read_text()) + assert {h["note"] for h in frag["files"][0]["hunks"]} == {WITHHELD_NOTE} + assert json.loads((d / "meta.json").read_text())["withheld"] == {"1": ["foo.py"]} + + +def test_flagged_error_skips_escalate_and_names_the_batch_files(): + cw_testlib.require_posix() + with tempfile.TemporaryDirectory() as tmp, contextlib.ExitStack() as stack: + repo, base, head = cw_testlib.make_repo(tmp, {"foo.py": "a\nb\nc\n"}, {"foo.py": "a\nX\nc\n"}) + d, manifest = _prepare_batches(tmp, repo, base, head) + (d / "head").mkdir() + flagged = cw_testlib.claude_result(is_error=True, result="API Error: safeguards flagged this message") + fc = stack.enter_context(cw_testlib.fake_claude(tmp, {"sonnet": [flagged] * 3, "opus": [flagged] * 3})) + config = _config(profiles={"a": {"kind": "claude-code", "model": "sonnet"}, + "e": {"kind": "claude-code", "model": "opus"}}, + roles={"analysis": "a", "escalate": "e"}) + assert cw_run._run_batch(d, 1, manifest[0], config, None) is False + assert fc.count("opus") == 0 + step = json.loads((d / "meta.json").read_text())["steps"]["batch-1"] + assert step["attempts"] == 1 + assert "batch 1" in step["remedy"] and "foo.py" in step["remedy"] + + +def test_flagged_batch_retries_with_the_largest_file_stubbed(): + cw_testlib.require_posix() + with tempfile.TemporaryDirectory() as tmp, contextlib.ExitStack() as stack: + repo, base, head = cw_testlib.make_repo( + tmp, {"small.py": "a\nb\n", "large.py": "a\nb\n"}, + {"small.py": "a\nX\n", "large.py": "a\n" + "y = 1\n" * 200}) + d, manifest = _prepare_batches(tmp, repo, base, head, max_lines=1000) + (d / "head").mkdir() + entry = manifest[0] + assert len(manifest) == 1 + seed = json.loads(Path(entry["seed"]).read_text()) + flagged = cw_testlib.claude_result(is_error=True, result="API Error: safeguards flagged this message") + good = cw_testlib.claude_result(structured=_passing_fragment(seed)) + fc = stack.enter_context(cw_testlib.fake_claude(tmp, {"sonnet": [flagged, good], "opus": [good]})) + config = _config(profiles={"a": {"kind": "claude-code", "model": "sonnet"}, + "e": {"kind": "claude-code", "model": "opus"}}, + roles={"analysis": "a", "escalate": "e"}) + events = [] + assert cw_run._run_batch(d, 1, entry, config, lambda k, v: events.append(v)) is True + assert fc.count("sonnet") == 2 and fc.count("opus") == 0 + meta = json.loads((d / "meta.json").read_text()) + assert meta["withheld"] == {"1": ["large.py"]} + assert meta["steps"]["batch-1"]["withheld"] == ["large.py"] + frag = json.loads((d / "batches" / "fragment-1.json").read_text()) + by_path = {f["path"]: f for f in frag["files"]} + assert all(h["note"] == WITHHELD_NOTE for h in by_path["large.py"]["hunks"]) + assert all(h["note"] != WITHHELD_NOTE for h in by_path["small.py"]["hunks"]) + + +def test_batch_stays_failed_when_every_file_is_stubbed_and_still_flagged(): + cw_testlib.require_posix() + with tempfile.TemporaryDirectory() as tmp, contextlib.ExitStack() as stack: + repo, base, head = cw_testlib.make_repo(tmp, {"foo.py": "a\nb\n"}, {"foo.py": "a\nX\n"}) + d, manifest = _prepare_batches(tmp, repo, base, head) + (d / "head").mkdir() + flagged = cw_testlib.claude_result(is_error=True, result="API Error: safeguards flagged this message") + fc = stack.enter_context(cw_testlib.fake_claude(tmp, {"sonnet": [flagged] * 5, "opus": [flagged] * 5})) + config = _config(profiles={"a": {"kind": "claude-code", "model": "sonnet"}, + "e": {"kind": "claude-code", "model": "opus"}}, + roles={"analysis": "a", "escalate": "e"}) + assert cw_run._run_batch(d, 1, manifest[0], config, None) is False + assert fc.count("sonnet") == 2 and fc.count("opus") == 0 + assert "foo.py" in json.loads((d / "meta.json").read_text())["steps"]["batch-1"]["remedy"] + + +def test_flagged_recovery_stops_after_five_stubbed_files(): + cw_testlib.require_posix() + names = [f"f{i}.py" for i in range(7)] + with tempfile.TemporaryDirectory() as tmp, contextlib.ExitStack() as stack: + repo, base, head = cw_testlib.make_repo( + tmp, {n: "a\nb\n" for n in names}, {n: "a\nX\n" for n in names}) + d, manifest = _prepare_batches(tmp, repo, base, head, max_lines=1000) + (d / "head").mkdir() + flagged = cw_testlib.claude_result(is_error=True, result="API Error: safeguards flagged this message") + fc = stack.enter_context(cw_testlib.fake_claude(tmp, {"sonnet": [flagged] * 20, "opus": [flagged] * 20})) + config = _config(profiles={"a": {"kind": "claude-code", "model": "sonnet"}, + "e": {"kind": "claude-code", "model": "opus"}}, + roles={"analysis": "a", "escalate": "e"}) + assert cw_run._run_batch(d, 1, manifest[0], config, None) is False + assert fc.count("sonnet") == 6 and fc.count("opus") == 0 + + +def test_small_route_stubs_and_retries_on_flagged_without_escalating(): + cw_testlib.require_posix() + with tempfile.TemporaryDirectory() as tmp, contextlib.ExitStack() as stack: + repo, base, head = cw_testlib.make_repo( + tmp, {"small.py": "a\nb\n", "large.py": "a\nb\n"}, + {"small.py": "a\nX\n", "large.py": "a\n" + "y = 1\n" * 200}) + d, manifest = _prepare_batches(tmp, repo, base, head, max_lines=1000) + (d / "head").mkdir() + seed = json.loads(Path(manifest[0]["seed"]).read_text()) + analysis = {**_passing_fragment(seed), "overview": "Overview text", "verdict": "v"} + flagged = cw_testlib.claude_result(is_error=True, result="API Error: safeguards flagged this message") + good = cw_testlib.claude_result(structured=analysis) + fc = stack.enter_context(cw_testlib.fake_claude(tmp, {"sonnet": [flagged, good], "opus": [good]})) + config = _config(profiles={"a": {"kind": "claude-code", "model": "sonnet"}, + "e": {"kind": "claude-code", "model": "opus"}}, + roles={"prose": "a", "escalate": "e"}) + assert cw_run._run_small(d, {"target": "t"}, config, None) is True + assert fc.count("sonnet") == 2 and fc.count("opus") == 0 + assert json.loads((d / "meta.json").read_text())["withheld"] == {"1": ["large.py"]} + by_path = {f["path"]: f for f in json.loads((d / "analysis.json").read_text())["files"]} + assert all(h["note"] == WITHHELD_NOTE for h in by_path["large.py"]["hunks"]) + + +def test_clean_pass_clears_a_stale_withheld_entry_and_reuse_recomputes_it(): + with tempfile.TemporaryDirectory() as tmp: + repo, base, head = cw_testlib.make_repo(tmp, {"foo.py": "a\nb\nc\n"}, {"foo.py": "a\nX\nc\n"}) + d, manifest = _prepare_batches(tmp, repo, base, head) + entry = manifest[0] + seed = json.loads(Path(entry["seed"]).read_text()) + cw_store.update_meta(d, lambda m: m.update({"withheld": {"1": ["foo.py"]}})) + with cw_testlib.StubLLM({"m": [cw_testlib.tool_call("submit_fragment", _passing_fragment(seed))]}) as stub: + config = _config(profiles={"a": stub.profile("m")}, roles={"analysis": "a", "escalate": "a"}) + assert cw_run._run_batch(d, 1, entry, config, None) is True + assert "1" not in json.loads((d / "meta.json").read_text())["withheld"] + frag = _passing_fragment(seed) + frag["files"][0]["hunks"][0]["note"] = WITHHELD_NOTE + (d / "batches" / "fragment-1.json").write_text(json.dumps(frag)) + assert cw_run._run_batch(d, 1, entry, _config(), None) is True + assert json.loads((d / "meta.json").read_text())["withheld"] == {"1": ["foo.py"]} + + +def test_routed_retry_passes_withheld_paths_and_keeps_the_fragment_when_flagged(monkeypatch): + cw_testlib.require_posix() + with tempfile.TemporaryDirectory() as tmp, contextlib.ExitStack() as stack: + repo, base, head = cw_testlib.make_repo(tmp, {"b.py": "x\ny\nz\n"}, {"b.py": "x\nB\nz\n"}) + d, manifest = _prepare_batches(tmp, repo, base, head) + (d / "head").mkdir() + seed = json.loads(Path(manifest[0]["seed"]).read_text()) + good = _passing_fragment(seed) + (d / "batches" / "fragment-1.json").write_text(json.dumps(good)) + (d / "prose.json").write_text(json.dumps({"target": "t", "overview": "", "verdict": ""})) + cw_store.update_meta(d, lambda m: m.update({"withheld": {"1": ["b.py"]}})) + flagged = cw_testlib.claude_result(is_error=True, result="API Error: safeguards flagged this message") + stack.enter_context(cw_testlib.fake_claude(tmp, {"sonnet": [flagged] * 3})) + seen = [] + real = cw_run._run_fragment_conversation + monkeypatch.setattr(cw_run, "_run_fragment_conversation", + lambda *a, **k: (seen.append(k.get("stub_paths")), real(*a, **k))[1]) + config = _config(profiles={"a": {"kind": "claude-code", "model": "sonnet"}}, roles={"analysis": "a"}) + cw_run._route_and_finalize(d, {"target": "t"}, config, None, 1, manifest, ["b.py: role is empty"]) + assert seen == [{"b.py"}] + assert json.loads((d / "batches" / "fragment-1.json").read_text()) == good + assert "batch 1 (b.py)" in json.loads((d / "meta.json").read_text())["steps"]["routed"]["remedy"] + + def test_llm_error_counts_as_one_failed_conversation_and_moves_on(): with tempfile.TemporaryDirectory() as tmp: repo, base, head = cw_testlib.make_repo(tmp, {"foo.py": "a\nb\nc\n"}, {"foo.py": "a\nX\nc\n"}) diff --git a/skills/code-walkthrough/scripts/test_fanout.py b/skills/code-walkthrough/scripts/test_fanout.py index 14fdf22..5cbd618 100644 --- a/skills/code-walkthrough/scripts/test_fanout.py +++ b/skills/code-walkthrough/scripts/test_fanout.py @@ -10,7 +10,7 @@ from pathlib import Path sys.path.insert(0, str(Path(__file__).parent)) -from fanout import batch, do_split, merge, split_chunks # noqa: E402 +from fanout import batch, do_split, merge, split_chunks, stub_diff # noqa: E402 def _file_diff(path, hunks): @@ -20,9 +20,10 @@ def _file_diff(path, hunks): return body -def _args(diff, out, max_lines=400, max_batches=8): +def _args(diff, out, max_lines=400, max_batches=8, max_chars=10**9): """Stand-in for the argparse.Namespace do_split reads its options from.""" - return types.SimpleNamespace(diff=diff, out=out, max_lines=max_lines, max_batches=max_batches) + return types.SimpleNamespace(diff=diff, out=out, max_lines=max_lines, max_batches=max_batches, + max_chars=max_chars) DIFF = _file_diff("a.py", 1) + _file_diff("big.py", 30) + _file_diff("c.py", 1) @@ -253,3 +254,42 @@ def test_split_seed_marks_a_binary_file_with_no_hunks(): seed = json.loads((out_dir / "fragment-1.seed.json").read_text()) assert seed["files"] == [{"path": "logo.png", "role": "", "hunks": []}] + + +def test_batch_budgets_by_characters_and_gives_an_oversized_file_its_own_batch(): + chunks = [("a", "x" * 50), ("big", "y" * 500), ("b", "x" * 50), ("c", "x" * 50)] + got = batch(chunks, max_lines=1000, max_batches=1, max_chars=120) + assert [[p for p, _ in g] for g in got] == [["a"], ["big"], ["b", "c"]] + + +def _plain(path, body="+x\n"): + return f"diff --git a/{path} b/{path}\n--- a/{path}\n+++ b/{path}\n@@ -1,1 +1,2 @@\n ctx\n{body}" + + +def test_stub_diff_keeps_headers_and_withholds_golden_binary_long_line_and_big_files(): + big_body = "".join(f"+line {i} {'z' * 40}\n" for i in range(1500)) + diff = ( + _plain("ok.py") + + _plain("t/out.golden.xml", "+\n") + + "diff --git a/img.png b/img.png\nindex 1..2 100644\nGIT binary patch\nliteral 3\nabc\n\n" + + _plain("long.py", "+" + "q" * 2500 + "\n") + + _plain("huge.py", big_body) + ) + shown, withheld = stub_diff(diff) + assert withheld == {"t/out.golden.xml", "img.png", "long.py", "huge.py"} + assert "+x\n" in shown.split("diff --git a/t/out")[0] + for path in ("t/out.golden.xml", "long.py", "huge.py"): + assert f"+++ b/{path}\n@@ -1,1 +1,2 @@\n<" in shown + assert "q" * 2500 not in shown and "literal 3" not in shown and "KB omitted" in shown + assert split_chunks(shown)[0][0] == "ok.py" and len(split_chunks(shown)) == 5 + + +def test_stub_diff_extra_paths_are_withheld_too(): + shown, withheld = stub_diff(_plain("a.py") + _plain("b.py"), extra={"b.py"}) + assert withheld == {"b.py"} and "+x" in shown.split("diff --git a/b.py")[0] + + +def test_batch_counts_a_withheld_file_at_its_stubbed_size(): + golden = "diff --git a/x.golden.xml b/x.golden.xml\n--- a/x.golden.xml\n+++ b/x.golden.xml\n@@ -1,1 +1,1 @@\n+" + "g" * 5000 + "\n" + chunks = [("x.golden.xml", golden), ("a.py", _plain("a.py"))] + assert len(batch(chunks, max_lines=1000, max_batches=1, max_chars=1000)) == 1 diff --git a/skills/code-walkthrough/scripts/test_validate_analysis.py b/skills/code-walkthrough/scripts/test_validate_analysis.py index 2e4fafb..67da032 100644 --- a/skills/code-walkthrough/scripts/test_validate_analysis.py +++ b/skills/code-walkthrough/scripts/test_validate_analysis.py @@ -572,3 +572,11 @@ def test_overview_blob_gate(text, names_length): assert str(len(text)) in problems[0] else: assert problems == [] + + +def test_withheld_note_is_accepted_though_it_shares_no_identifier_with_the_hunk(): + from validate_analysis import WITHHELD_NOTE + diff = "diff --git a/f.py b/f.py\n--- a/f.py\n+++ b/f.py\n@@ -1,1 +1,1 @@\n-old_value = 1\n+new_value = 2\n" + frag = {"files": [{"path": "f.py", "role": "holds the settings", "hunks": [ + {"header": "@@ -1,1 +1,1 @@", "note": WITHHELD_NOTE}]}]} + assert validate(diff, frag, fragment=True) == [] diff --git a/skills/code-walkthrough/scripts/validate_analysis.py b/skills/code-walkthrough/scripts/validate_analysis.py index e54a735..96994a1 100644 --- a/skills/code-walkthrough/scripts/validate_analysis.py +++ b/skills/code-walkthrough/scripts/validate_analysis.py @@ -71,6 +71,8 @@ IDENTIFIER = re.compile(r"[A-Za-z_][A-Za-z0-9_]*") CAMEL_BOUNDARY = re.compile(r"(?<=[a-z0-9])(?=[A-Z])") MIN_IDENTIFIER_LEN = 4 +WITHHELD_NOTE = "content withheld: flagged by model safeguards" + # Per-hunk churn detection (rename/export/import-repoint/qualifier-drop vs. "real" change) # is not reliably decidable from a language-independent diff read: on a measured 45-file # analysis it produced 49 false rejections, each one genuine churn a subset-of-tokens test @@ -351,7 +353,7 @@ def validate(diff_text, analysis, fragment=False): covered[header] = hunk all_hunks.append(hunk) body = body_by_prefix.get(header) - if not _blank(hunk.get("note")) and body is not None: + if not _blank(hunk.get("note")) and hunk["note"].strip() != WITHHELD_NOTE and body is not None: changed = "\n".join(raw for kind, raw in body["lines"] if kind in ("a", "d")) hunk_ids = _identifiers(changed) if hunk_ids and hunk_ids.isdisjoint(_identifiers(hunk["note"])):