feat(harness): host capability traits + crate-owned session config - #87
Conversation
Phase 0 of relocating OpenHuman's agent runtime into this crate: the ten host-supplied capability traits that invert its 45 outbound domain dependencies, in the same generic-over-State style as the crate's existing 18 extension traits. Signatures only — Phase 1 lands them with default impls. Carries four open questions that block Phase 1, including a MemoryProvider name collision with tinyflows 0.5.1. Co-authored-by: Medulla <medulla@tinyhumans.ai>
Phase 3 of the agent-runtime relocation. A runtime generic over its host cannot read the host's config schema, so the crate declares what it needs and each host maps into it. SessionConfig / TurnConfig / ToolConfig / MemoryLimits / RequiredOutput / ToolDispatcher. Inert: serde + std only, so a host can build a mapper without pulling the rest of the crate. Chose structs over a ConfigProvider trait with ~40 getters: the struct makes 'what does the runtime actually depend on' answerable by existing, and gives the host schema exactly one place to meet the runtime. Defaults mirror OpenHuman's current values and are pinned by test — they are a compatibility surface for hosts using ..Default::default(). SessionConfig deliberately has no Default: guessing a path root would put agent output somewhere the user did not ask for. Co-authored-by: Medulla <medulla@tinyhumans.ai>
Mirrors the host contract's key semantics exactly so OpenHuman's enforcement logic can move onto the crate type unchanged. Includes the subtle case: a blank block_key makes the contract inert even when required_keys lists siblings, because the block key is the contract's defining key and enforcement can never name a block without it. Co-authored-by: Medulla <medulla@tinyhumans.ai>
Phase 1 of the agent-runtime relocation: the seams a host implements so the runtime can run a turn without knowing what a memory is, whether an action is permitted, or which model a role should use. src/harness/host/, one module per capability, plus a HostCapabilities<State> bundle (RFC 5.3) — four required capabilities as constructor args, six optional via with_*. Optionality is absence, never an erroring default: a stub that errors teaches the loop the capability exists and invites a retry. Clone on the bundle is hand-written; deriving it would demand State: Clone, which is wrong when every field is an Arc. Drafted in parallel, which showed up in the seams and was corrected here: - Identity was encoded four ways across ten modules. Normalised to agent_id / thread_id, with Option where a value can be absent. CallEstimate.agent was re-encoding absent as "" — a sentinel in the same change set as a module documenting at length why sentinels are wrong. - Two modules independently derived Default on a struct holding ThreadId, which has none. Dropped the derive rather than adding Default to the id newtype: a turn summary attributed to a blank thread is worse than none. - InMemoryExperienceStore mapped a poisoned mutex to Validation (the input-validation variant) and made len/is_empty fallible. Aligned with its two siblings: recover via into_inner, infallible accessors. - A security_gate test used "outside action_dir" as a deny reason. This crate is redistributed; genericised. Documented two overlaps that compile fine but would drift: BudgetGate's compression_hint vs SummarizationPolicy (union, not override — a host declining to ask for compression must not veto the policy and blow the context window), and ProgressEvent vs AgentEvent (AgentEvent is authoritative; ProgressEvent is derived from it by one conversion, not emitted in parallel). RFC amended to match what shipped: AgentId does not exist in this crate, so 3.3/3.9/3.10 now take &str; 3.10's ModelRequest collided with the existing harness::model::ModelRequest and is ModelResolveRequest. Known gap: capabilities are flat files, so value types share a module with async_trait and RFC 6's dependency-free requirement is not yet met. Deferred, recorded in 5.2 — it blocks publishing, not Phase 4. 1316 crate tests pass (122 new). No host change. Co-authored-by: Medulla <medulla@tinyhumans.ai>
Unit-struct `::default()` calls and an over-complex tuple return in the bundle test, both flagged by `clippy --all-targets -- -D warnings`. Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 58 seconds Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (17)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b299c4a2f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// host's job. It is also called for **failed** calls that still consumed | ||
| /// tokens, so an implementation must be additive and must not assume a | ||
| /// successful reply followed. | ||
| async fn record(&self, usage: &Usage) -> Result<()>; |
There was a problem hiding this comment.
Preserve call attribution when recording budget usage
When budgets are scoped per agent or thread, or multiple model calls overlap, record receives only Usage, which contains token counters but no model, agent/thread attribution, estimate, or permit id. The host therefore cannot reconcile the realized usage with the reservation created by acquire, despite CallEstimate explicitly supporting those scopes; pass a call identifier or the relevant estimate/permit metadata into this method.
Useful? React with 👍 / 👎.
| run: RunId, | ||
| /// The conversation thread, when the turn belongs to one. Absent for | ||
| /// one-shot runs that are not part of a thread. | ||
| thread: Option<ThreadId>, |
There was a problem hiding this comment.
Use the normalized identity names in progress events
For hosts serializing Started, this produces thread and agent keys even though the RFC added by this commit fixes the convention to thread_id and agent_id everywhere (docs/spec/host-capability-traits-rfc.md:293-303). This forces progress adapters to special-case one contract and freezes inconsistent wire names; rename these fields or apply matching serde names before publishing the API.
Useful? React with 👍 / 👎.
| pub mod cost; | ||
| pub mod embeddings; | ||
| pub mod events; | ||
| pub mod host; |
There was a problem hiding this comment.
Re-export the host capability API from the crate root
Making host public here exposes HostCapabilities and all ten traits only through nested tinyagents::harness::host paths, while src/lib.rs is unchanged. Add the intended crate-root exports so this new public surface follows the repository's centralized, predictable export convention.
AGENTS.md reference: AGENTS.md:L52-L57
Useful? React with 👍 / 👎.
| #[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] | ||
| pub struct MemoryId(String); |
There was a problem hiding this comment.
Split host types and tests into the prescribed module files
The new capability files co-locate public value types, implementations, and inline unit tests rather than placing type definitions in types.rs and module-local tests in test.rs; the RFC even records this split as deferred. Restructure the host feature before this public API is published so it follows the required module layout.
AGENTS.md reference: AGENTS.md:L13-L18
Useful? React with 👍 / 👎.
| pub mod agent_memory; | ||
| pub mod budget_gate; | ||
| pub mod context_composer; | ||
| pub mod definition_registry; | ||
| pub mod experience_store; |
There was a problem hiding this comment.
Add the required module-level README for the host area
This introduces a roughly 5,000-line host feature spanning ten capability modules, but src/harness/host/ contains no README.md. Add one documenting the design, public surface, and operational constraints as required for a complex module.
AGENTS.md reference: AGENTS.md:L79-L83
Useful? React with 👍 / 👎.
Summary
Adds the ten host capability traits a generic agent runtime needs, plus the
crate-owned session config it reads instead of a host's config schema. This is
Phase 0/1 of relocating OpenHuman's agent runtime (~70k LOC) into this crate:
the runtime is generic over its host, so everything it would otherwise have to
guess at — memory, permission, model routing, budget, progress — is asked
through a trait here.
Nothing is wired into the agent loop yet. This lands the seams so the move can
proceed incrementally, and a host can start implementing against them.
docs/spec/host-capability-traits-rfc.md— the accepted RFC, with thereference counts that justify each seam and what is deliberately not a trait.
harness::config—SessionConfig/TurnConfig/ToolConfig/MemoryLimits/RequiredOutput/ToolDispatcher. Inert (serde+std),so a host can build a mapper without pulling the rest of the crate. Chosen
over a
ConfigProvidertrait with ~40 getters: a struct makes "what does theruntime actually depend on" answerable by existing.
harness::host— one module per capability, plus aHostCapabilities<State>bundle. Four required (
ContextComposer,DefinitionRegistry,SecurityGate,ModelResolver), six optional viawith_*.Optionality is absence, never an erroring default. A stub that errors
teaches the loop the capability exists and invites a retry;
Nonesimply is notoffered. The
Noop*/InMemory*impls are for tests and embedding, not a wayto fake an unconfigured host.
API Or Behavior Changes
Additive only — new modules (
harness::config,harness::host), no existingsignature touched, no behavior change to the agent loop.
Two naming decisions worth flagging for review:
AgentMemory, notMemoryProvider.tinyflows0.5.1 shipped adifferent, flow-scoped
MemoryProvider, and both crates sit in one host'sdependency graph — sharing the name would force an import alias at every call
site.
ModelResolveRequest, notModelRequest. The RFC originally saidModelRequest, which collides with the existingharness::model::ModelRequest(a provider call payload; this one is a routingquestion). Documented at the definition so nobody "fixes" it later.
Known gap, recorded in the RFC (§5.2): capabilities are currently flat files, so
value types share a module with
async_traitand the "inert value-type module,dependency-free" criterion is not yet met. Deferred deliberately — it is ~30
files of pure motion and buys nothing until a host depends on the value types
without the crate. Worth doing before publishing.
Tests
165 new tests. Run locally on this branch:
cargo fmt --checkcargo clippy --all-targets -- -D warningscargo clippy --all-targets --all-features -- -D warningscargo build --all-targetscargo build --all-targets --all-featurescargo test— 1373 lib + all integration/doc suites greencargo test --all-featuresOne environment note for whoever runs this in CI: the doctests need real temp
space. On a box with a small
tmpfs/tmpthey fail en masse withDisk quota exceeded (os error 122), which reads like 41 test failures ratherthan a full disk.
TMPDIR=<dir on a real filesystem> cargo testis clean.Documentation
docs/spec/host-capability-traits-rfc.mdis the design record and is part ofthis PR. Every trait carries module-level docs explaining why the seam exists
and what the host must not do — in particular that
SecurityGatenever lets theruntime decide, and that
ProgressSink's coarse enum must not absorb a host'sricher UI progress type.