profit/lakehouse
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix: UpsertOutcome newtype serde panic (silent since Phase 26)

Browse Source 
Auditor found this via hybrid fixture 2026-04-22. Blocks the serde-tag-newtype shape by converting to struct-like variants. See PR #2 body for full context.

Manual merge: auditor commit status was failure due to 1 false-positive inference finding on a commit-message reference; underlying fix is verified (curl against live gateway confirmed all 3 upsert paths return valid JSON). Proceeding per human review.
This commit is contained in:
profit 2026-04-22 09:10:07 +00:00
commit b667fdeff1
1 changed files with 19 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
crates/vectord/src/playbook_memory.rs
View File

@ -252,11 +252,21 @@ pub struct BoostEntry {
/// Phase 26 — what happened during an upsert. The seed endpoint /// Phase 26 — what happened during an upsert. The seed endpoint
/// returns this shape so the caller sees whether its write was a new /// returns this shape so the caller sees whether its write was a new
/// entry, a merge, or a dedup'd no-op. /// entry, a merge, or a dedup'd no-op.
///
/// SHAPE NOTE: `#[serde(tag = "mode")]` requires struct-like variants —
/// bare `Added(String)` and `Noop(String)` newtype variants would
/// panic serialization at runtime. That bug silently 500-ed every
/// /seed call from Phase 26 (commit 640db8c) until 2026-04-22 when the
/// auditor's hybrid fixture surfaced it. All three variants are now
/// struct-like, producing uniform JSON:
/// {"mode":"added","playbook_id":"pb-..."}
/// {"mode":"updated","playbook_id":"pb-...","merged_names":[...]}
/// {"mode":"noop","playbook_id":"pb-..."}
#[derive(Debug, Clone, Serialize)] #[derive(Debug, Clone, Serialize)]
#[serde(tag = "mode", rename_all = "lowercase")] #[serde(tag = "mode", rename_all = "lowercase")]
pub enum UpsertOutcome { pub enum UpsertOutcome {
/// New playbook appended. Carries the new playbook_id. /// New playbook appended. Carries the new playbook_id.
Added(String), Added { playbook_id: String },
/// Existing same-day entry updated. Playbook_id unchanged; names /// Existing same-day entry updated. Playbook_id unchanged; names
/// merged (union, original order preserved, new names appended). /// merged (union, original order preserved, new names appended).
Updated { Updated {
@ -265,7 +275,7 @@ pub enum UpsertOutcome {
}, },
/// Identical same-day entry already exists; nothing changed. /// Identical same-day entry already exists; nothing changed.
/// Returns the stable playbook_id so caller still has a reference. /// Returns the stable playbook_id so caller still has a reference.
Noop(String), Noop { playbook_id: String },
} }
/// Phase 27 — shape returned from `revise_entry`. Reports both ends of /// Phase 27 — shape returned from `revise_entry`. Reports both ends of
@ -597,7 +607,7 @@ impl PlaybookMemory {
drop(state); drop(state);
self.persist().await?; self.persist().await?;
self.rebuild_geo_index().await; self.rebuild_geo_index().await;
Ok(UpsertOutcome::Added(pid)) Ok(UpsertOutcome::Added { playbook_id: pid })
} }
Some(i) => { Some(i) => {
let mut existing_names_sorted = state.entries[i].endorsed_names.clone(); let mut existing_names_sorted = state.entries[i].endorsed_names.clone();
@ -605,7 +615,7 @@ impl PlaybookMemory {
if existing_names_sorted == new_names_sorted { if existing_names_sorted == new_names_sorted {
// NOOP — identical data, just report the existing id // NOOP — identical data, just report the existing id
let pid = state.entries[i].playbook_id.clone(); let pid = state.entries[i].playbook_id.clone();
Ok(UpsertOutcome::Noop(pid)) Ok(UpsertOutcome::Noop { playbook_id: pid })
} else { } else {
// UPDATE — merge names (union, stable order). // UPDATE — merge names (union, stable order).
let existing = state.entries.get_mut(i).ok_or("index invalidated")?; let existing = state.entries.get_mut(i).ok_or("index invalidated")?;
@ -1600,7 +1610,7 @@ mod upsert_tests {
let pm = PlaybookMemory::new(Arc::new(InMemory::new())); let pm = PlaybookMemory::new(Arc::new(InMemory::new()));
let e = mk("fill: Welder x2 in Nashville, TN", "2026-04-21", &["Alice Smith"]); let e = mk("fill: Welder x2 in Nashville, TN", "2026-04-21", &["Alice Smith"]);
match pm.upsert_entry(e).await.unwrap() { match pm.upsert_entry(e).await.unwrap() {
UpsertOutcome::Added(_) => {} UpsertOutcome::Added { .. } => {}
other => panic!("expected Added, got {:?}", other), other => panic!("expected Added, got {:?}", other),
} }
assert_eq!(pm.entry_count().await, 1); assert_eq!(pm.entry_count().await, 1);
@ -1613,7 +1623,7 @@ mod upsert_tests {
let e2 = mk("fill: Welder x2 in Nashville, TN", "2026-04-21", &["Alice Smith", "Bob Jones"]); let e2 = mk("fill: Welder x2 in Nashville, TN", "2026-04-21", &["Alice Smith", "Bob Jones"]);
pm.upsert_entry(e1).await.unwrap(); pm.upsert_entry(e1).await.unwrap();
let outcome = pm.upsert_entry(e2).await.unwrap(); let outcome = pm.upsert_entry(e2).await.unwrap();
assert!(matches!(outcome, UpsertOutcome::Noop(_))); assert!(matches!(outcome, UpsertOutcome::Noop { .. }));
// Still exactly one entry, no duplicate from the re-seed. // Still exactly one entry, no duplicate from the re-seed.
assert_eq!(pm.entry_count().await, 1); assert_eq!(pm.entry_count().await, 1);
} }
@ -1625,7 +1635,7 @@ mod upsert_tests {
let e2 = mk("fill: Welder x2 in Nashville, TN", "2026-04-21", &["Alice Smith", "Bob Jones"]); let e2 = mk("fill: Welder x2 in Nashville, TN", "2026-04-21", &["Alice Smith", "Bob Jones"]);
let o1 = pm.upsert_entry(e1).await.unwrap(); let o1 = pm.upsert_entry(e1).await.unwrap();
let pid = match o1 { let pid = match o1 {
UpsertOutcome::Added(p) => p, UpsertOutcome::Added { playbook_id: p } => p,
other => panic!("expected Added, got {:?}", other), other => panic!("expected Added, got {:?}", other),
}; };
let o2 = pm.upsert_entry(e2).await.unwrap(); let o2 = pm.upsert_entry(e2).await.unwrap();
@ -1646,7 +1656,7 @@ mod upsert_tests {
let e2 = mk("fill: Welder x2 in Nashville, TN", "2026-04-22", &["Alice Smith"]); let e2 = mk("fill: Welder x2 in Nashville, TN", "2026-04-22", &["Alice Smith"]);
pm.upsert_entry(e1).await.unwrap(); pm.upsert_entry(e1).await.unwrap();
let o2 = pm.upsert_entry(e2).await.unwrap(); let o2 = pm.upsert_entry(e2).await.unwrap();
assert!(matches!(o2, UpsertOutcome::Added(_)), "different day → fresh ADD"); assert!(matches!(o2, UpsertOutcome::Added { .. }), "different day → fresh ADD");
assert_eq!(pm.entry_count().await, 2); assert_eq!(pm.entry_count().await, 2);
} }
@ -1659,7 +1669,7 @@ mod upsert_tests {
// A new seed on same day should ADD, not merge into the retired one. // A new seed on same day should ADD, not merge into the retired one.
let e2 = mk("fill: Welder x2 in Nashville, TN", "2026-04-21", &["Carol Davis"]); let e2 = mk("fill: Welder x2 in Nashville, TN", "2026-04-21", &["Carol Davis"]);
let o = pm.upsert_entry(e2).await.unwrap(); let o = pm.upsert_entry(e2).await.unwrap();
assert!(matches!(o, UpsertOutcome::Added(_))); assert!(matches!(o, UpsertOutcome::Added { .. }));
assert_eq!(pm.entry_count().await, 2); assert_eq!(pm.entry_count().await, 2);
} }
} }