Skip to content

fix: sync Fern root config during devnote publish#714

Closed
johnnygreco wants to merge 1 commit into
mainfrom
johnny/fix-fern-devnotes-theme-sync
Closed

fix: sync Fern root config during devnote publish#714
johnnygreco wants to merge 1 commit into
mainfrom
johnny/fix-fern-devnotes-theme-sync

Conversation

@johnnygreco
Copy link
Copy Markdown
Contributor

@johnnygreco johnnygreco commented May 29, 2026

📋 Summary

Fixes the Fern devnote publish path so docs-website adopts current Fern root config from main instead of combining current support files with stale root config. This lets root Fern configuration changes, including the NVIDIA global theme migration, propagate through devnote publishing while preserving the historical version archive.

🔗 Related Issue

N/A — fixes failing CI job https://github.com/NVIDIA-NeMo/DataDesigner/actions/runs/26648943945/job/78541965075

🔄 Changes

  • Sync fern/docs.yml and fern/fern.config.json during patch-devnotes, while preserving the published branch's historical versions: block.
  • Fail clearly if required Fern root config files are missing from the source checkout.
  • Add a regression test for the generic patch-devnotes contract: source root config wins, historical published versions survive, devnote assets are refreshed, and devnote pages are patched.

🧪 Testing

  • make test passes (not run; targeted docs publish script change)
  • Unit tests added/updated: uv run --group dev pytest packages/data-designer/tests/docs/test_fern_published_branch.py -q
  • E2E tests N/A — validated docs publish behavior with a temporary origin/main + origin/docs-website worktree and make check-fern-docs

Additional validation:

  • uv run --group dev ruff check fern/scripts/fern-published-branch.py packages/data-designer/tests/docs/test_fern_published_branch.py
  • uv run --group dev ruff format --check fern/scripts/fern-published-branch.py packages/data-designer/tests/docs/test_fern_published_branch.py
  • uv run python scripts/update_license_headers.py --check

✅ Checklist

  • Follows commit message conventions
  • Commits are signed off (DCO)
  • Architecture docs updated (N/A — docs publish behavior only)

@johnnygreco johnnygreco requested a review from a team as a code owner May 29, 2026 16:49
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 29, 2026

MkDocs preview: https://856a554f.dd-docs-preview.pages.dev

Fern preview: https://nvidia-preview-pr-714.docs.buildwithfern.com/nemo/datadesigner

Fern previews include the docs-website version archive with PR changes synced into latest. Notebook tutorials are rendered without execution outputs in previews.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 29, 2026

Greptile Summary

This PR fixes the patch-devnotes publish path so the docs-website branch picks up the current Fern root config (docs.yml, fern.config.json) from main instead of carrying stale branding keys (e.g., legacy footer / logo paths) into every devnote deploy.

  • Adds sync_fern_root_config which snapshots the published branch's versions: block, overwrites both root config files from the source checkout, restores the preserved versions, and validates redirect targets — all before the existing devnote support-path copies run.
  • Adds copy_required_path that fails loudly when a required source file is absent, preventing a silent no-op copy.
  • Adds a focused regression test that confirms global-theme keys are adopted and historical version entries are preserved across a patch_devnotes call.

Confidence Score: 5/5

Safe to merge — the change is narrowly scoped to the devnote publish script and its new regression test, with no impact on runtime application code.

The preserve-then-restore pattern for the versions: block is correct: versions_block and restore_versions_block are pre-existing, well-tested helpers, and the new sync_fern_root_config composes them in the right order. copy_required_path correctly surfaces missing source files rather than silently skipping them. The regression test exercises the exact failure case described in the PR and verifies both that stale keys are gone and that historical version entries survive.

No files require special attention.

Important Files Changed

Filename Overview
fern/scripts/fern-published-branch.py Adds FERN_ROOT_CONFIG_PATHS, copy_required_path, and sync_fern_root_config; calls sync_fern_root_config at the top of patch_devnotes to overwrite the published docs.yml and fern.config.json with source-branch content while preserving and restoring the published branch's historical versions: block.
packages/data-designer/tests/docs/test_fern_published_branch.py New regression test that exercises the stale-logo-config scenario end-to-end: source has global-theme: nvidia and one version; published has legacy layout keys and two versions. Verifies the root config keys are replaced and the historical v0.6.0 version is preserved.

Sequence Diagram

sequenceDiagram
    participant CI as CI / patch-devnotes
    participant S as source checkout (main)
    participant P as published branch (docs-website)

    CI->>P: read fern/docs.yml → extract + normalize versions: block
    CI->>S: copy_required_path(fern/docs.yml) → P/fern/docs.yml
    CI->>S: copy_required_path(fern/fern.config.json) → P/fern/fern.config.json
    CI->>P: restore preserved versions: block into P/fern/docs.yml
    CI->>P: validate_redirect_targets(published_root)
    CI->>S: copy FERN_DEVNOTE_SUPPORT_PATHS (assets, components, styles)
    CI->>S: extract Dev Notes block from versions/latest.yml
    CI->>P: rewrite + replace Dev Notes block in P/fern/versions/latest.yml
    CI->>P: write publish metadata
Loading

Reviews (2): Last reviewed commit: "fix: sync Fern root config during devnot..." | Re-trigger Greptile

nabinchha
nabinchha previously approved these changes May 29, 2026
@github-actions
Copy link
Copy Markdown
Contributor

PR #714 Review: fix: sync Fern root config during devnote publish

Summary

This PR fixes a stale-config bug in the Fern devnote publish path. Before this change, patch-devnotes only synced the Dev Notes navigation block plus a curated set of asset/component/style paths (FERN_DEVNOTE_SUPPORT_PATHS), leaving the published branch's fern/docs.yml and fern/fern.config.json untouched. The result: new dev-note components landed alongside a legacy docs.yml that still pointed at local NVIDIA SVGs and lacked global-theme: nvidia, blocking the global-theme migration from rolling forward through devnote publishes.

The fix introduces a sync_fern_root_config step that overwrites both root config files from source while preserving the published branch's versions: block (since historical version entries live only on docs-website). The behavior mirrors the existing preservation logic in sync_source, so the two publish paths stay consistent. A regression test exercises the legacy-logo → NVIDIA-theme migration end-to-end.

Findings

Correctness

  • Versions block preservation is structurally correct. sync_fern_root_config reads/normalizes the published versions: block first, copies the source files, then restores the preserved block before redirect validation (fern/scripts/fern-published-branch.py:330-335). Order matches sync_source (line 344-358), which is the right precedent.
  • Operation order in patch_devnotes is sound. sync_fern_root_config runs before FERN_DEVNOTE_SUPPORT_PATHS copy (line 404-407). That means even if the new source docs.yml references logo assets, the subsequent fern/assets copy will populate them. The published latest.yml existence check on line 401-402 still gates the whole operation, so the new copies don't leak into a half-initialized tree.
  • Edge cases covered by existing helpers. versions_block returns None when published_root/fern/docs.yml is missing or malformed; normalize_latest_display_name and restore_versions_block both no-op on None. So a fresh docs-website (no prior docs.yml) won't crash — the source file will land verbatim with no version-block preservation, which is the correct semantics.
  • validate_redirect_targets is now called twice in the patch flow if any future caller chains it, but currently it's only invoked once per path (sync vs patch), so no double-cost concern.

Code Quality / Conventions

  • copy_required_path is a tidy abstraction — wraps copy_path with a fail-fast check. Naming and placement next to copy_path are consistent with the file's style.
  • Module constant FERN_ROOT_CONFIG_PATHS is parallel to FERN_DEVNOTE_SUPPORT_PATHS and RETIRED_REFERENCE_CLEAN_PAGE_PATHS — good fit.
  • Minor duplication. The two-line preserved_versions_block = normalize_latest_display_name(versions_block(...)) + restore_versions_block(...) pattern now exists in both sync_source and sync_fern_root_config. Not worth extracting yet (still only two call sites), but a future third caller would justify a helper.
  • from __future__ import annotations present in both new/modified files; type annotations on all new functions; matches AGENTS.md invariants.

Tests

  • Single happy-path test, but it's the right test. test_patch_devnotes_syncs_current_fern_config_without_legacy_logo_assets reproduces exactly the bug this PR fixes: stale docs-website logo config + new global-theme source. Assertions cover (a) new theme present, (b) legacy logo paths gone from docs.yml, (c) legacy SVG asset files removed, (d) new devnote asset present, (e) historical v0.6.0 version entry preserved, (f) fern.config.json updated. Comprehensive.
  • Script-loading via importlib.util.spec_from_file_location is the correct workaround for the hyphenated fern-published-branch.py filename. Path traversal parents[4] is a bit fragile (will silently break if the test moves directories), but acceptable for an isolated docs-publish test.
  • Not covered: the copy_required_path fail-fast behavior when source files are missing. Adding a one-liner test (pytest.raises(PublishedBranchError) after deleting source/fern/fern.config.json) would lock in the new error contract. Not a blocker.

Risk / Blast Radius

  • Scope is tight. Only fern/scripts/fern-published-branch.py (a CI publish helper) and a new test file. No runtime code paths in the engine or interface packages.
  • Backward compat: the change overwrites published docs.yml and fern.config.json on every devnote publish. If a maintainer ever hand-edited those on docs-website directly, those edits will now be clobbered. This is the intended behavior (the whole point is to stop drift), but worth noting in case there are operational expectations to update.
  • No secrets, no permissions, no external calls.

Verdict

LGTM. Targeted fix, mirrors existing publish-path conventions, regression test pins the behavior. Optional nit: consider a small negative test for copy_required_path's missing-source error to harden the contract, but not required to merge.

Signed-off-by: Johnny Greco <jogreco@nvidia.com>
@johnnygreco johnnygreco force-pushed the johnny/fix-fern-devnotes-theme-sync branch from c3587b8 to 36c9d03 Compare May 29, 2026 16:56
lbliii added a commit that referenced this pull request May 29, 2026
Incorporates johnnygreco's fix so the devnote publish path syncs the
updated fern/docs.yml (global-theme) and fern/fern.config.json (CLI pin)
to docs-website instead of combining new support files with stale root
config — required for this PR's theme migration to propagate.

# Conflicts:
#	fern/scripts/fern-published-branch.py
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