Repository navigation
fix(core): finish removing semantic_search_enabled after today's merges - #1709
Merged
Merged
Conversation
#1696 removed `semantic_search_enabled` from BasicMemoryConfig, but it was tested against a main that predated several milestone merges that still read the flag, so main failed static checks and most test jobs: - `index_project_and_report_readiness` (#1704) passed `embeddings=app_config.semantic_search_enabled`, which raised AttributeError on every `bm project add`. Semantic search is always on, so the add always runs the embedding pass. - test_db_reindex (#1704) parametrized the add on the flag; it now asserts the embedding pass runs. - test_deferred_embedding_resume (#1700) set the flag on its config. - test_config_env_overrides_not_persisted (#1701) used the flag as its example env-overridden boolean; it now uses `format_on_save`. Also makes #1704's cloud-prune test compare against str(Path) so it holds on Windows, where the path prints with backslashes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea Signed-off-by: phernandez <paul@basicmachines.co>
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Main is red at 61d965e. #1696 removed
semantic_search_enabledfromBasicMemoryConfig, but its CI ran against a main from before several milestone PRs that still read the flag. On main, Static Checks and most SQLite, Postgres and Windows jobs fail (run 37886389135).Fixes
src/basic_memory/cli/commands/db.py:index_project_and_report_readiness(fix(cli): polish help and messages from the v0.24.0 live run #1704) passedembeddings=app_config.semantic_search_enabled, which raises AttributeError on everybm project add. It now always runs the embedding pass. That also fixestests/cli/test_project_add_indexing.pyand thetest-int/cli/test_project_commands_integration.pyfailures.tests/cli/test_db_reindex.py(fix(cli): polish help and messages from the v0.24.0 live run #1704): drops the flag parametrization and asserts that the embedding pass runs.tests/indexing/test_deferred_embedding_resume.py(fix(core): resume deferred oversized-entity embeddings on the next index pass #1700): no longer sets the flag.tests/test_config_env_overrides_not_persisted.py(fix(core): keep environment overrides out of config.json #1701): usesformat_on_saveas the example boolean that is overridden by env but has its own value in the file.tests/cli/cloud/test_project_sync_command.py(fix(cli): polish help and messages from the v0.24.0 live run #1704): compares againststr(Path(...))so the cloud-prune message test passes on Windows.Verification
just fast-check: clean. Before this change it reported the same 4 ty errors as CI.just fast-teston every test that failed on main: 84 + 7 + 57 passed.semantic_search_enabledreferences remain outside feat(core): remove semantic_search_enabled; semantic search is always on #1696's intentional compatibility shim inschemas/project_info.pyand its tests.🤖 Generated with Claude Code
https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea