refactor: address semantic function clustering outliers — doc comments, rename, dead code removal - #9278
Merged
lpcox merged 2 commits intoJul 13, 2026
Conversation
…_env.go, remove dead truncateAndSanitize
lpcox
marked this pull request as ready for review
July 13, 2026 21:59
Copilot
AI
changed the title
[WIP] Refactor semantic function clustering analysis for outliers and patterns
refactor: address semantic function clustering outliers — doc comments, rename, dead code removal
Jul 13, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Refines configuration and logger organization following semantic clustering analysis.
Changes:
- Documents strict configuration environment expansion semantics.
- Renames gateway environment helpers for clarity.
- Removes an unused sanitization wrapper and its tests.
Show a summary per file
| File | Description |
|---|---|
internal/config/expand.go |
Documents strict undefined-variable handling. |
internal/config/gateway_env.go |
Renames gateway-specific environment helpers. |
internal/logger/rpc_format.go |
Removes the dead wrapper and imports. |
internal/logger/rpc_helpers_test.go |
Removes obsolete wrapper tests. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
internal/config/gateway_env.go:3
- The rename leaves the project-structure guide at
AGENTS.md:31pointing to the now-nonexistentconfig_env.go. Update that entry togateway_env.goso contributor documentation remains accurate.
- Files reviewed: 4/4 changed files
- Comments generated: 0
- Review effort level: Medium
This was referenced Jul 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the Priority 1 and Priority 2 recommendations from the semantic function clustering analysis: one explanatory comment gap, one misleading filename, and one dead private wrapper.
Changes
internal/config/expand.go— Added package-level doc comment explaining why this file usesos.LookupEnvdirectly instead ofinternal/envutil. The distinction is semantic: envutil silently returns defaults for undefined vars; config expansion must fail hard on undefined references.internal/config/config_env.go→internal/config/gateway_env.go— Renamed viagit mvto accurately reflect the file's scope (gateway-specific env accessors), distinguishing it fromexpand.gowhich handles${VAR}substitution.internal/logger/rpc_format.go— Removed the deadtruncateAndSanitizeprivate wrapper and its unusedsanitize/utilimports. The function had no production call sites;rpc_logger.goalready callssanitize.SanitizeStringandutil.Truncatedirectly (and more efficiently, reusing the sanitized string across multiple truncation lengths). Removed the correspondingTestTruncateAndSanitizetest.internal/logger/global_state.go— No change; the cross-referencing "Log-Level Quad-Function Pattern" comment documenting the threelogWith*dispatchers already exists.