refactor(turns): typed-internals follow-ups (trust_level, GateKind, budget-tier) - #6398
Conversation
Four copy-paste families collapsed to shared helpers, all behavior-preserving:
- lifecycle.rs: the seven `TurnRunTransitionPort` terminal methods each built the
same event (kind + sanitized reason derived from the state) and published it.
Extract `publish_transition(state)`; the three hardcoded-kind arms
(Blocked/Completed/Cancelled) provably equal `event_kind_for_state` for their
guaranteed status and carry no sanitized reason, so routing them through the
same derivation is unchanged.
- coordinator.rs: `submit/resume/retry/cancel_wake` all built the same
`TurnRunWake` literal. Extract `wake_from(scope, run_id, status, cursor)`.
- run_profile/model.rs: the seven `stream_model` milestone sites repeated the
same `if let Err(error) { debug!(kind, diagnostic_ref, msg) }` block. Extract
`log_milestone_failure(result, message)`.
- store.rs: the four `replay_*` methods re-guarded their `Error` arm with
`self.operation == X`, already guaranteed by the early return. Drop the dead
guards.
ironclaw_turns suite + clippy --all-targets --all-features green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- `sanitize_ref_suffix` was byte-for-byte identical in `instruction_bundle.rs` and `skill_context.rs`. Move it to `snippet_ref.rs` (`pub(crate)`) — the module that already owns the shared ref-hash helpers — and import it in both. - `instruction_bundle::stable_ref_hash` re-implemented the FNV-1a between-fields layout that `snippet_ref::stable_skill_snippet_display_hash` already owns. Delegate to it (bit-identical: the pinned ref-hash tests pass unchanged). - `compute_snapshot_version` / `compute_legacy_snapshot_version` in `skill_context.rs` were identical but for one `activation_state` field feed. Fold into `compute_snapshot_version_inner(entries, include_activation_state)`; the digest byte stream — and every historical snapshot version — is preserved. ironclaw_turns suite + clippy --all-targets --all-features green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`interactive_profile` (resolver.rs) and its legacy-persisted reconstruction `ResolvedRunProfile::legacy_compatibility` (snapshot.rs) re-authored the same interactive-tier sub-policy values (checkpoint policy, resource budget, cancellation policy), and all three built-in profiles repeated the locked runtime-constraints literal — so an interactive-tier value change was a two-to-three-site edit. Add preset constructors on the policy types (`CheckpointPolicy::interactive`, `ResourceBudgetPolicy::interactive`, `CancellationPolicy::interactive`, `RuntimeProfileConstraints::locked`) and route the three profile literals through them. Behavior-preserving (identical values; the profile-value tests pass unchanged); the mirror `RunProfileDefinition`/`ResolvedRunProfile` structs stay separate (they evolve independently per type-placement). ironclaw_turns suite + clippy --all-targets --all-features green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Five bounded-ref newtypes with custom `new()` validators (`LoopCheckpointStateRef`, `LoopPromptBundleRef`, `LoopSafeSummary`, `CapabilitySurfaceVersion`, `CapabilityResumeToken`) each hand-wrote the same `AsRef<str>` + `Display` + validated-`Deserialize` block that the `bounded_loop_ref!` macro already generates for the prefix-validated refs — one of them (`LoopSafeSummary`) even split its `Deserialize` away from its `AsRef`/`Display`. Extract `impl_bounded_ref_traits!($name)` emitting those three impls, have `bounded_loop_ref!` use it, and invoke it for the five hand-written types. The trait surface is now generated from one source; the custom `new()`/`as_str()` stay per-type. Behavior-preserving (the impls were byte-identical; the newtype round-trip/serde tests pass unchanged), ~80 lines removed. ironclaw_turns suite + clippy --all-targets --all-features green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… module `host.rs` had grown to ~2,592 lines — the largest file in the crate, well past the 1,500 threshold (it carried an `arch-exempt: large_file`). It mixed ~11 independent host-contract clusters. Split it, move-only, into `host/`: - mod.rs (62) — module decls + `pub use` re-exports (public `run_profile::X` paths unchanged) - capability.rs (753), model.rs (334), refs.rs (322, both macros + all ten bounded-ref newtypes), validate.rs (305), progress.rs (245), run_context.rs (224), error.rs (157), context.rs (137), checkpoint.rs (120), transcript.rs (67), input.rs (63) Move-only: bodies unchanged; a few former module-private helpers were widened to `pub(crate)` so sibling submodules reach them (none were ever re-exported, so the public API is identical). No file now exceeds 1,500 lines; the `arch-exempt: large_file` marker is dropped. Also `cargo fmt`s the crate, absorbing formatting drift left by the earlier dedup commits in `coordinator.rs` / `lifecycle.rs` / `snapshot.rs`. ironclaw_turns suite (15 binaries, 607 lib tests) + clippy --all-targets --all-features + fmt --check green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…bility.rs The `provider_tool_names_stay_at_model_protocol_boundaries` boundary test allowlists the files permitted to reference `ProviderToolName` / `provider_tool_name` (the model-protocol identity). The `host.rs` -> `host/` decomposition moved the provider-tool-call DTOs into the `capability` submodule, so update the stale allowlist path from `run_profile/host.rs` to `run_profile/host/capability.rs`. No production change; restores the boundary gate after the move. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`LoopContextSnippetMetadata.trust_level` (transient) and `PromptSkillContextMetadata.trust_level` (milestone wire DTO) were `String`, and `instruction_bundle` compared the value against `SkillTrustLevel::Trusted .as_str()` — the exact stringly-typed-domain-value anti-pattern `types.md` forbids. The field's only source is a `SkillTrustLevel` (skill snippets set it via `self.trust`; non-skill snippets have no metadata), so narrowing is sound. Type both fields as `SkillTrustLevel`; the comparison becomes `== SkillTrustLevel::Trusted`, and the snippet builder / instruction-bundle clone pass the enum directly. The enum's `#[serde(rename_all = "snake_case")]` produces the same `"installed"`/`"trusted"` tokens, so the milestone wire shape is unchanged — pinned by a new round-trip test over historical JSON. Updated the `agent_loop_host_contract` and `thread_loop_host_contract` tests that built the DTOs with string literals. ironclaw_turns suite + clippy --all-targets --all-features green; ironclaw_loop_host tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…Kind enum Five representations of "which gate a run is parked on" — `TurnStatus::Blocked*`, `BlockedReason`, `TurnBlockedGateKind`, `LoopBlockedKind`, and `ResumeTurnPrecondition::Blocked*Gate` — were wired together by ~6 hand-written mapping tables, several using `_ => None` / `matches!` catch-alls, so adding a gate kind silently compiled wrong in the ones that lagged. Introduce a canonical `GateKind` enum owning the single authoritative `GateKind <-> TurnStatus` correspondence (`blocked_status` / `from_status`, the latter exhaustive over `TurnStatus`) plus `into_blocked_reason`. Every other mapping now derives from it: - `TurnStatus::is_blocked` = `GateKind::from_status(self).is_some()` - `BlockedReason::status` = `self.gate_kind().blocked_status()` - `TurnBlockedGateKind::from_status` via `From<GateKind>` - `LoopBlockedKind::to_blocked_reason` via `From<LoopBlockedKind> for GateKind` + `GateKind::into_blocked_reason` - `ResumeTurnPrecondition::required_status` via a new `gate_kind()` accessor The five wire enums keep their names and serde (no wire change); this only consolidates the internal mappings so a new gate kind is a compiler-forced edit in `GateKind` + one arm per representation instead of scattered across ~6 sites. Behavior-preserving; ironclaw_turns suite + clippy --all-targets --all-features green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…eilings
`resolve_resource_budget_policy` branched on `tier.as_str() == "mission_high"`
and clamped to `from_trusted_static("mission_standard")` with magic `.min(128)`
/ `.min(512)` — stringly-typed dispatch plus unnamed ceilings (`types.md`).
`ResourceBudgetTier` is an intentionally-open newtype (deployments may define
further tiers), so a closed enum would wrongly remove that extensibility.
Instead name the values: `ResourceBudgetTier::{MISSION_HIGH, MISSION_STANDARD}`
consts + `is_mission_high()` / `mission_standard()`, and
`ResourceBudgetPolicy::MISSION_STANDARD_MAX_{MODEL_CALLS,CAPABILITY_INVOCATIONS}`
ceiling consts. The clamp now reads `tier.is_mission_high()` and clamps to the
named ceilings — one source of truth, no wire change.
ironclaw_turns suite + clippy --all-targets --all-features green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
🔎 IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted reviewer state before this projection. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds loop-context host contracts, typed model-route snapshots, canonical gate mappings, typed trust metadata, profile policy constructors, and accessor-based routing integrations while preserving historical serialized route and milestone formats. ChangesTyped turn infrastructure
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
⚠️ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| 0 | 0 | 0 | df5e31abf28e |
Head: df5e31abf28e7ee0aa9814b07edaa6d69279da23
Next: Human review or validation is required before merging.
Run details
Status: Current
Needs human: no
Needs validation: yes
Summary
Static review found no concrete correctness, security, or wire-format regressions. Validation could not run because the checkout lacks a Cargo/Rust toolchain.
Findings
None.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas.
|
🚅 Deployed to the ironclaw-pr-6398 environment in ironclaw-ci-preview
|
Pure `cargo fmt` output on the `use`/`pub use` lists and the skill-instruction test call touched by this PR's GateKind and trust_level commits — no behavior change. Keeps the crate fmt-clean so the post-merge Code Style gate stays green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-field sentinel (#6402) * refactor(turns): model LoopModelRouteSnapshot as an enum, not a three-field sentinel `LoopModelRouteSnapshot` distinguished a caller-requested advisory hint from an operator-resolved route by smearing the string sentinel `"requested"` across its `provider_id`/`config_version`/`auth_version` fields — an advisory route was "a route whose three non-model components all happen to equal a magic string." That is exactly the stringly-typed domain value `types.md` forbids: the distinction lived in a convention no type enforced, readable only by anyone who knew the sentinel. Make the two route kinds distinct in-memory types: enum LoopModelRouteSnapshot { Advisory { model_id }, Resolved { provider_id, model_id, config_version, auth_version }, } `from_components` is now the single place the sentinel is interpreted (shared by `new` and `Deserialize`); `is_advisory()` replaces sentinel comparison at call sites. Wire-preserving, no migration. The type is persisted in `RunRecord.resolved_model_route`, so a custom `Serialize`/`Deserialize` keeps the exact historical flat four-component object — an advisory route still writes `"requested"` in the three non-model components — and `from_components` classifies on the way back in. Old stored routes round-trip byte-for-byte; a new round-trip test pins both the advisory (sentinel) and resolved flat shapes deserializing to the right variant and re-serializing identically. Cross-crate readers (`ironclaw_runner::model_gateway` / `loop_driver_host`, `ironclaw_product_workflow` route-model read) previously accessed the struct fields directly; they now go through the `model_id()` / `provider_id()` / `config_version()` / `auth_version()` accessors. Deferred #1 from #6398 (the only non-behavior-preserving item of the typed-internals batch), delivered on its own as planned. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style(fmt): apply cargo fmt (sort turns imports) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_turns/src/run_profile/host/checkpoint.rs`:
- Around line 50-64: Change StageCheckpointPayloadRequest.schema_id from String
to the existing CheckpointSchemaId domain type, matching
LoadCheckpointPayloadRequest.expected_schema_id and preserving serialization
behavior.
In `@crates/ironclaw_turns/src/run_profile/host/error.rs`:
- Around line 80-98: Update the AgentLoopHostError thiserror message to render
kind through AgentLoopHostErrorKind::as_str() instead of Debug formatting,
preserving the existing safe_summary output and error message structure.
In `@crates/ironclaw_turns/src/run_profile/host/model.rs`:
- Around line 134-223: Update LoopPromptBundleAuthority to evict grants that are
abandoned when a run terminates, or enforce a bounded TTL for entries in
latest_by_run. Ensure cleanup is invoked for terminal loop outcomes and/or
expired grants are removed during authority access, while preserving
authorization for active runs.
- Around line 253-294: Normalize Anthropic usage when constructing
LoopModelUsage at the gateway boundary, adding cache_read_input_tokens to the
provider’s uncached input_tokens before assigning input_tokens. Keep
cache_read_input_tokens and cache_creation_input_tokens populated separately,
and ensure downstream price_usage() and OpenAI-compatible usage builders receive
total input tokens without double-counting cache tokens.
In `@crates/ironclaw_turns/src/run_profile/host/refs.rs`:
- Around line 46-64: Update the bounded_loop_ref! macro and the listed validated
newtypes to replace #[serde(transparent)] with the established #[serde(try_from
= "String")] pattern. Add or reuse TryFrom<String> implementations that route
deserialization through each type’s new() validation, including every
macro-generated type plus LoopCheckpointStateRef, LoopPromptBundleRef,
LoopSafeSummary, CapabilitySurfaceVersion, and CapabilityResumeToken, while
preserving their existing validation behavior.
In `@crates/ironclaw_turns/src/run_profile/host/validate.rs`:
- Around line 225-264: Update model_token_needs_redaction to reuse the existing
comprehensive secret detector, is_secret_like_token or
contains_secret_like_token, used by validate_loop_safe_identifier. Preserve the
current normalization and redaction flow while ensuring GitHub, GCP, Google
OAuth, AWS, and marker-word patterns receive the same coverage as identifier
validation; retain any model-specific sentinel checks if they are not already
covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 896346f9-08a8-4897-86c9-f7b1610ed2b6
📒 Files selected for processing (37)
crates/ironclaw_architecture/tests/reborn_dependency_boundaries.rscrates/ironclaw_loop_host/tests/thread_loop_host_contract.rscrates/ironclaw_product_workflow/src/reborn_services/types.rscrates/ironclaw_runner/src/loop_driver_host.rscrates/ironclaw_runner/src/model_gateway.rscrates/ironclaw_turns/src/coordinator.rscrates/ironclaw_turns/src/events.rscrates/ironclaw_turns/src/lib.rscrates/ironclaw_turns/src/lifecycle.rscrates/ironclaw_turns/src/loop_exit.rscrates/ironclaw_turns/src/request.rscrates/ironclaw_turns/src/run_profile/host.rscrates/ironclaw_turns/src/run_profile/host/capability.rscrates/ironclaw_turns/src/run_profile/host/checkpoint.rscrates/ironclaw_turns/src/run_profile/host/context.rscrates/ironclaw_turns/src/run_profile/host/error.rscrates/ironclaw_turns/src/run_profile/host/input.rscrates/ironclaw_turns/src/run_profile/host/mod.rscrates/ironclaw_turns/src/run_profile/host/model.rscrates/ironclaw_turns/src/run_profile/host/progress.rscrates/ironclaw_turns/src/run_profile/host/refs.rscrates/ironclaw_turns/src/run_profile/host/run_context.rscrates/ironclaw_turns/src/run_profile/host/transcript.rscrates/ironclaw_turns/src/run_profile/host/validate.rscrates/ironclaw_turns/src/run_profile/instruction_bundle.rscrates/ironclaw_turns/src/run_profile/milestones.rscrates/ironclaw_turns/src/run_profile/model.rscrates/ironclaw_turns/src/run_profile/policy.rscrates/ironclaw_turns/src/run_profile/refs.rscrates/ironclaw_turns/src/run_profile/resolver.rscrates/ironclaw_turns/src/run_profile/skill_context.rscrates/ironclaw_turns/src/run_profile/snapshot.rscrates/ironclaw_turns/src/run_profile/snippet_ref.rscrates/ironclaw_turns/src/status.rscrates/ironclaw_turns/src/store.rscrates/ironclaw_turns/tests/agent_loop_host_contract.rscrates/ironclaw_turns/tests/turn_coordinator_contract.rs
💤 Files with no reviewable changes (1)
- crates/ironclaw_turns/src/run_profile/host.rs
| /// Token usage reported by a provider for a single model call. The accountant | ||
| /// uses this to record actual USD spend instead of the conservative | ||
| /// reservation estimate. | ||
| #[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize)] | ||
| pub struct LoopModelUsage { | ||
| pub input_tokens: u32, | ||
| pub output_tokens: u32, | ||
| /// Tokens read from the provider's server-side prompt cache (e.g. Anthropic | ||
| /// cache reads). A subset of `input_tokens`, billed at a discount. Zero when | ||
| /// caching is unsupported or on a cache miss. | ||
| #[serde(default, skip_serializing_if = "is_zero_u32")] | ||
| pub cache_read_input_tokens: u32, | ||
| /// Tokens written to the provider's server-side prompt cache. Zero when | ||
| /// caching is unsupported or no new prefix was cached. | ||
| #[serde(default, skip_serializing_if = "is_zero_u32")] | ||
| pub cache_creation_input_tokens: u32, | ||
| } | ||
|
|
||
| fn is_zero_u32(value: &u32) -> bool { | ||
| *value == 0 | ||
| } | ||
|
|
||
| impl LoopModelUsage { | ||
| /// Accumulate another call's usage into this running per-run total. | ||
| pub fn add_assign(&mut self, other: &LoopModelUsage) { | ||
| self.input_tokens = self.input_tokens.saturating_add(other.input_tokens); | ||
| self.output_tokens = self.output_tokens.saturating_add(other.output_tokens); | ||
| self.cache_read_input_tokens = self | ||
| .cache_read_input_tokens | ||
| .saturating_add(other.cache_read_input_tokens); | ||
| self.cache_creation_input_tokens = self | ||
| .cache_creation_input_tokens | ||
| .saturating_add(other.cache_creation_input_tokens); | ||
| } | ||
|
|
||
| /// Total billable tokens (input + output). Cache tokens are already counted | ||
| /// within `input_tokens` by every provider that reports them, so they are | ||
| /// not added again here. | ||
| pub fn total_tokens(&self) -> u32 { | ||
| self.input_tokens.saturating_add(self.output_tokens) | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
rg -n "LoopModelUsage|cache_read_input_tokens|cache_creation_input_tokens|total_tokens\(" crates/ironclaw_turns crates/ironclaw_runner crates -g '!**/target/**'Repository: nearai/ironclaw
Length of output: 43792
🏁 Script executed:
sed -n '1440,1705p' crates/ironclaw_runner/src/model_gateway.rs && printf '\n---\n' && sed -n '1510,1565p' crates/ironclaw_reborn_composition/src/llm_admin/openai_compat_serve.rs && printf '\n---\n' && sed -n '132,220p' crates/ironclaw_common/src/llm_costs.rsRepository: nearai/ironclaw
Length of output: 16881
🏁 Script executed:
sed -n '300,470p' crates/ironclaw_llm/src/anthropic_oauth.rs && printf '\n---\n' && sed -n '360,420p' crates/ironclaw_llm/src/provider.rs && printf '\n---\n' && sed -n '1210,1335p' crates/ironclaw_reborn_composition/src/llm_admin/openai_compat_serve/tests.rsRepository: nearai/ironclaw
Length of output: 13273
🏁 Script executed:
rg -n "total_tokens\(\)|total_tokens\b" crates/ironclaw_* crates/ironclaw_common -g '!**/target/**'Repository: nearai/ironclaw
Length of output: 4363
🏁 Script executed:
sed -n '360,420p' crates/ironclaw_llm/src/provider.rs && printf '\n---\n' && sed -n '330,380p' crates/ironclaw_llm/src/anthropic_oauth.rs && printf '\n---\n' && sed -n '136,170p' crates/ironclaw_common/src/llm_costs.rsRepository: nearai/ironclaw
Length of output: 5560
🏁 Script executed:
rg -n "struct AnthropicResponse|usage:.*input_tokens|cache_read_input_tokens|cache_creation_input_tokens|cached" crates/ironclaw_llm/src/anthropic_oauth.rs crates/ironclaw_llm/src -g '!**/target/**'Repository: nearai/ironclaw
Length of output: 29665
🌐 Web query:
Anthropic API usage object input_tokens cache_read_input_tokens cache_creation_input_tokens documentation
💡 Result:
In the Anthropic API, the usage object provides granular information about token consumption when prompt caching is enabled [1][2]. These fields help track how your prompt context is being processed, cached, and retrieved [1][3]. The primary fields related to prompt caching are: - cache_creation_input_tokens: The number of input tokens written to the cache during a request [1][3]. This occurs when you provide content with a cache_control breakpoint that has not yet been cached or needs to be refreshed [1][4]. These tokens are billed at a higher rate (typically 1.25x the base input token price) [3][5]. - cache_read_input_tokens: The number of input tokens successfully retrieved from the cache [1][3]. When this occurs, you benefit from lower costs (typically 0.1x the base input token price) and reduced latency [1][6]. - input_tokens: This field represents the number of input tokens that were not read from the cache or used to create a cache entry [1][3]. Essentially, these are the tokens provided after the last cache breakpoint in your request, which are processed as standard, uncached input [1][3]. The total input token count for a request can be calculated as [1][3]: total_input_tokens = cache_read_input_tokens + cache_creation_input_tokens + input_tokens If both cache_creation_input_tokens and cache_read_input_tokens are 0, it indicates that no caching occurred, often because the request did not meet the minimum token length requirements for caching [1][3]. By monitoring these fields, you can verify cache hits and diagnose issues such as silent invalidators (e.g., dynamic content like timestamps in your prompt prefix that prevent cache reuse) [7][8].
Citations:
- 1: https://platform.claude.com/docs/en/build-with-claude/prompt-caching
- 2: https://github.com/anthropics/anthropic-sdk-python/blob/04b468da/src/anthropic/types/usage.py
- 3: https://platform.claude.com/docs/en/build-with-claude/prompt-caching?fcdaa149_sort_date=desc
- 4: https://github.com/anthropics/anthropic-cookbook/blob/7dc310a7/misc/prompt_caching.ipynb
- 5: https://platform.claude.com/cookbook/misc-prompt-caching
- 6: https://claude.com/blog/prompt-caching
- 7: https://platform.claude.com/docs/en/build-with-claude/cache-diagnostics
- 8: https://github.com/anthropics/skills/blob/main/skills/claude-api/python/claude-api/README.md
Normalize Anthropic usage before populating LoopModelUsage
AnthropicResponse.usage.input_tokens is uncached-only, but price_usage() and the OpenAI-compat usage builder treat LoopModelUsage.input_tokens as total input with cache reads already included. This mapping forwards Anthropic’s raw values unchanged, so cached Anthropic calls are undercounted in cost/reporting. Convert to the shared contract at the gateway boundary instead of storing raw provider semantics here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/ironclaw_turns/src/run_profile/host/model.rs` around lines 253 - 294,
Normalize Anthropic usage when constructing LoopModelUsage at the gateway
boundary, adding cache_read_input_tokens to the provider’s uncached input_tokens
before assigning input_tokens. Keep cache_read_input_tokens and
cache_creation_input_tokens populated separately, and ensure downstream
price_usage() and OpenAI-compatible usage builders receive total input tokens
without double-counting cache tokens.
There was a problem hiding this comment.
Confirmed a valid, real cost-accounting bug: LoopModelUsage.input_tokens is contracted as TOTAL input (cache_read is a subset), but the Anthropic provider mapping forwards Anthropic input_tokens (uncached-only), so cached Anthropic calls undercount input in price_usage and the OpenAI-compat builder. The fix belongs at the provider/gateway mapping boundary (add cache_read_input_tokens to input_tokens when constructing LoopModelUsage from an Anthropic response), with a regression test. This is pre-existing (not introduced by this PR's typed-internals) and not CI-blocking; leaving OPEN and flagging for a focused follow-up rather than changing billing math inline here.
There was a problem hiding this comment.
@ilblackdragon, agreed. Keeping this open and non-blocking is appropriate: the defect predates #6398, while the correction changes provider cost-accounting semantics.
The focused follow-up should normalize Anthropic usage at the provider/gateway boundary:
LoopModelUsage.input_tokens = input_tokens + cache_read_input_tokens- retain
cache_read_input_tokensandcache_creation_input_tokensseparately - add a regression test covering both
price_usage()and OpenAI-compatible usage output
This follows the repository Rust invariant that every bug fix includes a regression test. Would you like me to open and assign a GitHub follow-up issue to you?
✏️ Learnings added
Learnt from: ilblackdragon
Repo: nearai/ironclaw PR: 6398
File: crates/ironclaw_turns/src/run_profile/host/model.rs:266-307
Timestamp: 2026-07-21T07:25:28.478Z
Learning: In the Rust model usage accounting contract, `LoopModelUsage.input_tokens` represents total input tokens and `cache_read_input_tokens` is a subset. Anthropic’s raw `usage.input_tokens` is uncached-only, so the Anthropic provider/gateway mapping must add `cache_read_input_tokens` when constructing `LoopModelUsage.input_tokens`, while retaining cache fields separately. This pre-existing cost-accounting bug is outside PR `#6398` and requires a focused follow-up with regression coverage for `price_usage()` and OpenAI-compatible usage output.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
# Conflicts: # crates/ironclaw_loop_host/tests/thread_loop_host_contract.rs # crates/ironclaw_turns/src/run_profile/host/capability.rs # crates/ironclaw_turns/src/run_profile/host/checkpoint.rs # crates/ironclaw_turns/src/run_profile/host/context.rs # crates/ironclaw_turns/src/run_profile/host/model.rs # crates/ironclaw_turns/src/run_profile/host/progress.rs # crates/ironclaw_turns/src/run_profile/host/run_context.rs # crates/ironclaw_turns/src/run_profile/host/validate.rs # crates/ironclaw_turns/src/run_profile/policy.rs # crates/ironclaw_turns/src/run_profile/skill_context.rs
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 86.34% — 319030 / 369523 lines Per-crate breakdown (65 crates, lowest-covered first)
This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors. Exemptions (3 entry/entries excluded from the accounting above)
|
What & why
The typed-internals follow-ups deferred from #6391 —
types.mdfixes that replace stringly-typed domain values / scattered mappings with proper types. Stacked on #6391 (refactor/turns-crate-dedup-pass-2); review/merge that one first.Three of the four deferred items are here. Each is behavior-preserving (no wire-format change) and independently green (
cargo build,cargo clippy -p ironclaw_turns --all-targets --all-featureszero-warning,cargo test -p ironclaw_turns;ironclaw_loop_hosttests for the trust_level DTO consumer).Commits
trust_level→SkillTrustLevel.LoopContextSnippetMetadata.trust_level(transient) andPromptSkillContextMetadata.trust_level(milestone wire DTO) wereString, compared via== SkillTrustLevel::Trusted.as_str()(the exact anti-patterntypes.mdforbids). The field's only source is aSkillTrustLevel, so narrowing is sound; the enum'ssnake_caseserde produces the same"installed"/"trusted"tokens (wire-compatible), pinned by a new round-trip test over historical JSON.GateKind. Five representations of "which gate a run is parked on" (TurnStatus::Blocked*,BlockedReason,TurnBlockedGateKind,LoopBlockedKind,ResumeTurnPrecondition::Blocked*Gate) were wired by ~6 hand-written mapping tables, several with_ => None/matches!catch-alls that silently mishandle a new gate kind. Introduce a canonicalGateKindowning the single authoritativeGateKind <-> TurnStatuscorrespondence (from_statusexhaustive overTurnStatus) +into_blocked_reason; every other mapping derives from it. The five wire enums keep their names/serde — only the internal mappings are consolidated, so a new gate kind is a compiler-forced edit in one cluster.resolve_resource_budget_policybranched ontier.as_str() == "mission_high"and clamped with magic.min(128)/.min(512).ResourceBudgetTieris an intentionally-open newtype (a closed enum would remove extensibility), so name the values instead:ResourceBudgetTier::{MISSION_HIGH,MISSION_STANDARD}+is_mission_high()/mission_standard(), andResourceBudgetPolicy::MISSION_STANDARD_MAX_{MODEL_CALLS,CAPABILITY_INVOCATIONS}ceiling consts.Deferred: #1 —
LoopModelRouteSnapshotadvisory sentinel → enumHeld out of this PR because it is not behavior-preserving — it is a persisted-state + cross-crate wire migration, materially riskier than the three above:
LoopModelRouteSnapshotis persisted inRunRecord.resolved_model_route, so changing the flat 4-field struct (advisory = the three"requested"sentinel fields) intoenum { Advisory { model_id } | Resolved { .. } }changes the serialized shape — stored run routes written by the old code must still deserialize.ironclaw_runner::model_gatewayreads.provider_id/.model_id/.config_version/.auth_version), so the enum needs accessors and the runner's reads must be updated.A dedicated follow-up PR should do it with: backward-compat
Deserializeaccepting the existing flat shape (or a wire-stable flat repr under an enum in-memory type), a round-trip test over historical persisted JSON, and theironclaw_runnerfield-access updates. Its stringly-typing today is already behind a named constant (ADVISORY_MODEL_ROUTE_COMPONENT) +is_advisory(); only the storage shape remains.🤖 Generated with Claude Code