Skip to content

[BHP1-1660] Fix RoS and Flame Length Not being Enabled - #310

Merged
Kcheung42 merged 7 commits into
mainfrom
kc-BHP1-1660-fix-non-reactive-output-enabling
Oct 9, 2026
Merged

Kcheung42 merged 7 commits into
mainfrom
kc-BHP1-1660-fix-non-reactive-output-enabling

Conversation

@Kcheung42

Copy link
Copy Markdown
Collaborator

Purpose

Related Issues

Closes BHP1-1660

Submission Checklist

  • Included Jira issue in the PR title (e.g. BHP1-### <title>)
  • Code passes linter rules (clj-kondo --lint components/**/src bases/**/src projects/**/src)
  • Feature(s) work when compiled (clojure -M:compile-cljs)
  1. Create a Surface run with Fire Shape Diagram as an output, but without selecting ROS or FL
  2. Complete the run
  3. Go back and select ROS and FL
  4. Complete the second run
  5. Note how ROS and FL ARE included in the Results

Kcheung42 and others added 7 commits September 30, 2026 15:24
The subs that decide which outputs appear in the results read the enabled
outputs with a bare `d/q` on `@@s/conn`, which re-frame cannot see: a
DataScript transaction does not touch `app-db`, and `[:worksheet ws-uuid]`
is a `ReactiveEntity` that is `=` to its previous value on every recompute
(it hashes on `:eid` alone), so its signal never propagates. Unticking an
output therefore changed none of their inputs and the cached list was served
stale, leaving a row for an output the last solve never computed until the
page was reloaded.

Introduce `:worksheet/enabled-output-uuids`, a re-posh `:query` sub, which
does track the conn, and make it the signal for
`:worksheet/output-uuids-conditionally-filtered`,
`:worksheet/output-uuids-filtered`,
`:worksheet/directional-parent-output-uuids`,
`:worksheet/graphed-output-uuids` and `:worksheet/all-output-uuids`. Their
remaining `d/q` only touches `@@vms-conn`, which is immutable at runtime.
Ordering, the `hide-result?`/`hide-graph?` removal, the conditional
evaluation and `(remove hidden?)` are unchanged. `:worksheet/all-output-uuids`
matters beyond display: it is the output set the solver reads.

Also stop a missing result cell from rendering as the string "NaN".
`:worksheet/first-row-results-gv-uuid->value` returns nil when there is no
cell, `(neg? nil)` is false in CLJS, so nil reached `continuous-fmt` and
`js/parseFloat` printed "NaN". Guard the formatter, which covers the flat
table, the matrices, the CSV export and the diagram summary table, and check
for nil explicitly alongside the `neg?`/`pos?` tests in the matrices.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ut uuids"""

Moved to kc-BHP1-1660-fix-non-reactive-output-enabling, which branches from
b08a079 so this revert cannot undo it on a later merge. The non-reactive
output subs and the "NaN" formatting are a separate concern from
:output/implicit?.

This reverts commit 8312c6898e6ba0cd0e8e4df6f3bb2c93adf63c93.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@rjsheperd

rjsheperd commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Is there a migration missing? I tried the testing steps but just got the "Fire Size" inputs (no inputs for ROS/FL)

After re-downloading the latest VMS data from Dev, it worked. Nice work!

@Kcheung42
Kcheung42 merged commit 809bc95 into main Oct 9, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants