Skip to content

feat(home): banners, welcome typewriter, conversation gating#936

Merged
senamakel merged 10 commits into
tinyhumansai:mainfrom
senamakel:feat/sat-apr-25
Apr 26, 2026
Merged

feat(home): banners, welcome typewriter, conversation gating#936
senamakel merged 10 commits into
tinyhumansai:mainfrom
senamakel:feat/sat-apr-25

Conversation

@senamakel
Copy link
Copy Markdown
Member

@senamakel senamakel commented Apr 26, 2026

Summary

  • Add usage-limit and promotional banners on the Home page; refactor Home layout for improved structure.
  • Replace anchor tags in banners with buttons for proper interaction; add unit tests covering banner behavior.
  • Add WelcomeThinkingTypewriter component and gate the conversation composer until the welcome flow completes.
  • Sanitize user session data and cache user resolution to improve identity-change handling.
  • Backfill snapshot.currentUser from auth.user when missing; tighten provider regression coverage.

Problem

Home lacked surfaces for usage limits and promos, anchor-based banner CTAs were inconsistent with the rest of the app's button affordances, and the composer could be used before the welcome agent completed onboarding. Identity flips also risked stale currentUser state in CoreStateProvider.

Solution

  • Extracted banner UI into HomeBanners with proper button semantics + tests.
  • Introduced typewriter welcome component and composer gating in Conversations.
  • Hardened CoreStateProvider to backfill currentUser and clear scoped caches across identity changes (covered by new tests).

Submission Checklist

  • Unit tests — Vitest coverage added for HomeBanners, Conversations composer gating, and CoreStateProvider identity flows.
  • E2E / integration — N/A; pure UI/state changes, no new RPC surface.
  • Doc comments — Existing module conventions preserved; no new public API.
  • Inline comments — Minimal, only where non-obvious.

Impact

  • Desktop UI only. No RPC/schema/migration changes.
  • No performance regressions expected; banners render conditionally.

Related

  • Closes:
  • Follow-up PR(s)/TODOs:

Summary by CodeRabbit

  • New Features

    • Added usage limit and promotional credits banners plus a Discord banner on the Home page.
    • Introduced animated welcome messages and a centered version label on Home/Conversations.
    • Connection modal now shows inline connection ID and adds a Close button.
  • Bug Fixes / Improvements

    • Improved composer interaction blocking and input disabling logic.
    • Sanitizes stored user payloads and adds cached current-user refresh with TTL.
    • Exposes promotional balance in usage data.
  • Tests

    • Added unit tests for banners and composer blocking; removed legacy bootstrap UI tests.
  • Chores

    • Ensured desktop build injects updater settings.

@senamakel senamakel requested a review from a team April 26, 2026 07:51
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 26, 2026

📝 Walkthrough

Walkthrough

Adds home-page banners and tests, changes composer blocking and welcome UI with a typewriter, refactors Home header and user resolution, adds CoreStateProvider user fallback, sanitizes and TTL-caches current-user in Rust, tweaks Tauri updater envs in the release workflow, and updates/removes several tests.

Changes

Cohort / File(s) Summary
Workflow config
\.github/workflows/release.yml
Injects Tauri updater env vars (BASE_URL, UPDATER_PUBLIC_KEY, UPDATER_ENDPOINT, UPDATER_REPO) and sets WITH_UPDATER: "true" for the desktop build step.
Home banners + tests
app/src/components/home/HomeBanners.tsx, app/src/components/home/__tests__/HomeBanners.test.tsx
Adds UsageLimitBanner, PromotionalCreditsBanner, DiscordBanner, and formatUsd; tests assert CTAs call openUrl with expected URLs.
Home page refactor + tests
app/src/pages/Home.tsx, app/src/pages/__tests__/Home.test.tsx
Removes Local AI bootstrap UI/polling, adds rotating typed welcome, integrates new banners, exposes resolveHomeUserName, and replaces bootstrap integration tests with unit tests for name resolution.
Conversations page + tests
app/src/pages/Conversations.tsx, app/src/pages/__tests__/Conversations.test.tsx
Adds exported isComposerInteractionBlocked predicate and uses it to gate send behavior; replaces welcome placeholder with a typewriter animation; adds unit tests for predicate.
Modal UI update
app/src/components/composio/ComposioConnectModal.tsx
Success view now shows inline connection id; action area changed from single full-width button to two-button grid with “Disconnect” and new “Close” calling onClose.
State provider & tests
app/src/providers/CoreStateProvider.tsx, app/src/providers/__tests__/CoreStateProvider.test.tsx
normalizeSnapshot now falls back from result.currentUser to result.auth.user; tests updated to assert fallback behavior.
Types
app/src/types/api.ts
IUserUsage gains optional promotionBalanceUsd?: number.
Rust: user sanitization & caching
src/openhuman/app_state/ops.rs, src/openhuman/credentials/ops.rs, src/openhuman/credentials/ops_tests.rs, src/openhuman/app_state/ops_tests.rs
Sanitizes stored auth.user (drops null/empty-object), introduces fetch_current_user_cached with in-memory TTL-based cache and conditional refresh, updates snapshot() to use cached fetch with fallback and logging; adds tests covering sanitization and TTL freshness.
Tests removed/updated / test setup
app/src/pages/__tests__/HomeBootstrapButtons.test.tsx (deleted), app/src/test/setup.ts
Removes Home bootstrap integration tests; test setup mock now exports APP_VERSION = '0.0.0-test'.

Sequence Diagram(s)

sequenceDiagram
  participant Snapshot as "Snapshot()"
  participant Cache as "CurrentUserCache"
  participant Backend as "Backend API"
  participant Logger as "Logger"

  Snapshot->>Cache: lookup(api_base, token)
  alt cache fresh
    Cache-->>Snapshot: return cached_user
  else cache missing or stale
    Snapshot->>Backend: fetch /current_user
    Backend-->>Snapshot: user_json
    Snapshot->>Snapshot: sanitize(user_json)
    alt sanitized non-empty
      Snapshot->>Cache: store (user_json, now)
      Snapshot-->>Snapshot: return sanitized user
    else sanitized empty
      Snapshot->>Cache: clear entry
      Logger-->>Snapshot: warn("empty user from backend")
      Snapshot-->>Snapshot: return stored/sanitized snapshot user fallback
    end
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

🐰 Banners bloom on home's fresh ground,
Caches tidy, empty users found,
Typewriter welcomes hop and play,
Composer gates keep stray thoughts away,
CI flags hop in—release on its way. 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the three main changes: home page banners, welcome typewriter animation, and conversation gating logic.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Bootstrap UI moved to settings/LocalModelPanel; the Home page no longer
renders Bootstrap/Retry/Running controls. Add APP_VERSION to the shared
config mock so other Home-rendering tests keep working.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (1)
app/src/pages/__tests__/Conversations.test.tsx (1)

5-27: Missing test case for rustChat: false.

The isComposerInteractionBlocked function returns true when rustChat is false, but this scenario has no test coverage. All three tests pass rustChat: true.

🧪 Suggested additional test case
   it('allows composer interaction when chat is idle and ready', () => {
     expect(
       isComposerInteractionBlocked({ activeThreadId: null, welcomePending: false, rustChat: true })
     ).toBe(false);
   });
+
+  it('blocks composer interaction when rust chat is unavailable', () => {
+    expect(
+      isComposerInteractionBlocked({ activeThreadId: null, welcomePending: false, rustChat: false })
+    ).toBe(true);
+  });
 });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/src/pages/__tests__/Conversations.test.tsx` around lines 5 - 27, Add a
test in the existing describe('isComposerInteractionBlocked') block that covers
the case when rustChat is false: call isComposerInteractionBlocked with
rustChat: false (e.g., activeThreadId: null and welcomePending: false) and
assert it returns true; reference the isComposerInteractionBlocked function name
to locate where to add the new it(...) test so the branch where rustChat is
false is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/release.yml:
- Around line 413-419: Remove the unresolved git conflict markers (<<<<<<<
Updated upstream, =======, >>>>>>> Stashed changes) and restore a valid env
block by keeping the intended variables BASE_URL, UPDATER_PUBLIC_KEY,
UPDATER_ENDPOINT and UPDATER_REPO; locate the conflict around the env mapping
that mentions BASE_URL and UPDATER_* and replace the entire conflict region with
a single clean YAML mapping containing those keys (ensuring UPDATER_PUBLIC_KEY
uses the secrets fallback expression shown and values remain valid YAML), then
validate the YAML to ensure the workflow parses.

In `@app/src/components/home/__tests__/HomeBanners.test.tsx`:
- Around line 8-40: The openUrl mock is shared across tests and accumulates
calls, so add a per-test reset (e.g., a beforeEach or afterEach) that clears
mocks to ensure each spec starts fresh; specifically, in the test suite
containing UsageLimitBanner, PromotionalCreditsBanner, DiscordBanner and the
openUrl/ DISCORD_INVITE_URL assertions, call vi.clearAllMocks() (or
openUrl.mockClear()) before each test to reset call history so each
expect(openUrl).toHaveBeenCalledWith(...) only reflects the current test's
interaction.

In `@app/src/components/home/HomeBanners.tsx`:
- Around line 23-38: The computed tone-specific button classes in styles.button
are never applied, so the CTA uses the hardcoded amber classes; update the JSX
in HomeBanners where the CTA button is rendered (instances that currently
hardcode amber classes around the CTA, including the second occurrence) to use
the dynamic styles.button instead of the static class string—locate the button
elements in HomeBanners.tsx and replace their className with styles.button (or
merge it if other classes are present) so the danger tone renders the correct
coral CTA.

In `@app/src/pages/Conversations.tsx`:
- Line 479: Remove the duplicate guard that checks composerInteractionBlocked
(the unreachable `if (composerInteractionBlocked) return;`) since the same
condition is already handled earlier; locate the redundant line near the handler
in Conversations.tsx (the function surrounding composer interaction logic) and
delete that second check, keeping only the original early return so control flow
and any subsequent logic remain unchanged.

In `@app/src/pages/Home.tsx`:
- Line 50: The console.log call in Home.tsx currently prints the raw user object
(console.log('showPromoBanner', showPromoBanner, user, promoCredits)); remove
the raw user from this render path and instead log only non-sensitive derived
values (e.g., showPromoBanner, promoCredits, user plan/flags or boolean
isSubscribed/isAdmin) or a sanitized user summary that omits identifiers and
PII; update the console.log invocation in the render path to only include those
safe fields or remove it entirely.

In `@src/openhuman/app_state/ops.rs`:
- Around line 492-518: Move the inline tests out of ops.rs into a new tests
module file (e.g., ops_tests.rs) to keep ops.rs under ~500 lines; keep the
existing sanitize_snapshot_user test but replace the TTL-only assertion that
checks Instant math with a behavioral test that exercises
fetch_current_user_cached using a CachedCurrentUser instance and
CURRENT_USER_REFRESH_TTL to assert cache-hit vs cache-miss behavior (e.g.,
prepare cached entry with fetched_at older/newer than CURRENT_USER_REFRESH_TTL
and call fetch_current_user_cached to validate it returns cached user when fresh
and triggers refresh when expired).

---

Nitpick comments:
In `@app/src/pages/__tests__/Conversations.test.tsx`:
- Around line 5-27: Add a test in the existing
describe('isComposerInteractionBlocked') block that covers the case when
rustChat is false: call isComposerInteractionBlocked with rustChat: false (e.g.,
activeThreadId: null and welcomePending: false) and assert it returns true;
reference the isComposerInteractionBlocked function name to locate where to add
the new it(...) test so the branch where rustChat is false is covered.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 64199675-9836-4ce9-a756-cf116537b4b2

📥 Commits

Reviewing files that changed from the base of the PR and between 339f44f and bef532f.

⛔ Files ignored due to path filters (2)
  • Cargo.lock is excluded by !**/*.lock
  • app/src-tauri/Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (14)
  • .github/workflows/release.yml
  • app/src/components/composio/ComposioConnectModal.tsx
  • app/src/components/home/HomeBanners.tsx
  • app/src/components/home/__tests__/HomeBanners.test.tsx
  • app/src/pages/Conversations.tsx
  • app/src/pages/Home.tsx
  • app/src/pages/__tests__/Conversations.test.tsx
  • app/src/pages/__tests__/Home.test.tsx
  • app/src/providers/CoreStateProvider.tsx
  • app/src/providers/__tests__/CoreStateProvider.test.tsx
  • app/src/types/api.ts
  • src/openhuman/app_state/ops.rs
  • src/openhuman/credentials/ops.rs
  • src/openhuman/credentials/ops_tests.rs

Comment thread .github/workflows/release.yml Outdated
Comment thread app/src/components/home/__tests__/HomeBanners.test.tsx
Comment thread app/src/components/home/HomeBanners.tsx
Comment thread app/src/pages/Conversations.tsx Outdated
Comment thread app/src/pages/Home.tsx Outdated
Comment thread src/openhuman/app_state/ops.rs Outdated
Comment on lines +492 to +518
#[cfg(test)]
mod tests {
use super::*;
use serde_json::json;

#[test]
fn sanitize_snapshot_user_drops_empty_payloads() {
assert_eq!(sanitize_snapshot_user(Some(json!({}))), None);
assert_eq!(sanitize_snapshot_user(Some(Value::Null)), None);
assert_eq!(
sanitize_snapshot_user(Some(json!({ "firstName": "steven" }))),
Some(json!({ "firstName": "steven" }))
);
}

#[test]
fn cached_current_user_entry_expires_after_ttl() {
let entry = CachedCurrentUser {
api_base: "https://staging-api.tinyhumans.ai".to_string(),
token: "tok".to_string(),
fetched_at: Instant::now() - (CURRENT_USER_REFRESH_TTL + Duration::from_millis(1)),
user: json!({ "firstName": "steven" }),
};

assert!(entry.fetched_at.elapsed() >= CURRENT_USER_REFRESH_TTL);
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Strengthen cache TTL test and move inline tests out of ops.rs.

Line 508’s test currently validates Instant math, not fetch_current_user_cached behavior; it won’t catch TTL-branch regressions. Also, this inline module pushes the file over the repository’s ~500-line target.

♻️ Proposed refactor
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use serde_json::json;
-
-    #[test]
-    fn sanitize_snapshot_user_drops_empty_payloads() {
-        assert_eq!(sanitize_snapshot_user(Some(json!({}))), None);
-        assert_eq!(sanitize_snapshot_user(Some(Value::Null)), None);
-        assert_eq!(
-            sanitize_snapshot_user(Some(json!({ "firstName": "steven" }))),
-            Some(json!({ "firstName": "steven" }))
-        );
-    }
-
-    #[test]
-    fn cached_current_user_entry_expires_after_ttl() {
-        let entry = CachedCurrentUser {
-            api_base: "https://staging-api.tinyhumans.ai".to_string(),
-            token: "tok".to_string(),
-            fetched_at: Instant::now() - (CURRENT_USER_REFRESH_TTL + Duration::from_millis(1)),
-            user: json!({ "firstName": "steven" }),
-        };
-
-        assert!(entry.fetched_at.elapsed() >= CURRENT_USER_REFRESH_TTL);
-    }
-}
+#[cfg(test)]
+#[path = "ops_tests.rs"]
+mod tests;
// src/openhuman/app_state/ops_tests.rs (new)
// Keep existing sanitize test, and replace TTL arithmetic-only check
// with a behavior test that calls fetch_current_user_cached and asserts
// cache-hit / cache-miss behavior.

As per coding guidelines, "src/**/*.rs: Source files should be ≤ ~500 lines; split modules when growing to improve maintainability" and "**/*.{rs,ts,tsx}: Ship unit tests and coverage for behavior you are adding or changing before building additional features on top."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/app_state/ops.rs` around lines 492 - 518, Move the inline tests
out of ops.rs into a new tests module file (e.g., ops_tests.rs) to keep ops.rs
under ~500 lines; keep the existing sanitize_snapshot_user test but replace the
TTL-only assertion that checks Instant math with a behavioral test that
exercises fetch_current_user_cached using a CachedCurrentUser instance and
CURRENT_USER_REFRESH_TTL to assert cache-hit vs cache-miss behavior (e.g.,
prepare cached entry with fetched_at older/newer than CURRENT_USER_REFRESH_TTL
and call fetch_current_user_cached to validate it returns cached user when fresh
and triggers refresh when expired).

- release.yml: drop the leftover stash conflict markers around the
  updater env block so the workflow YAML parses again.
- HomeBanners: actually apply the tone-specific button classes so the
  danger variant renders coral instead of falling back to amber.
- HomeBanners.test: clear the openUrl mock between cases so each
  assertion only sees its own click.
- Conversations: drop the duplicate composerInteractionBlocked guard
  (already early-returned upstream).
- Conversations.test: cover the rustChat=false branch.
- Home: don't log the raw user object on every render — it carries
  ids/PII; remove the diagnostic.
- app_state/ops: split inline tests into ops_tests.rs and assert both
  the fresh and expired branches of the current-user cache TTL.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
src/openhuman/app_state/ops_tests.rs (1)

23-36: ⚠️ Potential issue | 🟡 Minor

TTL tests still bypass the production freshness predicate.

These checks validate Instant math directly, but they won’t fail if the predicate inside fetch_current_user_cached changes (for example, < to <=). Consider extracting a small helper (used by production code) and asserting against that helper in tests.

♻️ Minimal refactor to bind tests to production logic
diff --git a/src/openhuman/app_state/ops.rs b/src/openhuman/app_state/ops.rs
@@
+fn is_cached_current_user_fresh(entry: &CachedCurrentUser) -> bool {
+    entry.fetched_at.elapsed() < CURRENT_USER_REFRESH_TTL
+}
@@
-                && entry.fetched_at.elapsed() < CURRENT_USER_REFRESH_TTL
+                && is_cached_current_user_fresh(entry)
diff --git a/src/openhuman/app_state/ops_tests.rs b/src/openhuman/app_state/ops_tests.rs
@@
-    assert!(fresh.fetched_at.elapsed() < CURRENT_USER_REFRESH_TTL);
+    assert!(is_cached_current_user_fresh(&fresh));
@@
-    assert!(expired.fetched_at.elapsed() >= CURRENT_USER_REFRESH_TTL);
+    assert!(!is_cached_current_user_fresh(&expired));

As per coding guidelines, "**/*.{rs,ts,tsx}: Ship unit tests and coverage for behavior you are adding or changing before building additional features on top."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/app_state/ops_tests.rs` around lines 23 - 36, The tests
directly compare Instant math against CURRENT_USER_REFRESH_TTL which can diverge
from production logic; extract the production freshness predicate into a small
helper (e.g., fn is_cached_entry_fresh(fetched_at: Instant) -> bool) and use
that helper inside fetch_current_user_cached and in the tests (replace the
assert!(...elapsed() < CURRENT_USER_REFRESH_TTL) / >= checks with calls to
is_cached_entry_fresh and its negation), keeping references to make_cached_entry
and CURRENT_USER_REFRESH_TTL to locate the impacted code.
🧹 Nitpick comments (4)
.github/workflows/release.yml (2)

413-416: Remove unused updater env vars from this step.

BASE_URL, UPDATER_PUBLIC_KEY, UPDATER_ENDPOINT, and UPDATER_REPO are not consumed by scripts/prepareTauriConfig.js (it only reads WITH_UPDATER / KEYPAIR_ALIAS), so these values are dead here and can cause config-drift assumptions.

♻️ Proposed cleanup
       - name: Define Tauri configuration overrides
         id: config-overrides
@@
         uses: actions/github-script@v7
         env:
-          BASE_URL: ${{ needs.prepare-build.outputs.base_url }}
-          UPDATER_PUBLIC_KEY: ${{ secrets.UPDATER_PUBLIC_KEY || vars.UPDATER_PUBLIC_KEY }}
-          UPDATER_ENDPOINT: ${{ vars.UPDATER_ENDPOINT }}
-          UPDATER_REPO: tinyhumansai/openhuman
           WITH_UPDATER: "true"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/release.yml around lines 413 - 416, Remove the unused
updater environment variables from the workflow step by deleting BASE_URL,
UPDATER_PUBLIC_KEY, UPDATER_ENDPOINT, and UPDATER_REPO entries so the step only
provides the vars actually consumed by scripts/prepareTauriConfig.js
(WITH_UPDATER and KEYPAIR_ALIAS); update the environment block in the step to
only export required vars and verify no other steps reference those removed
names (BASE_URL, UPDATER_PUBLIC_KEY, UPDATER_ENDPOINT, UPDATER_REPO) to avoid
breaking dependencies.

529-529: Gate WITH_UPDATER by release mode instead of hardcoding true.

At Line 529 this also enables updater artifact mode for staging builds (release_enabled == 'false'). Making it conditional keeps staging behavior lean and aligned with release-only updater packaging.

🔧 Suggested change (apply in both env blocks: Line 417 and Line 529)
-          WITH_UPDATER: "true"
+          WITH_UPDATER: ${{ needs.prepare-build.outputs.release_enabled == 'true' && 'true' || 'false' }}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/release.yml at line 529, The WITH_UPDATER env is hardcoded
to "true" and should be gated by the release mode; update the two occurrences of
WITH_UPDATER in .github/workflows/release.yml to derive their value from the
release_enabled environment variable (use the workflow expression that evaluates
release_enabled == 'true') so that WITH_UPDATER is true only for release builds;
change both env blocks that currently set WITH_UPDATER: "true" to set
WITH_UPDATER based on release_enabled (refer to the WITH_UPDATER key and the
release_enabled env var when locating the spots to change).
app/src/pages/Home.tsx (1)

172-242: Consider removing the large commented-out “Next steps” block.

Keeping this much dormant JSX in the component makes future maintenance harder; better to delete it or move it behind a feature flag/ADR reference.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/src/pages/Home.tsx` around lines 172 - 242, Remove the large
commented-out "Next steps" JSX block in Home.tsx to reduce clutter; either
delete it entirely or extract it behind a feature flag or into a separate
component (e.g., NextSteps) and reference that from the Home component so the
commented code no longer sits in the Home.tsx source.
src/openhuman/app_state/ops.rs (1)

308-314: Consider de-duplicating user-sanitization logic across domains.

sanitize_snapshot_user duplicates the same null/empty-object filtering already present in src/openhuman/credentials/ops.rs (sanitize_stored_session_user). A shared helper would reduce drift risk if rules evolve.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/app_state/ops.rs` around lines 308 - 314, Both
sanitize_snapshot_user and sanitize_stored_session_user duplicate the same
filtering logic (treating Value::Null and empty Object as None); extract that
logic into a single shared helper (e.g., sanitize_user_value) and have both
functions call it. Update references in src/openhuman/app_state/ops.rs
(sanitize_snapshot_user) and src/openhuman/credentials/ops.rs
(sanitize_stored_session_user) to delegate to the new helper so behavior is
centralized and future changes only touch one function.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/src/pages/Home.tsx`:
- Around line 62-68: statusCopy's object lookup for socketStatus can return
undefined for unexpected states; update the computation of statusCopy (where
socketStatus is used) to provide a defensive fallback string (e.g., a generic
"Status unknown — please try again" message) when the lookup yields undefined —
either by adding a default key in the object or by using a nullish coalescing
fallback after the lookup (statusCopy = {...}[socketStatus] ?? 'fallback
message') to ensure the UI always shows a description.

---

Duplicate comments:
In `@src/openhuman/app_state/ops_tests.rs`:
- Around line 23-36: The tests directly compare Instant math against
CURRENT_USER_REFRESH_TTL which can diverge from production logic; extract the
production freshness predicate into a small helper (e.g., fn
is_cached_entry_fresh(fetched_at: Instant) -> bool) and use that helper inside
fetch_current_user_cached and in the tests (replace the assert!(...elapsed() <
CURRENT_USER_REFRESH_TTL) / >= checks with calls to is_cached_entry_fresh and
its negation), keeping references to make_cached_entry and
CURRENT_USER_REFRESH_TTL to locate the impacted code.

---

Nitpick comments:
In @.github/workflows/release.yml:
- Around line 413-416: Remove the unused updater environment variables from the
workflow step by deleting BASE_URL, UPDATER_PUBLIC_KEY, UPDATER_ENDPOINT, and
UPDATER_REPO entries so the step only provides the vars actually consumed by
scripts/prepareTauriConfig.js (WITH_UPDATER and KEYPAIR_ALIAS); update the
environment block in the step to only export required vars and verify no other
steps reference those removed names (BASE_URL, UPDATER_PUBLIC_KEY,
UPDATER_ENDPOINT, UPDATER_REPO) to avoid breaking dependencies.
- Line 529: The WITH_UPDATER env is hardcoded to "true" and should be gated by
the release mode; update the two occurrences of WITH_UPDATER in
.github/workflows/release.yml to derive their value from the release_enabled
environment variable (use the workflow expression that evaluates release_enabled
== 'true') so that WITH_UPDATER is true only for release builds; change both env
blocks that currently set WITH_UPDATER: "true" to set WITH_UPDATER based on
release_enabled (refer to the WITH_UPDATER key and the release_enabled env var
when locating the spots to change).

In `@app/src/pages/Home.tsx`:
- Around line 172-242: Remove the large commented-out "Next steps" JSX block in
Home.tsx to reduce clutter; either delete it entirely or extract it behind a
feature flag or into a separate component (e.g., NextSteps) and reference that
from the Home component so the commented code no longer sits in the Home.tsx
source.

In `@src/openhuman/app_state/ops.rs`:
- Around line 308-314: Both sanitize_snapshot_user and
sanitize_stored_session_user duplicate the same filtering logic (treating
Value::Null and empty Object as None); extract that logic into a single shared
helper (e.g., sanitize_user_value) and have both functions call it. Update
references in src/openhuman/app_state/ops.rs (sanitize_snapshot_user) and
src/openhuman/credentials/ops.rs (sanitize_stored_session_user) to delegate to
the new helper so behavior is centralized and future changes only touch one
function.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f308e431-9c86-4ac2-a155-62aef696c4f8

📥 Commits

Reviewing files that changed from the base of the PR and between 7d35c9f and 39d3285.

📒 Files selected for processing (8)
  • .github/workflows/release.yml
  • app/src/components/home/HomeBanners.tsx
  • app/src/components/home/__tests__/HomeBanners.test.tsx
  • app/src/pages/Conversations.tsx
  • app/src/pages/Home.tsx
  • app/src/pages/__tests__/Conversations.test.tsx
  • src/openhuman/app_state/ops.rs
  • src/openhuman/app_state/ops_tests.rs
✅ Files skipped from review due to trivial changes (2)
  • app/src/components/home/tests/HomeBanners.test.tsx
  • app/src/components/home/HomeBanners.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
  • app/src/pages/tests/Conversations.test.tsx
  • app/src/pages/Conversations.tsx

Comment thread app/src/pages/Home.tsx
Comment on lines 62 to 68
const statusCopy = {
connected:
'Your device is connected to OpenHuman AI. Keep the app running to keep the connection alive — message your assistant with the button below.',
connecting: 'Connecting to OpenHuman AI. Hang tight, this usually takes a second.',
'Your device is connected. Keep the app running to keep the connection alive. Message your assistant with the button below.',
connecting: 'Connecting. Hang tight, this usually takes a second.',
disconnected:
'Your device is offline right now. Check your network or restart the app to reconnect.',
}[socketStatus];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add a defensive fallback for unexpected socket states.

Right now, an unrecognized socketStatus would render an empty description. A small fallback keeps UX stable if new states are introduced.

💡 Suggested adjustment
-  const statusCopy = {
+  const statusCopy = {
     connected:
       'Your device is connected. Keep the app running to keep the connection alive. Message your assistant with the button below.',
     connecting: 'Connecting. Hang tight, this usually takes a second.',
     disconnected:
       'Your device is offline right now. Check your network or restart the app to reconnect.',
-  }[socketStatus];
+  }[socketStatus] ?? 'Checking connection status...';
📝 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
const statusCopy = {
connected:
'Your device is connected to OpenHuman AI. Keep the app running to keep the connection alive — message your assistant with the button below.',
connecting: 'Connecting to OpenHuman AI. Hang tight, this usually takes a second.',
'Your device is connected. Keep the app running to keep the connection alive. Message your assistant with the button below.',
connecting: 'Connecting. Hang tight, this usually takes a second.',
disconnected:
'Your device is offline right now. Check your network or restart the app to reconnect.',
}[socketStatus];
const statusCopy = {
connected:
'Your device is connected. Keep the app running to keep the connection alive. Message your assistant with the button below.',
connecting: 'Connecting. Hang tight, this usually takes a second.',
disconnected:
'Your device is offline right now. Check your network or restart the app to reconnect.',
}[socketStatus] ?? 'Checking connection status...';
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/src/pages/Home.tsx` around lines 62 - 68, statusCopy's object lookup for
socketStatus can return undefined for unexpected states; update the computation
of statusCopy (where socketStatus is used) to provide a defensive fallback
string (e.g., a generic "Status unknown — please try again" message) when the
lookup yields undefined — either by adding a default key in the object or by
using a nullish coalescing fallback after the lookup (statusCopy =
{...}[socketStatus] ?? 'fallback message') to ensure the UI always shows a
description.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant