Skip to content

docs: add async engine dev note#490

Open
andreatgretel wants to merge 10 commits intomainfrom
andreatgretel/docs/async-blog
Open

docs: add async engine dev note#490
andreatgretel wants to merge 10 commits intomainfrom
andreatgretel/docs/async-blog

Conversation

@andreatgretel
Copy link
Copy Markdown
Contributor

📋 Summary

Add "Async All the Way Down" dev note covering the async task-queue scheduler and its impact on Data Designer pipeline performance. Covers the full async engine arc (PRs #356, #378, #404, #429, #456) in a single narrative post with benchmark results and original diagrams.

🔄 Changes

✨ Added

  • docs/devnotes/posts/async-engine.md - dev note post (~1600 words, slop-guard 93/100)
  • docs/devnotes/posts/assets/async-engine/ - 6 figures (NVIDIA-styled, dark background + green accent):
    • AI-generated hero image
    • Sync vs async Gantt timeline (values derived from real trace data)
    • DAG shape illustrations (4 benchmark workloads)
    • Grouped bar chart (sync vs async wall clock times)
    • Speedup scaling chart
    • Architecture layers SVG diagram

🔧 Changed

🔍 Attention Areas

⚠️ Reviewers: Please pay special attention to the following:

  • async-engine.md - technical claims were cross-checked against implementation code (Kahn's algorithm, AIMD, symmetric bridging, semaphores, etc.) and benchmark scripts (DAG shapes, column dependencies). The "At higher record counts" section discusses rate-limiting tradeoffs qualitatively.
  • Benchmark data is from 10-record runs. Supporting 20-record and 50-record data exist in tmp_blog_content/ (not committed) for reference.

🤖 Generated with AI

- Fix wall-clock claim: 41% -> 22% to match benchmark table
- Fix dual-model speedup rounding: 1.7x -> 1.6x (10.0/6.1 = 1.64)
- Fix run_config API: use dd.set_run_config() instead of passing to create()
Add "Async All the Way Down" dev note covering the async task-queue
scheduler built across PRs #356, #378, #404, #429, #456. Includes
benchmark results, architecture diagrams, and DAG shape illustrations.
Build MkDocs site on PRs that touch docs and deploy to Cloudflare
Pages. Each PR gets a browseable preview URL posted as a comment.
Notebook tutorials use placeholder stubs since they require API
keys to execute.

Requires CLOUDFLARE_API_TOKEN and CLOUDFLARE_ACCOUNT_ID repo secrets.
@andreatgretel andreatgretel requested a review from a team as a code owner April 2, 2026 15:48
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 2, 2026

Greptile Summary

This PR adds the "Async All the Way Down" dev note documenting the new cell-level async execution engine, along with six supporting figures, author metadata, a nav entry, and a new Cloudflare Pages preview workflow. All three technical accuracy issues flagged in the prior review round (percentage claim, speedup rounding, run_config API usage) have been corrected.

Confidence Score: 5/5

Safe to merge; the one open finding (fork PR secrets) is a P2 ops concern that doesn't block the docs content from being published.

All previously identified P0/P1 issues are resolved. The only remaining finding is a P2 workflow limitation (Cloudflare deploy silently failing for fork PRs) that doesn't affect correctness of the documentation or any production path.

.github/workflows/docs-preview.yml — secrets not available for fork-triggered pull_request events.

Important Files Changed

Filename Overview
docs/devnotes/posts/async-all-the-way-down.md New ~1,600-word dev note covering the async execution engine; prior review concerns (% claim, speedup rounding, run_config API) are all resolved in this revision.
.github/workflows/docs-preview.yml New CI workflow that builds and deploys a Cloudflare Pages preview; secrets won't be available for fork PRs, causing the deploy step to fail for external contributors.
docs/devnotes/.authors.yml Adds amanoel author entry with name, description, and GitHub avatar URL.
mkdocs.yml Adds nav entry for the new dev note in most-recent-first position, consistent with existing ordering convention.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    PR[PR opened/synchronized] --> Trigger{paths match?}
    Trigger -->|yes| Build[Checkout + Install deps]
    Build --> Stubs[Create notebook stubs]
    Stubs --> MkDocs[mkdocs build]
    MkDocs --> Deploy[wrangler-action: pages deploy]
    Deploy -->|secrets available| CF[Cloudflare Pages preview]
    Deploy -->|fork PR — secrets empty| Fail[❌ Auth failure]
    CF --> FindComment[find-comment]
    FindComment --> PostComment[create-or-update-comment\nwith preview URL]
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: .github/workflows/docs-preview.yml
Line: 4-5

Comment:
**Cloudflare deploy will silently fail for fork PRs**

The `pull_request` event does not expose repository secrets (`CLOUDFLARE_API_TOKEN`, `CLOUDFLARE_ACCOUNT_ID`) for workflows triggered by forks. When an external contributor opens a docs PR, the deploy step at line 81 will receive empty credentials and fail, marking the entire workflow run as failed and never posting the preview URL comment.

If external contributors are expected to use this preview, you'd need to split the deploy into a separate `pull_request_target` job (with care around the security implications) or add a conditional to skip the deploy gracefully when secrets are absent:
```yaml
      - name: Deploy to Cloudflare Pages
        if: ${{ secrets.CLOUDFLARE_API_TOKEN != '' }}
        id: deploy
```
If this preview is intentionally internal-only (NVIDIA contributors only), adding a comment noting that is enough.

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

Reviews (11): Last reviewed commit: "replace hero image" | Re-trigger Greptile

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

Docs preview: https://f922fd2a.dd-docs-preview.pages.dev

Notebook tutorials are placeholder-only in previews.

@andreatgretel andreatgretel force-pushed the andreatgretel/docs/async-blog branch from 7055573 to e434aad Compare April 2, 2026 18:01
andreatgretel and others added 3 commits April 2, 2026 15:14
Add DAG subtitle to sync-vs-async timeline figure and bridge the
surrounding text to explain which workload shape is being shown.
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