Add the option of using trigram word similarity for NGO and Cause search #645
Conversation
📝 WalkthroughWalkthroughThese changes introduce a feature flag-controlled enhancement to NGO and cause search functionality, enabling conditional use of trigram word similarity alongside configurable similarity thresholds. A new feature flag defaults to enabled and drives the search strategy selection. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
Greptile OverviewGreptile SummaryThis PR introduces Key Changes:
Impact:
Confidence Score: 4/5
|
| Filename | Overview |
|---|---|
| backend/redirectioneaza/settings/feature_flags.py | Added ENABLE_NGO_SEARCH_WORD_SIMILARITY feature flag with default value True, following existing patterns |
| backend/donations/views/common/search.py | Implemented word-based trigram similarity with configurable thresholds; includes TODO comment about existing logic issue and QuerySet union order change |
Sequence Diagram
sequenceDiagram
participant User
participant SearchMixin
participant FeatureFlag
participant PostgreSQL
User->>SearchMixin: Search query
SearchMixin->>FeatureFlag: Check ENABLE_NGO_SEARCH_WORD_SIMILARITY
alt Word Similarity Enabled (default)
FeatureFlag-->>SearchMixin: True
SearchMixin->>PostgreSQL: TrigramWordSimilarity(query, "name")
Note over SearchMixin,PostgreSQL: Threshold: 0.4<br/>Word-aware matching
else Character Similarity
FeatureFlag-->>SearchMixin: False
SearchMixin->>PostgreSQL: TrigramSimilarity("name", query)
Note over SearchMixin,PostgreSQL: Threshold: 0.3<br/>Character-level matching
end
SearchMixin->>PostgreSQL: SearchRank(vector, query)
PostgreSQL-->>SearchMixin: Annotated results
SearchMixin->>SearchMixin: Filter (rank >= 0.3 OR similarity > threshold)
SearchMixin-->>User: Search results
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| searched_causes = CauseSearchMixin.get_search_results(queryset, query, language_code) | ||
|
|
||
| return searched_causes | ngos_causes | ||
| return ngos_causes | searched_causes |
There was a problem hiding this comment.
QuerySet union order changed from searched_causes | ngos_causes to ngos_causes | searched_causes - verify this produces expected result ordering in searches
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/donations/views/common/search.py`:
- Around line 59-62: The code calls queryset.none() when queryset may be None,
causing AttributeError; update the logic in the method that manipulates queryset
so it never calls none() on a None object — e.g., change the block that reads
"if not queryset: if not self.queryset: return queryset.none()" to return a safe
empty queryset using either self.queryset.none() (if self.queryset is set), or
self.model.objects.none() (for model-agnostic mixins), or raise a clear
configuration error; reference the variables queryset and self.queryset and the
surrounding method to implement one of these safe alternatives.
🧹 Nitpick comments (2)
backend/redirectioneaza/settings/feature_flags.py (1)
4-5: Consider a more generic flag name.The flag is named
ENABLE_NGO_SEARCH_WORD_SIMILARITYbut it also controls the similarity strategy forCauseSearchMixin. A name likeENABLE_SEARCH_WORD_SIMILARITYorENABLE_TRIGRAM_WORD_SIMILARITYwould better reflect its actual scope.backend/donations/views/common/search.py (1)
82-87: Duplicated logic withCauseSearchMixin.Lines 82-87 are identical to lines 109-114. Consider extracting this to a helper method or extending
ConfigureSearchto keep the similarity strategy in one place.♻️ Proposed refactor
class ConfigureSearch: + `@staticmethod` + def trigram_similarity(query: str, field: str = "name"): + if settings.ENABLE_NGO_SEARCH_WORD_SIMILARITY: + return TrigramWordSimilarity(query, field), 0.4 + return TrigramSimilarity(field, query), 0.3 + `@staticmethod` def query(query: str, language_code: str) -> SearchQuery:Then in both mixins:
trigram_similarity, similarity_threshold = ConfigureSearch.trigram_similarity(query, "name")
| if not queryset: | ||
| if not self.queryset: | ||
| return queryset.none() | ||
| return queryset.none() # TODO: this looks weird | ||
| queryset = self.queryset |
There was a problem hiding this comment.
Critical bug: queryset.none() will fail when queryset is None.
When queryset is None (falsy), calling queryset.none() raises AttributeError: 'NoneType' object has no attribute 'none'. The TODO comment correctly identifies this as problematic.
🐛 Proposed fix
if not queryset:
if not self.queryset:
- return queryset.none() # TODO: this looks weird
+ return Ngo.objects.none() # Return an empty QuerySet
queryset = self.querysetAlternatively, if the mixin should be model-agnostic, consider using self.model.objects.none() or raising an exception to fail fast on misconfiguration.
🤖 Prompt for AI Agents
In `@backend/donations/views/common/search.py` around lines 59 - 62, The code
calls queryset.none() when queryset may be None, causing AttributeError; update
the logic in the method that manipulates queryset so it never calls none() on a
None object — e.g., change the block that reads "if not queryset: if not
self.queryset: return queryset.none()" to return a safe empty queryset using
either self.queryset.none() (if self.queryset is set), or
self.model.objects.none() (for model-agnostic mixins), or raise a clear
configuration error; reference the variables queryset and self.queryset and the
surrounding method to implement one of these safe alternatives.
1 participant
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes