Kalle/ATM
1
0
Fork 0
Code Issues Pull requests 14 Projects Releases 13 Packages Wiki Activity Actions

fix(pdf-import): stable lesson ids across re-imports (P0 data-loss) #76

Open
KnallerKalle wants to merge 4 commits from fix/pdf-reimport-stable-lesson-ids into dev
pull from: fix/pdf-reimport-stable-lesson-ids
merge into: Kalle:dev
Conversation 0 Commits 4 Files changed 7 +148 -29
KnallerKalle commented 2026-08-14 22:19:12 +00:00 (Migrated from github.com)
Copy link

Plan

Fix the P0 data-loss bug where re-importing the same PDF minted fresh randomUUID() lesson ids — replacing (not updating) the document's lessons and orphaning every user reference to them (explored/progress/share records) on every re-upload — and make blackbox 16b green on the testing deployment.

Outcome

  • Lesson ids extracted from a PDF are now derived from document identity: pdf-<docHash>-<number> via new stablePdfLessonId() / isStablePdfLessonId() in src/lib/pdf-import-reconcile.ts. A re-import updates the document's lessons in place.
  • pdf-process-smart/route.ts drops its per-import randomUUID() and refuses (logged error, lesson skipped) to persist any id that does not validate as stable — a guard against future regression to non-deterministic ids.
  • Blackbox suite gains 16e: re-import must preserve lesson ids (identity stability), not merely the lesson count. This is the exact hole that let the old code pass 16d while still destroying identity.
  • TDD: RED first (5 failing assertions committed alone), then implementation.

Impact on release goals

  • Release Hardening: "PDF re-import is non-destructive AND identity-stable" → [x].
  • Cross-run lesson continuity restored: a student who explored a PDF lesson keeps their explored/share history when the teacher re-uploads the same file.
  • Blackbox suite 47 → 48 checks; unit suite 176/176 (+11).

Blackbox

  • python3 scripts/blackbox_test.py --base http://127.0.0.1:3210 (real Next dev + real FastAPI + real pdfplumber extraction) → 47/48 green, incl. 16a–16e (non-destruction and identity stability).
  • Result: the single red is the pre-existing, documented 6b — the local stub TTS returns a 44-char placeholder audio, a local-env artifact unrelated to this fix (testing uses the real backend and passes 6b).
  • python3 scripts/blackbox_test.py --target testing (pre-deploy baseline) → 46/48: 16b + 16e RED as expected — the currently deployed code destroys identity; both go green once this merges and deploys. This is the P0 this PR fixes.
  • Unit: 176/176 green · tsc --noEmit clean.

Six-perspectives check

  • Product: protects the felt-change anchor (share notes / explored history survive a teacher's re-upload) — the viral hook no longer silently loses its history.
  • Engineering: minimal diff (2 prod files + 1 test file + suite extension); no new deps; no silent failures (unstable id is logged and refused).
  • Psychology: identity continuity = investment continuity; no judgement/strain language touched.
  • Biomechanics / Anatomy: no cue or content changes.
  • Feldenkrais: no content or framing changes; the lesson is never "finished" — its records now aren't either.

Risk

  • One-time, expected migration cost: the first re-import of a previously-imported document after deploy swaps that document's ids one last time (pre-fix copies had random ids with no derivable identity); identity is stable forever after. Strictly better than the status quo, which re-destroyed identity on every re-import.
  • Until this merges and deploys, the P0 remains live on testing — do not upload PDFs there before then.

Checklist

  • Branch off latest dev (d8b4b57)
  • Conventional Commits (test: RED → fix: implementation → docs: bookkeeping)
  • Patchlog entry appended (2026-08-14)
  • Roadmap status updated
  • No secrets / no large binaries

This PR was created by an AI agent (OpenHands) on behalf of the ATM team.

## Plan Fix the P0 data-loss bug where re-importing the same PDF minted fresh `randomUUID()` lesson ids — replacing (not updating) the document's lessons and orphaning every user reference to them (explored/progress/share records) on every re-upload — and make blackbox 16b green on the testing deployment. ## Outcome - Lesson ids extracted from a PDF are now derived from document identity: `pdf-<docHash>-<number>` via new `stablePdfLessonId()` / `isStablePdfLessonId()` in `src/lib/pdf-import-reconcile.ts`. A re-import updates the document's lessons **in place**. - `pdf-process-smart/route.ts` drops its per-import `randomUUID()` and refuses (logged error, lesson skipped) to persist any id that does not validate as stable — a guard against future regression to non-deterministic ids. - Blackbox suite gains **16e**: re-import must preserve lesson *ids* (identity stability), not merely the lesson count. This is the exact hole that let the old code pass 16d while still destroying identity. - TDD: RED first (5 failing assertions committed alone), then implementation. ## Impact on release goals - Release Hardening: **"PDF re-import is non-destructive AND identity-stable" → `[x]`**. - Cross-run lesson continuity restored: a student who explored a PDF lesson keeps their explored/share history when the teacher re-uploads the same file. - Blackbox suite 47 → 48 checks; unit suite 176/176 (+11). ## Blackbox - [x] `python3 scripts/blackbox_test.py --base http://127.0.0.1:3210` (real Next dev + real FastAPI + real pdfplumber extraction) → **47/48 green**, incl. 16a–16e (non-destruction **and** identity stability). - Result: the single red is the pre-existing, documented **6b** — the local stub TTS returns a 44-char placeholder audio, a local-env artifact unrelated to this fix (testing uses the real backend and passes 6b). - [x] `python3 scripts/blackbox_test.py --target testing` (pre-deploy baseline) → 46/48: **16b + 16e RED as expected** — the currently deployed code destroys identity; both go green once this merges and deploys. This is the P0 this PR fixes. - Unit: 176/176 green · `tsc --noEmit` clean. ## Six-perspectives check - [x] **Product:** protects the felt-change anchor (share notes / explored history survive a teacher's re-upload) — the viral hook no longer silently loses its history. - [x] **Engineering:** minimal diff (2 prod files + 1 test file + suite extension); no new deps; no silent failures (unstable id is logged and refused). - [x] **Psychology:** identity continuity = investment continuity; no judgement/strain language touched. - [x] **Biomechanics / Anatomy:** no cue or content changes. - [x] **Feldenkrais:** no content or framing changes; the lesson is never "finished" — its *records* now aren't either. ## Risk - One-time, expected migration cost: the **first** re-import of a previously-imported document after deploy swaps that document's ids one last time (pre-fix copies had random ids with no derivable identity); identity is stable forever after. Strictly better than the status quo, which re-destroyed identity on *every* re-import. - Until this merges and deploys, the P0 remains live on testing — do not upload PDFs there before then. ## Checklist - [x] Branch off latest `dev` (d8b4b57) - [x] Conventional Commits (`test:` RED → `fix:` implementation → `docs:` bookkeeping) - [x] Patchlog entry appended (2026-08-14) - [x] Roadmap status updated - [x] No secrets / no large binaries --- *This PR was created by an AI agent (OpenHands) on behalf of the ATM team.*
This pull request has changes conflicting with the target branch.
  • .development/automation-state.json
  • .development/patchlog.md
  • .development/roadmap.md
  • scripts/blackbox_test.py
View command line instructions

Manual merge helper

Use this merge commit message when completing the merge manually.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin fix/pdf-reimport-stable-lesson-ids:fix/pdf-reimport-stable-lesson-ids
git switch fix/pdf-reimport-stable-lesson-ids

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch dev
git merge --no-ff fix/pdf-reimport-stable-lesson-ids
git switch fix/pdf-reimport-stable-lesson-ids
git rebase dev
git switch dev
git merge --ff-only fix/pdf-reimport-stable-lesson-ids
git switch fix/pdf-reimport-stable-lesson-ids
git rebase dev
git switch dev
git merge --no-ff fix/pdf-reimport-stable-lesson-ids
git switch dev
git merge --squash fix/pdf-reimport-stable-lesson-ids
git switch dev
git merge --ff-only fix/pdf-reimport-stable-lesson-ids
git switch dev
git merge fix/pdf-reimport-stable-lesson-ids
git push origin dev
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
backend
Backend development tasks

  
bug
Something isn't working

  
completed
Features and tasks that are complete and merged

  
documentation
Improvements or additions to documentation

  
duplicate
This issue or pull request already exists

  
enhancement
New feature or request

  
feature
New functionality

  
frontend
Frontend development

  
good first issue
Good for newcomers

  
help wanted
Extra attention is needed

  
in-progress

   
invalid
This doesn't seem right

  
performance
Performance improvements

  
phase-1
Phase 1 Foundation tasks

  
Phase1
Phase 1 Foundation work

  
Phase2
Phase 2 Core Features

  
Phase3
Phase 3 AI Generation

  
planning
Planning and roadmap items

  
question
Further information is requested

  
refactor
Code refactoring

  
security
Security issues

  
test
Testing related

  
testing
Testing and verification tasks

  
wontfix
This will not be worked on

No labels
backend
bug
completed
documentation
duplicate
enhancement
feature
frontend
good first issue
help wanted
in-progress
invalid
performance
phase-1
Phase1
Phase2
Phase3
planning
question
refactor
security
test
testing
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Kalle/ATM!76
No description provided.