web: hierarchy browser in static reports, search race fix, module coloring fixes#10288
Merged
maliberty merged 6 commits intoApr 28, 2026
Conversation
Six search methods (searchBoxShapes, searchSNetViaShapes, searchSNetShapes, searchBlockages, searchObstructions, searchRows) returned lazy R-tree iterators without holding a lock. When the background eagerInit thread was concurrently building R-trees, a tile request could iterate a partially-constructed or destroyed R-tree, causing a segfault. Fix: hold a shared_lock on the init mutex and eagerly materialize results into a vector before returning, matching the existing pattern used by searchInsts and searchFills. Signed-off-by: Matt Liberty <mliberty@eng.ucsd.edu> Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
Add module hierarchy data and colored _modules tiles to the static HTML report so the hierarchy browser works in offline mode. Server-side changes: - New module_color_palette.h: shared 31-color palette (single source of truth, previously duplicated in JS and Qt GUI). - HierarchyReport::getReport() assigns palette colors to MODULE nodes. - New serializeHierarchyResult() and computeDefaultModuleColors() shared helpers in hierarchy_report.cpp, used by both the WebSocket handler and saveReport(). - saveReport() embeds module_hierarchy JSON in the static cache and pre-renders _modules layer tiles with default module colors. Client-side changes: - hierarchy-browser.js reads server-assigned colors instead of maintaining a local palette copy. Removes MODULE_COLORS constant and _assignColors(); adds _readServerColors(). - Auto-loads hierarchy data in static mode. - websocket-manager.js: set_module_colors is a no-op in static mode (tiles are pre-rendered); module_hierarchy served from JSON cache. Signed-off-by: Matt Liberty <mliberty@eng.ucsd.edu> Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
…cells Two fixes to the _modules layer coloring: - Always include expanded modules in the color map. Previously, expanded modules with child modules were excluded, causing their direct leaf instances to lose color. The tile renderer looks up each instance's direct module, so parent and child colors never conflict. - Skip fill cells (CORE_SPACER) in the _modules layer rendering. These are physical-only cells that add visual noise to the module color overlay. Signed-off-by: Matt Liberty <mliberty@eng.ucsd.edu> Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
Use designated initializers for Color in hierarchy_report.h, and NOLINT the palette data table in module_color_palette.h where designated initializers would obscure the color data. Signed-off-by: Matt Liberty <mliberty@eng.ucsd.edu> Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces module hierarchy reporting and visualization support, including a color palette for modules, server-side serialization, and integration into the static report generation. Additionally, it improves thread safety in the search module by replacing lazy R-tree iterators with eager collection under shared locks. The review comments suggest refactoring the repetitive mutex locking and R-tree access logic into helper functions to improve maintainability and reduce code duplication.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Member
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
maliberty
approved these changes
Apr 28, 2026
maliberty
merged commit bf0b749
into
The-OpenROAD-Project:master
15 of 16 checks passed
oharboe
added a commit
to oharboe/OpenROAD
that referenced
this pull request
May 6, 2026
…message 1. Bazel test detail (The-OpenROAD-Project#7). The actual stderr (e.g. `AssertionError: 20 != 19`) lives in a Bazel `==================== Test output for <target>:` block — usually thousands of lines *before* the FAILED summary row in the same log. Restructure the parser to: - record FAILED summary rows into a per-target dict (collapses re-runs from `--keep_going`); - buffer Test-output block lines unconditionally as we see them, so blocks that precede their FAILED row aren't dropped; - use a length-78+ `=` rule as the block end (Bazel's closing fence is 80 wide; the unittest framework's internal 70-wide separator no longer terminates capture early). PR The-OpenROAD-Project#10287 dogfood: `//src/odb/test:odb_man_tcl_check` finding now carries `AssertionError: 20 != 19 : ./src/odb: help count (20) != readme count (19)` in its detail / AI directive. 2. Per-finding log URL (The-OpenROAD-Project#11). cli._scan_check threads `check.details_url` into every emitted finding; render_markdown appends "Full log: <url>." to each item in the AI directive. 3. "Couldn't extract" message (The-OpenROAD-Project#10). When the discovery layer reports N failing checks but no parser produced a finding, the table now says "Found N failing check(s) but couldn't extract any actionable findings — see <urls>" instead of an empty screen. `discover_findings` returns `(findings, failing_check_urls)` so the renderer can show the URL list. fix_main.py + 4 unit tests updated for the new tuple shape. Tests added/updated: - test_bazel_test: 3 new cases covering block-before-summary, block-truncation, no-block-fallback, plus the existing assertion cases. - test_render_table: empty-with-failing-urls case. - test_cli, test_fix_runs_recipes: updated stubs for tuple return. 30/30 Bazel tests pass. Final 10-PR dogfood survey (the same set used to motivate P0): - All 10 PRs produce useful output. - PR The-OpenROAD-Project#10287's bazel_test_fail finding now contains the actual `AssertionError`. - PR The-OpenROAD-Project#10288's purged-build case shows a `log_unavailable` info finding alongside the 5 reviewable Gemini comments on its diff. Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
This was referenced May 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
-webstartupweb_save_report)Type of Change
Impact
Search race fix: Six search methods returned lazy R-tree iterators without holding a lock. When the background
eagerInitthread was concurrently building R-trees, tile requests could iterate partially-constructed trees, causing a segfault. Now all search methods eagerly materialize results under ashared_lock, matching the existing pattern used bysearchInstsandsearchFills.Static hierarchy browser: The static HTML report now embeds module hierarchy data and pre-rendered
_moduleslayer tiles. The hierarchy browser auto-loads in static mode. The 31-color palette is consolidated into a singlemodule_color_palette.h(previously duplicated in JS and C++). The server assigns colors inHierarchyReport::getReport()and the JS client reads them from the response.Module coloring fixes: Expanded modules with child modules are no longer excluded from the color map — their direct leaf instances retain color since the tile renderer looks up each instance's direct module. Fill cells (
CORE_SPACER) are skipped in the_moduleslayer to reduce visual noise.Verification
bazel test //src/web/test/...— 23/23 pass).