Skip to content

refactor(reward): centralize post-score operations - #31

Draft
leviking98z-rgb wants to merge 1 commit into
refactor/bounded-recipe-compositionfrom
refactor/reward-ownership
Draft

refactor(reward): centralize post-score operations#31
leviking98z-rgb wants to merge 1 commit into
refactor/bounded-recipe-compositionfrom
refactor/reward-ownership

Conversation

@leviking98z-rgb

Copy link
Copy Markdown
Owner

Summary

  • Add unirl.reward.ops as the single framework owner for reward-service dispatch, TensorRef materialization, scalar statistics, and lineage credit assignment.
  • Route synchronous AR, diffusion, PE, unified-model, agentic, async, and multi-reward eval paths through that owner. Async reap keeps a dedicated attach-without-hydration operation so transfer remains deferred until a buffered sample is selected.
  • Move propagate_rewards off the Sample transport type and into the reward layer.
  • Extend the existing framework contract guard so trainers cannot grow direct score_and_attach or lineage-propagation paths again.

This is an ownership refactor, not a LOC-reduction PR: it is 270 insertions / 148 deletions (net +122), including 49 lines of static guard. It removes seven copies of reward hydration/statistics logic and prevents the wire type from owning reward policy.

This is stacked on refactor/bounded-recipe-composition (#30 in this fork), not on main.

Related Issue

N/A

Test Plan

  • pre-commit run --all-files — passed, including recipe composition, architecture, framework, and experimental-boundary guards.
  • python -m compileall -q unirl lint — passed.
  • python lint/check_framework_contracts.py — passed; reports one reward-operation contract in addition to the existing framework contracts.
  • python lint/check_architecture_boundaries.py — passed (476 files / 4,279 internal import edges).
  • One-off CPU harness under torch==2.7.1+cpu exercised service dispatch, real-tensor materialization, component materialization, count/sum/mean statistics, and two-parent mean credit assignment — passed.
  • Not run: GPU training, rollout, or remote reward-service smoke. Reward backends and score numerics are unchanged.

Compatibility / Risk

  • Sample.propagate_rewards(...) is removed. Out-of-tree callers must use unirl.reward.ops.propagate_rewards(sample, ...).
  • Trainer behavior is preserved: synchronous paths materialize at the same post-score point, and async paths still defer materialization until training consumes the selected batch.
  • RewardOutcome.mean preserves the previous float32 mean and returns 0.0 when no reward is attached.
  • No recipe, checkpoint, dataset, backend inference, or HTTP wire-format change.

Reviewer Notes

Checklist

  • I reviewed the changed code and removed unrelated/generated artifacts.
  • I updated tests, docs, and configs where needed, or explained why not.

@leviking98z-rgb
leviking98z-rgb force-pushed the refactor/bounded-recipe-composition branch from 56c9bf7 to 86d9673 Compare August 2, 2026 17:28
@leviking98z-rgb
leviking98z-rgb force-pushed the refactor/reward-ownership branch from 3cdf886 to 39165d1 Compare August 2, 2026 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant