fix(ai-chat): plural token strings + cover reasoning UI + cleanups (#151 review)
Review of #158 (Request changes) — core logic verified correct; addressed the test-coverage + localization items: 1. i18n pluralization: the token-count keys were called with {count} but had one form, so ru-RU always rendered the genitive ("1 токенов"). Added _one/_other (en) and _one/_few/_many (ru: токен/токена/токенов) for both "Thinking… · {{count}} tokens" and "Thinking · {{count}} tokens"; de-duped the PR-added duplicate "Thinking" key. Call sites unchanged. 2. ReasoningBlock: new reasoning-block.test.tsx (4 branches: authoritative count wins / estimate fallback / header-only when count-but-no-text / body render). 3. Reasoning-token attribution: extracted the #151 anti-double-count rule into a pure `reasoningTokensForPart(message)` (single reasoning part -> authoritative turn total; multiple/none -> undefined so each estimates). message-item uses it; removed the now-dead lastReasoningIndex reduce (review #5). Unit-tested. 6. adopt-chat-id.ts: refreshed 3 stale `chatStreamStartMetadata` -> `chatStreamMetadata` comment references. 7. chat-markdown.test.ts: assert the export footer's `reasoning: N` line appears when reasoningTokens>0 and is absent at 0/undefined. Skipped optional #4 (mantine useThrottledCallback): the manual throttle has two distinct exit paths (turn-end revert-to-null + the captured-total trailing emit) with no guarding test; remapping risks the streaming behavior — non-blocking. Client tsc clean; ai-chat suite green (171 tests). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -1148,8 +1148,11 @@
|
||||
"Ask a question…": "Ask a question…",
|
||||
"Thinking…": "Thinking…",
|
||||
"Thinking… · {{count}} tokens": "Thinking… · {{count}} tokens",
|
||||
"Thinking": "Thinking",
|
||||
"Thinking… · {{count}} tokens_one": "Thinking… · {{count}} token",
|
||||
"Thinking… · {{count}} tokens_other": "Thinking… · {{count}} tokens",
|
||||
"Thinking · {{count}} tokens": "Thinking · {{count}} tokens",
|
||||
"Thinking · {{count}} tokens_one": "Thinking · {{count}} token",
|
||||
"Thinking · {{count}} tokens_other": "Thinking · {{count}} tokens",
|
||||
"The assistant is unavailable right now. Please try again.": "The assistant is unavailable right now. Please try again.",
|
||||
"Public share assistant": "Public share assistant",
|
||||
"Let anonymous visitors of public shares ask an AI assistant scoped to that share's pages. You pay for the tokens.": "Let anonymous visitors of public shares ask an AI assistant scoped to that share's pages. You pay for the tokens.",
|
||||
|
||||
@@ -681,8 +681,13 @@
|
||||
"{{name}} is typing…": "{{name}} печатает…",
|
||||
"Thinking…": "Думаю…",
|
||||
"Thinking… · {{count}} tokens": "Думаю… · {{count}} токенов",
|
||||
"Thinking": "Размышления",
|
||||
"Thinking… · {{count}} tokens_one": "Думаю… · {{count}} токен",
|
||||
"Thinking… · {{count}} tokens_few": "Думаю… · {{count}} токена",
|
||||
"Thinking… · {{count}} tokens_many": "Думаю… · {{count}} токенов",
|
||||
"Thinking · {{count}} tokens": "Размышления · {{count}} токенов",
|
||||
"Thinking · {{count}} tokens_one": "Размышления · {{count}} токен",
|
||||
"Thinking · {{count}} tokens_few": "Размышления · {{count}} токена",
|
||||
"Thinking · {{count}} tokens_many": "Размышления · {{count}} токенов",
|
||||
"Agent role": "Роль агента",
|
||||
"AI chat": "AI-чат",
|
||||
"AI chat is disabled for this workspace.": "AI-чат отключён для этого рабочего пространства.",
|
||||
|
||||
@@ -9,6 +9,7 @@ import { ToolUiPart, isToolPart } from "@/features/ai-chat/utils/tool-parts.tsx"
|
||||
import { assistantMessageHasVisibleContent } from "@/features/ai-chat/utils/message-content.ts";
|
||||
import { renderChatMarkdown } from "@/features/ai-chat/utils/markdown.ts";
|
||||
import { resolveAssistantName } from "@/features/ai-chat/utils/assistant-name.ts";
|
||||
import { reasoningTokensForPart } from "@/features/ai-chat/utils/reasoning-tokens.ts";
|
||||
import { describeChatError } from "@/features/ai-chat/utils/error-message.ts";
|
||||
import classes from "@/features/ai-chat/components/ai-chat.module.css";
|
||||
|
||||
@@ -78,25 +79,12 @@ export default function MessageItem({
|
||||
// return won't fire for them.
|
||||
if (!assistantMessageHasVisibleContent(message)) return null;
|
||||
|
||||
// Authoritative reasoning token count for the turn, if the server attached it
|
||||
// (incl. providers that report a reasoning COUNT without streaming the text).
|
||||
// It is the TURN TOTAL, so it may only be attributed to a block when there is a
|
||||
// SINGLE reasoning part (the common one-step turn) — then that block shows the
|
||||
// exact figure. With multiple reasoning parts (multi-step agent turn) every
|
||||
// block falls back to its own per-part estimate; attributing the turn total to
|
||||
// one of them would double-count against the others' estimates (#151 review).
|
||||
// The authoritative turn total is still surfaced live in the header badge.
|
||||
const reasoningTokens = (
|
||||
message.metadata as { usage?: { reasoningTokens?: number } } | undefined
|
||||
)?.usage?.reasoningTokens;
|
||||
const reasoningPartCount = message.parts.reduce(
|
||||
(acc, p) => (p.type === "reasoning" ? acc + 1 : acc),
|
||||
0,
|
||||
);
|
||||
const lastReasoningIndex = message.parts.reduce(
|
||||
(acc, p, i) => (p.type === "reasoning" ? i : acc),
|
||||
-1,
|
||||
);
|
||||
// Authoritative reasoning token count to attribute to a reasoning block, or
|
||||
// undefined when the block must estimate on its own. See reasoningTokensForPart
|
||||
// for the #151 anti-double-count rule (only a single reasoning part may carry
|
||||
// the turn total). The authoritative turn total is still surfaced live in the
|
||||
// header badge regardless.
|
||||
const reasoningTokens = reasoningTokensForPart(message);
|
||||
|
||||
return (
|
||||
<Box className={classes.messageRow}>
|
||||
@@ -109,12 +97,11 @@ export default function MessageItem({
|
||||
// count. Empty/whitespace reasoning with no authoritative count carries
|
||||
// nothing to show, so skip it (avoids an empty 0-token block).
|
||||
const text = (part as { text?: string }).text ?? "";
|
||||
const tokens =
|
||||
reasoningPartCount === 1 && index === lastReasoningIndex
|
||||
? reasoningTokens
|
||||
: undefined;
|
||||
if (!text.trim() && !(tokens && tokens > 0)) return null;
|
||||
return <ReasoningBlock key={index} text={text} tokens={tokens} />;
|
||||
if (!text.trim() && !(reasoningTokens && reasoningTokens > 0))
|
||||
return null;
|
||||
return (
|
||||
<ReasoningBlock key={index} text={text} tokens={reasoningTokens} />
|
||||
);
|
||||
}
|
||||
|
||||
if (part.type === "text") {
|
||||
|
||||
@@ -0,0 +1,65 @@
|
||||
import { describe, it, expect, vi } from "vitest";
|
||||
import { render, screen } from "@testing-library/react";
|
||||
import { MantineProvider } from "@mantine/core";
|
||||
|
||||
// Stub react-i18next so `t` returns the key with `{{count}}` interpolated. This
|
||||
// keeps the assertions on the component's OWN count logic (authoritative vs
|
||||
// estimate) rather than on translation, and mirrors the t-mock pattern used by
|
||||
// other component tests in the repo.
|
||||
vi.mock("react-i18next", () => ({
|
||||
useTranslation: () => ({
|
||||
t: (key: string, opts?: { count?: number }) =>
|
||||
opts && typeof opts.count === "number"
|
||||
? key.replace("{{count}}", String(opts.count))
|
||||
: key,
|
||||
}),
|
||||
}));
|
||||
|
||||
import ReasoningBlock from "./reasoning-block";
|
||||
import { estimateTokens } from "@/features/ai-chat/utils/count-stream-tokens.ts";
|
||||
|
||||
// matchMedia (read by MantineProvider) is stubbed globally in vitest.setup.ts.
|
||||
|
||||
function renderBlock(props: { text: string; tokens?: number }) {
|
||||
return render(
|
||||
<MantineProvider>
|
||||
<ReasoningBlock {...props} />
|
||||
</MantineProvider>,
|
||||
);
|
||||
}
|
||||
|
||||
describe("ReasoningBlock", () => {
|
||||
it("shows the authoritative count in the header when tokens > 0", () => {
|
||||
// Text "thinking…" estimates to ceil(9/4) = 3, but the authoritative 42
|
||||
// must win, so the header shows 42 (and NOT the 3-token estimate).
|
||||
renderBlock({ text: "thinking…", tokens: 42 });
|
||||
expect(screen.getByText("Thinking · 42 tokens")).toBeDefined();
|
||||
expect(screen.queryByText("Thinking · 3 tokens")).toBeNull();
|
||||
});
|
||||
|
||||
it("falls back to the text-length estimate when no authoritative tokens", () => {
|
||||
const text = "some reasoning prose that streams in";
|
||||
const estimate = estimateTokens(text);
|
||||
renderBlock({ text });
|
||||
expect(estimate).toBeGreaterThan(0);
|
||||
expect(screen.getByText(new RegExp(`${estimate} tokens`))).toBeDefined();
|
||||
});
|
||||
|
||||
it("header-only when text is empty but an authoritative count is present", () => {
|
||||
renderBlock({ text: "", tokens: 17 });
|
||||
expect(screen.getByText(/17 tokens/)).toBeDefined();
|
||||
// No disclosure body to expand: the toggle button is disabled.
|
||||
const button = screen.getByRole("button");
|
||||
expect((button as HTMLButtonElement).disabled).toBe(true);
|
||||
});
|
||||
|
||||
it("renders the reasoning body (markdown or raw-text fallback)", () => {
|
||||
renderBlock({ text: "**bold** reasoning", tokens: 5 });
|
||||
// The toggle is enabled because there IS body text to expand.
|
||||
const button = screen.getByRole("button");
|
||||
expect((button as HTMLButtonElement).disabled).toBe(false);
|
||||
// The body prose renders (markdown -> sanitized html, or raw-text fallback);
|
||||
// either way the text is present in the document.
|
||||
expect(screen.getByText(/reasoning/)).toBeDefined();
|
||||
});
|
||||
});
|
||||
@@ -4,7 +4,7 @@
|
||||
* ============================ CANONICAL #137 NOTE ============================
|
||||
* This docblock is the single authoritative explanation of the new-chat id
|
||||
* adoption design and the #137 two-tab race it fixes. Other call sites
|
||||
* (use-chat-session.ts, the server's `chatStreamStartMetadata`) reference here
|
||||
* (use-chat-session.ts, the server's `chatStreamMetadata`) reference here
|
||||
* rather than restating it.
|
||||
*
|
||||
* When a user sends the first turn of a BRAND-NEW chat, the client has no chat
|
||||
@@ -17,7 +17,7 @@
|
||||
* leak its later turns into it (#137). We adopt by IDENTITY instead, two ways:
|
||||
*
|
||||
* PRIMARY path: the server streams the real chat id on the assistant message
|
||||
* metadata's `start` part (see `chatStreamStartMetadata` server-side);
|
||||
* metadata's `start` part (see `chatStreamMetadata` server-side);
|
||||
* `extractServerChatId` reads it off the finished message and
|
||||
* `resolveAdoptedChatId` turns it into the id to adopt for a new chat. This is
|
||||
* authoritative and immune to the race.
|
||||
@@ -46,7 +46,7 @@ export function resolveAdoptedChatId(
|
||||
/**
|
||||
* Read the authoritative server chat id off a finished assistant message. The
|
||||
* server attaches it as `message.metadata.chatId` on the `start` part (see
|
||||
* `chatStreamStartMetadata`). Returns it only when it is a string; undefined for
|
||||
* `chatStreamMetadata`). Returns it only when it is a string; undefined for
|
||||
* a missing message, missing metadata, or a non-string `chatId`.
|
||||
*/
|
||||
export function extractServerChatId(
|
||||
|
||||
@@ -314,6 +314,57 @@ describe("buildChatMarkdown — token totals", () => {
|
||||
});
|
||||
expect(md).toContain("- Total tokens: 99");
|
||||
});
|
||||
|
||||
it("appends the reasoning figure to the row footer when reasoningTokens > 0", () => {
|
||||
const md = buildChatMarkdown({
|
||||
title: "t",
|
||||
chatId: "c",
|
||||
rows: [
|
||||
row({
|
||||
role: "assistant",
|
||||
content: "x",
|
||||
metadata: {
|
||||
usage: { inputTokens: 10, outputTokens: 8, reasoningTokens: 3 },
|
||||
},
|
||||
}),
|
||||
],
|
||||
t,
|
||||
});
|
||||
expect(md).toContain("_Tokens — in: 10, out: 8, reasoning: 3, total: 18_");
|
||||
});
|
||||
|
||||
it("omits the reasoning figure when reasoningTokens is 0 / absent", () => {
|
||||
const zero = buildChatMarkdown({
|
||||
title: "t",
|
||||
chatId: "c",
|
||||
rows: [
|
||||
row({
|
||||
role: "assistant",
|
||||
content: "x",
|
||||
metadata: {
|
||||
usage: { inputTokens: 10, outputTokens: 5, reasoningTokens: 0 },
|
||||
},
|
||||
}),
|
||||
],
|
||||
t,
|
||||
});
|
||||
expect(zero).toContain("_Tokens — in: 10, out: 5, total: 15_");
|
||||
expect(zero).not.toContain("reasoning:");
|
||||
|
||||
const absent = buildChatMarkdown({
|
||||
title: "t",
|
||||
chatId: "c",
|
||||
rows: [
|
||||
row({
|
||||
role: "assistant",
|
||||
content: "x",
|
||||
metadata: { usage: { inputTokens: 10, outputTokens: 5 } },
|
||||
}),
|
||||
],
|
||||
t,
|
||||
});
|
||||
expect(absent).not.toContain("reasoning:");
|
||||
});
|
||||
});
|
||||
|
||||
describe("buildChatMarkdown — pending / in-progress messages", () => {
|
||||
|
||||
@@ -0,0 +1,56 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import type { UIMessage } from "@ai-sdk/react";
|
||||
import { reasoningTokensForPart } from "@/features/ai-chat/utils/reasoning-tokens.ts";
|
||||
|
||||
/**
|
||||
* Pure-helper tests for `reasoningTokensForPart`, the #151 anti-double-count
|
||||
* rule: the authoritative `usage.reasoningTokens` is the TURN TOTAL, so it may
|
||||
* only be attributed when the turn has exactly one reasoning part. With multiple
|
||||
* reasoning parts (or no authoritative usage) every part falls back to its own
|
||||
* per-part estimate, signalled here by `undefined`.
|
||||
*/
|
||||
const msg = (
|
||||
parts: UIMessage["parts"],
|
||||
metadata?: unknown,
|
||||
): UIMessage =>
|
||||
({
|
||||
id: Math.random().toString(),
|
||||
role: "assistant",
|
||||
parts,
|
||||
metadata,
|
||||
}) as UIMessage;
|
||||
|
||||
describe("reasoningTokensForPart", () => {
|
||||
it("single reasoning part -> the authoritative turn total", () => {
|
||||
const m = msg(
|
||||
[
|
||||
{ type: "reasoning", text: "thinking…" } as never,
|
||||
{ type: "text", text: "answer" },
|
||||
],
|
||||
{ usage: { reasoningTokens: 42 } },
|
||||
);
|
||||
expect(reasoningTokensForPart(m)).toBe(42);
|
||||
});
|
||||
|
||||
it("multiple reasoning parts -> undefined (each estimates on its own)", () => {
|
||||
const m = msg(
|
||||
[
|
||||
{ type: "reasoning", text: "step one" } as never,
|
||||
{ type: "reasoning", text: "step two" } as never,
|
||||
{ type: "text", text: "answer" },
|
||||
],
|
||||
{ usage: { reasoningTokens: 99 } },
|
||||
);
|
||||
// Even with an authoritative total, two reasoning parts must each estimate
|
||||
// (attributing the total to one would double-count against the other).
|
||||
expect(reasoningTokensForPart(m)).toBeUndefined();
|
||||
});
|
||||
|
||||
it("no authoritative usage -> undefined even for a single reasoning part", () => {
|
||||
const m = msg([
|
||||
{ type: "reasoning", text: "thinking…" } as never,
|
||||
{ type: "text", text: "answer" },
|
||||
]);
|
||||
expect(reasoningTokensForPart(m)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
34
apps/client/src/features/ai-chat/utils/reasoning-tokens.ts
Normal file
34
apps/client/src/features/ai-chat/utils/reasoning-tokens.ts
Normal file
@@ -0,0 +1,34 @@
|
||||
import type { UIMessage } from "@ai-sdk/react";
|
||||
|
||||
/**
|
||||
* Decide the authoritative reasoning token count to attribute to a single
|
||||
* `reasoning` part of an assistant message — or `undefined` when the part should
|
||||
* fall back to its own per-part estimate.
|
||||
*
|
||||
* `usage.reasoningTokens` is the TURN TOTAL, so it may only be attributed to a
|
||||
* block when the turn has exactly ONE reasoning part (the common one-step turn):
|
||||
* then that block can show the exact figure. With MULTIPLE reasoning parts (a
|
||||
* multi-step agent turn) every block must fall back to its own estimate —
|
||||
* attributing the turn total to one of them would double-count against the
|
||||
* others' estimates (#151 review anti-double-count rule). When there is no
|
||||
* authoritative usage at all, every part estimates.
|
||||
*
|
||||
* Returns the authoritative `reasoningTokens` only for the single-reasoning-part
|
||||
* case; `undefined` otherwise (the caller estimates from the part text).
|
||||
*/
|
||||
export function reasoningTokensForPart(
|
||||
message: UIMessage,
|
||||
): number | undefined {
|
||||
const reasoningTokens = (
|
||||
message.metadata as { usage?: { reasoningTokens?: number } } | undefined
|
||||
)?.usage?.reasoningTokens;
|
||||
|
||||
const reasoningPartCount = (message.parts ?? []).reduce(
|
||||
(acc, p) => (p.type === "reasoning" ? acc + 1 : acc),
|
||||
0,
|
||||
);
|
||||
|
||||
// Exactly one reasoning part -> attribute the authoritative turn total to it.
|
||||
// Otherwise (zero or multiple) each part estimates on its own.
|
||||
return reasoningPartCount === 1 ? reasoningTokens : undefined;
|
||||
}
|
||||
Reference in New Issue
Block a user