fix(storage): stop swallowing the neuron_state write, and reclaim orphans explicitly - #186
Merged
Merged
Conversation
…hans explicitly Two defects in opposite directions, both silent. add_neuron wrote the neuron and then its neuron_state row inside a bare `except: pass`. A failed state write still returned success, so "the state was written" and "the state write failed" were indistinguishable afterwards. That matters more than a missing row: neuron_state holds access_frequency and activation_level, so a neuron without one looks permanently un-accessed — it never receives the activation boost and is a standing candidate for dead-neuron pruning, for a reason no log records (#174). Both the single and the batch path now log and re-raise; the single path also rolls the neuron back so a retry does not collide with a half-written pair. Nothing ever removed states whose neuron was already gone. Both delete paths clean up correctly today, so survivors are historical — but they are not inert: apply_decay iterates get_all_neuron_states(), so each orphan is decay computed for a neuron that no longer exists, and it inflates any per-pass count taken from that loop. Reclaiming them is an explicitly invoked command, NOT a consolidation stage: smem prune-orphan-states # report only (default) smem prune-orphan-states --apply # delete The detection compares neuron ids against neuron_state.neuron_id, and the two sides spell the same neuron differently: a record id is `neuron:<uuid>` with `-` folded to `_` by _to_surreal_id, while the state keeps the dashed uuid. Comparing them unnormalised marks EVERY live state as an orphan and empties the table. Both sides now go through the same fold, a regression test asserts it, and the operation defaults to reporting rather than deleting — the failure mode is severe enough that it should not run unattended on a schedule.
The reference is generated from the command tree, so adding a command leaves it stale. Caught by the Docs Freshness gate, which derives the truth from code rather than trusting the checked-in file.
Merged
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
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.
Closes #174.
What
Two defects in opposite directions, both silent.
1. A swallowed write reported success (#174)
add_neuronwrote the neuron, then itsneuron_staterow inside a bareexcept: pass. A failed state write still returned the neuron id, so afterwards"the state was written" and "the state write failed" were indistinguishable.
This matters more than a missing row.
neuron_stateholdsaccess_frequencyandactivation_level, so a neuron without one looks permanently un-accessed: it neverreceives the activation boost, and it is a standing candidate for dead-neuron pruning —
for a reason nothing logged.
Both paths now log and re-raise. The single path additionally rolls the neuron back, so a
caller's retry does not collide with a half-written pair. The batch path does not unwind —
its neurons are already written in bulk, and a partial unwind is a worse outcome than a
loud failure the caller can retry.
2. Nothing reclaimed orphaned states
Both delete paths clean up their own state today, so surviving orphans are historical. They
are not inert:
apply_decayiteratesget_all_neuron_states(), so each orphan is decaycomputed for a neuron that no longer exists, and it inflates any per-pass count taken from
that loop — which would directly corrupt the decay telemetry proposed in #183.
Reclaiming is explicit, not automatic
Deliberately not a consolidation stage. The detection compares neuron ids against
neuron_state.neuron_id, and the two sides spell the same neuron differently:Comparing them unnormalised marks every live state as an orphan and empties the table.
Both sides now go through the same fold, and a regression test asserts it. Given that
failure mode, the operation defaults to reporting and runs only when someone asks for it —
it is not something that should execute unattended on a schedule.
count_orphaned_neuron_statesexists so the destructive path can be inspected before it istaken; both it and the delete share one detection function, so they cannot drift apart.
Verification
test_unified_config; it passes in isolation on this branch and onmain, and did notreproduce on re-run — cross-test state, not a regression here)
state coverage was verified unchanged afterwards