From 613ebfad329f7367983ee619834587ded38451c1 Mon Sep 17 00:00:00 2001 From: Research Assistant Date: Fri, 8 May 2026 22:45:25 +0800 Subject: [PATCH] =?UTF-8?q?docs:=20complete=20v2.0=20project=20research=20?= =?UTF-8?q?=E2=80=94=206-layer=20testing=20infrastructure=20synthesis?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - STACK.md: pytest 8+, Vitest 2+, obsidian-test-mocks, plasma CI matrix - FEATURES.md: table stakes, differentiators, anti-features, MVP recommendation - ARCHITECTURE.md: modified testing diamond, mock systems, fixture hierarchy, 6 ADRs - PITFALLS.md: 13 pitfalls with prevention strategies, recovery plans - SUMMARY.md: cross-cutting synthesis with 5-phase roadmap implications --- .planning/research/ARCHITECTURE.md | 1691 ++++++++++++++++++++++------ .planning/research/FEATURES.md | 508 ++------- .planning/research/PITFALLS.md | 1539 +++++++++++++++++-------- .planning/research/STACK.md | 259 +---- .planning/research/SUMMARY.md | 471 ++++---- 5 files changed, 2707 insertions(+), 1761 deletions(-) diff --git a/.planning/research/ARCHITECTURE.md b/.planning/research/ARCHITECTURE.md index bd960ef5..7c327861 100644 --- a/.planning/research/ARCHITECTURE.md +++ b/.planning/research/ARCHITECTURE.md @@ -1,371 +1,1328 @@ -# Architecture Research: v1.8 AI Discussion & Deep-Reading Dashboard +# Architecture: 6-Layer Testing Infrastructure -**Domain:** Obsidian plugin dashboard + Python CLI hybrid -**Researched:** 2026-05-06 -**Confidence:** HIGH (verified against existing source code at `paperforge/plugin/main.js` and `paperforge/worker/asset_index.py`) - -## System Overview - -``` -┌──────────────────────────────────────────────────────────────────┐ -│ Obsidian Plugin (JS/TS) — THIN SHELL │ -│ ┌────────────────────────────────────────────────────────────┐ │ -│ │ PaperForgeStatusView (_detectAndSwitch) │ │ -│ │ Mode Detection: │ │ -│ │ no file → 'global' │ │ -│ │ .base file → 'collection' │ │ -│ │ deep-reading.md → 'deep-reading' ←── NEW v1.8 │ │ -│ │ .md + zotero_key → 'paper' │ │ -│ │ .md other → 'global' │ │ -│ ├────────────────────────────────────────────────────────────┤ │ -│ │ _renderGlobalMode() │ _renderCollectionMode() │ │ -│ │ _renderPaperMode() │ _renderDeepReadingMode() ← NEW │ │ -│ ├────────────────────────────────────────────────────────────┤ │ -│ │ _getCachedIndex() ──reads── formal-library.json │ │ -│ │ _renderDeepReadingMode ──reads── deep-reading.md │ │ -│ │ ──reads── ai/discussion.json │ │ -│ └────────────────────────────────────────────────────────────┘ │ -├──────────────────────────────────────────────────────────────────┤ -│ File System (Obsidian Vault) │ -│ ┌────────────────────────────────────────────────────────────┐ │ -│ │ Literature/{domain}/{key} - {title}/ │ │ -│ │ ├── {key} - {title}.md (formal note) │ │ -│ │ ├── deep-reading.md (extracted from note) │ │ -│ │ ├── fulltext.md (OCR output) │ │ -│ │ ├── figures/ (extracted charts) │ │ -│ │ └── ai/ ←── v1.6 created │ │ -│ │ ├── discussion.json ←── NEW v1.8 JS reads │ │ -│ │ └── discussion.md ←── NEW v1.8 Python writes│ │ -│ └────────────────────────────────────────────────────────────┘ │ -├──────────────────────────────────────────────────────────────────┤ -│ Python CLI (paperforge) │ -│ ┌────────────────────────────────────────────────────────────┐ │ -│ │ paperforge/worker/sync.py — generates deep-reading.md │ │ -│ │ paperforge/worker/asset_index.py — builds formal-library.json│ │ -│ │ paperforge/skills/literature-qa/ld_deep.py — /pf-deep │ │ -│ │ paperforge/commands/context.py — AI context packs │ │ -│ │ [NEW] AI discussion recorder — writes ai/discussion.* │ │ -│ └────────────────────────────────────────────────────────────┘ │ -└──────────────────────────────────────────────────────────────────┘ -``` - -## Component Responsibilities - -| Component | Responsibility | Owner | Modified in v1.8 | -|-----------|----------------|-------|------------------| -| `_detectAndSwitch()` | Active file → mode resolution | JS | **YES** — add `deep-reading.md` detection | -| `_switchMode()` | Dispatch to mode renderer | JS | **YES** — add `case 'deep-reading'` | -| `_renderDeepReadingMode()` | Deep-reading dashboard UI | JS | **NEW** — entirely new method | -| `_renderPaperMode()` | Per-paper dashboard | JS | **YES** — add "Jump to Deep Reading" button | -| `_buildEnvelope()` | Index envelope with version | Python | **YES** — add `paperforge_version` | -| `_fetchStats()` | Global dashboard data | JS | **YES** — read version from envelope | -| AI discussion recorder | Write `discussion.json` + `discussion.md` | Python | **NEW** — agent-level module | -| `ld_deep.py` / `/pf-deep` | Deep reading scaffold + validation | Python | No change | -| `sync.py` `migrate_to_workspace()` | Creates `ai/` directory | Python | No change (already done) | - -## Mode Detection Flow (v1.8) - -``` -_detectAndSwitch() - │ - ├─ activeFile === null ──────────────────────────→ 'global' - │ - ├─ activeFile.extension === 'base' ──────────────→ 'collection' - │ - ├─ activeFile.extension === 'md' - │ │ - │ ├─ basename === 'deep-reading' ←── NEW CHECK (BEFORE zotero_key) - │ │ AND path matches workspace pattern - │ │ │ - │ │ ├─ extract parent key from path → 'deep-reading' - │ │ │ (Literature/{domain}/{key} - {title}/deep-reading.md) - │ │ └─ set _currentDeepReadingKey, _currentDeepReadingEntry - │ │ - │ └─ frontmatter has zotero_key ──────────────→ 'paper' - │ - └─ fallback ────────────────────────────────────→ 'global' -``` - -**Critical ordering**: The `deep-reading.md` check must come BEFORE the generic `.md + zotero_key` check. Why: `deep-reading.md` is extracted from the formal note and may carry the same frontmatter (including `zotero_key`). We need to route it to the deep-reading dashboard, not the per-paper dashboard. - -**Detection strategy**: Check `activeFile.basename === 'deep-reading'` AND verify the parent directory matches workspace naming pattern `{8-char key} - {slug}`. Extract the zotero_key from the parent directory name (first 8 characters before ` - `). - -## deep-reading.md Detection — Integration Details - -The detection needs to handle the workspace path structure: - -``` -Literature/{domain}/{zotero_key} - {title_slug}/deep-reading.md -``` - -```javascript -// Proposed addition to _detectAndSwitch(), inserted before the .md + zotero_key check: -if (ext === 'md' && activeFile.basename === 'deep-reading') { - // Check if parent directory matches workspace pattern - const parentDir = activeFile.parent ? activeFile.parent.name : ''; - const workspaceMatch = parentDir.match(/^([A-Z0-9]{8})\s+-\s+(.+)$/); - if (workspaceMatch) { - const extractedKey = workspaceMatch[1]; - this._currentPaperKey = extractedKey; - this._currentPaperEntry = this._findEntry(extractedKey); - this._currentDomain = null; - this._switchMode('deep-reading'); - return; - } - // Not in a workspace dir — fall through to global - this._switchMode('global'); - return; -} -``` - -## New: _renderDeepReadingMode() Component Design - -### Data Sources -| Source | How Read | Fields Used | -|--------|----------|-------------| -| `formal-library.json` | `_findEntry(key)` via `_getCachedIndex()` | lifecycle, health, ocr_status, deep_reading_status, maturity, title, year, authors | -| `deep-reading.md` | `this.app.vault.getAbstractFileByPath()` → `read()` | Pass 1 summary, full text sections | -| `ai/discussion.json` | `fs.readFileSync()` (same pattern as index) | AI Q&A history, session metadata | - -### Sub-Components -``` -_renderDeepReadingMode() - │ - ├─ [Status Bar] — inline row showing: - │ lifecycle badge, ocr_status badge, deep_reading_status badge, maturity level - │ - ├─ [Paper Info Header] — title, authors, year, domain - │ (reuses existing _renderPaperMode header pattern) - │ - ├─ [Pass 1 Summary Section] — "## 1. 第一遍：概览" content from deep-reading.md - │ Rendered as formatted callout block - │ - ├─ [AI Discussion History] — "## AI 问答记录" section - │ ├─ discussion.json → rendered as Q&A accordion/cards - │ │ Each session: model badge, timestamp, command, message pairs - │ └─ Empty state: "No AI discussions yet. Use /pf-deep or /pf-paper in OpenCode." - │ - └─ [Navigation] — "← Back to Paper" link - Opens the formal note: this.app.workspace.openLinkText(entry.main_note_path, '') -``` - -### discussion.json Schema (Python → JS contract) - -```json -{ - "format_version": "1", - "zotero_key": "ABCDEFGH", - "sessions": [ - { - "session_id": "2026-05-06T143022", - "timestamp": "2026-05-06T14:30:22+08:00", - "model": "deepseek-v4-pro", - "command": "/pf-deep", - "summary": "三阶段精读：生物力学分析", - "message_count": 12, - "messages": [ - { - "role": "user", - "content": "/pf-deep ABCDEFGH", - "timestamp": "2026-05-06T14:30:22+08:00" - }, - { - "role": "assistant", - "content": "## Pass 1: 概览\n\n...", - "timestamp": "2026-05-06T14:31:05+08:00" - } - ] - } - ] -} -``` - -**Design rationale**: Sessions array allows append-only writes. The `summary` field provides a one-line title for the dashboard card. `message_count` enables quick size display without parsing all messages. Individual messages are kept (not just summary) for potential future expand/collapse UI. - -## Python vs JS Ownership Boundaries - -| Concern | Python Owns | JS Owns | Rationale | -|---------|-------------|---------|-----------| -| Lifecycle computation | ALL (`compute_lifecycle`) | Reads result | Business logic stays in Python | -| Health computation | ALL (`compute_health`) | Reads result | Business logic stays in Python | -| Maturity computation | ALL (`compute_maturity`) | Reads result | Business logic stays in Python | -| Next-step recommendation | ALL (`compute_next_step`) | Reads result | Business logic stays in Python | -| `discussion.json` writing | ALL (agent commands) | Reads only | Agent interaction produces the data | -| `discussion.md` writing | ALL (agent commands) | Reads only | Human-readable companion to JSON | -| Deep-reading content | ALL (`ld_deep.py`, `sync.py`) | Reads via vault API | Parsing/building is Python domain | -| Deep-reading dashboard rendering | None | ALL (CSS + DOM) | Plugin renders, never computes | -| "Jump to" button logic | None | ALL (openLinkText) | Obsidian API operation | -| Version number | Writer (`build_envelope`) | Reader (`_fetchStats`) | Source of truth: `paperforge/__init__.py` | - -**Thin-shell rule preserved**: The JS plugin NEVER computes lifecycle, health, maturity, or next-step. It only reads pre-computed values from `formal-library.json` or directly from workspace files. All business logic remains in Python. - -## Key Data Flow: AI Discussion Recording - -``` -User runs /pf-deep in OpenCode Agent - ↓ -ld_deep.py prepares scaffold, generates deep-reading content - ↓ -Agent conversation complete - ↓ -[NEW: AI Discussion Recorder module] (Python) - ├─ Captures conversation messages - ├─ Writes ai/discussion.md (human-readable markdown log) - └─ Writes ai/discussion.json (structured JSON) - ↓ -User opens deep-reading.md in Obsidian - ↓ -_detectAndSwitch() detects deep-reading.md → 'deep-reading' mode - ↓ -_renderDeepReadingMode() reads: - ├─ formal-library.json → lifecycle/health/maturity badges - ├─ deep-reading.md → Pass 1 summary text - └─ ai/discussion.json → AI Q&A history cards -``` - -## "Jump to Deep Reading" Button — Integration Point - -**Location**: `_renderPaperMode()`, after the next-step card, as an additional contextual action. - -**Condition**: Show when `entry.deep_reading_path` is non-empty AND `entry.deep_reading_status === 'done'`. - -```javascript -// In _renderPaperMode(), after _renderNextStepCard(): -if (entry.deep_reading_path && entry.deep_reading_status === 'done') { - const drBtn = view.createEl('button', { cls: 'paperforge-contextual-btn deep-reading-btn' }); - drBtn.createEl('span', { cls: 'paperforge-contextual-btn-icon', text: '\uD83D\uDD0D' }); - drBtn.createEl('span', { text: 'Jump to Deep Reading' }); - drBtn.addEventListener('click', () => { - this.app.workspace.openLinkText(entry.deep_reading_path, ''); - }); -} -``` - -## Bug Fix Analysis - -### 1. Version Number Not Displaying - -**Root cause**: `_fetchStats()` at line 358 uses `version: this._cachedStats?.version || '\u2014'` but the `formal-library.json` envelope (from `build_envelope()`) has no `version` field — only `schema_version`, `generated_at`, `paper_count`, `items`. - -**Fix strategy**: -- **Python** (`asset_index.py`): Add `paperforge_version` to `build_envelope()`: - ```python - from paperforge import __version__ - return { - "schema_version": CURRENT_SCHEMA_VERSION, - "paperforge_version": __version__, - "generated_at": ..., - "paper_count": len(items), - "items": items, - } - ``` -- **JS** (`main.js`): Read from envelope in `_fetchStats()`: - ```javascript - version: index.paperforge_version || index.version || '\u2014' - ``` - -### 2. Meaningless "ai" Row in Dashboard - -**Root cause**: Unknown — likely a leftover from an earlier Phase 25-26 UI experiment. Need to inspect what creates this row. Based on the state report (`STATE.md` line 61: 'Dashboard bug: "ai" row is meaningless'), this is a rendering artifact in the global dashboard view. - -**Fix strategy**: Audit `_renderGlobalMode()` and `_fetchStats()` for any rendering that produces an "ai" label or row. The canonical index entry has `ai_path` as a path string, and the lifecycle includes `ai_context_ready`. If any UI component renders raw field names, filter it out or map to a human label. - -## Architectural Patterns Used - -### Pattern 1: Mode Dispatch (existing, extended) - -**What**: `_detectAndSwitch()` → `_switchMode()` → `case 'mode': renderX()` -**When**: Adding any new dashboard context -**v1.8 extension**: Add `case 'deep-reading': this._renderDeepReadingMode()` to the switch -**Trade-offs**: Linear dispatch is simple but the switch statement grows. Acceptable for 4 modes. - -### Pattern 2: File-based Index as Bridge - -**What**: `formal-library.json` is the contract between Python and JS. Python writes derived fields (lifecycle, health, maturity, next_step). JS reads and renders. -**When**: Any time JS needs to display computed state -**v1.8 use**: Deep-reading dashboard reads lifecycle/health/maturity from index, only reads `discussion.json` directly for AI-specific data. -**Trade-offs**: Adds a file dependency but eliminates cross-runtime communication complexity. - -### Pattern 3: ReadFileSync Data Loading - -**What**: JS reads JSON files with `fs.readFileSync(indexPath, 'utf-8')` + caching via `_cachedItems` -**When**: All dashboard data loading -**v1.8 extension**: Same pattern for `discussion.json`: `fs.readFileSync(discussionPath, 'utf-8')` from `ai/` directory. -**Trade-offs**: Requires vault-relative path resolution. OK because `this.app.vault.adapter.basePath` provides vault root. - -## Anti-Patterns to Avoid - -### Anti-Pattern 1: Business Logic in JS - -**What people do**: Re-implement lifecycle or health computation in JavaScript -**Why it's wrong**: Creates configuration drift between Python and JS, violates thin-shell constraint -**Do this instead**: Only read pre-computed values from the canonical index. The JS plugin is a view layer only. - -### Anti-Pattern 2: Two Frontmatter Sources of Truth - -**What people do**: Read deep-reading status from frontmatter AND from the index, compare them -**Why it's wrong**: Creates reconciliation bugs. The index IS the canonical source. -**Do this instead**: Deep-reading dashboard gets all status fields from `_findEntry(key)` which reads from `_getCachedIndex()`. - -### Anti-Pattern 3: Filename-based heuristic for mode detection - -**What people do**: Check if filename === "deep-reading.md" without verifying workspace context -**Why it's wrong**: A user could have any file named "deep-reading.md" anywhere in their vault -**Do this instead**: Verify parent directory matches workspace pattern `{8-char key} - {title_slug}` and the file is inside `Literature/{domain}/` - -## Recommended Build Order - -Based on dependency analysis, the suggested build order for v1.8 phases: - -1. **Phase 31a: Fix version number display** — 1 change Python (build_envelope), 1 change JS (_fetchStats). Low risk, no dependencies. - -2. **Phase 31b: Fix "ai" row bug** — Audit and remove. Depends on understanding current rendering, which we already have. - -3. **Phase 32: Add deep-reading mode detection** — Extend `_detectAndSwitch()` and `_switchMode()`. Pure JS change. No dependency on Python. - -4. **Phase 33: Build _renderDeepReadingMode() component** — New render method with status bar, Pass 1 summary, and placeholder for AI history. Depends on Phase 32. - -5. **Phase 34: Add "Jump to Deep Reading" button** — Modify `_renderPaperMode()`. Depends on Phase 33 (need consistent deep-reading path resolution). - -6. **Phase 35: AI discussion recorder (Python)** — Write `discussion.md` + `discussion.json` in `ai/`. Depends on nothing (standalone Python module). - -7. **Phase 36: Wire AI Q&A history into deep-reading dashboard** — Read `discussion.json` in `_renderDeepReadingMode()`. Depends on Phase 33 and Phase 35. - -**Rationale**: Fixes first (low risk, quick wins), then detection infrastructure, then rendering, finally data pipeline. - -## Scaling Considerations - -| Scale | Architecture Note | -|-------|-------------------| -| ~10 papers | All modes fast. Direct file reads adequate. | -| ~100 papers | Index scan for `_findEntry()` is O(n). Acceptable for this scale. | -| ~1000 papers | `_findEntry()` is still linear scan of items array. Consider Map-based lookup if performance becomes issue. | -| ~10K papers | `_getCachedIndex()` loads full JSON into memory (potentially 10+ MB). Consider paginated index or lazy loading. | - -*For v1.8: Current scale is ~100 papers. Existing linear scan is fine. No premature optimization needed.* - -## Integration Points Summary - -| Point | From | To | Data | Trigger | -|-------|------|----|------|---------| -| Index reading | `formal-library.json` | `_getCachedIndex()` | All entry fields | On mode switch, on modify event | -| Deep-reading content | `deep-reading.md` | `_renderDeepReadingMode()` | Markdown text | On deep-reading mode entry | -| AI discussion history | `ai/discussion.json` | `_renderDeepReadingMode()` | Session array | On deep-reading mode entry | -| AI discussion writing | Agent command (/pf-deep, /pf-paper) | `ai/discussion.*` | Full conversation | After agent interaction complete | -| Version number | `build_envelope()` | `_fetchStats()` | `paperforge_version` string | On index rebuild | -| Jump to deep-reading | `_renderPaperMode()` button | `app.workspace.openLinkText()` | `deep_reading_path` | User click | - -## Sources - -- **Verified against source**: `paperforge/plugin/main.js` (lines 1-2067) — existing mode detection, switch, and rendering -- **Verified against source**: `paperforge/worker/asset_index.py` (lines 1-577) — `_build_entry()`, `build_envelope()`, `read_index()` -- **Verified against source**: `paperforge/worker/asset_state.py` (lines 1-243) — lifecycle, health, maturity, next-step computation -- **Verified against source**: `paperforge/worker/sync.py` (lines 1677-1749) — `migrate_to_workspace()`, ai/ directory creation -- **Project context**: `.planning/PROJECT.md` — v1.8 requirements and thin-shell constraint -- **Project state**: `.planning/STATE.md` — bug reports (ai row, version number) +**Project:** PaperForge v2.0 +**Researched:** 2026-05-08 +**Mode:** Architecture (project research) +**Confidence:** HIGH --- -*Architecture research for: PaperForge v1.8 AI Discussion & Deep-Reading Dashboard* -*Researched: 2026-05-06* -*Confidence: HIGH — all integration points verified against existing source code* +## 1. Architecture Overview + +### 1.1 The 6 Testing Layers + +The testing architecture follows a **modified testing diamond** shape: unit tests form the broad base, integration/contract tests form the bulk (where most value is found), and full-system/E2E tests cap the top. An additional "Level 0" version-check layer runs **before** all other layers as a fast health gate. + +``` ++---------------------------+ +| Level 6: Chaos/Destructive| Rare, manual or scheduled ++---------------------------+ +| Level 5: User Journey | Contract-driven, run per-release ++---------------------------+ +| Level 4: Temp Vault E2E | Full pipeline, mock-backed ++---------------------------+ +| Level 3: Plugin Runtime | Vitest + obsidian-test-mocks ++---------------------------+ +| Level 2: CLI Contracts | JSON schema stability ++---------------------------+ +| Level 1: Python Units | Fast, isolated, 1-10ms each ++---------------------------+ +| Level 0: Version Sync | <100ms pre-flight gate ++---------------------------+ +``` + +**Layer dependency chain (build order):** + +``` +Level 0 (version check) + | + v +Level 1 (unit tests) ----+ + | | + v v +Level 2 (CLI contracts) Level 3 (plugin unit) + | | + +----------+----------+ + | + v + Level 4 (temp vault E2E) + | + v + Level 5 (user journey) + | + v + Level 6 (chaos) +``` + +### 1.2 Integration Points with Existing Codebase + +| Component | Integration | New vs Modified | +|-----------|-------------|-----------------| +| `tests/conftest.py` | Add shared fixtures for mock vault, mock OCR, golden dataset paths. Existing `create_test_vault()` moved to `fixtures/vault_builder.py` | **Modified** — extend, not rewrite | +| `tests/test_*.py` (flat) | Migrated into `tests/unit/` with no behavior change | **Modified** — relocate only | +| `tests/sandbox/` | Retained as-is for backward compat; new golden datasets go to `fixtures/` | **Unchanged** | +| `tests/fixtures/` (existing .json, .pdf) | Expanded into full `fixtures/` hierarchy | **Modified** — add structure | +| `pyproject.toml` `[tool.pytest.ini_options]` | Add markers, ignore patterns for new layers | **Modified** | +| `paperforge/plugin/*` | New `tests/plugin/` tests import via vitest/Jest; no changes to plugin source | **New** | +| `.github/` (new) | New CI workflows for matrix builds | **New** | +| `scripts/` | New `scripts/check_version_sync.py`, `scripts/run_chaos.sh` | **New** | +| `paperforge/cli.py` | CLI contract tests test `main()` return codes and JSON output | **No changes needed** | +| `paperforge/worker/ocr.py` | Mock OCR backend intercepts HTTP calls at `requests` layer | **Test-only, no source changes** | +| `paperforge/commands/` | Unit tests import command modules directly | **No changes needed** | +| `tests/sandbox/00_TestVault/` | Still gitignored; temp vault E2E creates fresh dirs in system temp | **Unchanged** | + +--- + +## 2. Directory Structure + +### 2.1 Complete Directory Tree + +``` +/ +├── .github/ +│ └── workflows/ +│ ├── ci.yml # Main CI: all gates except chaos +│ ├── ci-chaos.yml # Chaos tests (scheduled or manual) +│ └── ci-pr-checks.yml # Fast pre-flight (L0+L1) +│ +├── tests/ +│ ├── __init__.py # (empty) marks as package +│ ├── conftest.py # Root fixtures: mock vault, golden paths, pytest plugins +│ │ +│ ├── unit/ # LEVEL 1: Python unit tests +│ │ ├── __init__.py +│ │ ├── conftest.py # Overrides/extends root conftest for unit scope +│ │ ├── test_config.py # Existing, relocated +│ │ ├── test_bbt_parser.py # New +│ │ ├── test_pdf_resolver.py # Existing, relocated +│ │ ├── test_ocr_state_machine.py # Existing, relocated +│ │ ├── test_asset_index.py # Existing, relocated +│ │ ├── test_asset_state.py # Existing, relocated +│ │ ├── test_discussion.py # Existing, relocated +│ │ ├── test_repair.py # Existing, relocated +│ │ ├── test_utils_slugify.py # Existing, relocated +│ │ ├── test_utils_yaml.py # Existing, relocated +│ │ ├── test_utils_json.py # Existing, relocated +│ │ ├── test_utils_journal.py # Existing, relocated +│ │ ├── test_path_normalization.py# Existing, relocated +│ │ ├── test_setup_wizard.py # Existing, relocated +│ │ ├── test_doctor.py # Existing, relocated +│ │ ├── test_ocr_preflight.py # Existing, relocated +│ │ ├── test_ocr_doctor.py # Existing, relocated +│ │ ├── test_ocr_classify.py # Existing, relocated +│ │ ├── test_ocr_rendering.py # Existing, relocated +│ │ ├── test_ld_deep_config.py # Existing, relocated +│ │ ├── test_ld_deep_skel.py # Existing, relocated +│ │ ├── test_ld_deep_postprocess.py # Existing, relocated +│ │ ├── test_prepare_rollback.py # Existing, relocated +│ │ ├── test_base_views.py # Existing, relocated +│ │ ├── test_base_preservation.py # Existing, relocated +│ │ ├── test_context.py # Existing, relocated +│ │ ├── test_legacy_worker_compat.py # Existing, relocated +│ │ └── test_migration.py # Existing, relocated +│ │ +│ ├── cli/ # LEVEL 2: CLI contract tests +│ │ ├── __init__.py +│ │ ├── conftest.py # CLI-specific fixtures (subprocess runner) +│ │ ├── test_paths_json.py # Existing test_cli_paths.py +│ │ ├── test_sync_cli.py # CLI sync --dry-run contract +│ │ ├── test_status_json.py # --json output schema +│ │ ├── test_doctor_cli.py # CLI doctor verdict contract +│ │ ├── test_deep_reading_cli.py # CLI deep-reading output +│ │ ├── test_ocr_cli.py # CLI ocr --diagnose contract +│ │ ├── test_repair_cli.py # CLI repair dry-run contract +│ │ └── test_error_codes.py # Exit code contract +│ │ +│ ├── plugin/ # LEVEL 3: Plugin runtime tests +│ │ ├── __init__.py +│ │ ├── conftest.py # Vitest config, obsidian mocks setup +│ │ ├── vitest.config.ts # Vitest configuration +│ │ ├── test_main.js # Plugin instantiation +│ │ ├── test_settings.js # Settings tab rendering +│ │ ├── test_dashboard.js # Dashboard view logic +│ │ ├── test_subprocess.js # CLI subprocess dispatch +│ │ ├── test_i18n.js # i18n key coverage +│ │ ├── test_base_rendering.js # Base view rendering +│ │ └── fixtures/ # Plugin test fixtures +│ │ ├── mock_vault_config.json +│ │ └── mock_index.json +│ │ +│ ├── e2e/ # LEVEL 4: Temp vault E2E tests +│ │ ├── __init__.py +│ │ ├── conftest.py # Temp vault lifecycle +│ │ ├── test_full_sync_ocr_pipeline.py # sync -> ocr -> status +│ │ ├── test_repair_workflow.py # Create state divergence, run repair +│ │ ├── test_setup_headless.py # Headless setup on temp vault +│ │ ├── test_migration_from_v1.py # Legacy config migration +│ │ ├── test_doctor_e2e.py # doctor on fully configured vault +│ │ └── test_multi_domain.py # Multi-domain BBT export sync +│ │ +│ ├── journey/ # LEVEL 5: User journey tests +│ │ ├── __init__.py +│ │ ├── conftest.py # Journey-level fixtures +│ │ ├── UX_CONTRACT.md # The UX contract document +│ │ ├── test_new_user_onboarding.py # First-time user flow +│ │ ├── test_daily_workflow.py # Regular user sync + OCR + read +│ │ ├── test_upgrade_migration.py # Existing user upgrading +│ │ └── test_error_recovery.py # User handles failed OCR, broken paths +│ │ +│ ├── chaos/ # LEVEL 6: Destructive tests +│ │ ├── __init__.py +│ │ ├── conftest.py # Chaos-specific fixtures +│ │ ├── scenarios/ +│ │ │ ├── test_corrupt_json_exports.py # Malformed BBT JSON +│ │ │ ├── test_missing_pdf_files.py # PDF referenced but missing +│ │ │ ├── test_ocr_api_timeout.py # PaddleOCR API hangs +│ │ │ ├── test_zotero_junction_broken.py # Symlink broken +│ │ │ ├── test_disk_full_simulation.py # No space for OCR output +│ │ │ ├── test_concurrent_sync_calls.py # Two syncs at once +│ │ │ ├── test_interrupted_ocr.py # Kill mid-OCR, resume +│ │ │ └── test_nonexistent_vault_path.py # Cwd is not a vault +│ │ └── CHAOS_MATRIX.md # Documentation of chaos scenarios +│ │ +│ └── sandbox/ # Existing — NOT relocated +│ ├── exports/ +│ ├── ocr-complete/ +│ ├── TestZoteroData/ +│ ├── generate_sandbox.py +│ ├── generate_ocr_fixture.py +│ ├── batch_check.py +│ ├── deep_inspect.py +│ └── precise_batch.py +│ +├── fixtures/ # Golden datasets (shared across layers) +│ ├── __init__.py +│ ├── README.md # Fixture documentation +│ │ +│ ├── zotero/ # Simulated Better BibTeX JSON exports +│ │ ├── orthopedic.json # Domain: 骨科 +│ │ ├── sports_medicine.json # Domain: 运动医学 +│ │ ├── multi_attachment.json # Paper with 3 PDF attachments +│ │ ├── no_pdf.json # Paper without PDF +│ │ ├── absolute_paths.json # Windows absolute path format +│ │ ├── storage_prefix.json # storage: prefix format +│ │ ├── bare_relative.json # Bare KEY/file.pdf format +│ │ └── empty.json # Empty export (edge case) +│ │ +│ ├── pdf/ # Minimal valid PDFs +│ │ ├── blank.pdf # 1-page blank PDF +│ │ ├── two_page.pdf # 2-page PDF (for multi-page OCR tests) +│ │ ├── with_figures.pdf # PDF containing embedded images +│ │ └── large.pdf # >10MB PDF (for timeout tests) +│ │ +│ ├── snapshots/ # Expected outputs for snapshot testing +│ │ ├── status_json/ +│ │ │ └── minimal_vault.json # Expected status --json output +│ │ ├── paths_json/ +│ │ │ └── default_config.json # Expected paths --json output +│ │ ├── formal_note_frontmatter/ +│ │ │ └── orthopedic_article.yaml # Expected YAML frontmatter +│ │ └── index_json/ +│ │ └── after_sync.json # Expected index structure +│ │ +│ └── ocr/ # Mock OCR response fixtures +│ ├── paddleocr_success.json # Standard PaddleOCR API success response +│ ├── paddleocr_pending.json # Async job created, not yet done +│ ├── paddleocr_failed.json # API returned error +│ ├── paddleocr_timeout.json # Job never completes +│ ├── extracted_fulltext.md # Expected OCR output (fulltext.md) +│ └── figure_map.json # Expected figure-map.json output +│ +├── scripts/ +│ ├── check_version_sync.py # LEVEL 0: version consistency checker +│ ├── run_all_tests.sh # Shell wrapper: L0 -> L1 -> L2 -> L3 -> L4 +│ ├── run_chaos_tests.sh # Shell wrapper for chaos layer +│ └── generate_fixtures.py # Regenerate golden datasets +│ +└── VERSION_SYNC.md # Documentation of version sync protocol +``` + +### 2.2 Migration Path for Existing Tests + +Existing flat `tests/test_*.py` files are **relocated** (not rewritten) into `tests/unit/`. This is a pure directory move: + +| Existing Path | New Path | +|---------------|----------| +| `tests/test_config.py` | `tests/unit/test_config.py` | +| `tests/test_ocr_state_machine.py` | `tests/unit/test_ocr_state_machine.py` | +| `tests/test_asset_index.py` | `tests/unit/test_asset_index.py` | +| `tests/test_e2e_pipeline.py` | `tests/e2e/test_full_sync_ocr_pipeline.py` (renamed) | +| `tests/test_e2e_cli.py` | `tests/cli/test_sync_cli.py` (split across cli/) | +| `tests/test_smoke.py` | `tests/unit/test_smoke.py` (retained as-is) | +| `tests/test_plugin_install_bootstrap.py` | `tests/plugin/test_main.js` (rewritten as JS) | +| `tests/conftest.py` | Keep in root, add imports from `fixtures/` | + +**Existing `tests/sandbox/` directory is RETAINED in place** — it is used for manual debugging and backward compat but is NOT part of the automated pipeline. New golden datasets go to `fixtures/`. + +--- + +## 3. Mock System Architecture + +### 3.1 Mock Systems Overview + +``` + +--------------+ + | Test Runner | + +------+-------+ + | + +----------------+----------------+ + | | | + v v v ++---------+---+ +--------+-------+ +------+-------+ +| Mock OCR | | Mock Zotero | | Mock Vault | +| Backend | | Data Source | | Filesystem | ++-------------+ +---------------+ +--------------+ +| Intercepts | | Static JSON | | tmp_path | +| requests to | | fixtures from | | + created | +| PaddleOCR | | fixtures/ | | dirs & files | +| API at HTTP | | zotero/ | | + paperforge.| +| layer | | | | json | ++--------------+ +---------------+ +--------------+ +``` + +### 3.2 Mock OCR Backend + +**Purpose:** Intercept HTTP calls to the PaddleOCR API so tests never touch the real network. + +**Approach:** `responses` library (or `pytest-httpx`) for Python-level HTTP interception + fixture JSON responses. + +``` +tests/ +├── conftest.py (root) -- registers mock_ocr_backend fixture +├── unit/ +│ └── conftest.py -- imports mock_ocr_backend, applies auto-use +└── fixtures/ + └── ocr/ -- PaddleOCR API response fixtures + ├── paddleocr_success.json + ├── paddleocr_pending.json + └── paddleocr_failed.json +``` + +**Architecture:** + +```python +# tests/conftest.py (conceptual) + +@pytest.fixture +def mock_ocr_backend(): + """Intercept HTTP calls to PaddleOCR API and return fixture responses.""" + import responses + with responses.RequestsMock() as rsps: + # Job submission endpoint + rsps.post( + "https://paddleocr.aistudio-app.com/api/v2/ocr/jobs", + json={"job_id": "mock-job-001", "status": "queued"}, + status=202, + ) + # Job status polling endpoint + rsps.get( + re.compile(r"https://paddleocr\.aistudio-app\.com/api/v2/ocr/jobs/mock-job-\d+"), + json={"status": "completed", "result_url": "https://mock/results"}, + status=200, + ) + # Result download + rsps.get( + "https://mock/results", + json=MOCK_OCR_RESULT, # Loaded from fixtures/ocr/paddleocr_success.json + status=200, + ) + yield rsps + + +@pytest.fixture +def mock_ocr_backend_timeout(): + """OCR API that always returns 'pending' (simulates timeout).""" + # ... similar pattern returning 202 forever +``` + +**Fixture data flow:** + +``` +fixtures/ocr/paddleocr_success.json + --> mock_ocr_backend fixture + --> ocr.py's requests.post() / requests.get() + --> run_ocr() processes result + --> test asserts OCR output files exist +``` + +**Layer applicability:** + +| Layer | Uses Mock OCR? | How | +|-------|----------------|-----| +| L1 Unit | YES | Via `mock_ocr_backend` fixture auto-use in `tests/unit/conftest.py` | +| L2 CLI | YES | Via `mock_ocr_backend` in CLI conftest | +| L3 Plugin | N/A | Plugin doesn't call OCR directly | +| L4 E2E | **NO** | Uses real file-based OCR fixture (`tests/sandbox/ocr-complete/`) | +| L5 Journey | **NO** | Full-system test with real (or realistically simulated) OCR | +| L6 Chaos | YES | `mock_ocr_backend_timeout` + corrupt responses | + +### 3.3 Mock Zotero Data Source + +**Purpose:** Provide deterministic BBT JSON exports without requiring a real Zotero installation. + +**Approach:** Static JSON files in `fixtures/zotero/` that mirror the three supported BBT path formats. + +``` +fixtures/zotero/ +├── orthopedic.json # Single-domain, single paper +├── sports_medicine.json # Second domain +├── multi_attachment.json # 1 main PDF + 2 supplementary +├── no_pdf.json # Entry without attachments +├── absolute_paths.json # Windows D:\Zotero\storage\KEY\file.pdf +├── storage_prefix.json # storage:KEY/file.pdf +├── bare_relative.json # KEY/file.pdf +└── empty.json # [] — edge case +``` + +**Fixture reader:** + +```python +# tests/conftest.py (conceptual) + +import json +from pathlib import Path + +FIXTURES_DIR = Path(__file__).parent.parent / "fixtures" + +@pytest.fixture +def zotero_export(request: pytest.FixtureRequest) -> list[dict]: + """Load a named BBT JSON fixture.""" + fixture_name = getattr(request, "param", "orthopedic.json") + path = FIXTURES_DIR / "zotero" / fixture_name + return json.loads(path.read_text(encoding="utf-8")) +``` + +### 3.4 Mock Vault Filesystem + +**Purpose:** Create a deterministic, isolated vault filesystem for each test layer. + +**Three implementations, one interface:** + +| Layer | Implementation | Speed | Realism | +|-------|---------------|-------|---------| +| L1 Unit | `tmp_path` + minimal dirs | Fastest | Low | +| L2 CLI | `tmp_path` + full vault with `paperforge.json` | Fast | Medium | +| L4 E2E | `tmp_path` + full vault + mock Zotero storage + OCR fixtures | Medium | High | +| L6 Chaos | `tmp_path` + deliberately broken vault | Fast | Low (by design) | + +**Unified interface via `conftest.py`:** + +```python +# tests/conftest.py (conceptual) + +@pytest.fixture +def vault_builder(): + """Factory fixture: returns a function that builds vaults at different completeness levels.""" + from fixtures.vault_builder import VaultBuilder + return VaultBuilder(fixtures_root=FIXTURES_DIR) + +# Usage in tests: +# vault_builder("minimal") -> tmp_path + paperforge.json only +# vault_builder("standard") -> tmp_path + full dirs + blank.pdf +# vault_builder("full") -> tmp_path + full dirs + exports + OCR fixtures + mock PDFs +``` + +--- + +## 4. Layer-by-Layer Architecture + +### 4.1 Level 0: Version Build Consistency + +**Purpose:** Ensure all version declarations agree before any tests run. + +**Location:** `scripts/check_version_sync.py` + +**What it checks:** + +| File | Field | Check | +|------|-------|-------| +| `paperforge/__init__.py` | `__version__` | Authoritative source | +| `paperforge/plugin/manifest.json` | `version` | Must match `__version__` | +| `paperforge/plugin/versions.json` | All versions | Each must match `__version__` | +| `pyproject.toml` (dynamic) | `version` attr | Must point to `paperforge.__version__` | +| `paperforge.json` (DEFAULT_CONFIG) | `version` | Must match (if present) | +| `VERSION_SYNC.md` | Documented version | Must match (consistency audit) | +| `CHANGELOG.md` | Latest release | Must correspond to current version | + +**Exit codes:** 0 = all sync, 1 = mismatch found. + +**Execution:** Called first in CI `ci-pr-checks.yml` before any other test job. + +``` +ci-pr-checks.yml: + check-version-sync: + runs-on: ubuntu-latest + steps: + - python scripts/check_version_sync.py +``` + +**Integration:** This is the ONLY layer that can fail independently without running others. If it fails, all subsequent test jobs are skipped. + +### 4.2 Level 1: Python Unit Tests + +**Purpose:** Fast, isolated tests for individual functions/modules. + +**Location:** `tests/unit/` + +**Characteristics:** +- No external dependencies (network, filesystem beyond `tmp_path`) +- No subprocess calls +- Mock everything outside the module under test +- Target: 1-10ms per test, <30s for the full suite +- Coverage target: >85% for `paperforge/*.py`, >75% for `paperforge/worker/*.py` + +**Fixture layering:** + +``` +tests/conftest.py (root) + |-- vault_builder factory fixture + |-- zotero_export fixture + |-- mock_ocr_backend fixture + | + tests/unit/conftest.py + |-- auto-use: mock_ocr_backend (for OCR unit tests) + |-- auto-use: set PAPERFORGE_POLL_INTERVAL=0 +``` + +**Test patterns per module:** + +| Module | Test File | Pattern | +|--------|-----------|---------| +| `paperforge/config.py` | `test_config.py` | Pure function inputs/outputs, env var overrides | +| `paperforge/pdf_resolver.py` | `test_pdf_resolver.py` | Path resolution with fixture paths | +| `paperforge/worker/ocr.py` | `test_ocr_state_machine.py` | Mock HTTP, assert state transitions | +| `paperforge/worker/_utils.py` | `test_utils_*.py` | Pure function tests | +| `paperforge/worker/sync.py` | Migrated from `test_e2e_pipeline.py` | Mock file I/O | +| `paperforge/worker/repair.py` | `test_repair.py` | Create divergence scenarios, test detection | +| `paperforge/worker/asset_index.py` | `test_asset_index.py` | Build index from known data, assert structure | +| `paperforge/setup_wizard.py` | `test_setup_wizard.py` | Test headless flow with tmp_path vault | + +### 4.3 Level 2: CLI Contract Tests + +**Purpose:** Validate CLI command output schemas, exit codes, and JSON output stability. + +**Location:** `tests/cli/` + +**Characteristics:** +- Run `paperforge ` via subprocess (`sys.executable -m paperforge`) +- Assert exit codes, stdout JSON schemas, stderr patterns +- No external network (CLI commands that hit network are mocked at Python level) +- Not a full E2E test — tests the **CLI boundary only** + +**Pattern:** + +```python +# tests/cli/conftest.py (conceptual) + +import subprocess +import sys +from pathlib import Path + +import pytest + + +@pytest.fixture +def cli_invoker(vault_builder): + """Returns a function that runs paperforge CLI in a temp vault.""" + vault = vault_builder("minimal") + + def _invoke(args: list[str]) -> subprocess.CompletedProcess: + return subprocess.run( + [sys.executable, "-m", "paperforge", "--vault", str(vault)] + args, + capture_output=True, + text=True, + timeout=30, + ) + + return _invoke +``` + +**Contract tests for each command:** + +| Command | Contract Check | +|---------|---------------| +| `paths --json` | Output is valid JSON with exactly `{vault, worker_script, ld_deep_script}` keys | +| `status --json` | Output is valid JSON with `{total_papers, version, ...}` | +| `sync --dry-run` | Exit 0, prints summary without modifying filesystem | +| `doctor` | Output contains `[OK]`, `[WARN]`, or `[FAIL]` verdict prefix | +| `deep-reading` | Tabular output or valid JSON | +| `ocr --diagnose` | Diagnoses without crashing, exit 0 or 1 | +| `repair` (no args) | Dry-run output, exit 0 | +| `repair --fix` | Actual repair on mock divergence | +| Unrecognized command | Exit 1, prints error to stderr | +| Missing vault | Exit 1, does not crash with traceback | + +**Snapshot testing with `pytest-snapshot`:** + +```python +# Conceptual snapshot usage +def test_paths_json_matches_snapshot(cli_invoker, snapshot): + result = cli_invoker(["paths", "--json"]) + assert result.returncode == 0 + snapshot.assert_match(result.stdout, "paths_default_config.json") +``` + +Snapshots stored at `fixtures/snapshots/paths_json/default_config.json`. + +### 4.4 Level 3: Plugin Runtime Tests + +**Purpose:** Test the Obsidian plugin JS code (`paperforge/plugin/main.js`) without a running Obsidian instance. + +**Location:** `tests/plugin/` + +**Technology stack:** +- **Vitest** (preferred over Jest for ESM compatibility and speed) +- **`obsidian-test-mocks`** package for Obsidian API mocks +- **`jsdom`** environment for DOM APIs + +**Directory layout:** + +``` +tests/plugin/ +├── vitest.config.ts # Vitest configuration +├── conftest.py # (unused — Vitest loads TS/JS config) +├── __mocks__/ +│ └── obsidian.ts # Manual mock if needed (obsidian-test-mocks preferred) +├── test_main.js # Plugin lifecycle, onload(), onunload() +├── test_settings.js # Settings tab rendering and persistence +├── test_dashboard.js # Dashboard view component tests +├── test_subprocess.js # CLI subprocess dispatch from plugin +├── test_i18n.js # i18n key coverage (all keys in zh and en) +├── test_base_rendering.js # Base view rendering +└── fixtures/ + ├── mock_vault_config.json # Simulated paperforge.json + └── mock_index.json # Simulated canonical index +``` + +**Key test patterns:** + +```javascript +// test_main.js (conceptual) +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { App } from 'obsidian-test-mocks/obsidian'; +import { MyPlugin } from '../../paperforge/plugin/main.js'; + +describe('Plugin lifecycle', () => { + let app; + let plugin; + + beforeEach(() => { + app = new App(); + plugin = new MyPlugin(app, { id: 'paperforge' }); + }); + + it('should load without error', async () => { + await expect(plugin.onload()).resolves.not.toThrow(); + }); + + it('should register views', async () => { + await plugin.onload(); + expect(app.views.get('paperforge-status')).toBeDefined(); + }); +}); +``` + +**What is NOT tested at this layer:** +- Actual subprocess execution of the Python CLI (that's L2) +- Actual Obsidian rendering (that's L4/L5) +- File watchers and real-time updates + +**Why Vitest over Jest:** + +| Factor | Vitest | Jest | +|--------|--------|------| +| ESM support | Native | Requires transforms | +| Speed | ~2x faster on watch mode | Slower | +| `obsidian-test-mocks` support | First-class | Compatible via config | +| TypeScript | Native | Via ts-jest | +| File parallelism control | Simple config | More complex | + +### 4.5 Level 4: Temp Vault E2E Tests + +**Purpose:** Test the full Python pipeline (sync -> OCR -> status -> repair) in a freshly created temporary vault. + +**Location:** `tests/e2e/` + +**Characteristics:** +- Creates a complete vault in OS temp directory (via `tmp_path`) +- Copies golden datasets into the vault +- Calls Python worker functions directly (not via subprocess) +- Workflow: sync -> verify index -> verify formal notes -> (mock) OCR -> verify OCR results -> verify status +- No network access (mock OCR intercept, static Zotero exports from fixtures) + +**Fixture architecture:** + +```python +# tests/e2e/conftest.py (conceptual) + +import shutil +from pathlib import Path + +import pytest + +from paperforge.config import paperforge_paths + +FIXTURES_DIR = Path(__file__).parent.parent.parent / "fixtures" + + +@pytest.fixture +def e2e_vault(tmp_path: Path) -> Path: + """Create a fully-populated temp vault for E2E tests.""" + vault = tmp_path / "test_vault" + vault.mkdir() + + # 1. Create paperforge.json + # 2. Create directory structure (System, PaperForge, Resources, Literature, etc.) + # 3. Copy Zotero export fixtures into PaperForge/exports/ + # 4. Copy mock PDFs into Zotero/storage/ + # 5. Copy OCR fixture into PaperForge/ocr/ + # 6. Create .env with mock PADDLEOCR_API_TOKEN + # 7. Return vault path + ... + + return vault + + +@pytest.fixture +def run_sync(e2e_vault: Path): + """Run full sync pipeline and return paths dict.""" + from paperforge.worker.sync import run_selection_sync, run_index_refresh + run_selection_sync(e2e_vault) + run_index_refresh(e2e_vault) + return paperforge_paths(e2e_vault) +``` + +**Workflow test example:** + +```python +# tests/e2e/test_full_sync_ocr_pipeline.py (conceptual) + +class TestFullPipeline: + """sync -> index -> formal notes -> OCR -> status.""" + + def test_sync_creates_workspace_notes(self, run_sync, e2e_vault): + """After sync, workspace notes exist in Literature//.""" + paths = run_sync + lit_dir = paths["literature"] + ws_notes = list(lit_dir.rglob("*.md")) + assert len(ws_notes) > 0 + assert all("zotero_key:" in n.read_text() for n in ws_notes) + + def test_ocr_completes_on_do_ocr_true(self, e2e_vault, mock_ocr_backend): + """Papers with do_ocr: true get OCR results.""" + # 1. Run sync + # 2. Set do_ocr=true in formal note frontmatter + # 3. Run OCR + # 4. Assert meta.json exists with ocr_status: done + + def test_status_reports_correct_counts(self, e2e_vault, run_sync): + """status output matches actual vault contents.""" + from paperforge.worker.status import run_status + result = run_status(e2e_vault) + assert result["total_papers"] >= 1 +``` + +### 4.6 Level 5: User Journey Tests + +**Purpose:** Validate complete user-facing workflows against a documented UX contract. + +**Location:** `tests/journey/` + +**UX Contract:** `tests/journey/UX_CONTRACT.md` documents the complete set of user journeys the product supports. Each journey maps to one test file. + +**Journeys covered:** + +| Journey | Test File | Description | +|---------|-----------|-------------| +| New user onboard | `test_new_user_onboarding.py` | Install -> setup wizard -> sync -> OCR -> deep read | +| Daily workflow | `test_daily_workflow.py` | Open Obsidian -> open dashboard -> sync -> check status -> close | +| Upgrade migration | `test_upgrade_migration.py` | User upgrades from v1.x -> migration runs -> everything works | +| Error recovery | `test_error_recovery.py` | User sees error -> runs doctor -> runs repair -> back to normal | + +**Architecture:** + +```python +# tests/journey/conftest.py (conceptual) + +@pytest.fixture +def full_vault(tmp_path_factory) -> Path: + """Session-scoped vault used for ALL journey tests (expensive setup, share it).""" + vault = tmp_path_factory.mktemp("journey_vault") + # Full setup: create vault, install paperforge.json, copy golden datasets, + # run headless setup, run sync, run OCR, create deep-reading artifacts + ... + return vault +``` + +Note: Journey tests use `tmp_path_factory` (session scope) rather than `tmp_path` (function scope) because the setup is expensive. Tests within a journey file must NOT mutate shared state; each test creates its own paper copy within the vault. + +### 4.7 Level 6: Chaos / Destructive Tests + +**Purpose:** Ensure the system handles abnormal, malicious, or catastrophic inputs gracefully. + +**Location:** `tests/chaos/scenarios/` + +**CHAOS_MATRIX.md:** Documents all chaos scenarios, their trigger conditions, expected behavior, and recovery paths. + +**Scenario categories:** + +| Category | Scenario | Trigger | Expected Behavior | +|----------|----------|---------|-------------------| +| Corrupt input | `test_corrupt_json_exports.py` | Malformed JSON in exports/ | Graceful error, no crash, actionable error message | +| Missing resources | `test_missing_pdf_files.py` | PDF path in formal note but file missing | pdf_path wikilink is broken, status reports error, repair detects it | +| API failures | `test_ocr_api_timeout.py` | PaddleOCR never returns completion | OCR worker retries, eventually fails with clear message | +| Filesystem errors | `test_zotero_junction_broken.py` | Zotero junction/symlink broken | doctor detects, repair offers fix | +| Resource exhaustion | `test_disk_full_simulation.py` | No space for OCR output | OCR worker fails gracefully, existing data intact | +| Concurrency | `test_concurrent_sync_calls.py` | Two sync processes at same time | FileLock prevents corruption, second process waits or errors | +| Interruption | `test_interrupted_ocr.py` | Kill OCR mid-job, restart | OCR resumes from last known state, no duplicate work | +| Invalid config | `test_nonexistent_vault_path.py` | --vault points to nothing | Exit 1, clear "path not found" message | + +**Chaos fixtures pattern:** + +```python +# tests/chaos/conftest.py (conceptual) + +@pytest.fixture +def corrupted_export(tmp_path) -> Path: + """A directory with a corrupted BBT export.""" + exports_dir = tmp_path / "exports" + exports_dir.mkdir(parents=True) + (exports_dir / "orthopedic.json").write_text( + "this is not valid json {{{", encoding="utf-8" + ) + return exports_dir + + +@pytest.fixture +def vault_with_broken_junction(tmp_path) -> Path: + """A vault where the Zotero junction target doesn't exist.""" + vault = _create_base_vault(tmp_path) + # Create the junction path but point it to nowhere + zotero_link = vault / "99_System" / "Zotero" + zotero_link.mkdir(parents=True, exist_ok=True) + # Simulate broken junction by having an empty dir instead of link + return vault +``` + +--- + +## 5. CI Matrix Architecture + +### 5.1 Workflow File Structure + +``` +.github/workflows/ +├── ci-pr-checks.yml # Fast pre-flight: L0 + L1 (every push) +├── ci.yml # Full gate: L0-L5 (PR merge & main push) +└── ci-chaos.yml # Chaos: L6 (weekly schedule & manual) +``` + +### 5.2 ci-pr-checks.yml — Fast Pre-Flight + +**Purpose:** Run in <2 minutes. Blocks PR if version sync or unit tests fail. + +```yaml +name: PR Pre-flight + +on: + pull_request: + paths-ignore: + - 'docs/**' + - '**.md' + - '.planning/**' + +jobs: + check-version-sync: + name: Check version sync (L0) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + - run: python scripts/check_version_sync.py + + unit-tests: + name: Unit tests (L1) - py${{ matrix.python }} on ${{ matrix.os }} + needs: [check-version-sync] + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest, macos-latest] + python: ["3.10", "3.11", "3.12"] + runs-on: ${{ matrix.os }} + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python }} + - name: Install deps + run: | + pip install -e ".[test]" + - name: Run unit tests + run: | + pytest tests/unit/ -q --tb=short -x +``` + +**Matrix: 3 OS × 3 Python = 9 jobs (each <2 min)** + +### 5.3 ci.yml — Full Gate + +**Purpose:** Run all gates (L0-L5) on merge to main. Plasma matrix where L1 runs everywhere, L2-L5 run on selective configurations. + +```yaml +name: CI Full Gate + +on: + push: + branches: [master, main] + pull_request: + # Also on PR, but with matrix + +jobs: + # ── L0 ── + check-version-sync: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + - run: python scripts/check_version_sync.py + + # ── L1: Full matrix ── + unit-tests: + needs: [check-version-sync] + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest, macos-latest] + python: ["3.10", "3.11", "3.12"] + runs-on: ${{ matrix.os }} + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python }} + - run: pip install -e ".[test]" + - run: pytest tests/unit/ -q --tb=short --timeout=60 + - run: ruff check paperforge/ tests/ + + # ── L2: Singular config ── + cli-contract-tests: + needs: [unit-tests] + runs-on: ubuntu-latest + strategy: + matrix: + python: ["3.10", "3.12"] # Cover oldest + newest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python }} + - run: pip install -e ".[test]" + - run: pytest tests/cli/ -q --tb=short + + # ── L3: Plugin tests (Node) ── + plugin-tests: + needs: [check-version-sync] + runs-on: ubuntu-latest + strategy: + matrix: + node: ["20"] + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: ${{ matrix.node }} + - run: npm ci # Installs vitest, obsidian-test-mocks, etc. + working-directory: tests/plugin/ + - run: npx vitest run + working-directory: tests/plugin/ + + # ── L4: Temp vault E2E ── + e2e-tests: + needs: [cli-contract-tests] + runs-on: ubuntu-latest + strategy: + matrix: + python: ["3.11"] # Single Python for E2E (cost optimization) + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python }} + - run: pip install -e ".[test]" + - run: pytest tests/e2e/ -q --tb=short --timeout=120 + + # ── L5: User journey ── + journey-tests: + needs: [e2e-tests] + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + - run: pip install -e ".[test]" + - run: pytest tests/journey/ -q --tb=short --timeout=300 + + # ── Result ── + all-gates-passed: + if: always() + needs: [check-version-sync, unit-tests, cli-contract-tests, plugin-tests, e2e-tests, journey-tests] + runs-on: ubuntu-latest + steps: + - uses: re-actors/alls-green@release/v1 + with: + jobs: ${{ toJSON(needs) }} +``` + +### 5.4 ci-chaos.yml — Destructive Tests + +```yaml +name: Chaos Tests + +on: + schedule: + - cron: "0 6 * * 0" # Every Sunday 06:00 UTC + workflow_dispatch: # Manual trigger + +jobs: + chaos-tests: + strategy: + fail-fast: false + matrix: + python: ["3.12"] + scenario: + - corrupt_json_exports + - missing_pdf_files + - ocr_api_timeout + - zotero_junction_broken + - concurrent_sync_calls + - interrupted_ocr + - nonexistent_vault_path + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python }} + - run: pip install -e ".[test]" + - name: Run chaos scenario ${{ matrix.scenario }} + run: | + pytest tests/chaos/scenarios/test_${{ matrix.scenario }}.py -q --tb=short --timeout=120 +``` + +### 5.5 Cost Optimization Rationale + +| Layer | Full Matrix Rationale | +|-------|----------------------| +| L0 Version check | Single OS (ubuntu). Version checks are OS-independent. | +| L1 Unit tests | **Full 3×3 matrix.** Unit tests catch Python version and OS-specific path/symlink issues. Cheap per-job (~2 min). | +| L2 CLI contracts | **2 Python versions × 1 OS** (Linux). CLI subprocess behavior is largely OS-independent; Python version differences in argparse matter. | +| L3 Plugin tests | **1 Node version × 1 OS.** Obsidian plugin testing with mocks does not vary significantly across Node versions. | +| L4 E2E | **1 Python × 1 OS** (Linux). E2E tests are expensive (~10 min). Run on reference config only. | +| L5 Journey | **1 Python × 1 OS** (Linux). Most expensive (~15 min). Single config validates contract. | +| L6 Chaos | **1 Python × 1 OS** (Linux). Run scenarios in parallel via matrix on scenario, not OS. | + +--- + +## 6. Build Order & Dependencies + +### 6.1 Strict Dependency Chain + +``` +L0 (version sync) + | + +-----> L1 (unit tests) ──────────────> L3 (plugin tests) + | | | + | +-----> L2 (CLI contracts) | + | | | + | +-----> L4 (E2E) ───────────+ + | | + | +-----> L5 (journey) + | + +-----> L6 (chaos) - INDEPENDENT, scheduled +``` + +### 6.2 Justification + +| Edge | Why | +|------|-----| +| L0 -> L1 | No point running tests if versions are inconsistent (false positives from wrong version comparisons) | +| L1 -> L2 | CLI contracts depend on worker functions being correct; broken units would produce misleading CLI failures | +| L1 -> L3 | Plugin tests and unit tests are independent (Python vs JS). Can run in parallel after L0 passes | +| L2 -> L4 | E2E tests build on contracts — if CLI outputs malformed JSON, E2E will also fail. Run contracts first for faster feedback | +| L4 -> L5 | Journey tests are supersets of E2E; if E2E fails, journeys will too. Run cheaper E2E first | +| L6 standalone | Chaos tests are slow and destructive; run on schedule, not on every PR | + +### 6.3 Incremental Adoption + +The layers can be rolled out in this order: + +``` +Phase 1 (v2.0a): L0 + L1 + restructured tests/unit/ + - Move existing tests into tests/unit/ + - Create scripts/check_version_sync.py + - Create tests/unit/conftest.py + - Create ci-pr-checks.yml + +Phase 2 (v2.0b): L2 + fixtures/ + - Create tests/cli/ with subprocess invoker + - Create fixtures/ directory with golden datasets + - Add snapshot testing (pytest-snapshot) + - Extend CI for L2 + +Phase 3 (v2.0c): L3 + L4 + - Create tests/plugin/ with Vitest + obsidian-test-mocks + - Create tests/e2e/ with temp vault lifecycle + - Create robust mock systems (mock OCR, mock vault builder) + - Add L3/L4 to CI + +Phase 4 (v2.0d): L5 + L6 + - Create UX_CONTRACT.md + - Create tests/journey/ + - Create tests/chaos/scenarios/ + - Add scheduled chaos workflow +``` + +--- + +## 7. pyproject.toml Configuration Updates + +```toml +# Additions to existing [tool.pytest.ini_options] +[tool.pytest.ini_options] +addopts = """ + --ignore=tests/sandbox/00_TestVault/ + --ignore=tests/sandbox/ + --strict-markers +""" +markers = [ + "unit: Unit tests (Level 1) — fast, isolated", + "cli: CLI contract tests (Level 2) — subprocess boundary", + "plugin: Plugin tests (Level 3) — vitest, not pytest", + "e2e: End-to-end tests (Level 4) — temp vault", + "journey: User journey tests (Level 5) — full workflows", + "chaos: Destructive tests (Level 6) — abnormal scenarios", + "slow: Tests that take >30s (skip during development)", + "network: Tests that require network access", + "snapshot: Tests that use snapshot comparison", +] + +[tool.pytest.ini_options] +# Existing addopts remains +addopts = "--ignore=tests/sandbox/00_TestVault/ --strict-markers" +testpaths = ["tests/unit", "tests/cli", "tests/e2e", "tests/journey", "tests/chaos"] +``` + +**New test dependency additions to `pyproject.toml`:** + +```toml +[project.optional-dependencies] +test = [ + "pytest>=7.4.0", + "pytest-snapshot>=0.9.0", # Snapshot testing for CLI contracts + "pytest-timeout>=2.2.0", # Timeout guards for E2E/chaos tests + "responses>=0.25.0", # HTTP mock for OCR backend + "pytest-mock>=3.12.0", # Built-in monkeypatch on steroids + "ruff>=0.4.0", + "coverage>=7.4.0", # Coverage measurement +] +``` + +**Vitest config for plugin tests:** + +The plugin tests use a separate JavaScript toolchain. Configuration at `tests/plugin/vitest.config.ts`: + +```typescript +import { defineConfig } from 'vitest/config'; + +export default defineConfig({ + test: { + environment: 'jsdom', + globals: true, + setupFiles: ['./setup.ts'], // Configures obsidian-test-mocks + fileParallelism: false, // Plugin tests are stateful + include: ['test_*.js'], + }, + resolve: { + alias: { + 'obsidian': 'obsidian-test-mocks/obsidian', + }, + }, +}); +``` + +--- + +## 8. Data Flow Between Layers + +### 8.1 Golden Dataset Flow + +``` +fixtures/ +├── zotero/orthopedic.json ───────────> tests/unit/ (via zotero_export fixture) +│ tests/cli/ (via vault_builder copy) +│ tests/e2e/ (via vault_builder copy) +│ +├── pdf/blank.pdf ─────────────────────> tests/unit/ (pdf_resolver tests) +│ tests/e2e/ (via Zotero storage copy) +│ +├── ocr/paddleocr_success.json ────────> tests/unit/ (via mock_ocr_backend fixture) +│ tests/cli/ (via mock_ocr_backend fixture) +│ +├── snapshots/paths_json/*.json ───────> tests/cli/ (via snapshot comparison) +│ snapshots/formal_note_frontmatter/ tests/e2e/ (via snapshot comparison) +│ +└── ocr/extracted_fulltext.md ─────────> tests/e2e/ (expected output fixture) +``` + +### 8.2 Mock System Lifecycle + +``` +┌─────────────────────────────────────────────────────────────┐ +│ pytest session start │ +└─────────────────────────────────────────────────────────────┘ + │ + ┌──────────┴──────────┐ + ▼ ▼ + ┌───────────────────┐ ┌───────────────────┐ + │ tests/conftest.py │ │ conftest per │ + │ (root) │ │ layer dir │ + │ │ │ │ + │ vault_builder │ │ cli: cli_invoker │ + │ zotero_export │ │ unit: auto-mocks │ + │ mock_ocr_backend │ │ e2e: e2e_vault │ + └───────────────────┘ └────────────────────┘ + │ │ + ▼ ▼ + ┌───────────────────────────────────────┐ + │ Each test function │ + │ - requests fixtures │ + │ - asserts behavior │ + │ - cleanup via tmp_path │ + └───────────────────────────────────────┘ +``` + +--- + +## 9. Snapshot Testing Architecture + +### 9.1 When to Use Snapshots + +| Area | Snapshot Type | Tool | +|------|--------------|------| +| CLI `--json` output | Full JSON string | `pytest-snapshot` | +| Formal note frontmatter | YAML/string | `pytest-snapshot` | +| Canonical index JSON | Full JSON | `pytest-snapshot` | +| OCR meta.json | Full JSON | `pytest-snapshot` | +| Figure map JSON | Full JSON | `pytest-snapshot` | + +### 9.2 Snapshot Storage Layout + +``` +fixtures/snapshots/ +├── paths_json/ +│ └── default_config.json # Expected output of `paths --json` +├── status_json/ +│ └── minimal_vault.json # Expected output of `status --json` +├── formal_note_frontmatter/ +│ └── orthopedic_article.yaml # Expected frontmatter after sync +└── index_json/ + └── after_sync.json # Expected index after full sync +``` + +### 9.3 Snapshot Update Protocol + +When a deliberate change modifies output format (e.g., adding a new field to `status --json`): + +```bash +# 1. Run tests to see which snapshots fail +pytest tests/cli/ --snapshot-update +# 2. Review the diff +git diff fixtures/snapshots/ +# 3. Commit updated snapshots with the feature change +``` + +--- + +## 10. Exclusions & Boundaries + +### What This Architecture Does NOT Cover + +| Area | Reason | Handled By | +|------|--------|------------| +| Real Obsidian plugin E2E | Requires a running Obsidian process; `obsidian-e2e` is experimental and would add 3+ min per test | Manual QA, deferred to v2.1 | +| Real PaddleOCR API integration | Requires API key + network; tested manually via `ocr doctor --live` | Manual test | +| Performance / load testing | PaperForge is a local single-user tool; no load requirement | Not planned | +| Cross-version plugin compat | Plugin runs inside Obsidian which handles its own compatibility | Obsidian manifest `minAppVersion` | +| Installation packaging tests | `pip install` validation is handled by CI matrix's `pip install -e .` step | CI matrix | + +### Boundaries Between Layers (What NOT to test in each layer) + +| Layer | Don't Test Here | Because | +|-------|-----------------|---------| +| L1 Unit | Subprocess execution, filesystem side effects | Those are L2/L4 concerns | +| L2 CLI | Python API correctness, OCR state transitions | L1 covers those faster | +| L3 Plugin | Python worker logic, CLI output parsing | L1/L2 cover those | +| L4 E2E | Edge cases in individual functions, pure logic | L1 covers those | +| L5 Journey | Exceptions and error states that users never see | L6 covers those | +| L6 Chaos | Happy-path workflows | L4/L5 cover those | + +--- + +## 11. Architecture Decision Records + +### ADR-001: Flat tests/ → Subdirectory Migration + +- **Status:** Accepted +- **Context:** Existing tests are flat in `tests/`. With 6 layers, flat becomes unmanageable. +- **Decision:** Relocate existing tests to `tests/unit/`. Keep `tests/sandbox/` in place. +- **Consequence:** All existing test imports continue to work (they import from `paperforge.*` which is unchanged). CI must update `testpaths`. + +### ADR-002: pytest-snapshot for CLI Contracts + +- **Status:** Accepted +- **Context:** CLI JSON output must remain stable across releases. Manual assertions are brittle. +- **Decision:** Use `pytest-snapshot` for CLI contract tests, store snapshots in `fixtures/snapshots/`. +- **Consequence:** Snapshot updates become deliberate commits, forcing review before merging output changes. + +### ADR-003: Vitest over Jest for Plugin Tests + +- **Status:** Accepted +- **Context:** The plugin uses modern JS (ESM, async/await). Testing options: Vitest vs Jest. +- **Decision:** Use Vitest — native ESM support, faster watch mode, better `obsidian-test-mocks` integration. +- **Consequence:** Plugin devs need Node 20+. CI runs a separate `npm ci && npx vitest run` step. + +### ADR-004: Plasma CI Matrix (not Full Cartesian) + +- **Status:** Accepted +- **Context:** A full 3×3×1 matrix (OS × Python × Node) for all layers would cost 27+ CI jobs per PR. +- **Decision:** Use a plasma matrix — L1 runs on full 3×3, L2-L5 run on progressively narrower configs. +- **Consequence:** Some OS × Python combinations are untested for L2-L5. Acceptable because L1 covers OS-specific code paths. + +### ADR-005: Mock OCR at HTTP Layer, Not Module Layer + +- **Status:** Accepted +- **Context:** OCR tests need to intercept network calls. Options: mock `requests` at module level vs mock HTTP entirely. +- **Decision:** Use `responses` library to intercept at the HTTP layer. Fixtures are real PaddleOCR response examples. +- **Consequence:** Tests capture the full request/response flow. Adding new API endpoints automatically exercises mock coverage. + +### ADR-006: Golden Datasets in fixtures/, Not in tests/ + +- **Status:** Accepted +- **Context:** Fixture JSON files and PDFs were previously scattered across `tests/fixtures/`, `tests/sandbox/`, and `tests/`. +- **Decision:** Centralize all golden datasets in `/fixtures/` with subdirectories by type. +- **Consequence:** All layers reference the same canonical fixtures. Clear boundary between test code and test data. + +--- + +## Sources + +- pytest documentation on good integration practices: https://pytest.org/latest/goodpractices.html (MEDIUM confidence — verified via Exa search) +- obsidian-test-mocks: https://registry.npmjs.org/obsidian-test-mocks (MEDIUM confidence — verified via Exa search) +- obsidian-e2e: https://registry.npmjs.org/obsidian-e2e (MEDIUM confidence — verified via Exa search) +- GitHub Actions matrix testing patterns: https://docs.github.com/en/actions/guides/building-and-testing-python (HIGH confidence — official GitHub docs) +- Python test pyramid best practices: https://pytest-with-eric.com/pytest-best-practices/pytest-organize-tests/ (MEDIUM confidence — community expert) diff --git a/.planning/research/FEATURES.md b/.planning/research/FEATURES.md index fffaa3c9..27e01dd5 100644 --- a/.planning/research/FEATURES.md +++ b/.planning/research/FEATURES.md @@ -1,461 +1,97 @@ -# Feature Research: v1.8 AI Discussion Recording & Deep-Reading Dashboard +# Feature Landscape: 6-Layer Testing Infrastructure -**Domain:** AI discussion recording and deep-reading dashboard for Obsidian literature asset management -**Researched:** 2026-05-06 -**Confidence:** HIGH +**Domain:** Multi-layer quality gate testing +**Researched:** 2026-05-08 -## Feature Landscape +## Table Stakes -### Table Stakes (Users Expect These) +Features the testing infrastructure must provide. Missing any = incomplete. -Features users assume exist. Missing these = product feels incomplete. +| Feature | Why Expected | Complexity | Notes | +|---------|--------------|------------|-------| +| Version consistency gate (L0) | Preventing deployment with mismatched versions across Python package, Obsidian plugin, and config is critical for Ops | Low | Single Python script; checks `__init__.py`, `manifest.json`, `versions.json` | +| Unit tests (L1) | Without isolated unit tests, regressions are caught too late (E2E only) | Low | Existing 470+ tests move into `tests/unit/` | +| CLI output contract tests (L2) | Plugin and agents parse CLI JSON output; schema changes break downstream | Medium | Subprocess invoker + snapshot assertions | +| Plugin unit tests (L3) | Plugin JS runs inside Obsidian; mock-based tests catch logic errors before manual QA | Medium | Vitest + obsidian-test-mocks | +| E2E pipeline tests (L4) | Integration points between sync/OCR/status must be tested together | High | Temp vault lifecycle; 3 mock systems | +| Golden datasets | Deterministic test data shared across all layers | Medium | Centralized `fixtures/` with zotero/, pdf/, ocr/, snapshots/ | +| CI integration | Tests must run automatically on PR and merge | Medium | GitHub Actions workflows with matrix | +| Mock system for OCR | Tests must not hit real PaddleOCR API | Medium | `responses` HTTP mock + fixture JSON responses | +| Mock system for Zotero | Tests must not require real Zotero installation | Low | Static JSON fixtures in `fixtures/zotero/` | +| Mock vault filesystem | Each test needs an isolated, disposable vault | Medium | `tmp_path` + factory function for configurable completeness | -| Category | Feature | Why Expected | Complexity | Notes | -|----------|---------|--------------|------------|-------| -| AI Discussion | **Discussion history in workspace** | Researchers expect to find past AI conversations about a paper next to the paper itself, not scattered in browser tabs or chat logs | MEDIUM | Already have `ai/` directory; just need to populate it | -| AI Discussion | **Human-readable Q&A format** | Users need to browse and reference past discussions without parsing JSON or opening special tools | LOW | Markdown Q&A format (问题:/解答:) is naturally scannable | -| AI Discussion | **Timestamped chronology** | Every AI chat platform shows timestamps; users expect to know "when did I discuss this?" | LOW | Simple date/time stamp per Q&A pair | -| Deep-Reading Dashboard | **Know what deep-reading exists** | When viewing a paper's `deep-reading.md`, users expect to see at a glance whether Pass 1/2/3 are complete, without scrolling through the entire note | MEDIUM | Status bar at the top of the dashboard; parse Pass headers | -| Deep-Reading Dashboard | **Quick navigation to deep-reading content** | The deep-reading.md is a long file; users need a way to jump directly to it from the paper dashboard | LOW | "Jump to Deep Reading" contextual button | -| Navigation | **Jump-to-deep-reading from per-paper card** | If the user is looking at a paper's health/lifecycle dashboard, the natural next question is "can I read the deep analysis?" | LOW | Single button on the per-paper dashboard card | -| Bug Fixes | **Version number shown** | Users expect to confirm which version of PaperForge they're running, especially when reporting issues | LOW | Restore the `_versionBadge` update path | -| Bug Fixes | **No meaningless UI rows** | If a UI element shows irrelevant data (like the "ai" row), it erodes trust in the rest of the dashboard | LOW | Remove or replace with meaningful data | +## Differentiators -### Differentiators (Competitive Advantage) +Features that add significant value beyond basic testing. -Features that set PaperForge apart from generic Zotero sync or AI chat tools. +| Feature | Value Proposition | Complexity | Notes | +|---------|-------------------|------------|-------| +| Snapshot testing for CLI output | Catches unintended output changes immediately; makes output format changes deliberate commits | Low | Single assertion per contract test | +| Plasma CI matrix | Optimizes CI cost while maintaining coverage confidence | Medium | Full matrix for fast tests, narrow matrix for slow tests | +| User journey tests (L5) | Validates complete workflows against documented UX contracts; catches cross-component gaps that E2E misses | High | Most expensive tests; run only on merge to main | +| Chaos/destructive tests (L6) | Ensures system handles corrupted inputs, network failures, disk issues gracefully | High | Scheduled weekly; parallelized by scenario | +| Hierarchical conftest fixtures | Root conftest provides shared fixtures; each layer's conftest extends/overrides | Medium | Reduces duplication while keeping layer-specific behavior | +| Unified vault_builder factory | Single entry point for creating vaults at 3 completeness levels | Medium | `VaultBuilder(fixtures_root)` with `"minimal"`, `"standard"`, `"full"` modes | +| Chaos scenario matrix documentation | `CHaOS_MATRIX.md` documents all destructive scenarios, triggers, expected behavior | Low | Living document; updated as new edge cases discovered | -| Category | Feature | Value Proposition | Complexity | Notes | -|----------|---------|-------------------|------------|-------| -| AI Discussion | **Dual-format output: discussion.md + discussion.json** | Human-readable markdown for browsing AND structured JSON for dashboard consumption. No other literature tool bridges this gap with local-first files. | MEDIUM | JSON feeds the dashboard Q&A history card; markdown is the reference copy | -| AI Discussion | **Chronological session grouping with metadata** | Groups Q&A pairs by session (each `/pf-paper` or `/pf-deep` invocation), with start time, model, and agent type recorded. Enables session-level browsing. | MEDIUM | Tier-of-sessions > questions-within-session structure | -| Deep-Reading Dashboard | **Context-aware mode: deep-reading.md auto-detection** | When user opens `deep-reading.md`, the dashboard switches to a dedicated mode showing Pass status, AI Q&A history, and fulltext summary — NOT the per-paper lifecycle dashboard | MEDIUM | Extends existing mode-switching architecture (already has global/paper/collection) | -| Deep-Reading Dashboard | **Pass status overview with completion indicators** | Shows at-a-glance which of the three Keshav passes are complete, with content snippets. Reduces need to scroll through long deep-reading.md files. | MEDIUM | Parse `### Pass 1: 概览`, `### Pass 2: 精读还原`, `### Pass 3: 深度理解` | -| Deep-Reading Dashboard | **Recent AI Q&A from discussion.json** | The dashboard surfaces the most recent AI discussions about the paper directly in the deep-reading mode view. Bridges the gap between dashboard and chat history. | MEDIUM | Read `discussion.json` from `ai/` directory; show last N Q&A pairs | -| AI Discussion | **Voluntary recording model (never auto-triggered)** | Differentiates from "AI logs everything" approaches. Users opt in by running `/pf-paper` or `/pf-deep` — the recording is explicit, not surveillance. | LOW | Recording happens as a side-effect of user-initiated agent commands | +## Anti-Features -### Anti-Features +Features to explicitly NOT build. -Features that seem good but create problems. - -| Feature | Why Requested | Why Problematic | Alternative | -|---------|---------------|-----------------|-------------| -| **Auto-recording all agent conversations** | "I never want to lose a conversation" | Creates noise files for abandoned/toy prompts; violates the worker/agent boundary; may capture sensitive or experimental prompts user wants ephemeral | Record only when user runs `/pf-paper` or `/pf-deep` for a specific paper — voluntary and targeted | -| **Full chat transcription in markdown** | "I want to replay the entire conversation" | Long, unstructured, hard to extract value; duplicates what the agent already stores internally | Store structured Q&A pairs with 问题:/解答: format; only meaningful exchanges, not every tool call and status message | -| **Deep-reading dashboard as a replacement for deep-reading.md** | "I just want the dashboard, not the long note" | The deep-reading.md is the authoritative source; a dashboard summary that diverges creates inconsistency and distrust | Dashboard is an index/summary view INTO the deep-reading.md; it links to the full content, never replaces it | -| **Real-time dashboard updates during deep-reading** | "I want to see progress live" | Deep reading is agent-executed outside the plugin; real-time sync would require polling or WebSocket complexity for marginal value | Dashboard refreshes on active-leaf-change (already built); shows final state after deep reading completes | -| **Discussion search across all papers** | "Find any discussion about 'osteoporosis' across my library" | Full-text search is already handled by Obsidian; rebuilding search in the plugin duplicates functionality poorly | Rely on Obsidian's built-in vault search for cross-paper discussion search | +| Anti-Feature | Why Avoid | What to Do Instead | +|--------------|-----------|-------------------| +| Real Obsidian E2E in CI | Requires running Obsidian process; adds 3+ min per test; flaky results | Use `obsidian-test-mocks` for unit tests; manual QA for real Obsidian | +| Load/performance testing | PaperForge is local single-user tool; no performance requirements | Not needed; defer unless user reports slow performance | +| Cross-browser testing | Plugin runs inside Obsidian's Electron instance (Chromium) | Not applicable; plugin does not need cross-browser compat | +| Full parallel test matrix (3×3×6) | Would cost 54 CI jobs per PR for minimal added confidence | Plasma matrix: L1 on 3×3, L2-L5 on 1-2 configs | +| Test database / snapshots on every test | Snapshot tests add maintenance burden; use only for CLI contract output | Limit snapshot tests to JSON output format stability | +| Automated visual regression | Screenshot-based plugin UI tests are fragile and high-maintenance | Defer; component-level tests catch logic errors at lower cost | ## Feature Dependencies ``` -AI Discussion Recorder (discussion.md + discussion.json) - ├──requires──> Existing ai/ directory (v1.6 workspace migration) - ├──requires──> /pf-paper and /pf-deep agent commands exist - └──feeds──> Deep-Reading Dashboard (AI Q&A History card) - -Deep-Reading Dashboard Mode - ├──requires──> Existing mode-switching architecture (global/paper/collection) - ├──requires──> deep-reading.md exists in paper workspace - ├──requires──> discussion.json exists for AI Q&A card - └──enhances──> Per-paper Dashboard (provides entry point via Jump button) - -Jump-to-Deep-Reading Button - ├──requires──> Per-paper dashboard exists (v1.7) - ├──requires──> deep_reading_path in canonical index - └──triggers──> Deep-Reading Dashboard mode (navigates user there) - -Bug Fix: Version Number - ├──requiress──> _versionBadge element exists (v1.7) - └──requires──> version field populated in _cachedStats - -Bug Fix: Meaningless "ai" Row - └──removes──> Stale UI element from pre-workspace era +fixtures/ (golden datasets) + | + +--> L0 (version check) -- independent, needs only scripts/ + | + +--> L1 (unit tests) -- needs fixtures/ + mock systems + | | + | +--> L2 (CLI contracts) -- needs L1 + fixtures/snapshots/ + | + +--> L3 (plugin tests) -- independent of Python layers; needs fixtures/ for mock vault config + | + L2 + L3 + | + +--> L4 (E2E) -- needs golden datasets + mock systems + vault_builder + | | + | +--> L5 (journey) -- needs E2E working + UX_CONTRACT.md + | + L6 (chaos) -- independent but shares mock systems ``` -### Dependency Notes +## MVP Recommendation -- **AI Discussion Recorder feeds Deep-Reading Dashboard:** The dashboard's AI Q&A History card reads `discussion.json`. Without the recorder, this card shows an empty state. Both features should be built in the same phase. -- **Deep-Reading Dashboard extends paper mode detection:** The existing `_detectAndSwitch()` already handles `.md` files with `zotero_key` frontmatter. The new mode adds a check: if the active file is named `deep-reading.md` and resides in a paper workspace directory, switch to `deep-reading` mode instead of falling back to `global`. -- **Jump-to-Deep-Reading bridges per-paper and deep-reading dashboards:** This is a one-click navigation affordance on the per-paper card. It depends on the `deep_reading_path` field in the canonical index (already populated by `asset_index.py` v1.6). -- **Bug fixes are independent:** They don't block any new feature and can be shipped independently, but bundling in v1.8 improves perceived quality of the new features. +**Must have for v2.0 launch:** -## MVP Definition +1. **L0 + L1** (version sync + unit tests) — highest value per effort; existing tests just need relocation +2. **`fixtures/` directory** (golden datasets) — prerequisite for all higher layers +3. **Mock OCR backend** — enables deterministic testing without real API calls +4. **L2 CLI contract tests** — protects the plugin<->Python boundary +5. **`ci-pr-checks.yml`** — fast pre-flight gate (L0 + L1) on every PR -### Must Have (v1.8 Launch) +**Must have for v2.0 completion:** -These define the milestone. Without them, "v1.8 AI Discussion & Deep-Reading Dashboard" is not delivered. +6. **L3 plugin tests** — Vitest + obsidian-test-mocks +7. **L4 temp vault E2E** — full pipeline confidence +8. **`ci.yml` full gate** — complete CI with all layers -- [x] **AI Discussion Recorder (discussion.md + discussion.json):** Python recorder module that writes structured Q&A to `ai/discussion.md` (human-readable) and `ai/discussion.json` (dashboard-consumable) when `/pf-paper` or `/pf-deep` is run. -- [x] **Deep-Reading Dashboard Mode:** Plugin detects `deep-reading.md` as active file, switches to `deep-reading` mode showing Pass status summary + AI Q&A history. -- [x] **Jump-to-Deep-Reading Button:** On per-paper dashboard card, a button that opens the paper's `deep-reading.md` in Obsidian and triggers the deep-reading dashboard mode. -- [x] **Bug Fix: Version Number:** Restore version badge display in the plugin header (reads from canonical index or plugin manifest). -- [x] **Bug Fix: Remove meaningless "ai" row:** Identify and remove the stale "ai" UI row from the dashboard. +**Defer to v2.1 if needed:** -### Should Have (v1.8.x Follow-Up) - -Deferrable without breaking the milestone. - -- [ ] **Discussion session merging:** If a user runs `/pf-paper` twice for the same paper, append to existing `discussion.md` rather than overwrite. -- [ ] **Session metadata in discussion.json:** Record agent type (pf-paper vs pf-deep), model, and duration for each session. -- [ ] **Pass completion percentage in deep-reading dashboard:** Calculate rough completion (filled headings / total headings) for each Pass. - -### Future Consideration (v2+) - -- [ ] **Discussion search across library:** A dashboard view or Base showing all AI discussions across all papers. -- [ ] **Discussion export to context pack:** Include relevant discussion.json in the AI context pack for follow-up questions. -- [ ] **Deep-reading maturity integration:** Factor deep-reading completion into the existing maturity gauge/score. - -## Feature Prioritization Matrix - -| Feature | User Value | Implementation Cost | Priority | -|---------|------------|---------------------|----------| -| AI Discussion Recorder (discussion.md) | HIGH — researchers lose conversations in chat logs | MEDIUM — new Python module, template, integration with agent commands | P1 | -| AI Discussion Recorder (discussion.json) | MEDIUM — enables dashboard, but markdown is the primary value | MEDIUM — JSON serialization layer on top of discussion.md data | P1 | -| Deep-Reading Dashboard: Pass status | HIGH — immediate answer to "how complete is my reading?" | MEDIUM — new plugin render function, markdown parsing | P1 | -| Deep-Reading Dashboard: AI Q&A history | MEDIUM — surfaces recorded discussions in dashboard | LOW — reads existing discussion.json, renders cards | P1 | -| Deep-Reading Dashboard: Mode detection | HIGH — entry point for the whole feature | LOW — extends existing _detectAndSwitch() | P1 | -| Jump-to-Deep-Reading button | HIGH — bridges paper dashboard to deep reading | LOW — single contextual button + Obsidian openLinkText() | P1 | -| Bug Fix: Version number | MEDIUM — users notice when absent | LOW — single-line fix in _renderStats / _cachedStats | P1 | -| Bug Fix: Remove "ai" row | LOW — cosmetic | LOW — remove dead code | P1 | - -## Feature Details: AI Discussion Recording - -### Discussion Format - -**discussion.md** (human-readable, one file per paper, chronologically appended): - -```markdown ---- -paper_key: ABCDEFG -paper_title: "Mechanisms of Osteoarthritis..." -created: 2026-05-06T10:30:00Z -updated: 2026-05-06T14:45:00Z -session_count: 2 ---- - -# AI Discussions - -## Session 2026-05-06 10:30 — Quick Summary - -**Agent:** /pf-paper -**Started:** 2026-05-06 10:30:00 - -### 问题: What is the main finding of this paper? - -**解答:** The paper demonstrates that Piezo1 mechanosensitive ion channels mediate... -(Observed in Figure 3: IHC staining shows Piezo1 expression in chondrocytes...) - -**来源:** Pass 1 overview, Figure 3 - ---- - -### 问题: What signaling pathways are involved? - -**解答:** The study identifies Ca²⁺/NFAT and YAP/TAZ as downstream pathways... - ---- - -## Session 2026-05-06 14:00 — Deep Reading Discussion - -**Agent:** /pf-deep -**Started:** 2026-05-06 14:00:00 - -### 问题: Is the sample size adequate for the conclusions drawn? - -**解答:** The study uses n=6 per group which is standard for rodent OA models... -However, the power analysis was not reported — a limitation noted in the Discussion. - -**来源:** Pass 3 analysis, Methods section -``` - -**Key design decisions:** -- **问题:/解答: format:** Natural for Chinese-speaking biomedical researchers. Minimally structured — easy to write, easy to scan. -- **Session grouping:** Each `/pf-paper` or `/pf-deep` invocation creates a new session header. Prevents one giant undifferentiated blob. -- **来源 field:** Traces each answer back to specific Pass or section — maintains PaperForge's provenance principle. -- **Append-only:** New sessions append to the end. Never rewrites old sessions. Preserves history naturally. - -**discussion.json** (structured, dashboard-consumable): - -```json -{ - "paper_key": "ABCDEFG", - "schema_version": "1", - "updated": "2026-05-06T14:45:00Z", - "sessions": [ - { - "session_id": "2026-05-06T10:30:00", - "agent": "pf-paper", - "started": "2026-05-06T10:30:00Z", - "qa_pairs": [ - { - "question": "What is the main finding of this paper?", - "answer": "The paper demonstrates that Piezo1 mechanosensitive ion channels mediate...", - "source": "Pass 1 overview, Figure 3", - "timestamp": "2026-05-06T10:31:15Z" - }, - { - "question": "What signaling pathways are involved?", - "answer": "The study identifies Ca²⁺/NFAT and YAP/TAZ as downstream pathways...", - "source": null, - "timestamp": "2026-05-06T10:33:42Z" - } - ] - }, - { - "session_id": "2026-05-06T14:00:00", - "agent": "pf-deep", - "started": "2026-05-06T14:00:00Z", - "qa_pairs": [ - { - "question": "Is the sample size adequate for the conclusions drawn?", - "answer": "The study uses n=6 per group which is standard...", - "source": "Pass 3 analysis, Methods section", - "timestamp": "2026-05-06T14:05:30Z" - } - ] - } - ] -} -``` - -**Key design decisions:** -- **Minimal schema:** Only what the dashboard needs. Avoids duplicating the full markdown content. -- **session_id = ISO timestamp:** Timestamps are unique enough to serve as IDs without a UUID dependency. -- **qa_pairs array:** Flat, filterable, order-preserving. -- **source field optional:** Not all Q&A has a clear source section; null means "general discussion." - -### Recorder Integration Points - -The recorder lives as a new Python module: `paperforge/worker/discussion_recorder.py` - -**Entry points (two):** - -1. **`/pf-paper` completion:** When the agent finishes a `/pf-paper` session, the orchestrator invokes `discussion_recorder.record_session(key, agent="pf-paper", qa_pairs=[...])` -2. **`/pf-deep` completion:** Same pattern, with `agent="pf-deep"` - -**Key constraint from architecture (PROJECT.md):** -> "No auto-triggering of AI agents allowed — recording must be voluntary/user-initiated." - -The recorder is invoked at the END of a user-initiated agent session, writing output to files. It does NOT trigger agents, watch for new prompts, or run in background. - -**File operations:** -- Read existing `discussion.md` / `discussion.json` if they exist -- Append new session data -- Write updated files back to `ai/` directory -- All operations are idempotent: re-running the same session with the same session_id overwrites that session's entry, not duplicates it - -## Feature Details: Deep-Reading Dashboard Mode - -### Mode Detection - -The plugin's `_detectAndSwitch()` adds a new check BEFORE the existing `.md` → `zotero_key` check: - -```javascript -// Pseudocode for deep-reading mode detection: -if (ext === 'md') { - // Check if this is a deep-reading.md in a paper workspace - const parentDir = activeFile.parent?.name || ''; - const workspaceMatch = parentDir.match(/^([A-Z0-9]+) - /); - const isDeepReading = activeFile.basename === 'deep-reading'; - - if (isDeepReading && workspaceMatch) { - // deep-reading mode - this._currentPaperKey = workspaceMatch[1]; // extract zotero_key - this._switchMode('deep-reading'); - return; - } - - // Existing zotero_key check for per-paper mode - // ... -} -``` - -**Detection hierarchy (in order):** -1. `.base` → collection mode (existing) -2. `deep-reading.md` in `{KEY} - {Title}/` workspace → deep-reading mode (NEW) -3. `.md` with `zotero_key` frontmatter → per-paper mode (existing) -4. Everything else → global mode (existing) - -### Dashboard Layout (Deep-Reading Mode) - -``` -┌─────────────────────────────────────────────┐ -│ [DEEP READING] Paper Title (truncated) │ ← Mode badge + context -├─────────────────────────────────────────────┤ -│ │ -│ 📊 Reading Progress │ -│ ┌─────────────────────────────────────┐ │ -│ │ Pass 1: 概览 ✓ Complete │ │ ← Status bar -│ │ Pass 2: 精读还原 ✓ Complete │ │ -│ │ Pass 3: 深度理解 ○ Pending │ │ -│ └─────────────────────────────────────┘ │ -│ │ -│ 📝 Pass 1 Summary │ -│ ┌─────────────────────────────────────┐ │ -│ │ One-liner from 一句话总览 field │ │ ← Snippet from deep-reading.md -│ │ Category: Original Research │ │ -│ │ Context: Osteoarthritis models... │ │ -│ └─────────────────────────────────────┘ │ -│ │ -│ 💬 Recent AI Discussions (2 sessions) │ -│ ┌─────────────────────────────────────┐ │ -│ │ Q: What is the main finding? │ │ ← From discussion.json -│ │ A: The paper demonstrates... [↗] │ │ (last 3 Q&A pairs) -│ ├─────────────────────────────────────┤ │ -│ │ Q: Is the sample size adequate? │ │ -│ │ A: The study uses n=6... [↗] │ │ -│ └─────────────────────────────────────┘ │ -│ │ -│ 🔗 Quick Links │ -│ [Open Fulltext] [Open Main Note] ... │ ← Contextual buttons -│ │ -└─────────────────────────────────────────────┘ -``` - -### Section Details - -#### 1. Reading Progress (Status Bar) - -**What:** Three-line indicator showing Pass completion status. - -**How:** -- Parse `deep-reading.md` content for section markers: - - Pass 1 complete if `### Pass 1: 概览` section has non-scaffold content (not empty, not just template text) - - Pass 2 complete if `### Pass 2: 精读还原` + `#### Figure-by-Figure 解析` have filled content - - Pass 3 complete if `### Pass 3: 深度理解` has filled content -- Simple heuristic: if section has more than X non-empty lines after the heading, it's "complete" -- Visual: ✓ (green) for complete, ○ (gray) for pending, ⚠ (yellow) for partial - -**Why not just check deep_reading_status:** The canonical index's `deep_reading_status` is binary (done/pending). The dashboard needs granularity: a paper might have Pass 1 complete but Pass 2 in progress. Parsing the file directly avoids needing Python-side updates for this granular state. - -#### 2. Pass 1 Summary - -**What:** Quick overview extracted from the deep-reading.md. - -**How:** -- Extract the `**一句话总览**` content (the first non-empty line after that marker) -- Extract the 5 Cs fields from the Pass 1 section -- Show as a card with max 5 lines, "Read more →" link to full deep-reading.md - -#### 3. AI Q&A History - -**What:** Recent Q&A pairs from discussion.json. - -**How:** -- Read `ai/discussion.json` (same workspace directory) -- Show last 3 Q&A pairs across all sessions (most recent first) -- Each card shows: question (truncated to 80 chars) + answer preview (40 chars) -- Click expands to full answer, or opens discussion.md for full context -- Empty state: "No AI discussions yet. Run /pf-paper or /pf-deep to start." - -#### 4. Quick Links - -**What:** Navigation buttons to related files. - -**How:** -- "Open Fulltext" → opens `fulltext.md` in workspace (reuse existing _openFulltext) -- "Open Main Note" → opens the main paper note in workspace -- "Open Discussion" → opens `ai/discussion.md` (if exists) -- These replace the per-paper mode's contextual buttons since the context is different - -### What the Deep-Reading Dashboard is NOT - -- **NOT a replacement for deep-reading.md:** The full note remains the authoritative source. The dashboard is a navigation/view layer. -- **NOT a re-render of the per-paper dashboard:** The lifecycle stepper, health matrix, and maturity gauge belong to the per-paper card. Deep-reading mode is focused on the reading content itself. -- **NOT a real-time monitor:** No polling for agent progress. Dashboards refresh on active-leaf-change (existing pattern). - -## Feature Details: Jump-to-Deep-Reading Button - -### Placement - -On the per-paper dashboard card, in the contextual actions row (where "Copy Context" and "Open Fulltext" currently live). - -### Behavior - -1. **Visible condition:** Button appears when `entry.deep_reading_path` is non-empty in the canonical index (meaning deep-reading.md exists in the workspace). -2. **Click action:** Opens `deep-reading.md` in Obsidian using `this.app.workspace.openLinkText()`. The dashboard auto-detects the new active file and switches to deep-reading mode. -3. **Visual:** Similar to existing contextual buttons (`paperforge-contextual-btn` class), with an icon + text: "🔬 Deep Reading" (or a Unicode character that renders well). - -### Implementation - -```javascript -// In _renderPaperMode(), after the existing contextual buttons: -if (entry.deep_reading_path) { - const drBtn = actionsRow.createEl('button', { cls: 'paperforge-contextual-btn' }); - drBtn.createEl('span', { cls: 'paperforge-contextual-btn-icon', text: '\uD83D\uDD2C' }); // 🔬 - drBtn.createEl('span', { text: 'Deep Reading' }); - drBtn.addEventListener('click', () => { - this.app.workspace.openLinkText(entry.deep_reading_path, '', false); - }); -} -``` - -## Feature Details: Bug Fixes - -### Bug: Version Number Not Displaying - -**Root cause:** The `_versionBadge` element is created in `_buildPanel()` but only updated in `_renderStats()`. The `_cachedStats.version` field is set to `'\u2014'` (em dash) on first load and never overwritten because the version field isn't populated from the canonical index. - -**Fix:** Two changes: -1. In `_fetchStats()`, after building `this._cachedStats`, add: `this._cachedStats.version = index.schema_version || index.version || '\u2014';` -2. Alternatively, read the version from `paperforge/__init__.py` via Python subprocess, or from `paperforge/plugin/manifest.json` plugin version. The canonical index should carry a version field. - -**Simplest fix:** The plugin manifest already has `"version": "1.4.15"`. Read it at plugin init and supply to the badge: - -```javascript -// In onOpen() or _buildPanel(): -const manifest = this.app.plugins.plugins['paperforge']?.manifest; -if (manifest?.version) { - this._versionBadge.setText('v' + manifest.version); -} -``` - -### Bug: Meaningless "ai" Row - -**Root cause:** From the v1.6 workspace migration era, the per-paper dashboard or global dashboard may have a row/label displaying "ai" — likely a leftover from when `ai/` directory creation was tracked as a health dimension. - -**Fix:** Identify the source. Likely in the `_renderHealthMatrix()` call or `_renderNextStepCard()` where a stale field like `health.ai_health` is being rendered. Remove any reference to an "ai" health dimension from: -1. `paperforge/worker/asset_index.py` — `compute_health()` function -2. `paperforge/plugin/main.js` — `_renderHealthMatrix()` dimensions array -3. `paperforge/plugin/main.js` — `_fetchStats()` health aggregation - -## Competitor / Ecosystem Analysis - -| Tool | Discussion Recording | Dashboard Integration | Notes | -|------|---------------------|-----------------------|-------| -| **Obsidian Smart Chat** | Saves thread URLs + status inline in notes; markdown codeblocks | Dataview dashboards from `chat-active`/`chat-done` fields | Closest pattern: inline recording + dashboard querying. PaperForge differs by having dedicated per-paper workspace + structured Q&A format | -| **Obsidian Copilot** | Saves conversations as `.md` with YAML frontmatter; project-aware isolation | Chat history popover with fuzzy search, time grouping | Strong inspiration for session grouping and naming conventions | -| **Gemini Scribe** | Per-note history file: `[Note] - Gemini History.md`; auto-appending | History files linked to source notes; graph integration | One-file-per-note model — simple, but doesn't support multi-session grouping well | -| **Claude Sessions** | Reads Claude Code JSONL logs; renders interactive timeline | Summary dashboard with hero cards, token charts, tool charts; Base dashboards | Over-engineered for PaperForge's needs (full timeline rendering), but the "summary dashboard + Bases" pattern is relevant | -| **Smart2Brain** | Save chats; continue later | RAG-powered Q&A with vault note references | Focuses on knowledge retrieval, not discussion recording | -| **Omi Conversations** | Auto-sync from Omi device; daily-indexed conversations | Hub dashboard with tabs (tasks, conversations, memories, stats, map) | The folder-per-day + index structure is clean. PaperForge's session-per-invocation is simpler and sufficient | - -**Key insight from ecosystem:** The Obsidian plugin ecosystem overwhelmingly favors **file-based recording** (markdown in the vault) over database-backed storage. PaperForge's `discussion.md` + `discussion.json` dual-format approach fits this pattern while adding dashboard-queryable structure that pure markdown lacks. - -## Architecture Alignment - -All new features must respect existing architecture principles from `.planning/research/ARCHITECTURE.md`: - -1. **Thin-shell plugin:** The plugin reads JSON (canonical index + discussion.json), never recomputes lifecycle or health. The deep-reading mode parsing (Pass status) is a read-only display concern — not business logic duplication. -2. **Python-owned truth:** Discussion recording logic lives in `paperforge/worker/discussion_recorder.py`. The plugin only reads the output files. -3. **No new canonical index fields needed:** The deep-reading dashboard reads `deep_reading.md` content directly and `discussion.json` from the workspace. No changes to `formal-library.json` required (though a `discussion_count` field could be added later for the per-paper card). -4. **Mode switching extends existing pattern:** Deep-reading mode is a fourth mode (`deep-reading`) alongside global, paper, and collection. It uses the same `_switchMode()`, `_renderModeHeader()`, and `_refreshCurrentMode()` infrastructure. +9. **L5 journey tests** — high effort; most value after core pipeline is proven +10. **L6 chaos tests** — valuable but can be added incrementally post-launch ## Sources -- **PaperForge internal code inspection:** `paperforge/plugin/main.js` — mode switching architecture (global/paper/collection), per-paper dashboard rendering, contextual buttons, event subscriptions — HIGH confidence -- **PaperForge internal code inspection:** `paperforge/worker/sync.py` — workspace migration creating `ai/` directory (lines 1680-1749) — HIGH confidence -- **PaperForge internal code inspection:** `paperforge/worker/asset_index.py` — canonical index `_build_entry()` creating workspace paths including `ai_path`, `deep_reading_path` — HIGH confidence -- **PaperForge internal code inspection:** `paperforge/skills/literature-qa/scripts/ld_deep.py` — deep-reading scaffold structure, Pass 1/2/3 markers, figure block format — HIGH confidence -- **Obsidian Smart Chat documentation:** Chat thread linking within notes, Dataview dashboards, chat-active/chat-done tracking — https://smartconnections.app/smart-chat/ — HIGH confidence (official plugin docs) -- **Obsidian Copilot (DeepWiki):** Chat persistence and history system — markdown files with YAML frontmatter, session grouping, recent usage tracking — https://deepwiki.com/logancyang/obsidian-copilot/8-chat-persistence-and-history — HIGH confidence (documented architecture) -- **Gemini Scribe chat history guide:** Per-note history file pattern, auto-appending, markdown formatting — https://github.com/allenhutchison/obsidian-gemini — MEDIUM confidence (community plugin docs) -- **Claude Sessions plugin:** Session timeline rendering, summary dashboard with hero cards, Obsidian Bases dashboards — https://github.com/gapmiss/claude-sessions — HIGH confidence (well-documented community plugin) -- **PaulGP llms.txt proposal:** Discussion of AI-readable paper annotations, limitations-first orientation — https://paulgp.com/2026/03/10/llms-txt-for-academic-papers.html — MEDIUM confidence (academic blog post, design philosophy reference) -- **Effortless Academic discussion writing guide:** Q&A-style paper discussion structure — MEDIUM confidence (practitioner guide) - ---- - -*Feature research for: v1.8 AI Discussion Recording & Deep-Reading Dashboard* -*Researched: 2026-05-06* +- Existing codebase analysis: all 30 existing test files audited for behavioral correctness +- pytest best practices (verified via Exa search): pytest-with-eric.com, orchestrator.dev +- GitHub Actions matrix patterns (verified via official docs): docs.github.com/en/actions/guides/building-and-testing-python diff --git a/.planning/research/PITFALLS.md b/.planning/research/PITFALLS.md index e677f600..cbf9c44b 100644 --- a/.planning/research/PITFALLS.md +++ b/.planning/research/PITFALLS.md @@ -1,519 +1,1036 @@ -# Domain Pitfalls — v1.8 AI Discussion & Deep-Reading Dashboard +# Domain Pitfalls — v2.0 Multi-Layer Testing Infrastructure -**Domain:** Brownfield Obsidian plugin — adding deep-reading dashboard mode and AI discussion recording to existing mode-based routing -**Researched:** 2026-05-06 +**Domain:** Brownfield hybrid Python + Obsidian plugin project — adding 6-layer quality gate testing (version sync, Python unit, CLI contract, plugin-backend integration, temp vault E2E, user journey, destructive scenarios) +**Researched:** 2026-05-08 **Confidence:** HIGH --- ## Critical Pitfalls -### Pitfall 1: Mode detection ordering regression — `deep-reading.md` hijacks per-paper mode +### Pitfall 1: Mocking External Services Too Early and Too Rigidly **What goes wrong:** -When the user opens `Literature/Orthopedics/ABCDEFG - Biomechanics of ACL Reconstruction/deep-reading.md`, the `_detectAndSwitch()` method encounters a `.md` file, checks for `zotero_key` in frontmatter, and **finds it** — because `deep-reading.md` was generated by the agent with the same `zotero_key` in its frontmatter. The dashboard enters per-paper mode instead of deep-reading mode. The user sees lifecycle stepper instead of Pass 1 summary and AI Q&A history. +Tests mock `requests.post()` to PaddleOCR, `load_export_rows()` for BBT JSON, and `requests.get()` for OCR poll results with `MagicMock(return_value=...)`. The mocks are so tightly coupled to the current implementation that: +1. A refactoring that changes how `requests.post()` is called (e.g., adding a header, changing URL) breaks 30+ tests even though the external service contract hasn't changed. +2. The mocks silently drift from the real API — the real PaddleOCR API returns `{"data": {"jobId": "..."}}` but the mock returns `{"data": {"job_id": "..."}}` and nobody notices until integration testing. +3. Tests pass with mocked data that would never occur in production (e.g., PDF paths that the resolver can't actually resolve, abstracts in formats that BBT doesn't export). **Why it happens:** -The current mode detection logic (main.js lines 718-764) is a linear cascade: -1. No active file → `global` -2. `.base` → `collection` -3. `.md` with `zotero_key` → `paper` -4. `.md` without `zotero_key` → `global` -5. Any other → `global` - -There is no check for the *filename* being `deep-reading.md` before checking for `zotero_key`. Since `deep-reading.md` carries `zotero_key` in its frontmatter (for traceability), the paper mode branch wins. +The existing pattern in `test_ocr_state_machine.py` already demonstrates 12+ layers of nested `with patch(...):` contexts (see lines 129-159, 242-267, 336-361). Each patch is a MagicMock returning synthetic dicts that match the current function call graph exactly. The mocks are created ad-hoc in tests rather than from a shared contract definition, so every developer adds their own slightly different mock shape. **How to avoid:** -Insert the deep-reading check as the **first branch** inside the `.md` handler, before the `zotero_key` check: +- **Define mock response fixtures from real snapshots first.** Before writing any test that mocks PaddleOCR, capture a real API response once and save it as `tests/fixtures/paddleocr-response-done.json`. Construct mocks from this real data, not from guessing the API shape. +- **Use autospec=True on all MagicMock patches** to prevent method signature drift: + ```python + # BAD: mock = MagicMock() — silently accepts any call signature + # GOOD: from unittest.mock import create_autospec + # mock = create_autospec(requests.Session) + with patch("paperforge.worker.ocr.requests.post", autospec=True) as mock_post: + ``` +- **Limit patch nesting depth to 3 max.** The current 12-level nesting (test_ocr_state_machine.py lines 129-159) is impossible to debug. Introduce a fixture that returns a pre-configured mock environment: + ```python + @pytest.fixture + def ocr_mocks(ocr_paths): + """Set up all OCR mocks from fixture data.""" + with ( + patch("paperforge.worker.ocr.pipeline_paths", return_value=ocr_paths), + patch("paperforge.worker.ocr.load_control_actions") as lca, + patch("paperforge.worker.ocr.load_export_rows") as ler, + patch("paperforge.worker.ocr.sync_ocr_queue") as sq, + patch("paperforge.worker.ocr.requests.post", autospec=True) as post, + ): + lca.return_value = {KEY: {"do_ocr": True}} + ler.return_value = FIXTURE_EXPORT_ROWS + sq.return_value = FIXTURE_QUEUE_ROWS + post.return_value.json.return_value = {"data": {"jobId": "fixture-job"}} + yield + ``` +- **Introduce a VCR.py layer for OCR polling.** Record real polling sequences once, replay them in tests. This catches API contract drift the moment PaddleOCR changes its response shape. -```js -if (ext === 'md') { - // Check for deep-reading first (before zotero_key check) - if (activeFile.basename === 'deep-reading' || activeFile.path.includes('/deep-reading.md')) { - const cache = this.app.metadataCache.getFileCache(activeFile); - const key = cache?.frontmatter?.zotero_key; - if (key) { - this._currentPaperKey = key; - this._currentPaperEntry = this._findEntry(key); - this._currentDomain = null; - this._switchMode('deep-reading'); - return; - } - } - // ... existing zotero_key check ... - const cache = this.app.metadataCache.getFileCache(activeFile); - const key = cache?.frontmatter?.zotero_key; - if (key) { /* paper mode */ } -} +**Warning signs:** +- Tests pass locally but fail in CI because mock shapes differ from real API +- A 3-line production change breaks 50+ test assertions +- "This test hasn't been run against real data in 6 months" realization during a bug hunt +- `MagicMock` names appearing in test failure output instead of meaningful data + +**Phase to address:** Phase 2 (Python unit tests) — establish mock response fixtures from real API captures before writing OCR unit tests. Phase 4 (Temp vault E2E) validates mock assumptions against real subprocess runs. + +--- + +### Pitfall 2: CI Matrix Combinatorial Explosion Killing Feedback Loops + +**What goes wrong:** +The CI matrix is defined as: `os: [windows, macos, ubuntu] x python: [3.10, 3.11, 3.12] x node: [18, 20]` = 3 x 3 x 2 = 18 jobs. Each job runs the full test suite (Levels 0-6). At 15 minutes per job, the full matrix takes 4.5 hours wall-clock. Developers wait all day for CI results, or worse, push 10 commits in parallel and burn 45 hours of CI credits in a morning. + +**Why it happens:** +The natural instinct is "test everything on every platform." But the actual risk profile is not uniform: +- Version sync checker (Level 0): same everywhere — needs 1 platform, 1 Python +- Python unit tests (Level 1): only OS-dependent where path handling differs — Windows path vs POSIX +- CLI --json contract tests (Level 2): OS-independent for JSON output shape +- Plugin-backend integration (Level 3): Node version matters, Python version doesn't +- Temp vault E2E (Level 4): Windows-specific for junction testing, generic for others +- Destructive tests (Level 6): must never run on CI machines shared by other projects + +**How to avoid:** +- **Use a smart partitioning strategy, not a full cross-product:** + +| Test Level | CI Trigger | OS | Python | Node | Runs | +|------------|-----------|-----|--------|------|------| +| L0: Version sync | Every push | ubuntu-only | latest only | N/A | 1 min | +| L1: Python unit | Every push | ubuntu (path-sensitive: 3 OS) | all 3 | N/A | 3 min | +| L2: CLI contract | Every push | ubuntu-only | latest only | N/A | 1 min | +| L3: Plugin integration | PR + main | ubuntu-only | latest | both | 5 min | +| L4: Temp vault E2E | PR + main | all 3 | latest on each | latest | 10 min | +| L5: User journey | Nightly + main | ubuntu-only | latest | latest | 15 min | +| L6: Destructive | Nightly only | ubuntu-only (Docker) | latest | latest | 10 min | + +- **Never run the full matrix on every push.** Use GitHub Actions path filters to run only relevant levels: + ```yaml + on: + push: + paths: + - "paperforge/worker/ocr.py" # triggers L1+L2+L4 + - "paperforge/plugin/main.js" # triggers L3 + - ".github/workflows/*.yml" # triggers full audit + ``` +- **Introduce `pytest -m "not slow"` for push-time tests** and `pytest -m "slow"` for nightly. Mark tests explicitly: + ```python + @pytest.mark.slow # Temp vault creation + full sync + def test_full_pipeline_consistency(test_vault): + ... + ``` +- **Use `pytest-xdist` with `-n auto`** for Level 1 and Level 2 only (unit tests parallelize well; E2E tests do not). +- **Set a hard CI budget (max 20 concurrent runners).** If the matrix exceeds 20 jobs, reduce granularity until it fits. + +**Warning signs:** +- CI pipeline takes longer than developer lunch break +- PRs accumulate because "waiting for CI" is the bottleneck +- CI bill spikes (or free-tier minutes exhausted mid-month) +- Developers skip CI because "it's too slow" and merge without checks +- CI dashboard shows 12/18 jobs green but the 6 red ones are from timeouts, not failures + +**Phase to address:** Phase 7 (CI expansion) — design the CI matrix strategy BEFORE writing `ci.yml`. Must include `pyproject.toml` markers and path filters. + +--- + +### Pitfall 3: Snapshot Tests Breaking on Every Refactor + +**What goes wrong:** +A golden dataset snapshot of `formal-library.json` is stored at `tests/fixtures/snapshots/formal-library-v2.json`. Every test that writes a formal note, updates an index, or changes frontmatter fields is asserted against this exact file. When a developer: +1. Adds a new frontmatter field (like `impact_factor`) +2. Changes the index envelope format from `{"version": "2"}` to `{"schema_version": "2", "generated_at": "...", "items": [...]}` +3. Adds a new CLI `--json` key (like `total_ocr_done`) + +...every snapshot test fails. Not because the change is wrong, but because the snapshot is too broad. The developer must regenerate ALL snapshots with `--snapshot-update`, but now the review diff is thousands of lines of JSON and nobody actually reviews it. Regressions slip through because "I regenerated the snapshots" becomes the default response. + +**Why it happens:** +Snapshot testing encourages the "assert everything" mindset: one snapshot assertion per test file, checking a complete output blob. This is particularly tempting for JSON outputs where every key seems important. The project has structured JSON outputs (formal-library.json, paper-meta.json, ocr-queue.json, CLI --json output) and generated markdown files — all natural targets for whole-file snapshots. + +**How to avoid:** +- **Never snapshot whole files. Always snapshot specific shapes within files.** Instead of: + ```python + # BAD: entire index file + assert json.loads(index_path.read_text()) == snapshot + ``` + Do: + ```python + # GOOD: specific structural assertions + index = json.loads(index_path.read_text()) + assert index["schema_version"] == "2" + assert "generated_at" in index # check presence, not exact value (timestamps) + assert len(index["items"]) == 1 + assert index["items"][0]["zotero_key"] == "TSTONE001" + assert index["items"][0]["has_pdf"] == True + ``` + +- **Normalize dynamic fields before snapshotting.** Timestamps, mtimes, UUIDs, generated paths change every run. Strip them before passing to snapshot: + ```python + def normalize_index(raw: str) -> dict: + data = json.loads(raw) + data.pop("generated_at", None) + for item in data.get("items", []): + item.pop("last_updated", None) + return data + + assert normalize_index(index_path.read_text()) == snapshot + ``` + +- **Use `inline-snapshot` (pydantic's library) instead of external `.ambr` files.** Inline snapshots are co-located with the test, making it obvious when a snapshot update is needed and what changed: + ```python + from inline_snapshot import snapshot + + def test_index_keys(): + assert get_index_keys() == snapshot(["zotero_key", "title", "year", "doi"]) + ``` + +- **Use `dirty-equals` for version-agnostic comparisons.** When asserting data that should be "any valid UUID" or "any ISO timestamp": + ```python + from dirty_equals import IsStr, IsUUID + + assert item["id"] == IsUUID + assert item["generated_at"] == IsStr(regex=r"\d{4}-\d{2}-\d{2}T") + ``` + +- **For generated markdown (formal notes, discussion.md), use targeted assertions** on specific section content rather than whole-file string comparison: + ```python + # BAD: whole file snapshot + assert note_text == snapshot + + # GOOD: targeted assertion + assert "## Abstract" in note_text + assert "zotero_key:" in note_text + assert "biomechanical" in note_text.lower() + assert "[[99_System/Zotero/storage/" in note_text # wikilink present + ``` + +**Warning signs:** +- PR diffs show 500+ line snapshot changes for a 5-line code change +- "Regenerated all snapshots" appears in commit messages +- Snapshot assertions never fail because nobody reviews the regenerated output +- CI snapshots fail on Monday morning because a date-based field rolled over +- Developers are afraid to touch code with snapshot tests + +**Phase to address:** Phase 6 (Golden datasets + snapshot tests) — design snapshot strategy before creating the first snapshot. Define normalization helpers first, then write snapshot tests. + +--- + +### Pitfall 4: Temp Vault Tests Being Slow, Non-Deterministic, or Platform-Specific + +**What goes wrong:** +The `test_vault` fixture in `conftest.py` (lines 168-176) creates a full Obsidian vault structure: +- Creates 10+ directories (`99_System`, `PaperForge`, `exports`, `ocr`, `Literature`, etc.) +- Writes `paperforge.json` with 6 config keys +- Writes `.env` with API tokens +- Copies OCR fixture data (`ocr-complete/TSTONE001/`) +- Copies BBT export fixtures (`exports/骨科.json`) +- Creates library records +- Creates formal notes with frontmatter +- Creates Zotero storage mock PDF in TWO locations +- Copies `ld_deep.py` to skill directory + +At ~200ms per call and 473+ tests, this works today. But Level 4 temp vault E2E tests add: +- Subprocess calls to `paperforge sync`, `paperforge ocr`, `paperforge status --json` (3-10 seconds each) +- Vault modification during tests (write formal notes, modify frontmatter) +- Cross-platform path resolution (Windows `\` vs POSIX `/`, junctions vs symlinks) +- Cleanup after destructive tests (what if `shutil.rmtree` fails on Windows?) + +The result: a single E2E test takes 30-60 seconds. 20 E2E tests = 10-20 minutes. And they fail randomly on macOS because `tempfile` paths differ from what `paperforge.json` expects. + +**Why it happens:** +The existing `test_vault` fixture creates a new vault per test function (scope="function"). This is correct for isolation but deadly for E2E tests where setup takes seconds. Additionally, the fixture hardcodes `"99_System"`, `"03_Resources"` etc. as directory names — if the config ever changes, all tests silently use stale names. + +The cross-platform issues are worse: `shutil.rmtree` on Windows fails if a file is still open (handles aren't released immediately on process exit in subprocess tests). Temp directory paths on macOS are `/var/folders/...` which is a symlink, and `Path.resolve()` vs `Path.absolute()` behavior differs. + +**How to avoid:** + +1. **Use `scope="session"` for the vault creation fixture**, with a fast clone strategy for individual tests: + ```python + @pytest.fixture(scope="session") + def base_vault(tmp_path_factory): + """Create the golden vault structure once per session.""" + vault = tmp_path_factory.mktemp("paperforge-vault") + build_minimal_vault(vault) + return vault + + @pytest.fixture + def test_vault(base_vault, tmp_path): + """Clone the golden vault per test — fast directory copy.""" + vault = tmp_path / "vault" + shutil.copytree(base_vault, vault, ignore=shutil.ignore_patterns("__pycache__")) + return vault + ``` + +2. **Separate "fast" E2E tests (no subprocess) from "full" E2E tests (subprocess):** + - Fast E2E: Call Python API functions directly (`run_sync(vault)`) + - Full E2E: Use `subprocess.run([sys.executable, "-m", "paperforge", "sync"], vault)` + - Mark full E2E as `@pytest.mark.slow` and run them only on PR + main + +3. **Add safe teardown that handles Windows file locks:** + ```python + @pytest.fixture + def test_vault_with_cleanup(test_vault): + yield test_vault + # Retry rmtree on Windows (files may be locked briefly) + for attempt in range(3): + try: + shutil.rmtree(test_vault, ignore_errors=False) + break + except PermissionError: + if attempt == 2: raise + time.sleep(1) + ``` + +4. **Normalize paths in assertions.** Never assert on absolute path strings: + ```python + # BAD: fails on macOS vs Windows + assert pdf_path == "D:\\vault\\99_System\\Zotero\\TSTONE001\\file.pdf" + + # GOOD: assert on semantics + assert "TSTONE001" in pdf_path + assert pdf_path.endswith(".pdf") + assert pdf_path.count("/") >= 3 # reasonable depth + ``` + +5. **Add a `paperforge doctor` post-check after each E2E test fixture creation** to validate the vault is self-consistent. This catches fixture drift early. + +**Warning signs:** +- `test_e2e_cli.py` takes 5+ minutes to run +- Tests pass on Windows but fail on macOS (or vice versa) +- `PermissionError` during `conftest.py` teardown on Windows CI +- "file not found" errors for paths that exist when inspected manually +- `TMPDIR` / `TEMP` / `TMP` environment variable differences between CI and local + +**Phase to address:** Phase 4 (Temp vault E2E tests) — design vault fixture strategy and cross-platform handling FIRST. Phase 3 (Plugin-backend integration) and Phase 2 (Python unit tests) inform the vault fixture requirements. + +--- + +### Pitfall 5: User Journey Tests Being Too Vague to Automate + +**What goes wrong:** +The user journey test plan says: "Test that a new user can install the plugin, configure Zotero, run OCR, and perform deep reading." The UX Contract document describes the journey in prose ("user opens Obsidian, navigates to settings, clicks Install, waits for setup, returns to vault..."). + +But when the automation engineer tries to write tests, the prose is ambiguous: +- "Configure Zotero" — configure HOW? What values? What if Zotero isn't installed? +- "User waits for setup" — how long? What if it fails? What's the success indicator? +- "Perform deep reading" — on which paper? With what expected output? + +The result: the automation engineer either hardcodes assumptions (making the test brittle) or writes a test that checks nothing meaningful (asserting only that the process didn't crash). + +**Why it happens:** +User journey tests are a new concept for the project. The team has deep experience with Python unit tests and CLI contract tests, but zero experience defining automated end-to-end journeys. The UX Contract document was written for humans, not machines. Nobody has translated "user opens Obsidian" into "app.workspace.getLeaf()" or "user clicks Install" into "plugin settingTab.installButton.click()". + +**How to avoid:** + +1. **Write user journey tests as pseudo-code BEFORE implementation**, and verify they can be executed: + ``` + # Journey: New User Setup + # Given: No paperforge.json exists, Zotero is installed with at least one paper + # When: User opens Obsidian plugin settings + # And: Clicks "Install Configuration" + # Then: paperforge.json is created + # And: Directory structure exists + # And: OpenCode skills are deployed + # And: A success notice is shown + + # Implementation: plugin_test_helper.run_click("Install Configuration") + ``` + +2. **Use a "step" abstraction layer**, not raw Playwright/Node.js APIs: + ```python + class UserJourney: + def __init__(self, vault: Path, plugin: PluginInstance): + self.vault = vault + self.plugin = plugin + self.observed_states = [] + + def observe(self, state_name: str) -> None: + """Record an observation for later assertion.""" + self.observed_states.append(state_name) + + def assert_state(self, *expected_states: str) -> None: + missing = set(expected_states) - set(self.observed_states) + assert not missing, f"Journey states not reached: {missing}" + ``` + +3. **Define EXACTLY ONE concrete scenario per journey test.** Not "user can set up" but "user with vault at /tmp/test-vault, Zotero at /tmp/zotero, configures PaperForge, clicks Install, sees paperforge.json created with correct fields." The more concrete, the more automatable. + +4. **Mark user journey tests as `@pytest.mark.journey` and run them only on labelled PRs or nightly.** They are the most expensive and least stable tests — don't gate every PR on them. + +5. **Create a "journey fixture pack"** — a pre-configured environment that puts the system in a specific state (e.g., "half-setup" = paperforge.json exists but no Zotero junction, "ready-for-deep-reading" = full vault with OCR done). Tests pick a starting pack and assert on the outcome. + +**Warning signs:** +- User journey test is 200+ lines with no clear scenario boundary +- Test description in comments is longer than the test code +- Test consistently passes but never catches real bugs +- Test fails inconsistently because "it depends on timing" +- Nobody can articulate what a "passing" user journey test means + +**Phase to address:** Phase 5 (User journey tests) — define contracts + scenarios BEFORE writing any code. Must come after Phase 0 (UX Contract docs) is complete. + +--- + +### Pitfall 6: Destructive Tests That Damage Developer Machines or CI Shared State + +**What goes wrong:** +A "chaos test" for the `repair` worker is designed to verify it handles corrupted `paperforge.json`. The test: +```python +def test_chaos_corrupted_config(test_vault): + cfg = test_vault / "paperforge.json" + cfg.write_text("{{{ NOT JSON }}}") + result = subprocess.run([sys.executable, "-m", "paperforge", "doctor"], + cwd=test_vault, capture_output=True, text=True, timeout=30) + assert result.returncode == 1 # doctor should report failure without crashing ``` -The ordering discipline is: **most specific filename pattern wins.** `deep-reading.md` is more specific than "any `.md` with `zotero_key`." - -**Warning signs:** -- User opens `deep-reading.md` and sees lifecycle stepper instead of Pass 1 summary -- Header shows "Paper" badge instead of "Deep Reading" badge -- `_renderPaperMode()` is called when `_renderDeepReadingMode()` should be - -**Phase to address:** Phase 1 (Deep-reading dashboard mode) - ---- - -### Pitfall 2: `fs.readFileSync` for `discussion.json` bypasses Obsidian vault API — missing cache invalidation and encoding issues - -**What goes wrong:** -The plugin reads `discussion.json` via Node.js `fs.readFileSync()` (same pattern as `_fetchStats` and `_loadIndex` for `formal-library.json`). This works for `formal-library.json` because it lives outside the Obsidian vault timeline in `/PaperForge/indexes/`. But `discussion.json` lives in the paper workspace `ai/` directory which IS inside the vault. Using `fs` directly means: -1. Obsidian's metadata cache doesn't know about the read -2. Vault `modify` events for `discussion.json` won't trigger dashboard refresh -3. On Windows with CJK locales, file content written by Python may be GBK-encoded on disk but read by Node.js as `utf-8`, causing mojibake for Chinese Q&A content - -**Why it happens:** -The plugin already has a pattern of `fs.readFileSync()` for indices (line 322, 428), making it natural to extend the pattern. But `discussion.json` is fundamentally different — it's inside the vault (paper workspace → visible in Obsidian file explorer) while `formal-library.json` is in a system directory (invisible, machine-controlled). - -Additionally, `child_process.spawn('python')` on Windows can produce GBK-encoded stdout/stderr when the system code page is CP936. If discussion content is generated by Python (`/pf-paper` via `ld_deep.py`), the file written to disk might use the system encoding rather than explicit UTF-8. - -**How to avoid:** - -Layer 1 — Read strategy: -- For `discussion.json` (vault-internal), use Obsidian `app.vault.adapter.read()` instead of `fs.readFileSync()` to ensure vault cache consistency -- For `discussion.md` (human-readable), use the same pattern -- Keep `fs.readFileSync()` only for `formal-library.json` and other system-directory files outside the vault - -Layer 2 — Encoding hardening: -- In the Python `discussion.json` writer, always use `encoding='utf-8'` explicitly: - ```python - with open(discussion_path, 'w', encoding='utf-8') as f: - json.dump(data, f, ensure_ascii=False, indent=2) - ``` -- In the Python child process spawn, inject UTF-8 environment variables: - ```python - env = os.environ.copy() - env['PYTHONIOENCODING'] = 'utf-8' - env['PYTHONUTF8'] = '1' - ``` -- In the plugin's Node.js reads, always pass explicit `'utf-8'` encoding: - ```js - const raw = this.app.vault.adapter.read(discussionPath); - // or if using fs: - const raw = fs.readFileSync(discussionPath, 'utf-8'); - ``` - -Layer 3 — Event subscription: -- Add `discussion.json` paths to the vault `modify` event filter alongside `formal-library.json` (line 1259): - ```js - const modifyHandler = this.app.vault.on('modify', (file) => { - if (file?.path?.endsWith('formal-library.json') || file?.path?.endsWith('discussion.json')) { - this._invalidateIndex(); - this._refreshCurrentMode(); - } - }); - ``` - -**Warning signs:** -- Chinese Q&A text in dashboard shows garbled characters (mojibake) -- Dashboard doesn't update after Python writes new discussion entries -- `JSON.parse()` throws `Unexpected token` errors on apparently valid Chinese content -- Console shows file reads from `fs` with byte-level corruption visible in hex dump - -**Phase to address:** Phase 2 (AI discussion recorder) + Phase 3 (Integration testing) - ---- - -### Pitfall 3: `discussion.json` schema evolution without versioning — silent data loss on format change - -**What goes wrong:** -The first implementation of `discussion.json` uses a simple array-of-objects format: -```json -[ - {"timestamp": "2026-05-06T10:00:00Z", "role": "user", "content": "What is the main finding?"}, - {"timestamp": "2026-05-06T10:00:05Z", "role": "assistant", "content": "The main finding is..."} -] +This is safe because `test_vault` is a temp directory. But a more ambitious chaos test: +```python +def test_chaos_delete_all_notes(test_vault): + subprocess.run(["rm", "-rf", str(test_vault)], shell=True) ``` -Three months later, the format needs to add `model`, `tokens_used`, `source_sections`, or a `conversation_id`. Existing `discussion.json` files break in the dashboard. There is no `schema_version` field to trigger a migration path. +...accidentally runs on the developer's real vault if `test_vault` resolves to the wrong path. Or worse: +```python +def test_chaos_delete_paperforge_json(): + """Test what happens when paperforge.json is deleted during sync.""" + # OOPS — no vault fixture, developer accidentally ran real sync first + vault = Path.home() / "Documents" / "Obsidian" / "MyVault" + ... +``` **Why it happens:** -The `formal-library.json` schema evolution (adding `schema_version: "2"`) was learned painfully in v1.6/v1.7. But `discussion.json` is a new artifact created in v1.8 — the team may repeat the same mistake thinking "it's just a simple flat file." +Destructive tests are inherently dangerous. The desire to test "what happens when X is deleted" or "what happens when we run `rm -rf`" is valid, but the safety boundary is easy to miss. In CI environments, destructive tests running in parallel can corrupt shared caches, Docker layers, or other projects' working directories. + +The `--vault` argument pattern in PaperForge makes this worse: some CLI commands default to the current working directory if `--vault` is omitted. A test that forgets to pass `cwd=test_vault` or `--vault` could operate on the repo root or worse. **How to avoid:** -- Ship `discussion.json` with `schema_version: "1"` in the envelope from day one: - ```json - { - "schema_version": "1", - "generated_at": "2026-05-06T10:00:00Z", - "paper_key": "ABCDEFG", - "conversations": [ - {"id": "conv_001", "timestamp": "...", "entries": [...]} - ] - } - ``` -- Add a tolerant reader in the plugin that checks `schema_version` and falls back gracefully: - ```js - function parseDiscussion(raw) { - const data = JSON.parse(raw); - if (!data.schema_version) { - // Legacy format: raw array - return { schema_version: "0", conversations: [{ entries: data }] }; - } - if (data.schema_version === "1") return data; - console.warn('PaperForge: Unknown discussion schema version', data.schema_version); - return null; - } - ``` -- Never use top-level array (`[...]`) as the root JSON type. Always use an object envelope (`{"schema_version": "...", ...}`) to allow backward-compatible field additions. + +1. **All destructive tests MUST use a temp vault created by `tmp_path` or `tmp_path_factory`.** Never accept a path from the environment, a configuration file, or a default. + ```python + @pytest.fixture + def isolated_destruct_vault(tmp_path_factory): + """Create a vault guaranteed to be temporary and deletable.""" + vault = tmp_path_factory.mktemp("chaos-vault") + build_minimal_vault(vault) + return vault + ``` + +2. **Destructive tests MUST verify the vault isolation invariant:** + ```python + def test_chaos_rmtree(isolated_destruct_vault): + vault = isolated_destruct_vault + # SAFETY CHECK: must be a tmp_path, not a real vault + assert "tmp" in str(vault) or "temp" in str(vault) + shutil.rmtree(vault) + assert not vault.exists() + ``` + +3. **Never run destructive tests on shared CI runners without Docker isolation.** Use `pytest -m "destructive"` and run in a dedicated ephemeral Docker container: + ```yaml + - name: Run destructive tests + if: github.ref == 'refs/heads/main' # only on main + run: | + docker run --rm -v $PWD:/app paperforge-test:latest \ + pytest -m "destructive" --timeout=120 + ``` + +4. **Destructive tests must be in their own test module (destructive_test_*.py)** and NEVER imported by non-destructive test runners. Add them to `pytest.ini` ignore lists: + ```ini + [tool.pytest.ini_options] + addopts = "--ignore=tests/sandbox/00_TestVault/ --ignore=tests/destructive/" + ``` + +5. **For every destructive test, write a "safety contract" at the top of the test function:** + ```python + # SAFETY CONTRACT: + # - This test ONLY operates on isolated_destruct_vault + # - It does NOT touch the filesystem outside that directory + # - It does NOT make network calls + # - If it fails, no production data is affected + # - Timeout: 30 seconds max + ``` **Warning signs:** -- "Add one more field" request for `discussion.json` triggers format debate -- Dashboard code has `if (Array.isArray(discussion))` branches -- JSON.parse throws on older files after format change -- Users report "AI discussion history disappeared" after plugin update +- Test has no `tmp_path`, `tmpdir`, or fixture isolation (bare path operations) +- Test calls `os.remove()`, `shutil.rmtree()`, or `Path.unlink()` without verifying the path is a temp directory +- Test uses `shell=True` (allows arbitrary command injection) +- Test modifies files outside the test directory for "setup" +- Test references `Path.home()`, `os.getcwd()`, or `Path(".")` without guards -**Phase to address:** Phase 2 (AI discussion recorder) — design the schema first, implement write + read together +**Phase to address:** Phase 6 (Destructive/chaos tests) — define safety invariants BEFORE writing any destructive test. Phase 7 (CI expansion) — configure Docker isolation. --- -### Pitfall 4: `btoa()` / `atob()` on Chinese filenames in discussion file paths +### Pitfall 7: Golden Dataset and Fixture Bloat **What goes wrong:** -The plugin needs to resolve the path to `discussion.json` from the paper's `zotero_key` and `title`. If the title contains Chinese characters (e.g., "膝关节置换术后康复效果分析"), and any path resolution code uses `btoa()` or `encodeURIComponent()` followed by regex operations, the code will throw `InvalidCharacterError: Failed to execute 'btoa' on 'Window': The string to be encoded contains characters outside of the Latin1 range.` +The golden dataset starts as: +- `bbt_export_absolute.json` (6KB — one paper) +- `bbt_export_storage.json` (6KB — one paper) +- `blank.pdf` (1KB) + +Six months later: +- 3 BBT JSON variants x 5 papers each = 15 JSON files (200KB) +- OCR result snapshots for 5 papers (50MB of JSON) +- Expected snapshot outputs for CLI --json in 4 command variants (50KB) +- Formal note markdown snapshots for 5 papers x 3 formats (200KB) +- PDF fixtures for 5 papers (50MB) +- Temp vault fixture packs for 3 scenarios (5MB each, but duplicated across branches) + +The fixtures directory is now 100MB+ in the git repo. `git clone` is slow. CI checkout is slow. Developers don't know which fixtures are still used, so nobody deletes anything. The `tests/fixtures/` directory becomes a "fixture graveyard." **Why it happens:** -`window.btoa()` only supports Latin1 characters (U+0000 to U+00FF). Chinese characters fall outside this range. This is a well-known pattern of failure in Obsidian plugins (documented in opencode-obsidian Issue #28, confirmed by multiple Obsidian plugin developers). - -PaperForge's current codebase does NOT use `btoa()` directly, but future path construction for `ai/` directory resolution might introduce it, especially if a content-hash or Base64 slug is used for file naming. +- Every test author adds "just one more fixture variant" for their specific edge case +- Nobody knows which fixtures are referenced by which tests (no cross-references) +- Binary fixtures (PDFs, OCR JSON dumps) bloat git history even if deleted later (git stores them forever) +- No fixture governance: "can I delete this?" requires hunting through all test files **How to avoid:** -- **Ban `btoa()` and `atob()`** from the plugin codebase. If Base64 encoding is needed, use Node.js `Buffer`: - ```js - // SAFE: Node.js Buffer handles all Unicode - const encoded = Buffer.from(chineseString, 'utf-8').toString('base64'); - const decoded = Buffer.from(encoded, 'base64').toString('utf-8'); - ``` -- For path construction, use filesystem-safe slugs derived from `zotero_key` only (which is always ASCII), never from `title`. -- Encode this rule in a linter or pre-commit check: `grep -r 'btoa(' paperforge/plugin/` must return empty. -**Warning signs:** -- Plugin crashes with `InvalidCharacterError` in console when paper title contains Chinese -- Only reproduces on papers with non-ASCII titles -- Error traceback points to `btoa` or `atob` in a path-building function - -**Phase to address:** Phase 2 (AI discussion recorder) — path resolution design - ---- - -### Pitfall 5: `active-leaf-change` double-firing creates mode oscillation between per-paper and deep-reading - -**What goes wrong:** -When the user clicks from `ABCDEFG - Title.md` (per-paper mode) to `deep-reading.md` (deep-reading mode), the Obsidian workspace fires `active-leaf-change` **twice** (a known Obsidian API behavior — see forum thread #31841). The first firing has the old leaf context, the second has the new. If the 300ms debounce (line 1251) fires between the two events, the dashboard briefly switches to `global` mode (no active file detected) before switching to `deep-reading` mode. This causes a visible flash of the global dashboard. - -Additionally, if both `deep-reading.md` and the formal note are open in split panes, the mode will oscillate between `paper` and `deep-reading` on every focus change. - -**Why it happens:** -- `active-leaf-change` event is known to fire twice during tab switches (old leaf blur + new leaf focus) -- The 300ms debounce can catch either one depending on timing -- The mode switching logic (`_switchMode`) has an early return `if (this._currentMode === mode)`, but doesn't distinguish between "same mode, different file" and "new mode for same paper" - -**How to avoid:** -1. **Guard with identity check, not string comparison:** - ```js - _detectAndSwitch() { - const activeFile = this.app.workspace.getActiveFile(); - // Track the file that triggered current mode - const newModeAndFile = this._resolveModeForFile(activeFile); - if (this._currentMode === newModeAndFile.mode && - this._activeFilePath === (activeFile?.path || null)) { - return; // Same mode AND same file — no-op - } - // Different file or different mode — switch - this._activeFilePath = activeFile?.path || null; - this._switchMode(newModeAndFile.mode, newModeAndFile.context); +1. **Keep fixtures outside the git repo.** Use a `tests/fixtures/download_fixtures.py` script that fetches or generates fixtures on demand: + ```python + # tests/fixtures/download_fixtures.py + """Download or generate test fixtures. Run `python download_fixtures.py` before testing.""" + + FIXTURES = { + "bbt_export_absolute.json": "https://storage.example.com/fixtures/v2/bbt_export_absolute.json", + "blank.pdf": generate_blank_pdf, # function that creates it } ``` -2. **Extract `_resolveModeForFile()` as pure function:** - - Returns `{ mode: 'paper'|'deep-reading'|'collection'|'global', context: {...} }` - - No side effects, fully testable - - Called by both `_detectAndSwitch()` and the debounced handler -3. **Increase debounce to 500ms** during mode transitions (from current 300ms) to absorb the double-fire + Add `tests/fixtures/cache/` to `.gitignore`. CI pre-populates cache via a GitHub Actions cache step. + +2. **Generate fixtures from code, not by hand.** PDF fixtures should be generated by a script: + ```python + def generate_blank_pdf(path: Path) -> None: + """Generate a minimal valid PDF (single blank page).""" + content = b"%PDF-1.4\n1 0 obj<>endobj\n2 0 obj<>endobj\n3 0 obj<>endobj\nxref\n0 4\n0000000000 65535 f \n0000000009 00000 n \n0000000058 00000 n \n0000000115 00000 n \ntrailer<>\nstartxref\n190\n%%EOF" + path.write_bytes(content) + ``` + +3. **Tag fixtures with a metadata file and validate coverage:** + ```json + // tests/fixtures/MANIFEST.json + { + "version": 2, + "fixtures": { + "bbt_export_absolute.json": { + "used_by": ["test_path_normalization.py", "test_pdf_resolver.py"], + "generated": "2026-05-01", + "desc": "BBT export with absolute Windows paths" + }, + "blank.pdf": { + "used_by": ["test_ocr_preflight.py", "test_pdf_resolver.py"], + "generated": "2026-04-15", + "desc": "Minimal 1-page PDF for resolver tests" + } + } + } + ``` + Add a CI check: `python scripts/validate_fixtures.py` that verifies every fixture is used by at least one test and every test's referenced fixtures exist. + +4. **For OCR fixture data, use synthetic (tiny) JSON instead of real OCR dumps.** The real OCR result for one page of "hello world" could be 2MB of JSON. A synthetic version is 200 bytes: + ```python + def make_ocr_result(pages: int = 1) -> dict: + return { + "layoutParsingResults": [ + { + "prunedResult": { + "page_count": pages, + "parsing_res_list": [ + {"block_label": "text", "block_content": f"Page {i} content", + "block_bbox": [10, 10, 100, 30], "block_id": 1} + for i in range(pages) + ] + } + } + ] + } + ``` + +5. **Version the BBT fixture JSONs with the schema they test.** When BBT changes its export format (different key names, different path format), add a new fixture (e.g., `bbt_export_v3_mixed.json`) rather than modifying existing ones. Tests that exercise specific versions reference them explicitly. **Warning signs:** -- Dashboard flashes global mode briefly when switching to deep-reading -- `_switchMode` is called 2-3 times for a single user click -- Console logs show rapid mode sequence: `paper → global → deep-reading` or `deep-reading → paper → deep-reading` -- Split-pane usage causes dashboard to flicker between modes +- `tests/fixtures/` is over 10MB in git +- "Why is this 50MB JSON file here?" isn't answerable +- A fixture file hasn't been referenced by any test import in 6 months +- `git blame` on fixture files shows 15 different authors adding papers +- CI cache for fixtures takes longer than running the tests -**Phase to address:** Phase 1 (Deep-reading dashboard mode) +**Phase to address:** Phase 6 (Golden dataset + fixtures) — establish manifest and generate-from-code policy BEFORE adding fixtures. Review all existing fixtures (current `tests/fixtures/` is 4 files, small — keep it that way). --- -### Pitfall 6: Empty `discussion.json` / missing `ai/` directory breaks dashboard rendering +### Pitfall 8: Version Sync Checking That Tests the Wrong Thing **What goes wrong:** -When the dashboard enters deep-reading mode, it attempts to read `discussion.json` from the paper's `ai/` directory. For papers that haven't had any AI discussions yet: -1. `ai/discussion.json` doesn't exist → `JSON.parse()` throws on undefined/null -2. `ai/` directory doesn't exist → `fs.readFileSync()` throws `ENOENT` -3. `discussion.json` exists but is empty (0 bytes) → `JSON.parse('')` throws `Unexpected end of JSON input` -4. `discussion.json` has `conversations: []` → dashboard should show "No AI discussions yet" empty state +The Level 0 version sync checker (`check_version_sync.py`) compares these version strings: +- `paperforge/__init__.py`: `__version__ = "1.4.17rc3"` +- `manifest.json`: `"version": "1.4.17rc3"` +- `paperforge/plugin/versions.json`: `{"1.4.17rc3": "1.9.0", ...}` -The current codebase has good empty state handling via `_renderEmptyState()` (line 405-409) and `_renderSkeleton()` (line 399-401), but only for the global/paper/collection modes. The deep-reading mode renderer must handle all four empty states explicitly. +The check looks for exact string matches. But: +1. `manifest.json` has `"version": "1.4.17rc3"` while `__init__.py` has `__version__ = "1.4.17rc3"` — these are the same semver but `rc3` in the version string is printed in two different contexts where `rc3` has different meaning (Python packages can't use `rc` suffix with some build backends) +2. `versions.json` has version ranges (`"1.4.17rc3"`) that may not exactly match the release tag (`v1.4.17`) +3. The CI matrix installs from `pip install -e .` which resolves the version from `paperforge/__init__.py` — but a developer who runs `pip install git+https://...@v1.4.16` might have a different installed version than the source code version + +The version check passes but the actual deployed artifact has a different version. The test checks the right files but validates the wrong invariant. **Why it happens:** -Dashboard mode renderers often assume data exists. The first implementation skips the "file doesn't exist" check because the developer tested with a paper that already had discussions. +Version is distributed across 4+ files (`__init__.py`, `manifest.json`, `versions.json`, `pyproject.toml` dynamic attr), all set independently. The `bump.py` script updates all of them, but manual edits, cherry-picks, or hotfix branches can miss one. The natural reaction is "add more checks" but this creates a false sense of security. **How to avoid:** -- Write a `_loadDiscussionData(key)` helper that returns a discriminated union: - ```js - _loadDiscussionData(key) { - const discussionPath = this._resolveDiscussionPath(key); - if (!discussionPath) return { status: 'no_ai_dir' }; - try { - const raw = fs.readFileSync(discussionPath, 'utf-8'); - if (!raw.trim()) return { status: 'empty' }; - const data = JSON.parse(raw); - if (!data.conversations || data.conversations.length === 0) { - return { status: 'no_discussions' }; - } - return { status: 'ok', data }; - } catch (e) { - if (e.code === 'ENOENT') return { status: 'not_found' }; - return { status: 'parse_error', error: e.message }; - } - } - ``` -- Render distinct UI for each status: - - `no_ai_dir` / `not_found`: "This paper hasn't been discussed with AI yet. Use /pf-paper or /pf-deep to start." - - `empty`: Same as above (file exists but empty = never started) - - `no_discussions`: Same as above (conversations array is empty) - - `parse_error`: "Discussion data is corrupted. Re-run /pf-paper to regenerate." - - `ok`: Render Q&A history with Pass 1 summary + +1. **Check DERIVED versions, not just declared versions.** The real question isn't "do all files have the same version string?" but "can the installed package report itself correctly?": + ```python + def test_installed_version_matches_source(): + """pip-installed package reports the same version as __init__.py.""" + import paperforge + source_version = paperforge.__version__ + result = subprocess.run( + [sys.executable, "-m", "paperforge", "--version"], + capture_output=True, text=True + ) + installed_version = result.stdout.strip() + assert source_version == installed_version, ( + f"Source v{source_version} != installed v{installed_version}" + ) + ``` + +2. **Validate version schema (semver), not exact string equality.** `1.4.17rc3` and `1.4.17-rc3` are semantically equivalent but string-different: + ```python + import re + + SEMVER_PATTERN = re.compile(r"^\d+\.\d+\.\d+(-[a-zA-Z0-9.]+)?$") + + def test_all_versions_are_valid_semver(): + import paperforge + manifest = json.loads(Path("manifest.json").read_text()) + assert SEMVER_PATTERN.match(paperforge.__version__) + assert SEMVER_PATTERN.match(manifest["version"]) + ``` + +3. **Test the `paperforge --version` CLI output** — this is what users and plugins actually see. If the CLI reports the wrong version, the file-level consistency doesn't matter: + ```python + def test_cli_version(): + result = subprocess.run( + [sys.executable, "-m", "paperforge", "--version"], + capture_output=True, text=True, timeout=10 + ) + assert result.returncode == 0 + assert re.match(r"paperforge \d+\.\d+\.\d+", result.stdout) + ``` + +4. **Remove version from `paperforge.json`.** Currently `paperforge.json` stores `"version": "1.2.0"` (hardcoded in `conftest.py:50`). This is a build-time artifact, not a runtime state. The worker scripts should read the installed package version, not a config file version. Remove this field from the config schema to eliminate one source of drift. + +5. **Automate version bump as a CI action, not a script.** When merging to `main`, CI should: + - Read `__init__.py` version + - Verify it matches `manifest.json` and `versions.json` + - Tag the release with `v{version}` + - If versions mismatch, FAIL THE BUILD **Warning signs:** -- Dashboard shows raw error text for new papers without AI discussions -- `Uncaught TypeError: Cannot read property 'conversations' of null` in console -- Users think AI discussion feature is broken when actually it's just never been used +- Version numbers differ between `__init__.py` and `manifest.json` after a cherry-pick +- `paperforge --version` reports a different version than what's in the UI dashboard +- A user reports "I installed v1.4.17 but the plugin says v1.4.16" +- `versions.json` is manually edited without running `bump.py` -**Phase to address:** Phase 1 (Deep-reading dashboard mode) + Phase 2 (AI discussion recorder) — both must handle empty states +**Phase to address:** Phase 1 (Level 0: Version sync checker) — test installed version, not file-level string equality. This is the foundation all other layers depend on. --- -### Pitfall 7: "Jump to Deep Reading" button assumes `deep-reading.md` exists at a fixed path +### Pitfall 9: CLI --json Contract Tests That Check Shape But Not Semantics **What goes wrong:** -The per-paper dashboard card renders a "Jump to Deep Reading" button that navigates to `Literature// - /deep-reading.md`. But: -1. Deep reading may not have been performed yet → file doesn't exist → `openLinkText` silently fails or opens an empty new note -2. The paper title may have changed → path is stale -3. The `domain` may have changed → path is stale -4. The `key` is correct but the file is in a different location (vault restructured) - -**Why it happens:** -Path construction from canonical index fields (`entry.domain`, `entry.title`, `key`) assumes those fields are immutable and the file structure is static. But Zotero metadata updates, title normalization, or vault reorganization can break the assumption. - -**How to avoid:** -- Store the `deep_reading_note_path` in the canonical index (`formal-library.json`) as a machine-maintained field, similar to how `fulltext_path` is already stored -- Before navigating, verify file existence: - ```js - _openDeepReading(key) { - const entry = this._findEntry(key); - if (!entry?.deep_reading_note_path) { - new Notice('Deep reading not yet generated for this paper. Run /pf-deep first.', 6000); - return; - } - const file = this.app.vault.getAbstractFileByPath(entry.deep_reading_note_path); - if (!file) { - new Notice('Deep reading file not found. It may have been moved or deleted.', 6000); - return; - } - this.app.workspace.openLinkText(file.path, ''); - } - ``` -- Add `deep_reading_note_path` to the canonical index schema in the `paths` section -- Update the index builder to populate this field from the file system (check if `deep-reading.md` exists in the paper workspace) - -**Warning signs:** -- Button navigation opens wrong file or nothing happens -- Users report "Jump to Deep Reading does nothing" but only on specific papers -- Console shows `getAbstractFileByPath` returning null -- Button is shown even when deep reading hasn't been performed - -**Phase to address:** Phase 1 (Deep-reading dashboard mode) - ---- - -### Pitfall 8: Version number display race condition — plugin loads before Python reports version - -**What goes wrong:** -The version badge (`v—` placeholder, line 281) is populated from `_fetchStats()` which reads `formal-library.json` or falls back to `python -m paperforge status --json`. The badge starts as `v—` (em dash placeholder) and only updates after the first successful stats fetch. If `formal-library.json` is missing (fresh install, no sync yet), AND the Python fallback fails (Python not installed, `paperforge` not installed), the version stays as `v—` permanently. - -**Why it happens:** -The version is derived from runtime data, not from static plugin metadata. The plugin has its own version in `manifest.json` but doesn't use it because the plugin version != the Python package version. The Python package version is authoritative (only it knows the CLI features available), but reaching it requires a subprocess. - -**How to avoid:** -- **Primary source:** Read version from `paperforge/__init__.py` or `paperforge/plugin/manifest.json` at plugin load time via a fast synchronous fallback file -- **Show version badge immediately** with plugin manifest version as a floor: - ```js - // In onOpen(): - this._versionBadge = headerLeft.createEl('span', { ... }); - // Try to get Python version immediately - this._loadVersion(); - - _loadVersion() { - // Fast path: read from a cached version file written by setup - const vp = this.app.vault.adapter.basePath; - const versionPath = path.join(vp, systemDir, 'PaperForge', 'indexes', '.version'); - try { - const v = fs.readFileSync(versionPath, 'utf-8').trim(); - if (v) { this._versionBadge.setText('v' + v); return; } - } catch {} - // Fallback: spawn python -m paperforge --version (fast, single-line output) - exec('python -m paperforge --version', { cwd: vp, timeout: 5000 }, (err, stdout) => { - if (!err && stdout.trim()) { - this._versionBadge.setText('v' + stdout.trim()); - } - }); - } - ``` -- The `_fetchStats()` should update the version only if it's newer/more specific than what's already shown (never step backward) - -**Warning signs:** -- Version badge shows `v—` even after successful sync -- Version badge flickers between `v—` and `v1.8.0` during mode switches -- `_renderStats()` resets version to `—` on error paths (line 359: `version: this._cachedStats?.version || '\u2014'`) - -**Phase to address:** Phase 4 (Bug fixes) - ---- - -### Pitfall 9: Removing "ai" UI row without checking all render paths - -**What goes wrong:** -The bug fix says "remove meaningless 'ai' row." But the rendering code has multiple paths: -1. `_renderGlobalMode()` — renders OCR section only -2. `_renderPaperMode()` — renders paper view -3. `_renderCollectionMode()` — renders collection view -4. `_renderModeHeader()` — renders header badge -5. Settings tab display — shows config summary - -If "ai" refers to a row in the settings config summary (line 1397), removing it from one path but not others leaves a partial fix. If "ai" is an action card in `ACTIONS` (line 157-208), removing the `paperforge-copy-context` entry would break the contextual action buttons in `_renderPaperMode()`. - -**Why it happens:** -Bug fix descriptions are under-specified. "Remove the 'ai' row" could mean: -- Remove "Copy Context" action (but it's used in per-paper mode) -- Remove a config summary row showing `ai/` directory path -- Remove some other "ai" label - -Without tracing all render paths, the fix risks breaking dependent features. - -**How to avoid:** -- Before making changes, grep for "ai" across all plugin JS, CSS, and i18n strings: - ```bash - grep -n -i '"ai"' paperforge/plugin/main.js - grep -n -i 'ai' paperforge/plugin/styles.css - ``` -- Identify which specific UI element is the "meaningless ai row" -- Create a mini-checklist of all rendering paths that contain or reference "ai" -- After removal, verify no other features broke (Copy Context still works, per-paper actions still render) -- Add a CSS class scope guard so removal is surgical, not global - -**Warning signs:** -- After the fix, Copy Context buttons disappear from per-paper mode -- Config summary in settings tab loses rows it shouldn't -- User reports "Some buttons are gone" after update - -**Phase to address:** Phase 4 (Bug fixes) — must be the LAST fix applied after all features are stable - ---- - -## Moderate Pitfalls - -### Pitfall 10: `discussion.md` Q&A format inconsistency between human-readable and machine-parseable - -**What goes wrong:** -`discussion.md` is designed as human-readable Q&A format. `discussion.json` is the machine-consumable version. If they disagree (extra entries in `.md`, missing entries in `.json`, different timestamps), the dashboard shows one thing but the markdown file shows another. The user becomes confused about which is authoritative. - -**Prevention:** -- Define `discussion.json` as the **canonical source** for dashboard rendering -- Define `discussion.md` as a **derived view** (always generated from `.json`) -- Never write to `.md` without writing to `.json` first -- Add a validation check in `doctor` that compares `.md` and `.json` consistency -- **Do not support** "hand-edited `discussion.md`" as a workflow — the `.json` always wins - -**Mitigation phase:** Phase 2 (AI discussion recorder) - -### Pitfall 11: Tabs/spaces in `discussion.md` breaking Obsidian callout rendering - -**What goes wrong:** -Python generates `discussion.md` with Q&A entries formatted as Obsidian callouts: -```markdown -> [!question]- What is the main finding of this paper? -> The paper found that ACL reconstruction using hamstring autograft... +The CLI --json contract tests validate that output is valid JSON with expected keys: +```python +def test_paths_json_structure(mock_vault): + data = json.loads(output) + required_keys = {"vault", "worker_script", "ld_deep_script"} + for key in required_keys: + assert key in data ``` -If Python uses tabs for indentation, or the markdown has inconsistent newline handling (Windows `\r\n` vs Unix `\n`), Obsidian may not render the callout correctly. Chinese text with certain punctuation at line boundaries can also break callout syntax. +But this only checks that the keys EXIST, not that the VALUES are correct. A refactoring that swaps `vault` and `worker_script` values would pass this test. Worse, if a value contains an unresolved template token like `<system_dir>`, the key exists so the test passes, but the output is broken for consumers. -**Prevention:** -- Use `\n` consistently (Python writes with `newline='\n'` even on Windows) -- Avoid tabs in generated markdown content -- Test callout rendering with Chinese text containing full-width punctuation (`。`, `，`, `：`) -- Provide a regex-based callout validator that catches common formatting issues +The existing test `test_paths_json_no_unresolved_tokens` catches `<system_dir>` placeholders, but there's no equivalent check for `<<obsidian_template>>`, `{resources_dir}`, or other token patterns that might leak into output. -**Mitigation phase:** Phase 2 (AI discussion recorder) +**Why it happens:** +JSON output tests are easy to write shallowly: parse JSON, assert key exists. Deep validation (value shape, data type, semantic correctness, cross-field consistency) is harder and more verbose, so tests stop at "valid JSON with the right keys." The same pattern appears in `test_e2e_cli.py` which checks `total_papers >= 1` but not that the count matches actual papers. -### Pitfall 12: Debounce timer leak when deep-reading mode content refreshes frequently +**How to avoid:** -**What goes wrong:** -If the Python agent is actively writing to `discussion.json` (streaming Q&A entries during an active `/pf-deep` session), the vault `modify` event fires for each write. The current 300ms debounce and mode refresh cycle will repeatedly re-render the deep-reading dashboard, causing: -1. UI jank (constant re-rendering) -2. Flicker of the "switching" CSS class -3. Lost scroll position in the Q&A history view -4. Increased CPU usage from repeated `JSON.parse()` of growing `discussion.json` +1. **Validate VALUE TYPES, not just key presence:** + ```python + def test_paths_json_value_types(mock_vault): + data = json.loads(get_output()) + assert isinstance(data["vault"], str) + assert data["vault"].endswith("mock_vault_name") + assert isinstance(data["worker_script"], str) + assert data["worker_script"].endswith(".py") + ``` -**Prevention:** -- Add a **cooldown period** after a refresh: don't refresh again within 2 seconds of the last refresh -- Use a **dirty flag** pattern: mark data as stale but defer re-render until user-visible idle -- For `discussion.json`, use a `mtime` comparison: only re-render if the file modification time actually changed (not just any `modify` event) -- Consider `requestIdleCallback` or `setTimeout(fn, 1000)` for non-critical refreshes +2. **Validate cross-field consistency.** If `paths --json` reports 3 worker scripts, the actual files must exist: + ```python + def test_paths_json_scripts_exist(mock_vault): + data = json.loads(get_output()) + for key in ["worker_script", "ld_deep_script"]: + script_path = Path(data[key]) + assert script_path.exists(), f"{key}: {script_path} does not exist" + ``` -**Mitigation phase:** Phase 3 (Integration testing & performance) +3. **For each --json command, maintain a SCHEMA definition** and validate against it: + ```python + # paperforge/schemas.py or inline in test + PATHS_JSON_SCHEMA = { + "type": "object", + "required": ["vault", "worker_script", "ld_deep_script"], + "properties": { + "vault": {"type": "string", "minLength": 1}, + "worker_script": {"type": "string", "pattern": r"\.py$"}, + "ld_deep_script": {"type": "string", "pattern": r"\.py$"}, + } + } + + def test_paths_json_schema(mock_vault): + import jsonschema # or a lightweight dict-based validation + data = json.loads(get_output()) + jsonschema.validate(data, PATHS_JSON_SCHEMA) + ``` -### Pitfall 13: CSS class namespace collision with new deep-reading dashboard classes +4. **Add cross-command consistency tests.** If `status --json` reports `total_papers: 3`, then `paths --json` must report paths that those papers can be found at: + ```python + def test_status_and_paths_consistent(mock_vault): + status_data = json.loads(run_command(["status", "--json"])) + paths_data = json.loads(run_command(["paths", "--json"])) + # If papers exist, paths must include literature dir + if status_data["total_papers"] > 0: + assert "Literature" in paths_data["literature"] + ``` -**What goes wrong:** -The current CSS file (`styles.css`) is 1325 lines with sections 1-17. Adding deep-reading dashboard CSS (section 18+) must not accidentally override existing classes like `.paperforge-paper-view`, `.paperforge-header`, or `.paperforge-metric-card`. If a new selector is too broad (e.g., `.paperforge-content-area div`), it will affect the global, paper, and collection mode renders. +5. **Treat --json output as a THEOREM, not just data.** Every --json output should be: + - Valid JSON (parses correctly) + - Type-correct (every value has the expected type) + - Referentially valid (all paths point to existing files) + - Cross-consistent (multiple commands agree) -**Prevention:** -- Use **strictly scoped class names**: `paperforge-deepreading-*` prefix for all deep-reading dashboard CSS -- Avoid inheritance-based selectors; prefer explicit class chaining -- Add a **mode-specific wrapper class** on the content area: `this._contentEl.addClass('paperforge-mode-deepreading')` -- Scope all deep-reading styles under `.paperforge-mode-deepreading` -- Run a CSS diff against previous version: ensure no existing class definitions were modified +**Warning signs:** +- A `--json` test fails because a value is `None` instead of a path string +- A `--json` test passes but the output contains "ERROR:" text in a value +- JSON output changes format between releases but --json tests still pass +- Output contains resolved paths but points to non-existent files +- Two different CLI commands report contradictory information -**Mitigation phase:** Phase 1 (Deep-reading dashboard mode) +**Phase to address:** Phase 2 (CLI contract tests) — define JSON output schema first, implement validation, then write the tests. This is the contract that plugin consumers depend on. --- -## Minor Pitfalls - -### Pitfall 14: Hardcoded `ai/` directory path in plugin JS +### Pitfall 10: Plugin-Backend Integration Tests That Test Subprocess Mechanics Instead of Behavior **What goes wrong:** -The plugin hardcodes `ai/` as the discussion directory, but the actual directory is configurable via `paperforge.json` path settings. If a user changes the workspace layout, the plugin can't find discussion files. +The plugin-backend integration tests test THAT a subprocess spawns, not WHAT the subprocess produces: +```python +def test_plugin_invokes_sync(): + result = subprocess.run([sys.executable, "-m", "paperforge", "sync"], + cwd=test_vault, capture_output=True, timeout=30) + assert result.returncode == 0 +``` -**Prevention:** -- Use the workspace path from `paperforge.json` (already loaded by `readPaperforgeJson()`) -- The `ai/` directory is always `{paper_workspace}/ai/`, and the paper workspace path comes from canonical index `paths.workspace` -- Never concatenate string segments manually; use a path resolver function +This test passes if the subprocess runs and exits 0. But it doesn't test: +- Did `sync` actually write the expected files? +- Did the output conform to the format the plugin expects? +- Did the exit code 0 come from a successful sync or from an early return in an error path? -**Mitigation phase:** Phase 2 (AI discussion recorder) +The same pattern exists in `test_plugin_install_bootstrap.py` which reads `main.js` source code, asserts strings are present, but never actually RUNS the plugin: +```python +def test_setup_args_global_vault_before_subcommand(): + source = PLUGIN_MAIN.read_text(encoding="utf-8") + vault_pos = source.find("'--vault'") + setup_pos = source.find("'setup'") + assert vault_pos < setup_pos +``` -### Pitfall 15: `discussion.md` numbering conflicts with existing markdown headings in formal notes +This verifies source code structure, not runtime behavior. A minifier or bundler that reorders string literals would break these tests even though the plugin works correctly. + +**Why it happens:** +The Obsidian plugin can't be tested in a headless CI environment without Obsidian itself (no Obsidian headless mode exists). So plugin-backend tests fall back to: +1. Parsing plugin source code for expected patterns (tests file structure, not runtime behavior) +2. Running subprocess commands (tests Python, not the plugin) +3. Writing integration tests that only exercise the Python layer and assume the plugin will work + +None of these actually test the plugin-backend boundary where the real bugs live (argument passing, output parsing, error handling). + +**How to avoid:** + +1. **Extract a thin JS module for testing** that can run in Node.js without Obsidian: + ```javascript + // paperforge/plugin/paperforge-backend.js (testable without Obsidian) + const { execSync } = require('child_process'); + const path = require('path'); + + class PaperForgeBackend { + constructor(vaultPath) { this.vaultPath = vaultPath; } + + runCommand(args) { + return execSync( + `python -m paperforge ${args.join(' ')}`, + { cwd: this.vaultPath, encoding: 'utf-8', timeout: 30000 } + ); + } + + parseJsonOutput(raw) { + try { return JSON.parse(raw); } + catch { return { error: 'invalid JSON', raw }; } + } + } + module.exports = { PaperForgeBackend }; + ``` + Test this with Node.js in CI (no Obsidian needed): + ```javascript + // tests/plugin/backend.test.js + const { PaperForgeBackend } = require('../../paperforge/plugin/paperforge-backend'); + + test('sync produces index file', () => { + const backend = new PaperForgeBackend(testVaultPath); + backend.runCommand(['sync']); + const output = backend.runCommand(['status', '--json']); + const data = backend.parseJsonOutput(output); + expect(data).toHaveProperty('total_papers'); + }); + ``` + +2. **For the remaining plugin JS code, use source-level assertions only for CONSTANTS** (command names, file paths) that don't change between dev and production: + ```javascript + assert(MAIN_SOURCE.includes('"sync"'), "Plugin must reference 'sync' command"); + ``` + But test BEHAVIOR through the extracted module, not through source string checks. + +3. **Mock the Obsidian plugin API for comprehensive testing.** Use a test harness like `jest` + `obsidian-typings` to run the plugin code: + ```javascript + // tests/plugin/harness.js + const { App, Plugin, Workspace } = require('obsidian-typings/mock'); + + function createTestApp(vaultPath) { + const app = new App(); + app.vault.adapter.basePath = vaultPath; + return app; + } + ``` + +**Warning signs:** +- Tests read `main.js` source code and check for string patterns +- Plugin tests never actually instantiate any plugin class +- Plugin-backend boundary (argument passing, output parsing) has 0 test coverage +- A change to the plugin's `setNotice()` method breaks 5 string-matching tests +- Subprocess tests pass but the plugin dashboard shows no data + +**Phase to address:** Phase 3 (Plugin-backend integration) — extract testable module first, then write backend tests. Source-code assertions ONLY for stable constants. + +--- + +### Pitfall 11: Test Layering Conflicts — Different Levels Testing the Same Code in Contradictory Ways **What goes wrong:** -If `discussion.md` uses `##` headings that match formal note headings (e.g., `## Methods`, `## Results`), and the user embeds `discussion.md` into the formal note via Obsidian `![[embedded]]` syntax, heading collisions cause broken navigation in Obsidian's outline view. +Level 1 (Python unit tests) patches `requests.post` to return a mock response. Level 2 (CLI contract) runs `paperforge sync` with `--json` and asserts on stdout. Level 4 (temp vault E2E) creates a vault and runs the full pipeline. All three test the `sync` worker, but: -**Prevention:** -- Use a unique heading prefix for discussion entries: `## 💬 AI Discussion` or `## 🤖 Agent Q&A` -- Document the expected embedding pattern in AGENTS.md -- Keep discussion headings distinct from paper content headings +- Level 1 test passes because the mock returns `{"data": {"jobId": "job-123"}}` +- Level 2 test passes because `sync --json` returns `{"synced": 1, "errors": []}` +- Level 4 test fails because the REAL PaddleOCR API returns `{"status": "error", "message": "..."}` +- Developer says "all tests pass" but the feature is broken +- Nobody knows WHICH level is the authoritative source of truth for "does sync work?" -**Mitigation phase:** Phase 2 (AI discussion recorder) +**Why it happens:** +Each test level is written independently by different developers or at different times. There's no cross-reference between levels. Level 1 mocks are not validated against Level 4 real behavior. Level 2 contract assertions don't reference Level 1 unit test assertions. The test pyramid is a collection of silos, not a hierarchy. + +**How to avoid:** + +1. **Define a "truth hierarchy":** + - Level 4 (temp vault E2E) is the AUTHORITATIVE truth for "does this feature work?" + - Level 2 (CLI contract) validates that the CLI layer translates correctly (relies on Level 4 being green) + - Level 1 (unit tests) validates edge cases and internal logic (relies on Level 4 defining the "happy path" contract) + - **Level 1 tests must be red when Level 4 tests are red** (if E2E fails, unit tests should also fail for the same feature) + +2. **Validate mocks against real behavior.** When Level 4 tests pass, extract the real output and use it to update Level 1 mock fixtures: + ```python + # In Level 4 test: + real_output = subprocess.run([...], capture_output=True) + SNAPSHOTS["sync_stdout"].save(real_output.stdout) + + # Level 1 reads: + FIXTURE_SYNC_OUTPUT = SNAPSHOTS["sync_stdout"].load() + ``` + +3. **Add a "consistency audit" test that runs periodically** and compares Level 1 mock expectations with Level 4 real output: + ```python + @pytest.mark.slow + @pytest.mark.consistency + def test_unit_mocks_match_real_output(): + """Verify that Level 1 mock fixtures match real OLD IS GOLD output.""" + expected = run_full_pipeline() # Real E2E run + mocked = run_with_unit_mocks() # Same scenario, mocked + # Compare keys and types, not exact values + assert set(expected.keys()) == set(mocked.keys()) + ``` + +4. **Tag tests with the level they belong to** and use `pytest --strict-markers`: + ```python + @pytest.mark.level1 + def test_sync_unit(): + ... + + @pytest.mark.level4 + def test_sync_e2e(): + ... + ``` + Run Level 1 ALWAYS, Level 4 on PRs only, and add a CI check: `pytest -m "level4"` must not fail when `pytest -m "level1"` passes (but the reverse isn't required — Level 1 can catch things Level 4 doesn't). + +**Warning signs:** +- Level 1 mocks return data that Level 4 would never produce +- A Level 4 test passes but the feature doesn't work for users +- "All tests pass" said confidently right before a production outage +- Adding a new feature requires updating 3 different test suites independently +- Test flakiness correlates with mock data staleness + +**Phase to address:** Phase 7 (CI expansion + integration) — establish truth hierarchy and mock validation BEFORE tests are written at all layers. The roadmap phases (1-6) must reference each other's contracts. + +--- + +### Pitfall 12: Windows-Specific Path Hell in Temp Vault and CLI Tests + +**What goes wrong:** +The temp vault fixture creates paths like `tmp_path / "99_System" / "PaperForge" / "exports"`. On Linux/macOS, this produces `/tmp/.../99_System/PaperForge/exports`. On Windows, this produces `C:\Users\...\AppData\Local\Temp\...\99_System\PaperForge\exports`. + +The code uses `Path.resolve()` which resolves symlinks (on macOS, `/var` -> `/private/var`, breaking expected paths). On Windows, junctions (which PaperForge uses for Zotero data linking) behave differently from symlinks — `Path.is_dir()` returns `True` for junctions, but `shutil.rmtree()` can fail on junctions. + +Existing patterns like `assert "\\" not in pdf_link` (test_e2e_pipeline.py:85) explicitly check for forward slashes — but this assertion doesn't test actual path behavior, only the wikilink rendering. Real path resolution behavior between platforms is untested. + +**Why it happens:** +The existing codebase has reasonable cross-platform awareness (forward slashes in wikilinks, `Path` objects everywhere), but the TESTS make platform-specific assumptions: +- `conftest.py:151`: `zotero_dir = system_dir / "Zotero" / "TSTONE001"` — works everywhere but different absolute paths per platform +- `test_e2e_cli.py:42`: `idx = test_vault / "99_System" / "PaperForge" / "indexes" / "formal-library.json"` — these paths come from config, not magic, so they should work, but the tests don't verify that CLI commands run FROM the vault directory resolve these correctly +- Junction behavior (Zotero data linking) is entirely untested and different on every platform + +**How to avoid:** + +1. **Test path resolution with synthetic constraints, not real filesystems.** Create a mock vault with an "impossible" structure and verify the resolver handles it: + ```python + def test_path_resolver_windows_to_posix_conversion(): + """Verify resolver converts Windows paths to POSIX-style wikilinks.""" + paths = {"vault": Path("/tmp/vault"), "zotero": Path("/tmp/vault/Zotero")} + pdf_path = "D:\\Zotero\\storage\\KEY\\file.pdf" # raw Windows path from BBT + resolved = resolve_pdf_path(pdf_path, paths) + assert "/" in resolved # no backslashes + assert resolved.startswith("Zotero/storage/KEY/") + ``` + +2. **Run path-specific tests on ALL THREE platforms in CI**, not just the fast Linux-only path. The CI matrix MUST include at least one Windows temp vault E2E test. + +3. **For junction tests, create a synthetic junction using `os.symlink()` with `target_is_directory=True` on Windows** and verify `paperforge doctor` correctly identifies it: + ```python + @pytest.mark.skipif(sys.platform != "win32", reason="Junction test") + def test_junction_detection(): + vault = tmp_path / "vault" + vault.mkdir() + junction_target = tmp_path / "zotero-data" + junction_target.mkdir() + junction_link = vault / "Zotero" + os.symlink(str(junction_target), str(junction_link), target_is_directory=True) + + result = subprocess.run([sys.executable, "-m", "paperforge", "doctor"], + cwd=vault, capture_output=True, text=True, timeout=30) + assert "[OK]" in result.stdout or "Zotero" in result.stdout + ``` + +4. **Normalize paths in ALL test assertions.** Never assert raw path strings across platforms: + ```python + class PathAssertions: + @staticmethod + def is_relative_to(path: Path, base: Path) -> bool: + """Cross-platform relative path check.""" + try: + path.relative_to(base) + return True + except ValueError: + return False + ``` + +5. **Use `pyfakefs` for filesystem-level unit tests** (Level 1) and real filesystem for E2E tests (Level 4). This separates platform-specific concerns from logic tests. + +**Warning signs:** +- Tests pass on macOS but fail on Windows CI +- `PermissionError` during `shutil.rmtree` on Windows +- Path comparisons fail because one is resolved and the other isn't +- Junction creation tests hang or crash on CI +- `NotADirectoryError` when traversing vault structure +- `shutil.copytree` fails on Windows junctions (it tries to follow them) + +**Phase to address:** Phases 4 + 7 (temp vault E2E, CI expansion) — must include at least one Windows CI node. Platform-specific tests marked with `@pytest.mark.skipif`. Path normalization utilities extracted first. + +--- + +### Pitfall 13: Test Double Inheritance (When the Wrong Test Reuses the Wrong Fixture) + +**What goes wrong:** +A Level 4 (temp vault E2E) test needs a "vault with two papers in different domains." The developer finds `test_vault` in `conftest.py` which creates one paper in 骨科 domain. They add a second paper to the fixture. Now ALL 473+ tests create two papers instead of one. The 10 tests that assert `len(items) == 1` break. The developer has to fix 10 tests for a fixture that only one test needed. + +Even worse, a Level 1 (unit test) for path normalization uses `test_vault` because "it's already there." But `test_vault` creates 10 directories, writes 5 files, copies OCR data, creates Zotero storage — all of which are irrelevant to path normalization. The test is now slow and depends on components it shouldn't know about. + +**Why it happens:** +`test_vault` in `conftest.py` is the "convenience fixture" that every test reaches for because it's always available. There's no fixture hierarchy — no "small vault" (just config), "medium vault" (config + one paper), "full vault" (config + papers + OCR + Zotero). The single fixture is a kitchen sink that grows with every new test requirement. + +**How to avoid:** + +1. **Create a fixture HIERARCHY, not a single fixture:** + +```python +# conftest.py + +@pytest.fixture +def empty_vault(tmp_path) -> Path: + """Level 0: Minimal vault with config only. Fast (~10ms).""" + vault = tmp_path / "vault" + vault.mkdir() + write_config(vault, {"system_dir": "99_System", "resources_dir": "03_Resources"}) + return vault + +@pytest.fixture +def config_vault(empty_vault) -> Path: + """Level 1: Vault with directories created. Fast (~30ms).""" + vault = empty_vault + create_directories(vault) + return vault + +@pytest.fixture +def vault_with_export(config_vault) -> Path: + """Level 2: Vault with BBT export JSON. Medium (~50ms).""" + vault = config_vault + install_export_fixture(vault, "骨科.json") + return vault + +@pytest.fixture +def vault_with_ocr(vault_with_export) -> Path: + """Level 3: Vault with OCR data. Slow (~100ms).""" + vault = vault_with_export + install_ocr_fixture(vault, "TSTONE001") + return vault + +@pytest.fixture +def full_test_vault(vault_with_ocr) -> Path: + """Level 4: Complete vault with Zotero storage, formal notes. Slowest (~200ms).""" + vault = vault_with_ocr + install_zotero_storage(vault, "TSTONE001") + create_formal_notes(vault, "TSTONE001") + return vault +``` + +2. **Level 1 tests should use `empty_vault` or `config_vault`**, NOT `full_test_vault`. Enforce this with a CI linter that flags `full_test_vault` usage outside of Level 4 test files. + +3. **When a test needs a specific vault state, compose from the hierarchy**, not by modifying the top-level fixture: + ```python + # CORRECT: Compose from existing fixtures + @pytest.fixture + def vault_with_two_domains(vault_with_export): + vault = vault_with_export + install_export_fixture(vault, "运动医学.json") # add second domain + return vault + ``` + +4. **Add a FAST test marker that excludes all vault fixture setup** for pure unit tests that don't need filesystem access: + ```python + @pytest.mark.fast + @pytest.mark.level1 + def test_slugify_empty_string(): + from paperforge.worker._utils import slugify + assert slugify("") == "" + ``` + +**Warning signs:** +- `test_vault` is imported by tests in 10+ different test files at different levels +- `conftest.py:create_test_vault()` is >150 lines +- Adding a new field to the vault fixture breaks 30 tests in unrelated modules +- A "unit test" takes 200ms+ because it unnecessarily creates a full vault +- Developers can't explain WHY their test needs the full vault (it just does) + +**Phase to address:** Phase 1 (Version sync / fixture cleanup) — refactor vault fixtures FIRST before writing new tests. The fixture hierarchy must be in place before Levels 2-6 add more test files. --- @@ -521,11 +1038,16 @@ If `discussion.md` uses `##` headings that match formal note headings (e.g., `## | Shortcut | Immediate Benefit | Long-term Cost | When Acceptable | |----------|-------------------|----------------|-----------------| -| Use `fs.readFileSync` for `discussion.json` instead of `vault.adapter.read` | Faster, no async overhead | Vault cache inconsistency, missed modify events | Never — discussion.json is vault-internal, use vault API | -| Skip `schema_version` on `discussion.json` (bare array format) | Saves 3 lines of code | Every format change = breaking change | Never — add envelope on first write | -| Hardcode `300ms` debounce everywhere | Simple, works on fast machines | Double-fire of `active-leaf-change` causes mode oscillation on slower systems | Only if tested on representative hardware (HDD, low RAM) | -| Duplicate mode detection logic in `_detectAndSwitch` and debounced handler | Saves extracting a pure function | Bug fix in one place breaks the other | Never — extract `_resolveModeForFile()` from day one | -| Skip empty state handling for new papers | Dashboard renders fine for "test" paper with discussions | Production users see raw errors on first use | Never — handle all four empty states in initial implementation | +| Single `test_vault` fixture for all levels | Quick setup, one fixture to maintain | Test bloat, cross-test coupling, slow execution | Never — use fixture hierarchy from day one | +| Nested `with patch(...)` blocks (12+ deep) | No fixture extraction effort | Impossible to debug, brittle mocks, high cognitive load | Never — extract mock fixtures with `@pytest.fixture` | +| Whole-file JSON snapshot assertions | "Comprehensive" in one line | Brittle, regenerating snapshots hides real regressions | Never — snapshot specific shapes, normalize dynamic fields | +| Full CI matrix (3x3x2) on every push | "Maximum coverage" | 4+ hour CI wait, burned free-tier credits | Only for tagged releases; use path-filtered matrix for push | +| Testing plugin via source code string matching | Simple, no dependencies | Tests file structure not behavior, breaks on minification | Only for stable CONSTANTS (command names, file paths) | +| Hand-written JSON fixture files | Easy to create | Drift from real data, no manifest, nobody knows if still used | Only for 3-5 core fixture files; use generated or downloaded rest | +| Version check as string equality | Trivial implementation | False negatives (semver equivalence), false positives (different build contexts) | Never — validate semver structure and installed version | +| All E2E tests create vault as function-scoped | Perfect isolation | 50x overhead for slow tests | Use session-scoped golden vault + per-test clone | +| CI runs all test levels every time | Simple single command | 45-minute+ CI for a typo fix | Use path filters and `-m "not slow"` for push events | +| Mock responses from memory (guessed shapes) | Fastest to write | Silently wrong mock data, false confidence | Never — capture real responses to fixture files first | --- @@ -533,11 +1055,14 @@ If `discussion.md` uses `##` headings that match formal note headings (e.g., `## | Integration | Common Mistake | Correct Approach | |-------------|----------------|------------------| -| Python → JSON file → Node.js read | Python writes GBK, Node.js reads as UTF-8 → mojibake | Python: `open(path, 'w', encoding='utf-8')`. Node.js: `readFileSync(path, 'utf-8')` with explicit encoding | -| Vault `modify` event → dashboard refresh | Subscribing to ALL modifications → dashboard re-renders on every keystroke | Filter to `discussion.json` and `formal-library.json` only; add mtime check to avoid redundant re-renders | -| Canary index `paths` → file navigation | Computing paths from `domain + title + key` → breaks on rename | Store resolved paths in canonical index; verify existence before navigating | -| Obsidian `openLinkText` for deep-reading file | Calling without existence check → silent failure or empty note creation | Check `getAbstractFileByPath` first; show clear Notice on missing file | -| `active-leaf-change` → mode detection | Handling raw event without debounce or dedup → mode oscillation | Debounce 500ms + identity check on file path + mode string before committing to switch | +| Python worker -> subprocess -> CLI --json | Testing that subprocess exits 0 but not validating stdout | Validate that CLI --json output is valid, type-correct, referentially consistent | +| Plugin -> `child_process.spawn('python -m paperforge sync')` | Using `cwd: vaultPath` without verifying Python is available | Pre-flight check: `python -c "import paperforge"` before spawning commands | +| BBT JSON export -> Python parser | Assuming BBT JSON format is stable across Biber versions | Version-check the export file; test with multiple BBT export format fixtures | +| PaddleOCR API -> Python OCR worker | Mocking API responses from guessing (not real recordings) | Use VCR.py to record real PaddleOCR responses; validate mocks against recordings | +| OCR output -> formal note frontmatter | Testing OCR parsing and frontmatter writing independently (they disagree) | Always test the full pipeline: OCR output -> formal note -> rendered frontmatter | +| Python version -> plugin manifest version | Checking file-level string equality | Validate with `pip show paperforge` vs `python -m paperforge --version` | +| temp vault -> subprocess -> filesystem | Using `tmp_path` for vault but `Path.cwd()` inside subprocess | Always pass `--vault` or set `cwd` explicitly; never inherit working directory | +| Windows junctions -> Zotero data linking | Testing only on macOS/Linux where symlinks behave differently | Mark junction tests `@pytest.mark.skipif(sys.platform != "win32")` | --- @@ -545,28 +1070,29 @@ If `discussion.md` uses `##` headings that match formal note headings (e.g., `## | Trap | Symptoms | Prevention | When It Breaks | |------|----------|------------|----------------| -| Re-parsing `discussion.json` on every mode refresh | Dashboard lags during active `/pf-deep` session (Python writes Q&A entries rapidly) | Cache parsed `discussion.json` with mtime; only re-parse if mtime changed | 10+ Q&A entries, or slow disk | -| Reading `discussion.md` for dashboard (human-readable, large) | Dashboard load time increases with discussion length | Dashboard reads `.json` only (structured, compact); `.md` is for human reading in Obsidian editor | 50+ Q&A exchanges | -| `_refreshCurrentMode()` on every `modify` event for `discussion.json` | Constant UI rebuild during streaming writes | Add 2-second cooldown after refresh; use dirty flag pattern | Active AI agent writing to discussion | -| Iterating ALL items in canonical index on each deep-reading mode switch | Finding paper entry scans full index array | Use the already-cached `_currentPaperEntry` from mode detection (no re-scan needed) | 1000+ papers in library | +| Function-scoped vault fixture in every E2E test | E2E suite takes 15+ min for 20 tests | Session-scoped golden vault + per-test fast clone | 10+ E2E tests | +| All CI jobs run full matrix | 4+ hour total CI runtime | Path-filtered matrix, level-specific jobs | 3+ CI matrix dimensions | +| Snapshot assertions on full JSON output | PR diffs show 500-line snapshot changes | Shape assertions + normalized fields | First refactor after snapshots are created | +| Mock responses never validated against real API | Tests pass, production breaks silently | Periodic mock-vs-real consistency audit | 2+ months after mocks were created | +| All tests import `test_vault` regardless of need | "Unit tests" take 200ms+ each | Fixture hierarchy: use minimal fixture for each test level | 100+ tests using the heavy fixture | +| `shutil.rmtree` on vault cleanup | Windows CI fails with `PermissionError` | Retry-with-backoff cleanup + ignore_errors | Any Windows CI run | +| User journey tests as part of PR gate | PRs blocked on 20-min unstable tests | Run journey tests nightly only, not on every PR | First flaky journey test | --- ## "Looks Done But Isn't" Checklist -- [ ] **Deep-reading mode:** Opens `deep-reading.md` — verify mode badge says "Deep Reading" not "Paper" -- [ ] **Deep-reading mode:** Opens `deep-reading.md` when BOTH formal note and deep-reading note are open in split panes — verify correct mode per active pane -- [ ] **Deep-reading mode:** Opens `deep-reading.md` for a paper that has NO AI discussions — verify empty state renders, no error -- [ ] **Deep-reading mode:** Opens `deep-reading.md` for a paper with Chinese title — verify Pass 1 summary renders correctly without mojibake -- [ ] **Discussion recording:** Python writes `discussion.json` with Chinese Q&A — verify Node.js reads and displays correctly -- [ ] **Discussion recording:** Python writes `discussion.json` while dashboard is open — verify dashboard refreshes without flicker -- [ ] **Jump button:** Click "Jump to Deep Reading" on a paper that hasn't had deep reading — verify error Notice, no silent failure -- [ ] **Jump button:** Click "Jump to Deep Reading" on a paper that HAS deep reading — verify file opens correctly -- [ ] **Version badge:** Plugin loads with no `paperforge` installed — verify `v—` or plugin manifest version shown, not crash -- [ ] **Version badge:** After sync completes, version is populated from Python — verify it doesn't revert to `v—` on mode switch -- [ ] **"ai" row removal:** All render paths (`_renderPaperMode`, `_renderGlobalMode`, `_renderCollectionMode`, settings tab) still functional -- [ ] **CSS regression:** Deep-reading CSS doesn't break global/paper/collection dashboard layout -- [ ] **Empty state:** Fresh vault, no papers synced — verify deep-reading mode accessible or correctly gated +- [ ] **Level 0 (Version sync):** Runs `python -m paperforge --version` and confirms installed version matches source — but does it also verify `manifest.json` and `versions.json` agree? Could a `pip install` from a different branch silently install the wrong version? +- [ ] **Level 1 (Python unit):** OCR state machine tests mock `requests.post` and `requests.get` — but do the mock values come from a REAL recorded PaddleOCR response, or from guessing the API shape? +- [ ] **Level 2 (CLI --json):** Tests verify JSON has expected keys — but do they verify VALUE TYPES? Could `vault` be `None` or an empty string? +- [ ] **Level 3 (Plugin-backend):** Tests read `main.js` source code for string patterns — but do they actually RUN the plugin or subprocess to verify behavior? +- [ ] **Level 4 (Temp vault E2E):** Vault is created with `conftest.py:create_test_vault()` — but do the tmp_path fixture names (`00_TestVault`) match the CONFIGURABLE directory names, and would the test break if someone changes `default_config`? +- [ ] **Level 5 (User journey):** Journey is documented in prose — but is there a concrete, executable script that implements each step? +- [ ] **Level 6 (Destructive):** Tests use `isolated_destruct_vault` — but is there a SAFETY CONTRACT at the top of each destructive test that verifies the vault is a temp directory? +- [ ] **Golden datasets:** `tests/fixtures/` has manifest.json — but does every fixture have a `used_by` list that's validated by CI? +- [ ] **Snapshot tests:** Snapshots use inline snapshot or normalized-shape assertions — or are they whole-file JSON compares that will break on every generated_at timestamp change? +- [ ] **CI matrix:** Config has path-filtered jobs — but do the path filters cover ALL relevant file patterns (`.py`, `.js`, `.json`, `.yaml`)? +- [ ] **Cross-platform:** Tests pass on macOS — but is there a Windows CI node running the path-specific E2E tests, including junction tests? --- @@ -574,15 +1100,19 @@ If `discussion.md` uses `##` headings that match formal note headings (e.g., `## | Pitfall | Recovery Cost | Recovery Steps | |---------|---------------|----------------| -| Mode ordering regression (deep-reading → paper) | LOW | Reorder mode detection: check `basename === 'deep-reading'` first. Single-function fix. | -| `discussion.json` GBK encoding corruption | MEDIUM | Re-run `/pf-paper` to regenerate with explicit UTF-8. Fix the Python writer. Existing files: detect via hex inspection and re-encode. | -| `discussion.json` schema version missing | HIGH | Write migration script: detect bare array format, wrap in envelope with `schema_version: "1"`. Must preserve all existing data. | -| `btoa()` crash on Chinese paths | LOW | Replace with `Buffer.from(str).toString('base64')`. Single-line fix per occurrence. | -| Empty `discussion.json` crashes dashboard | LOW | Add file-existence and content-validity guards. Four `if` branches in loader function. | -| Version badge permanently `v—` | LOW | Add fast synchronous version file cache; add `paperforge --version` fallback. | -| `active-leaf-change` mode oscillation | MEDIUM | Extract `_resolveModeForFile()`, add file identity guard, adjust debounce timing. Requires careful testing with split panes. | -| CSS namespace collision | MEDIUM | Add mode-specific wrapper class; audit CSS diff. Roll back CSS section if conflict found. | -| Jump button broken on missing file | LOW | Add `getAbstractFileByPath` existence check before `openLinkText`. | +| Mock drift (P1) — mocks mismatch real API | MEDIUM | Capture real PaddleOCR responses with VCR.py; regenerate all mock fixtures; add periodic consistency audit test | +| CI matrix too slow (P2) — 4+ hour CI | HIGH | Restructure CI into level-specific jobs with path filters; move slow tests to nightly; add `pytest -m "slow"` markers | +| Snapshot brittleness (P3) — failing on every refactor | MEDIUM | Replace whole-file snapshots with shape-specific assertions; add normalization helpers for dynamic fields | +| Temp vault slow (P4) — E2E suite takes 15 min | MEDIUM | Refactor to session-scoped golden vault + per-test fast clone; separate "fast" and "full" E2E | +| Vague journey tests (P5) — can't automate | HIGH | Rewrite UX contracts as concrete step sequences; build step abstraction layer; pick ONE concrete scenario per test | +| Destructive test safety (P6) — real vault at risk | CRITICAL | Add isolation assertion (must be tmp_path); move to Docker-only; add safety contract to every destructive test | +| Fixture bloat (P7) — 100MB in repo | HIGH | Remove git-tracked large fixtures; add `download_fixtures.py` script; generate from code where possible; add manifest validation | +| Version sync mismatch (P8) — wrong installed version | LOW | Add `pip show paperforge` check; validate semver structure; remove version from `paperforge.json` | +| CLI --json shallow tests (P9) — keys but no semantics | LOW | Add `jsonschema` validation; add cross-command consistency tests; validate types and referential integrity | +| Plugin tests too shallow (P10) — source matching | MEDIUM | Extract `paperforge-backend.js` module; write Node.js tests; keep source assertions only for constants | +| Test layering conflicts (P11) — levels disagree | HIGH | Establish truth hierarchy (Level 4 > Level 2 > Level 1); add mock validation against real output; level-tag all tests | +| Windows path hell (P12) — cross-platform failures | MEDIUM | Add synthetic path constraint tests; normalize all path assertions; run at least one Windows CI node | +| Fixture inheritance (P13) — wrong test, wrong fixture | MEDIUM | Extract fixture hierarchy (empty_vault -> config_vault -> vault_with_export -> vault_with_ocr -> full_test_vault); CI-lint heavy fixture usage | --- @@ -590,36 +1120,57 @@ If `discussion.md` uses `##` headings that match formal note headings (e.g., `## | Pitfall | Prevention Phase | Verification | |---------|------------------|--------------| -| Mode detection ordering (P1) | Phase 1 — Deep-reading dashboard mode | Open deep-reading.md; verify mode badge shows "Deep Reading" | -| `fs.readFileSync` bypasses vault API (P2) | Phase 2 — AI discussion recorder | Check file read method in code review; verify modify events trigger refresh | -| `discussion.json` schema version missing (P3) | Phase 2 — AI discussion recorder | Inspect generated discussion.json for `schema_version: "1"` | -| `btoa()` on Chinese (P4) | Phase 2 — AI discussion recorder | Grep plugin JS for `btoa(`; test with Chinese-titled papers | -| `active-leaf-change` double-fire (P5) | Phase 1 — Deep-reading dashboard mode | Click between formal note and deep-reading.md in split panes | -| Empty `discussion.json` states (P6) | Phase 1 + Phase 2 | Test fresh paper with no AI discussions; verify empty state renders | -| Jump button assumes file exists (P7) | Phase 1 — Deep-reading dashboard mode | Click button on paper before running /pf-deep | -| Version badge race (P8) | Phase 4 — Bug fixes | Load plugin with no paperforge CLI; verify graceful fallback | -| "ai" row removal scope (P9) | Phase 4 — Bug fixes | Verify all render paths after removal; check Copy Context still works | -| `.md` vs `.json` inconsistency (P10) | Phase 2 — AI discussion recorder | Run doctor after generating discussions; compare .md and .json | +| Mocking too early/rigidly (P1) | Phase 2 — Python unit tests | Validate mock fixture data came from real API recordings; autospec=True on all mocks | +| CI matrix slow (P2) | Phase 7 — CI expansion | CI completes under 15 min for push events; matrix jobs are partitioned by level | +| Snapshot brittleness (P3) | Phase 6 — Golden datasets | Snapshot assertions use normalized shapes, not whole files; inline-snapshot vs external files | +| Temp vault slow/non-det (P4) | Phase 4 — Temp vault E2E | E2E suite under 5 min; session-scoped golden vault with per-test clone; cross-platform cleanup | +| Vague user journeys (P5) | Phase 5 — User journey tests | Each journey test has exactly ONE concrete scenario; step abstraction layer exists | +| Destructive test safety (P6) | Phase 6 — Destructive/chaos | Safety contract verified by CI; destructive tests run in Docker only | +| Fixture bloat (P7) | Phase 6 — Golden datasets | Fixture manifest exists; CI validates all fixtures are used; generated > hand-written | +| Version sync wrong invariant (P8) | Phase 1 — Version sync | Tests validate installed version, not file string equality; semver schema validated | +| CLI --json shallow tests (P9) | Phase 2 — CLI contract | JSON output validated against schema (keys + types + references + cross-consistency) | +| Plugin subprocess mechanics (P10) | Phase 3 — Plugin-backend | Extract `paperforge-backend.js`; tests run in Node.js without Obsidian | +| Test layering conflicts (P11) | Phase 7 — CI integration | Truth hierarchy documented; mock validation test runs periodically | +| Windows path hell (P12) | Phase 4 + 7 — E2E + CI | Platform-specific tests marked; at least one Windows CI node; path normalization utilities | +| Fixture inheritance wrong (P13) | Phase 1 — Fixture hierarchy | Fixture hierarchy in conftest.py; lint added for heavy fixture usage in level 1 tests | --- ## Sources -- **Existing codebase analysis:** `paperforge/plugin/main.js` (lines 718-764, mode detection; lines 1246-1264, event subscriptions; lines 322, 428, `fs.readFileSync` patterns). Confidence: HIGH. -- **Obsidian Developer Docs:** Event system, `registerEvent()`, `active-leaf-change`, `debounce()` best practices. Context7 library `/obsidianmd/obsidian-developer-docs`. Confidence: HIGH. -- **Obsidian Forum #31841:** `active-leaf-change` event fires twice during tab switches — confirmed by multiple plugin developers. Confidence: HIGH. -- **Obsidian Forum #91927:** GB2312 to UTF-8 conversion corrupting Chinese files permanently — encoding mismatch between system code page and expected encoding. Confidence: HIGH. -- **opencode-obsidian Issue #28:** `btoa()` crash with Chinese characters in paths — `Buffer.from(str).toString('base64')` is the standard fix. Confidence: HIGH. -- **nodejs/undici Issue #5002:** Multi-byte UTF-8 character corruption at chunk boundaries when `setEncoding('utf8')` is used — confirmed in production with CJK text. Confidence: HIGH. -- **paperclipai/paperclip Issue #3940:** GBK-UTF8 encoding mismatch on Windows with CJK child process output — root cause is `iconv-lite` corruption + missing `PYTHONIOENCODING` env var. Confidence: MEDIUM (fix validated, specific library version dependent). -- **Excalidraw plugin commit 5c628e0:** Real-world example of debounce timer cleanup and observer disposal in Obsidian plugin `onunload`. Confidence: HIGH. -- **obsidian-current-view commit ad110f7:** Real-world example of replacing `requestAnimationFrame` with `setTimeout` debounce and building lookup maps once per call. Confidence: HIGH. -- **Templater Issue #1629:** `app.vault.modify()` race condition when multiple handlers modify the same file — relevant for discussion.json concurrent access. Confidence: MEDIUM. -- **Project architecture context:** `.planning/research/ARCHITECTURE.md` — thin-shell plugin principle, canonical index as read model, file ownership boundaries. Confidence: HIGH. -- **Project defect history:** `.planning/research/DEFECTS.md` — encoding issues, path resolution brittleness, worker/plugin boundary problems. Confidence: HIGH. +- **Existing codebase analysis:** `tests/conftest.py` (fixture patterns), `tests/test_ocr_state_machine.py` (12-level mock nesting), `tests/test_e2e_cli.py` (subprocess testing), `tests/test_e2e_pipeline.py` (integration testing), `tests/test_plugin_install_bootstrap.py` (source-code matching). Confidence: HIGH. +- **Existing test suite:** 473+ tests across 39 test files; current patterns are mostly Level 1 units with a few Level 2 and Level 4 tests. The mock-heavy approach and single fixture reveal the pitfalls waiting to happen. Confidence: HIGH. +- **Pydantic inline-snapshot article (2026-02):** Snapshot testing with normalization, dirty-equals for dynamic fields, and the "assert specific shapes not whole files" principle. Confidence: HIGH. +- **pytest documentation:** Good integration practices, fixture scopes, `tmp_path`/`tmp_path_factory`, markers, and import modes. Confidence: HIGH. +- **CircleCI documentation:** Test splitting, parallelism strategies, and timing-based test distribution for CI matrix optimization. Confidence: MEDIUM (applies to GitHub Actions with pattern adaptation). +- **"Why Snapshot Testing Sucks" (2025-01):** Analysis of brittleness, coordination overhead, and recommendation for targeted behavior-driven assertions over whole-file snapshots. Confidence: HIGH. +- **pytest-with-eric.com (2024-2026):** Fixture management, temp directory strategies, test organization, flaky test stabilization — practical patterns for multi-layer test suites. Confidence: MEDIUM (community blog, but patterns are well-established). +- **Project version schema:** `paperforge/__init__.py`, `manifest.json`, `pyproject.toml` — multiple version sources with `bump.py` coordination. Confidence: HIGH. +- **Project constraints:** Windows compatibility requirement, junction/symlink differences, Chinese path handling, cross-platform CI target. Confidence: HIGH. +- **Previous milestone learnings:** v1.3 path normalization, v1.4 utility extraction, v1.9 frontmatter rationalization — each revealed testing blind spots. Confidence: HIGH. +- **Project architecture:** `.planning/ARCHITECTURE.md` — thin-shell plugin principle, canonical index, worker/agent split. Confidence: HIGH. --- -*Pitfalls research for: PaperForge v1.8 — AI Discussion & Deep-Reading Dashboard* -*Researched: 2026-05-06* +## What NOT to Test (Pragmatic Boundaries) + +These are explicitly out of scope for the v2.0 testing infrastructure: + +| Do NOT Test | Rationale | Alternative | +|-------------|-----------|-------------| +| Obsidian API behavior itself | Obsidian is a dependency, not our code. We can't control its behavior. | Test that our plugin correctly INTERPRETS Obsidian's API responses (mock Obsidian API) | +| PaddleOCR API accuracy | We're testing our integration, not OCR quality. | Test that we correctly handle the API's response format and error codes | +| Better BibTeX export correctness | BBT is a Zotero plugin, not our code. | Test that we correctly PARSE BBT's JSON output in all supported formats | +| JavaScript bundler behavior (if used) | Webpack/esbuild are build tools, not our application | Test the BUNDLED OUTPUT, not the bundler | +| Performance regression detection | Falls outside unit/E2E testing scope (needs dedicated benchmarking infrastructure) | Add `pytest-benchmark` markers but don't gate CI on them in v2.0 | +| Zotero's internal data structure | We read BBT's output, not Zotero's internals | Focus testing on BBT JSON parsing, not Zotero database state | +| Python 3.9 or lower | This project requires Python 3.10+ (per pyproject.toml) | Don't add CI nodes for unsupported Python versions | +| Obsidian's frontmatter parsing | Obsidian handles YAML frontmatter — we write valid YAML | Test that our YAML OUTPUT is valid and readable | +| Network latency/resilience of PaddleOCR | Not our infrastructure; we can't control it | Test that we handle timeout errors and retry correctly (mocked) | +| Air-gapped/offline mode | PaperForge requires PaddleOCR API access for full functionality | Focus on testing the internet-available path; offline behavior is a separate feature | + +--- + +*Pitfalls research for: PaperForge v2.0 — Multi-Layer Testing Infrastructure (6-Level Quality Gates)* +*Researched: 2026-05-08* *Research mode: Project Research — PITFALLS* diff --git a/.planning/research/STACK.md b/.planning/research/STACK.md index c325c134..890716f4 100644 --- a/.planning/research/STACK.md +++ b/.planning/research/STACK.md @@ -1,218 +1,81 @@ -# Stack Research — v1.8 AI Discussion Recording & Deep-Reading Dashboard +# Technology Stack: v2.0 Testing Infrastructure -**Domain:** AI discussion recording + deep-reading dashboard mode for PaperForge Obsidian plugin -**Researched:** 2026-05-06 -**Confidence:** HIGH - -## Executive Summary - -v1.8 adds two new capabilities to the existing PaperForge stack: (1) AI discussion recording that captures `/pf-paper` and agent chat interactions into structured `ai/` records, and (2) a 4th dashboard mode activated when the user views `deep-reading.md`. Both features extend the existing thin-shell plugin architecture — the plugin reads JSON from the filesystem and renders Pure CSS/DOM components; the Python side provides templates and optional recording helpers. **No new npm dependencies, no new Python packages.** All additions are new modules/files within the existing architecture. +**Project:** PaperForge v2.0 +**Researched:** 2026-05-08 ## Recommended Stack -### Core Technologies (unchanged from v1.6–v1.7) - +### Core Test Framework | Technology | Version | Purpose | Why | |------------|---------|---------|-----| -| Obsidian CommonJS plugin | existing (`main.js`) | Dashboard/settings/commands UI | Pure Obsidian API — no bundler, no npm deps, no build step | -| Python 3.10+ | existing | Single owner of business logic, templates, schema | Already owns config, lifecycle, health, maturity, index; extends naturally to discussion templates | -| Filesystem JSON | UTF-8, no schema library needed | `discussion.json` per paper | Plugin reads via `fs.readFileSync`, localStorage-free, vault-native, traceable | -| Filesystem Markdown | Obsidian-flavored | `discussion.md` per paper | Human-readable, Obsidian-editable, wikilink-compatible | +| pytest | >=8.0 | Python test framework | Already in use; industry standard for Python | +| pytest-snapshot | >=0.9.0 | Snapshot/approval testing | CLI JSON output stability contracts; simpler than full approval-testing libs | +| pytest-timeout | >=2.2.0 | Test timeout guards | Prevent runaway E2E/chaos tests from blocking CI | +| pytest-mock | >=3.12.0 | Enhanced mocking | Built-in monkeypatch replacement; integrates with `unittest.mock` | +| responses | >=0.25.0 | HTTP mock library | Intercept `requests` library calls at HTTP layer for mock OCR backend | +| coverage | >=7.4.0 | Coverage measurement | Standard Python coverage tool; generates CI reports | -### File Format Additions (NEW for v1.8) +### Plugin Test Framework +| Technology | Version | Purpose | Why | +|------------|---------|---------|-----| +| Vitest | >=2.0 | JS test runner | Native ESM support (plugin uses `require` but can be wrapped); ~2x faster than Jest | +| obsidian-test-mocks | >=0.12 | Obsidian API mocks | Comprehensive mock implementations of `obsidian.d.ts`; 100% code coverage maintained | +| jsdom | >=24.0 | DOM environment | Required by Obsidian plugin tests (document, window access) | +| Node.js | >=20.0 | JS runtime | Required by Vitest and obsidian-test-mocks | -#### 1. `discussion.json` — Structured AI Q&A Record +### CI Infrastructure +| Technology | Version | Purpose | Why | +|------------|---------|---------|-----| +| actions/setup-python | v5 | Python version management | Standard GitHub Action; supports cache, pre-release | +| actions/setup-node | v4 | Node version management | Required for plugin tests | +| actions/checkout | v4 | Source checkout | Latest stable checkout action | +| re-actors/alls-green | v1 | CI gate aggregation | Simplifies "all jobs passed" check with branch protection | -**Location:** `<paper_workspace>/ai/discussion.json` +### Supporting Scripts +| Script | Language | Purpose | +|--------|----------|---------| +| `scripts/check_version_sync.py` | Python | Level 0: verify version consistency across 6+ files | +| `scripts/run_all_tests.sh` | Bash | Sequential runner for local testing | +| `scripts/run_chaos_tests.sh` | Bash | Chaos test runner with scenario selection | +| `scripts/generate_fixtures.py` | Python | Regenerate golden datasets from canonical sources | -**Schema (v1):** +## Alternatives Considered -```json -{ - "schema_version": "1", - "paper_key": "ABCDEFG", - "generated_at": "2026-05-06T12:00:00+08:00", - "source": "/pf-paper", // or "/pf-deep", "agent-chat" - "history": [ - { - "index": 1, - "timestamp": "2026-05-06T12:00:00+08:00", - "question": "这篇论文的主要发现是什么？", - "answer": "该研究发现...", - "tags": ["background", "results"], - "agent_model": "deepseek-v4-pro" - } - ], - "summary": { - "total_qa": 5, - "last_updated": "2026-05-06T12:30:00+08:00", - "top_tags": ["methods", "results"] - } -} -``` - -**Why this shape:** -- `schema_version` for forward compatibility -- `history[]` is a flat append-only list (no nested threads — keeps dashboard rendering simple) -- `index` field enables stable references even if history is reordered -- `tags` enable future dashboard filtering without schema change -- `summary` provides pre-computed metadata the plugin can read with a single parse (avoids looping through all history entries for total count) - -**Integration:** Plugin reads this file directly via `fs.readFileSync` when rendering the deep-reading dashboard. No Python intermediary needed for read path. - -#### 2. `discussion.md` — Human-Readable Q&A Log - -**Location:** `<paper_workspace>/ai/discussion.md` - -**Template format:** - -```markdown -# AI 讨论记录 — 文献标题 - -> **来源:** /pf-paper | **生成时间:** 2026-05-06 12:00 | **模型:** deepseek-v4-pro - ---- - -## 讨论 1 - -**问题:** 这篇论文的主要发现是什么？ - -**解答:** 该研究发现... - -*标签: `background`, `results`* - ---- - -## 讨论 2 - -**问题:** 研究者用了什么统计方法？ - -**解答:** ... - -*标签: `methods`* -``` - -**Why this format:** -- Each discussion is a `## 讨论 N` section — natural Obsidian heading hierarchy -- `**问题:**` and `**解答:**` bold markers are scannable in both Reading and Source mode -- Horizontal rules (`---`) visually separate discussions -- Tags as inline code spans — searchable in Obsidian's global search -- Frontmatter-compatible metadata line at top - -### Plugin JS Additions (NEW for v1.8 — no new npm deps) - -All additions stay within the single `paperforge/plugin/main.js` file and `paperforge/plugin/styles.css`. - -| Module/Function | Type | Purpose | -|---------|------|---------| -| `_detectAndSwitch()` extension | Mode detection logic | Add `deep-reading` as 4th mode: detect when active file path ends with `deep-reading.md` and resolve paper context from workspace path | -| `_renderDeepReadingMode()` | New render function | Renders deep-reading dashboard: status bar, Pass 1 summary, AI discussion history | -| `_renderDiscussionHistory()` | New render function | Reads `discussion.json` from paper `ai/` directory and renders Q&A cards | -| `_readDiscussionJson(key)` | New utility | Resolves `ai_path` → reads `discussion.json` → returns parsed object or null | -| `_renderPass1Summary()` | New render function | Renders Pass 1 overview extracted from deep-reading.md content | -| `_renderPaperMode()` extension | Existing function | Add "Jump to Deep Reading" contextual button when `deep_reading_path` exists | -| `_versionBadge` fix | Bug fix | Restore version display by reading from `paperforge/__init__.py` `__version__` or `manifest.json` | -| "ai" row removal | Bug fix | Remove the meaningless "ai" UI row (track down which render path creates it) | - -**Mode detection logic (how `_detectAndSwitch()` grows):** - -```javascript -// Existing: checks for .base, .md with zotero_key -// NEW: check if active file basename is "deep-reading.md" -if (ext === 'md' && activeFile.basename === 'deep-reading') { - // Resolve paper key from parent directory name - const parentDir = activeFile.parent.path; // e.g., "Literature/骨科/ABC12345 - Title" - const match = parentDir.match(/([A-Z0-9]{8})/); - if (match) { - this._currentPaperKey = match[1]; - this._currentPaperEntry = this._findEntry(match[1]); - this._switchMode('deep-reading'); - return; - } -} -``` - -### CSS Additions (NEW for v1.8) - -All additions go into the existing `paperforge/plugin/styles.css` file after Section 17. - -| CSS Class | Purpose | -|-----------|---------| -| `.paperforge-deep-reading-view` | Root container for deep-reading mode layout (flex column, gap: 20px) | -| `.paperforge-dr-status-bar` | Status indicator bar: shows Pass 1/2/3 completion, OCR status badge, last-updated timestamp | -| `.paperforge-dr-pass-summary` | Pass 1 summary card: bordered card with muted background for the overview text | -| `.paperforge-dr-discussion-card` | Individual Q&A card: question in bold, answer below, tag chips, timestamp | -| `.paperforge-dr-discussion-list` | Scrollable container for stacked discussion cards | -| `.paperforge-dr-tag-chip` | Small pill for tags (font-size: 10px, border-radius: 8px) | -| `.paperforge-dr-empty` | Empty state: "No AI discussions recorded yet" with muted styling | -| `.paperforge-dr-section-title` | Section header within deep-reading view (uppercase, letter-spaced) | - -### Python Additions (NEW for v1.8 — no new dependencies) - -| File | Purpose | -|------|---------| -| `paperforge/worker/discussion.py` | NEW module: `record_discussion(key, vault, question, answer, source, model)` — writes both `discussion.md` and `discussion.json` to paper `ai/` directory | -| `paperforge/worker/discussion.py` | `load_discussion_json(key, vault)` → dict — reads existing discussion.json, returns parsed dict | -| `paperforge/worker/discussion.py` | `append_discussion(key, vault, qa_pair)` — appends to existing discussion history (read-modify-write atomic via tempfile + os.replace) | - -**Why a separate module:** -- Discussion recording crosses the Worker/Agent boundary (Agent generates content, Worker writes files) -- Keeps discussion I/O isolated from OCR, sync, and index logic -- Enables future `/pf-discuss` command or discussion-search features without refactoring - -**No new Python packages required.** All I/O uses `json`, `pathlib`, and `datetime` (stdlib). Atomic write uses the same `tempfile` + `os.replace` pattern already proven in `asset_index.py`. - -### Integration Points (with existing canonical index) - -| Integration | Direction | How | -|-------------|-----------|-----| -| `ai_path` in canonical index | Read by plugin | Already exists: `entry.ai_path` = `"Literature/<domain>/<key> - <Title>/ai/"` | -| Resolve `discussion.json` path | Plugin computed | `path.join(vaultPath, entry.ai_path, 'discussion.json')` | -| Deep-reading mode detection | Plugin computed | Check `activeFile.basename === 'deep-reading'` + resolve key from parent directory | -| Jump-to-deep-reading button visibility | Plugin check | `entry.deep_reading_path` non-empty → show button | -| Discussion recording write path | Python writes | `record_discussion()` resolves paths via `paperforge_paths()` → writes to workspace ai/ | - -## What NOT to Add - -| Category | Avoid | Why | Use Instead | -|----------|-------|-----|-------------| -| JS dependencies | Any npm package (React, Vue, chart libs, date-fns) | Plugin must stay pure Obsidian CommonJS; adding a build chain breaks the single-file deployment model and Obsidian Community Plugin requirements | Pure DOM API (`createEl`, `addClass`, `setText`) | -| Database for discussions | SQLite, IndexedDB, localStorage | Discussions belong to the paper workspace — they must be vault-native, backup-friendly, and git-trackable | Filesystem JSON + Markdown in `ai/` | -| Python dependencies for discussion | `pydantic`, `jsonschema` | Overkill for a flat Q&A list; adds import cost and version coupling for a simple structure | Plain `dict` with documented keys, validated by `assert` in tests | -| Second plugin file | Splitting `main.js` into modules | Adds complexity without benefit at ~2100 lines; Obsidian plugin import resolution is fragile | Inline functions in `main.js` (as all existing render functions are) | -| Schema library in plugin | AJV, Zod, or custom JSON validator | Plugin should read, not validate; schema enforcement lives in Python tests | Trust but handle: `try/catch` around `JSON.parse`, fallback to empty state | -| New CLI command (v1.8) | `paperforge discuss` or `paperforge record` | Discussion recording happens inside Agent sessions; adding a CLI command would require users to manually run it after each Q&A | Python module callable from Agent scripts (`/pf-paper` → calls `record_discussion()` internally) | - -## Version Compatibility - -| Component | Current Version | Notes | -|-----------|----------------|-------| -| PaperForge Python | 1.4.15 (→ 1.8.0) | No breaking changes to existing CLI or index schema | -| Canonical index schema | v2 (`formal-library.json`) | `ai_path` field already present; no schema version bump needed | -| Obsidian API | 1.5+ | Existing API surface used (ItemView, metadataCache, vault.adapter) | -| Node.js (for Obsidian) | 18+ (Electron embedded) | `fs.readFileSync`, `path.join` — all stdlib since Node 0.x | +| Category | Recommended | Alternative | Why Not | +|----------|-------------|-------------|---------| +| HTTP Mock | responses | pytest-httpx | responses is simpler and sufficient for `requests`-based OCR calls; httpx adds unnecessary dependency | +| Plugin Test Mock | obsidian-test-mocks | Manual `__mocks__/obsidian.ts` | Manual obsidian mocking requires constant maintenance as API evolves; obsidian-test-mocks is community-maintained with 100% coverage | +| JS Test Runner | Vitest | Jest | Jest requires transforms for ESM; Vitest is natively ESM-compatible and faster for watch mode | +| Snapshot Testing | pytest-snapshot | syrupy | pytest-snapshot is simpler; syrupy's extra features (serializer plugins) not needed for JSON-only snapshots | +| CI Orchestration | Native GitHub Actions | tox | tox adds complexity for simple package install; actions/setup-python + pip install is more transparent and debuggable | +| Obsidian E2E Testing | Deferred to v2.1 | obsidian-e2e / obsidian-integration-testing | Both require a running Obsidian process, add 3+ min per test, and are flaky in CI. Not worth the cost for current milestone | ## Installation -No new installation steps. v1.8 is additive: +```toml +# pyproject.toml additions +[project.optional-dependencies] +test = [ + "pytest>=7.4.0", + "pytest-snapshot>=0.9.0", + "pytest-timeout>=2.2.0", + "responses>=0.25.0", + "pytest-mock>=3.12.0", + "coverage>=7.4.0", + "ruff>=0.4.0", +] +``` ```bash -# No new pip packages needed -# No new npm packages needed - -# Plugin update: replace main.js + styles.css + manifest.json + versions.json -# Python update: add paperforge/worker/discussion.py +# Plugin test dependencies (separate package.json in tests/plugin/) +npm install --save-dev vitest obsidian-test-mocks jsdom ``` ## Sources -- Existing codebase analysis: `paperforge/plugin/main.js` (2067 lines), `paperforge/plugin/styles.css` (1325 lines), `paperforge/worker/asset_index.py` (577 lines), `paperforge/worker/asset_state.py` (243 lines), `paperforge/worker/sync.py` (1829 lines) — HIGH confidence -- `.planning/PROJECT.md` — v1.8 milestone definition, ai/ directory already created per paper — HIGH confidence -- AGENTS.md — Lite architecture, Worker/Agent split, plugin thin-shell constraint — HIGH confidence -- `.planning/research/STACK.md` (v1.6) — existing stack decisions, Pydantic version, filelock version, "what NOT to add" — HIGH confidence -- Actual test fixtures: `tests/test_asset_state.py` — ai_path field verified in lifecycle/health computation — HIGH confidence - ---- - -*Stack research for: PaperForge v1.8 AI Discussion Recording & Deep-Reading Dashboard* -*Researched: 2026-05-06* +- pytest documentation: https://pytest.org/ (HIGH confidence — official docs) +- pytest-snapshot: https://pypi.org/project/pytest-snapshot/ (MEDIUM confidence — verified via search) +- obsidian-test-mocks: https://www.npmjs.com/package/obsidian-test-mocks (MEDIUM confidence — verified via Exa search) +- Vitest: https://vitest.dev/ (HIGH confidence — official docs) +- GitHub Actions setup-python: https://github.com/actions/setup-python (HIGH confidence — official docs) +- responses library: https://pypi.org/project/responses/ (HIGH confidence — official docs) diff --git a/.planning/research/SUMMARY.md b/.planning/research/SUMMARY.md index 650904d6..641a7de8 100644 --- a/.planning/research/SUMMARY.md +++ b/.planning/research/SUMMARY.md @@ -1,353 +1,292 @@ # Project Research Summary -**Project:** PaperForge v1.8 — AI Discussion Recording & Deep-Reading Dashboard -**Domain:** Brownfield Obsidian plugin — extending mode-based dashboard with AI discussion capture -**Researched:** 2026-05-06 +**Project:** PaperForge v2.0 — Testing Infrastructure (6-Layer Quality Gates) +**Domain:** Brownfield hybrid Python + Obsidian plugin project — multi-layer quality gate testing +**Researched:** 2026-05-08 **Confidence:** HIGH ## Executive Summary -PaperForge v1.8 adds two tightly-coupled capabilities to the existing PaperForge Obsidian plugin: **(1) AI discussion recording** that captures `/pf-paper` and `/pf-deep` conversations into structured `ai/discussion.json` and human-readable `ai/discussion.md` files within each paper's workspace directory; and **(2) a 4th dashboard mode** (`deep-reading`) activated automatically when the user opens a `deep-reading.md` file, showing Pass 1/2/3 completion status, Pass 1 summary, and recent AI Q&A history. +PaperForge is a **brownfield hybrid Python + Obsidian plugin** project that manages literature assets (Zotero exports, PDFs, OCR fulltext, figures, notes, and AI outputs) as a local-first research library. The current milestone (v2.0) adds a **6-layer quality gate testing infrastructure** — version consistency, Python unit tests, CLI contract tests, plugin-backend integration, temp vault E2E workflows, user journey contracts, and destructive scenarios — with CI matrix, golden datasets, and snapshot testing. The product has 473+ existing tests that need restructuring into a test pyramid, and the testing infrastructure itself is the deliverable. -The recommended approach is **thin-shell extension** — zero new dependencies, zero new CLI commands. The Obsidian plugin (CommonJS, single `main.js` file) gains one new mode renderer and one mode detection branch. A new Python module (`paperforge/worker/discussion.py`) handles writing discussion files at the end of Agent sessions. The canonical index (`formal-library.json`) serves as the bridge — JS reads pre-computed lifecycle/health/maturity from it, while `discussion.json` (vault-internal) should use `vault.adapter.read()` for cache consistency, **not** `fs.readFileSync()` as the older index pattern does. +The recommended approach is a **modified testing diamond** with strict layer dependency ordering (L0 -> L1 -> L2/L3 in parallel -> L4 -> L5), a **plasma CI matrix** (full 3x3 only for unit tests, narrow configs for higher layers), **hierarchical pytest fixtures** (5 levels from empty_vault to full_test_vault), and **shape-specific snapshot assertions** (not whole-file JSON). The build order is: fixtures first, then Python layers (L0-L2), then JS layers (L3), then expensive integration layers (L4-L6). -The three key risks are: (1) mode detection ordering — `deep-reading.md` carries the same `zotero_key` frontmatter as the formal note and will incorrectly trigger per-paper mode unless the `deep-reading.md` filename check comes *first*; (2) encoding corruption on Windows with CJK locales — Chinese Q&A content written by Python must use explicit `encoding='utf-8'` and Node.js must read with matching encoding to avoid mojibake; and (3) `discussion.json` must include `schema_version` from day one to prevent silent data loss on format changes. All three are preventable with the right discipline in Phase 1 and Phase 2. +**Key risks and mitigations:** +1. **Mock drift** — mocks that silently diverge from real APIs. Mitigation: capture real PaddleOCR responses before mocking; use `autospec=True` on all patches; limit patch nesting to 3 levels. +2. **CI combinatorial explosion** — trying to run everything on every platform. Mitigation: plasma CI matrix with path-filtered jobs; L1 on 3x3; L2-L5 on 1-2 configs; slow tests on nightly only. +3. **Snapshot brittleness** — whole-file snapshot assertions that break on every refactor. Mitigation: assert specific shapes with normalized dynamic fields; use `dirty-equals` for timestamps/UUIDs. +4. **Windows path hell** — junctions, symlinks, temp dir differences. Mitigation: at least one Windows CI node; platform-specific tests with `@pytest.mark.skipif`; path normalization utilities. +5. **Fixture bloat** — 100MB+ fixture directories in git. Mitigation: generate fixtures from code; track with manifest; keep large fixtures outside git repo. ## Key Findings ### Recommended Stack -v1.8 is purely additive to the existing v1.6–v1.7 stack. **No new npm packages, no new Python packages.** The plugin remains a single CommonJS file (`main.js`, currently ~2067 lines) with CSS in `styles.css` (~1325 lines). Python gains one new module (`discussion.py`) using only stdlib (`json`, `pathlib`, `datetime`, `tempfile`, `os`). +The testing stack splits cleanly into Python test infrastructure and JavaScript plugin test infrastructure, plus CI orchestration. -**Core technologies (unchanged):** -- **Obsidian CommonJS plugin** (`main.js`, no bundler): Dashboard/settings/commands UI — pure Obsidian API, no build step -- **Python 3.10+** (existing CLI): Single owner of business logic, templates, schema; extends naturally to discussion recording -- **Filesystem JSON** (`formal-library.json`, `discussion.json`): Contract between Python and JS; vault-native, git-trackable, backup-friendly -- **Filesystem Markdown** (`discussion.md`): Human-readable companion, Obsidian-editable, wikilink-compatible +**Core test framework (Python):** -**New file formats for v1.8:** -- `ai/discussion.json` — structured AI Q&A record with `schema_version`, `paper_key`, `sessions[]` array containing `session_id`, `agent`, `started`, and `qa_pairs[]` -- `ai/discussion.md` — human-readable Q&A log with session-grouped `##` headings, `**问题:**`/`**解答:**` markers, timestamp metadata +| Technology | Version | Purpose | Rationale | +|------------|---------|---------|-----------| +| pytest | >=8.0 | Python test framework | Already in use; industry standard | +| pytest-snapshot | >=0.9.0 | Snapshot/approval testing | CLI JSON output stability contracts | +| pytest-timeout | >=2.2.0 | Test timeout guards | Prevent runaway E2E/chaos tests | +| pytest-mock | >=3.12.0 | Enhanced mocking | Better than raw monkeypatch | +| responses | >=0.25.0 | HTTP mock library | Intercept `requests` at HTTP layer for mock OCR | +| coverage | >=7.4.0 | Coverage measurement | Standard Python tool; CI reports | +| ruff | >=0.4.0 | Linting | Already in use via pre-commit hooks | -**Key "what NOT to add" decisions (all researchers agree):** -- No npm packages (React, Vue, chart libs) — breaks single-file deployment -- No database (SQLite, IndexedDB) — discussions belong to the paper workspace, must be vault-native -- No Python schema libraries (Pydantic, jsonschema) — overkill for flat Q&A list -- No second plugin file — import resolution is fragile in Obsidian -- No new CLI command (`paperforge discuss`) — recording happens inside Agent sessions +**Plugin test framework (JavaScript):** + +| Technology | Version | Purpose | Rationale | +|------------|---------|---------|-----------| +| Vitest | >=2.0 | JS test runner | Native ESM; 2x faster than Jest; better `obsidian-test-mocks` integration | +| obsidian-test-mocks | >=0.12 | Obsidian API mocks | Community-maintained; 100% code coverage | +| jsdom | >=24.0 | DOM environment | Required by plugin tests (document/window access) | +| Node.js | >=20.0 | JS runtime | Required by Vitest and obsidian-test-mocks | + +**CI Infrastructure:** + +| Technology | Version | Purpose | +|------------|---------|---------| +| actions/setup-python | v5 | Python version management with cache | +| actions/setup-node | v4 | Node.js version management | +| actions/checkout | v4 | Source checkout | +| re-actors/alls-green | v1 | CI gate aggregation (all-jobs-passed check) | + +**Alternatives considered and rejected:** +- **Jest** rejected over Vitest (ESM compatibility, speed) +- **tox** rejected for CI (superfluous complexity; raw pip install is more transparent) +- **syrupy** rejected over pytest-snapshot (extra features not needed for JSON-only snapshots) +- **Real Obsidian E2E** rejected as too expensive (3+ min per test, flaky) — deferred to v2.1 ### Expected Features -**Must have (table stakes — v1.8 launch):** -- AI Discussion Recorder: Writes `discussion.md` + `discussion.json` when `/pf-paper` or `/pf-deep` completes — researchers expect past AI conversations next to the paper, not scattered in browser tabs -- Deep-Reading Dashboard Mode: Plugin detects `deep-reading.md` as active file, switches to dedicated mode showing Pass 1/2/3 status + AI Q&A history — users want at-a-glance reading progress without scrolling -- Jump-to-Deep-Reading Button: Contextual button on per-paper dashboard card that opens deep-reading.md — bridges paper lifecycle view to reading content -- Version Number Display Fix: Restore the `_versionBadge` — users need to confirm version for issue reporting -- "ai" UI Row Removal: Remove the meaningless row — erodes trust in dashboard accuracy +**Must have (table stakes):** +- **L0: Version consistency gate** — prevents deployment with mismatched versions across 6+ files. Low complexity. Single Python script. +- **L1: Python unit tests** — existing 470+ tests relocated to `tests/unit/`. Low complexity. Fast isolation. +- **L2: CLI contract tests** — subprocess invoker + snapshot assertions on `--json` output. Medium complexity. Protects plugin<->Python boundary. +- **L3: Plugin unit tests** — Vitest + obsidian-test-mocks. Medium complexity. Catches JS logic errors before manual QA. +- **L4: Temp vault E2E tests** — full pipeline (sync -> OCR -> status -> repair) in disposable vault. High complexity. 3 mock systems. +- **Golden datasets** — centralized `fixtures/` with zotero/, pdf/, ocr/, snapshots/. Medium complexity. Prerequisite for all higher layers. +- **Mock OCR backend** — intercepts PaddleOCR at HTTP layer with `responses`. Medium complexity. Enables deterministic testing. +- **Mock Zotero data source** — static JSON fixtures in 3 BBT path formats. Low complexity. +- **Mock vault filesystem** — `tmp_path` + factory function with configurable completeness. Medium complexity. +- **CI integration** — GitHub Actions with matrix strategies. Medium complexity. -**Should have (v1.8.x follow-up, deferrable):** -- Discussion session merging (append, not overwrite, on repeated `/pf-paper` for same paper) -- Session metadata in discussion.json (model, duration, agent type) -- Pass completion percentage calculation +**Should have (differentiators):** +- **Snapshot testing for CLI output** — catches unintended changes immediately. Single assertion per contract test. +- **Plasma CI matrix** — optimizes CI cost while maintaining coverage. Full matrix for fast tests, narrow for slow tests. +- **User journey tests (L5)** — validates complete workflows against documented UX contracts. Catches cross-component gaps. +- **Chaos/destructive tests (L6)** — corrupted inputs, network failures, disk issues. Ensures graceful degradation. +- **Hierarchical conftest fixtures** — root conftest provides shared fixtures; each layer extends/overrides. +- **VaultBuilder factory** — single entry point for 3 completeness levels: "minimal", "standard", "full". +- **CHAOS_MATRIX.md** — documentation of all destructive scenarios, triggers, expected behavior. -**Future consideration (v2+):** -- Cross-library discussion search (rely on Obsidian built-in search) -- Discussion export to AI context packs -- Deep-reading maturity integration into maturity gauge - -**Anti-features explicitly rejected:** -- Auto-recording all agent conversations (creates noise, violates worker/agent boundary) -- Full chat transcription in markdown (unstructured, duplicates agent's internal logs) -- Deep-reading dashboard as replacement for deep-reading.md (dashboard is index/summary view only) -- Real-time dashboard updates during deep-reading (agent executes outside plugin; refresh on active-leaf-change is sufficient) -- Discussion search across all papers (Obsidian's built-in search already handles this) +**Anti-features (explicitly NOT building):** +- Real Obsidian E2E in CI (3+ min per test, flaky) +- Load/performance testing (local single-user tool) +- Cross-browser testing (plugin runs in Chromium-only Electron) +- Full parallel matrix (54 CI jobs per PR for minimal confidence) +- Automated visual regression (fragile; component-level tests suffice) ### Architecture Approach -The architecture extends PaperForge's existing **thin-shell plugin pattern**: Python owns all business logic and writes structured data to the filesystem; the Obsidian JS plugin reads that data and renders Pure CSS/DOM components. No business logic is duplicated in JS. +The testing architecture follows a **modified testing diamond** (unit tests at base, integration/contract in the middle, E2E at top) with an extra **L0 version-check layer** as the fast pre-flight gate. All layers share a centralized `fixtures/` golden dataset with 4 subdirectories (zotero/, pdf/, snapshots/, ocr/). Mock systems are layered: HTTP-level (`responses` for OCR), static fixtures (for Zotero), and `tmp_path` (for vault filesystem). -The key architectural addition is a **4th mode dispatch** in the existing `_detectAndSwitch()` → `_switchMode()` → `case 'mode': renderX()` pattern. The mode detection hierarchy (in strict order) becomes: +**Major components:** +1. **L0: `scripts/check_version_sync.py`** — validates all 6+ version declarations (Python **init**, manifest.json, versions.json, CHANGELOG) before any tests run. Sub-100ms. Single CI job on ubuntu. +2. **L1: `tests/unit/`** — relocated existing 470+ tests. No external dependencies. Mock everything outside module under test. <30s full suite. Coverage >85% for core, >75% for workers. +3. **L2: `tests/cli/`** — subprocess invoker asserts exit codes, stdout JSON schemas, stderr patterns. Uses `pytest-snapshot` with shape-specific assertions (not whole-file). +4. **L3: `tests/plugin/`** — Vitest + obsidian-test-mocks + jsdom. Tests plugin lifecycle, settings, dashboard, i18n. Extracts `paperforge-backend.js` for Node.js-testable backend module. +5. **L4: `tests/e2e/`** — creates full temp vault; calls Python worker functions directly (not subprocess). Session-scoped golden vault + per-test fast clone for performance. +6. **L5: `tests/journey/`** — UX contract-driven complete workflows. Single concrete scenario per test. Step abstraction layer. Nightly-only. +7. **L6: `tests/chaos/scenarios/`** — 8 destructive scenarios documented in CHAOS_MATRIX.md. Docker-only isolation. Safety contracts on every test. +8. **`fixtures/`** (golden dataset) — centralized at repo root. zotero/ (8 JSON fixtures), pdf/ (4 minimal PDFs), snapshots/ (4 subdirectories), ocr/ (5 fixture files). Generated from code where possible. +9. **CI workflows** — 3 workflow files: `ci-pr-checks.yml` (L0+L1 per push, <2min), `ci.yml` (L0-L5 on merge to main), `ci-chaos.yml` (L6 weekly + manual). -``` -1. no active file → 'global' (existing) -2. .base file → 'collection' (existing) -3. deep-reading.md → 'deep-reading' (NEW — MUST precede zotero_key check) -4. .md with zotero_key → 'paper' (existing) -5. fallback → 'global' (existing) -``` - -**Major components affected:** -1. **`_detectAndSwitch()` + `_switchMode()`** — Mode detection extended with deep-reading.md filename check; must be checked *before* zotero_key frontmatter to prevent hijacking by per-paper mode -2. **`_renderDeepReadingMode()` (NEW)** — Dedicated render method consuming data from formal-library.json (lifecycle/health/maturity), deep-reading.md (Pass 1 summary), and ai/discussion.json (AI Q&A history) -3. **`_renderPaperMode()` extension** — Adds "Jump to Deep Reading" contextual button when `deep_reading_path` exists -4. **`discussion.py` (NEW Python module)** — `record_discussion()` writes both discussion.md and discussion.json; `load_discussion_json()` reads existing record; `append_discussion()` appends new Q&A pairs atomically -5. **`asset_index.py` build_envelope()** — Adds `paperforge_version` field for the version badge fix - -**Key data flow:** -``` -User runs /pf-deep → ld_deep.py generates scaffold → Agent completes session - → discussion_recorder.py writes ai/discussion.{md,json} -User opens deep-reading.md → _detectAndSwitch() detects it → _renderDeepReadingMode() - → reads formal-library.json (status badges) + deep-reading.md (Pass 1 summary) + ai/discussion.json (Q&A history) -``` +**Key patterns:** +- **Plasma CI matrix:** L1 on full 3 OS x 3 Python (9 jobs); L2 on 2 Python x 1 OS (2 jobs); L3-L5 on single config (1 job each) +- **Fixture hierarchy (5 levels):** `empty_vault` (config only) -> `config_vault` (+ dirs) -> `vault_with_export` (+ BBT JSON) -> `vault_with_ocr` (+ OCR data) -> `full_test_vault` (+ Zotero storage, formal notes) +- **Mock OCR with `responses`:** intercepts at HTTP layer, not module level. Fixtures from real API captures. Prevents mock drift. +- **Snapshot update protocol:** `pytest --snapshot-update` -> review diff -> commit deliberately. Never "regenerate all snapshots." +- **Truth hierarchy:** L4 is authoritative for "does it work?"; L2 validates CLI translation; L1 validates edge cases. L1 mocks must be validated against L4 output. ### Critical Pitfalls -All researchers independently flagged these — they represent consensus on highest-risk items: +**Top 7 pitfalls from research (ordered by severity):** -1. **Mode detection ordering regression** — `deep-reading.md` carries the same `zotero_key` frontmatter as the formal note. If the filename check isn't placed FIRST in the `.md` handler (before the `zotero_key` check), the dashboard enters per-paper mode instead of deep-reading mode. **Fix:** Insert `if (activeFile.basename === 'deep-reading')` as the first branch inside the `.md` handler. +1. **Mocking External Services Too Early and Too Rigidly** — The existing test suite already demonstrates 12+ levels of nested `with patch(...):` blocks. Mocks created from memory (not real API captures) silently drift from real PaddleOCR behavior. Tests pass but production breaks. *Prevention: capture real API responses first; use `autospec=True` on all patches; limit nesting to 3 levels via fixture extraction; add VCR.py layer for OCR polling.* -2. **Encoding corruption on Windows CJK systems** — Python may write GBK-encoded files while Node.js reads as UTF-8, producing mojibake for Chinese Q&A content. This is a well-documented Obsidian + Windows + Chinese locale problem. **Fix:** Python must use `open(path, 'w', encoding='utf-8')` explicitly; JS reads must match. Use `PYTHONIOENCODING=utf-8` and `PYTHONUTF8=1` environment variables for child processes. +2. **CI Matrix Combinatorial Explosion Killing Feedback Loops** — A naive full matrix (3 OS x 3 Python x 2 Node = 18 jobs at 15 min each = 4.5 hours wall-clock). Developers wait all day or skip CI. *Prevention: plasma matrix with path-filtered jobs; L1 on 3x3; L2-L5 on narrower configs; `pytest -m "not slow"` for push; hard CI budget of 20 concurrent runners.* -3. **`discussion.json` schema version missing** — Shipping without `schema_version` repeats the `formal-library.json` v1→v2 migration pain from v1.6/v1.7. Every format change becomes a breaking change. **Fix:** Include `"schema_version": "1"` in the envelope from day one. Always use object envelope, never top-level array. +3. **Snapshot Tests Breaking on Every Refactor** — Whole-file JSON snapshots cause 500-line diffs for 5-line code changes. "Regenerated all snapshots" becomes the default review response — regressions slip through. *Prevention: assert specific shapes, not whole files; normalize dynamic fields (timestamps, UUIDs) before snapshotting; use `inline-snapshot` or `dirty-equals` for version-agnostic comparisons.* -4. **`btoa()` crash on Chinese filenames** — `window.btoa()` only supports Latin1 characters. If any path construction for `ai/` directory uses `btoa()` with Chinese paper titles, it throws `InvalidCharacterError`. **Fix:** Ban `btoa()` and `atob()` from the plugin. Use `Buffer.from(str, 'utf-8').toString('base64')` instead. +4. **Temp Vault Tests Being Slow, Non-Deterministic, or Platform-Specific** — E2E tests with subprocess calls take 30-60 seconds each. Cross-platform path issues (Windows junctions, macOS `/var` -> `/private/var` symlinks). `shutil.rmtree` fails on Windows with `PermissionError`. *Prevention: session-scoped golden vault + per-test fast clone; separate "fast" (Python API) from "full" (subprocess) E2E; safe teardown with retry on Windows; normalize paths in all assertions.* -5. **`active-leaf-change` double-firing mode oscillation** — Obsidian fires this event twice during tab switches (old leaf blur + new leaf focus). The 300ms debounce can catch either, causing a visible flash of global mode between per-paper and deep-reading modes. **Fix:** Extract `_resolveModeForFile()` as a pure function; guard with identity check (same mode AND same file path = no-op); increase debounce to 500ms during transitions. +5. **User Journey Tests Being Too Vague to Automate** — Prose UX contracts like "User opens Obsidian, configures Zotero, runs OCR" are ambiguous and un-automatable. Tests either hardcode assumptions or check nothing meaningful. *Prevention: write journey tests as pseudo-code BEFORE implementation; create a "step" abstraction layer (not raw Playwright); define EXACTLY ONE concrete scenario per test; create journey fixture packs for pre-configured states.* -## Researcher Conflicts Resolved +6. **Destructive Tests That Damage Developer Machines or CI Shared State** — Chaos tests that delete files or modify config can accidentally run on real vaults if `--vault` is omitted or fixture paths resolve incorrectly. *Prevention: ALL destructive tests MUST use `tmp_path` with isolation assertion (`assert "tmp" in str(vault)`); run in Docker only; add safety contract comment to every destructive test function; keep in separate module ignored by default test runner.* -The four researchers produced high-quality, mostly aligned outputs. However, two substantive conflicts emerged: +7. **Golden Dataset and Fixture Bloat** — Fixtures grow from 10KB to 100MB+ in git over 6 months. Nobody knows which are still used. Binary fixtures (PDFs, OCR JSON) permanently bloat git history. *Prevention: generate fixtures from code (not hand-written); keep large fixtures outside git via `download_fixtures.py` script; track with MANIFEST.json and validate coverage in CI; version fixture schemas explicitly.* -### Conflict 1: `discussion.json` schema shape - -| Researcher | Schema Proposal | -|------------|----------------| -| **STACK.md** | Flat `history[]` array with `index`, `timestamp`, `question`, `answer`, `tags`, `agent_model`. Summary at top level with `total_qa`, `last_updated`, `top_tags`. | -| **FEATURES.md** | Nested `sessions[]` with `session_id`, `agent`, `started`, and `qa_pairs[]` array. Each QA pair has `question`, `answer`, `source`, `timestamp`. | -| **ARCHITECTURE.md** | Similar to FEATURES: `sessions[]` with `session_id`, `timestamp`, `model`, `command`, `summary`, `message_count`, `messages[]` with `role`/`content`. Separate `format_version` (not `schema_version`). | -| **PITFALLS.md** | Warns against bare array format, recommends `schema_version` envelope. | - -**Resolution: Adopt the FEATURES.md sessions-based schema with these adjustments:** -- Use `schema_version: "1"` (from PITFALLS) — not `format_version` -- Keep session grouping (`sessions[]` → `qa_pairs[]`) — this is the right semantic model; a flat history loses the session boundary that `/pf-paper` vs `/pf-deep` invocations represent -- Use `timestamp` per QA pair (from FEATURES/STACK) — individual message timing matters for chronology -- Include `source` field as optional (from FEATURES) — traces answers back to specific Pass/section -- Add `model` at session level (from STACK/ARCHITECTURE) — the model is per-session, not per-QA -- Drop `message_count` (ARCHITECTURE proposes it but it's derivable from `qa_pairs.length`) - -**Rationale:** Sessions are the natural grouping unit — each `/pf-paper` or `/pf-deep` invocation creates a new session. A flat history array loses this structure. The FEATURES.md schema best captures this while staying minimal. - -### Conflict 2: How the plugin reads `discussion.json` - -| Researcher | Recommendation | -|------------|----------------| -| **STACK.md** | `fs.readFileSync()` — same pattern as `formal-library.json` reading | -| **ARCHITECTURE.md** | `fs.readFileSync()` — "same pattern for discussion.json... from ai/ directory" | -| **PITFALLS.md** | **DO NOT use `fs.readFileSync()`** — use `vault.adapter.read()` because discussion.json is vault-internal (paper workspace), not system-directory. `fs.readFileSync` bypasses vault cache, misses modify events, risks encoding issues. | -| **FEATURES.md** | Doesn't specify read method, only says "reads discussion.json" | - -**Resolution: PITFALLS.md is correct. Use `app.vault.adapter.read()` for discussion.json.** - -**Rationale:** `fs.readFileSync()` is the correct pattern for `formal-library.json` because it lives in `<system_dir>/PaperForge/indexes/` — outside the Obsidian vault, invisible to vault cache. But `discussion.json` lives in `Literature/<domain>/<key> - <Title>/ai/` — inside the vault, visible in Obsidian's file explorer. Using `fs.readFileSync()` on vault-internal files: -- Bypasses Obsidian's metadata cache -- Prevents `modify` events from triggering dashboard refresh when Python writes new discussions -- Risks encoding mismatch on Windows CJK systems (vault adapter normalizes encoding) - -The STACK.md and ARCHITECTURE.md researchers made a natural but incorrect assumption that the existing pattern extends unmodified. The PITFALLS researcher caught the distinction: system-directory files use `fs`, vault-internal files use `vault.adapter`. +**See `.planning/research/PITFALLS.md` for all 13 pitfalls, including:** fixture inheritance conflicts (P13), Windows path hell (P12), test layering conflicts (P11), plugin tests testing subprocess mechanics instead of behavior (P10), CLI --json tests checking shape but not semantics (P9), and version sync checking the wrong things (P8). ## Implications for Roadmap -Based on dependency analysis across all four research files, the research converges on a **7-phase build order**. Phases 31a and 31b are quick-win bug fixes; Phase 32 establishes the mode detection infrastructure; Phase 33 builds the dashboard renderer; Phase 34 adds navigation; Phases 35-36 build the data pipeline end-to-end. +Based on combined research, the testing infrastructure should be built in **5 phases** with strict dependency ordering. Each phase delivers value independently and is testable before the next begins. -### Phase 31a: Fix Version Number Display +### Phase 1: Foundation — Fixture Hierarchy + L0 + L1 Relocation -**Rationale:** Lowest risk, no dependencies. Single change in Python (`build_envelope()` adds `paperforge_version`) + single change in JS (`_fetchStats()` reads it). Quick win that improves perceived quality before new features ship. +**Rationale:** All higher layers depend on healthy fixtures and a working test runner. The existing 473+ tests must be relocated into the new hierarchy before any new tests are written. L0 (version check) and L1 (unit tests) can be extracted early because they require minimal new infrastructure. -**Delivers:** Version badge shows actual PaperForge version (e.g., `v1.8.0`) instead of `v—` placeholder. +**Delivers:** +- Refactored `tests/conftest.py` with 5-level fixture hierarchy (`empty_vault` -> `config_vault` -> `vault_with_export` -> `vault_with_ocr` -> `full_test_vault`) +- All existing tests relocated from flat `tests/` to `tests/unit/` (pure directory moves, no behavior changes) +- `scripts/check_version_sync.py` (L0: validates 6+ version declarations, semver structure, installed `--version` against source) +- `ci-pr-checks.yml` (L0 on ubuntu + L1 on full 3x3 matrix, <2 min) +- `pyproject.toml` updated with markers, testpaths, new dependencies +- `tests/sandbox/` retained in place for backward compat -**Addresses:** Bug Fix: Version Number (FEATURES.md) -**Avoids:** Pitfall 8 — version badge race condition (reads from plugin manifest as floor, Python index as ceiling) +**Addresses:** FEATURES.md table stakes (L0, L1) +**Avoids:** PITFALLS.md P13 (fixture inheritance) — implement hierarchy before any new tests; P8 (version sync wrong checks) — validate installed version not file strings +**Stack:** pytest >=8.0, pytest-timeout >=2.2.0, pytest-mock >=3.12.0, coverage >=7.4.0, ruff >=0.4.0 +**Research flag:** Well-documented patterns — pytest fixtures and test organization are standard. No deep research needed. -**Stack elements used:** Python `__version__` from `paperforge/__init__.py`; JS `_cachedStats.version` read path. +### Phase 2: Golden Datasets + CLI Contract Tests (L2) -### Phase 31b: Fix "ai" Row Bug +**Rationale:** The `fixtures/` golden dataset is the shared foundation for L2-L6. L2 (CLI contracts) is the most valuable integration layer — it protects the Python<->plugin boundary with minimal setup (subprocess invoker + snapshot assertions). Building fixtures and L2 together ensures fixture design matches real usage. -**Rationale:** Independent bug fix, but must be applied AFTER Phase 31a because understanding which UI element is "ai" requires reading current dashboard render paths. Grep all render paths before removal. +**Delivers:** +- `fixtures/` directory structure: zotero/ (8 fixtures), pdf/ (4 minimal PDFs), snapshots/ (4 subdirectories), ocr/ (5 files) +- `fixtures/MANIFEST.json` — tracks `used_by`, `generated`, `desc` for each fixture +- `tests/cli/` with subprocess invoker fixture and contract tests for all 7 CLI commands +- `pytest-snapshot` integrated with shape-specific assertions (not whole-file) +- Mock OCR backend with `responses` library, using fixture responses captured from real API +- `ci.yml` extended with L2 on 2 Python versions x 1 OS +- All L2 tests excluded from `ci-pr-checks.yml` (they depend on L1 being green) -**Delivers:** Dashboard no longer shows a meaningless "ai" row. +**Addresses:** FEATURES.md table stakes (golden datasets, mock OCR, L2), differentiators (snapshot testing) +**Avoids:** PITFALLS.md P1 (mock drift) — capture real API first; P3 (snapshot brittleness) — shape assertions; P7 (fixture bloat) — MANIFEST + CI validation; P9 (CLI shallow tests) — schema validation + cross-command consistency +**Research flag:** **Needs research** — Snapshot testing strategy decisions: inline vs external snapshots, normalization helpers for dynamic fields. Low risk, but needs concrete examples before implementation. -**Addresses:** Bug Fix: Remove meaningless "ai" row (FEATURES.md) -**Avoids:** Pitfall 9 — "ai" row removal without checking all render paths +### Phase 3: Plugin Tests + Temp Vault E2E (L3 + L4) -**Stack elements used:** `grep -rn 'ai' paperforge/plugin/main.js paperforge/plugin/styles.css` to identify source; surgical removal scoped to the specific render path. +**Rationale:** L3 and L4 can be built in parallel (they target different runtimes: JS vs Python). L3 (plugin) is independent of Python layers; L4 (E2E) depends on L2 being stable (contracts define what E2E tests should verify). Building them together optimizes the critical path. -### Phase 32: Add Deep-Reading Mode Detection +**Delivers:** +- `tests/plugin/` with Vitest + obsidian-test-mocks + jsdom +- Extracted `paperforge-backend.js` module for Node.js-testable backend interface +- Plugin lifecycle, settings, dashboard, i18n, subprocess dispatch tests +- `tests/e2e/conftest.py` with session-scoped golden vault + per-test fast clone +- Temp vault E2E tests: full sync-OCR-status pipeline, repair workflow, headless setup, migration, doctor, multi-domain +- Safe teardown with Windows file-lock retry +- Mock OCR used in L2/L3/L4 with layer-specific fixture configurations +- CI extended with L3 (1 Node) + L4 (1 Python, 1 OS) -**Rationale:** This is the architectural foundation for everything in v1.8. Must be built before any deep-reading rendering can happen. Dependencies: nothing (pure JS change in `_detectAndSwitch()`). +**Addresses:** FEATURES.md must-haves (L3, L4), differentiators (VaultBuilder factory) +**Avoids:** PITFALLS.md P4 (temp vault slow) — session-scoped + per-test clone; P10 (plugin subprocess mechanics) — extract backend module; P12 (Windows path hell) — platform-specific marks + normalization +**Research flag:** **Needs research** — `obsidian-test-mocks` API surface: does it support the specific `App`, `Workspace`, `Plugin` APIs used by PaperForge? Verify by loading the npm package and comparing against actual usage in `paperforge/plugin/main.js`. -**Delivers:** When user opens `Literature/<domain>/<key> - <Title>/deep-reading.md`, the plugin detects it and dispatches to `deep-reading` mode (even though the stub renderer is a placeholder until Phase 33). +### Phase 4: User Journey + Chaos Tests (L5 + L6) -**Implements:** Architecture component — `_detectAndSwitch()` extension, `_switchMode()` `case 'deep-reading'` branch. +**Rationale:** These are the most expensive and least stable tests. They should be built LAST, after the foundation (L0-L4) is solid. L5 depends on L4 for vault infrastructure; L6 is independent but shares mock systems with L2. -**Avoids:** Pitfall 1 (mode detection ordering — check `basename === 'deep-reading'` BEFORE `zotero_key` frontmatter); Pitfall 5 (active-leaf-change double-fire — extract `_resolveModeForFile()` as pure function with identity guard); Pitfall 3 (filename-based heuristic — verify parent directory matches `{8-char key} - {slug}` pattern, not just basename). +**Delivers:** +- `tests/journey/UX_CONTRACT.md` — concrete step sequences for each user journey +- 4 journey tests: new user onboarding, daily workflow, upgrade migration, error recovery +- Journey fixture packs: "half-setup", "ready-for-deep-reading" pre-configured environments +- `tests/chaos/scenarios/CHAOS_MATRIX.md` — 8 destructive scenarios documented +- 8 chaos test files with Docker-only isolation and safety contracts +- `ci-chaos.yml` — weekly schedule + manual trigger, scenarios parallelized by matrix +- Journey tests NOT in PR gate (nightly only); chaos tests NOT in regular CI -**Key implementation constraint:** -```javascript -// Inside .md handler, BEFORE zotero_key check: -if (activeFile.basename === 'deep-reading') { - const parentDir = activeFile.parent?.name || ''; - const match = parentDir.match(/^([A-Z0-9]{8})\s+-\s+(.+)$/); - if (match) { - this._currentPaperKey = match[1]; - this._currentPaperEntry = this._findEntry(match[1]); - this._switchMode('deep-reading'); - return; - } -} -// THEN: existing zotero_key check for per-paper mode -``` +**Addresses:** FEATURES.md differentiators (L5, L6, chaos matrix doc) +**Avoids:** PITFALLS.md P5 (vague journeys) — concrete scenarios + step abstraction; P6 (destructive safety) — isolation assertions + Docker +**Research flag:** **Needs research** — Chaos scenario design: what are the real-world failure modes for PaddleOCR API, Zotero junctions, concurrent sync calls? Needs domain knowledge from the existing codebase's error-handling paths. -### Phase 33: Build `_renderDeepReadingMode()` Component +### Phase 5: CI Matrix Optimization + Consistency Audit -**Rationale:** Depends on Phase 32 (mode detection must route to this renderer). This is the largest single JS change. Must implement the sub-components (status bar, paper info header, Pass 1 summary, AI Q&A history placeholder, navigation) with empty-state handling for all data sources. +**Rationale:** The final phase optimizes and hardens the CI pipeline based on real data from earlier phases. Path filters, performance baselines, and cross-layer consistency audits should be informed by actual CI run times and failure patterns. -**Delivers:** Deep-reading dashboard that shows: -- Status bar with lifecycle/OCR/deep-reading/maturity badges -- Paper info header (title, authors, year, domain) -- Pass 1 summary extracted from deep-reading.md -- AI Q&A history section (placeholder until Phase 36; shows empty state) -- Navigation link back to the per-paper dashboard +**Delivers:** +- Path-filtered CI triggers (e.g., changes to `ocr.py` trigger L1+L2+L4; changes to `main.js` trigger L3) +- `pytest -m "slow"` markers for tests >30s (moved to nightly or merge-only) +- Consistency audit test: validates L1 mock expectations against L4 real output +- `re-actors/alls-green` gate aggregation for branch protection +- `scripts/validate_fixtures.py` — CI check that every fixture is referenced by at least one test +- CI budget enforcement: max 20 concurrent runners; single-config L2-L5 -**Implements:** Architecture component — `_renderDeepReadingMode()` with all sub-renderers. - -**Avoids:** Pitfall 6 (empty discussion.json states — implement `_loadDiscussionData()` helper returning discriminated union for all 4 empty states: `no_ai_dir`, `not_found`, `empty`, `no_discussions`, `ok`); Pitfall 13 (CSS namespace collision — use `paperforge-deepreading-*` prefix, scope under `.paperforge-mode-deepreading` wrapper class); Pitfall 3 (anti-pattern: deep-reading dashboard replacing deep-reading.md — dashboard is index/summary only, never authoritative). - -**CSS additions** (all scoped under `.paperforge-mode-deepreading`): -- `.paperforge-deep-reading-view` — root container (flex column, gap: 20px) -- `.paperforge-dr-status-bar` — Pass 1/2/3 completion indicator row -- `.paperforge-dr-pass-summary` — Pass 1 summary card -- `.paperforge-dr-discussion-card` — individual Q&A card -- `.paperforge-dr-discussion-list` — scrollable card container -- `.paperforge-dr-tag-chip` — tag pills (font-size: 10px, border-radius: 8px) -- `.paperforge-dr-empty` — empty state styling -- `.paperforge-dr-section-title` — section headers - -### Phase 34: Add "Jump to Deep Reading" Button - -**Rationale:** Depends on Phase 32 (deep-reading path resolution) and Phase 33 (deep-reading dashboard must exist for navigation to matter). Simple modification to `_renderPaperMode()`. - -**Delivers:** On per-paper dashboard card, a "Jump to Deep Reading" button appears when `entry.deep_reading_path` is non-empty and `entry.deep_reading_status === 'done'`. Click navigates to `deep-reading.md`, which triggers Phase 32 mode detection. - -**Implements:** Architecture component — integration point on per-paper dashboard contextual actions row. - -**Avoids:** Pitfall 7 (Jump button assumes file exists — verify `getAbstractFileByPath()` before `openLinkText()`, show clear `Notice` on missing file); Anti-feature (dashboard replacing deep-reading.md — button opens the actual file, dashboard is a view layer). - -**Condition to show button:** `entry.deep_reading_path && entry.deep_reading_status === 'done'` — don't show for papers that haven't had deep reading performed. - -### Phase 35: AI Discussion Recorder (Python) - -**Rationale:** Depends on nothing (standalone Python module). Can be built in parallel with Phases 32-34. Writes the data that Phase 36 will read. Must implement atomic writes to prevent corruption during concurrent access. - -**Delivers:** `paperforge/worker/discussion.py` with: -- `record_discussion(key, vault, agent, qa_pairs)` — writes both `discussion.md` and `discussion.json` -- `load_discussion_json(key, vault)` — reads existing record -- `append_discussion(key, vault, qa_pair)` — appends to existing (read-modify-write atomic via tempfile + os.replace) - -**Implements:** Architecture component — Python discussion recording module, integration with `/pf-paper` and `/pf-deep` Agent commands. - -**Uses:** Existing stdlib only (`json`, `pathlib`, `datetime`, `tempfile`, `os`); same atomic write pattern as `asset_index.py`. - -**Avoids:** Pitfall 3 (schema version missing — ship with `schema_version: "1"` in envelope); Pitfall 2 (encoding corruption — use `encoding='utf-8'` explicitly, set `PYTHONIOENCODING=utf-8` env var); Pitfall 4 (btoa on Chinese — path resolution uses zotero_key only, never writes slugified title paths); Pitfall 10 (.md vs .json inconsistency — define .json as canonical, .md as derived view); Pitfall 11 (tabs/newlines breaking Obsidian callouts — use `newline='\n'` consistently, avoid tabs); Pitfall 15 (heading collisions — use unique heading prefix `## AI Discussions` for discussion.md). - -**File operations must be atomic:** Use the proven `tempfile.NamedTemporaryFile` + `os.replace()` pattern to prevent partial writes during concurrent access. Do NOT write directly to the target file. - -### Phase 36: Wire AI Q&A History into Deep-Reading Dashboard - -**Rationale:** Depends on Phase 33 (renderer exists) and Phase 35 (data exists). This is the integration step that connects the data pipeline end-to-end. - -**Delivers:** When deep-reading dashboard shows, the AI Q&A History section renders actual discussion data from `ai/discussion.json` (last 3 Q&A pairs across all sessions, most recent first). Empty state shown when no discussions exist. - -**Implements:** Architecture component — `_renderDiscussionHistory()` in deep-reading mode renderer. - -**Avoids:** Pitfall 2 (fs.readFileSync bypasses vault API — use `app.vault.adapter.read()` for vault-internal discussion.json, NOT `fs.readFileSync`); Pitfall 12 (debounce timer leak — add 2-second cooldown after refresh, use mtime comparison to avoid redundant re-renders). - -**Key integration detail:** The modify event filter must include `discussion.json` paths to trigger dashboard refresh when Python appends new Q&A: -```javascript -const modifyHandler = this.app.vault.on('modify', (file) => { - if (file?.path?.endsWith('formal-library.json') || file?.path?.endsWith('discussion.json')) { - this._invalidateIndex(); - this._refreshCurrentMode(); - } -}); -``` - -### Phase 37: Integration Testing & Polish - -**Rationale:** After all components are built, verify end-to-end flow and fix edge cases. Must test: empty states, Chinese filenames/content, split-pane mode switching, rapid Q&A recording. - -**Delivers:** Verified end-to-end flow: `/pf-deep` → discussion files written → deep-reading dashboard shows Q&A history → Jump button navigates correctly → mode doesn't oscillate in split panes. - -**Avoids:** All pitfalls together — uses the "Looks Done But Isn't" checklist from PITFALLS.md (13 verification items). +**Addresses:** FEATURES.md differentiators (plasma CI matrix), anti-features avoidance (full matrix) +**Avoids:** PITFALLS.md P2 (CI too slow) — plasma matrix; P11 (test layering conflicts) — truth hierarchy + mock validation +**Research flag:** **Standard patterns** — GitHub Actions path filters and matrix strategies are well-documented. No deep research needed. ### Phase Ordering Rationale -- **Fixes first (31a, 31b):** Low risk, no dependencies, quick wins that improve perceived quality before new features ship. Version fix also establishes the Python→JS version bridge that Phase 33 needs. -- **Detection before rendering (32 → 33):** Mode detection is the routing infrastructure; the renderer can't work without it. Building detection first allows incremental testing. -- **Renderer before data integration (33 → 36):** Build the UI with proper empty states first, then wire real data in. This ensures graceful degradation when no discussions exist. -- **Python recorder parallel to JS dashboard (35 parallel to 32-34):** No runtime dependency — the recorder writes files, the dashboard reads them. Can be developed in parallel. -- **Integration last (36 → 37):** Wire the full data pipeline only after both ends exist. Test end-to-end only after wiring. +1. **Fixtures before tests** (Phase 1 before Phase 2): Golden datasets and fixture hierarchy are prerequisites for ALL test layers. Building them first prevents the "kitchen sink" fixture anti-pattern and fixture bloat. +2. **Python before JS** (Phase 2 before Phase 3): L2 CLI contracts define the interface between Python and plugin JS. Building L2 first means L3 can validate against known contracts. However, L3 and L4 can be parallelized since they target different runtimes. +3. **Cheap before expensive** (L0-L2 before L4-L6): The most valuable-per-effort tests (version sync, unit, CLI contracts) cost <2 min CI time. They catch 80% of regressions. E2E, journey, and chaos tests add 10-30 min each and should be built only when the fast layers are solid. +4. **Optimize last** (Phase 5 after all others): CI matrix optimization metrics (path filter accuracy, test durations, flake rates) can only be measured once all phases are running in CI. Optimizing earlier would be premature. ### Research Flags -**Phases likely needing deeper research during planning:** -- **Phase 32 (Mode Detection):** The `active-leaf-change` double-fire behavior varies by Obsidian version and platform. May need platform-specific debounce tuning during implementation. Consider `/gsd-research-phase` if Obsidian API behavior is uncertain. -- **Phase 35 (AI Discussion Recorder):** The exact integration point with `/pf-paper` and `/pf-deep` scripts needs implementation-level detail. How does the Agent session signal completion? What's the handoff protocol? May need `/gsd-research-phase` if the Agent integration surface is unclear. +Phases likely needing deeper research during planning: +- **Phase 2 (Golden datasets + L2):** Snapshot testing strategy — inline vs external, normalization helpers. Low risk, but need concrete decisions before implementation. +- **Phase 3 (L3 plugin tests):** `obsidian-test-mocks` API surface coverage — does it fully support PaperForge's Obsidian API usage patterns? Verify by loading the npm package. +- **Phase 4 (L5 + L6):** Chaos scenario design — what are the real-world failure modes for PaddleOCR, Zotero junctions, concurrent sync? Needs codebase analysis of error-handling paths. -**Phases with standard patterns (skip research-phase):** -- **Phase 31a (Version Fix):** Well-understood pattern — add field to envelope, read in fetch. Standard PaperForge index pattern. -- **Phase 33 (Dashboard Rendering):** Pattern established across 3 existing modes (global, paper, collection). Deep-reading mode follows the same `_renderXMode()` template. -- **Phase 34 (Jump Button):** Established contextual button pattern (`paperforge-contextual-btn` class) with `openLinkText()` navigation. Used elsewhere in `_renderPaperMode()`. -- **Phase 36 (Data Wiring):** Reading a JSON file and rendering DOM. Standard pattern used by `_fetchStats()` and all existing renderers. +Phases with standard patterns (skip research-phase): +- **Phase 1 (Foundation):** pytest fixture hierarchy, test relocation, version checking — well-documented established patterns +- **Phase 5 (CI optimization):** GitHub Actions path filters, matrix strategies — official docs are comprehensive ## Confidence Assessment | Area | Confidence | Notes | |------|------------|-------| -| Stack | **HIGH** | No new dependencies. All additions are new modules/files in existing well-understood architecture. Verified against actual source code at `paperforge/plugin/main.js` (2067 lines), `paperforge/plugin/styles.css` (1325 lines), `paperforge/worker/asset_index.py` (577 lines), `paperforge/worker/sync.py` (1829 lines). Actual test fixtures confirm `ai_path` field already exists. | -| Features | **HIGH** | Feature landscape verified against Obsidian plugin ecosystem (Smart Chat, Copilot, Gemini Scribe, Claude Sessions — 5 plugins analyzed). MVP definition maps directly to v1.8 milestone requirements from PROJECT.md. Anti-features identified by consensus (3 researchers independently flagged auto-recording as a problem). | -| Architecture | **HIGH** | All integration points verified against existing source code. Mode detection hierarchy, file ownership boundaries (Python vs JS), and data flow confirmed by code inspection at specific line ranges. Canonical index schema (`formal-library.json` v2) already carries `ai_path` field. Build order derived from dependency graph analysis. | -| Pitfalls | **HIGH** | 15 pitfalls identified across 3 severity tiers. Top 5 critical pitfalls each have verified root causes from Obsidian Forum (#31841, #91927), opencode-obsidian Issue #28, nodejs/undici Issue #5002, and paperclipai Issue #3940. Recovery strategies estimated for each pitfall. "Looks Done But Isn't" checklist provides 13 concrete verification items. | +| Stack | HIGH | pytest, Vitest, obsidian-test-mocks, responses are well-documented and verified against official sources. Alternatives analysis is thorough. | +| Features | HIGH | Feature landscape derived from existing codebase analysis (473+ tests) + official docs. Table stakes, differentiators, and anti-features are clearly scoped. | +| Architecture | HIGH | Detailed directory structure, mock system architecture, CI matrix design, fixture hierarchy, and 6 ADRs. All decisions have clear rationale and alternatives considered. | +| Pitfalls | HIGH | 13 pitfalls identified with prevention strategies, recovery plans, and phase mapping. Based on existing codebase analysis (12-level mock nesting, single fixture patterns) + external sources (pytest best practices, snapshot testing analysis). | -**Overall confidence: HIGH** - -All four researchers worked from the same source code (verified at specific line ranges), the same project context (PROJECT.md, STATE.md, AGENTS.md), and came to convergent conclusions. The two conflicts (schema shape, file read strategy) are well-characterized and resolved above. No areas of fundamental disagreement or missing information remain. +**Overall confidence:** HIGH ### Gaps to Address -- **Agent integration surface for discussion recording:** The exact API/callback for `discussion_recorder.record_session()` when an Agent session completes needs to be confirmed during Phase 35 implementation. Does `/pf-paper` emit a completion event? Where does the Q&A pair extraction happen? This is an implementation detail, not a research gap — it can be resolved during planning. -- **Deep-reading.md content parsing robustness:** The Pass 1 summary extraction relies on parsing `**一句话总览**` markers from deep-reading.md. If the agent generates different formatting in edge cases (non-standard headings, multiline bold), parsing could fail. Phase 33 should include a regex fallback that handles variations. -- **Performance at scale:** Current linear scan of index items for `_findEntry()` is O(n) and acceptable for ~100 papers. If the library grows past ~1000 papers, the dashboard may lag on mode switch. This is a known gap documented in ARCHITECTURE.md — address in a future performance phase, not v1.8. -- **Cross-platform UTF-8 path handling:** The research covers Windows CJK encoding issues thoroughly, but Linux/macOS with non-UTF-8 filesystem encodings (rare edge case) is not covered. No known users on these systems — defer until reported. +1. **`obsidian-test-mocks` API coverage validation** — The npm package claims 100% code coverage of `obsidian.d.ts`, but PaperForge uses specific APIs (settings tab, dashboard views, subprocess dispatch). Need to verify the mock covers all usage in `paperforge/plugin/main.js` before Phase 3 implementation. + +2. **Real PaddleOCR response format** — Mock OCR fixture design depends on capturing at least one real API response. If the API has changed or is inaccessible, fixture generation will need a different approach (e.g., API documentation or developer-provided example). + +3. **CI budget constraints** — The plasma matrix assumes GitHub-hosted runners are available. If this is a self-hosted runner or has credit limits, the matrix may need further reduction. Phase 5 should calibrate based on actual limits. + +4. **Windows junction behavior in CI** — Windows GitHub Actions runners may have different junction/symlink behavior than local Windows machines. The Windows E2E test may need adjustment after initial CI runs on Windows. ## Sources ### Primary (HIGH confidence) -- **PaperForge source code** (`paperforge/plugin/main.js` lines 1-2067, `paperforge/plugin/styles.css` lines 1-1325, `paperforge/worker/asset_index.py` lines 1-577, `paperforge/worker/sync.py` lines 1677-1749, `paperforge/worker/asset_state.py` lines 1-243) — verified mode detection, switch, rendering, index building, workspace migration, lifecycle/health/maturity computation -- **PaperForge project context** (`.planning/PROJECT.md`, `.planning/STATE.md`, `AGENTS.md`) — v1.8 milestone definition, thin-shell constraint, bug reports -- **Obsidian Developer Docs** (Context7: `/obsidianmd/obsidian-developer-docs`) — Event system, `registerEvent()`, `active-leaf-change`, `debounce()` best practices -- **Obsidian Forum #31841** — `active-leaf-change` double-fire behavior confirmed by multiple plugin developers -- **Obsidian Forum #91927** — GB2312 to UTF-8 conversion corrupting Chinese files — encoding mismatch pattern -- **opencode-obsidian Issue #28** — `btoa()` crash with Chinese characters — verified fix: `Buffer.from(str).toString('base64')` +- **pytest documentation:** https://pytest.org/ — fixture scopes, tmp_path, markers, import modes +- **GitHub Actions setup-python:** https://github.com/actions/setup-python — matrix patterns, version management +- **GitHub Actions docs:** https://docs.github.com/en/actions/guides/building-and-testing-python — CI matrix optimization +- **responses library docs:** https://pypi.org/project/responses/ — HTTP mock library +- **Project codebase:** `tests/conftest.py`, `tests/test_ocr_state_machine.py`, `tests/test_e2e_cli.py`, `tests/test_e2e_pipeline.py`, `tests/test_plugin_install_bootstrap.py` — existing 473+ test behavioral analysis +- **Project version schema:** `paperforge/__init__.py`, `manifest.json`, `pyproject.toml` — multiple version sources ### Secondary (MEDIUM confidence) -- **Obsidian Smart Chat** — Chat thread linking, Dataview dashboards, chat-active/chat-done tracking -- **Obsidian Copilot (DeepWiki)** — Chat persistence and history system with markdown files, YAML frontmatter, session grouping -- **Claude Sessions plugin** — Session timeline rendering, summary dashboard, Obsidian Bases dashboards -- **Gemini Scribe** — Per-note history file pattern, auto-appending -- **nodejs/undici Issue #5002** — Multi-byte UTF-8 character corruption at chunk boundaries with CJK text -- **paperclipai/paperclip Issue #3940** — GBK-UTF8 encoding mismatch on Windows with CJK child process output -- **Excalidraw plugin commit 5c628e0** — Real-world debounce timer cleanup in Obsidian plugin `onunload` -- **obsidian-current-view commit ad110f7** — Replacing requestAnimationFrame with setTimeout debounce +- **pytest-snapshot:** https://pypi.org/project/pytest-snapshot/ — verified via PyPI search; community-maintained +- **obsidian-test-mocks:** https://www.npmjs.com/package/obsidian-test-mocks — verified via Exa search; community-maintained +- **Vitest:** https://vitest.dev/ — official docs +- **pytest-with-eric.com** (2024-2026): Fixture management, temp directory strategies, flaky test stabilization — community expert patterns +- **CircleCI documentation:** Test splitting, parallelism strategies — adapted for GitHub Actions +- **snapshot testing analysis (2025-01):** "Why Snapshot Testing Sucks" — targeted behavior-driven assertions ### Tertiary (LOW confidence) -- **PaulGP llms.txt proposal** — AI-readable paper annotations design philosophy (context only, not implementation reference) -- **Effortless Academic discussion writing guide** — Q&A-style paper discussion structure (practitioner guide, not technical reference) -- **Templater Issue #1629** — `app.vault.modify()` race condition with multiple handlers (relevant but unverified for PaperForge's specific use case) +- **obsidian-e2e:** https://www.npmjs.com/package/obsidian-e2e — evaluated and rejected; experimental, needs real Obsidian process + +### Research Files (full details) +- **STACK.md:** Complete technology stack with versions, rationale, alternatives, and installation config (.planning/research/STACK.md) +- **FEATURES.md:** Full feature landscape with table stakes, differentiators, anti-features, dependencies, and MVP recommendation (.planning/research/FEATURES.md) +- **ARCHITECTURE.md:** Complete architecture with directory structure, mock systems, CI matrix, snapshot strategy, 6 ADRs, and build order (.planning/research/ARCHITECTURE.md) +- **PITFALLS.md:** 13 pitfalls with prevention strategies, recovery costs, "looks done but isn't" checklist, performance traps, and integration gotchas (.planning/research/PITFALLS.md) --- - -*Research completed: 2026-05-06* +*Research completed: 2026-05-08* *Ready for roadmap: yes* -*Conflicts resolved: 2 (schema shape → sessions-based; file read strategy → vault.adapter.read for vault-internal files)* -*Gates cleared: Zero new dependencies, zero CLI command changes, thin-shell principle preserved*