fix(core): make local Postgres backend usable (migrations, pooling, default project) - #1018
Conversation
The Postgres backend installs the uvloop policy at the process entrypoint (#831/#877), but alembic/env.py applied nest_asyncio assuming apply() would raise ValueError under uvloop and fall through to the thread-based migration fallback. With the installed nest_asyncio, apply() instead silently mis-patches the stdlib loop; the later asyncio.run() then fails with 'this event loop is already running' rather than the 'cannot be called from a running event loop' string the fallback matched, so the thread fallback never triggered and 'bm mcp' against Postgres crashed on startup migration. Detect uvloop up front and skip nest_asyncio (root cause), and broaden the fallback to also recognize the nest_asyncio/uvloop error string (defense in depth). Both decisions move to basic_memory.migration_loop so they are unit testable — env.py runs migrations at import and can't be imported in a test. Verified end to end: migrations + pgvector table init now complete under the uvloop policy against a pgvector testcontainer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
_create_postgres_engine used NullPool (a fresh connection per request), assuming a PgBouncer-style pooler sits in front. That holds for cloud (which overrides get_engine_factory with its own pooled engine) but not for a direct local Postgres: under concurrent writes -- plus each background materialization opening its own connection -- it stormed max_connections and collapsed (p99 478s, 21% write failures at C=32; benchmarks/docs/write-load-benchmark.md). Use AsyncAdaptedQueuePool wired to the existing db_pool_size/db_pool_overflow/ db_pool_recycle config (which were already defined but ignored). SQLite uses a separate engine path and is untouched; cloud overrides the factory so it is unaffected. statement_cache_size=0 stays for PgBouncer transaction-mode compat. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
BasicMemoryConfig.model_post_init skipped default-project seeding whenever the backend was Postgres, using the backend as a proxy for 'cloud mode'. That left a fresh LOCAL Postgres with no default project, so create_memory_project raised 'No default project configured'. Gate the skip on skip_initialization_sync (set by for_cloud_tenant) instead, so stateless/cloud still skips while local Postgres seeds 'main' like SQLite. SQLite and cloud behavior are unchanged. Tests cover both branches. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98d9eeabd4
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
With the 3 enabling fixes on main (PR #1018), ran a true apples-to-apples 2x2: main's direct/inline writes vs the branch's async (deferred + worker-pooled) writes, on SQLite and Postgres, interleaved in one session. The async path beats direct writes on both backends at every concurrency level (~3-4x lower p50, ~2x throughput at C=64); main plateaus while branch scales; Postgres gives the async path the most headroom (C=64 18.5 vs SQLite 10.3/s). Zero write failures in all 16 cells. Supersedes the earlier cross-run '+21ms at C=1' note. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
test_running_on_uvloop_true_when_policy_is_uvloop imports uvloop, which is not installed on Windows (the dep is sys_platform != 'win32'), so it broke the required Windows SQLite unit CI on collection. Mark it skipif win32, matching the existing uvloop policy tests. Addresses Codex P1 review on #1018. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
Mirror the #1018 fix: the uvloop-importing test breaks the Windows SQLite unit CI (uvloop is sys_platform != 'win32'). Mark it skipif win32. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
|
Addressed the Codex P1: |
There was a problem hiding this comment.
💡 Codex Review
basic-memory/src/basic_memory/config.py
Lines 750 to 752 in 52021de
Reconcile seeded Postgres defaults into the database
When BASIC_MEMORY_DATABASE_BACKEND=postgres is used on a fresh local install, these lines now add main only to the config. The startup path still returns early for every Postgres backend in initialize_app (src/basic_memory/services/initialization.py:199), so reconcile_projects_with_config() never creates the matching row in the projects table; get_project_client() then resolves the default from config but /v2/projects/resolve rejects it because the database is empty. Default read/write MCP calls remain unusable until a project is manually created, so local Postgres needs to run project reconciliation or otherwise insert the seeded default when skip_initialization_sync is false.
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The default-project fix seeded 'main' into config for local Postgres, but initialize_app returned early for every Postgres backend, so reconcile_projects_with_config never ran and the seeded default got no row in the projects table — /v2/projects/resolve then rejected it and default MCP calls were unusable until a project was created manually. Gate the skip on skip_initialization_sync (the stateless/cloud marker) instead of the backend, so local Postgres migrates + reconciles like SQLite while cloud still skips. Tests cover the stateless skip and the local-Postgres init path. Addresses Codex P1 on #1018. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
|
Addressed the P1 ( |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
basic-memory/src/basic_memory/services/initialization.py
Lines 229 to 231 in 0646c5e
Run sync initialization for local Postgres CLI
With a local Postgres config (database_backend=POSTGRES, skip_initialization_sync=False), synchronous CLI entrypoints still return here before initialize_app() runs. Commands outside skip_init_commands call ensure_initialization() from src/basic_memory/cli/app.py:107-109, so fresh local Postgres runs skip migrations/reconciliation even though initialize_app() now uses the new skip_initialization_sync gate; the seeded default project can remain missing from the DB until some API/MCP startup initializes it. Gate this wrapper on skip_initialization_sync too.
basic-memory/src/basic_memory/config.py
Lines 812 to 814 in 0646c5e
Create the seeded local Postgres project directory
When ~/basic-memory does not already exist, the new local-Postgres seeding path creates a main project in config, but this validator still treats every Postgres backend as cloud and returns before creating that directory. In a fresh local Postgres MCP/API startup, reconciliation adds the non-existent path to the projects table and sync/watch starts in LOCAL mode, so the initial scan/watch cycle errors on the missing directory instead of starting normally. Only skip path creation for stateless/cloud configs, not all Postgres configs.
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
ensure_initialization (the sync wrapper that CLI entrypoints call via cli/app.py) had the same backend-gated early return as initialize_app, so local Postgres CLI commands still skipped initialization and the seeded default got no DB row. Gate it on skip_initialization_sync as well. Addresses Codex P1 on #1018. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52fcac4ff1
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
ensure_project_paths_exists (a model validator) returned early for every Postgres backend, so a fresh local Postgres never created the seeded default's directory (~/basic-memory or BASIC_MEMORY_HOME) while SQLite did — the sync/watch path then hit the seeded project row with a non-existent directory. Gate the skip on skip_initialization_sync like the other paths. Tests cover create (local PG) and skip (stateless). Addresses Codex P2 on #1018. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a25fc48163
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Gating local init only on skip_initialization_sync regressed BASIC_MEMORY_CLOUD_MODE deployments: in-repo cloud containers build config via ConfigManager (not for_cloud_tenant), so the flag is false and startup fell through to reconcile_projects_with_config, whose synchronize_projects deletes DB projects absent from local config — removing tenant/workspace rows on cloud startup. Add a single skip_local_initialization = skip_initialization_sync or cloud_mode (cloud_mode reads BASIC_MEMORY_CLOUD_MODE) and gate all four paths on it: model_post_init seeding, ensure_project_paths_exists, initialize_app, ensure_initialization. Local Postgres matches neither marker and still initializes like SQLite. Tests cover the cloud-mode skip for seeding and initialize_app. Addresses Codex P1 on #1018. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
|
@codex review |
ensure_initialization (the sync wrapper that CLI entrypoints call via cli/app.py) had the same backend-gated early return as initialize_app, so local Postgres CLI commands still skipped initialization and the seeded default got no DB row. Gate it on skip_initialization_sync as well. Addresses Codex P1 on #1018. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
ensure_project_paths_exists (a model validator) returned early for every Postgres backend, so a fresh local Postgres never created the seeded default's directory (~/basic-memory or BASIC_MEMORY_HOME) while SQLite did — the sync/watch path then hit the seeded project row with a non-existent directory. Gate the skip on skip_initialization_sync like the other paths. Tests cover create (local PG) and skip (stateless). Addresses Codex P2 on #1018. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
Gating local init only on skip_initialization_sync regressed BASIC_MEMORY_CLOUD_MODE deployments: in-repo cloud containers build config via ConfigManager (not for_cloud_tenant), so the flag is false and startup fell through to reconcile_projects_with_config, whose synchronize_projects deletes DB projects absent from local config — removing tenant/workspace rows on cloud startup. Add a single skip_local_initialization = skip_initialization_sync or cloud_mode (cloud_mode reads BASIC_MEMORY_CLOUD_MODE) and gate all four paths on it: model_post_init seeding, ensure_project_paths_exists, initialize_app, ensure_initialization. Local Postgres matches neither marker and still initializes like SQLite. Tests cover the cloud-mode skip for seeding and initialize_app. Addresses Codex P1 on #1018. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Three independent bug fixes that make the local Postgres backend actually
usable. Each is a pre-existing bug on
main(not related to the in-flightaccepted-note refactor), surfaced while benchmarking the write path against a
local Postgres testcontainer.
1.
fix(core): startup migration crash under uvloopThe Postgres backend installs the uvloop policy at the entrypoint (#831/#877),
but
alembic/env.pyappliednest_asyncioassuming it would raiseValueErrorunder uvloop and fall through to the thread-based migration fallback. The
installed
nest_asyncioinstead silently mis-patches the loop;asyncio.run()then fails with "this event loop is already running" — a string the fallback
didn't match — so
bm mcpagainst Postgres crashed on startup migration.Detect uvloop up front and skip
nest_asyncio(root cause) + broaden thefallback match (defense in depth). Logic extracted to
basic_memory.migration_loopwith unit tests (env.py can't be imported in atest — it runs migrations at import).
2.
fix(core): pool connections for local Postgres instead of NullPool_create_postgres_engineusedNullPool(a fresh connection per request),assuming a PgBouncer-style pooler sits in front. That holds for cloud (which
overrides
get_engine_factory) but not for a direct local Postgres: underconcurrent writes it stormed
max_connectionsand collapsed. UseAsyncAdaptedQueuePoolwired to the existingdb_pool_size/db_pool_overflow/db_pool_recycleconfig (already defined, previously ignored). SQLite uses aseparate engine path and is untouched; cloud overrides the factory so it is
unaffected.
3.
fix(core): seed a default project for fresh local PostgresBasicMemoryConfig.model_post_initskipped default-project seeding whenever thebackend was Postgres, using the backend as a proxy for "cloud mode". That left a
fresh local Postgres with no default project, so
create_memory_projectraised"No default project configured". Gate the skip on
skip_initialization_sync(set by
for_cloud_tenant) instead, so stateless/cloud still skips while localPostgres seeds
mainlike SQLite. Tests cover both branches.Verification
Full test suite green on
main+ these three commits (2983 passed). Each commitis independently revertable.
🤖 Generated with Claude Code