From bf2c6adf2efda04752ab9c42886cc9bcfd90fead Mon Sep 17 00:00:00 2001 From: Dylan Trotter Date: Mon, 5 Oct 2026 13:56:56 +0000 Subject: [PATCH] Ask the summarizer for status instead of deriving it Done was a side effect of the model leaving nextStep and blockedOn blank, while the same prompt asked for "the single most concrete next action". On the bb-dylan server 11 of 21 Done threads were Done only by a hand pin. Read against their transcripts, they failed because an offer or optional check was recorded as the step, because nextStep/blockedOn were fed back from the previous brief and outlived the turns that retired them, or because the step left was the user's own. - The prompt asks for status directly, as the stored enum, with one definition and one example each, framed as: assume the user does what the thread asks; is the task then finished? nextStep becomes descriptive. - The four done-related rules and the stage/nextStep agreement rule are gone (prompt 1100 -> ~850 words), and so is reconcileStage. The parser keeps one hard guard: a non-empty blockedOn is never done. An unreadable status falls back to waiting-on-me. - The previous brief fed back carries only title, goal, currentState and constraints. - modelStatus is stored on the row; briefs written before it keep the old derivation (legacyStatus) until their thread is next summarized. - eval/ exports real threads from a server and scores any checkout's prompt against the live model. Fixtures stay out of the repo. Co-Authored-By: Claude Opus 5.5 --- plugins/thread-briefs/README.md | 98 ++++++--- plugins/thread-briefs/board.ts | 4 +- plugins/thread-briefs/brief.test.ts | 63 ++++-- plugins/thread-briefs/brief.ts | 68 ++---- plugins/thread-briefs/contract.ts | 21 +- plugins/thread-briefs/eval/export.ts | 121 +++++++++++ plugins/thread-briefs/eval/fixture.ts | 17 ++ plugins/thread-briefs/eval/run.ts | 201 ++++++++++++++++++ plugins/thread-briefs/server.test.ts | 21 ++ plugins/thread-briefs/server.ts | 11 +- .../skills/thread-briefs/SKILL.md | 178 +++++++--------- plugins/thread-briefs/summarize.test.ts | 87 ++++---- plugins/thread-briefs/summarize.ts | 85 ++++---- plugins/thread-briefs/transcript.ts | 7 +- plugins/thread-briefs/tsconfig.json | 3 +- 15 files changed, 686 insertions(+), 299 deletions(-) create mode 100644 plugins/thread-briefs/eval/export.ts create mode 100644 plugins/thread-briefs/eval/fixture.ts create mode 100644 plugins/thread-briefs/eval/run.ts diff --git a/plugins/thread-briefs/README.md b/plugins/thread-briefs/README.md index 7d8cfbd..53bf97f 100644 --- a/plugins/thread-briefs/README.md +++ b/plugins/thread-briefs/README.md @@ -6,35 +6,75 @@ every thread a short, durable **brief**, generated outside the working chat: - **goal** — what the thread is actually trying to achieve - **currentState** — what exists now, including half-done work -- **nextStep** — the single most concrete next action, or empty when nobody owes - the thread one. An open PR, a patch carried on a fork, or a workaround still in - place is owed; open-ended watching is not -- **nextStepActor** — who has to take it: `me`, `agent`, or `other`. The one - judgement the status needs that the prose cannot supply, since "test it and - tell me" and "keep going" read alike -- **blockedOn** — the party or artifact it is waiting on, when someone could go - chase it +- **nextStep** — the most useful next action, if there is one. Descriptive only: + a finished thread may still carry a suggestion here +- **nextStepActor** — who would take it: `me`, `agent`, or `other` +- **blockedOn** — the party or artifact outside the thread it is waiting on, when + someone could go chase it - **constraints** — facts learned in the thread that would break a naive re-plan - **title** — a 4–6 word name for the work, which can optionally replace bb's own thread title -Plus a derived **stage** (discovery / planning / implementation / review) and -**status** (working / waiting-on-me / waiting-on-other / done) — where `working` -comes from bb's live thread state and the other three from the brief. - -The two are orthogonal — stage says how far the work has got, status says who -owes the next move — but they are answered by one model call over one transcript, -and nothing in that call holds them to agreeing. So the parser reconciles them: -an empty `nextStep` means nobody owes the thread an action, which is only true -once the work is made, so a `stage` of `implementation` beside one is read as -`review`. Without it an agent's closing summary of what it built lands as -"Implementation — Done". A pinned stage is exempt; a pin is returned as given. +Plus a **stage** (discovery / planning / implementation / review) and a +**status** (working / waiting-on-me / waiting-on-other / done). `working` comes +from bb's live thread state; the other three statuses and the stage are asked of +the model directly. + +The status question is one sentence: *assume you do whatever the thread asks of +you — is the task then finished, or does the thread have more to do?* Yes is +`done`, even when a step is left that only you can take ("approve PR #12", "run +the rollout check"), because nothing more will happen in the thread either way. +No, because your answer or go-ahead starts more work here, is `waiting-on-me`. +No, because something outside the thread has to act first, is +`waiting-on-other`, and names that thing in `blockedOn`. + +### Why status is asked for, not derived + +Status used to be derived: an empty `nextStep` and an empty `blockedOn` meant +done. That made Done a side effect of the model leaving two strings blank, while +the same prompt asked for "the single most concrete next action" — and a model +asked for one finds one. Four rules accumulated in the prompt pulling the +boundary one way and the other, and on the bb-dylan server 11 of 21 Done threads +were Done only because someone had pinned them by hand. Reading each of those 11 +against its transcript gave three causes: + +- an offer or optional check recorded as the step ("want me to file an issue?", + "if you want the hash confirmed, run…"); +- a `nextStep` or `blockedOn` carried over from the previous brief, which was fed + back as the starting point and outlived the turns that retired it; +- a step that was the user's, done outside the thread. + +So the model now answers the status itself, with one definition and one example +per value; `nextStep` no longer decides anything; and the previous brief fed back +into the next summary carries only the fields that should hold still — title, +goal, currentState, constraints. `nextStep` and `blockedOn` are re-read from the +transcript every time. + +The parser applies one rule on top, which holds whatever the model thinks: a +brief that names a `blockedOn` is never `done`. An unreadable status falls back +to `waiting-on-me`, the reading whose mistake is cheap. There are no other +field-against-field corrections — stage and status are stored as answered. + +A brief written before this change has no stored status and keeps the old +derivation until its thread is next summarized. They are deliberately not +re-summarized in bulk: every old idle thread that now read done would be past +the archive threshold already, and the next sweep would take them all at once. + +### Measuring a prompt change + +`eval/` holds the harness that produced the numbers behind this. `export.ts` +freezes threads from a running server into fixtures (outline, last message, the +stored brief); `run.ts` runs a checkout's prompt over them against the live model +and reports agreement with hand-given labels, false Dones and missed Dones, with +and without the previous brief fed back. Point `--src` at a `git worktree` of +`main` to score the old prompt against the same set. Fixtures are real +transcripts and bb-plugins is public, so they live outside the repository. Either can be pinned by hand in the Brief panel, anchored to the thread's activity cursor so the pin retires on the next real turn. The status pin is what closes a thread whose next step was carried out somewhere the transcript cannot -see — "reload a client and confirm the panel opens" leaves nothing for a summary -to read, so the derivation would say `waiting-on-me` forever. +see — a go-ahead you gave in another thread, a PR you merged on github.com — +leaves nothing for a summary to read. ## Install @@ -159,7 +199,7 @@ takes it over; turning grouping off deletes the three sections and restores the sidebar preferences it changed, but cannot put a hand-made placement back. **The side panel** — a **Brief** tab holding the full five fields (empty ones -are skipped), the derived status, when it was last summarized, status and stage +are skipped), the status, when it was last summarized, status and stage controls for the manual overrides, and Re-summarize. The **Brief** button in the thread header opens it; so does the panel's own new-tab launcher, under Actions. It works the same on mobile and desktop — on a compact viewport the host reveals @@ -325,8 +365,8 @@ obvious: a statement that the model was right, and pinning it there would leave a pin that does nothing until it silently expires. - Dragging **out of** Done pins `waiting-on-me` rather than clearing the status - pin, because a done reading can come from the derivation as well as from a pin — - and clearing in that case would hand the card straight back to a derivation + pin, because a done reading can come from the model as well as from a pin — + and clearing in that case would hand the card straight back to a model reading that still says done, snapping it into the column you just dragged it out of. - **No stage** is not a drop target in either direction. @@ -385,6 +425,14 @@ The [grey ring](#where-briefs-show-up) is the warning: both go through the same sweep will take once the second threshold passes. A day of grey is the notice period. +Done is not "nothing left anywhere": a done thread may still carry a step that is +yours alone, such as approving a PR, and it is archived on the same clock. That +is deliberate. The step stays on the card in the Done section for two days and +the hover label says "archiving soon" for the second; a thread archived anyway is +one click from back, and un-archiving it is final (below). Waiting on you would +have kept it in the sidebar indefinitely, which is the failure this status was +rewritten to fix. + Every other rule is a reason *not* to archive, which is the right default for a sweep that runs unattended — a thread wrongly left in the sidebar costs a glance, a thread wrongly archived costs a search for something you believe you left on @@ -413,7 +461,7 @@ so a sweep running up to an hour late is invisible. | Concern | Mechanism | | --- | --- | | Trigger | `bb.events.on("thread.idle")` + a per-thread quiet-period debounce, with a `*/10 * * * *` sweep as the backstop. A thread with no brief yet skips the quiet period, and is summarized from `thread.active` as well — mid-turn, so a long first turn is not spent briefless | -| Summarizer input | `threads.conversationOutline()` head + tail with the middle elided, `threads.output()` for the last message in full, and the previous brief | +| Summarizer input | `threads.conversationOutline()` head + tail with the middle elided, `threads.output()` for the last message in full, and the previous brief's title, goal, currentState and constraints | | Storage | `bb.storage.kv`, one row per thread at `brief:` | | Sidebar glyph | a content script's `experimental_setThreadRowStatus`, fed by an `experimental_appOverlay` that owns the rpc + realtime subscription | | Ring artwork | `app.experimental_icons.register`, one inline SVG per stage plus the done ring, in every palette colour, plus one grey done ring — since a row status takes an icon *name* and not a component, every combination has to be registered at init, before any project is known | diff --git a/plugins/thread-briefs/board.ts b/plugins/thread-briefs/board.ts index 1ef5c2e..47fd45d 100644 --- a/plugins/thread-briefs/board.ts +++ b/plugins/thread-briefs/board.ts @@ -646,8 +646,8 @@ export function countByStatus( /** * The one thing the status badge cannot say. * - * `deriveStatus` collapses a next step the *agent* could take by itself into - * `waiting-on-me`, because the nudge is ours to give — so two cards reading + * `waiting-on-me` covers a thread the *agent* could carry on by itself as well + * as one that needs your answer, because the nudge is ours to give — so two cards reading * "Waiting on you" can want completely different amounts of work from you. On a * board, where the whole task is choosing between them, that difference is worth * a word. diff --git a/plugins/thread-briefs/brief.test.ts b/plugins/thread-briefs/brief.test.ts index b588d72..9c587cd 100644 --- a/plugins/thread-briefs/brief.test.ts +++ b/plugins/thread-briefs/brief.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; import { - deriveStatus, + legacyStatus, effectiveStage, effectiveStatus, idleFor, @@ -40,30 +40,59 @@ const stored = (overrides: Partial = {}): StoredBrief => ({ ...overrides, }); -describe("deriveStatus", () => { +describe("effectiveStatus", () => { + it("reports the status the summarizer judged", () => { + // Done with a suggestion still standing: nextStep no longer decides. + expect(effectiveStatus(stored({ modelStatus: "done" }))).toBe("done"); + expect( + effectiveStatus( + stored({ modelStatus: "waiting-on-me", fields: { ...stored().fields, nextStep: "" } }), + ), + ).toBe("waiting-on-me"); + }); + + it("reads a brief written before status was asked for the old way", () => { + // Not backfilled: such a brief keeps its reading until the thread is + // next summarized. + expect(effectiveStatus(stored())).toBe("waiting-on-me"); + expect( + effectiveStatus(stored({ fields: { ...stored().fields, nextStep: "" } })), + ).toBe("done"); + }); + + it("lets a pin in force win over the model", () => { + expect( + effectiveStatus( + stored({ modelStatus: "waiting-on-me", statusOverride: "done", statusOverrideSeq: 50 }), + ), + ).toBe("done"); + }); +}); + +describe("legacyStatus", () => { it("reports done only when nothing is outstanding and nothing blocking", () => { - expect(deriveStatus({ nextStep: " ", blockedOn: "" })).toBe("done"); + expect(legacyStatus({ nextStep: " ", blockedOn: "" })).toBe("done"); }); it("does not call a blocked thread done, whatever the next step says", () => { // The prompt promises a non-empty nextStep whenever anything is // outstanding. This is the guard for when it does not deliver one. - expect(deriveStatus({ nextStep: "", blockedOn: "Review from Dylan" })).toBe( + expect(legacyStatus({ nextStep: "", blockedOn: "Review from Dylan" })).toBe( "waiting-on-other", ); - expect(deriveStatus({ nextStep: " ", blockedOn: " Upstream fix " })).toBe( + expect(legacyStatus({ nextStep: " ", blockedOn: " Upstream fix " })).toBe( "waiting-on-other", ); }); it("reports waiting-on-other when something is blocking", () => { expect( - deriveStatus({ nextStep: "Merge it", blockedOn: "Review from Dylan" }), + legacyStatus({ nextStep: "Merge it", blockedOn: "Review from Dylan" }), ).toBe("waiting-on-other"); }); it("falls back to waiting-on-me for unfinished, unblocked work", () => { - expect(deriveStatus({ nextStep: "Pick an approach", blockedOn: "" })).toBe( + expect(legacyStatus({ nextStep: "Pick an approach", blockedOn: "" })).toBe( "waiting-on-me", ); }); @@ -71,16 +100,16 @@ describe("deriveStatus", () => { it("does not need a trailing question to report waiting-on-me", () => { // An idle thread with work left needs a human look either way, so the // question signal no longer changes the outcome. - expect(deriveStatus({ nextStep: "Keep going", blockedOn: "" })).toBe( + expect(legacyStatus({ nextStep: "Keep going", blockedOn: "" })).toBe( "waiting-on-me", ); }); }); -describe("deriveStatus with an actor", () => { +describe("legacyStatus with an actor", () => { it("treats an external actor as blocked even with no blockedOn text", () => { expect( - deriveStatus({ + legacyStatus({ nextStep: "Land the upstream PR", blockedOn: "", nextStepActor: "other", @@ -90,7 +119,7 @@ describe("deriveStatus with an actor", () => { it("reports waiting-on-me for a step only the user can take", () => { expect( - deriveStatus({ + legacyStatus({ nextStep: "Try it and say whether the glyph looks right", blockedOn: "", nextStepActor: "me", @@ -100,7 +129,7 @@ describe("deriveStatus with an actor", () => { it("reports waiting-on-me for a step the agent could take, since the nudge is ours", () => { expect( - deriveStatus({ + legacyStatus({ nextStep: "Keep porting the remaining call sites", blockedOn: "", nextStepActor: "agent", @@ -110,21 +139,21 @@ describe("deriveStatus with an actor", () => { it("lets done win over any actor, so a finished thread is never a prompt", () => { expect( - deriveStatus({ nextStep: "", blockedOn: "", nextStepActor: "other" }), + legacyStatus({ nextStep: "", blockedOn: "", nextStepActor: "other" }), ).toBe("done"); }); it("preserves the actor-free behaviour when the actor is absent", () => { // Every brief written before this field existed lands here. - expect(deriveStatus({ nextStep: "Keep going", blockedOn: "" })).toBe( - deriveStatus({ + expect(legacyStatus({ nextStep: "Keep going", blockedOn: "" })).toBe( + legacyStatus({ nextStep: "Keep going", blockedOn: "", nextStepActor: undefined, }), ); expect( - deriveStatus({ + legacyStatus({ nextStep: "Keep going", blockedOn: "", nextStepActor: undefined, @@ -213,7 +242,7 @@ describe("status overrides", () => { statusOverrideSeq: 50, lastActivitySeen: 50, }); - expect(deriveStatus(brief.fields)).toBe("waiting-on-me"); + expect(legacyStatus(brief.fields)).toBe("waiting-on-me"); expect(effectiveStatus(brief)).toBe("done"); expect(isStatusOverrideStale(brief)).toBe(false); }); diff --git a/plugins/thread-briefs/brief.ts b/plugins/thread-briefs/brief.ts index efb28e4..49ace63 100644 --- a/plugins/thread-briefs/brief.ts +++ b/plugins/thread-briefs/brief.ts @@ -57,73 +57,41 @@ export function isStatusOverrideStale(stored: StoredBrief): boolean { /** * The status this brief reports: the manual one while it holds, otherwise the - * derivation over the brief's own fields. + * one the summarizer judged. * - * The override sits *in front of* {@link deriveStatus} rather than editing the - * fields it reads, because `renderTranscript` feeds the previous brief into the - * next summary as a starting point: a `nextStep` blanked in storage would - * simply be written back, where a pin is a separate fact the summarizer never - * sees and cannot undo. - * - * It exists for the one thing the derivation cannot see. A `nextStep` addressed - * to you and carried out *outside the thread* — reload a client, check a - * rollout, confirm a glyph — leaves no trace in the transcript, so no summary - * can retire it and re-summarizing reads the same unresolved instruction back. - * That thread is `waiting-on-me` forever unless you can say otherwise. + * The manual pin exists for the one thing no summary can see: a step carried + * out somewhere the transcript does not reach. The summarizer is asked whether + * the task is finished *assuming* you do what the thread asks of you, so a step + * that is only yours already reads done; the pin is for the rest — a thread + * waiting on your go-ahead that you settled elsewhere, or a reading you simply + * disagree with. */ export function effectiveStatus(stored: StoredBrief): StoredBriefStatus { const override = stored.statusOverride ?? null; if (override !== null && !isStatusOverrideStale(stored)) return override; - return deriveStatus({ - nextStep: stored.fields.nextStep, - blockedOn: stored.fields.blockedOn, - nextStepActor: stored.fields.nextStepActor, - }); + return stored.modelStatus ?? legacyStatus(stored.fields); } /** - * Status as far as a *stored* brief can tell. Mechanical rather than a model - * judgement, so it stays right between summaries: - * - * - nothing to do and nothing blocking → the work is done - * - blocked on something → waiting on someone else - * - otherwise → waiting on me - * - * `done` requires *both* fields empty. The summarizer's prompt already promises - * a non-empty `nextStep` whenever anything is outstanding, but testing - * `blockedOn` here makes "a blocked thread is not done" a guarantee of this - * code rather than of the prompt, so one wayward summary cannot put a green - * tick on a thread that is waiting for a review. - * - * `nextStepActor` is the one input the model has to judge: a next step only we - * can take ("test it", "decide X", "reply to Y") reads the same in prose as one - * the agent could take unprompted. An actor of `other` therefore means waiting - * on someone else even when the summarizer named no `blockedOn`. - * - * `waiting-on-me` is the fallback because an idle thread with unfinished work - * needs a human look by default — whether or not the agent's last turn happened - * to end in a question, and whether or not we know the actor. + * The status of a brief written before the summarizer was asked for one, read + * from its fields the way it used to be: nothing to do and nothing blocking is + * done, a blocker or an outside actor is waiting on someone else, and anything + * else is waiting on you. * - * `working` is deliberately absent: it is live thread state, not a property of - * a brief, so it is applied per row by {@link rowDecoration}. The return type - * says so — narrowing to the stored statuses is what lets a caller that needs - * one, like the refresher's `writtenForStatus`, take this value without a cast. + * Only for old rows. Such a brief stays on this reading until its thread is + * next summarized, which writes a `modelStatus` that replaces it. */ -export function deriveStatus(args: { +export function legacyStatus(fields: { nextStep: string; blockedOn: string; nextStepActor?: NextStepActor | undefined; }): StoredBriefStatus { - const nextStep = args.nextStep.trim(); - const blockedOn = args.blockedOn.trim(); + const nextStep = fields.nextStep.trim(); + const blockedOn = fields.blockedOn.trim(); if (nextStep === "" && blockedOn === "") return "done"; - if (blockedOn !== "" || args.nextStepActor === "other") { + if (blockedOn !== "" || fields.nextStepActor === "other") { return "waiting-on-other"; } - // `agent` — an idle thread the agent could carry on by itself — has no status - // of its own yet, and collapses into waiting-on-me because the nudge is ours - // to give. If that turns out to be a common bucket in practice it earns its - // own status then, rather than being guessed at now. return "waiting-on-me"; } diff --git a/plugins/thread-briefs/contract.ts b/plugins/thread-briefs/contract.ts index 5786675..9434467 100644 --- a/plugins/thread-briefs/contract.ts +++ b/plugins/thread-briefs/contract.ts @@ -135,10 +135,11 @@ export const storedRefresherSchema = refresherProseSchema .strict(); export type StoredRefresher = z.infer; -/** What the summarizer returns: the five fields, the stage, the refresher. */ +/** What the summarizer returns: the five fields, the stage, the status, the refresher. */ export const summaryResultSchema = briefFieldsSchema .extend({ stage: briefStageSchema, + status: storedBriefStatusSchema, /** * Null when the model returned nothing usable for either variant. A brief * with no refresher is a brief that simply never shows one — the same @@ -153,10 +154,10 @@ export type SummaryResult = z.infer; * The persisted row, one per thread, under kv key `brief:`. * * `stage` and `status` are deliberately absent: `stage` is - * `stageOverride ?? modelStage` and `status` is `statusOverride` falling back to - * a derivation over `nextStep`, `blockedOn` and `nextStepActor`, all resolved on - * read so none goes stale between summaries. The live `working` override is applied later still, per row - * on the client. + * `stageOverride ?? modelStage` and `status` is `statusOverride ?? modelStatus`, + * both resolved on read so a pin that has expired stops applying without a + * write. The live `working` override is applied later still, per row on the + * client. * * New fields must be optional and `version` must stay at 1. `readBrief` deletes * any row that fails this parse, and briefs are never backfilled, so a required @@ -170,6 +171,16 @@ export const storedBriefSchema = z fields: briefFieldsSchema, /** The stage the summarizer judged from the transcript. */ modelStage: briefStageSchema, + /** + * The status the summarizer judged from the transcript. + * + * Absent on a brief written before the summarizer was asked for a status. + * Those keep the old reading, derived from `nextStep` and `blockedOn` (see + * `legacyStatus`), until their thread is next summarized; they are not + * backfilled, because re-summarizing every idle thread at once would put + * every one that now reads done straight onto the archive clock. + */ + modelStatus: storedBriefStatusSchema.optional(), /** A manual stage that wins over `modelStage` until real new activity. */ stageOverride: briefStageSchema.nullable(), /** diff --git a/plugins/thread-briefs/eval/export.ts b/plugins/thread-briefs/eval/export.ts new file mode 100644 index 0000000..499a6c9 --- /dev/null +++ b/plugins/thread-briefs/eval/export.ts @@ -0,0 +1,121 @@ +/** + * Freeze real threads from a running bb server into eval fixtures. + * + * npx vite-node eval/export.ts -- [threadId ...] + * + * With no thread ids, exports every thread that has a stored brief. Each + * fixture is what `summarizeThread` would hand `renderTranscript` if the thread + * were re-summarized now: the conversation outline, the agent's last message, + * and the stored brief as the previous brief. The outline is rebuilt from the + * event log rather than read from `threads.conversationOutline`, which only the + * plugin can call; `renderTranscript` clamps each preview to 400 characters, so + * the two differ only in which events bb counts as a message. + * + * Fixtures are real transcripts. Write them somewhere outside this repository: + * bb-plugins is public. + */ +import { execFileSync } from "node:child_process"; +import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import type { EvalFixture } from "./fixture.js"; + +function bb(args: string[]): string { + return execFileSync("bb", args, { encoding: "utf8", maxBuffer: 1 << 28 }); +} + +function rpc(method: string, input: unknown): unknown { + const dir = mkdtempSync(join(tmpdir(), "tb-eval-")); + const file = join(dir, "input.json"); + writeFileSync(file, JSON.stringify(input)); + return JSON.parse( + bb(["plugin", "rpc", "call", "thread-briefs", method, "--input-file", file, "--json"]), + ); +} + +interface LogEvent { + type: string; + data: Record; +} + +function outlineFromLog(events: LogEvent[]): EvalFixture["outline"] { + const outline: EvalFixture["outline"] = []; + for (const event of events) { + if ( + event.type === "client/turn/requested" && + event.data.initiator === "user" + ) { + const input = (event.data.input ?? []) as { type: string; text?: string }[]; + const text = input + .filter((part) => part.type === "text" && typeof part.text === "string") + .map((part) => part.text) + .join("\n") + .trim(); + if (text !== "") outline.push({ role: "user", preview: text }); + } + if (event.type === "item/completed") { + const item = event.data.item as { type?: string; text?: unknown } | undefined; + if (item?.type === "agentMessage" && typeof item.text === "string" && item.text.trim() !== "") { + outline.push({ role: "assistant", preview: item.text }); + } + } + } + return outline; +} + +const [outDir, ...requested] = process.argv.slice(2).filter((arg) => arg !== "--"); +if (outDir === undefined) { + console.error("usage: vite-node eval/export.ts -- [threadId ...]"); + process.exit(2); +} +mkdirSync(outDir, { recursive: true }); + +const threadIds = + requested.length > 0 + ? requested + : (rpc("listBriefCards", null) as { cards: { threadId: string }[] }).cards.map( + (card) => card.threadId, + ); + +for (const threadId of threadIds) { + const state = rpc("getBrief", { threadId }) as { + state: string; + brief?: Record; + }; + const brief = state.state === "ready" ? state.brief : undefined; + const { thread } = JSON.parse(bb(["thread", "show", threadId, "--json"])) as { + thread: { title: string | null; titleFallback: string | null }; + }; + const events = JSON.parse(bb(["thread", "log", threadId, "--json", "--all"])) as LogEvent[]; + const output = bb(["thread", "output", threadId]).trim(); + + const fixture: EvalFixture = { + threadId, + title: thread.title ?? thread.titleFallback ?? null, + outline: outlineFromLog(events), + lastAssistantText: output === "" || output === "(no output)" ? null : output, + previousBrief: + brief === undefined + ? null + : { + ...(typeof brief.title === "string" ? { title: brief.title } : {}), + goal: String(brief.goal ?? ""), + currentState: String(brief.currentState ?? ""), + nextStep: String(brief.nextStep ?? ""), + ...(typeof brief.nextStepActor === "string" + ? { nextStepActor: brief.nextStepActor as "me" | "agent" | "other" } + : {}), + blockedOn: String(brief.blockedOn ?? ""), + constraints: String(brief.constraints ?? ""), + }, + stored: + brief === undefined + ? null + : { + status: String(brief.status), + statusOverride: (brief.statusOverride as string | null) ?? null, + }, + }; + writeFileSync(join(outDir, `${threadId}.json`), `${JSON.stringify(fixture, null, 1)}\n`); + console.log(`${threadId} ${fixture.outline.length} messages ${fixture.title ?? ""}`); +} diff --git a/plugins/thread-briefs/eval/fixture.ts b/plugins/thread-briefs/eval/fixture.ts new file mode 100644 index 0000000..f414445 --- /dev/null +++ b/plugins/thread-briefs/eval/fixture.ts @@ -0,0 +1,17 @@ +import type { BriefFields, StoredBriefStatus } from "../contract.js"; +import type { OutlineItem } from "../transcript.js"; + +/** One frozen thread, as `eval/export.ts` writes it. */ +export interface EvalFixture { + threadId: string; + title: string | null; + outline: OutlineItem[]; + lastAssistantText: string | null; + /** The brief stored when the fixture was taken: the next summary's starting point. */ + previousBrief: BriefFields | null; + /** What the live plugin showed for this thread when the fixture was taken. */ + stored: { status: string; statusOverride: string | null } | null; +} + +/** `labels.json`: the status a person says each thread should read. */ +export type EvalLabels = Record; diff --git a/plugins/thread-briefs/eval/run.ts b/plugins/thread-briefs/eval/run.ts new file mode 100644 index 0000000..df789c7 --- /dev/null +++ b/plugins/thread-briefs/eval/run.ts @@ -0,0 +1,201 @@ +/** + * Run a checkout's summarizer prompt over frozen threads against the live + * model, and report how often the status it produces agrees with a person's. + * + * FIREWORKS_API_KEY=… npx vite-node eval/run.ts -- \ + * --fixtures --labels [--src ] \ + * [--feedback on|off|both] [--runs 1] [--out results.json] + * + * `--src` is the plugin directory whose `summarize.ts`, `transcript.ts` and + * `brief.ts` are used, so the same harness scores `main` and a candidate: + * check one out with `git worktree add` and point `--src` at it. + * + * `--feedback` controls whether the stored brief is fed back as the previous + * brief. `on` is what a Re-summarize press does today; `off` is a thread's + * first summary. Running both shows whether feeding the brief back makes a + * wrong reading stick. + * + * A status is read from the parsed summary's own `status` when the prompt asks + * for one, and from the checkout's `deriveStatus` over its fields otherwise — + * which is how `main` read status before the summarizer was asked for it. + */ +import { readdirSync, readFileSync, writeFileSync } from "node:fs"; +import { join, resolve } from "node:path"; +import { parseArgs } from "node:util"; +import type { StoredBriefStatus } from "../contract.js"; +import type { EvalFixture, EvalLabels } from "./fixture.js"; + +const { values } = parseArgs({ + args: process.argv.slice(2).filter((arg) => arg !== "--"), + options: { + fixtures: { type: "string" }, + labels: { type: "string" }, + src: { type: "string", default: resolve(import.meta.dirname, "..") }, + feedback: { type: "string", default: "both" }, + runs: { type: "string", default: "1" }, + out: { type: "string" }, + "base-url": { type: "string", default: "https://api.fireworks.ai/inference/v1" }, + model: { type: "string", default: "accounts/fireworks/models/glm-5p3-flash" }, + concurrency: { type: "string", default: "6" }, + }, +}); + +if (values.fixtures === undefined || values.labels === undefined) { + console.error("usage: vite-node eval/run.ts -- --fixtures --labels "); + process.exit(2); +} +const apiKey = process.env.FIREWORKS_API_KEY ?? process.env.SUMMARIZER_API_KEY ?? ""; +if (apiKey === "") { + console.error("set FIREWORKS_API_KEY"); + process.exit(2); +} + +const src = resolve(values.src); +const summarize = await import(join(src, "summarize.ts")); +const transcriptModule = await import(join(src, "transcript.ts")); +const brief = await import(join(src, "brief.ts")); + +const labels = JSON.parse(readFileSync(values.labels, "utf8")) as EvalLabels; +const fixtures = readdirSync(values.fixtures) + .filter((name) => name.endsWith(".json")) + .map((name) => JSON.parse(readFileSync(join(values.fixtures!, name), "utf8")) as EvalFixture) + .filter((fixture) => fixture.threadId in labels); + +const feedbackModes: boolean[] = + values.feedback === "both" ? [true, false] : [values.feedback === "on"]; +const runs = Number(values.runs); +const config = { + baseUrl: values["base-url"]!, + apiKey, + model: values.model!, + jsonMode: true, +}; + +interface Outcome { + threadId: string; + feedback: boolean; + run: number; + expected: StoredBriefStatus; + got: StoredBriefStatus | "error"; + stage?: string; + nextStep?: string; + blockedOn?: string; + /** The status exactly as the model wrote it, before the parser's guard and fallback. */ + rawStatus?: unknown; + error?: string; +} + +function statusOf(summary: Record): StoredBriefStatus { + if (typeof summary.status === "string") return summary.status as StoredBriefStatus; + return brief.deriveStatus(summary); +} + +async function evaluate( + fixture: EvalFixture, + feedback: boolean, + run: number, +): Promise { + const expected = labels[fixture.threadId]!; + const transcript = transcriptModule.renderTranscript({ + title: fixture.title, + outline: fixture.outline, + lastAssistantText: fixture.lastAssistantText, + previousBrief: feedback ? fixture.previousBrief : null, + }); + const userPrompt = summarize.buildUserPrompt({ transcript, fixedStage: null, pinnedStatus: null }); + for (let attempt = 0; ; attempt += 1) { + try { + const reply = await summarize.requestSummary(config, userPrompt, AbortSignal.timeout(120_000)); + const summary = summarize.parseSummary(reply, null) as Record; + const raw = summarize.extractJson(reply) as Record; + return { + threadId: fixture.threadId, + feedback, + run, + expected, + got: statusOf(summary), + stage: String(summary.stage), + nextStep: String(summary.nextStep ?? ""), + blockedOn: String(summary.blockedOn ?? ""), + rawStatus: raw.status, + }; + } catch (error) { + if (attempt < 2) continue; + return { threadId: fixture.threadId, feedback, run, expected, got: "error", error: String(error) }; + } + } +} + +const jobs: (() => Promise)[] = []; +for (const feedback of feedbackModes) { + for (let run = 0; run < runs; run += 1) { + for (const fixture of fixtures) jobs.push(() => evaluate(fixture, feedback, run)); + } +} + +const outcomes: Outcome[] = []; +let next = 0; +await Promise.all( + Array.from({ length: Number(values.concurrency) }, async () => { + while (next < jobs.length) { + const job = jobs[next++]!; + const outcome = await job(); + outcomes.push(outcome); + process.stderr.write(outcome.got === outcome.expected ? "." : "x"); + } + }), +); +process.stderr.write("\n"); + +const STATUSES = ["done", "waiting-on-me", "waiting-on-other"] as const; +const short = { done: "done", "waiting-on-me": "on-me", "waiting-on-other": "on-other", error: "error" }; + +console.log(`src: ${src}`); +console.log(`model: ${config.model}, ${fixtures.length} threads, ${runs} run(s) each\n`); +for (const feedback of feedbackModes) { + const subset = outcomes.filter((outcome) => outcome.feedback === feedback); + const agree = subset.filter((outcome) => outcome.got === outcome.expected).length; + // The expensive error: a thread someone still owes an action read as done, + // which puts it on the auto-archive clock. + const falseDone = subset.filter( + (outcome) => outcome.got === "done" && outcome.expected !== "done", + ).length; + const missedDone = subset.filter( + (outcome) => outcome.expected === "done" && outcome.got !== "done", + ).length; + console.log(`## previous brief fed back: ${feedback ? "yes" : "no"}`); + console.log( + `agreement ${agree}/${subset.length} (${Math.round((100 * agree) / subset.length)}%)` + + ` false done ${falseDone} missed done ${missedDone}`, + ); + console.log(`\n| expected \\ got | ${[...STATUSES, "error" as const].map((s) => short[s]).join(" | ")} |`); + console.log(`| --- |${" --- |".repeat(STATUSES.length + 1)}`); + for (const expected of STATUSES) { + const row = [...STATUSES, "error" as const].map( + (got) => + subset.filter((outcome) => outcome.expected === expected && outcome.got === got).length, + ); + if (row.every((count) => count === 0)) continue; + console.log(`| ${short[expected]} | ${row.join(" | ")} |`); + } + const misses = subset + .filter((outcome) => outcome.got !== outcome.expected) + .sort((a, b) => a.threadId.localeCompare(b.threadId)); + if (misses.length > 0) { + console.log("\nDisagreements:"); + for (const miss of misses) { + console.log( + `- ${miss.threadId} expected ${short[miss.expected]}, got ${short[miss.got]}` + + (miss.error !== undefined + ? `: ${miss.error.slice(0, 160)}` + : `${miss.rawStatus !== miss.got ? ` (model said ${JSON.stringify(miss.rawStatus) ?? "nothing"})` : ""}` + + `${miss.blockedOn ? ` blockedOn: "${miss.blockedOn}"` : ""} nextStep: "${miss.nextStep}"`), + ); + } + } + console.log(""); +} + +if (values.out !== undefined) { + writeFileSync(values.out, `${JSON.stringify(outcomes, null, 1)}\n`); +} diff --git a/plugins/thread-briefs/server.test.ts b/plugins/thread-briefs/server.test.ts index d7aa648..c713e02 100644 --- a/plugins/thread-briefs/server.test.ts +++ b/plugins/thread-briefs/server.test.ts @@ -13,6 +13,7 @@ const SUMMARY = { blockedOn: "", constraints: "bb exposes no additive per-row sidebar slot", stage: "review", + status: "waiting-on-me", }; function fakeCompletion(body: unknown) { @@ -229,6 +230,26 @@ describe("summarizing", () => { expect(stored?.fields.goal).toBe(SUMMARY.goal); }); + it("stores a done reading beside the suggestion it still carries", async () => { + const fetchMock = fakeCompletion({ ...SUMMARY, status: "done" }); + current = host({ fetch: fetchMock }); + await plugin(current.bb); + vi.useFakeTimers(); + + await current.harness.behavior.emitThreadEvent("thread.idle", { + thread, + lastAssistantText: "done", + }); + await settle(6_000); + + // Stored as the model answered: neither field is rewritten to agree + // with the other. + const stored = await current.bb.storage.kv.get("brief:thr_1"); + expect(stored?.modelStatus).toBe("done"); + expect(stored?.fields.nextStep).toBe(SUMMARY.nextStep); + expect(stored?.refresher?.writtenForStatus ?? "done").toBe("done"); + }); + it("summarizes a briefless thread as soon as it starts running", async () => { const fetchMock = fakeCompletion(SUMMARY); current = host({ fetch: fetchMock }); diff --git a/plugins/thread-briefs/server.ts b/plugins/thread-briefs/server.ts index 5a19d7a..122c633 100644 --- a/plugins/thread-briefs/server.ts +++ b/plugins/thread-briefs/server.ts @@ -13,7 +13,6 @@ import { import { briefCardFor, briefKey, - deriveStatus, effectiveStatus, overrideHolds, planRename, @@ -517,8 +516,9 @@ export default async function plugin(bb: BbPluginApi) { // A stage override in force is passed to the model as fixed; one the thread // has moved past is dropped here, which is what "sticks until real thread - // activity" means. The status override needs no such handoff — the model is - // never asked for a status — but expires on the same terms. + // activity" means. The status override expires on the same terms, but is not + // passed as fixed: the model still judges the status from the transcript, + // and only the refresher prose is told about the pin. const stagePin = carryOverride( stored?.stageOverride, stored?.stageOverrideSeq, @@ -586,7 +586,7 @@ export default async function plugin(bb: BbPluginApi) { ? null : { ...summary.refresher, - writtenForStatus: statusPin.value ?? deriveStatus(fields), + writtenForStatus: statusPin.value ?? summary.status, }; await writeBrief({ @@ -594,6 +594,7 @@ export default async function plugin(bb: BbPluginApi) { threadId, fields, modelStage: summary.stage, + modelStatus: summary.status, stageOverride: stagePin.value, stageOverrideSeq: stagePin.seq, statusOverride: statusPin.value, @@ -1263,6 +1264,4 @@ export default async function plugin(bb: BbPluginApi) { }); } -// Re-exported for tests that exercise the derivation without a server. -export { deriveStatus }; export type { BriefStage }; diff --git a/plugins/thread-briefs/skills/thread-briefs/SKILL.md b/plugins/thread-briefs/skills/thread-briefs/SKILL.md index 6c0243c..c361db9 100644 --- a/plugins/thread-briefs/skills/thread-briefs/SKILL.md +++ b/plugins/thread-briefs/skills/thread-briefs/SKILL.md @@ -134,16 +134,10 @@ override it; the override is anchored to the thread's activity cursor and retire itself on the next real turn. Clicking the active manual stage clears it. The override moves the row's ring too, immediately. -Stage and `nextStep` come back from one model call, and nothing in that call -makes the model answer both consistently — the usual failure is a stage left at -`implementation` beside an empty `nextStep`, on a thread whose agent has just -narrated what it built and deployed. Since an empty `nextStep` derives `done`, -that pair renders as "Implementation — Done" and drags out of the board's Done -column back into Implementation. `reconcileStage` in `summarize.ts` promotes it: -nothing owed means the work is made, so the stage is `review`. Only from -`implementation` — a `discovery` or `planning` thread with nothing owed was -dropped before any work existed, and there is nothing there to review. A pinned -stage short-circuits it, since a pin promises to come back as given. +Stage and status come back from one model call and are stored as answered. +The parser does not correct one against the other: "Implementation — Done" is a +model answer to fix in the prompt, not to paper over. A pinned stage is passed to +the model as fixed and returned as given. Both overrides share that anchor rule, and "the next real turn" means a summary whose conversation cursor has moved past where the pin was set — so @@ -151,22 +145,42 @@ whose conversation cursor has moved past where the pin was set — so actual turn drops it. The anchor is never re-stamped to the new cursor; one that advanced in step with the activity meant to expire it would never expire. -`status` is derived mechanically, so it stays correct between summaries. It has -a live half and a stored half, and the live half wins: +`status` has a live half and a stored half, and the live half wins: - the thread is `active`, `starting` or `pending` → **working**, whatever the brief says. A run in flight is newer information than the brief, which describes the last turn that finished. - otherwise, from the stored brief: - - a manual status override in force → that status, whatever the fields say - - `nextStep` **and** `blockedOn` both empty → **done** - - `blockedOn` non-empty, or `nextStepActor` is `other` → **waiting-on-other** - - otherwise → **waiting-on-me** - -**done** needs both fields empty, so a blocked thread cannot read as done even -if a summary comes back without a next step. And **waiting-on-me** is the -fallback: an idle thread with work left needs a human look whether or not its -last turn ended in a question, and whether or not the actor is known. + - a manual status override in force → that status + - the status the summarizer answered (`modelStatus` on the row) + - a brief written before the summarizer was asked for a status has none, and + reads the old way until its thread is next summarized: `nextStep` and + `blockedOn` both empty → done, `blockedOn` set or `nextStepActor` `other` → + waiting-on-other, otherwise waiting-on-me + +The summarizer is asked one question: **assume you do whatever the thread asks of +you — is the task then finished, or does the thread have more to do?** + +- **done** — finished. A step that is only yours ("approve PR #12", "run the + rollout check"), an offer of extra work ("want me to add a lint rule?") and an + optional check do not keep it open: nothing more happens in the thread either + way. `nextStep` may still carry it, as a suggestion. +- **waiting-on-me** — your answer, decision or go-ahead starts more work in the + thread ("the diff is ready but uncommitted — should I push?"), or the task is + otherwise unfinished. +- **waiting-on-other** — the work cannot go on until something outside the + thread acts, named in `blockedOn`. + +One rule holds whatever the model says: a brief with a non-empty `blockedOn` is +never **done** — it reads **waiting-on-other**. A status the parser cannot read +falls back to **waiting-on-me**, because a thread wrongly left waiting costs a +glance and one wrongly called done is archived two days later. + +`nextStep` and `blockedOn` are **not** fed back into the next summary as part of +the previous brief; only title, goal, currentState and constraints are. Fed +back, they outlived the turns that retired them — a step the agent had since +dropped, a rollout that had since finished — and kept finished threads out of +Done. Both are re-read from the transcript every time. ### Overriding the status by hand @@ -175,19 +189,15 @@ is anchored to the thread's activity cursor, retires on the next real turn, and clears if you click the one that is already pinned. It moves the sidebar section as well as the row, because sections are keyed on this status. -**Why it exists.** The derivation reads the brief's prose, and the prose can -record a `nextStep` that is addressed to you and carried out somewhere the -transcript cannot see — "reload an open client and confirm the panel tab -opens", "check the rollout landed", "confirm the glyph looks right". Doing it -leaves no trace for any summary to read, so **Re-summarize** just writes the -same unresolved instruction back and the thread is **waiting-on-me** forever. -The pin is the only way to say you did it. +**Why it exists.** The summarizer only sees the transcript. A go-ahead you gave +somewhere else, or a PR you merged on github.com that the thread was waiting +on, leaves no trace for any summary to read, so **Re-summarize** reads the same +open question back. The pin is the way to say it is settled — or simply that you +read the thread differently from the model. -The pin sits *in front of* the derivation rather than editing the fields it -reads. Blanking `nextStep` in storage would not work: `renderTranscript` feeds -the previous brief into the next summary as a starting point, so the field would -simply come back. A pin is a separate fact the summarizer is never shown and -cannot undo. +The pin sits *in front of* the model's status rather than replacing it. It is a +separate fact the summarizer is never shown as a status and cannot undo; it is +told about it only so the refresher prose agrees. Dragging the row into another sidebar section is **not** a substitute. Section assignment never feeds back into a brief, so the next reconcile — on plugin @@ -195,64 +205,18 @@ start, after any batch of briefs, or on a settings change — files the thread straight back where its status says. Pin the status instead and the section follows. -`nextStepActor` is the one input the model judges rather than the code: "try it -and tell me if the glyph looks right" and "keep porting the call sites" are both -concrete next actions, and nothing in the prose separates them. It is **optional** -— absent means unknown, which covers both a brief written before the field -existed and one whose `nextStep` is empty — and an actor the parser does not -recognise is dropped rather than failing the brief, the same bar as an -unrecognised stage. Either way the status falls back to the rules above with the -actor clause skipped. - -An actor of `agent` — an idle thread the agent could carry on by itself — has no -status of its own and currently reads **waiting-on-me**, because the nudge is -yours to give. If that bucket turns out to be common it earns its own status -then. - -Briefs are not re-summarized to pick the field up: old rows gain an actor on -their next natural summary. To see how far that has spread, compare the rows -that have one against the total. - -Beyond that guard, **done** is only as good as the summarizer's bar for -"finished", and the prompt sets that bar at **whether anybody owes the thread an -action** — something a person or team must do, that will not happen on its own, -and that would be dropped if the brief did not record it. - -That test cuts both ways, and the prompt names both halves because each has its -own failure: - -- An obligation **outside the chat** still counts, and is the one that gets - silently dropped: a PR open for review or merge, a patch carried on a fork - until it lands upstream, a temporary workaround to undo, a rollout to finish - and confirm. These earn a `nextStep` and a `blockedOn`, so the thread reads - **waiting-on-other** rather than done. -- **Nothing is owed to the passage of time.** Open-ended watching — "check back - in a few days", "keep an eye on it", "confirm it behaves in real use" — has no - owner and no definite outcome, so it does not keep a thread open. Neither does - work the transcript puts out of scope, nor an idea nobody adopted. - -The second half exists because the first, on its own, made `done` a function of -the agent's closing rhetoric rather than of the work. Agents habitually hedge -when they sign off — "worth a glance", "I'd flag this as open" — and a bar of -"nothing outstanding anywhere" is unfalsifiable, so any such sentence kept a -finished thread out of Done. Two threads that had both shipped and rolled out -landed in different sections purely because one agent volunteered a caveat. The -prompt now says to judge the state of the work, not the tone of the sign-off. - -`blockedOn` carries a **higher** bar than `nextStep`, because it is the field -that jams the door: any non-empty value forces **waiting-on-other**, and -`renderTranscript` feeds the previous brief back into the next summary, so a -stray value is sticky. It must name a party or artifact someone could go -chase — a specific review, a person, an upstream fix, a running build, an access -grant — never a duration, "real usage", or "more data". The Blocked section is -meant to be a list of things you could go poke; if you cannot say who would be -chased, it is not blocked. - -A thread whose brief disagrees with this bar is usually one written before the -bar changed: **Re-summarize** from the Brief panel. That re-reads the -transcript under the current prompt, but note it also feeds the old brief back -as a starting point, so a wrong `blockedOn` can survive if the transcript still -reads as though it were true. +`nextStepActor` says who would take `nextStep` — `me`, `agent` or `other` — and +no longer feeds the status. It is optional, dropped when `nextStep` is empty or +the word is not one of the three. Its one use is the board's +`agent can continue` hint. + +**Re-summarize** re-reads a thread under the current prompt. Old briefs are not +re-summarized in bulk: every idle thread that now read done would already be +past the archive threshold, and the next sweep would take them all at once. + +The numbers behind the current prompt, and the harness to re-measure a change +to it, are in the plugin's `eval/` directory and README ("Measuring a prompt +change"). ## The re-entry refresher @@ -337,8 +301,8 @@ shown. The pin is scoped to the two prose fields in the prompt, and the prompt says so: the five fields still describe the work as the transcript leaves it, for the -same reason the pin sits in front of the derivation rather than editing what it -reads. +same reason the pin sits in front of the model's status rather than editing +what it reads. ### Where it renders @@ -472,6 +436,12 @@ bb plugin config thread-briefs set doneStaleHours 24 # 0 keeps every done rin bb plugin config thread-briefs set doneArchiveHours 48 # 0 never auto-archives ``` +A done thread may still carry a step that is yours — approving a PR, running a +check — and is archived on the same clock. That is deliberate: the step sits on +the card in the Done section for two days, the hover label warns for the second, +and an archived thread un-archived by hand is never auto-archived again. If you +want longer, raise `doneArchiveHours`. + The grey **replaces** the project hue; there is no `-c` variant of it. The row has one channel, colour on it means a live project, and a thread about to leave the sidebar has no use for the mark saying whose it is. The shape is unchanged, @@ -611,8 +581,8 @@ project's colour (the same glyph as the sidebar row, including the grey one for cold done thread), a status badge, `blockedOn`, the project, and the idle age. `agent can continue` appears when `nextStepActor` is `agent` and the status is -`waiting-on-me`. The derivation collapses those two cases (see -[Stage and status](#stage-and-status)), so this is the only place the difference +`waiting-on-me`. That status covers both a thread waiting on your answer and +one the agent could carry on by itself, so this is the only place the difference shows. Order inside a column: pinned threads, then `waiting-on-me` → Blocked → Working → @@ -634,8 +604,8 @@ is what the `pinned` marker on the card is warning about. | to or from **No stage** | nothing | Dragging out of Done *pins* rather than clears because a done reading can come -from the derivation as well as a pin; clearing would hand the card back to a -derivation that still says done and it would snap straight back. +from the model as well as a pin; clearing would hand the card back to a model +reading that still says done and it would snap straight back. Drag-and-drop does not work on touch, so the **expanded card carries the panel's own stage and status controls**. On a compact viewport the columns stack into one @@ -773,10 +743,14 @@ no preference writes. prompt — is named by the pre-turn brief instead of waiting. If such a row keeps its prompt, the rename failed: check `bb plugin logs thread-briefs` for `could not rename`. -- A thread stuck on **Waiting on you** whose next step you have already carried - out: expected if the step happened outside the thread, because nothing in the - transcript can record that. Pin the status to **Done** in the Brief panel — - see [Overriding the status by hand](#overriding-the-status-by-hand). +- A finished thread stuck on **Waiting on you**: first check the brief is new — + a brief written before the summarizer answered status reads the old way, and + **Re-summarize** rewrites it. If it is new and the thread waits on a go-ahead + you gave elsewhere, pin the status to **Done** in the Brief panel — see + [Overriding the status by hand](#overriding-the-status-by-hand). +- A **Done** thread that still shows a next step: expected. Done means nothing + more happens in the thread once you do what it asked; the step left on the + card is yours, and the thread is archived on the usual clock. - A thread that will not stay in the section you drag it to: sections are keyed on status and nothing feeds an assignment back into a brief, so the next reconcile undoes the move. Pin the status instead. @@ -804,7 +778,7 @@ no preference writes. the `pinned` marker on the card says one is in force. - A card that will not stay out of **Done**: pin the status to something else from the expanded card. Dragging already does this, but a re-summary after the next - turn will re-derive `done` if the brief still has nothing outstanding. + turn may read `done` again if the transcript still says the task is finished. - Drag does nothing on a phone or tablet: expected — it is an HTML5 pointer drag. Use the stage and status controls on the expanded card. - No **Briefs** item in the sidebar: it is a nav panel, so it can be hidden or diff --git a/plugins/thread-briefs/summarize.test.ts b/plugins/thread-briefs/summarize.test.ts index a5de424..981fb37 100644 --- a/plugins/thread-briefs/summarize.test.ts +++ b/plugins/thread-briefs/summarize.test.ts @@ -6,8 +6,8 @@ import { extractJson, normalizeRefresher, normalizeTitle, + normalizeStatus, parseSummary, - reconcileStage, } from "./summarize.js"; import { MAX_REFRESHER_LENGTH, MAX_TITLE_LENGTH } from "./contract.js"; import { @@ -26,6 +26,7 @@ const full = { blockedOn: "", constraints: "", stage: "implementation", + status: "waiting-on-me", }; describe("extractJson", () => { @@ -258,60 +259,56 @@ describe("normalizeRefresher", () => { }); }); -describe("reconcileStage", () => { - it("promotes implementation to review when nothing is owed", () => { - expect(reconcileStage("implementation", "")).toBe("review"); - }); - - it("leaves implementation alone while a next step stands", () => { - expect(reconcileStage("implementation", "Run the tests")).toBe("implementation"); +describe("normalizeStatus", () => { + it("keeps each status the model can answer, case and spacing aside", () => { + expect(normalizeStatus("done", "")).toBe("done"); + expect(normalizeStatus(" Waiting-On-Me ", "")).toBe("waiting-on-me"); + expect(normalizeStatus("waiting-on-other", "Review from Sam")).toBe( + "waiting-on-other", + ); }); - it("never promotes a stage with no work behind it", () => { - // A discovery or planning thread with nothing owed was dropped before any - // work existed; calling it review would claim there is something to review. - expect(reconcileStage("discovery", "")).toBe("discovery"); - expect(reconcileStage("planning", "")).toBe("planning"); + it("falls back to waiting-on-me, the reading whose mistake is cheap", () => { + // A wrong waiting-on-me is sidebar noise; a wrong done is archived two + // days later. An answer we cannot read must land on the cheap side. + expect(normalizeStatus(undefined, "")).toBe("waiting-on-me"); + expect(normalizeStatus("finished", "")).toBe("waiting-on-me"); + expect(normalizeStatus(true, "")).toBe("waiting-on-me"); }); - it("leaves review where it is", () => { - expect(reconcileStage("review", "")).toBe("review"); + it("never calls a thread done while it names something it is waiting on", () => { + expect(normalizeStatus("done", "Review from Sam")).toBe("waiting-on-other"); }); }); -describe("parseSummary stage reconciliation", () => { - it("reads a completion narrative as review, not implementation", () => { - // The bug this exists for: one call returns both keys, and an agent signing - // off with what it built gets an empty nextStep beside an unmoved stage. - const parsed = parseSummary( - reply({ ...full, nextStep: "", stage: "implementation" }), - null, - ); - expect(parsed.stage).toBe("review"); - }); - - it("promotes the fallback stage too", () => { - // An unusable stage falls back to implementation, and a thread owing - // nothing is better guessed as review than as mid-build. +describe("parseSummary status", () => { + it("takes the status the model answered, not one read off nextStep", () => { + // A finished thread may carry a suggestion; an unfinished one may have no + // step written down. Neither field decides the other. + expect( + parseSummary(reply({ ...full, nextStep: "Add a lint rule", status: "done" }), null) + .status, + ).toBe("done"); expect( - parseSummary(reply({ ...full, nextStep: "", stage: "vibes" }), null).stage, - ).toBe("review"); + parseSummary(reply({ ...full, nextStep: "", status: "waiting-on-me" }), null) + .status, + ).toBe("waiting-on-me"); }); - it("treats an empty synonym as nothing owed", () => { + it("applies the blockedOn guard after empty synonyms are cleared", () => { expect( - parseSummary(reply({ ...full, nextStep: "N/A", stage: "implementation" }), null) - .stage, - ).toBe("review"); + parseSummary(reply({ ...full, blockedOn: "None", status: "done" }), null).status, + ).toBe("done"); + expect( + parseSummary(reply({ ...full, blockedOn: "Sam's review", status: "done" }), null) + .status, + ).toBe("waiting-on-other"); }); - it("leaves a pinned stage pinned", () => { - // The pin's promise is that it is returned whatever the transcript says. + it("leaves the stage as the model judged it, whatever nextStep says", () => { expect( - parseSummary( - reply({ ...full, nextStep: "", stage: "review" }), - "implementation", - ).stage, + parseSummary(reply({ ...full, nextStep: "", stage: "implementation" }), null) + .stage, ).toBe("implementation"); }); }); @@ -382,7 +379,7 @@ describe("selectOutline", () => { }); describe("renderTranscript", () => { - it("includes the previous brief so the summarizer updates rather than restarts", () => { + it("feeds back the settled fields of the previous brief, not where it stood", () => { const text = renderTranscript({ title: "Thread briefs", outline: [{ role: "user", preview: "Build it" }], @@ -397,7 +394,11 @@ describe("renderTranscript", () => { }); expect(text).toContain("Previous brief"); expect(text).toContain("kv rows cap at 256KB"); - expect(text).toContain("blockedOn: (empty)"); + expect(text).toContain("currentState: half done"); + // nextStep and blockedOn are re-read from the transcript every time: handed + // back, they outlive the turns that made them obsolete. + expect(text).not.toContain("nextStep"); + expect(text).not.toContain("blockedOn"); expect(text).toContain("User: Build it"); }); diff --git a/plugins/thread-briefs/summarize.ts b/plugins/thread-briefs/summarize.ts index 84a8266..52b884f 100644 --- a/plugins/thread-briefs/summarize.ts +++ b/plugins/thread-briefs/summarize.ts @@ -3,6 +3,7 @@ import { MAX_REFRESHER_LENGTH, MAX_TITLE_LENGTH, nextStepActorSchema, + storedBriefStatusSchema, summaryResultSchema, type BriefStage, type NextStepActor, @@ -18,29 +19,26 @@ Return ONLY a JSON object with exactly these keys: "title" A name for this thread, 4-6 words, that someone scanning a sidebar would recognise a day later. Name the work, not the conversation: the subsystem, file, or feature plus what is being done to it. No trailing punctuation, no quotes, no "thread"/"discussion"/"chat", no leading verb like "Add" unless adding is genuinely the whole job. "goal" One line: what this thread is actually trying to achieve. Not the opening prompt restated — the underlying objective, as it stands now. "currentState" What exists now, including half-finished work. Name the concrete artifacts (files, branches, PRs) where the transcript names them. - "nextStep" The single most concrete next action, phrased so the reader could start it without thinking. ONE action, not a plan. Empty string if nobody owes this thread an action. - "nextStepActor" Who has to take that next step. One of: "me" if only the user can (try it and report back, decide between options, reply to someone, merge, grant access), "agent" if the agent could carry on unprompted, "other" if it depends on someone or something outside this thread (a review, a colleague, an upstream fix, a rollout). - "blockedOn" The party or artifact the thread is waiting on, when someone could go chase it. Empty string otherwise. + "status" Picture the user having done everything this thread asks of them: run the command, merged or approved, checked the thing, answered the offer. Is the task then finished, with nothing left to happen in this thread? One of: + "done" — yes. Steps that are the user's alone, offers of extra work, and optional checks do not keep it open. Example: "Merged and deployed. Run kubectl rollout status to confirm — and want me to add a lint rule too?" + "waiting-on-me" — no: the user's answer, decision, or go-ahead starts more work in this thread, such as committing or shipping work the agent has made, or the task is otherwise unfinished. Example: "The diff is ready but uncommitted. Should I commit and push?" + "waiting-on-other" — no: the work cannot go on until the party in "blockedOn" acts. Example: "Waiting on Sam's review of PR #12 before I merge." + "nextStep" The most useful next action, if there is one, phrased so the reader could start it without thinking. ONE action, not a plan. A finished thread may still carry a suggestion here. Empty string if there is nothing worth doing. + "nextStepActor" Who would take "nextStep": "me" (the user), "agent", or "other" (someone outside this thread). Omit it when "nextStep" is empty. + "blockedOn" The party or artifact outside this thread that the work is waiting on, named so someone could go chase it: a review, a person, an upstream fix, an access grant. Never a duration, and never something that will finish on its own. Empty string otherwise. "constraints" Facts learned during the thread that would break a naive re-plan: API limits, rejected approaches, assumptions proven wrong. Empty string if none. "stage" How far round the arc the work itself has got. One of: "discovery" (still establishing what is true or what is wanted), "planning" (the shape is agreed, the making has not started), "implementation" (the work is being made), "review" (the work is made, and is being checked, tried, or waited on for a verdict). Judge the work, not the conversation: a thread whose agent has finished building and described what it built is at "review", whether or not anyone has looked at it yet. "refresherShort" One or two sentences of plain prose, addressed to the user as "you", for someone reopening this thread after a few hours: what they were doing, how far it got, what to do next. "refresherFull" The same thing for someone who has been away for days: two or three sentences, with enough named detail to stand on its own. Rules: -- Every field is a string except "nextStepActor", which is one of the three words above. Keep each to one or two lines. +- Every key above is required except "nextStepActor". Every field is a string; "nextStepActor" and "status" are one of the words listed for them. Keep each to one or two lines. - The two "refresher" fields are prose, not labelled fields: flowing sentences, no "Goal:" / "Next:" prefixes, no bullet points, no headings. Write them as you would say them to the person over their shoulder as they sit back down. - Write them in that order — what you were doing, how far it got, what to do next — and name things concretely: the file, the branch, the PR, the command. "You were partway through the sidebar sections" is useless; "the section sync lands but the order is not pinned yet" is the point. - Mention what is blocking, or a constraint learned in the thread, ONLY when it changes what to do next. A blocker that has already been routed around is history, not orientation. -- When "nextStep" is empty, the refreshers say so plainly — what the thread landed, and that nothing is owed. Never manufacture a next action for them that "nextStep" itself would not carry. +- When "status" is "done", the refreshers say what the thread landed, and offer "nextStep" (if any) as an option rather than as owed work. - "refresherFull" is not "refresherShort" with adjectives. It is allowed the detail the short one had to drop: the second half of the state, the constraint that will bite, the name of the thing that is blocked. - "title" describes what the thread turned out to be about, not what its opening message asked for. A thread that set out to fix a test and ended up rewriting the scheduler is named for the scheduler. -- Omit "nextStepActor" entirely when "nextStep" is the empty string — there is no actor for a step that does not exist. -- When "blockedOn" is non-empty, "nextStepActor" is "other". -- A thread is finished when nobody owes it an action. For any candidate next step, ask: must a person or team actually do this, will it not happen on its own, and would it be dropped if this brief did not record it? Yes to all three — that is "nextStep", and the thread is not done. Otherwise "nextStep" is the empty string. Never invent one; a brief that manufactures work devalues every real item next to it. -- An action can be owed outside the chat, and those are the ones that get silently dropped: a PR open for review or merge, a patch carried on a fork or side branch until it lands upstream, a temporary workaround to undo, a build or rollout to finish and confirm, a question put to someone who has not answered. Recording these is not inventing work — the transcript already named them. -- Nothing is owed to the passage of time. Open-ended watching has no owner and no definite outcome — "check back in a few days", "keep an eye on it", "confirm it behaves in real use" — and does NOT keep a thread open. Nor does work the transcript puts out of scope, nor an idea raised and not adopted. Judge the state of the work, not the tone of the sign-off: agents habitually hedge when they finish ("worth a glance", "I'd flag this as open"), and an item nobody must act on does not block done however the transcript labels it. Keep anything worth remembering in "currentState" or "constraints". -- "stage" and "nextStep" describe the same thread and must agree. An empty "nextStep" means nothing is owed, which is only true once the work is made — so the stage is "review". Never return "implementation" alongside an empty "nextStep". -- "blockedOn" is held to a higher bar than "nextStep": name a party or artifact someone could go chase — a specific review, a person, an upstream fix, a running build, an access grant. Never a duration, never "real usage" or "more data". If you cannot say who would be chased, leave it empty. - Use empty strings, not "none" / "N/A" / "nothing". - Write plainly and specifically. No preamble, no hedging, no restating these instructions. - Base every claim on the transcript. Do not speculate about what the code or the user probably wants.`; @@ -258,31 +256,27 @@ export function normalizeRefresher(record: { } /** - * The stage a summary lands on once it is reconciled with its own `nextStep`. + * The status a summary reports, or `waiting-on-me` when the model gave none we + * recognise. * - * Stage and `nextStep` come back in one JSON object from one call, and nothing - * holds the model to answering both consistently. The commonest way a good - * brief comes back wrong is `"implementation"` beside an empty `nextStep` — an - * agent whose last turn narrated what it built, deployed and handed over. - * `deriveStatus` reads that empty `nextStep` as `done`, so the pair renders as - * "Implementation — Done", and dragging the card out of the board's Done column - * puts it back into Implementation rather than Review. + * `waiting-on-me` is the fallback because it is the reading whose error is + * cheap: a thread wrongly left waiting is noise in the sidebar, where one + * wrongly called done goes grey and is archived two days later. * - * An empty `nextStep` means nobody owes the thread an action, which is only - * true once the work is made — so the stage is `review`. The prompt asks for - * exactly this; doing it here as well is what makes it a guarantee of this code - * rather than of the prompt, in the same spirit as `deriveStatus` testing - * `blockedOn` for itself. - * - * Only from `implementation`. A `discovery` or `planning` thread with nothing - * owed was concluded or abandoned before any work existed, and calling that - * `review` would claim there is something to review. + * The one rule applied on top of the model is true whatever the model thinks: a + * thread that names something it is waiting on is not finished. It reads the + * model's own `blockedOn` rather than any judgement about it, so it is a hard + * invariant, not a tie-break between two answers. */ -export function reconcileStage( - stage: BriefStage, - nextStep: string, -): BriefStage { - return stage === "implementation" && nextStep === "" ? "review" : stage; +export function normalizeStatus( + value: unknown, + blockedOn: string, +): StoredBriefStatus { + const parsed = storedBriefStatusSchema.safeParse( + typeof value === "string" ? value.trim().toLowerCase() : value, + ); + const status = parsed.success ? parsed.data : "waiting-on-me"; + return status === "done" && blockedOn !== "" ? "waiting-on-other" : status; } export function parseSummary( @@ -296,21 +290,19 @@ export function parseSummary( const record = raw as Record; const nextStep = normalizeField(record.nextStep); + const blockedOn = normalizeField(record.blockedOn); - // A pinned stage short-circuits both the fallback and the reconciliation: the - // prompt promises the user's pick is returned whatever the transcript says, - // and a pin overruled here would be a pin that silently did not hold. + // A pinned stage short-circuits the fallback: the prompt promises the user's + // pick is returned whatever the transcript says, and a pin overruled here + // would be a pin that silently did not hold. const rawStage = typeof record.stage === "string" ? record.stage.trim() : ""; const stage: BriefStage = fixedStage ?? - reconcileStage( - BRIEF_STAGES.includes(rawStage as BriefStage) - ? (rawStage as BriefStage) - : // An unrecognized stage is not worth failing the whole brief over; - // implementation is the safest neutral guess. - "implementation", - nextStep, - ); + (BRIEF_STAGES.includes(rawStage as BriefStage) + ? (rawStage as BriefStage) + : // An unrecognized stage is not worth failing the whole brief over; + // implementation is the safest neutral guess. + "implementation"); return summaryResultSchema.parse({ title: normalizeTitle(record.title), @@ -318,9 +310,10 @@ export function parseSummary( currentState: normalizeField(record.currentState), nextStep, nextStepActor: normalizeActor(record.nextStepActor, nextStep), - blockedOn: normalizeField(record.blockedOn), + blockedOn, constraints: normalizeField(record.constraints), stage, + status: normalizeStatus(record.status, blockedOn), refresher: normalizeRefresher({ short: record.refresherShort, full: record.refresherFull, diff --git a/plugins/thread-briefs/transcript.ts b/plugins/thread-briefs/transcript.ts index 7ee44f1..ca23deb 100644 --- a/plugins/thread-briefs/transcript.ts +++ b/plugins/thread-briefs/transcript.ts @@ -73,6 +73,11 @@ export function renderTranscript(input: TranscriptInput): string { const previous = input.previousBrief; parts.push( [ + // Only the fields that should hold still from one summary to the next. + // `nextStep`, `blockedOn` and the status are left out: they describe + // where the thread stands *now*, and handed back as a starting point + // they survive the turns that made them obsolete — a step the agent + // has since retired, a rollout that has since finished. "Previous brief (update it; keep what is still true, correct what is not):", // Fed back so the name only moves when the work moved. Without it the // model renames from scratch every summary and a settled thread @@ -80,8 +85,6 @@ export function renderTranscript(input: TranscriptInput): string { ` title: ${previous.title || "(empty)"}`, ` goal: ${previous.goal || "(empty)"}`, ` currentState: ${previous.currentState || "(empty)"}`, - ` nextStep: ${previous.nextStep || "(empty)"}`, - ` blockedOn: ${previous.blockedOn || "(empty)"}`, ` constraints: ${previous.constraints || "(empty)"}`, ].join("\n"), ); diff --git a/plugins/thread-briefs/tsconfig.json b/plugins/thread-briefs/tsconfig.json index be1a125..f3c40f1 100644 --- a/plugins/thread-briefs/tsconfig.json +++ b/plugins/thread-briefs/tsconfig.json @@ -33,6 +33,7 @@ "board-page.tsx", "app.tsx", "*.test.ts", - "*.test.tsx" + "*.test.tsx", + "eval/*.ts" ] }