Skip to content

fix: pass withsuffixtrie attribute to Redis Text and Tag fields#536

Open
nkanu17 wants to merge 1 commit intomainfrom
fix/withsuffixtrie-not-passed
Open

fix: pass withsuffixtrie attribute to Redis Text and Tag fields#536
nkanu17 wants to merge 1 commit intomainfrom
fix/withsuffixtrie-not-passed

Conversation

@nkanu17
Copy link
Collaborator

@nkanu17 nkanu17 commented Mar 19, 2026

Related: #535
The withsuffixtrie attribute was defined in TextFieldAttributes and TagFieldAttributes but never passed to the underlying Redis field constructors in as_redis_field() methods. This meant the attribute was silently ignored - indexes were created without suffix trie support even when explicitly specified.

Solution

  • Added withsuffixtrie parameter to both RedisTextField() and RedisTagField() constructor calls in redisvl/schema/fields.py.

Changes

  • TextField.as_redis_field() - now passes withsuffixtrie=True when enabled
  • TagField.as_redis_field() - now passes withsuffixtrie=True when enabled
  • Added 4 integration tests verifying WITHSUFFIXTRIE appears in FT.INFO output

Note

Low Risk
Low risk: the change only affects index creation when withsuffixtrie is explicitly enabled, plus adds integration coverage to detect regressions.

Overview
Fixes schema-to-Redis field translation so TextField.as_redis_field() and TagField.as_redis_field() now pass withsuffixtrie=True to the underlying Redis field constructors when the attribute is enabled.

Adds integration tests that create indexes with withsuffixtrie (and in combination with sortable/case_sensitive) and assert WITHSUFFIXTRIE appears in FT.INFO field attributes.

Written by Cursor Bugbot for commit 7267056. This will update automatically on new commits. Configure here.

Copilot AI review requested due to automatic review settings March 19, 2026 22:32
@jit-ci
Copy link

jit-ci bot commented Mar 19, 2026

🛡️ Jit Security Scan Results

CRITICAL HIGH MEDIUM

✅ No security findings were detected in this PR


Security scan by Jit

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a schema-to-Redis translation gap by ensuring the withsuffixtrie attribute on TextFieldAttributes / TagFieldAttributes is actually forwarded into the underlying Redis search field definitions, and adds integration coverage to validate the modifier shows up in FT.INFO.

Changes:

  • Pass withsuffixtrie=True through TextField.as_redis_field() into RedisTextField(...).
  • Pass withsuffixtrie=True through TagField.as_redis_field() into RedisTagField(...).
  • Add integration tests that create indexes with withsuffixtrie and assert WITHSUFFIXTRIE appears in the field attributes returned by FT.INFO.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
redisvl/schema/fields.py Forwards withsuffixtrie from schema field attributes into Redis field constructor kwargs for TEXT and TAG fields.
tests/integration/test_withsuffixtrie_integration.py Adds integration tests verifying WITHSUFFIXTRIE is present in FT.INFO attributes for TEXT/TAG fields when enabled.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +8 to +9
import pytest

Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pytest is imported but not used in this test module. Consider removing the import to avoid dead code / keep the test file minimal (or use it for an explicit skip/marker if intended).

Suggested change
import pytest

Copilot uses AI. Check for mistakes.
The withsuffixtrie attribute was defined in TextFieldAttributes and
TagFieldAttributes but never passed to the underlying Redis field
constructors in as_redis_field() methods.

- Add withsuffixtrie parameter to RedisTextField constructor call
- Add withsuffixtrie parameter to RedisTagField constructor call
- Add integration tests to verify WITHSUFFIXTRIE modifier in FT.INFO
@nkanu17 nkanu17 force-pushed the fix/withsuffixtrie-not-passed branch from ab0dbd0 to 7267056 Compare March 19, 2026 22:58
@nkanu17 nkanu17 requested a review from Copilot March 19, 2026 23:03
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +403 to +405
# Add WITHSUFFIXTRIE if enabled
if self.attrs.withsuffixtrie: # type: ignore
kwargs["withsuffixtrie"] = True
Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_normalize_field_modifiers() overwrites field.args_suffix with only the modifiers listed in canonical_order. Since WITHSUFFIXTRIE is not included in that list, enabling withsuffixtrie here is likely to be stripped back out before FT.CREATE is issued. Consider either (a) including WITHSUFFIXTRIE in the canonical order for TEXT fields, or (b) updating _normalize_field_modifiers to preserve non-canonical suffix modifiers while reordering the known ones.

Copilot uses AI. Check for mistakes.
Comment on lines +449 to +451
# Add WITHSUFFIXTRIE if enabled
if self.attrs.withsuffixtrie: # type: ignore
kwargs["withsuffixtrie"] = True
Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue as TEXT: after setting kwargs["withsuffixtrie"] = True, _normalize_field_modifiers() will reset args_suffix to only INDEXEMPTY/INDEXMISSING/SORTABLE/NOINDEX. If WITHSUFFIXTRIE is represented as a suffix modifier by redis-py, it will be dropped and the index will be created without suffix trie support. Update the canonical order to include WITHSUFFIXTRIE (or make _normalize_field_modifiers preserve unknown modifiers).

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants