Skip to content

Add claude.md#162

Merged
yuecideng merged 2 commits intomainfrom
yueci/claude
Mar 5, 2026
Merged

Add claude.md#162
yuecideng merged 2 commits intomainfrom
yueci/claude

Conversation

@yuecideng
Copy link
Contributor

@yuecideng yuecideng commented Mar 5, 2026

Description

Add CLAUDE.md for Embodichain.

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist

  • I have run the black . command to format the code base.
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • Dependencies have been updated, if applicable.

Copilot AI review requested due to automatic review settings March 5, 2026 07:47
@yuecideng yuecideng merged commit 57ee03e into main Mar 5, 2026
4 checks passed
@yuecideng yuecideng deleted the yueci/claude branch March 5, 2026 07:47
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds Claude Code–oriented contributor guidance to the repository by introducing a root CLAUDE.md developer reference and linking to it from the contribution guide.

Changes:

  • Add a new CLAUDE.md developer reference describing repository structure, style, and testing guidance.
  • Update CONTRIBUTING.md with instructions and example workflows for using Claude Code during contributions.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File Description
CONTRIBUTING.md Adds a “Using Claude Code for Contributions” section with setup + suggested prompts.
CLAUDE.md New repo-wide developer reference for Claude Code (structure, style, conventions, tests).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

**Understand a bug**

```
> I'm getting a KeyError in observation_manager.py at line 42 when env_ids is None.
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

The example prompt hard-codes a line number in observation_manager.py ("at line 42"). That line number is already not where a KeyError would occur and will drift over time, making the guidance misleading. Consider removing the line number and referencing a function name or just the file + traceback instead.

Suggested change
> I'm getting a KeyError in observation_manager.py at line 42 when env_ids is None.
> I'm getting a KeyError in observation_manager.py when env_ids is None. The traceback points into the observation lookup logic.

Copilot uses AI. Check for mistakes.
### Type Annotations

- Use full type hints on all public APIs.
- Use `from __future__ import annotations` at the top of every file.
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

This claims from __future__ import annotations should be at the top of every file, but several core modules (e.g. embodichain/lab/gym/envs/base_env.py, embodied_env.py, rl_env.py) do not include it. To avoid establishing an inaccurate convention, either update these files to match or soften the guidance (e.g. "prefer for new files" / "used in many modules").

Suggested change
- Use `from __future__ import annotations` at the top of every file.
- Prefer `from __future__ import annotations` at the top of new Python files and when updating existing modules, where feasible.

Copilot uses AI. Check for mistakes.
Comment on lines +67 to +74
Every source file begins with the Apache 2.0 copyright header:

```python
# ----------------------------------------------------------------------------
# Copyright (c) 2021-2026 DexForce Technology Co., Ltd.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# ...
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

“Every source file begins with the Apache 2.0 copyright header” is not currently true across the repo (for example docs/source/conf.py does not include this header). Consider scoping this to the directories that actually follow it (e.g. embodichain/ and tests/) or updating the other files to match.

Suggested change
Every source file begins with the Apache 2.0 copyright header:
```python
# ----------------------------------------------------------------------------
# Copyright (c) 2021-2026 DexForce Technology Co., Ltd.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# ...
Python source files in the `embodichain/` and `tests/` directories must begin with the Apache 2.0 copyright header:
```python
# ----------------------------------------------------------------------------
# Copyright (c) 2021-2026 DexForce Technology Co., Ltd.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

Copilot uses AI. Check for mistakes.
│ └── warp/kinematics/ # GPU kinematics via Warp
├── configs/ # Agent configs and task prompts (text/YAML)
├── docs/ # Sphinx documentation source + build
│ └── source/ # .md doc pages (overview, quick_start, features, resources)
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

The project-structure comment says docs/source/ contains “.md doc pages”, but the docs tree is a mix of .rst and .md (with MyST enabled). Adjusting this wording would better reflect the actual documentation setup.

Suggested change
│ └── source/ # .md doc pages (overview, quick_start, features, resources)
│ └── source/ # Sphinx doc sources (.rst and .md via MyST; overview, quick_start, features, resources)

Copilot uses AI. Check for mistakes.

### Documentation

- Docs are built with **Sphinx** using **Markdown** source files (`docs/source/`).
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

This documentation note says the docs are built with Sphinx using Markdown source files, but the repo uses both .rst and .md in docs/source/ (with MyST). Consider updating the wording to “Markdown and reStructuredText” to avoid confusing contributors.

Suggested change
- Docs are built with **Sphinx** using **Markdown** source files (`docs/source/`).
- Docs are built with **Sphinx** using **Markdown and reStructuredText** source files (`docs/source/`).

Copilot uses AI. Check for mistakes.
```python
from embodichain.lab.gym.envs.managers import Functor, FunctorCfg

class my_randomizer(Functor):
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

In the Functor example, class my_randomizer(Functor) uses a lowercase class name, which conflicts with standard Python class naming and can be confused with the function-style functor described above. Consider using a MyRandomizer-style class name (or make this example a plain function) to keep the pattern clear.

Suggested change
class my_randomizer(Functor):
class MyRandomizer(Functor):

Copilot uses AI. Check for mistakes.
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.

2 participants