langgraph-code-review

$npx mdskill add existential-birds/beagle/langgraph-code-review

Reviews LangGraph code for bugs, anti-patterns, and improvements.

  • Catches common mistakes in state management and graph structure.
  • Depends on LangGraph features like StateGraph, nodes, edges, and checkpointing.
  • Checks code against anti-confabulation rules and review gates.
  • Outputs findings with file:line and quoted code snippets.

SKILL.md

.github/skills/langgraph-code-reviewView on GitHub ↗
---
name: langgraph-code-review
description: Reviews LangGraph code for bugs, anti-patterns, and improvements. Use when reviewing code that uses StateGraph, nodes, edges, checkpointing, or other LangGraph features. Catches common mistakes in state management, graph structure, and async patterns.
---

# LangGraph Code Review

When reviewing LangGraph code, check for these categories of issues.

## Anti-confabulation (gate 0 — runs before every other gate)

Before issuing **any** finding — flag a bug, anti-pattern, or improvement — you MUST echo the exact artifact you are judging, quoted from a source you read in **this** turn:

- The code finding: its `file:line` plus the cited code, read freshly now.
- The graph/state code under review: the `StateGraph`, node, edge, or state-schema snippet your finding depends on, quoted from the file you just read.

> The artifact is the only source of truth. **Never** infer what you are reviewing from the branch name, the working directory, surrounding files, or recollection. If your mental model differs from the freshly read source, **the source wins.** A finding issued without a same-turn echo of its target is invalid — emit the echo first, or do not emit the finding.

This gate exists because an LLM under contextual priming will confidently flag code that is not in the file. It runs **before** the gates below.

## Review gates (sequenced)

Complete in order. Each step has an objective pass condition before moving on.

1. **Locate graph code** — Search the review scope for `StateGraph`, `compile(`, `invoke`, `ainvoke`, `add_node`, `add_edge`, `add_conditional_edges`. **Pass:** a short list of file paths (or explicit “none in scope” after searching).

2. **Map state schema** — For each graph state type (`TypedDict`, `BaseModel`, etc.), list fields that hold lists, dicts, or messages and whether `Annotated` + reducers (`add_messages`, `operator.add`, …) are present. **Pass:** every such field is either covered by a reducer pattern below or explicitly flagged as intentional overwrite.

3. **Trace persistence** — If interrupts, `thread_id`, or checkpoint APIs appear, follow them to `compile(..., checkpointer=...)` and invocation `config`. **Pass:** behavior matches the interrupt/checkpointer/thread_id guidance below—or you document a concrete mismatch with file:line.

4. **Report with evidence** — For each finding you will deliver, record **file path and line number(s)** (or a minimal quoted snippet). **Pass:** no critical or high-severity issue is stated without that citation.

5. **Run the checklist** — Use the checklist at the end of this skill; each item is **satisfied**, **not applicable** (with reason), or **open** with evidence. **Pass:** no item left silently unchecked.

## Critical Issues

### 1. State Mutation Instead of Return

```python
# BAD - mutates state directly
def my_node(state: State) -> None:
    state["messages"].append(new_message)  # Mutation!

# GOOD - returns partial update
def my_node(state: State) -> dict:
    return {"messages": [new_message]}  # Let reducer handle it
```

### 2. Missing Reducer for List Fields

```python
# BAD - no reducer, each node overwrites
class State(TypedDict):
    messages: list  # Will be overwritten, not appended!

# GOOD - reducer appends
class State(TypedDict):
    messages: Annotated[list, operator.add]
    # Or use add_messages for chat:
    messages: Annotated[list, add_messages]
```

### 3. Wrong Return Type from Conditional Edge

```python
# BAD - returns invalid node name
def router(state) -> str:
    return "nonexistent_node"  # Runtime error!

# GOOD - use Literal type hint for safety
def router(state) -> Literal["agent", "tools", "__end__"]:
    if condition:
        return "agent"
    return END  # Use constant, not string
```

### 4. Missing Checkpointer for Interrupts

```python
# BAD - interrupt without checkpointer
def my_node(state):
    answer = interrupt("question")  # Will fail!
    return {"answer": answer}

graph = builder.compile()  # No checkpointer!

# GOOD - checkpointer required for interrupts
graph = builder.compile(checkpointer=InMemorySaver())
```

### 5. Forgetting Thread ID with Checkpointer

```python
# BAD - no thread_id
graph.invoke({"messages": [...]})  # Error with checkpointer!

# GOOD - always provide thread_id
config = {"configurable": {"thread_id": "user-123"}}
graph.invoke({"messages": [...]}, config)
```

## State Schema Issues

### 6. Using add_messages Without Message Types

```python
# BAD - add_messages expects message-like objects
class State(TypedDict):
    messages: Annotated[list, add_messages]

def node(state):
    return {"messages": ["plain string"]}  # May fail!

# GOOD - use proper message types or tuples
def node(state):
    return {"messages": [("assistant", "response")]}
    # Or: [AIMessage(content="response")]
```

### 7. Returning Full State Instead of Partial

```python
# BAD - returns entire state (may reset other fields)
def my_node(state: State) -> State:
    return {
        "counter": state["counter"] + 1,
        "messages": state["messages"],  # Unnecessary!
        "other": state["other"]          # Unnecessary!
    }

# GOOD - return only changed fields
def my_node(state: State) -> dict:
    return {"counter": state["counter"] + 1}
```

### 8. Pydantic State Without Annotations

```python
# BAD - Pydantic model without reducer loses append behavior
class State(BaseModel):
    messages: list  # No reducer!

# GOOD - use Annotated even with Pydantic
class State(BaseModel):
    messages: Annotated[list, add_messages]
```

## Graph Structure Issues

### 9. Missing Entry Point

```python
# BAD - no edge from START
builder.add_node("process", process_fn)
builder.add_edge("process", END)
graph = builder.compile()  # Error: no entrypoint!

# GOOD - connect START
builder.add_edge(START, "process")
```

### 10. Unreachable Nodes

```python
# BAD - orphan node
builder.add_node("main", main_fn)
builder.add_node("orphan", orphan_fn)  # Never reached!
builder.add_edge(START, "main")
builder.add_edge("main", END)

# Check with visualization
print(graph.get_graph().draw_mermaid())
```

### 11. Conditional Edge Without All Paths

```python
# BAD - missing path in conditional
def router(state) -> Literal["a", "b", "c"]:
    ...

builder.add_conditional_edges("node", router, {"a": "a", "b": "b"})
# "c" path missing!

# GOOD - include all possible returns
builder.add_conditional_edges("node", router, {"a": "a", "b": "b", "c": "c"})
# Or omit path_map to use return values as node names
```

### 12. Command Without destinations

```python
# BAD - Command return without destinations (breaks visualization)
def dynamic(state) -> Command[Literal["next", "__end__"]]:
    return Command(goto="next")

builder.add_node("dynamic", dynamic)  # Graph viz won't show edges

# GOOD - declare destinations
builder.add_node("dynamic", dynamic, destinations=["next", END])
```

## Async Issues

### 13. Mixing Sync/Async Incorrectly

```python
# BAD - async node called with sync invoke
async def my_node(state):
    result = await async_operation()
    return {"result": result}

graph.invoke(input)  # May not await properly!

# GOOD - use ainvoke for async graphs
await graph.ainvoke(input)
# Or provide both sync and async versions
```

### 14. Blocking Calls in Async Context

```python
# BAD - blocking call in async node
async def my_node(state):
    result = requests.get(url)  # Blocks event loop!
    return {"result": result}

# GOOD - use async HTTP client
async def my_node(state):
    async with httpx.AsyncClient() as client:
        result = await client.get(url)
    return {"result": result}
```

## Tool Integration Issues

### 15. Tool Calls Without Corresponding ToolMessage

```python
# BAD - AI message with tool_calls but no tool execution
messages = [
    HumanMessage(content="search for X"),
    AIMessage(content="", tool_calls=[{"id": "1", "name": "search", ...}])
    # Missing ToolMessage! Next LLM call will fail
]

# GOOD - always pair tool_calls with ToolMessage
messages = [
    HumanMessage(content="search for X"),
    AIMessage(content="", tool_calls=[{"id": "1", "name": "search", ...}]),
    ToolMessage(content="results", tool_call_id="1")
]
```

### 16. Parallel Tool Calls Before Interrupt

```python
# BAD - model may call multiple tools including interrupt
model = ChatOpenAI().bind_tools([interrupt_tool, other_tool])
# If both called in parallel, interrupt behavior is undefined

# GOOD - disable parallel tool calls before interrupt
model = ChatOpenAI().bind_tools(
    [interrupt_tool, other_tool],
    parallel_tool_calls=False
)
```

## Checkpointing Issues

### 17. InMemorySaver in Production

```python
# BAD - in-memory checkpointer loses state on restart
graph = builder.compile(checkpointer=InMemorySaver())  # Testing only!

# GOOD - use persistent storage in production
from langgraph.checkpoint.postgres import PostgresSaver
checkpointer = PostgresSaver.from_conn_string(conn_string)
graph = builder.compile(checkpointer=checkpointer)
```

### 18. Subgraph Checkpointer Confusion

```python
# BAD - subgraph with explicit False prevents persistence
subgraph = sub_builder.compile(checkpointer=False)

# GOOD - use None to inherit parent's checkpointer
subgraph = sub_builder.compile(checkpointer=None)  # Inherits from parent
# Or True for independent checkpointing
subgraph = sub_builder.compile(checkpointer=True)
```

## Performance Issues

### 19. Large State in Every Update

```python
# BAD - returning large data in every node
def node(state):
    large_data = fetch_large_data()
    return {"large_field": large_data}  # Checkpointed every step!

# GOOD - use references or store
from langgraph.store.memory import InMemoryStore

def node(state, *, store: BaseStore):
    store.put(namespace, key, large_data)
    return {"data_ref": f"{namespace}/{key}"}
```

### 20. Missing Recursion Limit Handling

```python
# BAD - no protection against infinite loops
def router(state):
    return "agent"  # Always loops!

# GOOD - check remaining steps or use RemainingSteps
from langgraph.managed import RemainingSteps

class State(TypedDict):
    messages: Annotated[list, add_messages]
    remaining_steps: RemainingSteps

def check_limit(state):
    if state["remaining_steps"] < 2:
        return END
    return "continue"
```

## Code Review Checklist

1. [ ] State schema uses Annotated with reducers for collections
2. [ ] Nodes return partial state updates, not mutations
3. [ ] Conditional edges return valid node names or END
4. [ ] Graph has path from START to all nodes
5. [ ] Checkpointer provided if using interrupts
6. [ ] Thread ID provided in config when using checkpointer
7. [ ] Tool calls paired with ToolMessages
8. [ ] Async nodes use async operations
9. [ ] Production uses persistent checkpointer
10. [ ] Recursion limits considered for loops

More from existential-birds/beagle

SkillDescription
adr-decision-extractionUse when you need to mine a conversation, session transcript, or design discussion for architectural decisions before writing ADRs. Identifies problem-solution pairs, trade-off debates, technology choices, and explicit \"[ADR]\" tags. Triggers on \"what decisions did we make\", \"extract decisions from this chat\", \"find the choices in our discussion\", or \"summarize architectural decisions\". Also useful after long planning sessions to capture decisions that were made implicitly. Does NOT write ADR documents \u2014 use adr-writing or write-adr for that.
adr-writingUse when writing or formatting an ADR document using the MADR template, applying Definition of Done (E.C.A.D.R.) criteria, or verifying ADR completeness. Triggers on \"write the ADR\", \"format as MADR\", \"check ADR quality\", \"mark gaps in ADR\". Also triggers when a decision has been extracted and needs to become a document. Does NOT extract decisions from conversations (use adr-decision-extraction) or orchestrate the full extract-confirm-write workflow (use write-adr).
agent-architecture-analysisUse when auditing an agent codebase against the 12-Factor Agents methodology, reviewing LLM-powered system architecture, or assessing agentic app compliance. Triggers on \"analyze agent architecture\", \"12-factor audit\", \"how compliant is this agent\", or \"evaluate this LLM app\". Also applies when comparing frameworks or planning agent improvements. Not for quick checklists \u2014 this performs deep per-factor codebase analysis with file-level evidence.
app-intents-code-reviewReviews App Intents code for intent structure, entities, shortcuts, and parameters. Use when reviewing code with import AppIntents, @AppIntent, AppEntity, AppShortcutsProvider, or @Parameter.
artifact-analysisUse when the user wants a cited, structured read of local documents and project knowledge. Triggers on: \"analyze these docs\", \"scan my project for context\", \"read the docs folder\", \"summarize what's in .beagle/concepts/\", \"extract context from docs/\", \"what's in this folder\", \"go read everything in X and tell me what's there\". Also invoked programmatically by other beagle skills (prfaq-beagle Ignition, brainstorm-beagle reference points, strategy-interview context grounding) via the companion contract. Does NOT trigger on codebase lookups (\"find this function\", \"search the repo\"), web research (use web-research), LLM-as-judge evaluation (use llm-judge), or document editing (use humanize-beagle). Produces a written scan plan, parallel-subagent findings, and a cited synthesis report on disk — never inline prose, never unsourced claims.
axum-code-reviewReviews axum web framework code for routing patterns, extractor usage, middleware, state management, and error handling. Use when reviewing Rust code that uses axum, tower, or hyper for HTTP services. Covers axum 0.7+ patterns including State, Path, Query, Json extractors.
brainstorm-beagleUse when the user has a fuzzy idea and wants to shape it into a concrete project spec before planning or building. Triggers on: \"brainstorm this\", \"I have an idea for...\", \"help me think through this project\", \"what should I build\", \"spec this out\". Also catches vague feature descriptions needing structured questioning to clarify scope. Does NOT write code, plan implementation, review strategy docs, or run strategy interviews \u2014 produces a WHAT/WHY spec through dialogue, not a HOW plan.
bubbletea-code-reviewReviews BubbleTea TUI code for proper Elm architecture, model/update/view patterns, and Lipgloss styling. Use when reviewing terminal UI code using charmbracelet/bubbletea.
cloudkit-code-reviewReviews CloudKit code for container setup, record handling, subscriptions, and sharing patterns. Use when reviewing code with import CloudKit, CKContainer, CKRecord, CKShare, or CKSubscription.
combine-code-reviewReviews Combine framework code for memory leaks, operator misuse, and error handling. Use when reviewing code with import Combine, AnyPublisher, @Published, PassthroughSubject, or CurrentValueSubject.