Sweep comments to the load-bearing minimum

CLAUDE.md

@@ -110,8 +110,9 @@ a session, a transaction or a repository directly. Provider scopes:
 `BigIntBase`. `app/database/tables.py` shares metadata with
 `orm.DeclarativeBase.metadata` (`METADATA = orm_registry.metadata;
 orm.DeclarativeBase.metadata = METADATA`) so Alembic autogen sees everything —
-this line mutates a third-party base class at import time; see the comment
-above it in the source for why. Repositories are
+this line mutates a third-party base class at import time, because otherwise
+models register on `orm.DeclarativeBase`'s own metadata and autogen sees no
+tables at all. Repositories are
 `SQLAlchemyAsyncRepositoryService[Model]` with a nested
 `BaseRepository(SQLAlchemyAsyncRepository[Model])`, same shape as the
 template, but every service here is constructed with `auto_commit=False`.
@@ -172,6 +173,16 @@ env vars (see `docker-compose.yml`). `api_bootstrapper_config` builds the
   mapping table and the one deliberate exception (login's `401` via Litestar's
   own `NotAuthorizedException`) are in `architecture/messages.md` and
   `architecture/auth.md`.
+- **Comments.** None, unless the code would read as a bug without one; then a
+  single line. Rationale, design decisions and "why not X" belong in
+  `architecture/<capability>.md` and `planning/changes/`, never in the source —
+  those are the durable homes, and a comment restating them goes stale in place.
+  What survives in `app/` today is the whole permitted category: a setting that
+  looks arbitrary (`join_transaction_mode`, `populate_existing`,
+  `capture_parameters=False`), an `orm.foreign()` on a column with no
+  `ForeignKey`, a `return` from inside a transaction block, discarded work that
+  is not dead code (`AuthenticateUserUseCase`'s hash-anyway). Alembic's own
+  `# ###` autogenerate markers stay — they are regenerated on every migration.
 - `ruff` is configured with `select = ["ALL"]` and a line length of 120 —
   expect strict lint. Type-check with `ty`; use `# ty: ignore[<rule>]` for
   suppressions.

app/api/app.py

@@ -35,10 +35,6 @@ def build_app() -> litestar.Litestar:
                 ConflictError: exception_handlers.conflict_error_handler,
             },
             route_handlers=[auth_endpoints.ROUTER, chats_endpoints.ROUTER, messages_endpoints.ROUTER],
-            # autowired_groups exposes one Litestar dependency per UseCases provider, named
-            # after the provider attribute - which is what every handler parameter is already
-            # called. Database and Repositories are deliberately left out: route handlers have
-            # no business resolving a session, a transaction or a repository directly.
             plugins=[
                 modern_di_litestar.ModernDIPlugin(di_container, autowired_groups=[ioc.UseCases]),
                 JWTCookieAuthPlugin(),
@@ -47,8 +43,7 @@ def build_app() -> litestar.Litestar:
         ),
         opentelemetry_instrumentors=[
             SQLAlchemyInstrumentor(),
-            # False: bound query parameters include argon2 password hashes (every registration
-            # INSERTs one) - capturing them would ship credential material to the OTel collector.
+            # True would ship argon2 password hashes, bound as INSERT parameters, to the collector.
             AsyncPGInstrumentor(capture_parameters=False),
         ],
     )

app/api/auth.py

@@ -14,21 +14,14 @@
 from app.settings import settings
 
 
-# Every authenticated handler annotates its request with this; `request.user` is a UsersTable
-# because retrieve_user_handler below is what populates it.
 type AuthedRequest = litestar.Request[tables.UsersTable, Token, typing.Any]
 
 
 async def retrieve_user_handler(token: Token, connection: ASGIConnection) -> tables.UsersTable | None:
-    # Auth middleware runs before request-scoped DI is available, so resolve the app-scoped
-    # engine and open a short-lived session through the same factory the container uses. That
-    # factory sets join_transaction_mode="create_savepoint", which is what keeps the per-test
-    # rollback fixture intact when the engine provider is overridden with a live connection.
+    # Auth middleware runs before request-scoped DI exists; see architecture/auth.md.
     try:
         user_id = int(token.sub)
     except ValueError:
-        # Token.sub is only guaranteed to be a non-empty string; a malformed/forged subject
-        # must fail authentication (401 via the middleware), not crash the request (500).
         return None
     di_container: typing.Final = modern_di_litestar.fetch_di_container(connection.app)
     engine: typing.Final = di_container.resolve_provider(ioc.Database.database_engine)
@@ -44,32 +37,17 @@ async def retrieve_user_handler(token: Token, connection: ASGIConnection) -> tab
     token_secret=settings.jwt_secret,
     default_token_expiration=datetime.timedelta(seconds=settings.jwt_lifetime_seconds),
     secure=settings.jwt_cookie_secure,
+    # Anchored: Litestar matches the joined patterns with an unanchored findall.
     exclude=[
-        # /auth/register and /auth/login opt out via exclude_from_auth=True on the handlers
-        # themselves (see app/api/endpoints/auth.py) - that is their one policy home, not here.
-        # Litestar joins these into a single alternation and matches with an unanchored findall
-        # (litestar/middleware/_utils.py), so each pattern is anchored to the path start to avoid
-        # accidentally un-authenticating a future route that merely contains "/docs" or "/health"
-        # as a substring (e.g. "/api/chats/{id}/health").
         "^/docs",
         "^/health",
-        # Swagger's offline assets (settings.swagger_offline_docs=True) are served from here;
-        # without this the docs page loads but every asset request 401s for an anonymous visitor.
         "^/static",
-        # A Prometheus scrape target must be reachable without a session cookie; the endpoint
-        # carries no user data, only process/request metrics.
         "^/metrics",
     ],
 )
 
 
 class JWTCookieAuthPlugin(InitPlugin):
-    # AppConfig has no on_app_init field (that hook is a Litestar.__init__-only parameter,
-    # unavailable through LitestarBootstrapper's AppConfig -> Litestar.from_config path), and
-    # jwt_cookie_auth itself is an unhashable dataclass so it cannot sit in `plugins` directly
-    # (PluginRegistry stores plugins in a frozenset). This plugin wrapper is hashable by identity
-    # and forwards to jwt_cookie_auth.on_app_init, which Litestar.__init__ calls for every
-    # InitPluginProtocol member of `plugins` after the bootstrapper has finished mutating
-    # application_config (so openapi_config is already populated by then).
+    # jwt_cookie_auth is an unhashable dataclass, so it cannot go in `plugins` itself.
     def on_app_init(self, app_config: AppConfig) -> AppConfig:
         return jwt_cookie_auth.on_app_init(app_config)

app/api/exception_handlers.py

@@ -29,8 +29,7 @@ def duplicate_key_error_handler(_: object, __: DuplicateKeyError) -> litestar.Re
 def foreign_key_error_handler(_: object, __: ForeignKeyError) -> litestar.Response[dict[str, typing.Any]]:
     return litestar.Response(
         media_type=litestar.MediaType.JSON,
-        # Constant detail, not str(exc): the underlying integrity error can carry bound
-        # parameter values (e.g. ids from other tables) that shouldn't be echoed back verbatim.
+        # Not str(exc): integrity errors can carry bound parameter values from other rows.
         content={"detail": "Invalid reference"},
         status_code=status_codes.HTTP_400_BAD_REQUEST,
     )

app/database/resources.py

@@ -22,11 +22,7 @@ async def close_database_engine(engine: sa.AsyncEngine) -> None:
 
 
 def create_session(engine: sa.AsyncEngine | sa.AsyncConnection) -> sa.AsyncSession:
-    # join_transaction_mode is inert in production (the session binds to an engine); when tests bind
-    # the session to a connection already in a transaction, it makes the session own a savepoint so
-    # the outer transaction survives commits and the per-test rollback stays clean. The `db_session`
-    # test fixture overrides `Database.database_engine` with a live `AsyncConnection`, so DI-resolved
-    # sessions built through this same function get that connection, not just an `AsyncEngine`.
+    # join_transaction_mode is inert in production; it makes test-bound sessions nest as savepoints.
     return sa.AsyncSession(
         engine,
         expire_on_commit=False,

app/database/tables.py

@@ -10,11 +10,7 @@
 
 
 METADATA: typing.Final = orm_registry.metadata
-# Redirects SQLAlchemy's shared declarative base onto advanced-alchemy's registry metadata so
-# that every model below - which inherits BigIntAuditBase/BigIntBase, themselves built on
-# orm.DeclarativeBase - registers its table on METADATA. Alembic's env.py autogenerates against
-# METADATA directly; without this reassignment, models would register on orm.DeclarativeBase's
-# own separate metadata instead, and autogen would see no tables at all.
+# Without this, models register on orm.DeclarativeBase's own metadata and autogen sees no tables.
 orm.DeclarativeBase.metadata = METADATA
 
 
@@ -56,19 +52,8 @@ class ChatsTable(BigIntAuditBase):
         "ChatMembersTable", lazy="noload", uselist=True, viewonly=True
     )
 
-    # Per-viewer state that the chat listing needs alongside the chat's own columns. Both are
-    # mapped here rather than assembled in Python so that a listed chat is a plain ChatsTable
-    # whose attribute names already match the response schema - no per-row DTO, no aliases.
-    #
-    # unread_count is only populated when the query asks for it via with_expression(); every
-    # other query gets default_expr's literal 0, so the attribute is never None.
     unread_count: orm.Mapped[int] = orm.query_expression(default_expr=sa.literal(0))
-    # last_message_id deliberately carries no ForeignKey (a chats -> messages FK would close a
-    # cycle with messages.chat_id), so the join column has to be annotated foreign() by hand.
-    # The soft-delete guard lives in the join rather than at the call site: a chat must never
-    # preview a deleted message, whatever loads it. DeleteMessageUseCase still repoints
-    # last_message_id in the same commit as the delete - this only keeps the mapping correct on
-    # its own if some other write path ever fails to.
+    # last_message_id carries no ForeignKey (it would cycle with messages.chat_id), hence foreign().
     last_message: orm.Mapped[MessagesTable | None] = orm.relationship(
         "MessagesTable",
         primaryjoin=lambda: sa.and_(
@@ -96,11 +81,7 @@ class ChatMembersTable(BigIntBase):
 class MessagesTable(BigIntBase):
     __tablename__ = "messages"
     __table_args__ = (
-        # No standalone index on chat_id: this composite index already serves every query
-        # that would use one.
         sa.Index("ix_messages_chat_id_id", "chat_id", "id"),
-        # Idempotency is a property of "send this message to this chat" - two different chats
-        # are two different operations, so the key is unique per chat, not table-wide.
         sa.UniqueConstraint("chat_id", "idempotency_key", name="uk_messages_chat_id_idempotency_key"),
     )
 

app/repositories/chats_repository.py

@@ -49,10 +49,7 @@ async def list_for_user(self, user_id: int) -> Sequence[tables.ChatsTable]:
             .join(tables.ChatMembersTable, tables.ChatMembersTable.chat_id == tables.ChatsTable.id)
             .where(tables.ChatMembersTable.user_id == user_id)
             .order_by(sa.func.coalesce(tables.ChatsTable.last_message_id, 0).desc())
-            # Sessions run expire_on_commit=False, so a ChatsTable already in the identity map
-            # keeps whatever unread_count/last_message it was loaded with; without this, a second
-            # listing in the same session would hand back the first one's values. Safe here only
-            # because this query is read-only - populate_existing overwrites in-memory state.
+            # expire_on_commit=False would otherwise leave identity-mapped chats holding stale values.
             .execution_options(populate_existing=True)
         )
         result: typing.Final = await self.repository.session.execute(statement)

app/schemas/api.py

@@ -87,8 +87,6 @@ class Messages(Collection[Message]):
 
 
 class ChatListItem(Chat):
-    # ChatsTable maps unread_count and last_message itself (see app/database/tables.py), so a
-    # listed chat validates straight through from_attributes like any other ORM instance.
     last_message: Message | None = None
     unread_count: int = 0
 

app/settings.py

@@ -13,10 +13,6 @@ class Settings(pydantic_settings.BaseSettings):
     service_name: str = "chat-app"
     service_version: str = "1.0.0"
     service_environment: str = "local"
-    # Enabling this turns on SQLAlchemy's echo/echo_pool (app/database/resources.py), which logs
-    # every statement WITH ITS BOUND PARAMETERS - including argon2 password_hash values on every
-    # registration - and makes Litestar return stack traces in responses. Never set True outside
-    # a throwaway local session.
     service_debug: bool = False
     log_level: str = "info"
 
@@ -30,8 +26,7 @@ class Settings(pydantic_settings.BaseSettings):
 
     jwt_secret: str = INSECURE_JWT_SECRET
     jwt_lifetime_seconds: int = 60 * 60 * 24 * 7
-    # Litestar leaves this unset by default. Production MUST set this to True (it requires
-    # serving over HTTPS); left False here so local http:// development still gets the cookie.
+    # False so local http:// development still gets the cookie; production must set True.
     jwt_cookie_secure: bool = False
 
     opentelemetry_endpoint: str = ""
@@ -47,8 +42,6 @@ class Settings(pydantic_settings.BaseSettings):
     request_max_body_size: int = 1024 * 1024
 
     def ensure_jwt_secret_is_configured(self) -> None:
-        # The whole auth boundary is a token signed with jwt_secret: with the shipped default,
-        # anyone can forge a token for any user.id. Only "local" may run with it.
         if self.service_environment != "local" and self.jwt_secret == INSECURE_JWT_SECRET:
             message = (
                 f"jwt_secret is still the insecure default while service_environment="

app/use_cases/authenticate_user.py

@@ -16,8 +16,7 @@ class AuthenticateUserUseCase:
     async def __call__(self, *, username: str, password: str) -> tables.UsersTable | None:
         user: typing.Final = await self.users_repository.get_one_or_none(username=username)
         if user is None:
-            # Hash anyway: skipping the argon2 work on an unknown username makes the
-            # response measurably faster and turns login into a username oracle.
+            # Discarded, but skipping the argon2 work would turn login timing into a username oracle.
             security.hash_password(password)
             return None
         if not security.verify_password(user.password_hash, password):

app/use_cases/create_chat.py

@@ -32,16 +32,7 @@ async def __call__(self, *, actor: tables.UsersTable, data: CreateChatRequest) -
             low, high = sorted(member_ids)
             direct_key = tables.build_direct_key(low, high)
 
-            # Kept outside the `async with` block below: Transaction.__aexit__ unconditionally
-            # rolls back and closes the session whenever it is left with an open, uncommitted
-            # transaction (session.in_transaction() True with no prior commit()), which expires
-            # and detaches every loaded attribute. A `return` from inside the block after this
-            # read - with no commit following it - would trigger exactly that on `existing`.
-            # (For direct chats specifically, this SELECT autobegins the session's transaction,
-            # so the `async with self.transaction:` block below actually *joins* that same
-            # transaction rather than starting a new one - see Transaction.__aenter__. That does
-            # not change the __aexit__ hazard above; it only means direct and group chats reach
-            # the block by different routes.)
+            # Outside the block: __aexit__ rolls back an uncommitted transaction and detaches this.
             existing = await self.chats_repository.fetch_direct_by_key(direct_key)
             if existing is not None:
                 return existing, False
@@ -58,16 +49,9 @@ async def __call__(self, *, actor: tables.UsersTable, data: CreateChatRequest) -
                     )
                 )
             except DuplicateKeyError:
-                # Two concurrent requests to open the same direct chat both passed the
-                # fetch_direct_by_key pre-check above and both tried to insert; the loser hits
-                # uq_chats_direct_key here. Group chats have no unique constraint on `chats` to
-                # violate, so an unexpected DuplicateKeyError there re-raises and maps to the
-                # standard 409 instead of being funnelled into this direct-chat recovery path.
+                # Group chats have no unique constraint here, so a collision is not recoverable.
                 if direct_key is None:
                     raise
-                # Roll back and re-read the winner's row outside this block (same __aexit__
-                # hazard as the comment above): a `return` from in here without a commit
-                # first would detach whatever `fetch_direct_by_key` loaded, same as before.
                 await self.transaction.rollback()
             else:
                 for user_id in sorted(member_ids):
@@ -81,5 +65,4 @@ async def __call__(self, *, actor: tables.UsersTable, data: CreateChatRequest) -
                 raise RuntimeError(msg)
             return existing, False
 
-        # Kept outside the `async with` block for the same reason as the lookup above.
         return await self.chats_repository.fetch_with_members(chat.id), True

app/use_cases/create_message.py

@@ -43,11 +43,7 @@ async def __call__(
                     )
                 )
             except DuplicateKeyError:
-                # Two concurrent retries of the same (chat_id, key); the loser reads the winner's
-                # row. Roll back and re-read outside this block (Transaction.__aexit__
-                # unconditionally rolls back and closes the session on an open, uncommitted
-                # transaction, which expires every loaded attribute - a `return` from in here
-                # would detach whatever fetch_by_idempotency_key loaded).
+                # Re-read outside the block: __aexit__ rolls back and detaches on an open transaction.
                 await self.transaction.rollback()
             else:
                 await self.chats_repository.update(

app/use_cases/delete_message.py

@@ -28,20 +28,13 @@ async def __call__(self, *, actor: tables.UsersTable, message_id: int) -> None:
                 action="delete",
             )
             if message.deleted_at is not None:
-                # DELETE is idempotent under HTTP semantics: a second delete of an
-                # already-deleted message is not an error, unlike PATCH via EditMessageUseCase.
+                # DELETE is idempotent; a second delete is not an error, unlike an edit.
                 return
             message.deleted_at = datetime.datetime.now(tz=datetime.UTC)
             await self.messages_repository.update(message, item_id=message_id)
 
             chat = await self.chats_repository.get_one(id=message.chat_id)
             if chat.last_message_id == message_id:
-                # chats.last_message_id means "the newest non-deleted message in this chat" - the
-                # chat listing's preview and its ordering both read this column, so repointing it
-                # here (in the same commit as the soft delete) is what keeps a delete from being
-                # only half-effective: leaving it pointed at a deleted message would make the
-                # listing preview show deleted text and rank the chat by a message that no longer
-                # counts.
                 newest = await self.messages_repository.fetch_latest_active(message.chat_id)
                 await self.chats_repository.update(
                     tables.ChatsTable(id=message.chat_id, last_message_id=newest.id if newest is not None else None),