Skip to content

Commit cbfabb0

Browse files
Vishal VVishal V
authored andcommitted
fix(sampling): make sample final and add sample_index to majority-vote hook payloads
BaseMBRDSampling._sample fans out number_of_samples concurrent calls to BaseSamplingStrategy._sample, each receiving the same sampling_id and starting their own _subsample_iteration loop from subsample_index=0. This caused all branches to emit the same iteration numbers under one sampling_id, making (sampling_id, iteration) non-unique for consumers of SAMPLING_ITERATION and SAMPLING_REPAIR hooks. Fix by introducing sample_index: int | None = None on both payload classes and threading it through the emit helpers and _subsample_iteration so each fan-out branch carries a distinct 0-based ordinal. Non-fan-out strategies leave sample_index=None; no existing call sites change. - Renamed _sample_impl to _sample - Marked sample method as @Final enforces that subclasses override _sample rather than sample() - Add sample_index field to SamplingIterationPayload and SamplingRepairPayload (mellea/plugins/hooks/sampling.py) - Add sample_index kwarg to _emit_sampling_iteration and _emit_sampling_repair, forwarded to the payload (mellea/core/sampling.py) - Add sample_index param to BaseSamplingStrategy._sample and _subsample_iteration; forward to both _emit_* calls (mellea/stdlib/sampling/base.py) - Pass sample_index=i in the BaseMBRDSampling fan-out loop (mellea/stdlib/sampling/majority_voting.py) - Emit mellea.sampling.sample_index span-event attribute in SamplingTracingPlugin.on_iteration and on_repair when not None (mellea/telemetry/tracing_plugins.py) Tests added: - test_majority_vote_iteration_sample_index_is_unique: e2e regression proving (sampling_id, sample_index, iteration) is unique across all branches with number_of_samples=3, loop_budget=2 - test_majority_vote_repair_sample_index_matches_branch: repair events carry the same sample_index as the failed iteration that triggered them - test_sample_index_defaults_to_none / test_sample_index_construction on both payload classes - test_sampling_iteration_includes/omits_sample_index_when_set/none - test_sampling_repair_includes/omits_sample_index_when_set/none Assisted-by: IBM Bob Signed-off-by: Vishal V <VishalV@ibm.com>
1 parent d6c66a2 commit cbfabb0

11 files changed

Lines changed: 372 additions & 77 deletions

File tree

docs/docs/community/building-extensions.md

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,11 @@ Components, Sampling Strategies, and Backends without modifying the core library
1313

1414
Choose the pathway that fits the scope of your work:
1515

16-
| Pathway | When to use |
17-
| ------- | ----------- |
18-
| **Core repository** | General-purpose additions that benefit all users — open an issue first to discuss placement |
19-
| **Your own repo** (`mellea-` prefix) | Application-specific or domain-specific libraries |
20-
| **[mellea-contribs](https://github.com/generative-computing/mellea-contribs)** | Experimental or specialized components not yet ready for the standard library |
16+
| Pathway | When to use |
17+
| ------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------- |
18+
| **Core repository** | General-purpose additions that benefit all users — open an issue first to discuss placement |
19+
| **Your own repo** (`mellea-` prefix) | Application-specific or domain-specific libraries |
20+
| **[mellea-contribs](https://github.com/generative-computing/mellea-contribs)** | Experimental or specialized components not yet ready for the standard library |
2121

2222
> **Note:** For general-purpose Components, Requirements, or Sampling Strategies,
2323
> open an issue before submitting a PR. This avoids duplication and ensures
@@ -143,12 +143,14 @@ For a full walkthrough of the Component protocol and templating system, see
143143

144144
A [`SamplingStrategy`](../reference/glossary#sampling-strategy) controls how Mellea
145145
generates and validates outputs — for example, rejection sampling, best-of-n, or
146-
beam search. Subclass `SamplingStrategy` and implement `sample`:
146+
beam search. Subclass `SamplingStrategy` and implement `_sample`:
147+
148+
> **Note:** `sample()` is `@final` — it owns the sampling lifecycle (hooks, budget
149+
> management). Override `_sample()` instead; `sample()` calls it automatically.
147150
148151
```python
149-
import asyncio
150152
from mellea.core.backend import Backend
151-
from mellea.core.base import Component, Context, ModelOutputThunk, S
153+
from mellea.core.base import CBlock, Component, Context, ModelOutputThunk, S
152154
from mellea.core.requirement import Requirement
153155
from mellea.core.sampling import SamplingResult, SamplingStrategy
154156

@@ -164,36 +166,42 @@ class BestOfNStrategy(SamplingStrategy):
164166
"""
165167
self.n = n
166168

167-
async def sample(
169+
async def _sample(
168170
self,
169-
action: Component[S],
171+
action: Component[S] | CBlock | ModelOutputThunk,
170172
context: Context,
171173
backend: Backend,
172-
requirements: list[Requirement] | None,
174+
requirements: list[Requirement],
173175
*,
176+
effective_loop_budget: int,
174177
validation_ctx: Context | None = None,
175178
format: type | None = None,
176179
model_options: dict | None = None,
177180
tool_calls: bool = False,
181+
sampling_id: str,
182+
**kwargs,
178183
) -> SamplingResult[S]:
179184
"""Generate N candidates and return the best one.
180185
181186
Args:
182-
action: The component to generate a response for.
187+
action: The component, block, or thunk to generate a response for.
183188
context: The current session context.
184189
backend: The backend used for generation.
185190
requirements: Requirements to validate each candidate against.
191+
effective_loop_budget: Loop budget after hook modification.
186192
validation_ctx: Optional context override for validation.
187193
format: Structured output format, if any.
188194
model_options: Model options to pass to the backend.
189195
tool_calls: Whether to enable tool calls during generation.
196+
sampling_id: UUID correlating iteration/repair/end hooks to this loop.
197+
**kwargs: Additional keyword arguments (e.g., `show_progress`).
190198
191199
Returns:
192200
SamplingResult containing the selected candidate and validation details.
193201
"""
194202
generations: list[ModelOutputThunk[S]] = []
195203
contexts: list[Context] = []
196-
actions: list[Component[S]] = []
204+
actions: list[Component[S] | CBlock | ModelOutputThunk] = []
197205
validations: list[list[tuple[Requirement, object]]] = []
198206

199207
for _ in range(self.n):

mellea/core/sampling.py

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
import abc
1515
import uuid
1616
from collections.abc import Sequence
17-
from typing import Generic
17+
from typing import Generic, final
1818

1919
from .backend import Backend, BaseModelSubclass
2020
from .base import (
@@ -125,8 +125,17 @@ class SamplingStrategy(abc.ABC):
125125
126126
This class provides a template for creating concrete sampling strategies that can be used to generate model outputs based on given instructions.
127127
It allows setting custom validation and generation functions through properties.
128+
129+
Attributes:
130+
loop_budget: Maximum number of generate/validate cycles. Defaults to `1`.
131+
requirements: Global requirements evaluated on every sample. When set,
132+
overrides per-call requirements. Defaults to `None`.
128133
"""
129134

135+
loop_budget: int = 1
136+
requirements: list[Requirement] | None = None
137+
138+
@final
130139
async def sample(
131140
self,
132141
action: Component[S] | CBlock | ModelOutputThunk,
@@ -143,7 +152,7 @@ async def sample(
143152
"""Concrete wrapper: owns the sampling lifecycle and fires loop start/end hooks.
144153
145154
Mints a `sampling_id`, dispatches `sampling_loop_start` (which may modify
146-
`loop_budget`), delegates to `_sample_impl`, and dispatches
155+
`loop_budget`), delegates to `_sample`, and dispatches
147156
`sampling_loop_end` on every exit path — success, budget exhaustion, and
148157
raised exceptions.
149158
@@ -156,7 +165,7 @@ async def sample(
156165
format: output format for structured outputs.
157166
model_options: model options to pass to the backend during generation / validation.
158167
tool_calls: True if tool calls should be used during this sampling strategy.
159-
**kwargs: Additional keyword arguments forwarded to `_sample_impl`.
168+
**kwargs: Additional keyword arguments forwarded to `_sample`.
160169
161170
Returns:
162171
SamplingResult[S]: A result object indicating the success or failure of the sampling process.
@@ -174,8 +183,7 @@ async def sample(
174183

175184
try:
176185
reqs = self._merge_requirements(requirements)
177-
loop_budget = self._get_loop_budget()
178-
effective_loop_budget = loop_budget
186+
effective_loop_budget = self.loop_budget
179187

180188
# --- sampling_loop_start hook ---
181189
if has_plugins(HookType.SAMPLING_LOOP_START):
@@ -187,7 +195,7 @@ async def sample(
187195
action=action,
188196
context=context,
189197
requirements=reqs,
190-
loop_budget=loop_budget,
198+
loop_budget=self.loop_budget,
191199
)
192200
_, start_payload = await invoke_hook(
193201
HookType.SAMPLING_LOOP_START, start_payload, backend=backend
@@ -202,7 +210,7 @@ async def sample(
202210
f"{effective_loop_budget}; must be >= 1."
203211
)
204212

205-
s_result = await self._sample_impl(
213+
s_result = await self._sample(
206214
action=action,
207215
context=context,
208216
backend=backend,
@@ -255,7 +263,7 @@ async def sample(
255263
)
256264

257265
@abc.abstractmethod
258-
async def _sample_impl(
266+
async def _sample(
259267
self,
260268
action: Component[S] | CBlock | ModelOutputThunk,
261269
context: Context,
@@ -301,6 +309,8 @@ async def _emit_sampling_iteration(
301309
result: ModelOutputThunk,
302310
validation_results: list[tuple[Requirement, ValidationResult]],
303311
backend: Backend,
312+
*,
313+
sample_index: int | None = None,
304314
) -> None:
305315
"""Emit the sampling-iteration hook payload if any plugin is registered."""
306316
from ..plugins.manager import has_plugins, invoke_hook
@@ -316,6 +326,7 @@ async def _emit_sampling_iteration(
316326
sampling_id=sampling_id,
317327
strategy_name=type(self).__name__,
318328
iteration=iteration,
329+
sample_index=sample_index,
319330
action=action,
320331
result=result,
321332
validation_results=validation_results,
@@ -335,6 +346,8 @@ async def _emit_sampling_repair(
335346
repair_action: SampleActionType,
336347
repair_context: Context,
337348
backend: Backend,
349+
*,
350+
sample_index: int | None = None,
338351
) -> None:
339352
"""Emit the sampling-repair hook payload if any plugin is registered."""
340353
from ..plugins.manager import has_plugins, invoke_hook
@@ -354,6 +367,7 @@ async def _emit_sampling_repair(
354367
repair_action=repair_action,
355368
repair_context=repair_context,
356369
repair_iteration=repair_iteration,
370+
sample_index=sample_index,
357371
)
358372
await invoke_hook(HookType.SAMPLING_REPAIR, repair_payload, backend=backend)
359373

@@ -370,18 +384,9 @@ def _merge_requirements(
370384
Returns:
371385
Deduplicated list of requirements to use for this sampling call.
372386
"""
373-
strategy_reqs: list[Requirement] | None = getattr(self, "requirements", None)
374387
reqs: list[Requirement] = []
375-
if strategy_reqs is not None:
376-
reqs += strategy_reqs
388+
if self.requirements is not None:
389+
reqs += self.requirements
377390
elif call_requirements is not None:
378391
reqs += call_requirements
379392
return list(set(reqs))
380-
381-
def _get_loop_budget(self) -> int:
382-
"""Return the strategy's configured loop budget (default 1).
383-
384-
Returns:
385-
The loop budget from `self.loop_budget` if set, otherwise `1`.
386-
"""
387-
return getattr(self, "loop_budget", 1)

mellea/plugins/hooks/sampling.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,9 @@ class SamplingIterationPayload(MelleaBasePayload):
4141
strategy_name: Class name of the sampling strategy (e.g. `"RejectionSamplingStrategy"`).
4242
iteration: 1-based iteration number within the sampling loop. There is no guarantee that
4343
iteration will be monotonically increasing when concurrency is enabled.
44+
sample_index: Optional 0-based index identifying the outer concurrent sample branch —
45+
set by fan-out strategies such as majority voting; `None` for strategies without an
46+
outer fan-out.
4447
action: The `Component` used for this attempt.
4548
4649
result: The `ModelOutputThunk` produced by this attempt.
@@ -54,6 +57,7 @@ class SamplingIterationPayload(MelleaBasePayload):
5457
sampling_id: str = ""
5558
strategy_name: str = ""
5659
iteration: int = 0
60+
sample_index: int | None = None
5761
action: Any = None
5862
result: Any = None
5963
validation_results: list[tuple[Any, Any]] = []
@@ -75,6 +79,9 @@ class SamplingRepairPayload(MelleaBasePayload):
7579
repair_action: The repaired `Component` to use for the next attempt.
7680
repair_context: The `Context` to use for the next attempt.
7781
repair_iteration: 1-based iteration at which the repair was triggered.
82+
sample_index: Optional 0-based index identifying the outer concurrent sample branch —
83+
set by fan-out strategies such as majority voting; `None` for strategies without an
84+
outer fan-out.
7885
"""
7986

8087
sampling_id: str = ""
@@ -85,6 +92,7 @@ class SamplingRepairPayload(MelleaBasePayload):
8592
repair_action: Any = None
8693
repair_context: Any = None
8794
repair_iteration: int = 0
95+
sample_index: int | None = None
8896

8997

9098
class SamplingLoopEndPayload(MelleaBasePayload):

mellea/stdlib/sampling/base.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ def select_from_failure(
199199
"""
200200
...
201201

202-
async def _sample_impl(
202+
async def _sample(
203203
self,
204204
action: Component[S] | CBlock | ModelOutputThunk,
205205
context: Context,
@@ -213,6 +213,7 @@ async def _sample_impl(
213213
tool_calls: bool = False,
214214
sampling_id: str,
215215
show_progress: bool = True,
216+
sample_index: int | None = None,
216217
**kwargs,
217218
) -> SamplingResult[S]:
218219
"""Execute the rejection-sampling loop.
@@ -229,6 +230,7 @@ async def _sample_impl(
229230
tool_calls: True if tool calls should be used during this sampling strategy.
230231
sampling_id: UUID correlating iteration/repair/end hooks for this loop.
231232
show_progress: if true, a tqdm progress bar is used. Otherwise, messages will still be sent to flog.
233+
sample_index: Optional 0-based index of this branch within a fan-out strategy (e.g. majority voting). `None` for strategies without an outer fan-out.
232234
**kwargs: Additional keyword arguments forwarded by `SamplingStrategy.sample()`.
233235
234236
Returns:
@@ -279,6 +281,7 @@ async def _sample_impl(
279281
model_options=model_options,
280282
tool_calls=tool_calls,
281283
sampling_id=sampling_id,
284+
sample_index=sample_index,
282285
)
283286
for idx in range(self.concurrency_budget)
284287
]
@@ -406,6 +409,7 @@ async def _subsample_iteration(
406409
model_options: dict | None = None,
407410
tool_calls: bool = False,
408411
sampling_id: str,
412+
sample_index: int | None = None,
409413
) -> AsyncGenerator[_SamplingResultSlice[S], Any]:
410414
"""Run one concurrent subsample: up to `iterations` generate/validate/repair attempts.
411415
@@ -476,6 +480,7 @@ async def _subsample_iteration(
476480
result=result,
477481
validation_results=constraint_scores,
478482
backend=backend,
483+
sample_index=sample_index,
479484
)
480485

481486
if not all_validations_passed:
@@ -530,6 +535,7 @@ async def _subsample_iteration(
530535
repair_action=next_action,
531536
repair_context=next_context,
532537
backend=backend,
538+
sample_index=sample_index,
533539
)
534540

535541

mellea/stdlib/sampling/budget_forcing.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ def _get_repair_type() -> str:
102102
"""Return the repair-type label for telemetry hooks."""
103103
return "budget_forcing"
104104

105-
async def _sample_impl(
105+
async def _sample(
106106
self,
107107
action: Component[S] | CBlock | ModelOutputThunk,
108108
context: Context,

mellea/stdlib/sampling/majority_voting.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ def maybe_apply_weighted(self, scr: np.ndarray) -> np.ndarray:
134134

135135
return scr
136136

137-
async def _sample_impl(
137+
async def _sample(
138138
self,
139139
action: Component[S] | CBlock | ModelOutputThunk,
140140
context: Context,
@@ -152,7 +152,7 @@ async def _sample_impl(
152152
) -> SamplingResult[S]:
153153
"""Samples using majority voting.
154154
155-
Fan-out calls `_sample_impl` on the parent class directly so that each
155+
Fan-out calls `_sample` on the parent class directly so that each
156156
inner sample does not emit its own enclosing `sampling` span — there is
157157
exactly one `sampling` span for the top-level call, emitted by the
158158
`SamplingStrategy.sample` wrapper.
@@ -174,12 +174,12 @@ async def _sample_impl(
174174
Returns:
175175
SamplingResult[S]: A result object indicating the success or failure of the sampling process.
176176
"""
177-
# execute sampling concurrently — call _sample_impl directly on the
177+
# execute sampling concurrently — call _sample directly on the
178178
# parent so each inner sample does not fire its own lifecycle hooks.
179179
tasks: list[asyncio.Task[SamplingResult]] = []
180180
for i in range(self.number_of_samples):
181181
task = asyncio.create_task(
182-
super()._sample_impl(
182+
super()._sample(
183183
action,
184184
context,
185185
backend,
@@ -190,6 +190,7 @@ async def _sample_impl(
190190
tool_calls=tool_calls,
191191
sampling_id=sampling_id,
192192
show_progress=show_progress,
193+
sample_index=i,
193194
)
194195
)
195196
tasks.append(task)

mellea/stdlib/sampling/sofai.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -579,7 +579,7 @@ async def _generate_and_validate(
579579
# Main Sample Method
580580
# =========================================================================
581581

582-
async def _sample_impl(
582+
async def _sample(
583583
self,
584584
action: Component[S] | CBlock | ModelOutputThunk,
585585
context: Context,

0 commit comments

Comments
 (0)