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
171 changes: 139 additions & 32 deletions lib/request/request-transformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -864,12 +864,27 @@ const PROMPT_CACHE_METADATA_KEYS = [
"chatId",
];

const PROMPT_CACHE_FORK_KEYS = [
"forkId",
"fork_id",
"branchId",
"branch_id",
"parentConversationId",
"parent_conversation_id",
];

type PromptCacheKeySource = "existing" | "metadata" | "generated";

interface PromptCacheKeyResult {
key: string;
source: PromptCacheKeySource;
sourceKey?: string;
forkSourceKey?: string;
hintKeys?: string[];
unusableKeys?: string[];
forkHintKeys?: string[];
forkUnusableKeys?: string[];
fallbackHash?: string;
}

function extractString(value: unknown): string | undefined {
Expand All @@ -880,38 +895,96 @@ function extractString(value: unknown): string | undefined {
return trimmed.length > 0 ? trimmed : undefined;
}

function derivePromptCacheKeyFromBody(body: RequestBody): { value: string; sourceKey: string } | undefined {
function normalizeCacheKeyBase(base: string): string {
const trimmed = base.trim();
if (!trimmed) {
return `cache_${randomUUID()}`;
}
const sanitized = trimmed.replace(/\s+/g, "-");
return sanitized.startsWith("cache_") ? sanitized : `cache_${sanitized}`;
}

function normalizeForkSuffix(forkId: string): string {
const trimmed = forkId.trim();
if (!trimmed) return "fork";
return trimmed.replace(/\s+/g, "-");
}

function derivePromptCacheKeyFromBody(body: RequestBody): {
base?: string;
sourceKey?: string;
hintKeys: string[];
unusableKeys: string[];
forkId?: string;
forkSourceKey?: string;
forkHintKeys: string[];
forkUnusableKeys: string[];
} {
const metadata = body.metadata as Record<string, unknown> | undefined;
const root = body as Record<string, unknown>;

const getForkIdentifier = (): string | undefined => {
// Prefer metadata over root, and support both camelCase and snake_case
return (
extractString(metadata?.forkId) ||
extractString(metadata?.fork_id) ||
extractString(metadata?.branchId) ||
extractString(metadata?.branch_id) ||
extractString(root.forkId) ||
extractString(root.fork_id) ||
extractString(root.branchId) ||
extractString(root.branch_id)
);
};

const forkId = getForkIdentifier();
const hintKeys: string[] = [];
const unusableKeys: string[] = [];
let base: string | undefined;
let sourceKey: string | undefined;

for (const key of PROMPT_CACHE_METADATA_KEYS) {
const base = extractString(metadata?.[key]) ?? extractString(root[key]);
if (base) {
const value = forkId ? `${base}::fork::${forkId}` : base;
return { value, sourceKey: key };
const raw = metadata?.[key] ?? root[key];
if (raw !== undefined) {
hintKeys.push(key);
}
const value = extractString(raw);
if (value) {
base = value;
sourceKey = key;
break;
}
if (raw !== undefined) {
unusableKeys.push(key);
}
}
return undefined;

const forkHintKeys: string[] = [];
const forkUnusableKeys: string[] = [];
let forkId: string | undefined;
let forkSourceKey: string | undefined;

for (const key of PROMPT_CACHE_FORK_KEYS) {
const raw = metadata?.[key] ?? root[key];
if (raw !== undefined) {
forkHintKeys.push(key);
}
const value = extractString(raw);
if (value) {
forkId = value;
forkSourceKey = key;
break;
}
if (raw !== undefined) {
forkUnusableKeys.push(key);
}
}

return {
base,
sourceKey,
hintKeys,
unusableKeys,
forkId,
forkSourceKey,
forkHintKeys,
forkUnusableKeys,
};
}

function generatePromptCacheKey(): string {
return `cache_${randomUUID()}`;
function computeFallbackHashForBody(body: RequestBody): string {
const inputSlice = Array.isArray(body.input) ? body.input.slice(0, 3) : undefined;
const seed = stableStringify({
model: typeof body.model === "string" ? body.model : undefined,
metadata: body.metadata,
input: inputSlice,
});
return createHash("sha1").update(seed).digest("hex").slice(0, 12);
}

function ensurePromptCacheKey(body: RequestBody): PromptCacheKeyResult {
Expand All @@ -931,17 +1004,35 @@ function ensurePromptCacheKey(body: RequestBody): PromptCacheKeyResult {
}

const derived = derivePromptCacheKeyFromBody(body);
if (derived) {
const sanitized = extractString(derived.value) ?? generatePromptCacheKey();
body.prompt_cache_key = sanitized;
if (derived.base) {
const baseKey = normalizeCacheKeyBase(derived.base);
const suffix = derived.forkId ? `-fork-${normalizeForkSuffix(derived.forkId)}` : "";
const finalKey = `${baseKey}${suffix}`;
body.prompt_cache_key = finalKey;
// Don't set camelCase field for derived keys - only snake_case for Codex
return { key: sanitized, source: "metadata", sourceKey: derived.sourceKey };
return {
key: finalKey,
source: "metadata",
sourceKey: derived.sourceKey,
forkSourceKey: derived.forkSourceKey,
hintKeys: derived.hintKeys,
forkHintKeys: derived.forkHintKeys,
};
}

const generated = generatePromptCacheKey();
const fallbackHash = computeFallbackHashForBody(body);
const generated = `cache_${fallbackHash}`;
body.prompt_cache_key = generated;
// Don't set camelCase field for generated keys - only snake_case for Codex
return { key: generated, source: "generated" };
return {
key: generated,
source: "generated",
hintKeys: derived.hintKeys,
unusableKeys: derived.unusableKeys,
forkHintKeys: derived.forkHintKeys,
forkUnusableKeys: derived.forkUnusableKeys,
fallbackHash,
};
}

/**
Expand Down Expand Up @@ -1035,11 +1126,27 @@ export async function transformRequestBody(
logDebug("Prompt cache key missing; derived from metadata", {
promptCacheKey: cacheKeyResult.key,
sourceKey: cacheKeyResult.sourceKey,
forkSourceKey: cacheKeyResult.forkSourceKey,
forkHintKeys: cacheKeyResult.forkHintKeys,
});
} else if (cacheKeyResult.source === "generated") {
logWarn("Prompt cache key missing; generated fallback cache key", {
promptCacheKey: cacheKeyResult.key,
});
const hasHints = Boolean(
(cacheKeyResult.hintKeys && cacheKeyResult.hintKeys.length > 0) ||
(cacheKeyResult.forkHintKeys && cacheKeyResult.forkHintKeys.length > 0),
);
logWarn(
hasHints
? "Prompt cache key hints detected but unusable; generated fallback cache key"
: "Prompt cache key missing; generated fallback cache key",
{
promptCacheKey: cacheKeyResult.key,
fallbackHash: cacheKeyResult.fallbackHash,
hintKeys: cacheKeyResult.hintKeys,
unusableKeys: cacheKeyResult.unusableKeys,
forkHintKeys: cacheKeyResult.forkHintKeys,
forkUnusableKeys: cacheKeyResult.forkUnusableKeys,
},
);
}

// Tool behavior parity with Codex CLI (normalize shapes)
Expand Down
29 changes: 29 additions & 0 deletions spec/issue-4-prompt-cache-key.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Issue 4 – Fork-aware prompt_cache_key and non-structural overrides

**Issue**: https://github.com/open-hax/codex/issues/4 (open)

## Context & Current Behavior
- `lib/request/request-transformer.ts:856-1043` — `ensurePromptCacheKey` now normalizes metadata-derived keys to `cache_<base>` and appends `-fork-<id>` when `forkId/branchId/parentConversationId` is present; otherwise derives deterministic hashed fallback `cache_<hash>`.
- `lib/request/request-transformer.ts:915-1043` — Transform pipeline logs when deriving/generating keys with hint details and fallback hashes.
- `lib/session/session-manager.ts:83-206` — SessionManager derives session IDs from conversation metadata or host-provided cache key; resets cache key on prefix mismatch; preserves prompt_cache_key continuity when possible.
- `test/request-transformer.test.ts:715-850` — Tests cover preserving host keys, metadata derivation, fork suffix (`-fork-<id>`), stability across non-structural overrides, and deterministic fallback generation.

## Gaps vs Issue Requirements
- Fork derivation is normalized but not yet numbered; relies on provided fork identifiers/metadata.
- Fallback keys are hashed but still lack explicit numbering for forks (pending if required later).
- Logging does not surface when fallback occurs despite having conversation-like metadata; need stronger WARN.
- No tests mirroring Codex CLI semantics for: constant keys across soft overrides, distinct keys for forks with numbering/hashing, deterministic fallback reuse across transforms.

## Plan (Phases)
1) **Design & Hooks**: Decide fork-key schema (`cache_<base>` + `-fork-<n>`), define what counts as fork metadata (forkId/branchId, future parentConversationId), and how to seed numbering from metadata vs. fallback detection.
2) **Implementation**: Update `ensurePromptCacheKey` (and helpers) to:
- Normalize base cache key from metadata/host; seed fork suffix with deterministic numbering when forks requested; keep stability across soft overrides.
- Detect conversation-like hints when falling back; emit warn and include short hash of input/fallback seed (`cache_<hash>-<uuid>` or similar) to reduce accidental reuse.
- Ensure SessionManager interactions remain compatible (no regressions on prefix matching).
3) **Tests & Docs**: Add unit coverage in `test/request-transformer.test.ts` (fork numbering, fallback hash stability across transforms, soft-override stability, fork distinction). Update docs if behavior changes materially (configuration/getting-started sections mentioning prompt_cache_key behavior).

## Definition of Done / Requirements
- Prompt cache key derivation mirrors Codex CLI semantics: stable across soft overrides (temperature/max tokens/reasoning fields), distinct for explicit forks, deterministic fallback reuse for identical bodies, and warns when fallback occurs despite conversation hints.
- New/updated tests in `test/request-transformer.test.ts` cover: (a) stable key with overrides, (b) fork-specific keys with deterministic suffix/numbering, (c) fallback key reuse with hash component, (d) warning path when conversation-like metadata is unusable.
- Code builds and relevant tests pass (`pnpm test` at minimum; broader suites as needed).
- No regression to SessionManager behavior or existing prompt_cache_key consumers.
22 changes: 13 additions & 9 deletions test/request-transformer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -741,7 +741,7 @@ describe('runTransform', () => {
metadata: { conversation_id: 'meta-conv-123' },
};
const result: any = await runTransform(body, codexInstructions);
expect(result.prompt_cache_key).toBe('meta-conv-123');
expect(result.prompt_cache_key).toBe('cache_meta-conv-123');
});

it('derives fork-aware prompt_cache_key when fork id is present in metadata', async () => {
Expand All @@ -751,7 +751,7 @@ describe('runTransform', () => {
metadata: { conversation_id: 'meta-conv-123', forkId: 'branch-1' },
};
const result: any = await runTransform(body, codexInstructions);
expect(result.prompt_cache_key).toBe('meta-conv-123::fork::branch-1');
expect(result.prompt_cache_key).toBe('cache_meta-conv-123-fork-branch-1');
});

it('derives fork-aware prompt_cache_key when fork id is present in root', async () => {
Expand All @@ -762,7 +762,7 @@ describe('runTransform', () => {
forkId: 'branch-2' as any,
} as any;
const result: any = await runTransform(body, codexInstructions);
expect(result.prompt_cache_key).toBe('meta-conv-123::fork::branch-2');
expect(result.prompt_cache_key).toBe('cache_meta-conv-123-fork-branch-2');
});

it('reuses the same prompt_cache_key across non-structural overrides', async () => {
Expand All @@ -785,18 +785,22 @@ describe('runTransform', () => {
const result1: any = await runTransform(body1, codexInstructions);
const result2: any = await runTransform(body2, codexInstructions);

expect(result1.prompt_cache_key).toBe('meta-conv-789::fork::fork-x');
expect(result2.prompt_cache_key).toBe('meta-conv-789::fork::fork-x');
expect(result1.prompt_cache_key).toBe('cache_meta-conv-789-fork-fork-x');
expect(result2.prompt_cache_key).toBe('cache_meta-conv-789-fork-fork-x');
});

it('generates fallback prompt_cache_key when no identifiers exist', async () => {


it('generates deterministic fallback prompt_cache_key when no identifiers exist', async () => {
const body: RequestBody = {
model: 'gpt-5',
input: [],
};
const result: any = await runTransform(body, codexInstructions);
expect(typeof result.prompt_cache_key).toBe('string');
expect(result.prompt_cache_key).toMatch(/^cache_/);
const result1: any = await runTransform(body, codexInstructions);
const result2: any = await runTransform(body, codexInstructions);
expect(typeof result1.prompt_cache_key).toBe('string');
expect(result1.prompt_cache_key).toMatch(/^cache_[a-f0-9]{12}$/);
expect(result2.prompt_cache_key).toBe(result1.prompt_cache_key);
});

it('should set required Codex fields', async () => {
Expand Down
Loading