From 68f3394f903449e4340a9836af8f2139ad4f3a26 Mon Sep 17 00:00:00 2001 From: phernandez Date: Fri, 17 Jul 2026 20:01:14 -0500 Subject: [PATCH] feat(skills): add Pythonic code guidance Signed-off-by: phernandez --- .agents/skills/pythonic-code/SKILL.md | 112 ++++++++++ .../skills/pythonic-code/agents/openai.yaml | 4 + AGENTS.md | 17 +- docs/DOMAIN_MODEL.md | 207 ++++++++++++++++++ docs/ENGINEERING_STYLE.md | 49 ++++- 5 files changed, 379 insertions(+), 10 deletions(-) create mode 100644 .agents/skills/pythonic-code/SKILL.md create mode 100644 .agents/skills/pythonic-code/agents/openai.yaml create mode 100644 docs/DOMAIN_MODEL.md diff --git a/.agents/skills/pythonic-code/SKILL.md b/.agents/skills/pythonic-code/SKILL.md new file mode 100644 index 000000000..9a274d815 --- /dev/null +++ b/.agents/skills/pythonic-code/SKILL.md @@ -0,0 +1,112 @@ +--- +name: pythonic-code +description: >- + Write, refactor, and review Python for clarity, explicit behavior, local reasoning, strong + types, and minimal abstraction. Use when creating or changing nontrivial Python, simplifying + object-heavy or helper-heavy code, evaluating whether code is Pythonic, or reviewing Python + maintainability in Basic Memory repositories. +--- + +# Pythonic Code + +Write Python that makes domain behavior obvious to human and AI readers. Apply a WWGD lens: +choose the simplest correct design that feels native to Python and is easy to verify. + +## Orient Before Coding + +1. Read the repository's `AGENTS.md` or `CLAUDE.md` instructions. +2. Read `docs/ENGINEERING_STYLE.md` when present. +3. Read `docs/DOMAIN_MODEL.md` when the change touches domain language, ownership, identity, + source-of-truth rules, or lifecycle behavior. +4. Read each target file completely before editing it. +5. Identify the supported Python version, configured tools, surrounding patterns, and behavior + that must remain stable. + +Let local project rules override generic style advice. + +## Make Decisions In This Order + +1. Preserve correctness, domain invariants, and public behavior. +2. Respect repository conventions and compatibility constraints. +3. Make data flow, control flow, errors, and side effects obvious. +4. Choose the smallest abstraction that reduces cognitive load now. +5. Use Python idioms when they clarify intent rather than merely shorten code. +6. Prove the result with types, tests, and repository tooling. + +## Prefer Functions Before Hierarchies + +- Start with an ordinary, fully typed function. +- Pair functions with a dataclass when related state or an operation result needs a name. +- Use callbacks, closures, or `functools.partial` when binding behavior is clearer than creating + another object. +- Use `functools.singledispatch` only when behavior genuinely varies by the first argument's + runtime type and open registration is an intentional extension point. +- Use a narrow `Protocol` when callers need a capability instead of a concrete implementation. +- Use a concrete class when identity, cohesive mutable state, lifecycle, or resource ownership + requires one. +- Use an abstract base class only when runtime-enforced subclassing or shared skeletal behavior + is part of the current design. + +Do not replace one class hierarchy with clever functional machinery. Prefer the form with the +fewest concepts, hidden rules, and call hops. + +## Keep Reasoning Local + +- Keep a straightforward workflow together when top-to-bottom reading is clearest. +- Extract a helper only when its name captures a domain operation, it isolates a side effect or + constraint, it removes meaningful duplication, or it forms a cohesive testable computation. +- Do not extract helpers merely to shorten a function. +- Treat a class dominated by private methods as a signal that behavior may belong in explicit + module-level functions operating on typed values. +- Treat long chains of `_prepare_*`, `_resolve_*`, `_apply_*`, and `_build_*` calls as a prompt to + reconsider the data flow or name one meaningful phase object. +- Avoid manager, factory, base, adapter, strategy, and registry abstractions with only one real + implementation. +- Avoid dynamic registration, metaprogramming, and decorator-driven control flow unless the + product currently needs that extension mechanism. + +If extracting a helper makes the reader navigate more but understand no less, keep the logic +local. + +## Write Explicit Python + +- Name values after the domain concept they carry. +- Use full annotations and narrow types. Do not hide uncertainty with `Any`, broad casts, + speculative `getattr`, or unstructured dictionaries. +- Use dataclasses for internal values and Pydantic at validation and serialization boundaries. +- Prefer direct iteration, context managers, standard-library building blocks, and simple + comprehensions where their meaning is immediate. +- Distinguish absence from falsiness; use truth-value testing only when empty values share the + intended meaning. +- Keep async work, resource ownership, cancellation, and cleanup visible. +- Fail fast with specific errors. Do not add silent fallbacks or broad exception handling. +- Comment decisions and constraints, not mechanics. +- Optimize measured hot paths; do not trade readability for hypothetical performance. + +## Match The Requested Mode + +### Write + +Establish the contract and domain values first. Implement the direct path, then add only the +abstractions required by real variation, state, or boundaries. + +### Refactor + +Preserve observable behavior, keep the diff focused, and add or update a regression test when +the behavior is risky. Do not mechanically rewrite already-clear code to apply an idiom. + +### Review + +Report concrete readability, abstraction, typing, lifecycle, and domain-model risks. Explain the +smallest practical improvement. Do not edit unless the user asks for fixes. + +## Verify The Result + +Run the narrowest command that proves the change, then widen according to risk: + +1. Focused tests for the changed behavior. +2. Formatter, linter, and type checker configured by the project. +3. Repository health, package, integration, or full gates when boundaries are affected. + +Lead the final response with the outcome and verification. Explain design choices only when they +are non-obvious or materially affect future work. diff --git a/.agents/skills/pythonic-code/agents/openai.yaml b/.agents/skills/pythonic-code/agents/openai.yaml new file mode 100644 index 000000000..ca880394e --- /dev/null +++ b/.agents/skills/pythonic-code/agents/openai.yaml @@ -0,0 +1,4 @@ +interface: + display_name: "Pythonic Code" + short_description: "Write clear, direct, maintainable Python" + default_prompt: "Use $pythonic-code to write or refactor this Python code for clarity, directness, and maintainability." diff --git a/AGENTS.md b/AGENTS.md index aeabd1871..d8b42fb1e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -112,11 +112,14 @@ Before opening or updating a PR, run the checks that mirror the common required ### Programming Style -See [docs/ENGINEERING_STYLE.md](docs/ENGINEERING_STYLE.md) for the fuller house style. The -short version for agents: +See [docs/ENGINEERING_STYLE.md](docs/ENGINEERING_STYLE.md) for the fuller house style and +[docs/DOMAIN_MODEL.md](docs/DOMAIN_MODEL.md) for product language, ownership, identity, and +source-of-truth rules. 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. +- Prefer functions and typed values before classes, and concrete classes before abstract base + classes. Treat private-helper sprawl as a prompt to simplify the data flow. - 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 @@ -190,7 +193,9 @@ counter += 1 # track retries for backoff calculation ### Codebase Architecture -See [docs/ARCHITECTURE.md](docs/ARCHITECTURE.md) for detailed architecture documentation. +See [docs/ARCHITECTURE.md](docs/ARCHITECTURE.md) for code layers and dependency direction. See +[docs/DOMAIN_MODEL.md](docs/DOMAIN_MODEL.md) for the meaning and invariants of the concepts those +layers implement. **Directory Structure:** - `/alembic` - Alembic db migrations @@ -338,9 +343,11 @@ See `.claude/commands/release/release.md` (and `beta.md`, `release-check.md`, `c ### Knowledge Structure -- Entity: Any concept, document, or idea represented as a markdown file +- Project: The knowledge and isolation boundary for entities, graph state, and search +- Note: A user-facing Markdown document and the canonical representation of its knowledge +- Entity: The project-scoped indexed representation of a file or resource - Observation: A categorized fact about an entity (`- [category] content`) -- Relation: A directional link between entities (`- relation_type [[Target]]`) +- Relation: A directed semantic link owned by its source entity (`- relation_type [[Target]]`) - Frontmatter: YAML metadata at the top of markdown files - Knowledge representation follows precise markdown format: - Observations with [category] prefixes diff --git a/docs/DOMAIN_MODEL.md b/docs/DOMAIN_MODEL.md new file mode 100644 index 000000000..2900f9b2f --- /dev/null +++ b/docs/DOMAIN_MODEL.md @@ -0,0 +1,207 @@ +# Basic Memory Domain Model + +This document defines Basic Memory's product language, ownership, identity, and source-of-truth +rules. [ARCHITECTURE.md](ARCHITECTURE.md) describes code layers and dependency direction; +[ENGINEERING_STYLE.md](ENGINEERING_STYLE.md) describes how to implement changes. This document +describes what the system's concepts mean and which invariants the code must preserve. + +## Design Center + +Basic Memory gives humans and agents one canonical representation for note knowledge: Markdown. +Other resources may be addressable, but structured note and graph semantics are derived from +Markdown without making the resulting projections a competing source of knowledge. + +Use domain terms in names, types, APIs, tests, and documentation. Do not let a transport DTO, +database table, queue message, or UI state redefine the product concept it represents. + +## Core Concepts + +### Project + +A project is the core knowledge and isolation boundary. Every entity, observation, relation, and +search record belongs to exactly one project. + +- A local project maps to a configured directory. +- A hosted workspace or tenant may provide routing, authorization, and storage around a project, + but `workspace`, `tenant`, and `project` are not interchangeable domain terms. +- Project selection must be explicit or resolved once at an entrypoint. Lower layers receive the + resolved project context rather than rediscovering global state. + +### Note + +A note is the user-facing Markdown document. It combines optional YAML frontmatter, prose, +observations, and relations. + +- A note is Markdown, not a database row or editor view. +- `EntityMarkdown` is the parsed boundary representation of a note. +- A Markdown note maps to one project-scoped entity. An entity is broader than a note because the + resource model can also represent non-Markdown content. + +Use `note` in user-facing APIs and flows that specifically operate on Markdown. Use `entity` when +code genuinely operates on the broader indexed resource model. + +### Entity + +An entity is the project-scoped indexed representation of one file or resource. It is the node to +which graph and search projections attach. + +- `id` is an internal database identity. +- `external_id` is the stable external/API identity and must survive ordinary updates and moves. +- `file_path` is the current project-relative storage location. It is unique within a project and + may change when the resource moves. +- `permalink` is the human/agent-facing semantic address for Markdown content. It is + project-scoped, may be absent when permalinks are disabled or inapplicable, and changes only + according to the configured move and permalink policies. +- `title` is mutable display metadata, not identity. +- `checksum`, timestamps, size, parsed metadata, and indexed text describe synchronized state; + they are not independent knowledge sources. + +### Observation + +An observation is a categorized semantic statement owned by one source entity. It contains +content and may carry category, context, and tags. + +- An observation belongs to the same project as its entity. +- Its durable representation is the observation syntax in the source note. +- The database row and search document are projections rebuilt when the note is parsed. +- An observation has no independent lifecycle outside its source entity. + +### Relation + +A relation is a directed semantic statement owned by its source entity. + +- `from_id` identifies the source entity that contains the relation. +- `relation_type` names the meaning of the edge. +- `to_id` identifies the resolved target when it exists. +- `to_name` preserves the author's target text even while the relation is unresolved. +- Source and resolved target belong to the same project graph. +- The durable representation is the relation syntax in the source note. Resolution enriches that + statement; it does not replace it. + +Incoming graph navigation does not transfer ownership to the target. Re-indexing the source note +may recreate, resolve, or remove its outgoing relations. + +### NoteContent + +`NoteContent` is the operational record of accepted Markdown bytes and their file materialization +state for one note entity. It is not a second note or a separate product concept. + +- `markdown_content`, `db_version`, and `db_checksum` identify the accepted content version. +- `file_version` and `file_checksum` identify the version materialized to storage. +- `file_write_status` records whether materialization is pending, writing, synchronized, failed, + or blocked by an external change. +- A note entity has at most one current `NoteContent` record. + +Use this model to coordinate DB-first acceptance, retries, conflict detection, and read repair. +Do not let materialization mechanics leak into ordinary note or entity APIs unless callers need to +reason about acceptance or synchronization status. + +### Search And Graph Indexes + +Full-text rows, vector embeddings, chunks, observation rows, and relation rows are derived +projections. They exist to retrieve and traverse knowledge efficiently. + +- A projection may lag an accepted write during asynchronous work. +- A projection must be rebuildable or reconcilable from canonical Markdown state. +- Search results point back to entities and notes; they do not become independent documents. +- Deleting or rebuilding an index must not mutate canonical content. + +## Source Of Truth And Authority + +"Markdown is the source of truth" describes the product representation. Operational authority +depends on the write phase and runtime. + +### Local File-First Flow + +The Markdown file is the durable authority. Direct human edits, CLI writes, and local MCP/API +writes converge on the file. Entity, graph, and search state are reconciled from the bytes that +landed on disk. + +### Accepted DB-First Or Cloud-Style Flow + +The service derives the final Markdown once and records that exact accepted version in +`NoteContent`. Until materialization finishes, that version is the operational authority for what +the system accepted. A worker or local provider materializes the same bytes to storage, records +the resulting file version and checksum, and publishes the derived graph and search state. + +The DB-first accept path does not create a proprietary document model: the accepted value is still +Markdown, and materialization produces the portable file representation. + +### Project Registry Authority + +Project discovery differs by runtime. Local flows reconcile configured projects with the local +database; hosted flows may receive project and workspace context from the database or control +plane. Resolve that authority at the composition root or project service. Do not make repositories +or leaf helpers guess which registry wins. + +## Write And Reconciliation Flows + +### Create Or Update + +1. Resolve the project and validate the boundary request. +2. Derive final frontmatter, permalink behavior, path, and Markdown bytes without mutating the + caller's input. +3. Parse the accepted Markdown once into its entity, observations, and relations. +4. Persist through the runtime's file-first or DB-first path. +5. Reconcile the entity and its owned graph projections. +6. Update search projections from the same accepted or materialized content. +7. Resolve pending relation targets after the target entities exist. + +Do not independently rebuild Markdown in multiple layers. Preparation, persistence, response, and +indexing must agree on the accepted bytes. + +### External File Change + +1. Detect the changed project-relative path. +2. Read and parse the file that now owns the local truth. +3. Upsert the entity while preserving stable external identity. +4. Replace the source entity's observations and outgoing relations from the parsed note. +5. Refresh search projections and relation resolution. + +### Move + +A move changes `file_path`. It preserves `external_id`, updates storage atomically, and applies the +configured permalink policy. Code that handles a move must keep frontmatter, entity state, +materialization state, graph links, and search records coherent. + +### Delete + +A delete removes canonical content through the service that owns the storage boundary, then +removes or reconciles its derived entity, graph, materialization, and search state. A caller must +not delete only a projection and report that the note was deleted. + +## Boundary Vocabulary + +- Use **schema** or **request/response model** for Pydantic transport and validation types. +- Use **model** with a qualifier when ambiguity matters: domain value, persistence model, parsed + Markdown model, or runtime payload. +- Use **repository** for database access, not business decisions or file writes. +- Use **service** for domain operations that coordinate repositories, files, and projections. +- Use **client** for typed communication across an API boundary. +- Use **materialization** for writing accepted Markdown to durable file storage. +- Use **indexing** for producing retrieval state from content. +- Use **synchronization** or **reconciliation** for bringing canonical content and projections + back into agreement. + +Avoid generic names such as `manager`, `handler`, `data`, `item`, or `record` when the domain term +is known. + +## Domain Change Checklist + +Before adding a model, abstraction, or workflow, answer: + +1. Which domain concept owns this behavior or state? +2. What is the canonical representation at this point in the lifecycle? +3. Which identifier is stable, and which locations or labels may change? +4. Is this value a domain concept, a boundary schema, persistence state, or a derived projection? +5. Which project, workspace, or tenant boundary constrains it? +6. Can the code express the operation with functions and typed values before introducing another + service, hierarchy, or registry? +7. Which test proves the invariant across file, database, graph, search, and API surfaces? + +## Related Documentation + +- [Architecture](ARCHITECTURE.md) +- [Engineering Style](ENGINEERING_STYLE.md) +- [DeepWiki generated overview](https://deepwiki.com/basicmachines-co/basic-memory) — use as a + navigation aid; checked-in documentation and source code are authoritative. diff --git a/docs/ENGINEERING_STYLE.md b/docs/ENGINEERING_STYLE.md index 31497d35c..f0b28b188 100644 --- a/docs/ENGINEERING_STYLE.md +++ b/docs/ENGINEERING_STYLE.md @@ -1,13 +1,15 @@ # 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. +Style is how we make code easier to verify. Prefer explicit, typed, local-first code that keeps +Markdown as the canonical product representation while the file materialization, database, API, +and MCP surfaces stay 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. +- Basic Memory is local-first. In local flows, Markdown files are the durable source and + SQLite/Postgres indexes are derived state. DB-first and cloud-style writes may record the exact + accepted Markdown in `NoteContent` before materializing the file. Follow + [DOMAIN_MODEL.md](DOMAIN_MODEL.md) for the authority and projection rules in each phase. - 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 @@ -15,6 +17,22 @@ surfaces in sync. - 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. +## Functions Before Hierarchies + +- Start with an ordinary, fully typed function. Pair functions with a dataclass when related + state, inputs, or results need a name. +- Use callbacks, closures, or `functools.partial` when binding behavior produces a clearer call + site than another object. Use `functools.singledispatch` only when behavior genuinely varies by + the first argument's runtime type and open registration is an intentional extension point. +- Use a narrow `Protocol` for a capability contract. Prefer structural typing over requiring + implementations to inherit from a shared base. +- Use a concrete class when identity, cohesive mutable state, lifecycle, or resource ownership + requires one. Keep orchestration in the class and move independent computation into functions. +- Reserve abstract base classes for runtime-enforced extension frameworks or shared skeletal + behavior that exists now. Do not introduce inheritance for hypothetical implementations. +- Do not replace class hierarchies with dense functional machinery. Prefer the design with the + fewest concepts, hidden rules, and call hops. + ## Types And Data - Use full type annotations and Python 3.12 syntax. Introduce `type` aliases for repeated @@ -41,6 +59,27 @@ surfaces in sync. - Keep file mutations centralized through the existing file utilities/services so checksum, atomic write, and index synchronization behavior stays coherent. +## Local Reasoning And Abstraction Budget + +- Keep a straightforward workflow together when reading it top-to-bottom is clearer than + navigating helpers. Do not split code merely to reduce function length. +- Extract a helper when its name captures a domain operation, it isolates a side effect or + constraint, it removes meaningful duplication, or it forms a cohesive testable computation. +- Treat a class dominated by private methods as a signal that its behavior may belong in + module-level functions operating on typed values. +- Treat long chains of `_prepare_*`, `_resolve_*`, `_apply_*`, and `_build_*` helpers as a prompt + to simplify the data flow or introduce one meaningful phase value. Private helpers are useful; + private-helper sprawl is not. +- Avoid manager, factory, base, adapter, strategy, and registry abstractions with only one real + implementation. Add extension points when a second behavior or active integration requires + them, not in anticipation of one. +- Avoid dynamic registration, metaprogramming, and decorator-driven control flow unless the + product requires that mechanism and the lifecycle remains explicit. +- Make behavior traceable from an entrypoint to its domain decision and side effects without + reconstructing implicit state across many files. Optimize for human and AI readers alike. +- Every abstraction should reduce the number of concepts or call paths a reader must hold. If a + helper makes the reader navigate more but understand no less, keep the logic local. + ## Testing And Verification - Use evidence-first testing, not mechanical TDD. For bugs and risky behavior, add or update a