[PR #60] [MERGED] docs: add AGENTS.md covering codegen drift and the required checks #62

New issue

Closed

opened 2026-10-31 22:01:05 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 22:01:05 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfy-python-sdk/pull/60
Author: @christian-byrne
Created: 8/19/2026
Status: ✅ Merged
Merged: 8/23/2026
Merged by: @christian-byrne

Base: main ← Head: docs/agents-md

📝 Commits (1)

813e23a docs: add AGENTS.md covering codegen drift and the required checks

📊 Changes

1 file changed (+173 additions, -0 deletions)

View changed files

➕ AGENTS.md (+173 -0)

📄 Description

Closes #58

Adds AGENTS.md. Every claim is sourced from .github/workflows/ci.yml, scripts/, or pyproject.toml — no generic filler, and it does not restate pyproject.toml.

The two traps it exists for

1. Hand-editing a generated file passes local review and fails codegen-drift.
src/comfy_low/models/_generated.py is committed, so it reads as ordinary source, and nothing local objects: ruff has it in extend-exclude, ruff format skips it for the same reason, and mypy lists it in exclude. scripts/check_drift.py then regenerates it and compares byte for byte, so the edit only surfaces in CI. The file names all three exclusions so the reason it slips through is explicit, and flags that spec/openapi.yaml is a one-way vendored copy — editing the spec to make drift pass is the wrong fix in the other direction.

Reviewer check: pyproject.toml [tool.ruff] extend-exclude, [tool.mypy] exclude, and the byte comparison in scripts/check_drift.py.

2. mypy is required and undiscoverable.
There is no typecheck script, no Makefile, no pre-commit config — mypy src exists only as a step in ci.yml. The file lists all four required commands in CI order and notes it is mypy src, not the whole repo.

Reviewer check: the test job steps in .github/workflows/ci.yml.

Also covered

Generated vs hand-written table. models/__init__.py is hand-written and codegen does not touch it, so a new schema needs its import list and __all__ updated by hand.
Regenerate command, including that gen_models.sh is bash — python scripts/gen_models.sh (as README.md currently suggests) does not work — and why the datamodel-code-generator ~=0.68.1 pin is load-bearing for a byte-for-byte gate.
uv invocations: uv run --extra dev <cmd>. Dev tools are an extra, not a dependency group, so they are outside the default sync set. Notes that CI itself uses pip install -e .[dev].
public-repo-hygiene: all three categories from scripts/check_public_repo_hygiene.py (ticket-shaped IDs, internal collaboration-tool links, default-deny Comfy-Org/<repo> and @Comfy-Org/<team> allowlists), plus the rule that a false positive means an allowlist entry with a comment, never a looser regex.
build-check: the sdist include list is explicit, so a new top-level directory silently will not ship.
Adding a contract operation is a four-file change — tests/test_spec_coverage.py requires OPERATION_IDS, OPERATION_METHODS, and a method on both ComfyLow and AsyncComfyLow.
filterwarnings promotes deprecations to errors; the suite is network-free via the conftest.py stub server; the gateway e2e test is collected but skips without COMFY_BASE_URL + COMFY_API_KEY; __version__ comes from dist metadata so it must not be hardcoded.

python3 scripts/check_public_repo_hygiene.py passes on the new file.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/comfy-python-sdk/pull/60 **Author:** [@christian-byrne](https://github.com/christian-byrne) **Created:** 8/19/2026 **Status:** ✅ Merged **Merged:** 8/23/2026 **Merged by:** [@christian-byrne](https://github.com/christian-byrne) **Base:** `main` ← **Head:** `docs/agents-md` --- ### 📝 Commits (1) - [`813e23a`](https://github.com/Comfy-Org/comfy-python-sdk/commit/813e23a277e3d319feeddec60711651d40c62ea5) docs: add AGENTS.md covering codegen drift and the required checks ### 📊 Changes **1 file changed** (+173 additions, -0 deletions) <details> <summary>View changed files</summary> ➕ `AGENTS.md` (+173 -0) </details> ### 📄 Description Closes #58 Adds `AGENTS.md`. Every claim is sourced from `.github/workflows/ci.yml`, `scripts/`, or `pyproject.toml` — no generic filler, and it does not restate `pyproject.toml`. ## The two traps it exists for **1. Hand-editing a generated file passes local review and fails `codegen-drift`.** `src/comfy_low/models/_generated.py` is committed, so it reads as ordinary source, and *nothing local objects*: ruff has it in `extend-exclude`, `ruff format` skips it for the same reason, and mypy lists it in `exclude`. `scripts/check_drift.py` then regenerates it and compares byte for byte, so the edit only surfaces in CI. The file names all three exclusions so the reason it slips through is explicit, and flags that `spec/openapi.yaml` is a one-way vendored copy — editing the spec to make drift pass is the wrong fix in the other direction. Reviewer check: `pyproject.toml` `[tool.ruff] extend-exclude`, `[tool.mypy] exclude`, and the byte comparison in `scripts/check_drift.py`. **2. mypy is required and undiscoverable.** There is no `typecheck` script, no Makefile, no pre-commit config — `mypy src` exists only as a step in `ci.yml`. The file lists all four required commands in CI order and notes it is `mypy src`, not the whole repo. Reviewer check: the `test` job steps in `.github/workflows/ci.yml`. ## Also covered - Generated vs hand-written table. `models/__init__.py` is **hand-written** and codegen does not touch it, so a new schema needs its import list and `__all__` updated by hand. - Regenerate command, including that `gen_models.sh` is bash — `python scripts/gen_models.sh` (as `README.md` currently suggests) does not work — and why the `datamodel-code-generator ~=0.68.1` pin is load-bearing for a byte-for-byte gate. - uv invocations: `uv run --extra dev <cmd>`. Dev tools are an **extra**, not a dependency group, so they are outside the default sync set. Notes that CI itself uses `pip install -e .[dev]`. - `public-repo-hygiene`: all three categories from `scripts/check_public_repo_hygiene.py` (ticket-shaped IDs, internal collaboration-tool links, default-deny `Comfy-Org/<repo>` and `@Comfy-Org/<team>` allowlists), plus the rule that a false positive means an allowlist entry with a comment, never a looser regex. - `build-check`: the sdist `include` list is explicit, so a new top-level directory silently will not ship. - Adding a contract operation is a four-file change — `tests/test_spec_coverage.py` requires `OPERATION_IDS`, `OPERATION_METHODS`, and a method on both `ComfyLow` and `AsyncComfyLow`. - `filterwarnings` promotes deprecations to errors; the suite is network-free via the `conftest.py` stub server; the gateway e2e test is collected but skips without `COMFY_BASE_URL` + `COMFY_API_KEY`; `__version__` comes from dist metadata so it must not be hardcoded. `python3 scripts/check_public_repo_hygiene.py` passes on the new file. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 22:01:05 +00:00