Skip to content

refactor(config): non-breaking hygiene pass on RunConfig #807

Description

@nabinchha

Summary

RunConfig (packages/data-designer-config/src/data_designer/config/run_config.py) has grown to 15 flat fields spanning six unrelated concerns and has accumulated a few maintainability hazards. This issue tracks a non-breaking hygiene pass that cleans up the internals without changing the public API — every dd.RunConfig(...) call site, field name, type, and default stays identical.

This deliberately scopes out the deeper "should these be nested sub-configs" question (that would be a breaking change and is a separate discussion).

Motivation

Adding a single knob today is shotgun surgery. For example, #745 (add max_concurrent_row_groups) had to touch the class docstring, the field definition, the builder wiring, and two test files — and the field had to be documented in two separate places that can silently drift.

Smells this addresses

  • Duplicated, drift-prone docs — every field is documented twice (class docstring Attributes: block and Field(description=...)), inconsistently, in a different order than the field definitions.
  • Hidden field mutationnormalize_shutdown_settings overwrites a user's shutdown_error_rate with 1.0 when disable_early_shutdown=True, so the value doesn't round-trip (the interface layer even has to warn about this).
  • Deprecation baggage in the primary moduleThrottleConfig, to_request_admission_tuning, and _THROTTLE_DEPRECATION_MESSAGE (~80 lines) share the file with the live model; throttle is a phantom field (accepted via validator, no typed attribute); ThrottleConfig.ceiling_overshoot is accepted but never forwarded.
  • Inconsistent field declaration style — mix of bare defaults and Field(...) with no clear rule.

Explicitly out of scope: the low-cohesion / grab-bag structure itself (15 flat fields). That flat surface is preserved intentionally.

Proposed changes

  • Extract deprecated throttle code (ThrottleConfig, to_request_admission_tuning, _THROTTLE_DEPRECATION_MESSAGE) to a run_config_deprecated.py sibling; keep the public re-export from config/__init__.py unchanged.
  • Make Field(description=...) the single source of truth for field docs; give every field a description and remove the duplicated Attributes: block from the class docstring.
  • Replace the shutdown_error_rate mutation in normalize_shutdown_settings with a computed effective_shutdown_error_rate property; repoint the engine consumers (e.g. dataset_builder.py:812, validation.py:122) to it so the user's input round-trips faithfully.
  • Group fields by concern with section comments and normalize Field(...) usage across all fields.

Acceptance criteria

  • No changes to public field names, types, or defaults; existing dd.RunConfig(...) usage is unaffected.
  • disable_early_shutdown=True still yields an effective error rate of 1.0 at enforcement time, but shutdown_error_rate reads back the user's original value.
  • make check-all and the config test suite pass; new/updated tests cover the effective_shutdown_error_rate behavior.

Notes

Follow-up (not this issue): if the concurrency/observability clusters keep growing, consider nested sub-configs with a flat-compatible constructor (backward-compatible via a before-validator). Tracked separately if/when warranted.

Related: #745

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions