feat(named-function): SoA metadata store migration#848
Conversation
Move name/subscripts/parameters/description off NamedFunction, EvaluatedNamedFunction, and SampledNamedFunction onto a new NamedFunctionMetadataStore SoA store, kept as a sibling field on Instance / ParametricInstance / Solution / SampleSet. This is the NamedFunction sibling of the constraint/variable migration that landed in PR #843, with the same snapshot-model PyO3 wrappers (NamedFunction(pub ommx::NamedFunction, pub ommx::NamedFunctionMetadata)) and the same WithMetadata-based pandas rendering. Highlights: - Parse boundary moves to per-collection: Vec<v1::NamedFunction> parses to (BTreeMap, NamedFunctionMetadataStore); per-element helpers (named_function_to_v1 etc.) reattach metadata at emit. - Instance::evaluate / evaluate_samples thread the host store onto the produced Solution / SampleSet so metadata reads through the same channel as decision-variable and constraint metadata. - Element-level to_bytes/from_bytes on NamedFunction et al. removed; same rationale as PR #845 (per-element wire round-trip can no longer carry host-stored metadata). - METADATA_STORAGE_V3.md flipped from "deferred — separate PR" to "landed (this PR)". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…AGE_V3 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR completes the v3 metadata SoA migration for NamedFunction by moving name/subscripts/parameters/description off per-element structs and into a sibling, collection-level NamedFunctionMetadataStore carried by Instance/ParametricInstance/Solution/SampleSet, with corresponding Rust parse/serialize and Python snapshot-wrapper updates.
Changes:
- Introduces
NamedFunctionMetadata+NamedFunctionMetadataStore(SoA) and updates named-function parse/emit to drain/reattach metadata at collection boundaries. - Threads named-function metadata through evaluation and container conversions (
Instance::evaluate*,SampleSet::get,From/ParseforInstance/ParametricInstance/Solution/SampleSet). - Updates PyO3 wrappers + pandas rendering to snapshot and render named-function metadata, and removes per-element bytes round-trips for named-function element types.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| rust/ommx/src/solution/parse.rs | Parses/drains evaluated named-function metadata into Solution; reattaches on v1 emit. |
| rust/ommx/src/solution.rs | Adds Solution.named_function_metadata and updates extraction helpers to read from the store. |
| rust/ommx/src/sample_set/parse.rs | Parses/drains sampled named-function metadata into SampleSet; reattaches on v1 emit. |
| rust/ommx/src/sample_set/extract.rs | Updates SampleSet extraction utilities + tests to use the metadata store. |
| rust/ommx/src/sample_set.rs | Adds SampleSet.named_function_metadata and propagates it into per-sample Solution results. |
| rust/ommx/src/named_function/serialize.rs | Removes per-element to_bytes/from_bytes APIs for named-function element types. |
| rust/ommx/src/named_function/parse.rs | Changes per-element parse outputs to include drained metadata; adds explicit *_to_v1 helpers. |
| rust/ommx/src/named_function/metadata_store.rs | Implements the new NamedFunctionMetadataStore SoA type and its API. |
| rust/ommx/src/named_function/evaluate.rs | Removes metadata propagation from element-level evaluate outputs (now host-owned). |
| rust/ommx/src/named_function/arbitrary.rs | Updates Arbitrary generation for the new NamedFunction shape (no inline metadata). |
| rust/ommx/src/named_function.rs | Removes inline metadata fields, adds NamedFunctionMetadata, exports the store, updates Display. |
| rust/ommx/src/instance/penalty.rs | Preserves named_function_metadata when building derived instances. |
| rust/ommx/src/instance/parse.rs | Parses/drains named-function metadata for Instance/ParametricInstance; reattaches on v1 emit; adds regression test. |
| rust/ommx/src/instance/parametric_builder.rs | Initializes named_function_metadata for parametric builds; updates tests for new struct shape. |
| rust/ommx/src/instance/named_function.rs | Routes Instance named-function queries through the metadata store; updates tests. |
| rust/ommx/src/instance/logical_memory.rs | Updates logical memory snapshots to include the new store fields. |
| rust/ommx/src/instance/evaluate.rs | Threads named-function metadata into produced Solution/SampleSet. |
| rust/ommx/src/instance/convert.rs | Preserves named_function_metadata through Instance ↔ ParametricInstance conversions. |
| rust/ommx/src/instance/builder.rs | Initializes named_function_metadata in InstanceBuilder; updates tests for new struct shape. |
| rust/ommx/src/instance/arbitrary.rs | Initializes named_function_metadata in arbitrary Instance generation. |
| rust/ommx/src/instance.rs | Adds named_function_metadata fields + accessors on Instance and ParametricInstance. |
| python/ommx/src/solution.rs | Wraps evaluated named functions with metadata snapshots; updates named-functions DataFrame rendering. |
| python/ommx/src/sampled_named_function.rs | Updates wrapper to carry (inner, metadata) snapshot and serve metadata getters from it. |
| python/ommx/src/sample_set.rs | Wraps sampled named functions with metadata snapshots; updates named-functions DataFrame rendering. |
| python/ommx/src/parametric_instance.rs | Drains named-function wrapper metadata snapshots into the host store; updates accessors/dfs. |
| python/ommx/src/pandas.rs | Switches named-function ToPandasEntry impls to WithMetadata<..., NamedFunctionMetadata>. |
| python/ommx/src/named_function.rs | Updates NamedFunction wrapper to snapshot metadata; carries snapshot across per-element evaluate. |
| python/ommx/src/instance.rs | Drains named-function wrapper metadata snapshots into the host store; updates accessors/dfs. |
| python/ommx/src/evaluated_named_function.rs | Updates wrapper to carry (inner, metadata) snapshot and serve metadata getters from it. |
| python/ommx/ommx/_ommx_rust/init.pyi | Regenerates stubs to reflect updated named-function wrapper docs. |
| docs/api/api_reference.json | Updates generated API reference docs for the new wrapper docstrings. |
| METADATA_STORAGE_V3.md | Marks NamedFunction SoA migration as landed and updates plan sections accordingly. |
…on metadata Addresses Copilot review on PR #848: the existing test_solution_roundtrip_preserves_metadata and test_sample_set_roundtrip_preserves_metadata only asserted variable and constraint metadata. Extend each to include an evaluated / sampled named function with non-empty metadata and assert the recovered named_function_metadata values. Construction goes through the v1 parse helper because EvaluatedNamedFunction::used_decision_variable_ids and SampledNamedFunction's fields are module-private — same path Instance::evaluate / evaluate_samples uses internally. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Migrates NamedFunction (and evaluated/sampled variants) metadata storage in the Rust + Python SDKs from per-element AoS fields to a per-collection Struct-of-Arrays NamedFunctionMetadataStore, aligning named functions with the existing v3 metadata-store approach used for variables and constraints.
Changes:
- Introduces
NamedFunctionMetadataStoreand movesname/subscripts/parameters/descriptionoffNamedFunction/EvaluatedNamedFunction/SampledNamedFunction. - Updates v1 parse/serialize boundaries for
Instance/ParametricInstance/Solution/SampleSetto drain/reattach named-function metadata via helper conversions. - Threads named-function metadata through
evaluatepaths and updates PyO3 snapshot wrappers + pandas rendering to read from the host store.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| rust/ommx/src/solution/parse.rs | Parse/serialize Solution evaluated named functions by draining/reattaching metadata via the new store; adds regression coverage. |
| rust/ommx/src/solution.rs | Adds named_function_metadata to Solution and updates extraction/name helpers to read from the store. |
| rust/ommx/src/sample_set/parse.rs | Parse/serialize SampleSet named functions with drained/reattached metadata; adds regression coverage. |
| rust/ommx/src/sample_set/extract.rs | Updates named-function extraction helpers to read metadata from SampleSet’s store; adjusts tests to insert drained metadata. |
| rust/ommx/src/sample_set.rs | Adds named_function_metadata to SampleSet and threads it into per-sample Solution creation. |
| rust/ommx/src/named_function/serialize.rs | Removes per-element bytes round-trip APIs for named function types. |
| rust/ommx/src/named_function/parse.rs | Refactors parse layer to return (element, metadata) pairs and provides helper emitters that reattach metadata. |
| rust/ommx/src/named_function/metadata_store.rs | New SoA metadata store implementation for named functions (sparse per-field maps + neutral getters). |
| rust/ommx/src/named_function/evaluate.rs | Removes metadata propagation from per-element evaluate since metadata is now host-stored. |
| rust/ommx/src/named_function/arbitrary.rs | Updates proptest arbitrary generation for the slimmer NamedFunction struct. |
| rust/ommx/src/named_function.rs | Removes inline metadata fields from named-function structs; introduces NamedFunctionMetadata and exports the store. |
| rust/ommx/src/instance/penalty.rs | Ensures named-function metadata store is preserved across Instance penalty transformations. |
| rust/ommx/src/instance/parse.rs | Updates Instance/ParametricInstance parse/serialize to drain/reattach named-function metadata; adds regression test. |
| rust/ommx/src/instance/parametric_builder.rs | Initializes named_function_metadata for parametric instance builds; updates tests for new struct shape. |
| rust/ommx/src/instance/named_function.rs | Updates named-function lookup/creation helpers to write/read metadata via the store; updates tests. |
| rust/ommx/src/instance/logical_memory.rs | Extends logical memory snapshots to include the named-function metadata store fields. |
| rust/ommx/src/instance/evaluate.rs | Threads named_function_metadata through Instance::evaluate and evaluate_samples. |
| rust/ommx/src/instance/convert.rs | Preserves named-function metadata in Instance ↔ ParametricInstance conversions. |
| rust/ommx/src/instance/builder.rs | Initializes named_function_metadata in InstanceBuilder; updates tests for new struct shape. |
| rust/ommx/src/instance/arbitrary.rs | Initializes named_function_metadata for arbitrary Instance generation. |
| rust/ommx/src/instance.rs | Adds named_function_metadata field plus accessors on Instance and ParametricInstance. |
| python/ommx/src/solution.rs | Python Solution wrapper snapshots named-function metadata from the host store and uses it in accessors/DF rendering. |
| python/ommx/src/sampled_named_function.rs | Updates SampledNamedFunction Python wrapper to carry an owned metadata snapshot. |
| python/ommx/src/sample_set.rs | Python SampleSet wrapper snapshots named-function metadata and uses it in accessors/DF rendering. |
| python/ommx/src/parametric_instance.rs | Python ParametricInstance wrapper drains wrapper metadata snapshots into the host store; updates accessors/DF rendering. |
| python/ommx/src/pandas.rs | Switches named-function pandas rendering to WithMetadata so metadata comes from the host store snapshot. |
| python/ommx/src/named_function.rs | Updates Python NamedFunction wrapper to carry metadata snapshot and propagate it through per-element evaluate. |
| python/ommx/src/instance.rs | Python Instance wrapper drains named-function metadata snapshots into the host store; updates accessors/DF rendering. |
| python/ommx/src/evaluated_named_function.rs | Updates EvaluatedNamedFunction Python wrapper to carry an owned metadata snapshot. |
| python/ommx/ommx/_ommx_rust/init.pyi | Regenerates stubs to reflect updated named-function wrapper docs/shape. |
| docs/api/api_reference.json | Updates API reference docs for the modified Python wrapper docstrings. |
| METADATA_STORAGE_V3.md | Marks NamedFunction SoA migration as landed and updates the plan (Series accessors dropped, attached wrappers still deferred). |
The lead bullet in the §3.0 overview said "metadata moves off each constraint", but the SoA refactor moved metadata off decision variables (#843) and named functions (#848) too. Bullet now mentions constraint / decision variable / named function explicitly, lists all three per-host accessors, and clarifies that `provenance` is constraint-only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
BLOCKING:
- Per-element to_bytes / from_bytes paragraph: rewritten. The previous
text claimed Constraint / EvaluatedConstraint / SampledConstraint
"still expose to_bytes/from_bytes that take an explicit ConstraintID"
— these methods do not exist (rg "fn (to_bytes|from_bytes)" in
rust/ommx/src returns only the host types and Function). The
paragraph now says per-element to_bytes/from_bytes are no longer
provided on any constraint kind, motivated by id + metadata living
on the host.
- "Unified error surface" paragraph: softened from "the public API now
returns a single error type" to "the default error type is
ommx::Result<T>". The signal types (BoundError,
DecisionVariableError, DuplicatedSampleIDError, SubstitutionError,
...) are surfaced typed at specific public APIs (Bound::new,
DecisionVariable::new / set_bound / substitute, Sampled::append, the
Substitute trait family, ...) — they are not internal-only signals
reachable solely via downcast. The list now spells out where each is
returned typed.
NICE-TO-FIX:
- "PropagateOutcome records a Provenance chain" was wrong. The enum
has Active(T) / Consumed(T) / Transformed { original, new } — no
Provenance field. Provenance is appended by callers writing into
ConstraintMetadataStore. Bullet rewritten to describe the actual
variants and the caller-side provenance step.
- Metadata-storage section over-attributed Python-only PRs as Rust
evidence. Rust-side bundle trimmed to (#843, #848, #850, #853);
#846, #847, #849, #852 moved into per-bullet citations on the
Python-side paragraph.
- Tracing observability paragraph softened: it now acknowledges that
legacy `.context(...)` call sites in narrow-domain parsers and the
artifact layer remain, while new internal fail sites use the crate
fail-site macros.
Other minor:
- "Serialization moves to the host level" wording (was "collection
level") and ParametricInstance bullet mentions the sibling
VariableMetadataStore / NamedFunctionMetadataStore.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Summary Documentation hygiene pass for v3: 1. **Drop `METADATA_STORAGE_V3.md`** — design doc whose waves all landed. 2. **Consolidate Rust release note** into one file (`rust/ommx/doc/release_note.md`) covering the 3.0 line. Drop the per-alpha submodule (`crate::doc::release_note::v3_0_0_alpha_1` removed; `crate::doc::release_note` stays). 3. **Reframe the release note as topic-oriented "what & why"** — the API enumerations live in the Rust migration guide and the error handling tutorial; the release note links to them. Net 387 → 288 lines. 4. **Fix stale `metadata: ConstraintMetadata` references** in the Rust migration guide (those pre-dated the SoA refactor). 5. **Tag every section** of the Python migration guide with `(3.0.0aN, [#PR](url))` annotations and add three new sections covering 3.0.0a3 changes (DataFrame API, sidecar dfs, `AttachedX`). Rust and Python SDKs are versioned independently, so Rust release-note PR refs are bare `[#XXX](url)` and Python-guide tags use Python alpha labels (`3.0.0a1` / `3.0.0a2` / `3.0.0a3`). ## Rust release note (`rust/ommx/doc/release_note.md`) Topic-oriented v3 summary. Detail-heavy parts now defer to the Rust migration guide / tutorial: - `## 3.0` overview bullets — first-class special constraints, Stage / `ConstraintType` collapse, host-level serialization, per-collection SoA metadata stores, capability model, default `ommx::Result<T>` error surface, `v1_ext` removal. - `## Stage parameter and the ConstraintType trait` — motivation only; struct / trait code blocks moved to migration guide §1 and §New Types. - `## Collections and serialization` — `ConstraintCollection<T>` / `EvaluatedCollection<T>` / `SampledCollection<T>` rationale + host-level `to_bytes` / `from_bytes`. Migration guide owns the method lists. - `## Metadata storage: SoA store on the enclosing collection` — three store families (`ConstraintMetadataStore<ID>`, `VariableMetadataStore`, `NamedFunctionMetadataStore`), the `pub(crate)` mutator tightening, and a Python paragraph linking out to `PYTHON_SDK_MIGRATION_GUIDE.md` §9–11 with PR attributions for `AttachedX` (#849, #850, #852) and `*_df` (#846, #847). Rust-side bundle is (#843, #848, #850, #853). - `## Capability model` — `AdditionalCapability` / `Capabilities` / `required_capabilities()` / `reduce_capabilities(&supported)`. - `## Unified error surface` — default `ommx::Result<T>`, fail-site macros (`bail!` / `error!` / `ensure!`), narrow-domain parser exceptions, signal-type representative list. Full catalog deferred to the error handling tutorial. - `## Tracing-first observability` — acknowledges legacy `.context(...)` call sites still exist in `qplib/parser` and `artifact`. - `## Domain types replace v1_ext`, `## Other notable changes` — unchanged. ## Rust migration guide (`rust/ommx/doc/migration_guide.md`) - Remove stale `metadata: ConstraintMetadata` field references on `Constraint<S>` and the struct-construction examples. - New `### Metadata stores` section: per-host accessors on `Instance` / `ParametricInstance` (`constraint_metadata()` / `_mut()`, indicator / one-hot / sos1 variants, `variable_metadata()`, `named_function_metadata()`); how `Solution` / `SampleSet` reach constraint metadata via `evaluated_constraints().metadata()` etc.; the `ConstraintMetadataStore<ID>` API (per-field reads, `collect_for(id)`, write-through setters, bulk `insert` / `remove`); plus the `VariableMetadataStore` / `NamedFunctionMetadataStore` shape differences (no provenance; `NamedFunctionMetadataStore` also lacks `push_subscript` / `extend_subscripts`). - Add `metadata` field to `ConstraintCollection` / `EvaluatedCollection` / `SampledCollection` shape blocks. - Migration-checklist entry for `constraint.name` updated to point at the SoA accessors. ## Python migration guide (`PYTHON_SDK_MIGRATION_GUIDE.md`) - §6.6 wording: every `*_df` is a method. - New §9 — DataFrame API: methods, `kind=` / `include=` / `removed=` consolidating the per-kind / active-vs-removed family. `include=` defaults to `("metadata", "parameters")`; `"removed_reason"` opt-in (auto-enabled with `removed=True` on Instance / ParametricInstance). - New §10 — Long-format sidecar DataFrames (`constraint_metadata_df`, `constraint_provenance_df`, `variable_parameters_df`, …). - New §11 — `instance.constraints[id]` etc. now return `AttachedX` write-through handles. Three-column affected-types table (v2.5.1 / 3.0.0a2 / v3 final) so each hop is visible. `add_*` entry points and `attached.detach()`. - Convenience additions renumbered §9 → §12. - Per-section `(3.0.0aN, [#PR](url))` tag on every section in §1–§11; Overview themes (5, 6) and migration checklist extended. ## Test plan - [ ] `cargo doc -p ommx --no-deps` renders the release note and migration guide cleanly (verified locally). - [ ] Spot-check rendered `ommx::doc::release_note` and `ommx::doc::migration_guide` on the local `target/doc` build for cross-reference resolution and section ordering. - [ ] Spot-check rendered Python migration guide on GitHub: section numbering 1–12, all alpha+PR tags resolve. - [ ] No other doc / source file references `METADATA_STORAGE_V3.md` or `release_note::v3_0_0_alpha_1` (verified locally). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
name/subscripts/parameters/descriptionoffNamedFunction,EvaluatedNamedFunction,SampledNamedFunctiononto a newNamedFunctionMetadataStoreSoA store, kept as a sibling field onInstance/ParametricInstance/Solution/SampleSet. Same snapshot-model PyO3 wrappers andWithMetadata-based pandas rendering as PR feat(metadata-soa): move per-element metadata onto collection-level SoA stores #843 (which did this forConstraint/DecisionVariable).Vec<v1::NamedFunction>::Parsereturns(BTreeMap, NamedFunctionMetadataStore); per-element helpers (named_function_to_v1,evaluated_named_function_to_v1,sampled_named_function_to_v1) reattach metadata at emit.Instance::evaluate/evaluate_samplesthread the host store onto the producedSolution/SampleSet.SampleSet::getcarries the store onto each per-sampleSolution.to_bytes/from_bytesremoved fromNamedFunction/EvaluatedNamedFunction/SampledNamedFunction— same rationale as Remove to_bytes / from_bytes from non-top-level Python wrappers #845: a per-element wire round-trip can no longer carry host-stored metadata, so the methods became misleading rather than safe.End state
After this PR the v3 SoA metadata story is uniform across all four metadata-bearing element types (decision variables, regular / indicator / one-hot / sos1 constraints, named functions). The only remaining deferred item in METADATA_STORAGE_V3.md is the two-mode Standalone / Attached wrapper design for write-through metadata mutation.
Test plan