Skip to content

fix(ks_search_tool): resolve multiple bugs in ks_search_tool.py (Fixes #73)#98

Open
zohaib-7035 wants to merge 2 commits intoINCF:mainfrom
zohaib-7035:fix-ks-search-issues-73
Open

fix(ks_search_tool): resolve multiple bugs in ks_search_tool.py (Fixes #73)#98
zohaib-7035 wants to merge 2 commits intoINCF:mainfrom
zohaib-7035:fix-ks-search-issues-73

Conversation

@zohaib-7035
Copy link
Contributor

Summary

This PR fixes the multiple issues identified in Issue #73 regarding ks_search_tool.py.

Changes Made:

  • Dead stub classes: Removed unused BaseModel and Field leftovers from removing Pydantic.
  • Standardized result schema: Both general_search and _perform_search now return consistent _id, title, description fields.
  • Silent empty return: global_fuzzy_keyword_search now logs a warning when datasources_config.json is missing.
  • All fuzzy matches used: search_across_all_fields now iterates over all matches instead of only matches[0].
  • Deprecated API: Replaced asyncio.get_event_loop() with asyncio.get_running_loop().
  • print()logging: Replaced all 10+ print calls with logger.info() / logger.error().
  • Updated test_run.py for renamed field (title_guesstitle).

Verification

py_compile: backend/ks_search_tool.py ✓
py_compile: backend/test_run.py ✓
No errors or regressions.

Fixes #73

Copy link
Collaborator

@QuantumByte-01 QuantumByte-01 left a comment

Choose a reason for hiding this comment

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

All fixes from #73 are correctly addressed:

  • Dead BaseModel/Field stubs removed ✅
  • asyncio.get_event_loop()asyncio.get_running_loop()
  • All fuzzy matches iterated (not just matches[0]) ✅
  • Warning logged when datasources_config.json is missing ✅
  • print()logging throughout ✅
  • Schema unified (title_guesstitle, contentdescription) ✅

One issue before merge: please remove backend/test_run.py. It's a dev script that makes live network calls — it doesn't belong committed to the repo.

Also note: this PR and PR #97 both modify ks_search_tool.py from the same base — they will conflict. This one should be merged first (after PR #97 is fixed).

@zohaib-7035
Copy link
Contributor Author

Hi @QuantumByte-01 ,
Done! Removed backend/test_run.py as requested it was a dev script making live network calls and shouldn't be in the repo.

The PR should now be ready to merge. Let me know if anything else needs to be addressed!

@QuantumByte-01
Copy link
Collaborator

Both PR #97 and #98 have merge conflicts with main (introduced after #92 was merged). Please rebase both branches on the latest main:

git fetch origin main
git rebase origin/main
git push --force-with-lease

Once rebased, these will be mergeable.

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.

bug: Several real issues in ks_search_tool.py — inconsistent result schema, dead code, deprecated API usage

2 participants