refactor(persona): &ctx-pure RAG request + ctx-derived tracing span#1512
Open
joelteply wants to merge 2 commits into
Open
refactor(persona): &ctx-pure RAG request + ctx-derived tracing span#1512joelteply wants to merge 2 commits into
joelteply wants to merge 2 commits into
Conversation
Elegance pass on the patterns the slice-13 work established. Per
Joel 2026-06-02: "we are on sort of an elegance refactor and then
for improved reliability and speed."
What changed:
1. `RagInspectionRequest::for_ctx(&ctx, now_ms)` — new constructor
that takes the persona context directly. Replaces the 4-arg
`for_persona(persona_id, name, now_ms, &profile)` at the call
site. `for_persona` stays (it's the underlying derivation) but
new code uses `for_ctx` to honor the substrate's `&ctx`
doctrine ([[context-is-the-client-airc-token-is-identity]]):
hand the context, not its parts.
2. `PersonaContext::span()` — new method that returns a
`tracing::info_span!` tagged with `persona_id`, `agent_name`,
`peer_id`, `role`, `tier`, `ctx_len`, `model`. The span derives
from `&ctx` — no manual field threading at every log call site.
3. `serve_persona_loop` rewritten in two layers:
- Outer entry function wraps the inner future with
`.instrument(ctx.span())`. Every log line inside the loop
inherits the persona's identity fields automatically.
- Inner function drops the `let persona_id = hosted.identity.x`
extractions; reads `ctx.identity.peer_id` etc. directly at use
sites. Two internal `tracing::warn!` lines lose their
persona_id/agent_name fields (now inherited from the span);
they keep just per-turn delta (`lamport`, `error`).
Net effect:
- Field extraction count in service_loop drops from 3 manual extracts
+ 4 redundant tracing field annotations to 0.
- Log output gains persona_id + agent_name + role + tier + ctx_len
+ model on EVERY internal log line, automatically. The substrate's
observability is now span-shaped, not manual.
- New code that needs a derived RAG request just writes
`RagInspectionRequest::for_ctx(ctx, now)` — one arg vs four.
Why `.instrument` not `.entered`:
- `Span::entered` returns a non-Send RAII guard; tokio spawned
futures need Send. The two-function split (outer thin wrapper
with `.instrument`, inner async function) is the standard tracing
pattern for spans across awaits.
Verification:
- cargo build --lib --tests clean
- cargo test persona::service_loop — 4 passed
- cargo test persona::supervisor — 4 passed
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Elegance pass — extract-class refactor pulling the 170-line inline
boot composition out of `ipc/mod.rs::start_server` into a named
class. Per Joel 2026-06-02: "Must have elegance obsessively. Like a
Java dev. NO SHAME. It's better."
What changed:
1. `PersonaSpawnSupervisor` struct (in `persona/host.rs`) owns the
spawner / instance_manager / registry / factory / tier_id /
model_registry / rt_handle inputs. Construct once at boot; call
`.spawn_all(&mut provider)` to produce a `BootSummary`.
2. `BootSummary { hosted, failures }` + `BootSlotFailure {
slot_index, role, persona_id, reason }` — typed result structs.
Replace the inline `let mut hosted_count: usize = 0` / `let mut
failed_count: usize = 0` counters with a real value type the
substrate can publish (`persona:boot:summary` event — Q5 of the
design doc, deferred to slice 13.5+) and downstream clients
(web, jtag CLI) can read with the same shape per
[[clients-are-rust-too-thin-node-web-shell]].
3. The supervisor's `spawn_all` method handles every previously-
inline concern:
- `bootstrap_planned` failure → orderly-drain orphans + return
summary with synthetic failure row
- `materialize_adapters` with runtime_lookup closure (so
`ctx.runtime` is populated from the registry)
- Per-slot `spawn_and_attach` private method handles
`spawn_persona_service` + `attach_service_loop` + handle drain
on attach-failure (the BLOCKER 1/2 fixes from PR #1511 are
preserved, just relocated)
4. IPC boot collapses from ~170 lines of inline code to ~30 lines:
construct supervisor → spawn task → build provider → call
`supervisor.spawn_all(&mut provider).await` → log summary.
5. Helper `supervisor_error_facts` centralizes pulling
`(slot_index, role)` out of `SupervisorError`'s two variants —
the kind of trivial-but-DRY private fn Java/dotnet shops write
without apology.
Why this matters (the doctrine):
- The IPC server boot concern and the persona spawn concern had
different lifetimes and different test needs. Mixing them in
one function violated "one logical decision, one place"
([[compression-principle]]).
- `PersonaSpawnSupervisor` is now unit-testable in isolation. The
IPC server's test surface shrinks. Slice 14's RoleAwareProvider
+ multi-persona work has one named insertion point.
- `BootSummary` is the structured event payload the design doc's
Q5 named. Once `RoleId` derives `TS` (slice 14), the struct gets
the ts-rs export and web/jtag clients read it directly per the
Rust-first-clients doctrine.
Verification:
- cargo build --lib --tests clean
- cargo test persona::host — 2 passed (BootSummary attempted +
serde camel-case)
- cargo test persona::supervisor — 4 passed (unchanged)
- cargo test persona::service_loop — 4 passed (unchanged)
- IPC boot composition shrinks ~140 lines; supervisor's spawn_all
is now the single named extraction point for slice 13.5 / 14
changes.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Open
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Elegance pass on the patterns slice 13 established. Per Joel 2026-06-02: "elegance refactor → reliability + speed."
Three tightly-scoped changes that make the substrate's
&ctxdoctrine ([[context-is-the-client-airc-token-is-identity]]) more uniformly applied in the persona/cognition path:RagInspectionRequest::for_ctx(&ctx, now_ms)— derives RAG request from the persona context object directly. Replacesfor_persona(persona_id, name, now_ms, &profile)at call sites (4 args → 2).for_personastays as the underlying derivation.PersonaContext::span()— returns atracing::info_span!tagged with persona_id / agent_name / peer_id / role / tier / ctx_len / model. The span derives from&ctx— no manual field threading at every log site.serve_persona_looptwo-layer rewrite — outer thin entry wraps the inner future with.instrument(ctx.span()). Inner readsctx.identity.peer_idetc. directly. Internaltracing::warn!lines lose redundantpersona_id/agent_namefields (now span-inherited).Reliability + observability win
Every log line emitted during a persona's serve loop now carries her full identity automatically. The substrate's [[observability-is-half-the-architecture]] doctrine is now span-shaped, not manual.
Reference docs
Test plan
cargo build --lib --testscleancargo test persona::service_loop— 4 passedcargo test persona::supervisor— 4 passed🤖 Generated with Claude Code