Skip to content

feat: implement weighted search for title and description #87

Open
Kayd-06 wants to merge 4 commits into
fossgis:mainfrom
Kayd-06:feature/issue-83-weighted-search
Open

feat: implement weighted search for title and description #87
Kayd-06 wants to merge 4 commits into
fossgis:mainfrom
Kayd-06:feature/issue-83-weighted-search

Conversation

@Kayd-06

@Kayd-06 Kayd-06 commented Mar 25, 2026

Copy link
Copy Markdown
Contributor

Closes #83.

Description

This PR updates the search functionality to query both the title and description fields with a weighted ranking system, rather than just the generic icontains lookup. This significantly improves model discoverability by factoring in metadata relevance.

Changes made

  • PostgreSQL Full-Text Search: Utilized Django's PostgreSQL features (SearchVector, SearchQuery, and SearchRank).
  • Weighted Rankings:
    • title matches are assigned weight A.
    • description matches are assigned weight B.
  • Filtering & Ordering: Applied a minimum rank threshold (rank__gte=0.01) to exclude completely irrelevant results, and sorted the results by -rank to ensure the most contextually relevant models appear first.
  • Frontend & API: Updated both mainapp/views.py (frontend search) and mainapp/api.py (API endpoints search_full and search_title) to maintain consistency across the entire app.
  • Fixed a minor precedence bug where sorting by -rank in the views.py search view mistakenly applied to unrelated querysets (like category).

Testing

  • Ensure PostgreSQL is set up for the developer environment (as the project already expects django.contrib.postgres).
  • Ran existing tests in test_search.py and test_search_full.py, which all pass successfully.

@Kayd-06

Kayd-06 commented Mar 25, 2026

Copy link
Copy Markdown
Contributor Author

Hi @AyushDharDubey, I've opened a PR to address this issue.

I implemented PostgreSQL's full-text search with a weighted ranking system (A for title, B for description) to improve model discoverability and adjusted the API/views to utilize it.

@AyushDharDubey AyushDharDubey left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks mostly good, could you please include tests to support the implementation.

Comment thread mainapp/api.py Outdated
Comment thread mainapp/api.py
Comment thread mainapp/views.py Outdated
@Kayd-06

Kayd-06 commented Mar 27, 2026

Copy link
Copy Markdown
Contributor Author

@AyushDharDubey I've pushed the requested changes:

  • Renamed API Endpoint: Changed /api/search/title to /api/search/model across the codebase and docs.
  • Fixed Query Filter Loop: Refactored views.py from elif to if statements. This fixes an UnboundLocalError bug and ensures a search query correctly applies on top of any tag/category filters.
  • Added Typo Tolerance: Added a migration for PostgreSQL's pg_trgm extension and implemented fuzzy matching using TrigramSimilarity on both titles and descriptions.
  • Updated Tests: Adjusted the test suite to expect and accurately verify the new fuzzy trigram matching behavior.

Everything is tested and passing locally.

@AyushDharDubey AyushDharDubey requested a review from lonvia March 30, 2026 08:25

@lonvia lonvia left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I haven't tried it but looks good to me in general. We might want to adjust the similarity threshold a bit upwards later. 0.1 is quite small. But we can try with real data first to see how it comes out.

Comment thread mainapp/api.py Outdated
Comment thread mainapp/views.py Outdated
@Kayd-06

Kayd-06 commented Mar 30, 2026

Copy link
Copy Markdown
Contributor Author

@lonvia I originally kept the variable when refactoring the filtering precedence, but you're absolutely right that it's no longer necessary. I've updated views.py to remove the unused variable and assign directly to filtered_models.

@Kayd-06

Kayd-06 commented Apr 11, 2026

Copy link
Copy Markdown
Contributor Author

Hi @AyushDharDubey, thanks for the approval! Just a quick bump to see if you or @lonvia could approve the GitHub actions workflow to run so this can merge. No rush!

@AyushDharDubey

Copy link
Copy Markdown
Collaborator

Hello, we were actually expecting for you to address Sarah's review, which may have been overlooked.

@Kayd-06 Kayd-06 force-pushed the feature/issue-83-weighted-search branch from aec8c9a to df8a8f8 Compare April 12, 2026 10:17
@Kayd-06 Kayd-06 force-pushed the feature/issue-83-weighted-search branch from 008b138 to 9cb70c7 Compare April 12, 2026 11:04
@Kayd-06

Kayd-06 commented Apr 12, 2026

Copy link
Copy Markdown
Contributor Author

@AyushDharDubey I have done the changes. Is it good to go?

@AyushDharDubey

Copy link
Copy Markdown
Collaborator

Hello Kunal, this partially addresses the previous review.

However, we still need to restore the title parameter for api/search/title/<title>. As Sarah indicated earlier, Changing or removing this would result in a breaking change for the existing API users.

The updated search_full seems fine.

@Kayd-06

Kayd-06 commented Apr 23, 2026

Copy link
Copy Markdown
Contributor Author

@AyushDharDubey Sorry for the partial address last time! I've now restored the /api/search/title/<title> endpoints and renamed search_model back to search_title, ensuring it only performs the weighted search on the title field to maintain backward compatibility. Is it good to go now?

@AyushDharDubey AyushDharDubey requested a review from lonvia April 26, 2026 16:07
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.

Include description tokens in search with weighted ranking

3 participants