Repository navigation
Kiro: log the usage Kiro reports on every wire, and forward combo child usage after commit - #7433
Draft
hedging8563 wants to merge 2 commits into
Draft
hedging8563 wants to merge 2 commits into
hedging8563 wants to merge 2 commits into
Conversation
Problem 1. A combo child logs into its own context. The combo copies that context into the request context once, when the child commits. The bridge binds the adapter usage later, at the terminal frame, so the request context never learned that usage came from the bridge. The request log then re-read the bridged Responses wire. That wire has no providerCredits, zero-fills the cache detail, and derives the input count from the context checkpoint. The log row showed a high input count, zero cache and no credits on /v1/messages, /v1/responses and /v1/chat/completions alike. Problem 2. Every Kiro row was marked estimated and raised to the local input estimate, even when Kiro sent a tokenUsage block. Fix 1. The combo passes each child onBridgeUsageBound. The transport calls it when it binds bridge usage. After the child commits, the combo sets usageFromBridge and usage on its own context. Fix 2. The Kiro parser marks usage read from tokenUsage as upstreamReported. A two-leg turn keeps the mark only when both legs have it. usageForFinalLog and finalizedUsage skip the provider-wide estimated mark and the input floor for marked usage. Usage without the mark, and usage read off a wire, stays estimated. The wire that the client receives is unchanged. Tests: new tests/server/server-kiro-usage-log-e2e.test.ts replays one Kiro stream (metering and tokenUsage, then no tokenUsage, then two legs with tools) through /v1/responses, /v1/messages (stream and non-stream) and /v1/chat/completions, on a direct route and on a combo route. A unit test in request-log-estimate-cap.test.ts covers the mark. The Kiro stream test now expects the mark. 15 of the 21 new cases fail without the fix. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The combo forwarded bridge usage to the request context when the child set childCommitted. The flag was set before callbackGate.commit(). If the commit threw, a later usage binding of that child still reached the request context. The combo now keeps two flags for each target: childCommitted, set after the commit, and childUsageBound, set by the callback. It forwards only when both are true. A binding that comes before the commit waits for the commit. A target that fails or never commits forwards nothing. The callback takes no argument now, because the combo already holds the child context. Tests: new tests/responses/responses-combo-bridge-usage.test.ts drives executeComboResponses with a fake dispatcher. It covers a binding after the commit, a binding before the commit, a refused first target with a committed second target, and a commit that throws. A new case in request-log-estimate-cap.test.ts pins the aggregate rule: the upstreamReported mark needs usage on every attempt, and an empty list or attempts without usage give no mark. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
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 |
Contributor
|
✅ Deterministic PR hygiene checks passed. |
Contributor
⏳ DRAFT
What to do
Review readiness checklist
0/4 boxes ticked. This PR stays in draft until every box above is ticked. |
Owner
|
Follow-up intake at |
Draft
4 tasks
This branch has not been deployed
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
3 participants
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.
Problem: A combo child logs into its own context and the request log re-reads the bridged Responses wire, which has no providerCredits, zero-fills cache detail and derives input from the context checkpoint. Every Kiro row was also marked estimated and raised to the local input estimate, even when Kiro sent tokenUsage.
Fix:
Tests: new tests/server/server-kiro-usage-log-e2e.test.ts (direct and combo route; /v1/responses, /v1/messages stream and non-stream, /v1/chat/completions), tests/responses/responses-combo-bridge-usage.test.ts, new cases in request-log-estimate-cap.test.ts. tsc --noEmit clean. Related suites pass: tests/server/server-kiro*, tests/responses/responses-combo*, tests/combos, tests/usage (1303 pass, 0 fail), kiro-metering-usage and kiro-stream, test-layout (27 pass).
Overlap check against #7389 and later dev commits: none (#7389 touches core-combo.ts for failedAdapter only; cherry-picks applied cleanly).
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
Required local validation passed; commands, results, and any full-suite exception are documented.
I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.