fix: default all LLM backends to deterministic greedy decoding - #426
Merged
Conversation
Every backend previously inherited its provider's default temperature. Set temperature 0.0 and wire GENERATION_SEED through the router to all backends that accept one. Expose temperature and seed on SkillExtractorRefactored. Add tests/test_determinism.py covering decoding defaults, request payloads, router forwarding, and alignment stability. Correct the paper's FAISS description from approximate to exact and add a reference configuration to the README. Addresses JOSS review issue #424.
|
Code Hygiene The following items are defined but not yet in active use. Routine cleanup — no impact on functionality.
|
|
Complexity Analysis Average: C — Moderate (14.0) Hotspots tracked below. These are functional, well-exercised components — listed for visibility as the codebase scales.
|
|
📦 Preview package published! Install and test this PR: pip install --index-url https://test.pypi.org/simple/ laiser-preview==0.5.dev20260728033459 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR standardizes deterministic LLM decoding across LAiSER’s supported backends to improve reproducibility (greedy decoding by default) and documents these guarantees for the JOSS review. It also adds a dedicated determinism test suite and updates the paper’s FAISS description to reflect exact retrieval.
Changes:
- Introduces config-level deterministic decoding defaults (
DEFAULT_TEMPERATURE=0.0,DEFAULT_TOP_P=1.0) and a sharedGENERATION_SEED, and threads temperature/seed throughSkillExtractorRefactored→SkillExtractionService→LLMRouter. - Updates OpenAI, Anthropic, Gemini, local Transformers/vLLM, and llama.cpp backends to default to greedy decoding and (where supported) accept/forward a seed.
- Adds
tests/test_determinism.pyand updatespaper.mdwith a reproducibility section and corrected FAISS index description.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_determinism.py |
Adds regression tests for deterministic decoding defaults, request payloads, router forwarding, and alignment stability. |
pytest.ini |
Registers a new determinism marker. |
paper.md |
Updates FAISS discussion (exact vs approximate) and documents reproducibility strategy. |
laiser/skill_extractor_refactored.py |
Exposes temperature and seed on the public extractor API and forwards to services. |
laiser/services.py |
Stores and forwards temperature/seed into the LLM router initialization. |
laiser/llm_models/openai.py |
Adds default deterministic decoding params and includes them in the request payload. |
laiser/llm_models/llm_router.py |
Adds router-level temperature/seed and forwards them to the active backend. |
laiser/llm_models/llama_cpp_handler.py |
Defaults llama.cpp temperature to greedy; forwards seed when supported by installed version. |
laiser/llm_models/hugging_face_llm.py |
Adds deterministic defaults + optional seeding behavior for local Transformers and vLLM. |
laiser/llm_models/gemini.py |
Defaults Gemini config to deterministic temperature and forwards seed when supported. |
laiser/llm_models/anthropic.py |
Adds deterministic default temperature and optional top_p emission. |
laiser/llm_methods.py |
Aligns local generation helpers with deterministic config defaults; vLLM sampling params now include temperature/top_p/seed. |
laiser/config.py |
Adds DEFAULT_TEMPERATURE and DEFAULT_TOP_P (env-overridable) and documents determinism defaults. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
147
to
+151
| if self.backend == "llama_cpp": | ||
| print("LLMRouter: routing request to llama_cpp backend") | ||
| return llama_cpp_chat(prompt, self.llm) | ||
| return llama_cpp_chat( | ||
| prompt, self.llm, temperature=kwargs["temperature"], seed=self.seed | ||
| ) |
Comment on lines
153
to
+156
| print("LLMRouter: routing request to vLLM/transformer backend") | ||
| return llm_generate_vllm(prompt, self.llm) | ||
| return llm_generate_vllm( | ||
| prompt, self.llm, temperature=kwargs["temperature"], seed=self.seed | ||
| ) |
Comment on lines
+19
to
+26
| import importlib | ||
| import inspect | ||
|
|
||
| import pandas as pd | ||
| import pytest | ||
|
|
||
| from laiser.config import DEFAULT_TEMPERATURE, DEFAULT_TOP_P, GENERATION_SEED | ||
|
|
Member
|
LGTM!! Ship it. |
phanindra-max
added a commit
that referenced
this pull request
Aug 4, 2026
LLMRouter.generate() sent every non-Gemini, non-OpenAI, non-llama.cpp request to llm_generate_vllm(), regardless of what _initialize_components() had loaded. On CPU, and whenever vLLM initialization fails, initialization falls back to a Transformers tokenizer and model and leaves self.llm as None, so generation passed None into the vLLM backend and failed. The deterministic decoding defaults added to llm_generate() in #426 were therefore correct but dormant, and paper.md could not claim a working fallback. Local dispatch now branches on what is loaded: the vLLM engine when one is present, the Transformers model otherwise, and LAiSERError when neither is, rather than calling a backend with None. Two defects on that newly reachable path are fixed alongside it: - llm_generate() capped generation at 100 new tokens, which truncates skill-extraction JSON mid-object. It now accepts max_new_tokens and defaults to config.MAX_NEW_TOKENS. - llm_generate() decoded the full output sequence, returning the prompt echoed back along with the completion, so ResponseParser would have received the prompt's own JSON examples alongside the model's answer. Only the newly generated tokens are decoded now, matching the vLLM path. Also addresses the remaining Copilot review comments on #426: - generate() documents that explicit kwargs take precedence, but the llama.cpp and local branches read self.seed directly and discarded any caller-supplied seed. Seed now flows through kwargs.setdefault() like temperature. - The openai_generate ImportError branch defined no fallback, so a missing dependency raised NameError at call time instead of a useful message. - test_config_defaults_are_deterministic asserted against constants that laiser.config reads from LAISER_TEMPERATURE and LAISER_TOP_P at import time, so a runner exporting either turned the guarantee into a false pass. It now reloads the module with those overrides removed. - pytest.ini registered a determinism marker that no test applied; it is now applied at module level. Adds four LLMRouter tests covering vLLM forwarding, Transformers dispatch, caller seed override, and the no-model error. Three of the four fail against the previous router. Documents the reference configuration in README.md, which the description of #426 promised but the diff did not contain. Refs #424, #426.
phanindra-max
pushed a commit
that referenced
this pull request
Aug 4, 2026
…h the code Addresses the two parts of JOSS review issue #424 that PR #426 left untouched, and corrects a backend description that the code has since outgrown. Summary. The reviewer noted it was too technical to be understood by a reader without a technical background. It opened on a two-stage LLM and FAISS pipeline and closed with pip invocations. It now opens on the problem in plain language, that the same skill is described differently in a job posting, a syllabus and a credential, which defeats analysis at scale; explains what a skill taxonomy is before naming one; states who the tool is for and what they get from it; and defers pipeline internals and installation to later sections and the repository. Abbreviations. The reviewer asked for elaborate forms before first use. ESCO, FAISS, NER and spaCy are now introduced in full at first use. KSA, SCQF and LLM were already expanded. Every abbreviation in the manuscript was checked for an expansion at or before its first bare occurrence. Backends. The State of the field section named only vLLM, Transformers and the Gemini API, and claimed automatic fallback between them. The router also dispatches to llama.cpp and the OpenAI API, and the fallback claim was untrue until a649ea2 made the vLLM to Transformers fallback reachable. Both sections now list the backends the router actually dispatches to and state the fallback explicitly. Anthropic is omitted deliberately: anthropic.py exists but is not wired into the router, so naming it would overclaim. The determinism half of #424 was addressed in #426 and a649ea2. Refs #424.
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.
Every backend previously inherited its provider's default temperature. Set temperature 0.0 and wire GENERATION_SEED through the router to all backends that accept one. Expose temperature and seed on SkillExtractorRefactored. Add tests/test_determinism.py covering decoding defaults, request payloads, router forwarding, and alignment stability. Correct the paper's FAISS description from approximate to exact and add a reference configuration to the README.
Addresses JOSS review issue #424.