[refactor] stringutil/stats: divergent job-name normalizers and grab-bag stringutil.go

[github-actions]

🔧 Semantic Function Clustering Analysis

Scope: pkg/stats, pkg/stringutil (8 non-test Go files, precomputed slice for this run)

Executive Summary

Both packages are well organized. pkg/stringutil follows one-file-per-feature (ansi.go, urls.go, identifiers.go, sanitize.go, fuzzy_match.go, pat_validation.go), each with a paired _test.go, a README.md, and a spec_test.go pinning documented behavior. pkg/stats/statvar.go is a single cohesive type with no issues found.

Two actionable findings, both verified against real call sites:

#	Finding	Severity	Evidence
1	Three divergent reimplementations of job-name normalization; canonical helper NormalizeSafeOutputIdentifier is bypassed or round-tripped	Medium	3 call sites, 2 packages
2	stringutil.go is a grab-bag file in an otherwise feature-organized package	Low	6 unrelated functions

Two suspected duplicates were investigated and ruled out (details below) — reporting these so they are not re-flagged next run.

Finding 1: Divergent job-name normalizers (Medium)

stringutil.NormalizeSafeOutputIdentifier (pkg/stringutil/identifiers.go:62) is the package's canonical separator normalizer: it maps - and . to _. Three downstream functions each implement "normalize a job name" independently, and they do not agree on the canonical form:

Function	Location	Lowercase	Trim	Canonical form
sanitizeJobName	pkg/workflow/strings.go:237	no	no	hyphen
normalizeMaintenanceJobName	pkg/workflow/repo_config.go:122	yes	yes	hyphen
normalizeJobName	pkg/cli/logs_report.go:581	yes	yes	underscore

Two produce hyphen-separated output and one produces underscore-separated output, from the same conceptual input (a workflow/job name). Any code path that compares a value normalized by logs_report.normalizeJobName against one normalized by repo_config.normalizeMaintenanceJobName will mismatch on every name containing a separator.

Additionally, sanitizeJobName round-trips through the canonical helper and immediately undoes it:

// pkg/workflow/strings.go:237
func sanitizeJobName(workflowName string) string {
	normalized := stringutil.NormalizeSafeOutputIdentifier(workflowName) // "-" and "." -> "_"
	return strings.ReplaceAll(normalized, "_", "-")                      // "_" -> "-"
}

The net effect is simply ". and _ both become -". The detour through underscore exists only to borrow the .→_ step, which obscures the actual contract.

Recommendation: add a hyphen-form sibling to pkg/stringutil/identifiers.go (e.g. NormalizeIdentifierToHyphens) alongside the existing underscore-form function, with an explicit lowercase/trim policy documented for both. Then collapse the three call sites onto the two canonical helpers. Because spec_test.go pins the documented behavior of the existing function, the new sibling should get a matching spec test rather than changing NormalizeSafeOutputIdentifier.

Estimated effort: 2–3 hours. Benefit: removes a silent cross-package mismatch class.

Scope note: this finding is anchored on a symbol inside the analyzed slice and was found by checking that symbol's references, not by a repo-wide rescan.

Finding 2: stringutil.go is a grab-bag file (Low)

Every other file in the package is named for its feature. stringutil.go holds six functions with no shared theme beyond "operates on a string":

Function	Line	Nature
Truncate	20	length clamping
NormalizeWhitespace	34	whitespace
ParseVersionValue	54	any → string type coercion
FormatList	77	natural-language formatting
NormalizeLeadingWhitespace	97	whitespace
IsPositiveInteger	150	numeric predicate

Two are genuine outliers for a string utility package:

• ParseVersionValue(version any) string — takes any, not a string; it is a type switch over int/int64/uint64/float64. Both callers (pkg/parser/mcp.go:370,398 and pkg/workflow/engine.go:281,339) use it for one narrow purpose: coercing an MCP/engine version field parsed from YAML/JSON. This is version handling, not string manipulation.
• IsPositiveInteger(s string) bool — a numeric validity predicate with a single caller (pkg/workflow/safe_outputs_validation.go:199).

NormalizeWhitespace and NormalizeLeadingWhitespace are a coherent pair and would read well as whitespace.go.

Recommendation: split into whitespace.go (the two normalizers) and version.go (ParseVersionValue), leaving Truncate, FormatList, and IsPositiveInteger — or fold ParseVersionValue into pkg/parser if no third caller emerges. This is pure file movement within a package: no import changes for callers, and existing tests move with the functions.

Estimated effort: 1 hour. Benefit: restores the one-file-per-feature invariant the rest of the package already follows.

Investigated and ruled out — do not re-flag

SanitizeForFilename vs SanitizeName — NOT a duplicate.

SanitizeForFilename (sanitize.go:330) hand-rolls a character loop that keeps [a-zA-Z0-9-_.], which looks replaceable by SanitizeName(slug, &SanitizeOptions{PreserveSpecialChars: []rune{'.', '_'}, DefaultValue: "clone-mode"}) — the sanitizePatterns map even pre-compiles the matching a-z0-9-._ class. It is not replaceable: SanitizeName lowercases its input, and case preservation is contractual for SanitizeForFilename, asserted at pkg/stringutil/spec_test.go:606 ("should preserve letter case"). SanitizeName also collapses consecutive hyphens, which SanitizeForFilename does not. Consolidating would be a silent behavior break across six call sites in pkg/cli and pkg/workflow.

MarkdownToLockFile / LockFileToMarkdown — acceptable.

These two (identifiers.go:80, identifiers.go:104) are structural mirrors, but they are an inverse-converter pair, which is idiomatic Go. Collapsing them into one direction-parameterized function would hurt readability at every call site.

Trailing-whitespace trimming — weak signal, not filed.

strings.TrimRight(line, " \t") appears in ~6 files across pkg/workflow and pkg/cli while stringutil.NormalizeWhitespace exists. These were not counted as duplication: NormalizeWhitespace operates on a whole document (split, trim each line, enforce one trailing newline), whereas the scattered occurrences are single-line trims inside unrelated parsers and line writers. It is not a drop-in replacement, and forcing it would be a worse abstraction than the two-token call it replaces.

pkg/stats/statvar.go — clean.

Single type (StatVar) with one accumulator and seven accessors (Add, Count, Min, Max, Mean, SampleVariance, SampleStdDev, Median). Cohesive, correctly documented (Welford's algorithm, NaN caveat, memory trade-off), no outliers or duplicates found.

Function inventory and clustering

pkg/stats (1 file) — statvar.go: StatVar + Add/Count/Min/Max/Mean/SampleVariance/SampleStdDev/Median.

pkg/stringutil (7 files):

• ansi.go — StripANSI, skipEscapeSequence, skipCSISequence, skipOSCSequence, isFinalCSIChar, isCSIParameterChar
• fuzzy_match.go — FindClosestMatches, LevenshteinDistance
• identifiers.go — NormalizeWorkflowName, NormalizeSafeOutputIdentifier, MarkdownToLockFile, LockFileToMarkdown
• pat_validation.go — PATType + String/IsFineGrained/IsValid, ClassifyPAT, ValidateCopilotPAT, GetPATTypeDescription
• sanitize.go — SanitizeName, logSanitizeInput, normalizeSanitizeSeparators, buildSanitizePreservePattern, applySanitizePattern, SanitizeErrorMessage, SanitizeIdentifierName, SanitizeParameterName, SanitizePythonVariableName, SanitizeToolID, SanitizeForFilename, isASCIIAlphanumeric
• stringutil.go — see Finding 2
• urls.go — NormalizeGitHubHostURL, ExtractDomainFromURL, extractDomainFallback

Clusters identified:

Cluster	Files	Verdict
Sanitize*	sanitize.go	✅ Correctly colocated; SanitizeParameterName/SanitizePythonVariableName properly delegate to a shared SanitizeIdentifierName
skip*Sequence / is*Char	ansi.go	✅ Private helpers scoped to their one public entry point
Normalize*	stringutil.go, identifiers.go, urls.go, sanitize.go	⚠️ Split across 4 files — the name prefix is coincidental (whitespace vs. identifiers vs. URLs), so this is acceptable, but see Finding 1 for the identifier subset
PATType methods	pat_validation.go	✅ Clean value-type cluster
Accessor cluster	stats/statvar.go	✅ Clean

Next Actions

• Finding 1 — add a hyphen-form normalizer to identifiers.go with a spec test; migrate the three job-name call sites; verify no behavior change in logs_report matching
• Finding 2 — split stringutil.go into whitespace.go + version.go; move corresponding tests
• Confirm the ruled-out items above stay ruled out (no action needed)

Analysis metadata

• Packages analyzed: 2 (pkg/stats, pkg/stringutil)
• Non-test Go files analyzed: 8
• Functions cataloged: 41
• Clusters identified: 5
• Outliers found: 2 (ParseVersionValue, IsPositiveInteger)
• Duplicates confirmed: 1 cluster (3 divergent job-name normalizers)
• Duplicate candidates investigated and rejected: 3
• Method: Serena-configured Go workspace + symbol inventory, naming-pattern clustering, reference verification of every reported call site
• Analysis date: 2026-08-22
• Run: §32546776938

Generated by 🔧 Semantic Function Refactoring · sonnet46 · 223.5 AIC · ⌖ 72.6 AIC · ⊞ 10.6K · ◷

• expires on Aug 23, 2026, 6:46 PM UTC-08:00

[github-actions]

🍪 Issue Monster selected this for Copilot

I've identified this issue as a good candidate for automated resolution and requested assignment to the Copilot coding agent.

Why this one:

• focused refactor in pkg/stringutil / related normalizers
• separate from shell security work
• separate from safe-output config deduplication

If assignment succeeds, the Copilot coding agent will analyze the issue and create a pull request with the fix.

Om nom nom! 🍪

🍪 Om nom nom by Issue Monster · gpt54 · 3.62 AIC · ⌖ 10.8 AIC · ⊞ 11.6K · ◷