Compare commits

..
Author SHA1 Message Date
Garry TanandClaude Fable 5 303bbfb0b8 fix(extract): mirror plain-bullet Format 4 in parseTimelineEntries (db-source parity)
extractTimelineFromContent (fs-source) and parseTimelineEntries
(db-source extract + put_page auto_timeline) are documented as
kept-in-sync; adding Format 4 to only the fs parser left brain-authored
plain bullets invisible on the write path and `extract --source db`.
Same skip in the citation pass so a plain bullet carrying its own
[Source: ...] files exactly one entry on the db path too.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-22 11:22:44 -07:00
9efe229eee fix(extract): recognize plain-bullet timeline format as Format 4 (takeover of #1896)
Rebase of #1896 onto master: since it was opened, #2524 landed an
inline-citation arm as Format 3 at the same insertion point, so the
plain-bullet arm now lands as Format 4 and the citation loop's skip is
extended to plain-bullet lines — a plain bullet carrying its own
[Source: ...] citation (the shape quality.md mandates) files exactly
one entry instead of double-counting.

Co-authored-by: ElliotDrel <ElliotDrel@users.noreply.github.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-21 14:20:05 -07:00
10 changed files with 98 additions and 228 deletions
+19 -2
View File
@@ -509,8 +509,12 @@ export function extractTimelineFromContent(content: string, slug: string): Extra
// DB-level uniqueness cannot collapse.
const citationPattern = /\[Source:\s*([^\]]+?),\s*(\d{4}-\d{2}-\d{2})\s*\]/g;
const bulletLinePattern = /^-\s+\*\*\d{4}-\d{2}-\d{2}\*\*\s*\|/;
// Lines captured by Format 4 (plain bullet) are skipped for the same
// reason: quality.md mandates a trailing [Source: ...] on those bullets,
// and re-extracting the citation would double-count the event.
const plainBulletLinePattern = /^-\s+\d{4}-\d{2}-\d{2}\s*[—–-]/;
for (const line of content.split(/\r?\n/)) {
if (bulletLinePattern.test(line)) continue;
if (bulletLinePattern.test(line) || plainBulletLinePattern.test(line)) continue;
const lineMatches = [...line.matchAll(citationPattern)];
if (lineMatches.length === 0) continue;
// Strip every citation marker from the line to leave the annotated text.
@@ -526,6 +530,19 @@ export function extractTimelineFromContent(content: string, slug: string): Extra
}
}
// Format 4: Plain bullet — - YYYY-MM-DD — Summary
// This is the format gbrain's own enrich skill writes (no bold, no source
// pipe). Without it, every brain-authored timeline entry is invisible to
// extraction and timeline_coverage stays at 0%. Anchored at line start so a
// date inside a summary/link cannot start a spurious entry; a bold Format-1
// line (`- **…**`) cannot match here (a `*` follows the bullet, not a
// digit), and the citation loop above skips these lines, so a plain bullet
// carrying its own [Source: ...] files exactly one entry.
const plainBulletPattern = /^-\s+(\d{4}-\d{2}-\d{2})\s*[—–-]\s*(.+)$/gm;
while ((match = plainBulletPattern.exec(content)) !== null) {
entries.push({ slug, date: match[1], source: 'markdown', summary: match[2].trim() });
}
return entries;
}
@@ -1651,7 +1668,7 @@ async function extractTimelineFromDB(
* make re-extraction idempotent). EVERY processed page is stamped, including
* zero-link pages — they WERE processed.
*/
export async function extractStaleFromDB(
async function extractStaleFromDB(
engine: BrainEngine,
opts: {
dryRun: boolean;
+1 -39
View File
@@ -1479,31 +1479,7 @@ export async function registerBuiltinHandlers(
embedSkipReason = 'auto_embed_disabled';
}
// #2849: large-sync extract deferral follow-up. performSync skips inline
// link/timeline extraction when totalChanges > 100, leaving
// links_extracted_at unstamped. A standalone sync job (webhook push,
// sync trigger) has no autopilot extract phase behind it, so the pages
// would stay extraction-stale until a manual `gbrain extract --stale`.
// Queue a source-scoped stale sweep instead. Best-effort + idempotent:
// a duplicate sweep finds 0 stale pages and no-ops.
let extractJobId: number | null = null;
if (result.extractDeferred) {
try {
const { MinionQueue } = await import('../core/minions/queue.ts');
const queue = new MinionQueue(engine);
const followUp = await queue.add(
'extract',
{ stale: true, ...(sourceId ? { sourceId } : {}) },
{
idempotency_key: `sync-extract-stale:${sourceId ?? 'default'}:${Math.floor(Date.now() / 30_000)}`,
maxWaiting: 1,
},
);
extractJobId = followUp.id;
} catch { /* best-effort: extract --stale sweeps it later */ }
}
return { ...result, embed_job_id: embedJobId, embed_skip_reason: embedSkipReason, extract_stale_job_id: extractJobId };
return { ...result, embed_job_id: embedJobId, embed_skip_reason: embedSkipReason };
});
registerBuiltinJob(worker, engine, 'embed', async (job) => {
@@ -1676,20 +1652,6 @@ export async function registerBuiltinHandlers(
});
worker.register('extract', async (job) => {
// #2849: stale-sweep mode — the sync handler's large-sync deferral
// follow-up. DB-source (reads page content from the DB, so it runs on
// checkout-less brains), source-scopable, idempotent. Same core as
// `gbrain extract --stale`.
if (job.data.stale === true) {
const { extractStaleFromDB } = await import('./extract.ts');
return await extractStaleFromDB(engine, {
dryRun: !!job.data.dryRun,
jsonMode: false,
includeFrontmatter: false,
sourceIdFilter: typeof job.data.sourceId === 'string' ? job.data.sourceId : undefined,
catchUp: false,
});
}
const { runExtractCore } = await import('./extract.ts');
const mode = (typeof job.data.mode === 'string' && ['links', 'timeline', 'all'].includes(job.data.mode))
? (job.data.mode as 'links' | 'timeline' | 'all')
+2 -8
View File
@@ -2146,13 +2146,8 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption
// Other event types (ping, pull_request, etc.) return 202 'ignored'
// so GitHub doesn't retry.
// D15.5: HMAC compare uses the shared safeHexEqual helper.
// D18: submits 'sync' job with extraction + auto_embed_backfill enabled and
// priority -10 (above autopilot's 0). noExtract:false opts normal
// incremental pushes into sync's inline link/timeline extraction (#2849
// — the standalone sync handler defaults noExtract to TRUE, which left
// webhook-imported pages permanently stale). Large (>100 file) pushes
// defer inline extract; the sync handler queues an extract --stale
// follow-up job for that branch.
// D18: submits 'sync' job with auto_embed_backfill=true and priority -10
// (above autopilot's 0).
// ---------------------------------------------------------------------------
const githubWebhookLimiter = rateLimit({
windowMs: 60_000,
@@ -2272,7 +2267,6 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption
'sync',
{
sourceId: source.id,
noExtract: false,
auto_embed_backfill: true,
embed_reason: 'webhook',
},
+1 -19
View File
@@ -222,14 +222,6 @@ export interface SyncResult {
* everything," the exact misdiagnosis in the #1794 recurrence report.
*/
bankedFiles?: number;
/**
* #2849: true when extraction was REQUESTED (noExtract false) but this sync
* skipped inline link/timeline extraction because totalChanges > 100 (the
* #1794 large-sync deferral). links_extracted_at stays unstamped for the
* imported pages. The standalone `sync` job handler queues a source-scoped
* `extract --stale` follow-up when set; CLI runs print the manual hint.
*/
extractDeferred?: boolean;
}
/**
@@ -1387,10 +1379,6 @@ See also:
{
sourceId: sourceIdArg,
repoPath: source.local_path,
// #2849: opt in to inline extraction — the standalone sync handler
// defaults noExtract to TRUE (dedupe for doctor's [sync, extract]
// remediation plan), which would leave triggered syncs extraction-stale.
noExtract: false,
auto_embed_backfill: true,
embed_reason: 'sync_trigger',
},
@@ -3299,16 +3287,11 @@ async function performSyncInner(engine: BrainEngine, opts: SyncOpts): Promise<Sy
// the stale sweep scans the whole source, so banked-across-runs pages are
// covered regardless.
const extractOpts = opts.sourceId ? { sourceId: opts.sourceId } : undefined;
let extractDeferred = false;
if (!opts.noExtract && totalChanges > 100 && pagesAffected.length > 0) {
// #2849: surface the deferral to callers. A standalone sync job (webhook
// push, sync trigger) has no autopilot extract phase behind it, so the
// job handler queues an `extract --stale` follow-up off this flag.
extractDeferred = true;
slog(
` Large sync: deferring link/timeline extraction. ` +
`Run 'gbrain extract --stale${opts.sourceId ? ` --source-id ${opts.sourceId}` : ''}' ` +
`(sync jobs queue this follow-up automatically).`,
`(or let the autopilot cycle's extract phase sweep it).`,
);
}
if (!opts.noExtract && totalChanges <= 100 && pagesAffected.length > 0) {
@@ -3417,7 +3400,6 @@ async function performSyncInner(engine: BrainEngine, opts: SyncOpts): Promise<Sy
chunksCreated,
embedded,
pagesAffected,
extractDeferred,
};
}
+8 -3
View File
@@ -1103,6 +1103,11 @@ export interface TimelineCandidate {
// Match: `- **YYYY-MM-DD** | summary` or `- **YYYY-MM-DD** -- summary`
// or `- **YYYY-MM-DD** - summary` or just `**YYYY-MM-DD** | summary`.
const TIMELINE_LINE_RE = /^\s*-?\s*\*\*(\d{4}-\d{2}-\d{2})\*\*\s*[|\-–—]+\s*(.+?)\s*$/;
// Plain bullet: `- YYYY-MM-DD — summary` (no bold, no source pipe). Kept in
// sync with extractTimelineFromContent's Format 4 (the fs-source path).
// Anchored flush-left so a date inside an indented continuation line cannot
// start a spurious entry.
const PLAIN_TIMELINE_LINE_RE = /^-\s+(\d{4}-\d{2}-\d{2})\s*[—–-]\s*(.+?)\s*$/;
/**
* Parse timeline entries from content. Looks at:
@@ -1119,7 +1124,7 @@ export function parseTimelineEntries(content: string): TimelineCandidate[] {
let i = 0;
while (i < lines.length) {
const m = TIMELINE_LINE_RE.exec(lines[i]);
const m = TIMELINE_LINE_RE.exec(lines[i]) ?? PLAIN_TIMELINE_LINE_RE.exec(lines[i]);
if (!m) {
i++;
continue;
@@ -1136,7 +1141,7 @@ export function parseTimelineEntries(content: string): TimelineCandidate[] {
let j = i + 1;
while (j < lines.length) {
const next = lines[j];
if (TIMELINE_LINE_RE.test(next)) break;
if (TIMELINE_LINE_RE.test(next) || PLAIN_TIMELINE_LINE_RE.test(next)) break;
if (/^#{1,6}\s/.test(next)) break;
if (next.trim().length === 0 && detailLines.length === 0) {
// skip leading blank line; if we hit a blank after detail content
@@ -1166,7 +1171,7 @@ export function parseTimelineEntries(content: string): TimelineCandidate[] {
// bullet pass are skipped (a bullet often carries its own citation).
const citationRe = /\[Source:\s*([^\]]+?),\s*(\d{4}-\d{2}-\d{2})\s*\]/g;
for (const line of lines) {
if (TIMELINE_LINE_RE.test(line)) continue;
if (TIMELINE_LINE_RE.test(line) || PLAIN_TIMELINE_LINE_RE.test(line)) continue;
const matches = [...line.matchAll(citationRe)];
if (matches.length === 0) continue;
const summary = line
+43
View File
@@ -185,6 +185,49 @@ describe('extractTimelineFromContent', () => {
expect(entries).toHaveLength(1);
expect(entries[0].summary).toBe('Landed the enterprise pilot with acme-example.');
});
// Format 4: the plain bullet `- YYYY-MM-DD — Summary` that gbrain's own
// enrich skill writes. Before this was supported, every brain-authored
// timeline entry was invisible and timeline_coverage was stuck at 0%.
it('extracts plain bullet format (- YYYY-MM-DD — Summary)', () => {
const content = `## Timeline\n- 2026-06-01 — Catch-up call with alice-example; intros offered.`;
const entries = extractTimelineFromContent(content, 'people/alice-example');
expect(entries).toHaveLength(1);
expect(entries[0].date).toBe('2026-06-01');
expect(entries[0].source).toBe('markdown');
expect(entries[0].summary).toBe('Catch-up call with alice-example; intros offered.');
});
it('files exactly one entry for a plain bullet that carries its own citation', () => {
// quality.md mandates this shape; it must not double-count via the
// citation arm (Format 3) AND the plain-bullet arm (Format 4).
const content = `- 2026-05-31 — Confirmed full name. [Source: User, 2026-05-31]`;
const entries = extractTimelineFromContent(content, 'people/charlie-example');
expect(entries).toHaveLength(1);
expect(entries[0].date).toBe('2026-05-31');
expect(entries[0].source).toBe('markdown');
expect(entries[0].summary).toContain('Confirmed full name.');
});
it('does not double-count bold (Format 1) lines as plain bullets', () => {
const content = `- **2025-03-18** | Meeting — Discussed partnership`;
const entries = extractTimelineFromContent(content, 'test');
expect(entries).toHaveLength(1);
expect(entries[0].source).toBe('Meeting');
});
it('does not start a spurious entry from a date inside a summary', () => {
const content = `- 2026-06-01 — See [meeting](meetings/2026-06-01).`;
const entries = extractTimelineFromContent(content, 'people/alice-example');
expect(entries).toHaveLength(1);
expect(entries[0].date).toBe('2026-06-01');
});
it('handles en dash and hyphen separators in plain bullets', () => {
const content = `- 2026-06-01 First\n- 2026-06-02 - Second`;
const entries = extractTimelineFromContent(content, 'test');
expect(entries).toHaveLength(2);
});
});
describe('walkMarkdownFiles', () => {
+24
View File
@@ -720,6 +720,30 @@ More prose here.
const entries = parseTimelineEntries(content);
expect(entries.length).toBe(2);
});
// Plain bullet (extract.ts Format 4 parity — the db-source path must see
// the same entries as the fs-source path).
test('parses plain bullet: - YYYY-MM-DD — summary', () => {
const entries = parseTimelineEntries('## Timeline\n- 2026-06-01 — Catch-up call with alice-example');
expect(entries.length).toBe(1);
expect(entries[0].date).toBe('2026-06-01');
expect(entries[0].summary).toBe('Catch-up call with alice-example');
});
test('files exactly one entry for a plain bullet carrying its own citation', () => {
const entries = parseTimelineEntries('- 2026-05-31 — Confirmed full name. [Source: User, 2026-05-31]');
expect(entries.length).toBe(1);
expect(entries[0].date).toBe('2026-05-31');
});
test('skips invalid dates in plain bullets', () => {
expect(parseTimelineEntries('- 2026-13-45 — Bad date').length).toBe(0);
});
test('does not double-count a bold bullet as a plain bullet', () => {
const entries = parseTimelineEntries('- **2026-01-15** | Met with Alice');
expect(entries.length).toBe(1);
});
});
// ─── isAutoLinkEnabled ─────────────────────────────────────────
-23
View File
@@ -16,7 +16,6 @@
*/
import { describe, test, expect } from 'bun:test';
import { createHmac } from 'node:crypto';
import { readFileSync } from 'node:fs';
import { safeHexEqual } from '../src/core/timing-safe.ts';
const GITHUB_SECRET = 'super-secret-webhook-key';
@@ -124,25 +123,3 @@ describe('Branch ref construction (D5)', () => {
expect(pushedRef === `refs/heads/${trackedBranch}`).toBe(false);
});
});
describe('Webhook sync job extraction contract (#2849)', () => {
test('opts into extraction before the pushed commit is consumed', () => {
const serveSource = readFileSync(
new URL('../src/commands/serve-http.ts', import.meta.url),
'utf8',
);
const routeStart = serveSource.indexOf("'/webhooks/github'");
const queueStart = serveSource.indexOf('const job = await queue.add(', routeStart);
const responseStart = serveSource.indexOf('res.status(202)', queueStart);
expect(routeStart).toBeGreaterThanOrEqual(0);
expect(queueStart).toBeGreaterThan(routeStart);
expect(responseStart).toBeGreaterThan(queueStart);
const routeSource = serveSource.slice(queueStart, responseStart);
const payload = routeSource.match(
/queue\.add\(\s*'sync',\s*\{([\s\S]*?)\}\s*,\s*\{/,
);
expect(payload).not.toBeNull();
expect(payload?.[1]).toMatch(/\bnoExtract:\s*false\b/);
});
});
@@ -1,130 +0,0 @@
/**
* #2849 large-sync extract deferral queues an `extract --stale` follow-up.
*
* performSync's incremental path skips inline link/timeline extraction when
* totalChanges > 100 (the #1794 large-sync deferral), leaving
* links_extracted_at unstamped. Pre-fix, a standalone sync job (webhook push,
* `gbrain sync trigger`) had NOTHING behind it to sweep those pages the
* autopilot cycle's extract phase only walks that cycle's changedSlugs so a
* large webhook push left extraction permanently stale until a manual
* `gbrain extract --stale`.
*
* Pins:
* (a) performSync surfaces `extractDeferred: true` on the >100 branch and
* leaves the pages unstamped/unlinked.
* (b) the `sync` job handler queues an `extract` job with
* { stale: true, sourceId? } when extractDeferred is set.
* (c) the `extract` handler's stale mode actually sweeps: links created +
* watermark stamped (end-to-end recovery, no manual step).
*
* Marked .serial.test.ts spawns git subprocesses + shares one PGLite engine.
*/
import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
import { mkdtempSync, writeFileSync, rmSync, mkdirSync } from 'fs';
import { execSync } from 'child_process';
import { tmpdir } from 'os';
import { join } from 'path';
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
import { MinionWorker } from '../src/core/minions/worker.ts';
import { MinionQueue } from '../src/core/minions/queue.ts';
import { registerBuiltinHandlers } from '../src/commands/jobs.ts';
let engine: PGLiteEngine;
let worker: MinionWorker;
let repoPath: string;
function git(cmd: string): void { execSync(cmd, { cwd: repoPath, stdio: 'pipe' }); }
describe('#2849 — large sync defers extract and queues a stale sweep', () => {
beforeAll(async () => {
engine = new PGLiteEngine();
await engine.connect({});
await engine.initSchema();
worker = new MinionWorker(engine, { queue: 'test' });
await registerBuiltinHandlers(worker, engine, { quiet: true });
repoPath = mkdtempSync(join(tmpdir(), 'gbrain-large-defer-'));
git('git init');
git('git config user.email "t@t.com"');
git('git config user.name "T"');
mkdirSync(join(repoPath, 'people'), { recursive: true });
mkdirSync(join(repoPath, 'notes'), { recursive: true });
writeFileSync(join(repoPath, 'people/alice.md'), [
'---', 'type: person', 'title: Alice', '---', '', 'Alice is a founder.',
].join('\n'));
git('git add -A && git commit -m "initial"');
// Seed: full first sync imports the anchor page + sets last_commit.
const { performSync } = await import('../src/commands/sync.ts');
await performSync(engine, { repoPath, full: true, noPull: true, noEmbed: true });
// Second commit: 101 new pages → incremental totalChanges > 100.
for (let i = 0; i < 101; i++) {
writeFileSync(join(repoPath, `notes/n${i}.md`), [
'---', 'type: note', `title: Note ${i}`, '---', '',
`[Alice](people/alice) appears in note ${i}.`,
].join('\n'));
}
git('git add -A && git commit -m "add 101 pages"');
}, 120_000);
afterAll(async () => {
if (repoPath) rmSync(repoPath, { recursive: true, force: true });
if (engine) await engine.disconnect();
}, 60_000);
test('sync handler defers inline extract and queues extract{stale} follow-up; stale sweep recovers', async () => {
const syncHandler = (worker as unknown as { handlers: Map<string, (job: unknown) => Promise<unknown>> })
.handlers.get('sync');
expect(syncHandler).toBeDefined();
// Same payload shape the webhook submits (minus embed backfill noise).
const result = await syncHandler!({
data: { repoPath, noExtract: false, noPull: true, auto_embed_backfill: false },
signal: { aborted: false },
updateProgress: async () => {},
}) as { status: string; extractDeferred?: boolean; extract_stale_job_id?: number | null };
expect(result.status).toBe('synced');
// (a) inline extract was deferred, pages left stale.
expect(result.extractDeferred).toBe(true);
const staleBefore = await engine.countStalePagesForExtraction();
expect(staleBefore).toBeGreaterThan(100);
expect(await engine.getLinks('notes/n0')).toHaveLength(0);
// (b) a follow-up extract job with stale:true was queued.
expect(result.extract_stale_job_id).toBeGreaterThan(0);
const queue = new MinionQueue(engine);
const extractJobs = await queue.getJobs({ name: 'extract', limit: 5 });
expect(extractJobs.length).toBe(1);
expect((extractJobs[0].data as { stale: boolean }).stale).toBe(true);
// (c) running the extract handler's stale mode recovers: links + stamps.
const extractHandler = (worker as unknown as { handlers: Map<string, (job: unknown) => Promise<unknown>> })
.handlers.get('extract');
await extractHandler!({
data: extractJobs[0].data,
signal: { aborted: false },
updateProgress: async () => {},
});
const links = await engine.getLinks('notes/n0');
expect(links.some(l => l.to_slug === 'people/alice')).toBe(true);
const rows = await engine.executeRaw<{ links_extracted_at: string | null }>(
`SELECT links_extracted_at FROM pages WHERE slug = 'notes/n0'`,
);
expect(rows[0]?.links_extracted_at).not.toBeNull();
}, 180_000);
test('sub-threshold sync does NOT set extractDeferred (no spurious follow-up)', async () => {
// One more small commit → inline extract path, no deferral.
writeFileSync(join(repoPath, 'notes/small.md'), [
'---', 'type: note', 'title: Small', '---', '', 'No big deal.',
].join('\n'));
git('git add -A && git commit -m "one small page"');
const { performSync } = await import('../src/commands/sync.ts');
const result = await performSync(engine, { repoPath, noPull: true, noEmbed: true });
expect(result.status).toBe('synced');
expect(result.extractDeferred).toBeFalsy();
}, 60_000);
});
-4
View File
@@ -100,10 +100,6 @@ describe('runSyncTrigger', () => {
const job = jobs[0];
expect(job.priority).toBe(-10);
expect((job.data as { sourceId: string }).sourceId).toBe('default');
// #2849: opt in to inline extraction — the standalone sync handler
// defaults noExtract to TRUE, which would leave triggered syncs
// extraction-stale.
expect((job.data as { noExtract: boolean }).noExtract).toBe(false);
expect((job.data as { auto_embed_backfill: boolean }).auto_embed_backfill).toBe(true);
});