fix: validate OpenAIPrompt Java post-processing options - #2576
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Hey Minh Vu (@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. |
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). |
|
/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 86.07% 86.05% -0.02%
==========================================
Files 331 334 +3
Lines 17654 17793 +139
Branches 1646 1625 -21
==========================================
+ Hits 15195 15312 +117
- Misses 2459 2481 +22 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
/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>
|
/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>
|
/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>
|
/azp run |
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>
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>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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 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>
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>
fac9540 to
e28c59a
Compare
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
## Summary Replace brittle reflection against a protected code-generation method with a narrow test-only subclass accessor. ## Prompting Intent Resolve the active PR microsoft#2576 review comment, preserve the Java and Python post-processing validation coverage, and keep the fix minimal and compatible across Scala/JVM visibility rules. ## Linked Sources - Pull request: microsoft#2576 - Review comment: microsoft#2576 (comment) ## Rationale A test subclass exercises the protected method through normal language access controls, avoiding reflective visibility differences and accessibility overrides while leaving production APIs unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 81d39bfc-927c-418a-90a8-e0f2cd8fc128
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
## Summary Replace the concrete test-only OpenAIPrompt subclass with a package-private accessor on the real stage so the generated Python source can be asserted without reflection or introducing a discoverable fake PipelineStage. ## Prompting Intent Prepare PR microsoft#2576 for merge after Azure's core fuzzing shard discovered the nested test helper and failed because its JVM constructor captured the enclosing suite. Preserve the review-requested non-reflective access while keeping pipeline-stage discovery and public APIs clean. ## Linked Sources - Pull request: microsoft#2576 - Review comment: microsoft#2576 (comment) - Failed Azure build: https://msdata.visualstudio.com/b9b2accc-2d1c-45b3-9d24-0eb5d78cc47f/_build/results?buildId=230154143 ## Rationale FuzzingTest intentionally instantiates every concrete PipelineStage and has no generic test-helper exclusion. A package-private forwarding method adds no new stage, avoids brittle reflective accessibility, and limits visibility to the OpenAI package. This is smaller and safer than adding a hard-coded fuzzing exemption or allowing a duplicate helper stage into the discovery set. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 81d39bfc-927c-418a-90a8-e0f2cd8fc128
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
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)Discovery follow-up
The first post-review Azure run showed that a concrete nested test subclass is discovered by the repository-wide
FuzzingTestas a realPipelineStageand cannot be instantiated without its enclosing suite. The test now uses a package-private accessor onOpenAIPrompt, preserving normal Scala access without reflection or a fake discoverable stage.OpenAIPromptParamsSuiteand all 10FuzzingTestchecks pass.