fix(test/mocks): correct three Python truthiness/isinstance traps

[TrishaReddygari]

Summary

Fixes three pre-existing Python truthiness / isinstance bugs in test mocks, identified by CodeRabbit during review of #744. All three are the same class of bug, applied as one commit per file:

1. nodes/test/mocks/psycopg2/__init__.py — booleans were matching isinstance(p, int) and getting assigned as a SQL LIMIT. Fixed by adding not isinstance(p, bool) at both call sites (_handle_semantic_search line 184 and _extract_limit line 308).

2. nodes/test/mocks/chromadb/__init__.py — if limit and ... treated limit=0 as "no limit," returning all rows instead of zero. Fixed with is not None.

3. nodes/test/mocks/weaviate/__init__.py — sort key collapsed 0.0 cosine distance (exact match) into the 1.0 fallback, ranking exact matches last. Fixed with is not None.

Why this matters

Mock-only bugs are sneaky: tests pass against the mock, real behavior diverges in production, and the gap is hard to spot. Catching the truthiness trap class across all three mocks at once seemed worth doing as one PR.

Testing

Each fix is a one-liner with no behavior change in the cases that previously worked. Existing test suite should remain green; the bugs only manifested in edge cases that no current test exercises.

Breaking changes

None.

Fixes #747

Summary by CodeRabbit

• Bug Fixes
  • Fixed mock query handling so numeric controls correctly treat zero as a valid limit and do not conflate booleans with integers.
  • Improved mock vector search ordering so zero distances are recognized as valid and sorted appropriately.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 46716782-b594-4b26-ab97-a9ea5c6303a9

📥 Commits

Reviewing files that changed from the base of the PR and between 8fa4236 and 2a51dad.

📒 Files selected for processing (1)

• nodes/test/mocks/chromadb/__init__.py

---

📝 Walkthrough

Walkthrough

Three test-mock fixes: ChromaDB now respects limit=0; psycopg2 parsing excludes booleans from integer-limit/parameter detection; Weaviate sorts by distance when it is explicitly not None (so 0.0 sorts correctly). Changes are limited to mock implementations.

Changes

Test Mock Bug Fixes

Layer / File(s)	Summary
Behavior guard (early check) nodes/test/mocks/chromadb/__init__.py	MockCollection.get() now uses limit is not None for early termination so limit=0 yields zero results.
Type-aware parameter parsing nodes/test/mocks/psycopg2/__init__.py	_handle_semantic_search and _extract_limit now require not isinstance(p, bool) when treating int parameters, preventing True/False from being interpreted as numeric controls.
Explicit None check for sorting key nodes/test/mocks/weaviate/__init__.py	MockQuery.near_vector sorts by x.metadata.distance if x.metadata.distance is not None else 1.0, so 0.0 is treated as a valid distance value.
Tests / Docs (none changed)	No test or documentation files were modified in this PR.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related issues

• Python truthiness / isinstance traps in three test mocks #747 — Fixes the same truthiness/isinstance edge cases in these mock files (ChromaDB limit, psycopg2 int-vs-bool, Weaviate distance sorting).

Poem

🐰
I nibble bugs in mockland night,
Where zeros and falses find the light.
Explicit checks now lead the way,
Tests hop happy — hip hooray! 🥕

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically summarizes the main changes: fixing three Python truthiness/isinstance bugs in test mocks across three files.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

_{Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.}

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

[github-actions]

No description provided.

[coderabbitai]

Caution

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

⚠️ Outside diff range comments (1)

nodes/test/mocks/chromadb/__init__.py (1)

186-203: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

limit=0 still returns one row.

The new check runs after appending, so a zero limit still leaks the first item before the loop breaks. Move the limit guard before the append, or special-case 0 up front.

🐛 Proposed fix

         count = 0
         for id, data in items:
             metadata = data.get("metadata", {})
             document = data.get("document", "")
@@
             if count < offset:
                 count += 1
                 continue
+
+            if limit is not None and len(results["ids"]) >= limit:
+                break
 
             results["ids"].append(id)
             if "metadatas" in include:
                 results["metadatas"].append(self._serialize_metadata(metadata))
@@
             if "embeddings" in include:
                 results["embeddings"].append(data.get("embedding"))
 
             count += 1
-            
-            # Apply limit
-            if limit is not None and len(results["ids"]) >= limit:
-                break

🤖 Prompt for AI Agents

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

In `@nodes/test/mocks/chromadb/__init__.py` around lines 186 - 203, The loop
appends an item before enforcing the limit, so passing limit=0 still returns one
row; update the loop in nodes/test/mocks/chromadb/__init__.py (the block that
manipulates count, results, ids, include, metadata, document, data and calls
self._serialize_metadata) to guard against the limit before appending (or handle
limit==0 at the top of the method) — i.e., check if limit is not None and
len(results["ids"]) >= limit (or limit == 0) and break/return prior to pushing
id/metadatas/documents/embeddings to results so no item is emitted when limit is
zero.

🤖 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 `@nodes/test/mocks/chromadb/__init__.py`:
- Around line 186-203: The loop appends an item before enforcing the limit, so
passing limit=0 still returns one row; update the loop in
nodes/test/mocks/chromadb/__init__.py (the block that manipulates count,
results, ids, include, metadata, document, data and calls
self._serialize_metadata) to guard against the limit before appending (or handle
limit==0 at the top of the method) — i.e., check if limit is not None and
len(results["ids"]) >= limit (or limit == 0) and break/return prior to pushing
id/metadatas/documents/embeddings to results so no item is emitted when limit is
zero.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c3ec8356-5940-4c92-9656-770c6b16cd42

📥 Commits

Reviewing files that changed from the base of the PR and between 5d9963f and 8fa4236.

📒 Files selected for processing (3)

• nodes/test/mocks/chromadb/__init__.py
• nodes/test/mocks/psycopg2/__init__.py
• nodes/test/mocks/weaviate/__init__.py

[TrishaReddygari]

Good catch — the v1 fix correctly distinguished None from 0 but missed that the break ran after the append, so limit=0 still leaked one item before bailing. Pushed a follow-up commit moving the limit check ahead of the append. Traced through the three cases: limit=0 returns 0 items, limit=2 returns 2 items, limit=None returns all items.

[asclearuc]

Thanks for tackling these, @TrishaReddygari — three real bugs in one tightly-scoped PR, with clear per-commit messages and a good follow-up after CodeRabbit caught the chromadb append-ordering subtlety. Nice cleanup.

Value Assessment

These are mock-only bugs, but mock-only bugs are the worst kind: tests pass against the mock, real behavior diverges in production, and the gap is invisible until something breaks live. Catching the entire bool ⊂ int / 0 is falsy family across all three vector-store mocks at once is the right call — the maintenance cost of three one-liners is trivial, and bundling them avoids three round-trip reviews of the same class of fix.

[Rod-Christensen]

Hi @TrishaReddygari — thanks for tackling this! Your fix correctly identified all three truthiness/isinstance traps, and the chromadb restructuring (moving the limit check before the append) was a thoughtful touch.

We've merged #780 which addresses the same three bugs with a minimal diff. Closing this one in its favor, but appreciate the contribution!