Skip to content

perf(tui): defer startup skills refresh#18370

Merged
fcoury-oai merged 1 commit intomainfrom
fcoury/startup-performance
Apr 17, 2026
Merged

perf(tui): defer startup skills refresh#18370
fcoury-oai merged 1 commit intomainfrom
fcoury/startup-performance

Conversation

@fcoury-oai
Copy link
Copy Markdown
Contributor

Summary

This removes startup skills/list from the critical path to first input. In release measurements, median startup-to-input time improved from 307.5 ms to 191.0 ms across 30 measured runs with 5 warmups.

Background

Startup currently waits for a forced skills/list app-server request before scheduling the first usable TUI frame. That makes skill metadata freshness part of the process-launch-to-input path, even though the prompt can safely accept normal input before skill metadata has finished loading.

I measured startup from process launch until the TUI reports that the user can type. The measurement harness watched the startup measurement record, killed Codex after a successful sample, and enforced a timeout so repeated runs would not leave TUI processes behind. The debug runs had enough outliers that I used median as the primary signal and ran a baseline self-compare to understand the noise floor.

Why skills/list

The skills/list cut was the best practical optimization because it improved startup without changing the important readiness contract: when the prompt is shown, it is still backed by an active session. Only enrichment data arrives later.

Candidate Result Decision
Defer startup skills/list Debug median improved from 524.0 ms to 348.0 ms; release median improved from 307.5 ms to 191.0 ms. Keep
Defer fresh thread/start Debug median improved from 494.0 ms to 256.0 ms, but the prompt could appear before an active thread was attached. Reject as too risky for this PR
Avoid forced skills config reload Debug median moved from 509.0 ms to 512.0 ms. Reject as neutral
Skip fresh history metadata Debug median moved from 496.5 ms to 531.5 ms. Reject as regression/noise
Defer app-server startup Not implemented because it would only permit a loading frame unless the TUI gained a deliberate pre-server state. Out of scope

Implementation

App::refresh_startup_skills now clones the app-server request handle, spawns a background task, and issues the same forced skills/list request after the first frame is scheduled. When the request completes, the task sends AppEvent::SkillsListLoaded back through the normal app event queue.

The existing skills response handling still converts the app-server response, updates the chat widget, and emits invalid SKILL.md warnings. Explicit user-initiated skills refreshes still use the existing synchronous app command path, so callers that intentionally requested fresh skill state do not race ahead of their own refresh.

Tradeoffs

The main tradeoff is a narrow theoretical race at startup: skill mention completion depends on a background skills/list response, so it could briefly show stale or empty metadata if opened before that response arrives. In manual testing, pressing $ as soon as possible after launch still showed populated skill metadata, so this risk appears minimal in normal use. Plain input remains available immediately, and the UI updates through the existing skills response path once the refresh completes.

This PR does not change how skills are discovered, cached, force-reloaded, displayed, enabled, or warned about. It only changes when the startup refresh is allowed to complete relative to the first usable TUI frame.

Verification

  • cargo test -p codex-tui

Load startup skill metadata through the background app-server
request handle so initial TUI rendering is not blocked by
`skills/list`.

The UI still applies the same result once the request completes.
@fcoury-oai
Copy link
Copy Markdown
Contributor Author

@codex review

@fcoury-oai fcoury-oai marked this pull request as ready for review April 17, 2026 18:44
Copy link
Copy Markdown
Collaborator

@etraut-openai etraut-openai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I built and manually tested.

@fcoury-oai fcoury-oai merged commit 48f117d into main Apr 17, 2026
25 checks passed
@fcoury-oai fcoury-oai deleted the fcoury/startup-performance branch April 17, 2026 19:55
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 17, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants