Typist: Go Type Consistency Analysis #23673
Closed
Replies: 1 comment
-
|
This discussion has been marked as outdated by Typist - Go Type Analysis. A newer discussion is available at Discussion #23860. |
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 Go type consistency across the
pkg/directory ofgithub/gh-aw.The codebase is overall well-typed — most previously problematic patterns (like
MCPServerConfig) have already been addressed via a sharedpkg/typesbase. The main opportunities center on pervasivemap[string]anyfor frontmatter and a cluster of untyped string/numeric constants that would benefit from typed grouping.Full Analysis Report
Duplicated Type Definitions
Summary Statistics
EngineConfig)SpinnerWrapper,ProgressBar,RepositoryFeatures— WASM vs non-WASM)Cluster 1:
EngineConfig— Name Collision (Different Semantics)Type: Semantic name collision
Impact: Medium — same name in the same
clipackage scope (different file) vsworkflowpackageLocations:
pkg/workflow/engine.go:15— runtime engine configuration (ID, Model, MaxTurns, Args, Env, etc.)pkg/cli/audit_expanded.go:19— audit-report DTO (EngineID, EngineName, CLIVersion, MCPServers, etc.)Definition comparison:
Current state:
Suggested fix: Introduce a type alias in
pkg/workflow(orpkg/types) to document intent without changing behavior:Benefits: Grep-ability, self-documentation, easier future migration to a typed struct, no behavioral change required
Estimated effort: 2–3 hours (mechanical rename)
Category 2: Untyped Artifact Name Constants
Impact: Medium — artifact names mixed with other string constants, no compile-time check prevents wrong string being used as an artifact name
Location:
pkg/constants/constants.go:635–670Current state:
Suggested fix:
Benefits: Prevents accidentally passing a filepath or other string where an artifact name is expected; all 8 constants self-document as artifact names
Estimated effort: 1–2 hours (update usage sites to cast when passing to string-accepting APIs)
Category 3: Untyped Job Output Name Constants
Impact: Medium — output names interchangeable with arbitrary strings
Location:
pkg/constants/constants.go:736–746Current state:
Suggested fix: Introduce
type OutputName string(similar to the existingJobName,StepIDpatterns already in the file):Benefits: Consistent with existing
JobName/StepIDtyped constant patterns already in the fileEstimated effort: 1–2 hours
Category 4: Untyped Numeric/Bool Constants
Impact: Low — numeric constants without units or type constraints
Location:
pkg/constants/constants.goCurrent state:
Suggested fixes:
Benefits: Avoids unit confusion (already bit the project once based on comment style), prevents accidental type mismatches
Estimated effort: 30 minutes
Category 5:
DevcontainerFeatures map[string]any— Partially Addressed ✅Location:
pkg/cli/devcontainer.go:39Current state:
Status: The type alias already exists and is a good pattern — it documents intent. The
mergeFeatureshelper still takesmap[string]anyfor the second argument rather thanDevcontainerFeatures, which is a minor inconsistency.Suggested fix:
Estimated effort: 15 minutes
Refactoring Recommendations
Priority 1: Medium — Rename
EngineConfigAudit DTORecommendation: Rename
pkg/cli/audit_expanded.go:EngineConfigtoAuditEngineSnapshotSteps:
pkg/cli/audit_expanded.gogo build ./...to verifyEstimated effort: < 1 hour
Impact: Eliminates name collision, improves searchability
Priority 2: Medium — Introduce
FrontmatterType AliasRecommendation: Add
type Frontmatter = map[string]anytopkg/workflow(orpkg/types)Steps:
pkg/workflow/compiler_types.goorpkg/types/frontmatter.gopkg/workflow/andpkg/cli/go build ./...Estimated effort: 2–3 hours
Impact: Improves readability and grep-ability across the largest code surface
Priority 3: Medium — Type Artifact Name & Output Name Constants
Recommendation: Introduce
ArtifactNameandOutputNametypes — consistent with existingJobName,StepID,MCPServerIDpatternsSteps:
type ArtifactName stringandtype OutputName stringtopkg/constants/constants.go.String()orstring(...))Estimated effort: 1–2 hours
Impact: Compile-time safety for artifact and output name usage
Priority 4: Low — Fix
mergeFeaturesSignatureRecommendation: Align
mergeFeaturessecond argument toDevcontainerFeaturesEstimated effort: 15 minutes
Implementation Checklist
cli.EngineConfig(audit DTO) →AuditEngineSnapshottype Frontmatter = map[string]anyaliastype ArtifactName stringand group artifact constantstype OutputName stringand group output name constantsmergeFeaturessignature to useDevcontainerFeaturesgo build ./...after each changeAnalysis Metadata
EngineConfig)any/map[string]anyusagesReferences:
Beta Was this translation helpful? Give feedback.
All reactions