refactor(1/3): extract shared production helpers to reduce code duplication - #3
Merged
Conversation
…ication and cyclomatic complexity: SandboxChecker.matchesAllowedPathList(), OpenAIClient.buildRequestBody(), and Agent.closedQueryResult().
…eateTraceRecorder) from duplicated code in Agent.swift's two largest methods, reducing promptImpl from 678→625 lines and stream from 1120→1092 lines, with all 6209 tests passing.
…essionMessages) that eliminated 7 duplicated inline blocks across promptImpl and stream, reducing promptImpl from 641→610 lines and stream from 1108→1078 lines with all 6212 tests passing.
…createSubAgentSpawner) that eliminated duplicated skill resolution logic across executeSkill/executeSkillStream and SubAgentSpawner construction across promptImpl/stream, with all 5682 tests passing.
… executeToolWithLifecycle) from ToolExecutor.swift's executeSingleTool method, eliminating 5× duplicated error-return patterns, 2× ToolStartedEvent emission blocks, and 2× full tool execution lifecycle blocks, reducing executeSingleTool from ~250→99 lines with all 6211 tests passing.
…rver) from MCPClientManager.swift, eliminating 3× duplicated 6-case switch dispatch blocks across connectAll/reconnect/setServers and 2× duplicated 4-line server removal blocks in setServers, reducing the file from 622→610 lines with all 5684+12 tests passing.
…sultFromExecute, executionErrorResult) from ToolBuilder.swift, eliminating duplicated input decoding pipeline across CodableTool/StructuredCodableTool (2×26 identical lines), duplicated result mapping across RawInputTool/StructuredCodableTool (2×13 identical lines), and duplicated error-return catch blocks across 3 tool structs (3×6 identical lines), reducing the file from 430→409 lines with all 5682+12 tests passing.
…ppendFallbackEntry) from Compact.swift, eliminating 3× duplicated LLM response text extraction blocks and 3× duplicated fallback SessionMemoryEntry construction blocks, reducing the file from 485→477 lines with all 5682+12 tests passing.
…oryStore.swift, eliminating 2× duplicated 26-line filter/sort/limit blocks across InMemoryStore.query and FileBasedMemoryStore.query, reducing the file from 391→377 lines with all 5682+12 tests passing.
…ponse, stripCodeFences) into new LLMResponseHelpers.swift, eliminating 3× duplicated extractTextFromResponse implementations and 3× duplicated stripCodeFences implementations across LLMSkillEvolver.swift, PromptEvolverEngine.swift, and LLMExperienceExtractor.swift, reducing those 3 files by 93 lines combined with all 5682+12 tests passing.
…hropicClient.swift (eliminating 2×11-line duplicated URLError catch blocks) and validatePathSafeIdentifier() into EnvUtils.swift (eliminating 3×10-line duplicated path traversal validation methods across SessionStore, FactStore, and MemoryStore), net -18 lines with all 5682+12 tests passing.
…SkillsDir, atomicWriteJSON) into EnvUtils.swift, eliminating 2× duplicated skills directory resolution blocks (~15 lines each) and 2× duplicated atomic JSON file write patterns (~40 lines each) from SkillUsageStore and SkillCuratorStore, net -36 lines with all 5684+12 tests passing.
…ryDir()/defaultMemoryDir shared helpers into EnvUtils.swift, eliminating 9× duplicated 5-line dateFormatter initializations across store files and Logger, plus 2× duplicated ~18-line resolveMemoryDir methods from MemoryStore and FactStore, net -71 lines with all 5682+12 tests passing.
…wift (unifying 3 duplicated JSON parsing implementations from Agent.swift, OpenAIClient.swift, and Streaming.swift) and replaced 4 inline ISO8601DateFormatter constructions in RunTracker, RunRecoveryService, and ExperienceTypes with makeISO8601DateFormatter() calls, net -26 lines with all 6212+12 tests passing.
…lidateLLMHTTPResponse, resolveBaseURL) into LLMClient.swift, unifying duplicated URLError handling, HTTP response validation, and URL resolution between AnthropicClient and OpenAIClient, net -33 lines with all 6212+12 tests passing.
…ted, defaultHomeDir) eliminating 5× duplicated stop+sessionEnd hook blocks, 2× duplicated LLMRequestStartedEvent emissions, and 2× duplicated home directory resolution blocks, net -12 lines with all 5682+12 tests passing.
…BudgetExceededInStream) eliminating 2× duplicated error classification blocks, 3× duplicated budget check patterns, and 2× duplicated ~35-line budget exceeded handling blocks, net -3 lines with all 5682+12 tests passing.
…LMResponseAsObject, parseLLMResponseAsArray) into LLMResponseHelpers.swift, eliminating the identical trim→stripCodeFences→data→JSONSerialization pipeline duplicated across LLMSkillEvolver, PromptEvolverEngine, and LLMExperienceExtractor, with all 6212+12 tests passing.
…keSDKJSONDecoder) into EnvUtils.swift, eliminating 3× duplicated djb2 hash implementations + 1 inline hash + 3× duplicated JSON encoder/decoder initialization blocks + 3× inline decoder constructions across 6 files, net -28 lines with all 5682+12 tests passing.
…ed ~12-line skill reconstructions in review tools, and replaced inline JSONEncoder/JSONDecoder in MemoryBundleExport/ImportService with shared makeSDKJSONEncoder/makeSDKJSONDecoder factories, net -28 lines with all 5682+12 tests passing.
…tInterrupted and emitAgentFailed in Agent) eliminating 7× duplicated Logger+SDKError denial blocks and 8× duplicated if-let-eventBus+AgentTerminalEvent emission patterns, net -6 lines with all 5682+12 tests passing.
…fy) into EnvUtils.swift, eliminating the last remaining #if os(Linux) home directory resolution pattern from SessionStore and unifying 2 duplicated JSON serialization functions from ConfigTool and ReadMcpResourceTool, net -25 lines with all 5682+12 tests passing.
…xt) centralizing 4× duplicated LLMCostEvent constructions and 2× duplicated 24-field ToolContext constructions across promptImpl and stream, with all 5682+12 tests passing.
…orResponse, requireNonEmptyInput) centralizing 4× duplicated createDirectory+error blocks across store files and 14× duplicated error response patterns + 9× duplicated non-empty guard patterns across review tools, net -41 lines with all 5682+12 tests passing.
…ft, centralizing 4× duplicated ~28-line cost-tracking pipelines (TokenUsage parsing → estimateCost → recordUsage → emitLLMCostEvent → recordCostBreakdown) across promptImpl fallback path, promptImpl main loop, stream messageStart, and stream messageDelta, reducing Agent.swift from 3619→3586 lines with all 5682+12 tests passing.
…efaultSubAgentSpawner.swift and refactored simple spawn to delegate to enhanced spawn, reducing the file from 197→169 lines (-28 lines) by eliminating ~45 lines of duplicated tool filtering, Agent creation, and result processing logic across 2 spawn methods, with all 5682+12 tests passing.
…Value, anyToMCPValue, mcpValueToAny) into MCPTypes.swift, eliminating 8 duplicated private methods across InProcessMCPServer, MCPClientManager, and AgentMCPServer, net -54 lines with all 5682+12 tests passing.
…ver, createMCPSession) and moved ToolExecutionError to MCPTypes.swift, eliminating duplicated tool registration loop (~45 lines), session creation (2×5 lines), and error struct (2×8 lines) across InProcessMCPServer and AgentMCPServer, net -28 lines with all 5682+12 tests passing.
… EnvUtils.swift and 2 default directory constants (defaultTracesDir, defaultApiRunsDir), eliminating duplicated YAML escaping from CuratorRunReport and SkillWriter and inline home directory resolution from TraceRecorder and RunPersistenceService, net -6 lines with all 5682+12 tests passing.
… helper infrastructure from complexity reduction refactoring - project-context.md: added rules 48-56 (shared utility patterns, test infrastructure), updated file counts in rule 35 - architecture.md: added LLMResponseHelpers.swift and AgentEventSSEMapping.swift to Utils/ listing, added shared test infrastructure files to Tests/ listing
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
1 participant
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
29 iterations of shared helper extraction in production code, eliminating code duplication across Agent, ToolExecutor, MCP, LLM client, Store, and utility layers.
New shared infrastructure
LLMResponseHelpers.swift(NEW)EnvUtils.swift(expanded)LLMClient.swift(expanded)MCPTypes.swift(expanded)ReviewTools.swift(expanded)Agent.swift(internal)ToolExecutor.swift(internal)Key changes
Verification
🤖 Generated with Claude Code