Reliability fixes: history rollback, async race, 429 retry#44
Open
JNK234 wants to merge 2 commits into
Open
Conversation
Two related history-correctness fixes surfaced by the demos and workshop. #33: llm:chat-with-thinking appended the user prompt to history before the provider call and never rolled it back on failure, so a timeout or network error permanently corrupted that agent's conversation. It now snapshots history for the request and commits the user/assistant pair only on success, matching the other reporters. #32: async replies were written back to the shared per-agent ArrayBuffer from ExecutionContext.global threads with no synchronization. Overlapping llm:chat-async calls on the same agent could interleave pairs, lose writes, or corrupt the buffer mid-resize, and async writes raced the sync history primitives. All history reads/writes now go through a single lock, with each user/assistant exchange committed atomically. Cross-pair order follows completion order (documented): an overlapping request's context never included the other pair regardless, so completion order is the honest, simplest choice. Adds __TEST_FAIL and __TEST_DELAY triggers to the deterministic test provider plus failure-path and overlapping-async regression tests. Fixes #33 Fixes #32
Any provider rate-limit response (HTTP 429) previously threw an immediate hard error that halted the NetLogo run — the exact failure high-frequency demos (e.g. llm:choose every tick) hit against provider TPM limits. Rate-limited requests now retry up to 3 times with exponential backoff (1s, 2s, 4s). The server's Retry-After header is honored when it asks for a longer wait than the backoff, capped at 10s so a large value can't exceed the request timeout budget. Only 429 / rate_limit errors retry; all other errors still fail immediately, now with a message naming the attempts made. The retry loop lives in a shared BaseHttpProvider.executeWithRetry helper that both the base send path and Gemini's URL-specific override use. Backoff delays run on a single daemon-scheduled Future so they never block NetLogo's thread or starve the global execution context. The backend is now lazy and overridable so tests can substitute a stub. Adds RetrySpec covering retry-then-succeed, exhausted-retries, immediate failure on non-rate-limit errors, and Retry-After handling. Fixes #37
JNK234
force-pushed
the
fix/reliability-bugs
branch
from
July 20, 2026 17:56
23d1e34 to
2d083b6
Compare
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.
Fixes 3 reliability bugs from the July report. Closes #33, #32, #37.
What now works:
llm:chat-with-thinking— a failed call leaves history untouched (was: orphaned prompt corrupted the agent's conversation).llm:chat-async— overlapping calls on one agent can't interleave, drop, or corrupt history (was: unlocked writes from background threads).Retry-After, capped 10s) instead of halting the run. Shared by all providers incl. Gemini.Scope:
LLMExtension.scala(per-agent history lock),BaseHttpProvider.scala+GeminiProvider.scala(retry with backoff), plus tests.Base: rebased onto
main(NetLogo7.0.0-2486d1e). Intentionally NOT on the v1.0.0 line, whose7.0.4retarget breaks test compilation under Scala 3.7 — see #45 (separate build/infra issue, does not affect these fixes).Verified: CI green (
sbt testdeterministic); history fixes also confirmed via headless NetLogo smoke test.Fixes #33
Fixes #32
Fixes #37