mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-11-14 10:39:32 +01:00
fix(custom-model): core review fixes for context-length discovery and injection
- share one context-length resolution (discovered figure, endpoint default fallback) across the routes and boot recovery, with a recovery test - record listed per-slot context figures with a slot count of 1 and force the slot-count fallback once per rediscovery - open up the codex model launch token for qwen3-style ids (model-plain-colon), pinned by a spawn-golden case - pass the full discovered window to opencode as limit.context - translate the picker search strings and document the discovery precedence
This commit is contained in:
1 parent
dbd1b6e587
commit
1b8ac40baf
16 files changed
+256
-47
No files matched your search
@@ -171,6 +171,15 @@ describe('codex', () => {
|
||||
expect(cx({ model: 'gpt-5', resumeSessionId: 'roll_42' })).toBe('codex --model gpt-5 resume roll_42');
|
||||
});
|
||||
|
||||
it('carries an Ollama-style `qwen3:32b` model id, and still drops a leading-flag shape', () => {
|
||||
// `model-plain-colon`: the custom-model feature forces the selected id onto this param,
|
||||
// and ids like `qwen3:32b` carry a colon. The leading-alphanumeric rule is what keeps a
|
||||
// flag from riding the flag, so `:x` and `-x` must still drop out.
|
||||
expect(cx({ model: 'qwen3:32b' })).toBe('codex --model qwen3:32b');
|
||||
expect(cx({ model: ':leading' })).toBe('codex');
|
||||
expect(cx({ model: '-yolo' })).toBe('codex');
|
||||
});
|
||||
|
||||
it('sends reasoning effort as one model_reasoning_effort config value, for every level', () => {
|
||||
for (const level of CODEX_REASONING_EFFORTS) {
|
||||
expect(cx({ reasoningEffort: level })).toBe(`codex --config model_reasoning_effort=${level}`);
|
||||
|
||||
@@ -481,6 +481,52 @@ describe('refreshAllCustomModelHosts: parallel-slot enrichment (llama.cpp splits
|
||||
const [updated] = await readCustomModelHosts(dir);
|
||||
expect(updated.modelSlotCounts).toEqual({ a: 4 });
|
||||
});
|
||||
|
||||
it('a round where only the listed figure answers resets the slot count a loaded round learned', async () => {
|
||||
// The false-warning scenario, measured on a real llama-swap: round 1 has the model
|
||||
// loaded under `--ctx-size 262144 --parallel 4` — a TOTAL window shared across slots,
|
||||
// correctly stored with its count. Round 2 finds it unloaded and takes the LISTED
|
||||
// `context_length`, which llama-swap documents as the PER-SLOT window. If the stored 4
|
||||
// survived, the floor check would divide the per-slot figure a second time
|
||||
// (65536/4 = 16384) and warn about a model that serves 65536 to every request.
|
||||
const dir = getDataDir();
|
||||
await writeCustomModelHosts(dir, [host({ id: 'ep', baseUrl: 'http://localhost:8080' })]);
|
||||
fetchMock.mockImplementation(async (url: URL) => {
|
||||
if (url.pathname === '/v1/models') {
|
||||
return new Response(JSON.stringify({ data: [{ id: 'm', status: { value: 'loaded' } }] }), { status: 200 });
|
||||
}
|
||||
if (url.pathname === '/running') {
|
||||
return new Response(
|
||||
JSON.stringify({
|
||||
running: [{ model: 'm', state: 'ready', cmd: 'llama-server -m /models/m.gguf --ctx-size 262144 --parallel 4' }],
|
||||
}),
|
||||
{ status: 200 }
|
||||
);
|
||||
}
|
||||
throw new Error(`unexpected request: ${url.href}`); // the command answers both questions
|
||||
});
|
||||
|
||||
await refreshAllCustomModelHosts();
|
||||
|
||||
let [round1] = await readCustomModelHosts(dir);
|
||||
expect(round1.modelContextLengths).toEqual({ m: 262144 });
|
||||
expect(round1.modelSlotCounts).toEqual({ m: 4 });
|
||||
|
||||
// Round 2: unloaded, no /running or /props probe in order, listing its per-slot figure.
|
||||
fetchMock.mockResolvedValue(
|
||||
new Response(JSON.stringify({ data: [{ id: 'm', status: { value: 'unloaded' }, context_length: 65536 }] }), {
|
||||
status: 200,
|
||||
})
|
||||
);
|
||||
|
||||
await refreshAllCustomModelHosts();
|
||||
|
||||
const [round2] = await readCustomModelHosts(dir);
|
||||
expect(round2.modelContextLengths).toEqual({ m: 65536 });
|
||||
// 1 slot, not the stale 4: 65536 per slot with 1 recorded slot is the honest pair, so
|
||||
// the floor check reads 65536/request — exactly what the server gives a single request.
|
||||
expect(round2.modelSlotCounts).toEqual({ m: 1 });
|
||||
});
|
||||
});
|
||||
|
||||
describe('refreshAllCustomModelHosts: model-size enrichment (parsed from /v1/models description)', () => {
|
||||
|
||||
@@ -112,20 +112,20 @@ describe('buildCustomModelInjection', () => {
|
||||
expect(parsed.provider.custom.models.qwen3).toEqual({});
|
||||
});
|
||||
|
||||
it('opencode: threads a discovered context length into the model limit with response headroom (issue #749)', () => {
|
||||
it('opencode: threads a discovered context length into the model limit as the full window (issue #749)', () => {
|
||||
const result = buildCustomModelInjection(entryOrThrow('opencode'), endpoint, 'qwen3', 16384);
|
||||
if (result.kind !== 'env') throw new Error('unreachable');
|
||||
const parsed = JSON.parse(result.envOverrides.OPENCODE_CONFIG_CONTENT);
|
||||
// One quarter held back for the reply: context 16384-4096, output 4096 — the sum is the
|
||||
// real window, so a full-size prompt plus a max-length reply never exceeds the server.
|
||||
expect(parsed.provider.custom.models.qwen3).toEqual({ limit: { context: 12288, output: 4096 } });
|
||||
// `limit.context` IS the full window to opencode — it reserves for the reply itself —
|
||||
// so the discovered total passes through; `output` only caps the reply at a quarter.
|
||||
expect(parsed.provider.custom.models.qwen3).toEqual({ limit: { context: 16384, output: 4096 } });
|
||||
});
|
||||
|
||||
it('opencode: caps the response headroom reservation at 8192 for big windows', () => {
|
||||
const result = buildCustomModelInjection(entryOrThrow('opencode'), endpoint, 'qwen3', 262144);
|
||||
if (result.kind !== 'env') throw new Error('unreachable');
|
||||
const parsed = JSON.parse(result.envOverrides.OPENCODE_CONFIG_CONTENT);
|
||||
expect(parsed.provider.custom.models.qwen3).toEqual({ limit: { context: 253952, output: 8192 } });
|
||||
expect(parsed.provider.custom.models.qwen3).toEqual({ limit: { context: 262144, output: 8192 } });
|
||||
});
|
||||
|
||||
it('opencode: never invents a limit for an unknown or non-finite context length (issue #749 regression)', () => {
|
||||
|
||||
@@ -0,0 +1,91 @@
|
||||
/**
|
||||
* @fileoverview Boot recovery rebuilds a saved custom-model selection through the SAME
|
||||
* context-length resolution the routes use: the model's discovered figure, falling back to
|
||||
* the endpoint's `defaultContextLength` (`resolveCustomModelContextLength`). Before the
|
||||
* shared helper, `_rebuildCustomModelEnv` read `modelContextLengths` alone — so a session
|
||||
* on a model that only the endpoint default covers came back from a Codeman restart with
|
||||
* `CLAUDE_CODE_MAX_CONTEXT_TOKENS` (or a config file's context figure) silently gone,
|
||||
* re-arming exactly the window overflow the var exists to prevent.
|
||||
*
|
||||
* `_rebuildCustomModelEnv` reads no `this` state (it re-derives everything from the endpoint
|
||||
* store and the Session it is handed), so it is driven straight off the prototype with an
|
||||
* empty receiver — `restoreMuxSessions()` itself cannot be reached under vitest, the same
|
||||
* constraint recorded in test/session-model-recovery.test.ts.
|
||||
*/
|
||||
import { afterEach, describe, expect, it } from 'vitest';
|
||||
import { getDataDir } from '../src/config/instance.js';
|
||||
import { writeCustomModelHosts, type CustomModelHost } from '../src/custom-model-hosts.js';
|
||||
import { customModelConfigDir, removeConfigDir } from '../src/custom-model-injection-apply.js';
|
||||
import { Session } from '../src/session.js';
|
||||
import { WebServer } from '../src/web/server.js';
|
||||
import type { CustomModelBookkeeping } from '../src/types/session.js';
|
||||
|
||||
type RebuildFn = (session: Session, saved: CustomModelBookkeeping) => Promise<Record<string, string> | undefined>;
|
||||
|
||||
/** The real method, with an empty receiver — see the file header for why that is sound. */
|
||||
function rebuildCustomModelEnv(session: Session, saved: CustomModelBookkeeping) {
|
||||
const fn = (WebServer.prototype as unknown as { _rebuildCustomModelEnv: RebuildFn })._rebuildCustomModelEnv;
|
||||
expect(typeof fn, '_rebuildCustomModelEnv vanished from WebServer.prototype').toBe('function');
|
||||
return fn.call({}, session, saved);
|
||||
}
|
||||
|
||||
function endpoint(overrides: Partial<CustomModelHost> = {}): CustomModelHost {
|
||||
return {
|
||||
id: 'ep',
|
||||
label: 'Ep',
|
||||
baseUrl: 'http://localhost:8080',
|
||||
apiKey: 'recovery-probe-key',
|
||||
models: ['qwen3'],
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
describe('_rebuildCustomModelEnv: context-length fallback on boot recovery', () => {
|
||||
const configs: string[] = [];
|
||||
|
||||
afterEach(() => {
|
||||
for (const id of configs.splice(0)) removeConfigDir(customModelConfigDir(id));
|
||||
});
|
||||
|
||||
it('falls back to the endpoint defaultContextLength when discovery recorded no per-model figure', async () => {
|
||||
await writeCustomModelHosts(getDataDir(), [endpoint({ defaultContextLength: 32768 })]);
|
||||
const session = new Session({ workingDir: '/tmp', mode: 'claude' });
|
||||
configs.push(session.id);
|
||||
|
||||
const env = await rebuildCustomModelEnv(session, { endpointId: 'ep', modelId: 'qwen3', envKeys: [] });
|
||||
|
||||
expect(env?.CLAUDE_CODE_MAX_CONTEXT_TOKENS).toBe('32768');
|
||||
});
|
||||
|
||||
it('a discovered per-model figure still wins over the endpoint default', async () => {
|
||||
await writeCustomModelHosts(getDataDir(), [
|
||||
endpoint({ modelContextLengths: { qwen3: 131072 }, defaultContextLength: 32768 }),
|
||||
]);
|
||||
const session = new Session({ workingDir: '/tmp', mode: 'claude' });
|
||||
configs.push(session.id);
|
||||
|
||||
const env = await rebuildCustomModelEnv(session, { endpointId: 'ep', modelId: 'qwen3', envKeys: [] });
|
||||
|
||||
expect(env?.CLAUDE_CODE_MAX_CONTEXT_TOKENS).toBe('131072');
|
||||
});
|
||||
|
||||
it('states nothing rather than guessing when neither source knows a window', async () => {
|
||||
await writeCustomModelHosts(getDataDir(), [endpoint()]);
|
||||
const session = new Session({ workingDir: '/tmp', mode: 'claude' });
|
||||
configs.push(session.id);
|
||||
|
||||
const env = await rebuildCustomModelEnv(session, { endpointId: 'ep', modelId: 'qwen3', envKeys: [] });
|
||||
|
||||
expect(env?.CLAUDE_CODE_MAX_CONTEXT_TOKENS).toBeUndefined();
|
||||
});
|
||||
|
||||
it('drops the selection to undefined (kept for clearing) when the endpoint no longer exists', async () => {
|
||||
await writeCustomModelHosts(getDataDir(), []);
|
||||
const session = new Session({ workingDir: '/tmp', mode: 'claude' });
|
||||
configs.push(session.id);
|
||||
|
||||
await expect(rebuildCustomModelEnv(session, { endpointId: 'gone', modelId: 'qwen3', envKeys: [] })).resolves.toBe(
|
||||
undefined
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user