Sampling config is unused and samples(1) does not behave like sampled mode

[MaestroError]

Summary

The package exposes sampling defaults in config and docs, but the builder does not consume them. In addition, samples(1) currently behaves like a non-sampled run and returns EvalResult instead of SampleResults.

Why this matters

This creates an API and documentation mismatch that can confuse users and make sampling behavior harder to reason about.

Evidence

• config/evals.php:38-40 defines sampling.default_samples and sampling.default_minimum.
• README.md:1044-1045 documents those config keys as effective settings.
• src/EvalBuilder.php:173-177 requires explicit values in samples() and never reads sampling config.
• src/EvalBuilder.php:353-356 only treats counts greater than 1 as sampled.

Current behavior

• ->samples() always depends entirely on explicit arguments.
• sampling.default_samples and sampling.default_minimum appear to be dead config.
• ->samples(1) returns a plain EvalResult, not SampleResults.

Why this may hurt DX

A user reading the README/config may reasonably expect:

• sampling defaults to influence runtime behavior, or
• samples(1) to still produce sampling semantics consistently.

Today neither expectation is clearly satisfied.

Suggested fix

This is only a suggestion; the actual fix should be planned carefully.

Possible directions:

• Either wire sampling.default_samples / sampling.default_minimum into actual runtime behavior, or remove them from config/docs if they are not intended to be supported.
• Decide and document whether samples(1) should:
  • return SampleResults for consistency, or
  • be rejected / normalized differently.
• Add tests for configured defaults and one-sample semantics.

Notes

This looks more like an incomplete feature or API contract mismatch than a one-line bug, so the final fix probably needs a small design decision first.

[MaestroError]

I checked the 4.x branch against this issue and confirmed the mismatch is real:

• config/evals.php defines sampling.default_samples and sampling.default_minimum.
• README.md documents those keys as active runtime defaults.
• EvalBuilder::samples() currently requires an explicit $count, so there is no way to call it "without explicit overrides" as the config comment suggests.
• EvalBuilder::isSampled() only returns true when $sampleCount > 1, so ->samples(1) falls back to the single-run path and returns EvalResult instead of SampleResults.
• AgentRunner::runSamples() already supports count: 1, so the inconsistency is in EvalBuilder, not in the runner.

Proposed implementation plan:

1. Make sampling opt-in by method call, not by count > 1.

   • Add a dedicated boolean flag in EvalBuilder such as samplingRequested.
   • Set it inside samples() / repeat().
   • Change isSampled() to check whether sampling was explicitly requested, so ->samples(1) still uses the sampled execution path and returns SampleResults.

2. Let samples() consume config defaults when arguments are omitted.

   • Change the signature to samples(?int $count = null, ?int $minimum = null) and mirror that in repeat().
   • Resolve omitted values from config('evals.sampling.default_samples') and config('evals.sampling.default_minimum') at execution time.
   • Keep explicit arguments higher priority than config values.

3. Centralize sampling resolution in EvalBuilder.

   • Add a small internal helper that computes the effective sample count and effective minimum.
   • Use that helper from ensureRun() when calling AgentRunner::runSamples().
   • This keeps the runtime behavior in one place and avoids mixing raw nullable properties with the final values passed to the runner.

4. Validate the resolved values before execution.

   • Require sample count to be at least 1.
   • If minimum is not null, require it to be between 1 and the resolved count.
   • Throw a clear LogicException for invalid config or invalid explicit arguments so failures are easy to diagnose.

5. Add focused tests for the two broken contracts.

   • samples() with no arguments uses published config defaults.
   • repeat() mirrors the same behavior.
   • samples(1)->run() returns SampleResults with one output.
   • samples(1)->judge(...) also returns SampleResults with one judge result.
   • Invalid default config / invalid explicit minimum bounds fail clearly.

6. Update user-facing docs in the same change.

   • Adjust the Sampling and Configuration sections in README.md to show that ->samples() can now rely on config defaults.
   • Document clearly that calling ->samples(1) still enables sampled semantics and therefore returns SampleResults.
   • Add a short changelog entry because this changes user-visible behavior.

Why I think this is the smallest correct direction:

• It preserves the existing fluent API for explicit counts.
• It makes the documented config keys actually do something instead of removing them.
• It resolves samples(1) without touching AgentRunner, which already behaves correctly for a single sample.
• It keeps the behavioral rule easy to explain: if you call samples(), you are in sampled mode, even when the effective count is 1.

I would classify the implementation as a bug fix with a small backwards-compatible API expansion, not as a new feature.