Repository navigation
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db805ae383
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if len(non_null_branches) == 1: | ||
| return self._encode(obj, non_null_branches[0]) |
There was a problem hiding this comment.
When a schema-discovered client sends a plain JSON list to a nullable torch.Tensor or tf.Tensor root endpoint, this recursion now invokes TensorSchema.validate(), which accesses the framework's Tensor class and triggers its lazy import. Neither framework is a required BentoML dependency, so clients without the server's ML framework now raise MissingDependencyException before sending the request. Previously, the list was serialized unchanged and converted by the server. Preserve already-JSON-compatible inputs, or otherwise avoid requiring the server's framework merely to send a JSON list.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8dee33be. JSON lists now pass through the nullable tensor branch without loading the server framework; native arrays still use the existing encoder. Two tests drive the real schema-discovered client through GET/POST without Torch or TensorFlow installed: both fail on db805ae, pass on the original base, and pass with this fix. The complete related file now passes all 14 tests; Ruff/format pass and scoped Pyright diagnostics remain identical to base.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93494c4ed5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| else: | ||
| # Leave JSON input validation to the server's ML framework. | ||
| return obj | ||
| return self._encode(obj, branch) |
There was a problem hiding this comment.
When a schema-discovered client without pandas sends a JSON list/dict to an Optional[pd.DataFrame] root endpoint, this recursion now invokes DataframeSchema.validate(), which accesses the lazy pd.DataFrame and raises MissingDependencyException before the request. Pandas is optional (io-pandas in pyproject.toml), and these requests previously serialized unchanged for server-side conversion. The JSON passthrough above only protects tensors; preserve JSON-compatible dataframe inputs too, or restrict the new unwrapping to tensor conversion.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8754d1647f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| else: | ||
| # Leave JSON input validation to the server's optional dependencies. | ||
| return obj | ||
| return self._encode(obj, branch) |
There was a problem hiding this comment.
For a schema-discovered root endpoint typed Optional[list[torch.Tensor]], a client without Torch can no longer send JSON-native input such as [[1.0, 2.0]]. The non-null branch has type: "array", so the JSON passthrough guard is skipped and this recursion reaches TensorSchema.validate() for each item, triggering the framework import and raising MissingDependencyException before the POST. The same regression affects nullable lists of TensorFlow tensors or dataframes. The parent revision serialized these JSON inputs unchanged for server-side validation. Preserve JSON-compatible nullable containers before recursing too, or restrict the new unwrapping to the intended leaf schemas.
Useful? React with 👍 / 👎.
|
Addressed the JSON-only tensor, dataframe and nullable-container regressions in Both related files passed (100 cases), followed by 76 affected cases after the test-only cleanup. Ruff/format pass. Scoped Pyright retains the previous 18 errors and adds one pandas-stubs warning; it is not green. Native Torch/TF server conversion and the full matrix were not run. The description now reflects these results. |
What does this PR address?
Schema-discovered HTTP clients cannot send a NumPy array to a documented nullable positional-only input (
Optional[np.ndarray] = None). The root schema retainsanyOf(tensor, null), whichGenericSerdeignores, so JSON encoding fails before the request.Encode native values using the sole non-null schema branch. Preserve JSON-compatible values before that recursion, including tensor/dataframe inputs and nullable containers, so sending JSON does not require the server's optional ML dependencies.
Noneand general unions retain their existing behavior.The original ndarray regression fails on base; its controls pass. Additional tests exercise the real schema-discovered client and JSON requests with missing Torch/TF/pandas, as well as native NumPy/pandas conversion and nullable collections. Missing-framework fixtures are simulated in the new matrix; two existing client cases also run with Torch/TF genuinely absent.
Validation details
Python 3.12.8; runtime/test versions constrained to
pdm.lock, including NumPy 2.0.2, pandas 2.2.3, Pydantic 2.10.3, HTTPX 0.28.1 and Starlette 0.41.3.371fb080. After changing test assertions to use public serialization APIs, all 76 affected cases passed again.8dee33be; the additional warning is missing pandas stubs. All three added private-access errors have been removed. Type checking is not green.Full pre-commit/type/test matrices and native Torch/TF server conversion were not run.
Before submitting:
pre-commit run -a; changed files pass Ruff.