diff --git a/docs/checks/observability.md b/docs/checks/observability.md index 9e7f968..269c98d 100644 --- a/docs/checks/observability.md +++ b/docs/checks/observability.md @@ -58,7 +58,7 @@ Multi-locale sites usually list only one language in `llms.txt`, so localized si - When locale prefixes cover most of the sitemap, the prefix position is detected directly. - When translations are a minority of the sitemap (an unprefixed default language plus a few translated locales), the check confirms the prefixes by structural duplication: stripping a locale code must produce paths that match unprefixed URLs. -- `--doc-locale ` (or `preferredLocale` in config) overrides detection. If the sitemap has no URLs prefixed with that locale, it is treated as the unprefixed default and the localized variants are excluded. +- `--doc-locale ` (or `preferredLocale` in config) overrides detection. A regional or script variant of the same language counts as a match (`en` selects `/en-us/`). If the sitemap has no URLs for that language at all, it is treated as the unprefixed default and the localized variants are excluded. ### How to fix diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 3acdc12..383a25e 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -151,6 +151,8 @@ The resolution order for both flags is: Use the flags when the base URL doesn't contain locale or version segments but the site organizes content by locale or version. +A locale preference also matches regional and script variants of the same language when the exact code is absent: `en` selects `en-us` pages and sitemaps, and `--doc-locale en-US` selects bare `en` ones. When several variants exist, the one with the most pages is used; sitemap indexes contribute the child sitemaps of every variant of that language so the choice is made over pages, not over sitemap counts. + ```bash # Prefer French locale during discovery afdocs check https://docs.example.com --doc-locale fr diff --git a/src/checks/observability/llms-txt-coverage.ts b/src/checks/observability/llms-txt-coverage.ts index 953a133..6368e14 100644 --- a/src/checks/observability/llms-txt-coverage.ts +++ b/src/checks/observability/llms-txt-coverage.ts @@ -5,7 +5,12 @@ import { } from '../../helpers/get-page-urls.js'; import { isSameSite } from '../../helpers/host-equivalence.js'; import { isNonPageUrl } from '../../helpers/to-md-urls.js'; -import { isLocaleSegment, detectLocalePosition } from '../../helpers/locale-codes.js'; +import { + isLocaleSegment, + detectLocalePosition, + countLocaleCodesAt, + resolveLocaleVariant, +} from '../../helpers/locale-codes.js'; import { DEFAULT_COVERAGE_PASS_THRESHOLD, DEFAULT_COVERAGE_WARN_THRESHOLD, @@ -324,12 +329,25 @@ async function check(ctx: CheckContext): Promise { if (preferred) { // Explicit locale (--doc-locale / config preferredLocale) wins over // inference from llms.txt URLs. - const byPreferred = filterByLocale(scopedSitemapUrls, preferred, localePosition); + // Exact raw prefix first, as the page filter does, so a site-specific + // code such as `en-custom` is not displaced by its `en-us` variant. Only + // when absent does a regional or script variant of the same language + // stand in (`--doc-locale en` selects `/en-us/`, issue #164). + const exact = filterByLocale(scopedSitemapUrls, preferred, localePosition); + const resolved = + exact.length > 0 + ? preferred + : (resolveLocaleVariant( + preferred, + countLocaleCodesAt(scopedSitemapUrls, localePosition), + ) ?? preferred); + const byPreferred = + exact.length > 0 ? exact : filterByLocale(scopedSitemapUrls, resolved, localePosition); const before = scopedSitemapUrls.length; if (byPreferred.length > 0) { scopedSitemapUrls = byPreferred; localeFiltered = scopedSitemapUrls.length < before; - if (localeFiltered) detectedLocale = preferred; + if (localeFiltered) detectedLocale = resolved; } else { // No URLs carry the preferred locale as a prefix, so it must be the // unprefixed default locale: drop the locale-prefixed translations. diff --git a/src/helpers/get-page-urls.ts b/src/helpers/get-page-urls.ts index f21e564..ea4474c 100644 --- a/src/helpers/get-page-urls.ts +++ b/src/helpers/get-page-urls.ts @@ -4,7 +4,13 @@ import { getLlmsTxtFilesForAnalysis, selectCanonicalLlmsTxt } from './llms-txt.j import { isNonPageUrl, isMdUrl, toHtmlUrl } from './to-md-urls.js'; import { isSameSite, canonicalHost } from './host-equivalence.js'; import { fetchPage } from './fetch-page.js'; -import { isLocaleSegment, detectLocalePosition } from './locale-codes.js'; +import { + isLocaleSegment, + detectLocalePosition, + countLocaleCodesAt, + resolveLocaleVariant, + sameLanguageVariants, +} from './locale-codes.js'; import type { CheckContext, DiscoveredFile, UrlPathPattern } from '../types.js'; /** @@ -455,6 +461,13 @@ export function filterLocaleSitemaps( preferredLocale?: string | null, ): string[] { if (subSitemapUrls.length < 2) return subSitemapUrls; + // Distinct children only, so a repeated index entry is fetched once. + subSitemapUrls = [...new Set(subSitemapUrls)]; + // An explicit preference the locale helper does not recognize (`en-custom`) + // cannot be matched to a child by code, and resolving it by language would + // keep the wrong children and drop others. Keep every child so the + // page-level exact-prefix filter can decide (issue #164). + if (preferredLocale && !isLocaleSegment(preferredLocale)) return subSitemapUrls; // Locale shape shared by all patterns: language[-Script][-REGION], e.g. // `en`, `fr-fr`, `zh-hans`, `zh-hans-cn`. Fixing the subtag order keeps a @@ -517,9 +530,17 @@ export function filterLocaleSitemaps( // Need at least 2 distinct locale codes to consider this a locale-organized index if (locales.size < 2) return subSitemapUrls; - // Prefer the user's locale, then 'en', then any non-locale sitemaps - const locale = preferredLocale?.toLowerCase(); - const preferred = (locale && locales.get(locale)) ?? locales.get('en') ?? []; + // Prefer the user's locale, then 'en', then any non-locale sitemaps. An + // exact code selects its sitemaps alone. Otherwise every same-language + // variant is kept (`en` keeps `en-us` and `en-gb` shards, issue #164): + // sitemap counts are a poor proxy for page counts, and the page-level + // refinement, which also sees llms.txt, is the one place that picks a + // single variant. Fetching a variant it rejects costs bounded budget; + // skipping the one it picks loses pages. + const counts = new Map([...locales].map(([code, list]) => [code, list.length])); + const matched = preferredLocale ? sameLanguageVariants(preferredLocale, counts) : []; + const selected = matched.length > 0 ? matched : sameLanguageVariants('en', counts); + const preferred = selected.flatMap((code) => locales.get(code) ?? []); return [...preferred, ...nonLocale].length > 0 ? [...preferred, ...nonLocale] : subSitemapUrls; } @@ -555,19 +576,24 @@ export function filterLocalizedUrls(urls: string[], preferredLocale?: string | n // unprefixed fallback below (issue #158). Compare raw segments rather than // the detection counts so that an explicit preference the locale helper does // not recognize (e.g. a site-specific code such as `latam`) still wins. - const hasTargetLocale = urls.some((url) => { + const hasExactTarget = urls.some((url) => { try { return segmentAtLocalePosition(url) === targetLocale; } catch { return false; } }); + // Without an exact match, a regional or script variant of the same language + // stands in for the target (`en` selects `/en-us/` pages, issue #164). + const effectiveTarget = hasExactTarget + ? targetLocale + : resolveLocaleVariant(targetLocale, countLocaleCodesAt(urls, localePosition)); - if (hasTargetLocale) { + if (effectiveTarget) { return urls.filter((url) => { try { const seg = segmentAtLocalePosition(url); - return seg === null || seg === targetLocale; // keep URLs without enough segments + return seg === null || seg === effectiveTarget; // keep URLs without enough segments } catch { return true; } @@ -808,9 +834,17 @@ export interface SitemapOptions { maxUrls?: number; sitemapUrls?: string[]; originOverride?: string; - pathFilterBase?: string; + /** One or more base URLs; a URL is kept when it falls under any of them. */ + pathFilterBase?: string | string[]; /** Skip URL-level locale/version refinement. Use when the caller needs raw URLs (e.g. coverage check). */ skipRefinement?: boolean; + /** + * Return path-scoped URLs without locale/version refinement or the maxUrls + * cap so the caller can refine a pool merged from several sources. Unlike + * `skipRefinement`, child-sitemap selection and the collection limit behave + * as they do for refined discovery. + */ + deferRefinement?: boolean; } export async function getUrlsFromSitemap( @@ -818,14 +852,24 @@ export async function getUrlsFromSitemap( warnings: string[], opts: SitemapOptions = {}, ): Promise { - const { maxUrls = MAX_SITEMAP_URLS, originOverride, pathFilterBase, skipRefinement } = opts; + const { + maxUrls = MAX_SITEMAP_URLS, + originOverride, + pathFilterBase, + skipRefinement, + deferRefinement, + } = opts; const sitemapUrls = opts.sitemapUrls ?? (await discoverSitemapUrls(ctx, originOverride)); const urls: string[] = []; const matchOrigin = originOverride ?? ctx.origin; - // Pre-compute the path prefix so we can filter before counting against the cap. - // When pathFilterBase is provided, only URLs under that prefix consume cap slots. - const prefixPath = pathFilterBase ? new URL(pathFilterBase).pathname.replace(/\/$/, '') : ''; + // Pre-compute the path prefixes so we can filter before counting against the cap. + // When pathFilterBase is provided, only URLs under one of those prefixes consume + // cap slots. The first prefix is the primary scope used in warnings. + const prefixPaths = ( + Array.isArray(pathFilterBase) ? pathFilterBase : pathFilterBase ? [pathFilterBase] : [] + ).map((base) => new URL(base).pathname.replace(/\/$/, '')); + const prefixPath = prefixPaths[0] ?? ''; let examinedUrls = 0; let matchingUrls = 0; const budget: SitemapBudget = { fetches: 0, bytes: 0 }; @@ -847,6 +891,10 @@ export async function getUrlsFromSitemap( return fetchSitemap(ctx, sitemapUrl, warnings, budget); } + // Distinct URLs only. A page repeated within a sitemap or listed by two + // shards would otherwise count twice in the per-variant totals that choose + // a locale (issue #164) and in the prefix-coverage counts. + const seen = new Set(); function shouldInclude(url: string): boolean { try { new URL(url); @@ -854,8 +902,12 @@ export async function getUrlsFromSitemap( return false; } if (!isSameSite(url, matchOrigin)) return false; + if (seen.has(url)) return false; + seen.add(url); examinedUrls++; - if (prefixPath && !matchesPathPrefix(url, prefixPath)) return false; + if (prefixPaths.length > 0 && !prefixPaths.some((p) => matchesPathPrefix(url, p))) { + return false; + } matchingUrls++; return true; } @@ -869,6 +921,7 @@ export async function getUrlsFromSitemap( if (urls.length >= collectLimit) break; const parsed = await fetchWithinBudget(sitemapUrl); + if (!parsed) break; // Add direct URLs (filtered to same origin and path prefix) @@ -879,12 +932,29 @@ export async function getUrlsFromSitemap( } } - // Follow one level of sitemap index, filtering to default locale if detected + // Follow one level of sitemap index, filtering to the preferred locale + // when one is known. A raw coverage walk (skipRefinement) with no + // preference keeps every child: coverage infers the locale from llms.txt + // afterwards, and choosing a default variant here (`en-gb` over `en-us`) + // would discard the children it ends up needing (issue #164). The fetch + // and byte budgets still bound the walk. if (parsed.sitemapIndexUrls.length > 0 && urls.length < collectLimit) { - const filteredSubSitemaps = filterLocaleSitemaps( - parsed.sitemapIndexUrls, - ctx.options.preferredLocale ?? extractLocaleFromUrl(ctx.baseUrl), - ); + const knownLocale = ctx.options.preferredLocale ?? extractLocaleFromUrl(ctx.baseUrl); + // Fallback path scopes (issue #164) need their own children too: with + // scopes /en-gb and /en-us and children {en-au, en-us}, the preferred + // locale alone would resolve to en-au, whose pages neither scope keeps. + const scopeLocales = prefixPaths + .slice(1) + .map((p) => extractLocaleFromUrl(`${matchOrigin}${p}`)) + .filter((l): l is string => l !== null); + const filteredSubSitemaps = + skipRefinement && !knownLocale + ? parsed.sitemapIndexUrls + : [knownLocale, ...scopeLocales].reduce( + (kept, locale) => + mergeUrlSets(kept, filterLocaleSitemaps(parsed.sitemapIndexUrls, locale)), + [], + ); for (const subSitemapUrl of filteredSubSitemaps) { if (urls.length >= collectLimit) break; @@ -903,7 +973,7 @@ export async function getUrlsFromSitemap( warnOnNarrowPathPrefix(warnings, 'Sitemap', prefixPath, matchingUrls, examinedUrls); - if (skipRefinement) return urls; + if (skipRefinement || deferRefinement) return urls; const localeFiltered = filterLocalizedUrls( urls, @@ -950,7 +1020,8 @@ export function getPathFilterBase(ctx: CheckContext): string { * * For example, base URL `example.com/en/docs` with `--locale de` rewrites * to `example.com/de/docs` so that path prefix filtering matches the - * preferred locale's URL space. + * preferred locale's URL space. See `pathFilterBases` for the same-language + * case, where the original base is kept as a second scope. */ function rewriteFilterBase( filterBase: string, @@ -1034,6 +1105,37 @@ function mergeUrlSets(primary: string[], secondary: string[]): string[] { return merged; } +/** + * Path scopes for discovery, primary first. + * + * The primary scope is the base URL with the preferred locale and version + * substituted. When the preference is a same-language variant of the base + * URL's locale (`en-GB` over `/en-us/`, `en-US` over `/en/`), the original + * locale path is kept as a second scope. Scoping to the rewritten path alone + * would discard every page on a site that lacks the exact variant; scoping to + * the original alone would keep the base variant even when the requested one + * exists. With both in scope, `filterLocalizedUrls` takes the exact variant + * when present and resolves to the base variant otherwise (issue #164). + * A different language never falls back to the base locale. + */ +function pathFilterBases(ctx: CheckContext): string[] { + const original = getPathFilterBase(ctx); + const { preferredLocale, preferredVersion } = ctx.options; + const primary = rewriteFilterBase(original, preferredLocale, preferredVersion); + // Variant resolution only applies to recognized locale codes. A + // site-specific preference such as `en-custom` scopes to its own path + // alone, as it did before variants were resolved. + if (!preferredLocale || !isLocaleSegment(preferredLocale)) return [primary]; + + const detected = extractLocaleFromUrl(original); + if (!detected || detected === preferredLocale.toLowerCase()) return [primary]; + const sameLanguage = resolveLocaleVariant(preferredLocale, new Map([[detected, 1]])) !== null; + if (!sameLanguage) return [primary]; + + const secondary = rewriteFilterBase(original, undefined, preferredVersion); + return secondary === primary ? [primary] : [primary, secondary]; +} + /** * Discover page URLs from llms.txt links, sitemap, or fall back to baseUrl. * @@ -1055,11 +1157,8 @@ export async function getPageUrls(ctx: CheckContext): Promise { const locale = ctx.options.preferredLocale ?? extractLocaleFromUrl(ctx.baseUrl); const version = ctx.options.preferredVersion ?? extractVersionFromUrl(ctx.baseUrl); - const filterBase = rewriteFilterBase( - getPathFilterBase(ctx), - ctx.options.preferredLocale, - ctx.options.preferredVersion, - ); + const filterBases = pathFilterBases(ctx); + const filterPaths = filterBases.map((base) => new URL(base).pathname.replace(/\/$/, '')); /** Apply locale and version filtering to a discovered URL set. */ function refineUrls(urls: string[]): string[] { @@ -1067,16 +1166,11 @@ export async function getPageUrls(ctx: CheckContext): Promise { return deduplicateVersionedUrls(localeFiltered, version); } + /** Path-scope llms.txt URLs without refining them, warning on a narrow prefix. */ function scopeLlmsTxtUrls(urls: string[]): string[] { - const scoped = filterByPathPrefix(urls, filterBase); - warnOnNarrowPathPrefix( - warnings, - 'llms.txt', - new URL(filterBase).pathname.replace(/\/$/, ''), - scoped.length, - urls.length, - ); - return refineUrls(scoped); + const scoped = urls.filter((url) => filterPaths.some((p) => matchesPathPrefix(url, p))); + warnOnNarrowPathPrefix(warnings, 'llms.txt', filterPaths[0], scoped.length, urls.length); + return scoped; } /** Filter the originalMdUrls map to a subset of URLs. */ @@ -1094,13 +1188,15 @@ export async function getPageUrls(ctx: CheckContext): Promise { // 1. Try llms.txt links from cached results (if llms-txt-exists ran) const cached = await getUrlsFromCachedLlmsTxtWithOriginals(ctx); - let llmsTxtUrls = scopeLlmsTxtUrls(cached.pageUrls); + let scopedLlmsTxtUrls = scopeLlmsTxtUrls(cached.pageUrls); + let llmsTxtUrls = refineUrls(scopedLlmsTxtUrls); let originalMdUrls = cached.originalMdUrls; // 2. Try fetching llms.txt directly (standalone mode, llms-txt-exists didn't run) if (llmsTxtUrls.length === 0 && !ctx.previousResults.has('llms-txt-exists')) { const fetched = await fetchLlmsTxtUrls(ctx); - llmsTxtUrls = scopeLlmsTxtUrls(fetched.pageUrls); + scopedLlmsTxtUrls = scopeLlmsTxtUrls(fetched.pageUrls); + llmsTxtUrls = refineUrls(scopedLlmsTxtUrls); originalMdUrls = fetched.originalMdUrls; } @@ -1114,14 +1210,33 @@ export async function getPageUrls(ctx: CheckContext): Promise { } // llms.txt is thin — try sitemap to fill the gap - const sitemapUrls = await getUrlsFromSitemap(ctx, warnings, { pathFilterBase: filterBase }); + // Merge the path-scoped pools *before* refining. Refining each source on + // its own lets them disagree about the locale variant (llms.txt lists + // only /en-us/, the sitemap also has the requested /en-gb/) and discards + // pages of the winning variant that only the other source listed. One + // refinement over the merged pool keeps every page of the one variant + // that wins, with the exact requested variant first (issue #164). + const sitemapUrls = await getUrlsFromSitemap(ctx, warnings, { + pathFilterBase: filterBases, + deferRefinement: true, + }); if (sitemapUrls.length > 0) { sources.push('sitemap'); + // Refine once over the merged pool, then cap only the sitemap additions: + // llms.txt pages that survive refinement are kept in full, since + // maxLinksToTest has no upper bound and the 500-URL cap belongs to the + // sitemap contribution alone, as it did before the sources were merged. + const refined = refineUrls(mergeUrlSets(scopedLlmsTxtUrls, sitemapUrls)); + const fromLlmsTxt = new Set(scopedLlmsTxtUrls); + let sitemapAdditions = 0; + const merged = refined.filter( + (url) => fromLlmsTxt.has(url) || sitemapAdditions++ < MAX_SITEMAP_URLS, + ); return { - urls: mergeUrlSets(llmsTxtUrls, sitemapUrls), + urls: merged, warnings, sources, - originalMdUrls: filteredOriginals, + originalMdUrls: filterOriginalMdUrls(originalMdUrls, merged), }; } @@ -1130,7 +1245,7 @@ export async function getPageUrls(ctx: CheckContext): Promise { } // 3. Try sitemap (path, locale, and version filtering applied inside) - const sitemapUrls = await getUrlsFromSitemap(ctx, warnings, { pathFilterBase: filterBase }); + const sitemapUrls = await getUrlsFromSitemap(ctx, warnings, { pathFilterBase: filterBases }); if (sitemapUrls.length > 0) { sources.push('sitemap'); return { urls: sitemapUrls, warnings, sources }; diff --git a/src/helpers/locale-codes.ts b/src/helpers/locale-codes.ts index ff1df0a..d58fd67 100644 --- a/src/helpers/locale-codes.ts +++ b/src/helpers/locale-codes.ts @@ -251,6 +251,77 @@ export function isLocaleSegment(segment: string): boolean { return ISO_15924.has(parts[1]) && isRegionSubtag(parts[2]); } +/** + * Count the locale codes that appear at `position` across a URL set, keyed by + * lowercased code. Segments that are not locale codes are ignored. + */ +export function countLocaleCodesAt(urls: string[], position: number): Map { + const counts = new Map(); + for (const url of urls) { + try { + const segments = new URL(url).pathname.split('/').filter(Boolean); + if (segments.length <= position) continue; + const seg = segments[position].toLowerCase(); + if (isLocaleSegment(seg)) counts.set(seg, (counts.get(seg) ?? 0) + 1); + } catch { + continue; + } + } + return counts; +} + +function scriptOf(code: string): string | undefined { + return code + .split('-') + .slice(1) + .find((part) => ISO_15924.has(part)); +} + +/** + * Codes in `available` that can stand in for `preferred`: the exact code if + * present, otherwise every code with the same language whose script agrees + * when both specify one (`en` → `en-us`, `en-gb`; `zh-hans` → `zh-hans-cn`, + * never `zh-hant`). Ranked by count, then alphabetically, so the first entry + * is the best-represented variant. Empty when no code shares the language. + */ +export function sameLanguageVariants(preferred: string, available: Map): string[] { + const target = preferred.toLowerCase(); + if (available.has(target)) return [target]; + + const language = target.split('-')[0]; + const targetScript = scriptOf(target); + return [...available] + .filter(([code]) => { + if (code.split('-')[0] !== language) return false; + const script = scriptOf(code); + return !(targetScript && script && script !== targetScript); + }) + .sort(([codeA, countA], [codeB, countB]) => countB - countA || (codeA < codeB ? -1 : 1)) + .map(([code]) => code); +} + +/** + * Resolve a preferred locale against the codes a site actually uses. + * + * An exact match wins. Otherwise the best-represented code with the same + * language is chosen, provided the scripts agree when both specify one: + * `en` → `en-us`, `en-us` → `en`, `zh-hans` → `zh-hans-cn`, but never + * `zh-hans` → `zh-hant`. Ties break alphabetically so the choice is stable. + * Returns null when no code shares the language, which lets callers fall + * back to the unprefixed default (issue #164). + * + * One variant is returned rather than every same-language variant so that + * regional duplicates (`en-us` plus `en-gb`) do not both enter a sample. + * Callers that only *collect* candidates, such as child-sitemap selection, + * use `sameLanguageVariants` so the single page-level pass decides. + */ +export function resolveLocaleVariant( + preferred: string, + available: Map, +): string | null { + return sameLanguageVariants(preferred, available)[0] ?? null; +} + /** * Check whether a single locale code at `position` represents a real locale * prefix by testing for structural duplication: if stripping the code from diff --git a/test/unit/checks/llms-txt-coverage.test.ts b/test/unit/checks/llms-txt-coverage.test.ts index 56e2b7d..916a013 100644 --- a/test/unit/checks/llms-txt-coverage.test.ts +++ b/test/unit/checks/llms-txt-coverage.test.ts @@ -908,6 +908,137 @@ describe('llms-txt-coverage', () => { expect(result.details?.detectedLocale).toBe('fr'); }); + test('resolves preferredLocale to a regional variant when no exact prefix exists (issue #164)', async () => { + const host = 'preferred-variant.local'; + const enUsPages = [ + `http://${host}/docs/en-us/getting-started`, + `http://${host}/docs/en-us/api-reference`, + `http://${host}/docs/en-us/guides`, + ]; + const sitemapPages = [ + ...enUsPages, + `http://${host}/docs/fr-fr/getting-started`, + `http://${host}/docs/fr-fr/api-reference`, + `http://${host}/docs/fr-fr/guides`, + ]; + + const ctx = makeCtx(host, enUsPages, '/docs', { preferredLocale: 'en' }); + + server.use( + http.get( + `http://${host}/robots.txt`, + () => new HttpResponse(`Sitemap: http://${host}/sitemap.xml`, { status: 200 }), + ), + http.get( + `http://${host}/sitemap.xml`, + () => + new HttpResponse(makeSitemap(sitemapPages), { + headers: { 'content-type': 'application/xml' }, + }), + ), + ); + + const result = await check.run(ctx); + expect(result.status).toBe('pass'); + expect(result.details?.sitemapDocPages).toBe(3); + expect(result.details?.localeFiltered).toBe(true); + expect(result.details?.detectedLocale).toBe('en-us'); + }); + + test('prefers an exact custom locale prefix over a same-language variant', async () => { + const host = 'preferred-custom.local'; + const customPages = [ + `http://${host}/docs/en-custom/getting-started`, + `http://${host}/docs/en-custom/api-reference`, + `http://${host}/docs/en-custom/guides`, + ]; + const sitemapPages = [ + ...customPages, + `http://${host}/docs/en-us/getting-started`, + `http://${host}/docs/en-us/api-reference`, + `http://${host}/docs/en-us/guides`, + `http://${host}/docs/fr-fr/getting-started`, + `http://${host}/docs/fr-fr/api-reference`, + `http://${host}/docs/fr-fr/guides`, + ]; + + const ctx = makeCtx(host, customPages, '/docs', { preferredLocale: 'en-custom' }); + + server.use( + http.get( + `http://${host}/robots.txt`, + () => new HttpResponse(`Sitemap: http://${host}/sitemap.xml`, { status: 200 }), + ), + http.get( + `http://${host}/sitemap.xml`, + () => + new HttpResponse(makeSitemap(sitemapPages), { + headers: { 'content-type': 'application/xml' }, + }), + ), + ); + + const result = await check.run(ctx); + expect(result.status).toBe('pass'); + expect(result.details?.sitemapDocPages).toBe(3); + expect(result.details?.coverageRate).toBe(100); + expect(result.details?.detectedLocale).toBe('en-custom'); + }); + + test('keeps every locale child sitemap in the raw walk when no locale is known (issue #164)', async () => { + const host = 'raw-walk-variants.local'; + const enUsPages = [ + `http://${host}/docs/en-us/getting-started`, + `http://${host}/docs/en-us/api-reference`, + `http://${host}/docs/en-us/guides`, + ]; + const enGbPages = enUsPages.map((u) => u.replace('/en-us/', '/en-gb/')); + + // No --doc-locale and no locale in the base URL: coverage must not let + // the sitemap selector pick en-gb (alphabetical tie) before it infers + // en-us from llms.txt. + const ctx = makeCtx(host, enUsPages, '/docs'); + + mockSitemapNotFound(server, `http://${host}/docs`); + server.use( + http.get( + `http://${host}/robots.txt`, + () => new HttpResponse(`Sitemap: http://${host}/sitemap.xml`, { status: 200 }), + ), + http.get( + `http://${host}/sitemap.xml`, + () => + new HttpResponse( + makeSitemapIndex([ + `http://${host}/sitemap-en-gb.xml`, + `http://${host}/sitemap-en-us.xml`, + ]), + { headers: { 'content-type': 'application/xml' } }, + ), + ), + http.get( + `http://${host}/sitemap-en-gb.xml`, + () => + new HttpResponse(makeSitemap(enGbPages), { + headers: { 'content-type': 'application/xml' }, + }), + ), + http.get( + `http://${host}/sitemap-en-us.xml`, + () => + new HttpResponse(makeSitemap(enUsPages), { + headers: { 'content-type': 'application/xml' }, + }), + ), + ); + + const result = await check.run(ctx); + expect(result.status).toBe('pass'); + expect(result.details?.sitemapDocPages).toBe(3); + expect(result.details?.coverageRate).toBe(100); + expect(result.details?.detectedLocale).toBe('en-us'); + }); + test('treats preferredLocale with no prefixed URLs as the unprefixed default', async () => { const host = 'preferred-default.local'; const llmsPages = [ diff --git a/test/unit/helpers/get-page-urls.test.ts b/test/unit/helpers/get-page-urls.test.ts index 39a0701..a511102 100644 --- a/test/unit/helpers/get-page-urls.test.ts +++ b/test/unit/helpers/get-page-urls.test.ts @@ -216,6 +216,26 @@ describe('getPathFilterBase', () => { }); describe('filterLocaleSitemaps', () => { + it('selects regional variants of the default locale when no bare code exists (issue #164)', () => { + const urls = [ + 'https://example.com/dotnet_en-us_1.xml', + 'https://example.com/dotnet_en-us_2.xml', + 'https://example.com/dotnet_fr-fr_1.xml', + 'https://example.com/dotnet_pages.xml', + ]; + expect(filterLocaleSitemaps(urls)).toEqual([urls[0], urls[1], urls[3]]); + expect(filterLocaleSitemaps(urls, 'fr')).toEqual([urls[2], urls[3]]); + }); + + it('resolves a regional preference to bare-language sitemaps', () => { + const urls = [ + 'https://example.com/sitemap-en.xml', + 'https://example.com/sitemap-fr.xml', + 'https://example.com/sitemap-pages.xml', + ]; + expect(filterLocaleSitemaps(urls, 'en-US')).toEqual([urls[0], urls[2]]); + }); + it('falls through to a valid path locale when the filename capture is invalid', () => { const urls = [ 'https://example.com/en/sitemap-en-docs.xml', @@ -360,11 +380,48 @@ describe('filterLocaleSitemaps', () => { 'https://example.com/sitemap-fr-fr.xml', 'https://example.com/sitemap-de-de.xml', ]; - // No plain 'en', so all region-tagged locales returned (no preferred match) - // Actually, these are distinct locales with region tags; en is not present - const result = filterLocaleSitemaps(urls); - // No 'en' match, no non-locale sitemaps → falls back to all - expect(result).toEqual(urls); + // No plain 'en': the default resolves to its only English variant (issue + // #164). Before that change, no match meant every locale was returned. + expect(filterLocaleSitemaps(urls)).toEqual([urls[0]]); + expect(filterLocaleSitemaps(urls, 'de')).toEqual([urls[2]]); + }); + + it('keeps every same-language child when resolving by variant', () => { + // Two GB shards and one US shard: sitemap counts must not pick GB here, + // because the page-level pass (which also sees llms.txt) decides the + // variant and needs the US pages to be fetched. + const urls = [ + 'https://example.com/sitemap-en-gb-1.xml', + 'https://example.com/sitemap-en-gb-2.xml', + 'https://example.com/sitemap-en-us.xml', + 'https://example.com/sitemap-fr-fr.xml', + 'https://example.com/sitemap-pages.xml', + ]; + expect(filterLocaleSitemaps(urls)).toEqual([urls[0], urls[1], urls[2], urls[4]]); + // An exact code still selects its own sitemaps alone. + expect(filterLocaleSitemaps(urls, 'en-US')).toEqual([urls[2], urls[4]]); + }); + + it('fetches a repeated index entry once', () => { + const urls = [ + 'https://example.com/sitemap-en-us.xml', + 'https://example.com/sitemap-en-us.xml', + 'https://example.com/sitemap-fr-fr.xml', + 'https://example.com/sitemap-pages.xml', + ]; + expect(filterLocaleSitemaps(urls)).toEqual([urls[0], urls[3]]); + }); + + it('keeps every child for an explicit preference the locale helper does not recognize', () => { + // en-custom matches no child by code. Resolving by language would keep + // en-us and drop fr-fr, leaving a two-locale pool that the page-level + // detector cannot separate from the custom pages. + const urls = [ + 'https://example.com/sitemap-en-custom.xml', + 'https://example.com/sitemap-en-us.xml', + 'https://example.com/sitemap-fr-fr.xml', + ]; + expect(filterLocaleSitemaps(urls, 'en-custom')).toEqual(urls); }); it('prefers the locale from the base URL when preferredLocale is set', () => { @@ -605,6 +662,30 @@ describe('filterLocalizedUrls', () => { ]); }); + it('resolves the target to a regional variant when no exact code exists (issue #164)', () => { + const urls = [ + 'https://example.com/docs', + 'https://example.com/docs/en-us/intro', + 'https://example.com/docs/en-us/guide', + 'https://example.com/docs/fr-fr/intro', + 'https://example.com/docs/fr-fr/guide', + ]; + const enUs = [urls[0], urls[1], urls[2]]; + expect(filterLocalizedUrls(urls)).toEqual(enUs); + expect(filterLocalizedUrls(urls, 'en-GB')).toEqual(enUs); + expect(filterLocalizedUrls(urls, 'fr')).toEqual([urls[0], urls[3], urls[4]]); + }); + + it('resolves a script preference only to a variant with the same script', () => { + const urls = [ + 'https://example.com/docs/zh-Hans-CN/intro', + 'https://example.com/docs/zh-Hans-CN/guide', + 'https://example.com/docs/zh-Hant/intro', + 'https://example.com/docs/zh-Hant/guide', + ]; + expect(filterLocalizedUrls(urls, 'zh-Hans')).toEqual([urls[0], urls[1]]); + }); + it('treats script-subtag translations as localized (issue #161)', () => { const urls = [ 'https://example.com/docs', @@ -2483,6 +2564,352 @@ ${urls.map((url) => `${url}`).join('\n')} expect(result.urls).toEqual(['http://opt-locale.local/de/docs/intro']); }); + it('keeps a locale-prefixed base when preferredLocale is a same-language variant (issue #164)', async () => { + const content = `# Docs\n## Links\n- [EN](http://opt-variant.local/en/docs/intro): Intro\n- [EN guide](http://opt-variant.local/en/docs/guide): Guide\n- [FR](http://opt-variant.local/fr/docs/intro): Intro\n- [FR guide](http://opt-variant.local/fr/docs/guide): Guide\n`; + // Base URL has /en/ and the option says en-US. The site has no /en-us/, + // so the base path must not be rewritten away from the pages that exist. + const ctx = makeCtx('http://opt-variant.local/en', content); + ctx.options.preferredLocale = 'en-US'; + + const result = await getPageUrls(ctx); + expect(result.urls).toEqual([ + 'http://opt-variant.local/en/docs/intro', + 'http://opt-variant.local/en/docs/guide', + ]); + }); + + it('selects the requested same-language variant when both variants exist (issue #164)', async () => { + const content = `# Docs\n## Links\n- [US](http://opt-both.local/en-us/docs/intro): Intro\n- [US guide](http://opt-both.local/en-us/docs/guide): Guide\n- [GB](http://opt-both.local/en-gb/docs/intro): Intro\n- [GB guide](http://opt-both.local/en-gb/docs/guide): Guide\n- [FR](http://opt-both.local/fr-fr/docs/intro): Intro\n`; + // Base URL has /en-us/ and the option asks for en-GB, which the site has. + // The requested variant must win over the base variant. + const ctx = makeCtx('http://opt-both.local/en-us', content); + ctx.options.preferredLocale = 'en-GB'; + + const result = await getPageUrls(ctx); + expect(result.urls).toEqual([ + 'http://opt-both.local/en-gb/docs/intro', + 'http://opt-both.local/en-gb/docs/guide', + ]); + }); + + it('selects the requested same-language variant from a sitemap when both exist', async () => { + // With a path-bearing base, discovery also probes sitemap candidates under + // the /en-us subpath; mock them so the test does not wait on real DNS. + mockSitemapNotFound(server, 'http://opt-both-sitemap.local/en-us'); + server.use( + http.get( + 'http://opt-both-sitemap.local/robots.txt', + () => new HttpResponse('', { status: 404 }), + ), + http.get( + 'http://opt-both-sitemap.local/sitemap.xml', + () => + new HttpResponse( + ` + + http://opt-both-sitemap.local/en-us/docs/intro + http://opt-both-sitemap.local/en-us/docs/guide + http://opt-both-sitemap.local/en-gb/docs/intro + http://opt-both-sitemap.local/en-gb/docs/guide + http://opt-both-sitemap.local/fr-fr/docs/intro +`, + { status: 200, headers: { 'Content-Type': 'application/xml' } }, + ), + ), + ); + + const ctx = makeCtx('http://opt-both-sitemap.local/en-us'); + ctx.options.preferredLocale = 'en-GB'; + + const result = await getPageUrls(ctx); + expect(result.urls).toEqual([ + 'http://opt-both-sitemap.local/en-gb/docs/intro', + 'http://opt-both-sitemap.local/en-gb/docs/guide', + ]); + }); + + it('scopes a site-specific preference to its own path even over a locale-prefixed base', async () => { + const content = `# Docs\n## Links\n- [US](http://opt-custom.local/en-us/docs/intro): Intro\n- [US guide](http://opt-custom.local/en-us/docs/guide): Guide\n- [Custom](http://opt-custom.local/en-custom/docs/intro): Intro\n- [Custom guide](http://opt-custom.local/en-custom/docs/guide): Guide\n`; + // en-custom is not a recognized locale code, so no variant fallback: the + // requested path alone is in scope. + const ctx = makeCtx('http://opt-custom.local/en-us', content); + ctx.options.preferredLocale = 'en-custom'; + + const result = await getPageUrls(ctx); + expect(result.urls).toEqual([ + 'http://opt-custom.local/en-custom/docs/intro', + 'http://opt-custom.local/en-custom/docs/guide', + ]); + }); + + it('refines the merged llms.txt and sitemap pool to one locale variant', async () => { + // llms.txt is thin and lists only US pages; the sitemap has the requested + // GB variant. Refining each source alone would merge both variants. + const content = `# Docs\n## Links\n- [US](http://opt-merge.local/en-us/docs/intro): Intro\n- [US guide](http://opt-merge.local/en-us/docs/guide): Guide\n`; + // makeCtx registers sitemap 404s for the base; the sitemap handler below + // must be registered afterwards so it takes precedence. + const ctx = makeCtx('http://opt-merge.local/en-us', content); + ctx.options.preferredLocale = 'en-GB'; + server.use( + http.get('http://opt-merge.local/robots.txt', () => new HttpResponse('', { status: 404 })), + http.get( + 'http://opt-merge.local/sitemap.xml', + () => + new HttpResponse( + ` + + http://opt-merge.local/en-us/docs/intro + http://opt-merge.local/en-us/docs/guide + http://opt-merge.local/en-gb/docs/intro + http://opt-merge.local/en-gb/docs/guide +`, + { status: 200, headers: { 'Content-Type': 'application/xml' } }, + ), + ), + ); + const result = await getPageUrls(ctx); + expect(result.sources).toEqual(['llms-txt', 'sitemap']); + expect(result.urls).toEqual([ + 'http://opt-merge.local/en-gb/docs/intro', + 'http://opt-merge.local/en-gb/docs/guide', + ]); + }); + + it('keeps every page of the winning variant from both sources', async () => { + // Default en. llms.txt: 3 US + 2 GB pages; sitemap: the same 3 US + 4 other + // GB pages. Refining each source alone picks US from llms.txt and GB from + // the sitemap and loses the two GB pages only llms.txt listed. Merging the + // scoped pools first resolves en to en-gb (6 vs 3) and keeps all six. + const us = ['intro', 'guide', 'api'].map((p) => `http://opt-pool.local/en-us/docs/${p}`); + const gbLlms = ['intro', 'guide'].map((p) => `http://opt-pool.local/en-gb/docs/${p}`); + const gbSitemap = ['api', 'faq', 'cli', 'sdk'].map( + (p) => `http://opt-pool.local/en-gb/docs/${p}`, + ); + const content = `# Docs\n## Links\n${[...us, ...gbLlms].map((u) => `- [p](${u}): p`).join('\n')}\n`; + const ctx = makeCtx('http://opt-pool.local', content); + server.use( + http.get('http://opt-pool.local/robots.txt', () => new HttpResponse('', { status: 404 })), + http.get( + 'http://opt-pool.local/sitemap.xml', + () => + new HttpResponse( + `\n\n${[...us, ...gbSitemap].map((u) => `${u}`).join('\n')}\n`, + { status: 200, headers: { 'Content-Type': 'application/xml' } }, + ), + ), + ); + + const result = await getPageUrls(ctx); + expect(result.sources).toEqual(['llms-txt', 'sitemap']); + expect(result.urls).toEqual([...gbLlms, ...gbSitemap]); + }); + + it('caps only the sitemap additions, not surviving llms.txt pages', async () => { + // maxLinksToTest has no upper bound. 501 llms.txt pages plus one sitemap + // page must yield 502, not the sitemap cap of 500. + const llmsPages = Array.from({ length: 501 }, (_, i) => `http://opt-cap.local/docs/page-${i}`); + const content = `# Docs\n## Links\n${llmsPages.map((u) => `- [p](${u}): p`).join('\n')}\n`; + const ctx = makeCtx('http://opt-cap.local', content); + ctx.options.maxLinksToTest = 600; + server.use( + http.get('http://opt-cap.local/robots.txt', () => new HttpResponse('', { status: 404 })), + http.get( + 'http://opt-cap.local/sitemap.xml', + () => + new HttpResponse( + `\n\nhttp://opt-cap.local/docs/extra\n`, + { status: 200, headers: { 'Content-Type': 'application/xml' } }, + ), + ), + ); + + const result = await getPageUrls(ctx); + expect(result.sources).toEqual(['llms-txt', 'sitemap']); + expect(result.urls).toHaveLength(502); + expect(result.urls.slice(0, 501)).toEqual(llmsPages); + expect(result.urls[501]).toBe('http://opt-cap.local/docs/extra'); + }); + + it('fetches every same-language child so the page-level pass can pick the variant', async () => { + // Default en. Two GB shards, one US shard. llms.txt lists US pages, so + // the merged pool resolves to en-us; the US shard must have been fetched. + const usLlms = ['intro', 'guide', 'api'].map((p) => `http://opt-shards.local/en-us/docs/${p}`); + const content = `# Docs\n## Links\n${usLlms.map((u) => `- [p](${u}): p`).join('\n')}\n`; + const ctx = makeCtx('http://opt-shards.local', content); + const urlset = (locs: string[]) => + `\n\n${locs.map((u) => `${u}`).join('\n')}\n`; + server.use( + http.get('http://opt-shards.local/robots.txt', () => new HttpResponse('', { status: 404 })), + http.get( + 'http://opt-shards.local/sitemap.xml', + () => + new HttpResponse( + `\n\nhttp://opt-shards.local/sitemap-en-gb-1.xml\nhttp://opt-shards.local/sitemap-en-gb-2.xml\nhttp://opt-shards.local/sitemap-en-us.xml\n`, + { status: 200, headers: { 'Content-Type': 'application/xml' } }, + ), + ), + http.get( + 'http://opt-shards.local/sitemap-en-gb-1.xml', + () => + new HttpResponse(urlset(['http://opt-shards.local/en-gb/docs/intro']), { + status: 200, + headers: { 'Content-Type': 'application/xml' }, + }), + ), + http.get( + 'http://opt-shards.local/sitemap-en-gb-2.xml', + () => + new HttpResponse(urlset(['http://opt-shards.local/en-gb/docs/guide']), { + status: 200, + headers: { 'Content-Type': 'application/xml' }, + }), + ), + http.get( + 'http://opt-shards.local/sitemap-en-us.xml', + () => + new HttpResponse(urlset(['http://opt-shards.local/en-us/docs/extra']), { + status: 200, + headers: { 'Content-Type': 'application/xml' }, + }), + ), + ); + + const result = await getPageUrls(ctx); + expect(result.sources).toEqual(['llms-txt', 'sitemap']); + expect(result.urls).toEqual([...usLlms, 'http://opt-shards.local/en-us/docs/extra']); + }); + + it('selects custom-locale pages from an index when the preference is unrecognized', async () => { + const custom = ['intro', 'guide', 'api'].map( + (p) => `http://opt-custom-idx.local/en-custom/docs/${p}`, + ); + const urlset = (locs: string[]) => + `\n\n${locs.map((u) => `${u}`).join('\n')}\n`; + const child = (name: string, locs: string[]) => + http.get( + `http://opt-custom-idx.local/sitemap-${name}.xml`, + () => + new HttpResponse(urlset(locs), { + status: 200, + headers: { 'Content-Type': 'application/xml' }, + }), + ); + mockSitemapNotFound(server, 'http://opt-custom-idx.local'); + server.use( + http.get( + 'http://opt-custom-idx.local/robots.txt', + () => new HttpResponse('', { status: 404 }), + ), + http.get( + 'http://opt-custom-idx.local/sitemap.xml', + () => + new HttpResponse( + `\n\n${['en-custom', 'en-us', 'fr-fr'].map((n) => `http://opt-custom-idx.local/sitemap-${n}.xml`).join('\n')}\n`, + { status: 200, headers: { 'Content-Type': 'application/xml' } }, + ), + ), + child('en-custom', custom), + child( + 'en-us', + ['intro', 'guide', 'api'].map((p) => `http://opt-custom-idx.local/en-us/docs/${p}`), + ), + child( + 'fr-fr', + ['intro', 'guide', 'api'].map((p) => `http://opt-custom-idx.local/fr-fr/docs/${p}`), + ), + ); + const ctx = makeCtx('http://opt-custom-idx.local'); + ctx.options.preferredLocale = 'en-custom'; + + const result = await getPageUrls(ctx); + expect(result.sources).toEqual(['sitemap']); + expect(result.urls).toEqual(custom); + }); + + it('counts each sitemap URL once when choosing a locale variant', async () => { + // One GB page listed three times against two distinct US pages. Counting + // entries would pick GB (3 > 2); counting distinct pages picks US (2 > 1). + mockSitemapNotFound(server, 'http://opt-dupes.local'); + server.use( + http.get('http://opt-dupes.local/robots.txt', () => new HttpResponse('', { status: 404 })), + http.get( + 'http://opt-dupes.local/sitemap.xml', + () => + new HttpResponse( + ` + + http://opt-dupes.local/en-gb/docs/intro + http://opt-dupes.local/en-gb/docs/intro + http://opt-dupes.local/en-gb/docs/intro + http://opt-dupes.local/en-us/docs/intro + http://opt-dupes.local/en-us/docs/guide +`, + { status: 200, headers: { 'Content-Type': 'application/xml' } }, + ), + ), + ); + const ctx = makeCtx('http://opt-dupes.local'); + + const result = await getPageUrls(ctx); + expect(result.urls).toEqual([ + 'http://opt-dupes.local/en-us/docs/intro', + 'http://opt-dupes.local/en-us/docs/guide', + ]); + }); + + it('keeps child sitemaps needed by the fallback scope (issue #164)', async () => { + // Preferred en-GB resolves to en-au among the children (alphabetical tie), + // but the fallback scope /en-us needs the en-us child. + mockSitemapNotFound(server, 'http://opt-children.local/en-us'); + server.use( + http.get('http://opt-children.local/robots.txt', () => new HttpResponse('', { status: 404 })), + http.get( + 'http://opt-children.local/sitemap.xml', + () => + new HttpResponse( + ` + + http://opt-children.local/sitemap-en-au.xml + http://opt-children.local/sitemap-en-us.xml +`, + { status: 200, headers: { 'Content-Type': 'application/xml' } }, + ), + ), + http.get( + 'http://opt-children.local/sitemap-en-au.xml', + () => + new HttpResponse( + ` + + http://opt-children.local/en-au/docs/intro +`, + { status: 200, headers: { 'Content-Type': 'application/xml' } }, + ), + ), + http.get( + 'http://opt-children.local/sitemap-en-us.xml', + () => + new HttpResponse( + ` + + http://opt-children.local/en-us/docs/intro + http://opt-children.local/en-us/docs/guide +`, + { status: 200, headers: { 'Content-Type': 'application/xml' } }, + ), + ), + ); + const ctx = makeCtx('http://opt-children.local/en-us'); + ctx.options.preferredLocale = 'en-GB'; + + const result = await getPageUrls(ctx); + expect(result.sources).toEqual(['sitemap']); + expect(result.urls).toEqual([ + 'http://opt-children.local/en-us/docs/intro', + 'http://opt-children.local/en-us/docs/guide', + ]); + }); + it('explicit preferredVersion overrides version detected from base URL', async () => { const content = `# Docs\n## Links\n- [v1](http://opt-ver.local/docs/v1/intro): v1\n- [v2](http://opt-ver.local/docs/v2/intro): v2\n- [v3](http://opt-ver.local/docs/v3/intro): v3\n- [v1 Guide](http://opt-ver.local/docs/v1/guide): v1\n- [v2 Guide](http://opt-ver.local/docs/v2/guide): v2\n- [v3 Guide](http://opt-ver.local/docs/v3/guide): v3\n`; // Base URL has /v3/ but explicit option says 'v1' — path prefix @@ -2762,7 +3189,10 @@ ${urls.map((url) => `${url}`).join('\n')} server.use( http.get(`${origin}/*`, ({ request }) => { requested.push(request.url); - return new HttpResponse(`${origin}/page`); + // One distinct page per root: collected URLs are deduplicated, so a + // shared page would never reach the URL cap. + const index = /map-(\d+)\.xml$/.exec(request.url)?.[1]; + return new HttpResponse(`${origin}/page-${index}`); }), ); const ctx = createContext(origin, { requestDelay: 0 }); diff --git a/test/unit/helpers/locale-codes.test.ts b/test/unit/helpers/locale-codes.test.ts index 02b123d..6438eda 100644 --- a/test/unit/helpers/locale-codes.test.ts +++ b/test/unit/helpers/locale-codes.test.ts @@ -3,6 +3,9 @@ import { isLocaleSegment, hasStructuralDuplication, detectLocalePosition, + countLocaleCodesAt, + resolveLocaleVariant, + sameLanguageVariants, } from '../../../src/helpers/locale-codes.js'; describe('isLocaleSegment', () => { @@ -187,3 +190,153 @@ describe('detectLocalePosition', () => { expect(detectLocalePosition(urls)).toBeNull(); }); }); + +describe('countLocaleCodesAt', () => { + test('counts lowercased locale codes at a position and ignores other segments', () => { + const urls = [ + 'http://x.com/docs/en-US/intro', + 'http://x.com/docs/en-us/api', + 'http://x.com/docs/fr-fr/intro', + 'http://x.com/docs/intro', + 'http://x.com/docs', + 'not a url', + ]; + expect(countLocaleCodesAt(urls, 1)).toEqual( + new Map([ + ['en-us', 2], + ['fr-fr', 1], + ]), + ); + }); +}); + +describe('sameLanguageVariants', () => { + test('returns only the exact code when present', () => { + expect( + sameLanguageVariants( + 'en', + new Map([ + ['en', 1], + ['en-us', 9], + ]), + ), + ).toEqual(['en']); + }); + + test('returns every compatible variant ranked by count, then alphabetically', () => { + const available = new Map([ + ['en-gb', 2], + ['en-us', 7], + ['en-au', 2], + ['fr-fr', 9], + ]); + expect(sameLanguageVariants('en', available)).toEqual(['en-us', 'en-au', 'en-gb']); + expect(sameLanguageVariants('de', available)).toEqual([]); + }); + + test('excludes variants whose script conflicts', () => { + const available = new Map([ + ['zh-hant', 5], + ['zh-hans-cn', 1], + ['zh', 1], + ]); + expect(sameLanguageVariants('zh-Hans', available)).toEqual(['zh', 'zh-hans-cn']); + }); +}); + +describe('resolveLocaleVariant', () => { + const counts = (entries: [string, number][]) => new Map(entries); + + test('returns the exact code when present, case-insensitively', () => { + expect( + resolveLocaleVariant( + 'EN', + counts([ + ['en', 1], + ['en-us', 5], + ]), + ), + ).toBe('en'); + }); + + test('resolves a bare language to its regional variant (issue #164)', () => { + expect( + resolveLocaleVariant( + 'en', + counts([ + ['en-us', 3], + ['fr-fr', 3], + ]), + ), + ).toBe('en-us'); + }); + + test('resolves a regional preference to the bare language', () => { + expect( + resolveLocaleVariant( + 'en-US', + counts([ + ['en', 3], + ['fr', 3], + ]), + ), + ).toBe('en'); + }); + + test('picks the best-represented variant, then alphabetical on ties', () => { + expect( + resolveLocaleVariant( + 'en', + counts([ + ['en-gb', 2], + ['en-us', 7], + ]), + ), + ).toBe('en-us'); + expect( + resolveLocaleVariant( + 'en', + counts([ + ['en-us', 2], + ['en-gb', 2], + ]), + ), + ).toBe('en-gb'); + }); + + test('requires matching scripts when both sides specify one', () => { + expect( + resolveLocaleVariant( + 'zh-Hans', + counts([ + ['zh-hans-cn', 1], + ['zh-hant', 9], + ]), + ), + ).toBe('zh-hans-cn'); + expect(resolveLocaleVariant('zh-Hans', counts([['zh-hant', 9]]))).toBeNull(); + expect( + resolveLocaleVariant( + 'zh', + counts([ + ['zh-hant', 1], + ['zh-hans', 1], + ]), + ), + ).toBe('zh-hans'); + }); + + test('returns null when no code shares the language', () => { + expect( + resolveLocaleVariant( + 'en', + counts([ + ['es', 1], + ['fr', 1], + ['zh', 1], + ]), + ), + ).toBeNull(); + expect(resolveLocaleVariant('en', new Map())).toBeNull(); + }); +}); diff --git a/working-notes/page-discovery-notes.md b/working-notes/page-discovery-notes.md index 906b199..e7de334 100644 --- a/working-notes/page-discovery-notes.md +++ b/working-notes/page-discovery-notes.md @@ -458,6 +458,102 @@ used as locale prefixes, each a new false-positive surface); validating know, such as `latam`, is still honored by the raw-segment match from issue #158, and rejecting it would remove that escape hatch). +### October 2026: locale preferences resolve to same-language variants (issue #164) + +Locale preferences were matched by exact code in three places: the +locale-sitemap selector, the page-URL filter, and coverage's explicit-locale +branch. A site organized by `en-us` and `fr-fr` therefore matched neither the +default `en` nor a bare `--doc-locale en`, and `--doc-locale en-US` failed +against a site that uses bare `en`. Microsoft Learn worked only because its +base URL supplies `en-us`. Found while writing the PR #163 fixtures. + +Decision: **one resolver, `resolveLocaleVariant`, in +`src/helpers/locale-codes.ts`, used by all three call sites.** Exact match +wins. Otherwise the best-represented code with the same language is chosen, +and when both the preference and the candidate name a script they must +agree (`zh-Hans` resolves to `zh-Hans-CN`, never to `zh-Hant`). Ties break +alphabetically. No same-language code resolves to null, which preserves the +issue #158 unprefixed fallback. The page-URL filter still tries the exact +raw segment first so a site-specific code such as `latam` keeps working. + +Rejected alternatives: selecting every same-language variant (`en-us` plus +`en-gb` would put regional duplicates of each page into the sample, and the +page-level filter has no later step that would remove them); preferring a +canonical region such as `en-us` (arbitrary across languages; representation +count is a property of the site); validating or normalizing `--doc-locale` +at the CLI (the raw value is still useful as an escape hatch). + +Copilot review of the first draft found three gaps, all addressed: + +- Coverage's explicit-locale branch resolved before checking the exact raw + prefix, so `--doc-locale en-custom` selected `en-us`. It now mirrors the + page filter: exact prefix first, variant only when absent. +- Coverage's raw sitemap walk (`skipRefinement`) ran the sitemap selector + with the built-in default `en`, which now resolved `{en-gb, en-us}` to + `en-gb` and discarded the US children before coverage inferred `en-us` + from llms.txt. Before this change the same case returned every child + because no `en` key existed. The raw walk now keeps every child when no + locale is known from the flag or the base URL. The fetch and byte budgets + from issue #120 still bound it; the trade-off is that a site with many + locale shards and no known locale may spend budget on other locales. +- `rewriteFilterBase` replaced the base URL's locale with the preference + whenever they differed, so base `/en` with `--doc-locale en-US` scoped + discovery to `/en-us/` and found nothing. This predates the branch. The + first fix kept the base path for same-language variants, which Copilot + showed breaks the opposite case: base `/en-us` with `--doc-locale en-GB` + on a site that has both kept the US pages. Discovery is now scoped to + both paths when the preference is a same-language variant of the base + locale (primary: rewritten; secondary: original), for llms.txt links and + sitemap URLs alike, and `filterLocalizedUrls` takes the exact variant + when present and resolves to the base variant otherwise. A different + language still rewrites without fallback. +- Three consequences of the dual scope, found on the next review pass: + a site-specific preference (`en-custom`) over a `/en-us` base kept both + paths and, with no recognized second code, locale detection never fired, + so both locales were returned (dual scope now applies only to recognized + locale codes); the thin-llms.txt branch refined each source alone and + merged two variants, and refining the merge again could not recover pages + of the winning variant that only one source listed (the path-scoped, + unrefined pools are merged first and refined once; the sitemap walker + gained `deferRefinement` for this, distinct from coverage's + `skipRefinement` because child selection and the collection cap stay as + in refined discovery); the 500-URL sitemap cap is then applied to the + sitemap additions only, because `maxLinksToTest` has no upper bound and + llms.txt pages were never subject to that cap; +- A seventh review pass asked for human assessment of indexed-sitemap + pruning without a concrete finding. One real gap was identifiable: + child-sitemap selection resolved a variant by counting _sitemaps_, while + the page-level pass counts _pages_ and sees llms.txt. Two GB shards and + one US shard selected GB children only; if llms.txt then tipped the page + pass to en-us, the US sitemap pages had never been fetched. Child + selection is now inclusive when it resolves by variant: every + same-language child is fetched (`sameLanguageVariants`) and the single + page-level pass picks the variant. An exact code still selects its own + children alone. Fetching a rejected variant costs bounded budget; + skipping the chosen one loses pages. This is consistent with the raw + coverage walk keeping every child when no locale is known. The + same pass made an unrecognized explicit preference (`en-custom`) resolve + by language to the `en-us` child while dropping `fr-fr`, a two-locale pool + the page-level detector cannot separate from the custom pages. Child + pruning is now skipped entirely for a preference `isLocaleSegment` does + not recognize, matching the path-scope rule, so the page-level + exact-prefix filter decides. +- A further review summary raised two concerns without a finding. On + "fallback discovery overlooking an exact match": every selection point + checks the exact code or path first (page filter raw segment, coverage + exact prefix, sitemap selector exact code, dual scope primary path); the + base-URL fallback at the end of discovery deliberately uses the original + base, since a rewritten locale path may not exist. No change. On + "duplicate sitemap URLs skewing regional selection": real. Nothing before + the final sampler deduplicated sitemap URLs, so a page repeated in one + sitemap or listed by two shards counted twice in the per-variant totals + that now choose a locale. The walker now collects distinct URLs, counted + once for the narrow-prefix warning as well, and the child selector + fetches a repeated index entry once. Coverage was already comparing + normalized sets, so its rate was unaffected. child-sitemap selection resolved the preference + alone and could pick an `en-au` child whose pages neither scope keeps + (children needed by any fallback scope's locale are kept as well). + ## Documentation at scale: design considerations Drafting `docs/documentation-at-scale.md` after the #120 live reproduction