Let the reader put the session list in its own order
Nothing sorts the sessions tab any more. The order is the server's `sessions` list, which is the reader's arrangement: holding a row puts the screen in selection mode -- the same gesture and the same bottom bar as the import tab -- and each card grows a burger handle at its right edge that drags the row to a new place, with a tick of haptic feedback for each one it passes. The two attempts this replaces, sorting by activity and then by when each agent was turned on, were both looking for an order a session could not move itself out of; no rule computed from what a session is doing can be one. `POST /sessions/order` rewrites the config's order, so it is the same on every device and survives a backend restart, and `SessionConfig::started` goes with the sort that needed it. Rearranging is independent of the selection: the handle moves the row it is on, picked out or not. The click moved off the card and onto its contents so that a press landing on the handle cannot also select the row it is about to move. Selection's one action is Delete, which now takes the whole set. Two traps in `Reorder.kt`, both measured on the emulator and written down in `this-machine-android`: a crossing is decided from how far the finger has travelled, because a lazy list animates an item into its new place and its `offset` reports the old one for several frames; and the viewport is pinned with `requestScrollToItem` around each move, because a lazy list keeps its place by the key of the top item and would otherwise follow the row being dragged. Verified on the emulator against the sandbox: the order survives an app restart and a backend read-back, a two-row drag moves exactly two rows, a drag to the bottom edge scrolls the list and lands the row last, pressing the handle without moving changes nothing, and deleting two selected sessions leaves the rest in place.
This commit is contained in:
1 parent
942edd6b31
commit
b7fd18b195
12 files changed
+801
-339
No files matched your search
+58
-61
@@ -266,10 +266,6 @@ pub struct SessionInfo {
|
||||
pub status: SessionStatus,
|
||||
pub last_activity: f64,
|
||||
pub created: f64,
|
||||
/// Epoch seconds this session's agent was last turned on, which the
|
||||
/// session list orders the running ones by -- see
|
||||
/// [`SessionConfig::started`], whose fallback this resolves.
|
||||
pub started: f64,
|
||||
/// The provider's latest measured number of live background tasks.
|
||||
/// Absent until it reports one; absence is not a measured zero.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
@@ -622,7 +618,6 @@ impl LiveSession {
|
||||
status: *self.shared.status.lock().unwrap(),
|
||||
last_activity: *self.shared.last_activity.lock().unwrap(),
|
||||
created: self.meta.created,
|
||||
started: current.started_at(),
|
||||
background_tasks: self
|
||||
.driver()
|
||||
.and_then(|driver| driver.background_tasks())
|
||||
@@ -1179,7 +1174,6 @@ impl SessionManager {
|
||||
status: status_of_unlaunched(&self.data_dir.join(&meta.id)),
|
||||
last_activity: meta.created,
|
||||
created: meta.created,
|
||||
started: meta.started_at(),
|
||||
background_tasks: None,
|
||||
subagents: subagent::count(&self.data_dir.join(&meta.id)),
|
||||
},
|
||||
@@ -1371,9 +1365,6 @@ impl SessionManager {
|
||||
// do with it -- see `SessionConfig::throwaway`.
|
||||
throwaway: self.spawn_throwaway,
|
||||
created,
|
||||
// The spawn starts a process, so this session is turned on from
|
||||
// the moment it exists.
|
||||
started: Some(created),
|
||||
};
|
||||
|
||||
let session = launch(
|
||||
@@ -1722,6 +1713,35 @@ impl SessionManager {
|
||||
})
|
||||
}
|
||||
|
||||
/// Puts the session list in the order given, which is the order every
|
||||
/// screen then draws: the reader's arrangement, held on the server so it
|
||||
/// is the same on every device and survives a backend restart.
|
||||
///
|
||||
/// The request is the whole list as one phone last saw it, and it is
|
||||
/// applied as a preference rather than as a replacement: an id naming a
|
||||
/// session that is no longer here is ignored, and a session the caller
|
||||
/// did not name keeps its relative place at the end -- which is where a
|
||||
/// spawn puts one anyway. That is what makes a drag safe against a list
|
||||
/// that changed under it, for which the alternative was refusing the drag
|
||||
/// somebody has already made and watching the row spring back.
|
||||
pub fn reorder_sessions(&self, order: &[String]) -> Result<()> {
|
||||
let mut inner = self.inner.write().unwrap();
|
||||
let rank: HashMap<&str, usize> = order
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(at, id)| (id.as_str(), at))
|
||||
.collect();
|
||||
let mut candidate = inner.config.clone();
|
||||
// Stable, so the sessions with no rank of their own stay in the order
|
||||
// they are already in rather than in whatever order a sort leaves.
|
||||
candidate
|
||||
.sessions
|
||||
.sort_by_key(|meta| rank.get(meta.id.as_str()).copied().unwrap_or(usize::MAX));
|
||||
candidate.save(&self.config_path)?;
|
||||
inner.config = candidate;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Changes a session's model: persisted, so a respawn keeps it and the
|
||||
/// list shows it, and handed to the driver, which switches in place
|
||||
/// where its dialect can. Through the manager rather than the session,
|
||||
@@ -2071,18 +2091,6 @@ impl SessionManager {
|
||||
inner.live.insert(id.to_string(), session);
|
||||
}
|
||||
}
|
||||
// Turned on now, so it takes its place at the bottom of the running
|
||||
// group -- see `SessionConfig::started`. Best effort: a config that
|
||||
// cannot be written is not a reason to fail a start that has already
|
||||
// happened, and the cost is one row in the wrong place.
|
||||
let mut candidate = inner.config.clone();
|
||||
if let Some(meta) = candidate.sessions.iter_mut().find(|meta| meta.id == id) {
|
||||
meta.started = Some(now());
|
||||
match candidate.save(&self.config_path) {
|
||||
Ok(()) => inner.config = candidate,
|
||||
Err(err) => tracing::warn!("couldn't record that session {id} started: {err:#}"),
|
||||
}
|
||||
}
|
||||
// Nothing is announced from here. A driver that starts a process
|
||||
// reports the session idle itself, in order with everything else it
|
||||
// says about that process. Saying it here too would be a second
|
||||
@@ -4429,13 +4437,14 @@ mod tests {
|
||||
.await;
|
||||
}
|
||||
|
||||
/// The session list keeps the running sessions in the order their agents
|
||||
/// were turned on, so what that order is made of has to outlive both the
|
||||
/// process and the backend: a session started again belongs at the bottom
|
||||
/// of the group, and a restart adopts processes rather than starting them,
|
||||
/// so nothing in the new run could work the time out for itself.
|
||||
/// The order of the list is the reader's own, kept in the config, so it
|
||||
/// has to survive a backend restart: a phone reopening the tab must see
|
||||
/// the arrangement somebody dragged rather than the order things were
|
||||
/// spawned in. The ids left out of the request are the other half -- a
|
||||
/// session spawned on another device between the fetch and the drag is
|
||||
/// not in what the phone sent, and must not be dropped by it.
|
||||
#[tokio::test]
|
||||
async fn starting_a_session_again_records_when_it_was_turned_on() {
|
||||
async fn the_order_sessions_are_dragged_into_outlives_the_backend() {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let config_path = dir.path().join("config.ron");
|
||||
let data_dir = dir.path().join("sessions");
|
||||
@@ -4446,47 +4455,35 @@ mod tests {
|
||||
data_dir.join("models"),
|
||||
)
|
||||
.expect("manager");
|
||||
let info = manager.spawn_session(echo_spec()).expect("spawn");
|
||||
let spawned = info.started;
|
||||
assert!(
|
||||
(spawned - info.created).abs() < 1.0,
|
||||
"a spawned session was turned on at {spawned} rather than when it was created",
|
||||
let first = manager.spawn_session(echo_spec()).expect("spawn");
|
||||
let second = manager.spawn_session(echo_spec()).expect("spawn");
|
||||
let third = manager.spawn_session(echo_spec()).expect("spawn");
|
||||
let ids = |manager: &SessionManager| -> Vec<String> {
|
||||
manager
|
||||
.sessions()
|
||||
.into_iter()
|
||||
.map(|session| session.id)
|
||||
.collect()
|
||||
};
|
||||
assert_eq!(
|
||||
ids(&manager),
|
||||
vec![first.id.clone(), second.id.clone(), third.id.clone()],
|
||||
"a spawn belongs at the bottom of the list",
|
||||
);
|
||||
|
||||
let session = manager.session(&info.id).expect("live session");
|
||||
let mut rx = session.subscribe();
|
||||
let _ = session.sink.send(Event::Status {
|
||||
state: SessionStatus::Exited,
|
||||
});
|
||||
collect_until(&mut rx, |event| {
|
||||
matches!(
|
||||
event,
|
||||
Event::Status {
|
||||
state: SessionStatus::Exited
|
||||
}
|
||||
)
|
||||
})
|
||||
.await;
|
||||
// Otherwise the two starts can land in the same instant and the test
|
||||
// cannot tell a recorded time from a kept one.
|
||||
tokio::time::sleep(Duration::from_millis(20)).await;
|
||||
manager.start_session(&info.id).expect("start again");
|
||||
collect_until(&mut rx, is_idle).await;
|
||||
manager
|
||||
.reorder_sessions(&[third.id.clone(), first.id.clone()])
|
||||
.expect("reorder");
|
||||
let expected = vec![third.id.clone(), first.id.clone(), second.id.clone()];
|
||||
assert_eq!(ids(&manager), expected);
|
||||
|
||||
let restarted = manager.sessions()[0].started;
|
||||
assert!(
|
||||
restarted > spawned,
|
||||
"starting the session again left it reporting {restarted}, the {spawned} of the \
|
||||
process that had already ended",
|
||||
);
|
||||
drop(manager);
|
||||
|
||||
let manager = SessionManager::new(config_path, data_dir.clone(), data_dir.join("models"))
|
||||
.expect("manager restart");
|
||||
assert_eq!(
|
||||
manager.sessions()[0].started,
|
||||
restarted,
|
||||
"the backend restart lost when this session's agent was turned on",
|
||||
ids(&manager),
|
||||
expected,
|
||||
"the restart lost the reader's order"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user