Repository navigation
refactor(mcp): port search tests to bm_search and remove search_notes - #1963
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
9ab3a5c to
7d775ce
Compare
|
@codex review |
1 similar comment
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
@codex review |
1e1f3d9 to
3c96a93
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c96a93d9f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CMYVL1x91B8kJxy7nn2GU8 Signed-off-by: phernandez <paul@basicmachines.co>
bm_search is the only caller left, so search.py keeps just the shared query pipeline: the legacy markdown and error-guidance formatters go, the search across projects always returns rows, and the status/tags parameters only search_notes passed are dropped. strict_search_tags (search_notes' tags coercer) goes with it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CMYVL1x91B8kJxy7nn2GU8 Signed-off-by: phernandez <paul@basicmachines.co>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CMYVL1x91B8kJxy7nn2GU8 Signed-off-by: phernandez <paul@basicmachines.co>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CMYVL1x91B8kJxy7nn2GU8 Signed-off-by: phernandez <paul@basicmachines.co>
3c96a93 to
750db74
Compare
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CMYVL1x91B8kJxy7nn2GU8 Signed-off-by: phernandez <paul@basicmachines.co>
…n docs The ui templates served the retired view_note/search UI tools; nothing imports them. Comments and docstrings that named removed tools now describe the bm_* surface. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CMYVL1x91B8kJxy7nn2GU8 Signed-off-by: phernandez <paul@basicmachines.co>
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
Summary
Stacked on #1964 (
refactor/remove-cloud-only-legacy-tools), which removesread_note, the last core caller ofsearch_notes. Merge #1964 first.Two commits:
search_notesfunction to the publishedbm_searchtool.bm_searchis now the only caller, sosearch.pykeeps only the shared query pipeline (about 1,100 src lines deleted).search_notes,_format_search_error_response,_format_service_unavailable_response,_format_search_markdown,_NO_SEARCH_CRITERIA_MESSAGE,METADATA_FILTERS_DESCRIPTION, and thetools/__init__re-export._search_all_projectsno longer takesoutput_format. It always returns the merged rows.MultiProjectRancarriespayload, andMultiProjectFailedcarries onlyreason. This removesbm_search's# pragma: no covertext branch.statusparameter of_build_search_queryand thetagsparameter ofsplit_tag_shorthand; onlysearch_notespassed them.split_tag_shorthandnow returns tags in query order instead of set order.utils.strict_search_tags(the tags coercer forsearch_notes) and its tests intests/test_coerce.py.test_index_readiness.py, which refactor(mcp): port context and recent-activity tests to bm_context and bm_project #1962 also ports; the parity test intest_tool_posix.py, which refactor(cli)!: rebuild cat/grep/ls/find on bm_* tools and remove tail #1960 deletes; and thesearch_notesentries intest_tool_contracts.pyandimplementation_mcp.py.bm_searchpaths that were covered only incidentally before:(more matches from line N)hint;lines;Files
tests/mcp/test_tool_search.pytests/mcp/test_tool_bm_search_query.py(new)tests/mcp/test_mcp_note_type_normalization.pytest_tool_bm_search_query.pytests/mcp/test_search_total_exactness.pytest_tool_bm_search_query.pytests/mcp/test_tool_search_temporal.pytests/mcp/test_tool_bm_search_temporal.pytests/mcp/tools/test_search_notes_multi_project.pytests/mcp/tools/test_bm_search_multi_project.pytests/mcp/tools/test_search_notes_multi_project_temporal.pytest_bm_search_multi_project.pytests/mcp/tools/test_cjk_search_guidance.pytest_bm_search_multi_project.pytests/api/v2/test_cjk_search_guidance.pybm_searchtests/mcp/test_tool_bm_search.py_search_all_projectsnow run the real path or moved to the multi-project filePorted to real code paths
These tests used to stub
get_project_client,resolve_project_and_pathandSearchClientand inspect the captured payload. They now do one of two things:bm_searchend to end over notes written withbm_write: title and permalink modes, note types including snake_case canonicalization, items, categories,since, thenote_typefilter alias, tag shorthand, total exactness per mode, default hybrid mode, Boolean queries defaulting to text, configureddefault_search_type, never-indexed versus indexed misses, and comma or JSON list strings through the MCP layer.bm_searchuses on every request:_build_search_query,split_tag_shorthandand_search_type_for.The valid-time doc tests now check
bm-search(3)instead of thesearch_notesdocstring.Remaining monkeypatches, and why
get_client,ScopedSearchClientand the readiness probe. The merge, paging, attribution and failure handling run for real. A new test,test_hits_from_two_local_projects_name_their_project, runs the whole path with no stubs across two real local projects.ScopedSearchClient, to stand in for a server older than SPEC-82.semantic_runtime_available, because sqlite-vec loads in the test environment.Dropped, with reasons
TestSearchErrorFormatting,TestSearchToolErrorHandling, invalidsearch_typereturned as a string):bm_searchraises tool errors instead of returning guidance markdown. Its refusals are covered intest_tool_bm_search.py._format_search_markdowntests, and the valid-time label in markdown output: this is the legacy text format.bm_searchrenders with_format_results.search_notes:bm_searchroutes memory:// URLs throughopen_project_target. That path is covered bytest_note_targets.pyand the project-qualified memory URL test.search_notesparameter-annotation tests andtags=tests (comma-split, invalid types, direct-call CLI regression):bm_searchhas notagsorstatusparameters, and no CLI callssearch_notesany more. The equivalentbm_searchlist parameters are tested through MCP.bm_searchrequires an explicit scope, and that refusal is already tested.page_size=1pagination are already covered intest_tool_bm_search.py.Coverage
search.pyandranked_search.pyare both at 100%, measured over the touched test files.Verification
just fast-checkjust test-unit-sqlite -k "search or test_index_readiness or test_tool_contracts or test_coerce or ...": 1146 passed. One failure,test_tokenizing_scales_linearly_with_query_length, is a timing test; it passes when rerun on its own.just test-unit-postgres -k "bm_search or test_cjk_search_guidance or test_index_readiness or test_tool_contracts or test_coerce or bm_search_parity": all pass, after making one new test backend-neutral.just test-int-sqlite -k search: 64 passed🤖 Generated with Claude Code
https://claude.ai/code/session_01CMYVL1x91B8kJxy7nn2GU8