From 626c850ccb2c25be92a9ce575886a85f54e8683a Mon Sep 17 00:00:00 2001 From: jinliyl <6469360+jinliyl@users.noreply.github.com> Date: Mon, 24 Aug 2026 18:59:53 +0800 Subject: [PATCH 1/2] fix(daily-paper): sanitize Unicode surrogates (#487) * fix(daily-paper): sanitize Unicode surrogates * fix(daily-paper): sanitize analysis workflow state --- reme/steps/cookbook/daily_paper/_common.py | 28 +++++++- reme/steps/cookbook/daily_paper/analyze.py | 13 ++-- tests/unit/test_daily_paper.py | 81 +++++++++++++++++++++- 3 files changed, 111 insertions(+), 11 deletions(-) diff --git a/reme/steps/cookbook/daily_paper/_common.py b/reme/steps/cookbook/daily_paper/_common.py index 7d8dcd33..72ec0a90 100644 --- a/reme/steps/cookbook/daily_paper/_common.py +++ b/reme/steps/cookbook/daily_paper/_common.py @@ -22,6 +22,7 @@ _FRONTMATTER_PATTERN = re.compile(r"^---\s*\n.*?\n---\s*\n", re.DOTALL) _MARKDOWN_HEADING_PATTERN = re.compile(r"^#+\s*") _UNSAFE_FILENAME_CHARS = re.compile(r'[<>:"/\\|?*\x00-\x1f]') _CHINESE_PATTERN = re.compile(r"[\u3400-\u4dbf\u4e00-\u9fff]") +_SURROGATE_PATTERN = re.compile(r"[\ud800-\udfff]") _OutputT = TypeVar("_OutputT", bound=BaseModel) @@ -36,9 +37,28 @@ def strip_frontmatter(body: str) -> str: return _FRONTMATTER_PATTERN.sub("", body.strip(), count=1).strip() +def replace_surrogates(value: str) -> str: + """Replace invalid Unicode surrogate code points with replacement characters.""" + return _SURROGATE_PATTERN.sub("\ufffd", value) + + +def _replace_surrogates_recursive(value: Any) -> Any: + """Replace surrogates in strings nested in frontmatter metadata.""" + if isinstance(value, str): + return replace_surrogates(value) + if isinstance(value, dict): + return {_replace_surrogates_recursive(key): _replace_surrogates_recursive(item) for key, item in value.items()} + if isinstance(value, list): + return [_replace_surrogates_recursive(item) for item in value] + if isinstance(value, tuple): + return tuple(_replace_surrogates_recursive(item) for item in value) + return value + + def normalize_chinese_title(raw: str, fallback: str) -> str: """Return one safe Chinese title that can also be used as the filename stem.""" - title = _MARKDOWN_HEADING_PATTERN.sub("", str(raw or "").strip()) + title = replace_surrogates(str(raw or "").strip()) + title = _MARKDOWN_HEADING_PATTERN.sub("", title) if title.lower().endswith(".md"): title = title[:-3] title = _UNSAFE_FILENAME_CHARS.sub("-", title) @@ -92,7 +112,7 @@ async def write_atomic(path: Path, content: str | bytes) -> None: lock = await get_path_lock(path) async with lock: temp_path = path.with_name(f".{path.name}.{uuid4().hex}.tmp") - payload = content.encode("utf-8") if isinstance(content, str) else content + payload = replace_surrogates(content).encode("utf-8") if isinstance(content, str) else content try: async with aiofiles.open(temp_path, "wb") as stream: await stream.write(payload) @@ -104,7 +124,9 @@ async def write_atomic(path: Path, content: str | bytes) -> None: async def write_markdown(path: Path, body: str, metadata: dict[str, Any]) -> None: """Serialize a frontmatter Markdown document atomically.""" - rendered = frontmatter.dumps(frontmatter.Post(body.strip(), **metadata)) + safe_body = replace_surrogates(body).strip() + safe_metadata = _replace_surrogates_recursive(metadata) + rendered = frontmatter.dumps(frontmatter.Post(safe_body, **safe_metadata)) await write_atomic(path, rendered if rendered.endswith("\n") else f"{rendered}\n") diff --git a/reme/steps/cookbook/daily_paper/analyze.py b/reme/steps/cookbook/daily_paper/analyze.py index 6d1f177b..68e3579f 100644 --- a/reme/steps/cookbook/daily_paper/analyze.py +++ b/reme/steps/cookbook/daily_paper/analyze.py @@ -12,6 +12,7 @@ from ._common import ( DailyPaperStep, iter_note_metadata, normalize_chinese_title, + replace_surrogates, resolve_unique_note_path, strip_frontmatter, structured_output, @@ -43,7 +44,8 @@ class DailyPaperAnalyzeStep(DailyPaperStep): page_count = min(len(reader.pages), max_pages) truncated = len(reader.pages) > max_pages for page_number, page in enumerate(reader.pages[:page_count], start=1): - block = f"\n\n--- PAGE {page_number} ---\n\n{(page.extract_text() or '').strip()}" + page_text = replace_surrogates((page.extract_text() or "").strip()) + block = f"\n\n--- PAGE {page_number} ---\n\n{page_text}" if size + len(block) > max_chars: if (remaining := max_chars - size) > 0: chunks.append(block[:remaining]) @@ -126,8 +128,9 @@ class DailyPaperAnalyzeStep(DailyPaperStep): ) used_titles.add(title) note_rel = note_path.relative_to(self.workspace_path).as_posix() - body = strip_frontmatter(output.body) - if not output.desc.strip() or not body: + desc = replace_surrogates(output.desc.strip()) + body = replace_surrogates(strip_frontmatter(output.body)) + if not desc or not body: raise ValueError(f"Agent returned an empty paper note for {paper.arxiv_id}") await write_markdown( note_path, @@ -135,7 +138,7 @@ class DailyPaperAnalyzeStep(DailyPaperStep): { "name": title, "title": title, - "description": output.desc.strip(), + "description": desc, "kind": "daily-paper-analysis", "arxiv_id": paper.arxiv_id, "source_title": paper.title, @@ -163,7 +166,7 @@ class DailyPaperAnalyzeStep(DailyPaperStep): arxiv_id=paper.arxiv_id, reasoning=selected.reasoning, title=title, - desc=output.desc.strip(), + desc=desc, body=body, note_path=note_rel, pdf_path=pdf_rel, diff --git a/tests/unit/test_daily_paper.py b/tests/unit/test_daily_paper.py index e0a027a3..851a93da 100644 --- a/tests/unit/test_daily_paper.py +++ b/tests/unit/test_daily_paper.py @@ -25,6 +25,12 @@ from reme.steps.cookbook.daily_paper import ( DailyPaperSelectStep, ) from reme.steps.cookbook.daily_paper import analyze, collect +from reme.steps.cookbook.daily_paper._common import ( + normalize_chinese_title, + replace_surrogates, + write_atomic, + write_markdown, +) from reme.steps.cookbook.daily_paper.rank import build_candidate_pool, rrf_score from reme.steps.cookbook.dingtalk import DingTalkMarkdownSendStep from reme.steps.cookbook.dingtalk import send as dingtalk_send @@ -57,6 +63,73 @@ def _paper(arxiv_id: str, *, title: str = "Paper", upvotes: int = 10) -> PaperIn ) +def test_daily_paper_replaces_surrogates_in_text_and_titles(): + """Invalid surrogate code points become visible replacement characters.""" + assert replace_surrogates("before\ud800middle\udfffafter") == "before\ufffdmiddle\ufffdafter" + assert normalize_chinese_title("论文\ud800标题", "fallback") == "论文\ufffd标题" + + +@pytest.mark.asyncio +async def test_daily_paper_atomic_write_replaces_surrogates(tmp_path: Path): + """Markdown writes always produce valid UTF-8 even when model output is malformed.""" + target = tmp_path / "note.md" + + await write_atomic(target, "before\ud800after") + + assert target.read_text(encoding="utf-8") == "before\ufffdafter" + + +@pytest.mark.asyncio +async def test_daily_paper_markdown_write_replaces_surrogates_in_frontmatter(tmp_path: Path): + """Frontmatter serialization sanitizes nested metadata before YAML encoding.""" + target = tmp_path / "note.md" + + await write_markdown( + target, + "body\ud800text", + {"description": "meta\udffftext", "authors": ["safe", "author\ud800name"]}, + ) + + post = frontmatter.load(target) + assert post.content == "body\ufffdtext" + assert post.metadata == { + "description": "meta\ufffdtext", + "authors": ["safe", "author\ufffdname"], + } + + +def test_daily_paper_pdf_extraction_replaces_surrogates(monkeypatch, tmp_path: Path): + """Malformed PDF text is sanitized before it enters an agent prompt.""" + + class FakePage: + """Return text containing one unpaired surrogate.""" + + @staticmethod + def extract_text(): + """Return the malformed page text.""" + return "before\ud800after" + + class FakeReader: + """Expose one fake PDF page.""" + + def __init__(self, _path: str): + self.pages = [FakePage()] + + import pypdf + + monkeypatch.setattr(pypdf, "PdfReader", FakeReader) + + content, page_count, truncated = DailyPaperAnalyzeStep._extract_pdf_text_sync( # pylint: disable=protected-access + tmp_path / "paper.pdf", + 20, + 300_000, + ) + + assert content == "--- PAGE 1 ---\n\nbefore\ufffdafter" + assert page_count == 1 + assert truncated is False + + def test_hf_payload_and_html_normalization(): """HF list/detail shapes normalize and HTML rank order de-duplicates.""" payload = { @@ -705,8 +778,8 @@ async def test_pipeline_filters_strict_yesterday_and_writes_outputs( }, { "title": "记忆代理研究", - "desc": "Detailed note one", - "body": "Evidence one [p. 1].", + "desc": "Detailed note\ud800 one", + "body": "Evidence\udfff one [p. 1].", }, { "title": "上下文压缩研究", @@ -801,7 +874,9 @@ async def test_pipeline_filters_strict_yesterday_and_writes_outputs( assert "ReMe" not in analysis_prompt assert "# PDF 分页文本" in analysis_prompt digest_prompt = cc_wrapper.calls[-1]["inputs"] - assert "Evidence one [p. 1]." in digest_prompt + assert "Evidence\ufffd one [p. 1]." in digest_prompt + assert "Detailed note\ufffd one" in digest_prompt + assert not any("\ud800" <= character <= "\udfff" for character in digest_prompt) assert "调用 Read" not in digest_prompt assert "daily/2026-07-21" not in digest_prompt assert "长期记忆" not in digest_prompt From 15d12be6b678ff11fbb16d93c70c7082d47816f2 Mon Sep 17 00:00:00 2001 From: jinliyl <6469360+jinliyl@users.noreply.github.com> Date: Wed, 26 Aug 2026 16:16:26 +0800 Subject: [PATCH 2/2] fix(index): tolerate invalid text encoding (#490) * fix(index): tolerate invalid text encoding * fix(index): preserve text chunker compatibility --- docs/en/memory_as_file.md | 5 + docs/zh/memory_as_file.md | 4 + .../file_chunker/default_file_chunker.py | 44 ++++++++- .../file_chunker/markdown_file_chunker.py | 14 ++- tests/unit/test_default_file_chunker.py | 63 ++++++++++++ tests/unit/test_markdown_file_chunker.py | 98 +++++++++++++++++++ 6 files changed, 222 insertions(+), 6 deletions(-) diff --git a/docs/en/memory_as_file.md b/docs/en/memory_as_file.md index 5202887a..f519396a 100644 --- a/docs/en/memory_as_file.md +++ b/docs/en/memory_as_file.md @@ -388,3 +388,8 @@ This lets the agent see not only an isolated paragraph but also its structural p Non-Markdown files use `DefaultFileChunker` by default. It splits by byte size and preserves a small overlap. For Markdown, the chunker also avoids cutting `[[wikilinks]]` in the middle. + +`DefaultFileChunker` and `MarkdownFileChunker` decode files with their configured `encoding` and normalize platform +newlines to LF before indexing. Their default `invalid_encoding_policy: replace` keeps decodable content searchable +when a source contains invalid bytes, without modifying the source file. Set `invalid_encoding_policy: strict` on a +chunker component to reject such files instead. diff --git a/docs/zh/memory_as_file.md b/docs/zh/memory_as_file.md index f9532d42..6d792ed9 100644 --- a/docs/zh/memory_as_file.md +++ b/docs/zh/memory_as_file.md @@ -363,3 +363,7 @@ FileChunk[] 这样检索命中时,Agent 不只看到孤立段落,还能看到它在原文件中的结构位置。 非 Markdown 默认走 `DefaultFileChunker`:按字节大小切分,并保留少量 overlap;对 Markdown 则会避免把 `[[wikilink]]` 从中间切开。 + +`DefaultFileChunker` 和 `MarkdownFileChunker` 使用各自配置的 `encoding` 解码文件,并在索引前将平台换行符统一为 +LF。默认的 `invalid_encoding_policy: replace` 会在源文件含无效字节时保留其中可解码的内容用于检索,但不会修改源 +文件;如需拒绝此类文件,可在 chunker 组件上设置 `invalid_encoding_policy: strict`。 diff --git a/reme/components/file_chunker/default_file_chunker.py b/reme/components/file_chunker/default_file_chunker.py index 23457f5b..4fddc632 100644 --- a/reme/components/file_chunker/default_file_chunker.py +++ b/reme/components/file_chunker/default_file_chunker.py @@ -2,6 +2,7 @@ from bisect import bisect_right from pathlib import Path +from typing import Literal import aiofiles import yaml @@ -11,14 +12,29 @@ from ..component_registry import R from ...schema import FileChunk, FileFrontMatter, FileNode from ...utils.wikilink_handler import WikilinkHandler +InvalidEncodingPolicy = Literal["replace", "strict"] + @R.register("default") class DefaultFileChunker(BaseFileChunker): """Default chunker that splits files into byte-based overlapping chunks.""" - def __init__(self, encoding: str = "utf-8", chunk_byte_size: int = 10000, overlap_byte_size: int = 100, **kwargs): + def __init__( + self, + encoding: str = "utf-8", + chunk_byte_size: int = 10000, + overlap_byte_size: int = 100, + *, + invalid_encoding_policy: InvalidEncodingPolicy = "replace", + **kwargs, + ): super().__init__(**kwargs) + if invalid_encoding_policy not in {"replace", "strict"}: + raise ValueError( + f"invalid_encoding_policy must be 'replace' or 'strict', got {invalid_encoding_policy!r}", + ) self.encoding = encoding + self.invalid_encoding_policy = invalid_encoding_policy self.chunk_byte_size = max(100, chunk_byte_size) self.overlap_byte_size = max(4, overlap_byte_size) @@ -37,13 +53,35 @@ class DefaultFileChunker(BaseFileChunker): front_matter = FileFrontMatter() return front_matter, text[end_idx + 4 :].lstrip("\n") + async def _read_text_for_indexing(self, file_path: Path) -> str: + """Decode text for a derived index without modifying the source file.""" + async with aiofiles.open(file_path, "rb") as f: + data = await f.read() + try: + text = data.decode(self.encoding) + except UnicodeDecodeError as exc: + if self.invalid_encoding_policy == "strict": + raise + invalid_bytes = data[exc.start : exc.end].hex(" ") + self.logger.warning( + f"Invalid {self.encoding} in {file_path} at byte {exc.start} (bytes: {invalid_bytes}); " + "indexed with replacement characters; source file unchanged", + ) + text = data.decode(self.encoding, errors="replace") + # Some codecs (for example ASCII) cannot encode U+FFFD. Convert the + # decoded fallback to that codec's own replacement representation so + # later byte-based chunking remains safe. + text = text.encode(self.encoding, errors="replace").decode(self.encoding) + + # Match the universal-newline behavior of the previous text-mode reads. + return text.replace("\r\n", "\n").replace("\r", "\n") + async def chunk(self, path: str | Path) -> tuple[FileNode, list[FileChunk]]: file_path = Path(path) stat = file_path.stat() rel_path = self.to_workspace_relative(path) - async with aiofiles.open(file_path, encoding=self.encoding) as f: - text = await f.read() + text = await self._read_text_for_indexing(file_path) if not text: return FileNode(path=rel_path, st_mtime=stat.st_mtime), [] diff --git a/reme/components/file_chunker/markdown_file_chunker.py b/reme/components/file_chunker/markdown_file_chunker.py index 7da94ae6..06a07793 100644 --- a/reme/components/file_chunker/markdown_file_chunker.py +++ b/reme/components/file_chunker/markdown_file_chunker.py @@ -24,7 +24,7 @@ import yaml from pydantic import ValidationError -from .default_file_chunker import DefaultFileChunker +from .default_file_chunker import DefaultFileChunker, InvalidEncodingPolicy from ..component_registry import R from ...schema import ( FileChunk, @@ -108,9 +108,16 @@ class MarkdownFileChunker(DefaultFileChunker): max_ast_sections: int | None = 100, include_frontmatter_in_metadata: bool = False, include_frontmatter_keys_in_metadata: list[str] | None = None, + *, + invalid_encoding_policy: InvalidEncodingPolicy = "replace", **kwargs, ): - super().__init__(encoding=encoding, chunk_byte_size=chunk_byte_size, **kwargs) + super().__init__( + encoding=encoding, + invalid_encoding_policy=invalid_encoding_policy, + chunk_byte_size=chunk_byte_size, + **kwargs, + ) self.embed_toc = embed_toc self.max_ast_sections = max(0, max_ast_sections) if max_ast_sections is not None else None self.include_frontmatter_in_metadata = include_frontmatter_in_metadata @@ -119,7 +126,8 @@ class MarkdownFileChunker(DefaultFileChunker): async def chunk(self, path: str | Path) -> tuple[FileNode, list[FileChunk]]: file_path = Path(path) rel_path = self.to_workspace_relative(path) - front_matter, content, line_offset = self._parse_front_matter(file_path.read_text(encoding=self.encoding)) + text = await self._read_text_for_indexing(file_path) + front_matter, content, line_offset = self._parse_front_matter(text) chunks: list[FileChunk] = [] if content and content.strip(): diff --git a/tests/unit/test_default_file_chunker.py b/tests/unit/test_default_file_chunker.py index bf22d77e..aebd19ba 100644 --- a/tests/unit/test_default_file_chunker.py +++ b/tests/unit/test_default_file_chunker.py @@ -97,6 +97,69 @@ def test_parse_with_custom_encoding(): asyncio.run(run()) +def test_invalid_encoding_policy_is_validated(): + """Reject misspelled policies instead of silently changing decoding behavior.""" + try: + DefaultFileChunker(invalid_encoding_policy="ignore") + except ValueError as exc: + assert "invalid_encoding_policy" in str(exc) + else: + raise AssertionError("invalid encoding policy must be rejected") + + +def test_constructor_preserves_positional_arguments(): + """New decoding options must not reinterpret the established positional API.""" + chunker = DefaultFileChunker("utf-8", 5000, 100) + + assert chunker.encoding == "utf-8" + assert chunker.chunk_byte_size == 5000 + assert chunker.overlap_byte_size == 100 + + +def test_newlines_are_normalized_before_chunking(): + """Binary reads retain the universal-newline behavior of the old text reader.""" + + async def run(): + with tempfile.NamedTemporaryFile(delete=False, suffix=".txt") as f: + f.write(b"alpha\r\nbeta\rgamma\n") + temp_path = f.name + + try: + chunker = DefaultFileChunker() + _, original_chunks = await chunker.chunk(temp_path) + with open(temp_path, "wb") as f: + f.write(b"alpha\nbeta\ngamma\n") + _, normalized_chunks = await chunker.chunk(temp_path) + + assert [chunk.text for chunk in original_chunks] == ["alpha\nbeta\ngamma\n"] + assert [chunk.id for chunk in original_chunks] == [chunk.id for chunk in normalized_chunks] + finally: + os.unlink(temp_path) + + asyncio.run(run()) + + +def test_ascii_replacement_remains_encodable(): + """Replacement mode must survive byte-based chunking with a single-byte codec.""" + + async def run(): + source = b"valid\xffinvalid" + with tempfile.NamedTemporaryFile(delete=False, suffix=".txt") as f: + f.write(source) + temp_path = f.name + + try: + _, chunks = await DefaultFileChunker(encoding="ascii").chunk(temp_path) + + assert [chunk.text for chunk in chunks] == ["valid?invalid"] + with open(temp_path, "rb") as f: + assert f.read() == source + finally: + os.unlink(temp_path) + + asyncio.run(run()) + + def test_parse_links_bare(): """Bare wikilink: [[target]].""" links = WikilinkHandler.extract_links("see [[note]]", "src.md") diff --git a/tests/unit/test_markdown_file_chunker.py b/tests/unit/test_markdown_file_chunker.py index 118443ca..fbeeffaf 100644 --- a/tests/unit/test_markdown_file_chunker.py +++ b/tests/unit/test_markdown_file_chunker.py @@ -57,6 +57,104 @@ def test_parse_empty_file(): asyncio.run(run()) +def test_invalid_utf8_is_replaced_without_modifying_source(): + """Bad source bytes degrade the derived index but remain untouched on disk.""" + + async def run(): + with tempfile.TemporaryDirectory() as tmp, temp_chdir(tmp): + source = b"# valid\ncontent before \xcd content after\n" + path = os.path.join(tmp, "invalid.md") + with open(path, "wb") as f: + f.write(source) + + chunker = MarkdownFileChunker() + with patch.object(chunker.logger, "warning") as warning: + node, chunks = await chunker.chunk("invalid.md") + + assert node.path == "invalid.md" + assert "content before \ufffd content after" in "\n".join(chunk.text for chunk in chunks) + with open(path, "rb") as f: + assert f.read() == source + warning.assert_called_once_with( + "Invalid utf-8 in invalid.md at byte 23 (bytes: cd); " + "indexed with replacement characters; source file unchanged", + ) + + asyncio.run(run()) + + +def test_invalid_utf8_strict_policy_still_raises(): + """Strict mode remains available when callers require exact decoding.""" + + async def run(): + with tempfile.TemporaryDirectory() as tmp, temp_chdir(tmp): + with open(os.path.join(tmp, "invalid.md"), "wb") as f: + f.write(b"valid\xcdinvalid") + + chunker = MarkdownFileChunker(invalid_encoding_policy="strict") + try: + await chunker.chunk("invalid.md") + except UnicodeDecodeError as exc: + assert exc.start == 5 + else: + raise AssertionError("strict policy must reject invalid UTF-8") + + asyncio.run(run()) + + +def test_constructor_preserves_positional_arguments(): + """The decoding policy must not shift the established positional parameters.""" + chunker = MarkdownFileChunker("utf-8", 5000, False, 10, True, ["name"]) + + assert chunker.encoding == "utf-8" + assert chunker.chunk_byte_size == 5000 + assert chunker.embed_toc is False + assert chunker.max_ast_sections == 10 + assert chunker.include_frontmatter_in_metadata is True + assert chunker.include_frontmatter_keys_in_metadata == ["name"] + + +def test_plain_text_fallback_normalizes_newlines(): + """Markdown fallback chunks stay stable for equivalent platform newlines.""" + + async def run(): + with tempfile.TemporaryDirectory() as tmp, temp_chdir(tmp): + path = os.path.join(tmp, "fallback.md") + with open(path, "wb") as f: + f.write(b"# One\r\nbody\r# Two\r\nbody\r\n") + + chunker = MarkdownFileChunker(max_ast_sections=0) + _, original_chunks = await chunker.chunk("fallback.md") + with open(path, "wb") as f: + f.write(b"# One\nbody\n# Two\nbody\n") + _, normalized_chunks = await chunker.chunk("fallback.md") + + assert [chunk.text for chunk in original_chunks] == [chunk.text for chunk in normalized_chunks] + assert [chunk.id for chunk in original_chunks] == [chunk.id for chunk in normalized_chunks] + + asyncio.run(run()) + + +def test_invalid_ascii_is_replaced_for_markdown(): + """Markdown byte accounting accepts the configured codec's replacement text.""" + + async def run(): + with tempfile.TemporaryDirectory() as tmp, temp_chdir(tmp): + source = b"# valid\ncontent before \xff content after\n" + path = os.path.join(tmp, "invalid.md") + with open(path, "wb") as f: + f.write(source) + + chunker = MarkdownFileChunker(encoding="ascii") + _, chunks = await chunker.chunk("invalid.md") + + assert "content before ? content after" in "\n".join(chunk.text for chunk in chunks) + with open(path, "rb") as f: + assert f.read() == source + + asyncio.run(run()) + + def test_parse_frontmatter_only(): """A file with only frontmatter (no body) → no chunks, no links."""