database/changeset_repository: SqliteChangeSetStore._plan_map is in-memory only — empty changesets lost on restart #10422

New issue

Open

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

HAL9000 commented

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

Owner

Metadata

Commit message: fix(database): persist changeset_id→plan_id mapping to database in SqliteChangeSetStore
Branch name: fix/sqlite-changeset-store-plan-map-persistence

Background and Context

SqliteChangeSetStore maintains an in-memory _plan_map: dict[str, str] that maps changeset_id → plan_id. This mapping is never persisted to the database. After a process restart, get(changeset_id) returns None for any changeset that was started but has no entries yet, silently losing the changeset reference.

Summary

Evidence

src/cleveragents/infrastructure/database/changeset_repository.py:

class SqliteChangeSetStore:
    def __init__(self, session_factory: Callable[[], Session]) -> None:
        ...
        self._plan_map: dict[str, str] = {}   # ← in-memory only, lost on restart

    def start(self, plan_id: str) -> str:
        """Create a new empty ChangeSet for *plan_id*."""
        changeset_id = str(ULID())
        self._plan_map[changeset_id] = plan_id   # ← stored only in memory
        return changeset_id

    def get(self, changeset_id: str) -> SpecChangeSet | None:
        entries = self._entry_repo.get_entries_for_changeset(changeset_id)
        if not entries:
            plan_id = self._plan_map.get(changeset_id, "")   # ← empty after restart
            if not plan_id:
                return None   # ← returns None for valid empty changeset after restart
            return SpecChangeSet(changeset_id=changeset_id, plan_id=plan_id)
        ...

Impact

Any code that calls store.start(plan_id) and then retrieves the changeset after a restart will get None
The changeset is silently lost — no error is raised
This affects the changeset lifecycle: a changeset that was started but not yet populated with entries cannot be recovered after restart
Callers that depend on get() returning a valid SpecChangeSet for an empty changeset will fail silently

Steps to Reproduce

from cleveragents.infrastructure.database.changeset_repository import SqliteChangeSetStore

store1 = SqliteChangeSetStore(session_factory)
changeset_id = store1.start("01HXYZ1234567890ABCDEFGHIJ")

# Simulate restart
store2 = SqliteChangeSetStore(session_factory)
result = store2.get(changeset_id)
assert result is None  # ← data lost, should be a valid SpecChangeSet

Fix Path

Option A: Persist the changeset_id → plan_id mapping to the database (e.g., a changesets table with changeset_id and plan_id columns).

Option B: Derive plan_id from the entries when available, and store the mapping in the database for empty changesets.

Blocked By

Depends on TDD issue #10399 (failing test must be written first).

Expected Behavior

SqliteChangeSetStore.get(changeset_id) should return the correct SpecChangeSet even after a process restart (i.e., when a new SqliteChangeSetStore instance is created), including for changesets that have been started but have no entries yet.

Acceptance Criteria

A changesets table (or equivalent persistence mechanism) stores changeset_id → plan_id mappings
SqliteChangeSetStore.start(plan_id) persists the new mapping to the database
SqliteChangeSetStore.get(changeset_id) retrieves the plan_id from the database when no entries exist
The failing test from #10399 (test_sqlite_changeset_store_get_survives_restart) now passes
No regression in existing changeset tests
Test coverage >= 97%

Subtasks

Add a changesets table (migration) with changeset_id (PK) and plan_id columns
Update SqliteChangeSetStore.start() to persist the mapping to the database
Update SqliteChangeSetStore.get() to query the database for plan_id when _plan_map misses
Remove or deprecate the in-memory _plan_map field
Ensure the TDD test from #10399 passes
Run full test suite and confirm no regressions

Definition of Done

The TDD failing test from #10399 passes
SqliteChangeSetStore.get() returns a valid SpecChangeSet after restart for empty changesets
Database migration is in place
All existing tests pass
Test coverage >= 97%
PR reviewed and merged

Automated by CleverAgents Bot
Supervisor: Bug Hunt Pool | Agent: bug-hunt-pool-supervisor

Automated by CleverAgents Bot
Agent: new-issue-creator

## Metadata - **Commit message:** `fix(database): persist changeset_id→plan_id mapping to database in SqliteChangeSetStore` - **Branch name:** `fix/sqlite-changeset-store-plan-map-persistence` ## Background and Context `SqliteChangeSetStore` maintains an in-memory `_plan_map: dict[str, str]` that maps `changeset_id → plan_id`. This mapping is never persisted to the database. After a process restart, `get(changeset_id)` returns `None` for any changeset that was started but has no entries yet, silently losing the changeset reference. ## Summary `SqliteChangeSetStore` maintains an in-memory `_plan_map: dict[str, str]` that maps `changeset_id → plan_id`. This mapping is never persisted to the database. After a process restart, `get(changeset_id)` returns `None` for any changeset that was started but has no entries yet, silently losing the changeset reference. ## Evidence `src/cleveragents/infrastructure/database/changeset_repository.py`: ```python class SqliteChangeSetStore: def __init__(self, session_factory: Callable[[], Session]) -> None: ... self._plan_map: dict[str, str] = {} # ← in-memory only, lost on restart def start(self, plan_id: str) -> str: """Create a new empty ChangeSet for *plan_id*.""" changeset_id = str(ULID()) self._plan_map[changeset_id] = plan_id # ← stored only in memory return changeset_id def get(self, changeset_id: str) -> SpecChangeSet | None: entries = self._entry_repo.get_entries_for_changeset(changeset_id) if not entries: plan_id = self._plan_map.get(changeset_id, "") # ← empty after restart if not plan_id: return None # ← returns None for valid empty changeset after restart return SpecChangeSet(changeset_id=changeset_id, plan_id=plan_id) ... ``` ## Impact - Any code that calls `store.start(plan_id)` and then retrieves the changeset after a restart will get `None` - The changeset is silently lost — no error is raised - This affects the changeset lifecycle: a changeset that was started but not yet populated with entries cannot be recovered after restart - Callers that depend on `get()` returning a valid `SpecChangeSet` for an empty changeset will fail silently ## Steps to Reproduce ```python from cleveragents.infrastructure.database.changeset_repository import SqliteChangeSetStore store1 = SqliteChangeSetStore(session_factory) changeset_id = store1.start("01HXYZ1234567890ABCDEFGHIJ") # Simulate restart store2 = SqliteChangeSetStore(session_factory) result = store2.get(changeset_id) assert result is None # ← data lost, should be a valid SpecChangeSet ``` ## Fix Path Option A: Persist the `changeset_id → plan_id` mapping to the database (e.g., a `changesets` table with `changeset_id` and `plan_id` columns). Option B: Derive `plan_id` from the entries when available, and store the mapping in the database for empty changesets. ## Blocked By Depends on TDD issue #10399 (failing test must be written first). ## Expected Behavior `SqliteChangeSetStore.get(changeset_id)` should return the correct `SpecChangeSet` even after a process restart (i.e., when a new `SqliteChangeSetStore` instance is created), including for changesets that have been started but have no entries yet. ## Acceptance Criteria - [ ] A `changesets` table (or equivalent persistence mechanism) stores `changeset_id → plan_id` mappings - [ ] `SqliteChangeSetStore.start(plan_id)` persists the new mapping to the database - [ ] `SqliteChangeSetStore.get(changeset_id)` retrieves the `plan_id` from the database when no entries exist - [ ] The failing test from #10399 (`test_sqlite_changeset_store_get_survives_restart`) now passes - [ ] No regression in existing changeset tests - [ ] Test coverage >= 97% ## Subtasks - [ ] Add a `changesets` table (migration) with `changeset_id` (PK) and `plan_id` columns - [ ] Update `SqliteChangeSetStore.start()` to persist the mapping to the database - [ ] Update `SqliteChangeSetStore.get()` to query the database for `plan_id` when `_plan_map` misses - [ ] Remove or deprecate the in-memory `_plan_map` field - [ ] Ensure the TDD test from #10399 passes - [ ] Run full test suite and confirm no regressions ## Definition of Done - [ ] The TDD failing test from #10399 passes - [ ] `SqliteChangeSetStore.get()` returns a valid `SpecChangeSet` after restart for empty changesets - [ ] Database migration is in place - [ ] All existing tests pass - [ ] Test coverage >= 97% - [ ] PR reviewed and merged --- **Automated by CleverAgents Bot** Supervisor: Bug Hunt Pool | Agent: bug-hunt-pool-supervisor --- **Automated by CleverAgents Bot** Agent: new-issue-creator

HAL9000 added the

labels

2026-04-18 09:38:40 +00:00