Skip to content

Refactor tracer - integrate built-in tracing mechanism#2

Merged
ArtyomVancyan merged 23 commits intomainfrom
refactor-tracer
Feb 20, 2026
Merged

Refactor tracer - integrate built-in tracing mechanism#2
ArtyomVancyan merged 23 commits intomainfrom
refactor-tracer

Conversation

@ArtyomVancyan
Copy link
Member

This pull request refactors the trace inspector and related data flow to simplify the codebase and streamline the management and display of node execution data. The main change is the consolidation of node output and step logs into a unified nodeEntries structure, which simplifies both state management and UI rendering. The UI for the inspector tree and detail pane has also been updated for clarity and maintainability.

Important changes:

  • Removed the now-obsolete useInspectTree hook and all related logic, since the new data structure makes this unnecessary
  • Simplified the tree and detail rendering logic, and removed the complex NodeDetail and DetailSection components in favor of a more direct and readable implementation
  • Improved the inspector tree and detail pane UI, including better handling of icons, step status, and error highlighting for a more intuitive user experience
  • Cleaned up unused or redundant type definitions and properties, such as removing the old NodeKind union and updating node kind handling

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the trace inspector by replacing the custom callback-based tracing system with langchain_core's built-in AsyncBaseTracer. The main architectural change consolidates separate nodeOutputLog and nodeStepLog into a unified nodeEntries structure, simplifying data flow from backend to frontend.

Changes:

  • Replaced SubStepCallbackHandler with BroadcastingTracer that extends AsyncBaseTracer, using run.run_type for node classification instead of the custom classify_node function
  • Unified node execution tracking into a single NodeEntry type with hierarchical relationships via parent_run_id
  • Removed the complex useInspectTree hook and computeDepthMap logic, simplifying the inspector UI to directly render the flat list of entries

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
langgraphics/topology.py Removed classify_node function and node_kind field from extracted topology
langgraphics/streamer.py Replaced AsyncCallbackHandler with AsyncBaseTracer, using run.run_type for classification
tests/lib/test_classify.py Removed tests for the deleted classify_node function
langgraphics-web/src/types.ts Simplified types: removed old NodeKind union, added new NodeMessage/NodeEntry types
langgraphics-web/src/hooks/useWebSocket.ts Consolidated nodeStepLog and nodeOutputLog into single nodeEntries array
langgraphics-web/src/hooks/useInspectTree.tsx Deleted entire hook with depth computation logic
tests/web/depthMap.test.ts Removed tests for deleted computeDepthMap function
langgraphics-web/src/components/InspectPanel.tsx Simplified to directly render nodeEntries tree without depth computation
langgraphics-web/src/main.tsx Updated to pass nodeEntries instead of separate logs
langgraphics-web/src/layout.ts Set nodeKind to null in layout (moved to runtime via tracer)
langgraphics-web/src/index.css Updated inspector styles with new step status indicators and icons
langgraphics-web/tsconfig.app.json Changed target from ES2022 to ESNext
langgraphics-web/public/icons/chat_model.svg Added new icon for chat_model node type

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ArtyomVancyan
Copy link
Member Author

@claude, please review the changes

@claude
Copy link

claude bot commented Feb 20, 2026

Claude finished @ArtyomVancyan's task in 4m 23s —— View job


Code Review

  • Gathering context and reading changed files
  • Reviewing backend changes (streamer.py, topology.py)
  • Reviewing frontend changes (types, hooks, components)
  • Reviewing test and config changes
  • Compiling review feedback

Overall this is a strong refactoring. Replacing the custom callback handler with AsyncBaseTracer, unifying nodeOutputLog/nodeStepLog into nodeEntries, and deleting ~800 lines of complexity (the useInspectTree hook, computeDepthMap, etc.) all improve maintainability significantly. The new data flow is much easier to reason about.

A few real issues and some notes on the Copilot review comments below.


Issues

1. Auto-selection in InspectPanel picks the wrong entry

InspectPanel.tsx:47-51:

useEffect(() => {
    if (nodeEntries.length > 0 && !selectedKey) {
        setSelectedKey(nodeEntries[0].run_id);
    }
}, [nodeEntries, selectedKey]);

nodeEntries[0] is almost always a child step, not a root node. The execution order is: child LLM/tool ends first (_emit_end fires) → then the parent LangGraph node ends (_on_chain_end fires). So the first entry in nodeEntries will have a parent_run_id, and while the tree shows it (since expandedKeys is fully expanded), the initial selection jumps to a child rather than the root node. This creates a confusing first impression.

Fix: select the first root node instead:

setSelectedKey(nodeEntries.find(e => !e.parent_run_id)?.run_id ?? nodeEntries[0].run_id);

Fix this →


2. Tree depth is hard-coded to 2 levels

InspectPanel.tsx:26-43:

children: nodeEntries.filter(({parent_run_id}) => parent_run_id === entry.run_id).map(child => ({
    isLeaf: true,
    ...

The tree only renders root nodes → their direct children. If there is a chain-inside-a-chain scenario (e.g., a LangGraph node calls a chain which calls an LLM), the intermediate chain and the LLM step would each arrive with different parent_run_id values. The LLM step's parent_run_id points to the intermediate chain, not the root node — so it won't appear in the tree at all.

Whether this matters today depends on the graph structure being targeted, but it's worth documenting as a known limitation or implementing a recursive tree build.

Fix this →


3. self.node_kinds dict is redundant

streamer.py:98-109:

async def _on_chain_end(self, run: Run) -> None:
    self.node_kinds[run.name] = run.run_type  # set here
    ...
    "node_kind": self.node_kinds.get(run.name),  # immediately read back

self.node_kinds.get(run.name) always equals run.run_type at this point — the dict stores and immediately retrieves the same value. _emit_end (called from other handlers) already uses run.run_type directly. Using run.name as the dict key is also fragile — if the same node name appears in different runs with different types, the dict will hold whatever was last written.

The self.node_kinds dict and all assignments to it can be removed; replace self.node_kinds.get(run.name) with run.run_type in _on_chain_end. Fix this →


4. NodeKind type may not cover all run_type values emitted by langchain_core

types.ts:13:

export type NodeKind = "llm" | "chain" | "tool" | "retriever" | "embedding" | "prompt" | "parser";

langchain_core may emit run_type = "chat_model" for newer-style chat model runs (the PR description mentions adding a chat_model.svg icon, though it's not present in the current branch). The SVG icon is loaded dynamically via string interpolation, so TypeScript won't catch a mismatch — but unexpected node_kind values from the backend would produce a broken image link in the UI.

Adding "chat_model" to the union (and shipping the SVG) would make this robust. If intentionally excluding it (chat models always return "llm"), a comment in the type definition would clarify.


Minor Notes

tsconfig.app.json — ESNext target: The Copilot comment on this is valid. Since this is a Vite/bundler-mode project with noEmit: true, the TypeScript target primarily affects which type definitions are available, not actual output. That said, pinning to a concrete version like "ES2022" or "ES2024" avoids surprise behavior changes when a new ECMAScript spec ships.

layout.ts:114nodeKind removed from node data: Intentional and correct. Node kind is now a runtime property from the tracer, not topology metadata. The Copilot suggestion to retain (n as any).node_kind is not needed.


On the Copilot Review Comments

Several Copilot comments appear to reference an intermediate commit state rather than the final branch HEAD:

  • "Missing return in except block" (streamer.py:30) — return parse_message(res[0]) is present in the current file.
  • "Missing overflow-y: auto on tree pane" (index.css:228) — overflow-y: auto is present at line 229 in the current file.
  • "Missing status field in _on_chain_end""status": "error" if run.error else "ok" is present at line 110.
  • "_on_chain_error doesn't emit for nodes in node_names"_on_chain_error delegates to _on_chain_end which correctly handles both cases. The Copilot suggestion would duplicate the logic unnecessarily.

These can be dismissed.

@ArtyomVancyan
Copy link
Member Author

@claude, I like your suggestions, please make comments using GitHub suggestion blocks.

@claude
Copy link

claude bot commented Feb 20, 2026

Claude finished @ArtyomVancyan's task in 2m 50s —— View job


Implemented Review Suggestions

  • Read changed files
  • Fix 1: Auto-selection picks wrong entry (InspectPanel.tsx)
  • Fix 2: Recursive tree construction (InspectPanel.tsx)
  • Fix 3: Remove redundant node_kinds dict (streamer.py)
  • Fix 4: NodeKind type — already correct in current branch ("chat_model" is absent, "embedding"/"prompt"/"parser" are present, no chat_model.svg exists)

Committed as 022b2a0 with the following changes:

langgraphics-web/src/components/InspectPanel.tsx

Fix 1 — auto-selection (was nodeEntries[0], a child step):

// before
setSelectedKey(nodeEntries[0].run_id);

// after
setSelectedKey(nodeEntries.find(e => !e.parent_run_id)?.run_id ?? nodeEntries[0].run_id);

Fix 2 — recursive tree building (was hard-coded 2 levels):

// before — inline 2-level map, grandchildren silently dropped
const treeData = useMemo((): TreeDataNode[] => {
    return nodeEntries.filter(({parent_run_id}) => !parent_run_id).map(entry => ({
        ...
        children: nodeEntries.filter(({parent_run_id}) => parent_run_id === entry.run_id).map(child => ({
            isLeaf: true,
            ...
        })),
    }))
}, [nodeEntries]);

// after — standalone recursive function, handles arbitrary depth
function buildTree(entries: NodeEntry[], parentRunId: string | null | undefined): TreeDataNode[] {
    return entries
        .filter(e => (e.parent_run_id ?? null) === (parentRunId ?? null))
        .map(entry => {
            const children = buildTree(entries, entry.run_id);
            const isLeaf = children.length === 0;
            return { key: entry.run_id, isLeaf, title: ..., children: isLeaf ? undefined : children };
        });
}

langgraphics/streamer.py

Fix 3 — remove redundant node_kinds dict:

# before
self.node_kinds: dict[str, str] = {}
...
async def _on_chain_end(self, run: Run) -> None:
    self.node_kinds[run.name] = run.run_type   # stored
    ...
    "node_kind": self.node_kinds.get(run.name),  # immediately read back

async def _on_llm_end(self, run: Run) -> None:
    self.node_kinds[run.name] = run.run_type   # never read back
    await self._emit_end(run)
# ... same pattern for _on_llm_error, _on_tool_end/error, _on_retriever_end/error

# after
async def _on_chain_end(self, run: Run) -> None:
    ...
    "node_kind": run.run_type,   # direct, no dict needed

async def _on_llm_end(self, run: Run) -> None:
    await self._emit_end(run)   # _emit_end already uses run.run_type
# ... same for other handlers

·

github-actions bot and others added 4 commits February 20, 2026 18:07
…de_kinds

- InspectPanel: extract buildTree() as a recursive function so entries
  nested more than one level deep are reflected in the tree instead of
  being silently dropped
- InspectPanel: auto-select the first root entry (no parent_run_id)
  rather than nodeEntries[0], which is typically a child step that
  completes before its parent node
- streamer: remove the self.node_kinds dict and all assignments to it;
  replace self.node_kinds.get(run.name) in _on_chain_end with
  run.run_type directly, matching what _emit_end already does

Co-authored-by: Artyom Vancyan <ArtyomVancyan@users.noreply.github.com>
@ArtyomVancyan ArtyomVancyan merged commit 50573bf into main Feb 20, 2026
16 checks passed
@ArtyomVancyan ArtyomVancyan deleted the refactor-tracer branch February 20, 2026 19:14
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.

2 participants