Skip to content

Commit df73c79

Browse files
slayofferclaude
andauthored
fix(cli): hindsight memory retain --timestamp + correct fact-type values (#1881)
Two unrelated CLI bugs surfaced during sandbox testing on 2026-05-31. 1) `hindsight memory retain --timestamp <ISO 8601>` never worked. `MemoryItem.timestamp` is generated from the OpenAPI schema `anyOf: [{type: string, format: date-time}, {type: string}]`. Progenitor emits that as a struct with two `#[serde(flatten)]` Option subtypes — which serde refuses to serialize for primitives: "can only flatten structs and maps (got a string)" So even constructing the value manually fails at serialize time, before the request hits the wire. The CLI's `serde_json::from_value::<…>(String)` round-trip also fails (struct deserializer expects an object). Fixed at the codegen boundary by adding a pre-codegen spec-massage step `collapse_string_anyof_unions` in hindsight-clients/rust/build.rs that collapses any `anyOf` whose members are all `{type: string}` into a single `{type: string}`. The `format: date-time` distinction is lossless on the wire — both serialize to the same string — so this is safe. Result: `MemoryItem.timestamp: Option<String>`, no broken type generated. The CLI no longer needs to round-trip through a wrapper type; the user string is passed through directly. 2) `hindsight memory clear --fact-type` rejected the valid value `observation` and accepted stale values `agent` / `opinion` that the server silently treats as no-ops. Help text on `bank graph`, `memory list`, `memory recall`, and `memory clear` referred to a non-existent fact type `opinion`. The canonical fact types per the API are `world | experience | observation` (see hindsight_api.api.http.MemoryItem and the `Literal[…]` arm on fact_types in recall/reflect requests). Fixed: `opinion` → `observation` everywhere in CLI help / clap defaults, and `agent`/`opinion` → `experience`/`observation` in the clear command's value_parser allow-list. Regression test: hindsight-cli/tests/integration_test.rs:: test_memory_item_timestamp_serializes_as_plain_string Verified: - cargo build → clean - cargo test --bin hindsight → 55/55 pass - cargo test --test integration_test test_memory_item_timestamp_… → pass - cargo clippy → no new warnings (171 pre-existing uninlined_format_args) - hindsight memory clear --help → [possible values: world, experience, observation] - hindsight memory recall --help → [default: world experience observation] - hindsight bank graph --help → (world, experience, observation) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 2a9589f commit df73c79

5 files changed

Lines changed: 90 additions & 20 deletions

File tree

hindsight-cli/src/api.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1266,8 +1266,8 @@ impl ApiClient {
12661266

12671267
// Re-export types from the generated client for use in commands
12681268
pub use types::{
1269-
BankProfileResponse, MemoryItem, MemoryItemTimestamp, RecallRequest, RecallResponse,
1270-
RecallResult, ReflectRequest, ReflectResponse, RetainRequest,
1269+
BankProfileResponse, MemoryItem, RecallRequest, RecallResponse, RecallResult, ReflectRequest,
1270+
ReflectResponse, RetainRequest,
12711271
};
12721272

12731273
#[cfg(test)]

hindsight-cli/src/commands/memory.rs

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,7 @@ use std::fs;
33
use std::path::PathBuf;
44
use walkdir::WalkDir;
55

6-
use crate::api::{
7-
ApiClient, MemoryItem, MemoryItemTimestamp, RecallRequest, ReflectRequest, RetainRequest,
8-
};
6+
use crate::api::{ApiClient, MemoryItem, RecallRequest, ReflectRequest, RetainRequest};
97
use crate::config;
108
use crate::output::{self, OutputFormat};
119
use crate::ui;
@@ -454,15 +452,6 @@ pub fn retain(
454452
None
455453
};
456454

457-
// MemoryItem.timestamp is a progenitor anyOf enum; round-trip through JSON to pick the matching variant.
458-
let timestamp = match timestamp {
459-
Some(s) => Some(
460-
serde_json::from_value::<MemoryItemTimestamp>(serde_json::Value::String(s.clone()))
461-
.with_context(|| format!("invalid --timestamp value: {:?}", s))?,
462-
),
463-
None => None,
464-
};
465-
466455
let item = MemoryItem {
467456
content: content.clone(),
468457
context,

hindsight-cli/src/main.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,7 @@ enum BankCommands {
287287
/// Bank ID
288288
bank_id: String,
289289

290-
/// Filter by fact type (world, experience, opinion)
290+
/// Filter by fact type (world, experience, observation)
291291
#[arg(short = 't', long)]
292292
fact_type: Option<String>,
293293

@@ -455,7 +455,7 @@ enum MemoryCommands {
455455
/// Bank ID
456456
bank_id: String,
457457

458-
/// Filter by fact type (world, experience, opinion)
458+
/// Filter by fact type (world, experience, observation)
459459
#[arg(short = 't', long)]
460460
fact_type: Option<String>,
461461

@@ -489,8 +489,8 @@ enum MemoryCommands {
489489
/// Search query
490490
query: String,
491491

492-
/// Fact types to search (world, experience, opinion)
493-
#[arg(short = 't', long, value_delimiter = ',', default_values = &["world", "experience", "opinion"])]
492+
/// Fact types to search (world, experience, observation)
493+
#[arg(short = 't', long, value_delimiter = ',', default_values = &["world", "experience", "observation"])]
494494
fact_type: Vec<String>,
495495

496496
/// Thinking budget (low, mid, high)
@@ -645,8 +645,8 @@ enum MemoryCommands {
645645
/// Bank ID
646646
bank_id: String,
647647

648-
/// Fact type to clear (world, agent, opinion). If not specified, clears all types.
649-
#[arg(short = 't', long, value_parser = ["world", "agent", "opinion"])]
648+
/// Fact type to clear (world, experience, observation). If not specified, clears all types.
649+
#[arg(short = 't', long, value_parser = ["world", "experience", "observation"])]
650650
fact_type: Option<String>,
651651

652652
/// Skip confirmation prompt

hindsight-cli/tests/integration_test.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,35 @@ fn test_ui_command_with_config() {
9191
std::fs::remove_dir_all(&temp_dir).ok();
9292
}
9393

94+
#[test]
95+
fn test_memory_item_timestamp_serializes_as_plain_string() {
96+
// Regression: progenitor used to emit `MemoryItemTimestamp` as a struct
97+
// with two `#[serde(flatten)]` Option subtypes from the `anyOf:[datetime,
98+
// string]` schema. Serializing that errored with
99+
// "can only flatten structs and maps (got a string)"
100+
// — so `hindsight memory retain --timestamp` never worked. The build.rs
101+
// spec-massage step now collapses string-only anyOf unions into a plain
102+
// `{type: string}`, so the field is just `Option<String>`. Assert that.
103+
let item = hindsight_client::types::MemoryItem {
104+
content: "Bob went hiking yesterday".to_string(),
105+
context: None,
106+
metadata: None,
107+
timestamp: Some("2026-05-31T10:00:00Z".to_string()),
108+
document_id: Some("doc-1".to_string()),
109+
entities: None,
110+
tags: None,
111+
observation_scopes: None,
112+
strategy: None,
113+
update_mode: None,
114+
};
115+
let json = serde_json::to_string(&item).expect("MemoryItem must serialize");
116+
assert!(
117+
json.contains(r#""timestamp":"2026-05-31T10:00:00Z""#),
118+
"expected timestamp at top-level as a plain string, got: {}",
119+
json
120+
);
121+
}
122+
94123
#[test]
95124
fn test_memory_retain_exposes_timestamp_flag() {
96125
// Regression: `hindsight memory retain` historically had no way to set the

hindsight-clients/rust/build.rs

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,52 @@ fn filter_multipart_endpoints(spec: &mut serde_json::Value) {
4343
}
4444
}
4545

46+
/// Collapse `anyOf` whose members are all plain string types into a single
47+
/// `{"type": "string"}`. Without this, progenitor emits a struct like
48+
/// `MemoryItemTimestamp { #[serde(flatten)] subtype_0: Option<DateTime>, ... }`
49+
/// where flatten on a primitive is a runtime error
50+
/// (`"can only flatten structs and maps (got a string)"`). For client purposes
51+
/// the `format: date-time` distinction is lossless — the wire value is just a
52+
/// string either way — so collapsing to a plain string is safe and lets the
53+
/// generated client actually serialize.
54+
fn collapse_string_anyof_unions(value: &mut serde_json::Value) {
55+
if let serde_json::Value::Object(obj) = value {
56+
let all_strings = obj
57+
.get("anyOf")
58+
.and_then(|v| v.as_array())
59+
.map(|arr| {
60+
!arr.is_empty()
61+
&& arr.iter().all(|v| {
62+
v.as_object()
63+
.and_then(|m| m.get("type"))
64+
.and_then(|t| t.as_str())
65+
.map(|s| s == "string")
66+
.unwrap_or(false)
67+
})
68+
})
69+
.unwrap_or(false);
70+
71+
if all_strings {
72+
obj.remove("anyOf");
73+
obj.insert("type".to_string(), serde_json::json!("string"));
74+
}
75+
}
76+
77+
match value {
78+
serde_json::Value::Object(obj) => {
79+
for (_k, v) in obj.iter_mut() {
80+
collapse_string_anyof_unions(v);
81+
}
82+
}
83+
serde_json::Value::Array(arr) => {
84+
for v in arr.iter_mut() {
85+
collapse_string_anyof_unions(v);
86+
}
87+
}
88+
_ => {}
89+
}
90+
}
91+
4692
fn convert_anyof_to_nullable(value: &mut serde_json::Value) {
4793
match value {
4894
serde_json::Value::Object(obj) => {
@@ -132,6 +178,12 @@ fn main() {
132178
}
133179
}
134180

181+
// Collapse anyOf-of-only-strings into a plain string. Must run AFTER the
182+
// 3.1→3.0 nullable pass so we see `anyOf: [datetime, string]` without the
183+
// null arm. Keeps progenitor from emitting unserializable flatten-of-
184+
// primitive structs (see collapse_string_anyof_unions docs).
185+
collapse_string_anyof_unions(&mut spec_json);
186+
135187
// Filter out multipart/form-data endpoints (progenitor doesn't support them)
136188
filter_multipart_endpoints(&mut spec_json);
137189

0 commit comments

Comments
 (0)