Skip to content

Refactor env int parsing and clarify logger sink intent#6019

Merged
lpcox merged 4 commits into
mainfrom
copilot/refactor-semantic-function-clustering
May 19, 2026
Merged

Refactor env int parsing and clarify logger sink intent#6019
lpcox merged 4 commits into
mainfrom
copilot/refactor-semantic-function-clustering

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 19, 2026

The semantic clustering review found three refactor opportunities: ambiguous logger sink naming, duplicated int env parsing logic, and a generic JSON extraction helper hidden in DIFC logging code. This PR takes the low-risk path: consolidate parsing behavior and improve discoverability/intent via docs and comments without API churn.

  • Env parsing consolidation

    • Added envutil.GetEnvIntRaw(envKey) (int, bool, error) to centralize raw integer env parsing (present/not-present/parse-error semantics).
    • Refactored config.parseAndValidateIntEnv to delegate parsing to GetEnvIntRaw, keeping domain validation in config.
    • Updated GetEnvInt to reuse GetEnvIntRaw internally, removing duplicate parse logic.
  • Logger sink clarity (no rename)

    • Clarified godoc on LogInfo/LogWarn/LogError/LogDebug that unsuffixed functions target the unified file sink (mcp-gateway.log).
    • Added matching guidance in AGENTS.md to direct destination-specific usage toward Log*ToMarkdown / Log*ToServer.
  • DIFC helper discoverability

    • Added a targeted note on getFilteredItemStringField in internal/server/difc_log.go indicating it is intentionally generic and a candidate for future extraction if reuse grows.
  • Focused coverage update

    • Added tests for the new GetEnvIntRaw behavior in internal/envutil/envutil_test.go.
// config now delegates parse to envutil, then applies domain validation.
value, present, err := envutil.GetEnvIntRaw(envKey)
if !present {
	return 0, false, nil
}
if err != nil {
	return 0, false, fmt.Errorf("invalid %s value", envKey)
}
if validationErr := validate(value); validationErr != nil {
	return 0, false, fmt.Errorf("%s", validationErr.Message)
}

Copilot AI linked an issue May 19, 2026 that may be closed by this pull request
4 tasks
Copilot AI changed the title [WIP] Refactor semantic function clustering analysis Refactor env int parsing and clarify logger sink intent May 19, 2026
Copilot finished work on behalf of lpcox May 19, 2026 15:35
Copilot AI requested a review from lpcox May 19, 2026 15:35
@lpcox lpcox marked this pull request as ready for review May 19, 2026 21:25
Copilot AI review requested due to automatic review settings May 19, 2026 21:25
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors integer environment-variable parsing to centralize raw parsing semantics in envutil, and clarifies the intent/destination of logger sinks via improved godoc and documentation notes.

Changes:

  • Added envutil.GetEnvIntRaw(envKey) (int, bool, error) and refactored GetEnvInt + config.parseAndValidateIntEnv to reuse it.
  • Clarified logger godoc/agent guidance to distinguish unsuffixed Log* (unified file sink) from destination-specific Log*ToMarkdown / Log*ToServer.
  • Added tests covering GetEnvIntRaw behavior and a discoverability note for a generic DIFC JSON extraction helper.
Show a summary per file
File Description
internal/server/difc_log.go Adds a note clarifying the generic nature of getFilteredItemStringField for future extraction.
internal/logger/file_logger.go Updates exported Log* godoc to clarify it targets the unified file sink.
internal/envutil/envutil.go Introduces GetEnvIntRaw and refactors GetEnvInt to delegate parsing.
internal/envutil/envutil_test.go Adds unit tests for the new GetEnvIntRaw helper.
internal/config/config_env.go Refactors int env parsing/validation to delegate parsing to envutil.GetEnvIntRaw.
AGENTS.md Adds usage guidance distinguishing unified vs destination-specific logging helpers.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 6/6 changed files
  • Comments generated: 2

Comment thread internal/logger/file_logger.go Outdated
Comment thread internal/envutil/envutil_test.go Outdated
lpcox and others added 2 commits May 19, 2026 14:28
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@lpcox lpcox merged commit 0f16742 into main May 19, 2026
16 checks passed
@lpcox lpcox deleted the copilot/refactor-semantic-function-clustering branch May 19, 2026 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[refactor] Semantic Function Clustering Analysis

3 participants