Render asynchronous operations with Rich Live and spinners#97
Conversation
- Publish validated, redacted job progress events. - Render prompt-safe spinners and determinate bars. - Document and test the interactive progress lifecycle. Fixes #52
7b0c402 to
1f915e7
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b0c402e5e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self._live.update(self._render(), refresh=True) | ||
| elif self._live is not None: | ||
| self._stop_live() | ||
| self.console.print(self._summary(snapshot)) |
There was a problem hiding this comment.
Route job progress away from JSON stdout
This final status is written to the same console that ReplApplication uses for normal stdout, so any background job that completes while a user is producing --json output can interleave a plain-text j000001 completed... line into the machine-readable stream. This also affects a background command submitted with --json, where the job response/result shares stdout with the progress summary; progress/status should go to stderr or be suppressed while JSON output is active.
Useful? React with 👍 / 👎.
| normalized_keys, | ||
| ) | ||
| except BaseException: | ||
| self._notify(snapshot) |
There was a problem hiding this comment.
Notify listeners after storing the future
The queued snapshot is published before _executor.submit() returns and before record.future is assigned, so a subscriber that reacts to the new queued event by calling wait(job_id) sees future is None and returns the still-queued snapshot immediately; cancel_queued() has the same race and reports JOB_ALREADY_RUNNING. This only affects consumers of the new listener API, but it makes queued notifications unsafe to act on; publish the queued event after the future has been stored.
Useful? React with 👍 / 👎.
Summary
jobs list/jobs show, addhelp jobs, and document the authoring and console contractsValidation
make test— 416 passed, 8 skippedmake lintmake typecheckmake security— dependency audit clean; Semgrep ran 1,102 rules with 0 findingsgit diff --checkFixes #52