fix(memory): implement entity persistence in MemoryService or remove stub #10455

New issue

Closed

opened 2026-04-18 09:48:37 +00:00 by HAL9000 · 0 comments

HAL9000 commented

2026-04-18 09:48:37 +00:00

Owner

Metadata

Commit Message: fix(memory): implement entity persistence in MemoryService or remove stub
Branch: bugfix/auto3-memory-service-entity-persistence

Background and Context

[AUTO-BUG-3]

MemoryService in src/cleveragents/application/services/memory_service.py exposes a connection_string parameter that implies SQL-backed entity persistence. However, both persistence methods are unimplemented stubs:

_load_from_persistence() contains only pass — entities are never loaded from the database on startup.
_persist_if_needed() sets self._dirty = False without writing any data — entities are silently discarded on process exit.

This creates a silent data-loss bug: callers that supply a connection_string expect entities to survive process restarts, but they do not.

Actual Behavior

# memory_service.py
def _load_from_persistence(self) -> None:
    """Load entities from SQL persistence."""
    # For now, we use a simple in-memory approach
    # Future: Implement actual SQL persistence
    pass  # ← silently does nothing

def _persist_if_needed(self) -> None:
    """Persist entities to SQL if dirty and connection available."""
    if not self._dirty or not self.connection_string:
        return
    # For now, mark as clean without actual persistence
    # Future: Implement actual SQL persistence
    self._dirty = False  # ← marks clean without persisting

Entities stored via MemoryService are lost when the process exits, even when a connection_string is provided.

Expected Behavior

Either:

_load_from_persistence() loads entities from the database on initialization, and _persist_if_needed() writes dirty entities to the database; or
The connection_string parameter is removed and the class is documented as in-memory only, so callers are not misled.

Acceptance Criteria

Entity data survives a process restart when connection_string is provided; or the connection_string parameter is removed and the class is clearly documented as in-memory only.
No silent data loss: if persistence fails, an exception is raised rather than silently marking entities as clean.
All existing MemoryService tests continue to pass.
New TDD test (tagged @tdd_issue, @tdd_issue_<N>, @tdd_expected_fail) verifies entity round-trip through persistence.

Subtasks

Write a Behave TDD scenario that creates a MemoryService with a connection_string, stores an entity, restarts the service, and asserts the entity is still present; tag it @tdd_issue, @tdd_issue_<N>, @tdd_expected_fail.
Implement _load_from_persistence() to load entities from the database on init or remove the connection_string parameter and update all callers.
Implement _persist_if_needed() to write dirty entities to the database or remove the method entirely.
Ensure persistence failures raise an exception rather than silently succeeding.
Remove @tdd_expected_fail tag once the fix is in place.
Verify coverage ≥97% via nox -s coverage_report.
Run nox (all default sessions) and fix any errors.

Definition of Done

This issue is complete when:

All subtasks above are completed and checked off.
A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation.
The commit is pushed to the remote on the branch matching the Branch in Metadata exactly.
The commit is submitted as a pull request to master, reviewed, and merged before this issue is marked done.

Automated by CleverAgents Bot
Agent: new-issue-creator

## Metadata - **Commit Message**: `fix(memory): implement entity persistence in MemoryService or remove stub` - **Branch**: `bugfix/auto3-memory-service-entity-persistence` ## Background and Context > [AUTO-BUG-3] `MemoryService` in `src/cleveragents/application/services/memory_service.py` exposes a `connection_string` parameter that implies SQL-backed entity persistence. However, both persistence methods are unimplemented stubs: - `_load_from_persistence()` contains only `pass` — entities are never loaded from the database on startup. - `_persist_if_needed()` sets `self._dirty = False` without writing any data — entities are silently discarded on process exit. This creates a silent data-loss bug: callers that supply a `connection_string` expect entities to survive process restarts, but they do not. ## Actual Behavior ```python # memory_service.py def _load_from_persistence(self) -> None: """Load entities from SQL persistence.""" # For now, we use a simple in-memory approach # Future: Implement actual SQL persistence pass # ← silently does nothing def _persist_if_needed(self) -> None: """Persist entities to SQL if dirty and connection available.""" if not self._dirty or not self.connection_string: return # For now, mark as clean without actual persistence # Future: Implement actual SQL persistence self._dirty = False # ← marks clean without persisting ``` Entities stored via `MemoryService` are lost when the process exits, even when a `connection_string` is provided. ## Expected Behavior Either: 1. `_load_from_persistence()` loads entities from the database on initialization, and `_persist_if_needed()` writes dirty entities to the database; **or** 2. The `connection_string` parameter is removed and the class is documented as in-memory only, so callers are not misled. ## Acceptance Criteria - [ ] Entity data survives a process restart when `connection_string` is provided; **or** the `connection_string` parameter is removed and the class is clearly documented as in-memory only. - [ ] No silent data loss: if persistence fails, an exception is raised rather than silently marking entities as clean. - [ ] All existing `MemoryService` tests continue to pass. - [ ] New TDD test (tagged `@tdd_issue`, `@tdd_issue_<N>`, `@tdd_expected_fail`) verifies entity round-trip through persistence. ## Subtasks - [ ] Write a Behave TDD scenario that creates a `MemoryService` with a `connection_string`, stores an entity, restarts the service, and asserts the entity is still present; tag it `@tdd_issue`, `@tdd_issue_<N>`, `@tdd_expected_fail`. - [ ] Implement `_load_from_persistence()` to load entities from the database on init **or** remove the `connection_string` parameter and update all callers. - [ ] Implement `_persist_if_needed()` to write dirty entities to the database **or** remove the method entirely. - [ ] Ensure persistence failures raise an exception rather than silently succeeding. - [ ] Remove `@tdd_expected_fail` tag once the fix is in place. - [ ] Verify coverage ≥97% via `nox -s coverage_report`. - [ ] Run `nox` (all default sessions) and fix any errors. ## Definition of Done This issue is complete when: - All subtasks above are completed and checked off. - A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation. - The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly. - The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done. --- **Automated by CleverAgents Bot** Agent: new-issue-creator

Rows
Columns