Skip to content

[refactor] Semantic function clustering: consolidate duplicated log parsers, reinvented utils, and redundant engine overridesΒ #42851

Description

@github-actions

πŸ”§ Semantic Function Clustering Analysis

Analysis of github/gh-aw β€” read-only semantic + naming-pattern clustering across the pkg/ tree.

Overview

Analyzed 986 non-test .go files (~15.1k func declarations), concentrated in pkg/workflow (423 files) and pkg/cli (337 files). The codebase is generally well-factored β€” most repeated function names are legitimate interface implementations (RenderMCPConfig, GetSecretValidationStep, ...) that already delegate to shared builders, //go:build js||wasm stub pairs, or thin wrappers that already call the canonical util (sliceutil.SortedKeys, stringutil.Truncate, repoutil.SplitRepoSlug). Those were checked and excluded as false positives.

What remains is a small set of genuine, actionable duplicates and reinvented utilities. Each item below was confirmed by reading both function bodies.

Key findings (by impact)

# Category Location(s) Action
1 Duplicate log parser gemini_logs.go:22 ↔ antigravity_logs.go:22 Extract one shared stats-JSONL parser
2 Duplicate type GeminiResponse ↔ AntigravityResponse Single shared statsJSONResponse
3 Redundant overrides Get{LogFileForParsing,DefaultDetectionModel} on Gemini/Antigravity Delete β€” identical to BaseEngine default
4 Reinvented git helpers pr_command.go:259 Call existing getCurrentBranch / createAndSwitchBranch
5 Reinvented sliceutil.SortedKeys logs_report_firewall.go:74, codemod_dependabot_permissions.go:242 Delete, call util
6 Duplicate byte formatters console/format.go:6 ↔ console/progress_shared.go:6 Collapse to one formatter
7 Reinvented repo-slug split update_version_labels.go:124, dispatch_workflow_validation.go:176 Route through repoutil.SplitRepoSlug
8 Reinvented stdlib/util intent/resolver.go:170 cloneStrings; parser/schema_suggestions.go:231 wrappers slices.Clone / call stringutil.*

Priority 1 β€” high-confidence, low-risk

1 & 2. Gemini and Antigravity log parsing are near-identical duplicates
  • GeminiEngine.ParseLogMetrics β€” pkg/workflow/gemini_logs.go:22
  • AntigravityEngine.ParseLogMetrics β€” pkg/workflow/antigravity_logs.go:22

Both share the same LogMetrics{} init, the same strings.SplitSeq line loop, the same token extraction from Stats["models"] (input_tokens/output_tokens as float64), and the same Stats["tools"] β†’ toolCallCounts aggregation. Antigravity merely extracted the inner logic into parseAntigravityResponseLine / applyAntigravityResponseMetrics.

Their response structs are byte-for-byte identical:

  • GeminiResponse β€” pkg/workflow/gemini_logs.go:14
  • AntigravityResponse β€” pkg/workflow/antigravity_logs.go:14
type GeminiResponse struct {
    Response string         `json:"response"`
    Stats    map[string]any `json:"stats"`
}

Recommendation: Introduce a shared statsJSONResponse type and a parseStatsJSONLLogMetrics(logContent, ...) helper; have both engines delegate. Also route the hand-rolled for toolName, count := range toolCallCounts { append ToolCallInfo{...} } loops (gemini_logs.go:80, antigravity_logs.go:53) through the existing FinalizeToolMetrics(...) helper that PiEngine.ParseLogMetrics (pkg/workflow/pi_logs.go:100) already uses.

3. Redundant engine overrides identical to the BaseEngine default

Both engines embed BaseEngine and override two methods with the exact value the base already returns:

  • GetLogFileForParsing β†’ constants.AgentStdioLogPath
    • pkg/workflow/gemini_logs.go:99, pkg/workflow/antigravity_logs.go:113
    • Base default: pkg/workflow/agentic_engine.go:374
  • GetDefaultDetectionModel β†’ ""
    • pkg/workflow/gemini_logs.go:105, pkg/workflow/antigravity_logs.go:119
    • Base default: pkg/workflow/agentic_engine.go:361

Recommendation: Delete all four overrides; inheritance yields identical behavior. (Verify no interface-satisfaction requirement forces the explicit override before removing.)

4. pr_command.go reimplements existing git.go helpers

applyPatchToRepo (pkg/cli/pr_command.go:259) inlines exec.Command("git", "branch", "--show-current") β€” duplicating getCurrentBranch (pkg/cli/git.go:445) β€” and at pr_command.go:294 inlines exec.Command("git", "checkout", "-b", branchName) with an identical error wrap β€” duplicating createAndSwitchBranch (pkg/cli/git.go:465).

Recommendation: Behavior-preserving reuse of the existing getCurrentBranch() / createAndSwitchBranch() helpers.

5. Reinvented sliceutil.SortedKeys
  • convertDomainsToSortedSlices (pkg/cli/logs_report_firewall.go:74) manually ranges each map[string]struct{} then slices.Sorts β€” exactly sliceutil.SortedKeys (pkg/sliceutil/sliceutil.go:63).
  • sortedRemainingPermissionKeys (pkg/cli/codemod_dependabot_permissions.go:242) is a pure passthrough: keys := sliceutil.SortedKeys(remaining); return keys.

Recommendation: Replace both with direct sliceutil.SortedKeys(...) calls and delete the wrappers. (Its sibling sortedMissingPermissionKeys does a PermissionScope→string conversion and is justified — keep it.)

Priority 2 β€” medium-confidence cleanups

6. Two byte-size formatters in pkg/console
  • FormatFileSize(int64) β€” pkg/console/format.go:6 β†’ "1.5 KB" (spaced, up to TB)
  • formatBytes(int64) β€” pkg/console/progress_shared.go:6 β†’ "1.5KB" (compact, up to GB)

Same package, same job, only cosmetic output differences.

Recommendation: Collapse into one formatter (add a compact bool param if the progress-bar format is deliberate) so byte formatting lives in one place.

7. Local owner/repo splitters duplicating repoutil.SplitRepoSlug

repoutil.SplitRepoSlug (pkg/repoutil/repoutil.go:15) is canonical. Local reimplementations:

  • splitOwnerRepo β€” pkg/cli/update_version_labels.go:124 (SplitN/2 + empty checks, bool return)
  • parseRepoSlugLiteral β€” pkg/workflow/dispatch_workflow_validation.go:176 (same split + whitespace rejection)

pkg/cli/engine_secrets.go:696 splitRepoSlug already delegates to repoutil β€” the pattern to follow.

Recommendation: Route both through repoutil.SplitRepoSlug (adapt error→bool), keeping parseRepoSlugLiteral's whitespace strictness as a thin pre-check.

8. Reinvented stdlib / stringutil
  • cloneStrings β€” pkg/intent/resolver.go:170: hand-rolled make+copy; replace with stdlib slices.Clone (note the nil vs non-nil-empty return difference).
  • FindClosestMatches / LevenshteinDistance β€” pkg/parser/schema_suggestions.go:231,241: one-line wrappers shadowing the identical public stringutil.FindClosestMatches / stringutil.LevenshteinDistance. Call stringutil.* at the sites (keep only if the added debug log is valued).
Also near-duplicate (lower priority)
  • ghAPIGet / ghAPIGetArray (pkg/cli/outcome_eval.go:225,248) differ only in unmarshal target (map vs []map). Extract a shared ghAPIGetRaw(endpoint, repo) ([]byte, error) or a generic ghAPIGet[T].
  • GenerateCopilotInstallerSteps (pkg/workflow/copilot_installer.go:11) ↔ GenerateAntigravityInstallerSteps (pkg/workflow/antigravity_installer.go:12): same installer-step shape; extract generateCliInstallerStep(...). Parameterize carefully β€” Copilot has an extra ExpressionPattern security branch + GH_HOST env.
  • Outlier: durationStringToSeconds (pkg/workflow/mcp_renderer.go:58) is a generic time converter in an MCP-rendering file, used once β€” candidate to move to a time util (low priority).

Explicitly ruled out (not duplicates)

False positives verified and excluded
  • Repeated engine methods (RenderMCPConfig, GetSecretValidationStep, GetModelEnvVarName, GetExecutionSteps, GetSquidLogsSteps, ...) β€” interface polymorphism already delegating to shared builders.
  • validate* / docker* / getDefaultGHHost "duplicates" β€” //go:build js||wasm vs !js&&!wasm stub pairs (intentional).
  • AddMetrics, Error, Unwrap, UnmarshalJSON, String name collisions β€” distinct interface impls on different types.
  • Already-delegating wrappers (correct design): dedupeAllowedToolsβ†’sliceutil.Deduplicate, mergeUniqueLoggedβ†’sliceutil.MergeUnique, truncateSHAForLogβ†’stringutil.Truncate, engine_secrets.splitRepoSlugβ†’repoutil.SplitRepoSlug, resolveImportPath/checkCleanWorkingDirectory methodβ†’free-func wrappers.
  • No stray local min/max/contains/unique/fileExists reimplementations remain in non-test code.
  • The ~90 codemod_*.go files are one-codemod-per-file by design; template structural similarity was not flagged.

Suggested next actions

Analysis metadata

  • Non-test Go files analyzed: 986 Β· Dominant packages: pkg/workflow (423), pkg/cli (337)
  • Detection method: function-signature clustering (duplicate-name + naming-pattern) with per-finding body-diff confirmation
  • False-positive filtering: interface impls, build-tag stubs, and existing delegating wrappers excluded
  • Analysis date: 2026-07-02

References: Β§28555945946

Generated by πŸ”§ Semantic Function Refactoring Β· 663 AIC Β· βŒ– 16 AIC Β· ⊞ 9.2K Β· β—·

  • expires on Jul 3, 2026, 4:19 PM UTC-08:00

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions