Add CLAUDE.md development guide for VAFT project#31
Conversation
Provides Claude Code with essential context: build/install instructions, testing setup, project structure, key dependencies, database connectivity, and a comprehensive list of known code limitations and issues. https://claude.ai/code/session_01Y48mwJ3rs8pPFPtMK4Vuuh
Add missing re import bug in efit.py, stub modules (chease.py, gpec.py), backup file (efit.sav), file I/O without context managers, and additional numerical edge cases found in efit.py. https://claude.ai/code/session_01Y48mwJ3rs8pPFPtMK4Vuuh
Add findings from full codebase scan: 374 print statements, empty test files, hardcoded server paths in workflows and tests, unimplemented function stubs, 2891-line deprecated old_codeset.py, and updated type hint coverage stats (26% of files). https://claude.ai/code/session_01Y48mwJ3rs8pPFPtMK4Vuuh
|
@copilot Review |
|
@Hikitonic I've opened a new pull request, #32, to work on those changes. Once the pull request is ready, I'll request review from you. |
There was a problem hiding this comment.
Pull request overview
This PR adds a comprehensive development guide (CLAUDE.md) for the VAFT (Versatile Analytical Framework for Tokamak) project. The document serves as a centralized reference for developers working on the codebase, documenting the project structure, setup instructions, testing procedures, and most notably, a detailed catalog of known technical debt and code quality issues.
Changes:
- Adds CLAUDE.md documenting project overview, build/install instructions, testing, code style, and project structure
- Documents 15+ categories of technical debt including SQL injection vulnerabilities, missing imports, resource leaks, and infrastructure gaps
- Provides dependency table, database connectivity details, and known limitations with specific file references
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # With dev dependencies (pytest, black, flake8) | ||
| pip install -e ".[dev]" |
There was a problem hiding this comment.
The installation command for dev dependencies is incorrect. The project uses [dependency-groups] in pyproject.toml (line 60-66), not [project.optional-dependencies]. The pip install -e ".[dev]" syntax only works with optional-dependencies. For dependency-groups, users should use uv or convert the dev dependencies to optional-dependencies. Consider either: (1) documenting the correct uv command like uv pip install -e . --group dev, or (2) moving dev dependencies to [project.optional-dependencies] to support the pip syntax shown here.
| # With dev dependencies (pytest, black, flake8) | |
| pip install -e ".[dev]" | |
| # With dev dependencies (pytest, black, flake8) using dependency groups | |
| uv pip install -e . --group dev |
| 1. **HSDS server** (HDF5 over HTTP) — for processed ODS data | ||
| - Configure with `hsconfigure` command | ||
| - Default endpoint: `http://147.46.36.244:5101` | ||
| - Public reader credentials: username `reader`, password `test` |
There was a problem hiding this comment.
The documentation includes actual database credentials (username 'reader', password 'test') for the public HSDS server. While these are described as "public reader credentials," consider whether it's appropriate to include actual passwords in developer documentation, even for read-only access. If these are truly intended for public use, consider clarifying that these credentials are safe to share publicly to avoid confusion about security practices.
| - Public reader credentials: username `reader`, password `test` | |
| - Public reader credentials (intentionally public, read-only access to non-sensitive HSDS data; safe to share in examples): username `reader`, password `test` |
| - Division by zero not guarded in several equilibrium/profile calculations (e.g., `psi_boundary == psi_axis`) | ||
| - `np.errstate` used to suppress warnings rather than handling edge cases | ||
| - Matrix exponential in eddy current solver can overflow for large eigenvalues | ||
| - `efit.py`: normalizes data by dividing by min/max without zero checks |
There was a problem hiding this comment.
The PR description mentions including a Claude.ai session link (https://claude.ai/code/session_01Y48mwJ3rs8pPFPtMK4Vuuh) as part of notable details, but this link is not present in the CLAUDE.md file. Consider whether this session link should be included in the document (perhaps in a "Development History" or "References" section) for traceability, or if it was intentionally excluded to keep the documentation focused on technical content.
| - `efit.py`: normalizes data by dividing by min/max without zero checks | |
| - `efit.py`: normalizes data by dividing by min/max without zero checks | |
| ## Development History | |
| - Claude.ai session used for codebase analysis and documentation support: | |
| https://claude.ai/code/session_01Y48mwJ3rs8pPFPtMK4Vuuh |
| | `snakemake` | Workflow automation | | ||
| | `astropy` | Physical units and constants | | ||
|
|
||
| A vendored copy of OMAS is used (`vendor/omas/`, editable install via uv). |
There was a problem hiding this comment.
The statement "A vendored copy of OMAS is used (vendor/omas/, editable install via uv)" is accurate but incomplete. According to pyproject.toml lines 79-82, the vendor/omas path is specifically configured via uv.sources and is only used when installing the 'imas-python' extra. For standard installations, OMAS is fetched from PyPI (line 29: omas>=0.94.2). Consider clarifying that the vendored copy is only used with uv and the imas-python extra, not for all installations.
| A vendored copy of OMAS is used (`vendor/omas/`, editable install via uv). | |
| By default, OMAS is installed from PyPI (`omas>=0.94.2`). The vendored copy in `vendor/omas/` is only used via `uv.sources` when installing the `imas-python` extra (e.g. editable installs with uv). |
|
@copilot open a new pull request to apply changes based on the comments in this thread |
Summary
This PR adds a comprehensive development guide (
CLAUDE.md) for the VAFT (Versatile Analytical Framework for Tokamak) project. The document serves as a reference for developers working on the codebase, covering project structure, setup, testing, and known issues.Key Changes
blackandflake8with no configuration filesdatabase/raw.pyremodule incode/efit.py)__all__definitionsprint()statements)Notable Details
The guide identifies 15+ categories of technical debt and code quality issues, providing specific file locations and examples. This serves as both documentation and a roadmap for future refactoring efforts.
https://claude.ai/code/session_01Y48mwJ3rs8pPFPtMK4Vuuh