Skip to content

fix(memory): refresh derived state after memory updates 🤖🤖🤖 - #277

Open
Sampoornnagpal wants to merge 2 commits into
NVIDIA-NeMo:mainfrom
Sampoornnagpal:fix/memory-update-derived-state
Open

fix(memory): refresh derived state after memory updates 🤖🤖🤖#277
Sampoornnagpal wants to merge 2 commits into
NVIDIA-NeMo:mainfrom
Sampoornnagpal:fix/memory-update-derived-state

Conversation

@Sampoornnagpal

@Sampoornnagpal Sampoornnagpal commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Updating a memory's content leaves its size, token estimate, and sentence count describing the previous text. Updating only tags leaves the embedding describing the previous tags, even though tags are part of embedding_text().

Re-derive content metadata through the Memory schema on content changes and re-embed when the embedding input changes. Preserve record identity and history; unrelated edits and unchanged tags do not call the embedder.

Validation

Five new cases fail before the fix: longer, shorter, and empty content updates, plus tag replacement and removal. uv run pytest -q packages/nooa-memory/tests/memory/test_memory_manager.py packages/nooa-memory/tests/memory/test_memory_todo.py packages/nooa-memory/tests/memory/test_memory_owner.py packages/nooa-memory/tests/memory/test_memory_schema.py — 67 passed.

Windows/Python 3.12, deterministic offline hashing embedder. External import-only helpers bypass #84/#85 (fcntl/SIGUSR2); they are not part of this PR and do not validate locking or signals.

Related issues

No matching open issue found.

Checklist

  • Ruff lint and formatting pass.
  • Regression and related tests pass.
  • Update docstring describes the behavior.
  • Existing SPDX headers retained.

Summary by CodeRabbit

  • Bug Fixes

    • Updated memories now refresh derived metadata when their content changes while preserving existing history and identity.
    • Embeddings are refreshed whenever the text used to generate them changes, including tag updates.
    • Avoided unnecessary embedding updates when relevant text remains unchanged.
  • Tests

    • Added coverage for metadata preservation, reinforcement counts, embedding refreshes, and redundant update prevention.

Signed-off-by: Sampoorn Nagpal <nagpalsampoorn@gmail.com>
@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 4c173fc1-cdb2-46d6-a64e-32de95df7607

📥 Commits

Reviewing files that changed from the base of the PR and between 933ede0 and 3e770ec.

📒 Files selected for processing (1)
  • packages/nooa-memory/tests/memory/test_memory_manager.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/nooa-memory/tests/memory/test_memory_manager.py

Included review availability: Your plan provides up to 12 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

MemoryManager.update now rebuilds memory records when content changes and refreshes embeddings when derived embedding text changes. Tests cover metadata recalculation, field preservation, reinforcement updates, tag-based embedding refresh, and redundant-call avoidance.

Changes

Memory update behavior

Layer / File(s) Summary
Rebuild and embedding comparison
packages/nooa-memory/src/nooa_memory/manager.py
MemoryManager.update validates changed content to refresh derived metadata while preserving stable fields. It compares embedding text after all updates and replaces embeddings when that text changes.
Update integration coverage
packages/nooa-memory/tests/memory/test_memory_manager.py
Tests verify metadata recalculation, preserved identity and history fields, reinforcement increments, tag-triggered embedding regeneration, and skipped redundant embedding calls.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 3e770

Memory updates now refresh derived metadata and embeddings when relevant content changes, with no concrete merge-blocking risk identified.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: refreshing derived memory state after updates. The scope is clear, although the emojis add minor noise.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

Signed-off-by: Sampoorn Nagpal <nagpalsampoorn@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant