ci: skip notebook E2E for provably unrelated PR changes by ranadeepsingh · Pull Request #2736 · microsoft/SynapseML

Rana Singh (ranadeepsingh) · 2026-09-21T20:49:50Z

What changes are proposed?

Skip notebook E2E only when a PR changes audited inputs that those notebooks do
not consume. This can avoid five Databricks CPU jobs, one GPU job, and Fabric E2E.
Scheduled, manual, push, and tag builds retain all currently enabled tests.

Allowed inputs are module Python/R tests, the R test runner, website files,
Quick Examples Markdown, and explicit contributor/agent/review documentation.
Production code, Scala tests, shared fixtures, notebooks, unknown paths, build
definitions, dependencies, and CI tooling always retain every E2E job.

The detector checks the exact queued PR merge and both parents, then diffs
against the immutable first parent. It handles both sides of renames and fails
open for missing history/metadata, empty diffs, Git errors, symlinks, submodules,
and unknown inputs. Missing Azure output variables also mean run. An unexpected
detector crash fails visibly. fullTests=true bypasses PR selection.

This replaces the existing broad Databricks exemptions with stricter rules and
adds Fabric selection. It does not preserve the old CPU-only assumption for
production module changes.

What deliberately remains unchanged?

All 40 JVM, 7 Python, 6 R, and 1 website coverage-producing legs still run.
Review found that skipping them would prevent Codecov's fixed 54-upload
threshold from being reached. This PR does not weaken coverage reporting to
claim larger savings.

Style, compilation/cache preparation, Docker, publishing, compatibility,
explicit family-disable parameters, and the Fabric fork credential restriction
keep their existing gates. Previously disabled suites are not re-enabled.

An independent CIHelpers job now runs the CI regression suite. Dependency
installation retries; failed assertions do not. Its failure makes CI red without
preventing product tests from running.

How is this patch tested?

288 CI helper tests passed under Linux, including 121 selection/wiring cases.
Real Git fixtures cover moving targets, shallow history, rename/delete/type
changes, metadata mismatch, empty diffs, manual/scheduled runs, and exact
output-variable contracts.
A coverage invariant discovers the upload producers, verifies both configured
54-upload thresholds, and prevents those jobs from becoming path-gated.
Black 22.3.0 and whitespace checks passed.
GPT and Opus review artifacts include the coverage finding, all resolutions,
and an independent post-fix verification. The advertised Gemini family
returned HTTP 400, so the three-family gauntlet is not claimed complete.

Azure build 237012340
is running for head 33225c28311d192d9fbab940a1fcf6cf5cce6eea. The queued PR
merge's second parent was verified against that head. Because this PR changes
CI tooling, it must run all E2E jobs itself. Local tests do not prove that Azure
has scheduled a selectively filtered representative PR; that remains a rollout
gate. No wall-clock savings are claimed without service evidence.

Previous-head CI evidence

Azure build 236973898
was triggered by /azp run with reason=pullRequest, not a manual branch build.
Its merge commit has the reviewed head 3edd15aa0a052b7a1bcb615421649c9d1c4ed087
as its second parent.

The real detector step completed without fallback and emitted true for
Databricks CPU, GPU, and Fabric, as this CI-changing PR requires. The independent
CIHelpers job passed all 288 tests in 12.93 seconds on the hosted agent.
The full pipeline ended with a failed UnitTests exploratory job. That failure
has not been classified in this documentation-only follow-up. The existing fork
restriction can still skip Fabric even when the selector requests it. Copilot
review of that head reported no findings and requested final human review of
the CI change.

Review artifact organization

Review reports now live in reviews/pr-2736/.
AGENTS.md now requires reviews/pr-<pr_number>/, with that output directory
passed explicitly to review tools. Drafts stay in the session workspace until
the PR number exists, so reports are committed to the correct directory first.
Attempt/round/model filenames, reviewed commit SHAs, and resolution evidence
remain available for debugging. The two existing reports moved without content
changes. This follow-up changes no test behavior or CI conditions.

Scope

Targets master from an isolated worktree. Independent of the companion
obsolete-test cleanup #2735. No production API, dependency pin, release, or
GitHub workflow changes.

## Summary Use a positive allowlist for Databricks CPU/GPU and Fabric notebook E2E. Preserve full scheduled/manual builds and all 54 coverage-producing test legs. Run CI helper regressions independently of product build prerequisites. ## Prompting Intent Investigate safe CI efficiencies in a separate PR from obsolete-test cleanup. Only omit per-PR tests when changed inputs are demonstrably unrelated, and retain complete scheduled validation. ## Linked Sources - User request in this session for two independent test-efficiency PRs. - microsoft#2582 - pipeline.yaml and codecov.yaml at upstream master 714d365. - CodegenConfig.scala, TestGen.scala, CodegenPlugin.scala and website/doctest.py. - Committed GPT/Opus review artifacts and resolution notes under reviews/. ## Rationale Runtime/build/shared changes remain full because module names alone do not prove independence. Comparing the queued merge against its immutable first parent avoids false skips when the target advances. Rename expansion, file-mode checks, missing-output semantics and explicit full-run overrides fail safely. Do not filter coverage producers: their 54-upload contract is independent of test execution impact. Review caught that coupling before publication. The six-round three-family gauntlet was unavailable because Gemini model launches failed; independent working-model reviews and local tests are recorded without claiming that missing review gate passed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

azure-pipelines · 2026-09-21T20:50:00Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

github-actions · 2026-09-21T20:50:04Z

Hey Rana Singh (@ranadeepsingh) 👋!
Thank you so much for contributing to our repository 🙌.
Someone from SynapseML Team will be reviewing this pull request soon.

We use semantic commit messages to streamline the release process.
Before your pull request can be merged, you should make sure your first commit and PR title start with a semantic prefix.
This helps us to create release messages and credit you for your hard work!

Examples of commit messages with semantic prefixes:

fix: Fix LightGBM crashes with empty partitions
feat: Make HTTP on Spark back-offs configurable
docs: Update Spark Serving usage
build: Add codecov support
perf: improve LightGBM memory usage
refactor: make python code generation rely on classes
style: Remove nulls from CNTKModel
test: Add test coverage for CNTKModel

To test your commit locally, please follow our guild on building from source.
Check out the developer guide for additional guidance on testing your change.

Rana Singh (ranadeepsingh) · 2026-09-21T20:50:33Z

/azp run

azure-pipelines · 2026-09-21T20:50:46Z

Azure Pipelines: Successfully started running 1 pipeline(s).

Copilot

Copilot review overview

🔵 Needs a closer look

Broad CI selection and pipeline changes require final human review.

Review effort: Lite
Findings: None

What changed in this PR

This PR adds conservative, fail-open PR path selection for Databricks and Fabric notebook E2E jobs while preserving coverage-producing tests.

Changes:

Adds merge-aware impact detection and CI-helper validation.
Updates pipeline wiring and selector tests.
Removes the obsolete Databricks detector.
Documents selection rules and review evidence.

File	Description
`tools/ci/tests/test_pipeline_yaml.py`	Updates pipeline wiring assertions.
`tools/ci/tests/test_e2e_impact.py`	Tests selector behavior and pipeline invariants.
`tools/ci/tests/test_databricks_impact.py`	Removes obsolete detector tests.
`tools/ci/README.md`	Documents selection rules and limitations.
`tools/ci/e2e_impact.py`	Implements conservative PR impact detection.
`tools/ci/databricks_impact.py`	Removes the obsolete detector.
`reviews/task-ci-test-selection-attempt-1-review-robustness-claude-opus-5.md`	Records robustness review findings and resolutions.
`reviews/task-ci-test-selection-attempt-1-review-1-gpt-6-astra.md`	Records independent review history.
`pipeline.yaml`	Adds helper validation and selective E2E wiring.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

## Summary Document reviews/pr-<pr_number>/ in AGENTS.md and move this PR's two review reports into reviews/pr-2736 without changing their contents. ## Prompting Intent The user requested numbered review directories to avoid later relocation and make historical debugging easier, with the repository rule included in PR2736. ## Linked Sources - microsoft#2736 - User instruction: reviews should be in reviews/pr-<pr_number>. - AGENTS.md Pull requests and CI guidance. ## Rationale Pass the numbered directory explicitly to review tools instead of relying on their flat-directory defaults. Keep drafts in the session workspace until the PR number exists, so their first committed location is correct. Preserve report filenames, reviewed commit identities, and resolution evidence. This change has no effect on runtime tests or CI conditions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

## Summary Move the existing review report into reviews/pr-2735 with byte-identical content. ## Prompting Intent The user requested that review artifacts live in reviews/pr-<pr_number>. The corresponding repository-wide instruction is included in companion PR2736. ## Linked Sources - microsoft#2735 - microsoft#2736 - User instruction: reviews should be in reviews/pr-<pr_number>. ## Rationale Group review evidence by the PR it describes while retaining its original filename and historical content. Leave the GeospatialCoreSuite tests unchanged; the user asked for their rationale, not for their removal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Rana Singh (ranadeepsingh) · 2026-09-22T02:07:23Z

/azp run

azure-pipelines · 2026-09-22T02:07:37Z

Azure Pipelines: Successfully started running 1 pipeline(s).

Copilot

Copilot review overview

🔵 Needs a closer look

The changes require final human review because they are too complex or risky for automated approval.

Review effort: Lite
Findings: None

…2735) * test: remove obsolete Python runner and duplicate categorical passes ## Summary Delete the unreferenced Scala 2.11 Python test runner and two identical categorical test repetitions. Clean up imports from the previously removed Maps Spatial suite and preserve retired-stage serialization contracts offline. ## Prompting Intent Audit outdated or deprecated SynapseML tests in an isolated worktree and remove only coverage proven obsolete or redundant, separately from CI path selection. ## Linked Sources - User request in this session for two independent test-efficiency PRs. - #2485 - project/CodegenPlugin.scala and core codegen test sources at 714d365. - https://learn.microsoft.com/en-us/azure/ai-services/document-intelligence/overview?view=doc-intel-4.0.0 - Committed test-retirement review and audit decisions under reviews/. ## Rationale The old runner has no tracked caller and points at a superseded Scala version and test namespace. The removed loop variables never influence either body. Retain all named tests and assertions, genuine metadata variants, and supported legacy API coverage. No new retired-service suite was established for removal. The public retired Maps stage remains serializable, so add credential-free round-trip coverage rather than deleting its compatibility tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: group test-retirement review under its PR number ## Summary Move the existing review report into reviews/pr-2735 with byte-identical content. ## Prompting Intent The user requested that review artifacts live in reviews/pr-<pr_number>. The corresponding repository-wide instruction is included in companion PR2736. ## Linked Sources - #2735 - #2736 - User instruction: reviews should be in reviews/pr-<pr_number>. ## Rationale Group review evidence by the PR it describes while retaining its original filename and historical content. Leave the GeospatialCoreSuite tests unchanged; the user asked for their rationale, not for their removal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

## Summary Merge the exact master-first polling update, deterministic regressions, documentation, and review records into this Spark sync. ## Prompting Intent The engineer requested 30-second deletion-confirmation intervals with a five-minute per-item wait budget. Preserve this port's runtime settings and carry portable changes through normal merge ancestry. ## Linked Sources - Master prerequisite: microsoft#2732 - Spark 4.0 sync: microsoft#2733 - Spark 4.1 sync: microsoft#2734 - Shared review-layout guidance only: microsoft#2736 - Review records: reviews/pr-2732/task-cleanup-polling-30s-attempt-1-review-*.md ## Rationale The same helper and tests run on each supported Scala baseline. This port passes core compile, test compile, production/test Scala style, and all 50 tracker/naming tests. Polling performs one immediate read and at most ten 30-second waits per item, with request time additional, and never resends DELETE. This merge preserves runtime pins and existing Fabric enablement. Previous-head Azure results do not validate this new head; fresh remote checks remain required. The Gemini-family review limitation stays visible. Copy only the six-line shared review-layout guidance from latest master so AGENTS.md and CONTRIBUTING.md stay identical across the task branches. The remaining newly merged CI changes are not imported without approval. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

## Summary Merge master 0a7fdaf into the existing Spark 4.1 sync. Import the portable test cleanup and conservative notebook selection changes while retaining the Spark 4.1 runtime and Python compatibility. ## Prompting Intent The user requested updating both existing Spark sync PRs with the latest master changes, maximizing master compatibility while honoring Spark- and Python-specific differences. ## Linked Sources - Sync PR: microsoft#2734 - Cleanup prerequisite: microsoft#2732 - Test retirement: microsoft#2735 - Notebook selection: microsoft#2736 - Master: microsoft@0a7fdaf - Review evidence: reviews/pr-2734/task-latest-master-20260922-attempt-1-review-*.md ## Rationale Use a normal merge to retain master's ancestry, including the landed cleanup fixes already present in this PR. Accept the obsolete Python runner deletion rather than preserving its irrelevant Scala-version edit. Keep Fabric E2E disabled and assert that port requirement in the imported selector wiring test. The selector itself matches master. Production code, dependency pins, runtime profiles, streaming scheduling, and existing Spark/Python adaptations remain unchanged from the previously validated port head. Strengthen the full-test override regression with valid skippable merge metadata and the non-PR bypass regression with a detection tripwire. Document the disabled Fabric boundary accurately. These review fixes do not change selector behavior or enable unsupported runtime jobs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

## Summary Merge master 0a7fdaf into the existing Spark 4.0 sync. Import the portable test cleanup and conservative notebook selection changes while retaining the Spark 4.0 runtime and Python compatibility. ## Prompting Intent The user requested updating both existing Spark sync PRs with the latest master changes, maximizing master compatibility while honoring Spark- and Python-specific differences. ## Linked Sources - Sync PR: microsoft#2733 - Cleanup prerequisite: microsoft#2732 - Test retirement: microsoft#2735 - Notebook selection: microsoft#2736 - Master: microsoft@0a7fdaf - Review evidence: reviews/pr-2733/task-latest-master-20260922-attempt-1-review-*.md ## Rationale Use a normal merge to retain master's ancestry, including the landed cleanup fixes already present in this PR. Accept the obsolete Python runner deletion rather than preserving its irrelevant Scala-version edit. Keep Fabric E2E disabled and assert that port requirement in the imported selector wiring test. The selector itself matches master. Production code, dependency pins, runtime profiles, streaming scheduling, and existing Spark/Python adaptations remain unchanged from the previously validated port head. Strengthen the full-test override regression with valid skippable merge metadata and the non-PR bypass regression with a detection tripwire. Document the disabled Fabric boundary accurately. These review fixes do not change selector behavior or enable unsupported runtime jobs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

## Summary Adapt the incoming E2E pipeline contract to the release PR's conditional checkout. Assert full history and tags for release requests, preserve the ordinary two-commit checkout, and locate the single named impact step without depending on the release admission guard's position. Record the bounded rebase review and current validation evidence without replacing prior reports. ## Prompting Intent The engineer asked to restart the publishing automation work, beginning by updating PR microsoft#2628 with the latest master branch and rerunning CI. Preserve the existing release changes and master fixes without publishing version 1.2.0, creating release tags, or bypassing approvals. ## Linked Sources - Release automation PR: microsoft#2628 - Integrated master: microsoft@681bd96 - Incoming E2E impact selection: microsoft#2736 - Repository rules: AGENTS.md - Pipeline contract: tools/ci/tests/test_e2e_impact.py - Rebase review: reviews/pr-2628/pr-2628-attempt-3-review-1-gpt-6-astra.md ## Rationale Keep both upstream contracts rather than weakening the release guard or the E2E selector. The incoming assertion failed with KeyError for fetchDepth because the checkout now has explicit release and ordinary branches. Checking both branches and the unique impact step preserves the intended coverage without changing runtime behavior. Release and CI-helper contracts passed 959 cases and 63 subtests, with one explicit opt-in SBT check skipped. Native Windows version-bump/history coverage passed 228 cases. Black 22.3.0 accepted the changed test. Existing release review findings and full CI remain separate readiness gates. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 879c5a1c-4efa-4620-93ea-f8f0052112f1

Rana Singh (ranadeepsingh) requested a review from Brendan Walsh (BrendanWalsh) as a code owner September 21, 2026 20:49

Copilot AI lite review requested due to automatic review settings September 21, 2026 20:49

Rana Singh (ranadeepsingh) added the area/build label Sep 21, 2026

Copilot started reviewing on behalf of Rana Singh (ranadeepsingh) September 21, 2026 20:50 View session

Rana Singh (ranadeepsingh) mentioned this pull request Sep 21, 2026

test: remove obsolete Python runner and duplicate categorical passes #2735

Merged

Copilot AI reviewed Sep 21, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 22, 2026 02:07

Copilot started reviewing on behalf of Rana Singh (ranadeepsingh) September 22, 2026 02:07 View session

Copilot AI reviewed Sep 22, 2026

View reviewed changes

Rana Singh (ranadeepsingh) enabled auto-merge (squash) September 22, 2026 03:21

Brian J. Stafford (brianjstafford) approved these changes Sep 22, 2026

View reviewed changes

Rana Singh (ranadeepsingh) merged commit e6f8306 into microsoft:master Sep 22, 2026
79 checks passed

This was referenced Sep 22, 2026

chore: bring latest master fixes to Spark 4.0 #2733

Merged

chore: bring latest master fixes to Spark 4.1 #2734

Merged

Navigation Menu

ci: skip notebook E2E for provably unrelated PR changes - #2736

ci: skip notebook E2E for provably unrelated PR changes#2736
Rana Singh (ranadeepsingh) merged 2 commits into
microsoft:masterfrom
ranadeepsingh:ci/conservative-pr-tests-20260921

Rana Singh (ranadeepsingh) commented Sep 21, 2026 •

edited

Loading

Uh oh!

azure-pipelines Bot commented Sep 21, 2026

Uh oh!

github-actions Bot commented Sep 21, 2026

Uh oh!

Rana Singh (ranadeepsingh) commented Sep 21, 2026

Uh oh!

azure-pipelines Bot commented Sep 21, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Rana Singh (ranadeepsingh) commented Sep 22, 2026

Uh oh!

azure-pipelines Bot commented Sep 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Rana Singh (ranadeepsingh) commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes are proposed?

What deliberately remains unchanged?

How is this patch tested?

Previous-head CI evidence

Review artifact organization

Scope

Uh oh!

azure-pipelines Bot commented Sep 21, 2026

Uh oh!

github-actions Bot commented Sep 21, 2026

Uh oh!

Rana Singh (ranadeepsingh) commented Sep 21, 2026

Uh oh!

azure-pipelines Bot commented Sep 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Rana Singh (ranadeepsingh) commented Sep 22, 2026

Uh oh!

azure-pipelines Bot commented Sep 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Rana Singh (ranadeepsingh) commented Sep 21, 2026 •

edited

Loading