Revert "fix: default all LLM backends to deterministic greedy decoding" - #428
Conversation
|
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.678571428571429) 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.dev20260804025407 |
There was a problem hiding this comment.
🟡 Not ready to approve
It introduces at least one functional bug (Gemini max token limit parameters are ignored) and the paper now describes FAISS alignment as approximate/sub-linear despite the implementation using an exact IndexFlatIP scan.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Reverts the prior change that defaulted all LLM backends to deterministic greedy decoding, in order to restore the previous execution behavior across the extraction/alignment flow.
Changes:
- Removes deterministic decoding defaults (
DEFAULT_TEMPERATURE,DEFAULT_TOP_P) and associated temperature/seed plumbing across router/service/extractor and backend helpers. - Updates backend request/payload construction to no longer force deterministic decoding parameters (varies by backend).
- Removes the determinism-focused test module and updates project metadata/docs (pytest markers, paper text) accordingly.
File summaries
| File | Description |
|---|---|
| tests/test_determinism.py | Removes determinism and cross-backend consistency test suite. |
| pytest.ini | Drops the determinism marker entry (keeps other markers). |
| paper.md | Updates narrative around backend coverage and alignment approach; currently mismatched with implementation details. |
| laiser/skill_extractor_refactored.py | Removes temperature/seed arguments and forwarding to the service layer. |
| laiser/services.py | Removes temperature/seed storage/forwarding into the router initialization path. |
| laiser/llm_models/openai.py | Removes temperature/top_p parameters from the public helper and payload construction. |
| laiser/llm_models/llm_router.py | Simplifies router init and stops forwarding temperature/seed into backend calls. |
| laiser/llm_models/llama_cpp_handler.py | Reverts deterministic defaults/seed handling; changes defaults and removes per-call seeding. |
| laiser/llm_models/hugging_face_llm.py | Removes temperature/seed-driven deterministic generation paths; simplifies sampling params. |
| laiser/llm_models/gemini.py | Removes temperature/seed parameters from the public API; retains fixed temperature config and exposes max token parameters (currently not fully applied without fixes). |
| laiser/llm_models/anthropic.py | Removes temperature/top_p parameters from the public helper and request payload. |
| laiser/llm_methods.py | Forces sampling on for HF generation calls and simplifies vLLM sampling params. |
| laiser/config.py | Removes deterministic decoding defaults while keeping GENERATION_SEED. |
Review details
Suppressed comments (1)
laiser/llm_models/gemini.py:81
gemini_generate()acceptsmax_output_tokensbut doesn’t forward it intoGeminiAPI, so the argument has no effect.
client = GeminiAPI(api_key=api_key, model_name=model_name, timeout=timeout)
- Files reviewed: 13/13 changed files
- Comments generated: 3
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
📦 Preview package published! Install and test this PR: pip install --index-url https://test.pypi.org/simple/ laiser-preview==0.5.dev20260804040738 |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
📦 Preview package published! Install and test this PR: pip install --index-url https://test.pypi.org/simple/ laiser-preview==0.5.dev20260804040833 |
|
📦 Preview package published! Install and test this PR: pip install --index-url https://test.pypi.org/simple/ laiser-preview==0.5.dev20260804040953 |
Reverts #426 to fix pre-existing issues in the overall execution flows.