Repository navigation
Conversation
The watcher selected projects from the config snapshot it was created with, so a project added later could stay unwatched. Re-read the current config on every selection. A watch attaches after the project's first scan; files copied before attachment produced no events. Once a new root is watched, run one project index pass before handling its event batches, retry a failed pass on the next cycle, and keep it pending while a restart is requested. Track projects by identity, not root, so a path change does not rescan an empty directory and delete the existing index. A reload waits for the catch-up drain so watchfiles does not discard buffered batches; startup projects keep their existing initial scan and are not scanned twice. Fixes basicmachines-co#1817 Signed-off-by: TPSJASPER <592827953@qq.com>
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 was referenced Oct 25, 2026
Member
|
Thanks, @TPSJASPER, for investigating the new-project watcher gap and contributing the fix and thorough regression coverage! We cherry-picked your commit into #1866, preserving your authorship and sign-off. That PR also adds a real MCP/native-watcher lifecycle integration test and fixes an additional catch-up gap exposed by continuous ignored file activity. Closing this PR in favor of #1866, which is approved and passing CI. Thank you for the contribution! |
phernandez
added a commit
that referenced
this pull request
Oct 25, 2026
…1865) ## Why Two reported SQLite failures block reindexing or startup: deleting a large chunk manifest exceeds SQLite's expression-depth limit (#1850), and an interrupted script-ngram migration leaves a scratch table that prevents the next startup (#1853). Both reproduce on current main with the contributor's regression tests. Fixes #1850. Fixes #1853. ## What Changed - Cherry-pick @MbappeWU's fixes from #1857 and #1859, retaining authorship and sign-offs. - Batch pending-vector deletion predicates and row-id deletes in groups of 200. - Drop a leftover `search_index_rebuilt` scratch table before rebuilding the intact source index. - Add real SQLite integration tests covering a 1,201-record delete, protected generations/project partitions, repeated deletion, and upgrade/downgrade retries with empty or populated scratch tables. ## Implementation Details Deletion retains the existing project, key, source-hash and pending-status guards in one transaction. Batching limits expression depth and bound parameters without changing the concurrency protocol. The migration rebuilds from `search_index`; the scratch table has no authoritative state. Integration tests commit the interrupted state before opening a new transaction and verify preserved full-text matches afterward. They use Alembic's real Operations context and sqlite-vec without application mocks. Contributor workflow-evidence notes are omitted from the final diff. ## Testing - Before fixes: the two new contributor regressions fail on main with `Expression tree is too large` and `table search_index_rebuilt already exists`. - `uv run pytest tests/repository/test_sqlite_vector_search_repository.py tests/test_script_ngram_search_migration.py test-int/test_sqlite_index_recovery.py -q -o addopts= --tb=short`: 46 passed. - `just fast-check`: lint, format and full typecheck passed. - `just doctor`: sandboxed write, index and search passed. - `git diff --check`: passed. ## Risks / Follow-ups Changes are SQLite-specific; PostgreSQL production paths are unchanged. The recovery assumes the original FTS table remains intact, matching the reported interrupted upgrade. #1861's watcher lifecycle changes are reviewed separately. --------- Signed-off-by: MbappeWU <MbappeWU@users.noreply.github.com> Signed-off-by: phernandez <paul@basicmachines.co> Co-authored-by: MbappeWU <MbappeWU@users.noreply.github.com>
phernandez
added a commit
that referenced
this pull request
Oct 25, 2026
…1866) ## Why Files copied into a newly created project's empty folder before the next watcher reload produce no events and stay unindexed. The watcher also uses a startup config snapshot that can exclude projects added later. Fixes #1817. Carries forward @TPSJASPER's #1861, cherry-picked with original authorship and sign-off. ## What Changed - Reload current project config when selecting local projects to watch. - After attaching a newly seen project's watch, run a catch-up index pass. Preserve startup scan ownership and project identity across path moves. - Retain the contributor's retry, cancellation and reload-drain coverage. - Handle continuous ignored events during attachment: observe raw batches in catch-up cycles and apply the existing visibility filter before indexing. - Add a production MCP lifecycle integration test with a real native watcher, database, file indexing and embeddings. It creates a project through `bm_project`, copies a note, checks search, then edits the file and checks search again while hidden-file activity continues. ## Implementation Details The catch-up starts only after `awatch` has attached its watches. Running in the event consumer prevents the scan and event batches from overlapping. Project identities track completed catch-up, so changing an existing project's path does not scan a still-empty move destination and delete its index. The new integration test found an additional gap in the original contribution: watchfiles suppresses batches whose events are all filtered. Continuous hidden-file activity can therefore suppress both observable batches and timeout yields, preventing catch-up indefinitely. New-project cycles receive raw batches and filter them locally. Only an actual quiet batch clears the drain guard. The integration test uses the real MCP lifespan and normal database initialization, rather than API overrides or application mocks. Its only environment overrides select production runtime behavior; a bounded poll waits for observable search results. Existing inline tests remain available for fast feedback. ## Testing - The production lifecycle test failed against the unmodified cherry-picked #1861 (45-second search deadline); passes with the ignored-event fix. - `uv run pytest tests/index -q -o addopts= --tb=short`: 221 passed. - `uv run pytest tests/index/test_watch_new_projects.py tests/mcp/test_new_project_watching.py tests/services/test_initialization.py tests/index/test_watch_service.py -q -o addopts= --tb=short`: 33 passed. - `uv run pytest test-int/mcp/test_new_project_watch_lifecycle.py -q -o addopts= --tb=short`: passed on SQLite with continuous hidden-file activity. - `BASIC_MEMORY_TEST_POSTGRES=1 uv run pytest test-int/mcp/test_new_project_watch_lifecycle.py tests/index/test_watch_new_projects.py tests/services/test_initialization.py -q -o addopts= --tb=short`: 24 passed. - `just fast-check`: lint, format and full typecheck passed. - `just doctor`: sandboxed write, index and search passed. - `git diff --check`: passed. ## Risks / Follow-ups New projects receive one project-wide scan after watch attachment, which can delay event processing while that scan runs. Reload deferral remains bounded as in the contributor's implementation. This changes local watching only; hosted indexing is unaffected. Existing project path moves retain their current behavior. --------- Signed-off-by: TPSJASPER <592827953@qq.com> Signed-off-by: phernandez <paul@basicmachines.co> Co-authored-by: TPSJASPER <592827953@qq.com>
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
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.
Why
Fixes #1817
A project created in an empty folder is watched only after the next project reload. Files copied into it before that reload produce no watch events, so they stay unindexed until they change,
bm reindexruns, or the server restarts. The watcher also filtered projects with the config snapshot it was created with, so a project added later could stay unwatched even after a reload.What changed
_select_projects_to_watchre-reads the currentConfigManagerconfig and keeps the existing locally-syncable and constrained-project rules.WatchServicetracks projects it has finished catching up by project external id, not by root path. Startup projects register from their existing initial scan.bm project moverewrites the path before the user moves the files, so scanning the still-empty root would plan every old path as deleted and remove the project's index.The
bm-projectmanual page describes the new-project catch-up pass.Validation
tests/indexsuite on SQLite: 210 passed, 10 skipped, 1 deselected.bm_project, copies a nested Markdown file before attach, and finds it withbm_search.ruff check,ruff format --check,ty checkfor the changed files, andgit diff --checkpassed.doctor --localcompleted sandboxed write, index and search in an isolated config directory.The full-repository
ty checkstill reports the same 8 environment diagnostics as the unmodified baseline (optionalpymilvus/uvloop, Windowsresourcemembers); no ignores were added. The full test suite and Postgres tests were not run. The implementation was also replayed againstorigin/mainwithgit apply --check.Risks / rollback
The catch-up runs once per newly seen project identity after attachment; files written during the pass are still delivered by watch events. Startup still relies on its existing background initial scan rather than a second pass. Revert the commit to roll back.