Skip to content

Commit 0d21705

Browse files
committed
T2.7 fix 1: the memory provider on the default layer too, --layer default
sdm-contract-run --database memory --layer default runs the same non-SQL provider through sdm_host::default_root() in place of the second data layer's root, keyed <case>@memory, so the rewritten swapped-layer test compares two runs that differ in their data layer alone. host::run_memory takes the root. The pending list is empty. Signed-off-by: Adi Wadaskar <awadaskar@nvidia.com>
1 parent 5a1f631 commit 0d21705

5 files changed

Lines changed: 52 additions & 29 deletions

File tree

‎contract/tests/t27-pending.json‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
{
22
"about": "T2.7's acceptance tests (the non-SQL test provider and the swapped data layer) that failed when the test author wrote them, before the implementation (sdm-testkit's tests/t27_non_sql_provider.rs as rust:<test>). Each listed test must still fail (strict); the implementer takes a name out of 'pending' once it passes, and changes nothing else: 'pending' stays within 'authored', which is the test author's.",
3-
"pending": [
4-
"rust:the_same_prediction_runs_on_the_swapped_layer"
5-
],
3+
"pending": [],
64
"authored": [
75
"rust:the_kit_passes_on_the_non_sql_provider",
86
"rust:a_seeded_fault_fails_the_kit_on_the_non_sql_provider",

‎docs/design/rust-core/phase-2/tracker-T2.7.md‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ Both are dev-only, in `sdm-testkit`; nothing shipped reaches them (`l0_deps.py`:
3030

3131
- **The provider passes the kit, including two schema tables over one source**: `t27_non_sql_provider.rs`'s `the_kit_passes_on_the_non_sql_provider` passes (every kit case, `key-counts.one-source-twice` among them), and `a_seeded_fault_fails_the_kit_on_the_non_sql_provider` passes (truncation and a silenced capability each fail the cases `expected.json` names).
3232
Beyond the kit, `memory_provider.rs`'s `two_schema_tables_over_one_source_predict_as_two_sources`: `shape.same-table-twice.buyers` with both schema tables reading `customers` (and `buyers` dropped) predicts exactly as over the two equal tables.
33-
- **The same prediction runs on the swapped layer**: not met as the acceptance test words it; `the_same_prediction_runs_on_the_swapped_layer` stays pending (below).
34-
What holds (`memory_provider.rs`'s `a_prediction_is_duckdbs_but_for_its_drawn_examples`, on `base.binary` and `shape.same-table-twice.buyers`): every fetch planned as the typed request, no statement run; the description equal to DuckDB's; the prediction's result equal to DuckDB's; the materialized request and the model call body equal to DuckDB's but for `context`, the in-context examples.
33+
- **The same prediction runs on the swapped layer**: `the_same_prediction_runs_on_the_swapped_layer` passes, as the test author rewrote it (round 1, below): the provider on the swapped layer gives, whole, what it gives on the default layer (`--database memory --layer default`), and DuckDB's but for the drawn example rows.
34+
What also holds (`memory_provider.rs`'s `a_prediction_is_duckdbs_but_for_its_drawn_examples`, on `base.binary` and `shape.same-table-twice.buyers`): every fetch planned as the typed request, no statement run; the description equal to DuckDB's; the prediction's result equal to DuckDB's; the materialized request and the model call body equal to DuckDB's but for `context`, the in-context examples.
3535
- **A guard shows no domain crate changed for it**: `no_domain_crate_changed_for_t2_7` passes on this branch (no `T2.7` commit touches a crate `l0_deps.py` classes as domain); `git diff --stat d339e88c..HEAD` touches `sdm-testkit`, `sdm-host`, `rust/Cargo.lock`, the kit file and docs only.
3636

3737
### Probes
@@ -46,10 +46,16 @@ Both are dev-only, in `sdm-testkit`; nothing shipped reaches them (`l0_deps.py`:
4646
- `pytest contract/tests -n 4 --dist loadfile` on that build: 4,543 pass; the 132 failures are this machine's environment, not the change: 118 refuse to run outside the recorded `ref-pandas3` cell (this venv's pyarrow is 26.0.0, the cell's 25.0.1), and the gate-mode tests' check 1 is reported as observed in no cell for the same reason.
4747
`tests/test_release_contract.py`'s two snapshot tests fail in the shared venv, whose packages are editable installs of another checkout; no Python file changes here.
4848

49+
### Round 1 (the swapped prediction's test)
50+
51+
The test author rewrote `the_same_prediction_runs_on_the_swapped_layer` (`T2.7 tests:`, cherry-picked unedited): the swapped run against the same provider on the default layer, whole, and against DuckDB but for the example rows.
52+
For it, `sdm-contract-run --database memory --layer default` runs the same provider through `sdm_host::default_root()` (`host::run_memory` takes the root), keyed `<case>@memory` as the swapped run is; the pending list is empty.
53+
The T2.7 tests, the Rust kit tests (both hosts' `t25_provider_kit`, DuckDB and SQLite) and the gate's rust-host leg pass at the new head.
54+
4955
### Deviations and questions
5056

51-
- **The swapped prediction's acceptance test cannot pass as written** (`the_same_prediction_runs_on_the_swapped_layer`, pending): it asks the swapped run's `core.materialized` and `core.predicted` to equal DuckDB's whole, including the in-context examples, which are a seeded sample each provider draws its own way (DuckDB's reservoir, SQLite's RS-2 stream, Postgres's `setseed`, the fallback's RS-1).
57+
- **The swapped prediction's acceptance test, as first written, could not pass** (resolved in round 1): it asked the swapped run's `core.materialized` and `core.predicted` to equal DuckDB's whole, including the in-context examples, which are a seeded sample each provider draws its own way (DuckDB's reservoir, SQLite's RS-2 stream, Postgres's `setseed`, the fallback's RS-1).
5258
Matching DuckDB's would mean reimplementing DuckDB's reservoir sampler in the test provider.
53-
Proposed to the test author: compare everything but `context` (as `memory_provider.rs` does), or compare the swapped run to the same provider on the default root, which is equal whole.
59+
The test author took both proposals: everything but the example rows against DuckDB, and the whole run against the same provider on the default root.
5460
- **Found in the core, not changed here (a domain crate)**: `sdm-core/src/materialize.rs`'s `sampling_capabilities` refuses a provider that does not declare `time_bounds`, `sample_entities`, `neighbors` or `lookup` before it reads anything, even when the provider declares the in-memory fallback's limits, so a provider relying on the fallback cannot `materialize` (design choice 1 says such a request takes the fallback).
5561
This provider declares `time_bounds` and `sample_entities` itself instead; the core's check is for the orchestrator to route to its own package.

‎rust/crates/sdm-testkit/src/bin/sdm-contract-run.rs‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
//! sdm-contract-run --providers
1010
//! sdm-contract-run --replay RECORDING --out FILE CASE_OR_DIRECTORY...
1111
//! sdm-contract-run --database sqlite --out FILE CASE_OR_DIRECTORY...
12-
//! sdm-contract-run --database memory --out FILE CASE_OR_DIRECTORY...
12+
//! sdm-contract-run --database memory [--layer default] --out FILE CASE_OR_DIRECTORY...
1313
//! sdm-contract-run --kit DATABASE [--fault truncate|silent_capability] --out REPORT
1414
//! ```
1515
//!
@@ -29,7 +29,10 @@
2929
//! DuckDB base copied into the minimal non-SQL test provider
3030
//! ([`sdm_testkit::memory`]), through the second data layer swapped in at
3131
//! the composition root ([`sdm_testkit::layer`]), each observation keyed
32-
//! `<case>@memory`; a case that reads no data is not run there.
32+
//! `<case>@memory`; a case that reads no data is not run there. With
33+
//! `--layer default` the same provider runs through
34+
//! `sdm_host::default_root()`'s layer (`sdm-data`'s) instead, keyed the
35+
//! same, so the two runs differ in their data layer alone.
3336
//!
3437
//! `--replay` runs each case that runs on Postgres there, its fetches
3538
//! answered from a recording of a Postgres server's answers
@@ -86,6 +89,20 @@ fn run(arguments: &[String]) -> Result<(PathBuf, usize)> {
8689
_ => (None, arguments),
8790
};
8891
let sqlite = database == Some("sqlite");
92+
// The data layer a `--database memory` run composes: the second one
93+
// (T2.7), or, with `--layer default`, the default root's.
94+
let (root, arguments) = match arguments {
95+
[flag, layer, rest @ ..] if flag == "--layer" && layer == "default" => {
96+
if database != Some("memory") {
97+
bail!("--layer default: only with --database memory");
98+
}
99+
(sdm_host::default_root(), rest)
100+
}
101+
[flag, layer, ..] if flag == "--layer" => {
102+
bail!("--layer {layer}: the one layer to name is `default`")
103+
}
104+
_ => (sdm_testkit::layer::root(), arguments),
105+
};
89106
let (recording, arguments) = match arguments {
90107
[flag, path, rest @ ..] if flag == "--replay" => (
91108
Some(replay::Recording::load(std::path::Path::new(path))?),
@@ -99,7 +116,7 @@ fn run(arguments: &[String]) -> Result<(PathBuf, usize)> {
99116
cases.iter().map(PathBuf::from).collect::<Vec<_>>(),
100117
),
101118
_ => bail!(
102-
"usage: sdm-contract-run [--pre-given-specs] [--database sqlite|memory | --replay RECORDING] --out FILE CASE_OR_DIRECTORY..."
119+
"usage: sdm-contract-run [--pre-given-specs] [--database sqlite | --database memory [--layer default] | --replay RECORDING] --out FILE CASE_OR_DIRECTORY..."
103120
),
104121
};
105122
let mut partial = out.clone().into_os_string();
@@ -147,7 +164,7 @@ fn run(arguments: &[String]) -> Result<(PathBuf, usize)> {
147164
}
148165
let mut keyed = each.clone();
149166
keyed.case.id = format!("{}@memory", each.case.id);
150-
(host::run_memory(&each.case, &bases)?, keyed)
167+
(host::run_memory(&each.case, &bases, &root)?, keyed)
151168
}
152169
None => (host::run(&each.case, &bases)?, each.clone()),
153170
Some(recording) => {

‎rust/crates/sdm-testkit/src/host.rs‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,8 @@ enum Source<'a> {
529529
Duck(&'a duckdb::Connection),
530530
Sqlite(&'a Sqlite),
531531
Replay(&'a Replay<'a>),
532-
Memory(&'a Memory),
532+
/// The non-SQL provider, through the composition root given.
533+
Memory(&'a Memory, &'a CompositionRoot),
533534
}
534535

535536
impl Source<'_> {
@@ -546,18 +547,18 @@ impl Source<'_> {
546547
dialect: Dialect::new(Database::Postgres.name()),
547548
},
548549
// The non-SQL provider, a host provider, as it declares itself.
549-
Source::Memory(_) => ProviderChoice::Host {
550+
Source::Memory(..) => ProviderChoice::Host {
550551
capabilities: Memory::capabilities(),
551552
},
552553
}
553554
}
554555

555-
/// The composition root a run over this source starts with: the
556-
/// second data layer, for host providers alone, for the non-SQL
557-
/// provider ([`crate::layer`], T2.7); `sdm-data`'s for every other.
556+
/// The composition root a run over this source starts with: the one
557+
/// the non-SQL provider's run was given (T2.7); `sdm-data`'s for every
558+
/// other.
558559
fn root(self) -> CompositionRoot {
559560
match self {
560-
Source::Memory(_) => crate::layer::root(),
561+
Source::Memory(_, root) => root.clone(),
561562
_ => default_root(),
562563
}
563564
}
@@ -653,12 +654,13 @@ pub fn run_replayed(case: &Case, recording: &Recording) -> Result<Observed> {
653654
}
654655

655656
/// Runs `case` as [`run`] runs it on DuckDB, over its DuckDB base copied
656-
/// into the minimal non-SQL test provider ([`crate::memory`], T2.7), and
657-
/// through the second data layer swapped in at the composition root
658-
/// ([`crate::layer`]): every operation the case names, its fetches
659-
/// answered from in-memory Arrow tables, never by a statement. Its run
660-
/// records no driver: nothing runs SQL.
661-
pub fn run_memory(case: &Case, bases: &Bases) -> Result<Observed> {
657+
/// into the minimal non-SQL test provider ([`crate::memory`], T2.7), every
658+
/// run started with `root`: the second data layer's
659+
/// ([`crate::layer::root`]) to run it swapped, or `default_root()`, over
660+
/// `sdm-data`, to run the same provider on the default layer. Every
661+
/// operation the case names, its fetches answered from in-memory Arrow
662+
/// tables, never by a statement; its run records no driver.
663+
pub fn run_memory(case: &Case, bases: &Bases, root: &CompositionRoot) -> Result<Observed> {
662664
let Some(spec) = case.core.as_ref().and_then(|c| c.describe_source.as_ref()) else {
663665
bail!("{}: reads no data (names no describe_source)", case.id);
664666
};
@@ -675,7 +677,7 @@ pub fn run_memory(case: &Case, bases: &Bases) -> Result<Observed> {
675677
describe_on(
676678
case,
677679
spec,
678-
Source::Memory(&memory),
680+
Source::Memory(&memory, root),
679681
&mut DriverLog::default(),
680682
&edits,
681683
)
@@ -795,7 +797,7 @@ fn drive(
795797
log_replayed(log, &replayed);
796798
replayed.answer
797799
}
798-
Source::Memory(memory) => {
800+
Source::Memory(memory, _) => {
799801
let mut sets = BTreeMap::new();
800802
for (id, fetched) in &keys {
801803
sets.insert(

‎rust/crates/sdm-testkit/tests/memory_provider.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use sdm_contract::data::{DataLayer, Dialect, ProviderChoice};
2222
use sdm_testkit::bases;
2323
use sdm_testkit::case::{self, Case};
2424
use sdm_testkit::host::{self, Answer, Observed};
25-
use sdm_testkit::layer::HostLayer;
25+
use sdm_testkit::layer::{self, HostLayer};
2626
use serde_json::{Value, json};
2727

2828
fn repository() -> PathBuf {
@@ -63,7 +63,7 @@ fn a_prediction_is_duckdbs_but_for_its_drawn_examples() {
6363
for id in ["base.binary", "shape.same-table-twice.buyers"] {
6464
let given = case(id);
6565
let on_duckdb = host::run(&given, &bases).unwrap();
66-
let swapped = host::run_memory(&given, &bases).unwrap();
66+
let swapped = host::run_memory(&given, &bases, &layer::root()).unwrap();
6767
assert!(swapped.driver.is_none(), "{id}: the swapped run ran SQL");
6868
assert!(
6969
swapped
@@ -96,7 +96,7 @@ fn two_schema_tables_over_one_source_predict_as_two_sources() {
9696
// the same provider gives over the two tables.
9797
let bases = bases();
9898
let given = case("shape.same-table-twice.buyers");
99-
let two_sources = host::run_memory(&given, &bases).unwrap();
99+
let two_sources = host::run_memory(&given, &bases, &layer::root()).unwrap();
100100
let mut one_source = given.clone();
101101
one_source
102102
.input
@@ -113,7 +113,7 @@ fn two_schema_tables_over_one_source_predict_as_two_sources() {
113113
table["source"] = json!({"source": ["customers"]});
114114
}
115115
}
116-
let swapped = host::run_memory(&one_source, &bases).unwrap();
116+
let swapped = host::run_memory(&one_source, &bases, &layer::root()).unwrap();
117117
let read: Vec<&Value> = swapped
118118
.fetches
119119
.iter()

0 commit comments

Comments
 (0)