Skip to content

[refactor] Consolidate duplicate expression-detection checks and inline Truncate/Deduplicate/MapKeys reinventions #40701

Description

@github-actions

🔧 Semantic Function Clustering Analysis

Analysis of github/gh-aw — non-test Go source under pkg/ (excluding _test.go, testdata/, pkg/linters/, and _wasm.go build-tag variants).

Overview

This run clustered functions by name/purpose and used semantic body comparison to surface duplicates, reinvented shared utilities, and misplaced ("outlier") functions. The codebase is already well-organized — the "one file per feature" convention holds tightly, and dedicated util packages (setutil, sliceutil, stringutil, jsonutil, typeutil) exist and are widely used. Findings below are the verified, high-value exceptions: every location was read and confirmed, not just name-matched.

Key Metrics

Category Confirmed findings
Duplicate predicate (${{ }} expression check) 3 functions + 4 inline copies
Inline reinvention of stringutil.Truncate 8 sites
Inline reinvention of sliceutil.Deduplicate 3 sites
Inline reinvention of sliceutil.MapKeys ~14 sites
Misplaced (outlier) functions 2 high-confidence

1. Duplicate predicate: "is this a ${{ ... }} expression?"

Three standalone functions in pkg/workflow express the same predicate, and the identical check is inlined at 4+ more call sites.

Locations & bodies
// pkg/workflow/expression_patterns.go:84
func isExpression(s string) bool {
    return strings.HasPrefix(s, "${{") && strings.HasSuffix(s, "}}")
}
  • isGitHubExpressionpkg/workflow/publish_assets.go:17 (trim, then regex ^\$\{\{.*\}\}$)
  • isGitHubActionsExpressionpkg/workflow/observability_otlp.go:108 (trim, then prefix/suffix)

Inlined copies of strings.HasPrefix(x, "${{") && strings.HasSuffix(x, "}}"):

  • pkg/workflow/safe_outputs_config.go:802
  • pkg/workflow/engine_config_parser.go:73 and :93
  • pkg/workflow/expression_parser.go:20

Recommendation: Consolidate into one exported helper (e.g. IsGitHubExpression(value string) bool that trims then checks prefix/suffix) and replace the three functions + inline sites. ~90% / functional similarity.


2. Inline reinvention of stringutil.Truncate

stringutil.Truncate(s, n) already produces exactly s[:n-3] + "..." when len(s) > n (verified). These sites hand-roll it:

8 confirmed sites
# Location Current code Replace with
1 pkg/workflow/workflow_errors.go:67 v[:97]+"..." if len>100 stringutil.Truncate(v, 100)
2 pkg/workflow/workflow_errors.go:174 identical 2nd copy stringutil.Truncate(v, 100)
3 pkg/workflow/template_injection_utils.go:296 trimmed[:97]+"..." if len>100 stringutil.Truncate(trimmed, 100)
4 pkg/workflow/safe_outputs_steps_shell_expansion_validation.go:156 raw[:57]+"..." if len>60 stringutil.Truncate(raw, 60)
5 pkg/cli/gateway_logs_render.go:102 reason[:77]+"..." if len>80 stringutil.Truncate(reason, 80)
6 pkg/cli/gateway_logs_render.go:125 message[:57]+"..." if len>60 stringutil.Truncate(message, 60)
7 pkg/cli/logs_format_tsv.go:145 displayTitle[:47]+"..." if len>50 stringutil.Truncate(displayTitle, 50)
8 pkg/cli/mcp_tool_table.go:62 description[:TruncateLength-3]+"..." stringutil.Truncate(description, opts.TruncateLength)

⚠️ Not a clean match (do not auto-replace): pkg/cli/logs_download.go:831 does msg[:200]+"..." (keeps 200 + ellipsis); Truncate(msg,200) keeps only 197.

Recommendation: Replace items 1–8 with stringutil.Truncate. Behaviorally exact; lowest-risk fixes in this report.


3. Inline reinvention of sliceutil.Deduplicate / sliceutil.MapKeys

sliceutil.Deduplicate — 3 order-preserving seen-map dedupe loops
  • pkg/parser/schema_errors.go:179 (wantTypes)
  • pkg/parser/schema_errors.go:426 (allSuggestions)
  • pkg/cli/compile_schedule_calendar.go:44 (combined []int)
sliceutil.MapKeys — ~14 map-key-extraction loops (each followed by sort.Strings, so undefined order is fine)

pkg/workflow/call_workflow_secrets.go:83, engine_helpers.go:232 & :193, universal_llm_consumer_engine.go:139, compiler_yaml.go:728, dependabot.go:802, runtime_step_generator.go:135 & :172, repo_memory_prompt.go:151, pkg/cli/redacted_domains.go:66, logs_report_firewall.go:75/:80/:194, audit_report_experiments.go:200, compile_watch.go:203, interactive.go:615.

Pattern in each: make([]T,0,len(m)); for k := range m { append }; sort.Strings(...)keys := sliceutil.MapKeys(m); sort.Strings(keys).

Recommendation: Deduplicate sites are exact drop-ins (high value). MapKeys sites are a larger but mechanical sweep — lower priority, do as one batch.


4. Outlier functions (in the wrong file)

2 high-confidence outliers

parseYAMLMapKeypkg/cli/codemod_steps_run_secrets_env.go:492
A generic YAML key: value line parser reused by 5 unrelated files (4 other codemods + add_command.go), but defined in the narrowest one. → Move to pkg/cli/yaml_frontmatter_utils.go, which already holds the sibling line-level helpers (getIndentation, isTopLevelKey, isNestedUnder, ...).

escapeForYAMLDoubleQuotedpkg/workflow/expression_parser.go:491
A pure YAML double-quoted-scalar escaper sitting in a boolean-expression tokenizer/parser file. Its test already lives in yaml_encoding_test.go, and its sibling escapeYAMLSingleQuoted lives in strings.go. → Move to pkg/workflow/yaml.go (or strings.go).

Lower-confidence (noted, not recommended): extractStepOutput / findFirstFailingStep in pkg/cli/audit.go:972,1009 are raw job-log parsers in the audit orchestrator; a dedicated logs-parsing file would help only if reused.


Checked & already clean (no action)

Categories verified as already consolidated
  • Set membership uses setutil.Contains; no hasStringKey/stringInSlice clones remain (the earlier hasStringKey cluster was already resolved).
  • sliceutil.Deduplicate/MergeUnique are used (e.g. dedupeAllowedTools); the many bespoke seen-map loops that trim/filter/validate are not clean drop-ins and were excluded.
  • jsonutil.MarshalCompactNoHTMLEscape — no raw SetEscapeHTML(false) reinventions in scope.
  • Pointer helpers, fileutil.FileExists/DirExists, and mapsEqual (test-only) — no non-test duplicates.
  • The 9× repeated engine methods (GetExecutionSteps, RenderMCPConfig, ...) are legitimate interface implementations, not duplicates.

Next Actions (prioritized)

  1. P1 (exact, low-risk): §2 items 1–8 (Truncate) and §3 Deduplicate (3 sites).
  2. P1: §1 consolidate the ${{ }} expression predicate into one helper.
  3. P2 (mechanical batch): §3 MapKeys sweep (~14 sites).
  4. P2: §4 move the 2 outlier functions to their themed files.

Analysis Metadata

  • Scope: pkg/ non-test Go, excluding testdata/, pkg/linters/, _wasm.go variants (~850 files).
  • Method: name-pattern clustering + read-and-verify body comparison against shared util packages.
  • Every reported location was opened and confirmed; name-only matches were rejected.
  • Analysis date: 2026-06-22.

References: §27921777971

Generated by 🔧 Semantic Function Refactoring · 662.1 AIC · ⌖ 11.2 AIC · ⊞ 9.1K ·

  • expires on Jun 23, 2026, 4:17 PM UTC-08:00

Metadata

Metadata

Assignees

No one assigned

    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