Skip to content

refactor: simplify reported feedback and validate references at admission - #445

Merged
BobbyZhouZijian merged 1 commit into
mainfrom
codex/simplify-report-processing
Sep 13, 2026
Merged

BobbyZhouZijian merged 1 commit into
mainfrom
codex/simplify-report-processing

Conversation

@BobbyZhouZijian

Copy link
Copy Markdown
Contributor

Motivation

A supplied report could previously wait for missing inference records or be silently excluded from training by judge(), an eligibility flag, or a score threshold. Validate report references when the report arrives and assemble supplied feedback directly, so broken training data surfaces as an error with its inputs retained.

No accepted RFC for this contract change was found; this PR is a draft for interface review. Related work: #16 discusses grouped-batch rules; #444 overlaps the CORAL processor and its tests, and #432 overlaps dispatcher and recipe documentation.

Changes

  • Reject reports with missing, cross-scenario, duplicate, or non-inference references before persistence. Resolve against stored records and preserve idempotent retries, including retries after source purge.
  • Replace reported-feedback judge() and its TRAIN/WAIT/NEVER decisions with make_sample() returning ReportSample. Keep existing group completion, deduplication, reservation, consumption, and retention behavior.
  • Reject metadata.training.eligible and remove harness score-window filtering. Update SAO, TTTD, CORAL, and Cordis processors to assemble feedback directly.
  • Raise explicit assembly/backend errors for unusable training data instead of silently dropping reports. Retain inputs on failure.
  • Update shipped configurations, documentation, and contract/regression tests.

Compatibility and operational impact

This changes public processor, report, and configuration contracts:

  • Custom reported-feedback processors must migrate from judge() / ReportDecision to make_sample() / ReportSample; WAIT/NEVER decisions, eligibility helpers, and rejection counters are removed.
  • Remove metadata.training.eligible from report payloads, including when its value is true. New reports referencing unavailable records receive HTTP 400 immediately; clients must submit after the referenced inference records exist.
  • Remove max_score / max-score from Cordis harness recipe configuration. Valid scored feedback is no longer filtered by that threshold.
  • Invalid legacy report payloads or references encountered during processor replay now fail explicitly. Missing algorithm-required token/logprob data raises a training error rather than excluding the report.
  • Computed-feedback model judging and existing decide_group() behavior remain in place. No persisted schema or dependency changes are introduced.

Verification

  • source .venv/bin/activate && pre-commit run --all-files — passed. The final additional ordered-group regression test also passed file-scoped pre-commit.
  • GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null .venv/bin/python -m pytest tests/ -n 4 -q --tb=short — 3410 passed, 48 skipped, 1 failed. The failure is test_bridge_catalogs_paired_checkpoint_metrics_and_blocks_before_second_optimizer in tests/reef_service/test_slime_bridge.py: checkpoint storage reservation blocks the first job in this local environment. Reproduced the same failure on clean base commit c0927a6b.
  • Focused report, recipe, HTTP contract, configuration, and trainer suites passed. After adding the final ordered-group regression case, tests/reef_service/test_reported_feedback_processor.py passed all 21 tests.
  • .venv/bin/python -m mypy — 4 errors in 2 unchanged files (lora_transport.py and checkpoint.py, distributed collective typing). Reproduced the same four errors on clean base commit c0927a6b.
  • npm run check:docs in docs/site with Node 22 — passed (154 Markdown files, 39 RST files, 24 routes).
  • git diff --check — passed.

AI assistance

OpenAI Codex implemented the requested contract changes, migrated consumers and documentation, updated tests, ran verification, and prepared this PR under human direction. The human contributor remains responsible for reviewing and understanding the submitted changes and results.

Checklist

  • The change is focused and contains no unrelated cleanup.
  • Tests cover behavior changes, or this pull request does not change behavior.
  • Public interface changes include contract tests, or no public interface changes are present.
  • Affected user and developer documentation is updated, or no documentation update is required.
  • Root README changes update both languages and README.i18n.yaml, or do not affect README content.
  • An accepted RFC is linked, or this change does not require an RFC.
  • Relevant pre-commit and test checks pass.
  • Non-trivial AI assistance is disclosed above, or none was used.

Review and merge

The Merge Oncall is assigned automatically. See the
maintenance model
for review and merge requirements.

@github-actions

Copy link
Copy Markdown

Merge Oncall: @BobbyZhouZijian

The merge oncall will route reviews and keep this pull request moving toward a clear decision.

@github-actions github-actions Bot added area: ci Repository automation, continuous integration, and developer tooling area: core Core contracts and scenario lifecycle area: docs User and developer documentation area: examples Examples, recipes, and reference deployments area: harness Harness integration, recipes, and evolution surfaces area: service Service APIs, deployment, and command-line interfaces area: training Training execution, runtimes, algorithms, and pipeline reliability labels Sep 13, 2026
@BobbyZhouZijian
BobbyZhouZijian marked this pull request as ready for review September 13, 2026 18:46
@BobbyZhouZijian
BobbyZhouZijian enabled auto-merge (squash) September 13, 2026 18:47
@BobbyZhouZijian
BobbyZhouZijian merged commit 142e078 into main Sep 13, 2026
18 checks passed
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: ci Repository automation, continuous integration, and developer tooling area: core Core contracts and scenario lifecycle area: docs User and developer documentation area: examples Examples, recipes, and reference deployments area: harness Harness integration, recipes, and evolution surfaces area: service Service APIs, deployment, and command-line interfaces area: training Training execution, runtimes, algorithms, and pipeline reliability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant