fix(models): close mmproj integrity gaps (catch-up move + size floor)#23
Merged
Conversation
Two defects in the mmproj GGUF integrity validation (#18), found in post-merge review: 1. The catch-up guard (mmproj completes natively before the onComplete listener registers) only checked target existence on move-failure, silently keeping a corrupt-but-present sidecar as a vision projector. It now runs the same read-only validation as the onComplete handler. Both paths share a new reconcileMmProjMoveFailure() so they can't drift again. 2. mmProjFileValid / checkMmProjExists skipped the size check when expectedSize was 0/undefined (reachable on crash-restore), so a 0-byte/truncated stub with an inconclusive magic read passed as valid. Added an absolute MIN_MMPROJ_FILE_SIZE (1 KB) floor, independent of expectedSize, mirroring llmSafetyChecks. Tests: catch-up now downgrades-without-deleting on a corrupt present target, and rejects a sub-1KB stub when expectedSize is unknown.
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
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.
Follow-up to #18. A post-merge adversarial review found two real defects in the mmproj GGUF integrity validation that shipped with #18.
Bugs
Catch-up guard kept corrupt sidecars. The mmproj sidecar can complete natively before the
onCompletelistener registers, so there's a "catch-up guard" path. On move-failure that path only checkedRNFS.exists(target)— if a corrupt-but-present file sat at the target it was silently kept as the vision projector. TheonCompletehandler already validated properly; only the catch-up path was missed. Both paths now sharereconcileMmProjMoveFailure()(read-only validate → reuse if valid, downgrade to text-only if not, never delete), so they can't drift again.Size check bypassed when expected size is unknown.
mmProjFileValid/checkMmProjExistsonly compared sizeif (expectedSize). On crash-restore the reconstructed download can havemmProjFileSize= 0/undefined, so the size check was skipped and a 0-byte/truncated stub with an inconclusive magic read passed as valid. Added an absoluteMIN_MMPROJ_FILE_SIZE(1 KB) floor independent ofexpectedSize, mirroring the existingMIN_GGUF_FILE_SIZEfloor inllmSafetyChecks.Tests
expectedSizeis unknowntscand lint clean.