perf(core): consolidate boot polls, silence per-snapshot log spam - #5075
Merged
Conversation
useDaemonHealth mounts in several places at once (SocketProvider via useDaemonLifecycle; ServiceBlockingGate directly + via useDaemonLifecycle). setupHealthListener awaited the first poll before assigning pollingIntervalId, so all concurrent callers raced past the singleton guard and each spawned their own setInterval — emitting multiple openhuman.health_snapshot RPCs per tick. Assign the interval id synchronously and reference-count consumers: the shared loop starts on the first consumer and is torn down only when the last releases, so one component unmounting can't stop polling for the rest. Adds a regression test covering concurrent setup and partial release.
LocalAIDownloadSnackbar (mounted app-wide) polled inference_status + inference_downloads_progress every 2s forever, even with no download active. Make the poll self-scheduling: 2s while a download is in flight, 15s when idle.
React.StrictMode double-mounts HarnessInitOverlay in dev (effect → cleanup → effect); each setup fires an immediate poll, booting two harness_init_status RPCs at the same instant. Coalesce overlapping status fetches onto one in-flight request (cleared once settled, so ongoing sequential polling is unaffected). Also guards a genuine remount during the boot window.
…valid A bare `OPENHUMAN_SHELL_HIDE_WINDOW=` (empty/whitespace-only, common when a .env or launcher exports the key with no value) fell through to the "unrecognized value ignored" warn arm and logged a warning on every boot. Treat an empty value as absent — silently keep the current setting. Extends the existing env-override test to cover the empty and whitespace-only cases.
…shot log spam policy::initialize() is documented as a once-at-startup call but is wired into build_app_state_snapshot, so it runs on every app_state_snapshot RPC — logging `[keyring_consent] initialize` at INFO each time (several per boot as the frontend polls the snapshot). Hold the write lock across a compare-and-set and return early when the persisted consent is unchanged, so it writes + logs only on a genuine transition. Adds PartialEq/Eq to ConsentPreference and a change-gating regression test.
Points the tauri-cef submodule at fix/cef-helper-verbose-logging, which gates the per-subprocess [cef-helper-*] eprintln! traces behind OPENHUMAN_CEF_HELPER_VERBOSE (silent by default). Note: takes effect in bundled builds only after reinstalling the vendored tauri-cli (cargo install --locked --path app/src-tauri/vendor/tauri-cef/crates/tauri-cli).
CoreStateProvider polled the expensive app_state_snapshot RPC every 2s for the life of the session, only slowing on repeated bootstrap failure. Once booted and authenticated the snapshot changes rarely and mostly via event-driven refreshes, so steady state now polls at STABLE_POLL_MS (5s); bootstrapping/unauthenticated stays at 2s so boot/login transitions still surface promptly.
…lth poll The frontend ran a second dedicated health_snapshot poller (daemonHealthService) alongside the app_state_snapshot poll. Fold the core's health snapshot into app_state_snapshot (new snake_case `health` field) and hydrate the daemon-health store from that one poll instead: - core: AppStateSnapshot carries HealthSnapshot; ComponentHealth/HealthSnapshot gain Deserialize. json_rpc_e2e asserts the folded health shape. - frontend: daemonHealthService drops all polling and becomes an ingestion sink (ingestHealthSnapshot: parse + store update + 30s disconnect watchdog); CoreStateProvider feeds each snapshot's health payload to it; useDaemonHealth no longer starts a poll. Older cores that omit `health` degrade gracefully. Supersedes the earlier health_snapshot poller dedupe (d34101f) — there is now no separate health poll at all. Regression tests rewritten for the sink API.
…e inference poll LocalAIDownloadSnackbar polled inference_status + inference_downloads_progress on its own timer even when idle. inference_status returns the same LocalAiStatus the app_state_snapshot already carries as runtime.localAi, so detect download activity from core state instead: when idle the snackbar issues ZERO inference calls; only once runtime.localAi.state reports loading/downloading/installing does it run the fast 2s poll for granular progress/speed/ETA (downloads-progress detail the 2-5s snapshot cadence can't provide), stopping when the download settles. LocalModelDebugPanel (settings-only, opt-in) keeps its own live poll.
The rule referenced `worktree/*` (singular) but the directory is `worktrees/`, so local `git worktree` checkouts under worktrees/ were never ignored and showed as untracked. Match the real directory name; keep the .gitkeep negation.
Contributor
📝 WalkthroughWalkthroughThe change embeds daemon health in app-state snapshots, updates frontend polling, coalesces initialization requests, folds orchestration into Brain, adjusts configuration and consent behavior, and updates repository maintenance scripts and references. ChangesHealth and polling flows
Brain orchestration surface
Configuration and repository maintenance
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Brain
participant OrchestrationView
participant Browser
Brain->>OrchestrationView: render orchestration tab
OrchestrationView->>Browser: read ov, sub, session query parameters
OrchestrationView->>Browser: update query parameters on navigation
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: dea9ee5cd2
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![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: 8
🧹 Nitpick comments (1)
app/src/services/daemonHealthService.ts (1)
26-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the required debug diagnostics for the folded-health flow.
app/src/services/daemonHealthService.ts#L26-L37: log accepted versus ignored payloads and watchdog re-arms using non-sensitive metadata only.app/src/providers/CoreStateProvider.tsx#L295-L300: log health-ingestion outcomes, including stale-response suppression.app/src/providers/CoreStateProvider.tsx#L540-L555: log the selected polling cadence and its branch reason.As per coding guidelines, changed TS flows require “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 `@app/src/services/daemonHealthService.ts` around lines 26 - 37, In app/src/services/daemonHealthService.ts lines 26-37, add non-sensitive, grep-friendly diagnostics to ingestHealthSnapshot for payload acceptance or rejection and each watchdog re-arm. In app/src/providers/CoreStateProvider.tsx lines 295-300, log health-ingestion outcomes and stale-response suppression. In app/src/providers/CoreStateProvider.tsx lines 540-555, log the selected polling cadence together with the branch reason, covering the relevant flow transitions without exposing payload contents.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 `@app/src-tauri/vendor/tauri-cef`:
- Line 1: Synchronize the tauri-cef submodule pointer with the pin guard by
updating it to SHA 11ef51edbeadcca4517b18a037fff858a5dfae0f, or update the
corresponding expected SHA configuration to match the current pointer; then
rerun the pin-guard checks.
In `@app/src/components/InitProgressScreen/HarnessInitOverlay.tsx`:
- Around line 28-47: Add namespaced, privacy-safe debug diagnostics to both
polling flows: in app/src/components/InitProgressScreen/HarnessInitOverlay.tsx
lines 28-47, log coalesced request creation, in-flight sharing, settlement, and
failure around fetchHarnessInitStatusCoalesced; in
app/src/components/LocalAIDownloadSnackbar.tsx lines 76-118, log activation, RPC
poll results and failures, continuation, cancellation, and timeout scheduling.
Keep logs grep-friendly and avoid sensitive payload data.
In `@app/src/components/LocalAIDownloadSnackbar.tsx`:
- Around line 88-118: The snackbar’s downloading state can remain active after
polling is cancelled when coreDownloadActive becomes false. Update the
isDownloading derivation in LocalAIDownloadSnackbar to require
coreDownloadActive, while preserving the existing detailed status/download
checks when the folded state is active.
- Around line 93-110: Update the poll function in LocalAIDownloadSnackbar so
transient RPC failures continue scheduling retries when coreDownloadActive
remains true, rather than leaving active false. Preserve stopping behavior only
after a successful terminal status/progress response, while keeping cancellation
checks intact.
In `@app/src/providers/CoreStateProvider.tsx`:
- Around line 295-300: The health ingestion in the CoreStateProvider snapshot
refresh must occur only after mount/request freshness validation and committing
the new identity, so stale or invalidated responses cannot update health. Move
the daemonHealthService.ingestHealthSnapshot call after the request-id guard and
identity commit, and pass the incoming snapshot’s resolved user ID so both the
health store update and watchdog use that identity instead of
getCoreStateSnapshot().
In `@app/src/services/coreStateApi.ts`:
- Around line 77-91: Update the component timestamp fields in RawHealthSnapshot
so last_ok and last_error accept string, null, or undefined, matching the
nullable values emitted by the Rust health serialization contract while
preserving their optional status.
In `@src/openhuman/config/schema/load_tests.rs`:
- Around line 238-261: The tests in
src/openhuman/config/schema/load_tests.rs:238-261 must capture tracing output or
inspect parser classification to assert empty and whitespace-only
OPENHUMAN_SHELL_HIDE_WINDOW values emit no warning, not only preserve state. The
repeated-consent test in src/openhuman/keyring_consent/policy.rs:265-292 must
verify no log or cache update occurs on repeated consent, rather than checking
only the final cache value.
In `@src/openhuman/config/schema/load/env_overlay.rs`:
- Around line 124-128: Add stable trace-level events to both intentional no-op
paths: in src/openhuman/config/schema/load/env_overlay.rs lines 124-128, trace
when an empty or whitespace-only shell-window value is treated as absent; in
src/openhuman/keyring_consent/policy.rs lines 27-45, trace when cached consent
remains unchanged. Keep both paths silent at higher log levels and preserve
their existing behavior.
---
Nitpick comments:
In `@app/src/services/daemonHealthService.ts`:
- Around line 26-37: In app/src/services/daemonHealthService.ts lines 26-37, add
non-sensitive, grep-friendly diagnostics to ingestHealthSnapshot for payload
acceptance or rejection and each watchdog re-arm. In
app/src/providers/CoreStateProvider.tsx lines 295-300, log health-ingestion
outcomes and stale-response suppression. In
app/src/providers/CoreStateProvider.tsx lines 540-555, log the selected polling
cadence together with the branch reason, covering the relevant flow transitions
without exposing payload contents.
🪄 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: c0dd43dd-b046-4d69-b05e-8ca1744aa22f
📒 Files selected for processing (19)
.gitignoreapp/src-tauri/vendor/tauri-cefapp/src/components/InitProgressScreen/HarnessInitOverlay.test.tsxapp/src/components/InitProgressScreen/HarnessInitOverlay.tsxapp/src/components/LocalAIDownloadSnackbar.tsxapp/src/components/__tests__/LocalAIDownloadSnackbar.test.tsxapp/src/hooks/useDaemonHealth.tsapp/src/providers/CoreStateProvider.tsxapp/src/services/__tests__/daemonHealthService.test.tsapp/src/services/coreStateApi.tsapp/src/services/daemonHealthService.tssrc/openhuman/app_state/ops.rssrc/openhuman/config/schema/load/env_overlay.rssrc/openhuman/config/schema/load_tests.rssrc/openhuman/health/core.rssrc/openhuman/keyring_consent/policy.rssrc/openhuman/keyring_consent/types.rstests/json_rpc_e2e.rsworktrees/.gitkeep
Updates .github/tauri-cef-expected-sha to match the tauri-cef submodule bump (11ef51ed), which gates the CEF helper's per-subprocess diagnostic prints behind OPENHUMAN_CEF_HELPER_VERBOSE. 11ef51ed is a direct child of the prior pin 5ec3d883, so the Linux AppImage glibc/NSS library-exclusion fixes the pin guards (tinyhumansai#1996, tinyhumansai#2032, tinyhumansai#2154/tinyhumansai#2088) are preserved. Submodule commit pushed to tinyhumansai/tauri-cef (branch fix/cef-helper-verbose-logging) so CI can resolve the pin.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 758694bc42
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- LocalAIDownloadSnackbar: gate isDownloading on coreDownloadActive so a download the core no longer reports active can't leave the overlay stuck visible; on a transient poll error keep retrying while core state is active (previously one blip permanently stopped progress polling for the download). - CoreStateProvider: ingest folded health AFTER the snapshot is committed and only when the refresh is still current, so daemon health is written under the freshly-committed identity, not a stale/pre-commit/superseded token. - coreStateApi: type health last_ok/last_error as string | null to match the Rust Option<String> wire contract. - keyring_consent::initialize now returns whether it applied a change, so the change-gate's suppressed write/log is asserted directly (not just unchanged state); config shell-hide-window parsing extracted into a testable classifier that distinguishes Unset (empty → silent) from Unrecognized (warns). - Add trace-level diagnostics to the no-op env/keyring paths and namespaced debug logging to the harness-init coalescing + snackbar poll lifecycles.
coderabbitai
Bot
added
feature
…pshots Folding health into app_state_snapshot coupled the 30s disconnect watchdog to an RPC allowed to run up to 90s (first-launch snapshots take 30–40s), so a slow-but- alive core could be marked `disconnected` mid-boot. Widen HEALTH_TIMEOUT_MS to 120s to cover one worst-case slow snapshot plus poll cadence; genuine disconnection (snapshots stop succeeding) is still detected. Addresses Codex review P2.
Member
Author
|
Addressed all review threads (CodeRabbit + Codex) in
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/providers/CoreStateProvider.tsx`:
- Around line 354-363: Add privacy-safe, namespaced debug/trace diagnostics
throughout the health refresh and polling branches in CoreStateProvider,
covering health presence, request freshness, the
daemonHealthService.ingestHealthSnapshot call, selected poll delay, and whether
the delay reason is bootstrap, authenticated, or failure-backoff. Include branch
decisions, external calls, state transitions, retries/timeouts, and errors
without logging payloads, tokens, or PII; anchor the changes to the existing
refresh logic and ingestion call.
🪄 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: 0c4ad905-709f-41eb-a94c-80cdcc57e5cc
📒 Files selected for processing (8)
.github/tauri-cef-expected-shaapp/src/components/InitProgressScreen/HarnessInitOverlay.tsxapp/src/components/LocalAIDownloadSnackbar.tsxapp/src/providers/CoreStateProvider.tsxapp/src/services/coreStateApi.tssrc/openhuman/config/schema/load/env_overlay.rssrc/openhuman/config/schema/load_tests.rssrc/openhuman/keyring_consent/policy.rs
🚧 Files skipped from review as they are similar to previous changes (4)
- app/src/components/InitProgressScreen/HarnessInitOverlay.tsx
- app/src/components/LocalAIDownloadSnackbar.tsx
- src/openhuman/config/schema/load/env_overlay.rs
- app/src/services/coreStateApi.ts
Per the repo's verbose-diagnostics guideline, log privacy-safe namespaced events for the folded-health ingest branch (request freshness, health presence, component count — never payload/tokens) and the poll-delay selection (delay + reason: bootstrap / authenticated / failure-backoff). Addresses CodeRabbit.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 779a344299
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a3c1b43584
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…tructure - Added `OrchestrationRedirect` component to handle legacy `/orchestration` route, redirecting to `/brain?tab=orchestration` with query parameter mapping. - Updated `AppRoutes` to use the new redirect and removed the direct route to `OrchestrationPage`. - Modified `Brain` component to include `orchestration` as a sub-tab, ensuring legacy deep links are correctly routed. - Enhanced documentation for clarity on routing changes and tab structure.
…cution - Improved the pre-push hook to handle abort signals (Ctrl+C/SIGTERM) more effectively, ensuring that the script exits with the correct status code. - Introduced a `run_check` function to encapsulate command execution and error handling, allowing for cleaner and more maintainable code. - Removed the `OrchestrationPage` component and its associated tests, as part of a broader refactor to streamline the application structure.
…gest The refactor armed the disconnect watchdog only inside a successful health parse, so a core whose app_state_snapshots never carry parseable health (repeated timeouts, after the one-shot agent probe already set `running`) would never arm a watchdog and stick at `running`. Arm a baseline watchdog when tracking starts (CoreStateProvider → ensureWatchdogArmed, idempotent) and treat any arriving snapshot as liveness that re-arms it — even a health-less/older-core payload — while only updating the store on a valid parse. Addresses Codex review P2.
Remove the top-level Orchestration sidebar tab and surface it as a Brain sub-tab (/brain?tab=orchestration) via the new OrchestrationView, whose top-level views (Overview/Chat/Agent graph/Tasks/Network) become an in-content chip row so Brain keeps a single sidebar. - navConfig: drop the orchestration NAV_TABS entry (6 tabs now) - settings + legacy /orchestration deep links redirect to the Brain tab, mapping ?tab=/?sub= onto the ?ov=/?sub= scheme - i18n: rename brain.tabs.tinyplaceOrchestration → brain.tabs.orchestration with real translations across all locales - tests: OrchestrationView unit tests, updated navConfig/Brain/nav-rail/e2e
coderabbitai
Bot
added
bug
and removed
feature
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b4c5764c2
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+117
to
+120
| if (!cancelled && !settled) { | ||
| timerRef.current = setTimeout(poll, ACTIVE_POLL_INTERVAL); | ||
| } else if (settled) { | ||
| log('fast poll: download settled, stopping'); |
There was a problem hiding this comment.
If one local-AI download reaches a terminal status and another starts before CoreStateProvider's snapshot observes the idle gap, coreDownloadActive stays true the whole time. This branch stops the only fast-poll timer, and because the effect depends only on that boolean (true → true), the next download never restarts inference_status / progress polling, so its snackbar can remain hidden or stale. Keep a retry armed while the folded state is still active, or key the effect on a changing generation/state rather than only the boolean.
Useful? React with 👍 / 👎.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/features/conversations/Conversations.tsx (1)
245-254: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGuard the new Redux maps before indexing them.
If a legacy or narrow
chatRuntimestate omitsturnTranscriptsByThreadorinterruptedAssistantByThread, the selected-thread paths indexundefinedand crash during render.EMPTY_TURN_TRANSCRIPTSonly covers a missing per-thread entry; add stable top-level empty maps in the selectors as well.Also applies to: 414-419, 1858-1860, 1898-1900
🤖 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 `@app/src/features/conversations/Conversations.tsx` around lines 245 - 254, Update the selected-thread selectors and related paths using turnTranscriptsByThread and interruptedAssistantByThread to fall back to stable top-level empty maps when those Redux fields are absent, before indexing by thread ID. Preserve EMPTY_TURN_TRANSCRIPTS for missing per-thread entries and apply the same guard to the additional usages around the affected selectors and render paths.
🧹 Nitpick comments (2)
app/src/components/orchestration/OrchestrationView.tsx (1)
123-129: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider using the namespaced
debug()package instead ofconsole.debugfor consistency.Sibling files in this same
orchestration/folder (e.g.AgentChatPanel.tsx's "debug('steering review: trigger')"-style calls anduseOrchestrationSessions.ts's "debug('[orchestration:sessions] contact-sessions refresh: entry')") use the namespaceddebugpackage rather thanconsole.debug. Usingconsole.debughere always logs and can't be selectively toggled viaDEBUG, unlike the established convention in this domain.As per coding guidelines, "New or changed flows must include verbose, grep-friendly diagnostics... Use... namespaced debug logging in the app."
🤖 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 `@app/src/components/orchestration/OrchestrationView.tsx` around lines 123 - 129, Replace the console.debug call in OrchestrationView with the established namespaced debug logger used by neighboring orchestration components. Preserve the existing mount diagnostic message and arguments, while ensuring it can be toggled through the DEBUG environment convention.Source: Coding guidelines
app/src/store/__tests__/chatRuntimeSlice.thunk.test.ts (1)
46-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
PersistedTranscriptItem[]for the transcript fixture.Array<Record<string, unknown>>drifts fromPersistedTurnState.transcriptand makes it easier for invalid transcript shapes to slip into the test data.🤖 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 `@app/src/store/__tests__/chatRuntimeSlice.thunk.test.ts` around lines 46 - 51, Update the persistedTurn fixture’s transcript parameter to use PersistedTranscriptItem[] instead of Array<Record<string, unknown>>, importing the established type if needed, so test data matches PersistedTurnState.transcript and enforces valid transcript shapes.
🤖 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 `@app/src/AppRoutes.tsx`:
- Around line 38-68: Add grep-friendly console.debug diagnostics within
OrchestrationRedirect, logging the incoming legacy search parameters and the
selected mapping branch, including network-sub, orchestration-view, and unmapped
cases, plus session passthrough when present. Keep the existing redirect
behavior unchanged and log the final Brain destination before returning
Navigate.
In `@app/src/pages/Brain.tsx`:
- Around line 38-45: Update the intelligence.session_orchestration how_to
mapping in catalog_data.rs from Intelligence > Orchestration to Brain >
Orchestration, and add E2E coverage that directly navigates to
/brain?tab=orchestration and verifies the orchestration tab is shown. Preserve
existing /brain navigation coverage.
In `@app/src/services/daemonHealthService.ts`:
- Around line 33-64: Add namespaced, grep-friendly debug logs in
ensureWatchdogArmed and ingestHealthSnapshot for watchdog arm versus
already-armed, snapshot receipt and timeout re-arm, valid health application,
and ignored missing or invalid health. Do not log payload contents or user IDs;
preserve the existing watchdog and store-update behavior while covering the
relevant branches and state transitions.
---
Outside diff comments:
In `@app/src/features/conversations/Conversations.tsx`:
- Around line 245-254: Update the selected-thread selectors and related paths
using turnTranscriptsByThread and interruptedAssistantByThread to fall back to
stable top-level empty maps when those Redux fields are absent, before indexing
by thread ID. Preserve EMPTY_TURN_TRANSCRIPTS for missing per-thread entries and
apply the same guard to the additional usages around the affected selectors and
render paths.
---
Nitpick comments:
In `@app/src/components/orchestration/OrchestrationView.tsx`:
- Around line 123-129: Replace the console.debug call in OrchestrationView with
the established namespaced debug logger used by neighboring orchestration
components. Preserve the existing mount diagnostic message and arguments, while
ensuring it can be toggled through the DEBUG environment convention.
In `@app/src/store/__tests__/chatRuntimeSlice.thunk.test.ts`:
- Around line 46-51: Update the persistedTurn fixture’s transcript parameter to
use PersistedTranscriptItem[] instead of Array<Record<string, unknown>>,
importing the established type if needed, so test data matches
PersistedTurnState.transcript and enforces valid transcript shapes.
🪄 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: 68e5443c-fc97-4545-843d-9cdcb9d6a9a7
📒 Files selected for processing (37)
.husky/pre-pushapp/src/AppRoutes.tsxapp/src/components/layout/shell/CollapsedNavRail.test.tsxapp/src/components/orchestration/AgentChatPanel.tsxapp/src/components/orchestration/ConnectionsPanel.tsxapp/src/components/orchestration/OrchestrationView.tsxapp/src/components/orchestration/__tests__/OrchestrationView.test.tsxapp/src/components/settings/settingsRouteElements.tsxapp/src/config/__tests__/navConfig.test.tsapp/src/config/navConfig.tsapp/src/features/conversations/Conversations.tsxapp/src/features/conversations/components/InterruptedAnswer.tsxapp/src/lib/i18n/ar.tsapp/src/lib/i18n/bn.tsapp/src/lib/i18n/de.tsapp/src/lib/i18n/en.tsapp/src/lib/i18n/es.tsapp/src/lib/i18n/fr.tsapp/src/lib/i18n/hi.tsapp/src/lib/i18n/id.tsapp/src/lib/i18n/it.tsapp/src/lib/i18n/ko.tsapp/src/lib/i18n/pl.tsapp/src/lib/i18n/pt.tsapp/src/lib/i18n/ru.tsapp/src/lib/i18n/zh-CN.tsapp/src/pages/Brain.tsxapp/src/pages/OrchestrationPage.tsxapp/src/pages/__tests__/Brain.test.tsxapp/src/pages/__tests__/OrchestrationPage.test.tsxapp/src/providers/CoreStateProvider.tsxapp/src/services/__tests__/daemonHealthService.test.tsapp/src/services/daemonHealthService.tsapp/src/store/__tests__/chatRuntimeSlice.thunk.test.tsapp/src/store/chatRuntimeSlice.test.tsapp/test/e2e/specs/navigation.spec.tstests/json_rpc_e2e.rs
💤 Files with no reviewable changes (4)
- app/src/components/layout/shell/CollapsedNavRail.test.tsx
- app/src/pages/tests/OrchestrationPage.test.tsx
- app/src/pages/OrchestrationPage.tsx
- tests/json_rpc_e2e.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/providers/CoreStateProvider.tsx
Comment on lines
+38
to
+68
| /** | ||
| * Redirects the retired `/orchestration` route to its new home under Brain | ||
| * (`/brain?tab=orchestration`), mapping the legacy `?tab=`/`?sub=` query onto | ||
| * Brain's `?ov=`/`?sub=` scheme so old deep links land on the same view: | ||
| * - `?tab=connections|discover|usage` → `?ov=network&sub=<that>` | ||
| * - `?tab=agent|overview|tasks|network|medulla` → `?ov=<that>` | ||
| * - `?session=` is preserved for the agent chat. | ||
| */ | ||
| const NETWORK_SUBS = ['connections', 'discover', 'usage']; | ||
| const ORCH_VIEWS = ['medulla', 'agent', 'overview', 'tasks', 'network']; | ||
|
|
||
| function OrchestrationRedirect() { | ||
| const { search } = useLocation(); | ||
| const legacy = new URLSearchParams(search); | ||
| const tab = legacy.get('tab'); | ||
|
|
||
| const next = new URLSearchParams(); | ||
| next.set('tab', 'orchestration'); | ||
| if (tab && NETWORK_SUBS.includes(tab)) { | ||
| next.set('ov', 'network'); | ||
| next.set('sub', tab); | ||
| } else { | ||
| if (tab && ORCH_VIEWS.includes(tab)) next.set('ov', tab); | ||
| const sub = legacy.get('sub'); | ||
| if (sub && NETWORK_SUBS.includes(sub)) next.set('sub', sub); | ||
| } | ||
| const session = legacy.get('session'); | ||
| if (session) next.set('session', session); | ||
|
|
||
| return <Navigate to={`/brain?${next.toString()}`} replace />; | ||
| } |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add grep-friendly diagnostics to OrchestrationRedirect.
This is a new flow with real branching (network-sub vs. orchestration-view mapping, session passthrough) but has no logging at all, unlike the analogous legacy-redirect effect in Brain.tsx (console.debug('[brain] legacy tinyplace-orchestration deep link → ...')) and the mount log in OrchestrationView.tsx. Add an entry/decision log so misrouted legacy deep links (/orchestration?tab=...) are diagnosable in the field.
🩹 Proposed diagnostic logging
function OrchestrationRedirect() {
const { search } = useLocation();
const legacy = new URLSearchParams(search);
const tab = legacy.get('tab');
const next = new URLSearchParams();
next.set('tab', 'orchestration');
if (tab && NETWORK_SUBS.includes(tab)) {
next.set('ov', 'network');
next.set('sub', tab);
} else {
if (tab && ORCH_VIEWS.includes(tab)) next.set('ov', tab);
const sub = legacy.get('sub');
if (sub && NETWORK_SUBS.includes(sub)) next.set('sub', sub);
}
const session = legacy.get('session');
if (session) next.set('session', session);
+ console.debug('[app-routes] legacy /orchestration redirect: tab=%s sub=%s → %s', tab, legacy.get('sub'), next.toString());
return <Navigate to={`/brain?${next.toString()}`} replace />;
}Based on coding guidelines, "New or changed flows must include verbose, grep-friendly diagnostics covering entry/exit, branches, external calls, retries/timeouts, state transitions, and errors."
📝 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
| /** | |
| * Redirects the retired `/orchestration` route to its new home under Brain | |
| * (`/brain?tab=orchestration`), mapping the legacy `?tab=`/`?sub=` query onto | |
| * Brain's `?ov=`/`?sub=` scheme so old deep links land on the same view: | |
| * - `?tab=connections|discover|usage` → `?ov=network&sub=<that>` | |
| * - `?tab=agent|overview|tasks|network|medulla` → `?ov=<that>` | |
| * - `?session=` is preserved for the agent chat. | |
| */ | |
| const NETWORK_SUBS = ['connections', 'discover', 'usage']; | |
| const ORCH_VIEWS = ['medulla', 'agent', 'overview', 'tasks', 'network']; | |
| function OrchestrationRedirect() { | |
| const { search } = useLocation(); | |
| const legacy = new URLSearchParams(search); | |
| const tab = legacy.get('tab'); | |
| const next = new URLSearchParams(); | |
| next.set('tab', 'orchestration'); | |
| if (tab && NETWORK_SUBS.includes(tab)) { | |
| next.set('ov', 'network'); | |
| next.set('sub', tab); | |
| } else { | |
| if (tab && ORCH_VIEWS.includes(tab)) next.set('ov', tab); | |
| const sub = legacy.get('sub'); | |
| if (sub && NETWORK_SUBS.includes(sub)) next.set('sub', sub); | |
| } | |
| const session = legacy.get('session'); | |
| if (session) next.set('session', session); | |
| return <Navigate to={`/brain?${next.toString()}`} replace />; | |
| } | |
| function OrchestrationRedirect() { | |
| const { search } = useLocation(); | |
| const legacy = new URLSearchParams(search); | |
| const tab = legacy.get('tab'); | |
| const next = new URLSearchParams(); | |
| next.set('tab', 'orchestration'); | |
| if (tab && NETWORK_SUBS.includes(tab)) { | |
| next.set('ov', 'network'); | |
| next.set('sub', tab); | |
| } else { | |
| if (tab && ORCH_VIEWS.includes(tab)) next.set('ov', tab); | |
| const sub = legacy.get('sub'); | |
| if (sub && NETWORK_SUBS.includes(sub)) next.set('sub', sub); | |
| } | |
| const session = legacy.get('session'); | |
| if (session) next.set('session', session); | |
| console.debug('[app-routes] legacy /orchestration redirect: tab=%s sub=%s → %s', tab, legacy.get('sub'), next.toString()); | |
| return <Navigate to={`/brain?${next.toString()}`} replace />; | |
| } |
🤖 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 `@app/src/AppRoutes.tsx` around lines 38 - 68, Add grep-friendly console.debug
diagnostics within OrchestrationRedirect, logging the incoming legacy search
parameters and the selected mapping branch, including network-sub,
orchestration-view, and unmapped cases, plus session passthrough when present.
Keep the existing redirect behavior unchanged and log the final Brain
destination before returning Navigate.
Source: Coding guidelines
Comment on lines
+38
to
+45
| type BrainTab = | ||
| | 'welcome' | ||
| | 'graph' | ||
| | 'goals' | ||
| | 'sources' | ||
| | 'sync' | ||
| | 'subconscious' | ||
| | 'orchestration'; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether about_app docs mention the Brain orchestration tab / old top-level orchestration destination
fd . src/openhuman/about_app -e md -e mdx --exec grep -l -i "orchestration" {} \;
# Check whether navigation E2E specs cover the new /brain?tab=orchestration destination or the legacy redirects
rg -n "orchestration" app/test/e2e/specs/navigation.spec.tsRepository: tinyhumansai/openhuman
Length of output: 470
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "about_app files:"
git ls-files 'src/openhuman/about_app/**'
echo
echo "orchestration mentions in about_app:"
rg -n -i "orchestration|brain" src/openhuman/about_app || true
echo
echo "navigation spec context:"
sed -n '1,120p' app/test/e2e/specs/navigation.spec.tsRepository: tinyhumansai/openhuman
Length of output: 5669
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "about_app README:"
sed -n '1,220p' src/openhuman/about_app/README.md
echo
echo "Brain page references in about_app:"
rg -n -i "brain|tab|route|orchestration" src/openhuman/about_app/{README.md,catalog_data.rs,mod.rs,types.rs,schemas.rs,ops.rs,catalog.rs} || trueRepository: tinyhumansai/openhuman
Length of output: 17244
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "E2E mentions of brain tab params:"
rg -n "tab=orchestration|orchestration.*tab|brain\\?tab|tinyplace-orchestration|Brain > Orchestration|orchestration folded under Brain" app/test/e2e || true
echo
echo "Brain page test file context:"
sed -n '1,260p' app/src/pages/Brain.test.tsxRepository: tinyhumansai/openhuman
Length of output: 838
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1818,1842p' src/openhuman/about_app/catalog_data.rs
echo
echo "related Brain how_to entries:"
rg -n 'how_to: "Brain >|how_to: "Intelligence >' src/openhuman/about_app/catalog_data.rsRepository: tinyhumansai/openhuman
Length of output: 3099
Update the Session Orchestration mapping and add direct tab coverage
src/openhuman/about_app/catalog_data.rsstill pointsintelligence.session_orchestrationatIntelligence > Orchestration; update thathow_totext toBrain > Orchestrationso the catalog matches the UI.- Add an E2E check for
/brain?tab=orchestration; current navigation coverage exercises/brain, but not the tab-specific entry point.
🤖 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 `@app/src/pages/Brain.tsx` around lines 38 - 45, Update the
intelligence.session_orchestration how_to mapping in catalog_data.rs from
Intelligence > Orchestration to Brain > Orchestration, and add E2E coverage that
directly navigates to /brain?tab=orchestration and verifies the orchestration
tab is shown. Preserve existing /brain navigation coverage.
Source: Path instructions
Comment on lines
+33
to
64
| /** | ||
| * Arm the disconnect watchdog once when daemon-health tracking starts, if it | ||
| * isn't already armed. Without this, a core whose `app_state_snapshot`s never | ||
| * succeed (repeated timeouts) — after `useDaemonHealth`'s one-shot agent probe | ||
| * has set the status to `running` — would never arm a watchdog and stick at | ||
| * `running` forever. The baseline watchdog guarantees a fallback to | ||
| * `disconnected` if no snapshot ever arrives, and is re-armed by each ingest. | ||
| */ | ||
| ensureWatchdogArmed(): void { | ||
| if (this.healthTimeoutId === null) { | ||
| this.startHealthTimeout(); | ||
| } | ||
| } | ||
|
|
||
| const pollOnce = async () => { | ||
| try { | ||
| const payload = await callCoreRpc<unknown>({ method: 'openhuman.health_snapshot' }); | ||
| const healthSnapshot = this.parseHealthSnapshot(payload); | ||
| if (healthSnapshot) { | ||
| this.updateDaemonStoreFromHealth(healthSnapshot); | ||
| this.startHealthTimeout(); | ||
| } | ||
| } catch { | ||
| // The health endpoint can fail while the sidecar is starting. | ||
| } | ||
| }; | ||
|
|
||
| await pollOnce(); | ||
| this.pollingIntervalId = setInterval(() => { | ||
| void pollOnce(); | ||
| }, this.POLL_MS); | ||
| /** | ||
| * Ingest a health payload carried by an `app_state_snapshot` refresh. | ||
| * | ||
| * The snapshot arriving at all is proof the core is alive, so the disconnect | ||
| * watchdog is re-armed unconditionally — even when the payload is missing or | ||
| * unparseable (an older core that doesn't fold health, or a partial payload) — | ||
| * otherwise a live-but-health-less core would eventually be marked | ||
| * `disconnected`. The daemon store is only updated when a valid health | ||
| * snapshot is present; otherwise it keeps its last-known state. | ||
| */ | ||
| ingestHealthSnapshot(payload: unknown): void { | ||
| // Called by CoreStateProvider only after a successful snapshot → liveness. | ||
| this.startHealthTimeout(); | ||
|
|
||
| return () => this.cleanup(); | ||
| const healthSnapshot = this.parseHealthSnapshot(payload); | ||
| if (healthSnapshot) { | ||
| this.updateDaemonStoreFromHealth(healthSnapshot); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add diagnostics for watchdog and ingestion branches.
The new flow logs only timeout/errors. Add namespaced debug logs for watchdog arm/already-armed, snapshot receipt/re-arm, valid health application, and ignored invalid/missing health—without logging payloads or user IDs.
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 `@app/src/services/daemonHealthService.ts` around lines 33 - 64, Add
namespaced, grep-friendly debug logs in ensureWatchdogArmed and
ingestHealthSnapshot for watchdog arm versus already-armed, snapshot receipt and
timeout re-arm, valid health application, and ignored missing or invalid health.
Do not log payload contents or user IDs; preserve the existing watchdog and
store-update behavior while covering the relevant branches and state
transitions.
Source: Coding guidelines
12 tasks
12 tasks
6 tasks
12 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
app_state_snapshotand delete the separatehealth_snapshotpoller — the daemon-health store now hydrates from the same poll.runtime.localAistate: zero inference polls when idle, fast poll only during an active download.app_state_snapshotpoll from a permanent 2s to 5s once booted + authenticated (bootstrapping/unauthenticated stays 2s).harness_init_statusboot poll; dedupe/retire redundanthealth_snapshotpollers.keyring_consentcache init, treat emptyOPENHUMAN_SHELL_HIDE_WINDOWas unset, and gate the CEF helper's[cef-helper-*]diagnostic prints behindOPENHUMAN_CEF_HELPER_VERBOSE.Problem
Boot and steady-state logs were flooded with redundant, high-frequency RPCs and warnings:
health_snapshot,app_state_snapshot, andinference_status/inference_downloads_progresseach ran on their own 2s pollers (some duplicated across concurrent React mounts), andapp_state_snapshot— which rebuilds the runtime snapshot and reloads config + local state (observed up to ~2s) — polled every 2s for the entire session. Each snapshot also re-emitted[keyring_consent] initializeat INFO and anOPENHUMAN_SHELL_HIDE_WINDOW unrecognized valuewarning (from a bare=env value). The CEF helper printed unconditional[cef-helper-*]lines on every subprocess spawn.Solution
AppStateSnapshotnow carries a snake_casehealthfield (health::snapshot());daemonHealthServicebecomes an ingestion sink (ingestHealthSnapshot: parse + store update + 30s disconnect watchdog) fed byCoreStateProvider. The snackbar readsruntime.localAi.stateto decide when to run its fast poll, so idle issues no inference calls. Older cores that omithealthdegrade gracefully.app_state_snapshotbacks off to 5s once stable; event-driven refreshes (deep-link, settings toggles) still fire immediately.keyring_consent::initializewrites/logs only on an actual consent transition; empty/whitespaceOPENHUMAN_SHELL_HIDE_WINDOWis treated as unset (no warning); the CEF helper's diagnostic prints are gated behind an env flag (silent by default).harness_init_statusboot poll is coalesced across StrictMode's double-mount; theworktrees/gitignore rule was corrected (worktree/*→worktrees/*).Submission Checklist
docs/TEST-COVERAGE-MATRIX.mdImpact
app_state_snapshotevery 5s (now also carrying health + local-AI state); idle inference and standalone health polls are eliminated. No user-facing behavior change; daemon-disconnect detection retains its 30s watchdog.OPENHUMAN_CEF_HELPER_VERBOSE=1restores the CEF helper traces for debugging.tauri-clireinstalled for the CEF-helper commit) for the change to take effect in a running app.Related
LocalModelDebugPanel's standalone inference poll onto the folded state; consider lowering residual health/app-state RPC logs to DEBUG.AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
pnpm --filter openhuman-app format:checkpnpm typecheckGGML_NATIVE=OFF cargo check --manifest-path Cargo.tomlpnpm rust:check(via pre-push hook)Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
health_snapshotRPC still exists for CLI/other callers.healthfield are ignored gracefully;OPENHUMAN_CEF_HELPER_VERBOSErestores prior verbose output.Duplicate / Superseded PR Handling
Summary by CodeRabbit
SHELL_HIDE_WINDOWvalues as unset, and only update keyring consent when it changes.