From f6fe94bc1af7e885f60c3c1ecd1d76d2bc8b45f4 Mon Sep 17 00:00:00 2001 From: Fini Date: Mon, 13 Jul 2026 21:41:45 +0800 Subject: [PATCH] fix(agent): explain the fresh-sandbox rule, and paint rich text at real widths A batch died on "header is not defined": the model referenced a const from the PREVIOUS batch's script, but every script runs in a fresh sandbox. The error now says so and points at the fix (reference the node by its id string), and the program-DSL contract states it up front. The transcript's markdown also painted every span at the wrap ESTIMATE (6.6px per unit) while skia draws real glyph advances, so each span after the first sat at a slightly wrong x - code chips drifted off their words and a sentence read as fragments. Paint now advances by the measured width; wrapping stays estimate-based, since it must stay backend-free. --- .../src/widgets/ai_chat_transcript_richtext.rs | 16 ++++++++++++++-- crates/op-mcp/src/script_runner.rs | 17 +++++++++++++++++ crates/op-orchestrator/src/prompt.rs | 3 +++ 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/crates/op-editor-ui/src/widgets/ai_chat_transcript_richtext.rs b/crates/op-editor-ui/src/widgets/ai_chat_transcript_richtext.rs index ed7b8f501..39fa294d2 100644 --- a/crates/op-editor-ui/src/widgets/ai_chat_transcript_richtext.rs +++ b/crates/op-editor-ui/src/widgets/ai_chat_transcript_richtext.rs @@ -220,13 +220,25 @@ pub(crate) fn paint_rich(cx: &mut PaintCx<'_>, theme: &Theme, lines: &[RichLine] } let mut x = origin.x + line.inset; for span in &line.spans { - let units: u32 = span.text.chars().map(char_display_units).sum(); - let width = units as f32 * CHAR_UNIT_PX; let (color, weight) = match span.style { SpanStyle::Body => (theme.muted_foreground, 400), SpanStyle::Strong => (theme.foreground, 700), SpanStyle::Code => (theme.foreground, 400), }; + // Advance by the REAL glyph width, not the wrap estimate: paint + // used the same 6.6px-per-unit budget the wrapper does, so every + // span after the first sat at a slightly wrong x — code chips + // drifted off their words and the sentence read as fragments + // (user report 2026-07-12). Wrapping stays estimate-based (it must + // be backend-free and deterministic); only paint measures. + let measured = cx + .backend + .measure_text_weighted(&span.text, BODY_FONT, weight); + let width = if measured > 0.0 { + measured + } else { + span.text.chars().map(char_display_units).sum::() as f32 * CHAR_UNIT_PX + }; if span.style == SpanStyle::Code { cx.backend.fill_round_rect( Rect::xywh( diff --git a/crates/op-mcp/src/script_runner.rs b/crates/op-mcp/src/script_runner.rs index 7cb377130..0c6f87514 100644 --- a/crates/op-mcp/src/script_runner.rs +++ b/crates/op-mcp/src/script_runner.rs @@ -426,12 +426,29 @@ pub(crate) fn repair_truncated_script(script: &str) -> Option { Some(repaired) } +/// A bare "x is not defined" tells the model nothing about WHY. Every script +/// runs in a FRESH sandbox, so a variable that held a node in an earlier batch +/// is gone — the model must reference that node by its id STRING. Say so +/// (measured 2026-07-12: a batch died on `header is not defined`, where +/// `header` was a `const` from the previous batch's script). +fn explain_reference_error(msg: &str) -> Option { + let name = msg.strip_suffix(" is not defined")?; + Some(format!( + "script error: {msg}. Each script runs in a FRESH sandbox — a variable \ + from an earlier batch no longer exists. Reference nodes created in an \ + earlier batch by their id STRING instead: I(\"n12\", {{…}}), not I({name}, {{…}})." + )) +} + fn describe_js_error(ctx: &rquickjs::Ctx<'_>, err: rquickjs::Error) -> String { if err.is_exception() { let caught = ctx.catch(); if let Some(exc) = caught.as_exception() { let msg = exc.message().unwrap_or_default(); if !msg.is_empty() { + if let Some(explained) = explain_reference_error(&msg) { + return explained; + } return format!("script error: {msg}"); } } diff --git a/crates/op-orchestrator/src/prompt.rs b/crates/op-orchestrator/src/prompt.rs index b79bcccd4..31ba9cbb8 100644 --- a/crates/op-orchestrator/src/prompt.rs +++ b/crates/op-orchestrator/src/prompt.rs @@ -82,6 +82,9 @@ data array. PREFER a loop over copy-pasting near-identical I(...) calls. Each node object starts with type ("frame"/"text"/"rectangle"/"ellipse"/"path"/"icon_font") and uses camelCase props (cornerRadius, fontSize, fontWeight, justifyContent, alignItems, clipContent). Do NOT set x/y on children inside layout frames. +Each script runs in a FRESH sandbox: variables from an EARLIER batch do not exist. To attach +to a node an earlier batch created, pass its id STRING — I("n12", {...}) — never a `const` from +that batch. Ids come back in the batch result. EVERY frame with children MUST declare layout ("vertical" or "horizontal"; "none" for an absolute stack). A section that holds a title and a card rail is layout:"vertical" — omitting it stacks by default, but say it, because a row is only ever a row when you write it.