[sergo] Sergo Report: Function Complexity + Error Handling Analysis — 2026-04-17 #26935
Closed
Replies: 1 comment
-
|
This discussion was automatically closed because it expired on 2026-04-18T20:43:38.884Z.
|
Beta Was this translation helpful? Give feedback.
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.
-
Date: 2026-04-17
Strategy: Function Complexity Analysis + Error Handling Audit (First Run)
Success Score: 7/10
Run ID: §24585088732
This is the inaugural Sergo run for this repository. With no prior cache to draw from, 100% new exploration was applied across two analysis axes: function complexity (size/decomposability) and error handling hygiene (silent discards, missing wrapping).
Executive Summary
Analysis of 719 non-test Go files across
pkg/revealed two dominant code quality themes. First, several core functions have grown to extreme length (497–841 lines), concentrating high-risk logic into single untestable units. Second, post-compilation side effects silently swallow errors that users would benefit from seeing. Three actionable issues were created; no duplicates were found in existing open issues.Serena Tools Update
Tools Snapshot
Tool Capabilities Used Today
search_for_patternpanic(,_ =,fmt.Errorf(%v, mutex Lock callsget_symbols_overviewcache.golist_dirpkg/subdirectoriesfind_symbolAnalyzeinagentdrain/anomaly.goStrategy Selection
New Exploration (100% — first run, no cache)
Strategy name:
function-complexity-plus-error-handlingRationale: With no prior strategy history, I selected two complementary analysis axes that are high-yield in large Go codebases:
Function complexity analysis — large functions (>100 lines) are a leading predictor of bugs, test gaps, and merge conflicts. The approach: measure actual function line counts, rank by size, investigate the worst offenders for structural issues.
Error handling hygiene — Go's explicit error model is only as good as its discipline. Common failure modes:
_ =discards hiding real failures,%vinstead of%wbreaking error unwrapping, barereturn errlosing context. The approach: grep for_ =,fmt.Errorf.*%v.*err, andpanic(in production paths.Combined strategy rationale: Complexity and error handling are orthogonal—a simple function can have poor error handling, and a complex function can have fine error handling. Running both gives broader coverage per run.
Analysis Execution
Codebase Context
pkg/pkg/workflow/,pkg/parser/,pkg/cli/,pkg/agentdrain/*_wasm.goembedded blobs)Findings Summary
Detailed Findings
High Priority: Extreme Function Lengths
Finding H1 —
generateMCPSetup: 841 lines (pkg/workflow/mcp_setup_generator.go:77)The
(*Compiler).generateMCPSetupmethod spans 841 lines. It handles at least 6 distinct concerns: collecting which MCP tools are active, populating dispatch/call workflow file mappings, generating YAML for each tool type (GitHub, Playwright, safe-outputs, mcp-scripts, agentic-workflows, custom MCP), gateway routing config, environment variable injection, and volume mounts.No sub-function call structure exists — it is one monolithic body. This means:
Finding H2 —
processImportsFromFrontmatterWithManifestAndSource: 497 lines (pkg/parser/import_bfs.go:20)The import resolution pipeline (parse → BFS fetch → topological sort → merge) is implemented in a single 497-line function. The BFS loop, cache lookup, manifest application, circular-dependency detection, and result merging are all interleaved. Extracting even one phase (e.g., the topological sort, which already has a standalone
topologicalSortImportshelper) would immediately improve readability.Medium Priority: Error Handling
Finding M1 — Silent post-compile error discards (
pkg/cli/compile_file_operations.go:160–161)Both functions return errors and perform internal logging — but only at the debug logger level. In production non-verbose runs, a filesystem permission error on cache save produces no user-visible output. The compile appears to succeed while silently leaving the user with a degraded cache state.
Finding M2 —
context.Background()as a function return value (pkg/cli/engine_secrets.go:182)This is a well-commented defensive fallback and is not a bug per se. However, callers that pass a nil
Ctxget a non-cancellable context, meaning engine secret operations can't be interrupted if the caller's context is cancelled. Medium priority: consider makingCtxrequired or propagating cancellation.Low Priority: Panic at Init Time
Finding L1 — Package-level
panic()in embedded JSON loadingSeveral files panic when deserializing embedded JSON at startup:
pkg/workflow/permissions_toolset_data.go:44pkg/workflow/domains.go:198pkg/actionpins/actionpins.go:86pkg/workflow/github_tool_to_toolset.go:23This is an accepted Go pattern for
//go:embedinit—if the embedded resource is malformed, the binary is broken by definition and panicking at startup is appropriate. Not an issue to fix, but worth noting for awareness. Thepkg/workflow/strings.go:290panic oncrypto/randfailure is also justified.Improvement Tasks Generated
Task 1: Decompose
generateMCPSetup(841 lines) — HIGHIssue:
pkg/workflow/mcp_setup_generator.go:77single function handles entire MCP setup generationApproach:
(*Compiler)method of ≤80 linesEstimated Effort: Medium | Before starting: run
go test ./pkg/workflow/...to establish baselineTask 2: Fix silent post-compile error discards — MEDIUM
Issue:
pkg/cli/compile_file_operations.go:160–161discards errors from cache save and .gitattributes updateApproach:
Estimated Effort: Small (2–4 lines + 1 test)
Task 3: Decompose
processImportsFromFrontmatterWithManifestAndSource(497 lines) — HIGHIssue:
pkg/parser/import_bfs.go:20— entire import BFS pipeline in one functionApproach:
parseImportSpecs(frontmatter) ([]ImportSpec, error)— the switch on frontmatter typesbfsResolveImports(specs, baseDir, cache, path) ([]ProcessedImport, error)— the BFS looptopologicalSortImports(already standalone) as-ismergeImportResults(ordered, yamlContent) (*ImportsResult, error)— the accumulator mergeEstimated Effort: Medium | Existing
pkg/parser/import_*_test.gofiles provide safety netSuccess Metrics
This Run
Score Reasoning:
pkg/workflow/andpkg/parser/deeply;pkg/cli/was sampled. Did not reachpkg/agentdrain/deeply orpkg/types/. -1 for coverage.Historical Context
Strategy Performance
Cumulative Statistics
Recommendations
Immediate Actions
generateMCPSetup— 841 lines is a critical maintainability risk; every new MCP tool increases it further. File-level tests already exist for workflow YAML output.processImportsFromFrontmatterWithManifestAndSource— the BFS loop and merge logic are independently testable once separated.Long-term Improvements
funlenlinter in.golangci.yml) to prevent regressions. A threshold of 150 lines would catch both issues found today without being too restrictive.wrapcheckorerrorlintto golangci configuration to surface future%v-instead-of-%wregressions automatically.Next Run Preview
Suggested Focus Areas
ClaudeEngine,GeminiEngine,CrushEngine, etc.) fully implement theCodingAgentEngineinterface with no gapspkg/workflow/with zero test coveragecompile_watch.go,mcp_server_cache.go, anddocker_images.goare candidatesStrategy Evolution
Next run should reuse the function-complexity strategy (targeting the remaining large functions:
buildConsolidatedSafeOutputsJobat 500 lines,ScatterScheduleat 443 lines) while adding a new "interface audit" component for the 50% new-exploration slot.References:
Beta Was this translation helpful? Give feedback.
All reactions