Skip to content

design(sampling): a requirement that couldn't reach a verdict still triggers repair #1720

Open

Description

@planetf1

Decision: A2 (re-validate the same response once, then stop quietly). The options considered are kept below for the record.

Problem

When you attach requirements to a call with a sampling strategy, the reasonable expectation is that the response is regenerated only when a requirement says it's wrong. A requirement can also fail to reach a verdict at all: its adapter errors, or its judge output can't be parsed. ValidationResult already records that case. It carries the exception on error, and as_bool() returns False so nothing passes by accident (mellea/core/requirement.py:108-112).

The sampling loop doesn't tell the two apart. It checks bool() on each result (mellea/stdlib/sampling/base.py:476), and nothing under mellea/stdlib/sampling/ reads error. So a check that never ran counts as a failure, and the loop spends its budget regenerating.

With a stub backend and one requirement whose validation always returns ValidationResult(False, error=RuntimeError("adapter down")):

  • RepairTemplateStrategy(loop_budget=3) generated three times, and both retries were told:
    The following requirements failed before:
    * Response must be grounded
    
  • With a second requirement that genuinely failed (reason="too long"), the repair instruction listed both.
  • RejectionSamplingStrategy(loop_budget=3) generated three times.

What the user sees: every retry costs a full generation and a full validation pass, and the instruction steers the model away from an answer that was never judged. MultiTurnStrategy and SOFAI return a later, rewritten attempt; RejectionSamplingStrategy and RepairTemplateStrategy return the first attempt (base.py:562, :606), after spending the budget anyway.

This already happens in shipped code. Requirement.validate returns an errored result whenever a custom output_to_bool raises (mellea/core/requirement.py:440-453), which is how ALoraRequirement handles unparsable adapter output. The docs promise more than the loop delivers: docs/docs/tutorials/08-adapter-schema-migrations.md:129-132 says a schema mismatch "is never silently treated as 'requirement not met'", which holds for a direct validate() call but not under sampling. #1718 makes GroundednessRequirement produce errored results too, which makes the case routine.

Telemetry has the same blind spot. RequirementMetricsPlugin records an errored result as an ordinary failure labelled "LLM judgment" (mellea/telemetry/metrics_plugins.py:485-494), and the validation tracing plugin drops results with no reason (mellea/telemetry/tracing_plugins.py:601-602).

Requirements

  1. In the built-in strategies, an undetermined result never drives repair or resampling.
  2. When the only failures are undetermined, sampling doesn't regenerate. What happens instead is the decision below.
  3. When some failures are genuine, repair goes ahead on those, and undetermined requirements add nothing to the repair instruction.
  4. The outcome stays fail-closed: a run that ends on undetermined results doesn't report success.
  5. The exception stays reachable, on the ValidationResult in sample_validations and in the logs.
  6. Telemetry records undetermined results separately from failed ones.

Proposed approach

Split each iteration's failures into two groups: results with error set, and the rest. Only the second group drives repair() or a resample. The built-in repair() methods are all @staticmethod, and SOFAI doesn't subclass BaseSamplingStrategy (mellea/stdlib/sampling/sofai.py:45), so make the split a module-level function in mellea/stdlib/sampling/base.py and use it in:

  • the BaseSamplingStrategy loop's stop check (base.py:476-509);
  • BudgetForcingSamplingStrategy, which overrides _sample with its own stop check and repair call (mellea/stdlib/sampling/budget_forcing.py:253-285);
  • RepairTemplateStrategy.repair (base.py:631-646) and MultiTurnStrategy.repair (base.py:698-712);
  • ModelFriendlyRepairStrategy (mellea/stdlib/sampling/feedback.py);
  • SOFAI: its repair feedback (sofai.py:145-153), its S2 summary (sofai.py:487-492), and its S1 loop (sofai.py:723, :746-751). Leave undetermined results out of the "no improvement" comparison, and when the only failures left are undetermined, return before Phase 2 (sofai.py:784) so it doesn't escalate to S2.

RejectionSamplingStrategy.repair already ignores validation results (base.py:565-584), so for it only the stop check changes.

Custom strategies that override repair() still receive errored results in past_val. Requirement 1 covers the built-ins. Document how a custom strategy can detect an undetermined result (result.error is not None).

For telemetry, record undetermined results under their own attribute or count, following the existing naming in mellea/telemetry/, and stop counting them as "LLM judgment" failures.

Decision: what happens when the only failures are undetermined

Today a transient error can recover. If attempt 1's validation errors and the regenerated attempt 2 validates and passes, the run ends with success=True. Every option below changes that for some callers, so it's part of the choice.

A. Return the response (quiet stop). The loop ends early and takes the normal failure path: success=False, the response chosen by select_from_failure, and the errored results in sample_validations. Log a warning naming the requirement and the exception; the current failure message is info-level (base.py:380-385).

  • For: it's the convention ValidationResult already uses, where the exception is carried and not raised (core/requirement.py:440-453). The loop already treats a raising subsample the same way: it logs a warning and carries on, and raises only when no attempt produced anything (base.py:359-374). Callers still get a response back, as they do after an errored run today.
  • Against: a run that would have recovered on a later attempt now reports success=False. It's also easy to miss: a deployment with a permanently broken adapter returns unvalidated responses with only a warning in the logs.
  • Long term: an opt-in raise can be added later without breaking anyone.

A2. Re-validate the same response once, then quiet stop. Before stopping, run validation again on the same response, with no regeneration. If it now reaches a verdict, carry on as normal.

  • For: recovers transient failures (a dropped connection, a backend timeout) without regenerating a response that may be fine.
  • Against: one extra validation pass on every undetermined run. It doesn't help deterministic failures: an adapter at temperature 0 that produced unparsable output will usually do so again for the same input, where a regenerated response might have parsed.
  • Long term: the retry count could become configurable, but one is enough to start.

B. Raise. The loop raises an exception carrying the errored results.

  • For: impossible to miss. A broken deployment fails on the first call instead of quietly returning unvalidated output.
  • Against: a breaking change. Callers using ALoraRequirement or a custom output_to_bool get a response after an errored run today; under B they get an exception they've never had to handle. A single transient error also ends a run the caller might have accepted unvalidated.
  • Long term: hard to soften later, since switching back to returning is another behaviour change.

C. A strategy option, defaulting to A or A2.

  • For: callers who want B can opt in.
  • Against: more API surface for a case that's still uncommon.
  • Long term: this is where A ends up if anyone asks for B, so it can wait until someone does.

Decided: A2. It keeps the convention the codebase already uses in two places, recovers the transient case that A would lose, and leaves C available later without breaking anyone. B is the one option that's expensive to undo.

Testing and documentation

Unit tests with a stub backend like mocked_context_backend (test/stdlib/sampling/test_sampling_base_unit.py:121):

  • Undetermined only: each built-in strategy, including budget forcing and SOFAI, never regenerates or repairs, returns success=False, keeps the exception in sample_validations, and logs the warning.
  • Undetermined plus a genuine failure: the repair instruction names only the genuine one (repair template, multi-turn, SOFAI, model-friendly).
  • Mixed then resolved: undetermined plus a genuine failure on attempt 1, both pass on attempt 2: success=True.
  • SOFAI: no S2 escalation when the only failures are undetermined.
  • Re-validation: an error on the first validation and a pass on the re-validation gives success=True with no regeneration.
  • Telemetry: an undetermined result is recorded separately and not as an "LLM judgment" failure.

Docs:

  • docs/docs/concepts/instruct-validate-repair.md: describe what happens when a requirement can't reach a verdict, and how a custom strategy can detect it.
  • docs/docs/tutorials/08-adapter-schema-migrations.md:129-132: once this ships, its claim holds under sampling too; add a line saying so.
  • The PR description should call out the behaviour change, since the changelog is generated from PR titles.

#1718 and this issue both edit the ModelFriendlyRepairStrategy generic branch in feedback.py; doing #1718 first keeps that simple.

Line references are against 46ad3b3a5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

area/samplingSamplingStrategy, SamplingResult, ModelOption, generation optionsbugSomething isn't workingp2Medium/low: minor bugs, niche features, polish, docs, tests, cleanup. Scoped, lower urgency.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions