diff --git a/src/basic_memory/indexing/accepted_note_write_runner.py b/src/basic_memory/indexing/accepted_note_write_runner.py index 7cd511715..593b7ca17 100644 --- a/src/basic_memory/indexing/accepted_note_write_runner.py +++ b/src/basic_memory/indexing/accepted_note_write_runner.py @@ -100,6 +100,7 @@ class AcceptedPreparedEntityTarget(Protocol): content_type: str permalink: str | None file_path: RuntimeFilePath + created_at: datetime updated_at: datetime last_updated_by: str | None @@ -501,6 +502,30 @@ async def prepare_accepted_note_move( return result +def _timestamp_from_metadata( + entity_metadata: EntityMetadata, key: str, *, fallback: datetime +) -> datetime: + """Read a created/modified timestamp from prepared frontmatter — file as source of truth. + + BM's write path stamps created/modified into frontmatter as ISO strings (#238/#684), so a + cloud-accepted note's DB timestamp must come from that value rather than the request time; + otherwise the markdown shows one date while search/recent-activity show another. Falls back + to ``fallback`` only when the key is absent — a present-but-malformed value fails fast + (never silently substituted). + """ + value = entity_metadata.get(key) if entity_metadata else None + if value is None: + return fallback + if isinstance(value, datetime): + return value if value.tzinfo else value.astimezone() + if isinstance(value, str): + # A naive date-only value (e.g. "2024-03-15") is assumed local, matching + # EntityParser._frontmatter_timestamp so the cloud and local paths agree. + parsed = datetime.fromisoformat(value) + return parsed if parsed.tzinfo else parsed.astimezone() + return fallback + + def apply_accepted_prepared_entity_fields( entity: AcceptedPreparedEntityTarget, entity_fields: AcceptedPreparedEntityFields, @@ -515,7 +540,16 @@ def apply_accepted_prepared_entity_fields( entity.content_type = entity_fields.content_type entity.permalink = entity_fields.permalink entity.file_path = entity_fields.file_path - entity.updated_at = updated_at + # Honor the note's own timestamps (file is source of truth); the request time is only a + # fallback for modified. created_at is reconciled from frontmatter too — if the file's + # created changed it should win — but defaults to the entity's existing value so a normal + # update never moves it. + entity.created_at = _timestamp_from_metadata( + entity_fields.entity_metadata, "created", fallback=entity.created_at + ) + entity.updated_at = _timestamp_from_metadata( + entity_fields.entity_metadata, "modified", fallback=updated_at + ) entity.last_updated_by = user_profile_value @@ -528,6 +562,9 @@ def accepted_pending_entity_write_from_prepared( ) -> AcceptedPendingEntityWrite: """Map prepared Basic Memory entity fields to the pending entity DB write.""" fields = prepared.entity_fields + # File is the source of truth: seed created_at/updated_at from the note's own frontmatter + # timestamps (#238/#684) so a cloud-accepted note's DB row matches its markdown, with the + # request time only as a fallback when the frontmatter omits them. return AcceptedPendingEntityWrite( title=fields.title, note_type=fields.note_type, @@ -535,8 +572,8 @@ def accepted_pending_entity_write_from_prepared( content_type=fields.content_type, permalink=fields.permalink, file_path=fields.file_path, - created_at=now, - updated_at=now, + created_at=_timestamp_from_metadata(fields.entity_metadata, "created", fallback=now), + updated_at=_timestamp_from_metadata(fields.entity_metadata, "modified", fallback=now), created_by=user_profile_value, last_updated_by=user_profile_value, external_id=external_id, diff --git a/src/basic_memory/indexing/batch_indexer.py b/src/basic_memory/indexing/batch_indexer.py index 99e5c384f..f26a33cbf 100644 --- a/src/basic_memory/indexing/batch_indexer.py +++ b/src/basic_memory/indexing/batch_indexer.py @@ -631,7 +631,16 @@ async def _persist_markdown_file( session=session, ) prepared = await self._reconcile_persisted_permalink(prepared, entity) - metadata_updates = self._entity_metadata_updates(prepared.file, prepared.final_checksum) + # Markdown timestamps come from the parsed frontmatter via + # upsert_entity_from_markdown (file-stat fallback), not raw file stats (#238/#684). + metadata_updates = self._entity_metadata_updates( + prepared.file, prepared.final_checksum, include_semantic_timestamps=False + ) + # Re-assert the parsed timestamps on this UPDATE. They must be set explicitly: + # updated_at's onupdate=now default would otherwise clobber the note's modified time + # on every reindex, since this UPDATE does not otherwise touch that column. + metadata_updates["created_at"] = entity.created_at + metadata_updates["updated_at"] = entity.updated_at updated = await self.entity_repository.update_fields( session, entity.id, @@ -717,17 +726,29 @@ def _entity_metadata_updates( checksum: str, *, include_created_at: bool = True, + include_semantic_timestamps: bool = True, ) -> dict[str, object]: + # Trigger: persisting index metadata for a file. + # Why: file_path/checksum/size/mtime are always file-derived change-detection state. + # But a markdown note's semantic created_at/updated_at are owned by its parsed + # frontmatter (the parser already falls back to file stat times when frontmatter + # omits them, #238/#684), so re-deriving them from raw file stats here would clobber + # file-authored timestamps on every reindex. The file stays the source of truth + # either way — this only decides which file-derived value wins. + # Outcome: markdown callers pass include_semantic_timestamps=False so the timestamps + # set by upsert_entity_from_markdown stand; non-markdown files (no frontmatter) keep + # file stats as their created_at/updated_at. updates: dict[str, object] = { "file_path": file.path, "checksum": checksum, "size": file.size, } - if include_created_at and file.created_at is not None: - updates["created_at"] = file.created_at if file.last_modified is not None: - updates["updated_at"] = file.last_modified updates["mtime"] = file.last_modified.timestamp() + if include_semantic_timestamps: + updates["updated_at"] = file.last_modified + if include_created_at and include_semantic_timestamps and file.created_at is not None: + updates["created_at"] = file.created_at if file.content_type is not None: updates["content_type"] = file.content_type return updates diff --git a/src/basic_memory/indexing/note_materialization_runner.py b/src/basic_memory/indexing/note_materialization_runner.py index a757c474e..c0035e6f8 100644 --- a/src/basic_memory/indexing/note_materialization_runner.py +++ b/src/basic_memory/indexing/note_materialization_runner.py @@ -10,6 +10,7 @@ from loguru import logger from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker +from sqlalchemy.orm.attributes import flag_modified from basic_memory import db from basic_memory.indexing.note_content_reconciler import ( @@ -502,9 +503,14 @@ async def publish_written_file_state( file_path=written_file.file_path, file_checksum=written_file.file_checksum, ) - entity.updated_at = written_file.file_updated_at + # The physical write time is bookkeeping (mtime), not the note's semantic modified — + # its frontmatter owns that, and the accepted write already set entity.updated_at from + # it (#238/#684, file-as-source-of-truth). flag_modified forces updated_at into this + # flush with its current (semantic) value so the column's onupdate=now default does not + # overwrite it during materialization. entity.mtime = written_file.file_updated_at.timestamp() entity.size = len(prepared_write.markdown_content.encode("utf-8")) + flag_modified(entity, "updated_at") await session.flush() return publish_plan.result diff --git a/src/basic_memory/markdown/entity_parser.py b/src/basic_memory/markdown/entity_parser.py index f8676dd91..cf85effe1 100644 --- a/src/basic_memory/markdown/entity_parser.py +++ b/src/basic_memory/markdown/entity_parser.py @@ -26,6 +26,12 @@ md = MarkdownIt().use(observation_plugin).use(relation_plugin) +# Frontmatter keys accepted for the created/modified timestamps, canonical name first (#238). +# `date` and the `_at` aliases are read-time conveniences for notes imported from other tools; +# BM always writes the canonical `created`/`modified` keys (see parse_markdown_content below). +_CREATED_KEYS = ("created", "created_at", "date") +_MODIFIED_KEYS = ("modified", "updated_at") + def normalize_frontmatter_value(value: Any) -> Any: """Normalize frontmatter values to safe types for processing. @@ -184,6 +190,31 @@ def parse_date(self, value: Any) -> Optional[datetime]: return parsed return None + def _frontmatter_timestamp( + self, metadata: dict[str, Any], keys: tuple[str, ...] + ) -> Optional[datetime]: + """Resolve a created/modified timestamp from frontmatter, preferring the first alias. + + A naive result (e.g. from a date-only value like ``2024-03-15``) is assumed to be in + the local timezone rather than UTC (#238). + """ + for key in keys: + value = metadata.get(key) + if value is None: + continue + parsed = self.parse_date(value) + if parsed: + return parsed if parsed.tzinfo else parsed.astimezone() + # Trigger: a present timestamp value that parse_date cannot interpret. + # Why: silently falling back to file stats would leave the bad value on disk while + # the DB shows a different time — a file/DB disagreement the fail-fast rule + # forbids. But the canonical key (keys[0]: created/modified) is BM-written and + # must be valid; the trailing aliases (date/created_at/updated_at) are lenient + # read conveniences for imported notes, so a bad alias is skipped, not fatal. + if key == keys[0]: + raise ValueError(f"Invalid '{key}' timestamp in frontmatter: {value!r}") + return None + async def parse_file(self, path: Path | str) -> EntityMarkdown: """Parse markdown file into EntityMarkdown.""" @@ -299,10 +330,17 @@ async def parse_markdown_content( entity_frontmatter = EntityFrontmatter(metadata=metadata) entity_content = parse(post.content) - # Use provided timestamps or current time as fallback + # A user- or BM-supplied timestamp in frontmatter always wins over file stat times, + # so imported/backdated notes (#238) and BM's own written timestamps (#684) survive + # re-parsing. Fall back to file stats, then current time, when frontmatter has neither. now = datetime.now().astimezone() - created = datetime.fromtimestamp(ctime).astimezone() if ctime else now - modified = datetime.fromtimestamp(mtime).astimezone() if mtime else now + created = self._frontmatter_timestamp(metadata, _CREATED_KEYS) + if created is None: + created = datetime.fromtimestamp(ctime).astimezone() if ctime else now + + modified = self._frontmatter_timestamp(metadata, _MODIFIED_KEYS) + if modified is None: + modified = datetime.fromtimestamp(mtime).astimezone() if mtime else now return EntityMarkdown( frontmatter=entity_frontmatter, diff --git a/src/basic_memory/markdown/utils.py b/src/basic_memory/markdown/utils.py index 9d7c55526..e15891ad4 100644 --- a/src/basic_memory/markdown/utils.py +++ b/src/basic_memory/markdown/utils.py @@ -1,6 +1,7 @@ """Utilities for converting between markdown and entity models.""" import uuid +from datetime import datetime from pathlib import Path from typing import Any, Optional @@ -9,7 +10,10 @@ from basic_memory.file_utils import has_frontmatter, remove_frontmatter, parse_frontmatter from basic_memory.markdown import EntityMarkdown -from basic_memory.markdown.entity_parser import normalize_frontmatter_metadata +from basic_memory.markdown.entity_parser import ( + normalize_frontmatter_metadata, + normalize_frontmatter_value, +) from basic_memory.models import Entity from basic_memory.models import Observation as ObservationModel @@ -81,6 +85,44 @@ def entity_model_from_markdown( return model +def apply_default_timestamps( + metadata: dict[str, Any], + *, + incoming_metadata: dict[str, Any], + fallback_created: datetime, + now: datetime, +) -> None: + """Fill in missing `created`/`modified` frontmatter timestamps, in place. + + `metadata` is the full frontmatter about to be written; it may already carry + a `created`/`modified` value inherited from a prior save. `incoming_metadata` + is the subset of that state contributed by *this* write request, used to + distinguish a fresh user-supplied value from one merely carried over from an + earlier write. + + `created` is preserved once set (by BM or by the user) and only filled from + `fallback_created` the first time a note gets one. `modified` is bumped to + `now` on every write BM performs, unless this request explicitly supplied its + own `modified` value (a deliberate override, e.g. importing historical data). + """ + # Treat a null value the same as a missing key: `created:`/`modified:` with no value must + # still get the managed default rather than being written back to disk as YAML null. + if metadata.get("created") is None: + metadata["created"] = fallback_created.isoformat() + if incoming_metadata.get("modified") is None: + metadata["modified"] = now.isoformat() + + # Normalize to ISO strings so both timestamps round-trip identically and match how BM + # already serializes every other frontmatter date (#236). A user-supplied value arrives + # as a YAML-parsed date/datetime object; left as-is it dumps unquoted and reparses to a + # date object, while BM's own isoformat() values are strings that SafeDumper quotes — + # normalizing both here removes that quoted/unquoted split on disk. + for key in ("created", "modified"): + value = metadata.get(key) + if value is not None: + metadata[key] = normalize_frontmatter_value(value) + + async def schema_to_markdown(schema: Any) -> Post: """ Convert schema to markdown Post object. diff --git a/src/basic_memory/models/knowledge.py b/src/basic_memory/models/knowledge.py index cb31ea74d..b461b765e 100644 --- a/src/basic_memory/models/knowledge.py +++ b/src/basic_memory/models/knowledge.py @@ -95,7 +95,11 @@ class Entity(Base): updated_at: Mapped[datetime] = mapped_column( DateTime(timezone=True), default=lambda: datetime.now().astimezone(), - onupdate=lambda: datetime.now().astimezone(), + # No onupdate=now: updated_at is the note's semantic "modified", owned by its markdown + # frontmatter / file mtime and set explicitly on every content write (parse + accepted + # write). Auto-stamping on every row UPDATE let incidental writes (checksum, reindex, + # cloud materialization) clobber a file-authored timestamp — file is source of truth + # (#238/#684). ) # Who created this entity (cloud user_profile_id UUID, null for local/CLI usage) diff --git a/src/basic_memory/services/entity_service.py b/src/basic_memory/services/entity_service.py index 93261efd5..e51e968b4 100644 --- a/src/basic_memory/services/entity_service.py +++ b/src/basic_memory/services/entity_service.py @@ -26,7 +26,11 @@ _coerce_to_string, normalize_frontmatter_metadata, ) -from basic_memory.markdown.utils import entity_model_from_markdown, schema_to_markdown +from basic_memory.markdown.utils import ( + apply_default_timestamps, + entity_model_from_markdown, + schema_to_markdown, +) from basic_memory.models import Entity as EntityModel from basic_memory.models import Observation, Relation from basic_memory.models.knowledge import Entity @@ -98,6 +102,18 @@ def apply_prepared_entity_fields( entity.last_updated_by = user_profile_value +def file_bookkeeping_update(entity: EntityModel, **fields: object) -> dict[str, object]: + """Build a metadata-only entity update that preserves the note's semantic timestamps. + + File bookkeeping (checksum, size, mtime) must never bump created_at/updated_at — the note's + timestamps are owned by its frontmatter/file, not the moment BM happened to touch the row + (file-as-source-of-truth, #238/#684). Because ``updated_at`` carries ``onupdate=now``, the + preserved values have to be written explicitly into the UPDATE; otherwise the flush would + silently re-stamp ``modified`` to the wall clock on every reindex/checksum write. + """ + return {**fields, "created_at": entity.created_at, "updated_at": entity.updated_at} + + @dataclass(frozen=True) class PreparedEntityWrite: """Accepted note state before any persistence side effects happen. @@ -709,6 +725,17 @@ async def prepare_create_entity_content( # Build the final markdown once here. Local mode will write it immediately; cloud mode can # store it in note_content first and materialize later without re-deriving anything. post = await schema_to_markdown(schema) + # Trigger: a brand-new note has no prior frontmatter to inherit from. + # Why: #238/#684 want every note to carry visible created/modified timestamps, but a + # user-supplied value (from schema.content's own frontmatter) must still win. + # Outcome: both fields default to "now" unless the caller already set them. + now = datetime.now().astimezone() + apply_default_timestamps( + post.metadata, + incoming_metadata=post.metadata, + fallback_created=now, + now=now, + ) markdown_content = dump_frontmatter(post) entity_fields = self._build_entity_fields( file_path=file_path, @@ -781,6 +808,20 @@ async def prepare_update_entity_content( merged_metadata.update(post.metadata) merged_metadata["permalink"] = resolved_permalink + # Trigger: this write may be the first to add created/modified, or a later edit to a + # note that already has them. + # Why: created must survive edits once set (#238); modified should reflect the latest + # accepted write unless this request explicitly supplies its own value (#684). + # Outcome: created is filled once (from the entity's own created_at) and preserved + # afterward; modified is bumped to "now" on every BM-initiated write. + now = datetime.now().astimezone() + apply_default_timestamps( + merged_metadata, + incoming_metadata=post.metadata, + fallback_created=entity.created_at or now, + now=now, + ) + merged_post = frontmatter.Post(post.content) merged_post.metadata.update(merged_metadata) @@ -864,6 +905,37 @@ async def prepare_edit_entity_content( session=session, ) + # Trigger: edit operations (append/prepend/find_replace/replace_section) carry the + # pre-existing frontmatter block through; timestamps must be reconciled. + # Why: an edit is a BM-initiated write, so modified should reflect it (#684), while + # created must not move just because the body changed (#238). remove_frontmatter() + # is reused (it strips a BOM the same way has_frontmatter does) so BOM notes stay + # editable; body normalization here matches BM's write path elsewhere. + # Outcome: created is filled once and preserved; modified is bumped to now UNLESS this + # edit explicitly changed a timestamp, in which case the edited value wins. + pre_edit_frontmatter = ( + parse_frontmatter(current_content) if has_frontmatter(current_content) else {} + ) + # An edit "explicitly supplies" a timestamp only when it changed the value the note + # already carried; a value merely carried through from the prior write does not count. + edit_supplied_timestamps = { + key: content_frontmatter[key] + for key in ("created", "modified") + if key in content_frontmatter + and content_frontmatter.get(key) != pre_edit_frontmatter.get(key) + } + now = datetime.now().astimezone() + edit_post = frontmatter.Post(remove_frontmatter(markdown_content)) + edit_post.metadata.update(content_frontmatter) + apply_default_timestamps( + edit_post.metadata, + incoming_metadata=edit_supplied_timestamps, + fallback_created=entity.created_at or now, + now=now, + ) + markdown_content = dump_frontmatter(edit_post) + content_frontmatter = edit_post.metadata + normalized_metadata = normalize_frontmatter_metadata(content_frontmatter or {}) metadata = {k: v for k, v in normalized_metadata.items() if v is not None} or None @@ -1010,7 +1082,9 @@ async def create_entity_with_content(self, schema: EntitySchema) -> EntityWriteR is_new=True, session=session, ) - updated = await self.repository.update(session, entity.id, {"checksum": checksum}) + updated = await self.repository.update( + session, entity.id, file_bookkeeping_update(entity, checksum=checksum) + ) if not updated: # pragma: no cover raise ValueError(f"Failed to update entity checksum after create: {entity.id}") persisted_content, search_content = await self._read_persisted_write_content( @@ -1080,7 +1154,9 @@ async def update_entity_with_content( # Outcome: remove the stale old file so local Basic Memory mirrors cloud's queued cleanup. if not self._paths_share_storage_target(previous_file_path, prepared.file_path): await self.file_service.delete_file(previous_file_path) - entity = await self.repository.update(session, entity.id, {"checksum": checksum}) + entity = await self.repository.update( + session, entity.id, file_bookkeeping_update(entity, checksum=checksum) + ) if not entity: # pragma: no cover raise ValueError( f"Failed to update entity checksum after update: {prepared.file_path}" @@ -1561,7 +1637,9 @@ async def edit_entity_with_content( session=session, ) - entity = await self.repository.update(session, entity.id, {"checksum": checksum}) + entity = await self.repository.update( + session, entity.id, file_bookkeeping_update(entity, checksum=checksum) + ) if not entity: # pragma: no cover raise ValueError(f"Failed to update entity checksum after edit: {file_path}") persisted_content, search_content = await self._read_persisted_write_content(file_path) diff --git a/test-int/mcp/test_frontmatter_timestamps_integration.py b/test-int/mcp/test_frontmatter_timestamps_integration.py new file mode 100644 index 000000000..bce5fa320 --- /dev/null +++ b/test-int/mcp/test_frontmatter_timestamps_integration.py @@ -0,0 +1,138 @@ +""" +Integration tests for created/modified frontmatter timestamps (#238, #684). + +Covers the full file <-> DB round trip: BM auto-fills created/modified on +create, preserves an existing created across edits, and bumps modified on +every write it performs unless the request explicitly supplies its own. +""" + +from datetime import datetime +from pathlib import Path + +import pytest +from fastmcp import Client + +from basic_memory.file_utils import parse_frontmatter + + +def _read_frontmatter(test_project, relative_path: str) -> dict: + file_path = Path(test_project.path) / relative_path + return parse_frontmatter(file_path.read_text(encoding="utf-8")) + + +@pytest.mark.asyncio +async def test_write_note_auto_fills_created_and_modified(mcp_server, app, test_project): + """A note created without explicit timestamps gets created/modified stamped in frontmatter.""" + + async with Client(mcp_server) as client: + await client.call_tool( + "write_note", + { + "project": test_project.name, + "title": "Auto Timestamp Note", + "directory": "timestamps", + "content": "# Auto Timestamp Note\n\nBody text.", + }, + ) + + frontmatter = _read_frontmatter(test_project, "timestamps/Auto Timestamp Note.md") + + assert "created" in frontmatter + assert "modified" in frontmatter + # Both round-trip through ISO 8601 and a freshly created note stamps them identically. + assert datetime.fromisoformat(frontmatter["created"]) == datetime.fromisoformat( + frontmatter["modified"] + ) + + +@pytest.mark.asyncio +async def test_write_note_honors_user_supplied_date_only_created(mcp_server, app, test_project): + """A user-supplied date-only `created` value in frontmatter is preserved verbatim (#238).""" + + async with Client(mcp_server) as client: + await client.call_tool( + "write_note", + { + "project": test_project.name, + "title": "Historical Import", + "directory": "timestamps", + "content": "---\ncreated: 2024-03-15\n---\n# Historical Import\n\nImported body.", + }, + ) + + frontmatter = _read_frontmatter(test_project, "timestamps/Historical Import.md") + + # BM never overwrites a timestamp the frontmatter already carries. + assert frontmatter["created"] == "2024-03-15" + assert "modified" in frontmatter + + +@pytest.mark.asyncio +async def test_edit_note_preserves_created_and_bumps_modified(mcp_server, app, test_project): + """Editing a note keeps its created timestamp but bumps modified (#684).""" + + async with Client(mcp_server) as client: + await client.call_tool( + "write_note", + { + "project": test_project.name, + "title": "Edited Timestamp Note", + "directory": "timestamps", + "content": "# Edited Timestamp Note\n\nOriginal body.", + }, + ) + + before = _read_frontmatter(test_project, "timestamps/Edited Timestamp Note.md") + + await client.call_tool( + "edit_note", + { + "project": test_project.name, + "identifier": "timestamps/edited-timestamp-note", + "operation": "append", + "content": "\n\nAppended body.", + }, + ) + + after = _read_frontmatter(test_project, "timestamps/Edited Timestamp Note.md") + + assert after["created"] == before["created"] + assert datetime.fromisoformat(after["modified"]) >= datetime.fromisoformat( + before["modified"] + ) + + +@pytest.mark.asyncio +async def test_edit_note_preserves_hand_set_created_across_edit(mcp_server, app, test_project): + """A hand-set created date survives a subsequent edit (#238 + #684 agreement).""" + + async with Client(mcp_server) as client: + await client.call_tool( + "write_note", + { + "project": test_project.name, + "title": "Backdated Note", + "directory": "timestamps", + "content": "---\ncreated: 2020-01-01T09:00:00\n---\n# Backdated Note\n\nBody.", + }, + ) + + before = _read_frontmatter(test_project, "timestamps/Backdated Note.md") + assert before["created"] == "2020-01-01T09:00:00" + + await client.call_tool( + "edit_note", + { + "project": test_project.name, + "identifier": "timestamps/backdated-note", + "operation": "append", + "content": "\n\nMore body.", + }, + ) + + after = _read_frontmatter(test_project, "timestamps/Backdated Note.md") + + assert after["created"] == "2020-01-01T09:00:00" + assert datetime.fromisoformat(after["modified"]) >= datetime.fromisoformat( + before["modified"] + ) diff --git a/tests/indexing/test_accepted_note_write_runner.py b/tests/indexing/test_accepted_note_write_runner.py index bdcf18c28..f9e1a3916 100644 --- a/tests/indexing/test_accepted_note_write_runner.py +++ b/tests/indexing/test_accepted_note_write_runner.py @@ -595,6 +595,62 @@ def test_apply_accepted_prepared_entity_fields_updates_mutable_entity() -> None: assert entity.last_updated_by == "user-3" +def test_apply_accepted_prepared_entity_fields_honors_frontmatter_modified() -> None: + """File is source of truth: a note's own modified timestamp wins over the request time. + + Regression for the cloud-accepted path hardcoding updated_at to now (#238/#684): the + prepared frontmatter already carries the note's modified time as an ISO string. + """ + entity = _entity() + request_time = datetime(2026, 6, 19, 13, 0, tzinfo=UTC) + note_modified = datetime(2020, 1, 2, 9, 30, tzinfo=UTC) + + apply_accepted_prepared_entity_fields( + entity, + _PreparedFields( + title="Applied", + note_type="note", + entity_metadata={"modified": note_modified.isoformat()}, + content_type="text/markdown", + permalink="applied", + file_path="notes/applied.md", + ), + updated_at=request_time, + user_profile_value="user-3", + ) + + assert entity.updated_at == note_modified # not request_time + + +def test_accepted_pending_entity_write_uses_frontmatter_timestamps() -> None: + """The create accepted-write seeds created_at/updated_at from frontmatter, not now.""" + now = datetime(2026, 6, 19, 13, 0, tzinfo=UTC) + created = datetime(2019, 5, 1, 8, 0, tzinfo=UTC) + modified = datetime(2021, 7, 2, 16, 45, tzinfo=UTC) + + @dataclass + class _Source: + entity_fields: _PreparedFields + + write = accepted_pending_entity_write_from_prepared( + _Source( + entity_fields=_PreparedFields( + title="Historical", + note_type="note", + entity_metadata={"created": created.isoformat(), "modified": modified.isoformat()}, + content_type="text/markdown", + permalink="historical", + file_path="notes/historical.md", + ) + ), + now=now, + user_profile_value="user-1", + ) + + assert write.created_at == created + assert write.updated_at == modified + + @pytest.mark.asyncio async def test_prepare_accepted_note_move_without_permalink_update_keeps_current_markdown() -> None: session = _FlushSession() diff --git a/tests/indexing/test_batch_indexer.py b/tests/indexing/test_batch_indexer.py index 3766b41e3..e16fd5970 100644 --- a/tests/indexing/test_batch_indexer.py +++ b/tests/indexing/test_batch_indexer.py @@ -202,6 +202,62 @@ async def test_batch_indexer_creates_entities_with_real_db_session( assert beta.title == "Beta" +@pytest.mark.asyncio +async def test_batch_indexer_preserves_frontmatter_timestamps( + app_config, + entity_service, + entity_repository, + relation_repository, + search_service, + file_service, + project_config, +): + """Reindexing must keep a note's frontmatter created/modified, not file stat times. + + Regression (#238/#684): _entity_metadata_updates overwrote created_at/updated_at with the + file's stat times, so a historical `created`/`modified` in frontmatter was lost whenever the + batch indexer (sync/reindex, cloud) touched the note. File is the source of truth. + """ + path = "notes/historical.md" + await _create_file( + project_config.home / path, + dedent( + """ + --- + title: Historical + type: note + created: '2019-03-04T09:00:00' + modified: '2020-06-07T14:30:00' + --- + # Historical + """ + ).strip(), + ) + + batch_indexer = _make_batch_indexer( + app_config, + entity_service, + entity_repository, + relation_repository, + search_service, + file_service, + ) + result = await batch_indexer.index_files( + {path: await _load_input(file_service, path)}, + max_concurrent=1, + parse_max_concurrent=1, + ) + assert result.errors == [] + + async with db.scoped_session(search_service.session_maker) as session: + entity = await entity_repository.get_by_file_path(session, path) + + assert entity is not None + # Frontmatter dates win over the file's (current-year) stat times. + assert (entity.created_at.year, entity.created_at.month, entity.created_at.day) == (2019, 3, 4) + assert (entity.updated_at.year, entity.updated_at.month, entity.updated_at.day) == (2020, 6, 7) + + @pytest.mark.asyncio async def test_batch_indexer_returns_original_markdown_content_when_no_frontmatter_rewrite( app_config, diff --git a/tests/indexing/test_note_materialization_runner.py b/tests/indexing/test_note_materialization_runner.py index 34109ae6e..91f9c7a89 100644 --- a/tests/indexing/test_note_materialization_runner.py +++ b/tests/indexing/test_note_materialization_runner.py @@ -303,6 +303,10 @@ def materialization_entity(*, file_path: str = "notes/a.md") -> Entity: content_type="text/markdown", file_path=file_path, checksum="old-file-sum", + # Semantic timestamp set by the accepted write from the note's frontmatter; materialization + # must preserve it rather than overwrite it with the physical file-write time. + created_at=datetime(2019, 1, 1, 0, 0, tzinfo=UTC), + updated_at=datetime(2020, 1, 2, 9, 30, tzinfo=UTC), ) @@ -558,7 +562,9 @@ async def test_repository_note_materialization_publisher_updates_current_written }, ) ] - assert entity.updated_at == written.file_updated_at + # Semantic updated_at is preserved; the physical write time lands in mtime, not updated_at. + assert entity.updated_at == datetime(2020, 1, 2, 9, 30, tzinfo=UTC) + assert entity.mtime == written.file_updated_at.timestamp() assert entity.size == len(b"# A note\n") assert session.flush_count == 1 diff --git a/tests/markdown/test_entity_parser.py b/tests/markdown/test_entity_parser.py index bfcf46c19..b30c9d668 100644 --- a/tests/markdown/test_entity_parser.py +++ b/tests/markdown/test_entity_parser.py @@ -207,6 +207,143 @@ def test_parse_date_formats(entity_parser): assert entity_parser.parse_date("25:00:00") is None # Invalid time +@pytest.mark.asyncio +async def test_parse_frontmatter_created_modified_full_datetime(entity_parser, tmp_path): + """A full ISO 8601 datetime in frontmatter overrides file stat timestamps (#238).""" + content = dedent(""" + --- + title: Historical Note + type: note + created: 2024-03-15T14:30:00 + modified: 2024-03-16T09:00:00 + --- + + Some historical content. + """) + + entity = await entity_parser.parse_markdown_content( + file_path=tmp_path / "historical.md", + content=content, + mtime=0, + ctime=0, + ) + + assert entity.created is not None + assert entity.modified is not None + assert (entity.created.year, entity.created.month, entity.created.day) == (2024, 3, 15) + assert (entity.created.hour, entity.created.minute) == (14, 30) + assert (entity.modified.year, entity.modified.month, entity.modified.day) == (2024, 3, 16) + assert (entity.modified.hour, entity.modified.minute) == (9, 0) + # Naive frontmatter values are assumed to be local time, not UTC. + assert entity.created.tzinfo is not None + assert entity.modified.tzinfo is not None + + +@pytest.mark.asyncio +async def test_parse_frontmatter_created_date_only(entity_parser, tmp_path): + """A date-only value (no time component) is accepted for created (#238).""" + content = dedent(""" + --- + title: Date Only Note + type: note + created: 2024-03-15 + --- + + Content. + """) + + entity = await entity_parser.parse_markdown_content( + file_path=tmp_path / "date_only.md", + content=content, + mtime=0, + ctime=0, + ) + + assert entity.created is not None + assert (entity.created.year, entity.created.month, entity.created.day) == (2024, 3, 15) + assert entity.created.tzinfo is not None + + +@pytest.mark.asyncio +async def test_parse_frontmatter_created_explicit_timezone_preserved(entity_parser, tmp_path): + """An explicit UTC offset in frontmatter is preserved, not reinterpreted as local (#238).""" + content = dedent(""" + --- + title: Timezone Note + type: note + created: "2024-03-15T14:30:00+05:00" + --- + + Content. + """) + + entity = await entity_parser.parse_markdown_content( + file_path=tmp_path / "tz.md", + content=content, + mtime=0, + ctime=0, + ) + + assert entity.created is not None + assert entity.created.utcoffset().total_seconds() == 5 * 3600 + assert (entity.created.hour, entity.created.minute) == (14, 30) + + +@pytest.mark.asyncio +async def test_parse_frontmatter_timestamp_aliases(entity_parser, tmp_path): + """The date/created_at/updated_at aliases are accepted as cheap read-time conveniences.""" + content = dedent(""" + --- + title: Imported Note + type: note + date: 2023-01-01 + updated_at: 2023-06-15T10:00:00 + --- + + Content. + """) + + entity = await entity_parser.parse_markdown_content( + file_path=tmp_path / "imported.md", + content=content, + mtime=0, + ctime=0, + ) + + assert (entity.created.year, entity.created.month, entity.created.day) == (2023, 1, 1) + assert (entity.modified.year, entity.modified.month, entity.modified.day) == (2023, 6, 15) + + +@pytest.mark.asyncio +async def test_parse_missing_frontmatter_timestamps_falls_back_to_file_stats( + entity_parser, tmp_path +): + """Without frontmatter created/modified, file stat times are used (existing behavior).""" + content = dedent(""" + --- + title: No Timestamps + type: note + --- + + Content. + """) + + mtime = datetime(2022, 5, 1, 12, 0, 0).timestamp() + ctime = datetime(2022, 1, 1, 8, 0, 0).timestamp() + + entity = await entity_parser.parse_markdown_content( + file_path=tmp_path / "no_timestamps.md", + content=content, + mtime=mtime, + ctime=ctime, + ) + + assert entity.created.year == 2022 + assert entity.created.month == 1 + assert entity.modified.year == 2022 + assert entity.modified.month == 5 + + def test_parse_empty_content(): """Test parsing empty or minimal content.""" result = parse("") diff --git a/tests/mcp/test_tool_write_note.py b/tests/mcp/test_tool_write_note.py index bc5c25943..af8bde852 100644 --- a/tests/mcp/test_tool_write_note.py +++ b/tests/mcp/test_tool_write_note.py @@ -1,5 +1,6 @@ """Tests for note tools that exercise the full stack with SQLite.""" +import re from textwrap import dedent from typing import Any @@ -12,6 +13,20 @@ from basic_memory.repository.relation_repository import RelationRepository +def _without_managed_timestamps(markdown: str | dict[str, Any]) -> str: + """Drop BM's auto-managed created/modified frontmatter lines (#238/#684). + + Those timestamps are added to every note and are dynamic, so exact-content + assertions strip them and match on the rest of the note. The timestamp + behavior itself is covered by test_frontmatter_timestamps_integration.py. + + Accepts read_note()'s ``str | dict`` return and narrows to the text form the + note assertions use, so callers don't each repeat the isinstance guard. + """ + assert isinstance(markdown, str) + return re.sub(r"^(?:created|modified): .*\n", "", markdown, flags=re.MULTILINE) + + # --------------------------------------------------------------------------- # _compose_workspace_project_route unit tests # --------------------------------------------------------------------------- @@ -160,7 +175,10 @@ async def test_write_note(app, test_project): .format(permalink=f"{test_project.name}/test/test-note") .strip() ) - assert expected in content + assert expected in _without_managed_timestamps(content) + # write_note now stamps created/modified into frontmatter (#684) + assert "created:" in content + assert "modified:" in content @pytest.mark.asyncio @@ -191,7 +209,7 @@ async def test_write_note_no_tags(app, test_project): .format(permalink=f"{test_project.name}/test/simple-note") .strip() ) - assert expected in content + assert expected in _without_managed_timestamps(content) @pytest.mark.asyncio @@ -257,7 +275,7 @@ async def test_write_note_update_existing(app, test_project): ) .format(permalink=f"{test_project.name}/test/test-note") .strip() - == content + == _without_managed_timestamps(content) ) @@ -572,7 +590,7 @@ async def test_write_note_preserves_content_frontmatter(app, test_project): ) .format(permalink=f"{test_project.name}/test/test-note") .strip() - in content + in _without_managed_timestamps(content) ) @@ -723,7 +741,7 @@ async def test_write_note_with_custom_note_type(app, test_project): .format(permalink=f"{test_project.name}/guides/test-guide") .strip() ) - assert expected in content + assert expected in _without_managed_timestamps(content) @pytest.mark.asyncio diff --git a/tests/services/test_entity_service.py b/tests/services/test_entity_service.py index 9410c838c..1aca7f9be 100644 --- a/tests/services/test_entity_service.py +++ b/tests/services/test_entity_service.py @@ -1,5 +1,6 @@ """Tests for EntityService.""" +import re import uuid from pathlib import Path from textwrap import dedent @@ -28,6 +29,16 @@ def _permalink(entity: EntityModel | EntitySchema) -> str: return permalink +def _without_managed_timestamps(markdown: str) -> str: + """Drop BM's auto-managed created/modified frontmatter lines (#238/#684). + + Those timestamps are stamped into every note and are dynamic, so exact file + content assertions strip them and match on the rest of the note. The + timestamp behavior itself is covered by test_frontmatter_timestamps_integration.py. + """ + return re.sub(r"^(?:created|modified): .*\n", "", markdown, flags=re.MULTILINE) + + @pytest.mark.parametrize( ("lines", "expected"), [ @@ -206,7 +217,7 @@ async def test_create_entity_file_exists( file_content, _ = await file_service.read_file(file_path) assert ( f"---\ntitle: Test Entity\ntype: test\npermalink: {generate_permalink(project_config.name)}/test-entity\n---\n\nfirst" - == file_content + == _without_managed_timestamps(file_content) ) entity_data = EntitySchema( @@ -659,7 +670,35 @@ async def test_create_with_content(entity_service: EntityService, file_service: See the [[Git Cheat Sheet]] for reference. """).strip() - assert expected == file_content + assert expected == _without_managed_timestamps(file_content) + + +@pytest.mark.asyncio +async def test_create_preserves_frontmatter_timestamps_through_checksum_update( + entity_service: EntityService, +): + """The post-write checksum UPDATE must not clobber the note's created/modified. + + Regression (#238/#684): entity_service records the file checksum with a second UPDATE, and + updated_at's onupdate=now default would re-stamp modified to the wall clock unless the + semantic timestamps are preserved explicitly — the file, not the write moment, owns them. + """ + content = dedent( + """ + --- + created: '2019-03-04T09:00:00+00:00' + modified: '2020-06-07T14:30:00+00:00' + --- + # Historical + """ + ).strip() + entity, created = await entity_service.create_or_update_entity( + EntitySchema(title="Historical", directory="notes", note_type="note", content=content) + ) + + assert created is True + assert (entity.created_at.year, entity.created_at.month, entity.created_at.day) == (2019, 3, 4) + assert (entity.updated_at.year, entity.updated_at.month, entity.updated_at.day) == (2020, 6, 7) @pytest.mark.asyncio @@ -704,7 +743,7 @@ async def test_update_with_content( # Git Workflow Guide """ ).strip() - == file_content + == _without_managed_timestamps(file_content) ) # now update the content @@ -768,7 +807,7 @@ async def test_update_with_content( file_content, _ = await file_service.read_file(file_path) # assert content is in file - assert update_content.strip() == file_content + assert update_content.strip() == _without_managed_timestamps(file_content) @pytest.mark.asyncio diff --git a/tests/services/test_entity_service_prepare.py b/tests/services/test_entity_service_prepare.py index dcf3bcab0..0485c116a 100644 --- a/tests/services/test_entity_service_prepare.py +++ b/tests/services/test_entity_service_prepare.py @@ -3,6 +3,7 @@ from __future__ import annotations from dataclasses import FrozenInstanceError +from datetime import datetime import pytest @@ -10,7 +11,21 @@ from basic_memory.repository import AcceptedObservationWrite, AcceptedRelationWrite from basic_memory.schemas import Entity as EntitySchema from basic_memory.services.exceptions import EntityAlreadyExistsError -from basic_memory.services.entity_service import PreparedEntityFields + + +def _drop_timestamps(metadata: dict) -> dict: + """Strip created/modified before comparing metadata from two independent prepare calls. + + Each call stamps its own write time (#238/#684), so two otherwise-identical prepare + invocations legitimately produce different created/modified values; the parity tests + below only care that everything else matches. + """ + return {k: v for k, v in metadata.items() if k not in ("created", "modified")} + + +def _frontmatter_and_body_ignoring_timestamps(content: str) -> tuple[dict, str]: + """Parse (metadata, body), dropping created/modified (see `_drop_timestamps`).""" + return _drop_timestamps(parse_frontmatter(content)), remove_frontmatter(content) @pytest.mark.asyncio @@ -28,12 +43,16 @@ async def test_prepare_create_entity_content_matches_create_entity_with_content( result = await entity_service.create_entity_with_content(schema) assert prepared.file_path.as_posix() == result.entity.file_path - assert prepared.markdown_content == result.content + assert _frontmatter_and_body_ignoring_timestamps( + prepared.markdown_content + ) == _frontmatter_and_body_ignoring_timestamps(result.content) assert prepared.search_content == result.search_content assert prepared.entity_fields.title == result.entity.title assert prepared.entity_fields.note_type == result.entity.note_type assert prepared.entity_fields.permalink == result.entity.permalink - assert prepared.entity_fields.entity_metadata == result.entity.entity_metadata + assert _drop_timestamps(prepared.entity_fields.entity_metadata) == _drop_timestamps( + result.entity.entity_metadata + ) @pytest.mark.asyncio @@ -47,19 +66,26 @@ async def test_prepare_create_entity_content_returns_typed_entity_fields(entity_ ) ) - assert prepared.entity_fields == PreparedEntityFields( - title="Typed Fields", - note_type="decision", - entity_metadata={ - "title": "Typed Fields", - "type": "decision", - "status": "accepted", - "permalink": "test-project/notes/typed-fields", - }, - content_type="text/markdown", - permalink="test-project/notes/typed-fields", - file_path="notes/Typed Fields.md", + # created/modified are auto-filled with the current write time (#238/#684); a fresh + # note stamps both the same, so compare everything else exactly and check those two + # separately rather than hardcoding a timestamp. + entity_metadata = prepared.entity_fields.entity_metadata + assert entity_metadata is not None + assert datetime.fromisoformat(entity_metadata["created"]) == datetime.fromisoformat( + entity_metadata["modified"] ) + other_metadata = {k: v for k, v in entity_metadata.items() if k not in ("created", "modified")} + assert other_metadata == { + "title": "Typed Fields", + "type": "decision", + "status": "accepted", + "permalink": "test-project/notes/typed-fields", + } + assert prepared.entity_fields.title == "Typed Fields" + assert prepared.entity_fields.note_type == "decision" + assert prepared.entity_fields.content_type == "text/markdown" + assert prepared.entity_fields.permalink == "test-project/notes/typed-fields" + assert prepared.entity_fields.file_path == "notes/Typed Fields.md" with pytest.raises(FrozenInstanceError): setattr(prepared.entity_fields, "title", "Changed") @@ -166,7 +192,9 @@ async def test_prepare_update_entity_content_matches_update_entity_with_content( result = await entity_service.update_entity_with_content(created, update_schema) prepared_frontmatter = parse_frontmatter(prepared.markdown_content) - assert prepared.markdown_content == result.content + assert _frontmatter_and_body_ignoring_timestamps( + prepared.markdown_content + ) == _frontmatter_and_body_ignoring_timestamps(result.content) assert prepared.search_content == result.search_content assert prepared.entity_fields.title == result.entity.title assert prepared.entity_fields.note_type == result.entity.note_type @@ -174,6 +202,8 @@ async def test_prepare_update_entity_content_matches_update_entity_with_content( assert prepared_frontmatter["owner"] == "alice" assert prepared_frontmatter["status"] == "published" assert prepared_frontmatter["reviewed_by"] == "bob" + # created is inherited from the existing note (not re-stamped); modified is bumped. + assert prepared_frontmatter["created"] == parse_frontmatter(existing_content)["created"] @pytest.mark.asyncio @@ -208,7 +238,9 @@ async def test_prepare_update_entity_content_can_change_file_path( assert prepared.file_path.as_posix() == "journal/Renamed Note.md" assert result.entity.file_path == "journal/Renamed Note.md" - assert result.content == prepared.markdown_content + assert _frontmatter_and_body_ignoring_timestamps( + result.content + ) == _frontmatter_and_body_ignoring_timestamps(prepared.markdown_content) assert prepared.entity_fields.permalink != created.permalink assert prepared.entity_fields.permalink == result.entity.permalink assert not await file_service.exists("notes/Original Name.md") @@ -358,7 +390,9 @@ async def test_prepare_edit_entity_content_matches_edit_entity_with_content( find_text="Before edit", ) - assert prepared.markdown_content == result.content + assert _frontmatter_and_body_ignoring_timestamps( + prepared.markdown_content + ) == _frontmatter_and_body_ignoring_timestamps(result.content) assert prepared.search_content == result.search_content assert prepared.entity_fields.title == result.entity.title assert prepared.entity_fields.note_type == result.entity.note_type @@ -418,7 +452,14 @@ async def test_prepare_edit_entity_content_prepend_preserves_valid_frontmatter( content="Prepended line", ) - assert parse_frontmatter(prepared.markdown_content) == { + # Prepending is a BM-initiated write: created (stamped at create time) is preserved, + # and modified is bumped to reflect this edit (#238/#684). + parsed_frontmatter = parse_frontmatter(prepared.markdown_content) + created_ts = parsed_frontmatter.pop("created") + modified_ts = parsed_frontmatter.pop("modified") + assert datetime.fromisoformat(created_ts) is not None + assert datetime.fromisoformat(modified_ts) is not None + assert parsed_frontmatter == { "title": "Prepared Prepend Frontmatter", "type": "note", "status": "draft",