Skip to content

Harden BYOB config validation#270

Open
fallintoplace wants to merge 1 commit into
NVIDIA-NeMo:mainfrom
fallintoplace:fix/byob-config-validation
Open

Harden BYOB config validation#270
fallintoplace wants to merge 1 commit into
NVIDIA-NeMo:mainfrom
fallintoplace:fix/byob-config-validation

Conversation

@fallintoplace

Copy link
Copy Markdown

Summary

  • replace BYOB runtime config assert statements with explicit validation that raises ValueError
  • apply the same treatment to the translation config loader so python -O does not silently disable those checks
  • add focused regression coverage for missing fields, threshold validation, and the translation FAITH-eval guard

Root cause

ByobConfig.from_yaml() and ByobTranslationConfig.from_yaml() used assert for user-facing config validation. Under python -O, those assertions are stripped entirely, which means required-field and threshold checks can disappear at runtime.

What changed

I added a small _require() helper in src/nemotron/steps/byob/runtime/config.py and converted the runtime config validators over to explicit checks. The error messages stay the same, but config failures now consistently surface as ValueError even when Python optimization is enabled.

Validation

  • python3 -m py_compile src/nemotron/steps/byob/runtime/config.py tests/steps/byob/test_byob_config.py
  • uv run --no-project --with pytest --with numpy --with pyyaml --with datasets --with requests env PYTHONPATH=src pytest tests/steps/byob/test_byob_config.py

@chrisalexiuk-nvidia

Copy link
Copy Markdown
Contributor

Need signoff.

@fallintoplace

Copy link
Copy Markdown
Author

@chrisalexiuk-nvidia Let me have a look and push this.

(cherry picked from commit df5acc9)
Signed-off-by: Minh Vu <vuhoangminh97@gmail.com>
@fallintoplace
fallintoplace force-pushed the fix/byob-config-validation branch from 8f9b02b to f9db7be Compare July 8, 2026 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants