Skip to content

refactor(config): split step definitions into a discriminated union - #526

Open
hertznsk wants to merge 3 commits into
microsoft:mainfrom
hertznsk:refactor/discriminated-step-models
Open

hertznsk wants to merge 3 commits into
microsoft:mainfrom
hertznsk:refactor/discriminated-step-models

Conversation

@hertznsk

Copy link
Copy Markdown
Contributor

Closes #517.

What

Replaces the monolithic AgentDef — one model carrying every step type's fields, with a hand-maintained validator matrix deciding which fields each type: accepts — with concrete Pydantic models united by a static StepDef discriminated union:

  • AgentDef is now the provider-backed LLM step only.
  • New public models: HumanGateStepDef, QuestionsStepDef, ScriptStepDef, MCPStepDef, WaitStepDef, SetStepDef, TerminateStepDef (deliberately no routes), WorkflowStepDef, plus StepBase / RoutableStepBase. All exported from conductor.config and conductor.config.schema.
  • WorkflowConfig.agents: list[StepDef] and ForEachDef.agent: StepDef parse into concrete variants through Field(discriminator="type") with an outer BeforeValidator that canonicalizes the legacy shorthand (missing or explicit-null type) to "agent" at the boundary.
  • Every variant owns exactly the fields meaningful for its kind with extra="forbid", so a misplaced field is rejected next to its step instead of being silently ignored — this is what makes new step types cheap to add (no validator matrix to extend).
  • The published JSON Schema now exposes oneOf + discriminator with per-variant additionalProperties: false, so editors and external tooling get per-type completion and validation.

Compatibility

  • Workflow YAML is unchanged. Omitted type and type: null still load as LLM agents (canonicalized to type: "agent" at parse time).
  • AgentDef(...) keeps working for LLM agents, including AgentDef(type=None, ...), model_dump(exclude_none=True) round-trips, model_copy(update=...), and the synthetic agents built by OutputValidator and the ACA runner.
  • Intentional tightenings: constructing a non-LLM step via AgentDef(type="script", ...) no longer works (use the named step class); a field belonging to a different step type now fails with Pydantic's standard extra_forbidden rather than the custom "<type> agents cannot have '<field>'" messages (variant-owned invariant messages are unchanged); HumanGateStepDef lost the inert model field; human_gate is rejected as a for_each inline agent (concurrent iterations would compete for one interactive channel — route to a gate from the group's routes: instead).

Engine and consumers

  • Main / parallel / for-each dispatch narrows via isinstance, with defensive ExecutionError guards for variants the static validator would reject — a directly-constructed engine never runs it. The for-each guard fires before the empty-source early return so a forbidden inline variant can never pass silently.
  • Generic consumers narrowed the same way: validator routing graph and subworkflow/template scans, conductor show/run/plugin, the MCP conductor_plan_tree (a terminate step now renders with empty routes), executor skills/plugins helpers, and the synthetic provider agents (max_depth removed).
  • FileString (!file) preservation restored on human_gate/questions prompts so file-backed gate prompts keep resolving relative {% include %} paths.

Testing

  • Full suite: 9113 passed, 3 failed — all three failures reproduce identically on main in this environment (two chmod(0o000) permission tests and one timing-sensitive performance test); none touch this change.
  • ruff check clean; ty check src shows only the same pre-existing diagnostics as main; make validate-examples green.
  • New regression coverage: JSON Schema oneOf/discriminator/additionalProperties shape, ForEachDef.agent variant parsing, legacy missing/null-type canonicalization, model_dump round-trips for all nine variants, resume round-trip from a legacy no-type YAML, runtime defensive guards for parallel/for-each groups (including ahead of the empty-source early return), conductor_plan_tree with a terminate step, and an explicit-empty output_template: {} coverage-override regression.

…icrosoft#517)

Replace the monolithic AgentDef that carried every step type's fields
with concrete Pydantic models united by a static StepDef discriminated
union (Field(discriminator="type")):

- AgentDef is now the provider-backed LLM step only; new public models
  HumanGateStepDef, QuestionsStepDef, ScriptStepDef, MCPStepDef,
  WaitStepDef, SetStepDef, TerminateStepDef (no routes) and
  WorkflowStepDef own exactly the fields meaningful for their kind with
  extra="forbid", all exported from conductor.config[.schema].
- WorkflowConfig.agents and ForEachDef.agent are typed list[StepDef] /
  StepDef and parse into concrete variants; an omitted or explicit-null
  type still loads as an LLM agent and is canonicalized to "agent" at
  the boundary. The published JSON Schema now exposes oneOf +
  discriminator with per-variant additionalProperties: false.
- Engine main/parallel/for-each dispatch narrows via isinstance with
  defensive ExecutionError guards for variants the static validator
  would have rejected (a directly-constructed engine skips it);
  human_gate is rejected as a for_each inline agent statically and at
  runtime, including before the empty-source early return.
- Generic consumers narrowed accordingly: validator routing graph and
  subworkflow/template scans, cli run/app/plugin, MCP plan tree,
  executor skills/plugins helpers, synthetic provider agents.
- FileString (!file) preservation restored on human_gate/questions
  prompts; HumanGateStepDef loses the inert model field; foreign fields
  on a step now fail with Pydantic's standard extra_forbidden instead
  of the previous custom "<type> agents cannot have '<field>'"
  messages (per-variant invariant messages unchanged).

Tests migrated to the new contract: concrete constructors for valid
steps, model_validate + structural extra_forbidden assertions for
foreign fields, custom match= kept only for variant-owned invariants.
- JSON Schema shape: oneOf over all nine variants, full discriminator
  mapping, additionalProperties: false on every variant $def,
  TerminateStepDef without routes, ForEachDef.agent exposing the same
  union.
- ForEachDef.agent parsing for agent/workflow/set/mcp inline variants.
- Legacy compatibility: YAML without type, YAML with type: null, direct
  AgentDef(type=None), unknown type as union_tag_invalid, and
  model_dump(exclude_none=True) round-trips for every variant.
- Resume round-trip driven by a legacy no-type YAML file.
- Runtime defensive guards: unsupported variants in parallel and
  for-each groups raise ExecutionError (not AttributeError), including
  forbidden inline agents ahead of the empty-source early return.
- conductor_plan_tree renders a terminate step with empty routes.
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.

refactor(schema): replace monolithic AgentDef with Pydantic discriminated unions for step kinds

1 participant