Skip to content

refactor: extract duplicated request validators into core utilities#53

Open
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1784936555-shared-validators
Open

refactor: extract duplicated request validators into core utilities#53
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1784936555-shared-validators

Conversation

@devin-ai-integration

Copy link
Copy Markdown

Summary

Two Pydantic field validators were copy-pasted across request/model modules. This folds them into core/:

  • Non-negative limit — the same validator appeared 6 times (services/search, services/index, services/runs, and three in services/viewer). Consolidated into core.models:

    def _require_non_negative_limit(value): ...  # "limit must be non-negative"
    NonNegativeLimit         = Annotated[int | None, AfterValidator(...)]  # optional, default None
    RequiredNonNegativeLimit = Annotated[int,        AfterValidator(...)]  # required, e.g. default 50

    Fields become limit: NonNegativeLimit = None / limit: RequiredNonNegativeLimit = 50, dropping the per-model @field_validator/@classmethod boilerplate.

  • Optional-text — the if value is None: return None; return required_text(...) pattern appeared 5 times (services/runs/models, workflows/build ×2, workflows/garden, workflows/ingest). Consolidated into core.text.optional_text(value, field_name); each validator now delegates in one line, preserving its per-model error label.

Field-specific validators with distinct messages (page_limit, transcript/filesystem size, harness count/duration) were intentionally left alone — merging them would change error strings and over-abstract.

Why

Reduce copy-paste and give these validators one home so behavior/wording stays consistent as new request types are added. Net −34 lines across 10 files.

Note: the per-source errors.py duplication (unavailable_runtime/first_error_line) was deliberately not touched — tests/test_architecture.py enforces that split-by-responsibility layout.

Verification

uv run pytest        # 564 passed
uv run ruff check .  # All checks passed!

Also confirmed error messages/accept-reject behavior are byte-for-byte identical (negative limit → "limit must be non-negative"; required-int limit=None still rejected; empty title → "build request text must not be empty"; None accepted).

Docs and wiki

  • Not applicable.

Notes for reviewers

NonNegativeLimit keeps the validator inside Annotated[int | None, ...] (rather than Annotated[int, ...] | None) so a single clean ValidationError is produced — matching the previous @field_validator output that surfaces via cli/execution.py.

Link to Devin session: https://app.devin.ai/sessions/2ec4dbc36bd94d9da6e2b3e9ad796454
Requested by: @rohans0509

Consolidate the byte-identical non-negative-limit validator (6 copies) into
core.models.NonNegativeLimit / RequiredNonNegativeLimit, and the optional-text
validator (5 copies) into core.text.optional_text. Validation behavior and
error messages are unchanged.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@rohans0509 rohans0509 self-assigned this Jul 24, 2026
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant