Skip to content

[refactor] Semantic Function Clustering: Refactoring Opportunities in Go Source Files #2831

@github-actions

Description

@github-actions

Overview

Automated semantic function clustering analysis of 90 non-test Go files in internal/ (638 function definitions across 20 packages). The analysis identified 1 clear duplicate wrapper, 4 oversized files needing decomposition, 2 misplaced functions, and several consolidation opportunities.


1. Duplicate / Redundant Wrapper Function

server/http_helpers.go:writeJSONResponse — pure wrapper (no added value)

internal/server/http_helpers.go lines 20–24 defines a private wrapper that does nothing except delegate to the public httputil.WriteJSONResponse:

// internal/server/http_helpers.go
func writeJSONResponse(w http.ResponseWriter, statusCode int, body interface{}) {
    httputil.WriteJSONResponse(w, statusCode, body)   // ← one-liner passthrough
}

The implementation lives in internal/httputil/httputil.go:

// internal/httputil/httputil.go
func WriteJSONResponse(w http.ResponseWriter, statusCode int, body interface{}) {
    w.Header().Set("Content-Type", "application/json")
    w.WriteHeader(statusCode)
    json.NewEncoder(w).Encode(body)
}

Current callers of the private wrapper inside internal/server/:

File Line Call site
server/health.go 56 writeJSONResponse(w, http.StatusOK, response)
server/routed.go 27 writeJSONResponse(w, http.StatusServiceUnavailable, ...)
server/handlers.go 51 writeJSONResponse(w, http.StatusGone, ...)
server/handlers.go 63 writeJSONResponse(w, http.StatusOK, ...)

Recommendation: Remove writeJSONResponse from http_helpers.go and update the 4 call sites to use httputil.WriteJSONResponse directly. This eliminates an unnecessary indirection layer with zero behaviour change.


2. Misplaced Functions (Outliers)

2a. logRuntimeError in server/auth.go

internal/server/auth.go contains two authentication middleware functions (authMiddleware, applyAuthIfConfigured) plus a logging helper (logRuntimeError) that produces structured runtime error entries. The logging helper has nothing to do with auth — it formats and writes a JSON error log entry.

// internal/server/auth.go:14
func logRuntimeError(errorType, detail string, r *http.Request, serverName *string) {
    // Builds and writes a structured JSON runtime-error log entry
    ...
}

Recommendation: Move logRuntimeError to internal/server/http_helpers.go (already the home for server-scoped HTTP/response utilities) or create a dedicated internal/server/logging.go.


2b. Generic JSON map helpers in server/difc_log.go

internal/server/difc_log.go (purpose: log DIFC-filtered items) contains three domain-agnostic JSON map utilities at lines 64–119:

func getStringField(m map[string]interface{}, fields ...string) string { ... }
func extractAuthorLogin(m map[string]interface{}) string { ... }
func extractNumberField(m map[string]interface{}) string { ... }

These are general-purpose helpers that traverse map[string]interface{} values — not specific to DIFC logging. They have no callers outside difc_log.go today, but the pattern is re-invented in other places (e.g., proxy/handler.go JSON traversal).

Recommendation: Two options depending on appetite:

  • (Minimal) Leave in place but add a comment that they are local helpers.
  • (Better) Extract to a new internal/maputil/ package (GetStringField, GetNestedString, GetNumberAsString) that any package can import, avoiding re-invention.

3. Oversized Files — Candidates for Decomposition

The following files combine multiple distinct responsibilities into a single large file, making navigation and testing harder.

3a. internal/guard/wasm.go — 1 161 lines, 25 functions

This single file covers four distinct responsibilities:

Responsibility Functions Suggested new file
Guard lifecycle (constructor, Close) NewWasmGuard, NewWasmGuardFromBytes, NewWasmGuardWithOptions, Close keep in wasm.go
WASM runtime / memory management callWasmFunction, tryCallWasmFunction, wasmAlloc, wasmDealloc, instantiateHostFunctions, hostCallBackend, hostLog, isWasmTrap wasm_runtime.go
Payload building BuildLabelAgentPayload, buildStrictLabelAgentPayload, normalizePolicyPayload, isValidAllowOnlyRepos wasm_payload.go
Response parsing parseLabelAgentResponse, parseResourceResponse, parseCollectionLabeledData, parsePathLabeledResponse, checkBoolFailure wasm_parse.go

Recommendation: Split into wasm.go (lifecycle) + wasm_runtime.go + wasm_payload.go + wasm_parse.go. All remain in package guard; no API changes.


3b. internal/config/guard_policy.go — 721 lines, 21 functions

Four distinct responsibilities packed into one file:

Responsibility Functions Suggested new file
Core types + serialization UnmarshalJSON (×2), MarshalJSON (×2), IsWriteSinkPolicy keep in guard_policy.go
Validation ValidateGuardPolicy, ValidateWriteSinkPolicy, validateAcceptEntry, isValidRepoScope, isValidTokenString, isValidRepoOwner, isValidRepoName, isScopeTokenChar, validateGuardPolicies guard_policy_validate.go
Parsing ParseGuardPolicyJSON, ParsePolicyMap, ParseServerGuardPolicy, BuildAllowOnlyPolicy guard_policy_parse.go
Normalization NormalizeGuardPolicy, normalizeAndValidateScopeArray, NormalizeScopeKind guard_policy_normalize.go

Recommendation: Split into 4 files in internal/config/, all package config. No exported API changes.


3c. internal/mcp/connection.go — 649 lines, 25 functions (+ http_transport.go 583 lines, 21 functions)

connection.go mixes connection management, reconnect logic, and MCP method implementations:

Responsibility Functions Suggested new file
Core connection lifecycle NewConnection, NewHTTPConnection, IsHTTP, GetHTTP*, Close, context helpers keep in connection.go
Send / reconnect SendRequest, SendRequestWithServerID, callSDKMethod, callSDKMethodWithReconnect, reconnectPlainJSON, reconnectSDKTransport, requireSession connection_send.go
MCP method wrappers listTools, callTool, listResources, readResource, listPrompts, getPrompt, unmarshalParams, marshalToResponse connection_methods.go

Similarly, http_transport.go has a natural split:

Responsibility Functions Suggested new file
Client construction newMCPClient, newHTTPConnection, buildHTTPClientWithHeaders, RoundTrip keep in http_transport.go
Transport probing trySDKTransport, tryStreamableHTTPTransport, trySSETransport, tryPlainJSONTransport, initializeHTTPSession, buildSessionHeaderModifier http_transport_probe.go
Request / response createJSONRPCRequest, setupHTTPRequest, executeHTTPRequest, sendHTTPRequest, parseHTTPResult, parseJSONRPCResponseWithSSE, parseSSEResponse, ensureToolCallArguments http_transport_request.go

3d. internal/config/validation_schema.go — 551 lines, 8 functions

Mixes HTTP schema fetching, JSON-Schema compilation, and multi-level error formatting:

Responsibility Functions Suggested new file
Fetch + fix fetchAndFixSchema, isTransientHTTPError validation_schema_fetch.go
Compile + validate getOrCompileSchema, validateJSONSchema, validateStringPatterns keep in validation_schema.go
Error formatting formatSchemaError, formatValidationErrorRecursive, formatErrorContext validation_schema_format.go

4. Intentional Patterns (No Action Needed)

The following patterns appear duplicated but are correctly structured with existing documentation in internal/logger/common.go:

  • withLock on each logger type — identical 4-line body per type; correctly not shared because each method is on a different receiver type and Go has no mixin mechanism.
  • setup*Logger / handle*LoggerError quad — different fallback strategies per logger type (stdout, silent, strict, unified); the differentiation is intentional.
  • Log{Info,Warn,Error,Debug}[*] families — three public APIs with distinct signatures and callers; one-liner wrappers are trivial and idiomatic in Go.
  • initGlobal*Logger / closeGlobal*Logger in global_helpers.go — managed by a generic initLogger[T] helper; the repetition is in names only.

5. Implementation Checklist

Quick wins (< 1 hour each)

  • Remove server/http_helpers.go:writeJSONResponse wrapper; update 4 callers to use httputil.WriteJSONResponse
  • Move server/auth.go:logRuntimeError to server/http_helpers.go or new server/logging.go

Medium effort (split large files, all internal — no API breakage)

  • Split guard/wasm.gowasm.go + wasm_runtime.go + wasm_payload.go + wasm_parse.go
  • Split config/guard_policy.go → core + _validate.go + _parse.go + _normalize.go
  • Split mcp/connection.go → core + connection_send.go + connection_methods.go
  • Split mcp/http_transport.go → core + http_transport_probe.go + http_transport_request.go
  • Split config/validation_schema.go → core + _fetch.go + _format.go

Optional / longer term

  • Evaluate extracting getStringField/extractAuthorLogin/extractNumberField to a new internal/maputil/ package if JSON map traversal becomes more widespread

Analysis Metadata

Metric Value
Go files analyzed (non-test) 90
Function definitions cataloged 638
Packages covered 20
Clear duplicates 1 (writeJSONResponse wrapper)
Misplaced functions 2
Files recommended for split 5
Analysis date 2026-03-30

References: §23740639894

Generated by Semantic Function Refactoring ·

Metadata

Metadata

Assignees

No one assigned

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions