Fix document-level graph semantics, watcher debounce reliability, and test isolation - #15
Merged
Merged
Conversation
…tion Backlog fixes from the v1.1.0 quality gate and live dogfooding: - Collapse chunk rows to one result per note in search_notes and get_similar_notes (best-matching chunk wins); dedupe hub/orphan results by document; count DISTINCT connected documents in the connection-count refresh. A 87-chunk transcript previously filled the entire hub list with chunk-inflated counts (live-reproduced). Algorithm version is persisted in index_metadata so upgraded deployments refresh their materialized counts once automatically. - Rewrite get_similar_notes as a single any-chunk-to-any-chunk SQL query: deterministic for multi-chunk sources, self excluded in SQL, and no nested pool acquisition (deadlocked with max_connections=1). - Fix silent debounce drops in the file watcher: wall-clock timestamps compared against a monotonic sleep could abandon a pending change on sub-millisecond clock jitter (live-reproduced: a created note stayed unindexed until its next edit). Use monotonic time and re-sleep the remainder on early wakes; hold the per-file lock across the reindex. - Stop the connection-count refresh from overwriting last_indexed_at, which could mask genuinely stale files from the startup scan. - Close the pool when initialize() fails after pool creation. - Reject NaN thresholds in validate_float_range (NaN compares False against both bounds and reached the database). - Test isolation: integration fixtures delete their rows in teardown, and test_e2e_docker.py requires an explicit RUN_E2E_TESTS opt-in instead of running whenever credentials exist. - run_e2e_tests.py: add uniqueness regression checks for search, similar, hub, and orphan results (39 checks, all passing live).
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
- Failed connection-count refresh is no longer recorded as a completed algo migration: _do_refresh re-raises, set_metadata only runs after success, and the freshness check logs an actionable error naming the consequence (Critical/High from review) - Delete stale chunk rows when a note shrinks on re-index (watcher, indexer); batch upserts update total_chunks on conflict - get_similar_notes requires a non-NULL source embedding so a failed-embedding note errors instead of returning empty success - _cleanup_lock no longer pops a per-file lock that is held or has waiters, closing the residual concurrent-reindex window - Tests: NaN/inf validation, pool cleanup on failed initialize, debounce re-sleep regression (fails on the old early-return code), failed-migration honesty, staleness test exercises its named branch - test_tools fixture closes the pool even if cleanup delete fails
Owner
Author
Quality gate review (T3, 5 lenses) + live dogfoodingFindings remediated in 3ec0467:
Live verification on a real 234-document vault (rebuilt container):
150 unit tests pass; lint, mypy, black clean; CI green. |
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.
Summary
Follow-on to #14: fixes the backlog surfaced by the v1.1.0 quality gate, clawpatch, and live dogfooding against a real vault.
Fixes
Chunked notes distorted every graph tool (live-reproduced: an 87-chunk transcript occupied all 5 hub slots with identical inflated counts)
search_notes/get_similar_notes: one result per note (best-matching chunk viaDISTINCT ON), deterministic orderingget_hub_notes/get_orphaned_notes: one row per documentDISTINCTconnected documents, paginates over documents, and no longer overwriteslast_indexed_at(which could mask stale files from the startup scan)index_metadata→ upgraded deployments auto-refresh their materialized counts onceget_similar_notesrewritten as a single SQL query: any-chunk-to-any-chunk semantics for multi-chunk sources, self-exclusion in SQL, no nested pool acquisition (previously deadlocked withmax_connections=1)File watcher silently dropped change events (live-reproduced: a created note stayed unindexed until its next edit)
Robustness / correctness
initialize()closes the pool on post-creation failure instead of leaking itNaNthresholds rejected by validation (previously reached the database)Test isolation
ml/machine-learning.mdetc. into whatever DB was configured)test_e2e_docker.pyrequires explicitRUN_E2E_TESTS=1instead of running whenever credentials existTesting