mirror of
https://github.com/dtzp555-max/ocp.git
synced 2026-07-19 09:44:07 +00:00
fix(server): empty CLAUDE_MAX_PROMPT_CHARS falls back to derived default (PR #179 review)
Reviewer regression: `!= null` treated an EMPTY env value ("CLAUDE_MAX_PROMPT_CHARS="
in an EnvironmentFile/.env) as explicit -> parseInt("") = NaN -> guard disabled + a
false "[System] Note: 0 older messages were truncated" injected into every prompt.
Extracted resolvePromptCharBudget() (truthiness contract, matching the old
`parseInt(env || default)` behavior) into lib/prompt.mjs so the semantics are
mutation-tested: switching back to != null fails the empty-string test. +2 tests, 349/0.
Co-Authored-By: Claude <claude-opus-4-8> <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude <claude-opus-4-8> <noreply@anthropic.com>
parent
8405e30d0e
commit
e912903ba5
@@ -41,3 +41,14 @@ export function derivePromptCharBudget(models, { charsPerToken = 3, floor = 1500
|
||||
if (windows.length === 0) return floor;
|
||||
return Math.max(floor, Math.max(...windows) * charsPerToken);
|
||||
}
|
||||
|
||||
// Resolve the effective budget from the env var + SPOT. TRUTHINESS (not != null) on the env
|
||||
// value deliberately: an EMPTY value ("CLAUDE_MAX_PROMPT_CHARS=" in a systemd EnvironmentFile
|
||||
// or .env) must mean "use the default" — exactly the old `parseInt(env || "150000")` contract.
|
||||
// Treating "" as explicit gives parseInt("") = NaN, and a NaN cap silently DISABLES the
|
||||
// runaway-context guard while injecting a false "[System] Note: 0 older messages were
|
||||
// truncated" line into every prompt (caught in PR #179 review). Non-empty garbage still
|
||||
// parses to NaN — the pre-existing class, slated for parseIntEnv routing in PR #154.
|
||||
export function resolvePromptCharBudget(rawEnv, models, opts) {
|
||||
return rawEnv ? parseInt(rawEnv, 10) : derivePromptCharBudget(models, opts);
|
||||
}
|
||||
|
||||
+2
-4
@@ -49,7 +49,7 @@ import { TuiSemaphore, SemaphoreAbortError, recordTuiEntrypoint, buildTuiHealthB
|
||||
import { TuiPanePool, resolvePoolSize, POOL_MAX_SIZE } from "./lib/tui/pool.mjs";
|
||||
import { TuiDeltaAssembler, DEFAULT_HOLDBACK_CHARS, resolveStreamHoldback } from "./lib/tui/stream.mjs";
|
||||
import { createSerialMutex, createTtlCache, isTokenExpiring, orderLabelsLastGoodFirst } from "./lib/spawn-auth.mjs";
|
||||
import { appendOperatorPrompt, derivePromptCharBudget } from "./lib/prompt.mjs";
|
||||
import { appendOperatorPrompt, resolvePromptCharBudget } from "./lib/prompt.mjs";
|
||||
|
||||
const __dirname = dirname(fileURLToPath(import.meta.url));
|
||||
const _pkg = JSON.parse(readFileSync(join(__dirname, "package.json"), "utf8"));
|
||||
@@ -1154,9 +1154,7 @@ function buildCliArgs(cliModel, systemPrompt) {
|
||||
// English tokens), which silently under-delivered the advertised window by ~5×. The env
|
||||
// var (and the runtime settings API below) remain absolute operator overrides. If
|
||||
// models.json ever advertises a bigger window, this budget follows automatically.
|
||||
let MAX_PROMPT_CHARS = process.env.CLAUDE_MAX_PROMPT_CHARS != null
|
||||
? parseInt(process.env.CLAUDE_MAX_PROMPT_CHARS, 10)
|
||||
: derivePromptCharBudget(modelsConfig.models);
|
||||
let MAX_PROMPT_CHARS = resolvePromptCharBudget(process.env.CLAUDE_MAX_PROMPT_CHARS, modelsConfig.models);
|
||||
|
||||
// Flatten OpenAI content (string | array of parts) to plain text for the prompt.
|
||||
// Array content: concatenate text parts; replace non-text parts (e.g. image_url)
|
||||
|
||||
+16
-1
@@ -844,7 +844,7 @@ test("doctor falls back to currentVersion when origin/main unreachable (no stale
|
||||
// contract lives in lib/prompt.mjs. Mutation-proof: make appendOperatorPrompt
|
||||
// return `base` unconditionally and the first test fails; make it stop trimming
|
||||
// and the whitespace test fails.
|
||||
import { appendOperatorPrompt, derivePromptCharBudget } from "./lib/prompt.mjs";
|
||||
import { appendOperatorPrompt, derivePromptCharBudget, resolvePromptCharBudget } from "./lib/prompt.mjs";
|
||||
|
||||
console.log("\nPrompt-char budget (ADR 0009 — SPOT-derived):");
|
||||
|
||||
@@ -873,6 +873,21 @@ test("derivePromptCharBudget: charsPerToken and floor are tunable parameters", (
|
||||
assert.equal(derivePromptCharBudget([], { floor: 42 }), 42);
|
||||
});
|
||||
|
||||
// PR #179 review regression: EMPTY env value must mean "use the default" (the old
|
||||
// `parseInt(env || "150000")` contract). Mutation-proof: switch the resolver's
|
||||
// truthiness check to `!= null` and the empty-string test fails (NaN ≠ 600000).
|
||||
test("resolvePromptCharBudget: empty/unset env → SPOT-derived default, never NaN", () => {
|
||||
const models = [{ contextWindow: 200000 }];
|
||||
assert.equal(resolvePromptCharBudget("", models), 600000, "CLAUDE_MAX_PROMPT_CHARS= (empty) must fall back to derived");
|
||||
assert.equal(resolvePromptCharBudget(undefined, models), 600000);
|
||||
});
|
||||
|
||||
test("resolvePromptCharBudget: a set env value overrides the derivation absolutely", () => {
|
||||
const models = [{ contextWindow: 200000 }];
|
||||
assert.equal(resolvePromptCharBudget("300000", models), 300000);
|
||||
assert.equal(resolvePromptCharBudget("150000", models), 150000, "explicit legacy value wins over the bigger derived default");
|
||||
});
|
||||
|
||||
console.log("\nSystem-prompt operator append:");
|
||||
|
||||
test("appendOperatorPrompt: appends the operator prompt LAST, blank-line separated", () => {
|
||||
|
||||
Reference in New Issue
Block a user