Restructure the harness: layers, one loop, a session tree, hooks, compaction - #12
Merged
Merged
Conversation
There were four near-copies of the ReAct loop (sync/async x result/stream) and two independent agent classes. They had already drifted: - The streaming loop caught provider errors and returned without building a RunResult, so token usage from every completed turn was lost. A metered deployment silently under-billed itself. - AsyncAgent was missing from_dataframe, from_csv, enable_subagents, add_mcp_server, enable_cache, close, explain, and the on_code/code_only approval gate. _build_tools_for used getattr defaults to paper over it. Now: AsyncHarness._turns is the only loop, parameterised by whether the turn response comes from stream_events or chat. Harness is a facade that bridges a sync adapter onto it and drives it to completion, including from inside a running event loop. _AgentBase holds all configuration and tool wiring; Agent and AsyncAgent differ only in adapter kind and coroutine-ness. Also: - AsyncHarness.last_result exposes the RunResult for streamed runs. - Harness/AsyncHarness gain public inspection properties (system, tools, max_turns, cache, messages, reminders); tests no longer poke privates. - resolve_async_adapter mirrors resolve_adapter through shared routing. - make_subagent_spec accepts either adapter kind. 578 passing (was 547), 31 new regression tests.
An independent review found the sync facade was not behaviour-preserving. Driving the loop through asyncio for sync callers broke three things: - Tool handlers moved onto an asyncio.to_thread worker, so a connector holding a sqlite3 connection built at setup time started returning 'SQLite objects created in a thread can only be used in that same thread' to the model instead of data. - Ctrl-C stopped landing promptly: neither to_thread nor the executor shutdown is cancellable, so a 3s tool call absorbed the interrupt for its full duration (measured 0.41s before, 3.01s after). - asyncio.run cleared the calling thread's event loop, so a program that installed one and then used the *synchronous* API found it gone. Restructured so the loop is a driver-agnostic generator instead. _plan yields CallProvider / CallTool / ToolFinished effects and performs no I/O; failures are sent back in as Failed(exc) and the loop decides what they mean. Harness performs those effects inline on the calling thread; AsyncHarness awaits them and offloads blocking handlers to to_thread. Both inherit _HarnessBase, so loop state and the _call_tool seam exist on each and a subclass override is actually called (a facade silently ignored it). Also from the review: - run_coroutine_blocking restores the ambient event loop. - _SyncAdapterBridge documents that it blocks its loop; as_async_adapter says to prefer a real async adapter where concurrency matters. - The subagent tool gives a sync adapter the sync driver, so it keeps the parent's threading semantics. - Replay dispatches inline for Agent, offloaded for AsyncAgent. - A failed anthropic import no longer becomes "requires the None extra"; the real ImportError propagates. - Restored self-returning annotations on from_dataframe/from_csv/enable_*. Tests: 611 passing (was 547 at baseline). New coverage pins each defect above, plus the gaps the review named: resolve_async_adapter routing, session parity, streaming max_turns_exceeded, abandoned streams, tool events for failures and missing tools, nested driving, and the test_streaming case that should have caught the usage loss and did not.
Second independent review confirmed the sans-io restructure is correct (byte-identical RunResults, messages, dispatch order, stream event order and JSONL records vs main across seven exit paths) and found seven smaller problems: - Successes were sent back into the loop raw, so a handler that legitimately returned a Failed was reported to the model as having raised. Answers are now Ok(value) | Failed(exc); no return value can impersonate a failure. - _ambient_event_loop used get_event_loop(), which on 3.10/3.11 CREATES a loop when none is set. The helper that exists to protect the ambient loop was leaking one never-run, never-closed loop plus its selector fd per call. - Abandoning a stream never closed the provider's generator: a bare 'async for' does not close its iterator, and each generator can only clean up its own frame. Collapsed the driver back to a single generator and wrapped the outer 'async for' in aclosing, so one unwind releases the provider's HTTP stream instead of waiting for the GC. - AsyncAgentSession.ask_stream recorded an unstamped RunResult, so anything correlating turns by session_id lost every streamed turn. - AsyncAgent.run_stream ignored the replay cache in both directions: it neither served a hit nor recorded a success, so whether a caller paid for a repeated question depended on the entry point. A hit now yields the cached answer as synthetic text events. - _AgentBase grew last_result, since streamed and replayed runs return nothing to the caller. Replaces a test that asserted its deliberate absence under an older plan; the rewrite says why that constraint was lifted. - Dropped _SyncAdapterBridge/as_async_adapter, dead since the subagent stopped needing them. Corrected the module docstring: _plan does no *network* I/O, but it does write the JSONL log, which Phase 3 moves behind the store. Tests: 624 passing (was 547 at baseline). New tests/test_loop_protocol.py pins the seven invariants the reviewer's mutation testing showed the suite did not notice, and drives _plan by hand to fix the effect/answer sequence.
Third review found the previous round's stream-close fix stopped one layer short of the public API, and that one of its tests was vacuous. - AsyncAgent.run_stream and AsyncAgentSession.ask_stream still iterated the harness generator with a bare 'async for'. Closing a generator does not close the one it iterates, so every agent-level caller still leaked the provider's connection on an early break. The harness-level test could not see it. Both now use aclosing, and the new tests exercise the agent and session entry points rather than the harness. - The ambient-event-loop test was vacuous: it called set_event_loop(None) first, which flips the policy's _set_called flag, and the buggy implementation raises rather than creating once that is set. Verified by re-injecting the old implementation: the test passed. It now runs the probe in a subprocess, and re-injecting the bug fails it. - _ambient_event_loop's fallback for a non-default policy called get_event_loop(), reintroducing the leak it exists to prevent. It now reports _UNREADABLE and run_coroutine_blocking leaves the thread alone rather than guessing. - CallProvider/CallTool answered with None gave AttributeError several frames from the driver at fault; _require_answer names the mistake instead. - AsyncAgent.run_stream did not stamp a run id, so run_id was present on session streams and replay hits but absent here. - run_stream's docstring pointed at last_harness.last_result, which is None on a replay hit (no harness is built) and otherwise the previous question's result. Points at last_result now, which is correct on both paths. - Dropped two docstrings still describing the deleted sync-adapter bridge. - Corrected this file's own header: answering ToolFinished with a value is harmless, not a desync, so that was never the invariant worth pinning. Tests: 631 passing (was 547 at baseline).
Fourth review confirmed all seven round-3 fixes and found one real defect plus the accounting hole below. - The _UNREADABLE branch skipped restoring the ambient loop, but set_event_loop had already run, so it left the thread pointing at the loop it had just CLOSED: every later get_event_loop user got 'Event loop is closed'. Worse than the leak the branch exists to avoid. It now sets None, the only honest answer when the previous loop could not be read. The branch was also completely untested, which is how this got in; there is now a test with a policy that has no readable _local. - Abandoning a stream after the last event of a completed turn left last_result None, so a caller that read the answer and stopped lost every token the provider had already billed. That is the same defect this phase set out to fix, one case over. _plan now catches GeneratorExit and records what the finished turns cost. The test that asserted the old behaviour is rewritten to say why the premise changed. - _UNREADABLE is an enum member rather than object(), so _ambient_event_loop's return type is a union again instead of Any. - Dead run_id binding on the replay-hit path (replay mints its own). - _stamp documents that it is reached from data_harness.agent and mutates _last_result, since a caller reading it afterwards sees the stamped object. - Renamed a test whose name still described the deleted adapter bridge. Tests: 633 passing (was 547 at baseline).
data_harness was one flat 7.8k-LOC namespace where the core loop constructed a SessionCache and called format_tool_output directly. It is now four layers, bottom up: llm (provider adapters and the wire types they speak), core (the loop, RunResult, run logging), data (session cache, interpreter, SQL, connectors), app (Agent, ask, CLI). Each may import the layers below it and no others. The structural change is core no longer knowing what a DataFrame is: - core.loop took cache=SessionCache and called format_tool_output. It now takes a RunEnvironment, which answers two questions the loop cannot: how to render a tool's return value, and what the run's final state was. data.environment.CacheEnvironment is the session-cache implementation; NullEnvironment is the domain-free default. - data.harness.Harness/AsyncHarness are the cache-flavoured pair Agent builds and data_harness.loop resolves to, so cache= still works. - core.serialize hardcoded DataFrame and ndarray snapshotting for the run log. It now has a snapshotter registry that data populates on import. - RunResult._repr_html_ checked isinstance(value, pd.DataFrame). It ducks on _repr_html_ instead, which is both layer-clean and works for polars. - _unwrap became core.result.unwrap_text: the one place deciding which failure becomes which exception, reachable from every layer that needs it. Every pre-layering import path still resolves, via a meta-path finder rather than re-export shims, so data_harness.loop.Harness IS data.harness.Harness rather than a same-named copy that would fail isinstance across the two. Two subtleties are documented where they bite: the loader must return the already-imported module (not the target's spec, which re-executes the file), and the finder must go at the FRONT of sys.meta_path (or PathFinder resolves aliased submodules like data_harness.providers.base itself and loads a copy). Tests: 769 passing (was 633). tests/test_layers.py enforces the boundary statically over the AST, because a runtime import check would pass vacuously here: nearly every heavy dependency is already behind a function-local import. tests/test_core_standalone.py runs a full tool loop with no data domain at all, plugs in a third-party environment, and pins the legacy paths to module identity rather than mere importability.
Verifying the layering against a built wheel in a clean Python 3.14 venv turned up a forward-compatibility bug in the loop. _ambient_event_loop read asyncio's private policy slot unconditionally, because on 3.10-3.13 get_event_loop() CREATES a loop when none is set and would leave a never-run, never-closed one behind. On 3.14 that reasoning is obsolete: get_event_loop() raises instead, and get_event_loop_policy() is deprecated there and removed in 3.16. Under -W error the probe blew up, so any 3.14 user running warnings-as-errors would have hit it. Now version-gated: the supported API where one exists, the private slot only where it is the only option, and the docstring says which is which and why. Verified this round (the Phase 2 verification agent hit its session limit before reporting, so this was done directly): - Wheel builds and ships all four layer subpackages plus _legacy_paths. - 12 attacks on the legacy-path finder, run against the INSTALLED wheel with no source tree on the path: legacy-first import in a fresh interpreter, both submodule import forms, parent attribute access, identity across all 34 aliases, isinstance across paths, install() idempotence, typos still raising, inspect.getsource, pickling, reload, and an end-to-end run. - Snapshot output byte-identical to pre-registry for both the handle snapshot the model sees and to_jsonable's dataframe/ndarray records. - 8 mutants against the layer guards, all killed: top-level and function-local core->data imports, a lazy pandas import in core, naming SessionCache in core code, the alias loader returning a fresh module, the finder appended rather than inserted, NullEnvironment rendering with repr, and a stripped layer docstring. - Suite green on 3.10 (769) and on 3.14 from the wheel (761, the 8 failures being optional deps absent from that venv). Also fixed the import sorting my rewrite left in examples/.
The harness kept its state in a list of messages and wrote a separate runs/*.jsonl log that re-serialised the entire history every turn. That log was quadratic to write and could not be read back into anything runnable: no resume, no forking, no way to ask what the agent actually saw at turn 7. A session is now an append-only tree of typed entries, each naming its parent, with a movable leaf. The conversation is DERIVED by walking root to leaf and is never stored, so there is no second copy to disagree with the log. Everything else follows from that one inversion: - Resume: reopen a JsonlSessionStore in a different process, pass it to a harness, and ask() continues the conversation. Tested end to end. - Forking: move the leaf and append; both branches survive and either can be replayed. This is how a turn gets retried with a different model without destroying the first attempt. - Compaction is an entry that changes where the walk starts, not an edit. The compacted entries stay in the tree, so moving the leaf back before it restores the full history. Reversible and auditable. - Writing is one line per entry. 40 messages is 40 lines, where the old log wrote 800 message-copies. Layering held: the tree lives in core and knows nothing about the data domain. CustomEntry plus a projector is the extension point, so the data layer can record a cache write or a chart without core learning what either is. Projected entries are invisible to the model unless a projector opts them in, because a cache write is recorded for a human to trace, not for the model to re-read. Note the session store gets its own message encoder rather than reusing to_jsonable. That one is the *log* shape: it renames fields for readability and snapshots large values away. Lossy is right for a debugging log and wrong for a store meant to reconstruct a runnable conversation, and a round-trip test pins the difference on tool blocks specifically. Both stores (memory, jsonl) run against the same test suite via a fixture, so neither drifts from the protocol. Corrupt files, unknown entry types, future format versions, duplicate ids, unknown parents, and cycles all raise a typed SessionStoreError with a stable code rather than a KeyError somewhere downstream. Not yet wired: the data layer does not record cache_put/chart entries. The mechanism and its tests are in place; the plumbing is a separate change. Tests: 822 passing (was 769).
An independent review found the tree structure sound but the derivation layer and loop wiring broken in three serious ways and seven smaller ones. Serious: - A crash or Ctrl-C during a tool call persists an assistant tool_use with no matching tool_result. Resuming that session sent the provider a transcript it rejects with a 400, so resume failed precisely for the sessions most worth resuming. build_context now drops unpaired tool calls and unpaired tool results, so a derived context is always something a provider will accept. - _begin_run reset the working copy but not the session leaf, so after a second run() messages and build_context disagreed: the model was sent two messages and the log claimed four, linked as one conversation. A fresh run now starts a fresh root. The docstring claiming they 'agree at every turn boundary' was false and unpinned; it is now true and pinned for runs, repeated runs, asks, and streams. - context_entries silently discarded all pre-compaction history when first_kept_entry_id was off-path or pointed forward, and stacked compactions emitted two summaries in reverse chronological order while resurrecting entries the older one had dropped. append_compaction now rejects a cut that is not on the path, and an older compaction inside the kept tail is skipped rather than replayed. Smaller: - Reminders mutate an already-recorded message, so the JSONL store (which serialises on write) showed a prompt the model never saw. Both stores now snapshot on write, and the reminder is recorded as its own entry rather than retroactively editing an immutable one. - A torn final line, the normal result of a crash mid-append, made the whole file unreadable. The tail is now dropped and flagged via the truncated flag; corruption anywhere else is still fatal. - Added open_or_create, since create() truncates and the obvious restart mistake destroyed the session the feature exists to preserve. - A non-serialisable custom payload raised a bare TypeError from one store and was accepted by the other; it is a typed SessionStoreError now. - Corrected two false claims: the quadratic runs/*.jsonl log is still written alongside, and entries.py referenced a module that does not exist. Tests: 840 passing (was 822). Three mutants had survived the previous round and now die: an encoder writing every role as 'user', is_error always False (the old assertion checked the dataclass default), and removed cycle detection. test_both_stores_satisfy_the_protocol was near-vacuous, since isinstance against a runtime_checkable Protocol matches method names only; it now exercises the behaviour.
Mutation-testing the previous round's fixes found two survivors, both of which turned out to be my tests being wrong rather than the code. - The JSONL store serialised a snapshot to the file but kept the caller's object in memory, so a later mutation made its live view disagree with its own file: reading the session back showed something different from reading it now. The previous round only fixed the memory store. Both snapshot now. - test_both_stores_agree_after_a_message_is_mutated compared messages with a helper that reads content[0], so appending a second block was invisible to it. It compares every block now, and that is what exposed the store bug above. - test_stacked_compactions_leave_one_summary_in_order put the older compaction outside the newer one's kept tail, where skipping it changes nothing. The kept tail now starts before the first compaction, which is the case the skip exists for. All ten mutations against this phase's fixes are killed: unpaired tool blocks, stacked compaction, unvalidated compaction cut, both stores' copying, torn-tail tolerance, open_or_create, run() branching, reminder recording, message roles, and is_error. 840 passing.
The loop had one extension point, register_reminder, which could append text to the prompt suffix and nothing else. Everything more interesting was hardcoded: the interpreter approval gate lived inside the tool dispatcher and was keyed on the literal string 'python_interpreter', so the one general thing the loop could do about a tool call was reachable by exactly one feature. Four events, each with a decision a hook may return: BeforeTurn -> Reminder(text) | Stop(reason) BeforeToolCall -> Block(reason, is_error) AfterToolCall -> Replace(content, is_error) AfterTurn -> Stop(reason) The proof that the mechanism is sufficient rather than decorative is that the approval gate is now built from it: make_code_gate returns an ordinary BeforeToolCall hook, registered like any other when on_code or code_only is set, and a test asserts it is there rather than special-cased in the loop. The loop no longer mentions a tool by name. Design points worth keeping: - Every hook sees an event even after another has decided. A hook recording spend must not be skipped because an unrelated one asked to stop; precedence between conflicting decisions belongs to the caller. - Block defaults to is_error=False. A refusal is a decision, not a malfunction, and telling the model its code was broken makes it rewrite and retry rather than stop. - AfterTurn is where a spend cap belongs, because the tokens are already counted and the decision is made on real numbers. BeforeTurn stopping spends nothing at all. - Hooks must not raise. One that does is reported as HookError naming the hook and the event, rather than losing the run mid-turn. - register_reminder still works, as a BeforeTurn hook. 861 passing (was 841). Layer check still clean.
An independent review found the mechanism well shaped but three of the commit's own stated invariants false in the code that shipped. - A HookRegistry passed to the constructor was mutated in place: the harness added its approval gate to the CALLER's object, so a gate configured on one harness governed every other harness sharing the registry. That is exactly the reuse the hooks= parameter is documented to enable. Registries are copied on ingest now. - HookError escaped _plan entirely: no RunResult, last_result left None, and the tokens already billed discarded. Three docstrings and the commit message all claimed it became a failed run. Now it does, carrying the usage spent up to the failure. - Stop(reason) was written to a private field and never read, so a capped run was byte-identical to a model that answered with nothing: same status, same empty text, no error. RunResult.stopped_by records it. Also: - AfterToolCall fired only for successful calls, so a redaction hook could not see the case most likely to need it: an exception repr can carry a connection string. Every result now goes through one settle() path, including tool-not-found, handler failures, and blocked calls. - BeforeToolCall did not fire for an unknown tool name, so a policy hook could not observe attempts on tools it does not know about. - Agent had no way to register hooks at all, and since it builds a fresh Harness per run, one registered on a harness would be gone by the next call. Agent.on() and Agent.hooks now exist and are forwarded. - HookRegistry.first was annotated with the event TypeVar rather than a decision one, so its return was silently Any. - _on_code/_code_only on the harness are dead as switches (the gate closes over them at construction); documented rather than left as a trap. Tests: 873 passing (was 861). Four mutations had survived the previous round and now die: an AfterTurn stop discarding the model's answer, a BeforeTurn stop reporting error, a BeforeTurn stop dropping accumulated usage, and AfterTurn always reporting zero tokens. The token test was vacuous, asserting (0, 0) against an adapter that reports (0, 0); it now uses two turns and a scoring adapter so cumulative semantics are actually pinned.
Two things the harness had no answer for. CONTEXT. The only context management was max_turns, which is a wall rather than a strategy: a run needing thirty turns failed at twenty-five having paid for all of them. Compaction summarises older turns and replays the summary in their place. Two properties make it safe: - The cut lands on a turn boundary. An assistant tool call is never separated from the result answering it, and the conversation never opens with an assistant message. Both produce transcripts providers reject outright, and the split pair is the first bug everyone writes here. When there is nowhere safe to cut, nothing is cut. - Nothing is deleted. The summary is a session entry, so the compacted turns stay in the tree and moving the leaf back restores them. Compaction is a view, not an edit, and a test asserts the undo. The summariser is injected, because summarising means calling a model and which model at what cost is the application's decision. Token estimation is deliberately crude: the decision it feeds is 'are we near the limit', and being wrong by 20% moves the trigger slightly rather than breaking anything, while a real tokeniser would tie core to a provider's vocabulary. Worth recording: compaction costs this library less than it costs a coding agent. Data lives in the cache under handles, not in the transcript, so compacting away the turn that loaded a DataFrame loses the discussion of it, not the frame. Writing the test proved it: a 4000-character tool result never reached the transcript at all, because the cache had already turned it into a handle and a snapshot. ERRORS. Failures were bare RuntimeError/KeyError, or repr(exc) stuffed into a string, so a caller could not tell a rate-limited provider from a typo in the model's pandas from a sandbox timeout. Those are three different decisions: retry, show the user, bill the attempt. Every error now derives from DataHarnessError and carries a stable code; codes are API, messages are not. Added ProviderError, ExecutionError, and ConfigurationError; folded the existing SessionStoreError and HookError into the same base. MaxTurnsExceeded stays a RuntimeError and ToolNotFoundError a KeyError, since callers catch them that way. 903 passing (was 873).
…ction Verifying Phase 5 directly (the verification agent hit its session limit before reporting) found the error-taxonomy claim false and one real defect. ProviderError, ExecutionError and ConfigurationError were declared and never raised anywhere. The commit claiming a caller could now tell a rate-limited provider from a typo in the model's pandas from a sandbox timeout was simply untrue: nothing raised any of them. They are wired up now. - unwrap_text raises ProviderError for a failed run, not a bare RuntimeError. - The sandbox raises ExecutionError when code never got to run (wall-clock timeout, killed process) and PythonInterpreterError when the model's code ran and failed. That is the distinction the docstrings claimed; it did not exist. PythonInterpreterError joins the taxonomy too. - max_turns and CompactionSettings validation raise ConfigurationError. - ProviderError and ExecutionError also derive from RuntimeError, and ConfigurationError from ValueError, so callers written before the taxonomy keep working. A compactor that raised killed the run: exception escaped, no RunResult, and usage discarded. Summarising means calling a model, so a rate limit there is ordinary, and compaction is an optimisation: carrying on with the full context is strictly better than losing a run that was otherwise fine. If the context really was too big, the next provider call fails and reports it where it belongs. The attempt is recorded as a compaction_failed entry either way. The layer test caught me trying to import the taxonomy into llm/types.py. llm is the bottom layer and may not import core, so that check stays a plain ValueError, with a comment saying why. Verified directly this round: - Seven adversarial cut-point cases produce no unpaired tool blocks, no assistant-first conversation, and no empty messages: parallel tool calls, a user message mixing text and tool results, an assistant message mixing text and a call, a conversation opening with an assistant message, consecutive user messages, and repeated compaction. - Thirty turns compacting every turn stays bounded (254 tokens against a 300 trigger) rather than growing. - context_entries() is a subset of branch(), so a cut id found in one is never rejected by the other's validation. - Ten mutations, all killed. Three initially survived and each was a bad test rather than good code: the empty-replacement guard was never reached because the fixture was under the compaction trigger, ProviderError was only asserted as a RuntimeError, and the sandbox timeout test exercises the CPU-kill branch rather than the wall-clock one, which needed driving directly since model code cannot sleep. 913 passing (was 903).
- docs/guide/design.md gained sections for the four layers, the one-loop two-driver split, the session tree, hooks, compaction, and the error taxonomy. The page described an architecture that no longer existed. - CHANGELOG has an Unreleased entry covering all five phases. - The two ambient-event-loop tests asserted a 3.10-3.13 precondition (policy._local._set_called) that does not exist on 3.14, where the implementation takes the get_event_loop() branch instead. The pristine check is now version-gated and the custom-policy test skipped above 3.14, with the reason stated. Verified: 913 passing on 3.10; on 3.14 from the built wheel only six tests fail, all for optional dependencies absent from that venv (matplotlib, sqlalchemy, IPython). Zero layer violations. data-harness-ui's usage pattern still produces identical token accounting.
maxkskhor
added a commit
that referenced
this pull request
Aug 4, 2026
Merges the remaining cleanup on top of PR #12: the JSONL turn logger, _legacy_paths.py, register_reminder(), and AsyncProviderAdapter.stream() are removed, not deprecated. See CHANGELOG.md's 1.0.0 section.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Restructures the harness along the lines of the pi agent harness, taking its ideas rather than its code: pi is TypeScript, and the value here is the Python data domain.
Every previous import path still works, resolving to the same module object rather than a copy.
913 tests, up from 547.Zero layer violations.What changed
Layers. The package was one flat 7.8k-LOC namespace where the core loop constructed a
SessionCacheand calledformat_tool_outputdirectly. It is nowllm→core→data→app, each importing only the layers below it, enforced statically over the AST bytests/test_layers.py.The structural change is
coreno longer knowing what a DataFrame is. The loop takes aRunEnvironmentsupplying the two things it cannot decide for itself — how to render a tool's return value, and what the run's final state was.data_harness.corenow runs a full tool loop with no pandas anywhere, whichtests/test_core_standalone.pyexercises.One loop. Four near-copies (sync/async × result/stream) collapsed into a single sans-io generator that yields effects and is performed by two drivers. They had already drifted: the streaming copy discarded token usage on provider errors, and
AsyncAgentwas missing eight featuresAgenthad (from_dataframe, subagents, MCP, the replay cache, the approval gate…).The sync driver runs inline on the calling thread, so
KeyboardInterruptlands promptly, a handler holding asqlite3connection still works, and the caller's ambient event loop is untouched. The async driver awaits and offloads blocking handlers so they cannot stall a shared loop.Session tree. A session is an append-only tree of typed entries; the conversation is derived by walking root→leaf and never stored, so there is no second copy to disagree with the log. That single inversion buys resume across processes, forking a conversation without losing the original, reversible compaction, and one line per entry instead of re-serialising the whole history every turn.
Hooks.
BeforeTurn,BeforeToolCall,AfterToolCall,AfterTurn, returningReminder,Block,Replace, orStop. The evidence the mechanism is sufficient is that the interpreter approval gate is now built from it, rather than hardcoded inside the dispatcher and keyed on the literal string"python_interpreter".Compaction.
max_turnswas a wall, not a strategy: a run needing thirty turns failed at twenty-five having paid for all of them. Cuts land on turn boundaries so a tool call is never separated from its result, and nothing is deleted — the summary is an entry, so stepping the leaf back undoes it.Typed errors. Every failure carries a stable
codeunder a commonDataHarnessError. A caller can now tell a rate-limited provider from a bug in the model's code from a sandbox timeout — three different decisions. Existing base classes are kept, so code catchingRuntimeError/ValueErroris unaffected.For reviewers
The commit history is deliberately phase-by-phase, each phase followed by an independent adversarial review and a
Phase N reviewcommit fixing what it found. Every phase shipped with real defects I did not catch myself, and the review commits are where the interesting reasoning is. A few worth knowing about:ProviderError/ExecutionError/ConfigurationErrorwere declared and never raised, while the commit message claimed callers could distinguish failure kinds. Now wired to real sites.Mutation testing repeatedly caught tests that passed for the wrong reason — including the ambient-event-loop test passing against the exact bug it existed to catch, and an
is_errorassertion checking the dataclass default rather than the value written.Known gaps, noted in code rather than papered over
runs/*.jsonlturn log is still written alongside the session store, so per-turn write cost is unchanged until it is removed.cache_put/chartsession entries; the mechanism and its tests exist, the plumbing does not.Verification
data-harness-ui's usage pattern produces token accounting identical to the library's own.