Skip to content
Open
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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 8 additions & 5 deletions crates/jp_cli/src/cmd/query/tool/coordinator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -553,13 +553,14 @@ impl ToolCoordinator {
interactive: bool,
turn_state: &mut TurnState,
tool_renderer: &ToolRenderer,
printer: &Printer,
) -> ToolCallDecision {
// A tool call reached from a reasoning block sits inside that block's
// shading, and a prompt is a visual row like any other (RFD 095). The
// prompter writes through the printer rather than through the
// renderer, so it has to be handed the background rather than finding
// it.
prompter.set_background(tool_renderer.current_region());
// shading, and a prompt is a visual row like any other (RFD 095). This
// is the one place holding both the renderer that owns the region and
// the printer that draws the prompts, and the region is per tool, so
// the read happens here rather than at the tool-call boundary.
printer.set_prompt_background(tool_renderer.current_region());

// Step 1: decide.
let decision = self.decide_permission(executor, interactive, turn_state);
Expand Down Expand Up @@ -886,6 +887,7 @@ impl ToolCoordinator {
interactive: bool,
turn_state: &mut TurnState,
tool_renderer: &ToolRenderer,
printer: &Printer,
) -> (
Vec<(usize, Box<dyn Executor>)>,
Vec<(usize, ToolCallResponse)>,
Expand All @@ -905,6 +907,7 @@ impl ToolCoordinator {
interactive,
turn_state,
tool_renderer,
printer,
)
.await;

Expand Down
9 changes: 8 additions & 1 deletion crates/jp_cli/src/cmd/query/tool/coordinator_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,14 @@ async fn test_resolve_tool_call_decision_invalidates_prerender_on_edit() {
let mut turn_state = TurnState::default();

let decision = coordinator
.resolve_tool_call_decision(executor, &prompter, true, &mut turn_state, &tool_renderer)
.resolve_tool_call_decision(
executor,
&prompter,
true,
&mut turn_state,
&tool_renderer,
&printer,
)
.await;

match decision {
Expand Down
101 changes: 5 additions & 96 deletions crates/jp_cli/src/cmd/query/tool/prompter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,14 @@
//! imperative shell" principle.
//! The `jp_llm` crate remains pure.

use std::{
fmt::Write as _,
io::{self, Write as _},
sync::{Arc, Mutex, PoisonError},
};
use std::{io::Write as _, sync::Arc};

use crossterm::style::Stylize as _;
use jp_config::conversation::tool::{RunMode, ToolSource};
use jp_conversation::event::SelectOption;
use jp_editor::{EditOutcome, EditorBackend};
use jp_inquire::{InlineOption, ReplyEditMode, ReplyOutcome, prompt::PromptBackend};
use jp_llm::tool::executor::PermissionInfo;
use jp_md::{format::DefaultBackground, shade::ShadedWriter};
use jp_printer::{Printer, PromptWriter};
use jp_tool::AnswerType;
use serde_json::Value;
Expand Down Expand Up @@ -91,63 +86,6 @@ pub struct ToolPrompter {
edit_mode: ReplyEditMode,

printer: Arc<Printer>,

/// The reasoning-region background prompts are drawn against.
///
/// Set per tool call by the coordinator, which is the only place that knows
/// both this prompter and the renderer holding the region.
background: Mutex<Option<DefaultBackground>>,
}

/// A prompt writer carrying the reasoning background, when one is open.
///
/// A prompt drawn inside a reasoning block shows that block's background like
/// every other row (RFD 095), and a widget owns its own cursor — it rewrites
/// its line with `\r\x1b[K` on each keystroke, and that erase fills with
/// whatever background is active.
/// [`ShadedWriter`] is what keeps the fill right across the widget's own
/// escapes, including the resets it emits mid-line.
enum PromptCanvas<'a> {
/// No reasoning block is open; writes pass straight through.
Plain(PromptWriter<'a>),

/// Writes are shaded with the open block's background.
Shaded(Box<ShadedWriter<PromptWriter<'a>>>),
}

impl io::Write for PromptCanvas<'_> {
fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
match self {
Self::Plain(writer) => io::Write::write(writer, buf),
Self::Shaded(writer) => {
let text = str::from_utf8(buf).map_err(io::Error::other)?;
writer.write_str(text).map_err(io::Error::other)?;
Ok(buf.len())
}
}
}

/// A widget flushes before it reads a key, and on this path a flush is a
/// barrier rather than a buffer drain: it blocks until the printer's worker
/// has written everything queued ahead of it.
/// Shading decorates the writer and must not swallow that.
fn flush(&mut self) -> io::Result<()> {
match self {
Self::Plain(writer) => io::Write::flush(writer),
Self::Shaded(writer) => io::Write::flush(writer.get_mut()),
}
}
}

impl Drop for PromptCanvas<'_> {
/// Close the background here rather than at the end of each prompt: a
/// prompt can end by cancellation or by an error, and a background left
/// open paints everything printed after it.
fn drop(&mut self) {
if let Self::Shaded(writer) = self {
let _err = writer.finish();
}
}
}

impl ToolPrompter {
Expand All @@ -166,34 +104,6 @@ impl ToolPrompter {
prompt_backend,
edit_mode,
printer,
background: Mutex::new(None),
}
}

/// Draw prompts against `background` until it is replaced.
///
/// `None` while no reasoning block is open, which is the common case.
pub(crate) fn set_background(&self, background: Option<DefaultBackground>) {
*self
.background
.lock()
.unwrap_or_else(PoisonError::into_inner) = background;
}

/// A prompt writer shaded with the open reasoning block, if there is one.
fn canvas(&self) -> PromptCanvas<'_> {
let writer = self.printer.prompt_writer();

match self
.background
.lock()
.unwrap_or_else(PoisonError::into_inner)
.as_ref()
{
Some(background) => {
PromptCanvas::Shaded(Box::new(ShadedWriter::new(writer, background)))
}
None => PromptCanvas::Plain(writer),
}
}

Expand All @@ -205,7 +115,6 @@ impl ToolPrompter {
prompt_backend: Arc<dyn PromptBackend>,
) -> Self {
Self {
background: Mutex::new(None),
editor,
prompt_backend,
edit_mode: ReplyEditMode::Emacs,
Expand Down Expand Up @@ -280,7 +189,7 @@ impl ToolPrompter {

let inline_options = select_options_to_inline(&Self::permission_options());

let mut writer = self.canvas();
let mut writer = self.printer.prompt_writer();

match self
.prompt_backend
Expand Down Expand Up @@ -498,7 +407,7 @@ impl ToolPrompter {
/// - `Ok(true)` if user confirms delivery
/// - `Ok(false)` if user skips delivery
pub fn prompt_result_confirmation(&self, tool_name: &str) -> Result<bool, Error> {
let mut writer = self.canvas();
let mut writer = self.printer.prompt_writer();

let question = format!("Deliver {} result to assistant?", tool_name.yellow().bold());

Expand Down Expand Up @@ -539,7 +448,7 @@ impl ToolPrompter {
/// A `QuestionResult` containing the answer and `persist_level` which
/// indicates whether the answer should be remembered for this turn.
pub fn prompt_question(&self, question: &jp_tool::Question) -> Result<QuestionResult, Error> {
let mut writer = self.canvas();
let mut writer = self.printer.prompt_writer();

if let Some(pre_amble) = &question.pre_amble {
writeln!(writer, "{pre_amble}")?;
Expand Down Expand Up @@ -600,7 +509,7 @@ impl ToolPrompter {
fn prompt_boolean_git_style(
&self,
question: &jp_tool::Question,
writer: &mut PromptCanvas<'_>,
writer: &mut PromptWriter<'_>,
) -> Result<QuestionResult, Error> {
let options = vec![
InlineOption::new('y', "yes, just this once"),
Expand Down
128 changes: 28 additions & 100 deletions crates/jp_cli/src/cmd/query/tool/prompter_tests.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use std::{sync::Arc, time::Duration};
use std::sync::Arc;

use jp_editor::MockEditorBackend;
use jp_inquire::{ReplyOutcome, prompt::MockPromptBackend};
use jp_md::format::BackgroundFill;
use jp_printer::{OutputFormat, PrintableExt as _, SharedBuffer};
use jp_printer::{OutputFormat, SharedBuffer};
use jp_term::background::{BackgroundFill, DefaultBackground};
use serde_json::json;

use super::*;
Expand Down Expand Up @@ -32,65 +32,16 @@ fn terminal_region() -> DefaultBackground {
}

#[test]
fn a_prompt_inside_a_reasoning_block_carries_its_background() {
let (prompter, out) = prompter_with_output(MockPromptBackend::new());
prompter.set_background(Some(terminal_region()));

{
let mut canvas = prompter.canvas();
write!(canvas, "Run local shell tool?").unwrap();
}
prompter.printer.flush();

// The background is asserted before the text and closed once the widget is
// done with the terminal, so the row is shaded and nothing after it is.
assert_eq!(*out.lock(), "\x1b[48;5;236mRun local shell tool?\x1b[49m");
}

#[test]
fn a_prompt_outside_a_reasoning_block_is_unshaded() {
let (prompter, out) = prompter_with_output(MockPromptBackend::new());

{
let mut canvas = prompter.canvas();
write!(canvas, "Run local shell tool?").unwrap();
}
prompter.printer.flush();

assert_eq!(*out.lock(), "Run local shell tool?");
}

#[test]
fn a_cancelled_prompt_still_closes_its_background() {
// A widget can end by `Ctrl+C` or by an error, neither of which returns
// through the normal path. The close lives in `Drop` so the background
// cannot outlive the prompt and paint whatever is printed next.
let (prompter, out) = prompter_with_output(MockPromptBackend::new());
prompter.set_background(Some(terminal_region()));

{
let mut canvas = prompter.canvas();
write!(canvas, "Deliver result?").unwrap();
// No further writes: the prompt is abandoned mid-session.
}
prompter.printer.flush();

let rendered = out.lock().clone();
assert!(
rendered.ends_with("\x1b[49m"),
"an abandoned prompt must still close its background, got {rendered:?}"
);
}

#[test]
fn prompting_a_question_shades_through_the_canvas() {
// The tests above drive the canvas directly, so they hold even if a prompt
// method still reached for a bare prompt writer. This one goes through
// `prompt_question`, whose pre-amble is the one part of a prompt written by
// the prompter rather than by the widget.
fn prompting_a_question_carries_the_reasoning_background() {
// `jp_printer` owns the shading and pins its escapes; what this covers is
// that a real prompt method reaches it, rather than building a writer of
// its own. The pre-amble is the one part of a prompt written by the
// prompter rather than by the widget.
let (prompter, out) =
prompter_with_output(MockPromptBackend::new().with_inline_responses(['y']));
prompter.set_background(Some(terminal_region()));
prompter
.printer
.set_prompt_background(Some(terminal_region()));

let mut question = jp_tool::Question::boolean("confirm", "Proceed?").expect("valid question");
question.pre_amble = Some("About to run a shell command".to_owned());
Expand All @@ -110,53 +61,30 @@ fn prompting_a_question_shades_through_the_canvas() {
}

#[test]
fn flushing_a_shaded_canvas_waits_for_the_printer() {
// A widget flushes its writer before it reads a key, and on this path a
// flush is a barrier rather than a buffer drain: it is what makes the bytes
// have landed before the widget takes the cursor and the terminal's mode.
// Shading is a decoration over that writer and has no business swallowing
// it.
let (prompter, out) = prompter_with_output(MockPromptBackend::new());
prompter.set_background(Some(terminal_region()));

let mut canvas = prompter.canvas();

// Queued after acquisition drained the printer, and slow enough that the
// worker is certainly still inside it: without a real barrier the prompt's
// own text cannot have reached the terminal yet.
fn the_inline_reply_widget_carries_the_reasoning_background() {
// `edit_result` runs the inline reply widget, which owns its output stream
// and so takes `owned_prompt_writer` rather than the borrowed one. It is
// reached by `e` from a permission prompt, inside the same block.
let (prompter, out) = prompter_with_output(
MockPromptBackend::new().with_reply_outcomes([ReplyOutcome::Submit("edited".into())]),
);
prompter
.printer
.print("slow".typewriter(Duration::from_millis(100)));
.set_prompt_background(Some(terminal_region()));

write!(canvas, "Run local shell tool?").unwrap();
canvas.flush().unwrap();
prompter
.edit_result("original")
.expect("the mock submits the edit");
prompter.printer.flush();

// Deliberately no `printer.flush()`, which is the assertion.
let rendered = out.lock().clone();
assert!(
out.lock().contains("Run local shell tool?"),
"flushing the canvas must drain the printer, got {:?}",
*out.lock()
rendered.starts_with("\x1b[48;5;236m"),
"the widget's writes must open under the background, got {rendered:?}"
);
}

#[test]
fn clearing_the_background_unshades_later_prompts() {
let (prompter, out) = prompter_with_output(MockPromptBackend::new());

prompter.set_background(Some(terminal_region()));
drop(prompter.canvas());
prompter.set_background(None);

{
let mut canvas = prompter.canvas();
write!(canvas, "after").unwrap();
}
prompter.printer.flush();

assert!(
out.lock().ends_with("after"),
"a prompt after the block closed carries no background: {:?}",
*out.lock()
rendered.ends_with("\x1b[49m"),
"and close it when the widget gives the terminal back, got {rendered:?}"
);
}

Expand Down
2 changes: 1 addition & 1 deletion crates/jp_cli/src/cmd/query/turn/coordinator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ use jp_llm::{
event::{Event, EventPart, FinishReason},
event_builder::EventBuilder,
};
use jp_md::format::DefaultBackground;
use jp_printer::Printer;
use jp_term::background::DefaultBackground;

use crate::{
cmd::query::{interrupt::InterruptAction, stream::TurnView},
Expand Down
Loading
Loading