Skip to content

fix(search): separate channel candidate retrieval limit before fusion… - #146

Open
ayan-waqas wants to merge 1 commit into
grayhatdevelopers:mainfrom
ayan-waqas:fix/issue-88-candidate-limit
Open

fix(search): separate channel candidate retrieval limit before fusion…#146
ayan-waqas wants to merge 1 commit into
grayhatdevelopers:mainfrom
ayan-waqas:fix/issue-88-candidate-limit

Conversation

@ayan-waqas

Copy link
Copy Markdown

Related issue

Closes #88

Summary

  • Introduced resolve_candidate_limit(top_k) in src/vidxp/search_fusion.py to calculate internal retrieval limits for candidate pools per search channel (bounded between 50 and 500 candidates).
  • Updated VidXPApplication.search() and VidXPApplication.query_video() in src/vidxp/application.py to use candidate_limit when fetching channel candidates, while returning command.top_k final fused moments to callers.
  • Internal-only search retrieval change; preserves existing CLI, API, and MCP parameter definitions (top_k) and response schemas without breaking compatibility.

Validation

  • Added and executed unit tests for resolve_candidate_limit bounds and candidate expansion in tests/test_search_fusion.py:
    • Verified resolve_candidate_limit(3) returns 50, resolve_candidate_limit(200) returns 500, and 0/negative inputs remain un-expanded.
    • Verified regression case where a moment ranked 3rd in two individual channels (outside top_k=2) is retrieved in candidate pools and wins docs/open source v1 #1 in fused results.
  • Added and executed Application.search candidate limit verification in tests/test_application.py:
    • Verified underlying capability search handler receives candidate limit (50) for channel search when top_k=3 is requested.
  • Executed test suite via .venv/bin/pytest tests/test_search_fusion.py tests/test_application.py tests/test_query_service.py:
    • 47 passed in 2.80s.

…grayhatdevelopers#88)

- Introduce resolve_candidate_limit() in search_fusion to calculate bounded internal candidate pools per search channel
- Update Application.search() and Application.query_video() to use candidate limits for channel queries while returning top_k final fused moments
- Add unit and regression tests for candidate depth and top-k retrieval bounds
@SaadBazaz

Copy link
Copy Markdown
Member

Hey @ayan-waqas,
Thanks for the Pull Request.
As a starter, we require all contributors to "star" and "fork" the repo, in order to determine if there really is a human behind the wheel or an autonomous agent.
Please star the repo for our review to proceed.
Thanks!

@ayan-waqas

Copy link
Copy Markdown
Author

@SaadBazaz I have starred the repo , kindly review the PR now.
Thanks!

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.

Do not use the final result count as the retrieval candidate limit

2 participants