Feat/humanlike replies#168
Merged
graycyrus merged 7 commits intoApr 1, 2026
Merged
Conversation
subscribeChatEvents was declared async despite having no awaits, so the cleanup function was returned in a microtask after React's synchronous cleanup had already run. Each socket reconnect added another layer of listeners that were never removed, causing chat:done to fire N times and produce duplicate message bubbles. - Remove async keyword from subscribeChatEvents; return cleanup fn directly - Update useEffect call site to store cleanup synchronously and return it to React (eliminates the mounted/then race entirely) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Expose openhuman.local_ai_chat RPC method so the UI can run full conversation-history chat directly through the bundled Ollama model without touching the cloud inference API. - ollama_api.rs: add OllamaChatMessage / OllamaChatRequest / OllamaChatResponse types for the /api/chat endpoint - service/public_infer.rs: add LocalAiService::chat_with_history() — sends multi-turn message array to Ollama, updates latency/TPS status on response - ops.rs: add LocalAiChatMessage struct and local_ai_chat async op - schemas.rs: register local_ai_chat controller (schema, handler, params) in all_controller_schemas + all_registered_controllers Zero cloud tokens are consumed on this path; the call never reaches the backend socket or the /openai/v1/chat/completions endpoint. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When Ollama is ready (isLocalModelActive), handleSendMessage bypasses the cloud socket entirely and routes through openhumanLocalAiChat. Response is segmented and delivered as multiple typed bubbles with natural pauses — human-like reply behaviour at zero cloud token cost. UI / delivery - deliverLocalResponse(): segments full reply via segmentMessage(), dispatches each bubble with getSegmentDelay() pause between them; typing indicator (isDelivering) shows between segments - Socket-connected guard skipped on local path so offline local use works - Cloud socket path (chatSend → chat:done) fully unchanged Frontend RPC - tauriCommands: openhumanLocalAiChat(messages, maxTokens?) wraps openhuman.local_ai_chat via core RPC; LocalAiChatMessage type exported Tests (402 passing) - messageSegmentation: 4 new edge-case tests (whitespace, 80-char boundary, paragraph split, delay scaling) - localChatGating (new file): 9 tests — segmentation correctness, delay bounds [500, 1400] ms, sender→role mapping for message history build Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ules The previous "Minimal — match the user's style" instruction was too loose, causing the model to stack decorative emojis on every message (e.g. "Hey! 😄 Just cooking up some AI magic! 🚀🔥✨"). SOUL.md — add Emoji Rules section: - Hard cap: one emoji maximum per message; none is always acceptable - Contextual, not decorative: emoji must reinforce the specific content (🔥 for exciting news, 🤔 for uncertainty, ✅ for confirmations) - Never open a sentence with an emoji - Skip entirely in error/warning/technical/long responses - Mirror the user's own emoji usage pattern - Concrete good/bad examples so the model can calibrate BOOTSTRAP.md — tighten the Communication Preferences entry to reference the same rules rather than the old vague one-liner. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove unrelated package manager distribution content (was from a different branch). Description now covers only the 4 commits on this branch: socket listener fix, Rust local_ai_chat RPC, frontend local chat gate + multi-bubble delivery, and emoji prompt rules. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughAdds local AI support end-to-end: frontend gating and polling for local model readiness, a Tauri RPC wrapper, Rust Ollama integration and controller, segmented multi-bubble reply delivery with paced delays, reaction toggles, and unit tests for segmentation and gating logic. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Conversations Page
participant LocalStatus as useLocalModelStatus
participant Tauri as Tauri Bridge
participant LocalAI as Local AI Service
participant Seg as Message Segmentation
participant Store as Redux Store
User->>UI: Send message
UI->>LocalStatus: read isLocalModelActive
LocalStatus-->>UI: boolean
alt Local model active
UI->>Tauri: openhumanLocalAiChat(messages)
Tauri->>LocalAI: POST /api/chat (full history)
LocalAI-->>Tauri: response text
Tauri-->>UI: response string
UI->>Seg: segmentMessage(response)
Seg-->>UI: segments[]
loop per segment
UI->>UI: compute getSegmentDelay(segment)
UI->>Store: dispatch addInferenceResponse(segment)
Store-->>UI: state update
UI->>User: render segment (with typing pauses)
end
else Cloud model active
UI->>UI: send via socket/cloud flow (existing)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
6 tasks
…perations - Adjusted formatting in local_ai_chat function for better readability by adding line breaks. - Simplified the mapping of messages to OllamaChatMessage in ops.rs. - Streamlined the await syntax in schemas.rs for clarity. - Enhanced formatting in public_infer.rs for consistency in API request construction.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
app/src/store/threadSlice.ts (1)
1-1:⚠️ Potential issue | 🟡 MinorPipeline failure: Run
prettierto fix code style issues.CI reports formatting issues. Run before merge:
yarn workspace openhuman-app format:fix🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/store/threadSlice.ts` at line 1, The CI failed due to code formatting; run the project's Prettier formatter to fix style in app/src/store/threadSlice.ts (and the repo) — execute the workspace format command (yarn workspace openhuman-app format:fix) or run Prettier/ESLint auto-fix locally, then commit the resulting changes; ensure imports like createAsyncThunk and createSlice remain unchanged after formatting.app/src/services/chatService.ts (1)
1-1:⚠️ Potential issue | 🟡 MinorPipeline failure: Run
prettierto fix code style issues.The CI reports
format:checkfailed. Run formatting before merge:yarn workspace openhuman-app format:fix🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/services/chatService.ts` at line 1, The CI formatting check failed for app/src/services/chatService.ts; run the project formatter to fix style issues by executing the repository command (yarn workspace openhuman-app format:fix) locally, then stage the updated files (including chatService.ts) and amend or add a commit so the PR contains the formatted code; ensure Prettier/formatting changes are the only diffs before pushing.app/src/pages/Conversations.tsx (1)
1-1256:⚠️ Potential issue | 🟡 MinorFix Prettier formatting to unblock CI.
Pipeline reports code style issues. Run
yarn workspace openhuman-app format:checklocally and apply formatting fixes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/pages/Conversations.tsx` around lines 1 - 1256, Prettier formatting errors in the Conversations component are failing CI; run the repo formatter and commit the results: run yarn workspace openhuman-app format:check to see failures, then run the project formatter (e.g., yarn workspace openhuman-app format or the configured Prettier script) to reformat this file (Conversations component / Conversations.tsx) and any affected files, re-run format:check, and commit the updated formatting so CI passes; no code logic changes are required (just reformat and commit).
🧹 Nitpick comments (9)
app/src/store/threadSlice.ts (1)
323-349: Reaction toggle logic is correct but duplicated.The reducer properly toggles reactions in both
messagesByThreadId(persistent storage) andstate.messages(current view) when applicable. The duplicate logic for finding the message and toggling the array could be extracted to a helper function to reduce repetition.Optional refactor to reduce duplication
+const toggleReaction = (msg: ThreadMessage, emoji: string) => { + const prev = (msg.extraMetadata['myReactions'] as string[] | undefined) ?? []; + const idx = prev.indexOf(emoji); + const next = idx >= 0 ? prev.filter(e => e !== emoji) : [...prev, emoji]; + msg.extraMetadata = { ...msg.extraMetadata, myReactions: next }; +}; addReaction: ( state, action: { payload: { threadId: string; messageId: string; emoji: string }; } ) => { const { threadId, messageId, emoji } = action.payload; const stored = state.messagesByThreadId[threadId]; if (stored) { const msg = stored.find(m => m.id === messageId); - if (msg) { - const prev = (msg.extraMetadata['myReactions'] as string[] | undefined) ?? []; - const idx = prev.indexOf(emoji); - const next = idx >= 0 ? prev.filter(e => e !== emoji) : [...prev, emoji]; - msg.extraMetadata = { ...msg.extraMetadata, myReactions: next }; - } + if (msg) toggleReaction(msg, emoji); } if (threadId === state.selectedThreadId) { const viewMsg = state.messages.find(m => m.id === messageId); - if (viewMsg) { - const prev = (viewMsg.extraMetadata['myReactions'] as string[] | undefined) ?? []; - const idx = prev.indexOf(emoji); - const next = idx >= 0 ? prev.filter(e => e !== emoji) : [...prev, emoji]; - viewMsg.extraMetadata = { ...viewMsg.extraMetadata, myReactions: next }; - } + if (viewMsg) toggleReaction(viewMsg, emoji); } },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/store/threadSlice.ts` around lines 323 - 349, The addReaction reducer duplicates the same "find message + toggle myReactions array" logic for stored messages (state.messagesByThreadId) and the current view (state.messages); extract that into a small helper function (e.g., toggleReactionOnMessage(msg, emoji)) that accepts a message object and emoji and performs the existing steps (read myReactions from msg.extraMetadata, compute next by adding/removing emoji, and set msg.extraMetadata = {...msg.extraMetadata, myReactions: next}), then call it from addReaction for both the stored message (found via state.messagesByThreadId[threadId].find(...)) and the view message (found via state.messages.find(...)) to remove the duplicated code while keeping behavior identical.app/src/utils/__tests__/localChatGating.test.ts (1)
73-85: Role mapping tests are tautological and don't exercise production code.These tests duplicate the inline ternary logic rather than importing and testing the actual
transformFrontendMessageToBackendfunction from../intelligenceTransforms. Additionally, the test uses'agent'as the sender value, but according tointelligenceTransforms.ts:59, the frontend actually uses'ai'for non-user messages (sender: backendMessage.role === 'user' ? 'user' : 'ai').Consider either:
- Removing these tests (the logic is trivial), or
- Importing and testing the real transform function with accurate sender values
Example using the real function
+import { transformFrontendMessageToBackend } from '../intelligenceTransforms'; +import type { ChatMessage } from '../../types/chat'; describe('local chat gating — message role mapping', () => { - it('maps sender user → role user', () => { - const sender = 'user'; - const role = sender === 'user' ? 'user' : 'assistant'; - expect(role).toBe('user'); + it('maps sender user → role user', () => { + const msg: ChatMessage = { id: '1', content: 'hi', sender: 'user', timestamp: new Date() }; + const result = transformFrontendMessageToBackend(msg, 'thread-1'); + expect(result.role).toBe('user'); }); - it('maps sender agent → role assistant', () => { - const sender: string = 'agent'; - const role = sender === 'user' ? 'user' : 'assistant'; - expect(role).toBe('assistant'); + it('maps sender ai → role assistant', () => { + const msg: ChatMessage = { id: '2', content: 'hello', sender: 'ai', timestamp: new Date() }; + const result = transformFrontendMessageToBackend(msg, 'thread-1'); + expect(result.role).toBe('assistant'); }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/utils/__tests__/localChatGating.test.ts` around lines 73 - 85, The tests in localChatGating.test.ts are tautological and should either be removed or replaced to exercise the real mapping logic: import transformFrontendMessageToBackend from intelligenceTransforms and call it with representative frontend messages (use sender 'user' for user and 'ai' for non-user per the transform's sender mapping) and assert the returned backendMessage.role is 'user' or appropriate assistant role; alternatively delete these trivial tests if you prefer no coverage. Ensure you reference the transformFrontendMessageToBackend function when changing the tests and replace the hardcoded ternary assertions with assertions against the function's output using 'ai' (not 'agent') for non-user senders.app/src/utils/messageSegmentation.ts (2)
111-121:groupSentencescan return an empty array, causing fallback to fail silently.If all grouped sentences are shorter than
MIN_SEGMENT_CHARS(40), the filter on line 120 returns an empty array. The caller at line 42 checksgrouped.length >= 2, so it falls back correctly, but this is semantically odd—the function claims to "group sentences" but may return nothing even when valid sentences exist.Consider filtering before returning or adjusting the min-length check to preserve at least one group:
Suggested adjustment
function groupSentences(sentences: string[]): string[] { const targetCount = Math.min(3, Math.ceil(sentences.length / 2)); const groupSize = Math.ceil(sentences.length / targetCount); const groups: string[] = []; for (let i = 0; i < sentences.length; i += groupSize) { groups.push(sentences.slice(i, i + groupSize).join(' ')); } - return groups.filter(g => g.length >= MIN_SEGMENT_CHARS); + const filtered = groups.filter(g => g.length >= MIN_SEGMENT_CHARS); + // Preserve at least all groups if filtering would empty the result + return filtered.length > 0 ? filtered : groups; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/utils/messageSegmentation.ts` around lines 111 - 121, The groupSentences function currently filters out groups shorter than MIN_SEGMENT_CHARS and can return an empty array; update groupSentences to enforce the min-length without losing all groups by: after building groups (in function groupSentences), apply the MIN_SEGMENT_CHARS filter but if that yields an empty array, instead return a single merged group (e.g., groups.join(' ')) or return the longest group so the caller still receives at least one segment; reference symbols: groupSentences, MIN_SEGMENT_CHARS, and the groups variable when implementing this fallback.
80-105: Sentence splitting has false positives for abbreviations.The heuristic splits on
.followed by space and uppercase, which incorrectly splits text like:
- "Dr. Smith said hello." → splits after "Dr."
- "The U.S. Army arrived." → splits after "U." and "S."
This is a known hard problem. For the local-chat UX, occasional over-splitting is likely acceptable, but consider documenting this limitation or using a sentence-boundary library if precision becomes important.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/utils/messageSegmentation.ts` around lines 80 - 105, The splitSentences function's heuristic incorrectly splits on abbreviations (e.g., "Dr.", initials like "U.", "S."); update the splitting condition in splitSentences to skip splits when the period is part of a known abbreviation or an initial: extract the token before the dot (e.g., scan back to whitespace or start), and if it matches a small exceptions set (["Dr","Mr","Mrs","Ms","Prof","Sr","Jr","vs","etc"] case-insensitive) or is a single uppercase letter (initial), do not treat the dot as a sentence terminator; implement this by adding a helper check (isAbbreviationOrInitial) used in the if that currently checks (ch === '.' || ...), and add a short comment documenting the limitation and the decision to use a small exceptions list rather than a full NLP library.src/openhuman/local_ai/ops.rs (1)
461-467: Consider validatingrolevalues before passing to Ollama.The
LocalAiChatMessage.rolefield accepts any string, which is passed directly to Ollama's API. Invalid roles (e.g., typos like"assitant"or unexpected values like"system") may cause silent failures or unexpected behavior. Consider validating roles at the RPC boundary:Optional validation
+const VALID_ROLES: &[&str] = &["user", "assistant", "system"]; pub async fn local_ai_chat( config: &Config, messages: Vec<LocalAiChatMessage>, max_tokens: Option<u32>, ) -> Result<RpcOutcome<String>, String> { // ... if messages.is_empty() { return Err("messages must not be empty".to_string()); } + for msg in &messages { + if !VALID_ROLES.contains(&msg.role.as_str()) { + return Err(format!("invalid role '{}': expected user, assistant, or system", msg.role)); + } + } let ollama_messages: Vec<...> = messages🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/local_ai/ops.rs` around lines 461 - 467, The code maps LocalAiChatMessage.role directly into OllamaChatMessage.role (creating ollama_messages) which allows invalid roles to be sent; update the conversion in ops.rs to validate and normalize roles before constructing OllamaChatMessage (e.g., accept only "user", "assistant", "system" or explicitly map known aliases, and return an error or default to "user" for invalid values at the RPC boundary). Modify the message conversion logic that produces ollama_messages (where LocalAiChatMessage.role is read and OllamaChatMessage is created) to perform this validation/mapping and surface a clear error when a role is unrecognized.src/openhuman/local_ai/service/public_infer.rs (1)
136-138: Bootstrap call may silently fail without surfacing errors.If
bootstrapfails internally (e.g., Ollama not running), the method continues and the subsequent HTTP call to/api/chatwill fail. This is consistent with the existinginference()method pattern, but consider whether a bootstrap failure should short-circuit with an error message rather than proceeding to a likely-failing HTTP request.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/local_ai/service/public_infer.rs` around lines 136 - 138, The bootstrap call in public_infer.rs currently ignores failures and proceeds to make the /api/chat request; change the flow in the method containing the lines with self.bootstrap(config).await so that you await and check the Result/return value from bootstrap, and if it fails propagate or return an Err (or an appropriate EarlyReturn) instead of continuing; update the call site (the method that mirrors inference()) to short-circuit on bootstrap failure and log/return a clear error indicating bootstrap failed (reference self.bootstrap(config).await, self.status.lock().state and the enclosing inference-like method).app/src/hooks/useLocalModelStatus.ts (1)
1-44: Add debug logging for the polling flow.Per coding guidelines, new flows should include namespaced debug logging. Consider adding
console.debugcalls to trace polling lifecycle (start, poll results, cleanup).🔧 Suggested logging additions
export function useLocalModelStatus(): boolean { const [isActive, setIsActive] = useState(false); const mountedRef = useRef(true); useEffect(() => { mountedRef.current = true; - if (!isTauri()) return; + if (!isTauri()) { + console.debug('[useLocalModelStatus] skipped: not Tauri environment'); + return; + } const check = async () => { try { const res = await openhumanLocalAiStatus(); + console.debug('[useLocalModelStatus] poll result', { state: res.result?.state }); if (mountedRef.current) { setIsActive(res.result?.state === 'ready'); } } catch { if (mountedRef.current) setIsActive(false); } }; void check(); const id = setInterval(() => void check(), POLL_INTERVAL_MS); return () => { + console.debug('[useLocalModelStatus] cleanup'); mountedRef.current = false; clearInterval(id); }; }, []);As per coding guidelines: "Add substantial debug logging on new/changed flows using namespaced debug logs in React/app code".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/hooks/useLocalModelStatus.ts` around lines 1 - 44, Add namespaced debug logging to the polling flow in useLocalModelStatus: when the effect starts (after isTauri() check) log a start message including POLL_INTERVAL_MS and the interval id when created, log each poll attempt and the result inside check (including the resolved res.result?.state or error), and log cleanup when the effect returns; use a consistent namespace like 'hooks/useLocalModelStatus' and place logs near the isTauri() guard, inside the async check function (success and catch branches), and in the cleanup block where mountedRef and clearInterval(id) are invoked.app/src/pages/Conversations.tsx (1)
353-394: Extract shared multi-bubble delivery logic to reduce duplication.The segmentation and paced delivery logic is duplicated between the
onDonehandler (lines 360-394) anddeliverLocalResponse(lines 445-472). Consider extracting a shared helper to reduce code duplication and ensure consistent behavior.♻️ Suggested extraction
// Shared helper for paced multi-bubble delivery const deliverSegmentedResponse = async ( fullResponse: string, threadId: string ): Promise<void> => { const segments = segmentMessage(fullResponse); if (segments.length <= 1) { dispatch(addInferenceResponse({ content: fullResponse, threadId })); return; } setIsDelivering(true); deliveryActiveRef.current = true; for (let i = 0; i < segments.length; i++) { if (!deliveryActiveRef.current) break; if (i > 0) { await new Promise<void>(resolve => setTimeout(resolve, getSegmentDelay(segments[i - 1])) ); } if (!deliveryActiveRef.current) break; dispatch(addInferenceResponse({ content: segments[i], threadId })); } deliveryActiveRef.current = false; setIsDelivering(false); };Then use in both locations:
onDone:void deliverSegmentedResponse(event.full_response, event.thread_id).then(() => { setIsSending(false); });deliverLocalResponse: simply callawait deliverSegmentedResponse(reply, threadId);Also applies to: 445-472
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/pages/Conversations.tsx` around lines 353 - 394, The onDone handler and deliverLocalResponse both duplicate segmentation and paced delivery logic; extract that into a shared async helper (e.g., deliverSegmentedResponse) that accepts (fullResponse: string, threadId: string) and uses segmentMessage, dispatch(addInferenceResponse(...)), deliveryActiveRef, setIsDelivering and getSegmentDelay internally; then replace the duplicated blocks in the onDone handler and deliverLocalResponse with calls to deliverSegmentedResponse and ensure callers handle setting setIsSending(false) (onDone should await or .then to setIsSending(false) after the helper resolves, while deliverLocalResponse should await the helper before continuing).app/src/utils/tauriCommands.ts (1)
1332-1334: Unused interfaceLocalAiChatResult.
LocalAiChatResultis defined butopenhumanLocalAiChatreturnsCommandResponse<string>, notCommandResponse<LocalAiChatResult>. Either remove this dead interface or update the function's return type if it should match a structured response.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/utils/tauriCommands.ts` around lines 1332 - 1334, The LocalAiChatResult interface is unused because openhumanLocalAiChat currently returns CommandResponse<string>; either delete the dead interface LocalAiChatResult, or change openhumanLocalAiChat's return type to CommandResponse<LocalAiChatResult> and update any call sites that expect a string to handle the structured result; search for the function name openhumanLocalAiChat and the type CommandResponse<string> to locate where to update the signature and related handling if you choose the latter.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/utils/__tests__/messageSegmentation.test.ts`:
- Around line 103-108: The test for segmentMessage is using a too-weak assertion
(expect(result.length).toBeGreaterThanOrEqual(1)) which always passes; update
the assertion to reflect the intended behavior that the two paragraphs should
split into two segments — change it to
expect(result.length).toBeGreaterThanOrEqual(2) or preferably
expect(result.length).toEqual(2) in the it block that constructs text =
'a'.repeat(40) + '\n\n' + 'b'.repeat(40) so the test actually verifies
segmentMessage produces two segments.
- Around line 93-97: The test reveals segmentMessage currently returns [''] for
whitespace-only input; update segmentMessage so that after trimming each segment
you discard empty results (e.g., if trimmed input === '' return []), or
otherwise filter out empty strings before returning; update the test
expectations to expect [] (and ensure callers that rely on empty segments are
adjusted to handle no segments). Reference the function name segmentMessage and
alter its trimming/filtering logic to return an empty array for inputs that are
only whitespace.
In `@src/openhuman/local_ai/ops.rs`:
- Around line 460-476: The file has formatting issues flagged by CI around the
creation of ollama_messages and the subsequent service/chat_with_history call;
run rustfmt (cargo fmt --all) to reformat the file so the map->collect block and
the call chain to local_ai::global(config) and chat_with_history(config,
ollama_messages, max_tokens) follow the project’s style; after formatting,
re-run cargo fmt --check to ensure lines referencing OllamaChatMessage,
local_ai::global, chat_with_history, tracing::debug, and RpcOutcome::single_log
are properly wrapped and aligned.
In `@src/openhuman/local_ai/schemas.rs`:
- Around line 855-872: Run rustfmt (cargo fmt --all) to fix CI formatting;
specifically reformat the handle_local_ai_chat function so the call to
crate::openhuman::local_ai::rpc::local_ai_chat(...) and its .await are laid out
per rustfmt rules (preserve the to_json(...) wrapping and proper indentation of
the awaited call and its arguments: config, messages, p.max_tokens). After
formatting, verify the signature and use of
deserialize_params::<LocalAiChatParams>, messages mapping, and to_json call
remain unchanged and that the file compiles.
In `@src/openhuman/local_ai/service/public_infer.rs`:
- Around line 152-162: The struct literal for OllamaChatRequest (containing
OllamaGenerateOptions) and the subsequent .post(format!(...)) chain are
misformatted; run cargo fmt --all to automatically reformat the file or manually
apply rustfmt style so the OllamaGenerateOptions fields and the chained
.post(format!(...)) call adhere to rustfmt conventions (locate the
OllamaChatRequest construction and the .post(format!(...)) chain in
public_infer.rs and reformat them consistently).
---
Outside diff comments:
In `@app/src/pages/Conversations.tsx`:
- Around line 1-1256: Prettier formatting errors in the Conversations component
are failing CI; run the repo formatter and commit the results: run yarn
workspace openhuman-app format:check to see failures, then run the project
formatter (e.g., yarn workspace openhuman-app format or the configured Prettier
script) to reformat this file (Conversations component / Conversations.tsx) and
any affected files, re-run format:check, and commit the updated formatting so CI
passes; no code logic changes are required (just reformat and commit).
In `@app/src/services/chatService.ts`:
- Line 1: The CI formatting check failed for app/src/services/chatService.ts;
run the project formatter to fix style issues by executing the repository
command (yarn workspace openhuman-app format:fix) locally, then stage the
updated files (including chatService.ts) and amend or add a commit so the PR
contains the formatted code; ensure Prettier/formatting changes are the only
diffs before pushing.
In `@app/src/store/threadSlice.ts`:
- Line 1: The CI failed due to code formatting; run the project's Prettier
formatter to fix style in app/src/store/threadSlice.ts (and the repo) — execute
the workspace format command (yarn workspace openhuman-app format:fix) or run
Prettier/ESLint auto-fix locally, then commit the resulting changes; ensure
imports like createAsyncThunk and createSlice remain unchanged after formatting.
---
Nitpick comments:
In `@app/src/hooks/useLocalModelStatus.ts`:
- Around line 1-44: Add namespaced debug logging to the polling flow in
useLocalModelStatus: when the effect starts (after isTauri() check) log a start
message including POLL_INTERVAL_MS and the interval id when created, log each
poll attempt and the result inside check (including the resolved
res.result?.state or error), and log cleanup when the effect returns; use a
consistent namespace like 'hooks/useLocalModelStatus' and place logs near the
isTauri() guard, inside the async check function (success and catch branches),
and in the cleanup block where mountedRef and clearInterval(id) are invoked.
In `@app/src/pages/Conversations.tsx`:
- Around line 353-394: The onDone handler and deliverLocalResponse both
duplicate segmentation and paced delivery logic; extract that into a shared
async helper (e.g., deliverSegmentedResponse) that accepts (fullResponse:
string, threadId: string) and uses segmentMessage,
dispatch(addInferenceResponse(...)), deliveryActiveRef, setIsDelivering and
getSegmentDelay internally; then replace the duplicated blocks in the onDone
handler and deliverLocalResponse with calls to deliverSegmentedResponse and
ensure callers handle setting setIsSending(false) (onDone should await or .then
to setIsSending(false) after the helper resolves, while deliverLocalResponse
should await the helper before continuing).
In `@app/src/store/threadSlice.ts`:
- Around line 323-349: The addReaction reducer duplicates the same "find message
+ toggle myReactions array" logic for stored messages (state.messagesByThreadId)
and the current view (state.messages); extract that into a small helper function
(e.g., toggleReactionOnMessage(msg, emoji)) that accepts a message object and
emoji and performs the existing steps (read myReactions from msg.extraMetadata,
compute next by adding/removing emoji, and set msg.extraMetadata =
{...msg.extraMetadata, myReactions: next}), then call it from addReaction for
both the stored message (found via state.messagesByThreadId[threadId].find(...))
and the view message (found via state.messages.find(...)) to remove the
duplicated code while keeping behavior identical.
In `@app/src/utils/__tests__/localChatGating.test.ts`:
- Around line 73-85: The tests in localChatGating.test.ts are tautological and
should either be removed or replaced to exercise the real mapping logic: import
transformFrontendMessageToBackend from intelligenceTransforms and call it with
representative frontend messages (use sender 'user' for user and 'ai' for
non-user per the transform's sender mapping) and assert the returned
backendMessage.role is 'user' or appropriate assistant role; alternatively
delete these trivial tests if you prefer no coverage. Ensure you reference the
transformFrontendMessageToBackend function when changing the tests and replace
the hardcoded ternary assertions with assertions against the function's output
using 'ai' (not 'agent') for non-user senders.
In `@app/src/utils/messageSegmentation.ts`:
- Around line 111-121: The groupSentences function currently filters out groups
shorter than MIN_SEGMENT_CHARS and can return an empty array; update
groupSentences to enforce the min-length without losing all groups by: after
building groups (in function groupSentences), apply the MIN_SEGMENT_CHARS filter
but if that yields an empty array, instead return a single merged group (e.g.,
groups.join(' ')) or return the longest group so the caller still receives at
least one segment; reference symbols: groupSentences, MIN_SEGMENT_CHARS, and the
groups variable when implementing this fallback.
- Around line 80-105: The splitSentences function's heuristic incorrectly splits
on abbreviations (e.g., "Dr.", initials like "U.", "S."); update the splitting
condition in splitSentences to skip splits when the period is part of a known
abbreviation or an initial: extract the token before the dot (e.g., scan back to
whitespace or start), and if it matches a small exceptions set
(["Dr","Mr","Mrs","Ms","Prof","Sr","Jr","vs","etc"] case-insensitive) or is a
single uppercase letter (initial), do not treat the dot as a sentence
terminator; implement this by adding a helper check (isAbbreviationOrInitial)
used in the if that currently checks (ch === '.' || ...), and add a short
comment documenting the limitation and the decision to use a small exceptions
list rather than a full NLP library.
In `@app/src/utils/tauriCommands.ts`:
- Around line 1332-1334: The LocalAiChatResult interface is unused because
openhumanLocalAiChat currently returns CommandResponse<string>; either delete
the dead interface LocalAiChatResult, or change openhumanLocalAiChat's return
type to CommandResponse<LocalAiChatResult> and update any call sites that expect
a string to handle the structured result; search for the function name
openhumanLocalAiChat and the type CommandResponse<string> to locate where to
update the signature and related handling if you choose the latter.
In `@src/openhuman/local_ai/ops.rs`:
- Around line 461-467: The code maps LocalAiChatMessage.role directly into
OllamaChatMessage.role (creating ollama_messages) which allows invalid roles to
be sent; update the conversion in ops.rs to validate and normalize roles before
constructing OllamaChatMessage (e.g., accept only "user", "assistant", "system"
or explicitly map known aliases, and return an error or default to "user" for
invalid values at the RPC boundary). Modify the message conversion logic that
produces ollama_messages (where LocalAiChatMessage.role is read and
OllamaChatMessage is created) to perform this validation/mapping and surface a
clear error when a role is unrecognized.
In `@src/openhuman/local_ai/service/public_infer.rs`:
- Around line 136-138: The bootstrap call in public_infer.rs currently ignores
failures and proceeds to make the /api/chat request; change the flow in the
method containing the lines with self.bootstrap(config).await so that you await
and check the Result/return value from bootstrap, and if it fails propagate or
return an Err (or an appropriate EarlyReturn) instead of continuing; update the
call site (the method that mirrors inference()) to short-circuit on bootstrap
failure and log/return a clear error indicating bootstrap failed (reference
self.bootstrap(config).await, self.status.lock().state and the enclosing
inference-like method).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bca0ce98-3eab-4c7d-ab0a-4a193ff37872
⛔ Files ignored due to path filters (1)
app/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
app/src/hooks/useLocalModelStatus.tsapp/src/pages/Conversations.tsxapp/src/services/chatService.tsapp/src/store/threadSlice.tsapp/src/utils/__tests__/localChatGating.test.tsapp/src/utils/__tests__/messageSegmentation.test.tsapp/src/utils/messageSegmentation.tsapp/src/utils/tauriCommands.tssrc/openhuman/agent/prompts/BOOTSTRAP.mdsrc/openhuman/agent/prompts/SOUL.mdsrc/openhuman/local_ai/ollama_api.rssrc/openhuman/local_ai/ops.rssrc/openhuman/local_ai/schemas.rssrc/openhuman/local_ai/service/public_infer.rs
Comment on lines
+93
to
+97
| describe('edge cases', () => { | ||
| it('handles text with only whitespace', () => { | ||
| expect(segmentMessage(' ')).toEqual(['']); | ||
| }); | ||
|
|
Contributor
There was a problem hiding this comment.
Whitespace-only input returns [''] — consider if this is intended behavior.
The test on line 95 expects segmentMessage(' ') to return ['']. While technically correct (trim produces empty string), rendering an empty chat bubble in the UI seems unintended. Consider whether segmentMessage should return [] for empty/whitespace input, or if callers should filter out empty segments before rendering.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/utils/__tests__/messageSegmentation.test.ts` around lines 93 - 97,
The test reveals segmentMessage currently returns [''] for whitespace-only
input; update segmentMessage so that after trimming each segment you discard
empty results (e.g., if trimmed input === '' return []), or otherwise filter out
empty strings before returning; update the test expectations to expect [] (and
ensure callers that rely on empty segments are adjusted to handle no segments).
Reference the function name segmentMessage and alter its trimming/filtering
logic to return an empty array for inputs that are only whitespace.
Comment on lines
+103
to
+108
| it('handles exactly 81 characters with a paragraph break — may split', () => { | ||
| const text = 'a'.repeat(40) + '\n\n' + 'b'.repeat(40); | ||
| const result = segmentMessage(text); | ||
| // Both paragraphs are >= MIN_SEGMENT_CHARS so should split | ||
| expect(result.length).toBeGreaterThanOrEqual(1); | ||
| }); |
Contributor
There was a problem hiding this comment.
Assertion is too weak — length >= 1 is always true.
The test title says "may split" for 81 characters with a paragraph break, and the comment says "Both paragraphs are >= MIN_SEGMENT_CHARS so should split." However, the assertion expect(result.length).toBeGreaterThanOrEqual(1) passes even if no splitting occurs. Consider asserting toEqual(2) or at least toBeGreaterThanOrEqual(2) to match the expected behavior.
Suggested fix
it('handles exactly 81 characters with a paragraph break — may split', () => {
const text = 'a'.repeat(40) + '\n\n' + 'b'.repeat(40);
const result = segmentMessage(text);
// Both paragraphs are >= MIN_SEGMENT_CHARS so should split
- expect(result.length).toBeGreaterThanOrEqual(1);
+ expect(result.length).toBe(2);
});📝 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
| it('handles exactly 81 characters with a paragraph break — may split', () => { | |
| const text = 'a'.repeat(40) + '\n\n' + 'b'.repeat(40); | |
| const result = segmentMessage(text); | |
| // Both paragraphs are >= MIN_SEGMENT_CHARS so should split | |
| expect(result.length).toBeGreaterThanOrEqual(1); | |
| }); | |
| it('handles exactly 81 characters with a paragraph break — may split', () => { | |
| const text = 'a'.repeat(40) + '\n\n' + 'b'.repeat(40); | |
| const result = segmentMessage(text); | |
| // Both paragraphs are >= MIN_SEGMENT_CHARS so should split | |
| expect(result.length).toBe(2); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/utils/__tests__/messageSegmentation.test.ts` around lines 103 - 108,
The test for segmentMessage is using a too-weak assertion
(expect(result.length).toBeGreaterThanOrEqual(1)) which always passes; update
the assertion to reflect the intended behavior that the two paragraphs should
split into two segments — change it to
expect(result.length).toBeGreaterThanOrEqual(2) or preferably
expect(result.length).toEqual(2) in the it block that constructs text =
'a'.repeat(40) + '\n\n' + 'b'.repeat(40) so the test actually verifies
segmentMessage produces two segments.
…nversations component - Improved formatting and consistency in the Conversations component, including better alignment of dispatch calls and message handling logic. - Removed redundant imports and streamlined the mapping of stored messages for clarity. - Adjusted conditional rendering for improved readability in the UI logic.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/openhuman/local_ai/schemas.rs`:
- Around line 476-490: The ControllerSchema for "local_ai_chat" declares the
output field as json_output("reply", ...) which is inconsistent with the actual
serialized key produced by RpcOutcome::into_cli_compatible_json()/to_json (it
emits "result"); update the ControllerSchema outputs for the "local_ai_chat"
controller to use json_output("result", ...) (or otherwise rename to "result")
so the schema matches the runtime serialization and the frontend
LocalAiChatResult interface.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cc0a4a22-6a42-4e66-8403-4541f174d92c
📒 Files selected for processing (4)
PR_DESCRIPTION.mdsrc/openhuman/local_ai/ops.rssrc/openhuman/local_ai/schemas.rssrc/openhuman/local_ai/service/public_infer.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/openhuman/local_ai/service/public_infer.rs
Comment on lines
+476
to
+490
| "local_ai_chat" => ControllerSchema { | ||
| namespace: "local_ai", | ||
| function: "chat", | ||
| description: "Multi-turn chat completion via local Ollama model. Does not call the cloud API.", | ||
| inputs: vec![ | ||
| FieldSchema { | ||
| name: "messages", | ||
| ty: TypeSchema::Array(Box::new(TypeSchema::Json)), | ||
| comment: "Chat message history [{role, content}]. Last entry is the user turn.", | ||
| required: true, | ||
| }, | ||
| optional_u64("max_tokens", "Optional max output tokens."), | ||
| ], | ||
| outputs: vec![json_output("reply", "Assistant reply text.")], | ||
| }, |
Contributor
There was a problem hiding this comment.
Schema output name "reply" doesn't match actual serialized key "result".
The schema declares output as json_output("reply", ...) but RpcOutcome::into_cli_compatible_json() (called via to_json) serializes to { "result": value, "logs": [...] }. The frontend LocalAiChatResult interface in tauriCommands.ts correctly expects result, so runtime behavior is fine—but the schema documentation is misleading for API consumers.
📝 Suggested fix for consistency
"local_ai_chat" => ControllerSchema {
namespace: "local_ai",
function: "chat",
description: "Multi-turn chat completion via local Ollama model. Does not call the cloud API.",
inputs: vec![
FieldSchema {
name: "messages",
ty: TypeSchema::Array(Box::new(TypeSchema::Json)),
comment: "Chat message history [{role, content}]. Last entry is the user turn.",
required: true,
},
optional_u64("max_tokens", "Optional max output tokens."),
],
- outputs: vec![json_output("reply", "Assistant reply text.")],
+ outputs: vec![json_output("result", "Assistant reply text.")],
},📝 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
| "local_ai_chat" => ControllerSchema { | |
| namespace: "local_ai", | |
| function: "chat", | |
| description: "Multi-turn chat completion via local Ollama model. Does not call the cloud API.", | |
| inputs: vec![ | |
| FieldSchema { | |
| name: "messages", | |
| ty: TypeSchema::Array(Box::new(TypeSchema::Json)), | |
| comment: "Chat message history [{role, content}]. Last entry is the user turn.", | |
| required: true, | |
| }, | |
| optional_u64("max_tokens", "Optional max output tokens."), | |
| ], | |
| outputs: vec![json_output("reply", "Assistant reply text.")], | |
| }, | |
| "local_ai_chat" => ControllerSchema { | |
| namespace: "local_ai", | |
| function: "chat", | |
| description: "Multi-turn chat completion via local Ollama model. Does not call the cloud API.", | |
| inputs: vec![ | |
| FieldSchema { | |
| name: "messages", | |
| ty: TypeSchema::Array(Box::new(TypeSchema::Json)), | |
| comment: "Chat message history [{role, content}]. Last entry is the user turn.", | |
| required: true, | |
| }, | |
| optional_u64("max_tokens", "Optional max output tokens."), | |
| ], | |
| outputs: vec![json_output("result", "Assistant reply text.")], | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/openhuman/local_ai/schemas.rs` around lines 476 - 490, The
ControllerSchema for "local_ai_chat" declares the output field as
json_output("reply", ...) which is inconsistent with the actual serialized key
produced by RpcOutcome::into_cli_compatible_json()/to_json (it emits "result");
update the ControllerSchema outputs for the "local_ai_chat" controller to use
json_output("result", ...) (or otherwise rename to "result") so the schema
matches the runtime serialization and the frontend LocalAiChatResult interface.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/src/pages/Conversations.tsx (1)
442-472: Duplicated delivery logic withonDonehandler.The segmented delivery loop in
deliverLocalResponse(lines 458–471) is nearly identical to lines 378–396 in theonDonehandler. Consider extracting a shared helper to reduce duplication.♻️ Proposed extraction of shared delivery helper
// Extract to a helper (could be placed above the component or in a separate file) const deliverSegmentedResponse = async ( segments: string[], threadId: string, deliveryActiveRef: React.MutableRefObject<boolean>, dispatch: ReturnType<typeof useAppDispatch>, onComplete: () => void ) => { for (let i = 0; i < segments.length; i++) { if (!deliveryActiveRef.current) break; if (i > 0) { await new Promise<void>(resolve => setTimeout(resolve, getSegmentDelay(segments[i - 1])) ); } if (!deliveryActiveRef.current) break; dispatch(addInferenceResponse({ content: segments[i], threadId })); } if (deliveryActiveRef.current) { deliveryActiveRef.current = false; onComplete(); } };Then use it in both
onDoneanddeliverLocalResponse.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/pages/Conversations.tsx` around lines 442 - 472, The segmented delivery loop in deliverLocalResponse duplicates the logic in the onDone handler; extract that loop into a shared helper (e.g., deliverSegmentedResponse) that accepts segments: string[], threadId, deliveryActiveRef, dispatch, and an onComplete callback, and uses getSegmentDelay and dispatch(addInferenceResponse(...)) internally; then replace the loop in both deliverLocalResponse and onDone with calls to this helper and ensure the helper sets deliveryActiveRef.current = false and invokes onComplete (which can call setIsDelivering(false)) when finished or aborted.app/src/store/threadSlice.ts (1)
323-347: Duplicated reaction toggle logic can be extracted.The toggle logic (lines 332–335 and 341–344) is identical. Consider extracting a helper to reduce duplication and improve maintainability.
Also, since
extraMetadatais typed asRecord<string, unknown>, themyReactionsproperty is not formally declared onThreadMessage. This works at runtime but relies on structural typing. If messages are ever replaced wholesale from a server payload that lacksmyReactions, local reactions will be silently lost.♻️ Proposed refactor to extract toggle helper
+const toggleReaction = (msg: ThreadMessage, emoji: string): void => { + const prev = (msg.extraMetadata['myReactions'] as string[] | undefined) ?? []; + const idx = prev.indexOf(emoji); + const next = idx >= 0 ? prev.filter(e => e !== emoji) : [...prev, emoji]; + msg.extraMetadata = { ...msg.extraMetadata, myReactions: next }; +}; + addReaction: ( state, action: { payload: { threadId: string; messageId: string; emoji: string } } ) => { const { threadId, messageId, emoji } = action.payload; const stored = state.messagesByThreadId[threadId]; if (stored) { const msg = stored.find(m => m.id === messageId); - if (msg) { - const prev = (msg.extraMetadata['myReactions'] as string[] | undefined) ?? []; - const idx = prev.indexOf(emoji); - const next = idx >= 0 ? prev.filter(e => e !== emoji) : [...prev, emoji]; - msg.extraMetadata = { ...msg.extraMetadata, myReactions: next }; - } + if (msg) toggleReaction(msg, emoji); } if (threadId === state.selectedThreadId) { const viewMsg = state.messages.find(m => m.id === messageId); - if (viewMsg) { - const prev = (viewMsg.extraMetadata['myReactions'] as string[] | undefined) ?? []; - const idx = prev.indexOf(emoji); - const next = idx >= 0 ? prev.filter(e => e !== emoji) : [...prev, emoji]; - viewMsg.extraMetadata = { ...viewMsg.extraMetadata, myReactions: next }; - } + if (viewMsg) toggleReaction(viewMsg, emoji); } },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/store/threadSlice.ts` around lines 323 - 347, The addReaction reducer contains duplicated toggle logic; extract a small helper (e.g., toggleMyReaction(extraMetadata: Record<string, unknown>, emoji: string): string[]) and call it from both places to compute the new myReactions array, then set msg.extraMetadata = { ...msg.extraMetadata, myReactions: result } and likewise for viewMsg; also ensure the ThreadMessage type or its extraMetadata handling formally acknowledges myReactions (add myReactions?: string[] to the ThreadMessage interface or merge server message payloads with existing myReactions when replacing messages) so local reactions aren’t lost when messages are replaced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/pages/Conversations.tsx`:
- Around line 377-396: The final state updates after the delivery loop
(setIsDelivering(false), setIsSending(false)) can run after unmount; wrap the
async IIFE in a try/finally (or check a mountedRef) so the finally only calls
setIsDelivering/setIsSending if the component is still mounted (e.g.,
mountedRef.current) and ensure deliveryActiveRef is still honored; also add
namespaced debug logs (e.g., "Conversations:delivery") around loop start, each
addInferenceResponse(dispatch) call, delays (getSegmentDelay), and the
cleanup/finally block for observability. Reference: deliveryActiveRef, segments,
getSegmentDelay, addInferenceResponse, setIsDelivering, setIsSending.
---
Nitpick comments:
In `@app/src/pages/Conversations.tsx`:
- Around line 442-472: The segmented delivery loop in deliverLocalResponse
duplicates the logic in the onDone handler; extract that loop into a shared
helper (e.g., deliverSegmentedResponse) that accepts segments: string[],
threadId, deliveryActiveRef, dispatch, and an onComplete callback, and uses
getSegmentDelay and dispatch(addInferenceResponse(...)) internally; then replace
the loop in both deliverLocalResponse and onDone with calls to this helper and
ensure the helper sets deliveryActiveRef.current = false and invokes onComplete
(which can call setIsDelivering(false)) when finished or aborted.
In `@app/src/store/threadSlice.ts`:
- Around line 323-347: The addReaction reducer contains duplicated toggle logic;
extract a small helper (e.g., toggleMyReaction(extraMetadata: Record<string,
unknown>, emoji: string): string[]) and call it from both places to compute the
new myReactions array, then set msg.extraMetadata = { ...msg.extraMetadata,
myReactions: result } and likewise for viewMsg; also ensure the ThreadMessage
type or its extraMetadata handling formally acknowledges myReactions (add
myReactions?: string[] to the ThreadMessage interface or merge server message
payloads with existing myReactions when replacing messages) so local reactions
aren’t lost when messages are replaced.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8aad965c-4d63-48d8-aa75-571e4ff952c4
📒 Files selected for processing (2)
app/src/pages/Conversations.tsxapp/src/store/threadSlice.ts
Comment on lines
+377
to
+396
| void (async () => { | ||
| for (let i = 0; i < segments.length; i++) { | ||
| if (!deliveryActiveRef.current) break; | ||
|
|
||
| if (i > 0) { | ||
| await new Promise<void>(resolve => | ||
| setTimeout(resolve, getSegmentDelay(segments[i - 1])) | ||
| ); | ||
| } | ||
|
|
||
| if (!deliveryActiveRef.current) break; | ||
|
|
||
| dispatch(addInferenceResponse({ content: segments[i], threadId: event.thread_id })); | ||
| } | ||
|
|
||
| deliveryActiveRef.current = false; | ||
| setIsDelivering(false); | ||
| setIsSending(false); | ||
| // activeThreadId was already cleared by the first addInferenceResponse dispatch | ||
| })(); |
Contributor
There was a problem hiding this comment.
Potential state update after unmount; consider adding debug logging.
The delivery loop checks deliveryActiveRef before each dispatch, which is good. However, after the loop completes (line 392–394), setIsDelivering(false) and setIsSending(false) could execute on an unmounted component if unmount happened between the last ref check and these calls. This is a minor issue since React 18+ suppresses the warning, but you could guard with a mounted check or move these into a finally that respects the ref.
Also, per coding guidelines, consider adding debug logging for observability of this new multi-bubble delivery flow.
🛡️ Proposed fix to guard state updates
void (async () => {
for (let i = 0; i < segments.length; i++) {
if (!deliveryActiveRef.current) break;
if (i > 0) {
await new Promise<void>(resolve =>
setTimeout(resolve, getSegmentDelay(segments[i - 1]))
);
}
if (!deliveryActiveRef.current) break;
dispatch(addInferenceResponse({ content: segments[i], threadId: event.thread_id }));
}
- deliveryActiveRef.current = false;
- setIsDelivering(false);
- setIsSending(false);
- // activeThreadId was already cleared by the first addInferenceResponse dispatch
+ if (deliveryActiveRef.current) {
+ deliveryActiveRef.current = false;
+ setIsDelivering(false);
+ setIsSending(false);
+ }
+ console.debug('[conversations:cloud] multi-bubble delivery complete', {
+ threadId: event.thread_id,
+ segmentCount: segments.length,
+ });
})();Based on coding guidelines: "Add substantial debug logging on new/changed flows using namespaced debug logs in React/app code"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/pages/Conversations.tsx` around lines 377 - 396, The final state
updates after the delivery loop (setIsDelivering(false), setIsSending(false))
can run after unmount; wrap the async IIFE in a try/finally (or check a
mountedRef) so the finally only calls setIsDelivering/setIsSending if the
component is still mounted (e.g., mountedRef.current) and ensure
deliveryActiveRef is still honored; also add namespaced debug logs (e.g.,
"Conversations:delivery") around loop start, each addInferenceResponse(dispatch)
call, delays (getSegmentDelay), and the cleanup/finally block for observability.
Reference: deliveryActiveRef, segments, getSegmentDelay, addInferenceResponse,
setIsDelivering, setIsSending.
This was referenced Apr 1, 2026
This was referenced Apr 12, 2026
AusAgentSmith
pushed a commit
to AusAgentSmith/openhuman
that referenced
this pull request
May 23, 2026
AusAgentSmith
pushed a commit
to AusAgentSmith/openhuman
that referenced
this pull request
May 23, 2026
* fix(chat): prevent stacked socket listeners on reconnect subscribeChatEvents was declared async despite having no awaits, so the cleanup function was returned in a microtask after React's synchronous cleanup had already run. Each socket reconnect added another layer of listeners that were never removed, causing chat:done to fire N times and produce duplicate message bubbles. - Remove async keyword from subscribeChatEvents; return cleanup fn directly - Update useEffect call site to store cleanup synchronously and return it to React (eliminates the mounted/then race entirely) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(local-ai): add multi-turn chat via Ollama /api/chat Expose openhuman.local_ai_chat RPC method so the UI can run full conversation-history chat directly through the bundled Ollama model without touching the cloud inference API. - ollama_api.rs: add OllamaChatMessage / OllamaChatRequest / OllamaChatResponse types for the /api/chat endpoint - service/public_infer.rs: add LocalAiService::chat_with_history() — sends multi-turn message array to Ollama, updates latency/TPS status on response - ops.rs: add LocalAiChatMessage struct and local_ai_chat async op - schemas.rs: register local_ai_chat controller (schema, handler, params) in all_controller_schemas + all_registered_controllers Zero cloud tokens are consumed on this path; the call never reaches the backend socket or the /openai/v1/chat/completions endpoint. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(conversations): local-model chat gate with multi-bubble delivery When Ollama is ready (isLocalModelActive), handleSendMessage bypasses the cloud socket entirely and routes through openhumanLocalAiChat. Response is segmented and delivered as multiple typed bubbles with natural pauses — human-like reply behaviour at zero cloud token cost. UI / delivery - deliverLocalResponse(): segments full reply via segmentMessage(), dispatches each bubble with getSegmentDelay() pause between them; typing indicator (isDelivering) shows between segments - Socket-connected guard skipped on local path so offline local use works - Cloud socket path (chatSend → chat:done) fully unchanged Frontend RPC - tauriCommands: openhumanLocalAiChat(messages, maxTokens?) wraps openhuman.local_ai_chat via core RPC; LocalAiChatMessage type exported Tests (402 passing) - messageSegmentation: 4 new edge-case tests (whitespace, 80-char boundary, paragraph split, delay scaling) - localChatGating (new file): 9 tests — segmentation correctness, delay bounds [500, 1400] ms, sender→role mapping for message history build Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(prompts): replace vague emoji guidance with explicit contextual rules The previous "Minimal — match the user's style" instruction was too loose, causing the model to stack decorative emojis on every message (e.g. "Hey! 😄 Just cooking up some AI magic! 🚀🔥✨"). SOUL.md — add Emoji Rules section: - Hard cap: one emoji maximum per message; none is always acceptable - Contextual, not decorative: emoji must reinforce the specific content (🔥 for exciting news, 🤔 for uncertainty, ✅ for confirmations) - Never open a sentence with an emoji - Skip entirely in error/warning/technical/long responses - Mirror the user's own emoji usage pattern - Concrete good/bad examples so the model can calibrate BOOTSTRAP.md — tighten the Communication Preferences entry to reference the same rules rather than the old vague one-liner. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: scope PR_DESCRIPTION.md to feat/humanlike-replies only Remove unrelated package manager distribution content (was from a different branch). Description now covers only the 4 commits on this branch: socket listener fix, Rust local_ai_chat RPC, frontend local chat gate + multi-bubble delivery, and emoji prompt rules. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(local_ai): improve code formatting and readability in chat operations - Adjusted formatting in local_ai_chat function for better readability by adding line breaks. - Simplified the mapping of messages to OllamaChatMessage in ops.rs. - Streamlined the await syntax in schemas.rs for clarity. - Enhanced formatting in public_infer.rs for consistency in API request construction. * refactor(conversations): enhance code readability and structure in Conversations component - Improved formatting and consistency in the Conversations component, including better alignment of dispatch calls and message handling logic. - Removed redundant imports and streamlined the mapping of stored messages for clarity. - Adjusted conditional rendering for improved readability in the UI logic. --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
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
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Problem
Duplicate messages —
subscribeChatEventswas declaredasyncwith no actualawaitinside. The cleanup function returned by.then()lands in a microtask; React's synchronous cleanup had already run by then, so socket listeners were never removed. Each reconnect stacked another listener set, causingchat:doneto fire N times and produce N copies of every reply.Responses felt like AI slop — replies arrived as a single wall of text. No typing feel, no natural pacing.
Ollama was wired for utilities but not chat —
local_ai_status,suggest_questions, TTS, and Whisper all used Ollama already, but the main conversation loop always hit the cloud socket regardless of whether a local model was running.Generic emoji overuse — the SOUL prompt said "Enthusiastic / Witty / Genuinely human" and the only emoji rule was "Minimal — match user's style". The model interpreted this as license to add 😄🔥✨ on every message.
Solution
1 — Socket listener race condition
subscribeChatEventsno longer usesasync— the cleanup fn is returned synchronously so React can always call it. TheuseEffectstores it withconst cleanup = subscribeChatEvents(...)and returns it directly.2 — Rust:
openhuman.local_ai_chatRPCollama_api.rs—OllamaChatMessage/OllamaChatRequest/OllamaChatResponsefor/api/chatservice/public_infer.rs—LocalAiService::chat_with_history(): sends full message history to Ollama, updates latency/TPS statusops.rs—local_ai_chatasync opschemas.rs— registered controller: schema, handler, param deserialization3 — Frontend: local chat gate + multi-bubble delivery
When
isLocalModelActiveis true,handleSendMessagetakes the local path:Socket-connected guard is skipped on the local path so offline use works.
4 — Emoji rules in system prompts
SOUL.mdadds an explicit Emoji Rules section:BOOTSTRAP.mdCommunication Preferences entry updated to reference these rules.Files Changed
app/src/services/chatService.tsasyncfromsubscribeChatEvents; synchronous cleanupapp/src/pages/Conversations.tsxdeliverLocalResponse, updated socket effectapp/src/hooks/useLocalModelStatus.tslocal_ai_statusevery 12 s; returnstruewhenstate === "ready"app/src/utils/messageSegmentation.tssegmentMessage()+getSegmentDelay()app/src/utils/tauriCommands.tsopenhumanLocalAiChat()RPC wrapperapp/src/store/threadSlice.tsaddReactionreducer;activeThreadIdtrackingsrc/openhuman/local_ai/ollama_api.rs/api/chattypessrc/openhuman/local_ai/service/public_infer.rschat_with_history()methodsrc/openhuman/local_ai/ops.rslocal_ai_chatopsrc/openhuman/local_ai/schemas.rssrc/openhuman/agent/prompts/SOUL.mdsrc/openhuman/agent/prompts/BOOTSTRAP.mdSubmission Checklist
cargo check --manifest-path Cargo.tomlcleantsc --noEmit0 errorsyarn test); new files:messageSegmentation.test.ts(14 tests),localChatGating.test.ts(9 tests)Test validation
Related Issues
#143
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation