Skip to content

refactor: consolidate map utilities, deep clone, and logger constant#7156

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

refactor: consolidate map utilities, deep clone, and logger constant#7156
lpcox merged 4 commits into
mainfrom
copilot/refactor-semantic-function-clustering

Conversation

Copilot AI commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Four concrete code quality issues found via semantic function clustering: a misplaced utility, a near-duplicate private helper, a single-function package, and a dead constant alias.

Finding 1+2 — GetStringFromMap relocated and generalized; getFilteredItemStringField removed

GetStringFromMap was living in strutil/truncate.go with no relation to truncation. Moved to new strutil/maputil.go and upgraded to variadic keys with first-non-empty semantics — which exactly matches what server/difc_log.go's private getFilteredItemStringField was doing:

// Before: private copy in difc_log.go with a TODO comment to consolidate
entry.AuthorAssociation = getFilteredItemStringField(m, "author_association", "authorAssociation")

// After: shared utility handles camelCase/snake_case fallback directly
entry.AuthorAssociation = strutil.GetStringFromMap(m, "author_association", "authorAssociation")

Single-key callers in proxy/ are unaffected (identical behaviour for the non-empty case).

Finding 3 — internal/jsonutil/ package deleted

jsonutil.DeepCloneJSON was the only function in the package and had exactly one production caller (proxy/response_transform.go). Moved to internal/strutil/deepclone.go alongside the other general-purpose utilities. Import updated in response_transform.go and the three proxy test files.

Finding 4 — Dead logTimestampLayout alias removed

// Before: confusing two-constant indirection
logTimestampLayout  = "2006-01-02T15:04:05.000Z07:00"  // never referenced directly
jsonTimestampLayout = logTimestampLayout

// After: single constant
jsonTimestampLayout = "2006-01-02T15:04:05.000Z07:00"

…eanup

- Move GetStringFromMap from strutil/truncate.go to strutil/maputil.go
  and upgrade to variadic keys with first-non-empty semantics
- Remove getFilteredItemStringField from server/difc_log.go; replace its
  3 call sites with strutil.GetStringFromMap
- Move DeepCloneJSON from internal/jsonutil/ to internal/strutil/;
  delete the now-empty jsonutil package
- Collapse logTimestampLayout alias in logger/jsonl_logger.go to a single
  jsonTimestampLayout constant
Copilot AI changed the title [WIP] Refactor semantic function clustering analysis: outliers and near-duplicates refactor: consolidate map utilities, deep clone, and logger constant Jun 7, 2026
Copilot finished work on behalf of lpcox June 7, 2026 17:02
Copilot AI requested a review from lpcox June 7, 2026 17:02
@lpcox lpcox marked this pull request as ready for review June 7, 2026 17:04
Copilot AI review requested due to automatic review settings June 7, 2026 17:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 internal utilities by consolidating JSON deep-cloning and untyped map string extraction into internal/strutil, removing a now-redundant helper in server, deleting the single-function jsonutil package, and simplifying a logger timestamp constant.

Changes:

  • Moved and generalized GetStringFromMap into internal/strutil/maputil.go (variadic keys, first non-empty semantics) and replaced server’s private helper usage.
  • Moved DeepCloneJSON into internal/strutil and updated proxy code/tests to use strutil.DeepCloneJSON; removed the internal/jsonutil package tests.
  • Removed the unused logTimestampLayout alias in internal/logger/jsonl_logger.go.
Show a summary per file
File Description
internal/strutil/util_test.go Expanded GetStringFromMap unit tests to cover new variadic semantics.
internal/strutil/truncate.go Removed misplaced GetStringFromMap from truncation utilities.
internal/strutil/maputil.go Introduced consolidated GetStringFromMap (variadic keys, first non-empty).
internal/strutil/deepclone.go Relocated DeepCloneJSON into strutil.
internal/server/difc_log.go Replaced local string-field helper with strutil.GetStringFromMap.
internal/server/difc_log_helpers_test.go Removed tests for deleted helper; retained other helper tests.
internal/proxy/response_transform.go Switched deep-clone call from jsonutil to strutil.
internal/proxy/response_transform_test.go Updated deep-clone tests to use strutil.DeepCloneJSON.
internal/proxy/response_transform_coverage_test.go Updated coverage tests to use strutil.DeepCloneJSON.
internal/proxy/proxy_test.go Updated deep-clone test to use strutil.DeepCloneJSON.
internal/logger/jsonl_logger.go Simplified timestamp layout constant definition.
internal/jsonutil/clone_test.go Removed tests as internal/jsonutil package is deleted.

Copilot's findings

Tip

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

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

Comment thread internal/strutil/maputil.go Outdated
Comment thread internal/strutil/util_test.go
lpcox and others added 2 commits June 7, 2026 10:13
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 a3fb00e into main Jun 7, 2026
16 checks passed
@lpcox lpcox deleted the copilot/refactor-semantic-function-clustering branch June 7, 2026 17:16
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: outliers and near-duplicates in strutil, logger, and jsonutil

3 participants