Rebuild the advertised category set from config and persisted rows - #249
Rebuild the advertised category set from config and persisted rows#249oranjeai wants to merge 1 commit into
Conversation
memU shows the memorize LLM exactly one list of categories: the one built from MemoryService.category_configs. nerve fills that list from config at construction and afterwards only ever appends to it, so it drifts from the DB in both directions. A category created at runtime (web UI -> bridge.create_category) is appended in that process only. After a restart its row is in the DB but absent from the list, so the LLM is never told it exists and can never assign an item to it: a user-created category silently stops receiving memories while continuing to appear in the UI. Recall was unaffected - ctx.category_name_to_id is already rebuilt from the DB. In the other direction, MemoryService.__init__ already inserted every configured category, and _ensure_categories then seeds it through _create_category_impl, whose insertion was unconditional - so a clean first boot advertised each configured category twice. Worse after init: get_or_create_category returns the existing row on a name hit and nothing rebuilds the list, so every repeated create appended again. Three web-UI creates on one existing name gave four identical prompt lines and four copies of one ID in ctx.category_ids, against a single DB row. _rebuild_category_view() recomputes all three derived fields from (effective config, persisted rows), which makes them idempotent and removes the need for any is-it-present check. Config entries stay authoritative and keep their order: a row keeps whatever description it was created with, so letting the row win would make a config.yaml edit stop reaching the prompt. The baseline is memU's effective memory_categories rather than nerve's config, because the two differ on a deployment with no configured categories, where memU substitutes its own defaults and reading nerve's config would wipe them. Names are compared exactly, as memU stores and matches them - the schema has a non-unique name index, so procedures and PROCEDURES are distinct rows and a normalized comparison would silently omit one. Rows are appended name-sorted because list_categories issues no ORDER BY. _update_category_impl gets the same call: it wrote the row and re-embedded but left the advertised description stale for the rest of the process. Two prerequisites in _initialize_impl. The category table is loaded once while the repo cache is guaranteed cold, because Fix 5 short-circuits list_categories on a warm cache; that load sits before the availability flags, so a failure cannot leave the bridge advertising a service it could not initialize. The existing _attach_engine_pragmas() call moves ahead of it so the load runs with the 30s busy_timeout - dispose() recycles pooled connections but cannot re-run a read that already completed. That call and its comment are the only pre-existing lines this change relocates. ctx.category_ids is guarded separately, on the ID rather than on config presence: on first boot a name is already in category_configs while its ID is not yet in ctx, so a shared predicate would wrongly suppress a needed append. Costs no embedding calls: nerve sets ctx.categories_ready = True, which makes memU's own _initialize_categories unreachable, and a CategoryConfig built from a row carries no summary_prompt or target_length, so the one live consumer behaves exactly as it does for a miss today. Found while investigating the category map; not fixed here: a failed configured seed still leaves the category advertised with no persisted row (_ensure_categories discards the create's return value), and a deployment with no configured categories still seeds nothing.
Pre-PR validation gate (click to expand)
Session id: cron:clickhouse-impl-slot-41:20260803-122200 |
Internal second-model review (2 blockers raised, both refuted by measurement; 3 nits fixed) - click to expandAn independent model reviewed this diff cold against the repo and the pinned Both trees were exported with Blocker 1: an empty category config advertises memU's defaults with no rows and no resolvable IDsReal, and correctly described. Not introduced here, and not widened. Every line cited is unchanged context:
The unresolvable set is the same ten memU defaults in both trees. This change Blocker 2: exact-name dedup advertises case variants that the lowercased ID map collapsesAlso real, also pre-existing, and the "memories could be assigned to the wrong With Normalized dedup is not available as a cheap alternative: it silently omits a Nits, all fixed in this round
What I checked independentlyI enumerated the writers and consumers of memU's advertised category state
On test liveness: the 21 new cases are each pinned by at least one mutant, and I re-derived the suite figures from the raw logs rather than trusting the Verdicts: 3 agreed (all nits, all fixed here), 2 disagreed with the |
|
|
|
Closing per @pufit's directive on #247: memU is being rewritten and sunset, and Nerve fixes |
2 participants
Symptom
Two opposite ways the memorize prompt disagrees with the category table.
A category made from the web UI works until the next restart. After it the row is
still in the DB and still shows in the UI, but the memorize LLM is never told it
exists, so it can never assign anything to it: the category silently stops
receiving memories. Recall is unaffected.
In the other direction, a clean first boot advertises every configured category
twice, and nothing rebuilds the list afterwards, so each repeated web-UI create
on an existing name appends again -- three creates give four identical prompt lines
against one DB row.
Root cause
memU shows the LLM exactly one list: the one derived from
MemoryService.category_configs. nerve fills it fromconfig.memory.categoriesatconstruction (
memu_bridge.py:1538-1543) and afterwards only ever appends(
:3034), so it is at once missing every category nerve did not configure andduplicating every one it did. That append was unconditional, and
_create_category_implreaches it on an existing name too, sinceget_or_create_categoryreturns the existing row on a hit.The fix
_rebuild_category_view()recomputescategory_configs,category_config_mapand_category_prompt_strfrom(effective config, persisted rows), which makes themidempotent and removes the need for any is-it-present check. It runs after
_ensure_categories, after a create, and after an update -- the last because_update_category_implwrote the row but left the advertised description stale.Three choices, each pinned by a test that fails without it: config descriptions
win over a row's, the baseline is memU's effective
memory_categoriesratherthan nerve's config, and names compare exactly. The commit message carries
the reasoning for each.
_initialize_implgains one prerequisite: the table is loaded while the repo cacheis cold (Fix 5 short-circuits
list_categorieson a warm cache) and before theavailability flags, so a failure cannot leave the bridge advertising an
uninitialized service.
_attach_engine_pragmas()moves ahead of that load so itruns with the 30s
busy_timeout; that call and its comment are the onlypre-existing lines relocated. No new embedding calls. Two adjacent seeding gaps
are tracked separately.
Validation
21 new cases; 18 fail on a source-only revert with the tests kept. Reverting
only the helper leaves 2 failing and only the load 17, so neither half alone
suffices. 15 mutants, all killed. By name, not count:
tests/goes 7 failed /2933 passed to 7 failed / 2954, identical failed set (pre-existing timezone
artifacts).
Latent today -- this box's 8 rows match its 8 configured names -- but one web-UI
create arms it permanently.