feat(billing): highlight matched conditional multipliers in logs - #6561
feat(billing): highlight matched conditional multipliers in logs#6561seefs001 wants to merge 2 commits into
Conversation
WalkthroughBilling expressions now trace request-rule matches through compilation, settlement, usage logs, and the pricing breakdown. The UI uses traced rules when available and marks matched conditional multipliers. ChangesMatched request rule tracing
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant BillingExpressionCompiler
participant runProgram
participant ComputeTieredQuotaWithRequest
participant InjectTieredBillingInfo
participant DynamicPricingBreakdown
BillingExpressionCompiler->>runProgram: execute patched request-rule expressions
runProgram-->>ComputeTieredQuotaWithRequest: return RequestRules
ComputeTieredQuotaWithRequest->>InjectTieredBillingInfo: provide TieredResult.RequestRules
InjectTieredBillingInfo-->>DynamicPricingBreakdown: provide other.request_rules
DynamicPricingBreakdown->>DynamicPricingBreakdown: render matched multiplier rows
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web/src/features/pricing/components/dynamic-pricing-breakdown.tsx (1)
158-164: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer direct props access in this component.
The change extends props destructuring with
matchedRequestRules. Useprops.matchedRequestRulesinstead. This follows the frontend rule to avoid unnecessary component-props destructuring.🤖 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 `@web/src/features/pricing/components/dynamic-pricing-breakdown.tsx` around lines 158 - 164, Update DynamicPricingBreakdown to stop destructuring matchedRequestRules from its props; access it through props.matchedRequestRules throughout the component while preserving the existing behavior and other prop handling.Source: Coding guidelines
🤖 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 `@pkg/billingexpr/compile.go`:
- Around line 43-49: Define and use one canonical trace-key normalization
contract across pkg/billingexpr/compile.go at the _trace emission in compile and
web/src/features/pricing/lib/billing-expr.ts at the matchedRequestRules
comparison. Update compile to emit the normalized conditional key instead of
conditional.Cond.String(), and update the pricing breakdown logic to normalize
raw ternary condition text with the same contract before matching, ensuring both
sides produce identical stable keys.
---
Nitpick comments:
In `@web/src/features/pricing/components/dynamic-pricing-breakdown.tsx`:
- Around line 158-164: Update DynamicPricingBreakdown to stop destructuring
matchedRequestRules from its props; access it through props.matchedRequestRules
throughout the component while preserving the existing behavior and other prop
handling.
🪄 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 Plus
Run ID: 7caeb5fd-8975-4e06-a301-f5b7970d411d
📒 Files selected for processing (11)
controller/channel_test_internal_test.gopkg/billingexpr/billingexpr_test.gopkg/billingexpr/compile.gopkg/billingexpr/run.gopkg/billingexpr/settle.gopkg/billingexpr/types.goservice/log_info_generate.goweb/src/features/pricing/components/dynamic-pricing-breakdown.tsxweb/src/features/pricing/lib/billing-expr.tsweb/src/features/usage-logs/components/dialogs/details-dialog.tsxweb/src/features/usage-logs/types.ts
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/features/pricing/components/dynamic-pricing-breakdown.tsx (1)
283-295: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDesktop tier table still uses index-based keys; mobile view was fixed but not desktop.
Line 289 switches the mobile tier list to occurrence-based keys derived from tier content, to avoid key collisions/reconciliation issues for duplicate tier rows. The desktop
StaticDataTableat Line 359 still usesgetRowKey={(_tier, index) => \tier-${index}`}`, so the same duplicate-tier scenario that justified the mobile fix remains unaddressed there.Apply the same occurrence-based key strategy to the desktop table for parity.
🔧 Proposed fix
const mobileTierKeyOccurrences = new Map<string, number>() + const desktopTierKeyOccurrences = new Map<string, number>() const requestRuleKeyOccurrences = new Map<string, number>()data={tiers} - getRowKey={(_tier, index) => `tier-${index}`} + getRowKey={(tier) => + `tier-${nextOccurrenceKey(JSON.stringify(tier), desktopTierKeyOccurrences)}` + }Verify
StaticDataTableinvokesgetRowKeyexactly once per row per render before relying on a stateful occurrence map 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 `@web/src/features/pricing/components/dynamic-pricing-breakdown.tsx` around lines 283 - 295, Update the desktop StaticDataTable getRowKey callback to use the same tier-content occurrence-based strategy as the mobile tiers.map path, deriving keys from JSON.stringify(tier) and a desktop-scoped occurrence map rather than the row index. Ensure the occurrence map is reset appropriately per render and confirm getRowKey is invoked once per row per render before using this stateful approach.
🤖 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 `@pkg/billingexpr/expr.md`:
- Around line 125-127: Add the text language identifier to the fenced code block
containing the request-probe expression in expr.md, changing the opening fence
to use text while leaving the example content unchanged.
---
Outside diff comments:
In `@web/src/features/pricing/components/dynamic-pricing-breakdown.tsx`:
- Around line 283-295: Update the desktop StaticDataTable getRowKey callback to
use the same tier-content occurrence-based strategy as the mobile tiers.map
path, deriving keys from JSON.stringify(tier) and a desktop-scoped occurrence
map rather than the row index. Ensure the occurrence map is reset appropriately
per render and confirm getRowKey is invoked once per row per render before using
this stateful approach.
🪄 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 Plus
Run ID: b5cd8e82-b252-483e-92c6-eb96fc1b47e7
📒 Files selected for processing (12)
controller/channel_test_internal_test.gopkg/billingexpr/billingexpr_test.gopkg/billingexpr/compile.gopkg/billingexpr/expr.mdpkg/billingexpr/run.gopkg/billingexpr/settle.gopkg/billingexpr/types.goservice/log_info_generate.goweb/src/features/pricing/components/dynamic-pricing-breakdown.tsxweb/src/features/pricing/lib/billing-expr.tsweb/src/features/usage-logs/components/dialogs/details-dialog.tsxweb/src/features/usage-logs/types.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- controller/channel_test_internal_test.go
- web/src/features/usage-logs/components/dialogs/details-dialog.tsx
- pkg/billingexpr/settle.go
| ``` | ||
| <request-probe condition> ? <numeric literal> : 1 | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a language identifier to the fenced code block.
The fenced block at Line 125 has no language tag. Static analysis (markdownlint MD040) flags this.
📝 Proposed fix
-```
+```text
<request-probe condition> ? <numeric literal> : 1</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 125-125: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@pkg/billingexpr/expr.md` around lines 125 - 127, Add the text language
identifier to the fenced code block containing the request-probe expression in
expr.md, changing the opening fence to use text while leaving the example
content unchanged.
Source: Linters/SAST tools
1 participant
Important
📝 变更描述 / Description
(简述：做了什么？为什么这样改能生效？请基于你对代码逻辑的理解来写，避免粘贴未经整理的内容)
对表达式计费的命中条件进行高亮
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)
Summary by CodeRabbit
New Features
Bug Fixes