Skip to content

fix(core): turn-loop audit — solve-rate, token-efficiency and accounting fixes - #1185

Draft
macanderson wants to merge 1 commit into
mainfrom
claude/stella-turn-loop-audit-p3ebeh
Draft

fix(core): turn-loop audit — solve-rate, token-efficiency and accounting fixes#1185
macanderson wants to merge 1 commit into
mainfrom
claude/stella-turn-loop-audit-p3ebeh

Conversation

@macanderson

@macanderson macanderson commented Aug 2, 2026

Copy link
Copy Markdown
Owner

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_verdict scans candidate { offsets last-first, so a judge whose prose quotes braces can no longer have its met: true silently converted to not-met (at temperature 0 this recurred every round and scored solved goals Unmet at the round cap).
  • RetryPolicy::standard() raises max_retries 3 → 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-ceiling Retry-After failures now keep the provider's own message.
  • 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 SpeculationGate ticks the idle clock from every observer method — a generation that is one large write_file no longer dies as "stalled" after paying for itself.
  • The overflow summarizer runs under the same generation deadline as worker calls 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 (invariant 7).
  • Dedup's earliest-copy key is positional, so byte-identical outputs within one Tool message — a whole step's parallel batch — dedup at all.
  • Calibration's actual side now includes cache_write_tokens at both feed points (live record + 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 (~80× media over-estimate — right for context pressure, poison for calibration).

Turn correctness / accounting

  • Drained sub-agent spend lands in total_cost_usd as well as the guard, so TurnOutcome/Complete match their documented StepUsage-summary contract.
  • CancelUsageGuard gains an attempt_in_flight latch (the accounted_call discipline): a hard cancel landing in a backoff sleep no longer emits a phantom Cancelled envelope.
  • The soft-stop exit closes caller-supplied open tool_use pairs exactly as the cancel exit does.
  • loop_detect cycle arithmetic saturates; CallRecord borrows its ToolCall (Cow), ending the quadratic per-step deep clone of every call's input JSON.
  • summarize_overflow_span defers 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 !Send future, two "property test" citations naming scripted tests, and a pointer to a nonexistent stella_context::store counter.

Note for maintainers: make gate is red on maincheck-license-allowlist-parity cannot find allow-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

  • This PR includes witness tests (fail on 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_arithmetic

The gate

  • cargo fmt --check
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo test --workspace (5416 passed, 0 failed)
  • Docs updated where behavior changed (doc comments on every touched contract; StepUsage.estimated_input_tokens and drift_samples semantics documented)

Ground-rule check

  • No I/O added to stella-core; no new deps
  • No new outbound network calls

Anything reviewers should know?

  • Two compaction tests' hand-tuned budgets were widened (700→800, 1100→1250) because passes 3/4 now reclaim to the watermark; the invariants those tests assert are unchanged.
  • scripts/file-size-baseline.txt was updated via the sanctioned --update path for the files that grew (tests + doc comments); stella-model/src/zai.rs newly crossed 1500 lines by 7.
  • Deliberately NOT changed after verification refuted them as intentional: cancel-while-paused gate semantics (host-closed, test-pinned), judge-incomplete → run-fatal escalation (documented never-fabricate-a-verdict contract), max_steps staying the host's loop bound for run_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:

  • Introduce tool-input liveness signals in the streaming tool-call observer pipeline so call-only generations are tracked against idle deadlines.
  • Expose attachment-only token estimation and integrate it into drift calibration and telemetry as text-vs-media aware samples.

Bug Fixes:

  • Fix goal verdict parsing to tolerate braces in judge prose and inside JSON strings so solved goals are correctly marked as met.
  • Raise the standard retry policy ceiling to actually reach its configured backoff limit and preserve provider rate-limit messages when rejecting overlong Retry-After hints.
  • Ensure cache-write tokens are treated as real prompt tokens in calibration, telemetry, and drift samples to avoid underestimating actual usage.
  • Exclude attachment token weight from drift-sample estimates to prevent media-heavy steps from corrupting calibration and inflating effective compaction budgets.
  • Teach the idle deadline to observe all streamed tool-call activity so tool-only streams are no longer misclassified as stalled.
  • Apply the model timeout to overflow summarization and defer expensive transcript walks until after early-return guards to avoid wedging turns on summarizer failures.
  • Adjust compaction hysteresis to reclaim below the budget and fix tool-output dedup so duplicates within a single Tool message are properly stubbed without breaking cache-friendly prefixes.
  • Include drained sub-agent spend in turn totals so budget abort reasons and Complete cost summaries accurately reflect all child usage.
  • Constrain cancel accounting to genuine in-flight attempts so hard cancels during backoff sleep no longer emit phantom Cancelled usage envelopes.
  • Saturate loop-detection cycle arithmetic and borrow tool calls from the transcript to avoid overflows and quadratic cloning in long turns.

Enhancements:

  • Clarify driver, retry, goal, estimator, telemetry, and engine crate documentation around loop semantics, step caps, jitter ownership, and drift calibration contracts.
  • Refine compaction tests and tuning to align with the new low-watermark behavior while preserving existing invariants.
  • Document the per-step event sequencing and framing differences between run_turn and run_step, including host responsibility for enforcing step caps and emitting terminal errors.

Tests:

  • Add targeted witness tests covering calibration with cache writes, attachment-excluded drift samples, tool-only streaming liveness, retry backoff envelope behavior, soft-stop tool-call closure, cancel-during-backoff semantics, compaction hysteresis and dedup within a single Tool message, loop-detection saturation, and sub-agent spend accounting in turn outcomes.

…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
@vercel

vercel Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
stella-cli-docs Ignored Ignored Aug 2, 2026 9:49am

@sourcery-ai

sourcery-ai Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

This 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 accounting

sequenceDiagram
    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
Loading

Flow diagram for compact_measured hysteresis and dedup behavior

flowchart 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
Loading

File-Level Changes

Change Details Files
Compaction hysteresis and tool-output dedup now preserve provider prompt cache and correctly deduplicate duplicate tool results within a single Tool message.
  • Introduce low-watermark compaction target (budget minus one-eighth) in compact_measured and update aging/eviction to use target instead of raw budget.
  • Change dedup key from message index to (message index, result index) so identical tool outputs within the same Tool message dedup correctly.
  • Adjust existing compaction tests’ budgets and add tests to ensure hysteresis avoids recompacting every step and intra-message duplicates are stubbed.
  • Clarify compaction documentation around never dropping still-referenced tool results.
stella-core/src/compaction.rs
Retry behavior and cancellation accounting are fixed so backoff reaches its configured ceiling, rate-limit errors retain provider messages, and hard cancels during backoff do not emit phantom Cancelled usage events.
  • Increase RetryPolicy::standard max_retries from 3 to 6 and add a test verifying the exponential backoff envelope reaches max_delay_ms.
  • Preserve provider rate-limit error messages when server-suggested retry delay exceeds the configured hint ceiling.
  • Extend CancelUsageGuard with an attempt_in_flight latch and wire it through run_model_call so drop during backoff sleep does not produce a spurious UsageIncomplete::Cancelled.
  • Add hanging-sleeper witness test that drops the turn during backoff and asserts only the ProviderError envelope is emitted.
stella-core/src/retry.rs
stella-core/src/step.rs
stella-core/src/driver.rs
stella-core/src/driver/tests/audit_fixes.rs
Calibration and drift-sample handling now count cache-write tokens as real prompt tokens while excluding attachment weight from the drift estimate, aligning compaction budget with provider context windows.
  • Add estimate_conversation_attachment_tokens and use it to subtract attachment weight from estimated_input_tokens in run_model_call.
  • Change Calibration.record and handle_committed_result to treat actual tokens as input_tokens + cache_write_tokens.
  • Update telemetry schema and Store::drift_samples to sum cache_write_tokens into actual, and add tests covering cache-write dominated rows.
  • Add driver tests verifying calibration factor behavior with large cache_write_tokens and that persisted drift-sample estimates exclude attachment tokens.
stella-core/src/estimator.rs
stella-core/src/driver.rs
stella-core/src/driver/tests.rs
stella-store/src/telemetry.rs
stella-store/src/tests.rs
Goal verdict parsing is hardened so judges whose prose includes braces or brace-containing strings no longer cause met:true verdicts to be misclassified as not met.
  • Rewrite parse_verdict to scan candidate '{' positions from the end (last-first) and attempt JSON parse for each span ending at the final '}', rather than using the outermost first..last brace span.
  • Document the new end-scanning behavior in goal module docs with reference to JUDGE_SYSTEM_PROMPT contract.
  • Add tests that cover braces in judge prose and braces inside verdict strings, asserting correct parsing and met flag handling.
stella-core/src/goal.rs
Streaming-only tool-call generations are now visible to the idle deadline via a new tool_input_delta observer hook and SpeculationGate’s unified liveness tracking, preventing healthy tool-only streams from being killed as stalled.
  • Extend ToolCallObserver trait with tool_input_delta default method and implement it in SpeculationGate to tick StreamProgress from every observer method (text, reasoning, streamed calls, and argument fragments).
  • Update anthropic, openai, and zai streaming adapters to call tool_input_delta on argument fragments / input-json deltas.
  • Modify Engine::run_model_call to pass StreamProgress into SpeculationGate instead of tapping the event sender, and add CallOnlyStreamingProvider plus witness test that a call-only stream outlives the deadline.
  • Add anthropic test observer to verify input_json_delta produces one liveness tick.
stella-protocol/src/provider.rs
stella-core/src/speculation.rs
stella-core/src/driver.rs
stella-core/src/driver/tests.rs
stella-model/src/anthropic.rs
stella-model/src/anthropic/tests.rs
stella-model/src/openai.rs
stella-model/src/zai.rs
Overflow summarization and loop detection are made robust: summarizer runs under the normal generation timeout and avoids unnecessary transcript walks, and loop detection arithmetic and call storage avoid quadratic cloning and overflow.
  • Move the estimate_conversation_tokens walk in summarize_overflow_span behind early-return guards to avoid Θ(transcript) work when not needed.
  • Apply EngineConfig.model_timeout to summarizer requests instead of leaving them unbounded, and treat summarizer timeouts like other failures in latch logic.
  • Change CallRecord.call to Cow and have recent_call_records borrow calls from the transcript, reducing per-step cloning cost.
  • Make detect_short_cycle’s period * repeats_threshold multiply saturating and adjust LoopVerdict evidence construction to work with Cow; add test for usize::MAX thresholds not panicking.
stella-core/src/driver.rs
stella-core/src/driver/loop_evidence.rs
stella-core/src/loop_detect.rs
Turn accounting for sub-agents and budget checks is corrected so child spend is incorporated into total_cost_usd before abort decisions, and Complete/Aborted outcomes now match the StepUsage-derived spend exactly.
  • Update Engine::check_budget to drain sub-agent spend, record it via record_settled_cost, and add it into total_cost_usd before evaluating budget abort conditions.
  • Adjust subagent tests to assert that Aborted outcomes include child spend in cost_usd and that Completed outcomes carry child spend exactly once.
  • Re-document BudgetWarnings construction site to reflect TurnMemos::new and clarify settlement behavior.
stella-core/src/driver/settlement.rs
stella-core/src/subagent/tests.rs
Soft-stop steering now closes caller-supplied open tool_use pairs like cancel exits do, ensuring transcripts kept across turns remain valid for subsequent provider calls.
  • On soft_stop_requested path in run_step, call close_open_tool_calls with a SOFT_STOP_TOOL_RESULT error message before returning StepOutcome::Aborted.
  • Define SOFT_STOP_TOOL_RESULT constant and add StopNow steering implementation and test asserting orphan tool call is closed with user-stop wording and provider is never called.
  • Align steering docs and tests around soft-stop behavior at step boundary.
stella-core/src/driver.rs
stella-core/src/driver/tests/steer_midturn.rs
Docs and examples are updated to match actual behavior around retries, goal telemetry ordering, step-cap responsibilities, drift samples, and cost counters, without changing core semantics.
  • Clarify retry module docs about timeout vs jitter ownership and document standard policy’s parameters.
  • Update goal module telemetry description to note judge StepUsage and BudgetTick ordering.
  • Revise stella-engine crate docs example to show host-enforced step cap via max_steps/step_cap_reason and explain run_turn vs run_step framing.
  • Fix references from stella_context::store counters to cost_counters and adjust property-test mentions to scripted tests.
stella-core/src/retry.rs
stella-core/src/goal.rs
stella-engine/src/lib.rs
stella-core/src/estimator.rs
stella-core/src/driver.rs

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants