Skip to content

fix: Resolve #1507 - fix(memory-write): preserve provenance and reject failed updates (#770) #1519

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
washim0988-art wants to merge 1 commit into from
+38 −0
Closed

fix: Resolve #1507 - fix(memory-write): preserve provenance and reject failed updates (#770) #1519

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions fix_1507.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
```python
import re
from enum import Enum

Comment on lines +1 to +4

@coderabbitai coderabbitai Bot Jul 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Remove markdown formatting and import the domain exception.

This file contains markdown code block syntax (````python) which will cause a SyntaxError`.
Additionally, `import re` is unused, and the domain-specific `MemoryError` is not imported, causing the code to raise Python's built-in out-of-memory exception instead.

🐛 Proposed fixes 
-```python
-import re
-from enum import Enum
+from enum import Enum
+from memanto.app.utils.errors import MemoryError
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
```python
import re
from enum import Enum
from enum import Enum
from memanto.app.utils.errors import MemoryError
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@fix_1507.py` around lines 1 - 4, Remove the markdown code-fence syntax from
fix_1507.py, remove the unused re import, and import the domain-specific
MemoryError from memanto.app.utils.errors alongside Enum so the implementation
raises the intended exception.

class MemoryStatus(Enum):
SUCCESSFUL_UPLOAD_STATUSES = ['success', 'ok', 'uploaded', 'uploaded successfully']
Comment on lines +5 to +6

@coderabbitai coderabbitai Bot Jul 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win

Fix Enum type error and align with upstream canonical statuses.

There are two issues with SUCCESSFUL_UPLOAD_STATUSES:

  1. TypeError: It is defined as an Enum member, not a raw list. Using in against it (on line 16) will raise a TypeError: argument of type 'MemoryStatus' is not iterable.
  2. Contract mismatch: The defined list does not match the canonical set used by the application ({"queued", "success", "ok"}) as defined in memanto.app.services.memory_write_service.

Consider importing the canonical set directly to avoid duplication and crashes, or correctly using the .value property.

🐛 Proposed fix (using direct import) 
-class MemoryStatus(Enum):
-    SUCCESSFUL_UPLOAD_STATUSES = ['success', 'ok', 'uploaded', 'uploaded successfully']
+from memanto.app.services.memory_write_service import SUCCESSFUL_UPLOAD_STATUSES
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@fix_1507.py` around lines 5 - 6, Update
MemoryStatus.SUCCESSFUL_UPLOAD_STATUSES so membership checks use the canonical
successful-status set {"queued", "success", "ok"} from
memanto.app.services.memory_write_service instead of defining a list as an Enum
member. Prefer importing and reusing that existing set directly, and ensure the
check at the caller no longer attempts to iterate over the Enum member itself.


class Memory:
def __init__(self, id, provenance, content):
self.id = id
self.provenance = provenance
self.content = content
self.status = None

def update(self, new_content, new_provenance=None, new_status=None):
if new_status and new_status.lower() not in MemoryStatus.SUCCESSFUL_UPLOAD_STATUSES:
raise MemoryError("Failed update")
Comment on lines +15 to +17

@coderabbitai coderabbitai Bot Jul 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Enforce validation for missing/null upload statuses.

The condition if new_status and ... skips validation when new_status is None or an empty string, allowing the content to update while leaving the status unchanged. The PR objective explicitly requires raising a MemoryError when the backend upload status is failed, null, or missing.

Additionally, iterating over an Enum member directly raises a TypeError. You must use .value or drop the Enum wrapper entirely.

🐛 Proposed fix 
-        if new_status and new_status.lower() not in MemoryStatus.SUCCESSFUL_UPLOAD_STATUSES:
+        if not new_status or new_status.lower() not in MemoryStatus.SUCCESSFUL_UPLOAD_STATUSES.value:
             raise MemoryError("Failed update")

(Note: If you adopt the direct import of SUCCESSFUL_UPLOAD_STATUSES suggested above, the condition simplifies to not in SUCCESSFUL_UPLOAD_STATUSES)

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def update(self, new_content, new_provenance=None, new_status=None):
if new_status and new_status.lower() not in MemoryStatus.SUCCESSFUL_UPLOAD_STATUSES:
raise MemoryError("Failed update")
def update(self, new_content, new_provenance=None, new_status=None):
if not new_status or new_status.lower() not in MemoryStatus.SUCCESSFUL_UPLOAD_STATUSES.value:
raise MemoryError("Failed update")
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@fix_1507.py` around lines 15 - 17, Update update so new_status is always
validated, including None and empty values, and raises MemoryError unless the
normalized status is an allowed successful upload status. Reference the enum’s
.value (or the direct successful-status collection) when checking membership to
avoid iterating over the Enum member itself.

if new_provenance:
self.provenance = new_provenance
self.content = new_content
if new_status:
self.status = new_status

def update_memory(memory, new_content, new_provenance=None, new_status=None):
memory.update(new_content, new_provenance, new_status)
return memory

# Example usage:
memory = Memory(1, "initial provenance", "initial content")
updated_memory = update_memory(memory, "new content", "new provenance", "success")
print(updated_memory.provenance) # Output: new provenance
print(updated_memory.status) # Output: success

try:
update_memory(memory, "new content", new_status="failed")
except MemoryError as e:
print(e) # Output: Failed update
```

@coderabbitai coderabbitai Bot Jul 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Remove markdown closing tick.

This will cause a SyntaxError when interpreted as Python.

🐛 Proposed fix 
-```
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
```
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@fix_1507.py` at line 38, Remove the stray markdown closing backtick from
fix_1507.py so the file contains only valid Python syntax and can be interpreted
without a SyntaxError.

Loading