docs: fold METADATA_STORAGE_V3.md into the migration guides#854
docs: fold METADATA_STORAGE_V3.md into the migration guides#854
Conversation
The design doc has served its purpose — every wave it tracked is landed. The user-facing parts that weren't already covered move into the migration guides; the historical implementation log lives in git. Python migration guide: - New §9 / §10 / §11 covering the Wave 1.5+2+3 surface that wasn't documented yet: `*_df` methods with `kind=` / `include=` / `removed=`, long-format sidecar DataFrames, and the `AttachedX` write-through return-type changes on `instance.constraints[id]` etc. - §6.6 clarified — every `*_df` is a method, not a property. - Convenience additions renumbered §9 -> §12. - Overview themes and the migration checklist extended accordingly. Rust migration guide: - Removed the stale `metadata: ConstraintMetadata` field references that pre-dated the SoA refactor. Metadata now lives in the per-collection store and is queried via `instance.constraint_metadata().name(id)` etc. - Updated `Constraint`, `EvaluatedConstraint` struct-construction examples and the `getset Removal` text accordingly. - Added the `metadata` field to the `ConstraintCollection` / `EvaluatedCollection` / `SampledCollection` shape blocks. - New "Metadata stores" section covering the per-host accessors and the `ConstraintMetadataStore<ID>` API surface. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…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>
…ython section
Rust:
- Drop the per-alpha submodule structure: the Rust SDK is one connected
3.0.0 release line, so `release_note.md` becomes the consolidated
topic doc. `release_note/v3_0_0_alpha_1.md` and the
`pub mod v3_0_0_alpha_1 {}` submodule under `crate::doc::release_note`
are removed; `migration_guide.md`'s back-reference now points at
`crate::doc::release_note` directly.
Python migration guide:
- Per-section `(3.0.0aN, [#PR](url))` tags on every section in §1-§11.
Single v2 → v3 framing preserved; per-section attribution lets users
see which alpha each individual change shipped in without splitting
the doc by alpha.
- Rust and Python SDK versioning is intentionally separate, so Rust
release-note PR refs are bare `[#XXX](...)` (no version tag) and
Python migration-guide tags use Python alpha labels (`3.0.0a1`,
`3.0.0a2`, `3.0.0a3`).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- §8: rewrite to accurately describe v3 setter semantics. v3's Constraint.add_name/etc. still mutate in place; they additionally return self.clone(). Single-call usage is identical to v2; the divergence is in chained calls without reassignment, where the chain operates on clones after the first method. Section retitled "Snapshot Constraint setters return a clone, not `self`" and the example pair shows both the equal and divergent paths. - §5.1: write_mps -> save_mps tag corrected #780 -> #775. The rename effectively landed when the Python wrapper class was replaced by a direct Rust re-export (#775); #780 was a later docgen change. - §5.4: with_parameters plain dict signature tag corrected #776 -> #774. The HashMap<u64, f64> signature came in with the ParametricInstance Rust re-export (#774, d7122f3); #776 only removed the protobuf Parameters wrapper. - Migration checklist for §8 updated to point at the chained-call trap and the AttachedConstraint escape hatch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- §11 v2.5.1 example: replace `instance.constraints[5]` (which would index a list at position 5 in v2.5.1, not lookup id=5) with `instance.get_constraint_by_id(5)` — the actual v2.5.1 id-keyed lookup. - §9 wide-df index naming: `decision_variables_df()` keeps `id` as its index name, not `variable_id`. Only the long-format variable sidecars (`variable_metadata_df`, `variable_parameters_df`) use `variable_id`. Migration checklist line corrected to match. - §4.2 v3 example: `dict[int, Constraint]` -> `dict[int, AttachedConstraint]` for the active-constraint dict, with a closing paragraph noting the 3.0.0a2 (snapshot) -> 3.0.0a3 (write-through) evolution and pointing at §11 for the read/write semantics. Solution / removed dicts keep snapshot value types. - Rust release note: tighten the "tightens the invariants" paragraph to clarify that `metadata_mut` on `ConstraintCollection<T>` is pub (it can't break either invariant); only the active/removed map mutators are pub(crate). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
BLOCKING (Rust migration guide): - The "mirrored on Solution / SampleSet" claim for the `*_constraint_metadata()` accessors is wrong — Solution / SampleSet do not flatten constraint metadata to host-level methods. They expose variable / named-function stores at the host (`solution.variable_metadata()`, `sample_set.variable_metadata()`, etc.) but constraint metadata is reached through the evaluated / sampled collection getter and then `.metadata()` on the collection (e.g. `solution.evaluated_constraints().metadata()`, `sample_set.indicator_constraints().metadata()`). Section split into Instance / ParametricInstance vs. Solution / SampleSet examples. NICE-TO-FIX: - §6.6 tag was missing the primary PR. The `parameters` (list) / `parameters_df` (DataFrame) split landed in #774 (3.0.0a1) when ParametricInstance became a Rust re-export; #846 (3.0.0a3) only flipped `_df` from `#[getter]` to method. Tag now lists both alphas with the corresponding PRs and the prose disambiguates. - `VariableMetadataStore` / `NamedFunctionMetadataStore` shape note: not just "provenance omitted". `NamedFunctionMetadataStore` also lacks the `push_subscript` / `extend_subscripts` append helpers (callers use `set_subscripts(id, new_vec)` to extend). Note revised. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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>
There was a problem hiding this comment.
Pull request overview
Documentation consolidation for the v3 (3.0 alpha) metadata/SoA work: removes the now-obsolete design/status doc and folds the relevant user-facing guidance into the Rust release notes and the Rust/Python migration guides, while simplifying the Rust rustdoc release-note module layout.
Changes:
- Consolidate Rust release notes into
rust/ommx/doc/release_note.mdand remove the per-version submodule doc file. - Update Rust migration guide to reflect the SoA metadata-store access pattern (remove stale
metadata: ConstraintMetadatareferences). - Expand Python migration guide with per-section alpha/PR tags and new sections covering
*_dfmethod changes, sidecar DataFrames, andAttachedXwrite-through handles; deleteMETADATA_STORAGE_V3.md.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| rust/ommx/src/lib.rs | Simplifies the rustdoc release_note module to a single module backed by release_note.md. |
| rust/ommx/doc/release_note/v3_0_0_alpha_1.md | Deletes the old per-version release-note document (content moved into release_note.md). |
| rust/ommx/doc/release_note.md | Adds the consolidated “3.0” release notes, including the metadata-store SoA refactor summary. |
| rust/ommx/doc/migration_guide.md | Updates migration guidance to remove stale inline-metadata references and document SoA store accessors/API. |
| PYTHON_SDK_MIGRATION_GUIDE.md | Adds alpha/PR tags and new sections for DataFrame API changes, sidecars, and AttachedX semantics. |
| METADATA_STORAGE_V3.md | Deletes the completed design/status doc (superseded by migration guides + release notes). |
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>
The `include=("metadata","parameters","removed_reason")` shorthand
read like a tuple default, but the actual default is `include=None`
which expands to `("metadata", "parameters")`; `"removed_reason"` is
opt-in via `include=`, and only auto-enabled on Instance /
ParametricInstance when `removed=True`. Bullet rewritten to spell out
the default and the auto-enable rule.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The table claimed v2.5.1 already had `instance.indicator_constraints` / `one_hot_constraints` / `sos1_constraints` returning dict-typed snapshot wrappers, and `instance.constraints` returning a dict. None of that is true in v2.5.1: - `instance.constraints` was `list[Constraint]` (the dict shape arrived in 3.0.0a2, #806). - `IndicatorConstraint` did not exist as a type at all in v2.5.1 (#790, 3.0.0a2). - `one_hot_constraints` / `sos1_constraints` were not direct accessors; they lived inside `instance.constraint_hints` as legacy `OneHot` / `Sos1` objects. Table now has three columns (v2.5.1 / 3.0.0a2 / v3 final) so each hop is visible, and a closing line points at §4.2 for the list -> dict move which is unrelated to this section's wrap-into-AttachedX change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codex 3rd review flagged two §3.0-overview bullets that contradicted the rewritten body sections: - Serialization bullet listed only Instance / Solution / SampleSet, omitting ParametricInstance::to_bytes / from_bytes (which the Collections-and-serialization detail section correctly mentions). Bullet now lists all four hosts and reframes "collection is the natural unit" as "host is the natural unit", matching the body rewrite. - Error-surface bullet still said "the public error surface collapses to a single type" — too strong, and contradicting the detailed section which spells out where typed signal types (BoundError / DecisionVariableError / DuplicatedSampleIDError / SubstitutionError, …) are returned at their public APIs. Bullet rewritten as "the default error surface is ommx::Result<T>" with the typed-signal exceptions listed inline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Documentation hygiene update for the v3 docs: removes the now-obsolete METADATA_STORAGE_V3.md design/status doc, consolidates the Rust release notes into a single release_note.md, and updates the Rust/Python migration guides to reflect the SoA metadata-store refactor and related API changes.
Changes:
- Flatten Rust release notes by inlining the former
v3_0_0_alpha_1.mdcontent intorust/ommx/doc/release_note.mdand removing the nested rustdoc submodule. - Update the Rust migration guide to remove stale
metadata: ConstraintMetadatareferences and document the SoA metadata-store access pattern and APIs. - Expand/retag the Python migration guide with per-section
(3.0.0aN, #PR)annotations and new sections covering DataFrame accessor changes andAttachedXwrite-through handles; deleteMETADATA_STORAGE_V3.md.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| rust/ommx/src/lib.rs | Removes the nested release_note::v3_0_0_alpha_1 module mount, keeping a single doc::release_note entry point. |
| rust/ommx/doc/release_note/v3_0_0_alpha_1.md | Deletes the now-inlined per-version release-note file. |
| rust/ommx/doc/release_note.md | Becomes the consolidated 3.0 release notes, including the SoA metadata-store narrative and updated cross-links. |
| rust/ommx/doc/migration_guide.md | Updates examples and adds a “Metadata stores” section aligned to the SoA refactor and new accessor APIs. |
| PYTHON_SDK_MIGRATION_GUIDE.md | Adds alpha/PR tags and new sections documenting DataFrame API consolidation and AttachedX semantics. |
| METADATA_STORAGE_V3.md | Removes the completed design/status doc now that user-facing content is migrated into guides/notes. |
| // ✅ Insertion via the host's invariant-safe entry point — picks an | ||
| // unused id, drains the (optional) metadata into the SoA store, | ||
| // validates required_ids, returns the assigned id. | ||
| let id = instance.add_constraint( |
The release note had grown into a parallel reference for several APIs (struct definitions, accessor lists, signal-type catalogs) that were also in the Rust migration guide. Maintaining both copies meant every small fix needed to land in two places, which is why review passes kept finding "release-note overview vs detail vs migration guide" mismatches. This commit reframes the release note as a topic-oriented "what & why" summary and lets the migration guide / error-handling tutorial own the API enumerations: - Stage parameter section: drops the `Constraint<S>` / `ConstraintType` / `Stage` code blocks. Keeps the motivation (4×3 matrix collapse) and the two knock-on simplifications. Adds links to migration guide §1 (Constraint Field Access) and `## New Types`. - Collections and serialization: drops the per-trait method enumeration. Keeps the host-level serialization rationale and the per-host to_bytes/from_bytes entry points. Adds links to migration guide §ConstraintCollection / §EvaluatedCollection. - Metadata storage: drops the struct-shape code block, the per-host accessor enumeration, and the store-API list. Keeps the one-canonical-place architectural argument, the three store families, the invariant story, and the Python-side bullets (with the four PRs attributed inline). Adds a link to migration guide §Metadata stores. - Unified error surface: drops the full signal-type list and the per-API-entry-point mapping. Keeps the default-error-type rule, the fail-site macro story, the parser-error exceptions, and a short representative signal-type list. Defers to the error handling tutorial for the full catalog. Net effect: 387 -> 288 lines. The release note is now meaningfully shorter than the migration guide section it points at, which is the right shape — the release note tells the story, the migration guide documents the surface. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three NICE-TO-FIX items from Codex's fourth review pass: - Python paragraph in §Metadata storage: split the single PR-list paragraph into two bullets so AttachedX (#849, #850, #852) and *_df (#846, #847) attributions stand on their own without the reader needing to cross-check the Python guide. - Capability model wording (§3.0 overview bullet + §Capability model body): "any OMMX Instance can be fed to any adapter" overstated reduce_capabilities' infallibility. It returns Result, e.g. the Big-M encoding requires finite bounds. Both the bullet and the body sentence now say "valid OMMX Instance" and call out the fallible path with Err(ommx::Error). - "Full signal-type list" -> "curated signal-type list" in the cross-reference to the error handling tutorial, matching the tutorial's own framing (it already uses "curated set"). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…raint example Copilot review on PR #854 noted that the `instance.add_constraint(...)` example in `### 2. Struct Literal Construction` invokes a `&mut self` method but doesn't show that `instance` is mutable, so a reader copy-pasting the snippet would hit a borrow error. Add an explicit comment that `add_constraint` / `relax_constraint` / `restore_constraint` take `&mut self`, so `instance` must be a `mut` binding or be accessed via `&mut Instance`. The comment block was already calling out the pub(crate) tightening; this just adds the mut-binding requirement alongside. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Documentation hygiene pass for v3:
METADATA_STORAGE_V3.md— design doc whose waves all landed.rust/ommx/doc/release_note.md) covering the 3.0 line. Drop the per-alpha submodule (crate::doc::release_note::v3_0_0_alpha_1removed;crate::doc::release_notestays).metadata: ConstraintMetadatareferences in the Rust migration guide (those pre-dated the SoA refactor).(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.0overview bullets — first-class special constraints, Stage /ConstraintTypecollapse, host-level serialization, per-collection SoA metadata stores, capability model, defaultommx::Result<T>error surface,v1_extremoval.## 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-levelto_bytes/from_bytes. Migration guide owns the method lists.## Metadata storage: SoA store on the enclosing collection— three store families (ConstraintMetadataStore<ID>,VariableMetadataStore,NamedFunctionMetadataStore), thepub(crate)mutator tightening, and a Python paragraph linking out toPYTHON_SDK_MIGRATION_GUIDE.md§9–11 with PR attributions forAttachedX(feat(python): AttachedConstraint write-through wrapper for Instance #849, Extend AttachedX write-through pattern to all constraint kinds and DecisionVariable #850, feat(arith): centralize ToFunction extraction and switch decision_variables to AttachedDecisionVariable #852) and*_df(feat(pandas): include= and long-format sidecar dataframes #846, feat(pandas): unify per-kind constraints_df into kind=/include=/removed= dispatch #847). Rust-side bundle is (feat(metadata-soa): move per-element metadata onto collection-level SoA stores #843, feat(named-function): SoA metadata store migration #848, Extend AttachedX write-through pattern to all constraint kinds and DecisionVariable #850, refactor(rust): narrow ConstraintCollection mutation primitives to pub(crate) #853).## Capability model—AdditionalCapability/Capabilities/required_capabilities()/reduce_capabilities(&supported).## Unified error surface— defaultommx::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 inqplib/parserandartifact.## Domain types replace v1_ext,## Other notable changes— unchanged.Rust migration guide (
rust/ommx/doc/migration_guide.md)metadata: ConstraintMetadatafield references onConstraint<S>and the struct-construction examples.### Metadata storessection: per-host accessors onInstance/ParametricInstance(constraint_metadata()/_mut(), indicator / one-hot / sos1 variants,variable_metadata(),named_function_metadata()); howSolution/SampleSetreach constraint metadata viaevaluated_constraints().metadata()etc.; theConstraintMetadataStore<ID>API (per-field reads,collect_for(id), write-through setters, bulkinsert/remove); plus theVariableMetadataStore/NamedFunctionMetadataStoreshape differences (no provenance;NamedFunctionMetadataStorealso lackspush_subscript/extend_subscripts).metadatafield toConstraintCollection/EvaluatedCollection/SampledCollectionshape blocks.constraint.nameupdated to point at the SoA accessors.Python migration guide (
PYTHON_SDK_MIGRATION_GUIDE.md)*_dfis a method.kind=/include=/removed=consolidating the per-kind / active-vs-removed family.include=defaults to("metadata", "parameters");"removed_reason"opt-in (auto-enabled withremoved=Trueon Instance / ParametricInstance).constraint_metadata_df,constraint_provenance_df,variable_parameters_df, …).instance.constraints[id]etc. now returnAttachedXwrite-through handles. Three-column affected-types table (v2.5.1 / 3.0.0a2 / v3 final) so each hop is visible.add_*entry points andattached.detach().(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-depsrenders the release note and migration guide cleanly (verified locally).ommx::doc::release_noteandommx::doc::migration_guideon the localtarget/docbuild for cross-reference resolution and section ordering.METADATA_STORAGE_V3.mdorrelease_note::v3_0_0_alpha_1(verified locally).🤖 Generated with Claude Code