fix: resolve locale preferences to same-language variants by dacharyc · Pull Request #165 · agent-ecosystem/afdocs

dacharyc · 2026-11-01T00:59:02Z

Summary

Closes #164. Builds on #163, now merged.

Locale preferences were matched by exact code in three places: the locale-sitemap selector, the page-URL filter, and the coverage check's explicit-locale branch. A site organized by en-us and fr-fr matched neither the default en nor a bare --doc-locale en, so no locale sitemap was selected, every locale's pages stayed in the sample, and coverage treated en as the unprefixed default and dropped the English URLs. The reverse mismatch, --doc-locale en-US against a site that uses bare en, failed the same way. Microsoft Learn worked only because its base URL supplies en-us.

resolveLocaleVariant in src/helpers/locale-codes.ts is now the one place this is decided. An exact match wins. Otherwise the best-represented code with the same language is chosen, scripts must agree when both sides name one (zh-Hans resolves to zh-Hans-CN, never to zh-Hant), ties break alphabetically, and no same-language code resolves to null so the unprefixed fallback from #158 still applies. All three call sites use it. The page-URL filter still tries the exact raw segment first, so a site-specific code such as latam keeps working. One variant is chosen rather than every same-language variant so regional duplicates (en-us plus en-gb) do not both enter the page sample.

The CLI reference and the coverage page describe the variant matching, and the decision with rejected alternatives is in working-notes/page-discovery-notes.md. One existing test that pinned the old "no match returns every locale" sitemap behavior is updated to the new expectation.

Test plan

npm test (2041 passing), npm run lint, npm run format:check
New resolver, sitemap, page-filter, and coverage tests fail against the fix: recognize script-subtag locale codes such as zh-Hans #163 branch's helpers and pass here
Null-resolution case pinned so en against {es, fr, zh} still falls back to unprefixed pages

Copilot

🟡 Changes recommended

Unresolved locale-selection bugs can produce false 0% coverage by choosing the wrong locale or prematurely filtering sitemaps, and can exclude valid pages under locale-prefixed base URLs.

3 open issues

What changed in this PR

This PR addresses #164 by sharing locale-variant resolution across sitemap selection, page discovery, and llms.txt coverage, building on the script-subtag support in #163.

Changes:

Adds exact-first, same-language matching with script compatibility, representation counts, and alphabetical tie-breaking.
Integrates the resolver into all three locale-selection paths and adds regression tests.
Documents variant matching and the reasons for selecting a single variant.

File	Description
working-notes/page-discovery-notes.md	Records the resolution strategy and rejected alternatives.
test/unit/helpers/locale-codes.test.ts	Tests locale counting and variant resolution.
test/unit/helpers/get-page-urls.test.ts	Tests variant selection for sitemaps and page URLs.
test/unit/checks/llms-txt-coverage.test.ts	Tests coverage with a regional locale variant.
src/helpers/locale-codes.ts	Adds shared counting and resolution helpers.
src/helpers/get-page-urls.ts	Applies variant matching during discovery.
src/checks/observability/llms-txt-coverage.ts	Resolves explicit locale preferences for coverage.
docs/reference/cli.md	Explains locale-variant matching for CLI users.
docs/checks/observability.md	Updates coverage locale-selection documentation.

_{Review effort: Balanced}

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

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.

Copilot

🟡 Changes recommended

The locale filter retains /en-us when en-GB is requested and available, discarding GB links and returning US pages; locale resolution must precede path narrowing, with regression coverage for both variants being present.

1 open issue

Resolve requested locale before filtering links to the base locale · New

3 resolved since last review

_{Review effort: Balanced}

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.

… 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.

Copilot

🟡 Changes recommended

Locale-selection defects can mix requested and fallback variants, ignore an exact custom-locale prefix, or miss usable sitemap pages due to incompatible child-sitemap selection.

2 open issues

1 previously missed issue

In code that hasn't changed since last review

Include sitemap children compatible with fallback scopes

src/helpers/get-page-urls.ts:915

Child-sitemap selection does not account for the new fallback path scope. For base /en-us with --doc-locale en-GB, an index containing equally represented sitemap-en-au.xml and sitemap-en-us.xml resolves to en-au. Only that child is fetched, then its URLs are rejected by the /en-gb and /en-us scopes, so discovery falls back to the base URL despite a usable US sitemap. Keep the children needed by either scope, or restrict variant selection to locales compatible with those scopes.

1 resolved since last review

Resolve requested locale before filtering links to the base locale

_{Review effort: Balanced}

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.

dacharyc · 2026-11-01T01:41:41Z

Follow-up to the third finding in this review (child-sitemap selection and fallback scopes), which has no inline thread.

Confirmed and fixed in d410559. The sitemap walker now derives the fallback scopes' locales from the extra path bases and keeps the children needed by any of them, merged with the preferred locale's children in order. Regression test: base /en-us, --doc-locale en-GB, index with sitemap-en-au.xml and sitemap-en-us.xml, expects the US pages from the en-us child rather than a base-URL fallback.

Copilot

🟡 Changes recommended

The merge uses already locale-filtered inputs, discarding unique pages from the winning locale variant instead of preserving all pages from both sources.

1 open issue

Preserve unfiltered pages until the final locale merge · New

2 resolved since last review

_{Review effort: Balanced}

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.

Copilot

🟡 Changes recommended

The sitemap’s 500-URL cap now also limits llms.txt pages, dropping valid discovered pages even when maxLinksToTest allows more.

1 open issue

Sitemap cap incorrectly truncates llms.txt pages · New

1 resolved since last review

Preserve unfiltered pages until the final locale merge

_{Review effort: Balanced}

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.

Copilot

🔵 Needs a closer look

Human assessment is needed because indexed-sitemap locale pruning can mix locales, skip valid base-locale coverage, and lose pages from the ultimately selected locale variant.

No open issues 🎉

1 resolved since last review

Sitemap cap incorrectly truncates llms.txt pages

_{Review effort: Balanced}

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.

dacharyc · 2026-11-01T02:15:41Z

Follow-up to the latest review summary (indexed-sitemap locale pruning), which raised no inline finding.

Checked the three concerns against the code. Mixing locales in the raw pool is by design: the single page-level pass picks one variant. Base-locale coverage in the raw walk is unaffected: with a known locale the walk filters to it, and with none it keeps every child. The third concern was real: child-sitemap selection resolved a variant by counting sitemaps, while the page-level pass counts pages and sees llms.txt, so two GB shards and one US shard fetched GB only, and if llms.txt tipped the page pass to en-us the US sitemap pages had never been fetched.

Fixed in 488991a: when child selection resolves by variant rather than exact match, every same-language, script-compatible child is fetched and the page-level pass decides. An exact code still selects its own children alone. Regression tests cover the selector and an end-to-end index with two GB shards, one US shard, and US pages in llms.txt, expecting the US sitemap page to be included.

Copilot

🟡 Changes recommended

Sitemap pruning for unrecognized explicit locale preferences breaks locale detection, retaining multiple locales and incorrectly reporting 50% coverage for fully covered custom-locale pages.

1 open issue

Skip sitemap pruning for unrecognized explicit locales · New

_{Review effort: Balanced}

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.

Copilot

🔵 Needs a closer look

Human assessment is needed because fallback discovery can overlook an exact locale match, while duplicate sitemap URLs can skew regional selection and coverage.

No open issues 🎉

1 resolved since last review

Skip sitemap pruning for unrecognized explicit locales

_{Review effort: Balanced}

…hoice 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.

dacharyc · 2026-11-01T02:36:34Z

Follow-up to the latest review summary, which raised two concerns without an inline finding.

Exact locale match overlooked by fallback discovery. Checked each selection point: the page filter tests the exact raw segment first, coverage tests the exact prefix first, the sitemap selector returns only the exact code's children when present, and the dual scope's primary path is the exact requested locale. The one fallback that ignores the preference is the final base-URL fallback at the end of discovery, which deliberately uses the original base because a rewritten locale path may not exist. No change.

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. Fixed in 090a277: the walker collects distinct URLs (counted once for the narrow-prefix warning as well) and the child selector fetches a repeated index entry once. Coverage already compared normalized sets, so its rate was unaffected. Regression tests: a GB page listed three times against two distinct US pages now resolves the default en to US, and a repeated index entry is fetched once. One #120 budget fixture served the same page from all 20 roots and only reached the URL cap because duplicates were kept; it now serves a distinct page per root.

Copilot

🟡 Changes recommended

The changes can alter scores for unchanged sites, but the required scoring-version bump, history update, and synchronized documentation are missing.

1 open issue

Bump scoring version and synchronize scoring documentation · New

_{Review effort: Balanced}

Base automatically changed from fix/issue-161-script-subtag-locales to main November 1, 2026 01:03

dacharyc requested a balanced review from Copilot November 1, 2026 01:03

Copilot started reviewing on behalf of dacharyc November 1, 2026 01:03

View session