diff --git a/core/src/features/rag/rac_rag_proto_abi.cpp b/core/src/features/rag/rac_rag_proto_abi.cpp index 018291f68a..ed0a7e8ebd 100644 --- a/core/src/features/rag/rac_rag_proto_abi.cpp +++ b/core/src/features/rag/rac_rag_proto_abi.cpp @@ -422,11 +422,18 @@ rac_result_t execute_rag_query(const std::shared_ptr& s, // Base off RAC_LLM_OPTIONS_DEFAULT, then apply the embedded generation // knobs with the RAG pipeline defaults (max 512 tokens, top_p 0.9) when // unset. + // + // temperature and top_k are `optional` in llm_options.proto, so presence + // is what "unset" means: reading the value instead yields 0, which is a + // legal explicit setting for both (greedy / top-k disabled) and therefore + // cannot double as a sentinel. has_generation() is not a substitute -- a + // caller that sets any other generation knob makes the submessage present + // while leaving these two absent. rac_llm_options_t opts = RAC_LLM_OPTIONS_DEFAULT; opts.max_tokens = gen.max_output_tokens() > 0 ? gen.max_output_tokens() : 512; - opts.temperature = query_proto.has_generation() ? gen.temperature() : opts.temperature; + opts.temperature = gen.has_temperature() ? gen.temperature() : opts.temperature; opts.top_p = gen.top_p() > 0.0f ? gen.top_p() : 0.9f; - opts.top_k = gen.top_k(); + opts.top_k = gen.has_top_k() ? gen.top_k() : opts.top_k; opts.disable_thinking = (gen.has_reasoning() && gen.reasoning().mode() == runanywhere::v1::REASONING_MODE_OFF) ? RAC_TRUE diff --git a/core/tests/test_advanced_modality_proto_abi.cpp b/core/tests/test_advanced_modality_proto_abi.cpp index bb1948d59c..26c84bf00a 100644 --- a/core/tests/test_advanced_modality_proto_abi.cpp +++ b/core/tests/test_advanced_modality_proto_abi.cpp @@ -103,6 +103,7 @@ std::atomic g_dummy_embeddings_started{false}; std::atomic g_dummy_embeddings_release{false}; float g_dummy_llm_last_temperature = -1.0f; int32_t g_dummy_llm_last_max_tokens = 0; +int32_t g_dummy_llm_last_top_k = -1; rac_bool_t g_dummy_llm_last_disable_thinking = RAC_FALSE; std::string g_dummy_llm_stream_response; @@ -380,6 +381,7 @@ rac_result_t dummy_llm_stream(void* impl, const char*, const rac_llm_options_t* if (options) { g_dummy_llm_last_temperature = options->temperature; g_dummy_llm_last_max_tokens = options->max_tokens; + g_dummy_llm_last_top_k = options->top_k; g_dummy_llm_last_disable_thinking = options->disable_thinking; } if (g_dummy_llm_block_stream.load(std::memory_order_acquire)) { @@ -1112,6 +1114,51 @@ int test_rag_ingest_query_mocked_path() { CHECK(poll_capability(runanywhere::v1::CAPABILITY_OPERATION_EVENT_KIND_RAG_QUERY_COMPLETED), "RAG query publishes RAG_QUERY_COMPLETED"); + // temperature and top_k are `optional` in llm_options.proto, so a + // generation submessage that sets neither must leave the + // RAC_LLM_OPTIONS_DEFAULT values in place rather than collapse to the + // proto3 zero -- 0.0f and 0 are both legal explicit settings here + // (greedy sampling / top-k disabled), so neither can act as a sentinel. + runanywhere::v1::RAGQueryOptions unset_sampling_query; + unset_sampling_query.set_query("Where does RAG live?"); + unset_sampling_query.mutable_generation()->set_max_output_tokens(32); + std::vector unset_sampling_bytes; + CHECK(serialize(unset_sampling_query, &unset_sampling_bytes), + "RAGQueryOptions with no sampling knobs serializes"); + g_dummy_llm_last_temperature = -1.0f; + g_dummy_llm_last_top_k = -1; + rac_proto_buffer_init(&out); + rc = rac_rag_query_proto(session, unset_sampling_bytes.data(), unset_sampling_bytes.size(), + &out); + CHECK(rc == RAC_SUCCESS, "RAG query with no sampling knobs succeeds"); + CHECK(g_dummy_llm_last_temperature == RAC_LLM_OPTIONS_DEFAULT.temperature, + "RAG query keeps the default temperature when the field is unset"); + CHECK(g_dummy_llm_last_top_k == RAC_LLM_OPTIONS_DEFAULT.top_k, + "RAG query keeps the default top_k when the field is unset"); + rac_proto_buffer_free(&out); + rac_sdk_event_clear_queue(); + + // The other half of the same contract: 0 is a legal explicit top_k + // (top-k filtering disabled), so it has to survive rather than be read as + // "unset". This is what stops the bug from being "fixed" with a + // value-based sentinel such as `gen.top_k() > 0 ? gen.top_k() : default`, + // which would satisfy the unset case above while silently discarding a + // caller's explicit 0. + runanywhere::v1::RAGQueryOptions explicit_zero_query; + explicit_zero_query.set_query("Where does RAG live?"); + explicit_zero_query.mutable_generation()->set_max_output_tokens(32); + explicit_zero_query.mutable_generation()->set_top_k(0); + std::vector explicit_zero_bytes; + CHECK(serialize(explicit_zero_query, &explicit_zero_bytes), + "RAGQueryOptions with an explicit top_k of 0 serializes"); + g_dummy_llm_last_top_k = -1; + rac_proto_buffer_init(&out); + rc = rac_rag_query_proto(session, explicit_zero_bytes.data(), explicit_zero_bytes.size(), &out); + CHECK(rc == RAC_SUCCESS, "RAG query with an explicit top_k of 0 succeeds"); + CHECK(g_dummy_llm_last_top_k == 0, "RAG query honours an explicit top_k of 0"); + rac_proto_buffer_free(&out); + rac_sdk_event_clear_queue(); + // A token-limited thinking phase may omit its closing tag. The RAG result // must keep that private content out of answer while retaining typed // thinking_content for non-UI consumers.