[typist] Type Consistency Analysis — Duplicates & Untyped Usages in pkg/ #37811
Closed
Replies: 1 comment
-
|
This discussion has been marked as outdated by Typist - Go Type Analysis. A newer discussion is available at Discussion #38107. |
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 repository: github/gh-aw
Good news first: this is a healthily typed codebase. I swept every non-test
.gofile underpkg/(891 files, ~609 struct/interface definitions plus 38+ named string types) hunting for duplicated type definitions and weak typing — and the headline is that the team is already doing the right things. There are zerointerface{}usages left (everything is on modernany), zero problematic cross-package duplicate types (the few repeated names are deliberate alias re-exports or//go:build wasmvariants), and a dedicatedpkg/typeutilpackage already centralizes safe conversion out of dynamicmap[string]anydata.So this is more a consolidation review than a fix-these-duplicates list. The one genuinely actionable theme: a handful of polymorphic YAML/JSON fields typed
any(RunsOn,On,Headers,RunID/RunNumber/ID) recur across packages and each re-implements its own runtime type juggling. Shared union types with custom unmarshalers would remove that repetition and shrink the ~1,395 raw type assertions in the parsing layer.Full Analysis Report
Summary Statistics
pkg/)interface{}usagestype X = pkg.Yalias re-exports (good pattern)map[string]anyusages (non-test)[]anyusages (non-test)any.(map[string]any)/.(string)/.([]any)assertionsDuplicated Type Definitions — none to consolidate ✅
Every repeated type name falls into one of two intentional patterns, not duplication.
1. Alias re-exports (single source of truth, re-surfaced for a package API):
Same for
InputDefinition(pkg/types→pkg/workflow),SanitizeOptions(pkg/stringutil→pkg/workflow), andLogMetrics/ToolCallInfo(pkg/workflow→pkg/cli). This is exactly the pattern a duplicate-finder would recommend — already in place.2. Build-tag variants —
ProgressBar,SpinnerWrapper(pkg/console) andRepositoryFeatures(pkg/workflow) each have a*_wasm.gotwin guarded by//go:build. Compiled mutually-exclusively; correct by design.Untyped Usages
Category 1 — Polymorphic
anystruct fields (the actionable one)31 struct fields are typed
any. Most are legitimately polymorphic — comments say so (e.g.continue-on-errorcan be bool or string expression;runs-onis string, array, or object). The opportunity is that several recur across files, each re-deriving the type by hand:Recurring polymorphic fields (with locations)
RunsOn any(string | array | object):pkg/workflow/safe_jobs.go:20,pkg/cli/copilot_setup.go:149On any(string | array | map):pkg/cli/status_command.go:35,pkg/cli/list_workflows_command.go:25,pkg/cli/jsonworkflow_to_markdown.go:38,pkg/cli/copilot_setup.go:157Headers any(almost always string→string map):pkg/parser/import_field_extractor.go:752,pkg/workflow/frontmatter_types.go:208,238RunID/RunNumber/ID any(JSON string-or-number):pkg/cli/logs_models.go:304-305,pkg/cli/gateway_logs_types.go:148,159,pkg/cli/mcp_tools_privileged.go:254-255Imports/Include any(string | array):pkg/workflow/frontmatter_types.go:342,344Suggested fix — shared union types with custom unmarshalers:
Then
Imports/Include(and the[]stringhalf ofRunsOn) becomeStringOrSlice;RunID/RunNumber/IDbecomeFlexibleID;Headers anybecomesmap[string]stringwhere confirmed. Benefit: the type juggling lives in one tested place, and consumers get a concrete type instead of an assertion.Category 2 —
map[string]anyin the parsing layer (mostly idiomatic)2,113
map[string]anyusages sound alarming but are concentrated in YAML/JSON frontmatter handling, where input is schema-dynamic (hotspots:pkg/parser/import_field_extractor.go= 78,tools_merger.go= 19). Forcing concrete structs here fights the job of the parser.The real signal is the 573 raw
.(map[string]any)and 650 raw.(string)assertions. The team already builtpkg/typeutilfor this (LookupMap,LookupString,LookupStringPath,ParseBool,ConvertToInt, ...) — so the recommendation is not add types, it is route ad-hoc assertions through the existing helpers for nil-safety and consistency.Category 3 — Untyped constants — no findings
The codebase already defines 38+ semantic string types (
JobName,StepID,EngineName,PermissionLevel,SandboxType,ActionMode,OutcomeStatus, ...). Remaining bare numeric constants are idiomatic Go untyped constants — typing them would be noise, not safety.Refactoring Recommendations
Priority 1 — Shared union types for recurring polymorphic fields. Add
pkg/types/yamlunion.go(StringOrSlice,FlexibleID, plusRunsOnSpec/TriggerSpecfor object-accepting cases); migrateImports,Include,RunID,RunNumber,ID,Headers. Effort ~4–6h · Impact High.Priority 2 — Funnel raw assertions through
pkg/typeutil. Audit the 573.(map[string]any)+ 650.(string)sites; replace hand-rolled assertions withLookupMap/LookupString/LookupStringPath; consider a lint flagging new raw assertions on parsed config. Effort ~3–5h · Impact Medium.Priority 3 — Tighten confirmed-shape
anyfields. Narrow eachHeaders anytomap[string]stringwhere always headers; review anyanyfield whose comment does not describe a real union. Effort ~2h · Impact Medium.Implementation Checklist
pkg/types/yamlunion.go(StringOrSlice,FlexibleID,RunsOnSpec,TriggerSpec) + testsImports/Include/RunID/RunNumber/IDto the new union typesHeaders any→map[string]stringwhere confirmed.(map[string]any)assertions →typeutil.LookupMapgo test ./...Analysis Metadata
.gofiles inpkg/go run/python/awk/sedexecution was blocked by the run security policy, so extraction used the dedicated search tools instead.)References: §27138497703
Beta Was this translation helpful? Give feedback.
All reactions