Implement submenu ordering and sampling grouping features

.agents/skills/check-llama-gui-diff-and-prs/SKILL.md

@@ -0,0 +1,67 @@
+---
+name: check-llama-gui-diff-and-prs
+description: Review Llama-GUI working-tree, branch, commit, or pull-request changes for correctness, regressions, security issues, missing tests, and violations of repository invariants. Use when asked to check, inspect, audit, or review a Llama-GUI diff or PR, including pre-PR reviews and reviews of local uncommitted changes. Report findings only unless the user separately asks for fixes.
+---
+
+# Check Llama-GUI Diff and PRs
+
+Review changes as a code reviewer. Prioritize concrete defects introduced by the change; do not turn the review into a general cleanup exercise.
+
+## Establish scope
+
+1. Confirm the repository root contains `AGENTS.md`, `docs/directory.md`, and the Llama-GUI layout. Stop if this is a different repository.
+2. Read `AGENTS.md`, `docs/directory.md`, and `docs/tests.md`. Follow any newer nested `AGENTS.md` that governs a changed file.
+3. Inspect `git status --short`, the current branch, remotes, and recent commits.
+4. Determine the review target from the request:
+   - For local changes, inspect both unstaged and staged diffs, plus untracked source files.
+   - For a branch or pre-PR review, diff the merge base against the appropriate base branch. Do not assume `main` if upstream metadata identifies another base.
+   - For a PR, use available GitHub tooling or `gh` to read its base/head, description, changed files, commits, checks, and review threads. Review the actual patch locally when available.
+5. Do not fetch, checkout, modify files, post reviews, or change PR state unless the user explicitly authorizes that action. If remote PR data is unavailable, review the local diff and state the limitation.
+
+When `.codegraph/` exists, use CodeGraph before text search to locate symbols and call paths. If its MCP tool and CLI are unavailable, fall back to `rg` and direct reads.
+
+## Review the change
+
+Read the complete changed functions and enough surrounding callers, consumers, tests, and data flow to judge behavior. Do not review only the patch hunks.
+
+Check especially for:
+
+- violated invariants from `AGENTS.md`, including shared UI flag state, safe rendering, realm-safe type checks, backend locks, input validation, sanitized client errors, and process lifecycle rules;
+- cross-tab or command-preview desynchronization caused by duplicated state or direct `flagValues` mutation;
+- incorrect frontend script order or missing updates to `docs/directory.md` when scripts change;
+- backend race conditions, missing cleanup, unsafe paths, leaked secrets, overly broad exception handling, and Windows process-group mistakes;
+- stale or unsupported llama.cpp flags, conflicting chat-template arguments, incorrect enum values, or changed launch-argument behavior;
+- `fetchJson()` callers that mishandle `null` and `_BODY_HANDLED` comparisons that use equality instead of identity;
+- user/model content passed to `innerHTML`, error paths hidden by empty catches, or values crossing realms checked with `instanceof`;
+- behavioral changes without focused regression tests, and tests that pass without exercising the changed path;
+- PR description, implementation, and test evidence that disagree.
+
+Trace every suspicious condition to a realistic failure mode. Before reporting it, verify that existing guards, callers, or tests do not make it impossible. Ignore pre-existing problems unless the patch makes them newly reachable or materially worse.
+
+## Validate proportionally
+
+Start with read-only inspection. Run the narrow checks mapped in `AGENTS.md` and `docs/tests.md` when dependencies are already available. Typical choices include:
+
+- `node --check` for each touched JavaScript file;
+- focused Node tests for the modified feature;
+- `npm run test:flag-definitions` for flag changes;
+- `npm run test:frontend` for shared-state, mirrored-control, DOM-wiring, or command-preview changes;
+- `python -m unittest discover tests -v` for backend changes.
+
+Do not install dependencies or mutate external state merely to complete a review. Distinguish a failing check from a check that could not run, and include the relevant command and concise failure evidence.
+
+## Report findings
+
+Lead with findings ordered by severity. For each finding:
+
+- assign a priority: **P0** release-blocking or catastrophic, **P1** high-impact, **P2** ordinary correctness issue, or **P3** low-impact but actionable;
+- give a short imperative title;
+- cite the tightest changed-file line or PR diff location;
+- explain the concrete trigger and user-visible or operational impact;
+- avoid speculative wording when the failure can be demonstrated.
+
+Use inline code comments when the environment supports them. Otherwise use a compact Markdown list with clickable absolute file links. Keep summaries brief after the findings.
+
+If there are no actionable defects, say so explicitly, list the checks run, and mention any material coverage gap or residual risk. Never claim the change is correct merely because tests pass.
+
+Do not implement fixes during a review-only request. Offer to fix confirmed findings as a separate next step.

.agents/skills/check-llama-gui-diff-and-prs/agents/openai.yaml

@@ -0,0 +1,4 @@
+interface:
+  display_name: "Check Llama-GUI Diff and PRs"
+  short_description: "Review Llama-GUI changes for errors"
+  default_prompt: "Use $check-llama-gui-diff-and-prs to review the current Llama-GUI diff and related pull request for errors."

docs/directory.md

@@ -245,7 +245,9 @@ The frontend loads scripts in a strict dependency order via `ui/index.html`:
 - **`enum`**: Dropdown select from a predefined options list.
 - **`multi_enum`**: Multiple checkboxes for selecting zero or more values. Supports an `all` shortcut and `risk: "high"` badges with warnings for dangerous options (e.g., shell command execution).
 
-Categories can also define `submenu` entries that render as collapsible sub-accordions within the main category.
+Any flag can declare `submenu: "<name>"` to render inside a collapsible sub-accordion within its category instead of at the category's top level. Flags without `submenu` render first, in definition order; submenu blocks follow.
+
+A category may declare `submenuOrder: [...]` (`ui/js/flags/categories.js`) to control the order those blocks appear in. It is presentation-only and deliberately independent of the `FLAGS` array order, which determines CLI argument order in `buildLaunchArgs()` and must never be reordered for display purposes. Submenu names not listed in `submenuOrder` keep their definition order and sort after every listed name. `tests/frontend/flag_definitions_unit.cjs` fails the build if the two lists drift apart.
 
 ### Launch Args Generation (`flagCore.getLaunchArgs()`)
 
@@ -751,7 +753,9 @@ Defined in `BUILTIN_SAMPLER_PRESETS` in `ui/js/app-data.js` and managed by `ui/j
 - Quick Launch tab: Sampler Preset controls in the sampler section (Load, then Save / Rename / Delete).
 - Quick profiles reference preset names (e.g., `samplerPresetName: "Balanced"`).
 - Loading a preset calls `window.LlamaGui.samplerPresets.applySamplerPresetValues()` which writes through `window.LlamaGui.flagCore.setMultipleFlagValues()`.
-- Configure groups all DRY controls under the collapsible **DRY Sampling** submenu. `dry_sequence_breakers` uses a repeatable text list because llama.cpp requires one `--dry-sequence-breaker` argument per breaker.
+- Configure keeps six sampling flags at the top level — `--temp`, `--top-k`, `--top-p`, `--min-p`, `--repeat-penalty`, `--presence-penalty` — and groups the remaining 20 into eight collapsible submenus, displayed in this order: **Repetition Penalties**, **DRY Sampling**, **XTC Sampling**, **Advanced Truncation**, **Dynamic Temperature**, **Mirostat**, **Sampler Order**, **Generation Control**. All submenus start collapsed.
+- Grouping is presentation-only. Sampler presets still read and write every sampling flag (`sampler-presets.js` selects by `category === "sampling"`), and rows inside a collapsed submenu are still present in the DOM, so preset apply/save works without expanding anything.
+- `dry_sequence_breakers` uses a repeatable text list because llama.cpp requires one `--dry-sequence-breaker` argument per breaker.
 
 ### Model Load Mode
 
@@ -837,13 +841,13 @@ The Configure tab includes an advanced `Custom Launch Args` textarea near the co
 The Configure tab has a search input that filters visible flags in real-time.
 
 - Searches across: flag name (`--flag`), label, id, description, short description, beginner tip, submenu name, and all option labels/values.
-- When a search query is active, all accordion categories automatically expand to show matching flags.
+- When a search query is active, `openMatchingSearchSections()` expands all accordion categories, plus every submenu holding at least one matching flag — so a match is never hidden behind a collapsed submenu header.
+- The user's own submenu state is snapshotted into `savedOpenSubmenus` when a search begins and restored when the query is cleared, so searching does not destroy it. `resetOpenCategories()` (called on tool change) discards the snapshot, because submenu keys from the previous tool may no longer exist.
 - Partial matches are highlighted; unmatched flags within a category are hidden.
 - Empty results show "No configuration options match your search."
-- Escape key or clear button resets the search and collapses all categories.
+- Escape key or clear button resets the search and restores the pre-search submenu state. Categories opened by the search stay open.
 - "Expand All" opens all categories and submenus. "Collapse All" closes them.
-- Individual categories remember their open/closed state via `openCategories` Set.
-- Individual submenus remember their state via `openSubmenus` Set.
+- Individual categories remember their open/closed state via `openCategories` Set; submenus via `openSubmenus`, keyed `"<categoryId>::<submenuName>"`.
 
 ---
 

tests/frontend/flag_definitions_unit.cjs

@@ -79,6 +79,40 @@ function validateOptions(flag, addError, addWarning) {
     }
 }
 
+// Cross-checks flag.submenu against each category's optional submenuOrder hint, so a typo
+// in either place fails loudly instead of silently creating an extra unordered submenu.
+// Categories without submenuOrder are skipped: the hint is opt-in.
+function validateSubmenuOrder(flags, categories, addError) {
+    const usedByCategory = new Map();
+    for (const flag of flags) {
+        if (!flag || typeof flag !== "object") continue;
+        const submenu = typeof flag.submenu === "string" ? flag.submenu.trim() : "";
+        if (!submenu) continue;
+        if (!usedByCategory.has(flag.category)) usedByCategory.set(flag.category, new Set());
+        usedByCategory.get(flag.category).add(submenu);
+    }
+
+    for (const category of categories || []) {
+        if (!category || !Array.isArray(category.submenuOrder)) continue;
+        const declared = new Set(category.submenuOrder);
+        const used = usedByCategory.get(category.id) || new Set();
+
+        if (declared.size !== category.submenuOrder.length) {
+            addError(`Category "${category.id}" has duplicate names in submenuOrder.`);
+        }
+        for (const name of used) {
+            if (!declared.has(name)) {
+                addError(`Submenu "${name}" in category "${category.id}" is missing from submenuOrder.`);
+            }
+        }
+        for (const name of declared) {
+            if (!used.has(name)) {
+                addError(`Category "${category.id}" lists unused submenu "${name}" in submenuOrder.`);
+            }
+        }
+    }
+}
+
 function validateFlags(flags, categories) {
     const errors = [];
     const warnings = [];
@@ -153,6 +187,8 @@ function validateFlags(flags, categories) {
         }
     }
 
+    validateSubmenuOrder(flags, categories, addError);
+
     return { errors, warnings };
 }
 
@@ -198,11 +234,61 @@ assert.deepEqual(validateFlags(current.flags, current.categories), {
     ],
 });
 
+// Configure keeps exactly these six sampling flags at the top level; every other sampling
+// flag lives in a submenu. See docs/design-docs/V2-planning-log.md item 2.
+{
+    const SAMPLING_TOP_LEVEL = new Set([
+        "temperature",
+        "top_k",
+        "top_p",
+        "min_p",
+        "repeat_penalty",
+        "presence_penalty",
+    ]);
+
+    const samplingFlags = current.flags.filter((flag) => flag.category === "sampling");
+    const topLevel = samplingFlags
+        .filter((flag) => !hasText(flag.submenu))
+        .map((flag) => flag.id);
+
+    assert.deepEqual(
+        new Set(topLevel),
+        SAMPLING_TOP_LEVEL,
+        `sampling top-level flags drifted; got: ${topLevel.join(", ")}`
+    );
+    assert.equal(
+        samplingFlags.length - topLevel.length,
+        20,
+        "expected 20 sampling flags to be grouped into submenus"
+    );
+}
+
 assert.deepEqual(validateFlags(null, []), {
     errors: ["FLAGS must be an array."],
     warnings: [],
 });
 
+{
+    const categories = [{ id: "server", submenuOrder: ["Alpha", "Beta"] }];
+    const result = validateFlags(
+        [
+            makeFlag({ submenu: "Alpha" }),
+            makeFlag({ id: "second", flag: "--second", submenu: "Gamma" }),
+        ],
+        categories
+    );
+    assertIncludes(result.errors, 'Submenu "Gamma" in category "server" is missing from submenuOrder.');
+    assertIncludes(result.errors, 'Category "server" lists unused submenu "Beta" in submenuOrder.');
+}
+
+{
+    const result = validateFlags(
+        [makeFlag({ submenu: "Alpha" })],
+        [{ id: "server", submenuOrder: ["Alpha", "Alpha"] }]
+    );
+    assertIncludes(result.errors, 'Category "server" has duplicate names in submenuOrder.');
+}
+
 {
     const result = validateFlags(
         [makeFlag(), makeFlag({ flag: "--second" })],

tests/frontend/launch_args_unit.cjs

@@ -321,6 +321,19 @@ function launchResult() {
     assert.ok(args.includes("--jinja"));
 }
 
+{
+    vm.runInContext(`
+        window.LlamaGui.flagCore.replaceFlagValues(getDefaultValues());
+        window.LlamaGui.flagCore.setMultipleFlagValues({
+            spec_type: "draft-dspark",
+            draft_max: 7,
+        });
+    `, context);
+    const args = flatLaunchArgs();
+    assert.ok(args.includes("--spec-type") && args.includes("draft-dspark"));
+    assert.ok(args.includes("--spec-draft-n-max") && args.includes("7"));
+}
+
 {
     vm.runInContext(`
         window.LlamaGui.flagCore.replaceFlagValues(getDefaultValues());

ui/js/config-flags-ui.js

@@ -257,7 +257,7 @@
                 body.appendChild(createFlagRow(f));
             }
 
-            for (const [submenuName, submenuFlags] of submenuMap.entries()) {
+            for (const [submenuName, submenuFlags] of sortSubmenus(submenuMap, group.submenuOrder)) {
                 body.appendChild(createSubmenuBlock(catId, submenuName, submenuFlags));
             }
 
@@ -277,6 +277,20 @@
         dependencies.refreshQuickLaunchUI();
     }
 
+    // Orders submenu blocks by the category's optional submenuOrder hint. Names not listed
+    // there keep their definition order and sort after every listed name.
+    function sortSubmenus(submenuMap, submenuOrder) {
+        const entries = Array.from(submenuMap.entries());
+        if (!Array.isArray(submenuOrder) || submenuOrder.length === 0) return entries;
+
+        const rank = new Map(submenuOrder.map((name, index) => [name, index]));
+        const unlisted = submenuOrder.length;
+        return entries
+            .map((entry, index) => ({ entry, index, rank: rank.has(entry[0]) ? rank.get(entry[0]) : unlisted }))
+            .sort((a, b) => (a.rank - b.rank) || (a.index - b.index))
+            .map(item => item.entry);
+    }
+
     function createSubmenuBlock(categoryId, submenuName, submenuFlags) {
         const wrap = document.createElement("div");
         wrap.className = "flag-submenu";

ui/js/flags/categories.js

@@ -8,7 +8,24 @@ const FLAG_CATEGORIES = [
 	{ id: "cpu", name: "CPU & Threads", icon: "⚙️" },
 	{ id: "gpu", name: "GPU / Acceleration", icon: "🎮" },
 	{ id: "auto_fit", name: "Auto Fit", icon: "📐" },
-	{ id: "sampling", name: "Sampling", icon: "🎲" },
+	// submenuOrder controls only the display order of submenu blocks in the Configure
+	// tab. It is deliberately separate from the FLAGS array order, which determines CLI
+	// argument order in buildLaunchArgs() and must not be reordered for presentation.
+	{
+		id: "sampling",
+		name: "Sampling",
+		icon: "🎲",
+		submenuOrder: [
+			"Repetition Penalties",
+			"DRY Sampling",
+			"XTC Sampling",
+			"Advanced Truncation",
+			"Dynamic Temperature",
+			"Mirostat",
+			"Sampler Order",
+			"Generation Control",
+		],
+	},
 	{ id: "rope", name: "RoPE Scaling", icon: "📏" },
 	{ id: "conversation", name: "Conversation & Chat", icon: "💬" },
 	{ id: "lora", name: "LoRA & Control Vectors", icon: "🔗" },