Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 51 additions & 3 deletions backend/app/routes/chats.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from fastapi import APIRouter, Depends, HTTPException
from fastapi.responses import PlainTextResponse, Response
from pydantic import BaseModel, Field, field_validator
from sqlalchemy import func
from sqlalchemy import Text, case, cast, func
from sqlalchemy.orm import Session

from app import activity, auth, models, providers, questions
Expand Down Expand Up @@ -262,6 +262,28 @@ def _owner_chat_summary(chat: models.Chat) -> dict:
}


def _owner_chat_summary_projection(chat) -> dict:
"""Serialize the lightweight row projection used by the drawer list.

``GET /api/chats`` used to hydrate every complete ``Chat`` ORM object merely
to return eight summary fields. On a long-lived instance that decoded tens of
megabytes of transcript JSON on every drawer open and chat switch, contending
with the selected chat's small detail read. Keep transcript inspection inside
the database (``message_count``) and never materialize ``messages`` here.
"""
return {
"id": chat.id,
"title": chat.title,
"updated_at": chat.updated_at.isoformat(),
"activity_at": chat.activity_at.isoformat() if chat.activity_at else None,
"pinned_at": chat.pinned_at.isoformat() if chat.pinned_at else None,
"has_messages": bool(chat.message_count),
"created_by_app_id": chat.created_by_app_id,
"run_status": chat.run_status,
"running": chat.run_status == "running" or is_chat_running(chat.id),
}


def _chat_detail_response(
chat: models.Chat,
*,
Expand Down Expand Up @@ -419,7 +441,33 @@ def list_chats(
# a `desc()` on a nullable column would put NULL last under our
# SQLite collation, but making the boolean explicit is clearer and
# portable.
q = db.query(models.Chat).filter(models.Chat.deleted_at.is_(None))
# Drawer projection only. Selecting the Chat entity here hydrates its full
# ``messages`` JSON column even though the response needs only a boolean; on
# this owner's history that was ~47 MB of JSON decoding per refresh. Compare
# the canonical serialized empty-array value in SQL instead of parsing every
# JSON array with json_array_length: Chat.messages is a non-null list written
# by SQLAlchemy's canonical serializer, and CAST(... AS TEXT) is portable
# across SQLite and PostgreSQL. A future raw-import path must normalize JSON
# text first (PostgreSQL's json type preserves whitespace such as ``[ ]``).
# The database can reject non-empty values from their stored length without
# walking every transcript.
q = db.query(
models.Chat.id,
models.Chat.title,
models.Chat.updated_at,
models.Chat.activity_at,
models.Chat.pinned_at,
models.Chat.created_by_app_id,
# App-created owner-visible chats carry their visibility bit here. Owner
# chats normally keep this NULL, so this remains a tiny projection rather
# than pulling transcript/runtime JSON into the hot path.
models.Chat.agent_settings_json,
models.Chat.run_status,
case(
(cast(models.Chat.messages, Text) != "[]", 1),
else_=0,
).label("message_count"),
).filter(models.Chat.deleted_at.is_(None))
chats = (
q.order_by(
models.Chat.pinned_at.is_(None),
Expand All @@ -433,7 +481,7 @@ def list_chats(
# embedded app panels and stay hidden; an app can opt a spawned, first-class
# owner conversation into the drawer by setting owner_visible at creation.
chats = [c for c in chats if _visible_in_owner_drawer(c)]
return [_owner_chat_summary(c) for c in chats]
return [_owner_chat_summary_projection(c) for c in chats]


@router.get("/session-links")
Expand Down
35 changes: 34 additions & 1 deletion backend/tests/test_chats.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
import asyncio
from uuid import uuid4

from app import memory, questions
from app import memory, models, questions
from app.pending_questions import PendingQuestion
from sqlalchemy import event


def _make_pending() -> PendingQuestion:
Expand Down Expand Up @@ -116,6 +117,38 @@ def test_create_chat_returns_canonical_owner_drawer_summary(client, auth):
assert body["detail"] == detail_body


def test_chat_list_projects_summaries_without_hydrating_transcripts(
client, auth,
):
created = client.post(
"/api/chats",
json={
"title": "Projected row",
"messages": [{"role": "user", "content": "large history sentinel"}],
},
headers=auth,
)
assert created.status_code == 200

hydrated_chat_ids = []

def on_load(chat, _context):
hydrated_chat_ids.append(chat.id)

event.listen(models.Chat, "load", on_load)
try:
listed = client.get("/api/chats", headers=auth)
finally:
event.remove(models.Chat, "load", on_load)

assert listed.status_code == 200
row = next(item for item in listed.json() if item["id"] == created.json()["id"])
assert row["has_messages"] is True
assert hydrated_chat_ids == [], (
"the drawer list must not instantiate Chat objects and decode messages"
)


def test_update_chat_rejects_cross_site_request(client, auth, chat):
cross = client.put(
f"/api/chats/{chat.id}",
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/api/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ export const api = {
},
},
chats: {
list: () => apiFetch('/chats'),
list: (options = {}) => apiFetch('/chats', options),
create: (payload) => apiFetch('/chats', {
method: 'POST',
body: JSON.stringify(payload),
Expand Down
82 changes: 73 additions & 9 deletions frontend/src/components/Shell/Shell.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,11 @@ import {
addCreatedChatToList,
createdChatDetailCache,
currentReusableEmptyChat,
detailIsUntouchedEmptyChat,
enteredEmptySingleScreen,
mergeChatListWithCreatedGuards,
reconcileCreatedChatGuard,
rememberCreatedChat,
reusableChatDetailVerdict,
} from './newChatPolicy.js'
import {
reloadWhenWorkerTakesOver,
Expand Down Expand Up @@ -469,7 +472,20 @@ export default function Shell() {
const { loadTheme } = useTheme()
const queryClient = useQueryClient()
const appsQuery = appQueries.list.useQuery()
const chatsQuery = chatQueries.list.useQuery()
// Create responses are authoritative even when the next NetworkFirst list
// request has to fall back to a just-stale service-worker copy. Reconcile at
// the query function boundary so the protected row never disappears from
// cache/render between fetch settlement and an after-the-fact patch.
const recentlyCreatedChatsRef = useRef(new Map())
const reconcileCreatedChats = useCallback(
rows => mergeChatListWithCreatedGuards(
rows, recentlyCreatedChatsRef.current,
),
[],
)
const chatsQuery = chatQueries.list.useQuery({
reconcile: reconcileCreatedChats,
})
const apps = appsQuery.data ?? []
const chats = chatsQuery.data ?? []
// Warm the model registry as soon as a chat is open so the composer's
Expand Down Expand Up @@ -2596,14 +2612,52 @@ export default function Shell() {
})
if (empty && online) {
try {
const staleEmptyId = empty.id
const res = await apiFetch(
`/chats/${encodeURIComponent(empty.id)}?limit=1`,
{ timeoutMs: 5000 },
)
const detail = res.ok ? await res.json() : null
if (!detailIsUntouchedEmptyChat(detail)) {
let detail = null
if (res.ok) detail = await res.json()
const verdict = reusableChatDetailVerdict({
ok: res.ok,
status: res.status,
detail,
})
if (verdict !== 'empty') {
empty = null
void refreshChats()
reconcileCreatedChatGuard(
recentlyCreatedChatsRef.current,
staleEmptyId,
verdict,
)
if (verdict === 'missing') {
// A 404 is authoritative deletion, not evidence of content.
knownExistingOffListChatIdsRef.current.delete(String(staleEmptyId))
queryClient.setQueryData(chatQueries.keys.all, current => {
if (!Array.isArray(current)) return current
const next = current.filter(
chat => String(chat.id) !== String(staleEmptyId),
)
chatsRef.current = next
return next
})
} else if (verdict === 'occupied') {
// The complete successful detail read has given us the only fact
// New Chat needs: this row is no longer reusable. Publish that
// narrow correction instead of launching a drawer list beside the
// create request. Uncertain/malformed responses leave it unchanged.
queryClient.setQueryData(chatQueries.keys.all, current => {
if (!Array.isArray(current)) return current
const next = current.map(chat => (
String(chat.id) === String(staleEmptyId)
? { ...chat, has_messages: true }
: chat
))
chatsRef.current = next
return next
})
}
}
} catch {
empty = null
Expand All @@ -2620,8 +2674,18 @@ export default function Shell() {
if (creatingChatRef.current) return { chatId: null, reason: 'inflight' }
creatingChatRef.current = true
try {
// Opening the drawer may already have started a list read whose snapshot
// predates this POST. Cancel it before creation so it cannot land later
// and overwrite the optimistic row with a stale list. fetchChats consumes
// TanStack's AbortSignal, making this a real network cancellation rather
// than merely ignoring the query result.
await queryClient.cancelQueries({
queryKey: chatQueries.keys.all,
exact: true,
})
const res = await api.chats.create({ title: 'New chat' })
const chat = await jsonOrThrow(res, 'Chat creation failed')
rememberCreatedChat(recentlyCreatedChatsRef.current, chat)
const detailCache = createdChatDetailCache(chat)
if (detailCache) {
queryClient.setQueryData(chatMessagesQueryKey(chat.id), detailCache)
Expand All @@ -2631,9 +2695,9 @@ export default function Shell() {
chatsRef.current = next
return next
})
// Navigation and first paint can start from the authoritative create response.
// Both wider caches revalidate off the opening path.
void refreshChats()
// Navigation, drawer membership, and first paint all come from the
// authoritative create response. Do not immediately replace it with a
// second list read; ordinary drawer/run events revalidate later.
return { chatId: chat.id, reason: null }
} catch {
return { chatId: null, reason: 'error' }
Expand Down Expand Up @@ -2802,7 +2866,6 @@ export default function Shell() {
function selectChat(id) {
clearChatAttention(id)
navTo('chat', { chatId: id })
refreshChats()
}

async function deleteChat(id) {
Expand Down Expand Up @@ -2830,6 +2893,7 @@ export default function Shell() {
}
// A 404 means the server row is already gone; remove the local phantom.
}
recentlyCreatedChatsRef.current.delete(String(id))
try { sessionStorage.removeItem(`draft:${id}`) } catch {}
// Evict the cached messages so a future chat-ID collision (e.g.
// recovery) can't surface stale content.
Expand Down
Loading
Loading