Skip to content

ci: add llm reviewer - #236

Closed
vitonsky wants to merge 10 commits into
masterfrom
235-add-llm-reviewer
Closed

vitonsky wants to merge 10 commits into
masterfrom
235-add-llm-reviewer

Conversation

@vitonsky

@vitonsky vitonsky commented Mar 3, 2026 •

Copy link
Copy Markdown
Member

Closes #235

Summary by CodeRabbit

Here are the concise release notes for the pull request:

  • New Feature: Introduced AI-assisted code review automation using GitHub Actions to improve code quality and consistency.
  • Refactor: Consolidated workspace/profile initialization and adjusted imports for better organization and maintainability.

These changes aim to enhance the development experience and overall code health without affecting end-user functionality.

Summary by CodeRabbit

Release Notes

  • New Features

    • Redesigned vault and profile management experience with streamlined selection and creation flows
    • Improved workspace initialization with readiness indicators
  • Improvements

    • Enhanced handling of workspace state persistence and profile opening for both encrypted and unencrypted vaults
    • Better state management for active workspaces
  • Chores

    • Added automated code review workflow for pull requests

@vitonsky vitonsky linked an issue Mar 3, 2026 that may be closed by this pull request
@github-actions

github-actions Bot commented Mar 3, 2026 •

Copy link
Copy Markdown

Image description CodeRabbit

Walkthrough

This pull request introduces a new GitHub Actions workflow for automated AI-assisted code review and refactors the initialization of workspace/profile components, streamlining imports and logic.

Changes

File(s) Summary
.github/workflows/code-review.yml Adds a new GitHub Actions workflow for AI-assisted PR review using coderabbitai/ai-pr-reviewer.
src/features/App/* Consolidates changes to workspace/profile initialization and adjusts imports across Profile, Workspace, and ProfileLogin components.

"A code review so fine,
With AI assistance that shines!
Workflow automation in place,
Code quality takes a happy face 🐰💻"


Uplevel your code reviews with CodeRabbit Pro

CodeRabbit Pro

If you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, CodeRabbit Pro is free for open source projects.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Image description CodeRabbit

Commits Files that changed from the base of the PR and between 3039225 and 6bd613d commits.
Files selected (17)
  • .github/workflows/code-review.yml (1)
  • src/core/features/notes/controller/NotesController.test.ts (1)
  • src/features/App/Profile/index.tsx (5)
  • src/features/App/Profile/services/useVaultConfigSync.tsx (2)
  • src/features/App/ProfileLoginForm/index.tsx (1)
  • src/features/App/VaultScreenManager.tsx (1)
  • src/features/App/Workspace/index.tsx (5)
  • src/features/App/Workspace/services/WorkspaceServices.tsx (1)
  • src/features/App/Workspace/services/useWorkspaceConfigSync.tsx (1)
  • src/features/App/Workspace/useWorkspaceInitialization.tsx (1)
  • src/features/App/Workspace/useWorkspaceState.tsx (1)
  • src/features/App/WorkspaceManager/index.tsx (1)
  • src/features/App/index.tsx (3)
  • src/features/App/useProfilesList.ts (1)
  • src/features/App/useRecentProfile.ts (1)
  • src/features/App/useVaultOpener.tsx (1)
  • src/state/redux/profiles/profiles.ts (4)
Files not summarized due to errors (12)
  • src/features/App/Profile/services/useVaultConfigSync.tsx (nothing obtained from openai)
  • src/features/App/VaultScreenManager.tsx (nothing obtained from openai)
  • src/features/App/Workspace/services/useWorkspaceConfigSync.tsx (nothing obtained from openai)
  • src/core/features/notes/controller/NotesController.test.ts (nothing obtained from openai)
  • src/features/App/Workspace/services/WorkspaceServices.tsx (nothing obtained from openai)
  • src/features/App/Workspace/useWorkspaceState.tsx (nothing obtained from openai)
  • src/features/App/Workspace/useWorkspaceInitialization.tsx (nothing obtained from openai)
  • src/features/App/WorkspaceManager/index.tsx (nothing obtained from openai)
  • src/features/App/useVaultOpener.tsx (nothing obtained from openai)
  • src/features/App/index.tsx (nothing obtained from openai)
  • src/features/App/useRecentProfile.ts (nothing obtained from openai)
  • src/state/redux/profiles/profiles.ts (nothing obtained from openai)
Files not reviewed due to errors (17)
  • src/features/App/ProfileLoginForm/index.tsx (no response)
  • src/features/App/Profile/index.tsx (no response)
  • src/features/App/Profile/services/useVaultConfigSync.tsx (no response)
  • .github/workflows/code-review.yml (no response)
  • src/features/App/VaultScreenManager.tsx (no response)
  • src/core/features/notes/controller/NotesController.test.ts (no response)
  • src/features/App/Workspace/index.tsx (no response)
  • src/features/App/Workspace/services/WorkspaceServices.tsx (no response)
  • src/features/App/Workspace/services/useWorkspaceConfigSync.tsx (no response)
  • src/features/App/Workspace/useWorkspaceState.tsx (no response)
  • src/features/App/WorkspaceManager/index.tsx (no response)
  • src/features/App/index.tsx (no response)
  • src/features/App/useVaultOpener.tsx (no response)
  • src/features/App/useProfilesList.ts (no response)
  • src/features/App/Workspace/useWorkspaceInitialization.tsx (no response)
  • src/state/redux/profiles/profiles.ts (no response)
  • src/features/App/useRecentProfile.ts (no response)
Review comments generated (0)
  • Review: 0
  • LGTM: 0

Tips

Chat with Image description CodeRabbit Bot (@coderabbitai)

  • Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
  • Invite the bot into a review comment chain by tagging @coderabbitai in a reply.

Code suggestions

  • The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
  • You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.

Pausing incremental reviews

  • Add @coderabbitai: ignore anywhere in the PR description to pause further reviews from the bot.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Image description CodeRabbit

Commits Files that changed from the base of the PR and between 6bd613d and 9c7eca3 commits.
Files selected (1)
  • .github/workflows/code-review.yml (1)
Files not reviewed due to errors (1)
  • .github/workflows/code-review.yml (no response)
Review comments generated (0)
  • Review: 0
  • LGTM: 0

Tips

Chat with Image description CodeRabbit Bot (@coderabbitai)

  • Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
  • Invite the bot into a review comment chain by tagging @coderabbitai in a reply.

Code suggestions

  • The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
  • You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.

Pausing incremental reviews

  • Add @coderabbitai: ignore anywhere in the PR description to pause further reviews from the bot.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Image description CodeRabbit

Commits Files that changed from the base of the PR and between 9c7eca3 and c07a80b commits.
Files selected (1)
  • .github/workflows/code-review.yml (1)
Review comments generated (3)
  • Review: 3
  • LGTM: 0

Tips

Chat with Image description CodeRabbit Bot (@coderabbitai)

  • Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
  • Invite the bot into a review comment chain by tagging @coderabbitai in a reply.

Code suggestions

  • The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
  • You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.

Pausing incremental reviews

  • Add @coderabbitai: ignore anywhere in the PR description to pause further reviews from the bot.

Comment on lines +7 to +10
on:
pull_request:
pull_request_target:
types: [opened, synchronize, reopened]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Image description CodeRabbit

The workflow has both pull_request (line 8, no type filter) and pull_request_target (line 9, with type filters) triggers. This is problematic:

  1. pull_request will trigger on all PR events (opened, synchronize, reopened, etc.) without restriction
  2. pull_request_target runs in the context of the base repository with access to secrets — using it alongside pull_request for the same job is redundant and potentially dangerous, as pull_request_target can expose secrets to code from forked PRs

If this repo doesn't need to handle forks, use only pull_request. If fork support is needed, use only pull_request_target (with caution).

-on:
-  pull_request:
-  pull_request_target:
-    types: [opened, synchronize, reopened]
-  pull_request_review_comment:
-    types: [created]
+on:
+  pull_request:
+    types: [opened, synchronize, reopened]
+  pull_request_review_comment:
+    types: [created]

review:
runs-on: ubuntu-latest
steps:
- uses: coderabbitai/ai-pr-reviewer@latest

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Image description CodeRabbit

Pinning to @latest means any future breaking changes or compromised releases of coderabbitai/ai-pr-reviewer will automatically affect this workflow. Pin to a specific commit SHA for supply-chain security.

-      - uses: coderabbitai/ai-pr-reviewer@latest
+      - uses: coderabbitai/ai-pr-reviewer@<commit-sha>

Comment thread .github/workflows/code-review.yml Outdated
Comment on lines +33 to +34
openai_light_model: "openai/gpt-5-nano"
openai_heavy_model: "anthropic/claude-sonnet-4.6"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Image description CodeRabbit

The model names openai/gpt-5-nano and anthropic/claude-sonnet-4.6 do not appear to be valid/existing model identifiers as of the knowledge cutoff. Verify these are correct OpenRouter model slugs (e.g., openai/gpt-4o-mini for light, anthropic/claude-3-5-sonnet for heavy) to avoid silent failures where the reviewer falls back to defaults or errors out.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Image description CodeRabbit

Commits Files that changed from the base of the PR and between c07a80b and 50e9a3a commits.
Files selected (1)
  • .github/workflows/code-review.yml (1)
Files not summarized due to errors (1)
  • .github/workflows/code-review.yml (nothing obtained from openai)
Review comments generated (4)
  • Review: 2
  • LGTM: 2

Tips

Chat with Image description CodeRabbit Bot (@coderabbitai)

  • Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
  • Invite the bot into a review comment chain by tagging @coderabbitai in a reply.

Code suggestions

  • The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
  • You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.

Pausing incremental reviews

  • Add @coderabbitai: ignore anywhere in the PR description to pause further reviews from the bot.

Comment on lines +14 to +18
concurrency:
group: ${{ github.repository }}-${{ github.event.number || github.head_ref ||
github.sha }}-${{ github.workflow }}-${{ github.event_name ==
'pull_request_review_comment' && 'pr_comment' || 'pr' }}
cancel-in-progress: ${{ github.event_name != 'pull_request_review_comment' }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Image description CodeRabbit

The concurrency group name is quite complex and may lead to issues with workflow naming. Consider simplifying the group name to avoid potential conflicts.

-  concurrency:
-    group: ${{ github.repository }}-${{ github.event.number || github.head_ref ||
-      github.sha }}-${{ github.workflow }}-${{ github.event_name ==
-      'pull_request_review_comment' && 'pr_comment' || 'pr' }}
+  concurrency:
+    group: ${{ github.repository }}-${{ github.workflow }}-${{ github.event_name }}

Comment on lines +27 to +28
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
with:

@github-actions github-actions Bot Mar 3, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Image description CodeRabbit

It's a good practice to validate the secrets before using them. You can add a step to check if the secrets are set before running the ai-pr-reviewer action.

+  - name: Validate secrets
+    run: |
+      if [ -z "$GITHUB_TOKEN" ]; then
+        echo "GITHUB_TOKEN is not set"
+        exit 1
+      fi
+      if [ -z "$OPENAI_API_KEY" ]; then
+        echo "OPENAI_API_KEY is not set"
+        exit 1
+      fi

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Review all files

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Image description CodeRabbit

@vitonsky:

Here is a thorough review of the .github/workflows/code-review.yml file:

Security:
The workflow uses GITHUB_TOKEN and OPENAI_API_KEY secrets, which is good practice. However, as suggested by CodeRabbit, it's essential to validate these secrets before using them. The added step to check if the secrets are set before running the ai-pr-reviewer action is a great security measure.

Performance:
The workflow runs on ubuntu-latest, which is a good choice for most use cases. However, it's worth considering using a more lightweight environment if the ai-pr-reviewer action doesn't require a full-fledged Ubuntu environment.

Logic:
The concurrency group and cancel-in-progress logic seem correct, ensuring that only one review job runs at a time and canceling in-progress jobs when a new review comment is created.

Error Handling:
The workflow could benefit from more robust error handling. For example, you could add a try-except block around the ai-pr-reviewer action to catch and handle any errors that might occur during the review process.

Maintainability:
The workflow is well-structured and easy to follow. However, it would be beneficial to add comments explaining the purpose of each step and the logic behind the concurrency and cancel-in-progress settings.

Best Practices:
The workflow follows the DRY (Don't Repeat Yourself) principle by using variables for the secrets and the with block for the ai-pr-reviewer action. However, it's worth considering extracting the review job into a separate reusable workflow file to improve modularity.

Here's an updated version of the workflow incorporating the suggested changes:

name: Code Review

permissions:
  contents: read
  pull-requests: write

on:
  pull_request:
  pull_request_target:
    types: [opened, synchronize, reopened]
  pull_request_review_comment:
    types: [created]

concurrency:
  group: ${{ github.repository }}-${{ github.event.number || github.head_ref || github.sha }}-${{ github.workflow }}-${{ github.event_name == 'pull_request_review_comment' && 'pr_comment' || 'pr' }}
  cancel-in-progress: ${{ github.event_name != 'pull_request_review_comment' }}

jobs:
  review:
    runs-on: ubuntu-latest
    steps:
      - name: Validate secrets
        run: |
          if [ -z "$GITHUB_TOKEN" ]; then
            echo "GITHUB_TOKEN is not set"
            exit 1
          fi
          if [ -z "$OPENAI_API_KEY" ]; then
            echo "OPENAI_API_KEY is not set"
            exit 1
          fi
      - uses: coderabbitai/ai-pr-reviewer@latest
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
          OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
        with:
          debug: true
          review_simple_changes: false
          review_comment_lgtm: false
          openai_base_url: "https://openrouter.ai/api/v1"
          openai_light_model: "openai/gpt-5-nano"
          openai_heavy_model: "meta-llama/llama-3-70b-instruct"

Let me know if you have any further questions or concerns!

@vitonsky

vitonsky commented Mar 3, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review all files in this pr

1 similar comment
@vitonsky

vitonsky commented Mar 3, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review all files in this pr

@coderabbitai

coderabbitai Bot commented Mar 3, 2026 •

Copy link
Copy Markdown
Contributor

Warning

CodeRabbit GitHub Action detected

The repository is using both CodeRabbit Pro and CodeRabbit Open Source (via GitHub Actions), which is not recommended as it may lead to duplicate comments and extra noise. Please remove the CodeRabbit GitHub Action.

📝 Walkthrough

Walkthrough

This pull request implements a vault-centric architecture for profile and workspace management, introduces an AI code review GitHub Actions workflow, and refactors initialization logic into dedicated hooks. Core changes include removing WorkspaceManager, introducing VaultScreenManager, creating useVaultOpener for profile opening flows, and splitting workspace configuration synchronization into separate hooks.

Changes

Cohort / File(s) Summary
CI/CD Integration
.github/workflows/code-review.yml
Added GitHub Actions workflow for AI-powered code review with OpenAI integration, configured to run on PR events with debug and model specifications.
Redux State Management
src/state/redux/profiles/profiles.ts
Extended WorkspaceData type with isWorkspaceReady flag, added setIsWorkspaceReady reducer action, and introduced selectIsActiveWorkspaceReady selector for workspace readiness tracking.
Vault Management Core
src/features/App/VaultScreenManager.tsx, src/features/App/useVaultOpener.tsx
New VaultScreenManager component orchestrates profile login, creation, and selection flows; new useVaultOpener hook manages vault opening with encryption/password handling and auto-opening of unencrypted profiles.
Workspace Initialization & Sync
src/features/App/Workspace/useWorkspaceInitialization.tsx, src/features/App/Workspace/useWorkspaceConfigSync.tsx, src/features/App/Workspace/useWorkspaceState.tsx, src/features/App/Workspace/services/WorkspaceServices.tsx
New hooks implement workspace lifecycle management: useWorkspaceInitialization handles setup/teardown and state restoration, useWorkspaceConfigSync persists workspace config changes, useWorkspaceState provides persistent state file backing, and WorkspaceServices invokes the new sync hook.
Profile & App Component Updates
src/features/App/index.tsx, src/features/App/Profile/index.tsx, src/features/App/Workspace/index.tsx
Refactored App to use vault-centric flow with VaultScreenManager and useVaultOpener; Profile now gates workspace rendering with isActiveWorkspaceReady selector; Workspace simplified to use useWorkspaceInitialization hook instead of inline effects.
Profile Config Simplification
src/features/App/Profile/services/useVaultConfigSync.tsx
Removed multi-workspace config synchronization, retained only profile-level config sync via ProfileConfigScheme and selectProfile selector.
Profile Management Utilities
src/features/App/useProfilesList.ts, src/features/App/useRecentProfile.ts
Updated import path for NewProfile; introduced new RecentProfile type to structure recent profile state with isLoaded and profileId fields.
Import Path Updates
src/features/App/ProfileLoginForm/index.tsx
Changed OnPickProfile import source from parent directory to useVaultOpener module for consistency.
Component Removal & Test Addition
src/features/App/WorkspaceManager/index.tsx, src/core/features/notes/controller/NotesController.test.ts
Removed WorkspaceManager component (209 lines) replacing its functionality with VaultScreenManager; added test verifying getById ignores non-existent note IDs.

Sequence Diagram

sequenceDiagram
    participant User
    participant App
    participant VaultScreenManager
    participant useVaultOpener
    participant ProfileCreator
    participant ProfileLoginForm
    participant Redux
    participant Workspace

    User->>App: Mounts App component
    App->>useVaultOpener: Initialize vault opener
    useVaultOpener->>Redux: Check recent profile & profiles list
    
    alt Recent unencrypted profile exists
        useVaultOpener->>useVaultOpener: Auto-open profile
        useVaultOpener->>Redux: Set current profile ID
        useVaultOpener->>Workspace: Initialize workspace
        Workspace->>Redux: Load workspace state
    else No profiles exist
        App->>VaultScreenManager: Show creation flow
        User->>VaultScreenManager: Create new profile
        VaultScreenManager->>ProfileCreator: Render creation form
        ProfileCreator->>Redux: createProfile action
        ProfileCreator->>useVaultOpener: onCreateProfile callback
        useVaultOpener->>Workspace: Initialize new workspace
    else Profiles exist, none active
        App->>VaultScreenManager: Show profile selection
        User->>VaultScreenManager: Select profile
        VaultScreenManager->>useVaultOpener: onChooseProfile call
        
        alt Profile unencrypted
            useVaultOpener->>Workspace: Open directly
        else Profile encrypted
            VaultScreenManager->>ProfileLoginForm: Show login form
            User->>ProfileLoginForm: Enter password
            ProfileLoginForm->>useVaultOpener: onOpenProfile with password
            useVaultOpener->>Workspace: Open with validated password
        end
    end

    Workspace->>Workspace: useWorkspaceInitialization
    Workspace->>Redux: setIsWorkspaceReady(true)
    Redux->>App: Update workspace ready state
    App->>User: Render workspace UI
Loading
sequenceDiagram
    participant Workspace as useWorkspaceInitialization
    participant FileController as FileController / StateFile
    participant Redux
    participant TagsRegistry

    Workspace->>FileController: Load workspace config from workspaces/{id}/config.json
    FileController-->>Workspace: Config loaded
    Workspace->>Redux: setWorkspaceNoteTemplateConfig(title, tags)

    Workspace->>FileController: Load workspace state from workspaces/{id}/state.json
    FileController-->>Workspace: State loaded (openedNoteIds, activeNoteId, etc.)
    Workspace->>Redux: Initialize workspace state (tags, search, view, etc.)

    Workspace->>TagsRegistry: Subscribe to tag registry changes
    
    loop On tag registry changes
        TagsRegistry-->>Workspace: Emit new tags
        Workspace->>Redux: Update workspace tags
    end

    Note over Workspace,Redux: On unmount
    Workspace->>Redux: setIsWorkspaceReady(false)
    Workspace->>Redux: Clear active note, opened notes, note IDs
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 Hops with glee through vaults of code,
Where workspaces now safely load,
Profile picks and configs sync,
Faster initialization in a blink,
The vault's refactored, bright and bold! ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR fulfills the objective in issue #235 by integrating an LLM reviewer via GitHub Actions workflow (.github/workflows/code-review.yml using coderabbitai/ai-pr-reviewer action). However, the PR also includes numerous unrelated refactorings to workspace/profile initialization and imports that were not part of the stated objective to add an LLM reviewer. The workspace/profile initialization refactorings and import reorganizations are out of scope for issue #235 and should be separated into a dedicated PR for clarity and maintainability.
Out of Scope Changes check ⚠️ Warning The PR includes substantial out-of-scope changes unrelated to adding an LLM reviewer: workspace readiness logic, profile/workspace config synchronization refactoring, new hooks (useWorkspaceInitialization, useWorkspaceConfigSync, useWorkspaceState, useVaultOpener), removal of WorkspaceManager, and significant changes to App component flow. Extract workspace and profile initialization refactorings (src/features/App/*, src/state/redux/profiles/profiles.ts) into a separate PR. Keep the LLM reviewer CI addition (.github/workflows/code-review.yml) isolated for focused review.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'ci: add llm reviewer' directly and clearly describes the main change—adding an LLM-based reviewer to CI workflows via the new .github/workflows/code-review.yml file.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 235-add-llm-reviewer

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

@vitonsky

vitonsky commented Mar 3, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Mar 3, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@vitonsky

vitonsky commented Mar 3, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai help

@coderabbitai

coderabbitai Bot commented Mar 3, 2026

Copy link
Copy Markdown
Contributor

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

CodeRabbit commands

These commands are invoked using PR/Issue comments.

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai evaluate custom pre-merge check --instructions <custom-checks-instructions> --name <custom-checks-title> [--mode <error|warning>] to validate the custom pre-merge checks instructions. Defaults to error when --mode is omitted.
    • @coderabbitai ignore pre-merge checks to override pre-merge checks and get an approval on PR.
    • @coderabbitai run pre-merge checks to run pre-merge checks on the pull request.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve to resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai generate configuration to create a PR that adds the current resolved configuration as .coderabbit.yaml (or show it if already present).
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit configuration file (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, support, documentation and community

  • Visit our status page to check the current availability of CodeRabbit.
  • Create a ticket on our support page for assistance with any issues or questions.
  • Visit our documentation site for detailed information on how to use CodeRabbit.
  • Join our Discord community to connect with other users and get help from the community.
  • Follow us on X/Twitter for updates and announcements.

@vitonsky

vitonsky commented Mar 3, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Mar 3, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Full review triggered.

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 5

♻️ Duplicate comments (2)
.github/workflows/code-review.yml (2)

7-12: ⚠️ Potential issue | 🟠 Major

Use a single PR trigger strategy to avoid duplicate review runs.

Line [8] and Line [9]-[10] can both fire for the same PR updates, causing duplicate runs/comments and unnecessary token spend. Keep only one trigger path (typically pull_request_target for external-contributor reviews with secrets, or pull_request if fork-secret access is not needed).

Suggested trigger cleanup
 on:
-  pull_request:
   pull_request_target:
     types: [opened, synchronize, reopened]
   pull_request_review_comment:
     types: [created]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/code-review.yml around lines 7 - 12, The workflow
currently defines both pull_request and pull_request_target under the on:
triggers which can cause duplicate runs; pick and keep only one trigger path
(either remove the pull_request entry or remove pull_request_target) depending
on whether you need secrets for external contributors, e.g., remove the
pull_request key if you want pull_request_target semantics, or remove
pull_request_target if you prefer standard pull_request behavior, and update the
top-level on: block to contain just the chosen trigger and its types.

24-24: ⚠️ Potential issue | 🟠 Major

Pin the action to an immutable commit SHA instead of @latest.

Line [24] currently uses a mutable tag, which weakens supply-chain safety and reproducibility.

Suggested hard pin
-      - uses: coderabbitai/ai-pr-reviewer@latest
+      - uses: coderabbitai/ai-pr-reviewer@<full-commit-sha>
#!/bin/bash
# Verify mutable action refs in workflow files
rg -nP '^\s*-\s*uses:\s*[^@]+@latest\b|^\s*uses:\s*[^@]+@latest\b'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/code-review.yml at line 24, Replace the mutable action ref
"uses: coderabbitai/ai-pr-reviewer@latest" with an immutable commit SHA for the
coderabbitai/ai-pr-reviewer action (e.g., "uses:
coderabbitai/ai-pr-reviewer@<commit-sha>"); locate the line containing the exact
string "uses: coderabbitai/ai-pr-reviewer@latest" and update it to reference a
specific commit SHA (or verified tag) to ensure reproducible, supply-chain-safe
workflow runs.
🧹 Nitpick comments (2)
src/core/features/notes/controller/NotesController.test.ts (1)

396-411: Strengthen this test to verify exact IDs, not only result length.

Right now, this can still pass if getById returns wrong/duplicate notes with the same count. Assert IDs explicitly and avoid the hardcoded 10.

Proposed test tightening
 test('method getById returns all found notes and ignores missing ones', async () => {
 	const db = await openDatabase(dbFile);
 	const registry = new NotesController(db, FAKE_WORKSPACE_ID);

 	const notesId = await registry
 		.get({ limit: 10 })
 		.then((notes) => notes.map((note) => note.id));

 	const notExistingNote = getUUID();

-	await expect(
-		registry.getById([...notesId, notExistingNote]),
-	).resolves.toHaveLength(10);
+	const result = await registry.getById([...notesId, notExistingNote]);
+	expect(result).toHaveLength(notesId.length);
+	expect(result.map((note) => note.id)).toEqual(notesId);

 	await db.close();
 });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/features/notes/controller/NotesController.test.ts` around lines 396
- 411, The test for NotesController.getById should assert the actual returned
IDs instead of only length: call registry.get({ limit: 10 }) to collect notesId,
add a non-existent id via getUUID(), then call registry.getById([...notesId,
notExistingNote]) and assert that the resolved array contains exactly the
original notesId (order-insensitive or exact order depending on expected
behavior) and does not include notExistingNote; replace the hardcoded 10 length
check with notesId.length and close the db as before (references:
NotesController, registry.get, registry.getById, getUUID, notesId, db.close).
src/features/App/useRecentProfile.ts (1)

9-13: Consider renaming the state variable for clarity.

The state variable profileId holds a RecentProfile object (with isLoaded and profileId fields), but the name suggests it's just a string ID. This can be confusing.

✏️ Suggested rename
 export const useRecentProfile = (config: ConfigStorage) => {
-	const [profileId, setProfileId] = useState<RecentProfile>({
+	const [recentProfile, setRecentProfile] = useState<RecentProfile>({
 		isLoaded: false,
 		profileId: null,
 	});

 	// Restore
 	const isRestoredRef = useRef(false);
 	useEffect(() => {
 		if (isRestoredRef.current) return;

 		config.get('activeProfile').then((activeProfile) => {
 			isRestoredRef.current = true;
-			setProfileId({
+			setRecentProfile({
 				isLoaded: true,
 				profileId: activeProfile,
 			});
 		});
 	}, [config]);

-	return profileId;
+	return recentProfile;
 };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/features/App/useRecentProfile.ts` around lines 9 - 13, The state variable
profileId in useRecentProfile actually stores a RecentProfile object (shape {
isLoaded, profileId }), so rename it to recentProfile (and rename setProfileId
to setRecentProfile) to reflect its structure; update all references within
useRecentProfile and any exports or consumers to use the new identifiers (e.g.,
change usages of profileId.profileId to recentProfile.profileId) and ensure type
annotations still use RecentProfile and tests/lints are updated accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/features/App/Profile/index.tsx`:
- Around line 139-141: The selector factory selectIsActiveWorkspaceReady({
profileId }) is being recreated on every render, causing extra selector
instances and potential re-renders; wrap the selector creation in useMemo and
pass that memoized selector into useAppSelector (e.g. const selector =
useMemo(() => selectIsActiveWorkspaceReady({ profileId }), [profileId]); const
isActiveWorkspaceReady = useAppSelector(selector)) so the selector identity
remains stable across renders for the same profileId.

In `@src/features/App/useVaultOpener.tsx`:
- Around line 37-60: The async profile-opening callback can leave
isProfileOpening true on early returns or if profiles.openProfile throws; wrap
the entire handler logic in a try/finally (or ensure every early return calls
setIsProfileOpening(false)) so setIsProfileOpening(false) always runs.
Concretely, in the async callback (the onOpenProfile handler) move
setIsProfileOpening(true) to the top, put all logic (including the no-encryption
branch, password validation, and the try/catch for profiles.openProfile) inside
a try block, and call setIsProfileOpening(false) in a finally block; keep using
profiles.openProfile and maintain the same returned status objects.

In `@src/features/App/VaultScreenManager.tsx`:
- Around line 63-68: The current onCreateProfile callback calls
profiles.createProfile(...).then(newProfile => onOpenProfile(newProfile,
profile.password ?? undefined).then(console.warn)) which blindly logs results;
change it to await or chain properly and handle success and errors from
onOpenProfile: call onOpenProfile(newProfile, profile.password ?? undefined) and
then on success proceed (e.g., resolve or navigate) and on failure catch the
error and surface it to the creator form (or call an error handler) instead of
console.warn. Update the handler (the onCreateProfile closure) to use try/catch
or .then(...).catch(...) around both profiles.createProfile and onOpenProfile so
failures are handled and propagated appropriately.

In `@src/features/App/Workspace/services/useWorkspaceConfigSync.tsx`:
- Around line 39-41: The call to workspaceConfig.set(config) currently only
attaches a .then() so rejections are unhandled; update the code where
workspaceConfig.set(config) is invoked (the workspaceConfig.set call in
useWorkspaceConfigSync) to handle failures by either awaiting the promise inside
an async function with try/catch or by appending a .catch(...) to the promise
chain, and in the catch block log the error with processLogger or console.error
and optionally surface or retry the failure so persistence errors are not
swallowed.

In `@src/features/App/Workspace/useWorkspaceInitialization.tsx`:
- Around line 22-44: The effect launches async initializers (e.g., initConfig in
useWorkspaceInitialization.tsx and any other async initializers called from
useEffect in this file) without rejection handling, which can cause unhandled
promise rejections; fix by adding error handling inside each async initializer
(wrap the body of initConfig and other async functions in try/catch and
handle/log the error or dispatch an error action) and ensure the call site does
not leave a raw promise unhandled (either await the async call inside the effect
using an IIFE or call initConfig().catch(...)). Reference initConfig and the
useEffect that invokes it when applying the try/catch/.catch fix.

---

Duplicate comments:
In @.github/workflows/code-review.yml:
- Around line 7-12: The workflow currently defines both pull_request and
pull_request_target under the on: triggers which can cause duplicate runs; pick
and keep only one trigger path (either remove the pull_request entry or remove
pull_request_target) depending on whether you need secrets for external
contributors, e.g., remove the pull_request key if you want pull_request_target
semantics, or remove pull_request_target if you prefer standard pull_request
behavior, and update the top-level on: block to contain just the chosen trigger
and its types.
- Line 24: Replace the mutable action ref "uses:
coderabbitai/ai-pr-reviewer@latest" with an immutable commit SHA for the
coderabbitai/ai-pr-reviewer action (e.g., "uses:
coderabbitai/ai-pr-reviewer@<commit-sha>"); locate the line containing the exact
string "uses: coderabbitai/ai-pr-reviewer@latest" and update it to reference a
specific commit SHA (or verified tag) to ensure reproducible, supply-chain-safe
workflow runs.

---

Nitpick comments:
In `@src/core/features/notes/controller/NotesController.test.ts`:
- Around line 396-411: The test for NotesController.getById should assert the
actual returned IDs instead of only length: call registry.get({ limit: 10 }) to
collect notesId, add a non-existent id via getUUID(), then call
registry.getById([...notesId, notExistingNote]) and assert that the resolved
array contains exactly the original notesId (order-insensitive or exact order
depending on expected behavior) and does not include notExistingNote; replace
the hardcoded 10 length check with notesId.length and close the db as before
(references: NotesController, registry.get, registry.getById, getUUID, notesId,
db.close).

In `@src/features/App/useRecentProfile.ts`:
- Around line 9-13: The state variable profileId in useRecentProfile actually
stores a RecentProfile object (shape { isLoaded, profileId }), so rename it to
recentProfile (and rename setProfileId to setRecentProfile) to reflect its
structure; update all references within useRecentProfile and any exports or
consumers to use the new identifiers (e.g., change usages of profileId.profileId
to recentProfile.profileId) and ensure type annotations still use RecentProfile
and tests/lints are updated accordingly.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3039225 and 50e9a3a.

📒 Files selected for processing (19)
  • .github/workflows/code-review.yml
  • src/core/features/notes/controller/NotesController.test.ts
  • src/features/App/Profile/index.tsx
  • src/features/App/Profile/services/useVaultConfigSync.tsx
  • src/features/App/ProfileCreator/index.tsx
  • src/features/App/ProfileLoginForm/index.tsx
  • src/features/App/ProfilesForm.tsx
  • src/features/App/VaultScreenManager.tsx
  • src/features/App/Workspace/index.tsx
  • src/features/App/Workspace/services/WorkspaceServices.tsx
  • src/features/App/Workspace/services/useWorkspaceConfigSync.tsx
  • src/features/App/Workspace/useWorkspaceInitialization.tsx
  • src/features/App/Workspace/useWorkspaceState.tsx
  • src/features/App/WorkspaceManager/index.tsx
  • src/features/App/index.tsx
  • src/features/App/useProfilesList.ts
  • src/features/App/useRecentProfile.ts
  • src/features/App/useVaultOpener.tsx
  • src/state/redux/profiles/profiles.ts
💤 Files with no reviewable changes (1)
  • src/features/App/WorkspaceManager/index.tsx

Comment on lines +139 to +141
const isActiveWorkspaceReady = useAppSelector(
selectIsActiveWorkspaceReady({ profileId }),
);

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

Memoize the selector to prevent unnecessary re-renders.

selectIsActiveWorkspaceReady is a selector factory that returns a new selector on each call. Without memoization, a new selector instance is created on every render, which can cause unnecessary re-computations and re-renders.

🔧 Proposed fix
-	const isActiveWorkspaceReady = useAppSelector(
-		selectIsActiveWorkspaceReady({ profileId }),
-	);
+	const isActiveWorkspaceReady = useAppSelector(
+		useMemo(() => selectIsActiveWorkspaceReady({ profileId }), [profileId]),
+	);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/features/App/Profile/index.tsx` around lines 139 - 141, The selector
factory selectIsActiveWorkspaceReady({ profileId }) is being recreated on every
render, causing extra selector instances and potential re-renders; wrap the
selector creation in useMemo and pass that memoized selector into useAppSelector
(e.g. const selector = useMemo(() => selectIsActiveWorkspaceReady({ profileId
}), [profileId]); const isActiveWorkspaceReady = useAppSelector(selector)) so
the selector identity remains stable across renders for the same profileId.

Comment on lines +37 to +60
async (profile: ProfileObject, password?: string) => {
setIsProfileOpening(true);

// Profiles with no password
if (!profile.encryption) {
await profiles.openProfile({ profile }, true);
setIsProfileOpening(false);
return { status: 'ok' };
}

// Profiles with password
if (password === undefined)
return { status: 'error', message: 'Enter password' };

try {
await profiles.openProfile({ profile, password }, true);
return { status: 'ok' };
} catch (err) {
console.error(err);

return { status: 'error', message: 'Invalid password' };
} finally {
setIsProfileOpening(false);
}

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 | 🔴 Critical

Fix stuck loading state in onOpenProfile.

isProfileOpening can remain true (early return on missing password at Line 48, or exception in unencrypted open path). This can freeze loading UX.

Proposed fix
 	const onOpenProfile: OnPickProfile = useCallback(
 		async (profile: ProfileObject, password?: string) => {
-			setIsProfileOpening(true);
-
-			// Profiles with no password
-			if (!profile.encryption) {
-				await profiles.openProfile({ profile }, true);
-				setIsProfileOpening(false);
-				return { status: 'ok' };
-			}
-
-			// Profiles with password
-			if (password === undefined)
-				return { status: 'error', message: 'Enter password' };
+			// Profiles with password
+			if (profile.encryption && password === undefined) {
+				return { status: 'error', message: 'Enter password' };
+			}
+
+			setIsProfileOpening(true);

 			try {
-				await profiles.openProfile({ profile, password }, true);
+				if (!profile.encryption) {
+					await profiles.openProfile({ profile }, true);
+				} else {
+					await profiles.openProfile({ profile, password }, true);
+				}
 				return { status: 'ok' };
 			} catch (err) {
 				console.error(err);

 				return { status: 'error', message: 'Invalid password' };
 			} finally {
 				setIsProfileOpening(false);
 			}
 		},
 		[profiles],
 	);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/features/App/useVaultOpener.tsx` around lines 37 - 60, The async
profile-opening callback can leave isProfileOpening true on early returns or if
profiles.openProfile throws; wrap the entire handler logic in a try/finally (or
ensure every early return calls setIsProfileOpening(false)) so
setIsProfileOpening(false) always runs. Concretely, in the async callback (the
onOpenProfile handler) move setIsProfileOpening(true) to the top, put all logic
(including the no-encryption branch, password validation, and the try/catch for
profiles.openProfile) inside a try block, and call setIsProfileOpening(false) in
a finally block; keep using profiles.openProfile and maintain the same returned
status objects.

Comment on lines +63 to +68
onCreateProfile={(profile) =>
profiles.createProfile(profile).then((newProfile) => {
onOpenProfile(newProfile, profile.password ?? undefined).then(
console.warn,
);
})

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 | 🟠 Major

Handle vault-open result after profile creation instead of warning blindly.

Current flow ignores onOpenProfile outcome and uses .then(console.warn), which logs even successful responses and hides actionable failures from the creator form.

Proposed fix
 			<ProfileCreator
-				onCreateProfile={(profile) =>
-					profiles.createProfile(profile).then((newProfile) => {
-						onOpenProfile(newProfile, profile.password ?? undefined).then(
-							console.warn,
-						);
-					})
-				}
+				onCreateProfile={async (profile) => {
+					try {
+						const newProfile = await profiles.createProfile(profile);
+						const result = await onOpenProfile(
+							newProfile,
+							profile.password ?? undefined,
+						);
+
+						if (result.status === 'error') {
+							return result.message ?? 'Cannot open the created profile';
+						}
+					} catch (error) {
+						console.error(error);
+						return 'Cannot create profile';
+					}
+				}}
📝 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
onCreateProfile={(profile) =>
profiles.createProfile(profile).then((newProfile) => {
onOpenProfile(newProfile, profile.password ?? undefined).then(
console.warn,
);
})
onCreateProfile={async (profile) => {
try {
const newProfile = await profiles.createProfile(profile);
const result = await onOpenProfile(
newProfile,
profile.password ?? undefined,
);
if (result.status === 'error') {
return result.message ?? 'Cannot open the created profile';
}
} catch (error) {
console.error(error);
return 'Cannot create profile';
}
}}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/features/App/VaultScreenManager.tsx` around lines 63 - 68, The current
onCreateProfile callback calls profiles.createProfile(...).then(newProfile =>
onOpenProfile(newProfile, profile.password ?? undefined).then(console.warn))
which blindly logs results; change it to await or chain properly and handle
success and errors from onOpenProfile: call onOpenProfile(newProfile,
profile.password ?? undefined) and then on success proceed (e.g., resolve or
navigate) and on failure catch the error and surface it to the creator form (or
call an error handler) instead of console.warn. Update the handler (the
onCreateProfile closure) to use try/catch or .then(...).catch(...) around both
profiles.createProfile and onOpenProfile so failures are handled and propagated
appropriately.

Comment on lines +39 to +41
workspaceConfig.set(config).then(() => {
console.debug('Workspace config is saved');
});

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 | 🟠 Major

Handle config-write failures to avoid unhandled promise rejections.

workspaceConfig.set(config) is called with .then(...) only. If validation/write fails, the rejection is unhandled and persistence silently breaks.

Proposed fix
 			onChange(config) {
 				if (!config) return;

-				workspaceConfig.set(config).then(() => {
-					console.debug('Workspace config is saved');
-				});
+				void workspaceConfig
+					.set(config)
+					.then(() => {
+						console.debug('Workspace config is saved');
+					})
+					.catch((error) => {
+						console.error('Failed to save workspace config', error);
+					});
 			},
📝 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
workspaceConfig.set(config).then(() => {
console.debug('Workspace config is saved');
});
onChange(config) {
if (!config) return;
void workspaceConfig
.set(config)
.then(() => {
console.debug('Workspace config is saved');
})
.catch((error) => {
console.error('Failed to save workspace config', error);
});
},
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/features/App/Workspace/services/useWorkspaceConfigSync.tsx` around lines
39 - 41, The call to workspaceConfig.set(config) currently only attaches a
.then() so rejections are unhandled; update the code where
workspaceConfig.set(config) is invoked (the workspaceConfig.set call in
useWorkspaceConfigSync) to handle failures by either awaiting the promise inside
an async function with try/catch or by appending a .catch(...) to the promise
chain, and in the catch block log the error with processLogger or console.error
and optionally surface or retry the failure so persistence errors are not
swallowed.

Comment on lines +22 to +44
useEffect(() => {
const initConfig = async () => {
const workspaceConfig = await new StateFile(
new FileController(
`workspaces/${workspaceData.workspaceId}/config.json`,
controls.profile.files,
),
WorkspaceConfigScheme,
).get();

if (!workspaceConfig) return;

dispatch(
workspacesApi.setWorkspaceNoteTemplateConfig({
...workspaceData,
title: workspaceConfig.newNote.title,
tags: workspaceConfig.newNote.tags,
}),
);
};

initConfig();
}, [controls.profile.files, dispatch, workspaceData]);

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 | 🟠 Major

Catch rejected async initializers started from effects.

Line 43 and Line 128 invoke async functions without rejection handling. If any read fails, this can produce unhandled promise rejections.

Proposed fix
-		initConfig();
+		void initConfig().catch((error) => {
+			console.error('Failed to initialize workspace config', error);
+		});
 	}, [controls.profile.files, dispatch, workspaceData]);
@@
-		initWorkspace();
+		void initWorkspace().catch((error) => {
+			console.error('Failed to initialize workspace state', error);
+		});

Also applies to: 52-128

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

In `@src/features/App/Workspace/useWorkspaceInitialization.tsx` around lines 22 -
44, The effect launches async initializers (e.g., initConfig in
useWorkspaceInitialization.tsx and any other async initializers called from
useEffect in this file) without rejection handling, which can cause unhandled
promise rejections; fix by adding error handling inside each async initializer
(wrap the body of initConfig and other async functions in try/catch and
handle/log the error or dispatch an error action) and ensure the call site does
not leave a raw promise unhandled (either await the async call inside the effect
using an IIFE or call initConfig().catch(...)). Reference initConfig and the
useEffect that invokes it when applying the try/catch/.catch fix.

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 4

♻️ Duplicate comments (9)
.github/workflows/code-review.yml (3)

24-24: ⚠️ Potential issue | 🟠 Major

Pin the action to a full commit SHA, not @latest.

Line 24 is currently floating and can pull breaking or compromised updates automatically.

Suggested fix
-      - uses: coderabbitai/ai-pr-reviewer@latest
+      - uses: coderabbitai/ai-pr-reviewer@<full-commit-sha>
Best practice for pinning third-party GitHub Actions to commit SHAs and how to resolve a tag (like latest) to an immutable SHA.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/code-review.yml at line 24, Replace the floating tag
"coderabbitai/ai-pr-reviewer@latest" with an immutable commit SHA to prevent
accidental breaking/compromised updates; locate the workflow step that uses the
action (the uses entry currently set to coderabbitai/ai-pr-reviewer@latest) and
update it to coderabbitai/ai-pr-reviewer@<FULL_COMMIT_SHA> by resolving the
desired release/tag to its commit SHA (e.g., via the action repo's Releases page
or `git ls-remote`), commit the change, and include a short comment in the
workflow noting which release/tag the SHA corresponds to for future
maintainability.

33-34: ⚠️ Potential issue | 🟠 Major

Verify model slugs against the provider catalog.

If either model ID is invalid, the reviewer may fail or silently fall back.

#!/bin/bash
set -euo pipefail
# Verify the configured OpenRouter model IDs exist.
curl -fsSL https://openrouter.ai/api/v1/models \
  | jq -r '.data[].id' \
  | rg -n '^(openai/gpt-5-nano|meta-llama/llama-3-70b-instruct)$' || true
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/code-review.yml around lines 33 - 34, The model slugs
configured as openai_light_model and openai_heavy_model may be invalid; add a
validation step to the workflow that calls the provider catalog (e.g.,
OpenRouter API) and verifies the values of the openai_light_model and
openai_heavy_model keys exist, failing the job if they are not present;
alternatively, correct the values to the canonical slugs returned by the catalog
(ensure you reference the openai_light_model and openai_heavy_model keys when
implementing the check or update).

8-10: ⚠️ Potential issue | 🔴 Critical

Do not run both pull_request and pull_request_target for this job.

Line 8 + Line 9 together can duplicate execution, and pull_request_target introduces higher secret-exposure risk. Keep one trigger strategy only.

Suggested fix (safe default)
 on:
   pull_request:
+    types: [opened, synchronize, reopened]
-  pull_request_target:
-    types: [opened, synchronize, reopened]
   pull_request_review_comment:
     types: [created]
GitHub Actions: security implications of using pull_request_target with forked PRs and secrets, and recommended trigger patterns to avoid duplicate runs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/code-review.yml around lines 8 - 10, The workflow is
configured with both pull_request and pull_request_target triggers which can
cause duplicate runs and expose secrets to forked PRs; remove one trigger
(preferably delete the pull_request_target line and keep pull_request) so only a
single, safer trigger remains; update the top-level trigger block to only
include the chosen event (e.g., keep pull_request: types: [opened, synchronize,
reopened]) and remove the pull_request_target entry to eliminate duplication and
secret-exposure risk.
src/features/App/Profile/index.tsx (1)

139-141: ⚠️ Potential issue | 🟡 Minor

Memoize the selectIsActiveWorkspaceReady selector factory.

A new selector instance is created on each render. Reuse a memoized selector, same pattern already used above for selectWorkspacesInfo.

🔧 Proposed fix
-const isActiveWorkspaceReady = useAppSelector(
-	selectIsActiveWorkspaceReady({ profileId }),
-);
+const activeWorkspaceReadySelector = useMemo(
+	() => selectIsActiveWorkspaceReady({ profileId }),
+	[profileId],
+);
+const isActiveWorkspaceReady = useAppSelector(activeWorkspaceReadySelector);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/features/App/Profile/index.tsx` around lines 139 - 141, The selector
factory selectIsActiveWorkspaceReady is being re-created on every render;
memoize it like selectWorkspacesInfo to avoid new selector instances: inside the
component create a stable selector with useMemo(() =>
selectIsActiveWorkspaceReady({ profileId }), [profileId]) (or similar hook-based
memoization) and then pass that memoized selector into useAppSelector instead of
calling selectIsActiveWorkspaceReady inline.
src/features/App/useVaultOpener.tsx (1)

37-60: ⚠️ Potential issue | 🔴 Critical

Fix loading-state leak in onOpenProfile.

isProfileOpening can remain true on early return (missing password) or exception in the unencrypted branch. This can freeze the unlock flow.

🔧 Proposed fix
 	const onOpenProfile: OnPickProfile = useCallback(
 		async (profile: ProfileObject, password?: string) => {
-			setIsProfileOpening(true);
-
-			// Profiles with no password
-			if (!profile.encryption) {
-				await profiles.openProfile({ profile }, true);
-				setIsProfileOpening(false);
-				return { status: 'ok' };
-			}
-
-			// Profiles with password
-			if (password === undefined)
-				return { status: 'error', message: 'Enter password' };
+			if (profile.encryption && password === undefined) {
+				return { status: 'error', message: 'Enter password' };
+			}
+			setIsProfileOpening(true);
 
 			try {
-				await profiles.openProfile({ profile, password }, true);
+				if (!profile.encryption) {
+					await profiles.openProfile({ profile }, true);
+				} else {
+					await profiles.openProfile({ profile, password }, true);
+				}
 				return { status: 'ok' };
 			} catch (err) {
 				console.error(err);
 
 				return { status: 'error', message: 'Invalid password' };
 			} finally {
 				setIsProfileOpening(false);
 			}
 		},
 		[profiles],
 	);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/features/App/useVaultOpener.tsx` around lines 37 - 60, The loading-state
leak happens because setIsProfileOpening(true) is not guaranteed to be cleared
on all exits of the async handler (the anonymous async function used as
onOpenProfile), e.g. early return when password is missing or exceptions in the
unencrypted branch; wrap the whole handler body in a try { ... } finally {
setIsProfileOpening(false) } (or ensure setIsProfileOpening(false) is called
before every return) so that setIsProfileOpening is always reset, keeping
references to setIsProfileOpening and profiles.openProfile (and the
onOpenProfile/anonymous handler) to locate the code to change.
src/features/App/Workspace/useWorkspaceInitialization.tsx (2)

43-44: ⚠️ Potential issue | 🟠 Major

Handle rejected async initializer for config loading.

initConfig() is invoked without rejection handling. If the StateFile read or dispatch fails, this produces an unhandled promise rejection.

Proposed fix
-		initConfig();
+		void initConfig().catch((error) => {
+			console.error('Failed to initialize workspace config', error);
+		});
 	}, [controls.profile.files, dispatch, workspaceData]);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/features/App/Workspace/useWorkspaceInitialization.tsx` around lines 43 -
44, The call to initConfig() inside useWorkspaceInitialization is made without
handling rejections; update the effect so initConfig()'s returned promise is
awaited or its rejection is caught and handled (e.g., try/catch inside an async
wrapper or attach .catch) to avoid unhandled promise rejections during StateFile
reads or dispatch failures; specifically modify the effect that references
controls.profile.files, dispatch, workspaceData to call an async wrapper (or
append .catch) around initConfig() and log or dispatch an error action when
initConfig() or StateFile read fails.

128-128: ⚠️ Potential issue | 🟠 Major

Handle rejected async initializer for workspace state.

initWorkspace() is called without rejection handling. The try/finally block inside ensures readiness is set even on error, but if an exception escapes, it creates an unhandled promise rejection.

Proposed fix
-		initWorkspace();
+		void initWorkspace().catch((error) => {
+			console.error('Failed to initialize workspace state', error);
+		});

 		// Cleanup notes and ready state on unmount
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/features/App/Workspace/useWorkspaceInitialization.tsx` at line 128,
initWorkspace() is invoked without handling rejections which can cause unhandled
promise rejections; update the caller in useWorkspaceInitialization (the place
that currently calls initWorkspace()) to await the async initWorkspace call
inside a try/catch/finally (or at minimum append .catch(...)) so any thrown
error is caught and logged/handled, and keep the existing finally branch that
sets readiness (e.g., setIsReady(true)) so readiness still flips regardless of
success or failure. Ensure you reference and modify the invocation of
initWorkspace() itself so errors from the initializer are properly handled.
src/features/App/Workspace/services/useWorkspaceConfigSync.tsx (1)

39-41: ⚠️ Potential issue | 🟠 Major

Handle config-write failures to avoid unhandled promise rejections.

workspaceConfig.set(config) is called with .then(...) only. If validation or write fails, the rejection is unhandled and persistence silently breaks.

Proposed fix
 			onChange(config) {
 				if (!config) return;

-				workspaceConfig.set(config).then(() => {
-					console.debug('Workspace config is saved');
-				});
+				void workspaceConfig
+					.set(config)
+					.then(() => {
+						console.debug('Workspace config is saved');
+					})
+					.catch((error) => {
+						console.error('Failed to save workspace config', error);
+					});
 			},
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/features/App/Workspace/services/useWorkspaceConfigSync.tsx` around lines
39 - 41, workspaceConfig.set(config) is called with only .then(...) which leaves
rejections unhandled; update the useWorkspaceConfigSync effect/hook to handle
failures by awaiting or chaining a .catch on workspaceConfig.set(config) (or
wrapping the await call in try/catch) and log/report the error (e.g., via
console.error or processLogger) and optionally surface user feedback or retry
logic; locate the call to workspaceConfig.set in useWorkspaceConfigSync.tsx and
ensure any Promise rejection is caught and handled to avoid unhandled promise
rejections.
src/features/App/VaultScreenManager.tsx (1)

63-68: ⚠️ Potential issue | 🟠 Major

Handle vault-open result after profile creation instead of ignoring errors.

The .then(console.warn) pattern logs both successful responses and errors indiscriminately, hiding actionable failures from the user. This should properly handle the result and surface errors to the creator form.

Proposed fix
 			<ProfileCreator
-				onCreateProfile={(profile) =>
-					profiles.createProfile(profile).then((newProfile) => {
-						onOpenProfile(newProfile, profile.password ?? undefined).then(
-							console.warn,
-						);
-					})
-				}
+				onCreateProfile={async (profile) => {
+					try {
+						const newProfile = await profiles.createProfile(profile);
+						const result = await onOpenProfile(
+							newProfile,
+							profile.password ?? undefined,
+						);
+
+						if (result.status === 'error') {
+							return result.message ?? 'Cannot open the created profile';
+						}
+					} catch (error) {
+						console.error(error);
+						return 'Cannot create profile';
+					}
+				}}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/features/App/VaultScreenManager.tsx` around lines 63 - 68, The
onCreateProfile handler currently calls
profiles.createProfile(profile).then(newProfile => onOpenProfile(newProfile,
profile.password ?? undefined).then(console.warn)) which misuse
.then(console.warn) and swallows errors; update this to properly handle success
and failure from onOpenProfile (and profiles.createProfile) by returning the
promise chain and using explicit then/catch (or async/await) so errors are
caught and forwarded to the creator form UI (e.g., reject/throw or call the form
error handler). Specifically modify the onCreateProfile callback to await or
return profiles.createProfile(...), call onOpenProfile(newProfile,
profile.password ?? undefined), and on any rejection call the form's error
handler or propagate the error instead of logging with console.warn.
🧹 Nitpick comments (3)
src/features/App/useRecentProfile.ts (1)

10-13: Consider renaming profileId state to match its object shape.

This state stores { isLoaded, profileId }, so a name like recentProfile would be clearer and reduce cognitive load.

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

In `@src/features/App/useRecentProfile.ts` around lines 10 - 13, The state
variable profileId in useRecentProfile.ts actually holds an object of type
RecentProfile ({ isLoaded, profileId }), so rename the state and its setter to
recentProfile and setRecentProfile to match the shape; update the useState call
that currently declares profileId/setProfileId, change all references/usages of
profileId and setProfileId in this file (and any exports) to recentProfile and
setRecentProfile respectively to keep names consistent with the RecentProfile
type.
src/features/App/Workspace/services/useWorkspaceConfigSync.tsx (1)

22-44: Consider memoizing the StateFile instance.

The StateFile and FileController are created inside the effect but used across subscription callbacks. If any dependency changes, a new instance is created while the previous subscription may still be active briefly. This is likely fine given the cleanup, but you could move the StateFile creation outside the effect using useMemo for clarity.

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

In `@src/features/App/Workspace/services/useWorkspaceConfigSync.tsx` around lines
22 - 44, Move the StateFile/FileController creation out of the effect and
memoize it using React's useMemo so the same instance is reused across renders;
specifically memoize the new StateFile(new
FileController(`workspaces/${workspaceData.workspaceId}/config.json`, files),
WorkspaceConfigScheme) (referencing StateFile, FileController,
WorkspaceConfigScheme and the workspaceData.workspaceId/files inputs) and then
use that memoized workspaceConfig inside the useEffect that sets up
watchSelector/createAppSelector/selectWorkspace, ensuring the dependency array
uses the memoized value instead of recreating the instance on each dependency
change.
src/features/App/VaultScreenManager.tsx (1)

101-127: Remove redundant key prop on HStack.

The key={profile.id} on Line 106 is redundant since NestedList already uses item.id (which equals profile.id) as the key for the wrapping <li> element.

Proposed fix
 					content: (
 						<HStack
 							as="button"
-							key={profile.id}
 							sx={{
 								padding: '.8rem 1rem',
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/features/App/VaultScreenManager.tsx` around lines 101 - 127, Remove the
redundant key prop on the HStack rendered inside profiles.profiles.map; the
wrapper (NestedList) already uses item.id as the list key, so delete
key={profile.id} from the HStack element in the map callback (the block that
calls onChooseProfile/onOpenProfile and renders FaUser and Text) to avoid
duplicate keys.
🤖 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/code-review.yml:
- Line 29: Change the CI workflow to disable verbose OpenAI debugging by setting
the YAML key debug from true to false (replace the occurrence of "debug: true"
with "debug: false"); if you still need detailed logs locally, gate the debug
flag behind an environment variable (e.g., use a CI-only default false and allow
override via an env var like OPENAI_DEBUG for local/testing) so production CI
runs never emit detailed API request/response payloads.

In `@src/features/App/VaultScreenManager.tsx`:
- Around line 113-121: The click handler calls onOpenProfile(profile) for
unencrypted profiles but ignores its returned promise and immediately records
telemetry, which can mark failures as successes; update the onClick flow around
onChooseProfile, onOpenProfile, and
telemetry.track(TELEMETRY_EVENT_NAME.PROFILE_SELECTED) so that you wait for
onOpenProfile(profile) to resolve (or attach .then/.catch) before calling
telemetry, and handle rejections by logging/reporting the error and preventing
the telemetry call; reference the onChooseProfile(profile.id),
onOpenProfile(profile), profile.encryption check, and telemetry.track(...)
symbols when making the change.

In `@src/features/App/Workspace/useWorkspaceInitialization.tsx`:
- Around line 146-150: The change handler registered with
workspace.tagsRegistry.onChange currently calls
workspace.tagsRegistry.getTags().then(...) without handling rejections; update
the callback (the function passed to workspace.tagsRegistry.onChange) to await
or attach a .catch to getTags and handle errors (e.g., log via process/logger or
dispatch an error state instead of letting the promise rejection go unhandled)
before calling dispatch(workspacesApi.setTags({...workspaceData, tags})); ensure
you reference workspace.tagsRegistry.onChange, workspace.tagsRegistry.getTags,
dispatch, workspacesApi.setTags and workspaceData when adding the error
handling.

In `@src/features/App/Workspace/useWorkspaceState.tsx`:
- Around line 33-39: The StateFile is created once via useState and freezes the
initial workspaceId/files; change creation to recreate the
StateFile/FileController when workspaceId or files change (e.g., replace the
useState initializer with useMemo or recreate inside a useEffect and set local
state) so the FileController path `workspaces/${workspaceId}/state.json` updates
with dependencies, and add proper error handling around
workspaceState.set(state) (handle the returned promise or wrap in try/catch and
log/report failures and optionally retry) so async write errors are not silently
dropped; ensure references to StateFile, FileController and workspaceState.set
are updated accordingly.

---

Duplicate comments:
In @.github/workflows/code-review.yml:
- Line 24: Replace the floating tag "coderabbitai/ai-pr-reviewer@latest" with an
immutable commit SHA to prevent accidental breaking/compromised updates; locate
the workflow step that uses the action (the uses entry currently set to
coderabbitai/ai-pr-reviewer@latest) and update it to
coderabbitai/ai-pr-reviewer@<FULL_COMMIT_SHA> by resolving the desired
release/tag to its commit SHA (e.g., via the action repo's Releases page or `git
ls-remote`), commit the change, and include a short comment in the workflow
noting which release/tag the SHA corresponds to for future maintainability.
- Around line 33-34: The model slugs configured as openai_light_model and
openai_heavy_model may be invalid; add a validation step to the workflow that
calls the provider catalog (e.g., OpenRouter API) and verifies the values of the
openai_light_model and openai_heavy_model keys exist, failing the job if they
are not present; alternatively, correct the values to the canonical slugs
returned by the catalog (ensure you reference the openai_light_model and
openai_heavy_model keys when implementing the check or update).
- Around line 8-10: The workflow is configured with both pull_request and
pull_request_target triggers which can cause duplicate runs and expose secrets
to forked PRs; remove one trigger (preferably delete the pull_request_target
line and keep pull_request) so only a single, safer trigger remains; update the
top-level trigger block to only include the chosen event (e.g., keep
pull_request: types: [opened, synchronize, reopened]) and remove the
pull_request_target entry to eliminate duplication and secret-exposure risk.

In `@src/features/App/Profile/index.tsx`:
- Around line 139-141: The selector factory selectIsActiveWorkspaceReady is
being re-created on every render; memoize it like selectWorkspacesInfo to avoid
new selector instances: inside the component create a stable selector with
useMemo(() => selectIsActiveWorkspaceReady({ profileId }), [profileId]) (or
similar hook-based memoization) and then pass that memoized selector into
useAppSelector instead of calling selectIsActiveWorkspaceReady inline.

In `@src/features/App/useVaultOpener.tsx`:
- Around line 37-60: The loading-state leak happens because
setIsProfileOpening(true) is not guaranteed to be cleared on all exits of the
async handler (the anonymous async function used as onOpenProfile), e.g. early
return when password is missing or exceptions in the unencrypted branch; wrap
the whole handler body in a try { ... } finally { setIsProfileOpening(false) }
(or ensure setIsProfileOpening(false) is called before every return) so that
setIsProfileOpening is always reset, keeping references to setIsProfileOpening
and profiles.openProfile (and the onOpenProfile/anonymous handler) to locate the
code to change.

In `@src/features/App/VaultScreenManager.tsx`:
- Around line 63-68: The onCreateProfile handler currently calls
profiles.createProfile(profile).then(newProfile => onOpenProfile(newProfile,
profile.password ?? undefined).then(console.warn)) which misuse
.then(console.warn) and swallows errors; update this to properly handle success
and failure from onOpenProfile (and profiles.createProfile) by returning the
promise chain and using explicit then/catch (or async/await) so errors are
caught and forwarded to the creator form UI (e.g., reject/throw or call the form
error handler). Specifically modify the onCreateProfile callback to await or
return profiles.createProfile(...), call onOpenProfile(newProfile,
profile.password ?? undefined), and on any rejection call the form's error
handler or propagate the error instead of logging with console.warn.

In `@src/features/App/Workspace/services/useWorkspaceConfigSync.tsx`:
- Around line 39-41: workspaceConfig.set(config) is called with only .then(...)
which leaves rejections unhandled; update the useWorkspaceConfigSync effect/hook
to handle failures by awaiting or chaining a .catch on
workspaceConfig.set(config) (or wrapping the await call in try/catch) and
log/report the error (e.g., via console.error or processLogger) and optionally
surface user feedback or retry logic; locate the call to workspaceConfig.set in
useWorkspaceConfigSync.tsx and ensure any Promise rejection is caught and
handled to avoid unhandled promise rejections.

In `@src/features/App/Workspace/useWorkspaceInitialization.tsx`:
- Around line 43-44: The call to initConfig() inside useWorkspaceInitialization
is made without handling rejections; update the effect so initConfig()'s
returned promise is awaited or its rejection is caught and handled (e.g.,
try/catch inside an async wrapper or attach .catch) to avoid unhandled promise
rejections during StateFile reads or dispatch failures; specifically modify the
effect that references controls.profile.files, dispatch, workspaceData to call
an async wrapper (or append .catch) around initConfig() and log or dispatch an
error action when initConfig() or StateFile read fails.
- Line 128: initWorkspace() is invoked without handling rejections which can
cause unhandled promise rejections; update the caller in
useWorkspaceInitialization (the place that currently calls initWorkspace()) to
await the async initWorkspace call inside a try/catch/finally (or at minimum
append .catch(...)) so any thrown error is caught and logged/handled, and keep
the existing finally branch that sets readiness (e.g., setIsReady(true)) so
readiness still flips regardless of success or failure. Ensure you reference and
modify the invocation of initWorkspace() itself so errors from the initializer
are properly handled.

---

Nitpick comments:
In `@src/features/App/useRecentProfile.ts`:
- Around line 10-13: The state variable profileId in useRecentProfile.ts
actually holds an object of type RecentProfile ({ isLoaded, profileId }), so
rename the state and its setter to recentProfile and setRecentProfile to match
the shape; update the useState call that currently declares
profileId/setProfileId, change all references/usages of profileId and
setProfileId in this file (and any exports) to recentProfile and
setRecentProfile respectively to keep names consistent with the RecentProfile
type.

In `@src/features/App/VaultScreenManager.tsx`:
- Around line 101-127: Remove the redundant key prop on the HStack rendered
inside profiles.profiles.map; the wrapper (NestedList) already uses item.id as
the list key, so delete key={profile.id} from the HStack element in the map
callback (the block that calls onChooseProfile/onOpenProfile and renders FaUser
and Text) to avoid duplicate keys.

In `@src/features/App/Workspace/services/useWorkspaceConfigSync.tsx`:
- Around line 22-44: Move the StateFile/FileController creation out of the
effect and memoize it using React's useMemo so the same instance is reused
across renders; specifically memoize the new StateFile(new
FileController(`workspaces/${workspaceData.workspaceId}/config.json`, files),
WorkspaceConfigScheme) (referencing StateFile, FileController,
WorkspaceConfigScheme and the workspaceData.workspaceId/files inputs) and then
use that memoized workspaceConfig inside the useEffect that sets up
watchSelector/createAppSelector/selectWorkspace, ensuring the dependency array
uses the memoized value instead of recreating the instance on each dependency
change.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3039225 and 50e9a3a.

📒 Files selected for processing (19)
  • .github/workflows/code-review.yml
  • src/core/features/notes/controller/NotesController.test.ts
  • src/features/App/Profile/index.tsx
  • src/features/App/Profile/services/useVaultConfigSync.tsx
  • src/features/App/ProfileCreator/index.tsx
  • src/features/App/ProfileLoginForm/index.tsx
  • src/features/App/ProfilesForm.tsx
  • src/features/App/VaultScreenManager.tsx
  • src/features/App/Workspace/index.tsx
  • src/features/App/Workspace/services/WorkspaceServices.tsx
  • src/features/App/Workspace/services/useWorkspaceConfigSync.tsx
  • src/features/App/Workspace/useWorkspaceInitialization.tsx
  • src/features/App/Workspace/useWorkspaceState.tsx
  • src/features/App/WorkspaceManager/index.tsx
  • src/features/App/index.tsx
  • src/features/App/useProfilesList.ts
  • src/features/App/useRecentProfile.ts
  • src/features/App/useVaultOpener.tsx
  • src/state/redux/profiles/profiles.ts
💤 Files with no reviewable changes (1)
  • src/features/App/WorkspaceManager/index.tsx

GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
with:
debug: true

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 | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n .github/workflows/code-review.yml | head -40

Repository: vitonsky/deepink

Length of output: 1318


🌐 Web query:

coderabbitai ai-pr-reviewer github action debug option documentation

💡 Result:

The debug option for coderabbitai/ai-pr-reviewer is documented in:

  • action.yml (authoritative list of inputs): inputs.debug → “Enable debug mode”, default false. Set it via with: debug: 'true'. [1]
  • README.md (usage note): “Inspect the messages between OpenAI server” → set debug: true in the workflow to enable debug mode and show the messages. [2]
  • DeepWiki “Installation & Setup” (summary of options): debug default false, described as enabling detailed logs (including API requests). [3]

Example:

- uses: coderabbitai/ai-pr-reviewer@latest
  with:
    debug: 'true'

Sources: [1] [2] [3]


Disable debug: true for production CI runs.

Line 29 should default to false. Debug mode logs detailed API requests and responses to the OpenAI server, which can expose sensitive review payload information and create unnecessary noise in CI logs.

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

In @.github/workflows/code-review.yml at line 29, Change the CI workflow to
disable verbose OpenAI debugging by setting the YAML key debug from true to
false (replace the occurrence of "debug: true" with "debug: false"); if you
still need detailed logs locally, gate the debug flag behind an environment
variable (e.g., use a CI-only default false and allow override via an env var
like OPENAI_DEBUG for local/testing) so production CI runs never emit detailed
API request/response payloads.

Comment on lines +113 to +121
onClick={() => {
onChooseProfile(profile.id);

if (profile.encryption === null) {
onOpenProfile(profile);
}

telemetry.track(TELEMETRY_EVENT_NAME.PROFILE_SELECTED);
}}

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

Handle errors when opening unencrypted profiles.

For unencrypted profiles, onOpenProfile is called without handling the returned promise. If opening fails, the error is silently ignored while telemetry.track still executes, potentially recording misleading success events.

Proposed fix
 							onClick={() => {
 								onChooseProfile(profile.id);

 								if (profile.encryption === null) {
-									onOpenProfile(profile);
+									onOpenProfile(profile).catch((error) => {
+										console.error('Failed to open profile', error);
+									});
 								}

 								telemetry.track(TELEMETRY_EVENT_NAME.PROFILE_SELECTED);
 							}}
📝 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
onClick={() => {
onChooseProfile(profile.id);
if (profile.encryption === null) {
onOpenProfile(profile);
}
telemetry.track(TELEMETRY_EVENT_NAME.PROFILE_SELECTED);
}}
onClick={() => {
onChooseProfile(profile.id);
if (profile.encryption === null) {
onOpenProfile(profile).catch((error) => {
console.error('Failed to open profile', error);
});
}
telemetry.track(TELEMETRY_EVENT_NAME.PROFILE_SELECTED);
}}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/features/App/VaultScreenManager.tsx` around lines 113 - 121, The click
handler calls onOpenProfile(profile) for unencrypted profiles but ignores its
returned promise and immediately records telemetry, which can mark failures as
successes; update the onClick flow around onChooseProfile, onOpenProfile, and
telemetry.track(TELEMETRY_EVENT_NAME.PROFILE_SELECTED) so that you wait for
onOpenProfile(profile) to resolve (or attach .then/.catch) before calling
telemetry, and handle rejections by logging/reporting the error and preventing
the telemetry call; reference the onChooseProfile(profile.id),
onOpenProfile(profile), profile.encryption check, and telemetry.track(...)
symbols when making the change.

Comment on lines +146 to +150
return workspace.tagsRegistry.onChange(() => {
workspace.tagsRegistry.getTags().then((tags) => {
dispatch(workspacesApi.setTags({ ...workspaceData, tags }));
});
});

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

Handle errors when fetching tags on change.

workspace.tagsRegistry.getTags() is called with .then(...) only. If the fetch fails, the rejection is unhandled.

Proposed fix
 		return workspace.tagsRegistry.onChange(() => {
-			workspace.tagsRegistry.getTags().then((tags) => {
-				dispatch(workspacesApi.setTags({ ...workspaceData, tags }));
-			});
+			void workspace.tagsRegistry.getTags()
+				.then((tags) => {
+					dispatch(workspacesApi.setTags({ ...workspaceData, tags }));
+				})
+				.catch((error) => {
+					console.error('Failed to fetch tags on change', error);
+				});
 		});
📝 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
return workspace.tagsRegistry.onChange(() => {
workspace.tagsRegistry.getTags().then((tags) => {
dispatch(workspacesApi.setTags({ ...workspaceData, tags }));
});
});
return workspace.tagsRegistry.onChange(() => {
void workspace.tagsRegistry.getTags()
.then((tags) => {
dispatch(workspacesApi.setTags({ ...workspaceData, tags }));
})
.catch((error) => {
console.error('Failed to fetch tags on change', error);
});
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/features/App/Workspace/useWorkspaceInitialization.tsx` around lines 146 -
150, The change handler registered with workspace.tagsRegistry.onChange
currently calls workspace.tagsRegistry.getTags().then(...) without handling
rejections; update the callback (the function passed to
workspace.tagsRegistry.onChange) to await or attach a .catch to getTags and
handle errors (e.g., log via process/logger or dispatch an error state instead
of letting the promise rejection go unhandled) before calling
dispatch(workspacesApi.setTags({...workspaceData, tags})); ensure you reference
workspace.tagsRegistry.onChange, workspace.tagsRegistry.getTags, dispatch,
workspacesApi.setTags and workspaceData when adding the error handling.

Comment on lines +33 to +39
const [workspaceState] = useState(
() =>
new StateFile(
new FileController(`workspaces/${workspaceId}/state.json`, files),
WorkspaceStateScheme.partial(),
),
);

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 | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's find and examine the file in question
find . -name "useWorkspaceState.tsx" -type f

Repository: vitonsky/deepink

Length of output: 110


🏁 Script executed:

# Let's check the file size first
wc -l src/features/App/Workspace/useWorkspaceState.tsx

Repository: vitonsky/deepink

Length of output: 111


🏁 Script executed:

# Read the entire file to understand the context
cat -n src/features/App/Workspace/useWorkspaceState.tsx

Repository: vitonsky/deepink

Length of output: 2318


Recreate StateFile when dependencies change and handle async write failures.

StateFile is frozen to initial workspaceId and files values via useState initializer. If these props change while the component remains mounted, the instance continues targeting the original file path. Additionally, workspaceState.set(state) at line 63 has no error handling, silently dropping persistence failures.

🔧 Proposed fix
-import { useCallback, useEffect, useState } from 'react';
+import { useCallback, useEffect, useMemo } from 'react';
@@
-	const [workspaceState] = useState(
-		() =>
-			new StateFile(
-				new FileController(`workspaces/${workspaceId}/state.json`, files),
-				WorkspaceStateScheme.partial(),
-			),
-	);
+	const workspaceState = useMemo(
+		() =>
+			new StateFile(
+				new FileController(`workspaces/${workspaceId}/state.json`, files),
+				WorkspaceStateScheme.partial(),
+			),
+		[files, workspaceId],
+	);
@@
 			onChange(state) {
 				if (!state) return;
 
-				workspaceState.set(state);
+				void workspaceState.set(state).catch((error) => {
+					console.error('Failed to persist workspace state', error);
+				});
 			},
📝 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 [workspaceState] = useState(
() =>
new StateFile(
new FileController(`workspaces/${workspaceId}/state.json`, files),
WorkspaceStateScheme.partial(),
),
);
const workspaceState = useMemo(
() =>
new StateFile(
new FileController(`workspaces/${workspaceId}/state.json`, files),
WorkspaceStateScheme.partial(),
),
[files, workspaceId],
);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/features/App/Workspace/useWorkspaceState.tsx` around lines 33 - 39, The
StateFile is created once via useState and freezes the initial
workspaceId/files; change creation to recreate the StateFile/FileController when
workspaceId or files change (e.g., replace the useState initializer with useMemo
or recreate inside a useEffect and set local state) so the FileController path
`workspaces/${workspaceId}/state.json` updates with dependencies, and add proper
error handling around workspaceState.set(state) (handle the returned promise or
wrap in try/catch and log/report failures and optionally retry) so async write
errors are not silently dropped; ensure references to StateFile, FileController
and workspaceState.set are updated accordingly.

@vitonsky vitonsky closed this Mar 4, 2026
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.

Add LLM reviewer

1 participant