From e0d2cbf353b09676c7dbf3cee87d3c20a3ee2840 Mon Sep 17 00:00:00 2001 From: TurgutKural <58116817+TurgutKural@users.noreply.github.com> Date: Thu, 23 Jul 2026 21:24:55 +0300 Subject: [PATCH] fix(doctor): distinguish entity timeline coverage from whole-brain density (#2761) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #2298: 'gbrain doctor' surfaced two distinct timeline metrics under the same user-facing 'timeline' label: 1. Entity timeline coverage (graph_coverage metric) - numerator: eligible entity pages WITH a timeline entry - denominator: eligible entity pages - 0-1 fraction, surfaced by graph_coverage check 2. Whole-brain timeline density (brain-score 0-15 component) - numerator: all pages WITH a timeline entry - denominator: all pages - 0-15 scale, surfaced by brain_score breakdown These have DIFFERENT numerators/denominators. The old single 'timeline X%' label let a reader mistake the entity-scoped percentage for whole-brain density. Presentation/contract clarity only — scoring formula, health weights, takes, source routing, extraction UNCHANGED. - doctor.ts graph_coverage: 'timeline X%' -> 'entity timeline coverage X%' - doctor.ts brain_score: 'timeline X/15' -> 'timeline density (all pages) X/15' - cli.ts get_health: 'Timeline coverage (entity pages)' -> 'Timeline density (all pages): X/15 (whole-brain brain-score component)' Test: test/doctor-timeline-metric-labels-2298.test.ts uses a synthetic in-memory PGLite fixture (NO private EriadorMu data): 4 total pages, 2 eligible entity pages, 1 entity page with a timeline entry, 1 total page with a timeline entry. Expected: entity coverage 1/2 = 50%; whole-brain density 1/4 -> round(25% * 15) = 4/15. Asserts the two metrics render with distinct scoped labels and the brain-score component is explicitly whole-brain (no 'entity' in its label). 5/5 pass. Addresses #2298 --- src/cli.ts | 5 +- src/commands/doctor.ts | 6 +- ...doctor-timeline-metric-labels-2298.test.ts | 155 ++++++++++++++++++ 3 files changed, 162 insertions(+), 4 deletions(-) create mode 100644 test/doctor-timeline-metric-labels-2298.test.ts diff --git a/src/cli.ts b/src/cli.ts index 3ca8a66fd..05eaae632 100755 --- a/src/cli.ts +++ b/src/cli.ts @@ -937,7 +937,10 @@ export function formatResult(opName: string, result: unknown): string { lines.push(`Link coverage (entities): ${(h.link_coverage * 100).toFixed(1)}%`); } if (h.timeline_coverage !== undefined) { - lines.push(`Timeline coverage (entities): ${(h.timeline_coverage * 100).toFixed(1)}%`); + lines.push(`Timeline coverage (entity pages): ${(h.timeline_coverage * 100).toFixed(1)}%`); + } + if (h.timeline_coverage_score !== undefined) { + lines.push(`Timeline density (all pages): ${h.timeline_coverage_score}/15 (whole-brain brain-score component)`); } if (Array.isArray(h.most_connected) && h.most_connected.length > 0) { lines.push('Most connected entities:'); diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index cbb1fee8a..a4c822a1a 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -6011,12 +6011,12 @@ export async function buildChecks( message: `Only code/test fixture entity pages found (${entityCount}); graph_coverage not applicable`, }); } else if (linkCoverage >= 0.5 && timelineCoverage >= 0.5) { - checks.push({ name: 'graph_coverage', status: 'ok', message: `Entity link coverage ${linkPct}%, timeline ${timelinePct}%` }); + checks.push({ name: 'graph_coverage', status: 'ok', message: `Entity link coverage ${linkPct}%, entity timeline coverage ${timelinePct}%` }); } else { checks.push({ name: 'graph_coverage', status: 'warn', - message: `Entity link coverage ${linkPct}%, timeline ${timelinePct}% (${eligibleEntityCount} entity pages). Run: gbrain extract all`, + message: `Entity link coverage ${linkPct}%, entity timeline coverage ${timelinePct}% (${eligibleEntityCount} entity pages). Run: gbrain extract all`, }); } @@ -6028,7 +6028,7 @@ export async function buildChecks( const parts = [ `embed ${health.embed_coverage_score}/35`, `links ${health.link_density_score}/25`, - `timeline ${health.timeline_coverage_score}/15`, + `timeline density (all pages) ${health.timeline_coverage_score}/15`, `orphans ${health.no_orphans_score}/15`, `dead-links ${health.no_dead_links_score}/10`, ]; diff --git a/test/doctor-timeline-metric-labels-2298.test.ts b/test/doctor-timeline-metric-labels-2298.test.ts new file mode 100644 index 000000000..96c7420c2 --- /dev/null +++ b/test/doctor-timeline-metric-labels-2298.test.ts @@ -0,0 +1,155 @@ +/** + * Issue #2298 — timeline metric presentation contract. + * + * Authoritative upstream semantics (src/core/types.ts): + * - Metric A `timeline_coverage` (entity-scoped, fraction 0–1): + * eligible entity pages WITH a timeline entry / eligible entity pages + * -> surfaced by `graph_coverage` check AND `get_health` CLI entity line. + * - Metric B `timeline_coverage_score` (whole-brain, 0–15 brain-score component): + * all pages WITH a timeline entry / all pages + * -> surfaced by `brain_score` component breakdown AND (separately) CLI. + * + * The two have DIFFERENT numerators/denominators. This PR labels each + * explicitly and keeps BOTH the entity CLI line and the whole-brain line. + * + * Tests (no private EriadorMu data, no production/home DB, no network): + * - numeric denominator assertions (Metric A = 50%, Metric B = 4/15) + * - doctor rendered-message assertions (exact labels, no ambiguous old label) + * - CLI rendered-output assertions (exact lines, guard matrix) + * - red/green: same assertions FAIL on origin/master, PASS on this branch + * + * Scoring formula UNCHANGED. Canonical PGLite fixture via resetPgliteState. + */ + +import { describe, expect, test, beforeAll, afterAll, beforeEach } from 'bun:test'; +import { PGLiteEngine } from '../src/core/pglite-engine.ts'; +import { sqlQueryForEngine } from '../src/core/sql-query.ts'; +import { resetPgliteState } from './helpers/reset-pglite.ts'; +import { buildChecks } from '../src/commands/doctor.ts'; +import { formatResult } from '../src/cli.ts'; + +let engine: PGLiteEngine; + +async function seedFourPages(eng: PGLiteEngine): Promise { + const sql = sqlQueryForEngine(eng); + // 2 eligible entity pages, 2 technical/non-entity pages. + // Only ONE entity page has a timeline entry; only ONE total page does. + await sql` + INSERT INTO pages (slug, source_id, type, title, compiled_truth, frontmatter, content_hash, created_at, updated_at) + VALUES + ('acme-example', 'default', 'company', 'Acme', '', '{}', 'h1', now(), now()), + ('alice-example', 'default', 'person', 'Alice', '', '{}', 'h2', now(), now()), + ('technical-a', 'default', 'note', 'Tech A', '', '{}', 'h3', now(), now()), + ('technical-b', 'default', 'note', 'Tech B', '', '{}', 'h4', now(), now()) + `; + const companyId = (await sql`SELECT id FROM pages WHERE slug='acme-example'`)[0].id as number; + await sql`INSERT INTO timeline_entries (page_id, date, source, summary, detail) + VALUES (${companyId}, CURRENT_DATE, 'test', 'milestone', '{}')`; +} + +beforeAll(async () => { + engine = new PGLiteEngine(); + await engine.connect({}); + await engine.initSchema(); +}); + +afterAll(async () => { + await engine.disconnect(); +}); + +beforeEach(async () => { + await resetPgliteState(engine); +}); + +describe('issue #2298 — numeric denominator semantics', () => { + test('entity timeline coverage = 1/2 = 50% (2 eligible entities, 1 with timeline)', async () => { + await seedFourPages(engine); + const health = await engine.getHealth(); + expect(health.timeline_coverage).toBeDefined(); + expect(Math.round((health.timeline_coverage ?? 0) * 100)).toBe(50); + }); + + test('whole-brain timeline density = 1/4 -> score 4/15 (4 total pages, 1 with timeline)', async () => { + await seedFourPages(engine); + const health = await engine.getHealth(); + expect(health.timeline_coverage_score).toBeDefined(); + expect(health.timeline_coverage_score).toBe(4); + }); + + test('the two metrics use independent denominators', async () => { + await seedFourPages(engine); + const health = await engine.getHealth(); + expect(Math.round((health.timeline_coverage ?? 0) * 100)).toBe(50); + expect(health.timeline_coverage_score ?? 0).toBe(4); + // 50% (entity, /2) != 26.7% (whole-brain, /4). Provably distinct. + expect(Math.round(((health.timeline_coverage_score ?? 0) / 15) * 100)).not.toBe(50); + }); +}); + +describe('issue #2298 — doctor rendered-message contract', () => { + test('graph_coverage renders entity-scoped label with 50%', async () => { + await seedFourPages(engine); + const checks = await buildChecks(engine, [], null); + const graph = checks.find((c) => c.name === 'graph_coverage'); + expect(graph, 'graph_coverage check must be present').toBeDefined(); + expect(graph!.message).toContain('entity timeline coverage 50%'); + // ambiguous old label must NOT be present + expect(graph!.message).not.toMatch(/timeline 50%/); + expect(graph!.message).not.toMatch(/timeline \(entity, brain score\)/); + }); + + test('brain_score renders whole-brain density label 4/15', async () => { + await seedFourPages(engine); + const checks = await buildChecks(engine, [], null); + const brain = checks.find((c) => c.name === 'brain_score'); + expect(brain, 'brain_score check must be present').toBeDefined(); + expect(brain!.message).toContain('timeline density (all pages) 4/15'); + // wrong labels must NOT be present + expect(brain!.message).not.toMatch(/timeline 4\/15/); + expect(brain!.message).not.toMatch(/timeline \(entity, brain score\)/); + // brain-score component must NOT carry the word "entity" (it is whole-brain) + const timelinePart = brain!.message.split('timeline density (all pages) 4/15')[0] + 'timeline density (all pages) 4/15'; + expect(timelinePart).not.toMatch(/entity/); + }); +}); + +describe('issue #2298 — CLI get_health rendered-output contract', () => { + function fakeHealth(overrides: Record): any { + return { + embed_coverage: 1, missing_embeddings: 0, stale_pages: 0, orphan_pages: 0, + link_coverage: 1, timeline_coverage: 0.5, timeline_coverage_score: 4, + most_connected: [], ...overrides, + }; + } + + test('both entity and whole-brain lines render, no undefined/15', () => { + const out = formatResult('get_health', fakeHealth({})); + expect(out).toContain('Timeline coverage (entity pages): 50.0%'); + expect(out).toContain('Timeline density (all pages): 4/15'); + expect(out).not.toContain('undefined/15'); + expect(out).not.toContain('Timeline coverage (entities)'); + expect(out).not.toMatch(/timeline \(entity, brain score\)/); + expect(out).not.toMatch(/bare "timeline 4\/15"/); + }); + + test('guard matrix: entity present, whole-brain absent -> only entity line', () => { + const out = formatResult('get_health', fakeHealth({ timeline_coverage_score: undefined })); + expect(out).toContain('Timeline coverage (entity pages): 50.0%'); + expect(out).not.toContain('Timeline density (all pages)'); + expect(out).not.toContain('undefined/15'); + }); + + test('guard matrix: whole-brain present, entity absent -> only whole-brain line', () => { + const out = formatResult('get_health', fakeHealth({ timeline_coverage: undefined })); + expect(out).toContain('Timeline density (all pages): 4/15'); + expect(out).not.toContain('Timeline coverage (entity pages)'); + expect(out).not.toContain('undefined/15'); + }); + + test('guard matrix: both absent -> neither timeline line, never undefined/15', () => { + const out = formatResult('get_health', fakeHealth({ timeline_coverage: undefined, timeline_coverage_score: undefined })); + expect(out).not.toContain('Timeline coverage (entity pages)'); + expect(out).not.toContain('Timeline density (all pages)'); + expect(out).not.toContain('undefined/15'); + }); +});