chore: consolidate model utils and drive-by cleanups (#476)
Signed-off-by: Thibault Sottiaux <tibo@openai.com>
This commit is contained in:
committed by
GitHub
parent
dc276999a9
commit
3c4f1fea9b
@@ -14,7 +14,7 @@ import React, { useRef, useState } from "react";
|
||||
* The real `process.stdin` object exposed by Node.js inherits these methods
|
||||
* from `Socket`, but the lightweight stub used in tests only extends
|
||||
* `EventEmitter`. Ink calls the two methods when enabling/disabling raw
|
||||
* mode, so make them harmless no‑ops when they're absent to avoid runtime
|
||||
* mode, so make them harmless no-ops when they're absent to avoid runtime
|
||||
* failures during unit tests.
|
||||
* ----------------------------------------------------------------------- */
|
||||
|
||||
|
||||
@@ -1,113 +0,0 @@
|
||||
import type { ResponseItem } from "openai/resources/responses/responses.mjs";
|
||||
|
||||
import { approximateTokensUsed } from "../../utils/approximate-tokens-used.js";
|
||||
|
||||
/**
|
||||
* Type‑guard that narrows a {@link ResponseItem} to one that represents a
|
||||
* user‑authored message. The OpenAI SDK represents both input *and* output
|
||||
* messages with a discriminated union where:
|
||||
* • `type` is the string literal "message" and
|
||||
* • `role` is one of "user" | "assistant" | "system" | "developer".
|
||||
*
|
||||
* For the purposes of de‑duplication we only care about *user* messages so we
|
||||
* detect those here in a single, reusable helper.
|
||||
*/
|
||||
function isUserMessage(
|
||||
item: ResponseItem,
|
||||
): item is ResponseItem & { type: "message"; role: "user"; content: unknown } {
|
||||
return item.type === "message" && (item as { role?: string }).role === "user";
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the maximum context length (in tokens) for a given model.
|
||||
* These numbers are best‑effort guesses and provide a basis for UI percentages.
|
||||
*/
|
||||
export function maxTokensForModel(model: string): number {
|
||||
const lower = model.toLowerCase();
|
||||
if (lower.includes("32k")) {
|
||||
return 32000;
|
||||
}
|
||||
if (lower.includes("16k")) {
|
||||
return 16000;
|
||||
}
|
||||
if (lower.includes("8k")) {
|
||||
return 8000;
|
||||
}
|
||||
if (lower.includes("4k")) {
|
||||
return 4000;
|
||||
}
|
||||
// Default to 128k for newer long‑context models
|
||||
return 128000;
|
||||
}
|
||||
|
||||
/**
|
||||
* Calculates the percentage of tokens remaining in context for a model.
|
||||
*/
|
||||
export function calculateContextPercentRemaining(
|
||||
items: Array<ResponseItem>,
|
||||
model: string,
|
||||
): number {
|
||||
const used = approximateTokensUsed(items);
|
||||
const max = maxTokensForModel(model);
|
||||
const remaining = Math.max(0, max - used);
|
||||
return (remaining / max) * 100;
|
||||
}
|
||||
|
||||
/**
|
||||
* Deduplicate the stream of {@link ResponseItem}s before they are persisted in
|
||||
* component state.
|
||||
*
|
||||
* Historically we used the (optional) {@code id} field returned by the
|
||||
* OpenAI streaming API as the primary key: the first occurrence of any given
|
||||
* {@code id} “won” and subsequent duplicates were dropped. In practice this
|
||||
* proved brittle because locally‑generated user messages don’t include an
|
||||
* {@code id}. The result was that if a user quickly pressed <Enter> twice the
|
||||
* exact same message would appear twice in the transcript.
|
||||
*
|
||||
* The new rules are therefore:
|
||||
* 1. If a {@link ResponseItem} has an {@code id} keep only the *first*
|
||||
* occurrence of that {@code id} (this retains the previous behaviour for
|
||||
* assistant / tool messages).
|
||||
* 2. Additionally, collapse *consecutive* user messages with identical
|
||||
* content. Two messages are considered identical when their serialized
|
||||
* {@code content} array matches exactly. We purposefully restrict this
|
||||
* to **adjacent** duplicates so that legitimately repeated questions at
|
||||
* a later point in the conversation are still shown.
|
||||
*/
|
||||
export function uniqueById(items: Array<ResponseItem>): Array<ResponseItem> {
|
||||
const seenIds = new Set<string>();
|
||||
const deduped: Array<ResponseItem> = [];
|
||||
|
||||
for (const item of items) {
|
||||
// ──────────────────────────────────────────────────────────────────
|
||||
// Rule #1 – de‑duplicate by id when present
|
||||
// ──────────────────────────────────────────────────────────────────
|
||||
if (typeof item.id === "string" && item.id.length > 0) {
|
||||
if (seenIds.has(item.id)) {
|
||||
continue; // skip duplicates
|
||||
}
|
||||
seenIds.add(item.id);
|
||||
}
|
||||
|
||||
// ──────────────────────────────────────────────────────────────────
|
||||
// Rule #2 – collapse consecutive identical user messages
|
||||
// ──────────────────────────────────────────────────────────────────
|
||||
if (isUserMessage(item) && deduped.length > 0) {
|
||||
const prev = deduped[deduped.length - 1]!;
|
||||
|
||||
if (
|
||||
isUserMessage(prev) &&
|
||||
// Note: the `content` field is an array of message parts. Performing
|
||||
// a deep compare is over‑kill here; serialising to JSON is sufficient
|
||||
// (and fast for the tiny payloads involved).
|
||||
JSON.stringify(prev.content) === JSON.stringify(item.content)
|
||||
) {
|
||||
continue; // skip duplicate user message
|
||||
}
|
||||
}
|
||||
|
||||
deduped.push(item);
|
||||
}
|
||||
|
||||
return deduped;
|
||||
}
|
||||
@@ -6,10 +6,6 @@ import type { ResponseItem } from "openai/resources/responses/responses.mjs";
|
||||
|
||||
import TerminalChatInput from "./terminal-chat-input.js";
|
||||
import { TerminalChatToolCallCommand } from "./terminal-chat-tool-call-command.js";
|
||||
import {
|
||||
calculateContextPercentRemaining,
|
||||
uniqueById,
|
||||
} from "./terminal-chat-utils.js";
|
||||
import TerminalMessageHistory from "./terminal-message-history.js";
|
||||
import { formatCommandForDisplay } from "../../format-command.js";
|
||||
import { useConfirmation } from "../../hooks/use-confirmation.js";
|
||||
@@ -22,7 +18,11 @@ import { extractAppliedPatches as _extractAppliedPatches } from "../../utils/ext
|
||||
import { getGitDiff } from "../../utils/get-diff.js";
|
||||
import { createInputItem } from "../../utils/input-utils.js";
|
||||
import { log } from "../../utils/logger/log.js";
|
||||
import { getAvailableModels } from "../../utils/model-utils.js";
|
||||
import {
|
||||
getAvailableModels,
|
||||
calculateContextPercentRemaining,
|
||||
uniqueById,
|
||||
} from "../../utils/model-utils.js";
|
||||
import { CLI_VERSION } from "../../utils/session.js";
|
||||
import { shortCwd } from "../../utils/short-path.js";
|
||||
import { saveRollout } from "../../utils/storage/save-rollout.js";
|
||||
@@ -106,11 +106,8 @@ async function generateCommandExplanation(
|
||||
} catch (error) {
|
||||
log(`Error generating command explanation: ${error}`);
|
||||
|
||||
// Improved error handling with more specific error information
|
||||
let errorMessage = "Unable to generate explanation due to an error.";
|
||||
|
||||
if (error instanceof Error) {
|
||||
// Include specific error message for better debugging
|
||||
errorMessage = `Unable to generate explanation: ${error.message}`;
|
||||
|
||||
// If it's an API error, check for more specific information
|
||||
@@ -141,18 +138,17 @@ export default function TerminalChat({
|
||||
additionalWritableRoots,
|
||||
fullStdout,
|
||||
}: Props): React.ReactElement {
|
||||
// Desktop notification setting
|
||||
const notify = config.notify;
|
||||
const [model, setModel] = useState<string>(config.model);
|
||||
const [provider, setProvider] = useState<string>(config.provider || "openai");
|
||||
const [lastResponseId, setLastResponseId] = useState<string | null>(null);
|
||||
const [items, setItems] = useState<Array<ResponseItem>>([]);
|
||||
const [loading, setLoading] = useState<boolean>(false);
|
||||
// Allow switching approval modes at runtime via an overlay.
|
||||
const [approvalPolicy, setApprovalPolicy] = useState<ApprovalPolicy>(
|
||||
initialApprovalPolicy,
|
||||
);
|
||||
const [thinkingSeconds, setThinkingSeconds] = useState(0);
|
||||
|
||||
const handleCompact = async () => {
|
||||
setLoading(true);
|
||||
try {
|
||||
@@ -185,6 +181,7 @@ export default function TerminalChat({
|
||||
setLoading(false);
|
||||
}
|
||||
};
|
||||
|
||||
const {
|
||||
requestConfirmation,
|
||||
confirmationPrompt,
|
||||
@@ -215,13 +212,13 @@ export default function TerminalChat({
|
||||
// DEBUG: log every render w/ key bits of state
|
||||
// ────────────────────────────────────────────────────────────────
|
||||
log(
|
||||
`render – agent? ${Boolean(agentRef.current)} loading=${loading} items=${
|
||||
`render - agent? ${Boolean(agentRef.current)} loading=${loading} items=${
|
||||
items.length
|
||||
}`,
|
||||
);
|
||||
|
||||
useEffect(() => {
|
||||
// Skip recreating the agent if awaiting a decision on a pending confirmation
|
||||
// Skip recreating the agent if awaiting a decision on a pending confirmation.
|
||||
if (confirmationPrompt != null) {
|
||||
log("skip AgentLoop recreation due to pending confirmationPrompt");
|
||||
return;
|
||||
@@ -234,7 +231,7 @@ export default function TerminalChat({
|
||||
)} approvalPolicy=${approvalPolicy}`,
|
||||
);
|
||||
|
||||
// Tear down any existing loop before creating a new one
|
||||
// Tear down any existing loop before creating a new one.
|
||||
agentRef.current?.terminate();
|
||||
|
||||
const sessionId = crypto.randomUUID();
|
||||
@@ -267,11 +264,9 @@ export default function TerminalChat({
|
||||
<TerminalChatToolCallCommand commandForDisplay={commandForDisplay} />,
|
||||
);
|
||||
|
||||
// If the user wants an explanation, generate one and ask again
|
||||
// If the user wants an explanation, generate one and ask again.
|
||||
if (review === ReviewDecision.EXPLAIN) {
|
||||
log(`Generating explanation for command: ${commandForDisplay}`);
|
||||
|
||||
// Generate an explanation using the same model
|
||||
const explanation = await generateCommandExplanation(
|
||||
command,
|
||||
model,
|
||||
@@ -279,7 +274,7 @@ export default function TerminalChat({
|
||||
);
|
||||
log(`Generated explanation: ${explanation}`);
|
||||
|
||||
// Ask for confirmation again, but with the explanation
|
||||
// Ask for confirmation again, but with the explanation.
|
||||
const confirmResult = await requestConfirmation(
|
||||
<TerminalChatToolCallCommand
|
||||
commandForDisplay={commandForDisplay}
|
||||
@@ -287,11 +282,11 @@ export default function TerminalChat({
|
||||
/>,
|
||||
);
|
||||
|
||||
// Update the decision based on the second confirmation
|
||||
// Update the decision based on the second confirmation.
|
||||
review = confirmResult.decision;
|
||||
customDenyMessage = confirmResult.customDenyMessage;
|
||||
|
||||
// Return the final decision with the explanation
|
||||
// Return the final decision with the explanation.
|
||||
return { review, customDenyMessage, applyPatch, explanation };
|
||||
}
|
||||
|
||||
@@ -299,7 +294,7 @@ export default function TerminalChat({
|
||||
},
|
||||
});
|
||||
|
||||
// force a render so JSX below can "see" the freshly created agent
|
||||
// Force a render so JSX below can "see" the freshly created agent.
|
||||
forceUpdate();
|
||||
|
||||
log(`AgentLoop created: ${inspect(agentRef.current, { depth: 1 })}`);
|
||||
@@ -320,7 +315,7 @@ export default function TerminalChat({
|
||||
additionalWritableRoots,
|
||||
]);
|
||||
|
||||
// whenever loading starts/stops, reset or start a timer — but pause the
|
||||
// Whenever loading starts/stops, reset or start a timer — but pause the
|
||||
// timer while a confirmation overlay is displayed so we don't trigger a
|
||||
// re‑render every second during apply_patch reviews.
|
||||
useEffect(() => {
|
||||
@@ -345,14 +340,15 @@ export default function TerminalChat({
|
||||
};
|
||||
}, [loading, confirmationPrompt]);
|
||||
|
||||
// Notify desktop with a preview when an assistant response arrives
|
||||
// Notify desktop with a preview when an assistant response arrives.
|
||||
const prevLoadingRef = useRef<boolean>(false);
|
||||
useEffect(() => {
|
||||
// Only notify when notifications are enabled
|
||||
// Only notify when notifications are enabled.
|
||||
if (!notify) {
|
||||
prevLoadingRef.current = loading;
|
||||
return;
|
||||
}
|
||||
|
||||
if (
|
||||
prevLoadingRef.current &&
|
||||
!loading &&
|
||||
@@ -389,7 +385,7 @@ export default function TerminalChat({
|
||||
prevLoadingRef.current = loading;
|
||||
}, [notify, loading, confirmationPrompt, items, PWD]);
|
||||
|
||||
// Let's also track whenever the ref becomes available
|
||||
// Let's also track whenever the ref becomes available.
|
||||
const agent = agentRef.current;
|
||||
useEffect(() => {
|
||||
log(`agentRef.current is now ${Boolean(agent)}`);
|
||||
@@ -412,7 +408,7 @@ export default function TerminalChat({
|
||||
const inputItems = [
|
||||
await createInputItem(initialPrompt || "", initialImagePaths || []),
|
||||
];
|
||||
// Clear them to prevent subsequent runs
|
||||
// Clear them to prevent subsequent runs.
|
||||
setInitialPrompt("");
|
||||
setInitialImagePaths([]);
|
||||
agent?.run(inputItems);
|
||||
@@ -447,7 +443,7 @@ export default function TerminalChat({
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, []);
|
||||
|
||||
// Just render every item in order, no grouping/collapse
|
||||
// Just render every item in order, no grouping/collapse.
|
||||
const lastMessageBatch = items.map((item) => ({ item }));
|
||||
const groupCounts: Record<string, number> = {};
|
||||
const userMsgCount = items.filter(
|
||||
@@ -626,10 +622,10 @@ export default function TerminalChat({
|
||||
agent?.cancel();
|
||||
setLoading(false);
|
||||
|
||||
// Select default model for the new provider
|
||||
// Select default model for the new provider.
|
||||
const defaultModel = model;
|
||||
|
||||
// Save provider to config
|
||||
// Save provider to config.
|
||||
const updatedConfig = {
|
||||
...config,
|
||||
provider: newProvider,
|
||||
@@ -669,13 +665,12 @@ export default function TerminalChat({
|
||||
<ApprovalModeOverlay
|
||||
currentMode={approvalPolicy}
|
||||
onSelect={(newMode) => {
|
||||
// update approval policy without cancelling an in-progress session
|
||||
// Update approval policy without cancelling an in-progress session.
|
||||
if (newMode === approvalPolicy) {
|
||||
return;
|
||||
}
|
||||
// update state
|
||||
|
||||
setApprovalPolicy(newMode as ApprovalPolicy);
|
||||
// update existing AgentLoop instance
|
||||
if (agentRef.current) {
|
||||
(
|
||||
agentRef.current as unknown as {
|
||||
|
||||
@@ -34,9 +34,9 @@ const TerminalHeader: React.FC<TerminalHeaderProps> = ({
|
||||
{terminalRows < 10 ? (
|
||||
// Compact header for small terminal windows
|
||||
<Text>
|
||||
● Codex v{version} – {PWD} – {model} ({provider}) –{" "}
|
||||
● Codex v{version} - {PWD} - {model} ({provider}) -{" "}
|
||||
<Text color={colorsByPolicy[approvalPolicy]}>{approvalPolicy}</Text>
|
||||
{flexModeEnabled ? " – flex-mode" : ""}
|
||||
{flexModeEnabled ? " - flex-mode" : ""}
|
||||
</Text>
|
||||
) : (
|
||||
<>
|
||||
|
||||
Reference in New Issue
Block a user