Skip to content

fix: make Fern release publishing resilient#818

Open
andreatnvidia wants to merge 2 commits into
mainfrom
andreatgretel/fix/fern-release-publishing
Open

fix: make Fern release publishing resilient#818
andreatnvidia wants to merge 2 commits into
mainfrom
andreatgretel/fix/fern-release-publishing

Conversation

@andreatnvidia

@andreatnvidia andreatnvidia commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

📋 Summary

Ensures every DataDesigner release publishes an actual vX.Y.Z Fern docs version even when external model providers make notebook execution fail. It also keeps Latest's Dev Notes and Recipes rolling from main, preserves frozen release content, and restores reliable manual workflow dispatch.

🔗 Related Issue

N/A - requested from the DataDesigner docs release discussion.

🔄 Changes

  • Decouple release snapshot creation from notebook execution so a transient provider failure cannot block the versioned docs release.
  • Publish source notebooks as a fallback when executed notebook artifacts are unavailable, while leaving the notebook failure visible in CI.
  • Restore Dev Notes and Recipes from main during the release transaction without changing frozen version navigation, pages, or release metadata.
  • Add public post-publish checks for both Latest and the new versioned URL.
  • Reuse notebook artifacts only when source and runtime fingerprints match, and publish provenance hashes with each artifact.
  • Retry notebook execution with backoff and skip redundant model health probes in docs CI while retaining real generation requests.
  • Restore setup-uv v8.1.0 after a workflow-dispatch bisect confirmed v8.3.x fails before creating a job; checkout v7 remains enabled.
  • Add regression coverage for the CI health-check bypass and rolling-content preservation.

🔍 Attention Areas

⚠️ Reviewers: Please pay special attention to the release fallback and cache provenance behavior.

  • build-fern-docs.yml - intentionally publishes source notebooks if execution fails so the versioned release still goes live.
  • build-notebooks.yml - accepts cached artifacts only with matching source and runtime fingerprints.
  • readiness.py - the new model-probe bypass is enabled only by the notebook CI environment; real notebook generation calls still run.

🧪 Testing

  • make test passes (not run locally; the full GitHub CI matrix passes)
  • Unit tests added/updated - 15 focused readiness and Fern published-branch tests pass
  • E2E tests added/updated - N/A; no E2E test changes, and the existing E2E matrix passes
  • make check-fern-docs-locally passes
  • Workflow YAML, shell syntax, retry/cache simulation, and manual workflow startup validated
  • All PR checks pass, including Fern docs preview and coverage

✅ Checklist

  • Follows commit message conventions
  • Commits are signed off (DCO)
  • Architecture docs updated - N/A; no architecture changes

@github-actions

Copy link
Copy Markdown
Contributor

Fern preview: https://nvidia-preview-pr-818.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.

@andreatnvidia
andreatnvidia marked this pull request as ready for review July 14, 2026 19:52
@andreatnvidia
andreatnvidia requested a review from a team as a code owner July 14, 2026 19:52
@greptile-apps

greptile-apps Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR makes Fern docs publishing less dependent on live notebook execution. The main changes are:

  • Release snapshots are prepared before notebook execution completes.
  • Published docs can fall back to source notebooks when executed artifacts are missing.
  • Notebook cache entries now include source and runtime fingerprints.
  • Notebook execution retries transient failures with backoff.
  • Docs workflows pin setup-uv back to v8.1.0.
  • Notebook CI can skip model health probes while still running generation calls.

Confidence Score: 4/5

The changed flow looks mergeable after fixing the manual notebook cache toggle.

  • Manual runs that disable caching can still pass USE_CACHE=1 into notebook execution.
  • The release fallback and published-branch changes otherwise line up with the inspected workflow paths.

.github/workflows/build-notebooks.yml

Important Files Changed

Filename Overview
.github/workflows/build-fern-docs.yml Decouples release snapshot preparation and Fern publishing from notebook execution, adds source-notebook fallback, restores rolling content, and verifies published URLs.
.github/workflows/build-notebooks.yml Adds cache fingerprinting, artifact seeding, retry settings, and model health-check skipping, but now always passes USE_CACHE=1 to notebook execution.
.github/workflows/publish-fern-devnotes.yml Pins setup-uv back to v8.1.0 for the Dev Notes publishing workflow.
docs/scripts/build_notebooks_cached.sh Adds retry validation, retry cleanup, and runtime-context-aware notebook cache hashes.
packages/data-designer-engine/src/data_designer/engine/readiness.py Adds an environment-variable gate to skip model health checks before generation.
packages/data-designer-engine/tests/engine/test_readiness.py Adds coverage for skipping model health checks through the environment variable.
packages/data-designer/tests/docs/test_fern_published_branch.py Extends published-branch tests for release metadata and versioned Dev Notes and Recipes preservation.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[resolve release] --> B[prepare published release]
  A --> C[build notebooks]
  B --> D[publish Fern docs]
  C --> D
  C -->|success| E[download executed notebooks]
  C -->|failure or cancelled| F[publish source notebook fallback]
  E --> G[check and generate Fern docs]
  F --> G
  G --> H[verify latest and versioned URLs]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
  A[resolve release] --> B[prepare published release]
  A --> C[build notebooks]
  B --> D[publish Fern docs]
  C --> D
  C -->|success| E[download executed notebooks]
  C -->|failure or cancelled| F[publish source notebook fallback]
  E --> G[check and generate Fern docs]
  F --> G
  G --> H[verify latest and versioned URLs]
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
.github/workflows/build-notebooks.yml:97
**Cache Input Is Ignored**

Manual notebook runs with `use_cache: false` still execute `make convert-execute-notebooks USE_CACHE=1`. The cache restore and seed steps are skipped, but any `.notebook-cache` content left in the workspace can still be used by the script, so a run explicitly requesting fresh notebook execution can reuse cached notebooks instead.

```suggestion
          make convert-execute-notebooks ${{ inputs.use_cache && 'USE_CACHE=1' || '' }}
```

Reviews (1): Last reviewed commit: "fix: restore working setup-uv action" | Re-trigger Greptile

export DATA_DESIGNER_FLUX_2_PRO_CREATE_NUM_RECORDS=2
fi
make convert-execute-notebooks ${{ inputs.use_cache && 'USE_CACHE=1' || '' }}
make convert-execute-notebooks USE_CACHE=1

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.

P2 Cache Input Is Ignored

Manual notebook runs with use_cache: false still execute make convert-execute-notebooks USE_CACHE=1. The cache restore and seed steps are skipped, but any .notebook-cache content left in the workspace can still be used by the script, so a run explicitly requesting fresh notebook execution can reuse cached notebooks instead.

Suggested change
make convert-execute-notebooks USE_CACHE=1
make convert-execute-notebooks ${{ inputs.use_cache && 'USE_CACHE=1' || '' }}

Context Used: Do not suggest defensive coding patterns such as a... (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/build-notebooks.yml
Line: 97

Comment:
**Cache Input Is Ignored**

Manual notebook runs with `use_cache: false` still execute `make convert-execute-notebooks USE_CACHE=1`. The cache restore and seed steps are skipped, but any `.notebook-cache` content left in the workspace can still be used by the script, so a run explicitly requesting fresh notebook execution can reuse cached notebooks instead.

```suggestion
          make convert-execute-notebooks ${{ inputs.use_cache && 'USE_CACHE=1' || '' }}
```

**Context Used:** Do not suggest defensive coding patterns such as a... ([source](greptile.json))

How can I resolve this? If you propose a fix, please make it concise.

@github-actions

Copy link
Copy Markdown
Contributor

Thanks for putting this together, @andreatgretel — this untangles a genuinely gnarly release-workflow dependency chain, and the tests + PR writeup make the intent easy to follow.

Summary

This PR makes the Fern release path resilient to transient notebook-execution failures: it decouples release-snapshot preparation and publishing from the notebook job, adds provenance-aware artifact seeding, retries flaky notebook execution with backoff, restores rolling Dev Notes/Recipes into the release snapshot, verifies published URLs, and rolls setup-uv back from v8.3.2 to v8.1.0. It also adds a DATA_DESIGNER_SKIP_MODEL_HEALTH_CHECKS escape hatch so notebook CI can skip redundant model probes while keeping real generation calls. The implementation matches the stated intent, and the structural impact analysis confirms the Python change is localized and low-risk.

Findings

Warnings — Worth addressing

packages/data-designer-engine/src/data_designer/engine/readiness.py:79 — Global env var silently short-circuits the public check_models() API

  • What: _run_model_health_check is the shared code path documented as being invoked from both DatasetBuilder.build/build_preview and the public DataDesigner.check_models(). When DATA_DESIGNER_SKIP_MODEL_HEALTH_CHECKS=1 is set in the environment, a user (or CI job) calling check_models() to explicitly verify provider responsiveness will get a silent success — the model probe is skipped entirely, logged only at info level. The check_models() docstring (data_designer.py:527) still tells readers only that "Models whose ModelConfig has skip_health_check=True are skipped," with no mention of this global override.
  • Why: An operator-level kill-switch that turns an explicit "are my providers responsive?" call into a no-op is easy to trip over — e.g. a shared CI environment that exports this var for the notebook job could inadvertently neuter check_models() in a later step. The module docstring (lines 11-19) explicitly promises the two callers "must use the same code path here so the standalone method cannot drift" — this env var introduces a new drift vector that isn't surfaced.
  • Suggestion: Document the env var in the check_models() docstring and in architecture/models.md (which already describes health-check behavior), noting it globally overrides all model probes. Consider logging at warning level rather than info when the skip fires, since it's subverting an otherwise-explicit request. If the intent is purely a CI/notebook affordance, that's fine — just make the public-API side effect discoverable.

Suggestions — Take it or leave it

.github/workflows/build-notebooks.yml:97USE_CACHE=1 is now hardcoded, but the use_cache input still gates cache restore/seed

  • What: The convert step went from ${{ inputs.use_cache && 'USE_CACHE=1' || '' }} to an unconditional USE_CACHE=1, while the "Restore notebook cache" (line 50) and "Seed cache" (line 59) steps still key off if: inputs.use_cache. When use_cache is false, the build script runs in cache mode against an empty .notebook-cache, so every notebook is a miss and re-executes — functionally correct, just slightly indirect.
  • Why: A future reader may wonder why the convert step ignores the input the rest of the job respects. It's not a bug (the fingerprint check makes stale reuse safe), but the intent isn't obvious from the diff alone.
  • Suggestion: A one-line comment on the convert step ("cache dir is empty when use_cache is false, so this is a no-op reuse") would save the next reader a double-take.

docs/scripts/build_notebooks_cached.sh:78 — Empty NOTEBOOK_CACHE_CONTEXT produces a trailing-colon fingerprint locally

  • What: When NOTEBOOK_CACHE_CONTEXT is unset (local dev), the fingerprint becomes <sha>: with an empty context segment. This is internally consistent within a local run, and CI always sets the var, so cross-environment reuse never mixes the two formats.
  • Why: Purely cosmetic — worth a mental note that a local cache built with an empty context will never match a CI-seeded artifact (they carry a real context hash), which is the desired behavior anyway.
  • Suggestion: No change needed; flagging only so the empty-context case is a known, intended outcome rather than a surprise.

.github/workflows/build-fern-docs.yml:269 — URL verification retry budget may be tight for CDN propagation

  • What: The Verify published docs step polls with --retry 12 --retry-delay 10 --retry-max-time 180 (≈3 min ceiling). --retry-all-errors correctly covers transient 404s while docs.nvidia.com propagates the freshly published version.
  • Why: If CDN/edge propagation for a brand-new versioned slug occasionally exceeds ~3 minutes, this step could fail a release whose publish actually succeeded, turning a verification aid into a false-negative gate.
  • Suggestion: Consider whether a slightly larger --retry-max-time (or treating verification failure as a ::warning:: rather than a hard failure) better matches how quickly the site actually goes live. Leave as-is if 3 minutes is known to be comfortably sufficient.

What Looks Good

  • Provenance-aware seeding is a real correctness upgrade. Pairing the notebook artifact with a notebook-source-hashes artifact and comparing the full source_hash:runtime_context fingerprint (with the cache key now including uv.lock + all package src/) cleanly fixes the "stale outputs blessed with current hashes" bug, and lets you safely drop the old partial-restore guard since mismatches now self-heal by re-executing. Nice.
  • include-hidden-files: true on the source-hashes upload is a subtle but correct detail — the .notebook-cache path component is hidden and would otherwise be silently excluded by upload-artifact.
  • The retry helper in build_notebooks_cached.sh is tidy: input validation up front, rm -f of stale output between attempts, linear backoff, and a clean non-zero return on final failure that set -e propagates to fail the build. Removing actions: read from prepare-published-release alongside deleting its download step is good hygiene too.
  • Test coverage tracks the behavior change — the new test_run_readiness_check_skips_model_probe_from_environment asserts both arun_health_check and the async submit are untouched, and the patch-devnotes test now guards that versioned snapshots (v0.6.0) and publish-metadata.json are preserved while only latest is patched.

Structural Impact (graphify, 2.6s)

Risk: LOW (localized change)

  • 3 Python files, 5 AST entities, 1/81 clusters

High-Connectivity Changes

  • _run_model_health_check() (8 deps) in packages/data-designer-engine/src/data_designer/engine/readiness.py
  • run_readiness_check() (7 deps) in packages/data-designer-engine/src/data_designer/engine/readiness.py
  • _run_mcp_tool_health_check() (6 deps) in packages/data-designer-engine/src/data_designer/engine/readiness.py

Cross-Package Dependencies

  • .check_models() (interface) --calls--> run_readiness_check() (engine)
  • _run_model_health_check() (engine) --calls--> info() (config)
  • _run_model_health_check() (engine) --calls--> .get_model_aliases() (config)

Cross-package edges all flow interface → engine → config, consistent with the layering rule. The high-connectivity node _run_model_health_check() is the one touched by the new env-var short-circuit — see the Warning above re: verifying backward compatibility of the shared check_models() path.

Verdict

Needs changes — nothing here is blocking correctness of the workflow fix, and the single Warning is about discoverability rather than a bug: the DATA_DESIGNER_SKIP_MODEL_HEALTH_CHECKS override silently affects the public check_models() API and should be documented (docstring + architecture/models.md) before merge, ideally with a warning-level log. The remaining items are all take-it-or-leave-it. ruff check and ruff format --check pass on the changed Python files.


This review was generated by an AI assistant.

@nabinchha

Copy link
Copy Markdown
Contributor

Thanks for putting this together, @andreatgretel—the release-resiliency work is thoughtful, and I found a few cross-workflow edge cases worth tightening before merge.

Summary

This PR decouples Fern release snapshotting/publishing from notebook execution, adds retry and provenance handling for notebooks, restores rolling Dev Notes and Recipes, and introduces a notebook-CI model-probe bypass. The implementation largely matches the stated intent, but the cache and fallback paths can publish notebooks from the wrong execution context or regress them later, and cancellation no longer reliably stops deployment.

Findings

Warnings — Worth addressing

.github/workflows/build-notebooks.yml:40 — Cache provenance treats scheduled and release outputs as equivalent

  • What: Release events now enable caching in build-fern-docs.yml:191, while the seed query at line 64 selects the latest successful top-level notebook run on main—currently a scheduled run. Scheduled runs set DATA_DESIGNER_FLUX_2_PRO_CREATE_NUM_RECORDS=2 at lines 94–95, but tutorial 6 defaults to 5 records and neither the Actions cache key nor the per-file fingerprint includes that output-affecting value. On a non-exact restore, the new seeding loop can also overwrite an otherwise valid cached notebook with this scheduled artifact.
  • Why: Once a post-merge scheduled run publishes the new hash artifact, a release with matching source/package hashes can accept and publish the two-record scheduled notebook instead of executing the intended five-record release notebook. That makes the notebook output disagree with its source and defeats the provenance guarantee this PR is adding.
  • Suggestion: Define an explicit execution profile before computing the cache context and include all output-affecting values in both the Actions cache key and each .sha256 fingerprint. Including the builder script/Makefile in that context would also invalidate notebooks when conversion behavior changes. Alternatively, do not allow scheduled artifacts to seed release builds.

.github/workflows/build-fern-docs.yml:230 — A fallback release leaves the next Dev Notes publish vulnerable to stale notebooks

  • What: When build-notebooks fails, this job intentionally publishes from source, but the failed needed job leaves the overall workflow run with conclusion failure. publish-fern-devnotes.yml:110-123 later searches only --status success, downloads notebooks from an older run without the new provenance hashes, and Makefile:581-587 prefers those .ipynb files over the current docs/notebook_source/*.py files.
  • Why: The next unrelated Dev Notes or Recipes publish can replace the current source-derived notebook data with same-named outputs from a pre-release build, silently regressing Latest after the fallback release succeeded publicly.
  • Suggestion: Make the Dev Notes artifact reuse consume notebook-source-hashes and retain only notebooks whose source/runtime fingerprint matches the current published checkout; leave unmatched files absent so the Make target falls back to source. The simpler safe option is to stop reusing old notebook artifacts in that workflow.

.github/workflows/build-fern-docs.yml:196 — Normal cancellation can still reach the publish job

  • What: The condition keeps always() but removes the needs.build-notebooks.result == 'success' guard without excluding cancelled. If release preparation has completed and notebook execution is canceled, the remaining conditions still pass and the job treats cancellation as a source-fallback case.
  • Why: GitHub documents that always() remains true on cancellation, so a maintainer can cancel a release workflow and still have the external Fern deployment proceed.
  • Suggestion: Add !cancelled() or explicitly admit only needs.build-notebooks.result == 'success' || needs.build-notebooks.result == 'failure'. The fallback warning should likewise run on failure, not every non-success result.

What Looks Good

  • Decoupling snapshot creation from provider availability is a sensible way to guarantee that every release gets a real versioned docs URL while preserving the notebook failure signal in CI.
  • Publishing per-notebook fingerprints and removing partial .ipynb files between retries are both solid foundations for safer cache reuse.
  • The Fern regression test now checks idempotence, release metadata preservation, and frozen version nav/page preservation.

Verdict

Needs changes — please address the execution-profile cache mismatch, provenance-aware Dev Notes reuse, and cancellation guard before merge.


This review was generated by an AI assistant.

@andreatnvidia

Copy link
Copy Markdown
Contributor Author

@nabinchha Thanks, these were good catches. I reworked the flow as follows:

Situation Behavior
Notebook build succeeds Publish executed notebooks and persist their checksummed snapshot
Notebook build fails Publish source-rendered notebooks and persist that exact fallback snapshot
Notebook build is cancelled Do not run the publish job
Dev Notes publish Restore the exact persisted snapshot and validate without regenerating notebooks
Manual repair after fallback Rerun Build Fern docs for the same release tag. If notebooks succeed, update the snapshot and republish with outputs

Cache provenance now includes an explicit execution profile, separating scheduled 2-record runs from 5-record release runs. The profile, Makefile, and notebook builder are included in both the cache key and per-notebook fingerprint. Artifact seeding only accepts matching fingerprints and does not overwrite valid cached notebooks. use_cache: false also clears the cache before execution.

Dev Notes no longer search older successful workflow artifacts. Each full release stores its rendered notebooks as a checksummed GitHub Release asset, while docs-website records the exact asset used. This prevents a fallback release from later regressing to stale executed outputs.

The fallback is recoverable without creating another release: manually rerunning Build Fern docs for the same tag replaces the published snapshot when notebook execution succeeds.

The existing docs-website branch will need one successful full docs run after merge to initialize the pointer. Until then, Dev Notes fail early with a clear message.

Validation: 33 docs tests pass, workflow YAML and shell syntax validate, and both generated and restored Fern checks report 0 errors.

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