feat: move sampling options (temperature/topP/maxTokens) into modelOptions#660
Open
AlemTuzlak wants to merge 18 commits into
Open
feat: move sampling options (temperature/topP/maxTokens) into modelOptions#660AlemTuzlak wants to merge 18 commits into
AlemTuzlak wants to merge 18 commits into
Conversation
…t-completions base
…cast and flat root reads
…g attribute spellings
Contributor
📝 WalkthroughWalkthroughThis PR executes a breaking change removing root-level sampling options ( ChangesSampling Options Migration to modelOptions
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Contributor
🚀 Changeset Version Preview9 package(s) bumped directly, 21 bumped as dependents. 🟥 Major bumps
🟨 Minor bumps
🟩 Patch bumps
|
|
View your CI Pipeline Execution ↗ for commit 4e8afb8
☁️ Nx Cloud last updated this comment at |
@tanstack/ai
@tanstack/ai-anthropic
@tanstack/ai-client
@tanstack/ai-code-mode
@tanstack/ai-code-mode-skills
@tanstack/ai-devtools-core
@tanstack/ai-elevenlabs
@tanstack/ai-event-client
@tanstack/ai-fal
@tanstack/ai-gemini
@tanstack/ai-grok
@tanstack/ai-groq
@tanstack/ai-isolate-cloudflare
@tanstack/ai-isolate-node
@tanstack/ai-isolate-quickjs
@tanstack/ai-ollama
@tanstack/ai-openai
@tanstack/ai-openrouter
@tanstack/ai-preact
@tanstack/ai-react
@tanstack/ai-react-ui
@tanstack/ai-solid
@tanstack/ai-solid-ui
@tanstack/ai-svelte
@tanstack/ai-utils
@tanstack/ai-vue
@tanstack/ai-vue-ui
@tanstack/openai-base
@tanstack/preact-ai-devtools
@tanstack/react-ai-devtools
@tanstack/solid-ai-devtools
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ai-ollama/src/adapters/text.ts (1)
36-39:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNarrow the fallback
modelOptionstype to the subset this adapter actually forwards.For arbitrary model strings,
ResolveModelOptionsfalls back to the fullollamaChatRequest, butmapCommonOptionsToOllama()only readsmodelOptions.options(and thenformat,keep_alive,logprobs,top_logprobs, andthinkwhen present). It sourcesmodel/messagesfromoptions.model/options.messagesand convertstoolsonly fromoptions.tools, so request-level keys likemodel,messages,stream, andtoolstyped via the fallback can be silently ignored at runtime.type ResolveModelOptions<TModel extends string> = TModel extends keyof OllamaChatModelOptionsByName ? OllamaChatModelOptionsByName[TModel] : ChatRequest🤖 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 `@packages/ai-ollama/src/adapters/text.ts` around lines 36 - 39, The fallback for ResolveModelOptions is too broad (falls back to ChatRequest) even though mapCommonOptionsToOllama only forwards specific fields; change the fallback to a narrowed type that only includes the fields actually forwarded (e.g., an object with an optional options property limited to Pick<ChatRequest['options'], 'format'|'keep_alive'|'logprobs'|'top_logprobs'|'think'> and an optional tools property of ChatRequest['tools']) so ResolveModelOptions<TModel> returns either OllamaChatModelOptionsByName[TModel] or that minimal subset; update the type alias ResolveModelOptions accordingly so mapCommonOptionsToOllama and related code use the correct, narrower modelOptions shape.
🧹 Nitpick comments (4)
codemods/move-sampling-to-model-options/README.md (1)
18-23: ⚡ Quick winClarify OpenAI adapter split in the mapping table.
The table currently presents a single OpenAI mapping to
max_output_tokens, which can confuseopenaiChatCompletionsusers (they needmax_tokens). Please add an explicit note or split rows for OpenAI Responses vs Chat Completions.🤖 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 `@codemods/move-sampling-to-model-options/README.md` around lines 18 - 23, The mapping table currently lists a single OpenAI entry mapping `maxTokens` to `max_output_tokens`, which is ambiguous for users of `openaiChatCompletions`; update the table to split the OpenAI row into two rows (e.g., "openai (Responses)" and "openai (Chat Completions)") and set `maxTokens` -> `max_output_tokens` for the Responses row and `maxTokens` -> `max_tokens` for the Chat Completions row; mention the `openaiChatCompletions` identifier in the note so readers know which row to use.packages/ai/tests/summarize-max-length.test.ts (1)
1-139: ⚡ Quick winPlace this unit test alongside the summarize source file.
This new suite lives under
packages/ai/tests/, but the repo convention is to colocate*.test.tsfiles with the source they cover. Moving it next topackages/ai/src/activities/summarize/chat-stream-summarize.tswill keep the summarize contract and its regression coverage together.As per coding guidelines, "Place unit tests in *.test.ts files alongside source files".
🤖 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 `@packages/ai/tests/summarize-max-length.test.ts` around lines 1 - 139, Move the test file packages/ai/tests/summarize-max-length.test.ts next to the implementation file chat-stream-summarize.ts so the test is colocated with its source; update imports in summarize-max-length.test.ts to reference the local module path (adjust any ../src/... imports) and keep references to ChatStreamSummarizeAdapter, createRecordingTextAdapter, and the test helpers (resolveDebugOption, ev) intact so the suite continues to import the same symbols from the colocated files. Ensure the new location preserves the same filename and that any path changes are minimal and correct for the package module resolution.codemods/move-sampling-to-model-options/transform.ts (1)
341-356: 💤 Low valueConsider removing the type cast for clarity.
Line 351 casts
key.name as RootSamplingKey, butkey.namecould be any string. While safe at runtime (Set.has() returns false for non-members), the cast is technically incorrect.♻️ Clearer type-safe alternative
const key = (prop as Property).key if ( key.type === 'Identifier' && - movedSet.has(key.name as RootSamplingKey) + ROOT_SAMPLING_KEYS.includes(key.name as RootSamplingKey) && + movedSet.has(key.name as RootSamplingKey) ) { return falseOr use a type guard:
+ const isRootSamplingKey = (name: string): name is RootSamplingKey => + ROOT_SAMPLING_KEYS.includes(name as RootSamplingKey) + ... if ( key.type === 'Identifier' && - movedSet.has(key.name as RootSamplingKey) + isRootSamplingKey(key.name) && + movedSet.has(key.name) ) {🤖 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 `@codemods/move-sampling-to-model-options/transform.ts` around lines 341 - 356, The cast key.name as RootSamplingKey is unnecessary/unsafe; instead make the set string-typed and check the identifier name directly: change movedSet's type from Set<RootSamplingKey> to Set<string> when creating it (const movedSet = new Set<string>(presentKeys)) and inside the obj.properties filter, after confirming key.type === 'Identifier', use const name = key.name and call movedSet.has(name) (no cast) to decide removal.codemods/move-sampling-to-model-options/__testfixtures__/shorthand.output.ts (1)
14-16: 💤 Low valueConsider using shorthand property syntax.
The codemod could be enhanced to preserve or use ES6 shorthand syntax when the property name matches the identifier name. Line 15 uses
temperature: temperature,but modern JavaScript/TypeScript convention prefers the shorthand formtemperature,for better readability.Both forms correctly reference the identifier rather than inlining the literal value, so the functional requirement is met.
♻️ More idiomatic shorthand syntax
modelOptions: { - temperature: temperature, + temperature, },🤖 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 `@codemods/move-sampling-to-model-options/__testfixtures__/shorthand.output.ts` around lines 14 - 16, The object inside modelOptions uses verbose property syntax "temperature: temperature,"—update the codemod transform that emits the modelOptions object so when a property key equals its identifier (e.g., temperature) it emits the ES6 shorthand (temperature,) instead; locate the code that constructs or prints the modelOptions object (the logic producing the "modelOptions" node and its properties) and change it to detect identical key+identifier pairs and output the shorthand property form.
🤖 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 `@docs/migration/migration.md`:
- Around line 200-201: The "Complete Migration Example" still places sampling
props (temperature, maxTokens) at the chat() root even though the guide states
they were moved into provider-native modelOptions; update the "After" snippet(s)
referenced around the "Complete Migration Example" so that temperature and
maxTokens are removed from the chat() root and instead placed inside
modelOptions using provider-native keys (e.g., provider-specific names) while
keeping metadata at the root; ensure both affected blocks (around lines 461–468
and the final After snippet) reflect modelOptions usage for sampling.
In `@docs/migration/sampling-options-to-model-options.md`:
- Around line 210-211: The file ends with leaked XML/HTML tags that break
rendering; remove the accidental trailing tags "</content>" and "</invoke>" from
the end of docs/migration/sampling-options-to-model-options.md so the document
ends cleanly with the previous markdown content (ensure there are no other stray
markup fragments left).
In `@packages/ai-anthropic/src/adapters/text.ts`:
- Line 365: The code uses `modelOptions?.max_tokens || 1024` which treats
explicit 0 as falsy and overrides it; change this to use the nullish coalescing
operator so explicit 0 (or other falsy but defined values) are preserved:
replace that expression with `modelOptions?.max_tokens ?? 1024` (or an explicit
undefined check) so `validateTextProviderOptions()` / `validateMaxTokens()` can
correctly detect and reject invalid zero values.
In `@packages/ai-code-mode/models-eval/run-eval.ts`:
- Around line 224-225: The Ollama branch currently returns sampling params at
the top level (case 'ollama'), but the adapter expects them nested under
modelOptions.options; modify the 'ollama' case to return an object with an
options property containing num_predict: maxTokens and num_ctx: 32768 (i.e.,
move num_predict and num_ctx inside options) so the sampling parameters are
forwarded correctly to the Ollama adapter.
In `@packages/ai-openai/src/text/text-provider-options.ts`:
- Around line 17-38: OpenAIBaseOptions currently inherits temperature/top_p via
OpenAISamplingOptions which makes those fields valid for reasoning models;
remove temperature and top_p from the shared base by stopping OpenAIBaseOptions
from extending OpenAISamplingOptions (keep max_output_tokens in the base),
create a new OpenAISamplingOptions (or NonReasoningSamplingOptions) interface
that includes temperature/top_p and apply that only to non-reasoning model
option types in model-meta.ts (e.g., O1/O3 should not use OpenAIBaseOptions with
sampling fields); additionally ensure adapters/responses-text.ts
mapOptionsToRequest() does not blindly spread modelOptions into
ResponseCreateParams for reasoning models—either filter out temperature/top_p
there or only pass the sampling interface for non-reasoning models so the client
never sends temperature/top_p to the Responses API for reasoning models.
In `@packages/ai-openrouter/src/adapters/text.ts`:
- Around line 1148-1153: The request builder currently spreads only
restModelOptions into the ChatRequest (const request) so a root-level metadata
passed to chat({ metadata }) is dropped; update the construction of request in
the text adapter (the const request: Omit<ChatRequest, 'stream'> block) to also
include the root metadata (e.g., preserve options.metadata or a top-level
metadata parameter) alongside model, messages and tools so the ChatRequest
forwarded to OpenRouter contains metadata the same way responses adapter does.
In `@packages/ai/src/activities/summarize/chat-stream-summarize.ts`:
- Around line 80-87: In the Ollama branch (the adapterName === 'ollama' block)
ensure we don’t override caller-set token limits: before injecting
merged.options = { num_predict: maxLength, ...existing } check if existing
already contains any recognized flat token-limit keys (e.g., num_predict,
max_tokens, max_length, max_output_tokens) and if so return merged unchanged;
only set num_predict when none of those keys are present. Update the guard
around merged.options / existing and the assignment in that block to honor those
keys instead of always injecting num_predict.
In `@packages/ai/src/middlewares/otel.ts`:
- Around line 169-172: The helper firstNumber currently returns any value of
typeof 'number' (including NaN and Infinity); update its check in the
firstNumber function so it only returns numbers that are finite by using
Number.isFinite(candidate) (i.e., return candidate only if typeof candidate ===
'number' && Number.isFinite(candidate)), leaving the rest of the control flow
unchanged so invalid numeric values are filtered out and undefined is returned
when no finite number is found.
---
Outside diff comments:
In `@packages/ai-ollama/src/adapters/text.ts`:
- Around line 36-39: The fallback for ResolveModelOptions is too broad (falls
back to ChatRequest) even though mapCommonOptionsToOllama only forwards specific
fields; change the fallback to a narrowed type that only includes the fields
actually forwarded (e.g., an object with an optional options property limited to
Pick<ChatRequest['options'],
'format'|'keep_alive'|'logprobs'|'top_logprobs'|'think'> and an optional tools
property of ChatRequest['tools']) so ResolveModelOptions<TModel> returns either
OllamaChatModelOptionsByName[TModel] or that minimal subset; update the type
alias ResolveModelOptions accordingly so mapCommonOptionsToOllama and related
code use the correct, narrower modelOptions shape.
---
Nitpick comments:
In
`@codemods/move-sampling-to-model-options/__testfixtures__/shorthand.output.ts`:
- Around line 14-16: The object inside modelOptions uses verbose property syntax
"temperature: temperature,"—update the codemod transform that emits the
modelOptions object so when a property key equals its identifier (e.g.,
temperature) it emits the ES6 shorthand (temperature,) instead; locate the code
that constructs or prints the modelOptions object (the logic producing the
"modelOptions" node and its properties) and change it to detect identical
key+identifier pairs and output the shorthand property form.
In `@codemods/move-sampling-to-model-options/README.md`:
- Around line 18-23: The mapping table currently lists a single OpenAI entry
mapping `maxTokens` to `max_output_tokens`, which is ambiguous for users of
`openaiChatCompletions`; update the table to split the OpenAI row into two rows
(e.g., "openai (Responses)" and "openai (Chat Completions)") and set `maxTokens`
-> `max_output_tokens` for the Responses row and `maxTokens` -> `max_tokens` for
the Chat Completions row; mention the `openaiChatCompletions` identifier in the
note so readers know which row to use.
In `@codemods/move-sampling-to-model-options/transform.ts`:
- Around line 341-356: The cast key.name as RootSamplingKey is
unnecessary/unsafe; instead make the set string-typed and check the identifier
name directly: change movedSet's type from Set<RootSamplingKey> to Set<string>
when creating it (const movedSet = new Set<string>(presentKeys)) and inside the
obj.properties filter, after confirming key.type === 'Identifier', use const
name = key.name and call movedSet.has(name) (no cast) to decide removal.
In `@packages/ai/tests/summarize-max-length.test.ts`:
- Around line 1-139: Move the test file
packages/ai/tests/summarize-max-length.test.ts next to the implementation file
chat-stream-summarize.ts so the test is colocated with its source; update
imports in summarize-max-length.test.ts to reference the local module path
(adjust any ../src/... imports) and keep references to
ChatStreamSummarizeAdapter, createRecordingTextAdapter, and the test helpers
(resolveDebugOption, ev) intact so the suite continues to import the same
symbols from the colocated files. Ensure the new location preserves the same
filename and that any path changes are minimal and correct for the package
module resolution.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1e00706c-024b-453a-a0d4-8a667e653722
📒 Files selected for processing (86)
.changeset/sampling-options-to-model-options.mdcodemods/README.mdcodemods/move-sampling-to-model-options/README.mdcodemods/move-sampling-to-model-options/__testfixtures__/anthropic-merge.input.tscodemods/move-sampling-to-model-options/__testfixtures__/anthropic-merge.output.tscodemods/move-sampling-to-model-options/__testfixtures__/conflict.input.tscodemods/move-sampling-to-model-options/__testfixtures__/conflict.output.tscodemods/move-sampling-to-model-options/__testfixtures__/create-chat-options.input.tscodemods/move-sampling-to-model-options/__testfixtures__/create-chat-options.output.tscodemods/move-sampling-to-model-options/__testfixtures__/gemini-rename.input.tscodemods/move-sampling-to-model-options/__testfixtures__/gemini-rename.output.tscodemods/move-sampling-to-model-options/__testfixtures__/generate-and-ai.input.tscodemods/move-sampling-to-model-options/__testfixtures__/generate-and-ai.output.tscodemods/move-sampling-to-model-options/__testfixtures__/groq-maxtokens.input.tscodemods/move-sampling-to-model-options/__testfixtures__/groq-maxtokens.output.tscodemods/move-sampling-to-model-options/__testfixtures__/no-import.input.tscodemods/move-sampling-to-model-options/__testfixtures__/no-import.output.tscodemods/move-sampling-to-model-options/__testfixtures__/ollama-nested.input.tscodemods/move-sampling-to-model-options/__testfixtures__/ollama-nested.output.tscodemods/move-sampling-to-model-options/__testfixtures__/openai-basic.input.tscodemods/move-sampling-to-model-options/__testfixtures__/openai-basic.output.tscodemods/move-sampling-to-model-options/__testfixtures__/openrouter-maxtokens.input.tscodemods/move-sampling-to-model-options/__testfixtures__/openrouter-maxtokens.output.tscodemods/move-sampling-to-model-options/__testfixtures__/shorthand.input.tscodemods/move-sampling-to-model-options/__testfixtures__/shorthand.output.tscodemods/move-sampling-to-model-options/__testfixtures__/unresolvable-adapter.input.tscodemods/move-sampling-to-model-options/__testfixtures__/unresolvable-adapter.output.tscodemods/move-sampling-to-model-options/transform.test.tscodemods/move-sampling-to-model-options/transform.tscodemods/package.jsondocs/adapters/anthropic.mddocs/adapters/gemini.mddocs/adapters/grok.mddocs/adapters/groq.mddocs/adapters/ollama.mddocs/adapters/openai.mddocs/adapters/openrouter.mddocs/advanced/middleware.mddocs/advanced/typed-options.mddocs/api/ai.mddocs/config.jsondocs/migration/migration.mddocs/migration/sampling-options-to-model-options.mdpackage.jsonpackages/ai-anthropic/src/adapters/text.tspackages/ai-anthropic/src/text/text-provider-options.tspackages/ai-anthropic/tests/anthropic-adapter.test.tspackages/ai-code-mode/models-eval/judge.tspackages/ai-code-mode/models-eval/run-eval.tspackages/ai-gemini/src/adapters/text.tspackages/ai-gemini/src/experimental/text-interactions/adapter.tspackages/ai-gemini/src/text/text-provider-options.tspackages/ai-gemini/tests/gemini-adapter.test.tspackages/ai-gemini/tests/text-interactions-adapter.test.tspackages/ai-grok/tests/grok-adapter.test.tspackages/ai-groq/tests/groq-adapter.test.tspackages/ai-ollama/src/adapters/text.tspackages/ai-ollama/src/index.tspackages/ai-ollama/src/meta/models-meta.tspackages/ai-ollama/tests/text-adapter.test.tspackages/ai-openai/src/text/text-provider-options.tspackages/ai-openai/tests/chat-per-model-type-safety.test.tspackages/ai-openai/tests/openai-adapter.test.tspackages/ai-openrouter/src/adapters/responses-text.tspackages/ai-openrouter/src/adapters/text.tspackages/ai-openrouter/tests/openrouter-adapter.test.tspackages/ai-openrouter/tests/openrouter-responses-adapter.test.tspackages/ai/skills/ai-core/adapter-configuration/SKILL.mdpackages/ai/skills/ai-core/adapter-configuration/references/anthropic-adapter.mdpackages/ai/skills/ai-core/adapter-configuration/references/gemini-adapter.mdpackages/ai/skills/ai-core/adapter-configuration/references/ollama-adapter.mdpackages/ai/skills/ai-core/adapter-configuration/references/openai-adapter.mdpackages/ai/skills/ai-core/chat-experience/SKILL.mdpackages/ai/skills/ai-core/middleware/SKILL.mdpackages/ai/src/activities/chat/index.tspackages/ai/src/activities/chat/middleware/types.tspackages/ai/src/activities/summarize/chat-stream-summarize.tspackages/ai/src/middlewares/otel.tspackages/ai/src/types.tspackages/ai/tests/chat.test.tspackages/ai/tests/middleware.test.tspackages/ai/tests/middlewares/otel.test.tspackages/ai/tests/summarize-max-length.test.tspackages/openai-base/src/adapters/chat-completions-text.tspackages/openai-base/src/adapters/responses-text.tspackages/openai-base/tests/responses-text.test.ts
💤 Files with no reviewable changes (4)
- packages/ai-gemini/src/adapters/text.ts
- packages/ai-gemini/src/experimental/text-interactions/adapter.ts
- packages/ai/src/types.ts
- packages/ai-ollama/src/index.ts
Comment on lines
+200
to
+201
| > **Heads up — sampling has since moved (breaking).** In a later release, the sampling props (`temperature`, `topP`, `maxTokens`) were removed from the root of `chat()` and now live in provider-native `modelOptions`. Passing them at the root no longer type-checks or takes effect. See [Moving Sampling Options into modelOptions](./sampling-options-to-model-options) for the codemod and provider-native key names. `metadata` stays at the root. | ||
|
|
Contributor
There was a problem hiding this comment.
The migration guide now contradicts itself on sampling placement.
Line 200 says root-level temperature / maxTokens are removed, but the “Complete Migration Example” still uses those root props at Lines 464-465. Please move those values into modelOptions with provider-native keys in that final “After” snippet too, otherwise readers will copy a broken migration target.
Also applies to: 461-468
🤖 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 `@docs/migration/migration.md` around lines 200 - 201, The "Complete Migration
Example" still places sampling props (temperature, maxTokens) at the chat() root
even though the guide states they were moved into provider-native modelOptions;
update the "After" snippet(s) referenced around the "Complete Migration Example"
so that temperature and maxTokens are removed from the chat() root and instead
placed inside modelOptions using provider-native keys (e.g., provider-specific
names) while keeping metadata at the root; ensure both affected blocks (around
lines 461–468 and the final After snippet) reflect modelOptions usage for
sampling.
Comment on lines
+210
to
+211
| </content> | ||
| </invoke> |
Contributor
There was a problem hiding this comment.
Remove accidental trailing tags from the new migration doc.
</content> and </invoke> at the end of the file look like leaked markup artifacts. They should be removed to avoid rendering/build issues in docs pipelines.
🤖 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 `@docs/migration/sampling-options-to-model-options.md` around lines 210 - 211,
The file ends with leaked XML/HTML tags that break rendering; remove the
accidental trailing tags "</content>" and "</invoke>" from the end of
docs/migration/sampling-options-to-model-options.md so the document ends cleanly
with the previous markdown content (ensure there are no other stray markup
fragments left).
| ? validProviderOptions.thinking.budget_tokens | ||
| : undefined | ||
| const defaultMaxTokens = options.maxTokens || 1024 | ||
| const defaultMaxTokens = modelOptions?.max_tokens || 1024 |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the fallback in the adapter.
rg -n "defaultMaxTokens|max_tokens \\|\\| 1024" packages/ai-anthropic/src/adapters/text.ts
# Demonstrate the runtime semantics that make `||` incorrect here.
node - <<'NODE'
console.log({
withOr: (0 || 1024),
withNullish: (0 ?? 1024),
})
NODERepository: TanStack/ai
Length of output: 331
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="packages/ai-anthropic/src/adapters/text.ts"
# Print the relevant sections around line 365 and any validation logic for max_tokens.
sed -n '330,420p' "$file"
# Find the validateMaxTokens definition and all its call sites in this file.
rg -n "validateMaxTokens|max_tokens" "$file" | head -n 200Repository: TanStack/ai
Length of output: 4291
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="packages/ai-anthropic/src/adapters/text.ts"
# Narrow further around where validateMaxTokens is called (if present).
# (Grepping line numbers first, then printing context around them.)
lines=$(rg -n "validateMaxTokens" "$file" | cut -d: -f1 || true)
if [[ -n "${lines}" ]]; then
for ln in $lines; do
start=$((ln-40))
end=$((ln+40))
echo "---- context around validateMaxTokens at line $ln ----"
sed -n "${start},${end}p" "$file"
done
else
echo "No validateMaxTokens symbol found in $file"
fiRepository: TanStack/ai
Length of output: 135
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search repo for validateMaxTokens usage/definition.
rg -n "validateMaxTokens" -S . | head -n 200Repository: TanStack/ai
Length of output: 278
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="packages/ai-anthropic/src/text/text-provider-options.ts"
# Print validateMaxTokens implementation around the reported lines.
sed -n '280,380p' "$file"
# Find call sites of validateMaxTokens across ai-anthropic.
rg -n "validateMaxTokens\\(" packages/ai-anthropic/src | head -n 50
# Also inspect around where max_tokens is processed in that provider options file.
rg -n "max_tokens" "$file" | head -n 80Repository: TanStack/ai
Length of output: 2229
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="packages/ai-anthropic/src/adapters/text.ts"
# Where validation is invoked for provider options.
rg -n "validateTextProviderOptions" "$file"
# Also locate construction of max_tokens / modelOptions flow in this adapter.
rg -n "defaultMaxTokens|maxTokens|max_tokens" "$file" -n
# Show a wider slice around where requestParams is assembled and any validation call nearby.
sed -n '240,520p' "$file"Repository: TanStack/ai
Length of output: 10472
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find type declarations for max_tokens in ai-anthropic adapter text.
rg -n "max_tokens\\??\\s*:" packages/ai-anthropic/src/adapters/text.ts packages/ai-anthropic/src/text/text-provider-options.ts | head -n 200
# Also print the surrounding type block for the public modelOptions definition.
# (This gives context for whether 0 is allowed at the type level.)
rg -n "export (type|interface) .*Anthropic.*TextProviderOptions|interface .*TextProviderOptions|type .*TextProviderOptions" packages/ai-anthropic/src/adapters/text.ts packages/ai-anthropic/src/text/text-provider-options.ts | head -n 50Repository: TanStack/ai
Length of output: 684
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="packages/ai-anthropic/src/text/text-provider-options.ts"
# Print around the max_tokens declarations and the interfaces.
sed -n '180,280p' "$file"Repository: TanStack/ai
Length of output: 4055
Preserve explicit max_tokens values (don’t use ||)
In packages/ai-anthropic/src/adapters/text.ts, modelOptions?.max_tokens || 1024 turns max_tokens: 0 into 1024, so validateTextProviderOptions()/validateMaxTokens() never rejects the invalid value (max_tokens must be at least 1).
Suggested fix
- const defaultMaxTokens = modelOptions?.max_tokens || 1024
+ const defaultMaxTokens = modelOptions?.max_tokens ?? 1024📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const defaultMaxTokens = modelOptions?.max_tokens || 1024 | |
| const defaultMaxTokens = modelOptions?.max_tokens ?? 1024 |
🤖 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 `@packages/ai-anthropic/src/adapters/text.ts` at line 365, The code uses
`modelOptions?.max_tokens || 1024` which treats explicit 0 as falsy and
overrides it; change this to use the nullish coalescing operator so explicit 0
(or other falsy but defined values) are preserved: replace that expression with
`modelOptions?.max_tokens ?? 1024` (or an explicit undefined check) so
`validateTextProviderOptions()` / `validateMaxTokens()` can correctly detect and
reject invalid zero values.
Comment on lines
+224
to
+225
| case 'ollama': | ||
| return { num_predict: maxTokens, num_ctx: 32768 } |
Contributor
There was a problem hiding this comment.
Critical: Ollama requires nested options structure.
The Ollama adapter expects sampling options nested under modelOptions.options, not at the top level of modelOptions. This will cause the sampling parameters to be ignored.
Based on learnings from layer 6: "Update Ollama adapter to forward sampling via nested modelOptions.options".
🐛 Proposed fix
case 'ollama':
- return { num_predict: maxTokens, num_ctx: 32768 }
+ return { options: { num_predict: maxTokens, num_ctx: 32768 } }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| case 'ollama': | |
| return { num_predict: maxTokens, num_ctx: 32768 } | |
| case 'ollama': | |
| return { options: { num_predict: maxTokens, num_ctx: 32768 } } |
🤖 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 `@packages/ai-code-mode/models-eval/run-eval.ts` around lines 224 - 225, The
Ollama branch currently returns sampling params at the top level (case
'ollama'), but the adapter expects them nested under modelOptions.options;
modify the 'ollama' case to return an object with an options property containing
num_predict: maxTokens and num_ctx: 32768 (i.e., move num_predict and num_ctx
inside options) so the sampling parameters are forwarded correctly to the Ollama
adapter.
Comment on lines
+17
to
+38
| /** Sampling controls shared by all Responses-API models. */ | ||
| export interface OpenAISamplingOptions { | ||
| /** | ||
| * Sampling temperature, 0–2. Higher = more random. Recommend altering this or top_p, not both. | ||
| * Note: OpenAI reasoning models (o-series, GPT-5 reasoning) reject temperature/top_p. | ||
| * https://platform.openai.com/docs/api-reference/responses/create#responses_create-temperature | ||
| */ | ||
| temperature?: number | ||
| /** | ||
| * Nucleus sampling. 0.1 = only the top 10% probability mass is considered. | ||
| * https://platform.openai.com/docs/api-reference/responses/create#responses_create-top_p | ||
| */ | ||
| top_p?: number | ||
| /** | ||
| * Upper bound on generated tokens (visible output + reasoning tokens). | ||
| * https://platform.openai.com/docs/api-reference/responses/create#responses_create-max_output_tokens | ||
| */ | ||
| max_output_tokens?: number | ||
| } | ||
|
|
||
| // Core, always-available options for Responses API | ||
| export interface OpenAIBaseOptions { | ||
| export interface OpenAIBaseOptions extends OpenAISamplingOptions { |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Do OpenAI Responses API reasoning models (for example o3 and GPT-5 reasoning models) accept the temperatureandtop_p parameters, or are those parameters rejected?
💡 Result:
OpenAI's dedicated reasoning models, such as the o1 and o3 series (including o3-mini) and the GPT-5 series, generally do not accept the traditional temperature and top_p sampling parameters via the API [1][2][3][4]. Attempts to include these parameters in API requests for these models typically result in a 400-level invalid_request_error stating that they are unsupported [1][2]. These reasoning models instead utilize specialized parameters to control their behavior, most notably reasoning.effort (or simply reasoning_effort in some environments), which allows developers to adjust the depth of the model's internal thinking process (e.g., minimal, low, medium, high, or xhigh) [5][6][7][8][3]. The exclusion of sampling parameters like temperature is a deliberate design choice by OpenAI to maintain the reliability and determinism required for complex, multi-step reasoning tasks [2]. Because these models are optimized to generate a high-quality, singular "best" chain-of-thought, standard sampling techniques used for creative text generation are often incompatible with their internal architecture [2]. Developers building on these models should omit temperature and top_p from their API calls to avoid request errors [1][2][4].
Citations:
- 1: CLI erroneously sends unsupported parameters (temperature/top_p) to the o3-mini model openai/openai-python#2072
- 2: https://community.openai.com/t/o3-mini-unsupported-parameter-temperature/1140846
- 3: https://learn.microsoft.com/en-us/azure/foundry/openai/how-to/reasoning
- 4: https://forum.popclip.app/t/required-api-changes-for-gpt-5-x-reasoning-models/3661
- 5: https://developers.openai.com/api/docs/guides/reasoning
- 6: https://docs.x.ai/developers/model-capabilities/text/reasoning
- 7: https://developers.openai.com/api/docs/models/gpt-5
- 8: https://developers.openai.com/api/docs/guides/latest-model
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find all references to the shared sampling/base types
rg -n "OpenAISamplingOptions|OpenAIBaseOptions" packages/ai-openai/src/text/text-provider-options.ts packages/ai-openai/src -S
# Show the rest of the file around where base/model option maps are composed
FILE="packages/ai-openai/src/text/text-provider-options.ts"
if [ -f "$FILE" ]; then
echo "---- $FILE (context) ----"
# print around the shown snippet and further down for composition
nl -ba "$FILE" | sed -n '1,220p'
fi
# Look for option maps / reasoning models naming patterns
rg -n "reasoning|o3|o1|gpt-5|responses.*(create|temperature|top_p)" packages/ai-openai/src -S
# Identify where modelOptions types are built for text providers
rg -n "modelOptions|ModelOptions|options.*model" packages/ai-openai/src/text packages/ai-openai/src -SRepository: TanStack/ai
Length of output: 9554
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show key type definitions and where reasoning models include base options.
rg -n "interface OpenAIReasoningOptions|type OpenAIReasoningOptions|OpenAIReasoningOptions" packages/ai-openai/src/text/text-provider-options.ts packages/ai-openai/src/model-meta.ts -S
# Print relevant ranges from text-provider-options.ts around sampling/base/reasoning definitions
FILE="packages/ai-openai/src/text/text-provider-options.ts"
# Get line numbers via python and print small windows around important anchors
python3 - <<'PY'
import re, itertools
file_path="packages/ai-openai/src/text/text-provider-options.ts"
anchors=["export interface OpenAISamplingOptions","export interface OpenAIBaseOptions","OpenAIReasoningOptions","ExternalTextProviderOptions"]
with open(file_path,'r',encoding='utf-8') as f:
lines=f.readlines()
for i,line in enumerate(lines,1):
for a in anchors:
if a in line:
print(f"--- anchor:{a} at {i} ---")
start=max(1,i-25); end=min(len(lines),i+120)
for j in range(start,end+1):
# trim long lines a bit? just output as-is
print(f"{j}:{lines[j-1].rstrip()}")
PY
# Print relevant ranges from model-meta.ts around reasoning models (O3/O1) where OpenAIBaseOptions is used
python3 - <<'PY'
file_path="packages/ai-openai/src/model-meta.ts"
targets=["[O3.name]","[O1.name]","[O3_PRO.name]","[O1_PRO.name]","reasoning"]
with open(file_path,'r',encoding='utf-8') as f:
lines=f.readlines()
def match_any(line):
return any(t in line for t in targets)
for i,line in enumerate(lines,1):
if match_any(line):
print(f"--- match at {i}: {line.strip()} ---")
start=max(1,i-20); end=min(len(lines),i+40)
for j in range(start,end+1):
print(f"{j}:{lines[j-1].rstrip()}")
PY
# Search for how temperature/top_p is serialized into API requests.
rg -n "temperature|max_output_tokens|top_p" packages/ai-openai/src -SRepository: TanStack/ai
Length of output: 50367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show key reasoning type definitions and where reasoning models include base options.
rg -n "OpenAIReasoningOptions" packages/ai-openai/src/text/text-provider-options.ts packages/ai-openai/src/model-meta.ts -S
# Print relevant windows from text-provider-options.ts around sampling/base/reasoning/external option composition.
python3 - <<'PY'
file_path="packages/ai-openai/src/text/text-provider-options.ts"
anchors=[
"export interface OpenAISamplingOptions",
"export interface OpenAIBaseOptions",
"OpenAIReasoningOptions",
"export type ExternalTextProviderOptions"
]
with open(file_path,'r',encoding='utf-8') as f:
lines=f.readlines()
for i,line in enumerate(lines,1):
for a in anchors:
if a in line:
print(f"\n--- anchor:{a} at {i} ---")
start=max(1,i-25); end=min(len(lines),i+180)
for j in range(start,end+1):
print(f"{j}:{lines[j-1].rstrip()}")
PY
# Print relevant windows from model-meta.ts around reasoning models (O3/O1) where OpenAIBaseOptions is used.
python3 - <<'PY'
file_path="packages/ai-openai/src/model-meta.ts"
with open(file_path,'r',encoding='utf-8') as f:
lines=f.readlines()
# Print around O3/O1 entries explicitly
for i,line in enumerate(lines,1):
if "[O3.name]" in line or "[O1.name]" in line:
print(f"\n--- model-meta match at {i}: {line.strip()} ---")
start=max(1,i-25); end=min(len(lines),i+60)
for j in range(start,end+1):
print(f"{j}:{lines[j-1].rstrip()}")
PY
# Search for how temperature/top_p is serialized into API requests.
rg -n "temperature|max_output_tokens|top_p" packages/ai-openai/src -SRepository: TanStack/ai
Length of output: 50367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find non-type usages/serialization points for temperature/top_p.
# Limit output to keep it readable.
rg -n --type ts "(^|[^a-zA-Z0-9_])(temperature|top_p|max_output_tokens)([^a-zA-Z0-9_]|$)" packages/ai-openai/src -S \
| head -n 80
# Look specifically for payload construction / Responses create call sites.
rg -n --type ts "responses\.create|Responses\.create|createResponse|response.*payload|payload\s*=" packages/ai-openai/src -S \
| head -n 80
# Look for any conditional gating related to reasoning models.
rg -n --type ts "reasoning.*temperature|temperature.*reasoning|top_p.*reasoning|reasoning.*top_p" packages/ai-openai/src -S \
| head -n 80
rg -n --type ts "O1\.name|O3\.name|o-series|o3|gpt-5.*reasoning|reasoning model" packages/ai-openai/src -S \
| head -n 80Repository: TanStack/ai
Length of output: 8286
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for where temperature/top_p are mapped into Responses API payloads for text.
rg -n --type ts "top_p\s*:|temperature\s*:" packages/ai-openai/src/adapters packages/ai-openai/src/text -S
# Look for any explicit filtering based on model capability/name.
rg -n --type ts "O1\.name|O3\.name|o3|o1|reasoning model|reasoning" packages/ai-openai/src/adapters packages/ai-openai/src/text -S | head -n 200
# Also check the legacy/openai adapter entrypoint if it builds payload for responses.create.
rg -n --type ts "responses\.create|OpenAI\.responses\.create|create\(" packages/ai-openai/src/adapters packages/ai-openai/src/text -S | head -n 200Repository: TanStack/ai
Length of output: 37
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/ai-openai/src/adapters/text.ts"
python3 - <<'PY'
import itertools, pathlib
path=pathlib.Path("packages/ai-openai/src/adapters/text.ts")
text=path.read_text(encoding="utf-8").splitlines()
# print whole file if small? otherwise print key regions
print("LINES", len(text))
# find occurrences of responses.create or payload building
need=[]
for i,line in enumerate(text,1):
if "responses.create" in line or "Responses.create" in line or "openai.responses.create" in line:
need.append(i)
if "temperature" in line or "top_p" in line:
need.append(i)
if not need:
# still print first 260 lines and any later sections likely containing request building
slices=[(1,220),(220,440),(440,660),(660,880)]
else:
slices=[]
for i in sorted(set(need)):
slices.append((max(1,i-60), min(len(text), i+80)))
# dedupe slices by start
seen=set()
for a,b in slices:
key=(a,b)
if key in seen: continue
seen.add(key)
print(f"\n---- slice {a}-{b} ----")
for j in range(a,b+1):
print(f"{j}:{text[j-1]}")
PY
# Broader scan in adapters for responses payload creation / temperature
rg -n --type ts "responses\.create|OpenAI.*responses|response[s]?\s*\(" packages/ai-openai/src/adapters -S
rg -n --type ts "temperature|max_output_tokens|top_p" packages/ai-openai/src/adapters -S || trueRepository: TanStack/ai
Length of output: 5664
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "OpenAIBaseResponsesTextAdapter" -S packages
# If found, show the key parts where it builds the ResponseCreateParams payload.
# Otherwise, we may need to determine that the filtering (if any) lives in an external package.
rg -n "mapOptionsToRequest\\(|ResponseCreateParams|responses\\.create|temperature|top_p" -S packages | head -n 200Repository: TanStack/ai
Length of output: 28149
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/openai-base/src/adapters/responses-text.ts"
# show the region around the comment mentioning modelOptions is sole sampling surface
# (use python to avoid relying on nl)
python3 - <<'PY'
import pathlib
path=pathlib.Path("packages/openai-base/src/adapters/responses-text.ts")
lines=path.read_text(encoding="utf-8").splitlines()
# print around the first occurrence of the anchor text
anchor="modelOptions is the sole sampling surface"
idx=None
for i,l in enumerate(lines):
if anchor in l:
idx=i
break
print("ANCHOR_LINE_INDEX", idx+1 if idx is not None else None)
if idx is None:
raise SystemExit("anchor not found")
start=max(0, idx-80)
end=min(len(lines), idx+220)
for j in range(start,end):
print(f"{j+1}:{lines[j]}")
PY
# Then narrow specifically for temperature/top_p serialization in this file
rg -n "temperature|top_p|max_output_tokens" "$FILE" -S
# Print small windows around each temperature/top_p occurrence (keep concise)
python3 - <<'PY'
import pathlib,re
path=pathlib.Path("packages/openai-base/src/adapters/responses-text.ts")
lines=path.read_text(encoding="utf-8").splitlines()
hits=[]
for i,l in enumerate(lines,1):
if re.search(r"\b(temperature|top_p|max_output_tokens)\b", l):
hits.append(i)
# keep up to 10 hits for brevity
for i in hits[:12]:
start=max(1,i-15); end=min(len(lines),i+25)
print(f"\n--- window around line {i} ---")
for j in range(start,end+1):
print(f"{j}:{lines[j-1]}")
PYRepository: TanStack/ai
Length of output: 113
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/openai-base/src/adapters/responses-text.ts"
# Find where mapOptionsToRequest starts and where it mentions modelOptions/sampling.
rg -n --type ts "mapOptionsToRequest|modelOptions|sampling|temperature|top_p" "$FILE" -S
# Print around mapOptionsToRequest and around any temperature/top_p mentions.
python3 - <<'PY'
import pathlib,re
path=pathlib.Path("packages/openai-base/src/adapters/responses-text.ts")
lines=path.read_text(encoding="utf-8").splitlines()
# line numbers with temperature/top_p/max_output_tokens
targets=[]
for i,l in enumerate(lines,1):
if re.search(r"\b(temperature|top_p|max_output_tokens)\b", l):
targets.append(i)
# add mapOptionsToRequest region start
for i,l in enumerate(lines,1):
if "mapOptionsToRequest" in l:
targets.append(i)
targets=sorted(set(targets))
print("HIT_COUNT", len(targets))
# print windows
for i in targets[:30]:
start=max(1,i-25); end=min(len(lines), i+50)
print(f"\n---- window around {i} ----")
for j in range(start,end+1):
print(f"{j}:{lines[j-1]}")
PYRepository: TanStack/ai
Length of output: 37172
Don’t expose temperature/top_p on the shared OpenAI base options for reasoning models
OpenAIBaseOptionscurrently extendsOpenAISamplingOptions(temperature/top_p), andmodel-meta.tsassignsOpenAIBaseOptionsto reasoning models likeO1/O3, so these fields become valid for all models.- At runtime,
packages/openai-base/src/adapters/responses-text.tsmapOptionsToRequest()spreads...modelOptionsdirectly intoResponseCreateParams(the sole sampling surface), with no capability-based filtering—sotemperature/top_pwill be sent to/v1/responsesand rejected by OpenAI reasoning models.
Keep max_output_tokens in the shared base if needed, but gate/remove temperature/top_p from the shared base and put them only on the non-reasoning model option types.
🤖 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 `@packages/ai-openai/src/text/text-provider-options.ts` around lines 17 - 38,
OpenAIBaseOptions currently inherits temperature/top_p via OpenAISamplingOptions
which makes those fields valid for reasoning models; remove temperature and
top_p from the shared base by stopping OpenAIBaseOptions from extending
OpenAISamplingOptions (keep max_output_tokens in the base), create a new
OpenAISamplingOptions (or NonReasoningSamplingOptions) interface that includes
temperature/top_p and apply that only to non-reasoning model option types in
model-meta.ts (e.g., O1/O3 should not use OpenAIBaseOptions with sampling
fields); additionally ensure adapters/responses-text.ts mapOptionsToRequest()
does not blindly spread modelOptions into ResponseCreateParams for reasoning
models—either filter out temperature/top_p there or only pass the sampling
interface for non-reasoning models so the client never sends temperature/top_p
to the Responses API for reasoning models.
Comment on lines
1148
to
1153
| const request: Omit<ChatRequest, 'stream'> = { | ||
| ...modelOptions, | ||
| ...restModelOptions, | ||
| model: options.model + variantSuffix, | ||
| messages, | ||
| ...(options.temperature !== undefined && { | ||
| temperature: options.temperature, | ||
| }), | ||
| ...(options.maxTokens !== undefined && { | ||
| maxCompletionTokens: options.maxTokens, | ||
| }), | ||
| ...(options.topP !== undefined && { topP: options.topP }), | ||
| ...(tools && tools.length > 0 && { tools }), | ||
| } |
Contributor
There was a problem hiding this comment.
Preserve root metadata in the chat-completions request mapping.
This request builder now only forwards modelOptions, so chat({ metadata }) gets dropped on the OpenRouter chat adapter even though root metadata is still part of the migration contract and the responses adapter still preserves it.
Suggested fix
const request: Omit<ChatRequest, 'stream'> = {
...restModelOptions,
model: options.model + variantSuffix,
+ ...(options.metadata !== undefined && { metadata: options.metadata }),
messages,
...(tools && tools.length > 0 && { tools }),
}🤖 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 `@packages/ai-openrouter/src/adapters/text.ts` around lines 1148 - 1153, The
request builder currently spreads only restModelOptions into the ChatRequest
(const request) so a root-level metadata passed to chat({ metadata }) is
dropped; update the construction of request in the text adapter (the const
request: Omit<ChatRequest, 'stream'> block) to also include the root metadata
(e.g., preserve options.metadata or a top-level metadata parameter) alongside
model, messages and tools so the ChatRequest forwarded to OpenRouter contains
metadata the same way responses adapter does.
Comment on lines
+80
to
+87
| if (adapterName === 'ollama') { | ||
| const existing = | ||
| merged.options && typeof merged.options === 'object' | ||
| ? (merged.options as Record<string, unknown>) | ||
| : undefined | ||
| if (existing && typeof existing.num_predict === 'number') return merged | ||
| merged.options = { num_predict: maxLength, ...existing } | ||
| return merged |
Contributor
There was a problem hiding this comment.
Honor caller-set token limits in the Ollama branch too.
This branch only treats options.num_predict as caller-owned. If the caller already provided a recognized flat token limit (for example during migration or via shared config), maxLength still injects options.num_predict and silently overrides the effective limit for Ollama.
Suggested fix
if (adapterName === 'ollama') {
+ const callerSetFlatLimit = KNOWN_MAX_TOKENS_KEYS.some(
+ (k) => typeof merged[k] === 'number',
+ )
const existing =
merged.options && typeof merged.options === 'object'
? (merged.options as Record<string, unknown>)
: undefined
- if (existing && typeof existing.num_predict === 'number') return merged
+ if (callerSetFlatLimit || (existing && typeof existing.num_predict === 'number')) {
+ return merged
+ }
merged.options = { num_predict: maxLength, ...existing }
return merged
}🤖 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 `@packages/ai/src/activities/summarize/chat-stream-summarize.ts` around lines
80 - 87, In the Ollama branch (the adapterName === 'ollama' block) ensure we
don’t override caller-set token limits: before injecting merged.options = {
num_predict: maxLength, ...existing } check if existing already contains any
recognized flat token-limit keys (e.g., num_predict, max_tokens, max_length,
max_output_tokens) and if so return merged unchanged; only set num_predict when
none of those keys are present. Update the guard around merged.options /
existing and the assignment in that block to honor those keys instead of always
injecting num_predict.
Comment on lines
+169
to
+172
| function firstNumber(...candidates: Array<unknown>): number | undefined { | ||
| for (const candidate of candidates) { | ||
| if (typeof candidate === 'number') return candidate | ||
| } |
Contributor
There was a problem hiding this comment.
Reject non-finite sampling values here.
Line 171 currently accepts NaN and Infinity even though the helper promises a finite number. That can leak invalid numeric values into gen_ai.request.* span attributes and cause exporter/backend issues.
Suggested fix
function firstNumber(...candidates: Array<unknown>): number | undefined {
for (const candidate of candidates) {
- if (typeof candidate === 'number') return candidate
+ if (typeof candidate === 'number' && Number.isFinite(candidate)) {
+ return candidate
+ }
}
return undefined
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function firstNumber(...candidates: Array<unknown>): number | undefined { | |
| for (const candidate of candidates) { | |
| if (typeof candidate === 'number') return candidate | |
| } | |
| function firstNumber(...candidates: Array<unknown>): number | undefined { | |
| for (const candidate of candidates) { | |
| if (typeof candidate === 'number' && Number.isFinite(candidate)) { | |
| return candidate | |
| } | |
| } | |
| return undefined | |
| } |
🤖 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 `@packages/ai/src/middlewares/otel.ts` around lines 169 - 172, The helper
firstNumber currently returns any value of typeof 'number' (including NaN and
Infinity); update its check in the firstNumber function so it only returns
numbers that are finite by using Number.isFinite(candidate) (i.e., return
candidate only if typeof candidate === 'number' && Number.isFinite(candidate)),
leaving the rest of the control flow unchanged so invalid numeric values are
filtered out and undefined is returned when no finite number is found.
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.
Summary
Moves sampling options —
temperature,topP,maxTokens— off the root ofchat()/ai()/generate()and into provider-native, fully-typedmodelOptions.modelOptionsis now the single sampling surface; there is no generic root-level mapping anymore. Each provider exposes its real SDK/API key names.This continues and completes #499 by @harry-whorlow (which was built on the old
packages/typescript/layout and could not be rebased onto the restructuredpackages/tree). Supersedes #499.Provider-native
modelOptionskeystemperaturetop_pmax_output_tokenstemperaturetop_pmax_tokenstemperaturetopPmaxOutputTokenstemperaturetop_pmax_tokenstemperaturetop_pmax_completion_tokensoptions.temperatureoptions.top_poptions.num_predict(nested)temperaturetopPmaxCompletionTokenstemperaturetopPmaxOutputTokensWhat changed
@tanstack/ai): removedtemperature/topP/maxTokensfromTextOptions, the public activity options, andChatMiddlewareConfig(+ structured-output config). Removed all engine plumbing. Middleware now adjusts sampling viaconfig.modelOptions.metadatais unchanged and stays at the root.modelOptions(native keys, verified against each provider's API). The provider-options types gained the sampling fields where missing (OpenAI, Anthropic, Gemini, Ollama; Grok/Groq/OpenRouter already had them).optionsvs flat runtime spread); an OpenRoutervariantfield leaking into the request body; OTelgen_ai.request.*attributes now read a union of provider key spellings;summarize({ maxLength })now resolves the correct per-provider token key from the wrapped adapter (was about to become a silent no-op).move-sampling-to-model-options(jscodeshift): resolves the provider from the adapter factory and rewrites keys per provider (incl. Ollama nesting); reports + skips on conflicts or unresolvable adapters. 12 fixture cases.docs/migration/sampling-options-to-model-options), updated adapter pages, middlewaredynamic-temperatureexample, API reference, and typed-options.adapter-configuration,middleware,chat-experienceSKILL.md + reference files updated to the new surface.minoracross@tanstack/ai+ all affected adapter packages.Migration
pnpm codemod:move-sampling-to-model-options "src/**/*.{ts,tsx}"Testing
max_tokensdefault + no spurious dropped-key warning).modelOptions).pnpm test:prgreen:test:sherif, test:knip, test:docs, test:eslint, test:lib, test:types, test:build, buildacross 33 projects.Known follow-up (not blocking)
The
ts-code-mode-web/ts-react-chatexamples still pass a genericmaxTokenson calls with dynamically-resolved adapters — the codemod can't infer a provider there, and they're excluded from CI (examples/**). They need per-example provider mapping in a follow-up.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
Breaking Changes
temperature,topP,maxTokens) are no longer accepted at the root ofchat()/ai()/generate()calls. These must now be passed under provider-nativemodelOptionswith provider-specific key names.New Features
modelOptionsfor sampling configuration.Documentation
modelOptionsstructure.