Skip to content

Conversation

@the-dev-z
Copy link
Collaborator

@the-dev-z the-dev-z commented Nov 7, 2025

Pull Request - Frontend

Summary

This PR fixes issue #633 by properly handling missing data in the competition page's head-to-head comparison.

Problem

The original code would display NaN% when trader data was missing or invalid. The first attempted fix (PR #633, later reverted in #676) used null coalescing to default to 0%, but this was misleading as it suggested traders had equal performance when data was actually unavailable.

Solution

  • Added hasValidData validation that checks for null, undefined, and NaN values
  • Display (em dash) for invalid trader.total_pnl_pct instead of 0% or NaN%
  • Only show gap calculations (leadingBy, behindBy) when both traders have valid data
  • Show placeholder when data is unavailable

Why This Approach?

  • 0% means "no gap" (traders are tied) - misleading when data is missing
  • NaN% is a technical error that confuses users
  • clearly indicates "data unavailable" without misleading users

Testing

  • Tested with valid trader data (gap calculations work correctly)
  • Tested with null/undefined values (shows )
  • Tested with NaN values (shows )

Closes #633


🎯 Type of Change | 變更類型

  • 🐛 Bug fix | 修復 Bug
  • ✨ New feature | 新功能
  • 💥 Breaking change | 破壞性變更
  • 🎨 Code style update | 代碼樣式更新
  • ♻️ Refactoring | 重構
  • ⚡ Performance improvement | 性能優化

🔗 Related Issues | 相關 Issue

  • Closes # | 關閉 #
  • Related to # | 相關 #

🧪 Testing | 測試

Test Environment | 測試環境

  • OS | 操作系統: macOS / Linux
  • Node Version | Node 版本: 18+
  • Browser(s) | 瀏覽器: Chrome / Firefox / Safari

Manual Testing | 手動測試

  • Tested in development mode | 開發模式測試通過
  • Tested production build | 生產構建測試通過
  • Tested on multiple browsers | 多瀏覽器測試通過
  • Tested responsive design | 響應式設計測試通過
  • Verified no existing functionality broke | 確認沒有破壞現有功能

🌐 Internationalization | 國際化

  • All user-facing text supports i18n | 所有面向用戶的文本支持國際化
  • Both English and Chinese versions provided | 提供了中英文版本
  • N/A | 不適用

✅ Checklist | 檢查清單

Code Quality | 代碼質量

  • Code follows project style | 代碼遵循項目風格
  • Self-review completed | 已完成代碼自查
  • Comments added for complex logic | 已添加必要註釋
  • Code builds successfully | 代碼構建成功 (npm run build)
  • Ran npm run lint | 已運行 npm run lint
  • No console errors or warnings | 無控制台錯誤或警告

Documentation | 文檔

  • Updated relevant documentation | 已更新相關文檔
  • Updated type definitions (TypeScript) | 已更新類型定義
  • Added JSDoc comments where necessary | 已添加 JSDoc 註釋

Git

  • Commits follow conventional format | 提交遵循 Conventional Commits 格式
  • Rebased on latest dev branch | 已 rebase 到最新 dev 分支
  • No merge conflicts | 無合併衝突

By submitting this PR, I confirm | 提交此 PR,我確認:

  • I have read the Contributing Guidelines | 已閱讀貢獻指南
  • I agree to the Code of Conduct | 同意行為準則
  • My contribution is licensed under AGPL-3.0 | 貢獻遵循 AGPL-3.0 許可證

🌟 Thank you for your contribution! | 感謝你的貢獻!

…S#633)

- Add hasValidData validation for null/undefined/NaN
- Display '—' for invalid trader.total_pnl_pct
- Only show gap calculations when both values are valid
- Prevents misleading users with 0% when data is missing

Fixes NoFxAiOS#633
: '—'}
</div>
{isWinning && gap > 0 && (
{hasValidData && isWinning && gap > 0 && (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里的模式可以抽取可复用函数的,辛苦看下怎么处理比较好?

boolean && component 方式

@xqliu
Copy link
Contributor

xqliu commented Nov 8, 2025

代码审查报告 - PR #678

审查结果:✅ 通过(数据显示改进)

业务层面审查

✅ 需求验证

  • 需求明确:竞赛页面缺失数据时显示 '—' 而不是 'NaN%' 或 '0%'
  • 需求合理:提升用户体验,避免误导性的数字显示
  • 数据完整性:正确处理 null、undefined、NaN 的情况

✅ 功能完整性

  • 验证 total_pnl_pct 是否为有效数字
  • 缺失数据显示 '—'
  • 有效数据显示百分比(+/- 符号)
  • 领先/落后差距仅在有效数据时显示

技术层面审查

✅ 数据验证逻辑

验证条件

const hasValidData =
  trader.total_pnl_pct != null &&        // 不是 null 或 undefined
  opponent.total_pnl_pct != null &&
  !isNaN(trader.total_pnl_pct) &&        // 不是 NaN
  !isNaN(opponent.total_pnl_pct)

优点

  • 完整的边界条件检查
  • 使用 != null 同时检查 null 和 undefined
  • 显式检查 NaN(JavaScript 中 NaN !== NaN)

✅ 条件渲染

修改前

{/* 总是显示,可能是 NaN% 或 0.00% */}
{(trader.total_pnl ?? 0) >= 0 ? '+' : ''}
{trader.total_pnl_pct?.toFixed(2) || '0.00'}%

修改后

{trader.total_pnl_pct != null && !isNaN(trader.total_pnl_pct)
  ? `${trader.total_pnl_pct >= 0 ? '+' : ''}${trader.total_pnl_pct.toFixed(2)}%`
  : '—'}

改进

  • 明确区分有效数据和缺失数据
  • 使用 '—' 表示缺失,符合设计规范
  • 避免误导性的 '0.00%'

✅ Gap 计算逻辑

修改前

const gap = trader.total_pnl_pct - opponent.total_pnl_pct
// 可能计算出 NaN

修改后

const gap = hasValidData
  ? trader.total_pnl_pct - opponent.total_pnl_pct
  : NaN

改进

  • 先验证数据有效性
  • 无效数据不计算 gap
  • 避免 NaN 传播

✅ 领先/落后显示

修改

// Before: 可能显示 "NaN%" 的差距
{isWinning && gap > 0 && (...)}

// After: 仅在有效数据时显示
{hasValidData && isWinning && gap > 0 && (...)}

新增缺失数据提示

{!hasValidData && (
  <div className="text-xs font-semibold" style={{ color: '#848E9C' }}></div>
)}

E2E 验证报告

✅ 测试场景1:正常数据

数据:
- trader.total_pnl_pct = 10.5
- opponent.total_pnl_pct = 5.2

预期:
- 显示 "+10.50%"
- 显示 "领先 5.30%"

实际:✅ 符合预期

✅ 测试场景2:null 数据

数据:
- trader.total_pnl_pct = null
- opponent.total_pnl_pct = 5.2

预期:
- 显示 "—"
- 不显示领先/落后差距
- 底部显示 "—"

实际:✅ 符合预期

✅ 测试场景3:NaN 数据

数据:
- trader.total_pnl_pct = NaN
- opponent.total_pnl_pct = 5.2

预期:
- 显示 "—"
- 不显示领先/落后差距

实际:✅ 符合预期

✅ 测试场景4:undefined 数据

数据:
- trader.total_pnl_pct = undefined
- opponent.total_pnl_pct = 5.2

预期:
- 显示 "—"
- 不显示领先/落后差距

实际:✅ 符合预期(!= null 检查包含 undefined)

✅ 测试场景5:负数

数据:
- trader.total_pnl_pct = -5.3

预期:
- 显示 "-5.30%"(不带 + 号)
- 红色文本

实际:✅ 符合预期

代码质量审查

✅ 代码清晰度

  • hasValidData 变量命名清晰
  • 条件逻辑易于理解
  • 注释说明验证逻辑

✅ 一致性

  • 所有数据显示都使用相同的验证逻辑
  • 颜色使用一致(绿色 #0ECB81,红色 #F6465D,灰色 #848E9C)

✅ 错误处理

  • 完整的边界条件处理
  • 不会抛出异常
  • 降级策略明确(显示 '—')

UI/UX 设计验证

✅ 视觉一致性

缺失数据标识

  • 使用 '—'(em dash)而不是 '-'(hyphen)
  • 灰色文本(#848E9C),表示不可用状态
  • 与其他缺失数据显示一致

✅ 用户体验

Before

  • ❌ 显示 "NaN%" 令人困惑
  • ❌ 显示 "0.00%" 可能误导(实际是缺失数据)
  • ❌ 显示领先/落后 "NaN%" 毫无意义

After

  • ✅ '—' 清晰表示数据缺失
  • ✅ 不显示误导性数字
  • ✅ 缺失数据不计算差距

🟡 改进建议(非 BLOCKING)

建议1:提取验证函数

// 提取为独立函数,便于复用
const isValidNumber = (value: number | null | undefined): boolean => {
  return value != null && !isNaN(value)
}

// 使用
const hasValidData = 
  isValidNumber(trader.total_pnl_pct) && 
  isValidNumber(opponent.total_pnl_pct)

建议2:添加 Tooltip

// 对于缺失数据,提供悬停提示
{!hasValidData && (
  <Tooltip content="数据暂不可用">
    <div className="text-xs font-semibold" style={{ color: '#848E9C' }}></div>
  </Tooltip>
)}

建议3:类型安全

// 在 TypeScript 类型定义中明确可选性
interface Trader {
  total_pnl?: number | null  // 明确可以是 undefined 或 null
  total_pnl_pct?: number | null
}

建议4:单元测试

describe('CompetitionPage data display', () => {
  it('displays "—" for null data', () => {
    const trader = { total_pnl_pct: null }
    render(<CompetitionPage traders={[trader, ...]} />)
    expect(screen.getByText('—')).toBeInTheDocument()
  })
  
  it('displays "—" for NaN data', () => {
    const trader = { total_pnl_pct: NaN }
    render(<CompetitionPage traders={[trader, ...]} />)
    expect(screen.getByText('—')).toBeInTheDocument()
  })
  
  it('displays percentage for valid data', () => {
    const trader = { total_pnl_pct: 10.5 }
    render(<CompetitionPage traders={[trader, ...]} />)
    expect(screen.getByText('+10.50%')).toBeInTheDocument()
  })
})

总结

这是一个高质量的数据显示改进 PR

优点

  1. ✅ 完整的边界条件处理(null、undefined、NaN)
  2. ✅ 用户体验提升(不显示误导性数字)
  3. ✅ 视觉一致性好(使用 '—' 表示缺失)
  4. ✅ 逻辑清晰(hasValidData 变量)
  5. ✅ 代码简洁(三元表达式)

改进空间(非 BLOCKING)

  1. ⚠️ 提取验证函数(代码复用)
  2. ⚠️ 添加 Tooltip(用户提示)
  3. ⚠️ 类型安全(TypeScript 类型定义)
  4. ⚠️ 添加单元测试(验证边界条件)

审查结论:✅ 通过,建议合并

重要性

  • 修复用户体验问题(显示 NaN%)
  • 提升数据展示专业性
  • 避免误导性信息

影响范围

  • 只影响竞赛页面显示
  • 无API或业务逻辑更改
  • 低风险改动

@the-dev-z
Copy link
Collaborator Author

此 PR 已獲批准 ✅

可以合併了嗎?謝謝!🙏

- Test data validation logic (null/undefined/NaN detection)
- Test gap calculation with valid and invalid data
- Test display formatting (shows '—' instead of 'NaN%')
- Test leading/trailing message display conditions
- Test edge cases (Infinity, very small/large numbers)

All 25 test cases passed, covering:
1. hasValidData check (7 cases): valid/null/undefined/NaN/zero/negative
2. gap calculation (3 cases): valid data, invalid data, negative gap
3. display formatting (6 cases): positive/negative/null/undefined/NaN/zero
4. leading/trailing messages (5 cases): conditional display logic
5. edge cases (4 cases): Infinity, -Infinity, very small/large numbers

Related to PR NoFxAiOS#678 - ensures missing data displays as '—' instead of 'NaN%'.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@hzb1115 hzb1115 merged commit 0514610 into NoFxAiOS:dev Nov 11, 2025
16 checks passed
sparrow211 pushed a commit to sparrow211/nofx that referenced this pull request Nov 11, 2025
…S#678)

* fix(web): display '—' for missing data instead of NaN% or 0% (NoFxAiOS#633)

- Add hasValidData validation for null/undefined/NaN
- Display '—' for invalid trader.total_pnl_pct
- Only show gap calculations when both values are valid
- Prevents misleading users with 0% when data is missing

Fixes NoFxAiOS#633

* test(web): add comprehensive unit tests for CompetitionPage NaN handling

- Test data validation logic (null/undefined/NaN detection)
- Test gap calculation with valid and invalid data
- Test display formatting (shows '—' instead of 'NaN%')
- Test leading/trailing message display conditions
- Test edge cases (Infinity, very small/large numbers)

All 25 test cases passed, covering:
1. hasValidData check (7 cases): valid/null/undefined/NaN/zero/negative
2. gap calculation (3 cases): valid data, invalid data, negative gap
3. display formatting (6 cases): positive/negative/null/undefined/NaN/zero
4. leading/trailing messages (5 cases): conditional display logic
5. edge cases (4 cases): Infinity, -Infinity, very small/large numbers

Related to PR NoFxAiOS#678 - ensures missing data displays as '—' instead of 'NaN%'.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>

---------

Co-authored-by: ZhouYongyou <[email protected]>
Co-authored-by: Claude <[email protected]>
@the-dev-z the-dev-z deleted the fix/competition-missing-data-display branch November 12, 2025 08:37
bebest2010 pushed a commit to bebest2010/nofx that referenced this pull request Nov 18, 2025
…S#678)

* fix(web): display '—' for missing data instead of NaN% or 0% (NoFxAiOS#633)

- Add hasValidData validation for null/undefined/NaN
- Display '—' for invalid trader.total_pnl_pct
- Only show gap calculations when both values are valid
- Prevents misleading users with 0% when data is missing

Fixes NoFxAiOS#633

* test(web): add comprehensive unit tests for CompetitionPage NaN handling

- Test data validation logic (null/undefined/NaN detection)
- Test gap calculation with valid and invalid data
- Test display formatting (shows '—' instead of 'NaN%')
- Test leading/trailing message display conditions
- Test edge cases (Infinity, very small/large numbers)

All 25 test cases passed, covering:
1. hasValidData check (7 cases): valid/null/undefined/NaN/zero/negative
2. gap calculation (3 cases): valid data, invalid data, negative gap
3. display formatting (6 cases): positive/negative/null/undefined/NaN/zero
4. leading/trailing messages (5 cases): conditional display logic
5. edge cases (4 cases): Infinity, -Infinity, very small/large numbers

Related to PR NoFxAiOS#678 - ensures missing data displays as '—' instead of 'NaN%'.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>

---------

Co-authored-by: ZhouYongyou <[email protected]>
Co-authored-by: Claude <[email protected]>
tinkle-community added a commit that referenced this pull request Nov 26, 2025
* fix(web): display '—' for missing data instead of NaN% or 0% (#633)

- Add hasValidData validation for null/undefined/NaN
- Display '—' for invalid trader.total_pnl_pct
- Only show gap calculations when both values are valid
- Prevents misleading users with 0% when data is missing

Fixes #633

* test(web): add comprehensive unit tests for CompetitionPage NaN handling

- Test data validation logic (null/undefined/NaN detection)
- Test gap calculation with valid and invalid data
- Test display formatting (shows '—' instead of 'NaN%')
- Test leading/trailing message display conditions
- Test edge cases (Infinity, very small/large numbers)

All 25 test cases passed, covering:
1. hasValidData check (7 cases): valid/null/undefined/NaN/zero/negative
2. gap calculation (3 cases): valid data, invalid data, negative gap
3. display formatting (6 cases): positive/negative/null/undefined/NaN/zero
4. leading/trailing messages (5 cases): conditional display logic
5. edge cases (4 cases): Infinity, -Infinity, very small/large numbers

Related to PR #678 - ensures missing data displays as '—' instead of 'NaN%'.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: tinkle-community <[email protected]>

---------

Co-authored-by: ZhouYongyou <[email protected]>
Co-authored-by: tinkle-community <[email protected]>
tinkle-community added a commit that referenced this pull request Nov 26, 2025
* fix(web): display '—' for missing data instead of NaN% or 0% (#633)

- Add hasValidData validation for null/undefined/NaN
- Display '—' for invalid trader.total_pnl_pct
- Only show gap calculations when both values are valid
- Prevents misleading users with 0% when data is missing

Fixes #633

* test(web): add comprehensive unit tests for CompetitionPage NaN handling

- Test data validation logic (null/undefined/NaN detection)
- Test gap calculation with valid and invalid data
- Test display formatting (shows '—' instead of 'NaN%')
- Test leading/trailing message display conditions
- Test edge cases (Infinity, very small/large numbers)

All 25 test cases passed, covering:
1. hasValidData check (7 cases): valid/null/undefined/NaN/zero/negative
2. gap calculation (3 cases): valid data, invalid data, negative gap
3. display formatting (6 cases): positive/negative/null/undefined/NaN/zero
4. leading/trailing messages (5 cases): conditional display logic
5. edge cases (4 cases): Infinity, -Infinity, very small/large numbers

Related to PR #678 - ensures missing data displays as '—' instead of 'NaN%'.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: tinkle-community <[email protected]>

---------

Co-authored-by: ZhouYongyou <[email protected]>
Co-authored-by: tinkle-community <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

界面显示小问题: 当前差距显示了 NaN%

3 participants