Skip to content

[refactor] 🔧 Semantic Function Clustering Analysis #11413

Description

@github-actions

Executive Summary

Analyzed all 148 non-test .go files under internal/ (excluding _test.go). Overall the codebase is well-organized: most packages already extract shared logic into dedicated helper files (e.g. internal/difc/tagset.go backing both Label and Capabilities, and internal/httputil/response_writer.go embedded by internal/server/response_writer.go). No open [refactor] issues existed prior to this run, so none were closed. This pass focuses on the smaller number of genuine opportunities found: near-duplicate string-formatting helpers, and a couple of naming/organization nits. No functions were clearly "in the wrong file" at a level worth flagging — file-per-feature discipline is followed consistently (e.g. validation_*.go, flags_*.go, config_*.go families).

Function Inventory (highlights)

  • internal/logger/ — 13 files, heavy use of generics (bindGlobalLogger[T], initAndSetGlobalLogger[T]) to unify logger lifecycle code across FileLogger, MarkdownLogger, RPCLogger; well-factored already.
  • internal/difc/ — 12 files implementing DIFC labels/evaluator; tagset.go already centralizes set operations reused by Label (labels.go) and Capabilities (capabilities.go) via embedding — a good existing example of the "extract common" pattern.
  • internal/config/ — validation logic is split cleanly across validation_rules.go, validation_server.go, validation_schema.go, validation_gateway.go, validation_env.go, validation_tracing.go; each has a focused purpose.
  • internal/util/format.go, truncate.go, json.go, netutil.go, random.go are small, single-purpose utility files — good centralization, no scatter found.
  • internal/httputil/response_writer.go + internal/server/response_writer.go — correct example of shared base type + package-specific extension (not a duplicate).

Identified Issues

1. Near-duplicate "format tag level to human string" helpers

  • File: internal/difc/violations.go
  • Functions: formatIntegrityLevel(tags []Tag) string (line 125) and formatSecrecyLevel(tags []Tag) string (line 158)
  • Observation: Both scan a []Tag, strip a :-scope suffix, and build a human-readable label string, differing only in the specific level vocabulary (merged/approved/unapproved vs. private:<scope>/public). They are already co-located in the same file, which is good, but their shared scanning/stripping logic (strings.Index(s, ":") splitting) could be extracted into a small shared helper (e.g. splitTagScope(tag Tag) (base, scope string)) to reduce duplicated string-parsing logic and ease future additions of new tag categories.
  • Recommendation: Low-priority micro-refactor — extract the tag-scope-splitting logic into one shared unexported helper used by both formatters. Not urgent since both are only ~30 lines and already colocated.
  • Estimated Impact: Minor; improves consistency if a third label category is ever added.

2. Two error_format / FormatConfigError-style helpers in different packages

  • internal/config/error_format.go: FormatConfigError(err error) string, AppendConfigDocsFooter(sb *strings.Builder)
  • internal/config/validation_schema.go: formatSchemaError(err error) error, formatValidationErrorRecursive(...), formatErrorContext(...)
  • Observation: Both revolve around turning validation/schema errors into user-facing strings, but they solve different sub-problems (top-level docs footer vs. recursive JSON-schema error tree formatting) and already live in separate, appropriately named files within the same package. This is acceptable separation, not duplication — flagged only for visibility, no action needed.

3. File organization is already strong — no relocated-function outliers found

Reviewed candidate "risky" areas (validation logic inside server/, parsing inside mcp/, scattered helpers) and found no functions clearly misplaced:

  • internal/server/http_helpers.go contains only HTTP-request-adjacent helpers (rejectRequest, readAndRestoreRequestBody, logHTTPRequestBody) — correctly scoped to the server package rather than being validation logic.
  • internal/mcp/pagination.go and internal/mcp/helpers.go contain only MCP-protocol pagination/marshaling helpers — no stray unrelated functions.
  • internal/githubhttp/rate_limit.go cleanly separates ParseRateLimitResetHeader (from HTTP header) vs. ParseRateLimitResetFromText (from error text) — same concern, two well-named entry points, not true duplicates.

Refactoring Recommendations

Priority 3: Low-impact, optional

  1. Extract shared tag-scope-splitting helper in internal/difc/violations.go to consolidate the common substring logic between formatIntegrityLevel and formatSecrecyLevel.
    • Estimated effort: <1 hour
    • Benefit: Marginal reduction in duplicated parsing logic; easier to extend if new label categories are added.

Analysis Metadata

  • Total Go Files Analyzed: 148 (non-test, under internal/)
  • Total Functions Cataloged: ~900+ (sampled representative packages: logger, difc, config, util, sanitize, githubhttp, envutil, server, mcp, httputil)
  • Function Clusters Identified: ~15 (logging, DIFC labels, config validation, HTTP helpers, string/format utilities, rate-limiting, sanitization)
  • Outliers Found: 0 clear file-misplacement cases
  • Duplicates Detected: 1 near-duplicate pair (low severity, same file)
  • Detection Method: Static grep-based symbol enumeration + targeted pairwise code comparison (Serena MCP tools were not available in this environment; analysis performed via direct source inspection)
  • Analysis Date: 2026-08-17

Generated by Semantic Function Refactoring · auto · 49.1 AIC · ⊞ 14.4K ·

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions