fix: reject update payloads the padding would silently misread

CLAUDE.md

@@ -103,6 +103,15 @@ guards inputs by walking `Model.keywords`, which defaults to the keys of
 or a caller who omits that keyword gets a bare `KeyError` rather than a
 `CvxDataError`.
 
+**A model must also declare the dimensions its data implies.**
+`Model.dimensions` is abstract for the same reason `keywords` exists: `update`
+zero-pads short input up to the compiled size, so a payload that describes two
+assets to the risk model and four to the bounds does not fail on its own -- it
+leaves the padded tail riskless and unbounded and the solver empties the
+portfolio into it. `Problem.update` collects the `(variable, size)` claims of
+every model and rejects the payload before writing a single value. A model that
+consumes a keyword without declaring its size reopens that hole silently.
+
 **Everything raised derives from `CvxError`.** `CvxDataError`, `CvxBuildError`
 and `CvxSolverError` split the failure modes by whether retrying helps; the
 table in `README.md` is the contract. `tests/test_markowitz/test_public_api.py`

README.md

@@ -28,6 +28,14 @@ the number of assets fixed by setting the weights for the assets not used to
 zero. Hence we do **not** need to recompile the problem as a new asset has to be
 added.
 
+The padding is one-directional and it has to be consistent. Data for *more*
+assets than the problem was built for does not fit and raises a `CvxDataError`,
+and so does a payload whose models disagree about how large the universe is --
+handing the risk model two assets and the bounds four would otherwise leave the
+padded tail both riskless and unbounded, and the solver would put the whole
+portfolio there. `update` checks this across all models before it writes
+anything.
+
 Every problem has to be constructed by a Builder. Here's a builder for a classic
 [minimum variance problem](src/cvxmarkowitz/portfolios/min_var.py).
 The builder inherits from the [Builder](src/cvxmarkowitz/builder.py)

src/cvxmarkowitz/model.py

@@ -20,7 +20,7 @@
 
 import cvxpy as cp
 
-from cvxmarkowitz.types import Constraints, Matrix, Parameter, Variables
+from cvxmarkowitz.types import Constraints, Dimensions, Matrix, Parameter, Variables
 
 
 @dataclass(frozen=True)
@@ -79,6 +79,29 @@ def estimate(self, variables: Variables) -> cp.Expression:
         -- see `Bounds`, which is pure constraints.
         """
 
+    @abstractmethod
+    def dimensions(self, **kwargs: Matrix) -> Dimensions:
+        """Return the size every input in `kwargs` implies for a problem variable.
+
+        One `(variable name, size)` pair per input this model consumes, the
+        variable named by `DataNames` -- `WEIGHTS` for anything sized by the
+        asset universe, `FACTOR_WEIGHTS` for anything sized by the factors.
+        Several pairs may name the same variable; that is the point.
+
+        `Problem.update` collects these across every model and rejects a payload
+        whose claims disagree. It has to, because `update` pads short inputs up
+        to the compiled size (see `cvxmarkowitz.utils.fill`): a payload that
+        hands the risk model two assets and the bounds four does not fail on its
+        own -- it leaves the padded tail both zero-risk and unbounded, and the
+        solver puts the whole portfolio there. Each model already checks its own
+        inputs against each other; this is what checks them across models.
+
+        Abstract, rather than a default a model may quietly not override, for
+        the reason `keywords` documents: that shape of contract has already been
+        got wrong once here. A model with nothing to declare returns `()`, but it
+        forfeits the cross-check for every keyword it consumes.
+        """
+
     @abstractmethod
     def update(self, **kwargs: Matrix) -> None:
         """Write fresh values into this component's parameters, in place.

src/cvxmarkowitz/models/bounds.py

@@ -21,7 +21,7 @@
 import numpy as np
 
 from cvxmarkowitz.model import Model
-from cvxmarkowitz.types import Constraints, Matrix, Variables
+from cvxmarkowitz.types import Constraints, Dimensions, Matrix, Variables
 from cvxmarkowitz.utils.fill import fill_vector
 
 
@@ -65,8 +65,21 @@ def __post_init__(self) -> None:
             value=np.ones(self.assets),
         )
 
+    def dimensions(self, **kwargs: Matrix) -> Dimensions:
+        """Return the size both bound vectors imply for the variable bounded.
+
+        Both, not just the lower one: a payload giving a lower bound for two
+        assets and an upper bound for four would otherwise pad the lower bound
+        with zeros and leave the tail free between 0 and the real upper bound,
+        which is the same silent-tail failure `Problem.update` exists to catch.
+        """
+        return (
+            (self.acting_on, len(kwargs[self._f("lower")])),
+            (self.acting_on, len(kwargs[self._f("upper")])),
+        )
+
     def update(self, **kwargs: Matrix) -> None:
-        """Assign lower/upper vectors, padding or trimming to asset length."""
+        """Assign lower/upper vectors, zero-padding them to the compiled length."""
         self.data[self._f("lower")].value = fill_vector(num=self.assets, x=kwargs[self._f("lower")])
         self.data[self._f("upper")].value = fill_vector(num=self.assets, x=kwargs[self._f("upper")])
 

src/cvxmarkowitz/models/expected_returns.py

@@ -23,7 +23,7 @@
 from cvxmarkowitz.cvxerror import CvxDataError
 from cvxmarkowitz.model import Model
 from cvxmarkowitz.names import DataNames as D
-from cvxmarkowitz.types import Matrix, Variables
+from cvxmarkowitz.types import Dimensions, Matrix, Variables
 from cvxmarkowitz.utils.fill import fill_vector
 
 
@@ -69,6 +69,13 @@ def estimate(self, variables: Variables) -> cp.Expression:
         """
         return self.data[D.MU] @ variables[D.WEIGHTS] - self.parameter[D.MU_UNCERTAINTY] @ cp.abs(variables[D.WEIGHTS])
 
+    def dimensions(self, **kwargs: Matrix) -> Dimensions:
+        """Return the number of assets `mu` and its uncertainty imply."""
+        return (
+            (D.WEIGHTS, len(kwargs[D.MU])),
+            (D.WEIGHTS, len(kwargs[D.MU_UNCERTAINTY])),
+        )
+
     def update(self, **kwargs: Matrix) -> None:
         """Update expected returns and their uncertainty bounds.
 

src/cvxmarkowitz/models/holding_costs.py

@@ -22,7 +22,7 @@
 
 from cvxmarkowitz.model import Model
 from cvxmarkowitz.names import DataNames as D
-from cvxmarkowitz.types import Matrix, Variables
+from cvxmarkowitz.types import Dimensions, Matrix, Variables
 from cvxmarkowitz.utils.fill import fill_vector
 
 
@@ -38,6 +38,10 @@ def estimate(self, variables: Variables) -> cp.Expression:
         """Return total holding costs as -sum(w_i * c_i)."""
         return cp.sum(cp.neg(cp.multiply(variables[D.WEIGHTS], self.data[D.HOLDING_COSTS])))
 
+    def dimensions(self, **kwargs: Matrix) -> Dimensions:
+        """Return the number of assets the holding-cost vector implies."""
+        return ((D.WEIGHTS, len(kwargs[D.HOLDING_COSTS])),)
+
     def update(self, **kwargs: Matrix) -> None:
         """Update the holding-cost vector from kwargs[D.HOLDING_COSTS]."""
         self.data[D.HOLDING_COSTS].value = fill_vector(num=self.assets, x=kwargs[D.HOLDING_COSTS])

src/cvxmarkowitz/models/trading_costs.py

@@ -23,7 +23,7 @@
 from cvxmarkowitz.model import Model
 from cvxmarkowitz.names import DataNames as D
 from cvxmarkowitz.names import ParameterName as P
-from cvxmarkowitz.types import Matrix, Variables
+from cvxmarkowitz.types import Dimensions, Matrix, Variables
 from cvxmarkowitz.utils.fill import fill_vector
 
 
@@ -56,6 +56,10 @@ def estimate(self, variables: Variables) -> cp.Expression:
             )
         )
 
+    def dimensions(self, **kwargs: Matrix) -> Dimensions:
+        """Return the number of assets the previous weights imply."""
+        return ((D.WEIGHTS, len(kwargs[D.WEIGHTS])),)
+
     def update(self, **kwargs: Matrix) -> None:
         """Update cached data values.
 

src/cvxmarkowitz/problem.py

@@ -53,11 +53,45 @@ def update(self, **kwargs: Matrix) -> None:
         the same object do not yield two independently parametrized problems --
         the second overwrites the first. Call `build()` again for that.
 
+        The whole payload is validated against every model before the first
+        value is written, so a rejected payload leaves the problem exactly as it
+        was rather than half-overwritten. See `_validate`.
+
         Returns `None` (like `Model.update`) so the in-place semantics are
         visible at the call site.
 
         Raises:
-            CvxDataError: If any model is missing data for one of its parameters.
+            CvxDataError: If any model is missing data for one of its parameters,
+                or if the models disagree about how large the universe is.
+        """
+        self._validate(**kwargs)
+
+        for model in self.model.values():
+            # It's tempting to operate without the models at this stage.
+            # However, we would give up a lot of convenience. For example,
+            # the models can be prepared to deal with data that has not
+            # exactly the correct shape.
+            model.update(**kwargs)
+
+    def _validate(self, **kwargs: Matrix) -> None:
+        """Check the payload against every model, writing nothing.
+
+        Two passes, both over all models, both raising `CvxDataError`:
+
+        1. every keyword each model declares is present, and
+        2. the models agree on the size of each variable they describe.
+
+        The second is not redundant with the shape checks inside the models.
+        `Model.update` pads a short input up to the compiled size, so a payload
+        that describes two assets to the risk model and four to the bounds
+        solves without complaint -- and solves wrongly, because the padded tail
+        carries no risk while the bounds leave it free, which the solver reads as
+        two riskless assets. Nothing inside a single model can see that; only
+        comparing the models can.
+
+        Raises:
+            CvxDataError: On a missing keyword, or on models that disagree about
+                the size of a variable.
         """
         for name, model in self.model.items():
             # `Model.keywords`, not `model.data`: a model may consume a keyword
@@ -67,11 +101,17 @@ def update(self, **kwargs: Matrix) -> None:
                 if key not in kwargs:
                     raise CvxDataError(f"Missing data for {key} in model {name}")  # noqa: TRY003
 
-            # It's tempting to operate without the models at this stage.
-            # However, we would give up a lot of convenience. For example,
-            # the models can be prepared to deal with data that has not
-            # exactly the correct shape.
-            model.update(**kwargs)
+        claimed: dict[str, tuple[str, int]] = {}
+
+        for name, model in self.model.items():
+            for variable, size in model.dimensions(**kwargs):
+                first_name, first_size = claimed.setdefault(variable, (name, size))
+
+                if size != first_size:
+                    raise CvxDataError(  # noqa: TRY003
+                        f"Inconsistent size for {variable}: model {first_name} was given "
+                        f"{first_size}, model {name} was given {size}"
+                    )
 
     def solve(self, solver: str = cp.CLARABEL, **kwargs: Any) -> float:
         """Solve the problem."""

src/cvxmarkowitz/risk/cvar/cvar.py

@@ -23,7 +23,7 @@
 from cvxmarkowitz.cvxerror import CvxDataError
 from cvxmarkowitz.model import Model
 from cvxmarkowitz.names import DataNames as D
-from cvxmarkowitz.types import Matrix, Variables
+from cvxmarkowitz.types import Dimensions, Matrix, Variables
 from cvxmarkowitz.utils.fill import fill_matrix
 
 
@@ -74,6 +74,15 @@ def estimate(self, variables: Variables) -> cp.Expression:
         # average value of the k elements in the left tail
         return -cp.sum_smallest(self.data[D.RETURNS] @ variables[D.WEIGHTS], k=k) / k
 
+    def dimensions(self, **kwargs: Matrix) -> Dimensions:
+        """Return the number of assets the scenario matrix implies.
+
+        Its row count is the number of scenarios, which is this model's own
+        business rather than a size shared with the other models, so it is not
+        declared here.
+        """
+        return ((D.WEIGHTS, np.shape(kwargs[D.RETURNS])[1]),)
+
     def update(self, **kwargs: Matrix) -> None:
         """Update the returns matrix used by the CVaR model.
 

src/cvxmarkowitz/risk/factor/factor.py

@@ -23,7 +23,7 @@
 from cvxmarkowitz.cvxerror import CvxDataError
 from cvxmarkowitz.model import Model
 from cvxmarkowitz.names import DataNames as D
-from cvxmarkowitz.types import Constraints, Matrix, Variables
+from cvxmarkowitz.types import Constraints, Dimensions, Matrix, Variables
 from cvxmarkowitz.utils.fill import fill_matrix, fill_vector
 
 
@@ -113,6 +113,21 @@ def systematic_risk(self, variables: Variables) -> cp.Expression:
             )
         )
 
+    def dimensions(self, **kwargs: Matrix) -> Dimensions:
+        """Return the asset and factor counts every factor-model input implies."""
+        factors, assets = np.shape(kwargs[D.EXPOSURE])
+        chol_rows, chol_cols = np.shape(kwargs[D.CHOLESKY])
+
+        return (
+            (D.WEIGHTS, assets),
+            (D.WEIGHTS, len(kwargs[D.IDIOSYNCRATIC_VOLA])),
+            (D.WEIGHTS, len(kwargs[D.IDIOSYNCRATIC_VOLA_UNCERTAINTY])),
+            (D.FACTOR_WEIGHTS, factors),
+            (D.FACTOR_WEIGHTS, chol_rows),
+            (D.FACTOR_WEIGHTS, chol_cols),
+            (D.FACTOR_WEIGHTS, len(kwargs[D.SYSTEMATIC_VOLA_UNCERTAINTY])),
+        )
+
     def update(self, **kwargs: Matrix) -> None:
         """Validate and assign all factor-model inputs.
 

src/cvxmarkowitz/risk/sample/sample.py

@@ -23,7 +23,7 @@
 from cvxmarkowitz.cvxerror import CvxDataError
 from cvxmarkowitz.model import Model
 from cvxmarkowitz.names import DataNames as D
-from cvxmarkowitz.types import Constraints, Matrix, Variables
+from cvxmarkowitz.types import Constraints, Dimensions, Matrix, Variables
 from cvxmarkowitz.utils.fill import fill_matrix, fill_vector
 
 
@@ -58,6 +58,16 @@ def estimate(self, variables: Variables) -> cp.Expression:
             )
         )
 
+    def dimensions(self, **kwargs: Matrix) -> Dimensions:
+        """Return the number of assets the Cholesky factor and uncertainty imply."""
+        rows, cols = np.shape(kwargs[D.CHOLESKY])
+
+        return (
+            (D.WEIGHTS, rows),
+            (D.WEIGHTS, cols),
+            (D.WEIGHTS, len(kwargs[D.VOLA_UNCERTAINTY])),
+        )
+
     def update(self, **kwargs: Matrix) -> None:
         """Assign Cholesky factor and volatility-uncertainty vector.
 

src/cvxmarkowitz/types.py

@@ -24,3 +24,9 @@
 Constraints = dict[str, cp.Constraint]
 
 Matrix: TypeAlias = npt.NDArray[np.float64]
+
+# What `Model.dimensions` reports: (variable name, size) claims, one per input
+# the model consumes. A tuple rather than a mapping because several inputs of
+# one model speak about the same variable, and it is exactly their
+# disagreement that `Problem.update` is looking for.
+Dimensions: TypeAlias = tuple[tuple[str, int], ...]

src/cvxmarkowitz/utils/fill.py

@@ -15,20 +15,49 @@
 
 import numpy as np
 
+from cvxmarkowitz.cvxerror import CvxDataError
 from cvxmarkowitz.types import Matrix
 
 
 def fill_vector(x: Matrix, num: int) -> Matrix:
-    """Fill a vector of length num with x."""
+    """Return a vector of length ``num`` holding ``x`` in its leading entries.
+
+    The tail is zero. This is what lets one compiled problem serve a universe
+    smaller than the one it was built for: `Bounds` pads both of its bounds
+    this way, which pins the unused tail to ``0 <= w <= 0``.
+
+    Padding only ever goes one way. An ``x`` longer than ``num`` does not fit
+    the compiled problem at all, so it is reported as a `CvxDataError` rather
+    than truncated silently or left to escape as the `ValueError` numpy raises
+    on the assignment below -- `CvxDataError` is the failure mode the README
+    promises for input whose shapes do not fit.
+
+    Raises:
+        CvxDataError: If ``x`` is longer than ``num``.
+    """
+    if len(x) > num:
+        raise CvxDataError(f"Vector of length {len(x)} does not fit a problem built for {num}")  # noqa: TRY003
+
     z = np.zeros(num)
     z[: len(x)] = x
     return z
 
 
 def fill_matrix(x: Matrix, rows: int, cols: int) -> Matrix:
-    """Fill a matrix of size (rows, cols) with x."""
+    """Return a ``rows`` x ``cols`` matrix holding ``x`` in its top-left block.
+
+    The counterpart of `fill_vector`; see there for why the padding is only
+    ever one-directional.
+
+    Raises:
+        CvxDataError: If ``x`` does not fit into ``(rows, cols)``.
+    """
     # I had no luck with ndarray.resize()
-    z = np.zeros((rows, cols))
     (n, m) = np.shape(x)
+
+    if n > rows or m > cols:
+        raise CvxDataError(f"Matrix of shape {(n, m)} does not fit a problem built for {(rows, cols)}")  # noqa: TRY003
+
+    z = np.zeros((rows, cols))
     z[:n, :m] = x
     return z

tests/test_markowitz/test_models/test_holding_costs.py

@@ -25,3 +25,16 @@ def test_holding_costs():
     variables = {D.WEIGHTS: weights}
 
     assert model.estimate(variables).value == pytest.approx(0.04)
+
+
+def test_dimensions():
+    """The model reports the universe its holding-cost vector describes.
+
+    HoldingCosts pads a short vector to the compiled length like every other
+    model, so it has to declare what it was given -- otherwise a problem
+    carrying it could be handed a two-asset cost vector and four-asset bounds
+    without `Problem.update` noticing.
+    """
+    model = HoldingCosts(assets=3)
+
+    assert model.dimensions(**{D.HOLDING_COSTS: np.array([0.1, 0.2])}) == ((D.WEIGHTS, 2),)