diff --git a/src/memos/mem_os/utils/default_config.py b/src/memos/mem_os/utils/default_config.py index de79d535d..0f87c2d4c 100644 --- a/src/memos/mem_os/utils/default_config.py +++ b/src/memos/mem_os/utils/default_config.py @@ -14,6 +14,28 @@ logger = logging.getLogger(__name__) +# Text-memory backends that the default-config helpers know how to wire up. +# Keep in sync with the Literal type hint on text_mem_type below. +_SUPPORTED_TEXT_MEM_TYPES: tuple[str, ...] = ("tree_text", "general_text") + + +def _validate_text_mem_type(text_mem_type: str) -> None: + """Reject unknown text_mem_type values with a clear ValueError. + + The default-config helpers only wire up a handful of text-memory backends + (``tree_text`` and ``general_text``). Historically the helper fell through + silently when the caller passed anything else — for example a typo like + ``"tree-text"`` coming through the ``MOS_TEXT_MEM_TYPE`` env var or the + MCP ``create_cube`` path — and crashed with a bare ``UnboundLocalError``. + + See issue #2441. + """ + if text_mem_type not in _SUPPORTED_TEXT_MEM_TYPES: + supported = ", ".join(repr(t) for t in _SUPPORTED_TEXT_MEM_TYPES) + raise ValueError( + f"Unsupported text_mem_type={text_mem_type!r}. Expected one of: {supported}." + ) + def get_default_config( openai_api_key: str, @@ -45,6 +67,10 @@ def get_default_config( mos = MOS(config) ``` """ + # Validate up front so both MOS and MemCube helpers fail with the same + # clear error when the caller (or an env var like MOS_TEXT_MEM_TYPE) hands + # in a typo. See issue #2441. + _validate_text_mem_type(text_mem_type) # Base OpenAI configuration openai_config = { @@ -148,6 +174,9 @@ def get_default_cube_config( Returns: GeneralMemCubeConfig: Complete MemCube configuration object """ + # Fail fast on invalid backends so the caller sees a clear error instead of + # an UnboundLocalError deeper in the function (issue #2441). + _validate_text_mem_type(text_mem_type) # Base OpenAI configuration openai_config = { @@ -230,6 +259,14 @@ def get_default_cube_config( "embedder": embedder_config, }, } + else: + # Defensive: _validate_text_mem_type above rejects anything else, but + # keep an explicit branch so future refactors cannot silently reintroduce + # the UnboundLocalError this file used to raise (issue #2441). + raise ValueError( + f"Unsupported text_mem_type={text_mem_type!r}. " + f"Expected one of: {', '.join(repr(t) for t in _SUPPORTED_TEXT_MEM_TYPES)}." + ) # Configure activation memory if enabled. # KV cache activation memory requires a local HuggingFace/vLLM model (it diff --git a/tests/mem_os/utils/__init__.py b/tests/mem_os/utils/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/mem_os/utils/test_default_config.py b/tests/mem_os/utils/test_default_config.py new file mode 100644 index 000000000..f68f4e2a6 --- /dev/null +++ b/tests/mem_os/utils/test_default_config.py @@ -0,0 +1,102 @@ +"""Test suite for src/memos/mem_os/utils/default_config.py. + +Focus: get_default_cube_config / get_default_config defensive behavior +when the caller passes an unknown text_mem_type value. + +Related issue: #2441. +""" + +import pytest + +from memos.mem_os.utils.default_config import ( + get_default_config, + get_default_cube_config, +) + + +class TestGetDefaultCubeConfigInvalidTextMemType: + """`get_default_cube_config` must raise a clear ValueError, not UnboundLocalError.""" + + def test_invalid_text_mem_type_raises_value_error(self): + """Passing a typo like 'tree-text' must raise ValueError (issue #2441).""" + with pytest.raises(ValueError) as exc_info: + get_default_cube_config( + openai_api_key="sk-test", + text_mem_type="tree-text", # typo, should not crash silently + ) + + message = str(exc_info.value) + # Error must name the offending value so operators can find their typo fast. + assert "tree-text" in message + # Error must list the accepted values so callers know what to use. + assert "tree_text" in message + assert "general_text" in message + + def test_invalid_text_mem_type_does_not_leak_unbound_local(self): + """The old bug (issue #2441) raised UnboundLocalError; guard against regression.""" + with pytest.raises(ValueError): + get_default_cube_config( + openai_api_key="sk-test", + text_mem_type="totally_unknown_backend", + ) + + def test_empty_text_mem_type_raises_value_error(self): + """Empty string should also fail loudly, not fall through to UnboundLocalError.""" + with pytest.raises(ValueError): + get_default_cube_config( + openai_api_key="sk-test", + text_mem_type="", # type: ignore[arg-type] + ) + + +class TestGetDefaultConfigInvalidTextMemType: + """`get_default_config` accepts the same text_mem_type parameter and should + validate it symmetrically so both entry points fail with the same clear error + when they receive garbage input. + + ``get_default_config`` and ``get_default_cube_config`` each call + ``_validate_text_mem_type`` independently (they do not share a delegation + path), so we mirror the same three regression cases here — a typo, an + unknown backend name, and the empty string — to keep coverage symmetric + across both entry points for issue #2441. + """ + + def test_invalid_text_mem_type_raises_value_error(self): + with pytest.raises(ValueError) as exc_info: + get_default_config( + openai_api_key="sk-test", + text_mem_type="tree-text", + ) + + message = str(exc_info.value) + assert "tree-text" in message + assert "tree_text" in message + assert "general_text" in message + + def test_invalid_text_mem_type_does_not_leak_unbound_local(self): + """Unknown backend names must fail with ValueError, not UnboundLocalError.""" + with pytest.raises(ValueError): + get_default_config( + openai_api_key="sk-test", + text_mem_type="totally_unknown_backend", + ) + + def test_empty_text_mem_type_raises_value_error(self): + """Empty string should also fail loudly, symmetric with the cube helper.""" + with pytest.raises(ValueError): + get_default_config( + openai_api_key="sk-test", + text_mem_type="", # type: ignore[arg-type] + ) + + +class TestGetDefaultCubeConfigValidTextMemType: + """Valid values continue to build a MemCube config end-to-end.""" + + def test_general_text_still_builds(self): + cfg = get_default_cube_config( + openai_api_key="sk-test", + text_mem_type="general_text", + user_id="issue2441_user", + ) + assert cfg.text_mem.backend == "general_text"