Skip to content

CONTRIBUTING.md

Latest commit

 

History

History
182 lines (123 loc) · 14.9 KB

CONTRIBUTING.md

File metadata and controls

182 lines (123 loc) · 14.9 KB

Contributing to Maxim

Thank you for your interest in contributing to Maxim! This document provides guidelines for contributing code, documentation, and bug reports.

Getting Started

# Clone and install in development mode
git clone https://github.com/dennys246/Maxim.git
cd Maxim
pip install -e ".[test]"

# Run tests
python -m pytest tests/ -x -q -m "not slow" --ignore=tests/integration/test_memory_hub.py

# Lint
ruff check src/ tests/
ruff format src/ tests/

Code Style

  • Linter: ruff (configured in pyproject.toml)
  • Line length: 120 characters
  • Imports: Use lazy/deferred imports inside function bodies for optional dependencies. Module-level imports only for core deps (numpy, scipy, pyyaml, json-repair, rich).
  • Naming: Don't rename bio-system classes (Hippocampus, ATL, NAc, SCN, EC, AngularGyrus) — names are load-bearing for the mental model.

Architecture Rules

Maxim has a strict layer dependency graph. See ARCHITECTURE.md for full details.

Key rules:

  • Agents never call tools directly — they propose intents, the executor dispatches.
  • Memory tier progression is one-way: FORMING → SHORT_TERM → LONG_TERM. Active-reference context is in WorkingMemorySet (Exec-owned), not a tier.
  • LLM access goes through models/language/router.py — don't import backends directly.
  • Persistence uses maxim.utils.atomic_io.atomic_write_json — don't hand-roll open().write() + os.replace().
  • User data lives in ~/.maxim/, bundled defaults in src/maxim/_data/.

Testing

# Fast: just the module you changed
python -m pytest tests/unit/test_your_module.py -v

# Full suite (exclude known slow integration test and slow-marked tests)
python -m pytest tests/ -x -q -m "not slow" --ignore=tests/integration/test_memory_hub.py

# If touching memory/, decisions/, integration/:
python -m pytest tests/integration/test_memory_hub.py -q

Test markers:

  • @pytest.mark.slow — deselect with -m "not slow"
  • @pytest.mark.integration — requires multiple subsystems
  • @pytest.mark.robot — requires Reachy hardware
  • @pytest.mark.learning — convergence/ML tests

Pull Requests

  1. Fork the repository
  2. Create a feature branch (git checkout -b feat/my-feature)
  3. Make your changes with tests
  4. Run ruff check src/ tests/ and python -m pytest tests/ -x -q -m "not slow" --ignore=tests/integration/test_memory_hub.py
  5. Commit with a descriptive message (see format below)
  6. Push to your fork and open a PR

Pre-merge two-lens review

For any change that touches ≥~500 lines of src/ (a new mechanism, a bus/bridge/builder, a bio-system, a routing layer, a CLI subcommand), run a two-lens review before opening the PR — not after.

Spawn two independent reviews of the branch:

  • Executor lens — does each code path do what it claims under the inputs it will actually see? Checks for silent no-ops, swallowed exceptions, env-var races, wrong-direction comparisons, and call sites that diverge from the contract they advertise.
  • Architecture lens — does the change fit the layer graph and the existing invariants? Checks for band-aids (special cases, flags that toggle around broken behavior), duplicated source-of-truth, and mechanisms that should ride on existing infrastructure instead of being their own thing.

Fold the findings into the same branch with a follow-up commit before the PR opens. Fold cross-confirmed findings first — when both lenses flag the same issue independently, that is the strongest signal the finding is real and should be trusted even when it feels minor.

Why this is non-optional and not ceremony: across the LLM-path refinement rounds (R1/R2/R3) the review surfaced correctness bugs (an env-var race, a probe mis-classification, a canonical-shim bypass) that the test suite caught zero of — they were correctness issues in input spaces tests didn't cover. Do not merge first and ship a fix/<feature>-loose-ends PR after; that splits the bisect surface and leaves known-buggy code on main.

Adding an env var

Every MAXIM_* env var read in a startup or hot path must land with its scrub in the same commit:

  1. Add it to the env-var table in CLAUDE.md (the authoritative registry) with a one-line description of what it does, its default, and the file that reads it.
  2. Add an autouse _isolate_* scrub fixture in tests/conftest.py that pops the var before each test and restores it after. Copy the shape of an existing one (e.g. _isolate_maxim_nac_reward_bias_disabled): saved = os.environ.pop("MAXIM_FOO", None) → yield → restore in finally.

The scrub is load-bearing whenever the var gates a side effect reachable from runtime construction (build_primary_router, auto-spawn, tier detection, ensure_available, ...). Without it, any test that sets the var leaks it into every later test that builds the runtime, and the leaked side effect runs for real. This is the rule earned by the P5 9-minute pytest hang on a real 1 GB GGUF download to ~/.maxim/: a normalize_args unit test set MAXIM_AUTO_DOWNLOAD_MODELS=1, it leaked, and a later test's runtime construction acted on it. _isolate_maxim_llm_profile_env and _isolate_maxim_auto_download_env are the template fixtures.

CI invariant gates

Several architectural invariants are enforced by grep allow-lists in .github/workflows/test.yml — a grep over src/ (and sometimes tests/) that fails CI on a forbidden pattern, with an explicit allow-list of the file(s) permitted to match. Examples currently enforced:

  • urllib.request.urlopen is banned everywhere except utils/http.py (all outbound HTTP routes through the registry-backed module).
  • maxim_peer_backend.py may not contain retry/backoff/gateway (the one-HTTP-call, no-internal-retry invariant) except the two whitelisted parameter names.
  • write_config / mutate_config / set_field callers are restricted to config_writer.py + config_cli.py + a short allow-list (single-writer for config.json).
  • write_mesh_config callers are restricted to mesh_setup.py (single sanctioned writer for mesh.yml).
  • yaml.load( is banned in favor of yaml.safe_load; deprecated probe shims and dead mesh modules are blocked from new callers.

When you add a new gate, or legitimately add a new allowed call site, update the allow-list in test.yml and the matching [engineering] invariant in CLAUDE.md in the same commit. The grep pattern and the CLAUDE.md Regression guard: line are a pair — one without the other drifts. If you find yourself wanting to add many call sites to a strict allow-list across successive PRs, that's a signal the rule needs revisiting rather than another entry; raise it instead of silently widening.

Adding a bio-system

Bio-systems (buses, the memory hub, the default network) are constructed through canonical build_X builders, never by raw construction in production code. The rules:

  • Use the canonical builder. PainBus → proprioception/pain_bus.py::build_pain_bus; ReactionBus → reactions/bus.py::build_reaction_bus; MemoryHub → integration/memory_hub.py::build_memory_hub; DefaultNetwork → runtime/bootstrap.py::build_default_network; the full pipeline → runtime/bio_stack.py::build_bio_stack.
  • Required keyword-only learning subjects. Builders push silent-no-op invariants into the signature so forgetting is a TypeError, not a missing-learning bug. build_pain_bus requires hippocampus= and nac= (pass None to explicitly opt out). build_memory_hub requires the four core bio-systems hippocampus=, scn=, nac=, ec=. build_default_network requires nac=.
  • agent_id= is required. build_bio_stack and build_memory_hub take a required keyword-only agent_id= — the old default_agent default silently split the substrate-write and substrate-read paths in multi-agent runs. Test sites that don't care pass the literal "default_agent" explicitly.
  • _allow_raw=True is for tests only. Raw PainBus() / ReactionBus() / MemoryHub() construction raises TypeError (the C6 hard-error flip). Tests that genuinely need a bare instance pass _allow_raw=True; production code never does. The bio-system classes (Hippocampus, NAc, SCN, EC, ATL, AngularGyrus) are intentionally constructible directly — only the buses and the hub are gated.
  • Wire into build_bio_stack in dependency order. A new bio-system that the full pipeline owns gets composed in build_bio_stack, which builds the Wave 1+2 builders in the correct order (ReactionBus before PainBus, etc.). Don't hand-roll the construction in an entry point; add it to the stack so every agent path picks it up.

Don't rename the bio-system classes — the names are load-bearing for the mental model.

See docs/architecture/structural_enforcement.md for the full catalog of which invariants are pushed into types vs. enforced by review.

Adding a dependency-gated feature

Maxim ships a small core (numpy, scipy, pyyaml, json-repair) and gates everything heavier behind optional extras. Do not write a bare try: import X except ImportError: ... return None block — route through maxim/utils/optional_deps.py, which makes the behavior uniform and intentional, split by intent:

  • require_optional_dependency(import_name, *, extra=None, feature=None) — for a feature the caller explicitly requested (a configured cloud backend, a --vision flag, a selected TTS voice). A missing dependency here is a setup error, so this raises OptionalDependencyError with an actionable pip install pymaxim[<extra>] hint. OptionalDependencyError subclasses ImportError, so legacy except ImportError sites still catch it, but fail-soft layers (the LLM router) can distinguish "requested feature not installed → abort loudly" from "transient runtime failure → try next provider".
  • optional_dependency_available(import_name) — for a capability probe ("use X if it happens to be present", GPU detection). Absence is normal here, so this returns a bool and never logs.
  • warn_optional_fallback(import_name, *, fallback, extra=None, feature=None) — for a deliberate degradation path that has a real working fallback (sentence-transformers → bag-of-words embeddings, spacy → identity decomposition, ddgs → regex scraping). The caller keeps the fallback; this emits exactly one deduped log so the degradation is visible instead of silent.

The motivating failure (2026-06-05 cloud-dispatch incident): anthropic was not installed, the backend logged a single swallowed WARNING and returned None, the router treated the empty response as a transient hiccup, and the whole sim completed at cost=$0 with every action a _llm_unavailable fallback — the missing backbone was invisible. The EXTRA_FOR_IMPORT table in optional_deps.py maps import names to extras; add new optional deps there so the fix hint is accurate.

Key architecture docs

Doc What it covers
docs/architecture/llm_routing.md LLM router, provider fallback, lane tiers (large/medium/small), _MaximPeerBackend vs _OpenAIBackend, backend dispatch table
docs/architecture/structural_enforcement.md Which invariants are pushed into types (required kwargs, frozen dataclasses, _allow_raw gates) vs. enforced by CI grep vs. by review
docs/user/tool_side_effects.md Authoritative registry of well-known ToolOutput.side_effects keys (embodiment_failures, entity_acquired/entity_released, affordance_blocked)

Commit Message Format

type: short description

Longer description if needed.

Co-Authored-By: Your Name <email>

Types: feat, fix, docs, refactor, test, chore

Adding New Features

Adding a tool

  1. Create in src/maxim/tools/ or extend an existing tool file.
  2. Inherit from the Tool base class (tools/base.py).
  3. Register in the tool registry.
  4. Add tests in tests/unit/.

When implementing a non-trivial tool, mind these Tool contract surfaces:

  • input_schema is dual-format; export via to_json_schema(). Authors may write either the legacy custom format ({NAME: type} / {NAME: (type, default)} / {NAME: "description"}) or JSONSchema 2020-12 ({"type": "object", "properties": {...}, "required": [...]}). Tool.__init__ does not normalize — it leaves input_schema exactly as authored. Any consumer that reads the schema (prompt rendering, MCP export) must route through tool.to_json_schema() rather than iterating input_schema.items() directly, or @maxim.tool JSONSchema tools silently render with empty params. The custom format is strictly less expressive (no enum, pattern, oneOf, nested objects) — author JSONSchema directly when you need those.
  • side_effects is the typed bio-pipeline channel. ToolOutput.side_effects: dict[str, Any] | None carries well-known keys (embodiment_failures, entity_acquired / entity_released, affordance_blocked) consumed by the bio pipeline. The append-only registry of keys is authoritative in docs/user/tool_side_effects.md — add a new key there and wire its consumer (or mark it informational) in the same PR. Don't hijack output (the main result) or metadata (caller-facing extras) for bio signals.
  • cancel() is a thread-safe no-op by default. The base Tool.cancel() returns None so existing tools work unchanged. Tools that do heavy work (HTTP, subprocess, large reads) override it to set a threading.Event the executing thread checks at safe points. cancel() is called from a different thread than execute(), must be thread-safe, and must never raise. No 1.0 dispatch path calls it — it's forward-compat for 1.1+ cancellation work; don't make it @abstractmethod.
  • auto_fire=True marks a tool the agent loop dispatches every tick for passive perception (e.g. the SEM sense tools), rather than only when the agent proposes it. Set it on the dataclass; the registry exposes them via get_auto_fire_tools().

Adding a SEM component

  1. Create YAML in src/maxim/_data/components/{category}/
  2. Follow the component format (see existing files for examples)
  3. Test via ComponentRegistry.get() and instantiate()

Adding an encounter template

  1. Create YAML in src/maxim/_data/encounters/{category}/
  2. Follow the encounter format (see existing files)
  3. Test via EncounterLibrary.get()

Adding a simulation persona

  1. Add entry to SIMULATION_PERSONAS in src/maxim/simulation/personas.py
  2. Or use maxim.register_persona() at runtime

License

By contributing, you agree that your contributions will be licensed under the Apache License 2.0.

Exception: The [yolo] extra uses AGPL-3.0 licensed code (ultralytics). Contributions to YOLO-related code are licensed under AGPL-3.0.

Questions?

Open an issue at https://github.com/dennys246/Maxim/issues