Refactor App.tsx, fix MathEvaluator, and optimize bundles#10
Merged
Conversation
Contributor
|
Warning Review limit reached
More reviews will be available in 49 minutes and 32 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR decomposes a monolithic CodeMirror plugin into six modular ChangesEditor Plugin Decomposition, Math/Variable System, UI Extraction, Settings Keys, OpenAI Integration
Sequence Diagram(s)sequenceDiagram
participant App.tsx
participant scopeUpdaterPlugin
participant VariableScope
participant MathEvaluator
participant EditorView
App.tsx->>EditorView: mount with decomposedPlugins
EditorView->>scopeUpdaterPlugin: docChanged event
scopeUpdaterPlugin->>VariableScope: triggerScopeUpdate(docStr, view)
VariableScope->>VariableScope: 300ms debounce → parse /var, mathjs.evaluate
VariableScope->>MathEvaluator: triggerMathEvaluation(view)
MathEvaluator->>MathEvaluator: 300ms debounce → two-pass = scan
MathEvaluator->>EditorView: dispatch({ changes: replacements })
sequenceDiagram
participant User
participant NoteSearch
participant useAppStore
participant electronAPI
User->>NoteSearch: types query / navigates with arrows
NoteSearch->>useAppStore: setSelectedSearchIndex / setNoteSearchQuery
User->>NoteSearch: presses Enter (note selected)
NoteSearch->>useAppStore: setCurrentNoteIndex, setShowNoteSearch(false)
User->>NoteSearch: Cmd+K → per-item menu → Delete
NoteSearch->>electronAPI: deleteNote(id)
NoteSearch->>useAppStore: setNotes (filtered), adjusts currentNoteIndex
sequenceDiagram
participant App
participant electronAPI
participant ipcRenderer
participant main.ts
participant OpenAI
App->>electronAPI: window.electronAPI.openAIChat({ model, messages, apiKey, baseURL })
electronAPI->>ipcRenderer: invoke('openai-chat', args)
ipcRenderer->>main.ts: 'openai-chat' handler
main.ts->>main.ts: dynamically import openai
main.ts->>OpenAI: create client, call chat.completions.create
OpenAI-->>main.ts: completion
main.ts-->>ipcRenderer: Promise<completion>
ipcRenderer-->>App: Promise<completion>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (1)
src/lib/editor/widgets.test.ts (1)
28-28: 💤 Low valueConsider replacing
as anycasts on widget mocks with a proper type.The mock editor view is repeatedly cast to
any. While not flagged by linting, usingPartial<EditorView>would improve type safety consistency with the rest of the test suite.🔧 Proposed refactor (optional)
import { EditorView } from '`@codemirror/view`' // Near the top of the test, create a helper: const mockEditorView = (dispatch = vi.fn()): Partial<EditorView> => ({ dispatch, }) // Then replace all `{ dispatch: vi.fn() } as any` with: const mockView = mockEditorView()Or import the exact typing if available from
@codemirror/view.Also applies to: 37-37, 46-46
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/editor/widgets.test.ts` at line 28, The mockView variable declaration uses an `as any` type cast which reduces type safety. Replace all instances of the pattern `{ dispatch: vi.fn() } as any` throughout the test file (including at the mockView declarations) with proper typing using `Partial<EditorView>`. Import EditorView from `@codemirror/view` at the top of the file and update each mock view assignment to use `Partial<EditorView>` type annotation instead of the any cast to maintain type consistency across the test suite.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@electron/main.ts`:
- Around line 599-601: The settings window disables webSecurity to allow direct
API calls to OpenAI/OpenRouter, creating a security vulnerability. Fix this by
enabling webSecurity and contextIsolation in the settings window configuration,
then create an IPC handler in the main process to proxy API requests instead of
making them from the renderer. Update the renderer to call this new IPC handler
for API requests instead of using dangerouslyAllowBrowser: true. This leverages
the existing IPC infrastructure already used for other handlers in the codebase.
In `@src/components/NoteSearch.tsx`:
- Around line 150-154: When a tag pill is clicked and updates noteSearchQuery
via setNoteSearchQuery, the searchSelectedIndex is not reset, causing it to
become out of bounds when the filtered results shrink. In the onClick handler
for the tag pill that calls setNoteSearchQuery(newQ), also reset
searchSelectedIndex to 0 after updating the query to ensure the selection index
remains valid when the results are filtered based on the new search query.
- Around line 69-76: The deleteNote method returns a Promise<boolean> but is
called without await before mutating state with setNotes and other state
setters, creating a race condition where the UI updates before the backend
confirms deletion. Add await to the deleteNote call and check its return value,
only proceeding with state mutations if it succeeds. Additionally, fix the
currentNoteIndex adjustment logic in the condition checking if currentNoteIndex
>= notes.length - 1 to properly handle deletion of notes at any position in the
array, not just at the end, ensuring the index always points to a valid note
after deletion.
In `@src/components/NoteTitleBar.tsx`:
- Around line 26-51: In the handleRenameSubmit function, avoid optimistic state
writes and stale closures in the persistence flow. For the saveNote case around
line 37, await the saveNote call and handle potential errors before calling
setNotes, rather than updating state optimistically before the operation
completes. For the renameNote case in the .then() callback around lines 45-50,
use functional setState with setNotes(prev => ...) pattern instead of capturing
the stale notes and currentNoteIndex variables from the closure, ensuring that
if note order changes before the promise resolves, the correct note gets
updated. Consider converting the .then() callback to async/await for better
clarity and consistency.
In `@src/hooks/useReminders.test.ts`:
- Line 70: The hardcoded string 'papercache_notified' is used in both the test
and the handleDueReminders function in useReminders.ts, but it is not
centralized in the SETTINGS_KEYS constant. Add 'papercache_notified' to the
SETTINGS_KEYS constant in src/lib/settingsKeys.ts, then import SETTINGS_KEYS in
useReminders.ts and replace the hardcoded key string in the handleDueReminders
function with the appropriate SETTINGS_KEYS reference, and finally import
SETTINGS_KEYS in useReminders.test.ts and replace the hardcoded key string in
the test with the corresponding SETTINGS_KEYS reference.
- Around line 17-22: Replace the `as any` type cast on the window.electronAPI
mock assignment with a proper type annotation using `Partial<ElectronAPI>` to
maintain type safety instead of disabling type checking. Ensure that ElectronAPI
is imported from the appropriate type definitions file at the top of the
useReminders.test.ts file if it is not already imported.
In `@src/hooks/useVariables.test.ts`:
- Line 9: The test file repeatedly uses `(window as any).__globalVariables`
casts which bypass TypeScript type safety. Add a proper global type declaration
at the module scope (after imports) by using `declare global` to extend the
Window interface with `__globalVariables` as an optional property of type
`Record<string, unknown>`. Then replace all instances of `(window as
any).__globalVariables` throughout the file (affecting lines around 9, 26-28,
40-42, and 54) with the properly typed `window.__globalVariables`, using
optional chaining with `?.` when needed to safely access nested properties on
this optional field.
In `@src/lib/editor/MathEvaluator.ts`:
- Around line 17-72: The document can change between capturing docStr at the
beginning of the evaluation and dispatching changes at the end, invalidating all
calculated offsets. Capture a snapshot of the editor view state (or document
version) immediately when docStr is captured, then before calling view.dispatch
at the end of the function, verify that the current document state matches the
captured snapshot. Only proceed with the dispatch if the document hasn't
changed; if it has changed, skip the dispatch to avoid corrupting the editor
content with stale offset calculations.
In `@src/lib/editor/plugins.ts`:
- Around line 63-70: The scopeUpdaterPlugin class only triggers
VariableScope.triggerScopeUpdate when the document changes, causing variables in
initially loaded documents to remain unparsed until the user edits. Add a create
method to the ViewPlugin class that runs when the editor view is first created,
and call VariableScope.triggerScopeUpdate with the initial document content in
that create method to ensure variable scope is parsed immediately upon editor
initialization.
In `@src/lib/editor/taskPlugin.ts`:
- Around line 90-157: The update method in the taskPlugin captures document
changes and computes character positions in the changes array, but then
dispatches them 10ms later via setTimeout. If the document changes again before
that timeout fires, the stored positions become stale and can rewrite unrelated
text. Fix this by implementing proper debouncing: store the timeout ID as a
property on the plugin instance, and when update is called again before the
previous timeout fires, cancel the old timeout using clearTimeout before
creating a new one. This ensures only the most recent document state's changes
are dispatched with valid positions.
In `@src/lib/editor/variablePlugin.ts`:
- Around line 42-44: The skip condition in the variablePlugin.ts file currently
only excludes lines starting with '/var', but it should also exclude '/globvar'
definition lines to prevent them from rendering their own variable names as
value pills. Modify the condition that checks
`line.text.trim().startsWith('/var')` to also handle the '/globvar' prefix,
ensuring both variable definition types are properly skipped from rendering.
- Around line 28-33: The scopeKeys array is being directly joined into a RegExp
pattern without escaping regex metacharacters, which can cause the regex to fail
or behave unexpectedly if any keys contain special characters like dots,
asterisks, or plus signs. Before constructing the RegExp in the line with new
RegExp(`\b(${scopeKeys.join('|')})\b`, 'g'), escape each key in the scopeKeys
array using a regex escaping utility function (such as one that replaces special
regex characters with their escaped versions) before joining them with the pipe
delimiter.
In `@src/lib/editor/VariableScope.ts`:
- Around line 42-49: The empty effects dispatch in the globalScopeCache update
block does not trigger the variablePlugin to rebuild, causing variable
decorations to remain stale when only scope values change. Replace the
`view.dispatch({ effects: [] })` call with a StateEffect that signals scope
changes, then update the variablePlugin configuration to rebuild when this
effect is applied (in addition to or instead of the current docChanged,
viewportChanged, and selectionSet conditions). This ensures the plugin detects
and responds to scope updates properly.
- Around line 12-49: The async scopeEvalTimeout callback can race with document
changes, causing stale globalScopeCache writes and outdated math evaluation
triggers. Add a scope generation counter (like this.scopeVersion) that gets
incremented each time a new timeout is created, and capture its value at the
start of the callback. Before updating this.globalScopeCache and dispatching
effects, verify that the captured version still matches the current
this.scopeVersion to ensure only the latest callback can write stale data. If
versions mismatch, return early from the callback without updating the cache or
triggering evaluation.
In `@src/lib/settingsKeys.ts`:
- Around line 11-12: The keys for AI_COLOR and MATH_COLOR in settingsKeys.ts
have been changed to papercache-color-ai and papercache-color-math, but the
runtime code that reads these settings (such as the App storage refresh
mechanism) still uses the old key names papercache-ai-color and
papercache-math-color. This breaks backward compatibility with existing stored
user preferences. Restore the original key names papercache-ai-color and
papercache-math-color for the AI_COLOR and MATH_COLOR properties to preserve
backward compatibility with previously saved preferences and ensure cross-window
synchronization works correctly.
In `@src/store/useSettingsStore.ts`:
- Line 31: The fontFamily property initialization in useSettingsStore.ts
defaults to 'monospace', but the Settings.tsx component uses "'JetBrains Mono',
monospace" as its default. Update the fontFamily fallback value in the store
initialization (the localStorage.getItem call) to use the same default value as
specified in Settings.tsx to ensure consistency between the app state and the
settings UI for fresh profiles.
---
Nitpick comments:
In `@src/lib/editor/widgets.test.ts`:
- Line 28: The mockView variable declaration uses an `as any` type cast which
reduces type safety. Replace all instances of the pattern `{ dispatch: vi.fn() }
as any` throughout the test file (including at the mockView declarations) with
proper typing using `Partial<EditorView>`. Import EditorView from
`@codemirror/view` at the top of the file and update each mock view assignment to
use `Partial<EditorView>` type annotation instead of the any cast to maintain
type consistency across the test suite.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 09043176-7994-4f05-b62e-35b0d45d9ea5
📒 Files selected for processing (22)
electron/main.tssrc/App.tsxsrc/Settings.tsxsrc/components/MainActionMenu.tsxsrc/components/NoteSearch.tsxsrc/components/NoteTitleBar.tsxsrc/hooks/useReminders.test.tssrc/hooks/useVariables.test.tssrc/lib/editor/MathEvaluator.tssrc/lib/editor/VariableScope.tssrc/lib/editor/checkboxPlugin.tssrc/lib/editor/codeBlockPlugin.tssrc/lib/editor/formatPlugin.tssrc/lib/editor/markdownPlugin.tssrc/lib/editor/plugins.tssrc/lib/editor/taskPlugin.tssrc/lib/editor/variablePlugin.tssrc/lib/editor/widgets.test.tssrc/lib/settingsKeys.tssrc/setupTests.tssrc/store/useAppStore.test.tssrc/store/useSettingsStore.ts
Comment on lines
+17
to
+22
| if (!window.electronAPI) { | ||
| window.electronAPI = { | ||
| onPowerSuspend: vi.fn().mockReturnValue(vi.fn()), | ||
| onPowerResume: vi.fn().mockReturnValue(vi.fn()), | ||
| } as any | ||
| } |
Contributor
There was a problem hiding this comment.
Replace as any cast with a proper partial type for electronAPI mock.
The mock is cast to any, which disables type checking. Define a type or use Partial<ElectronAPI> instead to maintain type safety.
🔧 Proposed fix to add proper typing
- if (!window.electronAPI) {
- window.electronAPI = {
- onPowerSuspend: vi.fn().mockReturnValue(vi.fn()),
- onPowerResume: vi.fn().mockReturnValue(vi.fn()),
- } as any
- }
+ if (!window.electronAPI) {
+ window.electronAPI = {
+ onPowerSuspend: vi.fn().mockReturnValue(vi.fn()),
+ onPowerResume: vi.fn().mockReturnValue(vi.fn()),
+ } as Partial<ElectronAPI>
+ }Ensure ElectronAPI is imported from src/types.d.ts at the top of the file if not already present.
🧰 Tools
🪛 GitHub Check: ci
[warning] 21-21:
Unexpected any. Specify a different type
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hooks/useReminders.test.ts` around lines 17 - 22, Replace the `as any`
type cast on the window.electronAPI mock assignment with a proper type
annotation using `Partial<ElectronAPI>` to maintain type safety instead of
disabling type checking. Ensure that ElectronAPI is imported from the
appropriate type definitions file at the top of the useReminders.test.ts file if
it is not already imported.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/hooks/useReminders.test.ts (1)
22-22:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove the
as anyfallback in the electronAPI mock.Line 22 still disables type checking (
as ... as any). Keep the mock typed asPartial<Window['electronAPI']>(or a concrete test helper type) to preserve compile-time checks.Suggested minimal diff
- } as Partial<Window['electronAPI']> as any + } as Partial<Window['electronAPI']>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/useReminders.test.ts` at line 22, Remove the `as any` type assertion from the electronAPI mock object at line 22. Change the cast from `as Partial<Window['electronAPI']> as any` to just `as Partial<Window['electronAPI']>` to preserve compile-time type checking and ensure the mock object adheres to the proper type contract without circumventing TypeScript's type system.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@electron/main.ts`:
- Around line 621-624: The baseURL parameter passed to the OpenAI client
constructor is sourced from localStorage without validation, creating a security
vulnerability where malicious URLs could redirect API requests and leak
credentials. Create a whitelist constant containing trusted provider origins
such as https://api.openai.com/v1 and https://openrouter.ai/api/v1, then
validate the baseURL against this allowlist before passing it to the OpenAI
constructor at line 623. If the baseURL is not in the allowlist or is invalid,
use undefined instead to fall back to the default endpoint.
- Around line 617-631: In the IPC handler for 'openai-chat', the catch block
currently loses error context by catching with `error: any` and rethrowing only
the message. Change the catch parameter type from `error: any` to `error:
unknown`, then safely extract the message property from the unknown error type,
and rethrow a new Error with the original error attached via the `cause` option
in the Error constructor to preserve the full error context for debugging.
In `@electron/preload.ts`:
- Line 9: The openAIChat function definition has an inline object type that
exceeds the line length limit set by prettier. Refactor the args parameter type
definition by breaking it across multiple lines: move the object type properties
(model, messages, apiKey, baseURL) each to their own line and properly format
the nested messages type to comply with prettier formatting standards.
In `@src/types.d.ts`:
- Around line 7-12: The openAIChat function in the ElectronAPI type definition
requires apiKey and baseURL as mandatory fields, but the actual handler accepts
these with defaults making them optional. Mark both apiKey and baseURL as
optional fields in the openAIChat function signature within the type definition,
and then update the corresponding implementation in electron/preload.ts to match
this optional contract so the types are consistent between the exposed API and
the actual handler implementation.
---
Duplicate comments:
In `@src/hooks/useReminders.test.ts`:
- Line 22: Remove the `as any` type assertion from the electronAPI mock object
at line 22. Change the cast from `as Partial<Window['electronAPI']> as any` to
just `as Partial<Window['electronAPI']>` to preserve compile-time type checking
and ensure the mock object adheres to the proper type contract without
circumventing TypeScript's type system.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c338c5ef-c9ce-4161-a918-79b16a3ef14e
📒 Files selected for processing (16)
electron/main.tselectron/preload.tssrc/App.tsxsrc/components/NoteSearch.tsxsrc/components/NoteTitleBar.tsxsrc/hooks/useReminders.test.tssrc/hooks/useReminders.tssrc/hooks/useVariables.test.tssrc/lib/editor/VariableScope.tssrc/lib/editor/plugins.tssrc/lib/editor/taskPlugin.tssrc/lib/editor/variablePlugin.tssrc/lib/editor/widgets.test.tssrc/lib/settingsKeys.tssrc/store/useSettingsStore.tssrc/types.d.ts
✅ Files skipped from review due to trivial changes (1)
- src/lib/settingsKeys.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- src/lib/editor/widgets.test.ts
- src/hooks/useVariables.test.ts
- src/lib/editor/VariableScope.ts
- src/store/useSettingsStore.ts
- src/lib/editor/plugins.ts
- src/lib/editor/taskPlugin.ts
- src/App.tsx
- src/components/NoteSearch.tsx
Comment on lines
+617
to
+631
|
|
||
| ipcMain.handle('openai-chat', async (_, { model, messages, apiKey, baseURL }) => { | ||
| try { | ||
| const OpenAI = (await import('openai')).default | ||
| const openai = new OpenAI({ | ||
| apiKey: apiKey || 'dummy', | ||
| baseURL: baseURL || undefined, | ||
| }) | ||
| const completion = await openai.chat.completions.create({ | ||
| model: model, | ||
| messages: messages, | ||
| }) | ||
| return completion | ||
| } catch (error: any) { | ||
| throw new Error(error.message || 'Unknown API Error') |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect declared TS/Node/Electron targets for ErrorOptions support.
fd -i '^(package\.json|tsconfig.*)$' -x sh -c 'echo "===== $1"; sed -n "1,220p" "$1"' sh {}Repository: VariableThe/PaperCache
Length of output: 4605
🏁 Script executed:
# Locate electron/main.ts file
find . -name "main.ts" -type f | head -20Repository: VariableThe/PaperCache
Length of output: 85
🏁 Script executed:
# Check the directory structure
ls -la electron/ 2>/dev/null || echo "No electron directory found in root"
find . -path "*/electron/main.ts" -type fRepository: VariableThe/PaperCache
Length of output: 322
🏁 Script executed:
# Check if there's a separate tsconfig for electron
fd -i '^tsconfig.*\.json$' | xargs cat | grep -A 5 -B 5 "electron" 2>/dev/null || echo "No electron-specific tsconfig found"Repository: VariableThe/PaperCache
Length of output: 102
🏁 Script executed:
# Read lines around 617-631 to verify the code
sed -n '610,640p' ./electron/main.tsRepository: VariableThe/PaperCache
Length of output: 774
🏁 Script executed:
# Check file size and line count
wc -l ./electron/main.tsRepository: VariableThe/PaperCache
Length of output: 89
🏁 Script executed:
# Check vite.config.ts for electron configuration
cat vite.config.ts 2>/dev/null || echo "Not in root"
find . -name "vite.config.ts" -type f -exec cat {} \;Repository: VariableThe/PaperCache
Length of output: 1658
🏁 Script executed:
# Check package.json for build process and electron handling
grep -A 5 '"build"' ./package.jsonRepository: VariableThe/PaperCache
Length of output: 457
🏁 Script executed:
# Check ESLint configuration
find . -name ".eslintrc*" -o -name "eslint.config.*" | xargs cat 2>/dev/nullRepository: VariableThe/PaperCache
Length of output: 8847
🏁 Script executed:
# Verify ErrorOptions is supported by checking if TypeScript 6.0.2 supports Error constructor with cause
python3 << 'PY'
# ES2022 and later support Error constructor with { cause } option
# TypeScript 4.6+ (2022) supports this
# The project uses TypeScript 6.0.2 and target ES2023, so ErrorOptions is fully supported
ts_version = "6.0.2"
target = "es2023"
error_options_support = True # Available since TS 4.6 and ES2022
print(f"TypeScript version: {ts_version}")
print(f"Target: {target}")
print(f"ErrorOptions ({ cause: ... }) support: {error_options_support}")
print()
print("Analysis:")
print("✓ ES2023 target supports Error constructor with cause parameter")
print("✓ TypeScript 6.0.2 supports ErrorOptions type definition")
print("✓ The proposed fix is valid and will work without fallback")
PYRepository: VariableThe/PaperCache
Length of output: 262
Preserve the caught SDK error when rethrowing.
The current code at lines 617-631 loses error context by catching error: any and rethrowing only its message. Change the catch block to use unknown type, safely derive the message, and attach the original error as cause:
Proposed fix
ipcMain.handle('openai-chat', async (_, { model, messages, apiKey, baseURL }) => {
try {
const OpenAI = (await import('openai')).default
const openai = new OpenAI({
apiKey: apiKey || 'dummy',
baseURL: baseURL || undefined,
})
const completion = await openai.chat.completions.create({
model: model,
messages: messages,
})
return completion
- } catch (error: any) {
- throw new Error(error.message || 'Unknown API Error')
+ } catch (error: unknown) {
+ const message = error instanceof Error ? error.message : 'Unknown API Error'
+ throw new Error(message, { cause: error })
}
})Your project targets ES2023 with TypeScript 6.0.2, both of which fully support ErrorOptions with the cause parameter—no fallback needed.
🧰 Tools
🪛 ESLint
[error] 617-618: Delete ⏎
(prettier/prettier)
[error] 631-631: There is no cause attached to the symptom error being thrown.
(preserve-caught-error)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@electron/main.ts` around lines 617 - 631, In the IPC handler for
'openai-chat', the catch block currently loses error context by catching with
`error: any` and rethrowing only the message. Change the catch parameter type
from `error: any` to `error: unknown`, then safely extract the message property
from the unknown error type, and rethrow a new Error with the original error
attached via the `cause` option in the Error constructor to preserve the full
error context for debugging.
Source: Linters/SAST tools
Comment on lines
+621
to
+624
| const openai = new OpenAI({ | ||
| apiKey: apiKey || 'dummy', | ||
| baseURL: baseURL || undefined, | ||
| }) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect OpenAI IPC call sites and provider/baseURL sources.
# Expectation: `baseURL` should come from a trusted provider selection or validated settings, not arbitrary renderer text.
rg -n -C4 --type=ts --type=tsx '\bopenAIChat\s*\(|\bbaseURL\b|openrouter|api\.openai|openai-chat' src electronRepository: VariableThe/PaperCache
Length of output: 94
🏁 Script executed:
#!/bin/bash
# Find and inspect the openai-chat IPC handler and baseURL usage
rg -n 'openai-chat|baseURL' --type-list | head -20Repository: VariableThe/PaperCache
Length of output: 456
🏁 Script executed:
#!/bin/bash
# List all TypeScript files in the electron directory to understand structure
find electron src -name "*.ts" -o -name "*.js" | head -20Repository: VariableThe/PaperCache
Length of output: 641
🏁 Script executed:
#!/bin/bash
# Search for openai-chat IPC handler definition
rg -n 'openai-chat' --color=never -A 10 -B 2Repository: VariableThe/PaperCache
Length of output: 1909
🏁 Script executed:
#!/bin/bash
# Search for baseURL references to understand how it's passed
rg -n 'baseURL' --color=never -B 3 -A 3Repository: VariableThe/PaperCache
Length of output: 2313
🏁 Script executed:
#!/bin/bash
# Inspect App.tsx around the openAIChat call to understand finalBaseUrl source
cat -n src/App.tsx | sed -n '300,340p'Repository: VariableThe/PaperCache
Length of output: 2374
🏁 Script executed:
#!/bin/bash
# Search for where finalBaseUrl is defined
rg -n 'finalBaseUrl' --color=never -B 5 -A 2Repository: VariableThe/PaperCache
Length of output: 1404
🏁 Script executed:
#!/bin/bash
# Search for baseURL source in settings or provider configuration
rg -n 'baseURL|baseUrl|base_url' --color=never -B 2 -A 2 src/Repository: VariableThe/PaperCache
Length of output: 656
🏁 Script executed:
#!/bin/bash
# Find where apiBaseUrl is defined in App.tsx
rg -n 'apiBaseUrl' --color=never -B 5 -A 2 src/App.tsxRepository: VariableThe/PaperCache
Length of output: 1334
Validate the renderer-supplied baseURL before creating the OpenAI client.
The baseURL at line 623 is sourced from user settings in localStorage (papercache-api-base-url) and passed directly to the OpenAI client constructor without validation. This allows an attacker to redirect API requests to arbitrary servers, leaking the API key to attacker-controlled endpoints. Add an allowlist of trusted provider origins (e.g., https://api.openai.com/v1, https://openrouter.ai/api/v1) and validate the URL against it before use.
🛡️ Suggested allowlist guard
+const ALLOWED_OPENAI_BASE_URLS = new Set([
+ 'https://api.openai.com/v1',
+ 'https://openrouter.ai/api/v1',
+])
+
+function normalizeOpenAIBaseURL(baseURL?: string) {
+ if (!baseURL) return undefined
+
+ const url = new URL(baseURL)
+ url.hash = ''
+ url.search = ''
+ const normalized = url.toString().replace(/\/$/, '')
+
+ if (!ALLOWED_OPENAI_BASE_URLS.has(normalized)) {
+ throw new Error('Unsupported OpenAI base URL')
+ }
+
+ return normalized
+}
+
ipcMain.handle('openai-chat', async (_, { model, messages, apiKey, baseURL }) => {
try {
const OpenAI = (await import('openai')).default
const openai = new OpenAI({
apiKey: apiKey || 'dummy',
- baseURL: baseURL || undefined,
+ baseURL: normalizeOpenAIBaseURL(baseURL),
})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@electron/main.ts` around lines 621 - 624, The baseURL parameter passed to the
OpenAI client constructor is sourced from localStorage without validation,
creating a security vulnerability where malicious URLs could redirect API
requests and leak credentials. Create a whitelist constant containing trusted
provider origins such as https://api.openai.com/v1 and
https://openrouter.ai/api/v1, then validate the baseURL against this allowlist
before passing it to the OpenAI constructor at line 623. If the baseURL is not
in the allowlist or is invalid, use undefined instead to fall back to the
default endpoint.
| saveNote: (id: string, content: string) => ipcRenderer.invoke('save-note', { id, content }), | ||
| deleteNote: (id: string) => ipcRenderer.invoke('delete-note', id), | ||
| renameNote: (oldId: string, newId: string) => ipcRenderer.invoke('rename-note', { oldId, newId }), | ||
| openAIChat: (args: { model: string, messages: { role: string; content: string }[], apiKey: string, baseURL: string }) => ipcRenderer.invoke('openai-chat', args), |
Contributor
There was a problem hiding this comment.
Wrap the openAIChat payload type so lint passes.
The one-line object type is currently flagged by prettier/prettier.
🧹 Proposed formatting fix
- openAIChat: (args: { model: string, messages: { role: string; content: string }[], apiKey: string, baseURL: string }) => ipcRenderer.invoke('openai-chat', args),
+ openAIChat: (args: {
+ model: string
+ messages: { role: string; content: string }[]
+ apiKey: string
+ baseURL: string
+ }) => ipcRenderer.invoke('openai-chat', args),📝 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
| openAIChat: (args: { model: string, messages: { role: string; content: string }[], apiKey: string, baseURL: string }) => ipcRenderer.invoke('openai-chat', args), | |
| openAIChat: (args: { | |
| model: string | |
| messages: { role: string; content: string }[] | |
| apiKey: string | |
| baseURL: string | |
| }) => ipcRenderer.invoke('openai-chat', args), |
🧰 Tools
🪛 ESLint
[error] 9-9: Replace ·model:·string,·messages:·{·role:·string;·content:·string·}[],·apiKey:·string,·baseURL:·string with ⏎····model:·string⏎····messages:·{·role:·string;·content:·string·}[]⏎····apiKey:·string⏎····baseURL:·string⏎·
(prettier/prettier)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@electron/preload.ts` at line 9, The openAIChat function definition has an
inline object type that exceeds the line length limit set by prettier. Refactor
the args parameter type definition by breaking it across multiple lines: move
the object type properties (model, messages, apiKey, baseURL) each to their own
line and properly format the nested messages type to comply with prettier
formatting standards.
Source: Linters/SAST tools
Comment on lines
+7
to
+12
| openAIChat: (args: { | ||
| model: string | ||
| messages: { role: string; content: string }[] | ||
| apiKey: string | ||
| baseURL: string | ||
| }) => Promise<unknown> |
Contributor
There was a problem hiding this comment.
Align optional fields with the IPC handler contract.
The main handler accepts missing apiKey/baseURL by defaulting them, but the exposed ElectronAPI type requires both. Either reject missing values in the handler or mark these optional here and mirror that shape in electron/preload.ts.
🔧 Proposed type alignment
openAIChat: (args: {
model: string
messages: { role: string; content: string }[]
- apiKey: string
- baseURL: string
+ apiKey?: string
+ baseURL?: string
}) => Promise<unknown>📝 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
| openAIChat: (args: { | |
| model: string | |
| messages: { role: string; content: string }[] | |
| apiKey: string | |
| baseURL: string | |
| }) => Promise<unknown> | |
| openAIChat: (args: { | |
| model: string | |
| messages: { role: string; content: string }[] | |
| apiKey?: string | |
| baseURL?: string | |
| }) => Promise<unknown> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/types.d.ts` around lines 7 - 12, The openAIChat function in the
ElectronAPI type definition requires apiKey and baseURL as mandatory fields, but
the actual handler accepts these with defaults making them optional. Mark both
apiKey and baseURL as optional fields in the openAIChat function signature
within the type definition, and then update the corresponding implementation in
electron/preload.ts to match this optional contract so the types are consistent
between the exposed API and the actual handler implementation.
This was referenced Jun 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refactors large monolithic App.tsx into modular components, fixes bugs in auto-calculation related to race conditions, and improves main bundle size by using dynamic import for OpenAI.
Performance changes:
Before:
After:
Summary by CodeRabbit
Release Notes