feat(driver_cfg): add DriverCfg base for tool config sections - #20
Merged
Merged
Conversation
A DriverCfg holds exactly one driver's constructor keyword arguments, so a config section can be declared next to the tool it describes and handed to that tool with as_kwargs(). None is dropped at every layer, keeping the "None means use the known default" contract the drivers rely on. The extra dict is the escape hatch for driver options a config has not been taught yet and for one-off debugging sessions. It is merged key by key, so an unknown key raises TypeError from the driver constructor rather than being silently swallowed. Lives in lob-hlpr because it has no install requirements, which keeps lob-cfg out of the driver repositories that will declare sections. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
Review effort: Lite
Findings: 2
Open (2)
What changed in this PR
Adds a reusable DriverCfg dataclass for representing driver constructor keyword arguments, filtering unset values, and exposing configuration through the package API.
Changes:
- Adds
DriverCfg.as_kwargs()with declared-field,extra, and override precedence. - Adds tests covering defaults, precedence, mutable defaults, and driver construction.
- Exports
DriverCfgfromlob_hlpr.
| File | Description |
|---|---|
src/lob_hlpr/driver_cfg.py |
Implements the driver configuration base class and keyword assembly. |
src/lob_hlpr/__init__.py |
Makes DriverCfg part of the public package API. |
tests/test_driver_cfg.py |
Tests configuration serialization and precedence behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Review findings on #20, both real. extra carried a default and came first, so a subclass could not declare a driver argument without one: the dataclass raised "non-default argument follows default argument". It is keyword only now, which is what Cfg already does for its own extra and source. The extra layer skipped the None filtering the other two layers apply, so extra={"timeout": None} reached the driver and overrode its default or its autodiscovery, which is exactly what the "None means use the known default" contract exists to prevent. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A DriverCfg holds exactly one driver's constructor keyword arguments, so a config section can be declared next to the tool it describes and handed to that tool with as_kwargs(). None is dropped at every layer, keeping the "None means use the known default" contract the drivers rely on.
The extra dict is the escape hatch for driver options a config has not been taught yet and for one-off debugging sessions. It is merged key by key, so an unknown key raises TypeError from the driver constructor rather than being silently swallowed.
Lives in lob-hlpr because it has no install requirements, which keeps lob-cfg out of the driver repositories that will declare sections.