diff --git a/CHANGELOG.md b/CHANGELOG.md index 61d04f507..91b0bf82f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,15 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### New conversations are still named once some older ones could not be + +Every pass of the job that names conversations offered at most twenty of those still without a name. +A conversation it had tried and could not name, for example because the model answered with no text +or the conversation opened with only an attachment, keeps no name, so it stayed among those twenty +and took a place on every pass, although offering it again did nothing. Once enough of them had built +up, a new conversation could miss out on every pass and keep showing its plain name in the sidebar. +A pass now skips any conversation that the job already holds work for, so new ones get a place. One +it could not name is still tried again later, as before. ### An offboarding one app refuses still records the apps that answered Removing somebody withdraws each brokered account they connected. When the broker refused one of diff --git a/server/src/channels/summary.ts b/server/src/channels/summary.ts index 3686ab2b3..c1a464240 100644 --- a/server/src/channels/summary.ts +++ b/server/src/channels/summary.ts @@ -7,13 +7,14 @@ * Offer then claim, like `work/culler.ts`. The offer is derived from the table rather than from an * event, so a missed sweep costs two seconds where a missed event would cost the name entirely. */ -import { and, asc, eq, isNull, sql } from "drizzle-orm"; +import { and, asc, eq, isNull, notExists, sql } from "drizzle-orm"; import { readFiring } from "../../../shared/routine-firing"; import type { Database } from "../db/client"; import { channelMemberships, channels, intelligenceChannelMappings, + workItems, } from "../db/schema"; import { DEFAULT_MAX_ATTEMPTS, type WorkQueue } from "../work/queue"; import { CHANNEL_ACTIVITY_TOPIC, type ChannelActivityEvent } from "./events"; @@ -73,6 +74,25 @@ export async function offerChannelsAwaitingSummary( isNull(channels.summary), isNull(channels.deletedAt), sql`${channels.lastMessageAt} is not null`, + /* + * Not one the queue already holds a row for, pending or settled. + * + * Offering it again changes nothing, but it takes one of the `limit` places. A conversation + * this cannot name keeps no summary and so never leaves the set above, and enough of them + * ahead of a new conversation left the new one unoffered on every pass. Once its settled row + * is forgotten it is offered again, as before. + */ + notExists( + options.database + .select({ key: workItems.key }) + .from(workItems) + .where( + and( + eq(workItems.kind, CHANNEL_SUMMARY_KIND), + eq(workItems.key, channels.id), + ), + ), + ), ), ) .limit(options.limit ?? 20); diff --git a/server/tests/channel-summary.integration.test.ts b/server/tests/channel-summary.integration.test.ts index 83724a6e2..ab6ac951e 100644 --- a/server/tests/channel-summary.integration.test.ts +++ b/server/tests/channel-summary.integration.test.ts @@ -195,6 +195,46 @@ describe("offering conversations to be named", () => { .where(eq(workItems.key, channel.id)); expect(rows).toHaveLength(1); }); + + /* + * A pass offers at most `limit` conversations, twenty by default, and a conversation it could not + * name keeps no summary, so it stays in the set the pass reads. Offering it again while its settled + * work row stands does nothing, but it still used one of those places. Twenty such conversations + * ahead of a new one left nothing for it, pass after pass. + */ + test("does not spend a place on a conversation whose settled work still stands", async () => { + const owner = await createUser(); + const channel = await createUsedChannel(owner); + await offer(channel.id); + // Settled without a name, the way a model that answers with nothing leaves it. + await summariseClaimedChannels(options({ title: titler(null) })); + expect((await summaryOf(channel.id))?.summary).toBeNull(); + + const { offered } = await offerChannelsAwaitingSummary({ + database, + queue, + limit: 200, + }); + + expect(offered).not.toContain(channel.id); + }); + + test("offers it again once its settled work has been forgotten", async () => { + const owner = await createUser(); + const channel = await createUsedChannel(owner); + await offer(channel.id); + await summariseClaimedChannels(options({ title: titler(null) })); + // What `forgetSettledSummaries` does once the finished row is an hour old. + await database.delete(workItems).where(eq(workItems.key, channel.id)); + + const { offered } = await offerChannelsAwaitingSummary({ + database, + queue, + limit: 200, + }); + + expect(offered).toContain(channel.id); + }); }); /**