langchain-ai/deepagents
1
0
Fork 0
mirror of https://github.com/langchain-ai/deepagents.git synced 2026-08-27 10:51:26 -04:00
Code Issues 169 Packages Projects Releases 16 Wiki Activity

fix(code): remove notification settings expand flicker and trailing disclosure glyph (#5734)

Browse Source 
Pressing Enter on "Notification settings" no longer opens the pane with
a beat of unselected content and a late-arriving footer hint, and the
trailing triangle no longer flips between glyphs as the section opens
and closes — the leading cursor now carries the disclosure state.

---

Two papercuts from #5698:

**Lag before the footer hints update.** The app constructs the center
without the suppressed-keys snapshot, so the first expand read
`config.toml` (four `asyncio.to_thread` calls) before mounting any
checkboxes, focusing the first one, or refreshing the footer. The pane
was visibly open but nothing was selected, and only then did the hint
switch to "Space/Enter toggle" — a one-frame flicker. The screen now
preloads the suppressed keys in a background worker when it mounts, and
`_expand_settings` flips the expanded state, disclosure glyph, and
footer hint synchronously before its first await, so the expanded hints
render in the same frame the pane opens. A rapid Esc during an in-flight
read both collapses the section and stops the expand worker from
reopening it.

**Trailing disclosure triangle.** The `▸`/`▾` after "Notification
settings" is removed. The leading glyph carries the state instead: `›`
when the row is selected and collapsed (Enter opens), `▾` whenever the
section is expanded (Esc collapses).
This commit is contained in:
Mason Daugherty
2026-08-21 16:52:22 -04:00
committed by GitHub
parent 85c6833734
commit 5397812ad5
2 changed files with 299 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files
libs/code/deepagents_code/tui/widgets/notification_center.py
+144 -55
View File
@@ -20,6 +20,7 @@ inline under "Notification settings" so toggles never leave the hub.
from __future__ import annotations
import asyncio
import contextlib
import logging
from dataclasses import dataclass
from importlib import import_module
@@ -107,6 +108,14 @@ class NotificationSettingsRequested(Message):
"""
_PRELOAD_TIMEOUT_SECONDS = 3.0
"""How long an expand waits on the mount-time preferences read.
Generous for a local config read, short enough that a stalled filesystem
surfaces as a toast instead of an unresponsive settings row.
"""
IN_PLACE_ACTIONS: frozenset[ActionId] = frozenset({ActionId.ENTER_API_KEY})
"""Actions handled in place without dismissing the center.
@@ -225,7 +234,7 @@ class _NotificationSettingsRow(Static):
return self._index
def set_expanded(self, expanded: bool) -> None:
"""Point the disclosure arrow at the section's new state.
"""Redraw the leading glyph for the section's new state.
Args:
expanded: Whether the settings section is now expanded.
@@ -248,17 +257,21 @@ class _NotificationSettingsRow(Static):
self.update(self._render())
def _render(self) -> Content:
# One leading glyph carries both the cursor and the disclosure
# affordance, so a toggle changes a single position instead of two.
# While expanded the disclosure glyph deliberately takes the
# cursor's place: focus has moved to the checkboxes, so a row
# cursor there would point at something that is not selected.
glyphs = get_glyphs()
cursor = glyphs.cursor if self._is_selected else " "
disclosure = (
glyphs.disclosure_expanded
if self._expanded
else glyphs.disclosure_collapsed
)
if self._expanded:
cursor = glyphs.disclosure_expanded
elif self._is_selected:
cursor = glyphs.cursor
else:
cursor = " "
return Content.assemble(
f"{cursor} ",
("Notification settings", "bold"),
f" {disclosure}",
)
def on_click(self, event: Click) -> None:
@@ -325,7 +338,7 @@ class NotificationCenterScreen(ModalScreen[NotificationActionResult | None]):
Each `PendingNotification` is a single row followed by a settings
disclosure row. Up/Down (or j/k) moves the cursor; Enter or click drills
into a notification or toggles the inline settings section. Expanded
settings hand key focus to their checkboxes (Space/Enter toggle); Esc
settings hand key focus to their first checkbox (Space/Enter toggle); Esc
there collapses back to the row cursor. Esc on the row cursor returns
`None`.
"""
@@ -433,7 +446,8 @@ class NotificationCenterScreen(ModalScreen[NotificationActionResult | None]):
suppressed: Currently suppressed warning keys, used to render
checkbox state when the settings section expands. `None`
means the app has not supplied settings state; the screen
loads it from config the first time the section expands.
preloads it from config on mount so the first expand has
the values ready.
"""
super().__init__()
self._notifications = notifications
@@ -444,6 +458,7 @@ class NotificationCenterScreen(ModalScreen[NotificationActionResult | None]):
self._settings_expanded = False
self._settings_loading = False
self._settings_transitioning = False
self._settings_preloaded = asyncio.Event()
@property
def settings_expanded(self) -> bool:
@@ -500,7 +515,7 @@ class NotificationCenterScreen(ModalScreen[NotificationActionResult | None]):
yield Static(self._help_text(), classes="nc-help")
def on_mount(self) -> None:
"""Apply ASCII borders and highlight the first row."""
"""Apply ASCII borders, highlight the first row, preload settings."""
if is_ascii_mode():
container = self.query_one(Vertical)
colors = theme.get_theme_colors(self)
@@ -508,6 +523,32 @@ class NotificationCenterScreen(ModalScreen[NotificationActionResult | None]):
if self._rows:
self._rows[0].set_selected(selected=True)
self._rows[0].scroll_visible()
if self._suppressed is None:
# Read the suppressed keys now so the first expand can mount the
# checkboxes immediately. Loading on expand would leave the pane
# open but unfocused for the duration of the config read, which
# also delayed the footer hint's "Space/Enter toggle" verb.
self.run_worker(
self._preload_settings(),
group="nc-preload",
# A preferences read must not be able to kill the session.
# The default would route any escape to `WorkerFailed`.
exit_on_error=False,
)
async def _preload_settings(self) -> None:
"""Load suppressed keys in the background so expand need not wait."""
self._settings_loading = True
try:
suppressed = await self._load_suppressed()
if self._suppressed is None:
self._suppressed = suppressed
finally:
# Release the waiter even when the read raises or the worker is
# cancelled. `_expand_settings` blocks on this event, so a missed
# `set()` would wedge the settings row for the life of the screen.
self._settings_loading = False
self._settings_preloaded.set()
def _settings_has_focus(self) -> bool:
"""Whether key focus is inside the expanded settings checkboxes.
@@ -642,8 +683,6 @@ class NotificationCenterScreen(ModalScreen[NotificationActionResult | None]):
host app).
"""
message.stop()
if self._settings_loading:
return
self.run_worker(self._toggle_settings(), group="nc-settings")
async def _toggle_settings(self) -> None:
@@ -677,55 +716,93 @@ class NotificationCenterScreen(ModalScreen[NotificationActionResult | None]):
assert isinstance(row, _NotificationSettingsRow) # noqa: S101
return row
async def _await_preload(self) -> set[str] | None:
"""Wait for the mount preload to publish the suppressed keys.
Bounded so a stalled config read (an unresponsive network home
directory, a held lock) cannot block the expand forever. On timeout
or a failed read the section stays collapsed rather than rendering
every warning as enabled, which is the unsafe direction to lie in.
Returns:
The preloaded suppressed keys, or `None` when the read timed out
or failed and the expand must not go on.
"""
try:
async with asyncio.timeout(_PRELOAD_TIMEOUT_SECONDS):
await self._settings_preloaded.wait()
except TimeoutError:
logger.warning("Notification settings preload did not complete in time")
if self._suppressed is not None:
return self._suppressed
# The preload released the event without keys, so it raised or was
# cancelled; `_load_suppressed` handles its own errors and returns a
# set, so reaching here means an unexpected failure.
if self.is_mounted:
self.app.notify(
"Could not read notification preferences. Reopen to retry.",
severity="warning",
timeout=6,
markup=False,
)
return None
async def _expand_settings(self) -> None:
"""Mount the warning checkboxes under the settings row.
Loads suppressed keys from config on first expand when the app did
not supply them at construction. A `reload()` that rebuilt the row
list while the section was open calls this to remount the checkboxes,
so an already-`_settings_expanded` state only skips a *mounted*
section.
Uses the suppressed keys supplied at construction, or the ones
`on_mount` preloaded. Reaching the wait below means the preload has
not stored them yet, so that path always blocks; it is bounded so a
stalled config read cannot wedge the row. A `reload()` that rebuilt
the row list while the section was open calls this to remount the
checkboxes, so an already-`_settings_expanded` state only skips a
*mounted* section.
"""
if self._settings_expanded and self.query("#nc-settings-group"):
return
if self._settings_loading:
return
# Drop a stale group that a `reload()` rebuild or an interrupted
# collapse left mounted, so the mount below cannot raise
# `DuplicateIds` on `#nc-settings-group`.
for stale in self.query("#nc-settings-group"):
await stale.remove()
if self._suppressed is None:
self._settings_loading = True
try:
self._suppressed = await self._load_suppressed()
finally:
self._settings_loading = False
suppressed = self._suppressed
if suppressed is None:
suppressed = await self._await_preload()
if suppressed is None:
return
# A rapid Esc while the config read was in flight already
# dismissed the screen; expanding a dead screen would raise.
if not self.is_mounted:
return
self._settings_expanded = True
self._settings_row().set_expanded(True)
scroll = self.query_one(VerticalScroll)
group = _NotificationSettingsGroup(id="nc-settings-group")
await scroll.mount(group)
checkboxes = [
Checkbox(
label,
value=key not in self._suppressed,
id=f"ns-{key}",
)
for key, label in WARNING_TOGGLES
]
await group.mount(*checkboxes)
# Batch so the display timer cannot paint between the mount and the
# focus handoff; an unbatched timer would otherwise show the pane
# open with nothing focused (and the footer hint still on its
# collapsed verbs) for a frame before the focused checkbox lands.
with self.app.batch_update():
await scroll.mount(group)
checkboxes = [
Checkbox(
label,
value=key not in suppressed,
id=f"ns-{key}",
)
for key, label in WARNING_TOGGLES
]
await group.mount(*checkboxes)
self._set_selected(len(self._rows) - 1)
if checkboxes:
checkboxes[0].focus()
self._settings_row().scroll_visible()
self._refresh_help()
self._settings_expanded = True
self._settings_row().set_expanded(True)
self._refresh_help()
self._set_selected(len(self._rows) - 1)
if checkboxes:
# `Widget.focus()` defers via `app.call_later`.
# `Screen.set_focus` applies in this same turn, so the pane
# cannot paint open with nothing focused.
self.set_focus(checkboxes[0])
self._settings_row().scroll_visible()
async def _collapse_settings(self) -> None:
"""Unmount the warning checkboxes and return focus to the row."""
@@ -742,27 +819,35 @@ class NotificationCenterScreen(ModalScreen[NotificationActionResult | None]):
async def _load_suppressed(self) -> set[str]:
"""Read suppressed warning keys from config off the event loop.
An unreadable or malformed config falls back to empty (all warnings
shown) with a warning toast, matching the old settings modal.
Falls back to empty (all warnings shown) with a warning toast. Note
that `is_warning_suppressed` already swallows an unreadable or
malformed config and returns `False` per key, so the ordinary config
failure defaults silently and never reaches the handler here; this
catches the rest.
Returns:
The set of suppressed warning keys from `config.toml`.
"""
from deepagents_code.model_config import is_warning_suppressed
suppressed: set[str] = set()
try:
# Imported inside the try: a failure here must degrade to
# defaults like any other read failure, not escape the worker.
from deepagents_code.model_config import is_warning_suppressed
for key, _ in WARNING_TOGGLES:
if await asyncio.to_thread(is_warning_suppressed, key):
suppressed.add(key)
except Exception:
logger.warning("Failed to read notification settings", exc_info=True)
self.app.notify(
"Could not read notification preferences. Showing defaults.",
severity="warning",
timeout=6,
markup=False,
)
# Guarded: a raise from the recovery path would replace the
# handled error and escape as an unhandled one.
with contextlib.suppress(Exception):
self.app.notify(
"Could not read notification preferences. Showing defaults.",
severity="warning",
timeout=6,
markup=False,
)
return set()
return suppressed
@@ -883,12 +968,16 @@ class NotificationCenterScreen(ModalScreen[NotificationActionResult | None]):
Preserves a notification selection by key and keeps the settings row
selected across refreshes. When a selected notification disappears,
the cursor clamps to the remaining notification rows before falling
back to settings. An expanded settings section is remounted so the
refresh never silently collapses it.
back to settings. Warning preferences are refreshed from config so an
in-place suppression is reflected if the user expands settings next.
An expanded settings section is remounted so the refresh never silently
collapses it.
Args:
notifications: Current pending entries to display.
"""
self._suppressed = await self._load_suppressed()
prev_key: str | None = None
settings_selected = False
if self._rows and 0 <= self._selected < len(self._rows):
libs/code/tests/unit_tests/tui/widgets/test_notification_settings.py
+155
View File
@@ -2,13 +2,16 @@
from __future__ import annotations
import asyncio
from unittest.mock import patch
from textual.app import App
from textual.widgets import Checkbox, Static
from deepagents_code.approval_mode import YOLO_WARNING_KEY
from deepagents_code.config import get_glyphs
from deepagents_code.model_config import is_warning_suppressed, suppress_warning
from deepagents_code.tui.widgets import notification_center
from deepagents_code.tui.widgets.notification_center import NotificationCenterScreen
from deepagents_code.tui.widgets.notification_settings import WARNING_TOGGLES
@@ -17,6 +20,133 @@ class _NotificationSettingsHost(App[None]):
"""Minimal host app for mounting `NotificationCenterScreen` in tests."""
async def test_failed_preload_leaves_the_row_usable_not_wedged() -> None:
"""A raising preload must release the waiter instead of hanging expand.
`_settings_preloaded` is set in a `finally`, so an expand that blocks on
it returns rather than waiting forever with `_settings_transitioning`
stuck. The section stays collapsed instead of rendering every warning as
enabled, which would be the unsafe direction to lie in.
"""
async def failing_load( # noqa: RUF029 # awaited by the preload worker
_screen: NotificationCenterScreen,
) -> set[str]:
msg = "config unreadable"
raise RuntimeError(msg)
app = _NotificationSettingsHost()
with patch.object(NotificationCenterScreen, "_load_suppressed", failing_load):
async with app.run_test() as pilot:
screen = NotificationCenterScreen([])
await app.push_screen(screen)
await screen._settings_preloaded.wait()
await pilot.press("enter")
await pilot.pause()
assert not screen.settings_expanded
assert not list(screen.query(Checkbox))
# A second press must still be accepted, not blocked by a
# transition flag left set by an abandoned expand.
await pilot.press("enter")
await pilot.pause()
assert not screen.settings_expanded
async def test_stalled_preload_times_out_instead_of_blocking_expand() -> None:
"""A config read that never returns must not hang the expand forever.
Simulates an unresponsive filesystem: the read is held open past the
timeout, so the expand gives up and leaves the section collapsed rather
than waiting on an event that may never arrive.
"""
release = asyncio.Event()
async def stalled_load(_screen: NotificationCenterScreen) -> set[str]:
await release.wait()
return set()
app = _NotificationSettingsHost()
with (
patch.object(NotificationCenterScreen, "_load_suppressed", stalled_load),
patch.object(notification_center, "_PRELOAD_TIMEOUT_SECONDS", 0.05),
):
async with app.run_test() as pilot:
screen = NotificationCenterScreen([])
await app.push_screen(screen)
await pilot.pause()
await pilot.press("enter")
await pilot.pause()
await asyncio.sleep(0.1)
await pilot.pause()
assert not screen.settings_expanded
assert not list(screen.query(Checkbox))
# The load-bearing assertion: an expand still parked on the
# event would leave this set, wedging every later toggle. A
# collapsed section alone does not distinguish "gave up" from
# "still waiting".
assert not screen._settings_transitioning
release.set()
async def test_first_toggle_waits_for_settings_preload() -> None:
"""The first Enter is honored even while the config read is in flight."""
started = asyncio.Event()
release = asyncio.Event()
async def blocked_load(_screen: NotificationCenterScreen) -> set[str]:
started.set()
await release.wait()
return {YOLO_WARNING_KEY}
app = _NotificationSettingsHost()
with patch.object(NotificationCenterScreen, "_load_suppressed", blocked_load):
async with app.run_test() as pilot:
screen = NotificationCenterScreen([])
await app.push_screen(screen)
await started.wait()
await pilot.press("enter")
await pilot.pause()
assert not screen.settings_expanded
release.set()
await pilot.pause()
await pilot.pause()
assert screen.settings_expanded
checkbox = screen.query_one(f"#ns-{YOLO_WARNING_KEY}", Checkbox)
assert checkbox.value is False
async def test_reload_refreshes_preloaded_settings() -> None:
"""A reload picks up a suppression saved after the initial preload."""
suppressed: set[str] = set()
async def load( # noqa: RUF029 # awaited by the production reload path
_screen: NotificationCenterScreen,
) -> set[str]:
return set(suppressed)
app = _NotificationSettingsHost()
with patch.object(NotificationCenterScreen, "_load_suppressed", load):
async with app.run_test() as pilot:
screen = NotificationCenterScreen([])
await app.push_screen(screen)
await screen._settings_preloaded.wait()
suppressed.add(YOLO_WARNING_KEY)
await screen.reload([])
await pilot.press("enter")
await pilot.pause()
checkbox = screen.query_one(f"#ns-{YOLO_WARNING_KEY}", Checkbox)
assert checkbox.value is False
async def test_notification_center_dims_underlying_content() -> None:
"""The modal must inherit the translucent `ModalScreen` backdrop.
@@ -76,6 +206,31 @@ async def test_help_footer_documents_both_toggle_keys_when_expanded() -> None:
assert "Esc collapse" in help_text
async def test_settings_row_leading_glyph_marks_expanded_state() -> None:
"""The disclosure affordance lives in the leading glyph, not a suffix."""
glyphs = get_glyphs()
app = _NotificationSettingsHost()
async with app.run_test() as pilot:
screen = NotificationCenterScreen([], suppressed=set())
await app.push_screen(screen)
await pilot.pause()
row = screen.query_one("#nc-settings", Static)
assert str(row.content) == f"{glyphs.cursor} Notification settings"
await pilot.press("enter")
await pilot.pause()
assert str(row.content) == f"{glyphs.disclosure_expanded} Notification settings"
await pilot.press("escape")
await pilot.pause()
# Back to the cursor: a stale expanded glyph over a closed pane is
# the affordance bug this row's rendering exists to avoid.
assert str(row.content) == f"{glyphs.cursor} Notification settings"
def test_cold_cache_warning_is_listed() -> None:
"""The advisory spend gate can be disabled from `/notifications`."""
assert any(key == "cold-cache" for key, _ in WARNING_TOGGLES)