From 73b2f29e0352f3becc2104043e29a8e611680b98 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Sat, 29 Aug 2026 00:19:36 +0000 Subject: [PATCH 01/14] feat(chat): add optional fast-model output router Route completed tool-free assistant messages through AI_FAST_MODEL before delivery when experimental output-router is enabled. Handles NO_REPLY silence and long-reply compression with a tight structured prompt. Co-Authored-By: David Cramer --- TELEMETRY.md | 4 +- .../content/docs/reference/config-and-env.md | 8 + packages/junior/src/chat/README.md | 6 +- packages/junior/src/chat/agent/index.ts | 30 +- packages/junior/src/chat/experimental.ts | 6 +- .../junior/src/chat/services/output-router.ts | 363 ++++++++++++++++++ .../tests/fixtures/experimental-setup.ts | 1 + .../junior/tests/unit/experimental.test.ts | 15 +- .../tests/unit/services/output-router.test.ts | 220 +++++++++++ 9 files changed, 644 insertions(+), 9 deletions(-) create mode 100644 packages/junior/src/chat/services/output-router.ts create mode 100644 packages/junior/tests/unit/services/output-router.test.ts diff --git a/TELEMETRY.md b/TELEMETRY.md index 0f19af7d15..f32899f06e 100644 --- a/TELEMETRY.md +++ b/TELEMETRY.md @@ -174,6 +174,7 @@ Events: `agent.message.received`, `agent.message.generated`, `agent.turn.provider_error`, `agent.turn.execution.failed`, `agent.turn.empty_output.retrying`, `agent.turn.empty_output.exhausted`, `assistant.reply.generation.failed`, +`ai.output_router.decided`, `ai.output_router.failed`, `guardian.action_review.retrying`, `guardian.action_review.exhausted` `guardian.action_review.exhausted` is a tool-boundary Sentry capture after three @@ -181,7 +182,8 @@ consecutive action-review denials. The agent still receives a normal tool rejection that says not to keep retrying. Spans: `ai.generate_assistant_reply`, `ai.chat_completion`, -`chat.route_thinking`, `gen_ai.invoke_agent`, `gen_ai.chat` +`chat.route_thinking`, `chat.route_assistant_output`, `gen_ai.invoke_agent`, +`gen_ai.chat` Attributes: `gen_ai.operation.name`, `gen_ai.request.model`, `gen_ai.response.finish_reasons`, `app.ai.outcome`, diff --git a/packages/docs/src/content/docs/reference/config-and-env.md b/packages/docs/src/content/docs/reference/config-and-env.md index 3898582a55..39931b313f 100644 --- a/packages/docs/src/content/docs/reference/config-and-env.md +++ b/packages/docs/src/content/docs/reference/config-and-env.md @@ -139,6 +139,9 @@ import { createApp } from "@sentry/junior"; const app = await createApp({ experimental: { + // Fast-model pass over every completed assistant message before delivery. + // Handles [[NO_REPLY]] cleanup and long-reply compression. Off by default. + "output-router": true, // Reply to non-mention messages in Slack threads Junior already joined. // Off by default. Without this, Junior only replies to explicit @mentions // and resource-event notifications in those threads. @@ -169,6 +172,11 @@ request reaches the deployment request limit. Run `pnpm acp:local` in this repository for a loopback test with the official ACP SDK client. ACP remains a pre-stable surface. +`output-router` runs the fast model (`AI_FAST_MODEL`) on each completed +tool-free assistant message before destination delivery. It suppresses pure +`[[NO_REPLY]]` output, strips mixed markers, and can rewrite overly long +replies. Leave it unset unless you are testing that path. + `passive-routing` turns on replies to non-mention messages in threads Junior already joined. Leave it unset in production unless you are testing that path. diff --git a/packages/junior/src/chat/README.md b/packages/junior/src/chat/README.md index 77e9c867d3..dfa90fba0a 100644 --- a/packages/junior/src/chat/README.md +++ b/packages/junior/src/chat/README.md @@ -20,8 +20,10 @@ file. 6. `agent/` emits every completed, tool-free visible assistant message through one awaited delivery port with the completed Pi message that produced it; provider adapters deliver, then commit that agent message before the visible - reply in one transaction. Tool-bearing assistant text remains internal to - the agent loop. + reply in one transaction. When experimental `output-router` is enabled, the + fast model routes each message before delivery (silence marker handling and + length rewrite). Tool-bearing assistant text remains internal to the agent + loop. 7. The completed run result supplies diagnostics and artifacts; successful delivery or intentional no-reply completion commits the durable turn outcome. diff --git a/packages/junior/src/chat/agent/index.ts b/packages/junior/src/chat/agent/index.ts index c01760bf50..61458e116e 100644 --- a/packages/junior/src/chat/agent/index.ts +++ b/packages/junior/src/chat/agent/index.ts @@ -78,6 +78,8 @@ import { isTurnInputCommitLostError } from "@/chat/runtime/turn"; import type { AgentRunOutcome } from "@/chat/runtime/agent-run-outcome"; import { buildTurnResult } from "@/chat/services/turn-result"; import { decideReply } from "@/chat/services/assistant-reply"; +import { routeAssistantMessage } from "@/chat/services/output-router"; +import { isExperimentalFeatureEnabled } from "@/chat/experimental"; import { findProviderError, getProviderErrorAttributes, @@ -1082,12 +1084,34 @@ async function executeAgentRunInPrivacyContext( const deliverAssistantMessage = async ( message: Parameters[0], ): Promise => { - const decision = decideReply(message); - if (decision.kind !== "deliver" || !delivery) { + if (!delivery) { return; } + + let deliverable = message; + if (isExperimentalFeatureEnabled("output-router")) { + const routed = await routeAssistantMessage({ + completeObject, + context: { + conversationId, + runId, + }, + fastModelId: botConfig.fastModelId, + message, + }); + if (routed.kind === "skip" || routed.kind === "suppress") { + return; + } + deliverable = routed.message; + } else { + const decision = decideReply(message); + if (decision.kind !== "deliver") { + return; + } + } + try { - await delivery(message); + await delivery(deliverable); acceptedToolFreeAssistant = true; } catch (error) { assistantMessageDeliveryError = new AssistantMessageDeliveryError( diff --git a/packages/junior/src/chat/experimental.ts b/packages/junior/src/chat/experimental.ts index 3096ce8145..6fa33eb09f 100644 --- a/packages/junior/src/chat/experimental.ts +++ b/packages/junior/src/chat/experimental.ts @@ -3,7 +3,11 @@ * Add new keys here as features graduate from private experiments; remove them * once they become stable defaults. */ -export const EXPERIMENTAL_FEATURES = ["passive-routing", "subagents"] as const; +export const EXPERIMENTAL_FEATURES = [ + "output-router", + "passive-routing", + "subagents", +] as const; /** One known experimental feature name. */ export type ExperimentalFeature = (typeof EXPERIMENTAL_FEATURES)[number]; diff --git a/packages/junior/src/chat/services/output-router.ts b/packages/junior/src/chat/services/output-router.ts new file mode 100644 index 0000000000..608d49bb6b --- /dev/null +++ b/packages/junior/src/chat/services/output-router.ts @@ -0,0 +1,363 @@ +import type { AssistantMessage } from "@earendil-works/pi-ai"; +import { z } from "zod"; +import { NO_REPLY_MARKER, isNoReplyMarker } from "@/chat/no-reply"; +import { + logInfo, + logWarn, + setSpanAttributes, + withSpan, + type LogContext, +} from "@/chat/logging"; +import { + decideReply, + sanitizeAssistantText, +} from "@/chat/services/assistant-reply"; +import { extractAssistantText } from "@/chat/pi/transcript"; + +/** + * Destination reply length budget used by the optional output router. + * Matches the Slack output prompt target (~1–5 sentences). + */ +export const OUTPUT_REPLY_SOFT_MAX_CHARS = 800; +/** Hard cap after rewrite; longer routed text is truncated deterministically. */ +export const OUTPUT_REPLY_HARD_MAX_CHARS = 1_200; +const OUTPUT_ROUTER_MAX_TOKENS = 1_600; +const OUTPUT_ROUTER_PROMPT_MAX_CHARS = 12_000; + +const outputRouteSchema = z + .object({ + action: z.enum(["deliver", "suppress", "rewrite"]), + text: z.string().optional(), + reason: z.string().min(1), + }) + .strict(); + +export type OutputRouteAction = z.infer["action"]; + +export type OutputRoute = { + action: "deliver" | "suppress"; + costUsd?: number; + reason: string; + source: "deterministic" | "router" | "fallback"; + text?: string; +}; + +type CompleteObject = (args: { + modelId: string; + schema: typeof outputRouteSchema; + maxTokens: number; + metadata: Record; + prompt: string; + thinkingLevel?: "low" | "medium" | "high" | "xhigh"; + system: string; + temperature: number; + promptName?: string; +}) => Promise<{ costUsd?: number; object: unknown }>; + +function buildOutputRouterSystemPrompt(): string { + // Tight output guardrail. Pattern: OpenAI cookbook "output guardrails" + // (validate LLM output before delivery) + structured JSON decisions. + // https://developers.openai.com/cookbook/examples/how_to_use_guardrails/ + return [ + "You are Junior's final output router.", + "Input is one completed assistant message. No other context is available.", + "Decide the destination-visible reply before delivery.", + "", + "Actions:", + "- suppress: no visible reply should be delivered", + "- deliver: keep the message text as-is", + "- rewrite: replace the message with shorter destination-visible text", + "", + "Rules:", + `1. suppress when the whole message is intentional silence (exact ${NO_REPLY_MARKER}, or equivalent pure no-reply protocol text with no other answer).`, + `2. if ${NO_REPLY_MARKER} appears mixed with real answer text, rewrite: strip the marker and keep the answer.`, + "3. deliver short complete answers unchanged.", + `4. rewrite long answers that exceed ~${OUTPUT_REPLY_SOFT_MAX_CHARS} characters unless the user clearly asked for full detail, a long dump, or a large code/config block that must stay intact.`, + "5. rewrites must stay faithful: keep the outcome, decisive evidence, blockers, links, and required next actions. Do not invent facts.", + `6. rewritten text should usually be 1–5 sentences and under ${OUTPUT_REPLY_SOFT_MAX_CHARS} characters. Prefer a short summary plus a pointer (for example a canvas/doc link) when detail is too long.`, + "7. preserve fenced code only when it is essential and still short enough; otherwise summarize and point to where the full content lives.", + "8. never add preamble, meta commentary, or process narration.", + "", + "Return JSON only with action, reason, and text.", + "text is required for deliver and rewrite; omit text for suppress.", + "reason is one short sentence.", + ].join("\n"); +} + +function buildOutputRouterPrompt(text: string): string { + const body = + text.length <= OUTPUT_ROUTER_PROMPT_MAX_CHARS + ? text + : `${text.slice(0, OUTPUT_ROUTER_PROMPT_MAX_CHARS)}\n…[truncated]…`; + return ["", body, ""].join("\n"); +} + +function normalizeRoutedText(text: string | undefined): string | undefined { + if (text === undefined) return undefined; + const normalized = sanitizeAssistantText(text); + return normalized || undefined; +} + +function enforceHardCap(text: string): string { + if (text.length <= OUTPUT_REPLY_HARD_MAX_CHARS) { + return text; + } + return `${text.slice(0, OUTPUT_REPLY_HARD_MAX_CHARS - 1).trimEnd()}…`; +} + +function stripNoReplyMarker(text: string): string { + return sanitizeAssistantText( + text + .split(NO_REPLY_MARKER) + .join(" ") + .replace(/[ \t]+\n/g, "\n") + .replace(/\n{3,}/g, "\n\n"), + ); +} + +/** Deterministic pre-checks before spending a model call. */ +export function decideOutputRouteDeterministic(text: string): OutputRoute | null { + const trimmed = sanitizeAssistantText(text); + if (!trimmed) { + return { + action: "suppress", + reason: "empty_text", + source: "deterministic", + }; + } + if (isNoReplyMarker(trimmed)) { + return { + action: "suppress", + reason: "no_reply_marker", + source: "deterministic", + }; + } + if (trimmed.includes(NO_REPLY_MARKER)) { + const stripped = stripNoReplyMarker(trimmed); + if (!stripped) { + return { + action: "suppress", + reason: "no_reply_marker_only_after_strip", + source: "deterministic", + }; + } + return { + action: "deliver", + text: enforceHardCap(stripped), + reason: "stripped_mixed_no_reply_marker", + source: "deterministic", + }; + } + return null; +} + +function finalizeRouterObject( + object: unknown, + originalText: string, + costUsd?: number, +): OutputRoute { + const parsed = outputRouteSchema.parse(object); + const reason = parsed.reason.trim() || "router"; + + if (parsed.action === "suppress") { + return { + action: "suppress", + reason, + source: "router", + ...(costUsd !== undefined ? { costUsd } : undefined), + }; + } + + const candidate = + normalizeRoutedText(parsed.text) ?? + (parsed.action === "deliver" ? originalText : undefined); + if (!candidate) { + return { + action: "deliver", + text: originalText, + reason: `router_missing_text:${reason}`, + source: "fallback", + ...(costUsd !== undefined ? { costUsd } : undefined), + }; + } + + // Never let a rewrite reintroduce pure silence unless suppress was chosen. + if (isNoReplyMarker(candidate)) { + return { + action: "suppress", + reason: `router_rewrote_to_no_reply:${reason}`, + source: "router", + ...(costUsd !== undefined ? { costUsd } : undefined), + }; + } + + return { + action: "deliver", + text: enforceHardCap( + candidate.includes(NO_REPLY_MARKER) + ? stripNoReplyMarker(candidate) + : candidate, + ), + reason, + source: "router", + ...(costUsd !== undefined ? { costUsd } : undefined), + }; +} + +/** Route one assistant message text before destination delivery. */ +export async function routeAssistantOutput(args: { + completeObject: CompleteObject; + context?: { + conversationId?: string; + runId?: string; + }; + fastModelId: string; + text: string; +}): Promise { + const originalText = sanitizeAssistantText(args.text); + const deterministic = decideOutputRouteDeterministic(originalText); + if (deterministic) { + return deterministic; + } + + const logContext: LogContext = { + messageConversationId: args.context?.conversationId, + runId: args.context?.runId, + modelId: args.fastModelId, + }; + + return withSpan( + "chat.route_assistant_output", + "chat.route_assistant_output", + logContext, + async () => { + setSpanAttributes({ + "app.ai.output_router.input_char_count": originalText.length, + "app.ai.output_router.soft_max_chars": OUTPUT_REPLY_SOFT_MAX_CHARS, + }); + + try { + const result = await args.completeObject({ + modelId: args.fastModelId, + schema: outputRouteSchema, + maxTokens: OUTPUT_ROUTER_MAX_TOKENS, + metadata: { + modelId: args.fastModelId, + conversationId: args.context?.conversationId ?? "", + runId: args.context?.runId ?? "", + }, + prompt: buildOutputRouterPrompt(originalText), + thinkingLevel: "low", + system: buildOutputRouterSystemPrompt(), + temperature: 0, + promptName: "junior.output_route", + }); + + const routed = finalizeRouterObject( + result.object, + originalText, + result.costUsd, + ); + setSpanAttributes({ + "app.ai.output_router.action": routed.action, + "app.ai.output_router.source": routed.source, + "app.ai.output_router.reason": routed.reason, + ...(routed.text + ? { "app.ai.output_router.output_char_count": routed.text.length } + : undefined), + }); + logInfo("ai.output_router.decided", { + "app.ai.output_router.action": routed.action, + "app.ai.output_router.source": routed.source, + "app.ai.output_router.reason": routed.reason, + "app.ai.output_router.input_char_count": originalText.length, + ...(routed.text + ? { "app.ai.output_router.output_char_count": routed.text.length } + : undefined), + }); + return routed; + } catch (error) { + logWarn("ai.output_router.failed", { + "exception.message": + error instanceof Error ? error.message : String(error), + }); + // Fail open: keep the original deliverable text rather than blocking the turn. + return { + action: "deliver", + text: originalText, + reason: "classifier_error_passthrough", + source: "fallback", + }; + } + }, + ); +} + +/** + * Replace assistant text content parts while preserving non-text parts. + * Mutates in place so agent-history object identity stays stable for delivery. + */ +export function applyAssistantOutputText( + message: AssistantMessage, + text: string, +): AssistantMessage { + const content = message.content ?? []; + let replaced = false; + const nextContent = content.map((part) => { + if (part.type !== "text") { + return part; + } + if (replaced) { + return { ...part, text: "" }; + } + replaced = true; + return { ...part, text }; + }); + if (!replaced) { + nextContent.unshift({ type: "text", text }); + } + message.content = nextContent.filter( + (part) => part.type !== "text" || part.text.length > 0, + ) as AssistantMessage["content"]; + return message; +} + +/** Route one completed assistant message when the experimental feature is on. */ +export async function routeAssistantMessage(args: { + completeObject: CompleteObject; + context?: { + conversationId?: string; + runId?: string; + }; + fastModelId: string; + message: AssistantMessage; +}): Promise< + | { kind: "deliver"; message: AssistantMessage; route: OutputRoute } + | { kind: "suppress"; route: OutputRoute } + | { kind: "skip" } +> { + const decision = decideReply(args.message); + if (decision.kind !== "deliver") { + return { kind: "skip" }; + } + + const route = await routeAssistantOutput({ + completeObject: args.completeObject, + context: args.context, + fastModelId: args.fastModelId, + text: decision.text, + }); + + if (route.action === "suppress") { + return { kind: "suppress", route }; + } + + const nextText = route.text ?? decision.text; + if (nextText !== decision.text) { + applyAssistantOutputText(args.message, nextText); + } else if (sanitizeAssistantText(extractAssistantText(args.message)) !== nextText) { + applyAssistantOutputText(args.message, nextText); + } + + return { kind: "deliver", message: args.message, route }; +} diff --git a/packages/junior/tests/fixtures/experimental-setup.ts b/packages/junior/tests/fixtures/experimental-setup.ts index 5053d75cb3..dcd60cf9ad 100644 --- a/packages/junior/tests/fixtures/experimental-setup.ts +++ b/packages/junior/tests/fixtures/experimental-setup.ts @@ -6,6 +6,7 @@ import { setExperimentalFeatures } from "@/chat/experimental"; * exercises the real wiring path without an env flag. */ export const SUITE_EXPERIMENTAL = { + "output-router": false, "passive-routing": true, subagents: true, } as const; diff --git a/packages/junior/tests/unit/experimental.test.ts b/packages/junior/tests/unit/experimental.test.ts index d79b7b758e..95fb9e3011 100644 --- a/packages/junior/tests/unit/experimental.test.ts +++ b/packages/junior/tests/unit/experimental.test.ts @@ -11,18 +11,29 @@ afterEach(() => { describe("experimental features", () => { it("defaults experimental features off", () => { setExperimentalFeatures(undefined); + expect(isExperimentalFeatureEnabled("output-router")).toBe(false); expect(isExperimentalFeatureEnabled("passive-routing")).toBe(false); expect(isExperimentalFeatureEnabled("subagents")).toBe(false); }); it("enables features from createApp-style config", () => { - setExperimentalFeatures({ "passive-routing": true, subagents: true }); + setExperimentalFeatures({ + "output-router": true, + "passive-routing": true, + subagents: true, + }); + expect(isExperimentalFeatureEnabled("output-router")).toBe(true); expect(isExperimentalFeatureEnabled("passive-routing")).toBe(true); expect(isExperimentalFeatureEnabled("subagents")).toBe(true); }); it("treats explicit false as disabled", () => { - setExperimentalFeatures({ "passive-routing": false, subagents: false }); + setExperimentalFeatures({ + "output-router": false, + "passive-routing": false, + subagents: false, + }); + expect(isExperimentalFeatureEnabled("output-router")).toBe(false); expect(isExperimentalFeatureEnabled("passive-routing")).toBe(false); expect(isExperimentalFeatureEnabled("subagents")).toBe(false); }); diff --git a/packages/junior/tests/unit/services/output-router.test.ts b/packages/junior/tests/unit/services/output-router.test.ts new file mode 100644 index 0000000000..fb9e66faeb --- /dev/null +++ b/packages/junior/tests/unit/services/output-router.test.ts @@ -0,0 +1,220 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { AssistantMessage } from "@earendil-works/pi-ai"; +import { NO_REPLY_MARKER } from "@/chat/no-reply"; +import { + applyAssistantOutputText, + decideOutputRouteDeterministic, + OUTPUT_REPLY_HARD_MAX_CHARS, + routeAssistantMessage, + routeAssistantOutput, +} from "@/chat/services/output-router"; + +const mocks = vi.hoisted(() => ({ + logInfo: vi.fn(), + logWarn: vi.fn(), +})); + +vi.mock("@/chat/logging", async (importOriginal) => ({ + ...(await importOriginal()), + logInfo: mocks.logInfo, + logWarn: mocks.logWarn, +})); + +function assistant(text: string, withToolCall = false): AssistantMessage { + return { + role: "assistant", + content: [ + { type: "text", text }, + ...(withToolCall + ? [ + { + type: "toolCall" as const, + id: "call-1", + name: "bash", + arguments: {}, + }, + ] + : []), + ], + api: "responses", + provider: "openai", + model: "test-model", + usage: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 0, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + stopReason: "stop", + timestamp: 1, + }; +} + +describe("output router", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("suppresses exact no-reply markers without a model call", async () => { + const completeObject = vi.fn(); + await expect( + routeAssistantOutput({ + completeObject, + fastModelId: "openai/gpt-5.6-luna", + text: NO_REPLY_MARKER, + }), + ).resolves.toEqual({ + action: "suppress", + reason: "no_reply_marker", + source: "deterministic", + }); + expect(completeObject).not.toHaveBeenCalled(); + }); + + it("strips mixed no-reply markers deterministically", () => { + expect( + decideOutputRouteDeterministic( + `shipped it ${NO_REPLY_MARKER}\nmore detail`, + ), + ).toEqual({ + action: "deliver", + text: "shipped it\nmore detail", + reason: "stripped_mixed_no_reply_marker", + source: "deterministic", + }); + }); + + it("routes long answers through the fast model", async () => { + const completeObject = vi.fn(async () => ({ + costUsd: 0.0004, + object: { + action: "rewrite", + text: "Short answer with the outcome.", + reason: "too long", + }, + })); + + const route = await routeAssistantOutput({ + completeObject, + fastModelId: "openai/gpt-5.6-luna", + text: "A".repeat(900), + }); + + expect(route).toEqual({ + action: "deliver", + text: "Short answer with the outcome.", + reason: "too long", + source: "router", + costUsd: 0.0004, + }); + expect(completeObject).toHaveBeenCalledWith( + expect.objectContaining({ + modelId: "openai/gpt-5.6-luna", + promptName: "junior.output_route", + temperature: 0, + thinkingLevel: "low", + system: expect.stringContaining("final output router"), + }), + ); + }); + + it("fails open to the original text when the classifier errors", async () => { + const completeObject = vi.fn(async () => { + throw new Error("boom"); + }); + + await expect( + routeAssistantOutput({ + completeObject, + fastModelId: "openai/gpt-5.6-luna", + text: "Keep this answer.", + }), + ).resolves.toEqual({ + action: "deliver", + text: "Keep this answer.", + reason: "classifier_error_passthrough", + source: "fallback", + }); + expect(mocks.logWarn).toHaveBeenCalledWith( + "ai.output_router.failed", + expect.objectContaining({ "exception.message": "boom" }), + ); + }); + + it("hard-caps oversized routed text", async () => { + const completeObject = vi.fn(async () => ({ + object: { + action: "rewrite", + text: "B".repeat(OUTPUT_REPLY_HARD_MAX_CHARS + 50), + reason: "still long", + }, + })); + + const route = await routeAssistantOutput({ + completeObject, + fastModelId: "openai/gpt-5.6-luna", + text: "A".repeat(900), + }); + + expect(route.action).toBe("deliver"); + expect(route.text?.length).toBe(OUTPUT_REPLY_HARD_MAX_CHARS); + expect(route.text?.endsWith("…")).toBe(true); + }); + + it("rewrites the assistant message in place before delivery", async () => { + const message = assistant("A".repeat(900)); + const completeObject = vi.fn(async () => ({ + object: { + action: "rewrite", + text: "Condensed reply.", + reason: "too long", + }, + })); + + const routed = await routeAssistantMessage({ + completeObject, + fastModelId: "openai/gpt-5.6-luna", + message, + }); + + expect(routed).toMatchObject({ + kind: "deliver", + route: { action: "deliver", text: "Condensed reply." }, + }); + expect(message.content).toEqual([{ type: "text", text: "Condensed reply." }]); + }); + + it("skips tool-bearing assistant messages", async () => { + const completeObject = vi.fn(); + await expect( + routeAssistantMessage({ + completeObject, + fastModelId: "openai/gpt-5.6-luna", + message: assistant("working", true), + }), + ).resolves.toEqual({ kind: "skip" }); + expect(completeObject).not.toHaveBeenCalled(); + }); + + it("applies rewritten text while preserving non-text parts", () => { + const message = assistant("old"); + message.content.push({ + type: "toolCall", + id: "call-2", + name: "bash", + arguments: {}, + }); + applyAssistantOutputText(message, "new"); + expect(message.content).toEqual([ + { type: "text", text: "new" }, + { + type: "toolCall", + id: "call-2", + name: "bash", + arguments: {}, + }, + ]); + }); +}); From c4a348e6d2409e5e1310f11e33e37d7f4f261918 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Sat, 29 Aug 2026 00:27:55 +0000 Subject: [PATCH 02/14] refactor(chat): keep original agent text, simplify output router Prepare only the visible reply with the fast model. Leave the original agent message in history. Simplify the prompt and API surface. Co-Authored-By: David Cramer --- TELEMETRY.md | 2 +- .../content/docs/reference/config-and-env.md | 14 +- packages/junior/src/chat/README.md | 8 +- packages/junior/src/chat/agent/index.ts | 14 +- packages/junior/src/chat/agent/types.ts | 8 +- packages/junior/src/chat/local/runner.ts | 4 +- .../junior/src/chat/providers/slack/resume.ts | 4 +- .../junior/src/chat/providers/slack/turn.ts | 4 +- .../junior/src/chat/services/output-router.ts | 337 ++++++++---------- .../chat/task-execution/conversation-turn.ts | 4 +- .../tests/unit/services/output-router.test.ts | 102 ++---- 11 files changed, 216 insertions(+), 285 deletions(-) diff --git a/TELEMETRY.md b/TELEMETRY.md index f32899f06e..18fdb2dc05 100644 --- a/TELEMETRY.md +++ b/TELEMETRY.md @@ -182,7 +182,7 @@ consecutive action-review denials. The agent still receives a normal tool rejection that says not to keep retrying. Spans: `ai.generate_assistant_reply`, `ai.chat_completion`, -`chat.route_thinking`, `chat.route_assistant_output`, `gen_ai.invoke_agent`, +`chat.route_thinking`, `chat.prepare_assistant_reply`, `gen_ai.invoke_agent`, `gen_ai.chat` Attributes: `gen_ai.operation.name`, `gen_ai.request.model`, diff --git a/packages/docs/src/content/docs/reference/config-and-env.md b/packages/docs/src/content/docs/reference/config-and-env.md index 39931b313f..4c77fd15dd 100644 --- a/packages/docs/src/content/docs/reference/config-and-env.md +++ b/packages/docs/src/content/docs/reference/config-and-env.md @@ -139,8 +139,9 @@ import { createApp } from "@sentry/junior"; const app = await createApp({ experimental: { - // Fast-model pass over every completed assistant message before delivery. - // Handles [[NO_REPLY]] cleanup and long-reply compression. Off by default. + // Prepare the visible reply with the fast model before delivery. + // Can hide [[NO_REPLY]] and shorten long replies. Off by default. + // Original agent text stays in history; only the visible reply may change. "output-router": true, // Reply to non-mention messages in Slack threads Junior already joined. // Off by default. Without this, Junior only replies to explicit @mentions @@ -172,10 +173,11 @@ request reaches the deployment request limit. Run `pnpm acp:local` in this repository for a loopback test with the official ACP SDK client. ACP remains a pre-stable surface. -`output-router` runs the fast model (`AI_FAST_MODEL`) on each completed -tool-free assistant message before destination delivery. It suppresses pure -`[[NO_REPLY]]` output, strips mixed markers, and can rewrite overly long -replies. Leave it unset unless you are testing that path. +`output-router` uses the fast model (`AI_FAST_MODEL`) to prepare the visible +reply for each completed tool-free assistant message. Pure `[[NO_REPLY]]` stays +silent, mixed markers are removed, and long replies can be shortened. The +original agent text remains in conversation history. Leave it unset unless you +are testing that path. `passive-routing` turns on replies to non-mention messages in threads Junior already joined. Leave it unset in production unless you are testing that path. diff --git a/packages/junior/src/chat/README.md b/packages/junior/src/chat/README.md index dfa90fba0a..a17dae647c 100644 --- a/packages/junior/src/chat/README.md +++ b/packages/junior/src/chat/README.md @@ -20,10 +20,10 @@ file. 6. `agent/` emits every completed, tool-free visible assistant message through one awaited delivery port with the completed Pi message that produced it; provider adapters deliver, then commit that agent message before the visible - reply in one transaction. When experimental `output-router` is enabled, the - fast model routes each message before delivery (silence marker handling and - length rewrite). Tool-bearing assistant text remains internal to the agent - loop. + reply in one transaction. When experimental `output-router` is enabled, a + fast-model pass may change only the visible reply text. The original agent + message stays in history. Tool-bearing assistant text remains internal to the + agent loop. 7. The completed run result supplies diagnostics and artifacts; successful delivery or intentional no-reply completion commits the durable turn outcome. diff --git a/packages/junior/src/chat/agent/index.ts b/packages/junior/src/chat/agent/index.ts index 61458e116e..230cb0d4f0 100644 --- a/packages/junior/src/chat/agent/index.ts +++ b/packages/junior/src/chat/agent/index.ts @@ -78,7 +78,7 @@ import { isTurnInputCommitLostError } from "@/chat/runtime/turn"; import type { AgentRunOutcome } from "@/chat/runtime/agent-run-outcome"; import { buildTurnResult } from "@/chat/services/turn-result"; import { decideReply } from "@/chat/services/assistant-reply"; -import { routeAssistantMessage } from "@/chat/services/output-router"; +import { prepareAssistantMessage } from "@/chat/services/output-router"; import { isExperimentalFeatureEnabled } from "@/chat/experimental"; import { findProviderError, @@ -1088,9 +1088,11 @@ async function executeAgentRunInPrivacyContext( return; } - let deliverable = message; + // Keep the original agent message for history. Only the visible reply text + // may change when output-router is on. + let visibleText: string | undefined; if (isExperimentalFeatureEnabled("output-router")) { - const routed = await routeAssistantMessage({ + const prepared = await prepareAssistantMessage({ completeObject, context: { conversationId, @@ -1099,10 +1101,10 @@ async function executeAgentRunInPrivacyContext( fastModelId: botConfig.fastModelId, message, }); - if (routed.kind === "skip" || routed.kind === "suppress") { + if (prepared.kind === "skip" || prepared.kind === "silent") { return; } - deliverable = routed.message; + visibleText = prepared.text; } else { const decision = decideReply(message); if (decision.kind !== "deliver") { @@ -1111,7 +1113,7 @@ async function executeAgentRunInPrivacyContext( } try { - await delivery(deliverable); + await delivery(message, visibleText); acceptedToolFreeAssistant = true; } catch (error) { assistantMessageDeliveryError = new AssistantMessageDeliveryError( diff --git a/packages/junior/src/chat/agent/types.ts b/packages/junior/src/chat/agent/types.ts index a2fc233911..29e4e9b825 100644 --- a/packages/junior/src/chat/agent/types.ts +++ b/packages/junior/src/chat/agent/types.ts @@ -124,6 +124,9 @@ export type AgentRunState = { /** * Delivers completed tool-free assistant messages in model order. * + * `message` is the original agent message for history. `text` is the + * destination-visible reply when it differs from the agent message text. + * * The runner must commit the preceding agent boundary before invoking this * port; the accepted reply transaction appends only this message. * @@ -131,7 +134,10 @@ export type AgentRunState = { * implementations after the core Turn lifecycle stores each completed * assistant Message. */ -export type Delivery = (message: AssistantMessage) => void | Promise; +export type Delivery = ( + message: AssistantMessage, + text?: string, +) => void | Promise; /** Resume the agent turn after a transient or ambiguous delivery failure. */ export class RetryableDeliveryError extends Error { diff --git a/packages/junior/src/chat/local/runner.ts b/packages/junior/src/chat/local/runner.ts index f91b23f3b4..58870e82ad 100644 --- a/packages/junior/src/chat/local/runner.ts +++ b/packages/junior/src/chat/local/runner.ts @@ -260,10 +260,12 @@ async function runLocalAgentTurnInContext( /** Print and record one completed assistant message in local conversation order. */ const deliverAssistantMessage = async ( reply: AssistantMessage | string, + visibleText?: string, ): Promise => { const message = typeof reply === "string" ? undefined : reply; const text = - typeof reply === "string" ? reply : getAssistantReplyText(reply); + visibleText ?? + (typeof reply === "string" ? reply : getAssistantReplyText(reply)); if (!text?.trim()) { return; } diff --git a/packages/junior/src/chat/providers/slack/resume.ts b/packages/junior/src/chat/providers/slack/resume.ts index c75735c65c..4ca5d25078 100644 --- a/packages/junior/src/chat/providers/slack/resume.ts +++ b/packages/junior/src/chat/providers/slack/resume.ts @@ -554,10 +554,12 @@ async function resumeSlackTurnInContext( /** Post and record one completed assistant message for the resumed turn. */ const deliverAssistantMessage = async ( reply: AssistantMessage | string, + visibleText?: string, ): Promise => { const message = typeof reply === "string" ? undefined : reply; const text = - typeof reply === "string" ? reply : getAssistantReplyText(reply); + visibleText ?? + (typeof reply === "string" ? reply : getAssistantReplyText(reply)); if (!text?.trim()) { return; } diff --git a/packages/junior/src/chat/providers/slack/turn.ts b/packages/junior/src/chat/providers/slack/turn.ts index 84602c925b..00d94ec7eb 100644 --- a/packages/junior/src/chat/providers/slack/turn.ts +++ b/packages/junior/src/chat/providers/slack/turn.ts @@ -810,11 +810,13 @@ export function createSlackTurn(deps: SlackTurnDeps) { /** Post and record one completed assistant message in the active thread. */ const deliverAssistantMessage = async ( reply: AssistantMessage | string, + visibleText?: string, terminalDispatchOutcome?: "blocked" | "failed", ): Promise => { const agentMessage = typeof reply === "string" ? undefined : reply; const text = - typeof reply === "string" ? reply : getAssistantReplyText(reply); + visibleText ?? + (typeof reply === "string" ? reply : getAssistantReplyText(reply)); if (!text?.trim()) { return; } diff --git a/packages/junior/src/chat/services/output-router.ts b/packages/junior/src/chat/services/output-router.ts index 608d49bb6b..c07b577140 100644 --- a/packages/junior/src/chat/services/output-router.ts +++ b/packages/junior/src/chat/services/output-router.ts @@ -12,39 +12,44 @@ import { decideReply, sanitizeAssistantText, } from "@/chat/services/assistant-reply"; -import { extractAssistantText } from "@/chat/pi/transcript"; -/** - * Destination reply length budget used by the optional output router. - * Matches the Slack output prompt target (~1–5 sentences). - */ +/** Soft length target for visible replies. */ export const OUTPUT_REPLY_SOFT_MAX_CHARS = 800; -/** Hard cap after rewrite; longer routed text is truncated deterministically. */ +/** Absolute max for a rewritten visible reply. */ export const OUTPUT_REPLY_HARD_MAX_CHARS = 1_200; -const OUTPUT_ROUTER_MAX_TOKENS = 1_600; + +const OUTPUT_ROUTER_MAX_TOKENS = 1_200; const OUTPUT_ROUTER_PROMPT_MAX_CHARS = 12_000; -const outputRouteSchema = z +/** + * Model output is intentionally small: + * - text=null → no visible reply + * - text=string → that string is the visible reply + */ +const preparedReplySchema = z .object({ - action: z.enum(["deliver", "suppress", "rewrite"]), - text: z.string().optional(), + text: z.string().nullable(), reason: z.string().min(1), }) .strict(); -export type OutputRouteAction = z.infer["action"]; - -export type OutputRoute = { - action: "deliver" | "suppress"; - costUsd?: number; - reason: string; - source: "deterministic" | "router" | "fallback"; - text?: string; -}; +export type PreparedAssistantReply = + | { + kind: "silent"; + costUsd?: number; + reason: string; + } + | { + kind: "reply"; + costUsd?: number; + reason: string; + /** Visible reply text. May differ from the original agent text. */ + text: string; + }; type CompleteObject = (args: { modelId: string; - schema: typeof outputRouteSchema; + schema: typeof preparedReplySchema; maxTokens: number; metadata: Record; prompt: string; @@ -54,55 +59,40 @@ type CompleteObject = (args: { promptName?: string; }) => Promise<{ costUsd?: number; object: unknown }>; -function buildOutputRouterSystemPrompt(): string { - // Tight output guardrail. Pattern: OpenAI cookbook "output guardrails" - // (validate LLM output before delivery) + structured JSON decisions. - // https://developers.openai.com/cookbook/examples/how_to_use_guardrails/ +/** + * Prompt design notes: + * - short imperative system instructions first + * - one clear output contract (structured JSON) + * - rules as a short checklist + * - no roleplay / no extra context + * Related: OpenAI structured outputs + short instruction prompts; + * Anthropic: put the task first, be direct, prefer positive rules. + */ +function buildSystemPrompt(): string { return [ - "You are Junior's final output router.", - "Input is one completed assistant message. No other context is available.", - "Decide the destination-visible reply before delivery.", + "Edit one assistant message into the final user-visible reply.", + "You receive only that message. No other context.", "", - "Actions:", - "- suppress: no visible reply should be delivered", - "- deliver: keep the message text as-is", - "- rewrite: replace the message with shorter destination-visible text", + "Return JSON:", + "- text: the visible reply, or null for no visible reply", + "- reason: one short sentence", "", "Rules:", - `1. suppress when the whole message is intentional silence (exact ${NO_REPLY_MARKER}, or equivalent pure no-reply protocol text with no other answer).`, - `2. if ${NO_REPLY_MARKER} appears mixed with real answer text, rewrite: strip the marker and keep the answer.`, - "3. deliver short complete answers unchanged.", - `4. rewrite long answers that exceed ~${OUTPUT_REPLY_SOFT_MAX_CHARS} characters unless the user clearly asked for full detail, a long dump, or a large code/config block that must stay intact.`, - "5. rewrites must stay faithful: keep the outcome, decisive evidence, blockers, links, and required next actions. Do not invent facts.", - `6. rewritten text should usually be 1–5 sentences and under ${OUTPUT_REPLY_SOFT_MAX_CHARS} characters. Prefer a short summary plus a pointer (for example a canvas/doc link) when detail is too long.`, - "7. preserve fenced code only when it is essential and still short enough; otherwise summarize and point to where the full content lives.", - "8. never add preamble, meta commentary, or process narration.", - "", - "Return JSON only with action, reason, and text.", - "text is required for deliver and rewrite; omit text for suppress.", - "reason is one short sentence.", + `- Set text to null only when the message is empty or only ${NO_REPLY_MARKER}.`, + `- If ${NO_REPLY_MARKER} appears with real answer text, remove the marker and keep the answer.`, + `- Keep short clear replies as-is (about ${OUTPUT_REPLY_SOFT_MAX_CHARS} characters or less).`, + `- If the reply is too long, shorten it. Keep the answer, key facts, links, and next steps. Do not add facts.`, + "- Prefer 1-5 short sentences when shortening.", + "- Do not add a preface or meta commentary.", ].join("\n"); } -function buildOutputRouterPrompt(text: string): string { +function buildUserPrompt(text: string): string { const body = text.length <= OUTPUT_ROUTER_PROMPT_MAX_CHARS ? text : `${text.slice(0, OUTPUT_ROUTER_PROMPT_MAX_CHARS)}\n…[truncated]…`; - return ["", body, ""].join("\n"); -} - -function normalizeRoutedText(text: string | undefined): string | undefined { - if (text === undefined) return undefined; - const normalized = sanitizeAssistantText(text); - return normalized || undefined; -} - -function enforceHardCap(text: string): string { - if (text.length <= OUTPUT_REPLY_HARD_MAX_CHARS) { - return text; - } - return `${text.slice(0, OUTPUT_REPLY_HARD_MAX_CHARS - 1).trimEnd()}…`; + return body; } function stripNoReplyMarker(text: string): string { @@ -115,97 +105,89 @@ function stripNoReplyMarker(text: string): string { ); } -/** Deterministic pre-checks before spending a model call. */ -export function decideOutputRouteDeterministic(text: string): OutputRoute | null { +function capVisibleText(text: string): string { + if (text.length <= OUTPUT_REPLY_HARD_MAX_CHARS) { + return text; + } + return `${text.slice(0, OUTPUT_REPLY_HARD_MAX_CHARS - 1).trimEnd()}…`; +} + +function silent(reason: string, costUsd?: number): PreparedAssistantReply { + return { + kind: "silent", + reason, + ...(costUsd !== undefined ? { costUsd } : undefined), + }; +} + +function reply( + text: string, + reason: string, + costUsd?: number, +): PreparedAssistantReply { + return { + kind: "reply", + text, + reason, + ...(costUsd !== undefined ? { costUsd } : undefined), + }; +} + +/** Cheap local checks before calling the model. */ +export function prepareAssistantReplyLocal( + text: string, +): PreparedAssistantReply | null { const trimmed = sanitizeAssistantText(text); if (!trimmed) { - return { - action: "suppress", - reason: "empty_text", - source: "deterministic", - }; + return silent("empty"); } if (isNoReplyMarker(trimmed)) { - return { - action: "suppress", - reason: "no_reply_marker", - source: "deterministic", - }; + return silent("no_reply"); } if (trimmed.includes(NO_REPLY_MARKER)) { const stripped = stripNoReplyMarker(trimmed); if (!stripped) { - return { - action: "suppress", - reason: "no_reply_marker_only_after_strip", - source: "deterministic", - }; + return silent("no_reply"); } - return { - action: "deliver", - text: enforceHardCap(stripped), - reason: "stripped_mixed_no_reply_marker", - source: "deterministic", - }; + return reply(capVisibleText(stripped), "removed_no_reply_marker"); } return null; } -function finalizeRouterObject( +function finalizeModelResult( object: unknown, originalText: string, costUsd?: number, -): OutputRoute { - const parsed = outputRouteSchema.parse(object); - const reason = parsed.reason.trim() || "router"; +): PreparedAssistantReply { + const parsed = preparedReplySchema.parse(object); + const reason = parsed.reason.trim() || "prepared"; - if (parsed.action === "suppress") { - return { - action: "suppress", - reason, - source: "router", - ...(costUsd !== undefined ? { costUsd } : undefined), - }; + if (parsed.text === null) { + return silent(reason, costUsd); } - const candidate = - normalizeRoutedText(parsed.text) ?? - (parsed.action === "deliver" ? originalText : undefined); - if (!candidate) { - return { - action: "deliver", - text: originalText, - reason: `router_missing_text:${reason}`, - source: "fallback", - ...(costUsd !== undefined ? { costUsd } : undefined), - }; + let text = sanitizeAssistantText(parsed.text); + if (!text) { + // Model returned blank text. Keep the original visible reply. + return reply(originalText, `empty_model_text:${reason}`, costUsd); } - - // Never let a rewrite reintroduce pure silence unless suppress was chosen. - if (isNoReplyMarker(candidate)) { - return { - action: "suppress", - reason: `router_rewrote_to_no_reply:${reason}`, - source: "router", - ...(costUsd !== undefined ? { costUsd } : undefined), - }; + if (isNoReplyMarker(text)) { + return silent(`model_no_reply:${reason}`, costUsd); } - - return { - action: "deliver", - text: enforceHardCap( - candidate.includes(NO_REPLY_MARKER) - ? stripNoReplyMarker(candidate) - : candidate, - ), - reason, - source: "router", - ...(costUsd !== undefined ? { costUsd } : undefined), - }; + if (text.includes(NO_REPLY_MARKER)) { + text = stripNoReplyMarker(text); + if (!text) { + return silent(`model_no_reply:${reason}`, costUsd); + } + } + return reply(capVisibleText(text), reason, costUsd); } -/** Route one assistant message text before destination delivery. */ -export async function routeAssistantOutput(args: { +/** + * Prepare the visible reply for one assistant message. + * Does not change the original agent message text. + */ +export async function prepareAssistantReply(args: { completeObject: CompleteObject; context?: { conversationId?: string; @@ -213,11 +195,11 @@ export async function routeAssistantOutput(args: { }; fastModelId: string; text: string; -}): Promise { +}): Promise { const originalText = sanitizeAssistantText(args.text); - const deterministic = decideOutputRouteDeterministic(originalText); - if (deterministic) { - return deterministic; + const local = prepareAssistantReplyLocal(originalText); + if (local) { + return local; } const logContext: LogContext = { @@ -227,8 +209,8 @@ export async function routeAssistantOutput(args: { }; return withSpan( - "chat.route_assistant_output", - "chat.route_assistant_output", + "chat.prepare_assistant_reply", + "chat.prepare_assistant_reply", logContext, async () => { setSpanAttributes({ @@ -239,91 +221,58 @@ export async function routeAssistantOutput(args: { try { const result = await args.completeObject({ modelId: args.fastModelId, - schema: outputRouteSchema, + schema: preparedReplySchema, maxTokens: OUTPUT_ROUTER_MAX_TOKENS, metadata: { modelId: args.fastModelId, conversationId: args.context?.conversationId ?? "", runId: args.context?.runId ?? "", }, - prompt: buildOutputRouterPrompt(originalText), + prompt: buildUserPrompt(originalText), thinkingLevel: "low", - system: buildOutputRouterSystemPrompt(), + system: buildSystemPrompt(), temperature: 0, - promptName: "junior.output_route", + promptName: "junior.prepare_assistant_reply", }); - const routed = finalizeRouterObject( + const prepared = finalizeModelResult( result.object, originalText, result.costUsd, ); setSpanAttributes({ - "app.ai.output_router.action": routed.action, - "app.ai.output_router.source": routed.source, - "app.ai.output_router.reason": routed.reason, - ...(routed.text - ? { "app.ai.output_router.output_char_count": routed.text.length } + "app.ai.output_router.kind": prepared.kind, + "app.ai.output_router.reason": prepared.reason, + ...(prepared.kind === "reply" + ? { "app.ai.output_router.output_char_count": prepared.text.length } : undefined), }); logInfo("ai.output_router.decided", { - "app.ai.output_router.action": routed.action, - "app.ai.output_router.source": routed.source, - "app.ai.output_router.reason": routed.reason, + "app.ai.output_router.kind": prepared.kind, + "app.ai.output_router.reason": prepared.reason, "app.ai.output_router.input_char_count": originalText.length, - ...(routed.text - ? { "app.ai.output_router.output_char_count": routed.text.length } + ...(prepared.kind === "reply" + ? { "app.ai.output_router.output_char_count": prepared.text.length } : undefined), }); - return routed; + return prepared; } catch (error) { logWarn("ai.output_router.failed", { "exception.message": error instanceof Error ? error.message : String(error), }); - // Fail open: keep the original deliverable text rather than blocking the turn. - return { - action: "deliver", - text: originalText, - reason: "classifier_error_passthrough", - source: "fallback", - }; + // On failure, show the original text rather than dropping the reply. + return reply(originalText, "prepare_failed"); } }, ); } /** - * Replace assistant text content parts while preserving non-text parts. - * Mutates in place so agent-history object identity stays stable for delivery. + * Decide the visible reply for a completed assistant message. + * Returns silent/skip without changing the agent message. */ -export function applyAssistantOutputText( - message: AssistantMessage, - text: string, -): AssistantMessage { - const content = message.content ?? []; - let replaced = false; - const nextContent = content.map((part) => { - if (part.type !== "text") { - return part; - } - if (replaced) { - return { ...part, text: "" }; - } - replaced = true; - return { ...part, text }; - }); - if (!replaced) { - nextContent.unshift({ type: "text", text }); - } - message.content = nextContent.filter( - (part) => part.type !== "text" || part.text.length > 0, - ) as AssistantMessage["content"]; - return message; -} - -/** Route one completed assistant message when the experimental feature is on. */ -export async function routeAssistantMessage(args: { +export async function prepareAssistantMessage(args: { completeObject: CompleteObject; context?: { conversationId?: string; @@ -332,32 +281,24 @@ export async function routeAssistantMessage(args: { fastModelId: string; message: AssistantMessage; }): Promise< - | { kind: "deliver"; message: AssistantMessage; route: OutputRoute } - | { kind: "suppress"; route: OutputRoute } | { kind: "skip" } + | { kind: "silent"; prepared: PreparedAssistantReply } + | { kind: "reply"; text: string; prepared: PreparedAssistantReply } > { const decision = decideReply(args.message); if (decision.kind !== "deliver") { return { kind: "skip" }; } - const route = await routeAssistantOutput({ + const prepared = await prepareAssistantReply({ completeObject: args.completeObject, context: args.context, fastModelId: args.fastModelId, text: decision.text, }); - if (route.action === "suppress") { - return { kind: "suppress", route }; - } - - const nextText = route.text ?? decision.text; - if (nextText !== decision.text) { - applyAssistantOutputText(args.message, nextText); - } else if (sanitizeAssistantText(extractAssistantText(args.message)) !== nextText) { - applyAssistantOutputText(args.message, nextText); + if (prepared.kind === "silent") { + return { kind: "silent", prepared }; } - - return { kind: "deliver", message: args.message, route }; + return { kind: "reply", text: prepared.text, prepared }; } diff --git a/packages/junior/src/chat/task-execution/conversation-turn.ts b/packages/junior/src/chat/task-execution/conversation-turn.ts index e72bffa49f..bd75acf796 100644 --- a/packages/junior/src/chat/task-execution/conversation-turn.ts +++ b/packages/junior/src/chat/task-execution/conversation-turn.ts @@ -353,10 +353,12 @@ export function createConversationTurnWorker( const deliverAssistantMessage = async ( value: AssistantMessage | string, + visibleText?: string, ): Promise => { const agentMessage = typeof value === "string" ? undefined : value; const replyText = - typeof value === "string" ? value : getAssistantReplyText(value); + visibleText ?? + (typeof value === "string" ? value : getAssistantReplyText(value)); if (!replyText?.trim()) { return; } diff --git a/packages/junior/tests/unit/services/output-router.test.ts b/packages/junior/tests/unit/services/output-router.test.ts index fb9e66faeb..bd60097736 100644 --- a/packages/junior/tests/unit/services/output-router.test.ts +++ b/packages/junior/tests/unit/services/output-router.test.ts @@ -2,11 +2,10 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import type { AssistantMessage } from "@earendil-works/pi-ai"; import { NO_REPLY_MARKER } from "@/chat/no-reply"; import { - applyAssistantOutputText, - decideOutputRouteDeterministic, OUTPUT_REPLY_HARD_MAX_CHARS, - routeAssistantMessage, - routeAssistantOutput, + prepareAssistantMessage, + prepareAssistantReply, + prepareAssistantReplyLocal, } from "@/chat/services/output-router"; const mocks = vi.hoisted(() => ({ @@ -52,90 +51,83 @@ function assistant(text: string, withToolCall = false): AssistantMessage { }; } -describe("output router", () => { +describe("prepare assistant reply", () => { beforeEach(() => { vi.clearAllMocks(); }); - it("suppresses exact no-reply markers without a model call", async () => { + it("stays silent for exact no-reply markers without a model call", async () => { const completeObject = vi.fn(); await expect( - routeAssistantOutput({ + prepareAssistantReply({ completeObject, fastModelId: "openai/gpt-5.6-luna", text: NO_REPLY_MARKER, }), ).resolves.toEqual({ - action: "suppress", - reason: "no_reply_marker", - source: "deterministic", + kind: "silent", + reason: "no_reply", }); expect(completeObject).not.toHaveBeenCalled(); }); - it("strips mixed no-reply markers deterministically", () => { + it("removes mixed no-reply markers locally", () => { expect( - decideOutputRouteDeterministic( - `shipped it ${NO_REPLY_MARKER}\nmore detail`, - ), + prepareAssistantReplyLocal(`shipped it ${NO_REPLY_MARKER}\nmore detail`), ).toEqual({ - action: "deliver", + kind: "reply", text: "shipped it\nmore detail", - reason: "stripped_mixed_no_reply_marker", - source: "deterministic", + reason: "removed_no_reply_marker", }); }); - it("routes long answers through the fast model", async () => { + it("asks the fast model to shorten long replies", async () => { const completeObject = vi.fn(async () => ({ costUsd: 0.0004, object: { - action: "rewrite", text: "Short answer with the outcome.", reason: "too long", }, })); - const route = await routeAssistantOutput({ + const prepared = await prepareAssistantReply({ completeObject, fastModelId: "openai/gpt-5.6-luna", text: "A".repeat(900), }); - expect(route).toEqual({ - action: "deliver", + expect(prepared).toEqual({ + kind: "reply", text: "Short answer with the outcome.", reason: "too long", - source: "router", costUsd: 0.0004, }); expect(completeObject).toHaveBeenCalledWith( expect.objectContaining({ modelId: "openai/gpt-5.6-luna", - promptName: "junior.output_route", + promptName: "junior.prepare_assistant_reply", temperature: 0, thinkingLevel: "low", - system: expect.stringContaining("final output router"), + system: expect.stringContaining("Edit one assistant message"), }), ); }); - it("fails open to the original text when the classifier errors", async () => { + it("keeps the original text when the model call fails", async () => { const completeObject = vi.fn(async () => { throw new Error("boom"); }); await expect( - routeAssistantOutput({ + prepareAssistantReply({ completeObject, fastModelId: "openai/gpt-5.6-luna", text: "Keep this answer.", }), ).resolves.toEqual({ - action: "deliver", + kind: "reply", text: "Keep this answer.", - reason: "classifier_error_passthrough", - source: "fallback", + reason: "prepare_failed", }); expect(mocks.logWarn).toHaveBeenCalledWith( "ai.output_router.failed", @@ -143,53 +135,53 @@ describe("output router", () => { ); }); - it("hard-caps oversized routed text", async () => { + it("caps oversized model text", async () => { const completeObject = vi.fn(async () => ({ object: { - action: "rewrite", text: "B".repeat(OUTPUT_REPLY_HARD_MAX_CHARS + 50), reason: "still long", }, })); - const route = await routeAssistantOutput({ + const prepared = await prepareAssistantReply({ completeObject, fastModelId: "openai/gpt-5.6-luna", text: "A".repeat(900), }); - expect(route.action).toBe("deliver"); - expect(route.text?.length).toBe(OUTPUT_REPLY_HARD_MAX_CHARS); - expect(route.text?.endsWith("…")).toBe(true); + expect(prepared.kind).toBe("reply"); + if (prepared.kind !== "reply") return; + expect(prepared.text.length).toBe(OUTPUT_REPLY_HARD_MAX_CHARS); + expect(prepared.text.endsWith("…")).toBe(true); }); - it("rewrites the assistant message in place before delivery", async () => { - const message = assistant("A".repeat(900)); + it("returns visible text without changing the agent message", async () => { + const original = "A".repeat(900); + const message = assistant(original); const completeObject = vi.fn(async () => ({ object: { - action: "rewrite", text: "Condensed reply.", reason: "too long", }, })); - const routed = await routeAssistantMessage({ + const prepared = await prepareAssistantMessage({ completeObject, fastModelId: "openai/gpt-5.6-luna", message, }); - expect(routed).toMatchObject({ - kind: "deliver", - route: { action: "deliver", text: "Condensed reply." }, + expect(prepared).toMatchObject({ + kind: "reply", + text: "Condensed reply.", }); - expect(message.content).toEqual([{ type: "text", text: "Condensed reply." }]); + expect(message.content).toEqual([{ type: "text", text: original }]); }); it("skips tool-bearing assistant messages", async () => { const completeObject = vi.fn(); await expect( - routeAssistantMessage({ + prepareAssistantMessage({ completeObject, fastModelId: "openai/gpt-5.6-luna", message: assistant("working", true), @@ -197,24 +189,4 @@ describe("output router", () => { ).resolves.toEqual({ kind: "skip" }); expect(completeObject).not.toHaveBeenCalled(); }); - - it("applies rewritten text while preserving non-text parts", () => { - const message = assistant("old"); - message.content.push({ - type: "toolCall", - id: "call-2", - name: "bash", - arguments: {}, - }); - applyAssistantOutputText(message, "new"); - expect(message.content).toEqual([ - { type: "text", text: "new" }, - { - type: "toolCall", - id: "call-2", - name: "bash", - arguments: {}, - }, - ]); - }); }); From ce1f91fdfe32b7f2864a4f02576c8a770583e85d Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Sat, 29 Aug 2026 00:39:31 +0000 Subject: [PATCH 03/14] test(chat): add output-router silence evals from real failures Judge mixed [[NO_REPLY]] with the fast model instead of stripping deterministically. Snapshot the status-only silence failure, keep real answers that mention the marker, and wire cases into the guardian eval suite. Co-Authored-By: David Cramer --- .github/workflows/evals-guardian.yml | 3 + .../content/docs/reference/config-and-env.md | 9 +- packages/junior-evals/README.md | 12 +- packages/junior-evals/evals/github-actions.md | 4 +- .../evals/output-router/prepare-reply.eval.ts | 85 ++++++++++ .../junior-evals/src/output-router-harness.ts | 154 ++++++++++++++++++ .../vitest.evals.behavioral.config.ts | 6 +- .../vitest.evals.guardian.config.ts | 10 +- packages/junior/src/chat/README.md | 6 +- .../junior/src/chat/services/output-router.ts | 31 ++-- .../tests/unit/services/output-router.test.ts | 36 +++- policies/evals.md | 7 +- 12 files changed, 321 insertions(+), 42 deletions(-) create mode 100644 packages/junior-evals/evals/output-router/prepare-reply.eval.ts create mode 100644 packages/junior-evals/src/output-router-harness.ts diff --git a/.github/workflows/evals-guardian.yml b/.github/workflows/evals-guardian.yml index 95cd37c69c..226805b8d7 100644 --- a/.github/workflows/evals-guardian.yml +++ b/.github/workflows/evals-guardian.yml @@ -30,13 +30,16 @@ jobs: filters: | relevant: - 'packages/junior-evals/evals/guardian/**' + - 'packages/junior-evals/evals/output-router/**' - 'packages/junior-evals/src/guardian-harness.ts' + - 'packages/junior-evals/src/output-router-harness.ts' - 'packages/junior-evals/src/guardian-setup.ts' - 'packages/junior-evals/src/eval-ai-gateway-dispatcher.ts' - 'packages/junior-evals/guardian-global-setup.ts' - 'packages/junior-evals/vitest.evals.guardian.config.ts' - 'packages/junior-evals/package.json' - 'packages/junior/src/chat/services/guardian-action-policy.ts' + - 'packages/junior/src/chat/services/output-router.ts' - id: decision env: AI_GATEWAY_API_KEY: ${{ secrets.AI_GATEWAY_API_KEY }} diff --git a/packages/docs/src/content/docs/reference/config-and-env.md b/packages/docs/src/content/docs/reference/config-and-env.md index 4c77fd15dd..f502cdd864 100644 --- a/packages/docs/src/content/docs/reference/config-and-env.md +++ b/packages/docs/src/content/docs/reference/config-and-env.md @@ -174,10 +174,11 @@ repository for a loopback test with the official ACP SDK client. ACP remains a pre-stable surface. `output-router` uses the fast model (`AI_FAST_MODEL`) to prepare the visible -reply for each completed tool-free assistant message. Pure `[[NO_REPLY]]` stays -silent, mixed markers are removed, and long replies can be shortened. The -original agent text remains in conversation history. Leave it unset unless you -are testing that path. +reply for each completed tool-free assistant message. Exact `[[NO_REPLY]]` stays +silent. Mixed marker text is judged (status-only chatter can stay silent; a real +answer that mentions the marker still delivers). Long replies can be shortened. +The original agent text remains in conversation history. Leave it unset unless +you are testing that path. `passive-routing` turns on replies to non-mention messages in threads Junior already joined. Leave it unset in production unless you are testing that path. diff --git a/packages/junior-evals/README.md b/packages/junior-evals/README.md index a0d1e1d98b..5f009f6c06 100644 --- a/packages/junior-evals/README.md +++ b/packages/junior-evals/README.md @@ -8,7 +8,7 @@ There are three independently runnable suites: 1. **Integration** (`evals/integration/**`) — full agent/runtime runs for primary system functionality that should never regress. Failures are hard pass/fail. 2. **Behavioral** (domain folders under `evals/` except `integration/` and `guardian/`) — full agent/runtime runs that measure agent behavior and tolerate bounded variability. CI reports a suite score and only blocks below the configured floor. -3. **Guardian** (`evals/guardian/**`) — isolated decision snapshots scored only on `allow` / `ask` / `deny`. Failures are hard pass/fail. +3. **Guardian** (`evals/guardian/**` and `evals/output-router/**`) — isolated decision snapshots. Guardian scores only `allow` / `ask` / `deny`. Output-router scores only `silent` / `reply`. Failures are hard pass/fail. - We define conversation cases inline in TypeScript using `describeEval()` and the shared `slackEvals` harness options. - We run the real runtime/harness against those fixtures. @@ -57,8 +57,11 @@ Not in scope: - `evals/sentry/` - Isolated Guardian decisions: `evals/guardian/` - exact `ToolActionProposal` snapshots scored only on `allow` / `ask` / `deny` +- Isolated output-router decisions: `evals/output-router/` + - exact assistant message text snapshots scored only on `silent` / `reply` - Helpers and event builders: `src/helpers.ts` - Guardian harness: `src/guardian-harness.ts` +- Output-router harness: `src/output-router-harness.ts` - Harness/runtime adapter: `src/behavior-harness.ts` ## Execution Model @@ -105,13 +108,14 @@ Tool replay: - `pnpm evals` / `pnpm evals:behavioral`: Run the behavioral suite - `pnpm evals:integration`: Run the integration suite -- `pnpm evals:guardian`: Run isolated Guardian decision snapshots +- `pnpm evals:guardian`: Run isolated Guardian + output-router decision snapshots - `pnpm --filter @sentry/junior-evals evals:behavioral`: Run behavioral from any directory - `pnpm --filter @sentry/junior-evals evals:integration`: Run integration from any directory - `pnpm --filter @sentry/junior-evals evals:guardian`: Run Guardian from any directory - `pnpm --filter @sentry/junior-evals evals:behavioral evals/sentry/skills.eval.ts`: Run one behavioral file - `pnpm --filter @sentry/junior-evals evals:integration evals/integration/conversation/actions.eval.ts`: Run one integration file - `pnpm --filter @sentry/junior-evals evals:guardian evals/guardian/action-review.eval.ts -t "deny"`: Run one Guardian case +- `pnpm --filter @sentry/junior-evals evals:guardian evals/output-router/prepare-reply.eval.ts`: Run output-router snapshots - `pnpm --filter @sentry/junior-evals evals:behavioral --shard=1/4`: Run one of the four CI behavioral shards Pass eval file paths, `-t` filters, and shard options directly after the suite script. Do not use `pnpm exec vitest` directly, and do not insert `--` before eval arguments. @@ -129,7 +133,7 @@ Pass eval file paths, `-t` filters, and shard options directly after the suite s - Adding a trigger label fires immediately; unrelated labels do not. - Behavioral path triggers cover domain folders under `evals/{agent,conversation,github,memory,scheduler,sentry}/` and shared harness/config files under `packages/junior-evals/`. - Integration path triggers cover `evals/integration/**`, the integration config, and shared harness files under `packages/junior-evals/`. -- Guardian path triggers cover `evals/guardian/**`, the Guardian harness/config under `packages/junior-evals/`, and `packages/junior/src/chat/services/guardian-action-policy.ts`. +- Guardian path triggers cover `evals/guardian/**`, `evals/output-router/**`, the Guardian/output-router harness/config under `packages/junior-evals/`, `packages/junior/src/chat/services/guardian-action-policy.ts`, and `packages/junior/src/chat/services/output-router.ts`. - Other product source under `packages/junior/src/**` does not auto-run evals; use a `trigger-evals*` label for that. - Behavioral shards still fail individual cases under the per-case judge threshold (`0.75`), but the workflow no longer fails the shard job on those case failures alone. Each behavioral shard and the Guardian job publishes its own `vitest-evals` job summary (pass rate, scores, quality misses). - After all behavioral shards finish, `behavioral / report` combines results, writes the aggregate job summary, and publishes a `behavioral / score` Check Run. The Check Run title carries the gate line (for example `Eval pass rate 90.2% — floor 80.0%`). When that check publishes, the report step soft-fails so the Check Run owns green/red instead of canned job failure text. @@ -149,6 +153,7 @@ Behavioral and integration evals require real Vercel Sandbox access and public Q - Put full-runtime integration cases that must never regress under `evals/integration/**` using `describeEval()` with `slackEvals`. Prefer deterministic assertions; keep criteria only when the case still needs light quality scoring. - Put behavioral cases under `evals/conversation/`, `evals/agent/`, or `evals//` using `describeEval()` with `slackEvals`. - Add isolated Guardian decision snapshots under `evals/guardian/` using `describeEval()` with `guardianEvals`. Feed exact `ToolActionProposal` objects and assert only the expected `allow` / `ask` / `deny` decision. +- Add isolated output-router snapshots under `evals/output-router/` using `describeEval()` with `outputRouterEvals`. Feed exact assistant message text and assert only `silent` / `reply`. - Put messages that should be pending before processing starts in `initialEvents`. - Put ordinary later events in `events`; each is delivered after preceding work settles. - Wrap messages with `steer(...)` when they should arrive through normal ingress while the preceding agent run is active. @@ -206,6 +211,7 @@ Organize files by suite policy first, then by the user-visible area they exercis - `evals/integration/`: strict full-runtime integration cases (hard pass/fail). - `evals/conversation/`, `evals/agent/`, `evals//`: agent-behavior cases (score-gated in CI). - `evals/guardian/`: isolated Guardian decision snapshots (no main agent; hard pass/fail). +- `evals/output-router/`: isolated visible-reply prepare snapshots (no main agent; hard pass/fail). - Use short behavior nouns for filenames: `routing.eval.ts`, `delivery.eval.ts`, `credentials.eval.ts`. - Keep one coherent behavior area per file. Split files when cases exercise independently understandable journeys. - Keep shared setup in a nearby `helpers.ts`; helpers are not eval files and do not define suites. diff --git a/packages/junior-evals/evals/github-actions.md b/packages/junior-evals/evals/github-actions.md index 25954ee30a..47b2840992 100644 --- a/packages/junior-evals/evals/github-actions.md +++ b/packages/junior-evals/evals/github-actions.md @@ -69,9 +69,9 @@ Three independent workflows run on pull requests: - `Behavioral evals` runs Slack/agent evals when behavioral eval files/harness changed or the PR has `trigger-evals-behavioral` / `trigger-evals` - `Integration evals` runs system evals when integration eval files/harness changed or the PR has `trigger-evals-integration` / `trigger-evals` -- `Guardian evals` runs isolated Guardian snapshots when Guardian eval files/harness changed or the PR has `trigger-evals-guardian` / `trigger-evals` +- `Guardian evals` runs isolated Guardian and output-router snapshots when those eval files/harness changed, `output-router.ts` / Guardian policy changed, or the PR has `trigger-evals-guardian` / `trigger-evals` -Suite labels follow `trigger-evals-[domain]`. Adding a trigger label fires immediately. If the label is already on the PR, future `synchronize` events still run the matching suite(s). Product source under `packages/junior/src/**` does not auto-run evals, except Guardian policy changes in `packages/junior/src/chat/services/guardian-action-policy.ts`. +Suite labels follow `trigger-evals-[domain]`. Adding a trigger label fires immediately. If the label is already on the PR, future `synchronize` events still run the matching suite(s). Product source under `packages/junior/src/**` does not auto-run evals, except Guardian policy changes in `packages/junior/src/chat/services/guardian-action-policy.ts` and output-router changes in `packages/junior/src/chat/services/output-router.ts`. Guardian evals only need gateway credentials. Behavioral and integration evals still need gateway plus sandbox access. diff --git a/packages/junior-evals/evals/output-router/prepare-reply.eval.ts b/packages/junior-evals/evals/output-router/prepare-reply.eval.ts new file mode 100644 index 0000000000..1c45b9a5e2 --- /dev/null +++ b/packages/junior-evals/evals/output-router/prepare-reply.eval.ts @@ -0,0 +1,85 @@ +/** + * Isolated output-router corpus. + * + * Each case feeds exact assistant message text to prepareAssistantReply and + * asserts only silent vs reply. Fixtures are real failure shapes, not product + * prompt examples. + */ +import { describeEval } from "vitest-evals"; +import { NO_REPLY_MARKER } from "@/chat/no-reply"; +import { OUTPUT_REPLY_SOFT_MAX_CHARS } from "@/chat/services/output-router"; +import { outputRouterEvals } from "../../src/output-router-harness"; + +describeEval("Output Router Snapshots", outputRouterEvals, (it) => { + it("when the whole message is the silence marker, stay silent", async ({ + run, + }) => { + await run({ + expectedKind: "silent", + text: NO_REPLY_MARKER, + }); + }); + + it("when status-only chatter ends with the silence marker, stay silent", async ({ + run, + }) => { + // Real maintain-PR shape: process note + protocol silence, no user answer. + await run({ + expectedKind: "silent", + text: [ + "Same main baseline miss on createAgentDispatchWorkRouter — not caused by this PR. No PR fix.", + "", + NO_REPLY_MARKER, + ].join("\n"), + }); + }); + + it("when a real answer mentions the silence marker, keep the answer", async ({ + run, + }) => { + const result = await run({ + expectedKind: "reply", + text: `Earlier turn used ${NO_REPLY_MARKER} and then stopped.`, + }); + const text = String(result.output.text ?? ""); + if (!text.toLowerCase().includes("earlier turn")) { + throw new Error(`Expected the real answer to remain visible, got: ${text}`); + } + if (text.includes(NO_REPLY_MARKER)) { + throw new Error(`Expected the marker stripped from visible text, got: ${text}`); + } + }); + + it("when the reply is already short and clear, keep it", async ({ run }) => { + const result = await run({ + expectedKind: "reply", + text: "Draft PR is up: https://github.com/getsentry/junior/pull/1732", + }); + const text = String(result.output.text ?? ""); + if (!text.includes("1732")) { + throw new Error(`Expected the PR link to remain, got: ${text}`); + } + }); + + it("when the reply is far too long, shorten it", async ({ run }) => { + const filler = + "This section repeats background that is not needed in the final Slack reply. "; + const longText = [ + "Here is the outcome: the migration landed and traffic is healthy.", + "Next step: watch error rate for 30 minutes.", + filler.repeat(40), + "Also keep the deploy link: https://example.test/deploy/42", + ].join("\n"); + const result = await run({ + expectedKind: "reply", + maxVisibleChars: OUTPUT_REPLY_SOFT_MAX_CHARS, + text: longText, + }); + const text = String(result.output.text ?? ""); + if (!/migration|healthy|error rate|deploy/i.test(text)) { + throw new Error( + `Expected the shortened reply to keep the outcome, got: ${text}`, + ); + } + }); +}); diff --git a/packages/junior-evals/src/output-router-harness.ts b/packages/junior-evals/src/output-router-harness.ts new file mode 100644 index 0000000000..81bb18d26d --- /dev/null +++ b/packages/junior-evals/src/output-router-harness.ts @@ -0,0 +1,154 @@ +/** + * Isolated output-router harness. + * + * Feeds exact assistant message text to prepareAssistantReply without running + * the main agent, Slack transport, sandbox egress, or Postgres. + */ +import { + createHarness, + type DescribeEvalOptions, + type JsonValue, +} from "vitest-evals"; +import { completeObject } from "@/chat/pi/client"; +import { + prepareAssistantReply, + type PreparedAssistantReply, +} from "@/chat/services/output-router"; + +const OUTPUT_ROUTER_EVAL_TIMEOUT_MS = 60_000; + +export type OutputRouterExpectedKind = "silent" | "reply"; + +export interface OutputRouterEvalInput { + /** Exact assistant message text to prepare. */ + text: string; + /** Expected visible outcome. */ + expectedKind: OutputRouterExpectedKind; + /** + * Optional bound on visible reply length when a reply is expected. + * Use for long-input shortening cases. + */ + maxVisibleChars?: number; +} + +export interface OutputRouterEvalOutput extends Record { + costUsd: number | null; + expectedKind: OutputRouterExpectedKind; + kind: OutputRouterExpectedKind; + reason: string; + text: string | null; + textChars: number | null; +} + +function resolveFastModelId(): string { + const configured = process.env.AI_FAST_MODEL?.trim(); + if (configured) { + return configured; + } + return "openai/gpt-5.6-luna"; +} + +/** Run one assistant message through the production output-router boundary. */ +export async function prepareOutputRouterReply( + text: string, + options?: { signal?: AbortSignal }, +): Promise { + return prepareAssistantReply({ + completeObject: (args) => + completeObject({ + ...args, + ...(options?.signal ? { signal: options.signal } : undefined), + }), + fastModelId: resolveFastModelId(), + text, + }); +} + +/** Lightweight vitest-evals harness for isolated output-router cases. */ +export const outputRouterHarness = createHarness< + OutputRouterEvalInput, + OutputRouterEvalOutput +>({ + name: "output-router", + run: async ({ input, signal }) => { + const timeoutSignal = AbortSignal.timeout(OUTPUT_ROUTER_EVAL_TIMEOUT_MS); + const prepareSignal = signal + ? AbortSignal.any([signal, timeoutSignal]) + : timeoutSignal; + const prepared = await prepareOutputRouterReply(input.text, { + signal: prepareSignal, + }); + const kind: OutputRouterExpectedKind = + prepared.kind === "silent" ? "silent" : "reply"; + const text = prepared.kind === "reply" ? prepared.text : null; + const output: OutputRouterEvalOutput = { + costUsd: prepared.costUsd ?? null, + expectedKind: input.expectedKind, + kind, + reason: prepared.reason, + text, + textChars: text?.length ?? null, + }; + + if (kind !== input.expectedKind) { + throw new Error( + `Output router decided ${kind} (${prepared.reason}); expected ${input.expectedKind}`, + ); + } + if ( + kind === "reply" && + input.maxVisibleChars !== undefined && + (text?.length ?? 0) > input.maxVisibleChars + ) { + throw new Error( + `Output router reply length ${text?.length ?? 0} exceeds maxVisibleChars ${input.maxVisibleChars}`, + ); + } + + return { + output, + events: [ + { + type: "message", + role: "user", + content: [ + `Expected: ${input.expectedKind}`, + ...(input.maxVisibleChars !== undefined + ? [`Max visible chars: ${input.maxVisibleChars}`] + : []), + "Assistant message:", + input.text, + ].join("\n"), + }, + { + type: "message", + role: "assistant", + content: [ + `Kind: ${kind}`, + `Reason: ${prepared.reason}`, + ...(text !== null ? [`Text: ${text}`] : ["Text: null"]), + ].join("\n"), + }, + ], + usage: { + provider: "vercel-ai-gateway", + model: resolveFastModelId(), + ...(prepared.costUsd !== undefined + ? { metadata: { costUsd: prepared.costUsd } } + : {}), + }, + }; + }, +}); + +/** Shared vitest-evals suite options for isolated output-router evals. */ +export const outputRouterEvals = { + harness: outputRouterHarness, + // Exact kind match is asserted in the harness; no rubric judge. + judges: [], + judgeThreshold: null, +} satisfies DescribeEvalOptions< + OutputRouterEvalInput, + OutputRouterEvalOutput, + typeof outputRouterHarness +>; diff --git a/packages/junior-evals/vitest.evals.behavioral.config.ts b/packages/junior-evals/vitest.evals.behavioral.config.ts index f53a802d15..63fc14fb6e 100644 --- a/packages/junior-evals/vitest.evals.behavioral.config.ts +++ b/packages/junior-evals/vitest.evals.behavioral.config.ts @@ -64,7 +64,11 @@ export default defineConfig({ globalSetup: [path.resolve(__dirname, "global-setup.ts")], // Behavioral quality cases. Integration and Guardian suites have their own configs. include: ["evals/**/*.eval.ts"], - exclude: ["evals/guardian/**", "evals/integration/**"], + exclude: [ + "evals/guardian/**", + "evals/integration/**", + "evals/output-router/**", + ], maxWorkers: 1, setupFiles: [ path.resolve(__dirname, "src/setup.ts"), diff --git a/packages/junior-evals/vitest.evals.guardian.config.ts b/packages/junior-evals/vitest.evals.guardian.config.ts index 2550a313e2..1d861cc2ff 100644 --- a/packages/junior-evals/vitest.evals.guardian.config.ts +++ b/packages/junior-evals/vitest.evals.guardian.config.ts @@ -23,13 +23,14 @@ loadJuniorTestEnvFiles({ process.env.JUNIOR_SECRET = "junior-test-secret"; process.env.JUNIOR_BASE_URL ??= "https://junior.example.com"; -// Guardian cases do not touch Redis state, but keep a loopback default so any -// accidental shared import that reads REDIS_URL stays sandboxed. +// Guardian/output-router cases do not touch Redis state, but keep a loopback +// default so any accidental shared import that reads REDIS_URL stays sandboxed. process.env.JUNIOR_STATE_ADAPTER = "redis"; process.env.JUNIOR_STATE_KEY_PREFIX ??= `junior:eval-guardian:${randomUUID()}`; process.env.REDIS_URL = process.env.JUNIOR_EVAL_REDIS_URL?.trim() || "redis://127.0.0.1:6382"; process.env.AI_GUARDIAN_MODEL ??= "openai/gpt-5.6-luna"; +process.env.AI_FAST_MODEL ??= "openai/gpt-5.6-luna"; export default defineConfig({ resolve: { @@ -49,7 +50,10 @@ export default defineConfig({ environment: "node", fileParallelism: false, globalSetup: [path.resolve(__dirname, "guardian-global-setup.ts")], - include: ["evals/guardian/**/*.eval.ts"], + include: [ + "evals/guardian/**/*.eval.ts", + "evals/output-router/**/*.eval.ts", + ], maxWorkers: 1, setupFiles: [path.resolve(__dirname, "src/guardian-setup.ts")], outputFile: { json: evalReportPath }, diff --git a/packages/junior/src/chat/README.md b/packages/junior/src/chat/README.md index a17dae647c..b2219de336 100644 --- a/packages/junior/src/chat/README.md +++ b/packages/junior/src/chat/README.md @@ -21,9 +21,9 @@ file. one awaited delivery port with the completed Pi message that produced it; provider adapters deliver, then commit that agent message before the visible reply in one transaction. When experimental `output-router` is enabled, a - fast-model pass may change only the visible reply text. The original agent - message stays in history. Tool-bearing assistant text remains internal to the - agent loop. + fast-model pass may change only the visible reply text (silence, cleanup, or + shortening). The original agent message stays in history. Tool-bearing + assistant text remains internal to the agent loop. 7. The completed run result supplies diagnostics and artifacts; successful delivery or intentional no-reply completion commits the durable turn outcome. diff --git a/packages/junior/src/chat/services/output-router.ts b/packages/junior/src/chat/services/output-router.ts index c07b577140..e8ab94d13b 100644 --- a/packages/junior/src/chat/services/output-router.ts +++ b/packages/junior/src/chat/services/output-router.ts @@ -60,13 +60,11 @@ type CompleteObject = (args: { }) => Promise<{ costUsd?: number; object: unknown }>; /** - * Prompt design notes: - * - short imperative system instructions first - * - one clear output contract (structured JSON) - * - rules as a short checklist - * - no roleplay / no extra context - * Related: OpenAI structured outputs + short instruction prompts; - * Anthropic: put the task first, be direct, prefer positive rules. + * Prompt design: + * - task first, short imperative rules + * - one structured output contract + * - no extra context, no roleplay + * OpenAI structured outputs + short instructions; Anthropic: be direct. */ function buildSystemPrompt(): string { return [ @@ -78,10 +76,11 @@ function buildSystemPrompt(): string { "- reason: one short sentence", "", "Rules:", - `- Set text to null only when the message is empty or only ${NO_REPLY_MARKER}.`, - `- If ${NO_REPLY_MARKER} appears with real answer text, remove the marker and keep the answer.`, + `- Set text to null when there is no user-facing answer: empty text, only ${NO_REPLY_MARKER}, or status/process chatter that only exists to stay silent.`, + `- If the message mixes ${NO_REPLY_MARKER} with a real answer, keep the answer and remove the marker from visible text.`, + `- Do not silence a message only because it contains the string ${NO_REPLY_MARKER}. Judge whether the user still needs a visible reply.`, `- Keep short clear replies as-is (about ${OUTPUT_REPLY_SOFT_MAX_CHARS} characters or less).`, - `- If the reply is too long, shorten it. Keep the answer, key facts, links, and next steps. Do not add facts.`, + "- If the reply is too long, shorten it. Keep the answer, key facts, links, and next steps. Do not add facts.", "- Prefer 1-5 short sentences when shortening.", "- Do not add a preface or meta commentary.", ].join("\n"); @@ -133,7 +132,10 @@ function reply( }; } -/** Cheap local checks before calling the model. */ +/** + * Cheap local checks before calling the model. + * Only exact silence stays local. Mixed marker cases need judgment. + */ export function prepareAssistantReplyLocal( text: string, ): PreparedAssistantReply | null { @@ -144,13 +146,6 @@ export function prepareAssistantReplyLocal( if (isNoReplyMarker(trimmed)) { return silent("no_reply"); } - if (trimmed.includes(NO_REPLY_MARKER)) { - const stripped = stripNoReplyMarker(trimmed); - if (!stripped) { - return silent("no_reply"); - } - return reply(capVisibleText(stripped), "removed_no_reply_marker"); - } return null; } diff --git a/packages/junior/tests/unit/services/output-router.test.ts b/packages/junior/tests/unit/services/output-router.test.ts index bd60097736..dac4c95417 100644 --- a/packages/junior/tests/unit/services/output-router.test.ts +++ b/packages/junior/tests/unit/services/output-router.test.ts @@ -71,14 +71,39 @@ describe("prepare assistant reply", () => { expect(completeObject).not.toHaveBeenCalled(); }); - it("removes mixed no-reply markers locally", () => { + it("does not decide mixed no-reply markers locally", () => { + // Mixed marker cases need model judgment: silence for status-only chatter, + // keep answer when the marker is only mentioned in a real reply. expect( prepareAssistantReplyLocal(`shipped it ${NO_REPLY_MARKER}\nmore detail`), - ).toEqual({ - kind: "reply", - text: "shipped it\nmore detail", - reason: "removed_no_reply_marker", + ).toBeNull(); + }); + + it("asks the fast model for mixed marker messages", async () => { + const completeObject = vi.fn(async () => ({ + costUsd: 0.0002, + object: { + text: null, + reason: "status only, intentional silence", + }, + })); + + await expect( + prepareAssistantReply({ + completeObject, + fastModelId: "openai/gpt-5.6-luna", + text: [ + "Same baseline miss — not caused by this PR. No PR fix.", + "", + NO_REPLY_MARKER, + ].join("\n"), + }), + ).resolves.toEqual({ + kind: "silent", + reason: "status only, intentional silence", + costUsd: 0.0002, }); + expect(completeObject).toHaveBeenCalledOnce(); }); it("asks the fast model to shorten long replies", async () => { @@ -108,7 +133,6 @@ describe("prepare assistant reply", () => { promptName: "junior.prepare_assistant_reply", temperature: 0, thinkingLevel: "low", - system: expect.stringContaining("Edit one assistant message"), }), ); }); diff --git a/policies/evals.md b/policies/evals.md index 8d281d3d3e..1bfe197c8f 100644 --- a/policies/evals.md +++ b/policies/evals.md @@ -11,8 +11,9 @@ Suite policy: - **Behavioral** (domain folders under `evals/` except `integration/` and `guardian/`): agent behavior with bounded variability. CI gates on the aggregate suite floor, not a single weak case. -- **Guardian** (`evals/guardian/**`): isolated decision snapshots with exact - `allow` / `ask` / `deny` assertions. Failures are hard pass/fail. +- **Guardian** (`evals/guardian/**` and `evals/output-router/**`): isolated + decision snapshots. Guardian asserts exact `allow` / `ask` / `deny`. + Output-router asserts exact `silent` / `reply`. Failures are hard pass/fail. ## Policy @@ -20,6 +21,8 @@ Suite policy: - Assert behavior rules, not incidental wording or execution sequence. - Put never-break full-runtime integration coverage under `evals/integration/**`. Put agent-behavior measurement under behavioral domain folders. + Put isolated prepare/review decision snapshots under `evals/guardian/**` or + `evals/output-router/**`. - Do not patch product prompts with eval-shaped examples, fixture names, exact user messages, expected answers, or distinctive scenario phrases from eval files. From 8786d56fe9cfe057fc89f3d63c8244bbdc10fe86 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Sat, 29 Aug 2026 00:47:39 +0000 Subject: [PATCH 04/14] test(evals): split output-router into its own suite Give visible-reply prepare its own config, workflow, path triggers, and label so it runs only when needed. Keep Guardian separate and drop the shared-suite coupling. Co-Authored-By: David Cramer --- .github/workflows/evals-guardian.yml | 3 - .github/workflows/evals-output-router.yml | 112 ++++++++++++++++++ AGENTS.md | 2 + packages/junior-evals/README.md | 38 +++--- packages/junior-evals/evals/github-actions.md | 14 ++- .../evals/output-router/prepare-reply.eval.ts | 13 +- .../output-router-global-setup.ts | 15 +++ packages/junior-evals/package.json | 1 + .../junior-evals/src/output-router-harness.ts | 12 +- .../junior-evals/src/output-router-setup.ts | 6 + .../vitest.evals.guardian.config.ts | 10 +- .../vitest.evals.output-router.config.ts | 59 +++++++++ policies/evals.md | 18 +-- 13 files changed, 251 insertions(+), 52 deletions(-) create mode 100644 .github/workflows/evals-output-router.yml create mode 100644 packages/junior-evals/output-router-global-setup.ts create mode 100644 packages/junior-evals/src/output-router-setup.ts create mode 100644 packages/junior-evals/vitest.evals.output-router.config.ts diff --git a/.github/workflows/evals-guardian.yml b/.github/workflows/evals-guardian.yml index 226805b8d7..95cd37c69c 100644 --- a/.github/workflows/evals-guardian.yml +++ b/.github/workflows/evals-guardian.yml @@ -30,16 +30,13 @@ jobs: filters: | relevant: - 'packages/junior-evals/evals/guardian/**' - - 'packages/junior-evals/evals/output-router/**' - 'packages/junior-evals/src/guardian-harness.ts' - - 'packages/junior-evals/src/output-router-harness.ts' - 'packages/junior-evals/src/guardian-setup.ts' - 'packages/junior-evals/src/eval-ai-gateway-dispatcher.ts' - 'packages/junior-evals/guardian-global-setup.ts' - 'packages/junior-evals/vitest.evals.guardian.config.ts' - 'packages/junior-evals/package.json' - 'packages/junior/src/chat/services/guardian-action-policy.ts' - - 'packages/junior/src/chat/services/output-router.ts' - id: decision env: AI_GATEWAY_API_KEY: ${{ secrets.AI_GATEWAY_API_KEY }} diff --git a/.github/workflows/evals-output-router.yml b/.github/workflows/evals-output-router.yml new file mode 100644 index 0000000000..8290fb1776 --- /dev/null +++ b/.github/workflows/evals-output-router.yml @@ -0,0 +1,112 @@ +name: Output-router evals + +permissions: + contents: read + checks: write + +on: + pull_request: + branches: [main] + types: [opened, reopened, synchronize, labeled] + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + select: + name: output-router / select + runs-on: blacksmith-4vcpu-ubuntu-2404 + outputs: + should_run: ${{ steps.decision.outputs.should_run }} + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + persist-credentials: false + - uses: getsentry/action-filter-paths@98a158469c63115591d1c2952d34450838ab3bc1 # v0.1.0 + id: changes + with: + filters: | + relevant: + - 'packages/junior-evals/evals/output-router/**' + - 'packages/junior-evals/src/output-router-harness.ts' + - 'packages/junior-evals/src/output-router-setup.ts' + - 'packages/junior-evals/src/eval-ai-gateway-dispatcher.ts' + - 'packages/junior-evals/output-router-global-setup.ts' + - 'packages/junior-evals/vitest.evals.output-router.config.ts' + - 'packages/junior-evals/package.json' + - 'packages/junior/src/chat/services/output-router.ts' + - id: decision + env: + AI_GATEWAY_API_KEY: ${{ secrets.AI_GATEWAY_API_KEY }} + VERCEL_OIDC_TOKEN: ${{ secrets.VERCEL_OIDC_TOKEN }} + RELEVANT: ${{ steps.changes.outputs.relevant }} + PR_LABELS: ${{ join(github.event.pull_request.labels.*.name, ',') }} + run: | + set -euo pipefail + gateway_ready=false + requested=false + [[ -n "${AI_GATEWAY_API_KEY:-}" || -n "${VERCEL_OIDC_TOKEN:-}" ]] && gateway_ready=true + IFS=',' read -r -a labels <<< "${PR_LABELS:-}" + for label in "${labels[@]}"; do + if [[ "$label" == "trigger-evals" || "$label" == "trigger-evals-output-router" ]]; then + requested=true + fi + done + should_run=false + [[ "$gateway_ready" == "true" && ( "$RELEVANT" == "true" || "$requested" == "true" ) ]] && should_run=true + echo "should_run=$should_run" >> "$GITHUB_OUTPUT" + { + echo "## Output-router eval selection" + echo + echo "- relevant_files_changed: $RELEVANT" + echo "- requested: $requested" + echo "- gateway_ready: $gateway_ready" + echo "- will_run: $should_run" + } >> "$GITHUB_STEP_SUMMARY" + + output_router: + name: output-router / run + needs: select + if: needs.select.outputs.should_run == 'true' + runs-on: blacksmith-4vcpu-ubuntu-2404 + env: + AI_GATEWAY_API_KEY: ${{ secrets.AI_GATEWAY_API_KEY }} + VERCEL_OIDC_TOKEN: ${{ secrets.VERCEL_OIDC_TOKEN }} + steps: + - uses: actions/checkout@v4 + - uses: ./.github/actions/setup-node-pnpm + - name: Run output-router evals + id: run + continue-on-error: true + env: + VITEST_EVALS_OUTPUT_FILE: output-router-results.json + VITEST_EVALS_REPORT_LEVEL: info + run: pnpm --filter @sentry/junior-evals evals:output-router + - name: Require output-router eval results + id: results + if: steps.run.conclusion != 'skipped' + run: | + set -euo pipefail + result_file="packages/junior-evals/output-router-results.json" + if [[ ! -f "$result_file" ]]; then + echo "::error::missing output-router eval results ($result_file). Treat setup/runtime crashes as hard failures." + exit 1 + fi + - name: Publish output-router eval summary + if: steps.results.conclusion == 'success' + uses: getsentry/vitest-evals@v0.16.1 + with: + results: packages/junior-evals/output-router-results.json + publish-check: true + check-name: output-router / score + fail-on-failures: true + - name: Upload output-router eval results + if: steps.results.conclusion == 'success' + uses: actions/upload-artifact@v4 + with: + name: output-router-evals + path: packages/junior-evals/output-router-results.json + if-no-files-found: error + retention-days: 7 diff --git a/AGENTS.md b/AGENTS.md index d58af00abd..37b6f9d2e3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -21,6 +21,8 @@ Use **pnpm**: `pnpm install`, `pnpm dev`, `pnpm test`, `pnpm typecheck`, `pnpm s | Integration eval case | `pnpm --filter @sentry/junior-evals evals:integration path/to/file.eval.ts -t "case name"` | | Guardian eval file | `pnpm --filter @sentry/junior-evals evals:guardian path/to/file.eval.ts` | | Guardian eval case | `pnpm --filter @sentry/junior-evals evals:guardian path/to/file.eval.ts -t "case name"` | +| Output-router eval file | `pnpm --filter @sentry/junior-evals evals:output-router path/to/file.eval.ts` | +| Output-router eval case | `pnpm --filter @sentry/junior-evals evals:output-router path/to/file.eval.ts -t "case name"` | | Generate package schema | `pnpm --filter db:generate` | | Dashboard visual capture | `pnpm visual:dashboard -- --scenarios gallery-foundations` | | Release package alignment | `pnpm release:check` | diff --git a/packages/junior-evals/README.md b/packages/junior-evals/README.md index 5f009f6c06..1cbc6020bc 100644 --- a/packages/junior-evals/README.md +++ b/packages/junior-evals/README.md @@ -4,11 +4,12 @@ Evals are end-to-end Slack conversation evaluations. They are the integration-style test layer for agent-facing behavior when model interpretation is part of the contract. -There are three independently runnable suites: +There are four independently runnable suites: 1. **Integration** (`evals/integration/**`) — full agent/runtime runs for primary system functionality that should never regress. Failures are hard pass/fail. -2. **Behavioral** (domain folders under `evals/` except `integration/` and `guardian/`) — full agent/runtime runs that measure agent behavior and tolerate bounded variability. CI reports a suite score and only blocks below the configured floor. -3. **Guardian** (`evals/guardian/**` and `evals/output-router/**`) — isolated decision snapshots. Guardian scores only `allow` / `ask` / `deny`. Output-router scores only `silent` / `reply`. Failures are hard pass/fail. +2. **Behavioral** (domain folders under `evals/` except `integration/`, `guardian/`, and `output-router/`) — full agent/runtime runs that measure agent behavior and tolerate bounded variability. CI reports a suite score and only blocks below the configured floor. +3. **Guardian** (`evals/guardian/**`) — isolated action-review snapshots scored only on `allow` / `ask` / `deny`. Failures are hard pass/fail. +4. **Visible-reply prepare** (`evals/output-router/**`) — isolated prepare snapshots scored only on `silent` / `reply`. Failures are hard pass/fail. - We define conversation cases inline in TypeScript using `describeEval()` and the shared `slackEvals` harness options. - We run the real runtime/harness against those fixtures. @@ -57,11 +58,11 @@ Not in scope: - `evals/sentry/` - Isolated Guardian decisions: `evals/guardian/` - exact `ToolActionProposal` snapshots scored only on `allow` / `ask` / `deny` -- Isolated output-router decisions: `evals/output-router/` +- Isolated visible-reply prepare decisions: `evals/output-router/` - exact assistant message text snapshots scored only on `silent` / `reply` - Helpers and event builders: `src/helpers.ts` - Guardian harness: `src/guardian-harness.ts` -- Output-router harness: `src/output-router-harness.ts` +- Visible-reply prepare harness: `src/output-router-harness.ts` - Harness/runtime adapter: `src/behavior-harness.ts` ## Execution Model @@ -108,52 +109,57 @@ Tool replay: - `pnpm evals` / `pnpm evals:behavioral`: Run the behavioral suite - `pnpm evals:integration`: Run the integration suite -- `pnpm evals:guardian`: Run isolated Guardian + output-router decision snapshots +- `pnpm evals:guardian`: Run isolated Guardian action-review snapshots +- `pnpm evals:output-router`: Run isolated visible-reply prepare snapshots - `pnpm --filter @sentry/junior-evals evals:behavioral`: Run behavioral from any directory - `pnpm --filter @sentry/junior-evals evals:integration`: Run integration from any directory - `pnpm --filter @sentry/junior-evals evals:guardian`: Run Guardian from any directory +- `pnpm --filter @sentry/junior-evals evals:output-router`: Run visible-reply prepare from any directory - `pnpm --filter @sentry/junior-evals evals:behavioral evals/sentry/skills.eval.ts`: Run one behavioral file - `pnpm --filter @sentry/junior-evals evals:integration evals/integration/conversation/actions.eval.ts`: Run one integration file - `pnpm --filter @sentry/junior-evals evals:guardian evals/guardian/action-review.eval.ts -t "deny"`: Run one Guardian case -- `pnpm --filter @sentry/junior-evals evals:guardian evals/output-router/prepare-reply.eval.ts`: Run output-router snapshots +- `pnpm --filter @sentry/junior-evals evals:output-router evals/output-router/prepare-reply.eval.ts`: Run one prepare file - `pnpm --filter @sentry/junior-evals evals:behavioral --shard=1/4`: Run one of the four CI behavioral shards Pass eval file paths, `-t` filters, and shard options directly after the suite script. Do not use `pnpm exec vitest` directly, and do not insert `--` before eval arguments. ## Optional CI Runs -- On pull requests, three independent workflows run and report their own suites: +- On pull requests, four independent workflows run and report their own suites: - `Behavioral evals`: Slack/agent evals (`behavioral / shard *` + `behavioral / report` → `behavioral / score` Check Run) - `Integration evals`: system evals (`integration / shard *`) - - `Guardian evals`: isolated Guardian snapshots (`guardian / run`) + - `Guardian evals`: isolated action-review snapshots (`guardian / run`) + - `Output-router evals`: isolated visible-reply prepare snapshots (`output-router / run`) - Suite labels follow `trigger-evals-[domain]`: - `trigger-evals` starts all suites - - `trigger-evals-behavioral`, `trigger-evals-integration`, and `trigger-evals-guardian` start one suite -- Behavioral and integration evals require both gateway and sandbox secrets. Guardian only needs gateway credentials. + - `trigger-evals-behavioral`, `trigger-evals-integration`, `trigger-evals-guardian`, and `trigger-evals-output-router` start one suite +- Behavioral and integration evals require both gateway and sandbox secrets. Guardian and output-router only need gateway credentials. - Adding a trigger label fires immediately; unrelated labels do not. - Behavioral path triggers cover domain folders under `evals/{agent,conversation,github,memory,scheduler,sentry}/` and shared harness/config files under `packages/junior-evals/`. - Integration path triggers cover `evals/integration/**`, the integration config, and shared harness files under `packages/junior-evals/`. -- Guardian path triggers cover `evals/guardian/**`, `evals/output-router/**`, the Guardian/output-router harness/config under `packages/junior-evals/`, `packages/junior/src/chat/services/guardian-action-policy.ts`, and `packages/junior/src/chat/services/output-router.ts`. +- Guardian path triggers cover `evals/guardian/**`, the Guardian harness/config under `packages/junior-evals/`, and `packages/junior/src/chat/services/guardian-action-policy.ts`. +- Output-router path triggers cover `evals/output-router/**`, the prepare harness/config under `packages/junior-evals/`, and `packages/junior/src/chat/services/output-router.ts`. - Other product source under `packages/junior/src/**` does not auto-run evals; use a `trigger-evals*` label for that. -- Behavioral shards still fail individual cases under the per-case judge threshold (`0.75`), but the workflow no longer fails the shard job on those case failures alone. Each behavioral shard and the Guardian job publishes its own `vitest-evals` job summary (pass rate, scores, quality misses). +- Behavioral shards still fail individual cases under the per-case judge threshold (`0.75`), but the workflow no longer fails the shard job on those case failures alone. Each behavioral shard, Guardian job, and output-router job publishes its own `vitest-evals` job summary (pass rate, scores, quality misses). - After all behavioral shards finish, `behavioral / report` combines results, writes the aggregate job summary, and publishes a `behavioral / score` Check Run. The Check Run title carries the gate line (for example `Eval pass rate 90.2% — floor 80.0%`). When that check publishes, the report step soft-fails so the Check Run owns green/red instead of canned job failure text. - The behavioral floor is `EVAL_MIN_PASS_RATE=0.8` (`80%` of cases passed). `vitest-evals@0.16` owns the aggregate gate math; individual case misses are warnings when the floor still passes. Missing shard result files or setup/runtime crashes before results are written remain hard failures on the report job. - Integration cases fail the `integration / shard *` jobs hard on any miss. They do not use the aggregate pass-rate floor. - Guardian cases assert exact `allow` / `ask` / `deny` decisions and fail the `guardian / run` job hard on mismatch. They do not use the aggregate pass-rate floor. +- Output-router cases assert exact `silent` / `reply` decisions and fail the `output-router / run` job hard on mismatch. They do not use the aggregate pass-rate floor. - The simplest Gateway and Sandbox setup is `VERCEL_OIDC_TOKEN` alone. - The fallback CI setup is `AI_GATEWAY_API_KEY` plus `VERCEL_TOKEN` + `VERCEL_TEAM_ID` + `VERCEL_PROJECT_ID`. - Behavioral and integration global setup starts one Cloudflare Quick Tunnel for the suite so Vercel Sandbox can reach the eval egress proxy. Transient tunnel allocation failures retry up to five times with backoff. Local runs require `cloudflared` on `PATH`; CI installs a pinned binary. - Behavioral and integration state always uses a loopback Redis. Local runs default to `redis://127.0.0.1:6382`; CI sets `JUNIOR_EVAL_REDIS_URL` for its Redis service. - Setup details for GitHub Actions live in `evals/github-actions.md`. -Behavioral and integration evals require real Vercel Sandbox access and public Quick Tunnel connectivity. If either bootstrap fails, the eval fails immediately with no local fallback path. Guardian evals only need AI Gateway access. +Behavioral and integration evals require real Vercel Sandbox access and public Quick Tunnel connectivity. If either bootstrap fails, the eval fails immediately with no local fallback path. Guardian and output-router evals only need AI Gateway access. ## Authoring Rules - Put full-runtime integration cases that must never regress under `evals/integration/**` using `describeEval()` with `slackEvals`. Prefer deterministic assertions; keep criteria only when the case still needs light quality scoring. - Put behavioral cases under `evals/conversation/`, `evals/agent/`, or `evals//` using `describeEval()` with `slackEvals`. - Add isolated Guardian decision snapshots under `evals/guardian/` using `describeEval()` with `guardianEvals`. Feed exact `ToolActionProposal` objects and assert only the expected `allow` / `ask` / `deny` decision. -- Add isolated output-router snapshots under `evals/output-router/` using `describeEval()` with `outputRouterEvals`. Feed exact assistant message text and assert only `silent` / `reply`. +- Add isolated visible-reply prepare snapshots under `evals/output-router/` using `describeEval()` with `outputRouterEvals`. Feed exact assistant message text and assert only `silent` / `reply`. - Put messages that should be pending before processing starts in `initialEvents`. - Put ordinary later events in `events`; each is delivered after preceding work settles. - Wrap messages with `steer(...)` when they should arrive through normal ingress while the preceding agent run is active. @@ -210,7 +216,7 @@ Organize files by suite policy first, then by the user-visible area they exercis - `evals/integration/`: strict full-runtime integration cases (hard pass/fail). - `evals/conversation/`, `evals/agent/`, `evals//`: agent-behavior cases (score-gated in CI). -- `evals/guardian/`: isolated Guardian decision snapshots (no main agent; hard pass/fail). +- `evals/guardian/`: isolated action-review snapshots (no main agent; hard pass/fail). - `evals/output-router/`: isolated visible-reply prepare snapshots (no main agent; hard pass/fail). - Use short behavior nouns for filenames: `routing.eval.ts`, `delivery.eval.ts`, `credentials.eval.ts`. - Keep one coherent behavior area per file. Split files when cases exercise independently understandable journeys. diff --git a/packages/junior-evals/evals/github-actions.md b/packages/junior-evals/evals/github-actions.md index 47b2840992..e1c4727e45 100644 --- a/packages/junior-evals/evals/github-actions.md +++ b/packages/junior-evals/evals/github-actions.md @@ -65,26 +65,28 @@ Only needed for the token-based fallback above. Create an AI Gateway key in the ## Triggering Evals On A PR -Three independent workflows run on pull requests: +Four independent workflows run on pull requests: - `Behavioral evals` runs Slack/agent evals when behavioral eval files/harness changed or the PR has `trigger-evals-behavioral` / `trigger-evals` - `Integration evals` runs system evals when integration eval files/harness changed or the PR has `trigger-evals-integration` / `trigger-evals` -- `Guardian evals` runs isolated Guardian and output-router snapshots when those eval files/harness changed, `output-router.ts` / Guardian policy changed, or the PR has `trigger-evals-guardian` / `trigger-evals` +- `Guardian evals` runs isolated action-review snapshots when Guardian eval files/harness changed, Guardian policy changed, or the PR has `trigger-evals-guardian` / `trigger-evals` +- `Output-router evals` runs isolated visible-reply prepare snapshots when those eval files/harness changed, `output-router.ts` changed, or the PR has `trigger-evals-output-router` / `trigger-evals` -Suite labels follow `trigger-evals-[domain]`. Adding a trigger label fires immediately. If the label is already on the PR, future `synchronize` events still run the matching suite(s). Product source under `packages/junior/src/**` does not auto-run evals, except Guardian policy changes in `packages/junior/src/chat/services/guardian-action-policy.ts` and output-router changes in `packages/junior/src/chat/services/output-router.ts`. +Suite labels follow `trigger-evals-[domain]`. Adding a trigger label fires immediately. If the label is already on the PR, future `synchronize` events still run the matching suite(s). Product source under `packages/junior/src/**` does not auto-run evals, except Guardian policy changes in `packages/junior/src/chat/services/guardian-action-policy.ts` and prepare-path changes in `packages/junior/src/chat/services/output-router.ts`. -Guardian evals only need gateway credentials. Behavioral and integration evals still need gateway plus sandbox access. +Guardian and output-router evals only need gateway credentials. Behavioral and integration evals still need gateway plus sandbox access. ## Verification After adding secrets: 1. Push a commit to the PR, or add the matching `trigger-evals*` label. -2. Open the matching `Behavioral evals`, `Integration evals`, or `Guardian evals` workflow summary. +2. Open the matching `Behavioral evals`, `Integration evals`, `Guardian evals`, or `Output-router evals` workflow summary. 3. Confirm its `*/select` job reports `will_run: true` and the required credentials as ready. 4. For behavioral runs, confirm each `behavioral / shard *` job has a shard summary, `behavioral / report` has the combined summary, and the `behavioral / score` Check Run shows the pass-rate gate title. 5. For integration runs, confirm the `integration / shard *` jobs completed. Any case miss fails those jobs hard. 6. For Guardian runs, confirm the `guardian / run` job summary published and the job completed. Exact decision mismatches fail that job hard. +7. For output-router runs, confirm the `output-router / run` job summary published and the job completed. Exact silent/reply mismatches fail that job hard. ## Score-Based CI Gate @@ -100,7 +102,7 @@ If Check Run publishing is skipped or fails, the report step still fails on a re When the aggregate gate passes, individual case misses are warnings rather than failures. Setup crashes and missing result files still fail the report job hard. -Integration shards fail hard on any case miss and do not use the aggregate floor. Guardian snapshots assert exact `allow` / `ask` / `deny` decisions, publish their own job summary, and fail `guardian / run` on mismatch. +Integration shards fail hard on any case miss and do not use the aggregate floor. Guardian snapshots assert exact `allow` / `ask` / `deny` decisions, publish their own job summary, and fail `guardian / run` on mismatch. Output-router snapshots assert exact `silent` / `reply` decisions, publish their own job summary, and fail `output-router / run` on mismatch. If `sandbox_ready` is false, either `VERCEL_OIDC_TOKEN` is missing or the fallback token set is incomplete. diff --git a/packages/junior-evals/evals/output-router/prepare-reply.eval.ts b/packages/junior-evals/evals/output-router/prepare-reply.eval.ts index 1c45b9a5e2..4d5ce1ee0e 100644 --- a/packages/junior-evals/evals/output-router/prepare-reply.eval.ts +++ b/packages/junior-evals/evals/output-router/prepare-reply.eval.ts @@ -1,16 +1,15 @@ /** - * Isolated output-router corpus. + * Isolated visible-reply prepare cases. * * Each case feeds exact assistant message text to prepareAssistantReply and - * asserts only silent vs reply. Fixtures are real failure shapes, not product - * prompt examples. + * asserts only silent vs reply. Keep fixtures real-world and transcript-shaped. */ import { describeEval } from "vitest-evals"; import { NO_REPLY_MARKER } from "@/chat/no-reply"; import { OUTPUT_REPLY_SOFT_MAX_CHARS } from "@/chat/services/output-router"; import { outputRouterEvals } from "../../src/output-router-harness"; -describeEval("Output Router Snapshots", outputRouterEvals, (it) => { +describeEval("Visible Reply Prepare", outputRouterEvals, (it) => { it("when the whole message is the silence marker, stay silent", async ({ run, }) => { @@ -23,7 +22,7 @@ describeEval("Output Router Snapshots", outputRouterEvals, (it) => { it("when status-only chatter ends with the silence marker, stay silent", async ({ run, }) => { - // Real maintain-PR shape: process note + protocol silence, no user answer. + // Real maintain-PR shape: process note + silence marker, no user answer. await run({ expectedKind: "silent", text: [ @@ -46,7 +45,9 @@ describeEval("Output Router Snapshots", outputRouterEvals, (it) => { throw new Error(`Expected the real answer to remain visible, got: ${text}`); } if (text.includes(NO_REPLY_MARKER)) { - throw new Error(`Expected the marker stripped from visible text, got: ${text}`); + throw new Error( + `Expected the marker removed from visible text, got: ${text}`, + ); } }); diff --git a/packages/junior-evals/output-router-global-setup.ts b/packages/junior-evals/output-router-global-setup.ts new file mode 100644 index 0000000000..7bd1f34ee2 --- /dev/null +++ b/packages/junior-evals/output-router-global-setup.ts @@ -0,0 +1,15 @@ +import { installEvalAiGatewayDispatcher } from "./src/eval-ai-gateway-dispatcher"; + +/** + * Set up the lightweight visible-reply prepare eval invocation. + * + * These cases only need AI Gateway access. They intentionally skip Postgres, + * Redis fixtures, MSW, plugin catalogs, and sandbox egress. + */ +export default async function setup(): Promise<() => Promise> { + const restoreAiGatewayDispatcher = installEvalAiGatewayDispatcher(); + process.stdout.write( + "[evals:output-router] AI Gateway dispatcher ready (no sandbox egress)\n", + ); + return restoreAiGatewayDispatcher; +} diff --git a/packages/junior-evals/package.json b/packages/junior-evals/package.json index 129bebcf99..363c485bef 100644 --- a/packages/junior-evals/package.json +++ b/packages/junior-evals/package.json @@ -10,6 +10,7 @@ "evals:behavioral": "vitest run -c vitest.evals.behavioral.config.ts", "evals:integration": "vitest run -c vitest.evals.integration.config.ts", "evals:guardian": "vitest run -c vitest.evals.guardian.config.ts", + "evals:output-router": "vitest run -c vitest.evals.output-router.config.ts", "evals:record": "VITEST_EVALS_REPLAY_MODE=record vitest run -c vitest.evals.behavioral.config.ts" }, "devDependencies": { diff --git a/packages/junior-evals/src/output-router-harness.ts b/packages/junior-evals/src/output-router-harness.ts index 81bb18d26d..efda3eb53d 100644 --- a/packages/junior-evals/src/output-router-harness.ts +++ b/packages/junior-evals/src/output-router-harness.ts @@ -1,5 +1,5 @@ /** - * Isolated output-router harness. + * Isolated visible-reply prepare harness. * * Feeds exact assistant message text to prepareAssistantReply without running * the main agent, Slack transport, sandbox egress, or Postgres. @@ -48,7 +48,7 @@ function resolveFastModelId(): string { return "openai/gpt-5.6-luna"; } -/** Run one assistant message through the production output-router boundary. */ +/** Run one assistant message through the production prepare path. */ export async function prepareOutputRouterReply( text: string, options?: { signal?: AbortSignal }, @@ -64,7 +64,7 @@ export async function prepareOutputRouterReply( }); } -/** Lightweight vitest-evals harness for isolated output-router cases. */ +/** Lightweight vitest-evals harness for isolated visible-reply prepare cases. */ export const outputRouterHarness = createHarness< OutputRouterEvalInput, OutputRouterEvalOutput @@ -92,7 +92,7 @@ export const outputRouterHarness = createHarness< if (kind !== input.expectedKind) { throw new Error( - `Output router decided ${kind} (${prepared.reason}); expected ${input.expectedKind}`, + `Prepare path decided ${kind} (${prepared.reason}); expected ${input.expectedKind}`, ); } if ( @@ -101,7 +101,7 @@ export const outputRouterHarness = createHarness< (text?.length ?? 0) > input.maxVisibleChars ) { throw new Error( - `Output router reply length ${text?.length ?? 0} exceeds maxVisibleChars ${input.maxVisibleChars}`, + `Visible reply length ${text?.length ?? 0} exceeds maxVisibleChars ${input.maxVisibleChars}`, ); } @@ -141,7 +141,7 @@ export const outputRouterHarness = createHarness< }, }); -/** Shared vitest-evals suite options for isolated output-router evals. */ +/** Shared vitest-evals suite options for isolated visible-reply prepare evals. */ export const outputRouterEvals = { harness: outputRouterHarness, // Exact kind match is asserted in the harness; no rubric judge. diff --git a/packages/junior-evals/src/output-router-setup.ts b/packages/junior-evals/src/output-router-setup.ts new file mode 100644 index 0000000000..eb1a740609 --- /dev/null +++ b/packages/junior-evals/src/output-router-setup.ts @@ -0,0 +1,6 @@ +/** + * Per-file setup for isolated visible-reply prepare evals. + * + * Kept intentionally empty beyond documenting the boundary: these cases must + * not depend on Slack egress, Postgres, Redis fixture resets, or MSW. + */ diff --git a/packages/junior-evals/vitest.evals.guardian.config.ts b/packages/junior-evals/vitest.evals.guardian.config.ts index 1d861cc2ff..2550a313e2 100644 --- a/packages/junior-evals/vitest.evals.guardian.config.ts +++ b/packages/junior-evals/vitest.evals.guardian.config.ts @@ -23,14 +23,13 @@ loadJuniorTestEnvFiles({ process.env.JUNIOR_SECRET = "junior-test-secret"; process.env.JUNIOR_BASE_URL ??= "https://junior.example.com"; -// Guardian/output-router cases do not touch Redis state, but keep a loopback -// default so any accidental shared import that reads REDIS_URL stays sandboxed. +// Guardian cases do not touch Redis state, but keep a loopback default so any +// accidental shared import that reads REDIS_URL stays sandboxed. process.env.JUNIOR_STATE_ADAPTER = "redis"; process.env.JUNIOR_STATE_KEY_PREFIX ??= `junior:eval-guardian:${randomUUID()}`; process.env.REDIS_URL = process.env.JUNIOR_EVAL_REDIS_URL?.trim() || "redis://127.0.0.1:6382"; process.env.AI_GUARDIAN_MODEL ??= "openai/gpt-5.6-luna"; -process.env.AI_FAST_MODEL ??= "openai/gpt-5.6-luna"; export default defineConfig({ resolve: { @@ -50,10 +49,7 @@ export default defineConfig({ environment: "node", fileParallelism: false, globalSetup: [path.resolve(__dirname, "guardian-global-setup.ts")], - include: [ - "evals/guardian/**/*.eval.ts", - "evals/output-router/**/*.eval.ts", - ], + include: ["evals/guardian/**/*.eval.ts"], maxWorkers: 1, setupFiles: [path.resolve(__dirname, "src/guardian-setup.ts")], outputFile: { json: evalReportPath }, diff --git a/packages/junior-evals/vitest.evals.output-router.config.ts b/packages/junior-evals/vitest.evals.output-router.config.ts new file mode 100644 index 0000000000..668e1d9b4c --- /dev/null +++ b/packages/junior-evals/vitest.evals.output-router.config.ts @@ -0,0 +1,59 @@ +import { defineConfig } from "vitest/config"; +import { randomUUID } from "node:crypto"; +import DefaultEvalReporter from "vitest-evals/reporter"; +import path from "node:path"; +import { loadJuniorTestEnvFiles } from "../junior/tests/fixtures/env"; + +const juniorPackageRoot = path.resolve(__dirname, "../junior"); +const workspaceRoot = path.resolve(__dirname, "../.."); +const evalsPackageRoot = __dirname; +const pluginApiPackageRoot = path.resolve(__dirname, "../junior-plugin-api"); +const memoryPackageRoot = path.resolve(__dirname, "../junior-memory"); +// Leave room for provider retry inside the separate 60-second prepare budget. +const OUTPUT_ROUTER_EVAL_TEST_TIMEOUT_MS = 90_000; +const evalReportPath = path.resolve( + evalsPackageRoot, + process.env.VITEST_EVALS_OUTPUT_FILE ?? "output-router-results.json", +); + +loadJuniorTestEnvFiles({ + workspaceRoot, + packageRoots: [juniorPackageRoot, evalsPackageRoot], +}); + +process.env.JUNIOR_SECRET = "junior-test-secret"; +process.env.JUNIOR_BASE_URL ??= "https://junior.example.com"; +// These cases do not touch Redis state, but keep a loopback default so any +// accidental shared import that reads REDIS_URL stays sandboxed. +process.env.JUNIOR_STATE_ADAPTER = "redis"; +process.env.JUNIOR_STATE_KEY_PREFIX ??= `junior:eval-output-router:${randomUUID()}`; +process.env.REDIS_URL = + process.env.JUNIOR_EVAL_REDIS_URL?.trim() || "redis://127.0.0.1:6382"; +process.env.AI_FAST_MODEL ??= "openai/gpt-5.6-luna"; + +export default defineConfig({ + resolve: { + alias: { + "@": path.resolve(juniorPackageRoot, "src"), + "@sentry/junior-memory": path.resolve(memoryPackageRoot, "src/index.ts"), + "@sentry/junior-plugin-api": path.resolve( + pluginApiPackageRoot, + "src/index.ts", + ), + }, + // Vite 8 resolves tsconfig `paths` natively here: + // https://vite.dev/config/shared-options.html#resolve-tsconfigpaths + tsconfigPaths: true, + }, + test: { + environment: "node", + fileParallelism: false, + globalSetup: [path.resolve(__dirname, "output-router-global-setup.ts")], + include: ["evals/output-router/**/*.eval.ts"], + maxWorkers: 1, + setupFiles: [path.resolve(__dirname, "src/output-router-setup.ts")], + outputFile: { json: evalReportPath }, + reporters: [new DefaultEvalReporter(), "json"], + testTimeout: OUTPUT_ROUTER_EVAL_TEST_TIMEOUT_MS, + }, +}); diff --git a/policies/evals.md b/policies/evals.md index 1bfe197c8f..2a5ae480cb 100644 --- a/policies/evals.md +++ b/policies/evals.md @@ -8,12 +8,14 @@ Suite policy: - **Integration** (`evals/integration/**`): full-runtime integration coverage that must never regress. Failures are hard pass/fail. -- **Behavioral** (domain folders under `evals/` except `integration/` and - `guardian/`): agent behavior with bounded variability. CI gates on the - aggregate suite floor, not a single weak case. -- **Guardian** (`evals/guardian/**` and `evals/output-router/**`): isolated - decision snapshots. Guardian asserts exact `allow` / `ask` / `deny`. - Output-router asserts exact `silent` / `reply`. Failures are hard pass/fail. +- **Behavioral** (domain folders under `evals/` except `integration/`, + `guardian/`, and `output-router/`): agent behavior with bounded variability. + CI gates on the aggregate suite floor, not a single weak case. +- **Guardian** (`evals/guardian/**`): isolated action-review snapshots with + exact `allow` / `ask` / `deny` assertions. Failures are hard pass/fail. +- **Visible-reply prepare** (`evals/output-router/**`): isolated prepare + snapshots with exact `silent` / `reply` assertions. Failures are hard + pass/fail. ## Policy @@ -21,8 +23,8 @@ Suite policy: - Assert behavior rules, not incidental wording or execution sequence. - Put never-break full-runtime integration coverage under `evals/integration/**`. Put agent-behavior measurement under behavioral domain folders. - Put isolated prepare/review decision snapshots under `evals/guardian/**` or - `evals/output-router/**`. + Put isolated action-review snapshots under `evals/guardian/**`. + Put isolated visible-reply prepare snapshots under `evals/output-router/**`. - Do not patch product prompts with eval-shaped examples, fixture names, exact user messages, expected answers, or distinctive scenario phrases from eval files. From dadbda4a82dddeb8552d330b9c7bce77da55cb5e Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Sat, 29 Aug 2026 00:51:05 +0000 Subject: [PATCH 05/14] fix(chat): keep NO_REPLY explanations visible Stop stripping the silence marker from model replies. Real answers that explain or quote [[NO_REPLY]] must stay visible; only exact marker-only output is silent. Co-Authored-By: David Cramer --- .../evals/output-router/prepare-reply.eval.ts | 18 +++++++++++-- .../junior/src/chat/services/output-router.ts | 27 +++++-------------- .../tests/unit/services/output-router.test.ts | 25 +++++++++++++++++ 3 files changed, 48 insertions(+), 22 deletions(-) diff --git a/packages/junior-evals/evals/output-router/prepare-reply.eval.ts b/packages/junior-evals/evals/output-router/prepare-reply.eval.ts index 4d5ce1ee0e..babd01d3b1 100644 --- a/packages/junior-evals/evals/output-router/prepare-reply.eval.ts +++ b/packages/junior-evals/evals/output-router/prepare-reply.eval.ts @@ -44,9 +44,23 @@ describeEval("Visible Reply Prepare", outputRouterEvals, (it) => { if (!text.toLowerCase().includes("earlier turn")) { throw new Error(`Expected the real answer to remain visible, got: ${text}`); } - if (text.includes(NO_REPLY_MARKER)) { + }); + + it("when the message explains how the silence marker works, keep the answer", async ({ + run, + }) => { + const result = await run({ + expectedKind: "reply", + text: [ + `Intentional silence uses the exact whole-message marker ${NO_REPLY_MARKER}.`, + "If the marker is only mentioned in a normal answer, that answer should still post.", + "Only a message that is exactly the marker stays silent.", + ].join(" "), + }); + const text = String(result.output.text ?? ""); + if (!/silence|marker|exact/i.test(text)) { throw new Error( - `Expected the marker removed from visible text, got: ${text}`, + `Expected the explanation to remain visible, got: ${text}`, ); } }); diff --git a/packages/junior/src/chat/services/output-router.ts b/packages/junior/src/chat/services/output-router.ts index e8ab94d13b..861f0f8bc4 100644 --- a/packages/junior/src/chat/services/output-router.ts +++ b/packages/junior/src/chat/services/output-router.ts @@ -76,9 +76,10 @@ function buildSystemPrompt(): string { "- reason: one short sentence", "", "Rules:", - `- Set text to null when there is no user-facing answer: empty text, only ${NO_REPLY_MARKER}, or status/process chatter that only exists to stay silent.`, - `- If the message mixes ${NO_REPLY_MARKER} with a real answer, keep the answer and remove the marker from visible text.`, - `- Do not silence a message only because it contains the string ${NO_REPLY_MARKER}. Judge whether the user still needs a visible reply.`, + `- Set text to null only when there is no user-facing answer: empty text, only ${NO_REPLY_MARKER}, or status/process chatter that only exists to stay silent.`, + `- A real answer must stay a reply even if it contains ${NO_REPLY_MARKER}. That includes explanations of how silence works, quotes of the marker, or discussion of the protocol.`, + `- If ${NO_REPLY_MARKER} is only a trailing silence tag after a real answer, keep the answer and drop the tag.`, + `- If the answer itself is about the marker, keep the marker text when the user needs it.`, `- Keep short clear replies as-is (about ${OUTPUT_REPLY_SOFT_MAX_CHARS} characters or less).`, "- If the reply is too long, shorten it. Keep the answer, key facts, links, and next steps. Do not add facts.", "- Prefer 1-5 short sentences when shortening.", @@ -94,16 +95,6 @@ function buildUserPrompt(text: string): string { return body; } -function stripNoReplyMarker(text: string): string { - return sanitizeAssistantText( - text - .split(NO_REPLY_MARKER) - .join(" ") - .replace(/[ \t]+\n/g, "\n") - .replace(/\n{3,}/g, "\n\n"), - ); -} - function capVisibleText(text: string): string { if (text.length <= OUTPUT_REPLY_HARD_MAX_CHARS) { return text; @@ -161,20 +152,16 @@ function finalizeModelResult( return silent(reason, costUsd); } - let text = sanitizeAssistantText(parsed.text); + const text = sanitizeAssistantText(parsed.text); if (!text) { // Model returned blank text. Keep the original visible reply. return reply(originalText, `empty_model_text:${reason}`, costUsd); } + // Exact marker-only output is silence. Otherwise trust the model text, + // including answers that mention or explain the marker. if (isNoReplyMarker(text)) { return silent(`model_no_reply:${reason}`, costUsd); } - if (text.includes(NO_REPLY_MARKER)) { - text = stripNoReplyMarker(text); - if (!text) { - return silent(`model_no_reply:${reason}`, costUsd); - } - } return reply(capVisibleText(text), reason, costUsd); } diff --git a/packages/junior/tests/unit/services/output-router.test.ts b/packages/junior/tests/unit/services/output-router.test.ts index dac4c95417..9644588c59 100644 --- a/packages/junior/tests/unit/services/output-router.test.ts +++ b/packages/junior/tests/unit/services/output-router.test.ts @@ -106,6 +106,31 @@ describe("prepare assistant reply", () => { expect(completeObject).toHaveBeenCalledOnce(); }); + it("keeps explanations that mention the silence marker", async () => { + const explanation = [ + `Intentional silence uses the exact whole-message marker ${NO_REPLY_MARKER}.`, + "Normal answers that mention the marker should still post.", + ].join(" "); + const completeObject = vi.fn(async () => ({ + object: { + text: explanation, + reason: "explains silence protocol", + }, + })); + + await expect( + prepareAssistantReply({ + completeObject, + fastModelId: "openai/gpt-5.6-luna", + text: explanation, + }), + ).resolves.toEqual({ + kind: "reply", + text: explanation, + reason: "explains silence protocol", + }); + }); + it("asks the fast model to shorten long replies", async () => { const completeObject = vi.fn(async () => ({ costUsd: 0.0004, From 87be8add31e92dc46b589ca345bf94d6f9c46a3f Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Sat, 29 Aug 2026 00:57:06 +0000 Subject: [PATCH 06/14] test(evals): use real conversation cases for output-router Replace isolated text snapshots with full Slack/runtime cases. Script assistant replies from real transcripts (long steering essay, silence tag, protocol explanation) and assert what actually posts after prepare. Co-Authored-By: David Cramer --- .github/workflows/evals-output-router.yml | 57 +++++- packages/junior-evals/README.md | 27 ++- packages/junior-evals/evals/github-actions.md | 8 +- .../evals/output-router/prepare-reply.eval.ts | 100 ---------- .../evals/output-router/visible-reply.eval.ts | 174 ++++++++++++++++++ .../output-router-global-setup.ts | 15 -- .../junior-evals/src/output-router-harness.ts | 154 ---------------- .../junior-evals/src/output-router-setup.ts | 24 ++- .../vitest.evals.output-router.config.ts | 36 +++- policies/evals.md | 7 +- 10 files changed, 296 insertions(+), 306 deletions(-) delete mode 100644 packages/junior-evals/evals/output-router/prepare-reply.eval.ts create mode 100644 packages/junior-evals/evals/output-router/visible-reply.eval.ts delete mode 100644 packages/junior-evals/output-router-global-setup.ts delete mode 100644 packages/junior-evals/src/output-router-harness.ts diff --git a/.github/workflows/evals-output-router.yml b/.github/workflows/evals-output-router.yml index 8290fb1776..8a7e05cdac 100644 --- a/.github/workflows/evals-output-router.yml +++ b/.github/workflows/evals-output-router.yml @@ -30,24 +30,36 @@ jobs: filters: | relevant: - 'packages/junior-evals/evals/output-router/**' - - 'packages/junior-evals/src/output-router-harness.ts' - 'packages/junior-evals/src/output-router-setup.ts' + - 'packages/junior-evals/src/behavior-harness.ts' + - 'packages/junior-evals/src/helpers.ts' + - 'packages/junior-evals/src/setup.ts' - 'packages/junior-evals/src/eval-ai-gateway-dispatcher.ts' - - 'packages/junior-evals/output-router-global-setup.ts' + - 'packages/junior-evals/src/eval-context.ts' + - 'packages/junior-evals/src/eval-egress.ts' + - 'packages/junior-evals/global-setup.ts' + - 'packages/junior-evals/postgres-global-setup.ts' - 'packages/junior-evals/vitest.evals.output-router.config.ts' - 'packages/junior-evals/package.json' - 'packages/junior/src/chat/services/output-router.ts' + - 'packages/junior/src/chat/agent/index.ts' + - 'packages/junior/src/chat/experimental.ts' - id: decision env: AI_GATEWAY_API_KEY: ${{ secrets.AI_GATEWAY_API_KEY }} VERCEL_OIDC_TOKEN: ${{ secrets.VERCEL_OIDC_TOKEN }} + VERCEL_TOKEN: ${{ secrets.VERCEL_TOKEN }} + VERCEL_TEAM_ID: ${{ secrets.VERCEL_TEAM_ID }} + VERCEL_PROJECT_ID: ${{ secrets.VERCEL_PROJECT_ID }} RELEVANT: ${{ steps.changes.outputs.relevant }} PR_LABELS: ${{ join(github.event.pull_request.labels.*.name, ',') }} run: | set -euo pipefail gateway_ready=false + sandbox_ready=false requested=false [[ -n "${AI_GATEWAY_API_KEY:-}" || -n "${VERCEL_OIDC_TOKEN:-}" ]] && gateway_ready=true + [[ -n "${VERCEL_OIDC_TOKEN:-}" || ( -n "${VERCEL_TOKEN:-}" && -n "${VERCEL_TEAM_ID:-}" && -n "${VERCEL_PROJECT_ID:-}" ) ]] && sandbox_ready=true IFS=',' read -r -a labels <<< "${PR_LABELS:-}" for label in "${labels[@]}"; do if [[ "$label" == "trigger-evals" || "$label" == "trigger-evals-output-router" ]]; then @@ -55,7 +67,7 @@ jobs: fi done should_run=false - [[ "$gateway_ready" == "true" && ( "$RELEVANT" == "true" || "$requested" == "true" ) ]] && should_run=true + [[ "$gateway_ready" == "true" && "$sandbox_ready" == "true" && ( "$RELEVANT" == "true" || "$requested" == "true" ) ]] && should_run=true echo "should_run=$should_run" >> "$GITHUB_OUTPUT" { echo "## Output-router eval selection" @@ -63,6 +75,7 @@ jobs: echo "- relevant_files_changed: $RELEVANT" echo "- requested: $requested" echo "- gateway_ready: $gateway_ready" + echo "- sandbox_ready: $sandbox_ready" echo "- will_run: $should_run" } >> "$GITHUB_STEP_SUMMARY" @@ -71,12 +84,50 @@ jobs: needs: select if: needs.select.outputs.should_run == 'true' runs-on: blacksmith-4vcpu-ubuntu-2404 + services: + postgres: + image: pgvector/pgvector:pg17 + env: + POSTGRES_USER: junior + POSTGRES_PASSWORD: junior + POSTGRES_DB: junior + ports: + - 5432:5432 + options: >- + --health-cmd "pg_isready -U junior -d junior" + --health-interval 10s + --health-timeout 5s + --health-retries 5 + redis: + image: redis:7-alpine + ports: + - 6379:6379 + options: >- + --health-cmd "redis-cli ping" + --health-interval 10s + --health-timeout 5s + --health-retries 5 env: + JUNIOR_EVAL_REDIS_URL: redis://127.0.0.1:6379 + DATABASE_URL: postgres://junior:junior@localhost:5432/junior AI_GATEWAY_API_KEY: ${{ secrets.AI_GATEWAY_API_KEY }} + VERCEL_TOKEN: ${{ secrets.VERCEL_TOKEN }} + VERCEL_TEAM_ID: ${{ secrets.VERCEL_TEAM_ID }} + VERCEL_PROJECT_ID: ${{ secrets.VERCEL_PROJECT_ID }} VERCEL_OIDC_TOKEN: ${{ secrets.VERCEL_OIDC_TOKEN }} steps: - uses: actions/checkout@v4 - uses: ./.github/actions/setup-node-pnpm + - name: Install cloudflared + run: | + set -euo pipefail + curl --fail --location --silent --show-error \ + https://github.com/cloudflare/cloudflared/releases/download/2026.7.2/cloudflared-linux-amd64 \ + --output "$RUNNER_TEMP/cloudflared" + echo "ec905ea7b7e327ff8abdde8cb64697a2152de74dbcdbf6aec9db8364eb3886cd $RUNNER_TEMP/cloudflared" | sha256sum --check + chmod +x "$RUNNER_TEMP/cloudflared" + echo "$RUNNER_TEMP" >> "$GITHUB_PATH" + "$RUNNER_TEMP/cloudflared" version - name: Run output-router evals id: run continue-on-error: true diff --git a/packages/junior-evals/README.md b/packages/junior-evals/README.md index 1cbc6020bc..02e5758334 100644 --- a/packages/junior-evals/README.md +++ b/packages/junior-evals/README.md @@ -9,7 +9,7 @@ There are four independently runnable suites: 1. **Integration** (`evals/integration/**`) — full agent/runtime runs for primary system functionality that should never regress. Failures are hard pass/fail. 2. **Behavioral** (domain folders under `evals/` except `integration/`, `guardian/`, and `output-router/`) — full agent/runtime runs that measure agent behavior and tolerate bounded variability. CI reports a suite score and only blocks below the configured floor. 3. **Guardian** (`evals/guardian/**`) — isolated action-review snapshots scored only on `allow` / `ask` / `deny`. Failures are hard pass/fail. -4. **Visible-reply prepare** (`evals/output-router/**`) — isolated prepare snapshots scored only on `silent` / `reply`. Failures are hard pass/fail. +4. **Visible-reply prepare** (`evals/output-router/**`) — full Slack/runtime conversation cases for the optional prepare path. Failures are hard pass/fail. - We define conversation cases inline in TypeScript using `describeEval()` and the shared `slackEvals` harness options. - We run the real runtime/harness against those fixtures. @@ -58,11 +58,10 @@ Not in scope: - `evals/sentry/` - Isolated Guardian decisions: `evals/guardian/` - exact `ToolActionProposal` snapshots scored only on `allow` / `ask` / `deny` -- Isolated visible-reply prepare decisions: `evals/output-router/` - - exact assistant message text snapshots scored only on `silent` / `reply` +- Visible-reply prepare conversation cases: `evals/output-router/` + - scripted assistant text through the real prepare + delivery path - Helpers and event builders: `src/helpers.ts` - Guardian harness: `src/guardian-harness.ts` -- Visible-reply prepare harness: `src/output-router-harness.ts` - Harness/runtime adapter: `src/behavior-harness.ts` ## Execution Model @@ -110,15 +109,15 @@ Tool replay: - `pnpm evals` / `pnpm evals:behavioral`: Run the behavioral suite - `pnpm evals:integration`: Run the integration suite - `pnpm evals:guardian`: Run isolated Guardian action-review snapshots -- `pnpm evals:output-router`: Run isolated visible-reply prepare snapshots +- `pnpm evals:output-router`: Run visible-reply prepare conversation cases - `pnpm --filter @sentry/junior-evals evals:behavioral`: Run behavioral from any directory - `pnpm --filter @sentry/junior-evals evals:integration`: Run integration from any directory - `pnpm --filter @sentry/junior-evals evals:guardian`: Run Guardian from any directory -- `pnpm --filter @sentry/junior-evals evals:output-router`: Run visible-reply prepare from any directory +- `pnpm --filter @sentry/junior-evals evals:output-router`: Run visible-reply prepare conversation cases from any directory - `pnpm --filter @sentry/junior-evals evals:behavioral evals/sentry/skills.eval.ts`: Run one behavioral file - `pnpm --filter @sentry/junior-evals evals:integration evals/integration/conversation/actions.eval.ts`: Run one integration file - `pnpm --filter @sentry/junior-evals evals:guardian evals/guardian/action-review.eval.ts -t "deny"`: Run one Guardian case -- `pnpm --filter @sentry/junior-evals evals:output-router evals/output-router/prepare-reply.eval.ts`: Run one prepare file +- `pnpm --filter @sentry/junior-evals evals:output-router evals/output-router/visible-reply.eval.ts`: Run one prepare file - `pnpm --filter @sentry/junior-evals evals:behavioral --shard=1/4`: Run one of the four CI behavioral shards Pass eval file paths, `-t` filters, and shard options directly after the suite script. Do not use `pnpm exec vitest` directly, and do not insert `--` before eval arguments. @@ -129,37 +128,37 @@ Pass eval file paths, `-t` filters, and shard options directly after the suite s - `Behavioral evals`: Slack/agent evals (`behavioral / shard *` + `behavioral / report` → `behavioral / score` Check Run) - `Integration evals`: system evals (`integration / shard *`) - `Guardian evals`: isolated action-review snapshots (`guardian / run`) - - `Output-router evals`: isolated visible-reply prepare snapshots (`output-router / run`) + - `Output-router evals`: visible-reply prepare conversation cases (`output-router / run`) - Suite labels follow `trigger-evals-[domain]`: - `trigger-evals` starts all suites - `trigger-evals-behavioral`, `trigger-evals-integration`, `trigger-evals-guardian`, and `trigger-evals-output-router` start one suite -- Behavioral and integration evals require both gateway and sandbox secrets. Guardian and output-router only need gateway credentials. +- Behavioral, integration, and output-router evals require both gateway and sandbox secrets. Guardian only needs gateway credentials. - Adding a trigger label fires immediately; unrelated labels do not. - Behavioral path triggers cover domain folders under `evals/{agent,conversation,github,memory,scheduler,sentry}/` and shared harness/config files under `packages/junior-evals/`. - Integration path triggers cover `evals/integration/**`, the integration config, and shared harness files under `packages/junior-evals/`. - Guardian path triggers cover `evals/guardian/**`, the Guardian harness/config under `packages/junior-evals/`, and `packages/junior/src/chat/services/guardian-action-policy.ts`. -- Output-router path triggers cover `evals/output-router/**`, the prepare harness/config under `packages/junior-evals/`, and `packages/junior/src/chat/services/output-router.ts`. +- Output-router path triggers cover `evals/output-router/**`, shared conversation harness/config under `packages/junior-evals/`, `packages/junior/src/chat/services/output-router.ts`, and the agent delivery wire-up. - Other product source under `packages/junior/src/**` does not auto-run evals; use a `trigger-evals*` label for that. - Behavioral shards still fail individual cases under the per-case judge threshold (`0.75`), but the workflow no longer fails the shard job on those case failures alone. Each behavioral shard, Guardian job, and output-router job publishes its own `vitest-evals` job summary (pass rate, scores, quality misses). - After all behavioral shards finish, `behavioral / report` combines results, writes the aggregate job summary, and publishes a `behavioral / score` Check Run. The Check Run title carries the gate line (for example `Eval pass rate 90.2% — floor 80.0%`). When that check publishes, the report step soft-fails so the Check Run owns green/red instead of canned job failure text. - The behavioral floor is `EVAL_MIN_PASS_RATE=0.8` (`80%` of cases passed). `vitest-evals@0.16` owns the aggregate gate math; individual case misses are warnings when the floor still passes. Missing shard result files or setup/runtime crashes before results are written remain hard failures on the report job. - Integration cases fail the `integration / shard *` jobs hard on any miss. They do not use the aggregate pass-rate floor. - Guardian cases assert exact `allow` / `ask` / `deny` decisions and fail the `guardian / run` job hard on mismatch. They do not use the aggregate pass-rate floor. -- Output-router cases assert exact `silent` / `reply` decisions and fail the `output-router / run` job hard on mismatch. They do not use the aggregate pass-rate floor. +- Output-router cases assert delivered conversation replies (or silence) and fail the `output-router / run` job hard on mismatch. They do not use the aggregate pass-rate floor. - The simplest Gateway and Sandbox setup is `VERCEL_OIDC_TOKEN` alone. - The fallback CI setup is `AI_GATEWAY_API_KEY` plus `VERCEL_TOKEN` + `VERCEL_TEAM_ID` + `VERCEL_PROJECT_ID`. - Behavioral and integration global setup starts one Cloudflare Quick Tunnel for the suite so Vercel Sandbox can reach the eval egress proxy. Transient tunnel allocation failures retry up to five times with backoff. Local runs require `cloudflared` on `PATH`; CI installs a pinned binary. - Behavioral and integration state always uses a loopback Redis. Local runs default to `redis://127.0.0.1:6382`; CI sets `JUNIOR_EVAL_REDIS_URL` for its Redis service. - Setup details for GitHub Actions live in `evals/github-actions.md`. -Behavioral and integration evals require real Vercel Sandbox access and public Quick Tunnel connectivity. If either bootstrap fails, the eval fails immediately with no local fallback path. Guardian and output-router evals only need AI Gateway access. +Behavioral, integration, and output-router evals require real Vercel Sandbox access and public Quick Tunnel connectivity. If either bootstrap fails, the eval fails immediately with no local fallback path. Guardian evals only need AI Gateway access. ## Authoring Rules - Put full-runtime integration cases that must never regress under `evals/integration/**` using `describeEval()` with `slackEvals`. Prefer deterministic assertions; keep criteria only when the case still needs light quality scoring. - Put behavioral cases under `evals/conversation/`, `evals/agent/`, or `evals//` using `describeEval()` with `slackEvals`. - Add isolated Guardian decision snapshots under `evals/guardian/` using `describeEval()` with `guardianEvals`. Feed exact `ToolActionProposal` objects and assert only the expected `allow` / `ask` / `deny` decision. -- Add isolated visible-reply prepare snapshots under `evals/output-router/` using `describeEval()` with `outputRouterEvals`. Feed exact assistant message text and assert only `silent` / `reply`. +- Add visible-reply prepare conversation cases under `evals/output-router/` using `describeEval()` with `slackEvals`. Prefer scripted `reply_texts` from real transcripts, then assert what actually posts after prepare. - Put messages that should be pending before processing starts in `initialEvents`. - Put ordinary later events in `events`; each is delivered after preceding work settles. - Wrap messages with `steer(...)` when they should arrive through normal ingress while the preceding agent run is active. @@ -217,7 +216,7 @@ Organize files by suite policy first, then by the user-visible area they exercis - `evals/integration/`: strict full-runtime integration cases (hard pass/fail). - `evals/conversation/`, `evals/agent/`, `evals//`: agent-behavior cases (score-gated in CI). - `evals/guardian/`: isolated action-review snapshots (no main agent; hard pass/fail). -- `evals/output-router/`: isolated visible-reply prepare snapshots (no main agent; hard pass/fail). +- `evals/output-router/`: visible-reply prepare conversation cases (full runtime; hard pass/fail). - Use short behavior nouns for filenames: `routing.eval.ts`, `delivery.eval.ts`, `credentials.eval.ts`. - Keep one coherent behavior area per file. Split files when cases exercise independently understandable journeys. - Keep shared setup in a nearby `helpers.ts`; helpers are not eval files and do not define suites. diff --git a/packages/junior-evals/evals/github-actions.md b/packages/junior-evals/evals/github-actions.md index e1c4727e45..e6f1cae510 100644 --- a/packages/junior-evals/evals/github-actions.md +++ b/packages/junior-evals/evals/github-actions.md @@ -70,11 +70,11 @@ Four independent workflows run on pull requests: - `Behavioral evals` runs Slack/agent evals when behavioral eval files/harness changed or the PR has `trigger-evals-behavioral` / `trigger-evals` - `Integration evals` runs system evals when integration eval files/harness changed or the PR has `trigger-evals-integration` / `trigger-evals` - `Guardian evals` runs isolated action-review snapshots when Guardian eval files/harness changed, Guardian policy changed, or the PR has `trigger-evals-guardian` / `trigger-evals` -- `Output-router evals` runs isolated visible-reply prepare snapshots when those eval files/harness changed, `output-router.ts` changed, or the PR has `trigger-evals-output-router` / `trigger-evals` +- `Output-router evals` runs visible-reply prepare conversation cases when those eval files/harness changed, `output-router.ts` changed, or the PR has `trigger-evals-output-router` / `trigger-evals` Suite labels follow `trigger-evals-[domain]`. Adding a trigger label fires immediately. If the label is already on the PR, future `synchronize` events still run the matching suite(s). Product source under `packages/junior/src/**` does not auto-run evals, except Guardian policy changes in `packages/junior/src/chat/services/guardian-action-policy.ts` and prepare-path changes in `packages/junior/src/chat/services/output-router.ts`. -Guardian and output-router evals only need gateway credentials. Behavioral and integration evals still need gateway plus sandbox access. +Guardian evals only need gateway credentials. Behavioral, integration, and output-router evals still need gateway plus sandbox access. ## Verification @@ -86,7 +86,7 @@ After adding secrets: 4. For behavioral runs, confirm each `behavioral / shard *` job has a shard summary, `behavioral / report` has the combined summary, and the `behavioral / score` Check Run shows the pass-rate gate title. 5. For integration runs, confirm the `integration / shard *` jobs completed. Any case miss fails those jobs hard. 6. For Guardian runs, confirm the `guardian / run` job summary published and the job completed. Exact decision mismatches fail that job hard. -7. For output-router runs, confirm the `output-router / run` job summary published and the job completed. Exact silent/reply mismatches fail that job hard. +7. For output-router runs, confirm the `output-router / run` job summary published and the job completed. Delivered-reply or silence mismatches fail that job hard. ## Score-Based CI Gate @@ -102,7 +102,7 @@ If Check Run publishing is skipped or fails, the report step still fails on a re When the aggregate gate passes, individual case misses are warnings rather than failures. Setup crashes and missing result files still fail the report job hard. -Integration shards fail hard on any case miss and do not use the aggregate floor. Guardian snapshots assert exact `allow` / `ask` / `deny` decisions, publish their own job summary, and fail `guardian / run` on mismatch. Output-router snapshots assert exact `silent` / `reply` decisions, publish their own job summary, and fail `output-router / run` on mismatch. +Integration shards fail hard on any case miss and do not use the aggregate floor. Guardian snapshots assert exact `allow` / `ask` / `deny` decisions, publish their own job summary, and fail `guardian / run` on mismatch. Output-router conversation cases assert what posts after prepare, publish their own job summary, and fail `output-router / run` on mismatch. If `sandbox_ready` is false, either `VERCEL_OIDC_TOKEN` is missing or the fallback token set is incomplete. diff --git a/packages/junior-evals/evals/output-router/prepare-reply.eval.ts b/packages/junior-evals/evals/output-router/prepare-reply.eval.ts deleted file mode 100644 index babd01d3b1..0000000000 --- a/packages/junior-evals/evals/output-router/prepare-reply.eval.ts +++ /dev/null @@ -1,100 +0,0 @@ -/** - * Isolated visible-reply prepare cases. - * - * Each case feeds exact assistant message text to prepareAssistantReply and - * asserts only silent vs reply. Keep fixtures real-world and transcript-shaped. - */ -import { describeEval } from "vitest-evals"; -import { NO_REPLY_MARKER } from "@/chat/no-reply"; -import { OUTPUT_REPLY_SOFT_MAX_CHARS } from "@/chat/services/output-router"; -import { outputRouterEvals } from "../../src/output-router-harness"; - -describeEval("Visible Reply Prepare", outputRouterEvals, (it) => { - it("when the whole message is the silence marker, stay silent", async ({ - run, - }) => { - await run({ - expectedKind: "silent", - text: NO_REPLY_MARKER, - }); - }); - - it("when status-only chatter ends with the silence marker, stay silent", async ({ - run, - }) => { - // Real maintain-PR shape: process note + silence marker, no user answer. - await run({ - expectedKind: "silent", - text: [ - "Same main baseline miss on createAgentDispatchWorkRouter — not caused by this PR. No PR fix.", - "", - NO_REPLY_MARKER, - ].join("\n"), - }); - }); - - it("when a real answer mentions the silence marker, keep the answer", async ({ - run, - }) => { - const result = await run({ - expectedKind: "reply", - text: `Earlier turn used ${NO_REPLY_MARKER} and then stopped.`, - }); - const text = String(result.output.text ?? ""); - if (!text.toLowerCase().includes("earlier turn")) { - throw new Error(`Expected the real answer to remain visible, got: ${text}`); - } - }); - - it("when the message explains how the silence marker works, keep the answer", async ({ - run, - }) => { - const result = await run({ - expectedKind: "reply", - text: [ - `Intentional silence uses the exact whole-message marker ${NO_REPLY_MARKER}.`, - "If the marker is only mentioned in a normal answer, that answer should still post.", - "Only a message that is exactly the marker stays silent.", - ].join(" "), - }); - const text = String(result.output.text ?? ""); - if (!/silence|marker|exact/i.test(text)) { - throw new Error( - `Expected the explanation to remain visible, got: ${text}`, - ); - } - }); - - it("when the reply is already short and clear, keep it", async ({ run }) => { - const result = await run({ - expectedKind: "reply", - text: "Draft PR is up: https://github.com/getsentry/junior/pull/1732", - }); - const text = String(result.output.text ?? ""); - if (!text.includes("1732")) { - throw new Error(`Expected the PR link to remain, got: ${text}`); - } - }); - - it("when the reply is far too long, shorten it", async ({ run }) => { - const filler = - "This section repeats background that is not needed in the final Slack reply. "; - const longText = [ - "Here is the outcome: the migration landed and traffic is healthy.", - "Next step: watch error rate for 30 minutes.", - filler.repeat(40), - "Also keep the deploy link: https://example.test/deploy/42", - ].join("\n"); - const result = await run({ - expectedKind: "reply", - maxVisibleChars: OUTPUT_REPLY_SOFT_MAX_CHARS, - text: longText, - }); - const text = String(result.output.text ?? ""); - if (!/migration|healthy|error rate|deploy/i.test(text)) { - throw new Error( - `Expected the shortened reply to keep the outcome, got: ${text}`, - ); - } - }); -}); diff --git a/packages/junior-evals/evals/output-router/visible-reply.eval.ts b/packages/junior-evals/evals/output-router/visible-reply.eval.ts new file mode 100644 index 0000000000..5860f6c3e2 --- /dev/null +++ b/packages/junior-evals/evals/output-router/visible-reply.eval.ts @@ -0,0 +1,174 @@ +/** + * Conversation coverage for the optional visible-reply prepare path. + * + * These cases run the full Slack/runtime harness with scripted assistant + * text, then assert what actually posts after prepare. Fixtures are + * transcript-shaped (real long answers, silence tags, protocol explanations). + */ +import { describeEval } from "vitest-evals"; +import { expect } from "vitest"; +import { NO_REPLY_MARKER } from "@/chat/no-reply"; +import { OUTPUT_REPLY_SOFT_MAX_CHARS } from "@/chat/services/output-router"; +import { + mention, + rubric, + slackEvals, + visibleAssistantText, + visibleThreadReplies, +} from "../../src/helpers"; + +/** Real long steering comparison that should not post as a wall of text. */ +const LONG_STEERING_ESSAY = [ + "**yeah — steering is the weaker half of this comparison.** openclaw treats mid-run guidance as the default path; junior treats it as a gated special case. that mismatch is the reliability gap.", + "", + "### what openclaw does", + "", + "- default queue mode is **`steer`** for normal inbound messages while a run is active", + "- injects at **tool-launch + model** boundaries; unfinished sequential tools get synthetic `Skipped due to queued user message.` results, then the steer is model-visible before the next decision", + "- if the runtime can’t accept a steer, it **falls back to followup** instead of dropping", + "- explicit `/steer ` works **independent of queue mode**; `/queue interrupt|followup|collect` are first-class", + "", + "### what junior does", + "", + "1. **hard eligibility gate before anything mid-run**", + ' - mailbox `delivery: "interrupt"` only if `isMention` **or** leading `!!`', + " - otherwise `defer` → waits for the turn to finish", + "", + "2. **cross-actor default is inverted vs openclaw**", + " - `JUNIOR_CROSS_ACTOR_MID_RUN_MODE` defaults to **`follow_up`**", + " - mid-run accept only if: mode=`steer` **or** same author as active turn **or** `!!`", + "", + "3. **drain is late**", + " - steers are injected in `prepareNextTurnWithContext` only (after a tool finishes, before the next model call)", + " - `beforeToolCall` does **not** skip unstarted tools for a pending steer", + " - long tool / parallel tool batches keep going until the next model boundary", + "", + "4. **docs oversell the UX**", + '- “reply with plain language… course-correct at any point”', + " - code reality: plain non-mention reply mid-run is usually a **follow-up**, not a steer", + " - `!!` exists as the override, but it’s basically an internal escape hatch, not a real user contract", + "", + "### why it feels flaky", + "", + "| situation | junior | openclaw |", + "|---|---|---|", + "| plain reply mid-run, no `@` | usually waits (`defer`) | steers (default) |", + "| `@jr stop` mid-run, same author | steers at **next model** boundary | steers earlier (tool-launch too) |", + "| other person `@jr …` | waits unless mode=`steer` or `!!` | steers (sender labeled) |", + "| steer while tools still queued | tools keep running | unstarted tools skipped |", + "| inject fails | often warn + soft fail | park → followup fallback |", + "", + "### bottom line", + "", + "openclaw: **steer-by-default + early boundary + never drop**.", + "", + "junior: **mention/`!!`-gated + model-boundary-only + cross-actor follow_up default**.", + "", + "so “ours isn’t working reliably” is less random failure and more **product/runtime contract**: most of what people think is steering is actually deferred follow-up, and even real steers land later than openclaw’s.", + "", + "if you want parity, the minimum set is roughly:", + "", + "1. default mid-run path closer to openclaw `steer` (at least same-thread subscribed traffic)", + "2. drain / skip at tool-launch, not only `prepareNextTurn`", + "3. guaranteed fallback to next-turn follow-up when inject can’t land", + "4. docs that match the real gates (`@` / `!!` / cross-actor mode)", + "", + "want me to turn that into a concrete junior issue/PR plan?", +].join("\n"); + +describeEval("Visible Reply Prepare", slackEvals, (it) => { + it("when the assistant writes a long explanatory essay, post a short reply", async ({ + run, + }) => { + expect(LONG_STEERING_ESSAY.length).toBeGreaterThan( + OUTPUT_REPLY_SOFT_MAX_CHARS, + ); + + const result = await run({ + overrides: { + reply_texts: [LONG_STEERING_ESSAY], + }, + initialEvents: [ + mention( + "how does junior steering compare to openclaw? keep it practical", + ), + ], + requireSandboxReady: false, + criteria: rubric({ + pass: [ + "The reply is short and readable for Slack (about a few sentences or a tight bullet list).", + "The reply still covers the core gap: junior steers less by default and later than openclaw.", + ], + fail: [ + "Do not post the full multi-section essay, markdown table, or long numbered parity plan as the visible reply.", + "Do not open with process narration about checking docs or writing an essay.", + ], + }), + }); + + expect(visibleThreadReplies(result.session)).toHaveLength(1); + expect(visibleAssistantText(result.session).length).toBeLessThanOrEqual( + OUTPUT_REPLY_SOFT_MAX_CHARS, + ); + }); + + it("when maintain work ends with status chatter and a silence marker, stay silent", async ({ + run, + }) => { + const result = await run({ + overrides: { + reply_texts: [ + [ + "Same main baseline miss on createAgentDispatchWorkRouter — not caused by this PR. No PR fix.", + "", + NO_REPLY_MARKER, + ].join("\n"), + ], + }, + initialEvents: [ + mention( + "check the PR checks for the guardian ordinary-writes change and only ping if something needs a fix", + ), + ], + requireSandboxReady: false, + }); + + expect(visibleThreadReplies(result.session)).toEqual([]); + expect(visibleAssistantText(result.session)).not.toContain(NO_REPLY_MARKER); + }); + + it("when asked how silence works, keep the explanation visible", async ({ + run, + }) => { + const result = await run({ + overrides: { + reply_texts: [ + [ + `Intentional silence uses the exact whole-message marker ${NO_REPLY_MARKER}.`, + "If the marker is only mentioned in a normal answer, that answer should still post.", + "Only a message that is exactly the marker stays silent.", + ].join(" "), + ], + }, + initialEvents: [ + mention( + "how does junior's no-reply marker work? when does a message stay silent?", + ), + ], + requireSandboxReady: false, + criteria: rubric({ + pass: [ + "The reply explains that silence requires the whole message to be the marker, and that ordinary answers can still mention it.", + ], + fail: [ + "Do not stay silent or drop the explanation just because the marker string appears in the answer.", + ], + }), + }); + + expect(visibleThreadReplies(result.session)).toHaveLength(1); + const text = visibleAssistantText(result.session); + expect(text.length).toBeGreaterThan(0); + expect(/silence|marker|exact/i.test(text)).toBe(true); + }); +}); diff --git a/packages/junior-evals/output-router-global-setup.ts b/packages/junior-evals/output-router-global-setup.ts deleted file mode 100644 index 7bd1f34ee2..0000000000 --- a/packages/junior-evals/output-router-global-setup.ts +++ /dev/null @@ -1,15 +0,0 @@ -import { installEvalAiGatewayDispatcher } from "./src/eval-ai-gateway-dispatcher"; - -/** - * Set up the lightweight visible-reply prepare eval invocation. - * - * These cases only need AI Gateway access. They intentionally skip Postgres, - * Redis fixtures, MSW, plugin catalogs, and sandbox egress. - */ -export default async function setup(): Promise<() => Promise> { - const restoreAiGatewayDispatcher = installEvalAiGatewayDispatcher(); - process.stdout.write( - "[evals:output-router] AI Gateway dispatcher ready (no sandbox egress)\n", - ); - return restoreAiGatewayDispatcher; -} diff --git a/packages/junior-evals/src/output-router-harness.ts b/packages/junior-evals/src/output-router-harness.ts deleted file mode 100644 index efda3eb53d..0000000000 --- a/packages/junior-evals/src/output-router-harness.ts +++ /dev/null @@ -1,154 +0,0 @@ -/** - * Isolated visible-reply prepare harness. - * - * Feeds exact assistant message text to prepareAssistantReply without running - * the main agent, Slack transport, sandbox egress, or Postgres. - */ -import { - createHarness, - type DescribeEvalOptions, - type JsonValue, -} from "vitest-evals"; -import { completeObject } from "@/chat/pi/client"; -import { - prepareAssistantReply, - type PreparedAssistantReply, -} from "@/chat/services/output-router"; - -const OUTPUT_ROUTER_EVAL_TIMEOUT_MS = 60_000; - -export type OutputRouterExpectedKind = "silent" | "reply"; - -export interface OutputRouterEvalInput { - /** Exact assistant message text to prepare. */ - text: string; - /** Expected visible outcome. */ - expectedKind: OutputRouterExpectedKind; - /** - * Optional bound on visible reply length when a reply is expected. - * Use for long-input shortening cases. - */ - maxVisibleChars?: number; -} - -export interface OutputRouterEvalOutput extends Record { - costUsd: number | null; - expectedKind: OutputRouterExpectedKind; - kind: OutputRouterExpectedKind; - reason: string; - text: string | null; - textChars: number | null; -} - -function resolveFastModelId(): string { - const configured = process.env.AI_FAST_MODEL?.trim(); - if (configured) { - return configured; - } - return "openai/gpt-5.6-luna"; -} - -/** Run one assistant message through the production prepare path. */ -export async function prepareOutputRouterReply( - text: string, - options?: { signal?: AbortSignal }, -): Promise { - return prepareAssistantReply({ - completeObject: (args) => - completeObject({ - ...args, - ...(options?.signal ? { signal: options.signal } : undefined), - }), - fastModelId: resolveFastModelId(), - text, - }); -} - -/** Lightweight vitest-evals harness for isolated visible-reply prepare cases. */ -export const outputRouterHarness = createHarness< - OutputRouterEvalInput, - OutputRouterEvalOutput ->({ - name: "output-router", - run: async ({ input, signal }) => { - const timeoutSignal = AbortSignal.timeout(OUTPUT_ROUTER_EVAL_TIMEOUT_MS); - const prepareSignal = signal - ? AbortSignal.any([signal, timeoutSignal]) - : timeoutSignal; - const prepared = await prepareOutputRouterReply(input.text, { - signal: prepareSignal, - }); - const kind: OutputRouterExpectedKind = - prepared.kind === "silent" ? "silent" : "reply"; - const text = prepared.kind === "reply" ? prepared.text : null; - const output: OutputRouterEvalOutput = { - costUsd: prepared.costUsd ?? null, - expectedKind: input.expectedKind, - kind, - reason: prepared.reason, - text, - textChars: text?.length ?? null, - }; - - if (kind !== input.expectedKind) { - throw new Error( - `Prepare path decided ${kind} (${prepared.reason}); expected ${input.expectedKind}`, - ); - } - if ( - kind === "reply" && - input.maxVisibleChars !== undefined && - (text?.length ?? 0) > input.maxVisibleChars - ) { - throw new Error( - `Visible reply length ${text?.length ?? 0} exceeds maxVisibleChars ${input.maxVisibleChars}`, - ); - } - - return { - output, - events: [ - { - type: "message", - role: "user", - content: [ - `Expected: ${input.expectedKind}`, - ...(input.maxVisibleChars !== undefined - ? [`Max visible chars: ${input.maxVisibleChars}`] - : []), - "Assistant message:", - input.text, - ].join("\n"), - }, - { - type: "message", - role: "assistant", - content: [ - `Kind: ${kind}`, - `Reason: ${prepared.reason}`, - ...(text !== null ? [`Text: ${text}`] : ["Text: null"]), - ].join("\n"), - }, - ], - usage: { - provider: "vercel-ai-gateway", - model: resolveFastModelId(), - ...(prepared.costUsd !== undefined - ? { metadata: { costUsd: prepared.costUsd } } - : {}), - }, - }; - }, -}); - -/** Shared vitest-evals suite options for isolated visible-reply prepare evals. */ -export const outputRouterEvals = { - harness: outputRouterHarness, - // Exact kind match is asserted in the harness; no rubric judge. - judges: [], - judgeThreshold: null, -} satisfies DescribeEvalOptions< - OutputRouterEvalInput, - OutputRouterEvalOutput, - typeof outputRouterHarness ->; diff --git a/packages/junior-evals/src/output-router-setup.ts b/packages/junior-evals/src/output-router-setup.ts index eb1a740609..a560bc86ef 100644 --- a/packages/junior-evals/src/output-router-setup.ts +++ b/packages/junior-evals/src/output-router-setup.ts @@ -1,6 +1,22 @@ +import { beforeEach } from "vitest"; +import { setExperimentalFeatures } from "@/chat/experimental"; + /** - * Per-file setup for isolated visible-reply prepare evals. - * - * Kept intentionally empty beyond documenting the boundary: these cases must - * not depend on Slack egress, Postgres, Redis fixture resets, or MSW. + * Opt this suite into the visible-reply prepare path. + * Other eval suites keep output-router off. */ +const OUTPUT_ROUTER_SUITE_EXPERIMENTAL = { + "output-router": true, + "passive-routing": true, + subagents: true, +} as const; + +function restoreOutputRouterExperimentalFeatures(): void { + setExperimentalFeatures(OUTPUT_ROUTER_SUITE_EXPERIMENTAL); +} + +restoreOutputRouterExperimentalFeatures(); + +beforeEach(() => { + restoreOutputRouterExperimentalFeatures(); +}); diff --git a/packages/junior-evals/vitest.evals.output-router.config.ts b/packages/junior-evals/vitest.evals.output-router.config.ts index 668e1d9b4c..6a28f724c6 100644 --- a/packages/junior-evals/vitest.evals.output-router.config.ts +++ b/packages/junior-evals/vitest.evals.output-router.config.ts @@ -9,8 +9,9 @@ const workspaceRoot = path.resolve(__dirname, "../.."); const evalsPackageRoot = __dirname; const pluginApiPackageRoot = path.resolve(__dirname, "../junior-plugin-api"); const memoryPackageRoot = path.resolve(__dirname, "../junior-memory"); -// Leave room for provider retry inside the separate 60-second prepare budget. -const OUTPUT_ROUTER_EVAL_TEST_TIMEOUT_MS = 90_000; +// Leave room for harness cleanup and rubric judging after a reply reaches its +// separate 60-second behavior budget. +const EVAL_TEST_TIMEOUT_MS = 120_000; const evalReportPath = path.resolve( evalsPackageRoot, process.env.VITEST_EVALS_OUTPUT_FILE ?? "output-router-results.json", @@ -23,13 +24,25 @@ loadJuniorTestEnvFiles({ process.env.JUNIOR_SECRET = "junior-test-secret"; process.env.JUNIOR_BASE_URL ??= "https://junior.example.com"; -// These cases do not touch Redis state, but keep a loopback default so any -// accidental shared import that reads REDIS_URL stays sandboxed. process.env.JUNIOR_STATE_ADAPTER = "redis"; process.env.JUNIOR_STATE_KEY_PREFIX ??= `junior:eval-output-router:${randomUUID()}`; process.env.REDIS_URL = process.env.JUNIOR_EVAL_REDIS_URL?.trim() || "redis://127.0.0.1:6382"; -process.env.AI_FAST_MODEL ??= "openai/gpt-5.6-luna"; +const evalRedisHostname = new URL(process.env.REDIS_URL).hostname; +if (evalRedisHostname !== "localhost" && evalRedisHostname !== "127.0.0.1") { + throw new Error( + `JUNIOR_EVAL_REDIS_URL must point at localhost or 127.0.0.1, got ${evalRedisHostname}`, + ); +} +process.env.AI_MODEL = "xai/grok-4.5"; +// Prepare path uses the fast model on scripted assistant text. +process.env.AI_FAST_MODEL = "openai/gpt-5.6-luna"; +process.env.AI_GUARDIAN_MODEL = "openai/gpt-5.6-luna"; +process.env.AI_HANDOFF_MODEL = "openai/gpt-5.6-sol"; +process.env.AI_MODEL_PROFILES = JSON.stringify({ + coding: "openai/gpt-5.6-sol", +}); +process.env.VITEST_EVALS_REPLAY_MODE ??= "auto"; export default defineConfig({ resolve: { @@ -48,12 +61,19 @@ export default defineConfig({ test: { environment: "node", fileParallelism: false, - globalSetup: [path.resolve(__dirname, "output-router-global-setup.ts")], + // Full conversation harness: Postgres, Redis, gateway, sandbox egress. + globalSetup: [path.resolve(__dirname, "global-setup.ts")], include: ["evals/output-router/**/*.eval.ts"], maxWorkers: 1, - setupFiles: [path.resolve(__dirname, "src/output-router-setup.ts")], + setupFiles: [ + path.resolve(__dirname, "src/setup.ts"), + path.resolve(juniorPackageRoot, "tests/msw/setup.ts"), + path.resolve(juniorPackageRoot, "tests/fixtures/postgres/setup.ts"), + // Enable the prepare path after the shared experimental defaults. + path.resolve(__dirname, "src/output-router-setup.ts"), + ], outputFile: { json: evalReportPath }, reporters: [new DefaultEvalReporter(), "json"], - testTimeout: OUTPUT_ROUTER_EVAL_TEST_TIMEOUT_MS, + testTimeout: EVAL_TEST_TIMEOUT_MS, }, }); diff --git a/policies/evals.md b/policies/evals.md index 2a5ae480cb..711b51993e 100644 --- a/policies/evals.md +++ b/policies/evals.md @@ -13,9 +13,8 @@ Suite policy: CI gates on the aggregate suite floor, not a single weak case. - **Guardian** (`evals/guardian/**`): isolated action-review snapshots with exact `allow` / `ask` / `deny` assertions. Failures are hard pass/fail. -- **Visible-reply prepare** (`evals/output-router/**`): isolated prepare - snapshots with exact `silent` / `reply` assertions. Failures are hard - pass/fail. +- **Visible-reply prepare** (`evals/output-router/**`): full-runtime conversation + cases for the optional prepare path. Failures are hard pass/fail. ## Policy @@ -24,7 +23,7 @@ Suite policy: - Put never-break full-runtime integration coverage under `evals/integration/**`. Put agent-behavior measurement under behavioral domain folders. Put isolated action-review snapshots under `evals/guardian/**`. - Put isolated visible-reply prepare snapshots under `evals/output-router/**`. + Put visible-reply prepare conversation cases under `evals/output-router/**`. - Do not patch product prompts with eval-shaped examples, fixture names, exact user messages, expected answers, or distinctive scenario phrases from eval files. From 3e4289d7ae88f8ead2af943e1d993ee31ddb78e0 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Sat, 29 Aug 2026 01:00:18 +0000 Subject: [PATCH 07/14] feat(chat): keep SOUL personality in output-router rewrites Pass JUNIOR_PERSONALITY into the prepare-assistant system prompt so shortened visible replies still match the bot voice. Co-Authored-By: David Cramer --- .../src/content/docs/reference/config-and-env.md | 6 +++--- packages/junior/src/chat/README.md | 5 +++-- packages/junior/src/chat/services/output-router.ts | 12 +++++++++--- .../junior/tests/unit/services/output-router.test.ts | 7 +++++++ 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/packages/docs/src/content/docs/reference/config-and-env.md b/packages/docs/src/content/docs/reference/config-and-env.md index f502cdd864..209aa2ea7f 100644 --- a/packages/docs/src/content/docs/reference/config-and-env.md +++ b/packages/docs/src/content/docs/reference/config-and-env.md @@ -176,9 +176,9 @@ pre-stable surface. `output-router` uses the fast model (`AI_FAST_MODEL`) to prepare the visible reply for each completed tool-free assistant message. Exact `[[NO_REPLY]]` stays silent. Mixed marker text is judged (status-only chatter can stay silent; a real -answer that mentions the marker still delivers). Long replies can be shortened. -The original agent text remains in conversation history. Leave it unset unless -you are testing that path. +answer that mentions the marker still delivers). Long replies can be shortened +while keeping the `SOUL.md` personality voice. The original agent text remains +in conversation history. Leave it unset unless you are testing that path. `passive-routing` turns on replies to non-mention messages in threads Junior already joined. Leave it unset in production unless you are testing that path. diff --git a/packages/junior/src/chat/README.md b/packages/junior/src/chat/README.md index b2219de336..0f2ad1736a 100644 --- a/packages/junior/src/chat/README.md +++ b/packages/junior/src/chat/README.md @@ -22,8 +22,9 @@ file. provider adapters deliver, then commit that agent message before the visible reply in one transaction. When experimental `output-router` is enabled, a fast-model pass may change only the visible reply text (silence, cleanup, or - shortening). The original agent message stays in history. Tool-bearing - assistant text remains internal to the agent loop. + shortening) while keeping the `SOUL.md` personality voice. The original agent + message stays in history. Tool-bearing assistant text remains internal to the + agent loop. 7. The completed run result supplies diagnostics and artifacts; successful delivery or intentional no-reply completion commits the durable turn outcome. diff --git a/packages/junior/src/chat/services/output-router.ts b/packages/junior/src/chat/services/output-router.ts index 861f0f8bc4..4451b1e854 100644 --- a/packages/junior/src/chat/services/output-router.ts +++ b/packages/junior/src/chat/services/output-router.ts @@ -8,6 +8,7 @@ import { withSpan, type LogContext, } from "@/chat/logging"; +import { JUNIOR_PERSONALITY } from "@/chat/prompt"; import { decideReply, sanitizeAssistantText, @@ -63,13 +64,13 @@ type CompleteObject = (args: { * Prompt design: * - task first, short imperative rules * - one structured output contract - * - no extra context, no roleplay + * - personality from SOUL.md so rewrites keep the bot's voice * OpenAI structured outputs + short instructions; Anthropic: be direct. */ -function buildSystemPrompt(): string { +function buildSystemPrompt(personality: string = JUNIOR_PERSONALITY): string { return [ "Edit one assistant message into the final user-visible reply.", - "You receive only that message. No other context.", + "You receive only that message. No other conversation context.", "", "Return JSON:", "- text: the visible reply, or null for no visible reply", @@ -84,6 +85,11 @@ function buildSystemPrompt(): string { "- If the reply is too long, shorten it. Keep the answer, key facts, links, and next steps. Do not add facts.", "- Prefer 1-5 short sentences when shortening.", "- Do not add a preface or meta commentary.", + "- These rules override personality when they conflict.", + "", + "# Personality", + "When you keep or rewrite text, match this voice and tone:", + personality.trim(), ].join("\n"); } diff --git a/packages/junior/tests/unit/services/output-router.test.ts b/packages/junior/tests/unit/services/output-router.test.ts index 9644588c59..53a7acf432 100644 --- a/packages/junior/tests/unit/services/output-router.test.ts +++ b/packages/junior/tests/unit/services/output-router.test.ts @@ -1,6 +1,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import type { AssistantMessage } from "@earendil-works/pi-ai"; import { NO_REPLY_MARKER } from "@/chat/no-reply"; +import { JUNIOR_PERSONALITY } from "@/chat/prompt"; import { OUTPUT_REPLY_HARD_MAX_CHARS, prepareAssistantMessage, @@ -158,8 +159,14 @@ describe("prepare assistant reply", () => { promptName: "junior.prepare_assistant_reply", temperature: 0, thinkingLevel: "low", + system: expect.stringContaining(JUNIOR_PERSONALITY.trim()), }), ); + const system = completeObject.mock.calls[0]?.[0]?.system as string; + expect(system).toContain("# Personality"); + expect(system).toContain( + "These rules override personality when they conflict.", + ); }); it("keeps the original text when the model call fails", async () => { From 903a47da5b8341c5f7856c39fc23a2d2dcec70dd Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Sat, 29 Aug 2026 01:02:11 +0000 Subject: [PATCH 08/14] test(chat): keep output-router unit coverage minimal Drop model-judgment and static prompt string cases. Evals own reply quality; unit tests cover only local fixed prepare rules. Co-Authored-By: David Cramer --- .../tests/unit/services/output-router.test.ts | 128 ++---------------- 1 file changed, 13 insertions(+), 115 deletions(-) diff --git a/packages/junior/tests/unit/services/output-router.test.ts b/packages/junior/tests/unit/services/output-router.test.ts index 53a7acf432..11fbc05236 100644 --- a/packages/junior/tests/unit/services/output-router.test.ts +++ b/packages/junior/tests/unit/services/output-router.test.ts @@ -1,7 +1,6 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import type { AssistantMessage } from "@earendil-works/pi-ai"; import { NO_REPLY_MARKER } from "@/chat/no-reply"; -import { JUNIOR_PERSONALITY } from "@/chat/prompt"; import { OUTPUT_REPLY_HARD_MAX_CHARS, prepareAssistantMessage, @@ -9,17 +8,6 @@ import { prepareAssistantReplyLocal, } from "@/chat/services/output-router"; -const mocks = vi.hoisted(() => ({ - logInfo: vi.fn(), - logWarn: vi.fn(), -})); - -vi.mock("@/chat/logging", async (importOriginal) => ({ - ...(await importOriginal()), - logInfo: mocks.logInfo, - logWarn: mocks.logWarn, -})); - function assistant(text: string, withToolCall = false): AssistantMessage { return { role: "assistant", @@ -53,120 +41,34 @@ function assistant(text: string, withToolCall = false): AssistantMessage { } describe("prepare assistant reply", () => { - beforeEach(() => { - vi.clearAllMocks(); - }); - - it("stays silent for exact no-reply markers without a model call", async () => { - const completeObject = vi.fn(); - await expect( - prepareAssistantReply({ - completeObject, - fastModelId: "openai/gpt-5.6-luna", - text: NO_REPLY_MARKER, - }), - ).resolves.toEqual({ + it("handles empty and exact silence markers locally", () => { + expect(prepareAssistantReplyLocal("")).toEqual({ + kind: "silent", + reason: "empty", + }); + expect(prepareAssistantReplyLocal(NO_REPLY_MARKER)).toEqual({ kind: "silent", reason: "no_reply", }); - expect(completeObject).not.toHaveBeenCalled(); - }); - - it("does not decide mixed no-reply markers locally", () => { - // Mixed marker cases need model judgment: silence for status-only chatter, - // keep answer when the marker is only mentioned in a real reply. + // Mixed marker text needs model judgment. expect( prepareAssistantReplyLocal(`shipped it ${NO_REPLY_MARKER}\nmore detail`), ).toBeNull(); }); - it("asks the fast model for mixed marker messages", async () => { - const completeObject = vi.fn(async () => ({ - costUsd: 0.0002, - object: { - text: null, - reason: "status only, intentional silence", - }, - })); - + it("skips the model for exact silence markers", async () => { + const completeObject = vi.fn(); await expect( prepareAssistantReply({ completeObject, fastModelId: "openai/gpt-5.6-luna", - text: [ - "Same baseline miss — not caused by this PR. No PR fix.", - "", - NO_REPLY_MARKER, - ].join("\n"), + text: NO_REPLY_MARKER, }), ).resolves.toEqual({ kind: "silent", - reason: "status only, intentional silence", - costUsd: 0.0002, - }); - expect(completeObject).toHaveBeenCalledOnce(); - }); - - it("keeps explanations that mention the silence marker", async () => { - const explanation = [ - `Intentional silence uses the exact whole-message marker ${NO_REPLY_MARKER}.`, - "Normal answers that mention the marker should still post.", - ].join(" "); - const completeObject = vi.fn(async () => ({ - object: { - text: explanation, - reason: "explains silence protocol", - }, - })); - - await expect( - prepareAssistantReply({ - completeObject, - fastModelId: "openai/gpt-5.6-luna", - text: explanation, - }), - ).resolves.toEqual({ - kind: "reply", - text: explanation, - reason: "explains silence protocol", - }); - }); - - it("asks the fast model to shorten long replies", async () => { - const completeObject = vi.fn(async () => ({ - costUsd: 0.0004, - object: { - text: "Short answer with the outcome.", - reason: "too long", - }, - })); - - const prepared = await prepareAssistantReply({ - completeObject, - fastModelId: "openai/gpt-5.6-luna", - text: "A".repeat(900), - }); - - expect(prepared).toEqual({ - kind: "reply", - text: "Short answer with the outcome.", - reason: "too long", - costUsd: 0.0004, + reason: "no_reply", }); - expect(completeObject).toHaveBeenCalledWith( - expect.objectContaining({ - modelId: "openai/gpt-5.6-luna", - promptName: "junior.prepare_assistant_reply", - temperature: 0, - thinkingLevel: "low", - system: expect.stringContaining(JUNIOR_PERSONALITY.trim()), - }), - ); - const system = completeObject.mock.calls[0]?.[0]?.system as string; - expect(system).toContain("# Personality"); - expect(system).toContain( - "These rules override personality when they conflict.", - ); + expect(completeObject).not.toHaveBeenCalled(); }); it("keeps the original text when the model call fails", async () => { @@ -185,10 +87,6 @@ describe("prepare assistant reply", () => { text: "Keep this answer.", reason: "prepare_failed", }); - expect(mocks.logWarn).toHaveBeenCalledWith( - "ai.output_router.failed", - expect.objectContaining({ "exception.message": "boom" }), - ); }); it("caps oversized model text", async () => { From ce7617fdeac8d13a789c7ca76575206dbe597ffe Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Sat, 29 Aug 2026 01:06:50 +0000 Subject: [PATCH 09/14] refactor(evals): share full-runtime suite config Extract createFullRuntimeEvalConfig so behavioral, integration, and output-router suites only declare name/include/env. Drop the one-off output-router setup file in favor of JUNIOR_EVAL_OUTPUT_ROUTER. Co-Authored-By: David Cramer --- .github/workflows/evals-behavioral.yml | 1 + .github/workflows/evals-integration.yml | 1 + .github/workflows/evals-output-router.yml | 3 +- packages/junior-evals/README.md | 5 +- .../create-full-runtime-eval-config.ts | 109 ++++++++++++++++++ .../junior-evals/src/output-router-setup.ts | 22 ---- .../vitest.evals.behavioral.config.ts | 94 ++------------- .../vitest.evals.integration.config.ts | 81 +------------ .../vitest.evals.output-router.config.ts | 87 ++------------ .../tests/fixtures/experimental-setup.ts | 7 +- 10 files changed, 149 insertions(+), 261 deletions(-) create mode 100644 packages/junior-evals/create-full-runtime-eval-config.ts delete mode 100644 packages/junior-evals/src/output-router-setup.ts diff --git a/.github/workflows/evals-behavioral.yml b/.github/workflows/evals-behavioral.yml index c97ac2850f..fab2d58cb1 100644 --- a/.github/workflows/evals-behavioral.yml +++ b/.github/workflows/evals-behavioral.yml @@ -52,6 +52,7 @@ jobs: - 'packages/junior-evals/src/slack-link.ts' - 'packages/junior-evals/src/snapshot-warmup.ts' - 'packages/junior-evals/tests/**' + - 'packages/junior-evals/create-full-runtime-eval-config.ts' - 'packages/junior-evals/vitest.evals.config.ts' - 'packages/junior-evals/vitest.evals.behavioral.config.ts' diff --git a/.github/workflows/evals-integration.yml b/.github/workflows/evals-integration.yml index 1e5eb8d57e..6c54382460 100644 --- a/.github/workflows/evals-integration.yml +++ b/.github/workflows/evals-integration.yml @@ -41,6 +41,7 @@ jobs: - 'packages/junior-evals/src/setup.ts' - 'packages/junior-evals/src/slack-link.ts' - 'packages/junior-evals/src/snapshot-warmup.ts' + - 'packages/junior-evals/create-full-runtime-eval-config.ts' - 'packages/junior-evals/vitest.evals.integration.config.ts' - id: decision env: diff --git a/.github/workflows/evals-output-router.yml b/.github/workflows/evals-output-router.yml index 8a7e05cdac..dbff0e52d8 100644 --- a/.github/workflows/evals-output-router.yml +++ b/.github/workflows/evals-output-router.yml @@ -30,7 +30,7 @@ jobs: filters: | relevant: - 'packages/junior-evals/evals/output-router/**' - - 'packages/junior-evals/src/output-router-setup.ts' + - 'packages/junior-evals/create-full-runtime-eval-config.ts' - 'packages/junior-evals/src/behavior-harness.ts' - 'packages/junior-evals/src/helpers.ts' - 'packages/junior-evals/src/setup.ts' @@ -44,6 +44,7 @@ jobs: - 'packages/junior/src/chat/services/output-router.ts' - 'packages/junior/src/chat/agent/index.ts' - 'packages/junior/src/chat/experimental.ts' + - 'packages/junior/tests/fixtures/experimental-setup.ts' - id: decision env: AI_GATEWAY_API_KEY: ${{ secrets.AI_GATEWAY_API_KEY }} diff --git a/packages/junior-evals/README.md b/packages/junior-evals/README.md index 02e5758334..c37b6ec05e 100644 --- a/packages/junior-evals/README.md +++ b/packages/junior-evals/README.md @@ -63,6 +63,9 @@ Not in scope: - Helpers and event builders: `src/helpers.ts` - Guardian harness: `src/guardian-harness.ts` - Harness/runtime adapter: `src/behavior-harness.ts` +- Shared full-runtime suite config: `create-full-runtime-eval-config.ts` + (behavioral, integration, and output-router). Guardian stays on its own + lightweight config. ## Execution Model @@ -137,7 +140,7 @@ Pass eval file paths, `-t` filters, and shard options directly after the suite s - Behavioral path triggers cover domain folders under `evals/{agent,conversation,github,memory,scheduler,sentry}/` and shared harness/config files under `packages/junior-evals/`. - Integration path triggers cover `evals/integration/**`, the integration config, and shared harness files under `packages/junior-evals/`. - Guardian path triggers cover `evals/guardian/**`, the Guardian harness/config under `packages/junior-evals/`, and `packages/junior/src/chat/services/guardian-action-policy.ts`. -- Output-router path triggers cover `evals/output-router/**`, shared conversation harness/config under `packages/junior-evals/`, `packages/junior/src/chat/services/output-router.ts`, and the agent delivery wire-up. +- Output-router path triggers cover `evals/output-router/**`, shared full-runtime harness/config under `packages/junior-evals/`, `packages/junior/src/chat/services/output-router.ts`, and the agent delivery wire-up. - Other product source under `packages/junior/src/**` does not auto-run evals; use a `trigger-evals*` label for that. - Behavioral shards still fail individual cases under the per-case judge threshold (`0.75`), but the workflow no longer fails the shard job on those case failures alone. Each behavioral shard, Guardian job, and output-router job publishes its own `vitest-evals` job summary (pass rate, scores, quality misses). - After all behavioral shards finish, `behavioral / report` combines results, writes the aggregate job summary, and publishes a `behavioral / score` Check Run. The Check Run title carries the gate line (for example `Eval pass rate 90.2% — floor 80.0%`). When that check publishes, the report step soft-fails so the Check Run owns green/red instead of canned job failure text. diff --git a/packages/junior-evals/create-full-runtime-eval-config.ts b/packages/junior-evals/create-full-runtime-eval-config.ts new file mode 100644 index 0000000000..9c79379972 --- /dev/null +++ b/packages/junior-evals/create-full-runtime-eval-config.ts @@ -0,0 +1,109 @@ +import { randomUUID } from "node:crypto"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { defineConfig } from "vitest/config"; +import DefaultEvalReporter from "vitest-evals/reporter"; +import { loadJuniorTestEnvFiles } from "../junior/tests/fixtures/env"; + +const evalsPackageRoot = path.dirname(fileURLToPath(import.meta.url)); +const juniorPackageRoot = path.resolve(evalsPackageRoot, "../junior"); +const workspaceRoot = path.resolve(evalsPackageRoot, "../.."); +const pluginApiPackageRoot = path.resolve( + evalsPackageRoot, + "../junior-plugin-api", +); +const memoryPackageRoot = path.resolve(evalsPackageRoot, "../junior-memory"); + +// Leave room for harness cleanup and rubric judging after a reply reaches its +// separate 60-second behavior budget. +const EVAL_TEST_TIMEOUT_MS = 120_000; + +export type FullRuntimeEvalSuiteOptions = { + /** Suite id used for Redis key prefix and default results file name. */ + name: string; + include: string[]; + exclude?: string[]; + /** Extra setup files after the shared full-runtime setup chain. */ + setupFiles?: string[]; + env?: Record; +}; + +/** + * Shared Vitest config for full Slack/runtime eval suites. + * + * Suite configs stay thin: name, include/exclude, and optional env/setup only. + * Guardian stays on its own lightweight config. + */ +export function createFullRuntimeEvalConfig( + options: FullRuntimeEvalSuiteOptions, +) { + const evalReportPath = path.resolve( + evalsPackageRoot, + process.env.VITEST_EVALS_OUTPUT_FILE ?? `${options.name}-results.json`, + ); + + loadJuniorTestEnvFiles({ + workspaceRoot, + packageRoots: [juniorPackageRoot, evalsPackageRoot], + }); + + process.env.JUNIOR_SECRET = "junior-test-secret"; + process.env.JUNIOR_BASE_URL ??= "https://junior.example.com"; + process.env.JUNIOR_STATE_ADAPTER = "redis"; + process.env.JUNIOR_STATE_KEY_PREFIX ??= `junior:eval-${options.name}:${randomUUID()}`; + process.env.REDIS_URL = + process.env.JUNIOR_EVAL_REDIS_URL?.trim() || "redis://127.0.0.1:6382"; + const evalRedisHostname = new URL(process.env.REDIS_URL).hostname; + if (evalRedisHostname !== "localhost" && evalRedisHostname !== "127.0.0.1") { + throw new Error( + `JUNIOR_EVAL_REDIS_URL must point at localhost or 127.0.0.1, got ${evalRedisHostname}`, + ); + } + process.env.AI_MODEL = "xai/grok-4.5"; + process.env.AI_FAST_MODEL = "anthropic/claude-haiku-4.5"; + process.env.AI_GUARDIAN_MODEL = "openai/gpt-5.6-luna"; + process.env.AI_HANDOFF_MODEL = "openai/gpt-5.6-sol"; + process.env.AI_MODEL_PROFILES = JSON.stringify({ + coding: "openai/gpt-5.6-sol", + }); + process.env.VITEST_EVALS_REPLAY_MODE ??= "auto"; + + for (const [key, value] of Object.entries(options.env ?? {})) { + process.env[key] = value; + } + + return defineConfig({ + resolve: { + alias: { + "@": path.resolve(juniorPackageRoot, "src"), + "@sentry/junior-memory": path.resolve(memoryPackageRoot, "src/index.ts"), + "@sentry/junior-plugin-api": path.resolve( + pluginApiPackageRoot, + "src/index.ts", + ), + }, + // Vite 8 resolves tsconfig `paths` natively here: + // https://vite.dev/config/shared-options.html#resolve-tsconfigpaths + // The aliases above keep workspace package internals on source instead of package dist. + tsconfigPaths: true, + }, + test: { + environment: "node", + fileParallelism: false, + globalSetup: [path.resolve(evalsPackageRoot, "global-setup.ts")], + include: options.include, + ...(options.exclude ? { exclude: options.exclude } : undefined), + maxWorkers: 1, + setupFiles: [ + path.resolve(evalsPackageRoot, "src/setup.ts"), + path.resolve(juniorPackageRoot, "tests/msw/setup.ts"), + path.resolve(juniorPackageRoot, "tests/fixtures/postgres/setup.ts"), + path.resolve(juniorPackageRoot, "tests/fixtures/experimental-setup.ts"), + ...(options.setupFiles ?? []), + ], + outputFile: { json: evalReportPath }, + reporters: [new DefaultEvalReporter(), "json"], + testTimeout: EVAL_TEST_TIMEOUT_MS, + }, + }); +} diff --git a/packages/junior-evals/src/output-router-setup.ts b/packages/junior-evals/src/output-router-setup.ts deleted file mode 100644 index a560bc86ef..0000000000 --- a/packages/junior-evals/src/output-router-setup.ts +++ /dev/null @@ -1,22 +0,0 @@ -import { beforeEach } from "vitest"; -import { setExperimentalFeatures } from "@/chat/experimental"; - -/** - * Opt this suite into the visible-reply prepare path. - * Other eval suites keep output-router off. - */ -const OUTPUT_ROUTER_SUITE_EXPERIMENTAL = { - "output-router": true, - "passive-routing": true, - subagents: true, -} as const; - -function restoreOutputRouterExperimentalFeatures(): void { - setExperimentalFeatures(OUTPUT_ROUTER_SUITE_EXPERIMENTAL); -} - -restoreOutputRouterExperimentalFeatures(); - -beforeEach(() => { - restoreOutputRouterExperimentalFeatures(); -}); diff --git a/packages/junior-evals/vitest.evals.behavioral.config.ts b/packages/junior-evals/vitest.evals.behavioral.config.ts index 63fc14fb6e..4624851e71 100644 --- a/packages/junior-evals/vitest.evals.behavioral.config.ts +++ b/packages/junior-evals/vitest.evals.behavioral.config.ts @@ -1,83 +1,13 @@ -import { defineConfig } from "vitest/config"; -import { randomUUID } from "node:crypto"; -import DefaultEvalReporter from "vitest-evals/reporter"; -import path from "node:path"; -import { loadJuniorTestEnvFiles } from "../junior/tests/fixtures/env"; - -const juniorPackageRoot = path.resolve(__dirname, "../junior"); -const workspaceRoot = path.resolve(__dirname, "../.."); -const evalsPackageRoot = __dirname; -const pluginApiPackageRoot = path.resolve(__dirname, "../junior-plugin-api"); -const memoryPackageRoot = path.resolve(__dirname, "../junior-memory"); -// Leave room for harness cleanup and rubric judging after a reply reaches its -// separate 60-second behavior budget. -const EVAL_TEST_TIMEOUT_MS = 120_000; -const evalReportPath = path.resolve( - evalsPackageRoot, - process.env.VITEST_EVALS_OUTPUT_FILE ?? "behavioral-results.json", -); - -loadJuniorTestEnvFiles({ - workspaceRoot, - packageRoots: [juniorPackageRoot, evalsPackageRoot], -}); - -process.env.JUNIOR_SECRET = "junior-test-secret"; -process.env.JUNIOR_BASE_URL ??= "https://junior.example.com"; -process.env.JUNIOR_STATE_ADAPTER = "redis"; -process.env.JUNIOR_STATE_KEY_PREFIX ??= `junior:eval-behavioral:${randomUUID()}`; -process.env.REDIS_URL = - process.env.JUNIOR_EVAL_REDIS_URL?.trim() || "redis://127.0.0.1:6382"; -const evalRedisHostname = new URL(process.env.REDIS_URL).hostname; -if (evalRedisHostname !== "localhost" && evalRedisHostname !== "127.0.0.1") { - throw new Error( - `JUNIOR_EVAL_REDIS_URL must point at localhost or 127.0.0.1, got ${evalRedisHostname}`, - ); -} -process.env.AI_MODEL = "xai/grok-4.5"; -process.env.AI_FAST_MODEL = "anthropic/claude-haiku-4.5"; -process.env.AI_GUARDIAN_MODEL = "openai/gpt-5.6-luna"; -process.env.AI_HANDOFF_MODEL = "openai/gpt-5.6-sol"; -process.env.AI_MODEL_PROFILES = JSON.stringify({ - coding: "openai/gpt-5.6-sol", -}); -process.env.VITEST_EVALS_REPLAY_MODE ??= "auto"; - -export default defineConfig({ - resolve: { - alias: { - "@": path.resolve(juniorPackageRoot, "src"), - "@sentry/junior-memory": path.resolve(memoryPackageRoot, "src/index.ts"), - "@sentry/junior-plugin-api": path.resolve( - pluginApiPackageRoot, - "src/index.ts", - ), - }, - // Vite 8 resolves tsconfig `paths` natively here: - // https://vite.dev/config/shared-options.html#resolve-tsconfigpaths - // The aliases above keep workspace package internals on source instead of package dist. - tsconfigPaths: true, - }, - test: { - environment: "node", - fileParallelism: false, - globalSetup: [path.resolve(__dirname, "global-setup.ts")], - // Behavioral quality cases. Integration and Guardian suites have their own configs. - include: ["evals/**/*.eval.ts"], - exclude: [ - "evals/guardian/**", - "evals/integration/**", - "evals/output-router/**", - ], - maxWorkers: 1, - setupFiles: [ - path.resolve(__dirname, "src/setup.ts"), - path.resolve(juniorPackageRoot, "tests/msw/setup.ts"), - path.resolve(juniorPackageRoot, "tests/fixtures/postgres/setup.ts"), - path.resolve(juniorPackageRoot, "tests/fixtures/experimental-setup.ts"), - ], - outputFile: { json: evalReportPath }, - reporters: [new DefaultEvalReporter(), "json"], - testTimeout: EVAL_TEST_TIMEOUT_MS, - }, +import { createFullRuntimeEvalConfig } from "./create-full-runtime-eval-config"; + +// Behavioral quality cases. Integration, Guardian, and output-router have their +// own suite configs. +export default createFullRuntimeEvalConfig({ + name: "behavioral", + include: ["evals/**/*.eval.ts"], + exclude: [ + "evals/guardian/**", + "evals/integration/**", + "evals/output-router/**", + ], }); diff --git a/packages/junior-evals/vitest.evals.integration.config.ts b/packages/junior-evals/vitest.evals.integration.config.ts index 904c987fba..18d3a5c748 100644 --- a/packages/junior-evals/vitest.evals.integration.config.ts +++ b/packages/junior-evals/vitest.evals.integration.config.ts @@ -1,78 +1,7 @@ -import { defineConfig } from "vitest/config"; -import { randomUUID } from "node:crypto"; -import DefaultEvalReporter from "vitest-evals/reporter"; -import path from "node:path"; -import { loadJuniorTestEnvFiles } from "../junior/tests/fixtures/env"; +import { createFullRuntimeEvalConfig } from "./create-full-runtime-eval-config"; -const juniorPackageRoot = path.resolve(__dirname, "../junior"); -const workspaceRoot = path.resolve(__dirname, "../.."); -const evalsPackageRoot = __dirname; -const pluginApiPackageRoot = path.resolve(__dirname, "../junior-plugin-api"); -const memoryPackageRoot = path.resolve(__dirname, "../junior-memory"); -// Leave room for harness cleanup and rubric judging after a reply reaches its -// separate 60-second behavior budget. -const EVAL_TEST_TIMEOUT_MS = 120_000; -const evalReportPath = path.resolve( - evalsPackageRoot, - process.env.VITEST_EVALS_OUTPUT_FILE ?? "integration-results.json", -); - -loadJuniorTestEnvFiles({ - workspaceRoot, - packageRoots: [juniorPackageRoot, evalsPackageRoot], -}); - -process.env.JUNIOR_SECRET = "junior-test-secret"; -process.env.JUNIOR_BASE_URL ??= "https://junior.example.com"; -process.env.JUNIOR_STATE_ADAPTER = "redis"; -process.env.JUNIOR_STATE_KEY_PREFIX ??= `junior:eval-integration:${randomUUID()}`; -process.env.REDIS_URL = - process.env.JUNIOR_EVAL_REDIS_URL?.trim() || "redis://127.0.0.1:6382"; -const evalRedisHostname = new URL(process.env.REDIS_URL).hostname; -if (evalRedisHostname !== "localhost" && evalRedisHostname !== "127.0.0.1") { - throw new Error( - `JUNIOR_EVAL_REDIS_URL must point at localhost or 127.0.0.1, got ${evalRedisHostname}`, - ); -} -process.env.AI_MODEL = "xai/grok-4.5"; -process.env.AI_FAST_MODEL = "anthropic/claude-haiku-4.5"; -process.env.AI_GUARDIAN_MODEL = "openai/gpt-5.6-luna"; -process.env.AI_HANDOFF_MODEL = "openai/gpt-5.6-sol"; -process.env.AI_MODEL_PROFILES = JSON.stringify({ - coding: "openai/gpt-5.6-sol", -}); -process.env.VITEST_EVALS_REPLAY_MODE ??= "auto"; - -export default defineConfig({ - resolve: { - alias: { - "@": path.resolve(juniorPackageRoot, "src"), - "@sentry/junior-memory": path.resolve(memoryPackageRoot, "src/index.ts"), - "@sentry/junior-plugin-api": path.resolve( - pluginApiPackageRoot, - "src/index.ts", - ), - }, - // Vite 8 resolves tsconfig `paths` natively here: - // https://vite.dev/config/shared-options.html#resolve-tsconfigpaths - // The aliases above keep workspace package internals on source instead of package dist. - tsconfigPaths: true, - }, - test: { - environment: "node", - fileParallelism: false, - globalSetup: [path.resolve(__dirname, "global-setup.ts")], - // Strict system-correctness cases. Any failure fails the suite hard. - include: ["evals/integration/**/*.eval.ts"], - maxWorkers: 1, - setupFiles: [ - path.resolve(__dirname, "src/setup.ts"), - path.resolve(juniorPackageRoot, "tests/msw/setup.ts"), - path.resolve(juniorPackageRoot, "tests/fixtures/postgres/setup.ts"), - path.resolve(juniorPackageRoot, "tests/fixtures/experimental-setup.ts"), - ], - outputFile: { json: evalReportPath }, - reporters: [new DefaultEvalReporter(), "json"], - testTimeout: EVAL_TEST_TIMEOUT_MS, - }, +// Strict system-correctness cases. Any failure fails the suite hard. +export default createFullRuntimeEvalConfig({ + name: "integration", + include: ["evals/integration/**/*.eval.ts"], }); diff --git a/packages/junior-evals/vitest.evals.output-router.config.ts b/packages/junior-evals/vitest.evals.output-router.config.ts index 6a28f724c6..b0c685bd75 100644 --- a/packages/junior-evals/vitest.evals.output-router.config.ts +++ b/packages/junior-evals/vitest.evals.output-router.config.ts @@ -1,79 +1,12 @@ -import { defineConfig } from "vitest/config"; -import { randomUUID } from "node:crypto"; -import DefaultEvalReporter from "vitest-evals/reporter"; -import path from "node:path"; -import { loadJuniorTestEnvFiles } from "../junior/tests/fixtures/env"; - -const juniorPackageRoot = path.resolve(__dirname, "../junior"); -const workspaceRoot = path.resolve(__dirname, "../.."); -const evalsPackageRoot = __dirname; -const pluginApiPackageRoot = path.resolve(__dirname, "../junior-plugin-api"); -const memoryPackageRoot = path.resolve(__dirname, "../junior-memory"); -// Leave room for harness cleanup and rubric judging after a reply reaches its -// separate 60-second behavior budget. -const EVAL_TEST_TIMEOUT_MS = 120_000; -const evalReportPath = path.resolve( - evalsPackageRoot, - process.env.VITEST_EVALS_OUTPUT_FILE ?? "output-router-results.json", -); - -loadJuniorTestEnvFiles({ - workspaceRoot, - packageRoots: [juniorPackageRoot, evalsPackageRoot], -}); - -process.env.JUNIOR_SECRET = "junior-test-secret"; -process.env.JUNIOR_BASE_URL ??= "https://junior.example.com"; -process.env.JUNIOR_STATE_ADAPTER = "redis"; -process.env.JUNIOR_STATE_KEY_PREFIX ??= `junior:eval-output-router:${randomUUID()}`; -process.env.REDIS_URL = - process.env.JUNIOR_EVAL_REDIS_URL?.trim() || "redis://127.0.0.1:6382"; -const evalRedisHostname = new URL(process.env.REDIS_URL).hostname; -if (evalRedisHostname !== "localhost" && evalRedisHostname !== "127.0.0.1") { - throw new Error( - `JUNIOR_EVAL_REDIS_URL must point at localhost or 127.0.0.1, got ${evalRedisHostname}`, - ); -} -process.env.AI_MODEL = "xai/grok-4.5"; -// Prepare path uses the fast model on scripted assistant text. -process.env.AI_FAST_MODEL = "openai/gpt-5.6-luna"; -process.env.AI_GUARDIAN_MODEL = "openai/gpt-5.6-luna"; -process.env.AI_HANDOFF_MODEL = "openai/gpt-5.6-sol"; -process.env.AI_MODEL_PROFILES = JSON.stringify({ - coding: "openai/gpt-5.6-sol", -}); -process.env.VITEST_EVALS_REPLAY_MODE ??= "auto"; - -export default defineConfig({ - resolve: { - alias: { - "@": path.resolve(juniorPackageRoot, "src"), - "@sentry/junior-memory": path.resolve(memoryPackageRoot, "src/index.ts"), - "@sentry/junior-plugin-api": path.resolve( - pluginApiPackageRoot, - "src/index.ts", - ), - }, - // Vite 8 resolves tsconfig `paths` natively here: - // https://vite.dev/config/shared-options.html#resolve-tsconfigpaths - tsconfigPaths: true, - }, - test: { - environment: "node", - fileParallelism: false, - // Full conversation harness: Postgres, Redis, gateway, sandbox egress. - globalSetup: [path.resolve(__dirname, "global-setup.ts")], - include: ["evals/output-router/**/*.eval.ts"], - maxWorkers: 1, - setupFiles: [ - path.resolve(__dirname, "src/setup.ts"), - path.resolve(juniorPackageRoot, "tests/msw/setup.ts"), - path.resolve(juniorPackageRoot, "tests/fixtures/postgres/setup.ts"), - // Enable the prepare path after the shared experimental defaults. - path.resolve(__dirname, "src/output-router-setup.ts"), - ], - outputFile: { json: evalReportPath }, - reporters: [new DefaultEvalReporter(), "json"], - testTimeout: EVAL_TEST_TIMEOUT_MS, +import { createFullRuntimeEvalConfig } from "./create-full-runtime-eval-config"; + +// Full conversation cases for the optional prepare path. +export default createFullRuntimeEvalConfig({ + name: "output-router", + include: ["evals/output-router/**/*.eval.ts"], + env: { + // Prepare path uses the fast model on scripted assistant text. + AI_FAST_MODEL: "openai/gpt-5.6-luna", + JUNIOR_EVAL_OUTPUT_ROUTER: "1", }, }); diff --git a/packages/junior/tests/fixtures/experimental-setup.ts b/packages/junior/tests/fixtures/experimental-setup.ts index dcd60cf9ad..3b2cc7784f 100644 --- a/packages/junior/tests/fixtures/experimental-setup.ts +++ b/packages/junior/tests/fixtures/experimental-setup.ts @@ -2,11 +2,14 @@ import { beforeEach } from "vitest"; import { setExperimentalFeatures } from "@/chat/experimental"; /** - * Production leaves experimental features off. The suite opts in so coverage + * Production leaves experimental features off. Suites opt in so coverage * exercises the real wiring path without an env flag. + * + * Full-runtime eval suites can enable the prepare path with + * `JUNIOR_EVAL_OUTPUT_ROUTER=1` before this setup file loads. */ export const SUITE_EXPERIMENTAL = { - "output-router": false, + "output-router": process.env.JUNIOR_EVAL_OUTPUT_ROUTER === "1", "passive-routing": true, subagents: true, } as const; From b7e732594c37dde13271ab73e50fef229e54e9d3 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Sat, 29 Aug 2026 01:23:23 +0000 Subject: [PATCH 10/14] refactor(evals): isolate output-router as a function-call suite Mirror guardian: harness calls prepareAssistantReply directly with real assistant text. Drop full Slack/runtime deps from the dedicated suite. Co-Authored-By: David Cramer --- .github/workflows/evals-output-router.yml | 60 +----- packages/junior-evals/README.md | 30 +-- .../create-full-runtime-eval-config.ts | 2 +- packages/junior-evals/evals/github-actions.md | 8 +- .../evals/output-router/visible-reply.eval.ts | 138 +++++--------- .../output-router-global-setup.ts | 15 ++ .../junior-evals/src/output-router-harness.ts | 175 ++++++++++++++++++ .../junior-evals/src/output-router-setup.ts | 6 + .../vitest.evals.behavioral.config.ts | 4 +- .../vitest.evals.output-router.config.ts | 68 ++++++- .../tests/fixtures/experimental-setup.ts | 9 +- policies/evals.md | 7 +- 12 files changed, 337 insertions(+), 185 deletions(-) create mode 100644 packages/junior-evals/output-router-global-setup.ts create mode 100644 packages/junior-evals/src/output-router-harness.ts create mode 100644 packages/junior-evals/src/output-router-setup.ts diff --git a/.github/workflows/evals-output-router.yml b/.github/workflows/evals-output-router.yml index dbff0e52d8..8290fb1776 100644 --- a/.github/workflows/evals-output-router.yml +++ b/.github/workflows/evals-output-router.yml @@ -30,37 +30,24 @@ jobs: filters: | relevant: - 'packages/junior-evals/evals/output-router/**' - - 'packages/junior-evals/create-full-runtime-eval-config.ts' - - 'packages/junior-evals/src/behavior-harness.ts' - - 'packages/junior-evals/src/helpers.ts' - - 'packages/junior-evals/src/setup.ts' + - 'packages/junior-evals/src/output-router-harness.ts' + - 'packages/junior-evals/src/output-router-setup.ts' - 'packages/junior-evals/src/eval-ai-gateway-dispatcher.ts' - - 'packages/junior-evals/src/eval-context.ts' - - 'packages/junior-evals/src/eval-egress.ts' - - 'packages/junior-evals/global-setup.ts' - - 'packages/junior-evals/postgres-global-setup.ts' + - 'packages/junior-evals/output-router-global-setup.ts' - 'packages/junior-evals/vitest.evals.output-router.config.ts' - 'packages/junior-evals/package.json' - 'packages/junior/src/chat/services/output-router.ts' - - 'packages/junior/src/chat/agent/index.ts' - - 'packages/junior/src/chat/experimental.ts' - - 'packages/junior/tests/fixtures/experimental-setup.ts' - id: decision env: AI_GATEWAY_API_KEY: ${{ secrets.AI_GATEWAY_API_KEY }} VERCEL_OIDC_TOKEN: ${{ secrets.VERCEL_OIDC_TOKEN }} - VERCEL_TOKEN: ${{ secrets.VERCEL_TOKEN }} - VERCEL_TEAM_ID: ${{ secrets.VERCEL_TEAM_ID }} - VERCEL_PROJECT_ID: ${{ secrets.VERCEL_PROJECT_ID }} RELEVANT: ${{ steps.changes.outputs.relevant }} PR_LABELS: ${{ join(github.event.pull_request.labels.*.name, ',') }} run: | set -euo pipefail gateway_ready=false - sandbox_ready=false requested=false [[ -n "${AI_GATEWAY_API_KEY:-}" || -n "${VERCEL_OIDC_TOKEN:-}" ]] && gateway_ready=true - [[ -n "${VERCEL_OIDC_TOKEN:-}" || ( -n "${VERCEL_TOKEN:-}" && -n "${VERCEL_TEAM_ID:-}" && -n "${VERCEL_PROJECT_ID:-}" ) ]] && sandbox_ready=true IFS=',' read -r -a labels <<< "${PR_LABELS:-}" for label in "${labels[@]}"; do if [[ "$label" == "trigger-evals" || "$label" == "trigger-evals-output-router" ]]; then @@ -68,7 +55,7 @@ jobs: fi done should_run=false - [[ "$gateway_ready" == "true" && "$sandbox_ready" == "true" && ( "$RELEVANT" == "true" || "$requested" == "true" ) ]] && should_run=true + [[ "$gateway_ready" == "true" && ( "$RELEVANT" == "true" || "$requested" == "true" ) ]] && should_run=true echo "should_run=$should_run" >> "$GITHUB_OUTPUT" { echo "## Output-router eval selection" @@ -76,7 +63,6 @@ jobs: echo "- relevant_files_changed: $RELEVANT" echo "- requested: $requested" echo "- gateway_ready: $gateway_ready" - echo "- sandbox_ready: $sandbox_ready" echo "- will_run: $should_run" } >> "$GITHUB_STEP_SUMMARY" @@ -85,50 +71,12 @@ jobs: needs: select if: needs.select.outputs.should_run == 'true' runs-on: blacksmith-4vcpu-ubuntu-2404 - services: - postgres: - image: pgvector/pgvector:pg17 - env: - POSTGRES_USER: junior - POSTGRES_PASSWORD: junior - POSTGRES_DB: junior - ports: - - 5432:5432 - options: >- - --health-cmd "pg_isready -U junior -d junior" - --health-interval 10s - --health-timeout 5s - --health-retries 5 - redis: - image: redis:7-alpine - ports: - - 6379:6379 - options: >- - --health-cmd "redis-cli ping" - --health-interval 10s - --health-timeout 5s - --health-retries 5 env: - JUNIOR_EVAL_REDIS_URL: redis://127.0.0.1:6379 - DATABASE_URL: postgres://junior:junior@localhost:5432/junior AI_GATEWAY_API_KEY: ${{ secrets.AI_GATEWAY_API_KEY }} - VERCEL_TOKEN: ${{ secrets.VERCEL_TOKEN }} - VERCEL_TEAM_ID: ${{ secrets.VERCEL_TEAM_ID }} - VERCEL_PROJECT_ID: ${{ secrets.VERCEL_PROJECT_ID }} VERCEL_OIDC_TOKEN: ${{ secrets.VERCEL_OIDC_TOKEN }} steps: - uses: actions/checkout@v4 - uses: ./.github/actions/setup-node-pnpm - - name: Install cloudflared - run: | - set -euo pipefail - curl --fail --location --silent --show-error \ - https://github.com/cloudflare/cloudflared/releases/download/2026.7.2/cloudflared-linux-amd64 \ - --output "$RUNNER_TEMP/cloudflared" - echo "ec905ea7b7e327ff8abdde8cb64697a2152de74dbcdbf6aec9db8364eb3886cd $RUNNER_TEMP/cloudflared" | sha256sum --check - chmod +x "$RUNNER_TEMP/cloudflared" - echo "$RUNNER_TEMP" >> "$GITHUB_PATH" - "$RUNNER_TEMP/cloudflared" version - name: Run output-router evals id: run continue-on-error: true diff --git a/packages/junior-evals/README.md b/packages/junior-evals/README.md index c37b6ec05e..ee715258c9 100644 --- a/packages/junior-evals/README.md +++ b/packages/junior-evals/README.md @@ -9,7 +9,7 @@ There are four independently runnable suites: 1. **Integration** (`evals/integration/**`) — full agent/runtime runs for primary system functionality that should never regress. Failures are hard pass/fail. 2. **Behavioral** (domain folders under `evals/` except `integration/`, `guardian/`, and `output-router/`) — full agent/runtime runs that measure agent behavior and tolerate bounded variability. CI reports a suite score and only blocks below the configured floor. 3. **Guardian** (`evals/guardian/**`) — isolated action-review snapshots scored only on `allow` / `ask` / `deny`. Failures are hard pass/fail. -4. **Visible-reply prepare** (`evals/output-router/**`) — full Slack/runtime conversation cases for the optional prepare path. Failures are hard pass/fail. +4. **Visible-reply prepare** (`evals/output-router/**`) — isolated `prepareAssistantReply` snapshots scored on `silent` / `reply`. Failures are hard pass/fail. - We define conversation cases inline in TypeScript using `describeEval()` and the shared `slackEvals` harness options. - We run the real runtime/harness against those fixtures. @@ -58,14 +58,15 @@ Not in scope: - `evals/sentry/` - Isolated Guardian decisions: `evals/guardian/` - exact `ToolActionProposal` snapshots scored only on `allow` / `ask` / `deny` -- Visible-reply prepare conversation cases: `evals/output-router/` - - scripted assistant text through the real prepare + delivery path +- Isolated visible-reply prepare snapshots: `evals/output-router/` + - one assistant message through `prepareAssistantReply` - Helpers and event builders: `src/helpers.ts` - Guardian harness: `src/guardian-harness.ts` +- Output-router harness: `src/output-router-harness.ts` - Harness/runtime adapter: `src/behavior-harness.ts` - Shared full-runtime suite config: `create-full-runtime-eval-config.ts` - (behavioral, integration, and output-router). Guardian stays on its own - lightweight config. + (behavioral and integration). Guardian and output-router stay on their own + lightweight configs. ## Execution Model @@ -112,15 +113,16 @@ Tool replay: - `pnpm evals` / `pnpm evals:behavioral`: Run the behavioral suite - `pnpm evals:integration`: Run the integration suite - `pnpm evals:guardian`: Run isolated Guardian action-review snapshots -- `pnpm evals:output-router`: Run visible-reply prepare conversation cases +- `pnpm evals:output-router`: Run isolated visible-reply prepare snapshots - `pnpm --filter @sentry/junior-evals evals:behavioral`: Run behavioral from any directory - `pnpm --filter @sentry/junior-evals evals:integration`: Run integration from any directory - `pnpm --filter @sentry/junior-evals evals:guardian`: Run Guardian from any directory -- `pnpm --filter @sentry/junior-evals evals:output-router`: Run visible-reply prepare conversation cases from any directory +- `pnpm --filter @sentry/junior-evals evals:output-router`: Run isolated prepare snapshots from any directory - `pnpm --filter @sentry/junior-evals evals:behavioral evals/sentry/skills.eval.ts`: Run one behavioral file - `pnpm --filter @sentry/junior-evals evals:integration evals/integration/conversation/actions.eval.ts`: Run one integration file - `pnpm --filter @sentry/junior-evals evals:guardian evals/guardian/action-review.eval.ts -t "deny"`: Run one Guardian case - `pnpm --filter @sentry/junior-evals evals:output-router evals/output-router/visible-reply.eval.ts`: Run one prepare file +- `pnpm --filter @sentry/junior-evals evals:output-router evals/output-router/visible-reply.eval.ts -t "silent"`: Run one prepare case - `pnpm --filter @sentry/junior-evals evals:behavioral --shard=1/4`: Run one of the four CI behavioral shards Pass eval file paths, `-t` filters, and shard options directly after the suite script. Do not use `pnpm exec vitest` directly, and do not insert `--` before eval arguments. @@ -131,37 +133,37 @@ Pass eval file paths, `-t` filters, and shard options directly after the suite s - `Behavioral evals`: Slack/agent evals (`behavioral / shard *` + `behavioral / report` → `behavioral / score` Check Run) - `Integration evals`: system evals (`integration / shard *`) - `Guardian evals`: isolated action-review snapshots (`guardian / run`) - - `Output-router evals`: visible-reply prepare conversation cases (`output-router / run`) + - `Output-router evals`: isolated prepare snapshots (`output-router / run`) - Suite labels follow `trigger-evals-[domain]`: - `trigger-evals` starts all suites - `trigger-evals-behavioral`, `trigger-evals-integration`, `trigger-evals-guardian`, and `trigger-evals-output-router` start one suite -- Behavioral, integration, and output-router evals require both gateway and sandbox secrets. Guardian only needs gateway credentials. +- Behavioral and integration evals require both gateway and sandbox secrets. Guardian and output-router only need gateway credentials. - Adding a trigger label fires immediately; unrelated labels do not. - Behavioral path triggers cover domain folders under `evals/{agent,conversation,github,memory,scheduler,sentry}/` and shared harness/config files under `packages/junior-evals/`. - Integration path triggers cover `evals/integration/**`, the integration config, and shared harness files under `packages/junior-evals/`. - Guardian path triggers cover `evals/guardian/**`, the Guardian harness/config under `packages/junior-evals/`, and `packages/junior/src/chat/services/guardian-action-policy.ts`. -- Output-router path triggers cover `evals/output-router/**`, shared full-runtime harness/config under `packages/junior-evals/`, `packages/junior/src/chat/services/output-router.ts`, and the agent delivery wire-up. +- Output-router path triggers cover `evals/output-router/**`, the prepare harness/config under `packages/junior-evals/`, and `packages/junior/src/chat/services/output-router.ts`. - Other product source under `packages/junior/src/**` does not auto-run evals; use a `trigger-evals*` label for that. - Behavioral shards still fail individual cases under the per-case judge threshold (`0.75`), but the workflow no longer fails the shard job on those case failures alone. Each behavioral shard, Guardian job, and output-router job publishes its own `vitest-evals` job summary (pass rate, scores, quality misses). - After all behavioral shards finish, `behavioral / report` combines results, writes the aggregate job summary, and publishes a `behavioral / score` Check Run. The Check Run title carries the gate line (for example `Eval pass rate 90.2% — floor 80.0%`). When that check publishes, the report step soft-fails so the Check Run owns green/red instead of canned job failure text. - The behavioral floor is `EVAL_MIN_PASS_RATE=0.8` (`80%` of cases passed). `vitest-evals@0.16` owns the aggregate gate math; individual case misses are warnings when the floor still passes. Missing shard result files or setup/runtime crashes before results are written remain hard failures on the report job. - Integration cases fail the `integration / shard *` jobs hard on any miss. They do not use the aggregate pass-rate floor. - Guardian cases assert exact `allow` / `ask` / `deny` decisions and fail the `guardian / run` job hard on mismatch. They do not use the aggregate pass-rate floor. -- Output-router cases assert delivered conversation replies (or silence) and fail the `output-router / run` job hard on mismatch. They do not use the aggregate pass-rate floor. +- Output-router cases assert prepare `silent` / `reply` outcomes and fail the `output-router / run` job hard on mismatch. They do not use the aggregate pass-rate floor. - The simplest Gateway and Sandbox setup is `VERCEL_OIDC_TOKEN` alone. - The fallback CI setup is `AI_GATEWAY_API_KEY` plus `VERCEL_TOKEN` + `VERCEL_TEAM_ID` + `VERCEL_PROJECT_ID`. - Behavioral and integration global setup starts one Cloudflare Quick Tunnel for the suite so Vercel Sandbox can reach the eval egress proxy. Transient tunnel allocation failures retry up to five times with backoff. Local runs require `cloudflared` on `PATH`; CI installs a pinned binary. - Behavioral and integration state always uses a loopback Redis. Local runs default to `redis://127.0.0.1:6382`; CI sets `JUNIOR_EVAL_REDIS_URL` for its Redis service. - Setup details for GitHub Actions live in `evals/github-actions.md`. -Behavioral, integration, and output-router evals require real Vercel Sandbox access and public Quick Tunnel connectivity. If either bootstrap fails, the eval fails immediately with no local fallback path. Guardian evals only need AI Gateway access. +Behavioral and integration evals require real Vercel Sandbox access and public Quick Tunnel connectivity. If either bootstrap fails, the eval fails immediately with no local fallback path. Guardian and output-router evals only need AI Gateway access. ## Authoring Rules - Put full-runtime integration cases that must never regress under `evals/integration/**` using `describeEval()` with `slackEvals`. Prefer deterministic assertions; keep criteria only when the case still needs light quality scoring. - Put behavioral cases under `evals/conversation/`, `evals/agent/`, or `evals//` using `describeEval()` with `slackEvals`. - Add isolated Guardian decision snapshots under `evals/guardian/` using `describeEval()` with `guardianEvals`. Feed exact `ToolActionProposal` objects and assert only the expected `allow` / `ask` / `deny` decision. -- Add visible-reply prepare conversation cases under `evals/output-router/` using `describeEval()` with `slackEvals`. Prefer scripted `reply_texts` from real transcripts, then assert what actually posts after prepare. +- Add isolated visible-reply prepare snapshots under `evals/output-router/` using `describeEval()` with `outputRouterEvals`. Feed real assistant-message text and assert `silent` or `reply`. - Put messages that should be pending before processing starts in `initialEvents`. - Put ordinary later events in `events`; each is delivered after preceding work settles. - Wrap messages with `steer(...)` when they should arrive through normal ingress while the preceding agent run is active. @@ -219,7 +221,7 @@ Organize files by suite policy first, then by the user-visible area they exercis - `evals/integration/`: strict full-runtime integration cases (hard pass/fail). - `evals/conversation/`, `evals/agent/`, `evals//`: agent-behavior cases (score-gated in CI). - `evals/guardian/`: isolated action-review snapshots (no main agent; hard pass/fail). -- `evals/output-router/`: visible-reply prepare conversation cases (full runtime; hard pass/fail). +- `evals/output-router/`: isolated prepare snapshots (no main agent; hard pass/fail). - Use short behavior nouns for filenames: `routing.eval.ts`, `delivery.eval.ts`, `credentials.eval.ts`. - Keep one coherent behavior area per file. Split files when cases exercise independently understandable journeys. - Keep shared setup in a nearby `helpers.ts`; helpers are not eval files and do not define suites. diff --git a/packages/junior-evals/create-full-runtime-eval-config.ts b/packages/junior-evals/create-full-runtime-eval-config.ts index 9c79379972..b9277b6aa1 100644 --- a/packages/junior-evals/create-full-runtime-eval-config.ts +++ b/packages/junior-evals/create-full-runtime-eval-config.ts @@ -32,7 +32,7 @@ export type FullRuntimeEvalSuiteOptions = { * Shared Vitest config for full Slack/runtime eval suites. * * Suite configs stay thin: name, include/exclude, and optional env/setup only. - * Guardian stays on its own lightweight config. + * Guardian and output-router stay on their own lightweight configs. */ export function createFullRuntimeEvalConfig( options: FullRuntimeEvalSuiteOptions, diff --git a/packages/junior-evals/evals/github-actions.md b/packages/junior-evals/evals/github-actions.md index e6f1cae510..e84ff99b2f 100644 --- a/packages/junior-evals/evals/github-actions.md +++ b/packages/junior-evals/evals/github-actions.md @@ -70,11 +70,11 @@ Four independent workflows run on pull requests: - `Behavioral evals` runs Slack/agent evals when behavioral eval files/harness changed or the PR has `trigger-evals-behavioral` / `trigger-evals` - `Integration evals` runs system evals when integration eval files/harness changed or the PR has `trigger-evals-integration` / `trigger-evals` - `Guardian evals` runs isolated action-review snapshots when Guardian eval files/harness changed, Guardian policy changed, or the PR has `trigger-evals-guardian` / `trigger-evals` -- `Output-router evals` runs visible-reply prepare conversation cases when those eval files/harness changed, `output-router.ts` changed, or the PR has `trigger-evals-output-router` / `trigger-evals` +- `Output-router evals` runs isolated prepare snapshots when those eval files/harness changed, `output-router.ts` changed, or the PR has `trigger-evals-output-router` / `trigger-evals` Suite labels follow `trigger-evals-[domain]`. Adding a trigger label fires immediately. If the label is already on the PR, future `synchronize` events still run the matching suite(s). Product source under `packages/junior/src/**` does not auto-run evals, except Guardian policy changes in `packages/junior/src/chat/services/guardian-action-policy.ts` and prepare-path changes in `packages/junior/src/chat/services/output-router.ts`. -Guardian evals only need gateway credentials. Behavioral, integration, and output-router evals still need gateway plus sandbox access. +Guardian and output-router evals only need gateway credentials. Behavioral and integration evals still need gateway plus sandbox access. ## Verification @@ -86,7 +86,7 @@ After adding secrets: 4. For behavioral runs, confirm each `behavioral / shard *` job has a shard summary, `behavioral / report` has the combined summary, and the `behavioral / score` Check Run shows the pass-rate gate title. 5. For integration runs, confirm the `integration / shard *` jobs completed. Any case miss fails those jobs hard. 6. For Guardian runs, confirm the `guardian / run` job summary published and the job completed. Exact decision mismatches fail that job hard. -7. For output-router runs, confirm the `output-router / run` job summary published and the job completed. Delivered-reply or silence mismatches fail that job hard. +7. For output-router runs, confirm the `output-router / run` job summary published and the job completed. Prepare `silent` / `reply` mismatches fail that job hard. ## Score-Based CI Gate @@ -102,7 +102,7 @@ If Check Run publishing is skipped or fails, the report step still fails on a re When the aggregate gate passes, individual case misses are warnings rather than failures. Setup crashes and missing result files still fail the report job hard. -Integration shards fail hard on any case miss and do not use the aggregate floor. Guardian snapshots assert exact `allow` / `ask` / `deny` decisions, publish their own job summary, and fail `guardian / run` on mismatch. Output-router conversation cases assert what posts after prepare, publish their own job summary, and fail `output-router / run` on mismatch. +Integration shards fail hard on any case miss and do not use the aggregate floor. Guardian snapshots assert exact `allow` / `ask` / `deny` decisions, publish their own job summary, and fail `guardian / run` on mismatch. Output-router snapshots assert prepare `silent` / `reply` outcomes, publish their own job summary, and fail `output-router / run` on mismatch. If `sandbox_ready` is false, either `VERCEL_OIDC_TOKEN` is missing or the fallback token set is incomplete. diff --git a/packages/junior-evals/evals/output-router/visible-reply.eval.ts b/packages/junior-evals/evals/output-router/visible-reply.eval.ts index 5860f6c3e2..fb49b58384 100644 --- a/packages/junior-evals/evals/output-router/visible-reply.eval.ts +++ b/packages/junior-evals/evals/output-router/visible-reply.eval.ts @@ -1,23 +1,16 @@ /** - * Conversation coverage for the optional visible-reply prepare path. + * Isolated visible-reply prepare corpus. * - * These cases run the full Slack/runtime harness with scripted assistant - * text, then assert what actually posts after prepare. Fixtures are - * transcript-shaped (real long answers, silence tags, protocol explanations). + * Each case feeds real assistant-message text into prepareAssistantReply and + * asserts silent vs reply. This suite does not run the main agent or Slack + * transport. Delivery wiring is covered elsewhere. */ import { describeEval } from "vitest-evals"; -import { expect } from "vitest"; import { NO_REPLY_MARKER } from "@/chat/no-reply"; import { OUTPUT_REPLY_SOFT_MAX_CHARS } from "@/chat/services/output-router"; -import { - mention, - rubric, - slackEvals, - visibleAssistantText, - visibleThreadReplies, -} from "../../src/helpers"; +import { outputRouterEvals } from "../../src/output-router-harness"; -/** Real long steering comparison that should not post as a wall of text. */ +/** Real long steering comparison that should not remain a wall of text. */ const LONG_STEERING_ESSAY = [ "**yeah — steering is the weaker half of this comparison.** openclaw treats mid-run guidance as the default path; junior treats it as a gated special case. that mismatch is the reliability gap.", "", @@ -76,99 +69,62 @@ const LONG_STEERING_ESSAY = [ "want me to turn that into a concrete junior issue/PR plan?", ].join("\n"); -describeEval("Visible Reply Prepare", slackEvals, (it) => { - it("when the assistant writes a long explanatory essay, post a short reply", async ({ +/** Real maintain-PR status chatter that should stay silent. */ +const STATUS_ONLY_WITH_MARKER = [ + "Same main baseline miss on createAgentDispatchWorkRouter — not caused by this PR. No PR fix.", + "", + NO_REPLY_MARKER, +].join("\n"); + +/** Real explanation of silence that must stay a reply. */ +const SILENCE_PROTOCOL_EXPLANATION = [ + `Intentional silence uses the exact whole-message marker ${NO_REPLY_MARKER}.`, + "If the marker is only mentioned in a normal answer, that answer should still post.", + "Only a message that is exactly the marker stays silent.", +].join(" "); + +describeEval("Visible Reply Prepare", outputRouterEvals, (it) => { + it("when the assistant writes a long explanatory essay, return a short reply", async ({ run, }) => { - expect(LONG_STEERING_ESSAY.length).toBeGreaterThan( - OUTPUT_REPLY_SOFT_MAX_CHARS, - ); + if (LONG_STEERING_ESSAY.length <= OUTPUT_REPLY_SOFT_MAX_CHARS) { + throw new Error("fixture must exceed the soft max length"); + } - const result = await run({ - overrides: { - reply_texts: [LONG_STEERING_ESSAY], - }, - initialEvents: [ - mention( - "how does junior steering compare to openclaw? keep it practical", - ), - ], - requireSandboxReady: false, - criteria: rubric({ - pass: [ - "The reply is short and readable for Slack (about a few sentences or a tight bullet list).", - "The reply still covers the core gap: junior steers less by default and later than openclaw.", - ], - fail: [ - "Do not post the full multi-section essay, markdown table, or long numbered parity plan as the visible reply.", - "Do not open with process narration about checking docs or writing an essay.", - ], - }), + await run({ + text: LONG_STEERING_ESSAY, + expectedKind: "reply", + maxChars: OUTPUT_REPLY_SOFT_MAX_CHARS, + mustInclude: ["steer"], + mustNotInclude: ["### what openclaw does", "| situation | junior |"], }); - - expect(visibleThreadReplies(result.session)).toHaveLength(1); - expect(visibleAssistantText(result.session).length).toBeLessThanOrEqual( - OUTPUT_REPLY_SOFT_MAX_CHARS, - ); }); it("when maintain work ends with status chatter and a silence marker, stay silent", async ({ run, }) => { - const result = await run({ - overrides: { - reply_texts: [ - [ - "Same main baseline miss on createAgentDispatchWorkRouter — not caused by this PR. No PR fix.", - "", - NO_REPLY_MARKER, - ].join("\n"), - ], - }, - initialEvents: [ - mention( - "check the PR checks for the guardian ordinary-writes change and only ping if something needs a fix", - ), - ], - requireSandboxReady: false, + await run({ + text: STATUS_ONLY_WITH_MARKER, + expectedKind: "silent", }); - - expect(visibleThreadReplies(result.session)).toEqual([]); - expect(visibleAssistantText(result.session)).not.toContain(NO_REPLY_MARKER); }); - it("when asked how silence works, keep the explanation visible", async ({ + it("when the message explains how silence works, keep the explanation", async ({ run, }) => { - const result = await run({ - overrides: { - reply_texts: [ - [ - `Intentional silence uses the exact whole-message marker ${NO_REPLY_MARKER}.`, - "If the marker is only mentioned in a normal answer, that answer should still post.", - "Only a message that is exactly the marker stays silent.", - ].join(" "), - ], - }, - initialEvents: [ - mention( - "how does junior's no-reply marker work? when does a message stay silent?", - ), - ], - requireSandboxReady: false, - criteria: rubric({ - pass: [ - "The reply explains that silence requires the whole message to be the marker, and that ordinary answers can still mention it.", - ], - fail: [ - "Do not stay silent or drop the explanation just because the marker string appears in the answer.", - ], - }), + await run({ + text: SILENCE_PROTOCOL_EXPLANATION, + expectedKind: "reply", + mustInclude: ["marker", "exact"], }); + }); - expect(visibleThreadReplies(result.session)).toHaveLength(1); - const text = visibleAssistantText(result.session); - expect(text.length).toBeGreaterThan(0); - expect(/silence|marker|exact/i.test(text)).toBe(true); + it("when the whole message is only the silence marker, stay silent", async ({ + run, + }) => { + await run({ + text: NO_REPLY_MARKER, + expectedKind: "silent", + }); }); }); diff --git a/packages/junior-evals/output-router-global-setup.ts b/packages/junior-evals/output-router-global-setup.ts new file mode 100644 index 0000000000..2715bbb216 --- /dev/null +++ b/packages/junior-evals/output-router-global-setup.ts @@ -0,0 +1,15 @@ +import { installEvalAiGatewayDispatcher } from "./src/eval-ai-gateway-dispatcher"; + +/** + * Set up the lightweight output-router eval invocation. + * + * These cases only need AI Gateway access. They intentionally skip Postgres, + * Redis fixtures, MSW, plugin catalogs, and sandbox egress. + */ +export default async function setup(): Promise<() => Promise> { + const restoreAiGatewayDispatcher = installEvalAiGatewayDispatcher(); + process.stdout.write( + "[evals:output-router] AI Gateway dispatcher ready (no sandbox egress)\n", + ); + return restoreAiGatewayDispatcher; +} diff --git a/packages/junior-evals/src/output-router-harness.ts b/packages/junior-evals/src/output-router-harness.ts new file mode 100644 index 0000000000..2748ab868b --- /dev/null +++ b/packages/junior-evals/src/output-router-harness.ts @@ -0,0 +1,175 @@ +/** + * Isolated visible-reply prepare harness. + * + * Feeds one assistant message text into prepareAssistantReply without the main + * agent, Slack transport, sandbox egress, or Postgres. + */ +import { + createHarness, + type DescribeEvalOptions, + type JsonValue, +} from "vitest-evals"; +import { completeObject } from "@/chat/pi/client"; +import { + OUTPUT_REPLY_SOFT_MAX_CHARS, + prepareAssistantReply, + type PreparedAssistantReply, +} from "@/chat/services/output-router"; + +export type OutputRouterEvalKind = "silent" | "reply"; + +export interface OutputRouterEvalInput { + /** Original assistant message text. */ + text: string; + /** Expected prepare kind. */ + expectedKind: OutputRouterEvalKind; + /** + * Optional upper bound for reply text length. Defaults to the soft max when + * expectedKind is reply and this is omitted. + */ + maxChars?: number; + /** Substrings that must appear in a reply (case-insensitive). */ + mustInclude?: string[]; + /** Substrings that must not appear in a reply (case-insensitive). */ + mustNotInclude?: string[]; +} + +export interface OutputRouterEvalOutput extends Record { + costUsd: number | null; + expectedKind: OutputRouterEvalKind; + kind: OutputRouterEvalKind; + reason: string; + text: string | null; + textLength: number | null; +} + +function resolveFastModelId(): string { + const configured = process.env.AI_FAST_MODEL?.trim(); + if (configured) { + return configured; + } + return "openai/gpt-5.6-luna"; +} + +function includesInsensitive(haystack: string, needle: string): boolean { + return haystack.toLowerCase().includes(needle.toLowerCase()); +} + +/** Run one assistant message through the production prepare boundary. */ +export async function prepareVisibleReply( + text: string, +): Promise { + return prepareAssistantReply({ + completeObject, + fastModelId: resolveFastModelId(), + text, + }); +} + +function assertPreparedReply( + input: OutputRouterEvalInput, + prepared: PreparedAssistantReply, +): void { + if (prepared.kind !== input.expectedKind) { + throw new Error( + `output-router prepared ${prepared.kind} (${prepared.reason}); expected ${input.expectedKind}`, + ); + } + + if (prepared.kind === "silent") { + return; + } + + const maxChars = input.maxChars ?? OUTPUT_REPLY_SOFT_MAX_CHARS; + if (prepared.text.length > maxChars) { + throw new Error( + `output-router reply length ${prepared.text.length} exceeds max ${maxChars}`, + ); + } + + for (const needle of input.mustInclude ?? []) { + if (!includesInsensitive(prepared.text, needle)) { + throw new Error( + `output-router reply missing required text ${JSON.stringify(needle)}`, + ); + } + } + + for (const needle of input.mustNotInclude ?? []) { + if (includesInsensitive(prepared.text, needle)) { + throw new Error( + `output-router reply contains forbidden text ${JSON.stringify(needle)}`, + ); + } + } +} + +/** Lightweight vitest-evals harness for isolated prepare cases. */ +export const outputRouterHarness = createHarness< + OutputRouterEvalInput, + OutputRouterEvalOutput +>({ + name: "output-router", + run: async ({ input }) => { + const prepared = await prepareVisibleReply(input.text); + assertPreparedReply(input, prepared); + + const output: OutputRouterEvalOutput = { + costUsd: prepared.costUsd ?? null, + expectedKind: input.expectedKind, + kind: prepared.kind, + reason: prepared.reason, + text: prepared.kind === "reply" ? prepared.text : null, + textLength: prepared.kind === "reply" ? prepared.text.length : null, + }; + + return { + output, + events: [ + { + type: "message", + role: "user", + content: [ + `Expected kind: ${input.expectedKind}`, + "", + "Original assistant text:", + input.text, + ].join("\n"), + }, + { + type: "message", + role: "assistant", + content: + prepared.kind === "silent" + ? [`Kind: silent`, `Reason: ${prepared.reason}`].join("\n") + : [ + `Kind: reply`, + `Reason: ${prepared.reason}`, + `Length: ${prepared.text.length}`, + "", + prepared.text, + ].join("\n"), + }, + ], + usage: { + provider: "vercel-ai-gateway", + model: resolveFastModelId(), + ...(prepared.costUsd !== undefined + ? { metadata: { costUsd: prepared.costUsd } } + : {}), + }, + }; + }, +}); + +/** Shared vitest-evals suite options for isolated prepare evals. */ +export const outputRouterEvals = { + harness: outputRouterHarness, + // Kind/length/content contracts are asserted in the harness; no rubric judge. + judges: [], + judgeThreshold: null, +} satisfies DescribeEvalOptions< + OutputRouterEvalInput, + OutputRouterEvalOutput, + typeof outputRouterHarness +>; diff --git a/packages/junior-evals/src/output-router-setup.ts b/packages/junior-evals/src/output-router-setup.ts new file mode 100644 index 0000000000..d45a226eab --- /dev/null +++ b/packages/junior-evals/src/output-router-setup.ts @@ -0,0 +1,6 @@ +/** + * Per-file setup for isolated output-router evals. + * + * Kept intentionally empty beyond documenting the boundary: these cases must + * not depend on Slack egress, Postgres, Redis fixture resets, or MSW. + */ diff --git a/packages/junior-evals/vitest.evals.behavioral.config.ts b/packages/junior-evals/vitest.evals.behavioral.config.ts index 4624851e71..c04f97e084 100644 --- a/packages/junior-evals/vitest.evals.behavioral.config.ts +++ b/packages/junior-evals/vitest.evals.behavioral.config.ts @@ -1,7 +1,7 @@ import { createFullRuntimeEvalConfig } from "./create-full-runtime-eval-config"; -// Behavioral quality cases. Integration, Guardian, and output-router have their -// own suite configs. +// Behavioral quality cases. Integration, Guardian, and output-router each have +// their own suite configs. export default createFullRuntimeEvalConfig({ name: "behavioral", include: ["evals/**/*.eval.ts"], diff --git a/packages/junior-evals/vitest.evals.output-router.config.ts b/packages/junior-evals/vitest.evals.output-router.config.ts index b0c685bd75..ed68696612 100644 --- a/packages/junior-evals/vitest.evals.output-router.config.ts +++ b/packages/junior-evals/vitest.evals.output-router.config.ts @@ -1,12 +1,60 @@ -import { createFullRuntimeEvalConfig } from "./create-full-runtime-eval-config"; - -// Full conversation cases for the optional prepare path. -export default createFullRuntimeEvalConfig({ - name: "output-router", - include: ["evals/output-router/**/*.eval.ts"], - env: { - // Prepare path uses the fast model on scripted assistant text. - AI_FAST_MODEL: "openai/gpt-5.6-luna", - JUNIOR_EVAL_OUTPUT_ROUTER: "1", +import { defineConfig } from "vitest/config"; +import { randomUUID } from "node:crypto"; +import DefaultEvalReporter from "vitest-evals/reporter"; +import path from "node:path"; +import { loadJuniorTestEnvFiles } from "../junior/tests/fixtures/env"; + +const juniorPackageRoot = path.resolve(__dirname, "../junior"); +const workspaceRoot = path.resolve(__dirname, "../.."); +const evalsPackageRoot = __dirname; +const pluginApiPackageRoot = path.resolve(__dirname, "../junior-plugin-api"); +const memoryPackageRoot = path.resolve(__dirname, "../junior-memory"); +// Leave room for provider retry inside the separate 60-second prepare budget. +const OUTPUT_ROUTER_EVAL_TEST_TIMEOUT_MS = 90_000; +const evalReportPath = path.resolve( + evalsPackageRoot, + process.env.VITEST_EVALS_OUTPUT_FILE ?? "output-router-results.json", +); + +loadJuniorTestEnvFiles({ + workspaceRoot, + packageRoots: [juniorPackageRoot, evalsPackageRoot], +}); + +process.env.JUNIOR_SECRET = "junior-test-secret"; +process.env.JUNIOR_BASE_URL ??= "https://junior.example.com"; +// Prepare cases do not touch Redis state, but keep a loopback default so any +// accidental shared import that reads REDIS_URL stays sandboxed. +process.env.JUNIOR_STATE_ADAPTER = "redis"; +process.env.JUNIOR_STATE_KEY_PREFIX ??= `junior:eval-output-router:${randomUUID()}`; +process.env.REDIS_URL = + process.env.JUNIOR_EVAL_REDIS_URL?.trim() || "redis://127.0.0.1:6382"; +// Prepare path uses the fast model on one assistant message. +process.env.AI_FAST_MODEL ??= "openai/gpt-5.6-luna"; + +export default defineConfig({ + resolve: { + alias: { + "@": path.resolve(juniorPackageRoot, "src"), + "@sentry/junior-memory": path.resolve(memoryPackageRoot, "src/index.ts"), + "@sentry/junior-plugin-api": path.resolve( + pluginApiPackageRoot, + "src/index.ts", + ), + }, + // Vite 8 resolves tsconfig `paths` natively here: + // https://vite.dev/config/shared-options.html#resolve-tsconfigpaths + tsconfigPaths: true, + }, + test: { + environment: "node", + fileParallelism: false, + globalSetup: [path.resolve(__dirname, "output-router-global-setup.ts")], + include: ["evals/output-router/**/*.eval.ts"], + maxWorkers: 1, + setupFiles: [path.resolve(__dirname, "src/output-router-setup.ts")], + outputFile: { json: evalReportPath }, + reporters: [new DefaultEvalReporter(), "json"], + testTimeout: OUTPUT_ROUTER_EVAL_TEST_TIMEOUT_MS, }, }); diff --git a/packages/junior/tests/fixtures/experimental-setup.ts b/packages/junior/tests/fixtures/experimental-setup.ts index 3b2cc7784f..adf1ddb4c5 100644 --- a/packages/junior/tests/fixtures/experimental-setup.ts +++ b/packages/junior/tests/fixtures/experimental-setup.ts @@ -2,14 +2,15 @@ import { beforeEach } from "vitest"; import { setExperimentalFeatures } from "@/chat/experimental"; /** - * Production leaves experimental features off. Suites opt in so coverage + * Production leaves experimental features off. The suite opts in so coverage * exercises the real wiring path without an env flag. * - * Full-runtime eval suites can enable the prepare path with - * `JUNIOR_EVAL_OUTPUT_ROUTER=1` before this setup file loads. + * Isolated output-router evals call prepareAssistantReply directly and do not + * use this file. Delivery wiring stays covered by full-runtime suites with + * output-router left off unless a case opts in explicitly. */ export const SUITE_EXPERIMENTAL = { - "output-router": process.env.JUNIOR_EVAL_OUTPUT_ROUTER === "1", + "output-router": false, "passive-routing": true, subagents: true, } as const; diff --git a/policies/evals.md b/policies/evals.md index 711b51993e..35a49a9e07 100644 --- a/policies/evals.md +++ b/policies/evals.md @@ -13,8 +13,9 @@ Suite policy: CI gates on the aggregate suite floor, not a single weak case. - **Guardian** (`evals/guardian/**`): isolated action-review snapshots with exact `allow` / `ask` / `deny` assertions. Failures are hard pass/fail. -- **Visible-reply prepare** (`evals/output-router/**`): full-runtime conversation - cases for the optional prepare path. Failures are hard pass/fail. +- **Visible-reply prepare** (`evals/output-router/**`): isolated prepare + snapshots over one assistant message (`silent` / `reply`). Failures are hard + pass/fail. ## Policy @@ -23,7 +24,7 @@ Suite policy: - Put never-break full-runtime integration coverage under `evals/integration/**`. Put agent-behavior measurement under behavioral domain folders. Put isolated action-review snapshots under `evals/guardian/**`. - Put visible-reply prepare conversation cases under `evals/output-router/**`. + Put isolated visible-reply prepare snapshots under `evals/output-router/**`. - Do not patch product prompts with eval-shaped examples, fixture names, exact user messages, expected answers, or distinctive scenario phrases from eval files. From cbd5ee66e41d95fdb4dc773017e725ed873d5eb9 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Sat, 29 Aug 2026 01:28:06 +0000 Subject: [PATCH 11/14] fix(chat): silence internal status notes in output-router Clarify that trailing NO_REPLY after internal work status is silent, keep protocol explanations as replies, and make eval failure text include the prepared output. Co-Authored-By: David Cramer --- .../evals/output-router/visible-reply.eval.ts | 3 ++- .../junior-evals/src/output-router-harness.ts | 15 +++++++++++---- .../junior/src/chat/services/output-router.ts | 7 ++++--- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/packages/junior-evals/evals/output-router/visible-reply.eval.ts b/packages/junior-evals/evals/output-router/visible-reply.eval.ts index fb49b58384..b4837ff41b 100644 --- a/packages/junior-evals/evals/output-router/visible-reply.eval.ts +++ b/packages/junior-evals/evals/output-router/visible-reply.eval.ts @@ -115,7 +115,8 @@ describeEval("Visible Reply Prepare", outputRouterEvals, (it) => { await run({ text: SILENCE_PROTOCOL_EXPLANATION, expectedKind: "reply", - mustInclude: ["marker", "exact"], + // Keep the marker when explaining the protocol; do not assert incidental wording. + mustInclude: [NO_REPLY_MARKER], }); }); diff --git a/packages/junior-evals/src/output-router-harness.ts b/packages/junior-evals/src/output-router-harness.ts index 2748ab868b..f350e6b76d 100644 --- a/packages/junior-evals/src/output-router-harness.ts +++ b/packages/junior-evals/src/output-router-harness.ts @@ -66,13 +66,20 @@ export async function prepareVisibleReply( }); } +function preparedSummary(prepared: PreparedAssistantReply): string { + if (prepared.kind === "silent") { + return `kind=silent reason=${JSON.stringify(prepared.reason)}`; + } + return `kind=reply reason=${JSON.stringify(prepared.reason)} text=${JSON.stringify(prepared.text)}`; +} + function assertPreparedReply( input: OutputRouterEvalInput, prepared: PreparedAssistantReply, ): void { if (prepared.kind !== input.expectedKind) { throw new Error( - `output-router prepared ${prepared.kind} (${prepared.reason}); expected ${input.expectedKind}`, + `output-router prepared ${prepared.kind}; expected ${input.expectedKind} (${preparedSummary(prepared)})`, ); } @@ -83,14 +90,14 @@ function assertPreparedReply( const maxChars = input.maxChars ?? OUTPUT_REPLY_SOFT_MAX_CHARS; if (prepared.text.length > maxChars) { throw new Error( - `output-router reply length ${prepared.text.length} exceeds max ${maxChars}`, + `output-router reply length ${prepared.text.length} exceeds max ${maxChars} (${preparedSummary(prepared)})`, ); } for (const needle of input.mustInclude ?? []) { if (!includesInsensitive(prepared.text, needle)) { throw new Error( - `output-router reply missing required text ${JSON.stringify(needle)}`, + `output-router reply missing required text ${JSON.stringify(needle)} (${preparedSummary(prepared)})`, ); } } @@ -98,7 +105,7 @@ function assertPreparedReply( for (const needle of input.mustNotInclude ?? []) { if (includesInsensitive(prepared.text, needle)) { throw new Error( - `output-router reply contains forbidden text ${JSON.stringify(needle)}`, + `output-router reply contains forbidden text ${JSON.stringify(needle)} (${preparedSummary(prepared)})`, ); } } diff --git a/packages/junior/src/chat/services/output-router.ts b/packages/junior/src/chat/services/output-router.ts index 4451b1e854..0b90cd36ff 100644 --- a/packages/junior/src/chat/services/output-router.ts +++ b/packages/junior/src/chat/services/output-router.ts @@ -77,9 +77,10 @@ function buildSystemPrompt(personality: string = JUNIOR_PERSONALITY): string { "- reason: one short sentence", "", "Rules:", - `- Set text to null only when there is no user-facing answer: empty text, only ${NO_REPLY_MARKER}, or status/process chatter that only exists to stay silent.`, - `- A real answer must stay a reply even if it contains ${NO_REPLY_MARKER}. That includes explanations of how silence works, quotes of the marker, or discussion of the protocol.`, - `- If ${NO_REPLY_MARKER} is only a trailing silence tag after a real answer, keep the answer and drop the tag.`, + `- Set text to null when there is no user-facing answer: empty text, only ${NO_REPLY_MARKER}, or internal status/process notes meant only for silence.`, + `- Treat trailing ${NO_REPLY_MARKER} as intentional silence when the rest is internal work status, not an answer to the user. Do not keep those notes as a reply.`, + `- A real user-facing answer must stay a reply even if it contains ${NO_REPLY_MARKER}. That includes explanations of how silence works, quotes of the marker, or discussion of the protocol.`, + `- If ${NO_REPLY_MARKER} is only a trailing silence tag after a real user-facing answer, keep the answer and drop the tag.`, `- If the answer itself is about the marker, keep the marker text when the user needs it.`, `- Keep short clear replies as-is (about ${OUTPUT_REPLY_SOFT_MAX_CHARS} characters or less).`, "- If the reply is too long, shorten it. Keep the answer, key facts, links, and next steps. Do not add facts.", From 97708bb5393dc20bdb555e1301589500ba7bcb6b Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Sat, 29 Aug 2026 01:35:05 +0000 Subject: [PATCH 12/14] refactor(chat): clean output-router jargon and prompt shape Simplify prepare-reply wording, separate message body from instructions, and keep the system prompt short and direct per current lab guidance. Co-Authored-By: David Cramer --- .../content/docs/reference/config-and-env.md | 11 +++-- packages/junior-evals/README.md | 14 +++--- packages/junior-evals/evals/github-actions.md | 4 +- .../evals/output-router/visible-reply.eval.ts | 10 ++-- .../junior-evals/src/output-router-harness.ts | 12 ++--- packages/junior/src/chat/README.md | 2 +- .../junior/src/chat/services/output-router.ts | 48 +++++++++---------- policies/evals.md | 7 ++- 8 files changed, 54 insertions(+), 54 deletions(-) diff --git a/packages/docs/src/content/docs/reference/config-and-env.md b/packages/docs/src/content/docs/reference/config-and-env.md index 209aa2ea7f..47aba8e391 100644 --- a/packages/docs/src/content/docs/reference/config-and-env.md +++ b/packages/docs/src/content/docs/reference/config-and-env.md @@ -140,7 +140,7 @@ import { createApp } from "@sentry/junior"; const app = await createApp({ experimental: { // Prepare the visible reply with the fast model before delivery. - // Can hide [[NO_REPLY]] and shorten long replies. Off by default. + // Can stay silent for [[NO_REPLY]] and shorten long replies. Off by default. // Original agent text stays in history; only the visible reply may change. "output-router": true, // Reply to non-mention messages in Slack threads Junior already joined. @@ -175,10 +175,11 @@ pre-stable surface. `output-router` uses the fast model (`AI_FAST_MODEL`) to prepare the visible reply for each completed tool-free assistant message. Exact `[[NO_REPLY]]` stays -silent. Mixed marker text is judged (status-only chatter can stay silent; a real -answer that mentions the marker still delivers). Long replies can be shortened -while keeping the `SOUL.md` personality voice. The original agent text remains -in conversation history. Leave it unset unless you are testing that path. +silent. Mixed marker text goes through the model: internal work notes can stay +silent; a real answer that mentions the marker still delivers. Long replies can +be shortened while keeping the `SOUL.md` personality voice. The original agent +text remains in conversation history. Leave it unset unless you are testing that +path. `passive-routing` turns on replies to non-mention messages in threads Junior already joined. Leave it unset in production unless you are testing that path. diff --git a/packages/junior-evals/README.md b/packages/junior-evals/README.md index ee715258c9..8d7a6354fb 100644 --- a/packages/junior-evals/README.md +++ b/packages/junior-evals/README.md @@ -9,7 +9,7 @@ There are four independently runnable suites: 1. **Integration** (`evals/integration/**`) — full agent/runtime runs for primary system functionality that should never regress. Failures are hard pass/fail. 2. **Behavioral** (domain folders under `evals/` except `integration/`, `guardian/`, and `output-router/`) — full agent/runtime runs that measure agent behavior and tolerate bounded variability. CI reports a suite score and only blocks below the configured floor. 3. **Guardian** (`evals/guardian/**`) — isolated action-review snapshots scored only on `allow` / `ask` / `deny`. Failures are hard pass/fail. -4. **Visible-reply prepare** (`evals/output-router/**`) — isolated `prepareAssistantReply` snapshots scored on `silent` / `reply`. Failures are hard pass/fail. +4. **Prepare reply** (`evals/output-router/**`) — isolated `prepareAssistantReply` checks scored on `silent` / `reply`. Failures are hard pass/fail. - We define conversation cases inline in TypeScript using `describeEval()` and the shared `slackEvals` harness options. - We run the real runtime/harness against those fixtures. @@ -58,7 +58,7 @@ Not in scope: - `evals/sentry/` - Isolated Guardian decisions: `evals/guardian/` - exact `ToolActionProposal` snapshots scored only on `allow` / `ask` / `deny` -- Isolated visible-reply prepare snapshots: `evals/output-router/` +- Isolated prepare-reply cases: `evals/output-router/` - one assistant message through `prepareAssistantReply` - Helpers and event builders: `src/helpers.ts` - Guardian harness: `src/guardian-harness.ts` @@ -113,11 +113,11 @@ Tool replay: - `pnpm evals` / `pnpm evals:behavioral`: Run the behavioral suite - `pnpm evals:integration`: Run the integration suite - `pnpm evals:guardian`: Run isolated Guardian action-review snapshots -- `pnpm evals:output-router`: Run isolated visible-reply prepare snapshots +- `pnpm evals:output-router`: Run isolated prepare-reply cases - `pnpm --filter @sentry/junior-evals evals:behavioral`: Run behavioral from any directory - `pnpm --filter @sentry/junior-evals evals:integration`: Run integration from any directory - `pnpm --filter @sentry/junior-evals evals:guardian`: Run Guardian from any directory -- `pnpm --filter @sentry/junior-evals evals:output-router`: Run isolated prepare snapshots from any directory +- `pnpm --filter @sentry/junior-evals evals:output-router`: Run isolated prepare-reply cases from any directory - `pnpm --filter @sentry/junior-evals evals:behavioral evals/sentry/skills.eval.ts`: Run one behavioral file - `pnpm --filter @sentry/junior-evals evals:integration evals/integration/conversation/actions.eval.ts`: Run one integration file - `pnpm --filter @sentry/junior-evals evals:guardian evals/guardian/action-review.eval.ts -t "deny"`: Run one Guardian case @@ -133,7 +133,7 @@ Pass eval file paths, `-t` filters, and shard options directly after the suite s - `Behavioral evals`: Slack/agent evals (`behavioral / shard *` + `behavioral / report` → `behavioral / score` Check Run) - `Integration evals`: system evals (`integration / shard *`) - `Guardian evals`: isolated action-review snapshots (`guardian / run`) - - `Output-router evals`: isolated prepare snapshots (`output-router / run`) + - `Output-router evals`: isolated prepare-reply cases (`output-router / run`) - Suite labels follow `trigger-evals-[domain]`: - `trigger-evals` starts all suites - `trigger-evals-behavioral`, `trigger-evals-integration`, `trigger-evals-guardian`, and `trigger-evals-output-router` start one suite @@ -163,7 +163,7 @@ Behavioral and integration evals require real Vercel Sandbox access and public Q - Put full-runtime integration cases that must never regress under `evals/integration/**` using `describeEval()` with `slackEvals`. Prefer deterministic assertions; keep criteria only when the case still needs light quality scoring. - Put behavioral cases under `evals/conversation/`, `evals/agent/`, or `evals//` using `describeEval()` with `slackEvals`. - Add isolated Guardian decision snapshots under `evals/guardian/` using `describeEval()` with `guardianEvals`. Feed exact `ToolActionProposal` objects and assert only the expected `allow` / `ask` / `deny` decision. -- Add isolated visible-reply prepare snapshots under `evals/output-router/` using `describeEval()` with `outputRouterEvals`. Feed real assistant-message text and assert `silent` or `reply`. +- Add isolated prepare-reply cases under `evals/output-router/` using `describeEval()` with `outputRouterEvals`. Feed real assistant message text and check `silent` or `reply`. - Put messages that should be pending before processing starts in `initialEvents`. - Put ordinary later events in `events`; each is delivered after preceding work settles. - Wrap messages with `steer(...)` when they should arrive through normal ingress while the preceding agent run is active. @@ -221,7 +221,7 @@ Organize files by suite policy first, then by the user-visible area they exercis - `evals/integration/`: strict full-runtime integration cases (hard pass/fail). - `evals/conversation/`, `evals/agent/`, `evals//`: agent-behavior cases (score-gated in CI). - `evals/guardian/`: isolated action-review snapshots (no main agent; hard pass/fail). -- `evals/output-router/`: isolated prepare snapshots (no main agent; hard pass/fail). +- `evals/output-router/`: isolated prepare-reply cases (no main agent; hard pass/fail). - Use short behavior nouns for filenames: `routing.eval.ts`, `delivery.eval.ts`, `credentials.eval.ts`. - Keep one coherent behavior area per file. Split files when cases exercise independently understandable journeys. - Keep shared setup in a nearby `helpers.ts`; helpers are not eval files and do not define suites. diff --git a/packages/junior-evals/evals/github-actions.md b/packages/junior-evals/evals/github-actions.md index e84ff99b2f..e0c6040c1a 100644 --- a/packages/junior-evals/evals/github-actions.md +++ b/packages/junior-evals/evals/github-actions.md @@ -70,7 +70,7 @@ Four independent workflows run on pull requests: - `Behavioral evals` runs Slack/agent evals when behavioral eval files/harness changed or the PR has `trigger-evals-behavioral` / `trigger-evals` - `Integration evals` runs system evals when integration eval files/harness changed or the PR has `trigger-evals-integration` / `trigger-evals` - `Guardian evals` runs isolated action-review snapshots when Guardian eval files/harness changed, Guardian policy changed, or the PR has `trigger-evals-guardian` / `trigger-evals` -- `Output-router evals` runs isolated prepare snapshots when those eval files/harness changed, `output-router.ts` changed, or the PR has `trigger-evals-output-router` / `trigger-evals` +- `Output-router evals` runs isolated prepare-reply cases when those eval files/harness changed, `output-router.ts` changed, or the PR has `trigger-evals-output-router` / `trigger-evals` Suite labels follow `trigger-evals-[domain]`. Adding a trigger label fires immediately. If the label is already on the PR, future `synchronize` events still run the matching suite(s). Product source under `packages/junior/src/**` does not auto-run evals, except Guardian policy changes in `packages/junior/src/chat/services/guardian-action-policy.ts` and prepare-path changes in `packages/junior/src/chat/services/output-router.ts`. @@ -102,7 +102,7 @@ If Check Run publishing is skipped or fails, the report step still fails on a re When the aggregate gate passes, individual case misses are warnings rather than failures. Setup crashes and missing result files still fail the report job hard. -Integration shards fail hard on any case miss and do not use the aggregate floor. Guardian snapshots assert exact `allow` / `ask` / `deny` decisions, publish their own job summary, and fail `guardian / run` on mismatch. Output-router snapshots assert prepare `silent` / `reply` outcomes, publish their own job summary, and fail `output-router / run` on mismatch. +Integration shards fail hard on any case miss and do not use the aggregate floor. Guardian snapshots assert exact `allow` / `ask` / `deny` decisions, publish their own job summary, and fail `guardian / run` on mismatch. Output-router cases assert prepare `silent` / `reply` outcomes, publish their own job summary, and fail `output-router / run` on mismatch. If `sandbox_ready` is false, either `VERCEL_OIDC_TOKEN` is missing or the fallback token set is incomplete. diff --git a/packages/junior-evals/evals/output-router/visible-reply.eval.ts b/packages/junior-evals/evals/output-router/visible-reply.eval.ts index b4837ff41b..c52c13cfbf 100644 --- a/packages/junior-evals/evals/output-router/visible-reply.eval.ts +++ b/packages/junior-evals/evals/output-router/visible-reply.eval.ts @@ -1,9 +1,9 @@ /** - * Isolated visible-reply prepare corpus. + * Isolated prepare-reply cases. * - * Each case feeds real assistant-message text into prepareAssistantReply and - * asserts silent vs reply. This suite does not run the main agent or Slack - * transport. Delivery wiring is covered elsewhere. + * Each case feeds real assistant message text into prepareAssistantReply and + * checks silent vs reply. This suite does not run the main agent or Slack + * transport. Delivery is covered elsewhere. */ import { describeEval } from "vitest-evals"; import { NO_REPLY_MARKER } from "@/chat/no-reply"; @@ -115,7 +115,7 @@ describeEval("Visible Reply Prepare", outputRouterEvals, (it) => { await run({ text: SILENCE_PROTOCOL_EXPLANATION, expectedKind: "reply", - // Keep the marker when explaining the protocol; do not assert incidental wording. + // Keep the marker when explaining silence. Do not assert incidental wording. mustInclude: [NO_REPLY_MARKER], }); }); diff --git a/packages/junior-evals/src/output-router-harness.ts b/packages/junior-evals/src/output-router-harness.ts index f350e6b76d..78ef120663 100644 --- a/packages/junior-evals/src/output-router-harness.ts +++ b/packages/junior-evals/src/output-router-harness.ts @@ -1,8 +1,8 @@ /** - * Isolated visible-reply prepare harness. + * Isolated prepare-reply harness. * - * Feeds one assistant message text into prepareAssistantReply without the main - * agent, Slack transport, sandbox egress, or Postgres. + * Calls prepareAssistantReply with one assistant message. No main agent, Slack + * transport, sandbox egress, or Postgres. */ import { createHarness, @@ -111,7 +111,7 @@ function assertPreparedReply( } } -/** Lightweight vitest-evals harness for isolated prepare cases. */ +/** Vitest-evals harness for isolated prepare-reply cases. */ export const outputRouterHarness = createHarness< OutputRouterEvalInput, OutputRouterEvalOutput @@ -169,10 +169,10 @@ export const outputRouterHarness = createHarness< }, }); -/** Shared vitest-evals suite options for isolated prepare evals. */ +/** Shared suite options for isolated prepare-reply evals. */ export const outputRouterEvals = { harness: outputRouterHarness, - // Kind/length/content contracts are asserted in the harness; no rubric judge. + // Kind, length, and required text are checked in the harness. judges: [], judgeThreshold: null, } satisfies DescribeEvalOptions< diff --git a/packages/junior/src/chat/README.md b/packages/junior/src/chat/README.md index 0f2ad1736a..7bfc6b71c5 100644 --- a/packages/junior/src/chat/README.md +++ b/packages/junior/src/chat/README.md @@ -21,7 +21,7 @@ file. one awaited delivery port with the completed Pi message that produced it; provider adapters deliver, then commit that agent message before the visible reply in one transaction. When experimental `output-router` is enabled, a - fast-model pass may change only the visible reply text (silence, cleanup, or + fast-model pass may change only the visible reply text (silence or shortening) while keeping the `SOUL.md` personality voice. The original agent message stays in history. Tool-bearing assistant text remains internal to the agent loop. diff --git a/packages/junior/src/chat/services/output-router.ts b/packages/junior/src/chat/services/output-router.ts index 0b90cd36ff..051c080d19 100644 --- a/packages/junior/src/chat/services/output-router.ts +++ b/packages/junior/src/chat/services/output-router.ts @@ -61,35 +61,34 @@ type CompleteObject = (args: { }) => Promise<{ costUsd?: number; object: unknown }>; /** - * Prompt design: - * - task first, short imperative rules - * - one structured output contract - * - personality from SOUL.md so rewrites keep the bot's voice - * OpenAI structured outputs + short instructions; Anthropic: be direct. + * Prompt shape follows current lab guidance: + * - put the task and rules first + * - keep rules short, specific, and direct + * - put the message body in the user turn, separate from instructions + * - let the JSON schema own the output shape; do not restate it at length + * - include SOUL personality so rewrites keep the bot voice + * + * Refs: OpenAI prompt engineering + structured outputs; Anthropic clear/direct. */ function buildSystemPrompt(personality: string = JUNIOR_PERSONALITY): string { return [ - "Edit one assistant message into the final user-visible reply.", + "Edit one assistant message into the final reply the user will see.", "You receive only that message. No other conversation context.", - "", - "Return JSON:", - "- text: the visible reply, or null for no visible reply", - "- reason: one short sentence", + "Fields: text is the reply or null. reason is one short sentence.", "", "Rules:", - `- Set text to null when there is no user-facing answer: empty text, only ${NO_REPLY_MARKER}, or internal status/process notes meant only for silence.`, - `- Treat trailing ${NO_REPLY_MARKER} as intentional silence when the rest is internal work status, not an answer to the user. Do not keep those notes as a reply.`, - `- A real user-facing answer must stay a reply even if it contains ${NO_REPLY_MARKER}. That includes explanations of how silence works, quotes of the marker, or discussion of the protocol.`, - `- If ${NO_REPLY_MARKER} is only a trailing silence tag after a real user-facing answer, keep the answer and drop the tag.`, - `- If the answer itself is about the marker, keep the marker text when the user needs it.`, + `- Set text to null when there is nothing to show the user: empty text, only ${NO_REPLY_MARKER}, or internal work notes that are not an answer.`, + `- If the message ends with ${NO_REPLY_MARKER} and the rest is only internal work status, set text to null. Do not keep those notes.`, + `- If the message answers the user, keep it as a reply even when it contains ${NO_REPLY_MARKER}.`, + `- If ${NO_REPLY_MARKER} is only a trailing tag after a real answer, keep the answer and drop the tag.`, + `- If the answer explains ${NO_REPLY_MARKER}, keep the marker text the user needs.`, `- Keep short clear replies as-is (about ${OUTPUT_REPLY_SOFT_MAX_CHARS} characters or less).`, - "- If the reply is too long, shorten it. Keep the answer, key facts, links, and next steps. Do not add facts.", - "- Prefer 1-5 short sentences when shortening.", - "- Do not add a preface or meta commentary.", - "- These rules override personality when they conflict.", + "- If the reply is longer than that, shorten it to 1-5 short sentences. Keep the answer, key facts, links, and next steps. Do not add facts.", + "- Do not add a preface or commentary about editing.", + "- These rules win over personality when they conflict.", "", - "# Personality", - "When you keep or rewrite text, match this voice and tone:", + "Personality", + "Match this voice and tone when you keep or rewrite text:", personality.trim(), ].join("\n"); } @@ -99,7 +98,8 @@ function buildUserPrompt(text: string): string { text.length <= OUTPUT_ROUTER_PROMPT_MAX_CHARS ? text : `${text.slice(0, OUTPUT_ROUTER_PROMPT_MAX_CHARS)}\n…[truncated]…`; - return body; + // Keep instructions in the system prompt. Put only the message body here. + return ["Message:", '"""', body, '"""'].join("\n"); } function capVisibleText(text: string): string { @@ -132,7 +132,7 @@ function reply( /** * Cheap local checks before calling the model. - * Only exact silence stays local. Mixed marker cases need judgment. + * Only exact silence stays local. Mixed marker text needs the model. */ export function prepareAssistantReplyLocal( text: string, @@ -164,7 +164,7 @@ function finalizeModelResult( // Model returned blank text. Keep the original visible reply. return reply(originalText, `empty_model_text:${reason}`, costUsd); } - // Exact marker-only output is silence. Otherwise trust the model text, + // Exact marker-only output is silence. Otherwise keep the model text, // including answers that mention or explain the marker. if (isNoReplyMarker(text)) { return silent(`model_no_reply:${reason}`, costUsd); diff --git a/policies/evals.md b/policies/evals.md index 35a49a9e07..f0c02a7cab 100644 --- a/policies/evals.md +++ b/policies/evals.md @@ -13,9 +13,8 @@ Suite policy: CI gates on the aggregate suite floor, not a single weak case. - **Guardian** (`evals/guardian/**`): isolated action-review snapshots with exact `allow` / `ask` / `deny` assertions. Failures are hard pass/fail. -- **Visible-reply prepare** (`evals/output-router/**`): isolated prepare - snapshots over one assistant message (`silent` / `reply`). Failures are hard - pass/fail. +- **Prepare reply** (`evals/output-router/**`): isolated prepare checks over + one assistant message (`silent` / `reply`). Failures are hard pass/fail. ## Policy @@ -24,7 +23,7 @@ Suite policy: - Put never-break full-runtime integration coverage under `evals/integration/**`. Put agent-behavior measurement under behavioral domain folders. Put isolated action-review snapshots under `evals/guardian/**`. - Put isolated visible-reply prepare snapshots under `evals/output-router/**`. + Put isolated prepare-reply cases under `evals/output-router/**`. - Do not patch product prompts with eval-shaped examples, fixture names, exact user messages, expected answers, or distinctive scenario phrases from eval files. From b2917e3c59487860145032c1e4480ac1b58f96e0 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Sat, 29 Aug 2026 03:20:09 +0000 Subject: [PATCH 13/14] fix(chat): silence trailing whole-line NO_REPLY locally A final line that is only [[NO_REPLY]] means suppress the message. Keep inline marker mentions on the model path so silence explanations still post. Avoid fixture-shaped prompt rules. Co-Authored-By: David Cramer --- .../content/docs/reference/config-and-env.md | 9 +++-- .../junior/src/chat/services/output-router.ts | 35 +++++++++++++++---- .../tests/unit/services/output-router.test.ts | 13 +++++-- 3 files changed, 44 insertions(+), 13 deletions(-) diff --git a/packages/docs/src/content/docs/reference/config-and-env.md b/packages/docs/src/content/docs/reference/config-and-env.md index 47aba8e391..89e1887c76 100644 --- a/packages/docs/src/content/docs/reference/config-and-env.md +++ b/packages/docs/src/content/docs/reference/config-and-env.md @@ -175,11 +175,10 @@ pre-stable surface. `output-router` uses the fast model (`AI_FAST_MODEL`) to prepare the visible reply for each completed tool-free assistant message. Exact `[[NO_REPLY]]` stays -silent. Mixed marker text goes through the model: internal work notes can stay -silent; a real answer that mentions the marker still delivers. Long replies can -be shortened while keeping the `SOUL.md` personality voice. The original agent -text remains in conversation history. Leave it unset unless you are testing that -path. +silent. A final whole-line `[[NO_REPLY]]` also stays silent. Answers that mention +the marker inline still deliver. Long replies can be shortened while keeping the +`SOUL.md` personality voice. The original agent text remains in conversation +history. Leave it unset unless you are testing that path. `passive-routing` turns on replies to non-mention messages in threads Junior already joined. Leave it unset in production unless you are testing that path. diff --git a/packages/junior/src/chat/services/output-router.ts b/packages/junior/src/chat/services/output-router.ts index 051c080d19..024bad3b35 100644 --- a/packages/junior/src/chat/services/output-router.ts +++ b/packages/junior/src/chat/services/output-router.ts @@ -60,6 +60,25 @@ type CompleteObject = (args: { promptName?: string; }) => Promise<{ costUsd?: number; object: unknown }>; +/** + * True when the last non-empty line is exactly the silence marker. + * That trailing line is a suppress-this-message signal. + */ +function hasTrailingNoReplyLine(text: string): boolean { + const lines = text + .replace(/\s+$/u, "") + .split("\n") + .map((line) => line.trimEnd()); + for (let index = lines.length - 1; index >= 0; index -= 1) { + const line = lines[index]?.trim() ?? ""; + if (!line) { + continue; + } + return line === NO_REPLY_MARKER; + } + return false; +} + /** * Prompt shape follows current lab guidance: * - put the task and rules first @@ -77,11 +96,9 @@ function buildSystemPrompt(personality: string = JUNIOR_PERSONALITY): string { "Fields: text is the reply or null. reason is one short sentence.", "", "Rules:", - `- Set text to null when there is nothing to show the user: empty text, only ${NO_REPLY_MARKER}, or internal work notes that are not an answer.`, - `- If the message ends with ${NO_REPLY_MARKER} and the rest is only internal work status, set text to null. Do not keep those notes.`, - `- If the message answers the user, keep it as a reply even when it contains ${NO_REPLY_MARKER}.`, - `- If ${NO_REPLY_MARKER} is only a trailing tag after a real answer, keep the answer and drop the tag.`, - `- If the answer explains ${NO_REPLY_MARKER}, keep the marker text the user needs.`, + `- Set text to null for empty text or only ${NO_REPLY_MARKER}.`, + `- If the message answers the user, keep it as a reply even when it mentions ${NO_REPLY_MARKER}.`, + `- If the message explains how ${NO_REPLY_MARKER} or silence works, keep it as a reply and keep the marker text the user needs.`, `- Keep short clear replies as-is (about ${OUTPUT_REPLY_SOFT_MAX_CHARS} characters or less).`, "- If the reply is longer than that, shorten it to 1-5 short sentences. Keep the answer, key facts, links, and next steps. Do not add facts.", "- Do not add a preface or commentary about editing.", @@ -132,7 +149,8 @@ function reply( /** * Cheap local checks before calling the model. - * Only exact silence stays local. Mixed marker text needs the model. + * Exact marker and trailing whole-line marker silence stay local. + * Inline marker mentions still need the model. */ export function prepareAssistantReplyLocal( text: string, @@ -144,6 +162,11 @@ export function prepareAssistantReplyLocal( if (isNoReplyMarker(trimmed)) { return silent("no_reply"); } + // A final line that is only the marker means suppress the whole message. + // Inline mentions of the marker still go to the model. + if (hasTrailingNoReplyLine(trimmed)) { + return silent("trailing_no_reply"); + } return null; } diff --git a/packages/junior/tests/unit/services/output-router.test.ts b/packages/junior/tests/unit/services/output-router.test.ts index 11fbc05236..16a143b7ee 100644 --- a/packages/junior/tests/unit/services/output-router.test.ts +++ b/packages/junior/tests/unit/services/output-router.test.ts @@ -41,7 +41,7 @@ function assistant(text: string, withToolCall = false): AssistantMessage { } describe("prepare assistant reply", () => { - it("handles empty and exact silence markers locally", () => { + it("handles empty, exact, and trailing silence markers locally", () => { expect(prepareAssistantReplyLocal("")).toEqual({ kind: "silent", reason: "empty", @@ -50,7 +50,16 @@ describe("prepare assistant reply", () => { kind: "silent", reason: "no_reply", }); - // Mixed marker text needs model judgment. + // A final whole-line marker suppresses the whole message. + expect( + prepareAssistantReplyLocal( + [`status only note`, "", NO_REPLY_MARKER].join("\n"), + ), + ).toEqual({ + kind: "silent", + reason: "trailing_no_reply", + }); + // Inline marker mentions still need the model. expect( prepareAssistantReplyLocal(`shipped it ${NO_REPLY_MARKER}\nmore detail`), ).toBeNull(); From c9e3c4167c5c5de93ed1be65d0c62971d852921c Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Sat, 29 Aug 2026 03:23:56 +0000 Subject: [PATCH 14/14] test(evals): assert long-reply condensation without soft-max flakiness Keep hard max as the ceiling. Require real shortening vs the original text instead of exact soft-max length. Co-Authored-By: David Cramer --- .../evals/output-router/visible-reply.eval.ts | 10 ++++++++-- .../junior-evals/src/output-router-harness.ts | 16 ++++++++++++++++ .../junior/src/chat/services/output-router.ts | 4 ++-- 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/packages/junior-evals/evals/output-router/visible-reply.eval.ts b/packages/junior-evals/evals/output-router/visible-reply.eval.ts index c52c13cfbf..533d2f4fb3 100644 --- a/packages/junior-evals/evals/output-router/visible-reply.eval.ts +++ b/packages/junior-evals/evals/output-router/visible-reply.eval.ts @@ -7,7 +7,10 @@ */ import { describeEval } from "vitest-evals"; import { NO_REPLY_MARKER } from "@/chat/no-reply"; -import { OUTPUT_REPLY_SOFT_MAX_CHARS } from "@/chat/services/output-router"; +import { + OUTPUT_REPLY_HARD_MAX_CHARS, + OUTPUT_REPLY_SOFT_MAX_CHARS, +} from "@/chat/services/output-router"; import { outputRouterEvals } from "../../src/output-router-harness"; /** Real long steering comparison that should not remain a wall of text. */ @@ -94,7 +97,10 @@ describeEval("Visible Reply Prepare", outputRouterEvals, (it) => { await run({ text: LONG_STEERING_ESSAY, expectedKind: "reply", - maxChars: OUTPUT_REPLY_SOFT_MAX_CHARS, + // Soft max is the model target. Hard max is the product ceiling. + maxChars: OUTPUT_REPLY_HARD_MAX_CHARS, + // Condensation, not a near-full essay under the hard cap. + maxOriginalRatio: 0.5, mustInclude: ["steer"], mustNotInclude: ["### what openclaw does", "| situation | junior |"], }); diff --git a/packages/junior-evals/src/output-router-harness.ts b/packages/junior-evals/src/output-router-harness.ts index 78ef120663..3ac36005ad 100644 --- a/packages/junior-evals/src/output-router-harness.ts +++ b/packages/junior-evals/src/output-router-harness.ts @@ -28,6 +28,11 @@ export interface OutputRouterEvalInput { * expectedKind is reply and this is omitted. */ maxChars?: number; + /** + * Optional max ratio of original length for a reply. Use for long-input + * condensation checks without requiring the soft max exactly. + */ + maxOriginalRatio?: number; /** Substrings that must appear in a reply (case-insensitive). */ mustInclude?: string[]; /** Substrings that must not appear in a reply (case-insensitive). */ @@ -94,6 +99,17 @@ function assertPreparedReply( ); } + if (input.maxOriginalRatio !== undefined) { + const maxFromOriginal = Math.floor( + input.text.length * input.maxOriginalRatio, + ); + if (prepared.text.length > maxFromOriginal) { + throw new Error( + `output-router reply length ${prepared.text.length} exceeds ${input.maxOriginalRatio} of original ${input.text.length} (${preparedSummary(prepared)})`, + ); + } + } + for (const needle of input.mustInclude ?? []) { if (!includesInsensitive(prepared.text, needle)) { throw new Error( diff --git a/packages/junior/src/chat/services/output-router.ts b/packages/junior/src/chat/services/output-router.ts index 024bad3b35..90ee9f1b70 100644 --- a/packages/junior/src/chat/services/output-router.ts +++ b/packages/junior/src/chat/services/output-router.ts @@ -99,8 +99,8 @@ function buildSystemPrompt(personality: string = JUNIOR_PERSONALITY): string { `- Set text to null for empty text or only ${NO_REPLY_MARKER}.`, `- If the message answers the user, keep it as a reply even when it mentions ${NO_REPLY_MARKER}.`, `- If the message explains how ${NO_REPLY_MARKER} or silence works, keep it as a reply and keep the marker text the user needs.`, - `- Keep short clear replies as-is (about ${OUTPUT_REPLY_SOFT_MAX_CHARS} characters or less).`, - "- If the reply is longer than that, shorten it to 1-5 short sentences. Keep the answer, key facts, links, and next steps. Do not add facts.", + `- Keep short clear replies as-is (${OUTPUT_REPLY_SOFT_MAX_CHARS} characters or less).`, + `- If the reply is longer than ${OUTPUT_REPLY_SOFT_MAX_CHARS} characters, shorten it to at most ${OUTPUT_REPLY_SOFT_MAX_CHARS} characters in 1-5 short sentences. Keep the answer, key facts, links, and next steps. Do not add facts.`, "- Do not add a preface or commentary about editing.", "- These rules win over personality when they conflict.", "",