Skip to content

[refactor] Semantic function clustering: 8 verified de-duplication opportunities in pkg/cli and pkg/workflow #45853

Description

@github-actions

🔧 Semantic Function Clustering Analysis

Analysis of repository: github/gh-aw · 2026-07-16

Analyzed the non-test Go sources under pkg/ (~1,098 files; the two dominant packages are pkg/workflow with 452 files and pkg/cli with 356). Function clusters were extracted by naming pattern, then each candidate was verified by reading the actual function bodies — only confirmed, actionable findings are listed below. The codebase is generally well-consolidated (many util helpers are already delegated to correctly), so this focuses on the concrete remaining duplication.

Overview

Metric Value
Packages analyzed 33 under pkg/
Primary hotspots pkg/cli, pkg/workflow
Confirmed findings 8 (5 high-confidence)
Detection method Naming-cluster extraction + body-level verification

Critical actions (high confidence)

  1. Collapse the near-identical int parsers in pkg/workflow/engine_config_parser.go — 5 functions, 2 clusters, differ only by a log string / comparison operator.
  2. Consolidate 4 reimplementations of "workflow-ID-from-path" in pkg/cli onto the existing stringutil.NormalizeWorkflowName.
  3. Unify the 3 copies + 4 inline copies of the ${{ }}-expression predicate in pkg/workflow.
  4. De-duplicate the two byte-identical timeline row renderers in pkg/cli/gateway_logs_timeline_render.go.
1. Duplicate — near-identical integer parsers (pkg/workflow/engine_config_parser.go) · high

parseMaxRunsValue (L29) and parseMaxTurnCacheMissesValue (L44) are byte-for-byte identical except the log-message literal:

func parseMaxRunsValue(raw any) int {
    if val, ok := typeutil.ParseIntValue(raw); ok && val > 0 {
        return val
    }
    if rawStr, ok := raw.(string); ok {
        if parsed, err := strconv.Atoi(rawStr); err == nil && parsed > 0 {
            return parsed
        }
        engineLog.Printf("Ignoring invalid max-runs value: %q", rawStr) // only difference
    }
    return 0
}

A second near-identical cluster: parseMaxTurnsValue (L57), parseMaxToolDenialsValue (L104), parseNonNegativeIntOrExpressionValue (L84) — all ParseIntValue → strconv.Atoi(trimmed) → ${{ }} passthrough → log, differing only by the log string and >= 0 vs > 0.

Fix: collapse each cluster into one helper parameterized by (fieldName string, minValue int, allowExpression bool).

2. Scattered helper — "workflow-ID-from-path" reimplemented 4× (pkg/cli) · high

Canonical helper already exists: stringutil.NormalizeWorkflowName (pkg/stringutil/identifiers.go:29). Four local functions each hand-roll "base filename minus workflow extensions":

  • pkg/cli/logs_format_compact.go:18 workflowIDFromPath
  • pkg/cli/evals_branch.go:66 workflowIDFromRunPath
  • pkg/cli/workflows.go:172 extractWorkflowNameFromPath
  • pkg/cli/forecast_metadata.go:221 extractWorkflowIDFromName
// logs_format_compact.go:24-29
base = strings.TrimSuffix(base, ".lock.yml")
base = strings.TrimSuffix(base, ".yml")
base = strings.TrimSuffix(base, ".yaml")

Fix: extend NormalizeWorkflowName to also strip .yml/.yaml/.lock and delete the ad-hoc variants. normalizeWorkflowID (workflows.go:512) already delegates correctly — use it as the model.

3. Scattered helper — `${{ }}` expression predicate defined 3× + inlined 4× (pkg/workflow) · high

Same predicate (prefix ${{ + suffix }}, after TrimSpace) in three places:

  • isExpressionpkg/workflow/expression_patterns.go:84
  • isGitHubActionsExpressionpkg/workflow/observability_otlp.go:104
  • isGitHubExpressionpkg/workflow/publish_assets.go:17 (regex form)

Plus inline copies in engine_config_parser.go:72,96,116 and safe_outputs_config_base.go:25.

Fix: keep the canonical isExpression in expression_patterns.go, delete the other two, replace the inline copies.

4. Duplicate — two byte-identical timeline row renderers (pkg/cli/gateway_logs_timeline_render.go) · high

renderAgentAssistantMessageRow (L325) and renderAgentReasoningRow (L338) are identical except one constant:

func renderAgentAssistantMessageRow(evt UnifiedTimelineEvent) []string {
    ts := formatTimelineTime(evt)
    src := timelineSourceLabel(evt.Source)
    kind := timelineEventIcon(TimelineKindAssistantMessage) + " " + timelineEventKindLabel(TimelineKindAssistantMessage)
    detail := stringutil.Truncate(evt.MessageContent, 48)
    return []string{ts, src, kind, detail, ""}
}
// renderAgentReasoningRow: identical, but TimelineKindReasoning

Fix: collapse into one helper taking the TimelineKind as a parameter.

5. Scattered helper — inline os.Stat existence probes instead of fileutil (pkg/cli) · high

pkg/fileutil already provides FileExists and DirExists; several sites re-probe with os.Stat:

  • redacted_domains.go:91,101,112 — 3× if _, err := os.Stat(directPath); err == nil (discards error) → fileutil.FileExists
  • forecast_metadata.go:117-121 — candidate-path existence loop → fileutil.FileExists
  • firewall_policy.go:460os.Stat + !info.IsDir() guard → !fileutil.DirExists(...) (file already imports fileutil)
  • compile_repository_manifest.go:105 — README existence probe → fileutil.FileExists (medium; the check at :94 legitimately needs os.IsNotExist — leave it)

Fix: replace the pure existence probes with fileutil.FileExists/fileutil.DirExists.

6. Scattered helper — manual map-key collect-and-sort instead of sliceutil.SortedKeys (pkg/workflow) · medium

sliceutil.SortedKeys is already used 97× across 47 files here, yet a few sites hand-roll it:

  • lsp_manager.go:36-40 and permissions_operations.go:434-437 — pure collect + sort.Strings (clean SortedKeys cases)
  • cache.go:872-875, safe_jobs_needs_validation.go:161-164 — same shape with a per-key transform (SortedKeys + sliceutil.Map)

Fix: use sliceutil.SortedKeys in the clean cases.

7. In-file repetition — timeline row preamble copied across 13 renderers (pkg/cli/gateway_logs_timeline_render.go) · medium

The 3-line preamble (ts/src/kind) appears in 13 render*Row functions, and the "server/tool detail (truncated)" block is copied in renderGatewayToolCallRow:143, renderGatewayDIFCFilteredRow:173, renderGatewayGuardPolicyBlockedRow:198, renderAgentToolStartRow:284, renderAgentToolDoneRow:303.

Fix: extract newTimelineRow(evt, kind) and serverToolDetail(evt, maxLen) helpers in the same file.

8. Duplicate — shared section blocks between compact/verbose log renderers (pkg/cli/logs_format_compact.go) · medium

renderLogsCompact (L56) and renderLogsCompactVerbose (L225) share byte-identical [mcp-failures] (:196:357), [missing-tools] (:204:365), and [location] (:212:382) blocks.

Fix: extract shared renderCompactMCPFailures/MissingTools/Location(data) helpers used by both.

Explicitly rejected (verified as correct by-design, not duplicates)

  • containsWorkflowDispatch/containsWorkflowCall already delegate to containsTrigger; dedupeAllowedTools wraps sliceutil.Deduplicate; truncateSHAForLog wraps stringutil.Truncate; parseRolesValue/parseBotsValue delegate to parseStringSliceAny.
  • resolveDigestViaBuildx/Crane/Pull, render*DiffMarkdownSection vs render*DiffPrettySection, the sanitize* family, and getRepositorySlugFrom* — all genuinely distinct logic/output.
  • map[string]bool set-membership is not replaceable by setutil.Contains (needs map[K]struct{}).

Next actions

  • P1 (high, ~1–2h): items 1–5 — pure mechanical de-duplication, low risk, existing tests cover the behavior.
  • P2 (medium, ~1–2h): items 6–8 — in-file helper extraction.
  • Run go test ./pkg/cli/... ./pkg/workflow/... after each consolidation to confirm no behavior change.

References: §29460289143

Generated by 🔧 Semantic Function Refactoring · 568.1 AIC · ⌖ 16.1 AIC · ⊞ 9.4K ·

  • expires on Jul 17, 2026, 4:17 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