Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 91 additions & 0 deletions crates/jp_cli/src/cmd/conversation/print_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,97 @@ fn turn_separators_between_turns() {
assert!(output.contains("Second question"), "got: {output}");
}

#[test]
fn turn_header_shows_turn_number_and_relative_time() {
let (mut ctx, id, out, _err, _rt) = setup_ctx(vec![
ConversationEvent::new(TurnStart, ts(0, 0, 0)),
ConversationEvent::new(ChatRequest::from("First question"), ts(0, 0, 1)),
ConversationEvent::new(ChatResponse::message("First answer.\n\n"), ts(0, 0, 2)),
ConversationEvent::new(TurnStart, ts(0, 1, 0)),
ConversationEvent::new(ChatRequest::from("Second question"), ts(0, 1, 1)),
ConversationEvent::new(ChatResponse::message("Second answer.\n\n"), ts(0, 1, 2)),
]);

let print = Print {
target: PositionalIds::from_targets(vec![ConversationTarget::Id(id)]),
last: None,
turn: None,
current_config: false,
style: None,
compacted: false,
};
let h = ctx.workspace.acquire_conversation(&id).unwrap();
print.run(&mut ctx, &[h]).unwrap();
ctx.printer.flush();

let output = strip_ansi(&out.lock());
let lines: Vec<&str> = output.lines().collect();

// The first (user) header of each turn carries the 1-based turn number and
// a relative timestamp.
let user_headers: Vec<&&str> = lines
.iter()
.filter(|l| l.contains("\u{2500}\u{2500} user"))
.collect();
assert_eq!(
user_headers.len(),
2,
"expected one user header per turn, got: {output:?}"
);
assert!(
user_headers[0].contains("turn 1,") && user_headers[0].contains("ago"),
"first turn header should show `turn 1` and a relative time, got: {output:?}"
);
assert!(
user_headers[1].contains("turn 2,") && user_headers[1].contains("ago"),
"second turn header should show `turn 2` and a relative time, got: {output:?}"
);

// The assistant header within a turn is not the first shown header, so it
// carries no turn detail.
for line in lines.iter().filter(|l| l.contains("\u{2500}\u{2500} jp")) {
assert!(
!line.contains("turn "),
"only the first shown header in a turn carries the detail, got: {line:?}"
);
}
}

#[test]
fn turn_header_detail_on_assistant_first_turn() {
// A turn whose first shown event is an assistant response (no user request)
// must still carry the detail on the assistant header. This pins the
// `ensure_assistant_header` consumption path, which
// `turn_header_shows_turn_number_and_relative_time` (user-first) does not.
let (mut ctx, id, out, _err, _rt) = setup_ctx(vec![ConversationEvent::new(
ChatResponse::message("Answer only.\n\n"),
ts(0, 0, 0),
)]);

let print = Print {
target: PositionalIds::from_targets(vec![ConversationTarget::Id(id)]),
last: None,
turn: None,
current_config: false,
style: None,
compacted: false,
};
let h = ctx.workspace.acquire_conversation(&id).unwrap();
print.run(&mut ctx, &[h]).unwrap();
ctx.printer.flush();

let output = strip_ansi(&out.lock());
let jp_headers: Vec<&str> = output
.lines()
.filter(|l| l.contains("\u{2500}\u{2500} jp"))
.collect();
assert_eq!(jp_headers.len(), 1, "got: {output:?}");
assert!(
jp_headers[0].contains("turn 1,") && jp_headers[0].contains("ago"),
"assistant header opening the turn should carry the detail, got: {output:?}"
);
}

#[test]
fn prints_conversation_by_id() {
let (mut ctx, id, out, _err, _rt) = setup_ctx(vec![ConversationEvent::new(
Expand Down
40 changes: 31 additions & 9 deletions crates/jp_cli/src/render/chat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,8 @@ impl ChatRenderer {
/// Draws a single line with the label embedded near the left and an
/// optional dimmed suffix appended after it, with `─` characters filling
/// the remaining width.
/// An optional `detail` is appended dimmed at the right edge, after the
/// fill — e.g. `── alice ──…── turn 2, 12 minutes ago ──`.
/// Used by [`TurnRenderer`] to mark which participant is speaking next —
/// e.g. `── alice ──…` before a user turn, `── jp
/// (anthropic/claude-opus-4-8) ──…` before an assistant turn.
Expand All @@ -158,11 +160,17 @@ impl ChatRenderer {
/// boundaries from any HR markdown the assistant itself emits.
///
/// [`TurnRenderer`]: super::TurnRenderer
pub fn render_role_header(&mut self, label: &str, suffix: Option<&str>) {
pub fn render_role_header(&mut self, label: &str, suffix: Option<&str>, detail: Option<&str>) {
self.flush();

let pretty = self.printer.pretty_printing_enabled();
let line = build_role_header_line(label, suffix, self.config.markdown.wrap_width, pretty);
let line = build_role_header_line(
label,
suffix,
detail,
self.config.markdown.wrap_width,
pretty,
);

self.printer.println("");
self.printer.println(&line);
Expand Down Expand Up @@ -554,18 +562,29 @@ impl ChatRenderer {

/// Build a labeled horizontal rule used as a role-boundary marker.
///
/// Layout: `── <label> [(<suffix>)] ──…` filling `width` columns.
/// In `pretty` mode, the label is bold and the optional suffix is dimmed.
/// Layout: `── <label> [(<suffix>)] ──… [<detail> ──]` filling `width`
/// columns.
/// In `pretty` mode, the label is bold and the optional suffix and detail are
/// dimmed.
/// Plain mode emits the same characters without ANSI styling so it survives
/// ANSI-stripping pipes (e.g.
/// `jp c print | grep`).
fn build_role_header_line(label: &str, suffix: Option<&str>, width: usize, pretty: bool) -> String {
fn build_role_header_line(
label: &str,
suffix: Option<&str>,
detail: Option<&str>,
width: usize,
pretty: bool,
) -> String {
let suffix_part = suffix.map(|s| format!(" ({s})")).unwrap_or_default();
let detail_part = detail.map(|d| format!(" {d} ──")).unwrap_or_default();

// Compute fill against the unstyled width so ANSI escapes don't throw
// off the column count.
let unstyled = format!("── {label}{suffix_part} ");
let fill = width.saturating_sub(unstyled.chars().count()).max(3);
let left = format!("── {label}{suffix_part} ");
let fill = width
.saturating_sub(left.chars().count() + detail_part.chars().count())
.max(3);
let dashes = "─".repeat(fill);

if pretty {
Expand All @@ -575,9 +594,12 @@ fn build_role_header_line(label: &str, suffix: Option<&str>, width: usize, prett
} else {
String::new()
};
format!("── {label_styled}{suffix_styled} {dashes}")
let detail_styled = detail
.map(|d| format!(" {} ──", d.dim()))
.unwrap_or_default();
format!("── {label_styled}{suffix_styled} {dashes}{detail_styled}")
} else {
format!("{unstyled}{dashes}")
format!("{left}{dashes}{detail_part}")
}
}

Expand Down
24 changes: 24 additions & 0 deletions crates/jp_cli/src/render/turn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use std::{collections::HashMap, sync::Arc};

use camino::Utf8PathBuf;
use chrono::Utc;
use jp_config::{
AppConfig, PartialAppConfig,
conversation::tool::{ToolConfigWithDefaults, ToolsConfig, style::ParametersStyle},
Expand Down Expand Up @@ -101,6 +102,8 @@ impl TurnRenderer {
self.reconfigure(partial);
}

self.view.set_turn_detail(turn_detail(turn));

for event_with_cfg in turn {
match &event_with_cfg.event.kind {
EventKind::TurnStart(_) => {
Expand Down Expand Up @@ -203,6 +206,27 @@ impl TurnRenderer {
}
}

/// Build the dimmed right-aligned header detail for a turn: its 1-based number
/// and how long ago it started, e.g. `turn 2, 12 minutes ago`.
///
/// The timestamp comes from the turn's `TurnStart` marker, falling back to the
/// turn's first event when the marker is absent (the implicit leading turn of a
/// legacy stream).
fn turn_detail(turn: &Turn<'_>) -> Option<String> {
let started_at = turn
.iter()
.find(|e| e.event.is_turn_start())
.or_else(|| turn.iter().next())
.map(|e| e.event.timestamp)?;

// A negative duration (a `TurnStart` ahead of now, from clock skew or
// imported data) fails `to_std` and falls back to zero, which `timeago`
// renders as "now" rather than a misleading "... ago".
let elapsed = (Utc::now() - started_at).to_std().unwrap_or_default();
let ago = timeago::Formatter::new().convert(elapsed);
Some(format!("turn {}, {ago}", turn.index() + 1))
}

/// Render a partial model id as a display string, treating a fully-empty id as
/// "no model" rather than the empty string.
///
Expand Down
46 changes: 41 additions & 5 deletions crates/jp_cli/src/render/turn_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,15 @@ pub(crate) struct TurnView {
/// [`Self::ensure_assistant_header`] on first use.
assistant_header_rendered: bool,

/// Dimmed detail (e.g.
/// `turn 2, 12 minutes ago`) to append to the first role header rendered in
/// the current turn.
/// Consumed by [`Self::emit_role_header`], which every header path routes
/// through, so the first header in a turn takes it and later ones don't.
/// Set per turn by replay via [`Self::set_turn_detail`]; left `None` by the
/// live query path.
pending_turn_detail: Option<String>,

/// Shared with the [`ToolRenderer`] (wired via
/// [`Self::set_tool_separator`]): the flag a tool result or custom argument
/// block raises to owe a blank-line separator before the next tool call.
Expand All @@ -75,6 +84,7 @@ impl TurnView {
assistant_name,
model_id,
assistant_header_rendered: false,
pending_turn_detail: None,
tool_separator: Arc::new(AtomicBool::new(false)),
}
}
Expand All @@ -86,6 +96,16 @@ impl TurnView {
self.tool_separator = flag;
}

/// Set the dimmed detail attached to the first role header of the upcoming
/// turn (e.g.
/// `turn 2, 12 minutes ago`).
///
/// Consumed by whichever header — user or assistant — renders first;
/// later headers in the same turn render without it.
pub(crate) fn set_turn_detail(&mut self, detail: Option<String>) {
self.pending_turn_detail = detail;
}

/// Mark the start of a new turn.
/// The next assistant event will emit a fresh role header.
/// Closes any open structured fence so a turn that ended on a
Expand All @@ -105,7 +125,7 @@ impl TurnView {
self.structured.flush();
self.tool_separator.store(false, Ordering::Relaxed);
let label = req.author.as_deref().unwrap_or(DEFAULT_USER_LABEL);
self.chat.render_role_header(label, None);
self.emit_role_header(label, None);
self.chat.render_request(&req.content);
self.assistant_header_rendered = false;
}
Expand Down Expand Up @@ -221,16 +241,32 @@ impl TurnView {
self.model_id = model_id;
}

/// Emit a role-boundary header, attaching the pending turn detail to it.
///
/// The single place that consumes `pending_turn_detail`: the first header
/// rendered in a turn takes the detail, every later header renders without
/// it.
/// Both the user and assistant header paths route through here so the
/// "first header wins" rule lives in one spot instead of being
/// re-implemented at each call site.
fn emit_role_header(&mut self, label: &str, suffix: Option<&str>) {
let detail = self.pending_turn_detail.take();
self.chat
.render_role_header(label, suffix, detail.as_deref());
}

fn ensure_assistant_header(&mut self) {
if self.assistant_header_rendered {
return;
}
// Cloned into owned locals so the `&mut self` call to `emit_role_header`
// doesn't overlap with shared borrows of these fields.
let label = self
.assistant_name
.as_deref()
.unwrap_or(DEFAULT_ASSISTANT_LABEL);
self.chat
.render_role_header(label, self.model_id.as_deref());
.clone()
.unwrap_or_else(|| DEFAULT_ASSISTANT_LABEL.to_owned());
let suffix = self.model_id.clone();
self.emit_role_header(&label, suffix.as_deref());
self.assistant_header_rendered = true;
}
}
Expand Down
Loading