Fix issues that made long runs fail or hang - #182
Conversation
A long authoring turn (Opus thinking over a large prompt) can exceed the SDK's 600s non-streaming ceiling, and `timeout=None` explicitly DISABLES the SDK's timeouts (an explicit None is not not-given), so a socket that died silently mid-call hung the session forever — both observed on Crucible solana_vault runs. Stream every request so bytes keep flowing (no ceiling, no idle window for NAT killers to hit), and bound each httpx phase at 300s so a dead socket surfaces in minutes; for a streamed response that bounds the silence between chunks, not the whole turn. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The nomic model's remote code caches positional tensors per sequence length, so concurrent encodes race: every concurrent crucible_docs_search died with tensor-shape mismatches (the tool degrades to "no results", so authoring ran ungrounded and hallucinated the crucible API), and on Apple Silicon the auto-picked MPS backend segfaulted the whole process inside torch's Metal shader cache. One process-wide lock serializes encodes -- queries are short, so contention is noise -- and COMPOSER_EMBED_DEVICE=cpu lets a Mac host opt out of MPS entirely. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
State read back through the Postgres checkpointer can carry raw dicts where the schema declares models (the serializer's fallback when it cannot reconstruct the class). The readback then died on "'dict' object has no attribute 'property_title'" -- after every component session had already finished its paid authoring -- and the campaign reported empty with exit 0. Revalidate at the boundary so the readback is typed either way. Symptom fix: why the serializer falls back at all is still open, and other checkpoint readers may want the same guard. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
I ran into this myself today. It looks like it's an issue with the new "tool family" thing. I don't think this is the right fix though; I will open a separate PR for this. |
| # prompt) can exceed the SDK's 600s non-streaming ceiling, and a silent | ||
| # 10-minute wait is long enough for NAT/idle killers to drop the socket | ||
| # (surfaces as APIConnectionError mid-run). Streaming keeps bytes flowing. | ||
| streaming=True, |
There was a problem hiding this comment.
I happen to know this causes the langgraph api to produce streaming results in its own API. Please double check this doesn't utterly break the TUI and console display handlers, i.e. we aren't streaming chunks that our handlers have no idea what to do with, this somehow opts us out of the complete results, etc.
| return cast(ndarray, await asyncio.to_thread( | ||
| self.tr.encode_query, f"search_query: {query}", show_progress_bar=False | ||
| )) | ||
| return await asyncio.to_thread( | ||
| self._encode_query_locked, f"search_query: {query}" | ||
| ) | ||
|
|
||
| async def embed_docs( | ||
| self, doc: list[BlockChunk] | ||
| ) -> list[ndarray]: | ||
| return cast(list[ndarray], await asyncio.to_thread( | ||
| self.tr.encode_document, [f"search_document: {d.chunk}" for d in doc], show_progress_bar=False | ||
| )) | ||
| return await asyncio.to_thread( | ||
| self._encode_docs_locked, [f"search_document: {d.chunk}" for d in doc] | ||
| ) |
There was a problem hiding this comment.
this should also be applied to the DefaultEmbedder found in the indexed research agents, although there the API is sync so we can't just offload to to_thread....
| skipped=_revive(SkippedProperty, state["skipped"]), | ||
| property_checks=[ | ||
| (m.property_title, m.checks) | ||
| for m in _revive(PropertyCheckMapping, state["property_checks"]) | ||
| ], | ||
| verdicts={k: _revive_one(WireVerdict, v) for k, v in state["verdicts"].items()}, | ||
| ran=_revive(Target, state["ran"]), |
There was a problem hiding this comment.
ugh, this has bitten us before. It's unclear to me why langgraph sometimes deserializes the type, sometimes hands us back the raw dict. Having to add this defensive coding is madness, but idk what to do ...
There was a problem hiding this comment.
Hm, I guess if this is a pre-existing condition then ignore my previous comment. :) I'll look closer at the "tool family" problem.
There was a problem hiding this comment.
Ok, yeah, this was a pre-existing condition that also affected the new shared tools. :) See Certora/graphcore#35
There was a problem hiding this comment.
From the description of Certora/graphcore#35:
A checkpoint serializer (JsonPlusSerializer) names a pydantic value by cls.module and
cls.name, then restores it with getattr(import_module(module), name)(**kwargs). On failure
it returns the kwargs dict.create_model takes module from the calling frame, so both the bound clone and a rendering
of it claimed graphcore.tools.schemas. That name is not there, so a rendered value comes back as
a bare dict and the next read of it fails on an attribute it no longer has:
There was a problem hiding this comment.
so should we keep this change or no? Seems like it can help, no?
There was a problem hiding this comment.
I think we should revert this part of the change; my graphcore change should make this unnecessary. BTW the AutoProver side of that change is here, if anyone wants to approve: #184
Should I undo this then? |
A streamed response carries no raw response_metadata["usage"] dict (the non-streaming path's shape), so with streaming on, UsageCallback recorded zero tokens for every call while costs kept accumulating correctly (CostAccumulator already reads the normalized usage_metadata). Fall back to usage_metadata when the raw dict is absent, translating normalized input (total, cache included) back to the raw shape (cache excluded). Verified against live streamed and non-streamed responses. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review feedback: the indexed research agents embed through DefaultEmbedder's sync Embeddings API, which called the shared sentence-transformer with no serialization -- the same race the async ComposerRAGDB wrappers just got a lock for. The lock now lives in composer.rag.models and both paths take it, so sync and async encodes cannot race each other on the same model instance either. The sync API takes the threading.Lock directly (no to_thread needed -- callers are already off the event loop or tolerate the short block; encodes are sub-second). Verified with interleaved sync-thread + async encode hammer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
3 participants
I was trying to run AutoProver (crucible on the solana_vault demo) via Fable and it had to do these fixes to get it working. I wanted to make a PR with the fixes, in case these might help others too.
From Claude:
LLM calls died at 10 min, or hung forever. Non-streaming calls hit the Anthropic SDK's 10-minute limit, and our
timeout=Nonedisables timeouts completely, so a dropped connection hung the run indefinitely. Fix: stream responses (no limit) and time out after 5 min of silence. This will affects every Anthropic call in the repo. Is that ok?Doc search: broke under parallel agents. The embedding model isn't thread-safe: concurrent searches returned no results on CPU and segfaulted on Mac GPUs. Fix was to run one encode at a time; add COMPOSER_EMBED_DEVICE=cpu to skip the GPU.
Report: empty despite successful work. State read back from Postgres sometimes arrives as plain dicts instead of typed objects, crashing the final step after all the paid LLM work was done. Fix was to re-validate on read. (Why the round-trip does this, and why an all-failed run still exits 0, are left open.)