fix: prevent onboarding modal freeze on Textual 8.x#290
Conversation
git-miku
Bot
added
the
needs-triage
git-miku
Bot
added
size/M
|
Tip 👋 Hey @deepmhatre13 — Miku's on it. Here are the most useful commands for this PR:
Note 🎓 This repo is part of ECSOC26. Run Note ⭐ Star this repo to unlock all commands. Say 📖 All commands🤖 AI-powered — 14 commands
🔧 Issue & PR management — 18 commands
🎉 Community & utility — 9 commands
|
|
| Filename | Overview |
|---|---|
| termstory/insights.py | Duplicate sections 4 and 5 of the commit-fetch logic were inserted without removing the originals; the first pass deletes project_id from every session dict, causing the second pass to crash with KeyError on every real invocation. |
| termstory/tui.py | Introduces _DeferredDismissMixin and applies it to all five modal screens; cleanly replaces set_timer(0.0, ...) calls with dismiss_later(). One set_timer(0.8, self.dismiss) auto-dismiss in MatrixDefragScreen was intentionally left unchanged. |
| tests/test_tui.py | Replaces the structural onboarding test with a proper end-to-end regression test; correctly mocks save_config to prevent writing the developer's real config file. |
| tests/test_insights.py | Deletes test_detect_late_night_chaotic_sessions_no_duplicate_commits without a replacement, removing the only test that guards the commit deduplication logic that is broken in this PR. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant ModalScreen
participant _DeferredDismissMixin
participant TextualEventLoop
User->>ModalScreen: "clicks Save & Enable / presses key"
ModalScreen->>ModalScreen: "on_button_pressed() / action_*()"
ModalScreen->>_DeferredDismissMixin: dismiss_later(result)
_DeferredDismissMixin->>TextualEventLoop: set_timer(0.001, _do_dismiss)
Note over TextualEventLoop: next event-loop tick (clean state)
TextualEventLoop->>ModalScreen: "_do_dismiss() -> self.dismiss(result)"
ModalScreen-->>User: modal closes, result returned to app
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User
participant ModalScreen
participant _DeferredDismissMixin
participant TextualEventLoop
User->>ModalScreen: "clicks Save & Enable / presses key"
ModalScreen->>ModalScreen: "on_button_pressed() / action_*()"
ModalScreen->>_DeferredDismissMixin: dismiss_later(result)
_DeferredDismissMixin->>TextualEventLoop: set_timer(0.001, _do_dismiss)
Note over TextualEventLoop: next event-loop tick (clean state)
TextualEventLoop->>ModalScreen: "_do_dismiss() -> self.dismiss(result)"
ModalScreen-->>User: modal closes, result returned to app
Reviews (4): Last reviewed commit: "Merge branch 'main' into issue-288-textu..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
:miku |
|
@deepmhatre13 Can you prove the fix works? Can you record a video and send it here? I'll definitely give you a good backend label so you get the maximum points in ESOC. Also, if you find any other issues, can you just file them too? If you want, you can claim them. I'll definitely give you good labels so you can score better in ECSOC. |
|
I don’t have much time to search for and file issues here. I have a reliable bot that can file them, but it’s often just the usual situation. I was wondering if you could file some issues and perhaps claim them, or at least allow me to add the ECSOC labels to them. Also, the good labels can earn extra points, definitely. |
|
Also, could you find out whether the labels are stackable? If they are, I can give you multiple labels to earn a lot of points. I would appreciate it if you could look into this. Thank you. |
|
Hi @bitflicker64, I've verified the fix locally and in CI. Local verification
This includes the new end-to-end onboarding regression test that exercises the actual Save & Enable flow. CI verification
I've attached screenshots of the local test results and the successful CI checks as proof. |
|
Hi @bitflicker64! I checked the publicly available ECSoC documentation, but I couldn't find any official mention of whether labels are stackable for scoring. It doesn't seem to be covered in the program rules. If there's an internal guideline or admin clarification about label stacking, I'd be happy to follow that. Otherwise, I couldn't verify that multiple labels contribute additional points. |
Screen.Recording.2026-07-17.at.7.58.57.PM.mov |
|
@deepmhatre13 I'm currently working on containerization in one of Apache projects , so I'm a bit busy, but I'm still finding time to test it. I'll quickly make a video. You can test it in WSL since you have a Windows laptop, not a Mac or Linux. This was originally intended for Mac, but it should work on Linux as well. You can try it with WSL. You can contact me here, or we can connect on Discord. I'll send you a link right now; let me know if it works. Please confirm here, and I'll re‑run it. https://discord.gg/PHcXrachS |
|
:miku /label good-backend |
|
miku 🏷️ Added label miku 🏷️ Added label |
|
Until then, you can take on small PRs so you continue earning points, too. Thanks. |
|
Thanks for fixing the Textual timer crash. I think there is one more practical issue left: the onboarding modal is still tall enough that Save & Enable can sit below the visible area in normal terminals such as The fix I would apply is to shorten the onboarding copy and reduce the modal vertical spacing: diff --git a/termstory/tui.py b/termstory/tui.py
@@
Static("🔒 TermStory Privacy & AI Onboarding", id="modal-title"),
Static(
- "TermStory is completely offline by default. No data ever leaves your machine.\n\n"
- "To make your timeline readable, you can optionally enable the AI Categorization Engine. "
- "This will send sanitized terminal commands to an LLM to generate short, human-readable session memories.\n\n"
+ "TermStory is offline by default. If you enable AI, only sanitized terminal commands are sent "
+ "to your selected provider to create short session memories.\n\n"
"Before sending, TermStory scrubs passwords, environment variables, IPs/FQDNs, and drops "
- "sensitive sessions containing commands like 'vault' or 'aws configure' entirely.\n\n"
- "[dim]Shortcuts: [bold]Ctrl+g[/]: Groq, [bold]Ctrl+a[/]: OpenAI, [bold]Ctrl+l[/]: Ollama, [bold]Ctrl+c[/]: Custom, [bold]Ctrl+d[/]: Disable AI, [bold]Esc[/]: Cancel[/dim]",
+ "sensitive sessions such as vault or aws configure.\n\n"
+ "[dim]Shortcuts: Ctrl+g Groq, Ctrl+a OpenAI, Ctrl+l Ollama, Ctrl+c Custom, Ctrl+d Disable, Esc Cancel[/dim]",
id="modal-desc"
),
@@
width: 90%;
max-width: 75;
height: auto;
- padding: 2;
+ padding: 1;
content-align: center middle;
@@
.input-label {
color: #88889a;
- margin-top: 1;
+ margin-top: 0;
margin-left: 1;
text-style: bold;
}I checked this locally on the PR branch: pytest tests/test_tui.py -k "api_key_validation" -q
# 1 passed, 30 deselectedI also probed the Textual layout at |
bitflicker64
left a comment
bitflicker64
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approve with minor suggestions — real fix for Textual 8.x modal freeze; good E2E regression test. A few cleanups would make this more maintainable.
Intent
Stop onboarding / modal dismiss from freezing under Textual 8.x by scheduling dismiss off the button message pump with a non-zero timer delay (issue #288).
Impact
- Replaces
set_timer(0.0, ...)withdismiss_laterusing0.001s delay and optional result. - Applied across Help, Onboarding, ResetConfirm, MatrixDefrag, GhostTyper screens.
- Test rewritten to press real Save & Enable path and wait for screen pop — much better than the previous structural mock.
What’s correct
- Root cause analysis (AwaitComplete / ScreenError on zero-delay dismiss inside message handler) matches known Textual 8 behavior.
- CI green.
- Non-empty API key path exercised without network.
Suggestions
- DRY
dismiss_later— identical 3-line method is copy-pasted on 5 classes. Prefer a small mixin / helper base, e.g.class DeferredDismissMixin: ..., to avoid future drift. - Lambda style —
(self.dismiss(result), None)[1]works but is opaque;lambda r=result: self.dismiss(r)(or a tiny local def) is clearer and avoids the tuple trick. - Config isolation in the new test — confirm
save_configcannot write the developer’s real config file when Save is pressed (monkeypatchtermstory.tui.save_config/ config path if not already covered by workspace config override). - Comment why
0.001not0.0— one line indismiss_laterdocstring helps the next person not “optimize” it back to zero.
Verdict
Comment / soft approve — mergeable after optional DRY + config-safety check; not blocking if you confirm the test doesn’t touch real HOME config.
|
|
||
| def dismiss_later(self, result=None) -> None: | ||
| """Dismiss this modal screen on the next tick with a non-zero delay.""" | ||
| self.set_timer(0.001, lambda: (self.dismiss(result), None)[1]) |
There was a problem hiding this comment.
Works, but the tuple trick is hard to read and this method is duplicated on several screens. Consider:
def dismiss_later(self, result=None) -> None:
"""Dismiss on the next timer tick (non-zero delay required on Textual 8.x)."""
def _do_dismiss() -> None:
self.dismiss(result)
self.set_timer(0.001, _do_dismiss)and/or a shared mixin so Help/Onboarding/Reset/Matrix/GhostTyper don’t each redefine it.
| await pilot.pause() | ||
|
|
||
| # Verify the modal dismissed and the result reached the app. | ||
| assert not isinstance(app.screen, OnboardingScreen) |
There was a problem hiding this comment.
Nice end-to-end path. Please double-check (or monkeypatch) that Save does not persist to the real user config under $HOME during CI/dev runs — e.g. monkeypatch.setattr("termstory.tui.save_config", lambda cfg: None) if the onboarding success path writes config.
|
/triage |
git-miku
Bot
added
triage/triaged
|
miku ✅ Marked as triaged ( |
|
:miku /check-star |
git-miku
Bot
added
the
do-not-merge/needs-star
|
miku @deepmhatre13 still hasn't starred this repo. Applied Star this repo first, then run |
|
:miku /check-star |
git-miku
Bot
added
needs-triage
|
Tip 👋 Hey @deepmhatre13 — Miku's on it. Here are the most useful commands for this PR:
Note 🎓 This repo is part of ECSOC26. Run Note ⭐ Star this repo to unlock all commands. Say 📖 All commands🤖 AI-powered — 14 commands
🔧 Issue & PR management — 18 commands
🎉 Community & utility — 9 commands
|
|
miku ✅ @deepmhatre13 has starred the repo — removed |
…to match hermes_obs.py (fixes bitflicker64#258)
Optimize detect_late_night_chaotic_sessions to bulk-fetch commands and commits while preserving per-session filtering semantics.
git-miku
Bot
added
size/M
|
/ecsoc |
|
miku 🏷️ Added |
4 participants
Closes #288
Summary
This PR fixes the onboarding modal freeze that occurs on Textual 8.x when clicking Save & Enable.
The root cause was the use of
set_timer(0.0, ...)for modal dismissal, which can trigger aZeroDivisionErrorin newer Textual versions and prevent the onboarding flow from completing.Changes
dismiss_later()helper for modal screens.set_timer(0.0, ...)modal dismissal calls withdismiss_later().HelpScreenOnboardingScreenResetConfirmScreenMatrixDefragScreenGhostTyperScreen0.001s) to avoid the Textual timer issue.None, preventingAwaitComplete/ScreenErrorissues during modal dismissal.Regression Test
Added an end-to-end regression test for the onboarding flow.
The test:
gsk_test_key),The test uses
widget.press()becausepilot.click("#btn-save")results in anOutOfBoundserror in the test environment when the button is outside the visible viewport.widget.press()dispatches the sameButton.Pressedevent and exercises the production code path:Validation
Passed:
Result:
Passed:
pytest tests/test_tui.py -k "onboarding or api_key_validation" -vResult:
Verified that:
ZeroDivisionErroroccurs,ScreenErroris raised,Notes
The remaining failures in the full
tests/test_tui.pysuite are pre-existingNoMatchesfailures related toDetailsCanvasrendering (#btn-exec-*,#btn-bulk-*,.feed-container) and are unrelated to this onboarding modal fix.