diff --git a/packages/cultnet-rs/src/snapshot_query.rs b/packages/cultnet-rs/src/snapshot_query.rs index f8d7c233..a39532b1 100644 --- a/packages/cultnet-rs/src/snapshot_query.rs +++ b/packages/cultnet-rs/src/snapshot_query.rs @@ -76,11 +76,17 @@ pub fn serve_read_only_raw_snapshot( return Err(anyhow!("expected cultnet.snapshot_request.v0")); }; require_non_empty(message_id, "message_id")?; - // Self's ruling, 2026-09-22 (docs/cultnet-selection-cut.md, commit 2 fix batch): the door - // refuses an empty or blank entry in a *present* Selection list, but v0's own lowering keeps - // v0's old cleaning - an empty v0 list, or one made only of blanks, lowers to `null` (no - // filter) rather than reaching the door as something to refuse. That is a v0 compatibility - // rule the lowering owns, not a meaning the evaluator or the door carries for v1 lists. + // R-Q3/R-R (docs/cultnet-selection-cut.md, Self's rulings for fix batch 3, 2026-09-22 - + // corrects this comment's own earlier ruling from the commit 2 fix batch, which lowered an + // empty v0 list to "no filter"): `None` (the field omitted) still means "no filter", but a + // *present* v0 list - even one that empties out after trimming/deduping blanks - must lower to + // `Some(possibly-empty)`, never collapse to `None`. `matches_schema_keys_fields` already + // treats a present-but-empty selection list as "match nothing" (R-E), which is v0's own + // pre-cut behaviour (`recordKeys: []` answers empty, `CultNetDocumentRegistry. + // CreateRawSnapshotResponse` tests `filter?.RecordKeys != null`, not a length); collapsing to + // `None` here used to turn that into "match everything" instead. Unlike the C# reference, this + // function never calls `validate`, so there is no v1 door for a present-but-empty list to hit + // (R-R's fixup on the C# side does not apply here). // R-M: `lower_v0_list` already dedups (Soul: it dedups before `reject_duplicates` ever ran, // making that function's own duplicate check dead code - deleted below along with its calls). let schema_ids = lower_v0_list(schema_ids); @@ -126,6 +132,20 @@ pub fn serve_read_only_raw_snapshot( selected.push(document); } + // R-R: pre-cut v0 iterated the requested record keys in the caller's own order (a + // `HashSet` built from the array, which enumerates in insertion order); the shared + // evaluator carries no such tiebreak of its own, so this v0 lowering restores the requested + // order here rather than leaking plain snapshot-source order when the caller asked for + // particular keys. A stable sort keeps the relative order of documents that share a requested + // key (e.g. across schemas) as `raw_snapshot()` produced them. + if let Some(keys) = &record_keys { + selected.sort_by_key(|document| { + keys.iter() + .position(|key| key == &document.record_key) + .unwrap_or(usize::MAX) + }); + } + Ok(CultNetMessage::SnapshotResponseRaw { message_id: message_id.clone(), documents: selected, @@ -360,7 +380,10 @@ fn require_non_empty(value: &str, field: &str) -> Result<()> { /// rule the lowering owns, not a meaning the door or the evaluator carries for v1's own lists /// (those refuse `[]` and any blank entry outright, in `selection::validate`). fn lower_v0_list(list: &Option>) -> Option> { - let Some(values) = list else { return None }; + // R-Q3: `None` (the field omitted) still means "no filter"; a present list is always lowered + // to `Some`, even when trimming/deduping empties it out (R-R) - see this function's caller for + // why collapsing that case to `None` was wrong. + let values = list.as_ref()?; let mut seen = std::collections::HashSet::new(); let filtered: Vec = values .iter() @@ -368,5 +391,5 @@ fn lower_v0_list(list: &Option>) -> Option> { .filter(|value| seen.insert(value.as_str())) .cloned() .collect(); - if filtered.is_empty() { None } else { Some(filtered) } + Some(filtered) } diff --git a/packages/cultnet-rs/tests/cultnet.rs b/packages/cultnet-rs/tests/cultnet.rs index d19536ba..426e2ed9 100644 --- a/packages/cultnet-rs/tests/cultnet.rs +++ b/packages/cultnet-rs/tests/cultnet.rs @@ -2038,15 +2038,18 @@ fn raw_snapshot_replication_preserves_messagepack_payload_bytes() -> Result<()> Ok(()) } -// Self's ruling, 2026-09-22 (docs/cultnet-selection-cut.md, commit 2 fix batch): v0's own -// cleaning - an empty schema_ids/record_keys list, or one made only of blanks, lowers to `null` -// (no filter) rather than reaching the door as something to refuse or the evaluator as "match -// nothing". A v1 Selection with the same shape is refused outright (validation_refuses_a_blank_entry_in_schemas_or_keys, -// validation_refuses_undeclared_index_role_and_empty_lists in tests/selection.rs) - this is v0's -// own compatibility rule, owned by the lowering in snapshot_query.rs, not a meaning v1 carries. +// R-Q3/R-R (docs/cultnet-selection-cut.md, Self's rulings for fix batch 3, 2026-09-22): corrects +// this test's own earlier ruling from the commit 2 fix batch, which lowered an *explicit* empty +// or all-blank v0 list to "no filter". Only an *omitted* list (`None`) means "no filter" now - a +// present list, even one that empties out after trimming/deduping blanks, answers empty +// (`recordKeys: []` matches v0's own pre-cut server, which tested `filter?.RecordKeys != null`, +// not a length). A v1 Selection with the same shape is refused outright +// (validation_refuses_a_blank_entry_in_schemas_or_keys, +// validation_refuses_undeclared_index_role_and_empty_lists in tests/selection.rs) - that is a v1 +// door rule this v0 lowering never reaches (`serve_read_only_raw_snapshot` never calls `validate`). #[test] -fn serve_read_only_raw_snapshot_lowers_an_empty_or_blank_v0_schema_list_to_no_filter() -> Result<()> -{ +fn serve_read_only_raw_snapshot_omitted_v0_schema_list_is_no_filter_present_empty_or_blank_answers_empty() +-> Result<()> { let mut registry = CultNetDocumentRegistry::new(); registry.register(CultNetDocumentBinding::for_entry_with_schema_id::< GhostlightAgentStateFixture, @@ -2081,7 +2084,21 @@ fn serve_read_only_raw_snapshot_lowers_an_empty_or_blank_v0_schema_list_to_no_fi }, ]; - for schema_ids in [None, Some(Vec::new()), Some(vec![" ".to_string()])] { + // The field omitted entirely: no filter, both rows. + let omitted_request = CultNetMessage::SnapshotRequest { + message_id: "req-1".to_string(), + schema_ids: None, + record_keys: None, + }; + let omitted_response = + serve_read_only_raw_snapshot(®istry, &policy, &source, &omitted_request)?; + let CultNetMessage::SnapshotResponseRaw { documents, .. } = omitted_response else { + panic!("expected a raw snapshot response"); + }; + assert_eq!(documents.len(), 2, "schema_ids omitted must lower to no filter, not zero rows"); + + // A present list - empty, or made only of blanks - answers empty (R-R), not "no filter". + for schema_ids in [Some(Vec::new()), Some(vec![" ".to_string()])] { let request = CultNetMessage::SnapshotRequest { message_id: "req-1".to_string(), schema_ids: schema_ids.clone(), @@ -2093,8 +2110,9 @@ fn serve_read_only_raw_snapshot_lowers_an_empty_or_blank_v0_schema_list_to_no_fi }; assert_eq!( documents.len(), - 2, - "schema_ids = {schema_ids:?} must lower to no filter, not zero rows" + 0, + "schema_ids = {schema_ids:?} is present and empty after trimming, so it must answer \ + empty, not fall back to every row" ); } @@ -2122,6 +2140,72 @@ fn serve_read_only_raw_snapshot_lowers_an_empty_or_blank_v0_schema_list_to_no_fi Ok(()) } +// R-R: v0's pre-cut server preserved the caller's requested key order (a `HashSet` built +// from the array, which enumerates in insertion order), not plain snapshot-source order. +#[test] +fn serve_read_only_raw_snapshot_keeps_the_requested_record_key_order() -> Result<()> { + let mut registry = CultNetDocumentRegistry::new(); + registry.register(CultNetDocumentBinding::for_entry_with_schema_id::< + GhostlightAgentStateFixture, + >("ghostlight.agent-state".to_string(), None)); + + let mut policy = CultNetReadOnlySnapshotPolicy::new(); + policy.allow("ghostlight.agent-state", "row-1")?; + policy.allow("ghostlight.agent-state", "row-2")?; + policy.allow("ghostlight.agent-state", "row-3")?; + + // Source order is row-1, row-2, row-3; the request asks for them in the opposite order. + let source = vec![ + CultNetRawDocumentRecord { + schema_id: "ghostlight.agent-state".to_string(), + record_key: "row-1".to_string(), + stored_at: "now".to_string(), + payload_encoding: CultNetRawPayloadEncoding::Messagepack, + payload: vec![1], + source_runtime_id: None, + source_agent_id: None, + source_role: None, + tags: None, + }, + CultNetRawDocumentRecord { + schema_id: "ghostlight.agent-state".to_string(), + record_key: "row-2".to_string(), + stored_at: "now".to_string(), + payload_encoding: CultNetRawPayloadEncoding::Messagepack, + payload: vec![2], + source_runtime_id: None, + source_agent_id: None, + source_role: None, + tags: None, + }, + CultNetRawDocumentRecord { + schema_id: "ghostlight.agent-state".to_string(), + record_key: "row-3".to_string(), + stored_at: "now".to_string(), + payload_encoding: CultNetRawPayloadEncoding::Messagepack, + payload: vec![3], + source_runtime_id: None, + source_agent_id: None, + source_role: None, + tags: None, + }, + ]; + + let request = CultNetMessage::SnapshotRequest { + message_id: "req-1".to_string(), + schema_ids: None, + record_keys: Some(vec!["row-3".to_string(), "row-1".to_string(), "row-2".to_string()]), + }; + let response = serve_read_only_raw_snapshot(®istry, &policy, &source, &request)?; + let CultNetMessage::SnapshotResponseRaw { documents, .. } = response else { + panic!("expected a raw snapshot response"); + }; + let keys: Vec<&str> = documents.iter().map(|d| d.record_key.as_str()).collect(); + assert_eq!(keys, vec!["row-3", "row-1", "row-2"], "the requested key order must survive, not the source's"); + + Ok(()) +} + #[test] fn raw_snapshot_replication_hydrates_same_schema_rust_aliases() -> Result<()> { let temp = tempfile::tempdir()?;