fix: preserve provider-native compaction semantics

(cherry picked from commit 426ac1e147c08092c7d0b6c4a7af5f2e09eaf941)
This commit is contained in:
Roy
2026-07-26 16:35:57 +00:00
committed by can1357
parent 091f670ea0
commit 5ce80fdedc
7 changed files with 196 additions and 25 deletions
+3
View File
@@ -23,6 +23,9 @@
### Fixed
- Fixed proxy-stream clients dropping finalized provider-only content blocks, including Anthropic native web-search history, by allowing `done` and `error` events to carry terminal assistant content while retaining delta-reconstructed content from older proxy servers that omit it ([#6703](https://github.com/can1357/oh-my-pi/issues/6703)).
### Fixed
- Provider-native compaction failures now surface their transport error instead of silently switching to generic summarization; streaming V2 still falls back to native V1 when available.
## [17.1.4] - 2026-07-26
+9 -2
View File
@@ -1406,6 +1406,7 @@ export async function compact(
...recentMessages,
];
let usedRemoteCompaction = false;
let nativeCompactionError: unknown;
if (
settings.remoteEnabled !== false &&
settings.remoteStreamingV2Enabled !== false &&
@@ -1467,7 +1468,8 @@ export async function compact(
// swallowing it here would downgrade Esc into "fall back to local
// summarization" and keep compaction running on an aborted signal.
if (signal?.aborted) throw err;
logger.warn("OpenAI V2 remote compaction failed, falling back to V1/local summarization", {
nativeCompactionError = err;
logger.warn("OpenAI V2 remote compaction failed, falling back to V1 remote compaction", {
error: err instanceof Error ? err.message : String(err),
model: model.id,
provider: model.provider,
@@ -1517,7 +1519,8 @@ export async function compact(
// swallowing it here would downgrade Esc into "fall back to local
// summarization" and keep compaction running on an aborted signal.
if (signal?.aborted) throw err;
logger.warn("OpenAI remote compaction failed, falling back to local summarization", {
nativeCompactionError = err;
logger.warn("OpenAI remote compaction failed", {
error: err instanceof Error ? err.message : String(err),
model: model.id,
provider: model.provider,
@@ -1526,6 +1529,10 @@ export async function compact(
}
}
if (!usedRemoteCompaction && nativeCompactionError !== undefined) {
throw nativeCompactionError;
}
// Generate summaries (can be parallel if both needed) and merge into one
let summary: string;
+32 -7
View File
@@ -1687,6 +1687,31 @@ describe("compact() remote compaction failure handling", () => {
expect(JSON.stringify(sameProviderActive?.messagesToSummarize ?? [])).not.toContain("ORIGINAL ALPHA port 4242");
});
test("V2 native failure falls back to V1 without generic summarization", async () => {
const completeSpy = vi.spyOn(ai, "completeSimple").mockResolvedValue(localSummaryMessage("local summary"));
const preparation = makePreparation();
preparation.settings = { ...preparation.settings, remoteStreamingV2Enabled: true };
const model = makeOpenAiModel({
remoteCompaction: { enabled: true, v2StreamingEnabled: true },
});
const requestedUrls: string[] = [];
const fetchMock: FetchImpl = async input => {
const url = String(input);
requestedUrls.push(url);
if (url.endsWith("/responses/compact")) {
return Response.json({ output: [{ type: "compaction", encrypted_content: "enc-v1" }] });
}
return new Response("V2 unavailable", { status: 502, statusText: "Bad Gateway" });
};
const result = await compact(preparation, model, "test-key", undefined, undefined, { fetch: fetchMock });
expect(requestedUrls.some(url => url.endsWith("/responses"))).toBe(true);
expect(requestedUrls.some(url => url.endsWith("/responses/compact"))).toBe(true);
expect(result.shortSummary).toBe("Remote compaction");
expect(completeSpy).not.toHaveBeenCalled();
});
test("user abort during the remote compact request rejects without falling back to local summarization", async () => {
// Contract: Esc is a cancellation, not a remote failure. Before the fix
// the AbortError was swallowed by the fallback catch and compaction kept
@@ -1760,16 +1785,16 @@ describe("compact() remote compaction failure handling", () => {
});
});
test("remote compact server failure without abort still falls back to local summarization", async () => {
test("native compaction server failure rejects without generic summarization", async () => {
const completeSpy = vi.spyOn(ai, "completeSimple").mockResolvedValue(localSummaryMessage("local summary"));
const fetchMock: FetchImpl = async () =>
new Response("nope", { status: 500, statusText: "Internal Server Error" });
const result = await compact(makePreparation(), makeOpenAiModel(), "test-key", undefined, undefined, {
fetch: fetchMock,
});
expect(result.summary).toContain("local summary");
expect(completeSpy).toHaveBeenCalled();
await expect(
compact(makePreparation(), makeOpenAiModel(), "test-key", undefined, undefined, {
fetch: fetchMock,
}),
).rejects.toThrow("Remote compaction failed");
expect(completeSpy).not.toHaveBeenCalled();
});
});
+3
View File
@@ -161,6 +161,9 @@
- Fixed MiMo models using hashline edit mode by default despite needing the same replace-mode fallback as Kimi. ([#3772](https://github.com/can1357/oh-my-pi/issues/3772))
- Fixed `omp` refusing to start on Windows when no `bash.exe` is discoverable — most visibly with scoop-installed Git, whose manifest shims `sh.exe`/`git.exe` but never `bash.exe`, so PATH lookup missed it. Startup threw `No bash shell found` while merely building the bash tool description, even though bash tool commands always execute in the embedded brush-core shell and need no host bash. Shell discovery now also checks `GIT_INSTALL_ROOT`, scoop and per-user Git for Windows install roots, and `sh.exe` on PATH, then falls back to `cmd.exe` for the spawn-only paths (interactive PTY, ACP client terminals) instead of failing; the cmd fallback is never used to wrap user-shell commands — brush runs the POSIX line directly.
- Added a selectable voice setting for `/live` realtime sessions ([#6566](https://github.com/can1357/oh-my-pi/issues/6566)).
### Fixed
- Native compaction now keeps implicit role and largest-context fallbacks on the active provider, preventing a provider-native request from silently becoming another provider's generic summary. Explicit compaction models and soft compaction retain their existing fallback behavior.
## [17.1.4] - 2026-07-26
@@ -34,6 +34,7 @@ import {
type SummaryOptions,
shouldCompact,
shouldUseOpenAiRemoteCompaction,
shouldUseCompactionV2Streaming,
} from "@oh-my-pi/pi-agent-core/compaction";
import {
DEFAULT_PRUNE_CONFIG,
@@ -567,6 +568,7 @@ export class SessionMaintenance {
let compactionCandidates = this.#getCompactionModelCandidates(
availableModels,
requireProviderRemote ? shouldUseOpenAiRemoteCompaction : undefined,
effectiveSettings,
);
if (requireProviderRemote && compactionCandidates.length === 0) {
this.#host.emitNotice(
@@ -574,7 +576,7 @@ export class SessionMaintenance {
`remote compaction is unavailable for ${this.#model.id} (no remote endpoint configured and no provider-native remote-capable model in the fallback chain) — using a local summary instead`,
"compaction",
);
compactionCandidates = this.#getCompactionModelCandidates(availableModels);
compactionCandidates = this.#getCompactionModelCandidates(availableModels, undefined, effectiveSettings);
}
const pathEntries = this.#host.sessionManager.getBranch();
const preparation = prepareCompaction(pathEntries, effectiveSettings, this.#model);
@@ -1390,46 +1392,73 @@ export class SessionMaintenance {
return candidate;
}
#getCompactionModelCandidates(availableModels: Model[], filter?: (model: Model) => boolean): Model[] {
return this.resolveCompactionModelCandidates(this.#model, availableModels, filter);
#getCompactionModelCandidates(
availableModels: Model[],
filter: ((model: Model) => boolean) | undefined,
settings: Pick<CompactionSettings, "remoteEnabled" | "remoteStreamingV2Enabled">,
): Model[] {
return this.resolveCompactionModelCandidates(
this.#model,
availableModels,
filter,
settings.remoteEnabled !== false,
settings.remoteStreamingV2Enabled !== false,
);
}
resolveCompactionModelCandidates(
preferredModel: Model | null | undefined,
availableModels: Model[],
filter?: (model: Model) => boolean,
remoteEnabled = this.#host.settings.getGroup("compaction").remoteEnabled !== false,
remoteStreamingV2Enabled =
this.#host.settings.getGroup("compaction").remoteStreamingV2Enabled !== false,
): Model[] {
const candidates: Model[] = [];
const seen = new Set<string>();
const hasEffectiveNativeCompaction = (model: Model): boolean =>
remoteEnabled &&
(shouldUseOpenAiRemoteCompaction(model) ||
(remoteStreamingV2Enabled && shouldUseCompactionV2Streaming(model)));
const nativeProvider =
preferredModel && hasEffectiveNativeCompaction(preferredModel) ? preferredModel.provider : undefined;
const addCandidate = (model: Model | undefined): void => {
const addCandidate = (model: Model | undefined, source: "explicit" | "current" | "implicit"): void => {
if (!model) return;
const key = `${model.provider}/${model.id}`;
if (seen.has(key)) return;
seen.add(key);
// `seen` still tracks rejected models so the largest-context fallback
// scan below doesn't reintroduce them; the filter just suppresses
// inclusion in this caller's candidate chain.
// Explicit targets and the active model retain their established
// semantics. Implicit role/context fallbacks must not turn a native
// compaction request into a different provider's generic summary.
if (
source === "implicit" &&
nativeProvider !== undefined &&
(model.provider !== nativeProvider || !hasEffectiveNativeCompaction(model))
) {
return;
}
if (filter && !filter(model)) return;
candidates.push(model);
};
if (preferredModel) {
addCandidate(resolveCompactionConfiguredTarget(preferredModel, availableModels));
addCandidate(resolveCompactionConfiguredTarget(preferredModel, availableModels), "explicit");
}
addCandidate(preferredModel ?? undefined);
addCandidate(preferredModel ?? undefined, "current");
for (const role of MODEL_ROLE_IDS) {
addCandidate(
resolveRoleModelFull(this.#host.settings, role, availableModels, preferredModel ?? undefined).model,
"implicit",
);
}
const sortedByContext = [...availableModels].sort((a, b) => (b.contextWindow ?? 0) - (a.contextWindow ?? 0));
for (const model of sortedByContext) {
if (!seen.has(`${model.provider}/${model.id}`)) {
addCandidate(model);
break;
}
if (seen.has(`${model.provider}/${model.id}`)) continue;
const candidateCount = candidates.length;
addCandidate(model, "implicit");
if (candidates.length > candidateCount) break;
}
return candidates;
@@ -1456,7 +1485,8 @@ export class SessionMaintenance {
precomputedCandidates?: Model[],
): Promise<CompactionResult> {
const candidates =
precomputedCandidates ?? this.#getCompactionModelCandidates(this.#host.modelRegistry.getAvailable());
precomputedCandidates ??
this.#getCompactionModelCandidates(this.#host.modelRegistry.getAvailable(), undefined, preparation.settings);
const telemetry = resolveTelemetry(this.#host.agent.telemetry, this.#host.sessionId());
for (const candidate of candidates) {
@@ -2472,7 +2502,7 @@ export class SessionMaintenance {
details = snapcompactResult.details;
preserveData = { ...(compactionPrep.preserveData ?? {}), ...(snapcompactResult.preserveData ?? {}) };
} else {
const candidates = this.#getCompactionModelCandidates(availableModels);
const candidates = this.#getCompactionModelCandidates(availableModels, undefined, compactionSettings);
const retrySettings = this.#host.settings.getGroup("retry");
const telemetry = resolveTelemetry(this.#host.agent.telemetry, this.#host.sessionId());
let compactResult: CompactionResult | undefined;
@@ -37,7 +37,13 @@ describe("issue #986 compaction auth fallback", () => {
throw new Error("Expected bundled test models to exist");
}
const settings = Settings.isolated({ "compaction.keepRecentTokens": 1, "compaction.strategy": "context-full" });
const settings = Settings.isolated({
"compaction.keepRecentTokens": 1,
"compaction.strategy": "context-full",
// This suite covers the portable summarizer's auth fallback. Native
// compaction keeps its implicit candidate chain provider-isolated.
"compaction.remoteEnabled": false,
});
if (options?.fallbackModelRole) {
settings.setModelRole(options.fallbackModelRole, `${fallbackModel.provider}/${fallbackModel.id}`);
}
@@ -0,0 +1,97 @@
import { describe, expect, it } from "bun:test";
import type { Model } from "@oh-my-pi/pi-ai";
import { buildModel } from "@oh-my-pi/pi-catalog/build";
import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
import {
SessionMaintenance,
type SessionMaintenanceHost,
} from "@oh-my-pi/pi-coding-agent/session/session-maintenance";
function model(
id: string,
provider: string,
contextWindow: number,
remoteCompaction?: Model["remoteCompaction"],
): Model {
return buildModel({
id,
name: id,
api: "openai-responses",
provider,
baseUrl: "https://example.test/v1",
reasoning: false,
input: ["text"],
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 },
contextWindow,
maxTokens: 4096,
remoteCompaction,
});
}
function maintenance(settings: Settings): SessionMaintenance {
return new SessionMaintenance({ settings } as SessionMaintenanceHost);
}
describe("native compaction provider isolation", () => {
it("keeps implicit role and context fallbacks on the current native provider", () => {
const settings = Settings.isolated();
const current = model("native-current", "native-provider", 100_000, { enabled: true });
const genericRole = model("generic-role", "generic-provider", 90_000);
const nativeFallback = model("native-fallback", "native-provider", 80_000, { enabled: true });
settings.setModelRole("smol", `${genericRole.provider}/${genericRole.id}`);
const candidates = maintenance(settings).resolveCompactionModelCandidates(
current,
[current, genericRole, nativeFallback],
undefined,
true,
true,
);
expect(candidates.map(candidate => `${candidate.provider}/${candidate.id}`)).toEqual([
"native-provider/native-current",
"native-provider/native-fallback",
]);
});
it("retains generic implicit fallbacks when provider-native compaction is disabled", () => {
const settings = Settings.isolated();
const current = model("native-current", "native-provider", 100_000, { enabled: true });
const genericRole = model("generic-role", "generic-provider", 90_000);
settings.setModelRole("smol", `${genericRole.provider}/${genericRole.id}`);
const candidates = maintenance(settings).resolveCompactionModelCandidates(
current,
[current, genericRole],
undefined,
false,
true,
);
expect(candidates.map(candidate => `${candidate.provider}/${candidate.id}`)).toEqual([
"native-provider/native-current",
"generic-provider/generic-role",
]);
});
it("recognizes V2-only native capability under the effective streaming setting", () => {
const settings = Settings.isolated();
const current = model("v2-current", "v2-provider", 100_000, { v2StreamingEnabled: true });
const genericRole = model("generic-role", "generic-provider", 90_000);
const nativeFallback = model("v2-fallback", "v2-provider", 80_000, { v2StreamingEnabled: true });
settings.setModelRole("smol", `${genericRole.provider}/${genericRole.id}`);
const candidates = maintenance(settings).resolveCompactionModelCandidates(
current,
[current, genericRole, nativeFallback],
undefined,
true,
true,
);
expect(candidates.map(candidate => `${candidate.provider}/${candidate.id}`)).toEqual([
"v2-provider/v2-current",
"v2-provider/v2-fallback",
]);
});
});