-
-
Notifications
You must be signed in to change notification settings - Fork 88
feat: context aware tools #93
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
this was my initial thought, feel free to suggest enhancement or if there is a better way |
jherr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small API change, otherwise I love it!
|
hey @jherr , thanks for this suggestion ! yes it's much better to make this prop scalable.. |
|
View your CI Pipeline Execution ↗ for commit c818617
☁️ Nx Cloud last updated this comment at |
|
I have 2 questions on my mind here:
|
@tanstack/ai
@tanstack/ai-anthropic
@tanstack/ai-client
@tanstack/ai-devtools-core
@tanstack/ai-gemini
@tanstack/ai-ollama
@tanstack/ai-openai
@tanstack/ai-react
@tanstack/ai-react-ui
@tanstack/ai-solid
@tanstack/ai-solid-ui
@tanstack/react-ai-devtools
@tanstack/solid-ai-devtools
commit: |
hey @AlemTuzlak .. .server() will have server context isomorphic context :'D |
|
@AlemTuzlak , do you think we better also pass current serialisable_only within the request ? |
I think that was bad initial idea, or maybe not and we can make use of it but only serialisable will be sent |
1269b8c to
e89b25a
Compare
|
@jherr @AlemTuzlak anything is missing here 👀 ? would love to fix or enhance if any |
WalkthroughThis change introduces context propagation through the AI tool execution pipeline. A new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/typescript/ai/tests/tool-definition.test.ts (1)
63-71: Consider removingas anytype assertion.The
{ context: undefined as any }assertion works but bypasses type checking. SinceToolOptions<TContext>acceptsunknownby default andcontextis optional, you could simply use{ context: undefined }without the assertion.🔎 Suggested fix
- const result = await serverTool.execute( - { location: 'Paris' }, - { context: undefined as any }, - ) + const result = await serverTool.execute( + { location: 'Paris' }, + { context: undefined }, + ) expect(result).toEqual({ temperature: 72, conditions: 'sunny' }) - expect(executeFn).toHaveBeenCalledWith( - { location: 'Paris' }, - { context: undefined as any }, - ) + expect(executeFn).toHaveBeenCalledWith( + { location: 'Paris' }, + { context: undefined }, + )examples/ts-react-chat/src/routes/api.tanchat.ts (1)
39-45: Unusedoptionsparameter in server tool handler.The
optionsparameter is declared but not used in the handler. If context isn't needed here currently, consider using_optionsto indicate intentional non-use, or remove it until needed.🔎 Suggested change:
-const addToCartToolServer = addToCartToolDef.server((args, options) => ({ +const addToCartToolServer = addToCartToolDef.server((args, _options) => ({ success: true, cartId: 'CART_' + Date.now(), guitarId: args.guitarId, quantity: args.quantity, totalItems: args.quantity, }))packages/typescript/ai-client/tests/chat-client.test.ts (1)
612-624: Simplify the generic type annotation in the mock function.The generic
<TContext = unknown>on the mock function is unusual since the function is immediately specialized forTestContext. Consider simplifying the type annotation.🔎 Suggested simplification:
const executeFn = vi.fn( - async <TContext = unknown>( - _args: any, - options: ToolOptions<TContext>, - ) => { - const ctx = options.context as TestContext + async (_args: any, options: ToolOptions<TestContext>) => { + const ctx = options.context! ctx.localStorage.setItem( `pref_${ctx.userId}_${_args.key}`, _args.value, ) return { success: true } }, )packages/typescript/ai-client/src/chat-client.ts (1)
500-533: Consider addingcontexttoupdateOptionsfor dynamic context updates.The
updateOptionsmethod doesn't currently support updating context. If context needs to change during the client's lifetime (e.g., when user session changes), this would require recreating the client. Consider if this is the intended behavior.packages/typescript/ai/src/tools/tool-definition.ts (1)
197-223: Consider narrowing theas anycast for better type safety.The
execute as anycasts at lines 207 and 221 bypass type checking. While this works correctly at runtime since the execute function passed in has the correct signature, it could mask future type mismatches during refactoring.A slightly safer alternative would be to use a more targeted type assertion that preserves the function shape:
🔎 Suggested improvement
return { __toolSide: 'server', __contextType: undefined as TContext, ...config, - execute: execute as any, + execute: execute as Tool<TInput, TOutput, TName>['execute'], }Similarly for the client method. This maintains the function signature while allowing the TContext variance.
That said, given the complexity of the generic variance involved, the current approach is pragmatic and acceptable for a "Chill" review.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
examples/ts-react-chat/src/routes/api.tanchat.ts(3 hunks)packages/typescript/ai-client/src/chat-client.ts(4 hunks)packages/typescript/ai-client/src/types.ts(2 hunks)packages/typescript/ai-client/tests/chat-client.test.ts(3 hunks)packages/typescript/ai/src/core/chat.ts(5 hunks)packages/typescript/ai/src/tools/tool-calls.ts(7 hunks)packages/typescript/ai/src/tools/tool-definition.ts(7 hunks)packages/typescript/ai/src/types.ts(4 hunks)packages/typescript/ai/tests/ai-chat.test.ts(6 hunks)packages/typescript/ai/tests/tool-call-manager.test.ts(1 hunks)packages/typescript/ai/tests/tool-definition.test.ts(6 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use tree-shakeable adapter architecture for provider implementations - export specialized adapters (text, embedding, summarize, image) as separate imports from/adapterssubpath rather than monolithic adapters
Use Zod for runtime schema validation and type inference, particularly for tool input/output definitions withtoolDefinition()and Zod schema inference
Implement isomorphic tool system usingtoolDefinition()with.server()and.client()implementations for dual-environment execution
Use type-safe per-model configuration with provider options typed based on selected model to ensure compile-time safety
Implement stream processing with StreamProcessor for handling chunked responses and support partial JSON parsing for streaming AI responses
Files:
packages/typescript/ai/tests/tool-call-manager.test.tspackages/typescript/ai/src/tools/tool-calls.tspackages/typescript/ai/tests/ai-chat.test.tspackages/typescript/ai-client/src/chat-client.tspackages/typescript/ai/src/core/chat.tsexamples/ts-react-chat/src/routes/api.tanchat.tspackages/typescript/ai-client/src/types.tspackages/typescript/ai/tests/tool-definition.test.tspackages/typescript/ai-client/tests/chat-client.test.tspackages/typescript/ai/src/types.tspackages/typescript/ai/src/tools/tool-definition.ts
**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Write unit tests using Vitest alongside source files with
.test.tsnaming convention
Files:
packages/typescript/ai/tests/tool-call-manager.test.tspackages/typescript/ai/tests/ai-chat.test.tspackages/typescript/ai/tests/tool-definition.test.tspackages/typescript/ai-client/tests/chat-client.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for function and variable names throughout the codebase
Files:
packages/typescript/ai/tests/tool-call-manager.test.tspackages/typescript/ai/src/tools/tool-calls.tspackages/typescript/ai/tests/ai-chat.test.tspackages/typescript/ai-client/src/chat-client.tspackages/typescript/ai/src/core/chat.tsexamples/ts-react-chat/src/routes/api.tanchat.tspackages/typescript/ai-client/src/types.tspackages/typescript/ai/tests/tool-definition.test.tspackages/typescript/ai-client/tests/chat-client.test.tspackages/typescript/ai/src/types.tspackages/typescript/ai/src/tools/tool-definition.ts
examples/**
📄 CodeRabbit inference engine (CLAUDE.md)
Examples are not built by Nx and should be run independently from their directories with
pnpm devorpnpm install && pnpm dev
Files:
examples/ts-react-chat/src/routes/api.tanchat.ts
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: TanStack/ai PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T17:09:09.794Z
Learning: Implement framework integrations using the headless `tanstack/ai-client` for state management with framework-specific hooks (useChat) on top
📚 Learning: 2025-12-13T17:09:09.794Z
Learnt from: CR
Repo: TanStack/ai PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T17:09:09.794Z
Learning: Applies to **/*.{ts,tsx} : Implement isomorphic tool system using `toolDefinition()` with `.server()` and `.client()` implementations for dual-environment execution
Applied to files:
packages/typescript/ai/tests/tool-call-manager.test.tspackages/typescript/ai/src/tools/tool-calls.tspackages/typescript/ai/tests/ai-chat.test.tspackages/typescript/ai-client/src/chat-client.tspackages/typescript/ai/tests/tool-definition.test.tspackages/typescript/ai-client/tests/chat-client.test.tspackages/typescript/ai/src/types.tspackages/typescript/ai/src/tools/tool-definition.ts
📚 Learning: 2025-12-13T17:09:09.794Z
Learnt from: CR
Repo: TanStack/ai PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T17:09:09.794Z
Learning: Applies to **/*.{ts,tsx} : Use Zod for runtime schema validation and type inference, particularly for tool input/output definitions with `toolDefinition()` and Zod schema inference
Applied to files:
packages/typescript/ai/src/tools/tool-calls.tspackages/typescript/ai/tests/tool-definition.test.tspackages/typescript/ai/src/types.tspackages/typescript/ai/src/tools/tool-definition.ts
📚 Learning: 2025-12-13T17:09:09.794Z
Learnt from: CR
Repo: TanStack/ai PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T17:09:09.794Z
Learning: Applies to packages/typescript/*/src/adapters/*.ts : Create individual adapter implementations for each provider capability (text, embed, summarize, image) with separate exports to enable tree-shaking
Applied to files:
packages/typescript/ai/tests/ai-chat.test.ts
📚 Learning: 2025-12-13T17:09:09.794Z
Learnt from: CR
Repo: TanStack/ai PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T17:09:09.794Z
Learning: Applies to **/*.{ts,tsx} : Use tree-shakeable adapter architecture for provider implementations - export specialized adapters (text, embedding, summarize, image) as separate imports from `/adapters` subpath rather than monolithic adapters
Applied to files:
packages/typescript/ai/tests/ai-chat.test.ts
📚 Learning: 2025-12-13T17:09:09.794Z
Learnt from: CR
Repo: TanStack/ai PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T17:09:09.794Z
Learning: Applies to **/*.{ts,tsx} : Implement stream processing with StreamProcessor for handling chunked responses and support partial JSON parsing for streaming AI responses
Applied to files:
packages/typescript/ai/tests/ai-chat.test.tspackages/typescript/ai/src/types.ts
📚 Learning: 2025-12-13T17:09:09.794Z
Learnt from: CR
Repo: TanStack/ai PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T17:09:09.794Z
Learning: Implement framework integrations using the headless `tanstack/ai-client` for state management with framework-specific hooks (useChat) on top
Applied to files:
packages/typescript/ai-client/src/chat-client.ts
📚 Learning: 2025-12-13T17:09:09.794Z
Learnt from: CR
Repo: TanStack/ai PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T17:09:09.794Z
Learning: Applies to **/*.test.ts : Write unit tests using Vitest alongside source files with `.test.ts` naming convention
Applied to files:
packages/typescript/ai/tests/tool-definition.test.tspackages/typescript/ai-client/tests/chat-client.test.ts
📚 Learning: 2025-12-13T17:09:09.794Z
Learnt from: CR
Repo: TanStack/ai PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T17:09:09.794Z
Learning: Applies to **/*.{ts,tsx} : Use type-safe per-model configuration with provider options typed based on selected model to ensure compile-time safety
Applied to files:
packages/typescript/ai/src/types.ts
📚 Learning: 2025-12-13T17:09:09.794Z
Learnt from: CR
Repo: TanStack/ai PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T17:09:09.794Z
Learning: Applies to packages/typescript/*/src/model-meta.ts : Maintain model metadata files that define provider options and capabilities per model for per-model type safety
Applied to files:
packages/typescript/ai/src/types.ts
🧬 Code graph analysis (7)
packages/typescript/ai/src/tools/tool-calls.ts (1)
packages/typescript/ai/src/types.ts (3)
ToolOptions(75-77)ToolCall(62-69)Tool(328-434)
packages/typescript/ai/tests/ai-chat.test.ts (1)
packages/typescript/ai/src/types.ts (3)
ToolOptions(75-77)Tool(328-434)ChatOptions(561-619)
packages/typescript/ai-client/src/chat-client.ts (3)
packages/typescript/ai/src/tools/tool-definition.ts (1)
AnyClientTool(60-62)packages/typescript/ai/src/types.ts (1)
ToolOptions(75-77)packages/typescript/ai-client/src/types.ts (1)
ChatClientOptions(134-230)
packages/typescript/ai/src/core/chat.ts (1)
packages/typescript/ai/src/types.ts (1)
ToolOptions(75-77)
examples/ts-react-chat/src/routes/api.tanchat.ts (2)
examples/ts-solid-chat/src/lib/guitar-tools.ts (1)
addToCartToolServer(109-117)examples/ts-react-chat/src/lib/guitar-tools.ts (1)
addToCartToolDef(69-84)
packages/typescript/ai/tests/tool-definition.test.ts (1)
packages/typescript/ai/src/tools/tool-definition.ts (1)
toolDefinition(187-227)
packages/typescript/ai/src/tools/tool-definition.ts (1)
packages/typescript/ai/src/types.ts (3)
Tool(328-434)ToolOptions(75-77)InferSchemaType(60-60)
🔇 Additional comments (35)
packages/typescript/ai/src/types.ts (3)
71-77: Well-designed ToolOptions interface.The
ToolOptions<TContext>interface provides a clean, extensible pattern for passing execution context to tools. Using an options object rather than a positional parameter makes this backward-compatible and allows for future additions without breaking changes.
424-427: Execute signature change aligns with context propagation.The updated signature
<TContext = unknown>(args, options: ToolOptions<TContext>)cleanly threads context through tool execution while remaining backward compatible sincecontextis optional inToolOptions.
596-618: Excellent documentation for the context property.The
contextproperty onChatOptionsis well-documented with a practical example showing database access and user ID retrieval. This clearly communicates the intended use case for context-aware tools.packages/typescript/ai/tests/tool-call-manager.test.ts (1)
124-127: Test expectation correctly updated for new execute signature.The assertion now expects the second argument
{ context: undefined }, which accurately reflects the updated tool execution flow where context is always passed (asundefinedwhen not provided).packages/typescript/ai/tests/ai-chat.test.ts (7)
8-14: Import update aligns with new type usage.Adding
ToolOptionsto the imports is necessary for the new context support tests.
461-464: Test expectation correctly reflects context propagation.The updated assertion verifies that tool.execute receives the expected arguments including the options object with
context: undefinedwhen no context is provided.
571-574: Consistent update for streaming tool arguments test.
1504-1507: Approval flow test correctly updated.The pending tool execution with approval now properly expects the context option.
2619-2622: Approval response extraction test correctly updated.
3020-3166: Comprehensive context propagation test.This test effectively validates:
- Context is passed through to
tool.execute- Context properties (like
db) are accessible within the tool- Mock interactions work correctly with context
The test structure with a mock database is a good pattern for verifying context-aware behavior.
3168-3249: Good coverage for optional context scenario.This test verifies backward compatibility by ensuring tools work correctly when no context is provided. The assertion
{ context: undefined }confirms the options object is still passed.packages/typescript/ai/tests/tool-definition.test.ts (2)
1-1: Minor import reordering.Import order change is a stylistic adjustment with no functional impact.
277-338: Well-structured context support test.The test properly validates:
- Typed context interface (
TestContext)- Context propagation to execute function
- Mock verification for database calls
- Proper use of
tool.server<TestContext>(executeFn)for type-safe contextpackages/typescript/ai-client/src/types.ts (2)
134-136: TContext generic properly added to ChatClientOptions.The generic parameter enables type-safe context in client-side chat options while defaulting to
unknownfor backward compatibility.
212-229: Clear documentation distinguishing client vs server context.The JSDoc clearly explains that this
contextis for client-side tools only, and that server tools should receive their context from the server-sidechat()function. This distinction helps prevent confusion about context boundaries.examples/ts-react-chat/src/routes/api.tanchat.ts (1)
100-106: LGTM!The
serverContextscaffolding provides a clear extension point for adding server-side resources (db connections, user sessions) in the future. The context is correctly passed to thechat()function.Also applies to: 122-122
packages/typescript/ai-client/tests/chat-client.test.ts (3)
1-12: LGTM!Imports are correctly added for the new test dependencies:
toolDefinitionandzfor creating test tools, andToolOptionstype for type annotations.
670-694: LGTM!This test is important - it validates that context (which may contain non-serializable objects like
localStorage) is correctly kept client-side only and not sent over the wire. This addresses the concern about serializing complex objects.
696-731: LGTM!Good backward compatibility test - verifies that tools work correctly when no context is provided, with
context: undefinedpassed to the execute function.packages/typescript/ai/src/core/chat.ts (3)
19-19: LGTM!The
ToolOptionsimport and the new private fieldoptionsare correctly typed usingTParams['context']to preserve the context type from the chat parameters.Also applies to: 49-49
80-80: LGTM!Context is correctly extracted from configuration params and stored for propagation to tool executions.
382-388: LGTM!Context is consistently propagated through both tool execution paths (
checkForPendingToolCallsandprocessToolCalls), ensuring all server-side tool executions receive the context.Also applies to: 451-457
packages/typescript/ai-client/src/chat-client.ts (3)
22-25: LGTM!The
ChatClientclass is now properly generic overTToolsandTContext, enabling type-safe context propagation. The privateoptionsfield correctly stores the context for tool execution.Also applies to: 37-37
51-54: LGTM!Constructor correctly initializes the context from options, matching the updated
ChatClientOptions<TTools, TContext>interface.
148-150: LGTM!Context is correctly passed to client tool execution as the second argument, following the
ToolOptionsinterface pattern.packages/typescript/ai/src/tools/tool-calls.ts (5)
7-7: LGTM!The
ToolOptionsimport and updatedexecuteToolssignature correctly add optional context support with a sensible default of{}for backward compatibility.Also applies to: 122-125
156-159: LGTM!Context is correctly propagated to tool execution within
ToolCallManager.executeTools.
260-268: LGTM!The
executeToolCallsfunction signature is correctly updated with theoptionsparameter and proper JSDoc documentation explaining its purpose.
399-401: LGTM!Context is correctly passed to approved server tool execution, maintaining consistency across all execution paths.
464-466: LGTM!Context propagation is complete for all server tool execution paths - normal execution, approved execution, and via
ToolCallManager.packages/typescript/ai/src/tools/tool-definition.ts (5)
2-8: LGTM!The imports are correctly structured as type-only imports, and
ToolOptionsis properly imported to support the new context-aware execute signatures.
13-21: LGTM!The phantom type
__contextTypepattern is a valid TypeScript technique for carrying type information without runtime overhead. The optional modifier is appropriate since this is purely for type inference.
26-44: LGTM!The
ClientToolinterface correctly adds theTContextgeneric parameter and updates theexecutesignature to acceptToolOptions<TContext>. The phantom type pattern matchesServerTool.
60-62: LGTM!The
AnyClientTooltype correctly adds the fourth generic parameter to match the updatedClientToolinterface.
118-133: LGTM!The method-level generic
TContextallows callers to specify the context type when instantiating tools (e.g.,tool.server<{userId: string}>(...)), providing type-safe access tooptions.context. This addresses the previous review feedback about using anoptionsargument for scalability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/typescript/ai-client/src/chat-client.ts (1)
528-561: Consider adding context to updateOptions.The
updateOptionsmethod allows updating various client options dynamically (connection, body, tools, callbacks), but doesn't include context. If context needs to remain stable throughout the client's lifetime, this is fine. However, if there are use cases for updating context dynamically, consider adding it here for consistency.🔎 Possible enhancement
updateOptions(options: { connection?: ConnectionAdapter body?: Record<string, any> tools?: ReadonlyArray<AnyClientTool> + context?: TContext onResponse?: (response?: Response) => void | Promise<void> onChunk?: (chunk: StreamChunk) => void onFinish?: (message: UIMessage) => void onError?: (error: Error) => void }): void { if (options.connection !== undefined) { this.connection = options.connection } if (options.body !== undefined) { this.body = options.body } + if (options.context !== undefined) { + this.options = { context: options.context } + } if (options.tools !== undefined) { this.clientToolsRef.current = new Map() for (const tool of options.tools) { this.clientToolsRef.current.set(tool.name, tool) } } // ... rest of updates }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
examples/ts-react-chat/src/routes/api.tanchat.ts(2 hunks)packages/typescript/ai-client/src/chat-client.ts(4 hunks)packages/typescript/ai-client/src/types.ts(2 hunks)packages/typescript/ai-client/tests/chat-client.test.ts(3 hunks)packages/typescript/ai/src/activities/chat/index.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/typescript/ai-client/tests/chat-client.test.ts
- examples/ts-react-chat/src/routes/api.tanchat.ts
- packages/typescript/ai-client/src/types.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use tree-shakeable adapter architecture for provider implementations - export specialized adapters (text, embedding, summarize, image) as separate imports from/adapterssubpath rather than monolithic adapters
Use Zod for runtime schema validation and type inference, particularly for tool input/output definitions withtoolDefinition()and Zod schema inference
Implement isomorphic tool system usingtoolDefinition()with.server()and.client()implementations for dual-environment execution
Use type-safe per-model configuration with provider options typed based on selected model to ensure compile-time safety
Implement stream processing with StreamProcessor for handling chunked responses and support partial JSON parsing for streaming AI responses
Files:
packages/typescript/ai-client/src/chat-client.tspackages/typescript/ai/src/activities/chat/index.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for function and variable names throughout the codebase
Files:
packages/typescript/ai-client/src/chat-client.tspackages/typescript/ai/src/activities/chat/index.ts
🧠 Learnings (2)
📚 Learning: 2025-12-13T17:09:09.794Z
Learnt from: CR
Repo: TanStack/ai PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T17:09:09.794Z
Learning: Applies to **/*.{ts,tsx} : Implement isomorphic tool system using `toolDefinition()` with `.server()` and `.client()` implementations for dual-environment execution
Applied to files:
packages/typescript/ai-client/src/chat-client.tspackages/typescript/ai/src/activities/chat/index.ts
📚 Learning: 2025-12-13T17:09:09.794Z
Learnt from: CR
Repo: TanStack/ai PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T17:09:09.794Z
Learning: Implement framework integrations using the headless `tanstack/ai-client` for state management with framework-specific hooks (useChat) on top
Applied to files:
packages/typescript/ai-client/src/chat-client.ts
🧬 Code graph analysis (1)
packages/typescript/ai-client/src/chat-client.ts (2)
packages/typescript/ai/src/types.ts (1)
ToolOptions(100-102)packages/typescript/ai-client/src/types.ts (1)
ChatClientOptions(134-230)
🔇 Additional comments (8)
packages/typescript/ai/src/activities/chat/index.ts (4)
34-34: LGTM: Import addition is correct.The
ToolOptionsimport is necessary for typing the context-aware tool execution mechanism.
209-209: LGTM: Field declaration is well-typed.The
optionsfield correctly usesPartial<ToolOptions<TParams['context']>>to enable optional context passing to tools, with proper generic type threading.
240-240: LGTM: Context initialization is correct.The context is properly extracted from
config.params.contextand wrapped in the expectedToolOptionsshape for tool execution.
585-585: LGTM: Context propagation is consistent.Both invocations of
executeToolCallscorrectly passthis.options, ensuring context is available during tool execution in both the pending tool calls path and the new tool calls path.Also applies to: 654-654
packages/typescript/ai-client/src/chat-client.ts (4)
13-18: LGTM: Import updates support context-aware tooling.The addition of
ToolOptionsand consolidation of imports from@tanstack/aiproperly support the new generic context mechanism.
22-25: LGTM: Generic parameters enable type-safe context.The addition of
TToolsandTContextgenerics properly enables type-safe context propagation while maintaining backwards compatibility with default types.
149-151: Context propagation to client tools looks correct.The pattern of passing
{ context: this.options.context }toclientTool.executeis correct for context-aware client-side tool execution. However, this depends on the fix at line 55 to properly initializethis.options.
38-38: Context is intentionally local to the client and never serialized.The context parameter is only used for client-side tool execution and is explicitly excluded from the server request body. This design ensures context containing non-serializable objects (like
localStorage, DOM nodes, or functions) cannot cause issues since it never crosses the network boundary.No runtime validation, TypeScript constraints, or documentation changes are needed—this behavior is already by design and tested.
Likely an incorrect or invalid review comment.
| private currentStreamId: string | null = null | ||
| private currentMessageId: string | null = null | ||
| private postStreamActions: Array<() => Promise<void>> = [] | ||
| private options: Partial<ToolOptions<TContext>> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix type mismatch in options assignment.
There is a type mismatch at line 55. The field this.options is typed as Partial<ToolOptions<TContext>> (which only has a context property), but the assignment this.options = options attempts to assign ChatClientOptions<TTools, TContext> (which has many properties like connection, body, tools, callbacks, etc.).
This should follow the same pattern used in packages/typescript/ai/src/activities/chat/index.ts line 240.
🔎 Proposed fix
- this.options = options
+ this.options = { context: options.context }Also applies to: 52-52, 55-55
🤖 Prompt for AI Agents
In packages/typescript/ai-client/src/chat-client.ts around lines 38, 52 and 55,
the field this.options is declared as Partial<ToolOptions<TContext>> but later
assigned a ChatClientOptions<TTools, TContext> object, causing a type mismatch;
change the declaration of this.options to ChatClientOptions<TTools, TContext>
(or the appropriate partial of that exact type used in
packages/typescript/ai/src/activities/chat/index.ts line 240) so it matches the
shape being assigned, and update any related usages or imports accordingly to
keep types consistent.
|
Hey Ahmed, thank you for your patience, we will look at this this week hopefully! We were busy with the architecture changes |
|
@AlemTuzlak no worries, I think I broke it when I tried to resolve conflicts, I will fix that |
feat: introduce ToolOptions and update tool execution context fix: update context handling in ChatClient and API route
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/typescript/ai-client/src/chat-client.ts (1)
518-551: Consider addingcontexttoupdateOptionsfor runtime updates.The
updateOptionsmethod allows updatingconnection,body, andtoolsbut doesn't includecontext. If use cases require updating context after initialization (e.g., when user auth state changes), this could be limiting.🔎 Proposed addition
updateOptions(options: { connection?: ConnectionAdapter body?: Record<string, any> + context?: unknown tools?: ReadonlyArray<AnyClientTool> onResponse?: (response?: Response) => void | Promise<void> onChunk?: (chunk: StreamChunk) => void onFinish?: (message: UIMessage) => void onError?: (error: Error) => void }): void { if (options.connection !== undefined) { this.connection = options.connection } if (options.body !== undefined) { this.body = options.body } + if (options.context !== undefined) { + this.context = options.context + }packages/typescript/ai/src/activities/chat/tools/tool-definition.ts (1)
192-217: Minor formatting inconsistency.Line 192 has a space before the generic parameter (
server <TContext) while line 205 does not (client<TContext). Consider aligning the style for consistency.The context propagation logic is correct—the casts to
ToolOptions<TContext>appropriately preserve type safety for the user's execute callbacks.🔎 Suggested fix for consistent formatting
- server <TContext extends unknown>( + server<TContext extends unknown>(
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
packages/typescript/ai-client/src/chat-client.ts(3 hunks)packages/typescript/ai-client/src/types.ts(1 hunks)packages/typescript/ai/src/activities/chat/index.ts(4 hunks)packages/typescript/ai/src/activities/chat/tools/tool-calls.ts(7 hunks)packages/typescript/ai/src/activities/chat/tools/tool-definition.ts(4 hunks)packages/typescript/ai/src/types.ts(4 hunks)packages/typescript/ai/tests/ai-text.test.ts(9 hunks)packages/typescript/ai/tests/tool-call-manager.test.ts(3 hunks)packages/typescript/ai/tests/tool-definition.test.ts(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/typescript/ai/src/types.ts
- packages/typescript/ai/src/activities/chat/index.ts
- packages/typescript/ai-client/src/types.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use tree-shakeable adapter architecture for provider implementations - export specialized adapters (text, embedding, summarize, image) as separate imports from/adapterssubpath rather than monolithic adapters
Use Zod for runtime schema validation and type inference, particularly for tool input/output definitions withtoolDefinition()and Zod schema inference
Implement isomorphic tool system usingtoolDefinition()with.server()and.client()implementations for dual-environment execution
Use type-safe per-model configuration with provider options typed based on selected model to ensure compile-time safety
Implement stream processing with StreamProcessor for handling chunked responses and support partial JSON parsing for streaming AI responses
Files:
packages/typescript/ai/tests/tool-definition.test.tspackages/typescript/ai/src/activities/chat/tools/tool-calls.tspackages/typescript/ai/tests/ai-text.test.tspackages/typescript/ai-client/src/chat-client.tspackages/typescript/ai/src/activities/chat/tools/tool-definition.tspackages/typescript/ai/tests/tool-call-manager.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Write unit tests using Vitest alongside source files with
.test.tsnaming convention
Files:
packages/typescript/ai/tests/tool-definition.test.tspackages/typescript/ai/tests/ai-text.test.tspackages/typescript/ai/tests/tool-call-manager.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for function and variable names throughout the codebase
Files:
packages/typescript/ai/tests/tool-definition.test.tspackages/typescript/ai/src/activities/chat/tools/tool-calls.tspackages/typescript/ai/tests/ai-text.test.tspackages/typescript/ai-client/src/chat-client.tspackages/typescript/ai/src/activities/chat/tools/tool-definition.tspackages/typescript/ai/tests/tool-call-manager.test.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: TanStack/ai PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T17:09:09.794Z
Learning: Applies to **/*.{ts,tsx} : Implement isomorphic tool system using `toolDefinition()` with `.server()` and `.client()` implementations for dual-environment execution
📚 Learning: 2025-12-13T17:09:09.794Z
Learnt from: CR
Repo: TanStack/ai PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T17:09:09.794Z
Learning: Applies to **/*.{ts,tsx} : Implement isomorphic tool system using `toolDefinition()` with `.server()` and `.client()` implementations for dual-environment execution
Applied to files:
packages/typescript/ai/tests/tool-definition.test.tspackages/typescript/ai/src/activities/chat/tools/tool-calls.tspackages/typescript/ai/tests/ai-text.test.tspackages/typescript/ai-client/src/chat-client.tspackages/typescript/ai/src/activities/chat/tools/tool-definition.tspackages/typescript/ai/tests/tool-call-manager.test.ts
📚 Learning: 2025-12-13T17:09:09.794Z
Learnt from: CR
Repo: TanStack/ai PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T17:09:09.794Z
Learning: Applies to **/*.test.ts : Write unit tests using Vitest alongside source files with `.test.ts` naming convention
Applied to files:
packages/typescript/ai/tests/tool-definition.test.ts
📚 Learning: 2025-12-13T17:09:09.794Z
Learnt from: CR
Repo: TanStack/ai PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T17:09:09.794Z
Learning: Applies to **/*.{ts,tsx} : Use Zod for runtime schema validation and type inference, particularly for tool input/output definitions with `toolDefinition()` and Zod schema inference
Applied to files:
packages/typescript/ai/tests/tool-definition.test.tspackages/typescript/ai/src/activities/chat/tools/tool-calls.tspackages/typescript/ai/src/activities/chat/tools/tool-definition.ts
📚 Learning: 2025-12-13T17:09:09.794Z
Learnt from: CR
Repo: TanStack/ai PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T17:09:09.794Z
Learning: Applies to **/*.{ts,tsx} : Implement stream processing with StreamProcessor for handling chunked responses and support partial JSON parsing for streaming AI responses
Applied to files:
packages/typescript/ai/tests/ai-text.test.ts
🧬 Code graph analysis (5)
packages/typescript/ai/tests/tool-definition.test.ts (2)
packages/typescript/ai/src/activities/chat/tools/tool-definition.ts (1)
toolDefinition(182-222)packages/typescript/ai/src/types.ts (1)
ToolOptions(91-94)
packages/typescript/ai/src/activities/chat/tools/tool-calls.ts (1)
packages/typescript/ai/src/types.ts (1)
ToolOptions(91-94)
packages/typescript/ai/tests/ai-text.test.ts (2)
packages/python/tanstack-ai/src/tanstack_ai/tool_utils.py (1)
tool(12-65)packages/typescript/ai/src/types.ts (1)
Tool(337-449)
packages/typescript/ai/src/activities/chat/tools/tool-definition.ts (2)
packages/typescript/ai/src/types.ts (3)
Tool(337-449)InferSchemaType(84-85)ToolOptions(91-94)packages/typescript/ai/src/index.ts (2)
ServerTool(43-43)ClientTool(44-44)
packages/typescript/ai/tests/tool-call-manager.test.ts (1)
packages/typescript/ai/src/types.ts (1)
Tool(337-449)
🪛 Biome (2.1.2)
packages/typescript/ai/tests/tool-call-manager.test.ts
[error] 219-219: eval() exposes to security risks and performance issues.
See the MDN web docs for more details.
Refactor the code so that it doesn't need to call eval().
(lint/security/noGlobalEval)
🔇 Additional comments (19)
packages/typescript/ai/tests/tool-call-manager.test.ts (1)
21-24: LGTM! Test mocks correctly updated for context-aware tools.The mock function signature and assertion updates properly verify that the
ToolCallManagerpasses the{ context: undefined }options object to tool execute functions. This aligns with the newToolOptionsinterface.Also applies to: 124-124
packages/typescript/ai-client/src/chat-client.ts (2)
22-22: LGTM! Context field properly initialized and stored.The
contextfield is correctly added and initialized fromoptions.contextin the constructor, enabling context-aware client tool execution.Also applies to: 47-47
139-141: Context correctly passed to client tool execution.The context is properly propagated to client-side tools via
{ context: this.context }, aligning with theToolOptionsinterface pattern used across the codebase.packages/typescript/ai/tests/ai-text.test.ts (3)
383-386: LGTM! Tool mocks correctly updated for context-aware execution.The mock signatures and assertions consistently verify that
{ context: undefined }is passed as the second argument to tool execute functions, matching the implementation intool-calls.ts.Also applies to: 448-448
472-474: Consistent context propagation verification across tool tests.All tool execute mocks appropriately accept the optional
_optionsparameter and assertions verify the context object is passed correctly.Also applies to: 554-554, 564-564, 571-571, 662-662
1472-1472: Approval and temperature tool tests properly updated.The approval-required tool and temperature tool tests correctly verify context propagation with
{ context: undefined }.Also applies to: 2561-2561, 2825-2827
packages/typescript/ai/tests/tool-definition.test.ts (5)
1-4: LGTM! Imports properly updated for context-aware testing.The
ToolOptionstype import enables proper typing of the context parameter in the new tests.
50-66: Execute function mocks and assertions correctly updated.Both server and client tool tests properly pass
{ context: undefined }and verify the mock was called with the expected arguments.Also applies to: 83-96
260-293: Excellent coverage: Server tool context propagation test.This test verifies that server tools receive and can access context via
options.context. The test correctly demonstrates reading values from the context object. As per coding guidelines, this implements the isomorphic tool system with.server()for server context.
295-328: Excellent coverage: Client tool context propagation test.Parallel test for client tools, ensuring
.client()implementations also receive context properly. This aligns with the isomorphic tool system guidelines.
330-360: Important edge case: Missing context handled gracefully.This test ensures tools don't crash when context is empty or undefined. The implementation correctly checks
options?.context?.testData !== undefinedbefore accessing values.packages/typescript/ai/src/activities/chat/tools/tool-calls.ts (5)
7-9: LGTM! ToolOptions type correctly imported.The import enables type-safe context propagation throughout the tool execution pipeline.
113-116: LGTM! executeTools method signature updated for context.The optional
optionsparameter with a default empty object ensures backward compatibility while enabling context propagation.
152-154: Context correctly passed during tool execution.The context is extracted from options and passed to
tool.execute, enabling tools to access shared context data during execution.
262-268: LGTM! executeToolCalls properly propagates context.The function accepts an optional
optionsparameter and constructs atoolOptionsobject that is shared across all execution paths. This ensures consistent context handling regardless of tool type.Also applies to: 327-329
407-407: Context consistently passed in all tool execution paths.Both the approval-required path (line 407) and normal server tool path (line 465) correctly pass
toolOptionstotool.execute, ensuring context is available in all scenarios.Also applies to: 465-465
packages/typescript/ai/src/activities/chat/tools/tool-definition.ts (3)
1-8: LGTM!The import of
ToolOptionsis correctly added to support the new context-aware execute signatures throughout the file.
24-35: LGTM!The
TContextgeneric addition toClientToolcorrectly enables type-safe context access in the execute callback while maintaining backward compatibility through theunknowndefault.
109-124: Well-designed context-aware API.The generic
TContextparameter onserverandclientmethods enables the typing approach discussed in PR comments:toolDef.server<{ userId: string }>()correctly typesoptions.context.userIdwithin the execute callback. This aligns with the isomorphic tool system pattern from the coding guidelines.
| execute: vi.fn((args: any, _options?: any) => { | ||
| return JSON.stringify({ result: eval(args.expression) }) | ||
| }), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid eval() even in test code.
Static analysis correctly flags this as a security risk. While this is test-only code, using eval() sets a bad precedent and could mask injection vulnerabilities if test inputs become dynamic.
🔎 Proposed fix using a simple calculation instead
- execute: vi.fn((args: any, _options?: any) => {
- return JSON.stringify({ result: eval(args.expression) })
- }),
+ execute: vi.fn((args: any, _options?: any) => {
+ // Simple fixed calculation for test purposes
+ const result = args.expression === '5+3' ? 8 : 0
+ return JSON.stringify({ result })
+ }),📝 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.
| execute: vi.fn((args: any, _options?: any) => { | |
| return JSON.stringify({ result: eval(args.expression) }) | |
| }), | |
| execute: vi.fn((args: any, _options?: any) => { | |
| // Simple fixed calculation for test purposes | |
| const result = args.expression === '5+3' ? 8 : 0 | |
| return JSON.stringify({ result }) | |
| }), |
🧰 Tools
🪛 Biome (2.1.2)
[error] 219-219: eval() exposes to security risks and performance issues.
See the MDN web docs for more details.
Refactor the code so that it doesn't need to call eval().
(lint/security/noGlobalEval)
🤖 Prompt for AI Agents
packages/typescript/ai/tests/tool-call-manager.test.ts around lines 218 to 220:
the test currently uses eval(args.expression) which is a security risk; replace
it with a safe, deterministic evaluator — validate that args.expression contains
only digits, whitespace and arithmetic operators (e.g. /^[0-9+\-*/().\s]+$/) and
then compute the result using a restricted evaluator (for example a small parser
or a validated new Function call) or by implementing a simple arithmetic parser
for +,-,*,/ so no arbitrary code can execute; return JSON.stringify({ result:
calculatedValue }) instead of using eval.
|
Fixed |
🎯 Changes
Supporting context in tool execution..
to make tools behaviour context aware, the only way is to reconstruct them everytime you want to invoke a new trigger..
this PR suggests supporting context.
✅ Checklist
pnpm run test:pr.pnpm run test:lib.🚀 Release Impact
Summary by CodeRabbit
Release Notes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.