From 1dbff180a3475d2d66641375f01bf5bbaf102b08 Mon Sep 17 00:00:00 2001 From: sanil-23 Date: Wed, 20 May 2026 02:01:10 +0530 Subject: [PATCH] fix(memory): accept ISO-8601 / missing modified_at + default provider (#2237) --- .../memory/tree/canonicalize/document.rs | 140 +++++++++++++++++- 1 file changed, 138 insertions(+), 2 deletions(-) diff --git a/src/openhuman/memory/tree/canonicalize/document.rs b/src/openhuman/memory/tree/canonicalize/document.rs index da39d454f..191426b2d 100644 --- a/src/openhuman/memory/tree/canonicalize/document.rs +++ b/src/openhuman/memory/tree/canonicalize/document.rs @@ -6,22 +6,92 @@ //! is kept verbatim. use chrono::{DateTime, Utc}; -use serde::{Deserialize, Serialize}; +use serde::{Deserialize, Deserializer, Serialize}; use super::{normalize_source_ref, CanonicalisedSource}; use crate::openhuman::memory::tree::types::{Metadata, SourceKind}; +// ── Serde helpers ───────────────────────────────────────────────────────────── + +fn default_provider() -> String { + "unknown".to_string() +} + +fn now_utc() -> DateTime { + Utc::now() +} + +/// Deserialise a `DateTime` from either: +/// - a JSON integer = epoch **milliseconds** (legacy callers — back-compat), +/// - a JSON string = RFC 3339 / ISO-8601 (e.g. `"2026-05-17T19:30:00Z"`), or +/// a decimal string containing epoch milliseconds. +/// +/// On an unparseable string a serde error is returned (no silent default). +fn deserialize_flexible_timestamp<'de, D>(deserializer: D) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + /// Untagged helper so serde tries each variant in order. + #[derive(Deserialize)] + #[serde(untagged)] + enum RawTs { + Millis(i64), + Text(String), + } + + let raw = RawTs::deserialize(deserializer)?; + match raw { + RawTs::Millis(ms) => { + tracing::debug!("[memory][document] parsed modified_at as epoch-ms: {ms}"); + chrono::TimeZone::timestamp_millis_opt(&Utc, ms) + .single() + .ok_or_else(|| serde::de::Error::custom(format!("invalid epoch-ms: {ms}"))) + } + RawTs::Text(s) => { + // Try RFC 3339 / ISO-8601 first. + if let Ok(dt) = DateTime::parse_from_rfc3339(&s) { + tracing::debug!("[memory][document] parsed modified_at as ISO-8601 string: {s}"); + return Ok(dt.with_timezone(&Utc)); + } + // Fall back: numeric string = epoch milliseconds. + if let Ok(ms) = s.parse::() { + tracing::debug!( + "[memory][document] parsed modified_at as numeric-string epoch-ms: {s}" + ); + return chrono::TimeZone::timestamp_millis_opt(&Utc, ms) + .single() + .ok_or_else(|| { + serde::de::Error::custom(format!("invalid epoch-ms string: {s}")) + }); + } + Err(serde::de::Error::custom(format!( + "modified_at: cannot parse '{s}' as RFC 3339 or epoch-ms" + ))) + } + } +} + +// ── Input struct ────────────────────────────────────────────────────────────── + /// Adapter input for a single document. #[derive(Clone, Debug, Serialize, Deserialize)] pub struct DocumentInput { /// Provider name (e.g. `notion`, `drive`, `meeting_notes`). + /// Defaults to `"unknown"` when absent (fixes CORE-31). + #[serde(default = "default_provider")] pub provider: String, /// Document title. pub title: String, /// Document body (markdown preferred; plain text also accepted). pub body: String, /// When the document was last modified at the source. - #[serde(with = "chrono::serde::ts_milliseconds")] + /// + /// Accepts epoch-milliseconds integer (back-compat), RFC 3339 / ISO-8601 + /// string (fixes CORE-2K), or absent → `Utc::now()` (fixes CORE-2J). + #[serde( + default = "now_utc", + deserialize_with = "deserialize_flexible_timestamp" + )] pub modified_at: DateTime, /// Optional pointer back to source (URL, file path, Notion page id). #[serde(default)] @@ -135,4 +205,70 @@ mod tests { let out = canonicalise("d1", "alice", &[], input).unwrap().unwrap(); assert!(out.metadata.source_ref.is_none()); } + + // ── Serde regression / fix tests (CORE-2K / CORE-2J / CORE-31) ─────────── + + /// Regression: existing callers send epoch-ms as a JSON integer — must still work. + #[test] + fn modified_at_epoch_ms_integer_still_works() { + let json = r#"{ + "provider": "notion", + "title": "My doc", + "body": "content", + "modified_at": 1700000000000 + }"#; + let input: DocumentInput = + serde_json::from_str(json).expect("epoch-ms integer should parse"); + assert_eq!( + input.modified_at.timestamp_millis(), + 1_700_000_000_000, + "epoch-ms round-trip" + ); + } + + /// Fix CORE-2K: callers sending an ISO-8601 string must be accepted. + #[test] + fn modified_at_iso8601_string_accepted() { + let json = r#"{ + "provider": "drive", + "title": "Meeting notes", + "body": "agenda here", + "modified_at": "2026-05-17T19:30:00Z" + }"#; + let input: DocumentInput = + serde_json::from_str(json).expect("ISO-8601 string should parse"); + assert_eq!(input.modified_at.timestamp(), 1_779_046_200); + } + + /// Fix CORE-2J: omitting modified_at must default to approximately now (within 5 s). + #[test] + fn modified_at_missing_defaults_to_now() { + let before = Utc::now(); + let json = r#"{ + "provider": "notion", + "title": "No timestamp doc", + "body": "body text" + }"#; + let input: DocumentInput = + serde_json::from_str(json).expect("missing modified_at should parse"); + let after = Utc::now(); + assert!( + input.modified_at >= before && input.modified_at <= after, + "default modified_at {ts} must fall between {before} and {after}", + ts = input.modified_at, + ); + } + + /// Fix CORE-31: omitting provider must default to "unknown". + #[test] + fn provider_missing_defaults_to_unknown() { + let json = r#"{ + "title": "No provider doc", + "body": "body text", + "modified_at": 1700000000000 + }"#; + let input: DocumentInput = + serde_json::from_str(json).expect("missing provider should parse"); + assert_eq!(input.provider, "unknown"); + } }