fix(errors): address diagnostic review feedback

Signed-off-by: Paul Furgale <pfurgale@nvidia.com>

Author: furgalep

CHANGELOG.md

@@ -7,8 +7,10 @@ to follow semantic versioning.
 ## [Unreleased]
 
 - Breaking: custom CodeAct error formatters must implement
-  `format(error, code=None, *, line_offset=0, formatted_error="", max_error=None, tail_chars=None)`.
+  `format(error, code=None, *, line_offset=0, max_error=None, tail_chars=None)`.
   Reduced legacy signatures are no longer supported.
+- Breaking: sandboxed user-code failures are exposed as `SandboxExecutionError`;
+  inspect `original_type`, `original_error`, and `diagnostic` for worker-side details.
 - Initial public release of NVIDIA Object-Oriented Agents (NOOA).
 - Security: MCP server configurations no longer expand host environment variables
   from `${VAR}` placeholders. Trusted caller code must resolve secrets and pass

src/nooa/errors/formatting.py

@@ -49,15 +49,7 @@
 
 # Internal wrapper function names to replace with <module>
 _WRAPPER_NAMES = ("__repl_wrapper__", "__wrapper__")
-_TRUNCATED_DIAGNOSTIC_PATTERN = re.compile(
-    r"\A<truncated-output>\n"
-    r"Output too large \(([\d,]+) chars\)\. "
-    r"Showing first ([\d,]+) and last ([\d,]+) chars\.\n"
-    r"The ([\d,]+) chars in the middle are not recoverable\.\n\n"
-    r"(.*?)\n\n\.\.\. ([\d,]+) chars not shown \.\.\.\n\n"
-    r"(.*)\n</truncated-output>\Z",
-    re.DOTALL,
-)
+
 
 # ---------------------------------------------------------------------------
 # Targeted model-recovery hints
@@ -78,19 +70,17 @@
 _HEREDOC_RE = re.compile(r"<<-?\s*['\"]?\w+['\"]?")
 
 # Appended to SyntaxErrors that look like heredoc-in-quoted-string failures.
-_HEREDOC_HINT = """\
-Hint: this looks like a bash heredoc (`<<...`) embedded in a single- or
-double-quoted Python string. Python rejects multi-line single/double-quoted
-strings, which is why the parser errored before reaching the heredoc body.
+_HEREDOC_HINT = '''\
+Hint: this looks like a shell heredoc (`<<...`) embedded in a single- or
+multiple-line Python string. Put the complete command in a triple-quoted
+string, then pass that value using the callable's documented API:
 
-Fix 1 (preferred) — use a triple-quoted Python string so newlines are legal:
-    await shell.run(\"\"\"cat <<EOF
-    content
-    EOF\"\"\")
+    command = """cat <<'EOF'
+content
+EOF"""
 
-Fix 2 — write the script to a file first, then run it:
-    await shell.write("/tmp/script.sh", script_text)
-    await shell.run("bash /tmp/script.sh")"""
+Use `doc(...)` to inspect the available command runner and how it accepts
+commands or standard input.'''
 
 
 # Call-shape TypeError messages — a method/function called with the wrong
@@ -498,77 +488,11 @@ def _hard_bound_text(text: str, limit: int, *, closing: str = "") -> str:
     return marker[:limit]
 
 
-def _bound_preformatted_diagnostic(
-    text: str,
-    max_error: int | None,
-    tail_chars: int | None,
-) -> str:
-    """Bound trusted backend text without nesting a valid truncation envelope."""
-    text = text.rstrip()
-    limit, tail = _diagnostic_budget(max_error, tail_chars)
-    if len(text) <= limit:
-        return text
-
-    match = _TRUNCATED_DIAGNOSTIC_PATTERN.fullmatch(text)
-    if match is None:
-        return _bound_diagnostic(text, max_error, tail_chars)
-
-    total_text, old_head_text, old_tail_text, dropped_text, head, repeated_text, tail_text = (
-        match.groups()
-    )
-    try:
-        total = int(total_text.replace(",", ""))
-        old_head_chars = int(old_head_text.replace(",", ""))
-        old_tail_chars = int(old_tail_text.replace(",", ""))
-        dropped = int(dropped_text.replace(",", ""))
-        repeated_dropped = int(repeated_text.replace(",", ""))
-    except ValueError:
-        return _bound_diagnostic(text, max_error, tail_chars)
-    if (
-        old_head_chars != len(head)
-        or old_tail_chars != len(tail_text)
-        or dropped != repeated_dropped
-        or total != old_head_chars + old_tail_chars + dropped
-    ):
-        return _bound_diagnostic(text, max_error, tail_chars)
-
-    desired_tail = limit // 2 if tail is None else tail
-    desired_head = limit - desired_tail
-    if old_head_chars <= desired_head and old_tail_chars <= desired_tail:
-        return _hard_bound_text(
-            text,
-            limit + 1_024,
-            closing="\n</truncated-output>",
-        )
-
-    # The original middle is already gone, so retain as much of each requested
-    # window as remains available and accurately describe the larger omission.
-    bounded_head = head[:desired_head]
-    bounded_tail = tail_text[-desired_tail:] if desired_tail else ""
-    new_dropped = total - len(bounded_head) - len(bounded_tail)
-    rebuilt = (
-        "<truncated-output>\n"
-        f"Output too large ({total:,} chars). "
-        f"Showing first {len(bounded_head):,} and last {len(bounded_tail):,} chars.\n"
-        f"The {new_dropped:,} chars in the middle are not recoverable.\n\n"
-        f"{bounded_head}\n\n"
-        f"... {new_dropped:,} chars not shown ...\n\n"
-        f"{bounded_tail}\n"
-        "</truncated-output>"
-    )
-    return _hard_bound_text(
-        rebuilt,
-        limit + 1_024,
-        closing="\n</truncated-output>",
-    )
-
-
 class ErrorFormatter(Protocol):
     """Preferred strategy formatter contract.
 
-    Implementations receive trusted backend-rendered diagnostics and the resolved
-    per-call error budget. Custom strategy formatters must implement this complete
-    contract.
+    Implementations receive the exception, source context, and resolved per-call
+    error budget. Custom strategy formatters must implement this complete contract.
     """
 
     def format(
@@ -577,7 +501,6 @@ def format(
         code: str | None = None,
         *,
         line_offset: int = 0,
-        formatted_error: str = "",
         max_error: int | None = None,
         tail_chars: int | None = None,
     ) -> str: ...
@@ -601,7 +524,6 @@ def format(
         code: str | None = None,
         *,
         line_offset: int = 0,
-        formatted_error: str = "",
         max_error: int | None = None,
         tail_chars: int | None = None,
     ) -> str:
@@ -611,9 +533,6 @@ def format(
             error: The exception to format.
             code: Optional source code (used for syntax errors if text is missing).
             line_offset: Number of wrapper lines to subtract from line numbers.
-            formatted_error: Trusted diagnostic already rendered by an execution
-                backend. It bypasses local rendering so worker-local traceback
-                and source context are preserved.
             max_error: Maximum retained diagnostic characters. ``None`` uses the
                 framework default.
             tail_chars: Characters reserved for the retained tail. ``None`` uses
@@ -622,10 +541,15 @@ def format(
         Returns:
             Formatted error string with adjusted line numbers.
         """
-        if formatted_error:
-            # Worker-generated truncation envelopes have already applied the
-            # resolved policy; preserve one envelope while retaining a hard cap.
-            return _bound_preformatted_diagnostic(formatted_error, max_error, tail_chars)
+        from nooa.runtime.sandbox.errors import SandboxExecutionError
+
+        if isinstance(error, SandboxExecutionError):
+            limit, _ = _diagnostic_budget(max_error, tail_chars)
+            return _hard_bound_text(
+                error.diagnostic.rstrip() or str(error),
+                limit + 1_024,
+                closing="\n</truncated-output>",
+            )
 
         if isinstance(error, SyntaxError):
             formatted = self._format_syntax_error(error, code, line_offset)
@@ -699,7 +623,6 @@ def format_error_for_llm(
     code: str | None = None,
     *,
     line_offset: int = 0,
-    formatted_error: str = "",
     max_error: int | None = None,
     tail_chars: int | None = None,
 ) -> str:
@@ -719,11 +642,6 @@ def format_error_for_llm(
         line_offset: Number of wrapper lines to subtract from line numbers.
             This compensates for lines added by the async wrapper (e.g.,
             "async def __repl_wrapper__():", "try:", etc.).
-        formatted_error: Optional preformatted diagnostic produced by a trusted
-            execution backend such as the sandbox worker. When non-empty, it
-            bypasses local formatting (including ``code``, ``line_offset``, and
-            bad-call hint handling). The producer is responsible for applying any
-            required line adjustment.
         max_error: Maximum retained diagnostic characters. ``None`` uses the
             framework default.
         tail_chars: Characters reserved for the retained tail. ``None`` uses
@@ -737,7 +655,6 @@ def format_error_for_llm(
             error,
             code,
             line_offset=line_offset,
-            formatted_error=formatted_error,
             max_error=max_error,
             tail_chars=tail_chars,
         )

src/nooa/events.py

@@ -663,11 +663,6 @@ class ExecutionResult(BaseModel):
     stdout: str = Field(default="", description="Captured stdout from execution")
     stderr: str = Field(default="", description="Captured stderr from execution")
     error: Exception | None = Field(default=None, description="Exception if execution failed")
-    formatted_error: str = Field(
-        default="",
-        description="Parent-side formatted diagnostic reconstructed from a sandbox worker",
-        exclude=True,
-    )
     signal: ExecutionSignal | None = Field(
         default=None, description="Control flow signal (not an error), e.g. return_result()"
     )

src/nooa/runtime/event_backend.py

@@ -301,6 +301,15 @@ def remove_active_tag(self, tag: str) -> bool:
     def all_events(self) -> Iterator[EventBase]:
         return iter(self._events)
 
+    def iter_events(
+        self,
+        *,
+        event_type: str | None = None,
+        newest_first: bool = False,
+    ) -> Iterator[EventBase]:
+        events = reversed(self._events) if newest_first else iter(self._events)
+        return (event for event in events if event_type is None or event.event_type == event_type)
+
     def find_tag(self, event: EventBase) -> str | None:
         for tag, e in self._tag_to_event.items():
             if e is event or e.id == event.id:

src/nooa/runtime/event_manager.py

@@ -14,8 +14,8 @@
 import logging
 import re
 from collections import defaultdict
-from collections.abc import Awaitable, Callable
-from typing import TYPE_CHECKING, Any
+from collections.abc import Awaitable, Callable, Iterable, Iterator
+from typing import TYPE_CHECKING, Any, cast
 
 from nooa.context_blocks import EventStatus
 from nooa.context_blocks.models import Role
@@ -353,6 +353,7 @@ def filter(
         *,
         type: str | None = None,
         call_id: str | None = None,
+        execution_status: str | None = None,
         query: str | None = None,
         regex: bool = False,
         limit: int | None = None,
@@ -362,37 +363,57 @@ def filter(
         Args:
             type: Event type filter (e.g., "Task", "PythonOutput")
             call_id: Call ID filter (matches metadata.call_id)
+            execution_status: PythonOutput status filter (e.g. "error" or "complete")
             query: Text search (case-insensitive substring, or regex if regex=True)
             regex: If True, treat query as regex pattern
             limit: Maximum results (most recent first when limit < total)
 
         Returns:
             List of matching events.
         """
-        events = list(self._backend.all_events())
-
-        # Apply type filter
-        if type is not None:
-            events = [e for e in events if e.event_type == type]
-
-        # Apply call_id filter
-        if call_id is not None:
-            events = [e for e in events if e.metadata.get("call_id") == call_id]
-
-        # Apply text/regex filter
-        if query is not None:
-            if regex:
-                pattern = re.compile(query, re.IGNORECASE)
-                events = [e for e in events if pattern.search(self._get_searchable_text(e))]
-            else:
-                query_lower = query.lower()
-                events = [e for e in events if query_lower in self._get_searchable_text(e).lower()]
+        if limit is not None and limit <= 0:
+            return []
 
-        # Apply limit (from end = most recent)
-        if limit is not None and len(events) > limit:
-            events = events[-limit:]
+        newest_first = limit is not None
+        source: Iterable[EventBase]
+        iter_events = getattr(self._backend, "iter_events", None)
+        if newest_first and callable(iter_events):
+            optimized_iterator = cast(Callable[..., Iterator[EventBase]], iter_events)
+            source = optimized_iterator(event_type=type, newest_first=True)
+        else:
+            # ``iter_events`` is an optional optimization so existing custom
+            # EventBackend implementations remain compatible. Unlimited queries
+            # retain the backend's established snapshot behavior.
+            events = self._backend.all_events()
+            source = reversed(list(events)) if newest_first else events
+        matches: list[EventBase] = []
+        pattern = re.compile(query, re.IGNORECASE) if query is not None and regex else None
+        query_lower = query.lower() if query is not None and not regex else None
+
+        for event in source:
+            if type is not None and event.event_type != type:
+                continue
+            if call_id is not None and event.metadata.get("call_id") != call_id:
+                continue
+            if execution_status is not None:
+                status = getattr(event, "execution_status", None)
+                status_value = getattr(status, "value", status)
+                if status_value != execution_status:
+                    continue
+            if pattern is not None and not pattern.search(self._get_searchable_text(event)):
+                continue
+            if (
+                query_lower is not None
+                and query_lower not in self._get_searchable_text(event).lower()
+            ):
+                continue
+            matches.append(event)
+            if limit is not None and len(matches) >= limit:
+                break
 
-        return events
+        if newest_first:
+            matches.reverse()
+        return matches
 
     def _get_searchable_text(self, event: EventBase) -> str:
         """Extract searchable text from an event's public fields."""

src/nooa/runtime/events.py

@@ -29,6 +29,7 @@ class EventsApi(Skill):
         events.query(limit=50)                      # recent 50
         events.query(type="Task")                   # all task events
         events.query(type="PythonOutput")           # execution outputs
+        events.query(type="PythonOutput", execution_status="error", limit=1)
         events.query(call_id="abc123")              # events for one call
         events.query(query="error")                 # text search
         events.query(query="error.*db", regex=True) # regex search
@@ -45,15 +46,13 @@ class EventsApi(Skill):
         children = events[summary.children_tags]
 
     Examples:
-        # Find the last standalone Error event
+        # Find the most recent strategy/retry Error event
         errors = events.query(type="Error", limit=1)
 
-        # Execution failures are PythonOutput events, not Error events
-        failed_outputs = [
-            output
-            for output in events.query(type="PythonOutput")
-            if output.execution_status.value == "error"
-        ]
+        # Cell failures are PythonOutput events, not standalone Error events
+        failed_outputs = events.query(
+            type="PythonOutput", execution_status="error", limit=1
+        )
 
         # Get all outputs from current call
         outputs = events.query(type="PythonOutput", call_id=call_id)
@@ -83,6 +82,7 @@ def query(
         *,
         type: str | None = None,
         call_id: str | None = None,
+        execution_status: str | None = None,
         query: str | None = None,
         regex: bool = False,
         limit: int | None = None,
@@ -95,6 +95,7 @@ def query(
         Args:
             type: Event type filter (e.g., "Task", "PythonOutput", "ToolCallEvent")
             call_id: Call ID filter (matches metadata.call_id)
+            execution_status: PythonOutput status filter (e.g. "error" or "complete")
             query: Text search (case-insensitive substring, or regex if regex=True)
             regex: If True, treat query as regex pattern
             limit: Maximum results (most recent first when limit < total)
@@ -114,6 +115,7 @@ def query(
         return self._manager.filter(
             type=type,
             call_id=call_id,
+            execution_status=execution_status,
             query=query,
             regex=regex,
             limit=limit,