Bootstrap chat-app skeleton and core chat domain - #1
Merged
Conversation
Brief's ignore list was derived from litestar-sqlalchemy-template on an older ruff that did not yet select CPY001 under ALL. rchat already carries this exact ignore line on current ruff; requiring a copyright header per file in an MIT repo with a root LICENSE is noise.
…e it - reset_override() cleared the whole overrides registry instead of just the overridden database_engine provider; scope it so a later per-test override (a fake hasher, a stubbed use case) can't be silently discarded. - add a test that resolves Database.database_session from a request-scoped child container and asserts it sees an uncommitted write made through the db_session fixture, proving the override is shared through the real DI path the app uses in production, not just the fixture's own session. - drop a tautological __main__ import-only test in favor of omitting app/api/__main__.py from coverage, and a vacuous engine/session isinstance check in favor of one asserting settings-derived pool/url config. - de-duplicate db_session's manual AsyncSession construction by calling create_session(connection) instead, widening its type to accept a connection as well as an engine. - patch migrations/script.py.mako with the import and return-annotation fixes hand-applied to the initial migration, so future autogenerated migrations don't need the same manual patch.
… exclude anchoring) - Add Settings.jwt_cookie_secure and Settings.ensure_jwt_secret_is_configured() startup guard - Fix Collection.from_models annotation, drop the ty suppression it required - Anchor JWT auth-exclude patterns, drop dead /metrics entry, single-home register/login exclusion - Guard retrieve_user_handler against non-numeric token subjects (401, not 500) - Correct login (200) and logout (204) status codes - Move JWTCookieAuthPlugin next to jwt_cookie_auth in app/api/auth.py - Stop capturing SQL bind parameters for asyncpg spans (password hashes were leaking into OTel) - Add auth-boundary tests for tampered cookies and tokens for deleted users - Eliminate all test warnings at the root cause (NamedDependency, longer JWT secrets)
Adds ChatsTable/ChatMembersTable, the create-chat and fetch-chat use cases, and their endpoints. Restructures CreateChatUseCase so both read paths (direct-chat lookup and the post-commit refetch) run outside the Transaction context manager, since its __aexit__ rolls back and closes the session on any query left uncommitted inside the block, detaching the returned row. Adds create_constraint=True to the chat_type enum column so the migration emits the expected CHECK constraint alongside the VARCHAR storage.
…g, enum values) Handles concurrent direct-chat creation via DuplicateKeyError catch and re-read (mirroring Task 5's message-idempotency pattern), keeping the re-read outside the Transaction block for the same detachment reason already established for the other two reads. Adds ForeignKeyError (400) and ValidationError (400) handlers so a bad member_ids reference and a malformed direct-chat request no longer surface as 500/403. Returns tuple[ChatsTable, bool] from CreateChatUseCase so the endpoint can distinguish 201 (created) from 200 (already existed). Fixes the enum column to store lowercase values via values_callable, amending the existing migration in place, and drops the redundant chat_id index.
Removes the pragma: no cover markers that had excluded the entire DuplicateKeyError recovery path from coverage, and adds tests that exercise it for real by swapping in a ChatsRepository stub that simulates the race window while everything else (Transaction, the real committed winner row, the session/savepoint machinery) stays real. Restructures the impossible-state guard into the except clause so an unexpected DuplicateKeyError on a group chat re-raises and maps to 409 instead of narrowing into the direct-chat recovery path. Fixes the Justfile migration recipe's argument quoting and drops a redundant coverage pragma already covered by an omit entry.
Adds the messages table, CreateMessageUseCase (idempotency-key dedup with a DuplicateKeyError race-recovery path) and FetchMessagesUseCase (before_id/ after_id cursor pagination, index-only on ix_messages_chat_id_id), plus the send/list endpoints and DI wiring.
…oping, index cleanup) - reject limit < 1 with ValidationError instead of letting Postgres 500 on it - scope the idempotency-key lookup to (chat_id, idempotency_key) so a reused key from a different chat can no longer return that chat's message; the now-deterministic cross-chat mismatch raises ValidationError instead of an unreachable-pragma'd RuntimeError - drop the redundant ix_messages_chat_id index, superseded by the composite (chat_id, id) index, amending the not-yet-deployed migration in place - drop the extra get_one() re-fetch on the happy send path (no relationship to load, unlike create_chat's members) - make list_messages' cursor/limit params keyword-only to retire the PLR0917 suppression
Aligns the DB constraint with the already chat-scoped lookup so the DuplicateKeyError recovery guard's "the unique constraint guarantees a match" pragma is honest again, matching create_chat.py's precedent. Cross-chat key reuse is now a legitimate independent send rather than an error.
Edit and delete require message authorship, not just chat membership. Editing a deleted message is a 409 ConflictError (the author is authorized; the request conflicts with resource state), while deleting an already-deleted message is idempotent and returns 204.
…n check Edit and delete previously checked only message authorship, unlike every other actor-scoped use case (FetchMessagesUseCase, FetchChatUseCase), which check chat membership first. Extract the shared lookup+authorization block into fetch_message_for_author so the check order (existence, membership, authorship) is defined once.
carol/mallory non-member tests pass identically with or without the membership check, since the authorship check alone already rejects them. Add a test that removes alice's own chat_members row after she authored a message, the one state where authorship and membership disagree, so edit/delete are genuinely exercised by a check that would otherwise be silently deletable.
Adds GET /api/chats/ (unread counts and last message, no N+1) and
POST /api/chats/{id}/read/ with a monotonic, chat-scoped marker.
…read-marker, listing tests) Repoints chats.last_message_id when the deleted message was the pointer, so the listing preview and ordering stay consistent with a delete instead of surviving it half-effective. Makes mark-read monotonicity atomic via GREATEST() in the UPDATE instead of a Python read-modify-write, and tightens the listing tests (per-row unread counts, delete-repoint coverage, dropped an untestable case).
Documents the app as it actually shipped (error vocabulary, author-and-member message gating, per-chat idempotency scoping, atomic last_message_id repoint) rather than the original spec, wires the portable planning convention (templates, .convention-version, index.py, check-planning/index recipes), and carries the reviewed-and-deferred execution findings into planning/deferred.md.
…nto CI readme.md and the change file's finalized summary said "read receipts", the exact term architecture/glossary.md tells readers to avoid; reworded to read marker / unread counts, which is what the code actually implements. The Justfile's check-planning comment claimed CI runs the validator when it didn't; added the planning/index.py --check step to the lint job so the claim is true rather than correcting the comment down to match reality.
echo=True/echo_pool=True logs every SQL statement with its bound parameters, including argon2 password_hash values on every registration, and makes Litestar return stack traces in responses. This directly contradicted the AsyncPGInstrumentor(capture_parameters=False) already in app/api/app.py. Document the risk on Settings.service_debug so it isn't re-enabled by pattern-matching on the template.
swagger_offline_docs=True serves Swagger's assets from /static/*, which the JWT auth middleware's exclude list didn't anchor - /docs loaded but every asset request 401ed for an anonymous visitor. /metrics (registered by lite-bootstrap's prometheus_client integration) had the same problem: a scrape target returning 401 is a broken feature, and the endpoint carries no user data.
login's decorator declared no status_code, so the published schema said
201 even though jwt_cookie_auth.login(response_status_code=...) always
returns 200 at runtime; declare it explicitly like register/mark_read
already do.
POST /api/chats/ and POST /api/chats/{id}/messages/ return 201 on create
and 200 on an idempotent hit - the dual-status behaviour this repo exists
to demonstrate - but Swagger only documented the decorator's default.
Declare the 200 case via responses={...} on both handlers.
list_chats hand-built ChatListItem.model_validate(row.chat).model_copy(
update={...}) to inject unread_count/last_message - model_copy(update=)
skips validation, unlike every other collection response in this repo.
Give ChatListItem a from_row classmethod that validates chat, last_message
and unread_count together, and build the response with
schemas.Chats.from_models(...) like messages.py's list_messages already
does.
app/settings.py referenced "(Task 3)", and edit_message.py/mark_read.py each claimed to share a strategy with "Task 5's" sibling use case - none of that numbering means anything to a reader of the published repo, and edit_message.py's comparison was also wrong: CreateMessageUseCase returns from outside the async with block, the opposite of EditMessageUseCase's return-inside-right-after-commit shape. Drop the cross-references rather than replace them with another brittle inter-file comparison.
_register, _login, _create_direct_chat and _send were copy-pasted across four tests/api/*.py modules, and _send had silently diverged - only test_messages_api.py's version took a key parameter. Move all four into a shared module and reconcile _send on the key-accepting signature; each test module imports what it needs with the existing leading-underscore call-site names.
orm.DeclarativeBase.metadata = METADATA (app/database/tables.py) mutates a third-party base class at import time with no in-file explanation - the reasoning only existed in CLAUDE.md and planning/deferred.md. Put it in the file itself, and drop the now-redundant deferred.md entry.
Both use cases carried a `# pragma: no cover` on the "recovery re-read found nothing" branch, excused as unreachable given the unique constraint that guarantees a winner row exists. Add repository doubles whose fetch_direct_by_key/fetch_by_idempotency_key always return None while create() always raises DuplicateKeyError, proving each use case raises RuntimeError instead of returning None silently, and drop both pragmas.
"0 warnings" has been a stated constraint through every prior task with nothing in pytest config actually guarding it. Turning warnings into errors surfaced none in this suite - just wires the enforcement up.
Copyright year (2021) and license = "MIT License" (not a valid SPDX expression per PEP 639) were both inherited from a sibling repo this one was bootstrapped from. This is a repository people are meant to copy from, so fix both: the year to 2026, and the SPDX expression to "MIT".
It's 183 lines with zero coverage, escaping the 100% gate only because planning/ has no __init__.py so coverage's package walk never reaches it - an accident of discovery, not a stated exemption. Add it to [tool.coverage.run] omit. migrations/env.py's `# pragma: no cover` on is_offline_mode() was redundant with migrations/* already being in that same omit list; drop it.
"After git clone run just --list" was the whole quickstart. Add a line each for just run and just test so a reader can actually start the app from the readme.
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
1 participant
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.
Implements
planning/changes/2026-08-21.01-chat-app-bootstrap.md— see that file for the design and its rationale.Stands up the repository and the synchronous half of the domain: package skeleton derived from
litestar-sqlalchemy-template, onemodern-dicontainer spanning app and request scopes, JWT cookie authentication, and the chats / members / messages model over REST.What's here
JWTCookieAuthwith argon2, cookie-based specifically becauseEventSourcecannot send anAuthorizationheader, so the SSE stream added by the realtime change will authenticate identically to every other endpoint.direct_key, created through aDuplicateKeyError-and-re-read upsert rather than a read-then-race.(chat_id, idempotency_key), cursor pagination in both directions on(chat_id, id), author-and-member-gated edit and soft delete.last_read_message_idper member rather than per-message receipt rows, counted withIS DISTINCT FROMso system messages are not silently dropped.Notes for review
auto_commit=False; use cases own the transaction boundary, because the realtime change must write a domain row and an outbox row in one commit.modern-di3.x usescache=, not the 2.xcache_settings=the sibling template still uses.filterwarnings = ["error"]). No coverage pragmas inapp/,tests/ormigrations/.planning/deferred.md.Realtime delivery (outbox, SSE, typing and presence) and the browser client are scoped to follow-on changes.