Repository navigation
fix(api): preserve multi-graders alongside named grader maps - #729
markstuart-oai wants to merge 2 commits into
Conversation
Castiron custom codeMixed files: 87 → 90 3 newly customized · 0 customizations removed · 0 existing customizations changed · 0 generated baselines changed Compared
87 existing customizations unchanged
47 more in the full report. A changed generated baseline means this report cannot reliably identify which handwritten lines changed. Inspect the custom-code diffDownload the exact patch produced by this run (requires repository access): gh run download 37075524545 --repo openai/openai-ruby \
--name castiron-custom-code-37075524545-1 --dir /tmp/castiron-custom-code-37075524545-1
git apply --stat /tmp/castiron-custom-code-37075524545-1/custom-code.patch
cat /tmp/castiron-custom-code-37075524545-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin baaec487ca85117d6ee149cabdfe0cb8837d883c 9c3789498b9f2d4843c0949fcf31fa34c6b2e426
python3 scripts/castiron/custom_code_report.py report \
--base baaec487ca85117d6ee149cabdfe0cb8837d883c \
--head 9c3789498b9f2d4843c0949fcf31fa34c6b2e426 --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-9c3789498b9f
cat /tmp/castiron-custom-code-9c3789498b9f/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
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. |
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
not as a breaking change, nah
|
In response to the nonbreaking-release review: Agreed — this should not ship as a breaking release. The current diff really does remove I’m keeping this PR in draft with your change request outstanding. It needs an additive compatibility revision that retains the existing public names and legacy input behavior while supporting correctly named grader maps, plus regression coverage for both paths. The old flat payload must not be silently given an invented grader name or represented as an API-valid request. No compatibility fix or release is claimed yet. |
34732db to
203e0c1
Compare
markstuart-oai
changed the title
![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: 203e0c1a59
ℹ️ 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".
203e0c1 to
e04bcf4
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e04bcf4e10
ℹ️ 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".
e04bcf4 to
d1b2d68
Compare
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Re-reviewed d1b2d686d2f3ee1c9c2f1791e9fec91c1837edcd. The restored public union and the Sorbet/RBS input changes look good, but one reproducible legacy-input regression remains, so I am keeping changes requested.
Reviewed all seven changed files and the conversion machinery. A focused offline Ruby 3.3.12 probe accepts the omitted-type single-grader hash on exact base e1903035d709a6b8aa40c0049ea8305c63eccc61 and rejects it on the reviewed head. Existing exact-head Ruby 3.3/3.4/4.0, typecheck, package and lint checks are green; the added compatibility tests only cover explicit-type legacy hashes. The local Minitest file could not start because minitest/mock is not installed, so I am not claiming a local full-suite pass.
Castiron-Internal-PR: openai/openai-ruby-internal#129 Castiron-Source-SHA: a57df9ba45efb8d930a9ca21348d270b1932ced9 Castiron-Public-Base-SHA: dc3566d
d1b2d68 to
7d0a4d7
Compare
3 participants
A multi-grader needs explicit keys for the variables in
calculate_output. The SDK now accepts named grader maps and keeps the single-grader Ruby forms already exposed to callers. For example:The existing
MultiGrader::Gradersunion and RBSgradersalias remain available and retain their matching behavior. Named values useMultiGrader::Grader/grader. Existing single values still serialize in their original shape; names are never invented from display names or array positions. Use an explicit map when sending a multi-grader to the API.Validation: real second generation retained the Ruby model, RBI, RBS and tests byte-for-byte. Six tests / 46 assertions cover all five grader types, string/symbol keys, a variable called
type, unchanged legacy payloads and complete Run/Validate request/response round trips. Full RuboCop, Ruby/RBI and RBS formatting, Sorbet and 1,568 RBS signature checks passed. Native custom-code budget: 3,696/4,000.The local full SDK test runner could not download the pinned mock server's JSR dependencies, so the published PR's full CI must pass before review.