From d5389bbafe2b3576462105094834ce30b86baf42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Orell=20B=C3=BChler?= Date: Sun, 30 Aug 2026 18:12:31 +0200 Subject: [PATCH 1/3] tighten the MCP server MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes, from a read of the whole tool surface: - snap_to_beats returned `timeline()` where every other read uses `working_timeline()`, so inside a task it handed back the user's untouched cut next to a `cuts_aligned` count of moves that were not in it. - timeline_summary promised "any per-track gaps" and returned none. It now reports them, head gap included — a hole is black picture, which an agent never sees for itself. - download_speech_model filled the cache but never selected the model, so naming one and then analyzing still transcribed with the old one. Add set_speech_model, the write side of transcription_status. - core_err mapped everything to internal_error; a stale id or an out-of-range value is the caller's mistake, and invalid_params is what tells a model to fix its arguments instead of giving up. - Clamp the sizes a model picks out of a schema description (waveform buckets, frame widths), the way skim_asset already does. Optimizations: - Build the tool router once. #[tool_handler]'s default router expression is re-evaluated by call_tool, list_tools and get_tool, so every request rebuilt all 85 routes — ~250us of release-build work per tool call for an identical result. - platform_check resolved the cut summary twice (two timeline deserializes, two asset queries) for one answer. --- CLAUDE.md | 23 +++- crates/kerf-app/src/lib.rs | 2 +- crates/kerf-app/src/mcp.rs | 214 +++++++++++++++++++++++++++++++++---- 3 files changed, 218 insertions(+), 21 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 1112277..2f8e2c6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -467,7 +467,13 @@ It is spawned from `lib.rs`'s Tauri `.setup` hook on `tauri::async_runtime` and shares the **same** `Arc>` the Tauri commands hold, so the agent edits the project the user has open. Patterns that matter if you edit it: `#[tool_router]` on the impl + `#[tool_handler]` on `impl ServerHandler` — **no -`tool_router` field on the struct** (the macro calls `Self::tool_router()`). +`tool_router` field on the struct** (the macro would call `Self::tool_router()`). +That default is also the reason for the `router()` `OnceLock`: the generated +`call_tool` / `list_tools` / `get_tool` each *evaluate* the router expression, so +`Self::tool_router()` rebuilds all ~85 routes — a schema lookup, a boxed handler +and a map insert apiece, ~250 µs of release-build work — on **every request**. +The routes are fixed at compile time, so it is built once and +`#[tool_handler(router = router())]` hands out a borrow. `ServerInfo` is `#[non_exhaustive]`, so `get_info` builds it via `Default::default()` then mutates fields — including `server_info` (`server_identity`), because that default is filled from **rmcp's own** crate identity and left alone the server @@ -485,7 +491,20 @@ live in the GUI. Because agent edits **stage**, "live in the GUI" now means the proposal appears for review, not that the cut changes: the read tools (`get_timeline_state`, `timeline_summary`, `preview_timeline`, `export`) go through `working_timeline`, so the agent sees the cut it is building, and -`timeline_summary` carries `staged_changes` so it cannot mistake one for the other. +`timeline_summary` carries `staged_changes` so it cannot mistake one for the other, +and a per-track `gaps` list — a hole between clips (or before the first one) is +black picture, which is the kind of defect an agent has to be *told* about since +it never watches the cut. `core_err` splits the caller's mistakes (a stale id, an +out-of-range value, a stale staged edit) out as `invalid_params`: reported as +`internal_error`, a mistyped uuid reads to a model as a broken server rather than +as something it can fix and retry. Sizes an agent picks out of a schema +description — `get_waveform`/`get_energy` buckets, `get_frame`/`preview_timeline` +widths — are clamped rather than trusted, the way `skim_asset` already clamps its +grid. `set_speech_model` is the write side of `transcription_status` +(`download_speech_model` only fills the cache; transcription uses whichever model +is *selected*, so downloading without selecting was a silent no-op) — it makes +both writes the GUI picker makes, though the picker itself only re-reads at +launch, so a model an agent selects shows there on the next start. `smart_crop` frames each shot for the delivery frame (the server `instructions` pair it with `set_delivery_format`, since reshaping to 9:16 otherwise keeps whatever was in the middle). `generate_captions` / `clear_captions` caption the diff --git a/crates/kerf-app/src/lib.rs b/crates/kerf-app/src/lib.rs index 8984ea2..4125e16 100644 --- a/crates/kerf-app/src/lib.rs +++ b/crates/kerf-app/src/lib.rs @@ -337,7 +337,7 @@ async fn analyze_asset(app: AppHandle, state: State<'_, AppState>, asset_id: Str // ---- speech-to-text ------------------------------------------------------- /// The project-meta key holding the user's speech-model choice. -const SPEECH_MODEL_KEY: &str = "speech_model"; +pub(crate) const SPEECH_MODEL_KEY: &str = "speech_model"; /// Which transcription backend this build will use, and whether its model is /// already downloaded. The transcript tab reads this to explain an empty diff --git a/crates/kerf-app/src/mcp.rs b/crates/kerf-app/src/mcp.rs index 1a27e24..9f09234 100644 --- a/crates/kerf-app/src/mcp.rs +++ b/crates/kerf-app/src/mcp.rs @@ -11,7 +11,7 @@ //! [`EditSource::User`] the same way), so attribution stays correct even though //! both front doors share one `Project`. -use std::sync::{Arc, Mutex, MutexGuard}; +use std::sync::{Arc, Mutex, MutexGuard, OnceLock}; use base64::Engine as _; use kerf_core::{ @@ -31,6 +31,15 @@ use uuid::Uuid; /// Default localhost bind address for the MCP endpoint; override with `KERF_MCP_ADDR`. const DEFAULT_ADDR: &str = "127.0.0.1:7777"; +/// Ceilings on the sizes a tool call may ask for. These numbers reach us from a +/// model reading a schema description, not from a UI slider, so they are +/// clamped rather than trusted — `skim_asset` already clamps its grid the same +/// way. A frame far wider than the model can resolve costs a decode and a +/// base64 payload nobody benefits from, and a waveform of a million buckets is +/// megabytes of JSON that would bury the answer it was fetched to support. +const MAX_PREVIEW_WIDTH: u32 = 1920; +const MAX_WAVEFORM_BUCKETS: usize = 4096; + #[derive(Clone)] pub struct KerfMcp { project: Arc>, @@ -674,7 +683,7 @@ struct TaskIdParams { struct WaveformParams { #[schemars(description = "UUID of the asset")] asset_id: String, - #[schemars(description = "Number of peak-magnitude buckets to return")] + #[schemars(description = "Number of buckets to return (1–4096; a few hundred is plenty to read a shape from)")] buckets: usize, } @@ -684,7 +693,7 @@ struct FrameParams { asset_id: String, #[schemars(description = "Time in the source asset to decode (seconds)")] time_secs: f64, - #[schemars(description = "Maximum output width in pixels (default 640)")] + #[schemars(description = "Maximum output width in pixels (default 640, capped at 1920)")] max_width: Option, } @@ -708,7 +717,7 @@ struct SkimParams { struct TimelineFrameParams { #[schemars(description = "Timeline position to render (seconds)")] time_secs: f64, - #[schemars(description = "Maximum output width in pixels (default 640)")] + #[schemars(description = "Maximum output width in pixels (default 640, capped at 1920)")] max_width: Option, } @@ -726,6 +735,13 @@ struct AssetMetadata { analysis: Option, } +/// A span of a track where nothing renders. +#[derive(Serialize)] +struct Gap { + start_secs: f64, + end_secs: f64, +} + #[derive(Serialize)] struct TrackSummary { id: String, @@ -733,6 +749,11 @@ struct TrackSummary { kind: String, clip_count: usize, duration_secs: f64, + /// Where this track renders nothing — black picture on a video track, + /// silence on an audio one — including a hole at the head when the first + /// clip doesn't start at 0. Omitted entirely when the track is gapless. + #[serde(skip_serializing_if = "Vec::is_empty")] + gaps: Vec, } #[derive(Serialize)] @@ -794,7 +815,7 @@ impl KerfMcp { } #[tool( - description = "Download a speech-to-text model (whisper.cpp ggml) into Kerf's cache so the next analyze_asset transcribes without waiting for it. Names, smallest first: tiny, tiny.en, base, base.en, small, small.en, medium, medium.en, large-v3-turbo — the plain names are multilingual, `.en` ones are English-only but more accurate on English. Bigger is slower and more accurate; `base` is the default. This blocks for the length of the download (75 MB to 1.6 GB) and is a no-op if the model is already cached." + description = "Download a speech-to-text model (whisper.cpp ggml) into Kerf's cache, so the next analyze_asset does not wait for it. Names, smallest first: tiny, tiny.en, base, base.en, small, small.en, medium, medium.en, large-v3-turbo — the plain names are multilingual, `.en` ones are English-only but more accurate on English. Bigger is slower and more accurate; `base` is the default. This only fills the cache: transcription keeps using whichever model is *selected*, so to transcribe with a model you name here, call set_speech_model too. Blocks for the length of the download (75 MB to 1.6 GB) and is a no-op if the model is already cached." )] async fn download_speech_model(&self, Parameters(p): Parameters) -> Result { let name = p.name.unwrap_or_else(|| kerf_core::DEFAULT_SPEECH_MODEL.to_string()); @@ -824,6 +845,24 @@ impl KerfMcp { json(&analysis) } + #[tool( + description = "Pick the speech-to-text model transcription uses, remembered in the project (omit the name to \ + go back to the default). Same names as download_speech_model. Selecting a model does not \ + download it — the next transcription fetches it if the cache is cold, or call \ + download_speech_model first to get that wait out of the way. Returns the resulting \ + transcription status." + )] + fn set_speech_model(&self, Parameters(p): Parameters) -> Result { + let name = p.name.as_deref().map(str::trim).filter(|s| !s.is_empty()); + kerf_core::set_speech_model(name); + // Both writes the GUI's picker makes: the process setting transcription + // reads, and the project meta that restores the choice on reopen. + self.lock() + .set_meta(crate::SPEECH_MODEL_KEY, name.unwrap_or("")) + .map_err(core_err)?; + json(&kerf_core::transcription_status()) + } + #[tool(description = "Cut [start, end) of an asset and append it to the matching track")] fn cut_clip(&self, Parameters(p): Parameters) -> Result { let id = parse_id(&p.asset_id)?; @@ -1461,7 +1500,11 @@ impl KerfMcp { let track = p.track_id.as_deref().map(parse_id).transpose()?; let project = self.lock(); let aligned = project.snap_to_beats(track, p.tolerance).map_err(core_err)?; - let timeline = project.timeline().map_err(core_err)?; + // The *working* timeline, like every other read here: inside a task the + // alignment lands in the staged proposal, and `timeline()` would hand + // back the user's untouched cut — a timeline with none of the moves the + // `cuts_aligned` count beside it just reported. + let timeline = project.working_timeline().map_err(core_err)?; drop(project); self.changed(); json(&serde_json::json!({ "cuts_aligned": aligned, "timeline": timeline })) @@ -1618,8 +1661,11 @@ impl KerfMcp { letterboxed) and tips. Advisory: export is never blocked. Use before reporting a cut finished." )] fn platform_check(&self) -> Result { - let project = self.lock(); - let summary = project.cut_summary(None).map_err(core_err)?; + // One summary, not two: `Project::platform_check` resolves its own, and + // each resolve deserializes the whole timeline and re-reads every asset + // row to answer the identical question. + let summary = self.lock().cut_summary(None).map_err(core_err)?; + let targets = kerf_core::platform::check_all(&summary); json(&serde_json::json!({ "cut": { "duration_secs": summary.duration, @@ -1627,7 +1673,7 @@ impl KerfMcp { "has_audio": summary.has_audio, "has_text": summary.has_text, }, - "targets": project.platform_check(None).map_err(core_err)?, + "targets": targets, })) } @@ -1710,13 +1756,14 @@ impl KerfMcp { #[tool(description = "Get peak-magnitude waveform data (0.0–1.0) for an asset's first audio stream")] async fn get_waveform(&self, Parameters(p): Parameters) -> Result { let id = parse_id(&p.asset_id)?; + let count = p.buckets.clamp(1, MAX_WAVEFORM_BUCKETS); let project = self.project.clone(); let buckets = blocking(move || { // Resolve under the lock, decode the whole audio stream with it // released — bucketing a long source takes seconds and must not // stall the GUI's commands on the shared mutex. let asset = lock_agent(&project).require_asset(id).map_err(core_err)?; - Project::decode_waveform(&asset, p.buckets).map_err(core_err) + Project::decode_waveform(&asset, count).map_err(core_err) }) .await?; json(&buckets) @@ -1727,11 +1774,12 @@ impl KerfMcp { )] async fn get_energy(&self, Parameters(p): Parameters) -> Result { let id = parse_id(&p.asset_id)?; + let count = p.buckets.clamp(1, MAX_WAVEFORM_BUCKETS); let project = self.project.clone(); let energy = blocking(move || { // Same lock-free decode shape as `get_waveform`. let asset = lock_agent(&project).require_asset(id).map_err(core_err)?; - Project::decode_energy(&asset, p.buckets).map_err(core_err) + Project::decode_energy(&asset, count).map_err(core_err) }) .await?; json(&energy) @@ -1743,7 +1791,7 @@ impl KerfMcp { async fn get_frame(&self, Parameters(p): Parameters) -> Result { let id = parse_id(&p.asset_id)?; let project = self.project.clone(); - let (time_secs, max_width) = (p.time_secs, p.max_width.unwrap_or(640)); + let (time_secs, max_width) = (p.time_secs, p.max_width.unwrap_or(640).clamp(64, MAX_PREVIEW_WIDTH)); let jpeg = blocking(move || { // Resolve under the lock, decode with it released — mirrors the GUI's // `get_frame` so an agent drill-in can't stall the user's edits. @@ -1789,7 +1837,7 @@ impl KerfMcp { )] async fn preview_timeline(&self, Parameters(p): Parameters) -> Result { let project = self.project.clone(); - let (time_secs, max_width) = (p.time_secs, p.max_width.unwrap_or(640)); + let (time_secs, max_width) = (p.time_secs, p.max_width.unwrap_or(640).clamp(64, MAX_PREVIEW_WIDTH)); let jpeg = blocking(move || { // Snapshot the inputs under the lock, composite with it released — // mirrors the GUI's `get_timeline_frame`. @@ -1816,6 +1864,7 @@ impl KerfMcp { kind: format!("{:?}", t.kind).to_lowercase(), clip_count: t.clips.len(), duration_secs: t.end(), + gaps: track_gaps(t), }) .collect(); let total_clip_count = tracks.iter().map(|t| t.clip_count).sum(); @@ -1872,6 +1921,19 @@ async fn blocking(job: impl FnOnce() -> Result + .map_err(|e| McpError::internal_error(e.to_string(), None))? } +/// The tool router, built once for the life of the process. +/// +/// `#[tool_handler]`'s default router expression is `Self::tool_router()`, and +/// the generated `call_tool`, `list_tools` and `get_tool` each evaluate it — so +/// every request rebuilds all ~85 routes: a schema-cache lookup, a boxed +/// handler and a map insert apiece, ~250 µs of release-build work per tool call +/// that produces the identical router every time. The routes are fixed at +/// compile time, so build them once and hand out a borrow. +fn router() -> &'static rmcp::handler::server::router::tool::ToolRouter { + static ROUTER: OnceLock> = OnceLock::new(); + ROUTER.get_or_init(KerfMcp::tool_router) +} + /// How the server introduces itself to clients. `ServerInfo::default()` fills /// `server_info` in from *rmcp's* own build env, so an untouched default has /// every client listing this server as "rmcp". @@ -1879,7 +1941,7 @@ fn server_identity() -> Implementation { Implementation::new("kerf", env!("CARGO_PKG_VERSION")) } -#[tool_handler] +#[tool_handler(router = router())] impl ServerHandler for KerfMcp { fn get_info(&self) -> ServerInfo { let mut info = ServerInfo::default(); @@ -2077,8 +2139,29 @@ fn parse_transition(kind: Option, duration: Option) -> Result McpError { - McpError::internal_error(e.to_string(), None) + use kerf_core::Error as E; + match e { + E::AssetNotFound(_) + | E::ClipNotFound(_) + | E::TrackNotFound(_) + | E::OverlayNotFound(_) + | E::RevisionNotFound(_) + | E::TaskNotFound(_) + | E::InvalidArgument(_) + | E::NoStagedEdit + | E::StagedEditPending + | E::StagedEditStale => McpError::invalid_params(e.to_string(), None), + // A database, io, ffmpeg or engine failure is ours, not the caller's. + other => McpError::internal_error(other.to_string(), None), + } } /// Wrap JPEG bytes as an MCP tool result the model can *see*: a caption text @@ -2089,6 +2172,29 @@ fn image_result(caption: String, jpeg: Vec) -> CallToolResult { CallToolResult::success(vec![ContentBlock::text(caption), ContentBlock::image(b64, "image/jpeg")]) } +/// The spans of a track that render nothing: the hole before the first clip and +/// every hole between clips. Clips are not held in start order, so this walks a +/// sorted copy of their spans, and it tolerates overlap by carrying the furthest +/// end reached rather than the previous clip's. A hole under a millisecond is +/// float noise from a retrim, not a gap. +fn track_gaps(track: &kerf_core::Track) -> Vec { + const EPSILON: f64 = 1e-3; + let mut spans: Vec<(f64, f64)> = track.clips.iter().map(|c| (c.timeline_start, c.timeline_end())).collect(); + spans.sort_by(|a, b| a.0.total_cmp(&b.0)); + let mut gaps = Vec::new(); + let mut cursor = 0.0f64; + for (start, end) in spans { + if start - cursor > EPSILON { + gaps.push(Gap { + start_secs: cursor, + end_secs: start, + }); + } + cursor = cursor.max(end); + } + gaps +} + /// Format a seconds offset as `mm:ss.mmm` for frame / contact-sheet captions. /// Rounds to milliseconds *before* splitting so a value just under a minute /// carries into the minute (59.9999 → `01:00.000`, not `00:60.000`). @@ -2105,7 +2211,7 @@ fn json(value: &T) -> Result { #[cfg(test)] mod tests { - use super::{allowed_hosts, fmt_ts, image_result, server_identity, KerfMcp}; + use super::{allowed_hosts, core_err, fmt_ts, image_result, router, server_identity, track_gaps}; #[test] fn fmt_ts_carries_at_minute_boundaries() { @@ -2125,7 +2231,7 @@ mod tests { /// surface is generated. #[test] fn every_tool_has_a_description_and_object_schema() { - let tools = KerfMcp::tool_router().list_all(); + let tools = router().list_all(); assert!(tools.len() > 50, "expected the full tool surface, got {}", tools.len()); for tool in &tools { @@ -2145,7 +2251,7 @@ mod tests { /// requiring them would break that contract without failing to compile. #[test] fn optional_parameters_are_not_required() { - let tools = KerfMcp::tool_router().list_all(); + let tools = router().list_all(); let add_clip = tools .iter() .find(|t| t.name == "add_clip_to_timeline") @@ -2256,4 +2362,76 @@ mod tests { assert_eq!(value["isError"], false); } + + /// `timeline_summary` promises the agent "any per-track gaps", and a gap is + /// a real defect in a cut — the picture goes black there. The head of the + /// track counts: a cut that opens two seconds late opens on black. + #[test] + fn track_gaps_finds_holes_including_the_one_at_the_head() { + use kerf_core::{Clip, StreamKind, Track}; + let asset = uuid::Uuid::new_v4(); + let mut track = Track::new(StreamKind::Video, "V1"); + // Deliberately out of start order — clips are not stored sorted. + track.clips.push(Clip::new(asset, 0.0, 1.0, 6.0)); + track.clips.push(Clip::new(asset, 0.0, 2.0, 2.0)); + + let gaps = track_gaps(&track); + assert_eq!(gaps.len(), 2, "head gap + the hole between the clips"); + assert_eq!((gaps[0].start_secs, gaps[0].end_secs), (0.0, 2.0)); + assert_eq!((gaps[1].start_secs, gaps[1].end_secs), (4.0, 6.0)); + } + + /// A gapless track reports nothing, and float noise from a retrim is not a + /// gap — otherwise every summary would be a wall of sub-millisecond holes. + #[test] + fn track_gaps_ignores_a_gapless_track_and_float_noise() { + use kerf_core::{Clip, StreamKind, Track}; + let asset = uuid::Uuid::new_v4(); + let mut track = Track::new(StreamKind::Video, "V1"); + track.clips.push(Clip::new(asset, 0.0, 2.0, 0.0)); + track.clips.push(Clip::new(asset, 0.0, 2.0, 2.000_04)); + assert!( + track_gaps(&track).is_empty(), + "40 microseconds of drift is not a gap, got {:?}", + track_gaps(&track).len() + ); + + // An overlap must not be read as a gap by the clip that follows it. + let mut overlapping = Track::new(StreamKind::Video, "V1"); + overlapping.clips.push(Clip::new(asset, 0.0, 5.0, 0.0)); + overlapping.clips.push(Clip::new(asset, 0.0, 1.0, 1.0)); + assert!(track_gaps(&overlapping).is_empty()); + } + + /// A stale id or an out-of-range value is the caller's mistake, and the + /// model can only act on that if it is told so: `invalid_params` means "fix + /// the arguments", `internal_error` means "the server is broken". + #[test] + fn caller_mistakes_are_reported_as_invalid_params() { + use rmcp::model::ErrorCode; + let id = uuid::Uuid::new_v4(); + for e in [ + kerf_core::Error::ClipNotFound(id), + kerf_core::Error::AssetNotFound(id), + kerf_core::Error::TrackNotFound(id), + kerf_core::Error::InvalidArgument("marker time must be >= 0".to_string()), + kerf_core::Error::StagedEditStale, + ] { + let rendered = e.to_string(); + assert_eq!(core_err(e).code, ErrorCode::INVALID_PARAMS, "{rendered}"); + } + // Ours, not theirs. + assert_eq!( + core_err(kerf_core::Error::Engine("ffmpeg exited 1".to_string())).code, + ErrorCode::INTERNAL_ERROR + ); + } + + /// The router is fixed at compile time but the `#[tool_handler]` default + /// rebuilds it per request. This pins that it is built once — a regression + /// here is invisible except as latency on every single tool call. + #[test] + fn the_tool_router_is_built_once() { + assert!(std::ptr::eq(router(), router())); + } } From 53ad658b8f7f7fe495e163c190d98420a0d66742 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Orell=20B=C3=BChler?= Date: Sun, 30 Aug 2026 19:48:12 +0200 Subject: [PATCH 2/3] close the gaps the MCP review left open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Release the project lock before emitting `project-changed`. The event makes the GUI re-fetch and that re-fetch takes the same lock, so the notify belonged outside. An `edit()` helper does the whole shape once and takes the lock/mutate/notify boilerplate out of 61 tools. - export now reports progress and can be cancelled. A render runs for minutes; an agent could neither tell a slow export from a hung one nor abandon settings it already knew were wrong. Both ride the protocol — the client's `progressToken` and the cancellation token rmcp trips on `notifications/cancelled` — and a cancelled render deletes its half-written file from inside the blocking job, since cancelling the request can drop the handler future. - Add import_asset, so an agent can load media rather than only rearranging what it was handed. It deliberately does not stage: a file on disk is not an edit to the user's cut. - set_mask no longer swallows a failed timeline read. `.ok()` made a read that failed indistinguishable from "this clip has no mask yet", and silently reset every field the caller did not name. - set_speech_model emits `speech-model-changed`, which the webview listens for. The GUI reads transcription status only at launch, so a model an agent picked stayed invisible in the picker until restart. --- CLAUDE.md | 22 +- crates/kerf-app/src/lib.rs | 26 +- crates/kerf-app/src/mcp.rs | 804 +++++++++++++++++++------------ frontend/src/routes/+page.svelte | 4 + 4 files changed, 527 insertions(+), 329 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 2f8e2c6..8524152 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -485,9 +485,14 @@ parts) and `preview_timeline` (the composited cut at a timeline time) — return `Content::text` plus a `Content::image(bare_base64, "image/jpeg")` block the LLM can actually *see* (rmcp wants bare base64 + MIME, **not** a `data:` URL). The `lock()` helper sets `EditSource::Agent` per-op under the shared lock (the GUI's `project()` -helper sets `User` the same way); every **mutating** tool calls `self.changed()`, which -emits a `project-changed` Tauri event so the webview re-fetches and the edit shows up -live in the GUI. Because agent edits **stage**, "live in the GUI" now means the +helper sets `User` the same way); every **mutating** tool goes through the `edit()` +helper, which runs the op under the lock, **releases it**, and only then emits a +`project-changed` Tauri event so the webview re-fetches and the edit shows up +live in the GUI — that order matters, because the re-fetch the event triggers +takes the same lock. `set_speech_model` emits `speech-model-changed` instead, +which the webview listens for to re-read the transcription status: it reads that +once at launch, and `project-changed` would re-fetch the timeline, history and +task queue, none of which moved. Because agent edits **stage**, "live in the GUI" now means the proposal appears for review, not that the cut changes: the read tools (`get_timeline_state`, `timeline_summary`, `preview_timeline`, `export`) go through `working_timeline`, so the agent sees the cut it is building, and @@ -515,6 +520,17 @@ say to caption **last** and to re-run after any further edit, because captions are placed in timeline time and a later trim moves the words out from under them — which an agent has no way to infer from the tool list. +`import_asset` is the one write that does **not** stage — a file on disk is not +an edit to the user's cut, so imported media (and its background proxy) lands for +them immediately, reporting on the same `import-progress` event a lens-pair +stitch drives for the GUI. `export` takes rmcp's `RequestContext` beside its +`Parameters`: a render runs for minutes, so it forwards ffmpeg's progress to the +client's `progressToken` and passes `context.ct` as the cancel callback, deleting +the half-written file on cancel the way the GUI's export does. Progress goes +through an unbounded channel to a spawned forwarder because the render itself is +on the blocking pool and `notify_progress` is async; the forwarder drains the +channel even with no token, so a client that asked for no progress doesn't leave +ticks piling up. `platform_check` tells it whether the cut is publishable where it is going (and the server `instructions` tell it to run that before reporting a cut finished — an agent that assembles a four-minute Reel has done the work and lost diff --git a/crates/kerf-app/src/lib.rs b/crates/kerf-app/src/lib.rs index 4125e16..56d5e87 100644 --- a/crates/kerf-app/src/lib.rs +++ b/crates/kerf-app/src/lib.rs @@ -192,13 +192,27 @@ async fn save_project_as(state: State<'_, AppState>, path: String) -> CmdResult< /// Progress of a slow import (an Insta360 lens pair being stitched), tagged with /// the file the user picked so the UI can label it while several import at once. #[derive(Clone, serde::Serialize)] -struct ImportProgress { +pub(crate) struct ImportProgress { path: String, fraction: f64, elapsed_secs: f64, eta_secs: Option, } +impl ImportProgress { + /// Tag a render-progress tick with the file it belongs to. Shared with the + /// MCP `import_asset` tool so an agent's import reports on the same event + /// and drives the same overlay the user's own import does. + pub(crate) fn new(path: &str, p: kerf_core::ExportProgress) -> Self { + Self { + path: path.to_string(), + fraction: p.fraction, + elapsed_secs: p.elapsed_secs, + eta_secs: p.eta_secs, + } + } +} + #[tauri::command] async fn import_asset(app: AppHandle, state: State<'_, AppState>, path: String) -> CmdResult { let shared = state.project.clone(); @@ -207,15 +221,7 @@ async fn import_asset(app: AppHandle, state: State<'_, AppState>, path: String) // parallel imports really run in parallel and a multi-minute stitch never // freezes the GUI or the agent — then take it only for the quick insert. let mut on_progress = |p: kerf_core::ExportProgress| { - let _ = app.emit( - "import-progress", - ImportProgress { - path: path.clone(), - fraction: p.fraction, - elapsed_secs: p.elapsed_secs, - eta_secs: p.eta_secs, - }, - ); + let _ = app.emit("import-progress", ImportProgress::new(&path, p)); }; let asset = Project::probe_import(std::path::Path::new(&path), &mut on_progress).map_err(|e| e.to_string())?; // Importing the pair's other lens (or the same file twice) resolves to diff --git a/crates/kerf-app/src/mcp.rs b/crates/kerf-app/src/mcp.rs index 9f09234..0a9fdbc 100644 --- a/crates/kerf-app/src/mcp.rs +++ b/crates/kerf-app/src/mcp.rs @@ -19,7 +19,8 @@ use kerf_core::{ Projection, ReframeKeyframe, StreamKind, TextKeyframe, Transition, TransitionKind, VideoEffect, }; use rmcp::handler::server::wrapper::Parameters; -use rmcp::model::{CallToolResult, ContentBlock, Implementation, ServerCapabilities, ServerInfo}; +use rmcp::model::{CallToolResult, ContentBlock, Implementation, ProgressNotificationParam, ServerCapabilities, ServerInfo}; +use rmcp::service::{RequestContext, RoleServer}; use rmcp::transport::streamable_http_server::{ session::local::LocalSessionManager, StreamableHttpServerConfig, StreamableHttpService, }; @@ -70,6 +71,12 @@ struct RevisionDiffParams { from: Option, } +#[derive(Debug, serde::Deserialize, schemars::JsonSchema)] +struct ImportParams { + #[schemars(description = "Absolute path to the media file to import")] + path: String, +} + #[derive(Debug, serde::Deserialize, schemars::JsonSchema)] struct AssetIdParams { #[schemars(description = "UUID of the asset")] @@ -784,6 +791,36 @@ impl KerfMcp { json(&kerf_core::list_system_fonts()) } + #[tool( + description = "Import a media file into the project and return the Asset (probed streams, duration, \ + resolution, fps, codecs). Importing a file the project already holds resolves to that \ + asset instead of duplicating it, so this is safe to re-run. One Insta360 lens file pulls \ + in its sibling and stitches the pair into a single 360 asset — a full re-encode that can \ + run for minutes. An import is NOT part of your staged proposal: media lands for the user \ + immediately, because a file on disk is not an edit to their cut." + )] + async fn import_asset(&self, Parameters(p): Parameters) -> Result { + let project = self.project.clone(); + let app = self.app.clone(); + let path = p.path; + let asset = blocking(move || { + // Probe (and, for a lens pair, stitch) with the lock released, taking + // it only for the quick insert — the same shape as the GUI's import, + // so a multi-minute stitch never freezes the user's editing. + let mut on_progress = |pr: kerf_core::ExportProgress| { + let _ = app.emit("import-progress", crate::ImportProgress::new(&path, pr)); + }; + let probed = Project::probe_import(std::path::Path::new(&path), &mut on_progress).map_err(core_err)?; + lock_agent(&project).insert_or_get_asset(&probed).map_err(core_err) + }) + .await?; + // Preview decodes come off a proxy; queue it now so the first frame + // anyone asks for is not a seek into a long GOP. + crate::spawn_proxy(&self.app, &asset); + self.changed(); + json(&asset) + } + #[tool(description = "List all media assets in the project")] fn list_assets(&self) -> Result { let project = self.lock(); @@ -860,58 +897,63 @@ impl KerfMcp { self.lock() .set_meta(crate::SPEECH_MODEL_KEY, name.unwrap_or("")) .map_err(core_err)?; + // The GUI reads the transcription status once, at launch, so it has to be + // told the choice moved or its picker shows the old model until the next + // start. Deliberately not `project-changed`: that re-fetches the timeline, + // the history and the task queue, and none of those moved. + self.notify("speech-model-changed"); json(&kerf_core::transcription_status()) } #[tool(description = "Cut [start, end) of an asset and append it to the matching track")] fn cut_clip(&self, Parameters(p): Parameters) -> Result { let id = parse_id(&p.asset_id)?; - let project = self.lock(); - let out = project.cut_clip(id, p.start, p.end).map_err(core_err)?; - self.changed(); - json(&out) + self.edit(|project| { + let out = project.cut_clip(id, p.start, p.end).map_err(core_err)?; + json(&out) + }) } #[tool(description = "Add a clip referencing a source range of an asset to the timeline")] fn add_clip_to_timeline(&self, Parameters(p): Parameters) -> Result { let asset_id = parse_id(&p.asset_id)?; let track_id = p.track_id.as_deref().map(parse_id).transpose()?; - let project = self.lock(); - let out = project - .add_clip_to_timeline(asset_id, track_id, p.source_in, p.source_out, p.timeline_start) - .map_err(core_err)?; - self.changed(); - json(&out) + self.edit(|project| { + let out = project + .add_clip_to_timeline(asset_id, track_id, p.source_in, p.source_out, p.timeline_start) + .map_err(core_err)?; + json(&out) + }) } #[tool(description = "Split a timeline clip at a timeline time into two adjacent clips")] fn split_at(&self, Parameters(p): Parameters) -> Result { let clip_id = parse_id(&p.clip_id)?; - let project = self.lock(); - let (left, right) = project.split_at(clip_id, p.at).map_err(core_err)?; - self.changed(); - json(&serde_json::json!({ "left": left, "right": right })) + self.edit(|project| { + let (left, right) = project.split_at(clip_id, p.at).map_err(core_err)?; + json(&serde_json::json!({ "left": left, "right": right })) + }) } #[tool(description = "Trim a clip's source in/out points (timeline position preserved unless timeline_start is passed)")] fn trim(&self, Parameters(p): Parameters) -> Result { let clip_id = parse_id(&p.clip_id)?; - let project = self.lock(); - let out = project - .trim(clip_id, p.source_in, p.source_out, p.timeline_start) - .map_err(core_err)?; - self.changed(); - json(&out) + self.edit(|project| { + let out = project + .trim(clip_id, p.source_in, p.source_out, p.timeline_start) + .map_err(core_err)?; + json(&out) + }) } #[tool(description = "Move a clip to a new index within its track (re-flows the track gaplessly)")] fn reorder(&self, Parameters(p): Parameters) -> Result { let track_id = parse_id(&p.track_id)?; let clip_id = parse_id(&p.clip_id)?; - let project = self.lock(); - project.reorder(track_id, clip_id, p.new_index).map_err(core_err)?; - self.changed(); - Ok("ok".to_string()) + self.edit(|project| { + project.reorder(track_id, clip_id, p.new_index).map_err(core_err)?; + Ok("ok".to_string()) + }) } #[tool( @@ -920,10 +962,10 @@ impl KerfMcp { fn move_clip(&self, Parameters(p): Parameters) -> Result { let clip_id = parse_id(&p.clip_id)?; let track_id = p.track_id.as_deref().map(parse_id).transpose()?; - let project = self.lock(); - let out = project.move_clip(clip_id, p.timeline_start, track_id).map_err(core_err)?; - self.changed(); - json(&out) + self.edit(|project| { + let out = project.move_clip(clip_id, p.timeline_start, track_id).map_err(core_err)?; + json(&out) + }) } #[tool( @@ -931,10 +973,10 @@ impl KerfMcp { )] fn ripple_delete(&self, Parameters(p): Parameters) -> Result { let clip_id = parse_id(&p.clip_id)?; - let project = self.lock(); - project.ripple_delete(clip_id).map_err(core_err)?; - self.changed(); - Ok("ok".to_string()) + self.edit(|project| { + project.ripple_delete(clip_id).map_err(core_err)?; + Ok("ok".to_string()) + }) } #[tool( @@ -944,10 +986,10 @@ impl KerfMcp { )] fn cut_clip_range(&self, Parameters(p): Parameters) -> Result { let clip_id = parse_id(&p.clip_id)?; - let project = self.lock(); - let pieces = project.cut_clip_range(clip_id, p.from, p.to).map_err(core_err)?; - self.changed(); - json(&pieces) + self.edit(|project| { + let pieces = project.cut_clip_range(clip_id, p.from, p.to).map_err(core_err)?; + json(&pieces) + }) } #[tool( @@ -955,19 +997,19 @@ impl KerfMcp { )] fn add_track(&self, Parameters(p): Parameters) -> Result { let kind = parse_kind(&p.kind)?; - let project = self.lock(); - let track = project.add_track(kind, p.name).map_err(core_err)?; - self.changed(); - json(&track) + self.edit(|project| { + let track = project.add_track(kind, p.name).map_err(core_err)?; + json(&track) + }) } #[tool(description = "Remove a track and all of its clips (refuses to remove the last track)")] fn remove_track(&self, Parameters(p): Parameters) -> Result { let track_id = parse_id(&p.track_id)?; - let project = self.lock(); - project.remove_track(track_id).map_err(core_err)?; - self.changed(); - Ok("ok".to_string()) + self.edit(|project| { + project.remove_track(track_id).map_err(core_err)?; + Ok("ok".to_string()) + }) } #[tool( @@ -976,10 +1018,10 @@ impl KerfMcp { )] fn set_track_duck(&self, Parameters(p): Parameters) -> Result { let track_id = parse_id(&p.track_id)?; - let project = self.lock(); - let track = project.set_track_duck(track_id, p.duck).map_err(core_err)?; - self.changed(); - json(&track) + self.edit(|project| { + let track = project.set_track_duck(track_id, p.duck).map_err(core_err)?; + json(&track) + }) } #[tool(description = "Cut a clip to a shape: inside the shape the clip is kept, outside it goes \ @@ -993,35 +1035,39 @@ impl KerfMcp { the right thing.")] fn set_mask(&self, Parameters(p): Parameters) -> Result { let clip_id = parse_id(&p.clip_id)?; - let project = self.lock(); - let mask = match p.shape { - None => None, - Some(ref s) => { - let shape = MaskShape::parse(s).ok_or_else(|| { - McpError::invalid_params(format!("invalid mask shape '{s}'; expected \"rect\" or \"ellipse\""), None) - })?; - // Omitted fields keep the clip's current mask, so nudging one - // number (move the ellipse a little left) does not reset the - // size and feather back to the defaults out from under it. - let d = project - .working_timeline() - .ok() - .and_then(|tl| tl.locate(clip_id).and_then(|(ti, ci)| tl.tracks[ti].clips[ci].mask)) - .unwrap_or_default(); - Some(Mask { - shape, - x: p.x.unwrap_or(d.x), - y: p.y.unwrap_or(d.y), - width: p.width.unwrap_or(d.width), - height: p.height.unwrap_or(d.height), - feather: p.feather.unwrap_or(d.feather), - inverted: p.inverted.unwrap_or(d.inverted), - }) - } - }; - let clip = project.set_mask(clip_id, mask).map_err(core_err)?; - self.changed(); - json(&clip) + self.edit(|project| { + let mask = match p.shape { + None => None, + Some(ref s) => { + let shape = MaskShape::parse(s).ok_or_else(|| { + McpError::invalid_params(format!("invalid mask shape '{s}'; expected \"rect\" or \"ellipse\""), None) + })?; + // Omitted fields keep the clip's current mask, so nudging one + // number (move the ellipse a little left) does not reset the + // size and feather back to the defaults out from under it. + // Deliberately not `.ok()…unwrap_or_default()`: a read that + // failed is not the same answer as "this clip has no mask + // yet", and collapsing the two silently resets every field + // the caller did not name. + let timeline = project.working_timeline().map_err(core_err)?; + let (ti, ci) = timeline + .locate(clip_id) + .ok_or_else(|| core_err(kerf_core::Error::ClipNotFound(clip_id)))?; + let d = timeline.tracks[ti].clips[ci].mask.unwrap_or_default(); + Some(Mask { + shape, + x: p.x.unwrap_or(d.x), + y: p.y.unwrap_or(d.y), + width: p.width.unwrap_or(d.width), + height: p.height.unwrap_or(d.height), + feather: p.feather.unwrap_or(d.feather), + inverted: p.inverted.unwrap_or(d.inverted), + }) + } + }; + let clip = project.set_mask(clip_id, mask).map_err(core_err)?; + json(&clip) + }) } #[tool( @@ -1033,10 +1079,10 @@ impl KerfMcp { )] fn set_track_volume(&self, Parameters(p): Parameters) -> Result { let track_id = parse_id(&p.track_id)?; - let project = self.lock(); - let track = project.set_track_volume(track_id, p.volume).map_err(core_err)?; - self.changed(); - json(&track) + self.edit(|project| { + let track = project.set_track_volume(track_id, p.volume).map_err(core_err)?; + json(&track) + }) } #[tool( @@ -1047,10 +1093,10 @@ impl KerfMcp { )] fn set_track_pan(&self, Parameters(p): Parameters) -> Result { let track_id = parse_id(&p.track_id)?; - let project = self.lock(); - let track = project.set_track_pan(track_id, p.pan).map_err(core_err)?; - self.changed(); - json(&track) + self.edit(|project| { + let track = project.set_track_pan(track_id, p.pan).map_err(core_err)?; + json(&track) + }) } #[tool( @@ -1067,10 +1113,10 @@ impl KerfMcp { (Some(w), Some(h)) => Some(Delivery::new(w, h, p.fit.unwrap_or(Fit::Cover))), _ => None, }; - let project = self.lock(); - let timeline = project.set_delivery_format(format).map_err(core_err)?; - self.changed(); - json(&timeline) + self.edit(|project| { + let timeline = project.set_delivery_format(format).map_err(core_err)?; + json(&timeline) + }) } #[tool( @@ -1079,28 +1125,28 @@ impl KerfMcp { the user can see and jump to. Good for reporting findings from skim_asset." )] fn add_marker(&self, Parameters(p): Parameters) -> Result { - let project = self.lock(); - let marker = project.add_marker(p.time, p.name, p.color).map_err(core_err)?; - self.changed(); - json(&marker) + self.edit(|project| { + let marker = project.add_marker(p.time, p.name, p.color).map_err(core_err)?; + json(&marker) + }) } #[tool(description = "Move, rename or recolor a marker; omitted fields are left alone")] fn update_marker(&self, Parameters(p): Parameters) -> Result { let marker_id = parse_id(&p.marker_id)?; - let project = self.lock(); - let marker = project.update_marker(marker_id, p.time, p.name, p.color).map_err(core_err)?; - self.changed(); - json(&marker) + self.edit(|project| { + let marker = project.update_marker(marker_id, p.time, p.name, p.color).map_err(core_err)?; + json(&marker) + }) } #[tool(description = "Remove a marker from the timeline")] fn remove_marker(&self, Parameters(p): Parameters) -> Result { let marker_id = parse_id(&p.marker_id)?; - let project = self.lock(); - project.remove_marker(marker_id).map_err(core_err)?; - self.changed(); - Ok("ok".to_string()) + self.edit(|project| { + project.remove_marker(marker_id).map_err(core_err)?; + Ok("ok".to_string()) + }) } #[tool( @@ -1110,10 +1156,10 @@ impl KerfMcp { )] fn set_track_muted(&self, Parameters(p): Parameters) -> Result { let track_id = parse_id(&p.track_id)?; - let project = self.lock(); - let track = project.set_track_muted(track_id, p.muted).map_err(core_err)?; - self.changed(); - json(&track) + self.edit(|project| { + let track = project.set_track_muted(track_id, p.muted).map_err(core_err)?; + json(&track) + }) } #[tool( @@ -1123,10 +1169,10 @@ impl KerfMcp { )] fn set_track_solo(&self, Parameters(p): Parameters) -> Result { let track_id = parse_id(&p.track_id)?; - let project = self.lock(); - let track = project.set_track_solo(track_id, p.solo).map_err(core_err)?; - self.changed(); - json(&track) + self.edit(|project| { + let track = project.set_track_solo(track_id, p.solo).map_err(core_err)?; + json(&track) + }) } #[tool( @@ -1135,10 +1181,10 @@ impl KerfMcp { )] fn set_track_locked(&self, Parameters(p): Parameters) -> Result { let track_id = parse_id(&p.track_id)?; - let project = self.lock(); - let track = project.set_track_locked(track_id, p.locked).map_err(core_err)?; - self.changed(); - json(&track) + self.edit(|project| { + let track = project.set_track_locked(track_id, p.locked).map_err(core_err)?; + json(&track) + }) } #[tool( @@ -1147,10 +1193,10 @@ impl KerfMcp { )] fn set_clip_enabled(&self, Parameters(p): Parameters) -> Result { let clip_id = parse_id(&p.clip_id)?; - let project = self.lock(); - let clip = project.set_clip_enabled(clip_id, p.enabled).map_err(core_err)?; - self.changed(); - json(&clip) + self.edit(|project| { + let clip = project.set_clip_enabled(clip_id, p.enabled).map_err(core_err)?; + json(&clip) + }) } #[tool( @@ -1162,37 +1208,37 @@ impl KerfMcp { )] fn duplicate_clips(&self, Parameters(p): Parameters) -> Result { let ids = p.clip_ids.iter().map(|s| parse_id(s)).collect::, _>>()?; - let project = self.lock(); - let clips = project.duplicate_clips(&ids, p.at).map_err(core_err)?; - self.changed(); - json(&clips) + self.edit(|project| { + let clips = project.duplicate_clips(&ids, p.at).map_err(core_err)?; + json(&clips) + }) } #[tool(description = "Remove a clip from the timeline")] fn remove(&self, Parameters(p): Parameters) -> Result { let clip_id = parse_id(&p.clip_id)?; - let project = self.lock(); - project.remove(clip_id).map_err(core_err)?; - self.changed(); - Ok("ok".to_string()) + self.edit(|project| { + project.remove(clip_id).map_err(core_err)?; + Ok("ok".to_string()) + }) } #[tool(description = "Set the linear volume gain of a clip")] fn set_volume(&self, Parameters(p): Parameters) -> Result { let clip_id = parse_id(&p.clip_id)?; - let project = self.lock(); - let out = project.set_volume(clip_id, p.volume).map_err(core_err)?; - self.changed(); - json(&out) + self.edit(|project| { + let out = project.set_volume(clip_id, p.volume).map_err(core_err)?; + json(&out) + }) } #[tool(description = "Set a clip's fade-in / fade-out duration in seconds (omit a field to leave it unchanged, 0 to clear)")] fn set_fade(&self, Parameters(p): Parameters) -> Result { let clip_id = parse_id(&p.clip_id)?; - let project = self.lock(); - let out = project.set_fade(clip_id, p.fade_in, p.fade_out).map_err(core_err)?; - self.changed(); - json(&out) + self.edit(|project| { + let out = project.set_fade(clip_id, p.fade_in, p.fade_out).map_err(core_err)?; + json(&out) + }) } #[tool( @@ -1200,10 +1246,10 @@ impl KerfMcp { )] fn set_speed(&self, Parameters(p): Parameters) -> Result { let clip_id = parse_id(&p.clip_id)?; - let project = self.lock(); - let out = project.set_speed(clip_id, p.speed).map_err(core_err)?; - self.changed(); - json(&out) + self.edit(|project| { + let out = project.set_speed(clip_id, p.speed).map_err(core_err)?; + json(&out) + }) } #[tool( @@ -1211,23 +1257,23 @@ impl KerfMcp { )] fn set_transform(&self, Parameters(p): Parameters) -> Result { let clip_id = parse_id(&p.clip_id)?; - let project = self.lock(); - let out = project - .set_transform( - clip_id, - p.scale, - p.pos_x, - p.pos_y, - p.rotation, - p.opacity, - p.crop_left, - p.crop_right, - p.crop_top, - p.crop_bottom, - ) - .map_err(core_err)?; - self.changed(); - json(&out) + self.edit(|project| { + let out = project + .set_transform( + clip_id, + p.scale, + p.pos_x, + p.pos_y, + p.rotation, + p.opacity, + p.crop_left, + p.crop_right, + p.crop_top, + p.crop_bottom, + ) + .map_err(core_err)?; + json(&out) + }) } #[tool( @@ -1235,12 +1281,12 @@ impl KerfMcp { )] fn set_color(&self, Parameters(p): Parameters) -> Result { let clip_id = parse_id(&p.clip_id)?; - let project = self.lock(); - let out = project - .set_color(clip_id, p.brightness, p.contrast, p.saturation, p.gamma, p.temperature) - .map_err(core_err)?; - self.changed(); - json(&out) + self.edit(|project| { + let out = project + .set_color(clip_id, p.brightness, p.contrast, p.saturation, p.gamma, p.temperature) + .map_err(core_err)?; + json(&out) + }) } #[tool( @@ -1249,10 +1295,10 @@ impl KerfMcp { fn set_transition(&self, Parameters(p): Parameters) -> Result { let clip_id = parse_id(&p.clip_id)?; let transition = parse_transition(p.kind, p.duration)?; - let project = self.lock(); - let out = project.set_transition(clip_id, transition).map_err(core_err)?; - self.changed(); - json(&out) + self.edit(|project| { + let out = project.set_transition(clip_id, transition).map_err(core_err)?; + json(&out) + }) } #[tool( @@ -1260,10 +1306,10 @@ impl KerfMcp { )] fn set_video_effects(&self, Parameters(p): Parameters) -> Result { let clip_id = parse_id(&p.clip_id)?; - let project = self.lock(); - let out = project.set_video_effects(clip_id, p.effects).map_err(core_err)?; - self.changed(); - json(&out) + self.edit(|project| { + let out = project.set_video_effects(clip_id, p.effects).map_err(core_err)?; + json(&out) + }) } #[tool( @@ -1271,10 +1317,10 @@ impl KerfMcp { )] fn set_audio_effects(&self, Parameters(p): Parameters) -> Result { let clip_id = parse_id(&p.clip_id)?; - let project = self.lock(); - let out = project.set_audio_effects(clip_id, p.effects).map_err(core_err)?; - self.changed(); - json(&out) + self.edit(|project| { + let out = project.set_audio_effects(clip_id, p.effects).map_err(core_err)?; + json(&out) + }) } #[tool( @@ -1282,10 +1328,10 @@ impl KerfMcp { )] fn set_keyframes(&self, Parameters(p): Parameters) -> Result { let clip_id = parse_id(&p.clip_id)?; - let project = self.lock(); - let out = project.set_keyframes(clip_id, p.keyframes).map_err(core_err)?; - self.changed(); - json(&out) + self.edit(|project| { + let out = project.set_keyframes(clip_id, p.keyframes).map_err(core_err)?; + json(&out) + }) } #[tool( @@ -1293,21 +1339,21 @@ impl KerfMcp { )] fn add_keyframe(&self, Parameters(p): Parameters) -> Result { let clip_id = parse_id(&p.clip_id)?; - let project = self.lock(); - let out = project - .add_keyframe(clip_id, p.time, p.scale, p.pos_x, p.pos_y, p.rotation, p.opacity) - .map_err(core_err)?; - self.changed(); - json(&out) + self.edit(|project| { + let out = project + .add_keyframe(clip_id, p.time, p.scale, p.pos_x, p.pos_y, p.rotation, p.opacity) + .map_err(core_err)?; + json(&out) + }) } #[tool(description = "Remove all transform keyframes from a clip (back to its static transform)")] fn clear_keyframes(&self, Parameters(p): Parameters) -> Result { let clip_id = parse_id(&p.clip_id)?; - let project = self.lock(); - let out = project.clear_keyframes(clip_id).map_err(core_err)?; - self.changed(); - json(&out) + self.edit(|project| { + let out = project.clear_keyframes(clip_id).map_err(core_err)?; + json(&out) + }) } #[tool( @@ -1318,12 +1364,12 @@ impl KerfMcp { )] fn set_reframe(&self, Parameters(p): Parameters) -> Result { let clip_id = parse_id(&p.clip_id)?; - let project = self.lock(); - let out = project - .set_reframe(clip_id, p.yaw, p.pitch, p.roll, p.fov, p.lens_fov, p.input, p.output) - .map_err(core_err)?; - self.changed(); - json(&out) + self.edit(|project| { + let out = project + .set_reframe(clip_id, p.yaw, p.pitch, p.roll, p.fov, p.lens_fov, p.input, p.output) + .map_err(core_err)?; + json(&out) + }) } #[tool( @@ -1346,10 +1392,10 @@ impl KerfMcp { )] fn clear_reframe(&self, Parameters(p): Parameters) -> Result { let clip_id = parse_id(&p.clip_id)?; - let project = self.lock(); - let out = project.clear_reframe(clip_id).map_err(core_err)?; - self.changed(); - json(&out) + self.edit(|project| { + let out = project.clear_reframe(clip_id).map_err(core_err)?; + json(&out) + }) } #[tool( @@ -1359,10 +1405,10 @@ impl KerfMcp { )] fn set_reframe_keyframes(&self, Parameters(p): Parameters) -> Result { let clip_id = parse_id(&p.clip_id)?; - let project = self.lock(); - let out = project.set_reframe_keyframes(clip_id, p.keyframes).map_err(core_err)?; - self.changed(); - json(&out) + self.edit(|project| { + let out = project.set_reframe_keyframes(clip_id, p.keyframes).map_err(core_err)?; + json(&out) + }) } #[tool( @@ -1371,22 +1417,22 @@ impl KerfMcp { )] fn add_reframe_keyframe(&self, Parameters(p): Parameters) -> Result { let clip_id = parse_id(&p.clip_id)?; - let project = self.lock(); - let out = project - .add_reframe_keyframe(clip_id, p.time, p.yaw, p.pitch, p.roll, p.fov) - .map_err(core_err)?; - self.changed(); - json(&out) + self.edit(|project| { + let out = project + .add_reframe_keyframe(clip_id, p.time, p.yaw, p.pitch, p.roll, p.fov) + .map_err(core_err)?; + json(&out) + }) } #[tool( description = "Add a text overlay (title / lower-third / caption / watermark) drawn over the composited picture between start and end (timeline seconds). Returns the overlay; style or animate it with update_overlay / set_overlay_keyframes." )] fn add_overlay(&self, Parameters(p): Parameters) -> Result { - let project = self.lock(); - let out = project.add_overlay(p.text, p.start, p.end).map_err(core_err)?; - self.changed(); - json(&out) + self.edit(|project| { + let out = project.add_overlay(p.text, p.start, p.end).map_err(core_err)?; + json(&out) + }) } #[tool( @@ -1394,23 +1440,23 @@ impl KerfMcp { )] fn update_overlay(&self, Parameters(p): Parameters) -> Result { let overlay_id = parse_id(&p.overlay_id)?; - let project = self.lock(); - let out = project - .update_overlay( - overlay_id, p.text, p.start, p.end, p.pos_x, p.pos_y, p.size, p.color, p.bg, p.font, p.bold, - ) - .map_err(core_err)?; - self.changed(); - json(&out) + self.edit(|project| { + let out = project + .update_overlay( + overlay_id, p.text, p.start, p.end, p.pos_x, p.pos_y, p.size, p.color, p.bg, p.font, p.bold, + ) + .map_err(core_err)?; + json(&out) + }) } #[tool(description = "Remove a text overlay")] fn remove_overlay(&self, Parameters(p): Parameters) -> Result { let overlay_id = parse_id(&p.overlay_id)?; - let project = self.lock(); - project.remove_overlay(overlay_id).map_err(core_err)?; - self.changed(); - Ok("ok".to_string()) + self.edit(|project| { + project.remove_overlay(overlay_id).map_err(core_err)?; + Ok("ok".to_string()) + }) } #[tool( @@ -1418,10 +1464,10 @@ impl KerfMcp { )] fn set_overlay_keyframes(&self, Parameters(p): Parameters) -> Result { let overlay_id = parse_id(&p.overlay_id)?; - let project = self.lock(); - let out = project.set_overlay_keyframes(overlay_id, p.keyframes).map_err(core_err)?; - self.changed(); - json(&out) + self.edit(|project| { + let out = project.set_overlay_keyframes(overlay_id, p.keyframes).map_err(core_err)?; + json(&out) + }) } #[tool( @@ -1435,20 +1481,20 @@ impl KerfMcp { pos_y: p.pos_y, size: p.size, }; - let project = self.lock(); - let out = project.generate_captions(opts).map_err(core_err)?; - self.changed(); - json(&out) + self.edit(|project| { + let out = project.generate_captions(opts).map_err(core_err)?; + json(&out) + }) } #[tool( description = "Remove the captions generate_captions wrote, leaving hand-made titles and lower-thirds alone. Returns how many were removed." )] fn clear_captions(&self) -> Result { - let project = self.lock(); - let removed = project.clear_captions().map_err(core_err)?; - self.changed(); - Ok(format!("removed {removed} generated caption(s)")) + self.edit(|project| { + let removed = project.clear_captions().map_err(core_err)?; + Ok(format!("removed {removed} generated caption(s)")) + }) } #[tool(description = "Write an asset's cached transcript to a SubRip (.srt) subtitle file (run analyze_asset first)")] @@ -1465,10 +1511,10 @@ impl KerfMcp { #[tool(description = "Append the non-silent spans of an asset as clips, using cached analysis")] fn remove_silence(&self, Parameters(p): Parameters) -> Result { let id = parse_id(&p.asset_id)?; - let project = self.lock(); - let out = project.remove_silence(id).map_err(core_err)?; - self.changed(); - json(&out) + self.edit(|project| { + let out = project.remove_silence(id).map_err(core_err)?; + json(&out) + }) } #[tool( @@ -1498,34 +1544,33 @@ impl KerfMcp { )] fn snap_to_beats(&self, Parameters(p): Parameters) -> Result { let track = p.track_id.as_deref().map(parse_id).transpose()?; - let project = self.lock(); - let aligned = project.snap_to_beats(track, p.tolerance).map_err(core_err)?; - // The *working* timeline, like every other read here: inside a task the - // alignment lands in the staged proposal, and `timeline()` would hand - // back the user's untouched cut — a timeline with none of the moves the - // `cuts_aligned` count beside it just reported. - let timeline = project.working_timeline().map_err(core_err)?; - drop(project); - self.changed(); - json(&serde_json::json!({ "cuts_aligned": aligned, "timeline": timeline })) + self.edit(|project| { + let aligned = project.snap_to_beats(track, p.tolerance).map_err(core_err)?; + // The *working* timeline, like every other read here: inside a task the + // alignment lands in the staged proposal, and `timeline()` would hand + // back the user's untouched cut — a timeline with none of the moves the + // `cuts_aligned` count beside it just reported. + let timeline = project.working_timeline().map_err(core_err)?; + json(&serde_json::json!({ "cuts_aligned": aligned, "timeline": timeline })) + }) } #[tool(description = "Append the full audio of an asset to the first audio track")] fn extract_audio(&self, Parameters(p): Parameters) -> Result { let id = parse_id(&p.asset_id)?; - let project = self.lock(); - let out = project.extract_audio(id).map_err(core_err)?; - self.changed(); - json(&out) + self.edit(|project| { + let out = project.extract_audio(id).map_err(core_err)?; + json(&out) + }) } #[tool(description = "Stitch the full length of several assets together in order")] fn concatenate(&self, Parameters(p): Parameters) -> Result { let ids = p.asset_ids.iter().map(|s| parse_id(s)).collect::, _>>()?; - let project = self.lock(); - let out = project.concatenate(&ids).map_err(core_err)?; - self.changed(); - json(&out) + self.edit(|project| { + let out = project.concatenate(&ids).map_err(core_err)?; + json(&out) + }) } #[tool( @@ -1535,10 +1580,10 @@ impl KerfMcp { before handing it over. claim_next_task already does this for you." )] fn stage_edits(&self, Parameters(p): Parameters) -> Result { - let project = self.lock(); - let staged = project.begin_staging(None, p.note.as_deref()).map_err(core_err)?; - self.changed(); - json(&staged) + self.edit(|project| { + let staged = project.begin_staging(None, p.note.as_deref()).map_err(core_err)?; + json(&staged) + }) } #[tool( @@ -1562,18 +1607,18 @@ impl KerfMcp { the timeline since you staged, unless `force`." )] fn apply_staged_edits(&self, Parameters(p): Parameters) -> Result { - let project = self.lock(); - let timeline = project.apply_staged(p.force.unwrap_or(false)).map_err(core_err)?; - self.changed(); - json(&timeline) + self.edit(|project| { + let timeline = project.apply_staged(p.force.unwrap_or(false)).map_err(core_err)?; + json(&timeline) + }) } #[tool(description = "Throw your staged edits away, leaving the user's timeline untouched")] fn discard_staged_edits(&self) -> Result { - let project = self.lock(); - let timeline = project.discard_staged().map_err(core_err)?; - self.changed(); - json(&timeline) + self.edit(|project| { + let timeline = project.discard_staged().map_err(core_err)?; + json(&timeline) + }) } #[tool(description = "Explain what one revision changed (see history), as a list of edits to the cut")] @@ -1595,26 +1640,26 @@ impl KerfMcp { #[tool(description = "Undo the last timeline edit, returning the restored timeline")] fn undo(&self) -> Result { - let project = self.lock(); - let out = project.undo().map_err(core_err)?; - self.changed(); - json(&out) + self.edit(|project| { + let out = project.undo().map_err(core_err)?; + json(&out) + }) } #[tool(description = "Redo the next timeline edit, returning the restored timeline")] fn redo(&self) -> Result { - let project = self.lock(); - let out = project.redo().map_err(core_err)?; - self.changed(); - json(&out) + self.edit(|project| { + let out = project.redo().map_err(core_err)?; + json(&out) + }) } #[tool(description = "Revert the timeline to a specific revision seq (see history), returning the restored timeline")] fn revert_to(&self, Parameters(p): Parameters) -> Result { - let project = self.lock(); - let out = project.revert_to(p.seq).map_err(core_err)?; - self.changed(); - json(&out) + self.edit(|project| { + let out = project.revert_to(p.seq).map_err(core_err)?; + json(&out) + }) } #[tool( @@ -1630,12 +1675,47 @@ impl KerfMcp { #[tool( description = "Render the timeline to a file with full ffmpeg encode control (container, video/audio codec, \ rate control, resolution, fps, bitrate, faststart, gif, audio-only …). Omit `options` for the \ - safe H.264/AAC MP4 default." - )] - async fn export(&self, Parameters(p): Parameters) -> Result { + safe H.264/AAC MP4 default. A render takes minutes: send a `progressToken` in the request's \ + `_meta` to receive progress notifications while it runs, and cancel the request \ + (`notifications/cancelled`) to stop it — a cancelled render deletes its half-written file \ + rather than leaving a broken one behind." + )] + async fn export( + &self, + Parameters(p): Parameters, + context: RequestContext, + ) -> Result { let opts = p.options.unwrap_or_default(); let project = self.project.clone(); - let output_path = blocking(move || { + let output_path = p.output_path; + + // A render runs for minutes. Without progress an agent cannot tell a slow + // export from a hung one, and without cancellation it cannot abandon a + // render whose settings it already knows are wrong — it can only wait for + // a file it does not want. Both ride the protocol rather than a Kerf- + // specific channel: the client's `progressToken`, and the cancellation + // token rmcp trips when `notifications/cancelled` arrives. + let cancel = context.ct.clone(); + let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel::(); + let forward = { + let peer = context.peer.clone(); + let token = context.meta.get_progress_token(); + // The task drains the channel either way, so a client that asked for + // no progress does not leave ffmpeg's ticks piling up in it. + tauri::async_runtime::spawn(async move { + while let Some(progress) = rx.recv().await { + let Some(token) = token.clone() else { continue }; + let mut param = ProgressNotificationParam::new(token, progress.fraction).with_total(1.0); + if let Some(eta) = progress.eta_secs { + param = param.with_message(format!("rendering, {} to go", fmt_ts(eta))); + } + let _ = peer.notify_progress(param).await; + } + }) + }; + + let render_path = output_path.clone(); + let status = blocking(move || { // Snapshot the timeline + assets under the lock, then render with the // lock released so a long export doesn't freeze the GUI (or block other // agent tools) for its whole duration. @@ -1646,11 +1726,39 @@ impl KerfMcp { project.list_assets().map_err(core_err)?, ) }; - kerf_core::render_with(&timeline, &assets, std::path::Path::new(&p.output_path), &opts).map_err(core_err)?; - Ok(p.output_path) + let mut on_progress = |progress: kerf_core::ExportProgress| { + let _ = tx.send(progress); + }; + let status = kerf_core::render_with_progress( + &timeline, + &assets, + std::path::Path::new(&render_path), + &opts, + &mut on_progress, + &|| cancel.is_cancelled(), + ) + .map_err(core_err)?; + if status == kerf_core::RenderStatus::Cancelled { + // Mirrors the GUI: a cancelled export leaves no debris, and in + // particular no truncated file that reads as a finished render. + // Done *here*, not after the await — cancelling the request can + // drop this handler's future, while a blocking job runs to + // completion whether or not anyone is still holding its handle. + let _ = std::fs::remove_file(&render_path); + } + Ok(status) }) - .await?; - json(&serde_json::json!({ "output": output_path })) + .await; + // `tx` died with the closure, so the forwarder has already run dry. + let _ = forward.await; + + match status? { + kerf_core::RenderStatus::Completed => json(&serde_json::json!({ "output": output_path })), + kerf_core::RenderStatus::Cancelled => Err(McpError::internal_error( + format!("export cancelled; {output_path} was removed"), + None, + )), + } } #[tool( @@ -1703,54 +1811,54 @@ impl KerfMcp { #[tool(description = "Enqueue a new task (status: queued) for an agent to claim")] fn add_task(&self, Parameters(p): Parameters) -> Result { - let project = self.lock(); - let task = project.add_task(&p.prompt).map_err(core_err)?; - self.changed(); - json(&task) + self.edit(|project| { + let task = project.add_task(&p.prompt).map_err(core_err)?; + json(&task) + }) } #[tool(description = "Claim the oldest queued task (marks it working) and return it; returns null when the queue is empty")] fn claim_next_task(&self) -> Result { - let project = self.lock(); - let task = project.claim_next_task().map_err(core_err)?; - self.changed(); - json(&task) + self.edit(|project| { + let task = project.claim_next_task().map_err(core_err)?; + json(&task) + }) } #[tool(description = "Mark a claimed task ready for the user to review, with a summary of the edits made")] fn complete_task(&self, Parameters(p): Parameters) -> Result { let id = parse_id(&p.task_id)?; - let project = self.lock(); - let task = project.complete_task(id, p.result).map_err(core_err)?; - self.changed(); - json(&task) + self.edit(|project| { + let task = project.complete_task(id, p.result).map_err(core_err)?; + json(&task) + }) } #[tool(description = "Mark a task failed with an error message")] fn fail_task(&self, Parameters(p): Parameters) -> Result { let id = parse_id(&p.task_id)?; - let project = self.lock(); - let task = project.fail_task(id, &p.error).map_err(core_err)?; - self.changed(); - json(&task) + self.edit(|project| { + let task = project.fail_task(id, &p.error).map_err(core_err)?; + json(&task) + }) } #[tool(description = "Mark a task done (user accepted the staged edit), returning the updated task")] fn resolve_task(&self, Parameters(p): Parameters) -> Result { let id = parse_id(&p.task_id)?; - let project = self.lock(); - let task = project.resolve_task(id).map_err(core_err)?; - self.changed(); - json(&task) + self.edit(|project| { + let task = project.resolve_task(id).map_err(core_err)?; + json(&task) + }) } #[tool(description = "Remove a task from the queue permanently, returning the updated task list")] fn remove_task(&self, Parameters(p): Parameters) -> Result { let id = parse_id(&p.task_id)?; - let project = self.lock(); - project.remove_task(id).map_err(core_err)?; - self.changed(); - json(&project.list_tasks().map_err(core_err)?) + self.edit(|project| { + project.remove_task(id).map_err(core_err)?; + json(&project.list_tasks().map_err(core_err)?) + }) } #[tool(description = "Get peak-magnitude waveform data (0.0–1.0) for an asset's first audio stream")] @@ -1894,10 +2002,29 @@ impl KerfMcp { lock_agent(&self.project) } + /// Run one operation under the shared lock, release the lock, *then* tell the + /// webview. Every mutating tool is this shape, and the order matters: the + /// event makes the GUI re-fetch, and that re-fetch takes the same lock. A + /// failed operation notifies nothing, because nothing changed. + fn edit(&self, op: impl FnOnce(&Project) -> Result) -> Result { + // The guard is a temporary of this statement, so it is dropped here — + // before the emit, and before anything the emit wakes up comes asking + // for the lock. + let out = op(&self.lock())?; + self.changed(); + Ok(out) + } + /// Tell the webview the project changed so it re-fetches and renders live. fn changed(&self) { - if let Err(e) = self.app.emit("project-changed", ()) { - tracing::warn!(error = %e, "failed to emit project-changed"); + self.notify("project-changed"); + } + + /// Emit a webview event, logging rather than failing when there is no window + /// listening — a headless agent session is a perfectly good way to run this. + fn notify(&self, event: &str) { + if let Err(e) = self.app.emit(event, ()) { + tracing::warn!(error = %e, event, "failed to emit webview event"); } } } @@ -1951,7 +2078,8 @@ impl ServerHandler for KerfMcp { "Kerf MCP server. The user queues editing tasks in the desktop app; \ call claim_next_task to take the oldest one (or list_tasks to see \ the whole queue). To work a task, inspect loaded media with \ - list_assets / get_asset_metadata / get_timeline_state, run \ + list_assets / get_asset_metadata / get_timeline_state (import_asset \ + loads a file the user has not added yet), run \ analyze_asset to populate silence / scene / transcript / loudness \ (EBU R128 LUFS) / onset / tempo (BPM + beat grid) / speech-vs-music \ metadata. \ @@ -2434,4 +2562,48 @@ mod tests { fn the_tool_router_is_built_once() { assert!(std::ptr::eq(router(), router())); } + + /// `export` takes a `RequestContext` alongside its `Parameters` so it can + /// report progress and honour cancellation. That second argument must stay + /// out of the *input schema* — a context extractor leaking in would ask the + /// model to invent a request context, and the tool would be uncallable. + #[test] + fn the_context_extractor_stays_out_of_the_export_schema() { + let tools = router().list_all(); + let export = tools.iter().find(|t| t.name == "export").expect("export is registered"); + + let properties = export + .input_schema + .get("properties") + .and_then(|p| p.as_object()) + .expect("input schema has properties"); + assert!(properties.contains_key("output_path"), "{properties:?}"); + assert!(properties.contains_key("options"), "{properties:?}"); + assert_eq!(properties.len(), 2, "only the declared params belong here: {properties:?}"); + + let required: Vec<&str> = export + .input_schema + .get("required") + .and_then(|r| r.as_array()) + .map(|r| r.iter().filter_map(|v| v.as_str()).collect()) + .unwrap_or_default(); + assert_eq!(required, ["output_path"], "options is optional"); + } + + /// An agent with no way to load media can only rearrange what it was handed. + #[test] + fn media_can_be_imported_over_mcp() { + let tools = router().list_all(); + let import = tools + .iter() + .find(|t| t.name == "import_asset") + .expect("import_asset is registered"); + let required: Vec<&str> = import + .input_schema + .get("required") + .and_then(|r| r.as_array()) + .map(|r| r.iter().filter_map(|v| v.as_str()).collect()) + .unwrap_or_default(); + assert_eq!(required, ["path"]); + } } diff --git a/frontend/src/routes/+page.svelte b/frontend/src/routes/+page.svelte index e5a792a..c4b4f49 100644 --- a/frontend/src/routes/+page.svelte +++ b/frontend/src/routes/+page.svelte @@ -68,6 +68,10 @@ unlisteners.push( await listen('project-changed', () => void onProjectChanged()), await listen('proxy-ready', () => ui.refreshPreview()), + // An agent can pick the speech model over MCP; the status is + // otherwise only read at launch, so the picker would keep + // showing the previous model until the next start. + await listen('speech-model-changed', () => void ui.loadTranscriptionStatus()), // Only a 360 lens pair reports here — its stitch is a full // re-encode, so the import overlay shows how far along it is. await listen<{ fraction: number }>( From 4367c93139d13a74925132816df7bbefa29d2672 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Orell=20B=C3=BChler?= Date: Sun, 30 Aug 2026 20:20:52 +0200 Subject: [PATCH 3/3] fix the seven blockers from the UX review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each one was the UI saying or doing something that wasn't true. - The Inspector was gated on a clip being selected, so its Text overlays section — which belongs to the timeline, not to any clip — was unreachable until you clicked one. It is now always mounted (it already had an empty state) and toggled from the toolbar, so closing it is also how you get the width back. - The timeline's transition badge read `crossfade ? 'Crossfade' : 'Dip to black'`, mislabelling nine of eleven kinds. `transitionLabel()` already existed, unused. - "Drop media to start" now does: `+page.svelte` listens for Tauri's drag-drop event, filters by the same extension list the picker uses and runs the same import. - The export dialog opened on Web 1080p, so a 9:16 project opened its export already landscape and the readiness panel warned about the frame the user had just chosen. It now opens on the project frame. - The agent panel showed a green "live" dot whether or not anything was connected. A streamable-HTTP client holds no connection between calls, so `agent_status` reports when an agent last spoke to the endpoint (stamped in `lock_agent` and in `get_info`) and the panel judges from the age: connected / away / nothing yet. - Import ran a full analysis per asset with no way out — a model download and then minutes of inference each. Analysis is now cancellable end to end: `CancelFn` beside the `ProgressFn`, checked between steps, polled about once a second inside the ffmpeg whisper run, and per chunk during the model download (keeping the `.part` file so a retry resumes). The status bar names the step, says what is queued behind it, and stops it. - `editor.error` was recorded and never rendered, so a `.kerf` that would not open opened as silence. It is a dismissible banner now, and `editor.loading` shows in the status bar. --- CLAUDE.md | 53 ++++++++-- crates/kerf-app/src/lib.rs | 54 +++++++++- crates/kerf-app/src/mcp.rs | 35 +++++++ crates/kerf-core/src/analysis.rs | 71 +++++++++++--- crates/kerf-core/src/engine/whisper.rs | 98 ++++++++++++++++--- crates/kerf-core/src/error.rs | 3 + crates/kerf-core/src/lib.rs | 7 +- frontend/src/lib/api.ts | 44 ++++++--- .../lib/components/editor/AgentPanel.svelte | 80 ++++++++++++--- .../lib/components/editor/ExportDialog.svelte | 23 ++++- .../src/lib/components/editor/MediaBin.svelte | 13 ++- .../lib/components/editor/StatusBar.svelte | 23 ++++- .../src/lib/components/editor/Timeline.svelte | 5 +- .../src/lib/components/editor/Toolbar.svelte | 9 +- frontend/src/lib/editor-ui.svelte.ts | 72 +++++++++++++- frontend/src/lib/state.svelte.ts | 7 +- frontend/src/routes/+page.svelte | 95 ++++++++++++++++-- 17 files changed, 605 insertions(+), 87 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 8524152..5025e64 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -218,6 +218,13 @@ transcript tab and an agent both read it to explain an empty transcript. `analyze_asset_media_with_progress` streams a per-step `AnalysisProgress` (`silence`/`scenes`/`loudness`/`rhythm`/`download_model`/`transcribe`/`done`), and transcription runs **last** so the markers land before minutes of inference. +**A pass is abandonable** (`analyze_asset_media_cancellable` / `analyze_cancellable`, +a `CancelFn` alongside the `ProgressFn`): the check lands between steps *and* inside +transcription — the ffmpeg `whisper` run polls it about once a second off +`-stats_period 1` and kills the child, and the model download polls it per chunk, +keeping the `.part` file so the next attempt resumes rather than re-fetching 148 MB. +A cancelled pass returns `Error::Cancelled` and caches **nothing**: a half-analyzed +asset would read as analyzed, and its missing transcript as "no speech". Two more optional features: `libav-render` (above) and `whisper` (in-process `whisper-rs`; needs cmake, a C++ compiler and **libclang** at build time). Both are @@ -446,8 +453,10 @@ no editing logic in the adapter. re-decoding the whole file), `WhisperFilterTranscriber` (the ffmpeg `whisper` filter, always compiled) and `WhisperTranscriber` (in-process, `whisper` feature); `NullAnalyzer` is still the - fallback. `Transcriber::transcribe` takes a `ProgressFn` — alone among the - providers, because it can download a model and then run for minutes. + fallback. `Transcriber::transcribe` takes a `ProgressFn` *and* a `CancelFn` — + alone among the providers, because it can download a model and then run for + minutes, which is both the only step worth reporting on and the only one worth + being able to give up on. `Project::analyze_asset` wires them and caches the `AssetAnalysis`. - `error.rs` — `Error`/`Result`; the `Ffmpeg(#[from] ffmpeg_next::Error)` variant is itself `#[cfg(feature = "ffmpeg")]`. @@ -581,8 +590,16 @@ agent task queue (`list_tasks`, `add_task` → the new `Task`; `resolve_task` / `remove_task` → the refreshed `Task[]`), the agent's staged proposal (`get_staged_edit` → the `StagedEdit` *with its diff*, so the review card renders from one round-trip; `get_staged_timeline` for previewing it; `apply_staged_edit` / -`discard_staged_edit`) and `revision_diff`, and `export_timeline` (emits -`export-progress` events) / `cancel_export`. **No command runs on the main thread** (a plain sync +`discard_staged_edit`) and `revision_diff`, `export_timeline` (emits +`export-progress` events) / `cancel_export`, `cancel_analysis` (the same shape, +for the analysis pass — importing ten clips must not be an unbreakable +commitment to ten transcriptions) and `agent_status` (the MCP endpoint plus how +many seconds ago an agent last spoke to it, or `null` if none ever has — +`mcp::LAST_AGENT_ACTIVITY`, stamped in `lock_agent` and in `get_info`, since +`initialize` is the one moment an agent is known to be there; a +streamable-HTTP client holds no connection between calls, so there is no socket +to report and the panel judges from the age instead of the green dot it used to +show unconditionally). **No command runs on the main thread** (a plain sync command would freeze the window in Tauri v2): quick ops are `#[tauri::command(async)]`, and every heavy one (ffmpeg decode / analysis / export, disk-bound open/save) is an `async fn` that pushes its work onto the @@ -673,7 +690,11 @@ The editor UI is implemented from the **Kerf design system** (claude.ai/design): editor-grade workspace under `src/lib/components/editor/` — bespoke atoms (`Btn`, `IconBtn`, `Badge`, `Icon`, `KerfMark`) plus `TitleBar`, `Toolbar`, `MediaBin`, `Preview`, `Timeline`, `Inspector`, `AgentPanel`, `StatusBar`, composed by -`routes/+page.svelte`. The `Inspector` (right panel) edits the selected clip — +`routes/+page.svelte`. The `Inspector` (right panel) is **mounted whether or not a +clip is selected** and toggled from the toolbar (`ui.inspectorOpen`): its Text +overlays section belongs to the timeline rather than to any one clip, so gating the +whole panel on a selection made titles and captions unreachable until you clicked a +clip. It edits the selected clip — trim, volume, fades, speed, transform, color, **transition** (a grouped picker over `src/lib/transitions.ts` — fade / slide / push, then a direction, because that is the order the choice is actually made and a flat list of eleven names @@ -797,7 +818,11 @@ from the playhead rather than playing it out in slow motion against the sound. playback moves under `bun run dev` and that failure mode is reproducible without a desktop build. `ExportDialog` (⌘E) drives the full `ExportOptions` surface — presets, containers/codecs, rate control, resolution, -loudness normalize, and a **Range: In → out** choice when marks are set. `MediaBin`'s +loudness normalize, and a **Range: In → out** choice when marks are set. It **opens on +the frame the project is cut for** (`initialExport`): the preset whose resolution is +that frame when one matches, else the default preset with its resolution cleared so +"Project frame" renders — otherwise a 9:16 project opened its export already +landscape and the readiness panel warned about the shape the user had just chosen. `MediaBin`'s **Transcript tab is an editing surface**: lines resolve to the clip carrying them, click seeks, the playhead line highlights, and `×` cuts the sentence from the timeline (`cut_clip_range`); cut lines render struck through. When it is *empty* it says which @@ -859,11 +884,21 @@ keeps its placeholder). This browser sample is a **dev harness only** — the de uses the real backend and starts empty. State is two runes singletons: `src/lib/state.svelte.ts` (`export const editor` — assets, timeline, analyses, selection, and the editing actions that call the backend and apply the returned `Timeline`) and `src/lib/editor-ui.svelte.ts` -(`export const ui` — chrome state, playhead/zoom/playback, and `runAnalysis` which runs real -analysis and toggles the `analyzing` flag). There is **no scripted demo phase machine**: the +(`export const ui` — chrome state, playhead/zoom/playback, and `analyzeQueue` / +`runAnalysis` / `stopAnalysis`: a batch analyzes **one asset at a time** — each pass is +ffmpeg-bound, so running them together only makes each slower — and stopping drops +the whole rest of the queue). There is **no scripted demo phase machine**: the editor chrome derives from real state — `MediaBin` shows a dropzone until `editor.assets` is non-empty, `StatusBar` shows the selected asset's real fps/resolution/codec and timeline -duration, and `Preview` shows the decoded frame or a "No media loaded" placeholder. +duration (plus the analysis step, what is still queued behind it and a **Stop**), and +`Preview` shows the decoded frame or a "No media loaded" placeholder. +**Dropping files onto the window imports them** (`+page.svelte` listens for Tauri's +`onDragDropEvent`, filters by `isMediaPath` — the same extension list the picker +filters by, so a dropped folder of mixed files doesn't answer with one error per +README — and runs the same `editor.importPaths` the picker resolves to), which is +what the bin's "Drop media to start" had been promising. `editor.error` renders as a +dismissible banner under the toolbar: it was recorded and never shown, so a `.kerf` +that would not open opened as silence. ## Conventions diff --git a/crates/kerf-app/src/lib.rs b/crates/kerf-app/src/lib.rs index 56d5e87..2af1b70 100644 --- a/crates/kerf-app/src/lib.rs +++ b/crates/kerf-app/src/lib.rs @@ -33,6 +33,11 @@ struct AppState { /// Set by `cancel_export` and polled by the in-flight export; lives outside /// the project lock so a cancel lands even while a render holds it. export_cancel: Arc, + /// Same, for the in-flight analysis pass. Analysis downloads a speech model + /// and then transcribes for minutes, so it has to be abandonable — and the + /// GUI runs imported assets through it one after another, which is a long + /// commitment to make on the user's behalf without an exit. + analysis_cancel: Arc, } #[derive(Serialize)] @@ -314,6 +319,9 @@ struct AnalysisProgressEvent { async fn analyze_asset(app: AppHandle, state: State<'_, AppState>, asset_id: String) -> CmdResult { let id = id(&asset_id)?; let shared = state.project.clone(); + // Fresh cancel flag for this pass; `cancel_analysis` flips it from the UI. + let cancel = state.analysis_cancel.clone(); + cancel.store(false, Ordering::SeqCst); blocking(move || { // Resolve the asset under the lock, run the multi-second ffmpeg analysis // with the lock released, then re-acquire it only to cache the result — @@ -333,7 +341,15 @@ async fn analyze_asset(app: AppHandle, state: State<'_, AppState>, asset_id: Str }, ); }; - let analysis = kerf_core::analyze_asset_media_with_progress(&asset, &mut on_progress).map_err(|e| e.to_string())?; + let analysis = kerf_core::analyze_asset_media_cancellable(&asset, &mut on_progress, &|| cancel.load(Ordering::SeqCst)) + .map_err(|e| match e { + // A cancel is the user's own doing, not a failure — the + // caller keys off this string to stay quiet about it. + kerf_core::Error::Cancelled => ANALYSIS_CANCELLED.to_string(), + other => other.to_string(), + })?; + // Nothing is cached for a cancelled pass: a half-analyzed asset would + // read as analyzed, and the missing transcript as "no speech". lock_user(&shared).set_analysis(&analysis).map_err(|e| e.to_string())?; Ok(analysis) }) @@ -1438,6 +1454,18 @@ fn reveal_path(app: AppHandle, path: String) -> CmdResult<()> { .map_err(|e| e.to_string()) } +/// The error an abandoned analysis pass returns. The webview matches on it to +/// tell "the user stopped this" apart from "this broke". +const ANALYSIS_CANCELLED: &str = "analysis cancelled"; + +/// Request cancellation of the in-flight analysis pass (if any). The running +/// [`analyze_asset`] observes the flag between steps — and, during +/// transcription, about once a second — then gives up and caches nothing. +#[tauri::command(async)] +fn cancel_analysis(state: State<'_, AppState>) { + state.analysis_cancel.store(true, Ordering::SeqCst); +} + /// Request cancellation of the in-flight export (if any). The running /// [`export_timeline`] observes the flag on its next progress poll, stops /// ffmpeg, and returns the `"export cancelled"` error. @@ -1456,6 +1484,27 @@ fn mcp_endpoint() -> String { mcp::endpoint_url() } +/// Where the endpoint is, and how long ago an agent last used it. +/// +/// A streamable-HTTP client holds no connection between calls, so there is no +/// "is it plugged in" to report — `last_seen_secs` is `None` until something +/// has spoken to the endpoint at all, and the panel decides from its age +/// whether to call that connected. Anything else would be the green dot the +/// panel used to show whether or not an agent existed. +#[derive(Serialize)] +struct AgentStatus { + endpoint: String, + last_seen_secs: Option, +} + +#[tauri::command(async)] +fn agent_status() -> AgentStatus { + AgentStatus { + endpoint: mcp::endpoint_url(), + last_seen_secs: mcp::agent_last_seen_secs(), + } +} + // ---- diagnostics (logs) ---------------------------------------------------- #[tauri::command(async)] @@ -1570,6 +1619,7 @@ pub fn run() { .manage(AppState { project: project.clone(), export_cancel: Arc::new(AtomicBool::new(false)), + analysis_cancel: Arc::new(AtomicBool::new(false)), }) .setup(move |app| { // Logging needs the resolved platform log directory, so set it up here @@ -1685,11 +1735,13 @@ pub fn run() { hw_encoders, export_timeline, cancel_export, + cancel_analysis, export_cover, platform_targets, platform_check, reveal_path, mcp_endpoint, + agent_status, log_dir, reveal_logs ]) diff --git a/crates/kerf-app/src/mcp.rs b/crates/kerf-app/src/mcp.rs index 0a9fdbc..c740c64 100644 --- a/crates/kerf-app/src/mcp.rs +++ b/crates/kerf-app/src/mcp.rs @@ -11,6 +11,7 @@ //! [`EditSource::User`] the same way), so attribution stays correct even though //! both front doors share one `Project`. +use std::sync::atomic::{AtomicI64, Ordering}; use std::sync::{Arc, Mutex, MutexGuard, OnceLock}; use base64::Engine as _; @@ -2034,11 +2035,42 @@ impl KerfMcp { /// mutex (a panic while another op held the lock) rather than panicking here /// too — a single failed op shouldn't brick the agent endpoint for the session. fn lock_agent(project: &Mutex) -> MutexGuard<'_, Project> { + note_agent_activity(); let mut guard = project.lock().unwrap_or_else(|e| e.into_inner()); guard.set_actor(EditSource::Agent); guard } +// ---- agent presence -------------------------------------------------------- + +/// Unix seconds of the last thing an agent did — its `initialize` handshake, or +/// any tool call that reached the project — or 0 if nothing ever has. +/// +/// The agent panel used to show a green "live" dot unconditionally, which said +/// the same thing whether or not anything was connected. Nothing here claims a +/// *socket* is open: a streamable-HTTP client holds no connection between +/// calls, so the only honest signal is when it last spoke. Every agent-side +/// project access goes through `lock_agent`, which makes that the one choke +/// point worth stamping. +static LAST_AGENT_ACTIVITY: AtomicI64 = AtomicI64::new(0); + +fn note_agent_activity() { + LAST_AGENT_ACTIVITY.store(unix_now(), Ordering::Relaxed); +} + +/// Seconds since an agent last spoke, or `None` if none ever has. +pub fn agent_last_seen_secs() -> Option { + let at = LAST_AGENT_ACTIVITY.load(Ordering::Relaxed); + (at > 0).then(|| (unix_now() - at).max(0)) +} + +fn unix_now() -> i64 { + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs() as i64) + .unwrap_or(0) +} + /// Run a blocking (ffmpeg) job on the blocking thread pool and await it, so a /// slow decode / analysis / render doesn't pin one of the shared tokio workers /// serving the MCP endpoint (and the rest of the app) for its whole duration. @@ -2071,6 +2103,9 @@ fn server_identity() -> Implementation { #[tool_handler(router = router())] impl ServerHandler for KerfMcp { fn get_info(&self) -> ServerInfo { + // `initialize` is the one moment we know an agent is on the other end + // of the socket, so it counts as being seen even before it calls a tool. + note_agent_activity(); let mut info = ServerInfo::default(); info.server_info = server_identity(); info.capabilities = ServerCapabilities::builder().enable_tools().build(); diff --git a/crates/kerf-core/src/analysis.rs b/crates/kerf-core/src/analysis.rs index 6582a44..292f886 100644 --- a/crates/kerf-core/src/analysis.rs +++ b/crates/kerf-core/src/analysis.rs @@ -5,7 +5,7 @@ //! be swapped in without touching the rest of the core. use crate::engine::whisper; -use crate::error::Result; +use crate::error::{Error, Result}; use crate::model::{Asset, AssetAnalysis, Loudness, Rhythm, TimeRange, TranscriptSegment}; /// A step of an analysis pass, reported as it runs. @@ -47,6 +47,18 @@ impl AnalysisProgress { /// A progress sink for an analysis pass. pub type ProgressFn<'a> = &'a mut dyn FnMut(AnalysisProgress); +/// Polled while an analysis pass runs; once it returns true the pass gives up +/// with [`Error::Cancelled`] instead of finishing. +/// +/// Analysis is not a few seconds of ffmpeg any more — the first transcription +/// downloads a model and then runs inference for a good fraction of the media's +/// duration — so it has to be abandonable. The same shape as the export's +/// cancel callback. +pub type CancelFn<'a> = &'a dyn Fn() -> bool; + +/// The no-op cancel, for callers that never abandon a pass. +pub(crate) const NEVER_CANCEL: CancelFn<'static> = &|| false; + /// Detects silent spans in an asset's audio. pub trait SilenceDetector: Send + Sync { fn detect_silence(&self, asset: &Asset) -> Result>; @@ -62,7 +74,7 @@ pub trait SceneDetector: Send + Sync { /// Transcription is the one analysis step that can take minutes and, on its /// first run, download a model — so unlike its siblings it reports progress. pub trait Transcriber: Send + Sync { - fn transcribe(&self, asset: &Asset, progress: ProgressFn) -> Result>; + fn transcribe(&self, asset: &Asset, progress: ProgressFn, cancel: CancelFn) -> Result>; } /// Measures EBU R128 loudness of an asset's audio (`None` for silent / video-only @@ -189,8 +201,8 @@ pub struct WhisperFilterTranscriber { } impl Transcriber for WhisperFilterTranscriber { - fn transcribe(&self, asset: &Asset, progress: ProgressFn) -> Result> { - let model = whisper::ensure_model(&mut model_progress(progress))?; + fn transcribe(&self, asset: &Asset, progress: ProgressFn, cancel: CancelFn) -> Result> { + let model = whisper::ensure_model(&mut model_progress(progress), cancel)?; progress(AnalysisProgress::with_fraction("transcribe", 0.0)); whisper::transcribe( std::path::Path::new(&asset.path), @@ -198,6 +210,7 @@ impl Transcriber for WhisperFilterTranscriber { self.language.as_deref(), asset.duration, &mut |f| progress(AnalysisProgress::with_fraction("transcribe", f)), + cancel, ) } } @@ -214,11 +227,12 @@ pub struct WhisperTranscriber { #[cfg(feature = "whisper")] impl Transcriber for WhisperTranscriber { - fn transcribe(&self, asset: &Asset, progress: ProgressFn) -> Result> { - use crate::error::Error; - - let model = whisper::ensure_model(&mut model_progress(progress))?; + fn transcribe(&self, asset: &Asset, progress: ProgressFn, cancel: CancelFn) -> Result> { + let model = whisper::ensure_model(&mut model_progress(progress), cancel)?; progress(AnalysisProgress::with_fraction("transcribe", 0.0)); + if cancel() { + return Err(Error::Cancelled); + } let samples = crate::engine::decode_audio_16k_mono(std::path::Path::new(&asset.path))?; let language = self.language.clone(); @@ -373,7 +387,7 @@ impl SceneDetector for NullAnalyzer { } impl Transcriber for NullAnalyzer { - fn transcribe(&self, _asset: &Asset, _progress: ProgressFn) -> Result> { + fn transcribe(&self, _asset: &Asset, _progress: ProgressFn, _cancel: CancelFn) -> Result> { Ok(Vec::new()) } } @@ -459,6 +473,16 @@ pub fn analyze_asset_media(asset: &Asset) -> Result { /// so the adapters use this variant and forward the events to the GUI / an /// agent; the silent wrapper above stays for callers that don't care. pub fn analyze_asset_media_with_progress(asset: &Asset, progress: ProgressFn) -> Result { + analyze_asset_media_cancellable(asset, progress, NEVER_CANCEL) +} + +/// [`analyze_asset_media_with_progress`], abandoning the pass once `cancel` +/// returns true. +/// +/// The check lands between steps *and* inside transcription, which is where the +/// wait actually is: a model download and then inference for minutes. A +/// cancelled pass caches nothing — a half-analyzed asset would look analyzed. +pub fn analyze_asset_media_cancellable(asset: &Asset, progress: ProgressFn, cancel: CancelFn) -> Result { let silence = FfmpegSilenceDetector::default(); let scene = FfmpegSceneDetector::default(); let loudness = FfmpegLoudnessAnalyzer; @@ -475,7 +499,7 @@ pub fn analyze_asset_media_with_progress(asset: &Asset, progress: ProgressFn) -> loudness: &loudness, rhythm: &rhythm, }; - analyze_with_progress(asset, &providers, progress) + analyze_cancellable(asset, &providers, progress, cancel) } /// Run every configured provider and assemble an [`AssetAnalysis`]. @@ -485,19 +509,44 @@ pub fn analyze(asset: &Asset, providers: &AnalysisProviders) -> Result Result { + analyze_cancellable(asset, providers, progress, NEVER_CANCEL) +} + +/// [`analyze_with_progress`], checked against `cancel` before each step. +pub fn analyze_cancellable( + asset: &Asset, + providers: &AnalysisProviders, + progress: ProgressFn, + cancel: CancelFn, +) -> Result { + // Each step is one whole-file ffmpeg pass, so a cancel lands within one of + // them rather than instantly; transcription, the only step long enough for + // that to matter, polls the same callback itself. + macro_rules! bail_if_cancelled { + () => { + if cancel() { + return Err(Error::Cancelled); + } + }; + } + bail_if_cancelled!(); progress(AnalysisProgress::stage("silence")); let silence_segments = providers.silence.detect_silence(asset)?; + bail_if_cancelled!(); progress(AnalysisProgress::stage("scenes")); let scene_changes = providers.scene.detect_scenes(asset)?; + bail_if_cancelled!(); progress(AnalysisProgress::stage("loudness")); let loudness = providers.loudness.measure(asset)?; + bail_if_cancelled!(); progress(AnalysisProgress::stage("rhythm")); // One provider call for onsets + tempo + class: they share a single decode. let rhythm = providers.rhythm.analyze_rhythm(asset)?; + bail_if_cancelled!(); // Transcription runs last: it is by far the slowest step, and the ones above // are what the timeline draws — markers and waveform regions appear as soon // as the caller caches this, rather than waiting behind minutes of inference. - let transcript = providers.transcriber.transcribe(asset, progress)?; + let transcript = providers.transcriber.transcribe(asset, progress, cancel)?; progress(AnalysisProgress::stage("done")); Ok(AssetAnalysis { asset_id: asset.id, diff --git a/crates/kerf-core/src/engine/whisper.rs b/crates/kerf-core/src/engine/whisper.rs index f0dddef..5a9eb39 100644 --- a/crates/kerf-core/src/engine/whisper.rs +++ b/crates/kerf-core/src/engine/whisper.rs @@ -209,7 +209,11 @@ impl DownloadProgress { /// /// A user-supplied path is never fetched — if it is missing that is a /// configuration error, not something to paper over with a different model. -pub fn ensure_model(progress: &mut dyn FnMut(DownloadProgress)) -> Result { +/// `cancel` is polled while downloading: a model is hundreds of megabytes, and +/// without this, asking analysis to stop meant waiting out a fetch it had +/// already started. The partial `.part` file is deliberately *kept* on a +/// cancel, so the next attempt resumes it. +pub fn ensure_model(progress: &mut dyn FnMut(DownloadProgress), cancel: &dyn Fn() -> bool) -> Result { match configured_model() { ModelChoice::File(p) => { if p.is_file() { @@ -221,7 +225,7 @@ pub fn ensure_model(progress: &mut dyn FnMut(DownloadProgress)) -> Result download_model(&name, progress), + ModelChoice::Named(name) => download_model_cancellable(&name, progress, cancel), } } @@ -233,6 +237,15 @@ pub fn ensure_model(progress: &mut dyn FnMut(DownloadProgress)) -> Result Result { + download_model_cancellable(name, progress, &|| false) +} + +/// [`download_model`], polling `cancel` as it streams. +pub fn download_model_cancellable( + name: &str, + progress: &mut dyn FnMut(DownloadProgress), + cancel: &dyn Fn() -> bool, +) -> Result { if model_info(name).is_none() { return Err(Error::Engine(format!( "unknown speech model '{name}'; expected one of: {}", @@ -251,8 +264,13 @@ pub fn download_model(name: &str, progress: &mut dyn FnMut(DownloadProgress)) -> let tmp = dst.with_extension(format!("{}.part", std::process::id())); let url = model_url(name); tracing::info!(model = name, %url, "downloading speech model"); - stream_to_file(&url, &tmp, progress).inspect_err(|_| { - let _ = std::fs::remove_file(&tmp); + // A cancel keeps the `.part` file: it is a valid prefix of the model, and + // the next attempt resumes it with a range request. Any other failure is a + // file we can't trust, so it goes. + stream_to_file(&url, &tmp, progress, cancel).inspect_err(|e| { + if !matches!(e, Error::Cancelled) { + let _ = std::fs::remove_file(&tmp); + } })?; // A model that isn't one (an HTML error page, a truncated CDN response) @@ -271,7 +289,7 @@ pub fn download_model(name: &str, progress: &mut dyn FnMut(DownloadProgress)) -> } /// Stream `url` into `tmp`, resuming a partial file when one is there. -fn stream_to_file(url: &str, tmp: &Path, progress: &mut dyn FnMut(DownloadProgress)) -> Result<()> { +fn stream_to_file(url: &str, tmp: &Path, progress: &mut dyn FnMut(DownloadProgress), cancel: &dyn Fn() -> bool) -> Result<()> { let have = std::fs::metadata(tmp).map(|m| m.len()).unwrap_or(0); // A connect timeout but no global one: reaching the host should fail fast // (a firewalled or DNS-blackholed mirror otherwise stalls for minutes before @@ -312,6 +330,12 @@ fn stream_to_file(url: &str, tmp: &Path, progress: &mut dyn FnMut(DownloadProgre progress(DownloadProgress { downloaded, total }); let mut last_report = downloaded; loop { + if cancel() { + // Flush what we have so the `.part` file is a usable prefix to + // resume from rather than however much happened to reach the OS. + let _ = file.flush(); + return Err(Error::Cancelled); + } let n = reader .read(&mut buf) .map_err(|e| Error::Engine(format!("speech model download failed: {e}")))?; @@ -449,6 +473,7 @@ pub fn transcribe( language: Option<&str>, duration: f64, progress: &mut dyn FnMut(f64), + cancel: &dyn Fn() -> bool, ) -> Result> { let model_dir = model .parent() @@ -502,8 +527,17 @@ pub fn transcribe( }); let stdout = child.stdout.take().expect("stdout piped"); + // `-stats_period 1` makes ffmpeg write a progress block every second, so the + // cancel is polled about that often — inference on a long take runs for + // minutes and is the one wait worth being able to abandon. + let mut cancelled = false; for line in BufReader::new(stdout).lines() { let Ok(line) = line else { break }; + if cancel() { + cancelled = true; + let _ = child.kill(); + break; + } if line == "progress=end" { break; } @@ -516,6 +550,9 @@ pub fn transcribe( let status = child.wait().map_err(|e| Error::Engine(format!("ffmpeg wait failed: {e}")))?; let stderr_text = stderr_handle.join().unwrap_or_default(); + if cancelled { + return Err(Error::Cancelled); + } if !status.success() { let mut tail: Vec<&str> = stderr_text.lines().rev().take(12).collect(); tail.reverse(); @@ -771,7 +808,7 @@ mod tests { let _cleanup = TempFile(tmp.clone()); let mut seen = Vec::new(); - stream_to_file(&url, &tmp, &mut |p| seen.push(p)).expect("download"); + stream_to_file(&url, &tmp, &mut |p| seen.push(p), &|| false).expect("download"); assert_eq!(std::fs::read(&tmp).expect("read"), body); verify_ggml(&tmp).expect("magic"); @@ -780,6 +817,40 @@ mod tests { assert!(seen.len() > 2, "expected streaming progress, got {seen:?}"); } + /// Cancelling mid-download has to leave something worth resuming: the whole + /// point of abandoning a 148 MB fetch is not paying for it twice. + #[test] + fn a_cancelled_download_keeps_a_resumable_part_file() { + let body = fake_model(3 * 1024 * 1024); + let url = spawn_server(body.clone(), true, 0); + let tmp = temp_path("cancel"); + let _cleanup = TempFile(tmp.clone()); + + // Cancel as soon as the first megabyte has been reported. + let mut seen = 0u64; + let cancelled = std::cell::Cell::new(false); + let err = stream_to_file( + &url, + &tmp, + &mut |p| { + seen = p.downloaded; + if seen > 0 { + cancelled.set(true); + } + }, + &|| cancelled.get(), + ) + .expect_err("cancelled"); + + assert!(matches!(err, Error::Cancelled), "expected Cancelled, got {err:?}"); + let on_disk = std::fs::metadata(&tmp).expect("part file").len(); + assert!( + on_disk > 0 && on_disk < body.len() as u64, + "expected a partial file, got {on_disk}" + ); + assert_eq!(std::fs::read(&tmp).expect("read"), body[..on_disk as usize]); + } + #[test] fn resumes_a_partial_download_instead_of_restarting() { let body = fake_model(2 * 1024 * 1024); @@ -790,9 +861,14 @@ mod tests { std::fs::write(&tmp, &body[..body.len() / 2]).expect("seed"); let mut first = None; - stream_to_file(&url, &tmp, &mut |p| { - first.get_or_insert(p); - }) + stream_to_file( + &url, + &tmp, + &mut |p| { + first.get_or_insert(p); + }, + &|| false, + ) .expect("resume"); assert_eq!(std::fs::read(&tmp).expect("read"), body); @@ -812,7 +888,7 @@ mod tests { // Stale bytes that must not be prepended to the fresh 200 response. std::fs::write(&tmp, vec![0xffu8; 4096]).expect("seed"); - stream_to_file(&url, &tmp, &mut |_| {}).expect("download"); + stream_to_file(&url, &tmp, &mut |_| {}, &|| false).expect("download"); assert_eq!(std::fs::read(&tmp).expect("read"), body); } @@ -823,7 +899,7 @@ mod tests { let tmp = temp_path("short"); let _cleanup = TempFile(tmp.clone()); - assert!(stream_to_file(&url, &tmp, &mut |_| {}).is_err()); + assert!(stream_to_file(&url, &tmp, &mut |_| {}, &|| false).is_err()); } #[test] diff --git a/crates/kerf-core/src/error.rs b/crates/kerf-core/src/error.rs index 8c68a1a..0b4ae78 100644 --- a/crates/kerf-core/src/error.rs +++ b/crates/kerf-core/src/error.rs @@ -49,6 +49,9 @@ pub enum Error { #[error("media engine error: {0}")] Engine(String), + #[error("cancelled")] + Cancelled, + #[cfg(feature = "ffmpeg")] #[error("ffmpeg error: {0}")] Ffmpeg(#[from] ffmpeg_next::Error), diff --git a/crates/kerf-core/src/lib.rs b/crates/kerf-core/src/lib.rs index 6ba8703..c9a06f1 100644 --- a/crates/kerf-core/src/lib.rs +++ b/crates/kerf-core/src/lib.rs @@ -17,9 +17,10 @@ mod engine; #[cfg(feature = "whisper")] pub use analysis::WhisperTranscriber; pub use analysis::{ - analyze, analyze_asset_media, analyze_asset_media_with_progress, analyze_with_progress, transcription_status, - AnalysisProgress, AnalysisProviders, FfmpegRhythmAnalyzer, FfmpegSceneDetector, FfmpegSilenceDetector, NullAnalyzer, - ProgressFn, RhythmAnalyzer, SceneDetector, SilenceDetector, Transcriber, TranscriptionStatus, WhisperFilterTranscriber, + analyze, analyze_asset_media, analyze_asset_media_cancellable, analyze_asset_media_with_progress, analyze_cancellable, + analyze_with_progress, transcription_status, AnalysisProgress, AnalysisProviders, CancelFn, FfmpegRhythmAnalyzer, + FfmpegSceneDetector, FfmpegSilenceDetector, NullAnalyzer, ProgressFn, RhythmAnalyzer, SceneDetector, SilenceDetector, + Transcriber, TranscriptionStatus, WhisperFilterTranscriber, }; pub use engine::{ download_speech_model, export_still, generate_proxy, hw_encoders, insta360_pair, proxy_path, proxy_width, render_with, diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index 4ea2d87..a7699b5 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -259,22 +259,28 @@ export async function importAsset(path: string): Promise { } /** Open a native (multi-select) file picker and return the chosen media paths. */ +/** The file types Kerf imports. Shared by the picker's filter and the + * drag-and-drop handler, so dropping a file onto the window accepts exactly + * what browsing for one does. */ +export const MEDIA_EXTENSIONS = [ + 'mp4', 'mov', 'mkv', 'webm', 'wav', 'mp3', 'm4a', 'aac', + 'png', 'jpg', 'jpeg', 'webp', 'gif', 'bmp', 'tiff', 'tif', + // Insta360 captures (360 video / photo) — MP4 under a custom extension. + 'insv', 'insp' +]; + +/** Whether a path looks like media Kerf can import. */ +export function isMediaPath(path: string): boolean { + const ext = path.split('.').pop()?.toLowerCase() ?? ''; + return MEDIA_EXTENSIONS.includes(ext); +} + export async function pickMediaPaths(): Promise { if (!inTauri()) return []; const { open } = await import('@tauri-apps/plugin-dialog'); const selected = await open({ multiple: true, - filters: [ - { - name: 'Media', - extensions: [ - 'mp4', 'mov', 'mkv', 'webm', 'wav', 'mp3', 'm4a', 'aac', - 'png', 'jpg', 'jpeg', 'webp', 'gif', 'bmp', 'tiff', 'tif', - // Insta360 captures (360 video / photo) — MP4 under a custom extension. - 'insv', 'insp' - ] - } - ] + filters: [{ name: 'Media', extensions: MEDIA_EXTENSIONS }] }); if (selected == null) return []; return Array.isArray(selected) ? selected : [selected]; @@ -1676,6 +1682,13 @@ export async function cancelExport(): Promise { return invoke('cancel_export'); } +/** Ask the running analysis pass to give up. It stops between steps, and about + * once a second during transcription — the step that runs for minutes. */ +export async function cancelAnalysis(): Promise { + if (!inTauri()) return; + return invoke('cancel_analysis'); +} + /** Subscribe to `export-progress` events for the running render. Returns an unlisten fn. */ export async function onExportProgress(cb: (p: ExportProgress) => void): Promise<() => void> { if (!inTauri()) return () => {}; @@ -1758,6 +1771,15 @@ export async function mcpEndpoint(): Promise { return invoke('mcp_endpoint'); } +/** Where the endpoint is, and how long ago an agent last used it — `null` when + * none ever has. A streamable-HTTP client holds no connection between calls, + * so there is no socket to report as open; the panel judges from the age. In + * the browser harness there is no server at all, hence `null`. */ +export async function agentStatus(): Promise<{ endpoint: string; last_seen_secs: number | null }> { + if (!inTauri()) return { endpoint: 'http://127.0.0.1:7777/mcp', last_seen_secs: null }; + return invoke<{ endpoint: string; last_seen_secs: number | null }>('agent_status'); +} + // ---- diagnostics (logs) ---------------------------------------------------- /** The platform log directory Kerf writes its logfile to, or `null` in the browser. */ diff --git a/frontend/src/lib/components/editor/AgentPanel.svelte b/frontend/src/lib/components/editor/AgentPanel.svelte index 72ddf1a..16c9830 100644 --- a/frontend/src/lib/components/editor/AgentPanel.svelte +++ b/frontend/src/lib/components/editor/AgentPanel.svelte @@ -1,11 +1,11 @@