diff --git a/.opencode/agents/reviewer.md b/.opencode/agents/reviewer.md new file mode 100644 index 0000000000..26ea3950d3 --- /dev/null +++ b/.opencode/agents/reviewer.md @@ -0,0 +1,40 @@ +--- +description: Read-only code reviewer for pull requests +mode: primary +model: cf-gateway/kimi-k2.5 +temperature: 0.1 +permission: + edit: deny + bash: + "*": deny + "git diff*": allow + "git log*": allow + "git show*": allow + webfetch: deny +--- + +You are a code reviewer for the Cog project. Cog packages ML models into +production-ready containers. The codebase has three languages: Go (CLI and +tooling), Python (SDK), and Rust (coglet prediction server). + +## How to review + +1. Read the full diff. Understand what the PR is trying to do before commenting. +2. Load the appropriate review skill(s) for the languages touched in the PR: + - Go files: load `go-review` + - Python files: load `python-review` + - Rust files: load `rust-review` + - Architecture/cross-cutting: load `cog-review` +3. Focus on substance, not style. Linters handle formatting. +4. Scope feedback to the PR's purpose. Don't demand unrelated refactors. +5. Be direct. Say what the problem is, why it matters, and how to fix it. + +## Comment format + +- Reference specific files and lines: `path/to/file.go:42` +- Use ```suggestion blocks when proposing concrete code changes +- Categorize findings: + - **Blocker**: Must fix before merge (bugs, security, data loss) + - **Should fix**: Important but not blocking (error handling gaps, edge cases) + - **Nit**: Minor improvement, take it or leave it +- If the PR looks good, say so briefly. Don't invent problems. diff --git a/.opencode/skills/cog-review/SKILL.md b/.opencode/skills/cog-review/SKILL.md new file mode 100644 index 0000000000..fbd66d0ea9 --- /dev/null +++ b/.opencode/skills/cog-review/SKILL.md @@ -0,0 +1,51 @@ +--- +name: cog-review +description: Cog architecture and cross-cutting review guidelines +--- + +## Cog architecture review guidelines + +Cog packages ML models into production-ready containers. Use this skill for +changes that cross language boundaries or touch core architecture. + +### Component overview + +- **CLI** (Go): `cmd/cog/` and `pkg/` -- builds, runs, and deploys models +- **Python SDK**: `python/cog/` -- predictor interface, types, HTTP/queue server +- **Coglet** (Rust): `crates/` -- prediction server inside containers (HTTP, worker management, IPC) + +### Key design patterns + +**Wheel resolution**: The CLI discovers SDK and coglet wheels from `dist/` at +Docker build time. Wheels are NOT embedded in the binary. Changes to build +artifacts need to account for this. + +**Dockerfile generation**: `pkg/dockerfile/` generates Dockerfiles from `cog.yaml` +config. Template injection and escaping matter here. + +**Config parsing**: `pkg/config/config.go` parses `cog.yaml`. Schema is at +`pkg/config/data/config_schema_v1.0.json`. Changes must keep schema and Go +code in sync. + +**Two-process coglet**: Parent process (HTTP server + orchestrator) and child +worker process (Python predictor execution) communicate via IPC. Changes to +the IPC protocol affect both Rust and Python code. + +**Compatibility matrix**: CUDA/PyTorch/TensorFlow compatibility is managed in +`tools/compatgen/`. Framework version changes have wide blast radius. + +### Cross-cutting concerns + +- **VERSION.txt** is the single source of truth for versioning. `Cargo.toml` must match. +- Changes to `python/cog/base_predictor.py` affect all downstream model authors. +- Changes to `docs/` require running `mise run docs:llm` to regenerate `docs/llms.txt`. +- CLI reference docs are auto-generated -- changes to `cmd/` or `pkg/cli/` require `mise run docs:cli`. +- Integration tests in `integration-tests/` use Go's testscript and need a built cog binary. + +### What to watch for + +- Breaking changes to the predictor interface or cog.yaml schema +- Docker build changes that affect layer caching or build time +- IPC protocol changes without updating both Rust and Python sides +- Version bumps that miss one of the places VERSION.txt needs to match +- New ML framework versions without compatibility testing diff --git a/.opencode/skills/go-review/SKILL.md b/.opencode/skills/go-review/SKILL.md new file mode 100644 index 0000000000..60c5a43558 --- /dev/null +++ b/.opencode/skills/go-review/SKILL.md @@ -0,0 +1,44 @@ +--- +name: go-review +description: Go code review guidelines for the Cog codebase +--- + +## Go review guidelines + +This project uses Go for the CLI (`cmd/cog/`, `pkg/`) and support tooling (`tools/`). + +### What linters already catch (skip these) + +golangci-lint runs errcheck, gocritic, gosec, govet, ineffassign, misspell, +revive, staticcheck, and unused. Don't flag issues these would catch. + +### What to look for + +**Error handling** +- Errors returned but not checked or silently discarded +- User-facing errors should use `pkg/errors.CodedError` with error codes +- Generic error wrapping that loses context (`fmt.Errorf("failed")` with no `%w`) + +**Imports** +- Should be three groups: stdlib, third-party, internal (`github.com/replicate/cog/pkg/...`) +- Only flag if actually wrong, not cosmetic reordering + +**Testing** +- Must use `testify/require` for fatal assertions and `testify/assert` for non-fatal +- No raw `if` checks with `t.Fatal`/`t.Errorf` +- Prefer table-driven tests for similar cases +- Prefer specific assertions (`Equal`, `Contains`, `NoError`) over `True`/`False` + +**Concurrency** +- Goroutine leaks (no cleanup path, missing context cancellation) +- Shared state without synchronization +- Channel misuse (sends on closed channels, unbuffered channels in wrong contexts) + +**Docker/container patterns** +- The CLI uses the Docker Go SDK. Watch for leaked clients, unclosed response bodies +- Dockerfile generation is in `pkg/dockerfile/` -- template injection risks + +**Architecture** +- Commands belong in `pkg/cli/`, business logic in `pkg/` +- Config parsing/validation in `pkg/config/` +- Don't mix CLI concerns with library logic diff --git a/.opencode/skills/python-review/SKILL.md b/.opencode/skills/python-review/SKILL.md new file mode 100644 index 0000000000..7ac56147ce --- /dev/null +++ b/.opencode/skills/python-review/SKILL.md @@ -0,0 +1,47 @@ +--- +name: python-review +description: Python code review guidelines for the Cog SDK +--- + +## Python review guidelines + +This project uses Python for the SDK (`python/cog/`) which defines the predictor +interface, type system, and HTTP/queue server. + +### What linters already catch (skip these) + +ruff handles pycodestyle (E), Pyflakes (F), isort (I), warnings (W), bandit (S), +bugbear (B), and annotations (ANN). Don't flag issues these would catch. + +### What to look for + +**Type annotations** +- Required on all function signatures +- Use `typing_extensions` for backward compatibility +- Avoid `Any` where a concrete type is possible +- Check that type annotations actually match runtime behavior + +**Compatibility** +- Must support Python 3.10 through 3.13 +- Watch for syntax/stdlib features only available in newer versions +- `from __future__ import annotations` if using newer annotation syntax + +**Error handling** +- No bare `except:` or `except Exception:` that swallows everything +- Exceptions should have descriptive messages +- Resource cleanup with context managers, not try/finally when avoidable + +**Async patterns** +- Tests use pytest-asyncio -- async tests need proper fixtures +- Watch for blocking calls inside async functions +- Proper cleanup of async resources (aclose, async context managers) + +**Predictor interface** +- `base_predictor.py` is the core interface -- changes here affect all users +- `types.py` defines input/output types -- check backward compatibility +- Server code in `python/cog/server/` handles HTTP -- watch for request handling bugs + +**Testing** +- Uses pytest with fixtures +- tox runs tests across Python 3.10-3.13 +- Test isolation: don't rely on global state or test ordering diff --git a/.opencode/skills/rust-review/SKILL.md b/.opencode/skills/rust-review/SKILL.md new file mode 100644 index 0000000000..04f8881857 --- /dev/null +++ b/.opencode/skills/rust-review/SKILL.md @@ -0,0 +1,49 @@ +--- +name: rust-review +description: Rust code review guidelines for Coglet +--- + +## Rust review guidelines + +This project uses Rust for Coglet (`crates/`), the prediction server that runs +inside Cog containers. It handles HTTP requests, worker process management, and +prediction execution. + +### What linters already catch (skip these) + +clippy runs in CI. cargo-deny audits dependencies for license/advisory issues. +Don't flag issues these would catch. + +### What to look for + +**Error handling** +- Use `thiserror` for typed errors in library code, `anyhow` for application errors +- Don't use `.unwrap()` or `.expect()` in non-test code unless the invariant is documented +- Error context: use `.context()` or `.with_context()` from anyhow, not bare `?` + +**Ownership and lifetimes** +- Unnecessary cloning where a borrow would work +- Lifetime issues that suggest a design problem (not just annotation noise) +- Arc/Mutex when simpler patterns exist + +**Async (tokio)** +- Blocking operations inside async contexts (use `spawn_blocking`) +- Missing `.await` on futures (compiler catches some, but not all logical issues) +- Proper cancellation handling and cleanup +- Task/JoinHandle leaks + +**Safety** +- Any `unsafe` block needs justification and a safety comment +- FFI boundaries with Python (PyO3) -- check for panics across FFI, GIL handling +- Memory safety in IPC between parent and worker processes + +**Architecture** +- `crates/coglet/` is the core: HTTP server, worker orchestration, IPC +- `crates/coglet-python/` is PyO3 bindings for Python predictor integration +- Two-process architecture: parent (HTTP + orchestrator) and worker (Python execution) +- Don't mix IPC concerns with HTTP handling +- Snapshot tests use `insta` -- check if snapshots need updating + +**Dependencies** +- New dependencies should be justified -- `crates/deny.toml` audits them +- Prefer std or existing deps over adding new ones for small tasks