Skip to content

Update documentation to reflect dynamic geometry editing and clarify technical details#853

Merged
PythonFZ merged 3 commits intomainfrom
copilot/fix-docs-statements
Jan 16, 2026
Merged

Update documentation to reflect dynamic geometry editing and clarify technical details#853
PythonFZ merged 3 commits intomainfrom
copilot/fix-docs-statements

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jan 16, 2026

Removes outdated limitations and adds clarifications to the ZnDraw documentation based on current functionality.

Changes

  • Remove static-only editing restriction: Both python-api.rst and keyboard-shortcuts.rst incorrectly stated that only geometries with static positions (number arrays) could be edited. Dynamic positions (e.g., "arrays.positions") now update the underlying ase.Atoms object when transformed.

  • Remove axis color mapping: Deleted "(red for X, green for Y, blue for Z)" from axis constraint description, as this detail is redundant with the visual indicator already shown in screenshots.

  • Clarify dynamic properties source: Updated description to explicitly state that dynamic property references (arrays.positions, calc.energy, etc.) are computed from atoms.info, atoms.arrays, and atoms.calc.results dictionaries.

  • Add Ketcher dark mode notice: Added note that Ketcher editor lacks dark mode support with link to upstream issue #5353.

  • Update code comment: Changed # Auto-saves to backend to # adjust settings for accuracy.

Original prompt

This section details on the original issue you should resolve

<issue_title>fix docs statements</issue_title>
<issue_description>- Only geometries with static positions (number arrays) can be edited. Geometries using dynamic positions (e.g., "arrays.positions") cannot be transformed interactively. See [Keyboard Shortcuts](https://zndraw.readthedocs.io/en/latest/keyboard-shortcuts.html) for all controls. this is no longer true, dynamic can also be updated and will update the e.g. positions in the ase.Atoms object.

  • remove the color info (red for X, green for Y, blue for Z)
  • Available dynamic property references: <- these are dynamic and are computed from the atoms.info, atoms.arrays and if available atoms.calc.results dictionaries.
  • Ketcher Editor <- add a note, that there is no dark mode for ketcher (Is there a dark mode for Ketcher epam/ketcher#5353)
  • replace # Auto-saves to backend with # adjust settings
    </issue_description>

<agent_instructions>Update the documentation with these criteria</agent_instructions>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Summary by CodeRabbit

Release Notes

  • Documentation

    • Updated keyboard shortcuts guide by removing outdated editing limitation note
    • Expanded Python API documentation with details on dynamic property references and custom filesystem registration examples
    • Added Ketcher editor dark mode support information in relevant sections
  • Performance

    • Optimized application startup by deferring loading of heavy modules

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jan 16, 2026

📝 Walkthrough

Walkthrough

This PR updates documentation to reflect that dynamic geometries can now be edited, improves lazy-loading semantics in the codebase standards, clarifies dynamic property references in API docs, adds dark mode limitation notes for Ketcher, and optimizes the build configuration to defer loading of heavy modules like ketcher via React.lazy() instead of pre-bundling.

Changes

Cohort / File(s) Summary
Build Configuration Optimization
app/vite.config.ts
Removed ketcher modules from optimizeDeps.include to prevent pre-bundling; added module preload resolveDependencies hook to filter out heavy chunks (ketcher, plotly, ChatWindow) for on-demand loading; removed explicit ketcher chunk from manualChunks, relying instead on React.lazy() and code-splitting for lazy-loaded dependencies
Documentation — Geometry Editing
docs/source/keyboard-shortcuts.rst
Removed outdated note stating only static-position geometries can be edited; dynamic positions can now be transformed interactively
Documentation — Python API
docs/source/python-api.rst
Refined axis constraint description (removed color notation); clarified dynamic property references as computed from atoms.info, atoms.arrays, and atoms.calc.results; added dark mode limitation note for Ketcher editor; updated UI sessions comment from "Auto-saves to backend" to "adjust settings"; added new Custom Filesystems section with fsspec example; enhanced options table with additional entries
Code Standards
AGENTS.md
Updated import placement rule to allow lazy loading exception when imports affect startup time

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • lazy imports for better performance #848: Both PRs modify app/vite.config.ts to address lazy-loading and chunking strategies for ketcher, though with different approaches (this PR removes pre-bundling/manual chunks in favor of React.lazy).

Poem

🐰 Hop hop, the docs now dance so true,
Dynamic geometries edit, fresh and new!
Heavy chunks lazy-load with care,
Ketcher dreams in light, not darkness there. 🌙✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The PR includes out-of-scope changes: AGENTS.md import rule modification and vite.config.ts lazy loading optimizations are not mentioned in linked issue #852 and appear unrelated to documentation updates. Either document the rationale for AGENTS.md and vite.config.ts changes in the PR description, or move them to a separate PR focused on build optimization and coding standards.
Title check ❓ Inconclusive The title is partially related to the changeset; it mentions documentation updates and technical clarifications, which aligns with some changes but overlooks the significant vite.config.ts modifications for lazy loading optimization. Consider revising the title to better reflect the full scope of changes, particularly the vite.config.ts optimizations for lazy loading and dependency pre-bundling.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed The PR addresses all coding requirements from issue #852: removes outdated geometry editing statements, deletes axis color mapping text, clarifies dynamic property references, adds Ketcher dark mode note, and updates the code comment.
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


📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 062ff05 and 2c027d8.

📒 Files selected for processing (4)
  • AGENTS.md
  • app/vite.config.ts
  • docs/source/keyboard-shortcuts.rst
  • docs/source/python-api.rst
💤 Files with no reviewable changes (1)
  • docs/source/keyboard-shortcuts.rst
🧰 Additional context used
📓 Path-based instructions (1)
AGENTS.md

📄 CodeRabbit inference engine (CLAUDE.md)

Document agent behavior and interactions in AGENTS.md

Files:

  • AGENTS.md
🧠 Learnings (2)
📚 Learning: 2026-01-11T19:25:23.795Z
Learnt from: CR
Repo: zincware/ZnDraw PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-11T19:25:23.795Z
Learning: Applies to **/*.py : Imports should always be at the top of the file in Python.

Applied to files:

  • AGENTS.md
📚 Learning: 2026-01-11T19:25:23.795Z
Learnt from: CR
Repo: zincware/ZnDraw PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-11T19:25:23.795Z
Learning: Do not create markdown files, unless specifically requested!

Applied to files:

  • AGENTS.md
⏰ 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 (10)
AGENTS.md (1)

77-77: LGTM!

The exception for lazy loading is well-justified and aligns with the Vite configuration changes that defer loading of heavy dependencies like Ketcher. The guideline remains clear while accommodating legitimate performance optimization needs.

docs/source/python-api.rst (6)

431-431: LGTM!

Removal of the redundant axis color mapping text "(red for X, green for Y, blue for Z)" aligns with issue #852. The visual indication is self-explanatory from the UI screenshots.


447-449: LGTM!

Streamlined note that references the keyboard-shortcuts documentation rather than duplicating information here.


486-495: LGTM!

Excellent clarification that dynamic property references are computed from atoms.info, atoms.arrays, and atoms.calc.results dictionaries. This directly addresses issue #852 and helps users understand where available properties originate.


579-582: LGTM!

Helpful note about Ketcher's dark mode limitation with a direct link to the upstream issue for users who want to track progress.


779-779: LGTM!

Comment updated as specified in issue #852.


969-981: LGTM!

Clear documentation for the custom filesystems feature. The example with DirFileSystem is practical, and mentioning fsspec compatibility with S3, GCS, Azure, and HDFS helps users understand the breadth of supported backends.

app/vite.config.ts (3)

26-26: LGTM!

Clear comment documenting the intentional exclusion of ketcher from pre-bundling to support lazy loading.


57-58: LGTM!

Good documentation explaining that ketcher will naturally code-split via React.lazy() with the SmilesEditDialog component rather than requiring explicit chunk configuration.


37-48: LGTM!

The modulePreload.resolveDependencies filter correctly prevents preloading of heavy chunks. The filter logic properly returns dependencies that don't match the heavy chunk names, ensuring ketcher, plotly, and ChatWindow load only when needed.

ChatWindow uses React.lazy() with dynamic imports (import("../components/ChatWindow")), which naturally code-splits into a separate chunk. The modulePreload filter is appropriate here since ChatWindow should only be loaded on-demand when the chat is opened, not preloaded.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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.

❤️ Share

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

Co-authored-by: PythonFZ <46721498+PythonFZ@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix documentation statements for geometry editing Update documentation to reflect dynamic geometry editing and clarify technical details Jan 16, 2026
Copilot AI requested a review from PythonFZ January 16, 2026 08:29
@PythonFZ PythonFZ marked this pull request as ready for review January 16, 2026 09:09
Copy link
Copy Markdown
Member

@PythonFZ PythonFZ left a comment

Choose a reason for hiding this comment

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

LGTM once tests pass

@PythonFZ
Copy link
Copy Markdown
Member

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jan 16, 2026

✅ 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.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jan 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.98%. Comparing base (062ff05) to head (2c027d8).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #853   +/-   ##
=======================================
  Coverage   79.97%   79.98%           
=======================================
  Files         165      165           
  Lines       20143    20143           
=======================================
+ Hits        16110    16112    +2     
+ Misses       4033     4031    -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@PythonFZ PythonFZ merged commit 3328203 into main Jan 16, 2026
6 checks passed
@PythonFZ PythonFZ deleted the copilot/fix-docs-statements branch January 16, 2026 10:11
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.

fix docs statements

3 participants