Skip to content

feat(intl-pipeline): iterative patches and incremental reviews#18043

Merged
wackerow merged 12 commits into
devfrom
intl-pipeline-v7
Apr 28, 2026
Merged

feat(intl-pipeline): iterative patches and incremental reviews#18043
wackerow merged 12 commits into
devfrom
intl-pipeline-v7

Conversation

@myelinated-wackerow
Copy link
Copy Markdown
Collaborator

@myelinated-wackerow myelinated-wackerow commented Apr 27, 2026

Summary

  • Path-handling patches for the intl-pipeline: target-path normalization, soft-skip on excluded paths, do-not-translate list moved to a shared constant.
  • TranslationBanner consolidation: deprecates the old "translation out of date" banner (no longer needed now that the intl-pipeline keeps content current) and renames the English-only-pages banner from TranslationBannerLegal to TranslationBanner -- one banner, wired to the do-not-translate list.
  • Adds incremental translation reviews to /review-translations (CLI + GitHub Action): default scope is "files changed since the last LLM review on this PR," computed from each PR Review's GitHub-attached commit_id. Reviews are now submitted as proper PR Reviews (not issue comments) so the SHA travels for free.

Pipeline / config changes

  • intl-pipeline/config.ts, intl-pipeline/main.ts: hard validation now only throws on truly invalid paths (translations/, non-en intl/); excluded-list matches are logged and filtered, only failing if the entire batch is excluded.
  • normalizeTargetPath strips accidental public/content/translations/<locale>/ and src/intl/<non-en>/ prefixes, auto-prefixes JSON with src/intl/en/ and markdown with public/content/, tolerates leading ./ and /.
  • Resolution flow: normalize → exists-check → validate → excluded-skip, with leveled log() output for Netlify.
  • DO_NOT_TRANSLATE_PATHS moved to intl-pipeline/constants.ts so the banner can consume it without dragging in env-var initialization side effects.

TranslationBanner

  • The old "out-of-date translation" banner is deprecated -- intl-pipeline manifests now keep content in sync, so the alert no longer carries useful signal.
  • TranslationBannerLegal -> TranslationBanner (single component now). Wired to DO_NOT_TRANSLATE_PATHS, with localStorage key and pathname handling cleaned up (no /en/ prefix in the key, fixed return-early).

/review-translations changes (CLI + CI)

  • Default target is now the open PR for intl/pending-dev (was: branch sniffing).
  • Default scope is incremental -- diffs last_review.commit_id...PR_HEAD from the most recent submitted PR Review whose body looks like a translation review (no author filter, since reviews may be posted under either Claude's or a maintainer's GitHub identity when run locally).
  • New --full flag (CLI + workflow_dispatch input + comment regex) overrides incremental and re-reviews the full PR diff.
  • Reviews submitted via gh pr review --comment (or --approve only when zero critical issues remain; never --request-changes).
  • Standardized **Fixes:** line in the review body: Critical fixes applied: N / No fixes applied (review-only) / No critical issues found.
  • CI workflow trigger updated to match current branch naming (intl/pending- head ref).

Test plan

  • Local: /review-translations with no args → finds open PR for intl/pending-dev, scope-resolves correctly when no prior review exists (full PR diff).
  • Local: same after a review has been posted → incremental scope kicks in, only changed files reviewed.
  • Local: /review-translations --full → ignores prior review SHA.
  • CI: @claude /review-translations from an authorized PR comment fires the workflow, posts a PR Review (visible in the "Reviews" tab, not "Conversation"), commit_id is attached.
  • Pipeline: kick off a small file batch where one path matches /style-guide/ → job continues and skips that file (rather than aborting), other files translate normally.
  • Pipeline: pass developers/docs/blocks/index.md (no prefix) → normalization logs [pipeline] Normalizing "..." -> "public/content/..." and proceeds.
  • Banner: visit a do-not-translate page in a non-English locale → English content shows with banner; localStorage dismiss persists across visits.

Update

Phase 4b: dedicated JSX attribute translation pass for .md files. Translatable attributes (title, description, etc.) were not being translated -- regression from when the old i18n/ tree was removed. The architecture was already set up; just needed the orchestration. Self-healing filter ensures re-runs are no-ops.

Also fixed:

  • Bengali (and similar locales) getting native-script numerals despite existing rule -- strengthened indic/cyrillic group prompts.
  • JSX attribute quote escaping: translations containing inner quotes that match the surrounding attribute quote now escape to &quot;/&apos;.

Spec: PIPELINE-SPEC.md Phase 4b. Tests: jsx-attribute-translator.spec.ts.

myelinated-wackerow and others added 6 commits April 24, 2026 14:39
Co-Authored-By: wackerow <54227730+wackerow@users.noreply.github.com>
- Deprecate old banner, convert TranslationBannerLegal to TranslationBanner
- Utilize doNotTranslatePaths list from intl-pipeline for display boolean logic
- Remove dead code from BaseLayout

Co-Authored-By: wackerow <54227730+wackerow@users.noreply.github.com>
Excluded-list paths are now logged and filtered rather than failing the job. Hard validation errors still throw. Job aborts only if every target path is excluded.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: wackerow <54227730+wackerow@users.noreply.github.com>
Adds normalizeTargetPath: strips accidental translations/<locale>/ and src/intl/<non-en>/ prefixes, auto-prefixes JSON with src/intl/en/ and markdown with public/content/, and tolerates leading "./" or "/". Resolution is now normalize -> exists-check -> validate -> excluded-skip, failing early if any target is missing on disk. log() gains an optional level arg ("log" | "warn" | "error") so Netlify output is consistently tagged.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: wackerow <54227730+wackerow@users.noreply.github.com>
- Moves DO_NOT_TRANSLATE_PATHS to constants (updates casing), where no unrelated env vars will throw errors
- Update local storage key name, including usage of pathname -- removes use of `/en/` prefix and Posix
- Fixes "return early" logic

Co-Authored-By: wackerow <54227730+wackerow@users.noreply.github.com>
/review-translations now defaults to the open PR for intl/pending-dev and reviews only files changed since the last LLM review (read from each PR Review's GitHub-attached commit_id). Pass --full to override and re-review the entire PR. Reviews are submitted via gh pr review --comment, escalating to --approve only when zero critical issues remain. CI workflow updated to match: drops --scope input, adds --full, switches from gh pr comment to gh pr review, aligns trigger with intl/pending- branch naming.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: wackerow <54227730+wackerow@users.noreply.github.com>
@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 27, 2026

Deploy Preview for ethereumorg ready!

Name Link
🔨 Latest commit 6175536
🔍 Latest deploy log https://app.netlify.com/projects/ethereumorg/deploys/69f003bbd563950008290b40
😎 Deploy Preview https://deploy-preview-18043.ethereum.it
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
7 paths audited
Performance: 56 (🟢 up 7 from production)
Accessibility: 93 (no change from production)
Best Practices: 100 (no change from production)
SEO: 98 (🔴 down 1 from production)
PWA: 59 (no change from production)
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-actions github-actions Bot added the tooling 🔧 Changes related to tooling of the project label Apr 27, 2026
@wackerow wackerow marked this pull request as draft April 27, 2026 16:13
@wackerow
Copy link
Copy Markdown
Member

wackerow commented Apr 27, 2026

TO FIX:

  • Markdown attributes not being translated

Copy link
Copy Markdown
Member

@pettinarip pettinarip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

myelinated-wackerow and others added 6 commits April 27, 2026 13:40
Adds Phase 4b: a dedicated pass that translates the values of translatable JSX attributes (title, description, alt, aria-label, etc.) on JSX components in markdown content. Restores behavior that regressed when the old i18n/ tree was removed (a110822) and its leftover gemini.ts attribute helpers were stripped as "dead code" (7f244df). The pipeline architecture had remained set up for it -- TRANSLATABLE_ATTRIBUTES list, normalizer emits component-attribute leaves, deterministic apply path -- but no orchestration drove the leaves through an LLM.

The pass is self-healing: only sends leaves to the LLM whose English value still appears verbatim in the locale file, so re-runs are no-ops on already-translated attrs. Failures throw and skip manifest stamping per spec's "do no harm" partial-failure rule.

Adds a temporary `force_attrs` workflow input (FORCE_ATTRS env) to backfill files in intl/pending-dev that were translated before this pass existed. Self-removable once those files are backfilled.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: wackerow <54227730+wackerow@users.noreply.github.com>
…icals

The standalone sanitizer step pointed to src/scripts/i18n/sanitize-pr.ts (deleted in the i18n-to-intl-pipeline migration). Sanitization now runs as part of the intl-pipeline translation phase, so a separate sanitizer call here is redundant. Removed the step entirely -- review focuses on reviewing.

Adds an explicit note that reporting zero critical issues is acceptable. Counters the asymmetric-reward failure mode where agents fabricate criticals to "show their work."

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: wackerow <54227730+wackerow@users.noreply.github.com>
The flag served its one-shot purpose: backfilling JSX attribute translations on files in intl/pending-dev that pre-dated the Phase 4b pass. Backfill is complete (PR #18051 merged into pending). Going forward, normal pipeline runs handle attribute translation correctly via the self-healing filter, and the flag has no remaining use.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: wackerow <54227730+wackerow@users.noreply.github.com>
Bengali (and likely other indic locales) was getting native-script numerals (১২৩) from Gemini despite the existing rule. Strengthens the indic and cyrillic group rules to "ALWAYS" with explicit script examples, and adds the same rule to the JSX attribute prompt (which had no numeral guidance).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: wackerow <54227730+wackerow@users.noreply.github.com>
Translations containing the same quote character as the surrounding attribute (e.g. an Arabic title="..." whose value quotes an English phrase) produced invalid JSX. Now escapes inner double-quotes to &quot; in double-quoted attrs and inner single-quotes to &apos; in single-quoted attrs. Also handles single-quoted source attrs in both the self-healing filter and the apply path.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: wackerow <54227730+wackerow@users.noreply.github.com>
@wackerow wackerow marked this pull request as ready for review April 28, 2026 00:48
@wackerow wackerow merged commit b006c95 into dev Apr 28, 2026
9 checks passed
@wackerow wackerow deleted the intl-pipeline-v7 branch April 28, 2026 16:04
@wackerow wackerow mentioned this pull request May 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tooling 🔧 Changes related to tooling of the project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants