Repository navigation
Conversation
…/B path The LLM write-path reasoner (LlmReasoner::http_chat_call) built its reqwest client with `Client::new()` (no timeout) and sent requests with no `max_tokens`. A slow, hung, or runaway endpoint could therefore block the write path indefinitely, or a local model could generate for tens of seconds per call, making bulk ingest impractical. Add: - a per-request timeout (SYNAPTIC_LLM_TIMEOUT_SECS, default 90); on timeout the call errors and the reasoner fails open to its heuristic fallback, so ingest degrades rather than hangs. - a max_tokens cap (SYNAPTIC_LLM_MAX_TOKENS, default 512); extraction/resolution replies are small JSON, so this bounds per-call latency without truncating. Eval harness (run_eval): - `--qtype NAME` now drops conversations with no matching questions (previously only questions were filtered, so all conversations were still ingested — on the retrieval path that spawned one task per conversation this flooded the reasoner and wedged its HTTP client). - retrieval path: bounded ingest concurrency (RETRIEVAL_CONCURRENCY, default 2) instead of one spawned task per conversation. - retrieval path honors `--exclude-superseded` (bi-temporal A/B): ingest is unchanged (KG on marks supersessions), only retrieval drops superseded memories, so recall@k of the two arms is directly comparable. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
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
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.
Summary
The LLM write-path reasoner (
LlmReasoner::http_chat_call) built its reqwest client withClient::new()(no timeout) and sent requests with nomax_tokens. Found while running write-side eval experiments:Reasoner fixes (
src/memory/reasoning/llm.rs)SYNAPTIC_LLM_TIMEOUT_SECS(default 90). On timeout the call errors and the reasoner fails open to its heuristic fallback, so ingest degrades rather than hangs.max_tokenscap —SYNAPTIC_LLM_MAX_TOKENS(default 512). Bounds per-call latency; extraction/resolution replies are small JSON.Eval harness fixes (
tools/eval/src/bin/run_eval.rs)--qtype NAMEnow drops conversations with no matching questions (previously only questions were filtered → every conversation still ingested; on the retrieval path, which spawns one task per conversation, this flooded the reasoner and wedged its HTTP client).RETRIEVAL_CONCURRENCY, default 2).--exclude-superseded(bi-temporal A/B): ingest unchanged (KG marks supersessions), only retrieval drops superseded memories → recall@k of the two arms is directly comparable.Testing
cargo fmt --checkclean.Supersedes #122 (needed a fresh branch to include the rustfmt fix, per the repo's PR-only ruleset).
🤖 Generated with Claude Code