Skip to content

test: expand config tests — BakeryConfig/LoraConfig __post_init__ coverage - #28

Merged
marksverdhei merged 2 commits into
marksverdhei:mainfrom
hai-pilgrim:test/config-post-init
Mar 29, 2026
Merged

marksverdhei merged 2 commits into
marksverdhei:mainfrom
hai-pilgrim:test/config-post-init

Conversation

@hai-pilgrim

Copy link
Copy Markdown

Summary

Expands tests/test_config.py from 2 tests to 22, adding thorough coverage for both BakeryConfig.__post_init__ and LoraConfig.__post_init__.

New tests

TestBakeryConfigPostInit

  • remove_unused_columns is always False regardless of what's passed
  • String-typed float fields (learning_rate, temperature, sampling_temperature, warmup_ratio, max_grad_norm) are coerced to float — mimics HfArgumentParser.parse_yaml_file behaviour where numeric values sometimes arrive as strings
  • String-typed int fields (num_trajectories, trajectory_length, logging_steps, seed) are coerced to int
  • Invalid string values raise ValueError that names the offending field
  • Native float values pass through unchanged
  • system_prompt loaded from system_prompt_file when system_prompt is None
  • Explicit system_prompt takes priority over system_prompt_file

TestLoraConfigPostInit

  • target_modules="all" normalised to "all-linear"
  • target_modules=["all"] normalised to "all-linear"
  • target_modules=["all-linear"] normalised to "all-linear"
  • target_modules="all-linear" unchanged
  • Explicit module list unchanged

Test plan

All 22 tests pass (uv run pytest tests/test_config.py).

🤖 Generated with Claude Code

@marksverdhei

Copy link
Copy Markdown
Owner

Review

Good comprehensive config test coverage — this is a strict superset of #15. The parametrized coercion tests and system_prompt_file tests are particularly valuable.

Needs rebase: Now conflicts with main after #16 was merged (both modify test_config.py at the same location). Please rebase onto main and resolve the conflict — the test additions themselves are clean and ready to merge after that.

…onfig post_init

BakeryConfig.__post_init__:
- remove_unused_columns always False
- float string coercion for learning_rate, temperature, sampling_temperature,
  warmup_ratio, max_grad_norm
- int string coercion for num_trajectories, trajectory_length, logging_steps, seed
- invalid float/int strings raise ValueError with field name in message
- native float values pass through unchanged
- system_prompt loaded from file when system_prompt is None
- system_prompt takes priority over system_prompt_file

LoraConfig.__post_init__ target_modules normalization:
- "all" string → "all-linear"
- ["all"] list → "all-linear"
- ["all-linear"] list → "all-linear"
- "all-linear" string → unchanged
- explicit module list → unchanged

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@hai-pilgrim
hai-pilgrim force-pushed the test/config-post-init branch from 596b5d0 to be52f7b Compare March 29, 2026 17:48
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@marksverdhei marksverdhei left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed. Good restructuring into TestBakeryConfigPostInit and TestLoraConfigPostInit classes. The parametrized string coercion tests cover all float/int fields. Error path tests (invalid strings raising ValueError) and system_prompt_file priority logic are solid additions. Removes the old duplicate test_bakery_config_numeric_coercion. LGTM.

@marksverdhei
marksverdhei merged commit 7da97c1 into marksverdhei:main Mar 29, 2026
2 checks passed
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