Follow-up from #823 by @ayaangazali -- thanks again for that PR!
What
core/src/features/llm/llm_module.cpp:1597-1624 (options_from_request) has the same bug class this PR just fixed for RAG: temperature is gated on has_options instead of has_temperature(), and top_k, min_p, seed fall back to 0 whenever the options message (or the individual field) is absent, instead of the documented defaults in idl/llm_options.proto. After #823, rac_rag_query_proto and rac_llm_generate_proto now disagree on a sparse options message: RAG applies 0.7/40, generate applies greedy/0.
Why it matters
Direct proto callers (and any SDK binding that sends a sparse LLMGenerationOptions) get silent greedy decoding on the primary generate path -- the same user-visible bug #823 just fixed for RAG.
Suggested approach
Route LLM generate and RAG query through one shared presence-aware LLMGenerationOptions -> rac_llm_options_t adapter, modelled on rac_vlm_options_from_proto (core/src/foundation/rac_proto_adapters.cpp:344-360), so both paths honor has_temperature()/has_top_k()/etc. consistently.
Done when
Not blocking #823. @ayaangazali, you know this code well now -- you're welcome to take this one if you're interested.
Opened with help from Claude Code and Codex.
Reviewed with help from Claude Code and Codex.
Follow-up from #823 by @ayaangazali -- thanks again for that PR!
What
core/src/features/llm/llm_module.cpp:1597-1624(options_from_request) has the same bug class this PR just fixed for RAG:temperatureis gated onhas_optionsinstead ofhas_temperature(), andtop_k,min_p,seedfall back to0whenever the options message (or the individual field) is absent, instead of the documented defaults inidl/llm_options.proto. After #823,rac_rag_query_protoandrac_llm_generate_protonow disagree on a sparse options message: RAG applies 0.7/40, generate applies greedy/0.Why it matters
Direct proto callers (and any SDK binding that sends a sparse
LLMGenerationOptions) get silent greedy decoding on the primary generate path -- the same user-visible bug #823 just fixed for RAG.Suggested approach
Route LLM generate and RAG query through one shared presence-aware
LLMGenerationOptions -> rac_llm_options_tadapter, modelled onrac_vlm_options_from_proto(core/src/foundation/rac_proto_adapters.cpp:344-360), so both paths honorhas_temperature()/has_top_k()/etc. consistently.Done when
options_from_requestinllm_module.cppuses presence checks (nothas_options) for temperature, top_k, min_p, and seed, matching the RAG fix in fix(rag): an unset temperature or top_k collapses to 0 on the RAG query path #823.Not blocking #823. @ayaangazali, you know this code well now -- you're welcome to take this one if you're interested.
Opened with help from Claude Code and Codex.
Reviewed with help from Claude Code and Codex.