Repository navigation
refactor: Add comprehensive test suite and fix API field naming - #1
Merged
Merged
Conversation
The AgentExecution and AgentStep types claimed the server used Go's default PascalCase field names. It does not: agent.AgentExecution tags every field, so the wire format is snake_case like the rest of the API. Every read of an execution — status, duration, output, tool calls and the execution path in both the debug view and the store — was therefore reading undefined, and the debug panel showed a blank result for runs that had actually succeeded. Verified against a live server: the GoLangGraph repository now carries an end-to-end suite that drives each of these endpoints and asserts the field names this file declares. Changes: - AgentExecution now mirrors the real payload: id, timestamp, input, output, structured_output, tool_calls, duration, success, error, metadata, execution_path, state_changes. There is one timestamp marking the start plus a duration, rather than the separate StartTime and EndTime that were declared but never sent. - AgentStep is replaced by StateChange, which is what the server actually records per node visit. No AgentStep type exists on the server. - DebugView and useStudioStore read the corrected field names, and the execution status shown is derived from the success flag, since the server sends no separate status field. Typecheck, lint and production build are clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017JM31riXDsAT7MEtDs9crc
The repository had no tests, and CI ran lint and tests with `|| true`, so neither could ever fail the build. Nothing would have caught the type mismatch this branch fixes: the declared execution shape compiled fine while disagreeing with what the server actually sends, and a compiler cannot know the wire format. Adds 17 tests for the API client that assert the real payload shapes. The fixtures are the payloads the Go server produces, taken from the end-to-end suite that drives a live server, so the two repositories now pin the same contract from both sides. They cover the API key header, error-field surfacing, unreachable servers and non-JSON error bodies; agent configurations and the empty-list fallbacks; successful and failed executions in snake_case; tools and provider descriptions; the graph topology the graph view renders, including conditional edges; and the output and tool-call display helpers. CI now typechecks with the project's TypeScript, and lint and tests fail the build rather than being swallowed. Also removes interrupt_on from AgentConfig, which the server has never sent, and adds streaming_mode, which it does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017JM31riXDsAT7MEtDs9crc
The console hid failures. A failed run reported only its duration, and because a failed execution usually has empty output the assistant bubble said "No output returned." — the server sends the reason in `error` and nothing read it. The debug panel showed the word "Failed", an empty output block and no explanation. Both now show the reason, and the store carries it into the execution log and the message metadata. A failed run also painted the whole graph green: every node was marked completed unconditionally before the execution path was applied. The NodeStatus 'error' existed in the types and nothing in the codebase ever produced it. Nodes on the server's execution_path are now completed and the rest idle; a failed run is marked error. Other defects fixed in the store: - connect() caught and discarded failures from listAgents, listTools and listProviders, so a server whose /agents endpoint returned 500 looked identical to one with no agents: "Connected", an empty list, no message. - Selecting one agent and then another let the first request's slower response overwrite the second's graph; topology fetches are now guarded by a request token. - Selecting an unknown agent left the previous agent's graph on screen, silently attributed to the new selection. - Stop did not stop anything: the request kept running and its answer was appended to the thread seconds later. A superseded result is now dropped. - The value computed for currentGraphNode was discarded immediately by the finally block, so the graph never highlighted where a run ended. - Reconnection failed silently once the retry budget was spent, leaving a Retry button that did nothing with no explanation. - disconnect() cleared some state and kept the rest, so the debug view described a run whose logs had just been wiped. Header indexed a three-key style map with a type string the server controls, emitting `class="… undefined"` for anything else. Adds 62 tests across the store and every view, driving the real store through the real API client with only fetch stubbed, asserting on rendered DOM and resulting state rather than on mock calls. Reverting the store fixes makes 11 of them fail, so they hold. Test suite: 20 -> 79. GraphView's Math.min over an empty array was investigated and is already guarded by an early return; a test pins that guard rather than changing it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017JM31riXDsAT7MEtDs9crc
piotrlaczkowski
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds a comprehensive test suite covering the studio store, API client, and UI components, while fixing critical bugs in API field naming and state management that were causing data loss and incorrect behavior.
Key Changes
Test Infrastructure
src/test-utils/studioTestUtils.ts): Shared test harness with fixtures and mock server that stubs onlyfetch, allowing tests to drive the real store through the real API clientsrc/store/__tests__/useStudioStore.test.ts): 490 lines of behavioral tests covering connection lifecycle, graph data handling, agent execution, and state cleanupsrc/api/__tests__/client.test.ts): Contract tests validating the client correctly reads real Go server payloadsCritical Bug Fixes
tool_calls,execution_path,success) but types were incorrectly declared as PascalCase, causing all fields to read asundefineddisconnect()now properly clearslastExecution,currentGraphNode,isExecuting, and other fields that were left behind, preventing stale data from appearing in the UIType Corrections
AgentExecutioninterface to use correct snake_case field names matching Go struct JSON tagsAgentStepwithStateChangeto match actual server payload structureinterrupt_on) and added optional fields (streaming_mode)Store Improvements
describeError()utility to normalize error handlingattemptReconnection()to properly report when retry budget is exhaustedCI/CD Updates
|| true)CI=trueenvironment variable for test runsNotable Implementation Details
fetchstubbed, ensuring code under test matches production exactlyhttps://claude.ai/code/session_017JM31riXDsAT7MEtDs9crc