mirror of
https://github.com/basicmachines-co/basic-memory
synced 2026-06-21 13:47:35 +00:00
docs(core): add Basic Machines agent style guidance
Signed-off-by: phernandez <paul@basicmachines.co>
This commit is contained in:
@@ -0,0 +1,62 @@
|
||||
---
|
||||
name: basic-machines-review
|
||||
description: Use when reviewing Basic Machines code for house style, architecture risk, pre-merge hardening, or whether a change fits basic-memory/basic-memory-cloud conventions.
|
||||
license: MIT
|
||||
---
|
||||
|
||||
# Basic Machines Review
|
||||
|
||||
Use this skill for repo-local review passes where ordinary code review needs Basic Machines
|
||||
house style and architecture judgment. Report findings only; do not edit code unless the user
|
||||
asks you to fix specific findings.
|
||||
|
||||
## Scope
|
||||
|
||||
Review the current diff or named files against:
|
||||
|
||||
- The repo's `AGENTS.md` / `CLAUDE.md`
|
||||
- `docs/ENGINEERING_STYLE.md`
|
||||
- The touched code paths and tests
|
||||
|
||||
For `basic-memory`, prioritize local-first file/database/MCP boundaries. For
|
||||
`basic-memory-cloud`, prioritize tenant/workspace isolation, cloud worker behavior, and
|
||||
web-v2 state/runtime boundaries.
|
||||
|
||||
## Review Rubric
|
||||
|
||||
Report only concrete, falsifiable risks:
|
||||
|
||||
- **Cognitive load:** Is the change harder to understand than the problem requires?
|
||||
- **Change propagation:** Will one product change force edits across unrelated layers?
|
||||
- **Knowledge duplication:** Is the same rule encoded in multiple places that can drift?
|
||||
- **Accidental complexity:** Did the change add abstractions, fallbacks, or state without need?
|
||||
- **Dependency direction:** Are API/MCP/CLI, services, repositories, and UI stores respecting
|
||||
their intended boundaries?
|
||||
- **Domain model distortion:** Do names and types still match the product concept, or did a
|
||||
transport/storage detail leak into the domain?
|
||||
- **Test oracle quality:** Would the tests fail for the bug or regression the change claims to
|
||||
protect against?
|
||||
|
||||
## House Rules To Check Explicitly
|
||||
|
||||
- No speculative `getattr(obj, "attr", default)` for unknown model shapes.
|
||||
- No broad exception swallowing, warning-only failure paths, or hidden fallback behavior.
|
||||
- No casts or `Any` that hide an unclear type relationship.
|
||||
- Dataclasses for internal value/result objects; Pydantic at validation/serialization
|
||||
boundaries.
|
||||
- Narrow `Protocol`s when only a capability is needed.
|
||||
- Explicit async/resource ownership, cancellation, and cleanup.
|
||||
- Meaningful regression tests or verification for risky changes.
|
||||
- Comments explain why, not what.
|
||||
|
||||
## Reporting Format
|
||||
|
||||
Lead with findings ordered by severity. Each finding should include:
|
||||
|
||||
```text
|
||||
severity | file:line | risk category | claim
|
||||
Why: concrete behavior or code path that proves the risk.
|
||||
Fix: smallest practical change, or "none obvious" if the risk needs product input.
|
||||
```
|
||||
|
||||
If there are no findings, say so and note any verification gaps that remain.
|
||||
+1
@@ -0,0 +1 @@
|
||||
../../.agents/skills/basic-machines-review
|
||||
@@ -108,6 +108,27 @@ Before opening or updating a PR, run the checks that mirror the common required
|
||||
- Follow the repository pattern for data access
|
||||
- Tools communicate to api routers via the httpx ASGI client (in process)
|
||||
|
||||
### Programming Style
|
||||
|
||||
See [docs/ENGINEERING_STYLE.md](docs/ENGINEERING_STYLE.md) for the fuller house style. The
|
||||
short version for agents:
|
||||
|
||||
- Prefer type-safe, explicit designs over object-heavy indirection. Use Python 3.12 `type`
|
||||
aliases, full annotations, and narrow `Protocol`s when a caller only needs a capability.
|
||||
- Use dataclasses for internal value objects and operation results; use Pydantic v2 at API,
|
||||
CLI, MCP, and persistence boundaries where validation and serialization matter.
|
||||
- Keep async boundaries obvious. Resource-owning code should use context managers, propagate
|
||||
cancellation, and avoid hidden background work unless the lifecycle is explicit.
|
||||
- Fail fast. Do not add silent fallback logic, broad exception swallowing, speculative
|
||||
`getattr`, or casts that hide an unclear model shape.
|
||||
- Keep control flow simple and local. Push branching decisions up, keep leaf helpers focused,
|
||||
and name values after the domain concept they carry.
|
||||
- Use evidence-first testing. Add or update meaningful regression tests for bugs and risky
|
||||
behavior, prefer real code paths over mocks, and run the narrowest command that proves the
|
||||
change before widening verification.
|
||||
- Comments should explain why a branch, invariant, or constraint exists. Avoid comments that
|
||||
merely narrate obvious code.
|
||||
|
||||
### Code Change Guidelines
|
||||
|
||||
- **Full file read before edits**: Before editing any file, read it in full first to ensure complete context; partial reads lead to corrupted edits
|
||||
|
||||
@@ -0,0 +1,64 @@
|
||||
# Basic Memory Engineering Style
|
||||
|
||||
Style is how we make code easier to verify. Prefer explicit, typed, local-first code that
|
||||
preserves the file system as the source of truth while keeping the database, API, and MCP
|
||||
surfaces in sync.
|
||||
|
||||
## Design Center
|
||||
|
||||
- Basic Memory is local-first. Markdown files are the durable source; SQLite/Postgres indexes
|
||||
are derived state that should be rebuilt or reconciled from files when needed.
|
||||
- Keep the existing boundary order: CLI/MCP/API entrypoints compose dependencies, services own
|
||||
business behavior, repositories own database access, and file services own filesystem writes.
|
||||
- MCP tools should remain atomic and composable. They should call API routers through typed MCP
|
||||
clients, not reach around into services.
|
||||
- Prefer small, explicit abstractions that match a real domain boundary. Avoid object
|
||||
hierarchies when a function, dataclass, type alias, or protocol describes the concept better.
|
||||
|
||||
## Types And Data
|
||||
|
||||
- Use full type annotations and Python 3.12 syntax. Introduce `type` aliases for repeated
|
||||
structured shapes, callback signatures, or domain concepts that would otherwise become
|
||||
anonymous `dict[str, Any]` values.
|
||||
- Use dataclasses for internal values, operation inputs, and service results. Prefer
|
||||
`frozen=True` when the value should not change and `slots=True` when identity/dynamic
|
||||
attributes are not needed.
|
||||
- Use Pydantic v2 at boundaries that validate, serialize, or deserialize data: API payloads,
|
||||
CLI/MCP schemas, configuration, and persistence-adjacent schemas.
|
||||
- Use narrow `Protocol`s when a caller needs a capability rather than a concrete repository or
|
||||
service. Keep protocols small enough that fake implementations in tests are obvious.
|
||||
- Avoid speculative `getattr`, broad casts, or `Any` as a way to paper over uncertainty. Read
|
||||
the model or schema definition and make the type relationship explicit.
|
||||
|
||||
## Control Flow And Resources
|
||||
|
||||
- Fail fast when an invariant is broken. Do not swallow exceptions, add warning-only error
|
||||
handling, or introduce fallback behavior unless the user explicitly agrees to that behavior.
|
||||
- Keep control flow simple and close to the domain decision. Push `if` statements up into the
|
||||
function that owns orchestration; keep leaf helpers focused on computation or one side effect.
|
||||
- Make async/resource boundaries visible with context managers and explicit lifecycles. Do not
|
||||
start background work without a clear owner, cancellation story, and verification path.
|
||||
- Keep file mutations centralized through the existing file utilities/services so checksum,
|
||||
atomic write, and index synchronization behavior stays coherent.
|
||||
|
||||
## Testing And Verification
|
||||
|
||||
- Use evidence-first testing, not mechanical TDD. For bugs and risky behavior, add or update a
|
||||
regression test that would catch the failure. For small documentation-only edits, use the
|
||||
relevant doc/repo hygiene checks.
|
||||
- Prefer tests that exercise real code paths. Use mocks, doubles, or `monkeypatch` only when
|
||||
the external boundary would be slow, nondeterministic, or impossible to trigger directly.
|
||||
- Keep coverage at 100% for new code. Use `# pragma: no cover` only for code that would require
|
||||
disproportionate mocking and is covered through an integration or runtime path.
|
||||
- Start with targeted commands, then widen as risk grows: focused pytest, `just fast-check`,
|
||||
`just doctor`, package checks for agent packaging changes, and full SQLite/Postgres gates
|
||||
when behavior crosses shared boundaries.
|
||||
|
||||
## Comments And Names
|
||||
|
||||
- Name values after the domain concept they carry: project, entity, permalink, tenant, route,
|
||||
checksum, observation, relation, batch, or index state.
|
||||
- Comments should say why a branch, invariant, retry, lifecycle, or compatibility constraint
|
||||
exists. Section headers are useful when a function or file has clear phases.
|
||||
- Avoid comments that restate the code. If a comment cannot explain a decision, simplify the
|
||||
code or improve the name instead.
|
||||
Reference in New Issue
Block a user