fix: harden on-device AI lifecycle and agent guidance - #22
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThis PR restructures agent guidance and generated context, adds CI verification workflows, updates Expo and React Native wrappers, centralizes Android capability handling, and hardens Apple model package downloads, storage verification, loading, cancellation, and persistence. ChangesAgent workflows and guidance
Platform wrappers and runtime behavior
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the Locanara on-device AI framework across Apple, Android, Web, and wrapper platforms to align with the ML Kit GenAI Prompt API, clean up sensitive logging, and introduce the review-self Codex skill. Key improvements include adding streaming variants to the Expo and React Native wrappers, implementing robust Android capability probing, and removing simulated model management. The reviewer identified critical issues regarding missing conversationId session tracking in the React Native Android wrapper, a potential correctness bug in the Web module's delta stream parsing, and an opportunity to use Kotlin's idiomatic Mutex.withLock for cleaner synchronization in Locanara.kt.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
hyochan
marked this pull request as ready for review
July 11, 2026 18:06
Member
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.claude/knowledge/apple-intelligence.md (1)
21-73: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the stale FoundationModels signatures in
.claude/knowledge/apple-intelligence.md
session.prewarm()should beprewarm(promptPrefix:).session.append()isn’t aLanguageModelSessionAPI; session history is managed throughTranscript.includeSchemaInPromptbelongs onrespond(..., includeSchemaInPrompt:options:), notGenerationOptions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/knowledge/apple-intelligence.md around lines 21 - 73, Update the FoundationModels API references in the LanguageModelSession and GenerationOptions sections: replace session.prewarm() with prewarm(promptPrefix:), remove session.append() and describe Transcript-based history management, and move includeSchemaInPrompt from GenerationOptions to the respond(..., includeSchemaInPrompt:options:) signature. Preserve the remaining documented APIs.
🧹 Nitpick comments (6)
packages/apple/Sources/ModelManager/ModelDownloader.swift (1)
360-377: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
cancelDownloadhardcodes a 2-asset naming convention instead of using the package's actual asset list.
idsToCancel = [modelId, "\(modelId)-mmproj"]assumes every package has exactly a main model and an-mmprojcompanion.downloadModelitself iteratesmodelInfo.packageAssetsgenerically, so a future 3rd asset (or any asset not following the-mmprojsuffix convention) won't have its in-flightURLSessionDownloadTaskactively cancelled here.Correctness is still preserved for such a case —
didFinishDownloadingToindependently checkscancelledPackages.contains(info.packageModelId)and rejects/cleans up the transfer regardless — but cancellation would no longer be prompt: the outer stream would keep waiting on the un-cancelled asset's transfer to finish naturally before terminating.Since the current registry only defines main+mmproj packages, this is a latent gap rather than an active bug, but worth generalizing before a 3-asset package is added.
♻️ Sketch: track and cancel actual per-package asset ids
- /// Cancel a download in progress - /// - /// Also cancels the companion mmproj download for multimodal models. - /// - Parameter modelId: Model identifier - public func cancelDownload(_ modelId: String) { - queue.sync { - self.cancelledPackages.insert(modelId) - - // Cancel both the main model and its mmproj companion (if any) - let idsToCancel = [modelId, "\(modelId)-mmproj"] - for id in idsToCancel { + /// Cancel a download in progress, including any in-flight package assets. + /// - Parameter modelId: Model identifier + public func cancelDownload(_ modelId: String) { + queue.sync { + self.cancelledPackages.insert(modelId) + + // Cancel every currently active task belonging to this package, + // rather than assuming a fixed main+mmproj naming convention. + let idsToCancel = self.activeTasks.keys.filter { + self.taskInfo[$0]?.packageModelId == modelId + } + for id in idsToCancel { guard let task = activeTasks[id] else { continue } downloadResults[id] = false task.cancel() activeTasks.removeValue(forKey: id) taskInfo.removeValue(forKey: id) progressContinuations.removeValue(forKey: id)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/apple/Sources/ModelManager/ModelDownloader.swift` around lines 360 - 377, Update cancelDownload to derive the assets to cancel from the package’s actual packageAssets list rather than hardcoding modelId and the “-mmproj” suffix. Reuse the same package metadata and asset-ID construction used by downloadModel, then apply the existing task cancellation and cleanup logic to every asset ID, including future non-mmproj assets.packages/android/locanara/src/main/kotlin/com/locanara/engine/ExecuTorchEngine.kt (1)
76-79: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedacted logs still leak
e.messagevia the thrown exception.Across
generate(),generateStreaming(),cancel(),unload(), andcreate(), catch blocks were changed to log generic messages instead of${e.message}. But in the two places that rethrow (Line 129, Line 199, Line 323-324), the originale.messageis still interpolated straight into the newLocanaraException/exception message, so it isn't actually removed from what downstream callers can see/log — only from this file's ownLog.ecall. If the goal is avoiding leakage of native exception content, the mitigation is incomplete; if native exception messages aren't considered sensitive, the redaction elsewhere (Lines 76-79, 105-106, 151-152, 173, 212-213, 229-230, 320) trades away useful crash diagnostics for no real privacy benefit.Consider retaining at least
e::class.simpleName(or a short, non-content-bearing summary) in these logs for observability, since these are native-module/state errors rather than prompt/output content.Also applies to: 105-106, 122-129, 151-152, 173-173, 195-198, 212-213, 229-230, 320-320
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/android/locanara/src/main/kotlin/com/locanara/engine/ExecuTorchEngine.kt` around lines 76 - 79, Update the exception handling in generate(), generateStreaming(), cancel(), unload(), and create() consistently: retain a short non-content-bearing diagnostic such as e::class.simpleName in error logs and in rethrown exception messages, while removing direct e.message interpolation. Apply the same redaction and observability behavior to the listed catch blocks, preserving their existing control flow and exception types.Source: Coding guidelines
libraries/expo-ondevice-ai/src/ExpoOndeviceAiModule.web.ts (2)
604-648: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract duplicated caching and mapping logic into shared helpers.
translateStreaming(lines 614–627) duplicates the translator cache lookup, LRU eviction, and creation logic fromtranslate(lines 532–545). Similarly,rewriteStreaming(lines 658–675) duplicates thetoneMapandlengthMapobjects fromrewrite(lines 709–720). Extracting these into shared helpers would eliminate ~27 lines of copy-paste and prevent the two copies from diverging.♻️ Proposed refactor: shared helpers
+// Module-level constants for rewrite option mapping +const REWRITE_TONE_MAP: Record<string, 'more-casual' | 'more-formal' | 'as-is'> = { + FRIENDLY: 'more-casual', + PROFESSIONAL: 'more-formal', + ELABORATE: 'as-is', + SHORTEN: 'as-is', + EMOJIFY: 'more-casual', + REPHRASE: 'as-is', +}; + +const REWRITE_LENGTH_MAP: Record<string, 'shorter' | 'as-is' | 'longer'> = { + ELABORATE: 'longer', + SHORTEN: 'shorter', +}; + +// Shared translator cache helper +async function getOrCreateTranslator( + sourceLanguage: string, + targetLanguage: string, +): Promise<ChromeTranslator> { + const key = `${sourceLanguage}-${targetLanguage}`; + if (!cachedTranslators.has(key)) { + if (cachedTranslators.size >= MAX_CACHED_TRANSLATORS) { + const oldestKey = cachedTranslators.keys().next().value!; + cachedTranslators.get(oldestKey)?.destroy(); + cachedTranslators.delete(oldestKey); + } + const Translator = getTranslatorAPI(); + if (!Translator) throw new Error('Translator API not available in this browser'); + cachedTranslators.set( + key, + await Translator.create({ sourceLanguage, targetLanguage }), + ); + } + return cachedTranslators.get(key)!; +}Then update both
translateandtranslateStreamingto usegetOrCreateTranslator, and bothrewriteandrewriteStreamingto use the module-level maps.Also applies to: 650-693
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libraries/expo-ondevice-ai/src/ExpoOndeviceAiModule.web.ts` around lines 604 - 648, Extract the duplicated translator cache lookup, LRU eviction, and creation logic from translate and translateStreaming into a shared getOrCreateTranslator helper, then update both methods to use it. Move the duplicated toneMap and lengthMap definitions from rewrite and rewriteStreaming to module-level shared maps, and update both methods to reference those maps without changing existing translation or rewrite behavior.
176-191: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winEmit a final stream event on error to prevent hanging listeners.
If
consumeTextStreamthrows mid-stream (e.g., Chrome API failure), the caller rejects but noisFinal: trueevent is emitted. Listeners waiting for stream completion will hang. Consider wrappingconsumeTextStreamcalls with error handling that emits a terminal event.♻️ Proposed fix: error-aware consumeTextStream wrapper
async function consumeTextStream( eventName: string, stream: AsyncIterable<string>, ): Promise<string> { let accumulated = ''; for await (const chunk of stream) { const text = typeof chunk === 'string' ? chunk : String(chunk); accumulated += text; emitEvent(eventName, {delta: text, accumulated, isFinal: false}); } return accumulated; } + +async function consumeTextStreamSafe( + eventName: string, + stream: AsyncIterable<string>, +): Promise<string> { + try { + return await consumeTextStream(eventName, stream); + } catch (error) { + emitEvent(eventName, {delta: '', accumulated: '', isFinal: true}); + throw error; + } +}Then replace
consumeTextStreamcalls inchatStream,summarizeStreaming,translateStreaming, andrewriteStreamingwithconsumeTextStreamSafe.Also applies to: 505-511, 587-596, 634-642, 680-688
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libraries/expo-ondevice-ai/src/ExpoOndeviceAiModule.web.ts` around lines 176 - 191, Update consumeTextStream to emit a terminal event with isFinal: true when stream iteration fails, while preserving the existing error propagation. Apply this error-aware handling to the consumeTextStream call sites in chatStream, summarizeStreaming, translateStreaming, and rewriteStreaming so listeners always receive completion even when the Chrome API fails.packages/apple/Sources/Personalization/FeedbackCollector.swift (1)
293-302: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winWrap
deleteProfilein a transaction for atomicity.
deleteProfileperforms two DELETEs (feedback, then profile) without a transaction. If the first succeeds and the second fails, feedback rows are orphaned with no parent profile. The refactoredactivateProfilein this PR already usesBEGIN/COMMIT/ROLLBACK, and the AndroiddeleteProfileusesdb.beginTransaction(). The sameclearFeedbackpattern (lines 519-525) has the same gap, though its consequence is less severe (stale timestamp vs. orphaned data).🔒 Suggested fix for `deleteProfile`
public func deleteProfile(_ profileId: String) throws { guard isInitialized else { throw FeedbackError.notInitialized } - // Delete feedback first (due to foreign key) - try execute("DELETE FROM feedback WHERE profile_id = ?;", bindings: [.text(profileId)]) - - // Delete profile - try execute("DELETE FROM profiles WHERE id = ?;", bindings: [.text(profileId)]) + try execute("BEGIN TRANSACTION;") + do { + try execute("DELETE FROM feedback WHERE profile_id = ?;", bindings: [.text(profileId)]) + try execute("DELETE FROM profiles WHERE id = ?;", bindings: [.text(profileId)]) + try execute("COMMIT;") + } catch { + try? execute("ROLLBACK;") + throw error + } logger.info("Deleted feedback profile") }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/apple/Sources/Personalization/FeedbackCollector.swift` around lines 293 - 302, Update deleteProfile to execute both DELETE statements within an explicit transaction, matching the BEGIN/COMMIT/ROLLBACK pattern used by activateProfile: begin before deleting feedback, commit only after deleting the profile, and roll back before propagating any failure. Preserve the existing initialization guard and deletion order.libraries/expo-ondevice-ai/src/index.ts (1)
138-171: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared streaming subscription helper.
chatStream,summarizeStreaming,translateStreaming, andrewriteStreamingall repeat the same subscribe → striponChunk→ native call →setTimeout(0)flush → cleanup flow. A small helper would remove the duplicatedExpoOndeviceAiModule as unknown as ...cast and keep the flush behavior in one place.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libraries/expo-ondevice-ai/src/index.ts` around lines 138 - 171, Extract the repeated streaming lifecycle from chatStream, summarizeStreaming, translateStreaming, and rewriteStreaming into a shared helper that accepts the native operation and options, subscribes to onChunk through the typed EventEmitter interface, strips onChunk before invocation, flushes queued events with setTimeout(0), and removes the subscription in finally. Update each streaming method to use this helper and eliminate the duplicated ExpoOndeviceAiModule cast and cleanup logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@AGENTS.md`:
- Around line 593-599: Remove the entire claude-mem-context block, including its
machine-local session metadata, from the tracked AGENTS.md policy file. Keep the
file limited to repository-wide instructions and do not add generated or local
memory content.
In `@knowledge/_claude-context/context.md`:
- Around line 695-707: Update the source wording in the corresponding
knowledge/internal architecture files to use “Nitro-generated bridges” and
“on-device,” then rerun the knowledge compiler to regenerate context.md. Do not
edit the generated knowledge/_claude-context/context.md output directly.
In
`@libraries/expo-ondevice-ai/android/src/main/java/expo/modules/ondeviceai/ExpoOndeviceAiHelper.kt`:
- Around line 100-109: Wire ProofreadOptions.inputType through the complete
proofread path: update the Nitro spec, React Native TypeScript wrapper, and both
native implementations to pass and honor the value using the existing
proofreadInputType mapping. Ensure VOICE selects ProofreadInputType.VOICE and
the default remains KEYBOARD; alternatively remove inputType consistently from
all public wrapper APIs and native/spec definitions.
In
`@libraries/expo-ondevice-ai/android/src/main/java/expo/modules/ondeviceai/ExpoOndeviceAiModule.kt`:
- Around line 423-451: The chatStream flow must retain the generated
conversation ID from the streamed chunks. Update the collect block around
locanara.chatStream to capture the latest chunk.conversationId, then use that
captured ID in the promise.resolve result instead of parameters.conversationId,
while preserving the existing event payload.
- Around line 221-231: Update the AsyncFunctions getAvailableModels,
getDownloadedModels, and getLoadedModel to reject their promises with the same
ERR_UNSUPPORTED error used by downloadModel, loadModel, and deleteModel, rather
than resolving with empty collections or null. Preserve the existing
unsupported-error format and behavior used by those neighboring methods.
In
`@libraries/react-native-ondevice-ai/android/src/main/java/com/margelo/nitro/ondeviceai/HybridOndeviceAi.kt`:
- Around line 427-459: Update HybridOndeviceAi.chatStream so
NitroChatResult.canContinue is derived from the Locanara streaming result rather
than hardcoded to true. Use the stream’s canContinue value when available, or
capture the final chunk’s value/SDK ChatResult and fall back to false when the
stream indicates completion without continuation.
In `@libraries/react-native-ondevice-ai/src/index.ts`:
- Around line 533-545: Update the style mapping in rewriteStreaming to match
rewrite(): normalize falsy native style values to undefined before returning,
while preserving the existing RewriteOptions["outputType"] cast and all other
result fields.
In `@packages/android/locanara/src/main/kotlin/com/locanara/Locanara.kt`:
- Line 55: Add the `@Volatile` annotation to the geminiNanoInfo property, matching
the existing annotations on deviceCapability and promptApiStatus. Keep its
nullable type and initialization unchanged so reads in getGeminiNanoStatus()
observe writes from publishCapabilityLocked().
In
`@packages/android/locanara/src/main/kotlin/com/locanara/mlkit/AndroidCapability.kt`:
- Around line 204-227: Update MLKitCapabilityProbe.snapshot so
promptClient.checkStatus() uses the same failure handling as the task-feature
probes: preserve cancellation propagation, but catch other errors and map them
to PromptApiStatus.NotAvailable(...). Keep promptClient.close() in the existing
finally block and return the snapshot with the degraded prompt status.
In `@packages/apple/Sources/ModelManager/ModelRegistry.swift`:
- Around line 131-142: Update DeviceCapabilityDetector.recommendModel to derive
its recommendation from ModelRegistry rather than hardcoding gemma-2-2b-it-q4.
Ensure the selected model identifier exists in the registry and satisfies the
registered minMemoryMB requirement, including the 6000 MB Gemma model threshold.
In `@packages/apple/Sources/ModelManager/ModelStorage.swift`:
- Around line 564-593: Update listDownloadedModels to decode each directory’s
lastPathComponent back to the original model ID before passing it to
isModelDownloaded, so storageComponent is applied only once. Use
percent-decoding consistent with storageComponent, preserve valid IDs containing
special or Unicode characters, and skip entries that cannot be decoded safely.
In `@packages/apple/Tests/PersonalizationSecurityTests.swift`:
- Around line 12-15: Update the test cleanup defer block around
FeedbackCollector.shutdown so database shutdown completes before removing
dbPath. Replace the fire-and-forget Task with a synchronous close path, such as
a nonisolated FeedbackCollector method, or use structured concurrency that
awaits shutdown before FileManager.default.removeItem; ensure no cleanup task
outlives the test scope.
In `@packages/apple/Tests/RAGTests.swift`:
- Around line 33-38: Update the deletion test after
vectorStore.deleteCollection(id: adversarialID) to retrieve the adversarial
collection with getCollection(id:) and assert that the result is nil, while
preserving the existing assertions verifying the retained collection and
document remain.
- Around line 21-24: Update the test cleanup defer blocks to ensure
vectorStore.close() is awaited before removing the database file; replace the
fire-and-forget Task pattern with an async teardown or equivalent synchronous
cleanup approach, applying the same fix to both cleanup locations.
In `@scripts/agent/compile-context.ts`:
- Around line 173-217: Normalize all paths produced by path.relative() to POSIX
separators before comparison or serialization. Update readInternalKnowledge() so
actualNames and returned source values use forward slashes, and update
listExternalReferences() similarly, preserving the existing allowlist validation
and ordering.
---
Outside diff comments:
In @.claude/knowledge/apple-intelligence.md:
- Around line 21-73: Update the FoundationModels API references in the
LanguageModelSession and GenerationOptions sections: replace session.prewarm()
with prewarm(promptPrefix:), remove session.append() and describe
Transcript-based history management, and move includeSchemaInPrompt from
GenerationOptions to the respond(..., includeSchemaInPrompt:options:) signature.
Preserve the remaining documented APIs.
---
Nitpick comments:
In `@libraries/expo-ondevice-ai/src/ExpoOndeviceAiModule.web.ts`:
- Around line 604-648: Extract the duplicated translator cache lookup, LRU
eviction, and creation logic from translate and translateStreaming into a shared
getOrCreateTranslator helper, then update both methods to use it. Move the
duplicated toneMap and lengthMap definitions from rewrite and rewriteStreaming
to module-level shared maps, and update both methods to reference those maps
without changing existing translation or rewrite behavior.
- Around line 176-191: Update consumeTextStream to emit a terminal event with
isFinal: true when stream iteration fails, while preserving the existing error
propagation. Apply this error-aware handling to the consumeTextStream call sites
in chatStream, summarizeStreaming, translateStreaming, and rewriteStreaming so
listeners always receive completion even when the Chrome API fails.
In `@libraries/expo-ondevice-ai/src/index.ts`:
- Around line 138-171: Extract the repeated streaming lifecycle from chatStream,
summarizeStreaming, translateStreaming, and rewriteStreaming into a shared
helper that accepts the native operation and options, subscribes to onChunk
through the typed EventEmitter interface, strips onChunk before invocation,
flushes queued events with setTimeout(0), and removes the subscription in
finally. Update each streaming method to use this helper and eliminate the
duplicated ExpoOndeviceAiModule cast and cleanup logic.
In
`@packages/android/locanara/src/main/kotlin/com/locanara/engine/ExecuTorchEngine.kt`:
- Around line 76-79: Update the exception handling in generate(),
generateStreaming(), cancel(), unload(), and create() consistently: retain a
short non-content-bearing diagnostic such as e::class.simpleName in error logs
and in rethrown exception messages, while removing direct e.message
interpolation. Apply the same redaction and observability behavior to the listed
catch blocks, preserving their existing control flow and exception types.
In `@packages/apple/Sources/ModelManager/ModelDownloader.swift`:
- Around line 360-377: Update cancelDownload to derive the assets to cancel from
the package’s actual packageAssets list rather than hardcoding modelId and the
“-mmproj” suffix. Reuse the same package metadata and asset-ID construction used
by downloadModel, then apply the existing task cancellation and cleanup logic to
every asset ID, including future non-mmproj assets.
In `@packages/apple/Sources/Personalization/FeedbackCollector.swift`:
- Around line 293-302: Update deleteProfile to execute both DELETE statements
within an explicit transaction, matching the BEGIN/COMMIT/ROLLBACK pattern used
by activateProfile: begin before deleting feedback, commit only after deleting
the profile, and roll back before propagating any failure. Preserve the existing
initialization guard and deletion order.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0629e510-321c-428d-b30f-299663c50436
⛔ Files ignored due to path filters (6)
bun.lockis excluded by!**/*.locklibraries/react-native-ondevice-ai/nitrogen/generated/android/c++/JNitroChatOptions.hppis excluded by!**/generated/**libraries/react-native-ondevice-ai/nitrogen/generated/android/kotlin/com/margelo/nitro/ondeviceai/NitroChatOptions.ktis excluded by!**/generated/**libraries/react-native-ondevice-ai/nitrogen/generated/ios/swift/NitroChatOptions.swiftis excluded by!**/generated/**libraries/react-native-ondevice-ai/nitrogen/generated/shared/c++/NitroChatOptions.hppis excluded by!**/generated/**scripts/agent/bun.lockis excluded by!**/*.lock
📒 Files selected for processing (127)
.claude/commands/android.md.claude/commands/apple.md.claude/commands/audit-code.md.claude/commands/commit.md.claude/commands/docs.md.claude/commands/gql.md.claude/commands/knowledge-compile.md.claude/commands/locanara.md.claude/commands/resolve-issue.md.claude/commands/review-pr.md.claude/commands/skills-index.md.claude/commands/test.md.claude/commands/verify-all.md.claude/guides/01-overview.md.claude/guides/02-api-naming.md.claude/guides/03-deprecation.md.claude/guides/04-apple-package.md.claude/guides/05-android-package.md.claude/guides/06-gql-package.md.claude/guides/07-versioning.md.claude/guides/08-deployment.md.claude/guides/09-expo-ondevice-ai.md.claude/guides/09-platform-differences.md.claude/guides/10-todo-list.md.claude/guides/11-react-native-ondevice-ai.md.claude/guides/12-flutter-ondevice-ai.md.claude/guides/example-app-structure.md.claude/knowledge/DIGEST.md.claude/knowledge/apple-intelligence.md.claude/knowledge/chrome-built-in-ai.md.claude/knowledge/gemini-nano.md.claude/knowledge/llama-cpp.md.codex/skills/review-self/SKILL.md.codex/skills/review-self/agents/openai.yaml.github/workflows/ci-agent-context.yml.github/workflows/ci-version-mirrors.ymlAGENTS.mdSKILLS_INDEX.mdknowledge/README.mdknowledge/_claude-context/context.mdknowledge/external/foundation-models-api.mdknowledge/external/gemini-nano-api.mdknowledge/external/localllmclient-api.mdknowledge/internal/01-naming-conventions.mdknowledge/internal/02-architecture.mdknowledge/internal/03-coding-style.mdknowledge/internal/04-api-design.mdknowledge/internal/05-git-deployment.mdlibraries/expo-ondevice-ai/android/build.gradlelibraries/expo-ondevice-ai/android/src/main/java/expo/modules/ondeviceai/ExpoOndeviceAiHelper.ktlibraries/expo-ondevice-ai/android/src/main/java/expo/modules/ondeviceai/ExpoOndeviceAiModule.ktlibraries/expo-ondevice-ai/android/src/main/java/expo/modules/ondeviceai/ExpoOndeviceAiSerialization.ktlibraries/expo-ondevice-ai/plugin/src/withOndeviceAi.tslibraries/expo-ondevice-ai/src/ExpoOndeviceAiModule.web.tslibraries/expo-ondevice-ai/src/__mocks__/expo-modules-core.jslibraries/expo-ondevice-ai/src/__tests__/index.test.tslibraries/expo-ondevice-ai/src/__tests__/web-module.test.tslibraries/expo-ondevice-ai/src/index.tslibraries/flutter_ondevice_ai/example/ios/LocanaraLlamaBridge/Sources/LlamaCppBridgeEngine.swiftlibraries/react-native-ondevice-ai/README.mdlibraries/react-native-ondevice-ai/android/gradle.propertieslibraries/react-native-ondevice-ai/android/src/main/java/com/margelo/nitro/ondeviceai/HybridOndeviceAi.ktlibraries/react-native-ondevice-ai/android/src/main/java/com/margelo/nitro/ondeviceai/OndeviceAiHelper.ktlibraries/react-native-ondevice-ai/android/src/test/java/com/margelo/nitro/ondeviceai/OndeviceAiHelperTest.ktlibraries/react-native-ondevice-ai/ios/HybridOndeviceAi.swiftlibraries/react-native-ondevice-ai/ios/OndeviceAiHelper.swiftlibraries/react-native-ondevice-ai/src/__tests__/index.test.tslibraries/react-native-ondevice-ai/src/index.tslibraries/react-native-ondevice-ai/src/specs/OndeviceAi.nitro.tsllms-full.txtllms.txtpackage.jsonpackages/android/locanara/src/main/kotlin/com/locanara/Locanara.ktpackages/android/locanara/src/main/kotlin/com/locanara/builtin/ChatChain.ktpackages/android/locanara/src/main/kotlin/com/locanara/builtin/ClassifyChain.ktpackages/android/locanara/src/main/kotlin/com/locanara/builtin/ExtractChain.ktpackages/android/locanara/src/main/kotlin/com/locanara/builtin/ProofreadChain.ktpackages/android/locanara/src/main/kotlin/com/locanara/builtin/RewriteChain.ktpackages/android/locanara/src/main/kotlin/com/locanara/builtin/SummarizeChain.ktpackages/android/locanara/src/main/kotlin/com/locanara/builtin/TranslateChain.ktpackages/android/locanara/src/main/kotlin/com/locanara/engine/ExecuTorchEngine.ktpackages/android/locanara/src/main/kotlin/com/locanara/mlkit/AndroidCapability.ktpackages/android/locanara/src/main/kotlin/com/locanara/mlkit/MLKitClients.ktpackages/android/locanara/src/main/kotlin/com/locanara/mlkit/MLKitPromptClient.ktpackages/android/locanara/src/main/kotlin/com/locanara/personalization/FeedbackCollector.ktpackages/android/locanara/src/main/kotlin/com/locanara/personalization/PersonalizationManager.ktpackages/android/locanara/src/main/kotlin/com/locanara/rag/RAGManager.ktpackages/android/locanara/src/main/kotlin/com/locanara/rag/RAGQueryEngine.ktpackages/android/locanara/src/main/kotlin/com/locanara/rag/VectorStore.ktpackages/android/locanara/src/test/kotlin/com/locanara/AndroidCapabilityTests.ktpackages/android/locanara/src/test/kotlin/com/locanara/ConversationIdTests.ktpackages/android/locanara/src/test/kotlin/com/locanara/ErrorHandlingTests.ktpackages/apple/Example/LocanaraExample/components/pages/FrameworkShowcase/PipelineDemo.swiftpackages/apple/Sources/BuiltIn/ChatChain.swiftpackages/apple/Sources/BuiltIn/ClassifyChain.swiftpackages/apple/Sources/BuiltIn/ExtractChain.swiftpackages/apple/Sources/BuiltIn/ProofreadChain.swiftpackages/apple/Sources/BuiltIn/RewriteChain.swiftpackages/apple/Sources/BuiltIn/SummarizeChain.swiftpackages/apple/Sources/BuiltIn/TranslateChain.swiftpackages/apple/Sources/Engine/DeviceCapabilityDetector.swiftpackages/apple/Sources/Engine/EngineTypes.swiftpackages/apple/Sources/Engine/LlamaCppBridge.swiftpackages/apple/Sources/Engine/LlamaCppEngine.swiftpackages/apple/Sources/Engine/LocalModelInferenceProvider.swiftpackages/apple/Sources/ModelManager/ModelDownloader.swiftpackages/apple/Sources/ModelManager/ModelManager.swiftpackages/apple/Sources/ModelManager/ModelRegistry.swiftpackages/apple/Sources/ModelManager/ModelStorage.swiftpackages/apple/Sources/Personalization/FeedbackCollector.swiftpackages/apple/Sources/Personalization/PromptOptimizer.swiftpackages/apple/Sources/RAG/EmbeddingEngine.swiftpackages/apple/Sources/RAG/RAGCollectionManager.swiftpackages/apple/Sources/RAG/RAGQueryEngine.swiftpackages/apple/Sources/RAG/VectorStore.swiftpackages/apple/Tests/ModelPackageIntegrityTests.swiftpackages/apple/Tests/PersonalizationSecurityTests.swiftpackages/apple/Tests/RAGTests.swiftpackages/site/package.jsonpackages/site/public/llms-full.txtpackages/site/public/llms.txtpackages/web/src/Locanara.tspackages/web/tests/Locanara.test.tsscripts/agent/compile-context.test.tsscripts/agent/compile-context.tsscripts/agent/lint-markdown.tsscripts/agent/package.json
💤 Files with no reviewable changes (16)
- packages/android/locanara/src/main/kotlin/com/locanara/builtin/ExtractChain.kt
- packages/android/locanara/src/main/kotlin/com/locanara/builtin/SummarizeChain.kt
- packages/apple/Sources/BuiltIn/RewriteChain.swift
- packages/apple/Sources/BuiltIn/SummarizeChain.swift
- packages/android/locanara/src/main/kotlin/com/locanara/builtin/RewriteChain.kt
- packages/android/locanara/src/main/kotlin/com/locanara/builtin/ProofreadChain.kt
- packages/android/locanara/src/main/kotlin/com/locanara/builtin/TranslateChain.kt
- packages/android/locanara/src/main/kotlin/com/locanara/builtin/ClassifyChain.kt
- packages/apple/Sources/BuiltIn/TranslateChain.swift
- packages/apple/Sources/BuiltIn/ChatChain.swift
- packages/android/locanara/src/main/kotlin/com/locanara/builtin/ChatChain.kt
- packages/apple/Sources/RAG/RAGCollectionManager.swift
- packages/apple/Sources/BuiltIn/ExtractChain.swift
- packages/apple/Sources/BuiltIn/ProofreadChain.swift
- packages/apple/Sources/Engine/LocalModelInferenceProvider.swift
- packages/apple/Sources/BuiltIn/ClassifyChain.swift
Member
Author
|
Addressed the remaining review-body findings in 0bb361e.
Verified locally: Apple 177 tests and example build; Android 102 debug + 102 release tests, build, lint, and example assemble; Expo 48 tests, typecheck, lint (0 errors), build, and Prettier; agent-context checks 9 tests and Markdown lint. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libraries/react-native-ondevice-ai/ios/HybridOndeviceAi.swift (1)
268-305: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
chatStreamshould return the realcanContinuevalue instead of hardcodingtrue. This diverges fromchat()and can tell callers a conversation is still open when it isn't.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libraries/react-native-ondevice-ai/ios/HybridOndeviceAi.swift` around lines 268 - 305, The chatStream method currently hardcodes canContinue to true in its NitroChatResult. Update chatStream to obtain and return the same real canContinue value used by chat(), while preserving the existing streaming and final-result behavior.
♻️ Duplicate comments (1)
libraries/react-native-ondevice-ai/android/src/main/java/com/margelo/nitro/ondeviceai/HybridOndeviceAi.kt (1)
436-468: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
canContinueremains hardcoded totrue— previously flagged, still unresolved.Line 466 still returns
canContinue = trueunconditionally inchatStream, matching the exact pattern raised in an earlier review round on these same lines. The non-streamingchat()(line 303) correctly derivescanContinuefromresult.canContinue; the streaming path should do the same once a completion signal is available fromlocanara.chatStream(...).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libraries/react-native-ondevice-ai/android/src/main/java/com/margelo/nitro/ondeviceai/HybridOndeviceAi.kt` around lines 436 - 468, The streaming chatStream implementation still hardcodes NitroChatResult.canContinue to true. Update chatStream and the Locanara stream result handling to capture the completion canContinue value from locanara.chatStream(...), then return that value in the final NitroChatResult while preserving the existing accumulated message and conversationId behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@libraries/expo-ondevice-ai/android/src/main/java/expo/modules/ondeviceai/ExpoOndeviceAiModule.kt`:
- Around line 459-465: Update the chatStream response construction in the Kotlin
bridge so canContinue reflects the actual Locanara stream completion state
instead of always being true. Reuse the existing stream-state value or
completion result available in chatStream, while preserving the message and
conversationId fields.
In `@libraries/expo-ondevice-ai/src/__tests__/web-module.test.ts`:
- Around line 271-272: Remove the duplicated async factory callback declarations
in the create mocks at the visible location and the corresponding locations near
lines 319, 366, and 462. Keep one valid async options callback per factory,
ensuring each declaration is properly closed so the test file parses.
---
Outside diff comments:
In `@libraries/react-native-ondevice-ai/ios/HybridOndeviceAi.swift`:
- Around line 268-305: The chatStream method currently hardcodes canContinue to
true in its NitroChatResult. Update chatStream to obtain and return the same
real canContinue value used by chat(), while preserving the existing streaming
and final-result behavior.
---
Duplicate comments:
In
`@libraries/react-native-ondevice-ai/android/src/main/java/com/margelo/nitro/ondeviceai/HybridOndeviceAi.kt`:
- Around line 436-468: The streaming chatStream implementation still hardcodes
NitroChatResult.canContinue to true. Update chatStream and the Locanara stream
result handling to capture the completion canContinue value from
locanara.chatStream(...), then return that value in the final NitroChatResult
while preserving the existing accumulated message and conversationId behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 999eeae8-802f-415a-8d68-1be500b1170e
⛔ Files ignored due to path filters (16)
libraries/react-native-ondevice-ai/nitrogen/generated/android/c++/JHybridOndeviceAiSpec.cppis excluded by!**/generated/**libraries/react-native-ondevice-ai/nitrogen/generated/android/c++/JHybridOndeviceAiSpec.hppis excluded by!**/generated/**libraries/react-native-ondevice-ai/nitrogen/generated/android/c++/JNitroProofreadInputType.hppis excluded by!**/generated/**libraries/react-native-ondevice-ai/nitrogen/generated/android/c++/JNitroProofreadOptions.hppis excluded by!**/generated/**libraries/react-native-ondevice-ai/nitrogen/generated/android/kotlin/com/margelo/nitro/ondeviceai/HybridOndeviceAiSpec.ktis excluded by!**/generated/**libraries/react-native-ondevice-ai/nitrogen/generated/android/kotlin/com/margelo/nitro/ondeviceai/NitroProofreadInputType.ktis excluded by!**/generated/**libraries/react-native-ondevice-ai/nitrogen/generated/android/kotlin/com/margelo/nitro/ondeviceai/NitroProofreadOptions.ktis excluded by!**/generated/**libraries/react-native-ondevice-ai/nitrogen/generated/ios/NitroOndeviceAi-Swift-Cxx-Umbrella.hppis excluded by!**/generated/**libraries/react-native-ondevice-ai/nitrogen/generated/ios/c++/HybridOndeviceAiSpecSwift.hppis excluded by!**/generated/**libraries/react-native-ondevice-ai/nitrogen/generated/ios/swift/HybridOndeviceAiSpec.swiftis excluded by!**/generated/**libraries/react-native-ondevice-ai/nitrogen/generated/ios/swift/HybridOndeviceAiSpec_cxx.swiftis excluded by!**/generated/**libraries/react-native-ondevice-ai/nitrogen/generated/ios/swift/NitroProofreadInputType.swiftis excluded by!**/generated/**libraries/react-native-ondevice-ai/nitrogen/generated/ios/swift/NitroProofreadOptions.swiftis excluded by!**/generated/**libraries/react-native-ondevice-ai/nitrogen/generated/shared/c++/HybridOndeviceAiSpec.hppis excluded by!**/generated/**libraries/react-native-ondevice-ai/nitrogen/generated/shared/c++/NitroProofreadInputType.hppis excluded by!**/generated/**libraries/react-native-ondevice-ai/nitrogen/generated/shared/c++/NitroProofreadOptions.hppis excluded by!**/generated/**
📒 Files selected for processing (34)
.claude/knowledge/apple-intelligence.mdAGENTS.mdknowledge/_claude-context/context.mdknowledge/internal/01-naming-conventions.mdknowledge/internal/02-architecture.mdlibraries/expo-ondevice-ai/android/src/main/java/expo/modules/ondeviceai/ExpoOndeviceAiModule.ktlibraries/expo-ondevice-ai/src/ExpoOndeviceAiModule.web.tslibraries/expo-ondevice-ai/src/__mocks__/expo-modules-core.jslibraries/expo-ondevice-ai/src/__tests__/index.test.tslibraries/expo-ondevice-ai/src/__tests__/web-module.test.tslibraries/expo-ondevice-ai/src/index.tslibraries/react-native-ondevice-ai/android/src/main/java/com/margelo/nitro/ondeviceai/HybridOndeviceAi.ktlibraries/react-native-ondevice-ai/android/src/main/java/com/margelo/nitro/ondeviceai/OndeviceAiHelper.ktlibraries/react-native-ondevice-ai/android/src/test/java/com/margelo/nitro/ondeviceai/OndeviceAiHelperTest.ktlibraries/react-native-ondevice-ai/ios/HybridOndeviceAi.swiftlibraries/react-native-ondevice-ai/ios/OndeviceAiHelper.swiftlibraries/react-native-ondevice-ai/src/__tests__/index.test.tslibraries/react-native-ondevice-ai/src/index.tslibraries/react-native-ondevice-ai/src/specs/OndeviceAi.nitro.tspackages/android/locanara/src/main/kotlin/com/locanara/Locanara.ktpackages/android/locanara/src/main/kotlin/com/locanara/engine/ExecuTorchEngine.ktpackages/android/locanara/src/main/kotlin/com/locanara/mlkit/AndroidCapability.ktpackages/android/locanara/src/test/kotlin/com/locanara/AndroidCapabilityTests.ktpackages/android/locanara/src/test/kotlin/com/locanara/engine/ExecuTorchEnginePrivacyTests.ktpackages/apple/Sources/Engine/DeviceCapabilityDetector.swiftpackages/apple/Sources/ModelManager/ModelDownloader.swiftpackages/apple/Sources/ModelManager/ModelStorage.swiftpackages/apple/Sources/Personalization/FeedbackCollector.swiftpackages/apple/Tests/EngineIntegrationTests.swiftpackages/apple/Tests/ModelPackageIntegrityTests.swiftpackages/apple/Tests/PersonalizationSecurityTests.swiftpackages/apple/Tests/RAGTests.swiftscripts/agent/compile-context.test.tsscripts/agent/compile-context.ts
💤 Files with no reviewable changes (1)
- AGENTS.md
✅ Files skipped from review due to trivial changes (2)
- knowledge/internal/01-naming-conventions.md
- knowledge/internal/02-architecture.md
🚧 Files skipped from review as they are similar to previous changes (14)
- libraries/expo-ondevice-ai/src/mocks/expo-modules-core.js
- libraries/react-native-ondevice-ai/ios/OndeviceAiHelper.swift
- packages/apple/Tests/RAGTests.swift
- libraries/react-native-ondevice-ai/android/src/test/java/com/margelo/nitro/ondeviceai/OndeviceAiHelperTest.kt
- scripts/agent/compile-context.test.ts
- libraries/react-native-ondevice-ai/android/src/main/java/com/margelo/nitro/ondeviceai/OndeviceAiHelper.kt
- packages/android/locanara/src/main/kotlin/com/locanara/mlkit/AndroidCapability.kt
- packages/apple/Sources/Personalization/FeedbackCollector.swift
- libraries/expo-ondevice-ai/src/index.ts
- libraries/expo-ondevice-ai/src/ExpoOndeviceAiModule.web.ts
- packages/apple/Sources/ModelManager/ModelStorage.swift
- scripts/agent/compile-context.ts
- knowledge/_claude-context/context.md
- packages/android/locanara/src/main/kotlin/com/locanara/Locanara.kt
Member
Author
|
Follow-up review disposition: the chatStream canContinue observations for Expo Android and React Native Android/iOS are false positives under the current core contract. Every successful Apple and Android ChatResult path returns canContinue=true, while ChatStreamChunk has no continuation field; isFinal only marks completion of the current response. Inferring false from isFinal would incorrectly end every normal conversation, and re-running chat would duplicate inference and memory effects. Dynamic continuation semantics should be introduced later as a source-first GraphQL/core/all-wrapper contract change. The alleged duplicate async factories in the Expo Web test are also absent on the current head; Prettier, TypeScript checking, and the focused suite pass (19/19). The two inline threads now contain the detailed evidence and are resolved. |
10 of 11 tasks
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.
Summary
Root cause and impact
Several platform and wrapper layers had drifted from their behavioral sources of truth. Cached or synthetic capability data could advertise unavailable features, model operations could report success without a verified backend, and Apple model files were downloaded from a moving revision without complete package integrity. The external Apple bridge could also register globally before ModelManager revalidated its lifecycle token, allowing a late callback to race deletion.
The bridge now prepares without router side effects and commits only while the token and verified files are current. Existing third-party bridges that implement only the legacy one-phase protocol are rejected with an explicit migration error because adapting them silently would restore the race. Existing Apple external-model installs without the new package commit metadata are treated as unverified and must be downloaded again.
Site CI used Bun 1.1.0, which cannot consume the current text lockfile and re-resolved the caret Prettier range to 3.9.5. Pinning the already lockfile-resolved 3.8.1 makes formatting deterministic without changing a Locanara package version.
Test plan
Known gaps / skipped checks
No package was published, deployed, released, or version-bumped.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores