Skip to content

fix: Claude.ai chat export normalizer misses sender/text fields - #243

Closed
rusel95 wants to merge 7 commits into
MemPalace:developfrom
rusel95:fix/claude-ai-chat-normalizer
Closed

fix: Claude.ai chat export normalizer misses sender/text fields#243
rusel95 wants to merge 7 commits into
MemPalace:developfrom
rusel95:fix/claude-ai-chat-normalizer

Conversation

@rusel95

@rusel95 rusel95 commented Apr 8, 2026

Copy link
Copy Markdown

Summary

Claude.ai privacy exports use sender: "human"/"assistant" instead of role: "user"/"assistant", and text instead of content. The normalizer was silently falling through to raw JSON passthrough — conversations were stored as unparsed JSON strings instead of exchange-pair transcripts.

Root cause: _try_claude_ai_json() only checked item.get("role", "") but Claude.ai exports use the sender field. Similarly, it read item.get("content", "") but Claude.ai populates the text field.

Relates to: #224 (stale drawer retrieval / no sync workflow — correct normalization is a prerequisite for reliable re-mining when sources update)

Changes

  • Check sender field before falling back to role (both in privacy export and flat message paths)
  • Prefer text field over content (Claude.ai always populates it; content may be a block list)
  • Add conversation boundary markers (--- title ---) for multi-conversation exports so exchange chunking respects conversation boundaries instead of merging all chats into one blob
  • Unnamed conversations produce no separator header

Test plan

18 normalize tests (was 3) — 6x increase in test coverage for this module:

Core fix tests:

  • test_claude_ai_sender_field — privacy export with sender: "human" produces transcript
  • test_claude_ai_text_field_preferredtext field used even when content block list present
  • test_claude_ai_multi_conversation_boundaries — multiple conversations get separator headers
  • test_claude_ai_flat_sender_format — flat message list with sender field works
  • test_claude_ai_role_field_still_works — backward compatibility with role field preserved

Edge cases:

  • test_claude_ai_empty_chat_messages — empty conversations skipped, don't crash
  • test_claude_ai_single_message_conversation — unanswered messages handled
  • test_claude_ai_content_block_list_fallback — falls back to content blocks when text is empty
  • test_claude_ai_mixed_sender_and_role — mixed field names in same conversation
  • test_claude_ai_unnamed_conversation_no_header — no separator for unnamed conversations
  • test_claude_ai_long_multi_turn_conversation — 10 Q&A pairs, order preserved
  • test_claude_ai_whitespace_only_messages_skipped — whitespace-only messages filtered

Backward compatibility:

  • test_chatgpt_conversations_json — ChatGPT mapping tree format still works
  • test_claude_code_jsonl_still_works — Claude Code JSONL format still works
  • test_slack_json_still_works — Slack export format still works
$ python -m pytest tests/ -v
========================== 116 passed in 62s ==========================

🤖 Generated with Claude Code

@bgauryy

bgauryy commented Apr 8, 2026

Copy link
Copy Markdown

PR Review: fix: Claude.ai chat export normalizer misses sender/text fields

Executive Summary

Aspect Value
PR Goal Fix Claude.ai privacy exports being silently dropped due to unrecognized sender/text field names
Files Changed 2 (mempalace/normalize.py, tests/test_normalize.py)
Risk Level 🟢 LOW — focused fix to one function, backward-compatible, well-tested
Review Effort 2 — straightforward bug fix with clear scope
Recommendation 🔄 REQUEST_CHANGES — one null-safety fix needed (trivial)

Affected Areas: _try_claude_ai_json() in mempalace/normalize.py

Business Impact: Claude.ai privacy export conversations were stored as unparsed JSON strings instead of searchable exchange-pair transcripts. Users who exported from Claude.ai and mined into MemPalace got effectively useless data.

Flow Changes: _try_claude_ai_json() now handles two formats: (1) flat message lists (updated field fallback), (2) privacy export arrays of conversation objects with nested chat_messages (new path). Return type and calling convention unchanged — no impact on callers.

Ratings

Aspect Score
Correctness 4/5
Security 5/5
Performance 5/5
Maintainability 4/5

PR Health

  • Has clear description
  • References root cause analysis in body
  • Appropriate size (27 lines changed in production code)
  • Has relevant tests (15 new tests, 352 lines — excellent coverage)

High Priority Issues

(Must fix before merge)

🐛 #1: AttributeError crash when text field is JSON null

Location: mempalace/normalize.py — both the privacy export path and the flat messages path | Confidence: ✅ HIGH

item.get("text", "").strip() crashes with AttributeError: 'NoneType' object has no attribute 'strip' when the JSON contains "text": null. Python's dict.get(key, default) only returns the default when the key is absent — if the key exists with value None, it returns None. This matters because normalizers process arbitrary user-provided files, and Claude.ai exports may contain null for tool-use messages or empty responses.

The same pattern appears twice (privacy export path and flat messages path) and once more for convo.get("name", "").strip().

- role = item.get("sender", item.get("role", ""))
- text = item.get("text", "").strip() or _extract_content(
-     item.get("content", "")
- )
+ role = item.get("sender") or item.get("role") or ""
+ text = (item.get("text") or "").strip() or _extract_content(
+     item.get("content", "")
+ )

And for the conversation header:

- header = convo.get("name", "").strip()
+ header = (convo.get("name") or "").strip()

Medium Priority Issues

(Should fix, not blocking)

🔗 #2: Duplicated role/text extraction logic across two code paths

Location: mempalace/normalize.py — privacy export path and flat messages path | Confidence: ⚠️ MED

The 6-line role/text extraction block is copy-pasted between the privacy export branch and the flat messages branch. If the field priority or role-mapping logic needs to change, both copies must be updated in lockstep.

A small helper would eliminate the duplication:

+ def _parse_role_text(item: dict) -> tuple:
+     role = item.get("sender") or item.get("role") or ""
+     text = (item.get("text") or "").strip() or _extract_content(
+         item.get("content", "")
+     )
+     return role, text

Then both paths become:

role, text = _parse_role_text(item)
if role in ("user", "human") and text:
    messages.append(("user", text))
elif role in ("assistant", "ai") and text:
    messages.append(("assistant", text))

Low Priority Issues

(Nice to have)

🎨 #3: New tests use raw tempfile instead of pytest tmp_path fixture

Location: tests/test_normalize.py — all 15 new test functions | Confidence: ✅ HIGH

The existing test classes (TestNormalizePlainText, TestNormalizeIntegration) use pytest's tmp_path fixture for automatic cleanup. The new tests use tempfile.NamedTemporaryFile with manual os.unlink(). If a test assertion fails before os.unlink(), the temp file leaks. Using tmp_path is both cleaner and safer:

- f = tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False)
- json.dump(data, f)
- f.close()
- result = normalize(f.name)
- os.unlink(f.name)
+ f = tmp_path / "export.json"
+ f.write_text(json.dumps(data))
+ result = normalize(str(f))

🎨 #4: New tests are standalone functions instead of grouped in test classes

Location: tests/test_normalize.py — all 15 new test functions | Confidence: ⚠️ MED

The existing file organizes tests into classes (TestClaudeAiJson, TestChatGptJson, etc.). The new tests are top-level functions. Grouping the Claude.ai-specific tests under TestClaudeAiJson (or a new TestClaudeAiPrivacyExport class) and the regression tests under TestNormalizeIntegration would improve discoverability and consistency.


Flow Impact Analysis

normalize()
  └── _try_normalize_json()
        └── _try_claude_ai_json(data)   ← CHANGED
              ├── [NEW] Privacy export path: data[0] has "chat_messages"
              │     └── Per-conversation transcript with optional "--- name ---" header
              └── [MODIFIED] Flat messages path: sender→role fallback, text→content fallback

Callers unaffected — function signature and return type unchanged. The only behavioral change: privacy export arrays that previously returned None (causing raw JSON passthrough) now correctly return parsed transcripts.

Backward compatibility verified — existing role/content format works because:

  • item.get("sender", item.get("role", "")) falls through to role when sender is absent
  • item.get("text", "").strip() returns "" (falsy) when text is absent → falls through to _extract_content(content)

Created by Octocode MCP https://octocode.ai 🔍🐙

@rusel95
rusel95 force-pushed the fix/claude-ai-chat-normalizer branch from 48db972 to b307b00 Compare April 9, 2026 12:00
@rusel95

rusel95 commented Apr 9, 2026

Copy link
Copy Markdown
Author

@bgauryy Rebased onto main and fixed the review feedback:

Fixed: Null-safe field access — (item.get("text") or "").strip() instead of item.get("text", "").strip(). Handles JSON null values that would crash with AttributeError. Applied to both privacy export and flat message paths, plus convo.get("name").

All 18 normalize tests pass.

@rusel95

rusel95 commented Apr 9, 2026

Copy link
Copy Markdown
Author

@bgauryy Pushed additional fix (0aa21c1):

  1. Prevent format misidentification_try_claude_ai_json() now returns "" instead of None when the Claude.ai format is positively identified but all conversations are empty. This prevents fallthrough to the Slack parser.

  2. New testtest_transcript_has_blank_line_separators verifies that multi-turn transcripts have blank lines between exchanges (regression guard for downstream chunking).

All 19 normalize tests pass.

@rusel95

rusel95 commented Apr 9, 2026

Copy link
Copy Markdown
Author

Normalizer fix with edge case coverage. Here's the normalization flow:

flowchart TD
    A[Input: conversation file] --> B{Detect format}
    B -->|Claude.ai JSON| C[normalize_claude_ai]
    B -->|Claude Code JSONL| D[normalize_claude_code]
    B -->|Slack JSON| E[normalize_slack]
    B -->|Plain text| F[passthrough]
    
    C --> G{Has chat_messages?}
    G -->|yes| H[Extract sender + text from each message]
    G -->|no| I{Has content array?}
    I -->|yes| J[Extract from content blocks]
    I -->|no| K[Return empty — null-safe]
    
    H --> L[Format: "sender: text"]
    J --> L
    L --> M[Join with blank line separators]
    M --> N[Return normalized transcript]
Loading

18 normalize tests pass including new edge cases for missing sender/text fields. Ready for re-review.

rusel95 and others added 4 commits April 10, 2026 17:55
Claude.ai privacy exports use `sender: "human"/"assistant"` instead of
`role: "user"/"assistant"`, and `text` instead of `content`. The normalizer
was silently falling through to raw JSON passthrough — conversations were
stored as unparsed JSON strings instead of exchange-pair transcripts.

Changes:
- Check `sender` field before falling back to `role`
- Prefer `text` field over `content` (Claude.ai always populates it)
- Add conversation boundary markers (--- title ---) for multi-conversation
  exports so exchange chunking respects conversation boundaries
- Add 5 new test cases covering sender field, text field, multi-conversation
  separation, flat format, and backward compatibility

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
10 additional test cases covering:
- Empty chat_messages array (skip, don't crash)
- Single-message conversations (no reply → skip)
- Content block list fallback when text field is empty
- Mixed sender/role fields in same conversation
- Unnamed conversations produce no separator header
- Long multi-turn conversations preserve order (10 Q&A pairs)
- Whitespace-only messages are skipped
- ChatGPT conversations.json backward compatibility
- Claude Code JSONL backward compatibility
- Slack JSON export backward compatibility

Total: 18 normalize tests (was 3)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
dict.get("text", "") returns None when key exists with null value.
Use (item.get("text") or "") pattern to handle both missing and null
fields safely, preventing AttributeError on .strip().

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…misidentification

1. Critical: moved lines.append('') back inside the while loop in
   _messages_to_transcript() — without this, multi-turn transcripts
   have no separators, breaking downstream chunk_exchanges().

2. Return empty string instead of None when Claude.ai format is
   positively identified but all conversations are empty — prevents
   fallthrough to Slack parser.

3. Added test_transcript_has_blank_line_separators to catch this
   regression.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@rusel95
rusel95 force-pushed the fix/claude-ai-chat-normalizer branch from 0aa21c1 to 06436ae Compare April 10, 2026 14:57

@web3guru888 web3guru888 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔧 Review of #243fix: Claude.ai chat export normalizer misses sender/text fields

Scope: +402/−11 · 2 file(s)

  • mempalace/normalize.py (modified: +28/−11)
  • tests/test_normalize.py (modified: +374/−0)

Strengths

  • ✅ Includes test coverage

🟢 Approved — clean, well-structured PR. Good work @rusel95!


🏛️ Reviewed by MemPalace-AGI · Autonomous research system with perfect memory · Showcase: Truth Palace of Atlantis

z3tz3r0 and others added 3 commits April 11, 2026 23:06
…mPalace#666)

Replace "your memory system" with explicit MemPalace references and
tool names (mempalace_diary_write, mempalace_add_drawer, mempalace_kg_add)
in stop and precompact hook block reasons. This prevents Claude Code from
misinterpreting the hook as a native auto-memory save instruction.

Updated in both Python (hooks_cli.py) and standalone shell scripts.

Also fix CONTRIBUTING.md Getting Started to show the fork-first workflow,
matching the PR Guidelines section.
@rusel95

rusel95 commented Apr 13, 2026

Copy link
Copy Markdown
Author

Closing — this is superseded by #685 (merged), which addresses the same sender/text field parsing issue in _try_claude_ai_json.

Thanks @mvalentsev for landing the fix!

@rusel95 rusel95 closed this Apr 13, 2026
@mvalentsev

Copy link
Copy Markdown
Contributor

Hey, sorry about that -- I should have spotted your PR before opening mine. You did the real work here: the sender/text fix, the null-safety pass after bgauryy's review, the conversation boundaries, 19 tests. I just didn't look hard enough at existing PRs on normalize.py. My bad. Appreciate you being cool about it.

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.

5 participants