Skip to content

fix(providers): parse Kiro meteringEvent credits and preserve in usage ledger - #5951

Closed
codingbooo wants to merge 1 commit into
lidge-jun:devfrom
codingbooo:fix/issue-5948-kiro-metering
Closed

codingbooo wants to merge 1 commit into
lidge-jun:devfrom
codingbooo:fix/issue-5948-kiro-metering

Conversation

@codingbooo

@codingbooo codingbooo commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes #5948 by parsing the Kiro stream's meteringEvent frame (and initial-response), preserving the reported credit charge in the usage ledger (providerCredits), and emitting diagnostic events for unknown event types when provider debug is enabled.

Changes

  • Event stream parsing (src/adapters/kiro-events.ts):
    • Recognize :event-type=meteringEvent and extract unit and usage (or amount).
    • Recognize :event-type=initial-response carrying conversationId.
    • Log unrecognized event types safely via debugProviderDiagnostic when provider debug is enabled, without logging sensitive payload bodies.
  • Usage & Ledger accounting (src/adapters/kiro/stream.ts, src/types/request.ts, src/server/request-log.ts, src/usage/log.ts):
    • Record parsed credit spend as providerCredits on the request log / usage records.
    • Guard against empty completions and terminal events overwriting known credit values.
  • Testing:
    • Added unit test suites tests/providers/kiro/kiro-metering-events.test.ts and tests/providers/kiro/kiro-metering-usage.test.ts (12 tests passing).

Validation

  • bun x tsc --noEmit: 0 errors
  • bun test tests/providers/kiro/kiro-metering-events.test.ts tests/providers/kiro/kiro-metering-usage.test.ts: 12 passed, 0 failed
  • bun test tests/server/server-kiro-completion-e2e.test.ts: passed

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.

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

  • New Features
    • Kiro-reported credit usage is now captured in request logs and the usage ledger, including totals across completion attempts. Credit readings remain distinct from token estimates and USD costs.
    • Kiro’s initial response event is now recognized, and unknown event types can produce opt-in diagnostics.
  • Documentation
    • Added guidance on how provider credits are recorded, combined, and distinguished from other usage measures.

@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 Sep 26, 2026
@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 52cdbad2-6340-4fa1-b93c-618cb3758dcd

📥 Commits

Reviewing files that changed from the base of the PR and between ac38d0a and 9dcdfff.

📒 Files selected for processing (19)
  • docs-site/src/content/docs/guides/providers.md
  • scripts/test-layout/layout.json
  • src/adapters/kiro-events.ts
  • src/adapters/kiro/stream.ts
  • src/server/request-log.ts
  • src/server/responses/empty-completion-guard.ts
  • src/server/responses/terminal-guard.ts
  • src/types/request.ts
  • src/usage/log.ts
  • structure/dashboard-and-usage.md
  • structure/providers-and-adapters.md
  • structure/providers/kiro.md
  • tests/fixtures/test-layout-expected.json
  • tests/providers/kiro/kiro-metering-events.test.ts
  • tests/providers/kiro/kiro-metering-usage.test.ts
  • tests/responses/empty-completion-guard.test.ts
  • tests/server/server-kiro-completion-e2e.test.ts
  • tests/server/terminal-guard.test.ts
  • tests/usage/key-attribution.test.ts

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


📝 Walkthrough

Walkthrough

Kiro metering events now contribute provider-reported credits to request usage. The adapter parses credit readings, usage merging adds readings across attempts, and normalization persists valid values separately from token estimates and USD costs.

Changes

Kiro credit metering

Layer / File(s) Summary
Parse Kiro events and capture credits
src/adapters/kiro-events.ts, src/adapters/kiro/stream.ts, tests/providers/kiro/*, scripts/test-layout/layout.json, tests/fixtures/test-layout-expected.json, structure/providers/kiro.md
The adapter parses meteringEvent and initial-response events. The stream records credit-unit readings in providerCredits. Tests cover parsing, diagnostic behavior, and stream usage.
Merge and persist provider credits
src/types/request.ts, src/usage/log.ts, src/server/request-log.ts, src/server/responses/*, tests/responses/empty-completion-guard.test.ts, tests/server/*, tests/usage/key-attribution.test.ts, structure/dashboard-and-usage.md, structure/providers-and-adapters.md, docs-site/src/content/docs/guides/providers.md
OcxUsage adds optional providerCredits. Usage normalization validates and persists non-negative finite values, and request and response merging sums credits across attempts. Tests check aggregation and persisted usage. Documentation describes the field and its distinction from token estimates and USD costs.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant parseKiroEvent
  participant KiroStream
  participant OcxUsage
  participant UsageLog
  parseKiroEvent->>KiroStream: parsed metering event
  KiroStream->>OcxUsage: providerCredits from credit-unit reading
  OcxUsage->>UsageLog: normalized usage for persistence
Loading

Merge Risk: ⚪ Minimal · up to 9dcdf

No actionable merge-blocking risk is established; the change is mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 9dcdf

The new credit accounting path is validated and remains separate from token and dollar accounting, but it is not yet clear whether a credit charge survives every interrupted request. That uncertainty matters for the integrity of usage records.

Retained concerns

  • Medium · reliability · inferred: A cancellation after a metering frame but before terminal usage emission may leave the observed credit charge out of the final request record. The downstream cancellation handoff is unverified.
Security review details

Security Blast Radius

  • inferred — The new accounting exposure is limited to requests routed through the Kiro adapter, but their credit readings can reach shared request logs and persisted usage. Evidence does not establish tenant-wide reach or a change to spend enforcement.

Security Findings and Attack Paths

  • inferred — If a client can end a stream after its metering frame without a terminal usage handoff, that request's persisted credit record may omit the charge. No quota bypass, billing effect, or proven cancellation sequence was established.

Trust Boundaries and Controls

  • observed — Kiro responses are parsed after a credential-bearing request to a provider-configured endpoint. Numeric validation and a credit-unit check constrain accepted metering data, but those checks do not authenticate an individual response frame.

Resilience and Maintainability Implications

  • observed — Malformed credit values are rejected at parsing and persistence boundaries; a provider stream error includes usage already captured by the parser.

Hardening Proposals

  • proposed — Verify the client-cancel and upstream-disconnect handoff to final request logging; if it drops an observed reading, preserve attempt-local credits when finalizing the interrupted request.
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning Issue [#5948] requires Kiro credit preservation and unknown-event diagnostics. The PR implements these in src/adapters/kiro-events.ts (meteringEvent, initial-response, and `debugProviderDiagnost… Add support for usageEvent, metricsEvent, and tokenMetrics in src/adapters/kiro-events.ts. Extract the nested usage envelope and apply the same finite, non-negative credit validation and unit handling used for meteringEvent. Add p…
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 13 files. (6 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Out of Scope Changes check ✅ Passed The changes stay within Issue [#5948]. The providerCredits type, stream capture, normalization, request-log aggregation, empty-completion and terminal-guard merges, documentation, and related tests …
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main changes: parsing Kiro meteringEvent credits and preserving them in the usage ledger.
Full details: Linked Issues check

Explanation

Issue [#5948] requires Kiro credit preservation and unknown-event diagnostics. The PR implements these in src/adapters/kiro-events.ts (meteringEvent, initial-response, and debugProviderDiagnostic) and carries providerCredits through src/adapters/kiro/stream.ts, usage normalization, request-log aggregation, and guard merges. The added tests cover parsing, diagnostics, persistence, zero values, event order, and fallback attempts. However, src/adapters/kiro-events.ts recognizes only meteringEvent; KNOWN_EVENT_TYPES and the parser do not recognize the issue's requested defensive usageEvent, metricsEvent, or tokenMetrics wrappers. The linked issue therefore has one unmet coding requirement.

Resolution

Add support for usageEvent, metricsEvent, and tokenMetrics in src/adapters/kiro-events.ts. Extract the nested usage envelope and apply the same finite, non-negative credit validation and unit handling used for meteringEvent. Add parser and stream tests for each wrapper, including malformed and non-credit cases.

Full details: Docstring Coverage

Explanation

Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 13 files. (6 skipped: 6 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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

github-actions Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

✅ READY

  • all PR quality gates passed; the review readiness checklist is complete.

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.

✅ 4/4 boxes ticked.

This pull request is already Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers: @lidge-jun @Ingwannu

@github-actions
github-actions Bot marked this pull request as draft September 26, 2026 14:07
@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 54 / 80

Kiro는 답변 스트림 끝에 "이번 요청에 크레딧을 이만큼 썼다"는 meteringEvent 쪽지를 보낸다. 지금까지 어댑터는 그 종류를 몰라서 쪽지를 버렸다. 사용 기록에는 짐작한 토큰 수만 남았다.

이 PR은 그 쪽지를 읽어서 usage.providerCredits에 넣는다. 토큰 수나 달러 비용과는 별개의 숫자다. 쪽지가 없으면 칸을 비운다. 0이 오면 0으로 남긴다. 없는 사용량을 0으로 지어내지 않는다.

한 응답 안에서 쪽지가 여러 번 오면 마지막 숫자가 이긴다. 답이 비어서 한 번 더 부르거나, 시도가 나뉘어 합쳐질 때는 크레딧을 더한다. initial-response에 실린 대화 번호도 이제 챙긴다. 디버그를 켜면 모르는 쪽지 종류 이름만 로그에 남기고, 내용물은 남기지 않는다.

src/adapters/kiro-events.ts meteringEvent - 단위나 숫자가 형식에 안 맞으면 그 쪽지만 건너뛰지 않고 요청 전체가 에러가 된다. 예전에는 이 쪽지를 몰라도 답은 그대로 나왔다.

src/adapters/kiro/stream.ts providerCredits - 한 응답 안에서는 마지막 값으로 덮어쓴다. Kiro가 조각마다 사용한 양을 나눠 보내면 앞 조각은 사라진다. 이슈 #5948에 붙은 캡처는 요청마다 쪽지가 한 번이었다.

structure/providers-and-adapters.md - 크레딧 설명 링크가 kiro.md의 추론 서명 절(#kiro-reasoning-round-trip-signature)로 간다. 크레딧 문단은 그 절 맨 끝(126줄 근처)에 붙어 있다.

PR 본문 - 게이트가 읽는 아래쪽 체크리스트 네 칸은 비어 있다. 위에 따로 둔 목록만 체크되어 있어서 이 PR은 아직 DRAFT다.

메인테이너의 판단이 필요한 지점

이슈 예시는 creditUsage에 단위와 양을 같이 담는 형태였고, usageEvent, metricsEvent, tokenMetrics도 알아보길 바랐다. 이 PR은 단위가 credit 또는 credits일 때만 숫자 하나를 남긴다. 캡처에 실제로 나온 건 meteringEvent뿐이다. 이슈를 여기서 닫을지, 나머지 이름은 후속으로 둘지 정하면 된다.

대시보드와 토큰 합계(usageDisplayTotalTokens)는 입력·출력 토큰만 더한다. providerCredits는 usage.jsonl에 남지만 화면의 비용이나 토큰 합에는 들어가지 않는다. 토큰 합이 커지지는 않는다. 화면에 보여줄지는 이 PR 밖이다.

한 응답은 마지막 스냅샷, 이어진 요청끼리는 합산. 이 규칙을 그대로 둘지 정하면 된다.

너의 추천

장부에 크레딧을 남기는 방향은 이슈의 핵심과 맞다. 머지 전에 게이트가 보는 체크리스트 네 칸을 채워서 DRAFT를 풀어라.

잘못된 meteringEvent는 답을 끊지 말고, 디버그 로그만 남긴 뒤 건너뛰는 쪽이 안전하다. 다른 알려진 이벤트도 지금처럼 예외를 던지니, 그 관례를 이 쪽지에만 풀지는 네가 고르면 된다.

크레딧 문단은 추론 절에서 빼서 제목을 따로 달아라. 이슈 #5948은 "기록에 숫자가 남는다"까지는 이 PR로 닫아도 된다. 화면의 비용 칸까지 바꾸려면 다음 작업으로 남겨라.

이 댓글은 grok-bot이 작성했습니다

@codingbooo
codingbooo marked this pull request as ready for review September 26, 2026 14:15
lidge-jun added a commit that referenced this pull request Sep 26, 2026
)

Six focused fixes from the assigned bug batch remain as separate attributed commits.

| PR | Change | Author |
| --- | --- | --- |
| #5969 | Preserve Meta Muse tool-choice semantics and reject unsupported selectors before dispatch. | shawnkim |
| #5944 | Remove unsupported hosted web-search declarations for Xiaomi MiMo destinations. | codingbo |
| #5938 | Restart the Windows service child after unexpected exits, including exit 0, while reserving the intentional stay-out code. | codingbo |
| #5935 | Reject Claude message-thread state on translated routes so the client resends full history. | kaladinhonor |
| #5939 | Rewrite standalone `\\0` escapes in Meta tool-schema patterns to equivalent `\\x00`. | boblob6969 |
| #5951 | Preserve Kiro-reported credits across stream attempts and in the usage ledger. | codingbo |

A separate integration commit keeps upstream-controlled Kiro event-type text out of opt-in debug logs. The Kiro stream retains the previously landed bounded HTTP-error text when combined with credit metering.

Left out: #5977. Independent security review found that its local read capability authenticates the request but not the HTTP response. A substituted listener could return a shape-valid forged `protected` verdict. A correct server proof bound to the nonce, endpoint, and body is outside this batch. Both its source commit and status-validation follow-up were reverted in new commits; its test and layout entries are gone. The source PR remains open.

Co-authored-by: shawnkim <shawnkim@markncompany.co.kr>
Co-authored-by: codingbo <cnsdbo@163.com>
Co-authored-by: kaladinhonor <266145786+kaladinhonor@users.noreply.github.com>
Co-authored-by: boblob6969 <boblob6969@icloud.com>
@lidge-jun

Copy link
Copy Markdown
Owner

Thanks! This landed on dev through bug-PR merge train batch 9B, #5985 (merge bf04176). Your change is one commit on dev with you as the author and a Co-authored-by trailer. One follow-up was added on top: the Kiro unknown-event diagnostic now logs only the header length, not raw upstream text. Closing since the content is now on dev.

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

Labels

bug Something isn't working review-ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants