chore: code quality cleanup

bindings/python/build.rs

@@ -12,7 +12,7 @@
 // the approach used by maturin itself.
 //
 // Under maturin `CARGO_FEATURE_EXTENSION_MODULE` is set, so we skip the flag
-// to avoid a duplicate that maturin already injects (KD3).
+// to avoid a duplicate that maturin already injects.
 
 fn main() {
     #[cfg(target_os = "macos")]

bindings/python/src/conversions.rs

@@ -4,10 +4,10 @@
 // Authors: Ted Habeck
 //
 // PyObject ↔ serde_json::Value traversal, payload resolution, and
-// modified-payload serialization (R6, R3, KD1, KD2, KD5).
+// modified-payload serialization.
 //
 // Never calls Python's `json` module from Rust — all conversion is direct
-// PyObject inspection / construction (#2 / R6).
+// PyObject inspection / construction (#2).
 
 use cpex_core::cmf::MessagePayload;
 use cpex_core::context::PluginContextTable;
@@ -18,11 +18,7 @@ use pyo3::prelude::*;
 use pyo3::types::{PyBool, PyDict, PyFloat, PyInt, PyList, PyString};
 use serde_json::{Map, Value};
 
-// ---------------------------------------------------------------------------
-// GenericPayload — local struct for non-CMF hooks (KD5)
-// ---------------------------------------------------------------------------
-
-/// Wraps any serde_json::Value for hooks that are not `cmf.*` (KD1, KD2).
+/// Wraps any serde_json::Value for hooks that are not `cmf.*`.
 ///
 /// Defined locally because `cpex-core` exports the macro but not the struct
 /// itself (the FFI crate defines its own copy too).
@@ -33,16 +29,12 @@ pub struct GenericPayload {
 
 cpex_core::impl_plugin_payload!(GenericPayload);
 
-// ---------------------------------------------------------------------------
-// PyObject → serde_json::Value
-// ---------------------------------------------------------------------------
-
 /// Convert a Python object to a `serde_json::Value`.
 ///
 /// Supported types: `bool`, `int`, `float`, `str`, `None`, `list`, `dict`
 /// (with `str` keys). Any other type raises `ValueError` naming the type.
 ///
-/// Recursion is capped at 128 levels (R3). `depth` starts at 0.
+/// Recursion is capped at 128 levels. `depth` starts at 0.
 pub fn pyobj_to_json_value(
     _py: Python<'_>,
     obj: &Bound<'_, PyAny>,
@@ -104,10 +96,6 @@ pub fn pyobj_to_json_value(
     )))
 }
 
-// ---------------------------------------------------------------------------
-// serde_json::Value → PyObject
-// ---------------------------------------------------------------------------
-
 /// Convert a `serde_json::Value` to a Python object.
 ///
 /// `null` → `None`, booleans → `bool`, numbers → `int` or `float`,
@@ -145,14 +133,10 @@ pub fn json_value_to_pyobj<'py>(py: Python<'py>, v: &Value) -> PyResult<Bound<'p
     }
 }
 
-// ---------------------------------------------------------------------------
-// Payload resolution
-// ---------------------------------------------------------------------------
-
 /// Build the correct `Box<dyn PluginPayload>` for a hook.
 ///
 /// `cmf.*` hooks → `MessagePayload` (serde-constructed from the value).
-/// All other hook names → `GenericPayload { value }` (KD1, KD2).
+/// All other hook names → `GenericPayload { value }`.
 ///
 /// A `from_value` failure on a CMF payload raises `ValueError` rather than
 /// silently falling through to GenericPayload — the caller sent a cmf hook
@@ -170,15 +154,11 @@ pub fn resolve_payload(hook_name: &str, value: Value) -> PyResult<Box<dyn Plugin
     }
 }
 
-// ---------------------------------------------------------------------------
-// Payload serialization (for modified_payload in PipelineResult)
-// ---------------------------------------------------------------------------
-
 /// Serialize a `&dyn PluginPayload` back to a `serde_json::Value`.
 ///
 /// Returns `None` when the payload type is not in the local registry (unknown
 /// plugin-returned type). The caller should append a synthetic error record to
-/// `PipelineResult.errors` rather than silently dropping the modification (R2).
+/// `PipelineResult.errors` rather than silently dropping the modification.
 ///
 /// Downcast order: `MessagePayload` first (most common for `cmf.*` hooks),
 /// then `GenericPayload` — mirrors cpex-ffi's `serialize_payload` ordering.
@@ -192,10 +172,6 @@ pub fn serialize_payload(payload: &dyn PluginPayload) -> Option<Value> {
     None
 }
 
-// ---------------------------------------------------------------------------
-// Extensions / PluginContextTable helpers
-// ---------------------------------------------------------------------------
-
 /// Deserialize Python dict → `Extensions` via serde.
 ///
 /// An empty dict yields `Extensions::default()` (all fields `#[serde(default)]`).

bindings/python/src/error.rs

@@ -3,15 +3,15 @@
 // SPDX-License-Identifier: Apache-2.0
 // Authors: Ted Habeck
 //
-// Maps Rust PluginError variants to Python exception types (R2, KD9).
+// Maps Rust PluginError variants to Python exception types.
 
 use cpex_core::error::PluginError;
 use pyo3::exceptions::{PyRuntimeError, PyTimeoutError, PyValueError};
 use pyo3::PyErr;
 
 /// Convert a `Box<PluginError>` into the appropriate Python exception.
 ///
-/// Mapping (per C2, KD9):
+/// Mapping:
 ///   Config / UnknownHook → `ValueError`   (config/conversion failures)
 ///   Timeout              → `TimeoutError`
 ///   Execution / other    → `RuntimeError`
@@ -36,7 +36,7 @@ pub fn plugin_error_to_pyerr(e: Box<PluginError>) -> PyErr {
         } => PyTimeoutError::new_err(format!(
             "cpex plugin '{plugin_name}' timed out after {timeout_ms}ms"
         )),
-        // Violation is dead on the invoke_by_name path (KD9); treat defensively.
+        // Violation is dead on the invoke_by_name path; treat defensively.
         PluginError::Violation {
             plugin_name,
             violation,

bindings/python/src/lib.rs

@@ -7,7 +7,7 @@
 //
 // Module registration and tokio runtime initialization.
 // The runtime is initialized once with a multi-thread builder that honours
-// the `CPEX_PY_WORKER_THREADS` environment variable (KD8), mirroring the
+// the `CPEX_PY_WORKER_THREADS` environment variable, mirroring the
 // `CPEX_FFI_WORKER_THREADS` knob in cpex-ffi.
 
 use pyo3::prelude::*;
@@ -53,7 +53,7 @@ fn _lib(m: &Bound<'_, PyModule>) -> PyResult<()> {
     // Initialize the pyo3-async-runtimes tokio runtime with a multi-thread
     // builder so async methods are dispatched onto a real thread pool rather
     // than a single-threaded executor. This must run before any `future_into_py`
-    // call — doing it here at module import time is the correct hook (KD8).
+    // call — doing it here at module import time is the correct hook.
     //
     // This is a separate runtime from cpex-ffi's SHARED_RUNTIME; the
     // shared-budget philosophy is mirrored, not the runtime instance.

bindings/python/src/manager.rs

@@ -3,7 +3,7 @@
 // SPDX-License-Identifier: Apache-2.0
 // Authors: Ted Habeck
 //
-// `PyPluginManager` — PyO3 wrapper around `cpex_core::PluginManager` (R1, R3, KD4).
+// `PyPluginManager` — PyO3 wrapper around `cpex_core::PluginManager`.
 //
 // Construction is synchronous; lifecycle methods (`initialize`, `shutdown`,
 // `invoke_hook`) are returned as Python awaitables via `future_into_py`.
@@ -14,7 +14,7 @@
 //   [GIL re-acq.] pipeline_result_to_py
 //
 // BackgroundTasks are dropped (not awaited per call); fire-and-forget tasks
-// run on the manager's TaskTracker and are drained by `shutdown()` (KD4).
+// run on the manager's TaskTracker and are drained by `shutdown()`.
 
 use std::sync::Arc;
 use std::time::Duration;
@@ -34,7 +34,7 @@ use crate::error::plugin_error_to_pyerr;
 use crate::result::pipeline_result_to_py;
 
 /// Wall-clock timeout for every async call through the PyO3 boundary.
-/// Mirrors `FFI_WALL_CLOCK_TIMEOUT` in cpex-ffi (KD7).
+/// Mirrors `FFI_WALL_CLOCK_TIMEOUT` in cpex-ffi.
 const PY_WALL_CLOCK_TIMEOUT: Duration = Duration::from_secs(60);
 
 #[pyclass(name = "PluginManager")]
@@ -95,7 +95,7 @@ impl PyPluginManager {
         })
     }
 
-    /// Shut down all registered plugins and drain fire-and-forget tasks (KD4).
+    /// Shut down all registered plugins and drain fire-and-forget tasks.
     ///
     /// Returns an awaitable (coroutine).
     fn shutdown<'py>(&self, py: Python<'py>) -> PyResult<Bound<'py, PyAny>> {
@@ -132,7 +132,7 @@ impl PyPluginManager {
     ///   `ValueError`     — payload/extensions/context conversion failure,
     ///                      or depth > 128.
     ///   `RuntimeError`   — plugin execution error or panic at the boundary.
-    ///   `TimeoutError`   — wall-clock timeout exceeded (KD7).
+    ///   `TimeoutError`   — wall-clock timeout exceeded.
     #[pyo3(signature = (hook_name, payload, extensions=None, context_table=None))]
     fn invoke_hook<'py>(
         &self,
@@ -142,7 +142,6 @@ impl PyPluginManager {
         extensions: Option<&Bound<'_, PyAny>>,
         context_table: Option<&Bound<'_, PyAny>>,
     ) -> PyResult<Bound<'py, PyAny>> {
-        // --- GIL held: convert all arguments ---
         let payload_value = pyobj_to_json_value(py, payload, 0)?;
         let rust_payload = resolve_payload(hook_name, payload_value)?;
 
@@ -161,7 +160,7 @@ impl PyPluginManager {
         let manager = Arc::clone(&self.inner);
         let hook_name = hook_name.to_string();
 
-        // --- GIL released: async execution with wall-clock timeout (KD7) ---
+        // GIL released: async execution with wall-clock timeout.
         // future_into_py catches panics via tokio's JoinHandle and converts them
         // to pyo3_async_runtimes.RustPanic (a PyException subclass). To keep the
         // documented interface consistent — the docstring promises RuntimeError —
@@ -183,10 +182,6 @@ impl PyPluginManager {
     }
 }
 
-// ---------------------------------------------------------------------------
-// Async helper — extracted so tests can inject a short timeout
-// ---------------------------------------------------------------------------
-
 /// Drives a single `invoke_by_name` call with an isolated tokio task (panic
 /// capture) and a wall-clock timeout, returning a Python-compatible result.
 ///
@@ -239,10 +234,6 @@ pub(crate) async fn invoke_with_timeout(
     }
 }
 
-// ---------------------------------------------------------------------------
-// Tests
-// ---------------------------------------------------------------------------
-//
 // These tests exercise the tokio::spawn + JoinError::is_panic() panic-catching
 // path without requiring a live Python interpreter. They run under plain
 // `cargo test -p cpex-python` (cdylibs produce a separate test binary that
@@ -251,7 +242,6 @@ pub(crate) async fn invoke_with_timeout(
 // The test mirrors the FFI crate's `cpex_invoke_returns_rc_panic_when_plugin_panics`
 // pattern: register a plugin that unconditionally panics, invoke it, verify the
 // panic is caught and surfaced as a JoinError rather than aborting the process.
-
 #[cfg(test)]
 mod tests {
     use std::sync::Arc;

bindings/python/src/result.rs

@@ -3,13 +3,13 @@
 // SPDX-License-Identifier: Apache-2.0
 // Authors: Ted Habeck
 //
-// `PyPipelineResult` — read-only Python view of `cpex_core::PipelineResult` (R1, R2, R3).
+// `PyPipelineResult` — read-only Python view of `cpex_core::PipelineResult`.
 //
 // Mirrors the field set of `PipelineResult` exactly. All fields are read-only
 // getters; no setters are exposed.
 //
 // If the modified_payload could not be serialised back to a Python dict the
-// caller appends a synthetic `PluginErrorRecord` to `errors` (R2, #8);
+// caller appends a synthetic `PluginErrorRecord` to `errors` (#8);
 // `modified_payload` is exposed as `None` in that case — mirrors the pattern
 // at `crates/cpex-ffi/src/lib.rs:877`.
 
@@ -132,15 +132,11 @@ impl PyPipelineResult {
     }
 }
 
-// ---------------------------------------------------------------------------
-// Builder
-// ---------------------------------------------------------------------------
-
 /// Convert a `PipelineResult` from the Rust runtime into `PyPipelineResult`.
 ///
 /// If `modified_payload` is present but cannot be serialised, a synthetic
 /// `PluginErrorRecord` is appended to `errors` and `modified_payload` is
-/// exposed as `None` — mirrors cpex-ffi's behaviour at lib.rs:877 (R2, #8).
+/// exposed as `None` — mirrors cpex-ffi's behaviour at lib.rs:877 (#8).
 pub fn pipeline_result_to_py(mut result: PipelineResult) -> PyResult<PyPipelineResult> {
     // Serialise modified_payload; on failure emit a synthetic error record.
     let modified_payload_value: Option<Value> = match result.modified_payload.take() {

builtins/pdps/cedar-direct/src/cedar_attrs.rs

@@ -44,7 +44,7 @@ pub const ATTR_TEAMS: &str = "teams";
 /// `apl-cmf`'s `claim.*` bag keys.
 pub const ATTR_CLAIMS: &str = "claims";
 
-// ----- JSON wrapping keys (Cedar's entity-from-JSON shape) ---------
+// JSON wrapping keys (Cedar's entity-from-JSON shape).
 //
 // These aren't entity attributes per se — they're the top-level
 // keys of the JSON shape Cedar expects when reading an entity from

builtins/pdps/cedar-direct/src/decision.rs

@@ -23,8 +23,7 @@
 //
 //   - Obligations — Cedar 4.10 doesn't have first-class obligations.
 //     Policy annotations could carry them (`@obligation(...)`) but
-//     wiring the annotation vocabulary is deferred — see
-//     `docs/specs/cedar-context-contract.md`.
+//     wiring the annotation vocabulary is deferred.
 //
 // # Fail-closed on evaluation errors
 //

builtins/pdps/cedar-direct/src/entities.rs

@@ -193,10 +193,6 @@ pub fn build_resource(
     })
 }
 
-// =====================================================================
-// Helpers
-// =====================================================================
-
 /// Apply the optional namespace to a bare entity type. `Some("Acme")` +
 /// `"User"` → `"Acme::User"`. `None` → `"User"`. Lets operators with
 /// namespaced schemas (`Acme::User`, `Acme::Document`) work without

builtins/pdps/cedar-direct/src/lib.rs

@@ -71,8 +71,7 @@
 //   - `context.security`   — `{ labels: [...], classification }`.
 //
 // Operators document this layout in their Cedar schema; policy authors
-// rely on it. See `docs/specs/cedar-context-contract.md` for the
-// authoritative shape.
+// rely on it.
 //
 // # Schema (optional)
 //

builtins/pdps/cedar-direct/src/request.rs

@@ -125,7 +125,7 @@ pub fn parse<'a>(
 /// Build the CPEX-provided context block (everything under
 /// `context.delegation`, `context.meta`, `context.security`) from the
 /// `AttributeBag`. Operators reason about these in Cedar policies via
-/// the well-known paths documented in `docs/specs/cedar-context-contract.md`.
+/// the well-known paths.
 fn build_cpex_context(bag: &AttributeBag) -> Value {
     let mut root = Map::new();
 

builtins/pdps/cedar-direct/src/resolver.rs

@@ -120,7 +120,6 @@ impl CedarDirectResolver {
             .as_mapping()
             .ok_or_else(|| BuildError::ConfigShape("Cedar PDP config must be a mapping".into()))?;
 
-        // ----- policy source -----
         let policy_text = read_yaml_string(map, "policy_text");
         let policy_file = read_yaml_string(map, "policy_file");
         let policies = match (policy_text, policy_file) {
@@ -141,7 +140,6 @@ impl CedarDirectResolver {
             .parse()
             .map_err(|e: cedar_policy::ParseErrors| BuildError::PolicyParse(e.to_string()))?;
 
-        // ----- optional schema -----
         let schema_text = read_yaml_string(map, "schema_text");
         let schema_file = read_yaml_string(map, "schema_file");
         let schema = match (schema_text, schema_file) {
@@ -157,7 +155,6 @@ impl CedarDirectResolver {
             (None, None) => None,
         };
 
-        // ----- optional dialect override -----
         let dialect = match read_yaml_string(map, "dialect").as_deref() {
             None | Some("cedar") => PdpDialect::Cedar,
             Some(other) => PdpDialect::Custom(other.to_string()),
@@ -256,10 +253,6 @@ impl PdpResolver for CedarDirectResolver {
     }
 }
 
-// =====================================================================
-// Helpers
-// =====================================================================
-
 fn parse_schema(text: &str) -> Result<Schema, BuildError> {
     Schema::from_cedarschema_str(text)
         .map(|(schema, _warnings)| schema)

builtins/pdps/cel/src/lib.rs

@@ -68,7 +68,7 @@
 // `false → Deny`. A non-boolean result, an undeclared-variable reference,
 // a compile error, or any other evaluation error is **fail-closed → Deny**
 // with the cause in `PdpDecision.diagnostics` (matches APL's PDP
-// fail-closed default; DSL §8.9). Operators can flip a *runtime* error
+// fail-closed default). Operators can flip a *runtime* error
 // (undeclared variable, type error, non-boolean) to allow-through via
 // `on_error: allow` in the PDP config block, but the default is `deny`.
 // Compile errors are never flippable — see `resolver::OnError`.

builtins/plugins/delegator-biscuit/src/lib.rs

@@ -23,8 +23,8 @@
 // Subsequent hops can each append further blocks. Completion blocks
 // (post-execution audit) are a future hook family.
 //
-// Sub-step A scope: module structure only. Real implementation in
-// sub-step B; integration tests in sub-step C.
+// Scope: module structure only. Real implementation and integration
+// tests land later.
 
 pub mod config;
 pub mod delegator;

builtins/plugins/delegator-oauth/src/lib.rs

@@ -16,9 +16,8 @@
 // `access_token` becomes the `RawDelegatedToken` the framework
 // stashes under `Extensions.raw_credentials.delegated_tokens`.
 //
-// Sub-step A scope: data shapes + module structure only. Actual
-// HTTP exchange logic in sub-step B; mock-IdP integration tests in
-// sub-step C.
+// Scope: data shapes + module structure only. Actual HTTP exchange
+// logic and mock-IdP integration tests land later.
 
 pub mod config;
 pub mod delegator;