docs: ARCHITECTURE.md — system overview + data flow diagrams (#116)#135
docs: ARCHITECTURE.md — system overview + data flow diagrams (#116)#135
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c00ab6c14c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| - `PUT /items` — store memory | ||
| - `GET /items/:id` — retrieve by ID | ||
| - `POST /recall` — recall packs | ||
| - `POST /query` — text search | ||
| - `POST /search` — vector search |
There was a problem hiding this comment.
crates/server
The router exposed by wagl serve in crates/server/src/lib.rs:345-356 only registers GET/POST /items, GET/DELETE /items/{id}, GET/POST /intents, DELETE /intents/{id}, plus /health, /status, and /ws. Documenting PUT /items, /recall, /query, and /search here means anyone building against the new architecture page will hit 404/405 responses immediately because those endpoints do not exist in this commit.
Useful? React with 👍 / 👎.
| - 5 tools: `store`, `recall`, `query`, `search`, `forget` | ||
| - stdio transport (launched as subprocess by MCP clients) |
There was a problem hiding this comment.
search tool in the tool list
The MCP server registered in crates/mcp/src/handler.rs:71-173 exposes memory_store, memory_recall, memory_query, memory_context, and memory_forget; there is no search tool. Publishing store/recall/query/search/forget here will cause MCP clients to ask for a capability the server never registers, while also hiding the actual context-pack tool they can use.
Useful? React with 👍 / 👎.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| ┌────────┴─────────┐ | ||
| │ wagl-core │ | ||
| │ (types, no IO) │ | ||
| └──────────────────┘ | ||
| │ | ||
| ┌────────┴─────────┐ | ||
| │ libSQL │ | ||
| │ (local + Turso │ | ||
| │ embedded sync) │ | ||
| └──────────────────┘ |
There was a problem hiding this comment.
wagl-core → libSQL
The diagram draws wagl-core as a pass-through between wagl-db and libSQL, implying wagl-core depends on libSQL. But wagl-core is explicitly described as "types, no IO" with no database or network dependencies. Key Design Decision #6 also confirms the correct direction: core (types) → db (storage) → cli/server/mcp.
libSQL should branch off from wagl-db directly (alongside wagl-core), not below wagl-core. A corrected layout:
┌──────────────────┐
│ wagl-db │
│ (libSQL layer) │
└────────┬─────────┘
┌───────┴───────┐
│ │
┌──────────┴───┐ ┌───────┴──────┐
│ wagl-core │ │ libSQL │
│ (types, noIO)│ │ (local+Turso │
└──────────────┘ │ embedded) │
└──────────────┘
As written, this will mislead contributors into thinking wagl-core has a storage dependency.
| All libSQL/SQLite interaction lives here. Owns the schema and migrations. | ||
|
|
||
| - `MemoryDb` — connection wrapper with sync support | ||
| - `migrate.rs` — schema versioning (currently v1, v2 in PR) |
There was a problem hiding this comment.
"v2 in PR" is a development-time annotation that should be resolved before merging to dev. Either the v2 schema is already present (update the note to say "currently v2") or it is not yet merged (remove the forward reference).
| - `migrate.rs` — schema versioning (currently v1, v2 in PR) | |
| - `migrate.rs` — schema versioning (currently v1) |
| ## Embedding Strategy | ||
|
|
||
| - **Local CLI**: embeddings via configurable OpenAI-compatible endpoint | ||
| - **Cloud (zumie.ai)**: Gemini `gemini-embedding-001` (768 dimensions) |
There was a problem hiding this comment.
AGENTS.md explicitly instructs: "Do not commit … internal hostnames, internal paths, or identifying operational details" and to use https://example.com/... instead of real internal service URLs.
zumie.ai appears twice — here and on line 155. If this is an internal/private service, both references should be genericised (e.g., your-cloud-backend). If it is intentionally public-facing, a brief comment in the PR description would clarify that.
The same applies to line 155:
Per-user isolated databases in zumie.ai (multi-tenant)
Context Used: AGENTS.md (source)
| @@ -0,0 +1,168 @@ | |||
| # ARCHITECTURE.md | |||
There was a problem hiding this comment.
# ARCHITECTURE.md as a rendered heading looks odd (the .md extension is typically omitted). Compare with AGENTS.md which uses a plain prose heading. Consider:
| # ARCHITECTURE.md | |
| # Architecture |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
Self-review: ✅ System overview diagram shows all 4 surfaces → db → core → libSQL layering. Note: References "v2 in PR" for migrations — update after #127/#128 merge. |
|
Pushed fixes for review findings:
|
1 participant
ASCII art system diagram, crate structure table, recall data flow, scoring system explanation, embedding strategy, sync architecture, and key design decisions.
Partial close of #116 (diagrams done, cargo doc generation not yet addressed).
Pairs well with the AGENTS.md refresh (#126, already merged).
PR Review by Greptile
Greptile Summary
This PR introduces
ARCHITECTURE.mdas a top-level reference document for contributors: an ASCII system diagram, crate structure table, recall data-flow walkthrough, scoring system explanation, embedding strategy, sync architecture, and key design decisions. The content is accurate and well-structured overall, but one diagram error needs fixing before the doc is safe to use as a contributor reference.Key findings:
wagl-db → wagl-core → libSQL, implyingwagl-corehas a storage dependency. The crate description and Key Design Decision Phase 1: Add --dedupe-key and --upsert for idempotent puts #6 both confirmwagl-coreis purely types with no IO —libSQLshould branch offwagl-dbdirectly alongsidewagl-core, not below it.migrate.rs — schema versioning (currently v1, v2 in PR)is an unresolved development annotation; the "v2 in PR" suffix should be removed or updated to reflect the actual current state.zumie.aiis named twice (Embedding Strategy and Sync Architecture).AGENTS.mdinstructs against committing internal hostnames/service URLs; if this is a private backend, it should be genericised per that policy.ARCHITECTURE.md(with extension) —# Architectureis the conventional form.Confidence Score: 3/5
wagl-core → libSQLarrow, which would actively mislead contributors about the crate dependency structure.migrate.rsline (53).Important Files Changed
wagl-core→libSQL, misrepresenting core as having a storage dependency. Also contains a stale "v2 in PR" note and twozumie.aireferences that may violate AGENTS.md's internal-hostname policy.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A["Agent / Human\n(OpenClaw, ChatGPT, Claude, CLI)"] A --> CLI["crates/cli\nwagl binary"] A --> Server["crates/server\nHTTP REST"] A --> MCP["crates/mcp\nstdio transport"] A --> OC["OpenClaw plugin"] CLI --> DB Server --> DB MCP --> DB OC --> DB DB["crates/db\nwagl-db\n(libSQL layer)"] DB --> Core["crates/core\nwagl-core\n(types, no IO)"] DB --> LibSQL["libSQL\n(local + Turso\nembedded sync)"] LibSQL --> Local["Local SQLite file"] LibSQL --> Turso["Turso Cloud"]Last reviewed commit: "docs: add ARCHITECTU..."
Context used: