Skip to content
Open
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
2 changes: 1 addition & 1 deletion .agents/skills/jsonschema-to-pydantic-lungo/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ If a consumer fails because of a rename you introduced (e.g. it imported the old
- ❌ Using `@model_validator(mode="after")` to police `__pydantic_extra__` for fields that should belong to a sibling union variant.
- ❌ Putting field-count or per-field validity checks inside a `Discriminator` callable. The discriminator answers exactly one question: which schema `anyOf` branch.
- ❌ Adding `@field_validator` for a constraint that fits in `Annotated[..., Field(...)]`.
- ❌ Inventing class names (`AgentId` for `stable_agent_id`, `Node` for `regular_node`). Names come straight from `$defs` keys.
- ❌ Inventing class names (`AgentId` for `stable_agent_id`, `Node` for `base_node`). Names come straight from `$defs` keys.
- ❌ Hand-editing a small portion of the generated file in response to a test failure or a request for a tweak. Re-emit the whole file end-to-end via the skill instead — partial patches drift from the rules over time.

## Reference
Expand Down
50 changes: 25 additions & 25 deletions .agents/skills/jsonschema-to-pydantic-lungo/mapping-rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ Do not emit a separate `@field_validator` that loops the keys checking the patte
Schema:

```json
"partial_regular_node": {
"partial_base_node": {
"type": "object",
"required": ["id", "operation"],
"properties": {
Expand All @@ -70,9 +70,9 @@ Schema:
},
"additionalProperties": true
},
"regular_node": {
"base_node": {
"allOf": [
{ "$ref": "#/$defs/partial_regular_node" },
{ "$ref": "#/$defs/partial_base_node" },
{
"type": "object",
"required": ["id", "operation", "type", "label", "size", "layer_index"]
Expand All @@ -84,7 +84,7 @@ Schema:
Emit two unrelated classes (no inheritance):

```python
class PartialRegularNode(BaseModel):
class PartialBaseNode(BaseModel):
model_config = ConfigDict(extra="allow")

id: NodeId
Expand All @@ -95,7 +95,7 @@ class PartialRegularNode(BaseModel):
layer_index: float = 0


class RegularNode(BaseModel): # NOT (PartialRegularNode)
class BaseNode(BaseModel): # NOT (PartialBaseNode)
model_config = ConfigDict(extra="allow")

id: NodeId
Expand All @@ -106,7 +106,7 @@ class RegularNode(BaseModel): # NOT (PartialRegularNode)
layer_index: float
```

Why no subclassing: `type: ... | None = None` from `PartialRegularNode` would leak into `RegularNode` and `RegularNode(id=..., operation=...)` would silently validate.
Why no subclassing: `type: ... | None = None` from `PartialBaseNode` would leak into `BaseNode` and `BaseNode(id=..., operation=...)` would silently validate.

## §D — `anyOf` discriminated by sibling-key presence

Expand All @@ -129,21 +129,21 @@ Schema (the canonical lungo case for `partial_node` / `node`). The agent-flavour
},
"partial_agent_node": {
"allOf": [
{ "$ref": "#/$defs/partial_regular_node" },
{ "$ref": "#/$defs/partial_base_node" },
{ "$ref": "#/$defs/partial_node_agent_extension" }
]
},
"agent_node": {
"allOf": [
{ "$ref": "#/$defs/regular_node" },
{ "$ref": "#/$defs/base_node" },
{ "$ref": "#/$defs/node_agent_extension" }
]
},
"partial_node": {
"anyOf": [
{
"allOf": [
{ "$ref": "#/$defs/partial_regular_node" },
{ "$ref": "#/$defs/partial_base_node" },
{ "not": { "anyOf": [
{ "required": ["agent_record_uri"] },
{ "required": ["stable_agent_id"] }
Expand All @@ -153,68 +153,68 @@ Schema (the canonical lungo case for `partial_node` / `node`). The agent-flavour
{ "$ref": "#/$defs/partial_agent_node" }
]
},
"node": { /* same shape: regular branch + $ref agent_node */ },
"node": { /* same shape: base branch + $ref agent_node */ },
"topology_node_item": {
"anyOf": [{ "$ref": "#/$defs/partial_node" }, { "$ref": "#/$defs/node" }]
}
```

Step 1 — emit the four leaf classes. Names come straight from the `$defs` keys (no merged-class invention — see [`SKILL.md`](SKILL.md)). The agent variants subclass their regular counterparts because the extension is purely *additive* fields (no required-vs-optional drift, unlike §C):
Step 1 — emit the four leaf classes. Names come straight from the `$defs` keys (no merged-class invention — see [`SKILL.md`](SKILL.md)). The agent variants subclass their base counterparts because the extension is purely *additive* fields (no required-vs-optional drift, unlike §C):

```python
class PartialAgentNode(PartialRegularNode):
class PartialAgentNode(PartialBaseNode):
agent_record_uri: Annotated[str, Field(min_length=1)]
stable_agent_id: StableAgentId | None = None


class AgentNode(RegularNode):
class AgentNode(BaseNode):
agent_record_uri: Annotated[str, Field(min_length=1)]
stable_agent_id: StableAgentId
```

`partial_node_agent_extension` and `node_agent_extension` are **not** emitted as standalone classes: they are only ever referenced inside the `allOf`s that build `partial_agent_node` / `agent_node`, so their fields appear directly on the agent classes.

Step 2 — discriminator that mirrors **only** the sibling-key presence test that the regular branch's `not { anyOf: [...required...] }` clause is making:
Step 2 — discriminator that mirrors **only** the sibling-key presence test that the base branch's `not { anyOf: [...required...] }` clause is making:

```python
_REGULAR_TAG = "regular"
_BASE_TAG = "base"
_AGENT_TAG = "agent"


def _node_kind_discriminator(value: Any) -> str | None:
if isinstance(value, (AgentNode, PartialAgentNode)):
return _AGENT_TAG
if isinstance(value, (RegularNode, PartialRegularNode)):
return _REGULAR_TAG
if isinstance(value, (BaseNode, PartialBaseNode)):
return _BASE_TAG
if not isinstance(value, dict):
return None
if "agent_record_uri" in value or "stable_agent_id" in value:
return _AGENT_TAG
return _REGULAR_TAG
return _BASE_TAG
```

Step 3 — emit each `anyOf`-shaped `$def` as an `Annotated[Union[...], Discriminator(...)]`. Inside each tagged branch use a plain `Union[Full, Partial]` and let smart-union pick the right one based on field population:

```python
TopologyNodeItem = Annotated[
Union[
Annotated[Union[RegularNode, PartialRegularNode], Tag(_REGULAR_TAG)],
Annotated[Union[BaseNode, PartialBaseNode], Tag(_BASE_TAG)],
Annotated[Union[AgentNode, PartialAgentNode], Tag(_AGENT_TAG)],
],
Discriminator(_node_kind_discriminator),
]

PartialNode = Annotated[
Union[
Annotated[PartialRegularNode, Tag(_REGULAR_TAG)],
Annotated[PartialBaseNode, Tag(_BASE_TAG)],
Annotated[PartialAgentNode, Tag(_AGENT_TAG)],
],
Discriminator(_node_kind_discriminator),
]

Node = Annotated[
Union[
Annotated[RegularNode, Tag(_REGULAR_TAG)],
Annotated[BaseNode, Tag(_BASE_TAG)],
Annotated[AgentNode, Tag(_AGENT_TAG)],
],
Discriminator(_node_kind_discriminator),
Expand All @@ -225,8 +225,8 @@ Node = Annotated[

| Input | Routes to | Outcome |
|---|---|---|
| All regular fields, no extension keys | `RegularNode` | OK |
| Only `id`+`operation` | `PartialRegularNode` | OK |
| All base fields, no extension keys | `BaseNode` | OK |
| Only `id`+`operation` | `PartialBaseNode` | OK |
| All fields + `agent_record_uri` + valid `stable_agent_id` | `AgentNode` | OK |
| `agent_record_uri` only (partial) | `PartialAgentNode` | OK |
| `stable_agent_id` only, no `agent_record_uri` | agent branch → `PartialAgentNode` | rejected: `agent_record_uri` is required |
Expand All @@ -236,10 +236,10 @@ The error messages come straight from each branch's own `Field` constraints —

### Anti-pattern this replaces

The earlier (incorrect) approach was an `@model_validator(mode="after")` on `PartialRegularNode` / `RegularNode` that walked `__pydantic_extra__` and raised if `agent_record_uri` or `stable_agent_id` had leaked through `extra="allow"`. That approach:
The earlier (incorrect) approach was an `@model_validator(mode="after")` on `PartialBaseNode` / `BaseNode` that walked `__pydantic_extra__` and raised if `agent_record_uri` or `stable_agent_id` had leaked through `extra="allow"`. That approach:

- Required a parallel `_AGENT_EXTENSION_FIELDS` allow-list.
- Hid the schema's `pattern` / `min_length` errors behind a generic message.
- Coupled `RegularNode` to knowledge of its sibling extension class.
- Coupled `BaseNode` to knowledge of its sibling extension class.

The discriminator solves all three problems at once: routing alone is sufficient because each variant's own field constraints handle the rest.
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
Operation,
PartialEdge,
PartialNode,
PartialRegularNode,
PartialBaseNode,
PartialTopology,
Size,
Topology,
Expand Down Expand Up @@ -67,7 +67,7 @@ def make_node(
node_extras["agent_record_uri"] = f"agent-card://{stable_agent_id.removeprefix('agent://')}"
if extras:
node_extras.update(extras)
return PartialRegularNode(
return PartialBaseNode(
id=NodeId(node_id),
operation=operation,
type=node_type,
Expand Down
26 changes: 13 additions & 13 deletions coffeeAGNTCY/coffee_agents/lungo/schema/jsonschemas/event_v1.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@
},
"additionalProperties": false
},
"partial_regular_node": {
"partial_base_node": {
"type": "object",
"description": "Sparse regular node data (mainly for updates). Used in topology definitions.",
"description": "Sparse base node data (mainly for updates). Used in topology definitions.",
"required": ["id", "operation"],
"properties": {
"id": { "$ref": "#/$defs/node_id" },
Expand All @@ -69,12 +69,12 @@
},
"additionalProperties": true
},
"regular_node": {
"base_node": {
"allOf": [
{ "$ref": "#/$defs/partial_regular_node" },
{ "$ref": "#/$defs/partial_base_node" },
{
"type": "object",
"description": "Full regular node data (mainly for init/reset). Used in topology definitions.",
"description": "Full base node data (mainly for init/reset). Used in topology definitions.",
"required": ["id", "operation", "type", "label", "size", "layer_index"]
}
]
Expand All @@ -99,25 +99,25 @@
]
},
"partial_agent_node": {
"description": "Sparse agent node data (mainly for updates): a partial_regular_node combined with a partial_node_agent_extension.",
"description": "Sparse agent node data (mainly for updates): a partial_base_node combined with a partial_node_agent_extension.",
"allOf": [
{ "$ref": "#/$defs/partial_regular_node" },
{ "$ref": "#/$defs/partial_base_node" },
{ "$ref": "#/$defs/partial_node_agent_extension" }
]
},
"agent_node": {
"description": "Full agent node data (mainly for init/reset): a regular_node combined with a node_agent_extension.",
"description": "Full agent node data (mainly for init/reset): a base_node combined with a node_agent_extension.",
"allOf": [
{ "$ref": "#/$defs/regular_node" },
{ "$ref": "#/$defs/base_node" },
{ "$ref": "#/$defs/node_agent_extension" }
]
},
"partial_node": {
"description": "Sparse node data: either a partial_regular_node alone (without any agent extension fields), or a partial_agent_node.",
"description": "Sparse node data: either a partial_base_node alone (without any agent extension fields), or a partial_agent_node.",
"anyOf": [
{
"allOf": [
{ "$ref": "#/$defs/partial_regular_node" },
{ "$ref": "#/$defs/partial_base_node" },
{
"not": {
"anyOf": [
Expand All @@ -132,11 +132,11 @@
]
},
"node": {
"description": "Full node data: either a regular_node alone (without any agent extension fields), or an agent_node.",
"description": "Full node data: either a base_node alone (without any agent extension fields), or an agent_node.",
"anyOf": [
{
"allOf": [
{ "$ref": "#/$defs/regular_node" },
{ "$ref": "#/$defs/base_node" },
{
"not": {
"anyOf": [
Expand Down
8 changes: 4 additions & 4 deletions coffeeAGNTCY/coffee_agents/lungo/schema/types/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

from schema.types.event import (
AgentNode,
BaseNode,
Correlation,
CorrelationId,
Data,
Expand All @@ -28,11 +29,10 @@
NodeId,
Operation,
PartialAgentNode,
PartialBaseNode,
PartialEdge,
PartialNode,
PartialRegularNode,
PartialTopology,
RegularNode,
Size,
StableAgentId,
Topology,
Expand All @@ -51,6 +51,7 @@

__all__ = [
"AgentNode",
"BaseNode",
"Correlation",
"CorrelationId",
"Data",
Expand All @@ -65,11 +66,10 @@
"NodeId",
"Operation",
"PartialAgentNode",
"PartialBaseNode",
"PartialEdge",
"PartialNode",
"PartialRegularNode",
"PartialTopology",
"RegularNode",
"Size",
"StableAgentId",
"Topology",
Expand Down
Loading