Skip to content

feat(metadata-soa): move per-element metadata onto collection-level SoA stores#843

Merged
termoshtt merged 22 commits intomainfrom
feat/metadata-soa-rust
Apr 28, 2026
Merged

feat(metadata-soa): move per-element metadata onto collection-level SoA stores#843
termoshtt merged 22 commits intomainfrom
feat/metadata-soa-rust

Conversation

@termoshtt
Copy link
Copy Markdown
Member

@termoshtt termoshtt commented Apr 27, 2026

Summary

Moves per-constraint and per-variable auxiliary metadata (name, subscripts, parameters, description, provenance) off per-element structs and onto Struct-of-Arrays stores at the collection / Instance layer (METADATA_STORAGE_V3.md). The Rust SDK and the PyO3 bindings are both fully migrated; cargo test and task python:test pass end-to-end.

This PR is the runtime / Python-API prerequisite for #841 (special-constraint proto v3). The proto-schema decision in #841ConstraintMetadata inline per message vs. top-level columnar map — needed the runtime / Python-API direction settled first; that's what this PR establishes.

End state

Rust SDK

  • New ConstraintMetadataStore<ID> (generic over the four constraint ID types: regular / indicator / one-hot / SOS1) and VariableMetadataStore under rust/ommx/src/{constraint,decision_variable}/metadata_store.rs. Sparse FnvHashMap-per-field representation with per-field borrowing getters (name(id) -> Option<&str>, subscripts(id) -> &[i64], parameters(id) -> &FnvHashMap<…>, …) and a shared empty-sentinel for the collection-shaped fields. Owned exchange via insert(id, ConstraintMetadata) / remove(id) / collect_for(id).
  • ConstraintCollection<T> / EvaluatedCollection<T> / SampledCollection<T> each carry a metadata field with metadata() / metadata_mut() accessors and with_metadata(...) constructors. insert_with(id, c, metadata) performs atomic insert + metadata write.
  • Instance and ParametricInstance gain a variable_metadata field with the same accessor pattern, plus narrow per-kind metadata accessors (variable_metadata_mut(), constraint_metadata_mut(), indicator_constraint_metadata_mut(), one_hot_constraint_metadata_mut(), sos1_constraint_metadata_mut() and their immutable siblings) so callers can drain metadata into the SoA stores after builder().build() without exposing invariant-breaking access to the underlying collections.
  • The per-element metadata: ConstraintMetadata field is removed from Constraint<S> / IndicatorConstraint<S> / OneHotConstraint<S> / Sos1Constraint<S> / DecisionVariable / EvaluatedDecisionVariable / SampledDecisionVariable.
  • Parse / serialize boundary moves to the collection layer. Parse impls now produce (map, ConstraintMetadataStore) / (map, VariableMetadataStore) pairs at the collection level; the collection-level serializers (From<Instance> for v1::Instance, From<ParametricInstance> for v1::ParametricInstance, From<Solution> for v1::Solution, From<SampleSet> for v1::SampleSet) drain the SoA stores and overlay metadata onto each per-element proto via the explicit *_to_v1 helpers.
  • The default-metadata From<X> for v1::Y impls (impl From<DecisionVariable> for v1::DecisionVariable and the Evaluated / Sampled / Constraint siblings) are removed entirely. Per-element conversion would have to default every metadata field, which silently drops any caller-supplied metadata; making the helpers pub(crate) and require an explicit metadata: ConstraintMetadata | DecisionVariableMetadata argument forces the silent path to surface as a type error.
  • Bare-element bytes round-trips (DecisionVariable::to_bytes / from_bytes and the Constraint / EvaluatedConstraint / SampledConstraint / EvaluatedDecisionVariable / SampledDecisionVariable siblings) are removed. Top-level container to_bytes / from_bytes (Instance, ParametricInstance, Solution, SampleSet, plus the DTOs State, Samples, Parameters) preserve full metadata.
  • Evaluate path threads metadata through: Instance::evaluate and Instance::evaluate_samples clone variable_metadata into the produced Solution / SampleSet; SampleSet::get carries it forward into the per-sample Solution along with all four constraint-kind metadata stores. Constraint-side metadata flows through ConstraintCollection::evaluate into EvaluatedCollection / SampledCollection automatically.
  • Mutation sites in instance/{slack,sos1,one_hot,log_encode,indicator,evaluate}.rs write through variable_metadata_mut() and the per-kind *_metadata_mut() accessors. Special-constraint promotion paths (one-hot → constraint, indicator → constraint, SOS1 → constraint) carry metadata via insert_with and push_provenance.
  • MPS / QPLIB readers populate the SoA stores after Instance::new, replacing the previous per-element metadata writes.
  • Memory profile snapshots updated to account for the collection-level visits.

PyO3 wrappers (snapshot model)

  • Each wrapper struct now holds its own metadata snapshot:
    Constraint(pub ommx::Constraint, pub ommx::ConstraintMetadata)
    DecisionVariable(pub ommx::DecisionVariable, pub ommx::DecisionVariableMetadata)
    IndicatorConstraint(...), EvaluatedConstraint(...), SampledConstraint(...),
    EvaluatedDecisionVariable(...), SampledDecisionVariable(...)
    Standalone construction starts with Default::default() metadata. Reading from an Instance fills the snapshot from the SoA store via from_parts(inner, metadata.collect_for(id)). Instance.from_components(...) and ParametricInstance.from_components(...) drain each wrapper's metadata back into the instance's SoA stores. Mutations on a wrapper retrieved from an instance therefore do not propagate back; the caller must re-add the constraint / variable to apply changes (matches the prior clone()-based semantics).
  • pandas.rs introduces a WithMetadata<'a, T, M> wrapper. ToPandasEntry impls that previously read self.metadata.X now consume WithMetadata<'_, T, ConstraintMetadata | DecisionVariableMetadata>; call sites in Instance / ParametricInstance / Solution / SampleSet pre-snapshot the SoA store and zip the metadata in alongside each item before handing the iterator to entries_to_dataframe.
  • from_bytes / to_bytes removed from non-top-level Python wrappers (Linear, Quadratic, Polynomial, Function, DecisionVariable, EvaluatedDecisionVariable, SampledDecisionVariable, NamedFunction, EvaluatedNamedFunction, SampledNamedFunction, Parameter). Only the top-level types (Instance, ParametricInstance, Solution, SampleSet) plus the cross-evaluate DTOs (State, Samples, Parameters) keep them. __init__.pyi regenerated via task python:stubgen to match.

The originally-proposed Standalone / Attached two-mode design with Py<Instance> back-references (write-through getters, live shared state across wrappers pointing at the same id) is intentionally not implemented in this PR. The snapshot model preserves the v2 semantics with minimum surface change; the two-mode design lands together with the Series / include= work in the next wave.

Public API surface

The ommx crate's public surface matches main plus the new SoA accessors required by this refactor:

  • pub struct ConstraintMetadataStore<ID> and pub struct VariableMetadataStore (returned by the metadata accessors below).
  • pub fn variable_metadata() / variable_metadata_mut() / constraint_collection() / constraint_metadata() / constraint_metadata_mut() / indicator_constraint_collection() / indicator_constraint_metadata() / indicator_constraint_metadata_mut() / one_hot_constraint_collection() / one_hot_constraint_metadata() / one_hot_constraint_metadata_mut() / sos1_constraint_collection() / sos1_constraint_metadata() / sos1_constraint_metadata_mut() on Instance and ParametricInstance. The mutable surface is intentionally narrowed to metadata only — metadata is outside the constraint-collection invariants (a sparse ID-keyed store), so &mut is safe; full &mut ConstraintCollection<T> would expose active_mut() / removed_mut() / insert_with() and let callers register constraints that reference unknown variable IDs.
  • pub fn metadata() / metadata_mut() / with_metadata() / insert_with() on ConstraintCollection<T> / EvaluatedCollection<T> / SampledCollection<T>.

The *_to_v1 helpers and the parse submodules stay pub(crate). Module visibility (mod constraint, mod decision_variable, mod instance, mod indicator_constraint in lib.rs) is unchanged — git diff origin/main...HEAD -- rust/ommx/src/lib.rs is empty.

v1 wire-format limitations

v1::Solution and v1::SampleSet only have a single evaluated_constraints / constraints field for regular constraints — they have no fields for indicator / one-hot / sos1 evaluated/sampled constraints. The in-memory Rust types carry those four collections separately (with their own metadata stores), but to_bytes / from_bytes are lossy for the three special kinds. This is a pre-existing wire-format limitation (the matching Parse impls have always initialized those collections to Default::default()) and is documented on the From<Solution> for v1::Solution and From<SampleSet> for v1::SampleSet impls. Wire-shape resolution is the subject of #841.

Docs

METADATA_STORAGE_V3.md updated: status header reflects the two-wave landing, each section now carries an explicit (landed) / (deferred) tag, and the breaking-changes list is split between the two waves. The deferred sections are kept verbatim so the follow-up PR has a concrete spec to land against.

Test plan

  • cargo test -p ommx --lib474 tests passing, including four bytes-round-trip regression tests:
    • instance::parse::tests::test_parametric_instance_roundtrip_preserves_metadata
    • sample_set::tests::test_sample_set_get_preserves_metadata
    • solution::parse::tests::test_solution_roundtrip_preserves_metadata
    • sample_set::parse::tests::test_sample_set_roundtrip_preserves_metadata
  • cargo test (workspace, examples included) — clean compile, all green
  • task python:test — full Python suite including ommx + adapter tests (highs / pyscipopt / python-mip / openjij) passes
  • Snapshot review for the collection-level memory profile (accepted via cargo insta accept --workspace)
  • __init__.pyi regenerated via task python:stubgen

Out of scope (follow-up PRs)

  • Proposal: v3 special constraints proto schema #841 — special-constraint proto v3: picks the ConstraintMetadata wire shape (inline per message vs. top-level columnar map) on top of the runtime SoA stores landed here.
  • Tighten ConstraintCollection<T>::active_mut() / removed_mut() / insert_with() to pub(crate) (or smaller) — these are heavily used inside the crate but should not be on the public API surface either, per the same invariant-safety rationale that motivated narrowing the Instance accessors here.
  • Conversion of instance.constraints / decision_variables / *_constraints Python accessors from dict / list to pandas.Series[ID -> Object].
  • The *_df API reshape with the include= parameter and the long-format sidecar dfs (constraint_metadata_df / constraint_parameters_df / constraint_provenance_df / constraint_removed_reasons_df / variable_metadata_df / variable_parameters_df).
  • Standalone / Attached two-mode wrappers with Py<Instance> back-references (write-through metadata mutation).
  • Doc reorganization (METADATA_STORAGE_V3.md → SDK docs).

🤖 Generated with Claude Code

termoshtt and others added 11 commits April 27, 2026 18:14
ID-keyed Struct-of-Arrays storage for constraint and decision-variable
metadata. Generic over the ID type so all four constraint kinds
(regular / indicator / one-hot / SOS1) share one implementation.

Sparse FnvHashMap-per-field representation; absence encodes "no value".
Per-field borrowing getters hide the sparse layout behind a uniform
"missing = neutral" view (Option<&str>, empty slice, shared empty map).
Insert/remove exchange owned ConstraintMetadata / DecisionVariableMetadata
structs for I/O.

Pure additive change — types are not yet integrated into the collection
layer. Subsequent commits will move metadata storage off per-object
structs and onto the new stores.

Part of the metadata storage v3 redesign (METADATA_STORAGE_V3.md).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add the following SoA store fields, with accessor methods and
constructors:

- ConstraintCollection<T>::metadata: ConstraintMetadataStore<T::ID>
- EvaluatedCollection<T>::metadata: ConstraintMetadataStore<T::ID>
- SampledCollection<T>::metadata: ConstraintMetadataStore<T::ID>
- Instance::variable_metadata: VariableMetadataStore
- ParametricInstance::variable_metadata: VariableMetadataStore

Add ConstraintCollection::insert_with(id, c, metadata) for atomic
insert + metadata write, and with_metadata constructors on the three
collection types.

into_parts now returns (active, removed, metadata) so the metadata
store rides through transformations that destructure and rebuild
the collection. Update penalty / partial_evaluate / parse-side
serialization paths to thread the store through unchanged.

Evaluate impls clone the source collection's metadata into the
EvaluatedCollection / SampledCollection output via the new
with_metadata constructors. The per-element metadata field on
Constraint<S> stays unchanged in this commit; subsequent commits
flip the source of truth from per-element to per-collection.

Snapshot updates in instance/logical_memory: collection-level memory
now also accounts for the metadata store fields.

Part of the metadata storage v3 redesign (METADATA_STORAGE_V3.md).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drops metadata field from Constraint<S>, IndicatorConstraint<S>,
OneHotConstraint<S>, Sos1Constraint<S>, DecisionVariable,
EvaluatedDecisionVariable, SampledDecisionVariable. Also removes the
struct-init lines that populated it (Phase A: mechanical line deletions
in evaluate / arbitrary impls).

Build is broken at this commit; subsequent phases fix the readers,
serializers, and parse boundaries that previously consumed the field.
Per-constraint metadata now lives on the enclosing collection's
ConstraintMetadataStore; per-variable metadata lives on the Instance's
VariableMetadataStore.

Part of the metadata storage v3 redesign (METADATA_STORAGE_V3.md).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The per-element From<X> for v1::Y impls used to read c.metadata.foo;
now they pass `Default::default()` since the per-element struct no
longer carries metadata. The collection-level serializer overlays the
real metadata from the SoA store before emitting.

Add named helpers `evaluated_constraint_to_v1`, `sampled_constraint_to_v1`,
`decision_variable_to_v1`, `evaluated_decision_variable_to_v1`,
`sampled_decision_variable_to_v1` taking explicit metadata; the From
impls delegate with default metadata.

Update from_bytes round-trip helpers to discard the parsed metadata
(the standalone per-element API can't preserve it; full preservation
goes through the collection layer).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add `variable_metadata: VariableMetadataStore` field to Solution and
SampleSet (with builder accessor and getter). Rewrite the metadata-
based reader paths in `solution.rs::extract_decision_variables`,
`extract_all_decision_variables`, `extract_constraints`, and the
parallel `sample_set/extract.rs` siblings to look up names /
subscripts / parameters via the collection store rather than the
per-element `.metadata.*` field that was removed in Phase A.

The constraint reads use `EvaluatedCollection<T>::metadata()` /
`SampledCollection<T>::metadata()` (already added in commit a6b764e).
Decision-variable reads use the new `Solution::variable_metadata` /
`SampleSet::variable_metadata` field.

Also update solution.rs internal tests to populate variable_metadata
via the builder rather than mutating `dv.metadata` directly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Convert per-element `dv.metadata.foo = ...` and `c.metadata.foo = ...`
write sites to go through `instance.variable_metadata_mut().set_*` and
`instance.constraint_collection_mut().metadata_mut().set_*` /
`push_provenance(...)` instead.

Touched:
- instance/{slack,sos1,one_hot,log_encode,setter,decision_variable,
  indicator}.rs — slack / SOS1 indicator / one-hot promoted / log-encode
  binary auxiliary variables and constraints, plus the existing
  setter / by-name lookups
- mps/convert.rs — restructure `convert_dvars` and `convert_constraints`
  to also return `(id, name)` lists, then drain them into the SoA stores
  on the constructed Instance
- Add `Instance::constraint_collection_mut()` accessor

Tests in slack.rs / sos1.rs / one_hot.rs / indicator.rs / log_encode.rs
were updated to read names / subscripts / provenance from the SoA store
rather than from per-element `.metadata.*`.

The promoted-OneHot path now uses `ConstraintCollection::insert_with`
to atomically copy the OneHot's metadata + Provenance::OneHotConstraint
into the regular collection's store. The corresponding Indicator and
SOS1 promotion paths use `metadata_mut().push_provenance(new_id, ...)`
since the per-element source metadata transfer is intentionally
deferred (covered by the existing TODO note in
indicator_constraint/evaluate.rs).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ce / Solution / SampleSet

Library code (ommx crate) now compiles.

- ParametricInstance Parse drains metadata into the SoA store the same
  way the Instance Parse already does
- Solution Parse drains constraint and variable metadata, and the
  reverse `From<Solution> for v1::Solution` overlays them via
  evaluated_constraint_to_v1 / evaluated_decision_variable_to_v1
- SampleSet Parse drains both, builder accepts variable_metadata
- ParsedDecisionVariable / ParsedSampledDecisionVariable visibility
  promoted to `pub` (used in Parse trait `Output` types so they leak
  through the public surface)
- instance/evaluate.rs: indicator-promotion path uses
  ConstraintCollection::insert_with to carry metadata + provenance
- constraint/logical_memory.rs: drop per-element metadata visit and
  retire the now-meaningless `test_constraint_with_metadata_snapshot`
  (per-constraint metadata now visited at the collection layer)

PyO3 wrappers in python/ommx/src/ still reference the removed
metadata field; subsequent commit fixes those for the Standalone/
Attached two-mode design.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ommx crate is fully migrated off per-element metadata storage.

- mps/tests/read_tests.rs: rewrite all metadata reads to look up via
  instance.variable_metadata().name(*var_id) and
  instance.constraint_collection().metadata().name(*cid). Iteration
  patterns switched from .values() to .iter() to expose ids.
- sample_set/extract.rs: tests now build a VariableMetadataStore
  alongside decision_variables and pass it through
  SampleSet::builder().variable_metadata(...) instead of mutating
  per-element dv.metadata.
- decision_variable.rs / decision_variable/parse.rs / constraint/parse.rs:
  test code updated for the new ParsedDecisionVariable /
  ParsedSampledDecisionVariable wrapper types and the 4-tuple
  EvaluatedConstraint::Output that carries metadata as a separate value.
- Snapshot tests in instance/logical_memory.rs and
  decision_variable/logical_memory.rs updated; per-element metadata
  is no longer counted there (it lives at the collection level via
  ConstraintMetadataStore / VariableMetadataStore SoA fields).
- instance/{evaluate,slack,sos1,one_hot,indicator}.rs test reads
  switched to the SoA accessor.

PyO3 wrappers in python/ommx/src/ still reference the removed
metadata field (next phase).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…/ indicator etc.)

Each PyO3 wrapper struct now holds its own metadata snapshot:
  Constraint(pub ommx::Constraint, pub ommx::ConstraintMetadata)
  DecisionVariable(pub ommx::DecisionVariable, pub ommx::DecisionVariableMetadata)
  IndicatorConstraint(...), EvaluatedConstraint(...), SampledConstraint(...),
  EvaluatedDecisionVariable(...), SampledDecisionVariable(...).

Standalone construction starts with default metadata; reading from an
Instance fills the snapshot from the SoA store; from_components drains
each wrapper's metadata back into the instance's SoA stores. Mutations
on a snapshot do not propagate to the originating Instance.

Rust-side helpers exposed as pub for the byte-roundtrip path:
  decision_variable_to_v1, evaluated_decision_variable_to_v1,
  sampled_decision_variable_to_v1, constraint_to_v1, removed_constraint_to_v1,
  evaluated_constraint_to_v1, sampled_constraint_to_v1.

Added Instance::indicator/one_hot/sos1_constraint_collection_mut accessors.

Pandas.rs / parametric_instance / solution / sample_set still need the
matching changes; build is intentionally broken at this checkpoint.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ricInstance migrated

The pandas DataFrame rendering path no longer reads .metadata off the
inner struct. Each ToPandasEntry impl that needs metadata now consumes
WithMetadata<'a, T, M> instead of the bare T; call sites in
Instance / ParametricInstance / Solution / SampleSet pre-snapshot the
SoA store and zip the metadata in alongside each item.

Also migrated:
  - Solution / SampleSet emission of EvaluatedDecisionVariable /
    EvaluatedConstraint / SampledDecisionVariable / SampledConstraint
    now overlays metadata from the underlying SoA store.
  - ParametricInstance::from_components drains DV / Constraint metadata
    into the instance's SoA stores.
  - sample_set.rs summary_with_constraints reads constraint labels from
    the SoA store directly.
  - Exposed the *_to_v1 parse helpers as pub for the byte-roundtrip
    path on the wrappers (decision_variable, evaluated_decision_variable,
    sampled_decision_variable).

cargo build -p _ommx_rust now compiles. cargo test -p ommx --lib still
green (470 tests).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…e path

Instance::evaluate / Instance::evaluate_samples now thread their
variable_metadata into the produced Solution / SampleSet. Same for
SampleSet::get when materializing a Solution from a sample. Without
this, name-based extractors like Solution::extract_decision_variables
saw an empty metadata store on solver output and raised
"No decision variables with name X found".

cargo test -p ommx --lib still passes (470 tests).
task python:test now passes end-to-end across ommx + adapters
(highs / pyscipopt / python-mip / openjij).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@termoshtt termoshtt changed the title feat(rust): metadata SoA stores at the collection layer (in progress) feat(metadata-soa): move per-element metadata onto collection-level SoA stores Apr 27, 2026
termoshtt and others added 5 commits April 27, 2026 21:03
…_V3.md

Status header updated to reflect the actual implementation in PR #843:
Rust SDK SoA stores landed in full, Python wrappers landed using a
snapshot model rather than the originally proposed Py<Instance>
back-reference / two-mode design, and the Series / include= /
sidecar-df work deferred to a follow-up wave.

Each subsection in the Python design now carries an explicit
(landed) / (deferred) tag so readers can tell which parts are live
in v3 alpha and which are still proposal. The breaking-changes
list is split into the same two waves.

The deferred sections (Wrapper objects with back-reference,
Series-based collection accessors, *_df methods with include=,
long-format sidecar dfs, cross-ID-space join guidance) are kept
verbatim so the follow-up PR has a concrete spec to land against.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The example still set var.metadata.{name,subscripts} on bare
DecisionVariables, which no longer compiles after the per-element
metadata field was removed. Rewritten to attach metadata via
Instance::variable_metadata_mut() / .variable_metadata() — the
shape every other Rust caller now uses.

Also cleaned up two stale 'use crate::constraint::ConstraintMetadata'
imports in test modules (instance/logical_memory.rs,
instance/reduce_binary_power.rs) and one unused-variable warning
in mps/tests/read_tests.rs.

Full 'cargo test' (workspace, examples included) now compiles
cleanly with zero warnings; 470 lib tests + dataset / doc tests
all pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two silent metadata-loss paths flagged by Codex review of PR #843,
plus a scope reduction the user asked for: keep bytes round-trip only
on top-level types (Instance / ParametricInstance / Solution /
SampleSet / State / Samples / Parameters), and remove the
default-metadata From<X> for v1::Y impls that made silent drops easy.

Bug 1 (regression test + fix):
  ParametricInstance::to_bytes silently dropped both variable and
  constraint metadata. From<ParametricInstance> for v1::ParametricInstance
  bound 'variable_metadata: _' and discarded the constraint metadata via
  '(.., _metadata)' from into_parts. Rewrote it to drain the SoA stores
  exactly the same way From<Instance> does.
  Test: instance::parse::tests::test_parametric_instance_roundtrip_preserves_metadata.

Bug 2 (regression test + fix):
  SampleSet::get rebuilt the per-sample Solution's constraint
  collections via EvaluatedCollection::new(map, BTreeMap::new()),
  silently producing empty constraint metadata maps for every sampled
  Solution. variable_metadata was already threaded; constraint /
  indicator / one_hot / sos1 metadata were not. Now uses
  EvaluatedCollection::with_metadata(map, BTreeMap::new(),
  self.<kind>.metadata().clone()) for all four kinds.
  Test: sample_set::tests::test_sample_set_get_preserves_metadata.

Scope reduction (user-requested):
  - Removed Python from_bytes/to_bytes from non-top-level wrappers
    (Linear, Quadratic, Polynomial, Function, DecisionVariable,
    EvaluatedDecisionVariable, SampledDecisionVariable, NamedFunction,
    EvaluatedNamedFunction, SampledNamedFunction, Parameter). Top-level
    DTOs Instance / ParametricInstance / Solution / SampleSet keep
    them, and State / Samples / Parameters stay since they're cheap to
    maintain and are routinely passed across the evaluate boundary.
  - Removed the silent-drop From impls toward v1 types
    (impl From<DecisionVariable> for v1::DecisionVariable and the
    Evaluated/Sampled siblings; impl From<(ConstraintID,
    Constraint)> for v1::Constraint and the Evaluated/Sampled siblings).
    Replaced their NOTE with a comment explaining why they no longer
    exist. Internal call sites and tests that produced v1 messages via
    .into() now go through the explicit *_to_v1 helpers
    (decision_variable_to_v1, constraint_to_v1, removed_constraint_to_v1,
    evaluated_decision_variable_to_v1) which take the metadata as an
    argument so the drop is visible.
  - Deleted the Rust-side rust/ommx/src/decision_variable/serialize.rs
    and rust/ommx/src/constraint/serialize.rs (per-element to_bytes /
    from_bytes that relied on those From impls).
  - Reverted the pub-promotions on lib.rs modules (constraint /
    indicator_constraint / instance / decision_variable), the parse
    submodules, and the *_to_v1 helpers — they were only public to
    enable the now-removed wrapper bytes path. The crate's public
    surface is back to the pre-PR shape.

Doc cleanup (also flagged by Codex review):
  rust/ommx/src/constraint_type.rs:8 — 'Adding new constraint types'
  walkthrough now explains that the new struct must NOT carry a
  metadata field (it lives on the collection's ConstraintMetadataStore).

Test deletions:
  - python/ommx-tests/tests/test_serialize.py (DV bytes round-trip).
  - TestNamedFunctionSerialization::test_roundtrip in test_named_function.py.

cargo test: 472 lib tests pass. task python:test: full ommx + adapters
suite passes (highs / pyscipopt / python-mip / openjij).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Add round-trip regression tests for Solution::to_bytes/from_bytes and
  SampleSet::to_bytes/from_bytes verifying that variable_metadata and
  regular-constraint metadata survive a bytes cycle. The Solution test
  also covers removed-reasons via the evaluated_constraints collection;
  the SampleSet test covers regular sampled constraints with metadata.
  Tests:
  - solution::parse::tests::test_solution_roundtrip_preserves_metadata
  - sample_set::parse::tests::test_sample_set_roundtrip_preserves_metadata

- Document the v1 wire-format limitation explicitly in From<Solution>
  for v1::Solution and From<SampleSet> for v1::SampleSet: indicator /
  one-hot / sos1 evaluated/sampled constraint metadata is dropped on
  serialization because v1::Solution and v1::SampleSet have no fields
  for those collections. The matching Parse impls already init those
  collections to Default::default() for symmetry, so the in-memory
  representation has lossy proto round-trips for those kinds — a
  pre-existing wire-format limitation, not a SoA-refactor regression.

- Regenerate __init__.pyi via task python:stubgen so the deleted
  from_bytes/to_bytes methods on Linear / Quadratic / Polynomial /
  Function / DecisionVariable / EvaluatedDecisionVariable /
  SampledDecisionVariable / NamedFunction / EvaluatedNamedFunction /
  SampledNamedFunction / Parameter no longer appear in the stub.

cargo test: 474 lib tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@termoshtt termoshtt marked this pull request as ready for review April 28, 2026 03:18
Copilot AI review requested due to automatic review settings April 28, 2026 03:18
@termoshtt termoshtt added python Changes in Python SDK rust Changes in Rust SDK labels Apr 28, 2026
NamedFunction / EvaluatedNamedFunction / SampledNamedFunction
currently inline metadata as plain struct fields rather than via
a metadata: ConstraintMetadata-style substruct, so the v3 alpha
cutover did not include them. PR #843 only addresses constraints
and decision variables; named functions are a separate PR.

Add a dedicated subsection under 'Where the stores live' that:
- Documents the current shape (inline fields).
- Specifies the target shape: NamedFunctionMetadataStore (mirroring
  VariableMetadataStore — no provenance), per-element struct
  shrinks to (id, function), sibling store on each container that
  owns named_functions.
- Notes the work is mechanical (mirror VariableMetadataStore
  verbatim with NamedFunctionID).
- Explains why it ships as a separate PR (structurally distinct
  from the constraint / variable migration; no per-element
  metadata substruct to peel off, no Created → Evaluated → Sampled
  lifecycle).

Also added a bullet in 'Breaking changes / Deferred to follow-up
wave' so the downstream PR has visible scope.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

for id in &absorbed_ids {
constraints.remove(id);

P2 Badge Clear metadata for constraints absorbed by hints

This loop removes hinted regular constraints from constraints but leaves their entries in constraint_metadata, so the collection can keep orphan metadata for IDs that no longer exist. If a caller later reuses one of those IDs (for example via insert_constraint), the new constraint can pick up stale name/subscripts/parameters during lookups and v1 serialization. Remove metadata for each absorbed ID when dropping the constraint here (and in the analogous ParametricInstance parse path).

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

termoshtt and others added 3 commits April 28, 2026 12:39
…rker

The hand-written LogicalMemoryProfile impls on VariableMetadataStore
and ConstraintMetadataStore<ID> were exactly the kind of mechanical
'visit each field under TypeName.field' boilerplate that drifts
silently when a new field is added — the failure mode the derive
macro exists to prevent. Replacing them with derive removes the
drift risk.

LogicalMemoryProfile / LogicalMemoryVisitor were pub(crate). That
worked while every impl was hand-written, but it fights the derive:
putting LogicalMemoryProfile into a public type's bound (e.g.
'pub struct ConstraintMetadataStore<ID: ... + LogicalMemoryProfile>')
triggers private_bounds because the trait is more private than the
type. Promoted both traits to pub. The crate-doc rationale on
ommx-derive that previously said the trait is internal is now
inaccurate, but practically the user-facing entry point is still
Instance::logical_memory_profile / MemoryProfile; the trait is
exposed for derive ergonomics, not as a stability surface.

Added a new IDType marker trait in constraint_type.rs that bundles
the seven bounds shared by every ID newtype in the crate
(Clone + Copy + Ord + Hash + Debug + From<u64> + Into<u64> +
LogicalMemoryProfile). Used it to replace the open-coded bound on
ConstraintType::ID and ConstraintMetadataStore<ID>. A blanket impl
makes any concrete type satisfying the bounds an IDType
automatically, so callers don't implement it manually.

Net impl deletions: ~20 lines of hand-written visit_logical_memory
boilerplate gone. cargo test still passes (474 lib tests).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codex review of d6afb53 caught three doc-level inaccuracies introduced
when LogicalMemoryProfile was promoted from pub(crate) to pub:

1. The crate-doc on ommx-derive (rust/ommx-derive/src/lib.rs:7-11)
   still said the trait and derive are pub(crate) — contradicting
   the new declaration. Rewrote to explain accurately: the trait is
   declared 'pub' inside a 'pub(crate) mod logical_memory', so it is
   visible to internal modules (and to the private_bounds lint) but
   NOT reachable through the public API. The pub declaration exists
   only to satisfy the lint when the trait appears in the bound of
   a pub type (e.g. ConstraintMetadataStore<ID: ... + LMP>).

2. The doc on LogicalMemoryProfile itself (logical_memory.rs:80) and
   the comment on the derive re-export (logical_memory.rs:294) both
   suggested external crates could 'use ommx::LogicalMemoryProfile'.
   They cannot — the enclosing module is pub(crate). Updated both to
   describe the actual setup.

3. The IDType doc (constraint_type.rs:43) listed SampleID as one of
   the IDs satisfying the marker. SampleID does not impl
   LogicalMemoryProfile (it's a sample-set index, not a metadata-
   bearing entity), so the example was wrong. Removed SampleID from
   the list and added a one-line note explaining why it is excluded.

No code change. cargo test still passes (474 lib tests).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
termoshtt added a commit that referenced this pull request Apr 28, 2026
## Summary

Drops bytes serialization (`to_bytes` / `from_bytes`) from
component-level Python wrappers. Top-level container types and
cross-evaluate DTOs keep bytes serialization unchanged.

This is a Python SDK breaking change, split out from #843 so the larger
metadata-storage refactor can land separately. The Rust SDK is unchanged
in this PR — only the PyO3 binding surface narrows.

## Affected types

**Removed `to_bytes` / `from_bytes`:**
- `Function`, `Linear`, `Quadratic`, `Polynomial`
- `Parameter`
- `NamedFunction`, `EvaluatedNamedFunction`, `SampledNamedFunction`
- `DecisionVariable`, `EvaluatedDecisionVariable`,
`SampledDecisionVariable`

**Kept (top-level + cross-evaluate DTOs):**
- `Instance`, `ParametricInstance`, `Solution`, `SampleSet`
- `State`, `Samples`, `Parameters`

## Rationale

These methods originally existed because the pre-v3 Python SDK had its
own protobuf-based wrapper layer, so every value crossing the Python ↔
Rust boundary had to be serialized — `to_bytes` / `from_bytes` was the
bridge. With v3's switch to direct PyO3 re-exports the boundary is gone,
and element-level bytes round-trips no longer serve a purpose. The
upcoming metadata-storage redesign (#843 lineage) would only inflate the
maintenance cost of keeping them, so they're retired now.

## Test plan

- [x] `task python:stubgen` regenerated stubs and
`docs/api/api_reference.json`
- [x] `task python:test` passes (ommx core + highs / pyscipopt /
python-mip / openjij adapters)
- [x] `task format` clean

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
termoshtt and others added 2 commits April 28, 2026 13:25
The public `Instance::*_constraint_collection_mut()` accessors handed
out `&mut ConstraintCollection<T>`, which exposes `active_mut()`,
`removed_mut()`, and `insert_with()` — operations that can violate
Instance-level invariants (variable IDs in a constraint must be
registered, active/removed sets must be disjoint).

In practice, every public call site only chained `.metadata_mut()`
afterwards: the SoA metadata store is outside the invariants (a sparse
ID-keyed map of names/subscripts/parameters/description/provenance), so
mutating it is always safe.

Replace the 8 invariant-leaking accessors on `Instance` and
`ParametricInstance` with 8 narrow `*_metadata_mut()` plus 8 immutable
`*_metadata()` siblings, mirroring the existing
`variable_metadata`/`variable_metadata_mut` shape. All 6 call sites
(MPS convert, parse roundtrip test, two PyO3 wrappers) become shorter.

Crate-internal use of `ConstraintCollection::active_mut`/`removed_mut`/
`insert_with` is unchanged for now; tightening that surface is left to
a separate PR.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@termoshtt termoshtt merged commit 417bf80 into main Apr 28, 2026
30 checks passed
@termoshtt termoshtt deleted the feat/metadata-soa-rust branch April 28, 2026 04:50
@termoshtt termoshtt removed the python Changes in Python SDK label Apr 28, 2026
termoshtt added a commit that referenced this pull request Apr 28, 2026
## Summary

Wave 1.5 of the v3 alpha pandas surface, scoped to `include=` filtering
plus long-format / id-indexed sidecar DataFrames. The
`kind=Literal[...]` consolidation of today's per-kind `*_df` methods
stays out of scope for this PR.

- **`include=` parameter** on every wide `*_df` accessor across
`Instance` / `ParametricInstance` / `Solution` / `SampleSet`. Default
`("metadata", "parameters")` matches the v2-equivalent shape;
`include=[]` drops both, `include=["metadata"]` /
`include=["parameters"]` keep just the named family. Unknown values
raise `ValueError`. Implementation post-filters per-row dicts in
`entries_to_dataframe` based on a `IncludeFlags` derived from the
argument — the `ToPandasEntry` impls themselves are unchanged.

- **Property → method conversion** for every `*_df` accessor. v3-alpha
breaking change: `instance.constraints_df` →
`instance.constraints_df()`. The existing 11 in-tree call sites, the one
doctest in `solution.rs`, and the Sphinx book (en + ja) are all updated.
`removed_reasons_df` / `*_removed_reasons_df` also lose the property
syntax for API consistency, even though they don't take `include=` (no
metadata / parameters columns to filter).

- **Long-format / id-indexed sidecar DataFrames** on Instance /
ParametricInstance / Solution / SampleSet, reading directly from the SoA
metadata stores:
- `constraint_metadata_df(kind=...)` — id-indexed wide; columns `name` /
`subscripts` / `description`, index = `{kind}_constraint_id`.
- `constraint_parameters_df(kind=...)` — long format
(`{kind}_constraint_id`, `key`, `value`). One row per (id,
parameter_key) pair.
- `constraint_provenance_df(kind=...)` — long format
(`{kind}_constraint_id`, `step`, `source_kind`, `source_id`). One row
per (id, step) in the provenance chain.
- `constraint_removed_reasons_df(kind=...)` — long format
(`{kind}_constraint_id`, `reason`, `key`, `value`). One row per (id,
parameter_key) pair, plus one row with `key` / `value` = NA for ids
whose reason has no parameters.
  - `variable_metadata_df()` — id-indexed wide; index = `variable_id`.
- `variable_parameters_df()` — long format (`variable_id`, `key`,
`value`).

`kind` accepts `"regular"` / `"indicator"` / `"one_hot"` / `"sos1"`
(default `"regular"`); unknown values raise `ValueError`. Index column
names are kind-qualified (`regular_constraint_id` ≠
`indicator_constraint_id`) so accidental cross-id-space joins surface in
`df.head()` / `df.info()`. Long-format `*_parameters_df` /
`*_removed_reasons_df` rows are sorted by `(id, key)`, and empty
long-format DataFrames keep their column schema instead of returning a
column-less frame.

- **Macro refactor** of the per-host kind dispatch. Each of the four
hosts gets a `constraint_kind_collection!` macro that collapses the
16-arm `ConstraintKind` match (4 sidecars × 4 kinds) into 4 macro
invocations. Net **−354 lines** across instance.rs /
parametric_instance.rs / solution.rs / sample_set.rs.

- **`set_parameter_columns` sorts keys** before emitting
`parameters.{key}` columns, so wide-`*_df` parameter column order
matches the long-format sort and stays process-stable even when the
upstream `Constraint.set_parameters` is given a `std::HashMap` (whose
iteration is randomized per process). The helper is generic over the
hasher so it accepts both `FnvHashMap` (SoA stores) and `std::HashMap`
(`v1::Parameter`).

- **Codex review pass** caught three correctness bugs that are now
fixed:
- Solution / SampleSet sidecar accessors no longer double-count removed
constraints (`EvaluatedCollection.inner()` already includes them —
chaining `.removed_reasons().keys()` produced duplicate rows).
- `WithMetadata<EvaluatedDecisionVariable>` /
`WithMetadata<WithSampleIds<SampledDecisionVariable>>` now emit
`parameters.{key}` columns, so
`Solution.decision_variables_df(include=["parameters"])` and the
SampleSet equivalent return the right data.
- `WithSampleIds<SampledNamedFunction>` emits `parameters.{key}` columns
instead of a single bare `parameters` string column, so
`apply_include_filter` honours `include=[]` here too.

- **Book updates** (`docs/en/**/*.md` + `docs/ja/**/*.md`): tutorial /
user_guide / release-note pages flip to the method form, `{attr}`
cross-references for `*_df` switch to `{meth}`, and a new "Unreleased"
entry on `release_note/ommx-3.0.md` covers the property → method
conversion, `include=`, and the six new sidecar accessors. 2.x release
notes are added to `nb_execution_excludepatterns` (same treatment as 1.x
— historical code samples reference removed APIs).

- **Proposal doc** (`METADATA_STORAGE_V3.md`): reflects the narrow
per-collection metadata accessors landed in #843, the explicit "no
`*_collection_mut()`" policy, the actual `insert_with(id, c, metadata)
-> ()` shape, and now also the Wave 1.5 landing — `include=` and the
sidecar accessors are marked **landed (PR #846)**, with the deferred
`kind=Literal[...]` consolidation, Series accessors, and Attached
wrappers spelled out as the remaining Wave 2 work.

## Out of scope

- `constraints_df(kind=Literal["regular", "indicator", "one_hot",
"sos1"])` consolidating today's per-kind `*_constraints_df` family —
separate PR. This PR keeps the per-kind methods as-is.
- `pandas.Series[ID -> Object]` collection accessors and the Standalone
/ Attached two-mode wrappers — Wave 2 of `METADATA_STORAGE_V3.md`.
- `NamedFunction` SoA migration.

## Test plan

- [x] `task python:test` passes locally with the full PR.
- [x] `task python:book:test` rebuilds en + ja docs cleanly with all
code-cells executing.
- [x] Snapshot-based `python/ommx-tests/tests/test_dataframe_include.py`
— 9 cases covering default vs. empty vs. metadata-only vs.
parameters-only on `Instance.decision_variables_df`,
`Instance.constraints_df`, `Solution.decision_variables_df`,
`SampleSet.decision_variables_df`, plus an unknown-flag `ValueError`
check. Snapshots written to `__snapshots__/test_dataframe_include.ambr`.
- [x] Snapshot-based
`python/ommx-tests/tests/test_dataframe_sidecars.py` — 16 cases covering
all 6 sidecar accessors (id-indexed shape, long-format shape, `kind=`
dispatch on every constraint family, qualified index names,
empty-provenance baseline, provenance after
`convert_one_hot_to_constraint`, removed-reasons after
`relax_constraint`, removed-reasons with parameters after one-hot
conversion, Solution / SampleSet parity with Instance) plus 4 regression
tests for the Codex fixes (Solution / SampleSet sidecar dedup with a
relaxed constraint, Solution / SampleSet
`decision_variables_df(include=["parameters"])` actually emits the
columns). Snapshots in `__snapshots__/test_dataframe_sidecars.ambr`.
- [x] 5 consecutive `pytest` runs all pass — confirms determinism after
the long-format `(id, key)` sort and the alphabetised wide
`parameters.{key}` order.
- [x] Two Codex full-PR review passes; the second came back with one
MUST-FIX (a JA / EN inconsistency in `instance.md` already in place
before this PR but surfaced by the docs review) and two non-blocking
notes that are scoped out (`pd.DataFrame(columns=[...])` produces
`object` dtypes for the empty-frame case; historical 2.x release-note
prose references `*_df` without parens — kept as name reference, not
invocation).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
termoshtt added a commit that referenced this pull request Apr 30, 2026
## Summary

- Move `name` / `subscripts` / `parameters` / `description` off
`NamedFunction`, `EvaluatedNamedFunction`, `SampledNamedFunction` onto a
new `NamedFunctionMetadataStore` SoA store, kept as a sibling field on
`Instance` / `ParametricInstance` / `Solution` / `SampleSet`. Same
snapshot-model PyO3 wrappers and `WithMetadata`-based pandas rendering
as PR #843 (which did this for `Constraint` / `DecisionVariable`).
- Parse boundary moves to per-collection:
`Vec<v1::NamedFunction>::Parse` returns `(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_samples` thread the host store onto
the produced `Solution` / `SampleSet`. `SampleSet::get` carries the
store onto each per-sample `Solution`.
- Element-level `to_bytes` / `from_bytes` removed from `NamedFunction` /
`EvaluatedNamedFunction` / `SampledNamedFunction` — same rationale as
#845: a per-element wire round-trip can no longer carry host-stored
metadata, so the methods became misleading rather than safe.
- METADATA_STORAGE_V3.md flipped \`NamedFunction\` from \"deferred —
separate PR\" to \"landed (this PR)\".

## 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

- [x] \`cargo test -p ommx --lib\` — 475 passed (incl. new
\`test_instance_roundtrip_preserves_named_function_metadata\` regression
test)
- [x] \`task python:test\` — all adapter + ommx-tests pass; lint +
typecheck clean
- [x] \`cargo insta test --accept\` for the 5
\`instance::logical_memory\` snapshots that gained the new
\`Instance.named_function_metadata;…\` rows
- [x] Format / clippy clean on changed files

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
termoshtt added a commit that referenced this pull request May 1, 2026
…th alpha+PR

Rust release note (v3_0_0_alpha_1):
- Lead now mentions per-collection metadata stores alongside the
  collection-level serialization theme.
- Removed the stale `metadata: ConstraintMetadata` field from the
  example `Constraint<S>` struct (matches the actual code).
- New "Metadata storage" section covering ConstraintMetadataStore /
  VariableMetadataStore / NamedFunctionMetadataStore, the per-host
  accessors, the `pub(crate)` mutator tightening, and the Python-side
  AttachedX / *_df consequences. Lists the contributing PRs (#843,
  #846-#850, #852, #853) inline.

Python migration guide:
- Sections that describe 3.0.0a3 changes get a `(3.0.0aN, [#PR](url))`
  tag in the heading: §6.6 (method, not property), §9 (DataFrame API),
  §10 (long-format sidecars), §11 (AttachedX). Single v2 → v3 framing
  preserved; per-section attribution makes "where did this land?"
  obvious without splitting the doc by alpha.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
termoshtt added a commit that referenced this pull request May 1, 2026
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>
termoshtt added a commit that referenced this pull request May 1, 2026
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>
termoshtt added a commit that referenced this pull request May 1, 2026
## 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rust Changes in Rust SDK

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant