Skip to content

fix(main): drop stray tr.json keys and split runtime-file-read-client under max-lines - #34929

Merged
nwparker merged 2 commits into
mainfrom
nwparker/i18n-drop-queued-message-keys
Oct 28, 2026
Merged

nwparker merged 2 commits into
mainfrom
nwparker/i18n-drop-queued-message-keys

Conversation

@nwparker

@nwparker nwparker commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 0 0 0 0
Prod 3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​102 $\color{#cf222e}{\Huge{\mathbf{−}}}$​96 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​6

ELI5

Main's locale catalog check (verify:localization-catalogs) fails because tr.json still has three strings that the English catalog dropped. This PR removes those three strings, so the check passes again.

What Changed

From src/renderer/src/i18n/locales/tr.json only, this removes:

  • components.native-chat.queuedMessages.cancelEditing
  • components.native-chat.queuedMessages.pasteOrDropImages
  • components.native-chat.queuedMessages.saveNeedsContent

#31180 removed them from en.json, but the Turkish catalog still carried them. I compared every locale against en.json (plural suffixes ignored): these three are the only stray keys in any locale. The file was rewritten through a parse round-trip that reproduces it byte-for-byte, so the diff touches only those three entries.

Why

The catalog check treats any key missing from en.json as an error, so main's CI fails until these are gone. Nothing references them anymore.

This PR also splits src/renderer/src/runtime/runtime-file-read-client.ts (304 lines) back under the 300-line lint cap, which also fails main. The remote chunked-download helpers move unchanged into a new sibling file, runtime-remote-file-download.ts, and the exports stay the same. The two fixes land together because each one's CI fails on the other's breakage. This replaces #34930.

Linked Issue

N/A (maintainer, main CI fix).

Visual Proof

N/A: drops unused translation strings; no UI change.

Testing

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

All five localization checks pass on this branch:

  • verify:localization-catalogs
  • verify:localization-catalog
  • verify:localization-runtime-catalog
  • verify:localization-extraction
  • verify:localization-coverage

No test changes are needed; the checks are the test.

Review

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Turkish locale data only. No cross-platform, SSH or mobile impact.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

#31180 removed components.native-chat.queuedMessages.cancelEditing, .pasteOrDropImages and
.saveNeedsContent from en.json; tr.json, added since, still carried them, so the locale catalog
check failed on main. They are the only stray keys in any locale.
Move the remote chunked-download helpers into runtime-remote-file-download.ts so
runtime-file-read-client.ts is back under the 300-line lint cap. No behaviour change.
@nwparker nwparker changed the title fix(i18n): drop queued-message keys #31180 removed from en fix(main): drop stray tr.json keys and split runtime-file-read-client under max-lines Oct 28, 2026
@coderabbitai

coderabbitai Bot commented Oct 28, 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: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 0008abdb-23f1-4221-8836-d5fc13cbb740
Commits

Reviewing files that changed from the base of the PR and between 8377149 and 22496a1.

Files selected for processing: 3
  • src/renderer/src/i18n/locales/tr.json
  • src/renderer/src/runtime/runtime-file-read-client.ts
  • src/renderer/src/runtime/runtime-remote-file-download.ts

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

Walkthrough

The Turkish locale removes three queued-message translations. Remote file download helpers now reside in a dedicated module. The file-read client imports those helpers. The helpers probe chunk-read support, read chunks, download previews, and classify unsupported-preview errors.

Priority: Low

Merge Risk: Minimal · up to 22496

The change removes obsolete translations and relocates download helpers. No material merge risk was identified.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly identifies both primary changes: removing stray Turkish locale keys and splitting runtime-file-read-client to satisfy the max-lines limit.
Description check Passed The description follows the repository template and explains the changes, reasons, testing, visual proof, scope, and checklist. It does not report results for pnpm lint, pnpm typecheck, pnpm test, or …
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.
Full details: Description check

Explanation

The description follows the repository template and explains the changes, reasons, testing, visual proof, scope, and checklist. It does not report results for pnpm lint, pnpm typecheck, pnpm test, or pnpm build, but this is a non-critical omission because CI coverage is noted.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 2 files. (1 skipped: 1 unsupported.)

Resolution

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

Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • 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.

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.

1 participant