Skip to content

[refactor] Semantic Function Clustering Analysis — Outliers & Duplicates #2758

@github-actions

Description

@github-actions

Overview

Analysis of 89 non-test Go source files across internal/ (~625 function signatures catalogued) reveals three categories of actionable refactoring opportunities: one exact cross-package function duplicate, one redundant trivial wrapper, and one thin delegation function that obscures its origin. Overall the codebase is well-organized — findings are focused and low-risk.


1. Exact Duplicate: writeJSONResponse

Issue: Identical function defined in two separate packages.

Location Line
internal/server/http_helpers.go 22
internal/proxy/handler.go 356

Implementation (both are byte-for-byte identical):

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

Recommendation: Extract to a shared internal/httputil package, or move it to one package and have the other import it. The server and proxy packages are both HTTP-facing; a small shared httputil package would be a natural home and follows the existing pattern of strutil/, syncutil/, timeutil/, etc.

Estimated effort: 30 minutes
Impact: Eliminates a copy-paste hazard (a bug fix in one would silently miss the other).


2. Redundant Private Wrapper: getAgentTagsSnapshotFromContext

Issue: A private function that is a one-liner delegating to a public function defined three lines below it in the same file.

File: internal/mcp/connection.go, lines 42–44

// private wrapper
func getAgentTagsSnapshotFromContext(ctx context.Context) (*AgentTagsSnapshot, bool) {
    return GetAgentTagsSnapshotFromContext(ctx)
}

// public function (same file, line 48)
func GetAgentTagsSnapshotFromContext(ctx context.Context) (*AgentTagsSnapshot, bool) { ... }

The private wrapper is called from exactly one place (line 374 of the same file) and adds no value — the caller could directly call the exported function.

Recommendation: Remove getAgentTagsSnapshotFromContext and replace its single call-site with a direct call to GetAgentTagsSnapshotFromContext.

Estimated effort: 5 minutes
Impact: Removes dead indirection; cleaner reading of the file.


3. Thin Delegation: ValidateDIFCMode in cmd Package

Issue: ValidateDIFCMode lives in internal/cmd/flags_difc.go but is only a one-liner wrapping difc.ParseEnforcementMode.

// internal/cmd/flags_difc.go:93
func ValidateDIFCMode(mode string) error {
    _, err := difc.ParseEnforcementMode(mode)
    return err
}
```

It is exported (PascalCase) but used only within `internal/cmd` (called in `root.go:214` and `proxy.go:118`). The `difc` package already owns this logic with `ParseEnforcementMode`.

**Recommendation**: Either:
- **Inline it**: Replace the two call-sites (`root.go:214`, `proxy.go:118`) with direct calls to `difc.ParseEnforcementMode` and discard the wrapper, **or**
- **Unexport it**: Rename to `validateDIFCMode` since it is never called outside the `cmd` package.

The inline approach is slightly cleaner; the unexport approach is safer as a first step.

**Estimated effort**: 10 minutes  
**Impact**: Reduces indirection; makes clear that DIFC mode validation belongs to the `difc` package.

---

## 4. Single-Function File: `guard.go`

**Observation** (low priority): `internal/guard/guard.go` contains a single unexported helper function `emptyAgentLabelsResult` alongside the `Guard` interface definition.

```
internal/guard/
├── context.go      (4 funcs)
├── guard.go        (1 func + interface defs)   ← single helper
├── noop.go         (5 funcsimplements Guard)
├── registry.go     (11 funcs)
├── wasm.go         (24 funcsimplements Guard)
└── write_sink.go   (5 funcsimplements Guard)

emptyAgentLabelsResult is only called within wasm.go. Moving it to wasm.go or keeping it in guard.go (co-located with the Guard interface it supports) are both reasonable — the current placement is not wrong, just worth noting.

Recommendation: No immediate action required. If guard.go grows more helpers used only by wasm.go, consider consolidation.


Function Inventory Summary

Package Files Functions Notes
internal/difc 8 ~90 Well-organized by responsibility
internal/config 9 (+rules/) ~85 Feature-registry pattern is clean
internal/logger 10 ~75 Good split by output format
internal/mcp 6 ~65 connection.go + http_transport.go are large but purposeful
internal/server 14 ~90 Well-split by concern
internal/guard 5 ~50 Healthy; see note #4
internal/cmd 8 ~50 Good use of init()-based flag registration
internal/proxy 6 ~45 handler.go could benefit from #1
internal/launcher 3 ~25 log_helpers.go extraction is good practice
Other (auth, envutil, strutil, syncutil, timeutil, tty, sys, version, middleware) 10 ~30 Appropriately sized utility packages

Total: 89 files · ~625 functions


Refactoring Recommendations (Prioritized)

Priority 1 — High Impact / Low Risk

  1. Create internal/httputil package containing writeJSONResponse — eliminates exact duplicate across server and proxy packages.
  2. Remove getAgentTagsSnapshotFromContext private wrapper in mcp/connection.go — trivial cleanup.

Priority 2 — Medium Impact

  1. Inline or unexport ValidateDIFCMode in cmd/flags_difc.go — reduce unnecessary indirection.

Priority 3 — Long-term Observation

  1. Monitor guard.go single-helper situation — no action needed now.

Implementation Checklist

  • Create internal/httputil/httputil.go with shared WriteJSONResponse; update server/http_helpers.go and proxy/handler.go to use it (note: Go requires exported name for cross-package use)
  • Remove getAgentTagsSnapshotFromContext from mcp/connection.go; update its one call-site on line 374
  • Inline ValidateDIFCMode call-sites or rename to validateDIFCMode
  • Run make agent-finished to verify build, lint, and tests pass after each change

Analysis Metadata

Attribute Value
Total Go files analyzed 89 (non-test)
Total functions catalogued ~625
Exact duplicates found 1 (writeJSONResponse)
Trivial wrappers found 2 (getAgentTagsSnapshotFromContext, ValidateDIFCMode)
Outliers (wrong file) 0
Analysis method Naming pattern analysis + signature comparison + call-site tracing
Analysis date 2026-03-29

References: §23706954635

Generated by Semantic Function Refactoring ·

Metadata

Metadata

Assignees

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