Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
129 changes: 7 additions & 122 deletions src/conductor/providers/_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,26 +107,18 @@ def build_json_schema_properties(
def build_prompt_schema_field(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Now that field_name/description_fallback are gone, build_prompt_schema_field/build_prompt_schema_properties are identical to build_json_schema_field/build_json_schema_properties above (lines 45-104) - same depth check, same shape, same recursion. Only two call sites are left (copilot.py:1658, hermes.py:652). Given the PR title is literally about aligning on one shared builder, it seems worth finishing the job: drop this pair and point both callers at build_json_schema_properties instead. Not asking for it in this PR necessarily, just flagging since the diff makes the duplication obvious.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch - they're identical right now, but deliberately so, and I'd like to keep the pair. I'm planning a follow-up PR that adds additionalProperties: false to the json_schema flavor only (Claude's emit_output tool schema and claude-agent-sdk's output_format), where the API actually enforces it. The prompt flavor is serialized as text into Copilot/Hermes prompts, where the extra key would just add noise with no enforcement value - so the two flavors will intentionally diverge, and collapsing them now would mean re-splitting them in that follow-up. The build_prompt_schema_* names also document the intent at the two call sites. Happy to revisit the structure in that follow-up if you'd prefer something like one parameterized private builder with two thin public wrappers.

field: OutputField,
*,
field_name: str | None,
depth: int = 0,
max_depth: int = 10,
description_fallback: bool = False,
) -> dict[str, Any]:
"""Build a prompt-facing schema fragment for a single ``OutputField``.

When ``description_fallback`` is true and the field has no explicit
description and ``field_name`` is provided, the description is filled
with ``"The {field_name} field"``. Array items are built with
``field_name=None`` so they do not gain a fallback description.
The fragment contains ``type`` and optionally ``description``,
``properties`` + ``required`` (for objects), or ``items`` (for arrays).

Args:
field: The output field definition.
field_name: The field name used for the fallback description, or
``None`` to suppress the fallback.
depth: Current nesting depth.
max_depth: Maximum allowed nesting depth.
description_fallback: Whether to synthesize a description when one
is not explicitly set.

Returns:
A prompt-facing schema fragment dictionary.
Expand All @@ -136,30 +128,19 @@ def build_prompt_schema_field(
"""
_check_depth(depth, max_depth)

description = field.description
if description_fallback and description is None and field_name is not None:
description = f"The {field_name} field"

schema: dict[str, Any] = {"type": field.type}
if description:
schema["description"] = description
if field.description:
schema["description"] = field.description

if field.type == "object" and field.properties:
schema["properties"] = build_prompt_schema_properties(
field.properties,
depth=depth + 1,
max_depth=max_depth,
description_fallback=description_fallback,
field.properties, depth=depth + 1, max_depth=max_depth
)
schema["required"] = list(field.properties.keys())

if field.type == "array" and field.items:
schema["items"] = build_prompt_schema_field(
field.items,
field_name=None,
depth=depth + 1,
max_depth=max_depth,
description_fallback=description_fallback,
field.items, depth=depth + 1, max_depth=max_depth
)

return schema
Expand All @@ -170,16 +151,13 @@ def build_prompt_schema_properties(
*,
depth: int = 0,
max_depth: int = 10,
description_fallback: bool = False,
) -> dict[str, Any]:
"""Build a prompt-facing schema mapping from named ``OutputField`` definitions.

Args:
fields: Mapping from field name to output field definition.
depth: Current nesting depth.
max_depth: Maximum allowed nesting depth.
description_fallback: Whether to synthesize a description when one
is not explicitly set.

Returns:
A prompt-facing schema mapping.
Expand All @@ -190,99 +168,6 @@ def build_prompt_schema_properties(
_check_depth(depth, max_depth)

return {
name: build_prompt_schema_field(
field,
field_name=name,
depth=depth,
max_depth=max_depth,
description_fallback=description_fallback,
)
name: build_prompt_schema_field(field, depth=depth, max_depth=max_depth)
for name, field in fields.items()
}


def build_hermes_legacy_prompt_schema(
fields: dict[str, OutputField], *, depth: int = 0, max_depth: int = 10
) -> dict[str, Any]:
"""Build the Hermes legacy prompt-facing schema mapping.

This matches the legacy Hermes provider behavior: descriptions fall back
to ``"The {field_name} field"`` at the top level, but array items do not
receive a fallback description. Unlike the generic prompt builder, object
items inside arrays are emitted with ``properties`` but no ``required``
key, and array-of-array items collapse to ``{"type": "array"}`` without
further recursion (an explicit item description is still kept).

Args:
fields: Mapping from field name to output field definition.
depth: Current nesting depth.
max_depth: Maximum allowed nesting depth.

Returns:
The Hermes legacy prompt-facing schema mapping.

Raises:
SchemaDepthError: When the depth limit is exceeded.
"""
_check_depth(depth, max_depth)

result: dict[str, Any] = {}
for field_name, field_def in fields.items():
field_schema: dict[str, Any] = {"type": field_def.type}

if field_def.description:
field_schema["description"] = field_def.description
else:
field_schema["description"] = f"The {field_name} field"

if field_def.type == "object" and field_def.properties:
field_schema["properties"] = build_hermes_legacy_prompt_schema(
field_def.properties, depth=depth + 1, max_depth=max_depth
)
field_schema["required"] = list(field_def.properties.keys())

if field_def.type == "array" and field_def.items:
field_schema["items"] = _build_hermes_legacy_item_schema(
field_def.items, depth=depth + 1, max_depth=max_depth
)

result[field_name] = field_schema

return result


def _build_hermes_legacy_item_schema(
field: OutputField, *, depth: int, max_depth: int
) -> dict[str, Any]:
"""Build the Hermes legacy schema fragment for an array item.

Object items include ``properties`` but no ``required``. Array items of
any kind collapse to the bare ``{"type": "array"}`` shape with no
inner recursion (an explicit item description is still kept).

Args:
field: The array item output field definition.
depth: Current nesting depth.
max_depth: Maximum allowed nesting depth.

Returns:
The Hermes legacy item schema fragment.

Raises:
SchemaDepthError: When the depth limit is exceeded.
"""
item_schema: dict[str, Any] = {"type": field.type}

if field.description:
item_schema["description"] = field.description

if field.type == "object" and field.properties:
_check_depth(depth, max_depth)
# Pinned legacy counting: the array itself advanced depth by one when
# calling this helper, so the item's properties must recurse at the
# same depth (not depth + 1) to match pre-refactor Hermes behavior.
item_schema["properties"] = build_hermes_legacy_prompt_schema(
field.properties, depth=depth, max_depth=max_depth
)

return item_schema
1 change: 0 additions & 1 deletion src/conductor/providers/copilot.py
Original file line number Diff line number Diff line change
Expand Up @@ -1659,7 +1659,6 @@ def _build_prompt_schema(
schema,
depth=depth,
max_depth=self._max_schema_depth,
description_fallback=True,
)
except SchemaDepthError as exc:
raise ValidationError(
Expand Down
12 changes: 6 additions & 6 deletions src/conductor/providers/hermes.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@
from conductor.executor.output import parse_json_output, validate_output
from conductor.providers._schema import (
SchemaDepthError,
build_hermes_legacy_prompt_schema,
build_prompt_schema_properties,
)
from conductor.providers.base import AgentOutput, AgentProvider, EventCallback
from conductor.providers.capabilities import ProviderCapabilities
from conductor.providers.reasoning import ReasoningEffort, resolve_reasoning_effort

if TYPE_CHECKING:
from conductor.config.schema import AgentDef
from conductor.config.schema import AgentDef, OutputField

# The hermes-agent package ships its public API under the top-level module
# name "run_agent" (not "hermes_agent"). Catch only ModuleNotFoundError so
Expand Down Expand Up @@ -642,14 +642,14 @@ async def _wait_for_event(event: asyncio.Event) -> None:
await event.wait()


def _build_prompt_schema(schema: dict[str, Any], depth: int = 0) -> dict[str, Any]:
def _build_prompt_schema(schema: dict[str, OutputField], depth: int = 0) -> dict[str, Any]:
"""Build a prompt-facing schema description from OutputField definitions.

Wraps the shared Hermes legacy builder, converting core depth errors into
the provider's ValidationError with the exact legacy message and suggestion.
Wraps the shared recursive prompt builder, converting core depth errors into
the provider's ValidationError with the exact message and suggestion.
"""
try:
return build_hermes_legacy_prompt_schema(schema, depth=depth, max_depth=_MAX_SCHEMA_DEPTH)
return build_prompt_schema_properties(schema, depth=depth, max_depth=_MAX_SCHEMA_DEPTH)
except SchemaDepthError as exc:
raise ValidationError(
f"Schema nesting depth exceeds maximum of {_MAX_SCHEMA_DEPTH} levels",
Expand Down
46 changes: 45 additions & 1 deletion tests/test_providers/test_copilot.py
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,24 @@ def test_build_prompt_schema_recurses_through_nested_fields(self) -> None:
== "string"
)
assert schema["plan"]["required"] == ["questions", "areas", "sources"]
assert schema["summary"]["description"] == "The summary field"
assert "description" not in schema["summary"]

def test_no_fallback_description(self) -> None:
"""A field without an explicit description must not have a synthesized description key."""
provider = CopilotProvider(mock_handler=stub_handler)
agent = AgentDef(
name="summarizer",
model="gpt-4",
prompt="Summarize",
output={
"summary": {"type": "string"},
},
)

schema = provider._build_prompt_schema(agent.output or {})

assert schema["summary"]["type"] == "string"
assert "description" not in schema["summary"]

def test_build_prompt_schema_depth_limit_enforced(self) -> None:
"""Excessively nested schemas raise ValidationError with the pinned message.
Expand Down Expand Up @@ -601,6 +618,33 @@ def test_build_parse_recovery_prompt_basic(self) -> None:
assert '"value"' in prompt
assert "ONLY a valid JSON object" in prompt

def test_build_parse_recovery_prompt_has_no_fallback_descriptions(self) -> None:
"""The schema embedded in a parse recovery prompt must be built from OutputField
definitions so it contains no synthesized fallback descriptions."""
provider = CopilotProvider(mock_handler=stub_handler)
agent = AgentDef(
name="parser",
model="gpt-4",
prompt="Parse",
output={
"name": {"type": "string"},
"value": {"type": "number"},
},
)

schema = provider._build_prompt_schema(agent.output or {})
prompt = provider._build_parse_recovery_prompt(
parse_error="missing quotes",
original_response='{"name": "x}',
schema=schema,
)

assert '"name"' in prompt
assert '"value"' in prompt
assert '"The name field"' not in prompt
assert '"The value field"' not in prompt
assert "```json" in prompt

def test_build_parse_recovery_prompt_truncates_long_response(self) -> None:
"""Test that long responses are truncated in recovery prompt."""
provider = CopilotProvider(mock_handler=stub_handler)
Expand Down
Loading
Loading