feat(trim): support missingTargetMeasurement for Entities with unmeasured targetOutput

[VassilisVassiliadis]

Fixes #1064

[danielelotito]

All is good except one important algorithm subtlety, read below please.
There is one minor thing about a removal of a magic number, and a major thing about management of the holdout current_holdout_df.

TRIM: Training / Holdout Split Invariant

1. Parameters

Parameter	Symbol	Meaning
samplingBudget.minPoints	M	Minimum number of rows in source_df (real or injected) before iterative modeling starts. Default: 18. See Section 3 for how this is (partially) enforced.
iterationSize	S	Number of real measurements the sampler accumulates (Phase A) before building the first model (Phase B). Also the holdout window size. Enforced equal to holdoutSize by the set_holdout_size validator. Default: 5.
holdoutSize	S	Always equal to iterationSize (enforced by set_holdout_size). Capacity of the yielded_rows ring.
initial_source_df	D_0	Snapshot of all rows with a targetOutput value at the moment the iterative loop starts. Includes real measurements and injected-default rows from the no-priors pre-scan. Never updated after the snapshot. Size is at least M in the normal path — see Section 3 for the soft-fallback exception.
yielded_rows	ring	Fixed-capacity FIFO ring of capacity S. Holds the last S real rows added to source_df. Used to form current_holdout_df in Phase C.

---

2. Invariant

At every model-building iteration:

• Training set = all rows that have a targetOutput value, real or injected. No restriction on the origin of the label. Minimum size at Phase B: |D_0| rows, which is at least M in the normal path (see Section 3).
• Holdout set = exactly the last S real measurements. Must contain only rows where targetOutput was actually observed — injected defaults must never appear here, because holdout scores are the signal used to evaluate model quality.
• Training and holdout are disjoint.

The distinction between real and injected matters only for the holdout. Training can use both freely.

---

3. Minimum sizes: current state

Minimum training size — samplingBudget.minPoints (M)

M is the minimum for |D_0|, counting all rows with a targetOutput value (real or
injected). There are two enforcement points in the current code, each with different
strength:

Point 1 — operator.py lines 197–267 (hard stop, conditional):
If len(source_df) < M at startup, the no-priors phase runs. After no-priors completes,
source_df is re-queried (line 252). If still < M,
log_unable_to_proceed_with_iterative_modeling_and_raise_error is called and the
iterative phase never starts. This is a hard stop — but it only applies when no-priors
was triggered. If the space already had >= M rows at startup, this block is skipped
entirely.

Point 2 — entities_for_iterative_modeling_from_discovery_space lines 842–859 (soft):
This check runs unconditionally at sampler startup against source_df from
get_source_and_target_with_injected_defaults — i.e. real rows plus any pre-scan
injected defaults. The code is:

if len(source_df) < self.params.samplingBudget.minPoints:
    ...
    if len(source_df) > 10:
        self.log.warning("Attempting iterative modelling with 10 source space points")
    else:
        raise InsufficientDataError(...)

Two problems here:

1. The hard floor is 11, not M. If len(source_df) is anywhere in [11, M-1], the
   check logs an error and a warning and then falls through — iterative modeling
   continues with fewer rows than M. The raise only fires at <= 10.

2. The warning message is wrong. It says "Attempting iterative modelling with 10
   source space points" but the threshold is > 10, so the minimum that actually
   continues is 11. The message is misleading regardless of the actual count.

Current state summary:

Condition	What happens
len(source_df) >= M at startup	No-priors skipped. |D_0| >= M. ✅
len(source_df) < M at startup, no-priors reaches M	|D_0| == M. ✅
len(source_df) < M at startup, no-priors fails to reach M	Hard stop in operator.py. ✅
11 <= len(source_df) < M at sampler startup	Logs error + warning, continues. |D_0| may be 11–(M-1). ⚠️
len(source_df) <= 10 at sampler startup	Hard stop. ✅

The soft-fallback row (11 <= len < M) is the gap: M is not a true hard minimum.
This path can only be reached if the space already had >= M rows at operator.py
startup (so no-priors was skipped), but by the time the sampler runs the source space
has somehow shrunk, which should not happen under normal operation. In practice the
concern is a user-misconfigured M that is larger than what the space can provide.

Proposed fix for Point 2:

Replace the > 10 soft fallback with a hard stop that always enforces M:

# trim_sampler.py — entities_for_iterative_modeling_from_discovery_space
if len(source_df) < self.params.samplingBudget.minPoints:
    msg = (
        f"Insufficient data: need {self.params.samplingBudget.minPoints} rows "
        f"in source_df but only {len(source_df)} are available. "
        "This may happen when the target variable cannot be measured for all entities."
    )
    self.log.error(msg)
    raise InsufficientDataError(msg)

This removes the magic number 10, the misleading warning message, and makes M a true
hard minimum at the sampler level, consistent with the hard stop in operator.py.

Minimum holdout size — iterationSize (S)

holdoutSize == iterationSize == S is enforced by the set_holdout_size validator —
always true in the current code. Phase B fires when total_measured == S, at which point
exactly S real rows have been added since D_0. current_holdout_df therefore has
exactly S rows at Phase B (before Bug B contaminates it — see Section 7).

This guarantee is unconditional: there is no path through Phase B with fewer than S
real rows in current_holdout_df.

The len < 2 guard

Lines 461–471 are a last-resort safety net: if train_data or holdout_data has fewer
than 2 rows, AutoGluon is skipped for that iteration. It handles degenerate
configurations (e.g. S=1, or the soft-fallback case where |D_0| < M). It does not
enforce M.

Relationship between M and S

There is no enforced relationship between M and S. The defaults (M=18, S=5) give
|train_df| >= 18 and |holdout| == 5 at Phase B.

---

4. How the loop is structured

The main loop in _core_iterator_logic has three phases, gated on total_measured
(count of entities that actually measured targetOutput):

Phase A  total_measured < S          accumulate real rows, no model built
Phase B  total_measured == S         build first model, first holdout created
Phase C  total_measured > S          build subsequent models, holdout slides

Unmeasured entities (len(one_additional_row) == 0) increment total_unmeasured
and continue — they never increment total_measured and never advance the phase gate.

---

5. Phase-by-phase analysis (no injected defaults — original behaviour)

Phase A

No model is built. yielded_rows accumulates one real row per iteration.
previous_source_df advances each step. No splits occur.

Phase B (first model)

train_df, current_holdout_df = split_common_and_diff(
    longer=current_source_df,
    shorter=initial_source_df,
)

split_common_and_diff returns:

• train_df = rows in both current_source_df and D_0 = all pre-existing rows
• current_holdout_df = rows in current_source_df not in D_0 = the S real rows added during Phase A

These two sets are disjoint by construction. ✅

Phase C (subsequent models)

train_df, _ = split_common_and_diff(
    longer=current_source_df,
    shorter=previous_source_df,
)
current_holdout_df = pd.DataFrame(yielded_rows.df)

train_df = intersection of current and previous snapshot = all rows except the newest.
current_holdout_df = last S real rows from the ring.

The ring rows are a subset of train_df (they were measured earlier and appear in both
snapshots). So there is overlap. This is intentional: the holdout is a rolling window
that slides forward, evaluating each new model on the most recently seen S real points.

---

6. What InjectDefaultValue adds

When mode=InjectDefaultValue and an entity fails to measure targetOutput:

1. A synthetic row is appended to self.injected_defaults with
   targetOutput = missingTargetMeasurements.defaultValue.
2. get_source_and_target_with_injected_defaults concatenates self.injected_defaults
   onto the real source_df, so injected rows appear in every subsequent
   current_source_df.
3. total_measured is not incremented — the phase gate does not advance.
4. yielded_rows must not receive the injected row — it holds only real measurements
   for holdout construction.

---

7. Where the current code fails

Bug A — crash on line 341

# VV: This is a yielded row, just a synthetic one not one we actually measured
yielded_rows += one_additional_row   # one_additional_row is EMPTY (len == 0)

RowsRing.__iadd__ calls _normalize_row, which requires exactly 1 row when given a
pd.DataFrame. An empty DataFrame raises ValueError. So in InjectDefaultValue mode,
the first unmeasured entity during iterative modeling crashes the sampler.

Bug B — injected-default rows leak into current_holdout_df in Phase B

D_0 is snapshotted at line 226, after handle_unmeasured_targetOutputs_from_no_priors
has run and populated self.injected_defaults with no-priors failures. So no-priors
injected rows are already in D_0 — they appear in train_df in Phase B, not in
current_holdout_df. ✅

However, injected rows created during the iterative loop (Phase A or later) are added
to self.injected_defaults after D_0 was snapshotted. They enter current_source_df
but are absent from D_0. The Phase B split:

current_holdout_df = current_source_df - D_0

returns every row in current_source_df not in D_0 — which includes both the S real
Phase-A measurements and any injected-default rows created during Phase A. A
synthetic row with a fabricated targetOutput ends up in the holdout, violating the
invariant that holdout must contain only real measurements.

Summary of failures

Bug	Location	Effect
A	trim_sampler.py:341	ValueError crash on first iterative-phase unmeasured entity
B	trim_sampler.py:401-404	Injected-default rows created during Phase A leak into current_holdout_df, violating the real-only holdout invariant

---

8. The fix

Fix for Bug A — delete lines 340-341

# DELETE these two lines:
# VV: This is a yielded row, just a synthetic one not one we actually measured
yielded_rows += one_additional_row

The injected row is already in self.injected_defaults (lines 336-339) and flows into
train_df via get_source_and_target_with_injected_defaults. yielded_rows stays
clean — only real measurements enter it at line 359 (elif len(one_additional_row) == 1).

Fix for Bug B — exclude injected defaults from current_holdout_df in Phase B

After the Phase B split, remove any rows that appear in self.injected_defaults:

# Phase B — existing split
train_df, current_holdout_df = split_common_and_diff(
    longer_df_from_which_you_subtract=current_source_df,
    shorter_df_that_you_subtract=initial_source_df,
)

# NEW: purge synthetic rows from the holdout
if self.injected_defaults is not None and len(self.injected_defaults) > 0:
    _, current_holdout_df = split_common_and_diff(
        longer_df_from_which_you_subtract=current_holdout_df,
        shorter_df_that_you_subtract=self.injected_defaults,
    )

After this, current_holdout_df contains only real Phase-A measurements. Injected rows
remain in train_df.

Phase C is unaffected: current_holdout_df = pd.DataFrame(yielded_rows.df) and
yielded_rows never receives injected rows (Bug A fix guarantees this).

---

9. Why the fix satisfies the invariant

After both fixes, for every model-building iteration:

Set	Contents	Contains injected defaults?	Contains only real measurements?
train_df	D_0 ∪ injected rows ∪ real rows outside the ring	yes	no (by design)
current_holdout_df	last S real rows from yielded_rows ring	no	yes ✅

• Training uses all available targetOutput values (real or injected) — no restriction.
• Holdout contains only rows where targetOutput was actually observed — evaluation
  signal is never corrupted by fabricated labels.
• The two sets remain disjoint: Phase B uses split_common_and_diff followed by the
  injected-default purge; Phase C uses yielded_rows which holds only real rows by
  construction.
• The S-rows holdout size guarantee is restored: after the Bug B fix,
  current_holdout_df at Phase B has exactly S real rows.

[danielelotito]

One remaining issue on my side (see review comment).
You can consider adding a test like the following to assess a correct behaviour

@pytest.mark.timeout(900)
def test_trim_skips_no_priors_when_space_already_characterized(
    trim_minimal_discovery_space: DiscoverySpace,
    tmp_path: pathlib.Path,
) -> None:
    """TRIM must not crash when the no-priors phase is skipped.

    Regression test for the bug where ``op_output_characterization_no_prior.operation``
    is ``None`` (no-priors skipped) but the operator unconditionally accessed
    ``.operation.identifier``, raising ``AttributeError``.

    Strategy: run TRIM twice on the same space.
    - First run: minPoints=maxPoints=8 — fills the space to 8 measurements,
      no iterative phase (budget exhausted after no-priors).
    - Second run: minPoints=5 — since 8 ≥ 5 the no-priors block is skipped,
      and the iterative phase must start and succeed.
    """
    autogluon_args = AutoGluonArgs(
        fitArgs={
            "time_limit": 10,
            "presets": "medium_quality",
            "auto_stack": False,
            "excluded_model_types": ["CAT", "NN_TORCH", "FASTAI", "GBM", "XGB", "RF"],
        },
        tabularPredictorArgs={"problem_type": "regression", "verbosity": 0},
    )

    trim_fn = characterize.operators["trim"].function
    assert trim_fn is not None

    # --- First run: fill the space with 8 measurements, no iterative phase ---
    params_fill = TrimParameters(
        targetOutput="pressure",
        samplingBudget=SamplingBudget(minPoints=8, maxPoints=8),
        iterationSize=1,
        outputDirectory=str(tmp_path / "trim_fill"),
        debugDirectory=str(tmp_path / "debug_fill"),
        stoppingCriterion=StoppingCriterion(enabled=False),
        autoGluonArgs=autogluon_args,
        finalModelAutoGluonArgs=autogluon_args,
        noPriorParameters=NoPriorsParametersInternal(
            targetOutput="pressure",
            samples=8,
            batchSize=1,
            sampling_strategy="random",
        ),
    )
    output_fill = trim_fn(trim_minimal_discovery_space, **params_fill.model_dump())
    assert output_fill.exitStatus.exit_state == OperationExitStateEnum.SUCCESS

    # --- Second run: no-priors phase is skipped (8 ≥ 5), iterative phase runs ---
    params_iter = TrimParameters(
        targetOutput="pressure",
        samplingBudget=SamplingBudget(minPoints=5, maxPoints=9),
        iterationSize=1,
        outputDirectory=str(tmp_path / "trim_iter"),
        debugDirectory=str(tmp_path / "debug_iter"),
        stoppingCriterion=StoppingCriterion(enabled=False),
        autoGluonArgs=autogluon_args,
        finalModelAutoGluonArgs=autogluon_args,
        noPriorParameters=NoPriorsParametersInternal(
            targetOutput="pressure",
            samples=5,
            batchSize=1,
            sampling_strategy="random",
        ),
    )
    output_iter = trim_fn(trim_minimal_discovery_space, **params_iter.model_dump())

    assert output_iter.exitStatus is not None
    assert output_iter.exitStatus.exit_state == OperationExitStateEnum.SUCCESS
    assert output_iter.operation is not None
    # No no-priors resource — only the iterative operation
    assert len(output_iter.resources) == 1
    assert (
        output_iter.resources[0].config.metadata.model_dump()["completed operation"]
        == "Iterative Modeling Operation"
    )
    # Final model directory must exist
    assert (tmp_path / "trim_iter_finalized").is_dir(), (
        "finalize_model was never called: trim_iter_finalized directory was not created"
    )

in tests/operators/test_trim_example_integration.py

[VassilisVassiliadis]

Thank you! I updated the PR, let's see if it goes through the CI/CD.

[danielelotito]

I approve the PR

[danielelotito]

Congratulations!
You have earned the TRIM badge (display it in every important events)