Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/reliable-completion-feedback.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@open-codesign/core": patch
"@open-codesign/providers": patch
---

Recover a truncated design-session brief with one bounded larger-output retry, retaining the previous brief on failure and including reported usage from the truncated attempt. Keep length-stop errors typed without accepting partial completion content.
9 changes: 8 additions & 1 deletion apps/desktop/src/main/codex-oauth-ipc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { mkdtempSync, rmSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { ConfigV3Schema, toPersistedV3 } from '@open-codesign/shared';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';

const handlers = new Map<string, (...args: unknown[]) => unknown>();

Expand Down Expand Up @@ -89,6 +89,13 @@ vi.mock('@open-codesign/providers/codex', async () => {

let tmpConfigDir: string;

// Cold transformation of the OAuth/provider module graph is suite setup, not
// part of the first status handler's 5s assertion budget. Under concurrent
// Windows runs that setup alone can exceed the per-test timeout.
beforeAll(async () => {
await import('./codex-oauth-ipc');
}, 30_000);

beforeEach(() => {
tmpConfigDir = mkdtempSync(join(tmpdir(), 'codex-oauth-ipc-'));
fakeCachedConfig = null;
Expand Down
105 changes: 104 additions & 1 deletion packages/core/src/design-context.test.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
import type { AgentMessage } from '@mariozechner/pi-agent-core';
import { CompletionLengthError } from '@open-codesign/providers';
import type {
ChatMessageRow,
ChatToolCallPayload,
ModelRef,
ResourceStateV1,
} from '@open-codesign/shared';
import { describe, expect, it, vi } from 'vitest';
import { CodesignError, ERROR_CODES } from '@open-codesign/shared';
import { beforeEach, describe, expect, it, vi } from 'vitest';

const completeWithRetryMock = vi.fn();
beforeEach(() => {
completeWithRetryMock.mockReset();
});

vi.mock('@open-codesign/providers', async () => {
const actual = await vi.importActual<typeof import('@open-codesign/providers')>(
Expand Down Expand Up @@ -391,6 +396,104 @@ describe('updateDesignSessionBrief', () => {
expect(completeWithRetryMock.mock.calls[0]?.[1][1].content).toContain('## Workspace MEMORY.md');
});

it('retries a length stop once with a larger bounded budget and counts both attempts', async () => {
completeWithRetryMock
.mockRejectedValueOnce(
new CompletionLengthError({ inputTokens: 10, outputTokens: 2000, costUsd: 0.01 }),
)
.mockResolvedValueOnce({
content: JSON.stringify(baseBrief()),
inputTokens: 12,
outputTokens: 400,
costUsd: 0.02,
});
const result = await updateDesignSessionBrief({
existingBrief: baseBrief(),
conversationMessages: [agentUser('Make it warmer')],
designId: 'design-1',
designName: 'Onboarding',
model: MODEL,
apiKey: 'sk-test',
wire: 'openai-chat',
baseUrl: 'https://gateway.example/v1',
httpHeaders: { 'X-Test': 'brief' },
});
expect(completeWithRetryMock).toHaveBeenCalledTimes(2);
expect(completeWithRetryMock.mock.calls[0]?.[2].maxTokens).toBe(2000);
expect(completeWithRetryMock.mock.calls[1]?.[2]).toMatchObject({
maxTokens: 4000,
wire: 'openai-chat',
baseUrl: 'https://gateway.example/v1',
apiKey: 'sk-test',
httpHeaders: { 'X-Test': 'brief' },
});
expect(result).toMatchObject({ inputTokens: 22, outputTokens: 2400, costUsd: 0.03 });
});

it('stops after a second length error without mutating the previous brief', async () => {
const previous = baseBrief();
const original = structuredClone(previous);
const failure = new CompletionLengthError({
inputTokens: 10,
outputTokens: 4000,
costUsd: 0.01,
});
completeWithRetryMock.mockRejectedValue(failure);
await expect(
updateDesignSessionBrief({
existingBrief: previous,
conversationMessages: [],
designId: 'design-1',
designName: 'Onboarding',
model: MODEL,
apiKey: 'sk-test',
}),
).rejects.toBe(failure);
expect(completeWithRetryMock).toHaveBeenCalledTimes(2);
expect(previous).toEqual(original);
});

it.each([
new CodesignError('Authentication failed', ERROR_CODES.PROVIDER_ERROR),
new CodesignError('Aborted', ERROR_CODES.PROVIDER_ABORTED),
new Error('A gateway mentioned a token limit'),
])('does not treat other failures as output truncation: %s', async (failure) => {
completeWithRetryMock.mockRejectedValue(failure);
await expect(
updateDesignSessionBrief({
existingBrief: baseBrief(),
conversationMessages: [],
designId: 'design-1',
designName: 'Onboarding',
model: MODEL,
apiKey: 'sk-test',
}),
).rejects.toBe(failure);
expect(completeWithRetryMock).toHaveBeenCalledTimes(1);
});

it('rejects invalid JSON after truncation recovery without replacing the previous brief', async () => {
const previous = baseBrief();
const original = structuredClone(previous);
completeWithRetryMock
.mockRejectedValueOnce(
new CompletionLengthError({ inputTokens: 1, outputTokens: 2000, costUsd: 0 }),
)
.mockResolvedValueOnce({ content: '{"goal":', inputTokens: 1, outputTokens: 1, costUsd: 0 });
await expect(
updateDesignSessionBrief({
existingBrief: previous,
conversationMessages: [],
designId: 'design-1',
designName: 'Onboarding',
model: MODEL,
apiKey: 'sk-test',
}),
).rejects.toThrow(/valid JSON/);
expect(completeWithRetryMock).toHaveBeenCalledTimes(2);
expect(previous).toEqual(original);
});

it('rejects invalid JSON so callers keep the previous brief', async () => {
completeWithRetryMock.mockResolvedValueOnce({
content: 'not json',
Expand Down
68 changes: 45 additions & 23 deletions packages/core/src/design-context.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import type { AgentMessage } from '@mariozechner/pi-agent-core';
import { completeWithRetry } from '@open-codesign/providers';
import {
CompletionLengthError,
completeWithRetry,
type GenerateResult,
} from '@open-codesign/providers';
import type {
ChatMessage,
ChatMessageRow,
Expand Down Expand Up @@ -108,6 +112,7 @@ const BRIEF_MAX_ARRAY_ITEMS = 12;
const BRIEF_MAX_FIELD_CHARS = 1_200;
const BRIEF_MAX_ITEM_CHARS = 240;
const BRIEF_MAX_OUTPUT_TOKENS = 2_000;
const BRIEF_RETRY_OUTPUT_TOKENS = 4_000;

export const DESIGN_BRIEF_SYSTEM_PROMPT = [
'You maintain a compact structured brief for one Open CoDesign design session.',
Expand Down Expand Up @@ -135,7 +140,8 @@ export const DESIGN_BRIEF_SYSTEM_PROMPT = [
'- Do not copy large source code, tool outputs, or full token tables.',
'- Treat DESIGN.md as authoritative when mentioned; summarize decisions, not raw tokens.',
"- Use the same language as the user's prompts when practical.",
'- Keep the whole JSON compact enough for a prompt brief.',
'- Keep the whole JSON below 600 words; use at most five short items per array.',
'- Use one short sentence per string field. Omit repetition and implementation details.',
].join('\n');

function isRecord(value: unknown): value is Record<string, unknown> {
Expand Down Expand Up @@ -493,24 +499,40 @@ export async function updateDesignSessionBrief(
conversationLen: conversation.length,
});
try {
const result = await completeWithRetry(
input.model,
messages,
{
apiKey: input.apiKey,
...(input.baseUrl !== undefined ? { baseUrl: input.baseUrl } : {}),
...(input.wire !== undefined ? { wire: input.wire } : {}),
...(input.httpHeaders !== undefined ? { httpHeaders: input.httpHeaders } : {}),
...(input.allowKeyless === true ? { allowKeyless: true } : {}),
...(input.reasoningLevel !== undefined ? { reasoning: input.reasoningLevel } : {}),
maxTokens: BRIEF_MAX_OUTPUT_TOKENS,
},
{
logger: log,
provider: input.model.provider,
...(input.wire !== undefined ? { wire: input.wire } : {}),
},
);
const summarize = (maxTokens: number) =>
completeWithRetry(
input.model,
messages,
{
apiKey: input.apiKey,
...(input.baseUrl !== undefined ? { baseUrl: input.baseUrl } : {}),
...(input.wire !== undefined ? { wire: input.wire } : {}),
...(input.httpHeaders !== undefined ? { httpHeaders: input.httpHeaders } : {}),
...(input.allowKeyless === true ? { allowKeyless: true } : {}),
...(input.reasoningLevel !== undefined ? { reasoning: input.reasoningLevel } : {}),
maxTokens,
},
{
logger: log,
provider: input.model.provider,
...(input.wire !== undefined ? { wire: input.wire } : {}),
},
);
let result: GenerateResult;
let truncatedUsage: Omit<GenerateResult, 'content'> | undefined;
try {
result = await summarize(BRIEF_MAX_OUTPUT_TOKENS);
} catch (err) {
if (!(err instanceof CompletionLengthError)) throw err;
truncatedUsage = err.usage;
log.info('[design-brief] step=summarize.retry-length', {
designId: input.designId,
maxTokens: BRIEF_RETRY_OUTPUT_TOKENS,
});
// One bounded retry only. Never persist partial JSON or replace the
// previous brief when either completion/validation attempt fails.
result = await summarize(BRIEF_RETRY_OUTPUT_TOKENS);
}
let parsed: unknown;
try {
parsed = JSON.parse(stripJsonFence(result.content)) as unknown;
Expand All @@ -534,9 +556,9 @@ export async function updateDesignSessionBrief(
});
return {
brief,
inputTokens: result.inputTokens,
outputTokens: result.outputTokens,
costUsd: result.costUsd,
inputTokens: result.inputTokens + (truncatedUsage?.inputTokens ?? 0),
outputTokens: result.outputTokens + (truncatedUsage?.outputTokens ?? 0),
costUsd: result.costUsd + (truncatedUsage?.costUsd ?? 0),
};
} catch (err) {
log.warn('[design-brief] step=summarize.fail', {
Expand Down
11 changes: 4 additions & 7 deletions packages/providers/src/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
import {
type ChatMessage,
type CodesignError,
ERROR_CODES,
type ModelRef,
} from '@open-codesign/shared';
import { type ChatMessage, ERROR_CODES, type ModelRef } from '@open-codesign/shared';
import { afterEach, describe, expect, it, vi } from 'vitest';

const getModelMock = vi.fn();
Expand Down Expand Up @@ -163,9 +158,11 @@ describe('complete', () => {
await expect(
complete(MODEL, [{ role: 'user', content: 'hi' }], { apiKey: 'sk-test' }),
).rejects.toMatchObject({
name: 'CompletionLengthError',
code: ERROR_CODES.PROVIDER_ERROR,
message: expect.stringContaining('token limit'),
} satisfies Partial<CodesignError>);
usage: { inputTokens: 1, outputTokens: 1, costUsd: 0 },
});
});

it('synthesizes a pass-through Model when openrouter id is missing from registry', async () => {
Expand Down
27 changes: 22 additions & 5 deletions packages/providers/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,18 @@ export interface GenerateResult {
costUsd: number;
}

/** A length stop is not a successful completion. Keep usage, never partial
* content, so bounded callers can retry without under-reporting token cost. */
export class CompletionLengthError extends CodesignError {
constructor(public readonly usage: Omit<GenerateResult, 'content'>) {
super(
'Provider stopped before completion because the response hit the token limit',
ERROR_CODES.PROVIDER_ERROR,
);
this.name = 'CompletionLengthError';
}
}

interface PiTextContent {
type: 'text';
text: string;
Expand Down Expand Up @@ -475,12 +487,17 @@ function assertCompleteStop(result: PiAssistantMessage): void {
ERROR_CODES.PROVIDER_ABORTED,
);
}
if (result.stopReason === 'length') {
throw new CompletionLengthError({
inputTokens: result.usage?.input ?? 0,
outputTokens: result.usage?.output ?? 0,
costUsd: result.usage?.cost?.total ?? 0,
});
}
const message =
result.stopReason === 'length'
? 'Provider stopped before completion because the response hit the token limit'
: result.stopReason === 'toolUse'
? 'Provider returned an unresolved tool call in a non-tool completion'
: (result.errorMessage ?? 'Provider returned an error');
result.stopReason === 'toolUse'
? 'Provider returned an unresolved tool call in a non-tool completion'
: (result.errorMessage ?? 'Provider returned an error');
throw new CodesignError(message, ERROR_CODES.PROVIDER_ERROR);
}

Expand Down
Loading