[fix] normalize candidate probability display in request timeline#1085
Conversation
|
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 (9)
📝 WalkthroughWalkthrough跨多个语言的国际化消息文件进行了更新，移除了概率显示中的百分号，并添加了新的无概率消息变体。相应的格式化逻辑和测试也进行了调整，以支持有效和无效概率的条件渲染。 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 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 docstrings
🧪 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 |
github-actions
Bot
added
bug
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves the formatting of provider probabilities within the provider chain display by refactoring the formatting logic into dedicated helper functions and updating localization files. It introduces new translation keys to gracefully handle missing or invalid probability data, ensuring that UI elements like the technical timeline and descriptions do not display empty fragments or incorrect symbols. Additionally, the PR updates several test suites to verify these formatting improvements and adjusts some UI labels for better clarity. I have no feedback to provide.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
🧪 测试结果
总体结果: ✅ 所有测试通过 |
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
This PR successfully fixes a probability display bug in the Technical Timeline and Decision Chain visualization. The changes centralize probability formatting through the shared formatProbability() utility, ensuring consistent percentage display across all locales.
PR Size: M
- Lines changed: 329 (277 additions, 52 deletions)
- Files changed: 9
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Adequate (3 new test cases covering formatter paths, real component rendering, and edge cases)
- Code clarity - Good
Key Changes Verified
-
Core Fix (
src/lib/utils/provider-chain-formatter.ts:50-85): AddedformatDescriptionCandidate()andformatTimelineCandidate()helpers that pipe probability values throughformatProbability()before passing to i18n templates. -
i18n Templates (all 5 locales): Removed literal
%suffix fromdescription.candidateandtimeline.candidateInfotemplates; addedcandidateNoProbabilityandcandidateInfoNoProbabilityvariants for undefined/null/NaN cases. -
Test Coverage:
- Unit tests for formatted probability output (4.5% instead of 0.045...)
- Description formatting with real message bundles
- Edge case handling for invalid probabilities (undefined, NaN)
- Component-level integration test in error-details-dialog
The fix correctly addresses the root cause where raw probability values (0..1 decimals) were being passed to templates that appended %, resulting in incorrect display like 0.045454...% instead of 4.5%.
Automated review by Claude AI
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR.
PR Size: M
- Lines changed: 329
- Files changed: 9
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Adequate
- Code clarity - Good
Automated review by Codex AI
There was a problem hiding this comment.
- Identified PR
#1085and pulled metadata, changed files, and full diff. - Computed PR size as M (277 additions + 52 deletions = 329 lines; 9 files) and applied label
size/M. - Completed diff-scoped 6-perspective review + validation; no issues met the >=80 confidence threshold for reporting.
- Submitted the required PR review summary via
gh pr review --comment.
1 participant
Summary
%suffixes fromprovider-chaincandidate templates and let the formatter own percent output across localesRoot Cause
Technical Timeline candidate rows were passing raw
c.probabilityvalues like0.045454...into i18n templates that appended%, while the Decision Chain visualization already usedformatProbability(...). That split formatting responsibility caused the details dialog timeline to show0.045454...%instead of4.5%.Validation
bun run buildbun run typecheckbunx @biomejs/biome@2.4.12 check .(passes with pre-existing repo warnings/infos only)bunx vitest run src/lib/utils/provider-chain-formatter.test.ts src/app/[locale]/dashboard/logs/_components/error-details-dialog.test.tsx src/app/[locale]/dashboard/logs/_components/provider-chain-popover.test.tsx --reporter=dotgit diff --checkNotes
bun run teststill hits a repo-level Vitest worker OOM near the end of the suite (523 passed / 2 skipped / 1 unhandled worker OOMin the conservative rerun). I did not see any assertion failures tied to this change.Greptile Summary
This PR fixes a display bug where candidate probabilities in the Technical Timeline were showing raw
0.045454...%instead of formatted4.5%. The fix pipes all probability values through the sharedformatProbability()formatter before passing them to i18n templates, and removes the literal%suffix from the templates themselves. NewcandidateNoProbability/candidateInfoNoProbabilitytemplate variants handle cases where probability isundefined,null, orNaN. Tests cover all three paths (timeline formatter, description formatter, real component rendering).Confidence Score: 5/5
Safe to merge — targeted bug fix with no breaking changes and solid regression coverage.
The change is narrow and well-understood: raw probability values are now formatted before reaching i18n templates, the literal % is removed from templates, and fallback keys handle edge cases. Three test files cover the unit formatter, description formatter, and real component rendering. No P0/P1 issues found.
No files require special attention.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Candidate probability raw 0..1] --> B{formatProbability} B -->|valid| C[Formatted string e.g. 4.5%] B -->|null or NaN or negative| D[null] C --> E[description.candidate with name and probability] C --> F[timeline.candidateInfo with name weight cost probability] D --> G[description.candidateNoProbability with name only] D --> H[timeline.candidateInfoNoProbability with name weight cost]Reviews (1): Last reviewed commit: "fix: normalize candidate probability dis..." | Re-trigger Greptile