diff --git a/architecture.md b/architecture.md index 0eac250..6ce964f 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 (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/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/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..cef3301 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,19 @@ def validate(problem: Problem) -> ValidationResult: message="max_allocation constraint only applies to proportional mode; ignored in binary mode.", )) + # 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"} forced_out = {c.option for c in problem.constraints if c.type == "force_exclude"} @@ -475,24 +542,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."), )) - if c.min > c.max: + 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 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 +698,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 +779,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 +819,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 +838,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 +890,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 +904,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 +911,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, @@ -2786,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 @@ -2891,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())) @@ -2947,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 6486874..d0542d2 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,39 @@ 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. + """ + # 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): + 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"] = ( + "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 +825,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 @@ -943,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/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/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 a668073..420a935 100644 --- a/tests/test_optimizer.py +++ b/tests/test_optimizer.py @@ -1221,6 +1221,175 @@ 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_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 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..8dd3ee0 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,103 @@ 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}, + {"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 + 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."""