PR Review: feat: add Cursor agent transcript JSONL normalizer

Executive Summary

Affected Areas: mempalace/normalize.py (parser + dispatch), tests/test_normalize.py (9 new test functions)

Business Impact: Enables mining Cursor IDE conversations into the palace, expanding the supported conversation sources. Addresses part of #59.

Flow Changes: New parser slot in _try_normalize_json() dispatch chain — runs after _try_codex_jsonl, before json.loads(). No existing flows altered.

Ratings

PR Health

• Has clear description
• References ticket/issue (feat: add import support for more AI tool session formats (Cursor, Copilot, Codex, Windsurf, Aider, etc.) #59)
• Appropriate size (163 additions, 2 files)
• Has relevant tests (9 test functions)

High Priority Issues

None.

Medium Priority Issues

None.

Low Priority Issues

🐛 #1: has_cursor_structure flag can be set by empty-text entries

Location: mempalace/normalize.py:~178 (new code) | Confidence: ⚠️ MED

The has_cursor_structure = True flag is set whenever raw_content is a list, regardless of whether the entry actually contributes text to messages. In a pathological input where the only list-content entry has empty text (so it gets skipped), but two string-content entries survive, the parser would match — even though no list-typed entry actually contributed to the output.

Practically near-impossible with real Cursor transcripts, but the flag could be tightened:

-        if isinstance(raw_content, list):
-            has_cursor_structure = True
-
         text = _extract_content(raw_content)
         if not text:
             continue
+        if isinstance(raw_content, list):
+            has_cursor_structure = True
         messages.append((role, text))

This moves the flag after the emptiness check, so only entries that actually produce output set it.

🎨 #2: Missing test for entries with role but no message key

Location: tests/test_normalize.py (new tests section) | Confidence: ✅ HIGH

The parser correctly handles entries like {"role": "user", "other_key": "data"} (no message) — entry.get("message") returns None, isinstance(None, dict) is False, entry is skipped. However, no test covers this edge case. A brief test would document the contract:

def test_cursor_jsonl_skips_entries_without_message_key():
    """Entries with role but no message dict are skipped."""
    path = _write_jsonl([
        {"role": "user"},
        {"role": "user", "message": {"content": [{"type": "text", "text": "Q"}]}},
        {"role": "assistant", "message": {"content": [{"type": "text", "text": "A"}]}},
    ])
    result = normalize(path)
    assert "> Q" in result
    assert "A" in result
    os.unlink(path)

🔗 #3: Discrimination test couples to Claude Code parser output

Location: tests/test_normalize.py:test_cursor_jsonl_ignores_non_cursor_jsonl | Confidence: ⚠️ MED

The test asserts "> Hi from Claude Code" in result — verifying the Claude Code parser picks up the input after the Cursor parser rejects it. This is useful as an integration test but means a future change to _try_claude_code_jsonl output formatting could break this test even though the Cursor parser is correct. Consider splitting into a focused unit assertion (Cursor returns None) and a separate integration check.

Flow Impact Analysis

normalize(filepath)
  └─ _try_normalize_json(content)
       ├─ _try_claude_code_jsonl(content)   ← uses "type" key → no overlap
       ├─ _try_codex_jsonl(content)          ← uses "event_msg" / "session_meta" → no overlap
       ├─ _try_cursor_jsonl(content)         ← NEW: uses "role" + message.content as list
       ├─ json.loads(content) → ...
       │    ├─ _try_claude_ai_json(data)
       │    ├─ _try_chatgpt_json(data)
       │    └─ _try_slack_json(data)
       └─ return None

Discrimination matrix (why parsers don't interfere):

No cross-contamination risk between any parser pair.

Strengths

• Clean discrimination: The role vs type distinction is simple and bulletproof
• has_cursor_structure flag: Smart heuristic — requires list-typed content to prevent matching generic role-based JSONL
• Helper reuse: Leverages existing _extract_content() and _messages_to_transcript() — zero duplication of output logic
• Excellent test coverage: 9 tests covering happy path, multi-turn, empty content, single message rejection, format discrimination, multiple content blocks, malformed lines, and string-vs-list content
• Consistent style: Follows the exact same pattern as _try_claude_code_jsonl and _try_codex_jsonl

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

(Addendum to my review above — the shell ate the backtick formatting, sorry for the noise. The cleaned-up version:)

Detection logic is solid. The dual guard (role in {"user","assistant"} + isinstance(raw_content, list)) correctly discriminates against Claude Code JSONL (top-level type) and Codex JSONL (event_msg wrapper). The has_cursor_structure flag requiring at least one list-typed content block is a good extra check before committing.

Missing edge case worth testing: tool/image content blocks. Cursor transcripts can include non-text blocks ({"type": "tool_result", ...}, {"type": "image", ...}). The current _extract_content will silently skip these (correct), but worth adding a test that a turn containing only non-text blocks doesn't produce a phantom empty message in the output.

The SQLite investigation note is useful context. The explanation of why state.vscdb is incomplete (no paired turns in aiService.prompts) saves future contributors from going down that path. Consider preserving it in the docstring.

Test coverage otherwise looks thorough — malformed lines, single-message files, wrong schema, multi-block content all covered. The >=2 messages + list content heuristic is conservative enough to avoid false positives.

Conflicts with main. Cursor JSONL normalization is being addressed in #287. Thanks.