Add precise type annotations to search helpers - #5140
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughBackend search helpers now return typed search result payloads with narrower list annotations. Slack user block rendering now defaults missing location and company values to empty strings, with a unit test covering ChangesSearch result typing updates
Slack user metadata fallback
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
|
Contribution validation failed:
|
814a5ee to
7443044
Compare
|
I checked the underlying algoliasearch-django implementation. raw_search() returns SearchResponse.to_dict(), whose upstream annotation is Dict[str, Any], while search_single_index() itself returns the typed SearchResponse model. Would you prefer a shared local TypedDict for the raw search result contract here or should these helpers preserve the upstream dictionary return type ? |
068df81
7443044 to
068df81
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/src/apps/github/index/search/user.py (1)
31-33: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUpdate the docstring return type to match the annotation.
The Returns section still says
dictbut the function now returnsUserSearchResult. This same inconsistency exists across all search helper files in this PR.📝 Proposed docstring update
Returns: - dict: Search results containing users and metadata. + UserSearchResult: Search results containing users and metadata.🤖 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 `@backend/src/apps/github/index/search/user.py` around lines 31 - 33, The docstring return type is inconsistent with the function annotation in the user search helper. Update the Returns section in the search helper docstrings, including the one in user-related search functions, to match the actual return type `UserSearchResult` instead of `dict`; apply the same correction across the other search helper files in this PR so the documented and annotated return types stay aligned.
🤖 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 `@backend/src/apps/github/index/search/user.py`:
- Around line 31-33: The docstring return type is inconsistent with the function
annotation in the user search helper. Update the Returns section in the search
helper docstrings, including the one in user-related search functions, to match
the actual return type `UserSearchResult` instead of `dict`; apply the same
correction across the other search helper files in this PR so the documented and
annotated return types stay aligned.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a9f95f17-355d-42bb-b563-4923a6975e55
📒 Files selected for processing (8)
backend/src/apps/common/index_types.pybackend/src/apps/github/index/search/user.pybackend/src/apps/owasp/index/search/chapter.pybackend/src/apps/owasp/index/search/committee.pybackend/src/apps/owasp/index/search/issue.pybackend/src/apps/owasp/index/search/project.pybackend/src/apps/slack/common/handlers/users.pybackend/tests/unit/apps/slack/common/handlers/users_test.py
|
Contribution validation failed:
|
219eceb to
068df81
Compare
|
Contribution validation failed:
|
|
Addressed the Any concern by replacing the generic search result annotation with concrete TypedDict contracts for each search hit shape. |
a7ec75b to
7a4c575
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/src/apps/github/index/search/user.py (1)
32-32: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the docstring return type.
The Returns section still says
dict:but the return annotation is nowUserSearchResult. This should be updated to match, keeping the docstring accurate for IDE support — a stated goal of this PR.📝 Proposed fix
Returns: - dict: Search results containing users and metadata. + UserSearchResult: Search results containing users and metadata.🤖 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 `@backend/src/apps/github/index/search/user.py` at line 32, The docstring return type for UserSearchResult in the user search module is outdated and still says dict instead of the current annotated type. Update the Returns section in the docstring for the search user function or method to reference UserSearchResult so it matches the actual return annotation and keeps IDE/type hints accurate.
♻️ Duplicate comments (4)
backend/src/apps/owasp/index/search/chapter.py (1)
33-33: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the docstring return type.
Same stale
dict:docstring issue asuser.py— should sayChapterSearchResult.🤖 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 `@backend/src/apps/owasp/index/search/chapter.py` at line 33, Update the return type in the docstring for the chapter search function so it uses ChapterSearchResult instead of dict, matching the actual result type. Locate the docstring in the search chapter module by the ChapterSearchResult-related search function and replace the stale dict annotation with the correct type name.backend/src/apps/owasp/index/search/committee.py (1)
31-31: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the docstring return type.
Same stale
dict:docstring issue — should sayCommitteeSearchResult.🤖 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 `@backend/src/apps/owasp/index/search/committee.py` at line 31, The docstring on the committee search result needs its return type updated from the stale dict annotation to CommitteeSearchResult. Update the return description in the relevant docstring for the committee search function or method in committee.py so it matches the actual return type and aligns with the surrounding search result symbols.backend/src/apps/owasp/index/search/project.py (1)
33-33: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the docstring return type.
Same stale
dict:docstring issue — should sayProjectSearchResult.🤖 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 `@backend/src/apps/owasp/index/search/project.py` at line 33, The docstring for the project search result type is stale and still says dict instead of the actual return type. Update the return annotation text in the docstring associated with the search project result function/class in project.py to use ProjectSearchResult, and make sure any nearby docstrings in the same search module use the same concrete type naming.backend/src/apps/owasp/index/search/issue.py (1)
35-35: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the docstring return type.
Same stale
dict:docstring issue — should sayIssueSearchResult.🤖 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 `@backend/src/apps/owasp/index/search/issue.py` at line 35, The docstring for the search result return type is stale and still says dict instead of IssueSearchResult. Update the return annotation text in the relevant docstring for the search helper in issue.py so it matches the actual return type, and keep the surrounding documentation aligned with the IssueSearchResult symbol used by the search API.
🤖 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 `@backend/src/apps/github/index/search/user.py`:
- Line 32: The docstring return type for UserSearchResult in the user search
module is outdated and still says dict instead of the current annotated type.
Update the Returns section in the docstring for the search user function or
method to reference UserSearchResult so it matches the actual return annotation
and keeps IDE/type hints accurate.
---
Duplicate comments:
In `@backend/src/apps/owasp/index/search/chapter.py`:
- Line 33: Update the return type in the docstring for the chapter search
function so it uses ChapterSearchResult instead of dict, matching the actual
result type. Locate the docstring in the search chapter module by the
ChapterSearchResult-related search function and replace the stale dict
annotation with the correct type name.
In `@backend/src/apps/owasp/index/search/committee.py`:
- Line 31: The docstring on the committee search result needs its return type
updated from the stale dict annotation to CommitteeSearchResult. Update the
return description in the relevant docstring for the committee search function
or method in committee.py so it matches the actual return type and aligns with
the surrounding search result symbols.
In `@backend/src/apps/owasp/index/search/issue.py`:
- Line 35: The docstring for the search result return type is stale and still
says dict instead of IssueSearchResult. Update the return annotation text in the
relevant docstring for the search helper in issue.py so it matches the actual
return type, and keep the surrounding documentation aligned with the
IssueSearchResult symbol used by the search API.
In `@backend/src/apps/owasp/index/search/project.py`:
- Line 33: The docstring for the project search result type is stale and still
says dict instead of the actual return type. Update the return annotation text
in the docstring associated with the search project result function/class in
project.py to use ProjectSearchResult, and make sure any nearby docstrings in
the same search module use the same concrete type naming.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d616669c-b515-4ad1-9e14-65c319603819
📒 Files selected for processing (8)
backend/src/apps/common/index_types.pybackend/src/apps/github/index/search/user.pybackend/src/apps/owasp/index/search/chapter.pybackend/src/apps/owasp/index/search/committee.pybackend/src/apps/owasp/index/search/issue.pybackend/src/apps/owasp/index/search/project.pybackend/src/apps/slack/common/handlers/users.pybackend/tests/unit/apps/slack/common/handlers/users_test.py
|
dict[str, Any] result contract with concrete TypedDict search-hit shapes, handled the nullable metadata that stricter typing exposed and added regression coverage. |
|
Contribution validation failed:
|
7a94331 to
89e8819
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@backend/src/apps/common/index_types.py`:
- Around line 8-64: The `*SearchHit` TypedDicts in `index_types.py` are missing
several fields that the corresponding search helpers return by default, so
update `UserSearchHit`, `ChapterSearchHit`, `CommitteeSearchHit`,
`IssueSearchHit`, and `ProjectSearchHit` to include every default
`attributesToRetrieve` entry. Add the absent fields from `get_users`,
`get_chapters`, `get_committees`, `get_issues`, and `get_projects` (for example
`idx_email`, `idx_created_at`, `idx_updated_at`, `idx_related_urls`,
`idx_comments_count`, `idx_level`, `idx_topics`, etc.), and verify the exact
types for schema-dependent fields like `idx_top_contributors`,
`idx_related_urls`, `idx_hint`, and `idx_labels` against the Algolia index
schema. Keep the TypedDict definitions aligned with the search helper outputs so
future callers can access default fields without mypy errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4bfe9903-4ced-4ca9-843c-29e04bc64b6e
📒 Files selected for processing (8)
backend/src/apps/common/index_types.pybackend/src/apps/github/index/search/user.pybackend/src/apps/owasp/index/search/chapter.pybackend/src/apps/owasp/index/search/committee.pybackend/src/apps/owasp/index/search/issue.pybackend/src/apps/owasp/index/search/project.pybackend/src/apps/slack/common/handlers/users.pybackend/tests/unit/apps/slack/common/handlers/users_test.py
|
Contribution validation failed:
|
7bc91f7 to
9b1be05
Compare
Signed-off-by: Tanishq Meshram <tnshqmeshram@gmail.com>
9b1be05 to
371e2df
Compare
|
Contribution validation failed:
|
4d4b8cf to
78569f5
Compare
78569f5 to
371e2df
Compare
|
|
I reworked the search helpers to avoid exposing Any in the public return types by introducing concrete TypedDict definitions for the Algolia search results (index_types.py) and using those as the return types across the search helpers. Before I iterate further, I'd like to check whether this is the direction you'd prefer. Is using dedicated TypedDict models for these search results acceptable here, or would you rather see a different typing approach (for example, a shared generic type or another pattern already used in the project) ? I'm happy to refactor it to match the project's preferred typing style. |
2 participants
Summary
Adds precise type annotations to the Algolia search helpers by specifying string element types for search attribute lists and defining concrete
TypedDictcontracts for search result hits.The changes cover all search helpers that directly use
algoliasearch_django.raw_searchand avoidAnyin their public return annotations.Related issue
Fixes #5080
Changes
attributesandsearchable_attributesaslist[str] | None.TypedDictdefinitions for GitHub user and OWASP chapter, committee, issue, and project search hits.dict[str, Any].Testing
pre-commit run ruff-check --files ...— passedpre-commit run ruff-format --files ...— passedpre-commit run mypy --all-files— passedgit diff --check upstream/main..HEAD— passed