refactor(coding-agent): restructured validation and rendering for consistency
- Refactored theme color validation to use single source of truth with THEME_COLOR_RECORD object. - Simplified model registry to defer per-model overrides to dedicated method and use constant for role IDs. - Refactored tree list rendering to pre-render items once for consistent line counts across phases. - Refactored question result formatting to use early returns and consistently include question ID in output. - Updated hook editor hint text to include ctrl+g external editor option when prompt style is enabled. - Removed unused isLogicalLineStart property from LayoutLine interface in editor component.
This commit is contained in:
can1357
@@ -1,11 +1,15 @@
|
||||
# Changelog
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Added
|
||||
|
||||
- Added `browser.screenshotDir` setting to configure screenshot save directory with path expansion
|
||||
|
||||
### Changed
|
||||
|
||||
- Updated hook editor hint text to include `ctrl+g external editor` option when using prompt style
|
||||
- Refactored question result formatting to consistently include question ID in output
|
||||
|
||||
## [13.15.3] - 2026-03-26
|
||||
|
||||
### Added
|
||||
@@ -6334,4 +6338,4 @@ Initial public release.
|
||||
- Git branch display in footer
|
||||
- Message queueing during streaming responses
|
||||
- OAuth integration for Gmail and Google Calendar access
|
||||
- HTML export with syntax highlighting and collapsible sections
|
||||
- HTML export with syntax highlighting and collapsible sections
|
||||
@@ -69,9 +69,7 @@ export type RoleInfo = ModelRoleInfo;
|
||||
* entries across settings.
|
||||
*/
|
||||
export function getKnownRoleIds(settings: Settings): string[] {
|
||||
// Avoid MODEL_ROLE_IDS here: this helper is reached during selector initialization,
|
||||
// and model-registry participates in import cycles while the module is still evaluating.
|
||||
const roles = ["default", "smol", "slow", "vision", "plan", "commit", "task"];
|
||||
const roles = [...MODEL_ROLE_IDS] as string[];
|
||||
const seen = new Set<string>(roles);
|
||||
const addRole = (role: string) => {
|
||||
if (seen.has(role)) return;
|
||||
@@ -832,7 +830,7 @@ export class ModelRegistry {
|
||||
this.#modelOverrides = modelOverrides;
|
||||
|
||||
this.#addImplicitDiscoverableProviders(configuredProviders);
|
||||
const builtInModels = this.#applyHardcodedModelPolicies(this.#loadBuiltInModels(overrides, modelOverrides));
|
||||
const builtInModels = this.#applyHardcodedModelPolicies(this.#loadBuiltInModels(overrides));
|
||||
const cachedDiscoveries = this.#applyHardcodedModelPolicies(this.#loadCachedDiscoverableModels());
|
||||
const resolvedDefaults = this.#mergeResolvedModels(builtInModels, cachedDiscoveries);
|
||||
const combined = this.#mergeCustomModels(resolvedDefaults, this.#customModelOverlays);
|
||||
@@ -840,31 +838,21 @@ export class ModelRegistry {
|
||||
this.#models = this.#applyModelOverrides(combined, this.#modelOverrides);
|
||||
}
|
||||
|
||||
/** Load built-in models, applying provider and per-model overrides */
|
||||
#loadBuiltInModels(
|
||||
overrides: Map<string, ProviderOverride>,
|
||||
modelOverrides: Map<string, Map<string, ModelOverride>>,
|
||||
): Model<Api>[] {
|
||||
/** Load built-in models, applying provider-level overrides only.
|
||||
* Per-model overrides are applied later by #applyModelOverrides. */
|
||||
#loadBuiltInModels(overrides: Map<string, ProviderOverride>): Model<Api>[] {
|
||||
return getBundledProviders().flatMap(provider => {
|
||||
const models = getBundledModels(provider as Parameters<typeof getBundledModels>[0]) as Model<Api>[];
|
||||
const providerOverride = overrides.get(provider);
|
||||
const perModelOverrides = modelOverrides.get(provider);
|
||||
|
||||
return models.map(m => {
|
||||
let model = m;
|
||||
if (providerOverride) {
|
||||
model = {
|
||||
...model,
|
||||
baseUrl: providerOverride.baseUrl ?? model.baseUrl,
|
||||
headers: providerOverride.headers ? { ...model.headers, ...providerOverride.headers } : model.headers,
|
||||
compat: mergeCompat(model.compat, providerOverride.compat),
|
||||
};
|
||||
}
|
||||
const modelOverride = perModelOverrides?.get(m.id);
|
||||
if (modelOverride) {
|
||||
model = applyModelOverride(model, modelOverride);
|
||||
}
|
||||
return model;
|
||||
if (!providerOverride) return m;
|
||||
return {
|
||||
...m,
|
||||
baseUrl: providerOverride.baseUrl ?? m.baseUrl,
|
||||
headers: providerOverride.headers ? { ...m.headers, ...providerOverride.headers } : m.headers,
|
||||
compat: mergeCompat(m.compat, providerOverride.compat),
|
||||
};
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
@@ -62,7 +62,7 @@ export class HookEditorComponent extends Container {
|
||||
|
||||
// Hint
|
||||
const hint = this.#promptStyle
|
||||
? "enter submit esc cancel"
|
||||
? "enter submit esc cancel ctrl+g external editor"
|
||||
: "ctrl+enter submit esc cancel ctrl+g external editor";
|
||||
this.addChild(new Text(theme.fg("dim", hint), 1, 0));
|
||||
|
||||
|
||||
@@ -956,67 +956,69 @@ export type ThemeColor =
|
||||
| "statusLineSubagents";
|
||||
|
||||
/** Set of all valid ThemeColor string values for runtime validation */
|
||||
const VALID_THEME_COLORS: ReadonlySet<string> = new Set([
|
||||
"accent",
|
||||
"border",
|
||||
"borderAccent",
|
||||
"borderMuted",
|
||||
"success",
|
||||
"error",
|
||||
"warning",
|
||||
"muted",
|
||||
"dim",
|
||||
"text",
|
||||
"thinkingText",
|
||||
"userMessageText",
|
||||
"customMessageText",
|
||||
"customMessageLabel",
|
||||
"toolTitle",
|
||||
"toolOutput",
|
||||
"mdHeading",
|
||||
"mdLink",
|
||||
"mdLinkUrl",
|
||||
"mdCode",
|
||||
"mdCodeBlock",
|
||||
"mdCodeBlockBorder",
|
||||
"mdQuote",
|
||||
"mdQuoteBorder",
|
||||
"mdHr",
|
||||
"mdListBullet",
|
||||
"toolDiffAdded",
|
||||
"toolDiffRemoved",
|
||||
"toolDiffContext",
|
||||
"syntaxComment",
|
||||
"syntaxKeyword",
|
||||
"syntaxFunction",
|
||||
"syntaxVariable",
|
||||
"syntaxString",
|
||||
"syntaxNumber",
|
||||
"syntaxType",
|
||||
"syntaxOperator",
|
||||
"syntaxPunctuation",
|
||||
"thinkingOff",
|
||||
"thinkingMinimal",
|
||||
"thinkingLow",
|
||||
"thinkingMedium",
|
||||
"thinkingHigh",
|
||||
"thinkingXhigh",
|
||||
"bashMode",
|
||||
"pythonMode",
|
||||
"statusLineSep",
|
||||
"statusLineModel",
|
||||
"statusLinePath",
|
||||
"statusLineGitClean",
|
||||
"statusLineGitDirty",
|
||||
"statusLineContext",
|
||||
"statusLineSpend",
|
||||
"statusLineStaged",
|
||||
"statusLineDirty",
|
||||
"statusLineUntracked",
|
||||
"statusLineOutput",
|
||||
"statusLineCost",
|
||||
"statusLineSubagents",
|
||||
]);
|
||||
const THEME_COLOR_RECORD = {
|
||||
accent: true,
|
||||
border: true,
|
||||
borderAccent: true,
|
||||
borderMuted: true,
|
||||
success: true,
|
||||
error: true,
|
||||
warning: true,
|
||||
muted: true,
|
||||
dim: true,
|
||||
text: true,
|
||||
thinkingText: true,
|
||||
userMessageText: true,
|
||||
customMessageText: true,
|
||||
customMessageLabel: true,
|
||||
toolTitle: true,
|
||||
toolOutput: true,
|
||||
mdHeading: true,
|
||||
mdLink: true,
|
||||
mdLinkUrl: true,
|
||||
mdCode: true,
|
||||
mdCodeBlock: true,
|
||||
mdCodeBlockBorder: true,
|
||||
mdQuote: true,
|
||||
mdQuoteBorder: true,
|
||||
mdHr: true,
|
||||
mdListBullet: true,
|
||||
toolDiffAdded: true,
|
||||
toolDiffRemoved: true,
|
||||
toolDiffContext: true,
|
||||
syntaxComment: true,
|
||||
syntaxKeyword: true,
|
||||
syntaxFunction: true,
|
||||
syntaxVariable: true,
|
||||
syntaxString: true,
|
||||
syntaxNumber: true,
|
||||
syntaxType: true,
|
||||
syntaxOperator: true,
|
||||
syntaxPunctuation: true,
|
||||
thinkingOff: true,
|
||||
thinkingMinimal: true,
|
||||
thinkingLow: true,
|
||||
thinkingMedium: true,
|
||||
thinkingHigh: true,
|
||||
thinkingXhigh: true,
|
||||
bashMode: true,
|
||||
pythonMode: true,
|
||||
statusLineSep: true,
|
||||
statusLineModel: true,
|
||||
statusLinePath: true,
|
||||
statusLineGitClean: true,
|
||||
statusLineGitDirty: true,
|
||||
statusLineContext: true,
|
||||
statusLineSpend: true,
|
||||
statusLineStaged: true,
|
||||
statusLineDirty: true,
|
||||
statusLineUntracked: true,
|
||||
statusLineOutput: true,
|
||||
statusLineCost: true,
|
||||
statusLineSubagents: true,
|
||||
} satisfies Record<ThemeColor, true>;
|
||||
|
||||
const VALID_THEME_COLORS: ReadonlySet<string> = new Set(Object.keys(THEME_COLOR_RECORD));
|
||||
|
||||
/** Check if a string is a valid ThemeColor value */
|
||||
export function isValidThemeColor(color: string): color is ThemeColor {
|
||||
|
||||
@@ -354,14 +354,15 @@ async function askSingleQuestion(
|
||||
}
|
||||
|
||||
function formatQuestionResult(result: QuestionResult): string {
|
||||
const parts: string[] = [];
|
||||
if (result.selectedOptions.length > 0) {
|
||||
parts.push(result.multi ? `[${result.selectedOptions.join(", ")}]` : result.selectedOptions[0]!);
|
||||
}
|
||||
if (result.customInput !== undefined) {
|
||||
parts.push(`"${result.customInput}"`);
|
||||
return `${result.id}: "${result.customInput}"`;
|
||||
}
|
||||
return parts.length > 0 ? `${result.id}: ${parts.join(", ")}` : `${result.id}: (cancelled)`;
|
||||
if (result.selectedOptions.length > 0) {
|
||||
return result.multi
|
||||
? `${result.id}: [${result.selectedOptions.join(", ")}]`
|
||||
: `${result.id}: ${result.selectedOptions[0]}`;
|
||||
}
|
||||
return `${result.id}: (cancelled)`;
|
||||
}
|
||||
|
||||
// =============================================================================
|
||||
|
||||
@@ -15,6 +15,8 @@ export interface TreeListOptions<T> {
|
||||
*/
|
||||
maxCollapsedLines?: number;
|
||||
itemType?: string;
|
||||
/** Called once per item with `isLast: false` during budget calculation;
|
||||
* line count MUST NOT vary based on `isLast`. */
|
||||
renderItem: (item: T, context: TreeContext) => string | string[];
|
||||
}
|
||||
|
||||
@@ -23,22 +25,29 @@ export function renderTreeList<T>(options: TreeListOptions<T>, theme: Theme): st
|
||||
const maxItems = expanded ? items.length : Math.min(items.length, maxCollapsed);
|
||||
const linesBudget = !expanded && maxCollapsedLines !== undefined ? maxCollapsedLines : Infinity;
|
||||
|
||||
// Pass 1: determine how many items fit within both the item count and total line budget,
|
||||
// including the trailing summary row when one is needed.
|
||||
// Pre-render each candidate item once.
|
||||
// isLast cannot be known at this point (fittingCount is not yet determined);
|
||||
// renderItem implementations MUST NOT vary line count based on isLast.
|
||||
const preRendered: string[][] = [];
|
||||
for (let i = 0; i < maxItems; i++) {
|
||||
const rendered = renderItem(items[i], {
|
||||
index: i,
|
||||
isLast: false,
|
||||
depth: 0,
|
||||
theme,
|
||||
prefix: "",
|
||||
continuePrefix: "",
|
||||
});
|
||||
preRendered.push(Array.isArray(rendered) ? rendered : rendered ? [rendered] : []);
|
||||
}
|
||||
|
||||
// Determine how many items fit within the line budget.
|
||||
let fittingCount = maxItems;
|
||||
let fittedLineCount = 0;
|
||||
if (linesBudget !== Infinity) {
|
||||
fittingCount = 0;
|
||||
for (let i = 0; i < maxItems; i++) {
|
||||
const rendered = renderItem(items[i], {
|
||||
index: i,
|
||||
isLast: false,
|
||||
depth: 0,
|
||||
theme,
|
||||
prefix: "",
|
||||
continuePrefix: "",
|
||||
});
|
||||
const count = Array.isArray(rendered) ? rendered.length : rendered ? 1 : 0;
|
||||
const count = preRendered[i]!.length;
|
||||
const remainingAfter = items.length - (i + 1);
|
||||
const reservedSummaryLines = remainingAfter > 0 ? 1 : 0;
|
||||
if (fittedLineCount + count + reservedSummaryLines > linesBudget) break;
|
||||
@@ -50,30 +59,18 @@ export function renderTreeList<T>(options: TreeListOptions<T>, theme: Theme): st
|
||||
const remaining = items.length - fittingCount;
|
||||
const hasSummary = !expanded && remaining > 0 && (linesBudget === Infinity || fittedLineCount < linesBudget);
|
||||
|
||||
// Pass 2: render items with correct isLast and prefixes.
|
||||
// Emit pre-rendered content with correct isLast-based branch prefixes.
|
||||
const lines: string[] = [];
|
||||
for (let i = 0; i < fittingCount; i++) {
|
||||
const isLast = !hasSummary && i === fittingCount - 1;
|
||||
const branch = getTreeBranch(isLast, theme);
|
||||
const prefix = `${theme.fg("dim", branch)} `;
|
||||
const continuePrefix = `${theme.fg("dim", getTreeContinuePrefix(isLast, theme))}`;
|
||||
const context: TreeContext = {
|
||||
index: i,
|
||||
isLast,
|
||||
depth: 0,
|
||||
theme,
|
||||
prefix,
|
||||
continuePrefix,
|
||||
};
|
||||
const rendered = renderItem(items[i], context);
|
||||
if (Array.isArray(rendered)) {
|
||||
if (rendered.length === 0) continue;
|
||||
lines.push(`${prefix}${replaceTabs(rendered[0])}`);
|
||||
for (let j = 1; j < rendered.length; j++) {
|
||||
lines.push(`${continuePrefix}${replaceTabs(rendered[j])}`);
|
||||
}
|
||||
} else {
|
||||
lines.push(`${prefix}${replaceTabs(rendered)}`);
|
||||
const itemLines = preRendered[i]!;
|
||||
if (itemLines.length === 0) continue;
|
||||
lines.push(`${prefix}${replaceTabs(itemLines[0]!)}`);
|
||||
for (let j = 1; j < itemLines.length; j++) {
|
||||
lines.push(`${continuePrefix}${replaceTabs(itemLines[j]!)}`);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -281,7 +281,6 @@ interface LayoutLine {
|
||||
text: string;
|
||||
hasCursor: boolean;
|
||||
cursorPos?: number;
|
||||
isLogicalLineStart: boolean;
|
||||
}
|
||||
|
||||
export interface EditorTheme {
|
||||
@@ -1232,7 +1231,6 @@ export class Editor implements Component, Focusable {
|
||||
text: "",
|
||||
hasCursor: true,
|
||||
cursorPos: 0,
|
||||
isLogicalLineStart: true,
|
||||
});
|
||||
return layoutLines;
|
||||
}
|
||||
@@ -1250,13 +1248,11 @@ export class Editor implements Component, Focusable {
|
||||
text: line,
|
||||
hasCursor: true,
|
||||
cursorPos: this.#state.cursorCol,
|
||||
isLogicalLineStart: true,
|
||||
});
|
||||
} else {
|
||||
layoutLines.push({
|
||||
text: line,
|
||||
hasCursor: false,
|
||||
isLogicalLineStart: true,
|
||||
});
|
||||
}
|
||||
} else {
|
||||
@@ -1300,13 +1296,11 @@ export class Editor implements Component, Focusable {
|
||||
text: chunk.text,
|
||||
hasCursor: true,
|
||||
cursorPos: adjustedCursorPos,
|
||||
isLogicalLineStart: chunkIndex === 0,
|
||||
});
|
||||
} else {
|
||||
layoutLines.push({
|
||||
text: chunk.text,
|
||||
hasCursor: false,
|
||||
isLogicalLineStart: chunkIndex === 0,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user