Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions .github/workflows/modelkit-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,23 @@ jobs:
fail-fast: false
matrix:
include:
- group: unit
paths: tests/unit
- group: analyze
paths: tests/unit/analyze
- group: models
paths: tests/models tests/loader tests/dataset_tests tests/export
paths: >-
tests/unit/models tests/unit/loader tests/unit/datasets
tests/unit/export
- group: optim
paths: tests/optim
paths: tests/unit/optim
- group: commands
paths: >-
tests/commands tests/config tests/build tests/compiler
tests/session tests/eval
tests/unit/commands tests/unit/config tests/unit/build
tests/unit/compiler tests/unit/session tests/unit/eval
- group: remaining
paths: >-
tests/core tests/onnx tests/cache tests/utils tests/sysinfo
tests/inspect tests/regression tests/optracing
tests/test_cli.py tests/test_text_classification.py
tests/unit/core tests/unit/onnx tests/unit/cache
tests/unit/utils tests/unit/sysinfo tests/unit/inspect
tests/unit/optracing tests/regression

name: test (${{ matrix.group }})

Expand Down
37 changes: 37 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# CLAUDE.md

## Cardinal Rules

### 1. No Hardcoded Logic

Never hardcode model architecture names, node/operator names, input/output tensor names, layer naming patterns, or any model-specific logic. All solutions must be universal and architecture-agnostic.

### 2. Pytest Only

All testing uses pytest with code-generated results. Never create standalone test scripts, use LLM-generated expectations, or generate test data manually.

### 3. Mandatory Test Verification

Run `uv run pytest tests/` after every implementation or test revision. Never assume tests pass without verification.

### 4. Never Skip Failing Tests

Investigate root cause and fix the underlying issue. Never use `pytest.mark.skip` or `xfail` to hide failures. Skips are only acceptable for hardware/EP requirements (CUDA, DirectML, AVX).

## Development Commands

- **Python**: Always use `uv run` or activate venv first. Never run bare python commands.
- **Temp files**: Use `temp/` folder in project root.
- **Node.js**: Available via fnm. Use `eval "$(fnm env)"` before npm/npx commands.

## Code Quality

- Run `uv run ruff check --fix` after revising Python code
- Follow naming rules in [`/docs/naming-convention.md`](/docs/naming-convention.md) (ONNX, EP, QDQ, Op acronym casing)
- Always ask clarifying questions before planning if requirements are ambiguous
- Critically evaluate proposals — challenge design decisions when warranted

## Git

- Never add `Co-Authored-By` when doing git commit
- Do not include "Test plan" section in PR descriptions
100 changes: 100 additions & 0 deletions docs/naming-convention.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
# ModelKit Naming Convention

This document defines the naming rules for the ModelKit codebase. All new code and refactored code must follow these conventions.

## 1. Acronyms in Class Names

Domain acronyms in PascalCase class names **retain their uppercase form**, except for two-letter abbreviations used as generic prefixes.

### Canonical Acronym Table

| Acronym | Meaning | Class Casing | Example |
|---------|---------|--------------|---------|
| ONNX | Open Neural Network Exchange | `ONNX` | `ONNXStaticAnalyzer`, `ONNXLoader` |
| EP | Execution Provider | `EP` | `EPChecker`, `EPConfig`, `EPMonitor` |
| QDQ | Quantize-Dequantize | `QDQ` | `QDQParameterConfig`, `QDQGenerator` |
| QNN | Qualcomm Neural Network | `QNN` | `QNNMonitor` |
| Op | Operator (2-letter prefix) | `Op` | `OpUnsupportedError` |
| IO | Input/Output | `IO` | `IOConfigInfo` |
| HF | HuggingFace | `HF` | `HF_MODEL_CLASS_MAPPING` |
| HTP | Hexagon Tensor Processor | `HTP` | (directory/module level) |

### Why `Op` Not `OP`

Two-letter acronyms used as **class name prefixes** use PascalCase:

- `OPUnsupported` reads ambiguously as three tokens (O-P-Unsupported)
- `OpUnsupported` reads clearly as two tokens (Op-Unsupported)
- Consistent with conventions like `Id` vs `ID`

All-caps is acceptable in **constants** (e.g., `SUPPORTED_OPS`).

## 2. Module and Package Names

Follow PEP 8: all lowercase with underscores.

```
correct: onnx_op.py, ep_checker.py, qdq_fix.py
wrong: OnnxOp.py, EP_Checker.py
```

## 3. Function and Method Names

Snake_case, lowercase.

```
correct: normalize_ep_name(), generate_build_config()
wrong: normalizeEPName(), GenerateBuildConfig()
```

## 4. Constants

UPPER_CASE with underscores.

```
correct: SUPPORTED_EPS, EP_ALIASES, DEVICE_TO_DEVICE_TYPE
wrong: supportedEps, ep_aliases
```

## 5. Directory Abbreviation Policy

The codebase uses a mix of abbreviated and full directory names. The established names are frozen — do not rename existing directories for consistency alone. For **new** directories, prefer full names unless the abbreviation is widely recognized in the domain (e.g., `optim`, `eval`, `quant`).

| Established Abbreviation | Full Form |
|---|---|
| `optim` | optimization |
| `quant` | quantization |
| `eval` | evaluation |
| `sysinfo` | system information |
| `optracing` | operator tracing |

## 6. Avoid Name Collisions Across Hierarchy

Do not reuse a parent or sibling package name at a deeper level. When creating new subpackages, verify the name does not already exist elsewhere in the tree.

Known collisions to be aware of:

| Name | Locations | Issue |
|---|---|---|
| `winml` | top-level namespace, `modelkit/winml.py`, `models/winml/` | 3-level collision |
| `core` | `modelkit/core/`, `analyze/core/` | same name, different content |
| `models` | `modelkit/models/`, `analyze/models/` | ML models vs data models |
| `utils` | `modelkit/utils/`, `analyze/utils/` | no shared content |
| `pattern` | `modelkit/pattern/`, `analyze/pattern/` | active vs near-empty |
| `inspect` | `modelkit/inspect/` | shadows Python stdlib |

## 7. Current Violations

The following classes violate the acronym naming rules and should be renamed:

| Current | Correct | File |
|---|---|---|
| `OnnxOP` | `ONNXOp` | `src/winml/modelkit/analyze/models/onnx_op.py` |
| `OnnxConfigNotFoundError` | `ONNXConfigNotFoundError` | `src/winml/modelkit/export/io.py` |
| `OnnxModelOutput` | `ONNXModelOutput` | `src/winml/modelkit/export/htp/metadata_builder.py` |
| `EpContextNodeChecker` | `EPContextNodeChecker` | `src/winml/modelkit/analyze/core/node_checkers/ep_context_node_checker.py` |
| `EpPackage` | `EPPackage` | `src/winml/modelkit/sysinfo/software.py` |
| `QdqFixResult` | `QDQFixResult` | `src/winml/modelkit/quant/qdq_fix.py` |
| `OPOptionalInputSupportError` | `OpOptionalInputSupportError` | `src/winml/modelkit/analyze/exceptions.py` |
| `OPLackOfRequiredInformationError` | `OpLackOfRequiredInformationError` | `src/winml/modelkit/analyze/exceptions.py` |
| `OPUnsupportedError` | `OpUnsupportedError` | `src/winml/modelkit/analyze/exceptions.py` |
Loading
Loading