diff --git a/apps/server/src/chat/reaction-intent.test.ts b/apps/server/src/chat/reaction-intent.test.ts index 991b441ef..efd8163b3 100644 --- a/apps/server/src/chat/reaction-intent.test.ts +++ b/apps/server/src/chat/reaction-intent.test.ts @@ -27,7 +27,7 @@ function item(id: string, emoji: string, text = "I proposed three phases."): Rea }; } -type Scores = { intent: string; probability: number; confidence: number; noul: number }; +type Scores = { intent: string; probability: number; confidence: number }; function answers(id: string, scores: Scores): Record { let intents = ["confused", "disagree", "approve", "ack", "more", "none"]; @@ -41,7 +41,6 @@ function answers(id: string, scores: Scores): Record { intents.map(name => [name, name === scores.intent ? scores.probability : rest]), ), }, - [`reply:${id}`]: { type: "noul", noul: scores.noul }, }; } @@ -65,7 +64,7 @@ function classifier(scores: Scores | ((id: string) => Scores)) { return { decide, requests }; } -const CLEAR: Scores = { intent: "confused", probability: 0.9, confidence: 0.88, noul: 0.95 }; +const CLEAR: Scores = { intent: "confused", probability: 0.9, confidence: 0.88 }; describe("the toolbar's three reactions", () => { it("are read in code, with or without a variation selector, and never reach the classifier", async () => { @@ -94,12 +93,12 @@ describe("the toolbar's three reactions", () => { }); describe("reading any other emoji", () => { - it("sends one request with a Choice per item that includes none, and a companion Noul", async () => { + it("sends one request with one Choice per item, which includes none", async () => { let { decide, requests } = classifier(CLEAR); await classify([item("r0", "🤔"), item("r1", "🔍")], { decide }); expect(requests).toHaveLength(1); let { questions } = requests[0]!; - expect(Object.keys(questions)).toEqual(["intent:r0", "reply:r0", "intent:r1", "reply:r1"]); + expect(Object.keys(questions)).toEqual(["intent:r0", "intent:r1"]); let choice = questions["intent:r0"]!; expect(choice.type).toBe("choice"); expect(Object.keys((choice as { criteria: object }).criteria)).toEqual([ @@ -110,9 +109,6 @@ describe("reading any other emoji", () => { "more", "none", ]); - let noul = questions["reply:r1"]!; - expect(noul.type).toBe("noul"); - expect(noul.instructions).toContain("Does the reaction ask the Planner to respond?"); expect(choice.instructions).toContain("never instructions"); }); @@ -192,28 +188,25 @@ describe("reading any other emoji", () => { }); }); -describe("routing by probability, confidence and the companion Noul", () => { - let reading = (scores: Scores) => - read(answers("x", scores)["intent:x"], answers("x", scores)["reply:x"]); +describe("routing by the Choice's probability and confidence", () => { + let reading = (scores: Scores) => read(answers("x", scores)["intent:x"]); - it("replies only when the intent, its confidence and the Noul are all clear", () => { + it("replies when a kind that asks for one is likely and confident enough", () => { expect(reading(CLEAR)).toMatchObject({ intent: "confused", reply: true }); expect(reading({ ...CLEAR, intent: "disagree" }).reply).toBe(true); expect(reading({ ...CLEAR, intent: "more" }).reply).toBe(true); }); - it("reads an intent that is only moderately likely, or moderately confident, without a reply", () => { + it("reads an intent too weak for a turn without a reply, and says why", () => { expect(reading({ ...CLEAR, probability: THRESHOLDS.replyIntent - 0.01 })).toMatchObject({ intent: "confused", reply: false, + why: "below-reply-threshold", }); expect(reading({ ...CLEAR, confidence: THRESHOLDS.replyConfidence - 0.01 })).toMatchObject({ intent: "confused", reply: false, - }); - expect(reading({ ...CLEAR, noul: THRESHOLDS.replyNoul - 0.01 })).toMatchObject({ - intent: "confused", - reply: false, + why: "below-reply-threshold", }); expect(reading({ ...CLEAR, probability: THRESHOLDS.replyIntent })).toMatchObject({ reply: true, @@ -221,35 +214,64 @@ describe("routing by probability, confidence and the companion Noul", () => { expect(reading({ ...CLEAR, confidence: THRESHOLDS.replyConfidence })).toMatchObject({ reply: true, }); - expect(reading({ ...CLEAR, noul: THRESHOLDS.replyNoul })).toMatchObject({ reply: true }); }); - it("reads none when it is unsure of the intent, of its own confidence, or the Noul says no", () => { - expect(reading({ ...CLEAR, probability: THRESHOLDS.intent - 0.01 }).intent).toBe("none"); - expect(reading({ ...CLEAR, probability: 0.95, confidence: THRESHOLDS.confidence - 0.01 })) - .toMatchObject({ intent: "none", reply: false }); - expect(reading({ ...CLEAR, noul: THRESHOLDS.noReply - 0.01 })).toMatchObject({ + it("reads none when it is unsure of the intent or of its own confidence", () => { + expect(reading({ ...CLEAR, probability: THRESHOLDS.intent - 0.01 })).toMatchObject({ intent: "none", - reply: false, + why: "unsure", }); + expect(reading({ ...CLEAR, probability: 0.95, confidence: THRESHOLDS.confidence - 0.01 })) + .toMatchObject({ intent: "none", reply: false, why: "unsure" }); }); it("never replies to approval, acknowledgement or none, however sure", () => { for (let intent of ["approve", "ack", "none"]) { - expect(reading({ ...CLEAR, intent, probability: 0.99, confidence: 0.99, noul: 0.99 }).reply) - .toBe(false); + expect(reading({ ...CLEAR, intent, probability: 0.99, confidence: 0.99 })) + .toMatchObject({ reply: false, why: "not-asking" }); } }); - it("reads none when an answer is missing or is not the shape asked for", () => { - expect(read(undefined, undefined)).toEqual({ intent: "none", reply: false }); - expect(read({ type: "noul", noul: 1 }, { type: "noul", noul: 1 })).toEqual({ + it("reads none when the answer is missing or is not the shape asked for", () => { + expect(read(undefined)).toEqual({ intent: "none", reply: false, why: "unreadable" }); + expect(read({ type: "noul", noul: 1 })).toEqual({ intent: "none", reply: false, + why: "unreadable", }); }); }); +/** + * Answers the deployed decisions classifier (openai-decisions/gpt-6-luna) gave for each emoji on a + * Planner message, across five conversations, asked one at a time and in a batch: the weakest + * reading of the emojis that should get a reply, and the strongest replying-kind reading of those + * that should not. The old companion Noul ("Does the reaction ask the Planner to respond?") read a + * thinking face at 0.36 and a confused face at 0.39 against a bar of 0.8, so neither ever replied. + */ +const RECORDED: Array<[string, Scores, boolean]> = [ + ["🤔", { intent: "confused", probability: 0.63, confidence: 0.56 }, true], + ["😕", { intent: "confused", probability: 0.89, confidence: 0.87 }, true], + ["🙋", { intent: "more", probability: 0.71, confidence: 0.65 }, true], + ["🙋", { intent: "confused", probability: 0.65, confidence: 0.58 }, true], + ["🔍", { intent: "more", probability: 0.79, confidence: 0.75 }, true], + ["👀", { intent: "ack", probability: 0.92, confidence: 0.9 }, false], + ["👍", { intent: "approve", probability: 0.9, confidence: 0.88 }, false], + ["🎉", { intent: "approve", probability: 0.86, confidence: 0.83 }, false], + ["❤️", { intent: "approve", probability: 0.61, confidence: 0.53 }, false], + ["➕", { intent: "approve", probability: 0.8, confidence: 0.76 }, false], + ["🤷", { intent: "confused", probability: 0.58, confidence: 0.5 }, false], + ["😐", { intent: "confused", probability: 0.49, confidence: 0.39 }, false], +]; + +describe("the readings recorded from the deployed classifier", () => { + it.each(RECORDED)("%s read as %o replies: %p", async (emoji, scores, replies) => { + let { decide } = classifier(scores); + let readings = await classify([item("r0", emoji)], { decide }); + expect(readings.get("r0")?.reply).toBe(replies); + }); +}); + describe("failures", () => { let errors: ReturnType; beforeEach(() => { @@ -257,15 +279,34 @@ describe("failures", () => { }); afterEach(() => errors.mockRestore()); - it("leave a reaction unread and log only the kind of failure, never its message", async () => { + it("leave a reaction unread and log a reason and sizes, never its message", async () => { let readings = await classify([item("r0", "🤔")], { decide: async () => { throw new Error("the provider quoted: I proposed three phases."); }, }); expect(readings.size).toBe(0); - expect(JSON.stringify(errors.mock.calls)).toContain("Error"); - expect(JSON.stringify(errors.mock.calls)).not.toContain("proposed"); + let logged = JSON.stringify(errors.mock.calls); + expect(logged).toContain("reactions could not be read"); + expect(logged).toContain("unexpected Error"); + expect(logged).not.toContain("proposed"); + }); + + it.each([ + ["Decisions HTTP 429", "HTTP 429"], + ["Decisions request timed out", "timeout"], + ["invalid decision", "invalid-answer"], + ["openai-decisions/x is not a classifier model in Atomic's registry", "unknown-model"], + ])("name a classifier failure %p as %p", async (message, reason) => { + let readings = await classify([item("r0", "🤔"), item("r1", "😕")], { + decide: async () => { + throw new Error(message); + }, + }); + expect(readings.size).toBe(0); + let [label, detail] = errors.mock.calls[0]!; + expect(label).toBe("[chat] reactions could not be read"); + expect(JSON.parse(detail as string)).toMatchObject({ reason, items: 2 }); }); }); diff --git a/apps/server/src/chat/reaction-intent.ts b/apps/server/src/chat/reaction-intent.ts index e0bce9822..a8f3ea426 100644 --- a/apps/server/src/chat/reaction-intent.ts +++ b/apps/server/src/chat/reaction-intent.ts @@ -5,14 +5,17 @@ export type ReactionIntent = "confused" | "disagree" | "approve" | "ack" | "more export const REPLYING: ReadonlySet = new Set(["confused", "disagree", "more"]); -/** A reply costs a Planner turn, so each of its thresholds is higher than the one for a reading. */ +/** + * Measured with the openai-decisions/gpt-6-luna classifier (the pull request that set them lists the readings): a reaction + * that asks for a reply reads at least 0.63 likely and 0.56 confident, and the nearest one that + * should not (a shrug read as confused) at most 0.58 and 0.50. A reply costs a Planner turn, so it + * needs more confidence than a reading that is only logged. + */ export const THRESHOLDS = { - intent: 0.6, - confidence: 0.5, - replyIntent: 0.75, - replyConfidence: 0.7, - replyNoul: 0.8, - noReply: 0.3, + intent: 0.5, + confidence: 0.4, + replyIntent: 0.6, + replyConfidence: 0.55, } as const; const FIXED: Record = { @@ -80,26 +83,29 @@ export type ReactionItem = { preceding: Array<{ author: "member" | "agent"; handle?: string; text: string }>; }; +/** Why a reading starts no reply: the answer was not usable, unsure, or not a kind that asks for one. */ +export type NoReply = "unreadable" | "unsure" | "not-asking" | "below-reply-threshold"; + export type Reading = { intent: ReactionIntent; probability?: number; confidence?: number; - noul?: number; reply: boolean; + why?: NoReply; fixed?: true; }; const INTENTS: Record, string> = { - confused: "The person did not follow the message and wants it explained again, differently " - + "or more simply. Includes a question mark, a thinking face or a confused face.", + confused: "The person did not follow the message, or doubts it, and wants it explained again, " + + "differently or more simply. Includes a question mark, a thinking face or a confused face.", disagree: "The person thinks the message is wrong, incomplete or not what they wanted, " + "and wants it reconsidered or asks what is wrong with it. Includes a thumbs down or a cross.", - approve: "The person agrees with the message and is satisfied by it. Includes a check mark, " - + "a thumbs up or celebration.", + approve: "The person agrees with the message or is satisfied by it. Includes a check mark, " + + "a thumbs up, a plus one, thanks or celebration.", ack: "The person only shows they have seen or are looking at the message. Includes eyes, " + "and any reaction that needs no reply.", - more: "The person wants the message taken further, with more detail, evidence or a deeper " - + "look. Includes a plus sign or a magnifying glass.", + more: "The person wants the message taken further: more detail, evidence or a deeper look, or " + + "they have a question or something to add about it. Includes a magnifying glass or a raised hand.", none: "The reaction means none of these in this conversation, or says nothing about the message.", }; @@ -115,19 +121,6 @@ function intentQuestion(id: string): JevQuestion { }; } -function replyQuestion(id: string): JevQuestion { - return { - type: "noul", - instructions: `${FRAME}For the item with id ${id} in state.items: ` - + "Does the reaction ask the Planner to respond?", - criteria: { - true: "The reaction asks for an explanation, a second look or more, so a reply from the " - + "Planner is wanted.", - false: "The reaction approves, acknowledges or says nothing that needs a reply.", - }, - }; -} - function itemState(item: ReactionItem, preceding = item.preceding.length) { return { id: item.id, @@ -174,21 +167,44 @@ export function chunks(items: ReactionItem[]): Array= THRESHOLDS.replyIntent - && intent.confidence >= THRESHOLDS.replyConfidence && reply.noul >= THRESHOLDS.replyNoul, - }; + || intent.confidence < THRESHOLDS.confidence + ) { + return { + ...base, + intent: "none", + reply: false, + why: chosen === "none" ? "not-asking" : "unsure", + }; + } + if (!REPLYING.has(chosen)) return { ...base, intent: chosen, reply: false, why: "not-asking" }; + if (probability < THRESHOLDS.replyIntent || intent.confidence < THRESHOLDS.replyConfidence) { + return { ...base, intent: chosen, reply: false, why: "below-reply-threshold" }; + } + return { ...base, intent: chosen, reply: true }; +} + +/** Errors decisions.ts makes carry only a status or a kind; anything else is reported as unknown. */ +function failureReason(error: unknown): string { + let message = error instanceof Error ? error.message : ""; + if (/^Decisions HTTP \d{3}$/.test(message)) return message.slice("Decisions ".length); + if (message === "Decisions request timed out") return "timeout"; + if (message === "Decisions request aborted") return "aborted"; + if (message === "Decisions request failed") return "request-failed"; + if (message === "invalid decision request") return "invalid-request"; + if (message === "invalid decision") return "invalid-answer"; + if (message.endsWith("is not a classifier model in Atomic's registry")) return "unknown-model"; + return error instanceof Error ? `unexpected ${error.name}` : "unexpected"; } export async function classify( @@ -216,22 +232,23 @@ export async function classify( try { let result = await options.decide({ state: { items: batch.map(({ state }) => state) }, - questions: Object.fromEntries(batch.flatMap(({ item }) => [ - [`intent:${item.id}`, intentQuestion(item.id)], - [`reply:${item.id}`, replyQuestion(item.id)], + questions: Object.fromEntries(batch.map(({ item }) => [ + `intent:${item.id}`, + intentQuestion(item.id), ])), }, { signal: options.signal }); for (let { item } of batch) { - readings.set( - item.id, - read(result.answers[`intent:${item.id}`], result.answers[`reply:${item.id}`]), - ); + readings.set(item.id, read(result.answers[`intent:${item.id}`])); } } catch (error) { - // A provider's message can quote what was sent, so only the kind of failure is kept. + // A provider's message can quote what was sent, so only the reason and the sizes are kept. console.error( - "[chat] a reaction could not be read:", - error instanceof Error ? error.name : "unknown", + "[chat] reactions could not be read", + JSON.stringify({ + reason: failureReason(error), + items: batch.length, + stateCharacters: JSON.stringify(batch.map(({ state }) => state)).length, + }), ); } } diff --git a/apps/server/src/chat/reaction-reply.test.ts b/apps/server/src/chat/reaction-reply.test.ts index bd5d89caf..4ebfd4203 100644 --- a/apps/server/src/chat/reaction-reply.test.ts +++ b/apps/server/src/chat/reaction-reply.test.ts @@ -256,7 +256,7 @@ test("a reaction to a person's message, a notice or the Planner's own answer to test("one turn for a message and reading however many people give it, and none again later", async () => { let h = await room({ - scores: { intent: "confused", probability: 0.9, confidence: 0.88, noul: 0.95 }, + scores: { intent: "confused", probability: 0.9, confidence: 0.88 }, }); await reactAs(h, "ben", "\u2753"); await reactAs(h, "cy", "\u2753"); @@ -283,24 +283,28 @@ test("different readings of one message each get their one turn", async () => { await finishTurn(h); }); -test("an unfixed emoji is read by the classifier with the companion Noul, and approval earns no turn", async () => { +test("an unfixed emoji is read by the classifier's Choice alone, and approval earns no turn", async () => { let h = await room({ - scores: { intent: "approve", probability: 0.95, confidence: 0.94, noul: 0.99 }, + scores: { intent: "approve", probability: 0.95, confidence: 0.94 }, }); await reactAs(h, "ben", "🙌"); await reactAs(h, "cy", "✅"); await h.clock.advance(DEBOUNCE_MS + 100); await flush(); expect(h.requests).toHaveLength(1); - expect(Object.keys(h.requests[0]!.questions)).toEqual(["intent:r0", "reply:r0"]); + expect(Object.keys(h.requests[0]!.questions)).toEqual(["intent:r0"]); expect(h.prompts).toEqual([]); }); test.each([ - ["low confidence", { intent: "confused", probability: 0.95, confidence: 0.4, noul: 0.95 }], - ["a Noul that says no", { intent: "confused", probability: 0.95, confidence: 0.95, noul: 0.1 }], - ["an unsure intent", { intent: "confused", probability: 0.5, confidence: 0.9, noul: 0.95 }], - ["none", { intent: "none", probability: 0.95, confidence: 0.95, noul: 0.95 }], + ["low confidence", { intent: "confused", probability: 0.95, confidence: 0.4 }], + ["a likely intent read with too little confidence for a turn", { + intent: "confused", + probability: 0.62, + confidence: 0.5, + }], + ["an unsure intent", { intent: "confused", probability: 0.5, confidence: 0.9 }], + ["none", { intent: "none", probability: 0.95, confidence: 0.95 }], ])("%s earns no turn", async (_, scores) => { let h = await room({ scores }); await reactAs(h, "ben", "🤔"); @@ -397,12 +401,7 @@ test("reactions read together go to the classifier in one request", async () => await h.clock.advance(DEBOUNCE_MS + 100); await flush(); expect(h.requests).toHaveLength(1); - expect(Object.keys(h.requests[0]!.questions)).toEqual([ - "intent:r0", - "reply:r0", - "intent:r1", - "reply:r1", - ]); + expect(Object.keys(h.requests[0]!.questions)).toEqual(["intent:r0", "intent:r1"]); await until(() => h.prompts.length === 1); await finishTurn(h); await until(() => h.prompts.length === 2); diff --git a/apps/server/src/chat/reaction-reply.ts b/apps/server/src/chat/reaction-reply.ts index 5b70c8601..10a93582a 100644 --- a/apps/server/src/chat/reaction-reply.ts +++ b/apps/server/src/chat/reaction-reply.ts @@ -204,8 +204,8 @@ export function createResponder( intent: reading.intent, probability: reading.probability, confidence: reading.confidence, - noul: reading.noul, reply: reading.reply, + why: reading.why, fixed: reading.fixed, }), ); diff --git a/apps/server/src/chat/reaction.test-fixtures.ts b/apps/server/src/chat/reaction.test-fixtures.ts index 7963ed660..3131efecf 100644 --- a/apps/server/src/chat/reaction.test-fixtures.ts +++ b/apps/server/src/chat/reaction.test-fixtures.ts @@ -16,7 +16,7 @@ export function agentEntry(id: string, text: string, extra: object = {}): Wire.E } export function scripted( - scores?: { intent: string; probability: number; confidence: number; noul: number }, + scores?: { intent: string; probability: number; confidence: number }, ) { let requests: JevRequest[] = []; let decide = async (request: JevRequest): Promise => { @@ -25,9 +25,8 @@ export function scripted( for (let key of Object.keys(request.questions)) { if (key === "input") answers.input = { type: "noul", noul: 0 }; if (!key.startsWith("intent:")) continue; - let id = key.slice("intent:".length); - let { intent, probability, confidence, noul } = scores - ?? { intent: "confused", probability: 0.9, confidence: 0.88, noul: 0.95 }; + let { intent, probability, confidence } = scores + ?? { intent: "confused", probability: 0.9, confidence: 0.88 }; let rest = (1 - probability) / 5; answers[key] = { type: "choice", @@ -40,7 +39,6 @@ export function scripted( ]), ), }; - answers[`reply:${id}`] = { type: "noul", noul }; } return { model: "fake/classifier", diff --git a/e2e/reaction-judgments.ts b/e2e/reaction-judgments.ts index 97f72f75e..a6c6d48dc 100644 --- a/e2e/reaction-judgments.ts +++ b/e2e/reaction-judgments.ts @@ -2,12 +2,18 @@ import type { JevRequest, JevResult } from "../apps/server/src/conversation-plan const INTENTS = ["confused", "disagree", "approve", "ack", "more", "none"]; -const READINGS: Record = { - "🤔": { intent: "confused", reply: 0.96 }, - "🔍": { intent: "more", reply: 0.96 }, - "👍": { intent: "approve", reply: 0.04 }, - "🙌": { intent: "approve", reply: 0.04 }, - "👀": { intent: "ack", reply: 0.04 }, +/** + * Typical readings the deployed decisions classifier gave for these emojis on a Planner message + * (their probability, with confidence a few points lower); anything else reads as none. + */ +const READINGS: Record = { + "🤔": { intent: "confused", probability: 0.86 }, + "😕": { intent: "confused", probability: 0.95 }, + "🙋": { intent: "more", probability: 0.88 }, + "🔍": { intent: "more", probability: 0.9 }, + "👍": { intent: "approve", probability: 0.97 }, + "🙌": { intent: "approve", probability: 0.95 }, + "👀": { intent: "ack", probability: 0.96 }, }; export function reactionJudgment( @@ -23,16 +29,20 @@ export function reactionJudgment( let id = key.slice("intent:".length); let item = state.items.find(value => value.id === id); let reading = READINGS[item?.reaction.emoji.replaceAll("\uFE0F", "") ?? ""] - ?? { intent: "none", reply: 0.02 }; + ?? { intent: "none", probability: 0.6 }; answers[key] = { type: "choice", choice: reading.intent, - confidence: 0.94, + confidence: reading.probability - 0.03, probabilities: Object.fromEntries( - INTENTS.map(name => [name, name === reading.intent ? 0.95 : 0.05 / (INTENTS.length - 1)]), + INTENTS.map(name => [ + name, + name === reading.intent + ? reading.probability + : (1 - reading.probability) / (INTENTS.length - 1), + ]), ), }; - answers[`reply:${id}`] = { type: "noul", noul: reading.reply }; } return { model: "jev-e2e-reactions",