[codex] Complete milestone 7 analysis workspace polish#8
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9469d95cc2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Warning Gemini is experiencing higher than usual traffic and was unable to create the review. Please try again in a few hours by commenting |
|
/gemini review |
|
@codex review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces significant enhancements to the graphing and analysis capabilities of the application. Key changes include the implementation of a lap overlay system for comparing data across sessions, support for distance-based X-axis plotting, and the addition of embedded gauges within graph panels. The update also improves the UI for channel readouts and adds configuration support for these new features in workspace files. I have no feedback to provide as all changes appear to be well-implemented and align with the project's goals.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a9361aeeb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| match self.x_axis_mode { | ||
| GraphXAxis::Time => Some(OverlayAxisHandle { | ||
| axis: ActiveGraphXAxis::Time, | ||
| session_duration: session.ld_file.duration_secs(), | ||
| }), |
There was a problem hiding this comment.
When the user selects Distance X-axis but the main session cannot derive distance, resolve_x_axis falls back to time; however this method still branches on self.x_axis_mode for external overlays. In that fallback case external overlays are generated in distance space (or skipped) while the reference viewport is in time space, which makes overlays disappear or render misaligned. This should use the resolved active axis (x_axis) so all series share the same domain.
Useful? React with 👍 / 👎.
| .all(|g| g.channel.channel_id != id) | ||
| && let Some(pc) = self.plotted_channels.iter().find(|pc| pc.channel_id == id) | ||
| { | ||
| let style = default_style_for_name(""); |
There was a problem hiding this comment.
ContextAction::AddGauge always calls default_style_for_name(""), so gauges added from the graph context menu always default to Analog, even for steering channels. Other add paths infer style from the real channel name, so this introduces inconsistent behavior and loses the intended auto-selection for steering widgets.
Useful? React with 👍 / 👎.
1 participant
Summary
Why
This wraps up the remaining milestone 7 analysis workspace work and brings the default telemetry views much closer to the intended data-analysis workflow.
Validation
cargo test