Repository navigation
Mobile styling + performance pass, and dev-gated on-device Apple Intelligence provider - #77
Conversation
Carry the NoteEditorToolbar (Markdown formatting, wikilinks, AI text actions) into the new-note screen, mirroring the note editor's editing mode — keyboard-aware scroll + sticky toolbar + wikilink picker. Pure JS change; ships as an EAS Update to 0.1.1 builds.
Unify mobile UI with desktop and introduce a shared design system. Typography: - Add shared type scale + iconSize tokens to theme.ts (display/heading/ title/subtitle/control/body/label/caption/micro) as the single source of truth; migrate ~120 hardcoded font sizes across 21 screens/components. Leave markdown/mermaid render scales, uppercase overlines, and the dense calendar grid as intentional exceptions. Calendar & Graph: - Port desktop graph logic into shared/ui/graph.ts (forces, edge styling, hierarchy) and consume it from the RN WebView + desktop canvases. - Rewrite CalendarView / KnowledgeGraphWebView to match desktop; add the shared SearchField (roomy input + clear-X) to the graph header. - Widen calendar nav chevrons, enlarge period label, align graph pills. Navigation & chrome: - Move the project drill-down into the Projects tab stack (tab bar stays visible; re-tap pops to root); rename the tab dir index -> projects and add a root redirect. Extract shared screen components (src/screens/). - Per-origin back titles (Graph/Calendar/Search) via a back param. - Native header = surface bg; native toolbars for Calendar/Graph mode switches; Sync as a native modal; iCloud SF Symbol sync status button. - Add per-platform toolbar icons; StatusBar style follows scheme. Supporting: - listCardsWithDueDates query + CalendarCard type for the calendar. - Bundle D3 as an offline WebView asset (gen-webview-assets.js). - Publish OTA updates per native platform (publishupdate.sh).
Two hot-path optimizations from the perf audit.
1. Kill N+1 tag queries in list render (JS thread)
tagsForNote()/tagsForCard() each run a synchronous SELECT…IN. They were
called per-row, unmemoised, inside render — so a note list / board / day
list fired one query per item every render (the board re-queried every card
on each drag frame via setHoverColJs).
- Add tagsByRow(): resolves tags for a whole list in ONE query, returning a
Map<rowId, TagRow[]> that preserves per-row order.
- ProjectScreen, DraggableBoard, CalendarView now resolve tags once (memoised
on the rows) and pass them down as props; rows do no DB work on render.
- React.memo NoteRowItem / DraggableCard / CardBody so unaffected rows skip
re-render during a drag; memoise per-column card grouping + stripMarkdown.
- listProjectSummaries(): two grouped GROUP BY queries instead of two
COUNT(*) per project (was 2N queries for the projects list).
2. Lazy-load the ~1 MB inline WebView asset bundles
MermaidView/MathView/KnowledgeGraphWebView inline mermaid/katex/d3 sources as
giant strings. Static imports evaluated (and heap-allocated) those modules at
bundle init — every note open paid the mermaid+katex cost, app start paid d3.
Load them via React.lazy(() => import(...)) behind Suspense so the strings
only materialize when a diagram / math block / the Graph tab actually renders.
type-check + lint clean (4 pre-existing warnings only).
The board tracked the hovered column in JS state (hoverColJs) and called setHoverColJs via runOnJS on every pan frame — re-rendering the whole board (and, before the previous commit, re-querying every card's tags) ~60x/sec during a drag. Extract each column into a memoised BoardColumn whose hover highlight is a useAnimatedStyle reading the shared hoverColId/sourceColId values. The gesture worklets already maintain hoverColId on the UI thread, so the highlight now reacts there with no JS round-trip and no board re-render. Only the lifted card + overlay change on drag start/end (JS state 'dragging'), which no longer ticks per frame. Behaviour unchanged: same long-press lift, same drop-commit (onEnd reads the shared hover/source ids), same visual highlight. type-check + lint clean.
The BoardColumn hover-highlight useAnimatedStyle called withAlpha() (a plain JS helper) on the UI thread, which crashes as soon as a drag runs the worklet. - Precompute the active/idle colours on the JS thread; the worklet now only selects between already-resolved strings. - Move the measured column back to a plain View and render the highlight as an absolutely-positioned Animated.View overlay (pointerEvents none), so the reanimated node is never the target of measureInWindow.
- Chat: React.memo the Bubble. Streaming appends a token via setMessages that only replaces the streaming assistant message object; every prior message keeps its identity, so memo skips re-rendering (and re-parsing MarkdownView for) all of them. Previously every bubble re-parsed markdown on every token. - TagChips: the sheet has no theme-dependent values (tag colours are applied inline), so hoist it to module scope instead of rebuilding a StyleSheet on every render — it renders once per note/card/calendar row. - ConflictBanner: useMemo its themed styles (minor; single, rare instance). type-check + lint clean.
The notes list rendered every note/folder at once in a ScrollView, and each row got a fresh inline onPress closure — so React.memo on NoteRowItem never skipped (any filter keystroke / folder toggle re-rendered all rows). Projects can hold hundreds of notes, so both matter. - Flatten the on-screen content (filtered flat list, or the folder tree walked depth-first skipping collapsed subtrees) into one ListRow[] and render it through a single virtualized FlatList — only visible rows mount. - Stable openNote/toggle callbacks (useCallback); rows build their own onPress from note.id, so the memoised NoteRowItem/FolderRow actually skip re-render when unrelated state changes. - Replace the recursive FolderTree with a flat, memoised FolderRow header (children are now separate flattened rows). type-check + lint clean.
…e view In the nested Projects-tab flow the translucent tab bar overlaps the bottom of the notes hierarchy list and the single-note read view, so the last rows / end of the note couldn't scroll clear of it. Add bottom content padding = base + insets.bottom + tab bar (49pt) when nested; root-stack copies (Search/Graph/Conflicts) omit the tab-bar portion. Content still scrolls behind the translucent bar — this only reserves end-of-scroll space. NoteDetailScreen gains a prop (mirrors ProjectScreen), passed from app/(tabs)/projects/note/[id]; the editor scroll is unchanged (keyboard toolbar drives its own inset).
…-gated) Add an Apple Foundation Models (on-device Apple Intelligence, iOS 26+) chat provider via a new Expo local native module (modules/apple-llm), wrapping the FoundationModels framework. Native tool-calling uses guided generation: each Cairn tool JSON Schema becomes a DynamicGenerationSchema so the model is constrained to valid arguments, and a Swift<->JS continuation bridge executes tools locally (TOOL_MAP -> expo-sqlite) mid-turn. Persistent per-thread LanguageModelSession keeps the transcript natively; short system prompt + lean schemas to fit the 4096-token window. The on-device model proved too weak / context too small for Cairn's agentic tool use, so the provider is HIDDEN from end users: gated behind EXPO_PUBLIC_APPLE_LLM_DEV, set only in the local git-ignored .env. EAS builds don't get the flag, so it never appears or resolves in shipped apps (the native module still compiles, dormant). Kept in place for local iteration; the plan is to re-implement against Private Cloud Compute on iOS 27 once the managed entitlement is granted. - modules/apple-llm: Swift module (session cache, streaming, tool bridge, schema parser), typed JS bridge - src/chat/providers/apple.ts: ChatProvider (native tools, tool-executed events, persistent session, dev gate) - providers/types.ts: tool-executed StreamEvent (provider ran the tool; agent must not re-run) - ai-config/providers/index: ProviderPref adds 'apple'; resolve/hasProvider gate - AiSettingsForm: dev-gated 'On-device' segment - @modules/* path alias (tsconfig + babel) - docs/plans/apple-foundation-models-provider.md: full reimplementation plan (PCC) Verified: Swift compiles (xcodebuild AppleLlm scheme, iOS 26.5 SDK); type-check + lint clean. Not yet exercised end-to-end on an iOS 26 device.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Apple on-device AI and semantic indexing support, restructures navigation around shared screens, introduces calendar and graph features, and rolls out shared sheet, typography, and board/search rendering updates. ChangesApple AI and Semantic Indexing
Navigation Restructure and Screen Extraction
Knowledge Graph and Calendar Features
Design System Rollout and Board Optimization
Estimated code review effort: 5 (Critical) | ~180 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (11)
mobile/app/(tabs)/calendar/index.tsx (1)
34-61: 🧹 Nitpick | 🔵 TrivialLGTM!
Stack.Toolbar/Stack.Toolbar.Menu/Stack.Toolbar.MenuAction/Stack.Toolbar.Labelusage matches the documented expo-router API shape, and accessibility labels are properly set on the toolbar controls.Note this is an alpha API (per Expo docs, subject to breaking changes across SDK releases), so it's worth keeping an eye on future expo-router upgrades for breaking changes to this surface.
🤖 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 `@mobile/app/`(tabs)/calendar/index.tsx around lines 34 - 61, No code changes are needed here: the Stack.Toolbar, Stack.Toolbar.Menu, Stack.Toolbar.MenuAction, and Stack.Toolbar.Label usage already matches the expo-router API shape, and the accessibility labels are set correctly. Keep this calendar toolbar implementation as-is, and just be aware that the Stack.Toolbar API is alpha and may require updates when upgrading expo-router in the future.mobile/src/components/CalendarView.tsx (1)
504-504: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInconsistent use of typography tokens vs. raw fontSize.
This file already imports and uses
typeScale(e.g.typeScale.control,typeScale.title,typeScale.label) for several styles, butoverdueWas,weekday,dayNum,chipText, andmoreTextfall back to raw numericfontSizevalues (10, 9.5, 10.5). This is inconsistent within the same file and diverges from the PR's shared mobile typography-scale rollout (cohort 23) that other components adopted.♻️ Suggested approach
- overdueWas: { fontSize: 10, color: t.textTertiary, paddingHorizontal: 4 }, + overdueWas: { ...typeScale.micro, color: t.textTertiary, paddingHorizontal: 4 }, ... - weekday: { - flex: 1, - textAlign: "center", - fontSize: 10, - fontWeight: "700", + weekday: { + flex: 1, + textAlign: "center", + ...typeScale.micro, + fontWeight: "700", textTransform: "uppercase",Adjust to whichever
typeScaletoken best matches the intended size (micro/caption, etc.).Also applies to: 507-518, 541-541, 553-554
🤖 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 `@mobile/src/components/CalendarView.tsx` at line 504, Replace the raw numeric fontSize usage in CalendarView’s style definitions with shared typeScale tokens to match the rest of the file and the mobile typography rollout. Update the styles for overdueWas, weekday, dayNum, chipText, and moreText to use the appropriate typeScale values (for example micro/caption-like tokens) instead of hardcoded 9.5–10.5 sizes, keeping the existing color and spacing intact.src/components/graph/ForceGraphCanvas.tsx (1)
279-279: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
collideRadiushere to avoid re-hardcoding the+12constant.This inline
(radiusOf(n) + 12) * spacingduplicates the exact formula the sharedcollideRadius(type, spacing)now owns (the rebuild path already uses it at Line 237). If the shared collision padding ever changes, this spacing-update path silently diverges.♻️ Proposed change
- (sim.force("collide") as d3.ForceCollide<SimNode>)?.radius((n) => (radiusOf(n) + 12) * spacing); + (sim.force("collide") as d3.ForceCollide<SimNode>)?.radius((n) => collideRadius(n.nodeType, spacing));🤖 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/components/graph/ForceGraphCanvas.tsx` at line 279, The collision radius update in ForceGraphCanvas is duplicating the shared padding formula instead of reusing collideRadius(type, spacing). Update the sim.force("collide") radius callback to call the shared collideRadius helper so the spacing-update path stays consistent with the rebuild path and does not hardcode the +12 constant. Use the existing collideRadius symbol in ForceGraphCanvas to locate and replace the inline calculation.mobile/src/components/KnowledgeGraphWebView.tsx (1)
139-221: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftAvoid rebuilding the WebView for runtime toggles
htmlis recreated onshowHulls/labelModechanges, andWebViewreloads wheneversource={{ html }}changes.drawHulls()andlabelVisible()already read those flags inside the page, so update them viainjectJavaScriptor messaging instead of remounting; keep the HTML rebuild only for search-driven node-set changes.🤖 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 `@mobile/src/components/KnowledgeGraphWebView.tsx` around lines 139 - 221, The KnowledgeGraphWebView HTML is being rebuilt on runtime toggle changes, which causes the WebView to reload unnecessarily. Update the `useMemo` that builds `html` and the `WebView` usage so `showHulls` and `labelMode` are applied via `injectJavaScript` or message handling inside `drawHulls()` and `labelVisible()` instead of changing `source={{ html }}`; keep rebuilding `html` only when the filtered node/link data changes.mobile/src/components/BottomSheet.tsx (2)
156-163: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded raw color instead of a theme token.
withAlpha("#000000", 0.4)hardcodes a raw hex value rather than deriving from a theme token. SinceThemehas no scrim/overlay token, consider adding one (e.g.t.scrimor reusingt.textPrimary-derived overlay) so backdrop dimming stays consistent with the theming system rather than a magic literal.♻️ Suggested approach
- backgroundColor: withAlpha("`#000000`", 0.4), + backgroundColor: withAlpha(t.scrim ?? t.textPrimary, 0.4),Requires adding a
scrimtoken to theThemeinterface/implementation.As per coding guidelines, "All colours must use CSS variables (e.g.,
var(--background),var(--accent),var(--text-primary)) and never raw Tailwind colour names" / "never hardcodergba()" — the equivalent RN-token principle applies here since a raw hex literal bypasses the theme system.🤖 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 `@mobile/src/components/BottomSheet.tsx` around lines 156 - 163, The backdrop style in BottomSheet uses a hardcoded raw color via withAlpha("`#000000`", 0.4) instead of a theme token, so update the backdrop styling to derive its dimming color from Theme. Add a scrim/overlay token to the Theme interface and implementation (or reuse an existing theme-derived token path) and switch the BottomSheet backdrop style to use that token so the overlay remains consistent with the theming system.Source: Coding guidelines
125-151: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCancel/Done buttons lack accessible role/label.
Unlike other interactive controls in this rollout (e.g.
NoteEditorToolbar's formatting buttons useaccessibilityRole="button"+accessibilityLabel),BottomSheetHeader's Cancel/DonePressables have neither, so screen readers may not announce them as buttons.♻️ Suggested fix
- <Pressable onPress={onCancel} hitSlop={12}> + <Pressable onPress={onCancel} hitSlop={12} accessibilityRole="button" accessibilityLabel="Cancel"> <Animated.Text style={styles.cancel}>Cancel</Animated.Text> </Pressable> <Animated.Text style={styles.title}>{title}</Animated.Text> {onDone ? ( - <Pressable onPress={onDone} hitSlop={12}> + <Pressable onPress={onDone} hitSlop={12} accessibilityRole="button" accessibilityLabel="Done"> <Animated.Text style={styles.done}>Done</Animated.Text> </Pressable>🤖 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 `@mobile/src/components/BottomSheet.tsx` around lines 125 - 151, The Cancel/Done Pressable controls in BottomSheetHeader are missing accessibility metadata, so update the BottomSheetHeader component to give both interactive elements an accessibilityRole of button and clear accessibilityLabel values matching their actions. Use the existing Pressable elements in BottomSheetHeader as the fix point, and keep the labels consistent with the visible text so screen readers announce them correctly.mobile/app/note/new.tsx (1)
56-119: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate editor logic vs.
NoteDetailScreen.
onSelectionChange,onFormat,onWikilink, andonAIActionhere are essentially copy-pasted fromNoteDetailScreen.tsx(same state shape, same logic, same error-alert copy). Extracting a shared hook (e.g.useNoteFormattingToolbar(body, setBody)returning{ bodyRef, selection, selectionRef, onSelectionChange, hasSelection, onFormat, onWikilink, wikilinkOpen, setWikilinkOpen, aiLoading, onAIAction }) would let both screens share one implementation instead of risking drift when one copy gets fixed/extended and the other doesn't.♻️ Sketch of a shared hook
// mobile/src/hooks/useNoteFormattingToolbar.ts export function useNoteFormattingToolbar(body: string, setBody: (b: string) => void) { const bodyRef = useRef<TextInput>(null); const [selection, setSelection] = useState<Selection>({ start: 0, end: 0 }); const selectionRef = useRef<Selection>({ start: 0, end: 0 }); const [aiLoading, setAiLoading] = useState(false); const [wikilinkOpen, setWikilinkOpen] = useState(false); // ...move onSelectionChange/onFormat/onWikilink/onAIAction here, returning the same API... }Then both
NewNoteandNoteDetailScreencalluseNoteFormattingToolbar(body, setBody).🤖 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 `@mobile/app/note/new.tsx` around lines 56 - 119, The note editor callbacks in new.tsx are duplicated from NoteDetailScreen, so move the shared selection/formatting/AI workflow into a reusable hook. Extract the logic behind onSelectionChange, onFormat, onWikilink, onAIAction, hasSelection, selectionRef, wikilinkOpen, and aiLoading into a hook such as useNoteFormattingToolbar(body, setBody), then update both NewNote and NoteDetailScreen to consume that single implementation. Keep the returned API identical so both screens use the same behavior and stay in sync.mobile/src/screens/CardDetailScreen.tsx (1)
223-223: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInconsistent typography:
labelstyle bypasses the shared type scale.Every other text style in this file (
titleInput,priorityText,descToggle, etc.) spreads atypeScale.*token, butlabelhardcodesfontSize/fontWeightdirectly.♻️ Suggested fix
- label: { fontSize: 12, fontWeight: "700", color: t.textTertiary, textTransform: "uppercase", letterSpacing: 0.5, marginTop: 18, marginBottom: 8 }, + label: { ...typeScale.micro, fontWeight: "700", color: t.textTertiary, textTransform: "uppercase", letterSpacing: 0.5, marginTop: 18, marginBottom: 8 },🤖 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 `@mobile/src/screens/CardDetailScreen.tsx` at line 223, The `label` text style in `CardDetailScreen` is bypassing the shared type scale used by the other styles in this file. Update the `label` entry in the style object to spread the appropriate `typeScale` token and keep only the remaining spacing/color overrides, matching the pattern used by `titleInput`, `priorityText`, and `descToggle`.mobile/src/screens/NoteDetailScreen.tsx (1)
30-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
TAB_BAR_BASEconstant duplicated across files.The same
const TAB_BAR_BASE = 49;is redefined identically inProjectScreen.tsx. Centralizing it (e.g. intheme.ts, alongside the newiconSize/typography tokens) would prevent drift between the two nested-padding computations — exactly the kind of divergence flagged above for the editor toolbar'sbottomInset.🤖 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 `@mobile/src/screens/NoteDetailScreen.tsx` at line 30, The TAB_BAR_BASE constant is duplicated in NoteDetailScreen and ProjectScreen, so move it to a shared theme/token location (for example alongside the new iconSize and typography tokens in theme.ts) and update both screens to read from that shared source. Use the shared constant in the nested padding/bottom inset calculations so both screens stay in sync and do not drift apart over time.mobile/src/screens/ProjectScreen.tsx (1)
293-293: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded
#fffinstead of a theme color token.The active tag chip's text color is hardcoded as
"#fff", while the analogous priority chip inCardDetailScreenusest.accentFgfor text on a colored background. As per coding guidelines, colors should route through design tokens rather than raw hardcoded values, so consider using the theme's foreground token here for consistency (note: this doesn't apply totag.coloritself, which is a legitimate persisted data value).♻️ Suggested fix
- <Text style={[styles.tagFilterText, { color: active ? "`#fff`" : t.textSecondary }]} numberOfLines={1}> + <Text style={[styles.tagFilterText, { color: active ? t.accentFg : t.textSecondary }]} numberOfLines={1}>🤖 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 `@mobile/src/screens/ProjectScreen.tsx` at line 293, The active tag chip text color in ProjectScreen is using a hardcoded white value instead of a theme token. Update the Text styling in the tag filter render path to use the same theme foreground token pattern used by CardDetailScreen for the priority chip, so the active state color comes from t.accentFg (or the equivalent theme foreground token) rather than "`#fff`"; keep tag.color unchanged since it is persisted data.Source: Coding guidelines
mobile/modules/apple-llm/ios/AppleLlmSchemaParser.swift (1)
116-120: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winOff-by-one for fractional
exclusiveMaximumon integer schemas.
Int(exclusiveMax) - 1is correct whenexclusiveMaximumis integer-valued (e.g.5.0→4), but under-restricts when it's fractional (e.g.5.5on anintegertype should yield max5, but this yields4). NoteexclusiveMinimum'sInt(exclusiveMin) + 1doesn't have this asymmetry.🔧 Suggested fix
if let exclusiveMax = schema["exclusiveMaximum"] as? Double { return isInt - ? DynamicGenerationSchema(type: Int.self, guides: [GenerationGuide.maximum(Int(exclusiveMax) - 1)]) + ? DynamicGenerationSchema(type: Int.self, guides: [GenerationGuide.maximum(Int(exclusiveMax.rounded(.down)) - (exclusiveMax.truncatingRemainder(dividingBy: 1) == 0 ? 1 : 0))]) : DynamicGenerationSchema(type: Double.self, guides: [GenerationGuide.maximum(exclusiveMax.nextDown)]) }🤖 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 `@mobile/modules/apple-llm/ios/AppleLlmSchemaParser.swift` around lines 116 - 120, The exclusiveMaximum handling in AppleLlmSchemaParser’s schema parsing has an off-by-one bug for integer schemas when the value is fractional. Update the exclusiveMaximum branch in the parser to distinguish integer-valued and fractional bounds before building the DynamicGenerationSchema, so integer types clamp to the correct inclusive max for both cases while keeping the Double path using nextDown. Use the existing exclusiveMinimum logic and the exclusiveMaximum check in AppleLlmSchemaParser as the reference points.
🤖 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 `@mobile/modules/apple-llm/ios/AppleLlmModule.swift`:
- Around line 44-58: The shared dictionaries in AppleLlmModule are being
accessed from multiple callbacks and an unstructured Task without consistent
serialization, so `tasks` and `sessionStore` can race. Update `AppleLlmModule`
to use the same synchronization approach as `ToolBridge`—for example an `NSLock`
or actor-backed store—and make every read/write in `generate`, `cancel`,
`OnDestroy`, `resetSession`, and `prewarm` go through that guarded path.
In `@mobile/scripts/gen-webview-assets.js`:
- Around line 58-64: The webview asset script is relying on a hoisted transitive
d3 install instead of an explicit package dependency. Update the mobile package
setup to declare d3 directly so gen-webview-assets.js can resolve d3.min.js
reliably, and keep the lookup in D3_CANDIDATES only as a fallback for the
expected install locations used by the script.
In `@mobile/src/chat/providers/apple.ts`:
- Around line 240-297: The streamApple generator is missing a pre-flight
cancellation check, so an already-aborted AbortSignal can still allow
AppleLlm.generate to run. In streamApple, check signal.aborted before
registering the abort listener and before calling AppleLlm.generate; if it is
already aborted, cancel/short-circuit immediately by pushing or throwing an
AppleLLMError with an abort/canceled reason so no generation starts. Keep the
existing onAbort listener and requestId/sessionId flow unchanged.
In `@mobile/src/components/CalendarView.tsx`:
- Around line 177-182: The Chevron navigation Pressable buttons in CalendarView
are missing accessibility labels, so update the prev/next icon buttons to
include clear accessibilityLabel values that describe their actions, matching
the pattern used by Stack.Toolbar.Button and Stack.Toolbar.Menu in
calendar/index.tsx. Use the existing Pressable elements around ChevronLeft and
ChevronRight so screen readers can announce “previous” and “next” calendar
navigation correctly.
In `@mobile/src/components/DraggableBoard.tsx`:
- Around line 56-64: `DraggableBoard` is creating a new empty array for columns
with no cards via `cardsByColumn.get(col.id) ?? []`, which breaks `BoardColumn`
memoization for empty columns. Hoist a stable empty-array sentinel in
`DraggableBoard` and reuse it when a column has no entry in `cardsByColumn`, so
`BoardColumn` receives the same `cards` reference across re-renders and empty
columns don’t re-render on drag state changes.
In `@mobile/src/components/MarkdownView.tsx`:
- Around line 296-300: The math_block Suspense fallback in MarkdownView does not
match MathView’s display-mode min-height, causing layout shift when the lazy
component loads. Update the fallback used in the math_block renderer to match
MathView’s actual display=true minimum height, similar to how MermaidView’s
fallback aligns with its minHeight, and keep the fix localized to the
math_block/MathView rendering path.
In `@mobile/src/screens/NoteDetailScreen.tsx`:
- Around line 280-290: The nested editor toolbar is only accounting for the home
indicator inset, so in the Projects-tab flow it can sit behind the tab bar when
the keyboard is dismissed. Update the NoteDetailScreen toolbar props for
NoteEditorToolbar to include the nested tab bar offset by using the existing
nested flag together with TAB_BAR_BASE, while keeping the current insets.bottom
handling for the home indicator.
In `@mobile/src/screens/ProjectScreen.tsx`:
- Around line 240-251: The board padding passed from ProjectScreen to
DraggableBoard is missing the nested tab-bar offset, so the last cards can sit
under the native tab bar in the nested Projects flow. Update the bottomInset
calculation in ProjectScreen before passing it to DraggableBoard to include the
same nested-tab adjustment used by Notes listBottomPad, and make sure
DraggableBoard continues to consume the final inset value through its
bottomInset prop.
---
Nitpick comments:
In `@mobile/app/`(tabs)/calendar/index.tsx:
- Around line 34-61: No code changes are needed here: the Stack.Toolbar,
Stack.Toolbar.Menu, Stack.Toolbar.MenuAction, and Stack.Toolbar.Label usage
already matches the expo-router API shape, and the accessibility labels are set
correctly. Keep this calendar toolbar implementation as-is, and just be aware
that the Stack.Toolbar API is alpha and may require updates when upgrading
expo-router in the future.
In `@mobile/app/note/new.tsx`:
- Around line 56-119: The note editor callbacks in new.tsx are duplicated from
NoteDetailScreen, so move the shared selection/formatting/AI workflow into a
reusable hook. Extract the logic behind onSelectionChange, onFormat, onWikilink,
onAIAction, hasSelection, selectionRef, wikilinkOpen, and aiLoading into a hook
such as useNoteFormattingToolbar(body, setBody), then update both NewNote and
NoteDetailScreen to consume that single implementation. Keep the returned API
identical so both screens use the same behavior and stay in sync.
In `@mobile/modules/apple-llm/ios/AppleLlmSchemaParser.swift`:
- Around line 116-120: The exclusiveMaximum handling in AppleLlmSchemaParser’s
schema parsing has an off-by-one bug for integer schemas when the value is
fractional. Update the exclusiveMaximum branch in the parser to distinguish
integer-valued and fractional bounds before building the
DynamicGenerationSchema, so integer types clamp to the correct inclusive max for
both cases while keeping the Double path using nextDown. Use the existing
exclusiveMinimum logic and the exclusiveMaximum check in AppleLlmSchemaParser as
the reference points.
In `@mobile/src/components/BottomSheet.tsx`:
- Around line 156-163: The backdrop style in BottomSheet uses a hardcoded raw
color via withAlpha("`#000000`", 0.4) instead of a theme token, so update the
backdrop styling to derive its dimming color from Theme. Add a scrim/overlay
token to the Theme interface and implementation (or reuse an existing
theme-derived token path) and switch the BottomSheet backdrop style to use that
token so the overlay remains consistent with the theming system.
- Around line 125-151: The Cancel/Done Pressable controls in BottomSheetHeader
are missing accessibility metadata, so update the BottomSheetHeader component to
give both interactive elements an accessibilityRole of button and clear
accessibilityLabel values matching their actions. Use the existing Pressable
elements in BottomSheetHeader as the fix point, and keep the labels consistent
with the visible text so screen readers announce them correctly.
In `@mobile/src/components/CalendarView.tsx`:
- Line 504: Replace the raw numeric fontSize usage in CalendarView’s style
definitions with shared typeScale tokens to match the rest of the file and the
mobile typography rollout. Update the styles for overdueWas, weekday, dayNum,
chipText, and moreText to use the appropriate typeScale values (for example
micro/caption-like tokens) instead of hardcoded 9.5–10.5 sizes, keeping the
existing color and spacing intact.
In `@mobile/src/components/KnowledgeGraphWebView.tsx`:
- Around line 139-221: The KnowledgeGraphWebView HTML is being rebuilt on
runtime toggle changes, which causes the WebView to reload unnecessarily. Update
the `useMemo` that builds `html` and the `WebView` usage so `showHulls` and
`labelMode` are applied via `injectJavaScript` or message handling inside
`drawHulls()` and `labelVisible()` instead of changing `source={{ html }}`; keep
rebuilding `html` only when the filtered node/link data changes.
In `@mobile/src/screens/CardDetailScreen.tsx`:
- Line 223: The `label` text style in `CardDetailScreen` is bypassing the shared
type scale used by the other styles in this file. Update the `label` entry in
the style object to spread the appropriate `typeScale` token and keep only the
remaining spacing/color overrides, matching the pattern used by `titleInput`,
`priorityText`, and `descToggle`.
In `@mobile/src/screens/NoteDetailScreen.tsx`:
- Line 30: The TAB_BAR_BASE constant is duplicated in NoteDetailScreen and
ProjectScreen, so move it to a shared theme/token location (for example
alongside the new iconSize and typography tokens in theme.ts) and update both
screens to read from that shared source. Use the shared constant in the nested
padding/bottom inset calculations so both screens stay in sync and do not drift
apart over time.
In `@mobile/src/screens/ProjectScreen.tsx`:
- Line 293: The active tag chip text color in ProjectScreen is using a hardcoded
white value instead of a theme token. Update the Text styling in the tag filter
render path to use the same theme foreground token pattern used by
CardDetailScreen for the priority chip, so the active state color comes from
t.accentFg (or the equivalent theme foreground token) rather than "`#fff`"; keep
tag.color unchanged since it is persisted data.
In `@src/components/graph/ForceGraphCanvas.tsx`:
- Line 279: The collision radius update in ForceGraphCanvas is duplicating the
shared padding formula instead of reusing collideRadius(type, spacing). Update
the sim.force("collide") radius callback to call the shared collideRadius helper
so the spacing-update path stays consistent with the rebuild path and does not
hardcode the +12 constant. Use the existing collideRadius symbol in
ForceGraphCanvas to locate and replace the inline calculation.
🪄 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: e4968170-e850-4be6-aced-e85519894fbd
📒 Files selected for processing (72)
docs/plans/apple-foundation-models-provider.mdmobile/.env.examplemobile/app/(tabs)/_layout.tsxmobile/app/(tabs)/calendar/_layout.tsxmobile/app/(tabs)/calendar/index.tsxmobile/app/(tabs)/chat/index.tsxmobile/app/(tabs)/graph/_layout.tsxmobile/app/(tabs)/graph/index.tsxmobile/app/(tabs)/index/_layout.tsxmobile/app/(tabs)/projects/_layout.tsxmobile/app/(tabs)/projects/card/[id].tsxmobile/app/(tabs)/projects/index.tsxmobile/app/(tabs)/projects/note/[id].tsxmobile/app/(tabs)/projects/project/[id].tsxmobile/app/(tabs)/projects/project/calendar.tsxmobile/app/(tabs)/search/index.tsxmobile/app/_layout.tsxmobile/app/card/[id].tsxmobile/app/card/new.tsxmobile/app/conflicts.tsxmobile/app/index.tsxmobile/app/note/[id].tsxmobile/app/note/new.tsxmobile/app/project/[id].tsxmobile/app/project/calendar.tsxmobile/app/settings/ai.tsxmobile/app/sync.tsxmobile/babel.config.jsmobile/modules/apple-llm/AppleLlm.types.tsmobile/modules/apple-llm/expo-module.config.jsonmobile/modules/apple-llm/index.tsmobile/modules/apple-llm/ios/AppleLlm.podspecmobile/modules/apple-llm/ios/AppleLlmModule.swiftmobile/modules/apple-llm/ios/AppleLlmSchemaParser.swiftmobile/modules/apple-llm/package.jsonmobile/scripts/gen-webview-assets.jsmobile/src/chat/agent.tsmobile/src/chat/ai-config.tsmobile/src/chat/providers/apple.tsmobile/src/chat/providers/index.tsmobile/src/chat/providers/types.tsmobile/src/components/AiSettingsForm.tsxmobile/src/components/BottomSheet.tsxmobile/src/components/CalendarView.tsxmobile/src/components/CodeBlock.tsxmobile/src/components/ConflictBanner.tsxmobile/src/components/DraggableBoard.tsxmobile/src/components/DueDatePickerSheet.tsxmobile/src/components/KnowledgeGraphWebView.tsxmobile/src/components/MarkdownView.tsxmobile/src/components/NoteEditorToolbar.tsxmobile/src/components/SearchField.tsxmobile/src/components/SyncStatusBadge.tsxmobile/src/components/TabStack.tsxmobile/src/components/TagChips.tsxmobile/src/components/TagPickerSheet.tsxmobile/src/components/UpdateBanner.tsxmobile/src/components/WikilinkPickerSheet.tsxmobile/src/components/toolbar-icons.tsmobile/src/db/queries.tsmobile/src/screens/CardDetailScreen.tsxmobile/src/screens/NoteDetailScreen.tsxmobile/src/screens/ProjectCalendarScreen.tsxmobile/src/screens/ProjectScreen.tsxmobile/src/theme.tsmobile/src/webview-assets/d3-assets.tsmobile/tsconfig.jsonscripts/publishupdate.shshared/ui/graph.tssrc/components/graph/ForceGraphCanvas.tsxsrc/components/graph/RadialTreeCanvas.tsxsrc/store/slices/graph.ts
💤 Files with no reviewable changes (1)
- mobile/app/(tabs)/index/_layout.tsx
Address review findings on the mobile PR (verified each against current code; fixed valid ones, skipped substantial refactors): Correctness - apple-llm Swift: guard tasks/sessionStore with an NSLock (withState) so the Expo callbacks and the streaming Task can't race (mirrors ToolBridge). - apple-llm schema parser: fix exclusiveMaximum off-by-one for integer schemas with fractional bounds (ceil(x)-1, so 5.5 → max 5, 5.0 → max 4). - apple.ts: pre-flight signal.aborted check — throw before registering listeners / starting generate when already aborted. Layout / a11y - ProjectScreen + NoteDetailScreen: add nested tab-bar offset (TAB_BAR_BASE) to the board bottomInset and editor-toolbar bottomInset so cards / the toolbar clear the native tab bar in the Projects flow. - theme.ts: hoist shared TAB_BAR_BASE; both screens read it (was duplicated). - CalendarView: accessibilityRole/label on the prev/next chevron buttons. - BottomSheet: accessibilityRole/label on Cancel/Done; backdrop uses a new theme token instead of hardcoded withAlpha(#000000). Perf / consistency - DraggableBoard: reuse a stable EMPTY_CARDS sentinel for empty columns so BoardColumn memoisation isn't defeated on drag-state changes. - MarkdownView: math_block Suspense fallback height 24 → 40 to match MathView's display=true minHeight (no layout shift on lazy load). - CardDetailScreen: style spreads typeScale.micro (keeps overline overrides). - ProjectScreen: active tag chip text uses t.accentFg, not '#fff'. - ForceGraphCanvas: spacing-update collide radius reuses collideRadius() helper. - mobile package.json: declare d3 as a direct devDependency (was relying on a hoisted transitive install); gen-webview-assets keeps the candidate fallback. Skipped (with reason) - CalendarView grid font sizes (9.5–10.5): intentional dense sub-micro sizes, don't map to the type scale (smallest = micro 11) without enlarging the grid. - useNoteFormattingToolbar hook extraction: large cross-screen refactor, higher regression risk — out of scope for this batch. - KnowledgeGraphWebView injectJavaScript rework: behavioural WebView change, risky without device testing. Validated: mobile type-check + lint (only pre-existing set-state-in-effect warnings), desktop type-check:all, Swift compiles (xcodebuild AppleLlm, no Sendable warnings).
The note composer (app/note/new.tsx) and the note editor (NoteDetailScreen) had ~65 lines of identical editor-toolbar state + callbacks duplicated verbatim (onSelectionChange, onFormat, onWikilink, onAIAction, hasSelection, selection, aiLoading, wikilinkOpen). Any formatting/AI-action fix had to be made twice and they would inevitably drift. Move the shared logic into src/notes/useNoteFormattingToolbar(body, setBody); both screens now consume it with an identical API (screens still own body/setBody since NoteDetailScreen resets body on sync/refocus). Behaviour is unchanged. Net -150 lines across the two screens. Also drops a now-stale AppleMessage import in the apple-llm bridge (generate takes a prompt string, not messages). type-check + lint clean (only pre-existing set-state-in-effect warnings).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@mobile/src/notes/useNoteFormattingToolbar.ts`:
- Around line 70-97: Prevent `onAIAction` from applying a stale `body` snapshot
after `runTextAction` by disabling editing while AI is pending. Update the body
`TextInput` in both `new.tsx` and `NoteDetailScreen` to use
`editable={!fmt.aiLoading}` so users cannot change the text while
`useNoteFormattingToolbar` is awaiting the AI response and then calling
`setBody` with outdated content.
🪄 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: d905a025-4fab-40c0-90e5-3dd0b441aaa7
⛔ Files ignored due to path filters (1)
mobile/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (20)
mobile/app.jsonmobile/app/_layout.tsxmobile/app/note/new.tsxmobile/modules/apple-llm/index.tsmobile/modules/apple-llm/ios/AppleLlmModule.swiftmobile/modules/apple-llm/ios/AppleLlmSchemaParser.swiftmobile/package.jsonmobile/plugins/withStatusBarStyle.jsmobile/scripts/gen-webview-assets.jsmobile/src/chat/providers/apple.tsmobile/src/components/BottomSheet.tsxmobile/src/components/CalendarView.tsxmobile/src/components/DraggableBoard.tsxmobile/src/components/MarkdownView.tsxmobile/src/notes/useNoteFormattingToolbar.tsmobile/src/screens/CardDetailScreen.tsxmobile/src/screens/NoteDetailScreen.tsxmobile/src/screens/ProjectScreen.tsxmobile/src/theme.tssrc/components/graph/ForceGraphCanvas.tsx
💤 Files with no reviewable changes (1)
- mobile/modules/apple-llm/index.ts
✅ Files skipped from review due to trivial changes (2)
- mobile/plugins/withStatusBarStyle.js
- mobile/package.json
🚧 Files skipped from review as they are similar to previous changes (12)
- mobile/scripts/gen-webview-assets.js
- mobile/src/components/MarkdownView.tsx
- mobile/src/chat/providers/apple.ts
- mobile/modules/apple-llm/ios/AppleLlmModule.swift
- src/components/graph/ForceGraphCanvas.tsx
- mobile/src/components/BottomSheet.tsx
- mobile/modules/apple-llm/ios/AppleLlmSchemaParser.swift
- mobile/src/screens/CardDetailScreen.tsx
- mobile/src/screens/ProjectScreen.tsx
- mobile/src/screens/NoteDetailScreen.tsx
- mobile/src/components/CalendarView.tsx
- mobile/src/components/DraggableBoard.tsx
onAIAction (useNoteFormattingToolbar) captures the body snapshot at call time
and, after runTextAction resolves, splices its reply into that snapshot. If the
user edits the text while the request is in flight, those edits get clobbered
and the reply is spliced at now-stale offsets.
Set editable={!fmt.aiLoading} on the body TextInput in both the composer
(note/new.tsx) and the editor (NoteDetailScreen) so the field can't change while
the AI response is awaited.
Mirrors desktop's bge-small semantic search on mobile, running fully on-device through Apple's NaturalLanguage NLContextualEmbedding (iOS 17+). Vectors live in Apple's own space (not desktop-compatible), so the index is built + queried per-device and never syncs. - modules/apple-embeddings: new Expo native module. Swift wrapper mean-pools per-token vectors + L2-normalises; requestAssets/load; isSupported() (iOS 17+, regardless of asset download) vs isAvailable() (assets ready now). Compiles clean against the iOS 26.5 SDK. - Local-only note_embeddings table (no sync trigger); base schema + migration. - src/notes/embeddings.ts: incremental hash-gated reindex, brute-force cosine search, semantic-edge computation, coalesced catch-up + index-status store. - Startup + after-sync catch-up so imported / sync-pulled notes get indexed. - Search tab: Semantic scope (with % scores) + IndexingBar progress. - Knowledge graph: dashed accent semantic edges (weight-scaled width) + a Semantic-links toggle. Fixes semantic search not triggering: the UI gated on isAvailable() (assets ready), which races with on-demand asset download — the graph would load assets first and flip it true while the Search tab had already rendered with the scope hidden. UI now gates on isSupported() (stable on iOS 17+); the embed path calls ensureAssets() on demand, and selecting the Semantic scope kicks off a catch-up so the index exists. type-check + lint clean (only pre-existing set-state-in-effect warnings); Swift BUILD SUCCEEDED.
…able The Semantic segment was hidden whenever on-device embeddings weren't ready, which made it undiscoverable and looked like the feature was missing. Always render the scope; when unavailable, surface the native unavailableReason() in the results area instead of hiding it. This also removes the render-timing fragility around asset availability.
The in-body Notes/Tasks/Semantic segment wasn't visible on the native iOS search tab (role="search"): the native search field owns the header area, so a <View> segment below it gets clipped by content insets / has nowhere to anchor. Replace it with a Stack.Toolbar.Menu (same pattern as the Graph tab's Force/Radial switch) — the current scope shows as the menu icon+label, and picking an entry switches scope. Added ICON_NOTE / ICON_TASK; removed the now dead segment styles + Pressable import.
The results FlatList didn't clear the native search header or lift above the keyboard. Fixes: - contentInsetAdjustmentBehavior="automatic" so the list insets below the native search-tab header. - automaticallyAdjustKeyboardInsets + keyboardDismissMode="on-drag" so content scrolls clear of the keyboard and the keyboard dismisses on scroll. - Move IndexingBar into ListHeaderComponent so the FlatList is the screen's first/only scroll view (required for iOS to apply the automatic search-header inset — a sibling scroll view above it broke that). - Bottom padding (TAB_BAR_BASE) so the last row clears the tab bar. Shared as listProps across all three (notes/tasks/semantic) lists.
Match Calendar / Graph / Chat — TabStack largeTitle={false}. The native search
field owns the header area, so a large scrolling title just competed with the
results list and search bar for space.
…ass scope bar - Extract pure embedding math (pool + L2-normalise + dot) into EmbeddingMath.swift and add a standalone SwiftPM test package (modules/apple-embeddings/tests) with 13 XCTests covering unit-length pooling, order-independence, identical-input determinism (content-hash cache safety), zero-vector NaN guard, odd-width row handling, and cosine ranking. Run: cd modules/apple-embeddings/tests && swift test. - Sync modal: EmbeddingsCard shows indexed/total notes + a Reindex button (good home for catching up notes just synced from desktop). Adds embeddingIndexStats. - Search: move the scope switch out of the native toolbar (which minimizes with the keyboard) into a persistent GlassBar pinned to the bottom via KeyboardStickyView, lifted clear of the iOS 26 tab-bar search field both docked and keyboard-open. Pull-to-refresh on the Semantic list forces a reindex. - gitignore .build/ + .swiftpm/ (SwiftPM output). type-check + lint clean (only pre-existing warnings); 13/13 Swift tests pass; native module BUILD SUCCEEDED.
- Add idle hint text for Notes/Tasks scopes (not just Semantic). - Move all empty/hint text into a single absolutely-positioned overlay anchored at 75% from the bottom, so it no longer jumps with the keyboard, hides behind the header, or falls behind the keyboard. - Pin the scope bar just above the native tab-bar search field via KeyboardStickyView closed offset = -insets.bottom (was lifted too high).
…mport) Lint is now 0 warnings / 0 errors. Behaviour unchanged. - _layout: merge duplicate @/sync/controller import; open the DB via a lazy useState initializer (once, pre-paint) instead of setState in an effect. - AiSettingsForm: seed provider/baseUrl/model from their synchronous getters via lazy useState initializers; the effect now only loads the async API key. - chat: restore on-device history via lazy useState/useRef initializers rather than a setState-in-effect; drop the now-unused effect + useEffect import. - BottomSheet: derive mount from (visible || exiting) with a wasVisible ref so opening no longer does a synchronous setState in the effect; guard the exit callback against reopen-mid-close.
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
mobile/modules/apple-embeddings/index.ts (1)
99-107: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winValidate native payload length before reshaping.
If the native
embed()result length doesn't matchtexts.length * dim(partial failure, stale binary, etc.), this silently slices short/garbage vectors that get persisted tonote_embeddingswith no validation at write time — only the read path (semanticSearch) checksvec.length !== info.dimension. Bad rows would linger in the index until an unrelated full reindex prunes them.🛡️ Proposed validation
export async function embedTexts(texts: string[], dim: number): Promise<Float32Array[]> { if (!AppleEmbeddings || texts.length === 0) return []; const flat = await AppleEmbeddings.embed(texts); + if (flat.length !== texts.length * dim) { + console.warn("[apple-embeddings] unexpected embed() payload length"); + return []; + } const rows: Float32Array[] = []; for (let i = 0; i < texts.length; i++) { rows.push(Float32Array.from(flat.slice(i * dim, i * dim + dim))); } return rows; }🤖 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 `@mobile/modules/apple-embeddings/index.ts` around lines 99 - 107, Validate the native payload length in embedTexts before reshaping the result from AppleEmbeddings.embed. Compare the flat array length against texts.length * dim, and if it does not match, fail fast with an error instead of slicing into Float32Array rows; keep the check inside embedTexts so bad vectors are never returned for persistence to note_embeddings.mobile/app/_layout.tsx (1)
60-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInconsistent error logging between initial and data-change catch-up
The initial catch-up logs failures with
console.warn("[embeddings] initial catch-up failed:", e), but theonDataChanged-triggered catch-up silently swallows errors with.catch(() => {}). This makes data-change-triggered embedding failures invisible during debugging.🔧 Proposed fix: add consistent logging
const unsub = onDataChanged(() => { - catchUpIndex().catch(() => {}); + catchUpIndex().catch((e) => console.warn("[embeddings] data-change catch-up failed:", e)); });🤖 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 `@mobile/app/_layout.tsx` around lines 60 - 65, The catch-up error handling is inconsistent between the initial timeout and the onDataChanged callback in the _layout.tsx logic. Update the onDataChanged catch-up to log failures the same way as the initial catch-up by adding a warning in the catch-upIndex promise rejection handler instead of swallowing it, so both paths surface embedding sync issues consistently.
🤖 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 `@mobile/app/`(tabs)/chat/index.tsx:
- Around line 68-98: The `conversation` ref initializer is still doing
`loadChatHistory().map(...)` during every render, even though only the first
value is kept. Update the `conversation` setup in the chat screen to use a lazy
ref pattern so the history load/remap runs once at mount, matching the existing
lazy `useState` approach used for `messages`. Keep the same restoration logic
for `userMessage`, `assistantMessage`, and attachment mapping, but move the
expensive work behind a one-time initialization for `conversation`.
In `@mobile/app/`(tabs)/graph/index.tsx:
- Around line 49-62: `loadSemantic` in the graph tab needs cancellation and
error handling because it is invoked asynchronously from `useFocusEffect` and
`useDataChanged` without guarding against stale results or rejected promises.
Add a sequence/cancel flag similar to `semanticSeq` in `search/index.tsx` so
older `semanticEdges` results are ignored if `showSemantic` changes or a newer
load starts, and wrap the `semanticEdges(ws)` loop in `loadSemantic` with
try/catch so failures are handled instead of becoming unhandled rejections. Keep
the fix localized to `loadSemantic`, `useFocusEffect`, and `useDataChanged`.
In `@mobile/app/`(tabs)/search/index.tsx:
- Around line 127-136: The forceReindex callback currently only uses
try/finally, so errors from catchUpIndex() escape unhandled through
RefreshControl’s onRefresh and give no user feedback. Update forceReindex in
search/index.tsx to add a catch around catchUpIndex(), log the failure, and
surface an appropriate user-facing message before the finally block resets
reindexing; keep the existing refreshStats and run(query, "semantic") flow on
success.
- Around line 48-67: In the run callback’s semantic branch, invalidate any
in-flight search when the query is cleared by bumping semanticSeq.current before
returning, so older semanticSearch results can’t repopulate hits after
setHits([]). Also add error handling to the async IIFE in run by catching
rejected semanticSearch calls and swallowing or logging the failure, while
keeping the existing seq guard and listWorkspaceIds aggregation logic intact.
In `@mobile/src/components/EmbeddingsCard.tsx`:
- Around line 42-46: The onReindex callback in EmbeddingsCard is swallowing all
failures from catchUpIndex() by using an empty catch handler, which makes
reindex problems impossible to debug. Update onReindex to handle the rejection
explicitly by logging the caught error at minimum, and keep the existing refresh
flow intact when catchUpIndex() succeeds. Use the existing catchUpIndex,
refresh, and onReindex symbols to locate and adjust the async chain.
- Around line 39-49: `EmbeddingsCard` only refreshes stats on focus or manual
reindex, so the counts stay stale when indexing finishes automatically. Add a
`useEffect` in `EmbeddingsCard.tsx` that watches `status.running` and calls
`refresh()` when it transitions from true to false, keeping the existing
`useFocusEffect` and `onReindex` behavior intact.
In `@mobile/src/notes/embeddings.ts`:
- Around line 183-276: `reindexNote` and `reindexWorkspace` can embed the same
note at the same time because only `reindexNote` uses `singleInFlight` while
both paths share `embedNoteInner`. Move the in-flight protection into
`embedNoteInner` (or a helper it calls) so any caller serializes per-note work,
and keep `reindexNote` as an optional fast-path short-circuit if desired. Make
sure the shared guard covers the whole embed/delete/prune flow so
`deleteNoteEmbeddingsFrom`, `upsertNoteEmbedding`, and `embedOne` cannot race
for the same note ID.
---
Nitpick comments:
In `@mobile/app/_layout.tsx`:
- Around line 60-65: The catch-up error handling is inconsistent between the
initial timeout and the onDataChanged callback in the _layout.tsx logic. Update
the onDataChanged catch-up to log failures the same way as the initial catch-up
by adding a warning in the catch-upIndex promise rejection handler instead of
swallowing it, so both paths surface embedding sync issues consistently.
In `@mobile/modules/apple-embeddings/index.ts`:
- Around line 99-107: Validate the native payload length in embedTexts before
reshaping the result from AppleEmbeddings.embed. Compare the flat array length
against texts.length * dim, and if it does not match, fail fast with an error
instead of slicing into Float32Array rows; keep the check inside embedTexts so
bad vectors are never returned for persistence to note_embeddings.
🪄 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: ddc1c1ac-30af-47d9-8402-67cbf3bf0cd0
📒 Files selected for processing (30)
mobile/.gitignoremobile/app/(tabs)/chat/index.tsxmobile/app/(tabs)/graph/index.tsxmobile/app/(tabs)/search/_layout.tsxmobile/app/(tabs)/search/index.tsxmobile/app/_layout.tsxmobile/app/note/new.tsxmobile/app/sync.tsxmobile/modules/apple-embeddings/AppleEmbeddings.types.tsmobile/modules/apple-embeddings/expo-module.config.jsonmobile/modules/apple-embeddings/index.tsmobile/modules/apple-embeddings/ios/AppleEmbeddings.podspecmobile/modules/apple-embeddings/ios/AppleEmbeddingsModule.swiftmobile/modules/apple-embeddings/ios/EmbeddingMath.swiftmobile/modules/apple-embeddings/package.jsonmobile/modules/apple-embeddings/tests/Package.swiftmobile/modules/apple-embeddings/tests/Sources/EmbeddingMath/EmbeddingMath.swiftmobile/modules/apple-embeddings/tests/Tests/EmbeddingMathTests/EmbeddingMathTests.swiftmobile/src/components/AiSettingsForm.tsxmobile/src/components/BottomSheet.tsxmobile/src/components/EmbeddingsCard.tsxmobile/src/components/IndexingBar.tsxmobile/src/components/KnowledgeGraphWebView.tsxmobile/src/components/toolbar-icons.tsmobile/src/db/migrations.tsmobile/src/db/queries.tsmobile/src/db/schema.tsmobile/src/notes/embeddings.tsmobile/src/notes/useIndexStatus.tsmobile/src/screens/NoteDetailScreen.tsx
✅ Files skipped from review due to trivial changes (5)
- mobile/app/(tabs)/search/_layout.tsx
- mobile/modules/apple-embeddings/expo-module.config.json
- mobile/.gitignore
- mobile/modules/apple-embeddings/package.json
- mobile/modules/apple-embeddings/tests/Sources/EmbeddingMath/EmbeddingMath.swift
🚧 Files skipped from review as they are similar to previous changes (7)
- mobile/app/sync.tsx
- mobile/src/components/toolbar-icons.ts
- mobile/app/note/new.tsx
- mobile/src/screens/NoteDetailScreen.tsx
- mobile/src/components/BottomSheet.tsx
- mobile/src/components/KnowledgeGraphWebView.tsx
- mobile/src/components/AiSettingsForm.tsx
…scriminate Symptom: every semantic result showed ~90% and rankings barely separated. Root cause (reproduced in new live SwiftPM tests against real NLContextualEmbedding on macOS 14+): mean-pooled contextual vectors share a large common component, so raw cosine collapses into a narrow ~0.85–0.93 band — the correct note beats the runner-up by ~0.0005. It's not a pooling/normalise bug (self-sim=1.0, distinct notes differ), it's inherent to mean-pooled contextual embeddings. Fix: before cosine, subtract the corpus centroid from the query and every doc vector and renormalise (a standard 'all-but-the-mean' trick). Live tests show the top-2 spread widening ~50x and all three distinct queries (sync/kanban/ recipe) now ranking their note first with clear margins. Applied to both semanticSearch and semanticEdges (graph); SEMANTIC_FLOOR retuned for centred cosine (0.6→0.15); scores mapped to a 0–1 display range via displayScore so the UI shows real spread instead of everything ~90%. Adds EmbeddingLive test target (wraps NLContextualEmbedding, replicates the app pipeline) + regression test testCenteredPipelineRanksAllQueriesCorrectly. 18/18 Swift tests pass; type-check + lint clean.
The 'Represent this sentence for searching relevant passages: ' prefix is a BGE-model instruction the desktop bge-small model was trained on. Apple's NLContextualEmbedding was never trained on it, so it just adds ~10 tokens of shared boilerplate to every query, nudging all query vectors toward a common direction — the same common-component that corpus-centring removes. New live test testCenteredPipelinePrefixVsNoPrefix measures top-2 spread with vs without the prefix across three queries (centring on in both). Both rank correctly, but removing the prefix widens mean spread 0.308 -> 0.465 (+51%) — better score discrimination on every query. Removed QUERY_PREFIX; queries are now embedded verbatim like documents.
…tate - chat: seed conversation ref via null-sentinel one-time init so loadChatHistory()/remap runs once at mount, not every render - graph: guard loadSemantic with a semanticSeq token (ignore stale results when the toggle flips or data changes) and try/catch the semanticEdges loop - search: bump semanticSeq when the query is cleared so an in-flight result can't repopulate hits; catch the async semantic IIFE; catch forceReindex and surface an Alert on failure instead of an unhandled RefreshControl rejection - EmbeddingsCard: log onReindex failures instead of swallowing them; refresh stats when indexing finishes (running true→false), not just on focus - embeddings: move the per-note singleInFlight guard into embedNoteInner so reindexNote and reindexWorkspace can't race the same note's delete/upsert - _layout: log onDataChanged catch-up failures to match the initial catch-up - apple-embeddings: validate the native payload length in embedTexts and fail fast instead of persisting truncated/zero-padded vectors type-check + lint clean (0/0); 19/19 Swift tests pass.
…d pure cosine failing A real-data test against the actual 70-note cairn.db corpus revealed the fix so far was still wrong: 'How does semantic search work' ranked the semantic-search note 36th of 70. Pure mean-pooled contextual embeddings capture broad topic/ style but not keyword salience, so on a homogeneous corpus (mostly dev notes) generic notes outrank the obviously-correct one — centroid centring alone can't fix this. Fix: hybrid retrieval (dense + sparse), as production semantic search does. Blend min-max-normalised centred cosine with a title-weighted lexical score (query keywords, stopwords removed; title matches count 2×, body 1×) at SEMANTIC_WEIGHT=0.5, tuned on the real corpus. Every query with a clear target now ranks it #1 (semantic-search note 36→1; sync 11→1; embeddings 2→1). Display score still shows the raw centred cosine; only ranking is blended. Testing: new real-corpus live tests embed the actual notes with the real Apple model and assert the hybrid ranking. The real fixture (real_notes.json) is gitignored (private note content) and regenerated by scripts/export-notes-fixture.sh; a small committed sample_notes.json is the always-present fallback so the tests run on any machine/CI. Adds queries.noteTextByIds for lexical scoring. type-check + lint clean; 22/22 Swift tests pass with both the real and sample corpora.
Add a right-aligned zoom-to-fit button in the same bottom row as the graph legend (force mode only — radial has its own drill-in navigation). - WebView D3: window.__fit() computes the padded bounding box of all live node positions and applies a 450ms-transitioned d3.zoom transform that centres + scales the whole graph into the viewport (40px margin), clamped to the existing [0.2, 6] range. Reads live sim positions so it fits correctly after load, settle, or dragging. - Native: fitToView() injects window.__fit() via injectJavaScript. - Layout: legend + fit button share one bottom row (left/right aligned via space-between, spanning left:16/right:16, above the safe-area inset); pointerEvents=box-none so the gap doesn't block graph gestures. type-check + lint clean.
The hybrid re-rank (2cf0e4e) computed the correct blended dense+lexical order inside semanticSearch, but returned only a displayable `score` (raw centred cosine) — and the search screen then re-sorted the merged cross-workspace results by that `score`, throwing the hybrid ordering away. Net effect: the UI still showed pure-cosine order, so 'how does semantic search work' stayed buried (~36th) despite the fix — no reindex would have helped, the ranking was correct but discarded at the UI layer. Fix: SemanticHit now carries `rank` (the blended ordering score) alongside the human-readable `score`. semanticSearch sorts by rank; the search screen merges across workspaces sorting by rank too. Display % still shows centred cosine. No reindex needed — all three semantic fixes (centering, prefix, hybrid) are query-time only; stored vectors are unchanged. type-check + lint clean; the real-corpus Swift test still ranks the note #1.
… header on keyboard open - % now shows the blended relevance (rank), not the raw centred cosine (score). The cosine reads misleadingly low for keyword-heavy queries even when the note is correctly ranked #1, so the displayed % contradicted the ordering. rank is the actual match strength used for sorting, so the top result now reads high. - Removed automaticallyAdjustKeyboardInsets from the results FlatList. Combined with contentInsetAdjustmentBehavior=automatic (which owns the top search-header inset) it miscalculated the content offset when the keyboard opened, scrolling the first result up under the header. The bottom pad in styles.list already clears the keyboard/scope-bar/tab-bar, so keyboard insets aren't needed (the search field lives in the header, not the body). Matches the projects screen. type-check + lint clean.
Tapping a result while the search keyboard was still open pushed the detail screen with the native search bar focused. On return the header re-presented its focused/keyboard state, animating open → snapping closed → reopening — the visible jump. Add openResult(href): blur the native search bar (searchRef.blur()) + dismiss the keyboard, THEN navigate. The header is at rest before the push, so it returns cleanly; the tab's normal focus-effect re-focuses the bar afterwards. Wired into all three result lists (semantic / notes / tasks). type-check + lint clean.
Two features reusing the on-device hybrid semantic pipeline:
1. semantic_search_notes chat tool — the agent could previously only
keyword-search notes (search_notes, exact SQL LIKE), so conceptual questions
('what did I decide about auth') missed relevant notes. New tool retrieves by
MEANING across all workspaces, sorted by the hybrid rank, returning id/title/
section/score for the agent to get_note + ground its answer. Falls back with
a clear message when embeddings are unavailable. agent.ts now awaits
tool.run() (no-op for sync tools) so async tools work; ToolDef.run typed to
allow Promise.
2. Related Notes on the note detail screen — relatedNotes() finds a note's
top-K semantic neighbours (corpus-centred cosine, same discrimination fix as
search), rendered as a tappable list below the body in read mode. Renders
nothing when unavailable / note not indexed / no matches above the floor, so
no empty shell. Loads on focus, cancels stale loads. Added workspaceIdForNote
query helper.
type-check + lint clean; 22/22 Swift tests still pass.
… + sheet) Mirrors the desktop TOC on mobile. - shared/notes/toc.ts: extract headingSlug + extractHeadings into the shared package (one source of truth); desktop toc-utils.ts now re-exports them. - MarkdownView: custom heading1/2/3 render rules wrap each heading in an onLayout reporter (onHeadingLayout prop) keyed by GitHub-style slug, so the TOC can scroll to any heading. h4-h6 keep library defaults. - NoteDetailScreen: floating glass List button bottom-right (read mode, >=2 headings), lifted clear of the tab bar / home indicator. Opens a BottomSheet listing headings; tapping scrolls the read view to that heading (mdOffsetY + per-heading local y). Heading hierarchy shown by indent + colour + weight. TOC items use ONE uniform type token (control) across all levels — no per-level font sizes — so the font-scale setting applies uniformly (levels differ only by indent/colour/weight). type-check + lint clean on both mobile and desktop.
…the inset) Confirmed on device: on a tab screen the safe-area insets.bottom ALREADY includes the native tab bar, so the floating TOC button only needs insets.bottom + a small gap. Adding TAB_BAR_BASE on top double-counted and floated it too high in the nested (tab-bar-visible) flow; plain insets.bottom looks correct in both nested and non-nested. Updated the TAB_BAR_BASE doc in theme.ts to record this (prefer insets.bottom for fixed overlays). type-check + lint clean.
…l padding insets.bottom already includes the native tab bar on a tab screen (confirmed on device with the TOC button), so adding TAB_BAR_BASE to the content bottom pad over-reserved space in the nested Projects flow. Simplified: - NoteDetailScreen viewBottomPad: 40 + insets.bottom (was + nested*TAB_BAR_BASE) - ProjectScreen listBottomPad: 24 + insets.bottom (was + nested*TAB_BAR_BASE) Verified on device in both nested and non-nested flows. The edit-mode keyboard-sticky bottomInset props and the search/chat offsets are left as-is (different contexts — keyboard changes the safe area). type-check + lint clean.
… empty views Add a reusable EmptyState scaffold rendering the splash icon + 'Cairn' wordmark + app version (from expo-constants) above a title/subtitle or custom children. All text uses shared type tokens so it respects font-scale; no single-use sizes. Wired into the resting/empty states that read best branded: - Projects: 'No projects yet' - Graph: 'Nothing to graph yet' - Notes hierarchy (ProjectScreen): 'No notes yet' (align=top). The 'no notes match your filter' case keeps the light text hint (active-filter intent, not resting). - Search: branded resting state (no query); active-search 'no matches' keeps a light hint. Rendered as the FlatList ListEmptyComponent with flexGrow:1 so it sits BELOW the native search header and its top-bias measures against the real content area (an absolute screen overlay rendered behind the header). align='top' biases content to ~25% from the top so it stays readable with the keyboard open. type-check + lint clean.
Replace the chat tab's ad-hoc empty view with the shared EmptyState (Cairn icon + wordmark + version), keeping the 'Ask Cairn' title, hint, and the conditional 'Set up AI' button. EmptyState children now render as an action slot BELOW the title/subtitle (previously they replaced them), so title + hint + button coexist. Existing callers (Projects/Graph/Notes/Search) pass no children → unchanged. Chat's ScrollView content grows when empty (listEmpty: flexGrow) so align=top's 25% bias measures against the full area below the header, matching search. type-check + lint clean.
Easter egg: tap the Cairn icon in any EmptyState 5x within 3s to launch a Breakout game whose bricks are real workspace entities (projects/notes/tasks/ tags), coloured by kind like the knowledge graph. Plain rAF loop mutating a stable game object + immutable per-frame snapshot for render (no Skia); paddle at 20% up from the bottom; 7x6 short bricks; particle burst on break; pauses via AppState when the app isn't foregrounded (so rAF frame bursts on refocus can't speed up / teleport the ball). New listBreakoutBricks() query. Haptics: add expo-haptics + a reusable src/haptics module — a fire-and-forget 'haptics' object (impact/impactMedium/impactHeavy/rigid/soft/selection/success/ warning/error) that swallows errors, plus a useHaptics() hook. Wired at central entry points so coverage is broad with few touch points: - BottomSheet open (covers all sheets/modals) -> impact - Chat send -> selection; tool run -> impact; response -> success; error -> error - Board drag pickup -> impact; drop to new column -> impactMedium - Note save -> success; delete -> warning - Search scope switch + graph zoom-to-fit -> selection - Game bounce/break/win/lose + egg reveal Replaces the unreliable RN Vibration API. type-check + lint clean.
The BottomSheet open hook only covered in-app sheet modals; the sync, AI settings, new-note and new-task screens are native route modals (presentation: 'modal'), so they had no open feedback. Add useModalOpenHaptic() to the haptics module (fires one impact on mount — the native-modal equivalent of the BottomSheet open tap) and call it in all four modal screens, so every modal now gives consistent open feedback. type-check + lint clean.
Two central hooks so navigation and header actions feel tactile, keyed off the
user's tap rather than route changes (which would lag and double-fire with
programmatic navigation).
PressableScale — the shared press primitive behind nearly every row/card/chip —
gains an opt-outable `haptic` prop (default "selection") that fires on
`onPress` (commit), so a press cancelled by a scroll never buzzes. This covers
project rows, search results, note rows, board cards, calendar chips and
related-note rows in one place. The ToC FAB opts out (`haptic={false}`) since
the BottomSheet it opens already fires its own open tap.
toolbarPress(handler, intent) decorates Stack.Toolbar.Button / MenuAction
onPress with an intent-appropriate haptic (action=selection, confirm=success,
destroy=warning). A component wrapper isn't possible — the native iOS toolbar
reflects on child component identity (isChildOfType(child, StackToolbarButton))
and drops anything else — so we decorate the handler instead. Wired across
note/card detail, project, calendar, graph, chat, sync, new-note/new-task and
AI settings toolbars. Async or self-signalling handlers use "action" so the
tap and the completion haptic don't double-buzz; added the missing completion
haptics (archive warning, chat-clear warning, AI-settings success).
Tab-switch haptics intentionally omitted: NativeTabs exposes no tabPress hook,
and useFocusEffect would misfire on back-nav and deep links.
type-check + lint clean.
What does this PR do?
A large mobile (React Native / Expo) pass plus an experimental, dev-gated on-device AI provider. Three themes:
SearchField, iCloud sync badge, new-note formatting/AI toolbar, tab-bar safe-zone padding.modules/apple-llm) wrapping Apple's FoundationModels with native tool-calling. Hidden from end users behind a build-time flag; kept for local iteration only. Seedocs/plans/apple-foundation-models-provider.mdfor why (on-device model too weak / 4096-token context) and the plan to re-do it against Private Cloud Compute on iOS 27.Type of change
Screenshots / recording
Checklist
npm run type-check:allpassesnpm run lintpassesnpm testpassesnpm run test:e2epassestheme.tstoken scale /useTheme())text-[Npx]pixel font classes — rem/typescale onlyhandle()and returnIpcResult<T>schema.tselectron/db/queries.tsdependencies/devDependenciesadded toROLE_MAP+licenses.jsonregenerated--external:<pkg>allowlist updatedNotes for reviewer
EXPO_PUBLIC_APPLE_LLM_DEV, set only in the git-ignored localmobile/.env. EASpreview/productiondon't receive that flag, so the provider never appears in Settings or resolves in shipped builds. The native module still compiles (autolinked, dormant). Verified the Swift builds against the iOS 26.5 SDK (xcodebuild -scheme AppleLlm); it has not been exercised end-to-end on an iOS 26 device yet.shared/ui/graph.ts,src/components/graph/*,src/store/slices/graph.ts,scripts/publishupdate.sh) come from the "Mobile Styling Pass" commit — shared graph rendering touched to support the mobile graph WebView. No behavioural change to the desktop graph intended; worth a glance.@modules/*path alias added totsconfig.json+babel.config.js.docs/plans/apple-foundation-models-provider.mdis the reimplementation plan (Private Cloud Compute, iOS 27) for when the managed entitlement lands.Summary by CodeRabbit