fix: test(mocks) resolve Python truthiness and isinstance traps

[aryaMehta26]

Summary

• Fixed Python truthiness and isinstance traps in test mocks for ChromaDB, psycopg2, and Weaviate.
• Resolved an issue where empty lists [] or integers like 0 were incorrectly evaluating to False (e.g., if ids: vs if ids is not None:).
• Fixed an isinstance trap where False matched isinstance(x, int) since booleans are a subclass of integers in Python.
• Fixed a bug where False in params evaluated to True if params contained a 0 value.

Type

fix

Testing

• Tests added or updated
• Tested locally
• ./builder test passes

Checklist

• Commit messages follow conventional commits
• No secrets or credentials included
• Wiki updated (if applicable)
• Breaking changes documented (if applicable)

Linked Issue

Fixes #747

Summary by CodeRabbit

• Bug Fixes
  • Improved accuracy of test infrastructure for database and search operations.
  • Enhanced handling of edge cases with empty inputs and falsy values in mock services.

[coderabbitai]

📝 Walkthrough

Walkthrough

Three test mock modules are updated to fix Python truthiness bugs: chromadb's get/delete/update methods now check ids is not None instead of relying on falsy evaluation; psycopg2's limit detection, parameter parsing, and filter logic use explicit null/identity checks instead of truthiness; Weaviate's like-operator matching treats only None as missing values.

Changes

Falsy-value semantics in test mocks

Layer / File(s)	Summary
chromadb mock: empty ids list handling nodes/test/mocks/chromadb/__init__.py	MockCollection.get, delete, and update now check ids is not None instead of truthiness, so an explicitly provided empty ids list results in no matches, no deletions, or no updates rather than falling back to where-based filtering.
psycopg2 mock: limit and boolean type handling nodes/test/mocks/psycopg2/__init__.py	Select query limit condition, semantic-search parameter interpretation, limit extraction helper, and isdeleted filter all use explicit null and boolean identity checks: limit is not None, exclude booleans from integer type checks, and p is False instead of membership testing.
weaviate mock: falsy property value handling nodes/test/mocks/weaviate/__init__.py	Filter.matches() for the like operator checks prop_value is not None instead of relying on truthiness, so falsy-but-valid property values like 0 or "" are no longer treated as missing.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• rocketride-org/rocketride-server#780: Overlaps on falsy-value handling fixes in the psycopg2 mock's semantic-search and limit extraction logic, including boolean-vs-integer detection.

Suggested labels

module:nodes

Suggested reviewers

• jmaionchi
• stepmikhaylov
• Rod-Christensen

Poem

🐰 A rabbit's ode to truthiness tamed:

False and zero once confused,
is not None now used,
Where mock and real align,
Empty lists shine,
Tests and truth — no longer bruised!

🚥 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 describes the main change: fixing Python truthiness and isinstance traps in test mocks, which accurately reflects the core objective of this PR.
Linked Issues check	✅ Passed	All code changes directly address the three specific bugs identified in issue #747: psycopg2 isinstance(bool,int) trap, chromadb limit=0 truthiness issue, and weaviate 0.0 distance sorting, with corresponding fixes in each mock file.
Out of Scope Changes check	✅ Passed	All changes are narrowly scoped to fixing the three specific truthiness and isinstance bugs in the test mocks identified in issue #747; no extraneous modifications are present.
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

---

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.}

[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 (2)

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

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

limit=0 is still handled as “no limit” in get().

if limit and ... at Line 185 still relies on truthiness, so an explicit limit=0 returns rows instead of none. Use an explicit is not None check (and ideally early-return for zero).

Proposed fix

     def get(
         self,
         ids: List[str] = None,
         where: Optional[Dict] = None,
         where_document: Optional[Dict] = None,
         include: List[str] = None,
         limit: int = None,
         offset: int = 0,
     ) -> Dict[str, Any]:
         """Get items from the collection."""
         if include is None:
             include = ['metadatas', 'documents']

         results = {'ids': [], 'metadatas': [], 'documents': [], 'embeddings': []}
+        if limit == 0:
+            return results
@@
-            if limit and len(results['ids']) >= limit:
+            if limit is not None and len(results['ids']) >= limit:
                 break

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@nodes/test/mocks/chromadb/__init__.py` around lines 153 - 186, The loop in
the mock Chroma get() treats limit=0 as no limit because it uses truthiness (the
`if limit and len(results['ids']) >= limit` check); update the logic in the
method (the loop that appends to results and currently checks `if limit and
...`) to explicitly handle limit being None vs zero: if limit is 0, early-return
or stop adding results immediately (return empty), otherwise use an explicit
check like `if limit is not None and len(results['ids']) >= limit` to enforce
the limit; adjust the limit handling in the same function where `results['ids']`
are appended so zero behaves as “no rows” and None behaves as “no limit.”

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

280-282: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Distance sorting still treats 0.0 as falsy and de-prioritizes exact matches.

The sort key uses truthiness (if x.metadata.distance else 1.0), so 0.0 is converted to 1.0. This breaks exact-match-first ordering.

Proposed fix

-        results.sort(key=lambda x: x.metadata.distance if x.metadata.distance else 1.0)
+        results.sort(key=lambda x: x.metadata.distance if x.metadata.distance is not None else 1.0)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@nodes/test/mocks/weaviate/__init__.py` around lines 280 - 282, The sort key
currently treats 0.0 as falsy in results.sort(key=lambda x: x.metadata.distance
if x.metadata.distance else 1.0), causing exact zero-distance matches to be
replaced with 1.0; change the conditional to check explicitly for None (e.g.,
use "if x.metadata.distance is not None" or "if x.metadata.distance is None then
use a large value") so that 0.0 remains a valid low distance when sorting by
x.metadata.distance.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@nodes/test/mocks/chromadb/__init__.py`:
- Around line 153-186: The loop in the mock Chroma get() treats limit=0 as no
limit because it uses truthiness (the `if limit and len(results['ids']) >=
limit` check); update the logic in the method (the loop that appends to results
and currently checks `if limit and ...`) to explicitly handle limit being None
vs zero: if limit is 0, early-return or stop adding results immediately (return
empty), otherwise use an explicit check like `if limit is not None and
len(results['ids']) >= limit` to enforce the limit; adjust the limit handling in
the same function where `results['ids']` are appended so zero behaves as “no
rows” and None behaves as “no limit.”

In `@nodes/test/mocks/weaviate/__init__.py`:
- Around line 280-282: The sort key currently treats 0.0 as falsy in
results.sort(key=lambda x: x.metadata.distance if x.metadata.distance else 1.0),
causing exact zero-distance matches to be replaced with 1.0; change the
conditional to check explicitly for None (e.g., use "if x.metadata.distance is
not None" or "if x.metadata.distance is None then use a large value") so that
0.0 remains a valid low distance when sorting by x.metadata.distance.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c2caba8f-e0ed-4f17-89dc-45a5a5f4c18a

📥 Commits

Reviewing files that changed from the base of the PR and between 980be0a and 7d54f08.

📒 Files selected for processing (3)

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