fix(kb): include provider parameters in cache key

[adityasingh2400]

Summary

• Include all embedding search provider parameters in the knowledge base cache hash.
• Prevent stale Annoy cache reuse when parameters other than embedding_engine / embedding_model change, such as Gemini dimensionality.
• Keep deterministic cache keys by JSON-serializing provider parameters with sorted keys.

Fixes #1767

Verification

• python3 -m compileall nemoguardrails/kb/kb.py

PR overlap check

• Searched open PRs for 1767; no existing open PR covers this issue.

Summary by CodeRabbit

• Refactor
  • Improved the knowledge base indexing cache key generation mechanism to ensure more consistent behavior when handling different embedding provider configurations, enabling better index reuse and caching efficiency.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: bb9e8064-050e-4ece-ad7e-ebe346150459

📥 Commits

Reviewing files that changed from the base of the PR and between 6a8fdf3 and 1ddead7.

📒 Files selected for processing (1)

• nemoguardrails/kb/kb.py

---

📝 Walkthrough

Walkthrough

Modified the cache key generation in KnowledgeBase.build() to derive hash_prefix from a JSON-serialized representation of all embedding search provider parameters, replacing the previous concatenation of only engine and model names. This ensures cache invalidation when parameter configurations change.

Changes

Hash Prefix Calculation for Cache Invalidation

Layer / File(s)	Summary
Import Dependencies nemoguardrails/kb/kb.py (line 18)	Added import json to support JSON serialization of parameters.
Hash Prefix Calculation nemoguardrails/kb/kb.py (lines 116–122)	Replaced embedding_engine and embedding_model concatenation with json.dumps(self.config.embedding_search_provider.parameters, sort_keys=True, default=str) to capture all provider parameters in the cache key.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Test Results For Major Changes	⚠️ Warning	PR modifies cache key generation logic with no documented test results. Only a Python compilation check is mentioned. No unit or integration test results provided despite affecting cache behavior.	Provide test results showing: (1) unit tests verify cache invalidation with parameter changes, (2) existing tests pass without regression, (3) validation of cache behavior with provider parameters.
Linked Issues check	❓ Inconclusive	The PR addresses issue #1767 by including embedding provider parameters in cache key calculation, which prevents stale cache reuse when parameters change. However, it does not address the Pydantic validation concern mentioned in the issue.	Verify whether the Pydantic validation requirement from issue #1767 is addressed separately or if this PR should also include validation-stage handling of configuration mutations.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: modifying the cache key to include provider parameters in the knowledge base.
Out of Scope Changes check	✅ Passed	All changes are directly related to the cache key generation for the knowledge base, which aligns with the PR objectives and linked issue requirements.
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

---

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

[greptile-apps]

Greptile Summary

This PR fixes stale Annoy cache reuse by replacing the partial cache-key prefix (only two specific fields) with a full serialization of all embedding search provider parameters, so any parameter change — including Gemini's dimensionality — correctly invalidates the index cache.

• kb.py: The hash prefix now serializes the entire provider parameters dict with sorted keys, ensuring cache-key uniqueness across all provider configurations. A new json import is added.
• tests/test_kb_cache.py: New async test that patches compute_hash and asserts all three parameters appear in the hash input string.

Confidence Score: 5/5

Safe to merge — the fix is a minimal, well-scoped change that makes cache invalidation more correct, and the test validates the key behavior.

The core logic change is straightforward and correct: serializing all provider parameters with sorted keys produces a deterministic, complete hash prefix. The only concern is a test side effect that creates real filesystem artifacts, which does not affect production behavior.

tests/test_kb_cache.py — the test lacks mocking of filesystem I/O, causing it to create real files in a .cache directory on every run.

Important Files Changed

Filename	Overview
nemoguardrails/kb/kb.py	Replaces partial hash prefix with full JSON serialization of all provider parameters; import json added. Change is deterministic and handles non-serializable types via default=str.
tests/test_kb_cache.py	New test verifying all provider parameters appear in the hash input; leaves real .cache directory/file artifacts on the filesystem due to incomplete mocking of I/O in the save path.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[kb.build called] --> B{index_items empty?}
    B -- yes --> Z[return]
    B -- no --> C[Serialize all provider parameters with sorted keys]
    C --> D[Concatenate prefix with all text items]
    D --> E[compute hash value]
    E --> F[Derive .ann and .esize cache file paths]
    F --> G{provider is default AND both cache files exist?}
    G -- yes --> H[Load AnnoyIndex from disk - Skip re-embedding]
    G -- no --> I[Build index via embedding provider]
    I --> J{provider is default?}
    J -- yes --> K[Persist .ann and .esize to disk]
    J -- no --> L[Done]
    K --> L
    H --> L

Loading

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
tests/test_kb_cache.py:46-52
**Test leaves real filesystem artifacts**

The test patches `compute_hash` but does not patch `os.makedirs`, `os.path.exists`, or `open`. When the provider name is `"default"` and the cache files don't exist, `build()` falls into the save path, which calls `os.makedirs(CACHE_FOLDER, exist_ok=True)` and `open(embedding_size_file, "w")` — creating a real `.cache/` directory and a `.cache/cache-key.esize` file on every test run. Consider using `pytest`'s `tmp_path` fixture to redirect `CACHE_FOLDER`, or patch `open`/`os.makedirs` to keep the test hermetic.

_{Reviews (3): Last reviewed commit: "test(kb): cover provider parameters in c..." | Re-trigger Greptile}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!