[sergo] Sergo Report: Config Parser Consistency Analysis - 2026-04-26

[github-actions[bot]]

Date: 2026-04-26
Strategy: config-parser-consistency (new exploration — first run, no cached history)
Success Score: 7/10
Run ID: §24966166774

Executive Summary

Today's analysis focused on the pkg/workflow safe-output config parsing layer, tracing patterns from the recently-landed refactor (replace setStringFromMap with extractStringFromMap, commit 3714aaa). The investigation revealed that a generic parseConfigScaffold scaffold was introduced to standardise config parsing, but six handlers were not migrated to it. A second independent finding identified duplicate log statements in parseUpdateEntityConfig that produce double-verbose output on every config parse.

Both findings are actionable with well-defined fixes and no behaviour changes.

---

Serena Tools Update

Tools Snapshot

• Total Tools Available: 20
• New Tools Since Last Run: N/A (first run)
• Removed Tools: N/A
• Modified Tools: N/A

Tool Capabilities Used Today

Tool	Purpose
activate_project	Initialize Go LSP analysis session
find_symbol	Locate extractStringFromMap definition and MapToolConfig methods
find_referencing_symbols	Trace all callers of extractStringFromMap
get_symbols_overview	Survey mcp_config_types.go, config_helpers.go, validation_helpers.go
search_for_pattern	Find inline map extraction patterns, parseConfigScaffold usages, duplicate logs
list_dir / find_file	Navigate pkg/workflow, pkg/typeutil, pkg/sliceutil

Note: Serena MCP server became unresponsive mid-session (EOF errors). Analysis continued with native Grep/Read tools, which provided equivalent coverage for the remaining patterns.

---

Strategy Selection

Cached Reuse Component (50%)

No previous strategy cache existed (first run). The 50% reuse slot was filled by anchoring on the recent commit (setStringFromMap → extractStringFromMap). Using the committed refactor as a seed to trace related patterns is equivalent to "adapting a proven strategy" — it follows the code's own momentum.

• Seed: Commit 3714aaa — extractStringFromMap consolidation
• Extension: Trace all callers to find remaining inconsistencies in config parsing layer

New Exploration Component (50%)

Novel approach: Systematic survey of all parse*Config methods (48 total found) to classify which parsing idiom each uses:

1. parseConfigScaffold (new, generic) — preferred pattern
2. unmarshalConfig direct call (older, verbose) — migration candidates
3. Fully manual inline extraction (complex handlers, intentionally different)

This classification revealed the inconsistency in the 6 unmigrated handlers.

Combined Strategy Rationale

The seed-from-commit approach ensures findings are directly relevant to ongoing refactoring work. The systematic survey converts a single data point (recent commit) into a comprehensive map of the pattern across all 48 handlers — transforming a one-off fix into actionable scope.

---

Analysis Execution

Codebase Context

• Total Go files in pkg/: 727 (non-test)
• pkg/workflow non-test Go files: ~350+
• parse*Config handlers found: 48 methods
• Focus areas: config_helpers.go, templatables.go, validation_helpers.go, 6 legacy handlers, update_entity_helpers.go

Findings Summary

• Total Issues Found: 2
• Critical: 0
• High: 0
• Medium: 1
• Low: 1

---

Detailed Findings

Medium Priority: 6 config parsers not migrated to parseConfigScaffold

parseConfigScaffold[T any] was introduced in config_helpers.go as the canonical pattern for safe-output config parsing. It encapsulates: key existence check → log → unmarshalConfig → onError callback.

Newer handlers (assign_to_agent.go, assign_to_user.go, add_labels.go, remove_labels.go, assign_milestone.go, etc.) already use it correctly. But six older handlers still use the pre-scaffold unmarshalConfig direct call:

File	Function	Lines
pkg/workflow/create_issue.go	parseIssuesConfig	58–64
pkg/workflow/create_discussion.go	parseDiscussionsConfig	59–65
pkg/workflow/create_pull_request.go	parsePullRequestsConfig	132–138
pkg/workflow/add_comment.go	parseCommentsConfig	57–62
pkg/workflow/add_reviewer.go	parseAddReviewerConfig	50–55
pkg/workflow/close_entity_helpers.go	parseCloseEntityConfig	108–114

The migration path is demonstrated by assign_to_user.go:19–47: preprocess fields first (since preprocessBoolFieldAsString/preprocessIntFieldAsString mutate the sub-map in place), then call parseConfigScaffold.

View migration example (assign_to_user.go pattern)

// ✅ Correct pattern (assign_to_user.go)
func (c *Compiler) parseAssignToUserConfig(outputMap map[string]any) *AssignToUserConfig {
    if _, exists := outputMap["assign-to-user"]; !exists {
        return nil
    }
    configData, _ := outputMap["assign-to-user"].(map[string]any)
    if err := preprocessBoolFieldAsString(configData, "unassign-first", assignToUserLog); err != nil {
        assignToUserLog.Printf("Invalid unassign-first value: %v", err)
        return nil
    }
    config := parseConfigScaffold(outputMap, "assign-to-user", assignToUserLog, func(err error) *AssignToUserConfig {
        assignToUserLog.Printf("Failed to unmarshal config: %v", err)
        return &AssignToUserConfig{...}
    })
    ...
}

// ❌ Old pattern (create_issue.go)
var config CreateIssuesConfig
if err := unmarshalConfig(outputMap, "create-issue", &config, createIssueLog); err != nil {
    createIssueLog.Printf("Failed to unmarshal config: %v", err)
    config = CreateIssuesConfig{}  // backward compat fallback
}

Low Priority: Duplicate log statements in parseUpdateEntityConfig

pkg/workflow/update_entity_helpers.go:126 — the function receives a caller-provided logger parameter but also has a file-level updateEntityHelpersLog. Two lines log identical messages to both:

// Lines 128–129: identical message
updateEntityHelpersLog.Printf("Parsing %s configuration", params.ConfigKey)
logger.Printf("Parsing %s configuration", params.ConfigKey)

// Lines 136–137: near-duplicate
updateEntityHelpersLog.Printf("Invalid target-repo configuration for %s", params.ConfigKey)
logger.Print("Invalid target-repo configuration")

Every invocation of parseUpdateEntityConfig (covering update-issue, update-pull-request, update-discussion, and their variants) produces double-verbose output. Fix: remove the updateEntityHelpersLog calls at those two sites; the file-level logger remains useful on line 141 for detail not echoed by the caller.

View all callers of parseUpdateEntityConfig

Called by: update_issue_helpers.go, update_pull_request_helpers.go, update_discussion_helpers.go, update_entity_helpers.go (via parseUpdateEntityBase), and indirectly via parseUpdateEntityConfigWithFields.

---

Improvement Tasks Generated

Task 1: Migrate 6 legacy config parsers to parseConfigScaffold

Issue Type: Code Pattern Consistency

Problem: Six handlers use unmarshalConfig directly (old pattern) instead of parseConfigScaffold (canonical pattern). This creates inconsistency in a 48-handler codebase where the scaffold is the documented standard.

Locations:

• pkg/workflow/create_issue.go:58–64
• pkg/workflow/create_discussion.go:59–65
• pkg/workflow/create_pull_request.go:132–138
• pkg/workflow/add_comment.go:57–62
• pkg/workflow/add_reviewer.go:50–55
• pkg/workflow/close_entity_helpers.go:108–114

Recommendation: For each handler: extract configData from the map, run preprocessing (unchanged), replace the unmarshalConfig block with parseConfigScaffold. Reference: assign_to_user.go:19–47.

Estimated Effort: Small (each file is a ~5-line mechanical change)

---

Task 2: Remove duplicate log statements in update_entity_helpers.go

Issue Type: Debug Log Quality

Problem: Lines 128–129 and 136–137 of parseUpdateEntityConfig log identical messages to two loggers, doubling output on every update-entity config parse.

Location: pkg/workflow/update_entity_helpers.go:128–129, 136–137

Recommendation: Remove the updateEntityHelpersLog calls at lines 128 and 136; keep the logger calls (caller-scoped, more specific). Update line 137 to include the entity type for consistency with the removed line 136.

Estimated Effort: Trivial (2-line deletion)

---

Success Metrics

This Run

• Findings Generated: 2
• Tasks Created: 2
• Files Analyzed: ~30 in pkg/workflow/, plus DEADCODE.md, go.mod, Makefile
• Success Score: 7/10

Scoring Reasoning

• Findings Quality (5/6): Both findings are specific, actionable, and confirmed by code inspection. -1 because Serena crashed mid-session limiting depth.
• Coverage (3/5): Good coverage of the config-parsing layer; didn't reach pkg/parser, pkg/cli, or other packages. -2.
• Task Generation (3/3): 2 well-scoped tasks generated, matching findings. Full marks.

---

Historical Context

This Run Is the First Run

No previous strategy cache. Cumulative statistics start here.

Metric	Value
Total Runs	1
Total Findings	2
Total Tasks Generated	2
Average Success Score	7.0/10
Most Successful Strategy	config-parser-consistency

---

Recommendations

Immediate Actions

1. (Medium) Migrate create_issue.go, create_discussion.go, create_pull_request.go, add_comment.go, add_reviewer.go, close_entity_helpers.go to use parseConfigScaffold. Mechanical change, no behavior impact.
2. (Low) Remove duplicate log lines 128 and 136 from parseUpdateEntityConfig in update_entity_helpers.go.

Long-term Improvements

• Consider adding a linter rule or go vet check that flags direct unmarshalConfig calls outside of parseConfigScaffold itself — this would prevent regression as new handlers are added.
• The publish_artifacts.go:parseUploadArtifactConfig function uses 70+ lines of manual inline extraction (vs YAML-based unmarshaling). Unlike the 6 handlers above, this handler has structural complexity (bool-typed enable/disable, nested config structs with typeutil.ParseIntValue int fields) that makes it a candidate for a future, more involved refactor.

---

Next Run Preview

Suggested Focus Areas

• pkg/parser — phases 5–8 of dead code removal are marked complete, but their removal may have left inconsistencies in callers
• pkg/cli — three dead git helper functions listed in DEADCODE.md phase 5 (verified removed, but surrounding code worth auditing)
• Expression parsing layer (expression_parser.go, expression_nodes.go) — not touched in this run

Strategy Evolution

Next run should try a type-hierarchy inspection strategy using Serena's find_symbol with include_kinds filtering for interfaces. The ToolConfig interface in mcp_config_types.go has only one implementor (MapToolConfig) and GetAny is listed in DEADCODE.md as "not worth deleting" — but checking interface completeness across the codebase could reveal further consolidation opportunities.

---

References:

• §24966166774

Generated by Sergo - Serena Go Expert · ● 901.5K · ◷

• expires on Apr 27, 2026, 8:34 PM UTC

[github-actions[bot]]

This discussion has been marked as outdated by Sergo - Serena Go Expert.

A newer discussion is available at Discussion #28799.