Skip to content

refactor: add strutil.InterfaceToIntString to deduplicate JSON numeric extraction#7937

Merged
lpcox merged 4 commits into
mainfrom
copilot/duplicate-code-json-number-extraction
Jun 22, 2026
Merged

refactor: add strutil.InterfaceToIntString to deduplicate JSON numeric extraction#7937
lpcox merged 4 commits into
mainfrom
copilot/duplicate-code-json-number-extraction

Conversation

Copilot AI commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Three packages independently inlined float64/json.Number → integer-string type-switches, with inconsistent safety: difc_log.go silently truncated non-integer floats (123.9"123"), while router.go validated but lacked json.Number support entirely.

Changes

  • internal/strutil/numbers.go — new InterfaceToIntString(v interface{}) (string, bool) helper:
    • Explicit int64 range guard on float64 before conversion (avoids implementation-defined overflow)
    • Uses json.Number.Int64() to reject non-integer json.Number values
    • Returns ("", false) for all non-integer or out-of-range inputs
// float64(math.MaxInt64) rounds up, so >= catches the boundary overflow
if n < float64(math.MinInt64) || n >= float64(math.MaxInt64) {
    return "", false
}
i := int64(n)
if n != float64(i) {
    return "", false // non-integer float
}
  • internal/server/difc_log.go — replaces 6-line switch with strutil.InterfaceToIntString; non-integer floats now return "" (bug fix, consistent with router.go)
  • internal/proxy/router.go — replaces 12-line float64-only block with the helper, gaining json.Number support for free
  • internal/strutil/numbers_test.go — new unit tests covering all numeric/non-numeric input types and edge cases
  • internal/server/difc_log_test.go / internal/proxy/rest_backend_caller_tool_test.go — updated/added tests reflecting new behavior and json.Number coverage

GitHub Advanced Security started work on behalf of lpcox June 22, 2026 17:05 View session
GitHub Advanced Security finished work on behalf of lpcox June 22, 2026 17:06
…N numeric extraction

- Add strutil.InterfaceToIntString (float64 + json.Number → integer string)
  with explicit int64 range guard and json.Number integer validation
- Replace inline type-switch in server/difc_log.go with the helper
- Replace inline float64-only branch in proxy/router.go with the helper,
  adding json.Number support that was previously missing
- Update difc_log_test: non-integer floats now return "" (consistent with router.go)
- Add unit tests for the new helper and json.Number cases in router tests

Closes #7919
GitHub Advanced Security started work on behalf of lpcox June 22, 2026 17:15 View session
GitHub Advanced Security finished work on behalf of lpcox June 22, 2026 17:15
- Improve comment in numbers.go: clarify why < is used for lower bound
  (float64(math.MinInt64) is exactly representable as -(2^63))
- Restore numeric value in router.go debug log message for better diagnostics
GitHub Advanced Security started work on behalf of lpcox June 22, 2026 17:16 View session
Copilot AI changed the title [WIP] Refactor duplicate JSON numeric extraction code pattern refactor: add strutil.InterfaceToIntString to deduplicate JSON numeric extraction Jun 22, 2026
Copilot AI requested a review from lpcox June 22, 2026 17:16
Copilot finished work on behalf of lpcox June 22, 2026 17:16
GitHub Advanced Security finished work on behalf of lpcox June 22, 2026 17:16
@lpcox lpcox marked this pull request as ready for review June 22, 2026 17:20
Copilot AI review requested due to automatic review settings June 22, 2026 17:20

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 introduces a shared strutil.InterfaceToIntString helper to centralize “JSON-decoded number → integer string” extraction, replacing several ad-hoc type switches and aligning behavior across server logging and proxy routing while adding targeted unit tests.

Changes:

  • Added strutil.InterfaceToIntString(v interface{}) (string, bool) to safely accept only in-range integer float64 and integer json.Number inputs.
  • Refactored DIFC filtered-item logging and proxy routing to use the shared helper, eliminating duplicated numeric parsing logic.
  • Added/updated unit tests to cover integer vs non-integer numeric cases and json.Number support.
Show a summary per file
File Description
internal/strutil/numbers.go Adds the shared numeric interface→int-string conversion helper with range/non-integer rejection.
internal/strutil/numbers_test.go Introduces unit coverage for float64 and json.Number integer/non-integer and out-of-range cases.
internal/server/difc_log.go Uses the helper to populate filtered-item number without truncating non-integer floats.
internal/server/difc_log_test.go Updates expectations for decimal float handling (now rejected rather than truncated).
internal/proxy/router.go Refactors route argument numeric extraction to use the helper (and should preserve prior validation/error semantics).
internal/proxy/rest_backend_caller_tool_test.go Adds a json.Number test case to ensure numeric args are accepted beyond float64.

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: 3

Comment thread internal/strutil/numbers.go Outdated
Comment on lines +28 to +33
case json.Number:
// Validate that the json.Number represents a valid integer.
if _, err := n.Int64(); err != nil {
return "", false // non-integer or out-of-range json.Number
}
return n.String(), true
Comment on lines +48 to +51
t.Run("float64 out of int64 range returns false", func(t *testing.T) {
t.Parallel()
// 1e20 exceeds int64 max; round-trip check catches the overflow
s, ok := InterfaceToIntString(float64(1e20))
Comment thread internal/proxy/router.go
Comment on lines 79 to 83
if number == "" {
if n, ok := argsMap[numberKey].(float64); ok {
logRouter.Printf("extractOwnerRepoNumber: %s provided as float64=%v, parsing as integer for tool=%s", numberKey, n, toolName)
const maxInt64AsFloat = float64(int64(^uint64(0) >> 1))
if n < 0 || n > maxInt64AsFloat {
logRouter.Printf("extractOwnerRepoNumber: %s float64=%v out of int64 range [0,%v] for tool=%s", numberKey, n, maxInt64AsFloat, toolName)
err = fmt.Errorf("%s: invalid %s (out of range)", toolName, numberKey)
return
}
i := int64(n)
if n != float64(i) {
logRouter.Printf("extractOwnerRepoNumber: %s float64=%v is not a whole number for tool=%s", numberKey, n, toolName)
err = fmt.Errorf("%s: invalid %s (expected integer)", toolName, numberKey)
return
}
number = fmt.Sprintf("%d", i)
if s, ok := strutil.InterfaceToIntString(argsMap[numberKey]); ok {
logRouter.Printf("extractOwnerRepoNumber: %s provided as numeric=%v, parsing as integer for tool=%s", numberKey, argsMap[numberKey], toolName)
number = s
}
@lpcox

lpcox commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

@copilot address review feedback

GitHub Advanced Security started work on behalf of lpcox June 22, 2026 18:52 View session
GitHub Advanced Security finished work on behalf of lpcox June 22, 2026 18:52

Copilot AI commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

@copilot address review feedback

Addressed all three review items in commit fix: address review feedback on InterfaceToIntString and router validation:

  1. numbers.go: json.Number case now captures the int64 from n.Int64() and returns fmt.Sprintf("%d", i) — canonical form that eliminates non-standard representations like "00123".
  2. numbers_test.go: Updated the out-of-range float64 comment; added a leading-zero canonicalization test.
  3. router.go: Restored specific error messages by type-switching on float64, json.Number before calling InterfaceToIntString. Invalid values (non-integer, out-of-range) now return "invalid %s (out of range or not an integer)", negatives return "invalid %s (must be non-negative)", and truly absent fields continue to return the "missing" error. New test cases cover all four invalid/negative combinations.

Copilot finished work on behalf of lpcox June 22, 2026 18:53
@lpcox lpcox merged commit 7881a8d into main Jun 22, 2026
27 checks passed
@lpcox lpcox deleted the copilot/duplicate-code-json-number-extraction branch June 22, 2026 19:00
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.

3 participants