Skip to content
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

Clickable Comparsion Charts + Display Missing Values #1017

Merged
merged 12 commits into from
Dec 16, 2024

Conversation

ArneTR
Copy link
Member

@ArneTR ArneTR commented Dec 15, 2024

This PR:

  • Introduces new QoL feature to see in comparsion view which bar belongs to which run (See Screenshot)
  • Refactors how phase display works. Before ALL phases and charts where rendered. In practice however in 90% of the cases only the [RUNTIME] phase is really looked at. Which means GMT is doing 90% waste compute for every frontend display. In larger runs this can take up to 10 seconds to render. This now renders phases on demand only
Screenshot 2024-12-15 at 2 02 33 AM

@ArneTR ArneTR changed the title WIP [skip ci] Clickable Comparsion Charts + Display Missing Values Dec 15, 2024
@ArneTR ArneTR marked this pull request as ready for review December 15, 2024 21:41
@ArneTR ArneTR force-pushed the detailed-comparison-charts-click-display branch from 8c6f842 to 0ed3ffd Compare December 15, 2024 21:41
Copy link

@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.

PR Summary

Added clickable comparison charts and optimized phase rendering to improve performance and user experience in the Green Metrics Tool.

  • Added clickable chart overlays in /frontend/css/green-coding.css to enable interactive comparison of runs through tooltips and diff functionality
  • Implemented lazy loading of phase data in /frontend/js/ci.js and /frontend/js/helpers/phase-stats.js to only render phases on demand, significantly reducing initial load time
  • Added select_diff_buffer in /frontend/js/helpers/charts.js to enable run comparison directly from chart interface
  • Enhanced chart visualization in /api/api_helpers.py to properly handle and display missing values with visual indicators
  • Centralized number formatting utilities in /frontend/js/helpers/main.js for consistent decimal display across the application

💡 (2/5) Greptile learns from your feedback when you react with 👍/👎!

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

frontend/css/green-coding.css Show resolved Hide resolved
api/api_helpers.py Outdated Show resolved Hide resolved
api/api_helpers.py Show resolved Hide resolved
api/api_helpers.py Show resolved Hide resolved
@@ -47,18 +48,18 @@ $(document).ready( (e) => {
params.forEach( id => {
api_url = `${api_url}${id}`
});
var phase_stats_data = (await makeAPICall(api_url)).data
window.phase_stats_data = (await makeAPICall(api_url)).data
Copy link

Choose a reason for hiding this comment

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

style: storing phase_stats_data on window makes it a global mutable state which could cause issues with multiple instances or race conditions

frontend/js/helpers/charts.js Show resolved Hide resolved
frontend/js/helpers/charts.js Outdated Show resolved Hide resolved
frontend/js/helpers/phase-stats.js Outdated Show resolved Hide resolved
frontend/js/helpers/phase-stats.js Show resolved Hide resolved
frontend/js/stats.js Show resolved Hide resolved
@ArneTR
Copy link
Member Author

ArneTR commented Dec 15, 2024

Charts now also have missing values display:
Screenshot 2024-12-15 at 10 20 37 PM
Screenshot 2024-12-15 at 10 21 03 PM

Copy link

@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.

PR Summary

This PR continues to enhance the comparison functionality and phase rendering in the Green Metrics Tool, focusing on data handling and visualization improvements.

  • Added comparison_details and comparison_identifiers in api_helpers.py to maintain consistent data ordering and track run metadata
  • Implemented caching mechanism via window.__rendered_phases in phase-stats.js to prevent redundant phase re-rendering
  • Added dynamic standard deviation calculations for data zoom functionality in timeline.js
  • Enhanced error handling for empty data cases and statistical calculations across comparison features

The changes build upon the previous improvements while adding more robust data management and visualization capabilities.

12 file(s) reviewed, 8 comment(s)
Edit PR Review Bot Settings | Greptile

api_test.py Show resolved Hide resolved
api_test.py Show resolved Hide resolved
api_test.py Show resolved Hide resolved
api_test.py Show resolved Hide resolved
api/api_helpers.py Show resolved Hide resolved
frontend/js/compare.js Show resolved Hide resolved
frontend/js/helpers/phase-stats.js Outdated Show resolved Hide resolved
frontend/js/helpers/phase-stats.js Outdated Show resolved Hide resolved
Copy link

@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.

PR Summary

(updates since last review)

This PR continues to enhance the comparison and visualization features of the Green Metrics Tool, focusing on user interaction and performance optimizations.

  • Added tooltip functionality in /frontend/js/helpers/charts.js to display detailed run metadata when clicking chart bars, including commit hashes and timestamps
  • Optimized phase rendering in /frontend/js/helpers/phase-stats.js by adding URL hash parameter support for direct phase navigation
  • Enhanced chart data handling in /api/api_helpers.py to properly transform and structure comparison data using OrderedDict for consistent display
  • Improved error messaging for missing metrics with specific phase and detail name identification

The changes improve user experience while maintaining code quality and performance.

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

frontend/js/helpers/phase-stats.js Outdated Show resolved Hide resolved
Copy link

@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.

PR Summary

(updates since last review)

This PR continues to enhance the comparison charts and phase rendering functionality in the Green Metrics Tool. Here are the key changes since the last review:

  • Added addToDiffSelection() in frontend/js/helpers/charts.js to enable direct comparison between runs via chart tooltips
  • Implemented getCompareChartOptions() with missing data handling and visual indicators for incomplete metrics
  • Added CSS pointer-events support in frontend/css/green-coding.css to ensure chart overlays remain clickable
  • Enhanced chart tooltip formatting with detailed run metadata including commit hashes and timestamps

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

Copy link

@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.

PR Summary

(updates since last review)

This PR continues to enhance the comparison functionality in the Green Metrics Tool, focusing on data handling and visualization improvements.

  • Added transform_dict_to_list_two_level() in api_helpers.py to convert nested comparison data into a more efficient list format for chart rendering
  • Enhanced get_phase_stats_object() to pre-fill arrays with None values for missing metrics, ensuring consistent data visualization
  • Added html_escape_multi() to safely escape HTML in nested data structures including BaseModel objects
  • Improved error handling in get_comparison_details() by removing redundant data validation check

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

api/api_helpers.py Show resolved Hide resolved
api/api_helpers.py Show resolved Hide resolved
api/api_helpers.py Show resolved Hide resolved
Copy link

@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.

PR Summary

(updates since last review)

Based on the recent changes to the codebase, here's a concise summary of the latest modifications:

Enhanced chart interaction and phase rendering optimization with improved data handling for missing values and comparison functionality.

  • Added tooltip click handling in frontend/js/helpers/charts.js to prevent triggering on markLine/markArea components
  • Implemented phase caching via window.__rendered_phases in frontend/js/helpers/phase-stats.js to prevent redundant rendering
  • Added visual indicators for missing data points in comparison charts with mark_point configuration
  • Enhanced chart container CSS in frontend/css/green-coding.css to ensure clickable overlays work properly

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

frontend/js/helpers/charts.js Show resolved Hide resolved
@ArneTR ArneTR force-pushed the detailed-comparison-charts-click-display branch from 04cc436 to 551d4cc Compare December 16, 2024 19:03
Copy link

@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.

PR Summary

(updates since last review)

Based on the recent changes, here's a concise summary of the latest updates:

Enhanced comparison chart functionality and phase rendering with improved data validation and error handling.

  • Added showWarning() in phase-stats.js to display specific warnings when metrics are missing from comparisons
  • Improved phase tab creation with createPhaseTab() to handle runtime sub-phases correctly
  • Enhanced getPhaseStats() in api_helpers.py to maintain consistent ordering with comparison details
  • Added validation in get_phase_stats_object() to ensure unit consistency across metrics

The changes focus on improving data validation and error reporting while maintaining the core comparison functionality.

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

frontend/js/helpers/phase-stats.js Show resolved Hide resolved
Copy link

@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.

PR Summary

(updates since last review)

Based on the recent changes, here's a concise summary of the latest modifications:

Enhanced phase rendering and comparison chart functionality with improved error handling and data validation.

  • Added select_diff_buffer array in frontend/js/helpers/charts.js to store run IDs for comparison selection
  • Enhanced createTableHeader() in frontend/js/helpers/phase-stats.js to handle different comparison cases with appropriate column headers
  • Added unit validation in get_phase_stats_object() to raise RuntimeError when metrics have inconsistent units
  • Improved phase navigation with getAndShowPhase() to handle URL hash parameters for direct phase access

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

Copy link

@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.

PR Summary

(updates since last review)

Based on the recent changes, here's a concise summary of the latest modifications:

Enhanced phase rendering and comparison chart functionality with improved data validation and error handling.

  • Added validation in createTableHeader() to dynamically adjust columns based on comparison amounts and cases
  • Enhanced getAndShowPhase() with regex validation for sub-phase parameters to prevent XSS vulnerabilities
  • Improved error handling in showWarning() to properly map runtime phases to "All Flows" display
  • Added phase tab creation validation in createPhaseTab() to prevent duplicate tab creation

The changes focus on improving input validation and error handling while maintaining the core comparison functionality.

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

frontend/js/helpers/phase-stats.js Show resolved Hide resolved
Copy link

@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.

PR Summary

(updates since last review)

Based on the recent changes, here's a concise summary of the latest modifications:

Enhanced phase rendering and comparison chart functionality with improved data handling and security measures.

  • Added sub-phase regex validation in frontend/js/helpers/phase-stats.js using pattern /^[\.\s0-9a-zA-Z_\(\)-]+$/ to prevent XSS attacks
  • Improved phase mapping in showWarning() by converting [RUNTIME] to [[RUNTIME]] for consistent "All Flows" display
  • Enhanced chart tooltip handling in frontend/js/helpers/charts.js to prevent triggering on non-series components
  • Fixed phase tab selector in createPhaseTab() to properly handle runtime sub-phases and prevent duplicate creation

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

Copy link

@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.

PR Summary

(updates since last review)

Based on the recent changes, here's a concise summary of the latest modifications:

Enhanced phase rendering and comparison chart functionality with improved data validation and error handling.

  • Added sub_phase_regex pattern in getAndShowPhase() to validate phase parameters and prevent XSS attacks
  • Enhanced createTableHeader() to handle different comparison cases with dynamic column generation based on comparison amounts
  • Improved phase tab creation in createPhaseTab() with proper runtime sub-phase handling and duplicate prevention
  • Added missing value indicators in chart display with mark_point configuration for better data visualization

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

frontend/js/helpers/phase-stats.js Show resolved Hide resolved
@ArneTR ArneTR merged commit ef0f88e into main Dec 16, 2024
@ArneTR ArneTR deleted the detailed-comparison-charts-click-display branch December 16, 2024 19:30
ArneTR added a commit that referenced this pull request Dec 17, 2024
* main:
  Clickable Comparsion Charts + Display Missing Values (#1017)
  Bump uvicorn[standard] from 0.32.1 to 0.34.0 (#1019)
  Bump playwright/python in /docker/auxiliary-containers/gcb_playwright (#1018)
ArneTR added a commit that referenced this pull request Dec 19, 2024
* main: (46 commits)
  Updated cloud energy
  (fix): Unselect button broke repositories view
  Noise reduced run start (#1023)
  (improvement): More strict check
  Added unselect button [skip ci] (#1022)
  (fix): tests
  Clickable Comparsion Charts + Display Missing Values (#1017)
  Bump uvicorn[standard] from 0.32.1 to 0.34.0 (#1019)
  Bump playwright/python in /docker/auxiliary-containers/gcb_playwright (#1018)
  Updated EE
  Adding not implemented error
  Kill script for GMT must use full commandline when killing reporters
  +x for tcpdump
  Tcp dump (#919)
  Hash must be decoded to understand spaces [skip ci]
  Allowing Deeplinks to specific phases [skip ci] (#1016)
  Bump python from 3.13.0-slim-bookworm to 3.13.1-slim-bookworm in /docker (#1015)
  Bump pydantic from 2.10.2 to 2.10.3 (#1010)
  Bump fastapi[standard] from 0.115.5 to 0.115.6 (#1011)
  Bump aiohttp from 3.11.9 to 3.11.10 (#1013)
  ...
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