Fix: BYOK with llama-stack as a library

[Akrog]

Description

When using a BYOK rag database and llama-stack as a library the service blows up on start with:

yaml.constructor.ConstructorError: could not determine a constructor for the tag
'tag:yaml.org,2002:python/object/apply:pathlib._local.PosixPath'
  in "<unicode string>", line 122, column 16:
          db_path: !!python/object/apply:pathlib._l ...

And we can see in /tmp/llama_stack_enriched_config.yaml:

storage:
  backends:
    byok_rag_1_storage:
      db_path: !!python/object/apply:pathlib._local.PosixPath
        - /home/geguileo/osls/rags/llamastack/rag/vector_db/os_product_docs/faiss_store.db
      type: kv_sqlite

This is caused by how we define the db_path in our model and how it is being used:

In src/models/config.py we define it as a FilePath:

   1563     db_path: FilePath = Field(
   1564         ...,
   1565         title="DB path",
   1566         description="Path to RAG database.",
   1567     )

And in src/llama_stack_configuration.py we use it as a str:

    138     # add new backends for each BYOK RAG
    139     for brag in byok_rag:
    140         vector_db_id = brag.get("vector_db_id", "")
    141         backend_name = f"byok_{vector_db_id}_storage"
    142         output[backend_name] = {
    143             "type": "kv_sqlite",
    144             "db_path": brag.get("db_path", f".llama/{vector_db_id}.db"),
    145         }

This patch fixes this in the simplest way, making the field in the BYOK model match the other existing db_path field, in other words, changing it to a str.

Type of change

• Bug fix

Tools used to create PR

None

Checklist before requesting a review

• I have performed a self-review of my code.
• PR has passed all pre-merge test jobs.

Testing

• I have manually verified that /tmp/llama_stack_enriched_config.yaml looks good and the service no longer blows up.

Summary by CodeRabbit

• Refactor
  • Configuration database path field updated to accept string input instead of Path objects. This modifies validation behavior for RAG database path specifications and adjusts how paths are processed and stored in system configurations.

[coderabbitai]

Walkthrough

The ByokRag.db_path field type is changed from FilePath to str in the configuration model, removing Pydantic's FilePath validation. Tests are updated to reflect the new string type for comparisons.

Changes

Cohort / File(s)	Summary
ByokRag Config Type Change src/models/config.py, tests/unit/models/config/test_byok_rag.py	Updated db_path field type from FilePath to str in the ByokRag model, removing Pydantic path validation. Test assertions updated to work with string comparisons instead of Path objects.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Fix: BYOK with llama-stack as a library' is partially related to the changeset. It addresses the BYOK issue mentioned in the PR objectives but doesn't clearly convey that the fix involves changing db_path from FilePath to str, which is the actual core change in the code.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

tests/unit/models/config/test_byok_rag.py (1)

35-43: ⚠️ Potential issue | 🟡 Minor

Outdated docstring: db_path is no longer converted to a Path.

The docstring at lines 41-42 states "db_path is converted to a Path", but with the type change from FilePath to str, this is no longer accurate. The value remains a string.

📝 Proposed fix

 def test_byok_rag_configuration_nondefault_values() -> None:
     """Test the ByokRag constructor.

     Verify that ByokRag class accepts and stores non-default configuration values.

     Asserts that rag_id, rag_type, embedding_model, embedding_dimension, and
-    vector_db_id match the provided inputs and that db_path is converted to a
-    Path.
+    vector_db_id match the provided inputs and that db_path is stored as a string.
     """

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/unit/models/config/test_byok_rag.py` around lines 35 - 43, Update the
outdated docstring in test_byok_rag_configuration_nondefault_values: remove or
reword the sentence claiming "db_path is converted to a Path" because db_path is
now a str (FilePath → str) and remains a string; reference the ByokRag
constructor and the db_path attribute in the docstring so the test description
correctly states that db_path remains a string rather than being converted to a
Path.

🧹 Nitpick comments (2)

tests/unit/models/config/test_byok_rag.py (1)

62-73: Consider using string literals for db_path consistently across all tests.

Some tests (lines 24, 51) pass db_path as a string, while others (lines 72, 88, 111, 127, 143) still pass Path("tests/configuration/rag.txt"). While Pydantic will coerce Path to str, using consistent string literals would better reflect the model's actual str type and improve test clarity.

♻️ Example fix for one test

     with pytest.raises(ValidationError, match="should be greater than 0"):
         _ = ByokRag(
             rag_id="rag_id",
             rag_type="rag_type",
             embedding_model="embedding_model",
             embedding_dimension=-1024,
             vector_db_id="vector_db_id",
-            db_path=Path("tests/configuration/rag.txt"),
+            db_path="tests/configuration/rag.txt",
         )

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/unit/models/config/test_byok_rag.py` around lines 62 - 73, Update the
test to pass db_path as a string literal instead of a Path object for
consistency with other tests: in test_byok_rag_configuration_wrong_dimension
replace Path("tests/configuration/rag.txt") with the string
"tests/configuration/rag.txt" when constructing the ByokRag instance so db_path
usage across tests matches the model's str type (affects
test_byok_rag_configuration_wrong_dimension and any other tests still using
Path(...)).

src/models/config.py (1)

1562-1566: Consider adding min_length=1 to prevent empty path strings.

Other string fields in ByokRag (e.g., rag_id, rag_type, embedding_model, vector_db_id) have min_length=1 validation. An empty db_path would likely cause runtime errors when llama-stack attempts to use it.

🛡️ Proposed enhancement

     db_path: str = Field(
         ...,
+        min_length=1,
         title="DB path",
         description="Path to RAG database.",
     )

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/models/config.py` around lines 1562 - 1566, The db_path Field in the
ByokRag model lacks a min_length constraint, allowing empty strings that can
cause runtime errors; update the db_path Field declaration (the db_path: str =
Field(...)) to include min_length=1 to match validations used by rag_id,
rag_type, embedding_model, and vector_db_id so empty path strings are rejected
at validation time.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@tests/unit/models/config/test_byok_rag.py`:
- Around line 35-43: Update the outdated docstring in
test_byok_rag_configuration_nondefault_values: remove or reword the sentence
claiming "db_path is converted to a Path" because db_path is now a str (FilePath
→ str) and remains a string; reference the ByokRag constructor and the db_path
attribute in the docstring so the test description correctly states that db_path
remains a string rather than being converted to a Path.

---

Nitpick comments:
In `@src/models/config.py`:
- Around line 1562-1566: The db_path Field in the ByokRag model lacks a
min_length constraint, allowing empty strings that can cause runtime errors;
update the db_path Field declaration (the db_path: str = Field(...)) to include
min_length=1 to match validations used by rag_id, rag_type, embedding_model, and
vector_db_id so empty path strings are rejected at validation time.

In `@tests/unit/models/config/test_byok_rag.py`:
- Around line 62-73: Update the test to pass db_path as a string literal instead
of a Path object for consistency with other tests: in
test_byok_rag_configuration_wrong_dimension replace
Path("tests/configuration/rag.txt") with the string
"tests/configuration/rag.txt" when constructing the ByokRag instance so db_path
usage across tests matches the model's str type (affects
test_byok_rag_configuration_wrong_dimension and any other tests still using
Path(...)).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 642c1707-1543-4b55-864e-547f2b378c14

📥 Commits

Reviewing files that changed from the base of the PR and between 4380346 and c868e96.

📒 Files selected for processing (2)

• src/models/config.py
• tests/unit/models/config/test_byok_rag.py

[tisnik]

@Akrog Path can be converted to string automatically. Does it mean, that your path is not the real path?

[tisnik]

LGTM