Skip to content

fix(cli): stamp configured default locale on docs ledger manifest - #17531

Merged
thesandlord merged 1 commit into
mainfrom
devin/1787702962-cli-default-locale-manifest-key
Aug 26, 2026
Merged

fix(cli): stamp configured default locale on docs ledger manifest#17531
thesandlord merged 1 commit into
mainfrom
devin/1787702962-cli-default-locale-manifest-key

Conversation

@Ryan-Amirthan

@Ryan-Amirthan Ryan-Amirthan commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Description

The ledger publish path hardcoded "en" as the locale key for the base (default-locale) segment. buildLedgerInput defaulted its locale parameter to "en" and the base call site never passed one, so a site whose docs.yml declares

translations:
  - lang: en-US
    default: true
  - lang: pt-BR

published defaultLocale: "en" and locales: { en: {...}, "pt-BR": {...} } — the non-default translations kept their real tags (that call site already passes locale), only the base entry was flattened. publishInput.defaultLocale is derived from baseLocale.locale, so it inherited the same wrong value.

A reader that looks up the tag it read from docs.yml (en-US) misses manifest.locales. The Next.js reader survives via ledgerManifestClient's fallback chain (requested locale → manifest.defaultLocale → first published locale); the Astro static build had no such fallback and threw, emitting a docs site with zero routes that still deployed.

The configured default is already on the payload publishDocsLedger receives: DocsDefinitionResolver.getDocsTranslationsConfig() writes it to docsDefinition.config.translations.defaultLocale, so no new parameter or plumbing is needed.

Existing published manifests still key their base segment as en, so reader-side fallbacks must stay in place; this is not a breaking change for them.

Changes Made

  • buildLedgerInput resolves the segment locale as locale ?? docsDefinition.config.translations?.defaultLocale ?? "en" — the base segment (and therefore publishInput.defaultLocale) now uses the configured default, translation segments keep the explicit locale they already pass, and sites with no translations block still publish "en" unchanged.
  • The locale is treated as an opaque string: no normalization, canonicalization, or BCP-47 validation.
  • Changelog entry under packages/cli/cli/changes/unreleased/.

Testing

  • Unit tests added/updated — buildLedgerInput.test.ts: en-US default keys the base entry en-US; a de-DE default is preserved; an explicit pt-BR translation locale round-trips; no translations block still yields en.
  • Manual testing completed
pnpm check:fix    # no fixes applied
pnpm lint:biome   # clean
pnpm turbo run test --filter "*remote-workspace-runner*"   # 24 files, 269 tests passed (includes compile)

Deliberately out of scope: any change in fern-api/fern-platform (the Astro reader fallback is fern-platform#14141), locale-tag validation, and the FDR V2→ledger automigration path.

Link to Devin session: https://app.devin.ai/sessions/4868e3907f9843dd9edca82754a8fcd6
Requested by: @Ryan-Amirthan


Open in Devin Review

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Ported the one case these tests don't cover from the now-closed #17536: a non-English default plus a real en translationnl base + ["en", "de"] → base and translation segments must come back as two distinct keys ["nl", "en"]. That's the collision that made this data-destructive rather than cosmetic, since buildLedgerManifest.ts keys locales as a record, so a base flattened to "en" overwrites the site's actual en translation.

Test only, no production code, targeting this branch: #17539

@thesandlord
thesandlord merged commit 6de61c6 into main Aug 26, 2026
64 checks passed
@thesandlord
thesandlord deleted the devin/1787702962-cli-default-locale-manifest-key branch August 26, 2026 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants