Skip to content

A routing typo waited for a run to fail, and a reused run id merged two of them - #86

Merged
Shashankss1205 merged 1 commit into
mainfrom
fix/routing-and-run-id
Aug 4, 2026
Merged

A routing typo waited for a run to fail, and a reused run id merged two of them#86
Shashankss1205 merged 1 commit into
mainfrom
fix/routing-and-run-id

Conversation

@Shashankss1205

Copy link
Copy Markdown
Collaborator

Two declaration-time gaps of the same shape — something knowable before a run started was left for the run to discover, or not to. Different files (runtime kernel vs CLI), one commit.

Fixes #4
Fixes #29

#4 — a conditional edge's mapping is checked where it is declared

add_conditional_edge passed the router and its mapping to LangGraph untouched. A mapping target naming a node nobody added was accepted, an empty mapping was accepted, and the first branch to take one died on self.ends[key] — a bare KeyError from inside LangGraph's _branch.py, naming neither the graph, the source node, nor the router that produced the key.

What is checked now, at add_conditional_edge:

  1. Every mapping target must name a node this graph has, or END. All offending pairs are named at once ('go' -> 'sipn'), alongside the valid destinations.
  2. An empty mapping is refused — a branch with nowhere to go.
  3. A router that declares what it returns — a Literal[...] or an Enum return annotation — has those members held against the mapping's keys. The membership test is the same hash lookup LangGraph will perform, so the check predicts the run-time failure rather than approximating it (this matters for Enum, which hashes by member name while StrEnum compares by value).
  4. A router that declares nothing is left alone. Per the issue: predicting an arbitrary function's return value is not a check, and an invented requirement would be worse than the gap.

Beyond the issue's letter, and asked for in the task: (4) is no longer a bare KeyError. The router is wrapped so an unmapped key raises GraphRoutingError naming the node, the key and the keys that were declared — the error the kernel already raises for every other transition it cannot make. The wrapper uses functools.wraps and a *args/**kwargs signature deliberately: LangGraph names the branch after the callable's __name__ and infers the branch's input schema from its annotations, and it passes config to a router that asks for one. An async def router gets an async wrapper, or LangGraph would await the check's return value rather than the router's. A Send returned by a router still passes through to LangGraph's own dispatch untouched, as does a list of keys.

Error shape and ordering are unchanged elsewhere: dag=True still refuses a conditional edge before the mapping is looked at, so the cookbook's GraphCycleError example is byte-identical.

Before

>>> g.add_conditional_edge("a", lambda s: "go", {"go": "typo_node"})   # accepted
>>> g.add_conditional_edge("a", lambda s: "go", {})                    # accepted
>>> g.compile().invoke({"n": 0})
KeyError: 'stpo'

After

GraphRoutingError: the conditional edge on node 'a' maps to a destination graph 't' does not have:
'go' -> 'typo_node'. LangGraph resolves a branch target when a run reaches it, so this would surface
mid-run rather than here. Valid destinations: 'a', END

GraphRoutingError: the conditional edge on node 'a' was given an empty mapping, so there is no key
its router could return that leads anywhere; ...

GraphRoutingError: the conditional edge on node 'a' has a router declaring it returns 'again',
'stop', but 'stop' is not a key of its mapping; ...

GraphRoutingError: the router on node 'a' returned 'stpo', which is not a key of the mapping the
edge was declared with; ... Keys: 'stop', 'again'

#29 — a reused --run-id is refused, exit 2

Every executing command appends to its --trace (TraceRecorder opens "a"), which is right — grapharc diff reads two runs out of one file. Nothing checked whether the id the operator passed was already in that file, so two runs merged under one name: metrics summed both runs' tokens and node counts, viz welded the second path onto the end of the first, replay reconstructed a chimera, with no signal at any point.

The issue left the remedy shape open on one axis, so, as directed: fail closed rather than auto-renaming or warning. A run id is the name an operator will look the run up under later; silently picking a different one is the same class of surprise as silently merging, and a warning does not stop the file from being corrupted. plan, run and agent (both the sandboxed and the claude-cli executor) refuse an explicit --run-id that already has events in the target trace, with exit 2 naming the id, the event count and the file, before a single event is written — ahead of run's admission check, which writes the first event, so --check-only is guarded too.

Out of scope per the issue and untouched: repairing already-interleaved files, and the multi-run-per-file pattern diff depends on. demo has no --run-id flag (it derives live-<example>), so nothing to guard there.

  • Only an explicit id is checked; a generated one is fresh by construction and pays for no file scan.
  • The scan is a new grapharc/cli/runid.py: it reads one field per line rather than validating whole event models, and skips a line it cannot parse. The guard's job is to spot a collision, not to be the file's validator — the readers still report a torn trace in their own words.
  • --run-id's help text on all three commands now says it is refused if --trace already holds it.

Before

$ grapharc plan "goal one" --trace t.jsonl --run-id r1   # exit 0
$ grapharc plan "goal two" --trace t.jsonl --run-id r1   # exit 0, silent
$ grapharc metrics t.jsonl r1
nodes_executed: 6      # two runs
tokens: 2306
events: 30

After

$ grapharc plan "goal two" --trace t.jsonl --run-id r1
error: run id 'r1' already has 15 events in t.jsonl; pick a new --run-id or a new --trace file.
Appending a second run under one id merges the two, and metrics, replay and viz would then report
the blend as one run
$ echo $?
2
$ grapharc metrics t.jsonl r1
nodes_executed: 3      # still one run
tokens: 1153
events: 15

A different id in the same file, and a generated id, both still exit 0.

Tests

Each new test was confirmed red before its fix (by stashing the source change and re-running):

  • tests/test_runtime_discipline.py — 11 new: unknown target (single and several at once), empty mapping, Literal and Enum declarations checked, a correct mapping and an undeclared router unaffected, an unmapped return raising GraphRoutingError, an unmapped key inside a returned list, and the branch name surviving the wrapper. 7 fail without the fix.
  • tests/test_async_kernel.py — 2 new: an async def router's unmapped return, and one that maps.
  • tests/test_cli.py — 7 new: plan refused on reuse (text and JSON), run refused before the admission event, agent refused before the model is built, different ids in one file still supported, generated ids never guarded, and a unit test of the scan over a file with a torn line. 4 fail without the fix.

Full suite green, ruff check grapharc tests clean.

Docs

The README paragraph naming this limitation ("a typo surfaces as a KeyError at run time") and both cookbook sentences in docs/cookbook/01-basics.md now describe what is checked and what still is not. CHANGELOG.md gets an entry per issue under the existing ## Unreleased.

🤖 Generated with Claude Code

…wo of them

Two declaration-time gaps, one in the kernel and one in the CLI, both of the
same shape: something knowable before a run started was left for the run to
discover, or not to.

`add_conditional_edge` handed the router and its mapping to LangGraph
untouched, so a mapping pointing at a node nobody added was accepted and the
first branch to take it died on `self.ends[key]` — a bare `KeyError` from
inside LangGraph's branch machinery, naming neither the graph, the source node
nor the router. The mapping is topology and was checkable all along: an empty
mapping is refused now, every unreachable target is named alongside the key
that leads to it, and a router annotated with what it returns (a `Literal`, an
`Enum`) has those members held against the mapping's keys with the same hash
lookup LangGraph will use. A router that annotates nothing is still not
second-guessed — but the key it returns is checked when it returns one, and
raises `GraphRoutingError` naming the node, the key and the keys declared. The
wrapper keeps the router's name and annotations: LangGraph branches by the one
and infers the branch's input schema from the other.

Every executing command appends to its `--trace`, which is right — `diff`
reads two runs out of one file — but nothing checked whether the `--run-id`
the operator passed was already in there. Two runs then merged under one name,
and `metrics` summed both runs' tokens, `viz` welded the second path onto the
first, `replay` reconstructed a chimera, with no signal at any point. The
appendable file was never the defect; the reused id was. `plan`, `run` and
`agent` (both executors) refuse an explicit `--run-id` that already has events
in the target trace, exit 2, before a single event is written. Fail closed
rather than auto-rename: the id is the name an operator looks the run up under
later. Generated ids pay for no scan, and different ids in one file are
untouched.

Fixes #4
Fixes #29

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Shashankss1205
Shashankss1205 merged commit 6b115a2 into main Aug 4, 2026
6 checks passed
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.

cli: a reused --run-id silently interleaves two runs in one trace runtime: validate conditional-edge mappings when the edge is added, not mid-run

1 participant