Typist: Go Type Consistency Analysis — github/gh-aw #22871
Closed
Replies: 1 comment
-
|
This discussion has been marked as outdated by Typist - Go Type Analysis. A newer discussion is available at Discussion #23095. |
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.
-
Analyzed 614 non-test Go files across
pkg/for duplicated type definitions and untyped usages. The codebase demonstrates strong type-safety awareness — apkg/types/shared package exists,FrontmatterConfigprovides structured YAML parsing, andEngineName/LineLengthcustom types are used throughout. The analysis surfaces 3 duplicate clusters (all with clear rationale) and 7 concrete untyped usage opportunities, quantified below.Full Analysis Report
Duplicated Type Definitions
Summary Statistics
Cluster 1: WASM Build-Tag Stubs (Intentional — No Action Needed)
Types:
SpinnerWrapper,ProgressBar,RepositoryFeaturesPattern: Idiomatic Go platform-specific implementation via build tags
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:5The WASM stubs carry
//go:build js || wasmand provide no-op implementations. This is the correct Go pattern for platform-specific behaviour. No refactoring needed.Cluster 2: MCPServerConfig — Shared-Base Pattern (Already Resolved ✓)
Types:
parser.MCPServerConfig,workflow.MCPServerConfigStatus: ✅ Already uses
types.BaseMCPServerConfigembeddingBoth types embed
types.BaseMCPServerConfigfrompkg/types/mcp.goand add domain-specific fields:The shared
BaseMCPServerConfigcorrectly captures common fields. No further action needed.Cluster 3: Name Collisions — Different Abstractions (Rename Consideration)
Types:
FileTracker,LogParser,LogMetricsappearing in bothpkg/cliandpkg/workflowpkg/clipkg/workflowFileTrackerfile_tracker.go:18)compiler_types.go:51)LogParserfunc(logPath string, verbose bool) (T, error)(log_aggregation.go:30)agentic_engine.go:169)LogMetrics= workflow.LogMetrics(logs_models.go:67)metrics.go:24)Recommendation for
FileTracker: The interface inpkg/workflow/compiler_types.gois narrow (onlyTrackCreated). Renaming it toFileCreationTrackerwould avoid confusion with the full-featured CLI struct and clarify the interface's narrower contract.Recommendation for
LogParser: The CLI generic function type and workflow interface serve different call sites. Renaming the workflow interface toLogAnalyzerorLogProcessorwould remove the naming ambiguity.Untyped Usages
Summary Statistics
interface{}usages: 0 (none found — codebase uses modernanyalias exclusively ✓)anystruct fields: 8 locationsmap[string]anystruct fields (non-frontmatter): 6 locationsCategory 1:
any-Typed Struct FieldsExample 1:
AwInfo.RunIDandAwInfo.RunNumberpkg/cli/logs_models.go:275-276AwInforepresentsaw_info.jsonfiles. Theanytype accommodates GitHub Actions sending either a string or integer for these fields across different versions. All otherRunIDfields in the codebase areint64.json.Numberwhich safely represents both string-form and numeric JSON values without type assertion:anymarshaling, callers can use.Int64()or.String()explicitly, no more silent type assertion failuresExample 2:
WorkflowListItem.OnandWorkflowStatus.Onpkg/cli/list_workflows_command.go:25—On anypkg/cli/status_command.go:31—On anyon:field is intentionally polymorphic (string,[]string, ormap[string]any). Theanytype is accurate for the raw YAML value. However, both structs share this identical polymorphic field withconsole:"-"(hidden from output).Onasconsole:"-"— it is never displayed, only used for programmatic filtering (labelsFieldlookup atlist_workflows_command.go:183). Consider whether this field needs to be present in the serialized struct at all, or whether a helper method for label extraction would be cleaner.Example 3: JSON-RPC
IDFieldspkg/cli/gateway_logs.go:140,151idto be a string, number, or null —anyis correct here. No action needed.Example 4:
WorkflowStep.ContinueOnErrorpkg/workflow/step_types.go:25ContinueOnError any— GitHub Actions allows this to be a boolean literal OR a string expression ($\{\{ ... }}).Category 2:
map[string]anyStruct FieldsExample 1:
FrontmatterConfigLegacy Fieldspkg/workflow/frontmatter_types.go:154-157MCPServersandRuntimesare deprecated — documentation says to useToolsandRuntimesTypedinstead. These fields remain for backward compatibility.Jobsis documented as "too dynamic to type".MCPServersspecifically, sinceparser.MCPServerConfigandworkflow.MCPServerConfigalready exist, typing this asmap[string]workflow.MCPServerConfigcould be feasible but would require migration logic. Low priority given deprecation path.Example 2: Trial Result Fields
pkg/cli/trial_command.go:32-35TrialResultis a JSON output capturing dynamic workflow execution data. Themap[string]anyaccurately represents dynamically-shaped output. Acceptable as-is, but a typedSafeOutputsstruct would improve programmatic use.Example 3:
MapToolConfigpkg/workflow/mcp_config_types.go:66type MapToolConfig map[string]anytools_types.go:13notes this is a "structured alternative to the pervasivemap[string]anypattern" — the documentation acknowledges the trade-off. Acceptable.Category 3: Untyped Numeric Constants
pkg/constants/constants.goDefaultRateLimitWindow = 60— the comment says "minutes" but the value is unitless.DefaultMCPGatewayPayloadSizeThreshold = 524288lacks unit context in the name.Refactoring Recommendations
Priority 1: Medium —
json.NumberforAwInfo.RunID/RunNumberRecommendation: Replace
anywithjson.NumberinAwInfoSteps:
pkg/cli/logs_models.go:275-276Estimated effort: 1-2 hours
Impact: Medium — improves serialization safety for aw_info.json parsing
Priority 2: Low — Rename Ambiguous Interface/Type Names
Recommendation: Rename
workflow.FileTrackerinterface toFileCreationTrackerSteps:
pkg/workflow/compiler_types.go:51cli.FileTrackerstruct is unaffected)Estimated effort: 30-60 minutes
Impact: Low — code clarity improvement, no behavioral change
Priority 3: Low — Add Units to Numeric Constants
Recommendation: Rename
DefaultRateLimitWindowtoDefaultRateLimitWindowMinutesSteps:
pkg/constants/constants.go:766Estimated effort: 30 minutes
Impact: Low — prevents unit confusion in future development
Implementation Checklist
AwInfo.RunID/RunNumber anywithjson.Number(pkg/cli/logs_models.go)workflow.FileTrackerinterface →FileCreationTrackerfor clarityLogParserinterface inpkg/workflowto avoid collision with CLI generic typeDefaultRateLimitWindow→DefaultRateLimitWindowMinutesin constantsDefaultMCPGatewayPayloadSizeThresholdto include unit (Bytes)WorkflowListItem.On any— if only used for label extraction, consider replacing with a helper methodAnalysis Metadata
interface{}usagesanystruct field locationsmap[string]anystruct fieldsRecommendations
The codebase is in good shape overall — it already uses
pkg/types/mcp.gofor shared types,FrontmatterConfigfor structured YAML, and typed constants for most semantic values. The highest-value change is replacingAwInfo.RunID/RunNumber anywithjson.Numberto improve JSON marshaling safety. Renaming theworkflow.FileTrackerinterface and ambiguousLogParsertype would reduce reader confusion at low cost.References:
Beta Was this translation helpful? Give feedback.
All reactions