fix(kg): accept ISO datetimes for temporal inputs

[fatkobra]

What does this PR do?

Fixes #1374.

This expands KG temporal validation so MemPalace accepts canonical UTC ISO datetimes in addition to full dates, while preserving safe TEXT-based temporal comparisons in the knowledge graph.

What changed

• Added sanitize_iso_temporal().
• Kept sanitize_iso_date() as a backward-compatible wrapper.
• Accepted temporal forms are intentionally narrow:
  • YYYY-MM-DD
  • YYYY-MM-DDTHH:MM:SSZ
  • YYYY-MM-DDTHH:MM:SS+00:00, normalized to Z
• Rejected non-canonical datetime forms:
  • naive datetimes
  • non-UTC timezone offsets
  • fractional seconds
  • space-separated datetimes
  • partial dates / partial datetimes
• Added calendar validation so impossible values like 2026-02-31 are rejected.
• Updated KnowledgeGraph to validate temporal inputs at the core layer, not only the MCP layer.
• Updated KG temporal comparisons so legacy date-only facts still work with datetime as_of queries:
  • date-only valid_from compares as start-of-day
  • date-only valid_to compares as end-of-day
• Kept temporal filtering SQL-side using normalized CASE expressions, so SQLite filters rows before Python materializes them.
• Updated MCP KG call sites to use sanitize_iso_temporal() for:
  • as_of
  • valid_from
  • valid_to
  • ended
• Updated MCP tool descriptions to document the exact accepted datetime form.
• Added regression tests for:
  • canonical UTC datetime inputs
  • +00:00 normalization to Z
  • legacy date-only facts queried with datetime as_of
  • rejected timezone-offset datetimes
  • rejected naive / fractional / space-separated datetimes
  • date-only valid_to interval semantics
  • query_entity() and query_relationship() temporal filtering
• Added KG SQLite cleanup / test isolation changes needed for CI:
  • MCP KG cache cleanup no longer imports mcp_server just to clear caches
  • KnowledgeGraph supports explicit context-manager cleanup
  • palace-lock multiprocessing tests use spawn to avoid inherited fork state
  • child processes are terminated in finally so failed tests cannot hang CI

Why

#1374 reports that KG/MCP temporal inputs reject legitimate sub-day timestamps such as:

2026-05-06T14:23:00Z

The KG stores temporal values as TEXT, so accepting many ISO-like formats would be unsafe: mixed formats can sort incorrectly and silently return wrong facts.

This PR accepts only canonical UTC second-level datetimes plus full dates, while preserving backward compatibility for existing date-only KG facts.

How to test

ruff format mempalace/config.py mempalace/knowledge_graph.py mempalace/mcp_server.py tests/conftest.py 
ruff check mempalace/config.py mempalace/knowledge_graph.py mempalace/mcp_server.py tests/conftest.py
tests/test_config.py tests/test_knowledge_graph.py tests/test_mcp_server.py tests/test_palace_locks.py
python -m pytest tests/test_config.py tests/test_knowledge_graph.py -q
python -m pytest tests/test_mcp_server.py::TestKGTools -q
python -m pytest tests/test_palace_locks.py -q
python -m pytest tests/ -v

Checklist

• Tests pass (python -m pytest tests/ -v)
• No hardcoded paths
• Linter passes (ruff check .)

[lealbrunocalhau]

Thanks for tackling #1374 — the test coverage and backward-compat wrapper are well done. However, I found critical correctness bugs before merge.

The problem

Expanding accepted formats from 1 to 6 (date, datetime with/without TZ, T vs space separator, fractional seconds) introduces silent data loss and wrong query results. The KG uses lexicographic TEXT comparison in SQL:

AND (t.valid_from IS NULL OR t.valid_from <= ?) AND (t.valid_to IS NULL OR t.valid_to >= ?)

This only works when all temporal values share the same format. Before this PR that was guaranteed (only YYYY-MM-DD accepted). After this PR, mixed formats produce wrong answers — none of which the new tests catch because they use consistent formats.

Reproduced bugs

Bug 1 - Data loss for legacy date-only facts (CRITICAL)

kg.add_triple("Alice", "ate_at", "Cafe", valid_from="2026-05-06", valid_to="2026-05-06")
result = kg.query_entity("Alice", as_of="2026-05-06T15:00:00Z")
# Expected: 1 fact. Actual: 0 facts

Cause: "2026-05-06" >= "2026-05-06T15:00:00Z" is False (shorter prefix sorts first). Every palace with existing date-only data will silently lose facts when queried with sub-day precision after this ships.

Bug 2 - Mixed timezones → false positives (CRITICAL)

kg.add_triple("Bob", "works_at", "Globex", valid_from="2026-05-06T20:30:00-05:00")  # = 01:30Z next day
result = kg.query_entity("Bob", as_of="2026-05-07T01:00:00Z")  # 30 min BEFORE fact exists
# Expected: 0 facts. Actual: 1 fact

The KG returns facts that don't exist at the queried time.

Bug 3 - Mixed timezones → false negatives (CRITICAL)

kg.add_triple("Carol", "is_in", "NYC", valid_from="2026-05-07T01:23:00+05:00")  # = 20:23Z
result = kg.query_entity("Carol", as_of="2026-05-06T20:30:00Z")  # 7 min AFTER fact starts
# Expected: 1 fact. Actual: 0 facts

The KG silently drops facts that should match.

Bug 4 - Valid intervals rejected (HIGH)

kg.add_triple("Eve", "is_in", "London",
              valid_from="2026-05-06T15:00:00",    # T
              valid_to="2026-05-06 20:00:00")      # space
# Raises: "valid_to is before valid_from"

This is a valid 5-hour interval. The inverted-interval check at knowledge_graph.py:177 uses lexicographic comparison: space (0x20) < T (0x54), so it incorrectly rejects the interval.

Suggested fixes

Option A (recommended): Restrict to one canonical format — accept only YYYY-MM-DD or YYYY-MM-DDTHH:MM:SSZ (UTC only). Reject space separators, naive datetimes, and non-Z offsets. Smallest change, delivers #1374 value, preserves correctness.

Option B: Normalize on store. Convert all inputs to canonical UTC form before persisting. Loses verbatim round-trip but fixes comparisons.

Option C: Revert and split. Keep storage at date-only, accept datetime only in as_of queries. Resolves #1374 without touching writes.

Please add tests for mixed-format scenarios — current tests only exercise consistent formats, which is why these bugs slip through. Happy to help draft the fix if useful.

[fatkobra]

Very good catch and excellent feedback @lealbrunocalhau

Working on it. Thank you.

[fatkobra]

Thanks for the detailed review — you were right. The first version accepted too many temporal shapes for a KG that stores and compares temporal values as TEXT.

I updated the PR to use the safer canonical approach:

• accepted forms are now limited to YYYY-MM-DD and YYYY-MM-DDTHH:MM:SSZ;
• timezone offsets, naive datetimes, fractional seconds, and space-separated datetimes are rejected;
• KnowledgeGraph now validates temporal inputs at the core layer, not only MCP;
• legacy date-only facts still match datetime as_of queries by treating date-only valid_from as start-of-day and date-only valid_to as end-of-day;
• interval validation now uses temporal comparison keys instead of raw string comparison.

I also added regression tests for the mixed-format cases you called out, including the legacy date-only fact queried with a datetime as_of.

@lealbrunocalhau please check it out?

[Copilot]

Pull request overview

Expands MemPalace KnowledgeGraph temporal handling to accept canonical UTC ISO datetimes (YYYY-MM-DDTHH:MM:SSZ) in addition to full dates (YYYY-MM-DD), while preserving correct TEXT-based ordering semantics and improving temporal validation at the KG core layer.

Changes:

• Added sanitize_iso_temporal() (with calendar validation) and kept sanitize_iso_date() as a backward-compatible wrapper.
• Updated MCP KG tools and KG core (KnowledgeGraph) to validate temporal inputs and support legacy date-only facts with datetime as_of queries (start-of-day / end-of-day semantics).
• Added regression tests across config, MCP server, and KG temporal querying/invalidation.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
mempalace/config.py	Introduces sanitize_iso_temporal() and updates sanitize_iso_date() to wrap it.
mempalace/knowledge_graph.py	Validates temporals at the KG layer and adjusts temporal comparisons for date-only vs datetime compatibility.
mempalace/mcp_server.py	Switches KG tool call sites from sanitize_iso_date() to sanitize_iso_temporal() and updates tool schema descriptions.
tests/test_config.py	Adds unit tests for accepted/rejected temporal formats and wrapper behavior.
tests/test_mcp_server.py	Adds MCP-level regression tests for datetime acceptance and rejection of non-canonical forms.
tests/test_knowledge_graph.py	Adds KG-core tests for datetime/date-only compatibility and temporal validation errors.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[igorls]

Concerns from review

Copilot's perf comments on query_entity and query_relationship are worth taking seriously, and I would echo them.

Pulling the temporal predicate out of SQL means every row for the entity (or every row for the predicate) is materialized in Python and then filtered. For an entity with many facts, or a popular predicate like does / works_at with thousands of triples, this scales poorly.

It also discards the benefit of idx_triples_valid.

A SQL-side normalization keeps correctness while letting SQLite filter before transfer:

CASE WHEN length(valid_to) = 10 THEN valid_to || 'T23:59:59Z' ELSE valid_to END

...with the symmetric one for valid_from. Given the project's "memory should feel instant" budget, I think this is worth fixing here rather than as a follow-up.

---

Stale docstring

add_triple()'s example still shows valid_from="2026-01", valid_to="2026-02".

Those are now rejected by sanitize_iso_temporal, so the public example does not run.

---

+00:00 rejection

datetime.now(timezone.utc).isoformat() emits 2026-05-06T14:23:00+00:00, semantically identical to ...Z but rejected.

Worth either normalizing +00:00 to Z inside sanitize_iso_temporal, or documenting the expected idiom. It is a common integration pattern that will silently fail otherwise.

---

CI

Linux 3.13 and macOS jobs hang about 90 minutes after the test suite reports 1 failed, 1674 passed in 77.03s. Develop is green.

The failing test is test_palace_locks::test_reentrant_same_thread_passes_through (multiprocessing, _queue.Empty) and looks unrelated to KG.

But the 233 ResourceWarning: unclosed database warnings during the run point to KG/SQLite connections not being torn down, and Python 3.13 finalizes more strictly.

Worth investigating before merge since it blocks two of six matrix legs.

[fatkobra]

@igorls would appreciate another review, you or Copilot. Could you please trigger it?
Test-macos and linux3.13 still failing with the following, but the other issue is cleared:
=========================== short test summary info ============================ FAILED tests/test_palace_locks.py::test_reentrant_same_thread_passes_through - _queue.Empty ====== 1 failed, 1677 passed, 1 skipped, 234 warnings in 76.91s (0:01:16) ======

The failing test was test_palace_locks::test_reentrant_same_thread_passes_through, which forks a child process while the parent holds a palace lock. In the full Linux/macOS CI suite, that child can inherit open file descriptors and module state from earlier Chroma/MCP/KG imports, then fail to report back to the queue. That explains the _queue.Empty failure and the long CI hang after pytest reports the failure.

I can make the following changes:

• tests/conftest.py to no longer import mempalace.mcp_server just to clear caches; it only clears MCP caches if the module is already loaded.
• test_reentrant_same_thread_passes_through to use a clean spawn context for the child assertion, so the child does not inherit the parent's open lock fd or SQLite/Chroma process state. No more fork.
• Make the test to terminate the child in finally if it fails to report back, preventing the long post-failure CI hang.

What do you think?

[igorls]

Your diagnosis matches the symptoms exactly.

The 233 ResourceWarning: unclosed database warnings are direct evidence of the fork inheritance, and the post-pytest hang is consistent with an orphan child that cannot report to the queue. Windows passes precisely because _get_mp_context() already returns spawn there.

All three of your proposed changes look right to me:

1. Lazy import in conftest is the broad fix. It reduces parent-process state across every fork-using test, not just this one.

2. Spawn context for the reentrant test is the targeted fix. The child gets a fresh interpreter and cannot inherit the parent's flock fd or DB handles.

3. Finally-terminate guard is defensive cleanup. A future regression in the same area cannot hang CI for 90 minutes again.

Please go ahead with all three.

One follow-up thought, not blocking this PR: _get_mp_context() defaulting to fork on POSIX is going to keep biting as the codebase grows more import-heavy. Python 3.14 will change the default to forkserver/spawn for exactly this reason. Worth considering an "always spawn" default in that helper at some point so we are not hand-fixing each test that hits the pattern.

Canceling the currently-pending CI run since it will hit the same hang. Push the fix and trigger a fresh one.

[fatkobra]

test-macos and test-linux3.13 are now passing, but lint is failing.

Run ruff check . ruff check . shell: /usr/bin/bash -e {0} env: pythonLocation: /opt/hostedtoolcache/Python/3.11.15/x64 PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.11.15/x64/lib/pkgconfig Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.15/x64 Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.15/x64 Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.15/x64 LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.11.15/x64/lib All checks passed! 0s Run ruff format --check . ruff format --check . shell: /usr/bin/bash -e {0} env: pythonLocation: /opt/hostedtoolcache/Python/3.11.15/x64 PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.11.15/x64/lib/pkgconfig Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.15/x64 Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.15/x64 Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.15/x64 LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.11.15/x64/lib Would reformat: tests/test_palace_locks.py 1 file would be reformatted, 121 files already formatted

But it does not say which line would be formatted. I tried several formatting and pushed a formatting-only follow-up for tests/test_palace_locks.py to no avail.

@igorls I need help here, perhaps could you please run ruff formatting in the CI?

I also updated the PR description to reflect the current implementation: canonical UTC datetime support, +00:00 normalization, SQL-side temporal filtering, KG cleanup, and the palace-lock test isolation changes.

[fatkobra]

Pushed the CI/process-isolation fix Igor agreed with.

What changed:

• tests/conftest.py no longer imports mempalace.mcp_server just to clear caches. It only clears MCP caches if the module is already loaded.
• _get_mp_context() in tests/test_palace_locks.py now always uses spawn, including Linux/macOS. This avoids inheriting open lock fds, SQLite handles, and Chroma/MCP module state from the parent process.
• test_reentrant_same_thread_passes_through now terminates the child in finally if it fails to report back, so CI cannot hang for 90 minutes after a failure.

I also verified that the Copilot SQL-filtering concern is addressed in the current branch: both query_entity() and query_relationship() use _temporal_filter_sql(as_of), so temporal filtering stays SQL-side with date-only normalization instead of filtering every row in Python.