Skip to content

Commit 1123cdb

Browse files
are-cesclaude
andcommitted
LCORE-1426: address code review feedback
- Add SUPPORTED_RAG_BACKENDS constant and field validator on RagStore.backend to reject unsupported backends at config load time - Add model validator on RagConfiguration to reject retrieval source IDs not declared in byok.stores - Add assert that DEFAULT_RAG_BACKEND has a BACKEND_TO_LLAMA_STACK_PROVIDER mapping - Remove llama-stack references from docs, use run.yaml instead - Add backward compatibility note to RAG guide - Update tests for new validators Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 8cfb347 commit 1123cdb

7 files changed

Lines changed: 95 additions & 14 deletions

File tree

docs/byok_guide.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ byok_rag:
317317

318318
> [!NOTE]
319319
> pgvector is not yet supported via `byok_rag` in `lightspeed-stack.yaml` (see [LCORE-2437](https://redhat.atlassian.net/browse/LCORE-2437)).
320-
> It must be configured directly in the Llama Stack configuration file.
320+
> It must be configured directly in the `run.yaml` configuration file.
321321

322322
```yaml
323323
vector_io:
@@ -371,11 +371,11 @@ rag:
371371

372372
### Example 2: Multiple Knowledge Sources with pgvector
373373

374-
A configuration combining a local FAISS store (via `byok_rag`) with a remote pgvector store (configured directly in the Llama Stack configuration file):
374+
A configuration combining a local FAISS store (via `byok_rag`) with a remote pgvector store (configured directly in the `run.yaml` configuration file):
375375

376376
> [!NOTE]
377377
> pgvector is not yet supported via `byok_rag` in `lightspeed-stack.yaml` (see [LCORE-2437](https://redhat.atlassian.net/browse/LCORE-2437)).
378-
> The pgvector provider must be configured directly in the Llama Stack configuration file.
378+
> The pgvector provider must be configured directly in the `run.yaml` configuration file.
379379

380380
**`lightspeed-stack.yaml`** — FAISS store and RAG strategy:
381381

@@ -402,7 +402,7 @@ rag:
402402
- local-docs
403403
```
404404

405-
**Llama Stack configuration file** — pgvector provider:
405+
**`run.yaml` configuration file** — pgvector provider:
406406

407407
```yaml
408408
vector_io:

docs/rag_guide.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ Both strategies can be enabled independently via the `rag` section of `lightspee
3636

3737
> [!NOTE]
3838
> **Backward compatibility:** if neither `retrieval.inline.sources` nor `retrieval.tool.sources` is
39-
> configured, all BYOK vector stores registered in llama-stack are automatically exposed as
39+
> configured, all registered vector stores (BYOK and OKP) are automatically exposed as
4040
> Tool RAG (`file_search`). Inline RAG is **not** enabled in this fallback — only Tool RAG.
4141
4242
### Inline RAG chunk flow
@@ -122,7 +122,7 @@ This example shows how to configure a remote PostgreSQL database with the [pgvec
122122

123123
> [!NOTE]
124124
> pgvector is not yet supported via `byok_rag` in `lightspeed-stack.yaml` (see [LCORE-2437](https://redhat.atlassian.net/browse/LCORE-2437)).
125-
> It must be configured directly in the Llama Stack configuration file.
125+
> It must be configured directly in the `run.yaml` configuration file.
126126

127127
> You will need to install PostgreSQL with a matching version to pgvector, then log in with `psql` and enable the extension with:
128128
> ```sql

src/constants.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,9 @@
169169
CACHE_TYPE_NOOP: Final[str] = "noop"
170170

171171
# BYOK RAG
172+
# Backends that have enrichment support in llama_stack_configuration.py
173+
SUPPORTED_RAG_BACKENDS: Final[frozenset[str]] = frozenset({"faiss"})
174+
172175
# Default RAG backend for bring-your-own-knowledge RAG configurations
173176
DEFAULT_RAG_BACKEND: Final[str] = "faiss"
174177

src/llama_stack_configuration.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@
2222
# "pgvector": "remote::pgvector", # TODO(are-ces): add enrichment support
2323
}
2424

25+
assert constants.DEFAULT_RAG_BACKEND in BACKEND_TO_LLAMA_STACK_PROVIDER, (
26+
f"DEFAULT_RAG_BACKEND '{constants.DEFAULT_RAG_BACKEND}' has no entry in "
27+
f"BACKEND_TO_LLAMA_STACK_PROVIDER — add a mapping before changing the default."
28+
)
29+
2530

2631
class YamlDumper(yaml.Dumper): # pylint: disable=too-many-ancestors
2732
"""Custom YAML dumper with proper indentation levels."""

src/models/config.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1779,6 +1779,17 @@ class RagStore(ConfigurationBase):
17791779
description="Storage backend type (e.g. 'faiss').",
17801780
)
17811781

1782+
@field_validator("backend")
1783+
@classmethod
1784+
def validate_backend(cls, value: str) -> str:
1785+
"""Reject unsupported backend values at config load time."""
1786+
if value not in constants.SUPPORTED_RAG_BACKENDS:
1787+
raise ValueError(
1788+
f"Unsupported RAG backend '{value}'. "
1789+
f"Supported backends: {sorted(constants.SUPPORTED_RAG_BACKENDS)}"
1790+
)
1791+
return value
1792+
17821793
embedding_model: str = Field(
17831794
constants.DEFAULT_EMBEDDING_MODEL,
17841795
min_length=1,
@@ -2052,6 +2063,24 @@ class RagConfiguration(ConfigurationBase):
20522063
description="Inline and tool retrieval strategy settings.",
20532064
)
20542065

2066+
@model_validator(mode="after")
2067+
def validate_retrieval_sources(self) -> Self:
2068+
"""Reject retrieval source IDs not declared in byok.stores or OKP."""
2069+
known_ids = {store.rag_id for store in self.byok.stores}
2070+
known_ids.add(constants.OKP_RAG_ID)
2071+
2072+
for strategy_name in ("inline", "tool"):
2073+
strategy = getattr(self.retrieval, strategy_name)
2074+
unknown = set(strategy.sources) - known_ids
2075+
if unknown:
2076+
raise ValueError(
2077+
f"retrieval.{strategy_name}.sources contains unknown RAG IDs: "
2078+
f"{sorted(unknown)}. "
2079+
f"Declared IDs: {sorted(known_ids)}"
2080+
)
2081+
2082+
return self
2083+
20552084

20562085
class RerankerConfiguration(ConfigurationBase):
20572086
"""Reranker configuration for RAG chunk reranking."""

tests/unit/models/config/test_byok_rag.py

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ def test_rag_store_configuration_nondefault_values() -> None:
4848
"""
4949
rag_store = RagStore(
5050
rag_id="rag_id",
51-
backend="custom_backend",
51+
backend="faiss",
5252
embedding_model="embedding_model",
5353
embedding_dimension=1024,
5454
vector_db_id="vector_db_id",
@@ -57,7 +57,7 @@ def test_rag_store_configuration_nondefault_values() -> None:
5757
)
5858
assert rag_store is not None
5959
assert rag_store.rag_id == "rag_id"
60-
assert rag_store.backend == "custom_backend"
60+
assert rag_store.backend == "faiss"
6161
assert rag_store.embedding_model == "embedding_model"
6262
assert rag_store.embedding_dimension == 1024
6363
assert rag_store.vector_db_id == "vector_db_id"
@@ -75,7 +75,7 @@ def test_rag_store_configuration_wrong_dimension() -> None:
7575
with pytest.raises(ValidationError, match="should be greater than 0"):
7676
_ = RagStore(
7777
rag_id="rag_id",
78-
backend="custom_backend",
78+
backend="faiss",
7979
embedding_model="embedding_model",
8080
embedding_dimension=-1024,
8181
vector_db_id="vector_db_id",
@@ -97,7 +97,7 @@ def test_rag_store_configuration_empty_rag_id() -> None:
9797
):
9898
_ = RagStore(
9999
rag_id="",
100-
backend="custom_backend",
100+
backend="faiss",
101101
embedding_model="embedding_model",
102102
embedding_dimension=1024,
103103
vector_db_id="vector_db_id",
@@ -129,6 +129,19 @@ def test_rag_store_configuration_empty_backend() -> None:
129129
)
130130

131131

132+
def test_rag_store_configuration_unsupported_backend() -> None:
133+
"""Test that unsupported backend values are rejected."""
134+
with pytest.raises(ValidationError, match="Unsupported RAG backend"):
135+
_ = RagStore(
136+
rag_id="rag_id",
137+
backend="unsupported",
138+
embedding_model="embedding_model",
139+
embedding_dimension=1024,
140+
vector_db_id="vector_db_id",
141+
db_path="tests/configuration/rag.txt",
142+
)
143+
144+
132145
def test_rag_store_configuration_empty_embedding_model() -> None:
133146
"""Test the RagStore constructor.
134147
@@ -142,7 +155,7 @@ def test_rag_store_configuration_empty_embedding_model() -> None:
142155
):
143156
_ = RagStore(
144157
rag_id="rag_id",
145-
backend="custom_backend",
158+
backend="faiss",
146159
embedding_model="",
147160
embedding_dimension=1024,
148161
vector_db_id="vector_db_id",
@@ -164,7 +177,7 @@ def test_rag_store_configuration_empty_vector_db_id() -> None:
164177
):
165178
_ = RagStore(
166179
rag_id="rag_id",
167-
backend="custom_backend",
180+
backend="faiss",
168181
embedding_model="embedding_model",
169182
embedding_dimension=1024,
170183
vector_db_id="",
@@ -177,7 +190,7 @@ def test_rag_store_configuration_custom_score_multiplier() -> None:
177190
"""Test RagStore with custom score_multiplier."""
178191
rag_store = RagStore(
179192
rag_id="rag_id",
180-
backend="custom_backend",
193+
backend="faiss",
181194
vector_db_id="vector_db_id",
182195
embedding_model="embedding_model",
183196
embedding_dimension=1024,
@@ -192,7 +205,7 @@ def test_rag_store_configuration_score_multiplier_must_be_positive() -> None:
192205
with pytest.raises(ValidationError, match="greater than 0"):
193206
_ = RagStore(
194207
rag_id="rag_id",
195-
backend="custom_backend",
208+
backend="faiss",
196209
vector_db_id="vector_db_id",
197210
embedding_model="embedding_model",
198211
embedding_dimension=1024,

tests/unit/models/config/test_rag_configuration.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,12 @@ def test_default_values(self) -> None:
9797

9898
def test_inline_with_byok_ids(self) -> None:
9999
"""Test inline sources with BYOK rag IDs."""
100+
stores = [
101+
RagStore(rag_id="store-1", vector_db_id="vs_1", db_path="/tmp/s1.db"),
102+
RagStore(rag_id="store-2", vector_db_id="vs_2", db_path="/tmp/s2.db"),
103+
]
100104
config = RagConfiguration(
105+
byok=ByokConfiguration(stores=stores),
101106
retrieval=RetrievalConfiguration(
102107
inline=RetrievalStrategyConfiguration(sources=["store-1", "store-2"]),
103108
),
@@ -107,7 +112,9 @@ def test_inline_with_byok_ids(self) -> None:
107112

108113
def test_inline_with_okp_rag(self) -> None:
109114
"""Test inline sources including the special OKP ID."""
115+
store = RagStore(rag_id="store-1", vector_db_id="vs_1", db_path="/tmp/s1.db")
110116
config = RagConfiguration(
117+
byok=ByokConfiguration(stores=[store]),
111118
retrieval=RetrievalConfiguration(
112119
inline=RetrievalStrategyConfiguration(
113120
sources=[constants.OKP_RAG_ID, "store-1"]
@@ -119,7 +126,9 @@ def test_inline_with_okp_rag(self) -> None:
119126

120127
def test_tool_with_okp_rag_and_byok(self) -> None:
121128
"""Test tool sources with OKP and BYOK IDs."""
129+
store = RagStore(rag_id="store-1", vector_db_id="vs_1", db_path="/tmp/s1.db")
122130
config = RagConfiguration(
131+
byok=ByokConfiguration(stores=[store]),
123132
retrieval=RetrievalConfiguration(
124133
inline=RetrievalStrategyConfiguration(sources=["store-1"]),
125134
tool=RetrievalStrategyConfiguration(
@@ -144,6 +153,28 @@ def test_tool_default_is_empty_list(self) -> None:
144153
config = RagConfiguration()
145154
assert config.retrieval.tool.sources == []
146155

156+
def test_unknown_inline_source_rejected(self) -> None:
157+
"""Test that inline sources referencing undeclared rag_ids are rejected."""
158+
store = RagStore(rag_id="store-1", vector_db_id="vs_1", db_path="/tmp/s1.db")
159+
with pytest.raises(ValidationError, match="unknown RAG IDs"):
160+
RagConfiguration(
161+
byok=ByokConfiguration(stores=[store]),
162+
retrieval=RetrievalConfiguration(
163+
inline=RetrievalStrategyConfiguration(
164+
sources=["store-1", "nonexistent"]
165+
),
166+
),
167+
)
168+
169+
def test_unknown_tool_source_rejected(self) -> None:
170+
"""Test that tool sources referencing undeclared rag_ids are rejected."""
171+
with pytest.raises(ValidationError, match="unknown RAG IDs"):
172+
RagConfiguration(
173+
retrieval=RetrievalConfiguration(
174+
tool=RetrievalStrategyConfiguration(sources=["missing-store"]),
175+
),
176+
)
177+
147178
def test_no_unknown_fields_allowed(self) -> None:
148179
"""Test that RagConfiguration rejects unknown fields."""
149180
with pytest.raises(ValidationError, match="Extra inputs are not permitted"):

0 commit comments

Comments
 (0)