diff --git a/apps/fiona-slack/.env.sample b/apps/fiona-slack/.env.sample index d62d1988..78b4ff19 100644 --- a/apps/fiona-slack/.env.sample +++ b/apps/fiona-slack/.env.sample @@ -15,6 +15,12 @@ # Defaults to www.ed-fi.org,docs.ed-fi.org # PERPLEXITY_DOMAIN_FILTER=www.ed-fi.org,docs.ed-fi.org +# Optional, /fiona search settings. +# Maximum number of sources returned per search (default 5, hard cap 10). +# SEARCH_MAX_SOURCES=5 +# Maximum words per source snippet before truncation (default 160). +# SEARCH_SNIPPET_MAX_WORDS=160 + # Deployment type label recorded with feedback in Cosmos DB (local, insiders, production). DEPLOYMENT_TYPE=local diff --git a/apps/fiona-slack/package-lock.json b/apps/fiona-slack/package-lock.json index 85c5b00c..cc5b9c61 100644 --- a/apps/fiona-slack/package-lock.json +++ b/apps/fiona-slack/package-lock.json @@ -11,6 +11,7 @@ "dependencies": { "@azure/cosmos": "^4.9.1", "@azure/identity": "^4.13.0", + "@perplexity-ai/perplexity_ai": "^0.37.0", "@slack/bolt": "^4.6.0", "dotenv": "~17.3.1", "openai": "^6.25.0" @@ -1369,6 +1370,12 @@ "@jridgewell/sourcemap-codec": "^1.4.14" } }, + "node_modules/@perplexity-ai/perplexity_ai": { + "version": "0.37.0", + "resolved": "https://registry.npmjs.org/@perplexity-ai/perplexity_ai/-/perplexity_ai-0.37.0.tgz", + "integrity": "sha512-SAHwhMdrIhn25tq1PbSOpyr0QYz2Bp9jn/xZ87xPmEEVpXwEyTiuUCT1S5crIBbiHSVUAZR8MwiNNmeMhkwKDw==", + "license": "Apache-2.0" + }, "node_modules/@sinclair/typebox": { "version": "0.27.10", "resolved": "https://registry.npmjs.org/@sinclair/typebox/-/typebox-0.27.10.tgz", diff --git a/apps/fiona-slack/package.json b/apps/fiona-slack/package.json index c9f8579c..cf8eb539 100644 --- a/apps/fiona-slack/package.json +++ b/apps/fiona-slack/package.json @@ -30,6 +30,7 @@ "dependencies": { "@azure/cosmos": "^4.9.1", "@azure/identity": "^4.13.0", + "@perplexity-ai/perplexity_ai": "^0.37.0", "@slack/bolt": "^4.6.0", "dotenv": "~17.3.1", "openai": "^6.25.0" diff --git a/apps/fiona-slack/src/agent/feedback-response-types.js b/apps/fiona-slack/src/agent/feedback-response-types.js new file mode 100644 index 00000000..72989af4 --- /dev/null +++ b/apps/fiona-slack/src/agent/feedback-response-types.js @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: Apache-2.0 +// Licensed to the Ed-Fi Alliance under one or more agreements. +// The Ed-Fi Alliance licenses this file to you under the Apache License, Version 2.0. +// See the LICENSE and NOTICES files in the project root for more information. + +export const FEEDBACK_RESPONSE_TYPES = Object.freeze({ + SYNTHESIS: 'synthesis', + SEARCH: 'search', + ASK: 'ask', +}); diff --git a/apps/fiona-slack/src/agent/feedback-store.js b/apps/fiona-slack/src/agent/feedback-store.js index db73fa0e..d4f652ff 100644 --- a/apps/fiona-slack/src/agent/feedback-store.js +++ b/apps/fiona-slack/src/agent/feedback-store.js @@ -5,6 +5,7 @@ import { CosmosClient } from '@azure/cosmos'; import { DefaultAzureCredential } from '@azure/identity'; +import { FEEDBACK_RESPONSE_TYPES } from './feedback-response-types.js'; const COSMOS_ENDPOINT = process.env.COSMOS_ENDPOINT; const COSMOS_KEY = process.env.COSMOS_KEY; @@ -146,6 +147,7 @@ async function getContainer(logger) { * @param {string|null} [feedback.reason] - Optional reason for the feedback * @param {string|null} feedback.userMessage - The user's message that prompted the response * @param {string|null} feedback.botResponse - The bot's response being rated + * @param {'synthesis'|'search'|'ask'} [feedback.responseType] - Response category for the feedback * @param {string} [feedback.interactionType] - Optional interaction type (e.g., 'slash_escalate') * @param {{ warn?: (msg: string) => void }} [feedback.logger] - Optional logger for warnings */ @@ -157,12 +159,17 @@ export async function recordFeedback({ reason, userMessage, botResponse, + responseType, interactionType, logger, }) { const c = await getContainer(logger); if (!c) return; + const normalizedResponseType = Object.values(FEEDBACK_RESPONSE_TYPES).includes(responseType) + ? responseType + : FEEDBACK_RESPONSE_TYPES.SYNTHESIS; + const doc = { id: `${userId}_${messageTs}`, // feedbackId duplicates id because the partition key path requires /feedbackId; @@ -175,6 +182,7 @@ export async function recordFeedback({ reason: reason?.trim() ? reason.trim() : null, userMessage, botResponse, + responseType: normalizedResponseType, ...(interactionType ? { interactionType } : {}), deploymentType: process.env.DEPLOYMENT_TYPE || 'local', timestamp: new Date().toISOString(), diff --git a/apps/fiona-slack/src/agent/llm-caller.js b/apps/fiona-slack/src/agent/llm-caller.js index 67859190..1806f7e2 100644 --- a/apps/fiona-slack/src/agent/llm-caller.js +++ b/apps/fiona-slack/src/agent/llm-caller.js @@ -3,6 +3,7 @@ // The Ed-Fi Alliance licenses this file to you under the Apache License, Version 2.0. // See the LICENSE and NOTICES files in the project root for more information. +import Perplexity from '@perplexity-ai/perplexity_ai'; import { OpenAI } from 'openai'; import { incrementDegradedNoMetadataCount, @@ -88,12 +89,16 @@ const SYSTEM_PROMPT = process.env.SYSTEM_PROMPT || DEFAULT_SYSTEM_PROMPT; // use the OpenAI SDK with a custom baseURL. /** @type {OpenAI | undefined} */ let perplexityClient; +// Dedicated Perplexity SDK client for the Search API (POST /search). +/** @type {Perplexity | undefined} */ +let perplexitySearchClient; if (PERPLEXITY_API_KEY) { perplexityClient = new OpenAI({ apiKey: PERPLEXITY_API_KEY, baseURL: 'https://api.perplexity.ai', }); + perplexitySearchClient = new Perplexity({ apiKey: PERPLEXITY_API_KEY }); } /** @@ -485,6 +490,52 @@ export async function callLLM(streamer, prompts, logger) { return { metadata, botText, systemPromptVersion: SYSTEM_PROMPT_VERSION }; } +// ─── Source Search ───────────────────────────────────────────────────────── +// Read from env (default 5). Hard-capped at SEARCH_ABSOLUTE_MAX before the API call. +const SEARCH_MAX_SOURCES = parsePositiveIntEnv(process.env.SEARCH_MAX_SOURCES, 5); +const SEARCH_ABSOLUTE_MAX = 10; + +/** + * Call the Perplexity Search API to retrieve source documents for a query. + * Returns a list of normalized sources (URL, title, optional snippet) with no + * synthesized answer. Used by the `/fiona search` command. + * + * Uses POST /search (not chat completions) so the API handles result ranking + * and count limiting natively via `max_results`. + * + * @param {string} query - Search query from the user (unsanitized) + * @param {Object} [options] + * @param {number} [options.maxSources] - Maximum sources to return (defaults to SEARCH_MAX_SOURCES env, capped at 10) + * @param {import('@slack/logger').Logger} [options.logger] + * @returns {Promise>} + */ +export async function searchForSources(query, { maxSources = SEARCH_MAX_SOURCES, logger } = {}) { + if (!perplexitySearchClient) return []; + if (!query || !query.trim()) return []; + + const cappedMaxSources = Math.min(maxSources, SEARCH_ABSOLUTE_MAX); + + try { + const response = await perplexitySearchClient.search.create({ + query, + max_results: cappedMaxSources, + search_domain_filter: PERPLEXITY_DOMAIN_FILTER, + }); + + const rawResults = response?.results; + + if (Array.isArray(rawResults) && rawResults.length > 0) { + const { sources } = normalizeSources(rawResults, { maxSources: cappedMaxSources }); + return sources; + } + + return []; + } catch (error) { + logger?.warn?.(`Search failed: ${error.message}`); + return []; + } +} + // ─── Escalation Summary ───────────────────────────────────────────────────── const ESCALATION_SUMMARY_SYSTEM_PROMPT = 'You summarize a Slack conversation between a user and Fiona (an Ed-Fi AI assistant) for a human support team. ' + diff --git a/apps/fiona-slack/src/agent/search-caller.js b/apps/fiona-slack/src/agent/search-caller.js new file mode 100644 index 00000000..83e7ad0f --- /dev/null +++ b/apps/fiona-slack/src/agent/search-caller.js @@ -0,0 +1,113 @@ +// SPDX-License-Identifier: Apache-2.0 +// Licensed to the Ed-Fi Alliance under one or more agreements. +// The Ed-Fi Alliance licenses this file to you under the Apache License, Version 2.0. +// See the LICENSE and NOTICES files in the project root for more information. + +/** + * Search abstraction layer for the `/fiona search` skill. + * Wraps the LLM-layer search call and provides Slack-specific formatting. + * Imported by fiona.js so the slash command handler never depends on llm-caller + * directly (preserving the architectural separation of concerns). + */ + +import { searchForSources } from './llm-caller.js'; + +export { searchForSources }; + +const SEARCH_NO_RESULTS_TEXT = '🔍 No sources found for _"{{query}}"_. Try rephrasing your query.'; +// Exported so slash and say()-based handlers can share a single error string. +export const SEARCH_ERROR_TEXT = ':warning: Search encountered an error. Please try again later.'; + +// Read snippet max word count from SEARCH_SNIPPET_MAX_WORDS env var. +// Falls back to 160 when the env var is absent, non-numeric, or not a positive integer. +const _snippetMaxRaw = Number.parseInt(process.env.SEARCH_SNIPPET_MAX_WORDS ?? '', 10); +const SNIPPET_MAX_WORDS = Number.isFinite(_snippetMaxRaw) && _snippetMaxRaw > 0 ? _snippetMaxRaw : 160; + +/** + * Strip common markdown syntax from a snippet and truncate to the configured + * maximum word count (SNIPPET_MAX_WORDS). + * Perplexity Search API snippets can contain bold markers, headings, and multi- + * paragraph text; stripping and truncating keeps the Slack source list compact. + * + * Only `**bold**` and `### heading` patterns are removed; single-asterisk italic + * is intentionally left untouched to avoid false positives on maths/code notation + * (e.g. `a * b`). + * + * @param {string} text + * @returns {string} + */ +function truncateSnippet(text) { + if (!text || typeof text !== 'string') return ''; + // Collapse newlines and runs of whitespace to a single space + let cleaned = text + .replace(/\n+/g, ' ') + .replace(/\s{2,}/g, ' ') + .trim(); + // Strip bold markers (**text**) and heading markers (## …) + cleaned = cleaned.replace(/\*\*([\s\S]*?)\*\*/g, '$1').replace(/#{1,6}\s+/g, ''); + const words = cleaned.split(/\s+/); + if (words.length <= SNIPPET_MAX_WORDS) return cleaned; + return `${words.slice(0, SNIPPET_MAX_WORDS).join(' ')}…`; +} + +/** + * Escape user-supplied text before embedding it in a Slack mrkdwn message. + * Only `&`, `<`, and `>` need HTML-entity encoding per the Slack API spec. + * + * @param {string} text + * @returns {string} + */ +export function escapeMrkdwn(text) { + if (!text || typeof text !== 'string') return ''; + return text.replace(/&/g, '&').replace(//g, '>'); +} + +/** + * Format a list of normalized sources into a Slack Block Kit message with + * a mrkdwn plain-text fallback. Each result is rendered as a section block + * (title link) and an optional context block (snippet), separated by dividers. + * + * @param {string} query - The original search query (unsanitized) + * @param {Array} sources - Normalized source list + * @returns {{ text: string, blocks: Array | null }} Slack message payload + */ +export function formatSearchResults(query, sources) { + const safeQuery = escapeMrkdwn(query); + + if (!Array.isArray(sources) || sources.length === 0) { + return { text: SEARCH_NO_RESULTS_TEXT.replace('{{query}}', safeQuery), blocks: null }; + } + + const headerText = `🔍 *Search results for:* _"${safeQuery}"_`; + + // ── Block Kit layout ─────────────────────────────────────────────────────── + const blocks = [{ type: 'section', text: { type: 'mrkdwn', text: headerText } }, { type: 'divider' }]; + + for (let i = 0; i < sources.length; i++) { + const source = sources[i]; + const link = `*<${source.url}|${escapeMrkdwn(source.title || source.hostname)}>*`; + blocks.push({ type: 'section', text: { type: 'mrkdwn', text: `${i + 1}. ${link}` } }); + + const rawSnippet = truncateSnippet(source.snippet); + if (rawSnippet) { + blocks.push({ + type: 'context', + elements: [{ type: 'mrkdwn', text: `_"${escapeMrkdwn(rawSnippet)}"_` }], + }); + } + + if (i < sources.length - 1) { + blocks.push({ type: 'divider' }); + } + } + + // ── Plain-text fallback (used by Slack notifications / accessibility) ────── + const items = sources.map((source, i) => { + const rawSnippet = truncateSnippet(source.snippet); + const snippet = rawSnippet ? `\n${rawSnippet}` : ''; + return `${i + 1}. ${source.title || source.hostname} — ${source.url}${snippet}`; + }); + const text = `${headerText}\n\n${items.join('\n\n')}`; + + return { text, blocks }; +} diff --git a/apps/fiona-slack/src/listeners/actions/feedback.js b/apps/fiona-slack/src/listeners/actions/feedback.js index a044a468..09795a02 100644 --- a/apps/fiona-slack/src/listeners/actions/feedback.js +++ b/apps/fiona-slack/src/listeners/actions/feedback.js @@ -3,6 +3,38 @@ // The Ed-Fi Alliance licenses this file to you under the Apache License, Version 2.0. // See the LICENSE and NOTICES files in the project root for more information. +import { FEEDBACK_RESPONSE_TYPES, parseFeedbackBlockId } from '../views/feedback_block.js'; + +const SEARCH_QUERY_PATTERN = /^🔍 \*Search results for:\* _"([\s\S]+?)"_/; + +/** + * Resolve the contextual feedback block id from the action payload. + * + * @param {import("@slack/bolt").SlackAction} body + * @param {Record} action + * @returns {string|null} + */ +function getFeedbackBlockId(body, action) { + if (typeof action?.block_id === 'string' && action.block_id.length > 0) { + return action.block_id; + } + + if (!Array.isArray(body?.message?.blocks)) return null; + return body.message.blocks.find((block) => block?.type === 'context_actions')?.block_id ?? null; +} + +/** + * Extract the original query from a formatted Fiona search response. + * + * @param {string} messageText + * @returns {string|null} + */ +function extractSearchQuery(messageText) { + if (typeof messageText !== 'string') return null; + const match = messageText.match(SEARCH_QUERY_PATTERN); + return match?.[1] ?? null; +} + /** * The `feedbackActionCallback` action responds to the `feedbackBlock` that displays * positive and negative feedback icons. This block is attached to the bottom of LLM @@ -34,6 +66,7 @@ export const feedbackActionCallback = async ({ ack, body, client, logger }) => { const channel_id = body.channel.id; const user_id = body.user.id; const value = action.value; + const { responseType, interactionType } = parseFeedbackBlockId(getFeedbackBlockId(body, action)); if (value !== 'good-feedback' && value !== 'bad-feedback') { logger.warn('Received unexpected feedback value', { value, channel_id, user_id, message_ts }); @@ -61,6 +94,11 @@ export const feedbackActionCallback = async ({ ack, body, client, logger }) => { userId: user_id, value, thread_ts, + responseType, + interactionType, + ...(responseType === FEEDBACK_RESPONSE_TYPES.SEARCH + ? { searchQuery: extractSearchQuery(body.message?.text) } + : {}), }), blocks: [ { diff --git a/apps/fiona-slack/src/listeners/assistant/message.js b/apps/fiona-slack/src/listeners/assistant/message.js index 4527cdaa..8da2800f 100644 --- a/apps/fiona-slack/src/listeners/assistant/message.js +++ b/apps/fiona-slack/src/listeners/assistant/message.js @@ -17,7 +17,7 @@ import { buildThreadHistory } from '../../agent/thread-history.js'; import { generateResponseId, shouldFinalize } from '../../agent/utils/idempotent-finalize.js'; import { dispatchKeywordViaSay } from '../commands/command-dispatch.js'; import { parseCommandKeyword } from '../commands/command-handler.js'; -import { feedbackBlock } from '../views/feedback_block.js'; +import { createFeedbackBlock, FEEDBACK_RESPONSE_TYPES } from '../views/feedback_block.js'; /** * Handles when users send messages or select a prompt in an assistant thread @@ -113,6 +113,7 @@ export const message = async ({ client, context, logger, message, say, setStatus threadTs: thread_ts, messageTs, source: 'assistant_escalate', + interactionType: 'assistant_message', }); return; } @@ -211,7 +212,12 @@ export const message = async ({ client, context, logger, message, say, setStatus text: 'The crowd appears to be astounded and applauds :popcorn:', }, ], - blocks: [feedbackBlock], + blocks: [ + createFeedbackBlock({ + responseType: FEEDBACK_RESPONSE_TYPES.SYNTHESIS, + interactionType: 'assistant_message', + }), + ], }); } else { // This second example shows a generated text response for the provided prompt @@ -253,7 +259,14 @@ export const message = async ({ client, context, logger, message, say, setStatus logger.info(`[citations] state=${metadata.finalize_state} sources=${metadata.sources?.length ?? 0}`); } - await streamer.stop({ blocks: [feedbackBlock] }); + await streamer.stop({ + blocks: [ + createFeedbackBlock({ + responseType: FEEDBACK_RESPONSE_TYPES.SYNTHESIS, + interactionType: 'assistant_message', + }), + ], + }); finalizeMetadataEnvelope(metadata); try { diff --git a/apps/fiona-slack/src/listeners/commands/command-dispatch.js b/apps/fiona-slack/src/listeners/commands/command-dispatch.js index 8eeb8c0b..ea8c726e 100644 --- a/apps/fiona-slack/src/listeners/commands/command-dispatch.js +++ b/apps/fiona-slack/src/listeners/commands/command-dispatch.js @@ -42,6 +42,7 @@ export async function dispatchKeywordViaSay({ threadTs, messageTs, source, + interactionType, }) { if (cmd.keyword === 'escalate') { // postEscalation records the escalate interaction itself; suppress the @@ -61,5 +62,5 @@ export async function dispatchKeywordViaSay({ }); return; } - await routeCommandViaSay(say, logger, cmd); + await routeCommandViaSay(say, logger, cmd, { interactionType }); } diff --git a/apps/fiona-slack/src/listeners/commands/command-handler.js b/apps/fiona-slack/src/listeners/commands/command-handler.js index 2227c1c4..e11ba66f 100644 --- a/apps/fiona-slack/src/listeners/commands/command-handler.js +++ b/apps/fiona-slack/src/listeners/commands/command-handler.js @@ -3,6 +3,9 @@ // The Ed-Fi Alliance licenses this file to you under the Apache License, Version 2.0. // See the LICENSE and NOTICES files in the project root for more information. +import { formatSearchResults, searchForSources } from '../../agent/search-caller.js'; +import { createFeedbackBlock, FEEDBACK_RESPONSE_TYPES } from '../views/feedback_block.js'; + export const HELP_TEXT = `*Fiona — your Ed-Fi AI assistant* :wave: Fiona helps you navigate Ed-Fi documentation, standards, and community resources using natural language. @@ -10,7 +13,7 @@ Fiona helps you navigate Ed-Fi documentation, standards, and community resources \`\`\` help Show this help message ask Ask a question about Ed-Fi (coming soon) -search Search Ed-Fi documentation (coming soon) +search Search Ed-Fi documentation \`\`\` *How to reach Fiona:* @@ -25,11 +28,6 @@ export const ASK_NOT_YET_TEXT = `In the meantime, @-mention Fiona in any channel or send a direct message. ` + `When available, it will also work as \`@fiona ask \` in a thread or the agent panel.`; -export const SEARCH_NOT_YET_TEXT = - `*/fiona search* is not yet available. ` + - `In the meantime, @-mention Fiona in any channel or send a direct message. ` + - `When available, it will also work as \`@fiona search \` in a thread or the agent panel.`; - // User-facing escalation copy, shared by the slash sub-command (fiona.js) and the // keyword path (escalation.js escalateViaSay) so both entry points stay in lockstep. export const ESCALATE_CONFIRM_TEXT = '✅ Your conversation has been escalated. A team member will follow up shortly.'; @@ -83,11 +81,6 @@ export function parseCommandKeyword(text) { return null; } -const NOT_YET_TEXT = { - ask: ASK_NOT_YET_TEXT, - search: SEARCH_NOT_YET_TEXT, -}; - /** * Dispatches a parsed command to the appropriate say() response. * Centralizes routing so each handler only calls this once. @@ -96,12 +89,13 @@ const NOT_YET_TEXT = { * @param {import('@slack/logger').Logger} logger * @param {{ keyword: string, rawArgs: string }} cmd */ -export async function routeCommandViaSay(say, logger, cmd) { +export async function routeCommandViaSay(say, logger, cmd, options = {}) { if (cmd.keyword === 'help') { await handleHelpViaSay(say, logger); + } else if (cmd.keyword === 'search') { + await handleSearchViaSay(say, logger, cmd.rawArgs, options); } else { - const text = NOT_YET_TEXT[cmd.keyword] ?? SEARCH_NOT_YET_TEXT; - await handleComingSoonViaSay(say, logger, cmd.keyword, text); + await handleComingSoonViaSay(say, logger, cmd.keyword, ASK_NOT_YET_TEXT); } } @@ -121,17 +115,43 @@ export async function handleHelpViaSay(say, logger) { } /** - * Sends a "coming soon" response via say() for ask/search commands in non-slash contexts. + * Performs a source search and sends results via say() — visible to all + * thread/channel participants. Used in contexts where slash-command ack() + * is not available (threads, agent panel, @-mention). + * + * @param {Function} say + * @param {import('@slack/logger').Logger} logger + * @param {string} query - The search query (rawArgs from parseCommandKeyword) + */ +export async function handleSearchViaSay(say, logger, query, { interactionType = null } = {}) { + const sources = await searchForSources(query, { logger }); + const { text, blocks } = formatSearchResults(query, sources); + const feedbackBlock = createFeedbackBlock({ + responseType: FEEDBACK_RESPONSE_TYPES.SEARCH, + interactionType, + }); + const responseBlocks = Array.isArray(blocks) + ? [...blocks, { type: 'divider' }, feedbackBlock] + : [{ type: 'section', text: { type: 'mrkdwn', text } }, { type: 'divider' }, feedbackBlock]; + try { + await say({ text, blocks: responseBlocks, unfurl_links: false, unfurl_media: false }); + } catch (err) { + logger?.error?.(`Failed to send search response: ${err.name}: ${err.message}`); + } +} + +/** + * Sends a "coming soon" response via say() for ask commands in non-slash contexts. * * @param {Function} say * @param {import('@slack/logger').Logger} logger - * @param {string} subCommand - 'ask' or 'search' + * @param {string} keyword - The command keyword ('ask') * @param {string} text - The coming-soon message text to send. */ -export async function handleComingSoonViaSay(say, logger, subCommand, text) { +export async function handleComingSoonViaSay(say, logger, keyword, text) { try { await say(text); } catch (err) { - logger?.error?.(`Failed to send coming-soon response for ${subCommand}: ${err.name}`); + logger?.error?.(`Failed to send coming-soon response for ${keyword}: ${err.name}`); } } diff --git a/apps/fiona-slack/src/listeners/commands/fiona.js b/apps/fiona-slack/src/listeners/commands/fiona.js index 05694d94..54c431d5 100644 --- a/apps/fiona-slack/src/listeners/commands/fiona.js +++ b/apps/fiona-slack/src/listeners/commands/fiona.js @@ -6,13 +6,14 @@ import { postEscalation } from '../../agent/escalation.js'; import { recordInteraction } from '../../agent/interaction-store.js'; import { checkRateLimit, rateLimitMessage } from '../../agent/rate-limiter.js'; +import { formatSearchResults, SEARCH_ERROR_TEXT, searchForSources } from '../../agent/search-caller.js'; +import { createFeedbackBlock, FEEDBACK_RESPONSE_TYPES } from '../views/feedback_block.js'; import { ASK_NOT_YET_TEXT, ESCALATE_CONFIRM_TEXT, ESCALATE_DM_TEXT, ESCALATE_ERROR_TEXT, HELP_TEXT, - SEARCH_NOT_YET_TEXT, } from './command-handler.js'; /** @@ -32,7 +33,7 @@ export const fionaCommandCallback = async ({ command, ack, respond, client, logg await handleComingSoon({ command, ack, logger, subCommand: 'ask', text: ASK_NOT_YET_TEXT }); break; case 'search': - await handleComingSoon({ command, ack, logger, subCommand: 'search', text: SEARCH_NOT_YET_TEXT }); + await handleSearch({ command, ack, respond, logger }); break; case 'escalate': await handleEscalate({ command, ack, respond, client, logger }); @@ -98,6 +99,70 @@ async function handleComingSoon({ command, ack, logger, subCommand, text }) { fireAndForgetRecord({ command, logger, interactionType: `slash_${subCommand}` }); } +async function handleSearch({ command, ack, respond, logger }) { + const rawText = (command.text ?? '').trim(); + // Extract everything after the leading 'search' token as the query. + const query = rawText.slice('search'.length).trim(); + + if (!query) { + // Empty query: fall back to help (same as /fiona with no sub-command) + await handleHelp({ command, ack, logger }); + return; + } + + try { + await ack(); + } catch (err) { + logger?.error?.(`Failed to acknowledge /fiona search: ${err.name}`); + return; + } + + if (!hasRequiredFields(command)) { + logger?.warn?.('Missing required slash command fields; skipping search'); + await respond({ response_type: 'ephemeral', text: SEARCH_ERROR_TEXT }); + return; + } + + const { allowed, retryAfterMs } = checkRateLimit(command.user_id); + if (!allowed) { + await respond({ response_type: 'ephemeral', text: rateLimitMessage(retryAfterMs) }); + recordInteraction({ + ...slashInteractionRecord(command, 'slash_search'), + status: 'error', + errorType: 'rate_limited', + rateLimited: true, + logger, + }).catch((err) => logger?.warn?.(`Failed to record slash_search interaction: ${err.name}`)); + return; + } + + logger?.info?.(`/fiona search: querying for "${query}"`); + const sources = await searchForSources(query, { logger }); + const { text, blocks } = formatSearchResults(query, sources); + const feedbackBlock = createFeedbackBlock({ + responseType: FEEDBACK_RESPONSE_TYPES.SEARCH, + interactionType: 'slash_search', + }); + const responseBlocks = Array.isArray(blocks) + ? [...blocks, { type: 'divider' }, feedbackBlock] + : [{ type: 'section', text: { type: 'mrkdwn', text } }, { type: 'divider' }, feedbackBlock]; + + try { + await respond({ + response_type: 'ephemeral', + text, + blocks: responseBlocks, + unfurl_links: false, + unfurl_media: false, + }); + } catch (err) { + logger?.error?.(`Failed to respond to /fiona search: ${err.name}`); + return; + } + + fireAndForgetRecord({ command, logger, interactionType: 'slash_search' }); +} + async function handleUnknown({ command, ack, logger, subCommand }) { logger?.warn?.(`Unrecognized /fiona sub-command: "${subCommand}"`); try { diff --git a/apps/fiona-slack/src/listeners/events/app_mention.js b/apps/fiona-slack/src/listeners/events/app_mention.js index 901ed8ca..e59d17f5 100644 --- a/apps/fiona-slack/src/listeners/events/app_mention.js +++ b/apps/fiona-slack/src/listeners/events/app_mention.js @@ -17,7 +17,7 @@ import { buildThreadHistory } from '../../agent/thread-history.js'; import { generateResponseId, shouldFinalize } from '../../agent/utils/idempotent-finalize.js'; import { dispatchKeywordViaSay } from '../commands/command-dispatch.js'; import { parseCommandKeyword } from '../commands/command-handler.js'; -import { feedbackBlock } from '../views/feedback_block.js'; +import { createFeedbackBlock, FEEDBACK_RESPONSE_TYPES } from '../views/feedback_block.js'; /** * Handles the event when the app is mentioned in a Slack conversation @@ -93,6 +93,7 @@ export const appMentionCallback = async ({ event, client, logger, say }) => { threadTs: thread_ts, messageTs, source: 'mention_escalate', + interactionType: 'app_mention', }); return; } @@ -136,7 +137,14 @@ export const appMentionCallback = async ({ event, client, logger, say }) => { logger.info(`[citations] state=${metadata.finalize_state} sources=${metadata.sources?.length ?? 0}`); } - await streamer.stop({ blocks: [feedbackBlock] }); + await streamer.stop({ + blocks: [ + createFeedbackBlock({ + responseType: FEEDBACK_RESPONSE_TYPES.SYNTHESIS, + interactionType: 'app_mention', + }), + ], + }); finalizeMetadataEnvelope(metadata); try { diff --git a/apps/fiona-slack/src/listeners/views/feedback_block.js b/apps/fiona-slack/src/listeners/views/feedback_block.js index dbb65960..47d404e7 100644 --- a/apps/fiona-slack/src/listeners/views/feedback_block.js +++ b/apps/fiona-slack/src/listeners/views/feedback_block.js @@ -3,27 +3,63 @@ // The Ed-Fi Alliance licenses this file to you under the Apache License, Version 2.0. // See the LICENSE and NOTICES files in the project root for more information. +import { FEEDBACK_RESPONSE_TYPES } from '../../agent/feedback-response-types.js'; + +export { FEEDBACK_RESPONSE_TYPES }; + /** * `feedbackBlock` are feedback buttons included with messages. * * @type {import("@slack/bolt").types.ContextActionsBlock} */ -export const feedbackBlock = { - type: 'context_actions', - elements: [ - { - type: 'feedback_buttons', - action_id: 'feedback', - positive_button: { - text: { type: 'plain_text', text: 'Good Response' }, - accessibility_label: 'Submit positive feedback on this response', - value: 'good-feedback', - }, - negative_button: { - text: { type: 'plain_text', text: 'Bad Response' }, - accessibility_label: 'Submit negative feedback on this response', - value: 'bad-feedback', +const FEEDBACK_BLOCK_PREFIX = 'feedback'; + +export function buildFeedbackBlockId(responseType = FEEDBACK_RESPONSE_TYPES.SYNTHESIS, interactionType = null) { + return interactionType + ? `${FEEDBACK_BLOCK_PREFIX}|${responseType}|${interactionType}` + : `${FEEDBACK_BLOCK_PREFIX}|${responseType}`; +} + +export function parseFeedbackBlockId(blockId) { + if (typeof blockId !== 'string' || !blockId.startsWith(`${FEEDBACK_BLOCK_PREFIX}|`)) { + return { responseType: FEEDBACK_RESPONSE_TYPES.SYNTHESIS, interactionType: null }; + } + + const [prefix, responseType, interactionType = ''] = blockId.split('|'); + if (prefix !== FEEDBACK_BLOCK_PREFIX) { + return { responseType: FEEDBACK_RESPONSE_TYPES.SYNTHESIS, interactionType: null }; + } + const normalizedResponseType = Object.values(FEEDBACK_RESPONSE_TYPES).includes(responseType) + ? responseType + : FEEDBACK_RESPONSE_TYPES.SYNTHESIS; + + return { + responseType: normalizedResponseType, + interactionType: interactionType || null, + }; +} + +export function createFeedbackBlock({ responseType = FEEDBACK_RESPONSE_TYPES.SYNTHESIS, interactionType = null } = {}) { + return { + type: 'context_actions', + block_id: buildFeedbackBlockId(responseType, interactionType), + elements: [ + { + type: 'feedback_buttons', + action_id: 'feedback', + positive_button: { + text: { type: 'plain_text', text: 'Good Response' }, + accessibility_label: 'Submit positive feedback on this response', + value: 'good-feedback', + }, + negative_button: { + text: { type: 'plain_text', text: 'Bad Response' }, + accessibility_label: 'Submit negative feedback on this response', + value: 'bad-feedback', + }, }, - }, - ], -}; + ], + }; +} + +export const feedbackBlock = createFeedbackBlock(); diff --git a/apps/fiona-slack/src/listeners/views/feedback_reason.js b/apps/fiona-slack/src/listeners/views/feedback_reason.js index bb81328c..2b3470d8 100644 --- a/apps/fiona-slack/src/listeners/views/feedback_reason.js +++ b/apps/fiona-slack/src/listeners/views/feedback_reason.js @@ -4,6 +4,57 @@ // See the LICENSE and NOTICES files in the project root for more information. import { recordFeedback } from '../../agent/feedback-store.js'; +import { FEEDBACK_RESPONSE_TYPES } from './feedback_block.js'; + +/** + * Retrieve thread-based feedback context by locating the rated bot message and + * the user message immediately before it. + * + * @param {import("@slack/web-api").WebClient} client + * @param {string} channelId + * @param {string} threadTs + * @param {string} messageTs + * @returns {Promise<{ userMessage: string | null, botResponse: string | null }>} + */ +async function fetchThreadContext(client, channelId, threadTs, messageTs) { + const { messages } = await client.conversations.replies({ channel: channelId, ts: threadTs }); + if (!messages) { + return { userMessage: null, botResponse: null }; + } + + const botIndex = messages.findIndex((message) => message.ts === messageTs); + if (botIndex < 0) { + return { userMessage: null, botResponse: null }; + } + + const botResponse = messages[botIndex].text ?? null; + const preceding = botIndex > 0 ? messages[botIndex - 1] : null; + return { + userMessage: preceding?.text ?? null, + botResponse, + }; +} + +/** + * Retrieve the text of a single Slack message by timestamp. + * + * @param {import("@slack/web-api").WebClient} client + * @param {string} channelId + * @param {string} messageTs + * @returns {Promise} + */ +async function fetchMessageText(client, channelId, messageTs) { + const { messages } = await client.conversations.history({ + channel: channelId, + latest: messageTs, + inclusive: true, + limit: 1, + }); + if (!Array.isArray(messages)) { + return null; + } + return messages?.[0]?.text ?? null; +} /** * Handles the `feedback_reason` modal submission. Records the feedback and reason @@ -17,7 +68,10 @@ import { recordFeedback } from '../../agent/feedback-store.js'; */ export const feedbackReasonViewCallback = async ({ ack, view, client, logger }) => { try { - const { channelId, messageTs, userId, value, thread_ts } = JSON.parse(view.private_metadata); + const { channelId, messageTs, userId, value, thread_ts, responseType, interactionType, searchQuery } = JSON.parse( + view.private_metadata, + ); + const normalizedResponseType = responseType ?? FEEDBACK_RESPONSE_TYPES.SYNTHESIS; const rawReason = view.state.values?.reason_block?.reason_input?.value; const trimmedReason = typeof rawReason === 'string' ? rawReason.trim() : ''; @@ -27,20 +81,16 @@ export const feedbackReasonViewCallback = async ({ ack, view, client, logger }) } await ack(); - let userMessage = null; + let userMessage = normalizedResponseType === FEEDBACK_RESPONSE_TYPES.SEARCH ? (searchQuery ?? null) : null; let botResponse = null; try { - const { messages } = await client.conversations.replies({ channel: channelId, ts: thread_ts }); - if (messages) { - const botIndex = messages.findIndex((m) => m.ts === messageTs); - if (botIndex >= 0) { - botResponse = messages[botIndex].text ?? null; - const preceding = botIndex > 0 ? messages[botIndex - 1] : null; - if (preceding?.text) userMessage = preceding.text; - } + if (normalizedResponseType === FEEDBACK_RESPONSE_TYPES.SYNTHESIS) { + ({ userMessage, botResponse } = await fetchThreadContext(client, channelId, thread_ts, messageTs)); + } else { + botResponse = await fetchMessageText(client, channelId, messageTs); } } catch (e) { - logger.error('Failed to fetch thread context:', e); + logger.error('Failed to fetch feedback context:', e); } try { @@ -52,6 +102,8 @@ export const feedbackReasonViewCallback = async ({ ack, view, client, logger }) reason: rawReason, userMessage, botResponse, + responseType: normalizedResponseType, + interactionType, logger, }); } catch (e) { @@ -87,7 +139,8 @@ export const feedbackReasonViewCallback = async ({ ack, view, client, logger }) export const feedbackReasonClosedCallback = async ({ ack, view, logger }) => { try { await ack(); - const { channelId, messageTs, userId, value } = JSON.parse(view.private_metadata); + const { channelId, messageTs, userId, value, responseType, interactionType } = JSON.parse(view.private_metadata); + const normalizedResponseType = responseType ?? FEEDBACK_RESPONSE_TYPES.SYNTHESIS; if (value !== 'good-feedback') return; await recordFeedback({ userId, @@ -97,6 +150,8 @@ export const feedbackReasonClosedCallback = async ({ ack, view, logger }) => { reason: null, userMessage: null, botResponse: null, + responseType: normalizedResponseType, + interactionType, logger, }); } catch (error) { diff --git a/apps/fiona-slack/tests/agent/escalation.test.js b/apps/fiona-slack/tests/agent/escalation.test.js index f3cc979d..863d343f 100644 --- a/apps/fiona-slack/tests/agent/escalation.test.js +++ b/apps/fiona-slack/tests/agent/escalation.test.js @@ -11,7 +11,7 @@ const mockSummarize = jest.fn(); jest.unstable_mockModule('../../src/agent/interaction-store.js', () => ({ recordInteraction: mockRecordInteraction })); jest.unstable_mockModule('../../src/agent/feedback-store.js', () => ({ recordFeedback: mockRecordFeedback })); -jest.unstable_mockModule('../../src/agent/llm-caller.js', () => ({ summarizeForEscalation: mockSummarize })); +jest.unstable_mockModule('../../src/agent/llm-caller.js', () => ({ summarizeForEscalation: mockSummarize, searchForSources: jest.fn().mockResolvedValue([]) })); const { postEscalation, escalateViaSay } = await import('../../src/agent/escalation.js'); const { ESCALATE_CONFIRM_TEXT, ESCALATE_DM_TEXT, ESCALATE_ERROR_TEXT } = await import( diff --git a/apps/fiona-slack/tests/agent/feedback-store-cosmos.test.js b/apps/fiona-slack/tests/agent/feedback-store-cosmos.test.js index ae447286..9ab6ee4c 100644 --- a/apps/fiona-slack/tests/agent/feedback-store-cosmos.test.js +++ b/apps/fiona-slack/tests/agent/feedback-store-cosmos.test.js @@ -67,6 +67,7 @@ describe('recordFeedback - with connection string', () => { value: 'bad-feedback', userMessage: 'How do I use the API?', botResponse: 'Use the REST API.', + responseType: 'search', }); const [doc] = mockUpsert.mock.calls[0]; @@ -76,6 +77,7 @@ describe('recordFeedback - with connection string', () => { expect(doc.value).toBe('bad-feedback'); expect(doc.userMessage).toBe('How do I use the API?'); expect(doc.botResponse).toBe('Use the REST API.'); + expect(doc.responseType).toBe('search'); }); it('includes a timestamp in ISO 8601 format', async () => { @@ -229,4 +231,17 @@ describe('recordFeedback - with connection string', () => { const [doc] = mockUpsert.mock.calls[0]; expect(doc).not.toHaveProperty('interactionType'); }); + + it('defaults responseType to synthesis when not provided', async () => { + await recordFeedback({ + userId: 'U902', + channelId: 'C902', + messageTs: '1234567890.000100', + value: 'good-feedback', + userMessage: null, + botResponse: null, + }); + const [doc] = mockUpsert.mock.calls[0]; + expect(doc.responseType).toBe('synthesis'); + }); }); diff --git a/apps/fiona-slack/tests/agent/search-caller.test.js b/apps/fiona-slack/tests/agent/search-caller.test.js new file mode 100644 index 00000000..c475fb27 --- /dev/null +++ b/apps/fiona-slack/tests/agent/search-caller.test.js @@ -0,0 +1,328 @@ +// SPDX-License-Identifier: Apache-2.0 +// Licensed to the Ed-Fi Alliance under one or more agreements. +// The Ed-Fi Alliance licenses this file to you under the Apache License, Version 2.0. +// See the LICENSE and NOTICES files in the project root for more information. + +import { describe, it, expect, jest, beforeEach } from '@jest/globals'; + +// Mock the OpenAI module to prevent module init errors (llm-caller.js creates +// an OpenAI client on load when PERPLEXITY_API_KEY is set). +jest.unstable_mockModule('openai', () => ({ + OpenAI: jest.fn().mockImplementation(() => ({ + chat: { completions: { create: jest.fn() } }, + })), +})); + +// Mock the Perplexity SDK so tests control search results without hitting the API. +const mockSearchCreate = jest.fn(); +jest.unstable_mockModule('@perplexity-ai/perplexity_ai', () => ({ + default: jest.fn().mockImplementation(() => ({ + search: { create: mockSearchCreate }, + })), +})); + +process.env.PERPLEXITY_API_KEY = 'test-key'; + +const { searchForSources, formatSearchResults, escapeMrkdwn, SEARCH_ERROR_TEXT } = await import( + '../../src/agent/search-caller.js' +); + +/** + * Configure mockSearchCreate to resolve with the given results array. + * + * @param {Array<{url: string, title?: string, snippet?: string}>} results + */ +function mockSearchOk(results) { + mockSearchCreate.mockResolvedValue({ results }); +} + +describe('searchForSources', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('returns normalized sources from search results', async () => { + mockSearchOk([ + { url: 'https://docs.ed-fi.org/assessment', title: 'Assessment API', snippet: 'Snippet text' }, + { url: 'https://www.ed-fi.org/guide', title: 'API Guide', snippet: null }, + ]); + const sources = await searchForSources('assessment API'); + expect(sources).toHaveLength(2); + expect(sources[0].url).toBe('https://docs.ed-fi.org/assessment'); + expect(sources[0].title).toBe('Assessment API'); + expect(sources[0].snippet).toBe('Snippet text'); + expect(sources[1].url).toBe('https://www.ed-fi.org/guide'); + }); + + it('returns empty array when results array is empty', async () => { + mockSearchOk([]); + const sources = await searchForSources('nothing'); + expect(sources).toEqual([]); + }); + + it('returns empty array when results field is absent', async () => { + mockSearchCreate.mockResolvedValue({}); + const sources = await searchForSources('nothing'); + expect(sources).toEqual([]); + }); + + it('returns empty array for empty query without calling the API', async () => { + const sources = await searchForSources(''); + expect(mockSearchCreate).not.toHaveBeenCalled(); + expect(sources).toEqual([]); + }); + + it('returns empty array for whitespace-only query without calling the API', async () => { + const sources = await searchForSources(' '); + expect(mockSearchCreate).not.toHaveBeenCalled(); + expect(sources).toEqual([]); + }); + + it('caps max_results at 10 even when maxSources exceeds 10', async () => { + mockSearchOk([]); + await searchForSources('query', { maxSources: 20 }); + expect(mockSearchCreate).toHaveBeenCalledWith( + expect.objectContaining({ max_results: 10 }), + ); + }); + + it('passes max_results to the SDK and caps results via normalizeSources', async () => { + const manyResults = Array.from({ length: 10 }, (_, i) => ({ + url: `https://docs.ed-fi.org/page-${i}`, + title: `Page ${i}`, + snippet: `Snippet ${i}`, + })); + mockSearchOk(manyResults); + const sources = await searchForSources('query', { maxSources: 3 }); + expect(sources).toHaveLength(3); + expect(mockSearchCreate).toHaveBeenCalledWith( + expect.objectContaining({ max_results: 3 }), + ); + }); + + it('passes search_domain_filter to the SDK', async () => { + mockSearchOk([]); + await searchForSources('query'); + expect(mockSearchCreate).toHaveBeenCalledWith( + expect.objectContaining({ search_domain_filter: expect.any(Array) }), + ); + }); + + it('warns and returns empty array when the SDK throws', async () => { + mockSearchCreate.mockRejectedValue(new Error('network failure')); + const logger = { warn: jest.fn() }; + const sources = await searchForSources('query', { logger }); + expect(sources).toEqual([]); + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('Search failed')); + }); + + it('does not throw when the SDK throws', async () => { + mockSearchCreate.mockRejectedValue(new Error('boom')); + await expect(searchForSources('query')).resolves.toEqual([]); + }); +}); + +describe('formatSearchResults', () => { + const sampleSources = [ + { + url: 'https://docs.ed-fi.org/assessment', + title: 'Assessment API', + hostname: 'docs.ed-fi.org', + snippet: 'The Assessment API provides endpoints for assessment metadata.', + }, + { + url: 'https://www.ed-fi.org/guide', + title: 'API Guide', + hostname: 'www.ed-fi.org', + snippet: null, + }, + ]; + + it('includes the query in the header text', () => { + const { text } = formatSearchResults('assessment API', sampleSources); + expect(text).toContain('assessment API'); + }); + + it('includes source URLs in text fallback', () => { + const { text } = formatSearchResults('query', sampleSources); + expect(text).toContain('https://docs.ed-fi.org/assessment'); + expect(text).toContain('https://www.ed-fi.org/guide'); + }); + + it('includes source titles in text fallback', () => { + const { text } = formatSearchResults('query', sampleSources); + expect(text).toContain('Assessment API'); + expect(text).toContain('API Guide'); + }); + + it('includes snippet in text fallback when present', () => { + const { text } = formatSearchResults('query', sampleSources); + expect(text).toContain('The Assessment API provides endpoints for assessment metadata.'); + }); + + it('returns no-results text and null blocks for empty sources array', () => { + const { text, blocks } = formatSearchResults('unknown topic', []); + expect(text).toContain('No sources found'); + expect(text).toContain('unknown topic'); + expect(blocks).toBeNull(); + }); + + it('returns no-results text and null blocks for null/undefined sources', () => { + expect(formatSearchResults('query', null).text).toContain('No sources found'); + expect(formatSearchResults('query', null).blocks).toBeNull(); + expect(formatSearchResults('query', undefined).text).toContain('No sources found'); + }); + + it('numbers each result starting at 1 in text fallback', () => { + const { text } = formatSearchResults('query', sampleSources); + expect(text).toMatch(/1\./); + expect(text).toMatch(/2\./); + }); + + it('escapes & < > characters in the query', () => { + const { text } = formatSearchResults('search & more', []); + expect(text).not.toContain('&find'); + expect(text).toContain('&'); + expect(text).toContain('<'); + expect(text).toContain('>'); + }); + + it('returns blocks array for non-empty sources', () => { + const { blocks } = formatSearchResults('query', sampleSources); + expect(Array.isArray(blocks)).toBe(true); + expect(blocks.length).toBeGreaterThan(0); + }); + + it('blocks include a section with the header text', () => { + const { blocks, text } = formatSearchResults('my query', sampleSources); + const header = blocks.find((b) => b.type === 'section' && b.text?.text?.includes('my query')); + expect(header).toBeDefined(); + expect(text).toContain('my query'); + }); + + it('blocks include divider blocks between results', () => { + const { blocks } = formatSearchResults('query', sampleSources); + const dividers = blocks.filter((b) => b.type === 'divider'); + expect(dividers.length).toBeGreaterThanOrEqual(1); + }); + + it('blocks include a section block with source link for each result', () => { + const { blocks } = formatSearchResults('query', sampleSources); + const sections = blocks.filter((b) => b.type === 'section' && b.text?.text?.includes('docs.ed-fi.org/assessment')); + expect(sections.length).toBeGreaterThan(0); + }); + + it('blocks include a context block with snippet for sources with snippets', () => { + const { blocks } = formatSearchResults('query', sampleSources); + const contextBlocks = blocks.filter((b) => b.type === 'context'); + expect(contextBlocks.length).toBeGreaterThan(0); + const snippetBlock = contextBlocks.find((b) => + b.elements?.some((el) => el.text?.includes('Assessment API provides')), + ); + expect(snippetBlock).toBeDefined(); + }); + + it('strips ** bold markers from snippet in blocks', () => { + const sources = [ + { + url: 'https://docs.ed-fi.org/a', + title: 'A', + hostname: 'docs.ed-fi.org', + snippet: '**Ed-Fi API v8** is the next-generation platform.', + }, + ]; + const { blocks } = formatSearchResults('query', sources); + const contextBlocks = blocks.filter((b) => b.type === 'context'); + expect(contextBlocks.length).toBeGreaterThan(0); + const blockText = contextBlocks.map((b) => b.elements?.map((el) => el.text).join('')).join(''); + expect(blockText).not.toContain('**'); + expect(blockText).toContain('Ed-Fi API v8'); + }); + + it('strips markdown heading markers from snippet', () => { + const sources = [ + { + url: 'https://docs.ed-fi.org/a', + title: 'A', + hostname: 'docs.ed-fi.org', + snippet: '### Important Epics\n- DMS-928 - Relational storage model', + }, + ]; + const { text, blocks } = formatSearchResults('query', sources); + expect(text).not.toContain('###'); + expect(text).toContain('Important Epics'); + const contextBlocks = blocks.filter((b) => b.type === 'context'); + const blockText = contextBlocks.map((b) => b.elements?.map((el) => el.text).join('')).join(''); + expect(blockText).not.toContain('###'); + }); + + it('collapses newlines in snippet to a single line', () => { + const sources = [ + { + url: 'https://docs.ed-fi.org/a', + title: 'A', + hostname: 'docs.ed-fi.org', + snippet: 'Line one.\nLine two.\nLine three.', + }, + ]; + const { blocks } = formatSearchResults('query', sources); + const contextBlocks = blocks.filter((b) => b.type === 'context'); + const blockText = contextBlocks.map((b) => b.elements?.map((el) => el.text).join('')).join(''); + expect(blockText).not.toContain('\n'); + }); + + it('truncates long snippets to 160 words and appends ellipsis', () => { + const longSnippet = Array.from({ length: 200 }, (_, i) => `word${i}`).join(' '); + const sources = [ + { + url: 'https://docs.ed-fi.org/a', + title: 'A', + hostname: 'docs.ed-fi.org', + snippet: longSnippet, + }, + ]; + const { blocks } = formatSearchResults('query', sources); + const contextBlocks = blocks.filter((b) => b.type === 'context'); + const blockText = contextBlocks.map((b) => b.elements?.map((el) => el.text).join('')).join(''); + expect(blockText).toContain('…'); + // Extract the plain snippet text (strip the surrounding italic markers) + const snippetContent = blockText.replace(/^_"/, '').replace(/"_$/, ''); + const wordCount = snippetContent.replace('…', '').trim().split(/\s+/).length; + expect(wordCount).toBeLessThanOrEqual(160); + }); +}); + +describe('escapeMrkdwn', () => { + it('escapes & to &', () => { + expect(escapeMrkdwn('a & b')).toBe('a & b'); + }); + + it('escapes < to <', () => { + expect(escapeMrkdwn('a < b')).toBe('a < b'); + }); + + it('escapes > to >', () => { + expect(escapeMrkdwn('a > b')).toBe('a > b'); + }); + + it('handles empty string', () => { + expect(escapeMrkdwn('')).toBe(''); + }); + + it('returns empty string for non-string input', () => { + expect(escapeMrkdwn(null)).toBe(''); + expect(escapeMrkdwn(undefined)).toBe(''); + }); + + it('leaves plain text unchanged', () => { + expect(escapeMrkdwn('hello world')).toBe('hello world'); + }); +}); + +describe('SEARCH_ERROR_TEXT', () => { + it('is a non-empty string', () => { + expect(typeof SEARCH_ERROR_TEXT).toBe('string'); + expect(SEARCH_ERROR_TEXT.length).toBeGreaterThan(0); + }); +}); + diff --git a/apps/fiona-slack/tests/listeners/actions/feedback.test.js b/apps/fiona-slack/tests/listeners/actions/feedback.test.js index d32ef1e3..c19b40ac 100644 --- a/apps/fiona-slack/tests/listeners/actions/feedback.test.js +++ b/apps/fiona-slack/tests/listeners/actions/feedback.test.js @@ -31,9 +31,9 @@ describe('feedbackActionCallback', () => { message: { ts: '1234567890.000001', thread_ts: '1234567890.000000', - text: 'Bot response text', + text: '🔍 *Search results for:* _"Bot response text"_', }, - actions: [{ type: 'feedback_buttons', value: 'good-feedback' }], + actions: [{ type: 'feedback_buttons', value: 'good-feedback', block_id: 'feedback|search|slash_search' }], }; }); @@ -120,7 +120,7 @@ describe('feedbackActionCallback', () => { expect(view.blocks[0].optional).toBe(false); }); - it('private_metadata contains channelId, messageTs, userId, value, thread_ts', async () => { + it('private_metadata contains channelId, messageTs, userId, value, thread_ts, and feedback context', async () => { await feedbackActionCallback({ ack: mockAck, body: mockBody, client: mockClient, logger: mockLogger }); const [{ view }] = mockClient.views.open.mock.calls[0]; @@ -131,6 +131,9 @@ describe('feedbackActionCallback', () => { userId: 'U123', value: 'good-feedback', thread_ts: '1234567890.000000', + responseType: 'search', + interactionType: 'slash_search', + searchQuery: 'Bot response text', }); }); @@ -144,6 +147,18 @@ describe('feedbackActionCallback', () => { expect(meta.thread_ts).toBe('1234567890.000001'); }); + it('defaults feedback context to synthesis when block_id is absent', async () => { + delete mockBody.actions[0].block_id; + + await feedbackActionCallback({ ack: mockAck, body: mockBody, client: mockClient, logger: mockLogger }); + + const [{ view }] = mockClient.views.open.mock.calls[0]; + const meta = JSON.parse(view.private_metadata); + expect(meta.responseType).toBe('synthesis'); + expect(meta.interactionType).toBeNull(); + expect(meta).not.toHaveProperty('searchQuery'); + }); + it('logs error and does not throw when views.open rejects', async () => { mockClient.views.open.mockRejectedValueOnce(new Error('trigger expired')); diff --git a/apps/fiona-slack/tests/listeners/assistant/message.test.js b/apps/fiona-slack/tests/listeners/assistant/message.test.js index 4ada81fe..06be3df4 100644 --- a/apps/fiona-slack/tests/listeners/assistant/message.test.js +++ b/apps/fiona-slack/tests/listeners/assistant/message.test.js @@ -14,6 +14,7 @@ jest.unstable_mockModule('../../../src/agent/llm-caller.js', () => ({ callLLM: jest.fn().mockResolvedValue({ metadata: null, botText: '', systemPromptVersion: 'v1' }), finalizeMetadataEnvelope: jest.fn(), handleMetadataTimeout: jest.fn(), + searchForSources: jest.fn().mockResolvedValue([]), LLM_MODEL: 'sonar-pro', SYSTEM_PROMPT_VERSION: 'v1', CITATION_POLICY: { @@ -607,7 +608,7 @@ describe('message (assistant thread handler)', () => { expect(callLLM).not.toHaveBeenCalled(); }); - it('responds with coming-soon text when message starts with "search "', async () => { + it('responds with search results when message starts with "search "', async () => { mockMessage.text = 'search Data Standard 6.0'; await messageHandler({ @@ -620,7 +621,9 @@ describe('message (assistant thread handler)', () => { }); expect(mockSay).toHaveBeenCalledTimes(1); - expect(mockSay.mock.calls[0][0]).toMatch(/not yet available/i); + const sayArg = mockSay.mock.calls[0][0]; + const sayText = typeof sayArg === 'string' ? sayArg : sayArg?.text ?? ''; + expect(sayText).toMatch(/search results|No sources found/i); expect(callLLM).not.toHaveBeenCalled(); }); diff --git a/apps/fiona-slack/tests/listeners/commands/command-handler.test.js b/apps/fiona-slack/tests/listeners/commands/command-handler.test.js index cee33ddd..566233ee 100644 --- a/apps/fiona-slack/tests/listeners/commands/command-handler.test.js +++ b/apps/fiona-slack/tests/listeners/commands/command-handler.test.js @@ -5,8 +5,29 @@ import { describe, it, expect, jest, beforeEach } from '@jest/globals'; -const { parseCommandKeyword, handleHelpViaSay, handleComingSoonViaSay, routeCommandViaSay, HELP_TEXT, ASK_NOT_YET_TEXT, SEARCH_NOT_YET_TEXT } = - await import('../../../src/listeners/commands/command-handler.js'); +// Mock search-caller so command-handler tests don't load the LLM client. +const mockSearchForSources = jest.fn().mockResolvedValue([]); +const mockFormatSearchResults = jest.fn().mockImplementation((_q, sources) => ({ + text: sources.length === 0 ? '🔍 No sources found.' : `🔍 Found ${sources.length} source(s).`, + blocks: null, +})); + +jest.unstable_mockModule('../../../src/agent/search-caller.js', () => ({ + searchForSources: mockSearchForSources, + formatSearchResults: mockFormatSearchResults, + escapeMrkdwn: (t) => t, + SEARCH_ERROR_TEXT: ':warning: Search error.', +})); + +const { + parseCommandKeyword, + handleHelpViaSay, + handleSearchViaSay, + handleComingSoonViaSay, + routeCommandViaSay, + HELP_TEXT, + ASK_NOT_YET_TEXT, +} = await import('../../../src/listeners/commands/command-handler.js'); describe('parseCommandKeyword', () => { describe('help keyword', () => { @@ -193,6 +214,12 @@ describe('HELP_TEXT', () => { it('mentions fiona help as the two-word keyword alternative', () => { expect(HELP_TEXT).toMatch('fiona help'); }); + + it('lists search command without "(coming soon)"', () => { + expect(HELP_TEXT).toMatch('search '); + // Only 'ask' remains as coming soon; search is now available + expect(HELP_TEXT).not.toMatch(/search.*coming soon/); + }); }); describe('handleHelpViaSay', () => { @@ -222,6 +249,73 @@ describe('handleHelpViaSay', () => { }); }); +describe('handleSearchViaSay', () => { + let mockSay; + let mockLogger; + + beforeEach(() => { + jest.clearAllMocks(); + mockSay = jest.fn().mockResolvedValue(undefined); + mockLogger = { error: jest.fn(), warn: jest.fn(), info: jest.fn() }; + mockSearchForSources.mockResolvedValue([]); + mockFormatSearchResults.mockImplementation((_q, sources) => ({ + text: sources.length === 0 ? '🔍 No sources found.' : `🔍 Found ${sources.length} source(s).`, + blocks: null, + })); + }); + + it('calls searchForSources with the query and logger', async () => { + await handleSearchViaSay(mockSay, mockLogger, 'Ed-Fi API'); + expect(mockSearchForSources).toHaveBeenCalledWith('Ed-Fi API', { logger: mockLogger }); + }); + + it('calls say() with the formatted results', async () => { + mockSearchForSources.mockResolvedValueOnce([ + { url: 'https://docs.ed-fi.org/', title: 'Ed-Fi Docs', hostname: 'docs.ed-fi.org' }, + ]); + mockFormatSearchResults.mockReturnValueOnce({ text: '🔍 Found 1 source(s).', blocks: null }); + await handleSearchViaSay(mockSay, mockLogger, 'Ed-Fi API'); + expect(mockSay).toHaveBeenCalledWith( + expect.objectContaining({ + text: '🔍 Found 1 source(s).', + blocks: expect.arrayContaining([expect.objectContaining({ block_id: 'feedback|search' })]), + }), + ); + }); + + it('calls say() with no-results message when sources are empty', async () => { + await handleSearchViaSay(mockSay, mockLogger, 'unknown query'); + expect(mockSay).toHaveBeenCalledWith(expect.objectContaining({ text: '🔍 No sources found.' })); + }); + + it('disables link unfurls for search results', async () => { + await handleSearchViaSay(mockSay, mockLogger, 'Ed-Fi API'); + expect(mockSay).toHaveBeenCalledWith( + expect.objectContaining({ unfurl_links: false, unfurl_media: false }), + ); + }); + + it('tags say-based search feedback with the supplied interaction type', async () => { + await handleSearchViaSay(mockSay, mockLogger, 'Ed-Fi API', { interactionType: 'assistant_message' }); + expect(mockSay).toHaveBeenCalledWith( + expect.objectContaining({ + blocks: expect.arrayContaining([expect.objectContaining({ block_id: 'feedback|search|assistant_message' })]), + }), + ); + }); + + it('logs error when say() throws', async () => { + mockSay.mockRejectedValueOnce(new Error('say error')); + await handleSearchViaSay(mockSay, mockLogger, 'Ed-Fi API'); + expect(mockLogger.error).toHaveBeenCalledWith(expect.stringContaining('Failed to send search response')); + }); + + it('does not throw when say() throws', async () => { + mockSay.mockRejectedValueOnce(new Error('say error')); + await expect(handleSearchViaSay(mockSay, mockLogger, 'Ed-Fi API')).resolves.not.toThrow(); + }); +}); + describe('handleComingSoonViaSay', () => { let mockSay; let mockLogger; @@ -236,11 +330,6 @@ describe('handleComingSoonViaSay', () => { expect(mockSay).toHaveBeenCalledWith(ASK_NOT_YET_TEXT); }); - it('calls say() with SEARCH_NOT_YET_TEXT for search sub-command', async () => { - await handleComingSoonViaSay(mockSay, mockLogger, 'search', SEARCH_NOT_YET_TEXT); - expect(mockSay).toHaveBeenCalledWith(SEARCH_NOT_YET_TEXT); - }); - it('logs error when say() throws', async () => { mockSay.mockRejectedValueOnce(new Error('timeout')); await handleComingSoonViaSay(mockSay, mockLogger, 'ask', ASK_NOT_YET_TEXT); @@ -257,10 +346,6 @@ describe('handleComingSoonViaSay', () => { it('ASK_NOT_YET_TEXT mentions @fiona ask as alternative', () => { expect(ASK_NOT_YET_TEXT).toMatch('@fiona ask'); }); - - it('SEARCH_NOT_YET_TEXT mentions @fiona search as alternative', () => { - expect(SEARCH_NOT_YET_TEXT).toMatch('@fiona search'); - }); }); describe('routeCommandViaSay', () => { @@ -268,8 +353,11 @@ describe('routeCommandViaSay', () => { let mockLogger; beforeEach(() => { + jest.clearAllMocks(); mockSay = jest.fn().mockResolvedValue(undefined); mockLogger = { error: jest.fn(), warn: jest.fn(), info: jest.fn() }; + mockSearchForSources.mockResolvedValue([]); + mockFormatSearchResults.mockReturnValue({ text: '🔍 No sources found.', blocks: null }); }); it('sends HELP_TEXT when keyword is "help"', async () => { @@ -282,9 +370,17 @@ describe('routeCommandViaSay', () => { expect(mockSay).toHaveBeenCalledWith(ASK_NOT_YET_TEXT); }); - it('sends SEARCH_NOT_YET_TEXT when keyword is "search"', async () => { - await routeCommandViaSay(mockSay, mockLogger, { keyword: 'search', rawArgs: 'Data Standard' }); - expect(mockSay).toHaveBeenCalledWith(SEARCH_NOT_YET_TEXT); + it('calls searchForSources and say() results when keyword is "search"', async () => { + await routeCommandViaSay(mockSay, mockLogger, { keyword: 'search', rawArgs: 'Data Standard' }, { interactionType: 'app_mention' }); + expect(mockSearchForSources).toHaveBeenCalledWith('Data Standard', { logger: mockLogger }); + expect(mockSay).toHaveBeenCalledWith( + expect.objectContaining({ + text: '🔍 No sources found.', + unfurl_links: false, + unfurl_media: false, + blocks: expect.arrayContaining([expect.objectContaining({ block_id: 'feedback|search|app_mention' })]), + }), + ); }); it('does not throw when say() throws', async () => { diff --git a/apps/fiona-slack/tests/listeners/commands/fiona.test.js b/apps/fiona-slack/tests/listeners/commands/fiona.test.js index 710366ce..635806c9 100644 --- a/apps/fiona-slack/tests/listeners/commands/fiona.test.js +++ b/apps/fiona-slack/tests/listeners/commands/fiona.test.js @@ -12,13 +12,28 @@ jest.unstable_mockModule('../../../src/agent/interaction-store.js', () => ({ recordInteraction: mockRecordInteraction, })); -// Enforce the "no LLM call" requirement by failing if the handler imports llm-caller. +// Guard: fiona.js must not directly import llm-caller (use search-caller instead). // This guard intercepts static imports only; dynamic import() calls would bypass it. -// Acceptable trade-off: slash command handlers should never dynamically import the LLM. jest.unstable_mockModule('../../../src/agent/llm-caller.js', () => { - throw new Error('llm-caller must not be imported by /fiona slash command handlers'); + throw new Error('llm-caller must not be directly imported by /fiona slash command handlers'); }); +// Mock search-caller so tests control search results without hitting the LLM. +const mockSearchForSources = jest.fn().mockResolvedValue([]); +const mockFormatSearchResults = jest + .fn() + .mockImplementation((_query, sources) => ({ + text: sources.length === 0 ? '🔍 No sources found.' : `🔍 Found ${sources.length} source(s).`, + blocks: null, + })); +const MOCK_SEARCH_ERROR_TEXT = ':warning: Search encountered an error. Please try again later.'; + +jest.unstable_mockModule('../../../src/agent/search-caller.js', () => ({ + searchForSources: mockSearchForSources, + formatSearchResults: mockFormatSearchResults, + SEARCH_ERROR_TEXT: MOCK_SEARCH_ERROR_TEXT, +})); + const mockPostEscalation = jest.fn().mockResolvedValue({ ok: true, errorType: null }); jest.unstable_mockModule('../../../src/agent/escalation.js', () => ({ postEscalation: mockPostEscalation, @@ -186,31 +201,152 @@ describe('fionaCommandCallback', () => { }); describe('search sub-command', () => { + let mockRespond; + beforeEach(() => { - mockCommand.text = 'search'; + jest.clearAllMocks(); + mockRespond = jest.fn().mockResolvedValue(undefined); + mockSearchForSources.mockResolvedValue([]); + mockFormatSearchResults.mockImplementation((_q, sources) => ({ + text: sources.length === 0 ? '🔍 No sources found.' : `🔍 Found ${sources.length} source(s).`, + blocks: null, + })); }); - it('calls ack() exactly once', async () => { - await fionaCommandCallback({ command: mockCommand, ack: mockAck, logger: mockLogger }); - expect(mockAck).toHaveBeenCalledTimes(1); - }); + describe('bare search (no query)', () => { + beforeEach(() => { + mockCommand.text = 'search'; + }); - it('ack() response indicates the feature is not yet available', async () => { - await fionaCommandCallback({ command: mockCommand, ack: mockAck, logger: mockLogger }); - expect(mockAck).toHaveBeenCalledWith(expect.stringMatching(/not yet available|coming soon/i)); + it('falls back to help (ack receives help text)', async () => { + await fionaCommandCallback({ command: mockCommand, ack: mockAck, respond: mockRespond, logger: mockLogger }); + expect(mockAck).toHaveBeenCalledWith(expect.stringContaining('Fiona')); + }); + + it('records slash_help when no query is provided', async () => { + await fionaCommandCallback({ command: mockCommand, ack: mockAck, respond: mockRespond, logger: mockLogger }); + await flushMicrotasks(); + expect(mockRecordInteraction).toHaveBeenCalledWith( + expect.objectContaining({ interactionType: 'slash_help' }), + ); + }); + + it('does not call searchForSources when no query is provided', async () => { + await fionaCommandCallback({ command: mockCommand, ack: mockAck, respond: mockRespond, logger: mockLogger }); + expect(mockSearchForSources).not.toHaveBeenCalled(); + }); }); - it('ack() response does not show the full help menu', async () => { - await fionaCommandCallback({ command: mockCommand, ack: mockAck, logger: mockLogger }); - expect(mockAck).not.toHaveBeenCalledWith(expect.stringContaining('Available commands')); + describe('search with query', () => { + beforeEach(() => { + mockCommand.text = 'search Ed-Fi ODS API'; + }); + + it('calls ack() without arguments (deferred response)', async () => { + await fionaCommandCallback({ command: mockCommand, ack: mockAck, respond: mockRespond, logger: mockLogger }); + expect(mockAck).toHaveBeenCalledTimes(1); + expect(mockAck).toHaveBeenCalledWith(); + }); + + it('calls searchForSources with the extracted query', async () => { + await fionaCommandCallback({ command: mockCommand, ack: mockAck, respond: mockRespond, logger: mockLogger }); + expect(mockSearchForSources).toHaveBeenCalledWith('Ed-Fi ODS API', expect.objectContaining({ logger: mockLogger })); + }); + + it('calls respond() with response_type ephemeral', async () => { + await fionaCommandCallback({ command: mockCommand, ack: mockAck, respond: mockRespond, logger: mockLogger }); + expect(mockRespond).toHaveBeenCalledWith( + expect.objectContaining({ + response_type: 'ephemeral', + unfurl_links: false, + unfurl_media: false, + blocks: expect.arrayContaining([expect.objectContaining({ block_id: 'feedback|search|slash_search' })]), + }), + ); + }); + + it('respond() text contains formatted search results', async () => { + mockSearchForSources.mockResolvedValueOnce([{ url: 'https://docs.ed-fi.org/', title: 'Ed-Fi Docs', hostname: 'docs.ed-fi.org' }]); + mockFormatSearchResults.mockReturnValueOnce({ text: '🔍 Found 1 source(s).', blocks: null }); + await fionaCommandCallback({ command: mockCommand, ack: mockAck, respond: mockRespond, logger: mockLogger }); + expect(mockRespond).toHaveBeenCalledWith( + expect.objectContaining({ text: '🔍 Found 1 source(s).' }), + ); + }); + + it('records slash_search telemetry', async () => { + await fionaCommandCallback({ command: mockCommand, ack: mockAck, respond: mockRespond, logger: mockLogger }); + await flushMicrotasks(); + expect(mockRecordInteraction).toHaveBeenCalledWith( + expect.objectContaining({ interactionType: 'slash_search' }), + ); + }); + + it('records slash_search with status success', async () => { + await fionaCommandCallback({ command: mockCommand, ack: mockAck, respond: mockRespond, logger: mockLogger }); + await flushMicrotasks(); + expect(mockRecordInteraction).toHaveBeenCalledWith( + expect.objectContaining({ status: 'success' }), + ); + }); + + it('does not call respond() when ack() rejects', async () => { + mockAck.mockRejectedValueOnce(new Error('slack timeout')); + await fionaCommandCallback({ command: mockCommand, ack: mockAck, respond: mockRespond, logger: mockLogger }); + expect(mockRespond).not.toHaveBeenCalled(); + }); + + it('does not throw when respond() rejects', async () => { + mockRespond.mockRejectedValueOnce(new Error('respond failed')); + await expect( + fionaCommandCallback({ command: mockCommand, ack: mockAck, respond: mockRespond, logger: mockLogger }), + ).resolves.toBeUndefined(); + }); + + it('logs an error when respond() rejects', async () => { + mockRespond.mockRejectedValueOnce(new Error('respond failed')); + await fionaCommandCallback({ command: mockCommand, ack: mockAck, respond: mockRespond, logger: mockLogger }); + expect(mockLogger.error).toHaveBeenCalledWith(expect.stringContaining('Failed to respond')); + }); + + it('responds with error text when required fields are missing', async () => { + const { user_id: _u, ...cmd } = mockCommand; + cmd.text = 'search Ed-Fi ODS API'; + await fionaCommandCallback({ command: cmd, ack: mockAck, respond: mockRespond, logger: mockLogger }); + expect(mockRespond).toHaveBeenCalledWith( + expect.objectContaining({ text: MOCK_SEARCH_ERROR_TEXT }), + ); + }); + + it('does not call searchForSources when required fields are missing', async () => { + const { user_id: _u, ...cmd } = mockCommand; + cmd.text = 'search Ed-Fi ODS API'; + await fionaCommandCallback({ command: cmd, ack: mockAck, respond: mockRespond, logger: mockLogger }); + expect(mockSearchForSources).not.toHaveBeenCalled(); + }); }); - it('records slash_search telemetry', async () => { - await fionaCommandCallback({ command: mockCommand, ack: mockAck, logger: mockLogger }); - await flushMicrotasks(); - expect(mockRecordInteraction).toHaveBeenCalledWith( - expect.objectContaining({ interactionType: 'slash_search' }), - ); + describe('search rate limiting', () => { + it('responds with rate limit message when rate limited', async () => { + const { checkRateLimit } = await import('../../../src/agent/rate-limiter.js'); + for (let i = 0; i < 25; i++) checkRateLimit('U_RL_SEARCH'); + const cmd = { ...mockCommand, text: 'search Ed-Fi', user_id: 'U_RL_SEARCH' }; + await fionaCommandCallback({ command: cmd, ack: mockAck, respond: mockRespond, logger: mockLogger }); + expect(mockRespond).toHaveBeenCalledWith( + expect.objectContaining({ text: expect.stringContaining('request limit') }), + ); + }); + + it('records slash_search with rateLimited true when rate limited', async () => { + const { checkRateLimit } = await import('../../../src/agent/rate-limiter.js'); + for (let i = 0; i < 25; i++) checkRateLimit('U_RL_SEARCH2'); + const cmd = { ...mockCommand, text: 'search Ed-Fi', user_id: 'U_RL_SEARCH2' }; + await fionaCommandCallback({ command: cmd, ack: mockAck, respond: mockRespond, logger: mockLogger }); + await flushMicrotasks(); + expect(mockRecordInteraction).toHaveBeenCalledWith( + expect.objectContaining({ interactionType: 'slash_search', rateLimited: true }), + ); + }); }); }); diff --git a/apps/fiona-slack/tests/listeners/events/app-mention.test.js b/apps/fiona-slack/tests/listeners/events/app-mention.test.js index 25bc4ee8..4ebfac1f 100644 --- a/apps/fiona-slack/tests/listeners/events/app-mention.test.js +++ b/apps/fiona-slack/tests/listeners/events/app-mention.test.js @@ -14,6 +14,7 @@ jest.unstable_mockModule('../../../src/agent/llm-caller.js', () => ({ callLLM: jest.fn().mockResolvedValue({ metadata: null, botText: '', systemPromptVersion: 'v1' }), finalizeMetadataEnvelope: jest.fn(), handleMetadataTimeout: jest.fn(), + searchForSources: jest.fn().mockResolvedValue([]), LLM_MODEL: 'sonar-pro', SYSTEM_PROMPT_VERSION: 'v1', CITATION_POLICY: { @@ -386,13 +387,15 @@ describe('appMentionCallback', () => { expect(callLLM).not.toHaveBeenCalled(); }); - it('responds with coming-soon text when mention text starts with "search "', async () => { + it('responds with search results when mention text starts with "search "', async () => { mockEvent.text = '<@UFIONA> search Data Standard 6.0'; await appMentionCallback({ event: mockEvent, client: mockClient, logger: mockLogger, say: mockSay }); expect(mockSay).toHaveBeenCalledTimes(1); - expect(mockSay.mock.calls[0][0]).toMatch(/not yet available/i); + const sayArg = mockSay.mock.calls[0][0]; + const sayText = typeof sayArg === 'string' ? sayArg : sayArg?.text ?? ''; + expect(sayText).toMatch(/search results|No sources found/i); expect(callLLM).not.toHaveBeenCalled(); }); diff --git a/apps/fiona-slack/tests/listeners/views/feedback-block.test.js b/apps/fiona-slack/tests/listeners/views/feedback-block.test.js index 36f81c2f..eec83fe4 100644 --- a/apps/fiona-slack/tests/listeners/views/feedback-block.test.js +++ b/apps/fiona-slack/tests/listeners/views/feedback-block.test.js @@ -4,13 +4,23 @@ // See the LICENSE and NOTICES files in the project root for more information. import { describe, it, expect } from '@jest/globals'; -import { feedbackBlock } from '../../../src/listeners/views/feedback_block.js'; +import { + buildFeedbackBlockId, + createFeedbackBlock, + FEEDBACK_RESPONSE_TYPES, + feedbackBlock, + parseFeedbackBlockId, +} from '../../../src/listeners/views/feedback_block.js'; describe('feedbackBlock', () => { it('has type "context_actions"', () => { expect(feedbackBlock.type).toBe('context_actions'); }); + it('defaults block_id to synthesis feedback context', () => { + expect(feedbackBlock.block_id).toBe('feedback|synthesis'); + }); + it('has exactly one element', () => { expect(feedbackBlock.elements).toHaveLength(1); }); @@ -48,4 +58,23 @@ describe('feedbackBlock', () => { expect(typeof feedbackBlock.elements[0].negative_button.accessibility_label).toBe('string'); expect(feedbackBlock.elements[0].negative_button.accessibility_label.length).toBeGreaterThan(0); }); + + it('builds block ids with response and interaction types', () => { + expect(buildFeedbackBlockId(FEEDBACK_RESPONSE_TYPES.SEARCH, 'slash_search')).toBe('feedback|search|slash_search'); + }); + + it('creates custom feedback blocks with contextual block ids', () => { + const searchFeedbackBlock = createFeedbackBlock({ + responseType: FEEDBACK_RESPONSE_TYPES.SEARCH, + interactionType: 'app_mention', + }); + expect(searchFeedbackBlock.block_id).toBe('feedback|search|app_mention'); + }); + + it('parses custom feedback block ids', () => { + expect(parseFeedbackBlockId('feedback|ask|assistant_message')).toEqual({ + responseType: FEEDBACK_RESPONSE_TYPES.ASK, + interactionType: 'assistant_message', + }); + }); }); diff --git a/apps/fiona-slack/tests/listeners/views/feedback_reason.test.js b/apps/fiona-slack/tests/listeners/views/feedback_reason.test.js index 998e770a..ac2d66fe 100644 --- a/apps/fiona-slack/tests/listeners/views/feedback_reason.test.js +++ b/apps/fiona-slack/tests/listeners/views/feedback_reason.test.js @@ -27,6 +27,7 @@ describe('feedbackReasonViewCallback', () => { mockClient = { conversations: { replies: jest.fn().mockResolvedValue({ messages: [] }), + history: jest.fn().mockResolvedValue({ messages: [] }), }, chat: { postEphemeral: jest.fn().mockResolvedValue(undefined), @@ -39,6 +40,8 @@ describe('feedbackReasonViewCallback', () => { userId: 'U123', value: 'good-feedback', thread_ts: '1234567890.000000', + responseType: 'synthesis', + interactionType: 'assistant_message', }), state: { values: { @@ -73,6 +76,8 @@ describe('feedbackReasonViewCallback', () => { reason: 'Very helpful answer!', userMessage: 'User question', botResponse: 'Bot response', + responseType: 'synthesis', + interactionType: 'assistant_message', }), ); }); @@ -184,6 +189,37 @@ describe('feedbackReasonViewCallback', () => { expect(mockClient.chat.postEphemeral).toHaveBeenCalledTimes(1); }); + it('records search feedback using responseType metadata and fetched message text', async () => { + mockView.private_metadata = JSON.stringify({ + channelId: 'C456', + messageTs: '1234567890.000001', + userId: 'U123', + value: 'good-feedback', + thread_ts: '1234567890.000001', + responseType: 'search', + interactionType: 'slash_search', + searchQuery: 'What is Ed-Fi ODS?', + }); + mockClient.conversations.history.mockResolvedValueOnce({ + messages: [{ ts: '1234567890.000001', text: 'Search result response' }], + }); + + await feedbackReasonViewCallback({ ack: mockAck, view: mockView, client: mockClient, logger: mockLogger }); + + expect(mockClient.conversations.replies).not.toHaveBeenCalled(); + expect(mockClient.conversations.history).toHaveBeenCalledWith( + expect.objectContaining({ channel: 'C456', latest: '1234567890.000001', inclusive: true, limit: 1 }), + ); + expect(mockRecordFeedback).toHaveBeenCalledWith( + expect.objectContaining({ + userMessage: 'What is Ed-Fi ODS?', + botResponse: 'Search result response', + responseType: 'search', + interactionType: 'slash_search', + }), + ); + }); + it('logs error but does not throw when recordFeedback fails', async () => { mockRecordFeedback.mockRejectedValueOnce(new Error('Cosmos error')); @@ -230,6 +266,8 @@ describe('feedbackReasonClosedCallback', () => { userId: 'U123', value: 'good-feedback', thread_ts: '1234567890.000000', + responseType: 'synthesis', + interactionType: 'assistant_message', }), }; }); @@ -253,6 +291,8 @@ describe('feedbackReasonClosedCallback', () => { reason: null, userMessage: null, botResponse: null, + responseType: 'synthesis', + interactionType: 'assistant_message', }), ); });