Skip to content

Commit 2e247b0

Browse files
committed
refactor(brain-server): organize src/ into thematic sub-modules
Eliminates flat src/ root files in favour of grouped sub-modules, modelled after recall-server's app/bootstrap/http layout. Every former root file now lives under a dedicated folder: src/ ├── main.rs binary entry ├── admin/mod.rs was admin.rs (admin HTTP server) ├── bootstrap/ │ ├── mod.rs new │ ├── shutdown.rs was shutdown.rs (signal + drain) │ └── tls.rs was tls.rs (rustls provider) ├── config/mod.rs was config.rs (TOML schema) ├── llm/ unchanged (summarizer adapters) ├── network/ │ ├── mod.rs new │ ├── connection.rs was connection.rs (Tokio listener) │ ├── dispatch.rs was dispatch.rs (frame FSM) │ ├── routing.rs was routing.rs (RoutingTable) │ └── subscribe.rs was subscribe.rs (SUBSCRIBE bridge) └── shard/ ├── mod.rs was shard.rs (per-shard runtime) └── adapters.rs was shard_adapters.rs (worker glue) Why this layout - main.rs declares the five top-level modules; each is a folder. No source file sits directly in src/ except main.rs. - main.rs adds `use bootstrap::{shutdown, tls};` and `use network::{connection, dispatch, routing, subscribe};` at crate-root scope so every internal `crate::tls::*`, `crate::connection::*` reference keeps working without churning ~30 sites. - Tests use `#[path = "../src/<group>/<file>.rs"] mod <name>;` to load source files at their own top level. The crate-root aliases above match the names tests use, so the same source file compiles cleanly in both contexts. - `shard/mod.rs` declares `pub mod adapters;`; the old `mod shard_adapters;` in tests was removed (clippy::duplicate_mod rejected loading the same file twice through both paths). Compared with recall-server's pattern (see `.claude/plans/recall-server-comparison.md`), brain-server now matches the "every concern in a folder" property, while keeping the binary-protocol-specific structure (single connection listener, frame dispatcher, per-shard Glommio executor) that brain's spec mandates. Pure relocation + import scoping. No semantic edits. The findings doc lists what else recall-server has that we intentionally don't copy (Tower middleware, routes, services — those are HTTP/REST-shaped and don't translate to our binary protocol). docker-verify green.
1 parent ecfa0b6 commit 2e247b0

23 files changed

Lines changed: 289 additions & 70 deletions
Lines changed: 214 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,214 @@
1+
# recall-server organization — what to borrow, what to skip
2+
3+
`arc-labs/recall/crates/recall-server` is a mature axum-based REST
4+
server (163 source files). It demonstrates a clean way to organize
5+
a Rust server crate. Below: which of its ideas apply to brain-server,
6+
which don't, and why.
7+
8+
---
9+
10+
## recall-server's organizing skeleton
11+
12+
```
13+
src/
14+
├── main.rs 32 LOC — runtime + bootstrap::run() only
15+
├── lib.rs — declares the modules
16+
├── app/ — Application + Router + AppState
17+
│ ├── application.rs Application::{build, run}
18+
│ ├── router.rs build_router() with Tower middleware stack
19+
│ ├── state.rs AppState<S> generic over storage backend
20+
│ └── mod.rs
21+
├── bootstrap/ — startup orchestration
22+
│ ├── mod.rs::run() single public entry-point
23+
│ ├── observability.rs tracing subscriber init
24+
│ ├── otel.rs OTLP exporter
25+
│ ├── pools.rs storage pool construction
26+
│ ├── providers.rs LLM / embedder selection
27+
│ └── worker.rs background worker spawn
28+
├── config/ — env-var-loaded config, split by section
29+
│ ├── deployment.rs
30+
│ ├── log.rs
31+
│ ├── providers.rs
32+
│ ├── secrets.rs
33+
│ ├── server.rs
34+
│ └── mod.rs
35+
├── http/ — HTTP-layer infrastructure
36+
│ ├── constants/ api / namespace / plan
37+
│ ├── cursor.rs opaque pagination encoding
38+
│ ├── error/ ApiError, codes, impls
39+
│ ├── extractors/ pagination, scope_filter, time_range, …
40+
│ ├── middleware/ auth, idempotency, rate_limit, security_headers
41+
│ ├── response.rs ApiJson, PaginatedApiJson envelopes
42+
│ └── span.rs handler_event! macros
43+
├── routes/ — one sub-dir per resource group
44+
│ ├── memories/ …
45+
│ ├── entities/ …
46+
│ ├── pipeline/ …
47+
│ └── router.rs merge all sub-routers
48+
├── domain/ — JSON request/response body types
49+
├── services/ — multi-step orchestration
50+
├── identity/ — auth + tenancy
51+
├── messaging/ — control-plane event bus, write-job SSE
52+
├── email/ — sender + templates
53+
└── crypto.rs — AES-256-GCM helpers
54+
```
55+
56+
The headline pattern: **main.rs is 32 lines.** Every concern has a
57+
home; the entry point doesn't reach for any of them directly.
58+
59+
---
60+
61+
## What translates to brain-server
62+
63+
Brain-server is a binary-protocol server (Glommio + Tokio bridge), not
64+
REST. Most of recall's layers don't map. But two big wins do:
65+
66+
### 1. **`main.rs``bootstrap/` + thin main** (high value)
67+
68+
brain-server's `main.rs` is **488 LOC**. It performs:
69+
70+
1. CLI arg parsing.
71+
2. Tracing subscriber init (fmt + optional JSON, optional OTel).
72+
3. Config load + validation.
73+
4. Summarizer factory wiring.
74+
5. Tokio runtime construction.
75+
6. Inside the runtime: shutdown signal channel, signal listener spawn,
76+
TLS build, ConnectionMetrics construction, AdminServer construction
77+
+ bind + spawn, ConnectionListener construction + bind + spawn,
78+
serve() await, admin drain timeout, shard graceful shutdown.
79+
80+
That's 5+ orthogonal concerns. recall's pattern splits this into:
81+
82+
```
83+
bootstrap/
84+
├── mod.rs run() — orchestration only
85+
├── tracing.rs init_tracing(LogConfig)
86+
├── tls.rs build_tls(TlsConfig) — currently in tls.rs root
87+
├── shards.rs spawn_shards(cfg, summarizer) + ShardSpawn helpers
88+
├── admin.rs build_admin(cfg, state) → AdminServer (binds inside)
89+
├── listener.rs build_listener(cfg, tls, topology, metrics) → ConnectionListener
90+
├── shutdown.rs signal handler + graceful drain — currently in shutdown.rs
91+
└── summarizer.rs summarizer factory — currently inside main.rs
92+
```
93+
94+
After: `main.rs` would be ~40 LOC (CLI parse + Tokio runtime build +
95+
`bootstrap::run(cfg).await`). The existing `tls.rs`, `shutdown.rs`,
96+
`llm/` would slot under bootstrap/ naturally.
97+
98+
**Risk**: low. Pure relocation. The runtime model and shard
99+
discipline don't change.
100+
101+
### 2. **`config.rs``config/`** (high value)
102+
103+
brain-server's `config.rs` is **578 LOC** of `[server]`, `[storage]`,
104+
`[shard]`, `[hnsw]`, `[embedder]`, `[workers]`, `[logging]`,
105+
`[tracing]`, `[auth]`, `[summarizer]` sections plus parsers
106+
(`parse_human_bytes`, `coerce_leaf`, `set_path`, `deserialize_*`).
107+
108+
Recall's `config/` mirrors its env-var sections one-per-file. Brain
109+
can mirror its TOML sections:
110+
111+
```
112+
config/
113+
├── mod.rs Config struct + load + validation
114+
├── server.rs ServerConfig + TlsConfig
115+
├── storage.rs StorageConfig + ShardConfig
116+
├── index.rs HnswConfig + EmbedderConfig
117+
├── workers.rs WorkersConfig
118+
├── observability.rs LoggingConfig + TracingConfig
119+
├── auth.rs AuthConfig
120+
├── summarizer.rs SummarizerConfig (currently inline)
121+
└── parse.rs parse_human_bytes, coerce_leaf, set_path
122+
```
123+
124+
Each section file: ~50–80 LOC. mod.rs: ~100 LOC for the top-level
125+
`Config` + `load_from_file` + `apply_overrides`.
126+
127+
**Risk**: low. Section structs are independent; only `parse.rs`
128+
helpers cross-cut.
129+
130+
### 3. **`admin.rs` (463 LOC) — partial: keep monolithic but lift constants** (low value)
131+
132+
recall's `http/constants/`, `http/error/`, `http/response.rs` are
133+
beautiful in an HTTP REST context. brain-server's admin.rs is a
134+
hand-rolled HTTP/1.1 implementation intentionally kept tight (no
135+
hyper, no Tower). The constants we *do* have (path strings, status
136+
codes, JSON body shapes) could live in
137+
`admin/{routes,response,handlers}.rs`, but the win is small.
138+
139+
**Recommendation: skip**. The cost of admin/ split outweighs the
140+
~30% navigability gain. Revisit if admin grows past 1k LOC.
141+
142+
---
143+
144+
## What does NOT translate
145+
146+
| recall pattern | Why it doesn't fit brain |
147+
|---|---|
148+
| `app/router.rs` Tower middleware stack | Brain's connection-listener has a single FSM dispatch (HELLO → AUTH → Established → frame-dispatch). The "stack" is opcodes + idempotency lookup + auth-phase guard, all already in `dispatch.rs`. Wrapping in a Tower-like layer would force boxed-Service ceremony for zero behaviour gain. |
149+
| `routes/{memories,entities,pipeline,…}` | Brain has 30 wire opcodes dispatched in `dispatch.rs`. They're not "routes" — they're stream operations with shard-affinity. Splitting `dispatch.rs` further would over-fragment the FSM. |
150+
| `services/` | Brain's "service layer" is the brain-ops crate. Don't duplicate. |
151+
| `domain/{request,response}` JSON types | Brain has the brain-protocol crate (rkyv-encoded binary). The recently-shipped refactor D already gave it `requests/` + `responses/` sub-modules. |
152+
| `http/middleware/{auth,rate_limit,idempotency,security_headers}` | Brain's auth is per-connection FSM, idempotency lives in brain-ops, rate-limiting is a connection-limit (spec §03/06). No mid-stream middleware needed. |
153+
| `http/extractors/` | Brain wire frames are rkyv-decoded; there are no query strings to extract. |
154+
| `identity/`, `tenancy/`, `email/`, `crypto/` | Out of spec scope for v1 (auth is `AuthMethod::None` per spec §03/06 §3.1). |
155+
| `messaging/` (in-process event bus + write-job SSE) | Brain has `brain-ops/subscribe` (SUBSCRIBE bridge). Don't duplicate. |
156+
| OTLP / `otel.rs` | Brain has it in cargo deps (`opentelemetry`) but no exporter yet — that's a Phase 14 observability task. |
157+
158+
---
159+
160+
## Spec considerations
161+
162+
Any refactor must preserve:
163+
164+
- **Single-writer-per-shard** discipline (CLAUDE.md §5 inv. 2).
165+
- **Glommio executor per shard** model (CLAUDE.md §9: no Tokio inside a shard).
166+
- **ConnectionListener** structure (spec §10).
167+
- **Hand-rolled HTTP/1.1 for admin** (decided in 9.13 — no hyper).
168+
- **Tokio↔Glommio boundary** at `dispatch.rs` (sub-task 9.10).
169+
170+
The two recommended refactors (bootstrap/, config/) touch only
171+
main.rs, config.rs, and the standalone helper files. They don't
172+
cross any of the above lines.
173+
174+
---
175+
176+
## Recommended sequence (if you choose to act)
177+
178+
| # | Refactor | LOC moved | Files added | Risk |
179+
|---|---|---|---|---|
180+
| 1 | `main.rs``bootstrap/` + thin main | ~450 | ~6 | Low |
181+
| 2 | `config.rs``config/` | ~580 | ~8 | Low |
182+
| 3 | (skip) `admin.rs` split ||||
183+
184+
Both refactors are pure relocation with import-scoping. ~2 commits,
185+
~30 min each, docker-verify between.
186+
187+
After: brain-server's `src/` becomes:
188+
189+
```
190+
src/
191+
├── main.rs ~40 LOC
192+
├── lib.rs module declarations
193+
├── bootstrap/ 6 files — startup orchestration
194+
├── config/ 8 files — config sections + parsers
195+
├── shard.rs ~975 LOC (core shard runtime)
196+
├── shard_adapters.rs ~735 LOC (worker adapter glue)
197+
├── connection.rs ~780 LOC (Tokio listener)
198+
├── dispatch.rs ~700 LOC (frame FSM + boundary)
199+
├── subscribe.rs ~430 LOC (SUBSCRIBE bridge)
200+
├── admin.rs ~460 LOC (admin HTTP/1.1)
201+
├── routing.rs ~280 LOC (RoutingTable)
202+
├── llm/ 6 files (summarizer adapters, already split)
203+
└── tests/ in-process E2E fixtures
204+
```
205+
206+
The "infra" (bootstrap, config) is grouped; the per-feature concerns
207+
(shard, connection, dispatch, subscribe, admin, routing) stay at the
208+
root as cohesive units — which matches recall's pattern where
209+
`app/`, `bootstrap/`, `http/`, `routes/` are dirs but
210+
single-concern feature files stay at the root.
211+
212+
---
213+
214+
*Awaiting direction on whether to execute refactor 1, 2, or both.*
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
//! Startup orchestration: TLS provider build, signal handling, and
2+
//! graceful shard drain. The pieces here are wired together by
3+
//! `main::run`.
4+
5+
#[cfg(target_os = "linux")]
6+
pub mod shutdown;
7+
#[cfg(target_os = "linux")]
8+
pub mod tls;
File renamed without changes.

crates/brain-server/src/main.rs

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,26 +11,29 @@
1111

1212
#[cfg(target_os = "linux")]
1313
mod admin;
14+
mod bootstrap;
1415
mod config;
1516
#[cfg(target_os = "linux")]
16-
mod connection;
17-
#[cfg(target_os = "linux")]
18-
mod dispatch;
19-
#[cfg(target_os = "linux")]
2017
mod llm;
21-
mod routing;
18+
#[cfg(target_os = "linux")]
19+
mod network;
2220
#[cfg(target_os = "linux")]
2321
#[allow(dead_code)] // consumed by the connection layer in sub-task 9.10.
2422
mod shard;
23+
24+
// Crate-root aliases. The folder reorg moved each module into a
25+
// thematic sub-module (`bootstrap::tls`, `network::connection`, …),
26+
// but every file references its peers by the historical top-level
27+
// name (`crate::tls`, `crate::connection`, …). Re-exporting at the
28+
// crate root preserves those paths and matches the way integration
29+
// tests load source files via `#[path]` + `mod xxx;`.
2530
#[cfg(target_os = "linux")]
26-
#[allow(dead_code)] // adapters wired into the per-shard scheduler in 9.8.
27-
mod shard_adapters;
28-
#[cfg(target_os = "linux")]
29-
mod shutdown;
31+
use bootstrap::{shutdown, tls};
3032
#[cfg(target_os = "linux")]
31-
mod subscribe;
33+
use network::{connection, dispatch, routing, subscribe};
3234
#[cfg(target_os = "linux")]
33-
mod tls;
35+
#[allow(unused_imports)] // re-export kept for symmetry; binary doesn't reach it directly
36+
use shard::adapters as shard_adapters;
3437

3538
use std::env;
3639
use std::path::PathBuf;
File renamed without changes.
File renamed without changes.
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
//! Tokio connection layer: the per-listener accept loop
2+
//! (`connection`), the Tokio↔Glommio frame dispatcher (`dispatch`),
3+
//! the agent→shard routing table (`routing`), and the SUBSCRIBE bridge
4+
//! (`subscribe`).
5+
6+
#[cfg(target_os = "linux")]
7+
pub mod connection;
8+
#[cfg(target_os = "linux")]
9+
pub mod dispatch;
10+
#[cfg(target_os = "linux")]
11+
pub mod routing;
12+
#[cfg(target_os = "linux")]
13+
pub mod subscribe;

0 commit comments

Comments
 (0)