diff --git a/plugins/thread-briefs/README.md b/plugins/thread-briefs/README.md index 14aba71..de913ef 100644 --- a/plugins/thread-briefs/README.md +++ b/plugins/thread-briefs/README.md @@ -35,6 +35,13 @@ unsent-draft pencil and displacing that everywhere would cost more than it says. are skipped), the derived status, a stage control for the manual override, and Re-summarize. It works the same on mobile and desktop; nothing depends on hover. +Briefs are **never backfilled** — activity earns a brief. A thread that has been +dormant since before the plugin started stays briefless, and the popover says so +with **Summarize now** rather than showing a spinner that would never resolve. +Work on it again and it gets a brief like any other thread. The alternative — +summarizing every existing thread — is an unbounded burst the first time a key is +configured. + ## How it is built | Concern | Mechanism | diff --git a/plugins/thread-briefs/app.test.tsx b/plugins/thread-briefs/app.test.tsx index 7eb212d..dfe4c6e 100644 --- a/plugins/thread-briefs/app.test.tsx +++ b/plugins/thread-briefs/app.test.tsx @@ -104,6 +104,23 @@ describe("the header popover", () => { slot.lifecycle.unmount(); }); + it("offers to summarize a thread that has no brief, rather than spinning", async () => { + const slot = await render({ getBrief: () => ({ state: "absent" }) }); + fireEvent.click(await slot.findByRole("button", { name: "Thread brief" })); + + expect(await slot.findByText("No brief for this thread yet.")).toBeTruthy(); + // A dormant thread is never backfilled, so "Summarizing…" would never resolve. + expect(slot.queryByText("Summarizing…")).toBeNull(); + + fireEvent.click(await slot.findByRole("button", { name: "Summarize now" })); + await waitFor(() => + expect( + slot.inspection.rpcCalls.some((call) => call.method === "refresh"), + ).toBe(true), + ); + slot.lifecycle.unmount(); + }); + it("surfaces the unconfigured message instead of an empty brief", async () => { const slot = await render({ getBrief: () => ({ state: "unconfigured", message: "Add an API key." }), diff --git a/plugins/thread-briefs/app.tsx b/plugins/thread-briefs/app.tsx index 6d3d826..353006d 100644 --- a/plugins/thread-briefs/app.tsx +++ b/plugins/thread-briefs/app.tsx @@ -169,6 +169,24 @@ function BriefBody({ if (state.state === "summarizing") { return
Summarizing…
; } + if (state.state === "absent") { + // Threads that were already dormant when the plugin arrived are not + // backfilled, so say so and offer to make one rather than spinning. + return ( +
+
+ No brief for this thread yet. +
+ +
+ ); + } if (state.state === "unconfigured" || state.state === "error") { return
{state.message}
; } diff --git a/plugins/thread-briefs/contract.ts b/plugins/thread-briefs/contract.ts index e4410f3..9d0cd97 100644 --- a/plugins/thread-briefs/contract.ts +++ b/plugins/thread-briefs/contract.ts @@ -98,12 +98,18 @@ export const resolvedBriefSchema = briefFieldsSchema export type ResolvedBrief = z.infer; /** - * What the frontend sees for one thread. `summarizing` covers both "never - * summarized" and "queued for a refresh", which the UI renders the same way. + * What the frontend sees for one thread. + * + * `summarizing` means work is genuinely pending — debounced, queued, or in + * flight. `absent` means there is no brief and none is coming, which is the + * normal state for a thread that was already dormant when the plugin arrived: + * briefs are not backfilled, so the UI offers to make one on demand rather + * than claiming a summary is on its way. */ export const briefStateSchema = z.discriminatedUnion("state", [ z.object({ state: z.literal("ready"), brief: resolvedBriefSchema }).strict(), z.object({ state: z.literal("summarizing") }).strict(), + z.object({ state: z.literal("absent") }).strict(), z.object({ state: z.literal("unconfigured"), message: z.string() }).strict(), z.object({ state: z.literal("error"), message: z.string() }).strict(), ]); diff --git a/plugins/thread-briefs/server.test.ts b/plugins/thread-briefs/server.test.ts index c3a9cf4..8882407 100644 --- a/plugins/thread-briefs/server.test.ts +++ b/plugins/thread-briefs/server.test.ts @@ -183,6 +183,173 @@ describe("summarizing", () => { expect(fetchMock).toHaveBeenCalledTimes(1); }); + it("reports absent, not summarizing, for a thread with no brief and none queued", async () => { + current = host({ fetch: fakeCompletion(SUMMARY) }); + await plugin(current.bb); + + const state = (await current.harness.behavior.callRpc("getBrief", { + threadId: "thr_1", + })) as BriefState; + // Saying "summarizing" here would be a lie the UI could never resolve. + expect(state.state).toBe("absent"); + }); + + it("reports summarizing only while work is actually pending", async () => { + current = host({ fetch: fakeCompletion(SUMMARY) }); + await plugin(current.bb); + + // A queued refresh is genuinely pending. + await current.harness.behavior.callRpc("refresh", { threadId: "thr_2" }); + const pending = (await current.harness.behavior.callRpc("getBrief", { + threadId: "thr_2", + })) as BriefState; + expect(["summarizing", "ready"]).toContain(pending.state); + }); + + it("never backfills a thread whose last activity predates this load", async () => { + const fetchMock = fakeCompletion(SUMMARY); + const stale = makeThreadResponse({ + id: "thr_old", + title: "Ancient", + visibility: "visible", + status: "idle", + // Last touched well before the plugin started: no activity to react to. + updatedAt: Date.now() - 2 * 24 * 60 * 60 * 1000, + }); + globalThis.fetch = fetchMock as unknown as typeof globalThis.fetch; + current = createFakePluginHost({ + pluginId: "thread-briefs", + settings: { apiKey: "test-key", baseUrl: "https://api.test/v1" }, + sdk: { + threads: { + get: async () => stale, + list: async () => [stale], + output: async () => ({ output: "old" }), + conversationOutline: async () => ({ + items: [ + { id: "1", role: "user", preview: "old work", attachmentSummary: null }, + ], + maxSeq: 3, + }), + interactions: { list: async () => [] }, + }, + }, + }) as typeof current; + await plugin(current!.bb); + + await current!.harness.behavior.runSchedule("brief-sweep"); + await new Promise((resolve) => setTimeout(resolve, 50)); + + expect(fetchMock).not.toHaveBeenCalled(); + const state = (await current!.harness.behavior.callRpc("getBrief", { + threadId: "thr_old", + })) as BriefState; + expect(state.state).toBe("absent"); + }); + + it("gives a long-dormant thread a brief once it sees activity", async () => { + // The whole point of not backfilling: dormant costs nothing, and the next + // turn is what earns a brief. + const fetchMock = fakeCompletion(SUMMARY); + const old = makeThreadResponse({ + id: "thr_1", + title: "Dormant for months", + visibility: "visible", + status: "idle", + updatedAt: Date.now() - 90 * 24 * 60 * 60 * 1000, + }); + globalThis.fetch = fetchMock as unknown as typeof globalThis.fetch; + current = createFakePluginHost({ + pluginId: "thread-briefs", + settings: { apiKey: "test-key", baseUrl: "https://api.test/v1", quietSeconds: 1 }, + sdk: { + threads: { + get: async () => old, + list: async () => [old], + output: async () => ({ output: "Picked this back up." }), + conversationOutline: async () => ({ + items: [ + { id: "1", role: "user", preview: "resume this", attachmentSummary: null }, + ], + maxSeq: 9, + }), + interactions: { list: async () => [] }, + }, + }, + }) as typeof current; + await plugin(current!.bb); + + // Nothing from the sweep, because nothing has happened. + await current!.harness.behavior.runSchedule("brief-sweep"); + await new Promise((resolve) => setTimeout(resolve, 50)); + expect(fetchMock).not.toHaveBeenCalled(); + + // Now the thread is worked on again. + await current!.harness.behavior.emitThreadEvent("thread.idle", { + thread: old, + lastAssistantText: "Picked this back up.", + }); + const state = await waitFor(async () => { + const result = (await current!.harness.behavior.callRpc("getBrief", { + threadId: "thr_1", + })) as BriefState; + return result.state === "ready" ? result : null; + }); + if (state.state !== "ready") throw new Error("unreachable"); + expect(state.brief.goal).toBe(SUMMARY.goal); + }); + + it("catches a briefless thread whose activity postdates this load", async () => { + // The sweep's one job for a briefless thread: activity that happened while + // we were running, whose `thread.idle` we apparently missed. + const fetchMock = fakeCompletion(SUMMARY); + let sweepThread = makeThreadResponse({ id: "thr_live", status: "idle" }); + globalThis.fetch = fetchMock as unknown as typeof globalThis.fetch; + current = createFakePluginHost({ + pluginId: "thread-briefs", + // A 1s quiet period keeps the test fast. + settings: { apiKey: "test-key", baseUrl: "https://api.test/v1", quietSeconds: 1 }, + sdk: { + threads: { + get: async () => sweepThread, + list: async () => [sweepThread], + output: async () => ({ output: "progress" }), + conversationOutline: async () => ({ + items: [ + { id: "1", role: "user", preview: "do it", attachmentSummary: null }, + ], + maxSeq: 4, + }), + interactions: { list: async () => [] }, + }, + }, + }) as typeof current; + await plugin(current!.bb); + + // Activity just after load, then let the quiet period elapse. + sweepThread = makeThreadResponse({ + id: "thr_live", + status: "idle", + updatedAt: Date.now() + 5, + }); + await new Promise((resolve) => setTimeout(resolve, 1200)); + + await current!.harness.behavior.runSchedule("brief-sweep"); + await waitFor(async () => (fetchMock.mock.calls.length > 0 ? true : null)); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it("leaves a thread still inside its quiet period alone", async () => { + // `thread` defaults to updatedAt = now, so the sweep must not touch it. + const fetchMock = fakeCompletion(SUMMARY); + current = host({ fetch: fetchMock }); + await plugin(current.bb); + + await current.harness.behavior.runSchedule("brief-sweep"); + await new Promise((resolve) => setTimeout(resolve, 50)); + expect(fetchMock).not.toHaveBeenCalled(); + }); + it("emits a row signal per stored brief", async () => { current = host({ fetch: fakeCompletion(SUMMARY) }); await plugin(current.bb); diff --git a/plugins/thread-briefs/server.ts b/plugins/thread-briefs/server.ts index 4fceefb..2150ad7 100644 --- a/plugins/thread-briefs/server.ts +++ b/plugins/thread-briefs/server.ts @@ -96,14 +96,26 @@ export default async function plugin(bb: BbPluginApi) { // ------------------------------------------------------------ queue/timers const lifetime = new AbortController(); + /** + * When this plugin generation started. The cutoff for "has there been + * activity?": threads that last moved before we were running are not + * backfilled. + */ + const loadedAt = Date.now(); /** Per-thread debounce timers: the thread must stay quiet to be summarized. */ const debounces = new Map>(); /** Threads waiting for the single worker, in arrival order. */ const queue: string[] = []; /** Threads to summarize even when the activity cursor has not moved. */ const forced = new Set(); + /** The thread the single worker is summarizing right now, if any. */ + let inFlight: string | null = null; let draining = false; + /** Whether a summary for this thread is genuinely pending or running. */ + const isPending = (threadId: string) => + debounces.has(threadId) || queue.includes(threadId) || inFlight === threadId; + const enqueue = (threadId: string) => { if (!queue.includes(threadId)) queue.push(threadId); void drain(); @@ -142,6 +154,7 @@ export default async function plugin(bb: BbPluginApi) { const threadId = queue.shift(); if (threadId === undefined) break; const force = forced.delete(threadId); + inFlight = threadId; try { const changed = await summarizeThread(threadId, force); if (changed) announce(); @@ -151,6 +164,11 @@ export default async function plugin(bb: BbPluginApi) { error instanceof Error ? error.message : String(error) }`, ); + } finally { + inFlight = null; + // The thread drops back to `absent` on failure, so the UI stops + // saying "summarizing" and offers an explicit retry instead. + announce(); } } } finally { @@ -274,7 +292,10 @@ export default async function plugin(bb: BbPluginApi) { message: "Add an API key in this plugin's settings to generate briefs.", }; } - return { state: "summarizing" }; + // Only claim a summary is coming when one actually is. A thread that was + // already dormant when the plugin arrived is never backfilled, so it sits + // at `absent` until someone asks for a brief. + return isPending(threadId) ? { state: "summarizing" } : { state: "absent" }; } return { state: "ready", @@ -375,12 +396,24 @@ export default async function plugin(bb: BbPluginApi) { if (debounces.has(thread.id) || queue.includes(thread.id)) continue; const stored = await readBrief(thread.id); - // A thread with no brief yet, or one whose timestamp predates its last - // activity, is a candidate. `summarizeThread` re-checks the real cursor - // before spending a request. - if (stored === null || stored.lastSummarizedAt < thread.updatedAt) { - enqueue(thread.id); + if (stored === null) { + // Briefs are never backfilled. A thread gets its first brief from + // activity — `thread.idle` while we are running — so the sweep only + // considers a briefless thread whose activity postdates this load, + // which is activity whose event we should have seen and may have + // missed. Anything older stays briefless until it is next worked on. + // + // Without this bound every briefless thread would be re-enqueued on + // every sweep forever: an unbounded burst across the whole thread list + // the first time a key is configured, and an endless ten-minute retry + // for any thread whose summary keeps failing. + if (thread.updatedAt > loadedAt) enqueue(thread.id); + continue; } + // A stored brief older than the thread's last activity means activity we + // missed. `summarizeThread` re-checks the real cursor before spending a + // request. + if (stored.lastSummarizedAt < thread.updatedAt) enqueue(thread.id); } }); diff --git a/plugins/thread-briefs/skills/thread-briefs/SKILL.md b/plugins/thread-briefs/skills/thread-briefs/SKILL.md index 4aaca8c..b94d8e5 100644 --- a/plugins/thread-briefs/skills/thread-briefs/SKILL.md +++ b/plugins/thread-briefs/skills/thread-briefs/SKILL.md @@ -41,6 +41,24 @@ Re-summarize is available in the header popover; it bypasses the debounce. Hidden threads (plugin workers) and deleted threads never get briefs. +**Briefs are never backfilled — activity earns a brief.** A thread gets its +first brief from a turn happening while the plugin is running. The sweep will +only give a *briefless* thread a first brief if its last activity postdates the +current plugin load, which is activity whose `thread.idle` should have arrived +and may have been missed. A thread that has been dormant since before the plugin +started stays briefless, however old or recent, and the header popover says so +with a **Summarize now** button. Work on it again and it gets a brief like any +other thread. + +This is deliberate: without the bound, every briefless thread would be +re-enqueued on every sweep forever — an unbounded burst of requests across the +whole thread list the first time a key is configured, and an endless retry for +any thread whose summary keeps failing. + +So a thread reports `summarizing` only while work is genuinely debounced, +queued, or in flight; otherwise it reports `absent`, which the UI renders as an +offer rather than a spinner. A failed summary drops back to `absent`. + ## Stage and status `stage` is a semantic judgement from the transcript: discovery, planning, @@ -85,3 +103,5 @@ trip to stay current. `experimental_setThreadRowStatus`, which the content script feature-detects. - Briefs stuck on "Summarizing…": check `apiKey` is set and `bb plugin logs thread-briefs` for HTTP errors from `baseUrl`. +- "No brief for this thread yet" on an older thread is expected, not a fault — + briefs are never backfilled. Work the thread, or use **Summarize now**.