fix: detect Ollama vision capability from capabilities array#405
Conversation
Package-approach scaffolding to gate pro features (MCP, etc.) without coupling the open core to pro code: - tool extension registry (services/tools/extensions.ts) - screen + settings-section registries - proLicenseService (isPro receipt gate) + bootstrap loadProFeatures that optionally activates @offgrid/pro (stub fallback when absent) - wire generationToolLoop, ChatInput, ChatMessageArea, AppNavigator, SettingsScreen to read the registries - metro resolver for @offgrid/pro with stub fallback - unit + integration tests for the registry and license seam Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
- add private-offgrid as a git submodule at pro/ - repoint metro @offgrid/pro resolver from ../offgrid-pro sibling to ./pro - detect populated submodule via pro/package.json (empty dir when not checked out) Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
…Loop suite Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
- Wire logger.ts → debugLogsStore via setLogListener so JS logs are visible in-app (no more RNFS writes; in-memory ring buffer, 500 entries) - Add Debug Logs button in Settings dev section - Fix critical bug: MCP (and any ToolExtension) system prompt hints were never injected into the generation prompt; models had no idea tools existed. Now both send and regen paths call getSystemPromptHint() on every registered extension. - Simplify debugLogsStore to in-memory only (removes AsyncStorage dep) - Remove stale loadFromStorage call from DebugLogsScreen - Exclude pro/** from root tsconfig so it compiles under its own config - Bump pro submodule to include MCP diagnostic logging commit Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
- Add READ_CALENDAR and WRITE_CALENDAR permissions to AndroidManifest - Add NSCalendarsUsageDescription to iOS Info.plist - Add react-native-add-calendar-event and react-native-calendar-events packages - Update Podfile.lock with iOS calendar pod dependencies - Simplify settings.gradle autolinkLibrariesFromCommand call - Add null guard in handlers.ts: throws descriptive error if calendar native module unavailable Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
…tion loop - extensions.ts: ToolExtension interface and registerToolExtension / getToolExtensions API - registry.ts: resolves tools from both built-in registry and all registered extensions - generationToolLoop.ts: calls extensions on each loop iteration to include MCP tools Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
- Pass chat.enabledTools.length to enabledToolCount (was totalToolCount which included MCP tools) - Tools badge turns amber independently when built-in enabled tools >= 3 - MCP badge turns amber independently when MCP tools >= 3 - Neither badge's warning depends on the other's count Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
- paddingHorizontal: SPACING.lg (was hardcoded) - paddingBottom: SPACING.xl (was hardcoded) Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
…uttons - CustomAlert: add prominentMessage prop rendering message in body typography for readability - showAlert accepts options.prominentMessage to propagate the flag - Context-full dialog: remove OK button, add Settings and New chat actions - Dialog message reordered for clarity: consequence first, then context budget detail Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Includes P1-P5 from the pro package: - Type safety and selector naming fixes - MCP server list card redesign with avatar, toggle and stats row - McpToolsScreen full-screen tool picker - McpServerModal conditional auth dropdown redesign - McpGuideScreen with model recommendations and context tips Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
…reen - showAlert: drop 4th options param (max-params rule); caller spreads prominentMessage onto result - handleCreateCalendarEvent: collapse 5 params into single event object (max-params rule) - ChatScreen useEffect: add goTo to dependency array (react-hooks/exhaustive-deps warning) Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
loadFromStorage and loaded were removed from DebugLogsState. Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
… tests - registry.test.ts: expect 9 tools (was 6) — add send_email, create_calendar_event, read_calendar_events - toolExtensionLoop.test.ts: createConversation takes a modelId and returns the UUID; capture the returned ID instead of passing 'test-conv-1' as the conversationId — addMessage was silently no-oping because no conversation matched the hardcoded string Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
useChatStore and useAppStore inside jest.mock() factory shadowed the top-level imports on lines 19–20. Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
- Remove duplicate getSettingsSections render in SettingsScreen - Add dedup guards to registerScreen and registerSettingsSection - Validate dates before toISOString in calendar tool handlers - Wrap Linking.openURL in try/catch in handleSendEmail - Strip all logger.log/warn from generationToolLoop, useChatGenerationActions, loadProFeatures - Replace proLicenseService with RevenueCat-based implementation - Update proLicenseService tests to match new API Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
- Remove duplicate getSettingsSections render in SettingsScreen - Add dedup guards to registerScreen and registerSettingsSection - Validate dates before toISOString in calendar tool handlers - Wrap Linking.openURL in try/catch in handleSendEmail - Strip all logger.log/warn from generationToolLoop, useChatGenerationActions, loadProFeatures - Replace proLicenseService with RevenueCat-based implementation - Update proLicenseService tests to match new API Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
- Revert registerScreen dedup (existing test documents duplicates are intentional; React Navigation handles duplicate screen registrations) - Use jest.mock with virtual: true for react-native-purchases packages so tests run before npm install is done Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
…ed lib react-native-add-calendar-event uses jcenter() in its build.gradle, which Gradle 9.1 no longer supports, breaking the lint, test, and android-build CI jobs. The package is also deprecated upstream with no fix available. Switch create_calendar_event to RNCalendarEvents.saveEvent() from the already shipped react-native-calendar-events (also used for reading events), drop the deprecated dependency, and add unit tests for the create and read handlers. Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
…nse service - Replace proLicenseService AsyncStorage stub with full RevenueCat implementation (Purchases.getCustomerInfo, presentPaywall, restorePurchases, Keychain persistence) - Fix typeof comparisons: x !== undefined instead of typeof x !== 'undefined' (sonar) - Fix Array.find used for boolean intent: .find() -> .some() in extensions registry (sonar) - Fix nested template literals in handlers: extract query/subjectSuffix/locationSuffix/loc/notes - Replace isNaN() with Number.isNaN() in calendar handlers (sonar) - Replace new Error with new TypeError for type validation in handlers (sonar) - Extract nested ternary for mcpBadgeBg in Popovers (sonar) - Fix array index as React key: use Section.displayName in SettingsScreen (sonar) - Prefer node: protocol in metro.config.js require calls (sonar) - Remove unnecessary as ToolResult type assertions in integration test (sonar) - Gitignore vendored Hexagon HTP .so binaries Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Duplication fixes (4 blocks): - proLicenseService: extract setProInStore() helper, remove 4x repeated require+setState - handlers: extract ensureCalendarPermission(), remove duplicated package/permission check - toolExtensionLoop.test: extract setupProExtension(), collapse 4 identical mock setups - proLicenseService.test: use jest.fn() directly in mock factory, remove lambda wrappers Coverage fixes (+23 branches, 79.74% -> 80.01%): - proLicenseService.test: add tests for presentProPaywall, restorePro, clearProForTesting, configureRevenueCat (ios + android) - toolHandlers.test: add calendar handler tests (create/read, success/error paths, location/notes branches, permission denied, package unavailable) - toolHandlers.test: add web_search tests (no results, with results, missing query) - toolHandlers.test: add send_email tests (with/without subject, Linking failure) - loadProFeatures.test: new test file covering require-throws, null stub, and activate() call paths Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
- Configure RevenueCat at boot and gate the `pro` entitlement before loading pro features; cache the entitlement in the keychain and sync in the background. - Purchase directly via offerings.current so one Lifetime package serves App Store, Play Store, and Web products. - Externalize public SDK keys to src/config/revenueCatKeys.ts (placeholders committed; real keys kept local-only). - Add a global react-native-purchases test mock plus unit and integration tests for the configure -> check -> purchase -> entitlement flow. Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Address Gemini review on PR off-grid-ai#404: - App.tsx: isolate RevenueCat + Pro init in its own try/catch so an RC failure (missing native module, locked keychain, bad config) can never abort app init or hang the splash screen. - proLicenseService: keychain writes no longer throw out of writeLicense. A persist failure after a successful charge would otherwise surface as a bogus 'Purchase failed'/'Restore failed'; the entitlement is still live on RC and the next sync re-writes the cache, so we log and continue. - proLicenseService: select the $rc_lifetime package explicitly instead of availablePackages[0], falling back to the first package, so RC package reordering or future monthly/yearly packages don't break purchase. - ProDetailScreen: collapse the two useTheme() calls into one. Tests: add ProDetailScreen RNTL coverage (purchase/restore/dev-reset states) and unit coverage for the keychain-failure path, package fallback, configure failure, and resetProIdentityForTesting branches. Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Second round of Gemini review on PR off-grid-ai#404: - proLicenseService: track an isConfigured flag set by configureRevenueCat, and skip configuration on platforms react-native-purchases does not support (anything but iOS/Android, e.g. web). presentProPaywall and restorePro now fail loudly, and syncWithRevenueCat / resetProIdentityFor- Testing no-op, when the SDK was never configured, instead of hitting an absent native module. - loadProFeatures: accept an optional pre-read isPro so the boot path can reuse checkProStatus()'s keychain read instead of a second round-trip; standalone callers still fall back to reading the keychain. - App.tsx: pass the checkProStatus() result into loadProFeatures(). Tests: cover the unconfigured guard branches (isolated module), the web platform skip, and the loadProFeatures isPro fast-path. Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
…cat-payments feat: RevenueCat pro license service + cross-platform offering
Inject the current device-local date into the tool system prompt so on-device models can resolve relative dates without chaining a separate datetime tool. A precise minute/second timestamp is used only when a calendar tool is enabled (so "in half an hour" resolves), and a cache-friendly date-only line otherwise, preserving llama.rn prompt-prefix reuse for non-calendar sessions. Make create_calendar_event end_date optional, defaulting to one hour after the start, and tighten the tool/parameter descriptions. Add NSCalendarsFullAccessUsageDescription to the iOS Info.plist so calendar read/create works on iOS 17+ (the library uses the EventKit full-access flow, which requires this key). Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Newer Ollama versions (and models like Gemma 4) report multimodal support via a top-level `capabilities` array (e.g. ["vision", "tools"]) rather than via `model_info` keys. The old code only checked model_info, so these models were always detected as non-vision. Now checks capabilities array first, falls back to model_info key scan, then projector_info keys. Also wires supportsToolCalling from the capabilities array. Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Code Review
This pull request enhances Ollama model capability extraction by parsing the top-level capabilities array for vision and tool calling support, and fallback checking projector_info for multimodal models. Feedback highlights a potential issue where returning supportsToolCalling as undefined for older models could overwrite default capabilities during an object merge, and suggests conditionally including the property only when it is defined.
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.
| /^RENDERER\s/m.test(modelfile); | ||
|
|
||
| return { contextLength, supportsVision, supportsThinking }; | ||
| return { contextLength, supportsVision, supportsToolCalling, supportsThinking }; |
There was a problem hiding this comment.
If supportsToolCalling is undefined (which happens for older Ollama models that do not expose the capabilities array), returning it explicitly as undefined in the object will overwrite the default supportsToolCalling: true when merged via object spread in OpenAICompatibleProvider.updateCapabilities (i.e., { ...this.modelCapabilities, ...capabilities }). This would unintentionally disable tool calling for all older Ollama models.
We should conditionally include supportsToolCalling in the returned object only when it is explicitly defined.
return {
contextLength,
supportsVision,
...(supportsToolCalling !== undefined ? { supportsToolCalling } : {}),
supportsThinking,
};
|
98b32bc to
18ee7f1
Compare



Summary
capabilitiesarray (e.g.["completion", "vision", "tools", "thinking"]) for newer models like Gemma 4model_infokeys for "vision"/"clip" patterns — Gemma 4'smodel_infohas neither, so it was always detected as non-visionsupportsToolCallingfrom thecapabilitiesarray (was previously only detected for LM Studio)Detection priority order (most to least reliable):
capabilitiesarray — new Ollama format, explicitmodel_infokey scan — old format, still works for older modelsprojector_infokey scan — fallback for models with a projector but no capabilities arrayTest plan
huihui_ai/gemma-4-abliterated:12b) — image attachment button should appear in chatllava) — still detected correctly via model_info fallbacknpm test -- --testPathPattern=remoteModelCapabilities(25 tests, all pass)🤖 Generated with Claude Code