feat: add generation debug inspector for usage logs#5621
Closed
MaJingyiFighting wants to merge 2 commits into
Closed
feat: add generation debug inspector for usage logs#5621MaJingyiFighting wants to merge 2 commits into
MaJingyiFighting wants to merge 2 commits into
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (15)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (12)
WalkthroughAdds a new ChangesGeneration Debug Pipeline
Sequence Diagram(s)sequenceDiagram
participant Client
participant RelayHandler as TextHelper / ResponsesHelper
participant generationdebug as generationdebug pkg
participant Upstream as Upstream LLM Provider
participant QuotaService as text_quota / quota
participant LogDB as Log Storage
Client->>RelayHandler: Incoming request
RelayHandler->>generationdebug: Begin(ctx, requestID, streaming)
RelayHandler->>generationdebug: CaptureInboundRequest(ctx, req, bodyReader)
RelayHandler->>generationdebug: CaptureUpstreamRequest(ctx, upstreamJSON)
RelayHandler->>generationdebug: MarkUpstreamStart(ctx)
RelayHandler->>Upstream: DoRequest(httpReq)
Upstream-->>RelayHandler: httpResp
RelayHandler->>generationdebug: WrapResponseBody(ctx, httpResp.Body, streaming)
generationdebug-->>RelayHandler: tee ReadCloser (writes to limitedCapture)
RelayHandler->>generationdebug: MarkResponseComplete(ctx)
RelayHandler->>QuotaService: PostTextConsumeQuota(usage, ...)
QuotaService->>generationdebug: MergeContextIntoLogOther(ctx, other, usage, meta)
generationdebug->>generationdebug: ExtractOutputFromSSE / RawResponse
generationdebug->>generationdebug: BuildCacheStatsFromUsage
generationdebug->>generationdebug: ApplyPromptAccounting
generationdebug-->>QuotaService: other["generation_debug"] = Summary
generationdebug-->>QuotaService: other["admin_info"]["generation_debug_raw"] = RawDebug
QuotaService->>LogDB: WriteLog(other)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Author
提供两个截图供参考，样式是参照openrouter做的 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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
`@web/classic/src/components/table/usage-logs/generation-debug/PromptDebugPanel.jsx`:
- Around line 303-317: The selectedUnitIndex state is storing logical unit
indices (unit.index) but the code is using it as a direct array offset to access
the units array at line 317 (selectedUnit = units[selectedUnitIndex]). This
causes mismatches when unit indices are sparse or not sequentially aligned with
array positions. Fix this by either storing the array position/offset instead of
the logical unit index when setting selectedUnitIndex, or by finding the unit
from the units array by matching its index property against the stored logical
index value, rather than using it directly as an array offset.
In `@web/classic/src/components/table/usage-logs/generation-debug/utils.js`:
- Around line 113-118: The cacheStatus assignment logic for zero-token units
uses a strict less-than comparison (`<`) on line 115 that incorrectly marks
units as 'miss' when unit.cumulative_start exactly equals estimatedCachedTokens
at the boundary. Change the comparison operator from `<` to `<=` in the
condition checking `unit.cumulative_start < estimatedCachedTokens` to ensure
zero-token units at the exact boundary are marked as 'hit' instead of 'miss',
preserving cache continuity in the visualization.
In `@web/classic/src/i18n/locales/zh-CN.json`:
- Around line 3-79: The zh-CN.json locale file is missing the Copied translation
key that is used by PromptDebugPanel.jsx when displaying copy success feedback.
Add a Copied entry to the zh-CN.json file with an appropriate Chinese
translation to ensure Chinese users see the localized message instead of the
English fallback when a copy action succeeds. Place this new key in a logical
location among the existing translations, ideally near Copy path for
consistency.
In
`@web/default/src/features/usage-logs/components/generation-debug/json-viewer.tsx`:
- Line 51: The Badge component with variant='secondary' contains a hard-coded
string "JSON" that is not localized, preventing it from respecting locale
changes. Replace the hard-coded "JSON" text with a call to the i18n translation
function t() using the appropriate translation key for the JSON label. Ensure
the translation key is imported and used consistently with other localized
strings in the component.
In
`@web/default/src/features/usage-logs/components/generation-debug/prompt-debug-panel.tsx`:
- Around line 62-63: The selectedUnitIndex state is inconsistently used: it is
initialized as an array position at line 62, but at line 152 it is being set to
unit.index (the unit's index property) instead of the unit's position in the
units array. This causes line 87 to select the wrong unit when indices are
sparse or non-sequential. Fix this by ensuring that when storing the selected
unit at line 152, you store the actual array position of that unit in the units
array (using the findIndex method or similar) rather than the unit.index
property, so that selectedUnitIndex consistently represents the array position
throughout the component.
- Around line 199-203: In the promptTokens constant declaration, replace the
logical OR operators (||) with nullish coalescing operators (??). The current
implementation using || treats explicit zero values for providerPromptTokens as
falsy and incorrectly falls back to other estimates, which can misreport
authoritative vendor totals. The nullish coalescing operator ?? only treats null
and undefined as falsy values, which will properly preserve explicit zero values
from the provider while still falling back to estimates when the value is
actually null or undefined.
In `@web/default/src/features/usage-logs/components/generation-debug/utils.ts`:
- Around line 333-350: The boundary object construction in derivePromptCacheView
unconditionally overwrites the source and confidence fields with
'cache_boundary_inference' and 'inferred' values, replacing any authoritative
backend metadata from existingBoundary. To preserve existing cache-boundary
metadata when provided, conditionally set the source and confidence fields only
when existingBoundary does not already have these values, allowing fallback to
inferred values only when no existing boundary data is present. This way,
authoritative backend source and confidence values are retained when available
instead of being downgraded to inferred values.
In `@web/default/src/i18n/locales/ja.json`:
- Around line 651-684: In the ja.json file, normalize all Japanese debug-label
translations by replacing the English word "token" with its Japanese equivalent
"トークン" in the translation values. Specifically, update all entries in the
provided diff where "token" appears in English (such as in "Cache overlap",
"Cache write tokens", "Estimated prompt tokens", "Estimated tokens", "Provider
cached tokens", "Provider prompt tokens", and "Tokens per prompt field") to use
"トークン" instead for consistency in the Japanese UI.
In `@web/default/src/i18n/locales/ru.json`:
- Line 653: The Russian translation file is missing the cache_status subkeys
(hit, partial, miss, write, unknown) that are referenced by cacheStatusLabel.
Add a cache_status object with translations for all five status values (hit,
partial, miss, write, unknown) to the ru.json file, similar to how the "Cache
status" label is currently defined. This will prevent the UI from falling back
to English text for cache status values.
In `@web/default/src/i18n/locales/vi.json`:
- Around line 649-654: The Vietnamese locale file is missing the specific
cache_status.* keys that are referenced by the debug panel utility (specifically
in the generation-debug utils which maps cache statuses through
cache_status.hit, cache_status.partial, cache_status.miss, cache_status.write,
and cache_status.unknown). While the generic "Cache status" key exists, users
will fall back to English if these specific keys are not defined. Add the
following locale keys to the vi.json file: cache_status.hit,
cache_status.partial, cache_status.miss, cache_status.write, and
cache_status.unknown, each with an appropriate Vietnamese translation describing
that specific cache status type.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eea51309-44f6-447d-b245-49868c544711
📒 Files selected for processing (41)
pkg/generationdebug/capture.gopkg/generationdebug/config.gopkg/generationdebug/extract.gopkg/generationdebug/generationdebug_test.gopkg/generationdebug/merge.gopkg/generationdebug/sanitize.gopkg/generationdebug/types.gorelay/chat_completions_via_responses.gorelay/compatible_handler.gorelay/responses_handler.goservice/generation_debug_billing_test.goservice/quota.goservice/text_quota.goweb/classic/src/components/table/usage-logs/generation-debug/GenerationDebugEntry.jsxweb/classic/src/components/table/usage-logs/generation-debug/GenerationDebugModal.jsxweb/classic/src/components/table/usage-logs/generation-debug/JsonViewer.jsxweb/classic/src/components/table/usage-logs/generation-debug/PromptDebugPanel.jsxweb/classic/src/components/table/usage-logs/generation-debug/RawDebugPanel.jsxweb/classic/src/components/table/usage-logs/generation-debug/TokenMessageChart.jsxweb/classic/src/components/table/usage-logs/generation-debug/utils.jsweb/classic/src/components/table/usage-logs/index.jsxweb/classic/src/hooks/usage-logs/useUsageLogsData.jsxweb/classic/src/i18n/locales/zh-CN.jsonweb/classic/src/i18n/locales/zh-TW.jsonweb/classic/src/i18n/locales/zh.jsonweb/default/src/features/usage-logs/components/dialogs/details-dialog.tsxweb/default/src/features/usage-logs/components/generation-debug/completion-debug-panel.tsxweb/default/src/features/usage-logs/components/generation-debug/generation-debug-section.tsxweb/default/src/features/usage-logs/components/generation-debug/json-viewer.tsxweb/default/src/features/usage-logs/components/generation-debug/prompt-debug-panel.tsxweb/default/src/features/usage-logs/components/generation-debug/raw-debug-panel.tsxweb/default/src/features/usage-logs/components/generation-debug/token-message-chart.tsxweb/default/src/features/usage-logs/components/generation-debug/types.tsweb/default/src/features/usage-logs/components/generation-debug/utils.tsweb/default/src/features/usage-logs/types.tsweb/default/src/i18n/locales/en.jsonweb/default/src/i18n/locales/fr.jsonweb/default/src/i18n/locales/ja.jsonweb/default/src/i18n/locales/ru.jsonweb/default/src/i18n/locales/vi.jsonweb/default/src/i18n/locales/zh.json
Author
|
Addressed all 10 CodeRabbit findings in Validation rerun after the fixes:
The zero-token boundary regression test now covers a zero-length field exactly at the cache breakpoint. |
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
1 participant
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.
Important
📝 变更描述 / Description
新增可选的生成调试采集与查看能力。后端在请求转发和计费阶段记录经过脱敏、限长处理的提示字段、供应商 token 用量、缓存读取/写入及响应摘要；Default 与 Classic 日志界面使用同一份调试数据，展示字段 token 分布、缓存命中区间和断裂点。
供应商返回的 prompt/cached token 作为精确总量，字段级 token 和缓存边界按累计比例推断并明确标注置信度。零 token 字段按其所在缓存前缀继承状态，不再产生中途未命中、随后又命中的假断层。缓存写入与缓存命中分开统计，旧日志缺少边界数据时可由供应商总量重建视图。原始请求和响应仅管理员可见，且受环境配置控制。
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
📸 运行证明 / Proof of Work
以下检查均在合并最新 upstream/main 后通过：
缓存连续性新增单测覆盖：缓存前缀中的零 token 字段保持命中，真实断裂点仍落在后续部分命中的非空字段内。
Summary by CodeRabbit