Repository navigation
Use Pydantic's alias_generator instead of a custom CamelCaseField - #348
Conversation
All models now accept both camelCase and snake_case field names when loading from data. They always dump camelCase field names. File field_helpers.py is no longer needed, and is removed. This patch was generated by Claude; I skimmed it.
Bernhard Merkle (bmerkle)
left a comment
Bernhard Merkle (bmerkle)
left a comment
There was a problem hiding this comment.
Nice cleanup: swapping the descriptor hack for AliasGenerator is much more robust. Full suite passes locally (811 passed, pyright clean), and the generated TypeChat schemas are identical to main except for the four intended Action description comments.
A few notes, mostly about docs:
| def dump(obj: object) -> dict: | ||
| return obj.__pydantic_serializer__.to_python(obj, by_alias=True) # type: ignore[attr-defined] | ||
|
|
||
|
|
||
| def load[T](cls: type[T], data: dict) -> T: | ||
| return cls.__pydantic_validator__.validate_python(data) # type: ignore[attr-defined] |
There was a problem hiding this comment.
route the message tests through serialize_object/deserialize_object instead of pydantic_serializer/pydantic_validator, so they follow the path persisted data takes
| Fields are declared in snake_case. On input, both the snake_case name and | ||
| its camelCase form are accepted; on output (``by_alias=True``), camelCase is | ||
| written. An explicit alias on a field takes precedence. | ||
| """ |
There was a problem hiding this comment.
The module docstring is wrong for fields with explicit aliases .
"Both the snake_case name and its camelCase form are accepted" does not hold once a field sets its own alias:
- With Field(alias="X"), only X is accepted.
- With only serialization_alias= set, camelCase input is silently ignored, so the field doesn't round-trip.
| decorator now gives every field a camelCase alias. As a result, the | ||
| `source_id` field of messages and the `src_url` field of `EmailMessage` are | ||
| now saved as `sourceId` and `srcUrl`; data saved under the old names still | ||
| loads. |
There was a problem hiding this comment.
The CHANGELOG lists far fewer renamed fields than actually change.
It names only source_id and src_url. Comparing every pydantic dataclass on main against the PR, about 40 more multi-word fields now dump as camelCase when serialized with by_alias=True. Among them:
- KnowledgeResponse.inverse_actions, AnswerResponse.why_no_answer, DateTimeRange.start_date/stop_date
- SearchFilter., ActionTerm., WhenFilter., SearchOptions.
- AddMessagesResult., ChunkFailure., ConversationMetadata.*, ScoredMessageOrdinal.message_ordinal
Within the package only messages are persisted, so "saved as" is correct only for those two fields. Any other caller that serializes these classes will see the new keys. A one-line catch-all would cover it.
Bernhard Merkle (bmerkle)
left a comment
Bernhard Merkle (bmerkle)
left a comment
There was a problem hiding this comment.
Thanks, all my earlier points are addressed. The new tests pin down the explicit-alias and serialization-alias-only behaviour nicely. CI is green, and I also ran the alias/serialization tests against pydantic 2.12 on py3.12 (all pass).
On the 2.11 question: the 2.10 floor is already unreachable in practice, because pydantic-ai-slim>=2.25.0 requires pydantic>=2.12 (uv refuses to resolve pydantic==2.10.* against the package). So I'm fine with raising the declared floor.
LGTM
## Summary
- `CompiledSearchTerm` was a stdlib `@dataclass` subclassing the
pydantic dataclass `SearchTerm`. A stdlib dataclass takes the base
class's class-level value as the field default, and for `related_terms`
that value is a pydantic `FieldInfo` object, not `None`.
- `CompiledSearchTerm(Term("x")).related_terms` was therefore a truthy
`FieldInfo`, and `dedupe_related_terms` crashed on it with
`AttributeError: 'FieldInfo' object has no attribute 'text'`.
- Production code only creates `CompiledSearchTerm` via `cast`, so it
isn't hit there today; tests work around it by always passing
`related_terms=None`.
- Declare `CompiledSearchTerm` with typeagent's pydantic `dataclass`
decorator, like `SearchTerm` (same import as `universal_message.py`).
- A scan of all 103 dataclasses in the package found no other
non-pydantic dataclass inheriting a `FieldInfo` default.
Relation to #348: #348 changes this default to `Field(default=None,
...)`, which is still a `FieldInfo`, so the bug remains there. This PR
doesn't touch any file #348 changes; it applies cleanly on top of #348
and the affected tests pass there too.
## Validation
- New tests: a default-constructed `CompiledSearchTerm` has
`related_terms is None`, and `dedupe_related_terms` accepts such terms.
Both fail without the fix.
- make format check test
Co-authored-by: wtfyd-agent[bot] <328989958+wtfyd-agent[bot]@users.noreply.github.com>
3 participants
Written by Claude and ChatGPT.
Includes tests.
This also fixes and tests some pre-existing issues where certain descriptions didn't make it to the TypeScript schema generated by TypeChat. TypeChat doesn't render Field descriptions (it knows nothing about pydantic.Field), but it does know about Annotated[T, Doc("...")], and renders that doc string as a TypeScript comment.
Things to know about TypeChat: