Skip to content

fix(usage): retain Kiro prompt estimates in hosted tool paths - #7620

Closed
martinbcg wants to merge 3 commits into
lidge-jun:devfrom
martinbcg:codex/kiro-sidecar-usage
Closed

martinbcg wants to merge 3 commits into
lidge-jun:devfrom
martinbcg:codex/kiro-sidecar-usage

Conversation

@martinbcg

@martinbcg martinbcg commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Preserve AdapterRequest.usageLog.inputTokens when the Responses hosted web-search and image/video paths build a request. The callbacks already recorded reasoning and service-tier metadata, but omitted the adapter's prompt estimate before onAttemptSend and final usage logging.
  • This makes Kiro Usage totals, per-model input tokens, physical-attempt usage, and daily activity include replayed prompt history, matching ordinary adapter dispatch. Merely declaring web_search or image_generation could previously trigger the undercount even when the model did not call the tool.
  • Add an integration regression covering ordinary dispatch, hosted search, and hosted images with synthetic Kiro event streams, isolated credentials, and isolated state directories. Update the adapter documentation and transport ownership description.

Reproduction

Send a Kiro Responses request containing a long earlier user message, an assistant reply, and a short new user message, with a function tool and either web_search or image_generation declared. Before the fix, the ordinary route persisted more than 20,000 prompt tokens while both hosted-tool routes persisted only 4 input tokens for the same history. The new regression verifies the persisted request, its physical attempt, model totals, and daily totals.

The change forwards the adapter's existing estimate; it does not reinterpret contextTotalTokens as request spend, change client compaction accounting, or rewrite historical ledger entries. Kiro tokens remain estimated and provider credits remain separate.

Verification

Verified on current head 3d4c28417745736ada723bdf75c03f96db2e644e, rebased onto current upstream dev at 488d5f13ef9fe0edd37100d31e129d136a2b4629 after the maintainer's 2026-10-24 triage note.

  • Red/green reproduction before the fix: ordinary dispatch passed; hosted search and images failed with 4 input tokens instead of the replayed prompt estimate.
  • With the repository-pinned Bun 1.4.3, bun scripts/test.ts tests/responses/responses-sidecar-usage.test.ts tests/responses/responses-sidecar-context-overflow.test.ts tests/test-layout.test.ts tests/test-layout-tooling.test.ts — 40 passed, 0 failed.
  • bun run typecheck — passed.
  • bun run privacy:scan — passed.
  • bun run structure:check — passed.
  • In docs-site/, bun install --frozen-lockfile and bun run build — passed; 593 pages built and 84,861 internal links checked.
  • git diff upstream/dev...HEAD --check — passed.

Full-suite scope exception (AGENTS.md, Commands): the repository contains 2,608 test files, which is disproportionate to this two-line propagation of existing numeric usage metadata. The focused integration regression exercises all three dispatch paths through persisted request/attempt usage and daily/model aggregation; the adjacent context-overflow tests and test-layout checks also passed. The commands and exact-head results are recorded above. All listed tests, typecheck, privacy scan, structure check, and the documentation build were rerun after the rebase and shard-registration change and passed. The full suite and broader import-graph selection were not run; executable upstream CI remains pending maintainer approval.

Maintainer triage follow-up (2026-10-24)

  • Moved responses-sidecar-usage.test.ts out of the inline explicit map into scripts/test-layout/layout-usage.json, registered through explicitShards.
  • Preserved its responses mapping in tests/fixtures/test-layout-expected.json; both layout/oracle regression files pass.
  • Refreshed the complete PR onto upstream dev at 488d5f13ef9fe0edd37100d31e129d136a2b4629.
  • Fresh executable Cross-platform CI run 40099391124 targets 3d4c28417745736ada723bdf75c03f96db2e644e and is action_required, with no executable jobs started. The contributor account has repository READ permission, so an upstream maintainer must approve the run. This is pending CI evidence, not a successful run.
  • The code and local validation are ready for review. Successful executable CI and maintainer approval remain prerequisites for merging.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. The runtime change only copies existing numeric usage metadata; test credentials are synthetic and all test fetches reject unexpected destinations.

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.

Summary by CodeRabbit

  • Bug Fixes
    • Usage and daily activity now retain the full prompt estimate for requests with hosted search or image tools, including replayed conversation history, even when those tools aren’t invoked.
    • Prompt usage estimates are recorded consistently for requests with and without hosted tools.
  • Documentation
    • Updated the Kiro reference to clarify how prompt usage is reported for requests with hosted tools.

@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 619d328 and 3d4c284.

Files selected for processing: 3
  • scripts/test-layout/layout-usage.json
  • scripts/test-layout/layout.json
  • tests/fixtures/test-layout-expected.json

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Walkthrough

Kiro sidecar callbacks now record numeric input-token estimates from built requests. Tests cover usage accounting with no sidecar, web-search sidecar, and image-generation sidecar. Documentation describes the recorded estimates.

Changes: Kiro prompt usage accounting

Layer / File(s) Summary
Capture and validate prompt estimates
src/server/responses/sidecar-execution.ts, tests/responses/responses-sidecar-usage.test.ts, docs-site/src/content/docs/reference/adapters.md, structure/transports/responses.md, scripts/test-layout/layout-usage.json, scripts/test-layout/layout.json, tests/fixtures/test-layout-expected.json
The image-bridge and web-search callbacks copy numeric input-token estimates into the usage log. Tests check request, attempt, daily, and model input-token usage for requests with no sidecar, web search, and image generation. Documentation and test-layout mappings describe and register this coverage.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: Minimal · up to 3d4c2

No concrete merge-blocking risk remains in the reviewed change.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: preserving Kiro prompt estimates when hosted tool paths process requests. This matches the updates in src/server/responses/sidecar-execution.…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (3 skipped: 3 …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title clearly and concisely describes the main change: preserving Kiro prompt estimates when hosted tool paths process requests. This matches the updates in src/server/responses/sidecar-execution.ts, the usage tests, and the documentation.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (3 skipped: 3 unsupported.)

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Oct 23, 2026
@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • review readiness checklist open (0/4 boxes ticked).

What to do

  • Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

  • ⬜ 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.

0/4 boxes ticked.

This PR stays in draft until every box above is ticked.

@github-actions
github-actions Bot marked this pull request as ready for review October 23, 2026 13:53
@github-actions
github-actions Bot marked this pull request as draft October 23, 2026 14:01
@github-actions
github-actions Bot marked this pull request as ready for review October 23, 2026 14:03
@lidge-jun

Copy link
Copy Markdown
Owner

Triage note (2026-10-24, against dev at 2494848).

Needs fixes before this can land: move the new test registration into an explicit shard, preserve the fixture mapping, then obtain successful executable CI.

Still needed on dev: yes. Both callbacks still record reasoning/tier without copying usageLog.inputTokens (src/server/responses/sidecar-execution.ts:586, :696).

Exact-head CI at triage time: 619d328 — policy checks and CodeRabbit pass; ci.yml ACTION_REQUIRED, run 39966088929. A cancelled label run has a successful replacement. lane=all on exact head: n.a.

Please refresh onto current dev when addressing these; heads older than dev need a fresh exact-head CI run before review.

@martinbcg
martinbcg force-pushed the codex/kiro-sidecar-usage branch from 619d328 to 3d4c284 Compare October 24, 2026 13:43
@martinbcg
martinbcg marked this pull request as draft October 24, 2026 13:47
@martinbcg
martinbcg marked this pull request as ready for review October 24, 2026 14:00
@github-actions
github-actions Bot marked this pull request as draft October 24, 2026 15:28
lidge-jun added a commit that referenced this pull request Oct 26, 2026
Hosted web-search and image/video sidecar paths now record the adapter's usageLog.inputTokens estimate, so Kiro usage and daily activity include replayed prompt history. Carries #7620.

Co-authored-by: Martin Becerra <197874+martinbcg@users.noreply.github.com>
@lidge-jun

Copy link
Copy Markdown
Owner

Release triage for v2.93.0: carried and merged. Your change landed on dev through #7744 as squash commit 2d8463a, with a Co-authored-by trailer crediting you.

The source change is identical to this PR's diff. The only difference is that the new test is registered in the existing layout-adapters shard instead of a new shard, to avoid colliding with another PR. It passed exact-head CI and an independent review, and the regression test fails without the fix. Closing this draft in favour of the carry. Thank you!

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants