fix(plan): time share by approval mode dashboard reporting negative time shares#19847
fix(plan): time share by approval mode dashboard reporting negative time shares#19847
Conversation
Summary of ChangesHello @Adib234, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a bug causing negative telemetry durations in approval mode dashboard reporting. It achieves this by standardizing the use of Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request effectively resolves a bug that caused negative durations in telemetry by standardizing on performance.now() for all duration calculations related to approval mode switches. This ensures the use of a monotonic clock, preventing issues from system time adjustments. The refactoring to centralize the timestamp update logic within logCurrentModeDuration is a good improvement for maintainability and correctness, particularly for handling shutdown sequences. The addition of a guard against non-positive durations is a solid defensive programming practice. The changes are well-implemented and I approve them.
|
Size Change: +39 B (0%) Total Size: 25.2 MB ℹ️ View Unchanged
|
Summary
This PR fixes negative telemetry durations by switching to a consistent use of
performance.now()for tracking mode switches. It resolves a bug wherelastModeSwitchTimewas initialized with the system clock (Date.now()) but measured against the process timer (performance.now()).Details
lastModeSwitchTimeto useperformance.now(). This ensures that all duration calculations use the same monotonic time source, avoiding the massive offset between Unix epoch time and process uptime.logCurrentModeDurationto refreshlastModeSwitchTimeimmediately after logging. This prevents "double-counting" time during multiple rapid mode switches or shutdown sequences.Related Issues
Fixes #19846
How to Validate
Negative durations should never be logged going forward.
Pre-Merge Checklist