chore: sync spark4.1 with master - #2645
Open
Rana Singh (ranadeepsingh) wants to merge 92 commits into
Open
Conversation
Add a ReleaseBranchCompat job that runs on every PR to master. It rebases each release branch (starting with spark4.0) onto the PR HEAD and runs sbt compile test:compile to catch breakage before it lands in master. - Non-blocking (continueOnError: true) - Matrix-based for easy expansion to more release branches - Reports merge conflicts and compile failures as warnings Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ease demo image (#2557) Addresses MSRC case 110886 / incident 31000000570827. The mmlspark/release image (built from tools/docker/demo/Dockerfile) ships Spark 3.5.4, which pins netty 4.1.96.Final. That version is flagged for multiple CVEs (CVE-2023-44487, CVE-2024-29025, CVE-2025-24970, ...). Spark has not bumped netty in any 3.5.x release. netty 4.1.x is binary-compatible, so we replace all netty-*-4.1.96.Final*.jar files in /opt/spark/jars/ with 4.1.118.Final right after the Spark extract. This includes netty-codec-http2 (the specific artifact named by the finder). Also removes 'pyspark' from the conda install line. It was pulling a complete second Spark install (PySpark 4.0.1) into /usr/local/lib/python*/site-packages/pyspark/ that nothing in the demo image actually used (SPARK_HOME points at /opt/spark) and that doubled the surface area scanners report on. Validated locally: - /opt/spark/jars/netty-*-4.1.96.Final*.jar: 0 matches after build - /opt/spark/jars/netty-*-4.1.118.Final*.jar: full set present - /usr/local/lib/.../pyspark: no longer exists - spark-submit --version: works - spark.range(5).count(): returns 5 Jetty (shaded inside hadoop-client-runtime-3.3.4.jar at 9.4.43) is OUT OF SCOPE for this PR; that requires a Spark/Hadoop swap and will be tracked separately.
* chore: add SynapseML local setup skill ## Summary Add a project-scoped SynapseML agent skill that diagnoses local toolchain state, selects JDK 11 for SBT commands, runs a safe local Spark smoke test, and flags live-service tests before agents run them. ## Prompting Intent The engineer asked the agent to create a skill that helps any future agent get SynapseML working locally after the PR 2556 review exposed a local Java 21 and Scala 2.12 compiler-bridge failure. The engineer also asked to create a PR for the skill addition before continuing the original external PR review. ## Linked Sources - User request in current session: create a skill that will help any agent be able to get SynapseML working locally. - Follow-up user request in current session: create a PR for that skill addition and continue using it to review PR 2556. - Existing project-scoped skill convention: .agents/skills/code-review/SKILL.md. - Local validation output: doctor_status=ok, JDK 11 dry-run selected JAVA_HOME, smoke test passed, Azure Search tests flagged review_required. ## Rationale A project-scoped SynapseML skill keeps local setup guidance with the repository where future agents need it. The scripts use explicit parameters rather than session state, force JDK 11 for Scala 2.12 SBT commands, and include a live-service guard so agents do not accidentally create or delete Azure Search resources while validating changes. * chore: move SynapseML setup skill to Copilot path ## Summary Move the SynapseML local setup skill from `.agents/skills/` to `.github/skills/` so it uses the documented Copilot project-skill discovery path. ## Prompting Intent The engineer asked whether the `.agents` folder was correct and whether Copilot would pick it up. Investigation found that the local skill-authoring reference documents `.github/skills/<name>/` and `.claude/skills/<name>/` as project skill locations, so the open skill PR needed a path correction. ## Linked Sources - User question in current session: is this .agent folder correct? will copilot pick this up? - Skill-authoring reference: /home/brwals/.copilot/installed-plugins/copilot-toolkit-marketplace/common/skills/create-skill/references/REFERENCE.md - Existing PR: #2558 ## Rationale The existing `.agents/skills/code-review` directory was only evidence of a repo-local convention, not evidence of Copilot discovery. Moving the new skill to `.github/skills/synapseml-local-setup/` keeps the same skill content while placing it in the documented project-skill path.
#2560) * Add v1 OpenAI Endpoint support and remove legacy completions API * Fix FuzzingUnitTest * Add test to increase code coverage * Make v1 api assumption cleaner * Add OpenAICompletion deprecation * Remove deprecation warnings * Fix RAI test for OpenAIPrompt * Revert "Add OpenAICompletion deprecation" This reverts commit fa708e2. * Revert "Fix RAI test for OpenAIPrompt" This reverts commit 3ed6044. * Revert "Remove deprecation warnings" This reverts commit 9a40c5c. * Reapply "Remove deprecation warnings" This reverts commit 987484c. * Reapply "Fix RAI test for OpenAIPrompt" This reverts commit f06f1ad. * Reapply "Add OpenAICompletion deprecation" This reverts commit 10715cd.
## Summary Move the remaining SynapseML repo skill from `.agents/skills/` to `.github/skills/` so Copilot CLI can discover all repo-versioned skills from the documented project-skill path. Add README pointers under `.agents/` for tools or agents that inspect the older convention. ## Prompting Intent The engineer asked to migrate everything to the correct Copilot CLI path and suggested keeping a generic agents pointer. The goal was to make existing skills discoverable by Copilot while avoiding future confusion about `.agents/skills`. ## Linked Sources - User request in current session: migrate everything to the correct path for Copilot CLI and keep a generic agents pointer. - Skill location reference: /home/brwals/.copilot/installed-plugins/copilot-toolkit-marketplace/common/skills/create-skill/references/REFERENCE.md - Prior merged skill PR: #2558 ## Rationale `.github/skills/<name>/` is the documented Copilot CLI project-skill location. Keeping only README pointers under `.agents/` preserves a breadcrumb for other agent conventions without leaving duplicate or stale `SKILL.md` files in a path Copilot CLI may not load.
* add speechtotextsdk improvements * Fix ffmpeg output args * add ffmpeg url check * fix: address speech recording review feedback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: make OpenAIPrompt RAI test resilient Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Revert "test: make OpenAIPrompt RAI test resilient" This reverts commit fccce86. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* chore: remove Acrolinx integration config AB#5391146 AB#5391147 ## Summary Remove the retired Acrolinx repository configuration from SynapseML and add the Feature Registry pointer and repo-specific design notes for Feature 5391136. ## Prompting Intent Engineer asked the agent to complete the Acrolinx removal request from the Microsoft Learn authoring tools PM. The repository cleanup needed to remove stale source-controlled Acrolinx state while preserving Feature Registry traceability for the administrative webhook removal and the June 30 contract-expiration risk. ## Linked Sources - ADO Feature: https://msdata.visualstudio.com/A365/_workitems/edit/5391136 - Design Spec task: https://msdata.visualstudio.com/A365/_workitems/edit/5391146 - Deployment task: https://msdata.visualstudio.com/A365/_workitems/edit/5391147 - Feature Registry specs: https://msdata.visualstudio.com/A365/_git/FeatureRegistry?path=/Features/active/5391136 - Teams request: https://teams.microsoft.com/l/message/19:81ff723c-eac9-4b2a-ba9f-844542135555_cc1adbf9-6510-43d6-a849-adba51e66d59@unq.gbl.spaces/1782314980087?context=%7B%22contextType%22%3A%22chat%22%7D - Acrolinx config before cleanup: https://github.com/microsoft/SynapseML/blob/b0fa222cfdde5d0a2cbb2bc6a35630bbb61bc0e3/.acrolinx-config.edn ## Rationale Deleting `.acrolinx-config.edn` is the least invasive source change because the Acrolinx contract is ending and the repo-level webhook was already removed through GitHub administration. Keeping the Feature Registry folder in the repo gives future maintainers a durable pointer to the reason for the cleanup without adding runtime or build behavior. * chore: keep Feature Registry metadata out of SynapseML AB#5391146 AB#5391147 ## Summary Remove the Feature Registry scaffold files from the SynapseML cleanup branch so the public repository PR only deletes the retired Acrolinx config. ## Prompting Intent Engineer clarified that Feature Registry metadata must not be included in the external SynapseML repository. The agent adjusted the existing cleanup PR to keep registry tracking in FeatureRegistry only while preserving the Acrolinx source cleanup. ## Linked Sources - ADO Feature: https://msdata.visualstudio.com/A365/_workitems/edit/5391136 - SynapseML PR: #2570 - FeatureRegistry PR: https://msdata.visualstudio.com/A365/_git/FeatureRegistry/pullrequest/2169703 - User correction: do not include Feature Registry metadata in the external repo ## Rationale Keeping the public SynapseML PR scoped to `.acrolinx-config.edn` avoids adding internal Feature Registry process artifacts to an external repository. Feature-level tracking remains in the FeatureRegistry PR and ADO work items.
* fix: route AnalyzeText document errors to errorCol Move Azure AI Language document-level errors returned inside HTTP 200 AnalyzeText responses from the response payload into the configured error column after auto-batch flattening. Preserve transport error precedence and add a no-network regression test for mixed document success/error responses. AB#4638662 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci: pin PR validation sbt launcher Use the sbt launcher version from project/build.properties instead of installing the latest apt sbt package. This keeps the JDK 11 PR validation job on the repository's sbt 1.10.11 launcher and avoids sbt 2.x rejecting JDK 11 before scalastyle can run. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci: use pinned sbt wrapper in PR validation Invoke the downloaded sbt launcher explicitly so the GitHub runner does not resolve its preinstalled sbt 2.x binary under JDK 11. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci: prefer pinned sbt on PATH Keep PR validation commands as plain sbt while placing the repository-version launcher first on PATH for subsequent workflow steps. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: avoid ordering assumption in AnalyzeText error test Partition collected rows by error nullability instead of relying on collect order, addressing PR review feedback about Spark DataFrames being unordered. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
fix: restore Azure pipeline
Pin the shared Python test environment to MLflow 2.21.3, matching the Databricks test dependency. This constrains protobuf to a compatible major version and invalidates the stale conda cache that breaks Python test collection.
test: migrate OpenAI tests and examples to GPT-5.1
ci: migrate Databricks GPU pool to T4
fix: correct LightGBM improvement tolerance semantics
Bumps [amannn/action-semantic-pull-request](https://github.com/amannn/action-semantic-pull-request) from 5.4.0 to 6.1.1. - [Release notes](https://github.com/amannn/action-semantic-pull-request/releases) - [Changelog](https://github.com/amannn/action-semantic-pull-request/blob/main/CHANGELOG.md) - [Commits](amannn/action-semantic-pull-request@v5.4.0...v6.1.1) --- updated-dependencies: - dependency-name: amannn/action-semantic-pull-request dependency-version: 6.1.1 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Rana Singh <ranadeep.dtu@gmail.com>
Bumps [ossf/scorecard-action](https://github.com/ossf/scorecard-action) from 2.3.1 to 2.4.4. - [Release notes](https://github.com/ossf/scorecard-action/releases) - [Changelog](https://github.com/ossf/scorecard-action/blob/main/RELEASE.md) - [Commits](ossf/scorecard-action@0864cf1...2d11466) --- updated-dependencies: - dependency-name: ossf/scorecard-action dependency-version: 2.4.4 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [actions/setup-java](https://github.com/actions/setup-java) from 5.6.0 to 5.7.0. - [Release notes](https://github.com/actions/setup-java/releases) - [Commits](actions/setup-java@03ad4de...b6effb0) --- updated-dependencies: - dependency-name: actions/setup-java dependency-version: 5.7.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Rana Singh <ranadeep.dtu@gmail.com>
* docs: add T4 GPU local RAG quickstart ## Summary Add an end-to-end local RAG notebook that performs sentence embedding, exact retrieval, and Phi-4-mini generation on a Databricks T4 worker. Register the notebook in the active GPU smoke suite and documentation sidebar with pinned model dependencies. ## Prompting Intent Reassess the unmerged GPU demo from PR #2271 against current master. Add a maintainable integration example only if it fills a gap beyond the standalone GPU KNN, Hugging Face CausalLM/Phi, and PDF Q&A notebooks; use current T4 assumptions, avoid TensorRT-LLM and custom CUDA setup, provide deterministic smoke assertions, and make no unrelated pipeline changes. ## Linked Sources - Original proposal: #2271 - GPU KNN component: #2157 - Local embedding component: #2236 - Hugging Face CausalLM/Phi component: #2301 - Current Databricks T4 validation platform: #2579 - PDF Q&A reference: https://github.com/microsoft/SynapseML/blob/master/docs/Explore%20Algorithms/AI%20Services/Quickstart%20-%20Document%20Question%20and%20Answering%20with%20PDFs.ipynb ## Rationale The existing notebooks document the individual building blocks but not their local, service-free composition. Exact PyTorch cosine scoring keeps the tutorial small and fully testable on the active T4 suite without reviving the disabled RAPIDS pipeline or its obsolete CUDA/TensorRT initialization. The notebook uses supported current-master models, max_new_tokens rather than conflicting sequence limits, and a PR smoke mode that exercises every GPU stage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: harden GPU RAG reproducibility checks ## Summary Pin both Hugging Face repositories to immutable commit snapshots, load the Phi model and tokenizer from the same local snapshot with remote code disabled, and strengthen retrieval validation against input-order fallback. ## Prompting Intent Address independent review findings on PR #2588 by removing mutable model resolution and trust_remote_code, then make the smoke test prove that GPU similarity ranking—not corpus order—selects the answer document. ## Linked Sources - Follow-up pull request: #2588 - Original proposal: #2271 - Pinned embedding snapshot: https://huggingface.co/sentence-transformers/all-MiniLM-L6-v2/tree/1110a243fdf4706b3f48f1d95db1a4f5529b4d41 - Pinned Phi snapshot: https://huggingface.co/microsoft/Phi-4-mini-instruct/tree/cfbefacb99257ffa30c83adab238a50856ac3083 ## Rationale SentenceTransformer accepts an immutable revision for its complete model/tokenizer snapshot. HuggingFaceCausalLM loads its tokenizer separately, so Phi is first resolved to one pinned worker-local snapshot and both loaders receive that path. Transformers 4.49 natively supports the checkpoint's phi3 architecture, allowing remote model code to remain disabled. A persisted corpus ordinal and independent Python sort over all GPU scores prove the top-k result differs from the first input rows and has strict score ordering. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * style: format GPU RAG notebook cells ## Summary Apply the repository-pinned Black 22.3 Jupyter formatter to the updated GPU RAG notebook cells. ## Prompting Intent Resolve the Python Style CI failure on PR #2588 without changing notebook behavior or broadening the patch. ## Linked Sources - Pull request: #2588 - Failed Azure build: https://msdata.visualstudio.com/b9b2accc-2d1c-45b3-9d24-0eb5d78cc47f/_build/results?buildId=229241355 ## Rationale Black's Jupyter formatter omits the terminal newline stored in each code cell. Formatting only the touched notebook aligns its JSON representation with the CI environment while preserving all model-pinning and retrieval assertions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: pin GPU RAG hub client and FP16 ## Summary Pin huggingface-hub 0.26.0 in the Databricks GPU libraries and notebook setup, verify the Hugging Face dependency set in unit tests, and force Phi model loading to FP16 on T4 hardware. ## Prompting Intent Address the second independent re-review of PR #2588 by making snapshot_download's client version reproducible and preventing Phi's BF16 checkpoint metadata from selecting an unsupported native dtype on T4 GPUs. ## Linked Sources - Pull request: #2588 - Repository environment pin: environment.yml - Hugging Face Hub 0.26.0: https://pypi.org/project/huggingface-hub/0.26.0/ - Pinned Phi configuration: https://huggingface.co/microsoft/Phi-4-mini-instruct/blob/cfbefacb99257ffa30c83adab238a50856ac3083/config.json ## Rationale Version 0.26.0 is already the repository-pinned lower bound used with Transformers 4.49.0, so installing that exact version on the GPU cluster makes snapshot resolution deterministic without introducing a new dependency choice. Phi advertises bfloat16 in its configuration, while NVIDIA T4 compute capability 7.5 lacks native BF16; passing the supported float16 dtype explicitly avoids architecture-dependent auto selection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: guard accelerate GPU dependency pin ## Summary Assert that the Databricks GPU library manifest retains accelerate==0.26.0 alongside the pinned Hugging Face dependencies. ## Prompting Intent Address the remaining actionable review feedback on PR #2588 by preventing the runtime dependency used for distributed Phi loading from drifting without a focused unit-test failure. ## Linked Sources - Pull request: #2588 - Reviewed GPU library manifest: core/src/test/scala/com/microsoft/azure/synapse/ml/nbtest/DatabricksUtilities.scala ## Rationale The package is already explicitly pinned in GPULibraries, so extending the existing parsed-manifest test is the smallest regression guard and avoids duplicating library configuration or changing runtime behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
#2601) * chore(deps): bump github/codeql-action/autobuild from 4.37.3 to 4.37.5 --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: ranadeepsingh <16433904+ranadeepsingh@users.noreply.github.com> Co-authored-by: Rana Singh <ranadeep.dtu@gmail.com>
Bumps [postcss](https://github.com/postcss/postcss) from 8.5.19 to 8.5.25. - [Release notes](https://github.com/postcss/postcss/releases) - [Changelog](https://github.com/postcss/postcss/blob/main/CHANGELOG.md) - [Commits](postcss/postcss@8.5.19...8.5.25) --- updated-dependencies: - dependency-name: postcss dependency-version: 8.5.25 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Rana Singh <ranadeep.dtu@gmail.com>
* chore: migrate artifact links off retiring Azure CDN ## Summary Replace all 400 current-master references to mmlspark.azureedge.net with the repository-owned mmlspark Blob Storage origin across runtime package configuration, release output, examples, documentation, notebooks, and every published documentation version. ## Prompting Intent Recreate the intent of the stale CDN-removal PR on current master only after verifying the supported artifact destination and Azure CDN retirement path. Audit each endpoint use by semantics, preserve package and content paths, validate live artifacts and package resolution, and avoid changing or closing the original PR. ## Linked Sources - Original proposal: #2326 - Azure CDN retirement FAQ: https://learn.microsoft.com/en-us/azure/cdn/classic-cdn-retirement-faq - Azure CDN migration guidance: https://learn.microsoft.com/en-us/azure/cdn/migrate-tier - Azure Front Door/CDN comparison: https://learn.microsoft.com/en-us/azure/frontdoor/front-door-cdn-comparison ## Rationale SynapseML's release pipeline publishes artifacts directly to the mmlspark storage account, the repository already uses that public Blob Storage origin extensively, and byte-for-byte URL checks confirmed the CDN currently proxies the same content. Using the verified origin removes the retiring CDN hostname without inventing an unverified Front Door name, while preserving Maven, documentation, R-package, model, dataset, and icon path semantics. Historical links that already return 404 retain the same status and are not broadened into unrelated artifact-repair work. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: make R setup independent of retired CDN ## Summary Repair current and versioned R setup guidance so each release installs its six published, version-matched component archives and resolves SynapseML JVM artifacts through Blob Storage. Document the compatibility bypass required by already-published wrappers, correct the Databricks setup and LightGBM example, remove invalid HTML-page Maven repositories from the Docker demo, and add generator/docs regressions. ## Prompting Intent Investigate the review finding that published R archives still register the retired Azure CDN resolver. Make repository-controlled R installation work with that hostname unavailable, avoid claiming that externally published archives were rewritten, validate local and Databricks-oriented resolution paths, and state the exact external publishing prerequisite for a full artifact migration. ## Linked Sources - Original migration PR: #2326 - Current migration PR: #2589 - Maven repository review: #2589 (comment) - Azure CDN retirement FAQ: https://learn.microsoft.com/en-us/azure/cdn/classic-cdn-retirement-faq - Azure Front Door migration guidance: https://learn.microsoft.com/en-us/azure/cdn/migrate-tier - Apache Spark package repository configuration: https://spark.apache.org/docs/3.5.0/configuration.html#runtime-environment ## Rationale Existing release archives cannot be repaired by a source-only change because their generated sparklyr metadata is already published. Version-matched component downloads plus an explicit Blob resolver and `extensions = character()` provide a tested repository-controlled path without racing or misrepresenting external publication. Future generated archives inherit the corrected resolver from PackageUtils; fully repairing historical metadata still requires an authorized regeneration and publish to the `mmlspark/rrr` container (or a replacement release). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: correct Spark 3.4 compatibility guidance ## Summary Correct the Spark Packages and Python installation snippets so both identify SynapseML 1.0.15 as the compatible release for Spark 3.4 while retaining SynapseML 1.1.3 for Spark 3.5. ## Prompting Intent Address the remaining actionable review feedback on PR #2589 in the existing branch, verify the surrounding compatibility guidance stays consistent, run targeted website validation and code review, and rerun the full PR checks. ## Linked Sources - Pull request and review feedback: #2589 - Original migration context: #2326 ## Rationale The Databricks, Fabric, and SBT guidance already distinguishes SynapseML 1.1.3 for Spark 3.5 from 1.0.15 for Spark 3.4. Updating only the two stale explanatory references restores consistency without changing the Spark 3.5 commands that the snippets demonstrate. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
feat: add backward-compatible AAD auth for Azure Search
## Summary Count rows on the original DataFrame RDD so adaptive execution cannot coalesce a projected counting query into a different partition topology. Add a regression that exposes the old 20-to-fewer-partitions drift and verifies exact per-partition counts. ## Prompting Intent Recreate the valid intent behind ancient PR #2282 from current master only after reproducing issue #2278. Isolate distributed startup, feature-width bounds, and native pointer lifetime separately; use TDD and submit only a proven root cause with real regression coverage. ## Linked Sources - Reported failure: #2278 - Superseded ancient proposal: #2282 ## Rationale The literal-only projection was cheaper, but AQE could optimize it to fewer partitions than the training DataFrame. LightGBM then indexed that shortened count array with real task partition IDs, causing the primary ArrayIndexOutOfBoundsException and secondary connection failures. Counting the exact DataFrame RDD trades projection pruning for topology correctness. Feature-width validation and innerPredict cleanup were deliberately excluded because neither was demonstrated as the cause of #2278 or backed by a stable leak regression. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Bumps [github/codeql-action/upload-sarif](https://github.com/github/codeql-action) from 4.37.3 to 4.37.4. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@e4fba86...f205ea1) --- updated-dependencies: - dependency-name: github/codeql-action/upload-sarif dependency-version: 4.37.4 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [github/codeql-action/upload-sarif](https://github.com/github/codeql-action) from 4.37.4 to 4.37.5. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@f205ea1...d1ba80a) --- updated-dependencies: - dependency-name: github/codeql-action/upload-sarif dependency-version: 4.37.5 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
The Azure AI Anomaly Detector service has been retired by Microsoft. Every `anomalydetector` REST endpoint now answers HTTP 410 (Gone), verified across paths, API versions and regions.
* fix: correct LightGBM improvement tolerance semantics ## Summary Require lower-is-better validation metrics to improve by more than improvementTolerance before resetting the early-stopping counter. Clarify the parameter documentation and add focused regression coverage for both metric directions and zero tolerance. ## Prompting Intent Investigate GitHub issue #2565 from a new branch based on master, determine whether the report is valid, and implement a complete fix suitable for an upstream SynapseML pull request. ## Linked Sources - GitHub issue: #2565 ## Rationale The existing higher-is-better comparison already treats improvementTolerance as a minimum delta, while lower-is-better metrics accepted small regressions. A package-internal comparison helper makes the intended symmetric behavior directly testable without adding a slow native LightGBM fixture or changing public APIs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: harden LightGBM early stopping parameters ## Summary Expand improvement-tolerance coverage across representative LightGBM metrics and tolerance values. Preserve disabled early stopping when earlyStoppingRound is zero, validate both early-stopping parameters, and document their accepted ranges. ## Prompting Intent The engineer requested broader parameter testing to ensure the issue #2565 fix does not introduce downstream regressions. Cover related defaults, boundaries, metric families, invalid values, and early-stopping-round interactions before updating the pull request. ## Linked Sources - GitHub issue: #2565 - Pull request: #2578 - LightGBM 3.3.5 parameters: https://lightgbm.readthedocs.io/en/v3.3.5/Parameters.html#early-stopping-round ## Rationale Correct tolerance semantics classify more rounds as non-improving, so the wrapper must explicitly preserve LightGBM's zero-means-disabled behavior. Shared Spark parameter validators reject values that LightGBM does not support, while deterministic matrix tests cover the decision logic without depending on platform-specific native binaries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci: prevent sbt bootstrap Maven rate limits SynapseML's Azure Pipelines fans out ~30 hosted-agent matrix jobs that each cold-bootstrap the sbt launcher (org.scala-sbt:sbt:1.10.11, pinned in project/build.properties) and resolve Ivy dependencies from public Maven Central. When many fresh agents -- and several overlapping PR builds -- do this simultaneously, Maven Central returns HTTP 429 (rate limit) and "Setup repo" fails before any test runs (e.g. ADO build 229124511, UnitTests flaky). The pre-existing jittered retries only widened the window against a sustained throttle; they did not remove the thundering herd. Durable fix (cache-first, stagger as supplement): * templates/sbt_cache.yml (primary): Azure Cache@2 for the sbt launcher boot dir (~/.sbt/boot -- the artifact that 429s) and the Ivy cache (~/.ivy2/cache). In steady state, jobs restore these from Azure's cache service and never touch Maven Central. Keys derive from the bootstrap inputs (project/build.properties, project/plugins.sbt, build.sbt) so they invalidate exactly when those change; restoreKeys give a safe partial fallback and continueOnError keeps a cache miss/corruption non-fatal. * BuildAndCacheSbt prewarm job: warms those caches once per run, mirroring the existing BuildAndCacheCondaEnv job. * tools/ci/sbt_retry.sh: single tested helper replacing the duplicated inline retry blocks. Smooths only the cold-cache path with a bounded random start stagger (desynchronises concurrent cold bootstraps) plus bounded jittered exponential-backoff retries. Fails visibly on exhaustion -- no success fallback masking. Wired the shared cache template into every sbt-running job (Style, Publish, Databricks/Fabric E2E, BuildDocker, PythonTests, RTests, WebsiteSamplesTests, UnitTests, ReleaseBranchCompat) by reviving the dormant ivy_cache placeholders, and routed all `sbt setup` bootstraps through the helper. Tests (python -m pytest tools/ci/tests/): deterministically exercise the retry/backoff/stagger + visible-failure behaviour with a fake sbt, and assert pipeline.yaml parses, the cache keys invalidate on bootstrap inputs, and every sbt job is wired to the cache template + prewarm job. No LightGBM, Isolation Forest, GPU, or application changes. TLS verification, job coverage, and all tests are preserved. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci: serialize sbt cache prewarm before fan-out ## Summary Make the sbt bootstrap prewarm a mandatory gate before Azure Pipeline matrix jobs start. Add Coursier caching, require exact hits on the boot, Ivy, and Coursier caches before disabling the cold-cache stagger, wire the conditional release job, and strengthen pipeline tests around the dependency graph and cache lifecycle. ## Prompting Intent The engineer asked to fix Maven Central HTTP 429 setup failures in a new stacked PR. The solution must prevent fresh hosted agents from cold-bootstrapping sbt concurrently, allow at least the existing job fan-out after bootstrap is safe, retain bounded retry behavior for cache-service failures, and keep bootstrap failures visible rather than masking them. ## Linked Sources - Failing Azure job: https://msdata.visualstudio.com/b9b2accc-2d1c-45b3-9d24-0eb5d78cc47f/_build/results?buildId=229124511&view=logs&jobId=e97036a1-bcdb-5cd5-905e-b0cf2c8f33cf - Parent PR investigation: #2578 (comment) - Stacked PR: #2581 - Prewarm concurrency review: #2581 (comment) ## Rationale A best-effort prewarm running beside the matrix does not protect the first run for a new dependency key, so every sbt-running job now waits for one successful warm job. Cache-service errors remain non-fatal and fall back to staggered retries, but a failed warm blocks fan-out to avoid recreating the thundering herd. Coursier is cached alongside sbt boot and Ivy because modern resolution uses all three stores, and the stagger is suppressed only when every cache is an exact hit so dependency-only changes remain desynchronized. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci: skip Databricks E2E for non-impacting PRs ## Summary Add conservative pull-request impact detection for the six-leg Databricks E2E matrix. Clearly non-impacting documentation, website, GitHub metadata, CI helper, and isolated test-source changes skip Databricks, while all uncertain or runtime-affecting changes continue to run it. ## Prompting Intent The engineer asked to extend PR #2581 so expensive Databricks Azure Pipeline jobs are skipped when the pull request cannot affect notebook execution. The gate must preserve scheduled and branch coverage, avoid brittle CPU-shard mapping, and default to running whenever impact detection is incomplete or uncertain. ## Linked Sources - Stacked CI PR: #2581 - Full green baseline build: https://msdata.visualstudio.com/b9b2accc-2d1c-45b3-9d24-0eb5d78cc47f/_build/results?buildId=229164855 - Azure multi-job output variables: https://learn.microsoft.com/en-us/azure/devops/pipelines/process/conditions?view=azure-devops#job-output-variables-used-in-other-job-conditions ## Rationale Use one fail-open decision for the complete Databricks matrix because the five CPU partitions mix notebooks across modules and are not stable ownership boundaries. The detector skips only a narrow allowlist of clearly inert paths; runtime code, notebooks, build and pipeline files, Databricks test utilities, shared TestBase infrastructure, unknown paths, empty diffs, and fetch or classifier failures all keep E2E enabled. Non-PR builds always run to preserve scheduled and release coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci: gate Databricks CPU and GPU tests independently (#2582) * Gate Databricks CPU and GPU tests independently ## Summary Classify changed paths against the actual Databricks CPU and GPU runtime surfaces, emit separate fail-open decisions, and gate each matrix leg independently. ## Prompting Intent The engineer asked to determine exactly when Databricks tests should run, lock down the path rules, and deliver the work as a stacked pull request above PR #2581. ## Linked Sources - Base CI hardening PR: #2581 - GitHub stacked PR documentation: https://docs.github.com/en/pull-requests/how-tos/create-pull-requests/creating-stacked-pull-requests - ADO timing audit: build 229176406 ## Rationale CPU and GPU decisions are separated because most module changes cannot affect the expensive GPU notebooks. Unknown paths and shared build or test infrastructure remain fail-open, while explicit test-only and unrelated tooling paths skip safely. This preserves coverage while avoiding unrelated GPU capacity waits. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci: fix and streamline release branch compatibility (#2583) * Fix and streamline release branch compatibility checks ## Summary Run release compatibility checks for both GitHub target-branch formats and replace redundant compile, setup, credential, and per-package SBT tasks with one cached, project-scoped validation process. ## Prompting Intent The engineer asked to fix the silently skipped ReleaseBranchCompat job and simplify it before enabling it so the check is both reliable and efficient. ## Linked Sources - Base CI hardening PR: #2581 - Evidence build with skipped phase: ADO build 229176406 - Parent stack layer: ci/databricks-impact-gating ## Rationale The target condition accepts both values observed across Azure Repos and GitHub PR providers. A single SBT process retains full test compilation and the intended core, VW, and OpenCV compatibility suites while removing repeated build loading, root-wide IntelliJ setup, unnecessary Key Vault access, and Azure CLI authentication. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci: use Entra authentication for ACR cleanup (#2584) * Use Entra authentication for ACR cleanup Make the weekly ACR cleanup schedule-only, switch it to the dedicated cleanup service connection, replace storage connection-string authentication with Azure CLI Entra authentication, and add fail-safe cleanup tests. The engineer asked to repair the weekly cleanup failures caused by disabled key-based storage authentication, use the declared least-privileged identity, and prevent accidental CI or PR execution. - Failed scheduled build: ADO build 228250033 - Base CI hardening PR: #2581 - Azure CLI pipeline-run reference: https://learn.microsoft.com/en-us/cli/azure/acr/pipeline-run - Parent stack layer: ci/release-branch-compat Using az storage blob exists with auth-mode login keeps all operations inside the AzureCLI task identity and removes runtime SDK installation, Key Vault access, and storage keys. Images are deleted only after the archive is confirmed, and subprocess argument lists avoid shell interpolation of registry-controlled names. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove redundant CI authentication and Conda work (#2585) ## Summary Run non-Azure setup and coverage commands as Bash steps, install pinned Black without restoring the 8.6 GB Conda environment, and remove the ineffective standalone Conda cache consumer. ## Prompting Intent The engineer asked for additional improvements that should ship with the requested CI fixes to make builds faster and more reliable without broad behavioral changes. ## Linked Sources - CI efficiency audit from ADO build 229176406 - Base CI hardening PR: #2581 - Parent stack layer: ci/fix-acr-cleanup-auth ## Rationale AzureCLI tasks create an isolated login for every invocation, so setup and coverage steps that never call az gain no authentication benefit. The Style job only needs pinned Black, not the full cached environment. The standalone Conda job was not a dependency and therefore could not prewarm consumers or prevent cold-cache fan-out. 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> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci: extend Docker validation timeout ## Summary Raise the BuildDocker job timeout from 60 to 120 minutes and add a pipeline regression test that preserves enough time for both sequential image builds. ## Prompting Intent The engineer asked to diagnose and fix the remaining failure on #2581 and to continue full validation until the parent PR is ready, without hiding genuine test failures. ## Linked Sources - Parent PR: #2581 - Failed PR build: https://msdata.visualstudio.com/b9b2accc-2d1c-45b3-9d24-0eb5d78cc47f/_build/results?buildId=229579403 - Matching master failure: https://msdata.visualstudio.com/b9b2accc-2d1c-45b3-9d24-0eb5d78cc47f/_build/results?buildId=229578121 - Matching master failure: https://msdata.visualstudio.com/b9b2accc-2d1c-45b3-9d24-0eb5d78cc47f/_build/results?buildId=229580525 - Matching master failure: https://msdata.visualstudio.com/b9b2accc-2d1c-45b3-9d24-0eb5d78cc47f/_build/results?buildId=229576499 ## Rationale The Dockerfiles and image behavior were unchanged, but recent hosted-agent builds required roughly 51 minutes when successful and exceeded the default one-hour job cap in multiple master and PR runs. A 120-minute job budget keeps both image validations mandatory while tolerating current registry and package download latency. This is safer and more targeted than skipping an image or doubling agent usage by splitting the builds into parallel jobs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci: address PR review and protect package publishing ## Summary Resolve the blocking PR #2581 review findings by making ACR archival digest-safe, correcting PipelineRun names and sbt cache invalidation, warming cold agents before direct sbt calls, and validating the canonical package version before publishing. ## Prompting Intent The engineer asked to rebase PR #2581 onto current master, audit the new review feedback, fix valid actions, ensure the pipelines continue to publish package versions safely, review the complete change, and rerun Azure validation. ## Linked Sources - Integration PR and review threads: #2581 - Stacked CI changes: #2582 - Stacked CI changes: #2583 - Stacked CI changes: #2584 - Stacked CI changes: #2585 - ACR transfer guidance: https://learn.microsoft.com/azure/container-registry/container-registry-transfer-images - ACR image deletion behavior: https://learn.microsoft.com/azure/container-registry/container-registry-delete ## Rationale Immutable manifest digests prevent mutable tags such as latest from reusing the wrong backup or deleting an unarchived manifest. Per-agent warming is limited to unavailable or inexact cache restores so exact hits remain fast, while the prewarm job still verifies dependency resolution. Package versions are resolved from the SBT source of truth and release publication fails before side effects when the v-tag disagrees. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* feat: Add lossless string identifier support to SAR Refs #2275 Refs #2283 ## Summary Add deterministic, reversible user and item identifier mappings to SAR so string and wide numeric IDs are never cast into lossy caller-visible values. Persist mappings with the model, preserve identifier types in scores and recommendations, define null and unknown-ID behavior, restore typed item recommendation APIs, and add Scala and Python regression coverage. ## Prompting Intent Recreate the intent of the stale SAR string-ID change on current master without copying its lossy casts. Keep the SparkML API coherent and backward compatible for numeric users, use TDD, validate serialization and schema behavior, expose Python wrappers, and exercise targeted compile, style, code generation, Scala, and Python/JVM checks before opening a replacement PR. ## Linked Sources - Feature request: #2275 - Original pull request: #2283 - Current SAR implementation at the starting revision: https://github.com/microsoft/SynapseML/tree/7d9fabcc/core/src/main/scala/com/microsoft/azure/synapse/ml/recommendation - Repository review policy: .github/skills/code-review/SKILL.md ## Rationale Use model-owned typed mappings instead of composing RecommendationIndexer because that stage stringifies numeric identifiers, exposes index columns, and cannot recover every original type. Contiguous deterministic indices keep the existing matrix implementation viable, while persisted DataFrame parameters make decoding reversible after save/load. Inner mapping joins intentionally drop null or unseen scoring IDs, strict type validation prevents ambiguous conversions, and legacy numeric models fall back to identity mappings. The approach accepts a deterministic global sort and persisted mapping storage in exchange for lossless, reproducible SparkML behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: Address SAR identifier compatibility review Refs #2275 Refs #2594 ## Summary Resolve the four independent review findings on SAR string identifier support. Preserve typed IDs in ranking train/validation splits, accept only round-trip-safe numeric scoring casts, retain established integer recommendation schemas for safely representable numeric IDs, and rank only factor IDs that have real mappings. Add focused Scala and Python regressions and remove unnecessary mapping cache and interaction-count work identified during review. ## Prompting Intent The engineer asked to fix all medium correctness and compatibility findings on PR #2594, add a regression for each, rerun targeted Scala, code generation, formatting, and Python/JVM validation, then update the existing PR and request re-review without weakening lossless string or wide numeric behavior. ## Linked Sources - Pull request and review context: #2594 - Feature request: #2275 - Original pull request: #2283 - Repository review policy: .github/skills/code-review/SKILL.md ## Rationale Use Spark structs and array functions instead of Double UDF payloads so split schemas remain typed. Numeric scoring IDs are temporarily cast only when casting back reproduces the input, preventing overflow and fractional aliasing while retaining unknown-ID drop semantics. Recommendation decoding conditionally uses the historical integer schema only when every ID round-trips through Int; strings and wide or fractional numeric IDs remain lossless. Candidate indices are intersected with both factors and mappings before top-K so gaps cannot consume recommendation slots. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: Make SAR numeric identifier handling ANSI-safe Refs #2275 Refs #2594 ## Summary Use ANSI-safe try_cast expressions for numeric identifier compatibility and legacy mappings. Persist whether model-owned user and item mappings safely round-trip through IntegerType, reuse those flags when selecting recommendation output schemas, and limit destination-index collection to mapping-less legacy models. Add ANSI overflow, persisted-flag, legacy-default, and recommendation-planning regressions. ## Prompting Intent The engineer asked to resolve the second independent review of PR #2594: prevent CAST_OVERFLOW under spark.sql.ansi.enabled=true, eliminate repeated mapped-model recommendation scans and index collection, add focused regressions, rerun Scala/codegen/Python validation, update the existing PR, trigger Azure Pipelines, and request another re-review. ## Linked Sources - Pull request and review context: #2594 - Feature request: #2275 - Original pull request: #2283 - Repository review policy: .github/skills/code-review/SKILL.md ## Rationale Use Spark SQL try_cast in both cast directions rather than pre-cast comparisons so out-of-range values become null and are filtered even with ANSI mode enabled. Compute compatibility once while fitting and persist it with conservative false defaults for legacy models, avoiding full mapping scans on every recommendation call. New model mappings are contiguous, so mapped models rank the score vector directly; only mapping-less legacy models collect actual candidate indices to preserve gapped-ID correctness. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Configure a deterministic repository-local Git committer identity before replaying PR commits onto the Spark 3.5 and Spark 4.1 release branches. Distinguish genuine merge conflicts from rebase infrastructure failures and preserve successful rebase diagnostics.
* ci: preserve sbt retry helper during release replay ## Summary Stage the sbt retry helper outside the repository before switching to Spark release branches, and parameterize the shared cache template so it can invoke that stable path after rebase. ## Prompting Intent Investigate why Spark 3.5 and Spark 4.1 compatibility checks still failed after PR #2608, reproduce the failure with PR #2595 changes, implement the complete hotfix, and validate the real release replay path. ## Linked Sources - Failing PR: #2595 - Prior identity hotfix: #2608 - Failed Azure build: https://msdata.visualstudio.com/b9b2accc-2d1c-45b3-9d24-0eb5d78cc47f/_build/results?buildId=229969678 ## Rationale The rebased working tree intentionally comes from the Spark release branch, so master-only CI helpers cannot remain repository-relative. Copying the helper to Agent.TempDirectory preserves release-specific dependency resolution and avoids moving cache warming ahead of the rebase, where exact cache hits could hide missing release dependencies. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci: replay only release-relevant PR changes ## Summary Replace commit-history rebasing with a three-way application of the synthetic PR merge tree's release-relevant patch onto each Spark release branch. ## Prompting Intent Validate the compatibility hotfix with PR #2595's real source changes while ensuring CI-only commits do not conflict with old Spark branches that predate the current pipeline and helper files. ## Linked Sources - Validation PR source: #2595 - Prior identity hotfix: #2608 - Failed Azure build: https://msdata.visualstudio.com/b9b2accc-2d1c-45b3-9d24-0eb5d78cc47f/_build/results?buildId=229969678 ## Rationale The compatibility job needs the effective PR content on the release tree, not unrelated CI and documentation commits. Building the patch from the synthetic merge commit preserves GitHub's merge result, handles source branches behind master, retains three-way conflict detection, and avoids requiring commit identity. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci: focus release compatibility on Spark 4.1 compilation ## Summary Remove the redundant Spark 3.5 release matrix leg and replace broad Spark 4.1 runtime suites with full test compilation of the effective PR patch. ## Prompting Intent Explain why the release compatibility jobs exist and keep fixing the failures exposed by validation PR #2610, accounting for master already targeting Spark 3.5. ## Linked Sources - Original compatibility PR: #2550 - Streamlining PR: #2583 - Integration validation PR: #2610 - Azure validation build: https://msdata.visualstudio.com/b9b2accc-2d1c-45b3-9d24-0eb5d78cc47f/_build/results?buildId=229984834 ## Rationale Normal PR validation already compiles and tests master on Spark 3.5, so replaying onto the older spark3.5 maintenance snapshot duplicates coverage and introduces unrelated JVM drift. Spark 4.1 test compilation catches cross-version source and test API breakage, while the existing master test fan-out supplies runtime coverage without rerunning broad, memory-heavy suites on a constrained compatibility agent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: SynapseML CI <synapseml-ci@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
Author
Full Azure DevOps pipeline: green
Build 231429381 — This was run alone, with no other Spark 4 build in flight. That matters: Two things worth noting about what this run proves:
Commits added since this build was queuedThe build ran against Only |
Two files were doing the same job. `.github/copilot-instructions.md` held the architecture, codegen and convention guidance; `AGENTS.md` held the branch model and the rules for syncing. Agents read both, with no obvious precedence between them, and nothing said which one a new rule belonged in. `AGENTS.md` is the better survivor. It is the cross-tool convention, GitHub added support for it to Copilot in August 2025, and it is read by the coding agent, VS Code and the CLI alike, so folding one into the other loses no coverage. It also sits at the repository root next to `CONTRIBUTING.md`, which is where someone looks first. The merge is content-preserving: module map, directory layout, the code generation pipeline, the transformer/estimator pattern and its conventions, the cognitive service traits, file headers, build commands, Python and scalastyle rules, testing layout, CI/CD, and the numbered list of common mistakes all move across intact. Two things are deliberately different in the merged file. The first is that it carries no version numbers. The deleted file said "Spark 3.5.0, Scala 2.12.17" and pointed at `target/scala-2.12/generated/src/python/`, which was true on master and wrong on both Spark 4 branches -- where it directed agents at a generated-output directory that does not exist. That is not a typo anyone forgot to fix; it is the predictable result of restating in prose a fact that lives in `build.sbt`. The merged file names `build.sbt` and `environment.yml` as the source of truth, writes the generated path as `target/scala-<binary-version>/`, and defers per-branch specifics to `AGENTS_<branch>.md`. Being version-free is also what lets this file stay byte-identical on every branch, so syncing it is a no-op rather than a recurring conflict. The second is two additions earned the hard way rather than copied across: a short section on hand-written `__init__.py` files explaining that re-listing generated classes *narrows* the public API instead of extending it -- the exact defect that broke `PythonTests core` and seven website samples -- and a note that the `/azp run` comment does not trigger on every branch, so an absent pipeline run is not evidence that CI is broken. Deleted on `master`, `spark4.0` and `spark4.1` in the same change, so that no branch inherits a file the others have dropped and future syncs stay clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rana Singh (ranadeepsingh)
force-pushed
the
sync/spark4.1-with-master-2
branch
from
August 16, 2026 23:18
eee99c6 to
ebf1c49
Compare
…branch model (#2648) * docs: add AGENTS.md and document the branch model for contributors This repository maintains long-lived ports of the library to newer Spark versions on the `spark4.0` and `spark4.1` branches, but nothing in the repository said so. A contributor had no way to know which branch to target, and an agent working on a port branch had no way to tell a deliberate version-driven divergence from an accident. The practical consequence is that a "cleanup" reverting an intentional difference looks, in the diff, exactly like a tidy-up -- and only fails much later, in a pipeline that does not run on pull requests to those branches. Two files, both intended to be identical everywhere: - `AGENTS.md` (new) -- entry point for coding agents. Covers the branch model, the rule for resolving conflicts when master is merged into a port branch, how to verify a sync actually landed, and the repository-wide invariants that are easy to violate: Python wrappers are generated from Scala, generated output under `target/` must not be edited, a new stage needs `Wrappable`, `SynapseMLLogging` and a `DefaultParamsReadable` companion, and no RDD-based code. - `CONTRIBUTING.md` -- a short "Which branch should I target?" section saying master is the default, and that a fix applying everywhere should land here first so the port branches inherit it rather than conflicting with it. The branch-specific detail deliberately does *not* live here. Each port branch carries its own `AGENTS_<branch>.md` recording its toolchain and the reason for every divergence. This split exists so that these two files can stay byte-identical on every branch, which makes syncing them a no-op instead of a conflict someone resolves by hand on every merge. `AGENTS.md` states the rule and gives the tell: wanting to write a version number in a shared file means the content belongs in the branch file. The two files added here are byte-identical to the copies on both port branches. References to the branch files are written as plain code spans rather than links, since those files do not exist on master. The sync guidance is the part worth reading twice. Commit reachability does not prove a sync landed -- `git log master ^<branch>` coming back empty only proves the commits are ancestors, and a conflict resolution can discard master's side while leaving the merge commit perfectly intact. The file says to compare content instead. That distinction was not theoretical: checking it this way on the port branches turned up changes that had gone missing despite a clean-looking history. Documentation only; no code or build changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: consolidate .github/copilot-instructions.md into AGENTS.md Two files were doing the same job. `.github/copilot-instructions.md` held the architecture, codegen and convention guidance; `AGENTS.md` held the branch model and the rules for syncing. Agents read both, with no obvious precedence between them, and nothing said which one a new rule belonged in. `AGENTS.md` is the better survivor. It is the cross-tool convention, GitHub added support for it to Copilot in August 2025, and it is read by the coding agent, VS Code and the CLI alike, so folding one into the other loses no coverage. It also sits at the repository root next to `CONTRIBUTING.md`, which is where someone looks first. The merge is content-preserving: module map, directory layout, the code generation pipeline, the transformer/estimator pattern and its conventions, the cognitive service traits, file headers, build commands, Python and scalastyle rules, testing layout, CI/CD, and the numbered list of common mistakes all move across intact. Two things are deliberately different in the merged file. The first is that it carries no version numbers. The deleted file said "Spark 3.5.0, Scala 2.12.17" and pointed at `target/scala-2.12/generated/src/python/`, which was true on master and wrong on both Spark 4 branches -- where it directed agents at a generated-output directory that does not exist. That is not a typo anyone forgot to fix; it is the predictable result of restating in prose a fact that lives in `build.sbt`. The merged file names `build.sbt` and `environment.yml` as the source of truth, writes the generated path as `target/scala-<binary-version>/`, and defers per-branch specifics to `AGENTS_<branch>.md`. Being version-free is also what lets this file stay byte-identical on every branch, so syncing it is a no-op rather than a recurring conflict. The second is two additions earned the hard way rather than copied across: a short section on hand-written `__init__.py` files explaining that re-listing generated classes *narrows* the public API instead of extending it -- the exact defect that broke `PythonTests core` and seven website samples -- and a note that the `/azp run` comment does not trigger on every branch, so an absent pipeline run is not evidence that CI is broken. Deleted on `master`, `spark4.0` and `spark4.1` in the same change, so that no branch inherits a file the others have dropped and future syncs stay clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: sharpen AGENTS.md title, boundaries, and the version rule Addresses the review note that the "no version numbers" rule contradicted the branch table directly above it. The table has to name the Spark line each branch targets, and that text is identical on every branch, so it was never the problem. Reword the rule to target what a branch would actually have to edit -- specific Spark, Scala, Java and Python versions, and Scala-versioned paths -- and say plainly that naming the branches is fine. Retitle from "AGENTS.md" to name the project. Agents frequently receive the contents without the path, and a file that does not identify itself is hard to place. Add the two sections the current guidance for agent context files calls for that this file was missing: - Boundaries, split into never / ask first / safe. Most of these rules were already here but scattered across sections an agent reads late, if at all. - Secrets and credentials, stating that a skipped test is the correct local outcome. Otherwise the obvious "fix" for a skip is to inline a key. Also fold the pull request conventions into the CI section so they are not split across two places, and record that comparing before-and-after test results is what distinguishes a real fix from a coincidence. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: SynapseML CI <synapseml-ci@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Brings in #2648, which consolidates the agent instructions into a root AGENTS.md. AGENTS.md is resolved to master's copy verbatim: it is meant to be byte-identical on every branch so that syncing it is a no-op rather than a recurring conflict. Everything version-specific for this branch stays in AGENTS_spark4.1.md, which master does not carry. The rename/delete conflict is expected. This branch had already folded .github/copilot-instructions.md into AGENTS.md; master has now done the same, so the file is correctly gone on both sides. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The comment named target/scala-2.12/coverage-report/, which is right on master and wrong here -- this branch builds against a different Scala binary version. The glob underneath it was already version-agnostic, so only the comment was misleading, which is the worst kind of stale: it reads as authoritative while pointing at a directory that does not exist on this branch. Write the path with a placeholder instead, and say why the glob is loose, so the same text is correct on every branch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
Author
Re-validation after the docs merge: greenBuild 231447057
Why
|
Rana Singh (ranadeepsingh)
pushed a commit
that referenced
this pull request
Aug 17, 2026
## Summary Refactor the Spark 4 branch references into a shared template plus concise spark4.0 and spark4.1 overlays derived from PRs 2646 and 2645. Preserve their core toolchain, codegen, R, Databricks, Fabric, CI, failure-triage, and porting differences. ## Prompting Intent The engineer asked to validate that the new branch-specific skills are templatized versions of PRs 2645 and 2646, while retaining the important differences so those sync PRs can be updated later. ## Linked Sources - Pull request: #2649 - Spark 4.1 sync/context: #2645 - Spark 4.0 sync/context: #2646 ## Rationale Extract shared Spark 4 responsibilities once, keep branch-only facts in small overlays, and add a reusable branch-reference template. This retains the operational knowledge from the long branch manuals without duplicating hundreds of lines or treating snapshot values as permanent truth. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rana Singh (ranadeepsingh)
pushed a commit
that referenced
this pull request
Aug 17, 2026
## Summary Rename the skill to synapseml-branches, normalize references to branch-spark3p5/branch-spark4p0/branch-spark4p1, and explicitly map both master and spark3.5 to the Spark 3.5 context while preserving their different sync policies. ## Prompting Intent The engineer requested branch-oriented filenames without dots, compliant Agent Skills naming/frontmatter, and a clear mapping in the skill showing that master currently uses the Spark 3.5 baseline. ## Linked Sources - Pull request: #2649 - Spark 4.1 context source: #2645 - Spark 4.0 context source: #2646 ## Rationale Use predictable branch-<runtime> filenames, pluralize the routing skill because it covers multiple targets, and share one Spark 3.5 reference while distinguishing canonical master development from the shared spark3.5 release branch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
The ADO pull-request trigger filter was widened on 2026-08-17, so the guidance saying /azp run does nothing here is now wrong -- and wrong in the costly direction: an agent reading it would skip the comment and hand-queue every build, which is slower and easy to get subtly wrong by queueing the branch ref instead of the merge ref. Validated rather than assumed. Commenting /azp run on PR #2645 produced build 231455958 with reason=pullRequest and requestedFor=GitHub, where every build I queued by hand before it recorded reason=manual under my own name. That field is the discriminator worth writing down: a hand-queued build and a trigger-driven one are otherwise indistinguishable, and "I commented" is not evidence a build exists. Kept the UI-overrides-YAML explanation but demoted it from cause to diagnostic. It is still the first thing to check when a comment yields no build, because a UI-defined trigger silently ignores targets pipeline.yaml lists, and that failure mode is invisible -- no error, no build, no feedback. The merge-ref fallback stays for that case. The shared AGENTS.md line now says to confirm a build actually queued instead of claiming the comment does not work on every branch. That phrasing stays true regardless of how the trigger is configured later, so it will not rot the next time the filter changes. Verified AGENTS.md is byte-identical to the spark4.0 branch copy. Docs only; no code, so in-flight build 231455958 remains valid evidence for this head. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
* docs: make the agent guide concise and actionable ## Summary`nReduce AGENTS.md from a long duplicated reference to a short repository decision guide with direct links to authoritative build, setup, review, codegen, testing, branch, and CI sources. ## Prompting Intent`nEnsure every instruction in the new AGENTS.md is helpful and terse, and replace copied detail with links and pointers to what coding agents actually need. ## Linked Sources`n- Pull request: https://github.com/microsoft/SynapseML/pull/2648`n- Contributor guide: CONTRIBUTING.md`n- Repository skills: .github/skills/`n- Build sources: build.sbt, environment.yml, pyproject.toml, pipeline.yaml ## Rationale`nAgents need high-signal boundaries and navigation, not a second copy of implementation examples. Keeping durable rules while linking to source files reduces staleness, token cost, and branch-sync conflicts without losing actionable guidance. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: stop naming Spark versions in the two shared files AGENTS.md and CONTRIBUTING.md are required to be identical on every branch, but both enumerated the port branches by name -- three places in AGENTS.md and one in CONTRIBUTING.md. That makes adding a port branch an edit to a file that must then be re-synced everywhere, which is the exact churn the identical-everywhere rule exists to avoid. Describe the pattern instead of listing instances: port branches are named spark<version>, and `git branch -r` is the authoritative list. Same reason version numbers are read from build.sbt rather than restated -- an enumeration in prose is a copy that goes stale silently. State the boundary explicitly in "Keep this file useful", since it was implied rather than written: these two files may not name a Spark, Scala, Java or Python version, or a path containing one, while README, the website and module docs are free to be branch- and version-specific because nothing requires those to match across branches. Verified: no version-like token remains in either file, and every relative link still resolves. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: add a repeatable merge-readiness workflow ## Summary`nAdd a project skill for taking SynapseML issues and stale PRs through an evidence-based merge-readiness loop, with reusable readiness gates, CI triage, Spark performance guidance, and a live GitHub snapshot script. Link it tersely from AGENTS.md. ## Prompting Intent`nCapture the recurring 5/5 or 200%-ready development workflow from prior SynapseML sessions so agents consistently rebase, resolve all active and suppressed feedback, prove user value, add regression and end-to-end tests, protect compatibility and Spark performance, and iterate full CI to green. ## Linked Sources`n- Follow-up PR: https://github.com/microsoft/SynapseML/pull/2649`n- Original agent guide PR: https://github.com/microsoft/SynapseML/pull/2648`n- Project review skill: .github/skills/code-review/SKILL.md`n- Project local setup skill: .github/skills/synapseml-local-setup/SKILL.md`n- Agent Skills specification: https://agentskills.io/specification ## Rationale`nA dedicated skill provides repeatable progressive disclosure without bloating AGENTS.md. The workflow encodes evidence gates learned from real failures: stale targets, discarded conflict content, suppressed comments, helper-only tests, false-green skips, infrastructure failures, compatibility breaks, unshipped artifacts, and unmeasured Spark performance claims. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: correct two CI claims in the merge-ready skill from measured evidence Two statements in the new skill would send an agent down a path I measured as wrong while getting the spark4.0 and spark4.1 sync PRs green. "Push the exact validated head and comment /azp run" does not work for PRs targeting the port branches. The Azure Pipelines definition's pull-request trigger is defined in the pipeline UI with a branch filter of +master, and a UI-defined trigger overrides the pr: block in pipeline.yaml entirely, so the YAML listing the port branches has no effect. /azp run on such a PR silently does nothing -- no build queues and no error is reported -- which reads as "CI triggered" and then as "CI pending" forever. Replace it with: trigger, then confirm a build actually queued, and queue against refs/pull/N/merge when the target is not covered. Cite the build ID, since a comment is not evidence a build ran. CI triage described the four failure categories but not how to read a job result, and the mechanics are not binary. A filter of result -eq "succeeded" reports phantom failures, because succeededWithIssues is a normal outcome when a non-gating task -- usually dependency caching or TLS -- warns while every test passes. I hit exactly this and briefly reported a passing job as failed. It also cuts the other way: succeededWithIssues on a task that runs or publishes tests is a real failure. Add a section saying to identify the warning task and read published test results rather than trusting the badge in either direction. Same section records the harder lesson: compare per-test outcomes across builds. A fix that changes nothing leaves the same tests failing the same way, and a job-level summary hides that. Two changes I believed were fixes turned out to be placebos under that comparison. Also drop the remaining version numbers, matching the previous commit -- "spark4.x" becomes "spark<version>" and "Spark 4.1 compatibility" becomes "port-branch compatibility", so adding a branch does not require editing these files. Verified: no version token remains in the skill, and both sibling skill links resolve. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: paginate the readiness script and harden its input handling Review feedback on the script, verified case by case rather than applied wholesale. A readiness tool that under-reports is worse than no tool, so the truncation issues were the ones worth fixing properly. Pagination. The GraphQL query capped reviewThreads at 100, per-thread comments at 20 and reviews at 50, with no pageInfo, so a large PR would silently report fewer unresolved threads than it has -- reading as "review is clean" when it is not. That is the false-green pattern the skill itself tells you to reject. Both connections are now fully paginated through a shared helper, and the emitted JSON carries a completeness object with page counts and any thread whose comments were still truncated, so an incomplete snapshot is visible instead of silent. The loop throws if a page claims hasNextPage without returning a cursor rather than spinning forever. Suppressed-comment detection was an exact case-sensitive match on "Suppressed comments", which silently drops the signal the script exists to surface if GitHub varies the wording. Now a case-insensitive match. Repo validation accepted "owner/name/extra", because Split("/", 2) always yields two parts when a slash is present. Now requires exactly two non-empty segments and reports the offending value. Renamed the helper's local $args to $ghArgs; $args is an automatic variable inside a function and assigning to it is a trap for later edits. Two review points I did not treat as defects: statusCheckRollup was reported as an object whose checks live under .contexts. That is the GraphQL shape, but `gh pr view --json` flattens it. Measured: it returns Object[] of 13 CheckRun entries with name/status/conclusion. Verified the existing filter against a PR that genuinely fails and it returned exactly [Review Dependencies], matching `gh pr checks`. Left as is -- an all-green PR would not have proven this either way. The unescaped base ref in the compare URL was reported as breaking on branches containing "/". It does not: the API accepts sync/spark4.1-with-master-2 unescaped and returns the same result as the encoded form. Kept the encoding anyway as defensive, but it fixes no observed failure. Verified: parses clean, and runs against three PRs producing counts that match independent REST pagination. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: rename merge readiness skill to SynapseML PR loop ## Summary Rename the reusable skill and its directory from synapseml-merge-ready to synapseml-pr-loop, then update the AGENTS.md activation link. ## Prompting Intent The engineer asked for a clearer, more understandable skill name that describes the recurring SynapseML pull-request remediation loop. ## Linked Sources - Pull request: #2649 - Follow-up context: #2648 ## Rationale SynapseML PR loop communicates the skill's repeatable issue/PR workflow more directly than the outcome-oriented merge-ready name while preserving all existing progressive-disclosure content and behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: complete the SynapseML PR loop guidance ## Summary Add the remaining concise instructions needed for repeatable PR remediation: audit historical review feedback, verify published artifacts ship the capability, update public documentation without editing generated files, and safely validate external services. ## Prompting Intent The engineer asked for the SynapseML PR loop to contain all recurring instructions and checklists so future merge-readiness requests do not require repeated guidance, while keeping the skill terse. ## Linked Sources - Pull request: #2649 - Original guide: #2648 ## Rationale Keep the main workflow at 114 lines and place detailed exit, CI, and Spark checks in focused references. The added bullets close material workflow gaps without duplicating those references. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: add PR lifecycle gates to the SynapseML loop ## Summary Require the PR loop to inspect recently closed related work, remediate or close follow-ups, reconcile linked issues after merges, and keep PR titles and descriptions current and human-readable. ## Prompting Intent The engineer asked the reusable skill to complete lifecycle action items around closed PRs and issues, including rebasing valuable follow-ups, closing superseded work, and maintaining clear PR metadata without making the skill verbose. ## Linked Sources - Pull request: #2649 - Original guide: #2648 ## Rationale Add short workflow instructions plus matching exit gates so lifecycle cleanup and reviewer-facing metadata are enforced, while keeping deeper readiness details in the existing references. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: add reusable SynapseML branch context ## Summary Add a concise synapseml-branch skill with focused master, spark3.5, spark4.0, spark4.1, and fallback references. Wire AGENTS.md and the PR loop to resolve context from the PR base branch and add historical false-confidence gates. ## Prompting Intent The engineer asked to mine prior SynapseML PRs and Copilot sessions for durable lessons, make agent responsibilities and repeated checks explicit, provide branch-specific shared context, and ensure agents fall back to it without bloating the main PR loop. ## Linked Sources - Pull request: #2649 - Branch CI coverage: #2644 - Spark 4.1 synchronization: #2617 - Compatibility replay fix: #2611 - Compatibility identity fix: #2608 - Orphaned test-suite coverage: #2622 ## Rationale Use .github/skills because branch-local .agents guidance identifies it as authoritative. Keep common decision logic in one small skill, isolate volatile branch facts in references, derive context from the PR base rather than feature-branch names, and require live build/CI verification so historical notes cannot become stale authority. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: harden PR readiness snapshots ## Summary Handle GraphQL errors and missing data explicitly, reject non-advancing cursors, classify stale checks as failures, emit a stable JSON array shape, and warn that review content must remain local or be redacted. ## Prompting Intent The engineer asked agents to double- and triple-check the reusable PR loop. The loop's own final snapshot surfaced active and suppressed Copilot findings that needed to be fixed before the skill could be considered reliable. ## Linked Sources - Pull request: #2649 - GraphQL error review: #2649 (comment) - Stale-check review: #2649 (comment) - Snapshot privacy review: #2649 (comment) ## Rationale A readiness collector must fail closed and preserve a stable machine-readable contract. Explicit errors, cursor progress checks, stale-signal blocking, array output, and local/redacted evidence prevent false-green or accidental disclosure outcomes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: template Spark branch context from sync PRs ## Summary Refactor the Spark 4 branch references into a shared template plus concise spark4.0 and spark4.1 overlays derived from PRs 2646 and 2645. Preserve their core toolchain, codegen, R, Databricks, Fabric, CI, failure-triage, and porting differences. ## Prompting Intent The engineer asked to validate that the new branch-specific skills are templatized versions of PRs 2645 and 2646, while retaining the important differences so those sync PRs can be updated later. ## Linked Sources - Pull request: #2649 - Spark 4.1 sync/context: #2645 - Spark 4.0 sync/context: #2646 ## Rationale Extract shared Spark 4 responsibilities once, keep branch-only facts in small overlays, and add a reusable branch-reference template. This retains the operational knowledge from the long branch manuals without duplicating hundreds of lines or treating snapshot values as permanent truth. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: normalize SynapseML branch reference names ## Summary Rename the skill to synapseml-branches, normalize references to branch-spark3p5/branch-spark4p0/branch-spark4p1, and explicitly map both master and spark3.5 to the Spark 3.5 context while preserving their different sync policies. ## Prompting Intent The engineer requested branch-oriented filenames without dots, compliant Agent Skills naming/frontmatter, and a clear mapping in the skill showing that master currently uses the Spark 3.5 baseline. ## Linked Sources - Pull request: #2649 - Spark 4.1 context source: #2645 - Spark 4.0 context source: #2646 ## Rationale Use predictable branch-<runtime> filenames, pluralize the routing skill because it covers multiple targets, and share one Spark 3.5 reference while distinguishing canonical master development from the shared spark3.5 release branch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: /azp run now queues port-branch builds; correct the guidance I documented the opposite earlier today, and the pipeline trigger has since been fixed, so the guidance is now wrong in the direction that costs the most: an agent reading it would skip /azp run entirely and hand-queue every build. Measured before changing the text. Commenting /azp run on the two port-branch PRs produced builds 231455958 and 231455959, both recording reason=pullRequest and requestedFor=GitHub, where every build queued by hand beforehand recorded reason=manual under a personal account. The definition's pullRequest trigger filter now reads +master | +spark3.5 | +spark4.0 | +spark4.1. That reason field is the part worth writing down. A trigger-driven build and a hand-queued one are otherwise indistinguishable in the UI, so it is the cheapest way to answer "did my comment actually do anything" -- and the skill already insists a comment is not evidence a build ran. The UI-overrides-YAML explanation stays, demoted from cause to diagnostic: it remains the first thing to check when a comment produces no build, because that failure mode is completely silent -- no error, no build, no feedback anywhere. The merge-ref fallback stays for that case, along with the warning against queueing refs/heads/<branch>, which validates the branch rather than the merge result. The branch reference previously hedged with "historically did not queue ... verify live behavior". Now that it has been verified, it states the current filter and the date, so the next reader does not have to re-derive it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: SynapseML CI <synapseml-ci@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ills Brings in #2649, which rewrote AGENTS.md and CONTRIBUTING.md and added the synapseml-branches and synapseml-pr-loop skills. AGENTS.md conflicted because both sides rewrote it. Resolved by taking master wholesale rather than merging line by line: the file is required to stay byte-identical on every branch, so any blended result would be wrong by construction. Verified by blob hash after resolving, not by reading the diff. The spark4.0 sync resolved the same conflict the same way, and both branches now carry the identical blob. Nothing was lost in that resolution. The branch material this file used to carry now lives in the synapseml-branches skill, which arrives in this merge. synapseml-local-setup still differs from master, and should. #2649 never touched it; the divergence predates this merge and is version-driven, so taking master's side would have handed this branch a toolchain it does not use. AGENTS_spark4.1.md is untouched. Master's new model routes branch facts to the branch skill, and branch-spark4p1.md overlaps this file, so consolidation is a content decision and is deliberately kept out of a sync commit. Docs only. No Scala, Python, build, or pipeline changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
What this does
Merges
masterintospark4.1. The branch was 67 commits / 305 files behind — last synced on 2026-08-10 by #2617 — so it was missing every fix merged since, including the Spark 4 correctness fix in #2643.master@eb76ff6bd0→ merged intospark4.1@1d1d0dadf8. 144 files changed, +14363/−635. 33 files conflicted (~86 hunks); each was resolved individually rather than by a blanket--ours/--theirs.Why this matters
#2643 fixed model metadata read/write to go through
SparkSessioninstead of RDD APIs, which is what makes SynapseML work on Spark Connect and Databricks Unity Catalog standard/serverless clusters. That fix landed onmasteronly — so the branch that exists to support Spark 4 was still running the code path Spark 4 users hit. This sync closes that gap.Resolution rule
Two things made this decidable rather than a judgement call:
git log <merge-base>..spark4.1 -- <file>separates files touched only by the previous sync (take master) from files touched by a real Spark 4 upgrade commit (preserve).ManualInitPackageFoldersis base=0, master=0, spark4.1=2 ⇒ spark4.1 added it ⇒ keep ours.license_expressionis base=0, master=1, spark4.1=0 ⇒ master added it ⇒ take theirs.Spark 4 commits preserved:
b76c391be4(Spark 4.0 / Java 17 / Scala 2.13 / Python 3.12) and791af3237d(Spark 4.1.1 / Scala 2.13.17 / Python 3.13).Kept from spark4.1 (Spark 4 identity)
pipeline.yamlSPARK_VERSION=4.1.1, Java-17-safeSBT_OPTSSBT_OPTScarries-XX:+UseConcMarkSweepGC -XX:+CMSClassUnloadingEnabled, removed in Java 14+ — they abort JVM startup on Java 17pipeline.yamlcondition: falseon FabricE2Eenvironment.ymlpip=21.3,numpy==1.26.4(no 3.13 wheels),torch==2.1.2.github/skills/*(5 files)pr-validation.yml17java-version: 17had already auto-merged; only the label conflictedDatabricksUtilities.scalaDatabricksCPUStreamingTestsneedsStreamingNotebooks; dropping it breaks compilationOpenAIChatCompletion.scalascala.collection.Seq+.toSeqis required in 2.13, where bareSeqis immutableTaken from master
SparkSession-based model metadata I/O, plus theSynapseMLLoggingandSerializerfollow-ups. All four fixes confirmed present on the merged tree.NetworkManager.scala— theNetworkManagerSocketSupportrefactor anddriverUnreachableException(new fileNetworkManagerSocketSupport.scalacame in cleanly).pipeline.yaml— theInternalCompatjob, the release-compat prerequisite replay,publish_coverage_ado, andcontinueOnError: trueon "Load Codecov token".Deliberate non-adoptions
numpy==1.26.4— master pins it; 1.26.4 publishes no Python 3.13 wheels. An inline guard comment now sits abovenumpyinenvironment.ymlso a future sync doesn't silently re-apply the pin.test_pipeline_yaml.py— hunk 1 taken from master dropped theBUILD_SBT/PLUGINS_SBTconstants, which are still used; they were re-added by hand. The brittleassert 'branches: [ "master", "spark4.1" ]' in workflowstring match was replaced with a real YAML-parsing helper,_pr_validation_branches()(which also handles YAML 1.1 folding a bareon:key to booleanTrue). The FabricE2E assertion stayscondition is Falseto match this branch.One test adapted — and why it is not a regression
EnsembleByKeySuite→ "no active session should expose the documented case-resolution limitation" fails on Spark 4.1 as written.Both
EnsembleByKey.scalaandEnsembleByKeySuite.scalaare byte-identical to master on this branch (git diff ms/master --is empty), so the merge did not touch them. Measured on Spark 4.1.1:transformer.transformSchema(input.schema)key,id,score,featureskey,id,score,features— unchangedassembler.transformSchema(thatSchema)FEATURES does not existvectorthatSchema("FEATURES")FIELD_NOT_FOUND— unchangedSpark 3.5 resolved
VectorAssemblerinput columns with a case-sensitiveStructTypelookup, so it disagreed with the case-insensitive fallbackEnsembleByKeyapplies when no session is active, and the pipeline was rejected. Spark 4 routes that lookup throughSQLConf.get.resolver, which applies the same session-less fallback — so the two now agree and the pipeline builds. SynapseML's own behaviour is identical; only the downstream Spark consequence moved, and it moved in the safer direction.The assertion is retargeted at the transformed schema — the stable contract across both versions, and what the test set out to document: the fallback really did drop
FEATURES. The replacement passes on Spark 3.5 as well, somasterandspark4.1can carry the identical test and this does not become a recurring merge conflict.Validation
Every row below was executed, not inferred — Scala 2.13.17 / Spark 4.1.1 / Java 17.
sbt compile(all 6 modules)sbt Test/compilesbt scalastyle Test/scalastyle(all modules)core/testOnlyserialize + logging + train + stagespytest tools/ci/tests/test_pipeline_yaml.pyFollow-ups (not in this PR)
spark4.0to the GitHub Actions and ADO branch filters —spark4.0currently runs no CI at all.spark4.0needs the same sync; it is 547 files / ~4.5 months behind and should land after ci: run PR validation on the spark3.5, spark4.0 and spark4.1 branches #2644 so it isn't merged blind.spark4.0is deliberately still absent from theReleaseBranchCompatmatrix — enabling it before its sync lands would make it permanently red.EnsembleByKeySuitechange should be applied tomasterso the two branches converge.Updates since the description above
Correction to this description
The table above justifies
condition: falseonFabricE2Ewith "no Spark 4.1 Fabric runtime exists yet". That is no longer true. Fabric Runtime 2.0 has since reached general availability on Apache Spark 4.1 (Scala 2.13, Python 3.13, Java 21, Delta 4.x), which makes this the one Spark 4 branch Fabric can actually host, and makes the disabled job real lost coverage rather than an unavoidable skip.It is not re-enabled in this PR, deliberately. Checking what that would take turned up a concrete blocker beyond the pipeline condition:
core/src/test/scala/.../fabric/FabricOperations.scalahardcodes'SparkVersion': '3.5'in the workspace-creation payload, so flipping the condition alone would provision a 3.5 workspace and fail. Enabling it needs that change, the restored condition, and a Fabric capacity in thesempy-integration-regionable to provision Runtime 2.0 workspaces — and the only real test of the last one is a live pipeline run. That belongs in its own PR where the pipeline run is the test, rather than inside a sync PR where a Fabric provisioning failure would block an unrelated merge.The reasoning, the blocker, and the exact steps are recorded in
AGENTS_spark4.1.md(added here) so this is not rediscovered later. Note thatspark4.0reaches the opposite conclusion correctly — Fabric has no Spark 4.0 runtime at all, so it stays disabled there permanently.Additional fix
OpenAIPromptoverrides via MRO instead of a hardcoded class namepyInternalWrappermakes codegen emitclass _OpenAIPrompt, sosuper(OpenAIPrompt, self)raisesNameError— that name does not exist inside the generated moduleDocumentation
Adds
AGENTS_spark4.1.mdrecording every divergence on this branch and why, plus the sharedAGENTS.md/CONTRIBUTING.mdpair (byte-identical acrossmaster,spark4.0andspark4.1— verified by git blob hash; proposed for master in #2648).The branch file inverts the framing used on
spark4.0. That branch's file says "check spark4.1 first"; this one says "ask whether spark4.0 needs this too", because this branch descends from that one's upgrade commit and is the more actively maintained of the pair. It therefore lists the four things that are genuinely 4.1-only and states that everything else is a back-port candidate.Two of those four were measured rather than assumed, by running the same probe against real Spark 4.0.1 and 4.1.1 installs:
ImageTransformer.toNDArrayusingnp.frombufferbytesfor aBinaryTypecolumn where 4.0 returnsbytearray.np.asarrayhandlesbytearraybut raisesValueErroronbytesspark_utilsusingspark.createDataFrame(rdd, schema)rdd.toDF(schema)works on both 4.0.1 and 4.1.1df.rdd.zipWithIndex()is still an RDD callAdding these files also surfaced a bug:
.gitignorehas ignoredAGENTS.mdand.agents/sinceb76c391be4, the Spark 4.0 upgrade commit this branch inherits from. Master ignores neither. The failure mode is the quiet kind —git add AGENTS.mdprints a hint and exits zero, so the file is simply absent from the commit..agents/is worse: it holds tracked repo content, and a probe confirmed any new file under it is silently dropped. Both rules removed.Completeness audit
Reachability was not treated as sufficient evidence that the sync landed — an empty
git log master ^branchonly proves the commits are ancestors, and a conflict resolution can discard master's side while leaving the merge commit intact. Every file master changed since the merge base was compared line by line:All 71 were either false positives where master's intent survived a refactor, or deliberate documented adaptations — the
environment.ymlpins for Python 3.13 (whose comments now state the sync policy explicitly, e.g. "Intentionally unpinned: master pins numpy==1.26.4, which has no Python 3.13 wheels. Do not adopt master's pin when syncing."), and the CMS GC flags removed fromSBT_OPTSbecause they are invalid on Java 17.The same audit run against
spark4.0did find two real gaps, both fixed in #2646.