fix(ui): write cached web UI HTML atomically by pragnyanramtha · Pull Request #5490 · pydantic/pydantic-ai

pragnyanramtha · 2026-05-16T12:07:45Z

Summary

Addresses #5399, cache race item.

write cached web UI HTML through a same-directory temporary file and os.replace()
treat empty cache files as cache misses so zero-byte files are repaired on the next fetch
add regression coverage for empty cache recovery

Testing

uv run pytest tests/test_ui_web.py -k "get_ui_html_refetches_empty_cache_file or get_ui_html_cdn_fetch or get_ui_html_filesystem_cache_hit or get_ui_html_custom_url_caching"
uv run ruff check pydantic_ai_slim/pydantic_ai/ui/_web/app.py tests/test_ui_web.py
uv run pyright pydantic_ai_slim/pydantic_ai/ui/_web/app.py tests/test_ui_web.py
git diff --check

Checklist

Any AI generated code has been reviewed line-by-line by the human PR author, who stands by it.
No breaking changes in accordance with the version policy.
PR title is fit for the release changelog.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

…ests Add `test_get_ui_html_cache_write_is_atomic` — interposes on `os.replace` to assert deterministically that the cache file only materializes complete via the atomic rename (the source already holds the full content), so a reader can never catch it existing-but-empty. Add an `isolated_ui_cache` fixture so `test_chat_app_index_endpoint` and `test_chat_app_index_caching` point `_get_cache_dir` at a temp dir and stub the CDN fetch, removing the live jsdelivr dependency and real `~/.cache` pollution from the suite.

The `isolated_ui_cache` fixture monkeypatches `_get_cache_dir`, which left its real body uncovered now that the index-route tests no longer exercise it. Add `test_get_cache_dir_uses_xdg_cache_home` to cover it via the public XDG path.

pydanty · 2026-06-26T19:18:25Z

pydanty runtime failure

Run df_run_5e4061cb8cc24ff8a128 started from the GitHub webhook, but the Dogfooding runtime
failed before pydanty completed the workflow. The pydanty status comment above
(if any) is from a completed run and still stands.

Next step: maintainer attention needed; rerun after fixing the runtime failure.

Failure detail (tail)

imax/minimax-m3,z-ai/glm-5.2:exacto
19:12:41.845         running tool: shell
19:12:41.887         chat fallback:minimax/minimax-m3,z-ai/glm-5.2:exacto
19:13:24.504         running tool: list_repository_files
19:13:24.510         running tool: search_repository_text
19:13:24.541         chat fallback:minimax/minimax-m3,z-ai/glm-5.2:exacto
19:13:27.752         running tool: search_repository_text
19:13:27.774         chat fallback:minimax/minimax-m3,z-ai/glm-5.2:exacto
19:13:35.151         running tool: shell
19:13:35.192     chat fallback:z-ai/glm-5.2:exacto,moonshotai/kimi-k2.7-code
19:13:39.153     running tool: shell
19:13:39.168     chat fallback:z-ai/glm-5.2:exacto,moonshotai/kimi-k2.7-code
19:13:41.939     running tool: shell
19:13:41.963     chat fallback:z-ai/glm-5.2:exacto,moonshotai/kimi-k2.7-code
19:17:36.450     running tool: write_plan
19:17:36.460     chat fallback:z-ai/glm-5.2:exacto,moonshotai/kimi-k2.7-code
19:17:53.419     running tool: write_plan
19:17:53.429     chat fallback:z-ai/glm-5.2:exacto,moonshotai/kimi-k2.7-code
19:18:00.764     running tool: shell
19:18:00.785     chat fallback:z-ai/glm-5.2:exacto,moonshotai/kimi-k2.7-code
19:18:09.349     running tool: write_plan
19:18:09.360     chat fallback:z-ai/glm-5.2:exacto,moonshotai/kimi-k2.7-code
19:18:11.682     running tool: shell
19:18:11.699     chat fallback:z-ai/glm-5.2:exacto,moonshotai/kimi-k2.7-code
19:18:17.398     chat fallback:z-ai/glm-5.2:exacto,moonshotai/kimi-k2.7-code
AGENT_ERROR (relevant leg):
  File "/home/pydanty/dogfooding/.runner.py", line 206, in <module>
    asyncio.run(main())
  File "/home/pydanty/dogfooding/.runner.py", line 182, in main
    result = await result
             ^^^^^^^^^^^^
UnexpectedModelBehavior: Tool 'post_review_outcome' exceeded max retries count of 1

dsfaccini · 2026-06-30T17:17:46Z

David's AICA here: 🔍 Local review suite (automated, pre-merge)

Wave	Status	Blocking	Required (do)	Warnings (defer/skip)
1 · code-correctness	✅ closed	0	0	0
2 · tests/cassettes/docs	✅ closed	0	0	0
3 · cross-cutting/siblings	✅ closed	0	0	0

Verdict: ✅ all 3 waves passed, 0 blocking, 0 required
Reviewed at 2026-07-02T19:17:15Z · diff 794ac1aed048

- _write_cached_file: wrap both the write and os.replace in try/except so the delete=False temp file is unlinked on any failure (incl. write error), not just os.replace failure; flush before the rename so the temp holds complete content at replace time. - Add docstrings to _read_cached_file / _write_cached_file. - Dedupe the MockAsyncClient/MockResponse doubles behind a shared _stub_cdn_fetch helper used by the cache tests.

…callback types

…ache_dir` test

…solation refactor

coderabbitai · 2026-07-02T16:19:09Z

📝 Walkthrough

Walkthrough

This change updates UI HTML caching to use atomic file writes. New helpers treat missing or empty cache files as misses and write via a same-directory temp file plus os.replace, with cleanup on failure. _get_ui_html() now uses those helpers for both the default CDN path and custom URL path. Tests add cache isolation, a shared CDN fetch stub, and coverage for empty-cache refetching, rename failure cleanup, and atomic write behavior.

Related PRs: None specified.

Suggested labels: area: ui, testing

Suggested reviewers: None specified.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: atomic writes for cached web UI HTML.
Description check	✅ Passed	The description covers the summary, testing, issue reference, and checklist; it is mostly complete for the template.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (2)

pydantic_ai_slim/pydantic_ai/ui/_web/app.py (1)
100-108: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicated fetch-and-cache flow between the CDN and custom-URL branches.

Both branches now execute the same _read_cached_file → httpx fetch → raise_for_status → _write_cached_file sequence. Since both paths were touched in this PR, consider extracting a small _fetch_and_cache(url, cache_file) helper to avoid future drift between the two copies.
♻️ Proposed consolidation
+async def _fetch_and_cache(url: str, cache_file: Path) -> bytes:
+    if content := _read_cached_file(cache_file):
+        return content
+
+    async with httpx.AsyncClient() as client:
+        response = await client.get(url)
+        response.raise_for_status()
+        content = response.content
+
+    _write_cached_file(cache_file, content)
+    return content
Then call await _fetch_and_cache(DEFAULT_HTML_URL, cache_file) and await _fetch_and_cache(html_source, cache_file) from the two branches respectively.
Also applies to: 124-132
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pydantic_ai_slim/pydantic_ai/ui/_web/app.py` around lines 100 - 108, Both the
default HTML and custom HTML source branches in app.py duplicate the same
cache-read, HTTP fetch, status check, and cache-write flow. Extract that logic
into a shared helper such as _fetch_and_cache(url, cache_file) and update the
existing branches in the HTML-loading function to call it with DEFAULT_HTML_URL
or html_source so the behavior stays in one place and does not drift.
tests/test_ui_web.py (1)
194-223: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Migration to _stub_cdn_fetch is incomplete.

Good shared-helper extraction, but per the PR discussion, older inline MockResponse/MockAsyncClient setups elsewhere in this file (still present per current graph evidence) haven't been migrated to this helper yet, leaving duplicated mocking logic. Worth finishing the migration (or explicitly deferring) to avoid the two patterns diverging over time.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_ui_web.py` around lines 194 - 223, The migration to the shared CDN
fetch stub is only partially complete, and this file still has duplicated inline
`MockResponse`/`MockAsyncClient` mocking logic alongside `_stub_cdn_fetch`.
Update the remaining tests to use `_stub_cdn_fetch` instead of their local
`httpx.AsyncClient` stubs, or clearly defer those cases, so `_get_ui_html`
cache/CDN branch tests all share the same helper and stay consistent.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pydantic_ai_slim/pydantic_ai/ui/_web/app.py`:
- Around line 64-79: Move the atomic rename in _write_cached_file outside the
NamedTemporaryFile context so the temp file is closed before os.replace runs;
the current flow keeps tmp_file open during the replace, which can fail on
Windows. Keep the existing cleanup behavior by still unlinking tmp_path on any
exception, but ensure the write/flush happens inside the with block and the
replace happens after the file handle is closed.

---

Nitpick comments:
In `@pydantic_ai_slim/pydantic_ai/ui/_web/app.py`:
- Around line 100-108: Both the default HTML and custom HTML source branches in
app.py duplicate the same cache-read, HTTP fetch, status check, and cache-write
flow. Extract that logic into a shared helper such as _fetch_and_cache(url,
cache_file) and update the existing branches in the HTML-loading function to
call it with DEFAULT_HTML_URL or html_source so the behavior stays in one place
and does not drift.

In `@tests/test_ui_web.py`:
- Around line 194-223: The migration to the shared CDN fetch stub is only
partially complete, and this file still has duplicated inline
`MockResponse`/`MockAsyncClient` mocking logic alongside `_stub_cdn_fetch`.
Update the remaining tests to use `_stub_cdn_fetch` instead of their local
`httpx.AsyncClient` stubs, or clearly defer those cases, so `_get_ui_html`
cache/CDN branch tests all share the same helper and stay consistent.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d32b211b-b4db-4e08-a83d-6d41aa01e1cc

📥 Commits

Reviewing files that changed from the base of the PR and between 1a44ec2 and 370ddad.

📒 Files selected for processing (2)

pydantic_ai_slim/pydantic_ai/ui/_web/app.py
tests/test_ui_web.py

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

pydantic/logfire (auto-detected)
pydantic/pydantic (auto-detected)

…gnyan/fix-ui-html-cache-atomic-5399

…cache write On Windows, `os.replace` on a `NamedTemporaryFile` whose handle is still open raises `PermissionError` (WinError 32), so the atomic web UI HTML cache write in `_write_cached_file` failed there. Close the handle before the rename, and add a regression test pinning the close-before-replace ordering.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/test_ui_web.py`:
- Around line 337-370: The `capturing_named_temporary_file` helper uses a `dir`
keyword parameter to mirror the `tempfile.NamedTemporaryFile(dir=...)` call in
`_write_cached_file`, but that shadows the builtin and triggers Ruff A002. Keep
the parameter name unchanged so the monkeypatch still matches the production
call, and suppress the lint on that parameter or function with an explicit `#
noqa` annotation.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: dcaf2f98-ee7a-4488-afb2-657093ca97b0

📥 Commits

Reviewing files that changed from the base of the PR and between 370ddad and b86cbc2.

📒 Files selected for processing (2)

pydantic_ai_slim/pydantic_ai/ui/_web/app.py
tests/test_ui_web.py

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

pydantic/logfire (auto-detected)
pydantic/pydantic (auto-detected)

🚧 Files skipped from review as they are similar to previous changes (1)

pydantic_ai_slim/pydantic_ai/ui/_web/app.py

fix(ui): write cached web UI HTML atomically

4d7d7c9

github-actions Bot added size: S Small PR (≤100 weighted lines) bug Report that something isn't working, or PR implementing a fix labels May 16, 2026

test: cover cache write cleanup

018418a

pragnyanramtha force-pushed the pragnyan/fix-ui-html-cache-atomic-5399 branch from 165e622 to 018418a Compare May 17, 2026 08:44

pragnyanramtha marked this pull request as ready for review May 17, 2026 16:07

Copilot AI review requested due to automatic review settings May 17, 2026 16:07

Copilot started reviewing on behalf of pragnyanramtha May 17, 2026 16:08

View session

Copilot AI reviewed May 17, 2026

pragnyanramtha added 3 commits May 19, 2026 06:41

Merge branch 'main' into pragnyan/fix-ui-html-cache-atomic-5399

a2e4692

Merge branch 'main' into pragnyan/fix-ui-html-cache-atomic-5399

22b147b

github-actions Bot added size: M Medium PR (101-500 weighted lines) and removed size: S Small PR (≤100 weighted lines) labels Jun 26, 2026

dsfaccini added web-ui chore and removed bug Report that something isn't working, or PR implementing a fix labels Jun 26, 2026

dsfaccini mentioned this pull request Jun 26, 2026

Fix two flaky tests in test_online.py and test_dbos.py #6046

Closed

1 of 2 tasks

Merge branch 'main' into pragnyan/fix-ui-html-cache-atomic-5399

c819dfb

dsfaccini added the pydanty:review-branch Trigger pydanty review-branch auto-review on this PR label Jun 26, 2026

pydanty Bot added the pydanty:is-working Pydanty is mid-flight reviewing/remediating this PR; do not edit until it clears label Jun 26, 2026

dsfaccini removed pydanty:review-branch Trigger pydanty review-branch auto-review on this PR pydanty:is-working Pydanty is mid-flight reviewing/remediating this PR; do not edit until it clears labels Jun 30, 2026

Merge branch 'main' into pragnyan/fix-ui-html-cache-atomic-5399

40ac643

pragnyanramtha and others added 3 commits June 30, 2026 13:07

Merge branch 'main' into pragnyan/fix-ui-html-cache-atomic-5399

a98ecd4

test: dedupe CDN mock into _stub_cdn_fetch and narrow os.replace …

4dd2247

…callback types

dsfaccini added the douwebot reviews the PR that gets labeled and removes the label at the end label Jul 1, 2026

github-actions Bot reviewed Jul 1, 2026

View reviewed changes

Comment thread pydantic_ai_slim/pydantic_ai/ui/_web/app.py Resolved

github-actions Bot reviewed Jul 1, 2026

View reviewed changes

Comment thread tests/test_ui_web.py Resolved

github-actions Bot removed the douwebot reviews the PR that gets labeled and removes the label at the end label Jul 1, 2026

dsfaccini added 2 commits July 1, 2026 18:52

refactor: EAFP read in _read_cached_file; drop out-of-scope `_get_c…

ad06313

…ache_dir` test

test: restore _get_cache_dir coverage test dropped by index-route i…

370ddad

…solation refactor

coderabbitai Bot suggested changes Jul 2, 2026

View reviewed changes

Comment thread pydantic_ai_slim/pydantic_ai/ui/_web/app.py Resolved

dsfaccini added 2 commits July 2, 2026 12:34

Merge remote-tracking branch 'upstream/main' into pragnyanramtha--pra…

b5ee833

…gnyan/fix-ui-html-cache-atomic-5399

coderabbitai Bot suggested changes Jul 2, 2026

View reviewed changes

Comment thread tests/test_ui_web.py Resolved

adtyavrdhn mentioned this pull request Jul 2, 2026

Cache Temporal dev-server binaries in CI #6230

Merged

3 tasks done

coderabbitai Bot approved these changes Jul 2, 2026

View reviewed changes

dsfaccini merged commit 80017c7 into pydantic:main Jul 3, 2026
94 checks passed

Navigation Menu

Repository navigation

fix(ui): write cached web UI HTML atomically - #5490

fix(ui): write cached web UI HTML atomically#5490
dsfaccini merged 15 commits into
pydantic:mainfrom
pragnyanramtha:pragnyan/fix-ui-html-cache-atomic-5399

pragnyanramtha commented May 16, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

pydanty Bot commented Jun 26, 2026

Uh oh!

dsfaccini commented Jun 30, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 2, 2026 •

edited

Loading

Walkthrough

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

pragnyanramtha commented May 16, 2026

Summary

Testing

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

pydanty Bot commented Jun 26, 2026

pydanty runtime failure

Uh oh!

dsfaccini commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dsfaccini commented Jun 30, 2026 •

edited

Loading

coderabbitai Bot commented Jul 2, 2026 •

edited

Loading