[repository-quality] Repository Quality Improvement Report - Debug-logging hygiene

[github-actions[bot]]

🎯 Repository Quality Improvement Report - Debug-logging hygiene

Analysis Date: 2026-11-11
Focus Area: Debug-logging hygiene (pkg/logger usage)
Strategy Type: Custom (diversity roll 14, inside the 0–59 custom band)
Custom Area: Yes. Repository guidelines require pkg/logger for debug output and promise zero-overhead disabled logging. Existing linters cover stdlib log and console-formatted stderr, but nothing checks logger namespace conventions or call-site cost.

Executive Summary

The core logging discipline is sound. Production code has no stdlib log usage, every package-level logger is referenced, and the consolestderr and rawloginlib linters enforce the main rules. Three gaps remain.

First, the logger documents "zero overhead (not even string formatting)" when disabled, but Go evaluates call arguments before the enabled check. About 21 production call sites build strings.Join, json.Marshal, or yaml.Marshal arguments on every call, even when DEBUG is unset. The cost is usually small, so this is a doc-accuracy and hot-path issue rather than a correctness bug.

Second, verbose-gated fmt.Fprintf(os.Stderr, ...) writes sit beside logger calls in several functions. In pkg/workflow/claude_logs.go, the only confirmed exact duplicate is line 142 (logger) and line 144 (stderr). Other verbose writes there have no matching logger call. Some writes are user-facing --verbose output and must stay; others may be developer diagnostics.

Third, two production namespaces use hyphens, breaking the snake_case convention used elsewhere, and two debug messages carry a redundant literal DEBUG: prefix. Task 3 is small and independent. Task 4 depends on Task 3. Task 1 needs a policy decision first.

Full Analysis Report

Focus Area: Debug-logging hygiene

Current State Assessment

Metrics Collected:

Metric	Value	Status
Production files importing stdlib log	0	✅
Package-level loggers with no references (name-based scan)	0	✅
Non-test files calling logger.New	838	✅
Production namespaces containing hyphens	2 (workflow:mcp-config-custom, workflow:mcp-rendering)	⚠️
Logger messages with a literal DEBUG: prefix	2 (pkg/workflow/time_delta.go)	⚠️
Logger calls with eagerly built Join/Marshal arguments	21	⚠️
Verbose-gated direct os.Stderr writes (adjacent if verbose { heuristic)	~463	⚠️
Direct os.Stderr writes outside console/logger	2,587 across 289 files	i️ Mostly intentional, governed by consolestderr

Findings

Strengths

• No stdlib log usage in production code. The rawloginlib and logfatallibrary linters guard this.
• No package-level logger is left unreferenced. Some earlier per-file counts looked unused, but package-wide checks confirmed they are used (passed as arguments or struct fields).
• Namespaces in pkg/parser and most of pkg/workflow follow a clear package:component pattern.
• The consolestderr linter enforces console formatting for user-facing stderr.

Areas for Improvement

• ⚠️ Eager argument evaluation contradicts the documented zero-overhead claim. pkg/workflow/git_helpers.go:63 runs strings.Join(args, " ") before every git invocation, even when DEBUG is unset. Other sites are in pkg/cli/logs_download_artifacts.go (lines 109, 153, 241) and pkg/workflow/github_token.go:101.
• ⚠️ Verbose stderr partly duplicates logger output. In pkg/workflow/claude_logs.go, claudeLogsLog.Print("JSON array parse failed, trying JSONL format") at line 142 is followed by a --verbose fmt.Fprintf(os.Stderr, ...) of the same event at line 144. Verbose writes at lines 150, 155, 231, 278, and 321 have no exact logger counterpart. Classification is needed before any removal.
• ⚠️ Non-snake_case namespaces. pkg/workflow/mcp_config_custom.go:17 uses workflow:mcp-config-custom, and pkg/workflow/mcp_renderer_factory.go:36 uses workflow:mcp-rendering. A repository-wide grep found no references to these names outside the source files.
• ⚠️ Redundant message prefixes. pkg/workflow/time_delta.go:383 and :423 prefix messages with DEBUG:, although the namespace label already identifies the source.

Detailed Analysis

Logger semantics. In pkg/logger/logger.go, Printf and Print check l.enabled before formatting, so formatting is skipped when disabled. The caller's argument expressions are still evaluated first. The package docs (pkg/logger/doc.go) say "zero overhead (not even string formatting)", which is accurate only for simple arguments. The fix is either to guard expensive sites with log.Enabled() (94 guards already exist in non-test code) or to reword the claim.

Verbose output policy. --verbose is a user-facing flag, so some os.Stderr writes under if verbose { are intended output and should not move to DEBUG. The problem is only where the same information is also logged through logger, which means developers see it twice and the two channels can drift apart.

Namespace convention. Non-test namespaces are overwhelmingly lowercase snake_case with colon separators. The two hyphenated names are the only production exceptions. Test-only namespaces such as spec:slog-* and cli:drain3_* are out of scope.

---

🤖 Tasks for Copilot Agent

NOTE TO PLANNER AGENT: Split the following tasks into individual work items. Task 4 depends on Task 3.

Improvement Tasks

Task 1: Classify and deduplicate verbose stderr writes in claude_logs.go

Priority: Medium
Estimated Effort: Small
Focus Area: Debug-logging hygiene

Description: In pkg/workflow/claude_logs.go, classify the --verbose fmt.Fprintf(os.Stderr, ...) calls at lines 144, 150, 155, 231, 278, and 321. Only line 144 is confirmed to repeat a claudeLogsLog call (line 142). Classify each as user-facing --verbose output (keep) or developer diagnostic (remove only if it duplicates a logger call). Do not change user-visible --verbose behaviour without noting it in the PR.

Acceptance Criteria:

• Each os.Stderr write in claude_logs.go is classified as user-facing or developer-only, with the classification listed in the PR description
• Developer-only duplicates are removed
• User-facing verbose output is unchanged, or any change is called out in the PR
• make test-unit passes, including the Claude log parser tests

Code Region: pkg/workflow/claude_logs.go

In pkg/workflow/claude_logs.go, audit the verbose-only fmt.Fprintf(os.Stderr, ...) calls at about lines 144, 150, 155, 231, 278, and 321. Line 144 repeats the claudeLogsLog call at line 142, so consider removing it. For the others, decide whether each is user-facing --verbose output or a developer diagnostic. Remove only developer diagnostics that duplicate a logger call. Keep user-facing verbose output unchanged, and say in the PR description which lines you kept and why. Run make test-unit and make fmt before finishing.

---

Task 2: Make logger argument cost match the documented zero-overhead claim

Priority: Low
Estimated Effort: Small
Focus Area: Debug-logging hygiene

Description: About 21 logger calls build strings.Join, json.Marshal, or yaml.Marshal arguments on every invocation. Either guard the expensive ones with if log.Enabled(), or reword the zero-overhead claim in pkg/logger/doc.go and pkg/logger/README.md to say that caller-side argument evaluation still happens.

Acceptance Criteria:

• Each Join/Marshal argument is either guarded with Enabled() or documented as caller-evaluated
• pkg/logger/doc.go and pkg/logger/README.md describe disabled-logging cost accurately
• DEBUG=cli:* and DEBUG=workflow:* output is unchanged
• make test-unit passes

Code Region: pkg/workflow/git_helpers.go:63, pkg/cli/logs_download_artifacts.go:109,153,241, pkg/cli/logs_artifact_set.go:211,219, pkg/workflow/github_token.go:101, pkg/logger/doc.go, pkg/logger/README.md

Review the logger call sites that pass strings.Join, json.Marshal, or yaml.Marshal results as arguments (start with pkg/workflow/git_helpers.go:63 and pkg/cli/logs_download_artifacts.go). Guard expensive ones with if log.Enabled() so they do no work when DEBUG is unset. Update the zero-overhead wording in pkg/logger/doc.go and pkg/logger/README.md so it matches the actual behaviour. Keep debug output identical when enabled. Run make fmt and make test-unit.

---

Task 3: Normalise hyphenated logger namespaces and redundant DEBUG prefixes

Priority: Low
Estimated Effort: Small
Focus Area: Debug-logging hygiene

Description: Rename workflow:mcp-config-custom to workflow:mcp_config_custom and workflow:mcp-rendering to workflow:mcp_rendering. Remove the literal DEBUG: prefix from the two messages in pkg/workflow/time_delta.go at lines 383 and 423. A repository-wide search found no references to the old namespaces outside the source files.

Acceptance Criteria:

• No non-test logger.New namespace contains a hyphen
• DEBUG=workflow:mcp_* enables the renamed loggers
• The two time_delta.go messages no longer include the DEBUG: prefix
• make test-unit passes

Code Region: pkg/workflow/mcp_config_custom.go:17, pkg/workflow/mcp_renderer_factory.go:36, pkg/workflow/time_delta.go:383,423

Rename the logger namespaces in pkg/workflow/mcp_config_custom.go (line 17) and pkg/workflow/mcp_renderer_factory.go (line 36) from workflow:mcp-config-custom and workflow:mcp-rendering to workflow:mcp_config_custom and workflow:mcp_rendering. In pkg/workflow/time_delta.go, remove the literal "DEBUG: " prefix from the messages at about lines 383 and 423. Confirm with grep that no other file references the old namespaces. Run make fmt and make test-unit.

---

Task 4: Add a guardrail for logger namespace convention

Priority: Low
Estimated Effort: Medium
Focus Area: Debug-logging hygiene

Description: Add a test that scans non-test Go files under pkg/ and cmd/ for logger.New("...") string literals and fails on any namespace that does not match ^[a-z][a-z0-9_]*(:[a-z0-9_]+)+$. Exclude _test.go files and testdata. Document the convention in pkg/logger/README.md. This depends on Task 3 so the test passes on first run.

Acceptance Criteria:

• A test is added that fails on a hyphenated production namespace (confirmed once by temporarily introducing one)
• The test passes on the repository after Task 3
• pkg/logger/README.md documents the namespace convention
• make test-unit passes

Code Region: pkg/logger/ (new test file), pkg/logger/README.md

Add a Go test that walks pkg/ and cmd/, skips _test.go files and testdata directories, extracts string literals passed to logger.New(...), and fails if any namespace does not match ^[a-z][a-z0-9_]*(:[a-z0-9_]+)+$. Check that the test fails when a hyphenated namespace is introduced, then restore the code. Document the convention in pkg/logger/README.md. Run make fmt and make test-unit. Complete Task 3 first.

---

📊 Historical Context

Previous Focus Areas

Date	Focus Area	Type	Custom	Key Outcomes
2026-11-09	Panic & error-handling hygiene	Custom	Y	panic/Errorf audit, 4 tasks
2026-11-10	Test isolation & process-global state hygiene	Custom	Y	os.Chdir/os.Setenv in tests, sleeps, skip debt, 5 tasks
2026-11-11	Debug-logging hygiene (pkg/logger usage)	Custom	Y	Eager log args, verbose stderr duplication, namespace convention, 4 tasks

---

🎯 Recommendations

Immediate Actions (This Week)

1. Start Task 3 (namespace rename and DEBUG prefix cleanup). It is small, mechanical, and low risk. Priority: Low
2. Start Task 1 (claude_logs.go verbose duplicates). Priority: Medium

Short-term Actions (This Month)

1. Complete Task 2 (argument cost and doc accuracy). Priority: Low
2. Add Task 4 (namespace guardrail) after Task 3 lands. Priority: Low

Long-term Actions (This Quarter)

1. Decide a written policy for --verbose stderr vs DEBUG logging, then apply it across the roughly 463 verbose-gated writes in pkg/cli and pkg/workflow. Priority: Low

---

📈 Success Metrics

• Hyphenated production namespaces: 2 → 0
• Literal DEBUG: prefixes in logger messages: 2 → 0
• Logger calls with eager Join/Marshal args: 21 → 0 unguarded, or documented
• Verbose stderr writes duplicating a logger call (claude_logs.go): 1 confirmed → 0

---

Next Steps

1. Review and prioritise the tasks above
2. Assign tasks to Copilot coding agent via planner agent
3. Track progress on improvement items
4. Re-evaluate this focus area in 3 months

---

Generated by Repository Quality Improvement Agent
Next analysis: 2026-11-12 — Focus area selected by diversity algorithm

Generated by ⚡ Repository Quality Improvement Agent · copilot · routed: haiku55 xhigh · ~8 AIC · ⌖ ~2 AIC · ⊞ 5.7K · ◷

• expires on Nov 12, 2026, 5:04 AM UTC-08:00

[github-actions[bot]]

This discussion was automatically closed because it expired on 2026-11-12T13:04:12.116Z.

Closed by Workflow