Fix #2441: get_default_cube_config crashes with UnboundLocalError on invalid text_mem_type - #2445
Open
Memtensor-AI wants to merge 2 commits into
Open
Memtensor-AI wants to merge 2 commits into
Memtensor-AI wants to merge 2 commits into
Conversation
get_default_cube_config only assigned text_mem_config inside the 'tree_text' and 'general_text' branches. Any other value (a typo such as 'tree-text', or an unknown value coming through the MOS_TEXT_MEM_TYPE env var or the MCP create_cube path) fell through to the cube_config_dict assembly and crashed with a bare UnboundLocalError: UnboundLocalError: local variable 'text_mem_config' referenced before assignment Add a private _validate_text_mem_type helper that raises a clear ValueError naming the offending value and the accepted set, call it at the top of both get_default_config and get_default_cube_config so the two entry points fail identically, and add an explicit else branch inside the cube helper so a future refactor cannot silently reintroduce the UnboundLocalError. Adds tests/mem_os/utils/test_default_config.py with red-then-green cases plus a happy-path smoke test. Fixes MemTensor#2441
Memtensor-AI
added
ai:generated
Collaborator
Author
🤖 Open Code ReviewTarget: PR #2445 ✅ OpenCodeReview: Review complete: 0 finding(s) across 3 selected item(s). Generated by cloud-assistant via Open Code Review. |
Collaborator
Author
🔧 Open Code Review requested Agent fixOpen Code Review found 1 issue(s). I have resumed the development Agent to fix them.
The Agent will push a new commit to this PR branch. OCR will recheck after the commit is pushed. |
Extend TestGetDefaultConfigInvalidTextMemType with the same three regression cases already exercised by TestGetDefaultCubeConfigInvalidTextMemType: a typo, a fully unknown backend name, and the empty string. get_default_config and get_default_cube_config each call _validate_text_mem_type independently (there is no shared delegation path), so symmetric coverage is required to guard both entry points against the UnboundLocalError regression tracked in issue MemTensor#2441. Addresses OCR review on PR MemTensor#2445.
Collaborator
Author
✅ Automated Test Results: PASSEDAll tests passed (8/8 executed). memos_github_open_source/smoke: 1/1, memos_python_core/changed-repo-python: 7/7. Duration: 10s Branch: |
Memtensor-AI
added
status:ready
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes issue #2441:
get_default_cube_config(and its siblingget_default_config) insrc/memos/mem_os/utils/default_config.pyno longer crashes with a bareUnboundLocalErrorwhen a caller passes an unsupportedtext_mem_typesuch as the reported"tree-text"typo. Both helpers now call a new_validate_text_mem_typeguard at entry and raise a clearValueErrornaming the offending value and the accepted set ("tree_text","general_text"), so misconfigurations coming through theMOS_TEXT_MEM_TYPEenv var or the MCPcreate_cubepath fail loudly with an actionable message. As a defence-in-depth,get_default_cube_config'sif/elifchain gained an explicitelsebranch that re-raises the sameValueError, so a future refactor cannot silently reintroduce the fall-through.Added
tests/mem_os/utils/test_default_config.pywith five cases covering the reported reproducer ("tree-text"), other unknown backends, empty strings, symmetric behavior onget_default_config, and a happy-path smoke test for"general_text". All new tests pass; the fulltests/mem_os/suite (41 tests) continues to pass;ruff checkandruff formatare clean on the touched files. No public API, schema, or OpenAPI contract changes; no new dependencies.Related Issue (Required): Fixes #2441
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Automated tests are pending.
Checklist
@WeiminLee please review this PR.
Reviewer Checklist