Fix news-translate workflow losing translations after first PR call#1838
Fix news-translate workflow losing translations after first PR call#1838
Conversation
… lost translations Agent-Logs-Url: https://github.com/Hack23/riksdagsmonitor/sessions/a25b61db-fde7-43ac-a845-5c3b8cc6d357 Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
github-actions
bot
added
documentation
🏷️ Automatic Labeling SummaryThis PR has been automatically labeled based on the files changed and PR metadata. Applied Labels: documentation,workflow,ci-cd,size-m,news,agentic-workflow Label Categories
For more information, see |
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates the news-translate agent prompt and safe-outputs configuration to prevent translations from being dropped after the first safeoutputs___create_pull_request call by enabling multiple PRs and guiding the agent to use rolling batches.
Changes:
- Increased
create-pull-request.maxfrom 1 → 5 and surfaced that limit in the generated prompt/tools metadata. - Rewrote RULE 1 and the time/batch strategy to reflect that PR patches are snapshotted (“frozen”) at tool-call time and require multiple PRs per run.
- Updated performance/time targets and PR template to support batched PR creation (batch numbering, checkout guidance).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| .github/workflows/news-translate.md | Adjusts agent instructions to use rolling batches and multiple PRs; documents snapshot behavior and updated time budgets. |
| .github/workflows/news-translate.lock.yml | Regenerates compiled workflow prompt/config so the agent sees create_pull_request(max:5) and safe-outputs allows up to 5 PRs. |
| **The #1 cause of lost work was misunderstanding how `safeoutputs___create_pull_request` actually works.** The tool captures the patch from your **current commits at the moment you call it**. Any files you create or commit *after* that call on the same branch are **NOT** added to the PR — they are **silently lost** when the ephemeral agent workspace is discarded. Multiple commits to the same local branch after the first call do **NOT** update the PR. | ||
|
|
||
| > 🚨 **PRODUCTION INCIDENT**: On 2026-04-14, 10 translated files were committed locally but the agent delayed `safeoutputs___create_pull_request` until minute ~50. The safeoutputs session had expired. Every retry returned "session not found". The `detection` and `safe_outputs` jobs were SKIPPED. All 10 files were lost. | ||
| > 🚨 **PRODUCTION INCIDENT (2026-04-19, PR #1835)**: The agent translated 7 languages (`da`, `no`, `de`, `fi`, `fr`, `es`, `nl`) for `2026-04-18-breaking-1705`. After committing 4 languages at minute 21, it called `safeoutputs___create_pull_request` — patch frozen. It then translated `fr`, `es`, `nl` on the same branch believing "they'll be included in the PR". **They were not.** Only 4/7 translations reached `main`; 3 complete translations were discarded. |
There was a problem hiding this comment.
This incident list uses no, but the rest of the workflow consistently uses nb (and batch definitions also reference nb). If file naming/path conventions are nb, this inconsistency can cause the agent to generate the wrong locale code in filenames or language lists. Standardize the language code in the incident narrative to match the repo’s actual locale naming (likely nb).
| > 🚨 **PRODUCTION INCIDENT (2026-04-19, PR #1835)**: The agent translated 7 languages (`da`, `no`, `de`, `fi`, `fr`, `es`, `nl`) for `2026-04-18-breaking-1705`. After committing 4 languages at minute 21, it called `safeoutputs___create_pull_request` — patch frozen. It then translated `fr`, `es`, `nl` on the same branch believing "they'll be included in the PR". **They were not.** Only 4/7 translations reached `main`; 3 complete translations were discarded. | |
| > 🚨 **PRODUCTION INCIDENT (2026-04-19, PR #1835)**: The agent translated 7 languages (`da`, `nb`, `de`, `fi`, `fr`, `es`, `nl`) for `2026-04-18-breaking-1705`. After committing 4 languages at minute 21, it called `safeoutputs___create_pull_request` — patch frozen. It then translated `fr`, `es`, `nl` on the same branch believing "they'll be included in the PR". **They were not.** Only 4/7 translations reached `main`; 3 complete translations were discarded. |
| **Do NOT put everything into one PR.** Opening a new PR for each 3–4 language batch is the only way to avoid losing work — `create-pull-request.max: 5` in the frontmatter supports this. | ||
|
|
||
| **Counting rule**: Before calling safe output, count your translated files. If fewer than 5, explain why in the PR body (e.g., "all today's articles already translated, working on backlog"). | ||
| **Counting rule**: Before each `safeoutputs___create_pull_request` call, count the files staged in the *current* batch. Aim for 3–4 files per PR. Never let a single PR exceed 4 translated files (safe-output patch size: 4 KB headroom). |
There was a problem hiding this comment.
The guidance ties batch sizing to a “4 KB headroom” but the safe-outputs config limit shown in the compiled workflow is max_patch_size: 4096 (i.e., ~4 KB total, not ‘headroom’). With 400–500 line HTML (as referenced later), a single translation file’s diff is likely to exceed 4 KB, making 3–4 files per PR infeasible and potentially causing repeated PR creation failures. Recommend aligning the prompt with the actual constraint: either (a) increase the configured max_patch_size to a value that can accommodate the intended batch sizes, or (b) change the batching guidance to size by patch bytes (or to fewer files per PR) rather than a fixed 3–4 files.
| # After safeoutputs___create_pull_request succeeds for batch N | ||
| git status --short news/ | ||
| # Switch back to main so the next batch starts from a clean base | ||
| git checkout main 2>/dev/null || git checkout - |
There was a problem hiding this comment.
The fallback git checkout - is not guaranteed to land on a clean base (it just toggles to the previous branch, which could be the same PR branch depending on user history/commands). Since this snippet is meant to prevent stacking commits onto the already-snapshotted branch, it should be deterministic: check out the repo’s default branch explicitly (e.g., main only) and, if unavailable, instruct the agent to resolve the branch name rather than silently toggling.
| git checkout main 2>/dev/null || git checkout - | |
| git checkout main 2>/dev/null || { echo "Default branch 'main' is unavailable. Resolve the repository's default branch explicitly before continuing."; exit 1; } |
| ```bash | ||
| # Return to main so batch N+1 starts from a clean base. | ||
| # safeoutputs___create_pull_request will create a fresh branch for the next batch automatically. | ||
| git checkout main 2>/dev/null || true |
There was a problem hiding this comment.
Silently ignoring a failure to switch back to main defeats the safety goal of ensuring the next batch doesn’t build on the prior PR’s commit graph. If git checkout main fails, the agent may remain on the previous branch and continue committing, recreating the ‘stacking onto frozen patch’ failure mode you’re trying to eliminate. Prefer failing loudly (or using a reliable fallback that still guarantees a clean base) so the agent can correct course before starting the next batch.
| git checkout main 2>/dev/null || true | |
| git checkout main || { echo "ERROR: failed to switch back to main; aborting before next batch." >&2; exit 1; } | |
| [ "$(git branch --show-current)" = "main" ] || { echo "ERROR: repository is not on main after checkout; aborting before next batch." >&2; exit 1; } |
|
@copilot apply changes based on the comments in this thread |
…ax-patch-size units Agent-Logs-Url: https://github.com/Hack23/riksdagsmonitor/sessions/233df121-7c44-4391-a7e1-b3d74b38beae Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Applied all 4 review fixes in 9ca1aee:
|
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
| "title": "🌐 Article Translations - {date} batch {n} ({count} files)", | ||
| "body": "## Summary\n\nTranslated {article_type} articles into {count} languages (batch {n} of up to 3).\n\n### Translations\n- Source: EN\n- Languages (this batch): {lang_list}\n- Files: {count}\n- Method: AI translation (create tool)\n\n### Quality\n- Section headings: ✅ Translated\n- Body paragraphs: ✅ Translated\n- English leakage: ✅ None\n- HTMLHint: {htmlhint_status}\n\n### Source\n- Workflow: `news-translate`\n- Follow-up: batches {n+1}+ will ship as separate PRs", | ||
| "labels": ["agentic-news", "translation"] // add "needs-review" if HTMLHINT_FAILED=true | ||
| }) |
There was a problem hiding this comment.
The example safeoutputs___create_pull_request payload is shown as JSON, but it contains a // ... inline comment on the labels line. If the agent copies this as-is, the tool call will fail to parse. Move the note outside the JSON block or express it in a JSON-compatible way (e.g., in surrounding prose).
| "title": "🌐 Article Translations - {date} ({count} files)", | ||
| "body": "## Summary\n\nTranslated {article_type} articles into {count} languages.\n\n### Translations\n- Source: EN\n- Languages: {lang_list}\n- Files: {count}\n- Method: AI translation (create tool)\n\n### Quality\n- Section headings: ✅ Translated\n- Body paragraphs: ✅ Translated\n- English leakage: ✅ None\n- HTMLHint: {htmlhint_status}\n\n### Source\n- Workflow: `news-translate`", | ||
| "title": "🌐 Article Translations - {date} batch {n} ({count} files)", | ||
| "body": "## Summary\n\nTranslated {article_type} articles into {count} languages (batch {n} of up to 3).\n\n### Translations\n- Source: EN\n- Languages (this batch): {lang_list}\n- Files: {count}\n- Method: AI translation (create tool)\n\n### Quality\n- Section headings: ✅ Translated\n- Body paragraphs: ✅ Translated\n- English leakage: ✅ None\n- HTMLHint: {htmlhint_status}\n\n### Source\n- Workflow: `news-translate`\n- Follow-up: batches {n+1}+ will ship as separate PRs", |
There was a problem hiding this comment.
The example PR body text includes batches {n+1}+ which looks like an expression placeholder but won’t be automatically expanded. This is likely to end up literally in the PR description. Consider rewording to avoid arithmetic placeholders (e.g., "subsequent batches will ship as separate PRs") or use a plain placeholder name that can be manually substituted.
| "body": "## Summary\n\nTranslated {article_type} articles into {count} languages (batch {n} of up to 3).\n\n### Translations\n- Source: EN\n- Languages (this batch): {lang_list}\n- Files: {count}\n- Method: AI translation (create tool)\n\n### Quality\n- Section headings: ✅ Translated\n- Body paragraphs: ✅ Translated\n- English leakage: ✅ None\n- HTMLHint: {htmlhint_status}\n\n### Source\n- Workflow: `news-translate`\n- Follow-up: batches {n+1}+ will ship as separate PRs", | |
| "body": "## Summary\n\nTranslated {article_type} articles into {count} languages (batch {n} of up to 3).\n\n### Translations\n- Source: EN\n- Languages (this batch): {lang_list}\n- Files: {count}\n- Method: AI translation (create tool)\n\n### Quality\n- Section headings: ✅ Translated\n- Body paragraphs: ✅ Translated\n- English leakage: ✅ None\n- HTMLHint: {htmlhint_status}\n\n### Source\n- Workflow: `news-translate`\n- Follow-up: subsequent batches will ship as separate PRs", |
|
@copilot apply changes based on the comments in this thread |
… placeholder Agent-Logs-Url: https://github.com/Hack23/riksdagsmonitor/sessions/cb72b838-dc80-40e6-a3a8-7a225e1de644 Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Applied both review fixes in 81e59a5:
|
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
| ## ⏱️ Time Budget (55 minutes of work, hard stop at 55) | ||
|
|
||
| The budget is now organised around **3 rolling PR batches**, not one monolithic PR. This is the direct fix for PR #1835 where 3 completed translations (`fr`, `es`, `nl`) were lost because they were committed *after* the single `safeoutputs___create_pull_request` call had frozen the patch. |
There was a problem hiding this comment.
This hard-codes the workflow narrative to 3 rolling batches, but the workflow is configured for create-pull-request.max: 5 and elsewhere instructs agents they may open up to 5 PRs per run. This mismatch can cause agents to stop at 3 even when time remains (or when more than 3 PRs are needed for multiple articles/types). Consider changing wording to "typically 2–3 batches" while explicitly allowing up to 5, and align the rest of the document (including templates) with max: 5.
| | HTMLHint errors | Fix with `edit` tool or run `npx tsx scripts/article-quality-enhancer.ts --fix` | | ||
| | safeoutputs "session not found" | Session expired — ALL work is LOST. **Prevention: call `safeoutputs___create_pull_request` by minute 25.** | | ||
| | safeoutputs "session not found" | Session expired — all uncreated PR intents are LOST. **Prevention: call `safeoutputs___create_pull_request` for the first batch by minute 22.** Later batches must also be called promptly (never more than 15 minutes between successive calls). | | ||
| | Committed files on a branch that already has a PR | Those files are lost — the patch was frozen at the first `safeoutputs___create_pull_request` call. Switch to `main`, re-create the translations fresh, and call `safeoutputs___create_pull_request` again for a new PR. | |
There was a problem hiding this comment.
Saying "Those files are lost" is a bit absolute and may prompt unnecessary re-translation. More precisely: those commits won't be added to the already-created PR, but they may still be recoverable within the current workspace by creating a new snapshot/PR containing only the additional files (e.g., by moving back to main and re-applying the changes via cherry-pick/recommit so the new PR doesn’t include files already shipped in prior batches). Recommend rewording to: "Not included in the existing PR; to ship them create a new batch PR from main containing only the new translations" and optionally mention a recovery approach that avoids redoing translation.
| | Committed files on a branch that already has a PR | Those files are lost — the patch was frozen at the first `safeoutputs___create_pull_request` call. Switch to `main`, re-create the translations fresh, and call `safeoutputs___create_pull_request` again for a new PR. | | |
| | Committed files on a branch that already has a PR | Not included in the existing PR — the patch was frozen at the first `safeoutputs___create_pull_request` call. Switch to `main` and create a new batch PR containing only the new translations; if the changes still exist locally, re-apply/cherry-pick or recommit them so you do not need to re-translate from scratch. | |
|
@copilot apply changes based on the comments in this thread |
… recoverable files Agent-Logs-Url: https://github.com/Hack23/riksdagsmonitor/sessions/f13db3b0-0295-4404-9263-f987b5558ca7 Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Applied both review fixes in 347ca3c:
|
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
Root Cause Analysis — PR #1835
PR #1835 (
news/translate/2026-04-18/breaking-1705-batch1) translated only 4 files (da,no,de,fi) for2026-04-18-breaking-1705-en.html. The agent produced 7 complete translations locally but only 4 shipped tomain.Why: 3 compounding bugs in
.github/workflows/news-translate.mdcreate-pull-request.max: 1— agent could not open a second PR for the next batch.safeoutputs___create_pull_requestfreezes the patch at call time.Checklist
news-translate.mdcreate-pull-request.max: 5— frontmatter + lock file propagationnews-translate.lock.ymlwithgh aw compilenbstandardisation; fail-loudgit checkout mainin two places; correctmax-patch-sizeunit interpretation (KB → 4 MB).// ...inline comment from example JSON payload; replace{n+1}arithmetic placeholder with plain prose; add "substitute every {placeholder}" warning.create-pull-request.max: 5. Reworded to "typically 2–3 rolling PR batches (up to 5 allowed)" so agents don't artificially stop at 3 when more PRs are warranted.mainfor a fresh batch PR.