rbouckaert · Sep 10, 2026 · Sep 7, 2026
diff --git a/beast-fx/src/main/java/beastfx/app/inputeditor/BeautiConfig.java b/beast-fx/src/main/java/beastfx/app/inputeditor/BeautiConfig.java
@@ -1,25 +1,20 @@
 package beastfx.app.inputeditor;
 
 
-
-import java.io.File;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
-
-import beastfx.app.util.Alert;
-import beastfx.app.util.FXUtils;
-import javafx.scene.Parent;
 import beast.base.core.BEASTInterface;
 import beast.base.core.BEASTObject;
 import beast.base.core.Description;
 import beast.base.core.Input;
 import beast.base.core.Input.Validate;
 import beast.base.evolution.alignment.Alignment;
-import beast.base.spec.evolution.tree.MRCAPrior;
 import beast.base.parser.XMLParser;
+import beast.base.spec.evolution.tree.MRCAPrior;
+import beastfx.app.util.Alert;
+import beastfx.app.util.FXUtils;
+import javafx.scene.Parent;
+
+import java.io.File;
+import java.util.*;
 
 @Description("Beauti configuration object, used to find Beauti configuration " +
         "information from Beauti template files.")
@@ -131,17 +126,18 @@ public void initAndValidate() {
 		}
     }
 
-	final static String HYPER_PRIOR_XML = 
-    		"    <beast version='2.0'\n" +
-    		"    	       namespace='beastfx.app.beauti:beast.base.core:beast.base.evolution.operators:beast.base.inference.distribution'>\n" +
-    		"    	<!-- Parameter Hyper Prior -->\n" +
-    		"    	        <subtemplate spec='beastfx.app.inputeditor.BeautiSubTemplate' id='HyperPrior' class='beast.base.inference.distribution.Prior' mainid='HyperPrior.$(n)'>\n" +
+	final static String HYPER_PRIOR_XML =
+    		"    <beast version='2.8'\n" +
+    		"    	       namespace='beastfx.app.beauti:beast.base.core:beast.base.spec.inference.distribution:beast.base.spec.inference.parameter:beast.base.spec.inference.operator'>\n" +
+    		"    	<!-- Parameter Hyper Prior: a LogUniform distribution, same as ParametricDistributions.xml uses to replace the old OneOnX. -->\n" +
+    		"    	        <subtemplate spec='beastfx.app.inputeditor.BeautiSubTemplate' id='HyperPrior' class='beast.base.spec.inference.distribution.LogUniform' mainid='HyperPrior.$(n)'>\n" +
     		"    	<![CDATA[\n" +
-    		"    	        <beastObject id='HyperPrior.$(n)' spec='Prior' x='@parameter.$(n)'>\n" +
-    		"    	            <distr spec='OneOnX'/>\n" +
+    		"    	        <beastObject id='HyperPrior.$(n)' spec='beast.base.spec.inference.distribution.LogUniform' param='@parameter.$(n)'>\n" +
+    		"    	            <lower spec='beast.base.spec.inference.parameter.RealScalarParam' domain='PositiveReal' value='1.0E-8' estimate='false'/>\n" +
+    		"    	            <upper spec='beast.base.spec.inference.parameter.RealScalarParam' domain='PositiveReal' value='1.0E8' estimate='false'/>\n" +
     		"    			</beastObject>\n" +
     		"\n" +
-    		"    	        <beastObject id='hyperScaler.$(n)' spec='ScaleOperator' scaleFactor='0.5' weight='0.1' parameter='@parameter.$(n)'/>\n" +
+    		"    	        <beastObject id='hyperScaler.$(n)' spec='beast.base.spec.inference.operator.ScaleOperator' scaleFactor='0.5' weight='0.1' parameter='@parameter.$(n)'/>\n" +
     		"    	]]>\n" +
     		"    	            <connect spec='beastfx.app.inputeditor.BeautiConnector' srcID='parameter.$(n)'            targetID='state' inputName='stateNode' if='inposterior(parameter.$(n)) and parameter.$(n)/estimate=true'/>\n" +
     		"\n" +

diff --git a/beast-fx/src/main/java/beastfx/app/inputeditor/spec/ScalarInputEditor.java b/beast-fx/src/main/java/beastfx/app/inputeditor/spec/ScalarInputEditor.java
@@ -5,7 +5,6 @@
 import beast.base.core.BEASTInterface;
 import beast.base.core.Input;
 import beast.base.core.Log;
-import beast.base.inference.Distribution;
 import beast.base.inference.Operator;
 import beast.base.inference.StateNode;
 import beast.base.parser.PartitionContext;
@@ -273,25 +272,50 @@ public void toggleEstimate() {
             	}
 
             	String id = parameter2.getID();
-
 
-            	if (id.startsWith("RealParameter")) {
-                	ScalarDistribution<?,?> parent = null; 
+            	// Has a hyperprior already been created for this parameter? True if one of
+            	// parameter2's outputs is a ScalarDistribution whose own param is parameter2
+            	// (a hyperprior is always wired that way). Not true for the distribution
+            	// parameter2 is itself a hyperparameter *of* (e.g. Normal, if parameter2 is
+            	// "mean"): that Normal's param points at what IT is a prior for (e.g.
+            	// birthRate), not at parameter2.
+            	boolean hasHyperPrior = false;
+            	for (Object beastObject2 : parameter2.getOutputs()) {
+            		if (beastObject2 instanceof ScalarDistribution<?,?> sd && sd.paramInput.get() == parameter2) {
+            			hasHyperPrior = true;
+            			break;
+            		}
+            	}
+
+            	// First time for this parameter: find the distribution it's a hyperparameter
+            	// of, then rename it so a hyperprior can be created for it and found again
+            	// on every later call (see the rename below).
+            	if (!hasHyperPrior) {
+                	ScalarDistribution<?,?> parent = null;
     	            for (Object beastObject2 : parameter2.getOutputs()) {
-    	                if (beastObject2 instanceof ScalarDistribution<?,?>) {
-                    		parent = (ScalarDistribution<?,?>) beastObject2; 
+    	                // the distribution parameter2 is itself a hyperparameter of (e.g.
+    	                // Normal, if parameter2 is its "mean"). It's the only kind of
+    	                // ScalarDistribution parameter2 can have among its outputs here, since
+    	                // hasHyperPrior above already ruled out one whose "param" is parameter2.
+    	                if (beastObject2 instanceof ScalarDistribution<?,?> sd) {
+                    		parent = sd;
     	                    break;
     	                }
     	            }
-    	            Distribution grandparent = null; 
-    	            for (Object beastObject2 : parent.getOutputs()) {
-    	                if (beastObject2 instanceof Distribution) {
-                    		grandparent = (Distribution) beastObject2; 
-    	                    break;
-    	                }
+    	            if (parent == null) {
+    	            	// shouldn't happen given isParametricDistributionParameter, but defend
+    	            	// against an inconsistent model rather than dereferencing a null parent
+    	            	Log.err.println("Could not find the distribution owning " + parameter2.getID() + "; not adding a hyperprior");
+    	            	m_isEstimatedBox.setSelected(false);
+    	            	parameter2.isEstimatedInput.setValue(false, parameter2);
+    	            	return;
     	            }
-            		id = "parameter.hyper" + parent.getClass().getSimpleName() + "-" + 
-            				m_input.getName() + "-" + grandparent.getID();
+
+    	            // rename from the auto-generated id (e.g. "RealScalarParam.N") to
+    	            // "parameter.<context>", which the $(n) substitution below, and BeautiDoc's
+    	            // general "parameter.*" sync scan (see BeautiDoc.scrubAll), rely on to find it
+            		id = "parameter.hyper" + parent.getClass().getSimpleName() + "-" +
+            				m_input.getName() + "-" + parent.getID();
             		doc.pluginmap.remove(parameter2.getID());
             		parameter2.setID(id);
             		doc.addPlugin(parameter2);
@@ -300,12 +324,17 @@ public void toggleEstimate() {
 
             	PartitionContext context = new PartitionContext(id.substring("parameter.".length()));
             	Log.warning.println(context + " " + id);
+            	// ensures the hyperprior's own objects (HyperPrior.$(n), hyperScaler.$(n))
+            	// exist -- a safe no-op if they already do. Connecting/disconnecting them
+            	// (state, operator, tracelog, the priors list) based on the current estimate
+            	// value is done separately, by BeautiDoc's general "parameter.*" sync scan
+            	// (see BeautiDoc.scrubAll), which hardSync()/refreshPanel() below triggers.
             	doc.beautiConfig.hyperPriorTemplate.createSubNet(context, true);
             }
         	hardSync();
             refreshPanel();
         } catch (Exception ex) {
-            Log.err.println("ParameterInputEditor " + ex.getMessage());
+            Log.err.println("ScalarInputEditor " + ex.getMessage());
         }
     }
 

diff --git a/beast-fx/src/test/java/test/beastfx/app/beauti/HyperPriorTest.java b/beast-fx/src/test/java/test/beastfx/app/beauti/HyperPriorTest.java
@@ -0,0 +1,194 @@
+package test.beastfx.app.beauti;
+
+
+import beast.base.inference.CompoundDistribution;
+import beast.base.inference.Distribution;
+import beast.base.spec.inference.distribution.LogNormal;
+import beast.base.spec.inference.distribution.LogUniform;
+import beastfx.app.beauti.BeautiTabPane;
+import javafx.scene.control.ComboBox;
+import javafx.stage.Stage;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.testfx.api.FxRobot;
+import org.testfx.framework.junit5.ApplicationExtension;
+import org.testfx.framework.junit5.Start;
+import org.testfx.service.query.NodeQuery;
+import org.testfx.util.WaitForAsyncUtils;
+
+import java.io.File;
+import java.util.List;
+import java.util.Set;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+
+/**
+ * Checking "estimate" on a LogNormal prior's own parameter (M or S) in the Priors panel is meant
+ * to pop up a "Hyper prior" confirmation dialog, and, once confirmed, add a hyper prior (a
+ * LogUniform distribution) for that parameter as a new row in the Priors panel. This used to
+ * silently do nothing in the spec framework, because the parameter's auto-generated id
+ * ("RealScalarParam.N") never matched the legacy "RealParameter" prefix check that triggered
+ * renaming + hyper prior creation, and the legacy hyper prior object was also incompatible with
+ * spec-framework parameters -- so no row was ever added.
+ * <p>
+ * The same "estimate" checkbox in the Site Model and Clock Model panels must never pop up the
+ * hyper prior dialog at all, since those parameters (e.g. mutationRate, clock.rate) are not a
+ * distribution's own hyperparameter -- see spec.ScalarInputEditor#isHyperparameter.
+ */
+@ExtendWith(ApplicationExtension.class)
+public class HyperPriorTest extends BeautiBase {
+
+	@Start
+	public void start(Stage stage) {
+		System.setProperty("beast.is.junit.testing", "true");
+		try {
+			BeautiTabPane tabPane = BeautiTabPane.main2(new String[] {}, stage);
+			this.doc = tabPane.doc;
+			stage.show();
+		} catch (Exception e) {
+			e.printStackTrace();
+		}
+	}
+
+	/**
+	 * Both M and S are checked in a single test method (rather than one @Test each) because
+	 * every "estimate" checkbox in this window shares its plain fx:id across BEAUti windows
+	 * (e.g. "M.isEstimated" is not partition-qualified), and this test harness does not close a
+	 * previous test method's window before the next one opens (see FixedMeanRateTest's comment
+	 * on cascading windows) -- a second, separate @Test method here would risk resolving its
+	 * checkbox lookup against the first method's now-stale window instead of its own.
+	 */
+	@Test
+	public void addHyperPriorForLogNormalMAndS(FxRobot robot) throws Exception {
+		switchBirthRatePriorToLogNormal(robot);
+
+		// M is Real-domain, S is PositiveReal-domain: both get a LogUniform hyper prior
+		int priorCountBefore = priors().size();
+		clickOnNodesWithID(robot, "M.isEstimated");
+		WaitForAsyncUtils.waitForFxEvents();
+		robot.clickOn("Yes");
+		WaitForAsyncUtils.waitForFxEvents();
+
+		List<Distribution> priors = priors();
+		assertThat(priors.size()).as("a hyper prior for M should have been added to the Priors panel")
+				.isEqualTo(priorCountBefore + 1);
+		LogUniform hyperPriorForM = (LogUniform) lastPrior(priors);
+		assertThat(hyperPriorForM.paramInput.get()).as("the hyper prior must apply to M itself")
+				.isSameAs(findLogNormal(priors).MParameterInput.get());
+
+		priorCountBefore = priors().size();
+		clickOnNodesWithID(robot, "S.isEstimated");
+		WaitForAsyncUtils.waitForFxEvents();
+		robot.clickOn("Yes");
+		WaitForAsyncUtils.waitForFxEvents();
+
+		priors = priors();
+		assertThat(priors.size()).as("a hyper prior for S should have been added to the Priors panel")
+				.isEqualTo(priorCountBefore + 1);
+		LogUniform hyperPriorForS = (LogUniform) lastPrior(priors);
+		assertThat(hyperPriorForS.paramInput.get()).as("the hyper prior must apply to S itself")
+				.isSameAs(findLogNormal(priors).SParameterInput.get());
+
+		makeSureXMLParses();
+	}
+
+	@Test
+	public void siteModelEstimateNeverOffersHyperPrior(FxRobot robot) throws Exception {
+		importAlignment(NEXUS_DIR, new File("anolis.nex"));
+		WaitForAsyncUtils.waitForFxEvents();
+
+		selectTab(robot, "Site Model");
+		WaitForAsyncUtils.waitForFxEvents();
+		int priorCountBefore = priors().size();
+
+		// if this popped up the "Hyper prior" dialog, nothing here would dismiss it, and
+		// the assertion below would see a stale (unsynced) prior list
+		clickOnNodesWithID(robot, "mutationRate.isEstimated");
+		WaitForAsyncUtils.waitForFxEvents();
+
+		assertThat(priors().size()).as("estimating a Site Model parameter must never add a hyper prior")
+				.isEqualTo(priorCountBefore);
+	}
+
+	@Test
+	public void clockModelEstimateNeverOffersHyperPrior(FxRobot robot) throws Exception {
+		importAlignment(NEXUS_DIR, new File("anolis.nex"));
+		WaitForAsyncUtils.waitForFxEvents();
+
+		selectTab(robot, "Clock Model");
+		WaitForAsyncUtils.waitForFxEvents();
+		int priorCountBefore = priors().size();
+
+		clickOnNodesWithID(robot, "clock.rate.isEstimated");
+		WaitForAsyncUtils.waitForFxEvents();
+
+		assertThat(priors().size()).as("estimating a Clock Model parameter must never add a hyper prior")
+				.isEqualTo(priorCountBefore);
+	}
+
+	/** switch the birth rate's prior from the default Gamma to LogNormal, then expand its row so
+	 * its own M (Real-domain) and S (PositiveReal-domain) parameters -- collapsed by default,
+	 * like every prior row -- render their "estimate" checkboxes. LogNormal itself is
+	 * PositiveReal-domain, matching birthRate -- Normal (Real-domain) is not a domain-compatible
+	 * option for birthRate, so this is not just an arbitrary choice. */
+	private void switchBirthRatePriorToLogNormal(FxRobot robot) throws Exception {
+		importAlignment(NEXUS_DIR, new File("anolis.nex"));
+		WaitForAsyncUtils.waitForFxEvents();
+
+		selectTab(robot, "Priors");
+		WaitForAsyncUtils.waitForFxEvents();
+
+		selectDistribution(robot, "birthRate.t:anolis.distr", "Log Normal");
+		WaitForAsyncUtils.waitForFxEvents();
+
+		Distribution birthRatePrior = null;
+		for (Distribution d : priors()) {
+			if (d instanceof LogNormal) {
+				birthRatePrior = d;
+			}
+		}
+		assertThat(birthRatePrior).as("birth rate prior should now be a LogNormal").isNotNull();
+
+		clickOnNodesWithID(robot, birthRatePrior.getID() + ".editButton");
+		WaitForAsyncUtils.waitForFxEvents();
+	}
+
+	/** select an item (matched by its combo-box display text, e.g. "Log Normal") from a
+	 * distribution ComboBox, driving the ComboBox API directly rather than the dropdown UI
+	 * (avoids the ComboBox's popup window sticking open, which would then swallow later clicks). */
+	@SuppressWarnings("unchecked")
+	private void selectDistribution(FxRobot robot, String comboBoxId, String displayText) {
+		NodeQuery q = robot.lookup(target -> comboBoxId.equals(target.getId()) && target.isVisible());
+		Set<javafx.scene.Node> nodes = q.queryAll();
+		assertThat(nodes).as("no visible ComboBox with id " + comboBoxId).isNotEmpty();
+		ComboBox<Object> comboBox = (ComboBox<Object>) nodes.iterator().next();
+		robot.interact(() -> {
+			for (Object item : comboBox.getItems()) {
+				if (item.toString().equals(displayText)) {
+					comboBox.getSelectionModel().select(item);
+					return;
+				}
+			}
+			throw new AssertionError("No item \"" + displayText + "\" in ComboBox #" + comboBoxId);
+		});
+	}
+
+	private List<Distribution> priors() {
+		CompoundDistribution prior = (CompoundDistribution) doc.pluginmap.get("prior");
+		return prior.pDistributions.get();
+	}
+
+	private Distribution lastPrior(List<Distribution> priors) {
+		return priors.get(priors.size() - 1);
+	}
+
+	private LogNormal findLogNormal(List<Distribution> priors) {
+		for (Distribution d : priors) {
+			if (d instanceof LogNormal ln) {
+				return ln;
+			}
+		}
+		throw new AssertionError("No LogNormal found among the priors");
+	}
+}