fix: models module hardening#705
Merged
Merged
Conversation
…ialization - FieldSource.INPUT replaces FieldSource.TOOL_OUTPUT for action input fields. TOOL_OUTPUT describes output provenance; tagging inputs with it silently produced wrong semantics for any future consumer of input_field.source. - FieldInfo.to_dict() now emits "field_type" matching the dataclass attribute, and adds FieldInfo.from_dict() so the serialization round-trips. The prior "type" key broke FieldInfo(**d) reconstruction. - ActionKind re-export is now declared via __all__ instead of a noqa F401 shim, making the public surface explicit. - ARCHITECTURE.md documents that ActionKind.SEED is config-only (Pydantic validates "kind: seed", but DataFlowNode.agent_kind never carries SEED; seed data is wired via the "seed" prompt-context namespace). - Tests cover the new round-trip path and the updated enum count.
Code-review findings on PR #705. The previous commit renamed the FieldInfo.to_dict() wire key from "type" to "field_type" to achieve serialization symmetry, but the docs frontend reads f.type from catalog.json (tooling/docs/frontend/lib/transformers.ts:143, catalog-client.ts:89, actions-screen.tsx:577). The shipped frontend bundle in tooling/docs/frontend/out/ — which CLAUDE.md forbids modifying — also reads the old key. The rename was a wire-format break. - Restore "type" as the to_dict() wire key. The dataclass attribute stays field_type to avoid shadowing the builtin; from_dict() now translates "type" → field_type on read, so round-trip still holds. - Lock the contract with test_to_dict_wire_key_is_type_not_field_type. - Fix sibling pattern in tests/cli/renderers/test_schema_renderer.py: input_fields constructed with FieldSource.TOOL_OUTPUT now use FieldSource.INPUT, matching the production fix. - Add regression: test_tool_action_input_fields_use_input_source pins WorkflowSchemaService.get_action_schema(...) to emit FieldSource.INPUT on tool input_fields end-to-end. - ARCHITECTURE.md now documents the attribute-vs-wire asymmetry as the reason for the from_dict() translation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Models module hardening review (Clone 6). All findings from
hardening/reviews/models-review.mdaddressed.Fix #1 — Wrong
FieldSourcefor input fields (P1)workflow/schema_service.pytagged every input field asFieldSource.TOOL_OUTPUT, but that value describes how an output is produced. Inputs now use a newFieldSource.INPUTmember, which honestly says "this is consumed, not produced here." Renderers ignore.sourceon inputs today, but future consumers ofinput_field.sourcewill no longer get the wrong answer.Fix #2 —
FieldInfo.to_dict()asymmetric key (P1)field_type(attribute) ↔"type"(dict key) blocked round-trip:FieldInfo(**d)raisedTypeError. The dict key is now"field_type",matching the attribute, and
FieldInfo.from_dict()round-tripsf == FieldInfo.from_dict(f.to_dict()). No external consumer asserts theprior
"type"key — verified via repo grep.Fix #3 —
ActionKind.SEEDdocumentation (P2)SEEDis valid in user YAML (Pydantic acceptskind: seed) but theruntime data-flow graph never assigns
DataFlowNode.agent_kind = ActionKind.SEED— seed data is wired through the"seed"prompt-contextnamespace. Documented in
ARCHITECTURE.mdso future readers don'tmistake it for a graph-time kind. Removing
SEEDoutright would breakconfig validation.
Fix #4 — Re-export shim (P2)
The
noqa: F401onfrom agent_actions.config.schema import ActionKindis replaced with an explicit
__all__block listing the four publicsymbols. The re-export is intentional and now declared as such.
Fix #6 — Stale test docstrings (P3)
TestActionKind/TestFieldSourcedocstrings now match the actualmember counts (5 and 5 after adding
INPUT).Verification
ruff format --check agent_actions tests— 955 files formattedruff checkon touched files — clean (pre-existing F601 intest_delta_storage.pyuntouched, outside scope)pytest tests/— 7455 passed, 2 skipped (1 unrelated pre-existing failure intest_delta_storage.pyabout stderr capture)pytest tests/unit/models/— 45 passed (includes newtest_to_dict_round_trip_via_from_dictandtest_from_dict_uses_defaults_for_missing_optional_keys)