mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 22:15:33 +00:00
fix(facts): surface chat-gateway unavailability instead of silent success-shaped no-op (#3062)
- runFactsBackstop records skipped: 'chat_unavailable' (both modes) and declines to enqueue facts-absorb jobs guaranteed to no-op. - New gateway unavailableReason()/warnUnavailableOnce() name the configured model, the missing auth_env keys, and the recipe setup_url; once-per- process stderr warn fires at the extractFactsFromTurn guard and at expand()'s silent single-query degrade (tokenmax's headline knob). - doctor facts_health distinguishes '0 active facts, chat gateway unreachable' (warn) from a genuinely empty brain (ok). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
393805ab2c
commit
c11ec1166f
@@ -7090,11 +7090,16 @@ export async function buildChecks(
|
||||
);
|
||||
if (factsExists[0]?.exists) {
|
||||
const health = await engine.getFactsHealth('default');
|
||||
const status: 'ok' | 'warn' = health.total_active >= 0 ? 'ok' : 'warn';
|
||||
const top = health.top_entities
|
||||
.slice(0, 3)
|
||||
.map(t => `${t.entity_slug}:${t.count}`)
|
||||
.join(', ') || '—';
|
||||
// #3062: "0 active facts" used to read [OK] even when the chat
|
||||
// gateway had never been reachable — indistinguishable from a brain
|
||||
// with genuinely nothing to extract. Distinguish the two.
|
||||
const { unavailableReason } = await import('../core/ai/gateway.ts');
|
||||
const chatDown = health.total_active === 0 ? unavailableReason('chat') : null;
|
||||
const status: 'ok' | 'warn' = chatDown ? 'warn' : health.total_active >= 0 ? 'ok' : 'warn';
|
||||
checks.push({
|
||||
name: 'facts_health',
|
||||
status,
|
||||
@@ -7102,7 +7107,8 @@ export async function buildChecks(
|
||||
`facts_health(default): ${health.total_active} active, ` +
|
||||
`${health.total_today} today, ${health.total_week} this week, ` +
|
||||
`${health.total_consolidated} consolidated, ` +
|
||||
`top entities ${top}`,
|
||||
`top entities ${top}` +
|
||||
(chatDown ? ` — extraction has no chat gateway: ${chatDown}` : ''),
|
||||
});
|
||||
} else {
|
||||
checks.push({
|
||||
|
||||
+48
-1
@@ -621,6 +621,7 @@ export function resetGateway(): void {
|
||||
_embedTransportInstalled = false;
|
||||
_chatTransport = null;
|
||||
_warnedRecipes.clear();
|
||||
_warnedUnavailable.clear();
|
||||
_extendedModels.clear();
|
||||
}
|
||||
|
||||
@@ -887,6 +888,47 @@ export function isAvailable(touchpoint: TouchpointKind, modelOverride?: string):
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Human-readable reason a chat-capable touchpoint is unavailable, or null
|
||||
* when it IS available. Names the configured model, the missing
|
||||
* `auth_env.required` keys, and the recipe's `setup_url` so silent-degrade
|
||||
* guards (#3062: facts extraction, query expansion) can warn with an
|
||||
* actionable message instead of no-oping invisibly.
|
||||
*/
|
||||
export function unavailableReason(touchpoint: 'chat' | 'expansion'): string | null {
|
||||
if (isAvailable(touchpoint)) return null;
|
||||
if (!_config) return `${touchpoint} gateway not configured (no AI gateway config loaded)`;
|
||||
try {
|
||||
const modelStr = touchpoint === 'expansion' ? getExpansionModel() : getChatModel();
|
||||
const { recipe } = resolveRecipe(modelStr);
|
||||
if (!recipe.touchpoints[touchpoint]) {
|
||||
return `${touchpoint} model ${modelStr}: provider recipe "${recipe.id}" does not support the ${touchpoint} touchpoint`;
|
||||
}
|
||||
const missing = (recipe.auth_env?.required ?? []).filter(k => !_config!.env[k]);
|
||||
if (missing.length > 0) {
|
||||
const setup = recipe.auth_env?.setup_url ? ` (get a key: ${recipe.auth_env.setup_url})` : '';
|
||||
return `${touchpoint} model ${modelStr} needs ${missing.join(', ')}${setup}`;
|
||||
}
|
||||
return `${touchpoint} model ${modelStr} is unavailable`;
|
||||
} catch (e) {
|
||||
return `${touchpoint} gateway unavailable: ${e instanceof Error ? e.message : String(e)}`;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Once-per-process memo for silent-degrade warnings (#3062). Cleared by
|
||||
* resetGateway() so a reconfigure gets a fresh warning if still broken.
|
||||
*/
|
||||
const _warnedUnavailable = new Set<string>();
|
||||
export function warnUnavailableOnce(touchpoint: 'chat' | 'expansion', context: string): void {
|
||||
if (_warnedUnavailable.has(touchpoint)) return;
|
||||
const reason = unavailableReason(touchpoint);
|
||||
if (!reason) return;
|
||||
_warnedUnavailable.add(touchpoint);
|
||||
// eslint-disable-next-line no-console
|
||||
console.warn(`[ai.gateway] WARN: ${context} — ${reason}`);
|
||||
}
|
||||
|
||||
// ---- Embedding ----
|
||||
|
||||
/**
|
||||
@@ -2306,7 +2348,12 @@ const ExpansionSchema = z.object({
|
||||
*/
|
||||
export async function expand(query: string): Promise<string[]> {
|
||||
if (!query || !query.trim()) return [query];
|
||||
if (!isAvailable('expansion')) return [query];
|
||||
if (!isAvailable('expansion')) {
|
||||
// #3062: tokenmax's headline knob silently degrading to single-query
|
||||
// was invisible. Warn once per process; still degrade gracefully.
|
||||
warnUnavailableOnce('expansion', 'query expansion is configured but inert; searches run single-query');
|
||||
return [query];
|
||||
}
|
||||
|
||||
// Guardrail seam: classify the query before the expansion model call.
|
||||
await classifyGatewayGuardrail({
|
||||
|
||||
@@ -78,7 +78,7 @@ export type FactsBackstopResult =
|
||||
mode: 'queue';
|
||||
enqueued: boolean;
|
||||
queueDepth: number;
|
||||
skipped?: 'extraction_disabled' | 'queue_overflow' | 'queue_shutdown' | `eligibility_failed:${string}`;
|
||||
skipped?: 'extraction_disabled' | 'chat_unavailable' | 'queue_overflow' | 'queue_shutdown' | `eligibility_failed:${string}`;
|
||||
}
|
||||
| {
|
||||
mode: 'inline';
|
||||
@@ -86,7 +86,7 @@ export type FactsBackstopResult =
|
||||
duplicate: number;
|
||||
superseded: number;
|
||||
fact_ids: number[];
|
||||
skipped?: 'extraction_disabled' | `eligibility_failed:${string}`;
|
||||
skipped?: 'extraction_disabled' | 'chat_unavailable' | `eligibility_failed:${string}`;
|
||||
};
|
||||
|
||||
interface ParsedPageInput {
|
||||
@@ -155,6 +155,18 @@ export async function runFactsBackstop(
|
||||
: { mode: 'inline', inserted: 0, duplicate: 0, superseded: 0, fact_ids: [], skipped };
|
||||
}
|
||||
|
||||
// #3062: no chat gateway → extraction is guaranteed to yield nothing. The
|
||||
// result was previously byte-identical to a genuine empty extraction
|
||||
// (inserted: 0, no `skipped`), and queue mode enqueued jobs doomed to
|
||||
// no-op. Record WHY, warn once per process, and skip the queue entirely.
|
||||
const { isAvailable, warnUnavailableOnce } = await import('../ai/gateway.ts');
|
||||
if (!isAvailable('chat')) {
|
||||
warnUnavailableOnce('chat', 'facts extraction skipped');
|
||||
return mode === 'queue'
|
||||
? { mode: 'queue', enqueued: false, queueDepth: 0, skipped: 'chat_unavailable' }
|
||||
: { mode: 'inline', inserted: 0, duplicate: 0, superseded: 0, fact_ids: [], skipped: 'chat_unavailable' };
|
||||
}
|
||||
|
||||
// --- Mode dispatch ---
|
||||
if (mode === 'queue') {
|
||||
// Local patch 2026-06-11: in a one-shot CLI process the in-process queue
|
||||
|
||||
@@ -21,7 +21,7 @@
|
||||
* gateway-down errors are absorbed into NULL-embedding rows.
|
||||
*/
|
||||
|
||||
import { chat, embedOne, isAvailable } from '../ai/gateway.ts';
|
||||
import { chat, embedOne, isAvailable, warnUnavailableOnce } from '../ai/gateway.ts';
|
||||
import type { ChatResult } from '../ai/gateway.ts';
|
||||
import { INJECTION_PATTERNS } from '../think/sanitize.ts';
|
||||
import { resolveModel } from '../model-config.ts';
|
||||
@@ -175,7 +175,10 @@ export async function extractFactsFromTurn(input: ExtractInput): Promise<Extract
|
||||
|
||||
if (!isAvailable('chat')) {
|
||||
// No chat gateway → no extraction. Caller still inserts facts via direct
|
||||
// `gbrain take add` paths.
|
||||
// `gbrain take add` paths. #3062: warn once per process — silently
|
||||
// returning [] here made an unauthenticated brain byte-identical to a
|
||||
// genuine empty extraction across every health surface.
|
||||
warnUnavailableOnce('chat', 'facts extraction skipped');
|
||||
return [];
|
||||
}
|
||||
|
||||
|
||||
@@ -83,16 +83,16 @@ describe('put_page facts backstop', () => {
|
||||
'note/substantive',
|
||||
`---\ntype: note\ntitle: Substantive\n---\n${'this is some real content with meaningful claims. '.repeat(10)}`,
|
||||
);
|
||||
// Either queued (gateway configured) or skipped due to gateway absence
|
||||
// is acceptable; we only insist the gating doesn't reject on the
|
||||
// happy path.
|
||||
// Either queued (gateway configured) or skipped: 'chat_unavailable'
|
||||
// (#3062: the reset gateway has no chat credential, and the backstop
|
||||
// now records that instead of enqueueing a job doomed to no-op) is
|
||||
// acceptable; we only insist the gating doesn't reject on the happy path.
|
||||
expect(result).toBeDefined();
|
||||
const r = result!;
|
||||
if ('queued' in r) {
|
||||
expect(r.queued).toBe(true);
|
||||
} else {
|
||||
// 'backstop_error' or 'queue_shutdown' would be a real failure.
|
||||
expect(r.skipped).toMatch(/^(queue_shutdown|backstop_error)?$/);
|
||||
expect(r.skipped).toBe('chat_unavailable');
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
@@ -25,8 +25,11 @@ import {
|
||||
resetGateway,
|
||||
__setChatTransportForTests,
|
||||
getChatModel,
|
||||
unavailableReason,
|
||||
warnUnavailableOnce,
|
||||
} from '../src/core/ai/gateway.ts';
|
||||
import { extractFactsFromTurn } from '../src/core/facts/extract.ts';
|
||||
import { runFactsBackstop } from '../src/core/facts/backstop.ts';
|
||||
|
||||
beforeEach(() => {
|
||||
resetGateway();
|
||||
@@ -147,3 +150,89 @@ describe('facts extract — silent-no-op regression (v0.31.6 bug class)', () =>
|
||||
expect(chatCalled).toBe(true); // ← THE bug-class assertion
|
||||
});
|
||||
});
|
||||
|
||||
// #3062 — an unauthenticated chat gateway must be DIAGNOSABLE, not a
|
||||
// silent success-shaped no-op. Three surfaces pinned here:
|
||||
// 1. unavailableReason() names the missing auth_env key + model.
|
||||
// 2. warnUnavailableOnce() writes exactly one stderr warning per process.
|
||||
// 3. runFactsBackstop() records skipped: 'chat_unavailable' (and queue
|
||||
// mode declines to enqueue a job that is guaranteed to no-op).
|
||||
describe('#3062 — chat-unavailable is diagnosable, not silent', () => {
|
||||
test('unavailableReason names the missing auth env key and the model', () => {
|
||||
configureGateway({
|
||||
chat_model: 'anthropic:claude-sonnet-4-6',
|
||||
env: {},
|
||||
});
|
||||
expect(isAvailable('chat')).toBe(false);
|
||||
const reason = unavailableReason('chat');
|
||||
expect(reason).toContain('ANTHROPIC_API_KEY');
|
||||
expect(reason).toContain('anthropic:claude-sonnet-4-6');
|
||||
});
|
||||
|
||||
test('unavailableReason is null when the touchpoint is available', () => {
|
||||
configureGateway({
|
||||
chat_model: 'anthropic:claude-sonnet-4-6',
|
||||
env: { ANTHROPIC_API_KEY: 'sk-ant-test' },
|
||||
});
|
||||
expect(unavailableReason('chat')).toBeNull();
|
||||
});
|
||||
|
||||
test('warnUnavailableOnce warns exactly once per process per touchpoint', () => {
|
||||
configureGateway({
|
||||
chat_model: 'anthropic:claude-sonnet-4-6',
|
||||
env: {},
|
||||
});
|
||||
const seen: string[] = [];
|
||||
const orig = console.warn;
|
||||
// eslint-disable-next-line no-console
|
||||
console.warn = (msg: unknown) => { seen.push(String(msg)); };
|
||||
try {
|
||||
warnUnavailableOnce('chat', 'facts extraction skipped');
|
||||
warnUnavailableOnce('chat', 'facts extraction skipped');
|
||||
} finally {
|
||||
// eslint-disable-next-line no-console
|
||||
console.warn = orig;
|
||||
}
|
||||
expect(seen).toHaveLength(1);
|
||||
expect(seen[0]).toContain('ANTHROPIC_API_KEY');
|
||||
});
|
||||
|
||||
test('runFactsBackstop records skipped: chat_unavailable instead of a success-shaped empty result', async () => {
|
||||
configureGateway({
|
||||
chat_model: 'anthropic:claude-sonnet-4-6',
|
||||
env: {},
|
||||
});
|
||||
// The gate fires before any engine use beyond the kill-switch config
|
||||
// read, so a getConfig stub suffices — no PGLite needed.
|
||||
const stubEngine = { getConfig: async () => null } as unknown as import('../src/core/engine.ts').BrainEngine;
|
||||
const page = {
|
||||
slug: 'note/eligible',
|
||||
type: 'note' as const,
|
||||
compiled_truth: 'this is some real content with meaningful claims. '.repeat(10),
|
||||
frontmatter: {},
|
||||
};
|
||||
const inline = await runFactsBackstop(page, {
|
||||
engine: stubEngine,
|
||||
sourceId: 'default',
|
||||
sessionId: null,
|
||||
source: 'mcp:put_page',
|
||||
mode: 'inline',
|
||||
});
|
||||
expect(inline).toEqual({
|
||||
mode: 'inline', inserted: 0, duplicate: 0, superseded: 0, fact_ids: [],
|
||||
skipped: 'chat_unavailable',
|
||||
});
|
||||
|
||||
const queued = await runFactsBackstop(page, {
|
||||
engine: stubEngine,
|
||||
sourceId: 'default',
|
||||
sessionId: null,
|
||||
source: 'sync:import',
|
||||
mode: 'queue',
|
||||
});
|
||||
expect(queued).toEqual({
|
||||
mode: 'queue', enqueued: false, queueDepth: 0,
|
||||
skipped: 'chat_unavailable',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user