test: add retriever deterministic unit tests

[kavishkartha05]

Pull Request

Use a Conventional Commit title: feat: ..., fix: ..., docs: ..., chore: ..., refactor: ..., test: ..., build: ..., ci: ..., perf: ..., or revert: ....

Summary

Adds offline unit tests for Retriever.retrieve() verifying the adapter contract: query string reaches embed_fn, the resulting embedding and top_k are forwarded to memory_store.query, and chunks are returned unchanged. Also covers the missing embed_fn error path.

Linked context

Closes #16

PR Size

• Small (< 100 LOC delta)

PR Type

• Test

Context / Motivation

Retriever had no unit tests despite defining a clear contract between the embedding function and memory store. This PR documents and enforces that contract with fully offline tests.

Changes

Added tests/unit/test_retriever.py with 4 tests: normal retrieval path, default top_k=2, multi-chunk passthrough, and RuntimeError on missing embed_fn. Uses a small inline _SpyStore to record calls without any Chroma or live embedding dependency.

How to test

1. uv run pytest -m unit -v
2. All 4 tests in test_retriever.py should pass with no network or Ollama dependency

Checklist

• I ran the relevant local checks.
• I updated documentation or confirmed no docs changes were needed.
• I linked related issues or pull requests and confirmed this is not duplicate work.
• This change stays within the stated scope of the PR.

Notes for reviewers

test_retrieve_default_top_k asserts top_k=2, which follows the current hardcoded default in retriever.py, rather than endorsing it. Exposing top_k through AnchorConfig would be a separate improvement.

Summary by CodeRabbit

• Tests
  • Added unit tests for retrieval behavior: verifies the embedding function is called with the original query, that the resulting embedding and requested top_k are forwarded to the store, that top_k defaults to 2 when omitted, and that constructing without an embedding function raises an error.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 504565ae-9bba-4fde-bbda-ada8af1b88de

📥 Commits

Reviewing files that changed from the base of the PR and between a2db06a and 4d91da5.

📒 Files selected for processing (1)

• tests/unit/test_retriever.py

✅ Files skipped from review due to trivial changes (1)

• tests/unit/test_retriever.py

---

📝 Walkthrough

Walkthrough

Adds a new unit test module tests/unit/test_retriever.py that uses a _SpyStore to assert Retriever.retrieve(): it calls the provided embed function with the raw query, forwards the resulting embedding and top_k to the store's query, returns the store's chunks unchanged, and errors if no embed function.

Changes

Cohort / File(s)	Summary
Retriever Unit Tests tests/unit/test_retriever.py	Adds a test module with a _SpyStore helper and four tests verifying: raw query passed to embed_fn, embedding and top_k forwarded to store.query, default top_k is 2, and missing embed_fn raises RuntimeError containing "No embedding function".

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

needs: linked-context

Suggested reviewers

• AlexanderGardiner

Poem

🐰 I hopped along the test-time trail,

I sniffed each query, left no trail stale,
I watched embeddings take their flight,
I nudged the store with gentle might,
Hooray — the Retriever behaved just right! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ 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: adding deterministic unit tests for the Retriever component.
Linked Issues check	✅ Passed	The pull request fully addresses all coding requirements from issue #16: tests verify embed_fn invocation, embedding/top_k forwarding, chunk passthrough, and missing embed_fn error handling.
Out of Scope Changes check	✅ Passed	All changes are within scope: only a new unit test file is added with no modifications to existing source code or out-of-scope dependencies.

_{✏️ 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.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In `@tests/unit/test_retriever.py`:
- Around line 20-22: The one-line abstract-method stubs (add, delete, get)
trigger Ruff A002 and the invalid noqa E704 suppression; replace each one-line
form like "def add(self, id, ...): ...  # noqa: E704" with a proper multi-line
stub: declare the same signature (keeping the required parameter name id),
remove the invalid "# noqa: E704", add a valid noqa that matches the linter rule
you intend to suppress for using the name "id" (e.g., "# noqa: <correct-rule>"
if you must suppress the builtin-name check), and make the body raise
NotImplementedError (e.g., def add(...): raise NotImplementedError) so Ruff no
longer flags the abstract-method stub or invalid suppression for add, delete,
and get.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 44116fa5-2a46-4df1-85b1-aa1c7370e03c

📥 Commits

Reviewing files that changed from the base of the PR and between 8f9ac15 and a2db06a.

📒 Files selected for processing (1)

• tests/unit/test_retriever.py

[AlexanderGardiner]

Looks good! There is a fake memory store object in unit_harness.py, but these tests don't necessarily need that added complexity.