From 8f04396abea4b1392785e7f0048978827fc1e859 Mon Sep 17 00:00:00 2001 From: dacharyc Date: Sat, 31 Oct 2026 20:58:18 -0400 Subject: [PATCH 01/10] fix: resolve locale preferences to same-language variants Locale preferences were matched by exact code in the locale-sitemap selector, the page-URL filter, and coverage's explicit-locale branch. A site organized by en-us and fr-fr matched neither the default en nor a bare --doc-locale en, and --doc-locale en-US failed against a site that uses bare en. Add resolveLocaleVariant: exact match first, otherwise the best-represented code with the same language, with scripts required to agree when both sides name one, ties broken alphabetically, and null when no code shares the language so the unprefixed fallback from #158 still applies. Use it in all three places. One variant is chosen rather than every same-language variant so regional duplicates do not enter the page sample. Closes #164 --- docs/checks/observability.md | 2 +- docs/reference/cli.md | 2 + src/checks/observability/llms-txt-coverage.ts | 16 ++- src/helpers/get-page-urls.ts | 29 +++-- src/helpers/locale-codes.ts | 62 +++++++++ test/unit/checks/llms-txt-coverage.test.ts | 37 ++++++ test/unit/helpers/get-page-urls.test.ts | 53 +++++++- test/unit/helpers/locale-codes.test.ts | 118 ++++++++++++++++++ working-notes/page-discovery-notes.md | 25 ++++ 9 files changed, 328 insertions(+), 16 deletions(-) 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..4603368 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 URLs is used. + ```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..25b7856 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,17 @@ 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); + // A regional or script variant of the same language stands in for an + // absent exact code (`--doc-locale en` selects `/en-us/`, issue #164). + const resolved = + resolveLocaleVariant(preferred, countLocaleCodesAt(scopedSitemapUrls, localePosition)) ?? + preferred; + const byPreferred = 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..f75ae9b 100644 --- a/src/helpers/get-page-urls.ts +++ b/src/helpers/get-page-urls.ts @@ -4,7 +4,12 @@ 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, +} from './locale-codes.js'; import type { CheckContext, DiscoveredFile, UrlPathPattern } from '../types.js'; /** @@ -517,9 +522,14 @@ 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', resolving a regional or script + // variant when the exact code is absent (`en` selects `en-us` shards, + // issue #164), then any non-locale sitemaps. + const counts = new Map([...locales].map(([code, list]) => [code, list.length])); + const resolved = + (preferredLocale ? resolveLocaleVariant(preferredLocale, counts) : null) ?? + resolveLocaleVariant('en', counts); + const preferred = resolved ? locales.get(resolved)! : []; return [...preferred, ...nonLocale].length > 0 ? [...preferred, ...nonLocale] : subSitemapUrls; } @@ -555,19 +565,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; } diff --git a/src/helpers/locale-codes.ts b/src/helpers/locale-codes.ts index ff1df0a..38712f9 100644 --- a/src/helpers/locale-codes.ts +++ b/src/helpers/locale-codes.ts @@ -251,6 +251,68 @@ 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)); +} + +/** + * 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. + */ +export function resolveLocaleVariant( + preferred: string, + available: Map, +): string | null { + const target = preferred.toLowerCase(); + if (available.has(target)) return target; + + const language = target.split('-')[0]; + const targetScript = scriptOf(target); + let best: string | null = null; + let bestCount = -1; + for (const [code, count] of available) { + if (code.split('-')[0] !== language) continue; + const script = scriptOf(code); + if (targetScript && script && script !== targetScript) continue; + if (count > bestCount || (count === bestCount && best !== null && code < best)) { + best = code; + bestCount = count; + } + } + return best; +} + /** * 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..af65949 100644 --- a/test/unit/checks/llms-txt-coverage.test.ts +++ b/test/unit/checks/llms-txt-coverage.test.ts @@ -908,6 +908,43 @@ 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('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..6984b05 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,10 @@ 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('prefers the locale from the base URL when preferredLocale is set', () => { @@ -605,6 +624,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', diff --git a/test/unit/helpers/locale-codes.test.ts b/test/unit/helpers/locale-codes.test.ts index 02b123d..0d73b8d 100644 --- a/test/unit/helpers/locale-codes.test.ts +++ b/test/unit/helpers/locale-codes.test.ts @@ -3,6 +3,8 @@ import { isLocaleSegment, hasStructuralDuplication, detectLocalePosition, + countLocaleCodesAt, + resolveLocaleVariant, } from '../../../src/helpers/locale-codes.js'; describe('isLocaleSegment', () => { @@ -187,3 +189,119 @@ 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('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..941c5a3 100644 --- a/working-notes/page-discovery-notes.md +++ b/working-notes/page-discovery-notes.md @@ -458,6 +458,31 @@ 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). + ## Documentation at scale: design considerations Drafting `docs/documentation-at-scale.md` after the #120 live reproduction From efd97c969dc943dcecbaee6a0524cac9e254ce84 Mon Sep 17 00:00:00 2001 From: dacharyc Date: Sat, 31 Oct 2026 21:12:25 -0400 Subject: [PATCH 02/10] fix: check exact locale prefixes first and keep raw-walk children Copilot review found three gaps in the variant resolver wiring. Coverage's explicit-locale branch resolved a variant 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 ran the sitemap selector with the built-in default en, which resolved {en-gb, en-us} to en-gb and discarded the US children before coverage inferred en-us from llms.txt. The raw walk now keeps every child when no locale is known from the flag or the base URL; the fetch and byte budgets still bound it. 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. The base is now rewritten only when the preference is a different language or a conflicting script. --- src/checks/observability/llms-txt-coverage.ts | 18 +++- src/helpers/get-page-urls.ts | 25 +++-- test/unit/checks/llms-txt-coverage.test.ts | 94 +++++++++++++++++++ test/unit/helpers/get-page-urls.test.ts | 14 +++ working-notes/page-discovery-notes.md | 19 ++++ 5 files changed, 159 insertions(+), 11 deletions(-) diff --git a/src/checks/observability/llms-txt-coverage.ts b/src/checks/observability/llms-txt-coverage.ts index 25b7856..6368e14 100644 --- a/src/checks/observability/llms-txt-coverage.ts +++ b/src/checks/observability/llms-txt-coverage.ts @@ -329,12 +329,20 @@ async function check(ctx: CheckContext): Promise { if (preferred) { // Explicit locale (--doc-locale / config preferredLocale) wins over // inference from llms.txt URLs. - // A regional or script variant of the same language stands in for an - // absent exact code (`--doc-locale en` selects `/en-us/`, issue #164). + // 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 = - resolveLocaleVariant(preferred, countLocaleCodesAt(scopedSitemapUrls, localePosition)) ?? - preferred; - const byPreferred = filterByLocale(scopedSitemapUrls, resolved, localePosition); + 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; diff --git a/src/helpers/get-page-urls.ts b/src/helpers/get-page-urls.ts index f75ae9b..2b5be57 100644 --- a/src/helpers/get-page-urls.ts +++ b/src/helpers/get-page-urls.ts @@ -894,12 +894,18 @@ 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); + const filteredSubSitemaps = + skipRefinement && !knownLocale + ? parsed.sitemapIndexUrls + : filterLocaleSitemaps(parsed.sitemapIndexUrls, knownLocale); for (const subSitemapUrl of filteredSubSitemaps) { if (urls.length >= collectLimit) break; @@ -980,7 +986,14 @@ function rewriteFilterBase( if (preferredLocale) { const detected = extractLocaleFromUrl(filterBase); - if (detected && detected !== preferredLocale.toLowerCase()) { + // Rewrite only when the preference is a different language (or a + // conflicting script). A same-language variant such as `en-US` over a + // `/en/` base keeps the base path so the resolver can match the pages + // the site actually has (issue #164). + const sameLanguage = + detected !== null && + resolveLocaleVariant(preferredLocale, new Map([[detected, 1]])) !== null; + if (detected && !sameLanguage) { const idx = segments.findIndex((s) => s.toLowerCase() === detected); if (idx >= 0) segments[idx] = preferredLocale.toLowerCase(); } diff --git a/test/unit/checks/llms-txt-coverage.test.ts b/test/unit/checks/llms-txt-coverage.test.ts index af65949..916a013 100644 --- a/test/unit/checks/llms-txt-coverage.test.ts +++ b/test/unit/checks/llms-txt-coverage.test.ts @@ -945,6 +945,100 @@ describe('llms-txt-coverage', () => { 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 6984b05..1ef464f 100644 --- a/test/unit/helpers/get-page-urls.test.ts +++ b/test/unit/helpers/get-page-urls.test.ts @@ -2526,6 +2526,20 @@ ${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('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 diff --git a/working-notes/page-discovery-notes.md b/working-notes/page-discovery-notes.md index 941c5a3..675b6c6 100644 --- a/working-notes/page-discovery-notes.md +++ b/working-notes/page-discovery-notes.md @@ -483,6 +483,25 @@ 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 + base is now rewritten only when the preference is a different language or + a conflicting script, so the resolver can match what the site has. + ## Documentation at scale: design considerations Drafting `docs/documentation-at-scale.md` after the #120 live reproduction From 7cbc65d9ec7517feb2922043a1829d5e60e92a8c Mon Sep 17 00:00:00 2001 From: dacharyc Date: Sat, 31 Oct 2026 21:23:21 -0400 Subject: [PATCH 03/10] fix: scope discovery to both paths for same-language locale variants Keeping the base path for a same-language preference fixed base /en with --doc-locale en-US on a site that has only /en/, but broke base /en-us with --doc-locale en-GB on a site that has both: the GB links were scoped out before the locale filter ran, and discovery returned US pages. Scope discovery to both paths when the preference is a same-language variant of the base URL's locale (primary: rewritten; secondary: original), for llms.txt links and sitemap URLs alike. filterLocalizedUrls then takes the exact variant when present and resolves to the base variant otherwise. rewriteFilterBase returns to always rewriting, and a different language still never falls back to the base locale. --- src/helpers/get-page-urls.ts | 78 ++++++++++++++++--------- test/unit/helpers/get-page-urls.test.ts | 48 +++++++++++++++ working-notes/page-discovery-notes.md | 10 +++- 3 files changed, 105 insertions(+), 31 deletions(-) diff --git a/src/helpers/get-page-urls.ts b/src/helpers/get-page-urls.ts index 2b5be57..ef96ea4 100644 --- a/src/helpers/get-page-urls.ts +++ b/src/helpers/get-page-urls.ts @@ -823,7 +823,8 @@ 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; } @@ -838,9 +839,13 @@ export async function getUrlsFromSitemap( 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 }; @@ -870,7 +875,9 @@ export async function getUrlsFromSitemap( } if (!isSameSite(url, matchOrigin)) return false; examinedUrls++; - if (prefixPath && !matchesPathPrefix(url, prefixPath)) return false; + if (prefixPaths.length > 0 && !prefixPaths.some((p) => matchesPathPrefix(url, p))) { + return false; + } matchingUrls++; return true; } @@ -971,7 +978,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, @@ -986,14 +994,7 @@ function rewriteFilterBase( if (preferredLocale) { const detected = extractLocaleFromUrl(filterBase); - // Rewrite only when the preference is a different language (or a - // conflicting script). A same-language variant such as `en-US` over a - // `/en/` base keeps the base path so the resolver can match the pages - // the site actually has (issue #164). - const sameLanguage = - detected !== null && - resolveLocaleVariant(preferredLocale, new Map([[detected, 1]])) !== null; - if (detected && !sameLanguage) { + if (detected && detected !== preferredLocale.toLowerCase()) { const idx = segments.findIndex((s) => s.toLowerCase() === detected); if (idx >= 0) segments[idx] = preferredLocale.toLowerCase(); } @@ -1062,6 +1063,34 @@ 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); + if (!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. * @@ -1083,11 +1112,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[] { @@ -1096,14 +1122,8 @@ export async function getPageUrls(ctx: CheckContext): Promise { } function scopeLlmsTxtUrls(urls: string[]): string[] { - const scoped = filterByPathPrefix(urls, filterBase); - warnOnNarrowPathPrefix( - warnings, - 'llms.txt', - new URL(filterBase).pathname.replace(/\/$/, ''), - scoped.length, - urls.length, - ); + const scoped = urls.filter((url) => filterPaths.some((p) => matchesPathPrefix(url, p))); + warnOnNarrowPathPrefix(warnings, 'llms.txt', filterPaths[0], scoped.length, urls.length); return refineUrls(scoped); } @@ -1142,7 +1162,7 @@ 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 }); + const sitemapUrls = await getUrlsFromSitemap(ctx, warnings, { pathFilterBase: filterBases }); if (sitemapUrls.length > 0) { sources.push('sitemap'); return { @@ -1158,7 +1178,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/test/unit/helpers/get-page-urls.test.ts b/test/unit/helpers/get-page-urls.test.ts index 1ef464f..b821195 100644 --- a/test/unit/helpers/get-page-urls.test.ts +++ b/test/unit/helpers/get-page-urls.test.ts @@ -2540,6 +2540,54 @@ ${urls.map((url) => `${url}`).join('\n')} ]); }); + 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 () => { + mockSitemapNotFound(server, 'http://opt-both-sitemap.local'); + 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('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 diff --git a/working-notes/page-discovery-notes.md b/working-notes/page-discovery-notes.md index 675b6c6..6658d4c 100644 --- a/working-notes/page-discovery-notes.md +++ b/working-notes/page-discovery-notes.md @@ -499,8 +499,14 @@ Copilot review of the first draft found three gaps, all addressed: - `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 - base is now rewritten only when the preference is a different language or - a conflicting script, so the resolver can match what the site has. + 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. ## Documentation at scale: design considerations From 66eb48c3b3c270e329c9aeaafd387efed692f8bb Mon Sep 17 00:00:00 2001 From: dacharyc Date: Sat, 31 Oct 2026 21:25:23 -0400 Subject: [PATCH 04/10] test: mock the subpath llms.txt and sitemap probes in the two-variant sitemap test With a path-bearing base URL and no cached llms.txt result, discovery probes /en-us/llms.txt and /en-us/sitemap*.xml. Unmocked, those took 15s to fail; the test now completes in milliseconds. --- test/unit/helpers/get-page-urls.test.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/unit/helpers/get-page-urls.test.ts b/test/unit/helpers/get-page-urls.test.ts index b821195..7f6791e 100644 --- a/test/unit/helpers/get-page-urls.test.ts +++ b/test/unit/helpers/get-page-urls.test.ts @@ -2555,8 +2555,14 @@ ${urls.map((url) => `${url}`).join('\n')} }); it('selects the requested same-language variant from a sitemap when both exist', async () => { - mockSitemapNotFound(server, 'http://opt-both-sitemap.local'); + // No llms.txt result is cached, so discovery probes the llms.txt and + // sitemap candidates under both the origin and the /en-us subpath. + mockSitemapNotFound(server, 'http://opt-both-sitemap.local/en-us'); server.use( + http.get( + 'http://opt-both-sitemap.local/*llms.txt', + () => new HttpResponse('', { status: 404 }), + ), http.get( 'http://opt-both-sitemap.local/robots.txt', () => new HttpResponse('', { status: 404 }), From d410559e7a2cbf1ee5c00b26ed80690eb43abdf7 Mon Sep 17 00:00:00 2001 From: dacharyc Date: Sat, 31 Oct 2026 21:41:26 -0400 Subject: [PATCH 05/10] fix: close three gaps in dual-scope locale discovery Copilot review of the dual path scope found three consequences. A site-specific preference such as 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. The dual scope now applies only to recognized locale codes; other preferences scope to their own path alone. The thin-llms.txt branch refined each source on its own and then merged, so llms.txt (US only) and the sitemap (US and the requested GB) produced a mixed pool. The merged pool is refined once more, so one variant survives and the exact requested variant wins when present. Child-sitemap selection resolved the preference alone and could choose an en-au child whose pages neither scope keeps. Children needed by any fallback scope's locale are kept as well. --- src/helpers/get-page-urls.ts | 28 +++++- test/unit/helpers/get-page-urls.test.ts | 108 ++++++++++++++++++++++-- working-notes/page-discovery-notes.md | 9 ++ 3 files changed, 135 insertions(+), 10 deletions(-) diff --git a/src/helpers/get-page-urls.ts b/src/helpers/get-page-urls.ts index ef96ea4..c0914a0 100644 --- a/src/helpers/get-page-urls.ts +++ b/src/helpers/get-page-urls.ts @@ -891,6 +891,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) @@ -909,10 +910,21 @@ export async function getUrlsFromSitemap( // and byte budgets still bound the walk. if (parsed.sitemapIndexUrls.length > 0 && urls.length < collectLimit) { 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 - : filterLocaleSitemaps(parsed.sitemapIndexUrls, knownLocale); + : [knownLocale, ...scopeLocales].reduce( + (kept, locale) => + mergeUrlSets(kept, filterLocaleSitemaps(parsed.sitemapIndexUrls, locale)), + [], + ); for (const subSitemapUrl of filteredSubSitemaps) { if (urls.length >= collectLimit) break; @@ -1080,7 +1092,10 @@ function pathFilterBases(ctx: CheckContext): string[] { const original = getPathFilterBase(ctx); const { preferredLocale, preferredVersion } = ctx.options; const primary = rewriteFilterBase(original, preferredLocale, preferredVersion); - if (!preferredLocale) return [primary]; + // 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]; @@ -1165,11 +1180,16 @@ export async function getPageUrls(ctx: CheckContext): Promise { const sitemapUrls = await getUrlsFromSitemap(ctx, warnings, { pathFilterBase: filterBases }); if (sitemapUrls.length > 0) { sources.push('sitemap'); + // Each source was refined on its own, so they can disagree about the + // locale variant (llms.txt lists only /en-us/, the sitemap also has the + // requested /en-gb/). Refine the merged pool so one variant survives + // and the exact requested variant wins when present (issue #164). + const merged = refineUrls(mergeUrlSets(llmsTxtUrls, sitemapUrls)); return { - urls: mergeUrlSets(llmsTxtUrls, sitemapUrls), + urls: merged, warnings, sources, - originalMdUrls: filteredOriginals, + originalMdUrls: filterOriginalMdUrls(originalMdUrls, merged), }; } diff --git a/test/unit/helpers/get-page-urls.test.ts b/test/unit/helpers/get-page-urls.test.ts index 7f6791e..be413b6 100644 --- a/test/unit/helpers/get-page-urls.test.ts +++ b/test/unit/helpers/get-page-urls.test.ts @@ -2555,14 +2555,10 @@ ${urls.map((url) => `${url}`).join('\n')} }); it('selects the requested same-language variant from a sitemap when both exist', async () => { - // No llms.txt result is cached, so discovery probes the llms.txt and - // sitemap candidates under both the origin and the /en-us subpath. + // 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/*llms.txt', - () => new HttpResponse('', { status: 404 }), - ), http.get( 'http://opt-both-sitemap.local/robots.txt', () => new HttpResponse('', { status: 404 }), @@ -2594,6 +2590,106 @@ ${urls.map((url) => `${url}`).join('\n')} ]); }); + 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 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 diff --git a/working-notes/page-discovery-notes.md b/working-notes/page-discovery-notes.md index 6658d4c..3492ffb 100644 --- a/working-notes/page-discovery-notes.md +++ b/working-notes/page-discovery-notes.md @@ -507,6 +507,15 @@ Copilot review of the first draft found three gaps, all addressed: 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 (the merged pool is refined once more, so the exact + requested variant wins); 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 From 9fbaff9f1a0e6331476909e17394a45c336af66b Mon Sep 17 00:00:00 2001 From: dacharyc Date: Sat, 31 Oct 2026 21:56:29 -0400 Subject: [PATCH 06/10] fix: merge path-scoped pools before locale refinement Refining each source and then refining the merge could not recover pages of the winning variant that only one source listed: with default en, llms.txt holding three US and two GB pages and a sitemap holding the same US pages plus four other GB pages, the result was four GB pages instead of six. Keep the path-scoped, unrefined llms.txt pool, fetch the sitemap with a new deferRefinement option that returns path-scoped URLs without the locale/version pass or the maxUrls cap, merge the two, and refine once. deferRefinement is distinct from coverage's skipRefinement: child-sitemap selection and the collection limit behave as in refined discovery. --- src/helpers/get-page-urls.ts | 46 +++++++++++++++++++------ test/unit/helpers/get-page-urls.test.ts | 29 ++++++++++++++++ working-notes/page-discovery-notes.md | 8 +++-- 3 files changed, 70 insertions(+), 13 deletions(-) diff --git a/src/helpers/get-page-urls.ts b/src/helpers/get-page-urls.ts index c0914a0..ebb15dc 100644 --- a/src/helpers/get-page-urls.ts +++ b/src/helpers/get-page-urls.ts @@ -827,6 +827,13 @@ export interface SitemapOptions { 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( @@ -834,7 +841,13 @@ 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; @@ -943,7 +956,7 @@ export async function getUrlsFromSitemap( warnOnNarrowPathPrefix(warnings, 'Sitemap', prefixPath, matchingUrls, examinedUrls); - if (skipRefinement) return urls; + if (skipRefinement || deferRefinement) return urls; const localeFiltered = filterLocalizedUrls( urls, @@ -1136,10 +1149,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 = urls.filter((url) => filterPaths.some((p) => matchesPathPrefix(url, p))); warnOnNarrowPathPrefix(warnings, 'llms.txt', filterPaths[0], scoped.length, urls.length); - return refineUrls(scoped); + return scoped; } /** Filter the originalMdUrls map to a subset of URLs. */ @@ -1157,13 +1171,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; } @@ -1177,14 +1193,22 @@ export async function getPageUrls(ctx: CheckContext): Promise { } // llms.txt is thin — try sitemap to fill the gap - const sitemapUrls = await getUrlsFromSitemap(ctx, warnings, { pathFilterBase: filterBases }); + // 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'); - // Each source was refined on its own, so they can disagree about the - // locale variant (llms.txt lists only /en-us/, the sitemap also has the - // requested /en-gb/). Refine the merged pool so one variant survives - // and the exact requested variant wins when present (issue #164). - const merged = refineUrls(mergeUrlSets(llmsTxtUrls, sitemapUrls)); + const merged = refineUrls(mergeUrlSets(scopedLlmsTxtUrls, sitemapUrls)).slice( + 0, + MAX_SITEMAP_URLS, + ); return { urls: merged, warnings, diff --git a/test/unit/helpers/get-page-urls.test.ts b/test/unit/helpers/get-page-urls.test.ts index be413b6..f471c83 100644 --- a/test/unit/helpers/get-page-urls.test.ts +++ b/test/unit/helpers/get-page-urls.test.ts @@ -2637,6 +2637,35 @@ ${urls.map((url) => `${url}`).join('\n')} ]); }); + 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('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. diff --git a/working-notes/page-discovery-notes.md b/working-notes/page-discovery-notes.md index 3492ffb..cb5a686 100644 --- a/working-notes/page-discovery-notes.md +++ b/working-notes/page-discovery-notes.md @@ -512,8 +512,12 @@ Copilot review of the first draft found three gaps, all addressed: 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 (the merged pool is refined once more, so the exact - requested variant wins); child-sitemap selection resolved the preference + 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); 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). From 692dd0b0814ac3f03f9c40969f382ea99d440343 Mon Sep 17 00:00:00 2001 From: dacharyc Date: Sat, 31 Oct 2026 22:04:44 -0400 Subject: [PATCH 07/10] fix: cap only sitemap additions after the merged refinement pass Capping the merged pool at MAX_SITEMAP_URLS also truncated llms.txt pages: with maxLinksToTest 600, 501 llms.txt pages and one sitemap page yielded 500. maxLinksToTest has no upper bound and llms.txt pages were never subject to the sitemap cap. Keep the single refinement pass and apply the cap to sitemap additions alone. --- src/helpers/get-page-urls.ts | 12 +++++++++--- test/unit/helpers/get-page-urls.test.ts | 26 +++++++++++++++++++++++++ working-notes/page-discovery-notes.md | 4 +++- 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/helpers/get-page-urls.ts b/src/helpers/get-page-urls.ts index ebb15dc..1d4143a 100644 --- a/src/helpers/get-page-urls.ts +++ b/src/helpers/get-page-urls.ts @@ -1205,9 +1205,15 @@ export async function getPageUrls(ctx: CheckContext): Promise { }); if (sitemapUrls.length > 0) { sources.push('sitemap'); - const merged = refineUrls(mergeUrlSets(scopedLlmsTxtUrls, sitemapUrls)).slice( - 0, - MAX_SITEMAP_URLS, + // 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: merged, diff --git a/test/unit/helpers/get-page-urls.test.ts b/test/unit/helpers/get-page-urls.test.ts index f471c83..9e4eee7 100644 --- a/test/unit/helpers/get-page-urls.test.ts +++ b/test/unit/helpers/get-page-urls.test.ts @@ -2666,6 +2666,32 @@ ${urls.map((url) => `${url}`).join('\n')} 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('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. diff --git a/working-notes/page-discovery-notes.md b/working-notes/page-discovery-notes.md index cb5a686..fcf516a 100644 --- a/working-notes/page-discovery-notes.md +++ b/working-notes/page-discovery-notes.md @@ -517,7 +517,9 @@ Copilot review of the first draft found three gaps, all addressed: 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); child-sitemap selection resolved the preference + 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; 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). From 488991a5185033f00a3f5752ea2da448a66705da Mon Sep 17 00:00:00 2001 From: dacharyc Date: Sat, 31 Oct 2026 22:15:32 -0400 Subject: [PATCH 08/10] fix: fetch every same-language child sitemap when resolving by variant Child-sitemap selection resolved a locale variant by counting sitemaps, while the page-level pass counts pages and also sees llms.txt. With two en-gb shards and one en-us shard, only the GB children were fetched; if llms.txt then tipped the page-level choice to en-us, the US sitemap pages had never been collected. Before variants were resolved this case fetched every child, since no bare en key existed. Keep child selection inclusive when it resolves by variant: every same-language, script-compatible child is fetched and the single page-level refinement picks the variant. An exact code still selects its own children alone. sameLanguageVariants exposes the ranked list that resolveLocaleVariant already computed. --- docs/reference/cli.md | 2 +- src/helpers/get-page-urls.ts | 18 ++++--- src/helpers/locale-codes.ts | 43 +++++++++------- test/unit/helpers/get-page-urls.test.ts | 65 +++++++++++++++++++++++++ test/unit/helpers/locale-codes.test.ts | 35 +++++++++++++ working-notes/page-discovery-notes.md | 14 +++++- 6 files changed, 151 insertions(+), 26 deletions(-) diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 4603368..383a25e 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -151,7 +151,7 @@ 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 URLs is used. +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 diff --git a/src/helpers/get-page-urls.ts b/src/helpers/get-page-urls.ts index 1d4143a..dab0ddb 100644 --- a/src/helpers/get-page-urls.ts +++ b/src/helpers/get-page-urls.ts @@ -9,6 +9,7 @@ import { detectLocalePosition, countLocaleCodesAt, resolveLocaleVariant, + sameLanguageVariants, } from './locale-codes.js'; import type { CheckContext, DiscoveredFile, UrlPathPattern } from '../types.js'; @@ -522,14 +523,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', resolving a regional or script - // variant when the exact code is absent (`en` selects `en-us` shards, - // issue #164), then any non-locale sitemaps. + // 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 resolved = - (preferredLocale ? resolveLocaleVariant(preferredLocale, counts) : null) ?? - resolveLocaleVariant('en', counts); - const preferred = resolved ? locales.get(resolved)! : []; + 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; } diff --git a/src/helpers/locale-codes.ts b/src/helpers/locale-codes.ts index 38712f9..d58fd67 100644 --- a/src/helpers/locale-codes.ts +++ b/src/helpers/locale-codes.ts @@ -277,6 +277,29 @@ function scriptOf(code: string): string | undefined { .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. * @@ -289,28 +312,14 @@ function scriptOf(code: string): string | undefined { * * 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 { - const target = preferred.toLowerCase(); - if (available.has(target)) return target; - - const language = target.split('-')[0]; - const targetScript = scriptOf(target); - let best: string | null = null; - let bestCount = -1; - for (const [code, count] of available) { - if (code.split('-')[0] !== language) continue; - const script = scriptOf(code); - if (targetScript && script && script !== targetScript) continue; - if (count > bestCount || (count === bestCount && best !== null && code < best)) { - best = code; - bestCount = count; - } - } - return best; + return sameLanguageVariants(preferred, available)[0] ?? null; } /** diff --git a/test/unit/helpers/get-page-urls.test.ts b/test/unit/helpers/get-page-urls.test.ts index 9e4eee7..7332b1f 100644 --- a/test/unit/helpers/get-page-urls.test.ts +++ b/test/unit/helpers/get-page-urls.test.ts @@ -386,6 +386,22 @@ describe('filterLocaleSitemaps', () => { 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('prefers the locale from the base URL when preferredLocale is set', () => { const urls = [ 'https://example.com/sitemap-el.xml', @@ -2692,6 +2708,55 @@ ${urls.map((url) => `${url}`).join('\n')} 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('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. diff --git a/test/unit/helpers/locale-codes.test.ts b/test/unit/helpers/locale-codes.test.ts index 0d73b8d..6438eda 100644 --- a/test/unit/helpers/locale-codes.test.ts +++ b/test/unit/helpers/locale-codes.test.ts @@ -5,6 +5,7 @@ import { detectLocalePosition, countLocaleCodesAt, resolveLocaleVariant, + sameLanguageVariants, } from '../../../src/helpers/locale-codes.js'; describe('isLocaleSegment', () => { @@ -209,6 +210,40 @@ describe('countLocaleCodesAt', () => { }); }); +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); diff --git a/working-notes/page-discovery-notes.md b/working-notes/page-discovery-notes.md index fcf516a..4b1f1bd 100644 --- a/working-notes/page-discovery-notes.md +++ b/working-notes/page-discovery-notes.md @@ -519,7 +519,19 @@ Copilot review of the first draft found three gaps, all addressed: `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; child-sitemap selection resolved the preference + 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. 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). From 257ec1720f8bc387f2c3e173f2ff570036508247 Mon Sep 17 00:00:00 2001 From: dacharyc Date: Sat, 31 Oct 2026 22:24:20 -0400 Subject: [PATCH 09/10] fix: skip child-sitemap pruning for unrecognized explicit locales Inclusive child selection resolved an explicit preference the locale helper does not recognize (en-custom) by language, keeping the en-us child and dropping fr-fr. The resulting two-locale pool defeated page-level detection, so discovery retained both locales and coverage reported 50% for fully covered custom pages. Before this branch no child matched, so only the non-locale child was kept. A preference isLocaleSegment does not recognize cannot be matched to a child by code, so keep every child and let the page-level exact-prefix filter decide, matching the rule already applied to path scoping. --- src/helpers/get-page-urls.ts | 5 +++ test/unit/helpers/get-page-urls.test.ts | 59 +++++++++++++++++++++++++ working-notes/page-discovery-notes.md | 8 +++- 3 files changed, 71 insertions(+), 1 deletion(-) diff --git a/src/helpers/get-page-urls.ts b/src/helpers/get-page-urls.ts index dab0ddb..05f2e08 100644 --- a/src/helpers/get-page-urls.ts +++ b/src/helpers/get-page-urls.ts @@ -461,6 +461,11 @@ export function filterLocaleSitemaps( preferredLocale?: string | null, ): string[] { if (subSitemapUrls.length < 2) return 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 diff --git a/test/unit/helpers/get-page-urls.test.ts b/test/unit/helpers/get-page-urls.test.ts index 7332b1f..3920902 100644 --- a/test/unit/helpers/get-page-urls.test.ts +++ b/test/unit/helpers/get-page-urls.test.ts @@ -402,6 +402,18 @@ describe('filterLocaleSitemaps', () => { expect(filterLocaleSitemaps(urls, 'en-US')).toEqual([urls[2], urls[4]]); }); + 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', () => { const urls = [ 'https://example.com/sitemap-el.xml', @@ -2757,6 +2769,53 @@ ${urls.map((url) => `${url}`).join('\n')} 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('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. diff --git a/working-notes/page-discovery-notes.md b/working-notes/page-discovery-notes.md index 4b1f1bd..42c68e9 100644 --- a/working-notes/page-discovery-notes.md +++ b/working-notes/page-discovery-notes.md @@ -531,7 +531,13 @@ Copilot review of the first draft found three gaps, all addressed: 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. child-sitemap selection resolved the preference + 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. 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). From 090a27706ed9d6a4ae42b66e9808bcc88d974b3d Mon Sep 17 00:00:00 2001 From: dacharyc Date: Sat, 31 Oct 2026 22:36:25 -0400 Subject: [PATCH 10/10] fix: collect distinct sitemap URLs so duplicates do not skew locale choice Nothing before the final sampler deduplicated sitemap URLs. A page repeated within a sitemap or listed by two shards counted twice in the per-variant totals that now choose a locale, and twice in the narrow-prefix counts. The walker now collects distinct URLs and the child selector fetches a repeated index entry once. Coverage already compared normalized sets, so its rate was unaffected. The #120 budget test served one shared page from every root, which only reached the URL cap because duplicates were kept; each root now serves a distinct page so the test still exercises the cap. --- src/helpers/get-page-urls.ts | 8 +++++ test/unit/helpers/get-page-urls.test.ts | 46 ++++++++++++++++++++++++- working-notes/page-discovery-notes.md | 15 +++++++- 3 files changed, 67 insertions(+), 2 deletions(-) diff --git a/src/helpers/get-page-urls.ts b/src/helpers/get-page-urls.ts index 05f2e08..ea4474c 100644 --- a/src/helpers/get-page-urls.ts +++ b/src/helpers/get-page-urls.ts @@ -461,6 +461,8 @@ 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 @@ -889,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); @@ -896,6 +902,8 @@ export async function getUrlsFromSitemap( return false; } if (!isSameSite(url, matchOrigin)) return false; + if (seen.has(url)) return false; + seen.add(url); examinedUrls++; if (prefixPaths.length > 0 && !prefixPaths.some((p) => matchesPathPrefix(url, p))) { return false; diff --git a/test/unit/helpers/get-page-urls.test.ts b/test/unit/helpers/get-page-urls.test.ts index 3920902..a511102 100644 --- a/test/unit/helpers/get-page-urls.test.ts +++ b/test/unit/helpers/get-page-urls.test.ts @@ -402,6 +402,16 @@ describe('filterLocaleSitemaps', () => { 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 @@ -2816,6 +2826,37 @@ ${urls.map((url) => `${url}`).join('\n')} 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. @@ -3148,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/working-notes/page-discovery-notes.md b/working-notes/page-discovery-notes.md index 42c68e9..e7de334 100644 --- a/working-notes/page-discovery-notes.md +++ b/working-notes/page-discovery-notes.md @@ -537,7 +537,20 @@ Copilot review of the first draft found three gaps, all addressed: 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. child-sitemap selection resolved the preference + 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).