Repository navigation
feat: one first_speaker setting replaces persona_speaks_first - #293
Merged
Merged
Conversation
jgieringer
approved these changes
Oct 27, 2026
jgieringer
left a comment
Collaborator
There was a problem hiding this comment.
LGTM - unsure if the role enum has been removed or if it's usable here
|
|
||
| # Who opens the conversation, in the CLI's u/c vocabulary: the user-side LLM | ||
| # playing the persona, or the chatbot under test. | ||
| FIRST_SPEAKERS = ("user", "chatbot") |
Collaborator
There was a problem hiding this comment.
do we want to use the Role enums?
Collaborator
Author
There was a problem hiding this comment.
in your Enum era
Collaborator
Author
There was a problem hiding this comment.
but it's a good question, and the answer is that now Role uses the old wording so we can do that once we call the thing chatbot consistency everywhere
luca-belli
force-pushed
the
feat/first-speaker
branch
from
October 27, 2026 19:14
05bb655 to
6c66616
Compare
The flag said who speaks first from the chatbot's side (--provider-speaks-first) and the config field from the persona's (persona_speaks_first), so one had to be flipped to match the other. Both become first_speaker, "user" or "chatbot", in the CLI's u/c vocabulary: --first-speaker on the CLI (default user), first_speaker in a config file. A config with the old field stops with an error naming the new one. configs/ is updated. The generation domain still takes a bool; run_for_user_models converts at that one boundary. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
luca-belli
force-pushed
the
feat/first-speaker
branch
from
October 27, 2026 19:28
6c66616 to
3a1046c
Compare
This was referenced Oct 27, 2026
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.
Summary
From your review on #289: "this is confusing. we should add a ROLE and the role speaks first." Stacked on #291. It should ship with #289 so configs change only once.
The flag named the speaking order from the chatbot's side (
--provider-speaks-first) and the config field from the persona's side (persona_speaks_first), so the code had to flip one to match the other. Now there's one setting, named after who speaks:--provider-speaks-first(off by default)--first-speaker {user,chatbot}(defaultuser)"persona_speaks_first": true/false"first_speaker": "user"/"chatbot"Why this shape
user,chatbot), the same words-uand-cuse, rather than the internalRole.PERSONA/Role.PROVIDERnames.Role.Rolelives inllm_clients/llm_interface.py, andutils/(whereGenerationConfiglives) must not importllm_clients/; that's a layer rule. MovingRoleintoutils/role.py, asarchitecture.mdasks, would let the schema use the enum. That's a separate internal cleanup.run_for_user_modelsconverts at that one boundary (first_speaker == "user"). ThreadingRolethrough the runner and simulator belongs with docs: design draft — run naming, order of events, and its enforcement #268's internal renames.persona_speaks_firststops with "generation.persona_speaks_first was renamed: use "first_speaker": "user" in place of true, or "chatbot" in place of false", rather than a generic missing-field error.Changes
utils/config_schema.py:GenerationConfig.first_speaker, validated againstFIRST_SPEAKERS = ("user", "chatbot")vera_cli/generate.py: the flag,DEFAULTS,RESULT_FIELDS, and the rename errorconfigs/*.json, the replay and e2e test configs,cli.md,vera-cli-use-cases.md,architecture.md, and CHANGELOG (under Breaking)Testing
Ruff and pyright pass. I haven't run the tests, per your rule; CI isn't triggered automatically because this PR is stacked.
🤖 Generated with Claude Code