diff --git a/backend/apps/benchmarks/api.py b/backend/apps/benchmarks/api.py index 34ef928..5969939 100644 --- a/backend/apps/benchmarks/api.py +++ b/backend/apps/benchmarks/api.py @@ -257,7 +257,12 @@ def get_benchmark_case(request: HttpRequest, suite: str, case_id: str, revision: raise HttpError(404, f"Revision {target.revision} has no public case {case_id!r}.") runs = leaderboard.latest_per_tool_and_case(leaderboard.approved_runs(target), case_id=case.pk) with_public_screenshots(runs) - return {**_case_out(case), "revision": target.revision, "runs": [_run_out(r) for r in runs]} + return { + **_case_out(case), + "revision": target.revision, + "revision_is_current": target.is_current, + "runs": [_run_out(r) for r in runs], + } @router.get( @@ -298,6 +303,7 @@ def get_benchmark_tool_report( "tool": leaderboard.tool_ref(tool), "suite": suite, "revision": target.revision, + "revision_is_current": target.is_current, "scope": scope, "case_count": target.cases.count(), "surfaces": surfaces, @@ -331,6 +337,7 @@ def get_benchmark_run(request: HttpRequest, run_id: str): "tier": submission.tier, "notes": submission.notes, "revision": run.case.revision.revision, + "revision_is_current": run.case.revision.is_current, } diff --git a/backend/apps/benchmarks/schemas.py b/backend/apps/benchmarks/schemas.py index 1ba2e71..f05db6a 100644 --- a/backend/apps/benchmarks/schemas.py +++ b/backend/apps/benchmarks/schemas.py @@ -168,10 +168,14 @@ class RunDetailOut(RunOut): tier: str notes: str revision: str + #: False for a superseded revision - the site links only those with `?revision=`. + revision_is_current: bool class CaseDetailOut(CaseOut): revision: str + #: False for a superseded revision - the site links only those with `?revision=`. + revision_is_current: bool runs: list[RunOut] @@ -187,6 +191,8 @@ class ToolReportOut(Schema): tool: ToolRefOut suite: str revision: str + #: False for a superseded revision - the site links only those with `?revision=`. + revision_is_current: bool scope: Literal["all", "verified"] case_count: int surfaces: list[ToolSurfaceOut] diff --git a/backend/openapi.json b/backend/openapi.json index 81a1ef3..5128843 100644 --- a/backend/openapi.json +++ b/backend/openapi.json @@ -7198,6 +7198,10 @@ "title": "Revision", "type": "string" }, + "revision_is_current": { + "title": "Revision Is Current", + "type": "boolean" + }, "runs": { "items": { "$ref": "#/components/schemas/RunOut" @@ -7215,6 +7219,7 @@ "pdf_url", "preview", "revision", + "revision_is_current", "runs" ], "title": "CaseDetailOut", @@ -7482,6 +7487,10 @@ "title": "Revision", "type": "string" }, + "revision_is_current": { + "title": "Revision Is Current", + "type": "boolean" + }, "scope": { "enum": [ "all", @@ -7506,6 +7515,7 @@ "tool", "suite", "revision", + "revision_is_current", "scope", "case_count", "surfaces" @@ -7712,6 +7722,10 @@ "revision": { "title": "Revision", "type": "string" + }, + "revision_is_current": { + "title": "Revision Is Current", + "type": "boolean" } }, "required": [ @@ -7741,7 +7755,8 @@ "tool_version", "tier", "notes", - "revision" + "revision", + "revision_is_current" ], "title": "RunDetailOut", "type": "object" diff --git a/backend/tests/test_benchmarks_api.py b/backend/tests/test_benchmarks_api.py index 2316525..22298c0 100644 --- a/backend/tests/test_benchmarks_api.py +++ b/backend/tests/test_benchmarks_api.py @@ -121,6 +121,29 @@ def test_a_run_page_carries_the_whole_report(client, published): assert body["scored_by"] == "submitter" +def _detail_urls(run): + return [ + f"{BASE}/suites/pdf/cases/extraction-conditions-1?revision=v0.1.1", + f"{BASE}/suites/pdf/tools/pdf-redaction?revision=v0.1.1", + f"{BASE}/runs/{run.run_id}", + ] + + +def test_a_detail_page_says_its_revision_is_the_current_one(client, published): + """So the site can link the clean URL rather than a `?revision=` copy of it.""" + for url in _detail_urls(published): + assert client.get(url).json()["revision_is_current"] is True, url + + +def test_a_detail_page_says_when_its_revision_has_been_superseded(client, published): + revision = published.case.revision + revision.is_current = False + revision.save() + + for url in _detail_urls(published): + assert client.get(url).json()["revision_is_current"] is False, url + + def test_a_run_not_yet_approved_is_not_public(client, published): published.submission.status = SubmissionStatus.PENDING_REVIEW published.submission.save() diff --git a/frontend/src/app/(site)/benchmarks/[suite]/page.tsx b/frontend/src/app/(site)/benchmarks/[suite]/page.tsx index e135839..9a360e6 100644 --- a/frontend/src/app/(site)/benchmarks/[suite]/page.tsx +++ b/frontend/src/app/(site)/benchmarks/[suite]/page.tsx @@ -4,7 +4,7 @@ import { notFound } from 'next/navigation' import { BenchmarkSuiteView } from '@/features/benchmarks/benchmark-suite-view' import { getGetBenchmarkSuiteQueryKey } from '@/lib/api/generated/benchmarks/benchmarks' -import { parseBenchmarkParams } from '@/lib/benchmarks/params' +import { isAlternateView, parseBenchmarkParams } from '@/lib/benchmarks/params' import { fetchBenchmarkSuite } from '@/lib/benchmarks/server' import { clientEnv } from '@/lib/env' import { getQueryClient } from '@/lib/query/client' @@ -25,8 +25,10 @@ export async function generateMetadata({ title: `${data.name} benchmark ${data.revision.revision} — leak rates by tool`, description: `${data.leaderboard.length} redaction tools scored on ${data.cases.length} synthetic cases: how many sensitive values each leaves recoverable, with 95% intervals.`, ...canonicalMetadata(`/benchmarks/${suite}`), - // Another revision or the verified-only view is the same page sliced differently. - robots: query.revision || query.scope !== 'all' ? { index: false, follow: true } : undefined, + // An older revision or the verified-only view is the same page sliced differently. + robots: isAlternateView(query, data.revision.is_current) + ? { index: false, follow: true } + : undefined, } } diff --git a/frontend/src/app/(site)/benchmarks/[suite]/tools/[slug]/page.tsx b/frontend/src/app/(site)/benchmarks/[suite]/tools/[slug]/page.tsx index b296cb3..9888cc7 100644 --- a/frontend/src/app/(site)/benchmarks/[suite]/tools/[slug]/page.tsx +++ b/frontend/src/app/(site)/benchmarks/[suite]/tools/[slug]/page.tsx @@ -5,7 +5,7 @@ import { notFound } from 'next/navigation' import { ToolReportView } from '@/features/benchmarks/tool-report-view' import { getGetBenchmarkToolReportQueryKey } from '@/lib/api/generated/benchmarks/benchmarks' import { formatRate } from '@/lib/benchmarks/format' -import { parseBenchmarkParams } from '@/lib/benchmarks/params' +import { isAlternateView, parseBenchmarkParams } from '@/lib/benchmarks/params' import { fetchBenchmarkToolReport } from '@/lib/benchmarks/server' import { getQueryClient } from '@/lib/query/client' import { canonicalMetadata } from '@/lib/seo/canonical' @@ -25,7 +25,9 @@ export async function generateMetadata({ title: `${report.tool.name} redaction benchmark — ${headline ? formatRate(headline.leak_rate) : ''} leak rate`, description: `How many sensitive values ${report.tool.name} leaves recoverable in our ${suite.toUpperCase()} benchmark ${report.revision}, by layer, category and case, with overlays.`, ...canonicalMetadata(`/benchmarks/${suite}/tools/${slug}`), - robots: query.revision || query.scope !== 'all' ? { index: false, follow: true } : undefined, + robots: isAlternateView(query, report.revision_is_current) + ? { index: false, follow: true } + : undefined, } } diff --git a/frontend/src/features/benchmarks/benchmark-suite-view.test.tsx b/frontend/src/features/benchmarks/benchmark-suite-view.test.tsx index f8693ab..29e4b32 100644 --- a/frontend/src/features/benchmarks/benchmark-suite-view.test.tsx +++ b/frontend/src/features/benchmarks/benchmark-suite-view.test.tsx @@ -34,6 +34,15 @@ describe('BenchmarkSuiteView', () => { name: /first page of extraction-conditions-1/i, }) expect(thumbnail.closest('a')).toHaveAttribute( + 'href', + '/benchmarks/pdf/cases/extraction-conditions-1', + ) + }) + + it('names a superseded revision in its case links', () => { + render(makeSuite({ revision: makeRevision({ is_current: false }) })) + + expect(screen.getByRole('link', { name: 'extraction-conditions-1' })).toHaveAttribute( 'href', '/benchmarks/pdf/cases/extraction-conditions-1?revision=v0.1.1', ) @@ -65,12 +74,24 @@ describe('BenchmarkSuiteView', () => { expect(screen.getByText('v0.1.1')).toHaveAttribute('aria-current', 'page') }) + // Viewing an old revision, the way back is the plain page, not a named copy of it. + it('links the current revision by its plain URL', () => { + render( + makeSuite({ + revision: makeRevision({ revision: 'v0.1.0', is_current: false }), + revisions: [makeRevision(), makeRevision({ revision: 'v0.1.0', is_current: false })], + }), + ) + + expect(screen.getByRole('link', { name: 'v0.1.1' })).toHaveAttribute('href', '/benchmarks/pdf') + }) + it('switches between every result and the ones we scored or verified', () => { render() expect(screen.getByRole('link', { name: /verified only/i })).toHaveAttribute( 'href', - '/benchmarks/pdf?revision=v0.1.1&scope=verified', + '/benchmarks/pdf?scope=verified', ) }) diff --git a/frontend/src/features/benchmarks/benchmark-suite-view.tsx b/frontend/src/features/benchmarks/benchmark-suite-view.tsx index 1d3e7aa..ab15a4e 100644 --- a/frontend/src/features/benchmarks/benchmark-suite-view.tsx +++ b/frontend/src/features/benchmarks/benchmark-suite-view.tsx @@ -8,6 +8,7 @@ import { Card, CardDescription, CardTitle } from '@/components/ui/card' import { Skeleton } from '@/components/ui/skeleton' import { useGetBenchmarkSuite } from '@/lib/api/generated/benchmarks/benchmarks' import type { GetBenchmarkSuiteParams } from '@/lib/api/generated/model' +import { benchmarkHref } from '@/lib/benchmarks/params' import { cn } from '@/lib/utils' import { previewAlt } from './case-preview' @@ -30,7 +31,8 @@ export function BenchmarkSuiteView({ if (!data) return const revision = data.revision.revision - const base = `/benchmarks/${suite}?revision=${revision}` + const at = { revision, isCurrent: data.revision.is_current } + const caseHref = (caseId: string) => benchmarkHref(`/benchmarks/${suite}/cases/${caseId}`, at) return (
@@ -67,7 +69,10 @@ export function BenchmarkSuiteView({ ) : ( {r.revision} @@ -77,10 +82,16 @@ export function BenchmarkSuiteView({
@@ -88,6 +99,7 @@ export function BenchmarkSuiteView({ @@ -117,7 +129,7 @@ export function BenchmarkSuiteView({ {item.preview ? ( {/* The page itself: which case is which is easier to see than to read. */} @@ -136,10 +148,7 @@ export function BenchmarkSuiteView({ ) : null} - + {item.case_id} diff --git a/frontend/src/features/benchmarks/case-view.tsx b/frontend/src/features/benchmarks/case-view.tsx index 8fb4bbe..74123e8 100644 --- a/frontend/src/features/benchmarks/case-view.tsx +++ b/frontend/src/features/benchmarks/case-view.tsx @@ -6,6 +6,7 @@ import { Button } from '@/components/ui/button' import { Skeleton } from '@/components/ui/skeleton' import { useGetBenchmarkCase } from '@/lib/api/generated/benchmarks/benchmarks' import type { GetBenchmarkCaseParams } from '@/lib/api/generated/model' +import { benchmarkHref } from '@/lib/benchmarks/params' import { CasePreview } from './case-preview' import { RunCards } from './run-cards' @@ -37,7 +38,13 @@ export function CaseView({

- + {suite.toUpperCase()} benchmark {data.revision}

diff --git a/frontend/src/features/benchmarks/fixtures.ts b/frontend/src/features/benchmarks/fixtures.ts index 9170314..b952e13 100644 --- a/frontend/src/features/benchmarks/fixtures.ts +++ b/frontend/src/features/benchmarks/fixtures.ts @@ -152,6 +152,7 @@ export function makeToolReport(overrides: Partial = {}): ToolRepo tool: row.tool, suite: 'pdf', revision: 'v0.1.1', + revision_is_current: true, scope: 'all', case_count: 4, surfaces: [ @@ -176,7 +177,13 @@ export function makeToolReport(overrides: Partial = {}): ToolRepo } export function makeCaseDetail(overrides: Partial = {}): CaseDetailOut { - return { ...makeCase(), revision: 'v0.1.1', runs: [makeRun()], ...overrides } + return { + ...makeCase(), + revision: 'v0.1.1', + revision_is_current: true, + runs: [makeRun()], + ...overrides, + } } export function makeRunDetail(overrides: Partial = {}): RunDetailOut { @@ -201,6 +208,7 @@ export function makeRunDetail(overrides: Partial = {}): RunDetailO tier: 'Free', notes: '', revision: 'v0.1.1', + revision_is_current: true, ...overrides, } } diff --git a/frontend/src/features/benchmarks/leaderboard-table.test.tsx b/frontend/src/features/benchmarks/leaderboard-table.test.tsx index 297a5d4..abb3fe5 100644 --- a/frontend/src/features/benchmarks/leaderboard-table.test.tsx +++ b/frontend/src/features/benchmarks/leaderboard-table.test.tsx @@ -6,9 +6,15 @@ import { renderWithProviders } from '@/test/render' import { makeRate, makeRow } from './fixtures' import { LeaderboardTable } from './leaderboard-table' -function render(rows = [makeRow()], caseCount = 4) { +function render(rows = [makeRow()], caseCount = 4, isCurrent = true) { return renderWithProviders( - , + , ) } @@ -46,11 +52,20 @@ describe('LeaderboardTable', () => { expect(screen.getByRole('link', { name: 'PDF Redaction' })).toHaveAttribute( 'href', - '/benchmarks/pdf/tools/pdf-redaction?revision=v0.1.1', + '/benchmarks/pdf/tools/pdf-redaction', ) expect(screen.getByText('Hidden Tool').closest('a')).toBeNull() }) + it('names a superseded revision in the report link, since that is another page', () => { + render([makeRow()], 4, false) + + expect(screen.getByRole('link', { name: 'PDF Redaction' })).toHaveAttribute( + 'href', + '/benchmarks/pdf/tools/pdf-redaction?revision=v0.1.1', + ) + }) + it('links the logo wherever the name links, and nowhere for a tool with no page', () => { render([ makeRow(), @@ -65,7 +80,7 @@ describe('LeaderboardTable', () => { .querySelector('img, [data-testid="tool-monogram"]')! expect(logo('pdf-redaction').closest('a')).toHaveAttribute( 'href', - '/benchmarks/pdf/tools/pdf-redaction?revision=v0.1.1', + '/benchmarks/pdf/tools/pdf-redaction', ) expect(logo('hidden-tool').closest('a')).toBeNull() }) diff --git a/frontend/src/features/benchmarks/leaderboard-table.tsx b/frontend/src/features/benchmarks/leaderboard-table.tsx index 65a1463..0cedd40 100644 --- a/frontend/src/features/benchmarks/leaderboard-table.tsx +++ b/frontend/src/features/benchmarks/leaderboard-table.tsx @@ -13,6 +13,7 @@ import { import { ToolLogo } from '@/features/catalog/tool-logo' import type { LeaderboardRowOut } from '@/lib/api/generated/model' import { formatPercent, roleLabel, surfaceLabel } from '@/lib/benchmarks/format' +import { benchmarkHref } from '@/lib/benchmarks/params' import { ProvenanceBadge } from './provenance-badge' import { RateCell } from './rate-cell' @@ -24,15 +25,18 @@ import { RateCell } from './rate-cell' export function LeaderboardTable({ suite, revision, + isCurrent, rows, caseCount, }: { suite: string revision: string + isCurrent: boolean rows: LeaderboardRowOut[] caseCount: number }) { - const reportHref = (slug: string) => `/benchmarks/${suite}/tools/${slug}?revision=${revision}` + const reportHref = (slug: string) => + benchmarkHref(`/benchmarks/${suite}/tools/${slug}`, { revision, isCurrent }) if (rows.length === 0) { return ( diff --git a/frontend/src/features/benchmarks/run-and-case-views.test.tsx b/frontend/src/features/benchmarks/run-and-case-views.test.tsx index 5ffb4f4..e1bc7b4 100644 --- a/frontend/src/features/benchmarks/run-and-case-views.test.tsx +++ b/frontend/src/features/benchmarks/run-and-case-views.test.tsx @@ -46,6 +46,32 @@ function renderRun(run = makeRunDetail(), { staff = false } = {}) { } describe('RunReportView', () => { + it('links the leaderboard, the tool report and the case by their plain URLs', () => { + renderRun() + + expect(screen.getByRole('link', { name: /pdf benchmark v0\.1\.1/i })).toHaveAttribute( + 'href', + '/benchmarks/pdf', + ) + expect(screen.getByRole('link', { name: 'PDF Redaction' })).toHaveAttribute( + 'href', + '/benchmarks/pdf/tools/pdf-redaction', + ) + expect(screen.getByRole('link', { name: /the case, and every tool/i })).toHaveAttribute( + 'href', + '/benchmarks/pdf/cases/extraction-conditions-1', + ) + }) + + it('names a superseded revision in every link it makes', () => { + renderRun(makeRunDetail({ revision_is_current: false })) + + expect(screen.getByRole('link', { name: /the case, and every tool/i })).toHaveAttribute( + 'href', + '/benchmarks/pdf/cases/extraction-conditions-1?revision=v0.1.1', + ) + }) + it('leads with the run and who published it', () => { renderRun() @@ -146,6 +172,15 @@ describe('CaseView', () => { ) } + it('links back to the leaderboard by its plain URL when the revision is current', () => { + renderCase() + + expect(screen.getByRole('link', { name: /pdf benchmark v0\.1\.1/i })).toHaveAttribute( + 'href', + '/benchmarks/pdf', + ) + }) + it('offers the case PDF and says what is in it, without the values', () => { renderCase() diff --git a/frontend/src/features/benchmarks/run-report-view.tsx b/frontend/src/features/benchmarks/run-report-view.tsx index 2dad264..7510b9e 100644 --- a/frontend/src/features/benchmarks/run-report-view.tsx +++ b/frontend/src/features/benchmarks/run-report-view.tsx @@ -11,6 +11,7 @@ import { } from '@/lib/api/generated/benchmarks/benchmarks' import type { PooledOut, RateOut } from '@/lib/api/generated/model' import { roleLabel, surfaceLabel } from '@/lib/benchmarks/format' +import { benchmarkHref } from '@/lib/benchmarks/params' import { useQueryClient } from '@tanstack/react-query' @@ -35,6 +36,7 @@ export function RunReportView({ suite, runId }: { suite: string; runId: string } if (!run) return const report = run.report as Report + const at = { revision: run.revision, isCurrent: run.revision_is_current } const gates = report.survivability?.gates ?? [] // One run's report reads like a pooled one of one: the same sections, n of one case. const summary: PooledOut = { @@ -65,14 +67,14 @@ export function RunReportView({ suite, runId }: { suite: string; runId: string }

- + {suite.toUpperCase()} benchmark {run.revision} {run.tool.listable ? ( <> {' / '} {run.tool.name} @@ -132,7 +134,7 @@ export function RunReportView({ suite, runId }: { suite: string; runId: string } ) : null} The case, and every tool on it diff --git a/frontend/src/features/benchmarks/tool-report-view.test.tsx b/frontend/src/features/benchmarks/tool-report-view.test.tsx index dbba154..6ddd025 100644 --- a/frontend/src/features/benchmarks/tool-report-view.test.tsx +++ b/frontend/src/features/benchmarks/tool-report-view.test.tsx @@ -23,6 +23,24 @@ function render(report: ToolReportOut = makeToolReport()) { } describe('ToolReportView', () => { + it('links back to the leaderboard by its plain URL when the revision is current', () => { + render() + + expect(screen.getByRole('link', { name: /pdf benchmark v0\.1\.1/i })).toHaveAttribute( + 'href', + '/benchmarks/pdf', + ) + }) + + it('names a superseded revision in the way back', () => { + render(makeToolReport({ revision_is_current: false })) + + expect(screen.getByRole('link', { name: /pdf benchmark v0\.1\.1/i })).toHaveAttribute( + 'href', + '/benchmarks/pdf?revision=v0.1.1', + ) + }) + it('leads with the pooled verdict, its interval and its sample size', () => { render() diff --git a/frontend/src/features/benchmarks/tool-report-view.tsx b/frontend/src/features/benchmarks/tool-report-view.tsx index 12fe33c..6ef9c23 100644 --- a/frontend/src/features/benchmarks/tool-report-view.tsx +++ b/frontend/src/features/benchmarks/tool-report-view.tsx @@ -7,6 +7,7 @@ import { ToolLogo } from '@/features/catalog/tool-logo' import { useGetBenchmarkToolReport } from '@/lib/api/generated/benchmarks/benchmarks' import type { GetBenchmarkToolReportParams } from '@/lib/api/generated/model' import { surfaceLabel } from '@/lib/benchmarks/format' +import { benchmarkHref } from '@/lib/benchmarks/params' import { type Breakdowns, ReportBreakdowns, ReportHero } from './report-sections' import { RunCards } from './run-cards' @@ -28,7 +29,13 @@ export function ToolReportView({

- + {suite.toUpperCase()} benchmark {data.revision}

diff --git a/frontend/src/lib/api/generated/model/caseDetailOut.ts b/frontend/src/lib/api/generated/model/caseDetailOut.ts index e9ed7b8..97048c5 100644 --- a/frontend/src/lib/api/generated/model/caseDetailOut.ts +++ b/frontend/src/lib/api/generated/model/caseDetailOut.ts @@ -18,5 +18,6 @@ export interface CaseDetailOut { pdf_url: string; preview: PreviewOut | null; revision: string; + revision_is_current: boolean; runs: RunOut[]; } diff --git a/frontend/src/lib/api/generated/model/runDetailOut.ts b/frontend/src/lib/api/generated/model/runDetailOut.ts index d01b891..4e44a81 100644 --- a/frontend/src/lib/api/generated/model/runDetailOut.ts +++ b/frontend/src/lib/api/generated/model/runDetailOut.ts @@ -45,4 +45,5 @@ export interface RunDetailOut { tier: string; notes: string; revision: string; + revision_is_current: boolean; } diff --git a/frontend/src/lib/api/generated/model/toolReportOut.ts b/frontend/src/lib/api/generated/model/toolReportOut.ts index 440e2bc..38ded62 100644 --- a/frontend/src/lib/api/generated/model/toolReportOut.ts +++ b/frontend/src/lib/api/generated/model/toolReportOut.ts @@ -13,6 +13,7 @@ export interface ToolReportOut { tool: ToolRefOut; suite: string; revision: string; + revision_is_current: boolean; scope: ToolReportOutScope; case_count: number; surfaces: ToolSurfaceOut[]; diff --git a/frontend/src/lib/benchmarks/params.test.ts b/frontend/src/lib/benchmarks/params.test.ts index 90e2c17..9a9530e 100644 --- a/frontend/src/lib/benchmarks/params.test.ts +++ b/frontend/src/lib/benchmarks/params.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest' -import { parseBenchmarkParams } from './params' +import { benchmarkHref, isAlternateView, parseBenchmarkParams } from './params' describe('parseBenchmarkParams', () => { it('defaults to the current revision and every result', () => { @@ -21,3 +21,44 @@ describe('parseBenchmarkParams', () => { }) }) }) + +describe('benchmarkHref', () => { + // `?revision=` is the clean page under a second, noindexed URL. Linking + // it sent crawlers to the copy and left the canonical page unlinked. + it('leaves the current revision out of the URL', () => { + expect( + benchmarkHref('/benchmarks/pdf/tools/kept', { revision: 'v0.1.1', isCurrent: true }), + ).toBe('/benchmarks/pdf/tools/kept') + }) + + it('names a superseded revision, which is a different page', () => { + expect( + benchmarkHref('/benchmarks/pdf/tools/kept', { revision: 'v0.1.0', isCurrent: false }), + ).toBe('/benchmarks/pdf/tools/kept?revision=v0.1.0') + }) + + it('adds the verified scope after the revision', () => { + expect( + benchmarkHref('/benchmarks/pdf', { revision: 'v0.1.1', isCurrent: true, scope: 'verified' }), + ).toBe('/benchmarks/pdf?scope=verified') + expect( + benchmarkHref('/benchmarks/pdf', { revision: 'v0.1.0', isCurrent: false, scope: 'verified' }), + ).toBe('/benchmarks/pdf?revision=v0.1.0&scope=verified') + }) +}) + +describe('isAlternateView', () => { + it('treats the plain page as the one to index', () => { + expect(isAlternateView({ scope: 'all' }, true)).toBe(false) + }) + + // Old links and shares still carry it; the canonical folds it into the plain page. + it('does not treat the current revision named in the URL as another view', () => { + expect(isAlternateView({ revision: 'v0.1.1', scope: 'all' }, true)).toBe(false) + }) + + it('treats a superseded revision and the verified-only view as other views', () => { + expect(isAlternateView({ revision: 'v0.1.0', scope: 'all' }, false)).toBe(true) + expect(isAlternateView({ scope: 'verified' }, true)).toBe(true) + }) +}) diff --git a/frontend/src/lib/benchmarks/params.ts b/frontend/src/lib/benchmarks/params.ts index 50e7876..547cbed 100644 --- a/frontend/src/lib/benchmarks/params.ts +++ b/frontend/src/lib/benchmarks/params.ts @@ -16,3 +16,34 @@ export function parseBenchmarkParams(searchParams: SearchParams): BenchmarkParam const scope = first(searchParams.scope) === 'verified' ? 'verified' : 'all' return revision ? { revision, scope } : { scope } } + +/** + * A link to a benchmark page that names its revision only when it has to. + * + * Every page without `?revision=` already shows the current revision, and a page + * that names one is noindexed as a slice of it. So linking the current revision by + * name sends crawlers to the noindexed copy and leaves the canonical page unlinked. + */ +export function benchmarkHref( + path: string, + { + revision, + isCurrent, + scope = 'all', + }: { revision: string; isCurrent: boolean; scope?: BenchmarkParams['scope'] }, +): string { + const query = new URLSearchParams() + if (!isCurrent) query.set('revision', revision) + if (scope === 'verified') query.set('scope', scope) + const search = query.toString() + return search ? `${path}?${search}` : path +} + +/** + * Whether a page is a slice of its canonical one, and so followed but not indexed: + * a superseded revision, or the verified-only view. Naming the current revision is + * not a slice - it is the same page, and the canonical says so. + */ +export function isAlternateView(params: BenchmarkParams, revisionIsCurrent: boolean): boolean { + return (Boolean(params.revision) && !revisionIsCurrent) || params.scope !== 'all' +}