diff --git a/AGENTS.md b/AGENTS.md index f90a021..ebca7c5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -14,6 +14,7 @@ A spec is the accurate reference for the current code: it states the invariants - **`docs/specs/sync.md`** — Moving batches, images, decisions, and goldens between stores. - **`docs/specs/cloud.md`** — The Worker at `hackathon.selfie.dev`: its projects, origins, pages, and a commit's status. - **`docs/specs/feedback.md`** — Notes: how a person tells a coding agent what they see, and how the agent answers. +- **`docs/specs/chat.md`** — Asking Muse Spark about a story's change, with its images and the code from GitHub. - **`docs/specs/render.md`** — The renderer: rendering a built Storybook into PNGs in a pinned environment, the environment ID, and the manifest. - **`docs/specs/cli.md`** — The `selfie` CLI: `review`, `approve`, `reject`, and `ui`, their exit codes and JSON, and local data's layout. - **`docs/specs/ui.md`** — The review UI: its routes, the seam a platform implements, and what each screen must show. Read `docs/specs/access.md` → Local review server with it. diff --git a/README.md b/README.md index 24ccd97..aae987c 100644 --- a/README.md +++ b/README.md @@ -99,7 +99,7 @@ From a checkout, `pnpm selfie ` runs these. The local loop, CI, and commit statuses are all built and in use on this repository. -The cloud service at [hackathon.selfie.dev](https://hackathon.selfie.dev) is deployed for a few public repositories. Anyone can view them, and for now only one person can sign in to approve or upload. +The cloud service at [hackathon.selfie.dev](https://hackathon.selfie.dev) is deployed for a few public repositories. Anyone can view them, and ask Muse Spark about a story's change. For now only one person can sign in to approve or upload. Not built yet: @@ -115,9 +115,9 @@ We previously launched [Selfie](https://lib.selfie.dev), a snapshot testing libr ### Meta models -**As submitted, Selfie doesn't call a Meta model.** We spent the event building the foundation: rendering, storage, review, and sync. +Most of the event went on the foundation: rendering, storage, review, and sync. **Selfie calls a Meta model in one place so far.** On a changed story's page in the cloud service, anyone can ask Muse Spark about the change. It reads the before, after, and diff images, and the commits and code diff from GitHub between what was approved and this commit ([`docs/specs/chat.md`](docs/specs/chat.md)). -The planned integration is specified in [`docs/specs/system.md`](docs/specs/system.md) as `change-grouping`. It would send Muse Spark the before and after renders, the code diff, and the stated intent of the change, then group the changed renders by the code change that caused them, and flag the changes nobody asked for. Selfie's own code can say exactly which pixels differ and whether content moved, but not why, or whether anyone wanted it. That matters at agent scale: one style tweak we measured changed 290 of a Storybook's 541 stories. +Selfie's own code can say exactly which pixels differ and whether content moved, but not why, or whether anyone wanted it. That matters at agent scale: one style tweak we measured changed 290 of a Storybook's 541 stories, which is why the next step, specified in [`docs/specs/system.md`](docs/specs/system.md) as `change-grouping`, is to group changed stories by the code change that caused them, and flag the changes nobody asked for. ## Architecture diff --git a/docs/specs/access.md b/docs/specs/access.md index 3213d3a..4b18974 100644 --- a/docs/specs/access.md +++ b/docs/specs/access.md @@ -35,7 +35,7 @@ Every project here is public, and one person writes (rationale). - **Must sign a session cookie with the password, so that changing it signs everyone out, and name it `__Host-` with `HttpOnly`, `Secure`, and `SameSite=Strict`**, for 30 days. - **Must refuse a sign-in from another site, and send a person only to a path on this site after signing in.** - **Must keep `sameOriginWrites` and `pageHeaders` on the review routes.** The sync routes answer only to a bearer token, and never read the cookie. -- **Must use one GitHub App, Selfie Hackathon, with only `statuses: write` and `contents: read`, and no webhook**, to say whether a repository has a commit and to post a commit's status. **Its private key stays in Worker secrets.** +- **Must use one GitHub App, Selfie Hackathon, with only `statuses: write` and `contents: read`, and no webhook**, to say whether a repository has a commit, to post a commit's status, and to read what code changed for `docs/specs/chat.md`. **Its private key stays in Worker secrets.** - **Must refuse a commit GitHub doesn't have**, as `docs/specs/sync.md` → What goes says, and treat any other answer from GitHub as an error. ## Future @@ -100,4 +100,4 @@ GitHub decides. Selfie keeps no member list of its own. A batch isn't uploaded until GitHub has its commit (`docs/specs/sync.md` → What goes), so anonymous viewers see no unpushed work. - **Running an approved render pass.** Decide how a maintainer approves it and how it runs, since it executes the fork's code and needs a token to upload. GitHub's docs don't say whether a fork's `pull_request` batch can get an OIDC token, and `pull_request_target` runs with write access even from a public fork, so both need testing. The fork's code could forge its screenshots, so its batch must stay subject to normal approval. It would also be the one uploader without write access, so it must not reach the project's images through the route that takes a client's hash on trust: it could plant a false picture under the hash of an image that hasn't been rendered yet. -- **Reading code for `change-grouping`.** That scope sends code diffs to a Meta model, which needs more app permissions and means private code leaves GitHub. Decide how a repository owner consents. +- **Reading code for `change-grouping`.** `docs/specs/chat.md` sends a public repository's code diffs to a Meta model, for anyone who asks. For a private repository, that would mean private code leaves GitHub, and only for someone with read access. Decide how a repository owner consents. diff --git a/docs/specs/chat.md b/docs/specs/chat.md new file mode 100644 index 0000000..597f2e1 --- /dev/null +++ b/docs/specs/chat.md @@ -0,0 +1,57 @@ +# Chat + +> Status: built, and deployed at `hackathon.selfie.dev`, on one story at a time. +> +> - See `docs/specs/glossary.md` for story, batch, render, and reference. +> - Owns asking a model about a story's change: what it reads, what it says it read, and what it may do. `docs/specs/cloud.md` owns the Worker it's in, and `docs/specs/access.md` the GitHub App it reads through. + +Source of truth: `ui/src/worker/chat.ts`, `compare` in `ui/src/worker/github.ts`, `ui/src/shared/chat.ts`, `ui/src/client/ChatPanel.tsx`; tests in `ui/test/workerd/cloud.workerd.test.ts`, with stand-ins for GitHub and the model, and stories in `Parts/Chat`. + +## Asking + +`POST /:owner/:repo/api/batches/:batchId/chat`, among the review routes but not on `ReviewSource`, takes `{ story, messages }`, with `messages` as `[{ from: "person" | "model", text }]`, taking turns, starting and ending with the person. + +- **Must answer anyone, signed in or not**, since every project here is public (rationale). +- **Must keep `pageHeaders` and `sameOriginWrites` on it**, as on any review route (`docs/specs/cloud.md` → Pages). +- **Must take at most 10 questions a minute from one address**, by the `CHAT_LIMIT` binding, and answer 429 `too-many-questions` past that (rationale). +- **Must take at most 40 messages, with at most 4,000 characters in a question and 40,000 in an answer, in at most 1 MB**, and answer 400 `bad-request` otherwise. +- **Must offer it only on a hosted story that isn't unchanged.** `selfie ui` has no chat. Any other story is 404 `unknown-story`. + +## What the model reads + +Each question is worked out afresh, and nothing of a conversation is kept here or at Meta (`store: false`). + +- **Must send, in this order, so that the model's cache can match it from one question to the next:** the story, its state, and why it failed if it did; the commits and code; the image last approved, the batch's image, and the stored diff image, as whichever exist, in `data:` URLs; the stored measurements of their differences; and then the conversation. +- **Must read the code since the commit of the batch whose approval made the image the reference**, through `goldenBatchId`. +- **Must read a story with no reference from where its commit left the default branch, or if that's empty, what its commit itself changed.** +- **May keep what it worked out of the code for as long as a Worker lives, for at most 16 pairs of a reference and a commit, least recently used going first.** A story with no reference reads a moving branch through this as well, which at worst shows an earlier merge base. +- **Must leave out lockfiles, generated files, and `dist/`, and what has no text to show, and show the smallest files first, up to 160,000 characters of patches**, so one huge file can't push out many. +- **Must tell the model everything it wasn't shown, by name**, and that GitHub may list no more than 300 files. +- **Must still answer if GitHub can't say**, from the images alone, and tell both the model and the reader so. + +## What it says + +The body is lines of JSON, as `ChatEvent` in `ui/src/shared/chat.ts`: one `code`, then any number of `text`, and then `done` or `failed`. + +- **Must say what code was read**, from which commit to which and how many files, with what was left out, wherever an answer is shown, so nobody takes it for more than it read. +- **Must show a failure before the first piece of an answer as a JSON error**, 502 `model-failed`, and one part way through as `failed`, and never what the model or GitHub said about it, which stays in the log. +- **Must treat a body that ends with neither `done` nor `failed` as a failure**, and go back to before the question, with it ready to ask again, since a conversation goes on only from a whole answer. +- **Must stop reading from the model when whoever asked has gone**, so nothing's paid for that nobody will read. +- **Must log the tokens each question used**, since anyone can spend this key. + +## What it may do + +- **Must use `muse-spark-1.3`, never a `-contributor` model** (rationale). +- **Must keep `MODEL_API_KEY` only in Worker secrets.** +- **Never give the model a tool.** What it reads is a branch's own code and pixels, so an instruction written there must have nothing to act with (rationale). +- **Must show an answer as plain text**, with no links or images, so an answer can't send anything anywhere when it's shown. +- **Never let an answer approve, reject, or block anything.** What decides doesn't know chat exists. + +## Future + +**Scope: change-grouping** (`docs/specs/system.md` → Build order) goes on from here. What's left: + +- Asking about a whole batch rather than a story, for grouping changed stories by the code that caused them, and flagging what nobody asked for. +- Keeping a conversation, or sharing one by a link. +- Private repositories, which wait for **Scope: github-access**, and a decision on consent (`docs/specs/access.md` → Open questions). +- Chat in `selfie ui`, which has Git, and so doesn't need GitHub. diff --git a/docs/specs/chat.rationale.md b/docs/specs/chat.rationale.md new file mode 100644 index 0000000..aedc1fb --- /dev/null +++ b/docs/specs/chat.rationale.md @@ -0,0 +1,15 @@ +# Chat — Rationale + +> Informative companion to [chat.md](chat.md): the evidence behind its rules, keyed by that spec's headings. Nothing here is normative. + +## Asking + +**Anyone can ask.** Ned's call on 2026-11-04, for a service that's only up for the hackathon. Everything it reads is already public, so the only thing at risk is spending. Before this stays up longer, or takes a private repository, this should need a person signed in. + +**Ten questions a minute.** Without a limit, anyone could use this as a free model, since a page supplies its own conversation. The limit is per address and per Cloudflare location, and it's only approximate, so it slows someone down and doesn't cap the bill. A spend limit in Meta's dashboard is what does that. + +## What it may do + +**Standard, not contributor.** Contributor models cost about 8 to 20 times less, and they let Meta train on what's sent. The code here is public, but that's the repository owner's to agree to, and at about $0.04 a question there's nothing to save. They're also limited to 150 requests a minute, against 3,000. + +**No tools.** Anyone who can push a branch to a hosted repository can put words in front of the model, in code or in a rendered story. The instructions tell it to ignore them, which isn't a boundary. Having no tools and no links is, since the worst it can then do is say something wrong to the person reading, who's been told it can be wrong and decides for themselves. diff --git a/docs/specs/cloud.md b/docs/specs/cloud.md index 4946efa..5667d50 100644 --- a/docs/specs/cloud.md +++ b/docs/specs/cloud.md @@ -26,6 +26,7 @@ Source of truth: `ui/src/worker/index.ts`, `ui/src/worker/app.ts`, `ui/wrangler. | `/sign-in` | A password, which sets the session cookie (`docs/specs/access.md` → Who can do what) | | `/api/…` | The sync routes (`docs/specs/sync.md` → Routes) | | `/:owner/:repo/api/…` | The review routes (`docs/specs/ui.md` → Routes), with `pageHeaders` and `sameOriginWrites` | +| `/:owner/:repo/api/batches/:id/chat` | Asking Muse Spark about a story's change (`docs/specs/chat.md`), with those same headers, and only here | | `/:owner/:repo`, `/:owner/:repo/…` | The client's one document, with `` saying which project it's in | - **Must keep every client path and request under that base**, through `BASE` in `ui/src/client/base.ts`, which is empty for a local server, whose paths don't change. @@ -65,5 +66,5 @@ Source of truth: `commitStatusOf` in `ui/src/shared/commit-status.ts`, `publish` - A status on a fork's pull request, which gets no upload token (`docs/specs/access.md` → Pull requests from forks). - A list of a project's commits and batches. - A project's own `hunkMergeDistance`, since nothing here reads its config. Hunks group at the default of 12. -- Rate limits on anonymous requests (`docs/specs/access.md` → Serving images). +- Rate limits on anonymous requests other than chat's (`docs/specs/access.md` → Serving images). - More people, which is **Scope: github-access** (`docs/specs/access.md` → Future). diff --git a/docs/specs/system.md b/docs/specs/system.md index 0beabc5..b827c33 100644 --- a/docs/specs/system.md +++ b/docs/specs/system.md @@ -67,7 +67,7 @@ Selfie is for fully opaque images, and transparency is never allowed: `docs/spec **Scope: trust** — The cloud accepts local results without re-rendering. -**Scope: change-grouping** — Muse Spark reads before/after renders with the code diff and the stated intent, groups changed renders by the code change that caused them, and flags changes nobody asked for, for batch approval or rejection. Needs `cloud-deploy`, and is not ordered against `sync` or `trust`. +**Scope: change-grouping** — Muse Spark reads before/after renders with the code diff and the stated intent, groups changed renders by the code change that caused them, and flags changes nobody asked for, for batch approval or rejection. Asking it about one story's change is built (`docs/specs/chat.md`). Is not ordered against `sync` or `trust`. **Scope: platforms-upgrade** — Possible future: reach each project's existing D1 through Workers for Platforms bindings instead of the REST API, when the shared API rate limit constrains the service. Requires no data migration. diff --git a/docs/specs/ui.md b/docs/specs/ui.md index 04ca052..85d099a 100644 --- a/docs/specs/ui.md +++ b/docs/specs/ui.md @@ -16,6 +16,7 @@ Source of truth: `ui/`. | `ui/src/shared/diff-image.ts`, exported as `selfie-ui/diff-image` | How a diff image draws what didn't change and where something was taken away, for the differ and a page redrawing one | | `ui/src/shared/deltas.ts`, exported as `selfie-ui/deltas` | What a [delta](#screens) is, which to offer, and what redrawing one leaves; tests in `ui/test/deltas.test.ts`, and in `cli/test/deltas.test.ts` against the real differ | | `ui/src/shared/notes.ts`, exported as `selfie-ui/notes` | Shared note validation and open-note state (`docs/specs/feedback.md`) | +| `ui/src/shared/chat.ts` | What crosses between the page and the Worker's chat route (`docs/specs/chat.md`), which isn't on `ReviewSource`, since only the cloud has it, through `createReviewApi`'s extra routes | | `ui/src/shared/renderProgress.ts`, exported as `selfie-ui/render-progress` | A render's estimate and how it's said, for the client and `selfie flakes`; tests in `ui/test/renderProgress.test.ts` | | `selfie-ui/sync`, `selfie-ui/sync-api` | `docs/specs/sync.md` → Routes | | `ui/src/server/app.ts`, exported as `selfie-ui/app` | The routes under `/api`, over a `ReviewSource`, on Hono, and `sameOriginWrites` and `pageHeaders`. It re-exports the wire types | diff --git a/ui/src/client/ChatPanel.stories.tsx b/ui/src/client/ChatPanel.stories.tsx new file mode 100644 index 0000000..a1d3c14 --- /dev/null +++ b/ui/src/client/ChatPanel.stories.tsx @@ -0,0 +1,48 @@ +import type { Meta, StoryObj } from "@storybook/react-vite"; +import { expect, userEvent, within } from "storybook/test"; +import type { ChatUpdate } from "../shared/chat.ts"; +import { RequestError } from "./api.ts"; +import { ChatPanel, type ChatPanelProps } from "./ChatPanel.tsx"; +import { BATCH_ID } from "./stories/fixtures.ts"; + +const meta = { title: "Parts/Chat", component: ChatPanel, args: { batchId: BATCH_ID, story: "components-button--primary" } } satisfies Meta; +export default meta; +type Story = StoryObj; + +const BASE = "05a05880e31243f9906ab509d06acb76de444250"; +const HEAD = "46f7d4e1c0f5a6b7d8e9f0a1b2c3d4e5f6a7b8c9"; +const QUESTION = { from: "person", text: "What code caused this change?" } as const; +const ANSWER = { from: "model", text: "The button got taller. My best guess is ui/src/client/styles.css, where this commit changes padding: 4px 8px to padding: 8px 12px on .selfie-button. I'm inferring that from the diff, and nothing else in it touches buttons.\n\nThe commit message only mentions spacing in the list, so check that this was wanted." } as const; +const ask = (updates: readonly ChatUpdate[]): ChatPanelProps["ask"] => + async (_batchId, _question, onUpdate) => { + for (const update of updates) onUpdate(update); + }; + +export const NothingAskedYet: Story = {}; +export const AnAnswer: Story = { args: { initial: { messages: [QUESTION, ANSWER], code: { base: BASE, head: HEAD, files: 4, leftOut: [] } } } }; +export const SomeCodeLeftOut: Story = { args: { initial: { messages: [QUESTION, ANSWER], code: { base: BASE, head: HEAD, files: 40, leftOut: ["pnpm-lock.yaml", "ui/src/client/stories/images.generated.ts", "branding/mascot.webp", "docs/big.md", "cli/src/huge.ts", "store/src/huge.ts"] } } } }; +export const NoCodeChanged: Story = { args: { initial: { messages: [QUESTION, { from: "model", text: "No code changed between these commits, so this probably came from something unstable in the render." }], code: { base: HEAD, head: HEAD, files: 0, leftOut: [] } } } }; +export const GitHubCouldNotSay: Story = { args: { initial: { messages: [QUESTION, { from: "model", text: "I only have the images. The button is taller, and its text moved down by 4 pixels." }], code: null } } }; +export const CouldNotAnswer: Story = { args: { initial: { messages: [], error: "Muse Spark couldn't answer that. Try again in a moment." } } }; +export const AskingWithAStarter: Story = { + args: { ask: ask([{ type: "code", code: { base: BASE, head: HEAD, files: 4, leftOut: [] } }, { type: "text", text: "The button got " }, { type: "text", text: "taller." }]) }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await userEvent.click(canvas.getByRole("button", { name: "Does this look intended?" })); + await expect(await canvas.findByText("The button got taller.")).toBeVisible(); + await expect(canvas.queryByRole("button", { name: "Does this look intended?" })).toBeNull(); + }, +}; +export const AQuestionThatFails: Story = { + args: { + ask: async () => { + throw new RequestError("too-many-questions", "That's a lot of questions. Wait a minute, and ask again."); + }, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await userEvent.type(canvas.getByRole("textbox"), "Why is this taller?{Enter}"); + await expect(await canvas.findByRole("alert")).toHaveTextContent("a lot of questions"); + await expect(canvas.getByRole("textbox")).toHaveValue("Why is this taller?"); + }, +}; diff --git a/ui/src/client/ChatPanel.tsx b/ui/src/client/ChatPanel.tsx new file mode 100644 index 0000000..4b84adf --- /dev/null +++ b/ui/src/client/ChatPanel.tsx @@ -0,0 +1,138 @@ +// Asking Muse Spark about the change on this page. It's only ever asked, and decides nothing. +import { Button, Stack, Text, Textarea } from "@primer/react"; +import { useEffect, useRef, useState } from "react"; +import { MAX_QUESTION_CHARACTERS, type ChatCode, type ChatMessage, type ChatUpdate } from "../shared/chat.ts"; +import { plural } from "../shared/plural.ts"; +import { askAboutStory } from "./api.ts"; +import { asRequestError } from "./hooks.ts"; +import { Sha } from "./words.tsx"; + +const STARTERS = ["What code caused this change?", "Does this look intended?"]; + +/** What the model was shown, so that nobody takes an answer for more than it read. */ +function CodeShown({ code }: { readonly code: ChatCode | null | undefined }) { + if (code === undefined) return null; + if (code === null) return GitHub couldn't say what code changed, so this comes from the images alone.; + const range = ( + <> + … + + ); + const leftOut = code.leftOut.length === 0 ? "" : `, without the text of ${code.leftOut.length}: ${code.leftOut.slice(0, 5).join(", ")}${code.leftOut.length > 5 ? ", …" : ""}`; + return ( + + {code.files === 0 ? <>No code changed in {range}. : <>Read {plural(code.files, "file")} from {range}{leftOut}.} + + ); +} + +export interface ChatPanelProps { + readonly batchId: string; + readonly story: string; + /** Stories stand in for the server here, and start part way through a conversation. */ + readonly ask?: typeof askAboutStory; + readonly initial?: { readonly messages: readonly ChatMessage[]; readonly code?: ChatCode | null; readonly error?: string }; +} + +export function ChatPanel({ batchId, story, ask = askAboutStory, initial }: ChatPanelProps) { + const [messages, setMessages] = useState(initial?.messages ?? []); + const [code, setCode] = useState(initial?.code); + const [error, setError] = useState(initial?.error ?? null); + const [text, setText] = useState(""); + const [asking, setAsking] = useState(false); + const inFlight = useRef(null); + useEffect(() => () => inFlight.current?.abort(), []); + + const send = async (question: string) => { + if (asking || question.trim() === "") return; + const asked: readonly ChatMessage[] = [...messages, { from: "person", text: question }]; + setMessages(asked); + setText(""); + setError(null); + setAsking(true); + const controller = new AbortController(); + inFlight.current = controller; + let answer = ""; + const onUpdate = (update: ChatUpdate) => { + if (update.type === "code") { + setCode(update.code); + } else { + answer += update.text; + setMessages([...asked, { from: "model", text: answer }]); + } + }; + try { + await ask(batchId, { story, messages: asked }, onUpdate, controller.signal); + } catch (thrown) { + if (controller.signal.aborted) return; + setError(asRequestError(thrown).message); + // A conversation goes on only from a whole answer, so this goes back to before the question, ready to ask again. + setMessages(messages); + setText(question); + } + setAsking(false); + }; + + return ( +
+ {messages.length > 0 && ( +
+ {messages.map(({ from, text: said }, index) => ( + + {from === "person" ? "You: " : "Muse Spark: "} + {said} + + ))} + {asking && messages.at(-1)?.from === "person" && Reading the images and the code…} + +
+ )} + {error !== null && ( + + {error} + + )} +
{ + event.preventDefault(); + void send(text); + }} + > + + +