fix: download-speed 0 render crash + restore Add Server on NoModelScreen#25
Merged
Merged
Conversation
…odelScreen
Two defects found in post-merge review of code already on main:
1. DownloadManagerScreen/items.tsx guarded the speed label with
`item.downloadSpeed && … && (…)`. downloadSpeed is 0 on the first progress
tick of every download, so the && chain evaluates to the number 0 and React
renders a bare "0" text node outside a <Text> — a hard crash on the real RN
renderer (RNTL's test renderer doesn't enforce this, so it slipped through).
Coerce with `!!(…)` so the row is simply hidden at 0 B/s.
2. NoModelScreen's ModelSelectorModal lost the onAddServer prop in a refactor,
so its "Add Server" button silently just closed the modal instead of opening
the Remote Servers screen. Wire onAddServer to navigation.navigate('RemoteServers').
Test: DownloadManager renders without crashing and hides the speed row when
downloadSpeed is 0.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two defects found in the post-merge adversarial review of code already on
main.🔴 Critical — app crash on download progress
DownloadManagerScreen/items.tsxgated the speed label withitem.downloadSpeed && item.downloadSpeed > 0 && item.status === 'running' && (…).computeDownloadSpeedreturns 0 on the first progress tick of every download, so the&&chain evaluates to the number0, which React renders as a bare text node outside a<Text>→ "Text strings must be rendered within a<Text>component" — a hard crash whenever the Download Manager is open during an active download. RNTL'sreact-test-rendererdoesn't enforce this invariant, so the existing test (which only used a positive speed) never caught it. Fixed by coercing the guard to a boolean (!!(…)), so the row is simply hidden at 0 B/s.🟠 High — "Add Server" no-op on the No-Model screen
A refactor split the chat
ModelSelectorModalinto two instances and droppedonAddServerfrom the one embedded inNoModelScreen. Its "Add Server" button therefore just closed the modal instead of navigating to Remote Servers. Re-wiredonAddServer={() => navigation.navigate('RemoteServers')}.Tests
downloadSpeedis 0.tscand lint clean.Related review follow-ups still open (not in this PR): shared main/mmproj EMA speed state (High), stale speed on retry (Medium),
restoreConversationModelFnorphaned-selection + hydration-race (Medium). mmproj integrity fixes are in #23.