diff --git a/packages/junior/src/chat/prompt.ts b/packages/junior/src/chat/prompt.ts index a76cb4f00..5b65004a1 100644 --- a/packages/junior/src/chat/prompt.ts +++ b/packages/junior/src/chat/prompt.ts @@ -23,6 +23,7 @@ import { sandboxSkillDir, } from "@/chat/sandbox/paths"; import type { SlackConversationContext } from "@/chat/slack/conversation-context"; +import { slackOutputPolicy } from "@/chat/slack/output"; import type { SkillMetadata } from "@/chat/skills"; import type { ActiveMcpCatalogSummary } from "@/chat/tool-support/skill/mcp-tool-summary"; import { escapeXml } from "@/chat/xml"; @@ -376,10 +377,18 @@ function buildBehaviorSection(platform: PromptPlatform): string { return sections.join("\n\n"); } +const DEFAULT_REPLY_TARGET_CHARS = 800; + +/** One short length rule shared by every platform. */ +function buildMaxMessageSizeRule(): string { + return `- Keep replies short: usually 1–5 sentences and under ${DEFAULT_REPLY_TARGET_CHARS} characters. Stay inside one message (≤${slackOutputPolicy.maxInlineChars} characters / ≤${slackOutputPolicy.maxInlineLines} lines). Prefer a canvas or linked artifact for longer detail.`; +} + function buildOutputSection(platform: PromptPlatform): string { if (platform === "local") { return [ ``, + buildMaxMessageSizeRule(), "- Start with the answer or result, not internal process narration.", "- Use concise Markdown suitable for terminal and web output: short paragraphs, bullets, links, fenced code blocks, and GFM tables when a grid is clearer than bullets.", "- End every turn with a final user-facing response.", @@ -389,7 +398,7 @@ function buildOutputSection(platform: PromptPlatform): string { return [ ``, - "- Default to the shortest complete reply—usually 1–5 sentences and under 800 characters. Include only the outcome, decisive evidence, and any blocker or required next action. If useful detail would exceed that, put it in a Slack canvas and reply with the link. An explicit user request for detail overrides this target.", + buildMaxMessageSizeRule(), "- Start with the answer or result, not internal process narration.", "- Use Slack-flavored Markdown: **bold** section labels, `code`, [text](url) links, bullet lists, and fenced code blocks. No hash-prefixed headings and no tables. When the answer primarily lists several URLs, show each URL bare instead of as a labeled link.", "- End every turn with a final user-facing markdown response unless the Slack action rules allow a no-reply completion.", diff --git a/packages/junior/src/chat/slack/output.ts b/packages/junior/src/chat/slack/output.ts index 089e7101e..8b976f3ed 100644 --- a/packages/junior/src/chat/slack/output.ts +++ b/packages/junior/src/chat/slack/output.ts @@ -2,8 +2,8 @@ import type { PostableMessage } from "chat"; import { getInterruptionMarker } from "@/chat/interruption-marker"; import { normalizeSlackReplyMarkdown } from "@/chat/slack/mrkdwn"; -const MAX_INLINE_CHARS = 2200; -const MAX_INLINE_LINES = 45; +const MAX_INLINE_CHARS = 1200; +const MAX_INLINE_LINES = 12; const CONTINUED_MARKER = "\n\n[Continued below]"; function countSlackLines(text: string): number { diff --git a/packages/junior/tests/component/slack/reply-delivery.test.ts b/packages/junior/tests/component/slack/reply-delivery.test.ts index ce0899c88..ca2557bea 100644 --- a/packages/junior/tests/component/slack/reply-delivery.test.ts +++ b/packages/junior/tests/component/slack/reply-delivery.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { slackOutputPolicy } from "@/chat/slack/output"; import { sendSlackReply } from "@/chat/slack/reply"; import { getCapturedSlackApiCalls, @@ -63,14 +64,14 @@ describe("sendSlackReply", () => { const messageTs = await sendSlackReply({ channelId: "C123", conversationId: "agent-dispatch:dispatch-1", - text: "a".repeat(4_500), + text: "a".repeat(slackOutputPolicy.maxInlineChars * 2 + 100), }); - expect(messageTs).toHaveLength(3); + expect(messageTs.length).toBeGreaterThan(1); expect(messageTs.every(Boolean)).toBe(true); const posts = getCapturedSlackApiCalls("chat.postMessage"); - expect(posts).toHaveLength(3); + expect(posts).toHaveLength(messageTs.length); expect(posts[0]?.params).toEqual( expect.objectContaining({ channel: "C123", @@ -78,18 +79,14 @@ describe("sendSlackReply", () => { ); expect(posts[0]?.params).not.toHaveProperty("thread_ts"); // Later chunks reply under the first posted message. - expect(posts[1]?.params).toEqual( - expect.objectContaining({ - channel: "C123", - thread_ts: messageTs[0], - }), - ); - expect(posts[2]?.params).toEqual( - expect.objectContaining({ - channel: "C123", - thread_ts: messageTs[0], - }), - ); + for (const post of posts.slice(1)) { + expect(post.params).toEqual( + expect.objectContaining({ + channel: "C123", + thread_ts: messageTs[0], + }), + ); + } }); it("keeps the original thread for every chunk", async () => { diff --git a/packages/junior/tests/integration/oauth-resume-slack.test.ts b/packages/junior/tests/integration/oauth-resume-slack.test.ts index 43c2bbb17..38044a7b6 100644 --- a/packages/junior/tests/integration/oauth-resume-slack.test.ts +++ b/packages/junior/tests/integration/oauth-resume-slack.test.ts @@ -4,6 +4,7 @@ import { fauxAssistantMessage } from "@earendil-works/pi-ai/providers/faux"; import { getSlackContinuationMarker, getSlackInterruptionMarker, + slackOutputPolicy, } from "@/chat/slack/output"; import { disconnectStateAdapter } from "@/chat/state/adapter"; import { getCapturedSlackApiCalls } from "../msw/handlers/slack-api"; @@ -281,8 +282,9 @@ describe("oauth resume slack integration", () => { it("chunks long resumed replies into explicit continuation messages", async () => { const { resumeSlackTurn } = await import("@/chat/runtime/slack-resume"); + const lineCount = slackOutputPolicy.maxInlineLines * 4; const longReply = Array.from( - { length: 80 }, + { length: lineCount }, (_, i) => `line ${i + 1}`, ).join("\n"); @@ -305,27 +307,26 @@ describe("oauth resume slack integration", () => { }); const postCalls = getCapturedSlackApiCalls("chat.postMessage"); - expect(postCalls).toHaveLength(5); + expect(postCalls.length).toBeGreaterThan(2); expect(postCalls[0]?.params).toMatchObject({ channel: "C123", thread_ts: "1700000000.002", text: "Connected. Continuing...", }); - expect(postCalls[1]?.params.text).toContain(getSlackContinuationMarker()); - expect(postCalls[2]?.params.text).toContain(getSlackContinuationMarker()); - expect(postCalls[3]?.params.text).toContain(getSlackContinuationMarker()); - expect(postCalls[4]?.params.text).not.toContain( - getSlackContinuationMarker(), - ); - expect(postCalls[4]?.params.text).toContain("line 80"); - // Continuations keep body blocks only; the conversation footer is final-chunk only. - for (const call of postCalls.slice(1, 4)) { + const replyPosts = postCalls.slice(1); + expect(replyPosts.length).toBeGreaterThan(1); + for (const call of replyPosts.slice(0, -1)) { + expect(call.params.text).toContain(getSlackContinuationMarker()); + // Continuations keep body blocks only; the conversation footer is final-chunk only. expect(JSON.stringify(call.params.blocks ?? [])).not.toContain( "slack:C123:1700000000.002", ); } + const finalReply = replyPosts.at(-1)!; + expect(finalReply.params.text).not.toContain(getSlackContinuationMarker()); + expect(finalReply.params.text).toContain(`line ${lineCount}`); expectBlocksIncludeConversationId( - postCalls[4]!.params, + finalReply.params, "slack:C123:1700000000.002", ); }); diff --git a/packages/junior/tests/unit/misc/output.test.ts b/packages/junior/tests/unit/misc/output.test.ts index 4d0accad0..f320cea90 100644 --- a/packages/junior/tests/unit/misc/output.test.ts +++ b/packages/junior/tests/unit/misc/output.test.ts @@ -267,9 +267,12 @@ describe("splitSlackReplyText", () => { (_, i) => `const value${i + 1} = ${i + 1};`, ).join("\n"); const chunks = splitSlackReplyText(`\`\`\`ts\n${code}\n\`\`\``); + const firstBody = chunks[0]?.endsWith(getSlackContinuationMarker()) + ? chunks[0].slice(0, -getSlackContinuationMarker().length) + : chunks[0]; expect(chunks.length).toBeGreaterThan(1); - expect(chunks[0]?.endsWith("```")).toBe(true); + expect(firstBody?.endsWith("```")).toBe(true); expect(chunks[1]?.startsWith("```ts\n")).toBe(true); expect(chunks.every((chunk) => fitsSlackInlineBudget(chunk))).toBe(true); });