Record the clear that happened, not the one that was asked for
`ClaudeDriver::clear` emitted `Event::Cleared` beside the `/clear` it sent, so the divider recorded a request. A reader scrolling back takes that mark as a fact about the conversation -- the session no longer has what is above this -- and a request is a different claim from a result. Compaction already gets this right by taking its mark from the CLI's own `compact_boundary` rather than from somebody pressing Compact; this is the same rule, and it was the one place left applying it to the request. Caught in review by the session this was measured against, which also established that the CLI's `/clear` is declared `supportsNonInteractive` and returns empty text with no result line -- so a fresh `init` bearing a different `session_id` is the only trace it leaves. The reader already watches for exactly that in order to persist the resume token, so the mark now goes out there. It has to be a *replacement* rather than any change, and the tests pin both ways of getting that wrong. The first `init` sets the id from nothing, which would otherwise open every session with a divider announcing a clear that never happened. And a compaction re-announces `init` carrying the *same* id, which would otherwise draw a clear on top of the compaction's own mark -- that one was found by writing the test rather than by reasoning about the change. 73 tests, clippy clean, rustfmt clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VETa8afmpWaYezLCqJhDB8
This commit is contained in:
1 parent
cf3ed68511
commit
5f6fd34aba
1 file changed
+90
-8
@@ -570,12 +570,13 @@ impl Driver for ClaudeDriver {
|
|||||||
}
|
}
|
||||||
|
|
||||||
fn clear(&self) {
|
fn clear(&self) {
|
||||||
// The CLI answers this with a fresh `init` carrying a new
|
// Nothing is emitted here on purpose. The CLI answers this with a
|
||||||
// `session_id`, and the reader persists that as the resume token
|
// fresh `init` carrying a new `session_id`, the reader persists
|
||||||
// the moment it changes -- so the next launch of this session
|
// that as the resume token, and `Event::Cleared` is emitted at
|
||||||
// resumes the cleared conversation rather than the old one, with
|
// that observation -- so the transcript records a clear that
|
||||||
// nothing here to keep in step.
|
// happened rather than one that was asked for, and the next
|
||||||
let _ = self.sink.send(Event::Cleared);
|
// launch resumes the cleared conversation with nothing here to
|
||||||
|
// keep in step.
|
||||||
self.local_command("/clear".to_string());
|
self.local_command("/clear".to_string());
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -789,16 +790,36 @@ fn translate_line(
|
|||||||
tracing::warn!("unparseable claude output line: {shown}");
|
tracing::warn!("unparseable claude output line: {shown}");
|
||||||
return true;
|
return true;
|
||||||
};
|
};
|
||||||
let (events, new_session_id) = {
|
let (events, new_session_id, replaced) = {
|
||||||
let mut state = state.lock().unwrap();
|
let mut state = state.lock().unwrap();
|
||||||
let before = state.session_id.clone();
|
let before = state.session_id.clone();
|
||||||
let events = state.translate(&message);
|
let events = state.translate(&message);
|
||||||
let after = state.session_id.clone();
|
let after = state.session_id.clone();
|
||||||
(events, if before != after { after } else { None })
|
// A *replacement* -- an id where there already was a different
|
||||||
|
// one -- rather than merely a change. The first `init` of a
|
||||||
|
// session sets the id from nothing, which is a session starting
|
||||||
|
// and not a conversation being dropped.
|
||||||
|
let replaced = before.is_some() && before != after;
|
||||||
|
(events, if before != after { after } else { None }, replaced)
|
||||||
};
|
};
|
||||||
if let Some(session_id) = new_session_id {
|
if let Some(session_id) = new_session_id {
|
||||||
write_resume_token(session_dir, &session_id);
|
write_resume_token(session_dir, &session_id);
|
||||||
}
|
}
|
||||||
|
// Here rather than where `/clear` is sent, because this is the CLI
|
||||||
|
// saying it happened and that is a different claim from us having
|
||||||
|
// asked. The divider is read as a fact about the conversation -- a
|
||||||
|
// reader scrolling back takes it to mean the session no longer has
|
||||||
|
// what is above it -- so it has to be anchored to the observation,
|
||||||
|
// the same way `Compacted` is anchored to `compact_boundary` rather
|
||||||
|
// than to somebody pressing Compact.
|
||||||
|
//
|
||||||
|
// The CLI's `/clear` returns empty text and leaves no result line, so
|
||||||
|
// a fresh `init` bearing a different `session_id` is the only trace
|
||||||
|
// it leaves. That is this, and it is already being watched for in
|
||||||
|
// order to persist the resume token.
|
||||||
|
if replaced && sink.send(Event::Cleared).is_err() {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
for event in events {
|
for event in events {
|
||||||
// Anything the CLI says after a steer was written is proof it has
|
// Anything the CLI says after a steer was written is proof it has
|
||||||
// been round the model again, and the steer went with it -- so
|
// been round the model again, and the steer went with it -- so
|
||||||
@@ -1016,6 +1037,67 @@ fn attachment_block(session_dir: &Path, id: &str) -> Result<Value> {
|
|||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
|
|
||||||
|
/// Drives real CLI output lines through the reader and collects what
|
||||||
|
/// came out, which is the only way to check the wiring between "the
|
||||||
|
/// CLI said this" and "the transcript records that".
|
||||||
|
fn events_from_lines(lines: &[&str]) -> Vec<Event> {
|
||||||
|
let dir = tempfile::tempdir().expect("temp dir");
|
||||||
|
let state = Arc::new(Mutex::new(Translator::new(dir.path().to_path_buf())));
|
||||||
|
let queue = Arc::new(Mutex::new(Queue::default()));
|
||||||
|
let (sink, mut out) = mpsc::unbounded_channel::<Event>();
|
||||||
|
for line in lines {
|
||||||
|
assert!(translate_line(line, dir.path(), &state, &sink, &queue));
|
||||||
|
}
|
||||||
|
drop(sink);
|
||||||
|
let mut events = Vec::new();
|
||||||
|
while let Ok(event) = out.try_recv() {
|
||||||
|
events.push(event);
|
||||||
|
}
|
||||||
|
events
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A session starting is not a conversation being dropped.
|
||||||
|
///
|
||||||
|
/// The first `init` sets the session id from nothing, which looks like
|
||||||
|
/// a change and is not a replacement -- reading it as one would open
|
||||||
|
/// every session with a divider announcing a clear that never
|
||||||
|
/// happened.
|
||||||
|
#[test]
|
||||||
|
fn the_first_init_is_not_a_clear() {
|
||||||
|
let events = events_from_lines(&[
|
||||||
|
r#"{"type":"system","subtype":"init","session_id":"first","tools":[],"model":"claude-haiku-4-5-20251001"}"#,
|
||||||
|
]);
|
||||||
|
assert!(!events.contains(&Event::Cleared), "got {events:?}");
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The divider is anchored to the CLI saying it happened, not to us
|
||||||
|
/// asking. `/clear` returns empty text and leaves no result line, so a
|
||||||
|
/// fresh `init` bearing a *different* session id is the only trace it
|
||||||
|
/// leaves -- and that trace is what records the clear.
|
||||||
|
#[test]
|
||||||
|
fn a_second_init_with_a_new_id_is_a_clear() {
|
||||||
|
let events = events_from_lines(&[
|
||||||
|
r#"{"type":"system","subtype":"init","session_id":"first","tools":[],"model":"claude-haiku-4-5-20251001"}"#,
|
||||||
|
r#"{"type":"system","subtype":"init","session_id":"second","tools":[],"model":"claude-haiku-4-5-20251001"}"#,
|
||||||
|
]);
|
||||||
|
assert_eq!(
|
||||||
|
events.iter().filter(|e| **e == Event::Cleared).count(),
|
||||||
|
1,
|
||||||
|
"got {events:?}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A compaction also re-announces `init`, carrying the *same* session
|
||||||
|
/// id. That must not draw a clear on top of the compaction's own mark.
|
||||||
|
#[test]
|
||||||
|
fn an_init_repeating_the_same_id_is_not_a_clear() {
|
||||||
|
let events = events_from_lines(&[
|
||||||
|
r#"{"type":"system","subtype":"init","session_id":"same","tools":[],"model":"claude-haiku-4-5-20251001"}"#,
|
||||||
|
r#"{"type":"system","subtype":"init","session_id":"same","tools":[],"model":"claude-haiku-4-5-20251001"}"#,
|
||||||
|
]);
|
||||||
|
assert!(!events.contains(&Event::Cleared), "got {events:?}");
|
||||||
|
}
|
||||||
|
|
||||||
/// The failure this exists for: a shell's complaint ends with a blank
|
/// The failure this exists for: a shell's complaint ends with a blank
|
||||||
/// line, so reporting "the last line of stderr" reported nothing, and
|
/// line, so reporting "the last line of stderr" reported nothing, and
|
||||||
/// the phone showed a bare exit status while the reason sat in the
|
/// the phone showed a bare exit status while the reason sat in the
|
||||||
|
|||||||
Reference in new issue
Block a user