From 0f9555e505569aa8337263e0abb62d171fcb1bee Mon Sep 17 00:00:00 2001 From: cafzal Date: Mon, 3 Aug 2026 10:37:37 -0700 Subject: [PATCH 1/3] Whole-plan constraints: one resolver, so the engine and the validator agree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit max_allocation and cardinality describe the WHOLE plan, so several rows of either type resolve to a single applied value — and the two sides of the engine resolved them differently. _parse_constraints kept the LAST row while the pre-solve checks read the FIRST via next(...), so validate could hard-error a model on a cap the solver was never going to apply. Verified both ways: max_allocation 30 then 60, three options — solver applies 60 (feasible), while validate reasons about 30 and errors "cap (30%) × 3 = 90% < 100%": ready=False on a model that solves. cardinality (1,3) then (1,5) with four force_includes — solver applies max 5 (feasible), while validate errors "force_include count (4) exceeds cardinality max (3)". Fix, by the same vocabulary-coherence logic the allocation_bound merge used: several rows INTERSECT (tightest cap; range intersection), and both sides read one pair of resolvers — merged_max_allocation() and merged_cardinality() — which is what keeps them in agreement. Resolution is order-independent, a single row resolves to itself unchanged, validate warns per merged type, and an empty cardinality intersection is an error instead of a silently applied range. Also closes the create-time echo gap: constraints passed at model create now get the same validation_issues + constraints_merged_note echo an update gives them, rather than going unchecked until the next update or the solve. Docs: architecture.md create/update rows, the Cardinality/MaxAllocation model docstrings, the constraints param description, the create docstring, and the problem_framing constraint schema. Tests: 8 in tests/test_optimizer.py (both disagreements, order independence, intersection, empty range, warnings, single-row identity, the group-floor check's cardinality max) and 2 in tests/test_server.py (update echo, create echo). Full suite: 959 passed. --- architecture.md | 4 +- engine/models.py | 6 + engine/optimizer.py | 143 +++++++++++++++---- mcp_server/server.py | 62 ++++++-- skills/problem_framing/references/schemas.md | 4 +- tests/test_optimizer.py | 108 ++++++++++++++ tests/test_server.py | 64 ++++++++- 7 files changed, 345 insertions(+), 46 deletions(-) diff --git a/architecture.md b/architecture.md index 0eac250..0aa5cce 100644 --- a/architecture.md +++ b/architecture.md @@ -23,8 +23,8 @@ Frontier exposes 4 tools – 3 domain tools with multiple actions, plus a skill | Tool | Action | Purpose | |------|--------|---------| -| **model** | `create` | Start a new optimization problem (name, domain, context, approach) | -| | `update` | Add/modify objectives (≥2 enforced; 2–7 is the designed envelope), options, scores, constraints, reference points, scenarios. Scores and interaction matrices merge (upsert) — matrices by objective, and cell-wise within a matrix when `mode="upsert"`, so one larger than a single tool call is built across several (same merge as scenario overrides, via `optimizer.apply_matrix_override`); objectives, options, constraints, and reference points are full replacement — the contract is restated on each data param's schema description, `status` echoes the constraint count, and any update that *shrinks* the constraint set (a partial replacement list, or an objectives/options replacement cascade-dropping referencing rules) carries a `constraints_note` so rules can't silently vanish; several `allocation_bound` rows on ONE option apply *intersected* (the tightest box — max of the mins, min of the maxes, the way several `objective_bound` rows on one objective all bind), echoed as `constraints_merged_note`, reported by `validate` (a warning per merged option, an error when the intersection is empty), and read back as the applied box — one line per option, captioned with the row count — in `get`'s formulation card and its ASCII twin, so a floor row sent beside a cap row keeps both; matrix writes echo `interaction_matrix_cells` per touched objective (post-merge cell count, plus a note naming the cause when the count *fell* — the signature of a chunk sent without `mode="upsert"` wiping its predecessors). Marks results stale on structural changes — each stored frontier compared against the fingerprint of the inputs *it* reads (base runs against the base inputs; the scenario set against those plus `scenario_config`), so a scenarios-only edit never flags a base frontier it cannot affect. | +| **model** | `create` | Start a new optimization problem (name, domain, context, approach). Constraints passed here are checked on the spot, like an update's — the response carries `validation_issues` and `constraints_merged_note`. | +| | `update` | Add/modify objectives (≥2 enforced; 2–7 is the designed envelope), options, scores, constraints, reference points, scenarios. Scores and interaction matrices merge (upsert) — matrices by objective, and cell-wise within a matrix when `mode="upsert"`, so one larger than a single tool call is built across several (same merge as scenario overrides, via `optimizer.apply_matrix_override`); objectives, options, constraints, and reference points are full replacement — the contract is restated on each data param's schema description, `status` echoes the constraint count, and any update that *shrinks* the constraint set (a partial replacement list, or an objectives/options replacement cascade-dropping referencing rules) carries a `constraints_note` so rules can't silently vanish; several `allocation_bound` rows on ONE option apply *intersected* (the tightest box — max of the mins, min of the maxes, the way several `objective_bound` rows on one objective all bind), echoed as `constraints_merged_note`, reported by `validate` (a warning per merged option, an error when the intersection is empty), and read back as the applied box — one line per option, captioned with the row count — in `get`'s formulation card and its ASCII twin, so a floor row sent beside a cap row keeps both; the whole-plan types take one row each — several `max_allocation` rows resolve to the tightest cap and several `cardinality` rows to the intersection of their ranges (`optimizer.merged_max_allocation` / `merged_cardinality`, the single resolvers BOTH the solve encoding and the pre-solve checks read, so the two can't reason about different rows), echoed as `constraints_merged_note` with a `validate` warning per merged type and an error on an empty cardinality intersection; matrix writes echo `interaction_matrix_cells` per touched objective (post-merge cell count, plus a note naming the cause when the count *fell* — the signature of a chunk sent without `mode="upsert"` wiping its predecessors). Marks results stale on structural changes — each stored frontier compared against the fingerprint of the inputs *it* reads (base runs against the base inputs; the scenario set against those plus `scenario_config`), so a scenarios-only edit never flags a base frontier it cannot affect. | | | `get` | Return problem state. Defaults to the `summary` slice (counts + status flags + the decision-question `context` — always small). Optional `section` for targeted slices: summary, objectives, options, scores, constraints, matrices, scenarios, run, runs, exact_run, curated, references — or `full` for the complete dump (opt-in; can exceed token caps on large models). | | | `list` | List all problems with metadata snapshots | | | `delete` | Remove a problem and its data file | diff --git a/engine/models.py b/engine/models.py index f12d115..d43f7c5 100644 --- a/engine/models.py +++ b/engine/models.py @@ -125,6 +125,9 @@ class _Motivated(BaseModel): class CardinalityConstraint(_Motivated): + """How many options the plan selects. A whole-plan rule, so ONE row states it: several + rows apply intersected (max of the mins, min of the maxes), which `validate` echoes, + and an empty intersection is a validation error.""" type: Literal["cardinality"] = "cardinality" min: int max: int @@ -167,6 +170,9 @@ class GroupLimitConstraint(_Motivated): class MaxAllocationConstraint(_Motivated): + """One global cap on every option's allocation percentage. A whole-plan rule, so ONE + row states it: several rows apply as the tightest cap (the minimum), which `validate` + echoes. Per-option floors and caps belong on AllocationBoundConstraint.""" type: Literal["max_allocation"] = "max_allocation" max: int # maximum allocation percentage for any single option (1-100) diff --git a/engine/optimizer.py b/engine/optimizer.py index 0ec8ce9..37b6785 100644 --- a/engine/optimizer.py +++ b/engine/optimizer.py @@ -305,6 +305,60 @@ def allocation_bound_merges(constraints) -> list[dict]: for o, n in counts.items() if n > 1] +# ─── Whole-plan (singleton) constraint resolution ─── +# +# `max_allocation` and `cardinality` describe the WHOLE plan (one global per-option cap, +# one selection-count range), so several rows of either type resolve to a single applied +# value. Assigning row by row made that value order-dependent — and, worse, the engine and +# the validator picked different rows: `_parse_constraints` kept the LAST row while the +# conflict checks read the FIRST via `next(...)`, so a model could be hard-errored by +# validate on a cap the solver was never going to apply. Both sides now read these two +# functions, which is what keeps them in agreement; several rows intersect (the tightest +# combination), the way several `objective_bound` rows on one objective all bind. + + +def merged_max_allocation(constraints) -> int | None: + """The global per-option allocation cap actually applied: the tightest of every + ``max_allocation`` row (``None`` when the model carries none).""" + caps = [int(c.max) for c in (constraints or []) + if getattr(c, "type", "") == "max_allocation"] + return min(caps) if caps else None + + +def merged_cardinality(constraints) -> tuple[int, int] | None: + """The selection-count range actually applied: the intersection of every + ``cardinality`` row — max of the mins, min of the maxes (``None`` when the model + carries none, leaving the caller's own default in place). + + An empty intersection (merged min > merged max) is returned intact and reported by + ``validate``: two rows that cannot both hold are a conflict to name, not something to + clamp into a range neither row asked for. + """ + rows = [c for c in (constraints or []) if getattr(c, "type", "") == "cardinality"] + if not rows: + return None + return (max(int(c.min) for c in rows), min(int(c.max) for c in rows)) + + +def singleton_constraint_merges(constraints) -> list[dict]: + """The whole-plan constraint types that carried MORE than one row, each with the value + actually applied — the echo that keeps the resolution visible.""" + merges: list[dict] = [] + counts: dict[str, int] = {} + for c in (constraints or []): + t = getattr(c, "type", "") + if t in ("max_allocation", "cardinality"): + counts[t] = counts.get(t, 0) + 1 + if counts.get("max_allocation", 0) > 1: + merges.append({"type": "max_allocation", "rows": counts["max_allocation"], + "max": merged_max_allocation(constraints)}) + if counts.get("cardinality", 0) > 1: + lo, hi = merged_cardinality(constraints) + merges.append({"type": "cardinality", "rows": counts["cardinality"], + "min": lo, "max": hi}) + return merges + + def validate(problem: Problem) -> ValidationResult: """Check if a problem is ready to optimize.""" issues: list[ValidationIssue] = [] @@ -465,6 +519,15 @@ def validate(problem: Problem) -> ValidationResult: message="max_allocation constraint only applies to proportional mode; ignored in binary mode.", )) + for m in singleton_constraint_merges(problem.constraints): + if m["type"] == "max_allocation": + issues.append(ValidationIssue( + severity="warning", + message=(f"{m['rows']} max_allocation rows apply as the tightest cap " + f"(≤{m['max']}% per option). Send one max_allocation row to state " + "that directly."), + )) + # Check force_include + force_exclude conflict forced_in = {c.option for c in problem.constraints if c.type == "force_include"} forced_out = {c.option for c in problem.constraints if c.type == "force_exclude"} @@ -475,24 +538,45 @@ def validate(problem: Problem) -> ValidationResult: message=f"Options both force_include and force_exclude: {conflict}.", )) - # Check cardinality feasibility + # Check cardinality feasibility — against the range the solver will actually apply + # (the intersection of every row), so validate and the engine reason about one model. available = len(opt_names - forced_out) for c in problem.constraints: - if c.type == "cardinality": - if c.min > available: + if c.type == "cardinality" and c.min > c.max: + issues.append(ValidationIssue( + severity="error", + message=f"Cardinality min ({c.min}) > max ({c.max}).", + )) + card = merged_cardinality(problem.constraints) + if card is not None: + card_min, card_max = card + for m in singleton_constraint_merges(problem.constraints): + if m["type"] != "cardinality": + continue + if card_min > card_max: issues.append(ValidationIssue( severity="error", - message=f"Cardinality min ({c.min}) exceeds available options ({available}).", + message=(f"{m['rows']} cardinality rows intersect to an empty range " + f"(merged min {card_min} > merged max {card_max}) — the rows " + "cannot both hold. Send ONE cardinality row."), + )) + else: + issues.append(ValidationIssue( + severity="warning", + message=(f"{m['rows']} cardinality rows apply intersected, as the " + f"tightest range (select {card_min}–{card_max}). Send one " + "cardinality row to state that directly."), )) - if c.min > c.max: + if card_min <= card_max: + if card_min > available: issues.append(ValidationIssue( severity="error", - message=f"Cardinality min ({c.min}) > max ({c.max}).", + message=f"Cardinality min ({card_min}) exceeds available options ({available}).", )) - if len(forced_in) > c.max: + if len(forced_in) > card_max: issues.append(ValidationIssue( severity="error", - message=f"force_include count ({len(forced_in)}) exceeds cardinality max ({c.max}).", + message=f"force_include count ({len(forced_in)}) exceeds cardinality max ({card_max}).", )) # ── 1.9 Pre-solve constraint conflict detection ── @@ -610,7 +694,8 @@ def _check_constraint_conflicts( # Floors vs the cardinality cap: sound only over pairwise-disjoint floored groups # (overlapping groups would double-count), so overlapping floors are left to the solver. if group_floors: - card_max = next((c.max for c in problem.constraints if c.type == "cardinality"), None) + merged_card = merged_cardinality(problem.constraints) + card_max = merged_card[1] if merged_card else None all_disjoint = all( not set(a.options) & set(b.options) for i, a in enumerate(group_floors) for b in group_floors[i + 1:] @@ -690,16 +775,17 @@ def _dfs(node: str, stack: list[str]) -> None: # E. MaxAllocation / allocation_bound arithmetic (proportional mode only) if problem.approach == Approach.proportional: - for c in problem.constraints: - if c.type == "max_allocation": - if c.max * available < 100: - issues.append(ValidationIssue( - severity="error", - message=( - f"max_allocation cap ({c.max}%) × available options ({available}) = " - f"{c.max * available}% < 100% required. Allocation cannot sum to 100." - ), - )) + # The APPLIED cap, not each row: checking rows one by one hard-errored models on a + # cap the solver would never apply (it took a different row). + applied_cap = merged_max_allocation(problem.constraints) + if applied_cap is not None and applied_cap * available < 100: + issues.append(ValidationIssue( + severity="error", + message=( + f"max_allocation cap ({applied_cap}%) × available options ({available}) = " + f"{applied_cap * available}% < 100% required. Allocation cannot sum to 100." + ), + )) ab = [c for c in problem.constraints if c.type == "allocation_bound"] if ab: # Several rows on one option apply intersected, so every arithmetic check below @@ -729,8 +815,7 @@ def _dfs(node: str, stack: list[str]) -> None: message=(f"allocation_bound floors sum to {floor_sum}% > 100% — " "the floors cannot all be met."), )) - global_cap_for_floors = next( - (c.max for c in problem.constraints if c.type == "max_allocation"), 100) + global_cap_for_floors = merged_max_allocation(problem.constraints) or 100 for name, (lo, hi) in bounded.items(): if lo > hi: continue # empty intersection — already named above, in its own terms @@ -749,8 +834,7 @@ def _dfs(node: str, stack: list[str]) -> None: message=(f"allocation_bound floor ({lo}%) on '{name}' conflicts " "with force_exclude on the same option."), )) - global_cap = next((c.max for c in problem.constraints if c.type == "max_allocation"), - None) + global_cap = merged_max_allocation(problem.constraints) cap_sum = sum( min(global_cap or 100, bounded[o][1]) if o in bounded else (global_cap or 100) for o in (opt.name for opt in problem.options) if o not in forced_out @@ -802,9 +886,6 @@ def _opt_idx(name: str, c) -> int: forced_in_idx.add(_opt_idx(c.option, c)) elif c.type == "force_exclude": forced_out_idx.add(_opt_idx(c.option, c)) - elif c.type == "cardinality": - cardinality_min = c.min - cardinality_max = c.max elif c.type == "objective_bound": obj_idx = next((j for j, o in enumerate(obj_list) if o.name == c.objective), None) if obj_idx is None: @@ -819,8 +900,6 @@ def _opt_idx(name: str, c) -> int: elif c.type == "group_limit": group_indices = [_opt_idx(o, c) for o in c.options] group_limits.append((group_indices, int(c.min), int(c.max))) - elif c.type == "max_allocation": - max_allocation = c.max elif c.type == "allocation_bound": # Rows on the same option intersect (see merged_allocation_bounds) — assigning # into the dict would keep only the last one and drop the rest in silence. @@ -828,6 +907,14 @@ def _opt_idx(name: str, c) -> int: allocation_bounds[idx] = _intersect_bounds( allocation_bounds.get(idx), int(c.min), int(c.max)) + # Whole-plan constraints resolve through the shared functions the validator also + # reads, so the two sides can never reason about different rows (see the resolution + # note above them). A single row of either type resolves to itself. + card = merged_cardinality(problem.constraints) + if card is not None: + cardinality_min, cardinality_max = card + max_allocation = merged_max_allocation(problem.constraints) + return { "forced_in": forced_in_idx, "forced_out": forced_out_idx, diff --git a/mcp_server/server.py b/mcp_server/server.py index 6486874..85d8196 100644 --- a/mcp_server/server.py +++ b/mcp_server/server.py @@ -302,7 +302,10 @@ def model( "Several \"allocation_bound\" rows on ONE option apply intersected — the " "tightest box (max of the mins, min of the maxes), echoed as " "constraints_merged_note; an empty intersection is a validation " - "error.")] = None, + "error. The whole-plan types — \"max_allocation\" (one global per-option " + "cap) and \"cardinality\" (one selection-count range) — take ONE row " + "each the same way: several rows resolve to their tightest combination, " + "echoed in that same note.")] = None, approach: str | None = None, reference_points: Annotated[list[dict] | None, Field( description="On update: FULL REPLACEMENT — send the complete list.")] = None, @@ -346,6 +349,8 @@ def model( NOT applied at create — passing them errors with a pointer to update. `source` belongs to action='load' (it rebuilds a saved/example bundle), so passing it to create errors with a pointer there. + Constraints passed here are checked on the spot, like an update's: + the response carries validation_issues and constraints_merged_note. update — Modify problem. Params: problem_id (required), plus any of: name, domain, context, objectives, options, scores, constraints, approach ("binary" or "proportional"), @@ -489,6 +494,19 @@ def _model_create(params: dict) -> dict: "options": len(p.options), "constraints": len(p.constraints), } + # Constraints passed at create get the same self-certifying echo an update gives them + # — the merge note plus the non-scoring validation issues. Without it a one-shot + # framing call carrying, say, two cardinality rows learned what the model actually + # applies only on the next update, or at solve. + if "constraints" in params: + _attach_constraint_merge_note(result, p) + try: + issues = [json.loads(i.model_dump_json()) for i in optimizer.validate(p).issues + if "Score matrix incomplete" not in i.message] + if issues: + result["validation_issues"] = issues + except Exception: + pass # advisory, like the update path — never block a create on validation # Next step is scoring — inject data_collection guidance (throttled in _inject_skill) _inject_skill(result, "data_collection", "Problem created. Use this guide when entering scores — " @@ -497,6 +515,35 @@ def _model_create(params: dict) -> dict: return result +def _attach_constraint_merge_note(result: dict, p: Problem) -> None: + """Name what the model actually applies wherever several rows collapse into one rule. + + Two kinds collapse, and ONE note carries both: several `allocation_bound` rows on one + option intersect into a per-option box, and the whole-plan types (`max_allocation`, + `cardinality`) resolve to their tightest combination. In each case the applied rule is + not any single row the caller sent, so the response names it — the same self-certifying + echo `constraints_note` gives a shrinking constraint set, and it beats leaving the + difference for the allocations or the frontier to reveal. + + Both kinds route through this one builder deliberately: two writers assigning + `constraints_merged_note` independently is exactly how one of them goes silently + missing, which is the defect class these notes exist to prevent. + """ + parts = [] + for m in optimizer.allocation_bound_merges(p.constraints)[:5]: + parts.append(f"'{m['option']}' ({m['rows']} allocation_bound rows) → " + f"min {m['min']}%, max {m['max']}%") + for m in optimizer.singleton_constraint_merges(p.constraints): + applied = (f"≤{m['max']}% per option" if m["type"] == "max_allocation" + else f"select {m['min']}–{m['max']}") + parts.append(f"{m['rows']} {m['type']} rows → {applied}") + if not parts: + return + result["constraints_merged_note"] = ( + "Rules that collapse apply as their tightest combination (max of the mins, min of " + "the maxes): " + "; ".join(parts) + ". Send one row each to state that directly.") + + def _results_stale(p: Problem) -> bool: """Does any stored frontier predate the current model? Each frontier is compared against the fingerprint of the inputs IT reads — base runs against the base inputs, the scenario @@ -774,18 +821,7 @@ def _cells(m): "rules referencing removed options/objectives were dropped") + f". Dropped: {listed}.") - # Several allocation_bound rows on one option apply intersected, so the applied box is - # not any single row the caller sent — name it here rather than leaving the difference - # for the allocations to reveal. - merges = optimizer.allocation_bound_merges(p.constraints) - if merges: - listed = "; ".join( - f"'{m['option']}' ({m['rows']} rows) → min {m['min']}%, max {m['max']}%" - for m in merges[:5]) + (" …" if len(merges) > 5 else "") - result["constraints_merged_note"] = ( - "Options carrying several allocation_bound rows apply as the tightest box " - f"(max of the mins, min of the maxes): {listed}. Send one merged row per option " - "to state that directly.") + _attach_constraint_merge_note(result, p) if matrix_cells_echo: result["interaction_matrix_cells"] = matrix_cells_echo diff --git a/skills/problem_framing/references/schemas.md b/skills/problem_framing/references/schemas.md index f86f5e7..f60121e 100644 --- a/skills/problem_framing/references/schemas.md +++ b/skills/problem_framing/references/schemas.md @@ -7,14 +7,14 @@ JSON shapes for the structured fields passed through `model/create` and `model/u Pass to the `constraints` param as a list of dicts: ``` -{"type": "cardinality", "min": , "max": } +{"type": "cardinality", "min": , "max": } (whole-plan: ONE row states the selection count — several rows apply intersected, and an empty intersection is a validation error) {"type": "force_include", "option": ""} {"type": "force_exclude", "option": ""} {"type": "objective_bound", "objective": "", "operator": "min"|"max", "value": } {"type": "exclusion_pair", "option_a": "", "option_b": ""} {"type": "dependency", "if_option": "", "then_option": ""} {"type": "group_limit", "options": ["", ...], "max": } (optional "min": floor — at least that many selected/active from the group; exact-certifiable on binary, NSGA-only on proportional) -{"type": "max_allocation", "max": } (proportional only: one global cap on every option's allocation) +{"type": "max_allocation", "max": } (proportional only, whole-plan: ONE row states the global cap on every option's allocation — several rows apply as the tightest; per-option floors/caps belong on allocation_bound) {"type": "allocation_bound", "option": "", "min": , "max": } (proportional only: per-option floor/cap in percent — contractual minimums, service floors, per-channel caps; effective cap = min(global, per-option); a floor > 0 force-activates the option and carries a dual on the exact LP/QP path; several rows on ONE option apply intersected — the tightest box, max of the mins and min of the maxes — so a floor row sent beside a separate cap row keeps both, and an empty intersection is a validation error) ``` diff --git a/tests/test_optimizer.py b/tests/test_optimizer.py index a668073..f69402b 100644 --- a/tests/test_optimizer.py +++ b/tests/test_optimizer.py @@ -1221,6 +1221,114 @@ def test_cap_sum_reads_the_merged_cap(self): assert v.ready is False +# ─── Whole-plan (singleton) constraint resolution ─── + + +class TestSingletonConstraintMerging: + """`max_allocation` and `cardinality` describe the whole plan, so several rows resolve + to one applied value. They used to resolve to DIFFERENT values on the two sides: + `_parse_constraints` took the last row, the conflict checks took the first.""" + + def _props(self, cons, names="ABC"): + return Problem( + approach="proportional", + objectives=[Objective(name="Revenue", direction="maximize"), + Objective(name="Effort", direction="minimize")], + options=[Option(name=n) for n in names], + scores=[Score(option=n, objective=o, value=5) + for n in names for o in ("Revenue", "Effort")], + constraints=cons) + + def test_engine_and_validator_read_the_same_cap(self): + """The verified disagreement: rows 30 then 60 — the solver applied 60 while + validate reasoned about 30 and hard-errored a model the solver would have run.""" + from engine.models import MaxAllocationConstraint + from engine.optimizer import _parse_constraints, merged_max_allocation + + p = self._props([MaxAllocationConstraint(max=30), MaxAllocationConstraint(max=60)]) + assert _parse_constraints(p)["max_allocation"] == 30 # the tightest, not the last + assert merged_max_allocation(p.constraints) == 30 + v = validate(p) + # Both sides now say 30, and 30 × 3 options = 90% < 100% is a REAL infeasibility. + assert any("cap (30%)" in i.message and i.severity == "error" for i in v.issues) + + def test_cap_resolution_is_order_independent(self): + from engine.models import MaxAllocationConstraint + from engine.optimizer import _parse_constraints + + rows = [MaxAllocationConstraint(max=60), MaxAllocationConstraint(max=30)] + assert _parse_constraints(self._props(rows))["max_allocation"] == 30 + assert _parse_constraints(self._props(rows[::-1]))["max_allocation"] == 30 + + def test_engine_and_validator_read_the_same_cardinality(self): + """The mirror case: rows (1,3) and (1,5) — the solver applied max 5 while validate + rejected 4 force_includes against max 3.""" + from engine.optimizer import _parse_constraints, merged_cardinality + + p = _make_problem(constraints=[ + CardinalityConstraint(min=1, max=3), CardinalityConstraint(min=1, max=5), + *(ForceIncludeConstraint(option=n) for n in "ABCD")]) + cp = _parse_constraints(p) + assert (cp["cardinality_min"], cp["cardinality_max"]) == (1, 3) + assert merged_cardinality(p.constraints) == (1, 3) + # Validate's verdict now describes the model the solver runs: 4 forced > max 3. + assert any("exceeds cardinality max (3)" in i.message for i in validate(p).issues) + + def test_cardinality_rows_intersect(self): + from engine.optimizer import _parse_constraints + + cp = _parse_constraints(_make_problem(constraints=[ + CardinalityConstraint(min=2, max=4), CardinalityConstraint(min=1, max=3)])) + assert (cp["cardinality_min"], cp["cardinality_max"]) == (2, 3) + + def test_empty_cardinality_intersection_is_an_error(self): + v = validate(_make_problem(constraints=[ + CardinalityConstraint(min=2, max=3), CardinalityConstraint(min=4, max=5)])) + assert any("intersect to an empty range" in i.message and "min 4 > merged max 3" + in i.message and i.severity == "error" for i in v.issues) + + def test_merges_are_echoed_as_warnings(self): + from engine.models import MaxAllocationConstraint + from engine.optimizer import singleton_constraint_merges + + p = self._props([MaxAllocationConstraint(max=50), MaxAllocationConstraint(max=40), + CardinalityConstraint(min=1, max=3), + CardinalityConstraint(min=2, max=3)], names="ABC") + assert singleton_constraint_merges(p.constraints) == [ + {"type": "max_allocation", "rows": 2, "max": 40}, + {"type": "cardinality", "rows": 2, "min": 2, "max": 3}] + msgs = [i.message for i in validate(p).issues if i.severity == "warning"] + assert any("2 max_allocation rows apply as the tightest cap (≤40% per option)" in m + for m in msgs) + assert any("2 cardinality rows apply intersected" in m and "select 2–3" in m + for m in msgs) + + def test_single_rows_resolve_to_themselves(self): + """One row of either type keeps its exact pre-merge behavior, warning-free.""" + from engine.models import MaxAllocationConstraint + from engine.optimizer import _parse_constraints + + p = self._props([MaxAllocationConstraint(max=40), + CardinalityConstraint(min=1, max=2)]) + cp = _parse_constraints(p) + assert cp["max_allocation"] == 40 + assert (cp["cardinality_min"], cp["cardinality_max"]) == (1, 2) + assert not any("apply as the tightest" in i.message or "apply intersected" in i.message + for i in validate(p).issues) + + def test_group_floor_check_reads_the_merged_cardinality_max(self): + """The group-floor-vs-cardinality check took the FIRST row; with rows (1,5) then + (1,2) the applied max is 2, so disjoint floors summing to 3 are a real conflict.""" + from engine.models import GroupLimitConstraint + + v = validate(_make_problem(constraints=[ + CardinalityConstraint(min=1, max=5), CardinalityConstraint(min=1, max=2), + GroupLimitConstraint(options=["A", "B"], min=2, max=2), + GroupLimitConstraint(options=["C", "D"], min=1, max=2)])) + assert any("floors sum to 3, above the cardinality max (2)" in i.message + for i in v.issues) + + # ─── Elite preservation (Fix 5) ─── diff --git a/tests/test_server.py b/tests/test_server.py index 1f9ee55..27c0f01 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -217,7 +217,7 @@ def test_duplicate_allocation_bounds_are_merged_and_echoed(self): ]) assert r["status"]["constraints"] == 2 note = r["constraints_merged_note"] - assert "'A' (2 rows) → min 20%, max 40%" in note + assert "'A' (2 allocation_bound rows) → min 20%, max 40%" in note assert any("2 allocation_bound rows" in i["message"] and i["severity"] == "warning" for i in r["validation_issues"]) # The formulation reads the APPLIED box back, once, on both surfaces — the raw @@ -235,6 +235,68 @@ def test_duplicate_allocation_bounds_are_merged_and_echoed(self): clean_card = srv.model(action="get", problem_id=pid, section="summary") assert clean_card["viz_data"]["constraints"] == ["A allocation 20–40%"] + def test_whole_plan_constraint_merge_is_echoed(self): + """Several rows of a whole-plan type resolve to one applied value, which is not + any single row the caller sent — the SAME note names it.""" + pid = srv.model( + action="create", approach="proportional", + options=[{"name": "A"}, {"name": "B"}, {"name": "C"}, {"name": "D"}], + objectives=[{"name": "Value", "direction": "maximize"}, + {"name": "Cost", "direction": "minimize"}], + )["problem_id"] + r = srv.model(action="update", problem_id=pid, constraints=[ + {"type": "max_allocation", "max": 50}, + {"type": "max_allocation", "max": 40}, + {"type": "cardinality", "min": 1, "max": 3}, + {"type": "cardinality", "min": 2, "max": 4}, + ]) + note = r["constraints_merged_note"] + assert "2 max_allocation rows → ≤40% per option" in note + assert "2 cardinality rows → select 2–3" in note + assert any("apply as the tightest cap" in i["message"] and i["severity"] == "warning" + for i in r["validation_issues"]) + + # One row per type says nothing. + clean = srv.model(action="update", problem_id=pid, constraints=[ + {"type": "max_allocation", "max": 40}]) + assert "constraints_merged_note" not in clean + + def test_both_merge_kinds_share_one_note(self): + """One builder writes `constraints_merged_note`, so a model carrying both kinds + reports both. Two independent writers would have dropped one silently — the exact + failure class these notes exist to prevent.""" + pid = srv.model( + action="create", approach="proportional", + options=[{"name": "A"}, {"name": "B"}, {"name": "C"}, {"name": "D"}], + objectives=[{"name": "Value", "direction": "maximize"}, + {"name": "Cost", "direction": "minimize"}], + )["problem_id"] + note = srv.model(action="update", problem_id=pid, constraints=[ + {"type": "allocation_bound", "option": "A", "min": 20, "max": 100}, + {"type": "allocation_bound", "option": "A", "min": 0, "max": 40}, + {"type": "max_allocation", "max": 50}, + {"type": "max_allocation", "max": 45}, + ])["constraints_merged_note"] + assert "'A' (2 allocation_bound rows) → min 20%, max 40%" in note + assert "2 max_allocation rows → ≤45% per option" in note + + def test_create_checks_the_constraints_it_was_given(self): + """The create-time echo gap: constraints passed at create went unchecked until the + next update, so a one-shot framing call learned what its model applies at solve.""" + created = srv.model( + action="create", approach="proportional", + options=[{"name": "A"}, {"name": "B"}, {"name": "C"}], + objectives=[{"name": "Value", "direction": "maximize"}, + {"name": "Cost", "direction": "minimize"}], + constraints=[{"type": "cardinality", "min": 1, "max": 2}, + {"type": "cardinality", "min": 1, "max": 3}], + ) + assert "2 cardinality rows → select 1–2" in created["constraints_merged_note"] + assert any("apply intersected" in i["message"] for i in created["validation_issues"]) + # A create without constraints keeps its lean response. + bare = srv.model(action="create", options=[{"name": "A"}]) + assert "constraints_merged_note" not in bare and "validation_issues" not in bare + def test_option_removal_cascade_carries_constraints_note(self): """An options replacement that cascade-drops referencing constraints is named too — rules must never vanish without a callout.""" From 4c73fb72fea5340d1c22937f83f4aa3b26101829 Mon Sep 17 00:00:00 2001 From: cafzal Date: Mon, 3 Aug 2026 11:11:32 -0700 Subject: [PATCH 2/3] =?UTF-8?q?Route=20every=20consumer=20through=20the=20?= =?UTF-8?q?resolvers=20=E2=80=94=20the=20exact=20path=20most=20of=20all?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found the resolvers had consumers I missed, one of them blocking. BLOCKING — the exact path bypassed merged_cardinality. _build_milp_data still read a single row into mc["card"], and mc is what both exact backends encode, so the NSGA half intersected while the exact half kept the last row: a NEW engine-vs-engine disagreement (rows (1,2) then (1,5) — NSGA applies (1,2), HiGHS returned plans of size 1–5). explore certify compares exactly those two frontiers, so its optimality gap and its NSGA-never-dominates invariant would have been measured across two different feasible sets. mc["card"] now resolves through merged_cardinality and declines an empty intersection in words. Three guards, all verified to fail against the un-fixed encoding: the mc["card"] == merged_cardinality pin, the decline, and an end-to-end HiGHS frontier check (nothing in the suite exercised duplicate cardinality on the exact path, which is why it was green despite the bug). analyze_infeasibility diagnosed an empty search against a range the search never applied, and appended a cardinality entry per row — duplicating the binding rule AND its suggestion. Now resolver-backed, with one cardinality entry however many rows, in the per-rule list and the jointly-infeasible fallback alike. Verified: rows (1,2)+(1,5) with three force_includes used to blame cardinality twice; it now names the forced trio, which is the actual conflict. solution_quality read the FIRST max_allocation row for its at-a-bound flag (the fourth next(...) of the family), so with rows 60 then 30 the flag reasoned with 60 while the engine applied 30 and silently missed pinned allocations. Nits: the merge note skips a cardinality pair with an empty intersection (it had rendered "select 4–2" under "their tightest combination" — the validation error beside it says that case correctly); the max_allocation merge warning is gated on proportional, so a binary model no longer hears the applied value of a constraint just declared ignored. --- architecture.md | 2 +- engine/explorer.py | 6 ++-- engine/optimizer.py | 54 ++++++++++++++++++++++---------- mcp_server/server.py | 18 ++++++----- solvers/_scalarization.py | 16 ++++++++-- tests/test_highs_backend.py | 17 +++++++++++ tests/test_optimizer.py | 61 +++++++++++++++++++++++++++++++++++++ 7 files changed, 145 insertions(+), 29 deletions(-) diff --git a/architecture.md b/architecture.md index 0aa5cce..cc9eda4 100644 --- a/architecture.md +++ b/architecture.md @@ -24,7 +24,7 @@ Frontier exposes 4 tools – 3 domain tools with multiple actions, plus a skill | Tool | Action | Purpose | |------|--------|---------| | **model** | `create` | Start a new optimization problem (name, domain, context, approach). Constraints passed here are checked on the spot, like an update's — the response carries `validation_issues` and `constraints_merged_note`. | -| | `update` | Add/modify objectives (≥2 enforced; 2–7 is the designed envelope), options, scores, constraints, reference points, scenarios. Scores and interaction matrices merge (upsert) — matrices by objective, and cell-wise within a matrix when `mode="upsert"`, so one larger than a single tool call is built across several (same merge as scenario overrides, via `optimizer.apply_matrix_override`); objectives, options, constraints, and reference points are full replacement — the contract is restated on each data param's schema description, `status` echoes the constraint count, and any update that *shrinks* the constraint set (a partial replacement list, or an objectives/options replacement cascade-dropping referencing rules) carries a `constraints_note` so rules can't silently vanish; several `allocation_bound` rows on ONE option apply *intersected* (the tightest box — max of the mins, min of the maxes, the way several `objective_bound` rows on one objective all bind), echoed as `constraints_merged_note`, reported by `validate` (a warning per merged option, an error when the intersection is empty), and read back as the applied box — one line per option, captioned with the row count — in `get`'s formulation card and its ASCII twin, so a floor row sent beside a cap row keeps both; the whole-plan types take one row each — several `max_allocation` rows resolve to the tightest cap and several `cardinality` rows to the intersection of their ranges (`optimizer.merged_max_allocation` / `merged_cardinality`, the single resolvers BOTH the solve encoding and the pre-solve checks read, so the two can't reason about different rows), echoed as `constraints_merged_note` with a `validate` warning per merged type and an error on an empty cardinality intersection; matrix writes echo `interaction_matrix_cells` per touched objective (post-merge cell count, plus a note naming the cause when the count *fell* — the signature of a chunk sent without `mode="upsert"` wiping its predecessors). Marks results stale on structural changes — each stored frontier compared against the fingerprint of the inputs *it* reads (base runs against the base inputs; the scenario set against those plus `scenario_config`), so a scenarios-only edit never flags a base frontier it cannot affect. | +| | `update` | Add/modify objectives (≥2 enforced; 2–7 is the designed envelope), options, scores, constraints, reference points, scenarios. Scores and interaction matrices merge (upsert) — matrices by objective, and cell-wise within a matrix when `mode="upsert"`, so one larger than a single tool call is built across several (same merge as scenario overrides, via `optimizer.apply_matrix_override`); objectives, options, constraints, and reference points are full replacement — the contract is restated on each data param's schema description, `status` echoes the constraint count, and any update that *shrinks* the constraint set (a partial replacement list, or an objectives/options replacement cascade-dropping referencing rules) carries a `constraints_note` so rules can't silently vanish; several `allocation_bound` rows on ONE option apply *intersected* (the tightest box — max of the mins, min of the maxes, the way several `objective_bound` rows on one objective all bind), echoed as `constraints_merged_note`, reported by `validate` (a warning per merged option, an error when the intersection is empty), and read back as the applied box — one line per option, captioned with the row count — in `get`'s formulation card and its ASCII twin, so a floor row sent beside a cap row keeps both; the whole-plan types take one row each — several `max_allocation` rows resolve to the tightest cap and several `cardinality` rows to the intersection of their ranges (`optimizer.merged_max_allocation` / `merged_cardinality`, the single resolvers every consumer reads — the NSGA encoding, the exact backends' MILP data, the pre-solve checks, the infeasibility diagnosis, and the allocation quality flags — so no two of them can reason about different rows), echoed as `constraints_merged_note` with a `validate` warning per merged type and an error on an empty cardinality intersection; matrix writes echo `interaction_matrix_cells` per touched objective (post-merge cell count, plus a note naming the cause when the count *fell* — the signature of a chunk sent without `mode="upsert"` wiping its predecessors). Marks results stale on structural changes — each stored frontier compared against the fingerprint of the inputs *it* reads (base runs against the base inputs; the scenario set against those plus `scenario_config`), so a scenarios-only edit never flags a base frontier it cannot affect. | | | `get` | Return problem state. Defaults to the `summary` slice (counts + status flags + the decision-question `context` — always small). Optional `section` for targeted slices: summary, objectives, options, scores, constraints, matrices, scenarios, run, runs, exact_run, curated, references — or `full` for the complete dump (opt-in; can exceed token caps on large models). | | | `list` | List all problems with metadata snapshots | | | `delete` | Remove a problem and its data file | diff --git a/engine/explorer.py b/engine/explorer.py index 21cbd16..cb80b8e 100644 --- a/engine/explorer.py +++ b/engine/explorer.py @@ -1909,8 +1909,10 @@ def solution_quality(problem: Problem, selected_options: list[str], allocations: "if you expected a spread, add a max_allocation cap or revisit the " "scores/interactions that let one option dominate", }) - cap = next((c.max for c in problem.constraints or [] - if getattr(c, "type", "") == "max_allocation"), 100) + # The APPLIED cap (the tightest row), so the at-a-bound test measures allocations + # against the same edge the solver placed them on. + from .optimizer import merged_max_allocation + cap = merged_max_allocation(problem.constraints) or 100 if n >= 3: at_bounds = sum(1 for o in problem.options if alloc.get(o.name, 0) in (0, cap)) if at_bounds >= 0.9 * n: diff --git a/engine/optimizer.py b/engine/optimizer.py index 37b6785..cef3301 100644 --- a/engine/optimizer.py +++ b/engine/optimizer.py @@ -519,14 +519,18 @@ def validate(problem: Problem) -> ValidationResult: message="max_allocation constraint only applies to proportional mode; ignored in binary mode.", )) - for m in singleton_constraint_merges(problem.constraints): - if m["type"] == "max_allocation": - issues.append(ValidationIssue( - severity="warning", - message=(f"{m['rows']} max_allocation rows apply as the tightest cap " - f"(≤{m['max']}% per option). Send one max_allocation row to state " - "that directly."), - )) + # Proportional only: naming the applied cap of a constraint the block above just + # declared ignored in binary mode would talk past its own advice (the same gate + # allocation_bound's merge notice uses). + if problem.approach == Approach.proportional: + for m in singleton_constraint_merges(problem.constraints): + if m["type"] == "max_allocation": + issues.append(ValidationIssue( + severity="warning", + message=(f"{m['rows']} max_allocation rows apply as the tightest cap " + f"(≤{m['max']}% per option). Send one max_allocation row to " + "state that directly."), + )) # Check force_include + force_exclude conflict forced_in = {c.option for c in problem.constraints if c.type == "force_include"} @@ -2873,11 +2877,14 @@ def analyze_infeasibility(problem: Problem) -> dict: # Floor of 1 mirrors the EA's search_floor: this diagnoses why the SEARCH came # back empty, so it reasons over the plans the search actually proposes. card_min, card_max = 1, n_options + # Through the resolver, like the rest of the stack: this explains why the SEARCH came + # back empty, so it has to reason about the range the search actually applied. + card = merged_cardinality(problem.constraints) + if card is not None: + card_min, card_max = card obj_bounds = [] for c in problem.constraints: - if c.type == "cardinality": - card_min, card_max = c.min, c.max - elif c.type == "objective_bound": + if c.type == "objective_bound": obj_bounds.append((c.objective, c.operator, c.value)) # Build interaction matrix lookup for quadratic objectives @@ -2978,12 +2985,21 @@ def has_feasible(skip_constraint=None) -> bool: return False # can't tell, assume not return False - # Build constraint labels for testing + # Build constraint labels for testing. Cardinality gets ONE entry however many rows the + # model carries — they resolve to a single applied range, and skipping "cardinality" + # lifts all of them at once, so a per-row entry duplicated both the binding rule and its + # suggestion. Several rows report the applied range; a single row reports itself (its + # motivated_by and all). + card_rows = [c for c in problem.constraints if c.type == "cardinality"] constraint_labels = [] + if card_rows: + constraint_labels.append(( + "cardinality", + card_rows[0].model_dump() if len(card_rows) == 1 + else {"type": "cardinality", "min": card_min, "max": card_max}, + )) for c in problem.constraints: - if c.type == "cardinality": - constraint_labels.append(("cardinality", c.model_dump())) - elif c.type == "force_include": + if c.type == "force_include": constraint_labels.append((f"force_include:{c.option}", c.model_dump())) elif c.type == "force_exclude": constraint_labels.append((f"force_exclude:{c.option}", c.model_dump())) @@ -3034,7 +3050,13 @@ def has_feasible(skip_constraint=None) -> bool: suggestions.append(f"Relaxing group limit may help.") if not binding: - binding = [c.model_dump() for c in problem.constraints] + # Nothing isolated the failure, so hand back the whole rule set — as the model the + # search ran: the cardinality rows collapse to the one applied range, the way the + # per-rule entries above do. + binding = [c.model_dump() for c in problem.constraints if c.type != "cardinality"] + if card_rows: + binding.insert(0, card_rows[0].model_dump() if len(card_rows) == 1 + else {"type": "cardinality", "min": card_min, "max": card_max}) suggestions = ["Constraints may be jointly infeasible. Try relaxing multiple constraints."] result = {"binding_constraints": binding, "suggestions": suggestions} diff --git a/mcp_server/server.py b/mcp_server/server.py index 85d8196..8ba5bf2 100644 --- a/mcp_server/server.py +++ b/mcp_server/server.py @@ -529,14 +529,18 @@ def _attach_constraint_merge_note(result: dict, p: Problem) -> None: `constraints_merged_note` independently is exactly how one of them goes silently missing, which is the defect class these notes exist to prevent. """ - parts = [] - for m in optimizer.allocation_bound_merges(p.constraints)[:5]: - parts.append(f"'{m['option']}' ({m['rows']} allocation_bound rows) → " - f"min {m['min']}%, max {m['max']}%") + # Empty intersections are skipped on BOTH kinds: "min 60%, max 40%" or "select 4–2", + # captioned as a tightest combination, describes a rule the model applies. The + # validation error beside this note states those cases correctly — leave it to. + parts = [f"'{m['option']}' ({m['rows']} allocation_bound rows) → " + f"min {m['min']}%, max {m['max']}%" + for m in optimizer.allocation_bound_merges(p.constraints)[:5] + if m["min"] <= m["max"]] for m in optimizer.singleton_constraint_merges(p.constraints): - applied = (f"≤{m['max']}% per option" if m["type"] == "max_allocation" - else f"select {m['min']}–{m['max']}") - parts.append(f"{m['rows']} {m['type']} rows → {applied}") + if m["type"] == "max_allocation": + parts.append(f"{m['rows']} max_allocation rows → ≤{m['max']}% per option") + elif m["min"] <= m["max"]: + parts.append(f"{m['rows']} cardinality rows → select {m['min']}–{m['max']}") if not parts: return result["constraints_merged_note"] = ( diff --git a/solvers/_scalarization.py b/solvers/_scalarization.py index d407a3e..74906a8 100644 --- a/solvers/_scalarization.py +++ b/solvers/_scalarization.py @@ -138,11 +138,21 @@ def _ref(name, kind, c): return table[name] mc = {"card": None, "bounds": [], "force_in": [], "force_out": [], "deps": [], "excl": [], "groups": []} + # Cardinality resolves through the engine's own resolver, exactly as the NSGA encoding + # and the pre-solve checks do — several rows apply intersected. Reading a single row + # here would encode a DIFFERENT feasible set into the exact backends than the heuristic + # pass searched, and `explore certify` compares those two frontiers: an optimality gap + # and a never-dominates invariant measured across two feasible sets certify nothing. + mc["card"] = _opt.merged_cardinality(problem.constraints) + if mc["card"] is not None and mc["card"][0] > mc["card"][1]: + lo, hi = mc["card"] + raise ValueError( + f"cardinality rows intersect to an empty range (merged min {lo} > merged max " + f"{hi}) — no plan can satisfy them. Send ONE cardinality row (model update), " + "then retry.") for c in (problem.constraints or []): t = c.type - if t == "cardinality": - mc["card"] = (int(c.min), int(c.max)) - elif t == "objective_bound": + if t == "objective_bound": mc["bounds"].append((S[:, _ref(c.objective, "objective", c)].copy(), c.operator, float(c.value))) elif t == "force_include": mc["force_in"].append(_ref(c.option, "option", c)) diff --git a/tests/test_highs_backend.py b/tests/test_highs_backend.py index 865cfde..ac078d9 100644 --- a/tests/test_highs_backend.py +++ b/tests/test_highs_backend.py @@ -308,6 +308,23 @@ def test_respects_cardinality_and_force_include(self): assert 2 <= len(s.selected_options) <= 3 assert "A" in s.selected_options + def test_duplicate_cardinality_rows_bind_the_exact_frontier(self): + """End to end: the exact frontier obeys the SAME merged range the NSGA pass + searched. Encoding one row here (the last, max 6) returns plans the heuristic + frontier could never contain — and `explore certify` compares those two frontiers, + so its optimality gap and never-dominates invariant would span two feasible sets.""" + from engine.optimizer import merged_cardinality + + p = _binary_problem(constraints=[CardinalityConstraint(min=1, max=2), + CardinalityConstraint(min=1, max=6)]) + assert merged_cardinality(p.constraints) == (1, 2) + exact = _optimize_highs(p, mode=OptimizeMode.fast) + assert exact.solutions + assert all(1 <= len(s.selected_options) <= 2 for s in exact.solutions) + # The heuristic half agrees, which is what makes the two frontiers comparable. + nsga = optimize(p, mode=OptimizeMode.fast, seed=3) + assert all(1 <= len(s.selected_options) <= 2 for s in nsga.solutions) + def test_deterministic(self): p = _binary_problem() a = _optimize_highs(p, mode=OptimizeMode.fast) diff --git a/tests/test_optimizer.py b/tests/test_optimizer.py index f69402b..420a935 100644 --- a/tests/test_optimizer.py +++ b/tests/test_optimizer.py @@ -1316,6 +1316,67 @@ def test_single_rows_resolve_to_themselves(self): assert not any("apply as the tightest" in i.message or "apply intersected" in i.message for i in validate(p).issues) + def test_exact_encoding_reads_the_same_resolver(self): + """The exact backends encode `_build_milp_data`'s `mc`, so a row read there hands + HiGHS/cuOpt a different feasible set than the NSGA pass searched — and `explore + certify` measures its optimality gap and its never-dominates invariant ACROSS those + two frontiers. Pinned here so the exact path can't drift again.""" + from engine.optimizer import merged_cardinality + from solvers._scalarization import _build_milp_data + + p = _make_problem(constraints=[ + CardinalityConstraint(min=1, max=2), CardinalityConstraint(min=1, max=5)]) + mc = _build_milp_data(p)[-1] + assert mc["card"] == merged_cardinality(p.constraints) == (1, 2) + + # A single row still encodes itself. + one = _make_problem(constraints=[CardinalityConstraint(min=2, max=3)]) + assert _build_milp_data(one)[-1]["card"] == (2, 3) + + def test_exact_encoding_declines_an_empty_cardinality_intersection(self): + """No plan satisfies it, so the exact path declines in words instead of encoding an + empty model and reporting the infeasibility as a solver outcome.""" + from solvers._scalarization import _build_milp_data + + p = _make_problem(constraints=[ + CardinalityConstraint(min=4, max=5), CardinalityConstraint(min=1, max=2)]) + with pytest.raises(ValueError, match="empty range"): + _build_milp_data(p) + + def test_infeasibility_diagnosis_reads_the_applied_range(self): + """analyze_infeasibility explains why the SEARCH came back empty, so it has to + reason about the range the search applied. Rows (1,2) then (1,5) with three + force_includes: the applied max is 2, so the forced trio is the real conflict. + Reading the last row (max 5) instead described a model where the trio fits, and + blamed cardinality — twice, once per row, suggestion and all.""" + p = _make_problem(constraints=[ + CardinalityConstraint(min=1, max=2), CardinalityConstraint(min=1, max=5), + *(ForceIncludeConstraint(option=n) for n in "ABC")]) + d = analyze_infeasibility(p) + assert not any("Relaxing cardinality" in s for s in d["suggestions"]) + assert [c.get("type") for c in d["binding_constraints"]] == ["force_include"] * 3 + # One cardinality entry however many rows, wherever the diagnosis names it. + assert sum(c.get("type") == "cardinality" + for c in analyze_infeasibility(_make_problem(constraints=[ + CardinalityConstraint(min=4, max=4), + CardinalityConstraint(min=4, max=5), + ForceExcludeConstraint(option="A"), + ForceExcludeConstraint(option="B"), + ForceExcludeConstraint(option="C")]))["binding_constraints"]) <= 1 + + def test_quality_flag_reads_the_applied_cap(self): + """solution_quality's at-a-bound check measures allocations against the cap the + solver placed them on — reading the first row (60) missed them silently.""" + from engine.explorer import solution_quality + from engine.models import MaxAllocationConstraint + + p = self._props([MaxAllocationConstraint(max=60), MaxAllocationConstraint(max=30)], + names="ABCD") + alloc = {"A": 30, "B": 30, "C": 30, "D": 0} # three at the applied cap, one at 0 + flags = solution_quality(p, [k for k, v in alloc.items() if v], alloc)["flags"] + assert any(f["check"] == "allocations_at_bounds" and "30%" in f["message"] + for f in flags) + def test_group_floor_check_reads_the_merged_cardinality_max(self): """The group-floor-vs-cardinality check took the FIRST row; with rows (1,5) then (1,2) the applied max is 2, so disjoint floors summing to 3 are a real conflict.""" From cffd78349f6163ee5dcdbb8b22e0fc499a179e5f Mon Sep 17 00:00:00 2001 From: cafzal Date: Mon, 3 Aug 2026 12:01:36 -0700 Subject: [PATCH 3/3] Fold both merge kinds into one note builder and one read path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rebase resolution onto #132 plus the read-path gap the fold surfaced. _attach_constraint_merge_note is the single writer of constraints_merged_note, carrying #132's per-option allocation_bound boxes AND the whole-plan resolutions, called from create and update alike — two independent writers of one key is how one of them goes silently missing, which is the defect class these notes exist to prevent. Empty intersections are skipped on both kinds now: a note reports what the model applies, and the validation error beside it states the conflict. _formatted_constraints collapses every rule that collapses, not just allocation_bound: six rows in, three applied lines out, each captioned with the rows behind it. Rendering max_allocation and cardinality raw described a model the solver would not run — the same read-path defect #132 fixed one type at a time. --- architecture.md | 2 +- mcp_server/server.py | 48 ++++++++++++++++++++++++++++---------------- tests/test_server.py | 35 ++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 18 deletions(-) diff --git a/architecture.md b/architecture.md index cc9eda4..6ce964f 100644 --- a/architecture.md +++ b/architecture.md @@ -24,7 +24,7 @@ Frontier exposes 4 tools – 3 domain tools with multiple actions, plus a skill | Tool | Action | Purpose | |------|--------|---------| | **model** | `create` | Start a new optimization problem (name, domain, context, approach). Constraints passed here are checked on the spot, like an update's — the response carries `validation_issues` and `constraints_merged_note`. | -| | `update` | Add/modify objectives (≥2 enforced; 2–7 is the designed envelope), options, scores, constraints, reference points, scenarios. Scores and interaction matrices merge (upsert) — matrices by objective, and cell-wise within a matrix when `mode="upsert"`, so one larger than a single tool call is built across several (same merge as scenario overrides, via `optimizer.apply_matrix_override`); objectives, options, constraints, and reference points are full replacement — the contract is restated on each data param's schema description, `status` echoes the constraint count, and any update that *shrinks* the constraint set (a partial replacement list, or an objectives/options replacement cascade-dropping referencing rules) carries a `constraints_note` so rules can't silently vanish; several `allocation_bound` rows on ONE option apply *intersected* (the tightest box — max of the mins, min of the maxes, the way several `objective_bound` rows on one objective all bind), echoed as `constraints_merged_note`, reported by `validate` (a warning per merged option, an error when the intersection is empty), and read back as the applied box — one line per option, captioned with the row count — in `get`'s formulation card and its ASCII twin, so a floor row sent beside a cap row keeps both; the whole-plan types take one row each — several `max_allocation` rows resolve to the tightest cap and several `cardinality` rows to the intersection of their ranges (`optimizer.merged_max_allocation` / `merged_cardinality`, the single resolvers every consumer reads — the NSGA encoding, the exact backends' MILP data, the pre-solve checks, the infeasibility diagnosis, and the allocation quality flags — so no two of them can reason about different rows), echoed as `constraints_merged_note` with a `validate` warning per merged type and an error on an empty cardinality intersection; matrix writes echo `interaction_matrix_cells` per touched objective (post-merge cell count, plus a note naming the cause when the count *fell* — the signature of a chunk sent without `mode="upsert"` wiping its predecessors). Marks results stale on structural changes — each stored frontier compared against the fingerprint of the inputs *it* reads (base runs against the base inputs; the scenario set against those plus `scenario_config`), so a scenarios-only edit never flags a base frontier it cannot affect. | +| | `update` | Add/modify objectives (≥2 enforced; 2–7 is the designed envelope), options, scores, constraints, reference points, scenarios. Scores and interaction matrices merge (upsert) — matrices by objective, and cell-wise within a matrix when `mode="upsert"`, so one larger than a single tool call is built across several (same merge as scenario overrides, via `optimizer.apply_matrix_override`); objectives, options, constraints, and reference points are full replacement — the contract is restated on each data param's schema description, `status` echoes the constraint count, and any update that *shrinks* the constraint set (a partial replacement list, or an objectives/options replacement cascade-dropping referencing rules) carries a `constraints_note` so rules can't silently vanish; several `allocation_bound` rows on ONE option apply *intersected* (the tightest box — max of the mins, min of the maxes, the way several `objective_bound` rows on one objective all bind), echoed as `constraints_merged_note`, reported by `validate` (a warning per merged option, an error when the intersection is empty), and read back as the applied box — one line per option, captioned with the row count — in `get`'s formulation card and its ASCII twin (which renders EVERY collapsing rule as the rule applied, whole-plan types included), so a floor row sent beside a cap row keeps both; the whole-plan types take one row each — several `max_allocation` rows resolve to the tightest cap and several `cardinality` rows to the intersection of their ranges (`optimizer.merged_max_allocation` / `merged_cardinality`, the single resolvers every consumer reads — the NSGA encoding, the exact backends' MILP data, the pre-solve checks, the infeasibility diagnosis, and the allocation quality flags — so no two of them can reason about different rows), echoed in that same `constraints_merged_note` (one builder writes it, so a model carrying both kinds reports both) with a `validate` warning per merged type and an error on an empty cardinality intersection; matrix writes echo `interaction_matrix_cells` per touched objective (post-merge cell count, plus a note naming the cause when the count *fell* — the signature of a chunk sent without `mode="upsert"` wiping its predecessors). Marks results stale on structural changes — each stored frontier compared against the fingerprint of the inputs *it* reads (base runs against the base inputs; the scenario set against those plus `scenario_config`), so a scenarios-only edit never flags a base frontier it cannot affect. | | | `get` | Return problem state. Defaults to the `summary` slice (counts + status flags + the decision-question `context` — always small). Optional `section` for targeted slices: summary, objectives, options, scores, constraints, matrices, scenarios, run, runs, exact_run, curated, references — or `full` for the complete dump (opt-in; can exceed token caps on large models). | | | `list` | List all problems with metadata snapshots | | | `delete` | Remove a problem and its data file | diff --git a/mcp_server/server.py b/mcp_server/server.py index 8ba5bf2..d0542d2 100644 --- a/mcp_server/server.py +++ b/mcp_server/server.py @@ -983,32 +983,46 @@ def _format_constraint(c, units: dict | None = None) -> str: def _formatted_constraints(p: Problem) -> list[str]: """Constraint lines for the formulation card and its ASCII twin. - Several ``allocation_bound`` rows on one option apply INTERSECTED, so the formulation - states the applied box once per option (naming how many rows produced it) instead of - the raw rows: the card's job is to describe the problem the solver will run, and two - raw lines — "A allocation 20–100%" and "A allocation 0–40%" — describe neither. Every - other type renders row for row; none of them carries a per-option merge. + Every rule that COLLAPSES renders as the rule actually applied, once, captioned with + the number of rows behind it: several ``allocation_bound`` rows on one option intersect + into a per-option box, and the whole-plan types (``max_allocation``, ``cardinality``) + resolve to their tightest combination. The card's job is to describe the problem the + solver will run, and raw lines — "A allocation 20–100%" beside "A allocation 0–40%", + or "≤50% per option" beside "≤45% per option" — describe neither. Every other type + renders row for row; none of them collapses. """ units = {o.name: o.unit for o in p.objectives} merged = optimizer.merged_allocation_bounds(p.constraints) + card = optimizer.merged_cardinality(p.constraints) + cap = optimizer.merged_max_allocation(p.constraints) counts: dict[str, int] = {} for c in p.constraints: - if c.type == "allocation_bound": - counts[c.option] = counts.get(c.option, 0) + 1 + key = c.option if c.type == "allocation_bound" else c.type + counts[key] = counts.get(key, 0) + 1 + + def _caption(line: str, key: str) -> str: + return line + (f" ({counts[key]} rows merged)" if counts[key] > 1 else "") + lines: list[str] = [] shown: set[str] = set() for c in p.constraints: - if c.type != "allocation_bound": + if c.type == "allocation_bound": + if c.option in shown: + continue + shown.add(c.option) + lo, hi = merged[c.option] + lines.append(_caption(_format_constraint( + {"type": "allocation_bound", "option": c.option, "min": lo, "max": hi}, + units), c.option)) + elif c.type in ("max_allocation", "cardinality"): + if c.type in shown: + continue + shown.add(c.type) + applied = ({"type": "max_allocation", "max": cap} if c.type == "max_allocation" + else {"type": "cardinality", "min": card[0], "max": card[1]}) + lines.append(_caption(_format_constraint(applied, units), c.type)) + else: lines.append(_format_constraint(c, units)) - continue - if c.option in shown: - continue - shown.add(c.option) - lo, hi = merged[c.option] - line = _format_constraint( - {"type": "allocation_bound", "option": c.option, "min": lo, "max": hi}, units) - lines.append(line + (f" ({counts[c.option]} rows merged)" - if counts[c.option] > 1 else "")) return lines diff --git a/tests/test_server.py b/tests/test_server.py index 27c0f01..8dd3ee0 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -276,9 +276,44 @@ def test_both_merge_kinds_share_one_note(self): {"type": "allocation_bound", "option": "A", "min": 0, "max": 40}, {"type": "max_allocation", "max": 50}, {"type": "max_allocation", "max": 45}, + {"type": "cardinality", "min": 1, "max": 3}, + {"type": "cardinality", "min": 2, "max": 4}, ])["constraints_merged_note"] assert "'A' (2 allocation_bound rows) → min 20%, max 40%" in note assert "2 max_allocation rows → ≤45% per option" in note + assert "2 cardinality rows → select 2–3" in note + + # The formulation reads every collapsed rule back as the rule applied — six rows + # in, three applied lines out, each captioned with the rows behind it. + card = srv.model(action="get", problem_id=pid, section="summary") + assert card["viz_data"]["constraints"] == [ + "A allocation 20–40% (2 rows merged)", + "≤45% per option (2 rows merged)", + "select 2–3 (2 rows merged)", + ] + for raw in ("≤50% per option", "select 1–3", "select 2–4"): + assert raw not in card["visualization"] + + def test_empty_intersections_are_left_to_the_validation_error(self): + """The note reports resolutions; "select 4–2" or "min 60%, max 40%" captioned as a + tightest combination describes a rule the model applies. The errors beside the note + state those cases correctly, so the note stays quiet on them.""" + pid = srv.model( + action="create", approach="proportional", + options=[{"name": "A"}, {"name": "B"}, {"name": "C"}], + objectives=[{"name": "Value", "direction": "maximize"}, + {"name": "Cost", "direction": "minimize"}], + )["problem_id"] + r = srv.model(action="update", problem_id=pid, constraints=[ + {"type": "cardinality", "min": 4, "max": 5}, + {"type": "cardinality", "min": 1, "max": 2}, + {"type": "allocation_bound", "option": "A", "min": 60, "max": 100}, + {"type": "allocation_bound", "option": "A", "min": 0, "max": 40}, + ]) + assert "constraints_merged_note" not in r + msgs = [i["message"] for i in r["validation_issues"] if i["severity"] == "error"] + assert any("empty range" in m and "min 4 > merged max 2" in m for m in msgs) + assert any("empty box" in m and "60% > merged max 40%" in m for m in msgs) def test_create_checks_the_constraints_it_was_given(self): """The create-time echo gap: constraints passed at create went unchecked until the