fix(tui): defer _reload_projects to survive pytest-xdist race#121
fix(tui): defer _reload_projects to survive pytest-xdist race#121aorumbayev merged 1 commit intomainfrom
Conversation
…st race
on_mount called _reload_projects synchronously, which immediately ran
query_one('#project-list') before compose() children were fully in the
DOM under pytest-xdist parallel workers. Defer via call_after_refresh so
the whole mount sequence runs after the first layout pass.
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Greptile SummaryWraps the Confidence Score: 5/5Safe to merge — the fix is correct and the only finding is a minor style point. The change correctly uses Textual's No files require special attention.
|
| Filename | Overview |
|---|---|
| src/kagan/tui/screens/welcome.py | Defers _reload_projects and DOM queries from on_mount to call_after_refresh, fixing the pytest-xdist race. on_screen_resume still queries the DOM directly but is a known follow-up. |
Sequence Diagram
sequenceDiagram
participant Textual
participant WelcomeScreen
participant DOM
Note over Textual,DOM: Before fix
Textual->>WelcomeScreen: on_mount()
WelcomeScreen->>DOM: query_one('#project-list') ❌ DOM not ready
Note over Textual,DOM: After fix
Textual->>WelcomeScreen: on_mount()
WelcomeScreen->>Textual: call_after_refresh(_on_mount_deferred)
Textual->>DOM: first refresh / layout pass
Textual->>WelcomeScreen: _on_mount_deferred()
WelcomeScreen->>DOM: query_one('#project-list') ✅ DOM stable
Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/kagan/tui/screens/welcome.py
Line: 99
Comment:
**Unnecessary `async` on `on_mount`**
`on_mount` no longer awaits anything — it only calls the synchronous `call_after_refresh`. Declaring it `async def` carries a tiny overhead and can be misleading; a plain `def` is cleaner and equally correct here.
```suggestion
def on_mount(self) -> None:
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(tui): defer _reload_projects to call..." | Re-trigger Greptile
| @@ -97,6 +97,9 @@ def compose(self) -> ComposeResult: | |||
| yield KeybindingHint(id="welcome-hint") | |||
|
|
|||
| async def on_mount(self) -> None: | |||
There was a problem hiding this comment.
Unnecessary
async on on_mount
on_mount no longer awaits anything — it only calls the synchronous call_after_refresh. Declaring it async def carries a tiny overhead and can be misleading; a plain def is cleaner and equally correct here.
| async def on_mount(self) -> None: | |
| def on_mount(self) -> None: |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/kagan/tui/screens/welcome.py
Line: 99
Comment:
**Unnecessary `async` on `on_mount`**
`on_mount` no longer awaits anything — it only calls the synchronous `call_after_refresh`. Declaring it `async def` carries a tiny overhead and can be misleading; a plain `def` is cleaner and equally correct here.
```suggestion
def on_mount(self) -> None:
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Since _reload_projects was deferred into _on_mount_deferred, on_mount no longer awaits anything — just calls synchronous call_after_refresh. Dropping async makes the signature clearer. Addresses Greptile comment on PR #121.
Summary
query_onerace inWelcomeScreen.on_mountthat manifests under pytest-xdist parallel workers.test_doctor_modal_skip_button_dismisses_modalfailed withNo nodes match '#project-list' on WelcomeScreen(id='welcome-screen')—on_mountfires before the composed children are queryable.on_mountbody incall_after_refresh(self._on_mount_deferred)so reload runs after the DOM is stable. Textual's idiomatic defer pattern.Test plan
pytest -n auto— all pass.tests/tui/ -m "not snapshot" -n auto— 129/129 pass.tests/tui/test_doctor_modal.py— 16/16 pass (no regression in non-parallel mode).Known follow-ups (not in this PR)
WelcomeScreen.on_screen_resumehas the same pattern — lower risk but worth deferring for consistency._update_cwd_banner_hintand_cwd_banner_visibleboth callquery_onewithout guard — could race during early resumes.session_resume_modal.py,kanban.pyetc) — systematic audit needed in a follow-up.🤖 Generated with Claude Code