xiawiie 2e15e3fe0d
fix: generate fallback title for interrupted first-turn runs (#3874)
* fix: generate title for interrupted first turn

* test(title): cover partial-exchange + dict-form messages

Harden the interrupted-run fallback path added in 19fc34fd:

- TitleMiddleware._should_generate_title now accepts a lone first-turn
  user message when allow_partial_exchange=True, so the worker can still
  derive a title if cancellation lands before any AI chunk is checkpointed.
- runtime/runs/worker._ensure_interrupted_title computes the next
  checkpoint step defensively (treat missing/non-int step as 0) and
  renames a shadowed ckpt_config local for readability.
- Add four unit tests in tests/test_title_middleware_core_logic.py:
  partial-exchange allows user-only, partial-exchange still respects an
  existing title, dict-form messages are recognized, and the sync
  fallback path derives a title from dict-form messages — matching what
  channel_values stores in the checkpoint.

Refs #3859.

* fix: persist interrupted-title via channel_versions bump

Address PR #3874 review feedback: ``_ensure_interrupted_title`` previously
called ``aput(..., new_versions={})``. LangGraph's DB-backed savers
(``PostgresSaver`` and the v4 ``SqliteSaver`` blob layout) strip inline
``channel_values`` from ``put`` and only persist blobs for channels named
in ``new_versions`` — so the fallback ``title`` channel was dropped on
read-back and ``threads_meta.display_name`` stayed ``"Untitled"`` after
refresh on those backends. The original in-memory e2e passed because
``InMemorySaver`` keeps the inline snapshot verbatim.

Fix mirrors ``_rollback_to_pre_run_checkpoint`` in the same file: bump
``channel_versions["title"]`` (via ``checkpointer.get_next_version`` when
available, else int/string fallbacks), persist the new version on the
checkpoint, and declare it in ``new_versions`` so the DB savers actually
write the blob.

Regression coverage in ``tests/test_run_worker_rollback.py``:

- ``test_ensure_interrupted_title_bumps_channel_version_and_declares_it_in_new_versions``
  — exact ``aput`` invariants: ``new_versions == {"title": 1}``, the
  written checkpoint's ``channel_versions["title"]`` is bumped, and the
  pre-existing ``messages`` version is preserved.
- ``test_ensure_interrupted_title_bumps_existing_string_version`` —
  string-shaped prior version (some savers use UUID-style versions);
  bumped value must differ from the prior, no overwrite-in-place.
- ``test_ensure_interrupted_title_skips_when_title_already_set`` — title
  short-circuit; no extra ``aput``.
- ``test_ensure_interrupted_title_returns_none_when_no_checkpoint`` —
  no checkpoint yet; returns ``None`` without writing.
- ``test_ensure_interrupted_title_round_trip_with_real_sqlite_checkpointer``
  — full round-trip against a real ``AsyncSqliteSaver`` on a disk-backed
  DB, then closes and re-opens the saver to simulate a fresh
  connection. The fallback title must still be present on the second
  ``aget_tuple``. This is the exact scenario the review flagged.

Validated locally with the full backend suite: 5195 passed, 18 skipped.

Refs #3859. Addresses review on #3874.

* test(worker, title): harden interrupted-title fallback for every saver

Defensive coverage on top of the channel_versions fix (commit 05253957),
addressing edge cases surfaced during a second-pass review of #3874.

Worker:
- Extract version bump into ``_bump_channel_version(checkpointer, current)``
  with explicit fallbacks for int / float / numeric-string / UUID-shaped
  string / None / bool, AND a wrap-around defense when the saver's
  ``get_next_version`` raises or returns an unchanged value. The
  invariant is: returned version MUST differ from the prior. Without
  this, a saver bug (or a custom backend) could leave
  ``new_versions={"title": v}`` no-op on DB savers — the very class of
  bug the original review pointed out.

Title middleware:
- Coerce ``state.get("messages")`` from ``None`` to ``[]`` on both
  ``_should_generate_title`` and ``_build_title_prompt``. A
  partially-initialized checkpoint can carry ``messages=None`` on the
  channel_values channel (the worker reads raw channel_values, not
  BaseMessages), and the default kwarg only protects against a missing
  key. Repro: ``TypeError: 'NoneType' object is not iterable`` from
  the next() generator — confirmed by reverting the fix and watching
  ``test_*_handles_none_messages_channel`` go red.

Tests (TDD-verified red→green for the new asserts):
- ``test_run_worker_rollback.py``:
  * ``_bump_channel_version`` — 8 tests covering every version type
    (int, float, numeric string, UUID-style string, None, bool) and
    every saver-side fault mode (no ``get_next_version`` / raising /
    stuck on identity).
  * ``test_ensure_interrupted_title_*`` — 5 additional helper
    boundary tests: title.enabled=false short-circuit; empty
    messages list; messages=None; aput-error propagation (helper
    contract: caller swallows, not the helper); idempotency on a
    real InMemorySaver across two invocations.
  * ``test_ensure_interrupted_title_preserves_non_title_channel_versions``
    — pins that ``new_versions`` only contains ``"title"`` and that
    other channels' versions are untouched (regression anchor for a
    sloppier draft that bumped every channel).
  * ``test_worker_finally_block_swallows_helper_exceptions`` — pins
    the integration contract: even if the helper raises, the worker's
    threads_meta status sync still runs and ``publish_end`` is still
    awaited so the SSE stream closes cleanly.

- ``test_title_middleware_core_logic.py``:
  * 4 additional tests: ``messages=None`` on both
    ``_should_generate_title`` and ``_build_title_prompt``; the
    ``role: user`` / ``role: assistant`` (OpenAI-style) dict
    normalization; partial-exchange path with a dict-form message.

Verification:
- ``PYTHONPATH=. uv run pytest tests/ -x --ignore=tests/blocking_io -q``
  → 5215 passed, 18 skipped.
- ``ruff check`` + ``ruff format --check`` clean on every touched file.
- Red/green TDD verification: temporarily reverted the
  ``new_versions={}`` fix → 4 new tests went red as expected; restored
  and the suite is green again. Same red/green dance for the
  ``messages=None`` coercion.

Refs #3859. Addresses second-pass review on #3874.

* fix(title): ignore dict context reminders in fallback

* fix(worker): link interrupted-title checkpoint to its parent

The title-bump checkpoint written by ``_ensure_interrupted_title`` was
landing without a ``parent_checkpoint_id`` — a real orphan in the
LangGraph history graph. Reproduction (disk-backed AsyncSqliteSaver):

  [seed] checkpoint_id = 1f173dbc...
  [helper] wrote title = "Why is the sky blue?"
  [issue 1] new checkpoint = 1f173dbc..., parent = None
  [issue 1] is new checkpoint orphaned? True

Root cause: ``_ensure_interrupted_title`` built ``write_config`` as
``{"thread_id": ..., "checkpoint_ns": ...}`` only. ``BaseCheckpointSaver``
implementations read ``configurable.checkpoint_id`` from that config as
the *parent* id when inserting (see ``langgraph/checkpoint/sqlite/aio.py``
``aput``: ``config["configurable"].get("checkpoint_id")`` becomes the
``parent_checkpoint_id`` column). With no value, the saver writes NULL —
the new checkpoint is a tree root.

Consequences:
- Any future LangGraph ``runs.resume_from`` / time-travel feature has no
  backward edge to walk past the title-bump.
- History-visualization UIs built on ``alist()`` render the title-bump as
  a sibling of the prior checkpoint, not its descendant.

Fix: read ``checkpoint_id`` off the tuple's own config and thread it into
``write_config["configurable"]["checkpoint_id"]`` before calling ``aput``,
the same pattern every middleware-driven write uses.

Three new regression tests against real ``AsyncSqliteSaver`` (disk-backed,
fresh connections so we exercise the on-disk read path):

- ``test_ensure_interrupted_title_links_new_checkpoint_to_its_parent`` —
  asserts ``latest.parent_config["configurable"]["checkpoint_id"]`` equals
  the seeded checkpoint id. TDD red-green verified: reverting the fix
  flips this test red with ``AssertionError: title-bump checkpoint must
  have a parent_config``.
- ``test_ensure_interrupted_title_appears_in_history_with_audit_marker``
  — pins the audit contract: the title-bump entry in ``alist()`` carries
  ``metadata.source == "update"`` and ``metadata.writes`` contains
  ``runtime_interrupt_title``. This is a deliberate design choice — we
  do NOT hide the entry from history (audit trail belongs in the saver),
  but its source and writes marker MUST be unambiguous so UIs/tools can
  identify it.
- ``test_ensure_interrupted_title_survives_immediate_next_turn`` —
  cancel → immediate user follow-up scenario. Simulates the agent's next
  turn appending a (user, ai) pair without touching the title channel,
  then opens a fresh saver and verifies the title is still present after
  the next-turn checkpoint write. Pins the channel-version-blob
  invariant established by commit 05253957 — without the
  ``new_versions={"title": v}`` declaration there, the title blob would
  vanish from the DB and this test would read back ``None``.

Verification:
- ``PYTHONPATH=. uv run pytest tests/ -x --ignore=tests/blocking_io -q``
  → 5222 passed, 15 skipped.
- ``ruff check`` + ``ruff format --check`` clean on every touched file.
- Reproduction script confirms ``parent_checkpoint_id`` is now non-null
  and the next-turn read-back preserves the fallback title.

Refs #3859.

* Revert "fix(worker): link interrupted-title checkpoint to its parent"

This reverts commit c763ed9334781db1acdce0f5f33d663d8d5f80ff.

* test: trim over-engineered test coverage

Reduce review surface area on PR #3874 by dropping defensive tests that
don't pin a real invariant. After self-review:

- ``_bump_channel_version``: 8 tests → 2 (happy path + saver-error
  fallback). Dropped float / bool / numeric-string / UUID-string /
  missing-get-next-version / stuck-get-next-version branches — those
  are speculative scaffolding for savers we don't ship.
- ``_ensure_interrupted_title``: dropped ``returns_none_when_title_disabled``,
  ``returns_none_with_no_user_message``, ``returns_none_when_no_checkpoint``
  — boundary guards already exercised by the e2e test and the
  ``handles_none_messages_channel`` regression anchor.

Net: -107 lines of test code. Remaining coverage still pins every
red-green-verified invariant (channel_versions bump, string-version
bump, idempotency, sqlite round-trip, non-title channel preservation,
aput-error contract, worker finally swallowing, partial-exchange).

Verification: 5209 passed, 15 skipped.

* fix: harden interrupted title finalization

* fix: serialize interrupted title finalization

* fix: preserve interrupt semantics during title finalization

* fix: preserve delayed interrupted title recovery

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
2026-07-02 00:13:01 +08:00

416 lines
13 KiB
TypeScript

import { describe, expect, rs, test } from "@rstest/core";
import {
QueryClient,
QueryObserver,
type InfiniteData,
} from "@tanstack/react-query";
import {
filterInfiniteThreadsCache,
getInfiniteThreadsNextPageParam,
INFINITE_THREADS_PAGE_SIZE,
INFINITE_THREADS_QUERY_KEY_PREFIX,
invalidateStoppedThreadCaches,
mapInfiniteThreadsCache,
STOP_THREAD_FINALIZATION_REFETCH_DELAY_MS,
stopThreadAndInvalidateCaches,
upsertThreadInInfiniteCache,
} from "@/core/threads/hooks";
import type { AgentThread } from "@/core/threads/types";
// Issue #3482: the sidebar and /workspace/chats list used to be capped at
// 50 threads because `useThreads()` exits as soon as `threads.length >=
// params.limit`. These pure helpers back the `useInfiniteThreads()`
// pagination logic and the mirrored cache writes that keep rename / delete
// / stream-finish in sync with both the legacy array cache and the new
// infinite cache.
function makeThread(id: string, title = `Title ${id}`): AgentThread {
return {
thread_id: id,
created_at: "2025-01-01T00:00:00Z",
updated_at: "2025-01-01T00:00:00Z",
metadata: {},
status: "idle",
values: { title },
} as unknown as AgentThread;
}
function makePage(start: number, size: number): AgentThread[] {
return Array.from({ length: size }, (_, i) => makeThread(`t-${start + i}`));
}
function makeInfiniteData(pages: AgentThread[][]): InfiniteData<AgentThread[]> {
return {
pages,
pageParams: pages.map((_, i) => i * INFINITE_THREADS_PAGE_SIZE),
};
}
describe("getInfiniteThreadsNextPageParam", () => {
test("returns next offset when the last page is full", () => {
const page1 = makePage(0, INFINITE_THREADS_PAGE_SIZE);
expect(getInfiniteThreadsNextPageParam(page1, [page1])).toBe(
INFINITE_THREADS_PAGE_SIZE,
);
});
test("returns next offset across multiple full pages", () => {
const page1 = makePage(0, INFINITE_THREADS_PAGE_SIZE);
const page2 = makePage(
INFINITE_THREADS_PAGE_SIZE,
INFINITE_THREADS_PAGE_SIZE,
);
expect(getInfiniteThreadsNextPageParam(page2, [page1, page2])).toBe(
INFINITE_THREADS_PAGE_SIZE * 2,
);
});
test("returns undefined when the last page is short (end of list)", () => {
const page1 = makePage(0, INFINITE_THREADS_PAGE_SIZE);
const page2 = makePage(INFINITE_THREADS_PAGE_SIZE, 10);
expect(
getInfiniteThreadsNextPageParam(page2, [page1, page2]),
).toBeUndefined();
});
test("returns undefined when the last page is empty", () => {
const page1 = makePage(0, INFINITE_THREADS_PAGE_SIZE);
expect(getInfiniteThreadsNextPageParam([], [page1, []])).toBeUndefined();
});
test("respects a custom page size", () => {
const page1 = makePage(0, 5);
expect(getInfiniteThreadsNextPageParam(page1, [page1], 5)).toBe(5);
expect(getInfiniteThreadsNextPageParam(page1, [page1], 10)).toBeUndefined();
});
});
describe("mapInfiniteThreadsCache", () => {
test("returns undefined when oldData is undefined", () => {
expect(mapInfiniteThreadsCache(undefined, (t) => t)).toBeUndefined();
});
test("updates the matching thread across multiple pages", () => {
const page1 = [makeThread("a"), makeThread("b")];
const page2 = [makeThread("c"), makeThread("d")];
const data = makeInfiniteData([page1, page2]);
const updated = mapInfiniteThreadsCache(data, (t) =>
t.thread_id === "c"
? { ...t, values: { ...t.values, title: "renamed" } }
: t,
);
expect(updated?.pages[0]?.[0]?.values?.title).toBe("Title a");
expect(updated?.pages[1]?.[0]?.thread_id).toBe("c");
expect(updated?.pages[1]?.[0]?.values?.title).toBe("renamed");
expect(updated?.pages[1]?.[1]?.values?.title).toBe("Title d");
});
test("preserves pageParams", () => {
const data = makeInfiniteData([[makeThread("a")]]);
const updated = mapInfiniteThreadsCache(data, (t) => t);
expect(updated?.pageParams).toEqual(data.pageParams);
});
});
describe("filterInfiniteThreadsCache", () => {
test("returns undefined when oldData is undefined", () => {
expect(filterInfiniteThreadsCache(undefined, () => true)).toBeUndefined();
});
test("removes matching threads across all pages", () => {
const page1 = [makeThread("a"), makeThread("b")];
const page2 = [makeThread("b"), makeThread("c")];
const data = makeInfiniteData([page1, page2]);
const filtered = filterInfiniteThreadsCache(
data,
(t) => t.thread_id !== "b",
);
expect(filtered?.pages[0]?.map((t) => t.thread_id)).toEqual(["a"]);
expect(filtered?.pages[1]?.map((t) => t.thread_id)).toEqual(["c"]);
});
test("keeps an emptied page as an empty array (does not drop the page)", () => {
const page1 = [makeThread("a")];
const page2 = [makeThread("b")];
const data = makeInfiniteData([page1, page2]);
const filtered = filterInfiniteThreadsCache(
data,
(t) => t.thread_id !== "a",
);
expect(filtered?.pages).toHaveLength(2);
expect(filtered?.pages[0]).toEqual([]);
expect(filtered?.pages[1]?.[0]?.thread_id).toBe("b");
});
test("does not regress next offset when an earlier page has been shrunk by a delete", () => {
// Simulate two full pages already loaded.
const page1 = Array.from({ length: 50 }, (_, i) => ({
thread_id: `a${i}`,
}));
const page2 = Array.from({ length: 50 }, (_, i) => ({
thread_id: `b${i}`,
}));
// Offset right after fetching page 2 (this is the value TanStack Query
// freezes into pageParams).
const offsetAfterPage2 = getInfiniteThreadsNextPageParam(
page2 as unknown as AgentThread[],
[page1, page2] as unknown as AgentThread[][],
);
expect(offsetAfterPage2).toBe(100);
// Now a delete mutation runs filterInfiniteThreadsCache and shrinks
// page 1 from 50 to 49 entries. TanStack does NOT re-invoke
// getNextPageParam on cache mutations; the previously-computed offset
// (100) remains the param for the next fetchNextPage() call, so the
// helper is consistent with how the library uses its return value.
const shrunkPage1 = page1.slice(0, 49);
const recomputed = getInfiniteThreadsNextPageParam(
page2 as unknown as AgentThread[],
[shrunkPage1, page2] as unknown as AgentThread[][],
);
// We document the recomputed value for completeness, but in practice
// useDeleteThread invalidates the query in onSettled, so pages are
// refetched from offset 0 rather than relying on this number.
expect(recomputed).toBe(99);
});
});
describe("upsertThreadInInfiniteCache", () => {
function seedClient(initial?: InfiniteData<AgentThread[]>): QueryClient {
const client = new QueryClient();
if (initial) {
client.setQueryData([...INFINITE_THREADS_QUERY_KEY_PREFIX, {}], initial);
}
return client;
}
function readCache(
client: QueryClient,
): InfiniteData<AgentThread[]> | undefined {
return client.getQueryData([...INFINITE_THREADS_QUERY_KEY_PREFIX, {}]);
}
test("no-op when the infinite cache has not been initialised yet", () => {
const client = seedClient();
upsertThreadInInfiniteCache(client, makeThread("new"));
expect(readCache(client)).toBeUndefined();
});
test("prepends a brand-new thread to the first page", () => {
const client = seedClient({
pages: [[makeThread("a"), makeThread("b")]],
pageParams: [0],
});
upsertThreadInInfiniteCache(client, makeThread("new"));
const cache = readCache(client);
expect(cache?.pages[0]?.map((t) => t.thread_id)).toEqual(["new", "a", "b"]);
});
test("merges into the existing entry instead of duplicating it", () => {
const existing = makeThread("a", "Old title");
const client = seedClient({
pages: [[existing, makeThread("b")]],
pageParams: [0],
});
// Simulate an onCreated upsert that races with a thread already in cache:
// the cache copy should win for title/metadata (it represents later state),
// but no duplicate row should appear.
upsertThreadInInfiniteCache(client, {
...makeThread("a", "New title"),
status: "busy",
});
const cache = readCache(client);
const ids = cache?.pages[0]?.map((t) => t.thread_id);
expect(ids).toEqual(["a", "b"]);
expect(cache?.pages[0]?.[0]?.values.title).toBe("Old title");
});
});
describe("invalidateStoppedThreadCaches", () => {
function invalidatedQueryKeys(client: QueryClient) {
const invalidate = rs.spyOn(client, "invalidateQueries");
return {
invalidate,
queryKeys: () =>
invalidate.mock.calls.map(([filters]) => filters?.queryKey),
};
}
test("refreshes current thread and sidebar caches after fire-and-forget stop", () => {
const client = new QueryClient();
const { queryKeys } = invalidatedQueryKeys(client);
invalidateStoppedThreadCaches(client, "thread-1", false);
expect(queryKeys()).toContainEqual(["threads", "search"]);
expect(queryKeys()).toContainEqual(INFINITE_THREADS_QUERY_KEY_PREFIX);
expect(queryKeys()).toContainEqual(["thread", "thread-1"]);
expect(queryKeys()).toContainEqual([
"thread",
"metadata",
"thread-1",
false,
]);
expect(queryKeys()).toContainEqual(["thread-token-usage", "thread-1"]);
});
test("does not refresh per-thread API caches for mock threads", () => {
const client = new QueryClient();
const { queryKeys } = invalidatedQueryKeys(client);
invalidateStoppedThreadCaches(client, "thread-1", true);
expect(queryKeys()).toContainEqual(["threads", "search"]);
expect(queryKeys()).toContainEqual(INFINITE_THREADS_QUERY_KEY_PREFIX);
expect(queryKeys()).not.toContainEqual(["thread", "thread-1"]);
expect(queryKeys()).not.toContainEqual([
"thread",
"metadata",
"thread-1",
true,
]);
expect(queryKeys()).not.toContainEqual(["thread-token-usage", "thread-1"]);
});
test("wraps SDK stop and refreshes caches after it resolves", async () => {
const client = new QueryClient();
const stop = rs.fn(() => Promise.resolve());
const { queryKeys } = invalidatedQueryKeys(client);
await stopThreadAndInvalidateCaches(client, stop, "thread-1", false);
expect(stop).toHaveBeenCalledTimes(1);
expect(queryKeys()).toContainEqual([
"thread",
"metadata",
"thread-1",
false,
]);
});
test("still refreshes caches when SDK stop rejects", async () => {
const client = new QueryClient();
const stop = rs.fn(async () => {
throw new Error("cancel failed");
});
const { queryKeys } = invalidatedQueryKeys(client);
await expect(
stopThreadAndInvalidateCaches(client, stop, "thread-1", false),
).rejects.toThrow("cancel failed");
expect(queryKeys()).toContainEqual(["threads", "search"]);
expect(queryKeys()).toContainEqual([
"thread",
"metadata",
"thread-1",
false,
]);
});
test("schedules sidebar refetch even if stopped thread id is not known", async () => {
rs.useFakeTimers();
const client = new QueryClient();
const { queryKeys } = invalidatedQueryKeys(client);
try {
await stopThreadAndInvalidateCaches(
client,
() => Promise.resolve(),
null,
false,
);
const countSearchInvalidations = () =>
queryKeys().filter(
(queryKey) =>
queryKey?.length === 2 &&
queryKey[0] === "threads" &&
queryKey[1] === "search",
).length;
expect(countSearchInvalidations()).toBe(1);
await rs.advanceTimersByTimeAsync(
STOP_THREAD_FINALIZATION_REFETCH_DELAY_MS,
);
expect(countSearchInvalidations()).toBe(2);
expect(queryKeys()).not.toContainEqual(["thread", null]);
} finally {
client.clear();
rs.useRealTimers();
}
});
test("scheduled refetch lets sidebar receive delayed backend title finalization", async () => {
rs.useFakeTimers();
const client = new QueryClient({
defaultOptions: { queries: { retry: false } },
});
let finalized = false;
let fetchCount = 0;
const observer = new QueryObserver<AgentThread[]>(client, {
queryKey: ["threads", "search"],
queryFn: async () => {
fetchCount += 1;
return [
makeThread(
"thread-1",
finalized ? "Generated Title" : "New Conversation",
),
];
},
});
const unsubscribe = observer.subscribe((result) => {
void result.status;
});
try {
await observer.refetch();
expect(
client.getQueryData<AgentThread[]>(["threads", "search"])?.[0]?.values
?.title,
).toBe("New Conversation");
await stopThreadAndInvalidateCaches(
client,
() => Promise.resolve(),
"thread-1",
false,
);
await Promise.resolve();
expect(
client.getQueryData<AgentThread[]>(["threads", "search"])?.[0]?.values
?.title,
).toBe("New Conversation");
finalized = true;
await rs.advanceTimersByTimeAsync(
STOP_THREAD_FINALIZATION_REFETCH_DELAY_MS,
);
expect(
client.getQueryData<AgentThread[]>(["threads", "search"])?.[0]?.values
?.title,
).toBe("Generated Title");
expect(fetchCount).toBeGreaterThanOrEqual(3);
} finally {
unsubscribe();
client.clear();
rs.useRealTimers();
}
});
});