Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions src/basic_memory/api/v2/routers/importer_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,15 @@ async def import_memory_json(
)
except HTTPException:
raise
except (json.JSONDecodeError, UnicodeDecodeError) as e:
# Trigger: a line in the upload is not valid JSON, or the bytes are not
# UTF-8 (truncated archive, wrong file, binary upload).
# Why: this is a client-input problem, not a server fault (#1276).
# Outcome: a 400 with the parse position instead of an opaque 500.
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
detail=f"Import file is not valid JSON: {e}",
)
except Exception as e:
logger.exception("V2 Import failed")
raise HTTPException(
Expand Down Expand Up @@ -244,6 +253,15 @@ async def import_file[ImportResultT: ImportResult](

except HTTPException:
raise
except (json.JSONDecodeError, UnicodeDecodeError) as e:
# Trigger: the upload is not valid JSON, or the bytes are not UTF-8
# (truncated archive, wrong file, binary upload).
# Why: this is a client-input problem, not a server fault (#1276).
# Outcome: a 400 with the parse position instead of an opaque 500.
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
detail=f"Import file is not valid JSON: {e}",
)
except Exception as e:
logger.exception("V2 Import failed")
raise HTTPException(
Expand Down
57 changes: 49 additions & 8 deletions src/basic_memory/importers/chatgpt_importer.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@

logger = logging.getLogger(__name__)

# One day past the Unix epoch: deterministic, still obviously a sentinel, and
# never a pre-epoch value in any local timezone — Windows' CRT raises OSError
# converting epoch-adjacent times through fromtimestamp/astimezone.
UNKNOWN_DATE_SENTINEL = 86400.0


class ChatGPTImporter(Importer[ChatImportResult]):
"""Service for importing ChatGPT conversations."""
Expand Down Expand Up @@ -53,7 +58,7 @@ async def import_data(
chats_imported = 0

for chat in conversations:
created_at = chat["create_time"]
created_at, modified_at = self._resolve_timestamps(chat)
date_prefix = datetime.fromtimestamp(created_at).astimezone().strftime("%Y%m%d")
Comment thread
phernandez marked this conversation as resolved.
clean_title = clean_filename(chat["title"])
relative_path = (
Expand All @@ -64,7 +69,7 @@ async def import_data(
permalink, file_path = self.build_import_paths(relative_path)

# Convert to entity
entity = self._format_chat_content(chat, permalink)
entity = self._format_chat_content(chat, permalink, created_at, modified_at)

# Write file using relative path - FileService handles base_path
await self.write_entity(entity, file_path)
Expand Down Expand Up @@ -93,22 +98,58 @@ async def import_data(
logger.exception("Failed to import ChatGPT conversations")
return self.handle_error("Failed to import ChatGPT conversations", e)

def _resolve_timestamps(self, conversation: Dict[str, Any]) -> tuple[float, float]:
"""Resolve conversation timestamps, tolerating absent fields.

OpenAI's export format does not guarantee `create_time` or `update_time`
on every conversation object (#1276). Fall back in order: the earliest
message timestamp, the conversation's `update_time`, an epoch sentinel.
Every rung must stay stable across re-exports — the resolved date names
the output file (`YYYYMMDD-title.md`), so a value that changes between
exports would make the next import write a duplicate note under a new
name instead of updating the original. That is why the earliest message
time outranks `update_time` (existing messages keep their timestamps
while `update_time` advances whenever the conversation continues) and
why the last resort is a fixed sentinel whose obviously-wrong 1970
prefix reads as "date unknown" rather than faking a plausible one.

Args:
conversation: ChatGPT conversation data.

Returns:
Tuple of (created_at, modified_at) unix timestamps.
"""
created_at = conversation.get("create_time")
modified_at = conversation.get("update_time")
if created_at is None:
message_times = [
node["message"]["create_time"]
for node in conversation.get("mapping", {}).values()
if node.get("message") and node["message"].get("create_time") is not None
]
created_at = min(message_times) if message_times else None
if created_at is None:
created_at = modified_at
Comment thread
phernandez marked this conversation as resolved.
if created_at is None:
created_at = UNKNOWN_DATE_SENTINEL
if modified_at is None:
modified_at = created_at
return created_at, modified_at

def _format_chat_content(
self, conversation: Dict[str, Any], permalink: str
self, conversation: Dict[str, Any], permalink: str, created_at: float, modified_at: float
) -> EntityMarkdown: # pragma: no cover
"""Convert chat conversation to Basic Memory entity.

Args:
folder: Destination folder name.
conversation: ChatGPT conversation data.
permalink: Permalink for the entity.
created_at: Resolved creation timestamp.
modified_at: Resolved modification timestamp.

Returns:
EntityMarkdown instance representing the conversation.
"""
# Extract timestamps
created_at = conversation["create_time"]
modified_at = conversation["update_time"]

root_id = None
# Find root message
for node_id, node in conversation["mapping"].items():
Expand Down
60 changes: 42 additions & 18 deletions tests/api/v2/test_importer_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,33 @@ async def test_import_chatgpt_invalid_file(client: AsyncClient, tmp_path, v2_pro
# Send request - this should return an error
response = await client.post(f"{v2_project_url}/import/chatgpt", files=files, data=data)

# Check response
assert response.status_code == 500
assert "Import failed" in response.json()["detail"]
# Check response: undecodable JSON is a client-input problem, not a server fault (#1276)
assert response.status_code == 400
assert "not valid JSON" in response.json()["detail"]


@pytest.mark.asyncio
async def test_import_chatgpt_invalid_utf8_file(client: AsyncClient, v2_project_url: str):
"""Invalid UTF-8 bytes are a client-input problem too, not a 500 (#1276)."""
files = {"file": ("invalid.json", b"\xff\xfe not utf-8", "application/json")}
data = {"directory": "test_chatgpt"}

response = await client.post(f"{v2_project_url}/import/chatgpt", files=files, data=data)

assert response.status_code == 400
assert "not valid JSON" in response.json()["detail"]


@pytest.mark.asyncio
async def test_import_memory_json_invalid_utf8_file(client: AsyncClient, v2_project_url: str):
"""The line-oriented memory-json decode rejects invalid UTF-8 with a 400 as well."""
files = {"file": ("memory.json", b"\xff\xfe not utf-8", "application/json")}
data = {"directory": "test_memory"}

response = await client.post(f"{v2_project_url}/import/memory-json", files=files, data=data)

assert response.status_code == 400
assert "not valid JSON" in response.json()["detail"]


@pytest.mark.asyncio
Expand Down Expand Up @@ -261,9 +285,9 @@ async def test_import_claude_conversations_invalid_file(
f"{v2_project_url}/import/claude/conversations", files=files, data=data
)

# Check response
assert response.status_code == 500
assert "Import failed" in response.json()["detail"]
# Check response: undecodable JSON is a client-input problem, not a server fault (#1276)
assert response.status_code == 400
assert "not valid JSON" in response.json()["detail"]


@pytest.mark.asyncio
Expand Down Expand Up @@ -326,9 +350,9 @@ async def test_import_claude_projects_invalid_file(
f"{v2_project_url}/import/claude/projects", files=files, data=data
)

# Check response
assert response.status_code == 500
assert "Import failed" in response.json()["detail"]
# Check response: undecodable JSON is a client-input problem, not a server fault (#1276)
assert response.status_code == 400
assert "not valid JSON" in response.json()["detail"]


@pytest.mark.asyncio
Expand Down Expand Up @@ -414,9 +438,9 @@ async def test_import_memory_json_invalid_file(client: AsyncClient, tmp_path, v2
# Send request - this should return an error
response = await client.post(f"{v2_project_url}/import/memory-json", files=files, data=data)

# Check response
assert response.status_code == 500
assert "Import failed" in response.json()["detail"]
# Check response: undecodable JSON is a client-input problem, not a server fault (#1276)
assert response.status_code == 400
assert "not valid JSON" in response.json()["detail"]


@pytest.mark.asyncio
Expand Down Expand Up @@ -515,9 +539,9 @@ async def test_import_empty_file(client: AsyncClient, tmp_path, v2_project_url:
# Send request
response = await client.post(f"{v2_project_url}/import/chatgpt", files=files, data=data)

# Check response
assert response.status_code == 500
assert "Import failed" in response.json()["detail"]
# Check response: undecodable JSON is a client-input problem, not a server fault (#1276)
assert response.status_code == 400
assert "not valid JSON" in response.json()["detail"]


@pytest.mark.asyncio
Expand All @@ -544,6 +568,6 @@ async def test_import_malformed_json(client: AsyncClient, tmp_path, v2_project_u
# Send request
response = await client.post(endpoint, files=files, data=data)

# Check response
assert response.status_code == 500
assert "Import failed" in response.json()["detail"]
# Check response: undecodable JSON is a client-input problem, not a server fault (#1276)
assert response.status_code == 400
assert "not valid JSON" in response.json()["detail"]
90 changes: 90 additions & 0 deletions tests/cli/test_import_chatgpt.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
"""Tests for import_chatgpt command."""

import json
from datetime import datetime

import pytest
from typer.testing import CliRunner

from basic_memory.cli.app import app, import_app
from basic_memory.cli.commands import import_chatgpt # noqa
from basic_memory.config import get_project_config
from basic_memory.importers.chatgpt_importer import UNKNOWN_DATE_SENTINEL

# Set up CLI runner
runner = CliRunner()
Expand Down Expand Up @@ -197,3 +199,91 @@ def test_import_chatgpt_with_custom_folder(tmp_path, sample_chatgpt_json, monkey
# Check files in custom folder
conv_path = tmp_path / conversations_folder / "20250111-Test_Conversation.md"
assert conv_path.exists()


def test_import_chatgpt_missing_create_time(tmp_path, sample_conversation):
"""Without create_time, the earliest message time wins over update_time (#1276).

update_time advances whenever a conversation continues, so using it while
messages carry timestamps would rename the output file on the next export
and fork a duplicate note. The messages' 2025-01-11 prefix proves the
immutable rung was chosen over the later update_time day.
"""
config = get_project_config()
config.home = tmp_path
del sample_conversation["create_time"]
sample_conversation["update_time"] = 1736703003.0 # 2025-01-12, later than messages
json_file = tmp_path / "conversations.json"
json_file.write_text(json.dumps([sample_conversation]), encoding="utf-8")

result = runner.invoke(app, ["import", "chatgpt", str(json_file), "--folder", "chats"])
assert result.exit_code == 0
assert "Imported 1 conversations" in result.output

conv_path = tmp_path / "chats" / "20250111-Test_Conversation.md"
assert conv_path.exists()


def test_import_chatgpt_update_time_rung_when_no_message_times(tmp_path, sample_conversation):
"""update_time is the fallback only when no message carries a timestamp (#1276)."""
config = get_project_config()
config.home = tmp_path
del sample_conversation["create_time"]
sample_conversation["update_time"] = 1736703003.0 # 2025-01-12
for node in sample_conversation["mapping"].values():
if node.get("message"):
node["message"]["create_time"] = None
json_file = tmp_path / "conversations.json"
json_file.write_text(json.dumps([sample_conversation]), encoding="utf-8")

result = runner.invoke(app, ["import", "chatgpt", str(json_file), "--folder", "chats"])
assert result.exit_code == 0
assert "Imported 1 conversations" in result.output

conv_path = tmp_path / "chats" / "20250112-Test_Conversation.md"
assert conv_path.exists()


def test_import_chatgpt_null_create_time(tmp_path, sample_conversation):
"""Null conversation timestamps fall back to the earliest message time (#1276)."""
config = get_project_config()
config.home = tmp_path
sample_conversation["create_time"] = None
sample_conversation["update_time"] = None
# msg1 has no usable time either; msg2 keeps its timestamp and becomes the fallback
sample_conversation["mapping"]["msg1"]["message"]["create_time"] = None
json_file = tmp_path / "conversations.json"
json_file.write_text(json.dumps([sample_conversation]), encoding="utf-8")

result = runner.invoke(app, ["import", "chatgpt", str(json_file), "--folder", "chats"])
assert result.exit_code == 0
assert "Imported 1 conversations" in result.output
assert "Containing 2 messages" in result.output

conv_path = tmp_path / "chats" / "20250111-Test_Conversation.md"
assert conv_path.exists()


def test_import_chatgpt_no_timestamps_anywhere_is_deterministic(tmp_path, sample_conversation):
"""With no usable timestamp at all, the epoch sentinel keeps reimports stable (#1276).

The resolved date names the output file, so an import-time fallback would
write a duplicate note under a new name on a later reimport.
"""
config = get_project_config()
config.home = tmp_path
sample_conversation["create_time"] = None
sample_conversation["update_time"] = None
for node in sample_conversation["mapping"].values():
if node.get("message"):
node["message"]["create_time"] = None
json_file = tmp_path / "conversations.json"
json_file.write_text(json.dumps([sample_conversation]), encoding="utf-8")

result = runner.invoke(app, ["import", "chatgpt", str(json_file), "--folder", "chats"])
assert result.exit_code == 0
assert "Imported 1 conversations" in result.output

epoch_prefix = datetime.fromtimestamp(UNKNOWN_DATE_SENTINEL).astimezone().strftime("%Y%m%d")
conv_path = tmp_path / "chats" / f"{epoch_prefix}-Test_Conversation.md"
assert conv_path.exists()
Loading