Fix #2115: fix: SimpleTextSplitter fallback 在文件解析链路中报错：缺少 protect_urls#2116
Fix #2115: fix: SimpleTextSplitter fallback 在文件解析链路中报错：缺少 protect_urls#2116Memtensor-AI wants to merge 2 commits into
SimpleTextSplitter fallback 在文件解析链路中报错：缺少 protect_urls#2116Conversation
) `SimpleTextSplitter._simple_split_text()` called `self.protect_urls` and `self.restore_urls`, methods defined only on `BaseChunker`. Since `SimpleTextSplitter` does not inherit `BaseChunker`, every call raised `AttributeError: 'SimpleTextSplitter' object has no attribute 'protect_urls'`. The multi-modal file-parsing pipeline swallowed the error and fell back to returning the whole text as a single chunk, producing ~5.8k noisy log rows on ACK where langchain_text_splitters is missing and the fallback branch is actually exercised. Extract the URL protect/restore helpers into a small `URLProtectionMixin` in `chunkers/base.py`; have both `BaseChunker` and `SimpleTextSplitter` inherit it. This preserves BaseChunker's public API (mixin methods are inherited transparently), keeps SimpleTextSplitter's constructor and return type unchanged, and shares a single URL regex between the two paths. Add regression tests in tests/chunkers/test_simple_chunker.py covering short/long input, empty input, no-URL text, and parametrised (chunk_size, overlap) combinations to ensure the fallback never raises again.
Memtensor-AI
added
ai:generated
🤖 Open Code ReviewTarget: PR #2116 ✅ OpenCodeReview: No comments generated. Looks good to me. Generated by cloud-assistant via Open Code Review. |
🔧 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. |
Address OCR review on MemTensor#2116: the placeholder-leak assertion in tests/chunkers/test_simple_chunker.py hardcoded the string `'__URL_'`, which duplicates an implementation detail of `URLProtectionMixin.protect_urls` (formatted as `f'__URL_{len(url_map)}__'`). If the prefix ever changed in `base.py`, the assertion would silently keep passing while no longer catching real placeholder leaks. Expose the prefix as a class-level constant `URLProtectionMixin._URL_PLACEHOLDER_PREFIX = "__URL_"`, use it inside `protect_urls`, and import it in the test so the two paths stay in sync automatically. No behavior change: placeholders keep the same textual form (`__URL_<n>__`), so both the current base chunker and the fallback `SimpleTextSplitter` produce identical output to before.
✅ Automated Test Results: PASSEDAll tests passed (69/69 executed). memos_github_open_source/smoke: 1/1, memos_local_plugin/unit: 23/23, memos_python_core/changed-repo-python: 45/45. Duration: 12s [advisory, non-gating] AI-generated tests on branch test/auto-gen-e062b03201f62d58-20260716160757: 102/103 passed, 1 failed — these do NOT affect the PR verdict; review the branch manually. Branch: |
Memtensor-AI
added
status:ready
WeiminLee
left a comment
WeiminLee
left a comment
There was a problem hiding this comment.
LGTM. Agent addressed the OCR finding by extracting _URL_PLACEHOLDER_PREFIXto URLProtectionMixin, tests aligned correctly. CI/AutoTest all passed, ready to merge.
3 participants
Description
Fixes issue #2115:
SimpleTextSplitterfallback in the multi-modal file-parsing pipeline was raisingAttributeError: 'SimpleTextSplitter' object has no attribute 'protect_urls'. Root cause:SimpleTextSplitter._simple_split_text()callsself.protect_urls/self.restore_urls, but those helpers live onBaseChunkerandSimpleTextSplitterdoes not inherit from it. On ACK (wherelangchain_text_splittersis missing and the fallback is actually exercised) this produced ~5.8k noisy log rows and silently degraded chunking to a single-chunk pass-through.The fix extracts
protect_urls/restore_urlsinto a smallURLProtectionMixininsrc/memos/chunkers/base.py;BaseChunkernow inherits it (behavioural no-op — same public API), andSimpleTextSplitterinsrc/memos/chunkers/simple_chunker.pyalso inherits it, so the two paths share one URL regex and one placeholder scheme.SimpleTextSplitter's constructor signature andchunk() -> list[str]return type are unchanged; no factory / config / OpenAPI changes are required.Verification: 7 new regression tests in
tests/chunkers/test_simple_chunker.py(short/long input with URL, empty / whitespace input, no-URL text, parametrised chunk sizes) — all 7 failed before the fix with the exact AttributeError, all 7 pass after. Broaderpytest tests/chunkers/ tests/mem_reader/→ 63 pass; the 2 unrelated failures (test_doc_local_file,test_doc_mixed) fail identically on the pristine branch with the change stashed and are caused by the sandbox missingmarkitdown[all].ruff check+ruff format --checkare green on all 3 changed files.Working branch pushed to origin; opsp task + verification report archived to memos-autodev-specs main. Ready for reviewer @WeiminLee.
Related Issue (Required): Fixes #2115
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Not run; documentation-only change.
Checklist
@WeiminLee please review this PR.
Reviewer Checklist
SimpleTextSplitterfallback 在文件解析链路中报错：缺少protect_urls#2115