Skip to content

refactor(persona): &ctx-pure RAG request + ctx-derived tracing span#1512

Open
joelteply wants to merge 2 commits into
canaryfrom
refactor/persona-ctx-derivation
Open

refactor(persona): &ctx-pure RAG request + ctx-derived tracing span#1512
joelteply wants to merge 2 commits into
canaryfrom
refactor/persona-ctx-derivation

Conversation

@joelteply
Copy link
Copy Markdown
Contributor

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 &ctx doctrine ([[context-is-the-client-airc-token-is-identity]]) more uniformly applied in the persona/cognition path:

  1. RagInspectionRequest::for_ctx(&ctx, now_ms) — derives RAG request from the persona context object directly. Replaces for_persona(persona_id, name, now_ms, &profile) at call sites (4 args → 2). for_persona stays as the underlying derivation.
  2. PersonaContext::span() — 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 site.
  3. serve_persona_loop two-layer rewrite — outer thin entry wraps the inner future with .instrument(ctx.span()). Inner reads ctx.identity.peer_id etc. directly. Internal tracing::warn! lines lose redundant persona_id / agent_name fields (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 --tests clean
  • cargo test persona::service_loop — 4 passed
  • cargo test persona::supervisor — 4 passed
  • CI passes on cross-platform builds

🤖 Generated with Claude Code

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant