Refactor (packages/core/src/tool/websearch.ts): Function with many parameters - #32
Open
AlNaimi7 wants to merge 2 commits into
Open
Refactor (packages/core/src/tool/websearch.ts): Function with many parameters#32AlNaimi7 wants to merge 2 commits into
AlNaimi7 wants to merge 2 commits into
Conversation
Qlty flagged packages/core/src/tool/websearch.ts:152 as "Function with many parameters (count = 6): callMcp" (http, url, tool, args, value, headers). Replace the positional parameter list with a single McpCallOptions<F> object and update both call sites (Exa and Parallel) accordingly. Behavior is unchanged. Validated with `qlty smells --no-snippets packages/core/src/tool/websearch.ts`, which now reports no smells for the file (was 1 before). Covered by the existing packages/core/test/tool-websearch.test.ts suite (12/12 passing), which exercises callMcp indirectly through both providers, including header/body/URL assembly, byte-limit truncation, and timeout handling.
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
1 participant
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.
1. Issue
Link to the associated GitHub issue:
#29
Full path to the refactored file:
packages/core/src/tool/websearch.ts
What do you think this file does?
It implements opencode's built in 'websearch' tool. It runs a permission check on the query, sends a JSON RPC request to either the Exa or Parallel MCP backend, and returns the search text to the model.
What is the scope of your refactoring within that file?
Only the internal helper
callMcp(line 152) and its two call sites inside the tool'sexecuteclosure. No schemas, exports or behaviors were modified.Which Qlty-reported issue did you address?
Function with many parameters on callMcp, previously count = 6 (http, url, tool, args, value, headers).
2. Refactoring
How did the specific issue you chose impact the codebase's maintainability?
Six positional parameters made call sites fragile, since url and tool are both plain strings and swapping them would compile fine but only fail at run time. Any new caller also has to remember the exact order instead of reading named fields.
What changes did you make to resolve the issue?
Added an interface
McpCallOptions<F>and replaced the six positional parameters with a single options object { http, url, tool, args, value, headers? }. Both call sites (Exa and Parallel) were updated to send named fields.How do your changes improve maintainability? Did you consider alternatives?
Named arguments cannot be accidentally swapped, and order stops mattering if an optional field is added later. I considered splitting it into callExa and callParallel, but that would duplicate the shared request building, timeout and response parsing.
3. Validation
How did you validate that the change is correct?
packages/core/test/tool-websearch.test.tspasses 12/12 and exercises callMcp through both providers, asserting the request URL, headers, JSON RPC body, response parsing and failure paths, so a swapped argument would fail the suite. Coverage is 88.00% functions / 95.75% lines with only lines 73-81 (defaultConfigLayer) uncovered, andbunx oxlinton the file reports 0 warnings and 0 errors while the 698 warnings / 2 errors from repo widebun lintare pre existing elsewhere.Attach a screenshot of the test coverage showing the lines were executed by the tests.
Attach a screenshot showing the tests that cover the change passing during CI
Attach a screenshot of
qlty smells --no-snippets packages/core/src/tool/websearch.tsshowing fewer reported issues after the changes.Local bun lint and bun test: