Skip to content

Keep one copy of the 6 session-bound helpers, in R/ (part of #10) - #12

Merged
wolski merged 2 commits into
mainfrom
dedup-stateful-helpers
Nov 3, 2026
Merged

wolski merged 2 commits into
mainfrom
dedup-stateful-helpers

Conversation

@wolski

@wolski wolski commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Part of #10. Second of two steps, stacked on #11 (base switches to main once #11 is merged). With this, no helper is defined twice any more.

Why these 6 were different

  • bucket_add, bucket_remove, bucket_clear, resolve_features read featureBucket, seqAnnoReactive, eds; make_rep_heatmap, make_rep_dendrogram read input
  • These are per-session objects, so the server files kept their own copies inside the server scope

Changes

  • R/helperFunctions.R holds the app's version of all 6: eds-based bucket_add/resolve_features (the R/ copies still used inputDataReactive()), and make_rep_heatmap with rich level-label legends drawn by draw_heatmap()
  • New internal bind_to_scope(fn, env = parent.frame()): returns fn with its free variables resolved in the server scope (the production twin of the tests' with_mock_env())
  • server-featureBucket.R, server-replicability.R: each definition replaced by name <- bind_to_scope(name); the 18 call sites are unchanged
  • Tests: bucket and resolve_features mocks provide eds instead of inputDataReactive(); new test for the counts-only fallback (no DE table, so any dataset feature is valid)
  • Guard test: allowlist removed

Behaviour: none for the app; every helper runs the code the app ran before.

Verified locally

  • Full test suite: 1134 pass; the same 3 pre-existing test-prolfquapp_h5ad.R failures as on main (local prolfquapp 2.11.1), unrelated
  • Guard test fails when PR A's server-featureBucket.R is restored (names the 4 bucket helpers), passes after reverting
  • App smoke run from this branch with the demo dataset: Replicability heatmap + dendrogram draw and the heatmap PDF download works (1 page); features added from the DE table reach the bucket and Boxplots; "Empty the bucket?" clears it; no errors on page or in the R log
  • Not exercised in the app: adding or removing features through the paste box or selector (resolve_features, bucket_remove). Browser automation could not trigger those inputs, on unchanged main either; covered by the unit tests

Prepared with the assistance of an AI coding agent.

bucket_add, bucket_remove, bucket_clear, resolve_features (featureBucket,
seqAnnoReactive, eds) and make_rep_heatmap, make_rep_dendrogram (input)
read per-session state, which is why the server files had their own
copies. R/ now holds the app's version of each, and the server files bind
it to their scope with the new internal bind_to_scope() instead of
redefining it; call sites are unchanged.

- bucket_add / resolve_features: the eds-based app versions replace the
  R/ copies that still used inputDataReactive(); tests mock eds instead
- make_rep_heatmap: the app version with rich level-label legends
- Guard test: no allowlist left

Assisted-by: AI coding agent
@wolski
wolski changed the base branch from dedup-pure-helpers to main November 3, 2026 10:04
@wolski
wolski merged commit 564572b into main Nov 3, 2026
3 checks passed
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.

1 participant