[typist] 🔤 Typist - Go Type Consistency Analysis #47564
Closed
Replies: 1 comment
|
This discussion has been marked as outdated by Typist - Go Type Analysis. A newer discussion is available at Discussion #47781. |
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
Executive Summary
Good news first: this codebase is in excellent type-safety shape. I swept every non-test
.gofile underpkg/(1,137 files, 637 production struct definitions) hunting for duplicated types and weakly-typedinterface{}/anyusages — and came away with almost nothing to complain about. There are zero genuine duplicate type definitions to consolidate, and production code has effectively migrated offinterface{}entirely (the only literalinterface{}matches are linter test fixtures and doc-comment text). Naming discipline is unusually strong: the ~30 distinct*Configstructs, the many*Summary/*Diff/*Reporttypes, etc. are genuinely different concepts, not collisions.The one area worth a little polish is untyped string/numeric constants that form implicit enums. A handful of "enum-shaped" const groups (LLM providers, guard-policy error codes, URL policies, MCP status/argument kinds) still flow through the code as bare
string/intinstead of a named semantic type — even though the codebase already does this the right way elsewhere (EngineName,FeatureFlag,PermissionScope,ActionMode, ...). Adopting a named type for those groups would makeswitchsites exhaustive/greppable and prevent accidentally passing an arbitrary string where a provider or policy value is expected. None of this is urgent — these are refinements, not bug fixes.Full Analysis Report
Duplicated Type Definitions
Summary Statistics
_test.goandtestdata/excluded)Every apparent same-name collision falls into one of three intentional, correct Go patterns. None should be changed.
1. WASM build-tag variants (leave as-is) — mutually-exclusive
//go:buildconstraints; the compiler only ever sees one:ProgressBar—pkg/console/progress.go:31/pkg/console/progress_wasm.go:7SpinnerWrapper—pkg/console/spinner.go:92/pkg/console/spinner_wasm.go:10RepositoryFeatures—pkg/workflow/repository_features_validation.go:72/pkg/workflow/repository_features_validation_wasm.go:52. Type-alias re-exports (already single-source-of-truth) — the second occurrence is
type X = otherpkg.X, the idiomatic cross-package re-export:InputDefinition— realpkg/types/input_definition.go:15; aliaspkg/workflow/inputs.go:18SanitizeOptions— realpkg/stringutil/sanitize.go:65; aliaspkg/workflow/strings.go:95ToolCallInfo/LogMetrics— realpkg/workflow/metrics.go:16,26; aliaspkg/cli/logs_models.go:94,90ActionYAMLInput/ActionPin/ActionPinsData/ContainerPin/SHAResolver— realpkg/actionpins/actionpins.go; aliaspkg/workflow/action_pins.go3. Test-fixture collision (out of scope) —
Worker×3, all underpkg/linters/*/testdata/src/..., unrelated concepts.Untyped Usages (
interface{}/any)Summary Statistics
interface{}in production: 0 (17 total matches — 15 are linter testdata fixtures, 2 are[]interface{}inside doc comments)anyusages: 3,867 total, of which ~99% are idiomaticmap[string]any: ~2,300 (parse-boundary frontmatter/config)[]any: ~433 (YAML/JSON arrays)any: ~262 lines (mostlymap[string]anyextractors + generic converters)any: 36 (documented polymorphic YAML/JSON unions)[T any]type params: ~15 (idiomatic generics)...any: a handful (logging/format)Nearly all
anysits at genuine YAML/JSON parse boundaries for polymorphic GitHub Actions config (RunsOn,On,Endpoint,Headers,Jobs,Permissions, raw frontmatter maps, step slices, JSON-RPCID). These are correctlyany— the accepted forms are documented in comments and there is no single concrete type. Do not flag these.Genuine (modest) strong-typing opportunities
1.
pkg/workflow/safe_outputs_config_types.go:105—ReportFailureAsIssue anyComment says "bool, templatable expression string, or []string categories," and the parsed results already live in adjacent typed fields (
ReportFailureAsIssueCategories []string,...ExcludedCategories []string). This rawanyis transitional. A named type with a customUnmarshalYAMLthat normalizes the three forms would encode the union contract in the type system and remove ad-hoc type switches at call sites.2.
pkg/cli/logs_models.go:323-324—RunID any/RunNumber anyGitHub returns numbers, but
aw_info.jsoninputs can be strings. If normalized at decode, these could becomeRunID int64/RunNumber int, removing downstream type assertions. (Lower confidence — depends on whether both encodings really occur.)3.
pkg/console/console_types.go:50—FormField.Value anyA generic form widget (
flag.Value-style). If the four field kinds (input/password/confirm/select) all capture strings,Value *stringremoves an untyped pointer. (Verify no bool/int consumers first.)Untyped Constants (the main opportunity)
Summary Statistics
pkg/constants/— correctly plain strings, not enums)const (...)blocks: ~110 more (again mostly env-var namespaces — fine)The codebase already models many enums correctly (
EngineName,FeatureFlag,PermissionScope,PermissionLevel,ActionMode,GitHubMCPMode,GitHubIntegrityLevel). The groups below were missed and still flow as barestring/int.String-enum candidates
★ 1. LLM provider (HIGH value) —
pkg/workflow/llm_provider.go:13-17Flows through many signatures as bare
provider string(normalizeLLMProvider,llmProviderProfileFor,llmProviderSecretNames,llmProviderGatewayBaseURL,normalizeEngineProvider,normalizeProviderForPricing, ...).type LLMProvider stringmirrors the existingEngineNamepattern and makes theswitchsites exhaustive/greppable.2. Objective labels + multi-label logic (HIGH value) —
pkg/github/label_objective_mapping_constants.go:20-140~30 label-name consts (one conceptual set) plus a clean 3-value logic enum (
MultiLabelLogicMax/Sum/First) that selects branching behavior. Suggesttype ObjectiveLabel stringandtype MultiLabelLogic string.3. Safe-outputs URL policy —
pkg/workflow/safe_outputs_validation.go:13-16Two-value policy (
allowed-only,allowed-or-code-region) compared in aswitchagainst a bare-stringconfig.URLs.type SafeOutputsURLsPolicy stringties the config field and validator together.4. MCP registry status & argument type —
pkg/cli/mcp_registry_types.go:107-116StatusActive/InactiveandArgumentTypePositional/Named; the struct fields areStatus string/Type string. Suggesttype ServerStatus stringandtype ArgumentType string.5. Experiments storage backend —
pkg/workflow/compiler_experiments.go:24-29Two-value enum (
cache,repo) selecting persistence.type ExperimentsStorage stringclarifies intent.Numeric-enum candidates
★ 6. Guard-policy JSON-RPC error codes (HIGH value) —
pkg/cli/gateway_logs_types.go:61-68A closed set of magic ints compared against
GuardPolicyEvent.ErrorCode int(line 77).type GuardPolicyErrorCode inton both the consts and the struct field documents the -32001...-32006 range and prevents comparing against an unrelated int.7. Network / LLM-gateway ports —
pkg/constants/constants.go:100-140DefaultMCPGatewayPort,ClaudeLLMGatewayPort, ... all untypedint, and there's already a validation range (MinNetworkPort/MaxNetworkPort).type NetworkPort intis a natural fit (values are passed intogatewayPort intand formatted into URLs).8. Objective scoring weights —
pkg/github/label_objective_mapping_constants.go~25 untyped
ObjectiveValue*ints ("objective scores" combined by MultiLabelLogic).type ObjectiveScore intpairs naturally withObjectiveLabel(#2).Notable inconsistency worth flagging
In
pkg/constants/constants.gothe AI-credits guardrails mix representations for one concept:DefaultMaxAICredits int64 = 1000,DefaultDetectionMaxAICredits int64 = 400(typedint64)DefaultMaxDailyAICredits = "5000"(line 333) — a stringDefaultMaxRuns = 500,DefaultMaxTurnCacheMisses = 5,DefaultMaxToolDenials = 5(untypedint)A shared
type AICredits int64(and normalizing the string one) removes the ambiguity and the string/number split.Correctly untyped (skipped)
pkg/constants/paths/dirs/mounts/env-var names/image refs,pkg/cli/docker_images.goimage refs, artifact/file-name and step-output-key labels — namespaced string labels by design, not enumerations.🎯 What Should We Do About This?
Because there are no duplicates and no real weak-typing debt, the action plan is short and low-risk. Start with the two highest-leverage typed enums.
Priority 1: High-value typed enums (safety + clarity)
type LLMProvider string(rejig docs #1) andtype GuardPolicyErrorCode int(add cli flag to guard dropping a agentic workflow instructinos file #6) — these flow through the most signatures / struct fields.go build ./... && go test ./....switches; prevents string/int mixups.Priority 2: Remaining enum-shaped groups
MultiLabelLogic,SafeOutputsURLsPolicy, MCPServerStatus/ArgumentType,ExperimentsStorage,NetworkPort,ObjectiveLabel/ObjectiveScore.Priority 3: Consistency cleanup
type AICredits int64).anyrefinements (ReportFailureAsIssue,RunID/RunNumber,FormField.Value).Implementation Checklist
type LLMProvider string+ retype consts/signatures (Priority 1)type GuardPolicyErrorCode int+ retypeGuardPolicyEvent.ErrorCode(Priority 1)go build ./... && go test ./...after each changeinterface{}work needed — none foundAnalysis Metadata
pkg/, non-_test.go)interface{}in production: 0anyusages: 3,867 (~99% idiomatic parse-boundary)anyrefinements)All reactions