fix(core): turn-loop audit — solve-rate, token-efficiency and accounting fixes - #1185
Draft
macanderson wants to merge 1 commit into
Draft
fix(core): turn-loop audit — solve-rate, token-efficiency and accounting fixes#1185macanderson wants to merge 1 commit into
macanderson wants to merge 1 commit into
Conversation
…ing fixes
Findings from a 26-agent adversarially-verified audit of the turn loop
(driver, step, loop detection, retry, budget, estimator, compaction, goal),
each fix carrying its witness test:
Solve rate:
- goal: parse_verdict now scans candidate `{` offsets last-first, so a
judge whose prose quotes braces (`if x { }`, cited JSON) can no longer
have its met:true silently converted to not-met — which at temperature 0
re-converted every round and scored solved goals Unmet at the round cap.
- retry: standard() raises max_retries 3 -> 6 so the backoff ladder
actually spans its configured 8s ceiling (was: worst-case 1.75s total,
the 8s cap dead configuration; any ~2s provider brownout aborted a turn
mid-task). Over-ceiling Retry-After failures now keep the provider's
own message.
- driver/step: the model idle-deadline is no longer blind to tool-call-only
streams — ToolCallObserver grows a tool_input_delta liveness tick, the
anthropic/openai/zai adapters emit it per argument fragment, and the
SpeculationGate ticks the idle clock from every observer method
(including fenced mutating announcements), so a generation that is one
large write_file no longer dies as "stalled" after paying for itself.
- driver: the overflow summarizer now runs under the same generation
deadline as every worker call instead of an unbounded await.
Token efficiency:
- compaction: passes 3/4 reclaim to a low watermark an eighth below the
budget (trigger unchanged), so a saturated turn absorbs several steps of
growth per rewrite instead of destroying the provider prompt-cache
prefix on every step; dedup's earliest-copy key is now positional
(message, result), so byte-identical outputs within one Tool message —
a whole step's parallel batch — dedup at all.
- estimator/driver/store: calibration's actual side now includes
cache_write_tokens (real prompt tokens split out only for pricing) at
both feed points — the live record and drift_samples replay — closing
the falsely-low first-call ratio that inflated the effective compaction
budget past the provider window; the drift-sample estimate excludes
attachment weight (a deliberate ~80x media over-estimate that is right
for context pressure, poison for calibration).
Turn correctness / accounting:
- settlement: drained sub-agent spend now lands in the turn's
total_cost_usd as well as the guard, so TurnOutcome/Complete match the
StepUsage-summary contract and a budget-abort reason no longer
contradicts the cost beside it.
- step/driver: CancelUsageGuard gains an attempt_in_flight latch (the
accounted_call discipline), so a hard cancel landing in a backoff sleep
no longer emits a phantom Cancelled envelope for an attempt that already
reported its own failure.
- driver: the soft-stop exit closes caller-supplied open tool_use pairs
exactly as the cancel exit does, keeping the kept transcript valid.
- loop_detect: cycle arithmetic saturates (the "never panics on any
input" contract now covers pathological thresholds); CallRecord borrows
its ToolCall (Cow) so the per-step window stops deep-cloning every
call's input JSON quadratically across a turn.
- driver: summarize_overflow_span defers its full-transcript token walk
until after its early-return guards.
Stale comments/docs corrected: retry's jitter-ownership module doc, the
goal telemetry event order, stella-engine's canonical drive-loop example
(now applies the step cap the crate documents as the host's obligation)
and its "identical event sequence" claim (narrowed to per-step), the
stranded LENGTH_CONTINUATION_NUDGE comment, BudgetWarnings' construction
site, the Mutex-keeps-it-Send rationale on a documented !Send future, two
"property test" citations that name scripted tests, and estimator's
pointer to a nonexistent stella_context::store counter.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MWKn2zvxV9QEusjgHz4R9V
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
Reviewer's GuideThis PR audits and fixes defects in the turn loop around goal judging, retries, idle deadlines, compaction, calibration, loop detection, sub-agent accounting, and streaming tool-call handling, with focused witness tests for each behavior change. Sequence diagram for model call retry, idle deadline, and cancel accountingsequenceDiagram
participant Engine_run_model_call as Engine::run_model_call
participant RetryPolicy_standard as RetryPolicy::standard
participant bounded_generation as bounded_generation
participant CancelUsageGuard as CancelUsageGuard
participant SpeculationGate_new as SpeculationGate::new
participant Provider_complete_observed_ref as Provider::complete_observed_ref
participant ToolCallObserver as ToolCallObserver
Engine_run_model_call->>RetryPolicy_standard: standard()
RetryPolicy_standard-->>Engine_run_model_call: RetryPolicy
Engine_run_model_call->>bounded_generation: start idle deadline with StreamProgress
Engine_run_model_call->>CancelUsageGuard: construct { armed: true, attempt_in_flight }
loop each retry attempt
Engine_run_model_call->>SpeculationGate_new: new(read_only, safe, hook_gated, tx, events, StreamProgress)
SpeculationGate_new-->>Engine_run_model_call: SpeculationGate
Engine_run_model_call->>CancelUsageGuard: attempt_in_flight.store(true)
Engine_run_model_call->>Provider_complete_observed_ref: complete_observed_ref(req, SpeculationGate)
par streamed text
Provider_complete_observed_ref-->>ToolCallObserver: text_delta
ToolCallObserver-->>bounded_generation: progress.record()
and streamed reasoning
Provider_complete_observed_ref-->>ToolCallObserver: reasoning_delta
ToolCallObserver-->>bounded_generation: progress.record()
and streamed tool arguments
Provider_complete_observed_ref-->>ToolCallObserver: tool_input_delta
ToolCallObserver-->>bounded_generation: progress.record()
and streamed tool call
Provider_complete_observed_ref-->>ToolCallObserver: tool_call_streamed
ToolCallObserver-->>bounded_generation: progress.record()
end
Provider_complete_observed_ref-->>Engine_run_model_call: Result
Engine_run_model_call->>CancelUsageGuard: attempt_in_flight.store(false)
end
alt caller hard cancels while attempt_in_flight == true
Engine_run_model_call--xCancelUsageGuard: future dropped
CancelUsageGuard-->>Engine_run_model_call: emit AgentEvent::UsageIncomplete Cancelled
else caller drops during backoff sleep (attempt_in_flight == false)
Engine_run_model_call--xCancelUsageGuard: future dropped
CancelUsageGuard-->>Engine_run_model_call: no event
end
Flow diagram for compact_measured hysteresis and dedup behaviorflowchart TD
A["compact_measured(messages, budget_tokens)"] --> B{before_tokens <= budget_tokens?}
B -->|yes| C["return (before_tokens, None)"]
B -->|no| D[compute target_tokens = budget_tokens - budget_tokens / 8]
D --> E["Pass 1: dedup<br/>record earliest ToolResult positions keyed by (message_idx, result_idx)<br/>stub later duplicates"]
E --> F[Pass 2: supersede reruns]
F --> G["Pass 3: aging<br/>current_tokens = estimate_conversation_tokens"]
G --> H{current_tokens > target_tokens?}
H -->|yes| I["shrink old large tool outputs to head+tail<br/>update current_tokens<br/>stop when <= target_tokens"]
H -->|no| J[skip aging]
I --> K["Pass 4: eviction<br/>recompute current_tokens"]
J --> K
K --> L{current_tokens > target_tokens?}
L -->|yes| M["evict oldest large tool outputs<br/>update current_tokens<br/>stop when <= target_tokens"]
L -->|no| N[no eviction]
M --> O["return (current_tokens, Some(CompactionPass))"]
N --> O
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
What & why
A full-depth audit of the turn loop (driver, step, loop detection, retry, budget, estimator, compaction, goal loop) ran 6 specialized reviewers plus adversarial verification of every finding; the 15 confirmed defects — plus a handful of independently re-verified low-severity ones — are fixed here, each behavioral fix with its witness test.
Solve rate
goal::parse_verdictscans candidate{offsets last-first, so a judge whose prose quotes braces can no longer have itsmet: truesilently converted to not-met (at temperature 0 this recurred every round and scored solved goalsUnmetat the round cap).RetryPolicy::standard()raisesmax_retries3 → 6 so the backoff ladder actually spans its configured 8s ceiling. Previously the worst-case total backoff was 1.75s (the 8s cap was dead configuration), so any ~2s provider brownout aborted a turn mid-task. Over-ceilingRetry-Afterfailures now keep the provider's own message.ToolCallObservergrows atool_input_deltaliveness tick, the anthropic/openai/zai adapters emit it per argument fragment, andSpeculationGateticks the idle clock from every observer method — a generation that is one largewrite_fileno longer dies as "stalled" after paying for itself.Token efficiency
cache_write_tokensat both feed points (live record +drift_samplesreplay), closing the falsely-low first-call ratio that inflated the effective compaction budget past the provider window; the drift-sample estimate excludes attachment weight (~80× media over-estimate — right for context pressure, poison for calibration).Turn correctness / accounting
total_cost_usdas well as the guard, soTurnOutcome/Completematch their documented StepUsage-summary contract.CancelUsageGuardgains anattempt_in_flightlatch (theaccounted_calldiscipline): a hard cancel landing in a backoff sleep no longer emits a phantomCancelledenvelope.tool_usepairs exactly as the cancel exit does.loop_detectcycle arithmetic saturates;CallRecordborrows itsToolCall(Cow), ending the quadratic per-step deep clone of every call's input JSON.summarize_overflow_spandefers its Θ(transcript) token walk until after its early-return guards.Stale comments/docs corrected: retry's jitter-ownership claim, the goal telemetry event order, stella-engine's canonical drive-loop example (now applies the step cap the crate documents as the host's obligation) and its "identical event sequence" claim, a stranded test-only comment,
BudgetWarnings' construction site, a Mutex-keeps-it-Send rationale on a documented!Sendfuture, two "property test" citations naming scripted tests, and a pointer to a nonexistentstella_context::storecounter.Note for maintainers:
make gateis red on main —check-license-allowlist-paritycannot findallow-licenses:in.github/workflows/dependency-review.yml. Pre-existing, unrelated to this PR, left untouched because widening license config is a licensing decision.The witness
main, pass here):parse_verdict_survives_braces_in_the_judges_prose,standard_policy_backoff_envelope_actually_reaches_its_ceiling,a_compacted_conversation_absorbs_a_step_of_growth_without_recompacting,duplicates_within_one_tool_message_are_deduped,cache_write_tokens_count_toward_the_calibration_actual,attachment_weight_is_excluded_from_the_drift_sample_estimate,drift_samples_count_cache_writes_as_real_prompt_tokens,a_call_only_stream_outlives_the_deadline_because_it_is_not_stalled,hard_cancel_during_a_backoff_sleep_emits_no_phantom_cancelled_envelope,a_soft_stop_closes_caller_supplied_open_tool_calls,tool_dispatched_child_spend_aborts_the_parent_at_the_next_step_boundary(extended),pathological_thresholds_do_not_overflow_the_cycle_arithmeticThe gate
cargo fmt --checkcargo clippy --workspace --all-targets -- -D warningscargo test --workspace(5416 passed, 0 failed)StepUsage.estimated_input_tokensanddrift_samplessemantics documented)Ground-rule check
stella-core; no new depsAnything reviewers should know?
scripts/file-size-baseline.txtwas updated via the sanctioned--updatepath for the files that grew (tests + doc comments);stella-model/src/zai.rsnewly crossed 1500 lines by 7.max_stepsstaying the host's loop bound forrun_step(stella-serve still drives whole-loop Engine::run_turn, not stella-engine's step-scoped run_step facade #1129 design — only the doc example was fixed).🤖 Generated with Claude Code
https://claude.ai/code/session_01MWKn2zvxV9QEusjgHz4R9V
Generated by Claude Code
Summary by Sourcery
Audit and harden the turn loop around calibration, compaction, retries, streaming liveness, loop detection, sub-agent accounting, and goal judging, with witness tests for each defect.
New Features:
Bug Fixes:
Completecost summaries accurately reflect all child usage.Cancelledusage envelopes.Enhancements:
run_turnandrun_step, including host responsibility for enforcing step caps and emitting terminal errors.Tests: