Skip to content

Hotfix: Compare values were 3 orders of magnitude to low due to double division #1191

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 18, 2025

Conversation

ArneTR
Copy link
Member

@ArneTR ArneTR commented May 16, 2025

This PR introduces a QoL feature by:

  • Showing untransformed and unrounded values in title of values in metrics table
  • If a value is lower than the rounding resolution and/or really 0 a tooltip warning is added

Fix:

  • The frontend was displaying values wrongly as multiple orders of magnitude too low in comparison mode for diffing
    • Repeated runs were not affected
    • Single run display was not affected
    • API data retrieval was not affected.
    • Key metric boxes where unaffected
    • The relative deviation was not affected (percentage display in right column)

PR is already live on metrics.green-coding.io in this state.

However PR will stay open until tests are added to avoid regression

Screenshots QoL

Hovering: Screenshot 2025-05-16 at 3 21 37 PM

Tooltip
Screenshot 2025-05-16 at 3 17 41 PM

Greptile Summary

Fixed critical display issue in comparison mode where metric values were shown 3 orders of magnitude too low, and added tooltips for better value visibility.

  • Fixed value calculation in frontend/js/helpers/phase-stats.js by directly using untransformed mean values from detail_data['data'][key]?.mean
  • Added tooltips in frontend/js/helpers/metric-boxes.js to show original values when below rounding threshold
  • Added comprehensive test suite in tests/frontend/test_frontend.py for comparison values and tooltips
  • Note: Fix affects only comparison mode diffing; repeated runs, single run display, and key metric boxes were unaffected

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

2 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile

ArneTR added 3 commits May 16, 2025 15:22
* main:
  Sampling rate rework (#1194)
  Phase padding can now be turned on and off (#1193)
  User 0 should have flow_process_duration and total duration only at 30 minutes and no data in json 'measurement'
  AI-Tests can now activated and deactivated in tests
  (Testing QoL): JS errors in frontend tests are now reported
  typo
  added no-else-raise
  Checking in more cases now if github detected even if path broken
  AI Optimisations Frontend added to FOSS version as appetizer (#1192)
  Allow repo URLs with unknown schemes but issues warning
  Revert "Test fix\nwe changed from failing on unknowns to allowing them due to allowing other vendors or private repos with reduced capbility tokens that might be cloneable but do not expose the API"
  general wording
@ArneTR
Copy link
Member Author

ArneTR commented May 18, 2025

Regression tests added

@ArneTR
Copy link
Member Author

ArneTR commented May 18, 2025

@greptileai

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

LGTM

3 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

@ArneTR ArneTR merged commit 2cead07 into main May 18, 2025
1 of 2 checks passed
@ArneTR ArneTR deleted the hotfix-frontend-compare-display branch May 18, 2025 09:30
ArneTR added a commit that referenced this pull request May 18, 2025
* main: (22 commits)
  Hotfix: Compare values were 3 orders of magnitude to low due to double division (#1191)
  Sampling rate rework (#1194)
  Phase padding can now be turned on and off (#1193)
  User 0 should have flow_process_duration and total duration only at 30 minutes and no data in json 'measurement'
  AI-Tests can now activated and deactivated in tests
  (Testing QoL): JS errors in frontend tests are now reported
  typo
  added no-else-raise
  Checking in more cases now if github detected even if path broken
  AI Optimisations Frontend added to FOSS version as appetizer (#1192)
  Allow repo URLs with unknown schemes but issues warning
  Revert "Test fix\nwe changed from failing on unknowns to allowing them due to allowing other vendors or private repos with reduced capbility tokens that might be cloneable but do not expose the API"
  general wording
  Runtime phase reconstruction only when runtime phase is present
  (fix): shutdown_on_job_no must only be non false
  (fix): Null check for resolution must also be in system_checks
  (fix): Providers without resolution must also be mappable to _sampling_interval_padding
  Test fix\nwe changed from failing on unknowns to allowing them due to allowing other vendors or private repos with reduced capbility tokens that might be cloneable but do not expose the API
  Phase end cutoff mitigation (#1161)
  Guard clause that runner.run_until may never be used without a context
  ...
ArneTR added a commit that referenced this pull request May 19, 2025
* main: (73 commits)
  Forcing int64 in pandas to be safe
  Splits the diskio provider into reads and writes (#1189)
  Sorting by created_at now
  Hotfix: Compare values were 3 orders of magnitude to low due to double division (#1191)
  Sampling rate rework (#1194)
  Phase padding can now be turned on and off (#1193)
  User 0 should have flow_process_duration and total duration only at 30 minutes and no data in json 'measurement'
  AI-Tests can now activated and deactivated in tests
  (Testing QoL): JS errors in frontend tests are now reported
  typo
  added no-else-raise
  Checking in more cases now if github detected even if path broken
  AI Optimisations Frontend added to FOSS version as appetizer (#1192)
  Allow repo URLs with unknown schemes but issues warning
  Revert "Test fix\nwe changed from failing on unknowns to allowing them due to allowing other vendors or private repos with reduced capbility tokens that might be cloneable but do not expose the API"
  general wording
  Runtime phase reconstruction only when runtime phase is present
  (fix): shutdown_on_job_no must only be non false
  (fix): Null check for resolution must also be in system_checks
  (fix): Providers without resolution must also be mappable to _sampling_interval_padding
  ...
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.

1 participant