docs(arch): record the seams v1.4 actually produced, and close the one reverse dependency

docs/architecture.md

@@ -4,6 +4,79 @@ Long-lived decisions that outlive a single feature. Feature-specific design live
 in the issue tracker and in `PRODUCT.md` / `DESIGN.md`; this file records the
 seams that other work is allowed to depend on.
 
+## Module seams
+
+Three seams carry the v1.4 features. These are the names that **actually exist in the
+code** — #60 guessed `DocumentParser` / `SourceStore` / `CitationService`, and none of
+the three was needed. See [considered and rejected](#seams-considered-and-rejected)
+for why, so the next reader does not re-litigate it.
+
+### Document
+
+| Role                          | What it is                                                                     |
+| ----------------------------- | ------------------------------------------------------------------------------ |
+| Parsing contract              | `IDocumentLoader` — `src/main/services/loaders/types.ts`                       |
+| Format dispatch               | `FileParserService` — picks a loader by extension / MIME                       |
+| Implementations               | `PdfLoader`, `DocxLoader`, `MarkdownLoader`, `PptLoader`, `WebLoader`          |
+| Persistence and orchestration | `KnowledgeService` — `documents`, `document_blocks`, chunks, vectors, re-index |
+| Derived structure             | `services/blocks/documentBlocks.ts`, `ChunkingService`, `chunkProvenance.ts`   |
+
+Data invariants for this seam are in [Source provenance](#source-provenance).
+
+### Retrieval
+
+`Retriever` (interface) and `RetrievedEvidence` (result) in
+`src/main/services/retrieval/`; `DenseRetriever` is the only implementation today.
+Rules are in [Retrieval seam](#retrieval-seam).
+
+### Application
+
+**This seam is an architectural boundary, not a class.** There is deliberately no
+`ApplicationService` and no `CitationService`:
+
+- **IPC handlers** (`src/main/ipc/`) translate a request into a service call and
+  translate the answer back. No handler calls `getDatabase()` itself.
+- **Application services** — `ItemService`, `AnkiCardService`, `MindMapService`,
+  `QuizService` — own their tables and may call `getDatabase()`.
+- **Shared pure functions** — `src/shared/utils/`: `citations.ts`,
+  `citationResolution.ts`, `answerSources.ts`, `sourceAnchor.ts`, `excerpt.ts` — hold
+  citation parsing/resolution and source-anchor mapping, because the main _and_ the
+  renderer process both need them and they carry no state.
+
+**Known legacy exception, not addressed here.** `noteHandlers.ts`,
+`notebookHandlers.ts` and part of `chatHandlers.ts` call `db/queries` directly instead
+of a service. That predates v1.4 and #60 deliberately leaves it alone: its non-goals
+forbid removing a legacy path before a real feature exercises the replacement.
+
+### Call paths
+
+Concrete, checkable against the code:
+
+```text
+src/main/ipc/itemHandlers.ts
+  → ItemService                     (Application)
+    → getDatabase()
+      → Drizzle / SQLite
+
+src/main/ipc/knowledgeHandlers.ts
+  → KnowledgeService                (Document)
+    → getDatabase()
+      → Drizzle / SQLite
+
+knownote-doc://<documentId>
+src/main/protocol/documentProtocol.ts
+  → KnowledgeService.getDocumentLocalFilePath(id)   (Document, narrow query)
+    → getDatabase()
+      → documents.localFilePath
+```
+
+The protocol path is the one #60 changed. It used to run its own
+`select().from(documents)` (introduced by #116), which made an Electron plumbing module
+depend on the database from outside the Document/Application layers. It now receives
+the narrow query as a parameter, and `test/architectureBoundary.test.ts` fails if
+database access reappears under `src/main/protocol/`. The narrower query is deliberate:
+the handler needs “a readable file for this id”, not a whole `Document` row.
+
 ## Source provenance
 
 A document is an identity. Everything derived from it can be rebuilt:
@@ -133,3 +206,24 @@ Rules:
 - **The text layer is real text.** pdfjs's `TextLayer` is rendered over the canvas
   so `getSelection()` can map a DOM selection back to a source location; selecting
   the canvas would give nothing.
+
+## Seams considered and rejected
+
+Recorded so they are not proposed again without a new reason. Each was considered
+while building v1.4 and deliberately not introduced.
+
+- **`DocumentParser` — rejected.** `IDocumentLoader` plus `FileParserService` already
+  form the parsing seam, and all five format loaders implement it. A second interface
+  over the same boundary would be duplicate abstraction with no new consumer.
+- **`SourceStore` — currently rejected.** There is no storage consumer independent
+  enough to justify a repository layer: the provenance helpers
+  (`chunkProvenance.ts`) take the database handle as an explicit parameter, and
+  `KnowledgeService` still owns Document persistence orchestration. Revisit only if a
+  second, genuinely independent storage consumer appears.
+- **`CitationService` — rejected.** Citation parsing, resolution and source-anchor
+  mapping are stateless and shared between the main and renderer processes. Pure
+  functions in `src/shared/utils/` are the right shape; a service would add a
+  process-boundary round trip and a lifecycle for something that has no state.
+- **A Model seam — not part of this work.** #44 already established it
+  (`ModelClient` + `protocols/*`, driven by `ConnectionManager`); it is not rebuilt or
+  re-documented here.

src/main/index.ts

@@ -77,7 +77,13 @@ app.whenReady().then(async () => {
   initDatabase()
   runMigrations()
   initVectorStore()
-  registerDocumentProtocolHandler()
+  // The resolver is lazy on purpose: `knowledgeService` is constructed further down, and
+  // the protocol only ever answers requests from a renderer that loads after startup.
+  // Reading it per request keeps the dependency direction protocol → Document layer →
+  // database without reordering startup (#60).
+  registerDocumentProtocolHandler(
+    (documentId) => knowledgeService?.getDocumentLocalFilePath(documentId) ?? null
+  )
   Logger.info('Main', 'Database initialized')
 
   // Initialize electron-store

src/main/protocol/documentProtocol.ts

@@ -9,9 +9,6 @@
 
 import { net, protocol } from 'electron'
 import { pathToFileURL } from 'url'
-import { eq } from 'drizzle-orm'
-import { getDatabase } from '../db'
-import { documents } from '../db/schema'
 import { DOCUMENT_SCHEME, parseDocumentUrl } from '../../shared/utils/documentUrl'
 import Logger from '../../shared/utils/logger'
 
@@ -45,23 +42,22 @@ export function registerDocumentScheme(): void {
 }
 
 /** 在 app ready 之后调用，安装只读的字节服务 handler。 */
-export function registerDocumentProtocolHandler(): void {
+export function registerDocumentProtocolHandler(
+  resolveLocalFilePath: (documentId: string) => string | null
+): void {
   protocol.handle(DOCUMENT_SCHEME, async (request) => {
     const documentId = parseDocumentUrl(request.url)
     if (!documentId) return new Response('Not found', { status: 404 })
 
-    const document = getDatabase()
-      .select({ localFilePath: documents.localFilePath })
-      .from(documents)
-      .where(eq(documents.id, documentId))
-      .get()
-
-    if (!document?.localFilePath) return new Response('Not found', { status: 404 })
+    // 路径来自 Document 层，不来自这里，也不来自请求。handler 自己不碰数据库：
+    // 它只需要一个「可读文件」的答案，依赖方向保持 protocol → Document 层 → database（#60）。
+    const localFilePath = resolveLocalFilePath(documentId)
+    if (!localFilePath) return new Response('Not found', { status: 404 })
 
     try {
       // net.fetch handles file:// in the main process and streams the response, so
       // a large PDF is not read into memory to be served.
-      const response = await net.fetch(pathToFileURL(document.localFilePath).toString())
+      const response = await net.fetch(pathToFileURL(localFilePath).toString())
       const headers = new Headers(response.headers)
       const origin = request.headers.get('Origin') ?? 'null'
       if (ALLOWED_ORIGINS.has(origin)) headers.set('Access-Control-Allow-Origin', origin)

src/main/services/KnowledgeService.ts

@@ -815,6 +815,26 @@ export class KnowledgeService {
     return db.select().from(documents).where(eq(documents.id, documentId)).get()
   }
 
+  /**
+   * 只取一份来源的可读文件路径（#60）。
+   *
+   * 刻意做成一个窄查询，而不是让 `knownote-doc://` 的协议 handler 调 `getDocument()`：
+   * 那个 handler 是 Electron plumbing，它只需要「按 id 找一个能读的文件」，把整行
+   * `Document` 交给它会把依赖面撑得比实际需要宽。
+   *
+   * 存在的意义是固定依赖方向：protocol → Document 层 → database，而不是 protocol 自己
+   * 去 `getDatabase()`。不用为它另建 SourceStore 之类的 repository —— 一个方法就够。
+   */
+  getDocumentLocalFilePath(documentId: string): string | null {
+    const db = getDatabase()
+    const row = db
+      .select({ localFilePath: documents.localFilePath })
+      .from(documents)
+      .where(eq(documents.id, documentId))
+      .get()
+    return row?.localFilePath ?? null
+  }
+
   /**
    * 获取文档的所有 chunks
    */

src/main/smokeTest.ts

@@ -247,7 +247,12 @@ async function runChecks(): Promise<string[]> {
       Date.now()
     )
 
-  registerDocumentProtocolHandler()
+  // Hoisted from the re-index section below so the protocol handler is wired to the
+  // real Document-layer query rather than a stand-in: this check is what proves
+  // `knownote-doc://` serves bytes *through* that path, and that it 404s when the query
+  // answers null (#60).
+  const knowledge = new KnowledgeService(fakeEmbeddingService())
+  registerDocumentProtocolHandler((documentId) => knowledge.getDocumentLocalFilePath(documentId))
   const served = await net.fetch(documentUrl(protocolDocumentId))
   assert(served.ok, `an owned document did not serve (status ${served.status})`)
   assert((await served.text()) === protocolPayload, 'the document protocol served the wrong bytes')
@@ -551,7 +556,6 @@ async function runChecks(): Promise<string[]> {
     .prepare('INSERT INTO notebooks (id, title, created_at, updated_at) VALUES (?, ?, ?, ?)')
     .run(reindexNotebook, 'Smoke re-index notebook', reindexNow, reindexNow)
 
-  const knowledge = new KnowledgeService(fakeEmbeddingService())
   const reindexDocId = await knowledge.addDocumentFromFile(
     reindexNotebook,
     join(fixtures, 'sample.pdf')

test/architectureBoundary.test.ts

@@ -0,0 +1,141 @@
+import { test } from 'node:test'
+import assert from 'node:assert/strict'
+import { readFileSync, readdirSync, statSync } from 'node:fs'
+import { join } from 'node:path'
+
+/**
+ * 守住 Electron plumbing → Document 层的依赖方向（#60）。
+ *
+ * #60 的第三条 acceptance 不是文字要求，而是在约束依赖方向：
+ *
+ *   no new service reaches directly into getDatabase() from outside the
+ *   Document/Application layers
+ *
+ * `src/main/protocol/` 是 Electron plumbing —— 它既不是 Application 也不是 Document
+ * 层。`knownote-doc://` 曾经自己 `select().from(documents)` 取 `localFilePath`，是 v1.4
+ * （#116）新增代码里唯一一处这类反向依赖。现在它改为接收一个窄查询
+ * （`KnowledgeService.getDocumentLocalFilePath()`），方向变成：
+ *
+ *   protocol → Document 层 → getDatabase() → Drizzle/SQLite
+ *
+ * 这个测试把那条方向固定下来：谁在 protocol 层重新引入数据库访问，CI 会在这里失败。
+ * 失败时该怎么做：把需要的查询加到 Document 层并注入进来，不要只是把测试改绿。
+ */
+
+/** Electron plumbing：这里的模块只负责把请求翻译成对服务层的调用。 */
+const PLUMBING_DIR = join('src', 'main', 'protocol')
+
+const sourceFiles = (dir: string, files: string[] = []): string[] => {
+  for (const entry of readdirSync(dir)) {
+    const path = join(dir, entry)
+    if (statSync(path).isDirectory()) sourceFiles(path, files)
+    else if (/\.(ts|tsx|mts|cts)$/.test(path)) files.push(path)
+  }
+  return files
+}
+
+const importSpecifiers = (source: string): string[] => {
+  const pattern = /(?:from\s*|import\s*\(\s*|require\s*\(\s*|import\s+)['"]([^'"\n]+)['"]/g
+  return [...source.matchAll(pattern)].map((match) => match[1])
+}
+
+/**
+ * 数据库层的入口：直接拿连接、Drizzle，或 **`db` 目录下的任何东西**。
+ *
+ * 这里必须把整个 db 子树当作边界，而不是只列 `db` 和 `db/schema`。只匹配目录名本身
+ * 或固定后缀会让 `../db/queries` 漏过去 —— 而 `db/queries` 恰好就是
+ * `docs/architecture.md` 里记为「数据库直达」的那条 legacy 路径。一个漏掉自己文档里
+ * 点名情况的 guard，比没有这条规则更糟：它会让人相信一项未被执行的约束。
+ */
+const isDatabaseAccess = (specifier: string): boolean =>
+  specifier === 'drizzle-orm' ||
+  specifier.startsWith('drizzle-orm/') ||
+  /(^|\/)db(?:\/|$)/.test(specifier)
+
+test('the database matcher covers the whole db subtree, not just its exact name', () => {
+  // The false negative this pins: `../db/queries` used to pass the scan even though
+  // architecture.md records `db/queries` as a database-reaching legacy path. Anything
+  // under the db directory is database access.
+  for (const specifier of [
+    '../db',
+    '../db/schema',
+    '../db/queries',
+    '../../db/foo',
+    'db',
+    'db/queries',
+    'drizzle-orm',
+    'drizzle-orm/sqlite-core'
+  ]) {
+    assert.equal(isDatabaseAccess(specifier), true, `should be flagged: ${specifier}`)
+  }
+
+  // It must not swallow legitimate imports either: a guard that fires on ordinary code
+  // is one people route around, which is worse than a narrow one.
+  for (const specifier of [
+    '../services/KnowledgeService',
+    '../../shared/utils/documentUrl',
+    '../../shared/utils/logger',
+    'electron',
+    'url',
+    'db-utils',
+    '@shared/dbx'
+  ]) {
+    assert.equal(isDatabaseAccess(specifier), false, `should not be flagged: ${specifier}`)
+  }
+})
+
+test('the protocol layer does not reach into the database', () => {
+  const offenders: string[] = []
+
+  for (const file of sourceFiles(PLUMBING_DIR)) {
+    const source = readFileSync(file, 'utf8')
+
+    for (const specifier of importSpecifiers(source)) {
+      if (isDatabaseAccess(specifier)) offenders.push(`${file} imports ${specifier}`)
+    }
+
+    // Import form is not the only way in: a lazy `require('../../db')` or a direct
+    // call would be missed by the specifier scan, so check the call too.
+    if (/\bgetDatabase\s*\(/.test(source)) offenders.push(`${file} calls getDatabase()`)
+  }
+
+  assert.deepEqual(
+    offenders,
+    [],
+    'The protocol layer is Electron plumbing and must not query the database directly ' +
+      '(#60, acceptance 3). Add a narrow query to the Document layer ' +
+      '(KnowledgeService) and inject it via registerDocumentProtocolHandler() instead of ' +
+      'reaching for getDatabase() here.'
+  )
+})
+
+test('the protocol layer gets its data through an injected resolver', () => {
+  // The invariant is only meaningful if the handler really takes its answer from the
+  // caller: a module that had both the db import *and* the injection would pass the test
+  // above only until someone used the import.
+  const source = readFileSync(join(PLUMBING_DIR, 'documentProtocol.ts'), 'utf8')
+
+  assert.match(
+    source,
+    /resolveLocalFilePath\s*:\s*\(/,
+    'registerDocumentProtocolHandler must take the document-path resolver as a parameter'
+  )
+})
+
+test('the guard actually scans the protocol layer', () => {
+  // A boundary test that silently scans nothing is worse than no test: it would report
+  // the direction as enforced while reading no files.
+  const files = sourceFiles(PLUMBING_DIR)
+
+  assert.ok(files.length > 0, `expected to scan ${PLUMBING_DIR}, got ${files.length} files`)
+  assert.ok(
+    files.some((file) => file.endsWith('documentProtocol.ts')),
+    'the document protocol handler must be inside the scanned set'
+  )
+
+  // And the extraction really finds imports.
+  assert.ok(
+    importSpecifiers(readFileSync(files[0], 'utf8')).includes('electron'),
+    'import extraction found no specifiers in the protocol layer'
+  )
+})