From 474ec93118b7e6f3f8bd14fff3ff68fd38b9cbec Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Mon, 7 Sep 2026 10:23:23 -0700 Subject: [PATCH] fix(last): say when the folder fallback picked the transcript refs #57 --- README.md | 7 +++ crates/plannotator-tui/src/app/draw.rs | 10 ++-- crates/plannotator-tui/src/app/tests.rs | 11 ++++ crates/plannotator-tui/src/last/fallback.rs | 34 ++++++++---- crates/plannotator-tui/src/last/locate.rs | 58 +++++++++++++++++++-- crates/plannotator-tui/src/last/mod.rs | 52 +++++++++++++++++- 6 files changed, 153 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 0907702..3e2fe45 100644 --- a/README.md +++ b/README.md @@ -91,6 +91,13 @@ On Linux, an explicit Codex `--pid` selects the rollout opened by that process. cannot be identified uniquely, `last` reports the failure instead of choosing an unrelated session. `--session` and `--session-id` keep precedence over PID discovery. +Inside Herdr, the exact-session path needs the session id Herdr reports for the pane. Herdr's +Claude Code integration registers on `SessionStart`, so a session reports its id only when it +started after `herdr integration install claude`; a session that was already running when the +integration was installed reports none. Without an id, `last` shows the newest transcript for +the folder, which is a guess when several sessions share one directory, and says so in the +status line. + ## Where annotations live ``` diff --git a/crates/plannotator-tui/src/app/draw.rs b/crates/plannotator-tui/src/app/draw.rs index db0f76f..8f25a31 100644 --- a/crates/plannotator-tui/src/app/draw.rs +++ b/crates/plannotator-tui/src/app/draw.rs @@ -379,7 +379,10 @@ impl App { return; } let orphans = self.open.store.orphans(); - let mut parts = vec![ + // The status leads: it is the transient half of the line, and the name and counters + // it pushes right are on screen for the whole session anyway. + let mut parts: Vec = self.status.iter().cloned().collect(); + parts.extend([ self.open.source.name.clone(), format!( "{} annotations{}", @@ -393,10 +396,7 @@ impl App { } None => format!("block {}/{}", self.selected + 1, self.open.doc.blocks.len()), }, - ]; - if let Some(status) = &self.status { - parts.push(status.clone()); - } + ]); if frame.area().width < RAIL_MIN_TOTAL_WIDTH { parts.push("rail hidden: widen to ≥80 cols".into()); } diff --git a/crates/plannotator-tui/src/app/tests.rs b/crates/plannotator-tui/src/app/tests.rs index 406a053..1b58a33 100644 --- a/crates/plannotator-tui/src/app/tests.rs +++ b/crates/plannotator-tui/src/app/tests.rs @@ -126,6 +126,17 @@ fn the_picker_lists_newest_first_and_opens_the_chosen_message() { assert!(app.open.store.is_transient()); } +#[test] +fn a_status_leads_the_footer_so_a_narrow_pane_cannot_truncate_it_away() { + let mut app = App::open_message("claude", "/tmp/transcript.jsonl", candidates(), 60, Box::new(Discard)) + .expect("opens"); + app.handle_event(&Event::Key(KeyEvent::from(KeyCode::Esc))).expect("esc"); + app.set_status("no session id from Herdr, showing the newest transcript for this folder".to_owned()); + let rows = draw(&mut app); + let footer = row(&rows, rows.len() - 1); + assert!(footer.trim_start().starts_with("no session id from Herdr"), "footer was {footer:?}"); +} + #[test] fn escaping_the_picker_keeps_the_newest_message() { let mut app = App::open_message("claude", "/tmp/transcript.jsonl", candidates(), 60, Box::new(Discard)) diff --git a/crates/plannotator-tui/src/last/fallback.rs b/crates/plannotator-tui/src/last/fallback.rs index 60523ff..f792dfe 100644 --- a/crates/plannotator-tui/src/last/fallback.rs +++ b/crates/plannotator-tui/src/last/fallback.rs @@ -14,42 +14,58 @@ use super::exact; use super::readers; use super::roots::Roots; +/// How a transcript was chosen, so the UI can say when nothing identified it exactly. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum Discovery { + /// An explicit path, an exact session id, a thread id, or the file the agent process + /// itself has open: this transcript is the session, not a guess. + Exact, + /// The newest transcript filed under the agent's working directory. Sessions that share + /// one directory are indistinguishable here. + Folder, + /// The newest session the host recorded, not scoped to a directory. + Session, +} + pub(super) fn read( host: Host, options: &LastOptions, cwd: &Path, roots: &Roots, pick: usize, -) -> Result<(PathBuf, Vec)> { +) -> Result<(PathBuf, Vec, Discovery)> { match host { Host::ClaudeCode => { let path = find_claude_transcript(options.pid, cwd, roots)?; let messages = readers::claude_messages(&path, pick)?; - Ok((path, messages)) + Ok((path, messages, Discovery::Folder)) } Host::Codex => { #[cfg(target_os = "linux")] if let Some(pid) = options.pid { let path = find_codex_transcript(pid)?; - return readers::explicit(host, &path, None, pick); + let (path, messages) = readers::explicit(host, &path, None, pick)?; + return Ok((path, messages, Discovery::Exact)); } let thread = std::env::var("CODEX_THREAD_ID").ok().filter(|thread| !thread.is_empty()); - readers::codex_thread(&roots.codex_home, thread.as_deref(), pick) + let discovery = if thread.is_some() { Discovery::Exact } else { Discovery::Session }; + let (path, messages) = readers::codex_thread(&roots.codex_home, thread.as_deref(), pick)?; + Ok((path, messages, discovery)) } Host::Copilot => { let path = find_copilot_session(options.pid, cwd, roots)?; let messages = readers::copilot_messages(&path, pick)?; - Ok((path, messages)) + Ok((path, messages, Discovery::Folder)) } Host::Droid => { let path = find_droid_transcript(cwd, roots)?; let messages = readers::droid_messages(&path, pick)?; - Ok((path, messages)) + Ok((path, messages, Discovery::Folder)) } Host::Pi => { let path = find_pi_transcript(cwd, roots, ".pi/agent", "pi")?; let messages = readers::pi_messages(&path, pick)?; - Ok((path, messages)) + Ok((path, messages, Discovery::Folder)) } Host::Omp => bail!( "OMP session discovery without an exact path or id is unsupported; pass --session or --session-id" @@ -173,7 +189,7 @@ fn process_table() -> Vec<(u32, u32)> { .unwrap_or_default() } -fn opencode_for_cwd(cwd: &Path, roots: &Roots, pick: usize) -> Result<(PathBuf, Vec)> { +fn opencode_for_cwd(cwd: &Path, roots: &Roots, pick: usize) -> Result<(PathBuf, Vec, Discovery)> { let databases = roots.opencode_databases(); let mut best: Option<(PathBuf, opencode::Found)> = None; for database in &databases { @@ -187,5 +203,5 @@ fn opencode_for_cwd(cwd: &Path, roots: &Roots, pick: usize) -> Result<(PathBuf, format!("no OpenCode session for {} in {}", cwd.display(), exact::describe(&databases)) })?; let messages = opencode::messages_for_session(&database, &found.id, found.schema, pick)?; - Ok((database, messages)) + Ok((database, messages, Discovery::Folder)) } diff --git a/crates/plannotator-tui/src/last/locate.rs b/crates/plannotator-tui/src/last/locate.rs index f623c15..2f0bb52 100644 --- a/crates/plannotator-tui/src/last/locate.rs +++ b/crates/plannotator-tui/src/last/locate.rs @@ -7,6 +7,7 @@ use anyhow::{Context, Result, bail}; use plannotator_tui_hosts::{Host, HostError, Message, Role, detect_host, sniff}; use plannotator_tui_schema::{DocumentSource, Provenance}; +use super::fallback::Discovery; use super::roots::Roots; use super::{LastOptions, exact, fallback, readers}; @@ -16,6 +17,8 @@ pub(crate) struct Located { pub(crate) transcript: PathBuf, /// Assistant messages, newest first, at most `options.pick`. pub(crate) messages: Vec, + /// How the transcript was chosen; the UI says so when nothing identified it exactly. + pub(crate) discovery: Discovery, } pub(crate) fn locate(options: &LastOptions) -> Result { @@ -28,14 +31,16 @@ pub(crate) fn locate(options: &LastOptions) -> Result { }; let pick = options.pick.max(1); let roots = Roots::from_env(); - let (transcript, messages) = if let Some(path) = session { - readers::explicit(host, &path, options.session_id.as_deref(), pick)? + let (transcript, messages, discovery) = if let Some(path) = session { + let (transcript, messages) = readers::explicit(host, &path, options.session_id.as_deref(), pick)?; + (transcript, messages, Discovery::Exact) } else if let Some(id) = options.session_id.as_deref() { // Validation precedes cwd lookup and every resolver filesystem access. let id = plannotator_tui_hosts::validate_session_id(id)?; let cwd = agent_cwd()?; let exact = exact::resolve(host, id, &cwd, &roots)?; - readers::exact(host, id, exact, pick)? + let (transcript, messages) = readers::exact(host, id, exact, pick)?; + (transcript, messages, Discovery::Exact) } else { let cwd = agent_cwd()?; fallback::read(host, options, &cwd, &roots, pick)? @@ -45,7 +50,7 @@ pub(crate) fn locate(options: &LastOptions) -> Result { if messages.is_empty() { bail!("transcript {} has no assistant messages yet", transcript.display()); } - Ok(Located { host, transcript, messages }) + Ok(Located { host, transcript, messages, discovery }) } /// Was a host named explicitly, by flag or by the launcher? @@ -120,3 +125,48 @@ pub(crate) fn screen_fallback(env: &crate::herdr::context::HerdrEnv) -> Option PathBuf { + let path = dir.join("session.jsonl"); + std::fs::write( + &path, + concat!( + r#"{"parentUuid":null,"isSidechain":false,"type":"user","#, + r#""message":{"role":"user","content":"prompt"},"uuid":"u1","#, + r#""timestamp":"2026-08-28T10:00:00.000Z"}"#, + "\n", + r#"{"parentUuid":"u1","isSidechain":false,"type":"assistant","#, + r#""message":{"role":"assistant","content":[{"type":"text","text":"reply"}]},"#, + r#""uuid":"s1","timestamp":"2026-08-28T10:01:00.000Z"}"#, + "\n", + ), + ) + .expect("transcript"); + path + } + + #[test] + fn a_transcript_named_on_the_command_line_is_never_reported_as_a_guess() { + let dir = std::env::temp_dir().join(format!("plannotator locate ü-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).expect("temp dir"); + let options = LastOptions { + host: Some("claude".to_owned()), + session: Some(transcript(&dir)), + pick: 25, + ..LastOptions::default() + }; + + let located = locate(&options).expect("the named transcript is read"); + + assert_eq!(located.discovery, Discovery::Exact); + assert_eq!(located.messages.first().map(|m| m.text.as_str()), Some("reply")); + std::fs::remove_dir_all(&dir).expect("cleanup"); + } +} diff --git a/crates/plannotator-tui/src/last/mod.rs b/crates/plannotator-tui/src/last/mod.rs index ac57fe7..c2d709e 100644 --- a/crates/plannotator-tui/src/last/mod.rs +++ b/crates/plannotator-tui/src/last/mod.rs @@ -18,6 +18,7 @@ use anyhow::{Context, Result}; use plannotator_tui_hosts::Message; use plannotator_tui_schema::{DocumentSource, Provenance}; +use self::fallback::Discovery; use crate::app::App; use crate::cli; @@ -80,7 +81,28 @@ pub(crate) fn run(options: &LastOptions) -> Result<()> { let label = located.host.label(); let transcript = located.transcript.display().to_string(); let messages = located.messages; - cli::run_ui(|width| App::open_message(label, &transcript, messages, width, cli::delivery(true))) + let note = discovery_note(located.discovery, crate::herdr::context::HerdrEnv::from_env().in_herdr); + cli::run_ui(|width| { + let mut app = App::open_message(label, &transcript, messages, width, cli::delivery(true))?; + if let Some(note) = note { + app.set_status(note); + } + Ok(app) + }) +} + +/// What to say about a transcript nobody identified exactly, and `None` when one did. +/// +/// Inside Herdr the missing session id is the fact worth reporting: Herdr reports one only +/// for sessions that started after its integration was installed, so a session older than +/// the integration lands here even though the integration is present. +fn discovery_note(discovery: Discovery, in_herdr: bool) -> Option { + let shown = match discovery { + Discovery::Exact => return None, + Discovery::Folder => "showing the newest transcript for this folder", + Discovery::Session => "showing the newest session", + }; + Some(if in_herdr { format!("no session id from Herdr, {shown}") } else { shown.to_owned() }) } /// A message as a document: transient, provenance names the host, transcript and message. @@ -96,3 +118,31 @@ pub(crate) fn message_source(host: &str, transcript: &str, message: &Message) -> }, ) } + +#[cfg(test)] +#[allow(clippy::expect_used, reason = "tests assert by panicking")] +mod tests { + use super::*; + + #[test] + fn an_exactly_identified_transcript_is_never_annotated() { + assert_eq!(discovery_note(Discovery::Exact, true), None); + assert_eq!(discovery_note(Discovery::Exact, false), None); + } + + #[test] + fn inside_herdr_a_guessed_transcript_names_the_missing_session_id() { + let note = discovery_note(Discovery::Folder, true).expect("a guess is reported"); + assert_eq!(note, "no session id from Herdr, showing the newest transcript for this folder"); + let note = discovery_note(Discovery::Session, true).expect("a guess is reported"); + assert_eq!(note, "no session id from Herdr, showing the newest session"); + } + + #[test] + fn outside_herdr_a_guessed_transcript_says_what_was_shown_without_naming_herdr() { + let note = discovery_note(Discovery::Folder, false).expect("a guess is reported"); + assert_eq!(note, "showing the newest transcript for this folder"); + assert!(!note.contains("Herdr")); + assert_eq!(discovery_note(Discovery::Session, false).as_deref(), Some("showing the newest session")); + } +}