Skip to content

Add file/Redis/SQL conversation stores, modernize toolchain - #12

Merged
HartBrook merged 4 commits into
mainfrom
feature/store-backends-and-modernization
Aug 10, 2026
Merged

HartBrook merged 4 commits into
mainfrom
feature/store-backends-and-modernization

Conversation

@HartBrook

@HartBrook HartBrook commented Aug 10, 2026 •

Copy link
Copy Markdown
Owner

Summary

Implements the three conversation store backends that llmpane already advertised but never shipped, and brings the build, lint, and type-checking stack back up to date.

The bug this fixes

create_store() accepted "file", "redis", and "sql". pyproject.toml offered matching [file], [redis], [sql], and [stores] extras. The README documented the calls verbatim:

store = create_store("redis", url="redis://localhost:6379")

None of those modules existed. Every one of those paths raised ModuleNotFoundError, so anyone following the README hit a wall on first use. create_store had no test coverage, which is why CI stayed green over it.

New store backends

All three implement the existing ConversationStore protocol and are drop-in interchangeable with InMemoryStore:

Backend Extra Notes
FileStore llmpane[file] One JSON document per conversation. Atomic write-then-rename, so an interrupted write can't truncate a good file.
RedisStore llmpane[redis] JSON values plus a sorted set keyed by updated_at, so list_conversations() paginates without scanning the keyspace. Supports ttl.
SQLStore llmpane[sql] SQLAlchemy 2.0 async (SQLite, PostgreSQL, ...). Timestamps mirrored into columns so ordering and pagination happen in the database.

A couple of deliberate design points:

  • FileStore validates conversation IDs. They become filenames, so they're checked against a strict allowlist and anything else raises ValueError. There are explicit path-traversal tests.
  • RedisStore and SQLStore don't close what they don't own. Pass your own client= / engine= and it survives close(); only a connection llmpane created gets torn down.
  • Failure modes are legible. An unknown backend name lists the valid ones; a missing optional dependency names the extra that provides it, rather than surfacing a raw import error.

Tests are a shared conformance suite parameterized across all four backends, so the contract is verified identically for each, plus dedicated coverage for path safety and connection ownership. Python tests go from 122 to 202.

⚠️ Breaking change

The minimum supported Python is now 3.12 (previously 3.10). Python 3.10 and 3.11 are no longer tested or supported. This lands before any published release, so no released version changes behaviour.

The React package's supported peer range is unchanged (React >= 18).

Toolchain modernization

mypy --strict was configured but never actually run, and had accumulated 29 errors. Those are fixed, and mypy, ruff format --check, and linting of the tests/ directory are now CI gates rather than aspirational config.

Lint and test dependencies were several majors behind:

Before After
ESLint 8 (end of life, .eslintrc.cjs) 9, flat config
typescript-eslint 6 8
Vitest 1 4
jsdom 22 30
@vitejs/plugin-react 4 6
CI actions v4 v7 / v8 / v9
Node (CI) 20 (EOL) 22 and 24
Python (CI) 3.10–3.12 3.12–3.14

Three upgrades were deliberately not taken, each for a concrete reason rather than caution:

  • ESLint 9 rather than 10 — eslint-plugin-react has no ESLint 10 release, so moving to 10 would mean dropping React JSX lint coverage entirely.
  • TypeScript 5.9 rather than 7 — typescript-eslint@8 declares typescript: ">=4.8.4 <6.1.0", so TS 7 isn't lintable yet.
  • UP046 (PEP 695 generics) ignored, with the reason recorded in the config — it scopes type parameters to a single class, but MetadataT is intentionally shared across modules so ChatMessage and Conversation stay parameterized by the same type.

The newer react-hooks rules surfaced three findings. One was a genuine bug — a ref written during render in useConversations — now synced in an effect. The other two are suppressed with written justifications rather than reshaping working code to satisfy a conservative analysis.

Verification

  • Python: 202 passed, mypy --strict clean, ruff and format clean. Run against 3.12, 3.13, and 3.14.
  • React: 130 passed, lint clean, typecheck clean, build OK. Run against Node 22 and 24.
  • Downstream: validated against a production consumer of llmpane without modifying it, by shadowing its vendored copy at import time. Its full llmpane-dependent suite passed unchanged, and a custom ConversationStore implementation still satisfies the re-annotated protocol.

That downstream check paid for itself: it caught a cast("CursorResult[Any]", ...) that was fragile across SQLAlchemy 2.0.x patch releases, which type Session.execute() differently. Now version-robust.

Commits are scoped by area (Python / React / CI) to make review easier. CI is green on all five matrix jobs.

Follow-ups, intentionally not in this PR

Reviewing the codebase turned up four issues that are behaviour and security changes rather than modernization. They're listed here so they aren't lost, and are best handled as separate PRs:

  1. Error detail leakage. errors.py attaches details={"original_error": str(exc)} to every classified error, and that reaches the client. Provider exceptions can carry request content or response bodies.
  2. Terminal message ID mismatch. The done=True chunk carries a different ID than the one store.add_message persists, so clients can hold an ID that doesn't exist server-side. The terminal is also emitted before persistence, so a persistence failure still reports success.
  3. Conversation ID trust. An unknown caller-supplied conversation ID is created under that same ID.
  4. Protocol parity. Python's StreamChunk sends usage: ModelUsage, but the React types have no corresponding field.

Items 1 and 2 are the ones worth prioritizing.

create_store() advertised "file", "redis", and "sql" backends and the
README documented them, but the modules were never implemented, so any
caller following the docs hit ModuleNotFoundError. create_store had no
test coverage, which is why CI stayed green.

Implement all three against the existing ConversationStore protocol:

- FileStore: one JSON document per conversation, atomic write-then-rename.
  Conversation IDs become filenames, so they are validated against a
  strict allowlist to prevent path traversal.
- RedisStore: JSON values plus a sorted set keyed by updated_at, so
  list_conversations() paginates without scanning the keyspace.
- SQLStore: SQLAlchemy 2.0 async, with created_at/updated_at mirrored
  into columns so ordering happens in the database.

redis and sql accept an injected client/engine and do not close what they
do not own. Unknown backends now report the valid names, and a missing
optional dependency reports the extra that provides it.

Tests are a shared conformance suite parameterized across all four
backends, so they are provably interchangeable, plus dedicated coverage
for path safety and connection ownership.

Also raise the Python floor to 3.12 to match the consuming application,
and fix the 29 pre-existing mypy --strict errors so the strict config
that was already declared is actually satisfied.
The lint and test stack had drifted several majors behind: ESLint 8 (end
of life) on legacy .eslintrc.cjs, typescript-eslint 6, Vitest 1, jsdom 22.
The eslint config also pinned React 18.2 while the package builds against
React 19.

Move to flat-config ESLint 9, typescript-eslint 8, Vitest 4, jsdom 30,
@vitejs/plugin-react 6 and jest-dom 7.

Two deliberate holds:

- ESLint 9 rather than 10, because eslint-plugin-react has no ESLint 10
  release and upgrading would mean dropping React JSX lint coverage.
- TypeScript 5.9 rather than 7, because typescript-eslint 8 declares
  typescript ">=4.8.4 <6.1.0", and the consuming application compiles
  against the emitted declarations.

The newer react-hooks rules surfaced three real findings. useConversations
wrote to a ref during render; that is now synced in an effect. The other
two are suppressed with written justifications: a render prop receiving an
event handler that transitively closes over a ref, and fetch-on-mount
flipping isLoading synchronously.

Also drop the stale pnpm-lock.yaml, since CI installs with npm ci, stop
emitting test scaffolding into dist, and raise the compiler target to
ES2022.
CI ran only ruff check against the library source, so mypy --strict,
formatting, and the tests directory were all unenforced despite being
configured.

Gate on ruff check (source and tests), ruff format --check, and mypy.

Refresh the workflow environment, which was several majors behind:
checkout v4 to v7, setup-node v4 to v7, setup-uv v4 to v9,
upload-artifact v4 to v7, download-artifact v4 to v8.

Move the Python matrix to 3.12/3.13/3.14 and Node to 22/24. Node 20 is
end of life and jsdom 30's dependencies require ^22.13 or >=24. The
suite was run against 3.13 and 3.14 locally before claiming support.
astral-sh/setup-uv publishes release v9.0.0 but does not maintain a v9
moving major tag, so the reference could not be resolved and every Python
job failed at Set up job. Pin the exact release instead.
@HartBrook HartBrook changed the title Add file/Redis/SQL stores and modernize the toolchain Add file/Redis/SQL conversation stores, modernize toolchain Aug 10, 2026
@HartBrook
HartBrook merged commit 44bfda5 into main Aug 10, 2026
5 checks passed
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