Conversation
hassiebp
approved these changes
Sep 14, 2026
hassiebp
left a comment
Collaborator
There was a problem hiding this comment.
LGTM after merge conflicts have been resolved 👍
* feat: trace the remaining request parameters pi puts on the wire * test: assert sampling parameters against the wire, not the model config
milanagm
force-pushed
the
feat/model-parameters
branch
from
September 15, 2026 11:16
1e34892 to
ed5099e
Compare
Collaborator
Author
|
@hassiebp resolved |
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
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.
Closes #22.
Change
Generations carried
modelandmetadata.providerand nothing about how the model was called. Two runs of the same prompt at different thinking levels were indistinguishable, and astop_reason: lengtharrived without the cap that produced it.Every generation now carries
modelParameterswithmax_tokensand, for reasoning models with thinking on,thinking_level.Why
max_tokenscomes from the request payloadctx.model.maxTokensis the value configured inmodels.json. It is not what pi sends. Every provider'sstreamSimplegoes throughbuildBaseOptions(pi-ai/dist/api/simple-options.js):and the clamp subtracts the used context plus a 4096-token safety margin:
So the cap on the wire is
min(model.maxTokens, contextWindow − used − 4096). The configured value is correct only while the context has room.The payload spells the cap differently per dialect —
max_tokens(anthropic-messages, openai-completions, mistral),max_completion_tokens(newer OpenAI models),max_output_tokens(openai-responses, azure),config.maxOutputTokens(Google),inferenceConfig.maxTokens(Bedrock),options.maxTokens(pi-messages) — sofindNumberaccepts any of them, flat or nested, and never descends into message arrays.Why
thinking_levelcomes from the contextsetThinkingLevelalready clamps to the model's capabilities (["off"]for non-reasoning models) and re-clamps on a model switch, soctx.thinkingLevelis the effective level. Verified against the wire: it matches thereasoning_effortpi sends on every call. The payload would carry whichever effort or budget field the provider maps it to, which is not comparable across providers. The level is reported only whenmodel.reasoningis true and it is notoff, so a stale level cannot produce a misleading value.Robustness
extractModelParameterscannot throw. Otherwise a payload with a throwing getter would abort the argument evaluation ofstartObservation, leavingstate.openGenerationunassigned. pi catches extension errors per handler (core/extensions/runner.js), so the session would survive — but the generation, its usage and its cost would never reach Langfuse, andassistant_indexwould drift for the rest of the turn, silently.Tests
pnpm typecheckclean.pnpm test75/75, up from 62 onmain.off, no model / no cap cases); caps that are not positive whole numbers rejected (NaN,Infinity, negative, fractional, string); no descent into message arrays; no throw on a hostile payload.thinking_levelwithreasoning: trueanddefaultThinkingLevel: "high"; and a filled-context run asserting the clamped cap is what gets reported. That last test fails when the cap is read fromctx.model.maxTokensand passes here.