Skip to content

Align shared page score computation with local history popup#40

Merged
silas229 merged 1 commit into
mainfrom
copilot/ensure-shared-page-point-calculation
Jun 6, 2026
Merged

Align shared page score computation with local history popup#40
silas229 merged 1 commit into
mainfrom
copilot/ensure-shared-page-point-calculation

Conversation

Copilot AI commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

The shared round view was calculating points with a different B-part time unit than the local history popup, leading to divergent total scores for the same round. This change aligns both views to the same scoring input contract.

  • Scoring consistency fix

    • Updated shared page scoring input to pass B-part time in rounded seconds (§ 5.6 behavior), matching the local history popup logic.
    • Eliminates ms-vs-s mismatch that previously inflated/deflated shared-view totals.
  • Regression coverage

    • Added a focused unit test for SharedRoundCard that asserts the rendered total points for a representative round payload where unit mismatch would be visible.
const scoringParams: ScoringParameters = {
  // ...
  bPartTime: Math.round(roundData.totalTime / 1000), // align with local popup
  // ...
};

@sonarqubecloud

sonarqubecloud Bot commented Jun 6, 2026

Copy link
Copy Markdown

@codecov

codecov Bot commented Jun 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 52.96%. Comparing base (6c77f2f) to head (2f35193).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #40      +/-   ##
==========================================
+ Coverage   52.84%   52.96%   +0.11%     
==========================================
  Files          22       22              
  Lines        1790     1790              
  Branches      198      199       +1     
==========================================
+ Hits          946      948       +2     
+ Misses        844      842       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Aligns shared round score computation with the local history dialog by ensuring the B-part time passed into scoring is in rounded seconds (rather than milliseconds), and adds a regression test to prevent the mismatch from reappearing.

Changes:

  • Convert roundData.totalTime from ms → rounded seconds when populating ScoringParameters.bPartTime in SharedRoundCard.
  • Add a unit test asserting the rendered total points for a round payload where ms-vs-s would be observable.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
components/shared-round-card.tsx Converts B-part time input to rounded seconds for scoring to match the local dialog behavior.
tests/unit/components/shared-round-card.test.tsx Adds a regression test asserting the total points rendered by SharedRoundCard.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 49 to +53
aPartErrorPoints: roundData.aPartErrorPoints!,
knotTime: roundData.knotTime!,
aPartPenaltySeconds: roundData.aPartPenaltySeconds!,
overallImpression: roundData.overallImpression!,
bPartTime: roundData.totalTime,
bPartTime: Math.round(roundData.totalTime / 1000), // § 5.6
@silas229 silas229 marked this pull request as ready for review June 6, 2026 11:57
@silas229 silas229 merged commit 63293a2 into main Jun 6, 2026
8 checks passed
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.

3 participants