Enforce OpenAI generation penalty semantics - #1076
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Copilot started reviewing on behalf of
Baiju Meswani (baijumeswani)
September 6, 2026 16:22
View session
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Public C, C#, and Rust API documentation remains incomplete or contradictory about the zero-only constraint.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Enforces OpenAI penalty semantics by accepting zero as a no-op and rejecting nonzero values before inference.
Changes:
- Removes incorrect ORT penalty mappings.
- Adds validation across endpoints, converters, and direct SDK requests.
- Updates tests and multi-language documentation.
File summaries
| File | Description |
|---|---|
sdk_v2/rust/README.md |
Documents zero-only penalties. |
sdk_v2/rust/docs/api.md |
Updates builder API guidance. |
sdk_v2/python/test/unit/test_session_types.py |
Tests zero serialization. |
sdk_v2/python/test/unit/test_chat_settings.py |
Updates settings serialization coverage. |
sdk_v2/python/README.md |
Documents penalty restriction. |
sdk_v2/cs/README.md |
Updates C# example. |
sdk_v2/cpp/test/sdk_api/responses_test.cc |
Tests Responses rejection. |
sdk_v2/cpp/test/sdk_api/reasoning_model_test.cc |
Removes unsupported penalties. |
sdk_v2/cpp/test/sdk_api/chat_completions_test.cc |
Tests Chat Completions rejection. |
sdk_v2/cpp/test/internal_api/response_converter_test.cc |
Covers Responses conversion semantics. |
sdk_v2/cpp/test/internal_api/chat/search_options_test.cc |
Tests direct SDK behavior. |
sdk_v2/cpp/test/internal_api/chat_completions_converter_test.cc |
Covers chat conversion semantics. |
sdk_v2/cpp/src/service/responses_handler.cc |
Rejects unsupported penalties early. |
sdk_v2/cpp/src/service/chat_completions_handler.cc |
Adds endpoint validation. |
sdk_v2/cpp/src/inferencing/generative/openresponses/response_converter.cc |
Stops forwarding penalties. |
sdk_v2/cpp/src/inferencing/generative/chat/search_options.h |
Documents supported values. |
sdk_v2/cpp/src/inferencing/generative/chat/search_options.cc |
Removes incorrect ORT mappings. |
sdk_v2/cpp/src/contracts/responses.h |
Documents Responses contracts. |
sdk_v2/cpp/src/contracts/chat_completions.h |
Documents chat contracts. |
sdk_v2/cpp/src/contracts/chat_completions_converter.cc |
Validates chat parameters. |
sdk_v2/cpp/include/foundry_local/foundry_local_cpp.h |
Updates C++ API documentation. |
sdk_v2/cpp/include/foundry_local/foundry_local_c.h |
Updates C parameter documentation. |
Review details
- Files reviewed: 22/22 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Document the zero-only penalty contract in the C macros, C# IntelliSense, and Rust source rustdocs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bb683a10-24f0-42bd-85fc-8babb4878bd8
Copilot started reviewing on behalf of
Baiju Meswani (baijumeswani)
September 7, 2026 05:30
View session
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The behavior is consistently enforced across API layers with focused coverage for zero, positive, and negative values.
Review details
- Files reviewed: 26/26 changed files
- Comments generated: 0 new
- Review effort level: Balanced
bmehta001
approved these changes
Sep 8, 2026
bmehta001
added a commit
that referenced
this pull request
Sep 8, 2026
Preserve OpenAI penalty behavior from #1076 by continuing to accept neutral zero values and reject nonzero values, while removing the unreachable repetition-penalty Engine plan. Deduplicate validated stop strings in caller order so duplicate values do not consume matcher or Engine capacity. Reject true legacy beam-search early_stopping on Engine backends while preserving classic Generator support, and correct cross-SDK documentation. Pin non-release C++ CI to ORT GenAI 0.16.0-dev1001407373 so the Engine API used by this branch is compiled and tested. Files changed: .pipelines/foundry-local-packaging.yml; sdk_v2/cpp stop/search/Engine sources, public headers, and tests; C#, Python, and Rust option documentation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Foundry Local currently maps OpenAI
frequency_penaltyandpresence_penaltyvalues to ONNX Runtime GenAI options with different meanings.This can produce incorrect output. GitHub Copilot CLI also sends both fields with the neutral value
0.This change treats zero penalties as no-ops and rejects nonzero penalties before inference.
Behavior
frequency_penalty: 0is accepted and does not change model settings.presence_penalty: 0is accepted and does not change model settings.Why
OpenAI frequency and presence penalties are additive token-frequency controls.
ONNX Runtime GenAI repetition and diversity penalties have different semantics.
Silently mapping between them is not correct. Rejecting unsupported values is safer and makes the limitation clear.
Testing