Skip to content

feat(continuum-core/persona): L0-2-respond-context — required ResponderConfig, NeedsResponse outcome, no empty defaults#1467

Merged
joelteply merged 1 commit into
canaryfrom
8d11027b/feat-continuum-core-persona-l0-2-respond
May 29, 2026
Merged

feat(continuum-core/persona): L0-2-respond-context — required ResponderConfig, NeedsResponse outcome, no empty defaults#1467
joelteply merged 1 commit into
canaryfrom
8d11027b/feat-continuum-core-persona-l0-2-respond

Conversation

@joelteply
Copy link
Copy Markdown
Contributor

Reworked from the earlier L0-2-respond attempt (#1466, self-closed after audit). Card 8d11027b.

Why the rewrite

Audit found three doctrine violations in the first attempt:

  1. std::Mutex held across respond().await — blocks status/enroll/other personas for the full inference roundtrip
  2. Empty-default fields on RespondInput rationalized as 'fail loudly at inference' — that's the silent-default-substitution pattern this migration is deleting on the TS side
  3. RespondError as Ok outcome — circuit breaker never trips on repeated inference failures (silent degradation)

This slice fixes them by SHRINKING the scope: no respond() call yet. That's the next slice, which can rely on RespondInput being honestly constructed.

What this slice DOES

  • New ResponderConfig (model, system_prompt, capabilities, specialty). All required at enrollment. Validated non-empty with named errors for model + specialty.
  • EnrolledPersona extends with responder_config field
  • enroll signature requires ResponderConfig parameter; rejected enrollments don't mutate state
  • persona/enroll command parses model/system_prompt/specialty/capabilities from JSON; requires model loud
  • ServiceOnceOutcome updated:
    • SilentByDecision { message_id, decision } — gate said no
    • NeedsResponse { message_id, decision, respond_input } — gate said yes; respond_input is fully-formed from real config
    • UnsupportedItem unchanged
    • Evaluated REMOVED
  • service_once_for builds RespondInput from real persona config + per-message context; no empty-string defaults
  • The genuinely-empty Vec fields (recent_history, known_specialties, other_persona_names, message_media, recalled_engrams) are LEGITIMATELY empty for first-turn context, not silent substitutes

What this slice does NOT do

  • Call respond() — next slice owns the lock-around-await discipline + inference-error-trips-circuit-breaker contract
  • Wire persona/enroll from production code — L0-2-cutover

Tests — 19/19 passing

16 pre-existing + 3 new doctrine pins:

  • enroll_with_empty_model_is_rejected_loud
  • enroll_with_empty_specialty_is_rejected_loud
  • enroll_command_requires_model
  • service_once_for dispatch test extended to verify RespondInput carries the persona's real model/specialty/system_prompt, NOT empty defaults

🤖 Generated with Claude Code

…erConfig, NeedsResponse outcome, no empty defaults

Reworked from the earlier L0-2-respond attempt (#1466, self-closed)
after auditing three doctrine violations:
1. std::Mutex held across respond().await — blocks status/enroll/other
   personas' ticks for the full inference roundtrip
2. Empty-default fields on RespondInput (model: String::new(), etc.)
   wrapped as 'fail loudly at inference' — that's the silent-default-
   substitution pattern this migration is deleting on the TS side
3. RespondError as Ok outcome — circuit breaker never trips on
   repeated inference failures (silent degradation)

This slice fixes them all by SHRINKING the scope: no respond() call
yet. That's the next slice, which can rely on RespondInput being
honestly constructed.

What this slice does:

- New ResponderConfig struct (model, system_prompt, capabilities,
  specialty). All required at enrollment time; validated non-empty
  with named errors for model + specialty
- EnrolledPersona extends with responder_config field
- enroll signature requires ResponderConfig as a parameter; rejected
  enrollments don't mutate state (validate before lock)
- persona/enroll command parses model/system_prompt/specialty/
  capabilities from JSON params; requires model loud
- ServiceOnceOutcome updated:
  - SilentByDecision { message_id, decision } — gate said no
  - NeedsResponse { message_id, decision, respond_input } — gate
    said yes; respond_input is fully-formed from real config
  - UnsupportedItem unchanged
  - Idle unchanged
  - Evaluated REMOVED
- service_once_for: pops + evaluates; if should_respond, builds
  RespondInput from real persona config + per-message context; no
  empty-string defaults
- build_respond_input populates EVERY required field from
  responder_config + the chat wire. The genuinely-empty Vec fields
  (recent_history, known_specialties, other_persona_names,
  message_media, recalled_engrams) are LEGITIMATELY empty for
  first-turn fresh context, not silently-substituted defaults

What this slice does NOT do:
- Call respond(). Next slice owns that, plus the lock-around-await
  discipline + inference-error-trips-circuit-breaker contract
- Wire persona/enroll from production code. L0-2-cutover

Tests: 19/19 passing. 16 pre-existing + 3 new doctrine pins:
- enroll_with_empty_model_is_rejected_loud
- enroll_with_empty_specialty_is_rejected_loud
- enroll_command_requires_model
- service_once_for dispatch test extended to verify the
  RespondInput carries the persona's real model/specialty/
  system_prompt, not empty defaults

Verified on Xcode 26.3 + llama/metal feature.

Card: 8d11027b
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@joelteply joelteply merged commit 50312e4 into canary May 29, 2026
3 checks passed
@joelteply joelteply deleted the 8d11027b/feat-continuum-core-persona-l0-2-respond branch May 29, 2026 23:12
joelteply added a commit that referenced this pull request May 29, 2026
…around-await, inference CB threshold (#1468)

Stacks on L0-2-respond-context (#1467). Three contracts the previous
attempt got wrong, all specified properly + tested here:

1. **Lock discipline.** std::sync::Mutex on personas — the compiler
   forces correctness: can't be held across .await. drain_all_personas
   does the lock-decide-drop-respond-relock dance. Production safety:
   status/enroll/other personas don't block across multi-second
   inference calls.

2. **Inference errors trip CB with HIGHER threshold than service.**
   Two counters per persona:
   - consecutive_service_failures (threshold 5) for deserialization /
     channel access / lock failures
   - consecutive_inference_failures (threshold 15) for respond() errors
   Preserves 'transient hiccup ≠ broken persona' while still surfacing
   'model never loads' as back-pressure at the 15-error mark.

3. **Responder trait for DI.** Production uses DefaultResponder which
   calls persona::response::respond. Tests inject MockResponder that
   records calls + returns scripted outcomes (PersonaResponse::Spoke
   or Err) without loading a real model.

What changes:
- New Responder trait + DefaultResponder impl
- PersonaServiceModule holds Arc<dyn Responder>; new() defaults to
  DefaultResponder; with_responder() for test injection
- EnrolledPersona: consecutive_failures split into
  consecutive_service_failures + consecutive_inference_failures
- ServiceOnceOutcome (the caller-facing variants) restructured:
  Idle | SilentByDecision | Responded{response: PersonaResponse} |
  UnsupportedItem
- ServicePopDecision (NEW, sync-step output): Idle | Silent |
  NeedsResponse | UnsupportedItem — what service_once_for returns
  inside the lock
- service_once_for: signature changes to return ServicePopDecision
  (sync step). Same body, just renamed outcome
- drain_all_personas: rewritten with proper lock discipline. async,
  drops lock around responder.respond().await
- New helper with_persona(): briefly lock the map and mutate the
  named persona; closure runs sync inside lock
- tick: awaits drain_all_personas

What does NOT change yet:
- No production code calls persona/enroll. Tick still runs over
  empty map.
- TS PersonaAutonomousLoop still drives production. L0-2-cutover.
- Real inference still requires model loading — tests use mock.

Tests: 24/24 passing.
Pre-existing 19 + 5 new:
- drain_calls_responder_when_gate_says_yes
- drain_does_not_call_responder_when_gate_says_no
- inference_errors_eventually_trip_circuit_at_inference_threshold
- inference_failure_below_threshold_does_not_trip_circuit
- successful_response_resets_inference_failure_counter

Verified on Xcode 26.3 + llama/metal feature.

Card: 34f28611

Co-authored-by: Claude Opus 4.7 (1M context) <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