[typist] Typist - Go Type Consistency Analysis #25077
Closed
Replies: 1 comment
-
|
This discussion has been marked as outdated by Typist - Go Type Analysis. A newer discussion is available at Discussion #25280. |
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 659 non-test Go files across
pkg/identified 2 genuine cross-package name collisions, 3 intentional build-constraint duplicates (correctly separated), and approximately 1,825any-typed locations — the vast majority justified by YAML/JSON dynamism. Below are the actionable findings worth addressing.Summary
map[string]anyusagesanystruct fieldsinterface{}usagesFull Analysis Report
Duplicated Type Definitions
Cluster 1:
EngineConfig— Cross-Package Name Collision (Medium Priority)Type: Semantic name collision across packages (different purposes)
Locations:
pkg/workflow/engine.go:16— runtime engine configurationpkg/cli/audit_expanded.go:19— audit display recordDefinition Comparison:
Issue: The two types are in different packages (
workflowvscli) and have distinct purposes: one configures the engine at runtime, the other represents the engine identity for audit reporting. The shared name is misleading — developers browsing or searching forEngineConfigmay find the wrong one.Recommendation: Rename
pkg/cli/audit_expanded.go's type toAuditEngineConfigorEngineAuditRecordto reflect its audit-display purpose and eliminate the name ambiguity.pkg/cli)Cluster 2:
MCPServerConfig— Intentional Design (Already Addressed)Locations:
pkg/workflow/tools_types.go:366— workflow-specific fieldspkg/parser/mcp.go:35— parser-specific fieldsBoth embed
types.BaseMCPServerConfigfrompkg/types/mcp.go, which already consolidates the shared fields. This is correct architecture — no action required.Cluster 3: Build-Constraint Pairs (Intentional — No Action Required)
The following type pairs are intentional platform stubs using
//go:build !js && !wasm///go:build js || wasmsplit:SpinnerWrapperpkg/console/spinner.go:96pkg/console/spinner_wasm.go:10ProgressBarpkg/console/progress.go:31pkg/console/progress_wasm.go:7RepositoryFeaturespkg/workflow/repository_features_validation.go:57pkg/workflow/repository_features_validation_wasm.go:5These follow the standard Go build-tag pattern for platform stubs. ✅
Untyped Usages
Category 1:
type X anyType Aliases (Low Priority)Location:
pkg/workflow/tools_types.go:275This represents a sum type (
string | []string) that Go cannot express natively. A custom struct with ajson.Unmarshalerwould add clarity:Category 2: Struct Fields with
anyfor Variable-Type JSON (Acceptable / Documented)These fields use
anyfor intentional type flexibility, all with comments explaining why:pkg/workflow/frontmatter_types.goEngine anystringor object in YAMLpkg/cli/gateway_logs.go:194,205ID anystring | number | nullpkg/cli/logs_models.go:300,301RunID any,RunNumber anypkg/cli/copilot_setup.go:147,155RunsOn any,On anyruns-oncan be string or arrayThese are correct usage of
anyin Go. No change recommended.Category 3:
map[string]anyin Struct Fields (Medium Priority)Location:
pkg/cli/trial_types.goSafeOutputs,AgenticRunInfo, andAdditionalArtifactscarry dynamic JSON blobs from workflow outputs. If the schemas for these are known, typed structs would improve usability at call sites. However, if the structure is genuinely variable per workflow, the current approach is appropriate.Recommendation: Investigate whether
AgenticRunInfohas a stable schema (it may already mirrorAwInfoinpkg/cli/logs_models.go). If so:Category 4:
map[string]anyfor JSON Output Building (Low Priority)Location:
pkg/cli/deps_report.go:213This nested map is only used for JSON marshaling output. A typed struct would provide better compile-time guarantees:
Category 5:
map[string]anyfor YAML Frontmatter (Accepted Pattern)The
pkg/workflow/andpkg/parser/packages usemap[string]anyextensively (~1,600 usages) for YAML frontmatter manipulation. This is the correct Go pattern for working with dynamic YAML structures where the schema is partially known or varies by user configuration. The team has already documented the rationale in comments throughoutpkg/workflow/frontmatter_types.go.Notable well-structured workarounds already in place:
MapToolConfig(pkg/workflow/mcp_config_types.go:66) — named type alias formap[string]anywith helper methodsDevcontainerFeatures(pkg/cli/devcontainer.go:40) — named type alias for clarityparseRuntimesConfig,parsePermissionsConfig) to convert raw maps to strongly-typed structsRefactoring Priority Summary
cli.EngineConfig→AuditEngineConfigpkg/cli/audit_expanded.go:19WorkflowTrialResultfieldspkg/cli/trial_types.goGitHubReposScopewith custom unmarshalerpkg/workflow/tools_types.go:275DepsReportOutputstructpkg/cli/deps_report.go:213Implementation Checklist
cli.EngineConfigtoAuditEngineConfigand update all usages withinpkg/cli/WorkflowTrialResult.AgenticRunInfoschema — determine if it matchesAwInfomap[string]anyfields inWorkflowTrialResultwith typed structsGitHubReposScopeto replace theanyaliasDepsReportOutputstruct indeps_report.goAnalysis Metadata
pkg/)interface{}usages: 0 (clean — fully migrated toany)anytype usages: ~1,825 (most intentional YAML/JSON handling)EngineConfig,MCPServerConfig)MCPServerConfig: 0 (already addressed viaBaseMCPServerConfig)References:
Beta Was this translation helpful? Give feedback.
All reactions