Repository navigation
fix: stop console initialization after unmount - #320
Conversation
|
|
|
|
No actionable comments were generated in the recent review.
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Out of Scope Changes check |  |
The cancellation and cleanup changes stay within #319. The new toast.error("Unable to initialize console", ...) behavior and its Reload action add independent failure-reporting UI. Issue #319 does… |
Remove the initialization-failure toast and Reload action from this pull request, or link the behavior to an active issue that requires it. |
| Docstring Coverage | |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely describes stopping console initialization after component unmount. Description check The description directly explains cancellation, cleanup, error handling, verification, and the linked issue for the console initialization changes. Linked Issues check Issue #319requires cancellation after unmount, no post-unmount console setup or callback assignment, and callback detachment.HeadlessConsole.svelteaborts on destroy, checks the signal after `getP…Full details: Linked Issues check
-
Explanation
Issue
#319requires cancellation after unmount, no post-unmount console setup or callback assignment, and callback detachment.HeadlessConsole.svelteaborts on destroy, checks the signal aftergetPy()andinitConsole(signal), and assigns globals and callbacks only after those checks.onDestroyclears existingstdout_callbackandstderr_callback, andpushLogignores late updates.init.tsalso checks the signal before running the console initialization code.
-
Full details: Out of Scope Changes check
-
Explanation
The cancellation and cleanup changes stay within
#319. The newtoast.error("Unable to initialize console", ...)behavior and itsReloadaction add independent failure-reporting UI. Issue#319does not request this behavior, and the linked issue provides no requirement that supports it.Resolution
Remove the initialization-failure toast and
Reloadaction from this pull request, or link the behavior to an active issue that requires it.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/lib/components/console/HeadlessConsole.svelte" line_range="43" />
<code_context>
+
onMount(async () => {
const py = await getPy();
+ if (disposed)
+ return;
await initConsole();
</code_context>
<issue_to_address>
**Abandoned mounts overwrite console state**
When the component is destroyed while `initConsole()` is pending, `initConsole()` continues into `py.runPythonAsync(...)` and redefines the shared console and helper globals after navigation, so a later mount can observe state overwritten by the abandoned mount.
Prevent `initConsole()` from running Python initialization or changing shared globals after the component is disposed.
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: src/lib/components/console/HeadlessConsole.svelte:43
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Skip console setup when navigation finishes before Pyodide or the console module loads. Check cancellation before changing Python globals, detach output callbacks on unmount and ignore late updates. Report console initialization failures with a Reload action.
Verified deferred initialization and cleanup with the real Svelte component in Chromium. Production-preview failure injection also confirms the error appears and Reload restores a working console. Combined build, lint and type checks pass (24 existing warnings).
Closes #319.