[refactor] Split god-files and consolidate YAML path duplication in pkg/parser #65211

🔧 Semantic Function Clustering Analysis

Analysis scope: pkg/modelsdev/catalog.go + 58 non-test files in pkg/parser/ (15,510 lines), per the precomputed package slice for this run.

Executive Summary

Analyzed 59 Go source files (test files excluded). pkg/modelsdev/catalog.go is small (26 lines, 2 well-scoped functions) and shows no issues. pkg/parser/ is generally well-organized (most files are single-purpose, named after their feature), but three concrete, high-confidence findings stand out:

1. One outsized "god file" (import_field_extractor.go, 1129 lines / ~70 functions) mixing many unrelated extraction concerns on a single accumulator type.
2. One file (mcp.go, 786 lines) blending three distinct MCP configuration concerns that map cleanly onto three feature files.
3. A genuine functional duplicate: two independent implementations of "locate/extract a value at a JSON path inside raw YAML text," split across json_path_locator.go and schema_suggestions.go.

No other files in scope showed meaningful outliers, duplication, or scatter worth flagging — the WASM/native build-tag pairs (e.g. github.go/github_wasm.go, remote_resolve_path.go/remote_fetch_wasm.go, virtual_fs.go/virtual_fs_wasm.go) are intentional platform stubs, not duplicates, and were excluded.

Identified Issues

1. Oversized multi-concern file: pkg/parser/import_field_extractor.go (1129 lines)

Issue: Nearly all methods hang off a single *importAccumulator receiver, but they extract unrelated frontmatter concerns. By function-name clustering:

• Models/policies: appendModelsField, normalizeModelPolicies, normalizeModelAliases, parseModelPolicyField, sanitizeModelProvidersForCosts, isModelPolicyKey
• Activation: extractActivationFields, extractActivationSkipMatchFields, extractActivationGitHubToken, extractActivationGitHubAppFields, extractCheckoutField
• Engine/MCP config merge: extractEngineConfig, extractEngineMCPSettings, mergeSandboxAgentMounts
• Concurrency: extractConcurrencyInImport, extractConcurrencyJobDiscriminator
• Observability/features: extractFeatureAndObservabilityFields, appendObservabilityField, appendFeaturesField, appendCacheField
• Merge helpers: mergeBots, mergeSkipRoles, mergeSkipBots, mergeAmbientFolders, mergeExcludedEnv, mergeLabels, mergeJSONStringListField
• Generic field plumbing: extractFirstWinsJSONField, appendJSONBuilderField, appendJSONSliceField, appendYAMLBuilderField
• Result assembly: toImportsResult, buildImportsResult, populateImportsResultScalars

Recommendation: Keep importAccumulator (the type) in one place, but split its methods by concern into sibling files, e.g. import_field_models.go, import_field_activation.go, import_field_engine.go, import_field_concurrency.go, import_field_observability.go, leaving import_field_extractor.go with only the top-level extractImportFields/prepareFrontmatter orchestration and the generic field-plumbing helpers.
Estimated Impact: Easier to locate and test each extraction concern independently; reduces the odds of merge conflicts when multiple frontmatter fields are touched concurrently.

2. Mixed-concern file: pkg/parser/mcp.go (786 lines)

Issue: The file interleaves three independent MCP configuration concerns under one name:

• Safe-outputs/safe-jobs/mcpscripts injection: addSafeOutputsConfig, addSafeJobsConfig, addMCPScriptsConfig, safeOutputToolName, findOrCreateSafeOutputsConfig
• Builtin tool config building (Linear, GitHub): processBuiltinMCPTool, buildLinearBuiltinConfig, buildExpandedLinearBuiltinConfig, buildLinearAllowedTools, buildGitHubBuiltinConfig, githubBuiltinMode, appendBuiltinAllowedTools, applyGitHubBuiltinOverrides, replaceBuiltinImage, appendBuiltinArgs, extractBuiltinMCPTools
• Custom MCP server parsing (stdio/http): ParseMCPConfig, parseMCPSectionMap, inferOrReadMCPType, parseMCPRegistryValue, parseMCPStdioTypeConfig, configureMCPStdioContainer, configureMCPStdioCommand, appendContainerEnv, appendContainerMounts, appendNetworkProxyArgs, copyStringMapFromAny, parseMCPHTTPTypeConfig

Recommendation: Split into mcp_safe_outputs.go, mcp_builtin.go, and mcp_custom_server.go, keeping ExtractMCPConfigurations (the public entry point) and shared types (RegistryMCPServerConfig, IsMCPType, IsSimpleSecretExpression) in mcp.go.
Estimated Impact: Each file maps 1:1 to a feature a contributor is likely to touch (e.g. adding a new builtin tool vs. changing stdio/http parsing), reducing unrelated diff noise.

2.5. Duplicate logic: YAML path-value location implemented twice

Issue: Two independent implementations solve the same problem — "given a JSON path, find/extract the corresponding value or location inside raw YAML text":

• pkg/parser/json_path_locator.go: findPathInYAMLLines + matchesPathAtLevel — walks YAML lines tracking indentation level and array context to locate a path's line/column (used for error-location reporting).
• pkg/parser/schema_suggestions.go (lines 481–605): extractYAMLValueAtPath → extractTopLevelYAMLValue / extractNestedYAMLValue → buildYAMLScalarMatchers / matchYAMLScalar — a separate regex-based approach that extracts a scalar's value (not just location) at 1–2 level paths (used for suggestion generation).

Both solve "navigate raw YAML by JSON path," diverge in technique (indentation-based line scanner vs. regex matchers), and live in different files for unrelated reasons — schema_suggestions.go's YAML-navigation code is itself an outlier there, since the rest of that file is about generating human-readable suggestion text, not locating/extracting YAML values.

Recommendation: Move the YAML value-extraction cluster (extractYAMLValueAtPath and its helpers) out of schema_suggestions.go into json_path_locator.go (or a shared yaml_path.go), and consider whether findPathInYAMLLines's line-scanning could be reused/extended to also return the scalar value, eliminating the second regex-based implementation entirely.
Estimated Impact: Single source of truth for "YAML path navigation," removing ~125 lines of parallel logic and the risk of the two implementations drifting (e.g. one supporting array indices, the other not).

What Was Checked and Found Acceptable

• WASM/native build-tag pairs (GetGitHubToken in github.go vs github_wasm.go; ResolveIncludePath in remote_resolve_path.go vs remote_fetch_wasm.go; virtual-FS helpers in virtual_fs.go vs virtual_fs_wasm.go): same name, mutually-exclusive (go/redacted):build tags — intentional platform stubs per this repo's documented WASM-stub pattern, not duplicates.
• Error-message cleanup clusters (schema_errors.go's cleanJSONSchemaErrorMessage/cleanOneOfMessage/translateSchemaConstraintMessage vs. yaml_error.go's TranslateYAMLMessage/translateYAMLError): similarly named but operate on genuinely different error sources (jsonschema vs. goccy/go-yaml) with non-overlapping logic — acceptable domain separation.
• schema_compiler.go's four getCompiledXSchema functions: structurally identical (3 lines each, OnceLoader[T].Get(...)) but already minimal boilerplate using a generic cache; not worth collapsing further for 4 call sites.
• Small single-function files (path_section.go, frontmatter.go, github_token_env.go, validation_error.go, etc.) are intentionally minimal and correctly named for their single purpose.

Refactoring Recommendations

Priority 1

1. Split import_field_extractor.go by concern (models, activation, engine, concurrency, observability). Estimated effort: 3–5 hours. Benefit: isolates the highest-churn file in the package.
2. Split mcp.go by concern (safe-outputs, builtin, custom server). Estimated effort: 2–3 hours. Benefit: clearer ownership per MCP feature area.

Priority 2

3. Consolidate YAML path-value lookup between json_path_locator.go and schema_suggestions.go. Estimated effort: 2–4 hours (needs care to preserve both the indentation-based and regex-based edge cases during merge). Benefit: removes duplicate maintenance burden and the risk of behavioral drift between the two implementations.

Generated by 🔧 Semantic Function Refactoring · claude · sonnet50 · 244.4 AIC · ⌖ 37.5 AIC · ⊞ 5.7K · ◷

• expires on Oct 4, 2026, 6:52 PM UTC-08:00