Skip to content

CODE-RULES.md

Latest commit

 

History

History
215 lines (152 loc) · 8.49 KB

CODE-RULES.md

File metadata and controls

215 lines (152 loc) · 8.49 KB
Raw

Code Rules

These are the engineering standards for scriptbench. They are general-purpose principles; project-specific conventions (path types, logging, library boundaries, etc.) live in DESIGN.md §17.

Core Principles

90% Test Coverage

  • 90% line, branch, and function coverage on the package as a whole
  • Pure-logic modules (schemas.py, schedule.py, fingerprint.py, cache/, output/summary.py) target 95%+
  • Tests written before or with implementation
  • No skipped or commented tests
  • Check with pytest --cov=scriptbench --cov-report=term-missing
  • Coverage gate runs on every push via pre-push hook

GPU-required tests are marked @pytest.mark.gpu and excluded from the coverage gate; they run separately on a real GPU machine.

Type Safety

  • Type hints on every function signature, including return types
  • No Any without an inline comment explaining why
  • Rely on pydantic and Annotated[...] for inferred types; never manually duplicate inferred shapes
  • pyright --strict must pass — no warnings
  • from __future__ import annotations at the top of every module so forward references resolve cleanly

Fail Fast

  • Validate preconditions immediately
  • Never defer errors with fallback values
  • Missing config = immediate crash with a clear message
  • Invalid input = reject at the boundary, not deep in logic
  • Pydantic validates all data crossing module boundaries (DESIGN §17)

Never Hide Problems

  • No # type: ignore without a justification on the same line
  • No # noqa without a code and reason (e.g. # noqa: E501 long URL)
  • No bare except: or except Exception: that swallows
  • No try/except: pass to make a test go green
  • No print() in library code — output goes through output/ (DESIGN §17)
  • No --force flags, no --no-verify, no piping errors to /dev/null
  • Fix root causes, not symptoms

Error Handling

  • Never swallow errors silently
  • Use the typed exception hierarchy in errors.py (DESIGN §14)
  • Every external boundary (subprocess, NVML, filesystem, sqlite) catches the system exception and re-raises as a typed ScriptbenchError subclass with context
  • Log with logging, not print
  • Stack traces suppressed by default; printed with --verbose or SCRIPTBENCH_DEBUG=1

Patterns

Single Source of Truth

  • Pydantic schemas in schemas.py define every shape that crosses a module boundary
  • The Adapter Protocol in adapters/base.py defines the extension point
  • Types flow from these sources; never duplicate them in another module
  • The on-disk JSON / SQLite shapes are derived from the pydantic schema, not hand-written

Idempotency

scriptbench's whole architecture is built on idempotency:

  • Runs are keyed by fingerprint — content-addressable. Same inputs → same key → cache hit.
  • The runs SQLite table is append-only and immutable. Mutable state (user notes, flags) goes to the annotations side table.
  • Cache writes use tmp-then-rename for atomicity (DESIGN §17). Crash mid-write leaves no half-written file.
  • Re-running a session with the same arguments is always safe — every run either hits cache or produces a fresh run with a fresh run_id.

Subprocess Isolation

  • We never import adapter scripts into the orchestrator's process
  • Every script runs in a fresh subprocess with its own stdout/stderr capture
  • Per-adapter venvs (uv-managed) keep dependency conflicts contained (DESIGN §19)
  • This is non-negotiable; relaxing it has been considered and rejected (DESIGN §3.2)

Code Organization

Naming

  • Modules: snake_case.py
  • Packages: snake_case/
  • Classes / Pydantic models / Exceptions: PascalCase
  • Functions / methods / variables: snake_case
  • Constants / module-level singletons: SCREAMING_SNAKE_CASE
  • Test files: test_<module>.py
  • Test functions: test_<behavior>
  • Private helpers: leading underscore (_internal_helper)

Structure

  • Colocate tests under tests/ mirroring the package layout
  • Shared code lives in the package, never copy-pasted between modules
  • One public class or main concept per file; small private helpers can colocate
  • __init__.py files are for re-exports only — no logic
  • Module docstrings explain what the module does and what depends on it (DESIGN §17)
  • pathlib.Path everywhere a path is involved — never raw strings (DESIGN §17)

Imports

Order, separated by blank lines:

  1. from __future__ import annotations
  2. Standard library
  3. Third-party
  4. First-party (from scriptbench.foo import ...)
  5. Local relative (from .bar import ...)

TYPE_CHECKING-only imports go in a guarded block at the bottom of the import section. Sort within each group; ruff's import sorting (I rules) enforces this.

Testing

Requirements

  • Unit tests for every pure function and pydantic model
  • Integration tests for cache round-trip, subprocess invocation, GPU sampler shape
  • Property-based tests (hypothesis) for schedule.py and fingerprint.py
  • Adapter parser tests run against captured stdout fixtures (no GPU needed) — DESIGN §15
  • Tests must not depend on execution order
  • No hardcoded dates — use freezegun or pytest fixtures for time
  • Test behavior, not implementation
  • GPU-required tests marked @pytest.mark.gpu and excluded from default runs

What to Test

  • Happy paths
  • Error conditions (each typed exception in errors.py should be raised by at least one test)
  • Edge cases and boundaries (empty schedules, single-run sessions, fingerprint collisions, schema bumps)
  • Idempotency (same fingerprint → same cache hit)
  • Input validation (pydantic should reject malformed input at the boundary)

Security

  • Validate every external input with pydantic
  • Never trust adapter-provided IDs or paths — sanitize before filesystem operations
  • Never interpolate user input into shell strings; use subprocess.Popen with a list argv
  • Never hardcode or log secrets
  • Adapters are trusted code we ship — but their output is untrusted (we parse third-party stdout). Treat parser output as adversarial.

Performance

  • Measure before optimizing
  • The cache is the primary performance feature — keep its lookup path fast (single SQLite query, no full table scan)
  • GPU sampling overhead must stay under 1% of subprocess wallclock — sample on a background thread, not in a blocking loop
  • Stream large outputs (stdout files can be hundreds of MB during long runs); never .read() them whole into memory unless parsing
  • Aggregate metrics on the fly when possible

Documentation

When to Comment

Comment durable facts that a future reader with no context cannot derive from the code, names, types, or tests, when the information is load-bearing on correctness or future modification and survives the current task.

Examples:

  • Non-obvious domain logic (CIFAR-10 conventions, speedrun scoring rules)
  • Source-of-truth designations
  • Hidden coupling between modules
  • Race conditions and ordering constraints (e.g. GPU sampler thread lifecycle)
  • Performance traps (NVML overhead, subprocess pipe deadlocks)
  • Library or external API quirks (NVML field semantics, nsys output format)
  • Rejected alternatives with the reason for rejection
  • Code that looks removable but isn't (e.g. the pre-flight CUDA context init in DESIGN §8)
  • Exceptions to established rules
  • Subtle edge cases

A wrong comment is worse than no comment. If you can't state the durable fact precisely, leave it out.

When Not to Comment

  • Obvious operations
  • Self-explanatory names
  • Standard patterns
  • What code does (code shows this)
  • Code you didn't change
  • Step-by-step narration of an algorithm

Never Include

  • Specific file paths that may move
  • Hardcoded version numbers (use __version__ or pyproject metadata)
  • Specific timing estimates ("takes 2.3s")
  • Ephemeral values (run IDs, hashes, container IDs)
  • TODO / FIXME (file an issue or fix it)
  • Task-state narration ("added", "updated", "step 2 of 3", "extracted for clarity")

Enforcement

Configured via .pre-commit-config.yaml and pyproject.toml. Install hooks once with uv run pre-commit install after uv sync.

  • Pre-commit (fast): ruff format --check, ruff check --fix
  • Pre-push (full gate): pyright --strict, vulture, deptry, pylint (duplicate-code + too-many-nested-blocks), pytest -m 'not gpu' --cov with the 90% coverage gate
  • Manual before a release: pytest -m gpu on a real GPU box

If a check fails, fix it. Don't add to ignore lists without reading the failure first; ignore-list growth is a code smell.

No exceptions.