Conversation
Backend: Replace @requires_lock with @check_lock decorator
- Operations proceed unless blocked by another session's lock
- FIFO ordering handles concurrency when no lock exists
- Fine-grained locks: geometry:{key}, step, bookmarks, global
- Remove deprecated @requires_lock decorator
Frontend: Remove automatic lock acquisition
- Simplify createGeometry/deleteGeometry - remove withAutoLock
- Remove withAutoLock function from client.ts
- Simplify useStepControl - remove auto-lock acquisition
- Delete useLockManager.ts hook (no longer needed)
- Remove lockToken parameter from geometry persistence
- Add 423 error handling with snackbar notifications
- Fix GeometryListResponse type to include types property
Bug fix:
- Fix AttributeError in socket_manager.py bookmark exception handling
Tests:
- Add test_check_lock_decorator.py for new behavior
- Update test_step_endpoints.py for @check_lock semantics
- Update test_partial_frame_update.py
- Remove test_requires_lock_decorator.py
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
for more information, see https://pre-commit.ci
📝 WalkthroughWalkthroughAdds React lazy-loading and Suspense fallbacks for several UI components, adds Vite manualChunks for bundle splitting, defers heavy Python imports into analysis/building run methods, and introduces a new Distance analysis extension that computes inter-atomic distances and produces a Plotly figure. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as React UI
participant Server as Analysis Runner
participant Plotly as Plotly (fig)
participant Vis as Vis Manager
User->>UI: Open dialog / trigger Distance analysis
UI->>UI: React.lazy loads component (Suspense shows CircularProgress)
UI->>Server: submit analysis job with selected atoms
Server->>Server: (run) import pandas, plotly.express locally
Server->>Plotly: compute distances and build figure
Plotly-->>Server: returns figure object (with data, customdata, meta)
Server->>Vis: store figure in vis.figures["Distance"]
Vis-->>UI: updated figures list / event
UI->>User: render figure viewer with interactive Plotly figure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used📓 Path-based instructions (2)**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/test_*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧬 Code graph analysis (2)src/zndraw/extensions/analysis.py (3)
tests/test_vis_figures.py (1)
🪛 Ruff (0.14.11)src/zndraw/extensions/analysis.py41-41: Avoid specifying long messages outside the exception class (TRY003) tests/test_vis_figures.py382-382: Unused function argument: (ARG001) 424-424: Unused function argument: (ARG001) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (6)
✏️ Tip: You can disable this entire section by setting 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/src/pages/landingPage.tsx`:
- Around line 40-47: The file uses React.lazy to define ChatWindow before
importing React, causing a TDZ runtime error; move the React import (import
React, { useState, useEffect, Suspense } from "react") to the top of the import
block so React is available before calling React.lazy (affecting the ChatWindow
declaration), or alternatively replace React.lazy usage with a direct named
import pattern only after confirming React is imported first; ensure references
to ChatWindow, useState, useEffect, and Suspense remain correct after
reordering.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/src/components/WindowManager.tsxapp/src/components/jsonforms-renderers/CustomSmilesEditor.tsxapp/src/components/jsonforms-renderers/CustomSmilesPackEditor.tsxapp/src/pages/landingPage.tsxapp/vite.config.tssrc/zndraw/extensions/analysis.pysrc/zndraw/extensions/molecule_building.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: You MUST NEVER@pytest.mark.xfailor similar - all tests must pass!
All default values (e.g.,camera,particles) must be defined exclusively within the Pydantic model. Do not scatter fallback logic throughout the codebase.
Do not perform null checks combined with hardcoded literals for default values. Rely entirely on the schema to populate default values during initialization.
You can not use LUA scripts with Redis!
If sensible, implement collections.abc interfaces for your classes, such as MutableMapping or MutableSequence.
Use numpy style docstrings in Python code.
Docstrings must be concise and to the point.
Use type hints wherever possible in Python. Uselist[int|float] | Noneinstead oft.Optional[t.List[int|float]]!
Imports should always be at the top of the file in Python.
Files:
src/zndraw/extensions/analysis.pysrc/zndraw/extensions/molecule_building.py
🧬 Code graph analysis (4)
app/src/components/jsonforms-renderers/CustomSmilesEditor.tsx (1)
app/src/components/jsonforms-renderers/SmilesEditDialog.tsx (1)
SmilesEditDialog(43-109)
app/src/components/jsonforms-renderers/CustomSmilesPackEditor.tsx (1)
app/src/components/jsonforms-renderers/SmilesEditDialog.tsx (1)
SmilesEditDialog(43-109)
app/src/components/WindowManager.tsx (1)
app/src/stores/windowManagerStore.ts (1)
useWindowManagerStore(27-132)
src/zndraw/extensions/molecule_building.py (1)
src/zndraw/zndraw.py (1)
log(1720-1722)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: pytest (3.13, ubuntu-latest)
- GitHub Check: pytest (3.11, ubuntu-latest)
- GitHub Check: pytest (3.12, ubuntu-latest)
🔇 Additional comments (17)
src/zndraw/extensions/molecule_building.py (3)
21-22: Lazy import deviates from coding guidelines.The lazy import pattern conflicts with the guideline stating "Imports should always be at the top of the file in Python." However, for optional heavy dependencies like
molify, this is a recognized optimization pattern that improves load times and allows graceful handling when the dependency isn't installed.Please confirm this deviation is acceptable for performance-critical optional dependencies.
25-25: LGTM!The markdown fenced code block with
smileslanguage identifier will render nicely in the chat interface, improving readability of the SMILES notation.
52-54: LGTM!Consistent lazy import pattern with
AddFromSMILES.run. Same guideline consideration applies as noted above.src/zndraw/extensions/analysis.py (4)
32-34: Lazy imports approved for performance optimization.Moving
pandasandplotly.expressimports into therunmethods is a valid pattern for deferring heavy dependency loading until needed. Python caches imported modules, so subsequent calls incur minimal overhead.Note: This intentionally deviates from the coding guideline stating "Imports should always be at the top of the file in Python." Given the explicit PR objective of lazy imports for faster load times, this is an acceptable exception.
126-128: Consistent lazy import pattern.
219-221: Consistent lazy import pattern.
330-332: Consistent lazy import pattern.app/src/components/WindowManager.tsx (3)
1-7: LGTM! Clean lazy loading setup for Plotly.The
React.lazyimport with the descriptive comment about the 4.8MB Plotly bundle is helpful for understanding the motivation behind this optimization.
14-14: Good optimization: early return prevents unnecessary Suspense boundary.Returning
nullwhen there are no open windows avoids mounting the Suspense boundary unnecessarily, which is a small but worthwhile optimization.
17-35: Suspense fallback implementation looks good.The centered CircularProgress with
zIndex: 9999provides clear loading feedback. The Suspense boundary correctly wraps all FigureWindow instances.app/src/pages/landingPage.tsx (1)
528-547: Lazy loading pattern for ChatWindow is consistent with the rest of the codebase.The Suspense boundary with centered CircularProgress fallback matches the pattern used in WindowManager and the SMILES editors.
app/src/components/jsonforms-renderers/CustomSmilesEditor.tsx (2)
7-13: LGTM! Correct lazy loading setup.Imports are properly ordered, and the lazy-loaded
SmilesEditDialogwith the Ketcher editor will only load when the user clicks "Draw".
100-125: Good pattern: conditional render triggers lazy load only when needed.The
{dialogOpen && <Suspense>...}pattern ensures the Ketcher module is fetched only when the user opens the dialog, maximizing the performance benefit.app/src/components/jsonforms-renderers/CustomSmilesPackEditor.tsx (3)
6-13: LGTM! Imports are correctly structured.The
lazyandSuspenseimports along withCircularProgressare properly positioned before usage.Also applies to: 22-22
36-37: Good: Same lazy-loading pattern as CustomSmilesEditor.Both SMILES editor components now share the same deferred loading strategy for
SmilesEditDialog, ensuring consistency across the codebase.
356-384: Suspense implementation is consistent and correct.The conditional rendering with Suspense boundary matches the pattern in
CustomSmilesEditor.tsx, ensuring the Ketcher module loads only when the edit dialog is opened.app/vite.config.ts (1)
40-66: Manual chunk configuration is correct and well-organized.The chunking strategy effectively separates heavy dependencies into cacheable groups, which aligns well with the PR's lazy loading objectives. The comments explaining each chunk's purpose are helpful for maintainability.
Both
plotly.jsandplotly.js-dist-minare legitimately used in the codebase:FigureWindow.tsximports the minified distribution for runtime functionality while also importing internal modules directly from the fullplotly.jspackage (plotly.js/src/lib/array.jsfor thedecodeTypedArraySpecutility). Including both in the chunk is correct.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Add new analysis extension that calculates distances between 2 selected atoms across all frames with interactive frame selection support. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Move React import before React.lazy usage to prevent temporal dead zone runtime error when loading ChatWindow component. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #848 +/- ##
==========================================
- Coverage 80.02% 79.96% -0.07%
==========================================
Files 165 165
Lines 20084 20143 +59
==========================================
+ Hits 16073 16108 +35
- Misses 4011 4035 +24 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
Lazy import python modules for faster load times
split and lazy load js modules for faster site load times
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.