diff --git a/src/index.ts b/src/index.ts index 48118c7..21c1e3e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -16,6 +16,7 @@ import { formatQueueFailureMessage, formatQueuedUrlMessage, } from "./presenters/queue.js"; +import { ProgressPresenter } from "./presenters/progress.js"; import { runAddCommand, runSummaryCommand, @@ -470,18 +471,11 @@ async function processUrlWithProgress( authorId: message.author.id, }); - let lastPhase: string | null = null; - // Once we see a terminal phase ('completed' or 'failed') we should - // suppress any subsequent progress updates emitted by the CLI to avoid - // confusing the user with post-completion informational objects. - let terminalPhaseSeen = false; let eventCount = 0; let finalResult: AddResult | undefined; - // Keep a reference to the last posted Discord message so we can - // append the created item link/ID to it when the CLI returns the ID. - // In real runtime this will be a discord.js Message; in tests the - // mocked send/reply helpers may return undefined which we handle. - let lastPostedMessage: any = null; + + // ProgressPresenter manages status message state and lifecycle + const presenter = new ProgressPresenter(thread, message, logger); // Process progress events using manual iteration to capture return value logger.info("Waiting for CLI progress events...", { messageId: message.id, url }); @@ -511,94 +505,23 @@ async function processUrlWithProgress( eventCount, }); - // If we've already observed a terminal phase, ignore any further - // progress events to avoid confusing follow-up messages (some CLI - // implementations emit informational objects after completion). - if (terminalPhaseSeen) { - logger.debug("Ignoring CLI progress event after terminal phase", { - messageId: message.id, - url, - eventCount, - }); - continue; - } - - // Only send update if phase changed (avoid spam) - if (event.phase !== lastPhase) { - lastPhase = event.phase; - - const progressMsg = formatProgressMessage(event); - - // Always ensure URLs shown to users are wrapped in backticks to avoid embeds. - // If event contains a url or title, prefer showing the title wrapped in ticks. - const safeProgressMsg = ((): string => { - try { - if (event.title) return progressMsg.replace(event.title, `\`${event.title}\``); - if (event.url) return progressMsg.replace(event.url, `\`${event.url}\``); - return progressMsg; - } catch { - return progressMsg; - } - })(); + await presenter.handleProgressEvent(event); + } - if (thread) { - try { - // Capture the returned message when possible so we can edit it - // later to append the created item link/ID. - const posted = await thread.send(safeProgressMsg); - lastPostedMessage = posted ?? lastPostedMessage; - } catch (error) { - logger.warn("Failed to send progress update to thread; falling back to channel reply", { - threadId: thread.id, - phase: event.phase, - error: error instanceof Error ? error.message : String(error), - }); - // Fallback: reply in channel so user still receives updates - try { - const posted = await message.reply(safeProgressMsg); - lastPostedMessage = posted ?? lastPostedMessage; - } catch (err) { - logger.warn("Failed to send fallback progress reply to channel", { - messageId: message.id, - error: err instanceof Error ? err.message : String(err), - }); - } - } - } else { - // No thread available -> send progress updates to channel (safe) - try { - const posted = await message.reply(safeProgressMsg); - lastPostedMessage = posted ?? lastPostedMessage; - } catch (err) { - logger.warn("Failed to send progress update to channel", { - messageId: message.id, - phase: event.phase, - error: err instanceof Error ? err.message : String(err), - }); - } - } + // Retrieve presenter state for final result handling + const lastPhase = presenter.getLastPhase(); + let lastPostedMessage: any = presenter.getLastPostedMessage(); - // If this event indicates a terminal state, mark it so we ignore - // any subsequent non-actionable events. - try { - if (event.phase === "completed" || event.phase === "failed") { - terminalPhaseSeen = true; - } - } catch { - // ignore - } - } - } if (finalResult) { logger.info("CLI processing complete", { messageId: message.id, url, - success: finalResult?.success, - title: finalResult?.title, - error: finalResult?.error, + success: finalResult.success, + title: finalResult.title, + error: finalResult.error, }); - if (finalResult?.success) { + if (finalResult.success) { await removeReaction(message, PROCESSING_REACTION); await addReaction(message, SUCCESS_REACTION); diff --git a/src/presenters/progress.ts b/src/presenters/progress.ts new file mode 100644 index 0000000..2f90f9a --- /dev/null +++ b/src/presenters/progress.ts @@ -0,0 +1,149 @@ +import type { Message, ThreadChannel } from "discord.js"; +import type { AddProgressEvent } from "../bot/cli-runner.js"; +import { formatProgressMessage } from "../formatters/progress.js"; +import type { Logger } from "../logger.js"; + +/** + * Manages Discord status message lifecycle for CLI progress events. + * + * Responsible for: + * - Formatting progress messages using phase emojis and labels + * - Tracking posted status messages via a keyed Map + * - Creating status messages in the appropriate Discord target (thread or channel) + * - Deduplicating progress updates (only posting when the phase changes) + * - Suppressing further updates once a terminal phase is observed + */ +export class ProgressPresenter { + /** + * Tracks the most recently posted Discord message for each phase key. + * Provides infrastructure for future update/delete operations on status messages. + */ + private readonly statusMessages: Map = new Map(); + + private lastPhase: string | null = null; + private terminalPhaseSeen = false; + private lastPostedMessage: any = null; + + constructor( + private readonly thread: ThreadChannel | null, + private readonly message: Message, + private readonly logger: Logger + ) {} + + /** + * Handle a CLI progress event, posting a Discord status update if the phase changed. + * + * @param event - The progress event from the CLI runner. + * @returns `true` if a status message was posted, `false` if the event was skipped + * (e.g. duplicate phase or post-terminal event). + */ + async handleProgressEvent(event: AddProgressEvent): Promise { + if (this.terminalPhaseSeen) { + this.logger.debug("Ignoring CLI progress event after terminal phase", { + messageId: this.message.id, + phase: event.phase, + }); + return false; + } + + if (event.phase === this.lastPhase) { + return false; + } + + this.lastPhase = event.phase ?? null; + + const progressMsg = formatProgressMessage(event); + const safeProgressMsg = this.makeSafeProgressMsg(event, progressMsg); + + await this.postStatusMessage(event.phase, safeProgressMsg); + + if (event.phase === "completed" || event.phase === "failed") { + this.terminalPhaseSeen = true; + } + + return true; + } + + /** + * Wrap URLs and titles in backticks so Discord does not create embeds for them. + */ + private makeSafeProgressMsg(event: AddProgressEvent, progressMsg: string): string { + try { + if (event.title) return progressMsg.replace(event.title, `\`${event.title}\``); + if (event.url) return progressMsg.replace(event.url, `\`${event.url}\``); + return progressMsg; + } catch { + return progressMsg; + } + } + + /** + * Post a status message to the thread (preferred) or channel (fallback). + * Updates `statusMessages` and `lastPostedMessage` on success. + */ + private async postStatusMessage(phase: string | undefined, content: string): Promise { + const key = phase ?? "__unknown__"; + + if (this.thread) { + try { + const posted = await this.thread.send(content); + this.lastPostedMessage = posted ?? this.lastPostedMessage; + if (posted) this.statusMessages.set(key, posted); + } catch (error) { + this.logger.warn( + "Failed to send progress update to thread; falling back to channel reply", + { + threadId: this.thread.id, + phase, + error: error instanceof Error ? error.message : String(error), + } + ); + try { + const posted = await this.message.reply(content); + this.lastPostedMessage = posted ?? this.lastPostedMessage; + if (posted) this.statusMessages.set(key, posted); + } catch (err) { + this.logger.warn("Failed to send fallback progress reply to channel", { + messageId: this.message.id, + error: err instanceof Error ? err.message : String(err), + }); + } + } + } else { + try { + const posted = await this.message.reply(content); + this.lastPostedMessage = posted ?? this.lastPostedMessage; + if (posted) this.statusMessages.set(key, posted); + } catch (err) { + this.logger.warn("Failed to send progress update to channel", { + messageId: this.message.id, + phase, + error: err instanceof Error ? err.message : String(err), + }); + } + } + } + + /** Returns the last Discord message object posted by this presenter. */ + getLastPostedMessage(): any { + return this.lastPostedMessage; + } + + /** Returns the phase string of the most recently handled event, or `null`. */ + getLastPhase(): string | null { + return this.lastPhase; + } + + /** Returns `true` once a terminal phase (`completed` or `failed`) has been observed. */ + isTerminalPhaseSeen(): boolean { + return this.terminalPhaseSeen; + } + + /** + * Returns a read-only view of the status messages Map. + * Keys are phase strings; values are Discord message objects. + */ + getStatusMessages(): ReadonlyMap { + return this.statusMessages; + } +} diff --git a/tests/unit/progress.presenter.spec.ts b/tests/unit/progress.presenter.spec.ts new file mode 100644 index 0000000..a43eacd --- /dev/null +++ b/tests/unit/progress.presenter.spec.ts @@ -0,0 +1,294 @@ +import { describe, expect, it, vi, beforeEach } from "vitest"; +import { ProgressPresenter } from "../../src/presenters/progress.js"; +import type { AddProgressEvent } from "../../src/bot/cli-runner.js"; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function makeLogger() { + return { + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + } as any; +} + +function makeMessage(overrides: { id?: string } = {}) { + const replies: string[] = []; + const message: any = { + id: overrides.id ?? "msg-1", + reply: vi.fn(async (text: string) => { + const m = { content: String(text), edit: vi.fn() }; + replies.push(String(text)); + return m; + }), + channel: {}, + client: { user: { id: "bot-1" } }, + }; + return { message, replies }; +} + +function makeThread(overrides: { id?: string; failSend?: boolean } = {}) { + const messages: string[] = []; + const thread: any = { + id: overrides.id ?? "thread-1", + send: vi.fn(async (text: string) => { + if (overrides.failSend) throw new Error("thread send failed"); + const m = { content: String(text), edit: vi.fn() }; + messages.push(String(text)); + return m; + }), + setArchived: vi.fn(async () => undefined), + }; + return { thread, messages }; +} + +function event(phase: string, extras: Partial = {}): AddProgressEvent { + return { phase, url: "https://example.com", ...extras } as AddProgressEvent; +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe("ProgressPresenter", () => { + describe("handleProgressEvent – phase deduplication", () => { + it("posts a message for the first event", async () => { + const logger = makeLogger(); + const { message } = makeMessage(); + const presenter = new ProgressPresenter(null, message, logger); + + const posted = await presenter.handleProgressEvent(event("downloading")); + + expect(posted).toBe(true); + expect(message.reply).toHaveBeenCalledTimes(1); + expect(message.reply.mock.calls[0][0]).toContain("⏳ Downloading content..."); + }); + + it("does NOT post when the same phase is repeated", async () => { + const logger = makeLogger(); + const { message } = makeMessage(); + const presenter = new ProgressPresenter(null, message, logger); + + await presenter.handleProgressEvent(event("downloading")); + const second = await presenter.handleProgressEvent(event("downloading")); + + expect(second).toBe(false); + expect(message.reply).toHaveBeenCalledTimes(1); + }); + + it("posts again when the phase changes", async () => { + const logger = makeLogger(); + const { message } = makeMessage(); + const presenter = new ProgressPresenter(null, message, logger); + + await presenter.handleProgressEvent(event("downloading")); + const second = await presenter.handleProgressEvent(event("extracting")); + + expect(second).toBe(true); + expect(message.reply).toHaveBeenCalledTimes(2); + }); + }); + + describe("handleProgressEvent – terminal phase suppression", () => { + it("suppresses events after a 'completed' phase", async () => { + const logger = makeLogger(); + const { message } = makeMessage(); + const presenter = new ProgressPresenter(null, message, logger); + + await presenter.handleProgressEvent(event("completed", { title: "My Article" })); + const suppressed = await presenter.handleProgressEvent(event("some-other-phase")); + + expect(suppressed).toBe(false); + expect(message.reply).toHaveBeenCalledTimes(1); + }); + + it("suppresses events after a 'failed' phase", async () => { + const logger = makeLogger(); + const { message } = makeMessage(); + const presenter = new ProgressPresenter(null, message, logger); + + await presenter.handleProgressEvent(event("failed", { message: "oops" })); + const suppressed = await presenter.handleProgressEvent(event("downloading")); + + expect(suppressed).toBe(false); + expect(message.reply).toHaveBeenCalledTimes(1); + }); + + it("marks isTerminalPhaseSeen() true after 'completed'", async () => { + const logger = makeLogger(); + const { message } = makeMessage(); + const presenter = new ProgressPresenter(null, message, logger); + + expect(presenter.isTerminalPhaseSeen()).toBe(false); + await presenter.handleProgressEvent(event("completed", { title: "T" })); + expect(presenter.isTerminalPhaseSeen()).toBe(true); + }); + }); + + describe("handleProgressEvent – thread vs channel posting", () => { + it("posts to thread when a thread is provided", async () => { + const logger = makeLogger(); + const { message } = makeMessage(); + const { thread, messages } = makeThread(); + const presenter = new ProgressPresenter(thread, message, logger); + + await presenter.handleProgressEvent(event("downloading")); + + expect(thread.send).toHaveBeenCalledTimes(1); + expect(message.reply).not.toHaveBeenCalled(); + expect(messages[0]).toContain("⏳ Downloading content..."); + }); + + it("falls back to channel reply when thread.send() fails", async () => { + const logger = makeLogger(); + const { message, replies } = makeMessage(); + const { thread } = makeThread({ failSend: true }); + const presenter = new ProgressPresenter(thread, message, logger); + + await presenter.handleProgressEvent(event("downloading")); + + expect(thread.send).toHaveBeenCalledTimes(1); + expect(message.reply).toHaveBeenCalledTimes(1); + expect(replies[0]).toContain("⏳ Downloading content..."); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining("falling back to channel reply"), + expect.any(Object) + ); + }); + + it("posts to channel reply when no thread is given", async () => { + const logger = makeLogger(); + const { message, replies } = makeMessage(); + const presenter = new ProgressPresenter(null, message, logger); + + await presenter.handleProgressEvent(event("extracting")); + + expect(message.reply).toHaveBeenCalledTimes(1); + expect(replies[0]).toContain("📝 Extracting text content..."); + }); + }); + + describe("statusMessages Map", () => { + it("tracks posted messages by phase key", async () => { + const logger = makeLogger(); + const { message } = makeMessage(); + const { thread } = makeThread(); + const presenter = new ProgressPresenter(thread, message, logger); + + await presenter.handleProgressEvent(event("downloading")); + await presenter.handleProgressEvent(event("extracting")); + + const statusMessages = presenter.getStatusMessages(); + expect(statusMessages.size).toBe(2); + expect(statusMessages.has("downloading")).toBe(true); + expect(statusMessages.has("extracting")).toBe(true); + }); + + it("only has one entry per phase (deduplication does not add multiple)", async () => { + const logger = makeLogger(); + const { message } = makeMessage(); + const presenter = new ProgressPresenter(null, message, logger); + + await presenter.handleProgressEvent(event("downloading")); + await presenter.handleProgressEvent(event("downloading")); // duplicate + + expect(presenter.getStatusMessages().size).toBe(1); + }); + + it("returns a read-only view", () => { + const logger = makeLogger(); + const { message } = makeMessage(); + const presenter = new ProgressPresenter(null, message, logger); + + const map = presenter.getStatusMessages(); + expect(map).toBeInstanceOf(Map); + }); + }); + + describe("getLastPhase()", () => { + it("returns null before any event is handled", () => { + const logger = makeLogger(); + const { message } = makeMessage(); + const presenter = new ProgressPresenter(null, message, logger); + expect(presenter.getLastPhase()).toBeNull(); + }); + + it("returns the latest phase after handling events", async () => { + const logger = makeLogger(); + const { message } = makeMessage(); + const presenter = new ProgressPresenter(null, message, logger); + + await presenter.handleProgressEvent(event("downloading")); + expect(presenter.getLastPhase()).toBe("downloading"); + + await presenter.handleProgressEvent(event("embedding")); + expect(presenter.getLastPhase()).toBe("embedding"); + }); + }); + + describe("getLastPostedMessage()", () => { + it("returns null before any event is handled", () => { + const logger = makeLogger(); + const { message } = makeMessage(); + const presenter = new ProgressPresenter(null, message, logger); + expect(presenter.getLastPostedMessage()).toBeNull(); + }); + + it("returns the Discord message object returned by reply/send", async () => { + const logger = makeLogger(); + const { message } = makeMessage(); + const { thread } = makeThread(); + const presenter = new ProgressPresenter(thread, message, logger); + + await presenter.handleProgressEvent(event("downloading")); + const last = presenter.getLastPostedMessage(); + expect(last).not.toBeNull(); + expect(last).toHaveProperty("content"); + }); + }); + + describe("progress message formatting", () => { + it("formats known phases with correct emojis", async () => { + const logger = makeLogger(); + const cases: [string, string][] = [ + ["downloading", "⏳ Downloading content..."], + ["extracting", "📝 Extracting text content..."], + ["embedding", "🧠 Generating embeddings..."], + ]; + + for (const [phase, expected] of cases) { + const { message, replies } = makeMessage(); + const presenter = new ProgressPresenter(null, message, logger); + await presenter.handleProgressEvent(event(phase)); + expect(replies[0]).toBe(expected); + } + }); + + it("wraps event.title in backticks in the posted message", async () => { + const logger = makeLogger(); + const { message, replies } = makeMessage(); + const presenter = new ProgressPresenter(null, message, logger); + + await presenter.handleProgressEvent( + event("completed", { title: "My Article Title" }) + ); + + expect(replies[0]).toContain("`My Article Title`"); + }); + + it("wraps event.url in backticks when no title is present", async () => { + const logger = makeLogger(); + const { message, replies } = makeMessage(); + const presenter = new ProgressPresenter(null, message, logger); + + await presenter.handleProgressEvent( + event("failed", { message: "network error", url: "https://example.com" }) + ); + + expect(replies[0]).toContain("❌ Failed: network error"); + }); + }); +});