feat(tui): added shift+enter summarize-and-switch to the session tree selector
- Shift+Enter on a tree entry forks with a branch summary directly, skipping the summary prompt and ignoring the branchSummary.enabled gate. - Plain Enter keeps existing behavior: direct switch, with the summary prompt only for users who enabled branchSummary.enabled. - Updated the selector help line and added controller-level regression tests. Fixes #5152
This commit is contained in:
@@ -9,6 +9,7 @@
|
|||||||
- Added an optional `provider` parameter to `generate_image` (`auto` | `openai` | `openai-codex` | `antigravity` | `xai` | `gemini` | `openrouter`) that overrides the `providers.image` setting **for a single request** — so "generate this using gemini / codex / xai" routes per-call without changing the global setting. Absent → the `providers.image` setting applies, unchanged; the named provider uses the same resolution semantics (falls back to auto-detect if it has no credentials). File: `tools/image-gen.ts` (`imageProviderSchema`, `findImageApiKey` `preference` arg).
|
- Added an optional `provider` parameter to `generate_image` (`auto` | `openai` | `openai-codex` | `antigravity` | `xai` | `gemini` | `openrouter`) that overrides the `providers.image` setting **for a single request** — so "generate this using gemini / codex / xai" routes per-call without changing the global setting. Absent → the `providers.image` setting applies, unchanged; the named provider uses the same resolution semantics (falls back to auto-detect if it has no credentials). File: `tools/image-gen.ts` (`imageProviderSchema`, `findImageApiKey` `preference` arg).
|
||||||
- Added OpenTelemetry log and metric export alongside the existing trace export. When `OTEL_EXPORTER_OTLP_LOGS_ENDPOINT` (or the shared `OTEL_EXPORTER_OTLP_ENDPOINT`) is set, `omp` registers a `LoggerProvider` and forwards every centralized-logger event as an OTLP log record (severity + attributes + active span context for log↔trace correlation, min level via `OTEL_LOG_LEVEL`, plus a structured `agent run completed` summary event). When `OTEL_EXPORTER_OTLP_METRICS_ENDPOINT` (or the shared endpoint) is set, it registers a `MeterProvider` with a `PeriodicExportingMetricReader` and records GenAI-semconv `gen_ai.client.token.usage` plus `pi.omp.agent.*` counters/histograms (runs, steps, chat/tool calls by name+status+finish reason, latencies, estimated cost, errors) from the agent run summary and per-chat usage hooks. Each signal honors its own `OTEL_*_EXPORTER=none` kill switch, the global `OTEL_SDK_DISABLED`, and declines non-`http/protobuf` protocols independently ([#4604](https://github.com/can1357/oh-my-pi/issues/4604)).
|
- Added OpenTelemetry log and metric export alongside the existing trace export. When `OTEL_EXPORTER_OTLP_LOGS_ENDPOINT` (or the shared `OTEL_EXPORTER_OTLP_ENDPOINT`) is set, `omp` registers a `LoggerProvider` and forwards every centralized-logger event as an OTLP log record (severity + attributes + active span context for log↔trace correlation, min level via `OTEL_LOG_LEVEL`, plus a structured `agent run completed` summary event). When `OTEL_EXPORTER_OTLP_METRICS_ENDPOINT` (or the shared endpoint) is set, it registers a `MeterProvider` with a `PeriodicExportingMetricReader` and records GenAI-semconv `gen_ai.client.token.usage` plus `pi.omp.agent.*` counters/histograms (runs, steps, chat/tool calls by name+status+finish reason, latencies, estimated cost, errors) from the agent run summary and per-chat usage hooks. Each signal honors its own `OTEL_*_EXPORTER=none` kill switch, the global `OTEL_SDK_DISABLED`, and declines non-`http/protobuf` protocols independently ([#4604](https://github.com/can1357/oh-my-pi/issues/4604)).
|
||||||
- `retry.fallbackChains` wildcards now support id-prefixed targets and keys: a chain entry like `"openrouter/google/*"` re-prefixes the failing model's bare id (`google-antigravity/gemini-x` → `openrouter/google/gemini-x`), a plain `"provider/*"` entry falling back *from* an aggregator strips the vendor prefix when the target provider only knows the bare id (`openrouter/google/x` → `google-vertex/x`), and an id-prefixed key (`"openrouter/google/*"`) scopes a chain to that provider's ids under the prefix.
|
- `retry.fallbackChains` wildcards now support id-prefixed targets and keys: a chain entry like `"openrouter/google/*"` re-prefixes the failing model's bare id (`google-antigravity/gemini-x` → `openrouter/google/gemini-x`), a plain `"provider/*"` entry falling back *from* an aggregator strips the vendor prefix when the target provider only knows the bare id (`openrouter/google/x` → `google-vertex/x`), and an id-prefixed key (`"openrouter/google/*"`) scopes a chain to that provider's ids under the prefix.
|
||||||
|
- The session tree selector (`/tree`, `/branch`) now supports Shift+Enter to summarize-and-switch in one step: it forks from the selected entry with a branch summary, with no extra prompt and regardless of `branchSummary.enabled`. Plain Enter keeps the current behavior (direct switch by default; the summary prompt only when `branchSummary.enabled` is on). ([#5152](https://github.com/can1357/oh-my-pi/issues/5152))
|
||||||
|
|
||||||
### Changed
|
### Changed
|
||||||
|
|
||||||
|
|||||||
@@ -66,7 +66,7 @@ class TreeList implements Component {
|
|||||||
#activePathIds: Set<string> = new Set();
|
#activePathIds: Set<string> = new Set();
|
||||||
#lastSelectedId: string | null = null;
|
#lastSelectedId: string | null = null;
|
||||||
|
|
||||||
onSelect?: (entryId: string) => void;
|
onSelect?: (entryId: string, options: { summarize: boolean }) => void;
|
||||||
onCancel?: () => void;
|
onCancel?: () => void;
|
||||||
onLabelEdit?: (entryId: string, currentLabel: string | undefined) => void;
|
onLabelEdit?: (entryId: string, currentLabel: string | undefined) => void;
|
||||||
|
|
||||||
@@ -792,10 +792,16 @@ class TreeList implements Component {
|
|||||||
} else if (matchesKey(keyData, "right")) {
|
} else if (matchesKey(keyData, "right")) {
|
||||||
// Page down
|
// Page down
|
||||||
this.#selectedIndex = Math.min(this.#filteredNodes.length - 1, this.#selectedIndex + this.maxVisibleLines);
|
this.#selectedIndex = Math.min(this.#filteredNodes.length - 1, this.#selectedIndex + this.maxVisibleLines);
|
||||||
|
} else if (matchesKey(keyData, "shift+enter") || matchesKey(keyData, "shift+return")) {
|
||||||
|
// Summarize-and-switch: fork with a branch summary without the extra prompt.
|
||||||
|
const selected = this.#filteredNodes[this.#selectedIndex];
|
||||||
|
if (selected && this.onSelect) {
|
||||||
|
this.onSelect(selected.node.entry.id, { summarize: true });
|
||||||
|
}
|
||||||
} else if (matchesKey(keyData, "enter") || matchesKey(keyData, "return") || keyData === "\n") {
|
} else if (matchesKey(keyData, "enter") || matchesKey(keyData, "return") || keyData === "\n") {
|
||||||
const selected = this.#filteredNodes[this.#selectedIndex];
|
const selected = this.#filteredNodes[this.#selectedIndex];
|
||||||
if (selected && this.onSelect) {
|
if (selected && this.onSelect) {
|
||||||
this.onSelect(selected.node.entry.id);
|
this.onSelect(selected.node.entry.id, { summarize: false });
|
||||||
}
|
}
|
||||||
} else if (matchesAppInterrupt(keyData)) {
|
} else if (matchesAppInterrupt(keyData)) {
|
||||||
if (this.#searchQuery) {
|
if (this.#searchQuery) {
|
||||||
@@ -923,7 +929,7 @@ export class TreeSelectorComponent extends Container {
|
|||||||
tree: SessionTreeNode[],
|
tree: SessionTreeNode[],
|
||||||
currentLeafId: string | null,
|
currentLeafId: string | null,
|
||||||
terminalHeight: number,
|
terminalHeight: number,
|
||||||
onSelect: (entryId: string) => void,
|
onSelect: (entryId: string, options: { summarize: boolean }) => void,
|
||||||
onCancel: () => void,
|
onCancel: () => void,
|
||||||
private readonly onLabelChangeCallback?: (entryId: string, label: string | undefined) => void,
|
private readonly onLabelChangeCallback?: (entryId: string, label: string | undefined) => void,
|
||||||
initialFilterMode: FilterMode = "default",
|
initialFilterMode: FilterMode = "default",
|
||||||
@@ -948,7 +954,7 @@ export class TreeSelectorComponent extends Container {
|
|||||||
new TruncatedText(
|
new TruncatedText(
|
||||||
theme.fg(
|
theme.fg(
|
||||||
"muted",
|
"muted",
|
||||||
"Up/Down: move. Left/Right: page. Shift+L: label. Ctrl+O/Shift+Ctrl+O: filter. Alt+D/T/U/L/A: filter. Type to search",
|
"Enter: switch. Shift+Enter: summarize & switch. Shift+L: label. Ctrl+O: filter. Alt+D/T/U/L/A: filter. Type to search",
|
||||||
),
|
),
|
||||||
0,
|
0,
|
||||||
0,
|
0,
|
||||||
|
|||||||
@@ -1137,7 +1137,7 @@ export class SelectorController {
|
|||||||
tree,
|
tree,
|
||||||
realLeafId,
|
realLeafId,
|
||||||
this.ctx.ui.terminal.rows,
|
this.ctx.ui.terminal.rows,
|
||||||
async entryId => {
|
async (entryId, options) => {
|
||||||
// Selecting the current leaf is a no-op (already there)
|
// Selecting the current leaf is a no-op (already there)
|
||||||
if (entryId === realLeafId) {
|
if (entryId === realLeafId) {
|
||||||
done();
|
done();
|
||||||
@@ -1148,13 +1148,15 @@ export class SelectorController {
|
|||||||
// Ask about summarization
|
// Ask about summarization
|
||||||
done(); // Close selector first
|
done(); // Close selector first
|
||||||
|
|
||||||
// Loop until user makes a complete choice or cancels to tree
|
// Loop until user makes a complete choice or cancels to tree.
|
||||||
let wantsSummary = false;
|
// Shift+Enter in the tree selector pre-answers "Summarize" and
|
||||||
|
// skips the prompt entirely.
|
||||||
|
let wantsSummary = options.summarize;
|
||||||
let customInstructions: string | undefined;
|
let customInstructions: string | undefined;
|
||||||
|
|
||||||
const branchSummariesEnabled = settings.get("branchSummary.enabled");
|
const branchSummariesEnabled = settings.get("branchSummary.enabled");
|
||||||
|
|
||||||
while (branchSummariesEnabled) {
|
while (!wantsSummary && branchSummariesEnabled) {
|
||||||
const summaryChoice = await this.ctx.showHookSelector("Summarize branch?", [
|
const summaryChoice = await this.ctx.showHookSelector("Summarize branch?", [
|
||||||
"No summary",
|
"No summary",
|
||||||
"Summarize",
|
"Summarize",
|
||||||
|
|||||||
@@ -0,0 +1,184 @@
|
|||||||
|
import { afterEach, beforeAll, beforeEach, describe, expect, it, type Mock, vi } from "bun:test";
|
||||||
|
import type { AgentMessage } from "@oh-my-pi/pi-agent-core";
|
||||||
|
import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
|
||||||
|
import { SelectorController } from "@oh-my-pi/pi-coding-agent/modes/controllers/selector-controller";
|
||||||
|
import { initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme";
|
||||||
|
import type { InteractiveModeContext } from "@oh-my-pi/pi-coding-agent/modes/types";
|
||||||
|
import type { SessionTreeNode } from "@oh-my-pi/pi-coding-agent/session/session-entries";
|
||||||
|
import { setKittyProtocolActive } from "@oh-my-pi/pi-tui/keys";
|
||||||
|
import { beginSettingsTest, restoreSettingsTestState, type SettingsTestState } from "./helpers/settings-test-state";
|
||||||
|
|
||||||
|
const SHIFT_ENTER = "\x1b[13;2u";
|
||||||
|
|
||||||
|
let settingsState: SettingsTestState | undefined;
|
||||||
|
|
||||||
|
beforeAll(() => {
|
||||||
|
initTheme();
|
||||||
|
});
|
||||||
|
|
||||||
|
beforeEach(async () => {
|
||||||
|
settingsState = beginSettingsTest();
|
||||||
|
await Settings.init({ inMemory: true });
|
||||||
|
setKittyProtocolActive(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
setKittyProtocolActive(false);
|
||||||
|
restoreSettingsTestState(settingsState);
|
||||||
|
settingsState = undefined;
|
||||||
|
});
|
||||||
|
|
||||||
|
function userNode(id: string, parentId: string | null, text: string): SessionTreeNode {
|
||||||
|
const message: AgentMessage = { role: "user", content: text, timestamp: 1 };
|
||||||
|
return {
|
||||||
|
entry: {
|
||||||
|
type: "message",
|
||||||
|
id,
|
||||||
|
parentId,
|
||||||
|
timestamp: "2026-01-01T00:00:00Z",
|
||||||
|
message,
|
||||||
|
},
|
||||||
|
children: [],
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
type NavigateTree = (
|
||||||
|
entryId: string,
|
||||||
|
options: { summarize: boolean; customInstructions: string | undefined },
|
||||||
|
) => Promise<{ cancelled: boolean }>;
|
||||||
|
type ShowHookSelector = (title: string, options: string[]) => Promise<string | undefined>;
|
||||||
|
|
||||||
|
interface TreeSummaryHarness {
|
||||||
|
controller: SelectorController;
|
||||||
|
navigateTree: Mock<NavigateTree>;
|
||||||
|
navigation: Promise<void>;
|
||||||
|
selector(): { handleInput(key: string): void };
|
||||||
|
showHookSelector: Mock<ShowHookSelector>;
|
||||||
|
}
|
||||||
|
|
||||||
|
function createHarness(summaryChoice = "No summary"): TreeSummaryHarness {
|
||||||
|
const navigation = Promise.withResolvers<void>();
|
||||||
|
const root = userNode("root", null, "Root prompt");
|
||||||
|
const showHookSelector = vi.fn<ShowHookSelector>(async () => summaryChoice);
|
||||||
|
const navigateTree = vi.fn<NavigateTree>(async () => {
|
||||||
|
navigation.resolve();
|
||||||
|
return { cancelled: false };
|
||||||
|
});
|
||||||
|
let selector: { handleInput(key: string): void } | undefined;
|
||||||
|
const ctx = {
|
||||||
|
sessionManager: {
|
||||||
|
getTree: () => [root],
|
||||||
|
getLeafId: () => null,
|
||||||
|
appendLabelChange: vi.fn(),
|
||||||
|
},
|
||||||
|
ui: {
|
||||||
|
terminal: { rows: 40 },
|
||||||
|
setFocus: vi.fn(),
|
||||||
|
requestRender: vi.fn(),
|
||||||
|
requestComponentRender: vi.fn(),
|
||||||
|
},
|
||||||
|
editorContainer: {
|
||||||
|
clear: vi.fn(),
|
||||||
|
addChild: vi.fn(),
|
||||||
|
},
|
||||||
|
editor: {
|
||||||
|
getText: () => "",
|
||||||
|
setText: vi.fn(),
|
||||||
|
onEscape: undefined,
|
||||||
|
},
|
||||||
|
showStatus: vi.fn(),
|
||||||
|
showError: vi.fn(),
|
||||||
|
showHookSelector,
|
||||||
|
showHookEditor: vi.fn(),
|
||||||
|
chatContainer: { addChild: vi.fn() },
|
||||||
|
statusContainer: {
|
||||||
|
addChild: vi.fn(),
|
||||||
|
disposeChildren: vi.fn(),
|
||||||
|
},
|
||||||
|
renderInitialMessages: vi.fn(),
|
||||||
|
reloadTodos: vi.fn(async () => {}),
|
||||||
|
session: {
|
||||||
|
navigateTree,
|
||||||
|
abortBranchSummary: vi.fn(),
|
||||||
|
},
|
||||||
|
} as unknown as InteractiveModeContext;
|
||||||
|
const controller = new SelectorController(ctx);
|
||||||
|
controller.showSelector = create => {
|
||||||
|
const result = create(() => {});
|
||||||
|
selector = result.component as { handleInput(key: string): void };
|
||||||
|
};
|
||||||
|
return {
|
||||||
|
controller,
|
||||||
|
navigateTree,
|
||||||
|
navigation: navigation.promise,
|
||||||
|
selector: () => {
|
||||||
|
if (!selector) throw new Error("Expected tree selector to be shown");
|
||||||
|
return selector;
|
||||||
|
},
|
||||||
|
showHookSelector,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("SelectorController tree branch summaries", () => {
|
||||||
|
it("switches without a summary or prompt on plain enter by default", async () => {
|
||||||
|
const harness = createHarness();
|
||||||
|
|
||||||
|
harness.controller.showTreeSelector();
|
||||||
|
harness.selector().handleInput("\r");
|
||||||
|
await harness.navigation;
|
||||||
|
|
||||||
|
expect(harness.showHookSelector).not.toHaveBeenCalled();
|
||||||
|
expect(harness.navigateTree).toHaveBeenCalledWith("root", {
|
||||||
|
summarize: false,
|
||||||
|
customInstructions: undefined,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("summarizes and switches on shift+enter without showing the prompt", async () => {
|
||||||
|
const harness = createHarness();
|
||||||
|
|
||||||
|
harness.controller.showTreeSelector();
|
||||||
|
harness.selector().handleInput(SHIFT_ENTER);
|
||||||
|
await harness.navigation;
|
||||||
|
|
||||||
|
expect(harness.showHookSelector).not.toHaveBeenCalled();
|
||||||
|
expect(harness.navigateTree).toHaveBeenCalledWith("root", {
|
||||||
|
summarize: true,
|
||||||
|
customInstructions: undefined,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("skips the summary prompt on shift+enter even when branchSummary.enabled is on", async () => {
|
||||||
|
Settings.instance.set("branchSummary.enabled", true);
|
||||||
|
const harness = createHarness();
|
||||||
|
|
||||||
|
harness.controller.showTreeSelector();
|
||||||
|
harness.selector().handleInput(SHIFT_ENTER);
|
||||||
|
await harness.navigation;
|
||||||
|
|
||||||
|
expect(harness.showHookSelector).not.toHaveBeenCalled();
|
||||||
|
expect(harness.navigateTree).toHaveBeenCalledWith("root", {
|
||||||
|
summarize: true,
|
||||||
|
customInstructions: undefined,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("still offers the summary prompt on plain enter when branchSummary.enabled is on", async () => {
|
||||||
|
Settings.instance.set("branchSummary.enabled", true);
|
||||||
|
const harness = createHarness("Summarize");
|
||||||
|
|
||||||
|
harness.controller.showTreeSelector();
|
||||||
|
harness.selector().handleInput("\r");
|
||||||
|
await harness.navigation;
|
||||||
|
|
||||||
|
expect(harness.showHookSelector).toHaveBeenCalledWith("Summarize branch?", [
|
||||||
|
"No summary",
|
||||||
|
"Summarize",
|
||||||
|
"Summarize with custom prompt",
|
||||||
|
]);
|
||||||
|
expect(harness.navigateTree).toHaveBeenCalledWith("root", {
|
||||||
|
summarize: true,
|
||||||
|
customInstructions: undefined,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user