fix: pinned neurons and training-file records survive the round trip - #143
Merged
Conversation
Three deterministic bugs that only a live SurrealDB reproduces, so tests/integration/test_surrealdb_pinning.py (self-skipping without SURREALDB_URL, and CI has no server) went red on main unnoticed: * get_pinned_neuron_ids projected `SELECT VALUE neuron_ids`, which returns a list of arrays. _query unwraps any result whose first element is a list, so that collapsed to the first pinned fiber's array alone, and the caller then iterated those id *strings* character by character. Decay and prune received a set of single characters, matching no neuron id, so they still zeroed and deleted every pinned neuron — the precise data loss this mixin was added to prevent. Projecting the field keeps each row a dict, which _query passes through untouched. * upsert_training_file returned a dash-form uuid while storing the record under the underscore form _to_surreal_id produces, so the id a caller got back never equalled the one get_training_file_by_hash reported for the same row, and re-training a file looked like a new record. The id is now minted as uuid4().hex, already inside [A-Za-z0-9_], so _to_surreal_id is a no-op on it and one id means one row in both directions. Rows written by earlier versions carry the underscore form and keep resolving. * SurrealDB stores NONE by dropping the field, so an untrained row came back from SELECT * with no trained_at key at all and callers testing record["trained_at"] hit KeyError. The row mapper now defaults it. Verified against a real SurrealDB 3.2.0: tests/integration/ is 39 passed, 1 xfailed (was 5 failed, 12 passed in the pinning module), and make verify is green.
acidkill
force-pushed
the
claude/zen-turing-htd66b
branch
from
August 2, 2026 20:17
2d354c5 to
69e92c1
Compare
acidkill
added a commit
that referenced
this pull request
Aug 3, 2026
* fix: catch imports naming a module that no longer exists (#150) check_dead_modules computed reachability from every import it found, but never checked that the imported module actually exists — five benchmark and script files kept importing the SQLite backend removed in 3.0.0, and the guard reported "No unreachable modules." while all five would fail to run. It now resolves every root's imports against the tree and reports the ones that don't. stress_at_scale.py measured only the removed backend and is deleted; the other four (rehearsal_coverage_overhead, benchmark_mem0_vs_nm, benchmark_cognee_vs_nm, e2e_gemini_recall) now require a live SURREALDB_URL instead of silently falling back to SQLite. * fix: remove the dead smem_drift tool and its unwired session-summary write (#151) smem_drift always reported "clean" on the only shipped backend: its two inputs (tag co-occurrence, session summaries) raised on SurrealDB, were caught, and turned into empty results indistinguishable from a real analysis that found nothing. Those two things were never implemented on SurrealDB and never had been -- even before the SQLite backend's removal, temporal/activation drift detection had no working data source. The merge action never merged tags either; it only flipped a status field. Removed entirely: engine/drift_detection.py, mcp/drift_handler.py, the smem_drift MCP tool and its schema, the DETECT_DRIFT consolidation strategy, and the tag-cooccurrence write in the encode hot path. TagNormalizer.detect_drift needs no storage and already backs smem doctor's tag-drift warnings -- it remains and covers the same need. A second, unrelated swallowed failure in the same review: ReflexPipeline periodically tried to persist a session summary via a storage method that was removed with the same backend and never existed on SurrealDB. That write path is removed too; session tracking itself (record_query, get_top_topics) is untouched. BREAKING: the smem_drift MCP tool is gone. Docs/tool count regenerated (57 tools, was 58). * fix: stop hub.py's GET handlers from mutating the shared storage's brain (#152) hub_status and hub_devices take brain_id from the URL path and called storage.set_brain(brain_id) on the process-wide shared storage instance to answer their own lookup. Background maintenance loops (consolidation, decay) read storage.brain_id off that same shared instance on every tick, so a read-only GET for one brain could redirect the next scheduled pass onto a brain the operator never switched to. Both endpoints now read through storage_for_scope, a scoped-storage helper moved to server/dependencies.py from reasoning_training.py (the same pattern already protected its read endpoints with) -- it reuses the shared instance when it is already bound to the requested brain, or opens an isolated one otherwise, without ever mutating the shared instance's brain as a side effect of a read. * ci: run the full test suite against a live SurrealDB (#153) Every existing job either mocked the database or skipped whenever SURREALDB_URL was unset, so the only production backend never ran in CI -- the exact gap that let a query returning plausible garbage (#143's SELECT VALUE bug) reach main undetected by its own new test, which had been skipping since the day it was added. The job starts the official image via a plain `docker run` step: GitHub Actions' `services:` only accepts `options:` (docker-create flags), never a container command, and the image's ENTRYPOINT has no default CMD -- `services:` cannot pass the `start ...` arguments SurrealDB requires. Waits on `/surreal isready`, then runs pytest sequentially (no -n auto: parallel workers sharing one connection produce transaction-conflict errors that are about contention, not correctness). Verified end-to-end against a throwaway container matching this recipe: full suite passes in ~2m12s with zero flakiness. * fix: six small correctness gaps from a v3.0.3 review (#154) - Dashboard tool-stats: get_tool_stats took no `days` argument, so switching the 7/30/90-day range filter changed the daily chart but not the summary above it -- three ranges rendered a byte-identical summary. Now respects the same window as the daily series. - get_tool_stats / get_tool_stats_by_period existed only on the SurrealDB mixin, reachable through `# type: ignore[attr-defined]` with no declaration on NeuralStorage and no in-memory implementation -- AttributeError on any other backend. Declared on the interface with an inert default and a real in-memory implementation. - store.py's shared query helper is typed to return row dicts, which understates what a `SELECT VALUE` query actually returns: the selected field for one row can itself be an array, indistinguishable by shape alone from several scalar rows -- the mechanism behind #143's fixed bug. The one live SELECT VALUE call site (get_connected_neuron_ids) now goes through a separate, honestly-typed _query_values instead. - /health's schema_version moved from the 2.x SQLite constant (40) to the active backend's own version (9 on SurrealDB) with no line in the migration guide explaining it -- monitoring that parses the field could read it as a regression. Documented in Troubleshooting; the same table edit also drops a "drift clusters" carried-state row #151 made stale. - The Settings brain-files panel built and returned a path for every brain regardless of whether a file existed there -- a SurrealDB-only brain has none, so the panel showed a plausible path to nothing. size_bytes was already correctly 0; path is now omitted too when the file does not exist. - Renamed ensure_aiosqlite_or_exit_cli to ensure_sqlite_or_exit_cli: it has only ever checked the stdlib sqlite3 module, never aiosqlite, and the name misled anyone reading the CLI startup path. Internal-only. * fix: preserve ALL-CAPS acronyms that collide with Polish stop words (#64) A contributor's audit rejected the obvious fix -- removing ma/na/co/sa from STOP_WORDS_PL -- because N/A and S.A. don't actually collide: punctuation fragments them below the minimum word length before the stop-word check ever runs, so removing the words would trade real Polish function-word noise for a niche acronym gain. Only the bare uppercase forms (MA, NA, CO, SA) collide. A token that is ALL-CAPS in the source now survives even when its lowercased form is a stop word, but only when the token was uppercase in the original text (str.isupper(), which already rejects a sentence-initial "Ma"/"Na") -- every other token is tokenized and filtered exactly as before. Gated by how much of the whole text is uppercase: fully-capitalized text (a shouted note, a heading) has no acronym to rescue, only every word incidentally capitalized, and must not have its stop-word filtering disabled wholesale. * chore: bump version to 3.3.0 MINOR: mostly bug fixes plus one narrow, disclosed breaking removal (the never-functional smem_drift tool, #151) -- same class of change v3.2.0 shipped as MINOR rather than MAJOR.
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.
Three deterministic bugs that only a live SurrealDB reproduces, so
tests/integration/test_surrealdb_pinning.py (self-skipping without
SURREALDB_URL, and CI has no server) went red on main unnoticed:
get_pinned_neuron_ids projected
SELECT VALUE neuron_ids, which returnsa list of arrays. _query unwraps any result whose first element is a
list, so that collapsed to the first pinned fiber's array alone, and the
caller then iterated those id strings character by character. Decay and
prune received a set of single characters, matching no neuron id, so they
still zeroed and deleted every pinned neuron — the precise data loss this
mixin was added to prevent. Projecting the field keeps each row a dict,
which _query passes through untouched.
upsert_training_file returned a dash-form uuid while storing the record
under the underscore form to_surreal_id produces, so the id a caller got
back never equalled the one get_training_file_by_hash reported for the
same row, and re-training a file looked like a new record. The id is now
minted as uuid4().hex, already inside [A-Za-z0-9], so _to_surreal_id is
a no-op on it and one id means one row in both directions. Rows written
by earlier versions carry the underscore form and keep resolving.
SurrealDB stores NONE by dropping the field, so an untrained row came
back from SELECT * with no trained_at key at all and callers testing
record["trained_at"] hit KeyError. The row mapper now defaults it.
Verified against a real SurrealDB 3.2.0: tests/integration/ is 39 passed,
1 xfailed (was 5 failed, 12 passed in the pinning module), and make verify
is green.