[typist] Typist - Go Type Consistency Analysis #25455
Closed
Replies: 1 comment
-
|
This discussion has been marked as outdated by Typist - Go Type Analysis. A newer discussion is available at Discussion #25624. |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
-
Analysis of repository: github/gh-aw — run §24188100594
Executive Summary
666 non-test Go source files across 20 packages in
pkg/were analyzed, yielding 654 type definitions. The codebase demonstrates strong typing discipline overall — mostanyandmap[string]anyusages are intentional and well-documented. Three genuine naming conflicts were identified where different packages define a type with the same name but different structure and semantics. A handful ofmap[string]anystruct fields and one namedanyalias represent actionable refactoring opportunities.Counts at a glance:
any-typed struct fields: 10 (of which ~4 are actionable)map[string]anyvariable usages: 267 (majority in codemod YAML transformation functions)Full Analysis Report
Duplicated Type Definitions
Summary Statistics
Cluster 1:
RepoConfig— Completely Unrelated Types Sharing a NameType: Semantic conflict (different concepts, same name)
Impact: HIGH — two structs named
RepoConfigthat are totally unrelatedLocation 1 —
pkg/cli/trial_types.go:24Purpose: Trial run repository targeting
Location 2 —
pkg/workflow/repo_config.go:69Purpose: Repository-level maintenance feature settings (parsed from
aw.json)Recommendation:
pkg/cli/trial_types.go:RepoConfig→TrialRepoTargets(orTrialRepos)RepoConfigshould be reserved for the canonical workflow repo-level configpkg/cli/)Cluster 2:
EngineConfig— Audit DTO vs Runtime ConfigurationType: Semantic conflict (reporting model vs. configuration model)
Impact: MEDIUM — two unrelated structs in different packages with the same name
Location 1 —
pkg/cli/audit_expanded.go:19Purpose: Audit report data transfer object
Location 2 —
pkg/workflow/engine.go:16Purpose: Runtime engine configuration used by the compiler
Recommendation:
pkg/cli/audit_expanded.go:EngineConfig→AuditEngineRecord(it's a report DTO, not a config)Cluster 3:
MCPServerConfig— Shared Base, Different ExtensionsType: Near-duplicate (both embed
types.BaseMCPServerConfig)Impact: LOW-MEDIUM — the shared base already exists in
pkg/types/mcp.go; name collision can cause confusion when reading importsLocation 1 —
pkg/workflow/tools_types.go:366Purpose: Workflow compilation — runtime server deployment
Location 2 —
pkg/parser/mcp.go:35Purpose: MCP server inspection — used when introspecting tool capabilities
Recommendation:
BaseMCPServerConfigviapkg/types/mcp.gois already correctworkflow.MCPDeploymentConfigandparser.MCPInspectionConfigfor clarityIntentional WASM Build-Constrained Pairs (not refactoring targets)
These types are intentionally duplicated using
//go:builddirectives to provide platform-appropriate implementations:ProgressBarpkg/console/progress.gopkg/console/progress_wasm.goSpinnerWrapperpkg/console/spinner.gopkg/console/spinner_wasm.goRepositoryFeaturespkg/workflow/repository_features_validation.gopkg/workflow/repository_features_validation_wasm.goAll use
//go:build !js && !wasm///go:build js || wasmconstraints. This is correct Go idiom for conditional compilation.Untyped Usages
Summary Statistics
anytype alias (type X any)any-typed struct fields (intentional — spec compliance)map[string]anystruct fields (potentially typeable)map[string]anytype aliasesmap[string]anyvariable usagesCategory 1: Named
anyAlias —GitHubReposScopeLocation:
pkg/workflow/tools_types.go:275Used in
GitHubToolConfig:Analysis: The named alias documents intent but provides no type safety. The comment says it's
string | []any, meaning at runtime callers must type-assert.Recommendation:
canonicalReposScopeincache_integrity.go)Category 2:
map[string]anyStruct Fields — Trial Result TypesLocation:
pkg/cli/trial_types.go:9-12Analysis: These fields hold structured data from trial workflow runs.
SafeOutputsandAgenticRunInfohave known shapes from the rest of the codebase (SafeOutputsConfig,LogMetrics, etc.).Recommendation:
SafeOutputs: Replace withmap[string]stringif values are always strings, or define aTrialSafeOutputstructAgenticRunInfo: Consider*workflow.LogMetricsor a dedicatedAgenticRunSummarystructAdditionalArtifacts: May remainmap[string]anyif truly open-endedCategory 3:
map[string]anyin MCP Registry —MCPRegistryServerEntry.ConfigLocation:
pkg/cli/mcp_registry.go:28Analysis: MCP registry server configs are populated with
env,command,args, andheaderspatterns. These match the shape oftypes.BaseMCPServerConfig.Recommendation:
types.BaseMCPServerConfigfieldsmap[string]anywithtypes.BaseMCPServerConfigCategory 4: Intentional
anyStruct Fields (do not refactor)These usages are documented and spec-compliant. They should not be changed:
Engine anypkg/workflow/frontmatter_types.go:151stringor engine object; documented comment explains JSON unmarshal constraintPorts anypkg/workflow/service_ports.go:50int,string, or[]mixed; documentedID anypkg/cli/gateway_logs.go:194,205On anypkg/cli/list_workflows_command.go:25,status_command.go:31,copilot_setup.go:155on:field is polymorphicRunsOn anypkg/cli/copilot_setup.go:147runs-on:accepts string or arrayRunID any,RunNumber anypkg/cli/logs_models.go:300-301Category 5: Untyped Constants — Rate Limit Defaults
Location:
pkg/constants/job_constants.go:185-186The codebase already uses the pattern
type LineLength intinpkg/constants/constants.gofor semantic typed constants.Recommendation:
LineLengthpattern; prevents unit confusion (is 60 seconds? minutes? runs?)Refactoring Recommendations
Priority 1:
RepoConfigName CollisionAction: Rename
pkg/cli/trial_types.go:RepoConfig→TrialRepoTargetsSteps:
RepoConfiginpkg/cli/(only trial-related code should use it)trial_types.gopkg/cli/trial_command.goand related filesEstimated effort: 30 min | Risk: Low
Priority 2:
EngineConfigin audit_expanded.goAction: Rename
pkg/cli/audit_expanded.go:EngineConfig→AuditEngineRecordSteps:
audit_expanded.goaudit_expanded.goand any consumersengine_idetc., not the Go name)Estimated effort: 30 min | Risk: Low
Priority 3:
GitHubReposScope anyaliasAction: Replace with a proper union type supporting YAML unmarshaling
Steps:
type GitHubReposScope []stringwith a customUnmarshalYAMLthat handles single-string YAML valuescanonicalReposScopeinpkg/workflow/cache_integrity.go:131(already takesGitHubReposScope)Estimated effort: 2-3 hours | Risk: Medium (affects YAML parsing)
Priority 4:
WorkflowTrialResultweakly-typed fieldsAction: Replace
map[string]anywith typed structs where shapes are knownSteps:
SafeOutputsandAgenticRunInfoTrialSafeOutputs,AgenticRunSummary) or use existing typesEstimated effort: 2-4 hours | Risk: Medium (affects trial result parsing)
Implementation Checklist
cli.RepoConfig→TrialRepoTargets(Priority 1)cli.EngineConfig(in audit_expanded.go) →AuditEngineRecord(Priority 2)GitHubReposScope anywith typed union (Priority 3)WorkflowTrialResultmap fields (Priority 4)MCPRegistryServerEntry.Config→types.BaseMCPServerConfig(Low priority)DefaultRateLimitMax/DefaultRateLimitWindow(Low priority)Analysis Metadata
any-typed struct fieldsmap[string]anyvariable usagesReferences:
Beta Was this translation helpful? Give feedback.
All reactions