Repository navigation
Generate prepared conversion plans and correct plural catch types - #40
Conversation
|
@codex review Please review the current head against the base, focusing on conversion-plan generation, nullability and list depth, unions, recursive inputs, callback IDs, and compatibility with the runtime in zth/rescript-relay#674. Please report actionable issues with severity and a reproducing scenario. |
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: bd305ebbc2
ℹ️ 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".
|
@codex review the full current PR at 6247069, including the plural @catch fix. Please re-check correctness and any remaining major issues. Prior finding was investigated and answered inline; mounted reproduction and regression tests are in zth/rescript-relay#674 at 64c0934. Ordinary plural roots intentionally use the runtime implicit plural entry point, so record/union root plans need no redundant root list opcode. Please inspect the corresponding runtime when judging that contract. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6247069896
ℹ️ 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".
|
@codex review the full current PR at 19bfb3d. The nullable/abstract plural @catch finding is fixed and answered inline. Compiler regression matrix covers concrete/abstract, nullable/non-null, and both result/array wrapper orders. Mounted integration tests and generated ReScript consumers are in zth/rescript-relay#674 at 65d953f. Please check for any remaining major issues, consulting the corresponding runtime contract as needed. |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
zth
changed the title
1 participant
Generate version 2 ReScript conversion plans for the prepared runtime in zth/rescript-relay#674. Exact path segments and explicit list depth preserve nested lists and array-backed scalars without ambiguous underscore paths.
@catchtypes and conversions for concrete, nullable, union and interface payloads.compiler/test-project-resandcompiler/test-project-res-preloadable(113 files).Merge/release: merge this compiler PR first, then the matching runtime PR. Keep the pinned commit in history, or update the parent submodule after a squash merge. Ship compiler and runtime together; new generated artifacts require the new runtime.
Validation: 35 compiler library tests; both ReScript fixture projects pass
--validateand deterministic regeneration. The parent suite passes 263 JS tests, including mounted Relay and persisted queries, plus 20 targeted mutation checks and bundle-size budgets. Codex's findings were fixed and its final implementation review found no major issues.Known CI limitations: this ReScript fork's upstream Rust fixture/Flow tests and JS compiler-output checks still fail; the unchanged baseline reproduces the documented Flow snapshot failures. Rust lint fails because CI installs rustfmt for a different toolchain from
rust-toolchain.toml. These checks are not claimed green. Rust compiler build jobs pass; focused ReScript validation is listed above. See the parent conversion-contract document for the baseline comparison and test scope.Across the parent's fixed 129 artifacts, generated output is 1,254 bytes smaller gzip combined and 712 bytes smaller across separate bundles than master. Runtime size and timing are reported separately in the parent PR.