mirror of
https://github.com/garrytan/gbrain.git
synced 2026-08-14 08:53:22 +00:00
fix(facts): preserve remote fence writes (#3659)
Co-authored-by: gbrain contributor <contributor@example.com> Co-authored-by: Time Attakc <89218912+time-attack@users.noreply.github.com>
This commit is contained in:
co-authored by
gbrain contributor
Time Attakc
parent
4c0ec60275
commit
23003a2163
@@ -39,6 +39,7 @@ import { normalizeAliasList } from './search/alias-normalize.ts';
|
||||
import { isUndefinedTableError, warnOncePerProcess, validateSlug } from './utils.ts';
|
||||
import { computeCorpusGeneration } from './contextual-retrieval-service.ts';
|
||||
import { runGuardrails } from './guardrails.ts';
|
||||
import { FACTS_FENCE_BEGIN, FACTS_FENCE_END, parseFactsFence } from './facts-fence.ts';
|
||||
|
||||
/**
|
||||
* v0.20.0 Cathedral II Layer 8 D2 — markdown fence extraction helper.
|
||||
@@ -104,6 +105,27 @@ function fenceTagToPseudoPath(lang: string | undefined): string | null {
|
||||
*/
|
||||
const MAX_FENCES_PER_PAGE = Number.parseInt(process.env.GBRAIN_MAX_FENCES_PER_PAGE || '100', 10);
|
||||
|
||||
function extractFactsFenceBlock(body: string): string | null {
|
||||
const beginIdx = body.indexOf(FACTS_FENCE_BEGIN);
|
||||
if (beginIdx === -1) return null;
|
||||
const endIdx = body.indexOf(FACTS_FENCE_END, beginIdx + FACTS_FENCE_BEGIN.length);
|
||||
if (endIdx === -1) return null;
|
||||
return body.slice(beginIdx, endIdx + FACTS_FENCE_END.length);
|
||||
}
|
||||
|
||||
function replaceOrAppendFactsFence(body: string, fenceBlock: string): string {
|
||||
const beginIdx = body.indexOf(FACTS_FENCE_BEGIN);
|
||||
if (beginIdx !== -1) {
|
||||
const endIdx = body.indexOf(FACTS_FENCE_END, beginIdx + FACTS_FENCE_BEGIN.length);
|
||||
if (endIdx !== -1) {
|
||||
return body.slice(0, beginIdx) + fenceBlock + body.slice(endIdx + FACTS_FENCE_END.length);
|
||||
}
|
||||
}
|
||||
|
||||
const sep = body.endsWith('\n') ? '\n' : '\n\n';
|
||||
return `${body}${sep}## Facts\n\n${fenceBlock}\n`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Walk the marked lexer output and extract recognizable code fences.
|
||||
* Returns one ChunkInput per fence whose language tag maps to a grammar
|
||||
@@ -548,6 +570,26 @@ export async function importFromContent(
|
||||
// hash-match skip) and (b) the hash short-circuit below reuses this row.
|
||||
const existing = await engine.getPage(slug, sourceId ? { sourceId } : undefined);
|
||||
|
||||
// #2044: remote get_page intentionally strips private facts rows. A
|
||||
// documented get_page -> edit -> put_page round-trip can therefore arrive
|
||||
// with an empty/missing Facts fence even though the existing page still has
|
||||
// canonical fence rows. Preserve the old fence in that narrow case so the
|
||||
// system-of-record markdown is not truncated by the privacy boundary.
|
||||
if (opts.remote === true && existing?.compiled_truth) {
|
||||
const incomingFacts = parseFactsFence(parsed.compiled_truth);
|
||||
const existingFacts = parseFactsFence(existing.compiled_truth);
|
||||
const existingFenceBlock = extractFactsFenceBlock(existing.compiled_truth);
|
||||
if (
|
||||
incomingFacts.facts.length === 0 &&
|
||||
incomingFacts.warnings.length === 0 &&
|
||||
existingFacts.warnings.length === 0 &&
|
||||
existingFacts.facts.length > 0 &&
|
||||
existingFenceBlock
|
||||
) {
|
||||
parsed.compiled_truth = replaceOrAppendFactsFence(parsed.compiled_truth, existingFenceBlock);
|
||||
}
|
||||
}
|
||||
|
||||
// #1035: absence of an explicit frontmatter `type:` on an EXISTING page
|
||||
// means "preserve the stored type", not "re-infer". Pre-fix, a round-trip
|
||||
// put (get_page → edit body → put_page without `type:`) silently regressed
|
||||
|
||||
@@ -4309,7 +4309,11 @@ export class PGLiteEngine implements BrainEngine {
|
||||
$14, $15,
|
||||
$16, $17, $18, $19,
|
||||
$20
|
||||
) RETURNING id`
|
||||
)
|
||||
ON CONFLICT (source_id, source_markdown_slug, row_num)
|
||||
WHERE row_num IS NOT NULL
|
||||
DO NOTHING
|
||||
RETURNING id`
|
||||
: `INSERT INTO facts (
|
||||
source_id, entity_slug, fact, kind, visibility, notability, context,
|
||||
valid_from, valid_until, source, source_session, confidence,
|
||||
@@ -4323,12 +4327,16 @@ export class PGLiteEngine implements BrainEngine {
|
||||
$15, $16,
|
||||
$17, $18, $19, $20,
|
||||
$21
|
||||
) RETURNING id`,
|
||||
)
|
||||
ON CONFLICT (source_id, source_markdown_slug, row_num)
|
||||
WHERE row_num IS NOT NULL
|
||||
DO NOTHING
|
||||
RETURNING id`,
|
||||
embedStr === null
|
||||
? [ctx.source_id, entitySlug, input.fact, kind, visibility, notability, context, validFrom, validUntil, input.source, sourceSession, confidence, embeddedAt, input.row_num, input.source_markdown_slug, claimMetric, claimValue, claimUnit, claimPeriod, eventType]
|
||||
: [ctx.source_id, entitySlug, input.fact, kind, visibility, notability, context, validFrom, validUntil, input.source, sourceSession, confidence, embedStr, embeddedAt, input.row_num, input.source_markdown_slug, claimMetric, claimValue, claimUnit, claimPeriod, eventType],
|
||||
);
|
||||
out.push(ins.rows[0].id);
|
||||
if (ins.rows[0]) out.push(ins.rows[0].id);
|
||||
}
|
||||
return out;
|
||||
});
|
||||
|
||||
@@ -4491,9 +4491,13 @@ export class PostgresEngine implements BrainEngine {
|
||||
${input.row_num}, ${input.source_markdown_slug},
|
||||
${claimMetric}, ${claimValue}, ${claimUnit}, ${claimPeriod},
|
||||
${eventType}
|
||||
) RETURNING id
|
||||
)
|
||||
ON CONFLICT (source_id, source_markdown_slug, row_num)
|
||||
WHERE row_num IS NOT NULL
|
||||
DO NOTHING
|
||||
RETURNING id
|
||||
`;
|
||||
out.push(Number(ins[0].id));
|
||||
if (ins[0]) out.push(Number(ins[0].id));
|
||||
}
|
||||
return out;
|
||||
});
|
||||
|
||||
@@ -38,11 +38,11 @@ import { tmpdir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
|
||||
import { PGLiteEngine } from '../../src/core/pglite-engine.ts';
|
||||
import { importFromFile } from '../../src/core/import-file.ts';
|
||||
import { importFromContent, importFromFile } from '../../src/core/import-file.ts';
|
||||
import { runExtractCore } from '../../src/commands/extract.ts';
|
||||
import { extractTakes } from '../../src/core/cycle/extract-takes.ts';
|
||||
import { runExtractFacts } from '../../src/core/cycle/extract-facts.ts';
|
||||
import { stripFactsFence } from '../../src/core/facts-fence.ts';
|
||||
import { parseFactsFence, stripFactsFence } from '../../src/core/facts-fence.ts';
|
||||
|
||||
let engine: PGLiteEngine;
|
||||
let brainDir: string;
|
||||
@@ -305,6 +305,46 @@ describe('get_page privacy strip via stripFactsFence({keepVisibility:["world"]})
|
||||
expect(remoteBody).not.toContain('PRIVATE_DETAIL_PROOF'); // remote MCP strips
|
||||
expect(remoteBody).toContain('Founded Acme in 2017'); // world fact retained
|
||||
});
|
||||
|
||||
test('remote put_page round-trip preserves an existing private-only facts fence', async () => {
|
||||
const slug = 'people/private-only-facts';
|
||||
await importFromContent(engine, slug, `---
|
||||
type: person
|
||||
title: Private Only Facts
|
||||
slug: ${slug}
|
||||
---
|
||||
|
||||
# Private Only Facts
|
||||
|
||||
## Facts
|
||||
|
||||
<!--- gbrain:facts:begin -->
|
||||
| # | claim | kind | confidence | visibility | notability | valid_from | valid_until | source | context |
|
||||
|---|-------|------|------------|------------|------------|------------|-------------|--------|---------|
|
||||
| 1 | PRIVATE_ONLY_FACT | preference | 0.9 | private | medium | 2026-07-30 | | meeting | |
|
||||
<!--- gbrain:facts:end -->
|
||||
`, { noEmbed: true, sourceId: 'default' });
|
||||
|
||||
const trusted = await engine.getPage(slug, { sourceId: 'default' });
|
||||
expect(trusted).not.toBeNull();
|
||||
if (!trusted) return;
|
||||
|
||||
const remoteBody = stripFactsFence(trusted.compiled_truth ?? '', { keepVisibility: ['world'] });
|
||||
expect(remoteBody).toContain('gbrain:facts:begin');
|
||||
expect(remoteBody).not.toContain('PRIVATE_ONLY_FACT');
|
||||
|
||||
await importFromContent(engine, slug, `---
|
||||
type: person
|
||||
title: Private Only Facts
|
||||
slug: ${slug}
|
||||
---
|
||||
|
||||
${remoteBody}`, { noEmbed: true, sourceId: 'default', remote: true });
|
||||
|
||||
const after = await engine.getPage(slug, { sourceId: 'default' });
|
||||
expect(after?.compiled_truth).toContain('PRIVATE_ONLY_FACT');
|
||||
expect(parseFactsFence(after?.compiled_truth ?? '').facts).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
* - Batch insert N rows persists row_num + source_markdown_slug
|
||||
* - Empty batch is a no-op
|
||||
* - Returns ids in input-order
|
||||
* - v51 partial UNIQUE index rolls back the whole batch on a collision
|
||||
* - v51 partial UNIQUE index collisions are idempotently skipped
|
||||
* - deleteFactsForPage scopes by (source_id, source_markdown_slug);
|
||||
* never touches other pages or pre-v51 NULL-source_markdown_slug rows
|
||||
* - deleteFactsForPage on an empty page returns deleted:0 (idempotent)
|
||||
@@ -135,30 +135,26 @@ describe('engine.insertFacts — batch insert', () => {
|
||||
});
|
||||
});
|
||||
|
||||
test('v51 partial UNIQUE index rolls back the whole batch on collision', async () => {
|
||||
test('v51 partial UNIQUE index collision skips duplicate rows without rolling back the batch', async () => {
|
||||
// Seed row #1 first.
|
||||
await engine.insertFacts([fixtureFact(1, { fact: 'seeded' })], { source_id: 'default' });
|
||||
|
||||
// Now try to batch-insert rows that include a colliding row_num=1.
|
||||
let threw = false;
|
||||
try {
|
||||
await engine.insertFacts(
|
||||
[
|
||||
fixtureFact(2, { fact: 'second' }),
|
||||
fixtureFact(1, { fact: 'collides' }), // row_num=1 on same (source_id, source_markdown_slug)
|
||||
fixtureFact(3, { fact: 'third' }),
|
||||
],
|
||||
{ source_id: 'default' },
|
||||
);
|
||||
} catch {
|
||||
threw = true;
|
||||
}
|
||||
expect(threw).toBe(true);
|
||||
// Now try to batch-insert rows that include a colliding row_num=1. The
|
||||
// duplicate is skipped, but the non-conflicting rows still land.
|
||||
const r = await engine.insertFacts(
|
||||
[
|
||||
fixtureFact(2, { fact: 'second' }),
|
||||
fixtureFact(1, { fact: 'collides' }), // row_num=1 on same (source_id, source_markdown_slug)
|
||||
fixtureFact(3, { fact: 'third' }),
|
||||
],
|
||||
{ source_id: 'default' },
|
||||
);
|
||||
expect(r.inserted).toBe(2);
|
||||
expect(r.ids).toHaveLength(2);
|
||||
|
||||
// Verify the transaction rolled back — only the seeded row should remain.
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
const rows = await (engine as any).db.query('SELECT fact FROM facts ORDER BY id');
|
||||
expect(rows.rows.map((r: { fact: string }) => r.fact)).toEqual(['seeded']);
|
||||
const rows = await (engine as any).db.query('SELECT fact, row_num FROM facts ORDER BY row_num');
|
||||
expect(rows.rows.map((row: { fact: string }) => row.fact)).toEqual(['seeded', 'second', 'third']);
|
||||
});
|
||||
|
||||
test('different source_markdown_slug values DO NOT collide on the same row_num', async () => {
|
||||
|
||||
Reference in New Issue
Block a user