Skip to content

Latest commit

 

History

History
106 lines (71 loc) · 10.6 KB

File metadata and controls

106 lines (71 loc) · 10.6 KB

Design Decisions

What was considered and rejected for ACE and the PydanticAI migration — and why.

For architecture and concepts, see ACE_ARCHITECTURE.md. For code reference and examples, see ACE_REFERENCE.md.


PydanticAI Migration

What we replaced and why

ACE had three hand-rolled LLM client implementations (LiteLLMClient, InstructorClient, ClaudeCodeLLMClient) with inconsistent retry/validation behavior, ~3,500 lines of custom agent-loop plumbing in the Recursive Reflector, and manual code extraction via regex. PydanticAI handles all of this as maintained infrastructure.

Before After
3 LLM client implementations PydanticAI agents inside roles
Manual JSON extraction + Pydantic parse PydanticAI validates via tool-call schema, retries with error feedback
3 blind retries or Instructor PydanticAI native (configurable, with error context)
LiteLLM wrapper for provider support PydanticAI (native support for 15+ providers, wraps LiteLLM internally)
Custom SubRunner loop (~400 lines) PydanticAI's agent loop — LLM calls tools until it produces structured output
200 lines of regex code extraction Tool args are pre-parsed (code arrives as execute_code parameter)
Inner pipeline steps (~500 lines) ~50 lines of tool definitions
CallBudget + SubAgentLLM (~200 lines) ctx.usage shared budget + delegate agent
Custom RRIterationContext PydanticAI manages message state internally
Manual Opik span building (~356 lines) logfire.instrument_pydantic_ai() auto-instruments everything

Net result for RR: ~3,500 lines → ~1,000 lines (sandbox + prompts + trimming + agent definition). ~2,500 lines of loop/extraction/budget/context plumbing deleted.

What we kept

  • Pipeline engine (pipeline/) — requires/provides contracts, async_boundary, per-step max_workers, SampleResult error isolation. No framework offers this combination.
  • Skillbook & learning loop — Reflect → Update → Apply → Deduplicate. This is core IP.
  • Step compositionlearning_tail(), pipeline-as-step nesting, SkillbookView read/write split.
  • Domain-specific prompts — tightly coupled to skillbook format and ACE's reflection strategy.
  • All pipeline steps — they depend on protocols, not implementations. Completely unchanged.

Provider resolution design

The resolver routes LiteLLM model strings to PydanticAI through three paths:

  1. PydanticAI-native prefix — pass through unchanged
  2. LiteLLM prefix → native provider — rewrite / to : when the prefix matches a native provider. This is necessary because PydanticAI's litellm provider uses an OpenAI-compatible HTTP client under the hood, which doesn't work for providers with non-OpenAI APIs (Bedrock via SigV4, Anthropic's native API, etc.).
  3. Fallback — prefix with litellm: for the proxy provider

User-facing API is unchanged — same LiteLLM model strings as before.


ACE Architecture Decisions

Runner extends Pipeline: Making TraceAnalyser and ACE subclasses of Pipeline was considered. Rejected — the runner is not a pipeline. It owns the epoch loop. Composition (self.pipeline) keeps responsibilities separate.

Cross-sample state (reflection window): A rolling window of recent reflections that persists across samples was considered, with variants: on the runner, on StepContext, on step instances, via a shared mediator object. All rejected — each sample should be independent. The only cross-sample coupling is the skillbook itself. Adding a reflection window complicates the model (reset between epochs, eventual consistency with background steps, ordering issues with concurrent workers) for marginal benefit.

Separate Online and Offline classes: Keeping two runner classes for single-pass and multi-epoch was considered. Rejected — the only difference is epochs=1 vs epochs > 1, which is a parameter, not a class distinction. ACE handles both. TraceAnalyser is a separate class because its input type is fundamentally different (raw traces vs Sample + Environment).

Structured Trace dataclass: A @dataclass Trace with typed fields (task, output, feedback, reasoning, etc.) was considered. Rejected — it imposes a schema on trace data that doesn't match reality. External frameworks produce wildly different trace shapes (browser-use AgentHistoryList, LangChain result dicts, Claude Code transcripts). Forcing them through a common dataclass means either losing information or adding catch-all metadata buckets that defeat the purpose of typing. Instead, ctx.trace is object | None and the Reflector makes sense of whatever it receives.

Steps that accept both traces and samples: Making ReflectStep and UpdateStep polymorphic over input type was considered. Rejected — steps always receive StepContext with the same named fields. The runner (_build_context) is responsible for building the context correctly.

Observability in the runner: Keeping observability logic in ACERunner._track_observability_data() was considered. Rejected — it mixes concerns. Observability is handled by Logfire auto-instrumentation.

Custom AsyncLearningPipeline: The legacy ace/async_learning.py implements a manual thread pool with reflector and skill manager queues. Rejected — the pipeline engine's async_boundary and max_workers provide the same functionality with less code and consistent semantics.

Per-integration pipeline classes: Having each integration define its own pipeline class was considered. Rejected — every integration pipeline has the same learning tail; only the execute step differs. Instead, integrations provide execute steps that compose into an ACERunner subclass, reusing the shared _run() loop.

Checkpoints in the runner: Having the runner own checkpoint logic (via run() parameters) was considered. Rejected — a CheckpointStep at the end of the pipeline tail keeps checkpointing within the pipeline formalism. Configuration belongs at construction time (factory methods), not at call time (run()).

Mutable Skillbook directly on the context: Storing the real Skillbook as a field on ACEStepContext was the initial design. Rejected — StepContext is frozen, but Skillbook is mutable. Placing it on the context creates the illusion of immutability while allowing any step to mutate shared state through the reference. Instead, the context carries a SkillbookView (read-only projection). Write steps receive the real Skillbook via constructor injection.

Injection is ground truth; citation dropped. Earlier the Agent "cited" skills by writing [skill-id] markers in its reasoning, the Reflector scanned the text to produce skill_tags, and the SkillManager consumed those tags. Rejected — citation scanning does not scale: it is fragile (regex over free-form text), biased (agents forget to cite skills they used), and asks the Reflector to dictate downstream state mutation. Replaced with injection-based attribution: the AgentStep records ctx.injected_skill_ids (the set of active skills rendered into the agent prompt) and bumps used_count on each skill. The SkillManager — not the Reflector — now decides helpful/harmful/neutral per injected skill, using atomic tag_skill / remove_skill tool calls against the real Skillbook. The Reflector produces pure analysis; skill_tags and the SkillTag output type were removed.

SkillManager mutates directly; Reflector is analysis-only. The old SkillManager was a one-shot PydanticAI agent that emitted an UpdateBatch of planned operations, and a separate ApplyStep applied them to the skillbook. Rejected — two-phase (plan → apply) forced the LLM to commit to decisions without inspecting the skillbook, and serialisation of operations meant the agent could not dedupe-before-ADD, inspect counters, or sandbox-verify candidate strategies. Replaced with an agentic SkillManager built on RecursiveAgent with atomic mutation tools (add_skill, update_skill, remove_skill, tag_skill) and read-only inspection tools (search_skills, read_skill). Tools operate on the real Skillbook immediately — there is no staging. ApplyStep was deleted; UpdateStep is the sole SM invocation and the skillbook is already mutated when it returns. SkillManagerOutput now carries a post-hoc audit trail of the operations the tools executed, not a plan to be applied. UpdateStep.max_workers=1 is preserved and now guards the mutation critical section in addition to serialising LLM calls. AgenticConfig.max_requests controls loop budget; max_requests=1 approximates the old one-shot behaviour. UpdateBatch / apply_update / _apply_operation are retained for offline reconstruction and tests but the online path no longer flows through them.

Instructor auto-wrapping in implementations: The old ace/roles.py auto-wrapped LLM clients with Instructor if complete_structured was missing. Rejected — PydanticAI handles structured output natively via its result_type parameter.

Recursive Reflector (initial rejection, now implemented): The old ace/reflector/ subsystem supports recursive mode. Initially rejected for ace due to complexity. Now implemented as RRStep — a PydanticAI agent-based step that runs an iterative REPL loop. Satisfies both StepProtocol[ACEStepContext] and ReflectorLike.

Observability decorator on implementations: The old ace/roles.py uses @maybe_track() decorators for Opik tracing on every role method. Rejected — Logfire auto-instrumentation handles observability. Per-method decorators would double-count and create coupling.

Deduplication inside SkillManager: The old ace/roles.py SkillManager integrates with DeduplicationManager directly. Rejected — deduplication is now a separate DeduplicateStep in the pipeline. Cleaner separation: the SkillManager only produces output, deduplication runs at a configurable interval.

Shared ace/features.py module: A centralized feature detection module was considered. Rejected — the only code that needs it is deduplication/detector.py, which uses a local _has(module) helper. A shared module would add a file for a single 4-line function.

Separate wrapper classes for integration runners: Separate convenience classes (ACEAgent, ACELangChain, ACEClaudeCode) wrapping the runners were the initial design. Rejected — the wrappers only added from_model() and a few lifecycle helpers, which fit naturally on the runner class itself. Two classes for one concept forces users to choose between them. Exception: ACELiteLLM, which wraps two runners and exposes a fundamentally different API.