From e1962480580b4b256242f239233c242a314cfa8d Mon Sep 17 00:00:00 2001
From: Alex <a@tushynski.me>
Date: Wed, 5 Aug 2026 11:57:27 +0100
Subject: [PATCH] fix(workflows): redact CEL error values by default; tokenize
 attachments once
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two follow-ups from PR review.

_fit_attachment_text tokenized the extraction twice on the oversized
path — once for the budget check and once for chars_per_token. That is a
full BPE pass over the whole document (~12ms per 250k chars) on the hot
path of every attachment turn. Compute it once.

The CEL error summarizer kept quoted fragments that looked like
identifiers, on the theory that those are variable names rather than
data. They are not distinguishable: celpy interpolates state values into
messages (`StringType('SECRET')`), and a one-word user query such as
`invoice` is a valid identifier, so it survived redaction and reached
both the log line and — since config errors now bypass
sanitize_api_error — the user. The earlier test passed only because its
sample value contained hyphens.

Quoted fragments are now redacted by default, allowing only the two
positions known to hold names rather than data: the variable in
"undeclared reference to 'x'" and the class in "<class 'ValueError'>".
Tests cover identifier-shaped values, that the exception class survives,
the truncation backstop, and the empty-expression guard.
---
 application/agents/workflows/cel_evaluator.py | 15 ++++++--
 application/llm/openai.py                     |  7 +++-
 tests/agents/test_cel_evaluator.py            | 38 +++++++++++++++++--
 3 files changed, 51 insertions(+), 9 deletions(-)

diff --git a/application/agents/workflows/cel_evaluator.py b/application/agents/workflows/cel_evaluator.py
index 2a7a1748..121b5a28 100644
--- a/application/agents/workflows/cel_evaluator.py
+++ b/application/agents/workflows/cel_evaluator.py
@@ -52,12 +52,19 @@ def _summarize_cel_error(exc: Exception) -> str:
     if marker in text:
         text = text.split(marker, 1)[0]
     text = text.strip().lstrip("(").strip()
-    # Quoted fragments in celpy messages are either the offending identifier
-    # (safe, and the useful part) or a state value (not ours to echo). Keep
-    # bare identifiers, redact anything else.
+
+    # Redact quoted fragments by DEFAULT. celpy interpolates state values into
+    # messages (``StringType('SECRET')``), and a value that happens to look
+    # like an identifier is indistinguishable from a variable name — so allow
+    # only the two positions that are known to hold names, not data: the
+    # offending variable in "undeclared reference to 'x'", and the exception
+    # class in "<class 'ValueError'>".
     def _redact(match: "re.Match[str]") -> str:
         inner = match.group(1)
-        return f"'{inner}'" if inner.isidentifier() else "'…'"
+        prefix = text[max(0, match.start() - 14):match.start()]
+        if prefix.endswith("reference to ") or prefix.endswith("<class "):
+            return f"'{inner}'"
+        return "'…'"
 
     text = re.sub(r"'([^']*)'", _redact, text)
     text = text.strip().strip('"').strip()
diff --git a/application/llm/openai.py b/application/llm/openai.py
index 466ea0b1..3bd65446 100644
--- a/application/llm/openai.py
+++ b/application/llm/openai.py
@@ -1742,9 +1742,12 @@ class OpenAILLM(BaseLLM):
         # Half the window: the prompt, history and the answer share the rest.
         budget = int(limit * 0.5) if limit else 24000
         try:
-            if num_tokens_from_string(content) <= budget:
+            # Tokenize once: this is a full BPE pass over the whole extraction
+            # (~12ms per 250k chars), on the hot path of every attachment turn.
+            token_count = num_tokens_from_string(content)
+            if token_count <= budget:
                 return content
-            chars_per_token = len(content) / max(num_tokens_from_string(content), 1)
+            chars_per_token = len(content) / max(token_count, 1)
             keep = max(int(budget * chars_per_token * 0.95), 0)
         except Exception:
             keep = budget * 4
diff --git a/tests/agents/test_cel_evaluator.py b/tests/agents/test_cel_evaluator.py
index d42c274b..6592bc6a 100644
--- a/tests/agents/test_cel_evaluator.py
+++ b/tests/agents/test_cel_evaluator.py
@@ -204,12 +204,44 @@ class TestCelErrorMessages:
         assert "NameContainer" not in message
 
     @pytest.mark.unit
-    def test_error_does_not_echo_state_values(self):
+    @pytest.mark.parametrize(
+        "secret",
+        [
+            "SUPER-SECRET-VALUE",
+            # An identifier-shaped value is indistinguishable from a variable
+            # name, so it must be redacted too — this is the realistic case
+            # (a one-word user query) and the earlier redaction let it through.
+            "invoice",
+            "SECRET",
+        ],
+    )
+    def test_error_does_not_echo_state_values(self, secret):
         """Config errors skip sanitize_api_error now, so they must not carry
         state contents — a shared agent's runner is not its owner."""
         with pytest.raises(CelEvaluationError) as exc:
-            evaluate_cel("int(query)", {"query": "SUPER-SECRET-VALUE"})
-        assert "SUPER-SECRET-VALUE" not in str(exc.value)
+            evaluate_cel("int(query)", {"query": secret})
+        assert secret not in str(exc.value)
+
+    @pytest.mark.unit
+    def test_error_keeps_the_exception_class(self):
+        """Redaction must not swallow the one safe diagnostic."""
+        with pytest.raises(CelEvaluationError) as exc:
+            evaluate_cel("int(query)", {"query": "nope"})
+        assert "ValueError" in str(exc.value)
+
+    @pytest.mark.unit
+    def test_long_error_is_truncated(self):
+        """Backstop for celpy messages that survive activation-stripping."""
+        from application.agents.workflows.cel_evaluator import _summarize_cel_error
+
+        summary = _summarize_cel_error(Exception("word " * 200))
+        assert len(summary) <= 200
+        assert summary.endswith("…")
+
+    @pytest.mark.unit
+    def test_validate_rejects_empty_expression(self):
+        with pytest.raises(CelEvaluationError, match="Empty expression"):
+            validate_cel_expression("   ")
 
     @pytest.mark.unit
     @pytest.mark.parametrize(