Repository navigation
Conversation
Comparison refusals, transport errors and malformed payloads appear as No file changes. Decode accepted data, preserve loaded files on failed refresh, and offer Retry.
📝 Walkthrough
Priority: ⚪ Pending latest changes Merge Risk: 🔵 Low · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 4 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly summarizes the primary change: mobile commit comparison failures now appear with a retry action. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Description check The pull request description follows the required template and clearly explains the user impact, implementation, rationale, testing, visual evidence, compatibility considerations, and review status. I… Full details: Description check
-
Explanation
The pull request description follows the required template and clearly explains the user impact, implementation, rationale, testing, visual evidence, compatibility considerations, and review status. It also identifies that required PR CI checks and full native device verification were not complete. The Linked Issue section does not provide an issue reference, but it gives a maintainer-specific explanation that no matching issue was found.
-
✨ Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- 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.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
ce5c1639-a763-4f35-8cce-f8bd7d96087b
📒 Files selected for processing: 5
mobile/rpc-foundation/goldens/matrix-git.history-commit-files-git.commitcompare-1.jsonmobile/src/source-control/MobileGitHistoryList.test.tsxmobile/src/source-control/MobileGitHistoryList.tsxmobile/src/source-control/mobile-git-read-operations.tsmobile/tests-typecheck-baseline.txt
💤 Files with no reviewable changes: 1
- mobile/tests-typecheck-baseline.txt
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.
| ...prev, | ||
| [commitId]: { | ||
| files: prev[commitId]?.files, | ||
| error: err instanceof Error ? err.message : 'Unable to load commit files' |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use a fallback for an empty error message.
When a refusal or transport failure has no message, this assignment stores ''. The render checks treat '' as no error, so the commit stays in its loading state without Retry. The message-less failure checkpoints in mobile/rpc-foundation/goldens/matrix-git.history-commit-files-git.commitcompare-1.json reference the same state as the files-pending checkpoint. Use the fallback when err.message is empty, and update those expectations.
Proposed change
--- "a/mobile/src/source-control/MobileGitHistoryList.tsx"
+++ "b/mobile/src/source-control/MobileGitHistoryList.tsx"
@@ -132,7 +132,10 @@
...prev,
[commitId]: {
files: prev[commitId]?.files,
- error: err instanceof Error ? err.message : 'Unable to load commit files'
+ error:
+ err instanceof Error && err.message
+ ? err.message
+ : 'Unable to load commit files'
}
}))
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| error: err instanceof Error ? err.message : 'Unable to load commit files' | |
| error: | |
| err instanceof Error && err.message | |
| ? err.message | |
| : 'Unable to load commit files' |
There was a problem hiding this comment.
🧹 Nitpick comments: 1
mobile/src/source-control/MobileGitHistoryList.test.tsx: 1
234-259: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winExercise retry recovery after a failed refresh.
The failed-refresh test stops after asserting the retained files and error. It does not press
Retry commit filesor assert that a successful retry replaces the retained files. Add a distinct success response so this test covers the reconnect-refresh recovery path. This is separate from the existing message-less failure coverage.Suggested fix
let attempts = 0 + const recoveredCompareResponse: RpcResponse = { + id: 'compare', + ok: true, + result: { entries: [{ path: 'src/recovered.ts', added: 2, removed: 0 }] } + } const client = createFakeRpcClient() client.sendRequest = vi.fn(async (method): Promise<RpcResponse> => { if (method === 'git.history') { return historyResponse('expandable') } attempts += 1 - return attempts === 1 - ? compareResponse - : { + return attempts === 1 + ? compareResponse + : attempts === 2 + ? { id: 'compare', ok: false, error: { code: 'runtime_error', message: 'Commit refresh failed' } } + : recoveredCompareResponse }) ... expect(tree()).toContain('src/app.ts') expect(tree()).toContain('Commit refresh failed') expect(tree()).not.toContain('No file changes') + const retry = renderer?.root.findAll( + (node) => + String(node.type) === 'Pressable' && + node.props.accessibilityLabel === 'Retry commit files' + )[0] + expect(retry).toBeDefined() + await act(async () => retry?.props.onPress()) + expect(tree()).toContain('src/recovered.ts') + expect(tree()).not.toContain('src/app.ts') })
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
ba008ea2-83da-4e85-bd83-69b973343d0f
📒 Files selected for processing: 2
mobile/tests-typecheck-baseline.txtsrc/renderer/src/components/browser-pane/assemble-chrome/BrowserAddressBar.tsx
💤 Files with no reviewable changes: 1
- mobile/tests-typecheck-baseline.txt
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.
# Conflicts: # src/renderer/src/components/browser-pane/assemble-chrome/BrowserAddressBar.tsx
nwparker
changed the title
ELI5
Open a commit's changed files while the connection is failing, or when the computer refuses the request. The app can say "No file changes" even though it never loaded the result. This fix shows an error with Retry, and keeps any files already shown if refreshing them fails.
What Changed
Why
"No file changes" should mean the comparison succeeded and found none.
Linked Issue
Maintainer-discovered regression. Bounded GitHub symptom and exact-file searches found no exact duplicate. No matching issue or exact PR found in bounded symptom/file searches.
Visual Proof
Captured with the visual-proof skill. Before uses base
15246eb4a45; after uses this PR's standalone commit02c1c4aec61. The scenario passed asserted capture runs 3 times per version before representative screenshots were selected. The captures were visually inspected.Evidence scope: Production MobileGitHistoryList rendered through React Native Web; surrounding header/tabs/footer are fixture chrome. Headless Chromium / React Native Web, not iOS or Android. Real history component and production operation/schema path; sendRequest and reconnect provider are fixtures. Icons use simple glyph stand-ins. This capture covers refusal and retry recovery; transport rejection, malformed payload and failed-refresh preservation rely on separate focused tests.
The hook/transport boundary harnesses are labeled in the captures. These are browser evidence where stated; they do not establish native keyboard, gesture, permission dialog, safe-area or real-network behavior. Full code regressions cover additional cases described above.
Testing
ORCA_BACKGROUND_LAUNCH=1 pnpm --dir mobile test src/source-control/MobileGitHistoryList.test.tsxThe combined mobile change set passed 410 suites / 3,626 tests, production typecheck, the tests-typecheck ratchet, formatting, the changed-code quality gate, and the mobile-web build/asset verifier on macOS. These aggregate results are not a claim that the entire suite was rerun on this isolated branch. PR CI validates the branch against current main.
Recording follow-up
Commit
2c8f4f81b36updates test recordings only; production captures above still show the exact production code on this branch. The commit-comparison matrix golden now records the intended error/retry UI for refusal, transport failure and malformed replies. Both native and bridge matrix replays pass.CI follow-up
Updated with main
f47f632bd7d, retaining main's address-bar refactor. Includes two formatting-only object-argument simplifications in ClientHostedBrowserPagePane to satisfy its file-length limit, plus removal of two unused Portuguese default-zoom entries whose English keys were retired on main. Repository-wide lint and full localization-catalog verification pass locally. The mobile production source used by the attached proof is unchanged.Review
Merge risk: Low. A small change to error handling and retry display; successful comparisons keep the same behavior.
Previously reviewed by a separate agent team; reported issues were fixed and checked again.
Agent skill upstream boundary
Notes
Existing RPC methods and wire compatibility are retained. Review considered desktop host changes, remote execution authority, folder workspaces, and native/web consumers. Native simulator follow-up was limited after the three owned simulators became stuck shutting down; no unrelated simulator/process was reset.
Checklist