diff --git a/.gitignore b/.gitignore index 04ab7951..062080b7 100644 --- a/.gitignore +++ b/.gitignore @@ -10,6 +10,9 @@ dist/ .aider* security-reports/ +# Transient phased-work checklist (see CLAUDE.md, Phase Completion Protocol). +/todo.md + # Generated from the marimo .py notebooks at docs build time. docs/docs/examples/**/*.ipynb __marimo__/ diff --git a/AGENTS.md b/AGENTS.md index 1b4468d3..5b8f388d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -31,9 +31,9 @@ ## Phase Completion Protocol -Phased work uses a `memory-bank/todo.md` checklist. This file is **transient**: it is -created for the duration of a phased task and removed when the work lands, so it is not -part of the Memory Bank core files below and is normally absent from the repository. +Phased work uses a `todo.md` checklist at the repository root. This file is **transient**: it +is created for the duration of a phased task and removed when the work lands, so it is +normally absent from the repository. When such a `todo.md` exists and contains phases: @@ -96,15 +96,10 @@ When filing a GitHub issue (via `gh issue create` or otherwise), follow the stru Issues that meet this bar are eligible for the `good first issue` label without further sharpening. -## Memory Bank +## Project Context -The `memory-bank/` directory contains project context documentation. Read both files at the start of tasks to understand the project. - -### Core Files -| File | Purpose | -|------|---------| -| `systemPatterns.md` | System architecture, design patterns, the PropertySpec lifecycle | -| `activeContext.md` | Current work focus, what works, next steps, known issues | - -### Hierarchy -`systemPatterns.md` holds the durable architecture; `activeContext.md` holds the current epic state and must be refreshed when an epic advances. +`docs/docs/` is the published documentation tree. Start a task by reading `docs/docs/index.md` and +`mloda/core/README.md` for the architecture, then the `in_depth/` page for the subsystem you are +touching (`in_depth/property-mapping.md` is the PropertySpec lifecycle). In-tree READMEs +(`mloda/core/`, `mloda_plugins/`, `tests/`) and `CONTRIBUTING.md` cover the rest. Current work state +comes from git history and the issue tracker, not from a checked-in status file. diff --git a/CLAUDE.md b/CLAUDE.md index 11c2f0e6..98ae07e0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -32,9 +32,9 @@ ## Phase Completion Protocol -Phased work uses a `memory-bank/todo.md` checklist. This file is **transient**: it is -created for the duration of a phased task and removed when the work lands, so it is not -part of the Memory Bank core files below and is normally absent from the repository. +Phased work uses a `todo.md` checklist at the repository root. This file is **transient**: it +is created for the duration of a phased task and removed when the work lands, so it is +normally absent from the repository. When such a `todo.md` exists and contains phases: @@ -103,15 +103,10 @@ When filing a GitHub issue (via `gh issue create` or otherwise), follow the stru Issues that meet this bar are eligible for the `good first issue` label without further sharpening. -## Memory Bank +## Project Context -The `memory-bank/` directory contains project context documentation. Read both files at the start of tasks to understand the project. - -### Core Files -| File | Purpose | -|------|---------| -| `systemPatterns.md` | System architecture, design patterns, the PropertySpec lifecycle | -| `activeContext.md` | Current work focus, what works, next steps, known issues | - -### Hierarchy -`systemPatterns.md` holds the durable architecture; `activeContext.md` holds the current epic state and must be refreshed when an epic advances. +`docs/docs/` is the published documentation tree. Start a task by reading `docs/docs/index.md` and +`mloda/core/README.md` for the architecture, then the `in_depth/` page for the subsystem you are +touching (`in_depth/property-mapping.md` is the PropertySpec lifecycle). In-tree READMEs +(`mloda/core/`, `mloda_plugins/`, `tests/`) and `CONTRIBUTING.md` cover the rest. Current work state +comes from git history and the issue tracker, not from a checked-in status file. diff --git a/docs/docs/in_depth/property-mapping.md b/docs/docs/in_depth/property-mapping.md index 23a70599..33d35134 100644 --- a/docs/docs/in_depth/property-mapping.md +++ b/docs/docs/in_depth/property-mapping.md @@ -76,7 +76,7 @@ does not understand can be absorbed silently. | Match time (parser) | `allowed_values` membership | Each element of a **present** option is in the accepted set | One element | `ValueError`, surfaced to the end user | | Match time (parser) | `element_validator` | Each element of a **present** option satisfies a predicate | One element | `ValueError`, surfaced to the end user | | Match time (parser) | Required presence (config path) | A key that declares no `default` and no `required_when` was provided | The options | Non-match (`False`) | -| Match time (parser) | Required presence (string-named path) | Same, after declared defaults and name bindings resolve; `deferred_binding=True` and the source (`in_features`) key are exempt | The effective options | Non-match (`False`), with a warning naming the missing key(s) | +| Match time (parser) | Required presence (string-named path) | Same, after declared defaults and name bindings resolve; `deferred_binding=True` and the source (`in_features`) key are exempt | The name-bound options | Non-match (`False`), with a warning naming the missing key(s) | | Match time (mixin) | `match_guard` | The whole value has an acceptable shape | The raw value | Non-match (`False`) | | Match time (mixin) | `MIN/MAX_IN_FEATURES` | In-feature count is within bounds | The in-features | Non-match (`False`) | | Match time (guard installed at class definition) | `required_when` | A conditionally required option is present | `Options` | Non-match (`False`) | @@ -374,6 +374,10 @@ The framework applies this at two sites. At feature **intake** directly. `options_with_defaults` remains for pre-materialization contexts (e.g. `resolve_subtype` internals) and for read sites the framework hands pre-default options, `input_features` above all. +For an engine-driven request intake has already run, so the compute-boundary call is an idempotent +no-op. It carries the work only for direct `FeatureSet` use that bypasses the engine, and there the +collapsing twins raise instead of merging. + Both sites run *after* resolution, so materialization never changes **matching**. It does change how features **group**: intake materialization deliberately canonicalizes default-equivalent twins, so a feature that passes a declared default explicitly and one that omits it become equal and merge @@ -385,6 +389,22 @@ declared default does NOT reach an `input_features` read site. A group that want `options_with_defaults` itself, as `ConcatenatedFileContent` does (see [A pattern-less feature group sits in between](#a-pattern-less-feature-group-sits-in-between)). +Which stage sees which view of the options: + +| Stage | Options view | +| --- | --- | +| Parse, bind, match, resolve (subtype resolution applies defaults internally) | declared (pre-default) | +| `input_features()` and child option inheritance | declared (pre-default) | +| Splitting, planning, filter matching, `validate_input_features`, compute | effective (post-default) | + +The last row assumes intake has run, which is every engine-driven request. On the direct `FeatureSet` +path the safety net materializes inside `run_calculate_feature`, which is after +`validate_input_features`, not before it. + +Filter matching is the one place the same classmethod sees both views: `GlobalFilter` calls +`match_feature_group_criteria` again after intake, so it observes effective options where feature +resolution saw declared ones. + ``` python graph_type = feature.options.get("graph_type") # the declared default when the caller omitted it ``` @@ -591,6 +611,7 @@ if it really is a whole-value check. | Declared defaults materialized into runtime options | `tests/test_core/test_abstract_plugins/test_options_with_defaults.py` | | Declared defaults materialized at the compute boundary | `tests/test_core/test_abstract_plugins/test_materialize_defaults_boundary.py` | | Declared defaults materialized at intake, canonicalizing default-equivalent twins | `tests/test_core/test_abstract_plugins/test_intake_default_canonicalization.py` | +| Filter criteria matching observes effective options | `tests/test_core/test_filter/test_filter_criteria_effective_options.py` | | The reader surface: the spec type and its class-definition guard, the reserved framework key, the MRO merge, the loud undeclared key, the presence rule of `reader_option()` | `tests/.../test_components/test_reader_option_declarations.py` | | Per-reader declarations, the `runtime_default` that is load-bearing at selection, and the bare-path branch it does not reach | `tests/.../input_data/test_reader_option_declarations.py` | | A pattern-less group: enforced `required_when`, and defaults it applies itself | `tests/.../input_data/test_read_context_files_option_declarations.py` | diff --git a/memory-bank/activeContext.md b/memory-bank/activeContext.md deleted file mode 100644 index 69cf11ae..00000000 --- a/memory-bank/activeContext.md +++ /dev/null @@ -1,31 +0,0 @@ -# Active Context - -## Current Work Focus - -The active initiative is the **PROPERTY_MAPPING / PropertySpec consumption hardening** epic (from the mloda#750 audit): make the typed option contract safe from authoring through compute, migrate downstream plugins, and retire transitional code. - -Core hardening is complete and the full `tox` gate is green (170 skipped, plus format, lint, licenses, strict mypy, and bandit). - -## Status - -- ✅ Typed `PropertySpec` is the sole `PROPERTY_MAPPING` contract; raw-dict specs are a hard break. -- ✅ Defaults are materialized at feature intake (framework-enforced), with opt-in explicit-`None` semantics; default-equivalent twin requests canonicalize during planning, and the compute boundary remains an idempotent safety net (os-008). -- ✅ Structured name parsing and explicit capture-to-spec binding replace reverse lookup and fabricated captureless tokens. -- ✅ Required-presence enforcement on the string-named match path; all-optional universal-matcher guard. -- ✅ Resolution failures carry per-candidate elimination facts (`EvaluationResult`). -- ✅ Post-hardening cleanup: retired transitional parser seams (os-005) and consolidated the `PROPERTY_MAPPING` test suite around a public behavior matrix (os-006). - -## What Works - -- **Compute frameworks (9)**: PythonDict (dependency-free) and SQLite (PyArrow only); optional extras cover Pandas, PyArrow, Polars (eager and lazy), DuckDB, Iceberg, Spark. DuckDB and SQLite share a common SQL base. -- **Feature groups**: core transforms (Aggregated, TimeWindow, MissingValue), analytics (Clustering, DimensionalityReduction, Forecasting, NodeCentrality), sklearn family (Pipeline, Encoding, Scaling), processing (TextCleaning, GeoDistance), LLM feature groups, and infrastructure (environment introspection, dynamic factory, source-input composition, input-data reader suite). -- **Extenders**: OtelExtender (OpenTelemetry). - -## Next Steps / Known Issues - -- **Phase 6 downstream** (owning repos): migrate mloda-registry raw mappings and captureless patterns, align declared value spaces with runtime behavior, scaffold the plugin-template example, and refresh mloda.ai samples before raising the registry `>=0.10,<0.11` cap. -- **Local FeatureGroup subclass GC race** (mloda#868): shared-fixture test parametrization is deferred until this flake is resolved. - -## Architecture Snapshot - -The typed option lifecycle (author -> parse -> bind -> match -> resolve -> materialize defaults -> compute) is documented in `systemPatterns.md`. diff --git a/memory-bank/systemPatterns.md b/memory-bank/systemPatterns.md deleted file mode 100644 index 1006fa2e..00000000 --- a/memory-bank/systemPatterns.md +++ /dev/null @@ -1,70 +0,0 @@ -# System Patterns - -## Architecture Overview - -```mermaid -graph TB - subgraph Core - CE[Core Engine] - end - - subgraph Plugins - FG[Feature Groups] - CF[Compute Frameworks] - EX[Extenders] - end - - FG --> |Dependencies| CE - CF --> |Execution| CE - EX --> |Metadata| CE - CE --> |Orchestrates| Output[Transformed Features] - - style CE fill:#f9f,stroke:#333,stroke-width:3px - style Output fill:#dfd,stroke:#333,stroke-width:2px -``` - -## Key Design Decisions - -- **Transformations over static states**: define how data changes, not fixed outputs. -- **Plugin-based architecture**: Feature Groups, Compute Frameworks, Extenders; the engine selects plugins automatically. -- **Decoupled execution**: features are independent of the compute technology. -- **Dependency-free core**: the `mloda` core declares no runtime dependencies; the PythonDict backend also needs none, while every other backend is an optional extra. - -## PropertySpec lifecycle - -A FeatureGroup declares its configuration surface as `PROPERTY_MAPPING: dict[str, PropertySpec]`. The typed, frozen `PropertySpec` is the single contract; raw-dict specs are a hard error. Each option key flows through the stages below, from authoring to compute. Types live in `mloda/core/abstract_plugins/components/feature_chainer/`. - -```mermaid -flowchart LR - A[Author PropertySpec] --> P[Parse name] - P --> B[Bind captures] - B --> M[Match] - M --> R[Resolve] - R --> D[Materialize defaults] - D --> C[Compute] -``` - -1. **Author** (`property_spec.py`): `property_spec()` (or `PropertySpec(...)`) builds a frozen spec whose `__post_init__` enforces every invariant. `NO_DEFAULT` marks a key required; `is_no_default()` tests for it by type, not identity, so a reimported module copy still reads correctly. `PropertySpec`, `property_spec`, `NO_DEFAULT`, `is_no_default`, `is_positive_int` are all exported from `mloda.provider`. -2. **Parse**: `FeatureChainParser.parse_name` (`feature_chain_parser.py`) returns a frozen `ParsedFeatureName` (`parsed_feature_name.py`) recording exactly what `re` found. `operation_part` is the raw suffix, never a fabricated token. -3. **Bind** (`feature_chain_parser.py`): `bind_name_captures` binds named captures exclusively by name into an effective `Options`; a captureless pattern is a recognition predicate that identifies the group and binds nothing. A present option value always wins over a name-derived one. A transitional positional fallback (`_legacy_operation_config`) still reverse-looks-up single-capture legacy patterns, pending downstream migration (mloda-registry#327). -4. **Match**: `match_configuration_feature_chain_parser` validates present values (a bad value raises `PropertyValueRejection`, a `ValueError` verdict rather than a crash) and enforces required presence on the string-named path. -5. **Resolve** (`prepare/identify_feature_group.py`): the engine and the resolution test seam both enter through `resolve_or_raise` (the shared evaluate-render-raise helper, built on `evaluate_and_render`), under which `IdentifyFeatureGroupClass.evaluate` remains the non-raising matcher: it runs one resolution pass, recording per-candidate elimination facts (`EvaluationResult`, `Elimination`) so a failure explains which gate each near-miss failed. That surface spans three modules in import order, `prepare/resolution_types.py` (the captured facts), `prepare/resolution_failure_renderer.py` (message rendering), then `prepare/identify_feature_group.py` (the matcher), each importing only the earlier ones, and the moved names stay importable from `identify_feature_group`. Class-definition-time checks reject order-dependent bindings and install guards enforcing `required_when` and name-path required presence; an all-optional matcher that would match any name emits a definition-time warning unless the class sets `ALLOW_UNIVERSAL_MATCHER`. -6. **Materialize defaults at intake, then compute** (os-008): `Engine.add_feature_to_collection` rebinds each resolved feature's options through `options_with_defaults()` as it enters the plan, so default-equivalent twins (same name, one key explicitly set to its declared default) merge through the standard duplicate path with uuid remapping. `ComputeFramework.run_calculate_feature` (`@final`) still calls `FeatureSet.materialize_option_defaults` as an idempotent safety net for direct API use; its twin-collapse ValueError is unreachable from engine-driven requests. `options_with_defaults()` fills only absent keys that declare a concrete default; `NO_DEFAULT` and a declared `None` fill nothing. Presence honors the explicit-`None` policy: a present `None` counts as set only when the spec sets `allow_explicit_none=True`. - -Which lifecycle stages observe which options view: - -| Stage | Options view | -|-------|--------------| -| Parse, bind, match, resolve (candidate selection; subtype resolution applies defaults internally) | declared (pre-default) | -| `input_features()` and child option inheritance | declared (pre-default) | -| Splitting, planning, validators, filters, compute | effective (post-default) | - -## Component Roles - -```mermaid -flowchart LR - FG[Feature Groups] -->|Define| Trans[Transformations] - CF[Compute Frameworks] -->|Execute| Trans - EX[Extenders] -->|Extract| Meta[Metadata] - CE[Core Engine] -->|Orchestrate| All[All Components] -```