Skip to content

fix(memory): match memory ID prefixes literally 🤖🤖🤖 - #278

Open
Sampoornnagpal wants to merge 2 commits into
NVIDIA-NeMo:mainfrom
Sampoornnagpal:fix/memory-literal-id-prefixes
Open

fix(memory): match memory ID prefixes literally 🤖🤖🤖#278
Sampoornnagpal wants to merge 2 commits into
NVIDIA-NeMo:mainfrom
Sampoornnagpal:fix/memory-literal-id-prefixes

Conversation

@Sampoornnagpal

@Sampoornnagpal Sampoornnagpal commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

resolve_id() treats % and _ in an ID prefix as SQL LIKE wildcards. With one stored memory, update_memory("%%%%%%", ...) updates that record, and forget("______") can archive it even though neither is a prefix of its ID.

Escape LIKE metacharacters and the escape character so prefixes are matched literally. Keep exact matches, abbreviated IDs, and ambiguous-prefix errors. This is incorrect wildcard interpretation in a parameterized query, not SQL injection.

Validation

Eight new regression cases fail before the fix; a ninth verifies literal escape-character handling. Store and public-tool tests cover literal special characters and preservation of records after invalid update/forget requests.

uv run pytest -q packages/nooa-memory/tests/memory/test_memory_store.py packages/nooa-memory/tests/memory/test_memory_contract.py — 35 passed.

Windows/Python 3.12 with external import-only helpers for #84/#85. A broader run also passed 19 reference tests; the existing POSIX absolute-path error-message assertion fails on Windows. Helpers are not included and do not validate POSIX locks/signals.

Related issues

No matching open issue found.

Checklist

  • Ruff lint and formatting pass.
  • Regression and related store/tool tests pass.
  • Existing literal-prefix API contract retained.
  • Existing SPDX headers retained.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed memory ID resolution so %, _, and ! are treated as literal characters rather than wildcard patterns.
    • Prevented wildcard-like prefixes from incorrectly updating, forgetting, or archiving stored memories.
    • Improved reliability when searching for memories using IDs or prefixes containing special characters.
  • Tests

    • Added regression coverage for literal-character handling, unmatched prefixes, exact and prefix resolution, and safe memory operations.

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: c75f4472-d3eb-4708-a7a2-01c73b773338

📥 Commits

Reviewing files that changed from the base of the PR and between 28d862d and 08b32f7.

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

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


📝 Walkthrough

Walkthrough

Changes

The memory store now escapes !, %, and _ before using ID prefixes in SQLite LIKE queries. Tests verify literal matching and prevent wildcard-like prefixes from updating, forgetting, or archiving unrelated memories.

Memory ID resolution

Layer / File(s) Summary
Escape and validate literal ID prefixes
packages/nooa-memory/src/nooa_memory/store.py, packages/nooa-memory/tests/memory/test_memory_store.py, packages/nooa-memory/tests/memory/test_memory_contract.py
resolve_id escapes SQLite wildcard characters and the escape character. Tests verify exact and prefix resolution, plus safe update, forget, and archive behavior.

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

Merge Risk: ⚪ Minimal · up to 08b32

Memory ID prefixes containing SQL wildcard or escape characters now resolve literally, preventing unintended updates or archival of unrelated records. The targeted regression coverage reports passing results, and no current merge-blocking risk remains.

🚥 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 clearly and concisely describes the main change: literal matching of memory ID prefixes in resolve_id(). The emojis add minor noise but do not make the title unclear or unrelated.
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 7 functions across 3 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