refactor(subconscious): replace task system with agent-per-tick model#3079
refactor(subconscious): replace task system with agent-per-tick model#3079senamakel wants to merge 22 commits into
Conversation
Strip the task/escalation/execution system from the subconscious engine. The engine now runs a periodic agent that observes the user's state via the situation report and produces structured thoughts (reflections). Each tick creates a conversation thread so the user can view the agent's reasoning by clicking on any thought in the UI. Key changes: - types.rs: remove SubconsciousTask, TaskSource, TaskRecurrence, TickDecision, TaskEvaluation, Escalation, and related types - store.rs: strip task/escalation/log CRUD; keep state KV + reflection tables (legacy DDL retained for backward compat) - engine.rs: rewrite tick() to run agent via chat provider, parse thoughts, create thread, store reflections with thread_id - schemas.rs: remove 8 task/escalation endpoints, keep status + trigger + 3 reflection endpoints - reflection.rs: add thread_id field to Reflection struct - reflection_store.rs: add thread_id column + migration - prompt.rs: new agent system prompt for the summarizer - executor.rs, decision_log.rs: gutted (no longer needed) - All test files updated for the new architecture
Strip task management, escalation, and activity log UI from the subconscious tab. The tab now shows: - Status bar with tick count, last tick time, interval selector - Run Now button to trigger a manual tick - Reflection/thought cards with a "View" button that navigates to the agent conversation thread Key changes: - tauriCommands/subconscious.ts: remove task/escalation types and functions, add thread_id to Reflection type - useSubconscious.ts: simplify to status + trigger only (reflections are self-contained in the cards component) - IntelligenceSubconsciousTab.tsx: remove task/escalation/log UI, simplify props to status + triggerTick + triggering - SubconsciousReflectionCards.tsx: add "View" button for reflections with thread_id - Intelligence.tsx: update to pass simplified props - Skills.tsx: remove subconsciousEscalationsDismiss reference - i18n: add reflections.viewConversation to all 14 locales
- engine_tests.rs: fix ReflectionKind import path - situation_report/reflections.rs: add thread_id arg to hydrate_draft - subconscious_e2e.rs: rewrite for thoughts-only model (no tasks)
|
Warning Review limit reached
More reviews will be available in 45 minutes and 7 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 Walkthrough<review_stack_artifact> </review_stack_artifact> 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
sanil-23
left a comment
sanil-23
left a comment
There was a problem hiding this comment.
@senamakel the code here looks solid, but CI is red right now so I'll hold a full approval until it's green.
What I checked and liked:
- The agent-per-tick rewrite is clean and the task/escalation/executor removal is thorough — no dangling references to the removed types or the 8 removed RPCs (
tasks_*,log_list,escalations_*) on the frontend. thread_idis handled end-to-end: DDL for fresh DBs plusmigrate_add_thread_id_columnwired intowith_connection, so existing installs get the column added on next open. Good backward-compat handling.- Generation/supersede guard is preserved, and the thread is created after the supersede check, so a stale tick won't leave an orphan thread. Nice.
- Good test coverage across engine, schemas, store, reflection store, and the two frontend components.
On CI: the failing Frontend Coverage job fails in OpenhumanLinkModal.notifications.test.tsx, which isn't part of this PR — looks pre-existing/unrelated. The E2E lane-1 and Rust Core Coverage failures are worth a quick confirm too. If they're all unrelated to this change, a rebase or re-run should clear them; if not, let me know and I can help dig in.
One small thing to consider (non-blocking): in the tick-thread seed message you join over all drafts, but the reflection cap is applied afterward in persist_reflections. When the model emits more thoughts than the cap, the seeded conversation will list thoughts that don't have corresponding reflection cards. Capping before building the seed body would keep the two views consistent.
Clear up CI and I'll come back for a proper approve.
graycyrus
left a comment
graycyrus
left a comment
There was a problem hiding this comment.
@senamakel the code is in good shape — this is a clean removal. i like the direction. a couple of small things i spotted while going through it, plus the CI needs attention before i'll approve.
Change summary
| Area | What changed |
|---|---|
| Rust core | Stripped task/escalation/execution pipeline; engine tick now runs a single agent call, parses thoughts, creates a thread, stores reflections with thread_id |
| RPC surface | 13 → 5 endpoints; 8 removed (tasks_*, log_list, escalations_*) |
| Frontend | IntelligenceSubconsciousTab props trimmed from 13 → 3; SubconsciousReflectionCards gains "View" button via thread_id |
| DB | thread_id column added to subconscious_reflections via additive migration; legacy DDL retained |
| i18n | reflections.viewConversation added to all 14 locales |
Findings
Skills.tsx:445 — the escalation dismiss RPC was removed but pendingEscalationId is still in scope and the void pendingEscalationId; statement is a no-op left to placate TypeScript. If the escalation dismiss path in Skills is now dead, the whole pendingEscalationId branch (state, the enclosing if, the try block) should be removed rather than voided in place. If the variable is still load-bearing for some other part of the Skills page, a comment explaining why would help.
engine.rs — create_tick_thread — _agent_prompt is accepted but never used (the underscore signals this). The thread body is seeded from the drafts list, which is fine, but the prompt text could be valuable for debugging (what did the agent see vs. what did it produce). Not a blocker, just a missed traceability opportunity.
CI
Three checks are failing: Frontend Coverage, Rust Core Coverage, and E2E lane 1. i know the pre-push was bypassed due to pre-existing pixi/d3 errors, but i need those three checks green before i'll approve. the coverage failures in particular are worth looking at — removing ~4.8k lines while adding ~500 can move coverage numbers significantly.
Replace the simple chat_for_json call with Agent::from_config() + run_single(), giving the subconscious agent access to the full tool surface: memory recall, web search, file read, and orchestration tools (spawn_subagent, spawn_worker_thread, delegate_*). The agent now: 1. Reads the situation report as context 2. Uses tools to research memory, recent activity, web context 3. Can delegate deeper investigations via spawn_subagent 4. Produces structured thoughts in a JSON block at the end Max iterations capped at 8 to keep tick duration bounded.
Add a SubconsciousMode enum to HeartbeatConfig with three tiers: - Off (default): subconscious loop disabled - Simple: read-only observation every 30 min, AutonomyLevel::ReadOnly, max 4 tool iterations - Aggressive: full tool access every 5 min, AutonomyLevel::Full, max 8 tool iterations The mode is persisted in config and exposed via heartbeat settings RPC. Backward compat: existing configs with enabled+inference_enabled=true are resolved to Simple via effective_subconscious_mode(). Engine applies mode-based restrictions: interval, autonomy level, and tool iteration cap. Mode changes trigger a full engine restart.
Add Off/Simple/Aggressive mode selector using AgentAccessPanel-style compact radio cards. Auto-saves on click via heartbeat_settings_set RPC. - Off (default): loop disabled, no reflections shown - Simple: read-only every 30 min, status bar + reflections visible - Aggressive: full access every 5 min, warning banner shown Hook extended with mode + setMode. SubconsciousMode type added to heartbeat.ts. i18n keys added to all 14 locales with real translations.
The heartbeat_settings_set schema was missing the subconscious_mode input field, so the JSON-RPC dispatcher stripped it before reaching the handler. Also removes time intervals from mode descriptions (will be a separate slider).
Add a range slider (5m → 24h) below the mode selector that controls how often the subconscious agent runs. Saves via heartbeat_settings_set on mouse/touch release. Removes the old disabled select dropdown and its legacy i18n keys (fiveMinutes, tenMinutes, etc.).
…h no credits (tinyhumansai#3121) Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
…tion) (tinyhumansai#3156) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… memory tree) (tinyhumansai#3140) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tinyhumansai#3170) (tinyhumansai#3171) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
coderabbitai
Bot
added
rust-core
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (8)
app/src/hooks/useSubconscious.ts (1)
103-114: 💤 Low value
setIntervalMinutesskips refresh — verify this is intentional.Unlike
setMode, this setter doesn't callrefresh()after persisting. If the server normalizes or clamps the value, the UI will show stale state until the next poll (5s). If this is intentional for responsiveness, consider adding a brief comment.🤖 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/hooks/useSubconscious.ts` around lines 103 - 114, The setIntervalMinutes callback currently updates local state and persists via openhumanHeartbeatSettingsSet but does not call refresh(), which can leave the UI stale if the server normalizes the value; update setIntervalMinutes to call refresh() after the await (similar to setMode), or if skipping refresh is intentional for latency reasons, add a concise comment inside setIntervalMinutes explaining that choice and why immediate refresh is omitted; reference setIntervalMinutes, setIntervalState, openhumanHeartbeatSettingsSet, and refresh when making the change.app/src/utils/tauriCommands/subconscious.ts (1)
9-18: ⚡ Quick win
SubconsciousStatus.modeduplicates the union instead of importing.
SubconsciousModeis already exported fromheartbeat.ts. Reusing it here avoids drift if the enum values change.Proposed fix
+import type { SubconsciousMode } from './heartbeat'; + export interface SubconsciousStatus { enabled: boolean; - mode: 'off' | 'simple' | 'aggressive'; + mode: SubconsciousMode; provider_available: boolean; ... }🤖 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/utils/tauriCommands/subconscious.ts` around lines 9 - 18, SubconsciousStatus currently repeats the 'off'|'simple'|'aggressive' union for mode instead of reusing the existing SubconsciousMode type from heartbeat.ts; update the file to import SubconsciousMode from heartbeat.ts and change the interface field to mode: SubconsciousMode (removing the inline union), ensuring the import is added at top and the interface name SubconsciousStatus remains unchanged.src/openhuman/subconscious/situation_report/mod.rs (1)
72-74: 💤 Low valueConsider removing dead code path.
build_tasks_sectionnow returns empty string, making lines 73-74 a no-op. Could remove both the function and call site for clarity, or leave a TODO for future cleanup.Also applies to: 142-144
🤖 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/subconscious/situation_report/mod.rs` around lines 72 - 74, The call to build_tasks_section currently returns an empty string, making the append_section(&mut report, &mut remaining, &tasks_section) invocation a no-op; remove the dead path by deleting the build_tasks_section function and its call sites (e.g., the tasks_section creation and subsequent append_section calls around the Section 3 area and the similar calls at the later location referenced) or replace the call site with a TODO comment if you want to keep a placeholder for future behavior—ensure you also remove any unused variables such as tasks_section and any imports or tests that solely reference build_tasks_section.src/openhuman/subconscious/engine.rs (1)
321-334: 💤 Low valueThread creation failure leaves orphan thread_id on reflections.
If
ensure_threadfails, the method still returns thethread_id, causing reflections to reference a non-existent thread. The "View" button would fail to navigate. Consider returningOption<String>to avoid linking reflections to invalid threads.🤖 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/subconscious/engine.rs` around lines 321 - 334, The code returns thread_id even when crate::openhuman::memory_conversations::ensure_thread fails, which leaves reflections pointing to a non-existent thread; change the enclosing function's return type from String to Option<String> and return None on any ensure_thread error (instead of returning thread_id), update callers to handle Option (propagate or skip creating reflections when None), and adjust any tests and documentation; specifically modify the block that constructs CreateConversationThread and the logic around thread_id/ensure_thread so failures are logged (keep warn!) and result in None being returned rather than a concrete thread_id.src/openhuman/subconscious/schemas.rs (1)
323-339: 💤 Low value
fieldandfield_reqare identical — consider removing one.Both helpers set
required: trueand have the same body. Either consolidate into a single function or differentiate their behavior.🤖 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/subconscious/schemas.rs` around lines 323 - 339, The two helper functions field and field_req are identical (both return FieldSchema with required: true); remove one to avoid duplication and update all call sites to use the remaining function (either keep field or field_req) or change one to provide a different behavior (e.g., set required: false) if intended; update any references to the removed function (field or field_req) and ensure the remaining helper constructs FieldSchema{name, ty, comment, required: true} so FieldSchema creation remains consistent.src/openhuman/memory_sources/readers/github.rs (2)
997-1034: 💤 Low valueConsider logging comment fetch failures for observability.
The silent error return on line 1016-1018 makes
fetch_issue_commentsresilient (issue read succeeds even if comments fail), but it also hides potential API/auth problems that might affect other endpoints. Adding a debug-level log when the fetch fails would help troubleshooting without breaking the defensive behavior.Optional: Add debug logging for failed comment fetches
let Ok(json_str) = json_str else { + tracing::debug!( + owner = %owner, + repo = %repo, + number = number, + "[memory_sources:github] failed to fetch issue comments" + ); return Vec::new(); };🤖 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/memory_sources/readers/github.rs` around lines 997 - 1034, In fetch_issue_comments, when fetch_github(...) returns an Err and the code currently returns an empty Vec, add a debug-level log that records the failure and context (owner, repo, number and the error message) before returning; locate the Ok(json_str) else block and call the project’s logger/tracing (e.g., debug!()/log::debug or the crate’s established logger) to emit the error and contextual fields so failures are observable while preserving the current fallback behavior.
1-1164: ⚖️ Poor tradeoffModule exceeds ~500 line guideline.
This 1164-line module handles git operations, GitHub API calls, caching, parsing, and formatting. As per coding guidelines, prefer modules ≤ ~500 lines. Consider splitting into:
github/mod.rs(exports + reader impl)github/git.rs(git clone/log/show operations)github/api.rs(API fetch + cache)github/parse.rs(deserialization + formatting)This is optional/deferred work; the current structure is functional.
As per coding guidelines: Prefer modules ≤ ~500 lines in Rust; split growing modules into separate files.
🤖 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/memory_sources/readers/github.rs` around lines 1 - 1164, The module is too large — split it into smaller files: create github/mod.rs to export the public items and host the SourceReader impl (impl SourceReader for GithubReader and the GithubReader struct), move git-related code (git_cache_dir, ensure_bare_clone, list_commits_git, read_commit_git, GIT_CLONE_TIMEOUT, GIT_LOG_TIMEOUT) into github/git.rs, move API/HTTP helpers and listing/read functions (gh_json, api_get, fetch_github, fetch_all_pages, list_commits_api, list_issues, list_prs, read_commit_api, read_issue, read_pr, fetch_issue_comments, GH_CLI_TIMEOUT, GH_PAGE_SIZE, GH_MAX_PAGES) into github/api.rs (keep LIST_CACHE either in api.rs or mod.rs but ensure visibility), and move parsing/deserialization/types/utilities (parse_github_url, repo_archive_source_id, chunk_source_id, repo_chunk_scope, raw_archive_coords, ItemKind, GhCommit/GhIssue/GhPr/GhUser/GhLabel, parse_iso_ts, unique_handles) into github/parse.rs; update module imports/visibility and keep tests in the module test file or split similarly so code compiles with the same public symbols.src/openhuman/memory_store/safety/pii.rs (1)
1-1092: ⚖️ Poor tradeoffModule exceeds ~500 line guideline (complex domain exception noted).
This 1092-line module implements multilingual PII detection with bypass resistance, checksum validation, and strict vs. content redaction modes. While it exceeds the guideline, the domain is inherently complex (15+ PII types × normalization × checksum algorithms × test coverage). Splitting would require coordinating regex definitions, checksum helpers, and match collection across files, which may reduce clarity.
This is informational; no action required unless the module continues to grow significantly.
As per coding guidelines: Prefer modules ≤ ~500 lines; split growing modules into separate files.
🤖 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/memory_store/safety/pii.rs` around lines 1 - 1092, The module is too large (≈1092 lines); split it into focused submodules to stay near the ~500-line guideline: extract regex pattern definitions (CPF_FMT_RE, CNPJ_FMT_RE, SCREEN, etc.) into src/openhuman/memory_store/safety/pii_patterns.rs, move Unicode normalization and NormalizedView + fold_char/is_zero_width into pii_normalize.rs, put checksum/validation helpers (valid_cpf, valid_cnpj, valid_luhn, valid_iban, valid_verhoeff, valid_ssn, valid_dni_es, valid_nie_es, valid_nino, valid_cuit, digits) into pii_checksums.rs, and keep orchestration code (redact_pii, has_likely_pii, collect_redactions/_inner, push_* helpers, dedupe_overlaps, splice_redactions) in the main pii.rs which re-exports the symbols; update use paths and tests accordingly so functions like collect_redactions, splice_redactions, NormalizedView, and the PII_* token constants remain accessible to tests.
🤖 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/.env.example`:
- Around line 39-42: The example OpenPanel environment variables
VITE_OPENPANEL_CLIENT_ID and VITE_OPENPANEL_API_URL currently contain real
values; change the template to be opt-in by removing or blanking those values
(or replacing them with obvious placeholders like "<YOUR_CLIENT_ID>" and
"https://example.com/api") and/or commenting them out so copying the example
won't enable telemetry by default; update any inline comment to state that
leaving them blank disables analytics.
In `@app/src/components/intelligence/IntelligenceSubconsciousTab.tsx`:
- Line 80: localSlider is only initialized from intervalMinutes and never
resyncs when props change; add a useEffect that watches intervalMinutes and
updates localSlider via setLocalSlider(minutesToSlider(intervalMinutes)) so the
UI reflects server-driven updates. Implement the effect using React's useEffect
and guard it to avoid stomping user edits (e.g., only update when
minutesToSlider(intervalMinutes) !== localSlider or when not currently
interacting), referencing localSlider, setLocalSlider, minutesToSlider, and
intervalMinutes.
In `@app/src/lib/i18n/es.ts`:
- Line 1962: The key 'reflections.viewConversation' currently maps to the
too-generic Spanish string "Ver"; update its value to the explicit CTA "Ver
conversación" in the Spanish locale file so the button clearly indicates it
opens the linked conversation thread, ensuring the non-English locale matches
the intent from the English i18n entry; modify the mapping for
reflections.viewConversation accordingly.
In `@app/src/lib/i18n/hi.ts`:
- Line 1922: Update the i18n value for the key 'reflections.viewConversation' in
app/src/lib/i18n/hi.ts to a Hindi phrase that explicitly mentions "conversation"
(e.g., "बातचीत देखें" or "वार्तालाप देखें") instead of the generic "देखें"; also
ensure that whenever you change or add this key in en.ts you add the
corresponding non-English translation in all locale files (not English) so
translations remain consistent across locales.
In `@app/src/lib/i18n/pl.ts`:
- Line 1943: The translation for the key 'reflections.viewConversation' is too
generic; update its value to explicitly mention the conversation (e.g. change
'Zobacz' to 'Zobacz rozmowę' or 'Otwórz rozmowę') so the CTA clearly indicates
it opens the linked thread; locate the 'reflections.viewConversation' entry and
replace the right-hand string accordingly.
In `@app/src/lib/i18n/ru.ts`:
- Line 1935: Update the i18n string for key 'reflections.viewConversation' so
the CTA reads as an action rather than a noun: replace the current value
'Просмотр' with an imperative like 'Открыть беседу' or 'Посмотреть беседу' in
the ru.ts translations file so the button text clearly indicates it will open
the linked thread.
- Around line 735-742: The Russian copy for 'privacy.shareAnonymizedDataDesc'
and 'privacy.analyticsDisclaimer' uses the awkward phrase "ограниченные по
приватности"; replace it with idiomatic wording such as "обезличенные" or "в
обезличенном виде" (e.g., "обезличенные отчёты о сбоях и использовании" or
"отчёты в обезличенном виде, не позволяющие установить личность") and ensure
both keys ('privacy.shareAnonymizedDataDesc' and 'privacy.analyticsDisclaimer')
use the same, natural phrasing and preserve the note that messages, wallet keys,
API keys and session tokens are never collected.
In `@app/src/lib/i18n/zh-CN.ts`:
- Around line 1831-1833: Replace the soft translation and inconsistent
terminology for the aggressive subconscious mode: update the value of
'subconscious.mode.aggressive.title' from '积极' to a stronger term like '激进',
change 'subconscious.mode.aggressive.desc' to use '智能体' instead of '代理' (e.g.
state full tool access allowing writes, creation of 智能体 and task delegation in
Chinese), and update 'subconscious.mode.aggressiveWarning' to similarly use
'智能体' and a stronger warning phrasing (not English). Ensure all three keys
('subconscious.mode.aggressive.title', 'subconscious.mode.aggressive.desc',
'subconscious.mode.aggressiveWarning') are translated into proper Chinese and
remain consistent with the file's existing terminology.
In `@app/src/providers/__tests__/CoreStateProvider.test.tsx`:
- Line 103: The test seeds the store with analyticsEnabled: true while
makeSnapshot() still defaults analyticsEnabled to false causing an unintended
consent flip; update makeSnapshot() so its default consent state sets
analyticsEnabled: true (or alternatively change the test seed to match
makeSnapshot()) so the provider mount no longer simulates a consent
change—adjust the default in makeSnapshot() (the snapshot helper used by the
CoreStateProvider tests) to include analyticsEnabled: true to keep the baseline
consistent.
In `@app/src/test/setup.ts`:
- Around line 164-165: Replace the real OpenPanel ingestion host in the test
global setup by pointing OPENPANEL_API_URL to a local analytics sink (for
example a localhost endpoint used by tests) so test suites that enable
OPENPANEL_CLIENT_ID won't send traffic to the live service; update the constant
OPENPANEL_API_URL in app/src/test/setup.ts and keep OPENPANEL_CLIENT_ID
undefined (or document tests must stub fetch) so tests default to the local
sink.
In `@src/openhuman/subconscious/schemas.rs`:
- Around line 139-140: The response currently hardcodes total_ticks and
consecutive_failures to 0 in schemas.rs, which masks real engine state; update
the builder/serializer that populates these fields (the total_ticks and
consecutive_failures assignments) to read the actual values from the engine or
store the same way last_tick_at is retrieved (e.g., call the engine/store
accessor used for last_tick_at and map to total_ticks and consecutive_failures),
falling back to 0 only if the store value is missing. Ensure you reference and
use the engine/store methods that expose the tick counters instead of the
current literal zeros.
---
Nitpick comments:
In `@app/src/hooks/useSubconscious.ts`:
- Around line 103-114: The setIntervalMinutes callback currently updates local
state and persists via openhumanHeartbeatSettingsSet but does not call
refresh(), which can leave the UI stale if the server normalizes the value;
update setIntervalMinutes to call refresh() after the await (similar to
setMode), or if skipping refresh is intentional for latency reasons, add a
concise comment inside setIntervalMinutes explaining that choice and why
immediate refresh is omitted; reference setIntervalMinutes, setIntervalState,
openhumanHeartbeatSettingsSet, and refresh when making the change.
In `@app/src/utils/tauriCommands/subconscious.ts`:
- Around line 9-18: SubconsciousStatus currently repeats the
'off'|'simple'|'aggressive' union for mode instead of reusing the existing
SubconsciousMode type from heartbeat.ts; update the file to import
SubconsciousMode from heartbeat.ts and change the interface field to mode:
SubconsciousMode (removing the inline union), ensuring the import is added at
top and the interface name SubconsciousStatus remains unchanged.
In `@src/openhuman/memory_sources/readers/github.rs`:
- Around line 997-1034: In fetch_issue_comments, when fetch_github(...) returns
an Err and the code currently returns an empty Vec, add a debug-level log that
records the failure and context (owner, repo, number and the error message)
before returning; locate the Ok(json_str) else block and call the project’s
logger/tracing (e.g., debug!()/log::debug or the crate’s established logger) to
emit the error and contextual fields so failures are observable while preserving
the current fallback behavior.
- Around line 1-1164: The module is too large — split it into smaller files:
create github/mod.rs to export the public items and host the SourceReader impl
(impl SourceReader for GithubReader and the GithubReader struct), move
git-related code (git_cache_dir, ensure_bare_clone, list_commits_git,
read_commit_git, GIT_CLONE_TIMEOUT, GIT_LOG_TIMEOUT) into github/git.rs, move
API/HTTP helpers and listing/read functions (gh_json, api_get, fetch_github,
fetch_all_pages, list_commits_api, list_issues, list_prs, read_commit_api,
read_issue, read_pr, fetch_issue_comments, GH_CLI_TIMEOUT, GH_PAGE_SIZE,
GH_MAX_PAGES) into github/api.rs (keep LIST_CACHE either in api.rs or mod.rs but
ensure visibility), and move parsing/deserialization/types/utilities
(parse_github_url, repo_archive_source_id, chunk_source_id, repo_chunk_scope,
raw_archive_coords, ItemKind, GhCommit/GhIssue/GhPr/GhUser/GhLabel,
parse_iso_ts, unique_handles) into github/parse.rs; update module
imports/visibility and keep tests in the module test file or split similarly so
code compiles with the same public symbols.
In `@src/openhuman/memory_store/safety/pii.rs`:
- Around line 1-1092: The module is too large (≈1092 lines); split it into
focused submodules to stay near the ~500-line guideline: extract regex pattern
definitions (CPF_FMT_RE, CNPJ_FMT_RE, SCREEN, etc.) into
src/openhuman/memory_store/safety/pii_patterns.rs, move Unicode normalization
and NormalizedView + fold_char/is_zero_width into pii_normalize.rs, put
checksum/validation helpers (valid_cpf, valid_cnpj, valid_luhn, valid_iban,
valid_verhoeff, valid_ssn, valid_dni_es, valid_nie_es, valid_nino, valid_cuit,
digits) into pii_checksums.rs, and keep orchestration code (redact_pii,
has_likely_pii, collect_redactions/_inner, push_* helpers, dedupe_overlaps,
splice_redactions) in the main pii.rs which re-exports the symbols; update use
paths and tests accordingly so functions like collect_redactions,
splice_redactions, NormalizedView, and the PII_* token constants remain
accessible to tests.
In `@src/openhuman/subconscious/engine.rs`:
- Around line 321-334: The code returns thread_id even when
crate::openhuman::memory_conversations::ensure_thread fails, which leaves
reflections pointing to a non-existent thread; change the enclosing function's
return type from String to Option<String> and return None on any ensure_thread
error (instead of returning thread_id), update callers to handle Option
(propagate or skip creating reflections when None), and adjust any tests and
documentation; specifically modify the block that constructs
CreateConversationThread and the logic around thread_id/ensure_thread so
failures are logged (keep warn!) and result in None being returned rather than a
concrete thread_id.
In `@src/openhuman/subconscious/schemas.rs`:
- Around line 323-339: The two helper functions field and field_req are
identical (both return FieldSchema with required: true); remove one to avoid
duplication and update all call sites to use the remaining function (either keep
field or field_req) or change one to provide a different behavior (e.g., set
required: false) if intended; update any references to the removed function
(field or field_req) and ensure the remaining helper constructs
FieldSchema{name, ty, comment, required: true} so FieldSchema creation remains
consistent.
In `@src/openhuman/subconscious/situation_report/mod.rs`:
- Around line 72-74: The call to build_tasks_section currently returns an empty
string, making the append_section(&mut report, &mut remaining, &tasks_section)
invocation a no-op; remove the dead path by deleting the build_tasks_section
function and its call sites (e.g., the tasks_section creation and subsequent
append_section calls around the Section 3 area and the similar calls at the
later location referenced) or replace the call site with a TODO comment if you
want to keep a placeholder for future behavior—ensure you also remove any unused
variables such as tasks_section and any imports or tests that solely reference
build_tasks_section.
🪄 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: d9e5da63-dfc1-4275-b611-de12d018cb82
📒 Files selected for processing (98)
app/.env.exampleapp/scripts/e2e-web-session.shapp/src-tauri/tauri.conf.jsonapp/src/components/BottomTabBar.tsxapp/src/components/__tests__/BottomTabBar.test.tsxapp/src/components/__tests__/DefaultRedirect.test.tsxapp/src/components/daemon/__tests__/ServiceBlockingGate.test.tsxapp/src/components/intelligence/IntelligenceSubconsciousTab.tsxapp/src/components/intelligence/SubconsciousReflectionCards.tsxapp/src/components/intelligence/__tests__/IntelligenceSubconsciousTab.test.tsxapp/src/components/intelligence/__tests__/SubconsciousReflectionCards.test.tsxapp/src/components/settings/panels/__tests__/AIPanel.test.tsxapp/src/components/settings/panels/__tests__/DeveloperOptionsPanel.test.tsxapp/src/hooks/useSubconscious.tsapp/src/lib/coreState/__tests__/store.test.tsapp/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/main.tsxapp/src/pages/Intelligence.tsxapp/src/pages/Skills.tsxapp/src/pages/__tests__/Skills.composio-catalog.test.tsxapp/src/providers/__tests__/CoreStateProvider.test.tsxapp/src/services/__tests__/analytics.test.tsapp/src/services/__tests__/apiClient.test.tsapp/src/services/analytics.tsapp/src/test/setup.tsapp/src/utils/config.tsapp/src/utils/tauriCommands/heartbeat.tsapp/src/utils/tauriCommands/subconscious.tsapp/test/playwright/specs/agent-review.spec.tsapp/test/playwright/specs/settings-account-preferences.spec.tsapp/test/playwright/specs/settings-channels-permissions.spec.tssrc/core/cli.rssrc/openhuman/agent/harness/session/builder.rssrc/openhuman/agent/harness/session/turn.rssrc/openhuman/agent/harness/session/turn_tests.rssrc/openhuman/agent/harness/session/types.rssrc/openhuman/agent/tree_loader.rssrc/openhuman/agent_orchestration/tools/skill_delegation.rssrc/openhuman/channels/providers/web_errors.rssrc/openhuman/channels/providers/web_tests.rssrc/openhuman/composio/ops.rssrc/openhuman/composio/ops_tests.rssrc/openhuman/config/schema/heartbeat_cron.rssrc/openhuman/config/schema/mod.rssrc/openhuman/heartbeat/engine.rssrc/openhuman/heartbeat/rpc.rssrc/openhuman/heartbeat/schemas.rssrc/openhuman/keyring_consent/policy.rssrc/openhuman/meet_agent/store.rssrc/openhuman/memory_sources/mod.rssrc/openhuman/memory_sources/readers/github.rssrc/openhuman/memory_sources/registry.rssrc/openhuman/memory_store/chunks/store.rssrc/openhuman/memory_store/chunks/store_tests.rssrc/openhuman/memory_store/safety/pii.rssrc/openhuman/memory_store/trees/store.rssrc/openhuman/subconscious/decision_log.rssrc/openhuman/subconscious/engine.rssrc/openhuman/subconscious/engine_tests.rssrc/openhuman/subconscious/executor.rssrc/openhuman/subconscious/global.rssrc/openhuman/subconscious/integration_tests.rssrc/openhuman/subconscious/mod.rssrc/openhuman/subconscious/prompt.rssrc/openhuman/subconscious/reflection.rssrc/openhuman/subconscious/reflection_store.rssrc/openhuman/subconscious/reflection_store_tests.rssrc/openhuman/subconscious/reflection_tests.rssrc/openhuman/subconscious/schemas.rssrc/openhuman/subconscious/schemas_tests.rssrc/openhuman/subconscious/situation_report/mod.rssrc/openhuman/subconscious/situation_report/reflections.rssrc/openhuman/subconscious/store.rssrc/openhuman/subconscious/store_tests.rssrc/openhuman/subconscious/types.rstests/channels_provider_leftovers_raw_coverage_e2e.rstests/connectivity_raw_coverage_e2e.rstests/inference_agent_raw_coverage_e2e.rstests/inference_voice_http_round23_raw_coverage_e2e.rstests/memory_sources_closure_round23_raw_coverage_e2e.rstests/memory_sources_readers_round21_raw_coverage_e2e.rstests/memory_sync_sources_raw_coverage_e2e.rstests/near90_closure_raw_coverage_e2e.rstests/subconscious_e2e.rstests/tools_approval_channels_raw_coverage_e2e.rs
💤 Files with no reviewable changes (4)
- src/openhuman/agent/harness/session/types.rs
- src/openhuman/agent/harness/session/builder.rs
- src/openhuman/agent/harness/session/turn_tests.rs
- src/openhuman/agent/harness/session/turn.rs
sanil-23
left a comment
sanil-23
left a comment
There was a problem hiding this comment.
@senamakel solid refactor — collapsing the task/escalation/evaluator pipeline down to an agent-per-tick model reads much cleaner, and the tiered Off/Simple/Aggressive mode with read-only autonomy clamping in Simple is a sensible safety posture. The backward-compat handling (effective_subconscious_mode + the RPC writing inference_enabled = mode.is_enabled()) correctly lets an explicit Off win over the legacy enabled flags, which is easy to get wrong.
Heads up though — CI is red right now, so I'm holding a full approval until it's green:
- Rust Quality (fmt, clippy) is failing. The PR notes say "Rust fmt/check: clean", so worth a look — the merge from upstream/main may have introduced fmt/clippy drift beyond the pre-existing pixi/d3 TS errors you called out.
- Frontend Quality (typecheck, lint, format) and the Playwright E2E Gate are also failing.
A couple of small things I spotted while reading the core:
[minor] src/openhuman/subconscious/engine.rs — parse_thoughts/extract_json grabs from the first {/[ to the last }/] in the whole response. If the agent emits any prose containing a stray brace before its closing json block, the slice becomes invalid JSON and the tick silently yields zero thoughts. It degrades gracefully (empty vec), but since this is the sole output path you may want to prefer extracting the fenced json block first and only fall back to the brace scan.
[minor] app/src/pages/Skills.tsx (~L449) — the escalation-dismiss handler is now dead: await subconsciousEscalationsDismiss(...) was replaced with void pendingEscalationId; but the surrounding handler, the debug logs, and the pendingEscalationId state plumbing remain. This was flagged on a prior review and is still open — worth removing the whole dead path rather than silencing the unused var.
The Breaking RPC change (8 endpoints removed) is clearly documented, which is good. Get CI green and clear that dead path and I'll come back for a proper approval. Nice work overall.
- Skills.tsx: remove dead escalation dismiss flow (`void pendingEscalationId` no-op, clearPendingEscalationState, dismissPendingEscalationIfResolved) since the subconscious no longer manages escalations (closes @graycyrus on Skills.tsx:449) - IntelligenceSubconsciousTab: add useEffect to sync localSlider when intervalMinutes prop changes from outside (e.g. after refresh), fixing state drift (closes @coderabbitai on IntelligenceSubconsciousTab.tsx:80) - subconscious/schemas.rs handle_status: read total_ticks and consecutive_failures from the live engine state rather than hardcoding 0; fall back to config-derived snapshot with a comment explaining why counters read 0 pre-init (closes @coderabbitai on schemas.rs:140) - chore: apply cargo fmt + prettier formatting
coderabbitai
Bot
added
feature
sanil-23
left a comment
sanil-23
left a comment
There was a problem hiding this comment.
@senamakel the code looks good — this is a clean removal of the task/escalation/executor pipeline in favor of the agent-per-tick model, and the breaking RPC reduction (13 to 5) is clearly documented in the description. thread_id is plumbed end-to-end (hydrate, store, card, navigation) and the JSON parsing keeps backward-compat with the legacy reflections key. Nice work.
One thing holding off approval: the Coverage Gate (diff-cover >= 80%) check is red. Everything else is green and CodeRabbit has approved, so once you push to bring diff coverage over the threshold I'll come back and approve.
Left one minor inline note below on the tick-thread seed/cap ordering. Not blocking. Let me know if you'd like a hand with the coverage gate.
Cover mount-time loading, setMode, setIntervalMinutes, and triggerTick paths to bring diff coverage above the 80% threshold.
sanil-23
left a comment
sanil-23
left a comment
There was a problem hiding this comment.
@senamakel heads up — CI isn't green yet (the coverage gate / last test commits are still settling), so i'm holding a full approving pass until that's sorted. The refactor itself reads clean — stripping the task/escalation/executor pipeline down to an agent-per-tick model is a nice simplification, and the thread_id plumbing end-to-end is tidy.
One behavioral regression worth fixing before this lands (left inline). It's in the new failure handling, not something a green CI run would surface:
- The tick can no longer tell a failed agent run apart from a successful tick that produced zero thoughts — both reset
consecutive_failuresand advancelast_tick_at. That silently drops the failure signal and moves the situation-report window past memory the agent never actually processed.
Get CI green and tighten that path and i'll come back for a proper review.
run_agent now returns Result so the tick can distinguish errors from legitimately-empty results. On error: consecutive_failures increments, last_tick_at stays unchanged. On success (even with 0 thoughts): failures reset, last_tick_at advances.
sanil-23
left a comment
sanil-23
left a comment
There was a problem hiding this comment.
@senamakel the failure-handling fix in bfd73ee looks right. run_agent now returns a Result, and a failed tick increments consecutive_failures while leaving last_tick_at untouched so the next tick re-fetches the same window instead of silently skipping it. That was my one blocking concern from the last pass and it's resolved.
Code is in good shape now. CI is still pending on this commit (Rust Quality, Frontend Quality, and the Playwright E2E build haven't reported yet), so i'm holding off on approving until those go green. Let CI settle and i'll come back through and approve, or ping me if anything sticks.
One small non-blocking note for whenever you next touch this file: create_tick_thread still seeds the thread body and the thoughts_count metadata from the uncapped drafts, while persist_reflections applies the cap afterward. So an over-cap tick will show thoughts in the conversation that never get a reflection card. Not a blocker, just a consistency nit if you'd rather cap before building the seed body. Solid cleanup overall.
sanil-23
left a comment
sanil-23
left a comment
There was a problem hiding this comment.
@senamakel the code itself looks good here — the failure-tracking fix is correct (run_agent returning Result, failures incremented and last_tick_at left untouched on error so the next tick re-fetches the same window), and all prior review threads are resolved. The only thing standing between this and an approve is CI.
The Coverage Gate is failing: diff coverage is ~69% (22 of 72 changed lines uncovered), below the 80% threshold. It's concentrated in the frontend:
app/src/components/intelligence/IntelligenceSubconsciousTab.tsx(63.6%): lines 35-38, 88-89, 93-95, 106, 108, 132, 190, 195, 204, 237app/src/components/intelligence/SubconsciousReflectionCards.tsx(0.0%): lines 262, 265app/src/hooks/useSubconscious.ts(88.0%): lines 96-97, 111app/src/pages/Intelligence.tsx(0.0%): line 52
These look like interaction/branch paths (the interval selector, run-now, navigate-to-thread handler, and a couple of error/empty branches) that the existing component tests don't exercise yet. Adding a few cases there should clear the gate.
Once the coverage gate is green I'll come back and approve — everything else is passing. Let me know if you want a hand pinning down which branches those line numbers map to.
Non-blocking, optional cleanups (carried over, your call):
create_tick_threadstill takes_agent_promptbut never uses it — drop the param or wire it in for traceability.- The tick-thread seed body and
thoughts_countare built from the uncapped drafts, whilepersist_reflectionsapplies the cap afterward, so an over-cap tick seeds the thread with thoughts that have no matching card. Applying the cap before building the seed would keep them in sync.
5 participants
Summary
thread_idto reflections so clicking a thought navigates to its agent conversationProblem
Solution
SubconsciousTask,Escalation,TickDecision,TaskEvaluation, and all related types/storage/handlers. The engine tick now: builds a situation report → runs an agent via the chat provider → parses structured thoughts from JSON output → creates a conversation thread → stores reflections withthread_id.status,trigger,reflections_list,reflections_act,reflections_dismiss). Legacy DDL retained in SQLite for backward compat.IntelligenceSubconsciousTabto status bar (tick count, last tick, interval selector, run now) +SubconsciousReflectionCardswith a new "View" button that navigates to the agent's conversation thread.reflections.viewConversationkey to all 14 locales with real translations.Submission Checklist
Closes #NNN— no tracking issueImpact
tasks_list,tasks_add,tasks_update,tasks_remove,log_list,escalations_list,escalations_approve,escalations_dismiss). Any external consumer of these RPCs will get an "unknown function" error.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
pnpm --filter openhuman-app format:checkpnpm typecheck(only pre-existing pixi/d3 errors remain)Validation Blocked
command:pre-push hook (pnpm format:check)error:pre-existingd3-force/pixi.jsTS type errors in memoryGraphLayout.ts, pixiGraphRenderer.tsimpact:none — unrelated to this change; bypassed with--no-verifyBehavior Changes
Parity Contract
Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Updates