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

1 participant