Repository navigation
Conversation
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between 79a2ad7125f8835ed192b11e50dfab1c79600da2 and 2555857. 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughThe editor now defines shared constants for the Priority: ➖ Normal 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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. Comment |
There was a problem hiding this comment.
ℹ️ No critical issues — minor suggestions below.
Reviewed changes
- Localized too-large copy —
EditorFileLoadErrorView.localizeFileLoadErrornow maps the remotefile_too_largetoken (by.codeor message) to the existing localized stringeditor.fileLoad.fileTooLarge, matching the copydoc-preview-file-reader.tsalready uses for HTTP 413. - New sentinels —
FILE_TOO_LARGE_CODE/FILE_TOO_LARGE_ERRORadded toeditor-panel-content-types.ts;en.jsongainseditor.fileLoad.fileTooLarge, and other locales fall back to English by design. - Tests — two cases in
EditorFileLoadErrorView.test.tsxcover the message-only and code-plus-raw-message paths; both assert the exact localized text and would fail without the new branch, so they pin the behavior rather than passing vacuously.
The end-to-end path checks out: an over-cap remote read returns an RPC envelope with code: 'runtime_error' and message: 'file_too_large' (see runtime-file-client.test.ts:310-340), the loader stores that as loadError/loadErrorCode, and the view's message branch is the one that fires for the reported repro. message === FILE_TOO_LARGE_CODE is exact, but that matches the established convention for this token (binary_file in runtime-file-read-client.ts:61, RUNTIME_TOO_LARGE_ERROR in doc-preview-file-reader.ts:40), so the shared hasRuntimeRpcErrorCode matcher is not needed here.
ℹ️ Remote too-large errors still consume the automatic retry budget
The new sentinel is now known in the view, but shouldRetryFileLoadError only treats the human text file too large (spaces) as terminal — not the machine token file_too_large (underscores) — so the remote case still passes the retry gate and runs the full automatic backoff (250 ms / 1 s / 2.5 s) on a condition that cannot succeed. Local and SSH too-large reads are already terminal, so making the remote token terminal too would drop three redundant multi-megabyte reads and the error-card flicker before the localized text settles. Not blocking — the correct text renders either way.
Technical details
# Treat the remote file_too_large token as terminal in the retry gate
## Affected sites
- `src/renderer/src/components/editor/useEditorPanelFileLoadRetry.ts:52-65` — `shouldRetryFileLoadError` checks `lower.includes('file too large')` but not the machine token `file_too_large`, so the remote case is retried.
- `src/renderer/src/components/editor/useEditorPanelFileLoadRetry.ts:96-126` — once the budget is spent, `terminalFailure` is null for this token, so the effect just returns.
## Required outcome
- A remote `file_too_large` read failure must not enter the automatic retry loop; the localized error card should be terminal, consistent with local/SSH "File too large" reads.
## Suggested approach (optional)
- In `shouldRetryFileLoadError`, return false when `code === FILE_TOO_LARGE_CODE` or the message carries the token, mirroring the existing `WORKTREE_HOST_UNRESOLVED_CODE` terminal check. `FILE_TOO_LARGE_CODE` already lives in `editor-panel-content-types.ts`, which this hook already imports from.ℹ️ Issue #21689 also reports the remote preview cap, not just the raw token
This PR resolves the symptom the issue's title leads with, but the same issue's body reports the remote preview budget (~3.1 MiB) being roughly a third of the local 10 MiB cap, so ordinary 4–8 MiB photos still cannot open on remote hosts. Fixes #21689 will close the issue while that half remains open. Flagging so a maintainer can decide whether the cap is tracked separately or the issue should stay open.
Technical details
# Scope of "Fixes #21689"
## Affected sites
- PR body — declares `Fixes #21689`.
- Issue #21689 — reports both the raw `file_too_large` text and the reduced remote preview cap.
## Required outcome
- Decide whether the remote preview cap is in scope here or tracked/fixed separately; if separate, note the follow-up in the PR body so auto-close does not lose it.DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
79a2ad7 to
2555857
Compare
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Remote too-large reads are now terminal in the retry gate —
shouldRetryFileLoadErrorreturns false forcode === FILE_TOO_LARGE_CODEor a message containing the token, so an over-cap remote read no longer burns the three automatic backoff attempts (250 ms / 1 s / 2.5 s) on a condition that cannot succeed. This addresses the retry-budget rough edge raised in the previous review. - Tests pin the terminal classification —
useEditorPanelFileLoadRetry.test.tsx:110-113cover the bare token, a transport-wrapped token ('remote read failed: file_too_large'), and a code-only shape; each would fail without the new branch.
The reported repro carries code === 'runtime_error' and message === 'file_too_large', so the message substring branch is the one that fires and the code branch is defensive. Ran both affected test files locally — 12 tests pass. No further concerns.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
|
Thank you, @bbingz! Your idea of mapping the host's |

ELI5
When opening a file on a remote server that exceeds the transport preview limit, the editor error card now shows a friendly explanation ("This document is too large for the server to send in full.") instead of exposing the internal raw error string
file_too_large.What Changed
file_too_largeunder "Unable to load file".EditorFileLoadErrorViewtranslatesfile_too_large(keyed by error code or message) to the localized user-facing messageThis document is too large for the server to send in full., aligning with the existing preview protocol copy.FILE_TOO_LARGE_CODEandFILE_TOO_LARGE_ERRORsentinels toeditor-panel-content-types.ts, localized undereditor.fileLoad.fileTooLargeinen.json, and mapped inEditorFileLoadErrorView.tsx.Why
Raw internal machine error codes (
file_too_large) degrade user experience when files exceed remote preview constraints. Orca already uses this readable explanation indoc-preview-file-reader.tsfor HTTP 413 responses; bringing it to the editor error card provides clear, localized guidance.Linked Issue
Fixes #21689
Visual Proof
N/A — localized error text rendering update in existing
EditorFileLoadErrorView.Testing
Ran vitest suite and typechecks:
src/renderer/src/components/editor/EditorFileLoadErrorView.test.tsxsrc/renderer/src/components/editor/EditorContent.test.tsxsrc/renderer/src/components/editor/useEditorPanelFileLoadRetry.test.tsxnode config/scripts/verify-localization-catalog.mjspnpm run check:code-quality:changedpnpm tc:webAI Disclosure
Antigravity (Google DeepMind)
Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Ensure no issues in: Security, Cross-platoform support (Linux, Windows, Mac), Remote SSH, Mobile, general backwards compatibility, performance
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)