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
I scanned all 1,181 non-test .go files under pkg/ (~986 top-level struct/interface type declarations) looking for duplicated types and untyped (any/interface{}) usage that could be made stronger. The good news: the team has already applied real dedup patterns in several places (types.BaseMCPServerConfig, AnalysisBase, AggregatedSummaryBase), and interface{} itself is essentially gone from production code — everything has migrated to any. The remaining opportunities cluster tightly around one area: the pkg/cli audit/logs/gateway reporting subsystem, where the same "per-MCP-server stats" and "diff between two runs" concepts have been independently reinvented four and two times respectively as the tooling grew feature-by-feature. On the any side, most usage is legitimate (generic constraints, GitHub-Actions-analyzer boilerplate, or genuinely open YAML/JSON), but about 20 signatures pass around any/map[string]any for values whose shape is actually fixed and well-documented — the biggest single win being getParsedSchemaDoc in pkg/parser, whose any return forces the same type assertion into 10 different call sites.
Fixing the top handful of these (consolidating the MCP-server-stats structs, and typing the schema-doc/cache-config/engine-auth anys) would remove real drift risk in the audit reporting code and delete a good number of repeated type assertions — most of it is a few hours of mechanical work per cluster, not a big redesign.
Full Analysis Report
Duplicated Type Definitions
Summary Statistics
Total types analyzed: ~986 struct/interface declarations across 34 top-level pkg/ subpackages
Duplicate clusters found: 10 (reported below)
Exact duplicates: 3 (all intentional Go build-tag platform variants — not bugs)
Near duplicates: 3
Semantic duplicates: 4
Cluster 1: "Per-MCP-server stats/health" modeled four separate times
Type: Semantic duplicate Occurrences: 4 Impact: High — same concept reimplemented per report generator, real drift risk
Introduce a shared MCPServerStatsBase{ServerName string; RequestCount, ToolCallCount, ErrorCount int} (mirroring the existing AnalysisBase/AggregatedSummaryBase pattern already used in this same package) and have each report-specific struct embed it, adding only its own extra fields (latency, rate, cross-run counts, gateway filtering).
Estimated effort: 3-4 hours
Benefits: One place to fix "MCP server stats" bugs instead of four; new fields (e.g. a new counter) only need adding once.
Cluster 2: "Per-tool aggregate call stats" modeled three separate times
Type: Near/semantic duplicate Occurrences: 3 Impact: Medium-High — companion to Cluster 1, same root cause
All three carry CallCount, TotalInputSize/MaxInputSize, TotalOutputSize/MaxOutputSize, AvgDuration/MaxDuration, ErrorCount under different naming conventions.
Recommendation: Same base-struct treatment as Cluster 1; do both together since they share call sites in the audit report generators. Estimated effort: 2-3 hours (bundled with Cluster 1)
Cluster 3: Domain/firewall request-counter base duplicated
Type: Near duplicate Occurrences: 3 (1 already-fixed, 2 still duplicated) Impact: Medium
Locations:
pkg/cli/domain_buckets.go:41 — AnalysisBase{DomainBuckets, TotalRequests, AllowedRequests, BlockedRequests} — already used as a dedup base for DomainAnalysis/FirewallAnalysis (good existing pattern!)
pkg/cli/logs_report_firewall.go:17 — FirewallSummaryBase — the same 3 ints + 2 string slices, but flattened instead of reusing AnalysisBase
pkg/cli/firewall_policy.go:79 — PolicyAnalysis{TotalRequests, AllowedCount, DeniedCount, UniqueDomains} — same concept again
Recommendation: Point FirewallSummaryBase (used by AccessLogSummary/FirewallLogSummary) and PolicyAnalysis at the existing AnalysisBase instead of re-declaring the same fields — this one should be an easy win since the base type already exists and is proven. Estimated effort: 1-2 hours Benefits: Removes the last copy of a pattern the team already fixed once; prevents the fix from being "half-applied."
Cluster 4: Two independent "compare two audit runs" subsystems
Type: Semantic duplicate (subsystem-level) Occurrences: 2 parallel type families (~15 types total) Impact: Medium — different vocab for the same concern, but lower urgency (both work correctly today)
Recommendation: Longer-term — pick one vocabulary (the DiffEntryBase generic-delta pattern in audit_diff.go looks more reusable) and migrate audit_comparison.go onto it. This is a bigger effort than the others; treat as a backlog item rather than a quick fix. Estimated effort: 1-2 days Benefits: One diffing mental model for anyone extending audit comparisons.
SpinnerWrapper — pkg/console/spinner.go:92 vs pkg/console/spinner_wasm.go:10
RepositoryFeatures — pkg/workflow/repository_features_validation.go:73 vs repository_features_validation_wasm.go:31
These are mutually-exclusive compilation units (native vs WASM build), not accidental copy-paste. No action needed — flagged for completeness.
Cluster 8: EngineCapabilities vs EngineCapabilitiesDefinition
Type: Near duplicate (deliberate mirror, already partially handled) Locations: pkg/workflow/agentic_engine.go:113 vs pkg/workflow/engine_definition.go:126, connected by an explicit ToRuntimeCapabilities() converter at engine_definition.go:138.
Recommendation: Lower priority — already has a converter keeping them in sync. Worth a code comment noting "if you add a capability, update both structs + the converter," since nothing currently enforces that at compile time.
Cluster 9: MCP server config — mostly already consolidated, one more model exists
types.BaseMCPServerConfig (pkg/types/mcp.go:6) is already embedded by both parser.RegistryMCPServerConfig and workflow.MCPServerConfig — a good existing example of the base-struct pattern. pkg/cli/mcp_registry_types.go (ServerDetail/MCPPackage/Transport/etc.) models an overlapping concept a third way, but it mirrors an external upstream registry API schema, so this is likely intentional. No action recommended.
cachedLatestRelease/cachedSHA (pkg/cli/update_actions.go:48,54), cachedDefaultBranch/cachedBranchCommit (pkg/cli/update_workflows.go:46,51), dockerPullState (pkg/cli/docker_images.go:54) — each a tiny {value, err} memoization struct reinvented per cache. A generic Result[T] would be a nice-to-have but not urgent.
Untyped Usages
Summary Statistics
interface{} usages in production code: 0 (fully migrated to any; the handful of remaining hits are in pkg/linters/*/testdata test fixtures, out of scope)
any occurrences surveyed: legitimate uses dominate — ~11 correct generic-constraint usages (func Map[T any](...) etc.), ~65 framework-mandated (func run(pass *analysis.Pass) (any, error) required by golang.org/x/tools/go/analysis), and 500+ legitimate dynamic YAML/JSON/frontmatter parsing sites (open-schema user config, by design)
Real candidates for stronger typing: ~20 distinct signatures/fields
Untyped constants found: ~34 (2 in pkg/workflow, 2 in pkg/cli, ~30 in pkg/constants/*.go)
Category 1: any return/parameter types with a knowable concrete shape
Impact: High — forces repeated type assertions at every call site
Example 1: getParsedSchemaDoc — the single highest-impact fix found
Location: pkg/parser/schema_compiler.go:101
Current signature: func getParsedSchemaDoc(schemaJSON string) (any, error)
Actual usage: always unmarshals a JSON object; every one of 9+ callers in pkg/parser/schema_suggestions.go (lines 76, 101, 245, 321, 344, 360, 605, 648, 737) immediately does schemaDoc.(map[string]any)
Location: pkg/workflow/cache.go:445-510 — writeCachePath(builder, path any), writeCacheRestoreKeys(builder, restoreKeys any), writeCacheStepValue(builder, key string, value any), all fed from a cache map[string]any that actually models the fixed actions/cache action schema
Suggested fix: parse once into type cacheStepConfig struct{ Key string; Path []string; RestoreKeys []string; UploadChunkSize *int; FailOnCacheMiss, LookupOnly *bool }
Example 4: 2-variant sum type leaking across package boundary
Location: pkg/parser/import_observability.go:16-19 — observabilityImportEndpoint.Headers any, documented as "string or map", consumed by a switch in pkg/workflow over exactly string/map[string]any
Suggested fix: type OTLPHeaders struct{ Raw string; Map map[string]string }
Example 5: engine auth field
Location: pkg/workflow/behavior_defined_engine.go:685-691 — isEngineAuthConfigMapping(auth any) bool, documented as exactly one of []AuthBinding or {type: "github-oidc"}
Suggested fix: type EngineAuthField struct{ Bindings []AuthBinding; OIDC *EngineAuthConfig } with custom unmarshal logic
Example 6: model-cost merging (compiler-internal, not user data)
Location: pkg/workflow/compiler_model_pricing.go:23,124,165 — three functions pass map[string]any for a fixed internal shape {providers: {<name>: {models: {<name>: {cost: float64}}}}}
pkg/console/console_types.go:50 — FormField.Value any (Type is a closed 4-value enum); Value is unused in production outside a WASM stub
pkg/console/layout_wasm.go:16 — LayoutEmphasisBox(content string, color any): color is unused in the function body; docs say the intended type is lipgloss.Color
pkg/workflow/yaml.go:448-489 — formatYAMLValue(value any) string switches over 13 numeric variants, but its only caller only ever feeds string/bool/int/uint64/float64 — 9 branches are unreachable
Impact: Medium — lack of semantic clarity, and the compiler can't stop a stray string from being passed where the enum was meant
Example 1: model-mismatch reason codes
Current (pkg/cli/token_usage_types.go:94,147-148): ReasonCode string populated only via const modelMismatchReasonTokenUsageMissing = "TOKEN_USAGE_MISSING" and const modelMismatchReasonModelNotObserved = "REQUESTED_MODEL_NOT_OBSERVED".
Suggested fix: type ModelMismatchReason string with typed constants — the codebase already does this correctly for OutcomeResult in pkg/cli/outcome_eval.go:26-37; just extend the same pattern here.
Example 2: experiments storage mode
Current (pkg/workflow/workflow_data.go:188, compiler_experiments.go:25,29): ExperimentsStorage string // "cache" or "repo" with const ExperimentsStorageCache = "cache" / const ExperimentsStorageRepo = "repo".
Suggested fix: type ExperimentsStorageMode string with typed constants.
Example 3: pkg/constants/constants.go — mixed untyped semantic values
Network ports (lines ~100-136): DefaultMCPGatewayPort = 8080, MinNetworkPort = 1, MaxNetworkPort = 65535, ClaudeLLMGatewayPort = 10000 and 6 more — all untyped, would benefit from type NetworkPort int
DefaultMaxDailyAICredits = "5000" (line ~329) — a number stored as an untyped string, inconsistent with the sibling DefaultMaxAICredits int64 = 1000 a few lines away (this one looks like a real inconsistency, not just a style nit)
pkg/constants/job_constants.go:92-198 — ~19 untyped *ArtifactName/*Filename string constants sit right next to named JobName/StepID/MCPServerID types defined a few lines above in the same file — the typed pattern is already established here, just not applied consistently to every constant group.
Locations: pkg/constants/constants.go, pkg/constants/job_constants.go Benefits: Type safety, clearer intent, catches the string/int mismatch on DefaultMaxDailyAICredits at compile time instead of at parse time.
🎯 What Should We Do About This?
Here's a suggested action plan, prioritized by impact and effort. Start with the biggest, cheapest wins.
Recommendation: Point FirewallSummaryBase and PolicyAnalysis at the already-existing AnalysisBase instead of re-declaring the same 3 ints + domain fields.
Steps:
Update AccessLogSummary/FirewallLogSummary to embed AnalysisBase instead of FirewallSummaryBase
Update PolicyAnalysis similarly or fold it into the same base
Run tests
Estimated effort: 1-2 hours Impact: Medium — completes a pattern the team already validated once
Priority 2: High-value type fix — getParsedSchemaDoc and friends
Recommendation: Change getParsedSchemaDoc to return (map[string]any, error), dropping the type assertion at all 10 call sites; do the same for the cache-config and engine-auth any fields (Examples 1-5 above).
Steps:
Change the function signature and remove assertions in pkg/parser/schema_suggestions.go
Introduce cacheStepConfig, OTLPHeaders, EngineAuthField structs and update their few call sites
Run tests
Estimated effort: 4-6 hours total across the 5 examples Impact: High — removes ~15 repeated type assertions, closes several YAML-schema-drift bug classes
Recommendation: Introduce MCPServerStatsBase and a matching per-tool base, embed in the four/three report-specific structs.
Steps:
Create the base struct(s) in a shared location (e.g. alongside AnalysisBase in pkg/cli)
Refactor MCPServerStats, MCPServerHealthDetail, MCPServerCrossRunHealth, GatewayServerMetrics to embed it
Same for ToolUsageInfo/MCPToolSummary/GatewayToolMetrics
Run the full audit-report test suite carefully — these feed user-facing report output
Estimated effort: 5-7 hours Impact: High — removes the largest concrete duplication cluster found
Priority 4: Typed enums for constants
Recommendation: Add named types to the closed-enum constants (ModelMismatchReason, ExperimentsStorageMode, NetworkPort) and fix the DefaultMaxDailyAICredits string/int inconsistency.
Estimated effort: 2-3 hours Impact: Medium — improved clarity, catches at least one real inconsistency at compile time
Priority 5 (backlog): Unify the two audit-diff subsystems (Cluster 4)
Bigger effort, both subsystems work correctly today — track as a longer-term backlog item rather than doing it opportunistically.
Implementation Checklist
Point FirewallSummaryBase/PolicyAnalysis at existing AnalysisBase
Change getParsedSchemaDoc to return map[string]any and drop call-site assertions
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
I scanned all 1,181 non-test
.gofiles underpkg/(~986 top-level struct/interface type declarations) looking for duplicated types and untyped (any/interface{}) usage that could be made stronger. The good news: the team has already applied real dedup patterns in several places (types.BaseMCPServerConfig,AnalysisBase,AggregatedSummaryBase), andinterface{}itself is essentially gone from production code — everything has migrated toany. The remaining opportunities cluster tightly around one area: thepkg/cliaudit/logs/gateway reporting subsystem, where the same "per-MCP-server stats" and "diff between two runs" concepts have been independently reinvented four and two times respectively as the tooling grew feature-by-feature. On theanyside, most usage is legitimate (generic constraints, GitHub-Actions-analyzer boilerplate, or genuinely open YAML/JSON), but about 20 signatures pass aroundany/map[string]anyfor values whose shape is actually fixed and well-documented — the biggest single win beinggetParsedSchemaDocinpkg/parser, whoseanyreturn forces the same type assertion into 10 different call sites.Fixing the top handful of these (consolidating the MCP-server-stats structs, and typing the schema-doc/cache-config/engine-auth
anys) would remove real drift risk in the audit reporting code and delete a good number of repeated type assertions — most of it is a few hours of mechanical work per cluster, not a big redesign.Full Analysis Report
Duplicated Type Definitions
Summary Statistics
pkg/subpackagesCluster 1: "Per-MCP-server stats/health" modeled four separate times
Type: Semantic duplicate
Occurrences: 4
Impact: High — same concept reimplemented per report generator, real drift risk
Locations:
pkg/cli/audit_report.go:213—MCPServerStats{ServerName, RequestCount, ToolCallCount, TotalInputSize, TotalOutputSize, AvgDuration, ErrorCount}pkg/cli/audit_expanded.go:90—MCPServerHealthDetail{ServerName, RequestCount, ToolCalls, ErrorCount, ErrorRate, ErrorRateStr, AvgLatency, Status}pkg/cli/audit_cross_run.go:81—MCPServerCrossRunHealth{ServerName, RunsConnected, TotalRuns, TotalCalls, TotalErrors, ErrorRate, Unreliable}pkg/cli/gateway_logs_types.go:85—GatewayServerMetrics{ServerName, RequestCount, ToolCallCount, TotalDuration, ErrorCount, FilteredCount, GuardPolicyBlocked, Tools}Recommendation:
MCPServerStatsBase{ServerName string; RequestCount, ToolCallCount, ErrorCount int}(mirroring the existingAnalysisBase/AggregatedSummaryBasepattern already used in this same package) and have each report-specific struct embed it, adding only its own extra fields (latency, rate, cross-run counts, gateway filtering).Cluster 2: "Per-tool aggregate call stats" modeled three separate times
Type: Near/semantic duplicate
Occurrences: 3
Impact: Medium-High — companion to Cluster 1, same root cause
Locations:
pkg/cli/audit_report.go:166—ToolUsageInfopkg/cli/audit_report.go:184—MCPToolSummarypkg/cli/gateway_logs_types.go:97—GatewayToolMetricsAll three carry
CallCount,TotalInputSize/MaxInputSize,TotalOutputSize/MaxOutputSize,AvgDuration/MaxDuration,ErrorCountunder different naming conventions.Recommendation: Same base-struct treatment as Cluster 1; do both together since they share call sites in the audit report generators.
Estimated effort: 2-3 hours (bundled with Cluster 1)
Cluster 3: Domain/firewall request-counter base duplicated
Type: Near duplicate
Occurrences: 3 (1 already-fixed, 2 still duplicated)
Impact: Medium
Locations:
pkg/cli/domain_buckets.go:41—AnalysisBase{DomainBuckets, TotalRequests, AllowedRequests, BlockedRequests}— already used as a dedup base forDomainAnalysis/FirewallAnalysis(good existing pattern!)pkg/cli/logs_report_firewall.go:17—FirewallSummaryBase— the same 3 ints + 2 string slices, but flattened instead of reusingAnalysisBasepkg/cli/firewall_policy.go:79—PolicyAnalysis{TotalRequests, AllowedCount, DeniedCount, UniqueDomains}— same concept againRecommendation: Point
FirewallSummaryBase(used byAccessLogSummary/FirewallLogSummary) andPolicyAnalysisat the existingAnalysisBaseinstead of re-declaring the same fields — this one should be an easy win since the base type already exists and is proven.Estimated effort: 1-2 hours
Benefits: Removes the last copy of a pattern the team already fixed once; prevents the fix from being "half-applied."
Cluster 4: Two independent "compare two audit runs" subsystems
Type: Semantic duplicate (subsystem-level)
Occurrences: 2 parallel type families (~15 types total)
Impact: Medium — different vocab for the same concern, but lower urgency (both work correctly today)
Locations:
pkg/cli/audit_comparison.go:21-89—AuditComparisonData/AuditComparisonBaseline/AuditComparisonDelta/AuditComparisonIntDelta/AuditComparisonStringDelta/AuditComparisonMCPFailureDelta/AuditComparisonClassification/AuditComparisonRecommendation(Before/After/Changed vocabulary)pkg/cli/audit_diff.go:24-381—DiffEntryBase/DomainDiffEntry/FirewallDiff(Summary)/MCPToolDiffEntry/MCPToolsDiff(Summary)/TokenUsageDiff/ToolCallDiffEntry/BashCommandsDiff/ToolCallsDiff(Summary)/RunMetricsDiff/GitHubRateLimitDiff/AuditDiff(Run1/Run2/Status vocabulary)Recommendation: Longer-term — pick one vocabulary (the
DiffEntryBasegeneric-delta pattern inaudit_diff.golooks more reusable) and migrateaudit_comparison.goonto it. This is a bigger effort than the others; treat as a backlog item rather than a quick fix.Estimated effort: 1-2 days
Benefits: One diffing mental model for anyone extending audit comparisons.
Clusters 5-7: Exact-name duplicates — intentional build-tag platform variants (informational only, not bugs)
ProgressBar—pkg/console/progress.go:31(!js && !wasm) vspkg/console/progress_wasm.go:7(js || wasm)SpinnerWrapper—pkg/console/spinner.go:92vspkg/console/spinner_wasm.go:10RepositoryFeatures—pkg/workflow/repository_features_validation.go:73vsrepository_features_validation_wasm.go:31These are mutually-exclusive compilation units (native vs WASM build), not accidental copy-paste. No action needed — flagged for completeness.
Cluster 8:
EngineCapabilitiesvsEngineCapabilitiesDefinitionType: Near duplicate (deliberate mirror, already partially handled)
Locations:
pkg/workflow/agentic_engine.go:113vspkg/workflow/engine_definition.go:126, connected by an explicitToRuntimeCapabilities()converter atengine_definition.go:138.Recommendation: Lower priority — already has a converter keeping them in sync. Worth a code comment noting "if you add a capability, update both structs + the converter," since nothing currently enforces that at compile time.
Cluster 9: MCP server config — mostly already consolidated, one more model exists
types.BaseMCPServerConfig(pkg/types/mcp.go:6) is already embedded by bothparser.RegistryMCPServerConfigandworkflow.MCPServerConfig— a good existing example of the base-struct pattern.pkg/cli/mcp_registry_types.go(ServerDetail/MCPPackage/Transport/etc.) models an overlapping concept a third way, but it mirrors an external upstream registry API schema, so this is likely intentional. No action recommended.Cluster 10: Memoization "result+err" cache-entry pairs (minor, informational)
cachedLatestRelease/cachedSHA(pkg/cli/update_actions.go:48,54),cachedDefaultBranch/cachedBranchCommit(pkg/cli/update_workflows.go:46,51),dockerPullState(pkg/cli/docker_images.go:54) — each a tiny{value, err}memoization struct reinvented per cache. A genericResult[T]would be a nice-to-have but not urgent.Untyped Usages
Summary Statistics
interface{}usages in production code: 0 (fully migrated toany; the handful of remaining hits are inpkg/linters/*/testdatatest fixtures, out of scope)anyoccurrences surveyed: legitimate uses dominate — ~11 correct generic-constraint usages (func Map[T any](...)etc.), ~65 framework-mandated (func run(pass *analysis.Pass) (any, error)required bygolang.org/x/tools/go/analysis), and 500+ legitimate dynamic YAML/JSON/frontmatter parsing sites (open-schema user config, by design)pkg/workflow, 2 inpkg/cli, ~30 inpkg/constants/*.go)Category 1:
anyreturn/parameter types with a knowable concrete shapeImpact: High — forces repeated type assertions at every call site
Example 1:
getParsedSchemaDoc— the single highest-impact fix foundpkg/parser/schema_compiler.go:101func getParsedSchemaDoc(schemaJSON string) (any, error)pkg/parser/schema_suggestions.go(lines 76, 101, 245, 321, 344, 360, 605, 648, 737) immediately doesschemaDoc.(map[string]any)func getParsedSchemaDoc(schemaJSON string) (map[string]any, error)Example 2: audit "before/after state" tracking
pkg/cli/audit_report.go:152-153(CreatedItemReport.BeforeState/AfterState map[string]any), consumed inpkg/cli/outcome_eval_update.go:153-222title, body_hash, state, labels, assignees, base, draft, head_sha) with fixed value typestype MutableItemState struct{ Title, BodyHash, State, Base, HeadSHA string; Labels, Assignees []string; Draft bool }mutableString/mutableBool/mutableStringSlicehelper-assertion functions entirely.Example 3: cache action config
pkg/workflow/cache.go:445-510—writeCachePath(builder, path any),writeCacheRestoreKeys(builder, restoreKeys any),writeCacheStepValue(builder, key string, value any), all fed from acache map[string]anythat actually models the fixedactions/cacheaction schematype cacheStepConfig struct{ Key string; Path []string; RestoreKeys []string; UploadChunkSize *int; FailOnCacheMiss, LookupOnly *bool }Example 4: 2-variant sum type leaking across package boundary
pkg/parser/import_observability.go:16-19—observabilityImportEndpoint.Headers any, documented as "string or map", consumed by aswitchinpkg/workflowover exactlystring/map[string]anytype OTLPHeaders struct{ Raw string; Map map[string]string }Example 5: engine auth field
pkg/workflow/behavior_defined_engine.go:685-691—isEngineAuthConfigMapping(auth any) bool, documented as exactly one of[]AuthBindingor{type: "github-oidc"}type EngineAuthField struct{ Bindings []AuthBinding; OIDC *EngineAuthConfig }with custom unmarshal logicExample 6: model-cost merging (compiler-internal, not user data)
pkg/workflow/compiler_model_pricing.go:23,124,165— three functions passmap[string]anyfor a fixed internal shape{providers: {<name>: {models: {<name>: {cost: float64}}}}}ModelCostsConfig{Providers map[string]ModelProviderCosts}/ModelProviderCosts{Models map[string]ModelCostEntry}/ModelCostEntry{Cost float64}Example 7: GitHub REST API responses
pkg/cli/outcome_eval_review.go+outcome_eval.go:252,278—ghAPIGet/ghAPIGetArrayreturnmap[string]any/[]map[string]anyfor well-documented, fixed GitHub PR/review/commit schemas, forcing assertions likepr["merged"].(bool)PullRequestSummary{Merged bool; State, MergedAt string},Review{State, SubmittedAt string}Example 8: dead-code-adjacent
anyparamspkg/console/console_types.go:50—FormField.Value any(Type is a closed 4-value enum);Valueis unused in production outside a WASM stubpkg/console/layout_wasm.go:16—LayoutEmphasisBox(content string, color any):coloris unused in the function body; docs say the intended type islipgloss.Colorpkg/workflow/yaml.go:448-489—formatYAMLValue(value any) stringswitches over 13 numeric variants, but its only caller only ever feedsstring/bool/int/uint64/float64— 9 branches are unreachableCategory 2: Untyped constants representing closed enums
Impact: Medium — lack of semantic clarity, and the compiler can't stop a stray string from being passed where the enum was meant
Example 1: model-mismatch reason codes
Current (
pkg/cli/token_usage_types.go:94,147-148):ReasonCode stringpopulated only viaconst modelMismatchReasonTokenUsageMissing = "TOKEN_USAGE_MISSING"andconst modelMismatchReasonModelNotObserved = "REQUESTED_MODEL_NOT_OBSERVED".Suggested fix:
type ModelMismatchReason stringwith typed constants — the codebase already does this correctly forOutcomeResultinpkg/cli/outcome_eval.go:26-37; just extend the same pattern here.Example 2: experiments storage mode
Current (
pkg/workflow/workflow_data.go:188,compiler_experiments.go:25,29):ExperimentsStorage string // "cache" or "repo"withconst ExperimentsStorageCache = "cache"/const ExperimentsStorageRepo = "repo".Suggested fix:
type ExperimentsStorageMode stringwith typed constants.Example 3:
pkg/constants/constants.go— mixed untyped semantic valuesDefaultMCPGatewayPort = 8080,MinNetworkPort = 1,MaxNetworkPort = 65535,ClaudeLLMGatewayPort = 10000and 6 more — all untyped, would benefit fromtype NetworkPort intDefaultMaxDailyAICredits = "5000"(line ~329) — a number stored as an untyped string, inconsistent with the siblingDefaultMaxAICredits int64 = 1000a few lines away (this one looks like a real inconsistency, not just a style nit)DefaultMCPGatewayPayloadSizeThreshold = 524288(line ~245) — untyped byte-size constantpkg/constants/job_constants.go:92-198— ~19 untyped*ArtifactName/*Filenamestring constants sit right next to namedJobName/StepID/MCPServerIDtypes defined a few lines above in the same file — the typed pattern is already established here, just not applied consistently to every constant group.Locations:
pkg/constants/constants.go,pkg/constants/job_constants.goBenefits: Type safety, clearer intent, catches the string/int mismatch on
DefaultMaxDailyAICreditsat compile time instead of at parse time.🎯 What Should We Do About This?
Here's a suggested action plan, prioritized by impact and effort. Start with the biggest, cheapest wins.
Priority 1: Quick win — finish the firewall base-struct dedup (Cluster 3)
Recommendation: Point
FirewallSummaryBaseandPolicyAnalysisat the already-existingAnalysisBaseinstead of re-declaring the same 3 ints + domain fields.Steps:
AccessLogSummary/FirewallLogSummaryto embedAnalysisBaseinstead ofFirewallSummaryBasePolicyAnalysissimilarly or fold it into the same baseEstimated effort: 1-2 hours
Impact: Medium — completes a pattern the team already validated once
Priority 2: High-value type fix —
getParsedSchemaDocand friendsRecommendation: Change
getParsedSchemaDocto return(map[string]any, error), dropping the type assertion at all 10 call sites; do the same for the cache-config and engine-authanyfields (Examples 1-5 above).Steps:
pkg/parser/schema_suggestions.gocacheStepConfig,OTLPHeaders,EngineAuthFieldstructs and update their few call sitesEstimated effort: 4-6 hours total across the 5 examples
Impact: High — removes ~15 repeated type assertions, closes several YAML-schema-drift bug classes
Priority 3: Consolidate MCP-server-stats reporting structs (Clusters 1 & 2)
Recommendation: Introduce
MCPServerStatsBaseand a matching per-tool base, embed in the four/three report-specific structs.Steps:
AnalysisBaseinpkg/cli)MCPServerStats,MCPServerHealthDetail,MCPServerCrossRunHealth,GatewayServerMetricsto embed itToolUsageInfo/MCPToolSummary/GatewayToolMetricsEstimated effort: 5-7 hours
Impact: High — removes the largest concrete duplication cluster found
Priority 4: Typed enums for constants
Recommendation: Add named types to the closed-enum constants (
ModelMismatchReason,ExperimentsStorageMode,NetworkPort) and fix theDefaultMaxDailyAICreditsstring/int inconsistency.Estimated effort: 2-3 hours
Impact: Medium — improved clarity, catches at least one real inconsistency at compile time
Priority 5 (backlog): Unify the two audit-diff subsystems (Cluster 4)
Bigger effort, both subsystems work correctly today — track as a longer-term backlog item rather than doing it opportunistically.
Implementation Checklist
FirewallSummaryBase/PolicyAnalysisat existingAnalysisBasegetParsedSchemaDocto returnmap[string]anyand drop call-site assertionscacheStepConfig,OTLPHeaders,EngineAuthField,ModelCostsConfigtypesMCPServerStatsBase/ per-tool stats base and embed in the four report structsModelMismatchReasonandExperimentsStorageModenamed typesNetworkPorttype inpkg/constantsand fixDefaultMaxDailyAICreditsstring/int mismatchaudit_comparison.go/audit_diff.govocabulariesAnalysis Metadata
pkg/anycandidates + ~34 untyped constantsAll reactions