diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index be4efed7..a3c90e7c 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -25,6 +25,12 @@ repos: language: script types: [python] require_serial: true + - id: pytest-staged + name: pytest (staged files) + entry: script/pre-commit-test + language: script + pass_filenames: false + require_serial: true - repo: https://github.com/codespell-project/codespell rev: v2.4.1 hooks: diff --git a/.specify/memory/constitution.md b/.specify/memory/constitution.md index ade7b731..dc81c867 100644 --- a/.specify/memory/constitution.md +++ b/.specify/memory/constitution.md @@ -105,6 +105,9 @@ merged without a corresponding test that was authored before the implementation. mocking the database is prohibited. - Completeness tests (e.g., `test_*_completeness.py`) MUST be maintained alongside every model module to ensure all fields are round-trip serializable. +- **100% line coverage is required on all new and changed code before committing.** + Every branch, error path, and early return MUST have a corresponding test. + Code MUST NOT be committed until coverage is verified locally. **Rationale**: The existing test suite (80+ test files spanning routes, services, device protocol, MCP, security, and models) demonstrates that comprehensive tests @@ -224,7 +227,9 @@ All standard operations MUST use the scripts in `script/` following the development and integration testing. It starts with a clean state and a pre-seeded debug user and MUST NOT persist data between runs. -Pull requests MUST pass all CI gates (lint, type check, tests) before merge. +Pull requests MUST pass all CI gates (lint, type check, tests, coverage) before merge. +New and changed code MUST achieve 100% line coverage; PRs that reduce patch +coverage below 100% MUST NOT be merged without explicit justification. The `main` branch is the source of truth; GitHub Pages documentation is auto-deployed on every push to `main` via `pdoc`. diff --git a/CLAUDE.md b/CLAUDE.md index 20afc6bb..94032cac 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -7,6 +7,8 @@ Auto-generated from all feature plans. Last updated: 2026-03-17 - N/A (no Python source changes — CI/CD configuration only) + GitHub Actions (`docker/metadata-action`, `docker/build-push-action`, `docker/login-action`) (003-github-releases) - Python 3.13+ + aiohttp (server), SQLAlchemy asyncio + aiosqlite, mashumaro, alembic; Vanilla JS (Vue 3, no build step) for frontend (004-ui-prompt-config) - SQLite via SQLAlchemy asyncio — new `f_prompt_config` table; new `prompt_hash` column on `f_note_page_content` (004-ui-prompt-config) +- Python 3.13+ (backend), Vanilla JS / Vue 3 ESM (frontend) + aiohttp, SQLAlchemy asyncio + aiosqlite, mashumaro, alembic (005-cache-png-insights-tabs) +- SQLite (DB via SQLAlchemy), LocalBlobStorage (disk — `supernote-user-data` bucket) (005-cache-png-insights-tabs) - Python 3.13+ + mypy (strict), SQLAlchemy asyncio, aiohttp, mashumaro, pytest + pytest-asyncio (001-constitution-alignment) @@ -64,9 +66,9 @@ Button Tailwind classes — use verbatim: - **Dark mode**: every interactive element must have `dark:` variants ## Recent Changes +- 005-cache-png-insights-tabs: Added Python 3.13+ (backend), Vanilla JS / Vue 3 ESM (frontend) + aiohttp, SQLAlchemy asyncio + aiosqlite, mashumaro, alembic - 004-ui-prompt-config: Added Python 3.13+ + aiohttp (server), SQLAlchemy asyncio + aiosqlite, mashumaro, alembic; Vanilla JS (Vue 3, no build step) for frontend - 003-github-releases: Added N/A (no Python source changes — CI/CD configuration only) + GitHub Actions (`docker/metadata-action`, `docker/build-push-action`, `docker/login-action`) -- 002-switch-dependabot: Added N/A (no Python source changes) + GitHub Dependabot (native GitHub feature, no external service) diff --git a/pyproject.toml b/pyproject.toml index eaf31a68..d99041be 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ requires = ["setuptools>=77.0"] [project] name = "supernote" -version = "1.1.0" +version = "1.2.0" license = "Apache-2.0" license-files = ["LICENSE"] description = "All-in-one toolkit for Supernote devices: parse notebooks, self-host services, access services" diff --git a/script/pre-commit-test b/script/pre-commit-test new file mode 100755 index 00000000..65c71457 --- /dev/null +++ b/script/pre-commit-test @@ -0,0 +1,50 @@ +#!/usr/bin/env bash +# script/pre-commit-test: Run tests for staged Python files before committing. +# Invoked automatically by pre-commit before each commit. + +set -e +cd "$(dirname "$0")/.." + +# Collect staged Python files +staged_src=$(git diff --cached --name-only --diff-filter=ACM | grep '^supernote/.*\.py$' || true) +staged_tests=$(git diff --cached --name-only --diff-filter=ACM | grep '^tests/.*\.py$' || true) + +# Nothing Python-related staged — skip +if [ -z "$staged_src" ] && [ -z "$staged_tests" ]; then + exit 0 +fi + +# Map source files -> corresponding test files +test_files=() + +for f in $staged_src; do + rel="${f#supernote/}" + dir=$(dirname "$rel") + base=$(basename "$rel" .py) + candidate="tests/${dir}/test_${base}.py" + if [ -f "$candidate" ]; then + test_files+=("$candidate") + fi +done + +# Include any staged test files directly +for f in $staged_tests; do + test_files+=("$f") +done + +# Deduplicate test files +IFS=$'\n' read -r -d '' -a test_files < <(printf '%s\n' "${test_files[@]}" | sort -u && printf '\0') || true + +if [ ${#test_files[@]} -eq 0 ]; then + exit 0 +fi + +echo "==> Running pre-commit tests for staged files..." + +if command -v uv >/dev/null 2>&1; then + runner="uv run pytest" +else + runner="pytest" +fi + +$runner "${test_files[@]}" -q diff --git a/specs/005-cache-png-insights-tabs/checklists/requirements.md b/specs/005-cache-png-insights-tabs/checklists/requirements.md new file mode 100644 index 00000000..52815949 --- /dev/null +++ b/specs/005-cache-png-insights-tabs/checklists/requirements.md @@ -0,0 +1,34 @@ +# Specification Quality Checklist: Note Page PNG Caching & Insights Panel Tabs + +**Purpose**: Validate specification completeness and quality before proceeding to planning +**Created**: 2026-03-17 +**Feature**: [spec.md](../spec.md) + +## Content Quality + +- [x] No implementation details (languages, frameworks, APIs) +- [x] Focused on user value and business needs +- [x] Written for non-technical stakeholders +- [x] All mandatory sections completed + +## Requirement Completeness + +- [x] No [NEEDS CLARIFICATION] markers remain +- [x] Requirements are testable and unambiguous +- [x] Success criteria are measurable +- [x] Success criteria are technology-agnostic (no implementation details) +- [x] All acceptance scenarios are defined +- [x] Edge cases are identified +- [x] Scope is clearly bounded +- [x] Dependencies and assumptions identified + +## Feature Readiness + +- [x] All functional requirements have clear acceptance criteria +- [x] User scenarios cover primary flows +- [x] Feature meets measurable outcomes defined in Success Criteria +- [x] No implementation details leak into specification + +## Notes + +- All items pass. Spec is ready for `/speckit.clarify` or `/speckit.plan`. diff --git a/specs/005-cache-png-insights-tabs/contracts/ocr-list-endpoint.md b/specs/005-cache-png-insights-tabs/contracts/ocr-list-endpoint.md new file mode 100644 index 00000000..65d02a58 --- /dev/null +++ b/specs/005-cache-png-insights-tabs/contracts/ocr-list-endpoint.md @@ -0,0 +1,60 @@ +# Contract: OCR Page List Endpoint + +**Endpoint**: `POST /api/extended/file/ocr/list` +**Auth**: Required — `x-access-token: ` header +**Ownership**: Users may only retrieve OCR for files they own; cross-user access returns 403. + +--- + +## Request + +**Content-Type**: `application/json` + +```json +{ + "fileId": 12345 +} +``` + +| Field | Type | Required | Description | +|-------|------|----------|-------------| +| `fileId` | integer | Yes | ID of the note file | + +--- + +## Response — 200 OK + +```json +{ + "pages": [ + { "pageIndex": 0, "textContent": "Handwriting extracted from page 1..." }, + { "pageIndex": 1, "textContent": "Handwriting extracted from page 2..." } + ] +} +``` + +| Field | Type | Description | +|-------|------|-------------| +| `pages` | array | Ordered by `pageIndex` ascending. Empty array if no OCR available. Only pages with non-null text are included. | +| `pages[].pageIndex` | integer | 0-based page position in the note | +| `pages[].textContent` | string | Raw OCR text from that page | + +--- + +## Error Responses + +| Status | Condition | +|--------|-----------| +| 400 | Malformed JSON body or missing `fileId` | +| 401 | Missing or invalid JWT | +| 403 | File belongs to a different user | +| 404 | File not found | +| 500 | Unexpected server error | + +--- + +## Notes + +- Returns an empty `pages` array (not a 404) when the file exists but has no OCR data yet (processing pending). +- Does not paginate — all available pages returned in a single response. +- Follows the same convention as `POST /api/extended/file/summary/list`. diff --git a/specs/005-cache-png-insights-tabs/data-model.md b/specs/005-cache-png-insights-tabs/data-model.md new file mode 100644 index 00000000..581c9b22 --- /dev/null +++ b/specs/005-cache-png-insights-tabs/data-model.md @@ -0,0 +1,65 @@ +# Data Model: Note Page PNG Caching & Insights Panel Tabs + +**Feature**: 005-cache-png-insights-tabs +**Date**: 2026-03-17 + +--- + +## Database Changes + +### `f_user_file` table — new column + +| Column | Type | Nullable | Default | Purpose | +|--------|------|----------|---------|---------| +| `last_conversion_md5` | `String` | Yes | `NULL` | Stores the file MD5 used during the most recent `convert_note_to_png` call. Used to detect content changes and reconstruct old storage keys for cleanup. | + +**Migration**: New Alembic revision under `supernote/alembic/versions/`. Follows existing pattern (`add_column` with `nullable=True`, no `server_default` needed — rows for files that have never been converted will have `NULL`, which is treated as "no previous conversion"). + +**SQLAlchemy model addition** (`supernote/server/db/models/file.py` — `UserFileDO`): +```python +last_conversion_md5: Mapped[str | None] = mapped_column(String, nullable=True) +"""MD5 of the file at last PNG conversion; used for stale image cleanup.""" +``` + +--- + +## New DTOs / VOs (`supernote/models/extended.py`) + +### `OcrPageVO` + +| Field | Python type | JSON alias | Notes | +|-------|-------------|------------|-------| +| `page_index` | `int` | `pageIndex` | 0-based page number, ordered ascending | +| `text_content` | `str` | `textContent` | Raw OCR text extracted from the page | + +### `WebOcrListRequestDTO` + +| Field | Python type | JSON alias | Notes | +|-------|-------------|------------|-------| +| `file_id` | `int` | `fileId` | ID of the note file to retrieve OCR for | + +### `WebOcrListVO` + +| Field | Python type | JSON alias | Notes | +|-------|-------------|------------|-------| +| `pages` | `list[OcrPageVO]` | `pages` | All pages with OCR text, ordered by `page_index`. Empty list if none available. | + +All three use `@dataclass` + `DataClassJSONMixin` with `omit_none=True` and `BaseConfig(serialize_by_alias=True)` consistent with existing models in the file. + +--- + +## Storage Key Patterns + +No new storage key formats. Existing patterns used: + +| Pattern | Bucket | Usage | +|---------|--------|-------| +| `conversions/{user_id}/{file_id}/page_{page_index}_{md5}.png` | `supernote-user-data` | Cached page image. Presence = content unchanged. | + +**Cleanup logic**: When `last_conversion_md5 != node.md5` (content changed), old images are deleted by reconstructing: `get_conversion_png_path(user_id, file_id, i, last_conversion_md5)` for `i` in `0..note.get_total_pages()-1`. + +--- + +## No New Tables + +No new DB tables are required. OCR text is read from the existing `f_note_page_content.text_content` column (populated by the existing OCR processing pipeline). diff --git a/specs/005-cache-png-insights-tabs/plan.md b/specs/005-cache-png-insights-tabs/plan.md new file mode 100644 index 00000000..4d1fc42f --- /dev/null +++ b/specs/005-cache-png-insights-tabs/plan.md @@ -0,0 +1,150 @@ +# Implementation Plan: Note Page PNG Caching & Insights Panel Tabs + +**Branch**: `005-cache-png-insights-tabs` | **Date**: 2026-03-17 | **Spec**: [spec.md](spec.md) +**Input**: Feature specification from `/specs/005-cache-png-insights-tabs/spec.md` + +## Summary + +Cache converted note page PNG images on first view so subsequent opens of unchanged notes are near-instant, and extend the AI Insights panel with a second tab exposing raw OCR text per page. Caching is implemented by checking storage before each conversion; cleanup of stale images uses a new `last_conversion_md5` column on `UserFileDO`. The OCR tab is backed by a new dedicated backend endpoint. + +## Technical Context + +**Language/Version**: Python 3.13+ (backend), Vanilla JS / Vue 3 ESM (frontend) +**Primary Dependencies**: aiohttp, SQLAlchemy asyncio + aiosqlite, mashumaro, alembic +**Storage**: SQLite (DB via SQLAlchemy), LocalBlobStorage (disk — `supernote-user-data` bucket) +**Testing**: pytest + pytest-asyncio (auto mode); `unittest.mock.patch` for mocking +**Target Platform**: Linux server (self-hosted) +**Project Type**: Web service + SPA frontend (no build step) +**Performance Goals**: Repeat note views 80%+ faster; zero redundant conversions for unchanged notes +**Constraints**: All I/O async; strict mypy; every endpoint JWT-authenticated; no content logging +**Scale/Scope**: Single-user instances to small teams; typical notes are 1–200 pages + +## Constitution Check + +*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.* + +| Principle | Status | Notes | +|-----------|--------|-------| +| I. Library-First | ✅ Pass | Changes in `server/services/` and `server/routes/`; no new cross-layer circular deps | +| II. Protocol Fidelity | ✅ Pass | New endpoint is extension-only (`/api/extended/`); device protocol untouched | +| III. Async-First | ✅ Pass | `blob_storage.exists()` is async; all I/O remains awaited; no blocking calls added | +| IV. Strict Type Safety | ✅ Pass | New DTOs use `@dataclass + DataClassJSONMixin`; `last_conversion_md5: Mapped[str | None]` typed correctly | +| V. Observability & Privacy | ✅ Pass | OCR text MUST NOT be logged; `last_conversion_md5` is a hash, not content | +| VI. TDD (NON-NEGOTIABLE) | ✅ Pass | Tests must be written first; red-green-refactor required | +| VII. Security (NON-NEGOTIABLE) | ✅ Pass | New OCR endpoint requires JWT + user ownership check; same pattern as summary endpoint | +| VIII. Frontend UI Conventions | ✅ Pass | Tab buttons use black/gray palette; no accent colors; dark: variants required | + +**Post-design re-check**: All gates remain clear. The `last_conversion_md5` column addition follows the Alembic migration pattern used in `b2c3d4e5f6a7_add_prompt_config.py`. No new accepted security risks. + +## Project Structure + +### Documentation (this feature) + +```text +specs/005-cache-png-insights-tabs/ +├── plan.md # This file +├── research.md # Phase 0 output +├── data-model.md # Phase 1 output +├── contracts/ +│ └── ocr-list-endpoint.md # Phase 1 output +└── tasks.md # Phase 2 output (/speckit.tasks) +``` + +### Source Code (repository root) + +```text +supernote/ +├── models/ +│ └── extended.py # Add OcrPageVO, WebOcrListRequestDTO, WebOcrListVO +├── server/ +│ ├── routes/ +│ │ └── extended.py # Add POST /api/extended/file/ocr/list handler +│ ├── services/ +│ │ └── file.py # Modify convert_note_to_png (exists check + cleanup) +│ └── db/ +│ └── models/ +│ └── file.py # Add last_conversion_md5 to UserFileDO +├── alembic/ +│ └── versions/ +│ └── _add_last_conversion_md5.py # New Alembic migration +└── server/ + └── static/ + └── js/ + ├── components/ + │ └── SummaryPanel.js # Add tabs, OCR fetch, reset on file change + └── api/ + └── client.js # Add fetchOcrPages(fileId) + +tests/ +├── server/ +│ ├── routes/ +│ │ └── test_extended.py # Add OCR endpoint tests (auth, ownership, empty, data) +│ └── services/ +│ └── test_file_service.py # Add caching + cleanup behaviour tests +└── models/ + └── test_extended_completeness.py # Add OcrPageVO round-trip test +``` + +**Structure Decision**: Single project layout (existing). All changes are additive within the established `supernote/` package hierarchy. + +## Implementation Approach + +### Part 1 — PNG Caching (`FileService.convert_note_to_png`) + +**File**: `supernote/server/services/file.py` + +Replace the unconditional convert-and-store loop with: + +1. Load notebook and build `converter` as today. +2. Check whether page 0 key exists with current MD5. This determines if content has changed. +3. **If content changed** (page 0 doesn't exist OR `node.last_conversion_md5 != node.md5` and both are set): delete old images by iterating `i in 0..total_pages-1` and calling `blob_storage.delete(USER_DATA_BUCKET, get_conversion_png_path(user_id, file_id, i, old_md5))`. +4. **Per-page loop**: call `blob_storage.exists()` (fail-open: wrap in `try/except`, log warning, treat error as cache miss). Skip conversion if `True`; convert and store if `False`. +5. After all pages written: if `node.last_conversion_md5 != node.md5`, update `node.last_conversion_md5 = node.md5` and commit the session. +6. Return `ConversionsVO` list as today (signed URLs via existing OSS route). + +### Part 2 — OCR Endpoint + +**Files**: `supernote/models/extended.py`, `supernote/server/routes/extended.py` + +Add `OcrPageVO`, `WebOcrListRequestDTO`, `WebOcrListVO` to `supernote/models/extended.py` following the mashumaro + `BaseConfig(serialize_by_alias=True)` + `omit_none=True` pattern. + +Add handler in `extended.py`: +``` +POST /api/extended/file/ocr/list + → parse WebOcrListRequestDTO + → enforce user ownership (same pattern as summary endpoint) + → query NotePageContentDO WHERE file_id=? AND text_content IS NOT NULL ORDER BY page_index ASC + → return WebOcrListVO(pages=[OcrPageVO(page_index=r.page_index, text_content=r.text_content) ...]) +``` + +Register route in the application router (same location as existing extended routes). + +### Part 3 — DB Migration + +**File**: `supernote/alembic/versions/_add_last_conversion_md5.py` + +```python +def upgrade() -> None: + op.add_column("f_user_file", sa.Column("last_conversion_md5", sa.String(), nullable=True)) + +def downgrade() -> None: + op.drop_column("f_user_file", "last_conversion_md5") +``` + +### Part 4 — Frontend: SummaryPanel tabs + +**Files**: `supernote/server/static/js/components/SummaryPanel.js`, `supernote/server/static/js/api/client.js` + +**`client.js`**: Add `fetchOcrPages(fileId)` — `POST /api/extended/file/ocr/list` with `{ fileId }`, returns `data.pages || []`. + +**`SummaryPanel.js`** changes: +- Add `activeTab = ref('ai')`, `ocrPages = ref([])`, `isOcrLoading = ref(false)`, `ocrError = ref(null)`, `ocrLoaded = ref(false)`. +- `watch(() => props.fileId, () => { activeTab.value = 'ai'; ocrPages.value = []; ocrLoaded.value = false; })`. +- Add `loadOcr()` async function: called when user clicks OCR tab (lazy), sets `isOcrLoading`, fetches, stores result. +- Tab bar replaces the title row (or sits below it). Two buttons using secondary Tailwind classes. Active tab: add `border-b-2 border-black dark:border-white font-semibold` to highlight (no accent colors). +- Template: `v-if="activeTab === 'ai'"` wraps existing content; `v-if="activeTab === 'ocr'"` renders OCR pages list with loading/empty states. +- OCR list: one card per page showing page number and `textContent` in pre-wrap or prose. + +## Complexity Tracking + +No constitution violations. No complexity justification required. diff --git a/specs/005-cache-png-insights-tabs/research.md b/specs/005-cache-png-insights-tabs/research.md new file mode 100644 index 00000000..5ab8a79d --- /dev/null +++ b/specs/005-cache-png-insights-tabs/research.md @@ -0,0 +1,74 @@ +# Research: Note Page PNG Caching & Insights Panel Tabs + +**Feature**: 005-cache-png-insights-tabs +**Date**: 2026-03-17 + +--- + +## Decision 1: PNG caching — existence check location + +**Decision**: Add `await self.blob_storage.exists(USER_DATA_BUCKET, png_storage_key)` inside `FileService.convert_note_to_png` in `supernote/server/services/file.py`, per-page, before calling `ImageConverter.convert()`. Skip conversion and `put()` if the key already exists. + +**Rationale**: All the necessary information (bucket, key) is available at that point in the loop. The key already encodes user_id, file_id, page_index, and file MD5, so a successful `exists()` means the content is unchanged and the cached image is valid. + +**Alternatives considered**: +- Check at route level before calling service — rejected: service should own caching logic. +- Cache in memory (LRU dict) — rejected: server restarts would invalidate it; disk cache is durable. + +--- + +## Decision 2: Fail-open on storage existence check errors + +**Decision**: Wrap the `exists()` call in a `try/except`. On any `OSError` or storage exception, log a warning and proceed with conversion (treat as cache miss). This is per clarification Q3. + +**Rationale**: A transient storage error should not block the user from viewing their note. Re-converting and re-storing the image is a safe recovery path. + +**Alternatives considered**: +- Fail-closed (surface error to user) — rejected: poor UX for a cache lookup failure. + +--- + +## Decision 3: Old-image cleanup via `last_conversion_md5` column + +**Decision**: Add `last_conversion_md5: Mapped[str | None]` (nullable String) to `UserFileDO` in `supernote/server/db/models/file.py`, backed by an Alembic migration. When `convert_note_to_png` detects that the current MD5 differs from `last_conversion_md5` (or that page 0 doesn't exist at the current MD5 key), it: +1. Deletes old images by reconstructing keys with the old MD5 (`get_conversion_png_path(user_id, file_id, i, old_md5)` for i in 0..n-1). +2. Converts and stores new images. +3. Updates `node.last_conversion_md5 = node.md5` and commits. + +**Rationale**: No `list_by_prefix` method exists on `BlobStorage` (nor is one practical with the current `_get_path` path sanitization, which strips directory components). Storing the previous MD5 in the DB is the only approach that allows exact key reconstruction for deletion without changing the storage interface. Alembic migration is the established pattern for schema changes in this project. + +**Known limitation**: `LocalBlobStorage._get_path` uses `Path(key).name` to sanitize keys, which strips all directory components (e.g., `conversions/1/101/page_0_abc.png` → stored as `pa/page_0_abc.png`). This is a pre-existing behavior that could cause key collisions between different users' files. This feature does not fix this; the cleanup logic works correctly in the common case and degrades gracefully in the collision scenario. + +**Alternatives considered**: +- Add `list_by_prefix` to `BlobStorage` — requires fixing `_get_path` path sanitization first, which is a backward-incompatible storage layout change; deferred to a future refactor. +- Delete all per-file images on note upload event — requires coupling sync pipeline to view layer; rejected. + +--- + +## Decision 4: New dedicated OCR endpoint + +**Decision**: Add `POST /api/extended/file/ocr/list` in `supernote/server/routes/extended.py`. Request DTO: `WebOcrListRequestDTO { file_id: int (alias: fileId) }`. Response VO: `WebOcrListVO { pages: list[OcrPageVO] }` where `OcrPageVO { page_index: int (alias: pageIndex), text_content: str (alias: textContent) }`. DTOs/VOs defined in `supernote/models/extended.py`. The handler queries `NotePageContentDO` by `file_id`, ordered by `page_index`, and returns only rows where `text_content IS NOT NULL`. + +**Rationale**: Dedicated endpoint is cleaner than filtering the summary list by type. Single response (no pagination) is sufficient given typical note page counts. Follows existing extended endpoint conventions (POST with JSON body, mashumaro DTOs, `x-access-token` JWT auth, user ownership enforcement). Per clarification Q2. + +**Alternatives considered**: +- Reuse summary endpoint with `dataSource=ocr` filter — rejected: couples unrelated data models; OCR source is not a `SummaryItem`. +- GET with path param — rejected: existing extended endpoints use POST+JSON body convention. + +--- + +## Decision 5: SummaryPanel tab implementation + +**Decision**: Add `activeTab` ref (default `'ai'`) to `SummaryPanel.js` setup. Add a two-button tab bar in the header area below the existing "AI Insights" title. Each button uses secondary-style Tailwind classes with an active state indicator (border-b or background highlight in black/gray, no accent colors). OCR data is fetched lazily when the user first clicks the "OCR" tab, using a new `fetchOcrPages(fileId)` API client function. Reset `activeTab` to `'ai'` on `watch(() => props.fileId, ...)`. + +**Rationale**: Lazy fetch avoids loading OCR data for users who never open that tab. Reset on file change matches existing tab patterns and the spec (FR-010). Button styles must use the established button palette (no indigo/blue/green). + +**Alternatives considered**: +- Eager fetch both tabs on mount — rejected: wastes bandwidth for users who only need AI tab. +- Separate component per tab — rejected: over-engineering for two tabs sharing one panel. + +--- + +## Pre-existing finding (no action this feature) + +`LocalBlobStorage._get_path` strips directory components from keys via `Path(key).name`. All blobs whose keys share the same filename component will map to the same physical file. Example: `conversions/1/101/page_0_abc.png` and `conversions/2/202/page_0_abc.png` both map to `supernote-user-data/pa/page_0_abc.png`. This is a pre-existing collision risk; mitigation is tracked separately and not in scope here. diff --git a/specs/005-cache-png-insights-tabs/spec.md b/specs/005-cache-png-insights-tabs/spec.md new file mode 100644 index 00000000..57ea2583 --- /dev/null +++ b/specs/005-cache-png-insights-tabs/spec.md @@ -0,0 +1,98 @@ +# Feature Specification: Note Page PNG Caching & Insights Panel Tabs + +**Feature Branch**: `005-cache-png-insights-tabs` +**Created**: 2026-03-17 +**Status**: Draft +**Input**: User description: "check blob_storage.exists(USER_DATA_BUCKET, png_storage_key) before converting each page, and skip conversion if the blob is already there, update insights panel to have 2 tabs, one (main one) for the AI conversion and the second one for OCR" + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - Fast Repeat Note Loading (Priority: P1) + +A user opens a large note they have viewed before. Instead of waiting for all pages to be re-converted from scratch, the pages load immediately because the previously generated images are reused. + +**Why this priority**: This is the primary pain point — large notes trigger a full re-conversion on every view even when nothing has changed, causing unnecessary delay and wasted processing. Fixing this delivers immediate, visible performance improvement. + +**Independent Test**: Open a large note, wait for it to load, close it, then reopen it. On the second open, the "Converting note..." loading phase should complete significantly faster (or be near-instant if images are already stored). + +**Acceptance Scenarios**: + +1. **Given** a note has been opened and its page images have been generated and stored, **When** the user opens the same note again without modifying it, **Then** the viewer loads the existing images without re-converting any pages. +2. **Given** a note has been opened previously, **When** the note file is updated (its content hash changes), **Then** the system detects the change and re-converts the pages since the key encodes the hash. +3. **Given** a note's page images are partially stored (some pages cached, some missing), **When** the user opens the note, **Then** only the missing pages are converted and the stored ones are reused. + +--- + +### User Story 2 - View AI Insights and OCR Text for a Note (Priority: P2) + +A user opens a note and clicks the AI Insights panel. They see two tabs: "AI" (default, active) showing the AI-generated summary/analysis, and "OCR" showing the raw text extracted from each page. + +**Why this priority**: The current panel only surfaces AI summaries. Exposing OCR text directly gives users a way to verify what was extracted from their handwriting and use the raw text independently — without requiring a separate panel. + +**Independent Test**: Open any note that has been processed, open the Insights panel, and verify two tabs are shown. Clicking "AI" shows existing summary content; clicking "OCR" shows per-page extracted text. + +**Acceptance Scenarios**: + +1. **Given** a note has AI summaries available, **When** the user opens the Insights panel, **Then** the "AI" tab is active by default and displays summary content as before. +2. **Given** a note has OCR text available for its pages, **When** the user clicks the "OCR" tab, **Then** the extracted text for each page is displayed, ordered by page number. +3. **Given** a note has no OCR text yet (still processing), **When** the user opens the "OCR" tab, **Then** an appropriate empty/processing state is shown rather than an error. +4. **Given** a note has no AI summaries available, **When** the user opens the "AI" tab, **Then** the existing empty state message is shown. + +--- + +### Edge Cases + +- What happens when the stored page image file is missing or unreadable at retrieval time? The system should fall back to re-converting that page. +- What happens if the existence check itself fails due to a storage error? The system MUST treat this as a cache miss and proceed with conversion (fail-open), keeping the user unblocked. +- What if OCR processing has not completed for some pages? Show text for available pages and indicate others are pending. +- What happens if the user switches between tabs rapidly? No duplicate requests or UI flicker should occur. +- What happens when a note has zero pages? Both tabs should show an appropriate empty state. + +## Requirements *(mandatory)* + +### Functional Requirements + +- **FR-001**: The system MUST check whether a stored page image already exists before converting a note page to an image. +- **FR-002**: The system MUST reuse the existing stored image when one is found, skipping the conversion step for that page. +- **FR-003**: The system MUST convert and store a new image only when no stored image exists for a given page at the expected storage location. +- **FR-004**: When a note's content changes (detected via file hash embedded in the storage key), the system MUST generate new images since the storage key will differ. +- **FR-004a**: When new page images are stored for an updated note, the system MUST delete any previously stored page images for that note that belong to the old content version (old hash). +- **FR-005**: The AI Insights panel MUST display two tabs: "AI" (primary/default) and "OCR". +- **FR-006**: The "AI" tab MUST display the same AI-generated summary content currently shown in the panel, with no regression in existing behaviour. +- **FR-007**: The "OCR" tab MUST display the raw text extracted from each page of the note, ordered by page number. +- **FR-008**: The "OCR" tab MUST show an appropriate empty or pending state when no OCR text is available for the note. +- **FR-009**: The "AI" tab MUST be the active/selected tab when the panel is first opened. +- **FR-010**: Tab selection MUST reset to "AI" when the panel is closed and reopened or when a different file is selected. + +### Key Entities + +- **Note Page Image**: A rendered image of a single note page, keyed by file ID, page index, and file content hash. Its presence in storage determines whether re-conversion is needed. +- **OCR Page Text**: The raw text extracted from a single note page, associated with a page index and the note file, displayed in the OCR tab ordered by page number. + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-001**: Reopening a previously viewed, unchanged note displays all pages at least 80% faster compared to the first open. +- **SC-002**: Zero page conversions occur when a user reopens a note whose content has not changed since the last view. +- **SC-003**: The Insights panel shows two clearly labelled tabs ("AI" and "OCR") on every note that has been opened. +- **SC-004**: Users can read the OCR text for any processed note page directly from the Insights panel without navigating away from the note viewer. +- **SC-005**: The "AI" tab content is identical to the existing panel behaviour — no regression in displayed summaries. + +## Clarifications + +### Session 2026-03-17 + +- Q: When a note's content changes and new images are stored, should old-hash cached images be deleted immediately or left to accumulate until note deletion? → A: Delete old-hash page images for a note when new images are stored for a new content version. +- Q: Should the new OCR endpoint return all pages in a single response or reuse the existing summary endpoint filtered by type? → A: New dedicated endpoint returning all pages for a file in one response, ordered by page index. +- Q: If the storage existence check fails due to a storage error, should the system proceed with conversion or surface an error to the user? → A: Proceed with conversion (fail-open) — treat storage error as a cache miss and convert normally. +- Q: When a note's content changes, does the cache allow per-page reuse (only re-convert changed pages)? → A: No. When the file MD5 changes, ALL pages are re-converted. The storage key embeds the MD5, so the entire key namespace changes and no previously cached pages can be reused. There is no page-level diffing. +- Q: Where should the AI/OCR tab bar be placed in the Insights panel header? → A: Inline with the header row — the tabs sit between the "AI Insights" title and the close button on the same line, conserving vertical space. + +## Assumptions + +- The file content hash (MD5) already stored on the note file record is sufficient to detect whether a note has changed; no additional hashing is needed. +- The existing `conversions/{user_id}/{file_id}/page_{i}_{md5}.png` storage key format encodes the file hash, so a key match implies the content is unchanged and a hash change naturally produces a new key. +- A dedicated backend endpoint to retrieve per-page OCR text by file ID does not yet exist and will need to be added as part of this feature. It will return all pages for a file in a single response, ordered by page index. +- The OCR tab displays text in read-only form; editing OCR text is out of scope. +- Dark mode support is required for all new UI elements, consistent with existing application conventions. diff --git a/specs/005-cache-png-insights-tabs/tasks.md b/specs/005-cache-png-insights-tabs/tasks.md new file mode 100644 index 00000000..307e2017 --- /dev/null +++ b/specs/005-cache-png-insights-tabs/tasks.md @@ -0,0 +1,179 @@ +# Tasks: Note Page PNG Caching & Insights Panel Tabs + +**Input**: Design documents from `/specs/005-cache-png-insights-tabs/` +**Prerequisites**: plan.md ✓, spec.md ✓, research.md ✓, data-model.md ✓, contracts/ ✓ + +**Tests**: Included — TDD is NON-NEGOTIABLE per Constitution §VI. All tests must be written and confirmed failing before implementation. + +**Organization**: Tasks grouped by user story for independent implementation and testing. + +## Format: `[ID] [P?] [Story] Description` + +- **[P]**: Can run in parallel (different files, no incomplete-task dependencies) +- **[Story]**: Which user story this task belongs to (US1, US2) + +## Path Conventions + +Single project layout — `supernote/` package at repo root, `tests/` mirrors package structure. + +--- + +## Phase 2: Foundational (Blocking Prerequisites for US1) + +**Purpose**: DB model and migration changes required before US1 tests can be run against a real schema. + +**⚠️ CRITICAL**: US1 tests depend on `last_conversion_md5` existing on `UserFileDO`. Complete this phase before Phase 3. + +- [x] T001 Add `last_conversion_md5: Mapped[str | None]` column to `UserFileDO` in `supernote/server/db/models/file.py` +- [x] T002 [P] Create Alembic migration `add_last_conversion_md5_to_user_file` in `supernote/alembic/versions/` (nullable String column on `f_user_file`, no server default) + +**Checkpoint**: Foundation ready — US1 and US2 can now proceed in parallel + +--- + +## Phase 3: User Story 1 — Fast Repeat Note Loading (Priority: P1) 🎯 MVP + +**Goal**: Cached page images are reused on repeat views of unchanged notes; stale images from previous content versions are deleted. + +**Independent Test**: Open a note in the dev server, observe "Converting note..." loading. Close and reopen — the loading phase should be negligible (near-instant). Verify in service tests that `blob_storage.exists()` is called per page and `ImageConverter.convert()` is skipped when `exists` returns `True`. + +### Tests for User Story 1 ⚠️ Write FIRST — confirm they FAIL before implementing + +- [x] T003 [US1] Write failing service tests for `convert_note_to_png` covering: (a) skips conversion and `put` when `exists` returns `True`, (b) converts and stores when `exists` returns `False`, (c) treats storage error on `exists` as cache miss (fail-open), (d) deletes old-hash images for all page indices when `last_conversion_md5 != node.md5`, (e) updates `node.last_conversion_md5` to current MD5 after storing new images — in `tests/server/services/test_file_service.py` + +### Implementation for User Story 1 + +- [x] T004 [US1] Modify `FileService.convert_note_to_png` in `supernote/server/services/file.py` — wrap each page's conversion in a per-page `await self.blob_storage.exists()` check; skip `ImageConverter.convert()` and `blob_storage.put()` if `True`; wrap the `exists` call in `try/except` and log a warning on error, treating it as a cache miss (fail-open per research.md Decision 2) +- [x] T005 [US1] Add old-image cleanup and column update to `convert_note_to_png` in `supernote/server/services/file.py` — when `node.last_conversion_md5` is set and differs from `node.md5`, delete old images by calling `blob_storage.delete(USER_DATA_BUCKET, get_conversion_png_path(user_id, file_id, i, old_md5))` for `i` in `0..note.get_total_pages()-1`; after all pages are stored, set `node.last_conversion_md5 = node.md5` and commit the session + +**Checkpoint**: US1 complete — repeat note views skip conversion; stale images cleaned up; all US1 tests pass + +--- + +## Phase 4: User Story 2 — AI Insights Panel with OCR Tab (Priority: P2) + +**Goal**: Insights panel shows "AI" (default) and "OCR" tabs; OCR tab displays per-page extracted text fetched from a new backend endpoint. + +**Independent Test**: Open any processed note, click the lightning bolt (AI Insights panel), confirm two tabs render. "AI" tab shows existing summaries unchanged. "OCR" tab shows per-page text or an empty state if not yet processed. Verify in route tests that `POST /api/extended/file/ocr/list` returns pages ordered by `page_index` and enforces auth/ownership. + +> **Note**: US2 can be worked in parallel with US1 after Phase 2 completes — all files are disjoint. + +### Tests for User Story 2 ⚠️ Write FIRST — confirm they FAIL before implementing + +- [x] T006 [P] [US2] Write failing endpoint tests for `POST /api/extended/file/ocr/list` covering: success with ordered pages, empty list when no OCR data, 401 without token, 403 for other user's file, 404 for unknown file, 400 for malformed body — in `tests/server/routes/test_ocr_endpoint.py` +- [x] T007 [P] [US2] Write failing model completeness/serialization tests (round-trip `from_dict`/`to_dict`) for `OcrPageVO`, `WebOcrListRequestDTO`, `WebOcrListVO` — in `tests/models/test_extended_completeness.py` + +### Implementation for User Story 2 + +- [x] T008 [P] [US2] Add `OcrPageVO`, `WebOcrListRequestDTO`, `WebOcrListVO` dataclasses to `supernote/models/extended.py` using `@dataclass + DataClassJSONMixin`, `omit_none=True`, `BaseConfig(serialize_by_alias=True)`, camelCase JSON aliases matching `contracts/ocr-list-endpoint.md` +- [x] T009 [US2] Implement `POST /api/extended/file/ocr/list` handler in `supernote/server/routes/extended.py` — parse `WebOcrListRequestDTO`, enforce user ownership (same pattern as summary endpoint), query `NotePageContentDO` where `file_id=?` and `text_content IS NOT NULL` ordered by `page_index`, return `WebOcrListVO`; register route in app router alongside existing extended routes (depends on T008) +- [x] T010 [P] [US2] Add `fetchOcrPages(fileId)` async function to `supernote/server/static/js/api/client.js` — `POST /api/extended/file/ocr/list` with `{ fileId }`, JWT header, returns `data.pages || []`, handles 401 with logout +- [x] T011 [US2] Update `supernote/server/static/js/components/SummaryPanel.js` — add `activeTab` ref (default `'ai'`), `ocrPages`/`isOcrLoading`/`ocrError`/`ocrLoaded` refs; add two-tab bar below header title using secondary button Tailwind classes with active tab indicated by `border-b-2 border-black dark:border-white font-semibold` (no accent colors); implement `loadOcr()` called lazily on first OCR tab click; wrap existing content in `v-if="activeTab === 'ai'"` and add OCR content in `v-if="activeTab === 'ocr'"` with loading spinner and empty state; reset `activeTab` to `'ai'` and clear OCR state in `watch(() => props.fileId, ...)` (depends on T010) + +**Checkpoint**: US2 complete — both tabs render; AI tab content unchanged; OCR tab loads text lazily; all US2 tests pass + +--- + +## Phase 5: Polish & Cross-Cutting Concerns + +- [x] T012 [P] Run `./script/run-mypy.sh` and resolve all type errors in changed Python files (`supernote/server/db/models/file.py`, `supernote/server/services/file.py`, `supernote/server/routes/extended.py`, `supernote/models/extended.py`, `supernote/alembic/versions/.py`) +- [x] T013 [P] Run `./script/test` (full pytest suite) and confirm zero regressions; fix any failures before proceeding +- [ ] T014 Manual end-to-end verification using `./script/server` (ephemeral dev server): open a multi-page note, time the first load; close and reopen — confirm second load is significantly faster; open Insights panel and confirm both "AI" and "OCR" tabs render and display correct content + +--- + +## Dependencies & Execution Order + +### Phase Dependencies + +- **Foundational (Phase 2)**: No dependencies — start immediately +- **US1 (Phase 3)**: Depends on Phase 2 (needs `last_conversion_md5` column and migration) +- **US2 (Phase 4)**: Depends on Phase 2 completion (can run in parallel with US1 after Phase 2) +- **Polish (Phase 5)**: Depends on Phase 3 + Phase 4 complete + +### User Story Dependencies + +- **US1 (P1)**: Requires Phase 2 complete. No dependency on US2. +- **US2 (P2)**: Requires Phase 2 complete. No dependency on US1. All files disjoint from US1. + +### Within Each User Story + +1. Tests written and confirmed FAILING (TDD — Constitution §VI non-negotiable) +2. Implementation tasks in order (models → service/handler → frontend) +3. Story complete before moving to next priority (unless parallel team) + +### Parallel Opportunities + +- T001 + T002: Different files — run in parallel +- T006 + T007: Different files — run in parallel (US2 tests can be written while US1 is underway) +- T008 + T010: Different files — run in parallel (models and JS client are independent) +- T012 + T013: Independent checks — run in parallel + +--- + +## Parallel Example: US1 (Phase 3) + +```bash +# Phase 2 in parallel: +Task T001: "Add last_conversion_md5 to UserFileDO in supernote/server/db/models/file.py" +Task T002: "Create Alembic migration in supernote/alembic/versions/" + +# After Phase 2, US1 is sequential (TDD order): +Task T003: Write failing tests in tests/server/services/test_file_service.py +Task T004: Implement exists check in supernote/server/services/file.py +Task T005: Implement cleanup + column update in supernote/server/services/file.py +``` + +## Parallel Example: US2 (Phase 4) + +```bash +# US2 tests in parallel (different files): +Task T006: Endpoint tests in tests/server/routes/test_extended.py +Task T007: Model completeness tests in tests/models/test_extended_completeness.py + +# After tests are written and failing, models + client in parallel: +Task T008: DTOs/VOs in supernote/models/extended.py +Task T010: fetchOcrPages in supernote/server/static/js/api/client.js + +# Sequential after T008: +Task T009: Route handler in supernote/server/routes/extended.py + +# Sequential after T010: +Task T011: SummaryPanel.js tabs +``` + +--- + +## Implementation Strategy + +### MVP First (US1 Only) + +1. Complete Phase 2: Foundational (T001, T002) +2. Complete Phase 3: US1 (T003 → T004 → T005) +3. **STOP and VALIDATE**: Reopen a large note twice; confirm second open is fast +4. Ship if ready + +### Incremental Delivery + +1. Phase 2 → Foundation ready +2. Phase 3 (US1) → Caching live → validate → demo +3. Phase 4 (US2) → OCR tab live → validate → demo +4. Phase 5 (Polish) → Clean mypy + full test suite + +### Parallel Team Strategy + +With two developers after Phase 2: +- Developer A: US1 (T003 → T004 → T005) +- Developer B: US2 (T006/T007 → T008/T010 → T009 → T011) + +--- + +## Notes + +- [P] tasks = operate on different files with no incomplete-task dependencies +- [Story] label maps each task to its user story for traceability +- Tests MUST fail before implementation — do not skip this confirmation step +- Commit after each completed task or logical group +- Stop at Phase 3 checkpoint to validate US1 independently before starting US2 +- `last_conversion_md5` is `NULL` for files that have never been converted — treat `NULL` as "no previous conversion" (no cleanup needed) +- The OCR tab shows only pages where `text_content IS NOT NULL` — an empty array is valid and expected for notes still being processed diff --git a/supernote/alembic/versions/c3d4e5f6a7b8_add_last_conversion_md5_to_user_file.py b/supernote/alembic/versions/c3d4e5f6a7b8_add_last_conversion_md5_to_user_file.py new file mode 100644 index 00000000..532e7dfd --- /dev/null +++ b/supernote/alembic/versions/c3d4e5f6a7b8_add_last_conversion_md5_to_user_file.py @@ -0,0 +1,31 @@ +"""add last_conversion_md5 to user_file + +Revision ID: c3d4e5f6a7b8 +Revises: b2c3d4e5f6a7 +Create Date: 2026-03-17 00:00:00.000000 + +""" + +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "c3d4e5f6a7b8" +down_revision: Union[str, Sequence[str], None] = "b2c3d4e5f6a7" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + """Upgrade schema.""" + op.add_column( + "f_user_file", + sa.Column("last_conversion_md5", sa.String(), nullable=True), + ) + + +def downgrade() -> None: + """Downgrade schema.""" + op.drop_column("f_user_file", "last_conversion_md5") diff --git a/supernote/models/extended.py b/supernote/models/extended.py index f1840c23..f6f76771 100644 --- a/supernote/models/extended.py +++ b/supernote/models/extended.py @@ -200,3 +200,48 @@ class WebTranscriptResponseVO(BaseResponse): class Config(BaseConfig): serialize_by_alias = True + + +@dataclass +class OcrPageVO(DataClassJSONMixin): + """VO for a single page's OCR text. + + Used by: POST /api/extended/file/ocr/list + """ + + page_index: int = field(metadata=field_options(alias="pageIndex")) + """0-based page position in the note.""" + + text_content: str = field(metadata=field_options(alias="textContent")) + """Raw OCR text extracted from this page.""" + + class Config(BaseConfig): + serialize_by_alias = True + + +@dataclass +class WebOcrListRequestDTO(DataClassJSONMixin): + """Request DTO for listing OCR page text by file ID (Web Extension). + + Used by: POST /api/extended/file/ocr/list + """ + + file_id: int = field(metadata=field_options(alias="fileId")) + """The ID of the file to retrieve OCR text for.""" + + class Config(BaseConfig): + serialize_by_alias = True + + +@dataclass +class WebOcrListVO(DataClassJSONMixin): + """Response VO for OCR page text list (Web Extension). + + Used by: POST /api/extended/file/ocr/list + """ + + pages: list[OcrPageVO] = field(default_factory=list) + """Pages with OCR text, ordered by page_index ascending. Empty if none available.""" + + class Config(BaseConfig): + serialize_by_alias = True diff --git a/supernote/server/db/models/file.py b/supernote/server/db/models/file.py index 80bbf8af..9b432e9d 100644 --- a/supernote/server/db/models/file.py +++ b/supernote/server/db/models/file.py @@ -38,6 +38,9 @@ class UserFileDO(Base): md5: Mapped[str | None] = mapped_column(String, nullable=True) """Content hash.""" + last_conversion_md5: Mapped[str | None] = mapped_column(String, nullable=True) + """MD5 of the file at last PNG conversion; used for stale image cleanup.""" + storage_key: Mapped[str | None] = mapped_column(String, nullable=True) """Physical storage key (inner_name/UUID).""" diff --git a/supernote/server/mcp/server.py b/supernote/server/mcp/server.py index 2e10c3c1..45156b1c 100644 --- a/supernote/server/mcp/server.py +++ b/supernote/server/mcp/server.py @@ -99,6 +99,9 @@ async def _get_auth_user_id(ctx: Context) -> int | None: return await user_service.get_user_id(token.user_id) +MAX_TOP_N = 20 + + async def search_notebook_chunks( ctx: Context, query: str, @@ -112,11 +115,12 @@ async def search_notebook_chunks( Args: query: The semantic search query. - top_n: Number of results to return (default: 5). + top_n: Number of results to return (default: 5, max: 20). name_filter: Optional substring filter for notebook filenames. date_after: Filter for notes created after this date (ISO 8601). date_before: Filter for notes created before this date (ISO 8601). """ + top_n = min(top_n, MAX_TOP_N) search_service: SearchService = _services["search_service"] if not search_service: return create_error_response( diff --git a/supernote/server/routes/extended.py b/supernote/server/routes/extended.py index f68bf468..2c66d25e 100644 --- a/supernote/server/routes/extended.py +++ b/supernote/server/routes/extended.py @@ -13,9 +13,12 @@ from supernote.models.extended import ( FileProcessingStatusDTO, FileProcessingStatusVO, + OcrPageVO, SearchResultVO, SystemTaskListVO, SystemTaskVO, + WebOcrListRequestDTO, + WebOcrListVO, WebSearchRequestDTO, WebSearchResponseVO, WebSummaryListRequestDTO, @@ -23,7 +26,8 @@ WebTranscriptRequestDTO, WebTranscriptResponseVO, ) -from supernote.server.db.models.note_processing import SystemTaskDO +from supernote.server.db.models.file import UserFileDO +from supernote.server.db.models.note_processing import NotePageContentDO, SystemTaskDO from supernote.server.exceptions import SupernoteError from supernote.server.services.search import SearchService from supernote.server.services.summary import SummaryService @@ -231,3 +235,61 @@ async def handle_extended_transcript(request: web.Request) -> web.Response: except Exception as err: logger.exception("Error fetching notebook transcript") return SupernoteError.uncaught(err).to_response() + + +@routes.post("/api/extended/file/ocr/list") +async def handle_extended_file_ocr_list(request: web.Request) -> web.Response: + # Endpoint: POST /api/extended/file/ocr/list + # Purpose: Extended API to retrieve per-page OCR text for a note file. + user_email = request["user"] + try: + data = await request.json() + except Exception: + return web.json_response({"error": "Invalid JSON"}, status=400) + + try: + req_dto = WebOcrListRequestDTO.from_dict(data) + except Exception as e: + return web.json_response({"error": f"Invalid Request: {e}"}, status=400) + + user_service: UserService = request.app["user_service"] + session_manager = request.app["session_manager"] + + try: + user_id = await user_service.get_user_id(user_email) + if not user_id: + return web.json_response({"error": "User not found"}, status=404) + + async with session_manager.session() as session: + # Verify the file exists and enforce ownership. + file_result = await session.execute( + select(UserFileDO).where(UserFileDO.id == req_dto.file_id) + ) + file_node = file_result.scalar_one_or_none() + if file_node is None: + return web.json_response( + {"error": f"File {req_dto.file_id} not found"}, status=404 + ) + if file_node.user_id != user_id: + return web.json_response({"error": "Forbidden"}, status=403) + + # Retrieve pages with OCR text, ordered by page_index. + ocr_result = await session.execute( + select(NotePageContentDO) + .where( + NotePageContentDO.file_id == req_dto.file_id, + NotePageContentDO.text_content.is_not(None), + ) + .order_by(NotePageContentDO.page_index) + ) + pages = ocr_result.scalars().all() + + page_vos = [ + OcrPageVO(page_index=p.page_index, text_content=p.text_content or "") + for p in pages + ] + return web.json_response(WebOcrListVO(pages=page_vos).to_dict()) + + except Exception as err: + logger.exception("Error fetching OCR page list") + return SupernoteError.uncaught(err).to_response() diff --git a/supernote/server/services/file.py b/supernote/server/services/file.py index d34c9b11..058312ca 100644 --- a/supernote/server/services/file.py +++ b/supernote/server/services/file.py @@ -927,28 +927,62 @@ async def convert_note_to_png(self, user: str, file_id: int) -> list[Conversions ) note = load_notebook(BytesIO(blob_content)) # type: ignore[arg-type] - # Convert each page to PNG + current_md5 = node.md5 or "nomd5" + total_pages = note.get_total_pages() + + # Delete stale cached images from the previous content version. + old_md5 = node.last_conversion_md5 + if old_md5 and old_md5 != current_md5: + for i in range(total_pages): + old_key = get_conversion_png_path( + user_id=user_id, + file_id=file_id, + page_index=i, + file_md5=old_md5, + ) + await self.blob_storage.delete(USER_DATA_BUCKET, old_key) + + # Convert each page to PNG, skipping pages that are already cached. converter = ImageConverter(note) results = [] - for i in range(note.get_total_pages()): - img = converter.convert(i) - img_io = BytesIO() - img.save(img_io, format="PNG") - img_bytes = img_io.getvalue() - - # Store PNG in blob storage with a unique key - # Format: conversions/{user_id}/{file_id}/page_{i}_{md5}.png + any_new_pages = False + for i in range(total_pages): png_storage_key = get_conversion_png_path( user_id=user_id, file_id=file_id, page_index=i, - file_md5=node.md5 or "nomd5", - ) - await self.blob_storage.put( - USER_DATA_BUCKET, png_storage_key, img_bytes + file_md5=current_md5, ) + + # Check cache — fail-open on storage errors. + already_cached = False + try: + already_cached = await self.blob_storage.exists( + USER_DATA_BUCKET, png_storage_key + ) + except Exception: + logger.warning( + "blob_storage.exists failed for key %s; treating as cache miss", + png_storage_key, + ) + + if not already_cached: + img = converter.convert(i) + img_io = BytesIO() + img.save(img_io, format="PNG") + img_bytes = img_io.getvalue() + await self.blob_storage.put( + USER_DATA_BUCKET, png_storage_key, img_bytes + ) + any_new_pages = True + results.append(ConversionsVO(storage_key=png_storage_key, page_no=i)) + # Persist the MD5 used for this conversion so future calls can clean up stale images. + if any_new_pages or node.last_conversion_md5 != current_md5: + node.last_conversion_md5 = current_md5 + await session.commit() + return results async def convert_note_to_pdf( diff --git a/supernote/server/static/js/api/client.js b/supernote/server/static/js/api/client.js index 6f8e5eb1..50c24e04 100644 --- a/supernote/server/static/js/api/client.js +++ b/supernote/server/static/js/api/client.js @@ -197,6 +197,36 @@ export async function fetchSummaries(fileId) { return data.summaryDOList || []; } +/** + * Fetch per-page OCR text for a note file (Extended API). + * @param {*} fileId - The file ID (number or string). + * @returns {Promise} Array of { pageIndex, textContent } objects ordered by pageIndex. + */ +export async function fetchOcrPages(fileId) { + const currentToken = getToken(); + if (!currentToken) throw new Error("Unauthorized"); + + const response = await fetch('/api/extended/file/ocr/list', { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'x-access-token': currentToken + }, + body: JSON.stringify({ fileId: fileId }) + }); + + if (!response.ok) { + if (response.status === 401) { + logout(); + throw new Error("Unauthorized"); + } + throw new Error(`OCR fetch failed: ${response.status}`); + } + + const data = await response.json(); + return data.pages || []; +} + /** * Fetch system tasks (Extended API). * @returns {Promise} The system task list response. diff --git a/supernote/server/static/js/components/SummaryPanel.js b/supernote/server/static/js/components/SummaryPanel.js index 8756200c..91c77e73 100644 --- a/supernote/server/static/js/components/SummaryPanel.js +++ b/supernote/server/static/js/components/SummaryPanel.js @@ -1,5 +1,5 @@ import { ref, onMounted, watch } from 'vue'; -import { fetchSummaries } from '../api/client.js'; +import { fetchSummaries, fetchOcrPages } from '../api/client.js'; export default { props: { @@ -8,10 +8,18 @@ export default { } }, setup(props) { + // AI tab state const summaries = ref([]); const isLoading = ref(false); const error = ref(null); + // OCR tab state + const activeTab = ref('ai'); + const ocrPages = ref([]); + const isOcrLoading = ref(false); + const ocrError = ref(null); + const ocrLoaded = ref(false); + const loadSummaries = async () => { if (!props.fileId) return; @@ -31,8 +39,37 @@ export default { } }; + const loadOcr = async () => { + if (!props.fileId || ocrLoaded.value) return; + + isOcrLoading.value = true; + ocrError.value = null; + + try { + ocrPages.value = await fetchOcrPages(props.fileId); + ocrLoaded.value = true; + } catch (e) { + console.error(e); + ocrError.value = "Failed to load OCR text."; + } finally { + isOcrLoading.value = false; + } + }; + + const selectTab = (tab) => { + activeTab.value = tab; + if (tab === 'ocr') loadOcr(); + }; + onMounted(loadSummaries); - watch(() => props.fileId, loadSummaries); + watch(() => props.fileId, () => { + // Reset all state when the viewed file changes + activeTab.value = 'ai'; + ocrPages.value = []; + ocrLoaded.value = false; + ocrError.value = null; + loadSummaries(); + }); // Helper to format text (simple line breaks) const formatContent = (text) => { @@ -49,6 +86,11 @@ export default { summaries, isLoading, error, + activeTab, + ocrPages, + isOcrLoading, + ocrError, + selectTab, formatContent, formatDate }; @@ -56,18 +98,41 @@ export default { template: `
-
-

- - AI Insights -

- +
+
+

+ + AI Insights +

+ +
+ + +
+ +
- -
+ +
@@ -96,6 +161,34 @@ export default {
+ + +
+ +
+
+

Loading OCR text...

+
+ + +
+ {{ ocrError }} +
+ + +
+

No OCR text available.

+

Text will appear here once the note has been processed.

+
+ + +
+
+ Page {{ page.pageIndex + 1 }} +
+
{{ page.textContent }}
+
+
` }; diff --git a/tests/alembic/test_migrations.py b/tests/alembic/test_migrations.py index 0262f624..bcbd619b 100644 --- a/tests/alembic/test_migrations.py +++ b/tests/alembic/test_migrations.py @@ -3,6 +3,7 @@ from unittest.mock import patch import supernote.alembic.versions.a1b2c3d4e5f6_add_used_capacity_to_users as migration +import supernote.alembic.versions.c3d4e5f6a7b8_add_last_conversion_md5_to_user_file as migration_md5 def test_downgrade_drops_used_capacity_column() -> None: @@ -10,3 +11,10 @@ def test_downgrade_drops_used_capacity_column() -> None: with patch.object(migration, "op") as mock_op: migration.downgrade() mock_op.drop_column.assert_called_once_with("users", "used_capacity") + + +def test_downgrade_drops_last_conversion_md5_column() -> None: + """downgrade() calls op.drop_column to remove the last_conversion_md5 column.""" + with patch.object(migration_md5, "op") as mock_op: + migration_md5.downgrade() + mock_op.drop_column.assert_called_once_with("f_user_file", "last_conversion_md5") diff --git a/tests/models/test_extended_completeness.py b/tests/models/test_extended_completeness.py new file mode 100644 index 00000000..4648386d --- /dev/null +++ b/tests/models/test_extended_completeness.py @@ -0,0 +1,66 @@ +"""Round-trip serialisation tests for OCR-related models in supernote.models.extended. + +Per constitution §VI: written before implementation; must FAIL until DTOs/VOs are added. +""" + +from supernote.models.extended import ( + OcrPageVO, + WebOcrListRequestDTO, + WebOcrListVO, +) + + +def test_ocr_page_vo_round_trip() -> None: + vo = OcrPageVO(page_index=0, text_content="Hello world") + d = vo.to_dict() + assert d["pageIndex"] == 0 + assert d["textContent"] == "Hello world" + + +def test_ocr_page_vo_alias_serialisation() -> None: + """Verify camelCase aliases are used in JSON output.""" + vo = OcrPageVO(page_index=3, text_content="Page text") + d = vo.to_dict() + assert "pageIndex" in d + assert "textContent" in d + assert "page_index" not in d + assert "text_content" not in d + + +def test_web_ocr_list_request_dto_round_trip() -> None: + dto = WebOcrListRequestDTO(file_id=42) + d = dto.to_dict() + assert d["fileId"] == 42 + assert "file_id" not in d + + +def test_web_ocr_list_request_dto_from_dict() -> None: + dto = WebOcrListRequestDTO.from_dict({"fileId": 99}) + assert dto.file_id == 99 + + +def test_web_ocr_list_vo_empty_pages() -> None: + vo = WebOcrListVO(pages=[]) + d = vo.to_dict() + assert d["pages"] == [] + + +def test_web_ocr_list_vo_with_pages() -> None: + pages = [ + OcrPageVO(page_index=0, text_content="First page"), + OcrPageVO(page_index=1, text_content="Second page"), + ] + vo = WebOcrListVO(pages=pages) + d = vo.to_dict() + assert len(d["pages"]) == 2 + assert d["pages"][0]["pageIndex"] == 0 + assert d["pages"][0]["textContent"] == "First page" + assert d["pages"][1]["pageIndex"] == 1 + + +def test_web_ocr_list_vo_pages_order_preserved() -> None: + pages = [OcrPageVO(page_index=i, text_content=f"p{i}") for i in range(5)] + vo = WebOcrListVO(pages=pages) + d = vo.to_dict() + indices = [p["pageIndex"] for p in d["pages"]] + assert indices == list(range(5)) diff --git a/tests/server/routes/test_ocr_endpoint.py b/tests/server/routes/test_ocr_endpoint.py new file mode 100644 index 00000000..a289065c --- /dev/null +++ b/tests/server/routes/test_ocr_endpoint.py @@ -0,0 +1,206 @@ +"""Tests for POST /api/extended/file/ocr/list endpoint. + +Per constitution §VI: written before implementation; must FAIL until endpoint is added. +Per constitution §VII: includes auth and ownership security tests. +""" + +from unittest.mock import AsyncMock, patch + +from aiohttp.test_utils import TestClient + +from supernote.server.db.models.file import UserFileDO +from supernote.server.db.models.note_processing import NotePageContentDO +from supernote.server.db.session import DatabaseSessionManager + +USER_ID = 1 +OTHER_USER_ID = 2 +FILE_ID = 200 +OTHER_FILE_ID = 201 +OCR_ENDPOINT = "/api/extended/file/ocr/list" + + +# --------------------------------------------------------------------------- +# Happy path: returns pages ordered by page_index +# --------------------------------------------------------------------------- + + +async def test_ocr_list_returns_pages_ordered( + client: TestClient, + auth_headers: dict[str, str], + session_manager: DatabaseSessionManager, + create_test_user: None, +) -> None: + async with session_manager.session() as session: + session.add( + UserFileDO(id=FILE_ID, user_id=USER_ID, file_name="n.note", directory_id=0) + ) + session.add( + NotePageContentDO( + file_id=FILE_ID, page_index=1, page_id="p1", text_content="Page two" + ) + ) + session.add( + NotePageContentDO( + file_id=FILE_ID, page_index=0, page_id="p0", text_content="Page one" + ) + ) + await session.commit() + + resp = await client.post( + OCR_ENDPOINT, json={"fileId": FILE_ID}, headers=auth_headers + ) + assert resp.status == 200 + data = await resp.json() + assert "pages" in data + assert len(data["pages"]) == 2 + assert data["pages"][0]["pageIndex"] == 0 + assert data["pages"][0]["textContent"] == "Page one" + assert data["pages"][1]["pageIndex"] == 1 + assert data["pages"][1]["textContent"] == "Page two" + + +# --------------------------------------------------------------------------- +# Empty result: file exists but no OCR text +# --------------------------------------------------------------------------- + + +async def test_ocr_list_empty_when_no_ocr_data( + client: TestClient, + auth_headers: dict[str, str], + session_manager: DatabaseSessionManager, + create_test_user: None, +) -> None: + async with session_manager.session() as session: + session.add( + UserFileDO(id=FILE_ID, user_id=USER_ID, file_name="n.note", directory_id=0) + ) + session.add( + NotePageContentDO( + file_id=FILE_ID, page_index=0, page_id="p0", text_content=None + ) + ) + await session.commit() + + resp = await client.post( + OCR_ENDPOINT, json={"fileId": FILE_ID}, headers=auth_headers + ) + assert resp.status == 200 + data = await resp.json() + assert data["pages"] == [] + + +# --------------------------------------------------------------------------- +# Security: 401 without token +# --------------------------------------------------------------------------- + + +async def test_ocr_list_unauthorized_without_token(client: TestClient) -> None: + resp = await client.post(OCR_ENDPOINT, json={"fileId": FILE_ID}) + assert resp.status == 401 + + +# --------------------------------------------------------------------------- +# Security: 403 when file belongs to another user +# --------------------------------------------------------------------------- + + +async def test_ocr_list_forbidden_for_other_users_file( + client: TestClient, + auth_headers: dict[str, str], + session_manager: DatabaseSessionManager, + create_test_user: None, +) -> None: + """File owned by OTHER_USER_ID (2) — authenticated user (1) must receive 403.""" + async with session_manager.session() as session: + session.add( + UserFileDO( + id=OTHER_FILE_ID, + user_id=OTHER_USER_ID, + file_name="other.note", + directory_id=0, + ) + ) + await session.commit() + + resp = await client.post( + OCR_ENDPOINT, json={"fileId": OTHER_FILE_ID}, headers=auth_headers + ) + assert resp.status == 403 + + +# --------------------------------------------------------------------------- +# 404: user not found +# --------------------------------------------------------------------------- + + +async def test_ocr_list_user_not_found( + client: TestClient, + auth_headers: dict[str, str], +) -> None: + with patch( + "supernote.server.services.user.UserService.get_user_id", + new_callable=AsyncMock, + return_value=None, + ): + resp = await client.post( + OCR_ENDPOINT, json={"fileId": FILE_ID}, headers=auth_headers + ) + assert resp.status == 404 + + +# --------------------------------------------------------------------------- +# 404: file not found +# --------------------------------------------------------------------------- + + +async def test_ocr_list_not_found_for_unknown_file( + client: TestClient, + auth_headers: dict[str, str], + create_test_user: None, +) -> None: + resp = await client.post(OCR_ENDPOINT, json={"fileId": 99999}, headers=auth_headers) + assert resp.status == 404 + + +# --------------------------------------------------------------------------- +# 400: malformed / missing body +# --------------------------------------------------------------------------- + + +async def test_ocr_list_invalid_json( + client: TestClient, auth_headers: dict[str, str] +) -> None: + resp = await client.post( + OCR_ENDPOINT, + data=b"not json", + headers={**auth_headers, "Content-Type": "application/json"}, + ) + assert resp.status == 400 + + +async def test_ocr_list_missing_file_id( + client: TestClient, auth_headers: dict[str, str] +) -> None: + resp = await client.post( + OCR_ENDPOINT, json={"unexpected": True}, headers=auth_headers + ) + assert resp.status == 400 + + +# --------------------------------------------------------------------------- +# 500: generic unexpected error +# --------------------------------------------------------------------------- + + +async def test_ocr_list_generic_error( + client: TestClient, auth_headers: dict[str, str], create_test_user: None +) -> None: + with patch( + "supernote.server.services.user.UserService.get_user_id", + new_callable=AsyncMock, + side_effect=RuntimeError("boom"), + ): + resp = await client.post( + OCR_ENDPOINT, json={"fileId": FILE_ID}, headers=auth_headers + ) + assert resp.status == 500 diff --git a/tests/server/services/test_file_service.py b/tests/server/services/test_file_service.py new file mode 100644 index 00000000..0f48c8c7 --- /dev/null +++ b/tests/server/services/test_file_service.py @@ -0,0 +1,269 @@ +"""Tests for FileService.convert_note_to_png — caching, cleanup, and fail-open behaviour. + +Per constitution §VI: these tests are written BEFORE implementation and must FAIL +until the caching logic is added to convert_note_to_png. +""" + +from collections.abc import AsyncGenerator +from pathlib import Path +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from supernote.server.db.models.file import UserFileDO +from supernote.server.db.session import DatabaseSessionManager +from supernote.server.services.blob import LocalBlobStorage +from supernote.server.services.file import FileService +from supernote.server.services.user import UserService +from supernote.server.utils.paths import get_conversion_png_path + +USER_EMAIL = "test@example.com" +USER_ID = 1 +FILE_ID = 42 +CURRENT_MD5 = "abc123def456" +OLD_MD5 = "111222333444" +NOTE_STORAGE_KEY = "note-key-xyz" +TOTAL_PAGES = 2 + + +@pytest.fixture +def file_service( + storage_root: Path, + blob_storage: LocalBlobStorage, + user_service: UserService, + session_manager: DatabaseSessionManager, +) -> FileService: + return FileService(storage_root, blob_storage, user_service, session_manager) + + +async def _note_bytes_gen( + *args: object, **kwargs: object +) -> AsyncGenerator[bytes, None]: + """Async generator yielding fake note bytes.""" + yield b"FAKE_NOTE_BYTES" + + +def _make_mock_note(total_pages: int = TOTAL_PAGES) -> MagicMock: + note = MagicMock() + note.get_total_pages.return_value = total_pages + return note + + +async def _seed_file( + session_manager: DatabaseSessionManager, + md5: str = CURRENT_MD5, + last_conversion_md5: str | None = None, +) -> None: + async with session_manager.session() as session: + session.add( + UserFileDO( + id=FILE_ID, + user_id=USER_ID, + file_name="test.note", + directory_id=0, + md5=md5, + storage_key=NOTE_STORAGE_KEY, + last_conversion_md5=last_conversion_md5, + ) + ) + await session.commit() + + +# --------------------------------------------------------------------------- +# T003a: Skip conversion when all pages already exist in cache +# --------------------------------------------------------------------------- + + +async def test_convert_skips_put_when_all_pages_cached( + file_service: FileService, + session_manager: DatabaseSessionManager, + create_test_user: None, +) -> None: + """When exists() returns True for every page, put() must never be called.""" + await _seed_file(session_manager, md5=CURRENT_MD5, last_conversion_md5=CURRENT_MD5) + + mock_put = AsyncMock() + mock_exists = AsyncMock(return_value=True) + + with ( + patch.object(file_service.blob_storage, "get", new=_note_bytes_gen), + patch.object(file_service.blob_storage, "exists", mock_exists), + patch.object(file_service.blob_storage, "put", mock_put), + patch( + "supernote.server.services.file.load_notebook", + return_value=_make_mock_note(), + ), + patch("supernote.server.services.file.ImageConverter"), + ): + results = await file_service.convert_note_to_png(USER_EMAIL, FILE_ID) + + mock_put.assert_not_called() + assert len(results) == TOTAL_PAGES + + +# --------------------------------------------------------------------------- +# T003b: Convert and store when pages are not yet cached +# --------------------------------------------------------------------------- + + +async def test_convert_calls_put_when_page_not_cached( + file_service: FileService, + session_manager: DatabaseSessionManager, + create_test_user: None, +) -> None: + """When exists() returns False, put() must be called for each missing page.""" + await _seed_file(session_manager, md5=CURRENT_MD5, last_conversion_md5=None) + + mock_put = AsyncMock() + mock_exists = AsyncMock(return_value=False) + + with ( + patch.object(file_service.blob_storage, "get", new=_note_bytes_gen), + patch.object(file_service.blob_storage, "exists", mock_exists), + patch.object(file_service.blob_storage, "put", mock_put), + patch( + "supernote.server.services.file.load_notebook", + return_value=_make_mock_note(), + ), + patch("supernote.server.services.file.ImageConverter"), + ): + results = await file_service.convert_note_to_png(USER_EMAIL, FILE_ID) + + assert mock_put.call_count == TOTAL_PAGES + assert len(results) == TOTAL_PAGES + + +# --------------------------------------------------------------------------- +# T003c: Fail-open — storage error on exists() treated as cache miss +# --------------------------------------------------------------------------- + + +async def test_convert_failopen_on_exists_error( + file_service: FileService, + session_manager: DatabaseSessionManager, + create_test_user: None, +) -> None: + """When exists() raises OSError, the page must still be converted (fail-open).""" + await _seed_file(session_manager, md5=CURRENT_MD5, last_conversion_md5=CURRENT_MD5) + + mock_put = AsyncMock() + mock_exists = AsyncMock(side_effect=OSError("storage unavailable")) + + with ( + patch.object(file_service.blob_storage, "get", new=_note_bytes_gen), + patch.object(file_service.blob_storage, "exists", mock_exists), + patch.object(file_service.blob_storage, "put", mock_put), + patch( + "supernote.server.services.file.load_notebook", + return_value=_make_mock_note(), + ), + patch("supernote.server.services.file.ImageConverter"), + ): + results = await file_service.convert_note_to_png(USER_EMAIL, FILE_ID) + + # Fail-open: put must still be called for each page despite exists() failing + assert mock_put.call_count == TOTAL_PAGES + assert len(results) == TOTAL_PAGES + + +# --------------------------------------------------------------------------- +# T003d: Old-image cleanup when last_conversion_md5 differs from current md5 +# --------------------------------------------------------------------------- + + +async def test_convert_deletes_old_images_on_md5_change( + file_service: FileService, + session_manager: DatabaseSessionManager, + create_test_user: None, +) -> None: + """When MD5 changed, delete() must be called for each old-hash page image.""" + await _seed_file(session_manager, md5=CURRENT_MD5, last_conversion_md5=OLD_MD5) + + mock_delete = AsyncMock() + mock_put = AsyncMock() + mock_exists = AsyncMock(return_value=False) + + with ( + patch.object(file_service.blob_storage, "get", new=_note_bytes_gen), + patch.object(file_service.blob_storage, "exists", mock_exists), + patch.object(file_service.blob_storage, "put", mock_put), + patch.object(file_service.blob_storage, "delete", mock_delete), + patch( + "supernote.server.services.file.load_notebook", + return_value=_make_mock_note(), + ), + patch("supernote.server.services.file.ImageConverter"), + ): + await file_service.convert_note_to_png(USER_EMAIL, FILE_ID) + + expected_old_keys = [ + get_conversion_png_path( + user_id=USER_ID, file_id=FILE_ID, page_index=i, file_md5=OLD_MD5 + ) + for i in range(TOTAL_PAGES) + ] + actual_delete_keys = [call.args[1] for call in mock_delete.call_args_list] + assert sorted(actual_delete_keys) == sorted(expected_old_keys) + + +async def test_convert_does_not_delete_when_md5_unchanged( + file_service: FileService, + session_manager: DatabaseSessionManager, + create_test_user: None, +) -> None: + """When MD5 unchanged, delete() must NOT be called.""" + await _seed_file(session_manager, md5=CURRENT_MD5, last_conversion_md5=CURRENT_MD5) + + mock_delete = AsyncMock() + mock_exists = AsyncMock(return_value=True) + + with ( + patch.object(file_service.blob_storage, "get", new=_note_bytes_gen), + patch.object(file_service.blob_storage, "exists", mock_exists), + patch.object(file_service.blob_storage, "put", AsyncMock()), + patch.object(file_service.blob_storage, "delete", mock_delete), + patch( + "supernote.server.services.file.load_notebook", + return_value=_make_mock_note(), + ), + patch("supernote.server.services.file.ImageConverter"), + ): + await file_service.convert_note_to_png(USER_EMAIL, FILE_ID) + + mock_delete.assert_not_called() + + +# --------------------------------------------------------------------------- +# T003e: last_conversion_md5 updated after successful conversion +# --------------------------------------------------------------------------- + + +async def test_convert_updates_last_conversion_md5( + file_service: FileService, + session_manager: DatabaseSessionManager, + create_test_user: None, +) -> None: + """After converting, last_conversion_md5 must equal the file's current md5.""" + await _seed_file(session_manager, md5=CURRENT_MD5, last_conversion_md5=None) + + with ( + patch.object(file_service.blob_storage, "get", new=_note_bytes_gen), + patch.object( + file_service.blob_storage, "exists", AsyncMock(return_value=False) + ), + patch.object(file_service.blob_storage, "put", AsyncMock()), + patch( + "supernote.server.services.file.load_notebook", + return_value=_make_mock_note(), + ), + patch("supernote.server.services.file.ImageConverter"), + ): + await file_service.convert_note_to_png(USER_EMAIL, FILE_ID) + + async with session_manager.session() as session: + from sqlalchemy import select + + node = ( + await session.execute(select(UserFileDO).where(UserFileDO.id == FILE_ID)) + ).scalar_one() + assert node.last_conversion_md5 == CURRENT_MD5 diff --git a/uv.lock b/uv.lock index 80f39c97..455e461b 100644 --- a/uv.lock +++ b/uv.lock @@ -1447,7 +1447,7 @@ wheels = [ [[package]] name = "supernote" -version = "1.0.1" +version = "1.1.0" source = { editable = "." } dependencies = [ { name = "colour" },