[typist] Typist - Go Type Consistency Analysis

[github-actions[bot]]

🔤 Typist - Go Type Consistency Analysis

Analysis of repository: github/gh-aw

Executive Summary

I scanned all ~1,339 non-test struct/interface type definitions and the untyped-usage surface across pkg/. The good news first: this codebase has already migrated essentially all raw interface{} off to Go's any alias (only 2 hits remain, both in comments), and true duplicate type definitions are rare — most apparent "duplicates" turned out to be intentional, documented patterns (shared base structs, WASM build-tag stub pairs). The real opportunities are narrower but still worth doing: a handful of semantic/near-duplicate type families in pkg/cli's audit/reporting subsystem that reinvent the same shape with different names, and a systemic reliance on map[string]any (~3,000 non-test occurrences) for parsed YAML/JSON frontmatter, where values are extracted via repeated ad-hoc type assertions instead of validated structs.

The single highest-leverage fix is consolidating the IntDelta/StringDelta family in audit_comparison.go into one generic Delta[T], and tightening the handful of any-typed function signatures in pkg/typeutil/lookup.go and pkg/cli/token_usage_parse.go that sit at the center of frontmatter/log parsing. Everything else is lower-urgency cleanup.

Full Analysis Report

Duplicated Type Definitions

Summary Statistics

• Total type definitions analyzed: 1,339
• Duplicate clusters found: 9 (several judged "no action needed" after verification)
• Exact duplicates: 3
• Near duplicates: 4
• Semantic duplicates: 2 (one reclassified below after verification)

Cluster 1: Scanner Finding/Output wrappers (zizmor / grype / poutine / runner-guard)

Type: Semantic duplicate — verified as already well-handled, downgraded to Low impact
Occurrences:

1. pkg/cli/zizmor.go:26 — type zizmorFinding struct { Ident, Desc, URL string; Determinations struct{Severity string}; Locations []struct{...} }
2. pkg/cli/grype.go:54 — type grypeFinding struct { Vulnerability struct{ID, DataSource, Severity string; Fix struct{...}} }
3. pkg/cli/poutine.go:25 — type poutineFinding struct { RuleID, Purl string; Meta struct{Path string; Line int; Details string} }
4. pkg/cli/runner_guard.go:23 — type runnerGuardFinding struct { RuleID, Name, Severity, Description, Remediation, File, JobID string; Line int }
5. pkg/scanfindings/scanfindings.go:107 — type Finding struct { RuleID string; Severity SeverityLevel; Message string; File string; Line, Column int }

Verification note: I checked this cluster by hand after the initial scan. Each scanner integration already has a <scanner>FindingsToShared(...) []scanfindings.Finding adapter (zizmorFindingsToShared, grypeFindingsToShared, poutineFindingsToShared, runnerGuardFindingsToShared, plus grantFindingsToShared), and pkg/cli/audit_report.go consumes only scanfindings.Finding — none of the scanner-specific struct names leak into reporting. The per-scanner structs are necessary (they mirror each external tool's own JSON schema for unmarshaling), and the fan-in to one shared type is exactly the right pattern. No action needed here.

---

Cluster 2: Two parallel run-comparison frameworks (audit_diff.go vs audit_comparison.go)

Type: Semantic duplicate
Impact: High

Occurrences:

1. pkg/cli/audit_diff.go:24 — type DiffEntryBase struct { Status string; IsAnomaly bool; AnomalyNote string }
2. pkg/cli/audit_diff.go:384 — type AuditDiff struct { ...Run1ID, Run2ID... }
3. pkg/cli/audit_comparison.go:21 — type AuditComparisonData struct { Baseline *AuditComparisonBaseline; Delta *AuditComparisonDelta; ... }
4. pkg/cli/audit_comparison.go:47 — type AuditComparisonDelta struct { Turns AuditComparisonIntDelta; Posture AuditComparisonStringDelta; ... }

Why it's a problem: audit_diff.go compares two runs using a Run1X/Run2X + shared DiffEntryBase{Status, IsAnomaly, AnomalyNote} convention. audit_comparison.go compares a baseline vs. current run using an unrelated Before/After/Changed convention with its own Delta family. Same conceptual operation (diff two runs), two incompatible vocabularies in the same package.

Recommendation: Converge on one comparison vocabulary — prefer the Before/After/Changed style since it generalizes better — and share a single diffing helper/type family instead of maintaining two.
Estimated effort: 3-4 hours. Benefits: one mental model for "comparing two runs," less duplicated diff logic.

---

Cluster 3: AuditComparisonIntDelta / AuditComparisonStringDelta / AuditComparisonRouteDelta / AuditComparisonMCPFailureDelta

Type: Near duplicate
Impact: Medium

// pkg/cli/audit_comparison.go
type AuditComparisonIntDelta struct    { Before, After int; Changed bool }
type AuditComparisonStringDelta struct { Before, After string; Changed bool }
type AuditComparisonRouteDelta struct  { Before, After *AuditComparisonRoute; Changed bool }

Recommendation: IntDelta and StringDelta are identical in shape modulo the value type. Replace with one generic Delta[T any] struct { Before, After T; Changed bool } and instantiate as Delta[int], Delta[string], Delta[*AuditComparisonRoute]. Keep MCPFailureDelta separate — its NewlyPresent semantics genuinely differ from a plain before/after comparison.
Estimated effort: 1-2 hours. Benefits: one generic type instead of three hand-rolled ones; new delta kinds become a one-line instantiation.

---

Cluster 4: Hand-maintained "Wire" schema-mirror structs in mcp_schema.go duplicating domain structs' MarshalJSON shapes

Type: Near duplicate
Impact: Medium

Occurrences (representative pairs):

1. pkg/cli/mcp_schema.go:239 (toolUsageSummaryWire) mirrors pkg/cli/logs_report_tools.go:46 (ToolUsageSummary)
2. pkg/cli/mcp_schema.go:247 (mcpServerHealthDetailWire) mirrors pkg/cli/audit_expanded.go:91 (MCPServerHealthDetail, which has its own custom MarshalJSON)
3. pkg/cli/mcp_schema.go:258 (mcpServerCrossRunHealthWire) mirrors pkg/cli/audit_cross_run.go:86 (MCPServerCrossRunHealth)
4. pkg/cli/mcp_schema.go:268 (mcpFailureSummaryWire) mirrors pkg/cli/logs_models.go:240 (MCPFailureSummary)
5. pkg/cli/mcp_schema.go:275 (domainAnalysisWireSchema) mirrors pkg/cli/access_log.go:35 (DomainAnalysis)

Why it's a problem: each domain struct already defines a custom MarshalJSON with an inline anonymous struct to produce a stable flattened JSON shape (post-embedding-refactor compatibility), and mcp_schema.go independently redeclares that same flattened shape again just to generate a JSON Schema for MCP tool output. That's three places (domain struct, its MarshalJSON anon struct, and the Wire struct) that must stay in sync by hand.

Recommendation: Generate the JSON Schema from the real MarshalJSON output (reflection-based) or introduce one source-of-truth schema struct whose tags drive both marshaling and schema generation, eliminating the Wire copy.
Estimated effort: 4-6 hours (touches 5 type pairs). Benefits: removes an entire category of "forgot to update the schema mirror" bugs.

---

Cluster 5: RunOptions vs TrialOptions (workflow execution option bags)

Type: Near duplicate
Impact: Medium

• pkg/cli/run_workflow_execution.go:31 — RunOptions{ EngineOverride, RepoOverride, RefOverride string; AutoMergePRs, Push, WaitForCompletion bool; RepeatCount int; Inputs []string; Verbose, DryRun, JSON, Approve bool }
• pkg/cli/trial_types.go:41 — TrialOptions{ Repos TrialRepoContext; DeleteHostRepo, ForceDelete, Quiet, DryRun, JSONOutput bool; TimeoutMinutes int; TriggerContext string; RepeatCount int; AutoMergePRs bool; EngineOverride string; AppendText string; Verbose, DisableSecurityScanner bool }

Recommendation: Extract a shared CommonRunOptions{ EngineOverride string; RepeatCount int; AutoMergePRs, Verbose, DryRun bool } embedded by both, so the overlapping flags' semantics and defaults can't silently drift apart between the two commands.
Estimated effort: 1-2 hours.

---

Lower-priority / no-action clusters (verified, listed for completeness)

• ValidationError (pkg/validationerror/validationerror.go interface vs pkg/parser/validation_error.go struct) — intentional: the parser struct embeds validationerror.Payload to satisfy the shared interface. Same name across packages can look like accidental collision to newcomers; a cross-reference doc comment would help, but no structural change needed.
• ProgressBar / SpinnerWrapper WASM build-tag stub pairs (pkg/console/progress.go + progress_wasm.go, spinner.go + spinner_wasm.go) — deliberate platform-conditional stubs per the repo's documented WASM-stub convention.
• pkg/types.BaseMCPServerConfig vs pkg/parser MCP config types — already a well-factored, explicitly documented shared-base embedding pattern, not duplication.
• agentdrain Cluster/AnomalyReport vs audit_cross_run_clusters.go's RunCluster/ClusterPattern — same high-level idea (cluster similar items, surface patterns) applied to different domains with different algorithms (log-template mining vs. dimensional grouping). Not worth merging now; worth a shared cluster.Analysis[T] package if a third use case appears.
• CLI MCP-tool Args structs sharing a Workflows []string selector field — each tool needs genuinely distinct flags; only the selector field repeats. Not worth a shared type until a 5th tool needs it.

---

Untyped Usages

Summary Statistics

• Raw interface{} usages (non-test): 2 — both in comments only; the codebase has already standardized on any
• any usages (non-test): ~4,961
• map[string]any occurrences (non-test): ~2,997
• Untyped constants (non-test, outside pkg/linters): ~552

Category 1: any in Function Parameters/Returns Sitting at Parsing Chokepoints

Impact: High — these functions are the shared entry points for frontmatter/log parsing, so fixing them narrows the blast radius the most.

1. pkg/typeutil/lookup.go:9 — func ParseBool(m map[string]any, key string) bool
   Every caller passes parsed YAML frontmatter. Wrapping it in a named type Frontmatter map[string]any with typed accessor methods (f.Bool(key)) lets misuse show up at the call site instead of silently returning a zero value on type mismatch.
2. pkg/typeutil/lookup.go:21 — func LookupMap(m map[string]any, key string) (map[string]any, bool)
   Nested map[string]any lookups chain indefinitely with no schema guarantee. At minimum, a named recursive Frontmatter type documents intent and centralizes the assertion logic.
3. pkg/cli/token_usage_parse.go:146,154 — func extractUsageRecord(value any) map[string]any and func usageNumericValue(parsed, usage map[string]any, keys ...string) float64
value any comes from decoding a controlled, fixed-format log record. Decoding straight into a UsageRecord struct would move validation into json.Unmarshal and remove the scattered float64/json.Number/int/int64/string type switch currently needed to read the same logical field across two untyped maps.
4. pkg/cli/mcp_add.go:167 — func createMCPToolConfig(...) (map[string]any, error)
   The returned map is immediately serialized into YAML with a fixed key set (type, command, env). A MCPToolConfig struct prevents key-name typos and gets YAML marshal/unmarshal for free.
5. pkg/cli/domains_command.go:198 — func extractWorkflowDomainConfig(...) (engineID string, network *workflow.NetworkPermissions, tools map[string]any, runtimes map[string]any)
   Four-value return with two untyped maps is fragile to field renames; group into a DomainConfig struct.
6. pkg/cli/codemod_bots.go:45 — func mergeLegacyBots(onBots, topBots any) ([]string, bool)
   This repeats across roughly 15 codemod_*.go files — the frontmatter field is always a string or []string. A single small sum-type helper used across all of them would remove the pattern everywhere at once.

Category 2: any / interface{} in Struct Fields

Impact: Medium

• pkg/types/input_definition.go:18 — Default any \yaml:"default,omitempty"` // Can be string, number, or boolean — the comment already documents the exact 3 legal kinds; a tagged-union (InputDefault{ Str *string; Num *float64; Bool *bool }) would let validation code pattern-match instead of type-asserting at every consumer of workflow inputs:`.
• pkg/parser/import_observability.go:18 — Headers any \json:"headers,omitempty"`— HTTP headers are always string-keyed/valued; should just bemap[string]string`.
• pkg/cli/mcp_tools_privileged.go:458-459 — RunID any, RunIDOrURL any with a jsonschema tag literally stating "String or number" — a small custom-(Un)marshal union type would make this self-documenting.
• pkg/cli/list_workflows_command.go:29 — On any \json:"on,omitempty"`for workflow trigger config, which already has a fixed shape modeled inpkg/workflow/trigger_parser.go` — reuse that type instead of a second untyped representation.

Category 3: map[string]any Hotspots (JSON/YAML-like Untyped Maps)

Impact: High

• pkg/parser/mcp.go — 34 occurrences in one file for MCP server definitions, which already have a well-known finite schema (stdio/http/sse, command, env, tools) validated by JSON schema elsewhere in pkg/parser/schemas. Decoding once into a struct would remove dozens of scattered .(map[string]any) assertions.
• pkg/parser/import_field_extractor.go — 41 occurrences; effectively hand-rolled schema validation duplicating the declarative JSON schemas in pkg/parser/schemas.
• pkg/cli/compile_model_validation.go:95 — data.RawFrontmatter["engine"].(map[string]any) then engine["models"].(map[string]any) — two chained assertions for a well-known engine.models shape; a one-time typed decode removes the repeated assertion/ok-check boilerplate and its silent-no-op-on-mismatch failure mode.
• pkg/cli/codemod_min_integrity_none_bash.go:26 — frontmatter["tools"].(map[string]any) → toolsMap["github"].(map[string]any) — this exact pattern repeats across most of the ~30 codemod_*.go files; a shared typed accessor (using the existing GitHubToolConfig) would deduplicate and type-check all of them at once.

Category 4: Untyped Constants (Timeouts, Levels, Modes)

Impact: Medium

• pkg/cli/run_workflow_execution.go:25 — const workflowCompletionWaitTimeoutMinutes = 6 * 60 — a pre-multiplied bare int named "...Minutes" invites unit mistakes at every call site; prefer const workflowCompletionWaitTimeout = 6 * time.Hour.
• pkg/workflow/cache_integrity.go:17 — const defaultCacheIntegrityLevel = "none" compared against MinIntegrity elsewhere — should be a type CacheIntegrityLevel string enum so arbitrary strings can't compile where only "none"/"full" are valid.
• pkg/constants/constants.go:306 — const AWFDefaultLogLevel = "info", explicitly converted via string(constants.AWFDefaultLogLevel) at pkg/workflow/awf_command_builder.go:531 — that conversion is a tell callers already think of it as a distinct type. The same file already has the right pattern for other constants (CommandPrefix, LineLength); just not applied here.
• pkg/constants/constants.go:347 — const DevModeGhAwImage = "localhost/gh-aw:dev" — mixing plain image-ref strings with arbitrary strings elsewhere risks passing the wrong kind of string where an image ref is expected.

---

Refactoring Recommendations

Priority 1 — High: Consolidate the audit/comparison type families (Clusters 2 & 3)

Steps: 1) Pick the Before/After/Changed vocabulary as the standard. 2) Introduce Delta[T any] and replace AuditComparisonIntDelta/AuditComparisonStringDelta. 3) Migrate audit_diff.go's Run1X/Run2X fields onto the same convention. 4) Update callers and run tests.
Estimated effort: 4-6 hours combined. Impact: High — one comparison vocabulary across the audit subsystem.

Priority 2 — High: Introduce a Frontmatter/typed-config layer over map[string]any hotspots (Category 1 & 3 of untyped usages)

Steps: 1) Start with pkg/typeutil/lookup.go (ParseBool, LookupMap) since it's the shared entry point. 2) Add typed decode for pkg/parser/mcp.go and pkg/cli/compile_model_validation.go's engine.models lookup, both of which already have a JSON-schema-backed shape. 3) Extract one shared typed accessor for the repeated frontmatter["tools"].(map[string]any)["github"] pattern across codemod_*.go files.
Estimated effort: 6-10 hours (can be done incrementally, file by file). Impact: High — removes the largest source of silent runtime type-assertion failures.

Priority 3 — Medium: Mirror struct deduplication (Cluster 4) and RunOptions/TrialOptions (Cluster 5)

Steps: Generate mcp_schema.go's Wire structs from the real MarshalJSON output instead of hand-mirroring it; extract CommonRunOptions shared by RunOptions and TrialOptions.
Estimated effort: 5-8 hours combined. Impact: Medium.

Priority 4 — Medium: Type the semantically-meaningful constants (Category 4)

Steps: Add CacheIntegrityLevel, convert AWFDefaultLogLevel/DevModeGhAwImage to named types following the existing CommandPrefix/LineLength pattern already in pkg/constants/constants.go; fix the pre-multiplied timeout constant to use time.Duration.
Estimated effort: 2-3 hours. Impact: Medium — prevents unit-confusion and typo-driven bugs with low risk.

---

Implementation Checklist

• Consolidate AuditComparisonIntDelta/StringDelta into generic Delta[T]
• Converge audit_diff.go and audit_comparison.go on one comparison vocabulary
• Wrap frontmatter map[string]any access behind a typed Frontmatter accessor in pkg/typeutil
• Decode pkg/parser/mcp.go MCP server config into a typed struct instead of repeated map assertions
• Add a shared typed accessor for the frontmatter["tools"]["github"] codemod pattern
• Replace mcp_schema.go Wire structs with schema generation from real MarshalJSON output
• Extract CommonRunOptions shared by RunOptions/TrialOptions
• Add CacheIntegrityLevel, type AWFDefaultLogLevel/DevModeGhAwImage, fix pre-multiplied timeout constant
• Run full test suite after each incremental change

Analysis Metadata

• Total non-test Go files under pkg/: 1,450
• Total Type Definitions Analyzed: 1,339
• Duplicate Clusters Identified: 9 (5 actionable, 4 verified as intentional/no-action)
• Untyped Usage Locations: ~4,961 any usages, ~2,997 map[string]any, ~552 untyped constants
• Detection Method: Serena-assisted semantic review + targeted grep across pkg/, with manual cross-reference verification on the highest-impact clusters
• Analysis Date: 2026-10-08

References:

• §37770468801

Generated by ✍️ Typist - Go Type Analysis · claude · agent · 365.1 AIC · ⊞ 6.5K · ◷

• expires on Oct 9, 2026, 3:44 AM UTC-08:00

[github-actions[bot]]

This discussion was automatically closed because it expired on 2026-10-09T11:44:01.170Z.

Closed by Workflow