Skip to content

Commit abe8576

Browse files
committed
fix(kg): reject partial ISO dates to avoid silent empty result sets
Per qodo-ai review on PR #1167: sanitize_iso_date() previously accepted YYYY and YYYY-MM, but KnowledgeGraph.query_entity() compares valid_from/ valid_to TEXT columns lexicographically against as_of. Lexicographic comparison treats '2026-01-01' as greater than '2026' (because '-' > end-of-string), so partial as_of values silently excluded valid facts — re-introducing the silent-empty-results problem this PR was meant to fix. Tighten _ISO_DATE_RE to require YYYY-MM-DD only. Update docstring and error message accordingly. Invert the two test cases that asserted partials were accepted.
1 parent 4d98b05 commit abe8576

3 files changed

Lines changed: 30 additions & 15 deletions

File tree

mempalace/config.py

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -85,16 +85,21 @@ def sanitize_kg_value(value: str, field_name: str = "value") -> str:
8585
# (as_of, valid_from, valid_to, ended). Parameterized queries already
8686
# prevent SQL injection, but unvalidated date strings silently miss
8787
# every row — callers cannot distinguish "no fact at this time" from
88-
# "your date format was unrecognized." Accept YYYY, YYYY-MM, YYYY-MM-DD.
89-
_ISO_DATE_RE = re.compile(r"^\d{4}(?:-(?:0[1-9]|1[0-2])(?:-(?:0[1-9]|[12]\d|3[01]))?)?$")
88+
# "your date format was unrecognized." Require full YYYY-MM-DD: KG
89+
# queries compare TEXT dates lexicographically, so partials like "2026"
90+
# would re-introduce silent empty results (e.g. "2026-01-01" <= "2026"
91+
# is False), defeating the purpose of validation.
92+
_ISO_DATE_RE = re.compile(r"^\d{4}-(?:0[1-9]|1[0-2])-(?:0[1-9]|[12]\d|3[01])$")
9093

9194

9295
def sanitize_iso_date(value, field_name: str = "date"):
9396
"""Validate an ISO-8601 date string, accepting None or empty as-is.
9497
95-
Accepts ``YYYY``, ``YYYY-MM``, or ``YYYY-MM-DD``. Raises ValueError
96-
on any other non-empty input so the MCP layer can surface a clear
97-
error to the caller instead of silently returning empty results.
98+
Accepts only ``YYYY-MM-DD``. Raises ValueError on any other
99+
non-empty input so the MCP layer can surface a clear error to the
100+
caller instead of silently returning empty results. Partial dates
101+
(``YYYY``, ``YYYY-MM``) are rejected because KG queries compare
102+
TEXT dates lexicographically and would silently exclude valid facts.
98103
"""
99104
if value is None or value == "":
100105
return value
@@ -103,8 +108,7 @@ def sanitize_iso_date(value, field_name: str = "date"):
103108
value = value.strip()
104109
if not _ISO_DATE_RE.match(value):
105110
raise ValueError(
106-
f"{field_name}={value!r} is not a valid ISO-8601 date "
107-
f"(expected YYYY, YYYY-MM, or YYYY-MM-DD)"
111+
f"{field_name}={value!r} is not a valid ISO-8601 date " f"(expected YYYY-MM-DD)"
108112
)
109113
return value
110114

tests/test_config.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -223,12 +223,16 @@ def test_kg_value_rejects_over_length():
223223
# --- sanitize_iso_date ---
224224

225225

226-
def test_iso_date_accepts_year_only():
227-
assert sanitize_iso_date("2026") == "2026"
226+
def test_iso_date_rejects_year_only():
227+
# Partial dates re-introduce silent empty result sets via lexicographic
228+
# TEXT comparison in KG queries (e.g. "2026-01-01" <= "2026" is False).
229+
with pytest.raises(ValueError):
230+
sanitize_iso_date("2026")
228231

229232

230-
def test_iso_date_accepts_year_month():
231-
assert sanitize_iso_date("2026-03") == "2026-03"
233+
def test_iso_date_rejects_year_month():
234+
with pytest.raises(ValueError):
235+
sanitize_iso_date("2026-03")
232236

233237

234238
def test_iso_date_accepts_full_date():

tests/test_mcp_server.py

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -702,14 +702,21 @@ def test_kg_invalidate_rejects_invalid_ended(self, monkeypatch, config, palace_p
702702
assert result["success"] is False
703703
assert "ended" in result["error"]
704704

705-
def test_kg_query_accepts_partial_iso_dates(self, monkeypatch, config, palace_path, seeded_kg):
705+
def test_kg_query_rejects_partial_iso_dates(self, monkeypatch, config, palace_path, seeded_kg):
706706
_patch_mcp_server(monkeypatch, config, seeded_kg)
707707
from mempalace.mcp_server import tool_kg_query
708708

709-
# YYYY and YYYY-MM are valid ISO-8601 forms — must not be rejected.
710-
for value in ("2026", "2026-03", "2026-03-15"):
709+
# Partial ISO dates are rejected: KG queries compare TEXT dates
710+
# lexicographically, so "2026-01-01" <= "2026" is False, which
711+
# silently excludes facts. Reject at the boundary — only YYYY-MM-DD
712+
# produces correct results.
713+
for value in ("2026", "2026-03"):
711714
result = tool_kg_query(entity="Max", as_of=value)
712-
assert "error" not in result, f"rejected valid date {value!r}: {result}"
715+
assert "error" in result, f"accepted partial date {value!r}: {result}"
716+
717+
# Full ISO-8601 dates still pass.
718+
result = tool_kg_query(entity="Max", as_of="2026-03-15")
719+
assert "error" not in result, f"rejected valid date: {result}"
713720

714721

715722
# ── Diary Tools ─────────────────────────────────────────────────────────

0 commit comments

Comments
 (0)