Skip to content

surface voice_config in Agent.get_config and fail loudly on codegen n… - #113

Merged
berges99 merged 6 commits into
mainfrom
fix/voice-config-roundtrip-codegen
Aug 4, 2026
Merged

surface voice_config in Agent.get_config and fail loudly on codegen n…#113
berges99 merged 6 commits into
mainfrom
fix/voice-config-roundtrip-codegen

Conversation

@berges99

@berges99 berges99 commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

…o-ops

voice_config rode extra="allow" and was write-only: set-config saved it but get-flow never returned it, so Studio fell back to stale values on every refresh. Agent.get_config now emits it JSON-safely (dict passthrough, callable as "", VoiceConfig dumped).

Also fixes the write-side silent failures:

  • annotated assignments (agent: Agent = Agent(...)) are now transformed (AnnAssign support in cst_utils + AgentConfigSetter)
  • factory entry points (agent = build()) error instead of corrupting the call
  • unmatched shapes (agent = _agent) exit non-zero instead of phantom-saving; transformers report matched=True so idempotent saves still succeed, and idempotent ops (re-add tool/mcp, remove absent tool/step, dup edge) opt out via allow_noop

…o-ops

voice_config rode extra="allow" and was write-only: set-config saved it
but get-flow never returned it, so Studio fell back to stale values on
every refresh. Agent.get_config now emits it JSON-safely (dict passthrough,
callable as "<name>", VoiceConfig dumped).

Also fixes the write-side silent failures:
- annotated assignments (agent: Agent = Agent(...)) are now transformed
  (AnnAssign support in cst_utils + AgentConfigSetter)
- factory entry points (agent = build()) error instead of corrupting the call
- unmatched shapes (agent = _agent) exit non-zero instead of phantom-saving;
  transformers report matched=True so idempotent saves still succeed, and
  idempotent ops (re-add tool/mcp, remove absent tool/step, dup edge) opt
  out via allow_noop
@cursor

cursor Bot commented Aug 4, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches all codegen transformers and Studio config round-trip; behavior changes (errors on previously silent no-ops) could break workflows that relied on wrong FQNs or aliased agents, but reduces risk of corrupt or unchanged source files.

Overview
voice_config now round-trips through Studio: Agent.get_config() exposes voice_config (dict, callable placeholder, or VoiceConfig dump) so get-flow matches what set-config writes, fixing stale voice settings on refresh.

Codegen CST handling is broader and stricter: Entry resolution and transformers now support annotated assignments (agent: Agent = …), import aliases, and module-qualified constructors; factory entry points (agent = build()) error instead of getting kwargs injected; aliased assignments (agent = _agent) fail with a non-zero exit instead of phantom success. Shared apply_operation logic treats unchanged output as failure unless the transformer set matched=True (idempotent save) or allow_noop=True (re-add tool/MCP, remove absent step/edge, etc.). Workflow ops validate the FQN variable via has_step_expr where aliases still use .step() under the expected name.

Minor: Eval discovery skips evalconf.yaml; extensive tests cover the new shapes and behaviors.

Reviewed by Cursor Bugbot for commit 0d128a4. Bugbot is set up for automated code reviews on this repo. Configure here.

Comment thread python/timbal/core/agent.py
Comment thread python/timbal/codegen/transformers/set_config.py
Comment thread python/timbal/codegen/transformers/set_config.py
…lconf from eval discovery

voice_config rode extra="allow" and was write-only: set-config saved it but
get-flow never returned it, so Studio fell back to stale values on every
refresh. Agent.get_config now emits it JSON-safely (dict passthrough,
callable as "<name>" with a callable schema variant, VoiceConfig dumped).

Codegen write-side hardening:
- annotated assignments (agent: Agent = Agent(...)) now work for agent- and
  tool-level set-config (AnnAssign support in cst_utils, AgentConfigSetter,
  ToolConfigSetter, and the inline-tool migration insert point)
- factory entry points (agent = build(), local or imported) error instead of
  corrupting the call; aliased (Agent as A) and module-qualified
  (timbal.Agent) constructors are canonicalized so they keep working
- unmatched shapes (agent = _agent) exit non-zero instead of phantom-saving;
  transformers report matched=True so idempotent saves still succeed, and
  idempotent ops (re-add tool/mcp, remove absent tool/step, dup edge) opt out
  via allow_noop

evals: evalconf.yaml matches the eval*.yaml glob, so discover_eval_files
collected the shared config file as an eval suite and the CLI crashed with
"Invalid eval file" before running anything. Discovery now skips it.
Comment thread python/timbal/codegen/transformers/__init__.py
The unchanged-output guard only exempted transformers with allow_noop or
matched=True, which remove-edge, set-param, set-position, and add-step
never set — so idempotent calls (removing an already-absent edge, setting
a param/position to its current value, re-adding an identical step) raised
"produced no changes" instead of succeeding. Deletes/adds opt out via
allow_noop; the setters track matched so unmatched shapes still fail loudly.
Comment thread python/timbal/codegen/transformers/add_tool.py
Comment thread python/timbal/codegen/transformers/remove_tool.py
Comment thread python/timbal/codegen/transformers/set_position.py
… and set-position

collect_assignments now resolves `agent: Agent = Agent(...)`, but these three
transformers still only matched plain Assign nodes when updating the
constructor:

- add-tool passed validation, emitted the tool variable + import, but never
  appended to tools=[...] — and inserted the variable after the agent
- remove-tool left the tool in place and reported success via allow_noop
- set-position raised "produced no changes" instead of updating metadata

Add leave_AnnAssign to ToolAdder, ToolRemover, and ConstructorPositionSetter
(mirroring their leave_Assign logic), and teach ToolAdder's leave_Module
insertion scan to account for AnnAssign statements. Regression tests exec the
generated source for each operation against the annotated shape.
Comment thread python/timbal/codegen/transformers/set_config.py
…osition

Workflow steps declared as `agent_a: Agent = Agent(...)` resolve through
collect_assignments, so validation passed, but StepConstructorConfigSetter
and StepPositionSetter only transformed plain Assign nodes — matched stayed
false and the save failed with "produced no changes".

Add leave_AnnAssign to both (extracting the kwarg merge into a shared helper
in StepConstructorConfigSetter) and cover the annotated shape with regression
tests for both operations.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit b3a6851. Configure here.

Comment thread python/timbal/codegen/transformers/__init__.py
Systematic audit of all transformers against annotated assignments, aliased
entry points, and the allow_noop/matched no-op guard:

- add-mcp: MCPAdder now handles annotated agents (tools list, in-place server
  replacement, collision check) and inserts server variables before them
- add-step: validate the entry point exists; insert new step variables before
  annotated/aliased entry points instead of after their .step() call
  (NameError); update annotated step variables in place on re-add
- remove-step: validate the entry point so a wrong fqn fails loudly instead
  of riding allow_noop to a phantom success
- cst_utils: _BareFunctionWrapper inserts the Tool wrapper before the first
  entry-point reference; collect_chained_step_names sees annotated workflows;
  new has_step_expr helper keeps alias workflows (workflow = _wf) working
- remove_unused_code: clean up annotated dead variables too

Regression tests for each fix; add-edge/remove-edge/set-param verified safe
(Workflow type check + require_step).
@berges99
berges99 merged commit d6c8cf9 into main Aug 4, 2026
11 checks passed
berges99 added a commit that referenced this pull request Aug 5, 2026
Conflicts were codegen-only: main's transformer hardening (PR #113 —
annotated assignments, aliased/module-qualified entry points, loud no-op
failures) landed on the same files our dedupe refactor consolidated.

Resolution keeps the shared-helper structure and re-expresses main's
behavior through it:
- insert_before_assignments grew AnnAssign anchors and an optional
  step_calls_of anchor — one change instead of main's five copies
- assignment_resolves_to accepts AnnAssign; StepCallRewriter sets matched
- allow_noop / matched / leave_AnnAssign / has_step_expr validation applied
  per transformer exactly as on main

All 505 codegen tests (incl. main's new coverage and the full suite
(3080) pass.
EOF
)

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants