Skip to content

Conversation

@ParticularlyPythonicBS
Copy link
Member

@ParticularlyPythonicBS ParticularlyPythonicBS commented Jan 21, 2026

Added slider for label font size to the visualizer, added tests for special items in legends, updated visualizer embed in the docs

Summary by CodeRabbit

  • New Features

    • Added a Font Size slider (6–100) in Visual Settings for label sizing.
  • Improvements

    • Node placement now auto-scales with graph size for more appropriate layouts.
    • Updated default edge styling and label font defaults for clearer visuals.
    • Initial graph positions and sizes refreshed while preserving legends and view configurations.
  • Tests

    • Added unit tests covering graph styling and edge attributes.

✏️ Tip: You can customize this high-level summary in your review settings.

@ParticularlyPythonicBS ParticularlyPythonicBS added Maintenance Code quality fixes and deprecation management Feature Additional functionality bugfix labels Jan 21, 2026
@coderabbitai
Copy link

coderabbitai bot commented Jan 21, 2026

Walkthrough

Dynamic layout scaling for network graphs, a new Font Size control in the visualization UI, per-node font-size wiring in the client script, edge thickness switched to width-based values, and a new unit test for commodity graph styling.

Changes

Cohort / File(s) Summary
Static visualization & HTML
docs/source/default/static/Network_Graph_utopia_1990.html, temoa/utilities/network_vis_templates/graph_template.html
Added a Visual Settings group with a Label Font Size slider (range 6–100, default 14); embedded graph data payload reordered (primary/secondary node & edge blocks swapped) in the static HTML.
Client visualization script
temoa/utilities/network_vis_templates/graph_script.js
Added visualState and font-size slider wiring; introduced addWithCurrentFontSize and updateVisualSettings to apply per-node font sizes; replaced direct dataset adds with the helper; conditionally exposes window.__graph for non-production.
Client styling
temoa/utilities/network_vis_templates/graph_styles.css
Added CSS for a control-group layout (horizontal flex, gap, label sizing) and styling for advanced-controls toggle.
Layout / positioning logic
temoa/utilities/graph_utils.py
Replaced fixed layout_radius/jitter_radius with dynamic scaling based on sector and node/edge counts; computes layout_radius within larger ranges and adjusts jitter accordingly.
Visualizer defaults / backend
temoa/utilities/visualizer.py
Switched edge attribute from a value-based thickness to width = 2 + number_of_techs; added edge font settings in DEFAULT_VIS_OPTIONS and simplified layout options (removed randomSeed).
Tests
tests/test_commodity_visualizer.py
New unit test validating commodity graph node borders, titles, and edge styling (dashes/colors) for constructed NetworkModelData/EdgeTuple inputs.

Sequence Diagram(s)

(Skipped — changes are dispersed across UI, layout logic, styling, and tests but do not introduce a single new multi-component sequential flow requiring diagramming.)

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 45.45% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the primary change: adding a font size slider control to the visualizer UI, which is reflected in modifications across graph_script.js, graph_template.html, graph_styles.css, and visualizer.py.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Fix all issues with AI agents
In `@temoa/utilities/graph_utils.py`:
- Around line 234-241: The iterator all_edges is being consumed by num_edges =
sum(1 for _ in all_edges) causing the later loop for edge_tuple in all_edges to
see no items; instead materialize the iterable at the start (e.g., convert
all_edges to a list) before counting so num_edges and the subsequent loop both
operate on the same concrete sequence—update references around num_edges,
layout_radius, jitter_radius, and the for edge_tuple in all_edges loop to use
the materialized collection.

In `@temoa/utilities/network_vis_templates/graph_script.js`:
- Around line 35-42: The code attaches internal state to a global by assigning
window.__graph = { data, allNodesPrimary, allEdgesPrimary, allNodesSecondary,
allEdgesSecondary, optionsObject };; either remove this assignment before
production or gate it behind a clear flag/environment check and add a short
comment describing its debugging purpose. Update the block around the
window.__graph assignment (the symbol window.__graph and the variables data,
allNodesPrimary, allEdgesPrimary, allNodesSecondary, allEdgesSecondary,
optionsObject) to only export when a DEBUG/DEV_MODE flag is true or when
process.env.NODE_ENV !== 'production', and include a one-line comment stating
“Expose for debugging only — disable in production.”

In `@temoa/utilities/network_vis_templates/graph_template.html`:
- Around line 20-26: Summary: tighten the font-size slider range to a more
practical maximum. Update the input element with id "font-size-slider" to reduce
the max from 100 to 30 (and optionally add step="1" for finer control) while
keeping value="14" to match DEFAULT_VIS_OPTIONS; ensure any JS that reads the
slider (e.g., handlers referencing "font-size-slider") still accepts the new
range.

In `@tests/test_commodity_visualizer.py`:
- Around line 16-23: Replace the MagicMock usage in
tests/test_commodity_visualizer.py with a concrete NetworkModelData instance so
attribute lookups behave like the real class; instantiate NetworkModelData(),
then set network_data.physical_commodities = {'comm_inter'}, assign
network_data.source_commodities[(region, period)] = {'comm_source'} and
network_data.demand_commodities[(region, period)] = {'comm_demand'} and ensure
network_data.available_techs and network_data.tech_data use the class defaults
(or populate them minimally) before passing network_data into the
CommodityVisualizer test.

Comment on lines 20 to 26
<div class="legend-section">
<h4>Visual Settings</h4>
<div class="control-group">
<label for="font-size-slider">Label Font Size</label>
<input type="range" id="font-size-slider" min="6" max="100" value="14">
</div>
</div>
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

LGTM! Font size slider integration.

The slider is well-integrated with proper labeling and accessibility attributes. The default value of 14 correctly matches DEFAULT_VIS_OPTIONS.

One consideration: a max of 100 is quite large for label fonts. If most users will stay within 6-30, you could consider tightening the range for finer control, but the current range does allow for extreme zoom-out scenarios where larger labels might be useful.

🤖 Prompt for AI Agents
In `@temoa/utilities/network_vis_templates/graph_template.html` around lines 20 -
26, Summary: tighten the font-size slider range to a more practical maximum.
Update the input element with id "font-size-slider" to reduce the max from 100
to 30 (and optionally add step="1" for finer control) while keeping value="14"
to match DEFAULT_VIS_OPTIONS; ensure any JS that reads the slider (e.g.,
handlers referencing "font-size-slider") still accepts the new range.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@temoa/utilities/network_vis_templates/graph_script.js`:
- Around line 50-52: visualState.fontSize is hardcoded to 14 while optionsObject
(from options_json_str) may specify a different font size, causing a UI/render
mismatch; update the initialization so visualState.fontSize =
(optionsObject?.font_size ?? 14) and set the slider's initial value to that same
value during the UI setup (use the same slider variable or DOM element used
where the slider is created) so both the rendered network and the displayed
slider are synchronized; locate references to visualState, optionsObject, and
the slider initialization code (where the slider value is set) and replace the
hardcoded 14 with the derived value and apply it to the slider control.

Comment on lines +50 to +52
let visualState = {
fontSize: 14
};
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find and read the graph_script.js file
find . -name "graph_script.js" -type f

Repository: TemoaProject/temoa

Length of output: 162


🏁 Script executed:

#!/bin/bash
# Read the graph_script.js file to understand the full context
wc -l ./temoa/utilities/network_vis_templates/graph_script.js

Repository: TemoaProject/temoa

Length of output: 121


🏁 Script executed:

#!/bin/bash
# Read the relevant sections of the file
cat -n ./temoa/utilities/network_vis_templates/graph_script.js | head -120 | tail -80

Repository: TemoaProject/temoa

Length of output: 3994


🏁 Script executed:

#!/bin/bash
# Read the beginning of the file to understand how optionsObject is created
cat -n ./temoa/utilities/network_vis_templates/graph_script.js | head -50

Repository: TemoaProject/temoa

Length of output: 2122


🏁 Script executed:

#!/bin/bash
# Search for font size references and how optionsObject is used
rg -n "fontSize|font.*size" ./temoa/utilities/network_vis_templates/graph_script.js

Repository: TemoaProject/temoa

Length of output: 709


🏁 Script executed:

#!/bin/bash
# Search for optionsObject usage and structure to see if it contains font settings
rg -n "optionsObject" ./temoa/utilities/network_vis_templates/graph_script.js | head -20

Repository: TemoaProject/temoa

Length of output: 350


🏁 Script executed:

#!/bin/bash
# Look for where optionsRaw/options_json_str is generated, likely in a Python backend
find . -name "*.py" -type f | xargs rg -l "options_json_str|network_vis" | head -10

Repository: TemoaProject/temoa

Length of output: 93


🏁 Script executed:

#!/bin/bash
# Read the visualizer.py file to understand how options_json_str is generated
wc -l ./temoa/utilities/visualizer.py

Repository: TemoaProject/temoa

Length of output: 97


🏁 Script executed:

#!/bin/bash
# Read the visualizer.py file to find options_json_str generation
rg -n "options_json_str|nodes.*font|edges.*font" ./temoa/utilities/visualizer.py

Repository: TemoaProject/temoa

Length of output: 110


🏁 Script executed:

#!/bin/bash
# Read the visualizer.py file around line 242 and look for options construction
cat -n ./temoa/utilities/visualizer.py | sed -n '200,280p'

Repository: TemoaProject/temoa

Length of output: 3792


🏁 Script executed:

#!/bin/bash
# Look at the DEFAULT_VIS_OPTIONS to understand the structure
cat -n ./temoa/utilities/visualizer.py | sed -n '272,315p'

Repository: TemoaProject/temoa

Length of output: 1672


🏁 Script executed:

#!/bin/bash
# Check if the slider has a default value in the HTML template
rg -n "font-size-slider" ./temoa/utilities/network_vis_templates/ -A 2 -B 2

Repository: TemoaProject/temoa

Length of output: 1411


🏁 Script executed:

#!/bin/bash
# Verify scope - check that optionsObject is defined before line 50
cat -n ./temoa/utilities/network_vis_templates/graph_script.js | sed -n '30,55p'

Repository: TemoaProject/temoa

Length of output: 1021


Synchronize slider initial value with backend font size defaults to prevent UI/render mismatch.

visualState.fontSize and the slider are hardcoded to 14, but optionsObject (derived from backend options_json_str) may specify a different font size. When the network initializes, it uses the size from optionsObject, creating a mismatch where the actual render differs from the displayed slider value. Initialize both from optionsObject with a fallback to 14, and set the slider's initial value accordingly.

🛠️ Suggested fix
-    let visualState = {
-        fontSize: 14
-    };
+    const parsedFontSize = parseInt(
+        optionsObject?.nodes?.font?.size ?? optionsObject?.edges?.font?.size ?? 14,
+        10,
+    );
+    const defaultFontSize = Number.isFinite(parsedFontSize) ? parsedFontSize : 14;
+    let visualState = {
+        fontSize: defaultFontSize
+    };

And at line 60:

     const fontSizeSlider = document.getElementById('font-size-slider');
+    if (fontSizeSlider) fontSizeSlider.value = String(defaultFontSize);
🤖 Prompt for AI Agents
In `@temoa/utilities/network_vis_templates/graph_script.js` around lines 50 - 52,
visualState.fontSize is hardcoded to 14 while optionsObject (from
options_json_str) may specify a different font size, causing a UI/render
mismatch; update the initialization so visualState.fontSize =
(optionsObject?.font_size ?? 14) and set the slider's initial value to that same
value during the UI setup (use the same slider variable or DOM element used
where the slider is created) so both the rendered network and the displayed
slider are synchronized; locate references to visualState, optionsObject, and
the slider initialization code (where the slider value is set) and replace the
hardcoded 14 with the derived value and apply it to the slider control.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Feature Additional functionality Maintenance Code quality fixes and deprecation management

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants