refactor: retain native model config trees (#1276)

Keep built config nodes across reuse and final record replacement instead of reconstructing them from Python dictionaries. Preserve lazy opaque containers, expanded hooks and Python-authored config reads.

Amp-Thread-ID: https://ampcode.com/threads/T-01a18715-4e00-708e-a87d-6bd109748d66

Author: kvlonge

crates/sqlbuild-python/src/bindings/_helpers/input_loops/model_loop_config.rs

@@ -2,6 +2,7 @@
 
 use std::cell::RefCell;
 use std::collections::HashMap;
+use std::sync::Arc;
 
 use pyo3::prelude::{Bound, PyAny, PyAnyMethods, PyResult, Python};
 use pyo3::types::{PyDict, PyDictMethods, PyString, PyStringMethods, PyTuple};
@@ -13,9 +14,11 @@ use sqlbuild_model_config::config_presence::main::mutable_config_value::mutable_
 use crate::bindings::_helpers::input_loops::model_loop::{LoopContext, config_error};
 use crate::bindings::_helpers::model_config::authored_nodes::PyNode;
 use crate::bindings::_helpers::model_config::config_classes::{ConfigClasses, config_fields, view};
+use crate::bindings::_helpers::model_config::config_nodes::carried_node;
 use crate::bindings::_helpers::model_config::model_config_builder::{
     BuiltConfig, ConfigBuild, NativeModelConfigBuilder,
 };
+use crate::bindings::_helpers::model_config::typed_config::BuiltConfigValues;
 
 const COLUMNS_KEY: &str = "columns";
 const DESCRIPTION_KEY: &str = "description";
@@ -30,6 +33,7 @@ struct InheritedLayer<'py> {
 struct ReusedConfig<'py> {
     values: Bound<'py, PyDict>,
     fields: Bound<'py, PyDict>,
+    nodes: Option<BuiltConfigValues>,
 }
 
 /// `_ReusableModelConfigCache` for one compile.
@@ -66,7 +70,8 @@ impl<'py> ModelConfigs<'py> {
         {
             let values: Bound<'py, PyDict> = cached.values.copy()?;
             values.update(metadata.as_mapping())?;
-            return view(classes, &values, &cached.fields);
+            let nodes = metadata_nodes(cached.nodes.as_ref(), metadata, metadata)?;
+            return view(classes, &values, &cached.fields, nodes.as_ref());
         }
         let built: ConfigBuild<'py> =
             builder.built(py, (header, facts), matched.as_deref(), name)?;
@@ -78,9 +83,9 @@ impl<'py> ModelConfigs<'py> {
         };
         let fields: Bound<'py, PyDict> = config_fields(classes, &built, matched.as_deref())?;
         if reusable && let Some(metadata) = &metadata {
-            self.remember(&matched, metadata, &built.values, &fields)?;
+            self.remember(&matched, metadata, &built, &fields)?;
         }
-        view(classes, &built.values, &fields)
+        view(classes, &built.values, &fields, built.nodes.as_ref())
     }
 
     /// `_is_reusable`: no templates in the inherited values or the target namespace, nothing mutable.
@@ -116,9 +121,10 @@ impl<'py> ModelConfigs<'py> {
         &self,
         matched: &Option<String>,
         metadata: &Bound<'py, PyDict>,
-        values: &Bound<'py, PyDict>,
+        built: &BuiltConfig<'py>,
         fields: &Bound<'py, PyDict>,
     ) -> PyResult<()> {
+        let values = &built.values;
         let py: Python<'py> = values.py();
         let layers = self.layers.borrow();
         let Some(inherited) = layers.get(matched) else {
@@ -140,12 +146,36 @@ impl<'py> ModelConfigs<'py> {
             ReusedConfig {
                 values: reusable_values,
                 fields: fields.clone(),
+                nodes: metadata_nodes(built.nodes.as_ref(), metadata, &inherited.values)?,
             },
         );
         Ok(())
     }
 }
 
+/// Replace only one model's metadata in a reused tree, without opening unused containers.
+fn metadata_nodes(
+    base: Option<&BuiltConfigValues>,
+    metadata: &Bound<'_, PyDict>,
+    replacements: &Bound<'_, PyDict>,
+) -> PyResult<Option<BuiltConfigValues>> {
+    let Some(base) = base else {
+        return Ok(None);
+    };
+    let mut nodes = base.as_ref().clone();
+    for key in metadata.keys() {
+        let Ok(text) = key.downcast_exact::<PyString>() else {
+            return Ok(None);
+        };
+        let text = text.to_str()?;
+        nodes.remove(text);
+        if let Some(value) = replacements.get_item(&key)? {
+            nodes.insert(text.to_owned(), Arc::new(carried_node(&value)?));
+        }
+    }
+    Ok(Some(Arc::new(nodes)))
+}
+
 /// `_reusable_metadata`: the header when only plain columns and description vary, else `None`.
 fn reusable_metadata<'py>(
     header: &Bound<'py, PyDict>,

crates/sqlbuild-python/src/bindings/_helpers/input_loops/model_loop_host.rs

@@ -5,6 +5,7 @@ use std::sync::Arc;
 use pyo3::FromPyObject;
 use pyo3::prelude::{Bound, Py, PyAny, PyAnyMethods, PyResult, Python};
 use pyo3::types::{PyDict, PyDictMethods, PyTuple};
+use sqlbuild_analysis::compiled_project::main::header_metadata_keys::is_header_metadata_key;
 use sqlbuild_analysis::compiled_project::main::plain_model_facts::plain_model_facts;
 use sqlbuild_analysis::compiled_project::models::{
     CompiledModelFacts, ModelConfigFacts, PlainModel,
@@ -23,7 +24,11 @@ use crate::bindings::_helpers::compiled_views::model_input_view::{
 };
 use crate::bindings::_helpers::discovery::model_file_record::{ModelFileTexts, NativeModelFile};
 use crate::bindings::_helpers::input_loops::model_loop_header::AttachedHeader;
-use crate::bindings::_helpers::input_loops::model_loop_inputs::{reference_fact, stripped_values};
+use crate::bindings::_helpers::input_loops::model_loop_inputs::{
+    HOOK_KEYS, reference_fact, stripped_values,
+};
+use crate::bindings::_helpers::model_config::config_nodes::carried_node;
+use crate::bindings::_helpers::model_config::typed_config::{retain_values, typed_values};
 use crate::bindings::_helpers::records::fields::{field as native_field, replaced_keywords};
 use crate::bindings::_helpers::render::declaration_expansion::{ExpandedDeclarations, span_rows};
 use crate::bindings::_helpers::render::macro_call_rows::{SpanRow, span_row};
@@ -169,6 +174,17 @@ pub(crate) fn finished_record<'py>(
     let replaced: Bound<'py, PyDict> = PyDict::new(py);
     replaced.set_item(VALUES_ARGUMENT, &stripped)?;
     let config: Bound<'py, PyAny> = replaced_keywords(&finished.config, &replaced)?;
+    let (_, typed) = typed_values(&finished.config)?;
+    if let Some(built) = &typed.get().built {
+        let mut nodes = built.as_ref().clone();
+        nodes.retain(|key, _| !is_header_metadata_key(key));
+        for key in HOOK_KEYS {
+            if let Some(value) = stripped.get_item(key)? {
+                nodes.insert(key.to_owned(), Arc::new(carried_node(&value)?));
+            }
+        }
+        retain_values(&config, Some(&Arc::new(nodes)))?;
+    }
     let mut declaration_usages: Vec<Bound<'py, PyAny>> = unique(py, usages)?;
     declaration_usages.extend(finished.named_usages);
     let sql_macro_names: Vec<String> = finished.declarations.sql_macros.clone();

crates/sqlbuild-python/src/bindings/_helpers/model_config/config_classes.rs

@@ -8,6 +8,7 @@ use sqlbuild_model_config::model_validation::models::{RetentionOverride, TableTy
 use crate::bindings::_helpers::model_config::model_config_builder::{
     BuiltConfig, NativeModelConfigBuilder,
 };
+use crate::bindings::_helpers::model_config::typed_config::{BuiltConfigValues, retain_values};
 
 const MATERIALIZED_KEY: &str = "materialized";
 const VALUES_FIELD: &str = "values";
@@ -39,7 +40,7 @@ pub(crate) fn built_config<'py>(
     matched: Option<&str>,
 ) -> PyResult<Bound<'py, PyAny>> {
     let fields: Bound<'py, PyDict> = config_fields(classes, built, matched)?;
-    view(classes, &built.values, &fields)
+    view(classes, &built.values, &fields, built.nodes.as_ref())
 }
 
 /// Every `CompileModelConfig` field but `values`, recording the build's template reads first.
@@ -139,8 +140,11 @@ pub(crate) fn view<'py>(
     classes: &ConfigClasses<'py>,
     values: &Bound<'py, PyDict>,
     fields: &Bound<'py, PyDict>,
+    nodes: Option<&BuiltConfigValues>,
 ) -> PyResult<Bound<'py, PyAny>> {
     let arguments: Bound<'py, PyDict> = fields.copy()?;
     arguments.set_item(VALUES_FIELD, values)?;
-    classes.compile_model_config.call((), Some(&arguments))
+    let config = classes.compile_model_config.call((), Some(&arguments))?;
+    retain_values(&config, nodes)?;
+    Ok(config)
 }

crates/sqlbuild-python/src/bindings/_helpers/model_config/config_nodes.rs

@@ -174,9 +174,13 @@ fn convert(value: &Bound<'_, PyAny>, open: &[usize]) -> PyResult<Option<PyConfig
     }))
 }
 
-/// An exact `list`, `tuple` or `dict` carried as its own object, its items never converted.
-pub(crate) fn opaque_container(value: &Bound<'_, PyAny>) -> PyResult<PyConfigNode> {
-    host(value, HostChildren::None)
+/// Carry an uninspected value: exact containers keep their own objects without opening items.
+pub(crate) fn carried_node(value: &Bound<'_, PyAny>) -> PyResult<PyConfigNode> {
+    if is_exact_container(value) {
+        host(value, HostChildren::None)
+    } else {
+        config_node(value)
+    }
 }
 
 /// Whether `value` is an exact `list`, `tuple` or `dict`.

crates/sqlbuild-python/src/bindings/_helpers/model_config/model_config_builder.rs

@@ -25,11 +25,12 @@ use crate::bindings::_helpers::model_config::config_classes::{ConfigClasses, bui
 use crate::bindings::_helpers::model_config::config_errors::config_exception;
 use crate::bindings::_helpers::model_config::config_errors::native_config_error;
 use crate::bindings::_helpers::model_config::config_nodes::{
-    HostHandle, PyConfigEntries, PyConfigNode, config_dict, config_entries, config_node,
-    config_object, hook_entry, is_exact_container, opaque_container,
+    HostHandle, PyConfigEntries, PyConfigNode, carried_node, config_dict, config_entries,
+    config_node, config_object, hook_entry, is_exact_container,
 };
 use crate::bindings::_helpers::model_config::errors::NativeConfigError;
 use crate::bindings::_helpers::model_config::project_defaults::project_defaults_mapping;
+use crate::bindings::_helpers::model_config::typed_config::{BuiltConfigValues, built_values};
 
 /// The config keys a model's context values read.
 const CONTEXT_KEYS: [&str; 3] = ["database", "schema", "alias"];
@@ -46,6 +47,7 @@ pub(crate) struct NativeModelConfigBuilder {
 /// One model's built config before Python wraps it in `CompileModelConfig`.
 pub(crate) struct BuiltConfig<'py> {
     pub(crate) values: Bound<'py, PyDict>,
+    pub(crate) nodes: Option<BuiltConfigValues>,
     pub(crate) header_keys: Vec<String>,
     pub(crate) logical_schema: Option<Bound<'py, PyAny>>,
     pub(crate) layer_schema_configured: bool,
@@ -159,7 +161,7 @@ fn header_entries(
                 .downcast_exact::<PyString>()
                 .is_ok_and(|key| key.to_str().is_ok_and(|key| carried_whole(key, facts)));
         let node: PyConfigNode = if whole {
-            opaque_container(&value)?
+            carried_node(&value)?
         } else {
             config_node(&value)?
         };
@@ -297,6 +299,7 @@ fn projected_build<'py>(
     let py: Python<'py> = header.py();
     Ok(BuiltConfig {
         values: own_values_dict(header, &built)?,
+        nodes: built_values(built.values),
         header_keys: built.header_keys,
         logical_schema: projected(py, built.logical_schema)?,
         layer_schema_configured: built.layer_schema_configured,

crates/sqlbuild-python/src/bindings/_helpers/model_config/typed_config.rs

@@ -1,5 +1,6 @@
-//! Model config values read as typed config nodes, each key converted on first read and kept.
+//! Built model config trees retained natively, with Python-authored values converted on first read.
 
+use std::collections::HashMap;
 use std::sync::{Arc, Mutex};
 
 use pyo3::exceptions::PyRuntimeError;
@@ -12,11 +13,11 @@ use sqlbuild_model_config::config_values::main::checked_flag::checked_flag;
 use sqlbuild_model_config::config_values::main::checked_integer::checked_integer;
 use sqlbuild_model_config::config_values::main::checked_text::checked_text;
 use sqlbuild_model_config::config_values::main::checked_texts::checked_texts;
-use sqlbuild_model_config::config_values::models::{ConfigNode, CursorBound};
+use sqlbuild_model_config::config_values::models::{ConfigNode, CursorBound, HostChildren};
 
 use crate::bindings::_helpers::compile_driver::stage_work::{TYPED_CONFIG_READS, count_stage};
 use crate::bindings::_helpers::model_config::config_nodes::{
-    HostHandle, PyConfigNode, config_node, config_object,
+    HostHandle, PyConfigEntries, PyConfigNode, config_node, config_object, is_exact_container,
 };
 use crate::bindings::_helpers::model_config::config_values::config_value_type_error;
 use crate::bindings::_helpers::records::fields::{
@@ -31,19 +32,69 @@ const ISOFORMAT_METHOD: &str = "isoformat";
 const TYPED_VALUES_SLOT: &str = "__typed_values__";
 const POISONED: &str = "typed config values cache is poisoned";
 
-/// The typed nodes of a config record's values read so far, by key.
+/// A built config's string-keyed tree; values shared across reused configs stay native.
+pub(crate) type BuiltConfigValues = Arc<HashMap<String, Arc<PyConfigNode>>>;
+
+/// The built tree and any Python-authored values opened by the record's readers.
 #[pyclass(module = "sqlbuild._native", frozen)]
 pub(crate) struct TypedConfigValues {
+    pub(crate) built: Option<BuiltConfigValues>,
     read: Mutex<Vec<(String, Arc<PyConfigNode>)>>,
 }
 
+/// Keep the builder's tree without interpreting custom Python mapping keys natively.
+pub(crate) fn built_values(entries: PyConfigEntries) -> Option<BuiltConfigValues> {
+    let mut values = HashMap::with_capacity(entries.len());
+    for (key, value) in entries {
+        let ConfigNode::Str(key) = key else {
+            return None;
+        };
+        values.insert(key, Arc::new(value));
+    }
+    Some(Arc::new(values))
+}
+
+/// Attach the built tree to its public record, rather than reconstructing it from that view.
+pub(crate) fn retain_values(
+    config: &Bound<'_, PyAny>,
+    built: Option<&BuiltConfigValues>,
+) -> PyResult<()> {
+    if let Some(built) = built
+        && let Ok(record) = config.cast::<NativeRecord>()
+    {
+        let typed = Bound::new(
+            config.py(),
+            TypedConfigValues {
+                built: Some(Arc::clone(built)),
+                read: Mutex::new(Vec::new()),
+            },
+        )?;
+        store_cached(record, TYPED_VALUES_SLOT, &typed.into_any().unbind())?;
+    }
+    Ok(())
+}
+
 /// A config record's values and the typed nodes read of them, each key converted on first read.
 pub(crate) type ConfigValues<'py> = (Bound<'py, PyAny>, Bound<'py, TypedConfigValues>);
 
 /// `values.get(key)` as a typed node, `None` when absent; each key is converted once.
 fn node(values: &ConfigValues<'_>, key: &str) -> PyResult<Arc<PyConfigNode>> {
     let (mapping, typed) = values;
     let py: Python<'_> = mapping.py();
+    let built = typed.get().built.as_ref().map(|values| values.get(key));
+    let deferred = match built {
+        Some(Some(node)) => match node.as_ref() {
+            ConfigNode::Host(host)
+                if matches!(host.children, HostChildren::None)
+                    && is_exact_container(host.handle.0.bind(py)) =>
+            {
+                Some(host.handle.0.bind(py))
+            }
+            _ => return Ok(Arc::clone(node)),
+        },
+        Some(None) => return Ok(Arc::new(ConfigNode::Null)),
+        None => None,
+    };
     let read: &Mutex<Vec<(String, Arc<PyConfigNode>)>> = &typed.get().read;
     let poisoned = || PyRuntimeError::new_err(POISONED);
     let found: Option<Arc<PyConfigNode>> = read
@@ -55,7 +106,10 @@ fn node(values: &ConfigValues<'_>, key: &str) -> PyResult<Arc<PyConfigNode>> {
     if let Some(node) = found {
         return Ok(node);
     }
-    let node: Arc<PyConfigNode> = Arc::new(config_node(&mapping_get(mapping, key)?)?);
+    let node: Arc<PyConfigNode> = Arc::new(match deferred {
+        Some(value) => config_node(value)?,
+        None => config_node(&mapping_get(mapping, key)?)?,
+    });
     count_stage(py, TYPED_CONFIG_READS, 1)?;
     read.lock()
         .map_err(|_| poisoned())?
@@ -71,6 +125,7 @@ pub(crate) fn typed_values<'py>(config: &Bound<'py, PyAny>) -> PyResult<ConfigVa
         Bound::new(
             py,
             TypedConfigValues {
+                built: None,
                 read: Mutex::new(Vec::new()),
             },
         )

tests/integration/src/sqlbuild/cli/commands/main/helpers.py

@@ -1025,19 +1025,20 @@ def diagnostics_by_model(diagnostics: list[dict[str, object]]) -> dict[str, str]
 
 
 def typed_config_reads(*, project_dir: Path, extra_header: str) -> int:
-    """How many config values planning converted into typed nodes for three cursor models."""
+    """Config conversions for a fresh build, an inherited build and a reused config."""
 
     project_dir.mkdir(parents=True)
     (project_dir / "sqlbuild_project.toml").write_text(
         'name = "orders"\nadapter = "duckdb"\ndefault_target = "dev"\n\n'
-        '[connections.local]\ndatabase = ":memory:"\n\n[targets.dev]\nschema = "main"\n',
+        '[connections.local]\ndatabase = ":memory:"\n\n[defaults]\nmaterialized = "table"\n\n'
+        '[targets.dev]\nschema = "main"\n',
         encoding="utf-8",
     )
     models_dir: Path = project_dir / "models"
     models_dir.mkdir()
-    for index in range(3):
+    for index, own_config in enumerate((', materialized table, tags ["daily"]', "", "")):
         (models_dir / f"orders_{index}.sql").write_text(
-            f'MODEL (description "Orders.", materialized table, tags ["daily"]{extra_header});\n\n'
+            f'MODEL (description "Orders {index}."{own_config}{extra_header});\n\n'
             f"SELECT {index} AS order_id, 1 AS loaded_at\n",
             encoding="utf-8",
         )

tests/integration/src/sqlbuild/cli/commands/main/test_plan_typed_config.py

@@ -1,4 +1,4 @@
-"""Planning converts only the config values its readers ask for."""
+"""Planning retains native config trees and leaves unread Python containers unopened."""
 
 from __future__ import annotations
 
@@ -35,4 +35,4 @@ def test_given_config_keys_planning_never_reads_when_planning_then_they_are_not_
     )
     _ = capsys.readouterr()
 
-    assert (plain > 0, extended == plain) == (True, test_case.expected_same_reads)
+    assert (plain, extended == plain) == (0, test_case.expected_same_reads)