Repository navigation
refactor: share the vera command skeleton in vera_cli/config.py - #258
Conversation
|
|
||
|
|
||
| def render_invocation(run_config: RunConfig, *, command: str) -> str: | ||
| class _Renderable(Protocol): |
There was a problem hiding this comment.
what is this? what's the purpose? why there is no docstring
There was a problem hiding this comment.
It's a typing-only description of "anything with a to_dict()" (a Protocol). It exists so render_invocation, and run_command's --print path, accept both a RunConfig (generate, judge, score, pool) and a pipeline's PipelineRun. Those two share no base class, but both serialize to the config dict --print emits. Before this PR, pipeline carried its own copy of render_invocation (_render) for that reason. You're right that it needed a docstring; added in 145ac85.
| # (fields, defaults, the domain call, run-defining flags) live in the command | ||
| # modules; the identical parts live here. | ||
|
|
||
| Run = TypeVar("Run") |
There was a problem hiding this comment.
what is this supposed to wrap? every run command?
There was a problem hiding this comment.
It doesn't wrap anything. It's a type variable meaning "one resolved run, as this command's resolve_configs returns it": RunConfig for generate, judge, score and pool, PipelineRun for pipeline. Using a type variable instead of a fixed type lets the type checker confirm that what resolve returns is what execute, echo and debug_of receive, for whichever command calls run_command. In 145ac85 I renamed it ResolvedRun, added a comment saying this, and bounded it to _Renderable, which also removed one type: ignore.
145ac85 to
b52c0ba
Compare
There was a problem hiding this comment.
🟢 Approval recommended
The refactor preserves existing command semantics while consistently removing duplicated implementation.
0 open findings
What changed in this PR
Centralizes the shared CLI command lifecycle in vera_cli/config.py without changing behavior.
Changes:
- Adds shared execution, resolution, flag-registration, and config helpers.
- Updates all five command modules to use the shared skeleton.
- Documents the refactor and removes redundant private helpers.
| File | Description |
|---|---|
vera_cli/config.py |
Adds shared command helpers. |
vera_cli/generate.py |
Adopts shared command handling. |
vera_cli/judge.py |
Adopts helpers and shared list validation. |
vera_cli/pipeline.py |
Adopts shared handling and rendering. |
vera_cli/pool.py |
Adopts shared command handling. |
vera_cli/score.py |
Adopts shared command handling. |
vera_cli/README.md |
Documents the command skeleton. |
CHANGELOG.md |
Records the refactor. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
jgieringer
left a comment
jgieringer
left a comment
There was a problem hiding this comment.
configs: simplified.
b52c0ba to
b36658e
Compare
run_command, resolve_with, add_common_flags, config_section and string_list replace code each command repeated; the pass-through _run_config helpers and pipeline's render_invocation copy are gone. Help text, --print output and error messages are byte-identical. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
b36658e to
fc80328
Compare
Summary
The five command modules repeated the same
run,resolve_configs, common flags and pass-through_run_config. Following the #255 review discussion, this moves the shared parts intovera_cli/config.py. No behavior change.Stacked on the pipeline-pooling PR, because both change
vera_cli/pipeline.py.vera_cli/config.py:run_command: resolve,--print,--debug, echo, execute; handles sync and async_executeresolve_with: config-or-flags dispatch plusinvalid <label> configwrappingadd_common_flags:--config,-d/--debug,--print, optional--sample/--into, and the handler, in one fixed orderconfig_section,string_listrun/resolve_configsis now one call; the wrappers stay because tests andpipelineuse them._run_configs (RunConfigis built inline)pipeline._render(render_invocationnow accepts anything withto_dict())judge._string_list(nowconfig.string_list, whichconfig_pathsalso uses)DEFAULTS,_from_cli/_from_config,_execute, run-defining flags. Also kept: the type checks in_from_configthat guard path helpers before the dataclass exists, since removing them would change error messages.vera_cli/README.md("Adding a command"), module docstrings, CHANGELOG. Also fixes a stale code comment pointing at the deletedTODOfile.Not addressed here:
pipelinestill calls other commands' private resolvers. That's recorded in the roadmap Backlog.Test plan
--helpfor all 5 commands is byte-identical to before--printoutput (4 commands) and 7 error-path messages are byte-identical to beforeuv run pytest -m "not live"(1215 passed), with no test changesruff check,ruff format --check,pyright generate score utils vera_cli vera.py🤖 Generated with Claude Code