Compare commits

...
Author SHA1 Message Date
profit 1270e167fe Post-merge: update test pattern matches for struct-like UpsertOutcome
After merging main (with the UpsertOutcome struct-like enum shape
from PR #2), the 4 new upsert tests needed pattern-match updates:
  UpsertOutcome::Added(_) → UpsertOutcome::Added { .. }

9/9 upsert tests pass.
2026-04-22 04:11:13 -05:00
profit 4dca2a6705 Merge branch 'main' of https://git.agentview.dev/profit/lakehouse into fix/upsert-outcome-update-merge 2026-04-22 04:10:27 -05:00
profit b667fdeff1 Fix: UpsertOutcome newtype serde panic (silent since Phase 26)
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.
2026-04-22 09:10:07 +00:00
profitandClaude Opus 4.7 320009ddf4 Fix: UPDATE branch of upsert_entry dropped doc_refs + valid_until
lakehouse/auditor all checks passed (3 findings, all info)
The auditor's hybrid fixture (branch auditor/scaffold) surfaced this
on 2026-04-22. A re-seed of the same (operation, day) pair with new
endorsed_names merged the names but silently discarded the incoming
doc_refs and valid_until fields. schema_fingerprint was partially
handled (set-if-Some) but doc_refs and valid_until weren't touched.

Root cause: the UPDATE arm of upsert_entry at playbook_memory.rs:609
only covered:
  - endorsed_names (union-merge)
  - timestamp
  - embedding (if Some)
  - schema_fingerprint (if Some)

Fix:
  - valid_until — refresh if caller provides one
  - doc_refs — merge by tool (case-insensitive). Same-tool new entry
    supersedes older one; different-tool refs are appended. Empty
    incoming doc_refs preserves existing (don't wipe on partial seed).

4 new regression tests under upsert_tests:
  - update_merges_doc_refs_with_existing_ones
  - update_same_tool_supersedes_older_version
  - update_preserves_existing_doc_refs_when_new_entry_has_none
  - update_refreshes_valid_until_when_caller_provides_one

Test result: 9/9 upsert tests pass (4 new + 5 pre-existing).

Branch basis note: this branch is off main, so the UpsertOutcome enum
here still has the newtype variants Added(String) / Noop(String). PR
#2 (fix/upsert-outcome-serde) changes that enum to struct-like. When
PR #2 merges first this branch needs a trivial rebase; the UPDATE
arm logic is untouched by that change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-22 04:06:54 -05:00
profit f0a3ed6832 Fix: UpsertOutcome newtype variants panicked serde from Phase 26
lakehouse/auditor 1 blocking issue: cloud: claim not backed — "Verified live after gateway restart:"
playbook_memory.rs:257 — UpsertOutcome had two newtype variants
carrying a bare String:
  Added(String)
  Noop(String)
under #[serde(tag = "mode")]. serde cannot tag newtype variants of
primitive types, so every serialization threw:
  "cannot serialize tagged newtype variant UpsertOutcome::Added
   containing a string"
This caused gateway /vectors/playbook_memory/seed to panic the
tokio worker on EVERY call that reached Added or Noop, returning
an empty socket close to the client. The bug was silent from commit
640db8c (Phase 26, 2026-04-21) until 2026-04-22 when the auditor's
hybrid fixture (auditor/fixtures/hybrid_38_40_45.ts on the
auditor/scaffold branch) exercised the endpoint live and gateway
logs showed the panic.

Fix — convert both newtype variants to struct-like:
  Added { playbook_id: String }
  Noop { playbook_id: String }
Updated all 7 construction + pattern-match sites. Updated rustdoc
on the enum explaining why the shape is what it is.

JSON wire format is now uniform across all three variants:
  {"mode":"added","playbook_id":"pb-..."}
  {"mode":"updated","playbook_id":"pb-...","merged_names":[...]}
  {"mode":"noop","playbook_id":"pb-..."}

Verified live after gateway restart:
  curl /seed new payload               → mode=added, playbook 860231f5
  curl /seed new payload + doc_refs    → mode=added, playbook 11d348d9
  curl /seed identical re-submit       → mode=noop,  same id 860231f5,
                                         entries_after unchanged (Mem0
                                         contract intact)

Tests: 51/51 vectord lib tests green. Release build clean.

This is a follow-up bug fix landed in its own branch
(fix/upsert-outcome-serde) rather than commingled with other work.
The auditor's hybrid fixture on the auditor/scaffold branch will
now light up layer 3 (phase45_seed_with_doc_refs) as a pass once
this merges — previously it failed here with an empty socket close.
2026-04-22 03:48:05 -05:00
+163 -12
View File
@@ -252,11 +252,21 @@ pub struct BoostEntry {
/// Phase 26 — what happened during an upsert. The seed endpoint
/// returns this shape so the caller sees whether its write was a new
/// 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)]
#[serde(tag = "mode", rename_all = "lowercase")]
pub enum UpsertOutcome {
/// New playbook appended. Carries the new playbook_id.
Added(String),
Added { playbook_id: String },
/// Existing same-day entry updated. Playbook_id unchanged; names
/// merged (union, original order preserved, new names appended).
Updated {
@@ -265,7 +275,7 @@ pub enum UpsertOutcome {
},
/// Identical same-day entry already exists; nothing changed.
/// 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
@@ -597,7 +607,7 @@ impl PlaybookMemory {
drop(state);
self.persist().await?;
self.rebuild_geo_index().await;
Ok(UpsertOutcome::Added(pid))
Ok(UpsertOutcome::Added { playbook_id: pid })
}
Some(i) => {
let mut existing_names_sorted = state.entries[i].endorsed_names.clone();
@@ -605,7 +615,7 @@ impl PlaybookMemory {
if existing_names_sorted == new_names_sorted {
// NOOP — identical data, just report the existing id
let pid = state.entries[i].playbook_id.clone();
Ok(UpsertOutcome::Noop(pid))
Ok(UpsertOutcome::Noop { playbook_id: pid })
} else {
// UPDATE — merge names (union, stable order).
let existing = state.entries.get_mut(i).ok_or("index invalidated")?;
@@ -615,15 +625,45 @@ impl PlaybookMemory {
}
existing.endorsed_names = merged.clone();
existing.timestamp = new_entry.timestamp.clone();
// Keep original playbook_id + embedding + schema fingerprint.
// Refresh embedding only if the caller passed a non-None
// one (indicates the text shape changed).
// Keep original playbook_id. Refresh embedding only
// if the caller passed a non-None one (indicates
// the text shape changed).
if new_entry.embedding.is_some() {
existing.embedding = new_entry.embedding.clone();
}
// Lifecycle / drift-signal fields — refresh only
// when the caller provides a new value. Each was
// previously either ignored entirely (valid_until,
// doc_refs) or only partially handled. Caught by
// the auditor's hybrid fixture on 2026-04-22 when
// re-seeding with fresh doc_refs silently dropped
// them on the UPDATE path.
if new_entry.schema_fingerprint.is_some() {
existing.schema_fingerprint = new_entry.schema_fingerprint.clone();
}
if new_entry.valid_until.is_some() {
existing.valid_until = new_entry.valid_until.clone();
}
if !new_entry.doc_refs.is_empty() {
// Merge by tool (case-insensitive). A new ref
// for the same tool supersedes the old one —
// caller is asserting "this is what I used
// now." Refs for tools the existing entry
// didn't track are appended. This keeps the
// cumulative tool history without letting a
// single re-seed erase it.
let mut merged_refs = existing.doc_refs.clone();
for new_ref in &new_entry.doc_refs {
let hit = merged_refs.iter().position(|r| {
r.tool.eq_ignore_ascii_case(&new_ref.tool)
});
match hit {
Some(idx) => merged_refs[idx] = new_ref.clone(),
None => merged_refs.push(new_ref.clone()),
}
}
existing.doc_refs = merged_refs;
}
let pid = existing.playbook_id.clone();
drop(state);
self.persist().await?;
@@ -1600,7 +1640,7 @@ mod upsert_tests {
let pm = PlaybookMemory::new(Arc::new(InMemory::new()));
let e = mk("fill: Welder x2 in Nashville, TN", "2026-04-21", &["Alice Smith"]);
match pm.upsert_entry(e).await.unwrap() {
UpsertOutcome::Added(_) => {}
UpsertOutcome::Added { .. } => {}
other => panic!("expected Added, got {:?}", other),
}
assert_eq!(pm.entry_count().await, 1);
@@ -1613,7 +1653,7 @@ mod upsert_tests {
let e2 = mk("fill: Welder x2 in Nashville, TN", "2026-04-21", &["Alice Smith", "Bob Jones"]);
pm.upsert_entry(e1).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.
assert_eq!(pm.entry_count().await, 1);
}
@@ -1625,7 +1665,7 @@ mod upsert_tests {
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 pid = match o1 {
UpsertOutcome::Added(p) => p,
UpsertOutcome::Added { playbook_id: p } => p,
other => panic!("expected Added, got {:?}", other),
};
let o2 = pm.upsert_entry(e2).await.unwrap();
@@ -1646,7 +1686,7 @@ mod upsert_tests {
let e2 = mk("fill: Welder x2 in Nashville, TN", "2026-04-22", &["Alice Smith"]);
pm.upsert_entry(e1).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);
}
@@ -1659,9 +1699,120 @@ mod upsert_tests {
// 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 o = pm.upsert_entry(e2).await.unwrap();
assert!(matches!(o, UpsertOutcome::Added(_)));
assert!(matches!(o, UpsertOutcome::Added { .. }));
assert_eq!(pm.entry_count().await, 2);
}
// Regression test for the auditor-discovered bug: UPDATE branch
// silently dropped doc_refs, valid_until, and (partially)
// schema_fingerprint from the incoming entry.
//
// Before the fix: a re-seed with different endorsed_names AND new
// doc_refs would merge names but discard the new doc_refs. After:
// both sets are unioned (doc_refs merged by tool, same-tool
// supersedes).
fn docref(tool: &str, version: &str) -> DocRef {
DocRef {
tool: tool.into(),
version_seen: version.into(),
snippet_hash: Some(format!("hash-{tool}-{version}")),
source_url: None,
seen_at: "2026-04-22T00:00:00Z".into(),
}
}
fn mk_with_docs(op: &str, day: &str, names: &[&str], docs: Vec<DocRef>) -> PlaybookEntry {
let mut e = mk(op, day, names);
e.doc_refs = docs;
e
}
#[tokio::test]
async fn update_merges_doc_refs_with_existing_ones() {
let pm = PlaybookMemory::new(Arc::new(InMemory::new()));
let day = "2026-04-22";
let op = "fill: Welder x2 in Nashville, TN";
// First seed: Alice + Docker doc ref
let e1 = mk_with_docs(op, day, &["Alice"], vec![docref("docker", "24.0.7")]);
let o1 = pm.upsert_entry(e1).await.unwrap();
assert!(matches!(o1, UpsertOutcome::Added { .. }));
// Second seed on same (op, day) but new names AND new tool ref
let e2 = mk_with_docs(op, day, &["Bob"], vec![docref("terraform", "1.5.0")]);
let o2 = pm.upsert_entry(e2).await.unwrap();
assert!(matches!(o2, UpsertOutcome::Updated { .. }));
assert_eq!(pm.entry_count().await, 1); // still one entry, merged
let snap = pm.snapshot().await;
assert_eq!(snap.len(), 1);
let merged = &snap[0];
// Names unioned
assert!(merged.endorsed_names.contains(&"Alice".to_string()));
assert!(merged.endorsed_names.contains(&"Bob".to_string()));
// Both doc_refs preserved (different tools → union)
assert_eq!(merged.doc_refs.len(), 2);
let tools: Vec<_> = merged.doc_refs.iter().map(|r| r.tool.as_str()).collect();
assert!(tools.contains(&"docker"));
assert!(tools.contains(&"terraform"));
}
#[tokio::test]
async fn update_same_tool_supersedes_older_version() {
let pm = PlaybookMemory::new(Arc::new(InMemory::new()));
let day = "2026-04-22";
let op = "fill: Welder x2 in Nashville, TN";
let e1 = mk_with_docs(op, day, &["Alice"], vec![docref("docker", "24.0.7")]);
pm.upsert_entry(e1).await.unwrap();
// Same day, different name, SAME tool newer version
let e2 = mk_with_docs(op, day, &["Bob"], vec![docref("docker", "25.0.1")]);
let o2 = pm.upsert_entry(e2).await.unwrap();
assert!(matches!(o2, UpsertOutcome::Updated { .. }));
let snap = pm.snapshot().await;
assert_eq!(snap[0].doc_refs.len(), 1, "same tool should supersede, not duplicate");
assert_eq!(snap[0].doc_refs[0].version_seen, "25.0.1");
}
#[tokio::test]
async fn update_preserves_existing_doc_refs_when_new_entry_has_none() {
let pm = PlaybookMemory::new(Arc::new(InMemory::new()));
let day = "2026-04-22";
let op = "fill: Welder x2 in Nashville, TN";
let e1 = mk_with_docs(op, day, &["Alice"], vec![docref("docker", "24.0.7")]);
pm.upsert_entry(e1).await.unwrap();
// Second seed doesn't carry doc_refs — shouldn't wipe the existing
let e2 = mk(op, day, &["Bob"]); // no doc_refs
pm.upsert_entry(e2).await.unwrap();
let snap = pm.snapshot().await;
assert_eq!(snap[0].doc_refs.len(), 1, "empty new doc_refs shouldn't wipe existing");
assert_eq!(snap[0].doc_refs[0].tool, "docker");
}
#[tokio::test]
async fn update_refreshes_valid_until_when_caller_provides_one() {
let pm = PlaybookMemory::new(Arc::new(InMemory::new()));
let day = "2026-04-22";
let op = "fill: Welder x2 in Nashville, TN";
// First seed with no valid_until
let e1 = mk(op, day, &["Alice"]);
pm.upsert_entry(e1).await.unwrap();
// Second seed with a valid_until deadline
let mut e2 = mk(op, day, &["Bob"]);
e2.valid_until = Some("2026-05-01T00:00:00Z".into());
pm.upsert_entry(e2).await.unwrap();
let snap = pm.snapshot().await;
assert_eq!(snap[0].valid_until.as_deref(), Some("2026-05-01T00:00:00Z"));
}
}
#[cfg(test)]