diff --git a/packages/ui/README.md b/packages/ui/README.md index 9702a4f..b18051e 100644 --- a/packages/ui/README.md +++ b/packages/ui/README.md @@ -40,7 +40,7 @@ if a `node:*` import creeps back onto that path. | `src/lib/guests.ts` | Comments from people who are not members: the Constellation backlink query, the re-validation that makes the index a hint rather than an authority, and the rows the Community section draws. The only module that reads a non-member's repo, and nothing it returns enters the fold. | | `src/lib/userinput.ts` | The Userinput section: defensive parsers for a foreign feedback board, the trust fold over its grants and statuses, and the editable import draft. Authority is honoured only against the version a strongref pins; presentation resolves by URI, and latest-wins is decided by `core`'s comparators rather than the reader's locale. `discoverUserinput` reads only the board — the space's own goals are joined onto the result by `feedbackRows` at the point of drawing, so the network fan-out is a function of the board's address and not of the sync tick. Browser-only, and a member pressing Create is the only way any of it reaches the fold. | | `src/routes/p/[project]/userinput/` | The section itself. A piece of feedback is drawn with the unit row's own furniture — disc, tail, title, drawer — because it is answering the same question every other row answers; the body inside is the stranger's, so it is quoted in `.brief`, verbatim, through no markdown pass at all. The board's address is taken as either its page on userinput.app or the `at://` URI (`feedbackSourceUri` in `@radial/sidecar`, one parser for the form and the CLI). | -| `src/lib/issues.ts` | Issues on a project's tangled repository, offered as goals: the repository's own DID, the appview's issue list, the re-read from each filer's own repo that makes the index a hint, and which issues the space has already taken up. `issueRows` joins a look to the fold at the point of drawing, exactly as `feedbackRows` does, so the network fan-out is a function of the remote and not of the sync tick. Another module reading outside the space's members, and nothing it returns enters the fold either. | +| `src/lib/issues.ts` | Issues on a project's tangled repository, offered as goals: the repository's own DID, the appview's issue list, the re-read from each filer's own repo that makes the index a hint, and which issues the space has already taken up. Every open issue a look can read comes back, imported or not — an imported one is a row like any other — and `taken` only orders how a bounded look spends its budget. `issueRows` joins a look to the fold at the point of drawing, exactly as `feedbackRows` does, so the network fan-out is a function of the remote and not of the sync tick. Another module reading outside the space's members, and nothing it returns enters the fold either. | | `src/routes/p/[project]/tangled/` | The section itself, and the Userinput page's twin down to its markup: one unit row per open issue — disc, tail, the forge's own state as a flat chip, title, drawer — with the issue's body quoted verbatim in `.brief` and never through a markdown pass. Importing writes nothing; it opens the goal composer seeded with the issue's words (`NewGoal.svelte`), which is where a member reads them, edits them, and signs the goal. | | `src/lib/diagnostics.ts` | `index.ignored` and `index.edits`, grouped for display. | | `src/lib/build.ts` | Which copy of the app this tab is running — the constants `scripts/build-stamp.mjs` reads at build time and `vite.config.ts` injects, plus the origin the bundle was built for. Pure, and every input is an argument, so the one runtime value (where the tab is actually served from) is passed in. See *Build info* below. | @@ -153,7 +153,10 @@ Userinput section's twin, and drawn as one — a page of unit rows, each with a issue's own words — because it is the same kind of surface answering the same question about a different stranger's service, and its module (`issues.ts`) follows the same three rules `guests.ts` does. A candidate that somebody has already taken up keeps its row and says so, exactly as a piece of -feedback does; the ones a look never had to fetch are counted beside the section head. +feedback does — it is read from its filer's repo like any other, because the fold holds an imported +issue's URI and nothing else, so a look that skipped it could draw that row only in the tab that did +the importing and only for as long as it stayed open. What `taken` decides is the ORDER a bounded +look spends its budget in: the issues nobody has taken up first. **Nothing there is in the space, and no turn reads any of it.** Ingestion polls member repos for `com.disnetdev.radial.*`, so an `sh.tangled.repo.issue` is in no `RecordStore`, no `materialize()` diff --git a/packages/ui/src/lib/issues.test.ts b/packages/ui/src/lib/issues.test.ts index c775c0c..bf89a62 100644 --- a/packages/ui/src/lib/issues.test.ts +++ b/packages/ui/src/lib/issues.test.ts @@ -226,12 +226,12 @@ describe('discovering candidates', () => { const { fetcher } = appview([{ items: [open('newer'), open('older')] }]) const result = await discoverIssues({ repoDid: REPO, - imported: new Set(), + taken: new Set(), reader: reader([older, newer]), fetcher, }) expect(result.candidates.map((candidate) => candidate.uri)).toEqual([older.uri, newer.uri]) - expect(result).toMatchObject({ imported: 0, failed: 0, truncated: false }) + expect(result).toMatchObject({ failed: 0, truncated: false }) }) it('drops what tangled closed, and anything whose state it did not fold', async () => { @@ -240,7 +240,7 @@ describe('discovering candidates', () => { ]) const result = await discoverIssues({ repoDid: REPO, - imported: new Set(), + taken: new Set(), reader: reader([issue('one')]), fetcher, }) @@ -248,23 +248,42 @@ describe('discovering candidates', () => { expect(result.failed).toBe(0) }) - it('counts what the space already took up, and never fetches it', async () => { + it('reads an issue the space already took up, so a cold page can still draw its row', async () => { + // The regression this exists for: dropping an imported issue here left the fold holding its URI + // and nothing else — no title, no filer, no body — so the badge, the link to the goal and + // "import again" existed only for an import made in this tab, seconds ago. A member arriving + // afterwards saw the issue vanish, which reads as "somebody closed it on the forge". const asked: string[] = [] const { fetcher } = appview([{ items: [open('one'), open('two')] }]) const result = await discoverIssues({ repoDid: REPO, - imported: new Set([uriOf('two')]), + taken: new Set([uriOf('two')]), reader: { getForeignRecord: async (uri: string) => { asked.push(uri) - return issue('one') + return issue(uri.slice(uri.lastIndexOf('/') + 1)) }, }, fetcher, }) - expect(result.imported).toBe(1) - expect(result.candidates.map((candidate) => candidate.uri)).toEqual([uriOf('one')]) - expect(asked).toEqual([uriOf('one')]) + expect(result.candidates.map((candidate) => candidate.uri)).toEqual([uriOf('one'), uriOf('two')]) + expect(asked.sort()).toEqual([uriOf('one'), uriOf('two')]) + }) + + it('spends a bounded budget on the issues nobody has taken up first', async () => { + // Both kinds are rows, but a repository with a backlog longer than this view loads should spend + // what it has on the ones still waiting for somebody. An imported issue is dropped only when + // there is no room left — never on principle. + const { fetcher } = appview([{ items: [open('taken'), open('waiting')] }]) + const result = await discoverIssues({ + repoDid: REPO, + taken: new Set([uriOf('taken')]), + reader: reader([issue('waiting'), issue('taken')]), + fetcher, + limit: 1, + }) + expect(result.candidates.map((candidate) => candidate.uri)).toEqual([uriOf('waiting')]) + expect(result.truncated).toBe(true) }) it('counts a hit it could not read rather than showing the index\'s copy of it', async () => { @@ -274,7 +293,7 @@ describe('discovering candidates', () => { const { fetcher } = appview([{ items: [open('one'), open('gone'), open('moved')] }]) const result = await discoverIssues({ repoDid: REPO, - imported: new Set(), + taken: new Set(), reader: reader([issue('one'), issue('moved', { repo: 'did:plc:elsewhere' })]), fetcher, }) @@ -290,7 +309,7 @@ describe('discovering candidates', () => { ]) const result = await discoverIssues({ repoDid: REPO, - imported: new Set(), + taken: new Set(), reader: reader([issue('one'), issue('two'), issue('three')]), fetcher, pages: 3, @@ -306,7 +325,7 @@ describe('discovering candidates', () => { const { fetcher } = appview([{ items }]) const result = await discoverIssues({ repoDid: REPO, - imported: new Set(), + taken: new Set(), reader: reader(['a', 'b', 'c'].map((rkey) => issue(rkey))), fetcher, limit: 2, diff --git a/packages/ui/src/lib/issues.ts b/packages/ui/src/lib/issues.ts index 56bed08..eb9739d 100644 --- a/packages/ui/src/lib/issues.ts +++ b/packages/ui/src/lib/issues.ts @@ -26,8 +26,10 @@ // record; every candidate is re-read from its author's own PDS and must still say it belongs to // this repository. An index that could decide which repository somebody's words were filed under // would be an index that could put text in front of a reader under false pretences. -// - **Already-imported issues drop out.** The goal's `source.uri` is what says so, which is why it -// is on-protocol: a second member opening this list sees what the first one already took up. +// - **An issue this space took up keeps its row, and says so.** The goal's `source.uri` is what +// says so, which is why it is on-protocol rather than a note in somebody's browser: a second +// member opening this list — on a cold browser, having imported nothing — sees what the first one +// took up, which goal it became, and that importing it again is a decision rather than a mistake. import { GOAL_SOURCE_TANGLED_ISSUE, @@ -295,8 +297,8 @@ export function asIssue( * * Read off the goals rather than kept anywhere: `source` is on-protocol precisely so that the answer * is the same for every member and survives a cold browser. Ended goals count — an issue that was - * imported and then dropped has been decided about, and offering it again would be the list asking - * the space to re-litigate it every time somebody opens the section. + * imported and then dropped has been decided about, and a row that said nothing about it would have + * the next reader re-litigate that decision without knowing there had been one. * * Every goal that named an issue, not the first one: the shape `goalsByOrigin` has in the fold, and * for the same reason. Two members racing on one issue is a fact the row should be able to say out @@ -333,11 +335,13 @@ export const issueRows = ( ): IssueRow[] => found.map((candidate) => ({ ...candidate, imported: taken.get(candidate.uri) ?? [] })) export interface Discovery { - /** Open issues nobody has imported, oldest first — the order a backlog is worked through. */ + /** Every open issue this look could read, oldest first — the order a backlog is worked through. + * + * Including the ones this space has already taken up. They are what `issueRows` badges, links to + * their goals and offers again, and a look is the ONLY thing that can produce them: the fold knows + * an issue's URI and nothing else about it, so a page that dropped them here could draw an imported + * issue only in the seconds after somebody imported it, and never on a cold navigation. */ candidates: IssueCandidate[] - /** How many open issues this space has already taken up as goals. Counted, not listed: they are - * goals now, and the goal list above is where they live. */ - imported: number /** Hits that could not be turned into a candidate — a PDS that was down, a record deleted or * rewritten since Bobbin indexed it, an entry that never named this repository. Surfaced rather * than swallowed, for the reason `discoverGuestComments` surfaces its own. */ @@ -347,16 +351,23 @@ export interface Discovery { } /** - * The open issues on a repository that this space has not already taken up. + * The open issues on a repository, read from the repos of the people who filed them. * - * Order of operations is the point. Bobbin says WHERE issues are and which it folded as open; the - * already-imported ones drop out before anything is fetched, because a repository whose issues are - * mostly imported should cost one appview call and no PDS reads; and only then is each survivor read - * from its author's own repo, which is the copy the reader is shown and the version the goal pins. + * Order of operations is the point. Bobbin says WHERE issues are and which it folded as open; only + * then is each one read from its author's own repo, which is the copy the reader is shown and the + * version a goal pins. + * + * `taken` does not decide WHETHER an issue is read, only in which order the budget is spent on them. + * An earlier draft dropped an already-imported issue here, to make a mostly-imported repository cost + * one appview call and no PDS reads — and that saving was paid for by the row: an issue somebody took + * up last week has a URI in the fold and nothing else, no title, no body and no filer, so a page that + * never fetched it could not draw it at all. The badge, the link to the goal and "import again" would + * then be reachable only in the seconds after an import, in the one browser that made it. The budget + * still goes to what is still waiting, because that is what a backlog is for. */ export async function discoverIssues(input: { repoDid: string - imported: ReadonlySet + taken: ReadonlySet reader: Pick origin?: string fetcher?: Fetcher @@ -367,8 +378,8 @@ export async function discoverIssues(input: { const pages = input.pages ?? ISSUE_PAGES const ceiling = input.limit ?? CANDIDATE_LIMIT const seen = new Set() - const hits: IssueLocator[] = [] - let imported = 0 + const waiting: IssueLocator[] = [] + const already: IssueLocator[] = [] let cursor: string | undefined let truncated = false for (let page = 0; page < pages; page += 1) { @@ -384,18 +395,21 @@ export async function discoverIssues(input: { // recorded, and re-proposing it here would be Radial arguing with it. if (item.state !== 'open' || seen.has(item.uri)) continue seen.add(item.uri) - if (input.imported.has(item.uri)) { - imported += 1 - continue - } - if (hits.length < ceiling) hits.push(item) - else truncated = true + ;(input.taken.has(item.uri) ? already : waiting).push(item) } cursor = result.cursor if (!cursor) break if (page === pages - 1) truncated = true } + // Both, and the ones nobody has taken up first. A repository with more open issues than this view + // loads is one whose backlog belongs on the forge, and the rows worth spending that budget on are + // the ones still waiting for somebody — but an imported issue is dropped only when there is no room + // left, never on principle. + const found = [...waiting, ...already] + if (found.length > ceiling) truncated = true + const hits = found.slice(0, ceiling) + const fetched = await Promise.all( hits.map(async (hit) => { try { @@ -418,7 +432,7 @@ export async function discoverIssues(input: { (left, right) => left.createdAt.localeCompare(right.createdAt) || left.uri.localeCompare(right.uri), ) - return { candidates, imported, failed, truncated } + return { candidates, failed, truncated } } // ── what importing one writes ─────────────────────────────────────────────────────────────────── diff --git a/packages/ui/src/routes/p/[project]/tangled/+page.svelte b/packages/ui/src/routes/p/[project]/tangled/+page.svelte index 6c5c27b..f6f7622 100644 --- a/packages/ui/src/routes/p/[project]/tangled/+page.svelte +++ b/packages/ui/src/routes/p/[project]/tangled/+page.svelte @@ -66,10 +66,9 @@ ) let found = $state(undefined) const rows = $derived(issueRows(taken, found?.candidates ?? [])) - /** Open issues that are goals here: the ones a look never fetched, plus any taken up since. */ - const imported = $derived( - (found?.imported ?? 0) + rows.filter((row) => row.imported.length > 0).length, - ) + /** How many of the rows on screen are goals here — counted off the rows and not off the look, so + * that an issue taken up in this tab a second ago and one taken up last week are the same fact. */ + const imported = $derived(rows.filter((row) => row.imported.length > 0).length) let filers = $state>({}) let loading = $state(false) @@ -129,11 +128,14 @@ } const discovered = await discoverIssues({ repoDid, - // Read under `untrack` for the reason the derivations above are strings: the goals this - // space holds are what makes a look CHEAPER (an issue already taken up costs no PDS read), - // and reading them as a dependency would make every sync tick a second look at a stranger's - // repository — pulling the rows out from under whatever drawer somebody had open. - imported: untrack(() => new Set(taken.keys())), + // What this space has taken up already — which orders the look's budget and nothing else, so + // that a bounded view spends it on issues still waiting for somebody. Read under `untrack` + // for the reason the derivations above are strings: reading the fold as a dependency would + // make every sync tick a second look at a stranger's repository, pulling the rows out from + // under whatever drawer somebody had open. A set that has moved since costs a row nothing — + // every open issue a look could read comes back, and `issueRows` joins the current fold to it + // at the point of drawing. + taken: untrack(() => new Set(taken.keys())), reader, }) if (mine !== pass) return @@ -225,11 +227,12 @@

The fixture has no repository behind it.

{:else if rows.length === 0}
+

{#if loading}Looking at the repository…{:else if error}Nothing could be read from this - repository.{:else if imported > 0}Every open issue on tangled is already a goal here.{:else} - No open issues on tangled. - {/if} + repository.{:else}No open issues on tangled.{/if}

{:else} diff --git a/packages/ui/src/routes/p/[project]/tangled/tangled-page.svelte.test.ts b/packages/ui/src/routes/p/[project]/tangled/tangled-page.svelte.test.ts index 11a7b1e..0cd2ad2 100644 --- a/packages/ui/src/routes/p/[project]/tangled/tangled-page.svelte.test.ts +++ b/packages/ui/src/routes/p/[project]/tangled/tangled-page.svelte.test.ts @@ -37,6 +37,8 @@ const HOSTILE = ' **not bold**' const appview = vi.hoisted(() => vi.fn()) vi.stubGlobal('fetch', appview) +/** Every issue this page read from a filer's own repo, in order. */ +const reads = vi.hoisted(() => [] as string[]) vi.mock('$app/state', () => ({ page: { params: { project: 'widget' } } })) vi.mock('$lib/auth.svelte.js', () => ({ @@ -61,7 +63,7 @@ vi.mock('$lib/session.svelte.js', () => ({ ], }), getForeignRecord: async (uri: string) => ({ - uri, + uri: (reads.push(uri), uri), cid: 'cid-issue', value: { $type: 'sh.tangled.repo.issue', @@ -140,6 +142,7 @@ let component: Record | undefined beforeEach(() => { appview.mockReset() + reads.length = 0 appview.mockResolvedValue( new Response(JSON.stringify({ items: [{ uri: ISSUE, cid: 'cid-issue', state: 'open' }] }), { status: 200, @@ -251,6 +254,32 @@ describe('the tangled section', () => { expect(appview).toHaveBeenCalledTimes(1) }) + // The cold path, and the one a look is the only possible source for. The fold holds an imported + // issue's URI and nothing else — no title, no body, no filer — so a look that skipped an issue + // somebody had already taken up could draw its row only in the tab that did the taking, seconds + // afterwards. To everybody else the issue simply disappeared, which on an issue list reads as + // "somebody closed it on the forge". + it('draws an issue imported before this page was ever opened', async () => { + space = spaceOver([goalFrom(ISSUE)]) + render() + await vi.waitFor(() => expect(rows()).toHaveLength(1)) + + // Read from the filer's own repo like any other row: the badge is the fold's, the words are the + // forge's, and neither is inferred from the goal that was written out of it. + expect(reads).toEqual([ISSUE]) + expect(host.querySelector('.row .badge')?.textContent).toContain('imported') + expect(host.querySelector('.row .sub')?.textContent?.trim()).toBe( + 'repo operations: archive repository', + ) + expect(text()).toContain('1 imported') + + open() + expect(host.querySelector('.drawer .brief')?.textContent).toBe(HOSTILE) + const links = [...host.querySelectorAll('.drawer .kv a')] + expect(links.map((link) => link.textContent?.trim())).toContain('Archive a repository') + expect(text()).toContain('Import again') + }) + // The repository is an appview call plus a read per issue; the space around it is republished on a // ten second timer whether or not a record moved. Only the remote may drive a second look. it('reads the repository once, not once per sync tick', async () => {