feat: add Requesty as an OpenAI-compatible LLM provider#17
Open
Thibaultjaigu wants to merge 3 commits into
Open
feat: add Requesty as an OpenAI-compatible LLM provider#17Thibaultjaigu wants to merge 3 commits into
Thibaultjaigu wants to merge 3 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)
|
📝 WalkthroughWalkthroughAdds Requesty provider support across configuration, runtime defaults, provider registration, structured output handling, and the provider implementation itself. Also corrects OpenRouter streamed role selection. 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)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
![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
…m error contract, abort, embeddings payload)
Author
|
Thanks for the thorough reviews — pushed fixes in d431e8d:
Skipped the Sourcery "introduce helper types / reduce complexity" refactor to keep the diff minimal and the role-precedence nit (already resolved in eb55cf4).
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/api/runtime/provider-defaults.ts (1)
58-61: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
requestydefaults omit anembeddingmodel.Requesty exposes an OpenAI-compatible
/embeddingsendpoint and the newRequestyProviderimplementsgenerateEmbeddings(), butPROVIDER_DEFAULTS.requestyonly declarestextandcheap.autoDetectProvider('embedding')skips any provider lacking the requested task (Line 191), so Requesty will never be auto-selected for embeddings even whenREQUESTY_API_KEYis the only key set. Consider adding anembeddingdefault if that path is expected to work.🤖 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/api/runtime/provider-defaults.ts` around lines 58 - 61, The Requesty provider defaults are missing an embedding model, so auto-detection cannot select Requesty for embedding tasks. Update PROVIDER_DEFAULTS.requesty in provider-defaults to include an embedding entry that matches the model supported by RequestyProvider.generateEmbeddings(), so autoDetectProvider('embedding') can consider Requesty when only REQUESTY_API_KEY is available.
🤖 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.
Nitpick comments:
In `@src/api/runtime/provider-defaults.ts`:
- Around line 58-61: The Requesty provider defaults are missing an embedding
model, so auto-detection cannot select Requesty for embedding tasks. Update
PROVIDER_DEFAULTS.requesty in provider-defaults to include an embedding entry
that matches the model supported by RequestyProvider.generateEmbeddings(), so
autoDetectProvider('embedding') can consider Requesty when only REQUESTY_API_KEY
is available.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7e70acee-13ac-4c19-a8e0-e323d277bd19
📒 Files selected for processing (5)
src/api/model.tssrc/api/runtime/__tests__/provider-defaults.test.tssrc/api/runtime/provider-defaults.tssrc/core/config/AgentOSConfig.tssrc/core/llm/providers/implementations/RequestyProvider.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/core/config/AgentOSConfig.ts
- src/core/llm/providers/implementations/RequestyProvider.ts
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
REQUESTY_API_KEY(optionallyREQUESTY_BASE_URL), with support for chat completions, streaming, embeddings, and model discovery.