diff --git a/apps/staged/src-tauri/src/pikchr_mcp.rs b/apps/staged/src-tauri/src/pikchr_mcp.rs index 41d4c915..47236c63 100644 --- a/apps/staged/src-tauri/src/pikchr_mcp.rs +++ b/apps/staged/src-tauri/src/pikchr_mcp.rs @@ -17,6 +17,10 @@ //! The specialist accepts by ending its turn with the //! [`crate::pikchr_subsession::ACCEPT_SENTINEL`] token, and the host returns //! the slot's contents — so unvalidated source can never reach the caller. +//! While the specialist runs, `generate_pikchr` ticks MCP progress +//! notifications back to its caller (when the request carries a progress +//! token) so client-side idle timers don't abort a call whose run outlasts +//! them. //! //! Fidelity: rendering goes through the `pikchr` crate, which bundles the same //! official `pikchr.c` that the frontend's `pikchr-js` compiles to WASM. The @@ -45,11 +49,15 @@ use acp_client::{McpServer, McpServerHttp}; use axum::Router; use base64::Engine as _; use rmcp::handler::server::{router::tool::ToolRouter, wrapper::Parameters}; -use rmcp::model::{CallToolResult, Content, ServerCapabilities, ServerInfo}; +use rmcp::model::{ + CallToolResult, Content, ProgressNotificationParam, ProgressToken, ServerCapabilities, + ServerInfo, +}; +use rmcp::service::RequestContext; use rmcp::transport::streamable_http_server::{ session::local::LocalSessionManager, StreamableHttpServerConfig, StreamableHttpService, }; -use rmcp::{schemars, tool, tool_handler, tool_router, ErrorData, ServerHandler}; +use rmcp::{schemars, tool, tool_handler, tool_router, ErrorData, Peer, RoleServer, ServerHandler}; use crate::agent::AcpDriver; use crate::pikchr_subsession::{CancelReason, GenOutcome, LastRenderSlot, ACCEPT_SENTINEL}; @@ -59,7 +67,16 @@ use crate::store::{AcpMessageMetadata, CompletionReason, Session, SessionStatus, /// Wall-clock cap for one `generate_pikchr` call. Each call spins a provider /// subprocess and runs several turns; the cap keeps a stuck sub-agent from /// running indefinitely. Enforced by cancelling the sub-session's token. -const GENERATE_PIKCHR_TIMEOUT: Duration = Duration::from_secs(600); +const GENERATE_PIKCHR_TIMEOUT: Duration = Duration::from_secs(1200); + +/// Interval between MCP progress keep-alives sent to the caller while +/// `generate_pikchr` waits on its specialist run. Without them the whole run +/// is one silent request, and MCP clients cut those off long before +/// [`GENERATE_PIKCHR_TIMEOUT`]: Claude Code aborts any tool call that produces +/// no response or progress notification for 300 s, orphaning a worker that +/// then finishes into the void. 30 s keeps a generous margin under that (and +/// any comparable client-side idle timer) at negligible cost. +const PROGRESS_KEEPALIVE_INTERVAL: Duration = Duration::from_secs(30); /// Cap the rasterized PNG so a runaway diagram can't allocate a huge pixmap. const MAX_RENDER_DIMENSION: u32 = 4096; @@ -655,17 +672,13 @@ fn rasterize_tree_to_png(tree: &usvg::Tree, scale: f32) -> Option> { // ============================================================================= /// Outcome of rendering a candidate diagram: the PNG (if rasterization -/// succeeded), a text summary of dimensions and layout warnings, whether the -/// source failed to render at all, and the warning report on its own when -/// renderable geometry still overlaps or extends beyond the diagram bounds. +/// succeeded), a text summary of dimensions and layout warnings, and whether +/// the source failed to render at all. Layout warnings live inside `summary` +/// for the specialist to weigh; they are not carried past acceptance. pub(crate) struct PreviewOutcome { pub(crate) png: Option>, pub(crate) summary: String, pub(crate) is_error: bool, - /// The layout-warning portion of `summary`, when any warnings were - /// detected — carried separately so an accepted render's warnings can be - /// surfaced to the calling agent on their own. - pub(crate) warnings: Option, } /// Render + analyze a candidate Pikchr source. @@ -677,7 +690,6 @@ pub(crate) fn run_preview(source: &str, scale: f32) -> PreviewOutcome { png: None, summary: "Pikchr source is empty — nothing to render.".to_string(), is_error: true, - warnings: None, }; } let rendered = match render_pikchr_svg(source) { @@ -687,7 +699,6 @@ pub(crate) fn run_preview(source: &str, scale: f32) -> PreviewOutcome { png: None, summary: format!("Pikchr could not render this diagram:\n{}", err.trim()), is_error: true, - warnings: None, }; } }; @@ -705,7 +716,6 @@ the rendered SVG could not be parsed.)", rendered.width, rendered.height ), is_error: false, - warnings: None, }; }; @@ -722,7 +732,6 @@ the rendered SVG could not be parsed.)", png, summary, is_error: false, - warnings, } } @@ -757,7 +766,7 @@ struct PikchrToolsHandler { /// child-session announcements are written into it so the UI can link to /// the diagram session while the specialist is still running. parent_session_id: String, - /// Handle used to resolve the bundled Pikchr grammar reference for the + /// Handle used to load the bundled Pikchr grammar text inlined into the /// sub-agent's prompt. app_handle: tauri::AppHandle, /// Shared app store used to persist the child diagram session. @@ -796,13 +805,13 @@ An internal Pikchr specialist writes the diagram, then renders and visually revi session, iterating until it is satisfied. Prefer this over hand-writing Pikchr. Pass a fine-grained \ `description` (boxes, arrows, labels, layout, relationships). To revise an existing diagram, also \ pass its current source as `previous_pikchr` so it is edited rather than redrawn. Returns the \ -validated Pikchr source (drop it into a ```pikchr fenced code block), a filesystem path to a \ -rendered PNG preview you may open as an optional final check, and any layout warnings the \ -specialist deliberately accepted." +validated Pikchr source (drop it into a ```pikchr fenced code block) and a filesystem path to a \ +rendered PNG preview you may open as an optional final check." )] async fn generate_pikchr( &self, Parameters(p): Parameters, + ctx: RequestContext, ) -> Result { let scale = p.scale.unwrap_or(DEFAULT_SCALE); let provider_id = self.provider_id.clone(); @@ -811,10 +820,13 @@ specialist deliberately accepted." let inner_session_id = session.id.clone(); announce_pikchr_child_session(&self.store, &self.parent_session_id, &inner_session_id); let store = Arc::clone(&self.store); - // The sub-session always runs locally, so resolve a local grammar path - // (workspace_name = None). - let grammar_reference = - crate::session_commands::resolve_pikchr_grammar_reference(&self.app_handle, None); + // The full grammar text is inlined into the sub-agent's prompt rather + // than referenced by file path (the sub-session has no repo access). + // Remote note sessions keep referencing an uploaded grammar file — + // that path is resolved separately in `session_commands`. `None` + // (bundled grammar missing or unreadable) makes the prompt fall back + // to naming the public grammar URL. + let grammar = crate::session_commands::bundled_pikchr_grammar_text(&self.app_handle); // Cancellation token owned by *this* future (the parent MCP request). // The worker gets a clone; the parent keeps the token alive through a @@ -918,7 +930,7 @@ accepted a render, so the diagram run was cancelled.", &driver, worker_store, &worker_session_id, - &grammar_reference, + grammar.as_deref(), &p.description, p.previous_pikchr.as_deref(), &slot, @@ -930,8 +942,17 @@ accepted a render, so the diagram run was cancelled.", let _ = tx.send(result); }); - let outcome = rx - .await + // Await the worker while ticking progress keep-alives back to the + // caller so its idle timer doesn't sever a long run. The keep-alive + // loop never completes; the select ends when the worker reports (or + // this future is dropped, which also stops the keep-alives). + let received = tokio::select! { + received = rx => received, + _ = send_progress_keepalives(&ctx.peer, ctx.meta.get_progress_token()) => { + unreachable!("the progress keep-alive loop never completes") + } + }; + let outcome = received .map_err(|e| { let message = format!("generate_pikchr worker dropped: {e}"); mark_pikchr_child_session_error(&store, &inner_session_id, &message); @@ -959,11 +980,53 @@ accepted a render, so the diagram run was cancelled.", &inner_session_id, preview_image_path.as_deref(), &outcome.source, - outcome.warnings.as_deref(), )) } } +/// Build the progress keep-alive notification sent `elapsed_secs` into a +/// `generate_pikchr` run. Progress reports elapsed seconds with no total: +/// monotonically increasing, as the spec asks of an unbounded operation. +fn progress_keepalive( + progress_token: ProgressToken, + elapsed_secs: u64, +) -> ProgressNotificationParam { + ProgressNotificationParam { + progress_token, + progress: elapsed_secs as f64, + total: None, + message: Some(format!( + "Diagram specialist still working ({elapsed_secs}s elapsed)." + )), + } +} + +/// Tick an MCP progress notification to the caller every +/// [`PROGRESS_KEEPALIVE_INTERVAL`] for as long as this future is polled. +/// Never completes — run it under `select!` against the awaited work so it +/// stops when the work does. Progress notifications may only reference a +/// token the caller provided, so when the request carries none this pends +/// forever (rather than returning, which the caller treats as unreachable) +/// and the call proceeds without keep-alives. +async fn send_progress_keepalives(peer: &Peer, progress_token: Option) { + let Some(progress_token) = progress_token else { + return std::future::pending().await; + }; + let started = tokio::time::Instant::now(); + loop { + tokio::time::sleep(PROGRESS_KEEPALIVE_INTERVAL).await; + let elapsed_secs = started.elapsed().as_secs(); + if let Err(e) = peer + .notify_progress(progress_keepalive(progress_token.clone(), elapsed_secs)) + .await + { + // A failed keep-alive usually means the caller is gone; the + // worker result (or this future being dropped) settles the call. + log::debug!("[pikchr_mcp] failed to send generate_pikchr progress keep-alive: {e}"); + } + } +} + /// Cancel reason recorded when the user stops the child diagram session from /// its own UI, distinguishing a deliberate stop from caller abandonment on /// both the cancelled session row and the parent tool error. @@ -1036,7 +1099,6 @@ fn build_generate_pikchr_result( inner_session_id: &str, preview_image_path: Option<&str>, source: &str, - warnings: Option<&str>, ) -> CallToolResult { let mut content = Vec::new(); if let Some(path) = preview_image_path { @@ -1044,13 +1106,6 @@ fn build_generate_pikchr_result( "Rendered preview image path: {path}" ))); } - // The specialist saw these warnings in its render results and accepted - // anyway, so they're deliberate — surface them rather than swallow them. - if let Some(warnings) = warnings { - content.push(Content::text(format!( - "The specialist accepted this render despite layout warnings:\n{warnings}" - ))); - } content.push(Content::text(source.to_string())); let mut structured = serde_json::Map::new(); @@ -1064,12 +1119,6 @@ fn build_generate_pikchr_result( serde_json::Value::String(path.to_string()), ); } - if let Some(warnings) = warnings { - structured.insert( - "renderWarnings".to_string(), - serde_json::Value::String(warnings.to_string()), - ); - } structured.insert( "source".to_string(), serde_json::Value::String(source.to_string()), @@ -1098,8 +1147,8 @@ impl ServerHandler for PikchrToolsHandler { /// string is tolerated — the server still starts and `generate_pikchr` then /// fails per-call rather than failing session startup). `parent_session_id` is /// the session this server is attached to; child diagram sessions are -/// announced into its transcript as they start. `app_handle` resolves the -/// bundled Pikchr grammar reference for the sub-agent. `registry` holds each +/// announced into its transcript as they start. `app_handle` loads the +/// bundled Pikchr grammar text for the sub-agent. `registry` holds each /// child diagram session while it runs so a user Stop reaches its worker. pub async fn start_pikchr_mcp_server( provider_id: String, @@ -1223,7 +1272,6 @@ now would accept that earlier version, not this source." self.slot.store(GenOutcome { source: p.pikchr, png: preview.png, - warnings: preview.warnings, }); Ok(CallToolResult::success(content)) @@ -1336,6 +1384,20 @@ arrow from COLL.e to SNOW.w"#; assert_eq!(persisted.provider.as_deref(), Some("fake-agent")); } + #[test] + fn progress_keepalive_reports_elapsed_seconds_with_no_total() { + let token = ProgressToken(rmcp::model::NumberOrString::Number(7)); + + let notification = progress_keepalive(token.clone(), 90); + + assert_eq!(notification.progress_token, token); + // Elapsed seconds as the progress value keeps successive keep-alives + // monotonically increasing, and an unbounded run reports no total. + assert_eq!(notification.progress, 90.0); + assert_eq!(notification.total, None); + assert!(notification.message.expect("message").contains("90s")); + } + #[tokio::test] async fn forward_user_cancel_records_reason_and_arms_worker_token() { let user_cancel = CancellationToken::new(); @@ -1410,7 +1472,7 @@ arrow from COLL.e to SNOW.w"#; &driver, Arc::clone(&store), &session.id, - "/tmp/grammar.md", + Some("test grammar body"), "a friendly box", None, &slot, @@ -1472,7 +1534,6 @@ arrow from COLL.e to SNOW.w"#; "child-session-1", Some("/tmp/staged-pikchr-preview.png"), "box \"Clean\" fit", - None, ); let texts: Vec = result @@ -1498,45 +1559,6 @@ arrow from COLL.e to SNOW.w"#; "/tmp/staged-pikchr-preview.png" ); assert_eq!(structured["source"], "box \"Clean\" fit"); - assert!( - structured.get("renderWarnings").is_none(), - "no warnings field for a clean render" - ); - } - - #[test] - fn generate_pikchr_result_surfaces_accepted_warnings() { - let result = build_generate_pikchr_result( - "child-session-1", - None, - "box \"Busy\" fit", - Some("⚠ 1 overlapping pair(s) detected"), - ); - - let texts: Vec = result - .content - .iter() - .filter_map(|content| content.as_text().map(|text| text.text.clone())) - .collect(); - assert_eq!( - texts, - vec![ - "The specialist accepted this render despite layout warnings:\n\ -⚠ 1 overlapping pair(s) detected" - .to_string(), - "box \"Busy\" fit".to_string(), - ] - ); - - let structured = result - .structured_content - .as_ref() - .expect("structured content"); - assert_eq!( - structured["renderWarnings"], - "⚠ 1 overlapping pair(s) detected" - ); - assert_eq!(structured["source"], "box \"Busy\" fit"); } /// Render `source` and parse it into a usvg tree the way `run_preview` does, @@ -1689,7 +1711,7 @@ arrow from COLL.e to SNOW.w"#; fn valid_source_produces_png_and_dimensions() { let outcome = run_preview("box \"hello\"", DEFAULT_SCALE); assert!(!outcome.is_error); - assert!(outcome.warnings.is_none()); + assert!(outcome.summary.contains("No layout issues detected")); assert!(outcome.png.is_some(), "expected a PNG for valid source"); assert!(!outcome.png.unwrap().is_empty()); assert!(outcome.summary.contains("px")); @@ -1699,13 +1721,8 @@ arrow from COLL.e to SNOW.w"#; fn overlapping_source_reports_warnings() { let outcome = run_preview(OVERLAPPING_SOURCE, DEFAULT_SCALE); assert!(!outcome.is_error); + assert!(outcome.summary.contains('⚠')); assert!(outcome.summary.contains("overlapping pair")); - let warnings = outcome.warnings.expect("overlaps produce warnings"); - assert!(warnings.contains("overlapping pair")); - assert!( - outcome.summary.contains(&warnings), - "the summary embeds the warning report" - ); } #[test] @@ -1715,9 +1732,8 @@ arrow from COLL.e to SNOW.w"#; // the diagram edges on every host. let outcome = run_preview("margin = -0.2in\nbox \"Out\"", DEFAULT_SCALE); assert!(!outcome.is_error); + assert!(outcome.summary.contains('⚠')); assert!(outcome.summary.contains("beyond the diagram bounds")); - let warnings = outcome.warnings.expect("out-of-bounds produces warnings"); - assert!(warnings.contains("beyond the diagram bounds")); } #[test] @@ -1740,7 +1756,6 @@ arrow from COLL.e to SNOW.w"#; fn malformed_source_reports_error() { let outcome = run_preview("box \"unterminated", DEFAULT_SCALE); assert!(outcome.is_error); - assert!(outcome.warnings.is_none()); assert!(outcome.png.is_none()); assert!(outcome.summary.to_lowercase().contains("pikchr")); } @@ -1749,7 +1764,6 @@ arrow from COLL.e to SNOW.w"#; fn empty_source_reports_error() { let outcome = run_preview(" \n ", DEFAULT_SCALE); assert!(outcome.is_error); - assert!(outcome.warnings.is_none()); assert!(outcome.png.is_none()); } @@ -1879,7 +1893,6 @@ arrow from COLL.e to SNOW.w"#; let stored = slot.take().expect("slot holds the render"); assert_eq!(stored.source, "box \"hello\""); assert!(stored.png.is_some()); - assert!(stored.warnings.is_none()); } #[tokio::test] @@ -1924,7 +1937,7 @@ arrow from COLL.e to SNOW.w"#; } #[tokio::test] - async fn render_tool_stores_warnings_for_overlapping_source() { + async fn render_tool_stores_overlapping_render_despite_warnings() { let slot = Arc::new(LastRenderSlot::new()); let handler = PikchrPreviewHandler::new(DEFAULT_SCALE, Arc::clone(&slot)); @@ -1937,7 +1950,5 @@ arrow from COLL.e to SNOW.w"#; let stored = slot.take().expect("slot holds the flagged render"); assert_eq!(stored.source, OVERLAPPING_SOURCE); - let warnings = stored.warnings.expect("warnings recorded on the render"); - assert!(warnings.contains("overlapping pair")); } } diff --git a/apps/staged/src-tauri/src/pikchr_subsession.rs b/apps/staged/src-tauri/src/pikchr_subsession.rs index 2b648f74..ca02180b 100644 --- a/apps/staged/src-tauri/src/pikchr_subsession.rs +++ b/apps/staged/src-tauri/src/pikchr_subsession.rs @@ -22,6 +22,7 @@ use std::sync::{Arc, Mutex}; use tokio_util::sync::CancellationToken; use crate::agent::{AgentDriver, MessageWriter}; +use crate::session_commands::PIKCHR_GRAMMAR_URL; use crate::store::{CompletionReason, MessageRole, SessionStatus, Store}; /// Total sub-agent turns before giving up. The specialist iterates on the @@ -85,11 +86,6 @@ pub(crate) struct GenOutcome { pub(crate) source: String, /// Rendered PNG preview, if rasterization succeeded. pub(crate) png: Option>, - /// Layout warnings on the accepted render (overlaps, out-of-bounds - /// elements). The specialist saw these in the tool result and accepted - /// anyway, so they're deliberate — surfaced to the caller rather than - /// swallowed. - pub(crate) warnings: Option, } /// The last *successful* render produced through the specialist's @@ -128,7 +124,7 @@ pub(crate) async fn generate_pikchr_source( driver: &D, store: Arc, session_id: &str, - grammar_reference: &str, + grammar: Option<&str>, description: &str, previous_pikchr: Option<&str>, slot: &LastRenderSlot, @@ -139,7 +135,7 @@ pub(crate) async fn generate_pikchr_source( driver, Arc::clone(&store), session_id, - grammar_reference, + grammar, description, previous_pikchr, slot, @@ -228,17 +224,17 @@ async fn generate_pikchr_source_inner( driver: &D, store: Arc, session_id: &str, - grammar_reference: &str, + grammar: Option<&str>, description: &str, previous_pikchr: Option<&str>, slot: &LastRenderSlot, cancel_token: &CancellationToken, ) -> Result { - // The sub-agent needs no repo access; the grammar path is absolute. + // The sub-agent needs no repo access; the grammar is inlined in the prompt. let working_dir = std::env::temp_dir(); let store_dyn: Arc = store.clone(); - let mut prompt = initial_prompt(grammar_reference, description, previous_pikchr); + let mut prompt = initial_prompt(grammar, description, previous_pikchr); let mut agent_session_id: Option = None; for _ in 0..MAX_ATTEMPTS { @@ -355,10 +351,26 @@ fn reply_accepts_last_render(reply: &str) -> bool { // Prompt templates // ============================================================================= -fn initial_prompt(reference: &str, description: &str, previous_pikchr: Option<&str>) -> String { +/// `grammar` is the full grammar text to inline into the message — the +/// sub-session has no repo access, so a file path would be a dead reference. +/// `None` (bundled grammar missing or unreadable) falls back to naming the +/// public grammar URL instead. +fn initial_prompt( + grammar: Option<&str>, + description: &str, + previous_pikchr: Option<&str>, +) -> String { + let grammar_line = match grammar { + Some(_) => { + "Consult the full Pikchr grammar reference included in the `` block below for exact syntax." + .to_string() + } + None => format!( + "The Pikchr grammar reference is at {PIKCHR_GRAMMAR_URL}; consult it for exact syntax." + ), + }; let mut prompt = format!( - "You are a Pikchr diagram specialist. The Pikchr grammar reference is at `{reference}`; \ -consult it for exact syntax.\n\ + "You are a Pikchr diagram specialist. {grammar_line}\n\ \n\ Workflow: draft the diagram source, then call the `render_pikchr` tool with it (no code fences). \ Inspect the returned image and layout analysis, then revise and render again until the diagram \ @@ -367,6 +379,12 @@ matches the request. When you are satisfied, accept the render: your whole messa `{ACCEPT_SENTINEL}` as its own line. The accepted diagram is your last successful render — the \ rest of your reply text is ignored." ); + if let Some(grammar) = grammar { + prompt.push_str(&format!( + "\n\n\n{}\n", + grammar.trim_end() + )); + } if let Some(previous) = previous_pikchr { prompt.push_str(&format!( "\n\nHere is the current diagram to modify:\n```pikchr\n{previous}\n```\n\ @@ -423,7 +441,6 @@ mod tests { GenOutcome { source: source.to_string(), png: Some(vec![1, 2, 3]), - warnings: None, } } @@ -537,7 +554,7 @@ mod tests { driver, Arc::clone(store), session_id, - "/tmp/grammar.md", + Some("test grammar body"), "a friendly box", None, slot, @@ -568,7 +585,6 @@ mod tests { assert_eq!(outcome.source, CLEAN_SOURCE); assert!(outcome.png.is_some()); - assert!(outcome.warnings.is_none()); assert_eq!(*driver.calls.lock().unwrap(), 1); assert!(slot.is_empty(), "acceptance takes the slot"); @@ -681,33 +697,6 @@ mod tests { assert_eq!(*driver.calls.lock().unwrap(), 2); } - #[tokio::test] - async fn accepted_render_keeps_its_warnings() { - let slot = Arc::new(LastRenderSlot::new()); - let mut accepted = render(CLEAN_SOURCE); - accepted.warnings = Some("⚠ 1 overlapping pair(s) detected".to_string()); - let driver = FakeDriver::new( - Arc::clone(&slot), - vec![turn(Some(accepted), ACCEPT_SENTINEL)], - ); - let (store, session_id) = child_session(); - - let outcome = run_generation( - &driver, - &store, - &session_id, - &slot, - &CancellationToken::new(), - ) - .await - .expect("warnings don't block acceptance"); - - assert_eq!( - outcome.warnings.as_deref(), - Some("⚠ 1 overlapping pair(s) detected") - ); - } - #[tokio::test] async fn reprompts_when_accepting_before_any_render() { let slot = Arc::new(LastRenderSlot::new()); @@ -974,8 +963,8 @@ mod tests { #[test] fn initial_prompt_embeds_previous_source_when_revising() { - let prompt = initial_prompt("/tmp/grammar.md", "add a box", Some("box \"old\"")); - assert!(prompt.contains("/tmp/grammar.md")); + let prompt = initial_prompt(Some("GRAMMAR BODY"), "add a box", Some("box \"old\"")); + assert!(prompt.contains("\nGRAMMAR BODY\n")); assert!(prompt.contains("render_pikchr")); assert!(prompt.contains(ACCEPT_SENTINEL)); assert!(prompt.contains("current diagram to modify")); @@ -985,10 +974,17 @@ mod tests { #[test] fn initial_prompt_omits_revision_block_for_fresh_diagram() { - let prompt = initial_prompt("/tmp/grammar.md", "a fresh box", None); + let prompt = initial_prompt(Some("GRAMMAR BODY"), "a fresh box", None); assert!(!prompt.contains("current diagram to modify")); assert!(prompt.contains("render_pikchr")); assert!(prompt.contains(ACCEPT_SENTINEL)); assert!(prompt.contains("a fresh box")); } + + #[test] + fn initial_prompt_falls_back_to_grammar_url_without_bundled_grammar() { + let prompt = initial_prompt(None, "a fresh box", None); + assert!(prompt.contains(PIKCHR_GRAMMAR_URL)); + assert!(!prompt.contains("")); + } } diff --git a/apps/staged/src-tauri/src/session_commands.rs b/apps/staged/src-tauri/src/session_commands.rs index 811a0c1f..dfdcb05b 100644 --- a/apps/staged/src-tauri/src/session_commands.rs +++ b/apps/staged/src-tauri/src/session_commands.rs @@ -132,6 +132,17 @@ fn bundled_pikchr_grammar_bytes(app_handle: &tauri::AppHandle) -> Option } } +/// Full text of the bundled Pikchr grammar, for prompts that inline the +/// grammar rather than referencing it by path (the local `generate_pikchr` +/// sub-session). Remote note sessions keep referencing an uploaded grammar +/// file instead — see [`resolve_pikchr_grammar_reference`]. `None` when the +/// bundled resource is missing or unreadable; callers fall back to pointing +/// at [`PIKCHR_GRAMMAR_URL`]. +pub(crate) fn bundled_pikchr_grammar_text(app_handle: &tauri::AppHandle) -> Option { + bundled_pikchr_grammar_bytes(app_handle) + .map(|bytes| String::from_utf8_lossy(&bytes).into_owned()) +} + fn generated_pikchr_grammar_remote_path() -> String { format!( "{PIKCHR_GRAMMAR_REMOTE_PATH_PREFIX}{}{PIKCHR_GRAMMAR_REMOTE_PATH_SUFFIX}", diff --git a/apps/staged/src/lib/features/sessions/SessionChatPane.svelte b/apps/staged/src/lib/features/sessions/SessionChatPane.svelte index 9fd2b4ae..71c8585a 100644 --- a/apps/staged/src/lib/features/sessions/SessionChatPane.svelte +++ b/apps/staged/src/lib/features/sessions/SessionChatPane.svelte @@ -44,6 +44,7 @@ import ChevronDown from '@lucide/svelte/icons/chevron-down'; import Zap from '@lucide/svelte/icons/zap'; import GitBranch from '@lucide/svelte/icons/git-branch'; + import BookOpen from '@lucide/svelte/icons/book-open'; import FileText from '@lucide/svelte/icons/file-text'; import ExternalLink from '@lucide/svelte/icons/external-link'; import ImagePlus from '@lucide/svelte/icons/image-plus'; @@ -103,7 +104,7 @@ isMaybeTextFile, insertFilePathsAtCursor, } from '../branches/branchCardHelpers'; - import { hasXmlBlocks, sessionEndMessage } from './sessionModalHelpers'; + import { hasXmlBlocks, sessionEndMessage, XML_BLOCK_TAGS } from './sessionModalHelpers'; import { buildAcpTranscriptGroups, diffsFromAcpContent, @@ -1112,7 +1113,7 @@ const segments: ContentSegment[] = []; let remaining = content; - const tagPattern = /<(action|branch-history|launch-context)>([\s\S]*?)<\/\1>/g; + const tagPattern = new RegExp(`<(${XML_BLOCK_TAGS.join('|')})>([\\s\\S]*?)`, 'g'); let lastIndex = 0; let match: RegExpExecArray | null; @@ -1129,8 +1130,10 @@ ? 'Action instructions' : tag === 'branch-history' ? 'Branch history' - : 'Launch context'; - const icon = tag === 'action' ? Zap : GitBranch; + : tag === 'pikchr-grammar' + ? 'Pikchr grammar' + : 'Launch context'; + const icon = tag === 'action' ? Zap : tag === 'pikchr-grammar' ? BookOpen : GitBranch; segments.push({ type: 'xml-block', tag, label, content: match[2].trim(), icon }); lastIndex = match.index + match[0].length; diff --git a/apps/staged/src/lib/features/sessions/sessionModalHelpers.ts b/apps/staged/src/lib/features/sessions/sessionModalHelpers.ts index 1a94d645..51a13af7 100644 --- a/apps/staged/src/lib/features/sessions/sessionModalHelpers.ts +++ b/apps/staged/src/lib/features/sessions/sessionModalHelpers.ts @@ -25,14 +25,21 @@ export function sessionEndMessage(current: Pick): s return 'This session can be resumed.'; } +/** Tags wrapping injected context blocks in prompts/messages (rendered as collapsed cards). */ +export const XML_BLOCK_TAGS = ['action', 'branch-history', 'launch-context', 'pikchr-grammar']; + +const TAG_ALTERNATION = XML_BLOCK_TAGS.join('|'); + /** Pattern matching XML-tagged context blocks embedded in prompts/messages. */ -const XML_BLOCK_PATTERN = /<(action|branch-history|launch-context)>[\s\S]*?<\/\1>/g; +const XML_BLOCK_PATTERN = new RegExp(`<(${TAG_ALTERNATION})>[\\s\\S]*?`, 'g'); + +const XML_OPEN_TAG_PATTERN = new RegExp(`<(${TAG_ALTERNATION})>`); export function hasXmlBlocks(content: string): boolean { - return /<(action|branch-history|launch-context)>/.test(content); + return XML_OPEN_TAG_PATTERN.test(content); } -/** Strip XML-tagged context blocks (action, branch-history, launch-context) from display text. */ +/** Strip XML-tagged context blocks (XML_BLOCK_TAGS) from display text. */ export function stripXmlTags(text: string): string { return text.replace(XML_BLOCK_PATTERN, '').trim(); }