Repo review + fixes for 3 broken commands (vec reindex, vsearch, dream-cycle) - #171
Draft
TSchonleber wants to merge 3 commits into
Draft
Repo review + fixes for 3 broken commands (vec reindex, vsearch, dream-cycle)#171TSchonleber wants to merge 3 commits into
TSchonleber wants to merge 3 commits into
Conversation
Triage of 18 open PRs and 9 open issues against main @ c634808. Key findings: - 3 documented commands are fully broken with no PR yet: vec reindex (#160, missing sample_db_embedding_widths), vsearch (#161, vec_events/ vec_context never created), dream-cycle (#168, naive-vs-aware datetime crash in run_hebbian_pass). Flagged P0. - 3 tested fixes ready to land: #166/#167 (FTS, with a merge-order dependency #166->#167) and #170 (MCP extra error). - CI unrun on the 3 fork PRs (first-time-contributor gating). - 10 Dependabot PRs safe to batch-merge; 4 stale drafts need a call. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018eUi7mmueAJFfBssEezPiP
Implements the three P0 fixes identified in the repo review. #160 vec reindex ImportError: implement sample_db_embedding_widths() in embeddings.py. cmd_vec_reindex imported and called it but it was never defined, so the command died on import before doing any work. The probe reads a sample of vec_memories rows, computes each float32 width, and cross-checks against the DDL-declared dim so the destructive DROP+CREATE refuses to run on a mixed-width/corrupt index unless --force. #161 vsearch aborts on missing vec tables: guard _vsearch_table against sqlite3.OperationalError for an absent vec table (vec_events/vec_context are in the default table set but never created), returning [] for that source so vsearch still surfaces results from vec_memories. Implementing events/context vector indexing is left as a follow-up. #168 dream-cycle NREM crash: normalize parse_ts to always return aware UTC and coerce a naive 'now' to aware UTC inside days_since, so the Hebbian pass no longer mixes offset-naive datetime.now() with aware UTC created_at values. A repo-wide naive-vs-aware datetime.now() audit is noted as follow-up (the broader-note in #168). Adds regression tests for all three (13 new tests; full touched-module set of 60 tests green). Fixes #160 Fixes #161 Fixes #168 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018eUi7mmueAJFfBssEezPiP
TSchonleber
changed the title
… API An unbounded 'mcp>=1.27.0' now resolves to mcp 2.0.0, which deleted the decorator-style low-level Server API (@app.list_tools() / @app.call_tool()) that src/agentmemory/mcp_server.py is built on. Every 'import agentmemory.mcp_server' therefore blows up at import time with 'AttributeError: Server object has no attribute list_tools', taking out 12 test modules at collection and turning the required 'test' and 'docs-check' CI jobs red on every PR (and on main). Bound the pin below 2.0.0 in both the 'mcp' and 'all' extras until the server is migrated to the 2.x API. Verified against a clean venv: with mcp 1.29.0 the full suite collects and runs 2529 passed (the only failures are sandbox-only — a HuggingFace 403 in the rerank smoke and a root-user permission-fallback test — both green on non-root CI with network). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018eUi7mmueAJFfBssEezPiP
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
2 participants
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.
What does this PR do?
Two things, from a full sweep of the open PRs and issues:
docs/REPO_REVIEW_ACTION_PLAN.md— triage of all 18 open PRs and 9 openissues @
c634808, with a prioritized land/fix/close order.Fixes the three P0 bugs the review surfaced — each of which made a
documented command completely non-functional and had no PR:
brainctl vec reindexsample_db_embedding_widthsimported + called in_impl.pybut never defined →ImportErroron every runembeddings.py: probe float32 widths of a sample ofvec_memoriesrows, cross-check against the DDL-declared dim so a destructive DROP+CREATE won't paper over a mixed-width/corrupt index without--force.brainctl vsearchtablesincludeevents/context, butvec_events/vec_contextare never created → one missing table aborts the whole command_vsearch_tableagainstOperationalErrorfor an absent vec table and return[]for that source, so results fromvec_memoriesstill surface.dream-cycle/dream-daemondatetime.now()against aware-UTCcreated_at→TypeError: can't subtract offset-naive and offset-aware datetimesparse_tsto always return aware UTC and coerce a naivenowto aware UTC indays_since, so the subtraction is always aware − aware.Why?
All three commands are dead on any real database — confirmed against the source
at
c634808. Details and the rest of the backlog triage are in the plan doc.How to test
python3 -m pytest tests/test_datetime_tz_normalization.py \ tests/test_embedding_width_probe.py \ tests/test_vsearch_missing_vec_table.py -q # 13 new tests # Touched-module regression set stays green: python3 -m pytest tests/test_vec.py tests/test_mcp_tools_vsearch.py \ tests/test_hippocampus_resolve_contradictions.py -qFollow-ups (called out, not done here)
vec_events/vec_contextindexing ordrop them from the default
tablesset — this PR only stops the crash.datetime.now()(naive) appears ~20× inhippocampus.py; several silently compute wrong ages rather than crashing. Arepo-wide standardize-on-aware-UTC pass is worth scheduling.
Checklist
python3 -m py_compilepasses on all touched modulesFixes #160
Fixes #161
Fixes #168
🤖 Generated with Claude Code
https://claude.ai/code/session_018eUi7mmueAJFfBssEezPiP