Add file/Redis/SQL conversation stores, modernize toolchain - #12
Merged
Merged
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.tomloffered matching[file],[redis],[sql], and[stores]extras. The README documented the calls verbatim:None of those modules existed. Every one of those paths raised
ModuleNotFoundError, so anyone following the README hit a wall on first use.create_storehad no test coverage, which is why CI stayed green over it.New store backends
All three implement the existing
ConversationStoreprotocol and are drop-in interchangeable withInMemoryStore:FileStorellmpane[file]RedisStorellmpane[redis]updated_at, solist_conversations()paginates without scanning the keyspace. Supportsttl.SQLStorellmpane[sql]A couple of deliberate design points:
FileStorevalidates conversation IDs. They become filenames, so they're checked against a strict allowlist and anything else raisesValueError. There are explicit path-traversal tests.RedisStoreandSQLStoredon't close what they don't own. Pass your ownclient=/engine=and it survivesclose(); only a connection llmpane created gets torn down.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.
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 --strictwas configured but never actually run, and had accumulated 29 errors. Those are fixed, and mypy,ruff format --check, and linting of thetests/directory are now CI gates rather than aspirational config.Lint and test dependencies were several majors behind:
.eslintrc.cjs)@vitejs/plugin-reactThree upgrades were deliberately not taken, each for a concrete reason rather than caution:
eslint-plugin-reacthas no ESLint 10 release, so moving to 10 would mean dropping React JSX lint coverage entirely.typescript-eslint@8declarestypescript: ">=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, butMetadataTis intentionally shared across modules soChatMessageandConversationstay parameterized by the same type.The newer
react-hooksrules surfaced three findings. One was a genuine bug — a ref written during render inuseConversations— now synced in an effect. The other two are suppressed with written justifications rather than reshaping working code to satisfy a conservative analysis.Verification
mypy --strictclean, ruff and format clean. Run against 3.12, 3.13, and 3.14.ConversationStoreimplementation 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 typeSession.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:
errors.pyattachesdetails={"original_error": str(exc)}to every classified error, and that reaches the client. Provider exceptions can carry request content or response bodies.done=Truechunk carries a different ID than the onestore.add_messagepersists, 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.StreamChunksendsusage: ModelUsage, but the React types have no corresponding field.Items 1 and 2 are the ones worth prioritizing.