fix(learning): decouple always-on learning subscribers from channels startup (#5003)#5025
Merged
senamakel merged 21 commits intoJul 18, 2026
Merged
Conversation
…5002) Move channels/providers/web -> web_chat/, taking web_errors.rs and web_tests.rs with it (both were reached via #[path] from web/mod.rs and never declared in providers/mod.rs). Drop the now-redundant ../ path attribute on web_errors. No back-compat re-export: channels no longer exports web.
…humansai#5002) The 5 channels/ modules that consume the conduit (bus, host::adapters, proactive, telegram::remote_control, runtime::startup) now import it from its new home. Dependency is one-way: channels -> web_chat.
) Harness-family consumers (core::{socketio,jsonrpc,all,observability, event_bus}, agent::task_dispatcher, threads::ops, agentbox::invoker, cron::scheduler, flows::ops, test_support::introspect) now import the conduit from openhuman::web_chat. src/core/all.rs keeps its DomainGroup::Channels tag: the web RPC surface must stay hidden by DomainSet::harness() at runtime exactly as today. Re-tagging it would be an unreviewed behaviour change; tinyhumansai#4801 will cfg this registration site instead. rustfmt reflows 4 call sites purely because the shorter path now fits.
channels_large_round25 imported the module with {self}, which bound the
local name 'web'; rebound as {self as web} so its 7 bare web:: uses keep
resolving rather than renaming them.
Comment/README-only. Several already named a stale 'channels/providers/web.rs' (the module has been a directory for a while); the move made them wrong twice over. Historical plan snapshots under docs/plans, docs/superpowers and docs/tinyagents-full-migration-plan are left as-is -- they record a past state.
…le (tinyhumansai#5002) The four EventHandler::name() returns still read `channels::web::*`, naming a module that no longer exists after the relocation. AGENTS.md's convention is `<domain>::<purpose>`, and sibling subscribers follow it (`cron::delivery`, `memory::conversations::persistence`). Log-output only: `name()` is consumed at src/core/event_bus/bus.rs:145,169 and both uses are tracing fields. Dispatch filters on `domains()`, not `name()`. Verified nothing keys on the old strings — no classifier, filter, or frontend reference.
…chat (tinyhumansai#5002) The PromptEnforcementContext source label still read `channels.providers.web.start_chat`, naming a module that no longer exists. Siblings use the dotted module path (`agent.bus.run_turn`, `agent.runtime.run_single`), so the canonical value is `web_chat.start_chat`. Diagnostic-only: `source` is consumed at src/openhuman/prompt_injection/detector.rs:504 as a `tracing::info!` field. Verified nothing matches on the literal. Caught by CodeRabbit on tinyhumansai#5004 — the original grep proof searched the Rust path form (`channels::providers::web`) and was blind to the dot-separated string.
…nyhumansai#5002) Comment referenced `channels/providers/web::ArtifactSurfaceSubscriber`; the subscriber now lives in `web_chat`. Comment-only, no executable change.
Contributor
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (78)
💤 Files with no reviewable changes (2)
📝 WalkthroughWalkthroughThe PR extracts web-chat functionality into ChangesWeb chat extraction
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc3e3f5922
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+70
to
+72
| static CLIENT_HANDLES: OnceLock<(Option<SubscriptionHandle>, Option<SubscriptionHandle>)> = | ||
| OnceLock::new(); | ||
| CLIENT_HANDLES.get_or_init(|| register_with_client(client_if_ready(), &workspace_dir)); |
There was a problem hiding this comment.
When the desktop boots before login or later activates a different user, credentials::store_session and app_state::finish_revalidated_user_activation rebind memory::global to the new workspace, but this process-wide OnceLock permanently retains the original MemoryClient and workspace_dir. The rebuild loop therefore continues reading/writing the pre-login or previous user's profile database, and ProfileMdRenderer writes that workspace's PROFILE.md; make these subscribers resolve current workspace state dynamically or explicitly replace them during workspace rebinding.
Useful? React with 👍 / 👎.
| // Make the memory client ready so the full Platform wiring runs — no | ||
| // channel runtime is ever constructed in this test. | ||
| let _ = crate::openhuman::memory::global::init(tmp.path().join("workspace")); | ||
| register_learning_subscribers(tmp.path().to_path_buf()); |
There was a problem hiding this comment.
This test initializes the process-global OnceLock subscriptions on its own #[tokio::test] runtime. After that runtime is dropped, the stored handles remain initialized but their tasks are dead, so register_learning_subscribers_is_idempotent can observe the locks, skip resubscription on its separate runtime, and time out with zero candidates depending on test ordering. Use a fresh non-singleton subscription helper or serialize both assertions inside one runtime.
Useful? React with 👍 / 👎.
![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: 15
🧹 Nitpick comments (2)
src/openhuman/web_chat/progress_bridge.rs (1)
304-323: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSplit the progress bridge into focused modules.
spawn_progress_bridgeextends through Line 1457, mixing event translation, ledger persistence, tracing, heartbeat scheduling, and lifecycle state. Separate these responsibilities before this grows further.As per coding guidelines, prefer files of approximately 500 lines or fewer.
🤖 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/openhuman/web_chat/progress_bridge.rs` around lines 304 - 323, Split spawn_progress_bridge and its related logic into focused modules of approximately 500 lines or fewer, separating event translation, ledger persistence, tracing, heartbeat scheduling, and lifecycle state management. Keep spawn_progress_bridge as the orchestration entry point, moving each responsibility behind appropriately named functions or module boundaries without changing the existing progress-bridge behavior.Source: Coding guidelines
src/openhuman/web_chat/web_errors.rs (1)
507-936: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSplit the oversized classifier and test modules along existing concern boundaries.
src/openhuman/web_chat/web_errors.rs#L507-L936: extract parsing/detection helpers, managed-code mapping, and fallback classification into focused modules.src/openhuman/web_chat/web_tests.rs#L1486-L1499: split error classification, schema/session, and concurrency/cancellation tests into corresponding test modules.As per coding guidelines, Rust files should preferably remain approximately 500 lines or fewer.
🤖 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/openhuman/web_chat/web_errors.rs` around lines 507 - 936, The oversized web error implementation and test module should be split along existing concern boundaries. In src/openhuman/web_chat/web_errors.rs lines 507-936, extract parsing/detection helpers, managed error-code mapping, and fallback classification into focused modules while preserving classify_inference_error behavior; in src/openhuman/web_tests.rs lines 1486-1499, move error-classification, schema/session, and concurrency/cancellation tests into corresponding test modules. Keep each Rust file approximately 500 lines or fewer.Source: Coding guidelines
🤖 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 @.github/workflows/ci-lite.yml:
- Around line 435-465: Update the ACTUAL discovery command in the “Guard — new
feature-gated test modules must be acknowledged” workflow step to match any
feature name in cfg expressions instead of the hard-coded
voice/media/web3/meet/mcp/skills/flows list. Preserve the subsequent filtering
for Rust files containing tests and the existing path normalization and sorting,
so newly introduced feature-gated test modules are detected and compared with
EXPECTED.
In `@src/openhuman/approval/README.md`:
- Around line 59-62: Update the architecture note in the approval README to
consistently identify the subscriber as living under src/openhuman/web_chat/,
removing the stale reference to the channels web provider while preserving the
statement that this module only publishes.
In `@src/openhuman/learning/startup.rs`:
- Around line 45-72: The OnceLock-based registration in
register_learning_subscribers currently caches None and prevents retries after
the event bus or memory client becomes available. Store initialization state
only after successful handle acquisition, allowing absent registrations to retry
on later calls while preserving existing handles; update register_with_client
and the email-signature flow accordingly. Add an absent-then-ready regression
test, and include grep-friendly debug logs for entry/exit, already-registered
and retry branches, external registration calls, state transitions, and
failures.
In `@src/openhuman/web_chat/presentation_tests.rs`:
- Around line 1-2: Add Rust unit tests in presentation_tests.rs that exercise
deliver_response via the event subscriber, covering a single response, ordered
segments with exactly one terminal event, citation and reaction placement, usage
only on chat_done, and terminal-event emission when reactions fail or time out.
In `@src/openhuman/web_chat/presentation.rs`:
- Around line 223-225: Replace byte-based str.len() threshold checks with
chars().count() in the short-message handling and the corresponding logic at the
referenced locations, including the short-segment merging and maximum-delay
paths. Apply this consistently so all character thresholds count Unicode
characters rather than UTF-8 bytes.
- Around line 75-205: The terminal event delivery flow should add privacy-safe,
grep-friendly debug diagnostics. In the surrounding function, log entry with
request/thread identifiers, segment count, and citation/usage presence; log
whether the single-bubble or segmented branch is selected, including emitted
segment counts; and log successful terminal completion for both paths. Do not
log response or segment contents, and preserve the existing event behavior.
- Around line 64-73: Update the reaction flow around try_reaction and
reaction_handle.await so response delivery is never blocked: apply a short
presentation-specific timeout, abort the spawned task when it expires, and
continue with no reaction. Add verbose grep-friendly diagnostics for entry/exit,
disabled runtime, configuration loading outcomes, timeout/abort, join failures,
and other reaction branches, including the related flow around lines 443-471.
- Around line 21-45: Update usage_payload to return None when the supplied
LastTurnUsage represents an all-zero, no-spend turn, including the synthetic
budget-exhausted placeholder. Preserve the existing payload mapping for usage
records with any recorded spend or token activity.
In `@src/openhuman/web_chat/progress_bridge.rs`:
- Around line 371-401: Update the identity setup before TraceContext::new so
anonymous requests keep user_id as None; remove the client_id fallback from the
user_id chain and retain transport identifiers only through the existing
client_id field/with_client_id flow. Derive user_attributed from the resolved
persistent user identity or session attribution, not from transport identifiers,
while preserving authenticated identity and session-profile attribution.
In `@src/openhuman/web_chat/run_task.rs`:
- Around line 23-39: Add structured, grep-friendly entry and terminal outcome
diagnostics in run_chat_task, including request_id, fork, cache action, and a
result class while excluding message text and full PII. Ensure the cache action
reflects the relevant shared-session behavior and emit the outcome diagnostic on
every return path, including errors, without altering task execution.
- Around line 115-164: After the cached-or-rebuilt agent is selected in the
session-agent flow, call the agent’s set_event_context method with the current
client_id before processing the request. Apply this on cache hits and rebuilt
agents, preserving the existing thread-scoped cache behavior so emitted events
and traces use the current client context.
In `@src/openhuman/web_chat/session.rs`:
- Around line 36-43: Update provider_role_for_model_override to recognize
"reasoning-v1" alongside "hint:reasoning" and return "reasoning", while
preserving the existing fallback and other model mappings.
- Around line 129-134: Update the short_thread construction before
set_agent_definition_name to truncate thread_id by Unicode characters rather
than byte indices, preserving IDs of 12 or fewer characters and safely limiting
longer IDs to the first 12 characters without panicking on multibyte UTF-8.
In `@src/openhuman/web_chat/types.rs`:
- Around line 15-35: Locale is absent from the cached-agent fingerprint,
allowing stale locale-dependent system prompts to be reused. In
src/openhuman/web_chat/types.rs:15-35, add a normalized locale/signature field
to SessionCacheFingerprint; in src/openhuman/web_chat/session.rs:182-203, accept
and populate it during fingerprint construction; in
src/openhuman/web_chat/run_task.rs:97-104, pass the normalized locale through;
and in src/openhuman/web_chat/run_task.rs:138-147, retain it as an
agent-construction input and add coverage proving a locale change rebuilds the
cached agent.
In `@src/openhuman/web_chat/web_errors.rs`:
- Around line 668-696: The rate-limit branch in the error classification flow
currently matches any occurrence of “429”. Update the condition around
is_non_retryable_rate_limit_text and the ClassifiedError construction to require
a provider status envelope or the explicit “Too Many Requests” phrase, while
preserving the existing retry-after and business-rate-limit handling.
---
Nitpick comments:
In `@src/openhuman/web_chat/progress_bridge.rs`:
- Around line 304-323: Split spawn_progress_bridge and its related logic into
focused modules of approximately 500 lines or fewer, separating event
translation, ledger persistence, tracing, heartbeat scheduling, and lifecycle
state management. Keep spawn_progress_bridge as the orchestration entry point,
moving each responsibility behind appropriately named functions or module
boundaries without changing the existing progress-bridge behavior.
In `@src/openhuman/web_chat/web_errors.rs`:
- Around line 507-936: The oversized web error implementation and test module
should be split along existing concern boundaries. In
src/openhuman/web_chat/web_errors.rs lines 507-936, extract parsing/detection
helpers, managed error-code mapping, and fallback classification into focused
modules while preserving classify_inference_error behavior; in
src/openhuman/web_tests.rs lines 1486-1499, move error-classification,
schema/session, and concurrency/cancellation tests into corresponding test
modules. Keep each Rust file approximately 500 lines or fewer.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d352ea98-2dc4-497f-8f82-83d1b97e9c8a
📒 Files selected for processing (78)
.github/workflows/ci-lite.ymlapp/src-tauri/src/claude_code.rsapp/src-tauri/src/imessage_scanner/mod.rsapp/src-tauri/src/lib.rsapp/src-tauri/src/mascot_native_window.rsapp/src-tauri/src/native_notifications/mod.rsapp/src-tauri/src/notch_window.rsapp/src/services/chatService.tsdocs/TEST-COVERAGE-MATRIX.mdsrc/core/all.rssrc/core/all_tests.rssrc/core/event_bus/events.rssrc/core/jsonrpc.rssrc/core/observability.rssrc/core/socketio.rssrc/main.rssrc/openhuman/agent/README.mdsrc/openhuman/agent/progress_tracing/journal_projection.rssrc/openhuman/agent/task_dispatcher/executor.rssrc/openhuman/agent/task_dispatcher/registry.rssrc/openhuman/agent/triage/evaluator.rssrc/openhuman/agent/turn_origin.rssrc/openhuman/agent_orchestration/command_center/mod.rssrc/openhuman/agent_orchestration/run_ledger_finalize.rssrc/openhuman/agentbox/invoker.rssrc/openhuman/approval/README.mdsrc/openhuman/approval/gate.rssrc/openhuman/artifacts/store.rssrc/openhuman/channels/bus.rssrc/openhuman/channels/host/adapters.rssrc/openhuman/channels/mod.rssrc/openhuman/channels/proactive.rssrc/openhuman/channels/providers/mod.rssrc/openhuman/channels/providers/telegram/remote_control.rssrc/openhuman/channels/runtime/dispatch/processor.rssrc/openhuman/channels/runtime/startup.rssrc/openhuman/cron/scheduler.rssrc/openhuman/cron/scheduler_tests.rssrc/openhuman/flows/ops.rssrc/openhuman/inference/provider/error_code.rssrc/openhuman/learning/mod.rssrc/openhuman/learning/startup.rssrc/openhuman/meet_agent/brain/llm.rssrc/openhuman/mod.rssrc/openhuman/prompt_injection/README.mdsrc/openhuman/security/egress/mod.rssrc/openhuman/test_support/README.mdsrc/openhuman/test_support/introspect.rssrc/openhuman/threads/README.mdsrc/openhuman/threads/ops.rssrc/openhuman/wallet/README.mdsrc/openhuman/wallet/execution.rssrc/openhuman/web_chat/event_bus.rssrc/openhuman/web_chat/mod.rssrc/openhuman/web_chat/ops.rssrc/openhuman/web_chat/presentation.rssrc/openhuman/web_chat/presentation_tests.rssrc/openhuman/web_chat/progress_bridge.rssrc/openhuman/web_chat/run_task.rssrc/openhuman/web_chat/schemas.rssrc/openhuman/web_chat/session.rssrc/openhuman/web_chat/types.rssrc/openhuman/web_chat/web_errors.rssrc/openhuman/web_chat/web_tests.rstests/agent_harness_e2e.rstests/agentbox_e2e.rstests/composio_list_tools_stack_overflow_regression.rstests/raw_coverage/channels_bus_presentation_raw_coverage_e2e.rstests/raw_coverage/channels_large_round25_raw_coverage_e2e.rstests/raw_coverage/channels_provider_deep_raw_coverage_e2e.rstests/raw_coverage/channels_provider_leftovers_raw_coverage_e2e.rstests/raw_coverage/channels_runtime_raw_coverage_e2e.rstests/raw_coverage/channels_web_startup_raw_coverage_e2e.rstests/raw_coverage/channels_web_telegram_raw_coverage_e2e.rstests/raw_coverage/channels_web_yuanbao_round22_raw_coverage_e2e.rstests/raw_coverage/tools_approval_channels_raw_coverage_e2e.rstests/raw_coverage/tools_network_channels_raw_coverage_e2e.rstests/web_cancel_request_scoping.rs
💤 Files with no reviewable changes (2)
- src/openhuman/channels/mod.rs
- src/openhuman/channels/providers/mod.rs
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 15
🧹 Nitpick comments (2)
src/openhuman/web_chat/progress_bridge.rs (1)
304-323: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSplit the progress bridge into focused modules.
spawn_progress_bridgeextends through Line 1457, mixing event translation, ledger persistence, tracing, heartbeat scheduling, and lifecycle state. Separate these responsibilities before this grows further.As per coding guidelines, prefer files of approximately 500 lines or fewer.
🤖 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/openhuman/web_chat/progress_bridge.rs` around lines 304 - 323, Split spawn_progress_bridge and its related logic into focused modules of approximately 500 lines or fewer, separating event translation, ledger persistence, tracing, heartbeat scheduling, and lifecycle state management. Keep spawn_progress_bridge as the orchestration entry point, moving each responsibility behind appropriately named functions or module boundaries without changing the existing progress-bridge behavior.Source: Coding guidelines
src/openhuman/web_chat/web_errors.rs (1)
507-936: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSplit the oversized classifier and test modules along existing concern boundaries.
src/openhuman/web_chat/web_errors.rs#L507-L936: extract parsing/detection helpers, managed-code mapping, and fallback classification into focused modules.src/openhuman/web_chat/web_tests.rs#L1486-L1499: split error classification, schema/session, and concurrency/cancellation tests into corresponding test modules.As per coding guidelines, Rust files should preferably remain approximately 500 lines or fewer.
🤖 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/openhuman/web_chat/web_errors.rs` around lines 507 - 936, The oversized web error implementation and test module should be split along existing concern boundaries. In src/openhuman/web_chat/web_errors.rs lines 507-936, extract parsing/detection helpers, managed error-code mapping, and fallback classification into focused modules while preserving classify_inference_error behavior; in src/openhuman/web_tests.rs lines 1486-1499, move error-classification, schema/session, and concurrency/cancellation tests into corresponding test modules. Keep each Rust file approximately 500 lines or fewer.Source: Coding guidelines
🤖 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 @.github/workflows/ci-lite.yml:
- Around line 435-465: Update the ACTUAL discovery command in the “Guard — new
feature-gated test modules must be acknowledged” workflow step to match any
feature name in cfg expressions instead of the hard-coded
voice/media/web3/meet/mcp/skills/flows list. Preserve the subsequent filtering
for Rust files containing tests and the existing path normalization and sorting,
so newly introduced feature-gated test modules are detected and compared with
EXPECTED.
In `@src/openhuman/approval/README.md`:
- Around line 59-62: Update the architecture note in the approval README to
consistently identify the subscriber as living under src/openhuman/web_chat/,
removing the stale reference to the channels web provider while preserving the
statement that this module only publishes.
In `@src/openhuman/learning/startup.rs`:
- Around line 45-72: The OnceLock-based registration in
register_learning_subscribers currently caches None and prevents retries after
the event bus or memory client becomes available. Store initialization state
only after successful handle acquisition, allowing absent registrations to retry
on later calls while preserving existing handles; update register_with_client
and the email-signature flow accordingly. Add an absent-then-ready regression
test, and include grep-friendly debug logs for entry/exit, already-registered
and retry branches, external registration calls, state transitions, and
failures.
In `@src/openhuman/web_chat/presentation_tests.rs`:
- Around line 1-2: Add Rust unit tests in presentation_tests.rs that exercise
deliver_response via the event subscriber, covering a single response, ordered
segments with exactly one terminal event, citation and reaction placement, usage
only on chat_done, and terminal-event emission when reactions fail or time out.
In `@src/openhuman/web_chat/presentation.rs`:
- Around line 223-225: Replace byte-based str.len() threshold checks with
chars().count() in the short-message handling and the corresponding logic at the
referenced locations, including the short-segment merging and maximum-delay
paths. Apply this consistently so all character thresholds count Unicode
characters rather than UTF-8 bytes.
- Around line 75-205: The terminal event delivery flow should add privacy-safe,
grep-friendly debug diagnostics. In the surrounding function, log entry with
request/thread identifiers, segment count, and citation/usage presence; log
whether the single-bubble or segmented branch is selected, including emitted
segment counts; and log successful terminal completion for both paths. Do not
log response or segment contents, and preserve the existing event behavior.
- Around line 64-73: Update the reaction flow around try_reaction and
reaction_handle.await so response delivery is never blocked: apply a short
presentation-specific timeout, abort the spawned task when it expires, and
continue with no reaction. Add verbose grep-friendly diagnostics for entry/exit,
disabled runtime, configuration loading outcomes, timeout/abort, join failures,
and other reaction branches, including the related flow around lines 443-471.
- Around line 21-45: Update usage_payload to return None when the supplied
LastTurnUsage represents an all-zero, no-spend turn, including the synthetic
budget-exhausted placeholder. Preserve the existing payload mapping for usage
records with any recorded spend or token activity.
In `@src/openhuman/web_chat/progress_bridge.rs`:
- Around line 371-401: Update the identity setup before TraceContext::new so
anonymous requests keep user_id as None; remove the client_id fallback from the
user_id chain and retain transport identifiers only through the existing
client_id field/with_client_id flow. Derive user_attributed from the resolved
persistent user identity or session attribution, not from transport identifiers,
while preserving authenticated identity and session-profile attribution.
In `@src/openhuman/web_chat/run_task.rs`:
- Around line 23-39: Add structured, grep-friendly entry and terminal outcome
diagnostics in run_chat_task, including request_id, fork, cache action, and a
result class while excluding message text and full PII. Ensure the cache action
reflects the relevant shared-session behavior and emit the outcome diagnostic on
every return path, including errors, without altering task execution.
- Around line 115-164: After the cached-or-rebuilt agent is selected in the
session-agent flow, call the agent’s set_event_context method with the current
client_id before processing the request. Apply this on cache hits and rebuilt
agents, preserving the existing thread-scoped cache behavior so emitted events
and traces use the current client context.
In `@src/openhuman/web_chat/session.rs`:
- Around line 36-43: Update provider_role_for_model_override to recognize
"reasoning-v1" alongside "hint:reasoning" and return "reasoning", while
preserving the existing fallback and other model mappings.
- Around line 129-134: Update the short_thread construction before
set_agent_definition_name to truncate thread_id by Unicode characters rather
than byte indices, preserving IDs of 12 or fewer characters and safely limiting
longer IDs to the first 12 characters without panicking on multibyte UTF-8.
In `@src/openhuman/web_chat/types.rs`:
- Around line 15-35: Locale is absent from the cached-agent fingerprint,
allowing stale locale-dependent system prompts to be reused. In
src/openhuman/web_chat/types.rs:15-35, add a normalized locale/signature field
to SessionCacheFingerprint; in src/openhuman/web_chat/session.rs:182-203, accept
and populate it during fingerprint construction; in
src/openhuman/web_chat/run_task.rs:97-104, pass the normalized locale through;
and in src/openhuman/web_chat/run_task.rs:138-147, retain it as an
agent-construction input and add coverage proving a locale change rebuilds the
cached agent.
In `@src/openhuman/web_chat/web_errors.rs`:
- Around line 668-696: The rate-limit branch in the error classification flow
currently matches any occurrence of “429”. Update the condition around
is_non_retryable_rate_limit_text and the ClassifiedError construction to require
a provider status envelope or the explicit “Too Many Requests” phrase, while
preserving the existing retry-after and business-rate-limit handling.
---
Nitpick comments:
In `@src/openhuman/web_chat/progress_bridge.rs`:
- Around line 304-323: Split spawn_progress_bridge and its related logic into
focused modules of approximately 500 lines or fewer, separating event
translation, ledger persistence, tracing, heartbeat scheduling, and lifecycle
state management. Keep spawn_progress_bridge as the orchestration entry point,
moving each responsibility behind appropriately named functions or module
boundaries without changing the existing progress-bridge behavior.
In `@src/openhuman/web_chat/web_errors.rs`:
- Around line 507-936: The oversized web error implementation and test module
should be split along existing concern boundaries. In
src/openhuman/web_chat/web_errors.rs lines 507-936, extract parsing/detection
helpers, managed error-code mapping, and fallback classification into focused
modules while preserving classify_inference_error behavior; in
src/openhuman/web_tests.rs lines 1486-1499, move error-classification,
schema/session, and concurrency/cancellation tests into corresponding test
modules. Keep each Rust file approximately 500 lines or fewer.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d352ea98-2dc4-497f-8f82-83d1b97e9c8a
📒 Files selected for processing (78)
.github/workflows/ci-lite.ymlapp/src-tauri/src/claude_code.rsapp/src-tauri/src/imessage_scanner/mod.rsapp/src-tauri/src/lib.rsapp/src-tauri/src/mascot_native_window.rsapp/src-tauri/src/native_notifications/mod.rsapp/src-tauri/src/notch_window.rsapp/src/services/chatService.tsdocs/TEST-COVERAGE-MATRIX.mdsrc/core/all.rssrc/core/all_tests.rssrc/core/event_bus/events.rssrc/core/jsonrpc.rssrc/core/observability.rssrc/core/socketio.rssrc/main.rssrc/openhuman/agent/README.mdsrc/openhuman/agent/progress_tracing/journal_projection.rssrc/openhuman/agent/task_dispatcher/executor.rssrc/openhuman/agent/task_dispatcher/registry.rssrc/openhuman/agent/triage/evaluator.rssrc/openhuman/agent/turn_origin.rssrc/openhuman/agent_orchestration/command_center/mod.rssrc/openhuman/agent_orchestration/run_ledger_finalize.rssrc/openhuman/agentbox/invoker.rssrc/openhuman/approval/README.mdsrc/openhuman/approval/gate.rssrc/openhuman/artifacts/store.rssrc/openhuman/channels/bus.rssrc/openhuman/channels/host/adapters.rssrc/openhuman/channels/mod.rssrc/openhuman/channels/proactive.rssrc/openhuman/channels/providers/mod.rssrc/openhuman/channels/providers/telegram/remote_control.rssrc/openhuman/channels/runtime/dispatch/processor.rssrc/openhuman/channels/runtime/startup.rssrc/openhuman/cron/scheduler.rssrc/openhuman/cron/scheduler_tests.rssrc/openhuman/flows/ops.rssrc/openhuman/inference/provider/error_code.rssrc/openhuman/learning/mod.rssrc/openhuman/learning/startup.rssrc/openhuman/meet_agent/brain/llm.rssrc/openhuman/mod.rssrc/openhuman/prompt_injection/README.mdsrc/openhuman/security/egress/mod.rssrc/openhuman/test_support/README.mdsrc/openhuman/test_support/introspect.rssrc/openhuman/threads/README.mdsrc/openhuman/threads/ops.rssrc/openhuman/wallet/README.mdsrc/openhuman/wallet/execution.rssrc/openhuman/web_chat/event_bus.rssrc/openhuman/web_chat/mod.rssrc/openhuman/web_chat/ops.rssrc/openhuman/web_chat/presentation.rssrc/openhuman/web_chat/presentation_tests.rssrc/openhuman/web_chat/progress_bridge.rssrc/openhuman/web_chat/run_task.rssrc/openhuman/web_chat/schemas.rssrc/openhuman/web_chat/session.rssrc/openhuman/web_chat/types.rssrc/openhuman/web_chat/web_errors.rssrc/openhuman/web_chat/web_tests.rstests/agent_harness_e2e.rstests/agentbox_e2e.rstests/composio_list_tools_stack_overflow_regression.rstests/raw_coverage/channels_bus_presentation_raw_coverage_e2e.rstests/raw_coverage/channels_large_round25_raw_coverage_e2e.rstests/raw_coverage/channels_provider_deep_raw_coverage_e2e.rstests/raw_coverage/channels_provider_leftovers_raw_coverage_e2e.rstests/raw_coverage/channels_runtime_raw_coverage_e2e.rstests/raw_coverage/channels_web_startup_raw_coverage_e2e.rstests/raw_coverage/channels_web_telegram_raw_coverage_e2e.rstests/raw_coverage/channels_web_yuanbao_round22_raw_coverage_e2e.rstests/raw_coverage/tools_approval_channels_raw_coverage_e2e.rstests/raw_coverage/tools_network_channels_raw_coverage_e2e.rstests/web_cancel_request_scoping.rs
💤 Files with no reviewable changes (2)
- src/openhuman/channels/mod.rs
- src/openhuman/channels/providers/mod.rs
🛑 Comments failed to post (15)
.github/workflows/ci-lite.yml (1)
435-465: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Make the gated-test discovery independent of known feature names.
ACTUALonly recognizes the seven literal features in Line 464. A test gated by a newly introduced domain feature remains invisible, so this “self-maintaining” guard passes instead of requesting an allowlist update. Scan arbitraryfeature = "…"cfg expressions—or derive the disabled feature set fromCargo.toml—before filtering files containing tests.🤖 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 @.github/workflows/ci-lite.yml around lines 435 - 465, Update the ACTUAL discovery command in the “Guard — new feature-gated test modules must be acknowledged” workflow step to match any feature name in cfg expressions instead of the hard-coded voice/media/web3/meet/mcp/skills/flows list. Preserve the subsequent filtering for Rust files containing tests and the existing path normalization and sorting, so newly introduced feature-gated test modules are detected and compared with EXPECTED.src/openhuman/approval/README.md (1)
59-62: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the stale subscriber location.
Line 59 now documents
ApprovalSurfaceSubscriberundersrc/openhuman/web_chat/, but the following note still says it lives in the channels web provider. Update that sentence to avoid documenting the pre-extraction architecture.Proposed documentation fix
- No `bus.rs` in this module — it only publishes; the subscriber lives in the - `channels` web provider. + No `bus.rs` in this module — it only publishes; the subscriber lives in + `src/openhuman/web_chat/`.📝 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.- `DomainEvent::ApprovalRequested { request_id, tool_name, action_summary, args_redacted, session_id, thread_id, client_id }` — emitted when a call is parked. Bridged to the `approval_request` web-channel socket event by `ApprovalSurfaceSubscriber` (defined in `src/openhuman/web_chat/`). - `DomainEvent::ApprovalDecided { request_id, tool_name, decision }` — emitted when a decision is applied. No `bus.rs` in this module — it only publishes; the subscriber lives in `src/openhuman/web_chat/`.🤖 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/openhuman/approval/README.md` around lines 59 - 62, Update the architecture note in the approval README to consistently identify the subscriber as living under src/openhuman/web_chat/, removing the stale reference to the channels web provider while preserving the statement that this module only publishes.src/openhuman/learning/startup.rs (1)
45-72: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not cache failed registration attempts.
Both
OnceLocks permanently store failure states. If the event bus or memory client is unavailable on the first call, later calls cannot retry, despite the warning saying learning resumes when the client initializes. Only mark a subscriber initialized after obtaining its handle, and add an absent-then-ready regression test.Include debug entry/exit and already-registered/retrying diagnostics in the corrected flow. As per coding guidelines, “New or changed flows must include verbose, grep-friendly diagnostics covering entry/exit, branches, external calls, retries/timeouts, state transitions, and errors.”
🤖 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/openhuman/learning/startup.rs` around lines 45 - 72, The OnceLock-based registration in register_learning_subscribers currently caches None and prevents retries after the event bus or memory client becomes available. Store initialization state only after successful handle acquisition, allowing absent registrations to retry on later calls while preserving existing handles; update register_with_client and the email-signature flow accordingly. Add an absent-then-ready regression test, and include grep-friendly debug logs for entry/exit, already-registered and retry branches, external registration calls, state transitions, and failures.Source: Coding guidelines
src/openhuman/web_chat/presentation_tests.rs (1)
1-2: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Add unit tests for the actual delivery contract.
Exercise
deliver_responsethrough the event subscriber: single response, ordered segments followed by exactly one terminal event, citation/reaction placement, usage only onchat_done, and reaction failure/timeout still producing a terminal event.As per coding guidelines, “Follow the feature workflow: specify behavior, implement and unit-test it in Rust.”
🤖 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/openhuman/web_chat/presentation_tests.rs` around lines 1 - 2, Add Rust unit tests in presentation_tests.rs that exercise deliver_response via the event subscriber, covering a single response, ordered segments with exactly one terminal event, citation and reaction placement, usage only on chat_done, and terminal-event emission when reactions fail or time out.Source: Coding guidelines
src/openhuman/web_chat/presentation.rs (4)
21-45: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Honor the documented zero-spend omission.
usage_payloadreturnsSomefor every suppliedLastTurnUsage, including the synthetic all-zero case that the documentation says should be omitted.Proposed fix
fn usage_payload(usage: Option<&LastTurnUsage>) -> Option<TurnUsagePayload> { let usage = usage?; + let has_spend = usage.input_tokens != 0 + || usage.output_tokens != 0 + || usage.cached_input_tokens != 0 + || usage.cost_usd != 0.0 + || usage.subagents.iter().any(|s| { + s.usage.input_tokens != 0 + || s.usage.output_tokens != 0 + || s.usage.cached_input_tokens != 0 + || s.usage.charged_amount_usd != 0.0 + }); + if !has_spend { + return None; + }📝 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./// Convert a turn's [`LastTurnUsage`] into the wire payload carried on /// `chat_done`. Returns `None` for a turn that recorded no spend at all (e.g. a /// synthetic budget-exhausted placeholder) so the event stays compact. fn usage_payload(usage: Option<&LastTurnUsage>) -> Option<TurnUsagePayload> { let usage = usage?; let has_spend = usage.input_tokens != 0 || usage.output_tokens != 0 || usage.cached_input_tokens != 0 || usage.cost_usd != 0.0 || usage.subagents.iter().any(|s| { s.usage.input_tokens != 0 || s.usage.output_tokens != 0 || s.usage.cached_input_tokens != 0 || s.usage.charged_amount_usd != 0.0 }); if !has_spend { return None; } let subagents = usage .subagents .iter() .map(|s| SubagentUsagePayload { task_id: s.task_id.clone(), agent_id: s.agent_id.clone(), input_tokens: s.usage.input_tokens, output_tokens: s.usage.output_tokens, cost_usd: s.usage.charged_amount_usd, }) .collect(); Some(TurnUsagePayload { input_tokens: usage.input_tokens, output_tokens: usage.output_tokens, cached_input_tokens: usage.cached_input_tokens, cost_usd: usage.cost_usd, context_window: usage.context_window, subagents, }) }🤖 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/openhuman/web_chat/presentation.rs` around lines 21 - 45, Update usage_payload to return None when the supplied LastTurnUsage represents an all-zero, no-spend turn, including the synthetic budget-exhausted placeholder. Preserve the existing payload mapping for usage records with any recorded spend or token activity.
64-73: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not let the optional reaction block response delivery.
The task is spawned in parallel but immediately awaited before the first event, so local-model latency or a stall delays every
chat_segment/chat_done. Apply a short presentation-specific timeout, abort the task on expiry, and continue with no reaction. Log timeout, join failure, disabled-runtime, and config-load outcomes.As per coding guidelines, “New or changed flows must include verbose, grep-friendly diagnostics covering entry/exit, branches, external calls, retries/timeouts, state transitions, and errors.”
Also applies to: 443-471
🤖 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/openhuman/web_chat/presentation.rs` around lines 64 - 73, Update the reaction flow around try_reaction and reaction_handle.await so response delivery is never blocked: apply a short presentation-specific timeout, abort the spawned task when it expires, and continue with no reaction. Add verbose grep-friendly diagnostics for entry/exit, disabled runtime, configuration loading outcomes, timeout/abort, join failures, and other reaction branches, including the related flow around lines 443-471.Source: Coding guidelines
75-205: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add diagnostics around terminal event delivery.
Neither the single-bubble nor segmented branch logs entry, selected branch, event counts, or successful terminal completion. Add privacy-safe debug logs containing request/thread identifiers, segment count, and citation/usage presence—not message contents.
As per coding guidelines, “New or changed flows must include verbose, grep-friendly diagnostics covering entry/exit, branches, external calls, retries/timeouts, state transitions, and errors.”
🤖 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/openhuman/web_chat/presentation.rs` around lines 75 - 205, The terminal event delivery flow should add privacy-safe, grep-friendly debug diagnostics. In the surrounding function, log entry with request/thread identifiers, segment count, and citation/usage presence; log whether the single-bubble or segmented branch is selected, including emitted segment counts; and log successful terminal completion for both paths. Do not log response or segment contents, and preserve the existing event behavior.Source: Coding guidelines
223-225: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Count characters rather than UTF-8 bytes.
These “character” thresholds use
str::len(), so CJK and emoji responses are treated as much longer than equivalent ASCII: they split prematurely, bypass short-segment merging, and reach the maximum delay sooner. Use.chars().count()consistently.Also applies to: 348-352, 412-420, 431-436
🤖 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/openhuman/web_chat/presentation.rs` around lines 223 - 225, Replace byte-based str.len() threshold checks with chars().count() in the short-message handling and the corresponding logic at the referenced locations, including the short-segment merging and maximum-delay paths. Apply this consistently so all character thresholds count Unicode characters rather than UTF-8 bytes.src/openhuman/web_chat/progress_bridge.rs (1)
371-401: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Keep transport identifiers out of trace user attribution.
Anonymous turns currently pass
Some(client_id)asTraceContext.user_id, despite already recording it throughwith_client_id. This exports socket IDs or"system"as user identities and also makesuser_attributedinaccurate for identities loaded from disk.Proposed fix
let identity = crate::openhuman::app_state::peek_cached_current_user_identity(); - let user_attributed = identity.is_some(); let user_id = identity .and_then(|i| i.id.or(i.email)) - .or_else(|| session_profile_user_attribution(&config)) - .unwrap_or_else(|| client_id.clone()); + .or_else(|| session_profile_user_attribution(&config)); + let user_attributed = user_id.is_some(); ... - let mut trace_ctx = TraceContext::new(trace_id, Some(user_id)) + let mut trace_ctx = TraceContext::new(trace_id, user_id)The
TraceContextcontract requires anonymoususer_idto remainNoneand transport identifiers to stay inclient_id.📝 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.let identity = crate::openhuman::app_state::peek_cached_current_user_identity(); let user_id = identity .and_then(|i| i.id.or(i.email)) .or_else(|| session_profile_user_attribution(&config)); let user_attributed = user_id.is_some(); // Run origin for trace metadata: the request's source tag // ("ptt"/"dictation"/"type"/"agentbox"/"autonomous"/…), else a // plain interactive chat turn. let run_type = RunType::from_source(metadata.source.as_deref()); let channel_source = metadata .source .clone() .unwrap_or_else(|| "chat".to_string()); // Storage-level privacy gate (`#4454`): capture_content (off by // default) rides on the TraceContext so the collector only attaches // prompt/reply content to spans when the operator opted in — no // exporter can serialize prompt/reply text otherwise. let capture_content = config.observability.agent_tracing.capture_content; log::debug!( "[web_channel][bridge] trace context trace_id={} user_attributed={} \ agent_id={:?} channel_source={} run_type={} capture_content={} request_id={}", trace_id, user_attributed, metadata.agent_id, channel_source, run_type.as_str(), capture_content, request_id, ); let mut trace_ctx = TraceContext::new(trace_id, user_id)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/web_chat/progress_bridge.rs` around lines 371 - 401, Update the identity setup before TraceContext::new so anonymous requests keep user_id as None; remove the client_id fallback from the user_id chain and retain transport identifiers only through the existing client_id field/with_client_id flow. Derive user_attributed from the resolved persistent user identity or session attribution, not from transport identifiers, while preserving authenticated identity and session-profile attribution.src/openhuman/web_chat/run_task.rs (2)
23-39: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add grep-friendly entry and exit diagnostics for
run_chat_task.Log entry and terminal outcome, including
request_id,fork, cache action, and result class without message text or full PII.As per coding guidelines, new Rust flows must include grep-friendly entry/exit, branch, external-call, state-transition, and error diagnostics without secrets or full PII.
Also applies to: 392-392
🤖 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/openhuman/web_chat/run_task.rs` around lines 23 - 39, Add structured, grep-friendly entry and terminal outcome diagnostics in run_chat_task, including request_id, fork, cache action, and a result class while excluding message text and full PII. Ensure the cache action reflects the relevant shared-session behavior and emit the outcome diagnostic on every return path, including errors, without altering task execution.Source: Coding guidelines
115-164: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Refresh per-request event context on cache hits.
The cache is intentionally thread-scoped across socket reconnects, but a reused agent retains the original
client_idset during construction. Refreshset_event_contextafter selecting the agent so Agent-originated events and traces are attributed to the current client.Proposed fix
let (mut agent, was_built_fresh) = match prior { // ... }; + + agent.set_event_context( + serde_json::json!({ + "client_id": client_id, + "thread_id": thread_id, + }) + .to_string(), + "web_channel", + );📝 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.let (mut agent, was_built_fresh) = match prior { Some(entry) if entry.fingerprint == current_fp => { log::info!( "[web-channel] reusing cached session agent id={} for client={} thread={}", target_agent_id, client_id, thread_id ); (entry.agent, false) } Some(prior_entry) => { log::info!( "[web-channel] cache miss — rebuilding session agent \ (was id={}, now id={}; prior_provider_binding={}, now={}) \ for client={} thread={}", prior_entry.fingerprint.target_agent_id, target_agent_id, prior_entry.fingerprint.provider_binding, current_fp.provider_binding, client_id, thread_id ); ( build_session_agent( &config, client_id, thread_id, &target_agent_id, &profile, model_override.clone(), temperature, locale.as_deref(), )?, true, ) } None => ( build_session_agent( &config, client_id, thread_id, &target_agent_id, &profile, model_override.clone(), temperature, locale.as_deref(), )?, true, ), }; agent.set_event_context( serde_json::json!({ "client_id": client_id, "thread_id": thread_id, }) .to_string(), "web_channel", );🤖 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/openhuman/web_chat/run_task.rs` around lines 115 - 164, After the cached-or-rebuilt agent is selected in the session-agent flow, call the agent’s set_event_context method with the current client_id before processing the request. Apply this on cache hits and rebuilt agents, preserving the existing thread-scoped cache behavior so emitted events and traces use the current client context.src/openhuman/web_chat/session.rs (2)
36-43: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Route
reasoning-v1through the reasoning provider.Unlike the other tier aliases,
reasoning-v1falls through to"chat", selecting the wrong provider and cache binding.Proposed fix
- Some("hint:reasoning") => "reasoning", + Some("hint:reasoning") | Some("reasoning-v1") => "reasoning",📝 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.pub(crate) fn provider_role_for_model_override(model_override: Option<&str>) -> &'static str { match model_override.map(str::trim) { Some("hint:agentic") | Some("agentic-v1") => "agentic", Some("hint:coding") | Some("coding-v1") => "coding", Some("hint:summarization") | Some("summarization-v1") => "summarization", Some("hint:reasoning") | Some("reasoning-v1") => "reasoning", _ => "chat", }🤖 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/openhuman/web_chat/session.rs` around lines 36 - 43, Update provider_role_for_model_override to recognize "reasoning-v1" alongside "hint:reasoning" and return "reasoning", while preserving the existing fallback and other model mappings.
129-134: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Avoid byte-indexing a UTF-8 thread ID.
&thread_id[..12]panics when byte 12 lies inside a multibyte character. Truncate by characters instead.Proposed fix
- let short_thread = if thread_id.len() > 12 { - &thread_id[..12] - } else { - thread_id - }; + let short_thread: String = thread_id.chars().take(12).collect(); agent.set_agent_definition_name(format!("{target_agent_id}_{short_thread}"));📝 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.let short_thread: String = thread_id.chars().take(12).collect(); agent.set_agent_definition_name(format!("{target_agent_id}_{short_thread}"));🤖 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/openhuman/web_chat/session.rs` around lines 129 - 134, Update the short_thread construction before set_agent_definition_name to truncate thread_id by Unicode characters rather than byte indices, preserving IDs of 12 or fewer characters and safely limiting longer IDs to the first 12 characters without panicking on multibyte UTF-8.src/openhuman/web_chat/types.rs (1)
15-35: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Locale is missing from the cached-agent fingerprint. The agent's system prompt is locale-dependent, while cache equality ignores locale, so language changes on an existing thread reuse stale instructions.
src/openhuman/web_chat/types.rs#L15-L35: add a normalized locale/signature field toSessionCacheFingerprint.src/openhuman/web_chat/session.rs#L182-L203: accept and populate the locale field when constructing the fingerprint.src/openhuman/web_chat/run_task.rs#L97-L104: pass the normalized locale into fingerprint construction.src/openhuman/web_chat/run_task.rs#L138-L147: retain locale as an agent-construction input and add a test proving locale changes force rebuilding.📍 Affects 3 files
src/openhuman/web_chat/types.rs#L15-L35(this comment)src/openhuman/web_chat/session.rs#L182-L203src/openhuman/web_chat/run_task.rs#L97-L104src/openhuman/web_chat/run_task.rs#L138-L147🤖 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/openhuman/web_chat/types.rs` around lines 15 - 35, Locale is absent from the cached-agent fingerprint, allowing stale locale-dependent system prompts to be reused. In src/openhuman/web_chat/types.rs:15-35, add a normalized locale/signature field to SessionCacheFingerprint; in src/openhuman/web_chat/session.rs:182-203, accept and populate it during fingerprint construction; in src/openhuman/web_chat/run_task.rs:97-104, pass the normalized locale through; and in src/openhuman/web_chat/run_task.rs:138-147, retain it as an agent-construction input and add coverage proving a locale change rebuilds the cached agent.src/openhuman/web_chat/web_errors.rs (1)
668-696: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not classify every occurrence of
429as a rate limit.An unrelated request ID, token count, or URL containing those digits reaches this branch before more accurate classifications. Anchor the status to provider envelopes or
"Too Many Requests".Proposed fix
- } else if lower.contains("rate limit") || lower.contains("429") { + } else if lower.contains("rate limit") + || lower.contains("too many requests") + || lower.contains("api error (429") + {📝 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.} else if lower.contains("rate limit") || lower.contains("too many requests") || lower.contains("api error (429") { let retry_secs = parse_retry_after_secs_from_str(err); // Non-retryable business 429s ("plan does not include", balance // exhausted, known provider business codes like Z.AI 1311/1113) // also surface here — mark them non-retryable so the FE can hide // the "Retry" button and route the user to settings/billing. let non_retryable = is_non_retryable_rate_limit_text(&lower); let summary = if non_retryable { "Your AI provider is rejecting requests for billing or plan reasons \ (out of credits, plan limit, or unavailable model). Retrying won't \ help — open Settings to top up, upgrade your plan, or pick a \ different model." .to_string() } else { format!( "Your AI provider is rate-limiting requests. This is a transient upstream \ limit, not a thread-level block — you can retry in this thread.{}", retry_after_hint(retry_secs) ) }; ClassifiedError { error_type: "rate_limited", message: with_provider_detail(summary.as_str(), err), source: "provider", retryable: !non_retryable, retry_after_ms: retry_secs.map(|s| s.saturating_mul(1000)), provider, fallback_available, }🤖 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/openhuman/web_chat/web_errors.rs` around lines 668 - 696, The rate-limit branch in the error classification flow currently matches any occurrence of “429”. Update the condition around is_non_retryable_rate_limit_text and the ClassifiedError construction to require a provider status envelope or the explicit “Too Many Requests” phrase, while preserving the existing retry-after and business-rate-limit handling.
14 tasks
senamakel
pushed a commit
to M3gA-Mind/openhuman
that referenced
this pull request
Jul 18, 2026
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.
🥞 Stacked PR — merge order (bottom → top)
This is position 4/5. Stacked on #5023 → #5004 → #5020, so the diff currently shows their commits too; they self-clean as each merges below. Review the
#5003-scoped commits (feat(learning),feat(core),refactor(channels),test(learning)).Summary
Moves three always-on learning subscriber registrations out of the channels startup path and onto the always-on Platform boot path where they belong. Fixes a silent failure: users with no chat channel configured currently get no self-improvement/profile inference at all.
Problem
start_channels(src/openhuman/channels/runtime/startup.rs) is a misnamed process-wide bootstrap. It was the sole caller of threeDomainGroup::Platform(always-on) learning registrations:DocumentCanonicalized, emits identity candidates),ProfileMdRenderer(re-renders the five cache-derived PROFILE.md blocks onCacheRebuilt).spawn_channels_service(src/core/runtime/services.rs) skipsstart_channelsentirely when no chat integration is configured — logging only atdebug. So every channel-less user silently got no learning, with no error and no visible signal. Learning has nothing to do with channels; it was wired there by accident of bootstrap history (the same class of couplingflowswas already moved out of).Two of the three registrations also had a latent bug even on the channels path: they read the global memory client at that moment and permanently cached
Nonein aOnceLockif it wasn't ready yet. Registering on the Platform path — which runs aftermemory::global::init— fixes that too.Solution
Four micro-commits:
src/openhuman/learning/startup.rs—register_learning_subscribers(workspace_dir)holds the three registration blocks (moved verbatim), with theOnceLockidempotency guards inside. A pureregister_with_client(Option<MemoryClientRef>, &Path)helper makes both the ready and not-ready arms unit-testable without process globals. Logging is part of the fix:[learning]-prefixedinfo!on each successful registration, and the two previously-silentdebug!skip logs upgraded towarn!.core::jsonrpc::register_domain_subscribers— callregister_learning_subscribersinside the always-onDomainGroup::Platformblock (right after the device-tunnel subscriber). This is learning's real home; it runs regardless of channel configuration.channels/runtime/startup.rs— remove the three blocks; leave a NOTE comment pointing at the new home. After this,channels/has zero references intolearning(verified by grep) — which is exactly what the channels feature gate (feat(core): feature gate — channels & webviews #4801) needs.DocumentCanonicalized, assert an identity candidate lands; plus an idempotency test (register twice, one event, candidates not doubled) and the skip-arm test (no client → email-sig registers, trigger/renderer skip with a warn).Intentional behaviour change: channel-less users now get the email-signature learning and the 30-minute rebuild loop — that is the fix. It's the same load channel-configured users already carry; rebuild failures remain
warn-level and non-fatal. This PR is the revert handle if that's ever unwanted.Note (undercount corrected): the issue named two coupled registrations; there were three —
ProfileMdRendereris the same block and same class. Without it, the now-working rebuild loop would emitCacheRebuiltevents nobody renders on channel-less boots, leaving the symptom half-fixed. All three moved.Submission Checklist
cargo test --lib openhuman::learning→ 151 passed, 0 failed.Closesin## Related.Impact
warn; three new[learning]info logs on registration.DomainGroup::Platform(always-on), so the gates-off build stays green. Verified:cargo check/clippy -D warnings/fmtclean, gates-off compiles,grep -rn "learning" src/openhuman/channels/empty.Related
channels/, so the gate can compile channels out without taking learning with it.flowsmove out ofstart_channels.AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/5003-learning-decouple-channelsValidation Run
pnpm typecheck/format:check— no TypeScript or frontend files changedcargo fmt --checkclean;cargo check(enabled) clean;cargo clippy -- -D warningsexit 0cargo test --lib openhuman::learning→ 151 passed; 0 failed (4 new)GGML_NATIVE=OFF cargo check --no-default-features --features tokenjuice-treesittergrep -rn "learning" src/openhuman/channels/→ emptySummary by CodeRabbit
New Features
Bug Fixes