Skip to content

Fix stale PR translation revert issue#630

Merged
guchenhe merged 4 commits into
mainfrom
fix/stale-pr-translation-revert
Dec 24, 2025
Merged

Fix stale PR translation revert issue#630
guchenhe merged 4 commits into
mainfrom
fix/stale-pr-translation-revert

Conversation

@guchenhe
Copy link
Copy Markdown
Contributor

@guchenhe guchenhe commented Dec 24, 2025

Summary

Fixes the issue where translation PRs for stale source PRs would revert changes from PRs that were merged after the source PR was created.

Reported scenario:

  1. PR A is created before PR B
  2. PR B deletes content a, adds content b, modifies content c
  3. PR B is merged first
  4. When processing PR A, the translation PR reverts all of PR B's changes

Example from PR #593:

Root Cause

The translation workflow was using the source PR's complete working directory state (which is a snapshot from when that PR branch was created) rather than applying only the PR's net changes on top of the current main branch.

Problematic code in setup_translation_branch():

# For new branches (buggy):
self.run_git("checkout", "-b", self.sync_branch)
self.run_git("reset", "--soft", "origin/main")
self.run_git("reset")
# This kept PR's working directory which could be stale

For incremental branches: merge_docs_json_for_incremental_update() took the English section from PR HEAD, which was also stale for old PRs.

Fix

  1. For NEW branches: Create branch directly from origin/main instead of from PR's working directory

    self.run_git("checkout", "-b", self.sync_branch, "origin/main")

  2. For EXISTING branches: Replace merge_docs_json_for_incremental_update() with _merge_docs_json_from_main() which:

    • Uses main's full structure (includes all recent changes)
    • Preserves translation sections from the translation branch

  3. For BOTH: Selectively checkout only the files that the PR actually changed via new _checkout_pr_changed_files() method, rather than bringing in the entire working directory

Changes

  • _merge_docs_json_from_main(): New method that merges main's structure with branch's translations
  • _checkout_pr_changed_files(): New method that checkouts only PR's changed source files
  • setup_translation_branch(): Updated to start from main for new branches, use new merge logic for existing
  • run_translation_from_sync_plan(): Added call to selective checkout

Testing

  • Syntax validation: ✅
  • Import test: ✅
  • Full testing should be done with a test PR that simulates the stale PR scenario

Test Plan

  • Create a test branch that adds a new file
  • Before merging, make changes to main via another PR
  • Verify the translation PR for the test branch doesn't revert the other changes

🤖 Generated with Claude Code

@guchenhe @claude
Fix stale PR translation revert issue
22e2e8f 
When PR A is created before PR B but PR B merges first, the translation
workflow for PR A was reverting all of PR B's changes. This happened because
the translation workflow used PR A's working directory state (which is a
snapshot from before PR B existed) rather than applying only PR A's changes.

Root cause:
- setup_translation_branch() for new branches did:
  checkout -b branch → reset --soft origin/main → reset
  This kept PR's working directory which could be stale

- For incremental branches, merge_docs_json_for_incremental_update() took
  the English section from PR HEAD, which was also stale for old PRs

Fix:
- For NEW branches: Create branch directly from origin/main (not from PR's
  working directory). This ensures we start with the latest state including
  all changes from PRs merged after this PR was created.

- For EXISTING branches: Merge main's docs.json structure with our
  translations (instead of taking EN section from stale PR)

- For BOTH: Selectively checkout only the files that the PR actually changed
  from PR's head, rather than bringing in the entire working directory.
  This prevents overwriting files from other PRs.

Example issue (PR #593):
- PR #593 only added one file
- Translation PR #611 tried to delete 11 files and revert massive docs.json changes
- This was because it used PR #593's stale state from before other PRs merged

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@guchenhe guchenhe requested a review from RiskeyL as a code owner December 24, 2025 02:37
@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels Dec 24, 2025
@guchenhe @claude
Fix: Use PR's docs.json for finding file positions in navigation
1c396e3 
The initial fix had a side effect: since we start from main's docs.json,
and PR's new files aren't in main's English section yet, sync_docs_json_incremental()
couldn't find where to place new files in the translation navigation.

Fix: Add `reference_sha` parameter to sync_docs_json_incremental() that loads
PR's docs.json for finding file positions, while still modifying main's
translation sections. This ensures:
1. Main's docs.json structure is preserved (no reverts)
2. New files are found in PR's docs.json
3. Translations are added at the correct positions

This also removes the unused _apply_pr_english_section_to_main() method.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@guchenhe @claude
Fix EN section not updated when using reference_sha
d9558d4 
When the translation branch starts from main, the PR's docs.json
structural changes (new file entries in EN section) were not being
incorporated. This caused the translation PR to have mismatched
navigation entries.

The fix now also updates the EN section of the working directory's
docs.json when processing added files found in the reference
docs.json (from the PR).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@guchenhe @claude
Also remove deleted files from EN section in stale PR scenario
861917e 
When processing deleted files, the sync now also removes them from the
EN section of docs.json. This is needed when the translation branch
starts from main, which may still have the deleted file entries.

Verified with comprehensive local testing covering 10 scenarios:
- Basic stale PR, multiple files, modifications, deletions
- Nested groups, new dropdowns, mixed operations
- Backward compatibility, incremental syncs, structure changes

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@guchenhe guchenhe merged commit 61b37d8 into main Dec 24, 2025
1 check passed
@RiskeyL RiskeyL deleted the fix/stale-pr-translation-revert branch March 16, 2026 06:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RiskeyL RiskeyL Awaiting requested review from RiskeyL RiskeyL is a code owner

Assignees

No one assigned

Labels

bug Something isn't working size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@guchenhe