diff --git a/src/agent/environment.rs b/src/agent/environment.rs index a301894..cd09b48 100644 --- a/src/agent/environment.rs +++ b/src/agent/environment.rs @@ -99,7 +99,10 @@ impl SandboxStatus { /// Memory in use and its limit, in bytes. pub fn memory(mut self, used: u64, limit: u64) -> Self { - assert!(limit > 0 && used <= limit, "memory {used} of {limit}"); + assert!(limit > 0, "memory {used} of no limit"); + if used > limit { + log::error!("environment: memory {used} past its limit {limit}; the meter fills"); + } self.memory = Some((used, limit)); self } @@ -173,7 +176,7 @@ impl RenderOnce for SandboxStatus { Meter::new( (self.id.clone(), "memory"), "Memory", - used as f32 / limit as f32, + used.min(limit) as f32 / limit as f32, ) .detail(format!( "{} of {}", diff --git a/src/agent/memory.rs b/src/agent/memory.rs index 457dfc3..fae4bb9 100644 --- a/src/agent/memory.rs +++ b/src/agent/memory.rs @@ -166,7 +166,7 @@ pub struct Checkpoint { pub struct CheckpointList { id: ElementId, checkpoints: Vec, - current: usize, + current: Option, on_rewind: Option, } @@ -178,15 +178,17 @@ impl CheckpointList { current: usize, ) -> Self { let checkpoints: Vec = checkpoints.into_iter().collect(); - assert!( - current < checkpoints.len(), - "checkpoint {current} of {}", - checkpoints.len() - ); + let held = current < checkpoints.len(); + if !held { + log::error!( + "checkpoints: checkpoint {current} of {}; none current", + checkpoints.len() + ); + } Self { id: id.into(), checkpoints, - current, + current: held.then_some(current), on_rewind: None, } } @@ -210,7 +212,7 @@ impl RenderOnce for CheckpointList { let rewind = self .on_rewind .clone() - .filter(|_| ix != current) + .filter(|_| current != Some(ix)) .map(|rewind| { Button::new((self.id.clone(), format!("rewind-{ix}")), "Rewind to here") .variant(ButtonVariant::Ghost) @@ -233,12 +235,12 @@ impl RenderOnce for CheckpointList { .items_center() .gap_x_2() .child(plural(point.files as u64, "file", "files")) - .when(ix == current, |meta| { + .when(current == Some(ix), |meta| { meta.child(div().text_color(colors.accent).child("Here now")) }) .children(rewind), ); - timeline.item(if ix == current { + timeline.item(if current == Some(ix) { item.tone(Tone::Accent) } else { item diff --git a/src/agent/previews.rs b/src/agent/previews.rs index c100de3..fb9324c 100644 --- a/src/agent/previews.rs +++ b/src/agent/previews.rs @@ -71,7 +71,7 @@ pub struct BrowserPreview { id: ElementId, url: SharedString, frames: Vec, - shown: usize, + shown: Option, ratio: f32, loading: bool, on_show: Option, @@ -86,12 +86,18 @@ impl BrowserPreview { shown: usize, ) -> Self { let frames: Vec = frames.into_iter().map(Into::into).collect(); - assert!(shown < frames.len(), "frame {shown} of {}", frames.len()); + let held = shown < frames.len(); + if !held { + log::error!( + "browser preview: frame {shown} of {}; none shown", + frames.len() + ); + } Self { id: id.into(), url: url.into(), frames, - shown, + shown: held.then_some(shown), ratio: FRAME, loading: false, on_show: None, @@ -170,7 +176,7 @@ impl RenderOnce for BrowserPreview { .h(thumb) .rounded(theme.radius(Radius::Sm)) .border_1() - .border_color(if ix == shown { + .border_color(if shown == Some(ix) { colors.accent } else { colors.border @@ -205,7 +211,7 @@ impl RenderOnce for BrowserPreview { .when(strip.is_none(), |frame| { frame.rounded_b(theme.radius(Radius::Lg)) }) - .child({ + .children(shown.map(|shown| { let image = Image::new((self.id.clone(), "frame"), source(&self.frames[shown])) .size_full(); @@ -214,7 +220,7 @@ impl RenderOnce for BrowserPreview { } else { image } - }), + })), ) .children(strip) } diff --git a/src/agent/progress.rs b/src/agent/progress.rs index e5cf462..f07e231 100644 --- a/src/agent/progress.rs +++ b/src/agent/progress.rs @@ -262,7 +262,10 @@ impl AgentProgress { done: usize, total: usize, ) -> Self { - assert!(total > 0 && done <= total, "progress of {done} in {total}"); + assert!(total > 0, "progress of {done} in none"); + if done > total { + log::error!("agent progress: {done} done of {total}; the bar fills"); + } Self { id: id.into(), current: current.into(), @@ -308,7 +311,10 @@ impl RenderOnce for AgentProgress { })), ) .child({ - let bar = ProgressBar::new((self.id, "bar"), self.done as f32 / self.total as f32); + let bar = ProgressBar::new( + (self.id, "bar"), + self.done.min(self.total) as f32 / self.total as f32, + ); if self.total >= 2 { bar.segments(self.total) } else { diff --git a/src/agent/tests/environment.rs b/src/agent/tests/environment.rs index cce5e42..7c803bd 100644 --- a/src/agent/tests/environment.rs +++ b/src/agent/tests/environment.rs @@ -163,10 +163,29 @@ fn every_checkpoint_but_the_current_rewinds(cx: &mut TestAppContext) { ); } -#[test] -#[should_panic(expected = "memory 5 of 4")] -fn memory_past_its_limit_fails_loud() { - let _ = SandboxStatus::new("box", "python", SandboxState::Ready).memory(5, 4); +/// A sandbox past its memory limit and checkpoints whose current one left. +struct Overrun; + +impl Render for Overrun { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + let point = Checkpoint { + key: "one".into(), + label: "One".into(), + at: Timestamp::UNIX_EPOCH, + files: 1, + }; + div() + .w(px(360.0)) + .child(SandboxStatus::new("box", "python", SandboxState::Ready).memory(5, 4)) + .child(CheckpointList::new("points", [point], 4)) + } +} + +#[gpui::test] +fn memory_past_its_limit_and_a_gone_checkpoint_draw(cx: &mut TestAppContext) { + setup(cx); + let (_, cx) = cx.add_window_view(|_, _| Overrun); + settle(cx); } #[test] diff --git a/src/agent/tests/mod.rs b/src/agent/tests/mod.rs index 4fb859d..5d19385 100644 --- a/src/agent/tests/mod.rs +++ b/src/agent/tests/mod.rs @@ -129,13 +129,7 @@ fn the_prompt_walks_deny_always_then_once(cx: &mut TestAppContext) { ); } -#[test] -#[should_panic(expected = "progress of 6 in 5")] -fn progress_past_its_total_fails_loud() { - let _ = AgentProgress::new("progress", "Writing", 6, 5); -} - -/// A progress bar for a task of one step, `done` or not. +/// A progress bar for a task of one step, `done` or not; live counts may run past it. struct Single { done: usize, } @@ -148,6 +142,13 @@ impl Render for Single { } } +#[gpui::test] +fn progress_past_its_total_fills_the_bar(cx: &mut TestAppContext) { + cx.update(Theme::init); + let (_, cx) = cx.add_window_view(|_, _| Single { done: 6 }); + settle(cx); +} + #[gpui::test] fn a_task_of_one_step_draws_before_and_after(cx: &mut TestAppContext) { cx.update(Theme::init); diff --git a/src/agent/tests/previews.rs b/src/agent/tests/previews.rs index f7c0c1a..936e49c 100644 --- a/src/agent/tests/previews.rs +++ b/src/agent/tests/previews.rs @@ -52,10 +52,25 @@ fn a_frame_shows_from_the_keyboard(cx: &mut TestAppContext) { ); } -#[test] -#[should_panic(expected = "frame 3 of 3")] -fn a_frame_past_the_last_fails_loud() { - let _ = BrowserPreview::new("browser", "localhost", ["a.png", "b.png", "c.png"], 3); +/// A browser asked to show a frame it no longer holds. +struct Past; + +impl Render for Past { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + div().w(px(480.0)).child(BrowserPreview::new( + "browser", + "localhost", + ["a.png", "b.png", "c.png"], + 3, + )) + } +} + +#[gpui::test] +fn a_frame_past_the_last_shows_none(cx: &mut TestAppContext) { + setup(cx); + let (_, cx) = cx.add_window_view(|_, _| Past); + settle(cx); } #[test] diff --git a/tasks/panics.md b/tasks/panics.md index 4a7999b..cf8ddae 100644 --- a/tasks/panics.md +++ b/tasks/panics.md @@ -38,7 +38,7 @@ Each item reads every site in its folders, fixes what the rules name, updates ca - [x] P21 Documents — `src/documents` - [x] P22 Collaboration — `src/collab` - [x] P23 AI Chat — `src/chat` -- [ ] P24 Agent — `src/agent` +- [x] P24 Agent — `src/agent` - [ ] P25 Generative — `src/generative` - [ ] P26 Media — `src/media` - [ ] P27 Files — `src/files` @@ -89,3 +89,4 @@ One line per item: what changed, what stayed and why, review rounds. - P21: block editor listeners painted for a block, field, cell or kind since gone log and change nothing, a dropped picture drag leaves undo alone; a visual block press or edit over text the owner replaced is dropped; nested images parse; a version or find hit past its list selects none. Review: 4 rounds. - P22: annotations and suggestions outside their text log and stay out, picks and decisions keeping the owner's index; `accept` returns `Option`; a gone pick, active thread or followed peer logs; yours at no count comes off; a masked field clamps a stale offset, so remote cursors and selections draw. Kept: a thread's comments, presence max, one list's order, empty marks, constant role and mode tables. Review: 3 rounds. - P23: a branch version past its total logs and shows a dash, its arrows off; a parameter outside its range logs and pegs; a passage's matches outside it log and stay out. Kept: one-value contracts (progress, scores, shapes, peaks, levels, limits, prices, capacity, format, hour), the composer's trigger rows and stream reveal by construction; `places` skips zero, so `decimals` meets no zero step. Review: 2 rounds. +- P24: a checkpoint or browser frame past its list logs and marks none; progress past its total and memory past its limit log and fill their bars, the text keeping the real counts; three `should_panic` tests became render tests. Kept: the artifact's preview-or-source builder check, one-value contracts (cost, cpu, pointer, totals and limits above zero), registry lookups over a constant table. Review: 1 round.