From 74cb4933737e10e8b0ecc872376a5c0fffc71310 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 22 May 2026 11:39:57 -0400 Subject: [PATCH] fix(3784): expose adaptive in model_profile settings flow (#91) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(3784): expose adaptive in model_profile settings flow Split the single 4-option model-profile AskUserQuestion into a two-question flow: Q1 (Adaptive / Standard tier / Inherit) routes top-level intent; Q2 (Quality / Balanced / Budget) appears only when Standard tier is chosen. Updates the confirm table and success_criteria to include adaptive. Adds regression test asserting all five valid profiles are reachable interactively via the settings UI. Co-Authored-By: Claude Sonnet 4.6 * changeset: add Fixed entry for #3784 / PR #3795 Co-Authored-By: Claude Sonnet 4.6 * fix(3784): correct Q2-skip comment and remove duplicate brace in settings.md Codex review followup: - Replaced vague "preserve existing config" comment with accurate description: Q1 still writes model_profile on Adaptive/Inherit branches; only Q2 is skipped. - Removed stray duplicate `{` line before the Spawn Plan Researcher question block (pseudocode had two consecutive `{` openers, one spurious). Co-Authored-By: Claude Sonnet 4.6 * fix(3784): address review — gate Q2 structurally, define cancel rule, harden tests Addresses gsd-code-reviewer (M1/M2/m1/m2/m3/m4) and codex adversarial (Q2 gating, save-mapping, Claude-only wording, step-of-2 wording). - F1: Replace //comment-only Q2 gating with Conditional visibility block (mirrors code_review_depth / graphify.auto_update structural pattern) - F2: Define model_profile cancel rule in update_config step (leave existing value unchanged when Q1="Standard tier…" but Q2 cancelled) - F3: Fix Adaptive description — remove "Claude only" tail; describe heavy/light role tiers across all supported runtimes - F4: Remove "step 1 of 2 for standard profiles" from Q1 question text (2-step nature now structurally documented by Conditional visibility) - F5: Fix vacuously-true test disjunct (|| content.includes('Adaptive') always true — 6+ occurrences); assertion now requires role-based cost optimization + heavy roles wording - F6: Add 4-option cap enforcement test (ASK_USER_QUESTION_OPTION_CAP=4 named constant, counts per question object not per AskUserQuestion call) and brace-balance regression test (guards against bd53925f recurrence) * docs(3784): list adaptive in model_profile reference docs --------- Co-authored-by: Claude Sonnet 4.6 --- .changeset/mellow-lemurs-click.md | 5 + docs/CONFIGURATION.md | 40 ++-- get-shit-done/references/planning-config.md | 2 +- get-shit-done/workflows/settings.md | 46 +++- ...s-model-profile-ui-omits-adaptive.test.cjs | 198 ++++++++++++++++++ 5 files changed, 263 insertions(+), 28 deletions(-) create mode 100644 .changeset/mellow-lemurs-click.md create mode 100644 tests/bug-3784-gsd-settings-model-profile-ui-omits-adaptive.test.cjs diff --git a/.changeset/mellow-lemurs-click.md b/.changeset/mellow-lemurs-click.md new file mode 100644 index 000000000..f8a24fe86 --- /dev/null +++ b/.changeset/mellow-lemurs-click.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3795 +--- +Expose adaptive value in gsd-settings model_profile selection flow. diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 2c97c0bac..def9ebb77 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -759,26 +759,26 @@ Invalid flag tokens are sanitized and logged as warnings. Only recognized GSD fl ### Profile Definitions -| Agent | `quality` | `balanced` | `budget` | `inherit` | -|-------|-----------|------------|----------|-----------| -| gsd-planner | Opus | Opus | Sonnet | Inherit | -| gsd-roadmapper | Opus | Sonnet | Sonnet | Inherit | -| gsd-executor | Opus | Sonnet | Sonnet | Inherit | -| gsd-phase-researcher | Opus | Sonnet | Haiku | Inherit | -| gsd-project-researcher | Opus | Sonnet | Haiku | Inherit | -| gsd-research-synthesizer | Sonnet | Sonnet | Haiku | Inherit | -| gsd-debugger | Opus | Sonnet | Sonnet | Inherit | -| gsd-codebase-mapper | Sonnet | Haiku | Haiku | Inherit | -| gsd-verifier | Sonnet | Sonnet | Haiku | Inherit | -| gsd-plan-checker | Sonnet | Sonnet | Haiku | Inherit | -| gsd-integration-checker | Sonnet | Sonnet | Haiku | Inherit | -| gsd-nyquist-auditor | Sonnet | Sonnet | Haiku | Inherit | -| gsd-pattern-mapper | Sonnet | Sonnet | Haiku | Inherit | -| gsd-ui-researcher | Opus | Sonnet | Haiku | Inherit | -| gsd-ui-checker | Sonnet | Sonnet | Haiku | Inherit | -| gsd-ui-auditor | Sonnet | Sonnet | Haiku | Inherit | -| gsd-doc-writer | Opus | Sonnet | Haiku | Inherit | -| gsd-doc-verifier | Sonnet | Sonnet | Haiku | Inherit | +| Agent | `quality` | `balanced` | `budget` | `adaptive` | `inherit` | +|-------|-----------|------------|----------|------------|-----------| +| gsd-planner | Opus | Opus | Sonnet | Opus | Inherit | +| gsd-roadmapper | Opus | Sonnet | Sonnet | Opus | Inherit | +| gsd-executor | Opus | Sonnet | Sonnet | Sonnet | Inherit | +| gsd-phase-researcher | Opus | Sonnet | Haiku | Sonnet | Inherit | +| gsd-project-researcher | Opus | Sonnet | Haiku | Sonnet | Inherit | +| gsd-research-synthesizer | Sonnet | Sonnet | Haiku | Haiku | Inherit | +| gsd-debugger | Opus | Sonnet | Sonnet | Opus | Inherit | +| gsd-codebase-mapper | Sonnet | Haiku | Haiku | Haiku | Inherit | +| gsd-verifier | Sonnet | Sonnet | Haiku | Sonnet | Inherit | +| gsd-plan-checker | Sonnet | Sonnet | Haiku | Haiku | Inherit | +| gsd-integration-checker | Sonnet | Sonnet | Haiku | Haiku | Inherit | +| gsd-nyquist-auditor | Sonnet | Sonnet | Haiku | Haiku | Inherit | +| gsd-pattern-mapper | Sonnet | Sonnet | Haiku | Haiku | Inherit | +| gsd-ui-researcher | Opus | Sonnet | Haiku | Sonnet | Inherit | +| gsd-ui-checker | Sonnet | Sonnet | Haiku | Haiku | Inherit | +| gsd-ui-auditor | Sonnet | Sonnet | Haiku | Haiku | Inherit | +| gsd-doc-writer | Opus | Sonnet | Haiku | Sonnet | Inherit | +| gsd-doc-verifier | Sonnet | Sonnet | Haiku | Haiku | Inherit | > **All 33 shipped agents have explicit per-profile tier assignments** in the catalog (`sdk/shared/model-catalog.json`). The table above shows a representative subset of the most-used agents. For agents not listed here, `model_overrides` accepts any shipped agent name. The authoritative profile data is derived from `sdk/shared/model-catalog.json` via `get-shit-done/bin/lib/model-catalog.cjs` and `sdk/src/model-catalog.ts`. diff --git a/get-shit-done/references/planning-config.md b/get-shit-done/references/planning-config.md index d58d29bd4..1c972b893 100644 --- a/get-shit-done/references/planning-config.md +++ b/get-shit-done/references/planning-config.md @@ -226,7 +226,7 @@ Generated from `CONFIG_DEFAULTS` (core.cjs) and `VALID_CONFIG_KEYS` (config.cjs) | Key | Type | Default | Allowed Values | Description | |-----|------|---------|----------------|-------------| -| `model_profile` | string | `"balanced"` | `"quality"`, `"balanced"`, `"budget"`, `"inherit"` | Model selection preset for subagents | +| `model_profile` | string | `"balanced"` | `"quality"`, `"balanced"`, `"budget"`, `"adaptive"`, `"inherit"` | Model selection preset for subagents | | `mode` | string | `"interactive"` | `"interactive"`, `"yolo"` | Operation mode: `"interactive"` shows gates and confirmations; `"yolo"` runs autonomously without prompts | | `granularity` | string | (none) | `"coarse"`, `"standard"`, `"fine"` | Planning depth for phase plans (migrated from deprecated `depth`) | | `commit_docs` | boolean | `true` | `true`, `false` | Commit .planning/ artifacts to git (auto-false if .planning/ is gitignored) | diff --git a/get-shit-done/workflows/settings.md b/get-shit-done/workflows/settings.md index 851627bfe..d75c01948 100644 --- a/get-shit-done/workflows/settings.md +++ b/get-shit-done/workflows/settings.md @@ -104,18 +104,50 @@ Context Warnings, Research Qs **Conditional visibility — graphify.auto_update:** This question is shown only when the user's chosen `graphify.enabled` value is on. If `graphify.enabled` is off, omit the `graphify.auto_update` question and preserve the existing `graphify.auto_update` value in config (do not overwrite). Implementation: ask Graphify first; only ask Graph auto-update when Graphify is enabled. ``` +// Model profile is selected via a two-question split because AskUserQuestion enforces a +// hard 4-option cap and there are 5 valid profiles (quality, balanced, budget, adaptive, +// inherit). Q1 routes between adaptive/standard-tier/inherit; Q2 (shown only when the +// user chose "Standard tier" in Q1) picks among the three standard profiles. (#3784) AskUserQuestion([ { question: "Which model profile for agents?", header: "Model", multiSelect: false, options: [ - { label: "Quality", description: "Opus everywhere except verification (highest cost) — Claude only" }, - { label: "Balanced (Recommended)", description: "Opus for planning, Sonnet for research/execution/verification — Claude only" }, - { label: "Budget", description: "Sonnet for writing, Haiku for research/verification (lowest cost) — Claude only" }, + { label: "Adaptive (Recommended)", description: "Role-based cost optimization: heavy roles use the highest-tier model available on the active runtime, light roles use the cheapest. Best balance of quality and cost across all supported runtimes (Claude, Codex, Gemini, OpenRouter, local)." }, + { label: "Standard tier…", description: "Choose Quality, Balanced, or Budget — flat tier applied to all agents" }, { label: "Inherit", description: "Use current session model for all agents (required for non-Claude runtimes: Codex, Gemini CLI, OpenRouter, local models)" } ] - }, + } +]) + +**Conditional visibility — model_profile (Q2):** + Only ask this question when Q1's answer is "Standard tier…". + If Q1 = "Adaptive (Recommended)" → write model_profile=adaptive and SKIP Q2. + If Q1 = "Inherit" → write model_profile=inherit and SKIP Q2. + If user cancels Q2 after picking "Standard tier…" → leave existing model_profile value unchanged (mirror code_review_depth's cancellation rule). + +AskUserQuestion([ + { + question: "Which standard profile? (Quality / Balanced / Budget)", + header: "Model Tier", + multiSelect: false, + options: [ + { label: "Quality", description: "Opus everywhere except verification (highest cost) — Claude only" }, + { label: "Balanced", description: "Opus for planning, Sonnet for research/execution/verification — Claude only" }, + { label: "Budget", description: "Sonnet for writing, Haiku for research/verification (lowest cost) — Claude only" } + ] + } +]) + +// Map UI choices → config values: +// Q1 "Adaptive (Recommended)" → model_profile = "adaptive" +// Q1 "Inherit" → model_profile = "inherit" +// Q1 "Standard tier…" + Q2 "Quality" → model_profile = "quality" +// Q1 "Standard tier…" + Q2 "Balanced" → model_profile = "balanced" +// Q1 "Standard tier…" + Q2 "Budget" → model_profile = "budget" + +AskUserQuestion([ { question: "Spawn Plan Researcher? (researches domain before planning)", header: "Research", @@ -381,7 +413,7 @@ Merge new settings into existing config.json: } ``` -**Safe merge:** Apply each chosen value via `gsd-sdk query config-set ` so unrelated keys are never clobbered. `code_review_depth` is written only if the code_review question was answered `on`; otherwise leave the existing value in place. +**Safe merge:** Apply each chosen value via `gsd-sdk query config-set ` so unrelated keys are never clobbered. `code_review_depth` is written only if the code_review question was answered `on`; otherwise leave the existing value in place. `model_profile` is written on Q1 "Adaptive (Recommended)" (→ adaptive) or Q1 "Inherit" (→ inherit) immediately; for Q1 "Standard tier…", `model_profile` is written from Q2's answer. If Q1 = "Standard tier…" but Q2 is cancelled, leave the existing `model_profile` value unchanged — do not write any new value. Write updated config to `$GSD_CONFIG_PATH` (the workstream-aware path resolved in `ensure_and_load_config`). Never hardcode `.planning/config.json` — workstream installs route to `.planning/workstreams//config.json`. @@ -456,7 +488,7 @@ Display: | Setting | Value | |----------------------|-------| -| Model Profile | {quality/balanced/budget/inherit} | +| Model Profile | {quality/balanced/budget/adaptive/inherit} | | Plan Researcher | {On/Off} | | Plan Checker | {On/Off} | | Pattern Mapper | {On/Off} | @@ -495,7 +527,7 @@ Quick commands: - [ ] Current config read -- [ ] User presented with 23 settings (profile + workflow toggles + features + git branching + git tagging + ctx warnings), grouped into six sections: Planning, Execution, Docs & Output, Features, Model & Pipeline, Misc. `code_review_depth` is conditional on `code_review=on`. +- [ ] User presented with 23 settings (profile + workflow toggles + features + git branching + git tagging + ctx warnings), grouped into six sections: Planning, Execution, Docs & Output, Features, Model & Pipeline, Misc. `code_review_depth` is conditional on `code_review=on`. Model profile uses a two-question split (Q1: Adaptive / Standard tier / Inherit; Q2: Quality / Balanced / Budget — only when Standard tier chosen) to stay within the 4-option AskUserQuestion cap while exposing all 5 valid profiles (#3784). - [ ] Config updated with model_profile, workflow, and git sections - [ ] User offered to save as global defaults (~/.gsd/defaults.json) - [ ] Changes confirmed to user diff --git a/tests/bug-3784-gsd-settings-model-profile-ui-omits-adaptive.test.cjs b/tests/bug-3784-gsd-settings-model-profile-ui-omits-adaptive.test.cjs new file mode 100644 index 000000000..27861dafb --- /dev/null +++ b/tests/bug-3784-gsd-settings-model-profile-ui-omits-adaptive.test.cjs @@ -0,0 +1,198 @@ +'use strict'; + +// allow-test-rule: source-text-is-the-product +// The deployed settings.md IS the product — testing its text content tests the deployed contract. + +/** + * Regression test for bug #3784 + * + * /gsd-settings model profile UI omits `adaptive`. The AskUserQuestion block + * for model_profile in settings.md lists only four options (Quality, Balanced, + * Budget, Inherit) but the settings schema registers five valid profiles: + * quality, balanced, budget, adaptive, inherit. The `adaptive` profile is + * reachable by name via `gsd:config --profile adaptive` but cannot be selected + * interactively through `/gsd:settings`. + * + * Root cause: the options array in the model-profile AskUserQuestion block was + * written before the `adaptive` profile was introduced and was never updated. + * Because AskUserQuestion enforces a hard 4-option cap, the fix uses a two- + * question split: Q1 asks "Standard tier or Adaptive?" (2 options); if the + * user picks Standard, Q2 asks which of the three standard profiles to use + * (Quality, Balanced, Budget). This keeps every call within the 4-option cap + * while making all five profiles reachable. + * + * Fixes: #3784 + */ + +const { describe, test, before } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); + +const SETTINGS_PATH = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'settings.md'); + +/** + * Extract all AskUserQuestion option labels from a text block. + * Returns them lowercased for case-insensitive comparison. + */ +function extractOptionLabels(block) { + const labelPattern = /label:\s*"([^"]+)"/g; + const labels = []; + let match; + while ((match = labelPattern.exec(block)) !== null) { + labels.push(match[1].toLowerCase()); + } + return labels; +} + +describe('bug #3784: settings.md model profile UI exposes all 5 profiles', () => { + let content; + let presentBlock; + + before(() => { + content = fs.readFileSync(SETTINGS_PATH, 'utf-8'); + const presentMatch = content.match(/[\s\S]*?<\/step>/); + assert.ok(presentMatch, 'settings.md must have a present_settings step'); + presentBlock = presentMatch[0]; + }); + + // ── Core contract: all five valid profiles reachable via the settings UI ── + + test('present_settings step includes Adaptive as a selectable option (#3784)', () => { + // This is the primary assertion for bug #3784 — adaptive was missing. + const labels = extractOptionLabels(presentBlock); + assert.ok( + labels.some(l => l === 'adaptive' || l.startsWith('adaptive')), + [ + 'Bug #3784: present_settings step must include an "Adaptive" label in', + 'its model profile AskUserQuestion options so users can select it', + `interactively. Got labels: [${labels.join(', ')}]`, + ].join(' ') + ); + }); + + test('present_settings step includes Quality as a selectable option', () => { + const labels = extractOptionLabels(presentBlock); + assert.ok( + labels.some(l => l === 'quality' || l.startsWith('quality')), + `present_settings step must include a "Quality" option. Got: [${labels.join(', ')}]` + ); + }); + + test('present_settings step includes Balanced as a selectable option', () => { + const labels = extractOptionLabels(presentBlock); + assert.ok( + labels.some(l => l === 'balanced' || l.startsWith('balanced')), + `present_settings step must include a "Balanced" option. Got: [${labels.join(', ')}]` + ); + }); + + test('present_settings step includes Budget as a selectable option', () => { + const labels = extractOptionLabels(presentBlock); + assert.ok( + labels.some(l => l === 'budget' || l.startsWith('budget')), + `present_settings step must include a "Budget" option. Got: [${labels.join(', ')}]` + ); + }); + + test('present_settings step includes Inherit as a selectable option', () => { + const labels = extractOptionLabels(presentBlock); + assert.ok( + labels.some(l => l === 'inherit' || l.startsWith('inherit')), + `present_settings step must include an "Inherit" option. Got: [${labels.join(', ')}]` + ); + }); + + // ── update_config step writes adaptive as a valid value ── + + test('update_config step lists adaptive as a valid model_profile value', () => { + const updateMatch = content.match(/[\s\S]*?<\/step>/); + assert.ok(updateMatch, 'settings.md must have an update_config step'); + const block = updateMatch[0]; + assert.ok( + block.includes('adaptive'), + 'update_config step must list "adaptive" as a valid model_profile value' + ); + }); + + // ── confirm step displays adaptive as a possible profile value ── + + test('confirm step table shows adaptive as a possible model profile value', () => { + const confirmMatch = content.match(/[\s\S]*?<\/step>/); + assert.ok(confirmMatch, 'settings.md must have a confirm step'); + const block = confirmMatch[0]; + assert.ok( + block.includes('adaptive'), + 'confirm step must include "adaptive" in the Model Profile row placeholder' + ); + }); + + // ── adaptive described with role-based routing semantics ── + + test('settings.md describes adaptive profile with role-based routing semantics', () => { + // Adaptive uses heavy/light role tiers per routingTier. + // The UI description must convey role-based cost optimization and the heavy/light tier + // split — not just mention "Adaptive" somewhere (that word appears 6+ times in the file). + const lower = content.toLowerCase(); + assert.ok( + lower.includes('role-based cost optimization') && lower.includes('heavy roles'), + 'settings.md must describe the adaptive profile with "role-based cost optimization" and "heavy roles" wording so the description is meaningful across all supported runtimes' + ); + }); + + // ── 4-option cap enforcement ── + + test('each question object in present_settings AskUserQuestion blocks has at most 4 options (AskUserQuestion runtime cap)', () => { + // The AskUserQuestion runtime enforces a hard 4-option cap per individual question object + // (each { question:..., options:[...] } entry). This test guards against a naïve revert + // that puts all 5 profiles into a single question object instead of using the Q1/Q2 split. + const ASK_USER_QUESTION_OPTION_CAP = 4; // hard limit enforced by the AskUserQuestion runtime + + // Extract each individual options array by finding 'options: [' and walking to the + // matching balanced ']', then count label: entries within that span. + const optionsKeyRe = /\boptions\s*:\s*\[/g; + let match; + let questionIndex = 0; + while ((match = optionsKeyRe.exec(presentBlock)) !== null) { + questionIndex++; + // Walk forward from the opening '[' to find the balanced close ']'. + let depth = 0; + const start = match.index + match[0].length - 1; // points at '[' + let end = start; + for (let k = start; k < presentBlock.length; k++) { + if (presentBlock[k] === '[') { depth++; } + else if (presentBlock[k] === ']') { + depth--; + if (depth === 0) { end = k; break; } + } + } + const optionsBody = presentBlock.slice(start, end + 1); + const labelMatches = optionsBody.match(/label:\s*"[^"]+"/g) || []; + const optionCount = labelMatches.length; + assert.ok( + optionCount <= ASK_USER_QUESTION_OPTION_CAP, + `Question object ${questionIndex} in present_settings has ${optionCount} options — exceeds the runtime cap of ${ASK_USER_QUESTION_OPTION_CAP}. Split into multiple questions (as #3784 did for model_profile).` + ); + } + // Sanity check: there must be at least one options array found. + assert.ok(questionIndex > 0, 'present_settings must contain at least one AskUserQuestion options array'); + }); + + // ── Brace-balance regression (bd53925f fixed duplicate '{' from 35fc1d21) ── + + test('present_settings step has balanced braces — regression: brace-balance after #3784 split', () => { + // commit bd53925f fixed a duplicate '{' introduced by 35fc1d21 when the model-profile + // AskUserQuestion was split into Q1+Q2. This test guards against a recurrence. + let depth = 0; + const maxDepth = 0; + for (const ch of presentBlock) { + if (ch === '{') { depth++; } + if (ch === '}') { depth--; } + } + assert.strictEqual( + depth, + 0, + `present_settings step has unbalanced braces: net depth after full scan is ${depth} (positive = extra '{', negative = extra '}'). Regression guard for bd53925f / #3784.` + ); + }); +});