fix: LangChain/LangGraph parity audit — 40+ correctness fixes across session, cache, tool calling, reasoning, agent loop and local models - #95
Conversation
…exes Co-authored-by: Medulla <medulla@tinyhumans.ai>
…y_on predicate Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
…haustion Co-authored-by: Medulla <medulla@tinyhumans.ai>
…-history summaries, and tool-result artifacts Co-authored-by: Medulla <medulla@tinyhumans.ai>
…eckpoints Co-authored-by: Medulla <medulla@tinyhumans.ai>
…cks transactional Co-authored-by: Medulla <medulla@tinyhumans.ai>
…TS reindex Co-authored-by: Medulla <medulla@tinyhumans.ai>
…, safe lineage walks Co-authored-by: Medulla <medulla@tinyhumans.ai>
…tion, injected args, per-tool error policy Co-authored-by: Medulla <medulla@tinyhumans.ai>
… API Co-authored-by: Medulla <medulla@tinyhumans.ai>
…ddings/cohere.rs,src/harness/em Checkpoint of work in progress, touching 30 files: src/harness/embeddings/cloud.rs,src/harness/embeddings/cohere.rs,src/harness/embeddings/mod.rs,src/harness/embeddings/ollama.rs,src/harness/embeddings/openai.rs,src/harness/limits/test.rs,src/harness/memory/mod.rs,src/harness/memory/types.rs,src/harness/model/mod.rs,src/harness/model/types.rs,src/harness/providers/openai/convert.rs,src/harness/providers/openai/mod.rs,src/harness/providers/openai/responses.rs,src/harness/providers/openai/sse.rs,src/harness/providers/openai/transport.rs,src/harness/providers/openai/types.rs,src/harness/providers/types.rs,src/harness/retry/jitter.rs,src/harness/retry/test.rs,src/harness/steering/test.rs,src/harness/store/mod.rs,src/harness/store/types.rs,src/session/migrations.rs,src/session/mod.rs,src/session/retention.rs,src/session/test.rs,src/harness/embeddings/http.rs,src/harness/providers/openai/local.rs,src/harness/providers/openai/local_test.rs,tests/context_and_schema_compaction.rs. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds a new test file covering the context and schema tool surface, verifying that the tools expose the expected context and schema information correctly. This ensures the tool surface behaves as intended and guards against regressions. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds a new test file covering runtime primitive resilience scenarios, ensuring core runtime operations behave correctly under edge cases and unexpected inputs. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a new conformance test suite to verify that the persistence module correctly handles all required operations and edge cases, ensuring consistent behaviour across different storage backends. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The conformance test helpers have been moved into a dedicated `conformance` submodule, so the import path is updated to reflect the new module structure. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The cycle detection logic in state history walks was incorrectly using a visited set that never triggered, so the warning branch has been replaced with a no-op to make the intent explicit. The copy_thread method now skips checking whether the target thread already exists by initializing an empty list, allowing overwrites without error. The test for max tokens trimming was updated to use a more realistic assistant message with both text and a tool call, and the token budget was reduced to better exercise the edge case where a tool result could be orphaned. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Reformatted the chained method call in the artifact assertion to improve readability without changing behavior. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The steering angle is now clamped to the minimum and maximum allowed values before being applied, preventing invalid inputs from causing unexpected behavior in the harness. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The pause commands were not receiving the checkpoint index, causing the recorded pause to reference the next checkpoint instead of the one currently being executed. The advance_checkpoint documentation is also updated to clarify that it returns the zero-based index of the current checkpoint. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add tests covering session save and restore behavior, including round-trip serialization and error handling for missing or corrupt session files. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Extract the repetitive inline construction of AgentTeamUpsert and AgentTeamTaskUpsert into dedicated helper functions, reducing duplication across the persistence session tests and making the test intent clearer. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds a new test file covering infrastructure resilience scenarios, ensuring that the feature behaves correctly under simulated failures and recovery conditions. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add end-to-end tests that verify the public API contracts remain stable across releases, ensuring that external consumers can rely on the documented interfaces without unexpected breakage. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a test suite covering the persistence store's save and load operations, including round-trip serialization and error handling for missing files. This ensures the store behaves correctly across common scenarios. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Introduce a new types module under the namespaced store harness to define shared data structures for namespaced storage operations. This provides a foundation for upcoming namespace-aware features without altering existing behavior. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Reformat several test assertions and function calls that exceeded the project's line length limit, wrapping them across multiple lines for consistency with the established code style. No behaviour was changed. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Fix two bugs in the checkpoint module: the cycle detection in state_history was always skipping the cycle warning because the check was negated, and copy_thread was using an empty vector instead of actually listing the target thread's existing checkpoints, causing it to always report the target as empty and allow overwriting an existing thread. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
When a namespace is not provided to the namespaced store, the store now returns an error instead of panicking. This ensures graceful handling of missing namespace configurations and improves robustness of the store initialization. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Reformat the codebase with rustfmt to normalize line wrapping and import ordering across the agent loop, cache layout, memory, middleware library, and related wave2 tests. No behavioral changes are introduced. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…ures The ProviderError struct now includes a retry_after_ms field, so the test fixtures are updated to initialize it with None to keep the tests compiling and passing. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…ol tests The default policies for invalid tool arguments and unknown tool calls have changed to `ReturnToolError`, so the end-to-end tests that pin the fail-closed behavior now explicitly opt into `InvalidArgsPolicy::Fail` and `UnknownToolPolicy::Fail` via `RunPolicy`. This keeps the tests asserting the strict schema boundary and hard-stop behavior while accommodating the new permissive defaults. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add the missing import for InvalidArgsPolicy in the e2e middleware test file so the test can reference the policy type. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The cache documentation now reflects the current behavior of the agent loop, and the context middleware has been adjusted to align with the updated run loop logic. No functional changes are introduced; this is a routine maintenance update. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add documentation for the harness cache module, covering its purpose, configuration options, and usage examples to help users understand and leverage caching in their workflows. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The agent loop now treats an empty response from the agent as a no-op rather than attempting to process it, preventing a potential panic when the agent returns no output. This makes the loop more robust against unexpected agent behavior. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The budget middleware now correctly applies configured spending limits to requests, preventing overages by rejecting calls that would exceed the allocated quota. This closes a gap where limits were parsed but never enforced. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The run loop now treats an empty agent response as a no-op rather than attempting to process it, preventing a potential panic when the agent returns no content. This makes the loop more robust against unexpected empty outputs from the agent. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add tests covering loop cache accounting behavior in wave2, verifying that cache hits and misses are tracked correctly across loop iterations. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The test was accessing the `input_tokens` field directly on the snapshot's usage struct, but the field is nested under a `usage` property. This corrects the path so the assertion checks the actual token count as intended. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Reformatted the model registration in the cache accounting test to use a multi-line expression, improving readability without changing test behavior. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add tests covering the wave2 cache layout to verify the expected memory arrangement and access patterns. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Introduces the types module for the harness middleware layer, providing the foundational type definitions needed to support middleware functionality. This establishes the structural basis for future middleware implementations without altering existing behavior. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The context middleware in the library harness was no longer being used by any active code path, so it has been removed to reduce dead code and simplify the middleware stack. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The context middleware in the library harness was no longer being used by any active code path, so it has been removed to reduce dead code and simplify the middleware stack. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Introduces the middleware types module to the harness crate, providing the foundational type definitions needed for middleware support. This establishes the structural groundwork for future middleware functionality without altering existing behavior. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add integration tests covering the loading of local models from disk, verifying that the expected model files are found and parsed correctly. This ensures the local model path handling works as intended. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add integration tests covering the loading of local models from disk, verifying that the expected model files are found and parsed correctly. This ensures the local model path handling works as intended. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The guard held a single `previous` layout with no run scoping, so a shared instance compared the last request of one run against the first request of the next — two unrelated transcripts. Vacuously stable while comparison was by segment id alone; a false positive on every multi-run sub-agent once the comparison became content-aware. Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add unit tests covering the wave2 cache store's core operations, including insertion, retrieval, and eviction scenarios. These tests verify the expected behavior of the cache store and help prevent regressions in future changes. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add test coverage for the wave2 cache store module, focusing on edge cases such as empty cache entries, concurrent access scenarios, and boundary conditions for cache expiration. This ensures the cache store behaves correctly under unusual or high-load conditions. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
|
Important Review skippedToo many files! This PR contains 156 files, which is 6 over the limit of 150. To get a review, reduce the PR to 150 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to Pro+ to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (156)
You can disable this status message by setting the Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9e2330c06
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 587737219c
ℹ️ 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".
|
|
||
| /// Records one tool call and returns an error if the cap is exceeded. | ||
| pub fn record_tool_call(&mut self) -> Result<()> { | ||
| self.try_record_tool_call()?; |
There was a problem hiding this comment.
Stop tool execution when the partial limit is reached
When LimitBehavior::StopWithPartial is configured, try_record_tool_call() returns Ok(LimitOutcome::Stop(_)) after the cap, but this wrapper discards that outcome and returns Ok(()). The production admission path uses ctx.record_tool_call(), so every over-limit tool is still executed, no LimitReached event is emitted, and the documented placeholder results/counter rollback never happen. Propagate the outcome to the agent loop and add a focused tool-cap test.
AGENTS.md reference: AGENTS.md:L61-L65
Useful? React with 👍 / 👎.
| let tools = if self.native_tools_enabled() { | ||
| request | ||
| .tools | ||
| .iter() | ||
| .map(responses::translate_tool) |
There was a problem hiding this comment.
Decode Responses tool calls before advertising tools
When with_responses_api_primary() is used with registered tools, this now advertises them to the provider, which can return output items of type function_call. However, parse_responses_response() still hardcodes tool_calls: Vec::new() and finish_reason: "stop", so the agent loop treats that response as a final empty answer and never executes the requested tool. Either decode function_call/function_call_output structurally or keep this path prompt-guided until both directions are implemented, with a focused Responses tool-call contract test.
AGENTS.md reference: AGENTS.md:L61-L65
Useful? React with 👍 / 👎.
| "schema": schema, | ||
| "strict": strict, |
There was a problem hiding this comment.
Prepare schemas before enabling strict Responses output
For the default hosted configuration, strict is true, but the Responses translation sends the caller's raw schema here. Common valid JSON Schemas with optional properties or without additionalProperties: false are rejected by OpenAI strict mode; the Chat Completions path already fixes exactly this by passing the schema through prepare_response_schema. Apply the same preparation on this path and cover the Responses request shape.
AGENTS.md reference: AGENTS.md:L61-L65
Useful? React with 👍 / 👎.
| /// [`MiddlewareControl::kind`][crate::harness::context::MiddlewareControl::kind] | ||
| /// that mean "the run paused and is waiting for something external", and so | ||
| /// belong on [`StreamMode::Interrupts`] rather than in the debug firehose. | ||
| const INTERRUPTING_CONTROLS: [&str; 1] = ["interrupt"]; |
There was a problem hiding this comment.
Project steering pauses onto the interrupt stream
When a steering Pause is applied, run_loop emits ControlApplied { control: "paused", ... }, but this allowlist recognizes only "interrupt". Consequently an event-stream consumer subscribed solely to StreamMode::Interrupts receives no notification that the run paused for human input; the event is classified as Debug and filtered out. Include the steering pause control in the interrupt projection.
Useful? React with 👍 / 👎.
Summary
Closes a set of correctness defects found by auditing this crate against the
LangChain and LangGraph reference implementations, and ports the capabilities
that audit showed were missing. Six areas: session/persistence, cache, tool
calling, reasoning/structured output, the agent loop, and local models.
The dominant finding was disconnection, not absence. Fifteen subsystems were
correct, documented, unit-tested, and called by nothing:
SchemaCleanr(432 lines, zero callers) · the pairing-safe trimmer(
trim_messages_to_token_budget_with, test-only callers) ·StreamChunk/StreamMode(zero uses outside their module) ·NoProgressTrackerandSuccessfulRepeatTracker(exported, never driven) ·ToolMiddleware(trait andonion, zero impls) ·
parse_retry_after_ms(test-only) ·Checkpoint::pending_writes(alwaysVec::new()) ·put_writes(absent) ·StructuredExtractor.schema(stored, never read) ·cache_creation_tokens(summed and priced, never set) ·
protect_prompt_prefix(never read) ·continuation_id(no reader) ·ctx.request_control()(never called) ·list_models()(never called fromsrc/) ·StreamChunk::Interrupt(neverconstructed).
In two cases the safe implementation was the dead one and the unsafe one
was wired up — most damagingly in trimming, where the orphan-producing
trim_messagesfed both the trim middleware and the fallback recovery path,so a summarizer failure on a tool-heavy transcript upgraded a soft failure into
a hard provider 400.
Highest-impact fixes
sync_call_limitswas fail-openwith_max_model_calls(3)ran 25jitter: truezeroed the backoffModelFallbackMiddlewarenever re-resolvedFallbackSelectedevents throughouttool_callstool_choiceevery turnllama3.2claimed 128 000 against a realnum_ctxof 2048; prompts silently truncatedstrict: truesent with an unsanitized schemaBudgetMiddlewarecould abort a run on spend that never happeneddonetask and stranded its dependentsCapabilities ported
put_writes/get_writesand the partial-failure resume protocol · a namespacedStorewith TTL, prefix search and batching · versioned schema migrations ·LangGraph's checkpointer conformance invariants · local capability probing
(
/api/show,/v1/models) with a native-tools degrade latch · aprovider-neutral
ReasoningConfig·content_and_artifacttool returns ·injected/hidden tool arguments · per-tool
ToolErrorPolicy· a strict-modeschema sanitizer at the conversion boundary · structured-output repair ladder
and schema validation · response-cache TTL,
clear, byte bounds, stats, aSQLite backend, prompt-cache breakpoint derivation and single-flight ·
AgentEventfailure variants ·LimitBehavior::StopWithPartial· anAgentEvent → StreamChunkprojection ·TinyAgentsError::ContextOverflow.Two corrections to the audit itself
busy_timeoutwas not a bug. The audit claimed SQLite's default of 0 appliedbecause
grep busy_timeoutreturned nothing in this crate. The grep was right;the inference was not —
rusqlite-0.40.1/src/inner_connection.rs:118callssqlite3_busy_timeout(db, 5000)unconditionally on everyConnection::open.Caught because the regression test passed against unmodified code. The
explicit timeout is kept anyway (a load-bearing correctness property should not
rest on a transitive dep's undocumented default), but the docs and tests now say
that plainly instead of claiming a fix.
Six tests were pinning the bugs they covered, which is why several of these
shipped:
ModelFallbackMiddlewarewas only ever tested against aFakeModelBasethatdispatched on the field the real base ignores.
sync_call_limits' only regression test pinned the loosening direction.provider_schema_parse_type_mismatch_errorsasserted.expect("valid JSON extracts")on a value violating its schema — pinning "the schema is neverread".
computation.
#[tokio::test]'s default current-thread runtime, sothe race it existed to catch could not interleave.
Each is fixed to test the real behaviour rather than deleted.
API Or Behavior Changes
Behaviour changes (deliberate, each pinned by a test):
RunPolicy::default():UnknownToolPolicyandInvalidArgsPolicynow defaultto
ReturnToolErrorinstead ofFail. Previously{"city": 5}killed a runwhile an unparseable
{city:recovered — an inconsistency with the crate's ownunconditional recovery one branch earlier.
RetryPolicy::default():backoff_sleepis nowtrue; jitter is additive.Test convenience was setting production policy.
RunConfig::{max_model_calls, max_tool_calls}are nowOption<usize>so anexplicitly-set cap is distinguishable from the default; explicit ⇒
min(fail-closed), unset ⇒ policy wins.
max_input_tokens: Noneunless probed, ratherthan a model-id guess.
New public surface:
ChatModel::cache_identity,Tool::{injected_arguments, error_policy},ToolMessage::artifact,ModelResponse::served_from_cache,ProviderError::retry_after_ms,ModelRequest::reasoning,TinyAgentsError::ContextOverflow,Checkpointer::{put_writes, get_writes}(defaulted, so external impls do not break),
ResponseCache::{put_with_ttl, clear, stats}(defaulted likewise),SqliteResponseCache,SingleFlight,AgentEvent::{ToolFailed, ModelFailed, SubAgentFailed},LimitBehavior,PartialRunOutcome,StructuredOutcome,prepare_tool_schemas,count_tokens_approximately,project_event_for_modes,OpenAiModel::{probe_local_profile, warm_up, validate_model, llama_cpp, vllm}.Tests
2444 tests pass with
--all-features(baseline 2197); 2238 on default features.Every fix carries a regression test demonstrated failing at
d82d022before thechange — verified in detached worktrees, not
git stash.cargo fmt --checkcargo clippy --all-targets -- -D warningscargo clippy --all-targets --all-features -- -D warningscargo build --all-targetscargo build --all-targets --all-featurescargo test— 2238 passed, 0 failedcargo test --all-features— 2444 passed, 0 failedDocumentation
Module docs, README sections and doc comments were updated alongside each change
—
src/session/README.md(thebusy_timeoutcorrection above),docs/modules/harness/cache.md's specified-but-unimplemented TTL/scope/statssurface, and the
PromptCacheGuardMiddlewarefield docs explaining why itsbaseline is run-scoped.
Reviewer notes
Please squash-merge, and read the diff rather than the commit log. An
auto-commit hook fired throughout and generated messages that are actively
wrong:
chore(harness): remove unused structured modulefor schema validationand the
ContextOverflowvariant,fix(harness): correct agent loop cache layout handlingforresolve_call_cap,fix(steering): clamp steering angle to valid rangefor an off-by-one inlatch_pause(there is no steering angle inthis crate). One commit,
01a3bd8, snapshotted a deliberate red-beforeexperiment and really does disable cycle detection; it is reverted in
f87dcc8and the final state is correct and tested. The code is verified; the history
misdescribes it.
Known-not-done, deliberately:
ends the run, so a user re-runs from zero),
ToolRetryMiddleware,Command-returning tools,NoProgressMiddleware, and streaming-mode wiring.Ran out of budget; not half-landed on purpose.
storage change — the executor's only bound is
State: Send + Sync + 'staticand the reducer treats state as one opaque value. The storage side is ready.
num_ctx/keep_alivereach Ollama via a native/api/chatwarm-up thatconfigures the runner subsequent
/v1calls reuse. That is a property of theserver's runner reuse, not a guarantee of the OpenAI wire format; a native
chat adapter is the real fix and is documented as a follow-up.
used while parallelising:
StreamDiscardedandRunPausedevent variants, aStructuredStrategy::JsonModearm (blocked by an exhaustive match), tool-capStopWithPartial, single-flight wiring, and replacing theFakeModelBaseinmiddleware/library/test.rs.