unsloth/tests/studio/test_chat_thread_title_cas.py
Michael Han 467e564f56
Studio: keep chat titles whole so a wider sidebar shows more of them (#8078)
* Studio: keep chat titles whole so a wider sidebar shows more of them

Chat titles fell back to the first user message cut at 48 characters with
a literal "...", so the clip was baked into the stored title and widening
the sidebar could never reveal more. Store the whole first line and let
the sidebar row clip it with CSS, capped at 120 (the rename maxLength).

Titles already stored pre-cut are rewritten from their first message on
load, in one batched call. A title has to be the exact old cut of its own
message to qualify, so a rename ending in "..." is left alone, and the
patch leaves updatedAt alone so Recents keeps its order.

* Studio: harden the legacy chat title repair

Review follow-ups on the title migration.

Cut the fallback title on code points. A UTF-16 cut can halve an emoji,
and the lone surrogate that leaves parses fine but raises
UnicodeEncodeError when the backend binds it into SQLite, so the write
failed for those chats.

Read the titles again right before patching, so a rename that lands while
the messages request is in flight wins over the rewrite.

Keep a thread marked attempted only once it has been read and written. A
failed read or PATCH now clears the mark so a later refresh retries
instead of leaving the row clipped for the session.

Cap each pass at 100 rows and hold writes to 4 in flight, so a long
history drains over a few refreshes rather than putting its whole backlog
on a synchronous SQLite route at once.

* Studio: guard the legacy title rewrite with a conditional PATCH

The last review round narrowed the rename race to the patch round trip
but could not close it: a read and a write are two requests, so a rename
landing between them was still overwritten.

Add expectedTitle to PATCH /api/chat/threads/{id}. The guard rides in the
UPDATE's WHERE clause, so the check and the write are one statement, and
a stale rewrite gets 409 instead of clobbering the user's title. Other
callers are unaffected: without the field the patch applies as before.

The repair now sends the title it planned from and drops the extra list
call the previous commit added, since correctness comes from the guard
rather than from narrowing the window.

* Studio: keep the title repair moving through a long history

Three review follow-ups on the migration.

Reuse the messages the sidebar just fetched. listStoredChatThreadsWithMessages
already batched every listed thread's messages and threw the map away, so the
repair was refetching up to 100 of the same histories on startup. It now hands
the map over and the repair only calls out when it has none.

Read Dexie when the backend has nothing for a thread. A legacy chat whose
import has not landed yet reads empty from a successful batch call, so the
catch-based retry never fired and the row sat marked with its title still
clipped.

Schedule the next page instead of relying on a write. Paging drained on the
history update each PATCH fires, so a page that wrote nothing left the rest of
the backlog waiting on an unrelated refresh.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Studio: stop a failed repair page starving the rows behind it

The paging added last commit had a hole. Failed writes are unmarked so a
later refresh can retry them, but the next page was selected off the same
thread list, so it drew those rows straight back in: with a failing PATCH
the drain re-ran the same page every 500ms and never reached the rest.
The page now hands back the rows it did not take and the next one reads
from those.

Take the earliest user message rather than the first row of the array. A
Dexie read arrives in index order, so a chat whose ids do not sort by
createdAt could be compared against a later turn and skipped. The local
read is also sorted the way the backend lists messages.

Keep a row retryable when nothing was found for it. A read that failed
was landing in the map as an empty array, which the repair then took as
proof the chat had no opening message and wrote the row off for the
session. The map now carries only what was actually read, and rows that
come back with nothing stay eligible for a later refresh.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Studio: look locally for any title the read could not explain

The local read only fired when a thread came back with no messages at
all, so a part-imported legacy chat, holding its opening turn in Dexie
while later ones are already on the backend, read as complete. The plan
then had no opening message to compare against, found nothing to do, and
left the row marked with its title still clipped.

Any candidate the first plan could not explain now gets the local read,
and the two sources are merged by id before planning again.

* Studio: hold the repair to one pass, one row, one source of truth

Three review follow-ups.

Queue the passes. Several sidebars can be mounted at once and each was
free to claim its own page and its own four write lanes, so
REPAIR_CONCURRENCY only ever bound a single pass.

Let a local read fail on its own. A rejecting Dexie read came out of the
gather step and threw away the whole page, including repairs already
planned from backend data, and returned before scheduling the rest. It
now leaves that one row unexplained and the page carries on.

Stop trusting a local-only message once the import is done. Deleting a
message prunes the backend and leaves the Dexie copy behind, so merging
unconditionally could put a deleted opening prompt back and expand the
title from it. Local rows are now trusted on the same terms
listStoredChatMessages already merges them on: while the import is
unfinished, or when the backend holds nothing.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Studio: repair titles from stored messages only

The local read is gone. Its rule trusted Dexie whenever the backend held
nothing for a thread, and that is also what a chat looks like once every
message in it has been deleted: the prune clears the backend and leaves
the Dexie rows, so the repair could rewrite a title out of a prompt the
user removed. That is the second way this has been wrong, and narrowing
the rule again only moves the next case along.

A chat whose messages are not stored yet now reads as unknown, which
already leaves it retryable, so it gets rewritten on a later pass once
its import lands rather than by reading around the backend. The case
genuinely lost is an import that never completes, where the title stays
clipped.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Studio: repair titles from a read of its own, taken at write time

Two ways deleted content could still reach a title.

The sidebar's map was not backend-only. Its entries for a thread the
batch call returned nothing for come from listStoredChatMessages, whose
zero-backend branch merges the Dexie rows, and a chat whose messages have
all been deleted looks exactly like that. Removing the repair's own Dexie
read last commit closed one door and left this one open.

That map is also fetched at sidebar load, not at write time, so a prompt
deleted after the load could still be expanded into a title much later:
the title guard does not notice, since deleting a message leaves the
title alone.

The repair now makes its own batched call per page, immediately before
planning and writing, and takes nothing from callers. listStoredChatThreadsWithMessages
goes back to its original shape. The window is now one pass rather than
the whole session, though a delete inside that window is still possible.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Studio: confirm the title on the client too, not only in the guard

expectedTitle is enforced by the backend, so it only guards where the
backend knows the field. The desktop app ships its own frontend and can
meet an older one, which ignores the field and applies the write, and
this API layer already carries fallbacks for exactly that pairing. The
repair was assuming a guard it might not have.

Read the page's titles back immediately before writing and drop any row
that no longer holds the title it was planned from. One call for the
page, taken last. Where the backend does enforce expectedTitle the write
stays atomic; where it does not, this is what respects a rename.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Studio: only run the migration where the guard is enforced, and fit the cap

A backend from before expectedTitle drops the field and applies the write,
so on that pairing a rename landing between the title read and the PATCH
was still lost. Probing by sending a conditional patch is not available:
on the backend in question that request is itself the harm. The served
OpenAPI schema answers without touching anything, so the migration now
runs only when ChatThreadPatch declares the field, and anything
unreadable counts as unsupported. A failed probe is not cached, so one
hiccup does not park the migration for the session.

The cap also overran the rename input. It kept 120 units and then added
the ellipsis, and counted code points where maxLength counts UTF-16
units, so an emoji title could reach 241 of them and could not be edited
until a character was deleted. The ellipsis now comes out of the same
120-unit budget, still without splitting a pair.

* Studio: keep an HTTP hiccup from parking the migration

The capability probe cached a false for any non-success response, so a
401 while the token warms up, or a 503 during startup, meant the backend
read as unsupported for the rest of the session and no title was ever
repaired. Only a schema that arrived and parsed settles the question now;
anything else is retried on the next pass.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Studio: guard a repaired title on the prompt it came from

Deleting the opening message does not change the thread title, so
expectedTitle alone still matches and the repair could expand text the
user had just deleted.

The write now also carries expectedOpeningMessageId, checked in the same
UPDATE, so a prompt deleted between the read and the write answers 409
and leaves the title alone. Both sides pick the earliest user message the
same way, breaking a shared timestamp on id.

The schema probe looks for both fields.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Studio: pin that a deleted opening prompt is a decided answer

A candidate with no stored messages may still be importing, so it is
unmarked and retried. One that has messages but no matching opening
prompt is a complete answer: the prompt was deleted or edited and no
later pass can prove the title. Unmarking those would re-select them on
every refresh, since the title stays clipped and keeps matching the
pre-filter.

* Studio: keep the title migration off the storage layer

Three problems on the write path, all from going through
updateStoredChatThread and re-listing the history:

It ensures the thread first, so a thread deleted on another client was
re-imported from the Dexie rows still held here, resurrecting a
conversation the user deleted. The PATCH now goes to the backend
directly, so a missing row stays missing and answers 404.

The pass only runs where the backend enforces the guard, which makes the
client-side title revalidation redundant, and it listed every thread the
account has once per page of 100. Removed.

A chat whose messages were all deleted reads back the same as one still
importing, so it was unmarked and re-read on every refresh for the rest
of the session. The import ledger tells the two apart, and is only
fetched when a page actually has one.

* Studio: tighten the chat title migration comments

* Studio: drop lone surrogates from a title under the cap too

The cut sanitises what it walks, so only over-cap input was safe. A first
line already inside the budget was stored as it came, and an unpaired
surrogate in it survives JSON.stringify, reaches the backend intact and
fails the SQLite bind, so the title write 500s.

Dropping now happens on the whole first line, before whitespace is
collapsed, so a surrogate removed from between two spaces does not leave
the pair behind and one at the end leaves no trailing space.

* Studio: tighten the remaining chat title migration comments

---------

Co-authored-by: Unsloth <michaelhan@Michaels-MacBook-Pro.local>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: danielhanchen <danielhanchen@gmail.com>
2026-08-07 07:09:37 -07:00

185 lines
5.7 KiB
Python

# SPDX-License-Identifier: AGPL-3.0-only
# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved.
"""A title patch can carry guards, so a rename or a deleted opening message
beats a background rewrite.
studio_db imports its siblings by bare name (utils.paths), so the functional
checks run in a subprocess with studio/backend on PYTHONPATH rather than
putting those names on this session's sys.path.
"""
from __future__ import annotations
import json
import os
import subprocess
import sys
from pathlib import Path
import pytest
REPO = Path(__file__).resolve().parents[2]
BACKEND = REPO / "studio" / "backend"
ROUTE = BACKEND / "routes" / "chat_history.py"
PROBE = r"""
import json, sys
from storage import studio_db as db
def message(thread_id, message_id, role, text, created_at):
db.upsert_chat_message({
"id": message_id,
"threadId": thread_id,
"role": role,
"content": [{"type": "text", "text": text}],
"createdAt": created_at,
})
def thread(thread_id, title):
db.upsert_chat_thread({
"id": thread_id,
"title": title,
"modelType": "base",
"createdAt": 1,
"updatedAt": 1,
})
out = {}
# No guard: the write lands, as every other caller expects.
thread("t1", "old")
out["unguarded"] = db.update_chat_thread("t1", {"title": "new"})["title"]
# Guard matches: the rewrite lands.
thread("t2", "legacy title...")
out["guard_match"] = db.update_chat_thread(
"t2", {"title": "whole first line"}, expected_title = "legacy title..."
)["title"]
# A rename landed after the rewrite read the row: the rename has to win.
thread("t3", "legacy title...")
db.update_chat_thread("t3", {"title": "what the user typed"})
try:
db.update_chat_thread(
"t3", {"title": "whole first line"}, expected_title = "legacy title..."
)
out["guard_stale"] = "no error"
except db.ChatThreadPreconditionFailed:
out["guard_stale"] = "mismatch"
out["after_stale"] = db.get_chat_thread("t3")["title"]
# The opening message guard, while that message is still the opening one.
thread("t4", "legacy title...")
message("t4", "m2", "user", "second", 20)
message("t4", "m1", "user", "first", 10)
message("t4", "a1", "assistant", "reply", 15)
out["opening_match"] = db.update_chat_thread(
"t4",
{"title": "whole first line"},
expected_title = "legacy title...",
expected_opening_message_id = "m1",
)["title"]
# The opening message was deleted after the rewrite read it. Its text must not
# be expanded into the title.
thread("t5", "legacy title...")
message("t5", "t5m1", "user", "first", 10)
message("t5", "t5m2", "user", "second", 20)
db.sync_chat_messages("t5", [
{"id": "t5m2", "threadId": "t5", "role": "user",
"content": [{"type": "text", "text": "second"}], "createdAt": 20},
], prune_missing = True)
try:
db.update_chat_thread(
"t5",
{"title": "whole first line"},
expected_title = "legacy title...",
expected_opening_message_id = "t5m1",
)
out["opening_deleted"] = "no error"
except db.ChatThreadPreconditionFailed:
out["opening_deleted"] = "mismatch"
out["after_opening_deleted"] = db.get_chat_thread("t5")["title"]
# A thread whose messages are all gone: the subquery is NULL, still no match.
thread("t6", "legacy title...")
try:
db.update_chat_thread(
"t6",
{"title": "whole first line"},
expected_opening_message_id = "t6m1",
)
out["opening_empty"] = "no error"
except db.ChatThreadPreconditionFailed:
out["opening_empty"] = "mismatch"
# A thread that is gone reads as missing, not as a mismatch.
out["missing"] = db.update_chat_thread(
"gone", {"title": "anything"}, expected_title = "legacy title..."
)
# updatedAt is untouched by a title patch, so Recents keeps its order.
out["updated_at"] = db.get_chat_thread("t2")["updatedAt"]
print(json.dumps(out))
"""
@pytest.fixture(scope = "module")
def probe(tmp_path_factory) -> dict:
env = dict(os.environ)
env["UNSLOTH_STUDIO_HOME"] = str(tmp_path_factory.mktemp("studio_home"))
env["PYTHONPATH"] = str(BACKEND)
result = subprocess.run(
[sys.executable, "-c", PROBE],
capture_output = True,
text = True,
env = env,
)
assert result.returncode == 0, result.stderr
return json.loads(result.stdout.strip().splitlines()[-1])
def test_a_patch_without_a_guard_still_applies(probe):
assert probe["unguarded"] == "new"
def test_the_guard_lets_the_write_through_while_the_title_is_unchanged(probe):
assert probe["guard_match"] == "whole first line"
def test_a_rename_between_the_read_and_the_write_wins(probe):
assert probe["guard_stale"] == "mismatch"
assert probe["after_stale"] == "what the user typed"
def test_a_missing_thread_reads_as_missing_not_as_a_mismatch(probe):
assert probe["missing"] is None
def test_a_title_patch_leaves_updated_at_alone(probe):
assert probe["updated_at"] == 1
def test_the_opening_guard_matches_the_earliest_user_message(probe):
"""Earliest by createdAt, not by insertion order, and not the assistant's."""
assert probe["opening_match"] == "whole first line"
def test_a_prompt_deleted_between_the_read_and_the_write_wins(probe):
assert probe["opening_deleted"] == "mismatch"
assert probe["after_opening_deleted"] == "legacy title..."
def test_a_thread_with_no_messages_left_reads_as_a_mismatch(probe):
assert probe["opening_empty"] == "mismatch"
def test_the_route_turns_a_failed_precondition_into_409():
source = ROUTE.read_text(encoding = "utf-8")
assert 'expected_title = patch.pop("expectedTitle", None)' in source
assert 'expected_opening_message_id = patch.pop("expectedOpeningMessageId", None)' in source
assert "except ChatThreadPreconditionFailed:" in source
assert "status_code = 409," in source