mirror of
https://github.com/lllin000/PaperForge.git
synced 2026-07-22 06:50:53 +00:00
test: lock OCR phase2 figure inventory contract
This commit is contained in:
parent
514c045e0a
commit
a38e41ba78
49 changed files with 245 additions and 130 deletions
|
|
@ -1,56 +1,101 @@
|
|||
---
|
||||
phase: 02-code-review-command
|
||||
reviewed: 2026-05-24T09:10:00Z
|
||||
reviewed: 2026-06-04T15:30:00Z
|
||||
depth: standard
|
||||
files_reviewed: 1
|
||||
files_reviewed: 2
|
||||
files_reviewed_list:
|
||||
- paperforge/plugin/package.json
|
||||
- tests/test_ocr_artifacts.py
|
||||
- tests/test_ocr.py
|
||||
findings:
|
||||
critical: 0
|
||||
warning: 0
|
||||
warning: 1
|
||||
info: 0
|
||||
total: 0
|
||||
status: clean
|
||||
total: 1
|
||||
status: issues_found
|
||||
---
|
||||
|
||||
# Phase 02: Code Review Report
|
||||
# Phase 2: Code Review Report -- Task 1 OCR Structured Pipeline
|
||||
|
||||
**Reviewed:** 2026-05-24T09:10:00Z
|
||||
**Reviewed:** 2026-06-04T15:30:00Z
|
||||
**Depth:** standard
|
||||
**Files Reviewed:** 1
|
||||
**Status:** clean
|
||||
**Files Reviewed:** 2
|
||||
**Status:** issues_found
|
||||
|
||||
## Summary
|
||||
|
||||
Reviewed Task 1 implementation (commit `8982c67`) — installation of TypeScript + esbuild build system dependencies in `paperforge/plugin/package.json`, as specified in [plugin-ts-migration plan](docs/superpowers/plans/2026-05-24-plugin-ts-migration.md).
|
||||
This task creates three TDD contract tests across two files that lock the Phase 1 OCR artifact contract (directory layout, version payload schema, postprocess artifact emission) in a failing state, exactly as specified in the plan. The tests will be made to pass by subsequent tasks.
|
||||
|
||||
**Result: All reviewed files meet quality standards. No issues found.**
|
||||
The implementation is correct in intent and faithfully follows the plan. However, one code quality defect was found: a missing `Path` import in `tests/test_ocr.py` that creates a fragility when the annotation is evaluated on Python < 3.14 and violates the project's established convention (every other test file imports `Path`).
|
||||
|
||||
### What was verified
|
||||
All 3 new tests fail as expected in the current environment:
|
||||
- 2 fail with `ModuleNotFoundError` (target module does not exist yet) -- correct
|
||||
- 1 fails with `AssertionError` (artifact files not written yet) -- correct
|
||||
|
||||
| Check | Result |
|
||||
|-------|--------|
|
||||
| **Plan alignment** | EXACT match — every version, script, and dependency matches the plan verbatim |
|
||||
| **Scripts added** | `dev` and `build` — exactly as specified |
|
||||
| **DevDeps added** | `typescript ^5.4.0`, `esbuild ^0.25.0`, `builtin-modules ^3.3.0`, `@types/node ^20.0.0` |
|
||||
| **Existing deps preserved** | `vitest`, `obsidian-test-mocks`, `jsdom`, `obsidian` — untouched |
|
||||
| **`package-lock.json`** | Committed alongside package.json (659 insertions, 120 deletions) |
|
||||
| **`npm install`** | Clean — all 4 packages installed, `npm ci --dry-run` reports "up to date" |
|
||||
| **`npm audit`** | 0 vulnerabilities (dev-only deps are expected security-wise) |
|
||||
| **Installed versions** | TypeScript 5.9.3, esbuild 0.25.12, builtin-modules 3.3.0, @types/node 20.19.41 |
|
||||
| **Commit message** | `"chore(plugin): add esbuild, typescript, @types/node devDeps"` — matches plan |
|
||||
| **No extra files** | Only `package.json` and `package-lock.json` were modified |
|
||||
| **Version reasonableness** | All ranges are stable and compatible with the planned tech stack |
|
||||
All 4 pre-existing tests in `tests/test_ocr.py` continue to pass -- no regression detected.
|
||||
|
||||
### Specific concerns checked and cleared
|
||||
### Test Failure Diagnostics
|
||||
|
||||
- **`esbuild.config.mjs` not yet existing**: Expected — it will be added in Task 3. The scripts reference it preemptively, which is by design.
|
||||
- **Single-dash `-noEmit` / `-skipLibCheck`**: Intentional — matches the official Obsidian sample plugin convention, as specified in the plan.
|
||||
- **`builtin-modules` necessity**: Required by the esbuild config (Task 3) to extern Node.js built-ins at bundle time. Correct inclusion.
|
||||
- **`@types/node` scope**: `^20.0.0` correctly restricts to Node 20.x LTS types. No version creep risk.
|
||||
| Test | Failure Type | Expected? |
|
||||
|------|-------------|-----------|
|
||||
| `test_phase1_artifact_layout_is_paper_local` | `ModuleNotFoundError: No module named 'paperforge.worker.ocr_artifacts'` | YES -- TDD contract, module not yet created |
|
||||
| `test_raw_and_derived_version_payloads_have_separate_namespaces` | `ModuleNotFoundError: No module named 'paperforge.worker.ocr_artifacts'` | YES -- TDD contract, module not yet created |
|
||||
| `test_postprocess_writes_phase1_artifacts` | `AssertionError: assert False` at line 169 | YES -- `postprocess_ocr_result()` does not yet write Phase 1 artifacts |
|
||||
|
||||
## Warnings
|
||||
|
||||
### WR-01: Missing `Path` import in `test_ocr.py` creates annotation fragility
|
||||
|
||||
**File:** `tests/test_ocr.py:154`
|
||||
**Issue:** The function signature `def test_postprocess_writes_phase1_artifacts(tmp_path: Path) -> None:` references `Path` in a type annotation, but `Path` is not imported in `tests/test_ocr.py`. This file has zero module-level imports -- the only test file in the project with this characteristic. Every other test file consistently uses `from pathlib import Path`.
|
||||
|
||||
This works in Python 3.14 due to PEP 649 (deferred annotation evaluation), but it:
|
||||
- **Fails with `NameError` on Python < 3.14** if the module is imported without `from __future__ import annotations`
|
||||
- Creates a hidden dependency on the conftest's import order and PEP 649 being active
|
||||
- Violates the project's established convention (all other test files with `Path` annotations import it explicitly)
|
||||
|
||||
**Fix:** Add `from pathlib import Path` to `tests/test_ocr.py` at module level. This is consistent with the pattern used in `tests/test_ocr_redo.py`, `tests/test_ocr_artifacts.py`, `tests/test_context.py`, and every other test file in the project.
|
||||
|
||||
```python
|
||||
# Add to tests/test_ocr.py, before the first test function:
|
||||
from pathlib import Path
|
||||
```
|
||||
|
||||
Alternatively, if the project intentionally uses annotation-only mode for all test files, add `from __future__ import annotations` at the top of `tests/test_ocr.py` (matching `tests/test_ocr_artifacts.py` and `tests/conftest.py`).
|
||||
|
||||
## Assessment
|
||||
|
||||
### Strengths
|
||||
|
||||
1. **Plan alignment:** The implementation matches the plan exactly -- both `test_ocr_artifacts.py` tests and the `test_ocr.py` addition are verbatim from the spec.
|
||||
|
||||
2. **Design alignment:** The contract tests correctly reflect the design spec (`docs/superpowers/specs/2026-06-04-ocr-structured-pipeline-design.md`):
|
||||
- `raw/raw_meta.json` and `raw/source_metadata.json` match Layer 0/1 artifacts
|
||||
- `canonical/blocks.raw.jsonl` matches Layer 2
|
||||
- `structure/blocks.structured.jsonl` matches Layer 3
|
||||
- `raw_version`/`derived_version` payloads match Section 6 version model
|
||||
|
||||
3. **Well-scoped scope:** The task did not leak into implementation work -- no production source files were modified, no `ocr_artifacts.py` was prematurely stubbed.
|
||||
|
||||
4. **Explicit path assertions:** Using `.as_posix().endswith(...)` in `test_ocr_artifacts.py` correctly handles Windows vs POSIX path separators, making the tests cross-platform.
|
||||
|
||||
5. **Clean new file:** `tests/test_ocr_artifacts.py` is small (29 lines), has a single responsibility (contract tests), and is independently importable.
|
||||
|
||||
### Defects
|
||||
|
||||
| ID | Severity | Description | File:Line |
|
||||
|----|----------|-------------|-----------|
|
||||
| WR-01 | Warning | Missing `Path` import in `test_ocr.py` | `tests/test_ocr.py:154` |
|
||||
|
||||
### Verification
|
||||
|
||||
- Module-level imports absent from `tests/test_ocr.py`: confirmed by `rg` (zero results)
|
||||
- All other 21 test files import `Path` from `pathlib`: confirmed by `Select-String` across `tests/*.py`
|
||||
- All 3 new tests fail as expected: confirmed by running `pytest` against both files
|
||||
- All 4 pre-existing tests still pass: confirmed by `pytest` output
|
||||
- Design spec alignment: confirmed by reading the design doc
|
||||
|
||||
---
|
||||
|
||||
_Reviewed: 2026-05-24T09:10:00Z_
|
||||
_Reviewed: 2026-06-04T15:30:00Z_
|
||||
_Reviewer: VT-OS/OPENCODE (gsd-code-reviewer)_
|
||||
_Depth: standard_
|
||||
|
|
|
|||
|
|
@ -140,8 +140,8 @@ def run(args: argparse.Namespace) -> int:
|
|||
total = len(done_papers)
|
||||
print(f"EMBED_START:{total}", flush=True)
|
||||
|
||||
import os as _os
|
||||
import gc as _gc
|
||||
import os as _os
|
||||
_now = __import__('datetime').datetime.now(__import__('datetime').timezone.utc).isoformat
|
||||
|
||||
papers_embedded = 0
|
||||
|
|
|
|||
|
|
@ -4,7 +4,6 @@ import re
|
|||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
HIGH_SIGNAL_PRIORITY = [
|
||||
"doi",
|
||||
"zotero_key",
|
||||
|
|
|
|||
|
|
@ -267,7 +267,6 @@ def _build_entry(item: dict, vault: Path, paths: dict, domain: str, zotero_dir:
|
|||
Lazy imports inside avoid circular dependencies with ``sync.py``.
|
||||
"""
|
||||
# Lazy imports to avoid circular deps with sync.py
|
||||
import shutil
|
||||
|
||||
from paperforge import __version__ as PAPERFORGE_VERSION
|
||||
from paperforge.adapters.obsidian_frontmatter import has_deep_reading_content
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ from __future__ import annotations
|
|||
import logging
|
||||
from pathlib import Path
|
||||
|
||||
from paperforge.adapters.obsidian_frontmatter import has_deep_reading_content
|
||||
from paperforge.worker._domain import load_domain_config
|
||||
from paperforge.worker._utils import (
|
||||
get_analyze_queue,
|
||||
|
|
@ -10,7 +11,6 @@ from paperforge.worker._utils import (
|
|||
)
|
||||
from paperforge.worker.asset_index import refresh_index_entry
|
||||
from paperforge.worker.base_views import ensure_base_views
|
||||
from paperforge.adapters.obsidian_frontmatter import has_deep_reading_content
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
|
|
|||
|
|
@ -6,12 +6,10 @@ ALL chaos test vaults MUST use tmp_path and include the isolation assertion:
|
|||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
import random
|
||||
import shutil
|
||||
import stat
|
||||
import string
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
|
|
|||
|
|
@ -16,10 +16,8 @@ from tests.chaos.conftest import (
|
|||
corrupt_pdf,
|
||||
create_broken_meta_json,
|
||||
setup_vault_from_export,
|
||||
strip_frontmatter_fields,
|
||||
)
|
||||
|
||||
|
||||
pytestmark = pytest.mark.chaos
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -12,7 +12,6 @@ from pathlib import Path
|
|||
|
||||
import pytest
|
||||
|
||||
|
||||
pytestmark = pytest.mark.chaos
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -15,7 +15,6 @@ from pathlib import Path
|
|||
|
||||
import pytest
|
||||
|
||||
|
||||
pytestmark = pytest.mark.chaos
|
||||
|
||||
# Short timeout for network tests — the OCR module has retry+backoff logic
|
||||
|
|
|
|||
|
|
@ -80,7 +80,7 @@ def mock_ocr_backend():
|
|||
This fixture is context-managed — use it with 'with' statement.
|
||||
"""
|
||||
import json
|
||||
from pathlib import Path
|
||||
|
||||
import responses as _responses
|
||||
|
||||
ocr_fixtures_dir = Path(__file__).resolve().parent.parent.parent / "fixtures" / "ocr"
|
||||
|
|
@ -129,5 +129,4 @@ def mock_ocr_backend():
|
|||
@pytest.fixture
|
||||
def snapshot_dir():
|
||||
"""Return path to the snapshots directory."""
|
||||
from pathlib import Path
|
||||
return Path(__file__).resolve().parent.parent.parent / "fixtures" / "snapshots"
|
||||
|
|
|
|||
|
|
@ -4,7 +4,6 @@ from __future__ import annotations
|
|||
|
||||
import json
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
def normalize_snapshot(output: str, vault_path: str | None = None) -> str:
|
||||
|
|
|
|||
|
|
@ -2,8 +2,6 @@
|
|||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
class TestExitCodes:
|
||||
"""Contract: CLI commands exit with correct exit codes."""
|
||||
|
|
|
|||
|
|
@ -3,11 +3,9 @@
|
|||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
pytestmark = pytest.mark.e2e
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -10,7 +10,6 @@ from pathlib import Path
|
|||
|
||||
import pytest
|
||||
|
||||
|
||||
pytestmark = pytest.mark.e2e
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -6,7 +6,6 @@ import json
|
|||
|
||||
import pytest
|
||||
|
||||
|
||||
pytestmark = pytest.mark.e2e
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -3,11 +3,9 @@
|
|||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
pytestmark = pytest.mark.e2e
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -91,9 +91,10 @@ def journey_established_vault(tmp_path: Path) -> tuple[Path, object]:
|
|||
Returns:
|
||||
(vault_path, vault_builder) tuple so tests can inspect or extend.
|
||||
"""
|
||||
from fixtures.vault_builder import VaultBuilder
|
||||
import json
|
||||
|
||||
from fixtures.vault_builder import VaultBuilder
|
||||
|
||||
builder = VaultBuilder()
|
||||
vault = Path(builder.build("full"))
|
||||
|
||||
|
|
|
|||
|
|
@ -6,13 +6,11 @@ Simulates an existing user: established vault with papers -> add new paper -> sy
|
|||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import shutil
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
import yaml
|
||||
|
||||
|
||||
pytestmark = pytest.mark.journey
|
||||
|
||||
|
||||
|
|
@ -156,8 +154,8 @@ def test_existing_user_adds_paper(
|
|||
# Verify new note frontmatter
|
||||
fm = _read_frontmatter(new_note)
|
||||
assert fm.get("zotero_key") == new_key, f"Expected zotero_key {new_key}, got {fm.get('zotero_key')}"
|
||||
assert "domain" in fm and fm.get("domain"), f"Missing or empty domain in new note"
|
||||
assert "has_pdf" in fm, f"Missing has_pdf in new note"
|
||||
assert "domain" in fm and fm.get("domain"), "Missing or empty domain in new note"
|
||||
assert "has_pdf" in fm, "Missing has_pdf in new note"
|
||||
|
||||
# Verify existing notes are unchanged (same frontmatter hash)
|
||||
for n in current_notes:
|
||||
|
|
@ -196,8 +194,8 @@ def test_existing_user_adds_paper(
|
|||
# Verify new note frontmatter
|
||||
fm = _read_frontmatter(new_note)
|
||||
assert fm.get("zotero_key") == new_key, f"Expected zotero_key {new_key}, got {fm.get('zotero_key')}"
|
||||
assert "domain" in fm and fm.get("domain"), f"Missing or empty domain in new note"
|
||||
assert "has_pdf" in fm, f"Missing has_pdf in new note"
|
||||
assert "domain" in fm and fm.get("domain"), "Missing or empty domain in new note"
|
||||
assert "has_pdf" in fm, "Missing has_pdf in new note"
|
||||
assert fm.get("ocr_status") in ("pending",), f"Expected pending ocr_status, got {fm.get('ocr_status')}"
|
||||
|
||||
# Verify existing notes are unchanged (same frontmatter hash)
|
||||
|
|
|
|||
|
|
@ -5,7 +5,6 @@ Simulates a brand-new user: fresh vault -> sync -> OCR -> analyze -> deep-read r
|
|||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
|
|
@ -14,7 +13,6 @@ import yaml
|
|||
|
||||
from fixtures.ocr.mock_ocr_backend import mock_ocr_success
|
||||
|
||||
|
||||
pytestmark = pytest.mark.journey
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -1,6 +1,5 @@
|
|||
"""Precise batch defect report."""
|
||||
import re
|
||||
import json
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
|
|
@ -112,4 +111,4 @@ for key in papers:
|
|||
verdict = ", ".join(issues) if issues else "CLEAN"
|
||||
print(f" {key}: {img_num}i/{cap_num}c | {verdict}")
|
||||
|
||||
print(f"\nDone.")
|
||||
print("\nDone.")
|
||||
|
|
|
|||
|
|
@ -13,9 +13,6 @@ from __future__ import annotations
|
|||
import json
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helpers (matching test_asset_index.py pattern)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
@ -113,7 +110,6 @@ def _setup_incremental_vault(
|
|||
Path to the vault root.
|
||||
"""
|
||||
from paperforge.worker.asset_index import (
|
||||
build_envelope,
|
||||
get_index_path,
|
||||
)
|
||||
|
||||
|
|
@ -217,7 +213,6 @@ class TestIncrementalRefreshBehavior:
|
|||
def test_incremental_refresh_fallback_on_legacy(self, tmp_path: Path) -> None:
|
||||
"""refresh_index_entry falls back to build_index when index is legacy format."""
|
||||
from paperforge.worker.asset_index import (
|
||||
build_envelope,
|
||||
get_index_path,
|
||||
is_legacy_format,
|
||||
read_index,
|
||||
|
|
@ -298,8 +293,6 @@ class TestWorkspacePaths:
|
|||
def test_workspace_paths_in_entry(self, tmp_path: Path) -> None:
|
||||
"""Each entry in the built index contains all 5 workspace path fields."""
|
||||
from paperforge.worker.asset_index import (
|
||||
build_envelope,
|
||||
get_index_path,
|
||||
read_index,
|
||||
refresh_index_entry,
|
||||
)
|
||||
|
|
@ -464,7 +457,7 @@ class TestDerivedStateFields:
|
|||
assert "note_health" in health
|
||||
assert "asset_health" in health
|
||||
for v in health.values():
|
||||
assert isinstance(v, str) and len(v) > 0, f"health value should be non-empty string"
|
||||
assert isinstance(v, str) and len(v) > 0, "health value should be non-empty string"
|
||||
|
||||
def test_maturity_structure(self, tmp_path: Path) -> None:
|
||||
"""maturity field is a dict with level (int 1-4), level_name (str), checks (4 bools), blocking."""
|
||||
|
|
|
|||
|
|
@ -6,11 +6,9 @@ Test entries are plain dicts matching the canonical index entry shape.
|
|||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
from paperforge.worker.asset_state import (
|
||||
compute_lifecycle,
|
||||
compute_health,
|
||||
compute_lifecycle,
|
||||
compute_maturity,
|
||||
compute_next_step,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -508,6 +508,7 @@ def test_get_paperforge_schema_version_defaults_to_1(tmp_path: Path):
|
|||
def test_get_paperforge_schema_version_reads_2(tmp_path: Path):
|
||||
"""get_paperforge_schema_version returns 2 when schema_version is '2'."""
|
||||
import json
|
||||
|
||||
from paperforge.config import get_paperforge_schema_version
|
||||
|
||||
vault = tmp_path / "schema_v2"
|
||||
|
|
@ -539,6 +540,7 @@ def test_load_vault_config_excludes_schema_version(tmp_path: Path):
|
|||
def test_migrate_legacy_top_level_keys(tmp_path: Path):
|
||||
"""Legacy top-level path keys are migrated to vault_config block."""
|
||||
import json
|
||||
|
||||
from paperforge.config import migrate_paperforge_json
|
||||
|
||||
vault = tmp_path / "legacy_migrate"
|
||||
|
|
@ -569,6 +571,7 @@ def test_migrate_legacy_top_level_keys(tmp_path: Path):
|
|||
def test_migrate_idempotent(tmp_path: Path):
|
||||
"""Already-migrated files return False (no-op)."""
|
||||
import json
|
||||
|
||||
from paperforge.config import migrate_paperforge_json
|
||||
|
||||
vault = tmp_path / "already_migrated"
|
||||
|
|
@ -594,6 +597,7 @@ def test_migrate_no_file(tmp_path: Path):
|
|||
def test_migrate_non_path_keys_survive(tmp_path: Path):
|
||||
"""Non-path top-level keys survive migration in output root."""
|
||||
import json
|
||||
|
||||
from paperforge.config import migrate_paperforge_json
|
||||
|
||||
vault = tmp_path / "non_path_survive"
|
||||
|
|
@ -619,6 +623,7 @@ def test_migrate_non_path_keys_survive(tmp_path: Path):
|
|||
def test_migrate_no_vault_config_block_creates_it(tmp_path: Path):
|
||||
"""Top-level keys without vault_config block create a new vault_config."""
|
||||
import json
|
||||
|
||||
from paperforge.config import migrate_paperforge_json
|
||||
|
||||
vault = tmp_path / "creates_vc"
|
||||
|
|
|
|||
|
|
@ -10,9 +10,6 @@ import argparse
|
|||
import json
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -8,13 +8,10 @@ from __future__ import annotations
|
|||
|
||||
import json
|
||||
import os
|
||||
import tempfile
|
||||
from pathlib import Path
|
||||
|
||||
import filelock
|
||||
|
||||
import pytest
|
||||
|
||||
from paperforge.worker.discussion import record_session
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -14,8 +14,6 @@ from __future__ import annotations
|
|||
import re
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from paperforge.config import paperforge_paths
|
||||
from paperforge.worker.asset_index import build_index, read_index
|
||||
from paperforge.worker.sync import run_index_refresh, run_selection_sync
|
||||
|
|
|
|||
|
|
@ -7,7 +7,6 @@ from pathlib import Path
|
|||
|
||||
import pytest
|
||||
|
||||
|
||||
VAULT = Path(r"D:\L\OB\Literature-hub")
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -7,12 +7,10 @@ idempotency, backward compatibility, and run_index_refresh integration.
|
|||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -293,8 +293,6 @@ def test_postprocess_writes_phase1_artifacts(tmp_path: Path) -> None:
|
|||
assert (ocr_dir / "canonical" / "blocks.raw.jsonl").exists()
|
||||
assert (ocr_dir / "structure" / "blocks.structured.jsonl").exists()
|
||||
|
||||
import json
|
||||
|
||||
meta = json.loads((ocr_dir / "meta.json").read_text(encoding="utf-8"))
|
||||
assert "raw_version" in meta
|
||||
assert meta["raw_version"]["ocr_model"] == "PaddleOCR-VL-1.6"
|
||||
|
|
@ -329,3 +327,29 @@ def test_postprocess_writes_resolved_metadata(tmp_path: Path) -> None:
|
|||
# Phase 2: resolved_metadata.json does not exist yet — this test will fail
|
||||
# The assertion goes here as a contract test
|
||||
assert (ocr_dir / "metadata" / "resolved_metadata.json").exists()
|
||||
|
||||
|
||||
def test_postprocess_writes_figure_inventory(tmp_path: Path) -> None:
|
||||
import json
|
||||
|
||||
from paperforge.worker.ocr import postprocess_ocr_result
|
||||
|
||||
vault = tmp_path / "vault"
|
||||
vault.mkdir()
|
||||
(vault / "paperforge.json").write_text(
|
||||
json.dumps({"vault_config": {"system_dir": "System", "resources_dir": "Resources"}}),
|
||||
encoding="utf-8",
|
||||
)
|
||||
ocr_root = vault / "System" / "PaperForge" / "ocr"
|
||||
ocr_root.mkdir(parents=True)
|
||||
ocr_dir = ocr_root / "FIG001"
|
||||
ocr_dir.mkdir()
|
||||
(ocr_dir / "meta.json").write_text(
|
||||
'{"zotero_key":"FIG001","ocr_status":"done","ocr_model":"PaddleOCR"}',
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
_, _, _, _ = postprocess_ocr_result(vault, "FIG001", [])
|
||||
|
||||
# Phase 2: figure_inventory.json does not exist yet -- test will fail
|
||||
assert (ocr_dir / "structure" / "figure_inventory.json").exists()
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
|
|
|
|||
76
tests/test_ocr_figures.py
Normal file
76
tests/test_ocr_figures.py
Normal file
|
|
@ -0,0 +1,76 @@
|
|||
"""Phase 2 contract tests for figure inventory.
|
||||
|
||||
paperforge.worker.ocr_figures does not exist yet -- tests will fail until
|
||||
Task 5 implements the module.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
|
||||
def test_formal_figure_count_is_based_on_legends_not_raw_images() -> None:
|
||||
from paperforge.worker.ocr_figures import build_figure_inventory
|
||||
|
||||
structured_blocks = [
|
||||
{
|
||||
"paper_id": "KEY001",
|
||||
"page": 3,
|
||||
"block_id": "p3_b21",
|
||||
"role": "figure_caption",
|
||||
"text": "Figure 1. Left column figure.",
|
||||
"bbox": [66, 446, 559, 628],
|
||||
},
|
||||
{
|
||||
"paper_id": "KEY001",
|
||||
"page": 3,
|
||||
"block_id": "p3_b22",
|
||||
"role": "figure_asset",
|
||||
"text": "",
|
||||
"bbox": [80, 116, 546, 434],
|
||||
},
|
||||
{
|
||||
"paper_id": "KEY001",
|
||||
"page": 3,
|
||||
"block_id": "p3_b23",
|
||||
"role": "figure_asset",
|
||||
"text": "",
|
||||
"bbox": [598, 114, 1063, 493],
|
||||
},
|
||||
]
|
||||
|
||||
inventory = build_figure_inventory(structured_blocks)
|
||||
|
||||
assert inventory["official_figure_count"] == 1
|
||||
assert len(inventory["figure_legends"]) == 1
|
||||
|
||||
|
||||
def test_figure_inventory_includes_all_sections() -> None:
|
||||
from paperforge.worker.ocr_figures import build_figure_inventory
|
||||
|
||||
inventory = build_figure_inventory([])
|
||||
|
||||
assert "figure_legends" in inventory
|
||||
assert "figure_assets" in inventory
|
||||
assert "matched_figures" in inventory
|
||||
assert "unmatched_legends" in inventory
|
||||
assert "unmatched_assets" in inventory
|
||||
assert "official_figure_count" in inventory
|
||||
|
||||
|
||||
def test_unmatched_assets_are_preserved() -> None:
|
||||
from paperforge.worker.ocr_figures import build_figure_inventory
|
||||
|
||||
structured_blocks = [
|
||||
{
|
||||
"paper_id": "KEY001",
|
||||
"page": 5,
|
||||
"block_id": "p5_b10",
|
||||
"role": "figure_asset",
|
||||
"text": "",
|
||||
"bbox": [100, 100, 500, 400],
|
||||
},
|
||||
]
|
||||
|
||||
inventory = build_figure_inventory(structured_blocks)
|
||||
|
||||
assert inventory["official_figure_count"] == 0
|
||||
assert len(inventory["unmatched_assets"]) == 1
|
||||
|
|
@ -134,3 +134,26 @@ def test_fixture_raw_blocks_nonzero(tmp_path) -> None:
|
|||
write_raw_blocks_jsonl(out_path, rows)
|
||||
assert out_path.exists()
|
||||
assert out_path.stat().st_size > 0
|
||||
|
||||
|
||||
# Phase 2: forward-looking contract test; fails until ocr_figures module exists
|
||||
def test_fixture_figure_inventory_basic(tmp_path) -> None:
|
||||
"""Verify figure inventory can run against a real OCR fixture."""
|
||||
key = "2GN9LMCW"
|
||||
json_path = _load_json_path(key)
|
||||
if not json_path or not json_path.exists():
|
||||
pytest.skip("fixture not available")
|
||||
|
||||
data = json.loads(json_path.read_text(encoding="utf-8"))
|
||||
from paperforge.worker.ocr_blocks import build_raw_blocks_for_result_lines, build_structured_blocks
|
||||
|
||||
raw_blocks = build_raw_blocks_for_result_lines(key, data)
|
||||
structured = build_structured_blocks(raw_blocks)
|
||||
|
||||
from paperforge.worker.ocr_figures import build_figure_inventory
|
||||
|
||||
inventory = build_figure_inventory(structured)
|
||||
|
||||
assert isinstance(inventory["official_figure_count"], int)
|
||||
assert isinstance(inventory["figure_legends"], list)
|
||||
assert isinstance(inventory["figure_assets"], list)
|
||||
|
|
|
|||
|
|
@ -895,7 +895,6 @@ class TestOcrEdgeCases:
|
|||
"""max_items=1 throttles concurrency but never leaves items unprocessed."""
|
||||
vault, ocr_root, exports, library_records = _make_vault(tmp_path)
|
||||
paths = _mock_paths(vault, ocr_root, exports, library_records)
|
||||
import os as _os
|
||||
|
||||
with patch.dict(
|
||||
"os.environ",
|
||||
|
|
@ -1118,12 +1117,10 @@ class TestOcrEdgeCases:
|
|||
|
||||
def test_full_cycle_from_pending_to_done(self, tmp_path: Path) -> None:
|
||||
"""Happy path: pending -> upload -> queued -> poll done -> postprocess -> done."""
|
||||
import sys as _sys
|
||||
from pathlib import Path as P
|
||||
|
||||
import sys as _sys
|
||||
|
||||
_sys.path.insert(0, str(P(__file__).resolve().parent.parent))
|
||||
from paperforge.worker.ocr import postprocess_ocr_result
|
||||
|
||||
vault, ocr_root, exports, library_records = _make_vault(tmp_path)
|
||||
paths = _mock_paths(vault, ocr_root, exports, library_records)
|
||||
|
|
|
|||
|
|
@ -2,7 +2,6 @@ from __future__ import annotations
|
|||
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parent.parent
|
||||
PLUGIN_MAIN = REPO_ROOT / "paperforge" / "plugin" / "main.js"
|
||||
|
||||
|
|
|
|||
|
|
@ -4,8 +4,8 @@ from pathlib import Path
|
|||
|
||||
import pytest
|
||||
|
||||
from paperforge.query_planning import build_query_plan, enrich_query_plan_with_runtime
|
||||
from paperforge.commands.query_plan import run as query_plan_run
|
||||
from paperforge.query_planning import build_query_plan, enrich_query_plan_with_runtime
|
||||
|
||||
|
||||
class _Args:
|
||||
|
|
|
|||
|
|
@ -521,6 +521,7 @@ class TestRepairExceptionLogging:
|
|||
def test_index_write_failure_first_branch_logs_warning(self, tmp_path, caplog, monkeypatch):
|
||||
"""Index write exception in first --fix branch at line 306 logs warning."""
|
||||
import logging
|
||||
|
||||
import paperforge.worker.repair as repair_mod
|
||||
caplog.set_level(logging.WARNING, logger="paperforge.worker.repair")
|
||||
|
||||
|
|
@ -544,6 +545,7 @@ class TestRepairExceptionLogging:
|
|||
def test_index_write_failure_second_branch_logs_warning(self, tmp_path, caplog, monkeypatch):
|
||||
"""Index write exception in second --fix branch at line 347 logs warning."""
|
||||
import logging
|
||||
|
||||
import paperforge.worker.repair as repair_mod
|
||||
caplog.set_level(logging.WARNING, logger="paperforge.worker.repair")
|
||||
|
||||
|
|
@ -567,6 +569,7 @@ class TestRepairExceptionLogging:
|
|||
def test_meta_write_failure_logs_warning(self, tmp_path, caplog, monkeypatch):
|
||||
"""Meta write exception in second --fix branch at line 355 logs warning."""
|
||||
import logging
|
||||
|
||||
import paperforge.worker.repair as repair_mod
|
||||
caplog.set_level(logging.WARNING, logger="paperforge.worker.repair")
|
||||
|
||||
|
|
|
|||
|
|
@ -24,9 +24,8 @@ if str(REPO_ROOT) not in sys.path:
|
|||
sys.path.insert(0, str(REPO_ROOT))
|
||||
|
||||
# Import standalone functions and classes (not Textual-dependent)
|
||||
from paperforge.setup_wizard import CheckResult, EnvChecker, _find_vault
|
||||
from paperforge.services.skill_deploy import AGENT_SKILL_DIRS
|
||||
|
||||
from paperforge.setup_wizard import CheckResult, EnvChecker, _find_vault
|
||||
|
||||
# ===================================================================
|
||||
# AGENT_SKILL_DIRS (imported from skill_deploy, flat dict)
|
||||
|
|
@ -288,8 +287,8 @@ class TestEnvCheckerCheckJson:
|
|||
|
||||
def test_headless_setup_claude_skill_directory(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Claude Code uses skill_directory format — creates .claude/skills/pf-deep/SKILL.md etc."""
|
||||
from paperforge.setup_wizard import headless_setup
|
||||
import paperforge.setup_wizard as sw
|
||||
from paperforge.setup_wizard import headless_setup
|
||||
|
||||
def patched_check_deps(self) -> CheckResult:
|
||||
r = CheckResult("Dependencies")
|
||||
|
|
@ -320,8 +319,8 @@ def test_headless_setup_claude_skill_directory(tmp_path: Path, monkeypatch: pyte
|
|||
|
||||
def test_headless_setup_opencode_flat_command(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""OpenCode deploys literature-qa skill to skills dir; flat commands are bundled in SKILL.md."""
|
||||
from paperforge.setup_wizard import headless_setup
|
||||
import paperforge.setup_wizard as sw
|
||||
from paperforge.setup_wizard import headless_setup
|
||||
|
||||
def patched_check_deps(self) -> CheckResult:
|
||||
r = CheckResult("Dependencies")
|
||||
|
|
@ -349,8 +348,8 @@ def test_headless_setup_opencode_flat_command(tmp_path: Path, monkeypatch: pytes
|
|||
|
||||
def test_headless_setup_codex_skill_directory(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Codex deploys literature-qa skill under .codex/skills."""
|
||||
from paperforge.setup_wizard import headless_setup
|
||||
import paperforge.setup_wizard as sw
|
||||
from paperforge.setup_wizard import headless_setup
|
||||
|
||||
def patched_check_deps(self) -> CheckResult:
|
||||
r = CheckResult("Dependencies")
|
||||
|
|
@ -378,8 +377,8 @@ def test_headless_setup_codex_skill_directory(tmp_path: Path, monkeypatch: pytes
|
|||
|
||||
def test_headless_setup_cline_rules_file(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Cline deploys literature-qa skill under .clinerules."""
|
||||
from paperforge.setup_wizard import headless_setup
|
||||
import paperforge.setup_wizard as sw
|
||||
from paperforge.setup_wizard import headless_setup
|
||||
|
||||
def patched_check_deps(self) -> CheckResult:
|
||||
r = CheckResult("Dependencies")
|
||||
|
|
@ -403,14 +402,14 @@ def test_headless_setup_cline_rules_file(tmp_path: Path, monkeypatch: pytest.Mon
|
|||
clinerules = tmp_path / ".clinerules"
|
||||
assert clinerules.exists(), f".clinerules dir not created: {clinerules}"
|
||||
skill_dir = clinerules / "literature-qa"
|
||||
assert skill_dir.exists(), f"literature-qa subdir not created under .clinerules"
|
||||
assert skill_dir.exists(), "literature-qa subdir not created under .clinerules"
|
||||
assert (skill_dir / "references" / "deep-subagent.md").exists(), "deep-subagent.md not created"
|
||||
|
||||
|
||||
def test_headless_setup_preserves_existing_files(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Setup should create missing files without overwriting existing user content."""
|
||||
from paperforge.setup_wizard import headless_setup
|
||||
import paperforge.setup_wizard as sw
|
||||
from paperforge.setup_wizard import headless_setup
|
||||
|
||||
def patched_check_deps(self) -> CheckResult:
|
||||
r = CheckResult("Dependencies")
|
||||
|
|
@ -461,8 +460,8 @@ def test_headless_setup_preserves_existing_files(tmp_path: Path, monkeypatch: py
|
|||
|
||||
|
||||
def test_headless_setup_allows_empty_paddleocr_key(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
from paperforge.setup_wizard import headless_setup
|
||||
import paperforge.setup_wizard as sw
|
||||
from paperforge.setup_wizard import headless_setup
|
||||
|
||||
def patched_check_deps(self) -> CheckResult:
|
||||
r = CheckResult("Dependencies")
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parent.parent
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parent.parent
|
||||
|
|
|
|||
|
|
@ -9,9 +9,6 @@ from __future__ import annotations
|
|||
import json
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
@ -179,11 +176,11 @@ class TestDoctorIndexHealth:
|
|||
|
||||
def _run_doctor(self, vault: Path) -> str:
|
||||
"""Run doctor and return captured stdout."""
|
||||
from paperforge.worker.status import run_doctor
|
||||
|
||||
import io
|
||||
import sys
|
||||
|
||||
from paperforge.worker.status import run_doctor
|
||||
|
||||
old_stdout = sys.stdout
|
||||
sys.stdout = buf = io.StringIO()
|
||||
try:
|
||||
|
|
|
|||
|
|
@ -11,8 +11,6 @@ from __future__ import annotations
|
|||
import json
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from paperforge.worker import _utils
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -5,10 +5,6 @@ Covers: slugify_filename, _extract_year
|
|||
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from paperforge.worker._utils import _extract_year, slugify_filename
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -5,10 +5,6 @@ Covers: yaml_quote, yaml_block, yaml_list
|
|||
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from paperforge.worker._utils import yaml_block, yaml_list, yaml_quote
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -8,8 +8,8 @@ from paperforge.adapters.obsidian_frontmatter import (
|
|||
_legacy_control_flags,
|
||||
_read_frontmatter_bool_from_text,
|
||||
_read_frontmatter_optional_bool_from_text,
|
||||
canonicalize_decision,
|
||||
candidate_markdown,
|
||||
canonicalize_decision,
|
||||
compute_final_collection,
|
||||
extract_preserved_deep_reading,
|
||||
extract_preserved_tags,
|
||||
|
|
|
|||
|
|
@ -1,8 +1,7 @@
|
|||
from pathlib import Path
|
||||
|
||||
from paperforge.doctor.field_validator import (
|
||||
validate_entry_fields,
|
||||
validate_collection,
|
||||
validate_entry_fields,
|
||||
validate_frontmatter_from_file,
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -1,12 +1,11 @@
|
|||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
import json
|
||||
|
||||
from paperforge.memory.runtime_health import (
|
||||
get_runtime_health,
|
||||
_check_bootstrap,
|
||||
_check_write,
|
||||
get_runtime_health,
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -3,14 +3,14 @@ from __future__ import annotations
|
|||
import tempfile
|
||||
from pathlib import Path
|
||||
|
||||
from paperforge.memory.db import get_connection
|
||||
from paperforge.memory.schema import (
|
||||
ALL_TABLES,
|
||||
ensure_schema,
|
||||
drop_all_tables,
|
||||
get_schema_version,
|
||||
CURRENT_SCHEMA_VERSION,
|
||||
drop_all_tables,
|
||||
ensure_schema,
|
||||
get_schema_version,
|
||||
)
|
||||
from paperforge.memory.db import get_connection
|
||||
|
||||
|
||||
def test_ensure_schema_creates_all_tables():
|
||||
|
|
|
|||
|
|
@ -1,10 +1,9 @@
|
|||
from __future__ import annotations
|
||||
|
||||
from paperforge.embedding.build_state import (
|
||||
get_vector_build_state_path,
|
||||
mark_vector_build_state,
|
||||
read_vector_build_state,
|
||||
write_vector_build_state,
|
||||
mark_vector_build_state,
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -7,7 +7,6 @@ import pytest
|
|||
|
||||
from paperforge.worker.paper_resolver import PaperResolver, PaperWorkspace
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Fixtures
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
Loading…
Reference in a new issue