Reading-intake friction fixes (6 real-usage items)

skills/cricknote-reading-intake/SKILL.md

@@ -11,16 +11,19 @@ One paper at a time. All writes go through `cricknote tool`.
 1. `cricknote tool zotero_fetch_item '{"citekey":"<key>"}'` (or `{"doi":"..."}`).
 2. `cricknote tool zotero_prepare_bundle '{...}'` to copy the PDF into
    `Reading/attachments/<slug>/`.
-3. `cricknote tool create_reading_note '{"slug":"<slug>","title":"<t>","authors":["..."],"year":2026,"journal":"<j>","doi":"<doi>"}'`.
+3. `cricknote tool ingest_reading_bundle '{"slug":"<slug>","title":"<t>","authors":["..."],"year":2026,"journal":"<j>","doi":"<doi>"}'`
+   — discovers the copied PDF and registers it as a source, so the note compiles.
 
 ## From local files (no Zotero)
 1. Put files under `Reading/attachments/<slug>/`.
 2. `cricknote tool discover_reading_bundle '{"slug":"<slug>"}'`.
-3. `cricknote tool create_reading_note '{...}'`.
+3. `cricknote tool ingest_reading_bundle '{...}'` — registers the discovered files
+   as sources. (Use `create_reading_note` only for a note with no files yet.)
 
 ## Analyze the paper
 1. `cricknote tool compile_reading_note '{"path":"Reading/Papers/<slug>.md"}'`
-   — returns source text.
+   — returns source text, with `--- page N ---` markers between PDF pages
+   (use them to note which page each figure is on).
 2. Draft the **Figure Map** AND the CREATE sections. Show both to the user.
 
    **Figure Map rules** (goes at the top, before `## Claims`):
@@ -36,7 +39,9 @@ One paper at a time. All writes go through `cricknote tool`.
    - If no figures are found (review, theory, or methods paper): leave `## Figure Map` with a single line:
      `<!-- No data figures found in compiled sources -->`
 
-3. Write it: `cricknote tool vault_write '{"path":"Reading/Papers/<slug>.md","content":"<full note>"}'`.
+3. Write it: `cricknote tool vault_write_body '{"path":"Reading/Papers/<slug>.md","body":"<note body>"}'`
+   — preserves the frontmatter (authors, sources, etc.); you supply only the body
+   (Figure Map + CREATE sections). Use `vault_write` only when creating a file from scratch.
 
 ## Check status
 `cricknote tool reading_pipeline_status '{"path":"Reading/Papers/<slug>.md"}'`

src/agent/build-registry.ts

@@ -10,6 +10,22 @@ import { createContextTools } from './tools/context.js';
 import { createSerialTools } from './tools/serial-tools.js';
 import { createKbTools } from './tools/kb-tools.js';
 import { createZoteroTools } from './tools/zotero-tools.js';
+import { loadConfig } from '../config/config.js';
+
+/**
+ * Resolve the vault-relative attachments directory for the reading-intake
+ * pipeline. Mirrors where zotero_prepare_bundle writes PDFs
+ * (config.zotero.vault_pdf_dir) so discover/ingest/compile read from the same
+ * place a Zotero bundle was written to. Defaults to 'Reading/attachments' and
+ * never throws — a missing or unreadable config falls back to the default.
+ */
+function resolveAttachmentsDir(): string {
+  try {
+    return loadConfig().zotero?.vault_pdf_dir ?? 'Reading/attachments';
+  } catch {
+    return 'Reading/attachments';
+  }
+}
 
 /**
  * Build the complete CrickNote tool registry. Shared by the Obsidian runtime
@@ -31,14 +47,16 @@ export function buildToolRegistry(
     for (const h of handlers) registry.register(h);
   };
 
+  const attachmentsDir = resolveAttachmentsDir();
+
   add(createVaultTools(vaultPath, conflictDetector, db));
   add(createSearchTools(db));
   add(createTaskTools(vaultPath, conflictDetector));
-  add(createTemplateTools(vaultPath, conflictDetector));
-  add(createReadingIntakeTools(vaultPath, conflictDetector));
+  add(createTemplateTools(vaultPath, conflictDetector, attachmentsDir));
+  add(createReadingIntakeTools(vaultPath, conflictDetector, attachmentsDir));
   add(createContextTools(vaultPath));
   add(createSerialTools(vaultPath, db));
-  add(createKbTools(vaultPath));
+  add(createKbTools(vaultPath, undefined, attachmentsDir));
   add(createZoteroTools(vaultPath));
 
   return registry;

src/agent/tools/kb-tools.ts

@@ -37,6 +37,7 @@ function isValidSlug(s: string): boolean { return SLUG_RE.test(s); }
 export function createKbTools(
   vaultPath: string,
   injectedDb?: Database.Database,
+  attachmentsDir = 'Reading/attachments',
 ): ToolHandler[] {
   void log;
   void injectedDb;
@@ -94,7 +95,8 @@ export function createKbTools(
         const result = await loadSources(
           sources as Array<{ type: string; path: string }>,
           sourceSlug,
-          vaultPath
+          vaultPath,
+          attachmentsDir
         );
 
         return JSON.stringify({

src/agent/tools/reading-intake.ts

@@ -17,27 +17,12 @@ import {
   syncReadingBodyTitle,
   type ReadingSourceInput,
   type ReadingPipelineStep,
-  type ReadingSourceType,
 } from '../../knowledge/reading-note.js';
 import { readMappingArtifact } from '../../knowledge/mapping-artifact.js';
+import { discoverBundle } from '../../knowledge/reading-bundle.js';
 import { resolveVaultPath } from '../../utils/paths.js';
 import { renderNoteTemplate, type RenderResult, type TemplateKind } from '../../templates/template-loader.js';
 
-interface DiscoveredBundleFile {
-  path: string;
-  type: ReadingSourceType;
-  readable: boolean;
-}
-
-interface BundleDiscoveryResult {
-  slug: string;
-  folderExists: boolean;
-  bundlePath: string;
-  discoveredFiles: DiscoveredBundleFile[];
-  recommendedSources: ReadingSourceInput[];
-  warnings: string[];
-}
-
 interface MappingArtifactSummary {
   path?: string;
   status?: string;
@@ -46,102 +31,13 @@ interface MappingArtifactSummary {
   cleanupCandidates?: string[];
 }
 
-const TEXT_SOURCE_EXTENSIONS = new Set(['.md', '.txt']);
-const IGNORED_BUNDLE_FILES = new Set(['.ds_store']);
-
 function normalizeBundleSlug(value: unknown): string {
   if (typeof value !== 'string' || !value.trim()) {
     throw new Error('slug is required.');
   }
   return slugifyReadingTitle(value);
 }
 
-function classifyBundleFile(fileName: string): { type: ReadingSourceType; readable: boolean } {
-  const lower = fileName.toLowerCase();
-  const ext = path.extname(lower);
-
-  if (ext === '.pdf') {
-    return { type: 'pdf', readable: true };
-  }
-
-  if (TEXT_SOURCE_EXTENSIONS.has(ext)) {
-    if (lower.includes('notebooklm')) {
-      return { type: 'notebooklm', readable: true };
-    }
-    if (lower.includes('web')) {
-      return { type: 'web', readable: true };
-    }
-    return { type: 'notes', readable: true };
-  }
-
-  return { type: 'other', readable: false };
-}
-
-function discoverBundle(vaultPath: string, slug: string): BundleDiscoveryResult {
-  const bundlePath = resolveVaultPath(vaultPath, path.join('Reading', 'attachments', slug));
-  const warnings: string[] = [];
-
-  if (!fs.existsSync(bundlePath) || !fs.statSync(bundlePath).isDirectory()) {
-    return {
-      slug,
-      folderExists: false,
-      bundlePath,
-      discoveredFiles: [],
-      recommendedSources: [],
-      warnings: [`Reading bundle not found: Reading/attachments/${slug}`],
-    };
-  }
-
-  const discoveredFiles: DiscoveredBundleFile[] = [];
-
-  for (const entry of fs.readdirSync(bundlePath, { withFileTypes: true }).sort((a, b) => a.name.localeCompare(b.name))) {
-    if (IGNORED_BUNDLE_FILES.has(entry.name.toLowerCase())) {
-      continue;
-    }
-
-    if (!entry.isFile()) {
-      warnings.push(`Skipping non-file bundle entry "${entry.name}".`);
-      continue;
-    }
-
-    const relativePath = normalizeReadingSourcePath(entry.name);
-    const classified = classifyBundleFile(relativePath);
-    discoveredFiles.push({
-      path: relativePath,
-      type: classified.type,
-      readable: classified.readable,
-    });
-
-    if (!classified.readable) {
-      warnings.push(`Unsupported bundle file "${relativePath}" — only .pdf, .md, and .txt are used for reading intake.`);
-    }
-  }
-
-  const recommendedSources = normalizeReadingSources(
-    discoveredFiles
-      .filter((file) => file.readable)
-      .map((file) => ({ type: file.type, path: file.path }))
-  );
-
-  const pdfCount = discoveredFiles.filter((file) => file.type === 'pdf' && file.readable).length;
-  if (pdfCount > 1) {
-    warnings.push(`Multiple PDF files found in Reading/attachments/${slug}; review the recommended sources before ingesting.`);
-  }
-
-  if (recommendedSources.length === 0) {
-    warnings.push(`Reading bundle "${slug}" has no readable source files yet.`);
-  }
-
-  return {
-    slug,
-    folderExists: true,
-    bundlePath,
-    discoveredFiles,
-    recommendedSources,
-    warnings,
-  };
-}
-
 function normalizeExcludedPaths(paths: unknown): Set<string> {
   if (!Array.isArray(paths)) {
     return new Set<string>();
@@ -275,7 +171,8 @@ function determinePipelineStep(
 
 export function createReadingIntakeTools(
   vaultPath: string,
-  conflictDetector?: ConflictDetector
+  conflictDetector?: ConflictDetector,
+  attachmentsDir = 'Reading/attachments'
 ): ToolHandler[] {
   return [
     {
@@ -298,7 +195,7 @@ export function createReadingIntakeTools(
           return JSON.stringify({ error: (err as Error).message });
         }
 
-        const discovery = discoverBundle(vaultPath, slug);
+        const discovery = discoverBundle(vaultPath, slug, attachmentsDir);
         return JSON.stringify({
           slug: discovery.slug,
           folder_exists: discovery.folderExists,
@@ -352,10 +249,10 @@ export function createReadingIntakeTools(
           return JSON.stringify({ error: (err as Error).message });
         }
 
-        const discovery = discoverBundle(vaultPath, slug);
+        const discovery = discoverBundle(vaultPath, slug, attachmentsDir);
 
         if (!discovery.folderExists) {
-          return JSON.stringify({ error: `Reading bundle not found: Reading/attachments/${slug}` });
+          return JSON.stringify({ error: `Reading bundle not found: ${path.join(attachmentsDir, slug)}` });
         }
 
         let excludedPaths: Set<string>;
@@ -379,13 +276,13 @@ export function createReadingIntakeTools(
         selectedSources = selectedSources.filter((source) => !excludedPaths.has(source.path));
 
         if (selectedSources.length === 0) {
-          return JSON.stringify({ error: `No readable sources selected for Reading/attachments/${slug}` });
+          return JSON.stringify({ error: `No readable sources selected for ${path.join(attachmentsDir, slug)}` });
         }
 
         for (const source of selectedSources) {
           let sourcePath: string;
           try {
-            sourcePath = resolveVaultPath(vaultPath, path.join('Reading', 'attachments', slug, source.path));
+            sourcePath = resolveVaultPath(vaultPath, path.join(attachmentsDir, slug, source.path));
           } catch {
             return JSON.stringify({ error: `Selected source resolves outside the vault: "${source.path}"` });
           }
@@ -538,7 +435,7 @@ export function createReadingIntakeTools(
           }
         }
 
-        const discovery = discoverBundle(vaultPath, slug);
+        const discovery = discoverBundle(vaultPath, slug, attachmentsDir);
 
         if (!noteRef || !fs.existsSync(noteRef.absPath)) {
           return JSON.stringify({

src/agent/tools/templates.ts

@@ -14,9 +14,10 @@ import {
   type ReadingSourceInput,
 } from '../../knowledge/reading-note.js';
 import { resolveVaultPath } from '../../utils/paths.js';
+import { discoverBundle } from '../../knowledge/reading-bundle.js';
 import { renderNoteTemplate, type RenderResult } from '../../templates/template-loader.js';
 
-export function createTemplateTools(vaultPath: string, conflictDetector?: ConflictDetector): ToolHandler[] {
+export function createTemplateTools(vaultPath: string, conflictDetector?: ConflictDetector, attachmentsDir = 'Reading/attachments'): ToolHandler[] {
   return [
     {
       definition: {
@@ -57,6 +58,16 @@ export function createTemplateTools(vaultPath: string, conflictDetector?: Confli
           } catch (err) {
             return JSON.stringify({ error: (err as Error).message });
           }
+        } else {
+          // Defensive: if files are already in the bundle dir for this slug, register
+          // them so the note isn't created sourceless (which would make
+          // compile_reading_note report sources_missing). The no-bundle placeholder
+          // capability is preserved — discovery is skipped when the folder is absent
+          // or empty. (Prefer ingest_reading_bundle; this just closes the footgun.)
+          const discovery = discoverBundle(vaultPath, slug, attachmentsDir);
+          if (discovery.folderExists && discovery.recommendedSources.length > 0) {
+            normalizedSources = discovery.recommendedSources;
+          }
         }
         let notePath: string;
         try {

src/agent/tools/vault.ts

@@ -171,5 +171,48 @@ export function createVaultTools(
         });
       },
     },
+    {
+      definition: {
+        name: 'vault_write_body',
+        description: "Replace the body of an existing note while preserving its frontmatter exactly. Use to fill in or update a reading note's sections (Figure Map, Claims, Reasoning, …) without reproducing the frontmatter. Triggers safe edit flow (diff preview, user confirmation).",
+        parameters: {
+          type: 'object',
+          properties: {
+            path: { type: 'string', description: 'Relative path to the existing note' },
+            body: { type: 'string', description: 'New markdown body — everything after the frontmatter block' },
+          },
+          required: ['path', 'body'],
+        },
+      },
+      execute: async (args) => {
+        let notePath: string;
+        try {
+          notePath = resolveVaultPath(vaultPath, args.path as string);
+        } catch {
+          return JSON.stringify({ error: `Invalid path: "${args.path}"` });
+        }
+        if (!fs.existsSync(notePath)) {
+          return JSON.stringify({ error: `File not found: ${args.path}` });
+        }
+        const existing = fs.readFileSync(notePath, 'utf-8');
+        // Match the leading frontmatter block verbatim — no re-serialization, so
+        // folded YAML, key order, and long author lists are preserved exactly.
+        const fmMatch = existing.match(/^(---\r?\n[\s\S]*?\r?\n---)[ \t]*\r?\n?/);
+        if (!fmMatch) {
+          return JSON.stringify({ error: `No frontmatter block in ${args.path}. Use vault_write to set frontmatter and body together.` });
+        }
+        // Record snapshot before modification so conflict detection is active.
+        conflictDetector?.recordFileRead(notePath, existing);
+        const frontmatter = fmMatch[1];
+        const body = (args.body as string).replace(/^\n+/, '');
+        const newContent = `${frontmatter}\n\n${body}`;
+        return JSON.stringify({
+          type: 'pending_edit',
+          path: notePath,
+          newContent,
+          operation: 'update',
+        });
+      },
+    },
   ];
 }