Phase1: Feat: add unified vera generate command - #191
Conversation
306fd0b to
79b8aa5
Compare
327494b to
9e85cd9
Compare
| @@ -35,14 +35,18 @@ A complete bundle used for both generation and judging has this shape: | |||
|
|
|||
| ```text | |||
| data/NEW_RUBRIC/ | |||
There was a problem hiding this comment.
this should be NEW_TARGET
| ``` | ||
|
|
||
| The commands below use the legacy compatibility entry points and therefore | ||
| refer to `rubric_manifest.json`. |
There was a problem hiding this comment.
remove legacy support in the new CLI
|
|
||
| **These letters are `vera.py`-only and are not the same flags as today's scripts.** `generate.py`/`judge.py` already use `-c`/`-r` for unrelated things (`-c` is `--max-concurrent` in `generate.py` and `--conversation` in `judge.py`; `-r` is `--runs` in `generate.py` and `--rubrics` in `judge.py`). `vera.py` intentionally repurposes them for the `u`/`c`/`j` vocabulary above. There is no coexistence window: Phase 1 of the migration (see [architecture.md#migration-from-current-layout](./architecture.md#migration-from-current-layout)) deletes `generate.py`/`judge.py`/`run_pipeline.py` entirely in the same change that ships `vera.py`, so the old and new meanings of `-c`/`-r` never need to be told apart at runtime. | ||
| **These letters are `vera.py`-only and are not the same flags as the legacy scripts.** | ||
| `generate.py`/`judge.py` already use `-c`/`-r` for unrelated things (`-c` is |
There was a problem hiding this comment.
Specify those are legacy scripts in the process of being removed
| @@ -0,0 +1,157 @@ | |||
| """Canonical run configuration shared by unified CLI input forms.""" | |||
There was a problem hiding this comment.
Clarify better what is the meaning of this
| "sessions": None, | ||
| } | ||
|
|
||
| RUN_FIELDS = ( |
There was a problem hiding this comment.
what is this list? is this a list of the required items?
| dest="print_only", | ||
| help="Print the resolved invocation without executing it", | ||
| ) | ||
| parser.set_defaults(handler=run) |
There was a problem hiding this comment.
where is run defined?
| users: list[ModelSpec], | ||
| personas: list[str], | ||
| context: str, | ||
| **behavior: Any, |
There was a problem hiding this comment.
if behavior is defined below by a list of attributed, why not define it here as well?
| raise ConfigError(f"invalid generation config: {error}") from error | ||
|
|
||
|
|
||
| def _model_config(model: ModelSpec, *, chatbot: bool) -> dict[str, Any]: |
There was a problem hiding this comment.
if I want to have a consistent ModelSpec for both generate and judge, should we just pass a ModelSpec downstream and resolve there?
| ) | ||
|
|
||
|
|
||
| def resolve_generation_personas( |
There was a problem hiding this comment.
what does this function do?
| @@ -0,0 +1 @@ | |||
| """Unified VERA command-line adapters.""" | |||
There was a problem hiding this comment.
add a quick md file to this folder, to explain how things work, i.e. an operation needs to be registered
Rename the placeholder bundle NEW_RUBRIC to NEW_TARGET: the section describes a complete target (rubric, personas, prompts), not a rubric. State that manifest.json is the only manifest the unified CLI reads, with no rubric_manifest.json fallback, and move the judge/pipeline examples under an explicit "Legacy entry points (being removed)" heading that owns that requirement. Name generate.py, judge.py, and run_pipeline.py as legacy entry points being removed one at a time. Refresh the CLI layer description, package table, and generation boundary to the module-per-command layout that replaced the earlier arguments.py/*_config.py/*_command.py split. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Explain how the unified CLI fits together: the module layout, the register()/handler contract and the fact that an operation is unreachable until registered in vera.py, the rules a command must follow (one input form per run, flag presence as the signal, config runs state every field, unknown fields rejected, resolve fully then execute), and the transitional stopgaps in the generation boundary. Point the package docstring at it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Explain what "canonical resolved form" means, that both input forms converge on it before anything executes, and that this module validates and serializes only -- it defines no defaults. Contrast GenerationConfig and InvocationConfig on the axis that separates them: whether a value is part of the run's identity. That is also why invocation controls are the only fields allowed alongside --config. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Address review feedback on the generate CLI. Input handling: drop the hand-maintained RUN_DEFINING_FLAGS tuple. resolve_input now derives the run-defining set by subtracting the command's invocation-only flags from the parsed namespace, so the config-or-flags rule is structural and a newly added flag is covered without being listed anywhere second. The previous guard could only detect membership, not miscategorization; the replacement test asserts all ten run-defining flags are refused alongside --config. Config surface: reject a top-level judging field instead of accepting and ignoring it, and drop the judging.rubrics exclusivity check that belonged to a command that does not exist yet. Generation boundary: move user-model fan-out into the generation domain as run_for_user_models, taking a resolved GenerationConfig rather than eleven loose keywords. Targets stay sequential in the CLI -- run folder names carry only second-granularity timestamps, and max_concurrent applies within one generation, so concurrent starts would collide and multiply the caller's cap. Both the wrapper and its dict-flattening shim are labeled stopgaps. Explain the argparse.SUPPRESS convention, why the parser cannot enforce required flags, and why a selection may resolve to more than one run. Rename resolve_generation_personas to generation_persona_sets and document every function in targets.py. Correct two wrong explanations: run folders are built from the model key, not name (name is the provider display name, legal on the agent config only because the runner filters reserved keys there), and the config path reaches _run_config as dict[str, Any], so type enforcement happens at runtime in GenerationConfig, not statically. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| ROOT = Path(__file__).resolve().parents[1] | ||
| VERA_RUN_CONFIG_ENV = "VERA_RUN_CONFIG" | ||
|
|
||
| # Namespace attributes the dispatcher puts there, not flags the user passed. |
There was a problem hiding this comment.
what does this mean?
There was a problem hiding this comment.
Fair — the wording was cryptic. Rewritten in 2d972ca to name where each attribute comes from and why it matters.
The parsed namespace holds more than flags you typed: dispatch adds command (from add_subparsers(dest="command") in vera.py) and handler (from each command's set_defaults(handler=...)).
That matters because resolve_input now decides which run-defining flags you supplied by looking at what is present on the namespace. If these two were counted, every run would look CLI-defined and --config would always error.
| # Top-level config keys `generate` accepts. `judging` is deliberately absent: | ||
| # until `vera judge` exists there is nothing to do with it, and accepting a key | ||
| # this command ignores is worse than rejecting it. | ||
| ALLOWED_CONFIG_FIELDS = {"generation", "target"} |
There was a problem hiding this comment.
where does the generation flag comes from? would that be vera generate --generation? what does it mean?
There was a problem hiding this comment.
No --generation flag exists — generation is a key inside a --config JSON document, not a CLI flag. Clarified in 2d972ca, which now shows the shape inline:
{"target": "SI", "generation": {"chatbot": {...}, "user": [...], ...}}Your confusion had a real cause I had missed: that set mixes two different kinds of name. target exists in both input forms — as a config key and as the --target flag — while generation is config-only, because on the command line its contents are spelled as individual flags (-c, -u, --turns, …). The comment now says that explicitly, and notes invocation is always allowed and added by resolve_input.
| ) | ||
|
|
||
| # CLI behavior defaults. They live here, beside the flag definitions, rather than | ||
| # in the parser or the schema: the parser uses `argparse.SUPPRESS` so that flag |
There was a problem hiding this comment.
are they discoverable by -h?
There was a problem hiding this comment.
Mostly, but not fully — this found a real gap. Five of the six defaults stated themselves in -h; --sessions stated none.
Fixed in 2d972ca: it now reads (default: one session, using the chatbot's own session type), verified against generate_conversations/runner.py:412 — session_types=None falls back to [getattr(agent, "_session_type", "default")], i.e. a single session.
I also added a line to the DEFAULTS comment making the expectation explicit for whoever adds the next flag: every default there must be visible in -h, because a default a user cannot see is one they cannot predict.
One thing I deliberately did not do: build a parallel DEFAULT_HELP dict deriving all six help strings from DEFAULTS. --turns and --output interpolate the value directly; the other four need prose (None renders as "unlimited", not "None"), and a second dict mirroring the first is the kind of duplication that drifts. Trade-off: those four prose defaults could drift if someone changes a value — though None → unlimited is stable by construction.
Answer three review questions on the input-resolution comments. Explain DISPATCH_ATTRIBUTES by naming where each attribute comes from -- command from add_subparsers in vera.py, handler from set_defaults -- and why excluding them matters: resolve_input infers supplied flags from what is present on the namespace, so counting these would make every run look CLI-defined. State that ALLOWED_CONFIG_FIELDS lists keys inside a --config JSON document, not CLI flags; there is no --generation. Show the JSON shape and note the asymmetry that caused the confusion: target exists in both input forms, while generation is config-only because the command line spells its contents as individual flags. Document the --sessions default in its help text. It was the one CLI default not discoverable via -h; sessions=None runs a single session using the chatbot's own session type. Record the expectation for future flags. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Summary
Note from Luca (human): start from https://github.com/SpringCare/VERA-MH/pull/191/changes#diff-c0fa137c86b92cce877e68ca0c63d3bebfbfe498c2d01026448e2f493e8ce7ee which explains the logic
Adds the first unified CLI feature:
vera generate.This PR is intentionally stacked on #190. It resolves CLI/config/target input
into canonical
RunConfigvalues, then hands them to the generation domain. Itdoes not add judge, score, pool, pipeline, or resume.
Review guide
One module per command owns that command's flags, defaults, resolution, and
domain call. Shared concerns live in two focused modules.
vera.pyvera_cli/generate.pygeneratecommand, top-down:register→run→resolve_configs→_executevera_cli/config.pyvera_cli/targets.pyutils/config_schema.pyvera_cli/README.mddata/SI/manifest.jsonis canonical;rubric_manifest.jsonremains only for thelegacy scripts.
CLI behavior
Both forms resolve SI's persona files and persona context prompt to byte-identical
output.
--targetexpresses whole-target selection and honorsall;--personasis the explicit generation component selector.
Run-defining values come from either CLI flags or JSON config, never a mixture.
A flag is run-defining unless the command names it invocation-only, and the
run-defining set is derived by subtraction rather than listed — so a newly added
flag is covered by the rule automatically.
--sample,--debug, and--printare invocation-only and may accompany config input.
CLI behavior defaults live beside the flag definitions. Config-driven runs state
every behavior field explicitly.
--printemits a config that reproduces thesame run, so resolved runs round-trip.
Unknown top-level config fields are rejected rather than ignored, including
judging— untilvera judgeexists there is nothing to do with it.Transitional boundary
vera generatecalls into the rootgenerate.py, whose reusable functions havenot yet moved into a permanent
generate/package. Two stopgaps live there andare labeled as such:
run_for_user_models, which expands a run's user modelsinto individual generations, and
_legacy_model_config, which flattens aModelSpecinto the dict shape the old signature expects. Both disappear whenthe generation domain accepts
ModelSpecdirectly, which is also what letsgenerateandjudgedescribe models identically.Fan-out is split deliberately: user models expand inside the domain, while
targets stay sequential in the CLI. Run folder names carry only
second-granularity timestamps and
max_concurrentapplies within onegeneration, so concurrent starts would collide and multiply the caller's
concurrency cap against the same provider.
Validation
vera.py,vera_cli/,utils/config_schema.py,generate.py: 0 errors--target SI --printand--personas SI --printproduce identical output, andfeeding the emitted
VERA_RUN_CONFIGback in round-trips to the same config🤖 Generated with Claude Code