70 lines
12 KiB
Markdown
70 lines
12 KiB
Markdown
## Context
|
|
|
|
`agent-runtime` (`apex-agent-mvp`, code-complete but unapplied) runs a single Observe→Think→Act loop: `TaskRunner.run()` (`runtime/task.py`) calls `Planner.plan()` once per iteration and hands every produced `PlannedStep` to `Executor.execute()`, whose only failure signal is "did the tool call raise" — retried with bounded backoff (`max_retries`). Nothing in that loop ever asks "did the intended effect actually happen," using anything richer than the tool call's own return value: a `tap` on a send button can return `success=True` from the driver while the message never actually sent. `semantic-scene-runtime` (Milestone 5) and `world-model-runtime` (Milestone 6) are adding richer, semantically-informed state (`SemanticScene`, `WorldState`) that a single Planner+Executor pair has no natural place to use for verification or recovery — today that state is planning input only. This change adds three new collaborating roles — Observer, Verifier, Reflector — around the existing, unmodified Planner/Executor, so that semantic verification and bounded recovery become first-class steps in the loop, layered above (not replacing) the Executor's existing low-level retry/backoff safety net.
|
|
|
|
## Goals / Non-Goals
|
|
|
|
**Goals:**
|
|
- Define a handoff protocol — `Observation`, `VerificationVerdict`, `ReflectionOutcome` — as plain dataclasses passed between roles, so the protocol itself, not any one role's internal implementation, is the stable, spec'd contract.
|
|
- Add a `CollaborativeTaskRunner` that composes the existing `Planner`, `Executor`, and `TaskRunner` (`agent-runtime`) strictly by import, in a defined loop: Observer → Planner → Executor → Verifier → (Reflector only on a Verifier-flagged failure) → back to Observer.
|
|
- Make multi-agent collaboration opt-in per task run, defaulting to **disabled**, so applying this change never silently changes any existing task's cost, latency, or step count.
|
|
- Bound Reflector-triggered replanning with an explicit ceiling, separate from and on top of the Executor's own `max_retries`, so a persistently-failing step cannot loop forever between Verifier and Reflector.
|
|
- Reuse the existing `semantic/llm_client.py` LLM client abstraction (added by `semantic-scene-runtime`) for Verifier/Reflector reasoning, rather than introducing a second LLM client abstraction.
|
|
|
|
**Non-Goals:**
|
|
- No change to `runtime/planner.py`'s `Planner`, `runtime/executor.py`'s `Executor`, or `runtime/task.py`'s `TaskRunner` — multi-agent collaboration is an opt-in alternate driver of the same Task/Planner/Executor contracts, not a replacement for them.
|
|
- No new LLM provider/vendor integration — Verifier/Reflector reasoning reuses the existing client abstraction.
|
|
- No persistence of `Observation`/`VerificationVerdict`/`ReflectionOutcome` into `storage/timeline.py` in this change (see Open Questions).
|
|
- No change to `workflow/runner.py`'s step-handler contract — `CollaborativeTaskRunner` is usable as an alternate driver for a Workflow's planned-goal step, but wiring that composition is left to whichever change adopts it, not authored here.
|
|
|
|
## Decisions
|
|
|
|
### D1: New `agents/` package, not folded into `runtime/`
|
|
The Observer/Verifier/Reflector roles and the handoff protocol are conceptually "one layer above" the existing Observe→Think→Act loop, with different characteristics (LLM calls, optionality, per-task opt-in). A sibling `agents/` package (`models.py`, `observer.py`, `verifier.py`, `reflector.py`, `collab_runner.py`, `config.py`) keeps `runtime/`'s existing contract unchanged and makes the new roles' composition explicit at the package boundary, mirroring `semantic-scene-runtime`'s `semantic/` package precedent.
|
|
**Alternative considered**: add verification/reflection hooks directly onto `TaskRunner`. Rejected — it would force every `TaskRunner` caller to reason about the new roles' failure modes and config even when not using them.
|
|
|
|
### D2: Handoff protocol as plain dataclasses with no role-specific side effects
|
|
`Observation`, `VerificationVerdict`, and `ReflectionOutcome` (plus `ReflectionAction`) are plain dataclasses with `to_dict()`/`from_dict()`, mirroring `core/models.py`'s style. No role reaches into another role's internals; each role's public surface is "takes typed input, returns typed output."
|
|
**Alternative considered**: pass a single mutable shared context object between roles (similar to `TaskContext`). Rejected — a shared mutable object makes it harder to reason about which role produced which piece of state, and the whole point of this change is making each handoff an explicit, testable data contract.
|
|
|
|
### D3: `CollaborativeTaskRunner` composes `TaskRunner`/`Planner`/`Executor` by import, not fork
|
|
`agents/collab_runner.py` imports and calls into `runtime/task.py`'s `TaskRunner` as the underlying step-execution engine, reusing its existing planning/execution/timeline-append logic rather than reimplementing it. This is the same composition pattern `workflow-orchestration-runtime`'s `WorkflowRunner` already uses for its planned-goal steps.
|
|
**Alternative considered**: fork `TaskRunner`'s loop logic into `agents/` to have full control over each iteration. Rejected — forking creates two divergent copies of step-execution logic that must be kept in sync; composing by import guarantees `CollaborativeTaskRunner` and plain `TaskRunner` runs stay behaviorally identical wherever the new roles don't intervene.
|
|
|
|
### D4: Verifier/Reflector reuse `semantic/llm_client.py`, not a new client abstraction
|
|
Both roles need an LLM call (Verifier to judge whether a step's intended effect occurred; Reflector to propose a recovery action or replan request). Both reuse the existing `semantic/llm_client.py` abstraction added by `semantic-scene-runtime`, rather than introducing a second wrapper around the same underlying SDK.
|
|
**Alternative considered**: give each role its own bespoke client. Rejected — would duplicate timeout/retry/structured-output handling already solved once in `semantic/llm_client.py`, with no benefit specific to Verifier/Reflector's use case.
|
|
|
|
### D5: Reflector proposes a bounded recovery action or a replan request, never a blind re-issue
|
|
`Reflector.reflect(...)` is invoked only when the Verifier flags a step as not-achieved. It analyzes the `Observation`/`PlannedStep`/`StepResult`/verdict and returns a `ReflectionOutcome` carrying either a `ReflectionAction` (a bounded, different corrective action) or a replan request back to the Planner — never simply re-issuing the identical failed step, since that is already the Executor's own retry's job and re-issuing an already-failed-at-the-semantic-level step is unlikely to succeed differently.
|
|
**Alternative considered**: let Reflector re-issue the same `PlannedStep` up to N times. Rejected — the Executor already owns mechanical (tool-call-level) retry; the Reflector's value is specifically in proposing something *different* when the mechanical retry already reported success but the semantic effect didn't happen.
|
|
|
|
### D6: Collaboration is opt-in per task run, defaulting to disabled
|
|
A configuration flag (`agents/config.py`) enables `CollaborativeTaskRunner` per task run, defaulting to **disabled**, mirroring `semantic-scene-runtime`'s default-off precedent. When disabled, a task runs exactly as `runtime/task.py`'s existing `TaskRunner.run()` today.
|
|
**Alternative considered**: default to enabled for all new tasks. Rejected — this change must not silently change any existing task's cost, latency, or step count; requiring an explicit opt-in keeps the blast radius at zero for callers who don't ask for it.
|
|
|
|
### D7: Reflection-recovery ceiling is a separate, explicit counter from the Executor's `max_retries`
|
|
A configurable max-reflection-recovery-attempts ceiling (per task) is tracked independently in `agents/collab_runner.py`, distinct from and layered on top of `Executor`'s own `max_retries` (which bounds *mechanical* retries of a single tool call). Once the ceiling is reached, the loop stops attempting further reflection-driven recovery for that task and surfaces the failure instead of looping indefinitely.
|
|
**Alternative considered**: reuse/extend `Executor.max_retries` to also cover reflection attempts. Rejected — conflating the two counters would make it impossible to reason independently about "how many times did the tool call itself get retried" versus "how many times did we try a semantically different recovery," which are different failure classes with different appropriate bounds.
|
|
|
|
## Risks / Trade-offs
|
|
|
|
- **[Risk] Verifier/Reflector LLM calls add latency and cost to every collaboratively-run step** → Mitigation: opt-in default-off (D6) means only callers who explicitly enable collaboration pay this cost; reuse of the existing cached/structured-output client (D4) keeps per-call overhead in line with `semantic-scene-runtime`'s established pattern.
|
|
- **[Risk] Verifier/Reflector depend on `SemanticScene`/`WorldState`, both of which may be absent (disabled config, degrade path, or not-yet-applied milestones)** → Mitigation: Observer must work when `SemanticScene`/`WorldState` are `None`, falling back to the raw `Scene`/`PlannedStep`/`StepResult`, matching the existing skip/fallback degrade path those capabilities define.
|
|
- **[Risk] Verifier/Reflector could loop indefinitely on a step that never semantically succeeds** → Mitigation: the explicit reflection-recovery ceiling (D7), independent of and on top of `Executor.max_retries`.
|
|
- **[Risk] Verifier judgment quality (false positive/negative on "did the effect happen") is not directly testable against real model output in unit tests** → Mitigation: the LLM client is injected/mockable (following `semantic/llm_client.py`'s existing pattern), so unit tests exercise the verdict/reflection parsing and loop-control logic against canned responses; only a small, explicitly-marked integration test exercises the real API.
|
|
|
|
## Migration Plan
|
|
|
|
This is a purely additive change with no rollback complexity beyond removing the new package and config:
|
|
1. Add the `agents/` package (`models.py`, `observer.py`, `verifier.py`, `reflector.py`, `collab_runner.py`, `config.py`); no change to `runtime/`, `core/models.py`, or `storage/timeline.py`.
|
|
2. Add an `agents*` entry to `pyproject.toml`'s `[tool.setuptools.packages.find].include`; no new third-party dependency (reuses `semantic/llm_client.py`).
|
|
3. Add collaboration config (enabled/disabled per task run, defaulting to disabled; max-reflection-recovery-attempts ceiling), sourced from the same single config location pattern `semantic-scene-runtime` established.
|
|
4. Rollback is simply not enabling collaboration in config / not constructing a `CollaborativeTaskRunner`; no existing capability needs to be reverted.
|
|
|
|
## Open Questions
|
|
|
|
- Should `Observation`/`VerificationVerdict`/`ReflectionOutcome` be recorded in the timeline (`storage/timeline.py`) alongside `Task`/`StepResult`, or are they purely in-loop, non-persisted artifacts for this milestone? Leaning toward "not persisted in this change," mirroring `world-model-runtime`'s deferred `WorldState` persistence question, to keep the boundary with future milestones clean.
|
|
- Is the reflection-recovery ceiling scoped per task or per step? Leaning toward per-task for this milestone (simpler to reason about and configure); revisit if real usage shows a single problematic step exhausting the budget for an otherwise-healthy task.
|
|
- Should `CollaborativeTaskRunner`'s composition with `workflow-orchestration`'s `WorkflowRunner` (using it as the driver for a planned-goal step) be wired in this change or left to a follow-up change once both capabilities are applied? Leaning toward follow-up, since `workflow-orchestration-runtime`'s step-handler contract (`run(task) -> Task`) already accommodates either runner without modification.
|