From fd71d876e1ee80f488448253192ab0614f78a4e1 Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Sun, 6 Sep 2026 22:23:45 -0400 Subject: [PATCH] Never let one unreadable line take a transcript down Removing `Event::TaskNote` hours after adding it made every transcript that had recorded one unreadable. `Transcript::open` parses every line, so `launch` failed for those sessions and `SessionManager::new` logged "couldn't relaunch session " and skipped them -- and a skipped session has no pump and no driver. On the phone that is no status, no history and nothing sendable, for every live session that had run a background task. One unfamiliar word took down every conversation it appeared in. A transcript is append-only and permanent, so the set of kinds one can hold only ever grows: what this build writes is not what it may have to read. A line can come from a newer server, or from an older one that wrote a kind since dropped, and neither may be able to end the file. `Indexed::parse_at` degrades a line it cannot make sense of to `Event::Unreadable { kind }` instead of failing the whole read. It keeps the line's seq -- the cursors, the page bisection and the next-seq counter are all addressed by it, and dropping the line would hand out a seq the file already contains -- and carries the word the line called itself, so the phone can say what is missing rather than that something is. A line with no readable seq is still an error: that one cannot be placed at all. `Event::TaskNote` comes back retired rather than deleted: deserializable, never constructed, dated, with the reason on it. The phone folds it to no row, which is the point -- an unreadable line correctly draws a placeholder, and one per background task is the wall the row was removed for in the first place. Found while diagnosing a report that live sessions had lost their status and could not be sent to. 173 server tests pass, including the new one, which fails on the old code within a second. Co-Authored-By: Claude Opus 5 --- AGENTS.md | 13 ++ PLAN.md | 28 +++++ .../main/kotlin/com/example/aiapp/Events.kt | 22 ++++ .../com/example/aiapp/TranscriptItems.kt | 6 + server/src/session/driver.rs | 42 +++++++ server/src/session/transcript.rs | 111 ++++++++++++++++-- 6 files changed, 215 insertions(+), 7 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 07667f2..0468eed 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -296,6 +296,19 @@ written, and the fold uses that same predicate to decide a reply is settled. ## Things that have bitten +- **A transcript outlives the enum.** Removing `Event::TaskNote` hours after + adding it made every transcript that had recorded one unreadable, so + `launch` failed for those sessions and `SessionManager::new` skipped them — + no status, nothing sendable, no new messages, for every live session that + had run a background task. **The set of kinds a transcript can hold only + ever grows**: a line may come from a newer server or from an older one that + wrote a kind since dropped, and one unfamiliar word must never be able to + end the file. `Indexed::parse_at` degrades a line it cannot read to + `Event::Unreadable { kind }`, keeping its seq — which is what everything + downstream is addressed by — and the phone draws it as a placeholder saying + which kind. Never delete a variant instead of retiring it; `Event::TaskNote` + is what retiring looks like, and the phone folds it to no row. + Project-specific only — a lesson that would bite any project on this machine belongs in `~/.claude/TOOLCHAIN.md` or `~/.claude/MACHINE.md` instead. diff --git a/PLAN.md b/PLAN.md index a961dc8..9ffb5e1 100644 --- a/PLAN.md +++ b/PLAN.md @@ -706,6 +706,34 @@ those, and `open_tasks` covers the backgrounded command, which has no subagent to be found in the registry at all. Both are needed and neither subsumes the other. +### A transcript outlives this enum (2026-09-06) + +**The set of event kinds a transcript can hold only ever grows.** It is +append-only and permanent, so what this build *writes* is not what it may have +to *read*: a line can come from a newer server, or from an older one that wrote +a kind since dropped. + +That was learned the expensive way. `Event::TaskNote` was added and removed +again within hours, and every transcript that had recorded one became +unreadable — `Transcript::open` parses every line, so `launch` failed for those +sessions and `SessionManager::new` skipped them. On the phone that is a session +with no status, no history and nothing sendable: one unfamiliar word took down +every live conversation it appeared in. + +Two rules now. `Indexed::parse_at` degrades a line it cannot make sense of to +`Event::Unreadable { kind }` rather than failing the file, keeping its seq — +which is what the cursors, the bisection and the next-seq counter are all +addressed by — and carrying the word the line called itself, so the reader is +told what they are missing rather than that something is. The seq is still +required: a line that cannot say where it sits is not one this file can hold, +and dropping it silently would hand out a seq the file already contains. + +And a variant is **retired, not deleted**: kept deserializable, never +constructed, with the date and the reason on it. `Event::TaskNote` is the +example, and the phone folds it to no row — which is the point, since an +unreadable line correctly draws a placeholder and one per background task is +the wall the row was removed for. + ### A limit a subagent hits is the session's (2026-09-06) A background Task runs on long after its parent's turn ended, so **the account diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/Events.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/Events.kt index 8580867..203892e 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/Events.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/Events.kt @@ -108,6 +108,26 @@ sealed class SessionEvent { val turnStart: Long? = null, ) : SessionEvent() + /** + * A line in the transcript this build cannot read: a kind a newer server wrote, or one an older + * server wrote that has since been dropped. + * + * [kind] is the word the line called itself, so the row can say what is missing rather than + * that something is. The server makes these when reading; no driver sends one. + */ + data class Unreadable(val kind: String) : SessionEvent() + + /** + * Retired on 2026-09-06, hours after it was added: a background task finishing, which turned + * out to be a screenful of notices about work nobody was asking after. + * + * Kept because a transcript is append-only -- the sessions that ran a background task in that + * window have these lines for ever. It draws no row, which is the whole reason it is still + * named here rather than left to fall through to [Unknown]: that would draw a placeholder per + * background task, which is the same wall the row was removed for. + */ + object RetiredTaskNote : SessionEvent() + /** * A command the session was asked to run on itself and cannot run yet. Resolved by * [CommandSent] with the same id; a command that ran straight away has only that one. @@ -252,6 +272,8 @@ fun parseSeqEvent(json: String): SeqEvent { body.getString("text"), if (body.has("turnStart")) body.getLong("turnStart") else null, ) + "unreadable" -> SessionEvent.Unreadable(body.getString("kind")) + "taskNote" -> SessionEvent.RetiredTaskNote "commandQueued" -> SessionEvent.CommandQueued(body.getString("id"), body.getString("text")) "commandSent" -> SessionEvent.CommandSent(body.getString("id"), body.getString("text")) diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptItems.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptItems.kt index 19beb8e..27a78c9 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptItems.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptItems.kt @@ -515,6 +515,12 @@ fun foldEvent(items: List, entry: SeqEvent): List items + TranscriptItem.CompactedNote(entry.seq, event.preTokens, event.postTokens) is SessionEvent.Unknown -> items + TranscriptItem.Note(entry.seq, "[${event.type}]") + // Said rather than skipped: a line the server could not read is a hole in the conversation, + // and one that draws nothing is a hole nothing on screen ever mentions. + is SessionEvent.Unreadable -> + items + TranscriptItem.Note(entry.seq, "[unreadable: ${event.kind}]") + // No row: see [SessionEvent.RetiredTaskNote]. + is SessionEvent.RetiredTaskNote -> items // Screen-level state, not transcript rows -- see SessionScreen. is SessionEvent.UsageDelta -> items } diff --git a/server/src/session/driver.rs b/server/src/session/driver.rs index ad5351b..d67d34a 100644 --- a/server/src/session/driver.rs +++ b/server/src/session/driver.rs @@ -208,6 +208,48 @@ pub enum Event { #[serde(default, skip_serializing_if = "Option::is_none")] turn_start: Option, }, + /// **Retired on 2026-09-06, hours after it was added.** Kept only so the + /// transcripts written while it existed still read: a session that ran a + /// background task in that window has these lines for ever, and a + /// transcript is append-only, so there is no pass that could remove them. + /// + /// Never constructed. It reported a background task finishing, and drawing + /// a row per one turned out to be a screen of notices about work the + /// reader was not asking after -- see PLAN.md's "Two turns must never be + /// drawn as one". The phone folds it to no row at all, which is what makes + /// keeping it cheap. + /// + /// Deleting the variant instead is what broke every live session, and + /// [`Event::Unreadable`] is the reason that cannot happen again. This is + /// still here rather than left to that: an unreadable line draws a + /// placeholder, correctly, and one per background task is the same wall + /// the row was removed for. + TaskNote { + about: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + title: Option, + status: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + summary: Option, + }, + /// A line in a transcript that this build cannot read: a kind a newer + /// server wrote, a kind an older one wrote that has since been dropped, or + /// a line whose contents do not fit the kind it names. + /// + /// **Only ever made when reading, never sent by a driver**, and it is the + /// reason a transcript can outlive a change to this enum. See + /// `Indexed::parse_at` for the incident: removing a variant after + /// transcripts had recorded it made every read of those files fail, so + /// every session in them lost its status, its history and its ability to + /// be sent to. + /// + /// It carries the word the line called itself so a reader is told what + /// they are missing rather than that something is missing. `kind` is not + /// an enum for the obvious reason: the whole point of this variant is the + /// words that are not in one. + Unreadable { + kind: String, + }, /// The manager's record of a question being answered, so a rendered /// question card resolves on every device rather than only the one that /// answered. diff --git a/server/src/session/transcript.rs b/server/src/session/transcript.rs index 90a5c32..02d357d 100644 --- a/server/src/session/transcript.rs +++ b/server/src/session/transcript.rs @@ -309,18 +309,70 @@ impl<'a> Indexed<'a> { } fn parse(&self, range: Range) -> Result> { - self.lines[range] + self.lines[range.clone()] .iter() - .map(|at| { - serde_json::from_str(&self.text[at.clone()]) - .with_context(|| format!("bad transcript line in {}", self.path.display())) - }) + .enumerate() + .map(|(offset, at)| self.parse_at(range.start + offset, at.clone())) .collect() } fn parse_one(&self, index: usize) -> Result { - serde_json::from_str(&self.text[self.lines[index].clone()]) - .with_context(|| format!("bad transcript line in {}", self.path.display())) + self.parse_at(index, self.lines[index].clone()) + } + + /// One line, degrading to [`Event::Unreadable`] rather than failing when + /// this build cannot make sense of the event on it. + /// + /// **A transcript is append-only and permanent, so the set of kinds that + /// can appear in one only ever grows.** What this build writes is not what + /// it may have to read: a line may come from a newer server, or from an + /// older one that wrote a kind since dropped. Refusing the whole file for + /// one such line is what happened on 2026-09-06 -- a kind was removed after + /// transcripts had recorded it, every read of those files failed, and the + /// sessions in them could not be opened, listed, paged or sent to. One + /// unfamiliar word took down every conversation it appeared in. + /// + /// So the line survives as a line. It keeps its seq, which is the part + /// everything downstream is addressed by, and says what it was rather than + /// pretending to be something -- there is a row for that on the phone + /// already. + /// + /// The seq itself is still required, and this still fails without one: a + /// line that cannot say where it sits in the sequence is not a line this + /// file can hold, and quietly dropping it would hand out a seq the file + /// already contains. + fn parse_at(&self, index: usize, at: Range) -> Result { + let line = &self.text[at]; + match serde_json::from_str(line) { + Ok(entry) => Ok(entry), + Err(err) => { + #[derive(Deserialize)] + struct JustPlace { + seq: u64, + ts: f64, + #[serde(rename = "type")] + kind: Option, + } + let place: JustPlace = serde_json::from_str(line).with_context(|| { + format!("bad transcript line in {}", self.path.display()) + })?; + // Debug rather than a warning: a transcript written against a + // newer build has one of these per line it wrote, and the row + // on the phone is where this is actually reported. + tracing::debug!( + "transcript line {index} of {} (seq {}) is not one this build can read: {err}", + self.path.display(), + place.seq, + ); + Ok(SeqEvent { + seq: place.seq, + ts: place.ts, + event: Event::Unreadable { + kind: place.kind.unwrap_or_else(|| "no kind".to_string()), + }, + }) + } + } } /// The newest `limit` *rows* ending at line `end`, with each run of @@ -396,6 +448,51 @@ mod tests { } } + /// The failure that took every live session down on 2026-09-06: an event + /// kind was removed from the enum after transcripts had already recorded + /// it, so every read of those files failed and the sessions in them could + /// not be opened, listed, paged or sent to. + /// + /// A transcript is append-only and permanent, so **the set of kinds that + /// can appear in one only ever grows** -- what this build writes is not + /// what it may have to read. A line it cannot make sense of has to be a + /// line, not the end of the file. + #[test] + fn a_line_of_a_kind_this_build_does_not_know_does_not_break_the_file() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("transcript.jsonl"); + std::fs::write( + &path, + concat!( + r#"{"seq":1,"ts":1.0,"type":"status","state":"running"}"#, + "\n", + r#"{"seq":2,"ts":2.0,"type":"assistantText","delta":"hi"}"#, + "\n", + r#"{"seq":3,"ts":3.0,"type":"taskNote","about":"t","title":"h","status":"completed"}"#, + "\n", + r#"{"seq":4,"ts":4.0,"type":"somethingFromTheFuture","whatever":[1,2]}"#, + "\n", + r#"{"seq":5,"ts":5.0,"type":"status","state":"idle"}"#, + "\n", + ), + ) + .expect("write"); + + let entries = read_after(&path, 0).expect("a strange line is not a broken file"); + assert_eq!(entries.len(), 5, "every line is still a line: {entries:?}"); + assert_eq!(entries[4].seq, 5); + + let transcript = Transcript::open(&path).expect("open"); + assert_eq!(transcript.last_status(), Some(SessionStatus::Idle)); + // The next seq is counted from the newest *line*, whatever kind it is. + // Skipping the ones this build cannot read would hand out a seq the + // file already contains. + assert_eq!(transcript.next_seq, 6); + + let window = read_window(&path, None, None, 80, false).expect("window"); + assert_eq!(window.len(), 5, "{window:?}"); + } + #[test] fn assigns_increasing_seqs_and_replays_after_a_cursor() { let dir = tempfile::tempdir().expect("tempdir");