diff --git a/src/app/api/proxy/route.ts b/src/app/api/proxy/route.ts index 45a60af..d19b142 100644 --- a/src/app/api/proxy/route.ts +++ b/src/app/api/proxy/route.ts @@ -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("."); - 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); } diff --git a/src/lib/errors.ts b/src/lib/errors.ts index 7de3273..19a983b 100644 --- a/src/lib/errors.ts +++ b/src/lib/errors.ts @@ -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 = ""; +const PERSONA_CLOSE = ""; - 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. `…` 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: 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: ${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: 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}` ); } diff --git a/src/lib/messages.ts b/src/lib/messages.ts index c6141ec..72e2615 100644 --- a/src/lib/messages.ts +++ b/src/lib/messages.ts @@ -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); } diff --git a/tests/errors.test.ts b/tests/errors.test.ts index c5e0c8a..355b13d 100644 --- a/tests/errors.test.ts +++ b/tests/errors.test.ts @@ -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 = "a tired innkeeper"; +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("<.>", ".")), 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("", "."))!.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("<.>", "I am a knight"))!.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("<.>", ""))!.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("<.>", `${long}`))!.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("<.>", ". { + const msg = diagnoseSetup(ok("", "knight"))!.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("", "."), ok("<.>", "x"), 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))); } }); diff --git a/tests/messages.test.ts b/tests/messages.test.ts index 358ef1f..e809961 100644 --- a/tests/messages.test.ts +++ b/tests/messages.test.ts @@ -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 },