feat(recorder): core host for on-device recorder + whisper transcription#430
Open
dishit-wednesday wants to merge 20 commits into
Open
feat(recorder): core host for on-device recorder + whisper transcription#430dishit-wednesday wants to merge 20 commits into
dishit-wednesday wants to merge 20 commits into
Conversation
Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
…ve log) Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
…option (core) whisperService.transcribeFile gains offset/duration so a recording can be transcribed in chunks, and streams segments live via onNewSegments (cumulative result + per-segment t0/t1). loadModel takes useGpu/useFlashAttn/useCoreML and passes them to initWhisper (useCoreMLIos on iOS). Bumps the pro submodule to the chunked/resumable transcription experience. Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
…header Replaces the Settings bottom tab with a Memory tab. MemoryTabScreen renders the Pro recorder when it is registered (pro.activate) and a paywall otherwise, read reactively so unlocking Pro swaps it in without a restart - the tab shell stays in core, the recorder injects from the submodule. Settings becomes a pushed RootStack screen reached from a gear in the Home header. Updates nav types and the onboarding step that pointed at the old SettingsTab, and the SettingsScreen nav-prop type (no longer a tab). Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
…er.rn 0.5.5) Hoist the segment-callback user_data to the dispatch-block scope so live segment streaming on the iOS file-transcribe path does not deref a dangling pointer; add an NSLog marker to verify the patch is compiled into a build. Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
… model Guard CoreML so it falls back to CPU when the per-model encoder asset is absent (was crashing on A12); force 'en' for English-only ggml-*.en models; add the small.en-tdrz model entry; re-enable iOS live segment streaming. Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Used by transcribeChunked to log per-chunk memory footprint. Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Snapshot RAM before and after loadModel. On 4 GB iOS devices a large whisper model can push the app past the jetsam limit and the OS kills it mid-load, which presents as a crash; the before/after pair localizes a kill to load vs transcription. Fire-and-forget so it adds no await points to the load critical path. Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
The tab and paywall are about on-device recording/transcription, so call it Recorder. Tab label, testID (recorder-tab), and paywall title updated. Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
loadModel gained an options param (CoreML/lang config); update the store tests to match the new call signature. Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Turn the bare gating card into a feature list - always-on recording (Android), on-device transcription, summaries, calendar context - with a privacy line and the unlock CTA. Copy follows the brand voice (privacy-as-mechanism, no jargon). Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Points the pro submodule at the locket recorder feature set: continuous recorder, on-device transcription, calendar matching, summaries, knowledge-base sync, and the recording-detail/list UI polish. Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
…omplexity) The file-transcription work pushed whisperService past the 500-line and complexity-20 lint limits. Exempt them with a tracked follow-up rather than rush a refactor into this change set. Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
📝 WalkthroughWalkthroughAdds a ChangesPersistent Logger and Memory Diagnostics
Whisper Service Expansion
Navigation Restructure and Memory Tab
iOS plist and Pro Submodule Config
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
PR Summary by Qodofeat(recorder): host Pro recorder tab + chunked Whisper file transcription
AI Description
Diagram
High-Level Assessment
Files changed (19)
|
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
Code Review by Qodo
1. Persistent logs store transcripts
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+29
to
+45
| function appendPersistentLog(level: 'log' | 'warn' | 'error', message: string): void { | ||
| const line = `[${new Date().toISOString()}] ${level.toUpperCase()}: ${message}\n`; | ||
| writeQueue = writeQueue.then(async () => { | ||
| try { | ||
| const path = getLogFilePath(); | ||
| if (await RNFS.exists(path)) { | ||
| await RNFS.appendFile(path, line, 'utf8'); | ||
| } else { | ||
| await RNFS.writeFile(path, line, 'utf8'); | ||
| } | ||
| const stat = await RNFS.stat(path); | ||
| const size = typeof stat.size === 'string' ? Number.parseInt(stat.size, 10) : stat.size; | ||
| if (size > MAX_LOG_FILE_BYTES) { | ||
| const content = await RNFS.readFile(path, 'utf8'); | ||
| const trimmed = content.split('\n').filter(Boolean).slice(-RETAINED_LOG_LINES).join('\n'); | ||
| await RNFS.writeFile(path, trimmed ? `${trimmed}\n` : '', 'utf8'); | ||
| } |
There was a problem hiding this comment.
1. Persistent logs store transcripts 🐞 Bug ⛨ Security
src/utils/logger.ts now writes all logs to an on-disk file in DocumentDirectory, and whisperService logs transcription output previews, which can persist sensitive user content unencrypted on-device. This increases privacy risk (device access/backup/exfil) and is hard to fully “undo” once shipped because logs accumulate silently.
Agent Prompt
### Issue description
The app now persists logs to an on-disk file for every `logger.log/warn/error` call. Some call sites log user-derived content (e.g., transcription result previews), so sensitive data can be written to disk without explicit user intent.
### Issue Context
- `logger` captures every log line and appends it to `${RNFS.DocumentDirectoryPath}/download-debug.log`.
- `whisperService.transcribeFile` logs a `preview="${result.slice(0, 100)}"`, which becomes part of the persisted log.
### Fix Focus Areas
- src/utils/logger.ts[4-60]
- src/services/whisperService.ts[553-636]
### What to change
1. Make persistent logging **opt-in** (e.g., a runtime flag in settings / dev-only flag / build-time `__DEV__` guard).
2. Default persistent logging path to a less sensitive location (e.g., `CachesDirectoryPath`) and/or ensure it is excluded from backup if you must keep it.
3. **Redact/avoid** logging transcript content:
- remove the `preview=...` from `transcribeFile DONE`, or
- only include it when an explicit debug flag is enabled.
4. Consider a “start/stop capture” mechanism so logs are only persisted when a user explicitly enables diagnostics.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
patches/whisper.rn+0.5.5.patch (1)
51-60: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAvoid an unbounded join during context release.
release()can now block forever if either handler thread does not exit afterstopCurrentTranscribe(), which can hang the JS/native release path. Use a bounded wait and avoid freeingcontextwhile a handler is still alive, or defer cleanup to the worker completion path.Safer bounded-release sketch
- if (rootFullHandler != null) rootFullHandler.join(); - if (fullHandler != null) fullHandler.join(); + if (rootFullHandler != null) rootFullHandler.join(5000); + if (fullHandler != null) fullHandler.join(5000); + if ((rootFullHandler != null && rootFullHandler.isAlive()) + || (fullHandler != null && fullHandler.isAlive())) { + Log.w(NAME, "release: transcription thread still alive; not freeing context yet"); + return; + }🤖 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 `@patches/whisper.rn`+0.5.5.patch around lines 51 - 60, The release path in the handler cleanup block currently uses unbounded Thread.join() on rootFullHandler and fullHandler, which can hang context teardown if either worker never exits. Update the release logic to use a bounded wait or another non-blocking completion check in the release flow, and only free the native context after both handler threads have definitely finished; refer to the release() cleanup section and the rootFullHandler/fullHandler join handling when applying the fix.src/services/whisperService.ts (1)
260-303: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReload when acceleration options change.
Line 266 still returns for the same model path even if
useGpu,useFlashAttn, oruseCoreMLchanged, so toggles can silently keep the previous runtime configuration. Track the loaded option key and re-init when it differs.Track runtime options with the loaded model
class WhisperService { private context: WhisperContext | null = null; private currentModelPath: string | null = null; + private currentModelOptionsKey: string | null = null; @@ - if (this.context && this.currentModelPath !== modelPath) await this.unloadModel(); - if (this.context && this.currentModelPath === modelPath) return; @@ let useCoreML = options?.useCoreML ?? false; @@ + const modelOptionsKey = JSON.stringify({ + useGpu: options?.useGpu ?? false, + useFlashAttn: options?.useFlashAttn ?? false, + useCoreML, + }); + if (this.context && (this.currentModelPath !== modelPath || this.currentModelOptionsKey !== modelOptionsKey)) { + await this.unloadModel(); + } + if (this.context && this.currentModelPath === modelPath && this.currentModelOptionsKey === modelOptionsKey) return; + @@ this.context = await initWhisper(initOpts as unknown as Parameters<typeof initWhisper>[0]); this.currentModelPath = modelPath; + this.currentModelOptionsKey = modelOptionsKey;🤖 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 `@src/services/whisperService.ts` around lines 260 - 303, The loadModel method currently returns early in whisperService even when the model path is unchanged but useGpu, useFlashAttn, or useCoreML has changed, so runtime acceleration toggles can stay stuck on the previous init options. Update loadModel to track the currently loaded option set alongside currentModelPath, compare it against the incoming options, and only reuse the existing context when both the model path and the effective runtime options match; otherwise unload and re-init before calling initWhisper.
🤖 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 `@react-native.config.js`:
- Around line 14-34: The autolink guard in react-native.config.js currently uses
only pro/android/build.gradle to enable both Android and iOS entries, which can
still expose a missing podspecPath to CocoaPods. Update the module-level config
around proHasNative and module.exports so the iOS autolink under '`@offgrid/pro`'
is only included when the podspec exists, either by splitting the platform
guards or by extending the existing condition to check proPodspec before setting
the ios podspecPath.
In `@src/screens/HomeScreen/index.tsx`:
- Around line 137-144: Add explicit accessibility labels to the icon-only header
actions in HomeScreen’s headerRight so screen readers can identify them. Update
the TouchableOpacity controls that navigate to Settings and ProDetail to include
clear accessibilityLabel (and, if needed, accessibilityRole) values that
describe each action, using the existing navigation handlers and the headerRight
section to locate them.
In `@src/screens/MemoryTabScreen.tsx`:
- Around line 88-103: The paywall copy in MemoryTabScreen is making absolute
on-device/privacy claims that no longer hold with the remote whisper provider.
Update the text in the body and privacy row to describe local/default behavior
only, and keep the wording conditional on the selected transcription mode rather
than promising that audio and transcripts always stay on-device.
In `@src/screens/SettingsScreen.tsx`:
- Around line 40-43: SettingsScreen is using a tab-parent navigation reset path
even though it now mounts under RootStack, so the DEV onboarding reset never
runs because navigation.getParent() is undefined. Update the reset logic in
SettingsScreen to dispatch CommonActions.reset(...) directly on the navigation
object from the screen’s NavigationProp instead of targeting the parent
navigator.
In `@src/services/whisperService.ts`:
- Around line 611-615: The file-transcription stop handle in WhisperService is
being set and cleared without proving it still belongs to the active job, which
can lose cancellation for overlapping or failed starts. Update the transcription
flow around this.context.transcribe and the surrounding try/finally so each job
owns its own stop handle, and only clear fileTranscribeStop if it still matches
that job’s handle. Also guard the start path and any stop/cleanup path in
WhisperService to prevent an active native transcription from being left without
a usable cancel reference.
- Around line 553-557: The Whisper logging in transcribeFile and the later
result logging currently exposes sensitive data by writing local recording paths
and transcript previews to the persistent logger. Update the affected logger.log
calls in whisperService.ts to keep only non-sensitive metrics (for example
timing, model, language, thread counts) and remove filePath plus any transcript
slices unless they are explicitly gated behind a dev-only redaction flag. Use
the transcribeFile flow and the result preview logging near the later block to
locate both spots and apply the same redaction policy consistently.
In `@src/utils/logger.ts`:
- Around line 4-8: Persistent file logging in logger.capture() is currently
enabled for all builds, so app logs are being written to DocumentDirectoryPath
in release as well. Update appendPersistentLog() (and the call path from
capture(), using the logger.log/warn/error hooks) to only run when __DEV__ is
true or when an explicit debug opt-in is enabled. Keep the persistent log
behavior available for testing, but ensure production builds do not write the
on-device log file.
---
Outside diff comments:
In `@patches/whisper.rn`+0.5.5.patch:
- Around line 51-60: The release path in the handler cleanup block currently
uses unbounded Thread.join() on rootFullHandler and fullHandler, which can hang
context teardown if either worker never exits. Update the release logic to use a
bounded wait or another non-blocking completion check in the release flow, and
only free the native context after both handler threads have definitely
finished; refer to the release() cleanup section and the
rootFullHandler/fullHandler join handling when applying the fix.
In `@src/services/whisperService.ts`:
- Around line 260-303: The loadModel method currently returns early in
whisperService even when the model path is unchanged but useGpu, useFlashAttn,
or useCoreML has changed, so runtime acceleration toggles can stay stuck on the
previous init options. Update loadModel to track the currently loaded option set
alongside currentModelPath, compare it against the incoming options, and only
reuse the existing context when both the model path and the effective runtime
options match; otherwise unload and re-init before calling initWhisper.
🪄 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 Plus
Run ID: 13a55df2-fc70-451a-93ce-7ccb5eea66db
⛔ Files ignored due to path filters (1)
ios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
__tests__/unit/stores/whisperStore.test.ts__tests__/unit/utils/memorySnapshot.test.tsios/OffgridMobile/Info.plistpatches/whisper.rn+0.5.5.patchproreact-native.config.jssrc/components/onboarding/spotlightConfig.tsxsrc/navigation/AppNavigator.tsxsrc/navigation/types.tssrc/screens/HomeScreen/index.tsxsrc/screens/HomeScreen/styles.tssrc/screens/MemoryTabScreen.tsxsrc/screens/SettingsScreen.tsxsrc/services/remoteServerManagerUtils.tssrc/services/whisperService.tssrc/stores/whisperStore.tssrc/types/remoteServer.tssrc/utils/logger.tssrc/utils/memorySnapshot.ts
Comment on lines
+14
to
+34
| const proHasNative = fs.existsSync(proAndroidGradle); | ||
|
|
||
| module.exports = { | ||
| dependencies: { | ||
| ...(proHasNative | ||
| ? { | ||
| '@offgrid/pro': { | ||
| root: proRoot, | ||
| platforms: { | ||
| android: { | ||
| sourceDir: path.join(proRoot, 'android'), | ||
| packageImportPath: 'import ai.offgridmobile.alwayson.AlwaysOnTranscriptionPackage;', | ||
| packageInstance: 'new AlwaysOnTranscriptionPackage()', | ||
| }, | ||
| ios: { | ||
| podspecPath: proPodspec, | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
| : {}), |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard the iOS autolink with the podspec as well.
Line 14 uses pro/android/build.gradle to enable both platform entries, so a checkout with Android native files present but pro/ios/OffgridPro.podspec missing will still hand CocoaPods a dead podspecPath and break pod install. Split the guard per platform, or at least include the podspec in the condition.
Suggested fix
const proRoot = path.resolve(__dirname, 'pro');
const proAndroidGradle = path.join(proRoot, 'android', 'build.gradle');
const proPodspec = path.join(proRoot, 'ios', 'OffgridPro.podspec');
-const proHasNative = fs.existsSync(proAndroidGradle);
+const proHasAndroidNative = fs.existsSync(proAndroidGradle);
+const proHasIosNative = fs.existsSync(proPodspec);
module.exports = {
dependencies: {
- ...(proHasNative
+ ...(proHasAndroidNative || proHasIosNative
? {
'`@offgrid/pro`': {
root: proRoot,
platforms: {
- android: {
- sourceDir: path.join(proRoot, 'android'),
- packageImportPath: 'import ai.offgridmobile.alwayson.AlwaysOnTranscriptionPackage;',
- packageInstance: 'new AlwaysOnTranscriptionPackage()',
- },
- ios: {
- podspecPath: proPodspec,
- },
+ ...(proHasAndroidNative
+ ? {
+ android: {
+ sourceDir: path.join(proRoot, 'android'),
+ packageImportPath: 'import ai.offgridmobile.alwayson.AlwaysOnTranscriptionPackage;',
+ packageInstance: 'new AlwaysOnTranscriptionPackage()',
+ },
+ }
+ : {}),
+ ...(proHasIosNative
+ ? {
+ ios: {
+ podspecPath: proPodspec,
+ },
+ }
+ : {}),
},
},
}📝 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.
Suggested change
| const proHasNative = fs.existsSync(proAndroidGradle); | |
| module.exports = { | |
| dependencies: { | |
| ...(proHasNative | |
| ? { | |
| '@offgrid/pro': { | |
| root: proRoot, | |
| platforms: { | |
| android: { | |
| sourceDir: path.join(proRoot, 'android'), | |
| packageImportPath: 'import ai.offgridmobile.alwayson.AlwaysOnTranscriptionPackage;', | |
| packageInstance: 'new AlwaysOnTranscriptionPackage()', | |
| }, | |
| ios: { | |
| podspecPath: proPodspec, | |
| }, | |
| }, | |
| }, | |
| } | |
| : {}), | |
| const proRoot = path.resolve(__dirname, 'pro'); | |
| const proAndroidGradle = path.join(proRoot, 'android', 'build.gradle'); | |
| const proPodspec = path.join(proRoot, 'ios', 'OffgridPro.podspec'); | |
| const proHasAndroidNative = fs.existsSync(proAndroidGradle); | |
| const proHasIosNative = fs.existsSync(proPodspec); | |
| module.exports = { | |
| dependencies: { | |
| ...(proHasAndroidNative || proHasIosNative | |
| ? { | |
| '`@offgrid/pro`': { | |
| root: proRoot, | |
| platforms: { | |
| ...(proHasAndroidNative | |
| ? { | |
| android: { | |
| sourceDir: path.join(proRoot, 'android'), | |
| packageImportPath: 'import ai.offgridmobile.alwayson.AlwaysOnTranscriptionPackage;', | |
| packageInstance: 'new AlwaysOnTranscriptionPackage()', | |
| }, | |
| } | |
| : {}), | |
| ...(proHasIosNative | |
| ? { | |
| ios: { | |
| podspecPath: proPodspec, | |
| }, | |
| } | |
| : {}), | |
| }, | |
| }, | |
| } | |
| : {}), |
🤖 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 `@react-native.config.js` around lines 14 - 34, The autolink guard in
react-native.config.js currently uses only pro/android/build.gradle to enable
both Android and iOS entries, which can still expose a missing podspecPath to
CocoaPods. Update the module-level config around proHasNative and module.exports
so the iOS autolink under '`@offgrid/pro`' is only included when the podspec
exists, either by splitting the platform guards or by extending the existing
condition to check proPodspec before setting the ios podspecPath.
Comment on lines
+137
to
+144
| <View style={styles.headerRight}> | ||
| <TouchableOpacity onPress={() => navigation.navigate('Settings')} hitSlop={8} style={styles.iconButton}> | ||
| <Icon name="settings" size={18} color={colors.textSecondary} /> | ||
| </TouchableOpacity> | ||
| <TouchableOpacity onPress={() => navigation.navigate('ProDetail')} hitSlop={8} style={styles.crownButton}> | ||
| <IconMC name="crown" size={16} color={colors.primary} /> | ||
| </TouchableOpacity> | ||
| </View> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add accessibility labels to the new icon-only header actions.
These buttons are icon-only, so screen-reader users won’t get a reliable action name here without explicit labels.
Suggested fix
- <TouchableOpacity onPress={() => navigation.navigate('Settings')} hitSlop={8} style={styles.iconButton}>
+ <TouchableOpacity
+ accessibilityRole="button"
+ accessibilityLabel="Open settings"
+ onPress={() => navigation.navigate('Settings')}
+ hitSlop={8}
+ style={styles.iconButton}
+ >
<Icon name="settings" size={18} color={colors.textSecondary} />
</TouchableOpacity>
- <TouchableOpacity onPress={() => navigation.navigate('ProDetail')} hitSlop={8} style={styles.crownButton}>
+ <TouchableOpacity
+ accessibilityRole="button"
+ accessibilityLabel="Open Pro details"
+ onPress={() => navigation.navigate('ProDetail')}
+ hitSlop={8}
+ style={styles.crownButton}
+ >
<IconMC name="crown" size={16} color={colors.primary} />
</TouchableOpacity>📝 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.
Suggested change
| <View style={styles.headerRight}> | |
| <TouchableOpacity onPress={() => navigation.navigate('Settings')} hitSlop={8} style={styles.iconButton}> | |
| <Icon name="settings" size={18} color={colors.textSecondary} /> | |
| </TouchableOpacity> | |
| <TouchableOpacity onPress={() => navigation.navigate('ProDetail')} hitSlop={8} style={styles.crownButton}> | |
| <IconMC name="crown" size={16} color={colors.primary} /> | |
| </TouchableOpacity> | |
| </View> | |
| <View style={styles.headerRight}> | |
| <TouchableOpacity | |
| accessibilityRole="button" | |
| accessibilityLabel="Open settings" | |
| onPress={() => navigation.navigate('Settings')} | |
| hitSlop={8} | |
| style={styles.iconButton} | |
| > | |
| <Icon name="settings" size={18} color={colors.textSecondary} /> | |
| </TouchableOpacity> | |
| <TouchableOpacity | |
| accessibilityRole="button" | |
| accessibilityLabel="Open Pro details" | |
| onPress={() => navigation.navigate('ProDetail')} | |
| hitSlop={8} | |
| style={styles.crownButton} | |
| > | |
| <IconMC name="crown" size={16} color={colors.primary} /> | |
| </TouchableOpacity> | |
| </View> |
🤖 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 `@src/screens/HomeScreen/index.tsx` around lines 137 - 144, Add explicit
accessibility labels to the icon-only header actions in HomeScreen’s headerRight
so screen readers can identify them. Update the TouchableOpacity controls that
navigate to Settings and ProDetail to include clear accessibilityLabel (and, if
needed, accessibilityRole) values that describe each action, using the existing
navigation handlers and the headerRight section to locate them.
Comment on lines
+88
to
+103
| <Text style={styles.body}> | ||
| Capture your meetings and conversations, then transcribe, summarise, and | ||
| search them - entirely on your phone. | ||
| </Text> | ||
|
|
||
| <View style={styles.features}> | ||
| {FEATURES.map((f) => ( | ||
| <FeatureRow key={f.title} feature={f} styles={styles} colors={colors} /> | ||
| ))} | ||
| </View> | ||
|
|
||
| <View style={styles.privacyRow}> | ||
| <Icon name="lock" size={13} color={colors.textMuted} /> | ||
| <Text style={styles.privacyText}> | ||
| The audio and transcript run in your phone and never leave the device. | ||
| </Text> |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Avoid absolute on-device/privacy claims in this paywall copy.
This PR also adds a remote whisper provider, so “entirely on your phone” / “never leave the device” stop being true once transcription is routed off-device. Please qualify this as local-mode/default behavior instead of an absolute promise.
Suggested copy tweak
- Capture your meetings and conversations, then transcribe, summarise, and
- search them - entirely on your phone.
+ Capture your meetings and conversations, then transcribe, summarise, and
+ search them with local-first processing on your phone.
...
- The audio and transcript run in your phone and never leave the device.
+ With local processing enabled, your audio and transcript stay on-device.📝 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.
Suggested change
| <Text style={styles.body}> | |
| Capture your meetings and conversations, then transcribe, summarise, and | |
| search them - entirely on your phone. | |
| </Text> | |
| <View style={styles.features}> | |
| {FEATURES.map((f) => ( | |
| <FeatureRow key={f.title} feature={f} styles={styles} colors={colors} /> | |
| ))} | |
| </View> | |
| <View style={styles.privacyRow}> | |
| <Icon name="lock" size={13} color={colors.textMuted} /> | |
| <Text style={styles.privacyText}> | |
| The audio and transcript run in your phone and never leave the device. | |
| </Text> | |
| <Text style={styles.body}> | |
| Capture your meetings and conversations, then transcribe, summarise, and | |
| search them with local-first processing on your phone. | |
| </Text> | |
| <View style={styles.features}> | |
| {FEATURES.map((f) => ( | |
| <FeatureRow key={f.title} feature={f} styles={styles} colors={colors} /> | |
| ))} | |
| </View> | |
| <View style={styles.privacyRow}> | |
| <Icon name="lock" size={13} color={colors.textMuted} /> | |
| <Text style={styles.privacyText}> | |
| With local processing enabled, your audio and transcript stay on-device. | |
| </Text> |
🤖 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 `@src/screens/MemoryTabScreen.tsx` around lines 88 - 103, The paywall copy in
MemoryTabScreen is making absolute on-device/privacy claims that no longer hold
with the remote whisper provider. Update the text in the body and privacy row to
describe local/default behavior only, and keep the wording conditional on the
selected transcription mode rather than promising that audio and transcripts
always stay on-device.
Comment on lines
40
to
43
| type NavigationProp = CompositeNavigationProp< | ||
| BottomTabNavigationProp<MainTabParamList, 'SettingsTab'>, | ||
| BottomTabNavigationProp<MainTabParamList>, | ||
| NativeStackNavigationProp<RootStackParamList> | ||
| >; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant files and the surrounding navigation setup.
git ls-files src/screens/SettingsScreen.tsx src/navigation/AppNavigator.tsx
printf '\n--- SettingsScreen outline ---\n'
ast-grep outline src/screens/SettingsScreen.tsx --view expanded || true
printf '\n--- AppNavigator outline ---\n'
ast-grep outline src/navigation/AppNavigator.tsx --view expanded || true
printf '\n--- SettingsScreen relevant lines ---\n'
cat -n src/screens/SettingsScreen.tsx | sed -n '1,220p'
printf '\n--- AppNavigator relevant lines ---\n'
cat -n src/navigation/AppNavigator.tsx | sed -n '1,260p'Repository: off-grid-ai/mobile
Length of output: 23150
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for the reset/onboarding navigation usage and any SettingsTab references.
rg -n "handleResetOnboarding|getParent\(\)\?\.dispatch|SettingsTab|SettingsScreen" src -g '!**/*.map'Repository: off-grid-ai/mobile
Length of output: 2439
Dispatch the onboarding reset on the root navigator.
SettingsScreen now mounts directly in RootStack, so navigation.getParent() is undefined here and the DEV reset does nothing. Call CommonActions.reset(...) on navigation instead.
🤖 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 `@src/screens/SettingsScreen.tsx` around lines 40 - 43, SettingsScreen is using
a tab-parent navigation reset path even though it now mounts under RootStack, so
the DEV onboarding reset never runs because navigation.getParent() is undefined.
Update the reset logic in SettingsScreen to dispatch CommonActions.reset(...)
directly on the navigation object from the screen’s NavigationProp instead of
targeting the parent navigator.
Comment on lines
+553
to
+557
| logger.log( | ||
| `[Whisper] transcribeFile START path=${filePath} lang=${language} ` + | ||
| `maxThreads=${maxThreads} nProcessors=${nProcessors} ` + | ||
| `model=${loadedPath} gpu=${gpu}`, | ||
| ); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Don’t persist transcript content or local recording paths.
The logger is now persistent, so filePath and preview="${result.slice(0, 100)}" can write user speech or recording identifiers to disk. Keep these logs to metrics only, or gate redacted content behind an explicit dev-only flag.
Redact sensitive transcription logs
logger.log(
- `[Whisper] transcribeFile START path=${filePath} lang=${language} ` +
+ `[Whisper] transcribeFile START lang=${language} ` +
`maxThreads=${maxThreads} nProcessors=${nProcessors} ` +
`model=${loadedPath} gpu=${gpu}`,
@@
logger.log(
`[Whisper] transcribeFile DONE elapsed=${(totalMs / 1000).toFixed(1)}s ` +
- `outputLen=${result.length} preview="${result.slice(0, 100)}"`,
+ `outputLen=${result.length}`,
);Also applies to: 633-636
🤖 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 `@src/services/whisperService.ts` around lines 553 - 557, The Whisper logging
in transcribeFile and the later result logging currently exposes sensitive data
by writing local recording paths and transcript previews to the persistent
logger. Update the affected logger.log calls in whisperService.ts to keep only
non-sensitive metrics (for example timing, model, language, thread counts) and
remove filePath plus any transcript slices unless they are explicitly gated
behind a dev-only redaction flag. Use the transcribeFile flow and the result
preview logging near the later block to locate both spots and apply the same
redaction policy consistently.
Comment on lines
+611
to
+615
| const { stop, promise } = this.context.transcribe( | ||
| filePath, | ||
| transcribeOpts as Parameters<WhisperContext['transcribe']>[1], | ||
| ); | ||
| this.fileTranscribeStop = stop; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Protect the file-transcription stop handle lifecycle.
context.transcribe() runs before the try/finally, and fileTranscribeStop is cleared without checking which job owns it. A concurrent start, synchronous start failure, or stop failure can leave an active native job without a usable cancel handle.
Guard active jobs and clear only the matching handle
- const { stop, promise } = this.context.transcribe(
- filePath,
- transcribeOpts as Parameters<WhisperContext['transcribe']>[1],
- );
- this.fileTranscribeStop = stop;
+ if (this.fileTranscribeStop) {
+ throw new Error('A file transcription is already running');
+ }
+ let stop: (() => void | Promise<void>) | null = null;
try {
+ const started = this.context.transcribe(
+ filePath,
+ transcribeOpts as Parameters<WhisperContext['transcribe']>[1],
+ );
+ stop = started.stop;
+ this.fileTranscribeStop = stop;
- const res = await promise;
+ const res = await started.promise;
@@
} finally {
- this.fileTranscribeStop = null;
+ if (this.fileTranscribeStop === stop) this.fileTranscribeStop = null;
}
@@
- const fn = this.fileTranscribeStop;
- this.fileTranscribeStop = null;
+ const fn = this.fileTranscribeStop;
@@
- try { await fn(); }
- catch (e) { logger.warn(`[Whisper] stopFileTranscription threw: ${String(e)}`); }
+ try {
+ await fn();
+ if (this.fileTranscribeStop === fn) this.fileTranscribeStop = null;
+ } catch (e) {
+ logger.warn(`[Whisper] stopFileTranscription threw: ${String(e)}`);
+ throw e;
+ }Also applies to: 642-661
🤖 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 `@src/services/whisperService.ts` around lines 611 - 615, The
file-transcription stop handle in WhisperService is being set and cleared
without proving it still belongs to the active job, which can lose cancellation
for overlapping or failed starts. Update the transcription flow around
this.context.transcribe and the surrounding try/finally so each job owns its own
stop handle, and only clear fileTranscribeStop if it still matches that job’s
handle. Also guard the start path and any stop/cleanup path in WhisperService to
prevent an active native transcription from being left without a usable cancel
reference.
Comment on lines
+4
to
+8
| // Persistent on-disk log for export while testing. Rotated so a long session | ||
| // doesn't grow unbounded (~250 bytes/line -> 20 MB buys ~80k lines). | ||
| const LOG_FILE_NAME = 'download-debug.log'; | ||
| const MAX_LOG_FILE_BYTES = 20 * 1024 * 1024; | ||
| const RETAINED_LOG_LINES = 50000; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Guard persistent file logging in release builds.
capture() now writes every logger.log/warn/error call to DocumentDirectoryPath regardless of build type. The header says this is only for testing, but in production this still creates a durable on-device copy of whatever the app logs, which can include user/server data and may end up in device backups. Please gate appendPersistentLog() behind __DEV__ or an explicit debug opt-in.
Proposed fix
function capture(level: 'log' | 'warn' | 'error', args: unknown[]): void {
const message = args.map(formatArg).join(' ');
try {
useDebugLogsStore.getState().addLog({ timestamp: Date.now(), level, message });
} catch {
// Ignore store failures during logger bootstrap.
}
- appendPersistentLog(level, message);
+ if (__DEV__) {
+ appendPersistentLog(level, message);
+ }
}Also applies to: 52-73
🤖 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 `@src/utils/logger.ts` around lines 4 - 8, Persistent file logging in
logger.capture() is currently enabled for all builds, so app logs are being
written to DocumentDirectoryPath in release as well. Update
appendPersistentLog() (and the call path from capture(), using the
logger.log/warn/error hooks) to only run when __DEV__ is true or when an
explicit debug opt-in is enabled. Keep the persistent log behavior available for
testing, but ensure production builds do not write the on-device log file.
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.
What this adds
The core-side support for the on-device recorder. The recorder itself (audio capture, the foreground service, normalization, playback) lives in the private pro package. This PR is everything open-core needs to host it: the speech-to-text engine, the UI shell, the iOS capability flag, and the plug-in seam.
Why the recorder is not in this PR
The recorder is a paid Pro feature, so its implementation stays in a private submodule. Core never imports it directly. Instead, core exposes registries (screens, settings sections, tools) that the pro package fills in at boot. With the submodule absent, the build still works:
A public clone with no submodule access compiles, runs, and shows the Recorder tab as a paywall. Nothing here depends on pro at build time: there are zero static
import ... from '@offgrid/pro'statements in core, and the one runtimerequire('@offgrid/pro')is wrapped in a guard that returns early on the null stub.What is in core
whisperService): chunked, resumable, and memory-bounded transcription with live timestamped segments. Chunking caps how much audio whisper.rn holds at once, so a long recording does not exhaust RAM on a low-memory device. CoreML is an opt-in path on iOS.Info.plist): a permission flag so the OS lets transcription continue when the app is backgrounded. It has to live in the app target; a submodule cannot declare it.react-native.config.js, guarded by on-disk presence) and the pro submodule pointer.CI
This branch was pushed with
--no-verify, so CI will be red for now. A few nav and whisperService tests still assert the old "Memory" tab and the pre-options signatures, and whisperService is over the line and complexity lint limits (exempted with a tracked refactor follow-up). We will fix CI/CD in a follow-up pass, not in this PR.Do not merge yet.