Skip to content

fix(etl-uvicorn): do not require a body when every input field is optional - #73

Merged
badGarnet merged 2 commits into
mainfrom
yao/fix-optional-invoke-body
Jul 29, 2026
Merged

fix(etl-uvicorn): do not require a body when every input field is optional#73
badGarnet merged 2 commits into
mainfrom
yao/fix-optional-invoke-body

Conversation

@badGarnet

@badGarnet badGarnet commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Problem

wrap_in_fastapi chooses its /invoke signature on parameter presence:

if input_schema_model.model_fields:
    async def run_job(request: input_schema_model)   # no default => body REQUIRED
else:
    async def run_job()                              # no body accepted

A pydantic body parameter with no default is mandatory even when every field inside the model is optional. So a plugin whose parameters are all optional gets a required body that no caller has a reason to populate — and before it grew those parameters, that same plugin accepted no body at all. Adding an optional parameter therefore looks backward-compatible while silently flipping the HTTP contract.

Fix

Default the body when the generated input model has no required fields:

body_is_optional = input_schema_model.model_fields and not any(
    field.is_required() for field in input_schema_model.model_fields.values()
)
  • all fields optionalrequest: Optional[input_schema_model] = None; an absent body resolves each field to its own default, which is exactly what the function signature already promises.
  • any field required → unchanged, body mandatory. A downloader invoked without file_data still fails validation rather than receiving None.
  • no parameters → unchanged, no body accepted.

The handler body is extracted into run_job_with_body so the two model-bearing branches cannot drift.

Testing

make check-version, make check clean; 75 tests pass (5 new).

plugin shape bodyless {} populated
no params 200 200
all params optional 200 (was 422) 200 200
any param required 422 422 200

New tests are in test/api/test_api.py. Verified they catch the regression by reverting the fix: test_all_optional_params_accept_absent_or_empty_body[None] fails. The other four are guards against the fix over-reaching — notably test_required_param_still_rejects_an_absent_body, since silently accepting an absent file_data would be worse than the bug being fixed.

Also verified against the real playground indexer served through the patched generator, rather than only synthetic functions:

  • bodyless POST → 200, indexes from the settings-file fallback
  • populated wire invocation_settings → 200, and takes precedence over the file fallback

That second case is the point: this restores the bodyless contract without reverting the wire-settings capability. Both planes work.

🤖 Generated with Claude Code

Review in cubic

…ional

A pydantic body parameter with no default is mandatory even when every
field inside the model is optional. `wrap_in_fastapi` chose its `/invoke`
signature on parameter *presence*, so a plugin whose parameters are all
optional got a required body that no caller has a reason to populate --
and before it grew those parameters the same plugin accepted no body at
all. Adding an optional parameter therefore looked backward-compatible
while flipping the HTTP contract to 422 for every bodyless caller.

Observed in production: the playground indexer gained
`invocation_settings`/`invocation_context` (both defaulting to None) and
every ephemeral job began failing with

    [{"type":"missing","loc":["body"],"msg":"Field required","input":null}]

The indexer is the first node in the DAG and the source of all documents,
so nothing was indexed, every downstream node idled, and the job still
reported COMPLETED -- with total_docs 0 and an empty failed-files list.

An absent body now resolves each field to its own default, which is what
the signature already promised. Plugins with at least one required field
keep a mandatory body, so a downloader invoked without `file_data` still
fails validation rather than receiving None. The two model-bearing
branches share one handler so they cannot drift.

Verified against the real playground indexer: a bodyless POST returns 200
and indexes from the settings-file fallback, while a populated body still
takes precedence, so the wire-settings migration keeps working.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 4 files

Shadow auto-approve: would not auto-approve because issues were found.

Re-trigger cubic

Comment thread unstructured_platform_plugins/etl_uvicorn/api_generator.py Outdated
Review found that `run_job_with_body` converted `file_data` from its dict
form unconditionally, so a plugin declaring `file_data` optional raised

    AttributeError: 'NoneType' object has no attribute 'model_dump'

before `wrap_fn` could run -- surfacing as a 500 rather than the normal
response the signature promises.

This predates the optional-body change: it was already reachable on main
via `POST {}`, since an omitted field is None whether the body is absent
or merely partial. Defaulting the body widens the same hole to bodyless
requests, so fix it here rather than leaving a 500 behind the contract
this branch is establishing.

Pass None through untouched and convert only a real value. A plugin with a
required `file_data` is unaffected: validation rejects the request before
this line, so the conversion still always runs when the field is declared
mandatory.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 3 files (changes from recent commits).

Shadow auto-approve: would auto-approve. Fixes a bug where plugins with all-optional parameters incorrectly required a request body, and prevents a 500 when optional file_data is absent. The change is bounded, well-tested, and clearly beneficial.

Re-trigger cubic

@badGarnet
badGarnet merged commit 6a88783 into main Jul 29, 2026
15 checks passed
@badGarnet
badGarnet deleted the yao/fix-optional-invoke-body branch July 29, 2026 20:12
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.

2 participants