feat: add Requesty as an OpenAI-compatible LLM provider#17
Open
Thibaultjaigu wants to merge 2 commits into
Open
feat: add Requesty as an OpenAI-compatible LLM provider#17Thibaultjaigu wants to merge 2 commits into
Thibaultjaigu wants to merge 2 commits into
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Reviewer's GuideAdds a new OpenAI-compatible Requesty LLM provider that mirrors the existing OpenRouter provider, wires it into the provider manager and config, and ensures structured output and model-output behavior are consistent with other aggregators. Flow diagram for Requesty provider wiring and usageflowchart LR
Env[REQUESTY_API_KEY in EnvironmentConfig]
Config[createModelProviderManagerConfig]
Manager[AIModelProviderManager case 'requesty']
Provider[RequestyProvider.initialize]
API[Requesty router.requesty.ai/v1]
Env --> Config --> Manager --> Provider --> API
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The
RequestyProviderConfigused inAgentOSConfig.createModelProviderManagerConfigpassesmaxRetriesandtimeout, butRequestyProviderConfigonly definesrequestTimeout/streamRequestTimeout—consider aligning the config shape or mapping those fields explicitly so runtime behavior matches expectations. - The
RequestyProvidercurrently logs toconsole.log/console.warn/console.error; if the rest of the codebase uses a centralized logging utility, it would be more consistent and controllable to route these messages through that instead of raw console calls. - In
RequestyProvider.initialize, theUser-Agentis hardcoded toAgentOS/1.0; consider deriving this from a shared constant or package metadata so it stays in sync with the actual AgentOS version.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `RequestyProviderConfig` used in `AgentOSConfig.createModelProviderManagerConfig` passes `maxRetries` and `timeout`, but `RequestyProviderConfig` only defines `requestTimeout`/`streamRequestTimeout`—consider aligning the config shape or mapping those fields explicitly so runtime behavior matches expectations.
- The `RequestyProvider` currently logs to `console.log`/`console.warn`/`console.error`; if the rest of the codebase uses a centralized logging utility, it would be more consistent and controllable to route these messages through that instead of raw console calls.
- In `RequestyProvider.initialize`, the `User-Agent` is hardcoded to `AgentOS/1.0`; consider deriving this from a shared constant or package metadata so it stays in sync with the actual AgentOS version.
## Individual Comments
### Comment 1
<location path="src/core/llm/providers/implementations/RequestyProvider.ts" line_range="610-611" />
<code_context>
+ costUSD: apiChunk.usage.cost,
+ };
+ }
+ const finalMessage: ChatMessage = {
+ role: choice.delta?.role || accumulatedToolCalls.size > 0 ? 'assistant' : (choice.message?.role || 'assistant'),
+ content: responseTextDelta || (choice.message?.content || null),
+ tool_calls: Array.from(accumulatedToolCalls.values())
</code_context>
<issue_to_address>
**issue (bug_risk):** Operator precedence in role selection is likely wrong and changes intended behavior.
This expression:
```ts
role: choice.delta?.role || accumulatedToolCalls.size > 0
? 'assistant'
: (choice.message?.role || 'assistant'),
```
actually parses as:
```ts
role: (choice.delta?.role || accumulatedToolCalls.size > 0)
? 'assistant'
: (choice.message?.role || 'assistant');
```
so any truthy `choice.delta?.role` (or `accumulatedToolCalls.size > 0`) forces the role to `'assistant'`, and `delta.role` is never surfaced.
If you meant "use `delta.role` if present, else `'assistant'` when there are tool calls, else `message.role || 'assistant'`", please parenthesize explicitly, e.g.:
```ts
role: choice.delta?.role
?? (accumulatedToolCalls.size > 0
? 'assistant'
: (choice.message?.role ?? 'assistant')),
```
(or equivalent with extra `()` and `||`).
</issue_to_address>
### Comment 2
<location path="src/core/llm/providers/implementations/RequestyProvider.ts" line_range="347-352" />
<code_context>
+
+ const accumulatedToolCalls: Map<number, { id?: string; type?: 'function'; function?: { name?: string; arguments?: string; } }> = new Map();
+
+ const abortSignal = options.abortSignal;
+ if (abortSignal?.aborted) {
+ yield { id: `requesty-abort-${Date.now()}`, object: 'chat.completion.chunk', created: Math.floor(Date.now()/1000), modelId, choices: [], error: { message: 'Stream aborted prior to first chunk', type: 'abort' }, isFinal: true };
+ return;
+ }
+ const abortHandler = () => { /* passive; loop logic handles emission */ };
+ abortSignal?.addEventListener('abort', abortHandler, { once: true });
+
</code_context>
<issue_to_address>
**suggestion:** The abort handler is a no-op and only used to add/remove a listener.
Since all abort behavior is handled via `abortSignal?.aborted` checks in the loop, this listener adds no functional value and its callback never produces side effects. Either remove the listener and rely solely on the per-iteration checks, or make the handler meaningful (e.g., set a local flag and break the loop immediately on abort).
Suggested implementation:
```typescript
const accumulatedToolCalls: Map<number, { id?: string; type?: 'function'; function?: { name?: string; arguments?: string; } }> = new Map();
const abortSignal = options.abortSignal;
if (abortSignal?.aborted) {
yield {
id: `requesty-abort-${Date.now()}`,
object: 'chat.completion.chunk',
created: Math.floor(Date.now() / 1000),
modelId,
choices: [],
error: { message: 'Stream aborted prior to first chunk', type: 'abort' },
isFinal: true,
};
return;
}
```
1. Ensure that elsewhere in this streaming method, you continue to check `abortSignal?.aborted` inside the main loop to handle mid-stream aborts.
2. If there is any corresponding `abortSignal?.removeEventListener('abort', abortHandler)` in a `finally` block or cleanup path, remove it as well to avoid referencing the deleted handler.
</issue_to_address>
### Comment 3
<location path="src/core/llm/providers/implementations/RequestyProvider.ts" line_range="122" />
<code_context>
+ public defaultModelId?: string;
+
+ // Corrected: Changed type of this.config to satisfy the Readonly<Required<...>> assignment by providing defaults
+ private config!: Readonly<Required<Omit<RequestyProviderConfig, 'defaultModelId' | 'siteUrl' | 'appName' | 'baseURL' | 'requestTimeout' | 'streamRequestTimeout'>> & RequestyProviderConfig>;
+ private keyPool: ApiKeyPool | null = null;
+ private client!: AxiosInstance;
</code_context>
<issue_to_address>
**issue (complexity):** Consider introducing small helper types and functions (for config, payload building, streaming, tool-call accumulation, and error normalization) to make this provider’s behavior clearer while reducing duplication and inline branching.
You can cut a fair amount of cognitive load here with a few small extra helpers, without changing behavior.
---
### 1. Config typing / initialization
The `Readonly<Required<Omit<...>>> & RequestyProviderConfig` intersection makes it hard to see what the runtime shape is.
You can define a concrete internal type and keep the same defaults/behavior:
```ts
interface InternalRequestyConfig {
apiKey: string;
baseURL: string;
defaultModelId?: string;
siteUrl?: string;
appName?: string;
requestTimeout: number;
streamRequestTimeout: number;
}
private config!: Readonly<InternalRequestyConfig>;
```
Then in `initialize`:
```ts
public async initialize(config: RequestyProviderConfig): Promise<void> {
// ... apiKey guard, etc.
const internalConfig: InternalRequestyConfig = {
apiKey: config.apiKey,
baseURL: config.baseURL || 'https://router.requesty.ai/v1',
defaultModelId: config.defaultModelId,
siteUrl: config.siteUrl,
appName: config.appName,
requestTimeout: config.requestTimeout ?? 60000,
streamRequestTimeout: config.streamRequestTimeout ?? 180000,
};
this.config = Object.freeze(internalConfig);
this.keyPool = new ApiKeyPool(this.config.apiKey);
this.defaultModelId = this.config.defaultModelId;
// ...
}
```
This keeps the runtime shape explicit and removes the `Omit`/`Required` intersection.
---
### 2. Duplicated completion payload construction
`generateCompletion` and `generateCompletionStream` differ only in streaming flags and timeout. You can pull the payload into a shared helper so that max_tokens / tools / response_format etc. live in one place:
```ts
private buildChatCompletionPayload(
modelId: string,
messages: ChatMessage[],
options: ModelCompletionOptions,
stream: boolean
): Record<string, unknown> {
const requestyMessages = this.mapToRequestyMessages(messages);
return {
model: modelId,
messages: requestyMessages,
stream,
...(stream && { stream_options: { include_usage: true } }),
temperature: options.temperature,
top_p: options.topP,
max_tokens: clampMaxOutputTokens(modelId, options.maxTokens) ?? 4096,
presence_penalty: options.presencePenalty,
frequency_penalty: options.frequencyPenalty,
stop: options.stopSequences,
user: options.userId,
tools: options.tools,
tool_choice: options.toolChoice,
...(options.responseFormat?.type === 'json_object' && {
response_format: { type: 'json_object' },
}),
...(options.customModelParams || {}),
};
}
```
Then use it:
```ts
public async generateCompletion(...) {
this.ensureInitialized();
const payload = this.buildChatCompletionPayload(modelId, messages, options, false);
const apiResponseData = await this.makeApiRequest<RequestyChatCompletionAPIResponse>(
'/chat/completions',
'POST',
options.requestTimeout ?? this.config.requestTimeout,
payload
);
return this.mapApiToCompletionResponse(apiResponseData, modelId);
}
public async *generateCompletionStream(...) {
this.ensureInitialized();
const payload = this.buildChatCompletionPayload(modelId, messages, options, true);
const stream = await this.makeApiRequest<NodeJS.ReadableStream>(
'/chat/completions',
'POST',
options.requestTimeout ?? this.config.streamRequestTimeout,
payload,
true
);
// ...
}
```
---
### 3. Stream chunk handling / SSE loop
`generateCompletionStream` currently mixes abort handling, SSE parsing, DONE detection, JSON parsing, and mapping. That can be moved into a single helper to make the orchestration method easier to scan:
```ts
private async *processCompletionStream(
stream: NodeJS.ReadableStream,
modelId: string,
options: ModelCompletionOptions
): AsyncGenerator<ModelCompletionResponse, void, undefined> {
const accumulatedToolCalls = new Map<number, { id?: string; type?: 'function'; function?: { name?: string; arguments?: string; } }>();
const abortSignal = options.abortSignal;
const abortHandler = () => {};
abortSignal?.addEventListener('abort', abortHandler, { once: true });
try {
if (abortSignal?.aborted) {
yield this.makeAbortChunk(modelId, 'Stream aborted prior to first chunk');
return;
}
for await (const rawChunk of this.parseSseStream(stream)) {
if (abortSignal?.aborted) {
yield this.makeAbortChunk(modelId, 'Stream aborted by caller');
break;
}
if (rawChunk === 'data: [DONE]' || (rawChunk.startsWith('data: ') && rawChunk.trim().endsWith('[DONE]'))) {
break;
}
if (!rawChunk.startsWith('data: ')) continue;
const jsonData = rawChunk.substring('data: '.length);
try {
const apiChunk = JSON.parse(jsonData) as RequestyChatCompletionAPIResponse;
yield this.mapApiToStreamChunkResponse(apiChunk, modelId, accumulatedToolCalls);
} catch (err) {
console.warn('RequestyProvider: Failed to parse stream chunk JSON, skipping chunk. Data:', jsonData, 'Error:', err);
}
}
} finally {
abortSignal?.removeEventListener('abort', abortHandler);
}
}
```
Then `generateCompletionStream` reduces to:
```ts
public async *generateCompletionStream(...) {
this.ensureInitialized();
const payload = this.buildChatCompletionPayload(modelId, messages, options, true);
const stream = await this.makeApiRequest<NodeJS.ReadableStream>(
'/chat/completions',
'POST',
options.requestTimeout ?? this.config.streamRequestTimeout,
payload,
true
);
yield* this.processCompletionStream(stream, modelId, options);
}
```
This keeps the happy-path orchestration linear and pushes low-level SSE concerns into one place.
---
### 4. Stream tool-call accumulation
`mapApiToStreamChunkResponse` is doing both tool-call accumulation and final/delta message mapping. Extracting the accumulation into a small helper makes the function easier to reason about:
```ts
private updateToolCalls(
accumulated: Map<number, { id?: string; type?: 'function'; function?: { name?: string; arguments?: string; } }>,
toolCallDeltas: NonNullable<RequestyChatChoice['delta']>['tool_calls']
): ModelCompletionResponse['toolCallsDeltas'] {
const toolCallsDeltas: NonNullable<ModelCompletionResponse['toolCallsDeltas']> = [];
for (const tcDelta of toolCallDeltas) {
let current = accumulated.get(tcDelta.index) ?? { function: { name: '', arguments: '' } };
if (tcDelta.id) current.id = tcDelta.id;
if (tcDelta.type) current.type = 'function';
if (tcDelta.function?.name) current.function!.name += tcDelta.function.name;
if (tcDelta.function?.arguments) current.function!.arguments += tcDelta.function.arguments;
accumulated.set(tcDelta.index, current);
toolCallsDeltas.push({
index: tcDelta.index,
id: tcDelta.id,
type: 'function',
function: tcDelta.function && {
name: tcDelta.function.name,
arguments_delta: tcDelta.function.arguments,
},
});
}
return toolCallsDeltas;
}
```
Then in `mapApiToStreamChunkResponse`:
```ts
let toolCallsDeltas: ModelCompletionResponse['toolCallsDeltas'];
if (choice.delta?.tool_calls) {
toolCallsDeltas = this.updateToolCalls(accumulatedToolCalls, choice.delta.tool_calls);
}
```
This pulls the mutation logic out of the already-branchy mapping code.
---
### 5. Error normalization
`makeApiRequest` has substantial error-shaping logic. Pulling that into a helper clarifies the main method:
```ts
private normalizeAxiosError(error: unknown, endpoint: string, body?: Record<string, unknown>) {
let statusCode: number | undefined;
let errorData: any;
let message = 'Unknown Requesty API error';
let type = 'UNKNOWN_API_ERROR';
if (axios.isAxiosError(error)) {
statusCode = error.response?.status;
errorData = error.response?.data;
if (errorData?.error && typeof errorData.error === 'object') {
message = errorData.error.message || message;
type = errorData.error.type || type;
} else if (typeof errorData === 'string') {
message = errorData;
} else if ((error as Error).message) {
message = (error as Error).message;
}
} else if (error instanceof Error) {
message = error.message;
}
const decoratedMessage = statusCode ? `[${statusCode}] ${message}` : message;
return {
statusCode,
errorData,
errorType: type,
decoratedMessage,
details: {
requestEndpoint: endpoint,
requestBodyPreview: body ? JSON.stringify(body).substring(0, 200) + '...' : undefined,
responseData: errorData,
underlyingError: error,
},
};
}
```
And in `makeApiRequest`:
```ts
} catch (error: unknown) {
const { statusCode, errorType, decoratedMessage, details } =
this.normalizeAxiosError(error, endpoint, body);
throw new RequestyProviderError(
decoratedMessage,
'API_REQUEST_FAILED',
statusCode,
errorType,
details
);
}
```
---
### 6. Non‑null assertions on `c.message`
`mapApiToCompletionResponse` assumes `message` is always present but uses `!`. Either tighten `RequestyChatChoice` for non-stream responses, or add a small guard:
```ts
const choices = apiResponse.choices.map(c => {
if (!c.message) {
throw new RequestyProviderError(
"Received choice without message in non-stream response.",
"API_RESPONSE_MALFORMED",
undefined,
undefined,
{ responseId: apiResponse.id, choiceIndex: c.index }
);
}
return {
index: c.index,
message: {
role: c.message.role,
content: c.message.content,
tool_calls: c.message.tool_calls,
},
finishReason: c.finish_reason,
logprobs: c.logprobs,
};
});
```
Then:
```ts
return {
id: apiResponse.id,
object: apiResponse.object,
created: apiResponse.created,
modelId: apiResponse.model || requestedModelId,
choices,
usage,
};
```
This removes the non-null assertion and makes the assumption explicit.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+347
to
+352
| const abortSignal = options.abortSignal; | ||
| if (abortSignal?.aborted) { | ||
| yield { id: `requesty-abort-${Date.now()}`, object: 'chat.completion.chunk', created: Math.floor(Date.now()/1000), modelId, choices: [], error: { message: 'Stream aborted prior to first chunk', type: 'abort' }, isFinal: true }; | ||
| return; | ||
| } | ||
| const abortHandler = () => { /* passive; loop logic handles emission */ }; |
There was a problem hiding this comment.
suggestion: The abort handler is a no-op and only used to add/remove a listener.
Since all abort behavior is handled via abortSignal?.aborted checks in the loop, this listener adds no functional value and its callback never produces side effects. Either remove the listener and rely solely on the per-iteration checks, or make the handler meaningful (e.g., set a local flag and break the loop immediately on abort).
Suggested implementation:
const accumulatedToolCalls: Map<number, { id?: string; type?: 'function'; function?: { name?: string; arguments?: string; } }> = new Map();
const abortSignal = options.abortSignal;
if (abortSignal?.aborted) {
yield {
id: `requesty-abort-${Date.now()}`,
object: 'chat.completion.chunk',
created: Math.floor(Date.now() / 1000),
modelId,
choices: [],
error: { message: 'Stream aborted prior to first chunk', type: 'abort' },
isFinal: true,
};
return;
}- Ensure that elsewhere in this streaming method, you continue to check
abortSignal?.abortedinside the main loop to handle mid-stream aborts. - If there is any corresponding
abortSignal?.removeEventListener('abort', abortHandler)in afinallyblock or cleanup path, remove it as well to avoid referencing the deleted handler.
| public defaultModelId?: string; | ||
|
|
||
| // Corrected: Changed type of this.config to satisfy the Readonly<Required<...>> assignment by providing defaults | ||
| private config!: Readonly<Required<Omit<RequestyProviderConfig, 'defaultModelId' | 'siteUrl' | 'appName' | 'baseURL' | 'requestTimeout' | 'streamRequestTimeout'>> & RequestyProviderConfig>; |
There was a problem hiding this comment.
issue (complexity): Consider introducing small helper types and functions (for config, payload building, streaming, tool-call accumulation, and error normalization) to make this provider’s behavior clearer while reducing duplication and inline branching.
You can cut a fair amount of cognitive load here with a few small extra helpers, without changing behavior.
1. Config typing / initialization
The Readonly<Required<Omit<...>>> & RequestyProviderConfig intersection makes it hard to see what the runtime shape is.
You can define a concrete internal type and keep the same defaults/behavior:
interface InternalRequestyConfig {
apiKey: string;
baseURL: string;
defaultModelId?: string;
siteUrl?: string;
appName?: string;
requestTimeout: number;
streamRequestTimeout: number;
}
private config!: Readonly<InternalRequestyConfig>;Then in initialize:
public async initialize(config: RequestyProviderConfig): Promise<void> {
// ... apiKey guard, etc.
const internalConfig: InternalRequestyConfig = {
apiKey: config.apiKey,
baseURL: config.baseURL || 'https://router.requesty.ai/v1',
defaultModelId: config.defaultModelId,
siteUrl: config.siteUrl,
appName: config.appName,
requestTimeout: config.requestTimeout ?? 60000,
streamRequestTimeout: config.streamRequestTimeout ?? 180000,
};
this.config = Object.freeze(internalConfig);
this.keyPool = new ApiKeyPool(this.config.apiKey);
this.defaultModelId = this.config.defaultModelId;
// ...
}This keeps the runtime shape explicit and removes the Omit/Required intersection.
2. Duplicated completion payload construction
generateCompletion and generateCompletionStream differ only in streaming flags and timeout. You can pull the payload into a shared helper so that max_tokens / tools / response_format etc. live in one place:
private buildChatCompletionPayload(
modelId: string,
messages: ChatMessage[],
options: ModelCompletionOptions,
stream: boolean
): Record<string, unknown> {
const requestyMessages = this.mapToRequestyMessages(messages);
return {
model: modelId,
messages: requestyMessages,
stream,
...(stream && { stream_options: { include_usage: true } }),
temperature: options.temperature,
top_p: options.topP,
max_tokens: clampMaxOutputTokens(modelId, options.maxTokens) ?? 4096,
presence_penalty: options.presencePenalty,
frequency_penalty: options.frequencyPenalty,
stop: options.stopSequences,
user: options.userId,
tools: options.tools,
tool_choice: options.toolChoice,
...(options.responseFormat?.type === 'json_object' && {
response_format: { type: 'json_object' },
}),
...(options.customModelParams || {}),
};
}Then use it:
public async generateCompletion(...) {
this.ensureInitialized();
const payload = this.buildChatCompletionPayload(modelId, messages, options, false);
const apiResponseData = await this.makeApiRequest<RequestyChatCompletionAPIResponse>(
'/chat/completions',
'POST',
options.requestTimeout ?? this.config.requestTimeout,
payload
);
return this.mapApiToCompletionResponse(apiResponseData, modelId);
}
public async *generateCompletionStream(...) {
this.ensureInitialized();
const payload = this.buildChatCompletionPayload(modelId, messages, options, true);
const stream = await this.makeApiRequest<NodeJS.ReadableStream>(
'/chat/completions',
'POST',
options.requestTimeout ?? this.config.streamRequestTimeout,
payload,
true
);
// ...
}3. Stream chunk handling / SSE loop
generateCompletionStream currently mixes abort handling, SSE parsing, DONE detection, JSON parsing, and mapping. That can be moved into a single helper to make the orchestration method easier to scan:
private async *processCompletionStream(
stream: NodeJS.ReadableStream,
modelId: string,
options: ModelCompletionOptions
): AsyncGenerator<ModelCompletionResponse, void, undefined> {
const accumulatedToolCalls = new Map<number, { id?: string; type?: 'function'; function?: { name?: string; arguments?: string; } }>();
const abortSignal = options.abortSignal;
const abortHandler = () => {};
abortSignal?.addEventListener('abort', abortHandler, { once: true });
try {
if (abortSignal?.aborted) {
yield this.makeAbortChunk(modelId, 'Stream aborted prior to first chunk');
return;
}
for await (const rawChunk of this.parseSseStream(stream)) {
if (abortSignal?.aborted) {
yield this.makeAbortChunk(modelId, 'Stream aborted by caller');
break;
}
if (rawChunk === 'data: [DONE]' || (rawChunk.startsWith('data: ') && rawChunk.trim().endsWith('[DONE]'))) {
break;
}
if (!rawChunk.startsWith('data: ')) continue;
const jsonData = rawChunk.substring('data: '.length);
try {
const apiChunk = JSON.parse(jsonData) as RequestyChatCompletionAPIResponse;
yield this.mapApiToStreamChunkResponse(apiChunk, modelId, accumulatedToolCalls);
} catch (err) {
console.warn('RequestyProvider: Failed to parse stream chunk JSON, skipping chunk. Data:', jsonData, 'Error:', err);
}
}
} finally {
abortSignal?.removeEventListener('abort', abortHandler);
}
}Then generateCompletionStream reduces to:
public async *generateCompletionStream(...) {
this.ensureInitialized();
const payload = this.buildChatCompletionPayload(modelId, messages, options, true);
const stream = await this.makeApiRequest<NodeJS.ReadableStream>(
'/chat/completions',
'POST',
options.requestTimeout ?? this.config.streamRequestTimeout,
payload,
true
);
yield* this.processCompletionStream(stream, modelId, options);
}This keeps the happy-path orchestration linear and pushes low-level SSE concerns into one place.
4. Stream tool-call accumulation
mapApiToStreamChunkResponse is doing both tool-call accumulation and final/delta message mapping. Extracting the accumulation into a small helper makes the function easier to reason about:
private updateToolCalls(
accumulated: Map<number, { id?: string; type?: 'function'; function?: { name?: string; arguments?: string; } }>,
toolCallDeltas: NonNullable<RequestyChatChoice['delta']>['tool_calls']
): ModelCompletionResponse['toolCallsDeltas'] {
const toolCallsDeltas: NonNullable<ModelCompletionResponse['toolCallsDeltas']> = [];
for (const tcDelta of toolCallDeltas) {
let current = accumulated.get(tcDelta.index) ?? { function: { name: '', arguments: '' } };
if (tcDelta.id) current.id = tcDelta.id;
if (tcDelta.type) current.type = 'function';
if (tcDelta.function?.name) current.function!.name += tcDelta.function.name;
if (tcDelta.function?.arguments) current.function!.arguments += tcDelta.function.arguments;
accumulated.set(tcDelta.index, current);
toolCallsDeltas.push({
index: tcDelta.index,
id: tcDelta.id,
type: 'function',
function: tcDelta.function && {
name: tcDelta.function.name,
arguments_delta: tcDelta.function.arguments,
},
});
}
return toolCallsDeltas;
}Then in mapApiToStreamChunkResponse:
let toolCallsDeltas: ModelCompletionResponse['toolCallsDeltas'];
if (choice.delta?.tool_calls) {
toolCallsDeltas = this.updateToolCalls(accumulatedToolCalls, choice.delta.tool_calls);
}This pulls the mutation logic out of the already-branchy mapping code.
5. Error normalization
makeApiRequest has substantial error-shaping logic. Pulling that into a helper clarifies the main method:
private normalizeAxiosError(error: unknown, endpoint: string, body?: Record<string, unknown>) {
let statusCode: number | undefined;
let errorData: any;
let message = 'Unknown Requesty API error';
let type = 'UNKNOWN_API_ERROR';
if (axios.isAxiosError(error)) {
statusCode = error.response?.status;
errorData = error.response?.data;
if (errorData?.error && typeof errorData.error === 'object') {
message = errorData.error.message || message;
type = errorData.error.type || type;
} else if (typeof errorData === 'string') {
message = errorData;
} else if ((error as Error).message) {
message = (error as Error).message;
}
} else if (error instanceof Error) {
message = error.message;
}
const decoratedMessage = statusCode ? `[${statusCode}] ${message}` : message;
return {
statusCode,
errorData,
errorType: type,
decoratedMessage,
details: {
requestEndpoint: endpoint,
requestBodyPreview: body ? JSON.stringify(body).substring(0, 200) + '...' : undefined,
responseData: errorData,
underlyingError: error,
},
};
}And in makeApiRequest:
} catch (error: unknown) {
const { statusCode, errorType, decoratedMessage, details } =
this.normalizeAxiosError(error, endpoint, body);
throw new RequestyProviderError(
decoratedMessage,
'API_REQUEST_FAILED',
statusCode,
errorType,
details
);
}6. Non‑null assertions on c.message
mapApiToCompletionResponse assumes message is always present but uses !. Either tighten RequestyChatChoice for non-stream responses, or add a small guard:
const choices = apiResponse.choices.map(c => {
if (!c.message) {
throw new RequestyProviderError(
"Received choice without message in non-stream response.",
"API_RESPONSE_MALFORMED",
undefined,
undefined,
{ responseId: apiResponse.id, choiceIndex: c.index }
);
}
return {
index: c.index,
message: {
role: c.message.role,
content: c.message.content,
tool_calls: c.message.tool_calls,
},
finishReason: c.finish_reason,
logprobs: c.logprobs,
};
});Then:
return {
id: apiResponse.id,
object: apiResponse.object,
created: apiResponse.created,
modelId: apiResponse.model || requestedModelId,
choices,
usage,
};This removes the non-null assertion and makes the assumption explicit.
PR Summary by QodoAdd Requesty as an OpenAI-compatible LLM provider Description
Diagram
High-Level Assessment
Files changed (5)
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new ChangesRequesty LLM Provider Integration
OpenRouter Streaming Role Fix
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Code Review by Qodo
1. Requesty not env-resolved
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
+130
to
+132
| case 'requesty': | ||
| providerInstance = new RequestyProvider(); | ||
| break; |
There was a problem hiding this comment.
1. Requesty not env-resolved 🐞 Bug ≡ Correctness
The high-level API provider resolution does not read REQUESTY_API_KEY or provide defaults for
providerId='requesty', so Requesty cannot be selected via env auto-detect or via generateText({
provider: 'requesty' }) without explicit apiKey/model wiring.
Agent Prompt
### Issue description
Requesty is wired into the core provider manager, but the public/high-level API resolution layer (`src/api`) still has no knowledge of it. As a result:
- `autoDetectProvider()` will never choose Requesty even when `REQUESTY_API_KEY` is set.
- `resolveProvider('requesty', ...)` will not read `process.env.REQUESTY_API_KEY` because `ENV_KEY_MAP` omits `requesty`.
- `resolveModelOption({ provider: 'requesty' })` will throw because `PROVIDER_DEFAULTS` has no `requesty` entry.
### Issue Context
This PR adds `case 'requesty'` in the core provider factory, but common call paths for end-users go through `src/api/model.ts` + `src/api/runtime/provider-defaults.ts`.
### Fix Focus Areas
- src/api/model.ts[40-60]
- src/api/runtime/provider-defaults.ts[37-142]
- src/api/runtime/__tests__/provider-defaults.test.ts[1-220]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+236
to
+249
| // Requesty Provider (OpenAI-compatible LLM gateway) | ||
| if (env.REQUESTY_API_KEY) { | ||
| providers.push({ | ||
| providerId: 'requesty', | ||
| enabled: true, | ||
| isDefault: !env.OPENAI_API_KEY && !env.OPENROUTER_API_KEY, // Default to Requesty if OpenAI/OpenRouter not available | ||
| config: { | ||
| apiKey: env.REQUESTY_API_KEY, | ||
| baseURL: 'https://router.requesty.ai/v1', | ||
| defaultModel: 'openai/gpt-4o', | ||
| maxRetries: 3, | ||
| timeout: 60000, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
2. Requesty config keys wrong 🐞 Bug ≡ Correctness
AgentOSConfig registers Requesty with config fields (defaultModel/timeout/maxRetries) that RequestyProvider does not read (defaultModelId/requestTimeout/streamRequestTimeout), so the configured default model and timeouts are silently ignored.
Agent Prompt
### Issue description
`createModelProviderManagerConfig()` configures Requesty using keys that don't match `RequestyProviderConfig`. RequestyProvider reads `defaultModelId`, `requestTimeout`, and `streamRequestTimeout`, but the config block passes `defaultModel`, `timeout`, and `maxRetries`.
### Issue Context
This causes RequestyProvider to initialize with `defaultModelId` unset and to ignore the intended timeout configuration.
### Fix Focus Areas
- src/core/config/AgentOSConfig.ts[236-249]
- src/core/llm/providers/implementations/RequestyProvider.ts[31-39]
- src/core/llm/providers/implementations/RequestyProvider.ts[138-148]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+336
to
+384
| const stream = await this.makeApiRequest<NodeJS.ReadableStream>( | ||
| '/chat/completions', | ||
| 'POST', | ||
| // CR8: honor a per-call requestTimeout override over the stream default. | ||
| options.requestTimeout ?? this.config.streamRequestTimeout, | ||
| payload, | ||
| true | ||
| ); | ||
|
|
||
| const accumulatedToolCalls: Map<number, { id?: string; type?: 'function'; function?: { name?: string; arguments?: string; } }> = new Map(); | ||
|
|
||
| const abortSignal = options.abortSignal; | ||
| if (abortSignal?.aborted) { | ||
| yield { id: `requesty-abort-${Date.now()}`, object: 'chat.completion.chunk', created: Math.floor(Date.now()/1000), modelId, choices: [], error: { message: 'Stream aborted prior to first chunk', type: 'abort' }, isFinal: true }; | ||
| return; | ||
| } | ||
| const abortHandler = () => { /* passive; loop logic handles emission */ }; | ||
| abortSignal?.addEventListener('abort', abortHandler, { once: true }); | ||
|
|
||
| for await (const rawChunk of this.parseSseStream(stream)) { | ||
| if (abortSignal?.aborted) { | ||
| yield { id: `requesty-abort-${Date.now()}`, object: 'chat.completion.chunk', created: Math.floor(Date.now()/1000), modelId, choices: [], error: { message: 'Stream aborted by caller', type: 'abort' }, isFinal: true }; | ||
| break; | ||
| } | ||
| if (rawChunk.startsWith('data: ') && rawChunk.includes('[DONE]')) { | ||
| const doneData = rawChunk.substring('data: '.length).trim(); | ||
| if (doneData === '[DONE]') break; | ||
| } | ||
| if (rawChunk === 'data: [DONE]') { | ||
| break; | ||
| } | ||
|
|
||
| if (rawChunk.startsWith('data: ')) { | ||
| const jsonData = rawChunk.substring('data: '.length); | ||
| try { | ||
| const apiChunk = JSON.parse(jsonData) as RequestyChatCompletionAPIResponse; | ||
| yield this.mapApiToStreamChunkResponse(apiChunk, modelId, accumulatedToolCalls); | ||
| // Don't break on finish_reason: with stream_options.include_usage, | ||
| // Requesty (like OpenAI) emits a trailing usage-only chunk AFTER | ||
| // the finish_reason chunk and BEFORE [DONE]. Breaking here would | ||
| // skip the usage chunk and zero out the caller's token totals. The | ||
| // [DONE] marker check above is the right termination signal. | ||
| } catch (error: unknown) { | ||
| console.warn('RequestyProvider: Failed to parse stream chunk JSON, skipping chunk. Data:', jsonData, 'Error:', error); | ||
| } | ||
| } | ||
| } | ||
| abortSignal?.removeEventListener('abort', abortHandler); | ||
| } |
There was a problem hiding this comment.
3. Streaming errors escape generator 🐞 Bug ☼ Reliability
RequestyProvider.generateCompletionStream() can throw if makeApiRequest() fails or parseSseStream() errors, instead of emitting a final chunk with isFinal:true and an error payload, breaking the documented streaming semantics consumers rely on.
Agent Prompt
### Issue description
`generateCompletionStream()` does not wrap request setup and SSE iteration in a try/catch. If `makeApiRequest()` throws (network/auth/etc) or `parseSseStream()` throws while reading, the async generator itself throws and no terminal `ModelCompletionResponse` with `isFinal: true` is emitted.
### Issue Context
The provider contract explicitly states streamed calls MUST emit a terminal chunk with `isFinal: true` even on error, so downstream consumers can always teardown deterministically.
### Fix Focus Areas
- src/core/llm/providers/IProvider.ts[19-29]
- src/core/llm/providers/implementations/RequestyProvider.ts[336-384]
- src/core/llm/providers/implementations/RequestyProvider.ts[711-742]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/config/AgentOSConfig.ts`:
- Around line 236-248: The Requesty provider configuration block is using
incorrect field names that do not match what RequestyProviderConfig expects. In
the config object within the Requesty provider block, rename the field
`defaultModel` to `defaultModelId` and rename the field `timeout` to
`requestTimeout` to ensure these configuration values are properly read by the
RequestyProvider instead of being silently ignored.
- Line 52: The new REQUESTY_API_KEY field has been added to the AgentOSConfig
interface, but the validation logic that checks for "no LLM provider configured"
warning does not include this new key in its condition. Find the warning
condition that validates whether at least one LLM provider is configured (likely
checking other API keys like OPENAI_API_KEY, ANTHROPIC_API_KEY, etc.) and add
REQUESTY_API_KEY to that check so that a Requesty-only deployment does not
trigger the warning.
In `@src/core/llm/providers/implementations/RequestyProvider.ts`:
- Around line 701-707: Remove the requestBodyPreview property from the error
object passed to RequestyProviderError in the throw statement, as it exposes
sensitive user data like prompts and tool arguments. Keep only the safe
properties like requestEndpoint, responseData, and underlyingError in the error
context.
- Around line 401-412: The issue is that customModelParams is spread directly
onto the root payload object, but the code then attempts to write inputType to
payload.customModelParams, creating a new nested structure that won't be sent in
the request. Instead of conditionally assigning to payload.customModelParams,
directly assign the input_type property to the root payload object when
options?.inputType exists, similar to how encoding_format and dimensions are
already being handled via the spread operator pattern.
- Around line 336-384: The abort signal handling in the stream method has two
issues: first, the abort check happens after makeApiRequest is already called,
so already-aborted signals don't prevent the API request, and second, the
removeEventListener call is unprotected and won't execute if the stream loop
throws an error. Move the abort signal check to occur before calling
makeApiRequest on the stream so that aborted calls don't hit the API, and wrap
both the stream loop and the removeEventListener cleanup in a try-finally block
to ensure the listener is always removed even if an error occurs during
streaming.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5c55b6e7-f753-4058-a0e1-7ccdefee7b57
📒 Files selected for processing (5)
src/core/config/AgentOSConfig.tssrc/core/llm/providers/AIModelProviderManager.tssrc/core/llm/providers/errors/RequestyProviderError.tssrc/core/llm/providers/implementations/RequestyProvider.tssrc/core/llm/providers/structuredOutputFormat.ts
| OPENAI_API_KEY?: string; | ||
| ANTHROPIC_API_KEY?: string; | ||
| OPENROUTER_API_KEY?: string; | ||
| REQUESTY_API_KEY?: string; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Include Requesty in the “no LLM provider configured” warning condition.
After adding REQUESTY_API_KEY, a Requesty-only deployment still triggers the warning path because the condition doesn’t include this key.
💡 Suggested update
if (
!env.OPENAI_API_KEY &&
!env.ANTHROPIC_API_KEY &&
!env.OPENROUTER_API_KEY &&
+ !env.REQUESTY_API_KEY &&
!env.OLLAMA_BASE_URL
) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/config/AgentOSConfig.ts` at line 52, The new REQUESTY_API_KEY field
has been added to the AgentOSConfig interface, but the validation logic that
checks for "no LLM provider configured" warning does not include this new key in
its condition. Find the warning condition that validates whether at least one
LLM provider is configured (likely checking other API keys like OPENAI_API_KEY,
ANTHROPIC_API_KEY, etc.) and add REQUESTY_API_KEY to that check so that a
Requesty-only deployment does not trigger the warning.
Comment on lines
+236
to
+248
| // Requesty Provider (OpenAI-compatible LLM gateway) | ||
| if (env.REQUESTY_API_KEY) { | ||
| providers.push({ | ||
| providerId: 'requesty', | ||
| enabled: true, | ||
| isDefault: !env.OPENAI_API_KEY && !env.OPENROUTER_API_KEY, // Default to Requesty if OpenAI/OpenRouter not available | ||
| config: { | ||
| apiKey: env.REQUESTY_API_KEY, | ||
| baseURL: 'https://router.requesty.ai/v1', | ||
| defaultModel: 'openai/gpt-4o', | ||
| maxRetries: 3, | ||
| timeout: 60000, | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Use RequestyProviderConfig field names in the Requesty block.
The config uses defaultModel and timeout, but RequestyProvider reads defaultModelId and requestTimeout. As-is, the intended defaults are silently ignored.
💡 Proposed fix
config: {
apiKey: env.REQUESTY_API_KEY,
baseURL: 'https://router.requesty.ai/v1',
- defaultModel: 'openai/gpt-4o',
- maxRetries: 3,
- timeout: 60000,
+ defaultModelId: 'openai/gpt-4o',
+ requestTimeout: 60000,
+ streamRequestTimeout: 180000,
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Requesty Provider (OpenAI-compatible LLM gateway) | |
| if (env.REQUESTY_API_KEY) { | |
| providers.push({ | |
| providerId: 'requesty', | |
| enabled: true, | |
| isDefault: !env.OPENAI_API_KEY && !env.OPENROUTER_API_KEY, // Default to Requesty if OpenAI/OpenRouter not available | |
| config: { | |
| apiKey: env.REQUESTY_API_KEY, | |
| baseURL: 'https://router.requesty.ai/v1', | |
| defaultModel: 'openai/gpt-4o', | |
| maxRetries: 3, | |
| timeout: 60000, | |
| }, | |
| // Requesty Provider (OpenAI-compatible LLM gateway) | |
| if (env.REQUESTY_API_KEY) { | |
| providers.push({ | |
| providerId: 'requesty', | |
| enabled: true, | |
| isDefault: !env.OPENAI_API_KEY && !env.OPENROUTER_API_KEY, // Default to Requesty if OpenAI/OpenRouter not available | |
| config: { | |
| apiKey: env.REQUESTY_API_KEY, | |
| baseURL: 'https://router.requesty.ai/v1', | |
| defaultModelId: 'openai/gpt-4o', | |
| requestTimeout: 60000, | |
| streamRequestTimeout: 180000, | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/config/AgentOSConfig.ts` around lines 236 - 248, The Requesty
provider configuration block is using incorrect field names that do not match
what RequestyProviderConfig expects. In the config object within the Requesty
provider block, rename the field `defaultModel` to `defaultModelId` and rename
the field `timeout` to `requestTimeout` to ensure these configuration values are
properly read by the RequestyProvider instead of being silently ignored.
Comment on lines
+336
to
+384
| const stream = await this.makeApiRequest<NodeJS.ReadableStream>( | ||
| '/chat/completions', | ||
| 'POST', | ||
| // CR8: honor a per-call requestTimeout override over the stream default. | ||
| options.requestTimeout ?? this.config.streamRequestTimeout, | ||
| payload, | ||
| true | ||
| ); | ||
|
|
||
| const accumulatedToolCalls: Map<number, { id?: string; type?: 'function'; function?: { name?: string; arguments?: string; } }> = new Map(); | ||
|
|
||
| const abortSignal = options.abortSignal; | ||
| if (abortSignal?.aborted) { | ||
| yield { id: `requesty-abort-${Date.now()}`, object: 'chat.completion.chunk', created: Math.floor(Date.now()/1000), modelId, choices: [], error: { message: 'Stream aborted prior to first chunk', type: 'abort' }, isFinal: true }; | ||
| return; | ||
| } | ||
| const abortHandler = () => { /* passive; loop logic handles emission */ }; | ||
| abortSignal?.addEventListener('abort', abortHandler, { once: true }); | ||
|
|
||
| for await (const rawChunk of this.parseSseStream(stream)) { | ||
| if (abortSignal?.aborted) { | ||
| yield { id: `requesty-abort-${Date.now()}`, object: 'chat.completion.chunk', created: Math.floor(Date.now()/1000), modelId, choices: [], error: { message: 'Stream aborted by caller', type: 'abort' }, isFinal: true }; | ||
| break; | ||
| } | ||
| if (rawChunk.startsWith('data: ') && rawChunk.includes('[DONE]')) { | ||
| const doneData = rawChunk.substring('data: '.length).trim(); | ||
| if (doneData === '[DONE]') break; | ||
| } | ||
| if (rawChunk === 'data: [DONE]') { | ||
| break; | ||
| } | ||
|
|
||
| if (rawChunk.startsWith('data: ')) { | ||
| const jsonData = rawChunk.substring('data: '.length); | ||
| try { | ||
| const apiChunk = JSON.parse(jsonData) as RequestyChatCompletionAPIResponse; | ||
| yield this.mapApiToStreamChunkResponse(apiChunk, modelId, accumulatedToolCalls); | ||
| // Don't break on finish_reason: with stream_options.include_usage, | ||
| // Requesty (like OpenAI) emits a trailing usage-only chunk AFTER | ||
| // the finish_reason chunk and BEFORE [DONE]. Breaking here would | ||
| // skip the usage chunk and zero out the caller's token totals. The | ||
| // [DONE] marker check above is the right termination signal. | ||
| } catch (error: unknown) { | ||
| console.warn('RequestyProvider: Failed to parse stream chunk JSON, skipping chunk. Data:', jsonData, 'Error:', error); | ||
| } | ||
| } | ||
| } | ||
| abortSignal?.removeEventListener('abort', abortHandler); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Respect abort before opening the stream and always clean up listeners.
abortSignal is checked only after the streaming request is created, so already-aborted calls still hit /chat/completions. Also, listener removal is not protected if the stream loop throws.
💡 Proposed fix
+ const abortSignal = options.abortSignal;
+ if (abortSignal?.aborted) {
+ yield {
+ id: `requesty-abort-${Date.now()}`,
+ object: 'chat.completion.chunk',
+ created: Math.floor(Date.now() / 1000),
+ modelId,
+ choices: [],
+ error: { message: 'Stream aborted prior to request dispatch', type: 'abort' },
+ isFinal: true
+ };
+ return;
+ }
+
const stream = await this.makeApiRequest<NodeJS.ReadableStream>(
'/chat/completions',
'POST',
options.requestTimeout ?? this.config.streamRequestTimeout,
payload,
true
);
@@
- const abortSignal = options.abortSignal;
- if (abortSignal?.aborted) {
- yield { id: `requesty-abort-${Date.now()}`, object: 'chat.completion.chunk', created: Math.floor(Date.now()/1000), modelId, choices: [], error: { message: 'Stream aborted prior to first chunk', type: 'abort' }, isFinal: true };
- return;
- }
const abortHandler = () => { /* passive; loop logic handles emission */ };
abortSignal?.addEventListener('abort', abortHandler, { once: true });
-
- for await (const rawChunk of this.parseSseStream(stream)) {
+ try {
+ for await (const rawChunk of this.parseSseStream(stream)) {
...
- }
- abortSignal?.removeEventListener('abort', abortHandler);
+ }
+ } finally {
+ abortSignal?.removeEventListener('abort', abortHandler);
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/llm/providers/implementations/RequestyProvider.ts` around lines 336
- 384, The abort signal handling in the stream method has two issues: first, the
abort check happens after makeApiRequest is already called, so already-aborted
signals don't prevent the API request, and second, the removeEventListener call
is unprotected and won't execute if the stream loop throws an error. Move the
abort signal check to occur before calling makeApiRequest on the stream so that
aborted calls don't hit the API, and wrap both the stream loop and the
removeEventListener cleanup in a try-finally block to ensure the listener is
always removed even if an error occurs during streaming.
Comment on lines
+401
to
+412
| const payload: Record<string, unknown> = { | ||
| model: modelId, | ||
| input: texts, | ||
| ...(options?.encodingFormat && { encoding_format: options.encodingFormat }), | ||
| ...(options?.dimensions && { dimensions: options.dimensions }), | ||
| ...(options?.customModelParams || {}), | ||
| }; | ||
| if (options?.inputType && payload.customModelParams && typeof payload.customModelParams === 'object') { | ||
| (payload.customModelParams as Record<string, unknown>).input_type = options.inputType; | ||
| } else if (options?.inputType) { | ||
| payload.customModelParams = { input_type: options.inputType }; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
inputType is written to the wrong payload shape.
customModelParams is spread onto the root payload, but inputType is later written under payload.customModelParams. That nested key is not part of the current request shape and can drop the parameter.
💡 Proposed fix
const payload: Record<string, unknown> = {
model: modelId,
input: texts,
...(options?.encodingFormat && { encoding_format: options.encodingFormat }),
...(options?.dimensions && { dimensions: options.dimensions }),
+ ...(options?.inputType && { input_type: options.inputType }),
...(options?.customModelParams || {}),
};
- if (options?.inputType && payload.customModelParams && typeof payload.customModelParams === 'object') {
- (payload.customModelParams as Record<string, unknown>).input_type = options.inputType;
- } else if (options?.inputType) {
- payload.customModelParams = { input_type: options.inputType };
- }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/llm/providers/implementations/RequestyProvider.ts` around lines 401
- 412, The issue is that customModelParams is spread directly onto the root
payload object, but the code then attempts to write inputType to
payload.customModelParams, creating a new nested structure that won't be sent in
the request. Instead of conditionally assigning to payload.customModelParams,
directly assign the input_type property to the root payload object when
options?.inputType exists, similar to how encoding_format and dimensions are
already being handled via the spread operator pattern.
Comment on lines
+701
to
+707
| throw new RequestyProviderError( | ||
| decoratedMessage, | ||
| 'API_REQUEST_FAILED', | ||
| statusCode, | ||
| errorType, | ||
| { requestEndpoint: endpoint, requestBodyPreview: body ? JSON.stringify(body).substring(0, 200) + '...' : undefined, responseData: errorData, underlyingError: error } | ||
| ); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not attach raw request-body previews to provider errors.
requestBodyPreview can include user prompts/tool args and leaks sensitive data when error details are logged upstream.
💡 Proposed fix
throw new RequestyProviderError(
decoratedMessage,
'API_REQUEST_FAILED',
statusCode,
errorType,
- { requestEndpoint: endpoint, requestBodyPreview: body ? JSON.stringify(body).substring(0, 200) + '...' : undefined, responseData: errorData, underlyingError: error }
+ {
+ requestEndpoint: endpoint,
+ requestBodyKeys: body ? Object.keys(body) : undefined,
+ responseData: errorData,
+ underlyingError: error
+ }
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| throw new RequestyProviderError( | |
| decoratedMessage, | |
| 'API_REQUEST_FAILED', | |
| statusCode, | |
| errorType, | |
| { requestEndpoint: endpoint, requestBodyPreview: body ? JSON.stringify(body).substring(0, 200) + '...' : undefined, responseData: errorData, underlyingError: error } | |
| ); | |
| throw new RequestyProviderError( | |
| decoratedMessage, | |
| 'API_REQUEST_FAILED', | |
| statusCode, | |
| errorType, | |
| { | |
| requestEndpoint: endpoint, | |
| requestBodyKeys: body ? Object.keys(body) : undefined, | |
| responseData: errorData, | |
| underlyingError: error | |
| } | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/llm/providers/implementations/RequestyProvider.ts` around lines 701
- 707, Remove the requestBodyPreview property from the error object passed to
RequestyProviderError in the throw statement, as it exposes sensitive user data
like prompts and tool arguments. Keep only the safe properties like
requestEndpoint, responseData, and underlyingError in the error context.
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.
Add Requesty as an OpenAI-compatible LLM provider
This adds a native
RequestyProviderto the LLM provider system, mirroring theexisting
OpenRouterProvideras closely as possible. Requestyis an OpenAI-compatible LLM gateway (base URL
https://router.requesty.ai/v1)that exposes models from many upstreams under
provider/modelnaming(e.g.
openai/gpt-4o-mini) — the same aggregator shape AgentOS already supportsfor OpenRouter.
What's added
src/core/llm/providers/implementations/RequestyProvider.ts— a fullIProviderimplementation, a faithful copy ofOpenRouterProvider:axios client,
RequestyProviderConfig(apiKey,baseURL?,defaultModelId?,siteUrl?,appName?, timeouts), default base URLhttps://router.requesty.ai/v1,chat completions (sync + streaming with
stream_options.include_usage),embeddings, model listing, health check, and
clampMaxOutputTokensfrommodel-output-limits. Because Requesty is OpenAI-compatible exactly likeOpenRouter, all request/response/embeddings/error logic is identical. The
optional
HTTP-Referer/X-Titleheaders (fromsiteUrl/appName) arepreserved — Requesty accepts them too.
src/core/llm/providers/errors/RequestyProviderError.ts— error classmirroring
OpenRouterProviderError(same shape,providerId: 'requesty').Wiring sites
AIModelProviderManager.ts: added theRequestyProvider/RequestyProviderConfigimport, theRequestyProviderConfigmember of theprovider-config union, and a
case 'requesty':to the factory switch.structuredOutputFormat.ts: addedcase 'requesty':to the same branch asopenrouter— best-effortjson_object, since an aggregator may not enforce aJSON schema across all upstreams (caller-side Zod validation still runs).
model-output-limits.ts: no change needed — theprovider/prefix strippingis already generic (
/^[^/]+\//) and handles Requesty'sprovider/modelidsidentically to OpenRouter's.
AgentOSConfig.ts: added theREQUESTY_API_KEYenv field/passthrough and aprovider registration block (mirrors the OpenRouter one) so a
REQUESTY_API_KEYis auto-wired into the manager.
Verification
tsc --noEmit(pnpm run typecheck) passes cleanly.https://router.requesty.ai/v1/chat/completionswith
openai/gpt-4o-minireturned HTTP 200 and a real completion.Docs: https://requesty.ai · https://docs.requesty.ai
I work at Requesty. This mirrors the existing OpenRouter provider as closely as possible. Happy to adjust or close it if it's not a fit.
Summary by Sourcery
Add Requesty as a first-class OpenAI-compatible LLM provider and wire it into the model provider configuration and structured output handling.
New Features:
Enhancements:
Summary by CodeRabbit
Release Notes
New Features
REQUESTY_API_KEY. It supports chat completions, streaming responses, embeddings, and model discovery.Bug Fixes