Skip to content

fix(session): preserve Unicode separators in JSONL records - #1017

Open
cangen185-sudo wants to merge 1 commit into
modelscope:mainfrom
cangen185-sudo:codex/fix-session-jsonl-unicode
Open

cangen185-sudo wants to merge 1 commit into
modelscope:mainfrom
cangen185-sudo:codex/fix-session-jsonl-unicode

Conversation

@cangen185-sudo

Copy link
Copy Markdown

Problem

Persisting {'role': 'user', 'content': 'before\u2028after'} and reopening SessionLog returns no message. The next append can also reuse sequence 0. The writer emits valid UTF-8 JSON with ensure_ascii=False, but str.splitlines() splits JSON strings at U+0085, U+2028, and U+2029; the resulting fragments are silently skipped as invalid JSON.

Change

Split records on LF in the seven existing reading loops: message restoration, five event readers, and sequence recovery. Preserve the log format, source bytes, sidecar metadata, and existing malformed-record handling. Existing logs need no rewrite or migration.

Validation

  • 222 passed, 20 skipped: python -m pytest tests/session tests/memory/test_unified_memory.py tests/config/test_model_settings.py tests/config/test_tool_settings.py tests/config/test_env_override.py tests/utils/test_retry_classification.py -q.
  • All 30 new regression cases failed before the production change and pass afterward. They cover the three Unicode separators, LF/CRLF, the last record without a newline, event payloads, and continued sequence numbering.
  • The 20 skips are declared optional integrations: mempalace (12), reme-ai (7), and mem0 (1). They are not counted as passing tests.
  • Flake8 on both changed files, new-test YAPF formatting, git diff --check, and wheel/source distribution builds pass.
  • Windows 11, CPython 3.12.14. Full repository tests, real LLM/provider calls, and optional integration suites were not run.

JSONL uses LF record boundaries; Unicode separators inside a JSON string are content. This follows the JSON Lines format without adding a dependency or changing the serialized schema.

Related work: #982 introduces session forks and a separate replay reader. Its new physical-line reader partially avoids the message-side symptom, but the five existing event readers still use splitlines(). This focused fix covers the currently shipped SessionLog read paths; it does not implement or duplicate session forking.

Prepared with Codex assistance: local reproduction, implementation, tests, and a separate read-only final diff review. No account configuration or user session files were accessed or rewritten.

@cangen185-sudo
cangen185-sudo marked this pull request as ready for review October 15, 2026 13:39

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant