diff --git a/pyproject.toml b/pyproject.toml index 232892d..affb80b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "goodeye" -version = "0.25.3" +version = "0.25.4" description = "Goodeye CLI: a private home for the skills your AI follows and the verifiers its work must pass, from the terminal." readme = "README.md" license = { file = "LICENSE" } diff --git a/src/goodeye_cli/wire.py b/src/goodeye_cli/wire.py index 472faa5..614592b 100644 --- a/src/goodeye_cli/wire.py +++ b/src/goodeye_cli/wire.py @@ -8,9 +8,29 @@ from __future__ import annotations from datetime import datetime -from typing import Any +from typing import Annotated, Any -from pydantic import BaseModel, ConfigDict, Field +from pydantic import BaseModel, BeforeValidator, ConfigDict, Field + + +def _empty_when_null(value: Any) -> Any: + """Coerce an explicit JSON null to an empty string. + + Optional server columns arrive as null once a caller clears them, rather + than being omitted from the payload. Every field annotated with + ``NullableStr`` already defaults to "", so an absent value is expected; + without this coercion one null row fails validation and takes down the + whole response, including the rows that parsed fine. + """ + return "" if value is None else value + + +NullableStr = Annotated[str, BeforeValidator(_empty_when_null)] +"""A string field that tolerates null on the wire and reads as "" in Python. + +Use for any optional text the server may return as null. Consumers keep a +plain ``str``, so callers that format or concatenate the value stay safe. +""" class _WireBase(BaseModel): @@ -108,8 +128,8 @@ class WorkflowSummary(_WireBase): id: str name: str current_version: int - description: str = "" - outcome: str = "" + description: NullableStr = "" + outcome: NullableStr = "" tags: list[str] = Field(default_factory=list) updated_at: datetime | None = None owner_user_id: str | None = None @@ -136,8 +156,8 @@ class WorkflowSearchItem(_WireBase): match_reason: str slug: str | None = None name: str | None = None - description: str = "" - outcome: str = "" + description: NullableStr = "" + outcome: NullableStr = "" tags: list[str] = Field(default_factory=list) @@ -207,8 +227,8 @@ class WorkflowDetail(_WireBase): name: str version: int body: str - description: str = "" - outcome: str = "" + description: NullableStr = "" + outcome: NullableStr = "" tags: list[str] = Field(default_factory=list) owner_user_id: str | None = None updated_at: datetime | None = None @@ -255,7 +275,7 @@ class WorkflowFilePatchResult(_WireBase): version: int version_token: str name: str - slug: str = "" + slug: NullableStr = "" changed: list[str] = Field(default_factory=list) deleted: list[str] = Field(default_factory=list) carried_forward: int = 0 @@ -369,7 +389,7 @@ class DesignCheckCriterion(_WireBase): criterion: str passed: bool - reason: str = "" + reason: NullableStr = "" class DesignChecks(_WireBase): @@ -395,8 +415,8 @@ class TemplateSummary(_WireBase): handle: str owner_user_id: str latest_version: int - description: str = "" - outcome: str = "" + description: NullableStr = "" + outcome: NullableStr = "" tags: list[str] = Field(default_factory=list) publishing_handle: str safety_verification_status: str = "unverified" @@ -415,11 +435,11 @@ class TemplateSearchItem(_WireBase): id: str rank: int match_reason: str - slug: str = "" - name: str = "" - handle: str = "" - description: str = "" - outcome: str = "" + slug: NullableStr = "" + name: NullableStr = "" + handle: NullableStr = "" + description: NullableStr = "" + outcome: NullableStr = "" tags: list[str] = Field(default_factory=list) @@ -486,8 +506,8 @@ class TemplateDetail(_WireBase): owner_user_id: str version: int body: str - description: str = "" - outcome: str = "" + description: NullableStr = "" + outcome: NullableStr = "" tags: list[str] = Field(default_factory=list) release_notes: str | None = None publishing_handle: str @@ -797,7 +817,7 @@ class ImageGeneratorSummary(_WireBase): generator_id: str name: str - description: str = "" + description: NullableStr = "" current_version: int version_token: str status: str @@ -816,7 +836,7 @@ class ImageGeneratorDetail(_WireBase): generator_id: str name: str - description: str = "" + description: NullableStr = "" current_version: int version: int version_token: str @@ -836,7 +856,7 @@ class ImageGeneratorDeployResult(_WireBase): generator_id: str name: str - description: str = "" + description: NullableStr = "" current_version: int version: int version_token: str diff --git a/tests/test_wire_nullable_strings.py b/tests/test_wire_nullable_strings.py new file mode 100644 index 0000000..c4ba74c --- /dev/null +++ b/tests/test_wire_nullable_strings.py @@ -0,0 +1,121 @@ +"""Tests that optional wire strings tolerate an explicit JSON null. + +Optional server columns come back as null once a caller clears them. These +fields already default to "", so a null must read the same way rather than +failing validation and taking the whole response down with it. +""" + +from __future__ import annotations + +import pytest +from pydantic import ValidationError + +from goodeye_cli.wire import ( + TemplateList, + TemplateSearchResponse, + TemplateSummary, + WorkflowDetail, + WorkflowList, + WorkflowSearchResponse, + WorkflowSummary, +) + + +def _template_row(**overrides: object) -> dict[str, object]: + row: dict[str, object] = { + "id": "tmpl-1", + "slug": "writing-humanizer", + "name": "writing-humanizer", + "handle": "example", + "owner_user_id": "user-1", + "latest_version": 10, + "publishing_handle": "example", + } + row.update(overrides) + return row + + +def _workflow_row(**overrides: object) -> dict[str, object]: + row: dict[str, object] = {"id": "skill-1", "name": "example-skill", "current_version": 1} + row.update(overrides) + return row + + +@pytest.mark.parametrize("field", ["outcome", "description"]) +def test_template_summary_accepts_null(field: str) -> None: + summary = TemplateSummary.model_validate(_template_row(**{field: None})) + assert getattr(summary, field) == "" + + +@pytest.mark.parametrize("field", ["outcome", "description"]) +def test_workflow_summary_accepts_null(field: str) -> None: + summary = WorkflowSummary.model_validate(_workflow_row(**{field: None})) + assert getattr(summary, field) == "" + + +def test_null_outcome_does_not_fail_the_whole_template_list() -> None: + """One cleared row must not take down the rows that parsed fine.""" + payload = { + "items": [_template_row(id="tmpl-1"), _template_row(id="tmpl-2", outcome=None)], + "next_cursor": None, + } + listing = TemplateList.model_validate(payload) + assert [item.id for item in listing.items] == ["tmpl-1", "tmpl-2"] + assert listing.items[1].outcome == "" + + +def test_null_outcome_does_not_fail_the_whole_workflow_list() -> None: + payload = {"items": [_workflow_row(), _workflow_row(id="skill-2", outcome=None)]} + listing = WorkflowList.model_validate(payload) + assert listing.items[1].outcome == "" + + +def test_template_search_accepts_null_strings() -> None: + payload = { + "items": [ + { + "id": "tmpl-1", + "rank": 1, + "match_reason": "name match", + "slug": None, + "name": None, + "handle": None, + "description": None, + "outcome": None, + } + ], + "query": "humanize", + "limit": 10, + } + item = TemplateSearchResponse.model_validate(payload).items[0] + assert (item.slug, item.name, item.handle, item.description, item.outcome) == ("",) * 5 + + +def test_workflow_search_accepts_null_strings() -> None: + payload = { + "items": [{"id": "skill-1", "rank": 1, "match_reason": "m", "description": None}], + "query": "humanize", + "limit": 10, + } + assert WorkflowSearchResponse.model_validate(payload).items[0].description == "" + + +def test_workflow_detail_accepts_null_outcome() -> None: + detail = WorkflowDetail.model_validate( + {"id": "skill-1", "name": "n", "version": 1, "body": "b", "outcome": None} + ) + assert detail.outcome == "" + + +def test_absent_and_present_values_are_unchanged() -> None: + """The coercion must only touch null, leaving normal payloads alone.""" + absent = TemplateSummary.model_validate(_template_row()) + assert absent.outcome == "" + present = TemplateSummary.model_validate(_template_row(outcome="Ship faster.")) + assert present.outcome == "Ship faster." + + +def test_wrong_types_are_still_rejected() -> None: + """Tolerating null must not turn the field into an anything-goes field.""" + with pytest.raises(ValidationError): + TemplateSummary.model_validate(_template_row(outcome=123)) diff --git a/uv.lock b/uv.lock index 8c0e996..9ea194a 100644 --- a/uv.lock +++ b/uv.lock @@ -176,7 +176,7 @@ wheels = [ [[package]] name = "goodeye" -version = "0.25.3" +version = "0.25.4" source = { editable = "." } dependencies = [ { name = "httpx" },