Skip to content

Add activity-generated air speed toggle and fix adaptive comfort chart boundaries#93

Open
t-kramer wants to merge 2 commits into
developmentfrom
v-relative-fix
Open

Add activity-generated air speed toggle and fix adaptive comfort chart boundaries#93
t-kramer wants to merge 2 commits into
developmentfrom
v-relative-fix

Conversation

@t-kramer

@t-kramer t-kramer commented May 6, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR introduces a user-facing toggle to control whether activity-generated (relative) air
speed is included in PMV calculations.


Relative air speed toggle

ASHRAE 55 requires adding a self-generated air speed component (v + 0.3 × (met − 1) when met > 1
met) to the measured air speed. This is appropriate for occupants walking, but not for people at
elevated metabolic rates without generating air movement (e.g. exercising on stationary
equipment).

A new "Relative air speed" button has been added to the button group at the bottom of the input
panel on the ASHRAE, EN 16798, Compare, and Ranges tabs (Fans & Heat and PHS are unaffected as
they do not use relative air speed). Clicking the button opens a modal dialog explaining the
feature, with a checkbox to enable or disable it.

When enabled (default), the tool adds the activity-generated component as before. When disabled,
only the measured air speed is used. Toggling the checkbox immediately recalculates and redraws
all outputs.

A display note — "Relative air speed = X m/s" — appears near the chart when relative air speed is
actively applied (met > 1 and toggle enabled), matching the existing dynamic clothing display on
ASHRAE/EN.

The underlying flag (comf.useSelfGeneratedAirSpeed) is added to comfort-models.js alongside the
existing comf.relativeAirSpeed() helper.

Summary by CodeRabbit

Release Notes

  • New Features
    • Added a configurable "Relative air speed" setting across all comfort analysis tools.
    • Users can toggle whether activity-generated air speed is included via a new checkbox; relative air speed displays when metabolic rate exceeds 1 met.
    • Includes an explanatory dialog describing how activity-generated air speed is calculated.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 6, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: eb0751e7-535a-4564-bec4-b7de8c919fe3

📥 Commits

Reviewing files that changed from the base of the PR and between 6d9a880 and c7eafd0.

📒 Files selected for processing (9)
  • static/js/ASHRAE/ashrae.js
  • static/js/EN/en.js
  • static/js/comfort-models.js
  • static/js/compare/compare.js
  • static/js/ranges/ranges.js
  • templates/ashrae.html
  • templates/compare.html
  • templates/en.html
  • templates/ranges.html

📝 Walkthrough

Walkthrough

This PR adds a feature flag to conditionally enable or disable activity-generated air speed in comfort calculations. A checkbox on each page (ASHRAE, EN, Compare, Ranges) allows users to toggle comf.useSelfGeneratedAirSpeed. The core relativeAirSpeed() function now only applies the air-speed boost when the flag is enabled and metabolic rate exceeds 1 met; otherwise it returns the original value. Each page initializes the flag from its checkbox state, binds a change listener to update the flag and refresh display, and gates its relative air speed rendering accordingly.

Changes

Activity-generated air speed feature

Layer / File(s) Summary
Feature flag and core logic
static/js/comfort-models.js
Core flag comf.useSelfGeneratedAirSpeed (default true) and relativeAirSpeed() function conditional on both flag and met > 1.
ASHRAE page air speed control
static/js/ASHRAE/ashrae.js, templates/ashrae.html
Checkbox reference, dialog wiring, change listener updating flag and calling update(), and conditional relative air speed display gated by flag and met > 1.
EN page air speed control
static/js/EN/en.js, templates/en.html
Checkbox reference, dialog modal with open/close handlers, change listener updating flag and calling update(), and conditional relative air speed display gated by flag and met > 1.
Compare page air speed control with multi-input row management
static/js/compare/compare.js, templates/compare.html
Checkbox initialization, dialog wiring, updateVrRowVisibility() helper to manage shared output row visibility across three inputs, extended update(i) to compute or clear per-input relative air speed values and refresh row visibility.
Ranges page air speed control
static/js/ranges/ranges.js, templates/ranges.html
Checkbox initialization, dialog modal, change listener that updates flag and redraws active range before calling update(), and conditional relative air speed display with unit adjustment gated by flag and met > 1.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

enhancement

Suggested reviewers

  • FedericoTartarini

Poem

🐰 A swift toggle hops across the pages,
Air speed bending to the user's will,
Four comfort zones now wiser and more gentle,
Met by met, the feature flows with grace. ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The PR title partially aligns with the main changeset—it mentions the activity-generated air speed toggle, which is the primary user-facing feature across multiple files, but it also references 'adaptive comfort chart boundaries' which does not appear to be implemented in the provided file changes. Clarify whether the adaptive comfort chart boundaries fix is included in this PR or if the title should focus solely on the activity-generated air speed toggle feature.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch v-relative-fix

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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.

@t-kramer t-kramer requested a review from FedericoTartarini May 6, 2026 17:37
@t-kramer

t-kramer commented May 6, 2026

Copy link
Copy Markdown
Contributor Author

@FedericoTartarini Please review and merge into master if approved.

@FedericoTartarini

Copy link
Copy Markdown
Collaborator

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 26, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@FedericoTartarini

Copy link
Copy Markdown
Collaborator

@t-kramer I am struggling a bit to find the time to review all these pool requests. Shall I trust your work and we just can see what Code Rabbit says. If CodeRabbit doesn't have any major problem we can accept this pull request, please address the other issues it may find

@t-kramer

Copy link
Copy Markdown
Contributor Author

@FedericoTartarini Yes, I think we'll be fine. I didn't make any huge changes. Let's see what the review says and I'll make sure to implement any major comments.

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.

2 participants