Skip to content

[refactor] Consolidate duplicated model-normalization helpers (pkg/cli vs pkg/modelsdev)Β #43270

Description

@github-actions

πŸ”§ Semantic Function Clustering Analysis

Repository: github/gh-aw β€” analysis of non-test .go files under pkg/

Overview

Serena + naming-pattern analysis across 998 non-test Go files (~15k functions). The codebase is, on the whole, well organized: pkg/cli has zero duplicated unexported function names across its files, the codemod_*.go family follows a strict one-file-per-feature rule (62 files), and _wasm.go build-tag variants are correctly used for conditional compilation rather than copy-paste drift.

One genuine, actionable duplication cluster stands out, plus several patterns worth acknowledging as correct so they are not mistakenly "fixed" later.

Key finding β€” duplicated model-normalization helpers (pkg/cli ↔ pkg/modelsdev)

pkg/cli/model_costs.go already imports pkg/modelsdev and delegates pricing lookups to modelsdev.FindPricing (model_costs.go:215). Yet it re-implements two helpers that are byte-for-byte copies of unexported helpers in pkg/modelsdev/catalog.go:

Helper pkg/cli pkg/modelsdev Status
normalizeComparableModelID model_costs.go:154 catalog.go:225 Exact duplicate
normalizeCatalogProvider / normalizeProvider model_costs.go:145 catalog.go:216 Identical logic, different name
Code comparison
// pkg/cli/model_costs.go:154   AND   pkg/modelsdev/catalog.go:225  (identical)
func normalizeComparableModelID(value string) string {
	return strings.NewReplacer(".", "-", "_", "-").Replace(strings.ToLower(strings.TrimSpace(value)))
}

// pkg/cli/model_costs.go:145  (normalizeCatalogProvider)
// pkg/modelsdev/catalog.go:216 (normalizeProvider)  β€” same body
func normalize...Provider(provider string) string {
	switch strings.ToLower(strings.TrimSpace(provider)) {
	case "github", "copilot", "github_models":
		return "github-copilot"
	default:
		return strings.ToLower(strings.TrimSpace(provider))
	}
}

Why it happened: the modelsdev helpers are unexported, so pkg/cli copied them instead of reusing them. This is the classic "functional duplicate across a package boundary" β€” the two copies can silently diverge (e.g. a new provider alias added to one but not the other).

Recommendation: promote the canonical versions in pkg/modelsdev to exported functions (NormalizeComparableModelID, NormalizeProvider) and have pkg/cli/model_costs.go call them, deleting its local copies. model_costs.go already depends on modelsdev, so this adds no new coupling.

Impact: removes a divergence hazard in pricing/model-matching logic; low effort (~1 hr), no new import edges.

Patterns that are correct (do NOT flag as duplicates)

These surfaced as same-named functions but are intentional and should be left alone:

  • _wasm.go build-tag variants β€” github_cli.go/github_cli_wasm.go, git_helpers.go/git_helpers_wasm.go, remote_fetch.go/remote_fetch_wasm.go define the same symbols (RunGH, findGitRoot, ResolveIncludePath, ...) under mutually-exclusive build constraints. Conditional compilation, not duplication.
  • Thin delegating wrappers β€” workflow.SanitizeName β†’ stringutil.SanitizeName, and workflow.compileSchema β†’ parser.CompileSchema. These already centralize the real implementation; the local wrapper just preserves package-local call sites. Good pattern.

Positive organization signals

  • codemod_*.go: 62 files, one codemod per file β€” exemplary one-file-per-feature organization.
  • pkg/cli: no unexported function name is defined more than once across its files.

Next actions

  • Export NormalizeComparableModelID / NormalizeProvider from pkg/modelsdev; replace the copies in pkg/cli/model_costs.go and add a small test asserting parity.
  • (Optional, low priority) Audit waitForServer vs waitForServerReady in pkg/cli for consolidation.

Analysis metadata

  • Non-test Go files analyzed: 998 (pkg/)
  • Approx. functions cataloged: ~15,000
  • Genuine cross-package duplicates confirmed: 2 (1 cluster)
  • Build-variant / delegating false-positives correctly excluded: 5+
  • Method: Serena semantic analysis + naming-pattern clustering + manual verification
  • Analysis date: 2026-07-04

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

  • expires on Jul 5, 2026, 4:12 PM UTC-08:00

Metadata

Metadata

Type

No type

Fields

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