Refactor Python SDK Database Operations with Context Managers for Better Resource Management

[essiebx]

## Summary

I'd like to propose refactoring the Python SDK's database operations (`sdk/python/src/cq/store.py`) 
to use dedicated context managers for resource cleanup. This would improve code maintainability, 
reduce duplication, and align with the threading/async patterns already established in the codebase.

## Current State

The `LocalStore` class currently has repetitive patterns for lock acquisition and transaction management:

**Pattern 1: Lock acquisition** (appears 6+ times across methods)
```python
with self._lock:
    self._check_open()
    # database operations

Pattern 2: Transaction wrapping (in insert, update, delete)

with self._conn:
    self._conn.execute(...)
    self._conn.executemany(...)

Each method manually coordinates the lock and transaction lifecycle, increasing code volume and
cognitive overhead.

Proposed Solution

Extract two lightweight context managers:

1. Lock + State Guard

@contextmanager
def _locked(self):
    """Ensure lock is held and store is open."""
    with self._lock:
        self._check_open()
        yield

2. Transaction + Lock

@contextmanager
def _transact(self):
    """Execute a database transaction under lock."""
    with self._locked():
        with self._conn:
            yield self._conn

This reduces patterns like:

# Before (8 lines)
with self._lock:
    self._check_open()
    with self._conn:
        cursor = self._conn.execute(...)

# After (3 lines)
with self._transact():
    cursor = self._conn.execute(...)

Benefits

• Reduced duplication: Locks the pattern, eliminates copy-paste
• Clearer intent: Method body focuses on logic, not resource management
• Thread-safety: Single source of truth for lock discipline
• Consistency: Aligns with server backend's SQLAlchemy context manager patterns (with self._db.engine.begin(), with self._db.engine.connect())
• Maintainability: Easier to audit and test resource cleanup

Precedent

The codebase already embraces this pattern:

• LocalStore and Client implement __enter__/__exit__ for lifecycle management
• Server backend uses with self._db.engine.begin() as conn: and with self._db.engine.connect() as conn: extensively
• Python SDK docstring explicitly states: "Use as a context manager or call close() explicitly"

Scope

• New context manager methods in LocalStore
• Refactor insert(), update(), delete(), query(), stats() to use new patterns
• No API changes; existing public interface unchanged
• Add tests for context manager cleanup under normal and exception paths

---

Thoughts on this?

[peteski22]

Thanks for the thorough write-up @essiebx; this is a well-reasoned proposal and the patterns you've identified are real. Happy to see this move forward.

I've opened #396 to track it. Reference it with Closes #396 in your PR and it'll close automatically on merge.

A few notes before you start, so the PR lands cleanly:

Scope — keep it tight. Limit this to LocalStore in sdk/python/src/cq/store.py. No public API changes, no touching the server backend or Client. The goal is internal cleanup only.

On _transact. Have it yield self._conn as you proposed, but please actually use the yielded handle at the call sites (with self._transact() as conn: then conn.execute(...)), rather than continuing to reference self._conn inside the block. Otherwise the yield is dead and the two styles drift. Pick one and apply it consistently.

Lock discipline. self._lock is a non-reentrant threading.Lock, so make sure _transact composes _locked without re-acquiring (your sketch does this correctly — _transact → _locked acquires once). Please add a comment noting the lock is non-reentrant so a future change to _transact doesn't introduce a self-deadlock.

Tests are the bar for acceptance. Add coverage for both paths through each context manager:

• Normal path commits and releases the lock.
• Exception inside the block rolls back the transaction (the with self._conn: contract) and still releases the lock, leaving the store usable for the next call.

A test that asserts the lock is free after an exception (e.g. a subsequent operation succeeds) is the important one.

Out of scope for this PR: read methods that only need _locked (get, all, query, stats) are fine to convert too, but don't change their semantics; they should stay outside any with self._conn: transaction.

If that works for you, go ahead and open a PR against main and link it back here. Looking forward to it.