Skip to content

feat(tui): add alphabetical sorting keybinding ('O') and instant in-memory sorting - #261

Closed
VictorMorand wants to merge 2 commits into
experimaestro:masterfrom
VictorMorand:master
Closed

feat(tui): add alphabetical sorting keybinding ('O') and instant in-memory sorting#261
VictorMorand wants to merge 2 commits into
experimaestro:masterfrom
VictorMorand:master

Conversation

@VictorMorand

Copy link
Copy Markdown
Contributor

Commit 1: feat(tui): add 'O' keybinding for alphabetical sorting in TUI

• TUI Widgets: Added shortcut O (sort_alphabetical) to ExperimentsList and JobsTable to sort experiments by ID and jobs by
Job ID alphabetically (case-insensitive, toggleable asc/desc).
• UI & Documentation: Updated the in-app help modal (HelpScreen) and documentation (docs/source/interfaces.md) with
keybinding O.
• Testing: Added unit tests covering alphabetical sorting logic and binding definitions in test_tui_sorting.py.

Commit 2: perf(tui): make table sorting non-blocking via in-memory re-sorting

• In-Memory Sorting: Implemented apply_sort() on JobsTable and ExperimentsList to re-sort cached data (_cached_jobs,
_runs_counts) instantly on the UI thread with zero network/disk latency.
• I/O Decoupling: Prevented sort keybindings (O, S, D, T) and header clicks from triggering expensive background re-fetches
over RPC/disk (full re-queries remain reserved for explicit refresh r).
• Testing: Added unit test test_apply_sort_in_memory ensuring sorting bypasses background fetch workers when data is already
cached.

@bpiwowar bpiwowar self-assigned this Jul 31, 2026
@bpiwowar

Copy link
Copy Markdown
Collaborator

Thanks @VictorMorand! The alphabetical sorting is merged into master as 99dd50e, with a few adjustments:

  • reverted the show=Falseshow=True flip on the pre-existing f, ctrl+d, ctrl+k, S, D experiment bindings (unrelated to the feature, and it changed the footer); the new O binding is show=False like the other sort bindings;
  • extracted the sort logic into ExperimentsList._sort_experiments() and JobsTable._sort_jobs(), out of the two large display handlers;
  • reworked test_tui_sorting.py so it exercises those methods and the action_sort_* toggle logic rather than re-testing sorted().

The second commit (in-memory re-sorting with _cached_jobs / _runs_counts in the widgets) is not merged. The problem it addresses is real — but it lives in the wrong layer: the same caching would have to be duplicated in the web UI, and it doesn't address the actual cost, which is that both UIs re-query the state provider on every state event (5 provider calls per jobs refresh, plus one get_experiment_runs() per experiment for the runs column — that's 5 RPC round trips and N more on the SSH provider).

So caching is being moved into OfflineStateProvider instead: read queries (get_experiments, get_experiment_runs, get_jobs, get_tags_map, …) become cached by default and invalidated by the event stream that already feeds the provider (watchdog locally, push notifications remotely), generalizing the pattern get_services() already uses, with an explicit opt-out for a forced re-read. The UIs then need no cache of their own, and sorting becomes instant as a side effect. Follow-up work is tracked separately.

Closing this PR since the sorting part is in — the rest is superseded.

@bpiwowar bpiwowar closed this Jul 31, 2026
@VictorMorand

Copy link
Copy Markdown
Contributor Author

Definitely the way to go, thanks a lot !

@bpiwowar

Copy link
Copy Markdown
Collaborator

Version v2.6.2b0 should include both the sorting and the global cache - let me know if there is any problem before a stable release

VictorMorand pushed a commit to VictorMorand/experimaestro-python that referenced this pull request Jul 31, 2026
The UIs re-query the state provider on every state event: a jobs refresh
costs five provider calls, and the experiments list costs one call per
experiment for the runs column. On the workspace provider that means a
filesystem scan each time; on the SSH provider, that many blocking RPC
round-trips. Sorting a table paid the same price, since it simply asked
for the data again.

Read queries are now answered from a cache held by the provider, so no UI
needs a cache of its own:

- OfflineStateProvider implements the public get_* methods on top of a
  QueryCache, generalizing what get_services() already did; concrete
  providers implement the _fetch_* hooks, called only on a miss. Services
  are part of it, so a single cache of answers now sits alongside the two
  entity caches (jobs, experiments), which keep owning object identity
- events keep the cached answers up to date rather than dropping them:
  job state, progress and carbon events mutate the cached job objects in
  place, and a submitted job — whose event carries its task, tags,
  dependencies and submission time — is appended to the lists and maps it
  belongs to. Queries it cannot belong to (another run, task, state or
  tag filter) are untouched, those that cannot be decided (a `since`
  filter) are dropped, and the experiment list is left alone. Only
  payload-free events (experiment updated) and mutations invalidate
- get_*(..., refresh=True) forces a full re-read, and is wired to the
  explicit refresh actions only: 'r' in the jobs/orphans/stray panels,
  the TUI global refresh, and the web UI refresh message. The SSH client
  forwards it so the server re-reads as well
- results are copied on the way out, under the cache lock, so callers
  sorting or filtering them in place cannot corrupt the cache

The live Scheduler accepts refresh and ignores it — its state is never
stale.

Supersedes the widget-level caching of experimaestro#261, which was closed in favour
of solving this at the provider level; the sorting part of that PR was
merged separately in 99dd50e.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LU56vLBFQz5ryt2Sovf9Hj
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants