Spark Pr Contribution

Apache Spark PR Contribution Recipe

Also apply the shared OSS contribution quality protocol in ../../../references/contribution-quality.md. Project-specific instructions below override the shared protocol when they conflict.

• New PR (create from scratch): User says "pick a Spark issue" or "follow the recipe" → follow §1–§7 (pick JIRA, branch, implement, commit, push, open PR).
• Existing PR (take actions): User gives the PR URL (e.g. a PR created a few days ago; now there are reviewer comments or CI failures) → follow §8: fetch that PR, get branch + CI + reviewer comments, then take actions (fix, rebase, test, push, optional reply).

Issues are tracked in JIRA (SPARK-xxxxx); PRs are on GitHub. Before making any code changes for a new PR: sync with upstream, create the branch; then implement.

Shared execution guardrails

Apply these rules throughout the recipe:

• Think before coding. Do not silently assume JIRA scope, reviewer intent, or the right fix direction. If JIRA comments, PR comments, linked work, or overlapping open PRs point in different directions, stop and resolve that ambiguity before editing code.
• Simplicity first. Ship the smallest change that fixes the reported problem. Do not add new knobs, abstractions, cleanup refactors, or speculative edge-case handling unless the JIRA or reviewer explicitly calls for them.
• Surgical changes. Touch only the files and lines that trace directly to the JIRA, failing check, or requested review follow-up. Clean up only fallout caused by your change; do not restyle or "improve" unrelated nearby code.
• Goal-driven execution. Work in a tight verify loop: identify the concrete failure, implement the smallest fix, run the narrowest relevant validation first, then widen if needed. For open PR work, follow: inspect comments/checks/conflicts -> fix -> rebase -> rerun focused validation -> push -> leave a short PR comment.
• Merge-ready means more than pushed code. Treat a Spark PR as ready only when it is current with upstream/master, CI is green or any remaining red check is explained, actionable reviewer/bot comments are handled on the current head, scoped sbt/Python evidence is recorded, and the PR body/comment truthfully describes validation.
• Make human intervention exceptional. Keep working until the branch is merge-ready or blocked by permissions, unavailable logs/credentials, maintainer design direction, or a local environment limitation that cannot be worked around safely.

Closed-loop PR quality loop

Use this loop for both new Spark PRs and existing PR sweeps:

1. Prove the issue shape before editing. Turn the JIRA/report into a concrete failing path, error class, golden-file expectation, test gap, or module-level behavior before changing code.
2. Match existing patterns. Inspect nearby Spark tests/helpers/error classes/golden files for naming, compatibility, tags, SQLConf behavior, PySpark parity, and style before adding a new pattern.
3. Pre-answer reviewer and bot concerns. Before opening or updating the PR, ask what reviewers, linters, and CI are likely to flag: missing test coverage, stale golden output, missed PySpark/Scala parity, broad scope, style/formatting, or touching generated docs without regeneration.
4. Test the bug, the non-bug, and the edge seam. Prefer a regression test for the reported failure, keep an existing happy path green, and cover one boundary/negative case when the fix changes SQL behavior, error classes, compatibility, config, or Python/Scala bridging.
5. Self-review before commit. Run git diff --check, read the final diff as a reviewer, and remove accidental refactors, dead code, debug output, unrelated formatting, and untracked PR body files from the commit.
6. Close the loop after push. Re-read live CI and review comments on the current head. If feedback is actionable, fix it. If a red check is stale/cancelled/unrelated, verify the current head and retrigger with the least-invasive safe action, usually an empty commit when direct rerun is unavailable.
7. Report exact state. End with which PRs are green, which are rerunning, which still have actionable comments, and which are blocked by permissions, infrastructure, or maintainer direction.

1. Pick an issue (JIRA)

• Find issues in JIRA: Apache Spark JIRA. Filter for open/unresolved; label starter often indicates smaller scope.
• Prefer well-scoped issues (clear problem, single fix). Check the JIRA description and any linked design docs.
• Avoid duplicates: Before implementing, check the JIRA for "links to" or "mentioned in" an existing open GitHub PR. Search apache/spark pull requests for the JIRA id (e.g. SPARK-38743 or 38743). Skip the issue if an open PR from someone else already targets it.
• Note the JIRA id (e.g. SPARK-38743). Branch name, PR title, and commit message must reference it. PR title format: [SPARK-xxxxx][COMPONENT] Short title (e.g. [SPARK-38743][SQL] Test the error class: MISSING_STATIC_PARTITION_COLUMN).

2. Sync with upstream, create branch (before any code changes)

Do this before making any fixes. The default branch is master.

cd /Users/dejain/nvidia/oss/apache_spark
git remote add upstream https://github.com/apache/spark.git
git fetch upstream
git checkout master
git pull upstream master
git checkout -b SPARK-xxxxx-short-description

Use the actual JIRA number in the branch name (e.g. SPARK-38743-missing-static-partition-column-test). If there are uncommitted changes: git stash push -m "wip" before the above, then git stash pop after creating the branch.

Remotes (one-time): origin = your fork (e.g. git@github.com:deepujain/apache_spark.git), upstream = https://github.com/apache/spark.git.

3. Implement (only after the new branch exists)

• Make only the changes needed for the JIRA; keep scope clear.
• Code quality: Follow Spark contributing guide. Scala: run ./dev/scalafmt. Match existing code style in the touched module.
• Tests: Add or extend tests as required. In tests that fix a specific JIRA, include the JIRA id in the test name or comment (e.g. test("SPARK-38743: MISSING_STATIC_PARTITION_COLUMN") or // SPARK-38743: ...).
• Docs: For new features or behavior changes, add docstrings or update docs under docs/ as appropriate.

4. Commit

• Commit only the fix files. Do not add or commit any PR_SPARK-xxxxx_body.md if you create one for copy-paste.
• Author: Deepak Jain <deepujain@gmail.com> (must show as "Deepak Jain", not the GitHub username dejain). Always use --author on both commit and amend.
• Message: PR-style title with JIRA and component. Example: [SPARK-38743][SQL] Test the error class: MISSING_STATIC_PARTITION_COLUMN. Use single quotes in shell to avoid zsh history expansion.
• Commit command (author): git commit --no-verify --author="Deepak Jain <deepujain@gmail.com>" -m '[SPARK-xxxxx][COMPONENT] Title'

5. Push

• When giving push commands to the user, prefix with the amend step so they can remove host-added tool attribution or fix an incorrect author. Include --author so the author stays "Deepak Jain":

  cd /Users/dejain/nvidia/oss/apache_spark
  git commit --amend --no-verify --author="Deepak Jain <deepujain@gmail.com>" -m '[SPARK-xxxxx][COMPONENT] Title'
  git push --no-verify --set-upstream origin SPARK-xxxxx-short-description
  

  Use the actual JIRA id and commit message for the current PR.
• Push to your fork (origin), not upstream.
• After rebasing: git push --no-verify --force-with-lease origin <branch>.

6. Open the PR (GitHub)

• Where: GitHub. Create the PR from your fork's branch to apache/spark (base branch master).
• Deep link: Always provide a clickable URL that opens the "New PR" page with branches pre-selected: https://github.com/apache/spark/compare/master...<github-username>:spark:<branch>?expand=1 Replace <github-username> with the configured GitHub username and <branch> with the actual branch name.
• Title: [SPARK-xxxxx][COMPONENT] Short title (e.g. [SPARK-38743][SQL] Test the error class: MISSING_STATIC_PARTITION_COLUMN). Component examples: SQL, Core, PySpark, MLlib, Build.
• Description (format that gets merged):
  • Summary - One short paragraph: what problem and what the fix does.
  • Change - Bullet list: for each file, path then what changed.
  • Tests - What tests were added or how the change was tested (or Why no new tests with brief justification).
  • Fixes SPARK-xxxxx - So the JIRA is linked.
  • JIRA assignee for credit: deepujain - Required for Apache projects tracked in JIRA so committers can assign the JIRA to the contributor when the PR is merged.
• Writing pass: Before handing off PR_SPARK-xxxxx_body.md or any PR comment/review reply, run the final prose through the local humanizer-zh skill at /Users/dejain/nvidia/oss/.agents/skills/humanizer-zh/SKILL.md. Keep the JIRA id, commands, tests, and reviewer-facing facts unchanged.
• Optionally create a local PR_SPARK-xxxxx_body.md for copy-paste only; do not commit it.

7. After push: CI and rebase

• Keep the PR rebased. Rebase on latest master: git fetch upstream && git rebase upstream/master, then git push --no-verify --force-with-lease origin <branch>.
• If CI fails: fix and push; re-run may be automatic or trigger with an empty commit if needed.

8. Existing PR: user gives URL -> fetch latest, take actions

When the user shares a PR URL, it means there is something to act on: reviewer comments, CI/CD failures, merge conflicts, or a requested rebase. Read the PR page first to find out what needs attention before assuming "just rebase". Check reviewer comments, commit/PR statuses, and any requested changes, then act on what you find.

If the user shares a Spark author PR-list URL or says "open Spark PRs/MRs", treat that as a request to sweep every currently open PR for that author in apache/spark: list PRs, inspect review comments, CI checks, out-of-date state, and stale/cancelled statuses for each PR; fix actionable issues on existing branches; push follow-up commits directly; leave short PR status comments; then re-check and report green/rerunning/blocked status in a table with one row per PR.

At the start of every repeated sweep, reconcile the previous or recent authored PR set with the current open set. For every PR that disappeared, query its exact state and merge/close timestamps, then inspect final comments, reviews, timeline, linked JIRA, overlapping PRs, and any replacement commit. Record merged PRs as merged. For a PR closed without merge, establish whether it was duplicate, superseded, out of scope, policy-blocked, abandoned, or unresolved, and say whether the contribution survived in another PR. If the closure yields a reusable testing, design, review, or workflow lesson, add the smallest durable rule at the correct place in this skill, validate it, and commit/push the skill repository. Do not overfit unexplained closures; report no skill change needed when there is no reusable lesson. Include a departed-PR table before the open-PR table whenever anything merged or closed since the previous sweep.

Use this table format for Spark open-PR sweeps unless the user explicitly asks for a different format:

After every Spark sweep, include a second short table titled Lessons Learned / Skill Updates after the PR table:

Populate at least one row. If there is no durable lesson, say none and explain why no skill change was needed. If there is an evidence-backed reusable lesson, update Lessons learned (from real Spark contributions) in this skill during the same sweep, keep the new rule short and imperative, validate the skill with python3 /Users/dejain/.codex/skills/.system/skill-creator/scripts/quick_validate.py <skippy-root>/oss/apache/spark, then commit and push only the skill change from the Skippy repository. Do not batch unrelated local edits into the skill commit.

For the Review Comments column, always categorize by reviewer identity rather than giving only a total count. Include each bot type separately when present, and include human reviewers by GitHub login or display name. Use short statuses such as addressed, already addressed, stale, informational, not addressed, or blocked: needs maintainer decision.

Sweep replenishment after merge

Treat sweep as both PR maintenance and controlled replenishment, whether it is triggered by the user or by a scheduled task.

• If one or more authored Spark PRs merged since the previous sweep, and there is no higher-priority open-PR maintenance still pending, decide whether the queue is healthy enough to start another issue.
• Spark has no explicit numeric open-PR cap in this skill, so use queue health as the gate: do not open a new PR when authored branches are still stale, conflicting, red, duplicate-risky, or waiting on follow-up you can handle in the same sweep.
• If the queue is healthy, pick one new well-scoped JIRA issue using the normal Spark issue-selection rules in this skill and run the full new-PR recipe in the same sweep.
• Report the outcome explicitly in the sweep output: opened new PR, issue selected, PR in progress, or replenishment skipped with the exact blocker such as queue still unhealthy, no good candidate, or duplicate risk.

Entry point: User provides the PR URL (e.g. https://github.com/apache/spark/pull/54694). Start with 8.0 (fetch PR state and comments), then 8.1 → 8.5 as needed.

8.0 Fetch latest from PR and reviewer comments (do this first)

Fetch the PR page (e.g. with mcp_web_fetch or web search using the URL the user gave). From the PR page obtain:

1. PR details

   • Head branch (e.g. deepujain:SPARK-38719-cannot-cast-datatype-test) → local branch name = part after the colon, e.g. SPARK-38719-cannot-cast-datatype-test.
   • Base branch (usually apache:master).
   • CI status: Failing checks (e.g. "Build failed", "Tests failed", job names and log links).
   • Reviewer comments: From the Conversation tab and "Files changed" → Review: author, file/line if any, and the requested change or question.

2. List required actions

   • From CI: e.g. "Fix failing test X", "Fix compile error in Y", "Re-run after rebase".
   • From reviewers: e.g. "Use error class Z", "Add test for W", "Rename method to V". Group by file so edits are done in one pass where possible.

3. Proceed to 8.1 (checkout and sync branch), then implement each action (8.2–8.4), then push and optionally post a summary comment (8.5).

8.1 Switch to the PR branch and sync

Branch name: From the PR page, head is e.g. deepujain:SPARK-38719-cannot-cast-datatype-test → use the part after the colon: SPARK-38719-cannot-cast-datatype-test.

cd /Users/dejain/nvidia/oss/apache_spark
git fetch origin
git checkout <branch-name>   # e.g. SPARK-38719-cannot-cast-datatype-test
git fetch upstream
git rebase upstream/master

After rebase, push with lease: git push --no-verify --force-with-lease origin <branch-name>.

8.2 Run local tests (before or after pushing)

Spark uses sbt. Set Java and driver bind address so tests can run in restricted environments:

export JAVA_HOME=/opt/homebrew/opt/openjdk@17/libexec/openjdk.jdk/Contents/Home
export PATH="$JAVA_HOME/bin:$PATH"
export SBT_OPTS="-Dspark.driver.bindAddress=127.0.0.1 -Dspark.driver.host=127.0.0.1"
cd /Users/dejain/nvidia/oss/apache_spark

• SQL golden-file tests (e.g. for date/timestamp/error-class changes): ./build/sbt "sql/testOnly org.apache.spark.sql.SQLQueryTestSuite -- -z <name>.sql" Examples: -z date.sql, -z timestamp.sql, -z datetime-legacy.sql, -z timestamp-ansi.
• Single test suite or test name: ./build/sbt "sql/testOnly org.apache.spark.sql.errors.QueryExecutionAnsiErrorsSuite -- -z SPARK-49642" Or e.g. ./build/sbt "sql/testOnly *QueryCompilationErrorsSuite" for compilation-error tests.
• Full module tests (slower): ./build/sbt "sql/test" or ./build/sbt "catalyst/test".

If Maven/sbt dependency resolution fails (e.g. missing jar in ~/.m2), install the missing artifact (e.g. mvn dependency:get -Dartifact=groupId:artifactId:version) or run from repo root so dependencies resolve.

8.3 Push despite failing pre-push hooks

If the push fails because a pre-push hook fails (e.g. "Format Python code" / Black not found), and your PR does not change Python code, skip hooks:

git push --no-verify origin <branch-name>
# After rebase:
git push --no-verify --force-with-lease origin <branch-name>

Use --force-with-lease after a rebase so the remote branch is updated safely.

8.4 When CI fails or reviewers comment - take actions

For each CI failure or reviewer comment identified in 8.0:

1. Reproduce locally (if applicable): run the failing test(s) or the relevant testOnly as in 8.2.
2. Implement the change:
   • CI: Fix the failing test, compile error, or style issue; add or adjust tests as needed.
   • Reviewer: Apply the requested edit (e.g. rename, use different error class, add a test, update doc). If the comment is a question, answer it in code or in a PR reply.
3. Run relevant local tests (8.2) for the touched module so the fix is verified before push.
4. Commit with the same author and message style (§4): git add <files> then git commit --no-verify --author="Deepak Jain <deepujain@gmail.com>" -m '...'. Use an amend if it's a small follow-up: git commit --amend --no-verify --author="Deepak Jain <deepujain@gmail.com>" --no-edit.
5. Rebase if master has moved: git fetch upstream && git rebase upstream/master.
6. Push: git push --no-verify --force-with-lease origin <branch-name> (force-with-lease after rebase).
7. Leave a PR comment after every meaningful push. Rebase-only pushes, CI-refresh pushes, and validation-only updates should still leave a short PR comment. Generate a ready-to-paste PR comment using 8.5 below.

8.4a CI triage rules

• Check statuses even if comments are empty. A PR can have no review feedback but still have actionable CI failures. Do not conclude "nothing to do" until both comments and statuses are clean or still running.
• Check statuses first, then logs. Use PR/commit statuses to identify the failing jobs before diving into comments or guessing at the failure.
• Handle stale/cancelled duplicate statuses deliberately. If a newer check with the same name passed but an older cancelled/failing status still makes the PR red, verify the current head SHA. If direct rerun is unavailable, use a no-code empty commit to refresh checks and leave a short PR comment explaining the refresh.
• If GitHub Actions logs are blocked, say so immediately. If gh is unauthenticated or log access is unavailable, say that explicitly and ask the user either to authenticate gh or paste the failing check names/log snippets. Do not make the user infer that limitation.
• If local CI wrappers are blocked, run the closest direct check you can. For example, if a full CI reproduction path is blocked by the local environment, run the nearest local sbt test command for the touched module and say what remains unverified.
• Rebase and retest before adding more code. For an existing PR, first rebase onto current upstream/master and rerun the closest relevant local check. Do not assume the branch needs more edits until the rebased branch still reproduces the problem.
• Start with the narrowest relevant local validation. Run the smallest targeted build/sbt or python/run-tests command that covers the changed behavior first, then widen only if needed. If a broader suite fails for an unrelated local reason, document the scoped passing checks and call out the residual risk explicitly.

8.5 Generate a PR comment (changes + local test results)

After taking actions (rebase, fixes, local tests, push), generate a short PR comment the user can paste on the PR. Base it on what was actually done and the local test outcome.

Do this after every meaningful PR update, including rebase-only pushes, CI-refresh pushes, or validation-only updates where no source file changed.

Include (as applicable):

• Rebase: e.g. "Rebased on master."
• Changes made: One line per logical change - e.g. "Fixed X in path/to/file.scala.", "Addressed review: use error class Y.", "Resolved merge conflict in Z."
• Local tests: Which tests were run and result - e.g. "Ran ./build/sbt \"sql/testOnly org.apache.spark.sql.errors.QueryExecutionErrorsSuite -- -z SPARK-38719\" - passed." or "Ran sql/testOnly *QueryCompilationErrorsSuite - all passed."
• Closing line: e.g. "Ready for CI." / "Ready for re-review." / "Please take another look."

Style rules for PR comments:

• 2-4 sentences max. Sound like a human, not a changelog. A touch of humor is fine.
• Before posting a PR comment or review reply, run the final text through humanizer-zh and preserve the same JIRA id, commands, and test results.
• Never use the em dash character.
• Only mention tests that were actually run.

Examples:

• Rebased on master. QueryExecutionErrorsSuite passes locally. Ready for CI!
• Addressed review: renamed X to Y, added test for Z. All tests pass. Ready for another look!
• Fixed the failing error class assertion. Tests green locally, rebased and pushed. Back in business.

Lessons learned (from real Spark contributions)

• Treat global updater failures as infrastructure, not PR defects. When apache/spark update_build_status.yml repeatedly fails while mapping a check-run conclusion such as startup_failure to GitHub's allowed check conclusions, and the PR head, fork workflow run, review comments, and inline comments are unchanged, do not rebase or push just to shake loose queued Build statuses. Report the affected PRs as queued/blocked by updater infrastructure and keep checking until the current-head status changes.
• Use fork workflow runs as head evidence when upstream status mirroring is broken. For forked Spark branches, compare the fork Actions run head_sha to the PR headRefOid before deciding whether a queued or missing upstream Build status is actionable. A green fork run on the exact PR head is useful evidence, but it does not replace maintainer-visible upstream CI when the mirror recovers.

Avoid duplicate PRs

JIRA and credit

• Issues: Tracked in ASF JIRA (SPARK). No GitHub issues for Spark.
• JIRA id for credit: In every Spark PR description include **JIRA assignee for credit:** deepujain so committers can assign the JIRA to you when the PR is merged.
• Prefer a clean final PR history. Iterate locally as needed, but before the final push prefer squashing the branch to one clean [SPARK-xxxxx][COMPONENT] commit unless there is a clear reason to preserve multiple commits.

Summary

Trigger phrases (for the user)

• "sweep" (when the active repo/thread context identifies Spark.)
• "Pick a Spark issue and do the full PR recipe."
• "Next Spark PR: find a JIRA, implement, and prepare branch, commit, and PR."
• "Follow the Spark contribution recipe."
• Existing PR - user gives URL: "Here's my Spark PR: … - take actions" / "Go work on this PR" / "Address this Spark PR" → §8: Fetch PR (8.0), list actions, checkout (8.1), run tests (8.2), implement fixes (8.4), push (8.3/8.4), generate PR comment (8.5) from changes and local test results for user to paste.
• "Generate a PR comment for my Spark PR" / "Write a summary comment for this PR" → Use §8.5: produce a short comment based on changes made and local tests run (rebase, files changed, exact test command + result, "Ready for CI").
• "Spark PR has CI failures" / "Rebase my Spark PR" / "Run local tests for Spark PR" → Same §8 flow; end with generated PR comment (8.5).