Skip to content

refactor(ui): resolve var declarations and reduce cognitive complexity in pipeline.js - #173

Open
bhagathkrishnacdac wants to merge 5 commits into
omec-project:mainfrom
bhagathkrishnacdac:bess-code-quality
Open

refactor(ui): resolve var declarations and reduce cognitive complexity in pipeline.js#173
bhagathkrishnacdac wants to merge 5 commits into
omec-project:mainfrom
bhagathkrishnacdac:bess-code-quality

Conversation

@bhagathkrishnacdac

Copy link
Copy Markdown

Overview
This PR improves the code quality of the pipeline visualization script (bessctl/static/pipeline.js) by modernizing variable declarations and reducing function complexity.

Key Changes

  • Modernized Variables: Replaced legacy var declarations with block-scoped const and let keywords.
  • Resolved Cognitive Complexity in get_edge_label: Factored out label calculation, scaling, and HTML formatting into specialized helper functions.
  • Resolved Cognitive Complexity in graph_to_dot: Refactored DOM option gathering and Graphviz DOT-notation compilation into modular rendering helpers.
  • Removed Global State Reliance: Passed the UI option states down explicitly through parameters rather than relying on global variables.

No modifications were made to the visual layout generation, data collection, or UI-bound operations. The generated Graphviz DOT files and label rounding logic match the original behavior.

…y in pipeline.js

Signed-off-by: bhagathkrishnacdac <bhagath.krishna@cdac.in>
@bhagathkrishnacdac
bhagathkrishnacdac requested a review from a team July 26, 2026 14:09
Comment thread bessctl/static/pipeline.js Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the BESS pipeline visualization script (bessctl/static/pipeline.js) to modernize variable declarations and decompose get_edge_label() and graph_to_dot() into smaller helper functions, with the stated goal of preserving existing rendering/label behavior while reducing complexity.

Changes:

  • Replaced legacy var declarations with const/let.
  • Split edge-label computation/formatting into helper functions (calculate_*, format_*).
  • Refactored DOT generation into modular helpers for option gathering, node generation, and edge generation.
Comments suppressed due to low confidence (1)

bessctl/static/pipeline.js:203

  • create_module_node() takes an options parameter but never uses it, and also forwards options into gates_to_str() even though gates_to_str only accepts (gates, gate_type). This makes the data flow harder to follow and can trigger unused-parameter linting. Remove the unused parameter and stop passing it through (and consider also dropping the extra argument at the call site).
function create_module_node(module, module_name, options) {  
    const desc = module.desc ? `<font point-size="9">${module.desc}</font>` : '';  
    // Original gates_to_str might need options
    const igates = module.show_igates ? gates_to_str(module.igates, 'igate', options) : '';  
    const ogates = module.show_ogates ? gates_to_str(module.ogates, 'ogate', options) : '';  

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread bessctl/static/pipeline.js Outdated
Comment thread bessctl/static/pipeline.js Outdated
Comment thread bessctl/static/pipeline.js Outdated
bhagathkrishnacdac and others added 3 commits July 27, 2026 11:44
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: bhagathkrishnacdac <bhagath.krishna@cdac.in>
Signed-off-by: bhagathkrishnacdac <bhagath.krishna@cdac.in>
Signed-off-by: bhagathkrishnacdac <bhagath.krishna@cdac.in>

@gab-arrobo gab-arrobo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see lots of warnings when applying patch using git. Please address these issues

$ git apply pr.patch
pr.patch:101: trailing whitespace.
function get_edge_label(stats, options) {
pr.patch:102: trailing whitespace.
    if (!stats || stats.length === 0) {
pr.patch:103: trailing whitespace.
        return format_html_label('?');
pr.patch:104: trailing whitespace.
    }
pr.patch:105: trailing whitespace.

warning: squelched 175 whitespace errors
warning: 180 lines add whitespace errors.


return ` "${module_name}":${out_port} -> "${gate.name}":${in_port}${label}\n`;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove extra empty line

Suggested change

@andybavier

Copy link
Copy Markdown
Contributor

I've added a "whitespace" CI job that flags whitespace-related formatting issues. To fix the issues with this PR, run "pre-commit run --all-files" locally.

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.

4 participants