Skip to content

feat: record model parameters on generations - #29

Open
milanagm wants to merge 2 commits into
mainfrom
feat/model-parameters
Open

milanagm wants to merge 2 commits into
mainfrom
feat/model-parameters

Conversation

@milanagm

@milanagm milanagm commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Closes #22.

Change

Generations carried model and metadata.provider and nothing about how the model was called. Two runs of the same prompt at different thinking levels were indistinguishable, and a stop_reason: length arrived without the cap that produced it.

Every generation now carries modelParameters with max_tokens and, for reasoning models with thinking on, thinking_level.

Why max_tokens comes from the request payload

ctx.model.maxTokens is the value configured in models.json. It is not what pi sends. Every provider's streamSimple goes through buildBaseOptions (pi-ai/dist/api/simple-options.js):

maxTokens: clampMaxTokensToContext(model, context, options?.maxTokens ?? model.maxTokens)

and the clamp subtracts the used context plus a 4096-token safety margin:

const available = model.contextWindow - estimateContextTokens(context).tokens - CONTEXT_SAFETY_TOKENS;
return Math.min(maxTokens, Math.max(MIN_MAX_TOKENS, available));

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) — so findNumber accepts any of them, flat or nested, and never descends into message arrays.

Why thinking_level comes from the context

setThinkingLevel already clamps to the model's capabilities (["off"] for non-reasoning models) and re-clamps on a model switch, so ctx.thinkingLevel is the effective level. Verified against the wire: it matches the reasoning_effort pi 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 when model.reasoning is true and it is not off, so a stale level cannot produce a misleading value.

Robustness

extractModelParameters cannot throw. Otherwise a payload with a throwing getter would abort the argument evaluation of startObservation, leaving state.openGeneration unassigned. 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, and assistant_index would drift for the rest of the turn, silently.

Tests

pnpm typecheck clean. pnpm test 75/75, up from 62 on main.

  • Unit: every dialect spelling incl. the nested ones; the clamped-vs-configured distinction; thinking-level gating (reasoning models only, never 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.
  • Integration through the real pi CLI: max tokens on every generation of a plain model; thinking_level with reasoning: true and defaultThinkingLevel: "high"; and a filled-context run asserting the clamped cap is what gets reported. That last test fails when the cap is read from ctx.model.maxTokens and passes here.

@hassiebp hassiebp left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after merge conflicts have been resolved 👍

milanagm and others added 2 commits September 15, 2026 13:11
* feat: trace the remaining request parameters pi puts on the wire

* test: assert sampling parameters against the wire, not the model config
@milanagm
milanagm force-pushed the feat/model-parameters branch from 1e34892 to ed5099e Compare September 15, 2026 11:16

milanagm commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator Author

@hassiebp resolved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Record model parameters (max tokens, thinking level) on generations

2 participants