Conversation
Add human-readable __repr__ to SearchIndex/AsyncSearchIndex (via BaseSearchIndex), SemanticCache, SemanticRouter, MessageHistory, and SemanticMessageHistory. Repr output shows non-sensitive identity fields (name, prefix, storage_type, distance_threshold, ttl, route count, session_tag) and is stable for the same construction parameters.
|
Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset. In case there are security findings, they will be communicated to you as a comment inside the PR. Hope you’ll enjoy using Jit. Questions? Comments? Want to learn more? Get in touch with us. |
Co-authored-by: Vishal Bala <vishal-bala@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds deterministic, human-readable __repr__ methods to several core RedisVL objects to improve debugging/log readability, plus unit tests validating the exact output strings.
Changes:
- Implement
__repr__forBaseSearchIndex(coveringSearchIndex/AsyncSearchIndex). - Implement
__repr__forSemanticCache,SemanticRouter,MessageHistory, andSemanticMessageHistory. - Add
tests/unit/test_repr.pyto assert repr output without requiring Redis/model downloads (via patching/mocking).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
redisvl/index/index.py |
Adds BaseSearchIndex.__repr__ to surface index identity/config details. |
redisvl/extensions/cache/llm/semantic.py |
Adds SemanticCache.__repr__ including threshold/ttl for easier inspection. |
redisvl/extensions/router/semantic.py |
Adds SemanticRouter.__repr__ with route count for quick diagnostics. |
redisvl/extensions/message_history/message_history.py |
Adds MessageHistory.__repr__ showing name/session tag. |
redisvl/extensions/message_history/semantic_history.py |
Adds SemanticMessageHistory.__repr__ including distance threshold. |
tests/unit/test_repr.py |
Introduces unit tests asserting the new deterministic repr strings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/unit/test_repr.py
Outdated
| with ( | ||
| patch( | ||
| "redisvl.extensions.cache.llm.semantic.HFTextVectorizer", | ||
| return_value=mock_vec, | ||
| ), | ||
| patch( | ||
| "redisvl.extensions.cache.llm.semantic.SearchIndex", | ||
| return_value=mock_idx, | ||
| ), | ||
| ): | ||
| yield |
There was a problem hiding this comment.
This fixture uses the parenthesized multi-context-manager form with (..., ...):, which is only supported in Python 3.10+. The project declares support for Python 3.9, so this will raise a SyntaxError in 3.9 test runs. Use nested with statements or contextlib.ExitStack instead.
| with ( | |
| patch( | |
| "redisvl.extensions.cache.llm.semantic.HFTextVectorizer", | |
| return_value=mock_vec, | |
| ), | |
| patch( | |
| "redisvl.extensions.cache.llm.semantic.SearchIndex", | |
| return_value=mock_idx, | |
| ), | |
| ): | |
| yield | |
| with patch( | |
| "redisvl.extensions.cache.llm.semantic.HFTextVectorizer", | |
| return_value=mock_vec, | |
| ): | |
| with patch( | |
| "redisvl.extensions.cache.llm.semantic.SearchIndex", | |
| return_value=mock_idx, | |
| ): | |
| yield |
tests/unit/test_repr.py
Outdated
| with ( | ||
| patch( | ||
| "redisvl.extensions.message_history.semantic_history.HFTextVectorizer", | ||
| return_value=mock_vec, | ||
| ), | ||
| patch( | ||
| "redisvl.extensions.message_history.semantic_history.SearchIndex", | ||
| return_value=mock_idx, | ||
| ), | ||
| ): |
There was a problem hiding this comment.
This fixture uses the parenthesized multi-context-manager form with (..., ...):, which is only supported in Python 3.10+. Since the package supports Python 3.9, this will cause a SyntaxError under 3.9. Rewrite using nested with blocks or contextlib.ExitStack.
|
@tuecoder there is a mismatch between whats in the |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
redisvl/index/index.py
Outdated
| prefix_label = "prefix" | ||
| prefix_value = self.prefix | ||
|
|
||
| prefixes = getattr(self, "prefixes", None) | ||
| if isinstance(prefixes, (list, tuple)) and len(prefixes) != 1: | ||
| prefix_label = "prefixes" | ||
| prefix_value = prefixes | ||
|
|
There was a problem hiding this comment.
__repr__ calls self.prefix before inspecting self.prefixes. If schema.index.prefix is an empty list (allowed by the Union[str, List[str]] type), BaseSearchIndex.prefix will raise IndexError, and then repr(index) will raise as well. Consider deriving the displayed prefix from self.prefixes first (handle 0/1/many) so __repr__ never throws and matches the comment about only falling back when multiple prefixes are configured.
| prefix_label = "prefix" | |
| prefix_value = self.prefix | |
| prefixes = getattr(self, "prefixes", None) | |
| if isinstance(prefixes, (list, tuple)) and len(prefixes) != 1: | |
| prefix_label = "prefixes" | |
| prefix_value = prefixes | |
| prefixes = getattr(self, "prefixes", None) | |
| # Derive the label/value pair from `prefixes` first so we never | |
| # rely on `self.prefix` indexing into an empty collection. | |
| if isinstance(prefixes, (list, tuple)): | |
| if len(prefixes) == 1: | |
| prefix_label = "prefix" | |
| prefix_value = prefixes[0] | |
| elif len(prefixes) == 0: | |
| # No prefixes configured; show the empty collection. | |
| prefix_label = "prefixes" | |
| prefix_value = prefixes | |
| else: | |
| # Multiple prefixes configured; show them all. | |
| prefix_label = "prefixes" | |
| prefix_value = prefixes | |
| else: | |
| # Fall back to the singular `prefix` attribute when `prefixes` | |
| # is not a sequence. Guard against potential IndexError or | |
| # missing attribute so that __repr__ never raises. | |
| prefix_label = "prefix" | |
| try: | |
| prefix_value = self.prefix | |
| except (AttributeError, IndexError): | |
| prefix_label = "prefixes" | |
| prefix_value = prefixes |
tests/unit/test_repr.py
Outdated
| with ( | ||
| patch( | ||
| "redisvl.extensions.message_history.semantic_history.HFTextVectorizer", | ||
| return_value=mock_vec, | ||
| ), | ||
| patch( | ||
| "redisvl.extensions.message_history.semantic_history.SearchIndex", | ||
| return_value=mock_idx, | ||
| ), | ||
| ): | ||
| yield |
There was a problem hiding this comment.
Same Python-version concern here: with ( ... ): requires Python 3.10+, but the project declares Python 3.9 support. If tests are expected to run on 3.9, rewrite this with statement to a 3.9-compatible multi-context-manager pattern (or bump the supported minimum Python version).
| return ( | ||
| f"SemanticRouter(" f"name={self.name!r}, " f"routes={len(self.routes)}" f")" | ||
| ) | ||
|
|
||
| @property | ||
| def route_names(self) -> List[str]: |
There was a problem hiding this comment.
The __repr__ implementation is hard to read due to implicit concatenation of multiple adjacent f-strings. This can be simplified to a single f-string for clarity (no behavior change), which also makes the exact output format easier to audit/maintain.
| return ( | |
| f"SemanticRouter(" f"name={self.name!r}, " f"routes={len(self.routes)}" f")" | |
| ) | |
| @property | |
| def route_names(self) -> List[str]: | |
| return f"SemanticRouter(name={self.name!r}, routes={len(self.routes)})" | |
| @property | |
| def route_names(self) -> List[str]: | |
| def route_names(self) -> List[str]: |
|
@tylerhutcherson the mismatch has been addressed. |
There was a problem hiding this comment.
This is taking shape! There are some comments left by Copilot that would be good to address, around the syntax and maintaining Python 3.9 support (it's slated to be dropped, but for the moment we support it). I left a 👍 reaction on the Copilot comments I think are worth addressing.
|
@vishal-bala I have made the changes that you had put thumbs up on. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def __repr__(self) -> str: | ||
| return ( | ||
| f"{type(self).__name__}(name={self.name!r}, prefixes={self.prefixes!r}, " | ||
| f"storage_type={self.storage_type.value!r})" | ||
| ) |
There was a problem hiding this comment.
The PR description/example output shows SearchIndex/AsyncSearchIndex repr using prefix=..., but the implementation prints prefixes=[...]. Consider updating the PR description/examples to match, or adjust __repr__ to emit prefix= when there is a single prefix (and only use prefixes= for multi-prefix schemas) to keep the stated intent and actual output aligned.
Summary
__repr__toSearchIndex/AsyncSearchIndex(viaBaseSearchIndex) showingname,prefix, andstorage_type__repr__toSemanticCacheshowingname,distance_threshold, andttl__repr__toSemanticRoutershowingnameand route count__repr__toMessageHistoryshowingnameandsession_tag__repr__toSemanticMessageHistoryshowingname,session_tag, anddistance_thresholdtests/unit/test_repr.pywith 11 unit tests covering all new reprs (no Redis required)Motivation
Core objects previously fell back to the default
<ClassName object at 0x...>repr, making notebook debugging, log inspection, and agent-generated diagnostics hard to read. All new reprs are:Example output
For #483
Note
Low Risk
Low risk: adds string representations and unit tests only, with no changes to Redis interactions, query logic, or data handling.
Overview
Adds deterministic, human-readable
__repr__implementations to core RedisVL objects to improve debugging/logging output:BaseSearchIndex(coveringSearchIndex/AsyncSearchIndex),SemanticCache,SemanticRouter,MessageHistory, andSemanticMessageHistory.Introduces
tests/unit/test_repr.py, which validates the exact repr strings and uses mocking/model_constructto avoid requiring a running Redis instance or downloading embedding models.Written by Cursor Bugbot for commit 8bbb90b. This will update automatically on new commits. Configure here.