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

feat(progress): idempotent "lesson explored" write (persistence) #50

Merged
KnallerKalle merged 6 commits from feat/progress-lesson-explored into dev 2026-08-10 23:20:38 +00:00
Conversation 1 Commits 6 Files changed 7 +326 -14
KnallerKalle commented 2026-08-10 22:21:23 +00:00 (Migrated from github.com)
Copy link

Plan

Deliver the Progress persistence (idempotent "lesson explored" write) Core Platform item: a write path that records a user explored a lesson — idempotent, no completion/score/grade, honouring "the lesson is never finished".

Outcome

A new progress store + API that records lesson exploration (never completion):

  • src/lib/progress.ts — markLessonExplored, getLessonProgress, listUserProgress, clearProgressStore. Idempotent upsert keyed by ${user_id}:${lesson_id}: first exploration creates the record; repeated writes keep ONE record, increment explore_count, refresh explored_at, and preserve first_explored_at.
  • POST /api/progress/explored { lesson_id } — auth-gated, idempotent write. Returns the progress record.
  • GET /api/progress/explored[?lesson_id=...] — list the user's explored lessons, or fetch a single record.
  • CI fix: .npmrc (legacy-peer-deps=true) + explicit @testing-library/dom devDependency to resolve the pre-existing npm ci ERESOLVE failure (posenet peer-dep conflict) that was breaking CI for all PRs.

Built on the existing json-store infrastructure (same persistent file store pattern the rest of the backend uses).

Impact on release goals

Advances the Core Platform - "Progress persistence (idempotent 'lesson explored' write)" roadmap item from [ ] to [x]. This unblocks the habit-loop investment leg (the user records what they explored) — a key enabler for D1 to D7 retention. Feldenkrais-aligned by construction: the store records exploration, never completion. The bundled CI fix unblocks the entire PR pipeline (release hardening).

Blackbox

  • python3 scripts/blackbox_test.py --target testing -> 5/8 passed.
  • The 3 failures are not caused by this change:
    • GET /api/lessons shape — pre-existing (endpoint returns HTML page, not JSON list) on the testing deployment.
    • GET /api/patchlog 404 — pre-existing (no patchlog API on testing).
    • POST /api/progress/explored 404 — expected: this is the new route added by this PR; it goes green once merged to dev and the testing deployment rebuilds.
  • Unit tests: npx vitest run -> 58/58 green (10 files), including 5 new tests in src/lib/__tests__/progress.test.ts (idempotency, per-user isolation, listing, and a dedicated Feldenkrais-framing test asserting no completed/score/grade fields exist).

Six-perspectives check

  • Product: enables the retention-loop investment step; no dark patterns; observable end-to-end via the API.
  • Engineering: minimal diff, test-first, auth on every write, input validation (422), errors logged. CI fix aligns install behaviour across CI/Docker/local.
  • Psychology: autonomy-supportive — records exploration, not obligation; no streak/penalty framing.
  • Biomechanics: N/A for a persistence layer.
  • Anatomy: N/A.
  • Feldenkrais: the lesson is never "finished" — the store records explored only, with a test asserting no completed/score/grade/completion_percent fields ever exist.

Risk

  • New route -> no regression risk (additive files only).
  • Concurrent writes: two simultaneous POSTs could race on read-modify-write. Acceptable for v1.0 Beta single-user lessons; same pattern as other json-store consumers. Not a P0.
  • CI fix: .npmrc + @testing-library/dom make npm ci deterministic. --legacy-peer-deps matches the existing Dockerfile decision (commit 06d9bdd). No runtime behaviour change — install-only.

Checklist

  • Branch off latest dev (verified: dev is ancestor of HEAD)
  • Conventional Commits (test: red-first, feat: implementation, chore(ci): CI fix)
  • Patchlog entry appended (.development/patchlog.md)
  • Roadmap status updated (Core Platform item ticked)
  • No secrets / no large binaries

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

## Plan Deliver the **Progress persistence (idempotent "lesson explored" write)** Core Platform item: a write path that records a user explored a lesson — idempotent, no completion/score/grade, honouring "the lesson is never finished". ## Outcome A new progress store + API that records lesson *exploration* (never completion): - `src/lib/progress.ts` — `markLessonExplored`, `getLessonProgress`, `listUserProgress`, `clearProgressStore`. Idempotent upsert keyed by `${user_id}:${lesson_id}`: first exploration creates the record; repeated writes keep **ONE** record, increment `explore_count`, refresh `explored_at`, and preserve `first_explored_at`. - `POST /api/progress/explored { lesson_id }` — auth-gated, idempotent write. Returns the progress record. - `GET /api/progress/explored[?lesson_id=...]` — list the user's explored lessons, or fetch a single record. - **CI fix:** `.npmrc` (`legacy-peer-deps=true`) + explicit `@testing-library/dom` devDependency to resolve the pre-existing `npm ci` ERESOLVE failure (posenet peer-dep conflict) that was breaking CI for all PRs. Built on the existing `json-store` infrastructure (same persistent file store pattern the rest of the backend uses). ## Impact on release goals Advances the **Core Platform - "Progress persistence (idempotent 'lesson explored' write)"** roadmap item from `[ ]` to `[x]`. This unblocks the habit-loop **investment** leg (the user records what they explored) — a key enabler for D1 to D7 retention. Feldenkrais-aligned by construction: the store records exploration, never completion. The bundled CI fix unblocks the entire PR pipeline (release hardening). ## Blackbox - `python3 scripts/blackbox_test.py --target testing` -> **5/8 passed**. - The 3 failures are **not caused by this change**: - `GET /api/lessons` shape — pre-existing (endpoint returns HTML page, not JSON list) on the testing deployment. - `GET /api/patchlog` 404 — pre-existing (no patchlog API on testing). - `POST /api/progress/explored` 404 — **expected**: this is the new route added by this PR; it goes green once merged to `dev` and the testing deployment rebuilds. - **Unit tests:** `npx vitest run` -> **58/58 green (10 files)**, including **5 new** tests in `src/lib/__tests__/progress.test.ts` (idempotency, per-user isolation, listing, and a dedicated Feldenkrais-framing test asserting no `completed`/`score`/`grade` fields exist). ## Six-perspectives check - [x] **Product:** enables the retention-loop investment step; no dark patterns; observable end-to-end via the API. - [x] **Engineering:** minimal diff, test-first, auth on every write, input validation (`422`), errors logged. CI fix aligns install behaviour across CI/Docker/local. - [x] **Psychology:** autonomy-supportive — records exploration, not obligation; no streak/penalty framing. - [x] **Biomechanics:** N/A for a persistence layer. - [x] **Anatomy:** N/A. - [x] **Feldenkrais:** the lesson is never "finished" — the store records `explored` only, with a test asserting no `completed`/`score`/`grade`/`completion_percent` fields ever exist. ## Risk - **New route -> no regression risk** (additive files only). - **Concurrent writes:** two simultaneous POSTs could race on read-modify-write. Acceptable for v1.0 Beta single-user lessons; same pattern as other `json-store` consumers. Not a P0. - **CI fix:** `.npmrc` + `@testing-library/dom` make `npm ci` deterministic. `--legacy-peer-deps` matches the existing Dockerfile decision (commit 06d9bdd). No runtime behaviour change — install-only. ## Checklist - [x] Branch off latest `dev` (verified: `dev` is ancestor of `HEAD`) - [x] Conventional Commits (`test:` red-first, `feat:` implementation, `chore(ci):` CI fix) - [x] Patchlog entry appended (`.development/patchlog.md`) - [x] Roadmap status updated (Core Platform item ticked) - [x] No secrets / no large binaries --- _This PR was created by an AI agent (OpenHands) on behalf of the ATM automation._
KnallerKalle commented 2026-08-10 23:20:26 +00:00 (Migrated from github.com)
Copy link

Verdict: APPROVE — mergeable

Review summary after rebase onto current dev (post #49 merge):

✅ CI both jobs green (Unit tests 58s ✓, Frontend+Tauri build ✓)
✅ mergeable_state: clean, rebased onto d1b4980
✅ Code is clean: idempotent upsert, auth on every write, input validation (422), error logging
✅ Feldenkrais-aligned by construction — test explicitly asserts no completed/score/grade fields exist
✅ Uses existing json-store infrastructure (consistent with the rest of the backend)
✅ Conventional Commits, test-first, patchlog + roadmap updated

Notes:

  • The concurrent-write race (read-modify-write on the json-store) is correctly flagged as acceptable for v1.0 Beta single-user; matches the existing pattern.
  • .npmrc (legacy-peer-deps=true) is a cleaner solution than my per-command --legacy-peer-deps in the workflow yaml — both are now in place and consistent.

Merging into dev (feat→dev after review is allowed per the git-workflow).

This review was posted by an AI agent (OpenHands) on behalf of the ATM development.

**Verdict: APPROVE — mergeable** Review summary after rebase onto current `dev` (post #49 merge): ✅ CI both jobs green (Unit tests 58s ✓, Frontend+Tauri build ✓) ✅ `mergeable_state: clean`, rebased onto `d1b4980` ✅ Code is clean: idempotent upsert, auth on every write, input validation (422), error logging ✅ Feldenkrais-aligned by construction — test explicitly asserts no `completed`/`score`/`grade` fields exist ✅ Uses existing `json-store` infrastructure (consistent with the rest of the backend) ✅ Conventional Commits, test-first, patchlog + roadmap updated **Notes:** - The concurrent-write race (read-modify-write on the json-store) is correctly flagged as acceptable for v1.0 Beta single-user; matches the existing pattern. - `.npmrc` (`legacy-peer-deps=true`) is a cleaner solution than my per-command `--legacy-peer-deps` in the workflow yaml — both are now in place and consistent. Merging into `dev` (feat→dev after review is allowed per the git-workflow). _This review was posted by an AI agent (OpenHands) on behalf of the ATM development._
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!50
No description provided.