Skip to content

Refactor duplicated owner/repo/number parsing in REST backend caller#6751

Merged
lpcox merged 3 commits into
mainfrom
copilot/duplicate-code-numeric-extraction
May 30, 2026
Merged

Refactor duplicated owner/repo/number parsing in REST backend caller#6751
lpcox merged 3 commits into
mainfrom
copilot/duplicate-code-numeric-extraction

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 30, 2026

restBackendCaller.CallTool had duplicated logic for extracting owner, repo, and numeric resource IDs from JSON args in the pull_request_read and issue_read branches. Both paths implemented the same string/float64 fallback behavior, which made future changes easy to miss and encouraged more copy/paste as REST tools expand.

  • Shared argument parsing

    • Extracted a local helper in internal/proxy/proxy.go to parse:
      • owner
      • repo
      • numeric identifier fields passed as either string or JSON number
    • Preserved the existing error shape and missing-field validation per tool.
  • Call site cleanup

    • Updated pull_request_read to delegate pullNumber parsing to the helper.
    • Updated issue_read to delegate issue_number parsing to the same helper.
    • Kept API path construction unchanged after extraction.
  • Focused coverage

    • Added direct unit coverage for the shared helper to exercise:
      • string ID input
      • float64 JSON number input
      • missing/empty argument handling
owner, repo, number, err := extractOwnerRepoNumber(
    argsMap,
    "owner",
    "repo",
    "pullNumber",
    "pull_request_read",
)
if err != nil {
    return nil, err
}
apiPath = fmt.Sprintf("/repos/%s/%s/pulls/%s", owner, repo, number)

Copilot AI changed the title [WIP] Refactor duplicate numeric argument extraction in restBackendCaller Refactor duplicated owner/repo/number parsing in REST backend caller May 30, 2026
Copilot finished work on behalf of lpcox May 30, 2026 13:49
Copilot AI requested a review from lpcox May 30, 2026 13:49
@lpcox lpcox marked this pull request as ready for review May 30, 2026 14:00
Copilot AI review requested due to automatic review settings May 30, 2026 14:00
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 duplicated REST backend argument parsing for GitHub issue and pull request reads into a shared helper, reducing copy/paste while preserving existing API path behavior and error shapes.

Changes:

  • Added extractOwnerRepoNumber to parse owner, repo, and string/JSON-number resource IDs.
  • Updated pull_request_read and issue_read branches to use the helper.
  • Added focused unit coverage for helper behavior and error formatting.
Show a summary per file
File Description
internal/proxy/proxy.go Introduces shared owner/repo/number parsing and applies it to PR and issue read tool calls.
internal/proxy/rest_backend_caller_tool_test.go Adds table-driven tests for string IDs, float64 IDs, and missing argument errors.

Copilot's findings

Tip

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

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

@lpcox lpcox merged commit 17a64c6 into main May 30, 2026
29 checks passed
@lpcox lpcox deleted the copilot/duplicate-code-numeric-extraction branch May 30, 2026 14:03
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.

[duplicate-code] Duplicate Code Pattern: Numeric Argument Extraction in restBackendCaller

3 participants