test(code-rabbit): setup test#2
Conversation
|
Warning Ignoring CodeRabbit configuration file changes. For security, only the configuration from the base branch is applied for open source repositories. Warning
|
| Cohort / File(s) | Summary |
|---|---|
Config & runtime proxy/src/config.ts |
Normalized string literal quoting, updated RunnerConfig.type and Config.fallback.mode literal unions, changed default path/encoding quoting, added console.log('Please summarise this pr and suggest improvements code rabbit'), minor formatting. |
Server error handling & tsconfig proxy/src/server.ts, proxy/tsconfig.json |
Changed error param type to unknown, unified error message extraction for logs/responses; enabled noImplicitAny in proxy/tsconfig.json. |
Adapters & handlers proxy/src/adapters/..., proxy/src/adapters/bedrock-handlers/... |
Mostly quoting/import formatting, removed unused provider type imports, minor regex simplifications, cleaned up unused types in handlers; behavior unchanged. |
Tests / specs (typing) proxy/src/adapters/*.spec.ts, proxy/src/runners/*.spec.ts, proxy/src/runners/runner-orchestrator.spec.ts, proxy/src/adapters/bedrock.spec.ts |
Tightened TypeScript assertions and typings in multiple tests (replaced as any casts, explicit typed fixtures); some tests had input values adjusted from array to string in header mocks. |
Repo config & CI .coderabbit.yaml, .github/workflows/docker-build.yml |
Converted .coderabbit.yaml path_filters to negated-pattern form and enabled finishing_touches.unit_tests; removed pull_request trigger from Docker build workflow (runs on push/tags only) and adjusted YAML quoting. |
| Misc formatting various proxy/src/* files |
Consistent single-quote usage and import compacting across multiple source files; no functional changes indicated beyond quoting/import style. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
I nibble on changes, tidy and spry, 🐇
Quotes straightened like carrots in line. 🥕
Types tightened, logs give a peep,
CI trimmed so it sleeps not deep.
Hopping off—these tweaks make the code shine. ✨
🚥 Pre-merge checks | ✅ 3 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. | |
| Title check | ❓ Inconclusive | The title 'test(code-rabbit): setup test' is vague and generic, using the term 'setup test' which does not clearly describe the actual changeset of code quality improvements, quote normalization, and TypeScript strictness enablement. | Consider a more descriptive title that captures the main changes, such as 'refactor: normalize quotes and enable noImplicitAny TypeScript check' or 'chore: improve code quality with quote normalization and strict type checking'. |
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
feat/test-branch-code-rabbit
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@proxy/src/config.ts`:
- Line 38: Remove the stray console.log debug line in proxy/src/config.ts (the
console.log('Please summarise this pr and suggest improvements code rabbit')
call); either delete it entirely or replace it with a proper logger.debug/trace
invocation guarded by an environment-controlled verbose flag (e.g., use your
existing logger or check process.env.NODE_ENV/DEBUG) so it no longer pollutes
production logs.
| console.log(`Runners: ${config.runners.length}`); | ||
| console.log(`Providers: ${Object.keys(config.mappings).join(", ")}`); | ||
| console.log(`Providers: ${Object.keys(config.mappings).join(', ')}`); | ||
| console.log('Please summarise this pr and suggest improvements code rabbit'); |
There was a problem hiding this comment.
Remove the stray debug log before release.
This message looks like an internal prompt and will pollute production logs.
🤖 Prompt for AI Agents
In `@proxy/src/config.ts` at line 38, Remove the stray console.log debug line in
proxy/src/config.ts (the console.log('Please summarise this pr and suggest
improvements code rabbit') call); either delete it entirely or replace it with a
proper logger.debug/trace invocation guarded by an environment-controlled
verbose flag (e.g., use your existing logger or check
process.env.NODE_ENV/DEBUG) so it no longer pollutes production logs.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.coderabbit.yaml:
- Around line 17-23: The current path_filters entries ('!*.md', '!*.lock',
'!*.json', '!docs/**', '!examples/**') only match root-level files; update these
patterns under path_filters to use recursive globs (e.g., change '!*.md' to
'!**/*.md', '!*.lock' to '!**/*.lock', and '!*.json' to '!**/*.json') so
non-code files are ignored repository-wide while keeping the existing directory
exclusions ('!docs/**', '!examples/**') as appropriate.
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
❌ Failed to create PR with unit tests: AGENT_CHAT: Permission issue |
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
proxy/src/adapters/anthropic.spec.ts (1)
84-100: Test name no longer matches behavior and duplicates the next case.Changing the header value to the string
'key1'makes this test exercise the "string but wrong prefix" path, not the "not a string" path. WithProviderRequest.headers['x-api-key']typed as a string, the only reasoncanHandleRequestreturnsfalsehere is that'key1'failsstartsWith('sk-ant-')— which is exactly what the test on lines 102‑118 already covers with'different-prefix-123'. Theit(...)title ("should return false when x-api-key is not a string") is now misleading.Either drop this test as redundant, or rename it and genuinely exercise the non-string branch (e.g. cast through
unknownso typings aren't violated while still coveringtypeof req.headers["x-api-key"] !== "string").♻️ Option A: remove the now-redundant test
- it('should return false when x-api-key is not a string', () => { - // Arrange - const request: ProviderRequest = { - url: '/test', - method: 'POST', - headers: { - 'x-api-key': 'key1', - }, - body: {}, - }; - - // Act - const result = adapter.canHandleRequest(request); - - // Assert - expect(result).toBe(false); - }); - it('should return false when x-api-key does not start with sk-ant-', () => {♻️ Option B: actually cover the non-string branch
it('should return false when x-api-key is not a string', () => { // Arrange - const request: ProviderRequest = { + const request = { url: '/test', method: 'POST', headers: { - 'x-api-key': 'key1', + 'x-api-key': ['key1', 'key2'], }, body: {}, - }; + } as unknown as ProviderRequest;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@proxy/src/adapters/anthropic.spec.ts` around lines 84 - 100, The test's description doesn’t match its behavior: with headers['x-api-key'] set to 'key1' the spec actually exercises the "wrong prefix" branch of adapter.canHandleRequest rather than the "not a string" branch, duplicating the next test; either delete this redundant test, or update it to truly hit the non-string branch by changing the test to supply a non-string value for ProviderRequest.headers['x-api-key'] (e.g. inject a number/undefined via an unsafe cast through unknown to bypass TypeScript typing) and keep the title "should return false when x-api-key is not a string"; ensure you reference adapter.canHandleRequest and ProviderRequest.headers['x-api-key'] when making the change.proxy/src/adapters/openai.spec.ts (1)
82-98: Test title no longer matches what's being asserted; non-string branch is no longer covered.
'Bearer token'is a string, so this test no longer exercises thetypeof req.headers["authorization"] === "string"guard inopenai.ts— it only hits thestartsWith('Bearer sk-')check. Theit(...)title ("…when authorization is not a string") is now inaccurate, and unlike the Anthropic spec there isn't a separate "wrong prefix" test to back-fill it, so coverage on the non-string branch is effectively dropped.Either rename the test to reflect what it actually checks (wrong prefix), or keep the original intent by casting through
unknownso TS stays happy:♻️ Proposed fix (preserve non-string branch coverage)
it('should return false when authorization is not a string', () => { // Arrange - const request: ProviderRequest = { + const request = { url: '/test', method: 'POST', headers: { - authorization: 'Bearer token', + authorization: ['Bearer', 'token'], }, body: {}, - }; + } as unknown as ProviderRequest;Alternatively, rename to
'should return false when authorization does not start with "Bearer sk-"'and add a separate test for the non-string case.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@proxy/src/adapters/openai.spec.ts` around lines 82 - 98, The test title and assertion are inconsistent: update the spec for adapter.canHandleRequest in openai.spec.ts to either (A) preserve the original non-string branch by setting headers.authorization to a non-string value (e.g., undefined or cast a non-string via "authorization: (null as unknown as any)" or similar so TypeScript allows it) and keep the title "should return false when authorization is not a string", or (B) change the it(...) title to "should return false when authorization does not start with \"Bearer sk-\"" to reflect the current assertion; ensure the test targets adapter.canHandleRequest in openai.ts and add a separate test that explicitly passes a non-string authorization to cover that branch if you choose option B.proxy/src/adapters/bedrock.spec.ts (1)
305-305: Inconsistent cast: still usingas anyhere.Every other
adapter.transformRequest(request as ...)call in this file was tightened toas BedrockRequestin this PR, but this one was missed. For consistency with the PR's type-tightening goal (and withnoImplicitAnyenabled intsconfig.json), update this call too.♻️ Proposed fix
- const result = adapter.transformRequest(request as any); + const result = adapter.transformRequest(request as BedrockRequest);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@proxy/src/adapters/bedrock.spec.ts` at line 305, Replace the loose cast "as any" when calling adapter.transformRequest with the stricter type used elsewhere: cast the request variable to BedrockRequest (i.e., call adapter.transformRequest(request as BedrockRequest)). Update the call site that currently reads adapter.transformRequest(request as any) so it matches the other tightened casts in this spec file and satisfies noImplicitAny.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@proxy/src/adapters/bedrock.spec.ts`:
- Line 1: Add the missing type import used by transformResponse: import
AnthropicMessageResponse from the anthropic types module (i.e. add "import {
AnthropicMessageResponse } from '@/types/providers/anthropic.type';") so that
the transformResponse method's return type resolves; update the top of
anthropic-handler.ts to include this import.
---
Nitpick comments:
In `@proxy/src/adapters/anthropic.spec.ts`:
- Around line 84-100: The test's description doesn’t match its behavior: with
headers['x-api-key'] set to 'key1' the spec actually exercises the "wrong
prefix" branch of adapter.canHandleRequest rather than the "not a string"
branch, duplicating the next test; either delete this redundant test, or update
it to truly hit the non-string branch by changing the test to supply a
non-string value for ProviderRequest.headers['x-api-key'] (e.g. inject a
number/undefined via an unsafe cast through unknown to bypass TypeScript typing)
and keep the title "should return false when x-api-key is not a string"; ensure
you reference adapter.canHandleRequest and ProviderRequest.headers['x-api-key']
when making the change.
In `@proxy/src/adapters/bedrock.spec.ts`:
- Line 305: Replace the loose cast "as any" when calling
adapter.transformRequest with the stricter type used elsewhere: cast the request
variable to BedrockRequest (i.e., call adapter.transformRequest(request as
BedrockRequest)). Update the call site that currently reads
adapter.transformRequest(request as any) so it matches the other tightened casts
in this spec file and satisfies noImplicitAny.
In `@proxy/src/adapters/openai.spec.ts`:
- Around line 82-98: The test title and assertion are inconsistent: update the
spec for adapter.canHandleRequest in openai.spec.ts to either (A) preserve the
original non-string branch by setting headers.authorization to a non-string
value (e.g., undefined or cast a non-string via "authorization: (null as unknown
as any)" or similar so TypeScript allows it) and keep the title "should return
false when authorization is not a string", or (B) change the it(...) title to
"should return false when authorization does not start with \"Bearer sk-\"" to
reflect the current assertion; ensure the test targets adapter.canHandleRequest
in openai.ts and add a separate test that explicitly passes a non-string
authorization to cover that branch if you choose option B.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1aa2ef40-a7d4-4bd1-883c-0df344566ae1
📒 Files selected for processing (13)
.coderabbit.yaml.github/workflows/docker-build.ymlproxy/src/adapters/anthropic.spec.tsproxy/src/adapters/base.tsproxy/src/adapters/bedrock-handlers/anthropic-handler.spec.tsproxy/src/adapters/bedrock-handlers/anthropic-handler.tsproxy/src/adapters/bedrock.spec.tsproxy/src/adapters/bedrock.tsproxy/src/adapters/openai.spec.tsproxy/src/runners/ollama.spec.tsproxy/src/runners/runner-orchestrator.spec.tsproxy/src/server.tsproxy/tsconfig.json
✅ Files skipped from review due to trivial changes (7)
- proxy/tsconfig.json
- proxy/src/runners/ollama.spec.ts
- proxy/src/runners/runner-orchestrator.spec.ts
- proxy/src/adapters/base.ts
- proxy/src/adapters/bedrock-handlers/anthropic-handler.ts
- proxy/src/adapters/bedrock-handlers/anthropic-handler.spec.ts
- proxy/src/adapters/bedrock.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/docker-build.yml
- .coderabbit.yaml
| @@ -1,5 +1,6 @@ | |||
| import { BedrockAdapter } from './bedrock'; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP '\bAnthropicMessageResponse\b' -C2
fd -t f 'anthropic-handler\.ts$'Repository: JoshWheeler08/local-llm-gateway
Length of output: 2423
🏁 Script executed:
cat -n proxy/src/adapters/bedrock-handlers/anthropic-handler.ts | head -15Repository: JoshWheeler08/local-llm-gateway
Length of output: 761
🏁 Script executed:
sed -n '30,40p' proxy/src/adapters/bedrock-handlers/anthropic-handler.tsRepository: JoshWheeler08/local-llm-gateway
Length of output: 294
Add missing AnthropicMessageResponse import to anthropic-handler.ts.
The transformResponse method uses AnthropicMessageResponse as its return type (line 34), but the type is not imported. Add the import:
import { AnthropicMessageResponse } from '@/types/providers/anthropic.type';This will resolve the TS2304 error blocking the test suite.
🧰 Tools
🪛 GitHub Actions: PR Checks
[error] 1-1: Test suite failed to run due to TypeScript error in src/adapters/bedrock-handlers/anthropic-handler.ts (TS2304: Cannot find name 'AnthropicMessageResponse').
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@proxy/src/adapters/bedrock.spec.ts` at line 1, Add the missing type import
used by transformResponse: import AnthropicMessageResponse from the anthropic
types module (i.e. add "import { AnthropicMessageResponse } from
'@/types/providers/anthropic.type';") so that the transformResponse method's
return type resolves; update the top of anthropic-handler.ts to include this
import.
There was a problem hiding this comment.
Pull request overview
This PR primarily tightens TypeScript typing and normalizes formatting across the proxy codebase, while also adjusting some CI/tooling configuration (CodeRabbit + Docker build workflow triggers).
Changes:
- Strengthen error handling and reduce
anyusage by switching tounknownand more explicit casts in server and unit tests. - Normalize string quoting/import formatting across config and adapter-related modules.
- Update CodeRabbit configuration and restrict the Docker build workflow trigger behavior.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
proxy/tsconfig.json |
Explicitly enables noImplicitAny in compiler options. |
proxy/src/server.ts |
Uses unknown in catch and logs/sends a safe error message string. |
proxy/src/runners/runner-orchestrator.spec.ts |
Replaces any with a narrower union assertion in an invalid-config test case. |
proxy/src/runners/ollama.spec.ts |
Replaces any cast with unknown as Crypto for global.crypto mock. |
proxy/src/config.ts |
Normalizes quotes; introduces an additional console.log line during config load. |
proxy/src/adapters/openai.spec.ts |
Adjusts authorization header fixture to be a string. |
proxy/src/adapters/bedrock.ts |
Formatting/quote normalization and small regex cleanup. |
proxy/src/adapters/bedrock.spec.ts |
Replaces any casts with BedrockRequest typing in tests. |
proxy/src/adapters/bedrock-handlers/anthropic-handler.ts |
Import cleanup and quote normalization in the handler implementation. |
proxy/src/adapters/bedrock-handlers/anthropic-handler.spec.ts |
Removes any casts in a content-block test fixture. |
proxy/src/adapters/base.ts |
Removes unused imports and normalizes provider name formatting. |
proxy/src/adapters/anthropic.spec.ts |
Adjusts x-api-key header fixture to be a string. |
.github/workflows/docker-build.yml |
Removes pull_request trigger; normalizes quoting in path/tag patterns. |
.coderabbit.yaml |
Changes path filter configuration style and enables unit tests in finishing touches. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| id: 'unknown-runner', | ||
| endpoint: 'http://localhost:9999', | ||
| type: 'unknown' as any, | ||
| type: 'unknown' as 'vllm' | 'ollama' | 'tgi', |
| console.log(`Runners: ${config.runners.length}`); | ||
| console.log(`Providers: ${Object.keys(config.mappings).join(", ")}`); | ||
| console.log(`Providers: ${Object.keys(config.mappings).join(', ')}`); | ||
| console.log('Please summarise this pr and suggest improvements code rabbit'); |
| import { AnthropicMessageResponse } from "@/types/providers/anthropic.type"; | ||
| import { StandardMessage, StandardRequest, StandardResponse } from '@/types/standard-message.type'; | ||
| import { BedrockModelHandler } from './base-handler'; | ||
| import { BedrockAnthropicRequest } from '@/types/providers/bedrock/anthropic.type'; |
| { | ||
| role: 'user', | ||
| content: [{ type: 'image' as any, source: 'base64data' } as any], | ||
| content: [{ type: 'image', source: 'base64data' }], |
| const request: ProviderRequest = { | ||
| url: '/test', | ||
| method: 'POST', | ||
| headers: { | ||
| authorization: ['Bearer', 'token'] as any, | ||
| authorization: 'Bearer token', |
| const request: ProviderRequest = { | ||
| url: '/test', | ||
| method: 'POST', | ||
| headers: { | ||
| 'x-api-key': ['key1', 'key2'] as any, | ||
| 'x-api-key': 'key1', |
Description
Type of Change
Related Issue
Closes #(issue)
Changes Made
Testing
npm test)npm run lint)npm run typecheck)npm run build)Checklist
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
Style
Refactor
Bug Fixes
Chores
CI
Note: No user-facing feature changes.