fix: validate OpenAIPrompt Java post-processing options - #2576
Open
fallintoplace wants to merge 14 commits into
Open
fix: validate OpenAIPrompt Java post-processing options#2576fallintoplace wants to merge 14 commits into
fallintoplace wants to merge 14 commits into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Hey @fallintoplace 👋! We use semantic commit messages to streamline the release process. Examples of commit messages with semantic prefixes:
To test your commit locally, please follow our guild on building from source. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes inconsistent validation/inference behavior for OpenAIPrompt.setPostProcessingOptions when called from Java/Py4J by delegating the java.util.HashMap overload to the existing Scala Map overload, ensuring both call paths apply the same validation rules and post-processing inference.
Changes:
- Update the Java
HashMapoverload to delegate to the ScalaMap[String, String]overload (restoring validation + mode inference parity). - Add isolated unit tests covering CSV inference from
delimiter, regex requiringregexGroup, and Java/Scala invalid-option parity.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| cognitive/src/main/scala/com/microsoft/azure/synapse/ml/services/openai/OpenAIPrompt.scala | Delegate Java HashMap overload to Scala overload so Java/Py4J callers get the same validation and inference as Scala callers. |
| cognitive/src/test/scala/com/microsoft/azure/synapse/ml/services/openai/OpenAIPromptParamsSuite.scala | Add targeted tests validating Java overload behavior (mode inference + validation parity). |
Route generated Python setter and setParams calls through the validated JVM overload, and add Scala, Java, and Py4J regression coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #2576 +/- ##
==========================================
+ Coverage 84.78% 84.79% +0.01%
==========================================
Files 334 337 +3
Lines 17806 17944 +138
Branches 1623 1669 +46
==========================================
+ Hits 15097 15216 +119
- Misses 2709 2728 +19 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Keep generated constructor and explicit empty-map compatibility while routing non-empty post-processing options through JVM validation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
AB#2576 ## Summary Route Scala, Java, and generated Python OpenAIPrompt post-processing configuration through shared validation while preserving compatible empty-map and legacy persistence behavior. Add lifecycle regressions and preserve the complete six-round review record. ## Prompting Intent Take end-to-end ownership of GitHub PR 2576 after its upstream rebase. Audit API compatibility, Scala and Python behavior, Py4J, code generation, serialization, security, and tests through six sequential model review rounds; fix every finding; validate the final implementation; and push without rewriting history. ## Linked Sources - Pull request: microsoft#2576 - Review records: reviews/pr-2576/ ## Rationale Keep business rules on the JVM and expose narrow generated-Python hooks rather than duplicating validation semantics in hand-written wrappers. Use transient provenance for explicit-mode intent so historical serialized stages remain readable without changing public JVM signatures or persisted parameter shapes. Validate before mutating Python or JVM state so failed setters and setParams calls remain atomic. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
AB#2576 ## Summary Merge the latest remote PR head into the fully reviewed OpenAIPrompt changes so the branch includes the owner's newest master merge without rewriting either history. ## Prompting Intent Take end-to-end ownership of GitHub PR 2576 while verifying the remote head immediately before every push and never force-pushing or rewriting history. Reconcile any concurrent upstream movement safely before publishing the reviewed fixes. ## Linked Sources - Pull request: microsoft#2576 - Remote head: 1484e0b ## Rationale Use a normal merge because the PR owner advanced the branch with a new master merge after review completed. This preserves the remote commit as an ancestor, keeps the reviewed commit intact, and allows a standard fast-forward push without force or history replacement. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
AB#2576 ## Summary Remove the concrete test-only OpenAIPrompt subclass that the repository-wide fuzzing scanner treated as a production pipeline stage. Exercise generated Python output reflectively on the real stage instead. ## Prompting Intent Monitor the full Azure validation for GitHub PR 2576, investigate code failures, fix them without changing public behavior, rerun the exact failing coverage, and preserve the complete resolution record. ## Linked Sources - Pull request: microsoft#2576 - Failed Azure build: https://msdata.visualstudio.com/b9b2accc-2d1c-45b3-9d24-0eb5d78cc47f/_build/results?buildId=229226446 - Review record: reviews/pr-2576/task-2576-attempt-1-review-6-gemini-3.6-flash.md ## Rationale Reflection targets the existing JVM-public codegen method without adding a second concrete PipelineStage class. This retains the codegen drift regression while satisfying the repository invariant that every discoverable stage has fuzzing, serialization, Python, and R coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
AB#2576 ## Summary Delete the six internal multi-model review Markdown files so they are not included in the SynapseML pull request. ## Prompting Intent The engineer requested that review files be removed from the committed change set because they should not be merged into the repository. ## Linked Sources - Pull request: microsoft#2576 ## Rationale Keep the implementation, tests, and production-facing history in the PR while excluding internal review process artifacts that are not repository deliverables. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
AB#2576 ## Summary Merge SynapseML master at 617ad0f into the OpenAIPrompt PR branch without rewriting published history. ## Prompting Intent The engineer requested that the PR branch be updated with the latest changes from master. They selected a normal merge rather than rebasing and force-pushing the published branch. ## Linked Sources - Pull request: microsoft#2576 - Master commit: microsoft@617ad0f ## Rationale Use a merge commit to preserve the existing reviewed branch history and avoid a force-push. The incoming master changes do not overlap the OpenAIPrompt, codegen, or associated test files. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
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.
Problem
The
java.util.HashMapoverload ofOpenAIPrompt.setPostProcessingOptionswrites the parameter directly, bypassing the inference and validation performed by the ScalaMapoverload. Java and Py4J callers can therefore create configurations that Scala callers cannot, including regex options withoutregexGroupand unsupported option maps. The overload also fails to inferpostProcessingfrom valid options.Changes
Testing
cognitive/testOnly com.microsoft.azure.synapse.ml.services.openai.OpenAIPromptParamsSuite(3 tests passed)cognitive/Test/scalastyle(0 findings)cognitive/Compile/scalastyle(0 findings)