fix(tui): avoid NoMatches exception when SelectionList is missing on key events.

[hrshtmk]

Problem :

If a keystroke arrives during SelectionList transition or screen dismissal, an unhandled NoMatches exception is thrown, causing archinstall to crash:

NoMatches: No nodes match 'SelectionList' on SelectListScreen()

Replication :

1. Open the Additional packages menu.
2. Type as quick as you can and smash/press backspace when the menu is transitioning.

Reasons for the problem I figured :

1. Key events can throw NoMatches exception if SelectionList is not visible in DOM. (First assumption)
2. While fixing the list UI issues in Additional packages I figured that the main reason for the crash could have been the _update_options(...) being called over every single keystroke, which did the entire search cycle of filtering --> sorting --> widget on every key press by calling _group.get_focused_index(...).
   Which is expensive on larger lists and due to keystrokes overlapping the cycle queries the system crashed.

note : This issue was already marked in the menu_item.py but I think was never guarded against.

	def get_focused_index(self) -> int | None:
		items = self.get_enabled_items()

		if self.focus_item and items:
			try:
				return items.index(self.focus_item)
			except ValueError:
				# on large menus (15k+) when filtering very quickly
				# the index search is too slow while the items are reduced
				# by the filter and it will blow up as it cannot find the
				# focus item
				pass

		return None

Fix :

1. Added 2 helper functions to guard query_one calls on key events.

	def _selection_list(self) -> SelectionList | None: ...
	def _preview_widget(self) -> Label | None: ...

2. Made so that only a batch of keystrokes can call the _update_options by adding a slight delay (60ms, was found to be best in testing) on the event in on_input_changed. The delay is minuscule and doesn't really affect the overall UI experience.

[svartkanin]

it would be better to actual raise an issue and then link a PR with the fix to the issue. Folks who come search for similar problems will most likely search in the issues rather than past PRs.

[svartkanin]

Based on the description I assume this is LLM generated.
I cannot reproduce this so as long as you can't provide steps to reproduce it I can't validate the fix works.

[hrshtmk]

Hi @svartkanin! Sorry for the LLM jump scare 😄
This is my first PR so I was trying to be very formal with the report.
I am linking the asciinema terminal recording below showing the exact replication steps.

Now on replication, the best way is to :

1. Open the Additional packages menu.
2. Type as quick as you can and smash/press backspace when the menu is transitioning.
3. Additionally, if you type a string that is gibberish (0 matches) and then crash the menu you should be able to replicate the bug on line 574.

Once you get the hang of it, it's easy to replicate.

2 key things I have noticed :

1. The error doesn't log in the install.log of debug mode, for some reason. (Or I just wasn't able to catch it)
2. The new bug that I encountered in this recording, on line 574, it tells us that this is probably an architectural oversight and it's basically a family of DOM bugs.

The reason for the bugs I think is due to the event handler always assuming that the SelectionList is present in the DOM whenever an event fires, so it just throws the NoMatches when the menu is transitioning or if there is a fast input.

To fix that my line 577 changes were basically checking if the list is present or not before performing events.
But now that we have encountered another bug on the similar front, I think the changes should be made in SelectionList or directly how the event handler works. I'd like to work on that too if you want!

[svartkanin]

Thanks for the explanation I was able to replicate it and confirm the fix

[svartkanin]

The mypy check is failing

[hrshtmk]

I've kept on digging on this issue, trying to fix those "Issues that can be worked on next:" I mentioned in the earlier comment. My intuition was that the zero match freeze was somehow related to the NoMatches handling, and I was somewhat correct. This is not strictly a DOM related bug, but SelectListScreen._update_options() runs complete sorting and rendering of the filtered list on every single keystroke. With smaller lists this has no issues, but when the list is big, stacking these operations on top of each other on every keystroke was leaving the SelectionList unresponsive rather than crashing outright.

To fix this issue I added a slight delay between each of the operations and also refactored the code to fire the operations only on batches of keystrokes. The delay is minimal, at 60ms (best performance I found while testing), and can be tuned later if needed.
Also, I have kept the guards over every query_one call but created helper functions to reduce footprint.
I think the issue is fixed in the scope of the bug, happy to work further if you spot something I missed.

[h8d13]

Hey was just checking this out.

I'm wondering if it's not overdoing it. I.e adding debounce, and empty-clear when really the issue is just the guard.

Can you try something like:

diff --git a/archinstall/tui/components.py b/archinstall/tui/components.py
index 75cb82be..a7076331 100644
--- a/archinstall/tui/components.py
+++ b/archinstall/tui/components.py
@@ -332,6 +332,10 @@ class OptionListScreen(BaseScreen[ValueT]):
                self.query_one(OptionList).focus()
 
        def on_input_changed(self, event: Input.Changed) -> None:
+               # late Changed from fast typing can land after dismiss
+               if not self.is_current:
+                       return
+
                search_term = event.value.lower()
                self._group.set_filter_pattern(search_term)
                filtered_options = self._get_options()
@@ -581,6 +585,10 @@ class SelectListScreen(BaseScreen[ValueT]):
                _ = self.dismiss(Result(ResultType.Selection, _item=self._selected_items))
 
        def on_input_changed(self, event: Input.Changed) -> None:
+               # late Changed from fast typing can land after dismiss
+               if not self.is_current:
+                       return
+
                search_term = event.value.lower()
                self._group.set_filter_pattern(search_term)
                filtered_options = self._get_selections()

This uses only built-in self.is_current and I believe achieves a similar repro fix ? Would be great if you can test this out @hrshtmk for a more "minimal" patch. Or split it into different PRs.

[hrshtmk]

Hi @h8d13, appreciate the effort!
Through testing, it doesn't seem to solve the issue, unfortunately.
I ran 2 tests:

1. self.is_current only :
   
   Crashes outright, expected because the guards aren't in place.

2. self.is_current + my NoMatches guards :
   
   This too was expected because I had already figured that the menu freezes on fast input when the guards are added.

I still think we do need the debouncing to fix this, but I am open to suggestions.

[h8d13]

I'm guessing that you might want to try how much you can reduce the patch size for it to be "fixed". What I mean is that from the 75+ -11 LoC, how much is critical for your fix and how much is "nice-to-have". If that makes sense ?

What I learned from contributing here is that the smaller the patches, the faster you'll get them reviewed and merged.

Nice trick: You can add to the current url here .patch or .diff to see full current extent.

It might need all of it, I'm just saying maybe also parts aren't needed?

[hrshtmk]

Hi @svartkanin. Could you take another look at the latest commits, and approve the CI checks to run on them?
Really appreciate it!

[svartkanin]

As pointed out by @h8d13 this patch size has now significantly increase from it's original submission. I'll have to check the full impact and all cases. I'm very much not a fan of introducing any timers with randomly chosen timeouts to fix this. This is either a logic problem in archinstall or a bug in textual, the prior one can be addressed properly in the second instance it should be reported upstream

[h8d13]

Perhaps this can be re-submitted with just 123bfe3 and 61840e4

That was approved and already fixing the issue or only partially ?
Seems this is mostly decoration: 2c3f618

[hrshtmk]

Hi @svartkanin.
I dug deeper into the issue with a custom logger to track the exact exception chain, and I've identified the root cause. The bug is in how textual handles the _on_option_list_option_highlighted, the function throws an OptionDoesNotExist when the highlighting event is called on an index that no longer exists (cleared by the filter), then that exception down the line created a NoMatches exception.

What my current fix does:

1. Override the _on_option_list_option_highlighted's behavior in _SelectionList with a bound check to prevent stale indices from being accessed.
2. As a "nice to have" addition, pressing backspace once on no matches, clears the filter input.
   (Let me know if this should be omitted.)

I’m opening a PR upstream in Textual to fix the handler directly, but having this patch in archinstall keeps our menus safe in the meantime. Let me know if everything's fine!

Edit : PRs are restricted to collaborators only on texual, so idk what to do.