Typist - Go Type Consistency Analysis #23221
Closed
Replies: 1 comment
-
|
This discussion was automatically closed because it expired on 2026-03-28T11:23:39.600Z.
|
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 622 non-test Go files across 18 packages in
pkg/. The codebase already demonstrates strong typing discipline —pkg/constantsuses semantic type aliases extensively, and apkg/typesshared package exists to prevent cross-package duplication. The findings below focus on the remaining opportunities.Summary
map[string]anyusages[]anyusagesanystruct fieldsFull Analysis Report
Duplicated Type Definitions
Cluster 1:⚠️
EngineConfig— Naming CollisionType: Same name, completely different semantics across packages
Impact: Medium — reading code that imports both
cliandworkflowpackages requires context-switching; the nameEngineConfigcarries conflicting meaning.Locations:
pkg/cli/audit_expanded.go:19— Audit-display struct (JSON-tagged, read-only view)pkg/workflow/engine.go:15— Runtime engine configuration struct (mutable, operational)Definition Comparison:
Recommendation: Rename
pkg/cli/audit_expanded.go'sEngineConfigtoEngineAuditSnapshotorEngineAuditInfo. Theclipackage type represents a point-in-time audit view, not the operational configuration. This removes the ambiguity without any functional change.pkg/cli)Cluster 2:
MCPServerConfig— Intentionally Split (Already Addressed) ✅Locations:
pkg/workflow/tools_types.go:493— Workflow-specific (Mode, Toolsets, GuardPolicies, CustomFields)pkg/parser/mcp.go:35— Parser-specific (Name, Registry, ProxyArgs, Allowed)Both embed
types.BaseMCPServerConfigfrompkg/types/mcp.go. Thepkg/typespackage documentation explicitly explains this design: the shared fields live inBaseMCPServerConfig; each package extends with its own domain-specific fields. This is the correct pattern — no action needed.Cluster 3: WASM Platform Stubs — Expected Pattern ✅
Three type pairs exist because Go build constraints require separate files for WASM targets:
RepositoryFeaturespkg/workflow/repository_features_validation.go:57pkg/workflow/repository_features_validation_wasm.go:5SpinnerWrapperpkg/console/spinner.go:96pkg/console/spinner_wasm.go:10ProgressBarpkg/console/progress.go:31pkg/console/progress_wasm.go:7The WASM stubs contain only the fields accessible without native OS dependencies. This is idiomatic Go and requires no change.
Cluster 4:
FileTracker— Interface vs. Struct (Not a Duplicate) ✅pkg/workflow/compiler_types.go:51—type FileTracker interface { TrackCreated(filePath string) }pkg/cli/file_tracker.go:18—type FileTracker struct { ... }(implements the interface)This is the correct Go interface/implementation pattern. The struct in
pkg/clisatisfies the interface inpkg/workflow. No action needed.Cluster 5:
LogMetrics— Type Alias (Correct Pattern) ✅pkg/workflow/metrics.go:24— Canonical definitionpkg/cli/logs_models.go:68—type LogMetrics = workflow.LogMetrics(type alias for re-export)Type aliases are the idiomatic Go approach for re-exporting types without introducing a new type. No action needed.
Untyped Usages
Category 1:
map[string]any— YAML/JSON Parsing (1,423 occurrences)Impact: Low — largely intentional for dynamic YAML/JSON documents
The vast majority of
map[string]anyusages appear in:pkg/parser/) — YAML documents are structurally variable;map[string]anyis the standard Go approach for generic unmarshaling.pkg/cli/codemod_*.go) — These traverse arbitrary YAML structures.pkg/parser/tools_merger.go) — TheMergeToolsfunction must handle arbitrary tool-config shapes.Select opportunities for stronger typing:
1a.
DevcontainerFeaturespkg/cli/devcontainer.go:39type DevcontainerFeatures map[string]anymap[string]any{}(empty option maps) or maps of string settings. The feature keys are well-known URIs but values vary per feature.map[string]anyis appropriate here because devcontainer features have a heterogeneous, externally-defined schema. No change recommended.1b.
PostTransformFuncin codemod factorypkg/cli/codemod_factory.go:8type PostTransformFunc func(lines []string, frontmatter map[string]any, fieldValue any) []stringfieldValue anyparameter represents a YAML field value that could be a string, number, bool, map, or slice —anyis appropriate. No change recommended.1c.
WorkflowMCPMetadata.Frontmatterpkg/cli/mcp_workflow_scanner.go:21Frontmatter map[string]any1d. JSON log parsing in
logs_metrics.gopkg/cli/logs_metrics.go:651,698var logEntries []map[string]anyLogEntrystruct with the known fields (mcp_servers, etc.) could replace the map, making the code safer and more readable.Category 2:
[]any— YAML Array Parsing (292 occurrences)Most
[]anyusages are in YAML array traversal where element types are heterogeneous (mixed strings, maps, etc.). These are appropriate for the YAML parsing context.Notable exception:
pkg/parser/yaml_import.go:203GitHub Actions steps are always
[]map[string]any— tightening to[]map[string]anywould eliminate the inner type assertion. Low-effort, low-risk improvement.Category 3:
anyStruct Fields — JSON-RPC IDsLocation:
pkg/cli/gateway_logs.go:194,205Assessment: The JSON-RPC 2.0 specification requires that
idbe a string, number, or null. Usinganyis correct here becausejson.Unmarshalneeds to handle all three. Alternatives likejson.RawMessagewould add complexity without meaningful benefit. No change recommended.Category 4: Untyped File-Scope Constants
The
pkg/constantspackage already uses semantic type aliases extensively (e.g.,Version,CommandPrefix,URL,LineLength). A small number of file-scope constants outside that package lack types:pkg/cli/gateway_logs.go:35maxScannerBufferSize = 1024 * 1024int(implicit)constants.ByteSizeif that type exists, or keep as-is (local clarity)pkg/cli/secret_set_command.go:34publicKeySize = 32int(implicit)intor a namedPublicKeySizeconst — low prioritypkg/cli/audit_cross_run.go:13maxAuditReportRuns = 50int(implicit)pkg/cli/audit_comparison.go:84maxAuditComparisonCandidates = 10int(implicit)pkg/cli/audit_diff.go:17volumeChangeThresholdPercent = 100.0float64(implicit)constants.Percenttype could apply; moderate clarity improvementThese constants are all unexported (lowercase) and file-scoped, so their lack of explicit types has low blast radius. The existing typed constant pattern in
pkg/constantsalready handles the high-value cases.Refactoring Recommendations
Priority 1 (Medium): Rename
EngineConfiginpkg/cliAction: Rename
EngineConfiginpkg/cli/audit_expanded.gotoEngineAuditSnapshot(orEngineAuditInfo) to eliminate the naming collision withpkg/workflow.EngineConfig.Steps:
pkg/cli/audit_expanded.gopkg/cli/(likely audit-related files only)Estimated effort: 1 hour
Impact: Eliminates reader confusion; no behavioral change
Priority 2 (Low): Type JSON log entry structs in
logs_metrics.goAction: Replace
var logEntries []map[string]anywith typedLogEntrystructs.Steps:
LogEntryandMCPServerEntrystructs with known fieldsmcp_serversEstimated effort: 2–3 hours
Impact: 4 fewer type assertions; compile-time field validation
Priority 3 (Low): Tighten YAML step arrays in
yaml_import.goAction: Change
[]anyto[]map[string]anyfor step slices inpkg/parser/yaml_import.go.Estimated effort: 30 minutes
Impact: Minor — removes one level of type assertion
Implementation Checklist
EngineConfiginpkg/cli/audit_expanded.go→EngineAuditSnapshotLogEntrystruct for JSON log parsing inlogs_metrics.go[]anyto[]map[string]anyfor YAML step arrays inyaml_import.govolumeChangeThresholdPercentandmaxScannerBufferSizeconstantsAnalysis Metadata
map[string]anyoccurrences[]anyoccurrencesanystruct fieldsReferences: §23643710895
Beta Was this translation helpful? Give feedback.
All reactions