From a055e93e94d6c7f0aaa0178cb1303973bdc0bed1 Mon Sep 17 00:00:00 2001 From: Jorge Rivera Date: Thu, 29 Oct 2026 16:38:23 +0100 Subject: [PATCH 1/2] fix: keep crawlers out of event counts, keep preview cards, and file approved planner decisions track drops bot requests, page openGraph keeps the site name and root card, /data share and spec URLs send X-Robots-Tag, approved choices file planner decisions by choice_id, a refused chip refunds the guest prompt, shared wordings are numbered in the entity menu, and a stale switch without a current card ends read-only. --- next.config.ts | 6 ++- .../(studio)/insights/gallery/[slug]/page.tsx | 7 +-- src/app/layout.tsx | 3 +- .../organisms/DataSearch/SearchSentence.tsx | 9 +++- src/components/organisms/Studio/Studio.tsx | 20 +++++--- .../organisms/Studio/hooks/useChat.ts | 4 +- src/lib/bot.ts | 11 +++++ src/lib/crawl.ts | 48 +++++++++++++++---- src/lib/events/track.ts | 10 +++- src/lib/proxy/requestLog.ts | 8 +--- src/lib/search/parsedEdit.ts | 18 +++++++ src/lib/search/test/parsedEdit.test.ts | 10 ++++ src/lib/site.ts | 30 ++++++++++++ src/lib/studio/chips.ts | 16 ++++--- src/lib/studio/plan.ts | 21 ++++---- src/lib/studio/test/chips.test.ts | 3 +- .../test/fixtures/event-16-plan-approved.json | 37 ++++++++++++++ src/lib/studio/test/plan.test.ts | 44 +++++++++++++++-- src/lib/studio/test/transcript.test.ts | 10 +++- src/lib/studio/transcript.ts | 5 ++ src/lib/studio/wire.ts | 9 ++-- src/lib/test/bot.test.ts | 33 +++++++++++++ src/lib/test/crawl.test.ts | 46 +++++++++++++++--- 23 files changed, 346 insertions(+), 62 deletions(-) create mode 100644 src/lib/bot.ts create mode 100644 src/lib/studio/test/fixtures/event-16-plan-approved.json create mode 100644 src/lib/test/bot.test.ts diff --git a/next.config.ts b/next.config.ts index 5128860..cf681aa 100644 --- a/next.config.ts +++ b/next.config.ts @@ -2,6 +2,7 @@ import { withSentryConfig } from '@sentry/nextjs/config'; import path from 'path'; import type { NextConfig } from 'next'; +import gallery from './src/content/gallery/index.ts'; import { noindexHeaders } from './src/lib/crawl.ts'; const nextConfig: NextConfig = { @@ -21,9 +22,10 @@ const nextConfig: NextConfig = { .map((origin) => origin.trim()) .filter(Boolean), // Share and embed links, the Studio, history and sign-in say noindex on - // every response, redirects and errors included (src/lib/crawl.ts). + // every response, redirects and errors included, and so do the /data + // links kept out of search (src/lib/crawl.ts). async headers() { - return noindexHeaders(); + return noindexHeaders(gallery); }, // A docs page's raw markdown sits beside it: /docs/.md serves the // same file the docs corpus was built from (src/app/docs-md). diff --git a/src/app/(studio)/insights/gallery/[slug]/page.tsx b/src/app/(studio)/insights/gallery/[slug]/page.tsx index 465ce77..e07e032 100644 --- a/src/app/(studio)/insights/gallery/[slug]/page.tsx +++ b/src/app/(studio)/insights/gallery/[slug]/page.tsx @@ -5,6 +5,7 @@ import { Suspense } from 'react'; import { GALLERY_ITEMS, galleryItem } from '@/components/organisms/Gallery/galleryData'; import Studio from '@/components/organisms/Studio/Studio'; import { gallerySession } from '@/lib/gallery/sessions'; +import { pageOpenGraph } from '@/lib/site'; type GalleryItemPageProps = { params: Promise<{ slug: string }> }; @@ -26,11 +27,11 @@ export const generateMetadata = async ({ params }: GalleryItemPageProps): Promis title: `${item.title} — DFO Gallery`, description: item.description, robots: { index: true, follow: true }, - openGraph: { - type: 'article', + openGraph: pageOpenGraph({ title: item.title, description: item.description, - }, + ownCard: true, + }), }; }; diff --git a/src/app/layout.tsx b/src/app/layout.tsx index c689bbf..f6a0aaa 100644 --- a/src/app/layout.tsx +++ b/src/app/layout.tsx @@ -3,7 +3,7 @@ import Script from 'next/script'; import GoogleAnalytics from '@/components/atoms/GoogleAnalytics/GoogleAnalytics'; import { EMBED_PREFIX } from '@/lib/embed'; -import { SITE_ORIGIN } from '@/lib/site'; +import { SITE_NAME, SITE_ORIGIN } from '@/lib/site'; import '@fontsource/outfit/300.css'; import '@fontsource/outfit/400.css'; @@ -16,7 +16,6 @@ import '@fontsource/dm-mono/500.css'; import '@/styles/main.scss'; -const SITE_NAME = 'Development Finance Observatory'; const SITE_DESCRIPTION = 'The Development Finance Observatory integrates aid, debt, climate and health finance, domestic budgets, and remittances into a single open platform, where every number is traceable to its source.'; diff --git a/src/components/organisms/DataSearch/SearchSentence.tsx b/src/components/organisms/DataSearch/SearchSentence.tsx index e27de33..952444c 100644 --- a/src/components/organisms/DataSearch/SearchSentence.tsx +++ b/src/components/organisms/DataSearch/SearchSentence.tsx @@ -92,6 +92,7 @@ import { } from '@/lib/search/measureMenu'; import { entityChoiceGroups, + entityChoiceLabels, entityChoicePart, offersAmbiguousPicker, resolveTypedEdit, @@ -1901,6 +1902,8 @@ export const SearchSentenceHeading = ({ } if (!wordingParts.has(partIndex)) { + const labels = entityChoiceLabels(groups); + return withMarker( - {groups.flatMap((group) => + {groups.flatMap((group, groupIndex) => group.options.map((option, optionIndex) => ( {/* Two ambiguous wordings share one menu, each row naming its own. */} - 1 ? group.text : null}> + 1 ? labels[groupIndex] : null} + > {option.label} diff --git a/src/components/organisms/Studio/Studio.tsx b/src/components/organisms/Studio/Studio.tsx index b8abae6..53f7a33 100644 --- a/src/components/organisms/Studio/Studio.tsx +++ b/src/components/organisms/Studio/Studio.tsx @@ -42,7 +42,7 @@ import { logRefusedDescriptions } from '@/lib/studio/infoPanel'; import { GUEST_PROMPTS, promptEvent, type PromptIntent } from '@/lib/studio/promptAllowance'; import { takeSeeds } from '@/lib/studio/seedStore'; import { deriveTheme, themeVars } from '@/lib/studio/theme'; -import { isPrompt } from '@/lib/studio/transcript'; +import { isPrompt, isRefundedPrompt } from '@/lib/studio/transcript'; import type { Seed } from '@/lib/studio/wire'; import { pushConversation } from '@/lib/chat/historyClient'; import { uuid } from '@/lib/uuid'; @@ -165,20 +165,26 @@ const Studio = ({ const guestUsed = useSyncExternalStore(subscribeGuest, readGuestUsed, () => 0); // A prompt counts once it is actually sent (a queued one can still be // removed): the count follows the user turns in the transcript, so the - // queue's own drain is covered too. Only growth counts — New Chat empties - // the transcript without giving prompts back. + // queue's own drain is covered too. A prompt the harness refused before + // its turn started comes back, as the server's meter gives it back. Only + // growth counts, of either: New Chat empties the transcript without + // giving prompts back. const sentCount = conversation.messages.filter(isPrompt).length; + const refundedCount = conversation.messages.filter(isRefundedPrompt).length; const sentRef = useRef(sentCount); + const refundedRef = useRef(refundedCount); useEffect(() => { - const delta = sentCount - sentRef.current; + const spent = Math.max(0, sentCount - sentRef.current); + const refunded = Math.max(0, refundedCount - refundedRef.current); sentRef.current = sentCount; + refundedRef.current = refundedCount; - if (guest && delta > 0) { - writeGuestUsed(readGuestUsed() + delta); + if (guest && spent !== refunded) { + writeGuestUsed(Math.max(0, readGuestUsed() + spent - refunded)); } - }, [guest, sentCount]); + }, [guest, sentCount, refundedCount]); // Queued prompts hold their place in the limit until sent or removed. const walled = guest && guestUsed + conversation.queued.length >= GUEST_PROMPTS; diff --git a/src/components/organisms/Studio/hooks/useChat.ts b/src/components/organisms/Studio/hooks/useChat.ts index 8985af9..92f21bd 100644 --- a/src/components/organisms/Studio/hooks/useChat.ts +++ b/src/components/organisms/Studio/hooks/useChat.ts @@ -88,12 +88,14 @@ import type { CardRef, Widget } from '@/lib/chat/widgets'; // so the transcript shows what the question was about. `answer` marks the // user's answer to a paused turn (a clarify choice or text, a plan // amendment): shown like any user message, but part of the turn it answers. +// `refunded` marks a prompt the harness refused before its turn started (a +// chip that no longer applies), which the site's prompt meter gave back. // A bot turn's `followups` are prompts a click sends as the next message, // and its `chips` switch a scope choice of the answer's readings. `plan` is // the plan the user approved for this answer, saved with the transcript so // a reopened conversation can show it. export type ChatMessage = - | { role: 'user'; text: string; card?: { title: string }; answer?: true } + | { role: 'user'; text: string; card?: { title: string }; answer?: true; refunded?: true } | { role: 'bot'; text: string; diff --git a/src/lib/bot.ts b/src/lib/bot.ts new file mode 100644 index 0000000..79fd5e2 --- /dev/null +++ b/src/lib/bot.ts @@ -0,0 +1,11 @@ +import { isbot } from 'isbot'; + +/** A crawler, link-preview fetcher or script, by its user agent. A request + * with no user agent comes from a script, so it counts as a bot. */ +export const isBotAgent = (userAgent: string | null | undefined): boolean => + !userAgent || isbot(userAgent); + +/** Whether an event's request came from a bot. An event with no request + * behind it (null headers) is the server's own, never a bot's. */ +export const isBotRequest = (headers: Headers | null): boolean => + headers !== null && isBotAgent(headers.get('user-agent')); diff --git a/src/lib/crawl.ts b/src/lib/crawl.ts index 731c922..bf02af1 100644 --- a/src/lib/crawl.ts +++ b/src/lib/crawl.ts @@ -1,13 +1,15 @@ /** * What search engines may index. A crawler reads `noindex` only on a URL * robots.txt lets it fetch, so these routes stay open to crawlers (app/robots.ts) - * and say `noindex` themselves. No runtime imports, so next.config.ts and - * `node --test` can load it. + * and say `noindex` themselves. Its one runtime import, lib/site.ts, has + * none of its own, so next.config.ts and `node --test` can load it. */ -import type { Metadata } from 'next'; +import type { Metadata, NextConfig } from 'next'; import type { GalleryItem } from '@/components/organisms/Gallery/galleryData'; +import { pageOpenGraph } from './site.ts'; + /** * The routes kept out of search results, each with everything under it: * share and embed links are unlisted by design (anyone with the link can @@ -16,16 +18,44 @@ import type { GalleryItem } from '@/components/organisms/Gallery/galleryData'; */ export const NOINDEX_ROUTES = ['/s', '/embed', '/studio', '/history', '/login', '/register']; +type HeaderRule = Awaited>>[number]; + +const NOINDEX_HEADER = [{ key: 'X-Robots-Tag', value: 'noindex, nofollow' }]; + +/** A regular expression matching exactly the given values. */ +const anyOf = (values: readonly string[]): string => + `(?:${values.map((value) => value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')).join('|')})`; + /** * next.config.ts `headers()`: `X-Robots-Tag` on every response from a * noindex route, so its redirects, 404s and error pages say noindex too, * and a page whose metadata streams into `` says it in the headers. + * /data gets it for the URLs dataPageRobots keeps out of search: any + * version link (?spec=) and any search share (?share=) no gallery item + * opens. */ -export const noindexHeaders = () => - NOINDEX_ROUTES.map((route) => ({ - source: `${route}/:path*`, - headers: [{ key: 'X-Robots-Tag', value: 'noindex, nofollow' }], - })); +export const noindexHeaders = (gallery: readonly GalleryItem[]): HeaderRule[] => { + const gallerySearches = gallery.flatMap((item) => + item.kind === 'dataset' ? [item.sharedFrom] : [], + ); + + return [ + ...NOINDEX_ROUTES.map((route) => ({ source: `${route}/:path*`, headers: NOINDEX_HEADER })), + { + source: '/data', + has: [{ type: 'query', key: 'spec' }], + headers: NOINDEX_HEADER, + }, + { + source: '/data', + has: [{ type: 'query', key: 'share' }], + ...(gallerySearches.length > 0 && { + missing: [{ type: 'query', key: 'share', value: anyOf(gallerySearches) }], + }), + headers: NOINDEX_HEADER, + }, + ]; +}; const NOINDEX: Metadata['robots'] = { index: false, follow: false }; @@ -84,7 +114,7 @@ export const dataPageMetadata = ( return { title: `${item.title} — DFO Gallery`, description: item.description, - openGraph: { type: 'article', title: item.title, description: item.description }, + openGraph: pageOpenGraph({ title: item.title, description: item.description }), }; } diff --git a/src/lib/events/track.ts b/src/lib/events/track.ts index bfb6e7e..f21bb72 100644 --- a/src/lib/events/track.ts +++ b/src/lib/events/track.ts @@ -1,5 +1,6 @@ import 'server-only'; +import { isBotRequest } from '@/lib/bot'; import { db } from '@/lib/db'; import { event } from '../../../db/schema'; import { logEvent } from './eventLog'; @@ -77,9 +78,16 @@ export type Who = { * Record one event. Fire-and-forget: analytics never cost a request, so a * failed insert or forward is logged and swallowed, and each sink fails * independently of the others. The log line goes first and cannot throw, so - * Cloud Logging has the event whether the insert or the forward fails. + * Cloud Logging has the event whether the insert or the forward fails. An + * event whose request came from a crawler, link-preview fetcher or script + * (isBotRequest) is not recorded anywhere, since no stored row or log line + * keeps the user agent to tell it from a person's afterwards. */ export const track = async (name: EventName, props: EventProps, who: Who) => { + if (isBotRequest(who.headers)) { + return; + } + logEvent(name, props, who); try { diff --git a/src/lib/proxy/requestLog.ts b/src/lib/proxy/requestLog.ts index 78d11d0..23578fe 100644 --- a/src/lib/proxy/requestLog.ts +++ b/src/lib/proxy/requestLog.ts @@ -1,8 +1,7 @@ import { subscribe, unsubscribe } from 'node:diagnostics_channel'; import { IncomingMessage, ServerResponse } from 'node:http'; -import { isbot } from 'isbot'; - +import { isBotAgent } from '../bot.ts'; import { requestCaller } from './caller.ts'; /** @@ -95,9 +94,6 @@ const classifyRequest = ( return { kind: prerender || !purpose.includes('prefetch') ? 'page' : 'prefetch', prerender }; }; -/** A request with no user agent comes from a script, so it counts as a bot. */ -const isBot = (userAgent: string | null): boolean => !userAgent || isbot(userAgent); - /** The log entry for a logged request. */ export const requestLogEntry = ( { path, method, headers }: RequestMetadata, @@ -116,7 +112,7 @@ export const requestLogEntry = ( kind, status, finished, - bot: isBot(headers.get('user-agent')), + bot: isBotAgent(headers.get('user-agent')), prerender, }; }; diff --git a/src/lib/search/parsedEdit.ts b/src/lib/search/parsedEdit.ts index 24a3200..f4ff77c 100644 --- a/src/lib/search/parsedEdit.ts +++ b/src/lib/search/parsedEdit.ts @@ -137,6 +137,24 @@ export function entityChoiceGroups(noData: NoData | null | undefined): EntityCho return [...groups.values()]; } +/** + * What each group's rows say in a menu that merges every group: the + * wording, numbered in `entity_choice` order where two groups share it + * ("aid from Congo to Congo" gives "Congo (1)", "Congo (2)"). + */ +export function entityChoiceLabels(groups: readonly EntityChoiceGroup[]): string[] { + return groups.map(({ key, text }) => { + const alike = groups.filter((group) => group.text === text); + + if (alike.length === 1) return text; + + // Only entity_choice keys (numbers) can share a wording. + const place = alike.filter((group) => group.key < key).length + 1; + + return `${text} (${place})`; + }); +} + /** The part an entity choice posts: its option's one-part request, * unchanged. Null for an option carrying no part. */ export const entityChoicePart = (option: NoDataOption): ParsedPart | null => diff --git a/src/lib/search/test/parsedEdit.test.ts b/src/lib/search/test/parsedEdit.test.ts index fb76c34..252a78c 100644 --- a/src/lib/search/test/parsedEdit.test.ts +++ b/src/lib/search/test/parsedEdit.test.ts @@ -4,6 +4,7 @@ import test from 'node:test'; import { applyRestatementEdit, entityChoiceGroups, + entityChoiceLabels, entityChoicePart, offersAmbiguousPicker, resolveTypedEdit, @@ -289,6 +290,15 @@ test('entityChoiceGroups keeps one group per entity_choice, two written alike ap ); }); +test('entityChoiceLabels numbers groups that share a wording, in entity_choice order', () => { + const groups = entityChoiceGroups(congoToCongoNoData); + + assert.deepEqual(entityChoiceLabels(groups), ['Congo (1)', 'Congo (2)']); + assert.deepEqual(entityChoiceLabels([...groups].reverse()), ['Congo (2)', 'Congo (1)']); + // A wording no other group shares keeps its own text. + assert.deepEqual(entityChoiceLabels(entityChoiceGroups(odaToCongoNoData)), ['Congo']); +}); + // --------------------------------------------------------------------------- // offersAmbiguousPicker: SearchCards and studioHandoffParts' shared gate for // whether a no_data part gets the "Choose a Match" picker rather than the diff --git a/src/lib/site.ts b/src/lib/site.ts index fef65e7..961f5bc 100644 --- a/src/lib/site.ts +++ b/src/lib/site.ts @@ -1,5 +1,35 @@ +import type { Metadata } from 'next'; + /** * The production origin. Dev and production build the same metadata, link * previews and sitemap, so no build-time variable sets it. */ export const SITE_ORIGIN = 'https://developmentfinance.org'; + +export const SITE_NAME = 'Development Finance Observatory'; + +/** The site's own link-preview card, drawn by app/opengraph-image.tsx. */ +export const SITE_CARD = '/opengraph-image'; + +/** + * The Open Graph tags of a page that names itself. Next replaces a + * segment's `openGraph` whole, so a page that sets one would drop the root + * layout's site name and card; these keep both. A route with an + * `opengraph-image` file of its own passes `ownCard`, since Next reads the + * file only when the page's `openGraph` has no `images`. + */ +export const pageOpenGraph = ({ + title, + description, + ownCard = false, +}: { + title: string; + description: string; + ownCard?: boolean; +}): NonNullable => ({ + type: 'article', + siteName: SITE_NAME, + title, + description, + ...(!ownCard && { images: SITE_CARD }), +}); diff --git a/src/lib/studio/chips.ts b/src/lib/studio/chips.ts index 9585cc9..7839028 100644 --- a/src/lib/studio/chips.ts +++ b/src/lib/studio/chips.ts @@ -65,10 +65,12 @@ export const chipRefusalDetail = (refusal: StreamRefusal): string | null => : null; /** - * The transcript after the harness refused `chip`: the reply the click - * opened says why, and carries the answer's other chips and follow-ups, so - * the user can still pick one. `messages` ends with the user's chip label - * and its empty reply; the answer that offered the chip comes before them. + * The transcript after the harness refused `chip`: the chip's label is + * marked refunded, since the refused turn never started and the site's + * prompt meter gave its prompt back, and the reply the click opened says + * why and carries the answer's other chips and follow-ups, so the user can + * still pick one. `messages` ends with the user's chip label and its empty + * reply; the answer that offered the chip comes before them. */ export const withRefusedChip = ( messages: ChatMessage[], @@ -76,9 +78,10 @@ export const withRefusedChip = ( detail: string, ): ChatMessage[] => { const reply = messages.at(-1); + const label = messages.at(-2); const answer = messages.at(-3); - if (reply?.role !== 'bot') { + if (reply?.role !== 'bot' || label?.role !== 'user') { return messages; } @@ -86,7 +89,8 @@ export const withRefusedChip = ( const chips = (offered?.chips ?? []).filter((other) => !sameChip(other, chip)); return [ - ...messages.slice(0, -1), + ...messages.slice(0, -2), + { ...label, refunded: true }, { ...reply, text: `_${detail}_`, diff --git a/src/lib/studio/plan.ts b/src/lib/studio/plan.ts index 3aa509f..7183b9c 100644 --- a/src/lib/studio/plan.ts +++ b/src/lib/studio/plan.ts @@ -130,7 +130,7 @@ export const PLAN_REVISION_STALE = 'plan_revision_stale'; /** Shown when a stale refusal carries no current plan (a pause the harness * stored before it kept cards). */ export const STALE_NOTICE = - 'This plan changed after this page received it, so it can no longer be switched here. Reload the conversation to see the current plan.'; + 'This plan changed after this page received it, so it can no longer be answered here. Reload the conversation to see the current plan.'; /** The index of the step whose result answers the question, on a live * plan and an approved one alike: the one the harness flags, else the last @@ -381,7 +381,7 @@ export const planFromFrame = (frame: PlanProposed, previous?: PlanState): PlanSt /** The plan offers switching: it is live, lists the action and carries the * revision a switch must echo. */ -export const canSwitch = (plan: PlanState): boolean => +export const canSwitch = (plan: PlanState): plan is PlanState & { revision: string } => plan.live && plan.actions.includes('switch') && plan.revision !== null; /** A decision's switchable options, or null for an ordinary decision. */ @@ -414,7 +414,7 @@ export const switchReply = ( choiceId: string, optionId: string, ): SwitchReply | null => { - if (!canSwitch(plan) || plan.switching || plan.revision === null) { + if (!canSwitch(plan) || plan.switching) { return null; } @@ -444,8 +444,9 @@ export const startSwitch = (plan: PlanState, pick: PlanSwitch): PlanState => ({ notice: null, }); +/** The harness refused a switch or approve sent against an older plan. */ const isStale = (refusal: StreamRefusal): boolean => - refusal.error?.constraint_id === PLAN_REVISION_STALE; + refusal.status === 409 && refusal.error?.constraint_id === PLAN_REVISION_STALE; /** The constraint_id of a switch the harness refused because the plan does * not offer the option. */ @@ -531,8 +532,8 @@ const leavesPause = ({ status, error }: StreamRefusal): boolean => * The card after the harness refused its switch. The card always stays. * * A 409 `plan_revision_stale` brings the current plan, and switching works - * again on it. Without one the card keeps its revision, so an approve from it - * is refused as stale, and drops the switch action. A refusal that leaves + * again on it. Without one the card ends read-only, saying to reload, since + * the harness refuses an approve or a switch from it as stale. A refusal that leaves * the pause alone (leavesPause) says on the choice's row that the switch did * not apply, with the harness's detail when a ToolError carries it and the * card's own words otherwise, since a proxy's text is written for @@ -555,11 +556,11 @@ export const refusedSwitch = ( const pick = plan.switching; const settled = { ...plan, switching: null }; - if (refusal.status === 409 && refusal.error?.constraint_id === PLAN_REVISION_STALE) { + if (isStale(refusal)) { return ( stalePlan(refusal, plan) ?? { ...settled, - actions: plan.actions.filter((action) => action !== 'switch'), + live: false, notice: planNotice({ text: STALE_NOTICE }), } ); @@ -909,7 +910,9 @@ export const approvedView = (plan: ApprovedPlan): PlanView => { changedReason: null, reason: null, decision: null, - planner: false, + // A row saved before the harness sent choice ids counts as + // scope, as that harness listed only scope choices. + planner: choice.choice_id !== undefined && isPlannerChoice(choice.choice_id), })), moved: [], others: [], diff --git a/src/lib/studio/test/chips.test.ts b/src/lib/studio/test/chips.test.ts index c8a4c92..be64f91 100644 --- a/src/lib/studio/test/chips.test.ts +++ b/src/lib/studio/test/chips.test.ts @@ -95,7 +95,8 @@ test('withRefusedChip: the reply says why and keeps the answer’s other chips a ]; assert.deepEqual(withRefusedChip(messages, sector, 'Nothing changed.'), [ - ...messages.slice(0, 3), + ...messages.slice(0, 2), + { role: 'user', text: sector.label, refunded: true }, { role: 'bot', text: '_Nothing changed._', followups: ['By donor'], chips: [region] }, ]); }); diff --git a/src/lib/studio/test/fixtures/event-16-plan-approved.json b/src/lib/studio/test/fixtures/event-16-plan-approved.json new file mode 100644 index 0000000..a9f52a0 --- /dev/null +++ b/src/lib/studio/test/fixtures/event-16-plan-approved.json @@ -0,0 +1,37 @@ +{ + "event": "plan.approved", + "seq": 1, + "plan_version_id": "plan_01j9ze2k7c", + "version": 1, + "revision": "rev_01j9ze2k7c", + "title": "Health aid to Ethiopia, 2016\u20132024", + "summary": "Gross disbursements for health to Ethiopia from all donors, 2016\u20132024, in constant 2023 US dollars, from OECD CRS.", + "steps": [ + { + "label": "Find the health sector codes" + }, + { + "label": "Fetch disbursements to Ethiopia" + }, + { + "label": "Compare levels before and after 2021", + "gives_answer": true + } + ], + "choices": [ + { + "choice_id": "req_1:basis", + "question": "Basis", + "chosen": "Gross disbursements" + }, + { + "choice_id": "decision/prices", + "question": "Prices", + "chosen": "Constant 2023 USD", + "changed_from": "Current USD" + } + ], + "caveats": [ + "As asked, the inputs count different providers' spending: bilateral aid from the OECD CRS and all official development assistance from DAC1." + ] +} diff --git a/src/lib/studio/test/plan.test.ts b/src/lib/studio/test/plan.test.ts index 4d7b364..00c3c44 100644 --- a/src/lib/studio/test/plan.test.ts +++ b/src/lib/studio/test/plan.test.ts @@ -3,6 +3,7 @@ * refuses before sending, and the card after a refusal or a fresh plan. */ import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; import test from 'node:test'; import { @@ -755,7 +756,7 @@ test('an approve carries the revision of the card the user approved', () => { }); }); -test('a stale switch without a current card keeps its revision, so an approve is refused', () => { +test('a stale switch without a current card ends read-only, offering nothing the harness refuses', () => { const plan = startSwitch(planFromFrame(frame()), { choiceId: 'spec_1:measure_basis', optionId: 'measure_basis:net', @@ -772,9 +773,9 @@ test('a stale switch without a current card keeps its revision, so an approve is choiceId: null, optionId: null, }); + assert.equal(refused.live, false); assert.equal(canSwitch(refused), false); assert.equal(switchReply(refused, 'spec_1:measure_basis', 'measure_basis:net'), null); - assert.deepEqual(approveReply(refused), { decision: 'approve', plan_revision: 'rev-a' }); }); test('an unavailable option keeps the card and shows the harness reason', () => { @@ -1390,10 +1391,21 @@ const approvedFrame: PlanApproved = { { label: 'Chart' }, ], choices: [ - { question: 'Basis', chosen: 'net disbursements', changed_from: 'gross disbursements' }, + { + choice_id: 'spec_1:measure_basis', + question: 'Basis', + chosen: 'net disbursements', + changed_from: 'gross disbursements', + }, ], }; +/** The harness's wire fixture (`services/harness/tests/wire_fixtures/studio` + * in dfo-ai), copied. */ +const approvedFixture = JSON.parse( + readFileSync(new URL('./fixtures/event-16-plan-approved.json', import.meta.url), 'utf8'), +) as PlanApproved; + test('plan.approved rides on the reply the approve resumed and survives a save and a fork', () => { const messages: ChatMessage[] = [ { role: 'user', text: 'What share of US aid went to health in 2023?' }, @@ -1450,6 +1462,32 @@ test('a stored approved plan without steps or choices still gives a view', () => } }); +test('an approved plan files planner decisions under their own head, by choice id', () => { + assert.deepEqual( + ledgerSections(approvedView(approvedFromFrame(approvedFixture))).map((section) => [ + section.head, + section.choices.map((choice) => choice.name), + ]), + [ + [SCOPE_HEAD, ['Basis']], + [PLANNER_HEAD, ['Prices']], + ], + ); + + // Saved before the harness sent choice ids: every choice counts as scope. + const stored = JSON.parse( + JSON.stringify({ + ...approvedFixture, + choices: approvedFixture.choices.map(({ question, chosen }) => ({ question, chosen })), + }), + ) as PlanApproved; + + assert.deepEqual( + ledgerSections(approvedView(stored)).map((section) => section.head), + [SCOPE_HEAD], + ); +}); + test('an approved plan with no step flagged answers with its last step', () => { const view = approvedView( approvedFromFrame({ ...approvedFrame, steps: [{ label: 'Fetch' }, { label: 'Sum' }] }), diff --git a/src/lib/studio/test/transcript.test.ts b/src/lib/studio/test/transcript.test.ts index fefd2a5..3887ecc 100644 --- a/src/lib/studio/test/transcript.test.ts +++ b/src/lib/studio/test/transcript.test.ts @@ -3,7 +3,7 @@ import test from 'node:test'; import type { ChatMessage } from '../../../components/organisms/Studio/hooks/useChat.ts'; import type { ShareMark } from '../../chat/history.ts'; -import { isPrompt, shareMarkAfter, withAnswer } from '../transcript.ts'; +import { isPrompt, isRefundedPrompt, shareMarkAfter, withAnswer } from '../transcript.ts'; const paused: ChatMessage[] = [ { role: 'user', text: 'Show me aid to Kenya' }, @@ -45,6 +45,14 @@ test('only a turn-opening user message is a prompt', () => { assert.equal(isPrompt({ role: 'bot', text: 'According to the OECD…' }), false); }); +test('a refused chip stays a prompt and counts as refunded', () => { + const refused: ChatMessage = { role: 'user', text: 'Gross', refunded: true }; + + assert.equal(isPrompt(refused), true); + assert.equal(isRefundedPrompt(refused), true); + assert.equal(isRefundedPrompt({ role: 'user', text: 'Show me aid to Kenya' }), false); +}); + // The mark useConversationSave records for a share of `messages`. const markFor = (messages: ChatMessage[]): ShareMark => ({ messageIndex: messages.length, diff --git a/src/lib/studio/transcript.ts b/src/lib/studio/transcript.ts index 4ae9682..9f89f99 100644 --- a/src/lib/studio/transcript.ts +++ b/src/lib/studio/transcript.ts @@ -9,6 +9,11 @@ import type { ShareMark } from '@/lib/chat/history'; export const isPrompt = (message: ChatMessage): boolean => message.role === 'user' && !message.answer; +/** A prompt the harness refused before its turn started, which the site's + * prompt meter gave back, so it no longer counts against a guest's limit. */ +export const isRefundedPrompt = (message: ChatMessage): boolean => + message.role === 'user' && !!message.refunded; + /** * The transcript with the user's answer to a paused turn. It goes before the * turn's reply, which the resumed stream goes on writing into. diff --git a/src/lib/studio/wire.ts b/src/lib/studio/wire.ts index a599710..6c2c2b7 100644 --- a/src/lib/studio/wire.ts +++ b/src/lib/studio/wire.ts @@ -424,10 +424,13 @@ export type PlanSwitchFailed = { export type PlanApprovedStep = { label: string; gives_answer?: boolean | null }; -/** One choice of an approved plan: its short name, the option label it - * holds, and, when the user's switches moved it, the label it held on the - * turn's first plan. */ +/** One choice of an approved plan: its id (the plan card's + * `PlanDecision.choice_id`, `decision/` for a planner decision), its + * short name, the option label it holds, and, when the user's switches + * moved it, the label it held on the turn's first plan. */ export type PlanApprovedChoice = { + // Absent from a frame stored in a chat snapshot before the harness sent it. + choice_id?: string; question: string; chosen: string; changed_from?: string | null; diff --git a/src/lib/test/bot.test.ts b/src/lib/test/bot.test.ts new file mode 100644 index 0000000..a4605e8 --- /dev/null +++ b/src/lib/test/bot.test.ts @@ -0,0 +1,33 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; + +import { isBotRequest } from '../bot.ts'; + +const request = (userAgent?: string) => + new Headers(userAgent === undefined ? {} : { 'user-agent': userAgent }); + +test('isBotRequest: crawlers, link-preview fetchers and scripts are bots, browsers are not', () => { + for (const userAgent of [ + 'Mozilla/5.0 (compatible; Googlebot/2.1; +http://www.google.com/bot.html)', + 'facebookexternalhit/1.1 (+http://www.facebook.com/externalhit_uatext.php)', + 'Slackbot-LinkExpanding 1.0 (+https://api.slack.com/robots)', + 'Mozilla/5.0 AppleWebKit/537.36 (KHTML, like Gecko; compatible; GPTBot/1.2; +https://openai.com/gptbot)', + 'curl/8.7.1', + ]) { + assert.equal(isBotRequest(request(userAgent)), true, userAgent); + } + + assert.equal(isBotRequest(request()), true); + assert.equal( + isBotRequest( + request( + 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.4 Safari/605.1.15', + ), + ), + false, + ); +}); + +test('isBotRequest: an event with no request behind it is the server’s own', () => { + assert.equal(isBotRequest(null), false); +}); diff --git a/src/lib/test/crawl.test.ts b/src/lib/test/crawl.test.ts index 1fda1c2..90330b0 100644 --- a/src/lib/test/crawl.test.ts +++ b/src/lib/test/crawl.test.ts @@ -7,6 +7,7 @@ import assert from 'node:assert/strict'; import test from 'node:test'; import { getPathMatch } from 'next/dist/shared/lib/router/utils/path-match.js'; +import { matchHas } from 'next/dist/shared/lib/router/utils/prepare-destination.js'; import type { GalleryItem } from '@/components/organisms/Gallery/galleryData'; @@ -30,13 +31,22 @@ const blocked = (path: string): boolean => { }); }; -// The X-Robots-Tag next.config.ts sends for a path, matched the way Next -// matches a header rule's `source`. -const robotsHeader = (path: string): string | undefined => - noindexHeaders() - .filter((rule) => getPathMatch(rule.source)(path) !== false) +// The X-Robots-Tag next.config.ts sends for a URL, matched the way Next +// matches a header rule's `source`, `has` and `missing`. +const robotsHeader = (path: string): string | undefined => { + const url = new URL(path, 'https://example.org'); + const query = Object.fromEntries(url.searchParams); + const request = { headers: {}, cookies: {} } as Parameters[0]; + + return noindexHeaders(gallery) + .filter( + (rule) => + getPathMatch(rule.source)(url.pathname) !== false && + matchHas(request, query, rule.has, rule.missing) !== false, + ) .flatMap((rule) => rule.headers) .find((header) => header.key === 'X-Robots-Tag')?.value; +}; const base = { title: 'Item', @@ -86,6 +96,28 @@ test('every noindex route answers noindex, redirects and errors included', () => } }); +test('/data says noindex in its headers exactly where its metadata does', () => { + const queries: Record[] = [ + {}, + { share: 'bbbbbbbb' }, + { share: 'cccccccc' }, + { share: 'aaaaaaaa' }, + { spec: 'x' }, + { spec: 'x', share: 'bbbbbbbb' }, + { q: 'aid to Ghana' }, + ]; + + for (const query of queries) { + const path = `/data?${new URLSearchParams(query)}`; + + assert.equal( + robotsHeader(path), + dataPageRobots(query, gallery) ? 'noindex, nofollow' : undefined, + path, + ); + } +}); + test('crawlers may fetch the noindex routes and not the routes for programs', () => { for (const route of NOINDEX_ROUTES) { assert.equal(blocked(route), false, route); @@ -134,8 +166,10 @@ test('a gallery search carries its title and description, other shares do not', description: 'Aid to Ghana since 2000.', openGraph: { type: 'article', + siteName: 'Development Finance Observatory', title: 'Aid to Ghana', description: 'Aid to Ghana since 2000.', + images: '/opengraph-image', }, }); assert.deepEqual(dataPageMetadata({ share: 'cccccccc' }, gallery), { @@ -163,7 +197,7 @@ test('every page the sitemap lists may be fetched and indexed', () => { const url = new URL(path, 'https://example.org'); assert.equal(blocked(path), false, path); - assert.equal(robotsHeader(url.pathname), undefined, path); + assert.equal(robotsHeader(path), undefined, path); if (url.pathname === '/data') { const query = Object.fromEntries(url.searchParams); From 7fa86fa6bea0d89735a354136dd9cefc23f62a89 Mon Sep 17 00:00:00 2001 From: Jorge Rivera Date: Thu, 29 Oct 2026 16:52:48 +0100 Subject: [PATCH 2/2] fix: refund any refused guest turn and decide /data noindex in one function stream refunds a metered turn's prompt on any refusal, as meterTurn does. src/proxy.ts sets /data's X-Robots-Tag from dataPageRobots, the function the page metadata uses. Preview cards carry their size and type, and a gallery dataset previews with its own card. --- next.config.ts | 7 +- .../gallery/[slug]/opengraph-image.tsx | 47 ++++---- .../(studio)/insights/gallery/[slug]/page.tsx | 2 +- src/app/opengraph-image.tsx | 8 +- .../organisms/Studio/hooks/useChat.ts | 42 ++++--- src/lib/crawl.ts | 74 ++++++------ src/lib/search/parsedEdit.ts | 13 +-- src/lib/search/test/parsedEdit.test.ts | 17 ++- src/lib/site.ts | 43 +++++-- src/lib/studio/chips.ts | 16 +-- src/lib/studio/plan.ts | 4 +- src/lib/studio/test/chips.test.ts | 3 +- src/lib/studio/test/transcript.test.ts | 31 ++++- src/lib/studio/transcript.ts | 18 ++- src/lib/test/crawl.test.ts | 108 +++++++++++------- src/proxy.ts | 22 ++++ 16 files changed, 294 insertions(+), 161 deletions(-) create mode 100644 src/proxy.ts diff --git a/next.config.ts b/next.config.ts index cf681aa..ca39448 100644 --- a/next.config.ts +++ b/next.config.ts @@ -2,7 +2,6 @@ import { withSentryConfig } from '@sentry/nextjs/config'; import path from 'path'; import type { NextConfig } from 'next'; -import gallery from './src/content/gallery/index.ts'; import { noindexHeaders } from './src/lib/crawl.ts'; const nextConfig: NextConfig = { @@ -22,10 +21,10 @@ const nextConfig: NextConfig = { .map((origin) => origin.trim()) .filter(Boolean), // Share and embed links, the Studio, history and sign-in say noindex on - // every response, redirects and errors included, and so do the /data - // links kept out of search (src/lib/crawl.ts). + // every response, redirects and errors included (src/lib/crawl.ts). + // src/proxy.ts sends /data's. async headers() { - return noindexHeaders(gallery); + return noindexHeaders(); }, // A docs page's raw markdown sits beside it: /docs/.md serves the // same file the docs corpus was built from (src/app/docs-md). diff --git a/src/app/(studio)/insights/gallery/[slug]/opengraph-image.tsx b/src/app/(studio)/insights/gallery/[slug]/opengraph-image.tsx index e053dc5..08c4fdf 100644 --- a/src/app/(studio)/insights/gallery/[slug]/opengraph-image.tsx +++ b/src/app/(studio)/insights/gallery/[slug]/opengraph-image.tsx @@ -1,9 +1,10 @@ import { ImageResponse } from 'next/og'; import { GALLERY_ITEMS, galleryItem } from '@/components/organisms/Gallery/galleryData'; +import { CARD_SIZE, CARD_TYPE } from '@/lib/site'; -export const size = { width: 1200, height: 630 }; -export const contentType = 'image/png'; +export const size = CARD_SIZE; +export const contentType = CARD_TYPE; export const dynamicParams = false; @@ -19,32 +20,28 @@ const OpenGraphImage = async ({ params }: { params: Promise<{ slug: string }> }) const item = galleryItem((await params).slug); return new ImageResponse( - ( -
-
- DFO Gallery +
+
DFO Gallery
+
+
+ {item?.title ?? 'DFO Studio session'}
-
-
- {item?.title ?? 'DFO Studio session'} -
-
- {(item?.description ?? '').slice(0, 120)} -
+
+ {(item?.description ?? '').slice(0, 120)}
- ), +
, size, ); }; diff --git a/src/app/(studio)/insights/gallery/[slug]/page.tsx b/src/app/(studio)/insights/gallery/[slug]/page.tsx index e07e032..ba07908 100644 --- a/src/app/(studio)/insights/gallery/[slug]/page.tsx +++ b/src/app/(studio)/insights/gallery/[slug]/page.tsx @@ -30,7 +30,7 @@ export const generateMetadata = async ({ params }: GalleryItemPageProps): Promis openGraph: pageOpenGraph({ title: item.title, description: item.description, - ownCard: true, + image: null, }), }; }; diff --git a/src/app/opengraph-image.tsx b/src/app/opengraph-image.tsx index fea575a..f8a1944 100644 --- a/src/app/opengraph-image.tsx +++ b/src/app/opengraph-image.tsx @@ -3,9 +3,11 @@ import { join } from 'node:path'; import { ImageResponse } from 'next/og'; -export const alt = 'Development Finance Observatory: Development Finance Data. Simplified.'; -export const size = { width: 1200, height: 630 }; -export const contentType = 'image/png'; +import { CARD_SIZE, CARD_TYPE, SITE_CARD } from '@/lib/site'; + +export const alt = SITE_CARD.alt; +export const size = CARD_SIZE; +export const contentType = CARD_TYPE; // Outfit, the site's typeface, from the @fontsource package the layout // already loads. The image has no request-time input, so it renders once at diff --git a/src/components/organisms/Studio/hooks/useChat.ts b/src/components/organisms/Studio/hooks/useChat.ts index 92f21bd..890d0b6 100644 --- a/src/components/organisms/Studio/hooks/useChat.ts +++ b/src/components/organisms/Studio/hooks/useChat.ts @@ -58,7 +58,7 @@ import { type Pause, type PlanState, } from '@/lib/studio/plan'; -import { withAnswer } from '@/lib/studio/transcript'; +import { withAnswer, withRefundedPrompt } from '@/lib/studio/transcript'; import { cancelTurn, fetchComponentRevision, @@ -88,8 +88,8 @@ import type { CardRef, Widget } from '@/lib/chat/widgets'; // so the transcript shows what the question was about. `answer` marks the // user's answer to a paused turn (a clarify choice or text, a plan // amendment): shown like any user message, but part of the turn it answers. -// `refunded` marks a prompt the harness refused before its turn started (a -// chip that no longer applies), which the site's prompt meter gave back. +// `refunded` marks a prompt whose turn was refused, which the site's prompt +// meter gave back. // A bot turn's `followups` are prompts a click sends as the next message, // and its `chips` switch a scope choice of the answer's readings. `plan` is // the plan the user approved for this answer, saved with the transcript so @@ -414,7 +414,9 @@ export const useChat = (initial?: ChatSnapshot) => { // on screen. A caller without one gets `endOwn`. A caller with one (a // switch, whose harness may re-offer the plan after a Stop) also keeps // the frames after Stop, for up to SWITCH_STOP_GRACE_MS, with the chat - // line on "Stopping". + // line on "Stopping". `metered` marks a POST turn, whose prompt the + // site's meter gives back when the response is a refusal (meterTurn), so + // a refusal marks the turn's prompt refunded. const stream = useCallback( ( turn: number, @@ -423,10 +425,12 @@ export const useChat = (initial?: ChatSnapshot) => { onRefusal, onUnreachable, onEnd, + metered = false, }: { onRefusal?: (refusal: StreamRefusal) => boolean; onUnreachable?: () => boolean; onEnd?: (ending: StreamEnding) => void; + metered?: boolean; } = {}, ) => { const controller = new AbortController(); @@ -850,11 +854,17 @@ export const useChat = (initial?: ChatSnapshot) => { // handle() drops frames. A switch the user stopped then ends as // stopped (its `onEnd`), not on a live card whose buttons the // cancel has made useless. - const refuse = (refusal: StreamRefusal): boolean => - generationRef.current !== generation || - muted || - stopped || - (onRefusal?.(refusal) ?? false); + const refuse = (refusal: StreamRefusal): boolean => { + if (generationRef.current !== generation) { + return true; + } + + if (metered) { + setState((prev) => ({ ...prev, messages: withRefundedPrompt(prev.messages) })); + } + + return muted || stopped || (onRefusal?.(refusal) ?? false); + }; const unreachable = (): boolean => generationRef.current !== generation || muted || @@ -1014,11 +1024,14 @@ export const useChat = (initial?: ChatSnapshot) => { // A question handed over from search carries its seeds, the // datasets the user saw there, so the harness starts from them. - stream(turn, (handlers, signal) => - runTurn(conversation, recap ? `${recap}${text}` : text, handlers, signal, { - ...(componentRef && { component_ref: componentRef }), - ...(seeds && { seed_spec: seeds }), - }), + stream( + turn, + (handlers, signal) => + runTurn(conversation, recap ? `${recap}${text}` : text, handlers, signal, { + ...(componentRef && { component_ref: componentRef }), + ...(seeds && { seed_spec: seeds }), + }), + { metered: true }, ); }, [holdTurn, startTurn, stream], @@ -1320,6 +1333,7 @@ export const useChat = (initial?: ChatSnapshot) => { turn, (handlers, signal) => runApplyChoice(conversation, chip, handlers, signal), { + metered: true, onRefusal: (refusal) => { const detail = chipRefusalDetail(refusal); diff --git a/src/lib/crawl.ts b/src/lib/crawl.ts index bf02af1..a0fd661 100644 --- a/src/lib/crawl.ts +++ b/src/lib/crawl.ts @@ -4,11 +4,11 @@ * and say `noindex` themselves. Its one runtime import, lib/site.ts, has * none of its own, so next.config.ts and `node --test` can load it. */ -import type { Metadata, NextConfig } from 'next'; +import type { Metadata } from 'next'; import type { GalleryItem } from '@/components/organisms/Gallery/galleryData'; -import { pageOpenGraph } from './site.ts'; +import { galleryCard, pageOpenGraph } from './site.ts'; /** * The routes kept out of search results, each with everything under it: @@ -18,44 +18,18 @@ import { pageOpenGraph } from './site.ts'; */ export const NOINDEX_ROUTES = ['/s', '/embed', '/studio', '/history', '/login', '/register']; -type HeaderRule = Awaited>>[number]; - -const NOINDEX_HEADER = [{ key: 'X-Robots-Tag', value: 'noindex, nofollow' }]; - -/** A regular expression matching exactly the given values. */ -const anyOf = (values: readonly string[]): string => - `(?:${values.map((value) => value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')).join('|')})`; +/** The header that keeps a response out of search results. */ +export const NOINDEX_HEADER = { key: 'X-Robots-Tag', value: 'noindex, nofollow' }; /** * next.config.ts `headers()`: `X-Robots-Tag` on every response from a * noindex route, so its redirects, 404s and error pages say noindex too, * and a page whose metadata streams into `` says it in the headers. - * /data gets it for the URLs dataPageRobots keeps out of search: any - * version link (?spec=) and any search share (?share=) no gallery item - * opens. + * /data decides per URL, so src/proxy.ts sends its header + * (dataPageNoindex). */ -export const noindexHeaders = (gallery: readonly GalleryItem[]): HeaderRule[] => { - const gallerySearches = gallery.flatMap((item) => - item.kind === 'dataset' ? [item.sharedFrom] : [], - ); - - return [ - ...NOINDEX_ROUTES.map((route) => ({ source: `${route}/:path*`, headers: NOINDEX_HEADER })), - { - source: '/data', - has: [{ type: 'query', key: 'spec' }], - headers: NOINDEX_HEADER, - }, - { - source: '/data', - has: [{ type: 'query', key: 'share' }], - ...(gallerySearches.length > 0 && { - missing: [{ type: 'query', key: 'share', value: anyOf(gallerySearches) }], - }), - headers: NOINDEX_HEADER, - }, - ]; -}; +export const noindexHeaders = () => + NOINDEX_ROUTES.map((route) => ({ source: `${route}/:path*`, headers: [NOINDEX_HEADER] })); const NOINDEX: Metadata['robots'] = { index: false, follow: false }; @@ -114,7 +88,11 @@ export const dataPageMetadata = ( return { title: `${item.title} — DFO Gallery`, description: item.description, - openGraph: pageOpenGraph({ title: item.title, description: item.description }), + openGraph: pageOpenGraph({ + title: item.title, + description: item.description, + image: galleryCard(item.slug), + }), }; } @@ -122,3 +100,29 @@ export const dataPageMetadata = ( return robots ? { robots } : {}; }; + +/** + * A /data URL's query as the page's `searchParams` holds it: a key given + * once maps to its value, empty or not, and a repeated key to every value. + */ +export const dataPageQuery = (params: URLSearchParams): DataPageQuery => { + const query: Record = {}; + + for (const key of new Set(params.keys())) { + const values = params.getAll(key); + + query[key] = values.length > 1 ? values : (values[0] ?? ''); + } + + return query; +}; + +/** + * Whether src/proxy.ts sends `X-Robots-Tag: noindex` for a /data URL. It + * reads the URL through dataPageRobots, as the page's metadata does, so the + * header and the metadata cannot disagree. + */ +export const dataPageNoindex = ( + params: URLSearchParams, + gallery: readonly GalleryItem[], +): boolean => dataPageRobots(dataPageQuery(params), gallery) !== undefined; diff --git a/src/lib/search/parsedEdit.ts b/src/lib/search/parsedEdit.ts index f4ff77c..995ebc0 100644 --- a/src/lib/search/parsedEdit.ts +++ b/src/lib/search/parsedEdit.ts @@ -139,17 +139,14 @@ export function entityChoiceGroups(noData: NoData | null | undefined): EntityCho /** * What each group's rows say in a menu that merges every group: the - * wording, numbered in `entity_choice` order where two groups share it - * ("aid from Congo to Congo" gives "Congo (1)", "Congo (2)"). + * wording, numbered in menu order where two groups share it ("aid from + * Congo to Congo" gives "Congo (1)", "Congo (2)"). */ export function entityChoiceLabels(groups: readonly EntityChoiceGroup[]): string[] { - return groups.map(({ key, text }) => { - const alike = groups.filter((group) => group.text === text); + return groups.map(({ text }, index) => { + if (groups.filter((group) => group.text === text).length === 1) return text; - if (alike.length === 1) return text; - - // Only entity_choice keys (numbers) can share a wording. - const place = alike.filter((group) => group.key < key).length + 1; + const place = groups.slice(0, index + 1).filter((group) => group.text === text).length; return `${text} (${place})`; }); diff --git a/src/lib/search/test/parsedEdit.test.ts b/src/lib/search/test/parsedEdit.test.ts index 252a78c..7658954 100644 --- a/src/lib/search/test/parsedEdit.test.ts +++ b/src/lib/search/test/parsedEdit.test.ts @@ -290,11 +290,24 @@ test('entityChoiceGroups keeps one group per entity_choice, two written alike ap ); }); -test('entityChoiceLabels numbers groups that share a wording, in entity_choice order', () => { +test('entityChoiceLabels numbers groups that share a wording, in menu order', () => { const groups = entityChoiceGroups(congoToCongoNoData); + const [first, second] = congoToCongoNoData.options ?? []; + assert.ok(first && second); assert.deepEqual(entityChoiceLabels(groups), ['Congo (1)', 'Congo (2)']); - assert.deepEqual(entityChoiceLabels([...groups].reverse()), ['Congo (2)', 'Congo (1)']); + // One option settles its group by entity_choice, the other by mention: + // two keys of different kinds, one wording. + const mixed = entityChoiceGroups({ + ...congoToCongoNoData, + options: [first, { ...second, entity_choice: null, mentions: ['Congo'] }], + }); + + assert.deepEqual( + mixed.map((group) => group.key), + [0, 'Congo'], + ); + assert.deepEqual(entityChoiceLabels(mixed), ['Congo (1)', 'Congo (2)']); // A wording no other group shares keeps its own text. assert.deepEqual(entityChoiceLabels(entityChoiceGroups(odaToCongoNoData)), ['Congo']); }); diff --git a/src/lib/site.ts b/src/lib/site.ts index 961f5bc..d295886 100644 --- a/src/lib/site.ts +++ b/src/lib/site.ts @@ -8,28 +8,55 @@ export const SITE_ORIGIN = 'https://developmentfinance.org'; export const SITE_NAME = 'Development Finance Observatory'; -/** The site's own link-preview card, drawn by app/opengraph-image.tsx. */ -export const SITE_CARD = '/opengraph-image'; +/** The size and type of every link-preview card the site draws. */ +export const CARD_SIZE = { width: 1200, height: 630 }; +export const CARD_TYPE = 'image/png'; + +type Card = { url: string; width: number; height: number; type: string; alt?: string }; + +/** The site's own card, drawn by app/opengraph-image.tsx. */ +export const SITE_CARD: Card = { + url: '/opengraph-image', + ...CARD_SIZE, + type: CARD_TYPE, + alt: 'Development Finance Observatory: Development Finance Data. Simplified.', +}; + +/** + * Next suffixes a metadata image under a route group with a hash of its + * parent path, so the gallery item card's address carries one. + * src/lib/test/crawl.test.ts checks it against Next's own. + */ +const GALLERY_CARD_SUFFIX = '1pc5v7'; + +/** A gallery item's own card, drawn by + * app/(studio)/insights/gallery/[slug]/opengraph-image.tsx. */ +export const galleryCard = (slug: string): Card => ({ + url: `/insights/gallery/${slug}/opengraph-image-${GALLERY_CARD_SUFFIX}`, + ...CARD_SIZE, + type: CARD_TYPE, +}); /** * The Open Graph tags of a page that names itself. Next replaces a * segment's `openGraph` whole, so a page that sets one would drop the root - * layout's site name and card; these keep both. A route with an - * `opengraph-image` file of its own passes `ownCard`, since Next reads the - * file only when the page's `openGraph` has no `images`. + * layout's site name and card; these keep the site name and show `image`, + * the site's card unless the page names another. A route with an + * `opengraph-image` file of its own passes null, since Next reads the file + * only when the page's `openGraph` has no `images`. */ export const pageOpenGraph = ({ title, description, - ownCard = false, + image = SITE_CARD, }: { title: string; description: string; - ownCard?: boolean; + image?: Card | null; }): NonNullable => ({ type: 'article', siteName: SITE_NAME, title, description, - ...(!ownCard && { images: SITE_CARD }), + ...(image && { images: image }), }); diff --git a/src/lib/studio/chips.ts b/src/lib/studio/chips.ts index 7839028..9585cc9 100644 --- a/src/lib/studio/chips.ts +++ b/src/lib/studio/chips.ts @@ -65,12 +65,10 @@ export const chipRefusalDetail = (refusal: StreamRefusal): string | null => : null; /** - * The transcript after the harness refused `chip`: the chip's label is - * marked refunded, since the refused turn never started and the site's - * prompt meter gave its prompt back, and the reply the click opened says - * why and carries the answer's other chips and follow-ups, so the user can - * still pick one. `messages` ends with the user's chip label and its empty - * reply; the answer that offered the chip comes before them. + * The transcript after the harness refused `chip`: the reply the click + * opened says why, and carries the answer's other chips and follow-ups, so + * the user can still pick one. `messages` ends with the user's chip label + * and its empty reply; the answer that offered the chip comes before them. */ export const withRefusedChip = ( messages: ChatMessage[], @@ -78,10 +76,9 @@ export const withRefusedChip = ( detail: string, ): ChatMessage[] => { const reply = messages.at(-1); - const label = messages.at(-2); const answer = messages.at(-3); - if (reply?.role !== 'bot' || label?.role !== 'user') { + if (reply?.role !== 'bot') { return messages; } @@ -89,8 +86,7 @@ export const withRefusedChip = ( const chips = (offered?.chips ?? []).filter((other) => !sameChip(other, chip)); return [ - ...messages.slice(0, -2), - { ...label, refunded: true }, + ...messages.slice(0, -1), { ...reply, text: `_${detail}_`, diff --git a/src/lib/studio/plan.ts b/src/lib/studio/plan.ts index 7183b9c..2f555d5 100644 --- a/src/lib/studio/plan.ts +++ b/src/lib/studio/plan.ts @@ -911,7 +911,9 @@ export const approvedView = (plan: ApprovedPlan): PlanView => { reason: null, decision: null, // A row saved before the harness sent choice ids counts as - // scope, as that harness listed only scope choices. + // scope. The production harness then listed only scope choices; + // a development harness before dfo-ai#106 also listed planner + // decisions without ids, so chats saved there file them as scope. planner: choice.choice_id !== undefined && isPlannerChoice(choice.choice_id), })), moved: [], diff --git a/src/lib/studio/test/chips.test.ts b/src/lib/studio/test/chips.test.ts index be64f91..c8a4c92 100644 --- a/src/lib/studio/test/chips.test.ts +++ b/src/lib/studio/test/chips.test.ts @@ -95,8 +95,7 @@ test('withRefusedChip: the reply says why and keeps the answer’s other chips a ]; assert.deepEqual(withRefusedChip(messages, sector, 'Nothing changed.'), [ - ...messages.slice(0, 2), - { role: 'user', text: sector.label, refunded: true }, + ...messages.slice(0, 3), { role: 'bot', text: '_Nothing changed._', followups: ['By donor'], chips: [region] }, ]); }); diff --git a/src/lib/studio/test/transcript.test.ts b/src/lib/studio/test/transcript.test.ts index 3887ecc..b11840d 100644 --- a/src/lib/studio/test/transcript.test.ts +++ b/src/lib/studio/test/transcript.test.ts @@ -3,7 +3,13 @@ import test from 'node:test'; import type { ChatMessage } from '../../../components/organisms/Studio/hooks/useChat.ts'; import type { ShareMark } from '../../chat/history.ts'; -import { isPrompt, isRefundedPrompt, shareMarkAfter, withAnswer } from '../transcript.ts'; +import { + isPrompt, + isRefundedPrompt, + shareMarkAfter, + withAnswer, + withRefundedPrompt, +} from '../transcript.ts'; const paused: ChatMessage[] = [ { role: 'user', text: 'Show me aid to Kenya' }, @@ -45,12 +51,25 @@ test('only a turn-opening user message is a prompt', () => { assert.equal(isPrompt({ role: 'bot', text: 'According to the OECD…' }), false); }); -test('a refused chip stays a prompt and counts as refunded', () => { - const refused: ChatMessage = { role: 'user', text: 'Gross', refunded: true }; +test('a refused turn marks its prompt refunded, which still counts as a prompt', () => { + const messages: ChatMessage[] = [ + { role: 'user', text: 'Show me aid to Kenya' }, + { role: 'bot', text: 'USD 1.2bn' }, + { role: 'user', text: 'By donor' }, + { role: 'bot', text: '' }, + ]; + const refused = withRefundedPrompt(messages); - assert.equal(isPrompt(refused), true); - assert.equal(isRefundedPrompt(refused), true); - assert.equal(isRefundedPrompt({ role: 'user', text: 'Show me aid to Kenya' }), false); + assert.deepEqual(refused, [ + ...messages.slice(0, 2), + { role: 'user', text: 'By donor', refunded: true }, + messages[3], + ]); + assert.equal(refused.filter(isPrompt).length, 2); + assert.equal(refused.filter(isRefundedPrompt).length, 1); + // Marked once, and a transcript with no prompt is left as it was. + assert.equal(withRefundedPrompt(refused), refused); + assert.deepEqual(withRefundedPrompt([]), []); }); // The mark useConversationSave records for a share of `messages`. diff --git a/src/lib/studio/transcript.ts b/src/lib/studio/transcript.ts index 9f89f99..eb5a488 100644 --- a/src/lib/studio/transcript.ts +++ b/src/lib/studio/transcript.ts @@ -9,11 +9,25 @@ import type { ShareMark } from '@/lib/chat/history'; export const isPrompt = (message: ChatMessage): boolean => message.role === 'user' && !message.answer; -/** A prompt the harness refused before its turn started, which the site's - * prompt meter gave back, so it no longer counts against a guest's limit. */ +/** A prompt whose turn was refused, which the site's prompt meter gave + * back, so it no longer counts against a guest's limit. */ export const isRefundedPrompt = (message: ChatMessage): boolean => message.role === 'user' && !!message.refunded; +/** The transcript once the turn its last prompt opened was refused: that + * prompt marked refunded. A refused turn is the latest, since a new turn + * waits until the one in flight ends. */ +export const withRefundedPrompt = (messages: ChatMessage[]): ChatMessage[] => { + const index = messages.findLastIndex(isPrompt); + const prompt = messages[index]; + + if (prompt?.role !== 'user' || prompt.refunded) { + return messages; + } + + return messages.map((message, at) => (at === index ? { ...prompt, refunded: true } : message)); +}; + /** * The transcript with the user's answer to a paused turn. It goes before the * turn's reply, which the resumed stream goes on writing into. diff --git a/src/lib/test/crawl.test.ts b/src/lib/test/crawl.test.ts index 90330b0..bb90dd9 100644 --- a/src/lib/test/crawl.test.ts +++ b/src/lib/test/crawl.test.ts @@ -4,16 +4,26 @@ * sitemap lists pages it may fetch and index. */ import assert from 'node:assert/strict'; +import { existsSync } from 'node:fs'; import test from 'node:test'; import { getPathMatch } from 'next/dist/shared/lib/router/utils/path-match.js'; -import { matchHas } from 'next/dist/shared/lib/router/utils/prepare-destination.js'; +import metadataRoute from 'next/dist/lib/metadata/get-metadata-route.js'; +import { searchParamsToUrlQuery } from 'next/dist/shared/lib/router/utils/querystring.js'; import type { GalleryItem } from '@/components/organisms/Gallery/galleryData'; import robots from '../../app/robots.ts'; -import { dataPageMetadata, dataPageRobots, NOINDEX_ROUTES, noindexHeaders } from '../crawl.ts'; +import { + dataPageMetadata, + dataPageNoindex, + dataPageQuery, + dataPageRobots, + NOINDEX_ROUTES, + noindexHeaders, +} from '../crawl.ts'; import type { DocsPage } from '../docs/corpus.ts'; +import { galleryCard, pageOpenGraph, SITE_CARD, SITE_NAME } from '../site.ts'; import { sitemapPaths } from '../sitemap.ts'; // robots.txt matching as Google does it: a rule matches from the start of @@ -31,22 +41,13 @@ const blocked = (path: string): boolean => { }); }; -// The X-Robots-Tag next.config.ts sends for a URL, matched the way Next -// matches a header rule's `source`, `has` and `missing`. -const robotsHeader = (path: string): string | undefined => { - const url = new URL(path, 'https://example.org'); - const query = Object.fromEntries(url.searchParams); - const request = { headers: {}, cookies: {} } as Parameters[0]; - - return noindexHeaders(gallery) - .filter( - (rule) => - getPathMatch(rule.source)(url.pathname) !== false && - matchHas(request, query, rule.has, rule.missing) !== false, - ) +// The X-Robots-Tag next.config.ts sends for a path, matched the way Next +// matches a header rule's `source`. +const robotsHeader = (path: string): string | undefined => + noindexHeaders() + .filter((rule) => getPathMatch(rule.source)(path) !== false) .flatMap((rule) => rule.headers) .find((header) => header.key === 'X-Robots-Tag')?.value; -}; const base = { title: 'Item', @@ -97,25 +98,54 @@ test('every noindex route answers noindex, redirects and errors included', () => }); test('/data says noindex in its headers exactly where its metadata does', () => { - const queries: Record[] = [ - {}, - { share: 'bbbbbbbb' }, - { share: 'cccccccc' }, - { share: 'aaaaaaaa' }, - { spec: 'x' }, - { spec: 'x', share: 'bbbbbbbb' }, - { q: 'aid to Ghana' }, - ]; - - for (const query of queries) { - const path = `/data?${new URLSearchParams(query)}`; - - assert.equal( - robotsHeader(path), - dataPageRobots(query, gallery) ? 'noindex, nofollow' : undefined, - path, - ); + for (const search of [ + '', + 'share=bbbbbbbb', + 'share=cccccccc', + 'share=aaaaaaaa', + 'share=', + 'spec=', + 'spec=x', + 'spec=x&share=bbbbbbbb', + 'share=bbbbbbbb&share=bbbbbbbb', + 'share=cccccccc&share=bbbbbbbb', + 'q=aid+to+Ghana', + ]) { + const params = new URLSearchParams(search); + // The page's searchParams, as Next builds them from the URL. + const query = searchParamsToUrlQuery(params); + + assert.deepEqual(dataPageQuery(params), query, search); + assert.equal(dataPageNoindex(params, gallery), !!dataPageRobots(query, gallery), search); } + + assert.equal(dataPageNoindex(new URLSearchParams('share='), gallery), true); + assert.equal( + dataPageNoindex(new URLSearchParams('share=cccccccc&share=bbbbbbbb'), gallery), + true, + ); + assert.equal(dataPageNoindex(new URLSearchParams('share=bbbbbbbb'), gallery), false); +}); + +test('a page that names itself keeps the site name and shows the site card', () => { + assert.deepEqual(pageOpenGraph({ title: 'About', description: 'Who we are.' }), { + type: 'article', + siteName: SITE_NAME, + title: 'About', + description: 'Who we are.', + images: SITE_CARD, + }); + assert.equal('images' in pageOpenGraph({ title: 'A', description: 'B', image: null }), false); +}); + +test('a gallery item card sits where Next serves it', () => { + const segment = '/(studio)/insights/gallery/[slug]'; + + assert.ok(existsSync(new URL(`../../app${segment}/opengraph-image.tsx`, import.meta.url))); + assert.equal( + galleryCard('a-search').url, + metadataRoute.fillMetadataSegment(segment, { slug: 'a-search' }, 'opengraph-image', false), + ); }); test('crawlers may fetch the noindex routes and not the routes for programs', () => { @@ -166,10 +196,10 @@ test('a gallery search carries its title and description, other shares do not', description: 'Aid to Ghana since 2000.', openGraph: { type: 'article', - siteName: 'Development Finance Observatory', + siteName: SITE_NAME, title: 'Aid to Ghana', description: 'Aid to Ghana since 2000.', - images: '/opengraph-image', + images: galleryCard('a-search'), }, }); assert.deepEqual(dataPageMetadata({ share: 'cccccccc' }, gallery), { @@ -197,12 +227,10 @@ test('every page the sitemap lists may be fetched and indexed', () => { const url = new URL(path, 'https://example.org'); assert.equal(blocked(path), false, path); - assert.equal(robotsHeader(path), undefined, path); + assert.equal(robotsHeader(url.pathname), undefined, path); if (url.pathname === '/data') { - const query = Object.fromEntries(url.searchParams); - - assert.equal(dataPageRobots(query, gallery), undefined, path); + assert.equal(dataPageNoindex(url.searchParams, gallery), false, path); } } }); diff --git a/src/proxy.ts b/src/proxy.ts new file mode 100644 index 0000000..68827ea --- /dev/null +++ b/src/proxy.ts @@ -0,0 +1,22 @@ +import { NextResponse, type NextRequest } from 'next/server'; + +import { GALLERY_ITEMS } from '@/components/organisms/Gallery/galleryData'; +import { dataPageNoindex, NOINDEX_HEADER } from '@/lib/crawl'; + +/** + * `X-Robots-Tag` on a /data response the page's metadata keeps out of + * search (dataPageRobots): a version link, or a search share no gallery + * item opens. The header holds whatever the render does, redirects and + * errors included. + */ +export const proxy = (request: NextRequest) => { + const response = NextResponse.next(); + + if (dataPageNoindex(request.nextUrl.searchParams, GALLERY_ITEMS)) { + response.headers.set(NOINDEX_HEADER.key, NOINDEX_HEADER.value); + } + + return response; +}; + +export const config = { matcher: '/data' };