From aa7892fd43398f987f5ebc54b17812f5a75c2494 Mon Sep 17 00:00:00 2001 From: crisperity Date: Fri, 30 Oct 2026 09:36:19 +0100 Subject: [PATCH 1/2] feat(code-walkthrough): replace agent drafts, render visuals in parallel post_draft_comments items take an optional `replaces` (id of an existing local draft). The page creates the new draft first, then removes the old one, so revising a draft no longer leaves two. The agent prompt tells it to use `replaces`. Visuals ran as one serial LLM call for the overview and every group, 26 to 101 s on recent runs (101 s of a 163 s run on PR 74). Now it's one call for the overview and one per group, up to 4 at once. A failed group is dropped, the rest is kept. Total picture size is still capped, and problems are stored on the visuals step. --- .../assets/diff-review-template.html | 2 + skills/code-walkthrough/prompts/agent.md | 4 +- skills/code-walkthrough/scripts/cw_mcp.py | 30 ++- skills/code-walkthrough/scripts/cw_run.py | 107 ++++++++--- .../code-walkthrough/scripts/test_cw_mcp.py | 23 +++ .../code-walkthrough/scripts/test_cw_run.py | 175 ++++++++++++++++-- 6 files changed, 295 insertions(+), 46 deletions(-) diff --git a/skills/code-walkthrough/assets/diff-review-template.html b/skills/code-walkthrough/assets/diff-review-template.html index 1bd2aaf..77a4b5a 100644 --- a/skills/code-walkthrough/assets/diff-review-template.html +++ b/skills/code-walkthrough/assets/diff-review-template.html @@ -5106,6 +5106,8 @@ note.body=String(c.body||''); saveNote(note); made=true; + const old=c.replaces&¬eById(c.replaces); + if(old&&old.origin==='local'&&old.state==='draft') deleteNote(old.id); }); if(made) renderNotes(); if(ids.length&&!dcPosted.has(o.oid)){ diff --git a/skills/code-walkthrough/prompts/agent.md b/skills/code-walkthrough/prompts/agent.md index 8533b4f..1273ad3 100644 --- a/skills/code-walkthrough/prompts/agent.md +++ b/skills/code-walkthrough/prompts/agent.md @@ -19,7 +19,9 @@ http(s) links only. - `reload_walkthrough` shows your edits as a new walkthrough of the cw//agent commit, opened beside this one; it never rebuilds the current page. Call it once, after your last edit. - `post_draft_comments` puts review comments on diff lines as local drafts; nothing reaches GitHub - until the user submits the review. + until the user submits the review. To revise a draft (the user dislikes the wording), pass + `replaces: ""` on the new comment (ids are in your context under drafts); never post a + second draft for the same point. - When the user's message names a slash command (`/check-pr`, `/fix-pr`) or a skill, that is a request to run it, and it overrides "Reply by default". Call the Skill tool for it first and follow it. Anything the skill would post to GitHub goes through the cw tools instead: review comments via diff --git a/skills/code-walkthrough/scripts/cw_mcp.py b/skills/code-walkthrough/scripts/cw_mcp.py index e737168..1f2fec8 100644 --- a/skills/code-walkthrough/scripts/cw_mcp.py +++ b/skills/code-walkthrough/scripts/cw_mcp.py @@ -305,6 +305,8 @@ def rpc(tool, args, *, timeout=300): "keep or remove. Nothing reaches GitHub until the user submits the review. Each comment " "needs a path in the diff and a line (and optional end_line) inside one contiguous run " "of diff lines on that side. suggestion is replacement code for RIGHT-side lines only. " + "To revise a draft, set replaces to the id of that local draft (listed in your context): it is " + "swapped for the new comment, so never post a second draft for the same point. " "One call per turn; any invalid comment rejects the whole call."), "inputSchema": { "type": "object", "required": ["comments"], @@ -318,6 +320,7 @@ def rpc(tool, args, *, timeout=300): "side": {"enum": ["RIGHT", "LEFT"]}, "body": {"type": "string", "maxLength": DRAFT_BODY_CAP}, "suggestion": {"type": "string", "maxLength": DRAFT_BODY_CAP}, + "replaces": {"type": "string"}, }, }, }}, @@ -476,16 +479,33 @@ def _check_draft_comments(anchor_doc, args, already): ranges = notes.postable_ranges((Path(anchor_doc["dir"]) / "raw.diff").read_text(errors="replace")) except (KeyError, OSError): return "the diff is unavailable, so no comment can be drafted" + drafts = _local_draft_ids(anchor_doc["dir"]) + used = set() for i, item in enumerate(items): - error = _draft_item_error(item, ranges) + error = _draft_item_error(item, ranges, drafts) if error: return f"comments[{i}]: {error}" + replaces = item.get("replaces") + if replaces is not None: + if replaces in used: + return f"comments[{i}]: replaces {replaces} is already replaced by an earlier comment in this call" + used.add(replaces) if "draft_comments" in already: return "already posted draft comments in this turn" return None -def _draft_item_error(item, ranges): +def _local_draft_ids(d): + ids = set() + for name in ("state.json", "page-notes.json"): + doc = cw_store.read_json(Path(d) / name, default=None) + for n in doc.get("notes", []) if isinstance(doc, dict) else []: + if isinstance(n, dict) and n.get("origin") == "local" and n.get("state") == "draft": + ids.add(n.get("id")) + return ids + + +def _draft_item_error(item, ranges, drafts): if not isinstance(item, dict): return "must be an object" path, line, end_line = item.get("path"), item.get("line"), item.get("end_line") @@ -509,6 +529,10 @@ def _draft_item_error(item, ranges): return "body is required" if len(body.strip()) > DRAFT_BODY_CAP: return f"body must be at most {DRAFT_BODY_CAP} characters" + replaces = item.get("replaces") + if replaces is not None and (not isinstance(replaces, str) or not notes.VALID_ID_RE.fullmatch(replaces) + or replaces not in drafts): + return "replaces must be the id of an existing local draft (only drafts that already exist can be replaced, not ones created in this call)" suggestion = item.get("suggestion") if suggestion is not None: if not isinstance(suggestion, str) or len(suggestion) > DRAFT_BODY_CAP: @@ -530,6 +554,8 @@ def _draft_comments_payload(args, anchor_doc=None): row = {"path": item["path"], "line": item["line"], "end_line": end_line if end_line != item["line"] else None, "side": item.get("side", "RIGHT"), "body": body} + if item.get("replaces"): + row["replaces"] = item["replaces"] key = json.dumps(row, sort_keys=True) if key not in seen: seen.add(key) diff --git a/skills/code-walkthrough/scripts/cw_run.py b/skills/code-walkthrough/scripts/cw_run.py index 3199227..2bb83f4 100644 --- a/skills/code-walkthrough/scripts/cw_run.py +++ b/skills/code-walkthrough/scripts/cw_run.py @@ -22,6 +22,7 @@ import threading import time from collections import defaultdict +from concurrent.futures import ThreadPoolExecutor from pathlib import Path sys.path.insert(0, str(Path(__file__).parent)) @@ -979,45 +980,95 @@ def _run_visuals(d, meta, config, on_event, force=False): _emit_step(d, on_event, "visuals", "running") system_prompt = (SKILL_DIR / "prompts" / "visuals.md").read_text() + DELIVERY_SUFFIX.format( tool="submit_visuals") - messages = [{"role": "system", "content": system_prompt}, - {"role": "user", "content": _visuals_user_text(d, meta, analysis, symdelta, need_calls)}] tools, handlers = read_tools(d) - state: dict = {"attempt": 0} ctx = visuals.diff_ctx((d / "raw.diff").read_text(errors="replace"), symdelta) + on_usage = _make_on_usage(d, role, on_event, config) - def handler(args): - state["attempt"] += 1 - clean, problems = visuals.check_visuals(args, n_groups, ctx) - calls = [] - if need_calls: - calls, call_problems = visuals.check_calls(args.get("calls") if isinstance(args, dict) else None, ctx) - problems = problems + call_problems - if problems and state["attempt"] < 3: - return "gate: " + "\n".join(problems) + "\nfix and call submit_visuals again" - state["clean"], state["problems"], state["calls"] = clean, problems, calls - return cw_llm.Done({"ok": True}) + def author(*job): + """Never raises: a failed call is an empty result plus a problem, so the others still land.""" + try: + return _author(*job) + except Exception as e: + return {"pr": None, "overview": None, "groups": {}}, [f"{job[0]}: {e}"], [] + + def _author(name, scope, group=None): + """One scoped submit_visuals conversation -> (clean, problems, calls).""" + messages = [{"role": "system", "content": system_prompt}, + {"role": "user", "content": _visuals_user_text( + d, meta, analysis, symdelta, need_calls and group is None) + "\n\n" + scope}] + state: dict = {"attempt": 0} + + def handler(args): + state["attempt"] += 1 + args = args if isinstance(args, dict) else {} + if group is None: + scoped = {**args, "groups": []} + else: + gs = args.get("groups") + scoped = {"groups": [g for g in gs if isinstance(g, dict) and g.get("index") == group] + if isinstance(gs, list) else []} + clean, problems = visuals.check_visuals(scoped, n_groups, ctx) + if group is not None and group not in clean["groups"] and not problems: + problems = [f"visuals.groups: no picture for group {group}"] + calls = [] + if group is None and need_calls: + calls, call_problems = visuals.check_calls(args.get("calls"), ctx) + problems = problems + call_problems + if problems and state["attempt"] < 3: + return "gate: " + "\n".join(problems) + "\nfix and call submit_visuals again" + state["clean"], state["problems"], state["calls"] = clean, problems, calls + return cw_llm.Done({"ok": True}) - local_handlers = dict(handlers) - local_handlers["submit_visuals"] = handler - try: - cw_llm.run_tools( - profile, messages, tools + [_visuals_submit_tool(need_calls)], local_handlers, max_rounds=8, - max_tokens=config["max_conversation_tokens"], timeout=config["timeout_s"], - on_usage=_make_on_usage(d, role, on_event, config), cwd=d / "head") - finally: - _write_transcript(d, "visuals", profile, messages, result="clean" in state) - clean = state["clean"] - if state["calls"]: - _write_model_symdelta(d, meta, symdelta, state["calls"]) + local_handlers = dict(handlers) + local_handlers["submit_visuals"] = handler + error = None + try: + cw_llm.run_tools( + profile, messages, tools + [_visuals_submit_tool(need_calls and group is None)], + local_handlers, max_rounds=8, max_tokens=config["max_conversation_tokens"], + timeout=config["timeout_s"], on_usage=on_usage, cwd=d / "head") + except Exception as e: + error = f"{name}: {e}" + finally: + _write_transcript(d, f"visuals-{name}", profile, messages, result="clean" in state, error=error) + if "clean" not in state: + return {"pr": None, "overview": None, "groups": {}}, [error or f"{name}: nothing submitted"], [] + return state["clean"], state["problems"] + ([error] if error else []), state["calls"] + + jobs = [] + if visuals.symbol_digest(symdelta) or need_calls: + jobs.append(("overview", "Author only the overview" + (" (and `calls`)." if need_calls else "."), None)) + jobs += [(f"group-{i}", f"Author only group {i}: submit one entry in `groups` with index {i}, " + "and no overview.", i) for i in range(min(n_groups, visuals.MAX_GROUPS))] + with ThreadPoolExecutor(max_workers=4) as pool: + results = list(pool.map(lambda j: author(*j), jobs)) + clean = {"pr": None, "overview": None, "groups": {}} + problems, calls = [], [] + if n_groups > visuals.MAX_GROUPS: + problems.append(f"visuals.groups: at most {visuals.MAX_GROUPS} pictures, " + f"{n_groups - visuals.MAX_GROUPS} dropped") + for c, probs, cs in results: + clean["pr"] = clean["pr"] or c["pr"] + clean["overview"] = clean["overview"] or c["overview"] + clean["groups"].update(c["groups"]) + problems += probs + calls += cs + size = {i: len(h.encode("utf-8", "replace")) for i, h in clean["groups"].items()} + while sum(size.values()) > visuals.TOTAL_MAX: + big = max(size, key=size.get) + del size[big], clean["groups"][big] + problems.append(f"visuals.groups: picture {big} dropped, over the {visuals.TOTAL_MAX // 1024} KB total") + if calls: + _write_model_symdelta(d, meta, symdelta, calls) else: (d / "symdelta.model.json").unlink(missing_ok=True) if not clean["overview"] and not clean["pr"] and not clean["groups"]: - raise RuntimeError("no usable pictures; dropped: " + "; ".join(state["problems"])[:500]) + raise RuntimeError("no usable pictures; dropped: " + "; ".join(problems)[:500]) cw_store.write_json(out, { "overview": clean["overview"], "pr": clean["pr"], "groups": [{"index": i, "html": h} for i, h in sorted(clean["groups"].items())]}) - _emit_step(d, on_event, "visuals", "ok") + _emit_step(d, on_event, "visuals", "ok", **({"lines": problems} if problems else {})) return True except Exception as e: _emit_step(d, on_event, "visuals", "failed", error=str(e), remedy=getattr(e, "remedy", None)) diff --git a/skills/code-walkthrough/scripts/test_cw_mcp.py b/skills/code-walkthrough/scripts/test_cw_mcp.py index 7883040..d9fa860 100644 --- a/skills/code-walkthrough/scripts/test_cw_mcp.py +++ b/skills/code-walkthrough/scripts/test_cw_mcp.py @@ -556,6 +556,29 @@ def check(*comments, already=()): assert body.endswith(f"{fence}suggestion\n{text}\n{fence}") +def test_check_draft_comments_replaces(): + d = _diff_dir() + doc = {**_page_doc(), "dir": d} + drafts = [{"id": "n-1", "origin": "local", "state": "draft"}, + {"id": "n-2", "origin": "local", "state": "posted"}, + {"id": "n-3", "origin": "github", "state": "draft"}] + (Path(d) / "page-notes.json").write_text(json.dumps({"notes": drafts})) + + def check(replaces): + return cw_mcp.check_outcome(doc, "post_draft_comments", {"comments": [_dc(replaces=replaces)]}, ()) + + assert check("n-1") is None + for bad in ("n-2", "n-3", "n-9", "bad id", 5): + assert "replaces must be" in check(bad), bad + assert "already replaced" in cw_mcp.check_outcome( + doc, "post_draft_comments", {"comments": [_dc(replaces="n-1"), _dc(line=3, replaces="n-1")]}, ()) + (Path(d) / "page-notes.json").unlink() + (Path(d) / "state.json").write_text(json.dumps({"notes": drafts})) + assert check("n-1") is None + row = cw_mcp._draft_comments_payload({"comments": [_dc(replaces="n-1"), _dc(line=3)]})["comments"] + assert row[0]["replaces"] == "n-1" and "replaces" not in row[1] + + def test_outcomes_dispatch_rejects_tools_outside_the_mode(tmp_path): req = {"jsonrpc": "2.0", "id": 1, "method": "tools/call", "params": {"name": "propose_task", "arguments": {}}} called = [] diff --git a/skills/code-walkthrough/scripts/test_cw_run.py b/skills/code-walkthrough/scripts/test_cw_run.py index 5db2506..cf909ad 100644 --- a/skills/code-walkthrough/scripts/test_cw_run.py +++ b/skills/code-walkthrough/scripts/test_cw_run.py @@ -306,18 +306,47 @@ def _submit(args): return cw_testlib.tool_call("submit_visuals", args) +def _scope_of(body): + text = cw_testlib.last_user_text(body) + return "overview" if "Author only the overview" in text else "group-" + text.rsplit("Author only group ", 1)[1].split(":")[0] + + +def _scoped(**replies): + """Scripted replies per visuals call (overview=[...], group_0=[...]); each call walks its own list.""" + seen, lock = {}, threading.Lock() + + def script(body, _n): + scope = _scope_of(body) + with lock: + n = seen[scope] = seen.get(scope, -1) + 1 + entry = replies[scope.replace("-", "_")] + return entry[min(n, len(entry) - 1)] + return script + + +def _requests_for(stub, scope): + return [r for r in stub.requests if _scope_of(r) == scope] + + +def _tool_msgs(req): + return [m["content"] for m in req["messages"] if m.get("role") == "tool"] + + +G0 = _submit({"groups": [{"index": 0, "html": "

g

"}]}) + + def test_visuals_pass_repairs_then_writes_visuals_json(): with cw_testlib.temp_home() as home, tempfile.TemporaryDirectory() as tmp: with cw_testlib.StubLLM({}) as stub: d, meta = _visuals_dir(home, tmp, stub) bad = {"pr": {"title": "T", "html": ""}, "groups": []} - good = {"pr": {"title": "T", "html": GOOD_SVG}, "groups": [{"index": 0, "html": "

g

"}]} - stub.script = {"prose-m": [_submit(bad), _submit(good)]} + good = {"pr": {"title": "T", "html": GOOD_SVG}, "groups": []} + stub.script = _scoped(overview=[_submit(bad), _submit(good)], group_0=[G0]) assert cw_run._run_visuals(d, meta, cw_store.load_config(), None) - gate = [m["content"] for m in stub.requests[1]["messages"] if m.get("role") == "tool"][-1] + gate = _tool_msgs(_requests_for(stub, "overview")[1])[-1] assert gate.startswith("gate:") and " is not allowed" in gate assert gate.endswith("fix and call submit_visuals again") - assert "foo.py:run | foo.py:2-2 | changed" in cw_testlib.last_user_text(stub.requests[0]) + assert "foo.py:run | foo.py:2-2 | changed" in cw_testlib.last_user_text(_requests_for(stub, "overview")[0]) saved = json.loads((d / "visuals.json").read_text()) assert saved["pr"]["title"] == "T" and saved["groups"] == [{"index": 0, "html": "

g

"}] assert cw_store.read_meta(d)["steps"]["visuals"]["status"] == "ok" @@ -344,13 +373,13 @@ def test_visuals_pass_returns_overview_problems_and_writes_overview(): d, meta = _visuals_dir(home, tmp, stub) bad = {"overview": _overview("bar.py:not_there"), "groups": []} good = {"overview": _overview("foo.py:X"), "groups": []} - stub.script = {"prose-m": [_submit(bad), _submit(good)]} + stub.script = _scoped(overview=[_submit(bad), _submit(good)], group_0=[G0]) assert cw_run._run_visuals(d, meta, cw_store.load_config(), None) - gate = [m["content"] for m in stub.requests[1]["messages"] if m.get("role") == "tool"][-1] + gate = _tool_msgs(_requests_for(stub, "overview")[1])[-1] assert gate.startswith("gate:") and "overview.nodes[1].refs[0]: bar.py:not_there not in diff" in gate saved = json.loads((d / "visuals.json").read_text()) assert saved["overview"]["title"] == "Foo changes" and len(saved["overview"]["nodes"]) == 2 - assert saved["groups"] == [] and saved["pr"] is None + assert saved["groups"] == [{"index": 0, "html": "

g

"}] and saved["pr"] is None def test_visuals_pass_prefers_the_visuals_role_over_prose(): @@ -361,7 +390,7 @@ def test_visuals_pass_prefers_the_visuals_role_over_prose(): {"analysis": "p", "prose": "p", "visuals": "v"}) stub.script = {"vis-m": [_submit({"pr": None, "groups": [{"index": 0, "html": "

n

"}]})]} assert cw_run._run_visuals(d, meta, cw_store.load_config(), None) - assert stub.count("vis-m") == 1 and stub.count("prose-m") == 0 + assert stub.count("vis-m") == 2 and stub.count("prose-m") == 0 def test_visuals_pass_keeps_the_clean_part_on_the_final_attempt(): @@ -374,7 +403,7 @@ def test_visuals_pass_keeps_the_clean_part_on_the_final_attempt(): assert cw_run._run_visuals(d, meta, cw_store.load_config(), None) saved = json.loads((d / "visuals.json").read_text()) assert saved["pr"] is None and saved["groups"][0]["index"] == 0 - assert stub.count("prose-m") == 3 + assert stub.count("prose-m") == 4 def test_visuals_pass_fails_when_nothing_clean_survives_the_final_attempt(): @@ -412,6 +441,67 @@ def test_visuals_pass_runs_without_symbols_when_there_are_groups(): assert "0 | Core | the change | foo.py" in cw_testlib.last_user_text(stub.requests[0]) +def _two_group_dir(home, tmp, stub): + d, meta = _visuals_dir(home, tmp, stub) + analysis = json.loads((d / "analysis.json").read_text()) + analysis["groups"].append({"title": "Edge", "why": "w", "paths": ["foo.py"]}) + (d / "analysis.json").write_text(json.dumps(analysis)) + return d, meta + + +def _group(i): + return _submit({"groups": [{"index": i, "html": f"

g{i}

"}]}) + + +def test_visuals_groups_are_authored_in_parallel_calls_and_merged(): + with cw_testlib.temp_home() as home, tempfile.TemporaryDirectory() as tmp: + with cw_testlib.StubLLM({}, delay=0.2) as stub: + d, meta = _two_group_dir(home, tmp, stub) + stub.script = _scoped(overview=[_submit({"overview": _overview()})], + group_0=[_group(0)], group_1=[_group(1)]) + assert cw_run._run_visuals(d, meta, cw_store.load_config(), None) + assert stub.peak_in_flight == 3 + saved = json.loads((d / "visuals.json").read_text()) + assert set(saved) == {"overview", "pr", "groups"} and saved["overview"]["title"] == "Foo changes" + assert saved["groups"] == [{"index": 0, "html": "

g0

"}, {"index": 1, "html": "

g1

"}] + assert {p.name for p in (d / "runs").glob("visuals-*")} == { + "visuals-overview-1.json", "visuals-group-0-1.json", "visuals-group-1-1.json"} + assert cw_store.read_meta(d)["usage"]["prose"]["calls"] == 3 + + +def test_a_failing_group_is_dropped_and_the_rest_still_written(): + with cw_testlib.temp_home() as home, tempfile.TemporaryDirectory() as tmp: + with cw_testlib.StubLLM({}) as stub: + d, meta = _two_group_dir(home, tmp, stub) + stub.script = _scoped(overview=[_submit({"overview": _overview()})], group_0=[_group(0)], + group_1=[_submit({"groups": [{"index": 1, "html": ""}]})]) + assert cw_run._run_visuals(d, meta, cw_store.load_config(), None) + saved = json.loads((d / "visuals.json").read_text()) + assert saved["groups"] == [{"index": 0, "html": "

g0

"}] and saved["overview"] + assert cw_store.read_meta(d)["steps"]["visuals"]["status"] == "ok" + + +def test_a_group_call_that_errors_does_not_fail_the_others(): + with cw_testlib.temp_home() as home, tempfile.TemporaryDirectory() as tmp: + with cw_testlib.StubLLM({}) as stub: + d, meta = _two_group_dir(home, tmp, stub) + stub.script = _scoped(overview=[_submit({"overview": _overview()})], group_0=[_group(0)], + group_1=[cw_testlib.http_error(400, "nope")]) + assert cw_run._run_visuals(d, meta, cw_store.load_config(), None) + assert [g["index"] for g in json.loads((d / "visuals.json").read_text())["groups"]] == [0] + assert "nope" in json.loads((d / "runs" / "visuals-group-1-1.json").read_text())["error"] + + +def test_visuals_group_call_must_submit_its_own_index(): + with cw_testlib.temp_home() as home, tempfile.TemporaryDirectory() as tmp: + with cw_testlib.StubLLM({}) as stub: + d, meta = _visuals_dir(home, tmp, stub, symbols=False) + stub.script = _scoped(group_0=[_group(5), _group(0)]) + assert cw_run._run_visuals(d, meta, cw_store.load_config(), None) + assert "no picture for group 0" in _tool_msgs(_requests_for(stub, "group-0")[1])[-1] + assert json.loads((d / "visuals.json").read_text())["groups"][0]["html"] == "

g0

" + + def test_visuals_llm_error_marks_the_step_failed_and_never_raises(): with cw_testlib.temp_home() as home, tempfile.TemporaryDirectory() as tmp: with cw_testlib.StubLLM({}) as stub: @@ -736,7 +826,7 @@ def _calls_run(symdelta, groups=False): bad = {"groups": pic, "calls": [{"from": "foo.py:nope", "to": "foo.py:X", "state": "new"}]} good = {"groups": pic, "calls": [{"from": "foo.py:X", "to": "foo.py:b", "state": "new"}, {"from": "foo.py:b", "to": "foo.py:X", "state": "gone"}]} - stub.script = {"prose-m": [_submit(bad), _submit(good)]} + stub.script = _scoped(overview=[_submit(bad), _submit(good)], group_0=[_submit({"groups": pic})]) ran = cw_run._run_visuals(d, meta, cw_store.load_config(), None) model = d / "symdelta.model.json" return (model.read_text() if model.exists() else None), stub, ran @@ -745,10 +835,11 @@ def _calls_run(symdelta, groups=False): def test_null_language_asks_for_calls_and_writes_model_symdelta(): model, stub, ran = _calls_run(NULL_SYMDELTA) assert ran - props = stub.requests[0]["tools"][-1]["function"]["parameters"]["properties"] - assert "calls" in props - assert "also submit `calls`" in cw_testlib.last_user_text(stub.requests[0]) - gate = [m["content"] for m in stub.requests[1]["messages"] if m.get("role") == "tool"][-1] + first = _requests_for(stub, "overview")[0] + assert "calls" in first["tools"][-1]["function"]["parameters"]["properties"] + assert "also submit `calls`" in cw_testlib.last_user_text(first) + assert "calls" not in _requests_for(stub, "group-0")[0]["tools"][-1]["function"]["parameters"]["properties"] + gate = _tool_msgs(_requests_for(stub, "overview")[1])[-1] assert "calls[0].from: foo.py:nope not in diff" in gate saved = json.loads(model) assert saved["resolver"] == "model" and saved["language"] == "model" @@ -761,7 +852,7 @@ def test_real_symdelta_never_asks_for_calls_or_writes_model_file(): model, stub, _ran = _calls_run({"language": "python", "nodes": [ {"id": "foo.py:X", "label": "X", "kind": "symbol", "file": "foo.py", "state": "changed"}]}, groups=True) assert "calls" not in stub.requests[0]["tools"][-1]["function"]["parameters"]["properties"] - assert "also submit `calls`" not in cw_testlib.last_user_text(stub.requests[0]) + assert all("also submit `calls`" not in cw_testlib.last_user_text(r) for r in stub.requests) assert model is None @@ -938,3 +1029,57 @@ def test_agent_gh_token_is_the_repo_accounts_token(tmp_path): with cw_testlib.fake_gh(tmp_path, fail_ops=["viewer"]): assert cw_run.agent_gh_token({"repo": str(tmp_path)}) is None assert cw_run.agent_gh_token({}) is None + + +def _two_groups(d): + a = json.loads((d / "analysis.json").read_text()) + a["groups"] = [{"title": f"G{i}", "why": "w", "paths": ["foo.py"]} for i in range(2)] + (d / "analysis.json").write_text(json.dumps(a)) + + +def test_visuals_pass_survives_a_call_that_raises(monkeypatch): + with cw_testlib.temp_home() as home, tempfile.TemporaryDirectory() as tmp: + with cw_testlib.StubLLM({}) as stub: + d, meta = _visuals_dir(home, tmp, stub, symbols=False) + _two_groups(d) + real = cw_run._write_transcript + + def flaky(d_, role, *a, **k): + if role == "visuals-group-0": + raise OSError("disk full") + return real(d_, role, *a, **k) + monkeypatch.setattr(cw_run, "_write_transcript", flaky) + g = lambda i: _submit({"groups": [{"index": i, "html": "

g

"}]}) + stub.script = _scoped(group_0=[g(0)], group_1=[g(1)]) + assert cw_run._run_visuals(d, meta, cw_store.load_config(), None) + saved = json.loads((d / "visuals.json").read_text()) + assert [x["index"] for x in saved["groups"]] == [1] + assert any("disk full" in x for x in cw_store.read_meta(d)["steps"]["visuals"]["lines"]) + + +def test_visuals_pass_reapplies_the_total_cap_across_groups(monkeypatch): + with cw_testlib.temp_home() as home, tempfile.TemporaryDirectory() as tmp: + with cw_testlib.StubLLM({}) as stub: + d, meta = _visuals_dir(home, tmp, stub, symbols=False) + _two_groups(d) + monkeypatch.setattr(cw_run.visuals, "TOTAL_MAX", 12) + g = lambda i: _submit({"groups": [{"index": i, "html": "

g

"}]}) + stub.script = _scoped(group_0=[g(0)], group_1=[g(1)]) + assert cw_run._run_visuals(d, meta, cw_store.load_config(), None) + saved = json.loads((d / "visuals.json").read_text()) + assert len(saved["groups"]) == 1 + assert any("dropped, over" in x for x in cw_store.read_meta(d)["steps"]["visuals"]["lines"]) + + +def test_visuals_pass_notes_groups_beyond_the_max(): + with cw_testlib.temp_home() as home, tempfile.TemporaryDirectory() as tmp: + with cw_testlib.StubLLM({}) as stub: + d, meta = _visuals_dir(home, tmp, stub, symbols=False) + n = cw_run.visuals.MAX_GROUPS + 2 + a = json.loads((d / "analysis.json").read_text()) + a["groups"] = [{"title": f"G{i}", "why": "w", "paths": ["foo.py"]} for i in range(n)] + (d / "analysis.json").write_text(json.dumps(a)) + stub.script = _scoped(**{f"group_{i}": [_submit({"groups": [{"index": i, "html": "

g

"}]})] + for i in range(n)}) + assert cw_run._run_visuals(d, meta, cw_store.load_config(), None) + assert any("2 dropped" in x for x in cw_store.read_meta(d)["steps"]["visuals"]["lines"]) From 09985f9b014cb9ea9a2e22564d369e7217abd25e Mon Sep 17 00:00:00 2001 From: crisperity Date: Fri, 30 Oct 2026 09:37:33 +0100 Subject: [PATCH 2/2] chore: bump marketplace plugin to 1.3.0 --- .claude-plugin/marketplace.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 073beab..8aa6886 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.2.1", + "version": "1.3.0", "source": "./", "author": { "name": "crisperity"