mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-25 21:34:47 -06:00
fix(coord): rename ws_id->child_ws_id in spawn return JSON
Coordinator LLMs on large fan-outs recency-bias on seeing `ws_id` in a `spawn_workstream` / `spawn_batch` return -- calling `spawn_workstream(ws_id=...)` again instead of progressing to `wait_for_workstream(ws_ids=[...])`. On 10+ child fan-outs this cascades into self-inflicted re-spawn loops. Rename to `child_ws_id` (already an existing project term -- see `tasks` tool, `child_event_bus.py`) defuses the recency bias. Scope is the LLM-facing JSON only -- the server HTTP API at the spawn endpoint still returns `ws_id`, and the internal reads of that HTTP response are unchanged. Also updates the two tool descriptions, the operator-facing skill doc, and the bulk-endpoints example so docs don't undo the rename.
This commit is contained in:
@@ -113,8 +113,8 @@ owns it; the node is just currently unreachable.
|
||||
```json
|
||||
{
|
||||
"results": {
|
||||
"0": {"ws_id": "d4e5f6...", "name": "csrf-audit", "node_id": "gpu-3"},
|
||||
"2": {"ws_id": "f1a2b3...", "name": "xss-audit", "node_id": "gpu-1"}
|
||||
"0": {"child_ws_id": "d4e5f6...", "name": "csrf-audit", "node_id": "gpu-3"},
|
||||
"2": {"child_ws_id": "f1a2b3...", "name": "xss-audit", "node_id": "gpu-1"}
|
||||
},
|
||||
"denied": [
|
||||
{"idx": 1, "reason": "skill not found: nonexistent-skill"}
|
||||
|
||||
@@ -169,11 +169,15 @@ validates ws_id against `parent_ws_id=coord_ws_id` AND
|
||||
the wait into reporting "complete".
|
||||
|
||||
Pattern: capture each spawn result in the next tool call's input.
|
||||
The JSON tool-result carries `{"ws_id": "...", "name": "...",
|
||||
The JSON tool-result carries `{"child_ws_id": "...", "name": "...",
|
||||
"node_id": "...", "routing_strategy": "..."}`; the model should
|
||||
extract the ws_id and pass it to `inspect_workstream` /
|
||||
`wait_for_workstream` / `send_to_workstream` / `close_workstream`
|
||||
verbatim.
|
||||
extract the `child_ws_id` and pass it as `ws_id` (or in the `ws_ids`
|
||||
list) to `inspect_workstream` / `wait_for_workstream` /
|
||||
`send_to_workstream` / `close_workstream` verbatim. The asymmetry
|
||||
— spawn returns `child_ws_id` but the other tools accept `ws_id` /
|
||||
`ws_ids` — is intentional: it defuses a coordinator-LLM recency
|
||||
bias where seeing `ws_id` in a spawn return primed re-spawn loops
|
||||
instead of progression to the wait phase.
|
||||
|
||||
A UI that wants human-readable identifiers should render the `name`
|
||||
field and keep the ws_id as the click-through key.
|
||||
|
||||
@@ -215,7 +215,12 @@ def test_spawn_exec_does_not_surface_misleading_status_field(coord_session):
|
||||
summary tempted callers to write ``if result["status"] == "idle"``
|
||||
which silently never matched. The summary now omits the field
|
||||
entirely; lifecycle state lives on the workstream row and is read
|
||||
via inspect_workstream."""
|
||||
via inspect_workstream.
|
||||
|
||||
Also asserts the return key is ``child_ws_id`` (not ``ws_id``) so
|
||||
the coordinator LLM doesn't recency-bias toward feeding the spawn
|
||||
output back into another ``spawn_workstream(ws_id=...)`` call.
|
||||
"""
|
||||
sess, coord, _ui = coord_session
|
||||
coord.spawn.return_value = {
|
||||
"ws_id": "child-7",
|
||||
@@ -227,8 +232,9 @@ def test_spawn_exec_does_not_surface_misleading_status_field(coord_session):
|
||||
_call_id, output = sess._exec_spawn_workstream(item)
|
||||
body = json.loads(output)
|
||||
assert "status" not in body
|
||||
assert "ws_id" not in body
|
||||
# The substantive fields are still here.
|
||||
assert body["ws_id"] == "child-7"
|
||||
assert body["child_ws_id"] == "child-7"
|
||||
assert body["node_id"] == "node-1"
|
||||
|
||||
|
||||
@@ -248,6 +254,10 @@ def test_spawn_batch_exec_does_not_surface_misleading_status_field(coord_session
|
||||
body = json.loads(output)
|
||||
assert "0" in body["results"]
|
||||
assert "status" not in body["results"]["0"]
|
||||
# Per-result entries surface ``child_ws_id``, not ``ws_id`` — same
|
||||
# recency-bias rationale as the spawn_workstream test above.
|
||||
assert body["results"]["0"]["child_ws_id"] == "c-x"
|
||||
assert "ws_id" not in body["results"]["0"]
|
||||
|
||||
|
||||
def test_spawn_exec_surfaces_client_error(coord_session):
|
||||
@@ -1410,9 +1420,13 @@ def test_spawn_batch_exec_serialises_spawns_and_returns_results(coord_session):
|
||||
assert body["denied"] == []
|
||||
# Keyed by input index (stringified).
|
||||
assert set(body["results"].keys()) == {"0", "1", "2"}
|
||||
assert body["results"]["0"]["ws_id"] == "child-0"
|
||||
assert body["results"]["0"]["child_ws_id"] == "child-0"
|
||||
assert body["results"]["1"]["node_id"] == "n-1"
|
||||
assert body["results"]["2"]["ws_id"] == "child-2"
|
||||
assert body["results"]["2"]["child_ws_id"] == "child-2"
|
||||
# Confirm we don't leak the old ``ws_id`` key alongside the new
|
||||
# ``child_ws_id`` — see test_spawn_exec_does_not_surface_misleading_status_field
|
||||
# for the rationale on the rename.
|
||||
assert "ws_id" not in body["results"]["0"]
|
||||
|
||||
|
||||
def test_spawn_batch_exec_surfaces_per_item_errors_in_denied(coord_session):
|
||||
|
||||
@@ -7005,9 +7005,9 @@ class ChatSession:
|
||||
msg = f"Error: {result['error']}"
|
||||
self._report_tool_result(call_id, "spawn_workstream", msg, is_error=True)
|
||||
return call_id, msg
|
||||
# Successful spawn — surface ws_id + node_id + name + routing
|
||||
# strategy so the coordinator can follow up with inspect / send
|
||||
# and explain why a given node was chosen. ``status`` was
|
||||
# Successful spawn — surface child_ws_id + node_id + name +
|
||||
# routing strategy so the coordinator can follow up with inspect
|
||||
# / send and explain why a given node was chosen. ``status`` was
|
||||
# historically included but it was the routing-proxy's HTTP
|
||||
# code (always 200 on this branch); the absence of an
|
||||
# ``error`` field is the success signal. Dropped here to
|
||||
@@ -7018,7 +7018,12 @@ class ChatSession:
|
||||
# ``inspect_workstream``.
|
||||
summary = json.dumps(
|
||||
{
|
||||
"ws_id": result.get("ws_id"),
|
||||
# Key is ``child_ws_id`` (not ``ws_id``) so the coordinator
|
||||
# LLM doesn't recency-bias toward feeding the spawn-return
|
||||
# straight back into another ``spawn_workstream(ws_id=...)``
|
||||
# call. On large fan-outs this cascaded into self-inflicted
|
||||
# re-spawn loops instead of progressing to ``wait_for_workstream``.
|
||||
"child_ws_id": result.get("ws_id"),
|
||||
"name": result.get("name"),
|
||||
"node_id": result.get("node_id"),
|
||||
"routing_strategy": result.get("routing_strategy"),
|
||||
@@ -7156,7 +7161,9 @@ class ChatSession:
|
||||
denied.append({"idx": idx, "reason": "spawn returned no ws_id"})
|
||||
continue
|
||||
results[str(idx)] = {
|
||||
"ws_id": ws_id,
|
||||
# ``child_ws_id`` (not ``ws_id``) — see the matching
|
||||
# comment in ``_exec_spawn_workstream``.
|
||||
"child_ws_id": ws_id,
|
||||
"name": result.get("name", ""),
|
||||
"node_id": result.get("node_id", ""),
|
||||
# ``status`` deliberately omitted — see the matching
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "spawn_batch",
|
||||
"description": "Create up to 10 child workstreams in one call. Serialised in input order so sibling ordering (by `created_at`) is deterministic. Returns `{results: {idx: {ws_id, name, node_id}}, denied: [{idx, reason}]}` — `results` keyed by stringified input-array index, `denied` collects per-item validation / spawn failures. For >10 children make multiple calls (the batch hard-errors rather than truncating). Pair with `wait_for_workstream(ws_ids=[...], mode='all')` to synthesise the N outputs once every child has finished. Lifecycle state at spawn isn't returned — call inspect_workstream if you need it.",
|
||||
"description": "Create up to 10 child workstreams in one call. Serialised in input order so sibling ordering (by `created_at`) is deterministic. Returns `{results: {idx: {child_ws_id, name, node_id}}, denied: [{idx, reason}]}` — `results` keyed by stringified input-array index, `denied` collects per-item validation / spawn failures. Collect the `child_ws_id` values and pass them as a list to `wait_for_workstream(ws_ids=[...], mode='all')` to synthesise the N outputs once every child has finished. For >10 children make multiple calls (the batch hard-errors rather than truncating). Lifecycle state at spawn isn't returned — call inspect_workstream if you need it.",
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "spawn_workstream",
|
||||
"description": "Create a new child workstream, optionally dispatching an initial message. Kick off a focused sub-task on a different skill / model / node while the coordinator stays in charge. The child runs independently — drive it with send_to_workstream / inspect_workstream / close_workstream. Returns `{ws_id, name, node_id, routing_strategy}`. `routing_strategy` is `rendezvous` (default placement on the live-node set), `target_node` (your hint was honored), or `resume` (rebound to a still-alive prior owner on rehydrate). The returned `node_id` is the spawn-time binding; subsequent ops re-route via rendezvous over the live-node set, so a node join or drop after spawn can shift the active owner. Conversation state lives in storage (the new owner rehydrates lazily). Don't cache `node_id` for long-running callbacks — re-read with inspect_workstream. Lifecycle state (idle / running / etc.) is not in this response — read it via inspect_workstream.",
|
||||
"description": "Create a new child workstream, optionally dispatching an initial message. Kick off a focused sub-task on a different skill / model / node while the coordinator stays in charge. The child runs independently — drive it with send_to_workstream / inspect_workstream / close_workstream. Returns `{child_ws_id, name, node_id, routing_strategy}` — pass `child_ws_id` into `wait_for_workstream(ws_ids=[...])` and the other `ws_id`-taking tools. `routing_strategy` is `rendezvous` (default placement on the live-node set), `target_node` (your hint was honored), or `resume` (rebound to a still-alive prior owner on rehydrate). The returned `node_id` is the spawn-time binding; subsequent ops re-route via rendezvous over the live-node set, so a node join or drop after spawn can shift the active owner. Conversation state lives in storage (the new owner rehydrates lazily). Don't cache `node_id` for long-running callbacks — re-read with inspect_workstream. Lifecycle state (idle / running / etc.) is not in this response — read it via inspect_workstream.",
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
|
||||
Reference in New Issue
Block a user