web: filter timing paths by slack range on histogram click in static mode#10138
Conversation
…mode When a slack histogram column is clicked, the timing view should show only paths whose slack falls in that bin's range. In live WebSocket mode the server performs this filtering, but in static page mode the cache handler returned all paths regardless of slack_min/slack_max. Filter cached paths client-side in WebSocketManager._cacheRequest using the half-open [slack_min, slack_max) range matching the histogram's bin assignment. Tag each filtered path with _originalIndex so subsequent timing_highlight overlay lookups resolve to the correct pre-rendered image. Use _originalIndex in TimingWidget._selectPathRow when present. Fixes The-OpenROAD-Project#10020 Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
|
clang-tidy review says "All clean, LGTM! 👍" |
![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 implements slack range filtering for timing reports and introduces an _originalIndex property to maintain correct path highlighting when filters are applied. It also adds comprehensive unit tests for the TimingWidget and WebSocketManager. The review feedback identifies a missing update in _selectDetailRow that causes incorrect highlights when selecting pins in the detail table and suggests a more memory-efficient approach for filtering paths in the WebSocketManager to reduce garbage collection pressure.
Use _originalIndex in _selectDetailRow so pin overlay requests match the backend when paths are narrowed by histogram slack filter. Also fuse the filter map+filter into a single pass to avoid allocating a clone for every path in the unfiltered report. Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
|
@codex review |
|
clang-tidy review says "All clean, LGTM! 👍" |
![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: d2fee8e0fb
ℹ️ 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".
| && json.paths) { | ||
| const filtered = []; | ||
| json.paths.forEach((p, i) => { | ||
| if (p.slack >= msg.slack_min && p.slack < msg.slack_max) { |
There was a problem hiding this comment.
This new static-mode filter excludes p.slack === msg.slack_max unconditionally, but the histogram binning logic counts max-edge values in the last bin (it clamps computed bin indices to the final bin). As a result, clicking the last histogram bar can omit some paths that were counted in that bar, so the path table no longer matches the bar count and those max-edge paths cannot be selected/highlighted.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fair but just a limitation of having only the top N paths available.
maliberty
merged commit 2c25352
into
The-OpenROAD-Project:master
2 participants
Summary
In static-page mode, clicking a slack histogram column showed all timing paths instead of filtering to that bin's slack range. Live WebSocket mode filters server-side, but the static cache handler (
WebSocketManager._cacheRequest) ignored theslack_min/slack_maxparameters and returned the full cached report.This PR filters cached paths client-side using the half-open
[slack_min, slack_max)range matching the histogram's bin assignment. Each filtered path is tagged with an_originalIndexso subsequenttiming_highlightoverlay lookups still resolve to the correct pre-rendered image.Type of Change
Impact
Verification
bazel build //src/web/...).bazel test //src/web/test/...— 18/18 pass, including the newtiming_widget_testand expandedwebsocket_manager_test).Added tests
test-websocket-manager.js: 6 new tests covering passthrough,[min, max)range,_originalIndextagging, empty-range result, hold-side independence, and non-mutation of cached data.test-timing-widget.js(new file): 4 tests covering row-index fallback,_originalIndexuse,is_setuptab flag, and out-of-range guards.Related Issues
Fixes #10020