fix(pool,convert): atomic RPM admission, <tool_calls> plural dialect, failover knob doc - #356
Merged
Merged
Conversation
… failover knob doc - pool: MAX_REQUESTS_PER_MINUTE counted atomically at lease-grant time (tryAdmitRequest under the owning lock) instead of check-in-Acquire / record-in-Chat — the old split let concurrent bursts (agent spawn batches) pass the cap before any record landed. Same for bridge (bridgeTryAdmitRequest). Removes the now-dead recordRequestEntry paths. - convert: handle the <tool_calls> (plural) XML dialect some models emit: new xmlShapeToolCalls shape, closer, opener, dangling-tag scrub, and the 5th block-regex group — plural blocks now extract into native tool_calls deltas instead of leaking literal <tool_calls> text to the harness. - config: RATE_LIMIT_FAILOVER description now states turn_spend_limited is TERMINAL and excluded from failover (default true). - tests: pooled + bridge burst-atomicity (2 concurrent acquires, cap=1 -> exactly 1 admits), admission counted at grant, plural non-stream / split-opener / dangling-Flush. - config-meta.json fixture regenerated.
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.
Problems
Acquirebut recorded later inChat. A concurrent burst (agent spawn batches of 6-8) all passed the cap before any record landed — e.g. a 5/min cap admitted 8. Observed: user set 5/min and bursts went through.<tool_calls>plural dialect: some models emit the plural wrapper around the same<function=...>payload. The extractor only knew singular/pipe/fence forms, so plural blocks leaked as literal<tool_calls>text to the harness and never became tool calls.RATE_LIMIT_FAILOVERdescription did not sayturn_spend_limitedis TERMINAL and excluded from failover.Changes
pool:tryAdmitRequest(roster lock) +bridgeTryAdmitRequest(bridgeMu) record the admission atomically at lease-grant time; grant fails over to the next token when the window filled concurrently.Pool.Chatno longer records admissions (success-side RPD/usage records stay). DeadrecordRequestEntrypaths removed.convert: newxmlShapeToolCallsstream shape (opener/closer/split-opener/dangling scrub) + 5th block-regex group for the non-stream path; payload parsing (function heads/params/JSON) is shape-agnostic.keycatalog:RATE_LIMIT_FAILOVERdescription states the terminal exclusion.FP_REGEN_FIXTURE=1).Verification
TestAcquireRPMAdmitBurstIsAtomic/TestBridgeRPMAdmitBurstIsAtomic: 2 concurrent acquires, cap=1 -> exactly 1 admits (deterministic; pre-fix both admitted).TestAcquireCountsAdmissionAtGrant: count is 1 right after Acquire with no chat.TestExtractXMLToolCallsPluralDialect/TestXMLStreamExtractorPluralDialect/TestXMLStreamExtractorPluralDanglingFlush.go vetclean.