Diagnose setup by what is certain, and deliver errors the way the client asked

Review of the branch found the persona diagnosis was mostly truncation
wearing a name tag. The <.> marker sits at the start of the system
message and the persona marker after the card, so a card Janitor cut
short arrives with the prompt present and the persona gone — and was
told its persona was wrong, with truncation as an afterthought. The
original message was only ever about context size, which is the best
evidence there is about what actually happens.

diagnoseSetup() now says only what the markers make certain. A missing
<.> is a missing prompt, since truncation cannot remove the first three
characters. A persona marker that is present but not "." is a wrong
description, quoted back so there is nothing to guess. A persona marker
that is gone, or cut off mid-tag, leads with truncation and mentions the
persona second.

The error path decided "streaming?" from the Accept header while the
success path used body.stream, so a client that sent stream:true without
that header got JSON back for every error and never displayed it. The
flag is hoisted above the try; the header is only a fallback for when
the body could not be read.

Token search is restricted to after the greeting, in order. A card built
from a sucker-proxied chat could carry an expired token in its text, and
a lookup on it would fail forever. Conversation order also means the
token a chat was started with always wins over one pasted later. The
system fallback now takes a role-less first message only; a request with
roles but no system turn is reported as such rather than misdiagnosed as
a missing prompt. The echo feature reads the parser's lastUser instead of
its own case-sensitive lookup. "Nothing was saved" is dropped from the
unknown-error message, since it is not guaranteed.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NX2SsmUqQftWTZAY4BDuFa
This commit is contained in:
Severian
2026-09-11 06:46:52 +08:00
parent a8b3669c17
commit 7569fb8e82
5 changed files with 201 additions and 84 deletions
+16 -17
View File
@@ -9,7 +9,7 @@ import {
NO_CARD,
NO_GREETING,
TOO_FEW_MESSAGES,
missingSetup,
diagnoseSetup,
userFacingMessage,
} from "@/lib/errors";
@@ -165,12 +165,11 @@ function extractCardData(messages: Message[]): ExtractedCard {
`Name extraction: user="${nameFromUser}", inferred="${inferredName}", tracking="${cleanTrackingName}", display="${displayName}"`
);
// Each marker is one of the two REQUIRED steps in the instructions, so the
// error can name the one that did not arrive.
const hasPrompt = content.includes("<.>");
const hasPersona = content.includes("<UserPersona>.</UserPersona>");
if (!hasPrompt || !hasPersona) {
throw missingSetup(hasPrompt, hasPersona);
// The two markers are the two REQUIRED setup steps; the diagnosis names
// the one that is wrong, or says the card was cut short.
const setupProblem = diagnoseSetup(content);
if (setupProblem) {
throw setupProblem;
}
content = content.replace("<.>", "");
@@ -337,9 +336,12 @@ export async function POST(request: NextRequest) {
});
}
// Decided from the body, and read again in the catch below: an error must
// go back the same way a success would, or the client never shows it.
let isStreamingRequest = false;
try {
const body = await request.json();
const isStreamingRequest = body.stream === true;
isStreamingRequest = body.stream === true;
if (!body.messages || body.messages.length < 2) {
if (isStreamingRequest) {
@@ -516,12 +518,7 @@ export async function POST(request: NextRequest) {
// Echo back anything the user sent after an "ECHO:" marker in their
// latest message, appended after the normal status message
const lastUserMessage =
[...body.messages]
.reverse()
.find((m: { role?: string }) => m?.role === "user") ??
body.messages[body.messages.length - 1];
const echoText = parseEchoText(lastUserMessage?.content);
const echoText = parseEchoText(selectMessageSlots(body.messages).lastUser);
if (echoText) {
responseMessage += `\n\n${echoText}`;
}
@@ -568,11 +565,13 @@ export async function POST(request: NextRequest) {
const errorMessage = userFacingMessage(error);
// Check if this was a streaming request
// The body already said whether the client wanted a stream; the Accept
// header is only a fallback for when the body could not be read at all.
const acceptHeader = request.headers.get("accept");
const isStreamingRequest = acceptHeader?.includes("text/event-stream");
const streaming =
isStreamingRequest || (acceptHeader?.includes("text/event-stream") ?? false);
if (isStreamingRequest) {
if (streaming) {
return createSSEErrorResponse(errorMessage);
}
+77 -29
View File
@@ -9,8 +9,8 @@
* The split that matters is not explained-vs-unexplained, it is
* **fixable-vs-not**:
*
* - A truncated card is the one failure a user can act on: raise the
* context size. That message is shown verbatim.
* - A setup step or a truncated card is something a user can act on;
* those messages are shown verbatim, and name the step.
* - Everything else — a request with no card in it, no greeting, nothing
* at all — cannot happen through Janitor, and if it does the user has no
* way to cause or fix it. Telling them to "open the chat properly" would
@@ -66,42 +66,90 @@ export function userFacingMessage(error: unknown): string {
if (isSuckerError(error)) return error.message;
const detail = isImpossibleRequest(error) ? ` (${error.detail})` : "";
return (
"Something went wrong that sucker has no explanation for, and nothing was " +
`saved. This one is not your fault and there is nothing for you to fix — ` +
"Something went wrong that sucker has no explanation for. This one is " +
"not your fault and there is nothing for you to fix — " +
`please ${CONTACT} and say what you were pulling, because unreported ones ` +
`do not get fixed.${detail}`
);
}
/**
* The two markers sucker reads map one-to-one onto the two steps the
* instructions mark REQUIRED, so a missing one names the setting that is not
* coming through rather than sending people back to re-read everything.
*
* Truncation produces the same symptom — Janitor cuts a card bigger than the
* context size and takes the markers with it — so it is named too, second,
* because it only applies once the settings really are right.
*/
export function missingSetup(hasPrompt: boolean, hasPersona: boolean): SuckerError {
const PROMPT = "your custom prompt is not coming through as <.> — set it in your API settings";
const PERSONA =
"your persona is not coming through — it needs the name {{user}} and a description of just a dot";
const PERSONA_OPEN = "<UserPersona>";
const PERSONA_CLOSE = "</UserPersona>";
let what: string;
if (!hasPrompt && !hasPersona) {
what =
"neither of the two required settings came through: the custom prompt " +
"has to be <.>, and your persona needs the name {{user}} with a " +
"description of just a dot";
} else {
what = hasPrompt ? PERSONA : PROMPT;
/** What the persona marker says, or null if it is absent or cut off mid-tag. */
function personaDescription(content: string): string | null {
const open = content.indexOf(PERSONA_OPEN);
if (open === -1) return null;
const close = content.indexOf(PERSONA_CLOSE, open + PERSONA_OPEN.length);
if (close === -1) return null;
return content.slice(open + PERSONA_OPEN.length, close);
}
/** A short, single-line quote of what the user actually set. */
function quoted(text: string): string {
if (text.trim() === "") return "empty";
const flat = text.replace(/\s+/g, " ").trim();
return `"${flat.length > 40 ? flat.slice(0, 40) + "…" : flat}"`;
}
/**
* Reads the two markers the two REQUIRED setup steps produce and says which
* step is wrong — or, when a marker is simply gone, that the card was
* probably cut short. Returns null when both are as they should be.
*
* `<.>` is the custom prompt and sits at the very start of the system
* message, so if it is missing the prompt is not set: truncation cannot
* remove the first three characters. `<UserPersona>…</UserPersona>` is the
* persona description and sits after the card, so:
*
* - present with something other than "." inside → the description is
* wrong. Certain, and the message quotes what it currently says.
* - absent, or cut off mid-tag → either the persona is not set or Janitor
* truncated the card because the bot is bigger than the context size.
* Truncation is the common one, so it leads.
*/
export function diagnoseSetup(content: string): SuckerError | null {
const hasPrompt = content.includes("<.>");
const persona = personaDescription(content);
const personaOk = persona === ".";
if (hasPrompt && personaOk) return null;
const stillStuck = `Still stuck: ${CONTACT}.`;
if (!hasPrompt) {
let also = "";
if (persona === null) {
also =
" Your persona did not come through either: it needs the name " +
"{{user}} and a description of just a dot.";
} else if (!personaOk) {
also = ` Your persona description is ${quoted(persona)} as well — it has to be just a dot.`;
}
return new SuckerError(
"Sucker could not read that card: your custom prompt is not coming " +
"through as <.> — set it in your API settings." +
also +
` Save, then refresh Janitor, not sucker. ${stillStuck}`
);
}
if (persona !== null) {
// Present but not "." — the one case that is certain.
return new SuckerError(
`Sucker could not read that card: ${what}. Save it, then refresh Janitor — ` +
"the settings page, not sucker. If it is already set that way, the bot is " +
"probably bigger than your context size and Janitor cut the card short, " +
`so raise Context Size in Generation Settings. Still stuck: ${CONTACT}.`
`Sucker could not read that card: your persona description is ${quoted(persona)} — ` +
"it has to be just a dot, with the name {{user}}. Save, then refresh " +
`Janitor, not sucker. ${stillStuck}`
);
}
// Prompt is fine and the persona marker is gone entirely: the end of the
// card is missing, and the usual reason is that Janitor cut it short.
return new SuckerError(
"Sucker could not read that card: the end of it is missing. That usually " +
"means the bot is bigger than your context size and Janitor cut it " +
"short — raise Context Size in Generation Settings and try again. If it " +
"is already generous, check your persona: name {{user}}, description " +
`just a dot, then refresh Janitor. ${stillStuck}`
);
}
+21 -15
View File
@@ -79,9 +79,10 @@ export function selectMessageSlots(messages: Message[] | undefined): MessageSlot
const users = list.filter((m) => m.role === "user");
return {
// Falling back to the first message keeps working if a client ever omits
// the role on the card, which is where it has always been.
system: (list.find((m) => m.role === "system") ?? list[0])?.text,
// A first message with no role at all is taken as the card, which is
// where it has always been. A request that has roles but no system turn
// is left with no card, so it is reported rather than misdiagnosed.
system: (list.find((m) => m.role === "system") ?? (list[0]?.role === "" ? list[0] : undefined))?.text,
greeting: greetingIndex === -1 ? undefined : list[greetingIndex].text,
nameSlot:
greetingIndex === -1
@@ -92,20 +93,25 @@ export function selectMessageSlots(messages: Message[] | undefined): MessageSlot
}
/**
* Message contents to search for a conversation token, most promising first.
* Message contents to search for a conversation token, in conversation order,
* starting after the greeting.
*
* Only sucker's *first* reply carries a token — every later reply says
* "Character updated" or "unchanged" with no token in it. So a long
* conversation has to be searched all the way back rather than at its tail,
* or the link to the existing card is lost and a duplicate gets created.
* What the user just typed still wins, since pasting a token is deliberate.
* A legitimate token lives in exactly one of two places, both after the
* greeting: the first user turn, when a chat was started with one to capture
* an alternate greeting, or sucker's own first reply, which issued it. Every
* later reply says "updated" or "unchanged" with no token in it, so a long
* conversation has to be searched back to its start or the link is lost and a
* duplicate card gets created.
*
* The card and the greeting are deliberately excluded. A card built from a
* sucker-proxied chat could carry an old token in its text, and a lookup on
* that would fail forever — the card would never be extractable.
* Conversation order also means the token a chat was started with always
* wins over one pasted later, so a chat cannot flip between cards.
*/
export function tokenCandidates(messages: Message[] | undefined): string[] {
const list = usable(messages);
const users = list.filter((m) => m.role === "user");
const mostRecentUser = users[users.length - 1];
return [mostRecentUser, ...list.filter((m) => m !== mostRecentUser)]
.filter((m): m is { role: string; text: string } => Boolean(m))
.map((m) => m.text);
const greetingIndex = list.findIndex((m) => m.role === "assistant");
if (greetingIndex === -1) return [];
return list.slice(greetingIndex + 1).map((m) => m.text);
}
+61 -21
View File
@@ -1,5 +1,6 @@
/**
* Which failures a user can act on, and which just need reporting.
* Which failures a user can act on, which just need reporting, and whether
* the setup diagnosis names the right step.
*/
import { test } from "node:test";
import assert from "node:assert/strict";
@@ -10,31 +11,71 @@ import {
NO_GREETING,
SuckerError,
TOO_FEW_MESSAGES,
missingSetup,
diagnoseSetup,
isImpossibleRequest,
isSuckerError,
userFacingMessage,
} from "../src/lib/errors.ts";
test("a missing setting is named, not left to the reader to find", () => {
const noPrompt = missingSetup(false, true).message;
assert.match(noPrompt, /custom prompt is not coming through as <\.>/);
assert.ok(!/persona needs the name/.test(noPrompt), "must not blame the setting that is fine");
const CARD = "<Gaius's Persona>a tired innkeeper</Gaius's Persona>";
const ok = (prompt: string, persona: string) => `${prompt}\n${CARD}\n${persona}`;
const noPersona = missingSetup(true, false).message;
assert.match(noPersona, /persona is not coming through/);
assert.ok(!/custom prompt is not coming through/.test(noPersona));
const neither = missingSetup(false, false).message;
assert.match(neither, /neither of the two required settings/);
test("a correctly set up card has nothing to diagnose", () => {
assert.equal(diagnoseSetup(ok("<.>", "<UserPersona>.</UserPersona>")), null);
});
test("every setup message still offers truncation as the second cause", () => {
for (const m of [missingSetup(false, true), missingSetup(true, false), missingSetup(false, false)]) {
assert.match(m.message, /Context Size/);
assert.ok(m.message.includes(CONTACT));
// Shown verbatim: this is the one class of failure a user can act on.
assert.equal(userFacingMessage(m), m.message);
test("a missing custom prompt is named with certainty, not blamed on truncation", () => {
const msg = diagnoseSetup(ok("", "<UserPersona>.</UserPersona>"))!.message;
assert.match(msg, /custom prompt is not coming through as <\.>/);
assert.ok(!/cut it short|Context Size/.test(msg), "truncation cannot remove the first three characters");
assert.ok(!/persona/i.test(msg), "the persona is fine and must not be mentioned");
});
test("a wrong persona description is quoted back, so there is nothing to guess", () => {
const msg = diagnoseSetup(ok("<.>", "<UserPersona>I am a knight</UserPersona>"))!.message;
assert.match(msg, /persona description is "I am a knight"/);
assert.match(msg, /just a dot/);
assert.ok(!/Context Size/.test(msg), "this case is certain; truncation is not the story");
});
test("an empty persona description says so", () => {
assert.match(diagnoseSetup(ok("<.>", "<UserPersona></UserPersona>"))!.message, /description is empty/);
});
test("a long persona description is trimmed to one readable line", () => {
const long = "x".repeat(80) + "\n\n y";
const msg = diagnoseSetup(ok("<.>", `<UserPersona>${long}</UserPersona>`))!.message;
assert.match(msg, /"x{40}…"/);
assert.ok(!msg.includes("\n"), "no raw newlines from user content");
});
test("a persona marker that is gone leads with truncation, the common cause", () => {
const msg = diagnoseSetup(ok("<.>", ""))!.message;
assert.match(msg, /end of it is missing/);
assert.ok(msg.indexOf("Context Size") < msg.indexOf("persona"), "truncation first, persona second");
});
test("a persona marker cut off mid-tag is truncation, not a wrong description", () => {
const msg = diagnoseSetup(ok("<.>", "<UserPersona>.</UserPe"))!.message;
assert.match(msg, /Context Size/);
assert.ok(!/description is/.test(msg));
});
test("both settings wrong at once are both named", () => {
const msg = diagnoseSetup(ok("", "<UserPersona>knight</UserPersona>"))!.message;
assert.match(msg, /custom prompt is not coming through/);
assert.match(msg, /description is "knight" as well/);
const neither = diagnoseSetup(ok("", ""))!.message;
assert.match(neither, /custom prompt is not coming through/);
assert.match(neither, /persona did not come through either/);
});
test("every setup message is shown verbatim and says where to go when stuck", () => {
for (const card of [ok("", "<UserPersona>.</UserPersona>"), ok("<.>", "<UserPersona>x</UserPersona>"), ok("<.>", ""), ok("", "")]) {
const e = diagnoseSetup(card)!;
assert.ok(isSuckerError(e));
assert.equal(userFacingMessage(e), e.message);
assert.ok(e.message.includes(CONTACT));
}
});
@@ -43,8 +84,7 @@ test("shapes Janitor cannot produce never tell the user to fix them", () => {
const msg = userFacingMessage(e);
assert.match(msg, /not your fault and there is nothing for you to fix/i);
assert.ok(msg.includes(CONTACT), "must say where to report it");
// The old failure mode: instructions for something they cannot do.
assert.ok(!/open the chat|try again|point sucker/i.test(msg), `actionable advice leaked into: ${msg}`);
assert.ok(!/open the chat|try again|point sucker|nothing was saved/i.test(msg), `overclaim or advice leaked into: ${msg}`);
}
});
@@ -63,7 +103,7 @@ test("a genuine bug reports itself without leaking internals", () => {
});
test("nobody is called a dingus", () => {
for (const e of [missingSetup(false, false), NO_CARD, NO_GREETING, TOO_FEW_MESSAGES, new Error("x")]) {
for (const e of [diagnoseSetup("nothing")!, NO_CARD, NO_GREETING, TOO_FEW_MESSAGES, new Error("x")]) {
assert.ok(!/dingus/i.test(userFacingMessage(e)));
}
});
+26 -2
View File
@@ -62,13 +62,26 @@ test("the token is still found once later replies stop carrying it", () => {
assert.match(found!, /\[sucker:conv=abc123\]/);
});
test("a token the user just pasted outranks an older one", () => {
test("the token a chat was started with outranks one pasted later, so a chat cannot flip between cards", () => {
const candidates = tokenCandidates([
...withProbe,
{ role: "assistant", content: `created (v1). ${TOKEN}` },
{ role: "user", content: "{{user}}: [sucker:conv=zzz999]" },
]);
assert.match(candidates[0], /zzz999/);
const first = candidates.find((c) => c.includes("sucker:conv="));
assert.match(first!, /abc123/);
});
test("a token in the card or the greeting is never a candidate", () => {
const stale = "[sucker:conv=stale00]";
const candidates = tokenCandidates([
{ role: "system", content: `${CARD} made with sucker: ${stale}` },
{ role: "user", content: "." },
{ role: "assistant", content: `${GREETING} ${stale}` },
{ role: "user", content: NAME },
]);
assert.ok(!candidates.some((c) => c.includes(stale)), "an expired token baked into a card would make it unextractable forever");
assert.deepEqual(candidates, [NAME]);
});
test("roles are matched whatever their casing", () => {
@@ -106,6 +119,17 @@ test("messages with no usable content are skipped, not treated as slots", () =>
assert.equal(slots.nameSlot, NAME);
});
test("a request with roles but no system turn has no card, rather than a misdiagnosed one", () => {
const slots = selectMessageSlots([
{ role: "assistant", content: GREETING },
{ role: "user", content: NAME },
]);
// Taking the greeting as the card would produce a "custom prompt not set"
// message for a request the user could never have fixed.
assert.equal(slots.system, undefined);
assert.equal(slots.greeting, GREETING);
});
test("a card with no role on the system turn still resolves", () => {
const slots = selectMessageSlots([
{ content: CARD },