Typist - Go Type Consistency Analysis #23095
Closed
Replies: 1 comment
-
|
This discussion has been marked as outdated by Typist - Go Type Analysis. A newer discussion is available at Discussion #23221. |
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.
-
This report analyzes all 619 non-test Go source files in
pkg/for duplicated type definitions and untyped usages. The codebase is generally well-structured with strong typing, a dedicatedpkg/typesshared package, and an active migration away frommap[string]anyviaToolsConfig. The main opportunities for improvement are naming collisions between package-local types and untyped numeric constants, while the heavymap[string]anyusage in YAML frontmatter parsing is partially justified but worth continued migration effort.Stats: 619 files · 594 type definitions · 5 true duplicate name clusters · ~1,423
map[string]anyusages · 3 untyped numeric constantsFull Analysis Report
Duplicated Type Definitions
Summary Statistics
Cluster 1:⚠️
EngineConfig— Naming CollisionType: Naming collision — two unrelated types sharing the same name in different packages
Occurrences: 2
Impact: Medium — developers reading cross-package code may confuse the two
pkg/workflow/engine.go:15ID,Version,Model,MaxTurns,Firewall,Args,Envpkg/cli/audit_expanded.go:19EngineID,EngineName,CLIVersion,MCPServers,TriggerEventThese are semantically different — the workflow type drives compilation while the CLI type drives audit report rendering. Both are used within their own packages and never imported cross-package (verified by grep), so there is no actual type conflict. However, the shared name creates cognitive friction.
Recommendation:
cli.EngineConfig→AuditEngineInfoorRunEngineInfoto clarify intentCluster 2:⚠️
LogParser— Naming CollisionType: Naming collision — fundamentally different type shapes (interface vs generic function type)
Occurrences: 2
Impact: Low — used only within respective packages, no cross-package confusion
pkg/workflow/agentic_engine.go:169interface { ParseLogMetrics(...) LogMetrics; GetLogParserScriptId() string }pkg/cli/log_aggregation.go:30type LogParser[T LogAnalysis] func(logPath string, verbose bool) (T, error)These are entirely unrelated concepts. The function type in CLI could be renamed to
LogFileParserorLogAggregatorFn.Recommendation:
cli.LogParser[T]→LogFileParser[T]to reduce ambiguityCluster 3:
MCPServerConfig— Extension Pattern ✅ (Intentional)Type: Designed extension using shared
pkg/types.BaseMCPServerConfigOccurrences: 2 (plus base in
pkg/types)pkg/parser/mcp.go:35Name,Registry,ProxyArgs,Allowed(parser-specific)pkg/workflow/tools_types.go:493Mode,Toolsets,GuardPolicies,CustomFields(workflow-specific)Both embed
types.BaseMCPServerConfigfrom the sharedpkg/typespackage. This is the correct architecture. No action needed.Cluster 4:
RepositoryFeatures/ProgressBar/SpinnerWrapper— Build Stubs ✅ (Intentional)These use the standard Go build-constraint pattern where a WASM stub replaces the full implementation at compile time:
No action needed — this is idiomatic Go platform-specific code.
Cluster 5:
LogMetrics— Proper Type Alias ✅ (Intentional)The CLI package re-exports via alias for ergonomic use. No action needed.
Untyped Usages
Summary Statistics
interface{}usages: 0 (none — codebase uses modernany)anyusages (total): ~1,824[T any](idiomatic, not a concern): majoritymap[string]anyfor frontmatter: ~1,423 (see below)Category 1:
map[string]anyin YAML Frontmatter ParsingImpact: Medium — ongoing technical debt, but active migration in progress
The workflow compiler parses GitHub Actions workflow YAML via
map[string]anybecause YAML is inherently dynamic at parse time. This is a known trade-off; the team has already begun the migration withToolsConfig.Top files by occurrence:
pkg/workflow/compiler_safe_outputs_config.gopkg/workflow/frontmatter_types.gopkg/workflow/safe_outputs_tools_filtering.gopkg/workflow/compiler_orchestrator_workflow.gopkg/workflow/trigger_parser.gopkg/workflow/tools.gopkg/workflow/call_workflow_validation.goMigration in progress:
pkg/workflow/tools_types.godocuments the pattern explicitly and providesToolsConfigas the strongly-typed alternative. The file even includes the migration path with backward-compat wrappers. Thefrontmatter_types.gofile shows 9 typed structs have already been extracted (RuntimesConfig,GitHubActionsPermissionsConfig, etc.) alongside the remaining raw map usage.Recommendation:
ToolsConfigmigration pattern for additional frontmatter sections (triggers, permissions, concurrency blocks)trigger_parser.go(31 occurrences) andcall_workflow_validation.go(26 occurrences) as the next candidatesExample — current pattern:
Suggested direction:
Category 2: Untyped Numeric Constants⚠️
Impact: Low — reduces semantic clarity and allows accidental mixing of unrelated values
These two constants represent different domains (count vs minutes) but share the same inferred type
int. They could be mixed accidentally.Recommendation:
Note:
LineLengthtype is already defined and used forMaxExpressionLineLengthandExpressionBreakThreshold— the same pattern should apply here.Category 3:
anyin Generic Constraints and Variadics ✅ (Idiomatic)Both patterns are idiomatic Go. Generic type constraints and variadic
...anyfollowingfmt.Printfconventions are correct usage. No action needed.Refactoring Recommendations
Priority 1: Medium —
EngineConfigNaming CollisionAction: Rename
cli.EngineConfigtoAuditEngineInfoSteps:
pkg/cli/audit_expanded.go:19pkg/cli/(verified to not cross intopkg/workflow/)Estimated effort: 30 minutes | Impact: Medium (developer clarity)
Priority 2: Low —
LogParserNaming CollisionAction: Rename
cli.LogParser[T]toLogFileParser[T]Steps:
pkg/cli/log_aggregation.go:30pkg/cli/Estimated effort: 15 minutes | Impact: Low (developer clarity)
Priority 3: Medium — Continue
map[string]anyMigrationAction: Extend
ToolsConfigmigration pattern to triggers and permissionsSteps:
TriggersConfigstruct followingRuntimesConfigpattern infrontmatter_types.gotrigger_parser.goto use the new typeEstimated effort: 2–4 hours per module | Impact: High long-term
Priority 4: Low — Add Types to Numeric Constants
Action: Apply existing
LineLengthpattern to rate-limit and threshold constantsEstimated effort: 30 minutes | Impact: Low (semantic clarity)
Implementation Checklist
cli.EngineConfig→AuditEngineInfo(Priority 1)cli.LogParser[T]→LogFileParser[T](Priority 2)TriggersConfigtyped struct (Priority 3)trigger_parser.goto use typed config (Priority 3)ToolsConfig-pattern migration for remaining frontmatter sectionsAnalysis Metadata
pkg/)map[string]anyoccurrences: ~1,423 (across ~150 files)References:
Beta Was this translation helpful? Give feedback.
All reactions