Files
oh-my-pi/packages/coding-agent/test/agent-session-compaction-thinking-threading.test.ts
T
cognitive 25c6794cd5 fix(agent): compaction honors session thinking level and silent-clamps unsupported-effort models
Triple-stacked failure on the same axis (thinking effort) produced the
user-visible

    Error: Compaction failed: Thinking effort high is not supported by
           xai-oauth/grok-build.
    Supported efforts:

(empty list after the colon) whenever the active model was a curated
xAI catalog entry with compat.supportsReasoningEffort: false.

Three defects lined up. (1) Behavior: compaction at four call sites
in packages/agent/src/compaction/compaction.ts hardcoded
reasoning: Effort.High and never threaded session.thinkingLevel —
the user's /model :off selection (and any explicit low/medium) was
silently overridden. On every other model this was invisible.
(2) Validation: requireSupportedEffort threw at the openai-flavored
mapper layer before the wire-side omitReasoningEffort gate in
providers/xai-responses.ts ever ran; two contradictory guards on the
same wire param. (3) Message: when getSupportedEfforts returned [],
the rendered error tail was 'Supported efforts: ' with nothing after
the colon — disappears as a side-effect of fix #2.

Fix #1 — thread ThinkingLevel | undefined end-to-end. Add
SummaryOptions.thinkingLevel and HandoffOptions.thinkingLevel.
Convert via a single exhaustive switch (effortFromThinkingLevel) in
the new resolveCompactionEffort helper:
  - Off            → undefined  (omit reasoning entirely)
  - undefined/Inherit → Effort.High → clamp per model (preserves the
                                       historical default for users
                                       who never touched the dial)
  - explicit Effort → respect user → clamp per model

resolveCompactionEffort lives in compaction.ts; all four call sites
(generateSummary, generateHandoff, generateShortSummary,
generateTurnPrefixSummary) route through it. agent-session.ts threads
this.thinkingLevel into all three production compaction entry points
(manual /compact at L6201, auto-compaction at L6458 — the most-fired
path, originally missed in plan review — and direct generateHandoff
at L5465). The audit-gate test
(test/agent-session-compaction-thinking-threading.test.ts) scans the
file with a brace-balanced extractor and refuses any unthreaded site.

Fix #2 — silent-clamp at the openai-flavored mapper layer. Extract
exported modelOmitsReasoningEffort(model) in model-thinking.ts as the
single source of truth for compat.supportsReasoningEffort: false on
openai-responses* APIs. getSupportedEfforts now calls it instead of
inlining the check (pure refactor — observable behavior preserved).
resolveOpenAiReasoningEffort in stream.ts early-returns undefined
when the predicate is true, so the wire-side omitReasoningEffort
gate (providers/xai-responses.ts:78) becomes the single source of
truth for the actual strip — no redundant throw.

Three regression tests pin the contract:
  - packages/ai/test/xai-oauth-effort-strip.test.ts (5 tests):
    modelOmitsReasoningEffort returns true for grok-build and
    grok-4.20-0309-reasoning, false for grok-4.3 / Anthropic /
    openai-completions.
  - packages/agent/test/compaction-thinking-level.test.ts (5 tests):
    every ThinkingLevel outcome through generateHandoff — Off stays
    undefined (not coerced to High), Low stays Low, Inherit / undefined
    default to High, grok-build clamps to undefined regardless of
    requested level. Covers the Codex-caught Off-vs-not-provided
    distinction.
  - packages/coding-agent/test/agent-session-compaction-thinking-threading.test.ts
    (2 tests): brace-balanced source scan asserts every direct
    compact() / generateHandoff() in agent-session.ts threads
    'thinkingLevel: this.thinkingLevel'; floor of 3 threaded sites.

TDD red-green verified for fix #1: temporarily reverted the handoff
call-site back to hardcoded Effort.High → compaction-thinking-level
went 2 pass / 3 fail (Off coerced, Low overridden, grok-build throws);
restored → 5 pass / 0 fail.

Verified:
  - packages/agent:  127 pass / 0 fail
  - packages/ai:     1061 pass / 337 skip / 0 fail
  - packages/coding-agent (focused): 179 pass / 5 skip / 0 fail
  - biome + tsgo --noEmit clean across all three packages

Out of scope (follow-ups):
  - branch-summarization.ts:307 already passes no reasoning — no edit.
  - The empty-list error message at model-thinking.ts:296 is now
    structurally unreachable from the openai-responses path.
  - modelOmitsReasoningEffort and grokSupportsReasoningEffort
    (xai-responses.ts:22) overlap; collapse into a single predicate
    in a future commit.

Op: correct
Restores: spec:compaction-honors-session-thinking-level
Restores: spec:xai-oauth-grok-build-compaction-no-throw
(cherry picked from commit e07b47ee46769053c658819437e2478389a4cee0)
2026-05-27 15:01:20 +00:00

166 lines
5.9 KiB
TypeScript

import { describe, expect, test } from "bun:test";
// Audit gate for the compaction-effort fix. The plan calls out three
// production call sites in `agent-session.ts` that MUST thread
// `thinkingLevel: this.thinkingLevel` into the compaction LLM options:
//
// - `compact(...)` at the manual `/compact` site (`#compactWithFallbackModel`)
// - `compact(...)` at the auto-compaction site (the most-fired path)
// - `generateHandoff(...)` at the handoff site
//
// `SummaryOptions.thinkingLevel` is optional, so a future contributor adding
// a new `compact(...)` or `generateHandoff(...)` call without threading it
// would silently fall back to the historical `Effort.High` default — exactly
// the regression Codex caught during plan review (auto-compaction was
// initially missed). The typecheck won't catch this; this test does.
const AGENT_SESSION_PATH = `${import.meta.dir}/../src/session/agent-session.ts`;
// Lines that are NOT direct LLM call sites and don't need threading:
// - The `async compact(...)` method declaration itself.
// - `this.compact(...)` invocations that route through the method (and from
// there into the threaded `#compactWithFallbackModel`).
const NON_LLM_CALL_PATTERNS = [
/async compact\(customInstructions/, // method declaration
/await this\.compact\(/, // self-invocation routes to threaded site
];
interface CallSite {
line: number;
headerLine: string;
callExpression: string;
}
/**
* Scan source for `compact(` / `generateHandoff(` and extract the
* brace-balanced call expression for each match. Skips comments,
* string contents, and non-LLM lines (method declarations, self-calls).
*/
function findCompactionCallSites(src: string): CallSite[] {
const lines = src.split("\n");
const sites: CallSite[] = [];
for (let lineIdx = 0; lineIdx < lines.length; lineIdx++) {
const line = lines[lineIdx];
if (!line) continue;
const headerMatch = /\b(compact|generateHandoff)\(/.exec(line);
if (!headerMatch) continue;
if (NON_LLM_CALL_PATTERNS.some(rx => rx.test(line))) continue;
// Skip lines that are themselves comments
const trimmed = line.trim();
if (trimmed.startsWith("//") || trimmed.startsWith("*") || trimmed.startsWith("/*")) continue;
// Compute absolute offset of the opening `(` after the matched name
let offset = 0;
for (let l = 0; l < lineIdx; l++) {
offset += (lines[l]?.length ?? 0) + 1; // +1 for newline
}
const openParenIdx = offset + headerMatch.index + headerMatch[0].length - 1;
// Walk forward, balancing parens. Track string / comment context to
// avoid counting `(`/`)` inside literals.
let depth = 0;
let i = openParenIdx;
let inString: '"' | "'" | "`" | null = null;
let inLineComment = false;
let inBlockComment = false;
let end = -1;
for (; i < src.length; i++) {
const ch = src[i];
const next = src[i + 1];
if (inLineComment) {
if (ch === "\n") inLineComment = false;
continue;
}
if (inBlockComment) {
if (ch === "*" && next === "/") {
inBlockComment = false;
i++;
}
continue;
}
if (inString) {
if (ch === "\\") {
i++;
continue;
}
if (ch === inString) inString = null;
continue;
}
if (ch === "/" && next === "/") {
inLineComment = true;
continue;
}
if (ch === "/" && next === "*") {
inBlockComment = true;
i++;
continue;
}
if (ch === '"' || ch === "'" || ch === "`") {
inString = ch;
continue;
}
if (ch === "(") depth++;
else if (ch === ")") {
depth--;
if (depth === 0) {
end = i;
break;
}
}
}
if (end === -1) {
throw new Error(`Unterminated call expression at agent-session.ts:${lineIdx + 1}`);
}
sites.push({
line: lineIdx + 1,
headerLine: line.trim(),
callExpression: src.slice(openParenIdx + 1, end),
});
}
return sites;
}
function expressionThreadsThinkingLevel(callExpression: string): boolean {
// `thinkingLevel:` must appear as a property key — i.e. preceded by `{`
// or `,` or whitespace + `,` or start of expression, and followed by a
// value. Loose check via the substring is sufficient because the call
// expression is brace-balanced (we extracted only one call's contents).
// We also require the value to be the session field, to defend against
// `thinkingLevel: undefined` accidentally satisfying the gate.
return /\bthinkingLevel\s*:\s*this\.thinkingLevel\b/.test(callExpression);
}
describe("agent-session.ts compaction threading (audit gate)", () => {
test("every direct compact()/generateHandoff() call threads thinkingLevel: this.thinkingLevel", async () => {
const src = await Bun.file(AGENT_SESSION_PATH).text();
const sites = findCompactionCallSites(src);
const offenders = sites.filter(s => !expressionThreadsThinkingLevel(s.callExpression));
expect(
offenders,
`Found ${offenders.length} compaction call site(s) missing 'thinkingLevel: this.thinkingLevel':\n${offenders
.map(o => ` agent-session.ts:${o.line} — ${o.headerLine}`)
.join(
"\n",
)}\n\nFix: add 'thinkingLevel: this.thinkingLevel' to the options object on every direct compact()/generateHandoff() call. The historical Effort.High default lives in resolveCompactionEffort (packages/agent/src/compaction/compaction.ts) and applies when thinkingLevel is undefined.`,
).toEqual([]);
});
test("at least 3 threaded sites exist (manual /compact + auto-compaction + handoff)", async () => {
const src = await Bun.file(AGENT_SESSION_PATH).text();
const sites = findCompactionCallSites(src);
const threaded = sites.filter(s => expressionThreadsThinkingLevel(s.callExpression));
// Floor at 3 — the plan explicitly enumerates three sites. If the
// production surface grows new entry points, the first test fails
// until they thread too; this guards against accidental removal of
// any of the original three. Count is derived from the same
// brace-balanced scanner as the offender check above.
expect(threaded.length).toBeGreaterThanOrEqual(3);
});
});