Adds deco llm binding #107
Merged
Conversation
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
Signed-off-by: Marcos Candeia <[email protected]>
Signed-off-by: Marcos Candeia <[email protected]>
🚀 Preview Deployments Ready!Your changes have been deployed to preview environments: 📦
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
7 issues found across 24 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="deco-llm/server/main.ts">
<violation number="1" location="deco-llm/server/main.ts:77">
P2: `process.env` is not available in Cloudflare Workers environment. The `WALLET_VENDOR_ID` environment variable will always be undefined, causing the code to always fall back to `"deco"`. Consider accessing environment variables through the Deco runtime's `env` parameter instead.</violation>
</file>
<file name="shared/registry.ts">
<violation number="1" location="shared/registry.ts:41">
P1: `z.object({})` only validates empty objects and strips unknown properties. If arbitrary objects are expected (as the TypeScript types suggest), use `z.record(z.unknown())` or `z.object({}).passthrough()` to preserve the object contents.</violation>
<violation number="2" location="shared/registry.ts:47">
P1: `z.object({})` will strip all properties from the input schema. Use `z.record(z.unknown())` to preserve arbitrary object contents.</violation>
<violation number="3" location="shared/registry.ts:48">
P1: `z.object({})` will strip all properties from the output schema. Use `z.record(z.unknown())` to preserve arbitrary object contents.</violation>
</file>
<file name="deco-llm/server/usage.ts">
<violation number="1" location="deco-llm/server/usage.ts:25">
P1: Unit mismatch: `maxContextLength` is calculated as character count (via `JSON.stringify().length`), but it's multiplied by a per-token price. Characters and tokens are different units. Consider using a tokenizer to estimate token count, or apply a character-to-token ratio approximation (e.g., divide by 4 for rough estimate).</violation>
</file>
<file name="openrouter/server/lib/env.ts">
<violation number="1" location="openrouter/server/lib/env.ts:5">
P2: Error message is misleading after adding env var fallback. Users might not realize they can set `OPENROUTER_API_KEY` as an alternative to providing an authorization header.</violation>
</file>
<file name="openrouter/server/tools/llm-binding.ts">
<violation number="1" location="openrouter/server/tools/llm-binding.ts:508">
P1: Floating promise without error handling. If the `usage` promise rejects (e.g., stream cancelled) or `hook?.end?()` throws, this will cause an unhandled promise rejection. Consider adding a `.catch()` handler.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| name: z.string(), | ||
| description: z.string(), | ||
| inputSchema: z.object({}), | ||
| outputSchema: z.object({}), |
There was a problem hiding this comment.
P1: z.object({}) will strip all properties from the output schema. Use z.record(z.unknown()) to preserve arbitrary object contents.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At shared/registry.ts, line 48:
<comment>`z.object({})` will strip all properties from the output schema. Use `z.record(z.unknown())` to preserve arbitrary object contents.</comment>
<file context>
@@ -0,0 +1,99 @@
+ name: z.string(),
+ description: z.string(),
+ inputSchema: z.object({}),
+ outputSchema: z.object({}),
+ }),
+ ),
</file context>
Suggested change
| outputSchema: z.object({}), | |
| outputSchema: z.record(z.unknown()), |
| z.object({ | ||
| name: z.string(), | ||
| description: z.string(), | ||
| inputSchema: z.object({}), |
There was a problem hiding this comment.
P1: z.object({}) will strip all properties from the input schema. Use z.record(z.unknown()) to preserve arbitrary object contents.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At shared/registry.ts, line 47:
<comment>`z.object({})` will strip all properties from the input schema. Use `z.record(z.unknown())` to preserve arbitrary object contents.</comment>
<file context>
@@ -0,0 +1,99 @@
+ z.object({
+ name: z.string(),
+ description: z.string(),
+ inputSchema: z.object({}),
+ outputSchema: z.object({}),
+ }),
</file context>
Suggested change
| inputSchema: z.object({}), | |
| inputSchema: z.record(z.unknown()), |
| }>; | ||
| }; | ||
| const ConnectionSchema = z.object({ | ||
| configuration_state: z.object({}), |
There was a problem hiding this comment.
P1: z.object({}) only validates empty objects and strips unknown properties. If arbitrary objects are expected (as the TypeScript types suggest), use z.record(z.unknown()) or z.object({}).passthrough() to preserve the object contents.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At shared/registry.ts, line 41:
<comment>`z.object({})` only validates empty objects and strips unknown properties. If arbitrary objects are expected (as the TypeScript types suggest), use `z.record(z.unknown())` or `z.object({}).passthrough()` to preserve the object contents.</comment>
<file context>
@@ -0,0 +1,99 @@
+ }>;
+};
+const ConnectionSchema = z.object({
+ configuration_state: z.object({}),
+ configuration_scopes: z.array(z.string()),
+ tools: z.array(
</file context>
Suggested change
| configuration_state: z.object({}), | |
| configuration_state: z.record(z.unknown()), |
| params: z.infer<typeof LanguageModelInputSchema>, | ||
| ): string => { | ||
| const maxContextLength = Math.min( | ||
| JSON.stringify({ |
There was a problem hiding this comment.
P1: Unit mismatch: maxContextLength is calculated as character count (via JSON.stringify().length), but it's multiplied by a per-token price. Characters and tokens are different units. Consider using a tokenizer to estimate token count, or apply a character-to-token ratio approximation (e.g., divide by 4 for rough estimate).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At deco-llm/server/usage.ts, line 25:
<comment>Unit mismatch: `maxContextLength` is calculated as character count (via `JSON.stringify().length`), but it's multiplied by a per-token price. Characters and tokens are different units. Consider using a tokenizer to estimate token count, or apply a character-to-token ratio approximation (e.g., divide by 4 for rough estimate).</comment>
<file context>
@@ -0,0 +1,44 @@
+ params: z.infer<typeof LanguageModelInputSchema>,
+): string => {
+ const maxContextLength = Math.min(
+ JSON.stringify({
+ ...params.callOptions.prompt,
+ ...params.callOptions.tools,
</file context>
| const [usage, stream] = getUsageFromStream( | ||
| callResponse.stream as ReadableStream<LanguageModelV2StreamPart>, | ||
| ); | ||
| usage.then((usage) => hook?.end?.(usage)); |
There was a problem hiding this comment.
P1: Floating promise without error handling. If the usage promise rejects (e.g., stream cancelled) or hook?.end?() throws, this will cause an unhandled promise rejection. Consider adding a .catch() handler.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At openrouter/server/tools/llm-binding.ts, line 508:
<comment>Floating promise without error handling. If the `usage` promise rejects (e.g., stream cancelled) or `hook?.end?()` throws, this will cause an unhandled promise rejection. Consider adding a `.catch()` handler.</comment>
<file context>
@@ -493,13 +494,18 @@ export const createLLMStreamTool = (env: Env) =>
+ const [usage, stream] = getUsageFromStream(
callResponse.stream as ReadableStream<LanguageModelV2StreamPart>,
);
+ usage.then((usage) => hook?.end?.(usage));
const response = streamToResponse(stream);
</file context>
Suggested change
| usage.then((usage) => hook?.end?.(usage)); | |
| usage.then((usage) => hook?.end?.(usage)).catch(() => {}); |
| if (!isOpenRouterUsageReport(usage)) { | ||
| throw new Error("Usage cost not found"); | ||
| } | ||
| const vendorId = process.env.WALLET_VENDOR_ID ?? "deco"; |
There was a problem hiding this comment.
P2: process.env is not available in Cloudflare Workers environment. The WALLET_VENDOR_ID environment variable will always be undefined, causing the code to always fall back to "deco". Consider accessing environment variables through the Deco runtime's env parameter instead.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At deco-llm/server/main.ts, line 77:
<comment>`process.env` is not available in Cloudflare Workers environment. The `WALLET_VENDOR_ID` environment variable will always be undefined, causing the code to always fall back to `"deco"`. Consider accessing environment variables through the Deco runtime's `env` parameter instead.</comment>
<file context>
@@ -0,0 +1,104 @@
+ if (!isOpenRouterUsageReport(usage)) {
+ throw new Error("Usage cost not found");
+ }
+ const vendorId = process.env.WALLET_VENDOR_ID ?? "deco";
+ await env.MESH_REQUEST_CONTEXT.state.WALLET.COMMIT_PRE_AUTHORIZED_AMOUNT(
+ {
</file context>
| export const getOpenRouterApiKey = (env: Env) => { | ||
| const authorization = env.MESH_REQUEST_CONTEXT.authorization; | ||
| const authorization = | ||
| env.MESH_REQUEST_CONTEXT.authorization ?? process.env.OPENROUTER_API_KEY; |
There was a problem hiding this comment.
P2: Error message is misleading after adding env var fallback. Users might not realize they can set OPENROUTER_API_KEY as an alternative to providing an authorization header.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At openrouter/server/lib/env.ts, line 5:
<comment>Error message is misleading after adding env var fallback. Users might not realize they can set `OPENROUTER_API_KEY` as an alternative to providing an authorization header.</comment>
<file context>
@@ -1,7 +1,8 @@
export const getOpenRouterApiKey = (env: Env) => {
- const authorization = env.MESH_REQUEST_CONTEXT.authorization;
+ const authorization =
+ env.MESH_REQUEST_CONTEXT.authorization ?? process.env.OPENROUTER_API_KEY;
if (!authorization) {
throw new Error("Authorization header is required");
</file context>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary by cubic
Adds the Deco LLM app and binding powered by OpenRouter, with usage-aware billing via Deco Wallet (pre-authorization and commit). Enables metered LLM streaming and generation with centralized registry types.
New Features
Migration
Written for commit 336454a. Summary will update on new commits.