fix(memory): accept ISO-8601 / missing modified_at + default provider (#2237)

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> {
+    Utc::now()
+}
+
+/// Deserialise a `DateTime<Utc>` 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<DateTime<Utc>, 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::<i64>() {
+                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<Utc>,
     /// 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");
+    }
 }