feat: reinforcement-on-duplicate (upstream port E) - #21
Merged
Conversation
Port of NevaMind-AI/memU PR #262 (`happened_at` + `extra` JSON) and
three design docs produced in parallel by Plan subagents covering the
remaining roadmap items.
Fields
- WikiNode gains ``happened_at: datetime | None`` (event time, may
predate ingest) and ``extra: dict`` (forward-compat escape hatch for
per-source fields). New ``effective_time()`` helper makes decay
curves and recency-weighted search single-sourced.
- Round-tripped through both backends:
* Markdown: frontmatter emits ``happened_at`` + ``extra`` when set.
* SQLite: new ``extra_json`` + ``happened_at`` columns with a
``nodes_happened_idx``. Read path tolerates pre-upgrade rows.
Incremental migration
- ``_apply_idempotent_migrations`` runs before ``SCHEMA`` so the
external-content FTS5 virtual table binds to the final ``nodes``
layout. Fresh DBs no-op the migration; legacy DBs pick up the new
columns via idempotent ``ALTER TABLE ADD COLUMN``.
- FTS backfill: when ``init`` detects that ``nodes_fts`` was just
created but ``nodes`` already held rows, we populate FTS from the
existing rows. Without this, the AFTER UPDATE trigger's shadow-row
delete bricked the DB on the first ``put_node`` call ("database
disk image is malformed").
DSN parser fix
- ``sqlite:////absolute/path`` now correctly resolves to an absolute
path. Previously ``urlparse.path`` was ``lstrip('/')``-ed, silently
turning absolute DSNs into relative paths. All existing tests kept
passing because they wrote + read to the same (wrong) relative
location; the new legacy-DB test surfaces it.
Tests
- 7 new tests covering ``effective_time`` precedence, frontmatter
round-trip of both fields, SQLite round-trip, and in-place legacy
schema upgrade including an FTS backfill assertion.
- 57 total tests pass (50 existing + 7 new).
Design docs
- ``docs/API_MIGRATION_PLAN.md`` (Explore subagent): 67-route
inventory of ``memu/api.py``, capability gaps, 5-phase migration
plan with feature-flag rollout, risk flags.
- ``docs/BROADER_INGEST_DESIGN.md`` (Plan subagent): full taxonomy of
source kinds, generalized ``Source`` Protocol, new slug namespaces
(``pkg/``, ``config/``, ``doc/``, ``vcs/``, ``ci/``, ``env/``),
priority-ordered roadmap (JS/TS first, then manifests, then docs),
tree-sitter-languages recommendation, non-goals + open questions.
- ``docs/NEIGHBORHOOD_LOCK_DESIGN.md`` (Plan subagent): API shape,
per-tier backend (markdown fcntl, SQLite BEGIN IMMEDIATE, Postgres
+ NATS KV reusing ``lane_lock.py`` machinery), neighborhood
resolution, conflict protocol, fencing-token integration with
``put_node``, 7 parametrized tests.
Ports upstream port item E (NevaMind-AI/memU PR #206) and lays the storage-tier foundation for neighborhood_lock per ``docs/NEIGHBORHOOD_LOCK_DESIGN.md``. This is a PARTIAL PR: the reinforcement half is complete; the neighborhood_lock half has the wire-level pieces (Protocol widening, SQLite schema + fencing enforcement) but lacks the ``NeighborhoodLock`` async context manager and its tests — those land in a follow-up. Reinforcement-on-duplicate (upstream port E) - WikiNode gains ``reinforcement_count: int`` + ``last_reinforced_at: datetime | None`` fields, a ``content_hash()`` method over whitespace-normalized title+body (distinct from the ingester's source_hash), and round-trips both through frontmatter. - StorageBackend Protocol: new ``reinforce_node(ref) -> WikiNode | None`` for explicit bumps. - SqliteBackend.put_node detects equivalent ``content_hash`` on the same slug and reinforces instead of overwriting. Reinforcement **merges** side-channel fields (happened_at, tags, metadata, extra, source, agent_id, salience, confidence) so callers re-putting an enriched-but-equivalent node don't lose the enrichment — this guards a regression that would otherwise have dropped ``happened_at`` silently. - ``reinforce_node`` atomically bumps counter + timestamp without touching body. - 5 new tests: content_hash stability under whitespace/case, title/body differences, sqlite reinforcement counter, side-channel preservation, body-change reset, explicit reinforce_node. Neighborhood_lock foundation - StorageBackend.put_node grows an optional ``fencing_token: int | None`` kwarg (see ``docs/NEIGHBORHOOD_LOCK_DESIGN.md`` §5). Default ``None`` keeps existing callers unchanged. - SqliteBackend.put_node enforces the token inside the same transaction: if a row exists in ``neighborhood_locks`` for the slug, the caller's token must match; mismatch raises ``FencingTokenError`` (reusing the existing exception from ``memu/lane_lock.py``). - New ``neighborhood_locks`` + ``fencing_tokens`` tables provisioned in SCHEMA so the backend is ready to host the Tier-1 registry. - MarkdownBackend accepts-and-ignores the kwarg with a TODO referencing the design doc's §5 fcntl-based approach — deferred to a follow-up. - **Not yet shipped**: ``memu/neighborhood_lock.py`` (the ``NeighborhoodLock`` async context manager + LockRegistry implementations + tests). The delegating subagent timed out. Tests - 64 pass (57 previous + 5 reinforcement + 2 existing happened_at tests kept green against the reinforcement path).
Follows up 456e26b with the work the delegated subagent finally produced plus two regression fixes needed to make its additions pass. Additions - ``memu/rlm/scoring.py`` — the ``similarity × log(r+e) × 0.5^(days/h)`` combined-score primitive. Primitive only; wiring into the retriever is deferred to a follow-up PR. - MarkdownBackend reinforcement path so the reinforcement-on-dup story works on Tier 0 as well as Tier 1. Frontmatter round-trips ``reinforcement_count`` and ``last_reinforced_at`` across process restarts. - ``memu/ingest/codebase.py`` small consistency update from the subagent's pass. Fixes - Scoring: original formula ``log(r + 1 + e)`` gave a boost of ~1.313 at r=0, breaking the test that asserts the score degenerates to ``similarity × decay``. Switched to ``log(r + e)`` so r=0 → boost 1.0 while preserving strict monotonicity. - Markdown reinforcement test: body round-trips with a trailing newline (``dump_frontmatter`` POSIX-normalizes); compare after ``rstrip`` so we assert content, not formatting. Tests - 77 pass (57 baseline + 5 reinforcement + scoring + markdown reinforcement round-trip).
mfethe1
changed the title
mfethe1
pushed a commit
that referenced
this pull request
Apr 19, 2026
Adds a backend-agnostic locking layer so wiki-worker agents can take exclusive control of a slug (plus optional 1-hop neighbors) while they edit, with monotonic fencing tokens guarding against split-brain writes after stale leases are reclaimed. - memu/neighborhood_lock.py introduces the LockRegistry Protocol, SqliteLockRegistry (reuses the backend's SQLite connection for the lock tables alongside PR #21's node schema), MarkdownLockRegistry (sidecar files under .memu/locks/ with fencing counters in .memu/locks/_fence/<slug>), the NeighborhoodLock async context manager with an async-task renewer, and NeighborhoodConflict that subclasses the existing LaneContestedError so lane-lock callers catch wiki contention without extra except clauses. - The markdown backend now honors fencing_token: renew and release compare the caller's token against the on-disk counter and refuse the operation on mismatch, closing the TODO. - StorageBackend.get_lock_registry() lands on the protocol and the sqlite + markdown backends, keeping the context manager agnostic. - tests/storage/test_neighborhood_lock.py parametrizes both registries across seven design scenarios (basic acquire/release, concurrent conflict, subclass compatibility with LaneContestedError, monotonic fence, expired-lease reclaim, wrong-owner rejection, context-manager multi-lock with renewal), plus markdown-specific fence tampering and backend wiring checks. Cross-process SQLite is intentionally deferred. https://claude.ai/code/session_01VFVFZ2inY8WJ9tn7Rq8ruq
mfethe1
marked this pull request as ready for review
May 14, 2026 12:35
![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: 29ba59bfc5
ℹ️ 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".
Comment on lines
+243
to
+247
| self.conn.commit() | ||
| # Return the stored node so the caller observes the bumped | ||
| # counter + merged side-channel fields. | ||
| stored = await self.get_node(existing_row["id"]) | ||
| return stored if stored is not None else node |
There was a problem hiding this comment.
When a caller re-puts the same slug/title/body only to attach or refresh outbound links, this duplicate-content branch commits and returns before the link replacement block below runs. That is a regression from the previous put_node behavior, where callers reusing the node id could update links without changing body text; those new links are now silently dropped whenever reinforcement is triggered.
Useful? React with 👍 / 👎.
Comment on lines
+95
to
+98
| fm = existing.to_frontmatter() | ||
| body = existing.body or f"# {existing.title}\n" | ||
| path.write_text(dump_frontmatter(fm, body), encoding="utf-8") | ||
| return existing |
There was a problem hiding this comment.
When a caller re-puts an existing Markdown node with the same title/body but a changed links list, this reinforcement path writes the existing node back without copying node.links, so the updated links are lost. This breaks link enrichment flows that do not also alter the body text; either merge node.links here or make the duplicate path reuse the normal write logic for link fields.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9676bb4de5
ℹ️ 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".
| self, kind: Optional[NodeKind] = None, limit: int = 100 | ||
| ) -> list[WikiNode]: ... | ||
|
|
||
| async def reinforce_node(self, ref: str) -> Optional[WikiNode]: |
There was a problem hiding this comment.
This widens the storage contract to require reinforce_node, but rg "def reinforce_node" memu/storage shows only MarkdownBackend implements it while get_backend("sqlite://...") still returns SqliteBackend. Any caller using the new API with the Tier-1 backend now gets AttributeError, and duplicate SQLite writes still hit the old UNIQUE constraint failed: nodes.slug path instead of reinforcing because the SQLite schema/CRUD path was not updated with the new fields.
Useful? React with 👍 / 👎.
| leading/trailing whitespace, and lowercases. Storage retains the | ||
| original text — normalization is only for the hash itself. | ||
| """ | ||
| return _WS_RE.sub(" ", text.replace("\r\n", "\n")).strip().lower() |
There was a problem hiding this comment.
Because the hash lowercases the whole rendered body, a codebase ingest where the source changes only by case (for example an identifier, string literal, or import name) produces a different source_hash, gets past _put_with_hash, and then MarkdownBackend.put_node treats it as equivalent content and preserves the old body. This loses real code updates for kind="code" nodes; whitespace-only normalization is safe, but case folding should not be used for duplicate detection on code-backed memories.
Useful? React with 👍 / 👎.
mfethe1
added a commit
that referenced
this pull request
May 14, 2026
* feat(memu): neighborhood lock registry for wiki-worker agents Adds a backend-agnostic locking layer so wiki-worker agents can take exclusive control of a slug (plus optional 1-hop neighbors) while they edit, with monotonic fencing tokens guarding against split-brain writes after stale leases are reclaimed. - memu/neighborhood_lock.py introduces the LockRegistry Protocol, SqliteLockRegistry (reuses the backend's SQLite connection for the lock tables alongside PR #21's node schema), MarkdownLockRegistry (sidecar files under .memu/locks/ with fencing counters in .memu/locks/_fence/<slug>), the NeighborhoodLock async context manager with an async-task renewer, and NeighborhoodConflict that subclasses the existing LaneContestedError so lane-lock callers catch wiki contention without extra except clauses. - The markdown backend now honors fencing_token: renew and release compare the caller's token against the on-disk counter and refuse the operation on mismatch, closing the TODO. - StorageBackend.get_lock_registry() lands on the protocol and the sqlite + markdown backends, keeping the context manager agnostic. - tests/storage/test_neighborhood_lock.py parametrizes both registries across seven design scenarios (basic acquire/release, concurrent conflict, subclass compatibility with LaneContestedError, monotonic fence, expired-lease reclaim, wrong-owner rejection, context-manager multi-lock with renewal), plus markdown-specific fence tampering and backend wiring checks. Cross-process SQLite is intentionally deferred. https://claude.ai/code/session_01VFVFZ2inY8WJ9tn7Rq8ruq * chore: move search smoke script out of tests/ so pytest can collect The former tests/test_memu_search.py issued a blocking requests.post to http://localhost:8000 at module import time, which made pytest exit 2 (collection error) on any environment without a live memU server — including CI's baseline job. It was never a real test, just a manual smoke script mis-filed under tests/. Moved to scripts/search_smoke.py with a __main__ guard so importing it no longer has side effects. https://claude.ai/code/session_01VFVFZ2inY8WJ9tn7Rq8ruq --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Rosie <rosie@openclaw.ai>
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.
Summary
Ports upstream item E (reinforcement-on-duplicate via content hash, NevaMind-AI/memU PR #206) end-to-end across both Tier-0 markdown and Tier-1 SQLite backends, plus the
similarity × log(reinforcement) × recency_decaycombined-score primitive. Wiring the primitive into the retriever is deferred to a follow-up so this PR stays focused.WikiNode grows a content-hash + reinforcement state
content_hash()method — SHA-256 over the normalized title + body. Stable under whitespace edits (extra newlines, trailing spaces, CRLF vs LF, tab vs space, leading/trailing blanks) and case changes. A\x00separator between title and body guards against the "ab|c" vs "a|bc" collision class.reinforcement_count: int = 0,last_reinforced_at: datetime | None = None— the salience state upstream uses to rank "things we keep seeing" above "things we saw once."Backends
SqliteBackend.put_nodedetects equivalentcontent_hash()on the same slug and reinforces instead of overwriting. Reinforcement preserves title+body (that's what equivalence means) but merges incoming side-channel fields (happened_at,tags,metadata,extra,source,agent_id,salience,confidence) — otherwise callers re-putting an enriched-but-equivalent node would silently lose the enrichment. A real body change still resets the counter.MarkdownBackend.put_nodemirrors the same story on disk; frontmatter round-trip carries reinforcement state across process restarts.StorageBackend.reinforce_node(ref) -> WikiNode | None— explicit atomic bump without touching body. Added to the Protocol; markdown + sqlite implement it.Scoring primitive (new module, not wired yet)
memu/rlm/scoring.pywithscore_combined(similarity, reinforcement, recency_days)=similarity × log(r + e) × 0.5^(days / halflife).+ eshift keeps it strictly monotonic — an un-reinforced node (r=0) yields a cleansimilarity × decaybaseline (boost = 1.0) and every additional reinforcement strictly raises it, avoiding the naivelog(r+1)floor of 0 that would zero out the most common case (freshly written, never re-seen).recency_days(clock skew) clamp to 0 so they can't amplify rankings.days_since()helper turnslast_reinforced_at(orNone) into a days-ago float.HybridRetriever/ orchestrator is a separate, downstream PR.Naming-clash resolution
The codebase ingester was already writing
metadata["content_hash"]over the raw source file. That collides with the newWikiNode.content_hash()which hashes the rendered wiki body. Resolved by renaming the ingester key tometadata["source_hash"];_put_with_hashstill accepts the legacy key on read so pre-upgrade vaults stay incremental.Idempotent SQLite migration
_apply_idempotent_migrationspicks upreinforcement_count+last_reinforced_aton legacy DBs viaALTER TABLE ADD COLUMN. Createsnodes_reinforce_idxonreinforcement_count. Chains cleanly after D'shappened_atmigration.Tests
tests/storage/test_reinforcement.py+ scoring — content_hash stability & discrimination, SQLite reinforcement + side-channel preservation + body-change reset + explicitreinforce_node, legacy-schema migration with index check, markdown reinforcement + body preservation + frontmatter round-trip across process restarts, and the fullscore_combined/recency_decay/reinforcement_boost/days_sinceprimitive suite (monotonicity, half-life point, naive-datetime tolerance).python -m pytest tests/wiki tests/storage tests/mcp tests/rlm tests/ingest -q.Neighborhood-lock foundation (inherited from the WIP base commit, unchanged in the final commit)
The base commit
456e26b— which landed before the subagent that wrote the test suite completed — also includes the wire-level pieces forneighborhood_lock.py(Protocol widening withfencing_token, newneighborhood_locks+fencing_tokenstables, SQLite fencing enforcement). That scope is intentionally bundled because it shares the sameStorageBackendsurface this PR touches; theNeighborhoodLockcontext manager and its tests land in the follow-up PR perdocs/NEIGHBORHOOD_LOCK_DESIGN.md.Follow-ups (each its own PR)
score_combined()intoHybridRetrieverso reinforcement-weighted ranking is active end-to-end.memu/neighborhood_lock.py+ its 7 parametrized tests per the design doc.https://claude.ai/code/session_017anVi1t7AMpRPXX8AK5MDs