You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Good news first: this codebase is already in strong shape on type consistency. I analyzed ~820 top-level type definitions across 947 non-test .go files under pkg/, and there are zero exact cross-package duplicate structs needing a merge. Where a name repeats across packages it's almost always an intentional type X = pkg.Yalias (~20 of them, e.g. ActionPin, LogMetrics, SanitizeOptions, InputDefinition) or a //go:build js || wasm platform stub — exactly the pattern you want.
So this is a friendly nudge on a few small holdovers, not a big cleanup. On duplication I found 4 near-duplicate and 3 semantic-duplicate clusters, mostly in pkg/cli log/audit reporting where parallel structs drifted in field naming. On typing, interface{}/any usage is idiomatic everywhere (YAML/JSON frontmatter, the analysis.Pass framework signature, generics) — nothing fixable. The real quick wins are 3 string-const groups that are implicit enums but aren't named string types yet, mildly inconsistent given the codebase already does this well (ActionMode, UniversalLLMBackend).
Recommendation: Collapse to one canonical type (add the optional GuardPolicySummary), alias the other. No import churn. ~1 hour.
Cluster 2: DomainAnalysis vs FirewallAnalysis (near)
pkg/cli/access_log.go:34 (AllowedCount, BlockedCount) vs pkg/cli/firewall_log.go:131 (AllowedRequests, BlockedRequests + RequestsByDomain). Both embed DomainBuckets; counters differ only by name. Same divergence in AccessLogSummary (logs_report_firewall.go:12) and FirewallLogSummary (:22). Recommendation: Unify counter names, extract a shared base. 2-3 hours.
Cluster 3: Diff count summaries (near) — all in pkg/cli/audit_diff.go
FirewallDiffSummary (:50), MCPToolsDiffSummary (:240), ToolCallsDiffSummary (:299) repeat a "new/removed/changed counts + anomaly flags" shape. Recommendation: Extract DiffCountsSummary{ New, Removed, Changed int; HasAnomalies bool; AnomalyCount int } and embed. 2 hours.
Workflow-run models: WorkflowRunInfo (run_workflow_tracking.go:19, 5 fields) is a strict subset of WorkflowRun (logs_models.go:50, 28 fields). Slim struct intentional for the poll path.
Untyped Usages
interface{}: ~1 real (rest are linter fixtures) — 0 fixable
Implicit string-const enums: 3 quick-win groups + 3 struct fields
.(map[string]any) assertions: 34 across 15 files — idiomatic frontmatter walking, already mitigated by pkg/typeutil/convert.go
Category 1: interface{} / any — nothing to fix
Correct Go at the serialization boundary: map[string]any for decoded frontmatter, func run(*analysis.Pass) (any, error) mandated by golang.org/x/tools/go/analysis, generics. By-design polymorphic cases like ParseVersionValue(version any) (stringutil/stringutil.go:54) and ReportFailureAsIssue any (workflow/compiler_types.go:736, holds bool or []string) are appropriate. pkg/typeutil/convert.go is the well-designed typed boundary. No recommendations.
Category 2: Untyped string consts that are really enums — the real wins
These are implicit enums declared as untyped string, while the codebase already models the same shape as named types (ActionMode, UniversalLLMBackend).
LLM provider — pkg/workflow/llm_provider.go:11-13: LLMProviderGitHub = "github" etc. → type LLMProvider string. Note universal_llm_consumer_engine.go:15 already does this with type UniversalLLMBackend string for the same names — worth resolving.
Safe-outputs URL policy — pkg/workflow/safe_outputs_validation.go:13-14: "allowed-only" / "allowed-or-code-region" → type SafeOutputsURLsPolicy string + IsValid() (validated via a switch at line 23).
MCP scripts mode — pkg/workflow/mcp_scripts_parser.go:65: "http" → type MCPScriptsMode string.
Higher-impact stringly-typed struct fields in pkg/workflow/engine.go:
:46EngineConfig.ID string — compared against "copilot", "codex", "claude", "custom" in 8+ sites (e.g. permissions_compiler_validator.go:212, mcp_setup_generator.go:116, threat_detection.go:290). A type EngineID string removes scattered magic strings. Highest-value fix.
:49.LLMProvider string (comment documents github|anthropic|openai) → use LLMProvider.
:50.PermissionMode string ("acceptEdits", "plan" per claude_engine.go:237) → candidate type PermissionMode string.
Not recommended: port consts in constants/constants.go:105-139 flow into fmt.Sprintf("%d")/net APIs — a type Port int forces pervasive conversions for little gain.
Category 3: Type assertions — idiomatic
~34 .(map[string]any) assertions walk decoded frontmatter YAML. Inherent shape of dynamic config; safe helpers already factored into pkg/typeutil/convert.go (LookupMap, LookupString, ParseBool). A nil-safety cleanup, not a missing-type problem. No new type warranted.
Refactoring Recommendations
High — consolidate pkg/cli reporting near-duplicates: Cluster 1 then Cluster 2. ~3-4h, removes drift.
Medium — name implicit string enums: LLMProvider, SafeOutputsURLsPolicy, MCPScriptsMode, ideally EngineID. ~2-4h, improves switch exhaustiveness/grep-ability.
Low — extract DiffCountsSummary base (Cluster 3). ~2h.
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
Uh oh!
There was an error while loading. Please reload this page.
-
🔤 Typist - Go Type Consistency Analysis
Analysis of repository: github/gh-aw
Executive Summary
Good news first: this codebase is already in strong shape on type consistency. I analyzed ~820 top-level type definitions across 947 non-test
.gofiles underpkg/, and there are zero exact cross-package duplicate structs needing a merge. Where a name repeats across packages it's almost always an intentionaltype X = pkg.Yalias (~20 of them, e.g.ActionPin,LogMetrics,SanitizeOptions,InputDefinition) or a//go:build js || wasmplatform stub — exactly the pattern you want.So this is a friendly nudge on a few small holdovers, not a big cleanup. On duplication I found 4 near-duplicate and 3 semantic-duplicate clusters, mostly in
pkg/clilog/audit reporting where parallel structs drifted in field naming. On typing,interface{}/anyusage is idiomatic everywhere (YAML/JSON frontmatter, theanalysis.Passframework signature, generics) — nothing fixable. The real quick wins are 3 string-const groups that are implicit enums but aren't namedstringtypes yet, mildly inconsistent given the codebase already does this well (ActionMode,UniversalLLMBackend).Full Analysis Report
Duplicated Type Definitions
Cluster 1:
MCPToolUsageSummaryvsMCPToolUsageData(near) — highest value, lowest riskSame package, near-identical:
pkg/cli/logs_models.go:178andpkg/cli/audit_report.go:163.Recommendation: Collapse to one canonical type (add the optional
GuardPolicySummary), alias the other. No import churn. ~1 hour.Cluster 2:
DomainAnalysisvsFirewallAnalysis(near)pkg/cli/access_log.go:34(AllowedCount, BlockedCount) vspkg/cli/firewall_log.go:131(AllowedRequests, BlockedRequests+RequestsByDomain). Both embedDomainBuckets; counters differ only by name. Same divergence inAccessLogSummary(logs_report_firewall.go:12) andFirewallLogSummary(:22). Recommendation: Unify counter names, extract a shared base. 2-3 hours.Cluster 3: Diff count summaries (near) — all in
pkg/cli/audit_diff.goFirewallDiffSummary(:50),MCPToolsDiffSummary(:240),ToolCallsDiffSummary(:299) repeat a "new/removed/changed counts + anomaly flags" shape. Recommendation: ExtractDiffCountsSummary{ New, Removed, Changed int; HasAnomalies bool; AnomalyCount int }and embed. 2 hours.Clusters 4-6 (semantic, low priority — leave as-is)
AccessLogEntry(access_log.go:20) vsFirewallLogEntry(firewall_log.go:116) — different log formats; merging adds empty fields.PRInfo(pr_command.go:27),PullRequest(pr_automerge.go:21),PullRequestData(intent/resolver.go:49) — minimal projections of distinctgh --jsoncalls.WorkflowRunInfo(run_workflow_tracking.go:19, 5 fields) is a strict subset ofWorkflowRun(logs_models.go:50, 28 fields). Slim struct intentional for the poll path.Untyped Usages
interface{}: ~1 real (rest are linter fixtures) — 0 fixableany: idiomatic everywhere (YAML/JSON boundary,analysis.Passsignature, generics) — 0 fixable.(map[string]any)assertions: 34 across 15 files — idiomatic frontmatter walking, already mitigated bypkg/typeutil/convert.goCategory 1:
interface{}/any— nothing to fixCorrect Go at the serialization boundary:
map[string]anyfor decoded frontmatter,func run(*analysis.Pass) (any, error)mandated bygolang.org/x/tools/go/analysis, generics. By-design polymorphic cases likeParseVersionValue(version any)(stringutil/stringutil.go:54) andReportFailureAsIssue any(workflow/compiler_types.go:736, holdsboolor[]string) are appropriate.pkg/typeutil/convert.gois the well-designed typed boundary. No recommendations.Category 2: Untyped string consts that are really enums — the real wins
These are implicit enums declared as untyped
string, while the codebase already models the same shape as named types (ActionMode,UniversalLLMBackend).pkg/workflow/llm_provider.go:11-13:LLMProviderGitHub = "github"etc. →type LLMProvider string. Noteuniversal_llm_consumer_engine.go:15already does this withtype UniversalLLMBackend stringfor the same names — worth resolving.pkg/workflow/safe_outputs_validation.go:13-14:"allowed-only"/"allowed-or-code-region"→type SafeOutputsURLsPolicy string+IsValid()(validated via aswitchat line 23).pkg/workflow/mcp_scripts_parser.go:65:"http"→type MCPScriptsMode string.Higher-impact stringly-typed struct fields in
pkg/workflow/engine.go::46EngineConfig.ID string— compared against"copilot","codex","claude","custom"in 8+ sites (e.g.permissions_compiler_validator.go:212,mcp_setup_generator.go:116,threat_detection.go:290). Atype EngineID stringremoves scattered magic strings. Highest-value fix.:49.LLMProvider string(comment documentsgithub|anthropic|openai) → useLLMProvider.:50.PermissionMode string("acceptEdits","plan"perclaude_engine.go:237) → candidatetype PermissionMode string.Not recommended: port consts in
constants/constants.go:105-139flow intofmt.Sprintf("%d")/netAPIs — atype Port intforces pervasive conversions for little gain.Category 3: Type assertions — idiomatic
~34
.(map[string]any)assertions walk decoded frontmatter YAML. Inherent shape of dynamic config; safe helpers already factored intopkg/typeutil/convert.go(LookupMap,LookupString,ParseBool). A nil-safety cleanup, not a missing-type problem. No new type warranted.Refactoring Recommendations
pkg/clireporting near-duplicates: Cluster 1 then Cluster 2. ~3-4h, removes drift.LLMProvider,SafeOutputsURLsPolicy,MCPScriptsMode, ideallyEngineID. ~2-4h, improves switch exhaustiveness/grep-ability.DiffCountsSummarybase (Cluster 3). ~2h.Implementation Checklist
MCPToolUsageSummary/MCPToolUsageData(Cluster 1)DomainAnalysis/FirewallAnalysiscounter names + shared base (Cluster 2)DiffCountsSummarybase (Cluster 3)type LLMProvider string; retype consts +EngineConfig.LLMProvidertype SafeOutputsURLsPolicy string+IsValid()type MCPScriptsMode stringtype EngineID stringreplacing magic engine-ID literalsAnalysis Metadata
pkg/) · Type definitions: ~820 · Duplicate clusters: 7 (0 exact, 4 near, 3 semantic) · Untyped findings: 3 const-enum wins + 3 struct fieldsBeta Was this translation helpful? Give feedback.
All reactions