bugfix: Ensure schema is loaded before processing#221
Conversation
Schema wasn't loaded prior to processing the input paths this resulted in a noop nil response. To ensure we always load the schema first we now block on runner ready state before processing. Additionaly a nil doc is considered an error. In the cases we call ProcessInputPaths we can accept that error; in manager we explicitly check for doc being nil and emit an error log.
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a critical bug where the OpenAPI schema wasn't loaded before processing input paths, causing URL and base64 inputs to be passed through unchanged instead of being converted to local paths. The solution ensures the runner's setup is complete (including schema loading) before prediction processing begins, and adds proper error handling for nil schema cases.
- Adds a wait mechanism to ensure runner setup completes before predictions are processed
- Introduces explicit error handling when OpenAPI schema is unavailable
- Updates ProcessInputPaths to return an error when schema is nil instead of silently skipping processing
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/runner/manager.go | Adds blocking wait for runner setup completion before prediction processing |
| internal/runner/runner.go | Adds nil schema check and error logging for input path processing |
| internal/runner/path.go | Changes ProcessInputPaths to return error when schema unavailable |
| internal/runner/types.go | Adds context field to PendingPrediction struct |
| internal/runner/path_test.go | Updates test to expect error when schema is nil |
| python/tests/runners/path_input_https.py | Test predictor for HTTPS path input regression testing |
| python/tests/procedures/path_test/* | Test procedure files for path input testing |
| internal/tests/*_test.go | Comprehensive regression tests for path input processing |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Async predictions should not 500. However, since we now known that the runner failed before we send the prediction request down, we end up needing to work around the error and still send a 202 *then* let the webhook indicate failure. While this behavior is somewhat crazy, it is the behavioral contract we can revisit in the future.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 6 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
tempusfrangit
force-pushed
the
runtime-fixes
branch
from
14e8e12 to
46d34bb
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Schema wasn't loaded prior to processing the input paths this resulted in a noop nil response. To ensure we always load the schema first we now block on runner ready state before processing. Additionaly a nil doc is considered an error. In the cases we call ProcessInputPaths we can accept that error; in manager we explicitly check for doc being nil and emit an error log.