From 77223ad772244c09d4a5b701b3b7fbc4a9a2a216 Mon Sep 17 00:00:00 2001 From: Luca Martial Date: Mon, 11 May 2026 14:45:08 -0700 Subject: [PATCH] WIP: save local changes before main merge --- .../context/skills/notion_synthesize/SKILL.md | 1 + .../src/ingest/adapters/notion/chunk.ts | 2 +- .../src/ingest/adapters/notion/cluster.ts | 2 +- .../adapters/notion/notion.adapter.test.ts | 1 + .../src/ingest/local-bundle-ingest.test.ts | 4 +- .../src/ingest/local-bundle-runtime.ts | 20 ++++++++-- .../tools/tool-transcript-summary.test.ts | 17 ++++++++ .../ingest/tools/tool-transcript-summary.ts | 4 +- .../src/wiki/tools/wiki-read.tool.test.ts | 40 +++++++++++++++++++ .../context/src/wiki/tools/wiki-read.tool.ts | 3 +- .../src/wiki/tools/wiki-remove.tool.test.ts | 30 ++++++++++++++ .../src/wiki/tools/wiki-remove.tool.ts | 4 +- 12 files changed, 116 insertions(+), 12 deletions(-) create mode 100644 packages/context/src/wiki/tools/wiki-read.tool.test.ts diff --git a/packages/context/skills/notion_synthesize/SKILL.md b/packages/context/skills/notion_synthesize/SKILL.md index fc085e95..8c7e9b23 100644 --- a/packages/context/skills/notion_synthesize/SKILL.md +++ b/packages/context/skills/notion_synthesize/SKILL.md @@ -62,6 +62,7 @@ If a clustered WorkUnit includes several related pages, synthesize the shared ru - Prefer overlays on manifest-backed sources over standalone SQL. - If Notion describes a dashboard or metric but does not define executable logic, write a wiki page and attach `sl_refs` only after confirming the referenced source exists. - Do not create SL sources under the Notion connection just because a page mentions a warehouse, dbt, Looker, or Metabase object. Use the mapped warehouse/source connection after discovery, or emit an unmapped fallback and write wiki-only. +- Distinguish fallback reasons precisely: if a non-Notion warehouse/dbt connection exists but `sl_discover` cannot find the named table/source, use `no_physical_table`; reserve `no_connection_mapping` for cases where there is no plausible non-Notion target connection at all. ## Tools diff --git a/packages/context/src/ingest/adapters/notion/chunk.ts b/packages/context/src/ingest/adapters/notion/chunk.ts index 89b7e278..4b82a475 100644 --- a/packages/context/src/ingest/adapters/notion/chunk.ts +++ b/packages/context/src/ingest/adapters/notion/chunk.ts @@ -8,7 +8,7 @@ const MAX_NOTION_WORK_UNIT_CHARS = 40_000; export const NOTION_ORG_KNOWLEDGE_WARNING = 'Anything accessible to this Notion integration can become organization knowledge.'; const NOTION_SL_WRITE_GUIDANCE = - 'Write wiki entries with wiki_write. Only write or edit SL sources after sl_discover/sl_read_source confirms a mapped non-Notion target source; if no mapped target exists, emit_unmapped_fallback and keep the fact wiki-only. Do not create SL sources under the Notion connection just because a page mentions a warehouse table.'; + 'Write wiki entries with wiki_write. Only write or edit SL sources after sl_discover/sl_read_source confirms a mapped non-Notion target source; if no mapped target exists, emit_unmapped_fallback and keep the fact wiki-only. If a warehouse/dbt connection exists but the named table or source is absent, use reason no_physical_table rather than no_connection_mapping. Do not create SL sources under the Notion connection just because a page mentions a warehouse table.'; async function walk(root: string): Promise { const entries = await readdir(root, { withFileTypes: true, recursive: true }); diff --git a/packages/context/src/ingest/adapters/notion/cluster.ts b/packages/context/src/ingest/adapters/notion/cluster.ts index 74f3d836..b8f58f23 100644 --- a/packages/context/src/ingest/adapters/notion/cluster.ts +++ b/packages/context/src/ingest/adapters/notion/cluster.ts @@ -9,7 +9,7 @@ export const MIN_PAGES_TO_CLUSTER = 5; const CLUSTER_TEXT_BODY_CHARS = 1024; const CLUSTER_SEED = 42; const NOTION_CLUSTER_SL_WRITE_GUIDANCE = - 'Write wiki entries directly with wiki_write. Only write or edit SL sources after sl_discover/sl_read_source confirms a mapped non-Notion target source; if no mapped target exists, emit_unmapped_fallback and keep the fact wiki-only. Do not create SL sources under the Notion connection just because a page mentions a warehouse table.'; + 'Write wiki entries directly with wiki_write. Only write or edit SL sources after sl_discover/sl_read_source confirms a mapped non-Notion target source; if no mapped target exists, emit_unmapped_fallback and keep the fact wiki-only. If a warehouse/dbt connection exists but the named table or source is absent, use reason no_physical_table rather than no_connection_mapping. Do not create SL sources under the Notion connection just because a page mentions a warehouse table.'; interface ClusterNotionWorkUnitsArgs { workUnits: WorkUnit[]; diff --git a/packages/context/src/ingest/adapters/notion/notion.adapter.test.ts b/packages/context/src/ingest/adapters/notion/notion.adapter.test.ts index 0f425d0b..37baad18 100644 --- a/packages/context/src/ingest/adapters/notion/notion.adapter.test.ts +++ b/packages/context/src/ingest/adapters/notion/notion.adapter.test.ts @@ -242,6 +242,7 @@ describe('NotionSourceAdapter', () => { }); expect(result.workUnits[0].notes).toContain('Synthesize durable wiki and SL knowledge'); expect(result.workUnits[0].notes).toContain('emit_unmapped_fallback'); + expect(result.workUnits[0].notes).toContain('use reason no_physical_table rather than no_connection_mapping'); expect(result.workUnits[0].notes).toContain('Do not create SL sources under the Notion connection'); expect(result.reconcileNotes).toEqual([ 'Notion maxKnowledgeCreatesPerRun=25', diff --git a/packages/context/src/ingest/local-bundle-ingest.test.ts b/packages/context/src/ingest/local-bundle-ingest.test.ts index 6e9aa4aa..a373f528 100644 --- a/packages/context/src/ingest/local-bundle-ingest.test.ts +++ b/packages/context/src/ingest/local-bundle-ingest.test.ts @@ -300,8 +300,8 @@ describe('canonical local ingest', () => { expect(result.result.failedWorkUnits).toEqual([]); const db = new Database(join(project.projectDir, '.ktx', 'db.sqlite'), { readonly: true }); try { - expect(db.prepare('SELECT key, summary FROM knowledge_pages ORDER BY key').all()).toEqual([ - { key: 'orders_context', summary: 'Orders source context' }, + expect(db.prepare('SELECT key, summary, embedding_json IS NOT NULL AS has_embedding FROM knowledge_pages ORDER BY key').all()).toEqual([ + { key: 'orders_context', summary: 'Orders source context', has_embedding: 1 }, ]); } finally { db.close(); diff --git a/packages/context/src/ingest/local-bundle-runtime.ts b/packages/context/src/ingest/local-bundle-runtime.ts index f7c8be80..2634844a 100644 --- a/packages/context/src/ingest/local-bundle-runtime.ts +++ b/packages/context/src/ingest/local-bundle-runtime.ts @@ -52,6 +52,7 @@ import { type ToolSession, } from '../tools/index.js'; import { + buildKnowledgeSearchText, type KnowledgeEventPort, type KnowledgeIndexPort, KnowledgeWikiService, @@ -286,7 +287,10 @@ function scoreText(text: string, query: string): number { class LocalKnowledgeIndex implements KnowledgeIndexPort { private readonly sqlite: SqliteKnowledgeIndex; - constructor(private readonly project: KtxLocalProject) { + constructor( + private readonly project: KtxLocalProject, + private readonly embedding: KtxEmbeddingPort, + ) { this.sqlite = new SqliteKnowledgeIndex({ dbPath: ktxLocalStateDbPath(project) }); } @@ -388,6 +392,7 @@ class LocalKnowledgeIndex implements KnowledgeIndexPort { private async syncAllPagesFromDisk(): Promise { const listed = await this.project.fileStore.listFiles('knowledge', true); + const existingPages = this.sqlite.getExistingPages(); const pages: SqliteKnowledgeIndexPage[] = []; for (const file of listed.files.filter((entry) => entry.endsWith('.md'))) { const parsedPath = parseKnowledgeIndexPath(file); @@ -397,14 +402,21 @@ class LocalKnowledgeIndex implements KnowledgeIndexPort { const path = `knowledge/${file}`; const raw = await this.project.fileStore.readFile(path); const parsed = parseWiki(raw.content); + const tags = parseWikiTags(raw.content); + const searchText = buildKnowledgeSearchText(parsedPath.pageKey, parsed.summary, parsed.content, tags); + const existing = existingPages.get(path); + const embedding = + existing?.searchText === searchText && existing.embedding + ? existing.embedding + : await this.embedding.computeEmbedding(searchText).catch(() => null); pages.push({ path, key: parsedPath.pageKey, scope: parsedPath.scope, summary: parsed.summary, content: parsed.content, - tags: parseWikiTags(raw.content), - embedding: null, + tags, + embedding, }); } this.sqlite.sync(pages); @@ -566,7 +578,7 @@ export function createLocalBundleIngestRuntime( ); const slSourcesRepository = new SqliteSlSourcesIndex({ dbPath }); const slSearchService = new SlSearchService(embedding, slSourcesRepository, logger); - const knowledgeIndex = new LocalKnowledgeIndex(options.project); + const knowledgeIndex = new LocalKnowledgeIndex(options.project, embedding); const knowledgeEvents = new NoopKnowledgeEventPort(); const wikiService = new KnowledgeWikiService(rootFileStore, embedding, knowledgeIndex, options.project.git, logger); const { agentRunner, llmProvider } = resolveAgentRunner(options); diff --git a/packages/context/src/ingest/tools/tool-transcript-summary.test.ts b/packages/context/src/ingest/tools/tool-transcript-summary.test.ts index 8f1229c3..5b4d81cc 100644 --- a/packages/context/src/ingest/tools/tool-transcript-summary.test.ts +++ b/packages/context/src/ingest/tools/tool-transcript-summary.test.ts @@ -36,6 +36,23 @@ describe('tool transcript summaries', () => { expect(summary.fatalErrorCount).toBe(0); }); + it('counts unrecovered wiki_remove structured failures as fatal transcript errors', () => { + const summary = createMutableToolTranscriptSummary('reconcile', '/tmp/reconcile.jsonl'); + + recordToolTranscriptEntry(summary, { + ts: '2026-05-11T00:00:00.000Z', + wuKey: 'reconcile', + toolCallId: 'remove-1', + toolName: 'wiki_remove', + durationMs: 1, + input: { key: 'duplicate-page' }, + output: { structured: { success: false, key: 'duplicate-page' } }, + }); + + expect(summary.errorCount).toBe(1); + expect(summary.fatalErrorCount).toBe(1); + }); + it('keeps unrecovered structured write failures fatal', () => { const summary = createMutableToolTranscriptSummary('wu-1', '/tmp/wu-1.jsonl'); diff --git a/packages/context/src/ingest/tools/tool-transcript-summary.ts b/packages/context/src/ingest/tools/tool-transcript-summary.ts index a335f43a..5e8c2628 100644 --- a/packages/context/src/ingest/tools/tool-transcript-summary.ts +++ b/packages/context/src/ingest/tools/tool-transcript-summary.ts @@ -62,7 +62,7 @@ function recoverableStructuredFailureKey(entry: ToolCallLogEntry): string | null if (!isStructuredToolFailure(entry.output)) { return null; } - if (entry.toolName === 'wiki_write') { + if (entry.toolName === 'wiki_write' || entry.toolName === 'wiki_remove') { return wikiTargetKey(entry); } if (entry.toolName === 'sl_write_source') { @@ -75,7 +75,7 @@ function recoverableStructuredSuccessKey(entry: ToolCallLogEntry): string | null if (!isStructuredToolSuccess(entry.output)) { return null; } - if (entry.toolName === 'wiki_write') { + if (entry.toolName === 'wiki_write' || entry.toolName === 'wiki_remove') { return wikiTargetKey(entry); } if (entry.toolName === 'sl_write_source' || entry.toolName === 'sl_edit_source') { diff --git a/packages/context/src/wiki/tools/wiki-read.tool.test.ts b/packages/context/src/wiki/tools/wiki-read.tool.test.ts new file mode 100644 index 00000000..ccb60abd --- /dev/null +++ b/packages/context/src/wiki/tools/wiki-read.tool.test.ts @@ -0,0 +1,40 @@ +import { describe, expect, it, vi } from 'vitest'; +import type { ToolSession } from '../../tools/index.js'; +import { createTouchedSlSources, type ToolContext } from '../../tools/index.js'; +import { WikiReadTool } from './wiki-read.tool.js'; + +describe('WikiReadTool', () => { + const baseContext: ToolContext = { sourceId: 's', messageId: 'm', userId: 'u' }; + + it('reads from the session wiki service when a worktree-scoped ingest session is present', async () => { + const rootWikiService = { readPageForUser: vi.fn().mockResolvedValue(null) }; + const sessionWikiService = { + readPageForUser: vi.fn().mockResolvedValue({ + pageKey: 'staged-page', + scope: 'GLOBAL', + frontmatter: { summary: 'Staged', tags: ['notion'], refs: ['related'] }, + content: 'A page written earlier in the same ingest worktree.', + }), + }; + const pagesRepository = { findPageByKey: vi.fn().mockResolvedValue({ id: 'page-1' }), incrementUsageCount: vi.fn() }; + const tool = new WikiReadTool(rootWikiService as any, pagesRepository as any); + const session: ToolSession = { + connectionId: 'c', + isWorktreeScoped: true, + preHead: null, + touchedSlSources: createTouchedSlSources(), + actions: [], + semanticLayerService: {} as any, + wikiService: sessionWikiService as any, + configService: {} as any, + gitService: {} as any, + }; + + const result = await tool.call({ key: 'staged-page' }, { ...baseContext, session }); + + expect(rootWikiService.readPageForUser).not.toHaveBeenCalled(); + expect(sessionWikiService.readPageForUser).toHaveBeenCalledWith('u', 'staged-page'); + expect(result.structured).toMatchObject({ found: true, blockKey: 'staged-page', scope: 'GLOBAL' }); + expect(result.markdown).toContain('A page written earlier in the same ingest worktree.'); + }); +}); diff --git a/packages/context/src/wiki/tools/wiki-read.tool.ts b/packages/context/src/wiki/tools/wiki-read.tool.ts index 73eb7e40..f1ee09f7 100644 --- a/packages/context/src/wiki/tools/wiki-read.tool.ts +++ b/packages/context/src/wiki/tools/wiki-read.tool.ts @@ -43,7 +43,8 @@ export class WikiReadTool extends BaseTool { } async call(input: WikiReadInput, context: ToolContext): Promise> { - const page = await this.wikiService.readPageForUser(context.userId, input.key); + const wikiService = context.session?.wikiService ?? this.wikiService; + const page = await wikiService.readPageForUser(context.userId, input.key); if (!page) { return { diff --git a/packages/context/src/wiki/tools/wiki-remove.tool.test.ts b/packages/context/src/wiki/tools/wiki-remove.tool.test.ts index 7999dd26..71abcd12 100644 --- a/packages/context/src/wiki/tools/wiki-remove.tool.test.ts +++ b/packages/context/src/wiki/tools/wiki-remove.tool.test.ts @@ -24,6 +24,7 @@ describe('WikiRemoveTool', () => { it('skips deleteFromIndex when session is worktree-scoped', async () => { const wikiService = { + readPage: vi.fn().mockResolvedValue({ pageKey: 'old', frontmatter: { summary: 'Old' }, content: 'body' }), deletePage: vi.fn().mockResolvedValue(undefined), deleteFromIndex: vi.fn().mockResolvedValue(undefined), }; @@ -47,6 +48,35 @@ describe('WikiRemoveTool', () => { expect(session.actions).toContainEqual(expect.objectContaining({ target: 'wiki', type: 'removed', key: 'old' })); }); + it('finds pages through the session wiki service even when the shared index has not seen the worktree write', async () => { + const wikiService = { + readPage: vi.fn().mockResolvedValue({ pageKey: 'staged', frontmatter: { summary: 'Staged' }, content: 'body' }), + deletePage: vi.fn().mockResolvedValue(undefined), + deleteFromIndex: vi.fn().mockResolvedValue(undefined), + }; + const pagesRepository = { findPageByKey: vi.fn().mockResolvedValue(null) }; + const knowledgeRepository = { createEvent: vi.fn().mockResolvedValue(undefined) }; + const tool = new WikiRemoveTool(wikiService as any, pagesRepository as any, knowledgeRepository as any); + const session: ToolSession = { + connectionId: 'c', + isWorktreeScoped: true, + preHead: null, + touchedSlSources: createTouchedSlSources(), + actions: [], + semanticLayerService: {} as any, + wikiService: wikiService as any, + configService: {} as any, + gitService: {} as any, + }; + + const result = await tool.call({ key: 'staged' } as any, { ...baseContext, session }); + + expect(pagesRepository.findPageByKey).not.toHaveBeenCalled(); + expect(wikiService.readPage).toHaveBeenCalledWith('GLOBAL', null, 'staged'); + expect(wikiService.deletePage).toHaveBeenCalledTimes(1); + expect(result.structured).toEqual({ success: true, key: 'staged' }); + }); + it('returns a friendly message when the page does not exist', async () => { const wikiService = { deletePage: vi.fn(), deleteFromIndex: vi.fn() }; const pagesRepository = { findPageByKey: vi.fn().mockResolvedValue(null) }; diff --git a/packages/context/src/wiki/tools/wiki-remove.tool.ts b/packages/context/src/wiki/tools/wiki-remove.tool.ts index 0e5905ec..bb6865dd 100644 --- a/packages/context/src/wiki/tools/wiki-remove.tool.ts +++ b/packages/context/src/wiki/tools/wiki-remove.tool.ts @@ -46,7 +46,9 @@ export class WikiRemoveTool extends BaseTool { const scope: BlockScope = writesGlobal ? 'GLOBAL' : 'USER'; const scopeId = scope === 'USER' ? context.userId : null; - const existing = await this.pagesRepository.findPageByKey(scope, scopeId, input.key); + const existing = context.session + ? await wikiService.readPage(scope, scopeId, input.key) + : await this.pagesRepository.findPageByKey(scope, scopeId, input.key); if (!existing) { return { markdown: `Page "${input.key}" not found.`,