: config: straighten out naming (#1995)

Summary:

this diff follows up on mariusae's [comment in D87795973](https://www.internalfb.com/diff/D87795973?dst_version_fbid=861202266855169&transaction_fbid=1244851620803763) and completes the cleanup he hinted at. the main change is to make the Python-owned "Runtime" layer an explicit, coherent concept throughout the module. the Rust helpers are renamed (`*_py`, `configure_kwarg`, etc.) so their roles are unambiguous, and the documentation now reflects the actual data flow: `configure(**kwargs)` writes only into `Source::Runtime`, while `get_global_config` and `get_runtime_config` clearly separate merged vs. layer-local views.

on the Python side, the API surface is aligned with these semantics. the `configured(...)` context manager (the 'test_config.py' version) is rewritten to snapshot and restore only the Runtime layer instead of resetting global config, which makes overrides composable while continuing to prevent state leakage across tests. the .pyi file is updated to document the layered model and the historical naming of `configure(...)`.

this diff further formalizes the contract implied by the earlier code: Python exclusively owns the Runtime layer, and all read/write paths now reflect that consistently in naming, behavior, and documentation.

Reviewed By: dulinriley

Differential Revision: D87866946

Author: shayne-fletcher

monarch_hyperactor/src/config.rs

@@ -6,10 +6,15 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-//! Configuration for Monarch Hyperactor.
+//! Configuration bridge for Monarch Hyperactor.
 //!
-//! This module provides monarch-specific configuration attributes that extend
-//! the base hyperactor configuration system.
+//! This module defines Monarch-specific configuration keys and their
+//! Python bindings on top of the core `hyperactor::config::global`
+//! system. It wires those keys into the layered config and exposes
+//! Python-facing helpers such as `configure(...)`,
+//! `get_global_config()`, `get_runtime_config()`, and
+//! `clear_runtime_config()`, which together implement the "Runtime"
+//! configuration layer used by the Monarch Python API.
 
 use std::collections::HashMap;
 use std::fmt::Debug;
@@ -79,10 +84,14 @@ static TYPEHASH_TO_INFO: std::sync::LazyLock<HashMap<u64, &'static PythonConfigT
             .collect()
     });
 
-/// Given a key, get the associated `T`-typed value from the global config, then
-/// convert it to a `P`-typed object that can be converted to PyObject, and
-/// return that PyObject.
-fn get_global_config<'py, P, T>(
+/// Fetch a config value from the layered global config and convert it
+/// to Python.
+///
+/// Looks up `key` in the full configuration
+/// (Defaults/File/Env/Runtime/ TestOverride), clones the `T`-typed
+/// value if present, converts it to `P`, then into a `PyObject`. If
+/// the key is unset in all layers, returns `Ok(None)`.
+fn get_global_config_py<'py, P, T>(
     py: Python<'py>,
     key: &'static dyn ErasedKey,
 ) -> PyResult<Option<PyObject>>
@@ -91,8 +100,15 @@ where
     P: IntoPyObjectExt<'py>,
     PyErr: From<<T as TryInto<P>>::Error>,
 {
-    // Well, it can't fail unless there's a bug in the code in this file.
-    let key = key.downcast_ref::<T>().expect("cannot fail");
+    // The error case should never happen. If somehow it ever does
+    // we'll represent "our typing assumptions are wrong" by returning
+    // a PyTypeError rather than a panic.
+    let key = key.downcast_ref::<T>().ok_or_else(|| {
+        PyTypeError::new_err(format!(
+            "internal config type mismatch for key `{}`",
+            key.name(),
+        ))
+    })?;
     let val: Option<P> = hyperactor::config::global::try_get_cloned(key.clone())
         .map(|v| v.try_into())
         .transpose()?;
@@ -102,11 +118,12 @@ where
 /// Fetch a config value from the **Runtime** layer only and convert
 /// it to Python.
 ///
-/// This mirrors [`get_global_config`] but restricts the lookup to the
-/// `Source::Runtime` layer (ignoring TestOverride/Env/File/defaults).
-/// If the key has a runtime override, it is cloned as `T`, converted
-/// to `P`, then to a `PyObject`; otherwise `Ok(None)` is returned.
-fn get_runtime_config<'py, P, T>(
+/// This mirrors [`get_global_config_py`] but restricts the lookup to
+/// the `Source::Runtime` layer (ignoring
+/// TestOverride/Env/File/defaults). If the key has a runtime
+/// override, it is cloned as `T`, converted to `P`, then to a
+/// `PyObject`; otherwise `Ok(None)` is returned.
+fn get_runtime_config_py<'py, P, T>(
     py: Python<'py>,
     key: &'static dyn ErasedKey,
 ) -> PyResult<Option<PyObject>>
@@ -125,8 +142,16 @@ where
     val.map(|v| v.into_py_any(py)).transpose()
 }
 
-/// Note that this function writes strictly into the `Runtime` layer.
-fn set_runtime_config<T: AttrValue + Debug>(key: &'static dyn ErasedKey, value: T) -> PyResult<()> {
+/// Store a Python-provided config value into the **Runtime** layer.
+///
+/// This is the write-path for the "Python configuration layer": it
+/// takes a typed key/value and merges it into `Source::Runtime` via
+/// `create_or_merge`. No other layers
+/// (Env/File/TestOverride/Defaults) are affected.
+fn set_runtime_config_py<T: AttrValue + Debug>(
+    key: &'static dyn ErasedKey,
+    value: T,
+) -> PyResult<()> {
     // Again, can't fail unless there's a bug in the code in this file.
     let key = key.downcast_ref().expect("cannot fail");
     let mut attrs = Attrs::new();
@@ -135,8 +160,22 @@ fn set_runtime_config<T: AttrValue + Debug>(key: &'static dyn ErasedKey, value:
     Ok(())
 }
 
-fn set_runtime_config_from_py_obj(py: Python<'_>, name: &str, val: PyObject) -> PyResult<()> {
-    // Get the `ErasedKey` from the kwarg `name` passed to `monarch.configure(...)`.
+/// Bridge a single Python kwarg into a typed Runtime config update.
+///
+/// This is the write-path behind `configure(**kwargs)`:
+/// - `configure(...)` calls this for each `(name, value)` pair,
+/// - we resolve `name` to an erased config key via `KEY_BY_NAME`,
+/// - we use the key's `typehash` to find the registered
+///   `PythonConfigTypeInfo`,
+/// - and finally call its `set_runtime_config` closure, which
+///   downcasts the value and forwards to `set_runtime_config_py` to
+///   write into the `Source::Runtime` layer.
+///
+/// Unknown keys or keys without a Python conversion registered result
+/// in a `ValueError` / `TypeError` back to Python.
+fn configure_kwarg(py: Python<'_>, name: &str, val: PyObject) -> PyResult<()> {
+    // Get the `ErasedKey` from the kwarg `name` passed to
+    // `monarch.configure(...)`.
     let key = match KEY_BY_NAME.get(name) {
         None => {
             return Err(PyValueError::new_err(format!(
@@ -147,8 +186,9 @@ fn set_runtime_config_from_py_obj(py: Python<'_>, name: &str, val: PyObject) ->
         Some(key) => *key,
     };
 
-    // Using the typehash from the erased key, get/call the function that can downcast
-    // the key and set the value on the global config.
+    // Using the typehash from the erased key, get/call the function
+    // that can downcast the key and set the value on the global
+    // config.
     match TYPEHASH_TO_INFO.get(&key.typehash()) {
         None => Err(PyTypeError::new_err(format!(
             "configuration key `{}` has type `{}`, but configuring with values of this type from Python is not supported.",
@@ -159,22 +199,47 @@ fn set_runtime_config_from_py_obj(py: Python<'_>, name: &str, val: PyObject) ->
     }
 }
 
-/// Struct to associate a typehash with functions for getting/setting
-/// values in the global config with keys of type `Key<T>`, where
-/// `T::typehash() == PythonConfigTypeInfo::typehash()`.
+/// Per-type adapter for the Python config bridge.
+///
+/// Each `PythonConfigTypeInfo` provides type-specific get/set logic
+/// for a particular `Key<T>` via the type-erased `ErasedKey`
+/// interface.
+///
+/// Since we only have `&'static dyn ErasedKey` at runtime (we don't
+/// know `T`), we use **type erasure with recovery via function
+/// pointers**: the `declare_py_config_type!` macro bakes the concrete
+/// type `T` into each function pointer at compile time, allowing
+/// runtime dispatch to recover the type.
+///
+/// Fields:
+/// - `typehash`: Identifies the underlying `T` for runtime lookup
+/// - `set_global_config`: Knows how to extract `PyObject` as `T` and
+///   write to Runtime layer
+/// - `get_global_config`: Reads `T` from merged config (all layers)
+///   and converts to `PyObject`
+/// - `get_runtime_config`: Reads `T` from Runtime layer only and
+///   converts to `PyObject`
+///
+/// Instances are registered via `inventory` and collected into
+/// `TYPEHASH_TO_INFO`, enabling dynamic dispatch by typehash in
+/// `configure()` and `get_*_config()`.
 struct PythonConfigTypeInfo {
+    /// Identifies the underlying `T` (matches `T::typehash()`).
     typehash: fn() -> u64,
-
+    /// Read this key from the merged layered config into a PyObject.
     get_global_config:
         fn(py: Python<'_>, key: &'static dyn ErasedKey) -> PyResult<Option<PyObject>>,
-
+    /// Write a Python value into the Runtime layer for this key.
     set_runtime_config:
         fn(py: Python<'_>, key: &'static dyn ErasedKey, val: PyObject) -> PyResult<()>,
-
+    /// Read this key from the Runtime layer into a PyObject.
     get_runtime_config:
         fn(py: Python<'_>, key: &'static dyn ErasedKey) -> PyResult<Option<PyObject>>,
 }
 
+// Collect all `PythonConfigTypeInfo` instances registered by
+// `declare_py_config_type!`. These are later gathered into
+// `TYPEHASH_TO_INFO` via `inventory::iter()`.
 inventory::collect!(PythonConfigTypeInfo);
 
 /// Macro to declare that keys of this type can be configured
@@ -197,13 +262,13 @@ macro_rules! declare_py_config_type {
                                 "invalid value `{}` for configuration key `{}` ({})",
                                 val, key.name(), err
                             )))?;
-                            set_runtime_config(key, val)
+                            set_runtime_config_py(key, val)
                         },
                         get_global_config: |py, key| {
-                            get_global_config::<$ty, $ty>(py, key)
+                            get_global_config_py::<$ty, $ty>(py, key)
                         },
                         get_runtime_config: |py, key| {
-                            get_runtime_config::<$ty, $ty>(py, key)
+                            get_runtime_config_py::<$ty, $ty>(py, key)
                         }
                     }
                 }
@@ -220,13 +285,13 @@ macro_rules! declare_py_config_type {
                             "invalid value `{}` for configuration key `{}` ({})",
                             val, key.name(), err
                         )))?.into();
-                        set_runtime_config(key, val)
+                        set_runtime_config_py(key, val)
                     },
                     get_global_config: |py, key| {
-                        get_global_config::<$py_ty, $ty>(py, key)
+                        get_global_config_py::<$py_ty, $ty>(py, key)
                     },
                     get_runtime_config: |py, key| {
-                        get_runtime_config::<$py_ty, $ty>(py, key)
+                        get_runtime_config_py::<$py_ty, $ty>(py, key)
                     }
                 }
             }
@@ -239,29 +304,48 @@ declare_py_config_type!(
     i8, i16, i32, i64, u8, u16, u32, u64, usize, f32, f64, bool, String
 );
 
-/// Iterate over each key-value pair. Attempt to retrieve the `Key<T>`
-/// associated with the key and convert the value to `T`, then set
-/// them on the global config. The association between kwarg and
-/// `Key<T>` is specified using the `CONFIG` meta-attribute.
+/// Python entrypoint for `monarch_hyperactor.config.configure(...)`.
+///
+/// This takes the keyword arguments passed from Python, resolves each
+/// kwarg name to a typed config key via `KEY_BY_NAME` (populated from
+/// `@meta(CONFIG = ConfigAttr { py_name: Some(...), .. })`), and then
+/// uses `configure_kwarg` to downcast the value and write it into the
+/// **Runtime** configuration layer.
+///
+/// In other words, this is the write-path from `configure(**kwargs)`
+/// into `Source::Runtime`; other layers
+/// (Env/File/TestOverride/Defaults) are untouched.
+///
+/// The name `configure(...)` is historical – conceptually this is
+/// `set_runtime_config(...)` for the Python-owned Runtime layer, but
+/// we keep the shorter name for API stability.
 #[pyfunction]
 #[pyo3(signature = (**kwargs))]
 fn configure(py: Python<'_>, kwargs: Option<HashMap<String, PyObject>>) -> PyResult<()> {
     kwargs
         .map(|kwargs| {
             kwargs
                 .into_iter()
-                .try_for_each(|(key, val)| set_runtime_config_from_py_obj(py, &key, val))
+                .try_for_each(|(key, val)| configure_kwarg(py, &key, val))
         })
         .transpose()?;
     Ok(())
 }
 
-/// For all attribute keys whose `@meta(CONFIG = ConfigAttr { py_name:
-/// Some(...), .. })` specifies a kwarg name, return the current
-/// associated value in the global config. Keys with no value in the
-/// global config are omitted from the result.
+/// Return a snapshot of the current Hyperactor configuration for
+/// Python-exposed keys.
+///
+/// Iterates over all attribute keys whose `@meta(CONFIG = ConfigAttr
+/// { py_name: Some(...), .. })` declares a Python kwarg name, looks
+/// up each key in the **layered** global config
+/// (Defaults/File/Env/Runtime/TestOverride), and, if set, converts
+/// the value to a `PyObject`.
+///
+/// The result is a plain `HashMap` from kwarg name to value for all
+/// such keys that currently have a value in the global config; keys
+/// with no value in any layer are omitted.
 #[pyfunction]
-fn get_configuration(py: Python<'_>) -> PyResult<HashMap<String, PyObject>> {
+fn get_global_config(py: Python<'_>) -> PyResult<HashMap<String, PyObject>> {
     KEY_BY_NAME
         .iter()
         .filter_map(|(name, key)| match TYPEHASH_TO_INFO.get(&key.typehash()) {
@@ -282,25 +366,25 @@ fn get_configuration(py: Python<'_>) -> PyResult<HashMap<String, PyObject>> {
 /// `@meta(CONFIG = ConfigAttr { py_name: Some(...), .. })`) that are
 /// currently set in the Runtime layer.
 ///
-/// This is used by Python's `configured()` context manager to
+/// This can be used to implement a `configured()` context manager to
 /// snapshot and restore the Runtime layer for composable, nested
 /// configuration overrides:
 ///
 /// ```python
-/// prev = get_runtime_configuration()
+/// prev = get_runtime_config()
 /// try:
 ///     configure(**overrides)
-///     yield get_configuration()
+///     yield get_global_config()
 /// finally:
-///     clear_runtime_configuration()
+///     clear_runtime_config()
 ///     configure(**prev)
 /// ```
 ///
-/// Unlike `get_configuration()`, which returns the merged view across
-/// all layers (File, Env, Runtime, TestOverride), this returns only
-/// what's explicitly set in the Runtime layer.
+/// Unlike `get_global_config()`, which returns the merged view across
+/// all layers (File, Env, Runtime, TestOverride, defaults), this
+/// returns only what's explicitly set in the Runtime layer.
 #[pyfunction]
-fn get_runtime_configuration(py: Python<'_>) -> PyResult<HashMap<String, PyObject>> {
+fn get_runtime_config(py: Python<'_>) -> PyResult<HashMap<String, PyObject>> {
     KEY_BY_NAME
         .iter()
         .filter_map(|(name, key)| match TYPEHASH_TO_INFO.get(&key.typehash()) {
@@ -325,7 +409,7 @@ fn get_runtime_configuration(py: Python<'_>) -> PyResult<HashMap<String, PyObjec
 /// to restore configuration state after applying temporary overrides.
 /// Other layers (Env, File, TestOverride, defaults) are unaffected.
 #[pyfunction]
-fn clear_runtime_configuration(_py: Python<'_>) -> PyResult<()> {
+fn clear_runtime_config(_py: Python<'_>) -> PyResult<()> {
     hyperactor::config::global::clear(Source::Runtime);
     Ok(())
 }
@@ -353,26 +437,26 @@ pub fn register_python_bindings(module: &Bound<'_, PyModule>) -> PyResult<()> {
     )?;
     module.add_function(configure)?;
 
-    let get_configuration = wrap_pyfunction!(get_configuration, module)?;
-    get_configuration.setattr(
+    let get_global_config = wrap_pyfunction!(get_global_config, module)?;
+    get_global_config.setattr(
         "__module__",
         "monarch._rust_bindings.monarch_hyperactor.config",
     )?;
-    module.add_function(get_configuration)?;
+    module.add_function(get_global_config)?;
 
-    let get_runtime_configuration = wrap_pyfunction!(get_runtime_configuration, module)?;
-    get_runtime_configuration.setattr(
+    let get_runtime_config = wrap_pyfunction!(get_runtime_config, module)?;
+    get_runtime_config.setattr(
         "__module__",
         "monarch._rust_bindings.monarch_hyperactor.config",
     )?;
-    module.add_function(get_runtime_configuration)?;
+    module.add_function(get_runtime_config)?;
 
-    let clear_runtime_configuration = wrap_pyfunction!(clear_runtime_configuration, module)?;
-    clear_runtime_configuration.setattr(
+    let clear_runtime_config = wrap_pyfunction!(clear_runtime_config, module)?;
+    clear_runtime_config.setattr(
         "__module__",
         "monarch._rust_bindings.monarch_hyperactor.config",
     )?;
-    module.add_function(clear_runtime_configuration)?;
+    module.add_function(clear_runtime_config)?;
 
     Ok(())
 }