You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Trainer batch membership needs to point at concrete samples, not old run/definition membership. This creates the batch/sample join shape that later sampler and candidate-pool work can use directly.
What Actually Changes
RolloutBatch drops definition_id and gains nullable experiment_id and sampler_invocation_id.
Replaces RolloutBatchRun with RolloutBatchSampleMembership.
Adds membership table rollout_batch_sample_memberships with composite primary key (batch_id, sample_id), plus ordinal, nullable environment_id, and nullable pool_entry_id.
Updates RolloutService to write memberships, read sample IDs via _batch_sample_ids(...), preserve order by (ordinal, sample_id), and expose create/get helpers.
Adds RolloutBatchSummary DTO.
Adds GET /rollouts/batches/{batch_id}.
Gotchas / Review Risks
The RFC text says rollout_batch_samples; the implementation uses rollout_batch_sample_memberships. Worth reviewing the table name before downstream PRs depend on it.
RolloutService.submit(...) still accepts definition_id and creates definition-backed SampleRecords. This PR removes RolloutBatch.definition_id, not the old trainer submit input.
experiment_id and sampler_invocation_id are nullable bridges here. Real sampler/candidate-pool linkage lands later.
There is no rollout-specific standalone Alembic revision in this slice; existing-DB migration expectations should be read in light of the stack’s destructive migration posture.
If GitHub shows inherited stack churn in this PR, the rollout-specific review surface is the rollout service, DTO, telemetry model, route, and tests.
Cleaned Later
PR 04 adds persisted experiment candidate pools and sampler invocations.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Review Guide
Stack position: PR 02, base
codex/episode-sample-pr01-typed-wal, headcodex/episode-sample-pr02-rollout-batch-samples.Implements RFC Docs
03a-persistence-schemas.md: rollout tables.03b-implementation-churn.md: prereq PR 02, rollout batch samples.03c-postgres-table-inventory.md: rollout batch table inventory.09-implementation-plan/03-pr-02-rollout-batch-samples.md.Why This PR Exists
Trainer batch membership needs to point at concrete samples, not old run/definition membership. This creates the batch/sample join shape that later sampler and candidate-pool work can use directly.
What Actually Changes
RolloutBatchdropsdefinition_idand gains nullableexperiment_idandsampler_invocation_id.RolloutBatchRunwithRolloutBatchSampleMembership.rollout_batch_sample_membershipswith composite primary key(batch_id, sample_id), plusordinal, nullableenvironment_id, and nullablepool_entry_id.RolloutServiceto write memberships, read sample IDs via_batch_sample_ids(...), preserve order by(ordinal, sample_id), and expose create/get helpers.RolloutBatchSummaryDTO.GET /rollouts/batches/{batch_id}.Gotchas / Review Risks
rollout_batch_samples; the implementation usesrollout_batch_sample_memberships. Worth reviewing the table name before downstream PRs depend on it.RolloutService.submit(...)still acceptsdefinition_idand creates definition-backedSampleRecords. This PR removesRolloutBatch.definition_id, not the old trainer submit input.experiment_idandsampler_invocation_idare nullable bridges here. Real sampler/candidate-pool linkage lands later.Cleaned Later
Validation Notes
Key coverage includes
test_rollout_batch_sample_memberships.py, rollout membership architecture guards, rollout service/API tests, orderedsample_ids, nullable experiment/sampler provenance, and/rollouts/batches/{batch_id}responses.