PR #839 - RunanywhereAI/runanywhere-sdks

Description

Same defect as #836, in the RAG expander, four fields wide. Found by comparing every field config_loader.cpp parses against every field each expand_* forwards.

populate_rag parses eleven knobs. expand_rag forwards seven and silently drops four:

parsed by config_loader	forwarded by expand_rag
`embed_model_id`, `llm_model_id`, `rerank_model_id`	yes
`retrieve_k`, `rerank_top`	yes
`vector_store_path`	yes
`prompt_template`	yes
`vector_store`	no
`bm25_k1`	no
`bm25_b`	no
`rrf_k`	no

Operator params are the only converter-to-operator channel, so all four are read out of the YAML, stored on the proto, and discarded.

vector_store is the clearest of the four. It selects the backend that vector_store_path points into (VECTOR_STORE_USEARCH vs VECTOR_STORE_PGVECTOR), and the path is already forwarded on the line above it. Forwarding half of one setting and dropping the other half is not a defensible split.

The other three are retrieval tuning that the repo does implement, just not configurably:

// core/src/features/rag/bm25_index.h:33
static constexpr float k1_ = 1.2f;
static constexpr float b_  = 0.75f;

// core/src/features/rag/rag_fusion.cpp:17
static constexpr float kRRFConstant = 60.0f;

All four now ride the retrieve op, beside k / rerank_top / vector_store_path, guarded so an unset field stays unset.

What this does and does not claim. It makes the four reachable on the same terms as every other knob, so a host-supplied retrieve factory can honour them. It does not make the in-tree BM25/RRF path configurable: those values are static constexpr above, and changing that is retrieval-math work and a separate decision. I would rather forward them and say this plainly than either drop them silently or imply I made them live.

Same reasoning as #836 on the missing in-tree consumer: this file's header states these expansions "intentionally reference engine-backed operator type names without installing factories for them", so operators are host-supplied and interpret params themselves. prompt_template and rerank_top have no in-tree reader either.

VectorStore_Name() is used for the enum so the param carries the canonical generated spelling rather than an integer a host would have to map back.

Type of Change

Bug fix
New feature
Documentation update
Refactoring

Testing

Lint passes locally
Added/updated tests for changes

Added rag_retrieval_params_reach_the_retrieve_operator to core/tests/test_solution_runner.cpp, beside the existing rag_solution_compiles. It sets all four plus vector_store_path, then asserts the path and the type travel together and that the three tuning values land with their given values.

$ cmake -B build -DRAC_BUILD_TESTS=ON -DCMAKE_BUILD_TYPE=Debug && cmake --build build -j 10
build exit=0
$ ctest --test-dir build -j 10
100% tests passed out of 101

Fails on the unfixed code. Verified by removing only the forwarding block, with the relink confirmed first:

[100%] Linking CXX executable test_solution_runner
[FAIL] core/tests/test_solution_runner.cpp:1125 store != params.end()

On lint: unchecked because core/scripts/lint-cpp.sh needs the repo's pinned clang-format and only Apple clang-format 21 is available here. Both files are completely clean under it (zero diff hunks).

The edited block is inside #if defined(RAC_HAVE_PROTOBUF); protobuf-off builds do not compile it.

No platform boxes ticked: commons-only, exercised through the C++ suite on macOS.

Labels

SDKs:

Commons - Changes to shared native code (core)

Checklist

Code follows project style guidelines
Self-review completed
Documentation updated (if needed)

Summary by CodeRabbit

Bug Fixes
- RAG retrieval configurations now correctly pass vector store and ranking settings to retrieval, including BM25 and reciprocal rank fusion parameters.
- Non-default retrieval tuning values are preserved when solutions are expanded.
- Configured vector store paths and ranking values are now consistently honored during retrieval, improving the accuracy and predictability of RAG results.