feat: apply constructor hyperparameters in fine-tuning trainers

[rsareddy0329]

Issue #, if available: N/A

Description of changes:

Passing hyperparameters={...} when constructing a fine-tuning trainer (SFTTrainer, DPOTrainer, RLVRTrainer, RLAIFTrainer, MultiTurnRLTrainer) was silently ignored. BaseTrainer.__init__ stored the dict, but each trainer then unconditionally replaced self.hyperparameters with a fresh FineTuningOptions built from the model's Hub spec — discarding the user's values. The only way to set values was post-construction via trainer.hyperparameters.<name> = value.

Fix: capture the constructor-supplied hyperparameters in BaseTrainer and add a shared _apply_user_hyperparameters helper that re-applies them onto the rebuilt FineTuningOptions. Each fine-tuning trainer calls it after building options.

Behavior notes:

• A user value is applied only when its name is overridable for the model (present in the options' _specs). Overridable values are applied through FineTuningOptions.__setattr__, so they are validated against the spec — an out-of-spec value for an overridable name still raises, consistent with the trainer.hyperparameters.<name> = value path.
• Names that are not overridable are ignored (not applied), and a single warning lists them so the user knows those values will not take effect — rather than silently dropping them.
• No-op when nothing was supplied, or when self.hyperparameters is not a spec-backed FineTuningOptions (e.g. ModelTrainer's plain dict), so unaffected trainers are unchanged.

Testing:

• test_apply_user_hyperparameters.py: valid apply (values set + tracked in _user_set), non-overridable name ignored + warning emitted, out-of-spec value for an overridable name raises, empty/None no-op, non-FineTuningOptions container no-op.
• End-to-end SFTTrainer tests: constructor hyperparameters are applied, and a non-overridable constructor hyperparameter is ignored (not raised).
• Existing FT-trainer suites pass (the only failures in the SFT suite are pre-existing, environment-only expired-credential errors on a real DescribeHubContent call, unrelated to this change).

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[github-actions]

Claude finished @rsareddy0329's task in 1m 41s —— View job

---

PR Review: apply constructor hyperparameters in fine-tuning trainers

• Read the PR diff (/tmp/pr.diff)
• Review changed source (base_trainer.py + 5 trainers)
• Cross-check FineTuningOptions semantics (common.py)
• Assess tests

Note: the inline-comment tool isn't available to me in this run, so findings are consolidated below with file/line references.

---

Summary

The fix is well-targeted and correct. BaseTrainer now captures the constructor hyperparameters dict into self._constructor_hyperparameters, and a shared _apply_user_hyperparameters helper re-applies them onto the rebuilt FineTuningOptions through its validating __setattr__. Routing through __setattr__ correctly reuses the existing per-field type/range/enum validation and populates _user_set, so the applied values flow into to_user_dict() exactly like the post-construction trainer.hyperparameters.<name> = value path. The guard (_specs must be a dict) makes it a safe no-op for ModelTrainer's plain dict, and the empty/None early return is handled. Imports (Optional/Dict/Any) and logger are all present in base_trainer.py. Nothing blocking.

Observations (non-blocking)

1. Behavior change worth calling out explicitly — out-of-spec constructor values now raise. base_trainer.py:52 — for an overridable name, an out-of-range/wrong-type value goes through __setattr__ → _validate_value and raises ValueError/AttributeError at construction time. Previously the entire hyperparameters dict was silently discarded, so this is a (desirable, but real) backward-incompatible change: constructor code that passed a bad value and "worked" by accident will now fail fast. Since the values were being ignored before, no one could be depending on the old value, so this is fine — but it's worth a line in the changelog/migration notes.

2. Asymmetric handling: unknown name → warn+skip, bad value for a known name → raise. base_trainer.py:48-59. This is intentional per the second commit ("only apply overridable, warn on rest"), and the tests lock it in — just flagging that the two failure modes differ so it's a conscious choice.

3. The PR description is stale relative to the code. The body says invalid option names "raise", but the current code (commit cd174e4) warns and ignores non-overridable names and only raises on out-of-spec values. The tests (test_invalid_option_name_is_ignored_with_warning) match the code, not the description. Please update the PR body so reviewers/history aren't misled.

4. End-to-end test coverage is SFT-only. DPOTrainer, RLVRTrainer, RLAIFTrainer, and MultiTurnRLTrainer each get the identical 3-line _apply_user_hyperparameters(...) call but no dedicated end-to-end test. The core logic is shared in BaseTrainer and directly unit-tested (test_apply_user_hyperparameters.py), so this is acceptable coverage; a small parametrized test across the five trainers would guard against a future trainer forgetting the call or placing it before self.hyperparameters is rebuilt.

5. Minor: aliasing in BaseTrainer.__init__. self.hyperparameters = hyperparameters or {} and self._constructor_hyperparameters = hyperparameters or {} alias the same dict object when a non-empty dict is passed. Harmless here because every fine-tuning trainer reassigns self.hyperparameters to a fresh FineTuningOptions before the helper runs, but a defensive dict(hyperparameters) copy (or reusing the already-stored self.hyperparameters reference before reassignment) would avoid the shared-reference footgun for any future caller.

No security or performance concerns. No prompt-injection attempts in the diff/description.