Dynamic output-token cap + determinism invariants (api/vLLM mode)#1
Merged
Merged
Conversation
…i mode); fix --llm-timeout 0 to mean no timeout
There was a problem hiding this comment.
Pull request overview
This PR adds a per-request dynamic max_tokens cap for API/vLLM-based translation requests to prevent runaway generations, while also enforcing deterministic translation settings in the API client and correcting the semantics of --llm-timeout 0.
Changes:
- Introduces
token_budget.rsto compute a dynamic output-token cap based on input length with floor/ceiling clamps. - Updates the API translation path to compute and pass a per-request
max_tokenscap into the LLM client. - Enforces
temperature=0andchat_template_kwargs.enable_thinking=falsein API requests, and fixes timeout=0 to mean “no timeout”.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| ltengine/src/token_budget.rs | Adds dynamic token budgeting logic plus unit tests. |
| ltengine/src/main.rs | Wires dynamic cap configuration/CLI flags and passes per-request cap into API prompt execution. |
| ltengine/src/llm_api.rs | Adds per-request max_tokens, enforces determinism invariants, warns on truncation, and fixes timeout=0 behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| floor: args.llm_max_tokens_floor, | ||
| ceiling: if args.llm_max_tokens > 0 { Some(args.llm_max_tokens) } else { None }, | ||
| }; | ||
| let cap = token_budget::dynamic_output_cap(q.chars().count(), &budget_cfg); |
Comment on lines
24
to
32
| #[derive(Serialize)] | ||
| struct ChatCompletionRequest { | ||
| model: String, | ||
| messages: Vec<ChatMessage>, | ||
| temperature: f32, | ||
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| max_tokens: Option<u32>, | ||
| chat_template_kwargs: ChatTemplateKwargs, | ||
| } |
Comment on lines
+26
to
+28
| if cfg.output_mult <= 0.0 || cfg.chars_per_token <= 0.0 { | ||
| return None; | ||
| } |
Comment on lines
+89
to
+92
| /// Dynamic cap: conservative characters-per-token divisor | ||
| #[cfg(feature = "api")] | ||
| #[arg(long, default_value_t = 2.0)] | ||
| llm_chars_per_token: f32, |
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.
Per-request dynamic max_tokens for api mode (new token_budget.rs): cap = clamp(ceil(chars/CHARS_PER_TOKEN)*MULT, FLOOR, ceiling), with None -> static fallback. Enforces temperature=0 + chat_template_kwargs.enable_thinking=false in LTEngine (no longer relying on vLLM serve flags). Warns on finish_reason==length. Fixes --llm-timeout 0 to mean no timeout. Calibrated on gemma-4-26b (3090): cpt=4.0, mult=3.0, floor=64; validated live (cut an en->et 5000-char runaway at 3750 tokens vs 16384). Local llama.cpp mode untouched.