Public: anyone can read this; the crew can change it.
A pull's looks say which phone pages scroll sideways, to its crew, without showing the owner's picture #10210
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/9386-a-pull-s-looks-say-which-phone-pages-scr → main 3 commits fork 1 thread · 1 unresolved
5 files
changed +166
−4
docs/features/pull-looks.md +7 −1
| @@ -9,7 +9,13 @@ One list, one rule (`pullLooks` in `src/looks/readable.ts`), three doors: | ||
| 9 | 9 | |
| 10 | 10 | - **API** — `GET /api/v1/repos/:owner/:name/pulls/:number/looks` answers |
| 11 | 11 | `{ items: [{ id, width, says, by, commit, url, at }] }`, newest first. `url` |
| 12 | is the same one the POST answers. | |
| 12 | is the same one the POST answers. Beside `items` comes `fit`, for the pull's | |
| 13 | latest commit: one `{ page, width, scrollWidth, innerWidth, screens, | |
| 14 | sideways }` per page and width that has a stored size (`screens` = | |
| 15 | scrollHeight / innerHeight to one decimal, `sideways` = scrollWidth > | |
| 16 | innerWidth). Numbers only, no url, owner-taken looks included, for any | |
| 17 | reader of the pull; a sealed pull's `fit` is the writers'. Looks without a | |
| 18 | size are absent. | |
| 13 | 19 | - **MCP** — the `pull` tool carries `looks`, the same items (no new tool). |
| 14 | 20 | - **Terminal** — `I` on a pull opens its looks; the pull screen's conversation also lists them under a `Looks` heading (none, no heading), each with its width, who, the line it says and the absolute URL on its own line. It is the same `pullLooks` read as the API and MCP, so a reader sees only what they may fetch. |
| 15 | 21 |
src/http/api.ts +3 −2
| @@ -101,7 +101,7 @@ import { | ||
| 101 | 101 | import { landSlice } from "../actions/pull_slices.ts"; |
| 102 | 102 | import { attachLook } from "../actions/attach_look.ts"; |
| 103 | 103 | import { lookUrl } from "../core/looks.ts"; |
| 104 | import { pullLooks } from "../looks/readable.ts"; | |
| 104 | import { pullFit, pullLooks } from "../looks/readable.ts"; | |
| 105 | 105 | import { holdPull, releasePull } from "../actions/holds.ts"; |
| 106 | 106 | import { getHold, type Hold } from "../core/holds.ts"; |
| 107 | 107 | import { watchMilestone } from "../actions/watches.ts"; |
| @@ -2895,7 +2895,8 @@ export function apiRoutes(router: Router<Handler>): void { | ||
| 2895 | 2895 | const found = loadPull(ctx); |
| 2896 | 2896 | if ("error" in found) return found.error; |
| 2897 | 2897 | const items = pullLooks(ctx.app, found.repo, found.pull, ctx.auth.user); |
| 2898 | return json({ items }); | |
| 2898 | const fit = pullFit(ctx.app, found.repo, found.pull, ctx.auth.user); | |
| 2899 | return json({ items, fit }); | |
| 2899 | 2900 | }, |
| 2900 | 2901 | ); |
| 2901 | 2902 |
src/looks/readable.ts +50
| @@ -95,3 +95,53 @@ export function pullPairsLine( | ||
| 95 | 95 | const head = mine(previewEnv(pull.number)).filter((l) => l.theme === "light"); |
| 96 | 96 | return pairsLine(pairCounts(head, mine("production"))); |
| 97 | 97 | } |
| 98 | ||
| 99 | /** One page at one width: how it sat in the window it was pictured in. */ | |
| 100 | export interface PullFit { | |
| 101 | readonly page: string; | |
| 102 | readonly width: string; | |
| 103 | readonly scrollWidth: number; | |
| 104 | readonly innerWidth: number; | |
| 105 | readonly screens: number; | |
| 106 | readonly sideways: boolean; | |
| 107 | } | |
| 108 | ||
| 109 | /** | |
| 110 | * Which pages scroll sideways, from the sizes stored on a pull's looks of its | |
| 111 | * latest commit: numbers only, never a url or a picture, so the owner's own | |
| 112 | * looks count too. A sealed pull's is the writers'. Looks without a size are | |
| 113 | * left out. One query; no browser. | |
| 114 | */ | |
| 115 | export function pullFit( | |
| 116 | app: App, | |
| 117 | repo: Repo, | |
| 118 | pull: Pull, | |
| 119 | user: User | undefined, | |
| 120 | ): PullFit[] { | |
| 121 | const record = getAppByRepo(app.db, repo.id); | |
| 122 | if (!record) return []; | |
| 123 | const env = previewEnv(pull.number); | |
| 124 | if (sealedPreview(app.db, repo.id, env) && !canSeeOwnerFacts(app.db, repo, user)) | |
| 125 | return []; | |
| 126 | const looks = looksForEnv(app.db, record.id, env).filter((l) => l.error === null); | |
| 127 | const latest = looks[0]?.commitOid; | |
| 128 | const out: PullFit[] = []; | |
| 129 | // Newest first: the first look with a size per page and width is the one kept. | |
| 130 | const seen = new Set<string>(); | |
| 131 | for (const l of looks) { | |
| 132 | const s = l.size; | |
| 133 | if (l.commitOid !== latest || !s || s.innerHeight <= 0) continue; | |
| 134 | const key = `${l.page} ${l.kind}`; | |
| 135 | if (seen.has(key)) continue; | |
| 136 | seen.add(key); | |
| 137 | out.push({ | |
| 138 | page: l.page, | |
| 139 | width: l.kind, | |
| 140 | scrollWidth: s.scrollWidth, | |
| 141 | innerWidth: s.innerWidth, | |
| 142 | screens: Math.round((s.scrollHeight / s.innerHeight) * 10) / 10, | |
| 143 | sideways: s.scrollWidth > s.innerWidth, | |
| 144 | }); | |
| 145 | } | |
| 146 | return out; | |
| 147 | } |
test/pull-looks-fit.test.ts added +105
| @@ -0,0 +1,105 @@ | ||
| 1 | /** | |
| 2 | * `fit` beside `items` on GET …/pulls/:number/looks: sizes only, owner looks | |
| 3 | * included, none without a size, and a sealed pull's is the writers'. | |
| 4 | */ | |
| 5 | import { afterAll, beforeAll, expect, test } from "bun:test"; | |
| 6 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 7 | import { tmpdir } from "node:os"; | |
| 8 | import { join } from "node:path"; | |
| 9 | import { openPull } from "../src/actions/pulls.ts"; | |
| 10 | import { commitToBranch, createRepo } from "../src/actions/repos.ts"; | |
| 11 | import { createApp, type App } from "../src/app.ts"; | |
| 12 | import { previewEnv } from "../src/apps/data.ts"; | |
| 13 | import { createApp as insertApp } from "../src/core/apps.ts"; | |
| 14 | import { recordLook } from "../src/core/looks.ts"; | |
| 15 | import type { Pull } from "../src/core/pulls.ts"; | |
| 16 | import type { Repo } from "../src/core/repos.ts"; | |
| 17 | import { createToken } from "../src/core/tokens.ts"; | |
| 18 | import { createUser, type User } from "../src/core/users.ts"; | |
| 19 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 20 | import { pullFit } from "../src/looks/readable.ts"; | |
| 21 | import { quietLogs } from "./helpers/globals.ts"; | |
| 22 | ||
| 23 | quietLogs(); | |
| 24 | ||
| 25 | let dataDir: string; | |
| 26 | let app: App; | |
| 27 | let servers: Servers; | |
| 28 | let alice: User; | |
| 29 | let bob: User; | |
| 30 | let bobToken: string; | |
| 31 | let appId: string; | |
| 32 | let pull: Pull; | |
| 33 | let repo: Repo; | |
| 34 | ||
| 35 | const size = (scrollWidth: number) => ({ scrollWidth, scrollHeight: 1700, innerWidth: 390, innerHeight: 844 }); | |
| 36 | ||
| 37 | beforeAll(async () => { | |
| 38 | dataDir = await mkdtemp(join(tmpdir(), "forge-fit-")); | |
| 39 | app = createApp({ dataDir, port: 0 }); | |
| 40 | const person = async (username: string) => { | |
| 41 | const made = await createUser(app.db, { | |
| 42 | origin: "seed", trusted: true, username, email: `${username}@forge.local`, | |
| 43 | password: "a-good-passphrase", adminIfFirst: username === "alice", | |
| 44 | }); | |
| 45 | if (!made.ok) throw new Error(made.error); | |
| 46 | return made.value; | |
| 47 | }; | |
| 48 | alice = await person("alice"); | |
| 49 | bob = await person("bob"); | |
| 50 | const made = await createRepo(app, alice, { name: "garden", visibility: "public" }); | |
| 51 | if (!made.ok) throw new Error(String(made.error)); | |
| 52 | repo = made.value; | |
| 53 | await commitToBranch(app, alice, repo, { message: "seed", files: [{ path: "README.md", content: "# g\n" }] }); | |
| 54 | await commitToBranch(app, alice, repo, { branch: "one", base: "main", message: "one", files: [{ path: "a.md", content: "x\n" }] }); | |
| 55 | const opened = await openPull(app, alice, repo, { title: "One", body: "", baseBranch: "main", headBranch: "one" }); | |
| 56 | if (!opened.ok) throw new Error(String(opened.error)); | |
| 57 | pull = opened.value; | |
| 58 | const inserted = insertApp(app.db, { repoId: repo.id, slug: "garden", access: "public" }); | |
| 59 | if (!inserted.ok) throw new Error(inserted.error); | |
| 60 | appId = inserted.value.id; | |
| 61 | const base = { appId, env: previewEnv(pull.number), commitOid: "a".repeat(40), path: "looks/x.png" }; | |
| 62 | recordLook(app.db, { ...base, kind: "phone", takenAs: "owner", size: size(520) } as Parameters<typeof recordLook>[1]); | |
| 63 | recordLook(app.db, { ...base, kind: "desktop", takenAs: "owner" } as Parameters<typeof recordLook>[1]); | |
| 64 | servers = await serve(app); | |
| 65 | bobToken = createToken(app.db, bob.id, "t", "rw").token; | |
| 66 | }, 30_000); | |
| 67 | ||
| 68 | afterAll(async () => { | |
| 69 | servers.stop(); | |
| 70 | app.close(); | |
| 71 | await rm(dataDir, { recursive: true, force: true }); | |
| 72 | }); | |
| 73 | ||
| 74 | const fit = async () => { | |
| 75 | const res = await fetch(`${servers.url}/api/v1/repos/alice/garden/pulls/${pull.number}/looks`, { | |
| 76 | headers: { authorization: `Bearer ${bobToken}` }, | |
| 77 | }); | |
| 78 | return (await res.json()) as { items: unknown[]; fit: Record<string, unknown>[] }; | |
| 79 | }; | |
| 80 | ||
| 81 | test("an owner-taken sideways phone look is in fit, numbers only, and a look without a size is absent", async () => { | |
| 82 | const body = await fit(); | |
| 83 | expect(body.items).toEqual([]); | |
| 84 | expect(body.fit).toEqual([ | |
| 85 | { page: "/", width: "phone", scrollWidth: 520, innerWidth: 390, screens: 2, sideways: true }, | |
| 86 | ]); | |
| 87 | }); | |
| 88 | ||
| 89 | test("a sealed pull's fit is the writers'", () => { | |
| 90 | app.db.query("UPDATE issue SET sealed = 1 WHERE repo_id = ?1 AND number = ?2").run(repo.id, pull.number); | |
| 91 | expect(pullFit(app, repo, pull, bob)).toEqual([]); | |
| 92 | expect(pullFit(app, repo, pull, alice)).toHaveLength(1); | |
| 93 | }); | |
| 94 | ||
| 95 | test("a page retaken on the latest commit gives one, the newest, entry; a fitting page is not sideways", () => { | |
| 96 | app.db.query("UPDATE issue SET sealed = 0 WHERE repo_id = ?1 AND number = ?2").run(repo.id, pull.number); | |
| 97 | const oid = "b".repeat(40); | |
| 98 | const base = { appId, env: previewEnv(pull.number), commitOid: oid, path: "looks/y.png", takenAs: "nobody", page: "/about", kind: "phone" }; | |
| 99 | recordLook(app.db, { ...base, size: size(900) } as Parameters<typeof recordLook>[1]); | |
| 100 | recordLook(app.db, { ...base, size: { scrollWidth: 390, scrollHeight: 844, innerWidth: 390, innerHeight: 844 } } as Parameters<typeof recordLook>[1]); | |
| 101 | const about = pullFit(app, repo, pull, alice).filter((f) => f.page === "/about"); | |
| 102 | expect(about).toEqual([ | |
| 103 | { page: "/about", width: "phone", scrollWidth: 390, innerWidth: 390, screens: 1, sideways: false }, | |
| 104 | ]); | |
| 105 | }); |
test/pull-looks.test.ts +1 −1
| @@ -129,7 +129,7 @@ const record = (name: string, n: number, extra: Record<string, unknown>) => | ||
| 129 | 129 | test("a pull with no app answers an empty list", async () => { |
| 130 | 130 | const res = await get("bare", tokens["alice"]!); |
| 131 | 131 | expect(res.status).toBe(200); |
| 132 | expect(await res.json()).toEqual({ items: [] }); | |
| 132 | expect(await res.json()).toEqual({ items: [], fit: [] }); | |
| 133 | 133 | }); |
| 134 | 134 | |
| 135 | 135 | test("a nonexistent pull number is a 404", async () => { |
Dedupe by
${l.page} ${l.kind}before pushing (looks are newest first, so keep the first). Right now every retake of the latest commit adds a duplicate entry.