fix: restore Fern fallback live reload#788
Conversation
|
Fern preview: https://nvidia-preview-pr-788.docs.buildwithfern.com/nemo/datadesigner
|
|
Nice work on this one, @andreatgretel — the incremental-sync approach is a clean way to get live reload back without a file-watching dependency. SummaryThis restores live reload for the unauthenticated Fern fallback by materializing the preview tree as real files (instead of source-pointing symlinks) and polling the source tree every 250 ms to mirror additions, edits, deletions, FindingsSuggestions — Take it or leave it
What Looks Good
Structural Impact (graphify, 2.3s)Risk: LOW (localized change)
The structural analysis confirms a localized change contained to the VerdictShip it (with nits) — no blocking issues. The suggestions above (especially widening the transient-error retry and adding a test for the This review was generated by an AI assistant. |
|
Thanks for putting this together, @andreatgretel — the focused polling approach keeps the fallback self-contained and restores the expected local workflow. SummaryThis PR replaces source-pointing symlinks with a materialized Fern preview tree and incrementally mirrors source additions, edits, deletions, transformed FindingsWarnings — Worth addressing
What Looks Good
VerdictNeeds changes — preserve prior tracking for failed copies and add the race regression test so a permanent deletion cannot leave stale preview content behind. This review was generated by an AI assistant. |
|
Fixed. Failed copies for paths that existed in the previous snapshot now retain their previous state. If the source remains deleted, the next poll detects the removal and deletes the stale preview file and any component alias. Genuinely new failed paths remain untracked so they retry as additions. I added a regression test that deletes a changed source during copying, runs another sync, and verifies the stale preview file is removed. |
|
Thanks for putting this together, @andreatgretel — nice detective work tracing the broken reload back to the source-pointing symlinks. SummaryThis restores live reload for the unauthenticated Fern fallback by materializing the preview tree as real files inside Fern's watched temp dir and polling the source every 250 ms to mirror additions, changes, deletions, FindingsWarnings — Worth addressing
except (FileNotFoundError, yaml.YAMLError, AttributeError):
failed_paths.add(relative_path)Keeping the "fail loud and stop the child" behavior for genuinely unexpected errors is reasonable, but transient invalid config while editing shouldn't be fatal. Suggestions — Take it or leave it
What Looks Good
VerdictNeeds changes — one thing to address before merge:
Nice work overall — this is a well-scoped fix with solid tests. This review was generated by an AI assistant. |
|
Addressed in
Validation: 11 focused tests passed, Ruff passed, and Fern check reported 0 errors. I left the optional two-poll deletion debounce unchanged because it only avoids cosmetic reload churn while delaying legitimate deletions. |
|
Follow-up pass on the latest changes, @andreatgretel — just a couple of things left. Warnings — Worth addressing
Commits are not DCO signed-off — the checklist flags this; if DCO is enforced in CI it'll block merge. Suggestions — Take it or leave it
VerdictNeeds changes — no happy-path correctness issues; tightening the This review was generated by an AI assistant. |
|
Addressed in |
There was a problem hiding this comment.
Both findings from the prior review are resolved — snapshot_source now catches OSError (with a new test_snapshot_source_skips_unreadable_file regression test), and copy_preview_file uses the explicit source if target is None else target form. Ruff is clean and all 12 tests pass locally.
Approving on the code. Nice work on this one.
This review was generated by an AI assistant.
📋 Summary
The unauthenticated Fern fallback served source files through symlinks outside Fern's watched temporary tree, so documentation edits did not trigger live reload. This change keeps the deterministic NVIDIA-style fallback while mirroring source changes into real files that Fern can watch.
🔗 Related Issue
N/A
🔄 Changes
docs.ymlupdates.🔍 Attention Areas
serve-local-docs-preview.py- incremental synchronization and child-process lifecycle behavior.🧪 Testing
.venv/bin/ruff check .passes..venv/bin/ruff format --check .passes..venv/bin/pytest fern/scripts/tests/test_serve_local_docs_preview.pypasses with 6 tests.make check-fern-docspasses with 0 errors.docs.ymledits trigger Fern reloads and render updated content.✅ Checklist