Skip to content

fix: localize native export validation errors - #1387

Merged
EtienneLescot merged 1 commit into
getopenscreen:mainfrom
songoao25:fix/localize-export-validation-errors
Oct 24, 2026
Merged

EtienneLescot merged 1 commit into
getopenscreen:mainfrom
songoao25:fix/localize-export-validation-errors

Conversation

@songoao25

@songoao25 songoao25 commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Native frame/source validation errors from #1377 and #1379 reached ExportDialog as raw English, including in a Chinese UI. Present these known failures through the existing error panel, notification and editor translations in all 15 supported languages. Keep cancellation silent and preserve unknown bridge errors rather than inventing a diagnosis.

The adapter is intentionally limited to the existing native messages; contract tests catch native wording changes and exercise clip placeholders, decoder context and unknown-error passthrough. Update the export documentation and record the fresh macOS desktop check. No exporter, capture, file-publication or native bridge protocol changes.

Related issue

Follow-up to #1377, #1379 and #1386. Outside scope: the Presenter Overlay capture conflict tracked in issue #1378 remains unresolved.

Type of change

  • Bug fix
  • Documentation

Release impact

  • Patch

Desktop impact

  • Not platform-specific

Screenshots / video

Checked the actual Electron dialog in Simplified Chinese using synthetic media. The panel and notification displayed the same translated error without clipping. No user recordings or screenshots are attached.

Testing

macOS 15.7.1 / Apple M2, Electron 41.2.1, Node 22.22.1, Rust 1.99.1:

  • Before the fix, the new Chinese validation-error UI regression failed: 1 failed / 9 passed. After: focused UI/presentation tests 21 passed.
  • npm run test -- --maxWorkers=2: 362 files, 5011 passed / 2 skipped.
  • npx tsc --noEmit and npx tsc -p tsconfig.test.json --noEmit: passed.
  • npm run i18n:check: passed, all 15 locales; npm run docs:check: passed, 37 documents.
  • Biome on 1128 tracked supported files plus the new files: no errors, 25 existing warnings. Changed files: no diagnostics. The unrestricted npm run lint encounters a nested config in a pre-existing, ignored private baseline checkout; that checkout is not included in this PR.
  • npm run build-vite: passed.
  • Native tests with OPENSCREEN_TEST_MEDIA pointing at generated fixtures and MAC_FFMPEG_DIR/DYLD_LIBRARY_PATH pointing at the local FFmpeg SDK: cargo test --manifest-path crates/Cargo.toml -p openscreen-compositor --test export_timing -- --test-threads=1 15 passed; --lib export_control::tests 9 passed; --lib timeline_walk::tests 10 passed.
  • Additional direct native-addon checks: MP4/GIF each rejected zero-byte input, corrupt input and an empty timeline, preserving a seeded destination with no partial files (6/6). These use synthetic local fixtures.
  • npm run test:e2e -- tests/e2e/gif-export.spec.ts --workers=1: 3 passed (MP4, GIF, GIF cancel/preserve/retry), with the freshly built addon explicitly selected.
  • Computer-use desktop check: gap-start rejection and undecodable-tail output rejection both preserved an existing synthetic destination byte-for-byte, with no partial files; normal MP4 retry succeeded in the same session (120 decoded frames / 4.000 s, full-file decode exit 0). See the dated manual-checklist row for build identity and limits.

Verification limits: no Windows/Linux native runtime or full release-package validation. The macOS addon script compiled successfully but staging stopped on an existing @loader_path in the local SDK; the matching addon/libraries were staged separately for the development test, and process file mappings verified which addon was loaded. Rust sources are unchanged from d433e7ce: repository-wide cargo fmt --check still reports existing formatting differences, and Clippy still reports nine errors in the unchanged native code. These checks are not claimed as passing. Physical recording/Presenter Overlay and transcription were not tested. All-language keys/placeholders are checked automatically; only Simplified Chinese received a desktop visual check.

@coderabbitai

coderabbitai Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 8c99a239-2225-451b-b26e-78ed5a8c682c
Commits

Reviewing files that changed from the base of the PR and between fed3a45 and f6ea84f.

Files selected for processing: 21
  • src/components/ai-edition/ExportDialog.cancel.test.tsx
  • src/components/ai-edition/ExportDialog.tsx
  • src/components/ai-edition/exportError.test.ts
  • src/components/ai-edition/exportError.ts
  • src/i18n/locales/ar/editor.json
  • src/i18n/locales/cs/editor.json
  • src/i18n/locales/de/editor.json
  • src/i18n/locales/en/editor.json
  • src/i18n/locales/es/editor.json
  • src/i18n/locales/fr/editor.json
  • src/i18n/locales/it/editor.json
  • src/i18n/locales/ja-JP/editor.json
  • src/i18n/locales/ko-KR/editor.json
  • src/i18n/locales/pt-BR/editor.json
  • src/i18n/locales/ru/editor.json
  • src/i18n/locales/tr/editor.json
  • src/i18n/locales/vi/editor.json
  • src/i18n/locales/zh-CN/editor.json
  • src/i18n/locales/zh-TW/editor.json
  • technical-documentation/architecture/export-pipeline.md
  • technical-documentation/testing/manual-e2e-checklist.md

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

Walkthrough

The export dialog now maps recognized native export errors to localized messages. The changes add translations across supported locales, tests for the mapping and display, and documentation of error handling.

Changes: Export error presentation

Layer / File(s) Summary
Map known errors to localized messages
src/components/ai-edition/exportError.ts, src/components/ai-edition/exportError.test.ts, src/i18n/locales/*/editor.json
The mapper translates recognized validation, frame, and camera or screen clip errors. Locale files provide the messages, and tests cover the mappings and preservation of unrecognized errors.
Present translated errors in the export dialog
src/components/ai-edition/ExportDialog.tsx, src/components/ai-edition/ExportDialog.cancel.test.tsx, technical-documentation/architecture/export-pipeline.md, technical-documentation/testing/manual-e2e-checklist.md
The dialog uses mapped messages in its error display and failure toast. Tests check localized validation feedback. Documentation describes error handling and records a partial manual test.

Priority: Normal

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

Change: Bug fix

Possibly related issues

  • Issue 1244: This change localizes the no-readable-frames error, but does not address the reported frame drops or niri/NVIDIA DMA-BUF capture.
Merge Risk: Minimal · up to f6ea8

Known export errors reach the localized dialog and notification without an identified blocking issue. The change is ready to merge after normal checks.

Pre-merge checks: Passed 2 · Failed 3

Failed Failed checks: 3 warnings

Check name Status Explanation Resolution
Linked Issues check Warning The directly linked active issue [#1378] requires a clear explanation and guidance when macOS Presenter Overlay interrupts the independent camera recording. It also requires avoiding a silent unusable… Implement the chosen compatibility or recovery policy for [#1378]. Add capture-side detection or handling for the Presenter Overlay conflict, show clear user guidance, prevent silent unusable camera output, and add automated tests for the a…
Out of Scope Changes check Warning The directly linked issue [#1378] concerns macOS Presenter Overlay and independent camera capture. The changed ExportDialog adapter, 15 locale files, export tests, and export documentation address n… Remove the unrelated export-localization changes from this PR, or link and assess them against an active issue whose coding requirements cover native export-validation error presentation.
Docstring Coverage Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 4 files. (17 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 2 passed
Check name Status Explanation
Title check Passed The title "fix: localize native export validation errors" matches the main change. The PR routes known native frame and source validation errors through localized editor translations in ExportDialog, …
Description check Passed The pull request description is complete and follows the repository template. It includes the summary, related issues, change type, release impact, desktop impact, screenshots/video information, and d…
Full details: Title check

Explanation

The title "fix: localize native export validation errors" matches the main change. The PR routes known native frame and source validation errors through localized editor translations in ExportDialog, across all supported locales. The title is concise, uses a conventional fix prefix, and names the primary behavior change.

Full details: Description check

Explanation

The pull request description is complete and follows the repository template. It includes the summary, related issues, change type, release impact, desktop impact, screenshots/video information, and detailed testing results with known limitations.

Full details: Linked Issues check

Explanation

The directly linked active issue [#1378] requires a clear explanation and guidance when macOS Presenter Overlay interrupts the independent camera recording. It also requires avoiding a silent unusable camera track or an implication that a complete recording exists. The reviewed changes only localize native export validation errors in ExportDialog, translations, presentation tests, and documentation. They do not implement or test Presenter Overlay conflict detection, capture handling, or camera recovery.

Resolution

Implement the chosen compatibility or recovery policy for [#1378]. Add capture-side detection or handling for the Presenter Overlay conflict, show clear user guidance, prevent silent unusable camera output, and add automated tests for the affected behavior.

Full details: Out of Scope Changes check

Explanation

The directly linked issue [#1378] concerns macOS Presenter Overlay and independent camera capture. The changed ExportDialog adapter, 15 locale files, export tests, and export documentation address native export-validation error localization instead. The PR description explicitly states that it does not resolve [#1378].

Resolution

Remove the unrelated export-localization changes from this PR, or link and assess them against an active issue whose coding requirements cover native export-validation error presentation.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 4 files. (17 skipped: 17 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

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

@EtienneLescot

Copy link
Copy Markdown
Member

Thanks a lot for this, and for the thorough testing notes! Localized export errors are a real improvement for non-English users. Merging once CI is green.

@EtienneLescot
EtienneLescot merged commit 6e09c9d into getopenscreen:main Oct 24, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants