Keep skipped imports out of translation PRs

[hiciefte]

Problem and change

In Bisq2 #5059, Transifex replaced two Portuguese strings with English. An unrelated existing control character caused preflight validation to skip the file, but publication still collected the imported changes. The quality gate detected the regression and the PR was blocked.

Publication now excludes explicitly skipped files before semantic review, batching and staging. It saves their imported contents and validation summary for diagnosis. Healthy files may still be published; a run with skipped work exits unsuccessfully before advancing the source baseline or sending its success heartbeat. Missing or malformed skip records fail closed.

Unsupported locales and missing source files also produce explicit skip records, and preserved debug output cannot overwrite skipped inputs.

Validation

• Regression tests reproduce the publication leak with a real Git repository, including mixed healthy/skipped files, all-skipped input, new files and dry run.
• An offline replay using the exact original Portuguese file from #5059 excludes it and preserves its bytes.
• Independent review found no remaining blockers after checking publication, path validation, stale output and reporting.
• Full local suite: 3,282 passed, one skipped and 13 subtests passed; five Unix-socket tests initially hit the execution sandbox and all five passed with the required local socket access (3,287 tests verified in total).
• Focused unit/integration suite: 108 passed. Ruff, shell syntax and diff checks passed.
• Candidate Docker image passed non-root, network-disabled smoke checks without credentials or production volumes; all 70 application source files and publisher script match the signed candidate manifest.

Review follow-up

CodeRabbit completed a review of all 11 files at 1c74a610eb854c1e30a1c23132058414c9328392, with no actionable code findings and minimal merge risk. Its docstring-coverage warning is addressed by 2407fade0335c836f41a7aa0f9a539d15357f3a1: four docstrings in three files. Independent review confirms identical executable AST after removing docstrings and no missing docstrings among touched functions; 62 focused checks passed again. CodeRabbit's follow-up is rate-limited, so this does not claim a second completed external review. CI validates the final head.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 93e9f428-1c4a-4776-ac18-3b2f268bf4b6

📥 Commits

Reviewing files that changed from the base of the PR and between 3d84d97 and 1c74a61.

📒 Files selected for processing (11)

• README.md
• localize/connectors.py
• localize/pipeline_core.py
• localize/translate_localization_files.py
• localize/translation_publication.py
• tests/integration/test_skipped_translation_publication.py
• tests/integration/test_translate_localization_files.py
• tests/unit/test_pipeline_core.py
• tests/unit/test_translation_publication.py
• tests/unit/test_update_translations_script.py
• update-translations.sh

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The pipeline now records skipped translation inputs, validates skip records before publication, preserves skipped-file evidence, and excludes skipped paths from publication. Runs with skipped inputs exit unsuccessfully without advancing the Git-source baseline or sending the success heartbeat.

Changes

Skipped translation input publication

Layer / File(s)	Summary
Record skipped inputs and protect their contents localize/connectors.py, localize/pipeline_core.py, localize/translate_localization_files.py, tests/integration/test_translate_localization_files.py, tests/unit/test_pipeline_core.py, tests/unit/test_translation_publication.py	Queue processing records unsupported locales, unextractable language codes, and missing source files in the validation summary. Copy-back skips explicitly listed paths.
Validate skip records and prepare evidence localize/translation_publication.py, tests/unit/test_translation_publication.py	The planner validates skip paths and source files, removes valid skipped paths from candidates, and preserves skipped inputs with the summary in an evidence directory.
Apply the plan and stop skipped runs update-translations.sh, tests/integration/test_skipped_translation_publication.py, tests/unit/test_update_translations_script.py, README.md	The shell publisher uses the planner’s candidate list and exits unsuccessfully when skips remain, before baseline advancement or the success heartbeat. The README describes this behavior.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant UpdateScript as update-translations.sh
  participant Planner as localize.translation_publication
  participant PRBatching as PR batching and semantic review
  participant GitBaseline as Git-source baseline
  participant SuccessHeartbeat as success heartbeat
  UpdateScript->>Planner: Validate summary and candidate files
  Planner-->>UpdateScript: Return filtered files and evidence path
  UpdateScript->>PRBatching: Publish remaining files
  Note over UpdateScript,SuccessHeartbeat: If skipped inputs remain, exit with status 1
  Note over UpdateScript,GitBaseline: Do not advance the baseline
  Note over UpdateScript,SuccessHeartbeat: Withhold the success heartbeat

Loading

Merge Risk: ⚪ Minimal · up to 1c74a

This change keeps skipped translation inputs out of translation PRs and fails runs that still have skipped work. No merge-blocking risk was found in the supplied context.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 1c74a

The change strengthens exclusion of invalid imports and prevents incomplete runs from reporting success. No new security vulnerability was established, but external integrations and deployment isolation remain incompletely verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The reviewed exposure concerns translation files in the configured target repository, diagnostic copies, and bot-created publication batches. The examined changes constrain file selection rather than grant additional filesystem or publication authority.

Security Findings and Attack Paths

• observed — The built-in filesystem copy-back rejects resolved sources outside the translated queue and destinations outside the input root, and suppresses outputs matching skipped paths. Legacy adapters lacking the new keyword remain outside that copy-back guarantee; the available revision comparison shows continuation of their prior behavior, not a newly established attack path.

Trust Boundaries and Controls

• observed — Imported data reaches publication only through the filtered candidate set. Before commit, the publisher resets the index and verifies that staged paths exactly equal the current batch, preventing unrelated staged paths from silently joining it.

Resilience and Maintainability Implications

• observed — The publisher initializes skip evidence to an invalid sentinel before processing and refuses publication if planning fails. Remaining skips withhold both baseline advancement and the final heartbeat, preserving the distinction between partial publication and a successfully completed run.

Hardening Proposals

• proposed — For supported external adapters, require skip-aware copy-back whenever skips exist. If deployments share report storage across repositories, bind validation summaries to a repository and run identity rather than relying solely on the fixed report path.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 73.08% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 26 functions across 10 files. (1 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: preventing skipped imports from entering translation pull requests.

Full details: Docstring Coverage

Explanation

Docstring coverage is 73.08% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 26 functions across 10 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit checks the queue at dawn,
And marks each skipped file with care.
The safe translations hop along,
While evidence rests beside them there.
No heartbeat sounds; the baseline waits,
Until the next run clears the gates.}

_{Comment @coderabbitai help to get the list of available commands.}

[hiciefte]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.