[typist] Type Consistency Analysis — A (Mostly) Clean Bill of Health #38107
Closed
Replies: 1 comment
-
|
This discussion was automatically closed because it expired on 2026-06-10T12:30:59.531Z.
|
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.
-
🔤 Typist — Go Type Consistency Analysis
Analysis of repository:
github/gh-aw· 2026-06-09Executive Summary
Good news first: this is a well-typed codebase. I swept every non-test
.gofile underpkg/(~894 files, ~682 distinct named type definitions) looking for duplicated type definitions and weakly-typed code, and I came away impressed rather than alarmed. There are no genuine exact-duplicate type definitions — the only repeated struct names (SpinnerWrapper,ProgressBar,RepositoryFeatures) are intentional_wasm.gobuild-tag variants, which is exactly how you are supposed to do it. The team has also already migrated essentially 100% offinterface{}(only 7 occurrences remain, all inside linter test fixtures) and maintains a healthy set of 44 named scalar types (EngineName,ModelName,PermissionLevel, ...). That is real discipline.That said, I did find a few concrete, low-risk opportunities worth a look: one textbook generics candidate (two structurally-identical delta types), a small cluster of the same untyped field re-declared across packages (
On/RunsOn), and a large but mostly-justified population ofmap[string]anydriven by GitHub Actions YAML emission. None of these are fires — think of them as tidy-up wins, prioritized below.Full Analysis Report
Duplicated Type Definitions
Summary Statistics
Cluster 1:
AuditComparisonIntDelta/AuditComparisonStringDelta— structural duplicatesType: Near/semantic duplicate (identical shape, differing field type)
Occurrences: 2 type definitions, same file
Impact: Low-risk, moderate value — references are contained to one file (
buildAuditComparison)Locations:
pkg/cli/audit_comparison.go:44—type AuditComparisonIntDelta structpkg/cli/audit_comparison.go:50—type AuditComparisonStringDelta structDefinition comparison — identical except
Before/Afterelement type:Recommendation — unify with a generic type:
pkg/cli/audit_comparison.go)AuditComparisonDeltais already taken by the aggregate struct, so pick e.g.ComparisonDelta[T].Why there is nothing else here
The build-tag pairs below are correct Go idiom, not duplication — please do not consolidate them:
SpinnerWrapperpkg/console/spinner.go+spinner_wasm.goProgressBarpkg/console/progress.go+progress_wasm.goRepositoryFeaturespkg/workflow/repository_features_validation.go+_wasm.goUntyped Usages
Summary Statistics
interface{}occurrences: 7 — all inpkg/linters/.../testdata/fixtures (intentional; ignore)anyword-occurrences: ~3541map[string]any: 2034 (concentrated:pkg/workflow219 files,pkg/cli104,pkg/parser19)[]any: 393anyparameter: 760 (only 3 variadic)any: ~30Category 1: Repeated untyped fields for the same concept (best opportunity)
Impact: Medium — the same GitHub Actions field is independently re-declared as
anyin multiple structs across two packages. This is both an untyped usage and a near-duplicate.On any(4 sites) andRunsOn any(3 sites):Context: these mirror the GitHub Actions schema where
on:/runs-on:legitimately accept string | array | object. Go cannot express that union, soanyis defensible — but the concept is currently undocumented and re-invented per struct.Suggested fix — centralize one named, documented type in a shared package (e.g.
pkg/types):UnmarshalYAMLvalidation.Category 2: Polymorphic schema fields (mostly leave as-is)
Impact: Low — these are genuine string|array|object|bool YAML/JSON fields. Strong typing would require union types Go lacks;
anyis idiomatic here. Listed for transparency, not flagged for change:View the ~30 bare-
anystruct fieldsOne narrow win in this set — JSON-RPC / run identifiers declared
anyto accept "string or number":These could use
json.Number(or a tinyStringOrNumbertype) to make the "string-or-number" contract explicit and avoid downstreamswitch v := id.(type)assertions. Low priority, but cleaner.Category 3: Untyped constants
I deliberately did not flag Go's ordinary untyped constants (
const MaxRetries = 5) — that is idiomatic Go, and mechanically typing all of them would add noise, not safety. The codebase already promotes meaningful enums to named string/int types (44 of them), which is the right pattern. No action recommended here.Category 4:
map[string]anyvolume2034 occurrences sounds scary but is largely legitimate: this is a workflow compiler that parses arbitrary frontmatter and emits GitHub Actions YAML, where dynamic maps are unavoidable at the boundary. The concentration (
pkg/workflow,pkg/cli,pkg/parser) matches exactly the YAML/JSON serialization layer. Recommend leaving these alone except where a known, fixed schema is being passed around internally (case-by-case, not a sweep).Recommendations (prioritized)
Priority 1 — Unify the delta structs with generics
AuditComparisonIntDelta+AuditComparisonStringDeltawithComparisonDelta[T comparable].Priority 2 — Centralize the
On/RunsOnunionRunsOnValue/OnValuealias in a shared types package and reference it from the 7 sites.Priority 3 — Tighten string-or-number identifiers
ID/RunID/RunNumber anyfields tojson.Numberor a smallStringOrNumbertype.Implementation checklist
ComparisonDelta[T], updatebuildAuditComparison, runpkg/clitestsRunsOnValue/OnValueto shared types, repoint 7 fieldsjson.NumberforID/RunID/RunNumber_wasm.gopairs untouchedmap[string]any/ polymorphic schema fields untouchedgo build ./... && go test ./pkg/cli/... ./pkg/workflow/...Analysis Metadata
pkg/)interface{}: 7 (all test fixtures) ·any: ~3541 ·map[string]any: 2034find_referencing_symbols) + pattern matchingReferences: §27205569285
Beta Was this translation helpful? Give feedback.
All reactions