Repository navigation
refactor: unify training data and simplify report processing - #446
Merged
Merged
Conversation
BobbyZhouZijian
requested review from
Benjamin-eecs,
hanfeiyu and
simonucl
as code owners
September 13, 2026 19:30
|
Merge Oncall: @BobbyZhouZijian The merge oncall will route reviews and keep this pull request moving toward a clear decision. |
BobbyZhouZijian
enabled auto-merge (squash)
September 13, 2026 19:31
1 of 7 tasks
BobbyZhouZijian
added a commit
that referenced
this pull request
Sep 13, 2026
Resolves the overlap with the ATIF training-data refactor (#446) and the reported-feedback simplification (#445): main's TrainingBatch/TrajectoryItem model is kept everywhere, this branch's names stay (CandidateBackend, CandidatePluginFactory, SupportsReceipt, the runtime module split), main's edit to the removed reef/runtime/adapters/executor_runtime.py is carried into reef/train/runtime.py, and the Terminal-Bench example imports the proxy runtime from reef.inference.http. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
BobbyZhouZijian
added a commit
that referenced
this pull request
Sep 14, 2026
main reworked the ground this branch stands on: #432 gave Reef ownership of inference/training coordination, #446 unified training data as ATIF items, and #412 moved the scenario and runtime modules. The MLX backend is re-integrated rather than merely merged. Conflicts and how they resolved: - `reef/runtime/adapters/` and `reef/service/deploy/settings.py` are gone on main; the deletions are accepted. The MLX kind registers from `reef/train/__init__.py`, which is the direction the boundary allows. - `reef/surface/base.py` and `reef/inference/sglang/chat.py` take main's restructured versions; `RecoveryRestorer` is re-added in main's ABC idiom. - `reef/service/assembly.py` takes main's version: the function this branch had changed built a runtime from `ServiceSettings`, which no longer exists. Re-integration: - The batch path reads ATIF items. The captured tensors move from a `PolicySample`'s attributes to the item's `extra.reef.training`, where the openclawrl processor already writes them; the tests use main's own `policy_trajectory` helper so both sides agree on that layout. - `MLXServingRuntime` (new) owns the parameters, the admission gate and the served version; `MLXRuntime` keeps candidates, the objective and checkpoints, and reaches the engine through it. The factory returns the pair #432's contract names. The seam follows the state: three methods needed both halves, and all three have the same shape — hold serving, touch the engine, release — so serving exposes an explicit `hold`/`release` rather than a context manager, because activation deliberately stays held until Reef's commit. - `MLXInferenceBackend` implements both `InferenceHandler` and `InferenceBackend`; a receiver that runs as a separate service splits those, an in-process engine does not. The receiver operations that would mean moving memory Reef does not own (`offload`, `onload_*`, `unload_adapter`) raise instead of pretending, which is what the base class does for `load_adapter_files`. - `assistant_message.py` moves under the MLX backend: main's boundary test counts it as an unowned module in `reef/runtime/`, and the MLX backend is its only consumer. One judgement worth flagging: the shipped-config layout test now skips `results/` directories. A results directory records the deployment a past run used; migrating it to the current schema would make it a config that was never executed, which is worse than an old one, because the record's whole value is that it is what ran. Verified: 3528 passed, 88 skipped (slime/ray/inference modules excluded — those dependencies are absent here). mypy clean, ruff 0.14.7, isort 5.13.2, black 24.3.0, and the Python design and statement policies pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SuEa3FyrkW1xc3sjmAJSEB
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation
Processors currently expose different policy and trace sample/batch types, and report assembly adds another chain of public wrappers. Unify the boundary as
TrainingBatch.items: tuple[TrainDataItem, ...], so one batch can contain ATIF trajectories, Harbor tasks, or both. This is a maintainer-requested follow-up to #445.Related: #16 covers broader declarative group rules and introspection; this change preserves recipe-defined readiness rules. Open PR #444 also modifies the CORAL processor and its tests, so those files will need coordination when merging.
Changes
PolicySample,TraceSample, and their three batch subclasses.TrainDataItemisTrajectoryItem | TaskItem; trajectories contain ATIF documents exclusively.extra.reef; policy backends read that extension, while harness methods read trajectories or recorded exchanges.ReportSample, the reported processor'sCandidate, andBatchUnit, plus the TTTD/CORAL row wrappers. Recipes return training items directly; one private_PendingReportowns buffering and consumption state. Optionalgrouping(context)supplies collection-group/retry coordinates, anddecide_groupreceives items directly.Compatibility and operational impact
This changes Python extension contracts: custom processors and consumers must migrate from the removed types to training items, and grouped report processors must supply
grouping(context). Collection groups remain separate from training comparison groups. Old pickled batches containing removed classes are incompatible; service and worker code must be upgraded together. Existing raw records continue to replay through the processors.Current built-in backends explicitly reject task items until a recipe provides task rollout support. Text-only ATIF does not imply generic tokenization: algorithms requiring captured tokens or log probabilities fail explicitly when they are missing. No new runtime dependency is added.
Verification
pre-commit run --all-files: passed.npm run check:docsindocs/site: passed.GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null .venv/bin/python -m pytest tests/ -n 4 -q --tb=short --durations=10: 3436 passed, 48 skipped, 1 failed. The failure istest_bridge_catalogs_paired_checkpoint_metrics_and_blocks_before_second_optimizer: the local filesystem reservation blocks the first job. Reproduced on clean main142e0781..venv/bin/python -m mypy: the same four existing errors in unchangedlora_transport.pyandcheckpoint.py; no new errors.142e0781exactly.AI assistance
OpenAI Codex implemented the refactor, migrated consumers and documentation, added contract tests, and ran the checks above under maintainer direction. The human contributor remains responsible for reviewing and understanding the changes.
Checklist
README.i18n.yaml, or do not affect README content.Review and merge
The Merge Oncall is assigned automatically. See the
maintenance model
for review and merge requirements.