feat(config): add vision_agents.config with load_settings#566
Conversation
Promotes project-level config loading out of `cli/agent/config.py` into a top-level `vision_agents.config` module (peer to `core`, `cli`, `plugins`). Exposes: - `load_settings(cwd=None) -> Settings` — walks up to the nearest `pyproject.toml` and parses `[tool.vision-agents.*]` into a `Settings` dataclass. - `Settings` (with `agent: AgentSettings | None` + a `raw` escape hatch for plugin or user code reading unmodeled subsections). - `find_pyproject(start)` and `parse_entrypoint(spec)` — the lower-level helpers, now reusable outside the CLI. `cli/agent/config.py` shrinks to ~50 lines: it only wraps the neutral `FileNotFoundError` / `ValueError` from `load_settings` into CLI-domain `CliError`, then composes a `ResolvedEntrypoint`. All TOML parsing, walk-up logic, and entrypoint validation now live in one place that the CLI, future subcommands, plugins, and even user `agent.py` files can import.
📝 WalkthroughWalkthroughIntroduces a new centralized configuration module that discovers and parses Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1e735cfb-87c3-46dd-a14d-ade8110c6452
📒 Files selected for processing (2)
agents-core/vision_agents/cli/agent/config.pyagents-core/vision_agents/config.py
| module, _, attribute = spec.partition(":") | ||
| if not module or not attribute: | ||
| raise ValueError( | ||
| f"entrypoint {spec!r} is malformed; expected 'module:attribute'" | ||
| ) |
There was a problem hiding this comment.
Trim entrypoint segments before emptiness checks.
"pkg :runner" and "pkg: runner" currently pass validation but produce downstream import errors. Normalize whitespace in parser so invalid forms fail early with config errors.
Proposed fix
- module, _, attribute = spec.partition(":")
+ module, _, attribute = spec.partition(":")
+ module = module.strip()
+ attribute = attribute.strip()
if not module or not attribute:
raise ValueError(
f"entrypoint {spec!r} is malformed; expected 'module:attribute'"
)| tool = data.get("tool") | ||
| raw = tool.get("vision-agents") if isinstance(tool, dict) else None | ||
| if not isinstance(raw, dict): | ||
| raw = {} | ||
|
|
There was a problem hiding this comment.
Raise ValueError when [tool.vision-agents] exists but is not a table.
Current logic silently downgrades invalid schema to {}. That violates the loader’s malformed-schema contract and can hide user config mistakes.
Proposed fix
- tool = data.get("tool")
- raw = tool.get("vision-agents") if isinstance(tool, dict) else None
- if not isinstance(raw, dict):
- raw = {}
+ tool = data.get("tool")
+ if tool is not None and not isinstance(tool, dict):
+ raise ValueError("[tool] must be a table")
+
+ raw_section = tool.get("vision-agents") if isinstance(tool, dict) else None
+ if raw_section is not None and not isinstance(raw_section, dict):
+ raise ValueError(f"[{TOOL_TABLE}] must be a table")
+ raw = raw_section if isinstance(raw_section, dict) else {}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| tool = data.get("tool") | |
| raw = tool.get("vision-agents") if isinstance(tool, dict) else None | |
| if not isinstance(raw, dict): | |
| raw = {} | |
| tool = data.get("tool") | |
| if tool is not None and not isinstance(tool, dict): | |
| raise ValueError("[tool] must be a table") | |
| raw_section = tool.get("vision-agents") if isinstance(tool, dict) else None | |
| if raw_section is not None and not isinstance(raw_section, dict): | |
| raise ValueError(f"[{TOOL_TABLE}] must be a table") | |
| raw = raw_section if isinstance(raw_section, dict) else {} |
1 participant
Why
vision-agents agentreads the project'spyproject.tomlto find[tool.vision-agents.agent].entrypoint. The TOML parsing, walk-up to the nearestpyproject.toml, and entrypoint validation all currently live insidecli/agent/config.py— fine while there's only one consumer, but they're not CLI-domain concerns. They're project-level config primitives. If a second consumer (a future subcommand, a plugin, or user code insideagent.py) wants to read[tool.vision-agents.<something>], today they'd have to either duplicate the walk-up logic or import fromcli.agent, which is the wrong direction.This PR promotes those primitives into a top-level
vision_agents.configmodule, peer tocore,cli,plugins. The CLI keeps its CLI-specific concerns (wrapping neutral exceptions intoCliError, buildingResolvedEntrypoint) but no longer owns TOML reading or entrypoint format validation. Anyone who needs project config — including a user importing it from their ownagent.py— can callload_settings()and get a typedSettingsinstance.Errors are also relocated to the right layer:
find_pyprojectraisesFileNotFoundError(stdlib, neutral),load_settingsraisesValueErrorfor schema problems. The CLI catches both at the boundary and re-wraps toCliErrorso the redError: …UX is unchanged.What's exported
load_settings(cwd: Path | None = None) -> SettingsSettings— frozen dataclass withproject_root,pyproject_path,agent: AgentSettings | None,raw: dictAgentSettings— frozen dataclass withmodule,attribute, and anentrypointpropertyfind_pyproject(start: Path) -> Path— the walk-up helperparse_entrypoint(spec: object) -> tuple[str, str]— gunicorn-stylemodule:attributevalidatorTOOL_TABLE = "tool.vision-agents"andPYPROJECT_FILENAME = "pyproject.toml"constantsWhy this is an experiment branch
The new module is intentionally small (one
agentsection, plus arawescape hatch) — there's nodeploy, nodev, nolint. The shape is locked enough to be useful but not over-fitted. Marking thisexperiments/lets us merge or iterate based on how the API feels in real plugin/user code before committing to the public surface.