fix(facts): salvage valid extractor candidates (#3894)

docs/architecture/KEY_FILES.md

@@ -102,7 +102,7 @@ per-release `**vX.Y.Z:**` narration — CI enforces this
 - `src/commands/autopilot.ts` extension — tick body invokes `runNightlyQualityProbe` when `cfg.autopilot.nightly_quality_probe.enabled === true` (default OFF — opt-in to protect API spend). NO scheduler-side rate-limit check — `runNightlyQualityProbe`'s internal `shouldRunNightly` (reading the audit JSONL) is the single source of truth. Probe call wrapped in try/catch that logs via `logError` and does NOT bump `consecutiveErrors` (probe failure is informational, never crashes the loop). Default `max_usd` cap = 5. Pinned by `test/autopilot-nightly-probe-wiring.test.ts`.
 - `test/eval-replay-gate.test.ts` + `test/fixtures/eval-baselines/qrels-search.json` — hermetic retrieval qrels gate running in the standard PR unit-shard CI matrix (`.github/workflows/test.yml`, NOT the fixed-file E2E workflow). Uses the canonical PGLite block (test-isolation R3+R4) and the basis-vector embedding pattern from `test/e2e/search-quality.test.ts:23-28` for fully hermetic retrieval. The qrels fixture (12 queries) uses PLACEHOLDER names only (alice-example, widget-co-example, etc. — privacy rule) and embeds each query at a deterministic basis dimension so retrieval is reproducible. Each query lists `relevant_slugs[]` + `first_relevant_slug`; the test computes `top1_match_rate` (top-1 == first_relevant) and `recall@10` (fraction of relevant_slugs in top-10), asserting both meet floors (defaults `>= 0.80` and `>= 0.85`). Env-overridable floors `GBRAIN_REPLAY_GATE_TOP1_FLOOR` / `GBRAIN_REPLAY_GATE_RECALL_FLOOR` (via `withEnv()` per R1). Refresh discipline: when ranking changes intentionally move expected slugs, edit `qrels-search.json` directly with a `Why:` line in the commit body or the gate degrades to rubber-stamp. Pinned by `test/eval-replay-gate.test.ts` (incl. a privacy-grep regression guard against real-name reintroduction).
 - `src/core/cycle/nightly-quality-probe.ts` + `src/core/audit-quality-probe.ts` + `test/fixtures/longmemeval-nightly.jsonl` + `test/nightly-quality-probe.test.ts` — opt-in nightly cross-modal quality probe. The phase runs `gbrain eval longmemeval --by-type` against the committed 10-question placeholder fixture, pipes output through `gbrain eval cross-modal --batch --max-usd 5 --yes`, and writes one event per run to `~/.gbrain/audit/quality-probe-YYYY-Www.jsonl` (ISO-week-rotated, mirrors `audit-slug-fallback.ts`; honors `GBRAIN_AUDIT_DIR`). Default DISABLED — opt-in via `gbrain config set autopilot.nightly_quality_probe.enabled true` (prevents surprise API spend). 24h rate limit (pure `shouldRunNightly(now, recentEvents, windowMs?)`) skips with audit row `outcome: rate_limited`. Embedding-key short-circuit: longmemeval needs `gateway.embedQuery()`, so the phase exits early with `outcome: no_embedding_key` + stderr warn when no provider configured. Full DI surface via `NightlyProbeDeps` (`isEnabled`, `hasEmbeddingProvider`, `resolveMaxUsd`, `resolveRepoRoot`, `runLongMemEval`, `runCrossModalBatch`, `now`) so the unit test stubs every external effect. Cost ceiling: $5/run × 30 nights ≈ $150/month worst-case; expected ~$10.50/month. New `nightly_quality_probe_health` doctor check (`src/commands/doctor.ts`, right after `slug_fallback_audit`) reads last 7 days: SKIPPED when flag off (with enable command); OK when enabled + all PASS; WARN on any FAIL / ERROR / BUDGET_EXCEEDED with per-outcome counts. Pinned by `test/nightly-quality-probe.test.ts`.
-- `src/commands/eval-trajectory.ts` + `src/commands/founder-scorecard.ts` + `src/core/trajectory.ts` — temporal trajectory + founder scorecard. `gbrain eval trajectory <entity>` shows the chronological typed-claim history (mrr/arr/team_size/etc) with regressions auto-flagged inline; `gbrain founder scorecard <entity>` rolls up claim_accuracy / consistency / growth_trajectory / red_flags into one JSON. Pure-function math in `trajectory.ts`: `detectRegressions(points, threshold)` walks consecutive metric-value pairs per metric (10% drop default, env override `GBRAIN_TRAJECTORY_REGRESSION_THRESHOLD`); `computeDriftScore(points)` returns `1 - mean(cosine(emb[i], emb[i-1]))` over existing embeddings (null when <3 embedded points). Backed by `BrainEngine.findTrajectory(opts)` — both Postgres and PGLite, single SQL query, deterministic `ORDER BY valid_from ASC, id ASC`. Source-scoped via the `sourceId` scalar / `sourceIds` array dual pattern; visibility-filtered for remote callers. MCP op `find_trajectory` (read scope, NOT localOnly) registered after `find_experts`. Migration v67 adds optional typed-claim columns (`claim_metric`, `claim_value`, `claim_unit`, `claim_period`) + a partial index on `(entity_slug, claim_metric, valid_from) WHERE claim_metric IS NOT NULL`. Fence widens from 10 to 14 cells when any row has typed data; renderer stays at 10 cells when none do (no churn diff on existing fences). Metric labels normalize to lowercase snake_case via `normalizeMetricLabel` (15-entry seed map). The `consolidate` cycle phase does semantic upsert keyed on `(page_id, claim, since_date)` (fixes the duplicate-takes bug where re-running the cycle after `extract_facts` cleared `consolidated_at` appended duplicates via `MAX(row_num)+1`) and writes chronological `valid_until` on each cluster's older facts. The `extract_facts` cycle phase batch-embeds via `gateway.embed()` before insert AND threads `pages.effective_date` as the `pageEffectiveDate` fallback for `valid_from` (precedence: fence-row > pageEffectiveDate > now()). The contradiction probe MUST NOT write `valid_until` — grep guard at `test/eval-contradictions/no-valid-until-write.test.ts`. Haiku extraction lives in `src/core/facts/extract.ts` (not the `extract-facts.ts` cycle phase); its output cap is config `facts.extraction_max_tokens` (default 4000), a `stopReason: 'length'` response retries once at 2× the cap, and persistent truncation warns loudly on stderr instead of silently extracting zero facts; `pageEffectiveDate` is OPTIONAL because `fence-write.ts` callers have no Page object. Migration v89 adds a nullable `event_type TEXT` column on `facts` so the substrate carries event-shaped rows (`event_type='meeting'` / `'job_change'` / `'location_change'`) alongside metric rows. `TrajectoryPoint.event_type: string | null` projected by both engines. `TrajectoryOpts.kind?: 'metric' | 'event' | 'all'` filter (default `'all'`); `founder-scorecard` + `eval-trajectory` pass `kind: 'metric'` explicitly. Back-compat pinned by `test/regressions/v0_40_2_0-trajectory-backcompat.test.ts` (byte-identical `computeFounderScorecard` + `computeTrajectoryStats` with and without event rows); engine parity in `test/engine-parity-event-type.test.ts`.
+- `src/commands/eval-trajectory.ts` + `src/commands/founder-scorecard.ts` + `src/core/trajectory.ts` — temporal trajectory + founder scorecard. `gbrain eval trajectory <entity>` shows the chronological typed-claim history (mrr/arr/team_size/etc) with regressions auto-flagged inline; `gbrain founder scorecard <entity>` rolls up claim_accuracy / consistency / growth_trajectory / red_flags into one JSON. Pure-function math in `trajectory.ts`: `detectRegressions(points, threshold)` walks consecutive metric-value pairs per metric (10% drop default, env override `GBRAIN_TRAJECTORY_REGRESSION_THRESHOLD`); `computeDriftScore(points)` returns `1 - mean(cosine(emb[i], emb[i-1]))` over existing embeddings (null when <3 embedded points). Backed by `BrainEngine.findTrajectory(opts)` — both Postgres and PGLite, single SQL query, deterministic `ORDER BY valid_from ASC, id ASC`. Source-scoped via the `sourceId` scalar / `sourceIds` array dual pattern; visibility-filtered for remote callers. MCP op `find_trajectory` (read scope, NOT localOnly) registered after `find_experts`. Migration v67 adds optional typed-claim columns (`claim_metric`, `claim_value`, `claim_unit`, `claim_period`) + a partial index on `(entity_slug, claim_metric, valid_from) WHERE claim_metric IS NOT NULL`. Fence widens from 10 to 14 cells when any row has typed data; renderer stays at 10 cells when none do (no churn diff on existing fences). Metric labels normalize to lowercase snake_case via `normalizeMetricLabel` (15-entry seed map). The `consolidate` cycle phase does semantic upsert keyed on `(page_id, claim, since_date)` (fixes the duplicate-takes bug where re-running the cycle after `extract_facts` cleared `consolidated_at` appended duplicates via `MAX(row_num)+1`) and writes chronological `valid_until` on each cluster's older facts. The `extract_facts` cycle phase batch-embeds via `gateway.embed()` before insert AND threads `pages.effective_date` as the `pageEffectiveDate` fallback for `valid_from` (precedence: fence-row > pageEffectiveDate > now()). The contradiction probe MUST NOT write `valid_until` — grep guard at `test/eval-contradictions/no-valid-until-write.test.ts`. Haiku extraction lives in `src/core/facts/extract.ts` (not the `extract-facts.ts` cycle phase); its output cap is config `facts.extraction_max_tokens` (default 4000), a `stopReason: 'length'` response retries once at 2× the cap, and persistent truncation warns loudly on stderr instead of silently extracting zero facts. Mixed extractor arrays salvage valid candidates and warn with the dropped malformed count; all-malformed output still fails retryably, while an explicitly empty facts array remains a successful empty result. `pageEffectiveDate` is OPTIONAL because `fence-write.ts` callers have no Page object. Migration v89 adds a nullable `event_type TEXT` column on `facts` so the substrate carries event-shaped rows (`event_type='meeting'` / `'job_change'` / `'location_change'`) alongside metric rows. `TrajectoryPoint.event_type: string | null` projected by both engines. `TrajectoryOpts.kind?: 'metric' | 'event' | 'all'` filter (default `'all'`); `founder-scorecard` + `eval-trajectory` pass `kind: 'metric'` explicitly. Back-compat pinned by `test/regressions/v0_40_2_0-trajectory-backcompat.test.ts` (byte-identical `computeFounderScorecard` + `computeTrajectoryStats` with and without event rows); engine parity in `test/engine-parity-event-type.test.ts`.
 - `src/core/trajectory-format.ts` — shared `formatTrajectoryBlock(points, entitySlug, opts)` consumed by both `gbrain think` (production) and the LongMemEval harness (benchmark). Groups by `(metric ?? event_type)`, per-metric cap 20, total cap 100, knowledge_update intent annotates value-change rows with `(superseded prior)`. Emits a `<trajectory entity="...">` XML envelope — `INJECTION_PATTERNS` in `src/core/think/sanitize.ts` escapes `</trajectory>`, `<trajectory ...>` open tags, and attribute injection so adversarial fact text can't break out. Pinned by `test/trajectory-format.test.ts`.
 - `src/core/think/intent.ts` + `src/core/think/entity-extract.ts` — pure `classifyIntent(question)` returns `'temporal' | 'knowledge_update' | 'other'` (regex-first, no LLM, `'other'` fast path short-circuits with zero SQL). `extractCandidateEntities(question, retrievedSlugs)` pulls high-precision candidates from retrieved entity-prefix slugs (`people/`, `companies/`, `organizations/`) and medium-precision noun phrases. Stop-word boundaries + leading-verb stripper handle "When did I last meet Marco" → `marco`. Both consumed by `runThink` and the LongMemEval harness so the two paths cannot drift. Pinned by `test/think-intent.test.ts` and `test/think-entity-extract.test.ts`.
 - `src/commands/eval-suspected-contradictions.ts` + `src/core/eval-contradictions/{judge,runner,types,date-filter,cost-tracker,cache,severity-classify,cross-source,trends,calibration,judge-errors,auto-supersession,fixture-redact}.ts` — `gbrain eval suspected-contradictions [run|trend|review]`. Probe samples top-K retrieval pairs per query (cross-slug + intra-page chunk-vs-take), date pre-filters (3-rule layered — same-paragraph-dual-date overrides separation rule), LLM judge (query-conditioned; UTF-8-safe truncation; confidence-floor double-enforcement; resolution_kind output drives paste-ready commands), persistent cache keyed on `(chunk_a_hash, chunk_b_hash, model_id, prompt_version, truncation_policy)` (prompt edits cleanly invalidate prior verdicts), Wilson 95% CI calibration on the headline percentage with `small_sample_note` when n<30, judge_errors as first-class typed counters (parse_fail/refusal/timeout/http_5xx/unknown — avoids bias from silent skip), trend writes to `eval_contradictions_runs`, source-tier breakdown reuses `DEFAULT_SOURCE_BOOSTS` prefix logic, deterministic sampling (combined_score DESC + lex tiebreaker for stable cache hit-rate). Hermetic via `judgeFn` + `searchFn` DI in the runner; never touches the real gateway in tests. Engine surface: `BrainEngine.listActiveTakesForPages` (batched), `writeContradictionsRun` + `loadContradictionsTrend`, `getContradictionCacheEntry` + `putContradictionCacheEntry` + `sweepContradictionCache`. Schema migrations v51 + v52. MCP op `find_contradictions` (read scope, NOT localOnly, NOT in subagent allowlist — user-initiated only). Doctor check surfaces high-severity findings with paste-ready resolution commands; synthesize phase pre-fetches the latest probe's top-5-by-severity findings and threads them into `buildSynthesisPrompt` as an informational block. Architecture doc: `docs/contradictions.md`.

src/core/facts/extract.ts

@@ -308,9 +308,16 @@ export async function extractFactsFromTurnWithOutcome(
   }
 
   const parsedShape = parseExtractorJsonDetailed(result.text);
-  if (!parsedShape || parsedShape.invalidCandidates > 0) {
+  if (!parsedShape ||
+      (parsedShape.invalidCandidates > 0 && parsedShape.facts.length === 0)) {
     return { ok: false, reason: 'malformed_output' };
   }
+  if (parsedShape.invalidCandidates > 0) {
+    process.stderr.write(
+      `[facts-extract] WARN: dropped ${parsedShape.invalidCandidates} malformed candidate(s); ` +
+      `kept ${parsedShape.facts.length}\n`,
+    );
+  }
   const parsedRaw = parsedShape.facts;
 
   const facts: ExtractedFact[] = [];

test/facts-extract-salvage.test.ts

@@ -0,0 +1,75 @@
+import { afterEach, describe, expect, test } from 'bun:test';
+import {
+  __setChatTransportForTests,
+  resetGateway,
+  type ChatResult,
+} from '../src/core/ai/gateway.ts';
+import { extractFactsFromTurnWithOutcome } from '../src/core/facts/extract.ts';
+
+afterEach(() => {
+  __setChatTransportForTests(null);
+  resetGateway();
+});
+
+function stubFacts(facts: unknown[]): void {
+  __setChatTransportForTests(async (): Promise<ChatResult> => ({
+    text: JSON.stringify({ facts }),
+    blocks: [],
+    stopReason: 'end',
+    usage: {
+      input_tokens: 0,
+      output_tokens: 0,
+      cache_read_tokens: 0,
+      cache_creation_tokens: 0,
+    },
+    model: 'test:stub',
+    providerId: 'test',
+  }));
+}
+
+async function extract() {
+  return extractFactsFromTurnWithOutcome({
+    turnText: 'Conversation segment under test.',
+    source: 'test:salvage',
+  });
+}
+
+describe('facts extractor candidate salvage (#3866)', () => {
+  test('keeps valid facts when another candidate is malformed', async () => {
+    stubFacts([
+      {
+        fact: 'The migration completed',
+        kind: 'event',
+        entity: null,
+        confidence: 1.0,
+        notability: 'high',
+      },
+      {
+        fact: 'This candidate has no valid kind',
+        kind: null,
+      },
+    ]);
+
+    const outcome = await extract();
+
+    expect(outcome.ok).toBe(true);
+    if (!outcome.ok) throw new Error(outcome.reason);
+    expect(outcome.facts).toHaveLength(1);
+    expect(outcome.facts[0]!.fact).toBe('The migration completed');
+  });
+
+  test('keeps malformed_output when every candidate is invalid', async () => {
+    stubFacts([
+      { fact: 'Missing kind', kind: null },
+      { fact: null, kind: 'fact' },
+    ]);
+
+    expect(await extract()).toEqual({ ok: false, reason: 'malformed_output' });
+  });
+
+  test('keeps an explicitly empty array as a successful empty result', async () => {
+    stubFacts([]);
+
+    expect(await extract()).toEqual({ ok: true, facts: [] });
+  });
+});